[ovs-dev] [PATCH 2/2] datapath: Simplify do_execute_actions().

Jarno Rajahalme jarno at ovn.org
Fri Mar 3 19:03:45 UTC 2017


Acked-by: Jarno Rajahalme <jarno at ovn.org>

> On Mar 2, 2017, at 7:29 PM, Joe Stringer <joe at ovn.org> wrote:
> 
> From: andy zhou <azhou at ovn.org>
> 
> Upstream commit:
>    commit 5b8784aaf29be20ba8d363e1124d7436d42ef9bf
>    Author: Andy Zhou <azhou at ovn.org>
>    Date: Fri Jan 27 13:45:28 2017 -0800
> 
>    openvswitch: Simplify do_execute_actions().
> 
>    do_execute_actions() implements a worthwhile optimization: in case
>    an output action is the last action in an action list, skb_clone()
>    can be avoided by outputing the current skb. However, the
>    implementation is more complicated than necessary.  This patch
>    simplify this logic.
> 
>    Signed-off-by: Andy Zhou <azhou at ovn.org>
>    Acked-by: Pravin B Shelar <pshelar at ovn.org>
>    Signed-off-by: David S. Miller <davem at davemloft.net>
> 
> Upstream: 5b8784aaf29b ("openvswitch: Simplify do_execute_actions().")
> Signed-off-by: Joe Stringer <joe at ovn.org>
> ---
> datapath/actions.c | 42 ++++++++++++++++++++----------------------
> 1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 3af34357ecd0..abb6637133b0 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -1125,12 +1125,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> 			      struct sw_flow_key *key,
> 			      const struct nlattr *attr, int len)
> {
> -	/* Every output action needs a separate clone of 'skb', but the common
> -	 * case is just a single output action, so that doing a clone and
> -	 * then freeing the original skbuff is wasteful.  So the following code
> -	 * is slightly obscure just to avoid that.
> -	 */
> -	int prev_port = -1;
> 	const struct nlattr *a;
> 	int rem;
> 
> @@ -1138,20 +1132,28 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> 	     a = nla_next(a, &rem)) {
> 		int err = 0;
> 
> -		if (unlikely(prev_port != -1)) {
> -			struct sk_buff *out_skb = skb_clone(skb, GFP_ATOMIC);
> -
> -			if (out_skb)
> -				do_output(dp, out_skb, prev_port, key);
> +		switch (nla_type(a)) {
> +		case OVS_ACTION_ATTR_OUTPUT: {
> +			int port = nla_get_u32(a);
> +			struct sk_buff *clone;
> +
> +			/* Every output action needs a separate clone
> +			 * of 'skb', In case the output action is the
> +			 * last action, cloning can be avoided.
> +			 */
> +			if (nla_is_last(a, rem)) {
> +				do_output(dp, skb, port, key);
> +				/* 'skb' has been used for output.
> +				 */
> +				return 0;
> +			}
> 
> +			clone = skb_clone(skb, GFP_ATOMIC);
> +			if (clone)
> +				do_output(dp, clone, port, key);
> 			OVS_CB(skb)->cutlen = 0;
> -			prev_port = -1;
> -		}
> -
> -		switch (nla_type(a)) {
> -		case OVS_ACTION_ATTR_OUTPUT:
> -			prev_port = nla_get_u32(a);
> 			break;
> +		}
> 
> 		case OVS_ACTION_ATTR_TRUNC: {
> 			struct ovs_action_trunc *trunc = nla_data(a);
> @@ -1241,11 +1243,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> 		}
> 	}
> 
> -	if (prev_port != -1)
> -		do_output(dp, skb, prev_port, key);
> -	else
> -		consume_skb(skb);
> -
> +	consume_skb(skb);
> 	return 0;
> }
> 
> -- 
> 2.11.1
> 



More information about the dev mailing list