[NETMAP,1/4] Update to netmap_v11.1

Message ID	1415727359-22406-2-git-send-email-ciprian.barbu@linaro.org
State	New
Headers	show Return-Path: <patchwork-forward+bncBC42RN5IWUGBBHUSRGRQKGQEGQUR5LA@linaro.org> Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.46 as permitted sender) client-ip=209.85.215.46; Received-SPF: none (google.com: lng-odp-bounces@lists.linaro.org does not designate permitted sender hosts) client-ip=54.225.227.206; From: Ciprian Barbu <ciprian.barbu@linaro.org> To: lng-odp@lists.linaro.org Date: Tue, 11 Nov 2014 19:35:56 +0200 Message-Id: <1415727359-22406-2-git-send-email-ciprian.barbu@linaro.org> In-Reply-To: <1415727359-22406-1-git-send-email-ciprian.barbu@linaro.org> References: <1415727359-22406-1-git-send-email-ciprian.barbu@linaro.org> Subject: [lng-odp] [PATCH NETMAP 1/4] Update to netmap_v11.1 Precedence: list MIME-Version: 1.0 Errors-To: lng-odp-bounces@lists.linaro.org Sender: lng-odp-bounces@lists.linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit

Ciprian Barbu Nov. 11, 2014, 5:35 p.m. UTC

Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
---
 platform/linux-netmap/README                      | 23 ++++++++++-------
 platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
 platform/linux-netmap/odp_packet_netmap.c         | 30 +++++++++++------------
 3 files changed, 39 insertions(+), 34 deletions(-)

Mike Holmes Nov. 12, 2014, 8:27 p.m. UTC | #1

This does not build as a standalone patch, how can I be sure any failures
are related to the jump to v11.1 of netmap vs your other changes ? Can this
be done separately ?

On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:

> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
> ---
>  platform/linux-netmap/README                      | 23 ++++++++++-------
>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>  platform/linux-netmap/odp_packet_netmap.c         | 30
> +++++++++++------------
>  3 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
> index abcb187..3718072 100644
> --- a/platform/linux-netmap/README
> +++ b/platform/linux-netmap/README
> @@ -25,20 +25,18 @@ at runtime.
>  2.1 Building netmap
>  --------------------
>
> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
> -ODP only works at this point with netmap API version 10 so you will need
> -a specific revision of netmap.
> +Netmap is currently hosted on https://code.google.com/p/netmap/
>
>      git clone https://code.google.com/p/netmap/
>      cd netmap
> -    git reset --hard 1f462ef
> +    git co v11.1
>
>  Netmap consists of a core kernel module (netmap_lin.ko) and optional
> modified
>  device drivers.
>
>  Netmap builds as an out-of-tree kernel module, you need matching kernel
> sources
>  to compile it. General build instructions can be found in the packet
> README:
> -http://code.google.com/p/netmap/source/browse/README
> +https://code.google.com/p/netmap/source/browse/LINUX/README
>
>  2.1.1 Building netmap on Ubuntu with stock kernel
>
> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
> convenient place.
>  Now compile netmap:
>
>      cd LINUX
> -    make SRC=<path_to_kernel_source>
> +    ./configure --kernel-sources=<path_to_kernel_src>
> +    make
>
>  2.1.2 Building netmap for kernel built from sources
>
>      cd LINUX
> -    make KSRC=<path_to_kernel_source>
> +    ./configure --kernel-dir=<path_to_kernel_tree>
> +    make
>
>  2.2 Building ODP
>  ----------------
> @@ -69,13 +69,18 @@ Now compile netmap:
>  The default platform is linux-netmap, if you want linux-generic you need
> to
>  specify it to configure --with-platform.
>
> +ODP works with the latest release version of netmap, which is currently
> at API
> +version 11.1. There is one problem though, the default CFLAGS that ODP is
> built
> +with contains -Werror=cast-qual. A temporary workaround is be to disable
> this
> +flag using -Wno-cast-qual.
> +
>  The --with-sdk-install-path can be used to point to the netmap sources.
>
>      ./bootstrap
>
> -    ./configure
> +    ./configure CFLAGS='-Wno-cast-qual'
>  or
> -    ./configure --with-sdk-install-path=<netmap>
> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>
>  To configure ODP for linux-generic:
>      ./configure --with-platform=linux-generic
> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
> b/platform/linux-netmap/include/odp_packet_netmap.h
> index b4c523f..78b2379 100644
> --- a/platform/linux-netmap/include/odp_packet_netmap.h
> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
> @@ -26,17 +26,17 @@
>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>  typedef struct {
>         odp_buffer_pool_t pool;
> -       size_t max_frame_len; /**< max frame len = buf_size -
> sizeof(pkt_hdr) */
> -       size_t frame_offset; /**< frame start offset from start of pkt buf
> */
> -       size_t buf_size; /**< size of buffer payload in 'pool' */
> -       struct nm_desc_t *nm_desc;
> -       uint32_t begin;
> -       uint32_t end;
> -       struct netmap_ring *rxring;
> -       struct netmap_ring *txring;
> -       odp_queue_t tx_access; /* Used for exclusive access to send
> packets */
> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> +       size_t frame_offset;    /**< Frame start offset from start of pkt
> buf */
> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
> +       uint32_t begin;         /**< First ring to poll */
> +       uint32_t end;           /**< Last ring to poll */
> +       struct netmap_ring *rxring; /**< First rx ring */
> +       struct netmap_ring *txring; /**< First tx ring */
> +       odp_queue_t tx_access;  /** Used for exclusive access to send
> packets */
>         uint32_t if_flags;
> -       char ifname[32];
> +       char ifname[IFNAMSIZ];
>  } pkt_netmap_t;
>
>  /**
> diff --git a/platform/linux-netmap/odp_packet_netmap.c
> b/platform/linux-netmap/odp_packet_netmap.c
> index 6243040..ec4e0ea 100644
> --- a/platform/linux-netmap/odp_packet_netmap.c
> +++ b/platform/linux-netmap/odp_packet_netmap.c
> @@ -107,7 +107,7 @@ done:
>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>                      odp_buffer_pool_t pool)
>  {
> -       char ifname[32];
> +       char ifname[IFNAMSIZ];
>         odp_packet_t pkt;
>         uint8_t *pkt_buf;
>         uint8_t *l2_hdr;
> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
> const char *netdev,
>
>         odph_packet_free(pkt);
>
> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>
> -       if (pkt_nm->nm_desc == NULL) {
> +       if (pkt_nm->desc == NULL) {
>                 ODP_ERR("Error opening nm interface: %s\n",
> strerror(errno));
>                 return -1;
>         }
>
>         ODP_DBG("thread %d mmap addr %p\n",
>                 odp_thread_id(),
> -               pkt_nm->nm_desc->mem);
> +               pkt_nm->desc->mem);
>
>         pkt_nm->begin  = 0;
> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>
>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>         if (ret)
> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
> const char *netdev,
>
>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>  {
> -       if (pkt_nm->nm_desc != NULL) {
> -               nm_close(pkt_nm->nm_desc);
> -               pkt_nm->nm_desc = NULL;
> +       if (pkt_nm->desc != NULL) {
> +               nm_close(pkt_nm->desc);
> +               pkt_nm->desc = NULL;
>         }
>
>         return 0;
> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
> odp_packet_t pkt_table[],
>         int ret;
>  #endif
>
> -       fd = pkt_nm->nm_desc->fd;
> +       fd = pkt_nm->desc->fd;
>  #ifdef NETMAP_BLOCKING_IO
>         fds[0].fd = fd;
>         fds[0].events = POLLIN;
> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
> odp_packet_t pkt_table[],
>                                 break;
>                         }
>
> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
> ringid);
> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp, ringid);
>                 }
>
>                 limit = len - nb_rx;
> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
> odp_packet_t pkt_table[],
>         int ret;
>  #endif
>
> -       fd = pkt_nm->nm_desc->fd;
> +       fd = pkt_nm->desc->fd;
>  #ifdef NETMAP_BLOCKING_IO
>         fds[0].fd = fd;
>         fds[0].events = POLLOUT;
> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
> odp_packet_t pkt_table[],
>                                 break;
>                         }
>
> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
> ringid);
> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp, ringid);
>                 }
>
>                 limit = len - nb_tx;
> --
> 1.8.3.2
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>

Maxim Uvarov Nov. 13, 2014, 12:48 p.m. UTC | #2

On 11/11/2014 08:35 PM, Ciprian Barbu wrote:
> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
> ---
>   platform/linux-netmap/README                      | 23 ++++++++++-------
>   platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>   platform/linux-netmap/odp_packet_netmap.c         | 30 +++++++++++------------
>   3 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
> index abcb187..3718072 100644
> --- a/platform/linux-netmap/README
> +++ b/platform/linux-netmap/README
> @@ -25,20 +25,18 @@ at runtime.
>   2.1 Building netmap
>   --------------------
>   
> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
> -ODP only works at this point with netmap API version 10 so you will need
> -a specific revision of netmap.
> +Netmap is currently hosted on https://code.google.com/p/netmap/
>   
>       git clone https://code.google.com/p/netmap/
>       cd netmap
> -    git reset --hard 1f462ef
> +    git co v11.1
>   
>   Netmap consists of a core kernel module (netmap_lin.ko) and optional modified
>   device drivers.
>   
>   Netmap builds as an out-of-tree kernel module, you need matching kernel sources
>   to compile it. General build instructions can be found in the packet README:
> -http://code.google.com/p/netmap/source/browse/README
> +https://code.google.com/p/netmap/source/browse/LINUX/README
>   
>   2.1.1 Building netmap on Ubuntu with stock kernel
>   
> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a convenient place.
>   Now compile netmap:
>   
>       cd LINUX
> -    make SRC=<path_to_kernel_source>
> +    ./configure --kernel-sources=<path_to_kernel_src>
> +    make
>   
>   2.1.2 Building netmap for kernel built from sources
>   
>       cd LINUX
> -    make KSRC=<path_to_kernel_source>
> +    ./configure --kernel-dir=<path_to_kernel_tree>
> +    make
>   
>   2.2 Building ODP
>   ----------------
> @@ -69,13 +69,18 @@ Now compile netmap:
>   The default platform is linux-netmap, if you want linux-generic you need to
>   specify it to configure --with-platform.
>   
> +ODP works with the latest release version of netmap, which is currently at API
> +version 11.1. There is one problem though, the default CFLAGS that ODP is built
> +with contains -Werror=cast-qual. A temporary workaround is be to disable this
> +flag using -Wno-cast-qual.
> +

Can this problem be fixed with casting to void * first and after that to 
new type?

Maxim.


>   The --with-sdk-install-path can be used to point to the netmap sources.
>   
>       ./bootstrap
>   
> -    ./configure
> +    ./configure CFLAGS='-Wno-cast-qual'
>   or
> -    ./configure --with-sdk-install-path=<netmap>
> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>   
>   To configure ODP for linux-generic:
>       ./configure --with-platform=linux-generic
> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h b/platform/linux-netmap/include/odp_packet_netmap.h
> index b4c523f..78b2379 100644
> --- a/platform/linux-netmap/include/odp_packet_netmap.h
> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
> @@ -26,17 +26,17 @@
>   /** Packet socket using netmap mmaped rings for both Rx and Tx */
>   typedef struct {
>   	odp_buffer_pool_t pool;
> -	size_t max_frame_len; /**< max frame len = buf_size - sizeof(pkt_hdr) */
> -	size_t frame_offset; /**< frame start offset from start of pkt buf */
> -	size_t buf_size; /**< size of buffer payload in 'pool' */
> -	struct nm_desc_t *nm_desc;
> -	uint32_t begin;
> -	uint32_t end;
> -	struct netmap_ring *rxring;
> -	struct netmap_ring *txring;
> -	odp_queue_t tx_access; /* Used for exclusive access to send packets */
> +	size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> +	size_t frame_offset;	/**< Frame start offset from start of pkt buf */
> +	size_t buf_size;	/**< Size of buffer payload in 'pool' */
> +	struct nm_desc *desc;	/**< Netmap meta-data for the device */
> +	uint32_t begin;		/**< First ring to poll */
> +	uint32_t end;		/**< Last ring to poll */
> +	struct netmap_ring *rxring; /**< First rx ring */
> +	struct netmap_ring *txring; /**< First tx ring */
> +	odp_queue_t tx_access;	/** Used for exclusive access to send packets */
>   	uint32_t if_flags;
> -	char ifname[32];
> +	char ifname[IFNAMSIZ];
>   } pkt_netmap_t;
>   
>   /**
> diff --git a/platform/linux-netmap/odp_packet_netmap.c b/platform/linux-netmap/odp_packet_netmap.c
> index 6243040..ec4e0ea 100644
> --- a/platform/linux-netmap/odp_packet_netmap.c
> +++ b/platform/linux-netmap/odp_packet_netmap.c
> @@ -107,7 +107,7 @@ done:
>   int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>   		     odp_buffer_pool_t pool)
>   {
> -	char ifname[32];
> +	char ifname[IFNAMSIZ];
>   	odp_packet_t pkt;
>   	uint8_t *pkt_buf;
>   	uint8_t *l2_hdr;
> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>   
>   	odph_packet_free(pkt);
>   
> -	strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
> +	snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>   	snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
> -	pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
> +	pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>   
> -	if (pkt_nm->nm_desc == NULL) {
> +	if (pkt_nm->desc == NULL) {
>   		ODP_ERR("Error opening nm interface: %s\n", strerror(errno));
>   		return -1;
>   	}
>   
>   	ODP_DBG("thread %d mmap addr %p\n",
>   		odp_thread_id(),
> -		pkt_nm->nm_desc->mem);
> +		pkt_nm->desc->mem);
>   
>   	pkt_nm->begin  = 0;
> -	pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
> -	pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
> -	pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
> +	pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
> +	pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
> +	pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>   
>   	ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>   	if (ret)
> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>   
>   int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>   {
> -	if (pkt_nm->nm_desc != NULL) {
> -		nm_close(pkt_nm->nm_desc);
> -		pkt_nm->nm_desc = NULL;
> +	if (pkt_nm->desc != NULL) {
> +		nm_close(pkt_nm->desc);
> +		pkt_nm->desc = NULL;
>   	}
>   
>   	return 0;
> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>   	int ret;
>   #endif
>   
> -	fd = pkt_nm->nm_desc->fd;
> +	fd = pkt_nm->desc->fd;
>   #ifdef NETMAP_BLOCKING_IO
>   	fds[0].fd = fd;
>   	fds[0].events = POLLIN;
> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>   				break;
>   			}
>   
> -			rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, ringid);
> +			rxring = NETMAP_RXRING(pkt_nm->desc->nifp, ringid);
>   		}
>   
>   		limit = len - nb_rx;
> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>   	int ret;
>   #endif
>   
> -	fd = pkt_nm->nm_desc->fd;
> +	fd = pkt_nm->desc->fd;
>   #ifdef NETMAP_BLOCKING_IO
>   	fds[0].fd = fd;
>   	fds[0].events = POLLOUT;
> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>   				break;
>   			}
>   
> -			txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, ringid);
> +			txring = NETMAP_TXRING(pkt_nm->desc->nifp, ringid);
>   		}
>   
>   		limit = len - nb_tx;

Ciprian Barbu Nov. 13, 2014, 1:34 p.m. UTC | #3

On Wed, Nov 12, 2014 at 10:27 PM, Mike Holmes <mike.holmes@linaro.org> wrote:
> This does not build as a standalone patch, how can I be sure any failures
> are related to the jump to v11.1 of netmap vs your other changes ? Can this
> be done separately ?

Are you following the instructions in platform/linux-netmap/README ? I
just took odp-netmap fresh from git.linaro.org, applied this patch
only and built it.

./bootstrap
./configure --with-sdk-install-path=<netmap_path> CFLAGS='-Wno-cast-qual'
make

>
> On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
>>
>> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>> ---
>>  platform/linux-netmap/README                      | 23 ++++++++++-------
>>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>>  platform/linux-netmap/odp_packet_netmap.c         | 30
>> +++++++++++------------
>>  3 files changed, 39 insertions(+), 34 deletions(-)
>>
>> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
>> index abcb187..3718072 100644
>> --- a/platform/linux-netmap/README
>> +++ b/platform/linux-netmap/README
>> @@ -25,20 +25,18 @@ at runtime.
>>  2.1 Building netmap
>>  --------------------
>>
>> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>> -ODP only works at this point with netmap API version 10 so you will need
>> -a specific revision of netmap.
>> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>
>>      git clone https://code.google.com/p/netmap/
>>      cd netmap
>> -    git reset --hard 1f462ef
>> +    git co v11.1
>>
>>  Netmap consists of a core kernel module (netmap_lin.ko) and optional
>> modified
>>  device drivers.
>>
>>  Netmap builds as an out-of-tree kernel module, you need matching kernel
>> sources
>>  to compile it. General build instructions can be found in the packet
>> README:
>> -http://code.google.com/p/netmap/source/browse/README
>> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>
>>  2.1.1 Building netmap on Ubuntu with stock kernel
>>
>> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>> convenient place.
>>  Now compile netmap:
>>
>>      cd LINUX
>> -    make SRC=<path_to_kernel_source>
>> +    ./configure --kernel-sources=<path_to_kernel_src>
>> +    make
>>
>>  2.1.2 Building netmap for kernel built from sources
>>
>>      cd LINUX
>> -    make KSRC=<path_to_kernel_source>
>> +    ./configure --kernel-dir=<path_to_kernel_tree>
>> +    make
>>
>>  2.2 Building ODP
>>  ----------------
>> @@ -69,13 +69,18 @@ Now compile netmap:
>>  The default platform is linux-netmap, if you want linux-generic you need
>> to
>>  specify it to configure --with-platform.
>>
>> +ODP works with the latest release version of netmap, which is currently
>> at API
>> +version 11.1. There is one problem though, the default CFLAGS that ODP is
>> built
>> +with contains -Werror=cast-qual. A temporary workaround is be to disable
>> this
>> +flag using -Wno-cast-qual.
>> +
>>  The --with-sdk-install-path can be used to point to the netmap sources.
>>
>>      ./bootstrap
>>
>> -    ./configure
>> +    ./configure CFLAGS='-Wno-cast-qual'
>>  or
>> -    ./configure --with-sdk-install-path=<netmap>
>> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>>
>>  To configure ODP for linux-generic:
>>      ./configure --with-platform=linux-generic
>> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>> b/platform/linux-netmap/include/odp_packet_netmap.h
>> index b4c523f..78b2379 100644
>> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>> @@ -26,17 +26,17 @@
>>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>  typedef struct {
>>         odp_buffer_pool_t pool;
>> -       size_t max_frame_len; /**< max frame len = buf_size -
>> sizeof(pkt_hdr) */
>> -       size_t frame_offset; /**< frame start offset from start of pkt buf
>> */
>> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>> -       struct nm_desc_t *nm_desc;
>> -       uint32_t begin;
>> -       uint32_t end;
>> -       struct netmap_ring *rxring;
>> -       struct netmap_ring *txring;
>> -       odp_queue_t tx_access; /* Used for exclusive access to send
>> packets */
>> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>> +       size_t frame_offset;    /**< Frame start offset from start of pkt
>> buf */
>> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
>> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>> +       uint32_t begin;         /**< First ring to poll */
>> +       uint32_t end;           /**< Last ring to poll */
>> +       struct netmap_ring *rxring; /**< First rx ring */
>> +       struct netmap_ring *txring; /**< First tx ring */
>> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>> packets */
>>         uint32_t if_flags;
>> -       char ifname[32];
>> +       char ifname[IFNAMSIZ];
>>  } pkt_netmap_t;
>>
>>  /**
>> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>> b/platform/linux-netmap/odp_packet_netmap.c
>> index 6243040..ec4e0ea 100644
>> --- a/platform/linux-netmap/odp_packet_netmap.c
>> +++ b/platform/linux-netmap/odp_packet_netmap.c
>> @@ -107,7 +107,7 @@ done:
>>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>>                      odp_buffer_pool_t pool)
>>  {
>> -       char ifname[32];
>> +       char ifname[IFNAMSIZ];
>>         odp_packet_t pkt;
>>         uint8_t *pkt_buf;
>>         uint8_t *l2_hdr;
>> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> const char *netdev,
>>
>>         odph_packet_free(pkt);
>>
>> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>>
>> -       if (pkt_nm->nm_desc == NULL) {
>> +       if (pkt_nm->desc == NULL) {
>>                 ODP_ERR("Error opening nm interface: %s\n",
>> strerror(errno));
>>                 return -1;
>>         }
>>
>>         ODP_DBG("thread %d mmap addr %p\n",
>>                 odp_thread_id(),
>> -               pkt_nm->nm_desc->mem);
>> +               pkt_nm->desc->mem);
>>
>>         pkt_nm->begin  = 0;
>> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>>
>>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>>         if (ret)
>> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> const char *netdev,
>>
>>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>>  {
>> -       if (pkt_nm->nm_desc != NULL) {
>> -               nm_close(pkt_nm->nm_desc);
>> -               pkt_nm->nm_desc = NULL;
>> +       if (pkt_nm->desc != NULL) {
>> +               nm_close(pkt_nm->desc);
>> +               pkt_nm->desc = NULL;
>>         }
>>
>>         return 0;
>> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>         int ret;
>>  #endif
>>
>> -       fd = pkt_nm->nm_desc->fd;
>> +       fd = pkt_nm->desc->fd;
>>  #ifdef NETMAP_BLOCKING_IO
>>         fds[0].fd = fd;
>>         fds[0].events = POLLIN;
>> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>                                 break;
>>                         }
>>
>> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>> ringid);
>> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>> ringid);
>>                 }
>>
>>                 limit = len - nb_rx;
>> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>         int ret;
>>  #endif
>>
>> -       fd = pkt_nm->nm_desc->fd;
>> +       fd = pkt_nm->desc->fd;
>>  #ifdef NETMAP_BLOCKING_IO
>>         fds[0].fd = fd;
>>         fds[0].events = POLLOUT;
>> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>                                 break;
>>                         }
>>
>> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>> ringid);
>> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>> ringid);
>>                 }
>>
>>                 limit = len - nb_tx;
>> --
>> 1.8.3.2
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Linaro  Sr Technical Manager
> LNG - ODP

Ciprian Barbu Nov. 13, 2014, 1:42 p.m. UTC | #4

On Thu, Nov 13, 2014 at 2:48 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 11/11/2014 08:35 PM, Ciprian Barbu wrote:
>>
>> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>> ---
>>   platform/linux-netmap/README                      | 23 ++++++++++-------
>>   platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>>   platform/linux-netmap/odp_packet_netmap.c         | 30
>> +++++++++++------------
>>   3 files changed, 39 insertions(+), 34 deletions(-)
>>
>> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
>> index abcb187..3718072 100644
>> --- a/platform/linux-netmap/README
>> +++ b/platform/linux-netmap/README
>> @@ -25,20 +25,18 @@ at runtime.
>>   2.1 Building netmap
>>   --------------------
>>   -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>> -ODP only works at this point with netmap API version 10 so you will need
>> -a specific revision of netmap.
>> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>         git clone https://code.google.com/p/netmap/
>>       cd netmap
>> -    git reset --hard 1f462ef
>> +    git co v11.1
>>     Netmap consists of a core kernel module (netmap_lin.ko) and optional
>> modified
>>   device drivers.
>>     Netmap builds as an out-of-tree kernel module, you need matching
>> kernel sources
>>   to compile it. General build instructions can be found in the packet
>> README:
>> -http://code.google.com/p/netmap/source/browse/README
>> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>     2.1.1 Building netmap on Ubuntu with stock kernel
>>   @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>> convenient place.
>>   Now compile netmap:
>>         cd LINUX
>> -    make SRC=<path_to_kernel_source>
>> +    ./configure --kernel-sources=<path_to_kernel_src>
>> +    make
>>     2.1.2 Building netmap for kernel built from sources
>>         cd LINUX
>> -    make KSRC=<path_to_kernel_source>
>> +    ./configure --kernel-dir=<path_to_kernel_tree>
>> +    make
>>     2.2 Building ODP
>>   ----------------
>> @@ -69,13 +69,18 @@ Now compile netmap:
>>   The default platform is linux-netmap, if you want linux-generic you need
>> to
>>   specify it to configure --with-platform.
>>   +ODP works with the latest release version of netmap, which is currently
>> at API
>> +version 11.1. There is one problem though, the default CFLAGS that ODP is
>> built
>> +with contains -Werror=cast-qual. A temporary workaround is be to disable
>> this
>> +flag using -Wno-cast-qual.
>> +
>
>
> Can this problem be fixed with casting to void * first and after that to new
> type?

I have no control for doing that, I simply call nm_open from
odp_packet_netmap.c. The cast error is inside the netmap API right
here:
https://code.google.com/p/netmap/source/browse/sys/net/netmap_user.h#234

I'm not using P2NMD from my code, nm_open does that indirectly passing
a const pointer.

>
> Maxim.
>
>
>
>>   The --with-sdk-install-path can be used to point to the netmap sources.
>>         ./bootstrap
>>   -    ./configure
>> +    ./configure CFLAGS='-Wno-cast-qual'
>>   or
>> -    ./configure --with-sdk-install-path=<netmap>
>> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>>     To configure ODP for linux-generic:
>>       ./configure --with-platform=linux-generic
>> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>> b/platform/linux-netmap/include/odp_packet_netmap.h
>> index b4c523f..78b2379 100644
>> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>> @@ -26,17 +26,17 @@
>>   /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>   typedef struct {
>>         odp_buffer_pool_t pool;
>> -       size_t max_frame_len; /**< max frame len = buf_size -
>> sizeof(pkt_hdr) */
>> -       size_t frame_offset; /**< frame start offset from start of pkt buf
>> */
>> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>> -       struct nm_desc_t *nm_desc;
>> -       uint32_t begin;
>> -       uint32_t end;
>> -       struct netmap_ring *rxring;
>> -       struct netmap_ring *txring;
>> -       odp_queue_t tx_access; /* Used for exclusive access to send
>> packets */
>> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>> +       size_t frame_offset;    /**< Frame start offset from start of pkt
>> buf */
>> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
>> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>> +       uint32_t begin;         /**< First ring to poll */
>> +       uint32_t end;           /**< Last ring to poll */
>> +       struct netmap_ring *rxring; /**< First rx ring */
>> +       struct netmap_ring *txring; /**< First tx ring */
>> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>> packets */
>>         uint32_t if_flags;
>> -       char ifname[32];
>> +       char ifname[IFNAMSIZ];
>>   } pkt_netmap_t;
>>     /**
>> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>> b/platform/linux-netmap/odp_packet_netmap.c
>> index 6243040..ec4e0ea 100644
>> --- a/platform/linux-netmap/odp_packet_netmap.c
>> +++ b/platform/linux-netmap/odp_packet_netmap.c
>> @@ -107,7 +107,7 @@ done:
>>   int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>>                      odp_buffer_pool_t pool)
>>   {
>> -       char ifname[32];
>> +       char ifname[IFNAMSIZ];
>>         odp_packet_t pkt;
>>         uint8_t *pkt_buf;
>>         uint8_t *l2_hdr;
>> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> const char *netdev,
>>         odph_packet_free(pkt);
>>   -     strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>>   -     if (pkt_nm->nm_desc == NULL) {
>> +       if (pkt_nm->desc == NULL) {
>>                 ODP_ERR("Error opening nm interface: %s\n",
>> strerror(errno));
>>                 return -1;
>>         }
>>         ODP_DBG("thread %d mmap addr %p\n",
>>                 odp_thread_id(),
>> -               pkt_nm->nm_desc->mem);
>> +               pkt_nm->desc->mem);
>>         pkt_nm->begin  = 0;
>> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>>         if (ret)
>> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> const char *netdev,
>>     int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>>   {
>> -       if (pkt_nm->nm_desc != NULL) {
>> -               nm_close(pkt_nm->nm_desc);
>> -               pkt_nm->nm_desc = NULL;
>> +       if (pkt_nm->desc != NULL) {
>> +               nm_close(pkt_nm->desc);
>> +               pkt_nm->desc = NULL;
>>         }
>>         return 0;
>> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>         int ret;
>>   #endif
>>   -     fd = pkt_nm->nm_desc->fd;
>> +       fd = pkt_nm->desc->fd;
>>   #ifdef NETMAP_BLOCKING_IO
>>         fds[0].fd = fd;
>>         fds[0].events = POLLIN;
>> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>                                 break;
>>                         }
>>   -                     rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>> ringid);
>> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>> ringid);
>>                 }
>>                 limit = len - nb_rx;
>> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>         int ret;
>>   #endif
>>   -     fd = pkt_nm->nm_desc->fd;
>> +       fd = pkt_nm->desc->fd;
>>   #ifdef NETMAP_BLOCKING_IO
>>         fds[0].fd = fd;
>>         fds[0].events = POLLOUT;
>> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> odp_packet_t pkt_table[],
>>                                 break;
>>                         }
>>   -                     txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>> ringid);
>> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>> ringid);
>>                 }
>>                 limit = len - nb_tx;
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Mike Holmes Nov. 13, 2014, 1:50 p.m. UTC | #5

On 13 November 2014 08:34, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:

> On Wed, Nov 12, 2014 at 10:27 PM, Mike Holmes <mike.holmes@linaro.org>
> wrote:
> > This does not build as a standalone patch, how can I be sure any failures
> > are related to the jump to v11.1 of netmap vs your other changes ? Can
> this
> > be done separately ?
>
> Are you following the instructions in platform/linux-netmap/README ? I
> just took odp-netmap fresh from git.linaro.org, applied this patch
> only and built it.
>

I used the  make-netmap-test from the ci scripts
https://git.linaro.org/people/anders.roxell/check-odp.git

I altered it to pull netmap 11.1 as you describe in the read me, and I
applied your patch.
Can you post a patch to that script to update it also ?

If you confirm this works I will try again manually and check I did not
goof!


>
> ./bootstrap
> ./configure --with-sdk-install-path=<netmap_path> CFLAGS='-Wno-cast-qual'
> make
>
> >
> > On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org>
> wrote:
> >>
> >> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
> >> ---
> >>  platform/linux-netmap/README                      | 23
> ++++++++++-------
> >>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
> >>  platform/linux-netmap/odp_packet_netmap.c         | 30
> >> +++++++++++------------
> >>  3 files changed, 39 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
> >> index abcb187..3718072 100644
> >> --- a/platform/linux-netmap/README
> >> +++ b/platform/linux-netmap/README
> >> @@ -25,20 +25,18 @@ at runtime.
> >>  2.1 Building netmap
> >>  --------------------
> >>
> >> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
> >> -ODP only works at this point with netmap API version 10 so you will
> need
> >> -a specific revision of netmap.
> >> +Netmap is currently hosted on https://code.google.com/p/netmap/
> >>
> >>      git clone https://code.google.com/p/netmap/
> >>      cd netmap
> >> -    git reset --hard 1f462ef
> >> +    git co v11.1
> >>
> >>  Netmap consists of a core kernel module (netmap_lin.ko) and optional
> >> modified
> >>  device drivers.
> >>
> >>  Netmap builds as an out-of-tree kernel module, you need matching kernel
> >> sources
> >>  to compile it. General build instructions can be found in the packet
> >> README:
> >> -http://code.google.com/p/netmap/source/browse/README
> >> +https://code.google.com/p/netmap/source/browse/LINUX/README
> >>
> >>  2.1.1 Building netmap on Ubuntu with stock kernel
> >>
> >> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
> >> convenient place.
> >>  Now compile netmap:
> >>
> >>      cd LINUX
> >> -    make SRC=<path_to_kernel_source>
> >> +    ./configure --kernel-sources=<path_to_kernel_src>
> >> +    make
> >>
> >>  2.1.2 Building netmap for kernel built from sources
> >>
> >>      cd LINUX
> >> -    make KSRC=<path_to_kernel_source>
> >> +    ./configure --kernel-dir=<path_to_kernel_tree>
> >> +    make
> >>
> >>  2.2 Building ODP
> >>  ----------------
> >> @@ -69,13 +69,18 @@ Now compile netmap:
> >>  The default platform is linux-netmap, if you want linux-generic you
> need
> >> to
> >>  specify it to configure --with-platform.
> >>
> >> +ODP works with the latest release version of netmap, which is currently
> >> at API
> >> +version 11.1. There is one problem though, the default CFLAGS that ODP
> is
> >> built
> >> +with contains -Werror=cast-qual. A temporary workaround is be to
> disable
> >> this
> >> +flag using -Wno-cast-qual.
> >> +
> >>  The --with-sdk-install-path can be used to point to the netmap sources.
> >>
> >>      ./bootstrap
> >>
> >> -    ./configure
> >> +    ./configure CFLAGS='-Wno-cast-qual'
> >>  or
> >> -    ./configure --with-sdk-install-path=<netmap>
> >> +    ./configure --with-sdk-install-path=<netmap>
> CFLAGS='-Wno-cast-qual'
> >>
> >>  To configure ODP for linux-generic:
> >>      ./configure --with-platform=linux-generic
> >> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
> >> b/platform/linux-netmap/include/odp_packet_netmap.h
> >> index b4c523f..78b2379 100644
> >> --- a/platform/linux-netmap/include/odp_packet_netmap.h
> >> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
> >> @@ -26,17 +26,17 @@
> >>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
> >>  typedef struct {
> >>         odp_buffer_pool_t pool;
> >> -       size_t max_frame_len; /**< max frame len = buf_size -
> >> sizeof(pkt_hdr) */
> >> -       size_t frame_offset; /**< frame start offset from start of pkt
> buf
> >> */
> >> -       size_t buf_size; /**< size of buffer payload in 'pool' */
> >> -       struct nm_desc_t *nm_desc;
> >> -       uint32_t begin;
> >> -       uint32_t end;
> >> -       struct netmap_ring *rxring;
> >> -       struct netmap_ring *txring;
> >> -       odp_queue_t tx_access; /* Used for exclusive access to send
> >> packets */
> >> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> >> +       size_t frame_offset;    /**< Frame start offset from start of
> pkt
> >> buf */
> >> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
> >> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
> >> +       uint32_t begin;         /**< First ring to poll */
> >> +       uint32_t end;           /**< Last ring to poll */
> >> +       struct netmap_ring *rxring; /**< First rx ring */
> >> +       struct netmap_ring *txring; /**< First tx ring */
> >> +       odp_queue_t tx_access;  /** Used for exclusive access to send
> >> packets */
> >>         uint32_t if_flags;
> >> -       char ifname[32];
> >> +       char ifname[IFNAMSIZ];
> >>  } pkt_netmap_t;
> >>
> >>  /**
> >> diff --git a/platform/linux-netmap/odp_packet_netmap.c
> >> b/platform/linux-netmap/odp_packet_netmap.c
> >> index 6243040..ec4e0ea 100644
> >> --- a/platform/linux-netmap/odp_packet_netmap.c
> >> +++ b/platform/linux-netmap/odp_packet_netmap.c
> >> @@ -107,7 +107,7 @@ done:
> >>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
> >>                      odp_buffer_pool_t pool)
> >>  {
> >> -       char ifname[32];
> >> +       char ifname[IFNAMSIZ];
> >>         odp_packet_t pkt;
> >>         uint8_t *pkt_buf;
> >>         uint8_t *l2_hdr;
> >> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
> >> const char *netdev,
> >>
> >>         odph_packet_free(pkt);
> >>
> >> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
> >> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
> >>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
> >> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
> >> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
> >>
> >> -       if (pkt_nm->nm_desc == NULL) {
> >> +       if (pkt_nm->desc == NULL) {
> >>                 ODP_ERR("Error opening nm interface: %s\n",
> >> strerror(errno));
> >>                 return -1;
> >>         }
> >>
> >>         ODP_DBG("thread %d mmap addr %p\n",
> >>                 odp_thread_id(),
> >> -               pkt_nm->nm_desc->mem);
> >> +               pkt_nm->desc->mem);
> >>
> >>         pkt_nm->begin  = 0;
> >> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
> >> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
> >> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
> >> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
> >> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
> >> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
> >>
> >>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
> >>         if (ret)
> >> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
> >> const char *netdev,
> >>
> >>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
> >>  {
> >> -       if (pkt_nm->nm_desc != NULL) {
> >> -               nm_close(pkt_nm->nm_desc);
> >> -               pkt_nm->nm_desc = NULL;
> >> +       if (pkt_nm->desc != NULL) {
> >> +               nm_close(pkt_nm->desc);
> >> +               pkt_nm->desc = NULL;
> >>         }
> >>
> >>         return 0;
> >> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
> >> odp_packet_t pkt_table[],
> >>         int ret;
> >>  #endif
> >>
> >> -       fd = pkt_nm->nm_desc->fd;
> >> +       fd = pkt_nm->desc->fd;
> >>  #ifdef NETMAP_BLOCKING_IO
> >>         fds[0].fd = fd;
> >>         fds[0].events = POLLIN;
> >> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
> >> odp_packet_t pkt_table[],
> >>                                 break;
> >>                         }
> >>
> >> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
> >> ringid);
> >> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
> >> ringid);
> >>                 }
> >>
> >>                 limit = len - nb_rx;
> >> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
> >> odp_packet_t pkt_table[],
> >>         int ret;
> >>  #endif
> >>
> >> -       fd = pkt_nm->nm_desc->fd;
> >> +       fd = pkt_nm->desc->fd;
> >>  #ifdef NETMAP_BLOCKING_IO
> >>         fds[0].fd = fd;
> >>         fds[0].events = POLLOUT;
> >> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
> >> odp_packet_t pkt_table[],
> >>                                 break;
> >>                         }
> >>
> >> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
> >> ringid);
> >> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
> >> ringid);
> >>                 }
> >>
> >>                 limit = len - nb_tx;
> >> --
> >> 1.8.3.2
> >>
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
> >
> >
> > --
> > Mike Holmes
> > Linaro  Sr Technical Manager
> > LNG - ODP
>

Maxim Uvarov Nov. 13, 2014, 2:05 p.m. UTC | #6

On 11/13/2014 04:42 PM, Ciprian Barbu wrote:
> On Thu, Nov 13, 2014 at 2:48 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>> On 11/11/2014 08:35 PM, Ciprian Barbu wrote:
>>> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>>> ---
>>>    platform/linux-netmap/README                      | 23 ++++++++++-------
>>>    platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>>>    platform/linux-netmap/odp_packet_netmap.c         | 30
>>> +++++++++++------------
>>>    3 files changed, 39 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
>>> index abcb187..3718072 100644
>>> --- a/platform/linux-netmap/README
>>> +++ b/platform/linux-netmap/README
>>> @@ -25,20 +25,18 @@ at runtime.
>>>    2.1 Building netmap
>>>    --------------------
>>>    -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>>> -ODP only works at this point with netmap API version 10 so you will need
>>> -a specific revision of netmap.
>>> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>>          git clone https://code.google.com/p/netmap/
>>>        cd netmap
>>> -    git reset --hard 1f462ef
>>> +    git co v11.1
>>>      Netmap consists of a core kernel module (netmap_lin.ko) and optional
>>> modified
>>>    device drivers.
>>>      Netmap builds as an out-of-tree kernel module, you need matching
>>> kernel sources
>>>    to compile it. General build instructions can be found in the packet
>>> README:
>>> -http://code.google.com/p/netmap/source/browse/README
>>> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>>      2.1.1 Building netmap on Ubuntu with stock kernel
>>>    @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>>> convenient place.
>>>    Now compile netmap:
>>>          cd LINUX
>>> -    make SRC=<path_to_kernel_source>
>>> +    ./configure --kernel-sources=<path_to_kernel_src>
>>> +    make
>>>      2.1.2 Building netmap for kernel built from sources
>>>          cd LINUX
>>> -    make KSRC=<path_to_kernel_source>
>>> +    ./configure --kernel-dir=<path_to_kernel_tree>
>>> +    make
>>>      2.2 Building ODP
>>>    ----------------
>>> @@ -69,13 +69,18 @@ Now compile netmap:
>>>    The default platform is linux-netmap, if you want linux-generic you need
>>> to
>>>    specify it to configure --with-platform.
>>>    +ODP works with the latest release version of netmap, which is currently
>>> at API
>>> +version 11.1. There is one problem though, the default CFLAGS that ODP is
>>> built
>>> +with contains -Werror=cast-qual. A temporary workaround is be to disable
>>> this
>>> +flag using -Wno-cast-qual.
>>> +
>>
>> Can this problem be fixed with casting to void * first and after that to new
>> type?
> I have no control for doing that, I simply call nm_open from
> odp_packet_netmap.c. The cast error is inside the netmap API right
> here:
> https://code.google.com/p/netmap/source/browse/sys/net/netmap_user.h#234
>
> I'm not using P2NMD from my code, nm_open does that indirectly passing
> a const pointer.
If that will be:

#define  P2NMD(p)                 ((struct  nm_desc*)(void *)(p))

will it work? If yes you can try to send this patch upstream ask to 
correct that define.

Maxim.


>> Maxim.
>>
>>
>>
>>>    The --with-sdk-install-path can be used to point to the netmap sources.
>>>          ./bootstrap
>>>    -    ./configure
>>> +    ./configure CFLAGS='-Wno-cast-qual'
>>>    or
>>> -    ./configure --with-sdk-install-path=<netmap>
>>> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>>>      To configure ODP for linux-generic:
>>>        ./configure --with-platform=linux-generic
>>> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>>> b/platform/linux-netmap/include/odp_packet_netmap.h
>>> index b4c523f..78b2379 100644
>>> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>>> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>>> @@ -26,17 +26,17 @@
>>>    /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>>    typedef struct {
>>>          odp_buffer_pool_t pool;
>>> -       size_t max_frame_len; /**< max frame len = buf_size -
>>> sizeof(pkt_hdr) */
>>> -       size_t frame_offset; /**< frame start offset from start of pkt buf
>>> */
>>> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>>> -       struct nm_desc_t *nm_desc;
>>> -       uint32_t begin;
>>> -       uint32_t end;
>>> -       struct netmap_ring *rxring;
>>> -       struct netmap_ring *txring;
>>> -       odp_queue_t tx_access; /* Used for exclusive access to send
>>> packets */
>>> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>>> +       size_t frame_offset;    /**< Frame start offset from start of pkt
>>> buf */
>>> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
>>> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>>> +       uint32_t begin;         /**< First ring to poll */
>>> +       uint32_t end;           /**< Last ring to poll */
>>> +       struct netmap_ring *rxring; /**< First rx ring */
>>> +       struct netmap_ring *txring; /**< First tx ring */
>>> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>>> packets */
>>>          uint32_t if_flags;
>>> -       char ifname[32];
>>> +       char ifname[IFNAMSIZ];
>>>    } pkt_netmap_t;
>>>      /**
>>> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>>> b/platform/linux-netmap/odp_packet_netmap.c
>>> index 6243040..ec4e0ea 100644
>>> --- a/platform/linux-netmap/odp_packet_netmap.c
>>> +++ b/platform/linux-netmap/odp_packet_netmap.c
>>> @@ -107,7 +107,7 @@ done:
>>>    int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>>>                       odp_buffer_pool_t pool)
>>>    {
>>> -       char ifname[32];
>>> +       char ifname[IFNAMSIZ];
>>>          odp_packet_t pkt;
>>>          uint8_t *pkt_buf;
>>>          uint8_t *l2_hdr;
>>> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> const char *netdev,
>>>          odph_packet_free(pkt);
>>>    -     strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>>> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>>>          snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>>> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>>> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>>>    -     if (pkt_nm->nm_desc == NULL) {
>>> +       if (pkt_nm->desc == NULL) {
>>>                  ODP_ERR("Error opening nm interface: %s\n",
>>> strerror(errno));
>>>                  return -1;
>>>          }
>>>          ODP_DBG("thread %d mmap addr %p\n",
>>>                  odp_thread_id(),
>>> -               pkt_nm->nm_desc->mem);
>>> +               pkt_nm->desc->mem);
>>>          pkt_nm->begin  = 0;
>>> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>>> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>>> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>>> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>>> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>>> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>>>          ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>>>          if (ret)
>>> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> const char *netdev,
>>>      int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>>>    {
>>> -       if (pkt_nm->nm_desc != NULL) {
>>> -               nm_close(pkt_nm->nm_desc);
>>> -               pkt_nm->nm_desc = NULL;
>>> +       if (pkt_nm->desc != NULL) {
>>> +               nm_close(pkt_nm->desc);
>>> +               pkt_nm->desc = NULL;
>>>          }
>>>          return 0;
>>> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> odp_packet_t pkt_table[],
>>>          int ret;
>>>    #endif
>>>    -     fd = pkt_nm->nm_desc->fd;
>>> +       fd = pkt_nm->desc->fd;
>>>    #ifdef NETMAP_BLOCKING_IO
>>>          fds[0].fd = fd;
>>>          fds[0].events = POLLIN;
>>> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> odp_packet_t pkt_table[],
>>>                                  break;
>>>                          }
>>>    -                     rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>>> ringid);
>>> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>>> ringid);
>>>                  }
>>>                  limit = len - nb_rx;
>>> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> odp_packet_t pkt_table[],
>>>          int ret;
>>>    #endif
>>>    -     fd = pkt_nm->nm_desc->fd;
>>> +       fd = pkt_nm->desc->fd;
>>>    #ifdef NETMAP_BLOCKING_IO
>>>          fds[0].fd = fd;
>>>          fds[0].events = POLLOUT;
>>> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> odp_packet_t pkt_table[],
>>>                                  break;
>>>                          }
>>>    -                     txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>>> ringid);
>>> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>>> ringid);
>>>                  }
>>>                  limit = len - nb_tx;
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp

Ciprian Barbu Nov. 13, 2014, 2:34 p.m. UTC | #7

On Thu, Nov 13, 2014 at 4:05 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 11/13/2014 04:42 PM, Ciprian Barbu wrote:
>>
>> On Thu, Nov 13, 2014 at 2:48 PM, Maxim Uvarov <maxim.uvarov@linaro.org>
>> wrote:
>>>
>>> On 11/11/2014 08:35 PM, Ciprian Barbu wrote:
>>>>
>>>> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>>>> ---
>>>>    platform/linux-netmap/README                      | 23
>>>> ++++++++++-------
>>>>    platform/linux-netmap/include/odp_packet_netmap.h | 20
>>>> +++++++--------
>>>>    platform/linux-netmap/odp_packet_netmap.c         | 30
>>>> +++++++++++------------
>>>>    3 files changed, 39 insertions(+), 34 deletions(-)
>>>>
>>>> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
>>>> index abcb187..3718072 100644
>>>> --- a/platform/linux-netmap/README
>>>> +++ b/platform/linux-netmap/README
>>>> @@ -25,20 +25,18 @@ at runtime.
>>>>    2.1 Building netmap
>>>>    --------------------
>>>>    -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>>>> -ODP only works at this point with netmap API version 10 so you will
>>>> need
>>>> -a specific revision of netmap.
>>>> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>>>          git clone https://code.google.com/p/netmap/
>>>>        cd netmap
>>>> -    git reset --hard 1f462ef
>>>> +    git co v11.1
>>>>      Netmap consists of a core kernel module (netmap_lin.ko) and
>>>> optional
>>>> modified
>>>>    device drivers.
>>>>      Netmap builds as an out-of-tree kernel module, you need matching
>>>> kernel sources
>>>>    to compile it. General build instructions can be found in the packet
>>>> README:
>>>> -http://code.google.com/p/netmap/source/browse/README
>>>> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>>>      2.1.1 Building netmap on Ubuntu with stock kernel
>>>>    @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>>>> convenient place.
>>>>    Now compile netmap:
>>>>          cd LINUX
>>>> -    make SRC=<path_to_kernel_source>
>>>> +    ./configure --kernel-sources=<path_to_kernel_src>
>>>> +    make
>>>>      2.1.2 Building netmap for kernel built from sources
>>>>          cd LINUX
>>>> -    make KSRC=<path_to_kernel_source>
>>>> +    ./configure --kernel-dir=<path_to_kernel_tree>
>>>> +    make
>>>>      2.2 Building ODP
>>>>    ----------------
>>>> @@ -69,13 +69,18 @@ Now compile netmap:
>>>>    The default platform is linux-netmap, if you want linux-generic you
>>>> need
>>>> to
>>>>    specify it to configure --with-platform.
>>>>    +ODP works with the latest release version of netmap, which is
>>>> currently
>>>> at API
>>>> +version 11.1. There is one problem though, the default CFLAGS that ODP
>>>> is
>>>> built
>>>> +with contains -Werror=cast-qual. A temporary workaround is be to
>>>> disable
>>>> this
>>>> +flag using -Wno-cast-qual.
>>>> +
>>>
>>>
>>> Can this problem be fixed with casting to void * first and after that to
>>> new
>>> type?
>>
>> I have no control for doing that, I simply call nm_open from
>> odp_packet_netmap.c. The cast error is inside the netmap API right
>> here:
>> https://code.google.com/p/netmap/source/browse/sys/net/netmap_user.h#234
>>
>> I'm not using P2NMD from my code, nm_open does that indirectly passing
>> a const pointer.
>
> If that will be:
>
> #define  P2NMD(p)                 ((struct  nm_desc*)(void *)(p))
>
> will it work? If yes you can try to send this patch upstream ask to correct
> that define.

I already submitted an issue, but there is no response from them.
There are other issues with greater impact I suppose. I don't think we
can do anything until then, and -Werror-cast-qual is not very serious.

https://code.google.com/p/netmap/issues/detail?id=31

>
> Maxim.
>
>
>
>>> Maxim.
>>>
>>>
>>>
>>>>    The --with-sdk-install-path can be used to point to the netmap
>>>> sources.
>>>>          ./bootstrap
>>>>    -    ./configure
>>>> +    ./configure CFLAGS='-Wno-cast-qual'
>>>>    or
>>>> -    ./configure --with-sdk-install-path=<netmap>
>>>> +    ./configure --with-sdk-install-path=<netmap>
>>>> CFLAGS='-Wno-cast-qual'
>>>>      To configure ODP for linux-generic:
>>>>        ./configure --with-platform=linux-generic
>>>> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>>>> b/platform/linux-netmap/include/odp_packet_netmap.h
>>>> index b4c523f..78b2379 100644
>>>> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>>>> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>>>> @@ -26,17 +26,17 @@
>>>>    /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>>>    typedef struct {
>>>>          odp_buffer_pool_t pool;
>>>> -       size_t max_frame_len; /**< max frame len = buf_size -
>>>> sizeof(pkt_hdr) */
>>>> -       size_t frame_offset; /**< frame start offset from start of pkt
>>>> buf
>>>> */
>>>> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>>>> -       struct nm_desc_t *nm_desc;
>>>> -       uint32_t begin;
>>>> -       uint32_t end;
>>>> -       struct netmap_ring *rxring;
>>>> -       struct netmap_ring *txring;
>>>> -       odp_queue_t tx_access; /* Used for exclusive access to send
>>>> packets */
>>>> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>>>> +       size_t frame_offset;    /**< Frame start offset from start of
>>>> pkt
>>>> buf */
>>>> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
>>>> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>>>> +       uint32_t begin;         /**< First ring to poll */
>>>> +       uint32_t end;           /**< Last ring to poll */
>>>> +       struct netmap_ring *rxring; /**< First rx ring */
>>>> +       struct netmap_ring *txring; /**< First tx ring */
>>>> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>>>> packets */
>>>>          uint32_t if_flags;
>>>> -       char ifname[32];
>>>> +       char ifname[IFNAMSIZ];
>>>>    } pkt_netmap_t;
>>>>      /**
>>>> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>>>> b/platform/linux-netmap/odp_packet_netmap.c
>>>> index 6243040..ec4e0ea 100644
>>>> --- a/platform/linux-netmap/odp_packet_netmap.c
>>>> +++ b/platform/linux-netmap/odp_packet_netmap.c
>>>> @@ -107,7 +107,7 @@ done:
>>>>    int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>>>>                       odp_buffer_pool_t pool)
>>>>    {
>>>> -       char ifname[32];
>>>> +       char ifname[IFNAMSIZ];
>>>>          odp_packet_t pkt;
>>>>          uint8_t *pkt_buf;
>>>>          uint8_t *l2_hdr;
>>>> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>>> const char *netdev,
>>>>          odph_packet_free(pkt);
>>>>    -     strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>>>> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>>>>          snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>>>> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>>>> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>>>>    -     if (pkt_nm->nm_desc == NULL) {
>>>> +       if (pkt_nm->desc == NULL) {
>>>>                  ODP_ERR("Error opening nm interface: %s\n",
>>>> strerror(errno));
>>>>                  return -1;
>>>>          }
>>>>          ODP_DBG("thread %d mmap addr %p\n",
>>>>                  odp_thread_id(),
>>>> -               pkt_nm->nm_desc->mem);
>>>> +               pkt_nm->desc->mem);
>>>>          pkt_nm->begin  = 0;
>>>> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>>>> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>>>> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>>>> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>>>> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>>>> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>>>>          ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>>>>          if (ret)
>>>> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>>> const char *netdev,
>>>>      int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>>>>    {
>>>> -       if (pkt_nm->nm_desc != NULL) {
>>>> -               nm_close(pkt_nm->nm_desc);
>>>> -               pkt_nm->nm_desc = NULL;
>>>> +       if (pkt_nm->desc != NULL) {
>>>> +               nm_close(pkt_nm->desc);
>>>> +               pkt_nm->desc = NULL;
>>>>          }
>>>>          return 0;
>>>> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>>> odp_packet_t pkt_table[],
>>>>          int ret;
>>>>    #endif
>>>>    -     fd = pkt_nm->nm_desc->fd;
>>>> +       fd = pkt_nm->desc->fd;
>>>>    #ifdef NETMAP_BLOCKING_IO
>>>>          fds[0].fd = fd;
>>>>          fds[0].events = POLLIN;
>>>> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>>> odp_packet_t pkt_table[],
>>>>                                  break;
>>>>                          }
>>>>    -                     rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>>>> ringid);
>>>> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>>>> ringid);
>>>>                  }
>>>>                  limit = len - nb_rx;
>>>> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>>> odp_packet_t pkt_table[],
>>>>          int ret;
>>>>    #endif
>>>>    -     fd = pkt_nm->nm_desc->fd;
>>>> +       fd = pkt_nm->desc->fd;
>>>>    #ifdef NETMAP_BLOCKING_IO
>>>>          fds[0].fd = fd;
>>>>          fds[0].events = POLLOUT;
>>>> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>>> odp_packet_t pkt_table[],
>>>>                                  break;
>>>>                          }
>>>>    -                     txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>>>> ringid);
>>>> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>>>> ringid);
>>>>                  }
>>>>                  limit = len - nb_tx;
>>>
>>>
>>>
>>> _______________________________________________
>>> lng-odp mailing list
>>> lng-odp@lists.linaro.org
>>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>

Ciprian Barbu Nov. 13, 2014, 2:37 p.m. UTC | #8

On Thu, Nov 13, 2014 at 3:50 PM, Mike Holmes <mike.holmes@linaro.org> wrote:
>
>
> On 13 November 2014 08:34, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
>>
>> On Wed, Nov 12, 2014 at 10:27 PM, Mike Holmes <mike.holmes@linaro.org>
>> wrote:
>> > This does not build as a standalone patch, how can I be sure any
>> > failures
>> > are related to the jump to v11.1 of netmap vs your other changes ? Can
>> > this
>> > be done separately ?
>>
>> Are you following the instructions in platform/linux-netmap/README ? I
>> just took odp-netmap fresh from git.linaro.org, applied this patch
>> only and built it.
>
>
> I used the  make-netmap-test from the ci scripts
> https://git.linaro.org/people/anders.roxell/check-odp.git
>
> I altered it to pull netmap 11.1 as you describe in the read me, and I
> applied your patch.
> Can you post a patch to that script to update it also ?
>
> If you confirm this works I will try again manually and check I did not
> goof!

Ok, I can do that. But please be specific about this next time. The
natural way of testing this patch was by using the README. Other means
we use internally are not acceptable. The entire process is very easy
to follow, at least I think so, by reading the README. If not, then we
need to address that.

>
>>
>>
>> ./bootstrap
>> ./configure --with-sdk-install-path=<netmap_path> CFLAGS='-Wno-cast-qual'
>> make
>>
>> >
>> > On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org>
>> > wrote:
>> >>
>> >> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>> >> ---
>> >>  platform/linux-netmap/README                      | 23
>> >> ++++++++++-------
>> >>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>> >>  platform/linux-netmap/odp_packet_netmap.c         | 30
>> >> +++++++++++------------
>> >>  3 files changed, 39 insertions(+), 34 deletions(-)
>> >>
>> >> diff --git a/platform/linux-netmap/README
>> >> b/platform/linux-netmap/README
>> >> index abcb187..3718072 100644
>> >> --- a/platform/linux-netmap/README
>> >> +++ b/platform/linux-netmap/README
>> >> @@ -25,20 +25,18 @@ at runtime.
>> >>  2.1 Building netmap
>> >>  --------------------
>> >>
>> >> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>> >> -ODP only works at this point with netmap API version 10 so you will
>> >> need
>> >> -a specific revision of netmap.
>> >> +Netmap is currently hosted on https://code.google.com/p/netmap/
>> >>
>> >>      git clone https://code.google.com/p/netmap/
>> >>      cd netmap
>> >> -    git reset --hard 1f462ef
>> >> +    git co v11.1
>> >>
>> >>  Netmap consists of a core kernel module (netmap_lin.ko) and optional
>> >> modified
>> >>  device drivers.
>> >>
>> >>  Netmap builds as an out-of-tree kernel module, you need matching
>> >> kernel
>> >> sources
>> >>  to compile it. General build instructions can be found in the packet
>> >> README:
>> >> -http://code.google.com/p/netmap/source/browse/README
>> >> +https://code.google.com/p/netmap/source/browse/LINUX/README
>> >>
>> >>  2.1.1 Building netmap on Ubuntu with stock kernel
>> >>
>> >> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>> >> convenient place.
>> >>  Now compile netmap:
>> >>
>> >>      cd LINUX
>> >> -    make SRC=<path_to_kernel_source>
>> >> +    ./configure --kernel-sources=<path_to_kernel_src>
>> >> +    make
>> >>
>> >>  2.1.2 Building netmap for kernel built from sources
>> >>
>> >>      cd LINUX
>> >> -    make KSRC=<path_to_kernel_source>
>> >> +    ./configure --kernel-dir=<path_to_kernel_tree>
>> >> +    make
>> >>
>> >>  2.2 Building ODP
>> >>  ----------------
>> >> @@ -69,13 +69,18 @@ Now compile netmap:
>> >>  The default platform is linux-netmap, if you want linux-generic you
>> >> need
>> >> to
>> >>  specify it to configure --with-platform.
>> >>
>> >> +ODP works with the latest release version of netmap, which is
>> >> currently
>> >> at API
>> >> +version 11.1. There is one problem though, the default CFLAGS that ODP
>> >> is
>> >> built
>> >> +with contains -Werror=cast-qual. A temporary workaround is be to
>> >> disable
>> >> this
>> >> +flag using -Wno-cast-qual.
>> >> +
>> >>  The --with-sdk-install-path can be used to point to the netmap
>> >> sources.
>> >>
>> >>      ./bootstrap
>> >>
>> >> -    ./configure
>> >> +    ./configure CFLAGS='-Wno-cast-qual'
>> >>  or
>> >> -    ./configure --with-sdk-install-path=<netmap>
>> >> +    ./configure --with-sdk-install-path=<netmap>
>> >> CFLAGS='-Wno-cast-qual'
>> >>
>> >>  To configure ODP for linux-generic:
>> >>      ./configure --with-platform=linux-generic
>> >> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>> >> b/platform/linux-netmap/include/odp_packet_netmap.h
>> >> index b4c523f..78b2379 100644
>> >> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>> >> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>> >> @@ -26,17 +26,17 @@
>> >>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>> >>  typedef struct {
>> >>         odp_buffer_pool_t pool;
>> >> -       size_t max_frame_len; /**< max frame len = buf_size -
>> >> sizeof(pkt_hdr) */
>> >> -       size_t frame_offset; /**< frame start offset from start of pkt
>> >> buf
>> >> */
>> >> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>> >> -       struct nm_desc_t *nm_desc;
>> >> -       uint32_t begin;
>> >> -       uint32_t end;
>> >> -       struct netmap_ring *rxring;
>> >> -       struct netmap_ring *txring;
>> >> -       odp_queue_t tx_access; /* Used for exclusive access to send
>> >> packets */
>> >> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>> >> +       size_t frame_offset;    /**< Frame start offset from start of
>> >> pkt
>> >> buf */
>> >> +       size_t buf_size;        /**< Size of buffer payload in 'pool'
>> >> */
>> >> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>> >> +       uint32_t begin;         /**< First ring to poll */
>> >> +       uint32_t end;           /**< Last ring to poll */
>> >> +       struct netmap_ring *rxring; /**< First rx ring */
>> >> +       struct netmap_ring *txring; /**< First tx ring */
>> >> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>> >> packets */
>> >>         uint32_t if_flags;
>> >> -       char ifname[32];
>> >> +       char ifname[IFNAMSIZ];
>> >>  } pkt_netmap_t;
>> >>
>> >>  /**
>> >> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>> >> b/platform/linux-netmap/odp_packet_netmap.c
>> >> index 6243040..ec4e0ea 100644
>> >> --- a/platform/linux-netmap/odp_packet_netmap.c
>> >> +++ b/platform/linux-netmap/odp_packet_netmap.c
>> >> @@ -107,7 +107,7 @@ done:
>> >>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>> >>                      odp_buffer_pool_t pool)
>> >>  {
>> >> -       char ifname[32];
>> >> +       char ifname[IFNAMSIZ];
>> >>         odp_packet_t pkt;
>> >>         uint8_t *pkt_buf;
>> >>         uint8_t *l2_hdr;
>> >> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> >> const char *netdev,
>> >>
>> >>         odph_packet_free(pkt);
>> >>
>> >> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>> >> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>> >>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>> >> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>> >> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>> >>
>> >> -       if (pkt_nm->nm_desc == NULL) {
>> >> +       if (pkt_nm->desc == NULL) {
>> >>                 ODP_ERR("Error opening nm interface: %s\n",
>> >> strerror(errno));
>> >>                 return -1;
>> >>         }
>> >>
>> >>         ODP_DBG("thread %d mmap addr %p\n",
>> >>                 odp_thread_id(),
>> >> -               pkt_nm->nm_desc->mem);
>> >> +               pkt_nm->desc->mem);
>> >>
>> >>         pkt_nm->begin  = 0;
>> >> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>> >> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>> >> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>> >> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>> >> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>> >> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>> >>
>> >>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>> >>         if (ret)
>> >> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> >> const char *netdev,
>> >>
>> >>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>> >>  {
>> >> -       if (pkt_nm->nm_desc != NULL) {
>> >> -               nm_close(pkt_nm->nm_desc);
>> >> -               pkt_nm->nm_desc = NULL;
>> >> +       if (pkt_nm->desc != NULL) {
>> >> +               nm_close(pkt_nm->desc);
>> >> +               pkt_nm->desc = NULL;
>> >>         }
>> >>
>> >>         return 0;
>> >> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> >> odp_packet_t pkt_table[],
>> >>         int ret;
>> >>  #endif
>> >>
>> >> -       fd = pkt_nm->nm_desc->fd;
>> >> +       fd = pkt_nm->desc->fd;
>> >>  #ifdef NETMAP_BLOCKING_IO
>> >>         fds[0].fd = fd;
>> >>         fds[0].events = POLLIN;
>> >> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> >> odp_packet_t pkt_table[],
>> >>                                 break;
>> >>                         }
>> >>
>> >> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>> >> ringid);
>> >> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>> >> ringid);
>> >>                 }
>> >>
>> >>                 limit = len - nb_rx;
>> >> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> >> odp_packet_t pkt_table[],
>> >>         int ret;
>> >>  #endif
>> >>
>> >> -       fd = pkt_nm->nm_desc->fd;
>> >> +       fd = pkt_nm->desc->fd;
>> >>  #ifdef NETMAP_BLOCKING_IO
>> >>         fds[0].fd = fd;
>> >>         fds[0].events = POLLOUT;
>> >> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>> >> odp_packet_t pkt_table[],
>> >>                                 break;
>> >>                         }
>> >>
>> >> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>> >> ringid);
>> >> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>> >> ringid);
>> >>                 }
>> >>
>> >>                 limit = len - nb_tx;
>> >> --
>> >> 1.8.3.2
>> >>
>> >>
>> >> _______________________________________________
>> >> lng-odp mailing list
>> >> lng-odp@lists.linaro.org
>> >> http://lists.linaro.org/mailman/listinfo/lng-odp
>> >
>> >
>> >
>> >
>> > --
>> > Mike Holmes
>> > Linaro  Sr Technical Manager
>> > LNG - ODP
>
>
>
>
> --
> Mike Holmes
> Linaro  Sr Technical Manager
> LNG - ODP

Ciprian Barbu Nov. 13, 2014, 5:11 p.m. UTC | #9

On Thu, Nov 13, 2014 at 4:37 PM, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
> On Thu, Nov 13, 2014 at 3:50 PM, Mike Holmes <mike.holmes@linaro.org> wrote:
>>
>>
>> On 13 November 2014 08:34, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
>>>
>>> On Wed, Nov 12, 2014 at 10:27 PM, Mike Holmes <mike.holmes@linaro.org>
>>> wrote:
>>> > This does not build as a standalone patch, how can I be sure any
>>> > failures
>>> > are related to the jump to v11.1 of netmap vs your other changes ? Can
>>> > this
>>> > be done separately ?
>>>
>>> Are you following the instructions in platform/linux-netmap/README ? I
>>> just took odp-netmap fresh from git.linaro.org, applied this patch
>>> only and built it.
>>
>>
>> I used the  make-netmap-test from the ci scripts
>> https://git.linaro.org/people/anders.roxell/check-odp.git
>>
>> I altered it to pull netmap 11.1 as you describe in the read me, and I
>> applied your patch.
>> Can you post a patch to that script to update it also ?
>>
>> If you confirm this works I will try again manually and check I did not
>> goof!
>
> Ok, I can do that. But please be specific about this next time. The
> natural way of testing this patch was by using the README. Other means
> we use internally are not acceptable. The entire process is very easy
> to follow, at least I think so, by reading the README. If not, then we
> need to address that.

I checked the build using make-netmap-test.sh. The modifications
needed are really small. But it doesn't make sense to send a patch,
not before updating odp-netmap.git. The scripts in check-odp.git are
really hard to debug, they are very good for checking when API breaks,
once it is properly set up. But it doesn't make sense to try and use
it to check that odp-netmap builds, like this was the case. Sure you
can make it work but it means doing the other job of updating the
scripts.

So if you want to properly review this patch series please just follow
the README and do it by hand. Someone needs to get familiar with it
anyway.

/Ciprian

>
>>
>>>
>>>
>>> ./bootstrap
>>> ./configure --with-sdk-install-path=<netmap_path> CFLAGS='-Wno-cast-qual'
>>> make
>>>
>>> >
>>> > On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org>
>>> > wrote:
>>> >>
>>> >> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>>> >> ---
>>> >>  platform/linux-netmap/README                      | 23
>>> >> ++++++++++-------
>>> >>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>>> >>  platform/linux-netmap/odp_packet_netmap.c         | 30
>>> >> +++++++++++------------
>>> >>  3 files changed, 39 insertions(+), 34 deletions(-)
>>> >>
>>> >> diff --git a/platform/linux-netmap/README
>>> >> b/platform/linux-netmap/README
>>> >> index abcb187..3718072 100644
>>> >> --- a/platform/linux-netmap/README
>>> >> +++ b/platform/linux-netmap/README
>>> >> @@ -25,20 +25,18 @@ at runtime.
>>> >>  2.1 Building netmap
>>> >>  --------------------
>>> >>
>>> >> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>>> >> -ODP only works at this point with netmap API version 10 so you will
>>> >> need
>>> >> -a specific revision of netmap.
>>> >> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>> >>
>>> >>      git clone https://code.google.com/p/netmap/
>>> >>      cd netmap
>>> >> -    git reset --hard 1f462ef
>>> >> +    git co v11.1
>>> >>
>>> >>  Netmap consists of a core kernel module (netmap_lin.ko) and optional
>>> >> modified
>>> >>  device drivers.
>>> >>
>>> >>  Netmap builds as an out-of-tree kernel module, you need matching
>>> >> kernel
>>> >> sources
>>> >>  to compile it. General build instructions can be found in the packet
>>> >> README:
>>> >> -http://code.google.com/p/netmap/source/browse/README
>>> >> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>> >>
>>> >>  2.1.1 Building netmap on Ubuntu with stock kernel
>>> >>
>>> >> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>>> >> convenient place.
>>> >>  Now compile netmap:
>>> >>
>>> >>      cd LINUX
>>> >> -    make SRC=<path_to_kernel_source>
>>> >> +    ./configure --kernel-sources=<path_to_kernel_src>
>>> >> +    make
>>> >>
>>> >>  2.1.2 Building netmap for kernel built from sources
>>> >>
>>> >>      cd LINUX
>>> >> -    make KSRC=<path_to_kernel_source>
>>> >> +    ./configure --kernel-dir=<path_to_kernel_tree>
>>> >> +    make
>>> >>
>>> >>  2.2 Building ODP
>>> >>  ----------------
>>> >> @@ -69,13 +69,18 @@ Now compile netmap:
>>> >>  The default platform is linux-netmap, if you want linux-generic you
>>> >> need
>>> >> to
>>> >>  specify it to configure --with-platform.
>>> >>
>>> >> +ODP works with the latest release version of netmap, which is
>>> >> currently
>>> >> at API
>>> >> +version 11.1. There is one problem though, the default CFLAGS that ODP
>>> >> is
>>> >> built
>>> >> +with contains -Werror=cast-qual. A temporary workaround is be to
>>> >> disable
>>> >> this
>>> >> +flag using -Wno-cast-qual.
>>> >> +
>>> >>  The --with-sdk-install-path can be used to point to the netmap
>>> >> sources.
>>> >>
>>> >>      ./bootstrap
>>> >>
>>> >> -    ./configure
>>> >> +    ./configure CFLAGS='-Wno-cast-qual'
>>> >>  or
>>> >> -    ./configure --with-sdk-install-path=<netmap>
>>> >> +    ./configure --with-sdk-install-path=<netmap>
>>> >> CFLAGS='-Wno-cast-qual'
>>> >>
>>> >>  To configure ODP for linux-generic:
>>> >>      ./configure --with-platform=linux-generic
>>> >> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> b/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> index b4c523f..78b2379 100644
>>> >> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> @@ -26,17 +26,17 @@
>>> >>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>> >>  typedef struct {
>>> >>         odp_buffer_pool_t pool;
>>> >> -       size_t max_frame_len; /**< max frame len = buf_size -
>>> >> sizeof(pkt_hdr) */
>>> >> -       size_t frame_offset; /**< frame start offset from start of pkt
>>> >> buf
>>> >> */
>>> >> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>>> >> -       struct nm_desc_t *nm_desc;
>>> >> -       uint32_t begin;
>>> >> -       uint32_t end;
>>> >> -       struct netmap_ring *rxring;
>>> >> -       struct netmap_ring *txring;
>>> >> -       odp_queue_t tx_access; /* Used for exclusive access to send
>>> >> packets */
>>> >> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>>> >> +       size_t frame_offset;    /**< Frame start offset from start of
>>> >> pkt
>>> >> buf */
>>> >> +       size_t buf_size;        /**< Size of buffer payload in 'pool'
>>> >> */
>>> >> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>>> >> +       uint32_t begin;         /**< First ring to poll */
>>> >> +       uint32_t end;           /**< Last ring to poll */
>>> >> +       struct netmap_ring *rxring; /**< First rx ring */
>>> >> +       struct netmap_ring *txring; /**< First tx ring */
>>> >> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>>> >> packets */
>>> >>         uint32_t if_flags;
>>> >> -       char ifname[32];
>>> >> +       char ifname[IFNAMSIZ];
>>> >>  } pkt_netmap_t;
>>> >>
>>> >>  /**
>>> >> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>>> >> b/platform/linux-netmap/odp_packet_netmap.c
>>> >> index 6243040..ec4e0ea 100644
>>> >> --- a/platform/linux-netmap/odp_packet_netmap.c
>>> >> +++ b/platform/linux-netmap/odp_packet_netmap.c
>>> >> @@ -107,7 +107,7 @@ done:
>>> >>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>>> >>                      odp_buffer_pool_t pool)
>>> >>  {
>>> >> -       char ifname[32];
>>> >> +       char ifname[IFNAMSIZ];
>>> >>         odp_packet_t pkt;
>>> >>         uint8_t *pkt_buf;
>>> >>         uint8_t *l2_hdr;
>>> >> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> const char *netdev,
>>> >>
>>> >>         odph_packet_free(pkt);
>>> >>
>>> >> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>>> >> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>>> >>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>>> >> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>>> >> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>>> >>
>>> >> -       if (pkt_nm->nm_desc == NULL) {
>>> >> +       if (pkt_nm->desc == NULL) {
>>> >>                 ODP_ERR("Error opening nm interface: %s\n",
>>> >> strerror(errno));
>>> >>                 return -1;
>>> >>         }
>>> >>
>>> >>         ODP_DBG("thread %d mmap addr %p\n",
>>> >>                 odp_thread_id(),
>>> >> -               pkt_nm->nm_desc->mem);
>>> >> +               pkt_nm->desc->mem);
>>> >>
>>> >>         pkt_nm->begin  = 0;
>>> >> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>>> >> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>>> >> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>>> >> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>>> >> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>>> >> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>>> >>
>>> >>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>>> >>         if (ret)
>>> >> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> const char *netdev,
>>> >>
>>> >>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>>> >>  {
>>> >> -       if (pkt_nm->nm_desc != NULL) {
>>> >> -               nm_close(pkt_nm->nm_desc);
>>> >> -               pkt_nm->nm_desc = NULL;
>>> >> +       if (pkt_nm->desc != NULL) {
>>> >> +               nm_close(pkt_nm->desc);
>>> >> +               pkt_nm->desc = NULL;
>>> >>         }
>>> >>
>>> >>         return 0;
>>> >> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>         int ret;
>>> >>  #endif
>>> >>
>>> >> -       fd = pkt_nm->nm_desc->fd;
>>> >> +       fd = pkt_nm->desc->fd;
>>> >>  #ifdef NETMAP_BLOCKING_IO
>>> >>         fds[0].fd = fd;
>>> >>         fds[0].events = POLLIN;
>>> >> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>                                 break;
>>> >>                         }
>>> >>
>>> >> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>>> >> ringid);
>>> >> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>>> >> ringid);
>>> >>                 }
>>> >>
>>> >>                 limit = len - nb_rx;
>>> >> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>         int ret;
>>> >>  #endif
>>> >>
>>> >> -       fd = pkt_nm->nm_desc->fd;
>>> >> +       fd = pkt_nm->desc->fd;
>>> >>  #ifdef NETMAP_BLOCKING_IO
>>> >>         fds[0].fd = fd;
>>> >>         fds[0].events = POLLOUT;
>>> >> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>                                 break;
>>> >>                         }
>>> >>
>>> >> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>>> >> ringid);
>>> >> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>>> >> ringid);
>>> >>                 }
>>> >>
>>> >>                 limit = len - nb_tx;
>>> >> --
>>> >> 1.8.3.2
>>> >>
>>> >>
>>> >> _______________________________________________
>>> >> lng-odp mailing list
>>> >> lng-odp@lists.linaro.org
>>> >> http://lists.linaro.org/mailman/listinfo/lng-odp
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Mike Holmes
>>> > Linaro  Sr Technical Manager
>>> > LNG - ODP
>>
>>
>>
>>
>> --
>> Mike Holmes
>> Linaro  Sr Technical Manager
>> LNG - ODP

Mike Holmes Nov. 13, 2014, 5:42 p.m. UTC | #10

On 13 November 2014 09:37, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
> On Thu, Nov 13, 2014 at 3:50 PM, Mike Holmes <mike.holmes@linaro.org> wrote:
>>
>>
>> On 13 November 2014 08:34, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
>>>
>>> On Wed, Nov 12, 2014 at 10:27 PM, Mike Holmes <mike.holmes@linaro.org>
>>> wrote:
>>> > This does not build as a standalone patch, how can I be sure any
>>> > failures
>>> > are related to the jump to v11.1 of netmap vs your other changes ? Can
>>> > this
>>> > be done separately ?
>>>
>>> Are you following the instructions in platform/linux-netmap/README ? I
>>> just took odp-netmap fresh from git.linaro.org, applied this patch
>>> only and built it.
>>
>>
>> I used the  make-netmap-test from the ci scripts
>> https://git.linaro.org/people/anders.roxell/check-odp.git
>>
>> I altered it to pull netmap 11.1 as you describe in the read me, and I
>> applied your patch.
>> Can you post a patch to that script to update it also ?
>>
>> If you confirm this works I will try again manually and check I did not
>> goof!
>
> Ok, I can do that. But please be specific about this next time. The
> natural way of testing this patch was by using the README. Other means
> we use internally are not acceptable. The entire process is very easy
> to follow, at least I think so, by reading the README. If not, then we
> need to address that.

I agree, the scripts are a separate issue, but related since when this
patch gets in the CI loops will still fail.
I don't expect that they are needed for this patch to be accepted,
just immediately afterwards.

>
>>
>>>
>>>
>>> ./bootstrap
>>> ./configure --with-sdk-install-path=<netmap_path> CFLAGS='-Wno-cast-qual'
>>> make
>>>
>>> >
>>> > On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org>
>>> > wrote:
>>> >>
>>> >> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>>> >> ---
>>> >>  platform/linux-netmap/README                      | 23
>>> >> ++++++++++-------
>>> >>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>>> >>  platform/linux-netmap/odp_packet_netmap.c         | 30
>>> >> +++++++++++------------
>>> >>  3 files changed, 39 insertions(+), 34 deletions(-)
>>> >>
>>> >> diff --git a/platform/linux-netmap/README
>>> >> b/platform/linux-netmap/README
>>> >> index abcb187..3718072 100644
>>> >> --- a/platform/linux-netmap/README
>>> >> +++ b/platform/linux-netmap/README
>>> >> @@ -25,20 +25,18 @@ at runtime.
>>> >>  2.1 Building netmap
>>> >>  --------------------
>>> >>
>>> >> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>>> >> -ODP only works at this point with netmap API version 10 so you will
>>> >> need
>>> >> -a specific revision of netmap.
>>> >> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>> >>
>>> >>      git clone https://code.google.com/p/netmap/
>>> >>      cd netmap
>>> >> -    git reset --hard 1f462ef
>>> >> +    git co v11.1
>>> >>
>>> >>  Netmap consists of a core kernel module (netmap_lin.ko) and optional
>>> >> modified
>>> >>  device drivers.
>>> >>
>>> >>  Netmap builds as an out-of-tree kernel module, you need matching
>>> >> kernel
>>> >> sources
>>> >>  to compile it. General build instructions can be found in the packet
>>> >> README:
>>> >> -http://code.google.com/p/netmap/source/browse/README
>>> >> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>> >>
>>> >>  2.1.1 Building netmap on Ubuntu with stock kernel
>>> >>
>>> >> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a
>>> >> convenient place.
>>> >>  Now compile netmap:
>>> >>
>>> >>      cd LINUX
>>> >> -    make SRC=<path_to_kernel_source>
>>> >> +    ./configure --kernel-sources=<path_to_kernel_src>
>>> >> +    make
>>> >>
>>> >>  2.1.2 Building netmap for kernel built from sources
>>> >>
>>> >>      cd LINUX
>>> >> -    make KSRC=<path_to_kernel_source>
>>> >> +    ./configure --kernel-dir=<path_to_kernel_tree>
>>> >> +    make
>>> >>
>>> >>  2.2 Building ODP
>>> >>  ----------------
>>> >> @@ -69,13 +69,18 @@ Now compile netmap:
>>> >>  The default platform is linux-netmap, if you want linux-generic you
>>> >> need
>>> >> to
>>> >>  specify it to configure --with-platform.
>>> >>
>>> >> +ODP works with the latest release version of netmap, which is
>>> >> currently
>>> >> at API
>>> >> +version 11.1. There is one problem though, the default CFLAGS that ODP
>>> >> is
>>> >> built
>>> >> +with contains -Werror=cast-qual. A temporary workaround is be to
>>> >> disable
>>> >> this
>>> >> +flag using -Wno-cast-qual.
>>> >> +
>>> >>  The --with-sdk-install-path can be used to point to the netmap
>>> >> sources.
>>> >>
>>> >>      ./bootstrap
>>> >>
>>> >> -    ./configure
>>> >> +    ./configure CFLAGS='-Wno-cast-qual'
>>> >>  or
>>> >> -    ./configure --with-sdk-install-path=<netmap>
>>> >> +    ./configure --with-sdk-install-path=<netmap>
>>> >> CFLAGS='-Wno-cast-qual'
>>> >>
>>> >>  To configure ODP for linux-generic:
>>> >>      ./configure --with-platform=linux-generic
>>> >> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> b/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> index b4c523f..78b2379 100644
>>> >> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>>> >> @@ -26,17 +26,17 @@
>>> >>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>> >>  typedef struct {
>>> >>         odp_buffer_pool_t pool;
>>> >> -       size_t max_frame_len; /**< max frame len = buf_size -
>>> >> sizeof(pkt_hdr) */
>>> >> -       size_t frame_offset; /**< frame start offset from start of pkt
>>> >> buf
>>> >> */
>>> >> -       size_t buf_size; /**< size of buffer payload in 'pool' */
>>> >> -       struct nm_desc_t *nm_desc;
>>> >> -       uint32_t begin;
>>> >> -       uint32_t end;
>>> >> -       struct netmap_ring *rxring;
>>> >> -       struct netmap_ring *txring;
>>> >> -       odp_queue_t tx_access; /* Used for exclusive access to send
>>> >> packets */
>>> >> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>>> >> +       size_t frame_offset;    /**< Frame start offset from start of
>>> >> pkt
>>> >> buf */
>>> >> +       size_t buf_size;        /**< Size of buffer payload in 'pool'
>>> >> */
>>> >> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
>>> >> +       uint32_t begin;         /**< First ring to poll */
>>> >> +       uint32_t end;           /**< Last ring to poll */
>>> >> +       struct netmap_ring *rxring; /**< First rx ring */
>>> >> +       struct netmap_ring *txring; /**< First tx ring */
>>> >> +       odp_queue_t tx_access;  /** Used for exclusive access to send
>>> >> packets */
>>> >>         uint32_t if_flags;
>>> >> -       char ifname[32];
>>> >> +       char ifname[IFNAMSIZ];
>>> >>  } pkt_netmap_t;
>>> >>
>>> >>  /**
>>> >> diff --git a/platform/linux-netmap/odp_packet_netmap.c
>>> >> b/platform/linux-netmap/odp_packet_netmap.c
>>> >> index 6243040..ec4e0ea 100644
>>> >> --- a/platform/linux-netmap/odp_packet_netmap.c
>>> >> +++ b/platform/linux-netmap/odp_packet_netmap.c
>>> >> @@ -107,7 +107,7 @@ done:
>>> >>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>>> >>                      odp_buffer_pool_t pool)
>>> >>  {
>>> >> -       char ifname[32];
>>> >> +       char ifname[IFNAMSIZ];
>>> >>         odp_packet_t pkt;
>>> >>         uint8_t *pkt_buf;
>>> >>         uint8_t *l2_hdr;
>>> >> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> const char *netdev,
>>> >>
>>> >>         odph_packet_free(pkt);
>>> >>
>>> >> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
>>> >> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>>> >>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
>>> >> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
>>> >> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>>> >>
>>> >> -       if (pkt_nm->nm_desc == NULL) {
>>> >> +       if (pkt_nm->desc == NULL) {
>>> >>                 ODP_ERR("Error opening nm interface: %s\n",
>>> >> strerror(errno));
>>> >>                 return -1;
>>> >>         }
>>> >>
>>> >>         ODP_DBG("thread %d mmap addr %p\n",
>>> >>                 odp_thread_id(),
>>> >> -               pkt_nm->nm_desc->mem);
>>> >> +               pkt_nm->desc->mem);
>>> >>
>>> >>         pkt_nm->begin  = 0;
>>> >> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
>>> >> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
>>> >> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
>>> >> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
>>> >> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
>>> >> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>>> >>
>>> >>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>>> >>         if (ret)
>>> >> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> const char *netdev,
>>> >>
>>> >>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>>> >>  {
>>> >> -       if (pkt_nm->nm_desc != NULL) {
>>> >> -               nm_close(pkt_nm->nm_desc);
>>> >> -               pkt_nm->nm_desc = NULL;
>>> >> +       if (pkt_nm->desc != NULL) {
>>> >> +               nm_close(pkt_nm->desc);
>>> >> +               pkt_nm->desc = NULL;
>>> >>         }
>>> >>
>>> >>         return 0;
>>> >> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>         int ret;
>>> >>  #endif
>>> >>
>>> >> -       fd = pkt_nm->nm_desc->fd;
>>> >> +       fd = pkt_nm->desc->fd;
>>> >>  #ifdef NETMAP_BLOCKING_IO
>>> >>         fds[0].fd = fd;
>>> >>         fds[0].events = POLLIN;
>>> >> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>                                 break;
>>> >>                         }
>>> >>
>>> >> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp,
>>> >> ringid);
>>> >> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp,
>>> >> ringid);
>>> >>                 }
>>> >>
>>> >>                 limit = len - nb_rx;
>>> >> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>         int ret;
>>> >>  #endif
>>> >>
>>> >> -       fd = pkt_nm->nm_desc->fd;
>>> >> +       fd = pkt_nm->desc->fd;
>>> >>  #ifdef NETMAP_BLOCKING_IO
>>> >>         fds[0].fd = fd;
>>> >>         fds[0].events = POLLOUT;
>>> >> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm,
>>> >> odp_packet_t pkt_table[],
>>> >>                                 break;
>>> >>                         }
>>> >>
>>> >> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp,
>>> >> ringid);
>>> >> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp,
>>> >> ringid);
>>> >>                 }
>>> >>
>>> >>                 limit = len - nb_tx;
>>> >> --
>>> >> 1.8.3.2
>>> >>
>>> >>
>>> >> _______________________________________________
>>> >> lng-odp mailing list
>>> >> lng-odp@lists.linaro.org
>>> >> http://lists.linaro.org/mailman/listinfo/lng-odp
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Mike Holmes
>>> > Linaro  Sr Technical Manager
>>> > LNG - ODP
>>
>>
>>
>>
>> --
>> Mike Holmes
>> Linaro  Sr Technical Manager
>> LNG - ODP

Mike Holmes Nov. 13, 2014, 6:59 p.m. UTC | #11

On 11 November 2014 12:35, Ciprian Barbu <ciprian.barbu@linaro.org> wrote:
> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>

Reviewed-by: Mike Holmes <mike.holmes@linaro.org>

> ---
>  platform/linux-netmap/README                      | 23 ++++++++++-------
>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>  platform/linux-netmap/odp_packet_netmap.c         | 30 +++++++++++------------
>  3 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
> index abcb187..3718072 100644
> --- a/platform/linux-netmap/README
> +++ b/platform/linux-netmap/README
> @@ -25,20 +25,18 @@ at runtime.
>  2.1 Building netmap
>  --------------------
>
> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
> -ODP only works at this point with netmap API version 10 so you will need
> -a specific revision of netmap.
> +Netmap is currently hosted on https://code.google.com/p/netmap/
>
>      git clone https://code.google.com/p/netmap/
>      cd netmap
> -    git reset --hard 1f462ef
> +    git co v11.1
>
>  Netmap consists of a core kernel module (netmap_lin.ko) and optional modified
>  device drivers.
>
>  Netmap builds as an out-of-tree kernel module, you need matching kernel sources
>  to compile it. General build instructions can be found in the packet README:
> -http://code.google.com/p/netmap/source/browse/README
> +https://code.google.com/p/netmap/source/browse/LINUX/README
>
>  2.1.1 Building netmap on Ubuntu with stock kernel
>
> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a convenient place.
>  Now compile netmap:
>
>      cd LINUX
> -    make SRC=<path_to_kernel_source>
> +    ./configure --kernel-sources=<path_to_kernel_src>
> +    make
>
>  2.1.2 Building netmap for kernel built from sources
>
>      cd LINUX
> -    make KSRC=<path_to_kernel_source>
> +    ./configure --kernel-dir=<path_to_kernel_tree>
> +    make
>
>  2.2 Building ODP
>  ----------------
> @@ -69,13 +69,18 @@ Now compile netmap:
>  The default platform is linux-netmap, if you want linux-generic you need to
>  specify it to configure --with-platform.
>
> +ODP works with the latest release version of netmap, which is currently at API
> +version 11.1. There is one problem though, the default CFLAGS that ODP is built
> +with contains -Werror=cast-qual. A temporary workaround is be to disable this
> +flag using -Wno-cast-qual.
> +
>  The --with-sdk-install-path can be used to point to the netmap sources.
>
>      ./bootstrap
>
> -    ./configure
> +    ./configure CFLAGS='-Wno-cast-qual'
>  or
> -    ./configure --with-sdk-install-path=<netmap>
> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>
>  To configure ODP for linux-generic:
>      ./configure --with-platform=linux-generic
> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h b/platform/linux-netmap/include/odp_packet_netmap.h
> index b4c523f..78b2379 100644
> --- a/platform/linux-netmap/include/odp_packet_netmap.h
> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
> @@ -26,17 +26,17 @@
>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>  typedef struct {
>         odp_buffer_pool_t pool;
> -       size_t max_frame_len; /**< max frame len = buf_size - sizeof(pkt_hdr) */
> -       size_t frame_offset; /**< frame start offset from start of pkt buf */
> -       size_t buf_size; /**< size of buffer payload in 'pool' */
> -       struct nm_desc_t *nm_desc;
> -       uint32_t begin;
> -       uint32_t end;
> -       struct netmap_ring *rxring;
> -       struct netmap_ring *txring;
> -       odp_queue_t tx_access; /* Used for exclusive access to send packets */
> +       size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> +       size_t frame_offset;    /**< Frame start offset from start of pkt buf */
> +       size_t buf_size;        /**< Size of buffer payload in 'pool' */
> +       struct nm_desc *desc;   /**< Netmap meta-data for the device */
> +       uint32_t begin;         /**< First ring to poll */
> +       uint32_t end;           /**< Last ring to poll */
> +       struct netmap_ring *rxring; /**< First rx ring */
> +       struct netmap_ring *txring; /**< First tx ring */
> +       odp_queue_t tx_access;  /** Used for exclusive access to send packets */
>         uint32_t if_flags;
> -       char ifname[32];
> +       char ifname[IFNAMSIZ];
>  } pkt_netmap_t;
>
>  /**
> diff --git a/platform/linux-netmap/odp_packet_netmap.c b/platform/linux-netmap/odp_packet_netmap.c
> index 6243040..ec4e0ea 100644
> --- a/platform/linux-netmap/odp_packet_netmap.c
> +++ b/platform/linux-netmap/odp_packet_netmap.c
> @@ -107,7 +107,7 @@ done:
>  int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>                      odp_buffer_pool_t pool)
>  {
> -       char ifname[32];
> +       char ifname[IFNAMSIZ];
>         odp_packet_t pkt;
>         uint8_t *pkt_buf;
>         uint8_t *l2_hdr;
> @@ -132,23 +132,23 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>
>         odph_packet_free(pkt);
>
> -       strncpy(pkt_nm->ifname, netdev, sizeof(pkt_nm->ifname));
> +       snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev);
>         snprintf(ifname, sizeof(ifname), "netmap:%s", netdev);
> -       pkt_nm->nm_desc = nm_open(ifname, NULL, 0, 0);
> +       pkt_nm->desc = nm_open(ifname, NULL, 0, 0);
>
> -       if (pkt_nm->nm_desc == NULL) {
> +       if (pkt_nm->desc == NULL) {
>                 ODP_ERR("Error opening nm interface: %s\n", strerror(errno));
>                 return -1;
>         }
>
>         ODP_DBG("thread %d mmap addr %p\n",
>                 odp_thread_id(),
> -               pkt_nm->nm_desc->mem);
> +               pkt_nm->desc->mem);
>
>         pkt_nm->begin  = 0;
> -       pkt_nm->end    = pkt_nm->nm_desc->req.nr_rx_rings;
> -       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, 0);
> -       pkt_nm->txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, 0);
> +       pkt_nm->end    = pkt_nm->desc->req.nr_rx_rings;
> +       pkt_nm->rxring = NETMAP_RXRING(pkt_nm->desc->nifp, 0);
> +       pkt_nm->txring = NETMAP_TXRING(pkt_nm->desc->nifp, 0);
>
>         ret = nm_do_ioctl(pkt_nm, SIOCGIFFLAGS, 0);
>         if (ret)
> @@ -188,9 +188,9 @@ int setup_pkt_netmap(pkt_netmap_t * const pkt_nm, const char *netdev,
>
>  int close_pkt_netmap(pkt_netmap_t * const pkt_nm)
>  {
> -       if (pkt_nm->nm_desc != NULL) {
> -               nm_close(pkt_nm->nm_desc);
> -               pkt_nm->nm_desc = NULL;
> +       if (pkt_nm->desc != NULL) {
> +               nm_close(pkt_nm->desc);
> +               pkt_nm->desc = NULL;
>         }
>
>         return 0;
> @@ -210,7 +210,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>         int ret;
>  #endif
>
> -       fd = pkt_nm->nm_desc->fd;
> +       fd = pkt_nm->desc->fd;
>  #ifdef NETMAP_BLOCKING_IO
>         fds[0].fd = fd;
>         fds[0].events = POLLIN;
> @@ -236,7 +236,7 @@ int recv_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>                                 break;
>                         }
>
> -                       rxring = NETMAP_RXRING(pkt_nm->nm_desc->nifp, ringid);
> +                       rxring = NETMAP_RXRING(pkt_nm->desc->nifp, ringid);
>                 }
>
>                 limit = len - nb_rx;
> @@ -323,7 +323,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>         int ret;
>  #endif
>
> -       fd = pkt_nm->nm_desc->fd;
> +       fd = pkt_nm->desc->fd;
>  #ifdef NETMAP_BLOCKING_IO
>         fds[0].fd = fd;
>         fds[0].events = POLLOUT;
> @@ -349,7 +349,7 @@ int send_pkt_netmap(pkt_netmap_t * const pkt_nm, odp_packet_t pkt_table[],
>                                 break;
>                         }
>
> -                       txring = NETMAP_TXRING(pkt_nm->nm_desc->nifp, ringid);
> +                       txring = NETMAP_TXRING(pkt_nm->desc->nifp, ringid);
>                 }
>
>                 limit = len - nb_tx;
> --
> 1.8.3.2
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp

Anders Roxell Nov. 13, 2014, 8:14 p.m. UTC | #12

On 2014-11-11 19:35, Ciprian Barbu wrote:
> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
> ---
>  platform/linux-netmap/README                      | 23 ++++++++++-------
>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>  platform/linux-netmap/odp_packet_netmap.c         | 30 +++++++++++------------
>  3 files changed, 39 insertions(+), 34 deletions(-)
> 
> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
> index abcb187..3718072 100644
> --- a/platform/linux-netmap/README
> +++ b/platform/linux-netmap/README
> @@ -25,20 +25,18 @@ at runtime.
>  2.1 Building netmap
>  --------------------
>  
> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
> -ODP only works at this point with netmap API version 10 so you will need
> -a specific revision of netmap.
> +Netmap is currently hosted on https://code.google.com/p/netmap/
>  
>      git clone https://code.google.com/p/netmap/
>      cd netmap
> -    git reset --hard 1f462ef
> +    git co v11.1
>  
>  Netmap consists of a core kernel module (netmap_lin.ko) and optional modified
>  device drivers.
>  
>  Netmap builds as an out-of-tree kernel module, you need matching kernel sources
>  to compile it. General build instructions can be found in the packet README:
> -http://code.google.com/p/netmap/source/browse/README
> +https://code.google.com/p/netmap/source/browse/LINUX/README
>  
>  2.1.1 Building netmap on Ubuntu with stock kernel

Building netmap kernel modules on Ubuntu...

We build the kernel module and not the netmap...

>  
> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a convenient place.
>  Now compile netmap:

Its not manatory to build netmap kernel module before we build ODP.

>  
>      cd LINUX
> -    make SRC=<path_to_kernel_source>
> +    ./configure --kernel-sources=<path_to_kernel_src>
> +    make
>  
>  2.1.2 Building netmap for kernel built from sources
>  
>      cd LINUX
> -    make KSRC=<path_to_kernel_source>
> +    ./configure --kernel-dir=<path_to_kernel_tree>
> +    make
>  
>  2.2 Building ODP
>  ----------------
> @@ -69,13 +69,18 @@ Now compile netmap:
>  The default platform is linux-netmap, if you want linux-generic you need to
>  specify it to configure --with-platform.
>  
> +ODP works with the latest release version of netmap, which is currently at API
> +version 11.1. There is one problem though, the default CFLAGS that ODP is built
> +with contains -Werror=cast-qual. A temporary workaround is be to disable this
> +flag using -Wno-cast-qual.
> +
>  The --with-sdk-install-path can be used to point to the netmap sources.
>  
>      ./bootstrap
>  
> -    ./configure
> +    ./configure CFLAGS='-Wno-cast-qual'

The CFLAG should not be here, it should go into
platform/linux-netmap/Makefile.am

>  or
> -    ./configure --with-sdk-install-path=<netmap>
> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>  
>  To configure ODP for linux-generic:
>      ./configure --with-platform=linux-generic
> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h b/platform/linux-netmap/include/odp_packet_netmap.h
> index b4c523f..78b2379 100644
> --- a/platform/linux-netmap/include/odp_packet_netmap.h
> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
> @@ -26,17 +26,17 @@
>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>  typedef struct {
>  	odp_buffer_pool_t pool;
> -	size_t max_frame_len; /**< max frame len = buf_size - sizeof(pkt_hdr) */
> -	size_t frame_offset; /**< frame start offset from start of pkt buf */
> -	size_t buf_size; /**< size of buffer payload in 'pool' */
> -	struct nm_desc_t *nm_desc;
> -	uint32_t begin;
> -	uint32_t end;
> -	struct netmap_ring *rxring;
> -	struct netmap_ring *txring;
> -	odp_queue_t tx_access; /* Used for exclusive access to send packets */
> +	size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
> +	size_t frame_offset;	/**< Frame start offset from start of pkt buf */
> +	size_t buf_size;	/**< Size of buffer payload in 'pool' */
> +	struct nm_desc *desc;	/**< Netmap meta-data for the device */
> +	uint32_t begin;		/**< First ring to poll */
> +	uint32_t end;		/**< Last ring to poll */
> +	struct netmap_ring *rxring; /**< First rx ring */
> +	struct netmap_ring *txring; /**< First tx ring */
> +	odp_queue_t tx_access;	/** Used for exclusive access to send packets */

