[Intel-wired-lan] [PATCH v2 2/5] vxlan: Generalize udp based tunnel offload

Jesse Gross jesse at nicira.com
Wed Sep 9 21:30:05 UTC 2015


On Tue, Sep 8, 2015 at 2:55 PM, Anjali Singhai Jain
<anjali.singhai at intel.com> wrote:
> Replace add/del ndo ops for vxlan_port with tunnel_port so that all UDP
> based tunnels can use the same ndo op. Add a parameter to pass tunnel
> type to the ndo_op.
>
> Signed-off-by: Kiran Patil <kiran.patil at intel.com>
> Signed-off-by: Anjali Singhai Jain <anjali.singhai at intel.com>

You should squash the patch that comes after this into this one -
otherwise the kernel won't compile after this patch.

Did you mean to also add calls from the Geneve code to this? I think
you had it in the previous version.

I believe we also need a generalization of vxlan_get_rx_port().
Otherwise, there's no way for drivers to get the non-VXLAN tunnels
that were created when they weren't up.

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c69072b..b6f6beb 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> + * void (*ndo_add_tunnel_port)(struct  net_device *dev,
> + *                           sa_family_t sa_family, __be16 port, u32 type);
> + *     Called by UDP based tunnel modules to notify a driver about a UDP
> + *     port and socket address family that the tunnel is listening to. It is
> + *     called only when a new port starts listening. The operation is
> + *     protected by tunnel socket sock_lock.

This locking doesn't look safe to me. Each type of tunnel will have
its own socket lock but the data structures that are used in drivers
implementing these functions may be shared (such as in your i40e
patch). I believe that RTNL is currently held by both VXLAN and Geneve
when calling this with the possible exception of vxlan_get_rx_port().
In practice, my guess is that it is likely held then too, so it might
only require checking this and documenting/asserting it.

I might just call this ndo_add_udp_tunnel_port since that's what it
does anyways.

> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
> index c491c12..328f3ab 100644
> --- a/include/net/udp_tunnel.h
> +++ b/include/net/udp_tunnel.h
[...]
> +enum udp_tunnel_type {
> +       UDP_TUNNEL_NONE,
> +       UDP_TUNNEL_VXLAN,
> +       UDP_TUNNEL_GENEVE,
> +};

What does UDP_TUNNEL_NONE mean?


More information about the Intel-wired-lan mailing list