| Message ID | 1570578672-28261-2-git-send-email-gvrose8192@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,1/2] compat: Backport IP_TUNNEL_INFO_BRIDGE | expand |
Bleep bloop. Greetings Greg Rose, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author wenxu <wenxu@ucloud.cn> needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Greg Rose <gvrose8192@gmail.com> Lines checked: 209, Warnings: 1, Errors: 1 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 10/8/2019 4:51 PM, Greg Rose wrote: > From: wenxu <wenxu@ucloud.cn> > > Upstream commit: > commit 18b6f717483a835fb98de9f0df6c724df9324e78 > Author: wenxu <wenxu@ucloud.cn> > Date: Thu Mar 28 12:43:23 2019 +0800 > > openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode > > 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> > Acked-by: Pravin B Shelar <pshelar@ovn.org> > Signed-off-by: David S. Miller <davem@davemloft.net> > > Also fixup case statement in lib/odp-util.c to make the compiler happy. > > Cc: wenxu <wenxu@ucloud.cn> > Signed-off-by: Greg Rose <gvrose8192@gmail.com> Hi Wenxu, would you mind giving this patch a review - I just want to make sure I didn't miss anything. Thanks, - Greg > --- > datapath/flow_netlink.c | 46 ++++++++++++++++++----- > datapath/linux/compat/include/linux/openvswitch.h | 1 + > lib/odp-util.c | 2 + > 3 files changed, 39 insertions(+), 10 deletions(-) > > diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c > index 0f7ab53..b3d1069 100644 > --- a/datapath/flow_netlink.c > +++ b/datapath/flow_netlink.c > @@ -406,6 +406,7 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] > [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_IPV4_INFO_BRIDGE] = { .len = 0 }, > }; > > static const struct ovs_len_tbl > @@ -669,6 +670,7 @@ static int ip_tun_from_nlattr(const struct nlattr *attr, > bool log) > { > bool ttl = false, ipv4 = false, ipv6 = false; > + bool info_bridge_mode = false; > __be16 tun_flags = 0; > int opts_type = 0; > struct nlattr *a; > @@ -785,6 +787,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_IPV4_INFO_BRIDGE: > + info_bridge_mode = true; > + ipv4 = true; > + break; > default: > OVS_NLERR(log, "Unknown IP tunnel attribute %d", > type); > @@ -815,16 +821,29 @@ 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 (info_bridge_mode) { > + if (match->key->tun_key.u.ipv4.src || > + match->key->tun_key.u.ipv4.dst || > + match->key->tun_key.tp_src || > + match->key->tun_key.tp_dst || > + match->key->tun_key.ttl || > + match->key->tun_key.tos || > + tun_flags & ~TUNNEL_KEY) { > + OVS_NLERR(log, "IPv4 tun info is not correct"); > + return -EINVAL; > + } > + } else if (!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"); > return -EINVAL; > } > > - if (!ttl) { > + if (!ttl && !info_bridge_mode) { > OVS_NLERR(log, "IP tunnel TTL not specified."); > return -EINVAL; > } > @@ -853,12 +872,17 @@ static int vxlan_opt_to_nlattr(struct sk_buff *skb, > static int __ip_tun_to_nlattr(struct sk_buff *skb, > const struct ip_tunnel_key *output, > const void *tun_opts, int swkey_tun_opts_len, > - unsigned short tun_proto) > + unsigned short tun_proto, u8 mode) > { > if (output->tun_flags & TUNNEL_KEY && > nla_put_be64(skb, OVS_TUNNEL_KEY_ATTR_ID, output->tun_id, > OVS_TUNNEL_KEY_ATTR_PAD)) > return -EMSGSIZE; > + > + if (mode & IP_TUNNEL_INFO_BRIDGE) > + return nla_put_flag(skb, OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE) > + ? -EMSGSIZE : 0; > + > switch (tun_proto) { > case AF_INET: > if (output->u.ipv4.src && > @@ -921,7 +945,7 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb, > static int ip_tun_to_nlattr(struct sk_buff *skb, > const struct ip_tunnel_key *output, > const void *tun_opts, int swkey_tun_opts_len, > - unsigned short tun_proto) > + unsigned short tun_proto, u8 mode) > { > struct nlattr *nla; > int err; > @@ -931,7 +955,7 @@ static int ip_tun_to_nlattr(struct sk_buff *skb, > return -EMSGSIZE; > > err = __ip_tun_to_nlattr(skb, output, tun_opts, swkey_tun_opts_len, > - tun_proto); > + tun_proto, mode); > if (err) > return err; > > @@ -945,7 +969,7 @@ int ovs_nla_put_tunnel_info(struct sk_buff *skb, > return __ip_tun_to_nlattr(skb, &tun_info->key, > ip_tunnel_info_opts(tun_info), > tun_info->options_len, > - ip_tunnel_info_af(tun_info)); > + ip_tunnel_info_af(tun_info), tun_info->mode); > } > > static int encode_vlan_from_nlattrs(struct sw_flow_match *match, > @@ -1982,7 +2006,7 @@ static int __ovs_nla_put_key(const struct sw_flow_key *swkey, > opts = TUN_METADATA_OPTS(output, swkey->tun_opts_len); > > if (ip_tun_to_nlattr(skb, &output->tun_key, opts, > - swkey->tun_opts_len, swkey->tun_proto)) > + swkey->tun_opts_len, swkey->tun_proto, 0)) > goto nla_put_failure; > } > > @@ -2610,6 +2634,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 > @@ -3375,7 +3401,7 @@ static int set_action_to_attr(const struct nlattr *a, struct sk_buff *skb) > err = ip_tun_to_nlattr(skb, &tun_info->key, > ip_tunnel_info_opts(tun_info), > tun_info->options_len, > - ip_tunnel_info_af(tun_info)); > + ip_tunnel_info_af(tun_info), tun_info->mode); > if (err) > return err; > nla_nest_end(skb, start); > diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h > index 7b16b1d..2f781ca 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -400,6 +400,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_IPV4_INFO_BRIDGE, /* No argument. IPV4_INFO_BRIDGE mode.*/ > __OVS_TUNNEL_KEY_ATTR_MAX > }; > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 3a574bf..949fd90 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -3892,6 +3892,8 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr, > format_odp_tun_erspan_opt(a, ma, ds, verbose); > ds_put_cstr(ds, "),"); > break; > + case OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE: > + break; > case __OVS_TUNNEL_KEY_ATTR_MAX: > default: > format_unknown_key(ds, a, ma);
On 10/15/2019 5:14 AM, Gregory Rose wrote: > > > On 10/8/2019 4:51 PM, Greg Rose wrote: >> From: wenxu <wenxu@ucloud.cn> >> >> Upstream commit: >> commit 18b6f717483a835fb98de9f0df6c724df9324e78 >> Author: wenxu <wenxu@ucloud.cn> >> Date: Thu Mar 28 12:43:23 2019 +0800 >> >> openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode >> >> 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> >> Acked-by: Pravin B Shelar <pshelar@ovn.org> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> >> Also fixup case statement in lib/odp-util.c to make the compiler happy. >> >> Cc: wenxu <wenxu@ucloud.cn> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com> > > Hi Wenxu, > > would you mind giving this patch a review - I just want to make sure I didn't miss anything. > > Thanks, > > - Greg > Looks good to me! Thanks wenxu
On 10/14/2019 9:51 PM, wenxu wrote: > > On 10/15/2019 5:14 AM, Gregory Rose wrote: >> >> >> On 10/8/2019 4:51 PM, Greg Rose wrote: >>> From: wenxu <wenxu@ucloud.cn> >>> >>> Upstream commit: >>> commit 18b6f717483a835fb98de9f0df6c724df9324e78 >>> Author: wenxu <wenxu@ucloud.cn> >>> Date: Thu Mar 28 12:43:23 2019 +0800 >>> >>> openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode >>> >>> 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> >>> Acked-by: Pravin B Shelar <pshelar@ovn.org> >>> Signed-off-by: David S. Miller <davem@davemloft.net> >>> >>> Also fixup case statement in lib/odp-util.c to make the compiler happy. >>> >>> Cc: wenxu <wenxu@ucloud.cn> >>> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >> >> Hi Wenxu, >> >> would you mind giving this patch a review - I just want to make sure I didn't miss anything. >> >> Thanks, >> >> - Greg >> > Looks good to me! > > > Thanks > > wenxu > Maintainers, I submited these patches back in October 2019 and then never got further with them. Since then the patch set is moribund I guess but I'm wondering if I should resubmit? Thanks, - Greg
On 9/11/20 5:37 PM, Gregory Rose wrote: > > On 10/14/2019 9:51 PM, wenxu wrote: >> >> On 10/15/2019 5:14 AM, Gregory Rose wrote: >>> >>> >>> On 10/8/2019 4:51 PM, Greg Rose wrote: >>>> From: wenxu <wenxu@ucloud.cn> >>>> >>>> Upstream commit: >>>> commit 18b6f717483a835fb98de9f0df6c724df9324e78 >>>> Author: wenxu <wenxu@ucloud.cn> >>>> Date: Thu Mar 28 12:43:23 2019 +0800 >>>> >>>> openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode >>>> >>>> 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> >>>> Acked-by: Pravin B Shelar <pshelar@ovn.org> >>>> Signed-off-by: David S. Miller <davem@davemloft.net> >>>> >>>> Also fixup case statement in lib/odp-util.c to make the compiler happy. >>>> >>>> Cc: wenxu <wenxu@ucloud.cn> >>>> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >>> >>> Hi Wenxu, >>> >>> would you mind giving this patch a review - I just want to make sure I didn't miss anything. >>> >>> Thanks, >>> >>> - Greg >>> >> Looks good to me! >> >> >> Thanks >> >> wenxu >> > > Maintainers, > > I submited these patches back in October 2019 and then never got further > with them. Since then the patch set is moribund I guess but I'm wondering if I should resubmit? Hi. Yes, it'll be great if you can re-submit at least to check that it still applicable and up to date. Regarding this feature (IP_TUNNEL_INFO_BRIDGE) I also reviewed the userspace part last week, but had no reply so far. And so I'm not sure if anyone will actually continue to work on this userspace part: https://mail.openvswitch.org/pipermail/ovs-dev/2020-September/374733.html > > Thanks, > > - Greg
On 9/14/2020 10:24 AM, Ilya Maximets wrote: > On 9/11/20 5:37 PM, Gregory Rose wrote: >> >> On 10/14/2019 9:51 PM, wenxu wrote: >>> >>> On 10/15/2019 5:14 AM, Gregory Rose wrote: >>>> >>>> >>>> On 10/8/2019 4:51 PM, Greg Rose wrote: >>>>> From: wenxu <wenxu@ucloud.cn> >>>>> >>>>> Upstream commit: >>>>> commit 18b6f717483a835fb98de9f0df6c724df9324e78 >>>>> Author: wenxu <wenxu@ucloud.cn> >>>>> Date: Thu Mar 28 12:43:23 2019 +0800 >>>>> >>>>> openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode >>>>> >>>>> 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> >>>>> Acked-by: Pravin B Shelar <pshelar@ovn.org> >>>>> Signed-off-by: David S. Miller <davem@davemloft.net> >>>>> >>>>> Also fixup case statement in lib/odp-util.c to make the compiler happy. >>>>> >>>>> Cc: wenxu <wenxu@ucloud.cn> >>>>> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >>>> >>>> Hi Wenxu, >>>> >>>> would you mind giving this patch a review - I just want to make sure I didn't miss anything. >>>> >>>> Thanks, >>>> >>>> - Greg >>>> >>> Looks good to me! >>> >>> >>> Thanks >>> >>> wenxu >>> >> >> Maintainers, >> >> I submited these patches back in October 2019 and then never got further >> with them. Since then the patch set is moribund I guess but I'm wondering if I should resubmit? > > Hi. Yes, it'll be great if you can re-submit at least to check that it still > applicable and up to date. > > Regarding this feature (IP_TUNNEL_INFO_BRIDGE) I also reviewed the userspace > part last week, but had no reply so far. And so I'm not sure if anyone will > actually continue to work on this userspace part: > https://mail.openvswitch.org/pipermail/ovs-dev/2020-September/374733.html Well the kernel bits make no sense without the userspace bits. Let me see if I can advance both. Busy with some other stuff but should get back to this soon. I'll resubmit the kernel and the userspace patches together and copy wenxu when I do. - Greg
On 9/14/20 10:11 PM, Gregory Rose wrote: > > On 9/14/2020 10:24 AM, Ilya Maximets wrote: >> On 9/11/20 5:37 PM, Gregory Rose wrote: >>> >>> On 10/14/2019 9:51 PM, wenxu wrote: >>>> >>>> On 10/15/2019 5:14 AM, Gregory Rose wrote: >>>>> >>>>> >>>>> On 10/8/2019 4:51 PM, Greg Rose wrote: >>>>>> From: wenxu <wenxu@ucloud.cn> >>>>>> >>>>>> Upstream commit: >>>>>> commit 18b6f717483a835fb98de9f0df6c724df9324e78 >>>>>> Author: wenxu <wenxu@ucloud.cn> >>>>>> Date: Thu Mar 28 12:43:23 2019 +0800 >>>>>> >>>>>> openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode >>>>>> >>>>>> 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> >>>>>> Acked-by: Pravin B Shelar <pshelar@ovn.org> >>>>>> Signed-off-by: David S. Miller <davem@davemloft.net> >>>>>> >>>>>> Also fixup case statement in lib/odp-util.c to make the compiler happy. >>>>>> >>>>>> Cc: wenxu <wenxu@ucloud.cn> >>>>>> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >>>>> >>>>> Hi Wenxu, >>>>> >>>>> would you mind giving this patch a review - I just want to make sure I didn't miss anything. >>>>> >>>>> Thanks, >>>>> >>>>> - Greg >>>>> >>>> Looks good to me! >>>> >>>> >>>> Thanks >>>> >>>> wenxu >>>> >>> >>> Maintainers, >>> >>> I submited these patches back in October 2019 and then never got further >>> with them. Since then the patch set is moribund I guess but I'm wondering if I should resubmit? >> >> Hi. Yes, it'll be great if you can re-submit at least to check that it still >> applicable and up to date. >> >> Regarding this feature (IP_TUNNEL_INFO_BRIDGE) I also reviewed the userspace >> part last week, but had no reply so far. And so I'm not sure if anyone will >> actually continue to work on this userspace part: >> https://mail.openvswitch.org/pipermail/ovs-dev/2020-September/374733.html > > Well the kernel bits make no sense without the userspace bits. Let > me see if I can advance both. Busy with some other stuff but should get > back to this soon. I'll resubmit the kernel and the userspace patches > together and copy wenxu when I do. That would be very helpful. Thanks! Best regards, Ilya Maximets.
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c index 0f7ab53..b3d1069 100644 --- a/datapath/flow_netlink.c +++ b/datapath/flow_netlink.c @@ -406,6 +406,7 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] [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_IPV4_INFO_BRIDGE] = { .len = 0 }, }; static const struct ovs_len_tbl @@ -669,6 +670,7 @@ static int ip_tun_from_nlattr(const struct nlattr *attr, bool log) { bool ttl = false, ipv4 = false, ipv6 = false; + bool info_bridge_mode = false; __be16 tun_flags = 0; int opts_type = 0; struct nlattr *a; @@ -785,6 +787,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_IPV4_INFO_BRIDGE: + info_bridge_mode = true; + ipv4 = true; + break; default: OVS_NLERR(log, "Unknown IP tunnel attribute %d", type); @@ -815,16 +821,29 @@ 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 (info_bridge_mode) { + if (match->key->tun_key.u.ipv4.src || + match->key->tun_key.u.ipv4.dst || + match->key->tun_key.tp_src || + match->key->tun_key.tp_dst || + match->key->tun_key.ttl || + match->key->tun_key.tos || + tun_flags & ~TUNNEL_KEY) { + OVS_NLERR(log, "IPv4 tun info is not correct"); + return -EINVAL; + } + } else if (!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"); return -EINVAL; } - if (!ttl) { + if (!ttl && !info_bridge_mode) { OVS_NLERR(log, "IP tunnel TTL not specified."); return -EINVAL; } @@ -853,12 +872,17 @@ static int vxlan_opt_to_nlattr(struct sk_buff *skb, static int __ip_tun_to_nlattr(struct sk_buff *skb, const struct ip_tunnel_key *output, const void *tun_opts, int swkey_tun_opts_len, - unsigned short tun_proto) + unsigned short tun_proto, u8 mode) { if (output->tun_flags & TUNNEL_KEY && nla_put_be64(skb, OVS_TUNNEL_KEY_ATTR_ID, output->tun_id, OVS_TUNNEL_KEY_ATTR_PAD)) return -EMSGSIZE; + + if (mode & IP_TUNNEL_INFO_BRIDGE) + return nla_put_flag(skb, OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE) + ? -EMSGSIZE : 0; + switch (tun_proto) { case AF_INET: if (output->u.ipv4.src && @@ -921,7 +945,7 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb, static int ip_tun_to_nlattr(struct sk_buff *skb, const struct ip_tunnel_key *output, const void *tun_opts, int swkey_tun_opts_len, - unsigned short tun_proto) + unsigned short tun_proto, u8 mode) { struct nlattr *nla; int err; @@ -931,7 +955,7 @@ static int ip_tun_to_nlattr(struct sk_buff *skb, return -EMSGSIZE; err = __ip_tun_to_nlattr(skb, output, tun_opts, swkey_tun_opts_len, - tun_proto); + tun_proto, mode); if (err) return err; @@ -945,7 +969,7 @@ int ovs_nla_put_tunnel_info(struct sk_buff *skb, return __ip_tun_to_nlattr(skb, &tun_info->key, ip_tunnel_info_opts(tun_info), tun_info->options_len, - ip_tunnel_info_af(tun_info)); + ip_tunnel_info_af(tun_info), tun_info->mode); } static int encode_vlan_from_nlattrs(struct sw_flow_match *match, @@ -1982,7 +2006,7 @@ static int __ovs_nla_put_key(const struct sw_flow_key *swkey, opts = TUN_METADATA_OPTS(output, swkey->tun_opts_len); if (ip_tun_to_nlattr(skb, &output->tun_key, opts, - swkey->tun_opts_len, swkey->tun_proto)) + swkey->tun_opts_len, swkey->tun_proto, 0)) goto nla_put_failure; } @@ -2610,6 +2634,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 @@ -3375,7 +3401,7 @@ static int set_action_to_attr(const struct nlattr *a, struct sk_buff *skb) err = ip_tun_to_nlattr(skb, &tun_info->key, ip_tunnel_info_opts(tun_info), tun_info->options_len, - ip_tunnel_info_af(tun_info)); + ip_tunnel_info_af(tun_info), tun_info->mode); if (err) return err; nla_nest_end(skb, start); diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index 7b16b1d..2f781ca 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -400,6 +400,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_IPV4_INFO_BRIDGE, /* No argument. IPV4_INFO_BRIDGE mode.*/ __OVS_TUNNEL_KEY_ATTR_MAX }; diff --git a/lib/odp-util.c b/lib/odp-util.c index 3a574bf..949fd90 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -3892,6 +3892,8 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr, format_odp_tun_erspan_opt(a, ma, ds, verbose); ds_put_cstr(ds, "),"); break; + case OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE: + break; case __OVS_TUNNEL_KEY_ATTR_MAX: default: format_unknown_key(ds, a, ma);