Re: [Qemu-devel] [PATCH 4/5] net: add offloadings support to netmap backend

[Stefan Hajnoczi <stefanha@redhat.com>]

On Mon, Jan 13, 2014 at 04:11:25PM +0100, Vincenzo Maffione wrote:
> 2014/1/13 Stefan Hajnoczi <stefanha@gmail.com>
> 
> > On Fri, Dec 13, 2013 at 01:05:02PM +0100, Vincenzo Maffione wrote:
> > > +static void netmap_using_vnet_hdr(NetClientState *nc, bool enable)
> > > +{
> > > +}
> >
> > I was trying to figure out whether it's okay for this function to be a
> > nop.  I've come to the conclusion that it's okay:
> >
> > If the netdev supports vnet_hdr then we enable vnet_hdr.  If not, it
> > will not enable it.
> >
> > Other NICs never enable vnet_hdr even if the netdev supports it.
> >
> > This interface is basically useless because set_vnet_hdr_len() is
> > also needed and provides the same information, i.e. that we are
> > transitioning from vnet_hdr off -> on.
> >
> > The bool argument is misleading since we never use this function to
> > disable vnet_hdr.  It's impossible to transition on -> off.
> >
> > I suggest removing using_vnet_hdr() and instead relying solely on
> > set_vnet_hdr_len().  Do this as the first patch in the series before
> > introducing the function pointers, that way all your following patches
> > become simpler.
> >
> > Completely agree. As usual, I didn't want to change too much the existing
> code.
> This means that I have to remove the tap_using_vnet_hdr() calls from
> virtio-net and vmxnet3 NICs before this patches, haven't I?

Yep.  I suggest starting this patch series with a few cleanup patches to
get the tap_*() interface into shape.

Then you can move that cleaned-up interface to NetClient in the second
half of the patch series.  By doing the cleanup first, you make the
second half simpler.

Stefan