Hi Pravin,

On 23/01/2021 20:59, Jonas Bonn wrote:
From: Pravin B Shelar <pbshe...@fb.com>

@@ -617,29 +686,84 @@ static void gtp_push_header(struct sk_buff *skb, struct 
pdp_ctx *pctx,
  static int gtp_xmit_ip4(struct sk_buff *skb, struct net_device *dev)
  {
        struct gtp_dev *gtp = netdev_priv(dev);
+       struct gtpu_metadata *opts = NULL;
+       struct pdp_ctx md_pctx;
        struct pdp_ctx *pctx;
+       __be16 port;
        struct rtable *rt;
-       __be32 saddr;
        struct iphdr *iph;
+       __be32 saddr;
        int headroom;
-       __be16 port;
+       __u8 tos;
        int r;
- /* Read the IP destination address and resolve the PDP context.
-        * Prepend PDP header with TEI/TID from PDP ctx.
-        */
-       iph = ip_hdr(skb);
-       if (gtp->role == GTP_ROLE_SGSN)
-               pctx = ipv4_pdp_find(gtp, iph->saddr);
-       else
-               pctx = ipv4_pdp_find(gtp, iph->daddr);
+       if (gtp->collect_md) {

Why do we have this restriction that the device be exclusively "collect metadata" mode or PDP context mode? Why are we not able to mix the two?

Furthermore, since the collect_md_sock will effectively always be listening on INADDR_ANY, that precludes any other PDP context devices from co-existing with it. So setting up a secondary device for PDP contexts isn't a feasible workaround.

If mixing isn't possible, then I suppose PDP context management needs to be made to fail gracefully in "collect_md" mode... with the current patches I think that contexts can be set up but they are just silently ignored, which seems like a potential source of confusion.

/Jonas


+               /* LWT GTP1U encap */
+               struct ip_tunnel_info *info = NULL;
- if (!pctx) {
-               netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
-                          &iph->daddr);
-               return -ENOENT;
+               info = skb_tunnel_info(skb);
+               if (!info) {
+                       netdev_dbg(dev, "missing tunnel info");
+                       return -ENOENT;
+               }
+               if (info->key.tp_dst && ntohs(info->key.tp_dst) != GTP1U_PORT) {
+                       netdev_dbg(dev, "unexpected GTP dst port: %d", 
ntohs(info->key.tp_dst));
+                       return -EOPNOTSUPP;
+               }
+
+               if (!gtp->sk1u) {
+                       netdev_dbg(dev, "missing tunnel sock");
+                       return -EOPNOTSUPP;
+               }
+
+               pctx = &md_pctx;
+               memset(pctx, 0, sizeof(*pctx));
+               pctx->sk = gtp->sk1u;
+               pctx->gtp_version = GTP_V1;
+               pctx->u.v1.o_tei = ntohl(tunnel_id_to_key32(info->key.tun_id));
+               pctx->peer_addr_ip4.s_addr = info->key.u.ipv4.dst;
+
+               saddr = info->key.u.ipv4.src;
+               tos = info->key.tos;
+
+               if (info->options_len != 0) {
+                       if (info->key.tun_flags & TUNNEL_GTPU_OPT) {
+                               opts = ip_tunnel_info_opts(info);
+                       } else {
+                               netdev_dbg(dev, "missing tunnel metadata for control 
pkt");
+                               return -EOPNOTSUPP;
+                       }
+               }
+               netdev_dbg(dev, "flow-based GTP1U encap: tunnel id %d\n",
+                          pctx->u.v1.o_tei);
+       } else {
+               struct iphdr *iph;
+
+               if (ntohs(skb->protocol) != ETH_P_IP)
+                       return -EOPNOTSUPP;
+
+               iph = ip_hdr(skb);
+
+               /* Read the IP destination address and resolve the PDP context.
+                * Prepend PDP header with TEI/TID from PDP ctx.
+                */
+               if (gtp->role == GTP_ROLE_SGSN)
+                       pctx = ipv4_pdp_find(gtp, iph->saddr);
+               else
+                       pctx = ipv4_pdp_find(gtp, iph->daddr);
+
+               if (!pctx) {
+                       netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
+                                  &iph->daddr);
+                       return -ENOENT;
+               }
+               netdev_dbg(dev, "found PDP context %p\n", pctx);
+
+               saddr = inet_sk(pctx->sk)->inet_saddr;
+               tos = iph->tos;
+               netdev_dbg(dev, "gtp -> IP src: %pI4 dst: %pI4\n",
+                          &iph->saddr, &iph->daddr);
        }
-       netdev_dbg(dev, "found PDP context %p\n", pctx);
rt = gtp_get_v4_rt(skb, dev, pctx, &saddr);
        if (IS_ERR(rt)) {
@@ -691,7 +815,7 @@ static int gtp_xmit_ip4(struct sk_buff *skb, struct 
net_device *dev)
udp_tunnel_xmit_skb(rt, pctx->sk, skb,
                            saddr, pctx->peer_addr_ip4.s_addr,
-                           iph->tos,
+                           tos,
                            ip4_dst_hoplimit(&rt->dst),
                            0,
                            port, port,
diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h
index 79f9191bbb24..62aff78b7c56 100644
--- a/include/uapi/linux/gtp.h
+++ b/include/uapi/linux/gtp.h
@@ -2,6 +2,8 @@
  #ifndef _UAPI_LINUX_GTP_H_
  #define _UAPI_LINUX_GTP_H_
+#include <linux/types.h>
+
  #define GTP_GENL_MCGRP_NAME   "gtp"
enum gtp_genl_cmds {
@@ -34,4 +36,14 @@ enum gtp_attrs {
  };
  #define GTPA_MAX (__GTPA_MAX + 1)
+enum {
+       GTP_METADATA_V1
+};
+
+struct gtpu_metadata {
+       __u8    ver;
+       __u8    flags;
+       __u8    type;
+};
+
  #endif /* _UAPI_LINUX_GTP_H_ */
diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
index 7d9105533c7b..802da679fab1 100644
--- a/include/uapi/linux/if_tunnel.h
+++ b/include/uapi/linux/if_tunnel.h
@@ -176,6 +176,7 @@ enum {
  #define TUNNEL_VXLAN_OPT      __cpu_to_be16(0x1000)
  #define TUNNEL_NOCACHE                __cpu_to_be16(0x2000)
  #define TUNNEL_ERSPAN_OPT     __cpu_to_be16(0x4000)
+#define TUNNEL_GTPU_OPT                __cpu_to_be16(0x8000)
#define TUNNEL_OPTIONS_PRESENT \
                (TUNNEL_GENEVE_OPT | TUNNEL_VXLAN_OPT | TUNNEL_ERSPAN_OPT)
diff --git a/tools/include/uapi/linux/if_link.h 
b/tools/include/uapi/linux/if_link.h
index d208b2af697f..28d649bda686 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -617,6 +617,7 @@ enum {
        IFLA_GTP_FD1,
        IFLA_GTP_PDP_HASHSIZE,
        IFLA_GTP_ROLE,
+       IFLA_GTP_COLLECT_METADATA,
        __IFLA_GTP_MAX,
  };
  #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1)

Reply via email to