[ovs-dev] [PATCH 1/3] datapath: add skb_clone NULL check in the recirc action.

[Pravin Shelar]

On Thu, Jul 10, 2014 at 1:16 AM, Andy Zhou <azhou at nicira.com> wrote:
> Refactoring recirc action implementation.
>
> The main change is to fix a bug where the NULL check after skb clone()
> call is missing.  The fix is to return -ENOMEM whenever skb_clone()
> failed to create a clone.
>
> Also rearranged adjacent code to improve readability.
>
> Reported-by: Pravin B Shelar <pshelar at nicira.com>
> Signed-off-by: Andy Zhou <azhou at nicira.com>

> ---
>  datapath/actions.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 1f12f55..5cd7492 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -729,16 +729,17 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>                 case OVS_ACTION_ATTR_RECIRC: {
>                         struct sk_buff *recirc_skb;
>
> -                       if (!last_action(a, rem))
> -                               recirc_skb = skb_clone(skb, GFP_ATOMIC);
> -                       else
> -                               recirc_skb = skb;
> +                       if (last_action(a, rem))
> +                               return execute_recirc(dp, skb, a);
>
> -                       err = execute_recirc(dp, recirc_skb, a);
> +                       /* Recirc action is the not the last action
> +                        * of the action list. */
> +                       recirc_skb = skb_clone(skb, GFP_ATOMIC);
>
> -                       if (recirc_skb == skb)
> -                               return err;
> +                       if (!recirc_skb)
> +                               return -ENOMEM;
>
Can you continue with rest of actions, rather than aborting actions?

> +                       err = execute_recirc(dp, recirc_skb, a);
>                         break;
>                 }
>

otherwise looks good.

Acked-by: Pravin B Shelar <pshelar at nicira.com>
Thanks.

> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev