On Tue, Dec 22, 2020 at 01:11:11PM +0100, Eelco Chaudron wrote: > > > On 22 Dec 2020, at 13:05, Ilya Maximets wrote: > > > On 12/22/20 12:36 PM, Martin Varghese wrote: > > > On Tue, Dec 22, 2020 at 09:11:48AM +0100, Eelco Chaudron wrote: > > > > > > > > > > > > On 21 Dec 2020, at 20:32, Ilya Maximets wrote: > > > > > > > > > On 12/17/20 10:46 AM, Eelco Chaudron wrote: > > > > > > > > > > > > > > > > > > On 17 Dec 2020, at 8:18, Martin Varghese wrote: > > > > > > > > > > > > > From: Martin Varghese <[email protected]> > > > > > > > > > > > > > > There are various L3 encapsulation standards using UDP being > > > > > > > discussed to > > > > > > > leverage the UDP based load balancing capability of different > > > > > > > networks. > > > > > > > MPLSoUDP (__ https://tools.ietf.org/html/rfc7510) is > > > > > > > one among them. > > > > > > > > > > > > > > The Bareudp tunnel provides a generic L3 > > > > > > > encapsulation support for > > > > > > > tunnelling different L3 protocols like MPLS, IP, NSH etc. inside > > > > > > > a UDP > > > > > > > tunnel. > > > > > > > > > > > > > > An example to create bareudp device to tunnel MPLS traffic is > > > > > > > given > > > > > > > > > > > > > > $ ovs-vsctl add-port br_mpls udp_port -- set interface udp_port \ > > > > > > > type=bareudp options:remote_ip=2.1.1.3 > > > > > > > options:local_ip=2.1.1.2 \ > > > > > > > options:payload_type=0x8847 options:dst_port=6635 > > > > > > > > > > > > > > The bareudp device supports special handling for MPLS & IP as > > > > > > > they can have multiple ethertypes. MPLS procotcol can have > > > > > > > ethertypes > > > > > > > ETH_P_MPLS_UC (unicast) & ETH_P_MPLS_MC (multicast). IP protocol > > > > > > > can have > > > > > > > ethertypes ETH_P_IP (v4) & ETH_P_IPV6 (v6). > > > > > > > > > > > > > > The bareudp device to tunnel L3 traffic with multiple ethertypes > > > > > > > (MPLS & IP) can be created by passing the L3 protocol name as > > > > > > > string in > > > > > > > the field payload_type. An example to create bareudp device to > > > > > > > tunnel > > > > > > > MPLS unicast & multicast traffic is given below.:: > > > > > > > > > > > > > > $ ovs-vsctl add-port br_mpls udp_port -- set interface > > > > > > > udp_port \ > > > > > > > type=bareudp options:remote_ip=2.1.1.3 > > > > > > > options:local_ip=2.1.1.2 \ > > > > > > > options:payload_type=mpls options:dst_port=6635 > > > > > > > > > > > > > > Signed-off-by: Martin Varghese <[email protected]> > > > > > > > Acked-By: Greg Rose <[email protected]> > > > > > > > Tested-by: Greg Rose <[email protected]> > > > > > > > --- > > > > > > > Changes in v2: > > > > > > > - Removed vport-bareudp module. > > > > > > > > > > > > > > Changes in v3: > > > > > > > - Added net-next upstream commit id and message to commit > > > > > > > message. > > > > > > > > > > > > > > Changes in v4: > > > > > > > - Removed kernel datapath changes. > > > > > > > > > > > > > > Changes in v5: > > > > > > > - Fixed release notes errors. > > > > > > > - Fixed coding errors in dpif-nelink-rtnl.c. > > > > > > > > > > > > > > Changes in v6: > > > > > > > - Added code to enable rx metadata collection in the kernel > > > > > > > device. > > > > > > > - Added version history. > > > > > > > > > > > > > > Changes in v7: > > > > > > > - Fixed release notes errors. > > > > > > > - Added Skip tests for older kernels. > > > > > > > - Changes bareudp ovs_vport_type to 111. > > > > > > > - Added Acked-by & tested by from [email protected] > > > > > > > > > > > > > > Changes in v8: > > > > > > > - The code added in v6 to enable rx metadata collection in > > > > > > > the kernel device is removed. This flag was never added to > > > > > > > any of > > > > > > > the kernel release. The rx metadata collection is always > > > > > > > enabled in > > > > > > > kernel bareudp module. > > > > > > > > > > > > > > Changes in v9: > > > > > > > - Fixed documentation errors. > > > > > > > - Added example usage to create bareudp device for > > > > > > > tunnelling IP. > > > > > > > - Added tests for tunnelling IP. > > > > > > > - Check to restrict configuratoin of starting source port > > > > > > > range as > > > > > > > ephemeral port for MPLS alone is removed. > > > > > > > - Fixed errors in the handling of input string > > > > > > > for the argument > > > > > > > payload_type. > > > > > > > - Added bareudp details for ovs-vswitchd.conf.db > > > > > > > > > > > > > > Changed in v10: > > > > > > > - Re-ordered & fixed examples in documentation. > > > > > > > - Fixed ovs-vswitchd.conf.db. > > > > > > > - Renamed source port min macro name. > > > > > > > - Fixed v9 version change log to add ovs-vswitchd.conf.db > > > > > > > details. > > > > > > > > > > > > Thanks this version looks good to me! > > > > > > > > > > > > Acked-by: Eelco Chaudron <[email protected]> > > > > > > > > > > > > > > > > Hi. > > > > > > > > > > It's not possible to build with this change on a machine > > > > > with bareudp > > > > > support. > > > > > How did you test it? > > > > > > > My test & development enviornment was rhel 8.x with 5.10 kernel > > > > > > > It compiles without any errors on RHEL8.3. > > > > Yes. That is true. It compiles because if_link.h that is shipped > > within > > kernel-headers-4.18.0-240.8.1.el8_3.x86_64 doesn't have defines for > > bareudp. > > I'm assuming that it means that rhel8.3 doesn't support bareudp. > > Not in 8.3, guess 8.4 will. > > > > > I’ve tested this with some old > > > > RHEL kernel without bareudp support for some negative test cases and > > > > upstream 5.10.0-rc5. > > > > > > > > > > > > > > On my Fedora 31 I'm getting: > > > > > lib/dpif-netlink-rtnl.c:62:9: error: 'IFLA_BAREUDP_MAX' > > > > > macro redefined > > > > > [-Werror,-Wmacro-redefined] > > > > > #define IFLA_BAREUDP_MAX 0 > > > > > ^ > > > > > /usr/include/linux/if_link.h:604:9: note: previous > > > > > definition is here > > > > > #define IFLA_BAREUDP_MAX (__IFLA_BAREUDP_MAX - 1) > > > > > ^ > > > > > And that is perfectly correct, because enums and macros are > > > > > different > > > > > things > > > > > and 'ifdef __IFLA_BAREUDP_MAX' makes no sense. > > > > > > > > > > I fixed that locally by replacing __IFLA_BAREUDP_MAX with > > > > > IFLA_BAREUDP_MAX, > > > > > but now I'm concerned if this patch was ever tested at all. > > > > > > > It was embarassing. Apologies for this basic compilation error. > > > The regression runs were also good. > > > https://travis-ci.org/github/martinpattara/ovs/builds/749552661 > > > > Yep. Our CI is based on ubuntu 18.04 that doesn't support bareudp. We > > could > > migrate to 20.04, but it's based on kernel 5.4, so doesn't support > > bareudp too. > > > > > I will send out a new version post christmas. Due to annual shutdown > > > all my machines are down. > > > > There is no need for a new version. If you're OK with the below diff, I > > could > > squash it in before applying the patch: > > > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c > > index 4fc42daed..257b78d8c 100644 > > --- a/lib/dpif-netlink-rtnl.c > > +++ b/lib/dpif-netlink-rtnl.c > > @@ -58,7 +58,7 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl); > > #define IFLA_GENEVE_UDP_ZERO_CSUM6_RX 10 > > #endif > > > > -#ifndef __IFLA_BAREUDP_MAX > > +#ifndef IFLA_BAREUDP_MAX > > #define IFLA_BAREUDP_MAX 0 > > #endif > > #if IFLA_BAREUDP_MAX < 4 > > --- > > > > With above change, I can build OVS and system tests are passing on > > fedora 31. > > > > Martin, Eelco, what do you think? > > Ack from my side! > Ack from my side too. Thanks for fixing it. > > > > > > > I only replaced the old kernel, not its include files, hence the > > > > problem did > > > > not show up. > > > > I did test this patch using a XENA to verify and generate MPLS > > > > packets. > > > > Thanks. I just wanted to know that this patch was tested. Apparently, > > you both > > tested with rhel8 as a base image that doesn't support bareudp (at least > > has no > > defines in kernel headers) and without installing headers from the same > > upstream > > kernel you've tested with. > > I never install the kernel headers of the scratch upstream kernels I test > with, but it might be a good idea in the future ;) > > > > > > > > > Cheers, > > > > > > > > Eelco > > > > > > > Thanks Eelco for confirming your test results. > > > >
Thanks Martin _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
