[ovs-dev] [PATCH v10] Bareudp Tunnel Support
Ilya Maximets
i.maximets at ovn.org
Tue Dec 22 12:05:46 UTC 2020
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.
>> 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?
>
>> 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.
>>
>> Cheers,
>>
>> Eelco
>>
> Thanks Eelco for confirming your test results.
>
More information about the dev
mailing list