On 12/22/20 6:24 PM, Martin Varghese wrote: > 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.
Thanks! Applied to master. Best regards, Ilya Maximets. >>>> >>>>> 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
