Re: [Qemu-devel] [PATCH 4/5] net/dump: Add dump option for netdev devices

[Stefan Hajnoczi]

On Mon, Jun 29, 2015 at 11:57:15AM +0200, Thomas Huth wrote:
> On Fri, 26 Jun 2015 10:44:59 +0100
> Stefan Hajnoczi <address@hidden> wrote:
> 
> > On Wed, Jun 24, 2015 at 05:56:20PM +0200, Thomas Huth wrote:
> > > diff --git a/net/net.c b/net/net.c
> > > index cc36c7b..8871b77 100644
> > > --- a/net/net.c
> > > +++ b/net/net.c
> > > @@ -568,6 +568,12 @@ ssize_t qemu_deliver_packet(NetClientState *sender,
> > >          return 0;
> > >      }
> > >  
> > > +    if (nc->netdev_dump_enabled) {
> > > +        net_dump_receive(nc, data, size);
> > > +    } else if (sender->netdev_dump_enabled) {
> > > +        net_dump_receive(sender, data, size);
> > > +    }
> > 
> > Why "else if"?  If two interfaces have packet capture enabled then both
> > should get the full traffic log:
> > 
> >   if (nc->netdev_dump_enabled) {
> >       net_dump_receive(nc, data, size);
> >   }
> >   if (sender->netdev_dump_enabled) {
> >       net_dump_receive(sender, data, size);
> >   }
> 
> I think I assumed that only the interfaces that are created with
> "-netdev" can get a dump option. So it's either the receiver or the
> sender that dumps. If both interfaces would have the dump flag set,
> that would mean that two interfaces created with "-netdev" are talking
> to each other - and this can not happen, can it?

qemu_deliver_packet() doesn't know about -netdev vs -device.  It just
knows about NetClientState and peers.

Avoiding the 2nd memory load probably isn't worth baking the assumption
into this code.

pgpnnCXFsLTFG.pgp
Description: PGP signature