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

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

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);
  }

Perhaps dumping should happen after ->receive() has returned size.  If
->receive() returns -1 the packet is discarded, and if it returns 0 the
packet is queued (not delivered yet).

If you dump unconditionally before ->receive() you will see queued
packets dumped multiple times (each time the queue gets flushed).

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]