Project

General

Profile

Actions

Bug #6123

closed

IPv4/IPv6 address family has to be the same for inner layer and outer layer

Added by osmith 7 months ago. Updated 4 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Target version:
-
Start date:
07/31/2023
Due date:
% Done:

100%

Spec Reference:

Description

While implementing support for IPv6 in OsmoGGSN, I've noticed that both libgtpnl and the kernel module only support having the same address family for both the inner and outer layer:

But for OsmoGGSN it is required that these are independent, it must be possible to have v4-in-v6, v6-in-v6, v4-in-v4 and v6-in-v4.

pablo: am I misunderstanding something, or is this a limitation in the kernel module?


Files


Related issues

Related to Linux Kernel GTP-U - Bug #1952: IPv6 support for inner (user) IP layer missingIn Progresspablo02/18/2017

Actions
Related to Linux Kernel GTP-U - Bug #1953: IPv6 support for outer (transport) IP layer missingResolved02/18/2017

Actions
Blocks OsmoGGSN (former OpenGGSN) - Feature #6096: add support for kernel-GTP IPv6In Progressosmith07/12/2023

Actions
Actions #1

Updated by osmith 7 months ago

Actions #2

Updated by laforge 7 months ago

Note that the point here is not just for osmo-ggsn, but in general, for real-world deployments. The most normal situation is that the outer addresses of the GSNs are v4, and the inner (MS/US address) is either v4 or v6.

Conceptually, there is no connection between inner address version to outer address version in GTP-U, so the implementation should also not introduce any constraints.

Actions #3

Updated by laforge 7 months ago

  • Related to Bug #1952: IPv6 support for inner (user) IP layer missing added
Actions #4

Updated by laforge 7 months ago

  • Related to Bug #1953: IPv6 support for outer (transport) IP layer missing added
Actions #5

Updated by pablo 6 months ago

Thanks for the feedback! And apologies for taking a while to reply.

This requires a bit of work to allow IPv4 over IPv6 and vice-versa. I'm working on this, I will keep you posted.

Actions #6

Updated by laforge 6 months ago

  • Status changed from New to In Progress
Actions #7

Updated by pablo 5 months ago

This series adds support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP:

gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP
gtp: add helper function to build GTP packets from an IPv6 packet
gtp: add helper function to build GTP packets from an IPv4 packet
gtp: remove IPv4 and IPv6 header from context object
gtp: move debugging to skbuff build helper function

This batch is accessible through:

https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/

I am also attaching the test scripts for containers I have used for this.

Tested with ICMP echo/request and TCP traffic generated with iperf, packet traces reviewed with wireshark.

Actions #9

Updated by osmith 5 months ago

Thank you very much, pablo!

