[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