Please be consistent you've used spaces and tabs between the variables
and comments.


Cheers,
Anders

Ciprian Barbu Nov. 14, 2014, 9:37 a.m. UTC | #13

On Thu, Nov 13, 2014 at 10:14 PM, Anders Roxell
<anders.roxell@linaro.org> wrote:
> On 2014-11-11 19:35, Ciprian Barbu wrote:
>> Signed-off-by: Ciprian Barbu <ciprian.barbu@linaro.org>
>> ---
>>  platform/linux-netmap/README                      | 23 ++++++++++-------
>>  platform/linux-netmap/include/odp_packet_netmap.h | 20 +++++++--------
>>  platform/linux-netmap/odp_packet_netmap.c         | 30 +++++++++++------------
>>  3 files changed, 39 insertions(+), 34 deletions(-)
>>
>> diff --git a/platform/linux-netmap/README b/platform/linux-netmap/README
>> index abcb187..3718072 100644
>> --- a/platform/linux-netmap/README
>> +++ b/platform/linux-netmap/README
>> @@ -25,20 +25,18 @@ at runtime.
>>  2.1 Building netmap
>>  --------------------
>>
>> -Netmap is currently hosted on https://code.google.com/p/netmap/ but
>> -ODP only works at this point with netmap API version 10 so you will need
>> -a specific revision of netmap.
>> +Netmap is currently hosted on https://code.google.com/p/netmap/
>>
>>      git clone https://code.google.com/p/netmap/
>>      cd netmap
>> -    git reset --hard 1f462ef
>> +    git co v11.1
>>
>>  Netmap consists of a core kernel module (netmap_lin.ko) and optional modified
>>  device drivers.
>>
>>  Netmap builds as an out-of-tree kernel module, you need matching kernel sources
>>  to compile it. General build instructions can be found in the packet README:
>> -http://code.google.com/p/netmap/source/browse/README
>> +https://code.google.com/p/netmap/source/browse/LINUX/README
>>
>>  2.1.1 Building netmap on Ubuntu with stock kernel
>
> Building netmap kernel modules on Ubuntu...
>
> We build the kernel module and not the netmap...
>
>>
>> @@ -56,12 +54,14 @@ You will need to locate it and extract it to a convenient place.
>>  Now compile netmap:
>
> Its not manatory to build netmap kernel module before we build ODP.

