[ovs-dev,v1,1/2] userspace datapath: Add GTP-U tunnel support

Message ID 1519993921-120968-2-git-send-email-yi.y.yang@intel.com
State Superseded
Delegated to: Ian Stokes
Headers show
Series
  • Add GTP-U tunnel support in DPDK userspace
Related show

Commit Message

Yang, Yi March 2, 2018, 12:32 p.m.
From: Feng Yang <feng.yang@intel.com>

GPRS Tunneling Protocol (GTP) is a group of IP-based communications
protocols used to carry general packet radio service (GPRS) within
GSM, UMTS and LTE networks.

GTP can be decomposed into separate protocols, GTP-C, GTP-U and GTP'.

GTP-U is used for carrying user data within the GPRS core network and
between the radio access network and the core network. The user data
transported can be packets in any of IPv4, IPv6, or PPP formats.

GTP-U is user plane communication protocol between Serving-Gateway
(S-GW) and PDN-Gateway (P-GW) as well as Multi-access Edge Computing
(MEC).

This patch added a new tunnel type 'gtpu' into userspace datapath and
two new match fields tun_gtpu_flags and tun_gtpu_msgtype, tun_id is
reused for GTP Tunnel Endpoint ID (TEID).

The below are commands for gtpu tunnel add:

$ sudo ovs-vsctl add-br br-int -- set bridge br-int datapath_type=netdev protocols=OpenFlow13
$ sudo ovs-vsctl add-port br-int gtpu1 -- set interface gtpu1 type=gtpu options:packet_type=legacy_l3 options:remote_ip=flow options:key=flow options:dst_port=2152

test-flows-gtpu.txt is openflow table for gtpu tunnel test:

$ cat test-flows-gtpu.txt
table=0,icmp,in_port=4 actions=load:0xc0a83249->NXM_NX_TUN_IPV4_DST[],load:0x9->NXM_NX_TUN_ID[0..31],output:2
table=0,tcp,tp_dst=80,in_port=4 actions=load:0xc0a83249->NXM_NX_TUN_IPV4_DST[],load:0x9->NXM_NX_TUN_ID[0..31],output:2
table=0,in_port=2,tun_id=0x10,tun_gtpu_flags=0x30,tun_gtpu_msgtype=255 actions=set_field:00:00:22:22:22:22->eth_src,set_field:00:00:11:11:11:11->eth_dst,output:4
table=0,actions=NORMAL

Signed-off-by: Feng Yang <feng.yang@intel.com>
Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
 build-aux/extract-ofp-fields                      |   1 +
 datapath/linux/compat/include/linux/openvswitch.h |   3 +
 include/openvswitch/match.h                       |   4 +
 include/openvswitch/meta-flow.h                   |  28 ++++
 include/openvswitch/packets.h                     |   4 +-
 lib/dpif-netlink-rtnl.c                           |   5 +
 lib/dpif-netlink.c                                |   3 +
 lib/flow.c                                        |   5 +
 lib/match.c                                       |  29 +++++
 lib/meta-flow.c                                   |  38 ++++++
 lib/meta-flow.xml                                 |  76 +++++++++++
 lib/netdev-native-tnl.c                           | 150 +++++++++++++++++++++-
 lib/netdev-native-tnl.h                           |  12 ++
 lib/netdev-vport.c                                |  15 ++-
 lib/nx-match.c                                    |   4 +
 lib/odp-util.c                                    |  32 +++++
 lib/packets.h                                     |  17 +++
 lib/tnl-ports.c                                   |   3 +
 ofproto/ofproto-dpif-xlate.c                      |   1 +
 ofproto/tunnel.c                                  |   6 +
 20 files changed, 433 insertions(+), 3 deletions(-)

Comments

Joe Stringer March 4, 2018, 8:19 p.m. | #1
?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
> From: Feng Yang <feng.yang@intel.com>
>
> GPRS Tunneling Protocol (GTP) is a group of IP-based communications
> protocols used to carry general packet radio service (GPRS) within
> GSM, UMTS and LTE networks.
>
> GTP can be decomposed into separate protocols, GTP-C, GTP-U and GTP'.
>
> GTP-U is used for carrying user data within the GPRS core network and
> between the radio access network and the core network. The user data
> transported can be packets in any of IPv4, IPv6, or PPP formats.
>
> GTP-U is user plane communication protocol between Serving-Gateway
> (S-GW) and PDN-Gateway (P-GW) as well as Multi-access Edge Computing
> (MEC).
>
> This patch added a new tunnel type 'gtpu' into userspace datapath and
> two new match fields tun_gtpu_flags and tun_gtpu_msgtype, tun_id is
> reused for GTP Tunnel Endpoint ID (TEID).
>
> The below are commands for gtpu tunnel add:
>
> $ sudo ovs-vsctl add-br br-int -- set bridge br-int datapath_type=netdev protocols=OpenFlow13
> $ sudo ovs-vsctl add-port br-int gtpu1 -- set interface gtpu1 type=gtpu options:packet_type=legacy_l3 options:remote_ip=flow options:key=flow options:dst_port=2152
>
> test-flows-gtpu.txt is openflow table for gtpu tunnel test:
>
> $ cat test-flows-gtpu.txt
> table=0,icmp,in_port=4 actions=load:0xc0a83249->NXM_NX_TUN_IPV4_DST[],load:0x9->NXM_NX_TUN_ID[0..31],output:2
> table=0,tcp,tp_dst=80,in_port=4 actions=load:0xc0a83249->NXM_NX_TUN_IPV4_DST[],load:0x9->NXM_NX_TUN_ID[0..31],output:2
> table=0,in_port=2,tun_id=0x10,tun_gtpu_flags=0x30,tun_gtpu_msgtype=255 actions=set_field:00:00:22:22:22:22->eth_src,set_field:00:00:11:11:11:11->eth_dst,output:4
> table=0,actions=NORMAL
>
> Signed-off-by: Feng Yang <feng.yang@intel.com>
> Signed-off-by: Yi Yang <yi.y.yang@intel.com>
> ---

Hi Yi, thanks for this work. Some minor comments below, but this is
just a "drive-by" review. I hope that others with more experience with
GTP will take a look as well.

I see the comment that there is locking on the device whenever a GTP
header is built, I assume you intend to address that in a followup
iteration.

