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 <pshe...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to