Hi, Jeremy,
Sorry for the delay. I don't think this patch is required for branch
2.3. As you may have noticed, this part of code
is different on branch 2.3. And it seems to work on my test.
Do you have the core dump from kernel crash? If yes, would you please
post the back trace?
Thanks,
Andy
On Sun, May 17, 2015 at 9:41 AM, Alex Wang <al...@nicira.com> wrote:
> Fwd to Andy,~
>
> On Sun, May 17, 2015 at 4:29 AM, zhangzhiming <zhangzhim...@yunshan.net.cn>
> wrote:
>>
>> Hi,
>>
>> I found one patch was omitted to be pushed to branch-2.3, which leads to
>> double freed skb.
>> Could someone to confirm the patch and submit it to branch-2.3?
>> Thanks!
>>
>> Here is the patch information:
>>
>>
>> commit 867e37ba00091b3e319c4c47c1598f1ae84dd32e
>> Author: Andy Zhou <az...@nicira.com>
>> Date: Mon Aug 25 15:18:19 2014 -0700
>>
>> datapath: Fix recirc bug where skb is double freed.
>>
>> 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>
>>
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index ad22467..7f25553 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
>> @@ -820,19 +829,9 @@ static int execute_recirc(struct datapath *dp, struct
>> sk_buff *skb,
>> * continue on with the rest of the action list. */
>> if (!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);
>> + }
>>
>> flow_key_set_recirc_id(skb, nla_get_u32(a));
>> ovs_dp_process_packet(skb, true);
>> @@ -897,6 +896,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:
>>
>> ________________________________
>> Jeremy Zhang
>>
>> _______________________________________________
>> discuss mailing list
>> discuss@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/discuss
>>
>
_______________________________________________
discuss mailing list
discuss@openvswitch.org
http://openvswitch.org/mailman/listinfo/discuss