If you haven't already, it may be interesting to sync with Jiannan
(CC'd) who has previously done some work on GTP support in OVS for
kernel, to see whether the API (OpenFlow, OVSDB) makes sense for a
cross-datapath (ie kernel AND userspace) implementation perspective.

> diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
> index 84ebcaf..ad6ea64 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -236,6 +236,7 @@ enum ovs_vport_type {
>         OVS_VPORT_TYPE_GRE,      /* GRE tunnel. */
>         OVS_VPORT_TYPE_VXLAN,    /* VXLAN tunnel. */
>         OVS_VPORT_TYPE_GENEVE,   /* Geneve tunnel. */
> +       OVS_VPORT_TYPE_GTPU,     /* GTPU tunnel. */
>         OVS_VPORT_TYPE_LISP = 105,  /* LISP tunnel */
>         OVS_VPORT_TYPE_STT = 106, /* STT tunnel */
>         __OVS_VPORT_TYPE_MAX
> @@ -394,6 +395,8 @@ enum ovs_tunnel_key_attr {
>         OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS,         /* Nested OVS_VXLAN_EXT_* */
>         OVS_TUNNEL_KEY_ATTR_IPV6_SRC,           /* struct in6_addr src IPv6 address. */
>         OVS_TUNNEL_KEY_ATTR_IPV6_DST,           /* struct in6_addr dst IPv6 address. */
> +       OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS,         /* u8 GTP-U FLAGS. */
> +       OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE,       /* u8 GTP-U MSGTYPE. */
>         OVS_TUNNEL_KEY_ATTR_PAD,
>         __OVS_TUNNEL_KEY_ATTR_MAX
>  };

The above changes have not been added to upstream linux header file,
and the kernel uapi headers are the source of truth for these values.
Please define them in such a way that they only apply to userspace so
that we can prevent any potential conflict between expectations of
meaning of these enums. I see common ways to achieve this have been to
use `#ifndef __KERNEL__` conditional and by defining them as higher
numbers that are less likely to conflict like the existing tunnel
types.

> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
> index 61a67de..fb41736 100644
> --- a/include/openvswitch/match.h
> +++ b/include/openvswitch/match.h
> @@ -109,6 +109,10 @@ void match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16
>  void match_set_tun_gbp_id(struct match *match, ovs_be16 gbp_id);
>  void match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, uint8_t mask);
>  void match_set_tun_gbp_flags(struct match *match, uint8_t flags);
> +void match_set_tun_gtpu_flags(struct match *match, uint8_t gtpu_flags);
> +void match_set_tun_gtpu_flags_masked(struct match *match, uint8_t gtpu_flags,
> +                                     uint8_t mask);
> +void match_set_tun_gtpu_msgtype(struct match *match, uint8_t gtpu_msgtype);
>  void match_set_in_port(struct match *, ofp_port_t ofp_port);
>  void match_set_pkt_mark(struct match *, uint32_t pkt_mark);
>  void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t mask);
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index 98c9e1c..884a2f6 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -1848,6 +1848,34 @@ enum OVS_PACKED_ENUM mf_field_id {
>       */
>      MFF_NSH_TTL,
>
> +    /* "tun_gtpu_flags".
> +     *
> +     * The flags in a GTP-U tunnel header.
> +     *
> +     * Type: u8.
> +     * Maskable: bitwise.
> +     * Formatting: decimal.
> +     * Prerequisites: none.
> +     * Access: read/write.
> +     * NXM: none.
> +     * OXM: NXOXM_GTPU_FLAGS(1) since OF1.3 and v2.10.
> +     */
> +    MFF_TUN_GTPU_FLAGS,
> +
> +    /* "tun_gtpu_msgtype".
> +     *
> +     * The message type in a GTP-U tunnel header.
> +     *
> +     * Type: u8.
> +     * Maskable: no.
> +     * Formatting: decimal.
> +     * Prerequisites: none.
> +     * Access: read/write.
> +     * NXM: none.
> +     * OXM: NXOXM_GTPU_MSGTYPE(2) since OF1.3 and v2.10.
> +     */
> +    MFF_TUN_GTPU_MSGTYPE,
> +
>      MFF_N_IDS
>  };
>
> diff --git a/include/openvswitch/packets.h b/include/openvswitch/packets.h
> index fef756b..1f3b499 100644
> --- a/include/openvswitch/packets.h
> +++ b/include/openvswitch/packets.h
> @@ -39,7 +39,9 @@ struct flow_tnl {
>      ovs_be16 tp_dst;
>      ovs_be16 gbp_id;
>      uint8_t  gbp_flags;
> -    uint8_t  pad1[5];        /* Pad to 64 bits. */
> +    uint8_t  gtpu_flags;
> +    uint8_t  gtpu_msgtype;
> +    uint8_t  pad1[3];        /* Pad to 64 bits. */
>      struct tun_metadata metadata;
>  };
>
> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> index 40c4569..a628b72 100644
> --- a/lib/dpif-netlink-rtnl.c
> +++ b/lib/dpif-netlink-rtnl.c
> @@ -99,6 +99,8 @@ vport_type_to_kind(enum ovs_vport_type type,
>          }
>      case OVS_VPORT_TYPE_GENEVE:
>          return "geneve";
> +    case OVS_VPORT_TYPE_GTPU:
> +        return "gtpu";
>      case OVS_VPORT_TYPE_NETDEV:
>      case OVS_VPORT_TYPE_INTERNAL:
>      case OVS_VPORT_TYPE_LISP:
> @@ -262,6 +264,7 @@ dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg,
>      case OVS_VPORT_TYPE_INTERNAL:
>      case OVS_VPORT_TYPE_LISP:
>      case OVS_VPORT_TYPE_STT:
> +    case OVS_VPORT_TYPE_GTPU:
>      case OVS_VPORT_TYPE_UNSPEC:
>      case __OVS_VPORT_TYPE_MAX:
>      default:
> @@ -327,6 +330,7 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg,
>      case OVS_VPORT_TYPE_INTERNAL:
>      case OVS_VPORT_TYPE_LISP:
>      case OVS_VPORT_TYPE_STT:
> +    case OVS_VPORT_TYPE_GTPU:
>      case OVS_VPORT_TYPE_UNSPEC:
>      case __OVS_VPORT_TYPE_MAX:
>      default:
> @@ -438,6 +442,7 @@ dpif_netlink_rtnl_port_destroy(const char *name, const char *type)
>      case OVS_VPORT_TYPE_INTERNAL:
>      case OVS_VPORT_TYPE_LISP:
>      case OVS_VPORT_TYPE_STT:
> +    case OVS_VPORT_TYPE_GTPU:
>      case OVS_VPORT_TYPE_UNSPEC:
>      case __OVS_VPORT_TYPE_MAX:
>      default:
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 8543a2b..19ed182 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -786,6 +786,9 @@ get_vport_type(const struct dpif_netlink_vport *vport)
>      case OVS_VPORT_TYPE_STT:
>          return "stt";
>
> +    case OVS_VPORT_TYPE_GTPU:
> +        return "gtpu";
> +
>      case OVS_VPORT_TYPE_UNSPEC:
>      case __OVS_VPORT_TYPE_MAX:
>          break;
> diff --git a/lib/flow.c b/lib/flow.c
> index 38ff29c..1648667 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -695,6 +695,11 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
>          miniflow_push_be32(mf, packet_type, packet_type);
>      }
>
> +    /* It won't be parsed if the packet is GTP-U message but not GTP-U PDU */
> +    if (packet_type == htonl(PT_UNKNOWN)) {
> +        goto out;
> +    }
> +
>      /* Initialize packet's layer pointer and offsets. */
>      frame = data;
>      dp_packet_reset_offsets(packet);
> diff --git a/lib/match.c b/lib/match.c
> index 97a5282..5c845ed 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -320,6 +320,27 @@ match_set_tun_gbp_flags(struct match *match, uint8_t flags)
>  }
>
>  void
> +match_set_tun_gtpu_flags(struct match *match, uint8_t gtpu_flags)
> +{
> +    match_set_tun_gtpu_flags_masked(match, gtpu_flags, UINT8_MAX);
> +}
> +
> +void
> +match_set_tun_gtpu_flags_masked(struct match *match, uint8_t gtpu_flags,
> +                                uint8_t mask)
> +{
> +    match->wc.masks.tunnel.gtpu_flags = mask;
> +    match->flow.tunnel.gtpu_flags = gtpu_flags & mask;
> +}
> +
> +void
> +match_set_tun_gtpu_msgtype(struct match *match, uint8_t gtpu_msgtype)
> +{
> +    match->flow.tunnel.gtpu_msgtype = gtpu_msgtype;
> +    match->wc.masks.tunnel.gtpu_msgtype = UINT8_MAX;
> +}
> +
> +void
>  match_set_in_port(struct match *match, ofp_port_t ofp_port)
>  {
>      match->wc.masks.in_port.ofp_port = u16_to_ofp(UINT16_MAX);
> @@ -1239,6 +1260,14 @@ format_flow_tunnel(struct ds *s, const struct match *match)
>                              FLOW_TNL_F_MASK);
>          ds_put_char(s, ',');
>      }
> +
> +    if (wc->masks.tunnel.gtpu_flags) {
> +        ds_put_format(s, "tun_gtpu_flags=%"PRIx8",", tnl->gtpu_flags);
> +    }
> +    if (wc->masks.tunnel.gtpu_msgtype) {
> +        ds_put_format(s, "tun_gtpu_msgtype=%"PRIu8",", tnl->gtpu_msgtype);
> +    }
> +
>      tun_metadata_match_format(s, match);
>  }
>
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index aa2ec01..2ea5f63 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -231,6 +231,10 @@ mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
>          return !wc->masks.tunnel.gbp_id;
>      case MFF_TUN_GBP_FLAGS:
>          return !wc->masks.tunnel.gbp_flags;
> +    case MFF_TUN_GTPU_FLAGS:
> +        return !wc->masks.tunnel.gtpu_flags;
> +    case MFF_TUN_GTPU_MSGTYPE:
> +        return !wc->masks.tunnel.gtpu_msgtype;
>      CASE_MFF_TUN_METADATA:
>          return !ULLONG_GET(wc->masks.tunnel.metadata.present.map,
>                             mf->id - MFF_TUN_METADATA0);
> @@ -513,6 +517,8 @@ mf_is_value_valid(const struct mf_field *mf, const union mf_value *value)
>      case MFF_TUN_TTL:
>      case MFF_TUN_GBP_ID:
>      case MFF_TUN_GBP_FLAGS:
> +    case MFF_TUN_GTPU_FLAGS:
> +    case MFF_TUN_GTPU_MSGTYPE:
>      CASE_MFF_TUN_METADATA:
>      case MFF_METADATA:
>      case MFF_IN_PORT:
> @@ -674,6 +680,12 @@ mf_get_value(const struct mf_field *mf, const struct flow *flow,
>      case MFF_TUN_GBP_FLAGS:
>          value->u8 = flow->tunnel.gbp_flags;
>          break;
> +    case MFF_TUN_GTPU_FLAGS:
> +        value->u8 = flow->tunnel.gtpu_flags;
> +        break;
> +    case MFF_TUN_GTPU_MSGTYPE:
> +        value->u8 = flow->tunnel.gtpu_msgtype;
> +        break;
>      case MFF_TUN_TTL:
>          value->u8 = flow->tunnel.ip_ttl;
>          break;
> @@ -988,6 +1000,12 @@ mf_set_value(const struct mf_field *mf,
>      case MFF_TUN_GBP_FLAGS:
>           match_set_tun_gbp_flags(match, value->u8);
>           break;
> +    case MFF_TUN_GTPU_FLAGS:
> +        match_set_tun_gtpu_flags(match, value->u8);
> +        break;
> +    case MFF_TUN_GTPU_MSGTYPE:
> +        match_set_tun_gtpu_msgtype(match, value->u8);
> +        break;
>      case MFF_TUN_TOS:
>          match_set_tun_tos(match, value->u8);
>          break;
> @@ -1385,6 +1403,12 @@ mf_set_flow_value(const struct mf_field *mf,
>      case MFF_TUN_GBP_FLAGS:
>          flow->tunnel.gbp_flags = value->u8;
>          break;
> +    case MFF_TUN_GTPU_FLAGS:
> +        flow->tunnel.gtpu_flags = value->u8;
> +        break;
> +    case MFF_TUN_GTPU_MSGTYPE:
> +        flow->tunnel.gtpu_msgtype = value->u8;
> +        break;
>      case MFF_TUN_TOS:
>          flow->tunnel.ip_tos = value->u8;
>          break;
> @@ -1700,6 +1724,8 @@ mf_is_pipeline_field(const struct mf_field *mf)
>      case MFF_TUN_FLAGS:
>      case MFF_TUN_GBP_ID:
>      case MFF_TUN_GBP_FLAGS:
> +    case MFF_TUN_GTPU_FLAGS:
> +    case MFF_TUN_GTPU_MSGTYPE:
>      CASE_MFF_TUN_METADATA:
>      case MFF_METADATA:
>      case MFF_IN_PORT:
> @@ -1870,6 +1896,14 @@ mf_set_wild(const struct mf_field *mf, struct match *match, char **err_str)
>      case MFF_TUN_GBP_FLAGS:
>          match_set_tun_gbp_flags_masked(match, 0, 0);
>          break;
> +    case MFF_TUN_GTPU_FLAGS:
> +        match->wc.masks.tunnel.gtpu_flags = 0;
> +        match->flow.tunnel.gtpu_flags = 0;
> +        break;
> +    case MFF_TUN_GTPU_MSGTYPE:
> +        match->wc.masks.tunnel.gtpu_msgtype = 0;
> +        match->flow.tunnel.gtpu_msgtype = 0;
> +        break;
>      case MFF_TUN_TOS:
>          match_set_tun_tos_masked(match, 0, 0);
>          break;
> @@ -2221,6 +2255,7 @@ mf_set(const struct mf_field *mf,
>      case MFF_ICMPV4_CODE:
>      case MFF_ICMPV6_TYPE:
>      case MFF_ICMPV6_CODE:
> +    case MFF_TUN_GTPU_MSGTYPE:
>          return OFPUTIL_P_NONE;
>
>      case MFF_DP_HASH:
> @@ -2250,6 +2285,9 @@ mf_set(const struct mf_field *mf,
>      case MFF_TUN_GBP_FLAGS:
>          match_set_tun_gbp_flags_masked(match, value->u8, mask->u8);
>          break;
> +    case MFF_TUN_GTPU_FLAGS:
> +        match_set_tun_gtpu_flags_masked(match, value->u8, mask->u8);
> +        break;
>      case MFF_TUN_TTL:
>          match_set_tun_ttl_masked(match, value->u8, mask->u8);
>          break;
> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
> index 933d4b8..d169772 100644
> --- a/lib/meta-flow.xml
> +++ b/lib/meta-flow.xml
> @@ -731,6 +731,12 @@ tcp,tp_src=0x07c0/0xfff0
>        Used by Open vSwitch for NSH extensions, in the absence of an official
>        ONF-assigned class.  (This OUI is randomly generated.)
>      </dd>
> +
> +    <dt>0x005ad651 (<code>NXOXM_GTPU</code>)</dt>
> +    <dd>
> +      Used by Open vSwitch for GTP-U extensions, in the absence of an official
> +      ONF-assigned class.  (This OUI is randomly generated.)
> +    </dd>
>    </dl>
>
>    <p>
> @@ -1899,6 +1905,68 @@ ovs-ofctl add-flow br0 tun_metadata0=1234,actions=controller
>        </p>
>      </field>
>
> +    <h2>GTP-U Fields</h2>
> +
> +    <p>
> +      The GTP-U (v1) header is defined as follows [3GPP TS 29.281].
> +    </p>
> +
> +    <diagram>
> +      <header name="GTP-U tunnel flags">
> +        <bits name="V" above="3" width="0.15"/>
> +        <bits name="PT" above="1" width="0.15"/>
> +        <bits name="(*)" above="1" width="0.15"/>
> +        <bits name="E" above="1" width="0.15"/>
> +        <bits name="S" above="1" width="0.15"/>
> +        <bits name="PN" above="1" width="0.15"/>
> +      </header>
> +      <nospace/>
> +      <header>
> +         <bits name="Message Type" above="8" width="0.15"/>
> +         <bits name="Length" above="16" width="0.15"/>
> +         <bits name="TEID" above="32" width="0.15"/>
> +         <bits name="..."  above="" width="0.15"/>
> +      </header>
> +    </diagram>
> +
> +    <p>
> +       where
> +    </p>
> +    <ul>
> +      <li><code>V</code>: version fields, used to determine the version of the
> +      GTP-U protocol, which should be set to '1'.</li>
> +      <li><code>PT</code>: Packet Type, used as a protocol discriminator
> +      between GTP (when PT is '1') and GTP' (when PT is '0').</li>
> +      <li><code>E</code>: Extension Header flag, indicating the presence of a
> +      meaningful value of the Next Extension Header field.</li>
> +      <li><code>S</code>: Sequence number flag, indicating the presence of a
> +      meaningful value of the Sequence Number field.</li>
> +      <li><code>PN</code>: N-PDU Number flag, indicating the presence of a
> +      meaningful value of the N-PDU Number field.</li>
> +      <li><code>Message Type</code>: indicating the type of GTP-U message.
> +      </li>
> +      <li><code>Length</code>: indicating the length in octets of the payload.
> +      </li>
> +      <li><code>TEID</code>: Tunnel Endpoint Identifier,unambiguously
> +      identifying a tunnel endpoint in the receiving GTP-U protocol entity.
> +      Open vSwitch makes TEID availabe via <ref field="tun_id"/>.</li>
> +    </ul>
> +    <field id="MFF_TUN_GTPU_FLAGS" title="GTP-U tunnel flags">
> +      <p>
> +        For a packet tunneled over GTP-U, this field indicates the presence of
> +        optional headers.
> +      </p>
> +    </field>
> +
> +    <field id="MFF_TUN_GTPU_MSGTYPE" title="GTP-U tunnel message type">
> +      <p>
> +        For a packet tunneled over GTP-U, this field indicates whether it's a
> +        signalling message used for path management, or a user plane message
> +        which carries the original packet. The complete range of message types
> +        can be referred to [3GPP TS 29.281].
> +      </p>
> +    </field>
> +
>      <!-- Open vSwitch uses the following fields internally, but it
>           does not expose them to the user via OpenFlow, so we do not
>           document them. -->
> @@ -4662,6 +4730,14 @@ r r c c c.
>        <url href="https://tools.ietf.org/html/rfc7665"/>.
>      </dd>
>
> +    <dt>3GPP TS 29.281</dt>
> +    <dd>
> +      3GPP,
> +      ``General Packet Radio System (GPRS) Tunnelling Protocol User Plane
> +      (GTPv1-U),''
> +      <url href="http://www.3gpp.org/ftp/Specs/html-info/29281.htm"/>.
> +    </dd>
> +
>      <dt>Srinivasan</dt>
>      <dd>
>        V. Srinivasan, S. Suriy, and G. Varghese, ``Packet
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index fb5eab0..bcf599d 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -55,6 +55,9 @@ static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(60, 5);
>  #define GENEVE_BASE_HLEN   (sizeof(struct udp_header) +         \
>                              sizeof(struct genevehdr))
>
> +#define GTPU_HLEN   (sizeof(struct udp_header) +        \
> +                     sizeof(struct gtpuhdr))
> +
>  uint16_t tnl_udp_port_min = 32768;
>  uint16_t tnl_udp_port_max = 61000;
>
> @@ -715,7 +718,152 @@ netdev_geneve_build_header(const struct netdev *netdev,
>      return 0;
>  }
>
> -
> +void
> +netdev_gtpu_push_header(struct dp_packet *packet,
> +                        const struct ovs_action_push_tnl *data)
> +{
> +    struct udp_header *udp;
> +    struct gtpuhdr *gtph;
> +    int gtpu_len;
> +    int ip_tot_size;
> +
> +    gtpu_len = dp_packet_size(packet);
> +
> +    udp = netdev_tnl_push_ip_header(packet, data->header, data->header_len,
> +                                    &ip_tot_size);
> +    udp->udp_len = htons(ip_tot_size);
> +
> +    gtph = (struct gtpuhdr *)(udp + 1);
> +    gtph->len = htons(gtpu_len);
> +
> +    if (udp->udp_csum) {
> +        uint32_t csum;
> +        if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> +            csum = packet_csum_pseudoheader6(
> +                       netdev_tnl_ipv6_hdr(dp_packet_data(packet)));
> +        } else {
> +            csum = packet_csum_pseudoheader(
> +                       netdev_tnl_ip_hdr(dp_packet_data(packet)));
> +        }
> +
> +        csum = csum_continue(csum, udp, ip_tot_size);
> +        udp->udp_csum = csum_finish(csum);
> +
> +        if (!udp->udp_csum) {
> +            udp->udp_csum = htons(0xffff);

What's the motivation behind this?

> +        }
> +    }
> +
> +    packet->packet_type = htonl(PT_ETH);
> +}
> +
> +
> +int
> +netdev_gtpu_build_header(const struct netdev *netdev,
> +                          struct ovs_action_push_tnl *data,
> +                          const struct netdev_tnl_build_header_params *params)
> +{
> +    struct netdev_vport *dev = netdev_vport_cast(netdev);
> +    struct netdev_tunnel_config *tnl_cfg;
> +    struct gtpuhdr *gtph;
> +    struct udp_header *udp;
> +
> +    /* XXX: RCUfy tnl_cfg. */
> +    ovs_mutex_lock(&dev->mutex);
> +
> +    tnl_cfg = &dev->tnl_cfg;
> +    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP);
> +    udp->udp_dst = tnl_cfg->dst_port;
> +    udp->udp_src = htons(GTPU_DST_PORT);
> +
> +    if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
> +        /* Write a value in now to mark that we should compute the checksum
> +         * later. 0xffff is handy because it is transparent to the
> +         * calculation. */
> +        udp->udp_csum = htons(0xffff);

Maybe I missed it, but I didn't see where in this patch that it is
computed later due to this value.

> +    }
> +
> +    ovs_mutex_unlock(&dev->mutex);
> +    data->header_len += sizeof *udp;
> +
> +    gtph = (struct gtpuhdr *)(udp + 1);
> +    gtph->flags = GTPU_FLAGS_DEFAULT;
> +    gtph->msgtype = GTPU_MSGTYPE_GPDU;
> +    if (params->flow->tunnel.gtpu_flags != 0) {
> +        gtph->flags = params->flow->tunnel.gtpu_flags;
> +    }
> +    if (params->flow->tunnel.gtpu_msgtype != 0) {
> +        gtph->msgtype = params->flow->tunnel.gtpu_msgtype;
> +    }
> +    put_16aligned_be32(&gtph->teid,
> +                       htonl(ntohll(params->flow->tunnel.tun_id)));
> +    data->header_len += sizeof *gtph;
> +    data->tnl_type = OVS_VPORT_TYPE_GTPU;
> +    return 0;
> +}
> +
> +struct dp_packet *
> +netdev_gtpu_pop_header(struct dp_packet *packet)
> +{
> +    struct pkt_metadata *md = &packet->md;
> +    struct flow_tnl *tnl = &md->tunnel;
> +    struct gtpuhdr *gtph;
> +    unsigned int hlen;
> +
> +    pkt_metadata_init_tnl(md);
> +    if (GTPU_HLEN > dp_packet_l4_size(packet)) {
> +        VLOG_WARN_RL(&err_rl, "GTP-U packet too small: min header=%u "
> +                              "packet size=%"PRIuSIZE"\n",
> +                     (unsigned int)GTPU_HLEN, dp_packet_l4_size(packet));
> +        goto err;
> +    }
> +
> +    gtph = udp_extract_tnl_md(packet, tnl, &hlen);
> +    if (!gtph) {
> +        goto err;
> +    }
> +
> +    if (gtph->flags == 0x30) {

Should this use GTPU_FLAGS_DEFAULT?

> +    /* Only GTP-U v1 packets without optional fileds are processed, i.e.
> +     *     8   7   6  5    4   3   2   1
> +     *    |   ver   | PT | 0 | E | S | PN |
> +     *     0   0   1  1    0   0   0   0
> +     */
> +        tnl->gtpu_flags = gtph->flags;
> +    } else {
> +        goto err;

You might consider restricting matches on GTP headers to ensure that
flows MUST match on these supported flags and the value must be equal
before installing such a flow, so that controller writers will know
the first time they attempt to install the flow that OVS doesn't
support matching/handling other flags. This would be better warning
than deploying flows that seem to work, then finding out down the line
that if an extra bit is set then the packet will be silently dropped.

Perhaps also could put a ratelimited warn or some kind of drop counter
here, I don't know the protocol semantics of optional fields but it
seems a little surprising to me to just drop packets on the ground if
they're setting bits that this code doesn't know how to process.

Either way, this kind of behaviour should be documented too if it's not already.

> +    }
> +    tnl->gtpu_msgtype = gtph->msgtype;
> +    tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gtph->teid)));
> +
> +    /*Figure out whether the inner packet is IPv4, IPv6 or a GTP-U message.*/
> +    if (tnl->gtpu_msgtype == GTPU_MSGTYPE_GPDU) {
> +        struct ip_header *ip;
> +
> +        ip = (struct ip_header *)(gtph + 1);
> +        if (IP_VER(ip->ip_ihl_ver) == 4) {
> +            packet->packet_type = htonl(PT_IPV4);
> +        } else if (IP_VER(ip->ip_ihl_ver) == 6) {
> +            packet->packet_type = htonl(PT_IPV6);
> +        } else {
> +            goto err;
> +        }
> +    } else {
> +        /* tnl->gtpu_msgtype can identify if it is GTP-U message,
> +         * miniflow_extract won't parse it if tnl->gtpu_msgtype
> +         * isn't equal to GTPU_MSGTYPE_GPDU.
> +         */
> +        packet->packet_type = htonl(PT_UNKNOWN);

This is where the limits of my GTP understanding will show, I was
under the impression that GTP-U provides encapsulation for user
traffic, so would commonly encapsulate IPv4/IPv6 inside. Whereas for
GTP-C, it's signalling traffic so would not have such inner headers -
it's supposed to be handled by a GTP implementation. Why is it
important to be able to decapsulate GTP-C? Furthermore, what's
ensuring that we don't end up spitting inner GTP-C packet data onto
the wire without headers post-translation?

> +    }
> +
> +    dp_packet_reset_packet(packet, hlen + GTPU_HLEN);
> +
> +    return packet;
> +err:
> +    dp_packet_delete(packet);
> +    return NULL;
> +}
> +
>  void
>  netdev_tnl_egress_port_range(struct unixctl_conn *conn, int argc,
>                               const char *argv[], void *aux OVS_UNUSED)
> diff --git a/lib/netdev-native-tnl.h b/lib/netdev-native-tnl.h
> index a912ce9..9c4cc66 100644
> --- a/lib/netdev-native-tnl.h
> +++ b/lib/netdev-native-tnl.h
> @@ -58,6 +58,18 @@ netdev_vxlan_build_header(const struct netdev *netdev,
>  struct dp_packet *
>  netdev_vxlan_pop_header(struct dp_packet *packet);
>
> +void
> +netdev_gtpu_push_header(struct dp_packet *packet,
> +                        const struct ovs_action_push_tnl *data);
> +
> +int
> +netdev_gtpu_build_header(const struct netdev *netdev,
> +                         struct ovs_action_push_tnl *data,
> +                         const struct netdev_tnl_build_header_params *params);
> +
> +struct dp_packet *
> +netdev_gtpu_pop_header(struct dp_packet *packet);
> +
>  static inline bool
>  netdev_tnl_is_header_ipv6(const void *header)
>  {
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 52aa12d..a10b2f1 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -107,7 +107,10 @@ 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("lisp", type) || !strcmp("stt", type) ||
> +             !strcmp("gtpu", type)
> +            )
> +           );
>  }
>
>  const char *
> @@ -408,6 +411,8 @@ tunnel_supported_layers(const char *type,
>          return TNL_L3;
>      } else if (!strcmp(type, "gre")) {
>          return TNL_L2 | TNL_L3;
> +    } else if (!strcmp(type, "gtpu")) {
> +        return TNL_L3;
>      } else if (!strcmp(type, "vxlan")
>                 && tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GPE)) {
>          return TNL_L2 | TNL_L3;
> @@ -455,6 +460,10 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp)
>          tnl_cfg.dst_port = htons(STT_DST_PORT);
>      }
>
> +    if (!strcmp(type, "gtpu")) {
> +        tnl_cfg.dst_port = htons(GTPU_DST_PORT);
> +    }
> +
>      needs_dst_port = netdev_vport_needs_dst_port(dev_);
>      tnl_cfg.dont_fragment = true;
>
> @@ -979,6 +988,10 @@ netdev_vport_tunnel_register(void)
>                                             NETDEV_VPORT_GET_IFINDEX),
>          TUNNEL_CLASS("lisp", "lisp_sys", NULL, NULL, NULL, NULL),
>          TUNNEL_CLASS("stt", "stt_sys", NULL, NULL, NULL, NULL),
> +        TUNNEL_CLASS("gtpu", "gptu_sys", netdev_gtpu_build_header,
> +                                         netdev_gtpu_push_header,
> +                                         netdev_gtpu_pop_header,
> +                                         NULL),
>      };
>      static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index a8edb2e..c7f6c27 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -1153,6 +1153,10 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm, const struct match *match,
>                  flow->tunnel.gbp_id, match->wc.masks.tunnel.gbp_id);
>      nxm_put_8m(&ctx, MFF_TUN_GBP_FLAGS, oxm,
>                 flow->tunnel.gbp_flags, match->wc.masks.tunnel.gbp_flags);
> +    nxm_put_8m(&ctx, MFF_TUN_GTPU_FLAGS, oxm, flow->tunnel.gtpu_flags,
> +               match->wc.masks.tunnel.gtpu_flags);
> +    nxm_put_8m(&ctx, MFF_TUN_GTPU_MSGTYPE, oxm, flow->tunnel.gtpu_msgtype,
> +               match->wc.masks.tunnel.gtpu_msgtype);
>      tun_metadata_to_nx_match(b, oxm, match);
>
>      /* Network Service Header */
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 5da83b4..f200d90 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -712,6 +712,15 @@ format_odp_tnl_push_header(struct ds *ds, struct ovs_action_push_tnl *data)
>              options++;
>          }
>          ds_put_format(ds, ")");
> +    } else if (data->tnl_type == OVS_VPORT_TYPE_GTPU) {
> +        const struct gtpuhdr *gtph;
> +
> +        gtph = format_udp_tnl_push_header(ds, udp);
> +
> +        ds_put_format(ds, "gtpu(flags=0x%"PRIx32
> +                          ",msgtype=0x%"PRIx32",teid=0x%"PRIx32")",
> +                      gtph->flags, gtph->msgtype,
> +                      ntohl(get_16aligned_be32(&gtph->teid)));
>      }
>      ds_put_format(ds, ")");
>  }
> @@ -2373,6 +2382,8 @@ static const struct attr_len_tbl ovs_tun_key_attr_lens[OVS_TUNNEL_KEY_ATTR_MAX +
>      [OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS]    = { .len = ATTR_LEN_NESTED,
>                                              .next = ovs_vxlan_ext_attr_lens ,
>                                              .next_max = OVS_VXLAN_EXT_MAX},
> +    [OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS]    = { .len = 1 },
> +    [OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE]  = { .len = 1 },
>      [OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = 16 },
>      [OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = 16 },
>  };
> @@ -2698,6 +2709,12 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
>          case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS:
>              tun_metadata_from_geneve_nlattr(a, is_mask, tun);
>              break;
> +        case OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS:
> +            tun->gtpu_flags = nl_attr_get_u8(a);
> +            break;
> +        case OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE:
> +            tun->gtpu_msgtype = nl_attr_get_u8(a);
> +            break;
>
>          default:
>              /* Allow this to show up as unexpected, if there are unknown
> @@ -2775,6 +2792,13 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key,
>                         (tun_key->gbp_flags << 16) | ntohs(tun_key->gbp_id));
>          nl_msg_end_nested(a, vxlan_opts_ofs);
>      }
> +    if (tun_key->gtpu_flags) {
> +        nl_msg_put_u8(a, OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS, tun_key->gtpu_flags);
> +    }
> +    if (tun_key->gtpu_msgtype) {
> +        nl_msg_put_u8(a, OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE,
> +                      tun_key->gtpu_msgtype);
> +    }
>      tun_metadata_to_geneve_nlattr(tun_key, tun_flow_key, key_buf, a);
>
>      nl_msg_end_nested(a, tun_key_ofs);
> @@ -3477,6 +3501,14 @@ format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr,
>              format_odp_tun_geneve(a, ma, ds, verbose);
>              ds_put_cstr(ds, "),");
>              break;
> +        case OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS:
> +            format_u8x(ds, "gtpu_flags", nl_attr_get_u8(a),
> +                       ma ? nl_attr_get(ma) : NULL, verbose);
> +            break;
> +        case OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE:
> +            format_u8x(ds, "gtpu_msgtype", nl_attr_get_u8(a),
> +                       ma ? nl_attr_get(ma) : NULL, verbose);
> +            break;
>          case OVS_TUNNEL_KEY_ATTR_PAD:
>              break;
>          case __OVS_TUNNEL_KEY_ATTR_MAX:
> diff --git a/lib/packets.h b/lib/packets.h
> index 9a71aa3..709e9cb 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -1306,6 +1306,23 @@ BUILD_ASSERT_DECL(sizeof(struct vxlanhdr) == 8);
>  #define VXLAN_F_GPE  0x4000
>  #define VXLAN_HF_GPE 0x04000000
>
> +/* GTP-U protocol header */
> +struct gtpuhdr {
> +    uint8_t flags;
> +    uint8_t msgtype;
> +    ovs_be16 len;
> +    ovs_16aligned_be32 teid;
> +};
> +
> +/* GTP-U UDP port */
> +#define GTPU_DST_PORT 2152
> +
> +/* Default GTP-U flags */
> +#define GTPU_FLAGS_DEFAULT 0x30
> +
> +/* GTP-U message type for normal user plane PDU */
> +#define GTPU_MSGTYPE_GPDU 255
> +
>  /* Input values for PACKET_TYPE macros have to be in host byte order.
>   * The _BE postfix indicates result is in network byte order. Otherwise result
>   * is in host byte order. */
> diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
> index b814f7a..cb8fab8 100644
> --- a/lib/tnl-ports.c
> +++ b/lib/tnl-ports.c
> @@ -177,6 +177,9 @@ tnl_type_to_nw_proto(const char type[])
>      if (!strcmp(type, "vxlan")) {
>          return IPPROTO_UDP;
>      }
> +    if (!strcmp(type, "gtpu")) {
> +        return IPPROTO_UDP;
> +    }
>      return 0;
>  }
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index cc450a8..9438d8b 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3270,6 +3270,7 @@ propagate_tunnel_data_to_flow(struct xlate_ctx *ctx, struct eth_addr dmac,
>          break;
>      case OVS_VPORT_TYPE_VXLAN:
>      case OVS_VPORT_TYPE_GENEVE:
> +    case OVS_VPORT_TYPE_GTPU:
>          nw_proto = IPPROTO_UDP;
>          break;
>      case OVS_VPORT_TYPE_LISP:
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index e0214ce..becb752 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -384,6 +384,12 @@ tnl_wc_init(struct flow *flow, struct flow_wildcards *wc)
>          wc->masks.tunnel.tp_src = 0;
>          wc->masks.tunnel.tp_dst = 0;
>
> +        /* GTP-U header are set to be always unwildcarded for GTP-U packets*/
> +        if (flow->tunnel.gtpu_flags || flow->tunnel.gtpu_msgtype) {
> +            wc->masks.tunnel.gtpu_flags = UINT8_MAX;
> +            wc->masks.tunnel.gtpu_msgtype = UINT8_MAX;
> +        }
> +
>          if (is_ip_any(flow)
>              && IP_ECN_is_ce(flow->tunnel.ip_tos)) {
>              wc->masks.nw_tos |= IP_ECN_MASK;
> --
> 2.1.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Joe Stringer March 4, 2018, 8:27 p.m. | #2
On 4 March 2018 at 12:19, Joe Stringer <joe@ovn.org> wrote:
> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
>> From: Feng Yang <feng.yang@intel.com>
>>
>> GPRS Tunneling Protocol (GTP) is a group of IP-based communications
>> protocols used to carry general packet radio service (GPRS) within
>> GSM, UMTS and LTE networks.
>>
>> GTP can be decomposed into separate protocols, GTP-C, GTP-U and GTP'.
>>
>> GTP-U is used for carrying user data within the GPRS core network and
>> between the radio access network and the core network. The user data
>> transported can be packets in any of IPv4, IPv6, or PPP formats.
>>
>> GTP-U is user plane communication protocol between Serving-Gateway
>> (S-GW) and PDN-Gateway (P-GW) as well as Multi-access Edge Computing
>> (MEC).
>>
>> This patch added a new tunnel type 'gtpu' into userspace datapath and
>> two new match fields tun_gtpu_flags and tun_gtpu_msgtype, tun_id is
>> reused for GTP Tunnel Endpoint ID (TEID).
>>
>> The below are commands for gtpu tunnel add:
>>
>> $ sudo ovs-vsctl add-br br-int -- set bridge br-int datapath_type=netdev protocols=OpenFlow13
>> $ sudo ovs-vsctl add-port br-int gtpu1 -- set interface gtpu1 type=gtpu options:packet_type=legacy_l3 options:remote_ip=flow options:key=flow options:dst_port=2152
>>
>> test-flows-gtpu.txt is openflow table for gtpu tunnel test:
>>
>> $ cat test-flows-gtpu.txt
>> table=0,icmp,in_port=4 actions=load:0xc0a83249->NXM_NX_TUN_IPV4_DST[],load:0x9->NXM_NX_TUN_ID[0..31],output:2
>> table=0,tcp,tp_dst=80,in_port=4 actions=load:0xc0a83249->NXM_NX_TUN_IPV4_DST[],load:0x9->NXM_NX_TUN_ID[0..31],output:2
>> table=0,in_port=2,tun_id=0x10,tun_gtpu_flags=0x30,tun_gtpu_msgtype=255 actions=set_field:00:00:22:22:22:22->eth_src,set_field:00:00:11:11:11:11->eth_dst,output:4
>> table=0,actions=NORMAL
>>
>> Signed-off-by: Feng Yang <feng.yang@intel.com>
>> Signed-off-by: Yi Yang <yi.y.yang@intel.com>
>> ---
>
> Hi Yi, thanks for this work. Some minor comments below, but this is
> just a "drive-by" review. I hope that others with more experience with
> GTP will take a look as well.

Realised Feng was the author, thanks to you too!
Yang, Yi March 5, 2018, 2:48 a.m. | #3
On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
> >
> > Signed-off-by: Feng Yang <feng.yang@intel.com>
> > Signed-off-by: Yi Yang <yi.y.yang@intel.com>
> > ---
> 
> Hi Yi, thanks for this work. Some minor comments below, but this is
> just a "drive-by" review. I hope that others with more experience with
> GTP will take a look as well.
> 
> I see the comment that there is locking on the device whenever a GTP
> header is built, I assume you intend to address that in a followup
> iteration.

Joe, thank for your comments, can you elaborate "there is locking on the
device whenever a GTP  header is built" with more details? I don't know
this, or there is any existing document for this? We'll fix it in next
iteration.

> 
> If you haven't already, it may be interesting to sync with Jiannan
> (CC'd) who has previously done some work on GTP support in OVS for
> kernel, to see whether the API (OpenFlow, OVSDB) makes sense for a
> cross-datapath (ie kernel AND userspace) implementation perspective.

Yes, sync is necessary, we contacted Jiannan before, I'll ping him to
check the status of his kernel patch, we'll make sure they can work well
together.

> 
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
> > index 84ebcaf..ad6ea64 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -236,6 +236,7 @@ enum ovs_vport_type {
> >         OVS_VPORT_TYPE_GRE,      /* GRE tunnel. */
> >         OVS_VPORT_TYPE_VXLAN,    /* VXLAN tunnel. */
> >         OVS_VPORT_TYPE_GENEVE,   /* Geneve tunnel. */
> > +       OVS_VPORT_TYPE_GTPU,     /* GTPU tunnel. */
> >         OVS_VPORT_TYPE_LISP = 105,  /* LISP tunnel */
> >         OVS_VPORT_TYPE_STT = 106, /* STT tunnel */
> >         __OVS_VPORT_TYPE_MAX
> > @@ -394,6 +395,8 @@ enum ovs_tunnel_key_attr {
> >         OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS,         /* Nested OVS_VXLAN_EXT_* */
> >         OVS_TUNNEL_KEY_ATTR_IPV6_SRC,           /* struct in6_addr src IPv6 address. */
> >         OVS_TUNNEL_KEY_ATTR_IPV6_DST,           /* struct in6_addr dst IPv6 address. */
> > +       OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS,         /* u8 GTP-U FLAGS. */
> > +       OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE,       /* u8 GTP-U MSGTYPE. */
> >         OVS_TUNNEL_KEY_ATTR_PAD,
> >         __OVS_TUNNEL_KEY_ATTR_MAX
> >  };
> 
> The above changes have not been added to upstream linux header file,
> and the kernel uapi headers are the source of truth for these values.
> Please define them in such a way that they only apply to userspace so
> that we can prevent any potential conflict between expectations of
> meaning of these enums. I see common ways to achieve this have been to
> use `#ifndef __KERNEL__` conditional and by defining them as higher
> numbers that are less likely to conflict like the existing tunnel
> types.

Ok, will put them inside of `#ifndef __KERNEL__`.

> > +            csum = packet_csum_pseudoheader6(
> > +                       netdev_tnl_ipv6_hdr(dp_packet_data(packet)));
> > +        } else {
> > +            csum = packet_csum_pseudoheader(
> > +                       netdev_tnl_ip_hdr(dp_packet_data(packet)));
> > +        }
> > +
> > +        csum = csum_continue(csum, udp, ip_tot_size);
> > +        udp->udp_csum = csum_finish(csum);
> > +
> > +        if (!udp->udp_csum) {
> > +            udp->udp_csum = htons(0xffff);
> 
> What's the motivation behind this?

Feng, does this mean indicating protocol stack to ignore UDP checksum check?

> 
> > +        }
> > +    }
> > +
> > +    packet->packet_type = htonl(PT_ETH);
> > +}
> > +
> > +
> > +int
> > +netdev_gtpu_build_header(const struct netdev *netdev,
> > +                          struct ovs_action_push_tnl *data,
> > +                          const struct netdev_tnl_build_header_params *params)
> > +{
> > +    struct netdev_vport *dev = netdev_vport_cast(netdev);
> > +    struct netdev_tunnel_config *tnl_cfg;
> > +    struct gtpuhdr *gtph;
> > +    struct udp_header *udp;
> > +
> > +    /* XXX: RCUfy tnl_cfg. */
> > +    ovs_mutex_lock(&dev->mutex);
> > +
> > +    tnl_cfg = &dev->tnl_cfg;
> > +    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP);
> > +    udp->udp_dst = tnl_cfg->dst_port;
> > +    udp->udp_src = htons(GTPU_DST_PORT);
> > +
> > +    if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
> > +        /* Write a value in now to mark that we should compute the checksum
> > +         * later. 0xffff is handy because it is transparent to the
> > +         * calculation. */
> > +        udp->udp_csum = htons(0xffff);
> 
> Maybe I missed it, but I didn't see where in this patch that it is
> computed later due to this value.

Feng will check this.

> 
> > +    }
> > +
> > +    ovs_mutex_unlock(&dev->mutex);
> > +    data->header_len += sizeof *udp;
> > +
> > +    gtph = (struct gtpuhdr *)(udp + 1);
> > +    gtph->flags = GTPU_FLAGS_DEFAULT;
> > +    gtph->msgtype = GTPU_MSGTYPE_GPDU;
> > +    if (params->flow->tunnel.gtpu_flags != 0) {
> > +        gtph->flags = params->flow->tunnel.gtpu_flags;
> > +    }
> > +    if (params->flow->tunnel.gtpu_msgtype != 0) {
> > +        gtph->msgtype = params->flow->tunnel.gtpu_msgtype;
> > +    }
> > +    put_16aligned_be32(&gtph->teid,
> > +                       htonl(ntohll(params->flow->tunnel.tun_id)));
> > +    data->header_len += sizeof *gtph;
> > +    data->tnl_type = OVS_VPORT_TYPE_GTPU;
> > +    return 0;
> > +}
> > +
> > +struct dp_packet *
> > +netdev_gtpu_pop_header(struct dp_packet *packet)
> > +{
> > +    struct pkt_metadata *md = &packet->md;
> > +    struct flow_tnl *tnl = &md->tunnel;
> > +    struct gtpuhdr *gtph;
> > +    unsigned int hlen;
> > +
> > +    pkt_metadata_init_tnl(md);
> > +    if (GTPU_HLEN > dp_packet_l4_size(packet)) {
> > +        VLOG_WARN_RL(&err_rl, "GTP-U packet too small: min header=%u "
> > +                              "packet size=%"PRIuSIZE"\n",
> > +                     (unsigned int)GTPU_HLEN, dp_packet_l4_size(packet));
> > +        goto err;
> > +    }
> > +
> > +    gtph = udp_extract_tnl_md(packet, tnl, &hlen);
> > +    if (!gtph) {
> > +        goto err;
> > +    }
> > +
> > +    if (gtph->flags == 0x30) {
> 
> Should this use GTPU_FLAGS_DEFAULT?

Yes.

> 
> > +    /* Only GTP-U v1 packets without optional fileds are processed, i.e.
> > +     *     8   7   6  5    4   3   2   1
> > +     *    |   ver   | PT | 0 | E | S | PN |
> > +     *     0   0   1  1    0   0   0   0
> > +     */
> > +        tnl->gtpu_flags = gtph->flags;
> > +    } else {
> > +        goto err;
> 
> You might consider restricting matches on GTP headers to ensure that
> flows MUST match on these supported flags and the value must be equal
> before installing such a flow, so that controller writers will know
> the first time they attempt to install the flow that OVS doesn't
> support matching/handling other flags. This would be better warning
> than deploying flows that seem to work, then finding out down the line
> that if an extra bit is set then the packet will be silently dropped.
> 
> Perhaps also could put a ratelimited warn or some kind of drop counter
> here, I don't know the protocol semantics of optional fields but it
> seems a little surprising to me to just drop packets on the ground if
> they're setting bits that this code doesn't know how to process.
> 
> Either way, this kind of behaviour should be documented too if it's not already.
> 
Good comment, Feng and I will enhance it to handle all the flags.

> > +    }
> > +    tnl->gtpu_msgtype = gtph->msgtype;
> > +    tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gtph->teid)));
> > +
> > +    /*Figure out whether the inner packet is IPv4, IPv6 or a GTP-U message.*/
> > +    if (tnl->gtpu_msgtype == GTPU_MSGTYPE_GPDU) {
> > +        struct ip_header *ip;
> > +
> > +        ip = (struct ip_header *)(gtph + 1);
> > +        if (IP_VER(ip->ip_ihl_ver) == 4) {
> > +            packet->packet_type = htonl(PT_IPV4);
> > +        } else if (IP_VER(ip->ip_ihl_ver) == 6) {
> > +            packet->packet_type = htonl(PT_IPV6);
> > +        } else {
> > +            goto err;
> > +        }
> > +    } else {
> > +        /* tnl->gtpu_msgtype can identify if it is GTP-U message,
> > +         * miniflow_extract won't parse it if tnl->gtpu_msgtype
> > +         * isn't equal to GTPU_MSGTYPE_GPDU.
> > +         */
> > +        packet->packet_type = htonl(PT_UNKNOWN);
> 
> This is where the limits of my GTP understanding will show, I was
> under the impression that GTP-U provides encapsulation for user
> traffic, so would commonly encapsulate IPv4/IPv6 inside. Whereas for
> GTP-C, it's signalling traffic so would not have such inner headers -
> it's supposed to be handled by a GTP implementation. Why is it
> important to be able to decapsulate GTP-C? Furthermore, what's
> ensuring that we don't end up spitting inner GTP-C packet data onto
> the wire without headers post-translation?
> 
GTP-C is for control path, so I don't think it makes sense for OVS to
handle this, in addition GTP-C used different UDP port from GTP-U's, so
we must have different tunnel ports to handle them respectively. 

I don't know if Jiannan has implemented GTP-C support in kernel data
path, we can add GTP-C support if it is really needed, but I think it is
a nice-to-have thing.
Joe Stringer March 5, 2018, 3:40 a.m. | #4
On 4 March 2018 at 18:48, Yang, Yi <yi.y.yang@intel.com> wrote:
> On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
>> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
>> >
>> > Signed-off-by: Feng Yang <feng.yang@intel.com>
>> > Signed-off-by: Yi Yang <yi.y.yang@intel.com>
>> > ---
>>
>> Hi Yi, thanks for this work. Some minor comments below, but this is
>> just a "drive-by" review. I hope that others with more experience with
>> GTP will take a look as well.
>>
>> I see the comment that there is locking on the device whenever a GTP
>> header is built, I assume you intend to address that in a followup
>> iteration.
>
> Joe, thank for your comments, can you elaborate "there is locking on the
> device whenever a GTP  header is built" with more details? I don't know
> this, or there is any existing document for this? We'll fix it in next
> iteration.

See the comment " /* XXX: RCUfy tnl_cfg. */ " and related locking in the patch.

>>
>> If you haven't already, it may be interesting to sync with Jiannan
>> (CC'd) who has previously done some work on GTP support in OVS for
>> kernel, to see whether the API (OpenFlow, OVSDB) makes sense for a
>> cross-datapath (ie kernel AND userspace) implementation perspective.
>
> Yes, sync is necessary, we contacted Jiannan before, I'll ping him to
> check the status of his kernel patch, we'll make sure they can work well
> together.

Awesome.

>> > +    }
>> > +    tnl->gtpu_msgtype = gtph->msgtype;
>> > +    tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gtph->teid)));
>> > +
>> > +    /*Figure out whether the inner packet is IPv4, IPv6 or a GTP-U message.*/
>> > +    if (tnl->gtpu_msgtype == GTPU_MSGTYPE_GPDU) {
>> > +        struct ip_header *ip;
>> > +
>> > +        ip = (struct ip_header *)(gtph + 1);
>> > +        if (IP_VER(ip->ip_ihl_ver) == 4) {
>> > +            packet->packet_type = htonl(PT_IPV4);
>> > +        } else if (IP_VER(ip->ip_ihl_ver) == 6) {
>> > +            packet->packet_type = htonl(PT_IPV6);
>> > +        } else {
>> > +            goto err;
>> > +        }
>> > +    } else {
>> > +        /* tnl->gtpu_msgtype can identify if it is GTP-U message,
>> > +         * miniflow_extract won't parse it if tnl->gtpu_msgtype
>> > +         * isn't equal to GTPU_MSGTYPE_GPDU.
>> > +         */
>> > +        packet->packet_type = htonl(PT_UNKNOWN);
>>
>> This is where the limits of my GTP understanding will show, I was
>> under the impression that GTP-U provides encapsulation for user
>> traffic, so would commonly encapsulate IPv4/IPv6 inside. Whereas for
>> GTP-C, it's signalling traffic so would not have such inner headers -
>> it's supposed to be handled by a GTP implementation. Why is it
>> important to be able to decapsulate GTP-C? Furthermore, what's
>> ensuring that we don't end up spitting inner GTP-C packet data onto
>> the wire without headers post-translation?
>>
> GTP-C is for control path, so I don't think it makes sense for OVS to
> handle this, in addition GTP-C used different UDP port from GTP-U's, so
> we must have different tunnel ports to handle them respectively.

Re: GTP-C in OVS, I'm inclined to agree that it doesn't make sense to
be in OVS. There's a lot of complexity there and existing FOSS
projects already attempt to support this, such as those under Osmocom.

The thing I was trying to highlight with the above comment is that
this function appears to be attempting to handle both GTP-U and GTP-C,
but if it doesn't make sense for OVS to handle it then I'm not sure
why this code attempts to do so. I don't think that expecting people
to just send the right kind of traffic on the designated ports (eg,
GTP-U on the GTP-U port) is enough, these paths need to be resilient
to unexpected input. Perhaps it's enough to restrict installation of
flows with pop GTP actions based on presence of the appropriate GTP
msgtype in the flow key.

> I don't know if Jiannan has implemented GTP-C support in kernel data
> path, we can add GTP-C support if it is really needed, but I think it is
> a nice-to-have thing.

I believe there is some kind of implementation already in the kernel,
but you'd have to take a look to see whether its design fits with what
you're trying to do.

Cheers,
Joe
Yang, Yi March 5, 2018, 3:58 a.m. | #5
On Sun, Mar 04, 2018 at 07:40:49PM -0800, Joe Stringer wrote:
> On 4 March 2018 at 18:48, Yang, Yi <yi.y.yang@intel.com> wrote:
> > On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
> >> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
> >> > +            goto err;
> >> > +        }
> >> > +    } else {
> >> > +        /* tnl->gtpu_msgtype can identify if it is GTP-U message,
> >> > +         * miniflow_extract won't parse it if tnl->gtpu_msgtype
> >> > +         * isn't equal to GTPU_MSGTYPE_GPDU.
> >> > +         */
> >> > +        packet->packet_type = htonl(PT_UNKNOWN);
> >>
> >> This is where the limits of my GTP understanding will show, I was
> >> under the impression that GTP-U provides encapsulation for user
> >> traffic, so would commonly encapsulate IPv4/IPv6 inside. Whereas for
> >> GTP-C, it's signalling traffic so would not have such inner headers -
> >> it's supposed to be handled by a GTP implementation. Why is it
> >> important to be able to decapsulate GTP-C? Furthermore, what's
> >> ensuring that we don't end up spitting inner GTP-C packet data onto
> >> the wire without headers post-translation?
> >>
> > GTP-C is for control path, so I don't think it makes sense for OVS to
> > handle this, in addition GTP-C used different UDP port from GTP-U's, so
> > we must have different tunnel ports to handle them respectively.
> 
> Re: GTP-C in OVS, I'm inclined to agree that it doesn't make sense to
> be in OVS. There's a lot of complexity there and existing FOSS
> projects already attempt to support this, such as those under Osmocom.
> 
> The thing I was trying to highlight with the above comment is that
> this function appears to be attempting to handle both GTP-U and GTP-C,
> but if it doesn't make sense for OVS to handle it then I'm not sure
> why this code attempts to do so. I don't think that expecting people
> to just send the right kind of traffic on the designated ports (eg,
> GTP-U on the GTP-U port) is enough, these paths need to be resilient
> to unexpected input. Perhaps it's enough to restrict installation of
> flows with pop GTP actions based on presence of the appropriate GTP
> msgtype in the flow key.

GTP-U has two kinds of message types, normal GTP-U PDU and signaling
messages, so we have to handle signaling messages here, it isn't GTP-C,
http://www.3gpp.org/ftp/Specs/archive/29_series/29.281/29281-f10.zip
has detailed description.

By the way, Feng said the existing GTP kernel implementation also didn't
implement GTP-C and didn't handle other optional flag bits.

> 
> > I don't know if Jiannan has implemented GTP-C support in kernel data
> > path, we can add GTP-C support if it is really needed, but I think it is
> > a nice-to-have thing.
> 
> I believe there is some kind of implementation already in the kernel,
> but you'd have to take a look to see whether its design fits with what
> you're trying to do.
> 
> Cheers,
> Joe
Joe Stringer March 5, 2018, 5:59 a.m. | #6
On 4 March 2018 at 19:58, Yang, Yi <yi.y.yang@intel.com> wrote:
> On Sun, Mar 04, 2018 at 07:40:49PM -0800, Joe Stringer wrote:
>> On 4 March 2018 at 18:48, Yang, Yi <yi.y.yang@intel.com> wrote:
>> > On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
>> >> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
>> >> > +            goto err;
>> >> > +        }
>> >> > +    } else {
>> >> > +        /* tnl->gtpu_msgtype can identify if it is GTP-U message,
>> >> > +         * miniflow_extract won't parse it if tnl->gtpu_msgtype
>> >> > +         * isn't equal to GTPU_MSGTYPE_GPDU.
>> >> > +         */
>> >> > +        packet->packet_type = htonl(PT_UNKNOWN);
>> >>
>> >> This is where the limits of my GTP understanding will show, I was
>> >> under the impression that GTP-U provides encapsulation for user
>> >> traffic, so would commonly encapsulate IPv4/IPv6 inside. Whereas for
>> >> GTP-C, it's signalling traffic so would not have such inner headers -
>> >> it's supposed to be handled by a GTP implementation. Why is it
>> >> important to be able to decapsulate GTP-C? Furthermore, what's
>> >> ensuring that we don't end up spitting inner GTP-C packet data onto
>> >> the wire without headers post-translation?
>> >>
>> > GTP-C is for control path, so I don't think it makes sense for OVS to
>> > handle this, in addition GTP-C used different UDP port from GTP-U's, so
>> > we must have different tunnel ports to handle them respectively.
>>
>> Re: GTP-C in OVS, I'm inclined to agree that it doesn't make sense to
>> be in OVS. There's a lot of complexity there and existing FOSS
>> projects already attempt to support this, such as those under Osmocom.
>>
>> The thing I was trying to highlight with the above comment is that
>> this function appears to be attempting to handle both GTP-U and GTP-C,
>> but if it doesn't make sense for OVS to handle it then I'm not sure
>> why this code attempts to do so. I don't think that expecting people
>> to just send the right kind of traffic on the designated ports (eg,
>> GTP-U on the GTP-U port) is enough, these paths need to be resilient
>> to unexpected input. Perhaps it's enough to restrict installation of
>> flows with pop GTP actions based on presence of the appropriate GTP
>> msgtype in the flow key.
>
> GTP-U has two kinds of message types, normal GTP-U PDU and signaling
> messages, so we have to handle signaling messages here, it isn't GTP-C,
> http://www.3gpp.org/ftp/Specs/archive/29_series/29.281/29281-f10.zip
> has detailed description.

I see, thanks for the reference.

> By the way, Feng said the existing GTP kernel implementation also didn't
> implement GTP-C and didn't handle other optional flag bits.

Ah, okay. The thing I was thinking of was the way to configure a GTP
socket to forward signalling traffic to a userspace process for
handling:

https://www.kernel.org/doc/Documentation/networking/gtp.txt

>>
>> > I don't know if Jiannan has implemented GTP-C support in kernel data
>> > path, we can add GTP-C support if it is really needed, but I think it is
>> > a nice-to-have thing.
>>
>> I believe there is some kind of implementation already in the kernel,
>> but you'd have to take a look to see whether its design fits with what
>> you're trying to do.
>>
>> Cheers,
>> Joe
Yang, Yi March 5, 2018, 11:08 a.m. | #7
On Sun, Mar 04, 2018 at 09:59:18PM -0800, Joe Stringer wrote:
> On 4 March 2018 at 19:58, Yang, Yi <yi.y.yang@intel.com> wrote:
> > On Sun, Mar 04, 2018 at 07:40:49PM -0800, Joe Stringer wrote:
> >> On 4 March 2018 at 18:48, Yang, Yi <yi.y.yang@intel.com> wrote:
> >> > On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
> >> >> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
> 
> > By the way, Feng said the existing GTP kernel implementation also didn't
> > implement GTP-C and didn't handle other optional flag bits.
> 
> Ah, okay. The thing I was thinking of was the way to configure a GTP
> socket to forward signalling traffic to a userspace process for
> handling:
> 
> https://www.kernel.org/doc/Documentation/networking/gtp.txt
>

Yeah, this is just why we are to handle signaling message here, we can
add Openflow rule to forward such message to local or remote userspace
process, per Feng's explanation, this signaling message will be output
to the GTP-U tunnel with different destination IP.
Yang, Yi March 5, 2018, 11:17 a.m. | #8
On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
> ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
> > From: Feng Yang <feng.yang@intel.com>
> >
> > +
> > +        csum = csum_continue(csum, udp, ip_tot_size);
> > +        udp->udp_csum = csum_finish(csum);
> > +
> > +        if (!udp->udp_csum) {
> > +            udp->udp_csum = htons(0xffff);
> 
> What's the motivation behind this?

More comments, this is from the existing ovs code, Feng checked why.

[Feng] According to the standard RFC768,
https://tools.ietf.org/html/rfc768, I quote, "if the computed  checksum
is zero,  it is transmitted  as all ones (the equivalent  in one's
complement  arithmetic)". Actually, netdev_tnl_udp_push_header( ) did
the same.

> 
> > +        }
> > +    }
> > +
> > +    packet->packet_type = htonl(PT_ETH);
> > +}
> > +
> > +
> > +int
> > +netdev_gtpu_build_header(const struct netdev *netdev,
> > +                          struct ovs_action_push_tnl *data,
> > +                          const struct netdev_tnl_build_header_params *params)
> > +{
> > +    struct netdev_vport *dev = netdev_vport_cast(netdev);
> > +    struct netdev_tunnel_config *tnl_cfg;
> > +    struct gtpuhdr *gtph;
> > +    struct udp_header *udp;
> > +
> > +    /* XXX: RCUfy tnl_cfg. */
> > +    ovs_mutex_lock(&dev->mutex);
> > +
> > +    tnl_cfg = &dev->tnl_cfg;
> > +    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP);
> > +    udp->udp_dst = tnl_cfg->dst_port;
> > +    udp->udp_src = htons(GTPU_DST_PORT);
> > +
> > +    if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
> > +        /* Write a value in now to mark that we should compute the checksum
> > +         * later. 0xffff is handy because it is transparent to the
> > +         * calculation. */
> > +        udp->udp_csum = htons(0xffff);
> 
> Maybe I missed it, but I didn't see where in this patch that it is
> computed later due to this value.
> 

This is also from the existing code. udp_build_header is doing same
thing.

[Feng]  udp_csum is calculated in netdev_gtpu_push_header( ). In
principle, udp_csum can be arbitrarily set to any two octets except all
zero, which means transmitter generated no checksum, according to the
standard, https://tools.ietf.org/html/rfc768.
We followed what udp_build_header( ) did, by setting udp_csum to 0xffff.
Joe Stringer March 5, 2018, 4:39 p.m. | #9
Ok, thanks for the clarification.

On Mon, 5 Mar 2018, 03:23 Yang, Yi, <yi.y.yang@intel.com> wrote:

> On Mon, Mar 05, 2018 at 04:19:58AM +0800, Joe Stringer wrote:
> > ?On 2 March 2018 at 04:32, Yi Yang <yi.y.yang@intel.com> wrote:
> > > From: Feng Yang <feng.yang@intel.com>
> > >
> > > +
> > > +        csum = csum_continue(csum, udp, ip_tot_size);
> > > +        udp->udp_csum = csum_finish(csum);
> > > +
> > > +        if (!udp->udp_csum) {
> > > +            udp->udp_csum = htons(0xffff);
> >
> > What's the motivation behind this?
>
> More comments, this is from the existing ovs code, Feng checked why.
>
> [Feng] According to the standard RFC768,
> https://tools.ietf.org/html/rfc768, I quote, "if the computed  checksum
> is zero,  it is transmitted  as all ones (the equivalent  in one's
> complement  arithmetic)". Actually, netdev_tnl_udp_push_header( ) did
> the same.
>
> >
> > > +        }
> > > +    }
> > > +
> > > +    packet->packet_type = htonl(PT_ETH);
> > > +}
> > > +
> > > +
> > > +int
> > > +netdev_gtpu_build_header(const struct netdev *netdev,
> > > +                          struct ovs_action_push_tnl *data,
> > > +                          const struct netdev_tnl_build_header_params
> *params)
> > > +{
> > > +    struct netdev_vport *dev = netdev_vport_cast(netdev);
> > > +    struct netdev_tunnel_config *tnl_cfg;
> > > +    struct gtpuhdr *gtph;
> > > +    struct udp_header *udp;
> > > +
> > > +    /* XXX: RCUfy tnl_cfg. */
> > > +    ovs_mutex_lock(&dev->mutex);
> > > +
> > > +    tnl_cfg = &dev->tnl_cfg;
> > > +    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP);
> > > +    udp->udp_dst = tnl_cfg->dst_port;
> > > +    udp->udp_src = htons(GTPU_DST_PORT);
> > > +
> > > +    if (params->is_ipv6 || params->flow->tunnel.flags &
> FLOW_TNL_F_CSUM) {
> > > +        /* Write a value in now to mark that we should compute the
> checksum
> > > +         * later. 0xffff is handy because it is transparent to the
> > > +         * calculation. */
> > > +        udp->udp_csum = htons(0xffff);
> >
> > Maybe I missed it, but I didn't see where in this patch that it is
> > computed later due to this value.
> >
>
> This is also from the existing code. udp_build_header is doing same
> thing.
>
> [Feng]  udp_csum is calculated in netdev_gtpu_push_header( ). In
> principle, udp_csum can be arbitrarily set to any two octets except all
> zero, which means transmitter generated no checksum, according to the
> standard, https://tools.ietf.org/html/rfc768.
> We followed what udp_build_header( ) did, by setting udp_csum to 0xffff.
>
>
Jiannan Ouyang March 5, 2018, 8:28 p.m. | #10
On 3/4/18, 6:55 PM, "Yang, Yi" <yi.y.yang@intel.com> wrote:
  GTP-C is for control path, so I don't think it makes sense for OVS to
  handle this, in addition GTP-C used different UDP port from GTP-U's, so
  we must have different tunnel ports to handle them respectively. 
 
  I don't know if Jiannan has implemented GTP-C support in kernel data
  path, we can add GTP-C support if it is really needed, but I think it is
  a nice-to-have thing.

No, I didn’t implement GTP-C neither.

Patch

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 184b75e..944f095 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -68,6 +68,7 @@  PREREQS = {"none": "MFP_NONE",
 OXM_CLASSES = {"NXM_OF_":        (0,          0x0000),
                "NXM_NX_":        (0,          0x0001),
                "NXOXM_NSH_":     (0x005ad650, 0xffff),
+               "NXOXM_GTPU_":    (0x005ad651, 0xffff),
                "OXM_OF_":        (0,          0x8000),
                "OXM_OF_PKT_REG": (0,          0x8001),
                "ONFOXM_ET_":     (0x4f4e4600, 0xffff),
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 84ebcaf..ad6ea64 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -236,6 +236,7 @@  enum ovs_vport_type {
 	OVS_VPORT_TYPE_GRE,      /* GRE tunnel. */
 	OVS_VPORT_TYPE_VXLAN,	 /* VXLAN tunnel. */
 	OVS_VPORT_TYPE_GENEVE,	 /* Geneve tunnel. */
+	OVS_VPORT_TYPE_GTPU,     /* GTPU tunnel. */
 	OVS_VPORT_TYPE_LISP = 105,  /* LISP tunnel */
 	OVS_VPORT_TYPE_STT = 106, /* STT tunnel */
 	__OVS_VPORT_TYPE_MAX
@@ -394,6 +395,8 @@  enum ovs_tunnel_key_attr {
 	OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS,		/* Nested OVS_VXLAN_EXT_* */
 	OVS_TUNNEL_KEY_ATTR_IPV6_SRC,		/* struct in6_addr src IPv6 address. */
 	OVS_TUNNEL_KEY_ATTR_IPV6_DST,		/* struct in6_addr dst IPv6 address. */
+	OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS,         /* u8 GTP-U FLAGS. */
+	OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE,       /* u8 GTP-U MSGTYPE. */
 	OVS_TUNNEL_KEY_ATTR_PAD,
 	__OVS_TUNNEL_KEY_ATTR_MAX
 };
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 61a67de..fb41736 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -109,6 +109,10 @@  void match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16
 void match_set_tun_gbp_id(struct match *match, ovs_be16 gbp_id);
 void match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, uint8_t mask);
 void match_set_tun_gbp_flags(struct match *match, uint8_t flags);
+void match_set_tun_gtpu_flags(struct match *match, uint8_t gtpu_flags);
+void match_set_tun_gtpu_flags_masked(struct match *match, uint8_t gtpu_flags,
+                                     uint8_t mask);
+void match_set_tun_gtpu_msgtype(struct match *match, uint8_t gtpu_msgtype);
 void match_set_in_port(struct match *, ofp_port_t ofp_port);
 void match_set_pkt_mark(struct match *, uint32_t pkt_mark);
 void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t mask);
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 98c9e1c..884a2f6 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -1848,6 +1848,34 @@  enum OVS_PACKED_ENUM mf_field_id {
      */
     MFF_NSH_TTL,
 
+    /* "tun_gtpu_flags".
+     *
+     * The flags in a GTP-U tunnel header.
+     *
+     * Type: u8.
+     * Maskable: bitwise.
+     * Formatting: decimal.
+     * Prerequisites: none.
+     * Access: read/write.
+     * NXM: none.
+     * OXM: NXOXM_GTPU_FLAGS(1) since OF1.3 and v2.10.
+     */
+    MFF_TUN_GTPU_FLAGS,
+
+    /* "tun_gtpu_msgtype".
+     *
+     * The message type in a GTP-U tunnel header.
+     *
+     * Type: u8.
+     * Maskable: no.
+     * Formatting: decimal.
+     * Prerequisites: none.
+     * Access: read/write.
+     * NXM: none.
+     * OXM: NXOXM_GTPU_MSGTYPE(2) since OF1.3 and v2.10.
+     */
+    MFF_TUN_GTPU_MSGTYPE,
+
     MFF_N_IDS
 };
 
diff --git a/include/openvswitch/packets.h b/include/openvswitch/packets.h
index fef756b..1f3b499 100644
--- a/include/openvswitch/packets.h
+++ b/include/openvswitch/packets.h
@@ -39,7 +39,9 @@  struct flow_tnl {
     ovs_be16 tp_dst;
     ovs_be16 gbp_id;
     uint8_t  gbp_flags;
-    uint8_t  pad1[5];        /* Pad to 64 bits. */
+    uint8_t  gtpu_flags;
+    uint8_t  gtpu_msgtype;
+    uint8_t  pad1[3];        /* Pad to 64 bits. */
     struct tun_metadata metadata;
 };
 
diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 40c4569..a628b72 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -99,6 +99,8 @@  vport_type_to_kind(enum ovs_vport_type type,
         }
     case OVS_VPORT_TYPE_GENEVE:
         return "geneve";
+    case OVS_VPORT_TYPE_GTPU:
+        return "gtpu";
     case OVS_VPORT_TYPE_NETDEV:
     case OVS_VPORT_TYPE_INTERNAL:
     case OVS_VPORT_TYPE_LISP:
@@ -262,6 +264,7 @@  dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg,
     case OVS_VPORT_TYPE_INTERNAL:
     case OVS_VPORT_TYPE_LISP:
     case OVS_VPORT_TYPE_STT:
+    case OVS_VPORT_TYPE_GTPU:
     case OVS_VPORT_TYPE_UNSPEC:
     case __OVS_VPORT_TYPE_MAX:
     default:
@@ -327,6 +330,7 @@  dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg,
     case OVS_VPORT_TYPE_INTERNAL:
     case OVS_VPORT_TYPE_LISP:
     case OVS_VPORT_TYPE_STT:
+    case OVS_VPORT_TYPE_GTPU:
     case OVS_VPORT_TYPE_UNSPEC:
     case __OVS_VPORT_TYPE_MAX:
     default:
@@ -438,6 +442,7 @@  dpif_netlink_rtnl_port_destroy(const char *name, const char *type)
     case OVS_VPORT_TYPE_INTERNAL:
     case OVS_VPORT_TYPE_LISP:
     case OVS_VPORT_TYPE_STT:
+    case OVS_VPORT_TYPE_GTPU:
     case OVS_VPORT_TYPE_UNSPEC:
     case __OVS_VPORT_TYPE_MAX:
     default:
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 8543a2b..19ed182 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -786,6 +786,9 @@  get_vport_type(const struct dpif_netlink_vport *vport)
     case OVS_VPORT_TYPE_STT:
         return "stt";
 
+    case OVS_VPORT_TYPE_GTPU:
+        return "gtpu";
+
     case OVS_VPORT_TYPE_UNSPEC:
     case __OVS_VPORT_TYPE_MAX:
         break;
diff --git a/lib/flow.c b/lib/flow.c
index 38ff29c..1648667 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -695,6 +695,11 @@  miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
         miniflow_push_be32(mf, packet_type, packet_type);
     }
 
+    /* It won't be parsed if the packet is GTP-U message but not GTP-U PDU */
+    if (packet_type == htonl(PT_UNKNOWN)) {
+        goto out;
+    }
+
     /* Initialize packet's layer pointer and offsets. */
     frame = data;
     dp_packet_reset_offsets(packet);
diff --git a/lib/match.c b/lib/match.c
index 97a5282..5c845ed 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -320,6 +320,27 @@  match_set_tun_gbp_flags(struct match *match, uint8_t flags)
 }
 
 void
