On Thu, Jul 10, 2014 at 1:16 AM, Andy Zhou <az...@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 <pshe...@nicira.com>
> Signed-off-by: Andy Zhou <az...@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 <pshe...@nicira.com>
Thanks.
> --
> 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
