[ovs-dev] [PATCH 3/3] datapath: refactor do_output() to move skb_clone NULL check out of fast path

[Pravin Shelar]

On Thu, Jul 10, 2014 at 1:16 AM, Andy Zhou <azhou at nicira.com> wrote:
> skb_clone() NULL check is implemented in do_output(), as past of the
> common (fast) path. Refactoring so that NULL check is done in the
> slow path, immediately after skb_clone() is called.
>
> Besides optimization, this change also improves code readability by
> making the skb_clone() NULL check consistent within OVS datapath
> module.
>
> Signed-off-by: Andy Zhou <azhou at nicira.com>
> ---
>  datapath/actions.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/datapath/actions.c b/datapath/actions.c
> index daa7b43..6eb83b8 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -490,9 +490,6 @@ static int do_output(struct datapath *dp, struct sk_buff *skb, int out_port)
>  {
>         struct vport *vport;
>
> -       if (unlikely(!skb))
> -               return -ENOMEM;
> -
There is no check for do_output() return, Therefore we can make it return void.


>         vport = ovs_vport_rcu(dp, out_port);
>         if (unlikely(!vport)) {
>                 kfree_skb(skb);
> @@ -693,7 +690,12 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>                 int err = 0;
>
>                 if (prev_port != -1) {
This can be marked as unlikely

> -                       do_output(dp, skb_clone(skb, GFP_ATOMIC), prev_port);
> +                       struct sk_buff *out_skb = skb_clone(skb, GFP_ATOMIC);
> +
> +                       if (!out_skb)
> +                               return -ENOMEM;
> +
This changes behavior, we can just skip do_output if it skb_clone() fails.

> +                       do_output(dp, out_skb, prev_port);
>                         prev_port = -1;
>                 }
>
otherwise looks good.