+match_set_tun_gtpu_flags(struct match *match, uint8_t gtpu_flags)
+{
+    match_set_tun_gtpu_flags_masked(match, gtpu_flags, UINT8_MAX);
+}
+
+void
+match_set_tun_gtpu_flags_masked(struct match *match, uint8_t gtpu_flags,
+                                uint8_t mask)
+{
+    match->wc.masks.tunnel.gtpu_flags = mask;
+    match->flow.tunnel.gtpu_flags = gtpu_flags & mask;
+}
+
+void
+match_set_tun_gtpu_msgtype(struct match *match, uint8_t gtpu_msgtype)
+{
+    match->flow.tunnel.gtpu_msgtype = gtpu_msgtype;
+    match->wc.masks.tunnel.gtpu_msgtype = UINT8_MAX;
+}
+
+void
 match_set_in_port(struct match *match, ofp_port_t ofp_port)
 {
     match->wc.masks.in_port.ofp_port = u16_to_ofp(UINT16_MAX);
@@ -1239,6 +1260,14 @@  format_flow_tunnel(struct ds *s, const struct match *match)
                             FLOW_TNL_F_MASK);
         ds_put_char(s, ',');
     }
+
+    if (wc->masks.tunnel.gtpu_flags) {
+        ds_put_format(s, "tun_gtpu_flags=%"PRIx8",", tnl->gtpu_flags);
+    }
+    if (wc->masks.tunnel.gtpu_msgtype) {
+        ds_put_format(s, "tun_gtpu_msgtype=%"PRIu8",", tnl->gtpu_msgtype);
+    }
+
     tun_metadata_match_format(s, match);
 }
 
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index aa2ec01..2ea5f63 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -231,6 +231,10 @@  mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
         return !wc->masks.tunnel.gbp_id;
     case MFF_TUN_GBP_FLAGS:
         return !wc->masks.tunnel.gbp_flags;
