diff mbox series

[net-next,v2] openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode

Message ID 1553339014-13078-1-git-send-email-wenxu@ucloud.cn
State Changes Requested
Delegated to: David Miller
Headers show
Series [net-next,v2] openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode | expand

Commit Message

wenxu March 23, 2019, 11:03 a.m. UTC
From: wenxu <wenxu@ucloud.cn>

There is currently no support for the multicast/broadcast aspects
of VXLAN in ovs. In the datapath flow the tun_dst must specific.
But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
And the packet can forward through the fdb table of vxlan devcice. In
this mode the broadcast/multicast packet can be sent through the
following ways in ovs.

ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
        options:key=1000 options:remote_ip=flow
ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff,\
        action=output:vxlan

bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
        src_vni 1000 vni 1000 self
bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
src_vni 1000 vni 1000 self

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 include/uapi/linux/openvswitch.h |  1 +
 net/openvswitch/flow_netlink.c   | 19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

Comments

Pravin Shelar March 25, 2019, 7:23 p.m. UTC | #1
On Sat, Mar 23, 2019 at 4:03 AM <wenxu@ucloud.cn> wrote:
>
> From: wenxu <wenxu@ucloud.cn>
>
> There is currently no support for the multicast/broadcast aspects
> of VXLAN in ovs. In the datapath flow the tun_dst must specific.
> But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
> And the packet can forward through the fdb table of vxlan devcice. In
> this mode the broadcast/multicast packet can be sent through the
> following ways in ovs.
>
> ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
>         options:key=1000 options:remote_ip=flow
> ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff,\
>         action=output:vxlan
>
> bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
>         src_vni 1000 vni 1000 self
> bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
> src_vni 1000 vni 1000 self
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  include/uapi/linux/openvswitch.h |  1 +
>  net/openvswitch/flow_netlink.c   | 19 ++++++++++++++++---
>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index dbe0cbe..696a308 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -364,6 +364,7 @@ enum ovs_tunnel_key_attr {
>         OVS_TUNNEL_KEY_ATTR_IPV6_DST,           /* struct in6_addr dst IPv6 address. */
>         OVS_TUNNEL_KEY_ATTR_PAD,
>         OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,        /* struct erspan_metadata */
> +       OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST,        /* No argument. No dst IP address. */
This should be explicitly named to indicate its IP_TUNNEL_INFO_BRIDGE mode.

>         __OVS_TUNNEL_KEY_ATTR_MAX
>  };
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 691da85..7abea44 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -403,6 +403,7 @@ size_t ovs_key_attr_size(void)
>         [OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = sizeof(struct in6_addr) },
>         [OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = sizeof(struct in6_addr) },
>         [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
> +       [OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST]   = { .len = 0 },
>  };
>
>  static const struct ovs_len_tbl
> @@ -666,6 +667,7 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
>                               bool log)
>  {
>         bool ttl = false, ipv4 = false, ipv6 = false;
> +       bool no_ipv4_dst = false;
>         __be16 tun_flags = 0;
>         int opts_type = 0;
>         struct nlattr *a;
> @@ -782,6 +784,10 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
>                         tun_flags |= TUNNEL_ERSPAN_OPT;
>                         opts_type = type;
>                         break;
> +               case OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST:
> +                       no_ipv4_dst = true;
> +                       ipv4 = true;
> +                       break;
>                 default:
>                         OVS_NLERR(log, "Unknown IP tunnel attribute %d",
>                                   type);
> @@ -812,9 +818,14 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
>                         OVS_NLERR(log, "IP tunnel dst address not specified");
>                         return -EINVAL;
>                 }
> -               if (ipv4 && !match->key->tun_key.u.ipv4.dst) {
> -                       OVS_NLERR(log, "IPv4 tunnel dst address is zero");
> -                       return -EINVAL;
> +               if (ipv4) {
> +                       if (no_ipv4_dst && match->key->tun_key.u.ipv4.dst) {
> +                               OVS_NLERR(log, "IPv4 tunnel dst address is not zero");
> +                               return -EINVAL;
I am not sure why dst-ip is only validated here, in
IP_TUNNEL_INFO_BRIDGE mode all parameters except VNI are ignored.
either we need to check entire tunnel key or skip check for dst-ip, I
would prefer checking entire tun-key.


> +                       } else if (!no_ipv4_dst && !match->key->tun_key.u.ipv4.dst) {
> +                               OVS_NLERR(log, "IPv4 tunnel dst address is zero");
> +                               return -EINVAL;
> +                       }
>                 }
>                 if (ipv6 && ipv6_addr_any(&match->key->tun_key.u.ipv6.dst)) {
>                         OVS_NLERR(log, "IPv6 tunnel dst address is zero");
> @@ -2605,6 +2616,8 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
>         tun_info->mode = IP_TUNNEL_INFO_TX;
>         if (key.tun_proto == AF_INET6)
>                 tun_info->mode |= IP_TUNNEL_INFO_IPV6;
> +       else if (key.tun_proto == AF_INET && key.tun_key.u.ipv4.dst == 0)
> +               tun_info->mode |= IP_TUNNEL_INFO_BRIDGE;
>         tun_info->key = key.tun_key;
>
>         /* We need to store the options in the action itself since
> --
> 1.8.3.1
>
diff mbox series

Patch

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index dbe0cbe..696a308 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -364,6 +364,7 @@  enum ovs_tunnel_key_attr {
 	OVS_TUNNEL_KEY_ATTR_IPV6_DST,		/* struct in6_addr dst IPv6 address. */
 	OVS_TUNNEL_KEY_ATTR_PAD,
 	OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,	/* struct erspan_metadata */
+	OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST,	/* No argument. No dst IP address. */
 	__OVS_TUNNEL_KEY_ATTR_MAX
 };
 
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 691da85..7abea44 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -403,6 +403,7 @@  size_t ovs_key_attr_size(void)
 	[OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = sizeof(struct in6_addr) },
 	[OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = sizeof(struct in6_addr) },
 	[OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
+	[OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST]   = { .len = 0 },
 };
 
 static const struct ovs_len_tbl
@@ -666,6 +667,7 @@  static int ip_tun_from_nlattr(const struct nlattr *attr,
 			      bool log)
 {
 	bool ttl = false, ipv4 = false, ipv6 = false;
+	bool no_ipv4_dst = false;
 	__be16 tun_flags = 0;
 	int opts_type = 0;
 	struct nlattr *a;
@@ -782,6 +784,10 @@  static int ip_tun_from_nlattr(const struct nlattr *attr,
 			tun_flags |= TUNNEL_ERSPAN_OPT;
 			opts_type = type;
 			break;
+		case OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST:
+			no_ipv4_dst = true;
+			ipv4 = true;
+			break;
 		default:
 			OVS_NLERR(log, "Unknown IP tunnel attribute %d",
 				  type);
@@ -812,9 +818,14 @@  static int ip_tun_from_nlattr(const struct nlattr *attr,
 			OVS_NLERR(log, "IP tunnel dst address not specified");
 			return -EINVAL;
 		}
-		if (ipv4 && !match->key->tun_key.u.ipv4.dst) {
-			OVS_NLERR(log, "IPv4 tunnel dst address is zero");
-			return -EINVAL;
+		if (ipv4) {
+			if (no_ipv4_dst && match->key->tun_key.u.ipv4.dst) {
+				OVS_NLERR(log, "IPv4 tunnel dst address is not zero");
+				return -EINVAL;
+			} else if (!no_ipv4_dst && !match->key->tun_key.u.ipv4.dst) {
+				OVS_NLERR(log, "IPv4 tunnel dst address is zero");
+				return -EINVAL;
+			}
 		}
 		if (ipv6 && ipv6_addr_any(&match->key->tun_key.u.ipv6.dst)) {
 			OVS_NLERR(log, "IPv6 tunnel dst address is zero");
@@ -2605,6 +2616,8 @@  static int validate_and_copy_set_tun(const struct nlattr *attr,
 	tun_info->mode = IP_TUNNEL_INFO_TX;
 	if (key.tun_proto == AF_INET6)
 		tun_info->mode |= IP_TUNNEL_INFO_IPV6;
+	else if (key.tun_proto == AF_INET && key.tun_key.u.ipv4.dst == 0)
+		tun_info->mode |= IP_TUNNEL_INFO_BRIDGE;
 	tun_info->key = key.tun_key;
 
 	/* We need to store the options in the action itself since