On Thu, Dec 12, 2019 at 07:28:27AM +0000, Roni Bar Yanai wrote:
> Hi William,
> 
> I think the question here is what is the main target functionality?
> 3-5/G VNF switching?

I actually don't have a target use case. The original patch was from
Facebook and Intel, see:
https://lists.linuxfoundation.org/pipermail/ovs-dev/2018-May/346957.html
https://lists.linuxfoundation.org/pipermail/ovs-dev/2018-May/347029.html

Like other OVS tunnel supports, the patch allows OVS to
- create GTP-U tunnel dev
- decap/encap GTP-U 
- match on flags and msgtype

I don't have experience in deploying GTP-U, but I'd love to
know/learn from your use cases.
 
> Note that in many cases there is RSS issue here because GTP between
>  two  functions will have the exact same outer header, there is no
>  port entropy. All the packets between two peers will get to the same
>  core (per direction, according to RSS symmetric cfg).

I see. This looks like a performance issue.
Maybe we can improve it later.

> 
> See inline.
> 
> >-----Original Message-----
> >From: William Tu <[email protected]>
> >Sent: Wednesday, December 11, 2019 8:20 PM
> >To: Roni Bar Yanai <[email protected]>
> >Cc: [email protected]
> >Subject: Re: [ovs-dev] [PATCHv4] userspace: Add GTP-U support.
> >
> >On Sun, Dec 08, 2019 at 08:11:19AM +0000, Roni Bar Yanai wrote:
> >> Hi William,
> >>
> >> GTP-U header size is not constant, you *must* take into account the
> >> flags, mainly  the sequence. The use of sequence in GTP-U is optional
> >> but some devices do use  it.  see from 3GPP definition:
> >> "For PGW, SGW and eNodeB the usage of sequence numbers in G-PDUs is
> >> optional, but  if GTP-U protocol entities in these nodes are relaying
> >> G-PDUs to other nodes, they  shall relay the sequence numbers as well.
> >> An RNC, SGSN or GGSN shall reorder out  of sequence T-PDUs when in
> >sequence delivery is required. This is optional"
> >>
> >> The assumption that GTP-U is only data is not correct, you have some
> >> signaling for example END MARKER (like you wrote). This message is
> >> sent when there is a mobility  between cells, and the message contains
> >> a TEID and indicates that last packet sent from the origin cell, to
> >> prevent packet re-order as handover might have packets on the fly. So
> >> I would expected that END MARKER will do the exact same forwarding as the
> >GTP-U data.
> >>
> >> see inline
> >>
> >> >diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index
> >> >a78972888e33..7ae554c87964 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))
> >> This is the base header length, if S or N-PDU is on you have to add
> >> 4-bytes
> >> >+
> >> > uint16_t tnl_udp_port_min = 32768;
> >> > uint16_t tnl_udp_port_max = 61000;
> >> >
> >> >@@ -708,6 +711,88 @@ netdev_erspan_build_header(const struct netdev
> >> >*netdev,  }
> >> >
> >> > 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;
> >> >+
> >> >+    ovs_assert(packet->l3_ofs > 0);
> >> >+    ovs_assert(packet->l4_ofs > 0);
> >> >+
> >> >+    pkt_metadata_init_tnl(md);
> >> >+    if (GTPU_HLEN > dp_packet_l4_size(packet)) {
> >> >+        goto err;
> >> >+    }
> >> >+
> >> >+    gtph = udp_extract_tnl_md(packet, tnl, &hlen);
> >> >+    if (!gtph) {
> >> >+        goto err;
> >> >+    }
> >> >+
> >> >+    if (gtph->md.flags != GTPU_FLAGS_DEFAULT) {
> >> >+        VLOG_WARN_RL(&err_rl, "GTP-U not supported");
> >> >+        goto err;
> >> >+    }
> >> >+
> >> >+    tnl->gtpu_flags = gtph->md.flags;
> >> >+    tnl->gtpu_msgtype = gtph->md.msgtype;
> >> >+    tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gtph->teid)));
> >> >+
> >> >+    if (tnl->gtpu_msgtype == GTPU_MSGTYPE_GPDU) {
> >> >+        struct ip_header *ip;
> >> >+
> >> >+        ip = (struct ip_header *)(gtph + 1);
> >> No always correct, should be additional 4 bytes on sequence, n-pdu
> >> >+        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 {
> >> >+            VLOG_WARN_RL(&err_rl, "GTP-U: Receive non-IP packet");
> >> >+        }
> >> >+    } else {
> >> >+        /* non GPDU GTP-U messages, ex: echo request, end marker. */
> >> >+        packet->packet_type = htonl(PT_GTPU_MSG);
> >> >+    }
> >> >+
> >> >+    dp_packet_reset_packet(packet, hlen + GTPU_HLEN);
> >> Why do you want to reset on control such as end marker?
> >
> >Right, for non-GPDU, we shouldn't reset.
> >I will keep the original packet, no pop.
> >
> >> >+
> >> >+    return packet;
> >> >+
> >> >+err:
> >> >+    dp_packet_delete(packet);
> >> >+    return NULL;
> >> >+}
> >> >+
> >> >+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;
> >> >+
> >> >+    ovs_mutex_lock(&dev->mutex);
> >> >+    tnl_cfg = &dev->tnl_cfg;
> >> >+    gtph = udp_build_header(tnl_cfg, data, params);
> >> >+    ovs_mutex_unlock(&dev->mutex);
> >> >+
> >> >+    /* Set to default if not set in flow. */
> >> >+    gtph->md.flags = params->flow->tunnel.gtpu_flags ? :
> >GTPU_FLAGS_DEFAULT;
> >> >+    gtph->md.msgtype = params->flow->tunnel.gtpu_msgtype ? :
> >> >+                       GTPU_MSGTYPE_GPDU;
> >> >+    put_16aligned_be32(&gtph->teid,
> >> >+                       htonl(ntohll(params->flow->tunnel.tun_id)));
> >> >+
> >> >+    data->header_len += sizeof *gtph;
> >> >+    data->tnl_type = OVS_VPORT_TYPE_GTPU;
> >> Note that in some cases, application might need to replace TEID , if
> >> you have tnl_push you need to preserve the origin sequence.
> >What use case are you talking about here?
> >Are you saying that OVS receive a GTP-U packet, decap it, preserve its 
> >sequence
> >number, and send to anther GTPU tunnel?
> >
> >For replacing TEID, change the tun_id will do the work.
> 
> What is the use case you target here?
> I guess for switching this part can be skipped it is more VNF functionality.
> There are points on data path like p-gw that tunnel TEID changes because the
> allocation is done by the two peers of the connection and it depends on their
> available tunnels. So packet will arrive on one tunnel and leave for next 
> function
> with a different tunnel. If this is the use case and GTP header has a 
> sequence, the
> sequence should remain the same.

I see. So in this case, I image OVS works like this

0) create gtpu0 (teid=10) and gtpu1 (teid=11) tunnel ports
   and GTP-U packets arrives at an port
1) OVS decaps the rx packet, forwards to gtpu0 dev
2) controller adds an openflow rule
   in_port=gtpu0 actions=output:gtpu1
3) gtpu1 gets inner packet, and encap it with outer header
   with new teid, and tx 

However, the sequence number will not remain the same.
William

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to