Could you push your libgtpnl tree somewhere? (The libgtpnl patches don't apply on master, and the referenced commit in the first patch is not in the tree.)

Actions #10

Updated by osmith 5 months ago

nvm, the patches applied, after applying https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/commit/e0867c81a374c8cdf05fa7587fe357854be4c79e "update to prepare for IPv6 support" first.

Actions #11

Updated by osmith 5 months ago

I've made a change to the "add IPv6 support" patch: gtp_tunnel_alloc() needs to set t->family = AF_INET for backwards compatibility.

Submitted your patches as:

Actions #12

Updated by pablo 5 months ago

Thanks for submitting my patches via gerrit!

Kbuild robot reported an issue today for the gtp.git kernel tree:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git main
head:   230b7c1e9db7fcb2b796845017ce64f35e7430f1
commit: 131a295c85342beb299bbd4dcee11f732fa0d2c3 [9/11] gtp: add helper function to build GTP packets from an IPv4 packet
config: i386-randconfig-062-20231014 (https://download.01.org/0day-ci/archive/20231014/202310142352.LUjQLXaW-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310142352.LUjQLXaW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310142352.LUjQLXaW-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/gtp.c:1011:12: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16
 [usertype] df @@     got unsigned short [usertype] frag_off @@
   drivers/net/gtp.c:1011:12: sparse:     expected restricted __be16 [usertype] df
   drivers/net/gtp.c:1011:12: sparse:     got unsigned short [usertype] frag_off
>> drivers/net/gtp.c:1029:24: sparse: sparse: restricted __be16 degrades to integer
>> drivers/net/gtp.c:1073:48: sparse: sparse: incorrect type in argument 6 (different base types) @@     expected unsigned short
 [usertype] frag_off @@     got restricted __be16 [usertype] frag_off @@
   drivers/net/gtp.c:1073:48: sparse:     expected unsigned short [usertype] frag_off
   drivers/net/gtp.c:1073:48: sparse:     got restricted __be16 [usertype] frag_off
   drivers/net/gtp.c:1211:45: sparse: sparse: incorrect type in argument 9 (different base types) @@     expected restricted __be32
 [usertype] label @@     got unsigned char [addressable] [usertype] tos @@
   drivers/net/gtp.c:1211:45: sparse:     expected restricted __be32 [usertype] label
   drivers/net/gtp.c:1211:45: sparse:     got unsigned char [addressable] [usertype] tos

   985  static int __gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
   986                                 struct gtp_pktinfo *pktinfo,
   987                                 struct pdp_ctx *pctx, __u8 tos, __u16 frag_off)
   988  {
   989          struct rtable *rt;
   990          struct flowi4 fl4;
   991          __be16 df;
   992          int mtu;
   993
   994          rt = ip4_route_output_gtp(&fl4, pctx->sk, pctx->ip4.peer_addr.s_addr,
   995                                    inet_sk(pctx->sk)->inet_saddr);
   996          if (IS_ERR(rt)) {
   997                  netdev_dbg(dev, "no route to SSGN %pI4\n",
   998                             &pctx->ip4.peer_addr.s_addr);
   999                  dev->stats.tx_carrier_errors++;
  1000                  goto err;
  1001          }
  1002
  1003          if (rt->dst.dev == dev) {
  1004                  netdev_dbg(dev, "circular route to SSGN %pI4\n",
  1005                             &pctx->ip4.peer_addr.s_addr);
  1006                  dev->stats.collisions++;
  1007                  goto err_rt;
  1008          }
  1009
  1010          /* This is similar to tnl_update_pmtu(). */
> 1011          df = frag_off;

It is very easy to fix, I use __be16 frag_off instead.

kbuild robot reported also a second issue:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git main
head:   230b7c1e9db7fcb2b796845017ce64f35e7430f1
commit: 230b7c1e9db7fcb2b796845017ce64f35e7430f1 [11/11] gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP
config: i386-randconfig-062-20231014 (https://download.01.org/0day-ci/archive/20231015/202310150313.IPOOVXlp-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231015/202310150313.IPOOVXlp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310150313.IPOOVXlp-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/net/gtp.c:260:17: sparse: sparse: restricted __be16 degrades to integer
   drivers/net/gtp.c:260:17: sparse: sparse: restricted __be16 degrades to integer
>> drivers/net/gtp.c:281:30: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16
 [usertype] @@     got int @@
   drivers/net/gtp.c:281:30: sparse:     expected restricted __be16 [usertype]
   drivers/net/gtp.c:281:30: sparse:     got int
   drivers/net/gtp.c:284:30: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16
 [usertype] @@     got int @@
   drivers/net/gtp.c:284:30: sparse:     expected restricted __be16 [usertype]
   drivers/net/gtp.c:284:30: sparse:     got int
>> drivers/net/gtp.c:309:47: sparse: sparse: cast from restricted __be16
   drivers/net/gtp.c:1044:12: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __be16
 [usertype] df @@     got unsigned short [usertype] frag_off @@
   drivers/net/gtp.c:1044:12: sparse:     expected restricted __be16 [usertype] df
   drivers/net/gtp.c:1044:12: sparse:     got unsigned short [usertype] frag_off
   drivers/net/gtp.c:1062:24: sparse: sparse: restricted __be16 degrades to integer
   drivers/net/gtp.c:1166:56: sparse: sparse: incorrect type in argument 6 (different base types) @@     expected unsigned short
 [usertype] frag_off @@     got restricted __be16 [usertype] frag_off @@
   drivers/net/gtp.c:1166:56: sparse:     expected unsigned short [usertype] frag_off
   drivers/net/gtp.c:1166:56: sparse:     got restricted __be16 [usertype] frag_off
   drivers/net/gtp.c:1276:45: sparse: sparse: incorrect type in argument 9 (different base types) @@     expected restricted __be32
 [usertype] label @@     got unsigned char [addressable] [usertype] tos @@
   drivers/net/gtp.c:1276:45: sparse:     expected restricted __be32 [usertype] label
   drivers/net/gtp.c:1276:45: sparse:     got unsigned char [addressable] [usertype] tos

   269  static int gtp_inner_proto(struct sk_buff *skb, unsigned int hdrlen,
   270                             __be16 *inner_proto)
   271  {
   272          __u8 *ip_version, _ip_version;
   273
   274          ip_version = skb_header_pointer(skb, hdrlen, sizeof(ip_version),
   275                                          &_ip_version);
   276          if (!ip_version)
   277                  return -1;
   278
   279          switch (*ip_version & 0xf0) {
   280          case 0x40:
 > 281                  *inner_proto = ETH_P_IP;

Also easy to fix, use __u16 for inner_proto instead.

I have amended and forced a push to gtp.git for these two. Please, make sure you run a fresh clone of gtp.git kernel tree which include these fixes.

Thanks.

Actions #13

Updated by osmith 5 months ago

I have amended and forced a push to gtp.git for these two. Please, make sure you run a fresh clone of gtp.git kernel tree which include these fixes.

Thanks for the heads up, using your current version again.

Actions #14

Updated by osmith 4 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Closing, as this is implemented. See #1952 for follow-up discussion on IPv6 patches.

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)