[ovs-dev] [PATCH v10] Bareudp Tunnel Support

Eelco Chaudron echaudro at redhat.com
Tue Dec 22 12:11:11 UTC 2020



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 <martin.varghese at nokia.com>
>>>>>>
>>>>>> 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 <martin.varghese at nokia.com>
>>>>>> Acked-By: Greg Rose <gvrose8192 at gmail.com>
>>>>>> Tested-by: Greg Rose <gvrose8192 at gmail.com>
>>>>>> ---
>>>>>> 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 gvrose8192 at gmail.com
>>>>>>
>>>>>> 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 <echaudro at redhat.com>
>>>>>
>>>>
>>>> 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!

>>
>>> 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.
>>



More information about the dev mailing list