[ovs-dev] [PATCH v2 1/2] datapath: Free skb(s) on recirculation error

[Daniele Di Proietto]

If I understand correctly, the new code frees the skb only if execute_recirc() _returns_ an error and this can happen only if ovs_flow_extract() returns an error, in which case recirc_skb only gets freed once, but maybe I’m wrong.

I agree that it probably makes sense to have execute_recirc() take ownership of the skb, for the sake of clarity.


On May 12, 2014, at 3:48 PM, Jesse Gross <jesse at nicira.com> wrote:

> On Sat, May 10, 2014 at 7:46 PM, Simon Horman <horms at verge.net.au> wrote:
>> It is my understanding that on error execute_recirc() does not free the
>> skb passed to it.
>> 
>> Assuming this is true then on error skb should always be freed
>> if an error occurs in execute_recirc().
>> 
>> And if recirc_skb differs from skb, because it is a clone of skb,
>> then it should also be freed.
>> 
>> Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> I think this can introduce a double free in the normal case where
> execute_recirc() and is the last action. In this case, we free the skb
> in execute_recirc() and then continue down to the bottom of the
> do_execute_actions() since we are no longer returning and free the
> (same) skb again.
> 
> It may make sense to just have execute_recirc() always take ownership
> of the packet and free the skb on error itself.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=Eg%2FfHxEC3iuh2WnriHo1AveJd7tB9j1JgPoHg%2FGJnl4%3D%0A&s=122f5695e4138850c31c68726cb32337f48b293563a03af33dcc2d759c58c7ef