+    case MFF_TUN_GTPU_FLAGS:
+        return !wc->masks.tunnel.gtpu_flags;
+    case MFF_TUN_GTPU_MSGTYPE:
+        return !wc->masks.tunnel.gtpu_msgtype;
     CASE_MFF_TUN_METADATA:
         return !ULLONG_GET(wc->masks.tunnel.metadata.present.map,
                            mf->id - MFF_TUN_METADATA0);
@@ -513,6 +517,8 @@  mf_is_value_valid(const struct mf_field *mf, const union mf_value *value)
     case MFF_TUN_TTL:
     case MFF_TUN_GBP_ID:
     case MFF_TUN_GBP_FLAGS:
+    case MFF_TUN_GTPU_FLAGS:
+    case MFF_TUN_GTPU_MSGTYPE:
     CASE_MFF_TUN_METADATA:
     case MFF_METADATA:
     case MFF_IN_PORT:
@@ -674,6 +680,12 @@  mf_get_value(const struct mf_field *mf, const struct flow *flow,
     case MFF_TUN_GBP_FLAGS:
         value->u8 = flow->tunnel.gbp_flags;
         break;
+    case MFF_TUN_GTPU_FLAGS:
+        value->u8 = flow->tunnel.gtpu_flags;
+        break;
+    case MFF_TUN_GTPU_MSGTYPE:
+        value->u8 = flow->tunnel.gtpu_msgtype;
+        break;
     case MFF_TUN_TTL:
         value->u8 = flow->tunnel.ip_ttl;
         break;
@@ -988,6 +1000,12 @@  mf_set_value(const struct mf_field *mf,
     case MFF_TUN_GBP_FLAGS:
          match_set_tun_gbp_flags(match, value->u8);
          break;
+    case MFF_TUN_GTPU_FLAGS:
+        match_set_tun_gtpu_flags(match, value->u8);
+        break;
+    case MFF_TUN_GTPU_MSGTYPE:
+        match_set_tun_gtpu_msgtype(match, value->u8);
+        break;
     case MFF_TUN_TOS:
         match_set_tun_tos(match, value->u8);
         break;
@@ -1385,6 +1403,12 @@  mf_set_flow_value(const struct mf_field *mf,
     case MFF_TUN_GBP_FLAGS:
         flow->tunnel.gbp_flags = value->u8;
         break;
+    case MFF_TUN_GTPU_FLAGS:
+        flow->tunnel.gtpu_flags = value->u8;
+        break;
+    case MFF_TUN_GTPU_MSGTYPE:
+        flow->tunnel.gtpu_msgtype = value->u8;
+        break;
     case MFF_TUN_TOS:
         flow->tunnel.ip_tos = value->u8;
         break;
@@ -1700,6 +1724,8 @@  mf_is_pipeline_field(const struct mf_field *mf)
     case MFF_TUN_FLAGS:
     case MFF_TUN_GBP_ID:
     case MFF_TUN_GBP_FLAGS:
+    case MFF_TUN_GTPU_FLAGS:
+    case MFF_TUN_GTPU_MSGTYPE:
     CASE_MFF_TUN_METADATA:
     case MFF_METADATA:
     case MFF_IN_PORT:
@@ -1870,6 +1896,14 @@  mf_set_wild(const struct mf_field *mf, struct match *match, char **err_str)
     case MFF_TUN_GBP_FLAGS:
         match_set_tun_gbp_flags_masked(match, 0, 0);
         break;
+    case MFF_TUN_GTPU_FLAGS:
+        match->wc.masks.tunnel.gtpu_flags = 0;
+        match->flow.tunnel.gtpu_flags = 0;
+        break;
+    case MFF_TUN_GTPU_MSGTYPE:
+        match->wc.masks.tunnel.gtpu_msgtype = 0;
+        match->flow.tunnel.gtpu_msgtype = 0;
+        break;
     case MFF_TUN_TOS:
         match_set_tun_tos_masked(match, 0, 0);
         break;
@@ -2221,6 +2255,7 @@  mf_set(const struct mf_field *mf,
     case MFF_ICMPV4_CODE:
     case MFF_ICMPV6_TYPE:
     case MFF_ICMPV6_CODE:
+    case MFF_TUN_GTPU_MSGTYPE:
         return OFPUTIL_P_NONE;
 
     case MFF_DP_HASH:
@@ -2250,6 +2285,9 @@  mf_set(const struct mf_field *mf,
     case MFF_TUN_GBP_FLAGS:
         match_set_tun_gbp_flags_masked(match, value->u8, mask->u8);
         break;
+    case MFF_TUN_GTPU_FLAGS:
+        match_set_tun_gtpu_flags_masked(match, value->u8, mask->u8);
+        break;
     case MFF_TUN_TTL:
         match_set_tun_ttl_masked(match, value->u8, mask->u8);
         break;
diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
index 933d4b8..d169772 100644
--- a/lib/meta-flow.xml
+++ b/lib/meta-flow.xml
@@ -731,6 +731,12 @@  tcp,tp_src=0x07c0/0xfff0
       Used by Open vSwitch for NSH extensions, in the absence of an official
       ONF-assigned class.  (This OUI is randomly generated.)
     </dd>
+
+    <dt>0x005ad651 (<code>NXOXM_GTPU</code>)</dt>
+    <dd>
+      Used by Open vSwitch for GTP-U extensions, in the absence of an official
+      ONF-assigned class.  (This OUI is randomly generated.)
+    </dd>
   </dl>
 
   <p>
@@ -1899,6 +1905,68 @@  ovs-ofctl add-flow br0 tun_metadata0=1234,actions=controller
       </p>
     </field>
 
+    <h2>GTP-U Fields</h2>
+
+    <p>
+      The GTP-U (v1) header is defined as follows [3GPP TS 29.281].
+    </p>
+
+    <diagram>
+      <header name="GTP-U tunnel flags">
+        <bits name="V" above="3" width="0.15"/>
+        <bits name="PT" above="1" width="0.15"/>
+        <bits name="(*)" above="1" width="0.15"/>
+        <bits name="E" above="1" width="0.15"/>
+        <bits name="S" above="1" width="0.15"/>
+        <bits name="PN" above="1" width="0.15"/>
+      </header>
+      <nospace/>
+      <header>
+         <bits name="Message Type" above="8" width="0.15"/>
+         <bits name="Length" above="16" width="0.15"/>
+         <bits name="TEID" above="32" width="0.15"/>
+         <bits name="..."  above="" width="0.15"/>
+      </header>
+    </diagram>
+
+    <p>
+       where
+    </p>
+    <ul>
+      <li><code>V</code>: version fields, used to determine the version of the
+      GTP-U protocol, which should be set to '1'.</li>
+      <li><code>PT</code>: Packet Type, used as a protocol discriminator
+      between GTP (when PT is '1') and GTP' (when PT is '0').</li>
+      <li><code>E</code>: Extension Header flag, indicating the presence of a
+      meaningful value of the Next Extension Header field.</li>
+      <li><code>S</code>: Sequence number flag, indicating the presence of a
+      meaningful value of the Sequence Number field.</li>
+      <li><code>PN</code>: N-PDU Number flag, indicating the presence of a
+      meaningful value of the N-PDU Number field.</li>
+      <li><code>Message Type</code>: indicating the type of GTP-U message.
+      </li>
+      <li><code>Length</code>: indicating the length in octets of the payload.
+      </li>
+      <li><code>TEID</code>: Tunnel Endpoint Identifier,unambiguously
+      identifying a tunnel endpoint in the receiving GTP-U protocol entity.
+      Open vSwitch makes TEID availabe via <ref field="tun_id"/>.</li>
+    </ul>
+    <field id="MFF_TUN_GTPU_FLAGS" title="GTP-U tunnel flags">
+      <p>
+        For a packet tunneled over GTP-U, this field indicates the presence of
+        optional headers.
+      </p>
+    </field>
+
+    <field id="MFF_TUN_GTPU_MSGTYPE" title="GTP-U tunnel message type">
+      <p>
+        For a packet tunneled over GTP-U, this field indicates whether it's a
+        signalling message used for path management, or a user plane message
+        which carries the original packet. The complete range of message types
+        can be referred to [3GPP TS 29.281].
+      </p>
+    </field>
+
     <!-- Open vSwitch uses the following fields internally, but it
          does not expose them to the user via OpenFlow, so we do not
          document them. -->
@@ -4662,6 +4730,14 @@  r r c c c.
       <url href="https://tools.ietf.org/html/rfc7665"/>.
     </dd>
 
+    <dt>3GPP TS 29.281</dt>
+    <dd>
+      3GPP,
+      ``General Packet Radio System (GPRS) Tunnelling Protocol User Plane
+      (GTPv1-U),''
+      <url href="http://www.3gpp.org/ftp/Specs/html-info/29281.htm"/>.
+    </dd>
+
     <dt>Srinivasan</dt>
     <dd>
       V. Srinivasan, S. Suriy, and G. Varghese, ``Packet
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
index fb5eab0..bcf599d 100644
--- a/lib/netdev-native-tnl.c
+++ b/lib/netdev-native-tnl.c
@@ -55,6 +55,9 @@  static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(60, 5);
 #define GENEVE_BASE_HLEN   (sizeof(struct udp_header) +         \
                             sizeof(struct genevehdr))
 
+#define GTPU_HLEN   (sizeof(struct udp_header) +        \
+                     sizeof(struct gtpuhdr))
+
 uint16_t tnl_udp_port_min = 32768;
 uint16_t tnl_udp_port_max = 61000;
 
@@ -715,7 +718,152 @@  netdev_geneve_build_header(const struct netdev *netdev,
     return 0;
 }
 
-
+void
+netdev_gtpu_push_header(struct dp_packet *packet,
+                        const struct ovs_action_push_tnl *data)
+{
+    struct udp_header *udp;
+    struct gtpuhdr *gtph;
+    int gtpu_len;
+    int ip_tot_size;
+
+    gtpu_len = dp_packet_size(packet);
+
+    udp = netdev_tnl_push_ip_header(packet, data->header, data->header_len,
+                                    &ip_tot_size);
+    udp->udp_len = htons(ip_tot_size);
+
+    gtph = (struct gtpuhdr *)(udp + 1);
+    gtph->len = htons(gtpu_len);
+
+    if (udp->udp_csum) {
+        uint32_t csum;
+        if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
+            csum = packet_csum_pseudoheader6(
+                       netdev_tnl_ipv6_hdr(dp_packet_data(packet)));
+        } else {
+            csum = packet_csum_pseudoheader(
+                       netdev_tnl_ip_hdr(dp_packet_data(packet)));
+        }
+
+        csum = csum_continue(csum, udp, ip_tot_size);
+        udp->udp_csum = csum_finish(csum);
+
+        if (!udp->udp_csum) {
+            udp->udp_csum = htons(0xffff);
+        }
+    }
+
+    packet->packet_type = htonl(PT_ETH);
+}
+
+
+int
+netdev_gtpu_build_header(const struct netdev *netdev,
+                          struct ovs_action_push_tnl *data,
+                          const struct netdev_tnl_build_header_params *params)
+{
+    struct netdev_vport *dev = netdev_vport_cast(netdev);
+    struct netdev_tunnel_config *tnl_cfg;
+    struct gtpuhdr *gtph;
+    struct udp_header *udp;
+
+    /* XXX: RCUfy tnl_cfg. */
+    ovs_mutex_lock(&dev->mutex);
+
+    tnl_cfg = &dev->tnl_cfg;
+    udp = netdev_tnl_ip_build_header(data, params, IPPROTO_UDP);
+    udp->udp_dst = tnl_cfg->dst_port;
+    udp->udp_src = htons(GTPU_DST_PORT);
+
+    if (params->is_ipv6 || params->flow->tunnel.flags & FLOW_TNL_F_CSUM) {
+        /* Write a value in now to mark that we should compute the checksum
+         * later. 0xffff is handy because it is transparent to the
+         * calculation. */
+        udp->udp_csum = htons(0xffff);
+    }
+
+    ovs_mutex_unlock(&dev->mutex);
+    data->header_len += sizeof *udp;
+
+    gtph = (struct gtpuhdr *)(udp + 1);
+    gtph->flags = GTPU_FLAGS_DEFAULT;
+    gtph->msgtype = GTPU_MSGTYPE_GPDU;
+    if (params->flow->tunnel.gtpu_flags != 0) {
+        gtph->flags = params->flow->tunnel.gtpu_flags;
+    }
+    if (params->flow->tunnel.gtpu_msgtype != 0) {
+        gtph->msgtype = params->flow->tunnel.gtpu_msgtype;
+    }
+    put_16aligned_be32(&gtph->teid,
+                       htonl(ntohll(params->flow->tunnel.tun_id)));
+    data->header_len += sizeof *gtph;
+    data->tnl_type = OVS_VPORT_TYPE_GTPU;
+    return 0;
+}
+
+struct dp_packet *
+netdev_gtpu_pop_header(struct dp_packet *packet)
+{
+    struct pkt_metadata *md = &packet->md;
+    struct flow_tnl *tnl = &md->tunnel;
+    struct gtpuhdr *gtph;
+    unsigned int hlen;
+
+    pkt_metadata_init_tnl(md);
+    if (GTPU_HLEN > dp_packet_l4_size(packet)) {
+        VLOG_WARN_RL(&err_rl, "GTP-U packet too small: min header=%u "
+                              "packet size=%"PRIuSIZE"\n",
+                     (unsigned int)GTPU_HLEN, dp_packet_l4_size(packet));
+        goto err;
+    }
+
+    gtph = udp_extract_tnl_md(packet, tnl, &hlen);
+    if (!gtph) {
+        goto err;
+    }
+
+    if (gtph->flags == 0x30) {
+    /* Only GTP-U v1 packets without optional fileds are processed, i.e.
+     *     8   7   6  5    4   3   2   1
+     *    |   ver   | PT | 0 | E | S | PN |
+     *     0   0   1  1    0   0   0   0
+     */
+        tnl->gtpu_flags = gtph->flags;
+    } else {
+        goto err;
+    }
+    tnl->gtpu_msgtype = gtph->msgtype;
+    tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gtph->teid)));
+
+    /*Figure out whether the inner packet is IPv4, IPv6 or a GTP-U message.*/
+    if (tnl->gtpu_msgtype == GTPU_MSGTYPE_GPDU) {
+        struct ip_header *ip;
+
+        ip = (struct ip_header *)(gtph + 1);
+        if (IP_VER(ip->ip_ihl_ver) == 4) {
+            packet->packet_type = htonl(PT_IPV4);
+        } else if (IP_VER(ip->ip_ihl_ver) == 6) {
+            packet->packet_type = htonl(PT_IPV6);
+        } else {
+            goto err;
+        }
+    } else {
+        /* tnl->gtpu_msgtype can identify if it is GTP-U message,
+         * miniflow_extract won't parse it if tnl->gtpu_msgtype
+         * isn't equal to GTPU_MSGTYPE_GPDU.
+         */
+        packet->packet_type = htonl(PT_UNKNOWN);
+    }
+
+    dp_packet_reset_packet(packet, hlen + GTPU_HLEN);
+
+    return packet;
+err:
+    dp_packet_delete(packet);
+    return NULL;
+}
+
 void
 netdev_tnl_egress_port_range(struct unixctl_conn *conn, int argc,
                              const char *argv[], void *aux OVS_UNUSED)
