On Wed, Feb 19, 2014 at 10:30:03AM -0800, Luigi Rizzo wrote: > On Wed, Feb 19, 2014 at 7:30 AM, Stefan Hajnoczi <stefa...@gmail.com> 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
