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

Jesse Gross jesse at nicira.com
Tue Feb 26 23:55:45 UTC 2013


On Tue, Feb 26, 2013 at 1:39 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> diff --git a/datapath/tunnel.c b/datapath/tunnel.c
> index a05cf54..47cecb3 100644
> --- a/datapath/tunnel.c
> +++ b/datapath/tunnel.c
> @@ -543,6 +283,9 @@ int ovs_tnl_send(struct vport *vport, struct sk_buff *skb)
>         u8 ttl;
>         u8 tos;
>
> +       if (!OVS_CB(skb)->tun_key)

Can you add an unlikely() here?

> +               goto error_free;
> +
>         /* Validate the protocol headers before we try to use them. */
>         if (skb->protocol == htons(ETH_P_8021Q) &&
>             !vlan_tx_tag_present(skb)) {

I think we can remove this whole block of code where we validate the
inner protocol header since all the logic that used to use them is now
in userspace.

On the receive side, I think we can also drop the ecn_decapsulate()
function since that should also be handled in userspace.

> diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
> index 388d9fb..3bfb567 100644
> --- a/datapath/vport-vxlan.c
> +++ b/datapath/vport-vxlan.c
>  static void vxlan_tunnel_release(struct vxlan_port *vxlan_port)
>  {
> -       vxlan_port->count--;
> +       if (!vxlan_port)
> +               return;
>
> -       if (vxlan_port->count == 0) {
> -               /* Release old socket */
> -               sk_release_kernel(vxlan_port->vxlan_rcv_socket->sk);
> -               list_del(&vxlan_port->list);
> -               kfree(vxlan_port);
> -       }
> +       /* Release old socket */
> +       sk_release_kernel(vxlan_port->vxlan_rcv_socket->sk);
> +       list_del_rcu(&vxlan_port->list);
> +       kfree(vxlan_port);

Before you were protecting vxlan_port with a deferred free by RCU.
Don't we still need to do this?  I think it's possible to be using the
data structure after both the socket and list entry have been removed
(or does sk_release_kernel include a call to synchronize_rcu())?

> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index 63d1cac..2cf356b 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -243,6 +243,16 @@ enum {
>
>  #define OVS_PATCH_ATTR_MAX (__OVS_PATCH_ATTR_MAX - 1)
>
> +/* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
> + */
> +enum {
> +       OVS_TUNNEL_ATTR_UNSPEC,
> +       OVS_TUNNEL_ATTR_DST_PORT, /* 16-bit UDP port, used by VXLAN. */

This is now used by both LISP and VXLAN, maybe we should just say that
it is for L4 tunnels.



More information about the dev mailing list