[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net: Disable netmap backend when not supported
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] net: Disable netmap backend when not supported |
Date: |
Thu, 20 Feb 2014 10:46:56 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Feb 19, 2014 at 10:30:03AM -0800, Luigi Rizzo wrote:
> 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:
> > > 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
I see. A comment in the code would be nice to explain that.
> > > - 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.
Thanks for explaining. Please put justification for the
NETMAP_WITH_LIBS changes in the commit description.
Stefan