Re: [Qemu-devel] [PATCH] net: Disable netmap backend when not supported

[Luigi Rizzo]

On Wed, Feb 19, 2014 at 7:30 AM, Stefan Hajnoczi <address@hidden> wrote:

On Fri, Feb 14, 2014 at 05:40:24PM +0100, Vincenzo Maffione wrote:
> This patch fixes configure so that netmap is not compiled in if the
> host doesn't support an API version >= 11.
>
> Moreover, some modifications have been done to net/netmap.c in
> order to reflect the current netmap API (11).
>
> Signed-off-by: Vincenzo Maffione <address@hidden>
> ---
>  configure    |  3 +++
>  net/netmap.c | 57 ++++++++++++++-------------------------------------------
>  2 files changed, 17 insertions(+), 43 deletions(-)
>
> diff --git a/configure b/configure
> index 88133a1..61eb932 100755
> --- a/configure
> +++ b/configure
> @@ -2118,6 +2118,9 @@ if test "$netmap" != "no" ; then
>  #include <net/if.h>
>  #include <net/netmap.h>
>  #include <net/netmap_user.h>
> +#if (NETMAP_API < 11) || (NETMAP_API > 15)
> +#error
> +#endif

Why error when NETMAP_API > 15?

this is meant to simulate a minor/major version number.

We will mark minor new features with values up to 15,

and if something happens that requires a change in the

backend we will move above 15, at which point we

will submit backend fixes and also the necessary

update to ./configure

  

> -    ring->cur = NETMAP_RING_NEXT(ring, i);

> -    ring->avail--;
> +    ring->cur = ring->head = nm_ring_next(ring, i);
>      ioctl(s->me.fd, NIOCTXSYNC, NULL);
>
>      return size;

Are these changes related to the NETMAP_WITH_LIBS macro?  Please do that
in a separate patch so we keep the version checking change separate from
the NETMAP_WITH_LIBS change.

netmap version 11 and above has NETMAP_WITH_LIBS,

while previous versions do not, so this ./configure

change has to go together with the change in the backend.

The netmap 11 code has already been committed to the FreeBSD

source repositories (for HEAD, 10 and 9) and to

code.google.com/p/netmap/ (for those who want it on linux).

So there is really no point, in my opinion, to make one

intermediate commit just to ./configure to disable

netmap detection on FreeBSD (it is already off on linux),

immediately followed by this one that uses the new feature.

Just to clarify: with one exception (fields in struct netmap_ring)

the netmap changes that we have are not at the kernel/user boundary

but in a library which avoids replicating long and boring code

(interface name parsing, parameter setting) in applications.

Avoiding the single incompatible struct change would have

been of course possible, but at the cost

extra complexity in the kernel and in userspace

(to support two slightly different interfaces).

Since netmap is (so far) relatively little used I thought it

was more important to fix the API and keep it simple. 

cheers

luigi

-----------------------------------------+-------------------------------
 Prof. Luigi RIZZO, address@hidden  . Dip. di Ing. dell'Informazione
 http://www.iet.unipi.it/~luigi/        . Universita` di Pisa
 TEL      +39-050-2211611               . via Diotisalvi 2
 Mobile   +39-338-6809875               . 56122 PISA (Italy)
-----------------------------------------+-------------------------------