[ovs-dev] [PATCH] Tunnel: Cleanup old tunnel infrastructure.

Jesse Gross jesse at nicira.com
Wed Feb 20 18:44:41 UTC 2013


Here are a couple of small comments that I'd already written.  I
haven't gone through the main part of the patch yet but I figured that
I might as well send them if you are going to respin the patch.

On Tue, Feb 19, 2013 at 5:35 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> diff --git a/datapath/actions.c b/datapath/actions.c
> index f638ffc..cf44de3 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -439,22 +439,6 @@ static int execute_set_action(struct sk_buff *skb,
>                 skb_set_mark(skb, nla_get_u32(nested_attr));
>                 break;
>
> -       case OVS_KEY_ATTR_TUN_ID:
> -               /* If we're only using the TUN_ID action, store the value in a
> -                * temporary instance of struct ovs_key_ipv4_tunnel on the stack.
> -                * If both IPV4_TUNNEL and TUN_ID are being used together we
> -                * can't write into the IPV4_TUNNEL action, so make a copy and
> -                * write into that version.
> -                */
> -               if (!OVS_CB(skb)->tun_key)
> -                       memset(tun_key, 0, sizeof(*tun_key));
> -               else if (OVS_CB(skb)->tun_key != tun_key)
> -                       memcpy(tun_key, OVS_CB(skb)->tun_key, sizeof(*tun_key));
> -               OVS_CB(skb)->tun_key = tun_key;
> -
> -               OVS_CB(skb)->tun_key->tun_id = nla_get_be64(nested_attr);
> -               break;

I think we can further simplify this by removing the temporary tun_key
that we are keeping on the stack in ovs_execute_actions() and passing
around.

> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index 7ee31a2..12e85b2 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -242,6 +242,18 @@ enum {
>
>  #define OVS_PATCH_ATTR_MAX (__OVS_PATCH_ATTR_MAX - 1)
>
> +/* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
> + *
> + * OVS_TUNNEL_ATTR_DST_PORT is useful for vxlan.

I'm not sure that we need this comment about VXLAN here since it's
probably easier to just describe each element next to it.



More information about the dev mailing list