I'll try to reorganize this section

>
>>
>>      cd LINUX
>> -    make SRC=<path_to_kernel_source>
>> +    ./configure --kernel-sources=<path_to_kernel_src>
>> +    make
>>
>>  2.1.2 Building netmap for kernel built from sources
>>
>>      cd LINUX
>> -    make KSRC=<path_to_kernel_source>
>> +    ./configure --kernel-dir=<path_to_kernel_tree>
>> +    make
>>
>>  2.2 Building ODP
>>  ----------------
>> @@ -69,13 +69,18 @@ Now compile netmap:
>>  The default platform is linux-netmap, if you want linux-generic you need to
>>  specify it to configure --with-platform.
>>
>> +ODP works with the latest release version of netmap, which is currently at API
>> +version 11.1. There is one problem though, the default CFLAGS that ODP is built
>> +with contains -Werror=cast-qual. A temporary workaround is be to disable this
>> +flag using -Wno-cast-qual.
>> +
>>  The --with-sdk-install-path can be used to point to the netmap sources.
>>
>>      ./bootstrap
>>
>> -    ./configure
>> +    ./configure CFLAGS='-Wno-cast-qual'
>
> The CFLAG should not be here, it should go into
> platform/linux-netmap/Makefile.am

Sure.

>
>>  or
>> -    ./configure --with-sdk-install-path=<netmap>
>> +    ./configure --with-sdk-install-path=<netmap> CFLAGS='-Wno-cast-qual'
>>
>>  To configure ODP for linux-generic:
>>      ./configure --with-platform=linux-generic
>> diff --git a/platform/linux-netmap/include/odp_packet_netmap.h b/platform/linux-netmap/include/odp_packet_netmap.h
>> index b4c523f..78b2379 100644
>> --- a/platform/linux-netmap/include/odp_packet_netmap.h
>> +++ b/platform/linux-netmap/include/odp_packet_netmap.h
>> @@ -26,17 +26,17 @@
>>  /** Packet socket using netmap mmaped rings for both Rx and Tx */
>>  typedef struct {
>>       odp_buffer_pool_t pool;
>> -     size_t max_frame_len; /**< max frame len = buf_size - sizeof(pkt_hdr) */
>> -     size_t frame_offset; /**< frame start offset from start of pkt buf */
>> -     size_t buf_size; /**< size of buffer payload in 'pool' */
>> -     struct nm_desc_t *nm_desc;
>> -     uint32_t begin;
>> -     uint32_t end;
>> -     struct netmap_ring *rxring;
>> -     struct netmap_ring *txring;
>> -     odp_queue_t tx_access; /* Used for exclusive access to send packets */
>> +     size_t max_frame_len;   /**< buf_size - sizeof(pkt_hdr) */
>> +     size_t frame_offset;    /**< Frame start offset from start of pkt buf */
>> +     size_t buf_size;        /**< Size of buffer payload in 'pool' */
>> +     struct nm_desc *desc;   /**< Netmap meta-data for the device */
>> +     uint32_t begin;         /**< First ring to poll */
>> +     uint32_t end;           /**< Last ring to poll */
>> +     struct netmap_ring *rxring; /**< First rx ring */
>> +     struct netmap_ring *txring; /**< First tx ring */
>> +     odp_queue_t tx_access;  /** Used for exclusive access to send packets */
>
> Please be consistent you've used spaces and tabs between the variables
> and comments.

Ok, I missed that.

>
>
> Cheers,
> Anders

[NETMAP,1/4] Update to netmap_v11.1

Commit Message

Comments

Patch