Re: [Qemu-devel] [PATCH v8 04/11] netfilter: hook packets before net queue send

[Thomas Huth <thuth@redhat.com>]

On 26/08/15 11:59, Yang Hongyang wrote:
> Capture packets that will be sent.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
> v5: do not check ret against iov_size
>     pass sent_cb to filters
> ---
>  net/net.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/net/net.c b/net/net.c
> index 74f3592..00cca83 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -562,6 +562,44 @@ int qemu_can_send_packet(NetClientState *sender)
>      return 1;
>  }
>  
> +static ssize_t filter_receive_iov(NetClientState *nc, int chain,
> +                                  NetClientState *sender,
> +                                  unsigned flags,
> +                                  const struct iovec *iov,
> +                                  int iovcnt,
> +                                  NetPacketSent *sent_cb)
> +{
> +    ssize_t ret = 0;
> +    NetFilterState *nf = NULL;
> +
> +    QTAILQ_FOREACH(nf, &nc->filters, next) {
> +        if (nf->chain == chain || nf->chain == NET_FILTER_ALL) {
> +            ret = nf->info->receive_iov(nf, sender, flags,
> +                                        iov, iovcnt, sent_cb);
> +            if (ret) {
> +                return ret;
> +            }
> +        }
> +    }
> +
> +    return ret;

I think that could also be "return 0" since all other return values are
handled by the return within the loop already. Then you could also
remove the pre-init of "ret = 0" at the beginning of the function.

> +}
> +
> +static ssize_t filter_receive(NetClientState *nc, int chain,
> +                              NetClientState *sender,
> +                              unsigned flags,
> +                              const uint8_t *data,
> +                              size_t size,
> +                              NetPacketSent *sent_cb)
> +{
> +    struct iovec iov = {
> +        .iov_base = (void *)data,
> +        .iov_len = size
> +    };
> +
> +    return filter_receive_iov(nc, chain, sender, flags, &iov, 1, sent_cb);
> +}
> +
>  ssize_t qemu_deliver_packet(NetClientState *sender,
>                              unsigned flags,
>                              const uint8_t *data,
> @@ -633,6 +671,7 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
>                                                   NetPacketSent *sent_cb)
>  {
>      NetQueue *queue;
> +    int ret;
>  
>  #ifdef DEBUG_NET
>      printf("qemu_send_packet_async:\n");
> @@ -643,6 +682,19 @@ static ssize_t qemu_send_packet_async_with_flags(NetClientState *sender,
>          return size;
>      }
>  
> +    /* Let filters handle the packet first */
> +    ret = filter_receive(sender, NET_FILTER_OUT,
> +                         sender, flags, buf, size, sent_cb);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    ret = filter_receive(sender->peer, NET_FILTER_IN,
> +                         sender, flags, buf, size, sent_cb);
> +    if (ret) {
> +        return ret;
> +    }
> +
>      queue = sender->peer->incoming_queue;
>  
>      return qemu_net_queue_send(queue, sender, flags, buf, size, sent_cb);
> @@ -713,11 +765,25 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
>                                  NetPacketSent *sent_cb)
>  {
>      NetQueue *queue;
> +    int ret;
>  
>      if (sender->link_down || !sender->peer) {
>          return iov_size(iov, iovcnt);
>      }
>  
> +    /* Let filters handle the packet first */
> +    ret = filter_receive_iov(sender, NET_FILTER_OUT, sender,
> +                             QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    ret = filter_receive_iov(sender->peer, NET_FILTER_IN, sender,
> +                             QEMU_NET_PACKET_FLAG_NONE, iov, iovcnt, sent_cb);
> +    if (ret) {
> +        return ret;
> +    }
> +
>      queue = sender->peer->incoming_queue;
>  
>      return qemu_net_queue_send_iov(queue, sender,

My comment above was just cosmetics, patch looks already fine to me as
it is, so anyway:

Reviewed-by: Thomas Huth <thuth@redhat.com>