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

Martin Varghese martinvarghesenokia at gmail.com
Tue Dec 22 17:24:18 UTC 2020


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


More information about the dev mailing list