On Thu, Jan 21, 2016 at 3:41 PM, Jesse Gross <[email protected]> wrote:
> On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <[email protected]> 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.
>
if may_steal is false then we make a copy of the batch of packet, ref
dp_netdev_clone_pkt_batch().
>> 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.
>
ok, I will incorporate a fix in this patch.
> 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.
ok.
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
