[ovs-dev] [PATCH 4/8] netdev: Return number of packet from netdev_pop_header()

Jesse Gross jesse at kernel.org
Thu Jan 21 23:41:29 UTC 2016


On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index c746cc2..215e9b6 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3529,8 +3528,12 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
>                     packets = tnl_pkt;
>                  }
>
> -                err = netdev_pop_header(p->netdev, packets, cnt);
> +                err = netdev_pop_header(p->netdev, packets, &cnt);
> +                if (!cnt) {
> +                    return;
> +                }

Is this safe in the event that may_steal is false? It seems like the
caller could free a packet that we are trying to hold.

> diff --git a/lib/netdev.c b/lib/netdev.c
> index e3b70b1..e16a3be 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -758,12 +758,18 @@ netdev_pop_header(struct netdev *netdev, struct dp_packet **buffers, int cnt)
>      for (i = 0; i < cnt; i++) {
>          int err;
>
> -        err = netdev->netdev_class->pop_header(buffers[i]);
> +        err = netdev->netdev_class->pop_header(&buffers[i]);
>          if (err) {
>              dp_packet_clear(buffers[i]);
>          }
>      }

The current mechanism of releasing a packet in the event of an error
is a little hacky - it's just setting the size to zero and relying on
the fact that dp_netdev_input() will drop packets that don't have an
Ethernet header. It seems like with this we could do better and just
remove it from the list.

I also wonder if we could return a special error code to indicate that
a packet was held. That way we would only have a single mechanism for
a pop_header handler to report the status and we could just check the
value here to avoid freeing the memory.



More information about the dev mailing list