diff --git a/lib/netdev-native-tnl.h b/lib/netdev-native-tnl.h
index a912ce9..9c4cc66 100644
--- a/lib/netdev-native-tnl.h
+++ b/lib/netdev-native-tnl.h
@@ -58,6 +58,18 @@  netdev_vxlan_build_header(const struct netdev *netdev,
 struct dp_packet *
 netdev_vxlan_pop_header(struct dp_packet *packet);
 
+void
+netdev_gtpu_push_header(struct dp_packet *packet,
+                        const struct ovs_action_push_tnl *data);
+
+int
+netdev_gtpu_build_header(const struct netdev *netdev,
+                         struct ovs_action_push_tnl *data,
+                         const struct netdev_tnl_build_header_params *params);
+
+struct dp_packet *
+netdev_gtpu_pop_header(struct dp_packet *packet);
+
 static inline bool
 netdev_tnl_is_header_ipv6(const void *header)
 {
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 52aa12d..a10b2f1 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -107,7 +107,10 @@  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("lisp", type) || !strcmp("stt", type) ||
+             !strcmp("gtpu", type)
+            )
+           );
 }
 
 const char *
@@ -408,6 +411,8 @@  tunnel_supported_layers(const char *type,
         return TNL_L3;
     } else if (!strcmp(type, "gre")) {
         return TNL_L2 | TNL_L3;
+    } else if (!strcmp(type, "gtpu")) {
+        return TNL_L3;
     } else if (!strcmp(type, "vxlan")
                && tnl_cfg->exts & (1 << OVS_VXLAN_EXT_GPE)) {
         return TNL_L2 | TNL_L3;
@@ -455,6 +460,10 @@  set_tunnel_config(struct netdev *dev_, const struct smap *args, char **errp)
         tnl_cfg.dst_port = htons(STT_DST_PORT);
     }
 
