On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshe...@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev