Pushed with the fix suggested. Thanks.
On Tue, Aug 26, 2014 at 5:15 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Tue, Aug 26, 2014 at 3:36 PM, Andy Zhou <az...@nicira.com> wrote:
>> If recirc action is the last action of a action list, the SKB triggers
>> the recirc will be freed twice. This patch fixes this bug.
>>
>> Reported-by: Justin Pettit <jpet...@nicira.com>
>> Signed-off-by: Andy Zhou <az...@nicira.com>
>> ---
>> datapath/actions.c | 26 ++++++++++++++++----------
>> 1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index ad22467..3b39d6f 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>> @@ -809,7 +809,16 @@ static int execute_recirc(struct datapath *dp, struct
>> sk_buff *skb,
>> const struct nlattr *a, int rem)
>> {
>> struct sw_flow_key recirc_key;
>> - int err;
>> +
>> + if (!is_skb_flow_key_valid(skb)) {
>> + int err;
>> +
>> + err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
>> + if (err)
>> + return err;
>> +
>> + }
>> + BUG_ON(!is_skb_flow_key_valid(skb));
>>
>> if (!last_action(a, rem)) {
>> /* Recirc action is the not the last action
>> @@ -822,15 +831,6 @@ static int execute_recirc(struct datapath *dp, struct
>> sk_buff *skb,
>> return 0;
>> }
>>
>> - if (!is_skb_flow_key_valid(skb)) {
>> - err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
>> - if (err) {
>> - kfree_skb(skb);
>> - return err;
>> - }
>> - }
>> - BUG_ON(!is_skb_flow_key_valid(skb));
>> -
>> if (!last_action(a, rem))
>> flow_key_clone(skb, &recirc_key);
>
> Now these are two identical cases, we can just merge them.
>
> Otherwise looks good.
>
> Acked-by: Pravin B Shelar <pshe...@nicira.com>
>
>
>>
>> @@ -897,6 +897,12 @@ static int do_execute_actions(struct datapath *dp,
>> struct sk_buff *skb,
>>
>> case OVS_ACTION_ATTR_RECIRC:
>> err = execute_recirc(dp, skb, a, rem);
>> + if (last_action(a, rem)) {
>> + /* If this is the last action, the skb has
>> + * been consumed or freed.
>> + * Return immediately. */
>> + return err;
>> + }
>> break;
>>
>> case OVS_ACTION_ATTR_SET:
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