+    if (!strcmp(type, "gtpu")) {
+        tnl_cfg.dst_port = htons(GTPU_DST_PORT);
+    }
+
     needs_dst_port = netdev_vport_needs_dst_port(dev_);
     tnl_cfg.dont_fragment = true;
 
@@ -979,6 +988,10 @@  netdev_vport_tunnel_register(void)
                                            NETDEV_VPORT_GET_IFINDEX),
         TUNNEL_CLASS("lisp", "lisp_sys", NULL, NULL, NULL, NULL),
         TUNNEL_CLASS("stt", "stt_sys", NULL, NULL, NULL, NULL),
+        TUNNEL_CLASS("gtpu", "gptu_sys", netdev_gtpu_build_header,
+                                         netdev_gtpu_push_header,
+                                         netdev_gtpu_pop_header,
+                                         NULL),
     };
     static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 
diff --git a/lib/nx-match.c b/lib/nx-match.c
index a8edb2e..c7f6c27 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1153,6 +1153,10 @@  nx_put_raw(struct ofpbuf *b, enum ofp_version oxm, const struct match *match,
                 flow->tunnel.gbp_id, match->wc.masks.tunnel.gbp_id);
     nxm_put_8m(&ctx, MFF_TUN_GBP_FLAGS, oxm,
                flow->tunnel.gbp_flags, match->wc.masks.tunnel.gbp_flags);
