[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOrHB_Dkkp+onHg=-yfFDPkHygtnhutXuNiEwU6DNrWujP8h4A@mail.gmail.com>
Date: Sat, 23 Mar 2019 00:50:43 -0700
From: Pravin Shelar <pshelar@....org>
To: wenxu@...oud.cn
Cc: "David S. Miller" <davem@...emloft.net>,
Linux Kernel Network Developers <netdev@...r.kernel.org>,
ovs dev <dev@...nvswitch.org>
Subject: Re: [PATCH net-next] openvswitch: Make metadata_dst tunnel work in
IP_TUNNEL_INFO_BRIDGE mode
On Thu, Mar 21, 2019 at 3:34 AM <wenxu@...oud.cn> wrote:
>
> From: wenxu <wenxu@...oud.cn>
>
> There is currently no support for the multicasti/broadcst 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 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
>
This would make datapath bit complicated, can you give example of such use-case?
> Signed-off-by: wenxu <wenxu@...oud.cn>
> ---
> include/uapi/linux/openvswitch.h | 1 +
> net/openvswitch/flow_netlink.c | 32 ++++++++++++++++++++++++++------
> 2 files changed, 27 insertions(+), 6 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. */
> __OVS_TUNNEL_KEY_ATTR_MAX
> };
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 691da85..033df5c 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
> @@ -663,7 +664,7 @@ static int erspan_tun_opt_from_nlattr(const struct nlattr *a,
>
> static int ip_tun_from_nlattr(const struct nlattr *attr,
> struct sw_flow_match *match, bool is_mask,
> - bool log)
> + bool log, bool *no_ipv4_dst)
> {
> bool ttl = false, ipv4 = false, ipv6 = false;
> __be16 tun_flags = 0;
> @@ -671,6 +672,9 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
> struct nlattr *a;
> int rem;
>
> + if (no_ipv4_dst)
> + *no_ipv4_dst = false;
> +
> nla_for_each_nested(a, attr, rem) {
> int type = nla_type(a);
> int err;
> @@ -782,6 +786,12 @@ 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:
> + if (no_ipv4_dst) {
> + *no_ipv4_dst = true;
> + ipv4 = true;
> + }
> + break;
> default:
> OVS_NLERR(log, "Unknown IP tunnel attribute %d",
> type);
> @@ -812,9 +822,16 @@ 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) {
> + bool no_dst = no_ipv4_dst ? *no_ipv4_dst : false;
> +
> + if (no_dst && match->key->tun_key.u.ipv4.dst) {
> + OVS_NLERR(log, "IPv4 tunnel dst address is not zero");
> + return -EINVAL;
> + } else if (!no_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");
> @@ -1178,7 +1195,7 @@ static int metadata_from_nlattrs(struct net *net, struct sw_flow_match *match,
> }
> if (*attrs & (1 << OVS_KEY_ATTR_TUNNEL)) {
> if (ip_tun_from_nlattr(a[OVS_KEY_ATTR_TUNNEL], match,
> - is_mask, log) < 0)
> + is_mask, log, NULL) < 0)
Please pass non-null value here to have complete validation of tunnel
parameters.
> return -EINVAL;
> *attrs &= ~(1 << OVS_KEY_ATTR_TUNNEL);
> }
> @@ -2551,10 +2568,11 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
> struct nlattr *a;
> int err = 0, start, opts_type;
> __be16 dst_opt_type;
> + bool no_ipv4_dst;
>
> dst_opt_type = 0;
> ovs_match_init(&match, &key, true, NULL);
> - opts_type = ip_tun_from_nlattr(nla_data(attr), &match, false, log);
> + opts_type = ip_tun_from_nlattr(nla_data(attr), &match, false, log, &no_ipv4_dst);
> if (opts_type < 0)
> return opts_type;
>
> @@ -2605,6 +2623,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 && no_ipv4_dst)
> + 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
>
Powered by blists - more mailing lists