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

Tue Dec 22 11:36:13 UTC 2020

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

I  will send out a new version post christmas. Due to annual shutdown
all my machines are down.

> 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.
> 
> Cheers,
> 
> Eelco
> 
Thanks Eelco for confirming your test results.