+    nxm_put_8m(&ctx, MFF_TUN_GTPU_FLAGS, oxm, flow->tunnel.gtpu_flags,
+               match->wc.masks.tunnel.gtpu_flags);
+    nxm_put_8m(&ctx, MFF_TUN_GTPU_MSGTYPE, oxm, flow->tunnel.gtpu_msgtype,
+               match->wc.masks.tunnel.gtpu_msgtype);
     tun_metadata_to_nx_match(b, oxm, match);
 
     /* Network Service Header */
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5da83b4..f200d90 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -712,6 +712,15 @@  format_odp_tnl_push_header(struct ds *ds, struct ovs_action_push_tnl *data)
             options++;
         }
         ds_put_format(ds, ")");
+    } else if (data->tnl_type == OVS_VPORT_TYPE_GTPU) {
+        const struct gtpuhdr *gtph;
+
+        gtph = format_udp_tnl_push_header(ds, udp);
+
+        ds_put_format(ds, "gtpu(flags=0x%"PRIx32
+                          ",msgtype=0x%"PRIx32",teid=0x%"PRIx32")",
+                      gtph->flags, gtph->msgtype,
+                      ntohl(get_16aligned_be32(&gtph->teid)));
     }
     ds_put_format(ds, ")");
 }
@@ -2373,6 +2382,8 @@  static const struct attr_len_tbl ovs_tun_key_attr_lens[OVS_TUNNEL_KEY_ATTR_MAX +
     [OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS]    = { .len = ATTR_LEN_NESTED,
                                             .next = ovs_vxlan_ext_attr_lens ,
                                             .next_max = OVS_VXLAN_EXT_MAX},
+    [OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS]    = { .len = 1 },
+    [OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE]  = { .len = 1 },
     [OVS_TUNNEL_KEY_ATTR_IPV6_SRC]      = { .len = 16 },
     [OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = 16 },
 };
@@ -2698,6 +2709,12 @@  odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
         case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS:
             tun_metadata_from_geneve_nlattr(a, is_mask, tun);
             break;
+        case OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS:
+            tun->gtpu_flags = nl_attr_get_u8(a);
+            break;
+        case OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE:
+            tun->gtpu_msgtype = nl_attr_get_u8(a);
+            break;
 
         default:
             /* Allow this to show up as unexpected, if there are unknown
@@ -2775,6 +2792,13 @@  tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl *tun_key,
                        (tun_key->gbp_flags << 16) | ntohs(tun_key->gbp_id));
         nl_msg_end_nested(a, vxlan_opts_ofs);
     }
+    if (tun_key->gtpu_flags) {
+        nl_msg_put_u8(a, OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS, tun_key->gtpu_flags);
+    }
+    if (tun_key->gtpu_msgtype) {
+        nl_msg_put_u8(a, OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE,
+                      tun_key->gtpu_msgtype);
+    }
     tun_metadata_to_geneve_nlattr(tun_key, tun_flow_key, key_buf, a);
 
     nl_msg_end_nested(a, tun_key_ofs);
@@ -3477,6 +3501,14 @@  format_odp_tun_attr(const struct nlattr *attr, const struct nlattr *mask_attr,
             format_odp_tun_geneve(a, ma, ds, verbose);
             ds_put_cstr(ds, "),");
             break;
+        case OVS_TUNNEL_KEY_ATTR_GTPU_FLAGS:
+            format_u8x(ds, "gtpu_flags", nl_attr_get_u8(a),
+                       ma ? nl_attr_get(ma) : NULL, verbose);
+            break;
+        case OVS_TUNNEL_KEY_ATTR_GTPU_MSGTYPE:
+            format_u8x(ds, "gtpu_msgtype", nl_attr_get_u8(a),
+                       ma ? nl_attr_get(ma) : NULL, verbose);
+            break;
         case OVS_TUNNEL_KEY_ATTR_PAD:
             break;
         case __OVS_TUNNEL_KEY_ATTR_MAX:
diff --git a/lib/packets.h b/lib/packets.h
index 9a71aa3..709e9cb 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1306,6 +1306,23 @@  BUILD_ASSERT_DECL(sizeof(struct vxlanhdr) == 8);
 #define VXLAN_F_GPE  0x4000
 #define VXLAN_HF_GPE 0x04000000
 
+/* GTP-U protocol header */
+struct gtpuhdr {
+    uint8_t flags;
+    uint8_t msgtype;
+    ovs_be16 len;
+    ovs_16aligned_be32 teid;
+};
+
+/* GTP-U UDP port */
+#define GTPU_DST_PORT 2152
+
+/* Default GTP-U flags */
+#define GTPU_FLAGS_DEFAULT 0x30
+
+/* GTP-U message type for normal user plane PDU */
+#define GTPU_MSGTYPE_GPDU 255
+
 /* Input values for PACKET_TYPE macros have to be in host byte order.
  * The _BE postfix indicates result is in network byte order. Otherwise result
  * is in host byte order. */
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index b814f7a..cb8fab8 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -177,6 +177,9 @@  tnl_type_to_nw_proto(const char type[])
     if (!strcmp(type, "vxlan")) {
         return IPPROTO_UDP;
     }
+    if (!strcmp(type, "gtpu")) {
+        return IPPROTO_UDP;
+    }
     return 0;
 }
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index cc450a8..9438d8b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3270,6 +3270,7 @@  propagate_tunnel_data_to_flow(struct xlate_ctx *ctx, struct eth_addr dmac,
         break;
     case OVS_VPORT_TYPE_VXLAN:
     case OVS_VPORT_TYPE_GENEVE:
+    case OVS_VPORT_TYPE_GTPU:
         nw_proto = IPPROTO_UDP;
         break;
     case OVS_VPORT_TYPE_LISP:
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index e0214ce..becb752 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -384,6 +384,12 @@  tnl_wc_init(struct flow *flow, struct flow_wildcards *wc)
         wc->masks.tunnel.tp_src = 0;
         wc->masks.tunnel.tp_dst = 0;
 
+        /* GTP-U header are set to be always unwildcarded for GTP-U packets*/
+        if (flow->tunnel.gtpu_flags || flow->tunnel.gtpu_msgtype) {
+            wc->masks.tunnel.gtpu_flags = UINT8_MAX;
+            wc->masks.tunnel.gtpu_msgtype = UINT8_MAX;
+        }
+
         if (is_ip_any(flow)
             && IP_ECN_is_ce(flow->tunnel.ip_tos)) {
             wc->masks.nw_tos |= IP_ECN_MASK;