On Wed, Dec 16, 2020 at 01:46:16PM +0100, Eelco Chaudron wrote:
> 
> 
> On 15 Dec 2020, at 14:40, Martin Varghese wrote:
> 
> > On Tue, Dec 15, 2020 at 12:28:28PM +0100, Eelco Chaudron wrote:
> > > Hi Martin,
> > > 
> > > See below some small documentation comments.
> > > 
> > > The code itself looks good and tested fine.
> > > 
> > > //Eelco
> > > 
> > > 
> > > On 14 Dec 2020, at 13:40, 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.
> > > > 
> > > >  Documentation/automake.mk                     |  1 +
> > > >  Documentation/faq/bareudp.rst                 | 83 ++++++++++++++++
> > > >  Documentation/faq/index.rst                   |  1 +
> > > >  Documentation/faq/releases.rst                |  1 +
> > > >  NEWS                                          |  5 +-
> > > >  .../linux/compat/include/linux/openvswitch.h  |  9 ++
> > > >  lib/dpif-netlink-rtnl.c                       | 50 ++++++++++
> > > >  lib/dpif-netlink.c                            |  5 +
> > > >  lib/netdev-vport.c                            | 35 ++++++-
> > > >  lib/netdev.h                                  |  1 +
> > > >  ofproto/ofproto-dpif-xlate.c                  |  1 +
> > > >  tests/system-layer3-tunnels.at                | 96
> > > > +++++++++++++++++++
> > > >  vswitchd/vswitch.xml                          | 20 ++++
> > > >  13 files changed, 305 insertions(+), 3 deletions(-)
> > > >  create mode 100644 Documentation/faq/bareudp.rst
> > > > 
> > > > diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> > > > index f85c4320e..ea3475f35 100644
> > > > --- a/Documentation/automake.mk
> > > > +++ b/Documentation/automake.mk
> > > > @@ -88,6 +88,7 @@ DOC_SOURCE = \
> > > >         Documentation/faq/terminology.rst \
> > > >         Documentation/faq/vlan.rst \
> > > >         Documentation/faq/vxlan.rst \
> > > > +       Documentation/faq/bareudp.rst \
> > > >         Documentation/internals/index.rst \
> > > >         Documentation/internals/authors.rst \
> > > >         Documentation/internals/bugs.rst \
> > > > diff --git a/Documentation/faq/bareudp.rst
> > > > b/Documentation/faq/bareudp.rst
> > > > new file mode 100644
> > > > index 000000000..0e58a3b8e
> > > > --- /dev/null
> > > > +++ b/Documentation/faq/bareudp.rst
> > > > @@ -0,0 +1,83 @@
> > > > +..
> > > > +      Licensed under the Apache License, Version 2.0 (the
> > > > "License");
> > > > you may
> > > > +      not use this file except in compliance with the License.
> > > > You may
> > > > obtain
> > > > +      a copy of the License at
> > > > +
> > > > +          http://www.apache.org/licenses/LICENSE-2.0
> > > > +
> > > > +      Unless required by applicable law or agreed to in writing,
> > > > software
> > > > +      distributed under the License is distributed on an "AS
> > > > IS" BASIS,
> > > > WITHOUT
> > > > +      WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> > > > implied.
> > > > See the
> > > > +      License for the specific language governing permissions and
> > > > limitations
> > > > +      under the License.
> > > > +
> > > > +      Convention for heading levels in Open vSwitch documentation:
> > > > +
> > > > +      =======  Heading 0 (reserved for the title in a document)
> > > > +      -------  Heading 1
> > > > +      ~~~~~~~  Heading 2
> > > > +      +++++++  Heading 3
> > > > +      '''''''  Heading 4
> > > > +
> > > > +      Avoid deeper levels because they do not render well.
> > > > +
> > > > +=======
> > > > +Bareudp
> > > > +=======
> > > > +
> > > > +Q: What is Bareudp?
> > > > +
> > > > +    A: 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
> > > > unicasttraffic is
> > > > +       given below.::
> > > > +
> > > > +           $ ovs-vsctl add-port br0 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 option payload_type specifies the ethertype of the l3
> > > > protocol which
> > > > +       the bareudp device will be tunnelling.
> > > > +
> > > > +       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  br0 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
> > > > +
> > > > +       An example to create bareudp device to tunnel
> > > > +       IPv4 & IPv6 traffic is given below.::
> > > > +
> > > > +           $ ovs-vsctl add-port  br0 ip_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=IP options:dst_port=6636
> > > 
> > > Small nit, maybe put the flow example below, above the IP examples.
> > > 
> > will rearrange.
> > > > +       The below example ovs rule shows how a bareudp tunnel
> > > > port is
> > > > used to
> > > > +       tunnel an MPLS packet inside a UDP tunnel.::
> > > > +
> > > > +          $ ovs-ofctl -O OpenFlow13 add-flow br0
> > > > "in_port=10,dl_type=0x0800,\
> > > > +            actions=push_mpls:0x8847,set_field:13->mpls_label,\
> > > 
> > > Can you change 13 above to 3, as 13 is a reserved label that will
> > > mess up
> > > the packet decodes (on Wireshark for example)?
> > > 
> > Will change
> > > > +            output:mpls_udp_port"
> > > > +
> > > > +       This rule does MPLS encapsulation on IP packets and
> > > > sends the l3
> > > > MPLS
> > > > +       packets on a bareudp tunnel port which has its payload_type
> > > > configured
> > > > +       to 0x8847.
> > > > +
> > > > diff --git a/Documentation/faq/index.rst
> > > > b/Documentation/faq/index.rst
> > > > index 334b828b2..1dd29986a 100644
> > > > --- a/Documentation/faq/index.rst
> > > > +++ b/Documentation/faq/index.rst
> > > > @@ -30,6 +30,7 @@ Open vSwitch FAQ
> > > >  .. toctree::
> > > >     :maxdepth: 2
> > > > 
> > > > +   bareudp
> > > >     configuration
> > > >     contributing
> > > >     design
> > > > diff --git a/Documentation/faq/releases.rst
> > > > b/Documentation/faq/releases.rst
> > > > index 3623e3f40..68cbf1dbc 100644
> > > > --- a/Documentation/faq/releases.rst
> > > > +++ b/Documentation/faq/releases.rst
> > > > @@ -138,6 +138,7 @@ Q: Are all features available with all
> > > > datapaths?
> > > >      Tunnel - ERSPAN                 4.18           2.10
> > > > 2.10
> > > > NO
> > > >      Tunnel - ERSPAN-IPv6            4.18           2.10
> > > > 2.10
> > > > NO
> > > >      Tunnel - GTP-U                  NO             NO
> > > > 2.14
> > > > NO
> > > > +    Tunnel - Bareudp                5.7            NO           NO
> > > > NO
> > > >      QoS - Policing                  YES            1.1          2.6
> > > > NO
> > > >      QoS - Shaping                   YES            1.1          NO
> > > > NO
> > > >      sFlow                           YES            1.0          1.0
> > > > NO
> > > > diff --git a/NEWS b/NEWS
> > > > index 7e291a180..e3bc34a3f 100644
> > > > --- a/NEWS
> > > > +++ b/NEWS
> > > > @@ -75,7 +75,10 @@ v2.14.0 - 17 Aug 2020
> > > >     - GTP-U Tunnel Protocol
> > > >       * Add two new fields: tun_gtpu_flags, tun_gtpu_msgtype.
> > > >       * Only support for userspace datapath.
> > > > -
> > > > +   - Bareudp Tunnel
> > > > +     * Bareudp device support is present in linux kernel from
> > > > version
> > > > 5.7
> > > > +     * Kernel bareudp device is not backported to ovs tree.
> > > > +     * Userspace datapath support is not added
> > > > 
> > > >  v2.13.0 - 14 Feb 2020
> > > >  ---------------------
> > > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > > > b/datapath/linux/compat/include/linux/openvswitch.h
> > > > index 2d884312f..53d4225ec 100644
> > > > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > > > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > > > @@ -246,6 +246,7 @@ enum ovs_vport_type {
> > > >         OVS_VPORT_TYPE_IP6ERSPAN = 108, /* ERSPAN tunnel. */
> > > >         OVS_VPORT_TYPE_IP6GRE = 109,
> > > >         OVS_VPORT_TYPE_GTPU = 110,
> > > > +       OVS_VPORT_TYPE_BAREUDP = 111,  /* Bareudp tunnel. */
> > > >         __OVS_VPORT_TYPE_MAX
> > > >  };
> > > > 
> > > > @@ -308,6 +309,14 @@ enum {
> > > > 
> > > >  #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
> > > > 
> > > > +enum {
> > > > +        OVS_BAREUDP_EXT_UNSPEC,
> > > > +        OVS_BAREUDP_EXT_MULTIPROTO_MODE,
> > > > +        __OVS_BAREUDP_EXT_MAX,
> > > > +};
> > > > +
> > > > +#define OVS_BAREUDP_EXT_MAX (__OVS_BAREUDP_EXT_MAX - 1)
> > > > +
> > > >  /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
> > > >   */
> > > >  enum {
> > > > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> > > > index fd157ce2d..6e1243ae7 100644
> > > > --- a/lib/dpif-netlink-rtnl.c
> > > > +++ b/lib/dpif-netlink-rtnl.c
> > > > @@ -58,6 +58,18 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink_rtnl);
> > > >  #define IFLA_GENEVE_UDP_ZERO_CSUM6_RX 10
> > > >  #endif
> > > > 
> > > > +#ifndef __IFLA_BAREUDP_MAX
> > > > +#define IFLA_BAREUDP_MAX 0
> > > > +#endif
> > > > +#if IFLA_BAREUDP_MAX < 4
> > > > +#define IFLA_BAREUDP_PORT 1
> > > > +#define IFLA_BAREUDP_ETHERTYPE 2
> > > > +#define IFLA_BAREUDP_SRCPORT_MIN 3
> > > > +#define IFLA_BAREUDP_MULTIPROTO_MODE 4
> > > > +#endif
> > > > +
> > > > +#define BAREUDP_EPHEMERAL_SRCPORT_MIN 49153
> > > 
> > > Not sure if ephemeral makes sense here, as the stream could be very
> > > long-lived?
> > > Why not just cap it BAREUDP_SRCPORT_MIN?
> > > 
> > ok
> > > > +
> > > >  static const struct nl_policy rtlink_policy[] = {
> > > >      [IFLA_LINKINFO] = { .type = NL_A_NESTED },
> > > >  };
> > > > @@ -81,6 +93,10 @@ static const struct nl_policy geneve_policy[] = {
> > > >      [IFLA_GENEVE_UDP_ZERO_CSUM6_RX] = { .type = NL_A_U8 },
> > > >      [IFLA_GENEVE_PORT] = { .type = NL_A_U16 },
> > > >  };
> > > > +static const struct nl_policy bareudp_policy[] = {
> > > > +    [IFLA_BAREUDP_PORT] = { .type = NL_A_U16 },
> > > > +    [IFLA_BAREUDP_ETHERTYPE] = { .type = NL_A_U16 },
> > > > +};
> > > > 
> > > >  static const char *
> > > >  vport_type_to_kind(enum ovs_vport_type type,
> > > > @@ -113,6 +129,8 @@ vport_type_to_kind(enum ovs_vport_type type,
> > > >          }
> > > >      case OVS_VPORT_TYPE_GTPU:
> > > >          return NULL;
> > > > +    case OVS_VPORT_TYPE_BAREUDP:
> > > > +        return "bareudp";
> > > >      case OVS_VPORT_TYPE_NETDEV:
> > > >      case OVS_VPORT_TYPE_INTERNAL:
> > > >      case OVS_VPORT_TYPE_LISP:
> > > > @@ -243,6 +261,24 @@ dpif_netlink_rtnl_geneve_verify(const struct
> > > > netdev_tunnel_config *tnl_cfg,
> > > > 
> > > >      return err;
> > > >  }
> > > > +static int
> > > > +dpif_netlink_rtnl_bareudp_verify(const struct netdev_tunnel_config
> > > > *tnl_cfg,
> > > > +                                const char *kind, struct ofpbuf
> > > > *reply)
> > > > +{
> > > > +    struct nlattr *bareudp[ARRAY_SIZE(bareudp_policy)];
> > > > +    int err;
> > > > +
> > > > +    err = rtnl_policy_parse(kind, reply, bareudp_policy, bareudp,
> > > > +                            ARRAY_SIZE(bareudp_policy));
> > > > +    if (!err) {
> > > > +        if ((tnl_cfg->dst_port !=
> > > > nl_attr_get_be16(bareudp[IFLA_BAREUDP_PORT]))
> > > > +            || (tnl_cfg->payload_ethertype
> > > > +                !=
> > > > nl_attr_get_be16(bareudp[IFLA_BAREUDP_ETHERTYPE])))
> > > > {
> > > > +            err = EINVAL;
> > > > +        }
> > > > +    }
> > > > +    return err;
> > > > +}
> > > > 
> > > >  static int
> > > >  dpif_netlink_rtnl_verify(const struct netdev_tunnel_config
> > > > *tnl_cfg,
> > > > @@ -275,6 +311,9 @@ dpif_netlink_rtnl_verify(const struct
> > > > netdev_tunnel_config *tnl_cfg,
> > > >      case OVS_VPORT_TYPE_GENEVE:
> > > >          err = dpif_netlink_rtnl_geneve_verify(tnl_cfg, kind,
> > > > reply);
> > > >          break;
> > > > +    case OVS_VPORT_TYPE_BAREUDP:
> > > > +        err = dpif_netlink_rtnl_bareudp_verify(tnl_cfg, kind,
> > > > reply);
> > > > +        break;
> > > >      case OVS_VPORT_TYPE_NETDEV:
> > > >      case OVS_VPORT_TYPE_INTERNAL:
> > > >      case OVS_VPORT_TYPE_LISP:
> > > > @@ -357,6 +396,16 @@ dpif_netlink_rtnl_create(const struct
> > > > netdev_tunnel_config *tnl_cfg,
> > > >          nl_msg_put_u8(&request, IFLA_GENEVE_UDP_ZERO_CSUM6_RX, 1);
> > > >          nl_msg_put_be16(&request, IFLA_GENEVE_PORT,
> > > > tnl_cfg->dst_port);
> > > >          break;
> > > > +    case OVS_VPORT_TYPE_BAREUDP:
> > > > +        nl_msg_put_be16(&request, IFLA_BAREUDP_ETHERTYPE,
> > > > +                        tnl_cfg->payload_ethertype);
> > > > +        nl_msg_put_u16(&request, IFLA_BAREUDP_SRCPORT_MIN,
> > > > +                       BAREUDP_EPHEMERAL_SRCPORT_MIN);
> > > > +        nl_msg_put_be16(&request, IFLA_BAREUDP_PORT,
> > > > tnl_cfg->dst_port);
> > > > +        if (tnl_cfg->exts & (1 <<
> > > > OVS_BAREUDP_EXT_MULTIPROTO_MODE)) {
> > > > +            nl_msg_put_flag(&request,
> > > > IFLA_BAREUDP_MULTIPROTO_MODE);
> > > > +        }
> > > > +        break;
> > > >      case OVS_VPORT_TYPE_NETDEV:
> > > >      case OVS_VPORT_TYPE_INTERNAL:
> > > >      case OVS_VPORT_TYPE_LISP:
> > > > @@ -470,6 +519,7 @@ dpif_netlink_rtnl_port_destroy(const char *name,
> > > > const char *type)
> > > >      case OVS_VPORT_TYPE_ERSPAN:
> > > >      case OVS_VPORT_TYPE_IP6ERSPAN:
> > > >      case OVS_VPORT_TYPE_IP6GRE:
> > > > +    case OVS_VPORT_TYPE_BAREUDP:
> > > >          return dpif_netlink_rtnl_destroy(name);
> > > >      case OVS_VPORT_TYPE_NETDEV:
> > > >      case OVS_VPORT_TYPE_INTERNAL:
> > > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > > > index 6858ba612..d1a1cc723 100644
> > > > --- a/lib/dpif-netlink.c
> > > > +++ b/lib/dpif-netlink.c
> > > > @@ -752,6 +752,9 @@ get_vport_type(const struct dpif_netlink_vport
> > > > *vport)
> > > >      case OVS_VPORT_TYPE_GTPU:
> > > >          return "gtpu";
> > > > 
> > > > +    case OVS_VPORT_TYPE_BAREUDP:
> > > > +        return "bareudp";
> > > > +
> > > >      case OVS_VPORT_TYPE_UNSPEC:
> > > >      case __OVS_VPORT_TYPE_MAX:
> > > >          break;
> > > > @@ -787,6 +790,8 @@ netdev_to_ovs_vport_type(const char *type)
> > > >          return OVS_VPORT_TYPE_GRE;
> > > >      } else if (!strcmp(type, "gtpu")) {
> > > >          return OVS_VPORT_TYPE_GTPU;
> > > > +    } else if (!strcmp(type, "bareudp")) {
> > > > +        return OVS_VPORT_TYPE_BAREUDP;
> > > >      } else {
> > > >          return OVS_VPORT_TYPE_UNSPEC;
> > > >      }
> > > > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> > > > index 0252b61de..15567e524 100644
> > > > --- a/lib/netdev-vport.c
> > > > +++ b/lib/netdev-vport.c
> > > > @@ -47,6 +47,7 @@
> > > >  #include "unaligned.h"
> > > >  #include "unixctl.h"
> > > >  #include "openvswitch/vlog.h"
> > > > +#include "openvswitch/ofp-parse.h"
> > > >  #ifdef __linux__
> > > >  #include "netdev-linux.h"
> > > >  #endif
> > > > @@ -112,7 +113,7 @@ netdev_vport_needs_dst_port(const struct netdev
> > > > *dev)
> > > >      return (class->get_config == get_tunnel_config &&
> > > >              (!strcmp("geneve", type) || !strcmp("vxlan", type) ||
> > > >               !strcmp("lisp", type) || !strcmp("stt", type) ||
> > > > -             !strcmp("gtpu", type)));
> > > > +             !strcmp("gtpu", type) || !strcmp("bareudp",type)));
> > > >  }
> > > > 
> > > >  const char *
> > > > @@ -219,6 +220,8 @@ netdev_vport_construct(struct netdev *netdev_)
> > > >          dev->tnl_cfg.dst_port = port ? htons(port) :
> > > > htons(STT_DST_PORT);
> > > >      } else if (!strcmp(type, "gtpu")) {
> > > >          dev->tnl_cfg.dst_port = port ? htons(port) :
> > > > htons(GTPU_DST_PORT);
> > > > +    } else if (!strcmp(type, "bareudp")) {
> > > > +        dev->tnl_cfg.dst_port = htons(port);
> > > >      }
> > > > 
> > > >      dev->tnl_cfg.dont_fragment = true;
> > > > @@ -438,6 +441,8 @@ tunnel_supported_layers(const char *type,
> > > >          return TNL_L2 | TNL_L3;
> > > >      } else if (!strcmp(type, "gtpu")) {
> > > >          return TNL_L3;
> > > > +    } else if (!strcmp(type, "bareudp")) {
> > > > +        return TNL_L3;
> > > >      } else {
> > > >          return TNL_L2;
> > > >      }
> > > > @@ -745,6 +750,23 @@ set_tunnel_config(struct netdev *dev_,
> > > > const struct
> > > > smap *args, char **errp)
> > > >                      goto out;
> > > >                  }
> > > >              }
> > > > +        } else if (!strcmp(node->key, "payload_type")) {
> > > > +            if (!strcmp(node->value, "mpls")) {
> > > > +                 tnl_cfg.payload_ethertype = htons(ETH_TYPE_MPLS);
> > > > +                 tnl_cfg.exts |= (1 <<
> > > > OVS_BAREUDP_EXT_MULTIPROTO_MODE);
> > > > +            } else if (!strcmp(node->value, "ip")) {
> > > > +                 tnl_cfg.payload_ethertype = htons(ETH_TYPE_IP);
> > > > +                 tnl_cfg.exts |= (1 <<
> > > > OVS_BAREUDP_EXT_MULTIPROTO_MODE);
> > > > +            } else {
> > > > +                 uint16_t payload_ethertype;
> > > > +
> > > > +                 if (str_to_u16(node->value, "payload_type",
> > > > +                                &payload_ethertype)) {
> > > > +                     err = EINVAL;
> > > > +                     goto out;
> > > > +                 }
> > > > +                 tnl_cfg.payload_ethertype =
> > > > htons(payload_ethertype);
> > > > +            }
> > > >          } else {
> > > >              ds_put_format(&errors, "%s: unknown %s argument
> > > > '%s'\n",
> > > > name,
> > > >                            type, node->key);
> > > > @@ -917,7 +939,8 @@ get_tunnel_config(const struct netdev *dev,
> > > > struct
> > > > smap *args)
> > > >              (!strcmp("vxlan", type) && dst_port !=
> > > > VXLAN_DST_PORT) ||
> > > >              (!strcmp("lisp", type) && dst_port != LISP_DST_PORT) ||
> > > >              (!strcmp("stt", type) && dst_port != STT_DST_PORT) ||
> > > > -            (!strcmp("gtpu", type) && dst_port != GTPU_DST_PORT)) {
> > > > +            (!strcmp("gtpu", type) && dst_port != GTPU_DST_PORT) ||
> > > > +            !strcmp("bareudp", type)) {
> > > >              smap_add_format(args, "dst_port", "%d", dst_port);
> > > >          }
> > > >      }
> > > > @@ -1243,6 +1266,14 @@ netdev_vport_tunnel_register(void)
> > > >            },
> > > >            {{NULL, NULL, 0, 0}}
> > > >          },
> > > > +        { "udp_sys",
> > > > +          {
> > > > +              TUNNEL_FUNCTIONS_COMMON,
> > > > +              .type = "bareudp",
> > > > +              .get_ifindex = NETDEV_VPORT_GET_IFINDEX,
> > > > +          },
> > > > +          {{NULL, NULL, 0, 0}}
> > > > +        },
> > > > 
> > > >      };
> > > >      static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > > > diff --git a/lib/netdev.h b/lib/netdev.h
> > > > index fb5073056..b705a9e56 100644
> > > > --- a/lib/netdev.h
> > > > +++ b/lib/netdev.h
> > > > @@ -107,6 +107,7 @@ struct netdev_tunnel_config {
> > > >      bool out_key_flow;
> > > >      ovs_be64 out_key;
> > > > 
> > > > +    ovs_be16 payload_ethertype;
> > > >      ovs_be16 dst_port;
> > > > 
> > > >      bool ip_src_flow;
> > > > diff --git a/ofproto/ofproto-dpif-xlate.c
> > > > b/ofproto/ofproto-dpif-xlate.c
> > > > index 11aa20754..7eeff14f6 100644
> > > > --- a/ofproto/ofproto-dpif-xlate.c
> > > > +++ b/ofproto/ofproto-dpif-xlate.c
> > > > @@ -3573,6 +3573,7 @@ propagate_tunnel_data_to_flow(struct xlate_ctx
> > > > *ctx, struct eth_addr dmac,
> > > >      case OVS_VPORT_TYPE_VXLAN:
> > > >      case OVS_VPORT_TYPE_GENEVE:
> > > >      case OVS_VPORT_TYPE_GTPU:
> > > > +    case OVS_VPORT_TYPE_BAREUDP:
> > > >          nw_proto = IPPROTO_UDP;
> > > >          break;
> > > >      case OVS_VPORT_TYPE_LISP:
> > > > diff --git a/tests/system-layer3-tunnels.at
> > > > b/tests/system-layer3-tunnels.at
> > > > index 1232964bb..d21fd777d 100644
> > > > --- a/tests/system-layer3-tunnels.at
> > > > +++ b/tests/system-layer3-tunnels.at
> > > > @@ -152,3 +152,99 @@ AT_CHECK([tail -1 stdout], [0],
> > > > 
> > > >  OVS_VSWITCHD_STOP
> > > >  AT_CLEANUP
> > > > +
> > > > +AT_SETUP([layer3 - ping over MPLS Bareudp])
> > > > +OVS_CHECK_MIN_KERNEL(5, 7)
> > > > +OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])])
> > > > +ADD_NAMESPACES(at_ns0, at_ns1)
> > > > +
> > > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "36:b1:ee:7c:01:01")
> > > > +ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24", "36:b1:ee:7c:01:02")
> > > > +
> > > > +ADD_OVS_TUNNEL([bareudp], [br0], [at_bareudp0], [8.1.1.3],
> > > > [8.1.1.2/24],
> > > > +               [ options:local_ip=8.1.1.2
> > > > options:packet_type="legacy_l3" options:payload_type=mpls
> > > > options:dst_port=6635])
> > > > +
> > > > +ADD_OVS_TUNNEL([bareudp], [br1], [at_bareudp1], [8.1.1.2],
> > > > [8.1.1.3/24],
> > > > +               [options:local_ip=8.1.1.3
> > > > options:packet_type="legacy_l3" options:payload_type=mpls
> > > > options:dst_port=6635])
> > > > +
> > > > +AT_DATA([flows0.txt], [dnl
> > > > +table=0,priority=100,dl_type=0x0800
> > > > actions=push_mpls:0x8847,set_mpls_label:3,output:at_bareudp0
> > > > +table=0,priority=100,dl_type=0x8847 in_port=at_bareudp0 
> > > > actions=pop_mpls:0x0800,set_field:36:b1:ee:7c:01:01->dl_dst,set_field:36:b1:ee:7c:01:02->dl_src,output:ovs-p0
> > > > +table=0,priority=10 actions=normal
> > > > +])
> > > > +
> > > > +AT_DATA([flows1.txt], [dnl
> > > > +table=0,priority=100,dl_type=0x0800
> > > > actions=push_mpls:0x8847,set_mpls_label:3,output:at_bareudp1
> > > > +table=0,priority=100,dl_type=0x8847 in_port=at_bareudp1 
> > > > actions=pop_mpls:0x0800,set_field:36:b1:ee:7c:01:02->dl_dst,set_field:36:b1:ee:7c:01:01->dl_src,output:ovs-p1
> > > > +table=0,priority=10 actions=normal
> > > > +])
> > > > +
> > > > +AT_CHECK([ip link add patch0 type veth peer name patch1])
> > > > +on_exit 'ip link del patch0'
> > > > +
> > > > +AT_CHECK([ip link set dev patch0 up])
> > > > +AT_CHECK([ip link set dev patch1 up])
> > > > +AT_CHECK([ovs-vsctl add-port br0 patch0])
> > > > +AT_CHECK([ovs-vsctl add-port br1 patch1])
> > > > +
> > > > +
> > > > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows0.txt])
> > > > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br1 flows1.txt])
> > > > +
> > > > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
> > > > FORMAT_PING], [0], [dnl
> > > > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > > > +])
> > > > +
> > > > +NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 |
> > > > FORMAT_PING], [0], [dnl
> > > > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > > > +])
> > > > +OVS_TRAFFIC_VSWITCHD_STOP
> > > > +AT_CLEANUP
> > > > +
> > > > +AT_SETUP([layer3 - ping over Bareudp])
> > > > +OVS_CHECK_MIN_KERNEL(5, 7)
> > > > +OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])])
> > > > +ADD_NAMESPACES(at_ns0, at_ns1)
> > > > +
> > > > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "36:b1:ee:7c:01:01")
> > > > +ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24", "36:b1:ee:7c:01:02")
> > > > +
> > > > +ADD_OVS_TUNNEL([bareudp], [br0], [at_bareudp0], [8.1.1.3],
> > > > [8.1.1.2/24],
> > > > +               [ options:local_ip=8.1.1.2
> > > > options:packet_type="legacy_l3" options:payload_type=ip
> > > > options:dst_port=6636])
> > > > +
> > > > +ADD_OVS_TUNNEL([bareudp], [br1], [at_bareudp1], [8.1.1.2],
> > > > [8.1.1.3/24],
> > > > +               [options:local_ip=8.1.1.3
> > > > options:packet_type="legacy_l3" options:payload_type=ip
> > > > options:dst_port=6636])
> > > > +
> > > > +AT_DATA([flows0.txt], [dnl
> > > > +table=0,priority=100,dl_type=0x0800 in_port=ovs-p0,
> > > > actions=output:at_bareudp0
> > > > +table=0,priority=100,dl_type=0x0800 in_port=at_bareudp0 
> > > > actions=set_field:36:b1:ee:7c:01:01->dl_dst,set_field:36:b1:ee:7c:01:02->dl_src,output:ovs-p0
> > > > +table=0,priority=10 actions=normal
> > > > +])
> > > > +
> > > > +AT_DATA([flows1.txt], [dnl
> > > > +table=0,priority=100,dl_type=0x0800 in_port=ovs-p1
> > > > actions=output:at_bareudp1
> > > > +table=0,priority=100,dl_type=0x0800 in_port=at_bareudp1 
> > > > actions=set_field:36:b1:ee:7c:01:02->dl_dst,set_field:36:b1:ee:7c:01:01->dl_src,output:ovs-p1
> > > > +table=0,priority=10 actions=normal
> > > > +])
> > > > +
> > > > +AT_CHECK([ip link add patch0 type veth peer name patch1])
> > > > +on_exit 'ip link del patch0'
> > > > +
> > > > +AT_CHECK([ip link set dev patch0 up])
> > > > +AT_CHECK([ip link set dev patch1 up])
> > > > +AT_CHECK([ovs-vsctl add-port br0 patch0])
> > > > +AT_CHECK([ovs-vsctl add-port br1 patch1])
> > > > +
> > > > +
> > > > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows0.txt])
> > > > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br1 flows1.txt])
> > > > +
> > > > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
> > > > FORMAT_PING], [0], [dnl
> > > > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > > > +])
> > > > +
> > > > +NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 |
> > > > FORMAT_PING], [0], [dnl
> > > > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > > > +])
> > > > +OVS_TRAFFIC_VSWITCHD_STOP
> > > > +AT_CLEANUP
> > > > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > > > index 89a876796..75cc4263a 100644
> > > > --- a/vswitchd/vswitch.xml
> > > > +++ b/vswitchd/vswitch.xml
> > > > @@ -2694,6 +2694,15 @@
> > > >              </p>
> > > >            </dd>
> > > > 
> > > > +          <dt><code>Bareudp</code></dt>
> > > > +          <dd>
> > > > +            <p>
> > > > +            The Bareudp tunnel provides a generic L3 encapsulation
> > > > support for
> > > > +            tunnelling different L3 protocols like MPLS, IP,
> > > > NSH etc.
> > > > inside a
> > > > +            UDP tunnel.
> > > > +            </p>
> > > > +          </dd>
> > > > +
> > > >          </dl>
> > > >        </column>
> > > >      </group>
> > > > @@ -3092,6 +3101,17 @@
> > > >        </column>
> > > >      </group>
> > > > 
> > > > +    <group title="Tunnel Options: Bareudp only">
> > > > +      <column name="options" key="payload_type">
> > > > +        <p>
> > > > +           Specifies the ethertype of the l3 protocol the bareudp
> > > > +           device is tunnelling. For the tunnels which supports
> > > > multiple
> > > > +           ethertypes of a l3 protocol (IP, MPLS) this field
> > > > specifies
> > > > the
> > > > +           protocol name as a string.
> > > > +        </p>
> > > > +      </column>
> > > > +    </group>
> > > > +
> > > >      <group title="Patch Options">
> > > >        <p>
> > > >          These options apply only to <dfn>patch ports</dfn>,
> > > > that is,
> > > > interfaces
> > > 
> > > The tunnel option group text needs updating as [in_|_out_]key
> > > options do not
> > > apply to bareudp.
> > Yes i will add a note
> 
> 2701     <group title="Tunnel Options">
> 2702       <p>
> 2703         These options apply to interfaces with <ref column="type"/> of
> 2704         <code>geneve</code>, <code>gre</code>, <code>ip6gre</code>,
> 2705         <code>vxlan</code>, <code>lisp</code> and <code>stt</code>.
> 2706       </p>
> 
yes.
> Guess this section needs bareudp
> 
> > > Also, the individual global options that do not apply to bareudp
> > > might need
> > > a note.
> 
> 
> > Except the key option i see every options in the sections
> > Tunnels options & Tunnel options for gre,vlan,geneve is applicable for
> > bareudp also. Do you see anything obvious ?
> > 
> > I will add bareudp along vxlan,geneve etc in places relevant.
> > 
> 
> 2708       <p>
> 2709         Each tunnel must be uniquely identified by the combination of
> <ref
> 2710         column="type"/>, <ref column="options" key="remote_ip"/>, <ref
> 2711         column="options" key="local_ip"/>, and <ref column="options"
> 2712         key="in_key"/>.  If two ports are defined that are the same
> except one
> 2713         has an optional identifier and the other does not, the more
> specific
> 2714         one is matched first.  <ref column="options" key="in_key"/> is
> 2715         considered more specific than <ref column="options"
> key="local_ip"/> if
> 2716         a port defines one and another port defines the other.
> 2717       </p>
> 
> This section needs some wording that bareudp does not take in_key into
> consideration.
Yes.
The key is not applicable for bareudp tunnels. Hence it  is not
considered while identifying a bareudp tunnel.
> 
> 2782       <column name="options" key="in_key">
> 2809       <column name="options" key="out_key">
> 2838       <column name="options" key="key">
> 
> Need some comment that they do not apply to bareudp
> 
I am planning to add here. 
<p>Optional.  The key that received packets must contain, one of:</p>.
The key is not applicable for bareudp.

Similary for out_key

I propose not to add for key as it is a short hand for in_key & out_key
do you think otherwise


> I’ll assume the following options work with bareudp (I only tested TTL):
> 
> 2843       <column name="options" key="tos">
> 2852       <column name="options" key="ttl">
> 2859       <column name="options" key="df_default"
> 2866       <column name="options" key="egress_pkt_mark">
>
Yes
> > Thanks,
> > Martin
> > 
> > > 
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to