[ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink
Yang, Yi Y
yi.y.yang at intel.com
Tue Dec 6 07:17:20 UTC 2016
Hi, guys
This patch isn't updated from June on, Cascardo said he/Eric is still working on this, but six months passed, we don't see any following patch for this, now I have time to revisit it, for case 3 Pravin mentioned, I can make it work by applying the below patch [1] against https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316881.html, but it only can create vxlan port, it will also create vxlan port even if I create vxlan-gpe port by cmd "sudo ovs-vsctl add-port br-int vxlan_gpe1 -- set interface vxlan_gpe1 type=vxlan options:remote_ip=flow options:key=flow options:dst_port=4790 options:exts=gpe", the reason is very simple, vxlan_configure_exts in datapath/vport-vxlan.c will be called to set vxlan configuration, but it can't recognize vxlangpe extension and flags, you guys told me datapath/vport-vxlan.c and datapath/linux/compat/include/linux/openvswitch.h are compatibility code, we can't change them, but for case 3, we have to change them like patch [2], I know we should change them on Linux net-next kernel first, here I just show you we have to do so for case 3 Pravin mentioned, I'm happy to hear you have better way for this.
So my advice about this is we can push patch [2] to Linux net-next first, then apply patch series https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316879.html from Cascardo and apply [1], that can cover all the cases Pravin mentioned.
This patch has blocked us too long, we look forward to seeing progress on this.
[1]:
From f40b6fec09e1f9ad14e50ba224f46b1b9657399c Mon Sep 17 00:00:00 2001
From: Yi Yang <yi.y.yang at intel.com>
Date: Tue, 6 Dec 2016 12:39:41 +0800
Subject: [PATCH] Use ovs compat modules if USE_UPSTREAM_TUNNEL is defined
Signed-off-by: Yi Yang <yi.y.yang at intel.com>
---
lib/dpif-netlink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 0d03334..2286c3e 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1062,6 +1062,9 @@ dpif_netlink_port_query__(const struct dpif_netlink *dpif, odp_port_t port_no,
static int
dpif_netlink_port_create(struct netdev *netdev)
{
+#ifndef USE_UPSTREAM_TUNNEL
+ return EOPNOTSUPP;
+#else
switch (netdev_to_ovs_vport_type(netdev_get_type(netdev))) {
case OVS_VPORT_TYPE_VXLAN:
return netdev_vxlan_create(netdev);
@@ -1077,6 +1080,7 @@ dpif_netlink_port_create(struct netdev *netdev)
return EOPNOTSUPP;
}
return 0;
+#endif
}
static int
--
2.1.0
[2]:
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 12260d8..17e21cb 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -291,6 +291,7 @@ enum ovs_vport_attr {
enum {
OVS_VXLAN_EXT_UNSPEC,
OVS_VXLAN_EXT_GBP, /* Flag or __u32 */
+ OVS_VXLAN_EXT_GPE, /* Flag or __u32 */
__OVS_VXLAN_EXT_MAX,
};
diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
index 11965c0..a5882ed 100644
--- a/datapath/vport-vxlan.c
+++ b/datapath/vport-vxlan.c
@@ -54,11 +54,26 @@ static int vxlan_get_options(const struct vport *vport, struct sk_buff *skb)
nla_nest_end(skb, exts);
}
+ if (vxlan->flags & VXLAN_F_GPE) {
+ struct nlattr *exts;
+
+ exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION);
+ if (!exts)
+ return -EMSGSIZE;
+
+ if (vxlan->flags & VXLAN_F_GPE &&
+ nla_put_flag(skb, OVS_VXLAN_EXT_GPE))
+ return -EMSGSIZE;
+
+ nla_nest_end(skb, exts);
+ }
+
return 0;
}
static const struct nla_policy exts_policy[OVS_VXLAN_EXT_MAX + 1] = {
[OVS_VXLAN_EXT_GBP] = { .type = NLA_FLAG, },
+ [OVS_VXLAN_EXT_GPE] = { .type = NLA_FLAG, },
};
static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr,
@@ -76,6 +91,8 @@ static int vxlan_configure_exts(struct vport *vport, struct nlattr *attr,
if (exts[OVS_VXLAN_EXT_GBP])
conf->flags |= VXLAN_F_GBP;
+ if (exts[OVS_VXLAN_EXT_GPE])
+ conf->flags |= VXLAN_F_GPE;
return 0;
}
-----Original Message-----
From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Pravin Shelar
Sent: Friday, October 21, 2016 1:31 AM
To: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
Cc: ovs dev <dev at openvswitch.org>; Eric Garver <e at erig.me>
Subject: Re: [ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink
On Tue, Sep 20, 2016 at 7:01 AM, Thadeu Lima de Souza Cascardo <cascardo at redhat.com> wrote:
> In order to use rtnetlink to create new tunnel vports, the backported
> tunnels require some code that was removed from their upstream
> version, mainly the necessary code for newlink and for start_xmit.
>
> This patch adds back the necessary code for VXLAN, GRE and Geneve
> tunnels.
>
> Signed-off-by: Eric Garver <e at erig.me>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at redhat.com>
> ---
> datapath/linux/Modules.mk | 1 +
> datapath/linux/compat/geneve.c | 15 +--
> datapath/linux/compat/include/linux/if_tunnel.h | 71 ++++++++++++
> datapath/linux/compat/ip_gre.c | 65 ++++++++---
> datapath/linux/compat/vxlan.c | 147 +++++++++++++++++++++---
> 5 files changed, 261 insertions(+), 38 deletions(-) create mode
> 100644 datapath/linux/compat/include/linux/if_tunnel.h
>
> diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
> index 26f6d22..ad7d14a 100644
> --- a/datapath/linux/Modules.mk
> +++ b/datapath/linux/Modules.mk
> @@ -38,6 +38,7 @@ openvswitch_headers += \
> linux/compat/include/linux/if.h \
> linux/compat/include/linux/if_ether.h \
> linux/compat/include/linux/if_link.h \
> + linux/compat/include/linux/if_tunnel.h \
> linux/compat/include/linux/if_vlan.h \
> linux/compat/include/linux/in.h \
> linux/compat/include/linux/jiffies.h \ diff --git
> a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
> index 0c5b58a..79bb0ba 100644
> --- a/datapath/linux/compat/geneve.c
> +++ b/datapath/linux/compat/geneve.c
> @@ -1112,9 +1112,8 @@ tx_error:
> }
> #endif
>
> -netdev_tx_t rpl_geneve_xmit(struct sk_buff *skb)
> +static netdev_tx_t geneve_dev_xmit(struct sk_buff *skb, struct
> +net_device *dev)
> {
> - struct net_device *dev = skb->dev;
> struct geneve_dev *geneve = netdev_priv(dev);
> struct ip_tunnel_info *info = NULL;
>
> @@ -1128,18 +1127,12 @@ netdev_tx_t rpl_geneve_xmit(struct sk_buff
> *skb) #endif
> return geneve_xmit_skb(skb, dev, info); }
> -EXPORT_SYMBOL_GPL(rpl_geneve_xmit);
>
> -static netdev_tx_t geneve_dev_xmit(struct sk_buff *skb, struct
> net_device *dev)
> +netdev_tx_t rpl_geneve_xmit(struct sk_buff *skb)
> {
> - /* Drop All packets coming from networking stack. OVS-CB is
> - * not initialized for these packets.
> - */
> -
> - dev_kfree_skb(skb);
> - dev->stats.tx_dropped++;
> - return NETDEV_TX_OK;
> + return geneve_dev_xmit(skb, skb->dev);
> }
This would crash kernel.
OVS compat code expect dst entry in skb-cb, packet coming from kernel would not have it.
> +EXPORT_SYMBOL_GPL(rpl_geneve_xmit);
>
> static int __geneve_change_mtu(struct net_device *dev, int new_mtu,
> bool strict) { diff --git
> a/datapath/linux/compat/include/linux/if_tunnel.h
> b/datapath/linux/compat/include/linux/if_tunnel.h
> new file mode 100644
> index 0000000..476fe3c
> --- /dev/null
> +++ b/datapath/linux/compat/include/linux/if_tunnel.h
> @@ -0,0 +1,71 @@
> +#ifndef _LINUX_IF_TUNNEL_WRAPPER_H
> +#define _LINUX_IF_TUNNEL_WRAPPER_H
> +
> +#include_next<linux/if_tunnel.h>
> +
> +/* GRE section */
> +enum {
> +#define IFLA_GRE_UNSPEC rpl_IFLA_GRE_UNSPEC
> + IFLA_GRE_UNSPEC,
> +
> +#define IFLA_GRE_LINK rpl_IFLA_GRE_LINK
> + IFLA_GRE_LINK,
> +
> +#define IFLA_GRE_IFLAGS rpl_IFLA_GRE_IFLAGS
> + IFLA_GRE_IFLAGS,
> +
> +#define IFLA_GRE_OFLAGS rpl_IFLA_GRE_OFLAGS
> + IFLA_GRE_OFLAGS,
> +
> +#define IFLA_GRE_IKEY rpl_IFLA_GRE_IKEY
> + IFLA_GRE_IKEY,
> +
> +#define IFLA_GRE_OKEY rpl_IFLA_GRE_OKEY
> + IFLA_GRE_OKEY,
> +
> +#define IFLA_GRE_LOCAL rpl_IFLA_GRE_LOCAL
> + IFLA_GRE_LOCAL,
> +
> +#define IFLA_GRE_REMOTE rpl_IFLA_GRE_REMOTE
> + IFLA_GRE_REMOTE,
> +
> +#define IFLA_GRE_TTL rpl_IFLA_GRE_TTL
> + IFLA_GRE_TTL,
> +
> +#define IFLA_GRE_TOS rpl_IFLA_GRE_TOS
> + IFLA_GRE_TOS,
> +
> +#define IFLA_GRE_PMTUDISC rpl_IFLA_GRE_PMTUDISC
> + IFLA_GRE_PMTUDISC,
> +
> +#define IFLA_GRE_ENCAP_LIMIT rpl_IFLA_GRE_ENCAP_LIMIT
> + IFLA_GRE_ENCAP_LIMIT,
> +
> +#define IFLA_GRE_FLOWINFO rpl_IFLA_GRE_FLOWINFO
> + IFLA_GRE_FLOWINFO,
> +
> +#define IFLA_GRE_FLAGS rpl_IFLA_GRE_FLAGS
> + IFLA_GRE_FLAGS,
> +
> +#define IFLA_GRE_ENCAP_TYPE rpl_IFLA_GRE_ENCAP_TYPE
> + IFLA_GRE_ENCAP_TYPE,
> +
> +#define IFLA_GRE_ENCAP_FLAGS rpl_IFLA_GRE_ENCAP_FLAGS
> + IFLA_GRE_ENCAP_FLAGS,
> +
> +#define IFLA_GRE_ENCAP_SPORT rpl_IFLA_GRE_ENCAP_SPORT
> + IFLA_GRE_ENCAP_SPORT,
> +
> +#define IFLA_GRE_ENCAP_DPORT rpl_IFLA_GRE_ENCAP_DPORT
> + IFLA_GRE_ENCAP_DPORT,
> +
> +#define IFLA_GRE_COLLECT_METADATA rpl_IFLA_GRE_COLLECT_METADATA
> + IFLA_GRE_COLLECT_METADATA,
> +
> +#define __IFLA_GRE_MAX rpl__IFLA_GRE_MAX
> + __IFLA_GRE_MAX
> +};
> +#undef IFLA_GRE_MAX
> +#define IFLA_GRE_MAX (__IFLA_GRE_MAX - 1)
> +
After thinking about the actual issue of using netdevices for tunnel port this is what I think.
These are tree different implementations of OVS tunnel.
Case 1. OVS kernel module is upstream. It is straight forward to tunnel devices on upstream kernel module. STT and lisp are not available.
Case 2. OVS kernel module is out of tree. In this case OVS has compat code and USE_UPSTREAM_TUNNEL is defined. We are using upstream kernel modules for geneve, gre and vxlan, for rest of vport. (stt and lisp) we are using netdevices from compat code.
Case 3. OVS kernel module is out of tree and not using upstream tunnel devices. we have to fallback to OVS compat code for all tunnel modules.
Now to detect these cases userspace could probe for tunnel device "ovs_geneve" or "ovs_vxlan" if it exist it is case 3, and userspace vswitchd has to use existing vport APIs. Otherwise we could use netdev based tunnel devices. And create tunnel devices for each type of tunnel port.
_______________________________________________
dev mailing list
dev at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list