Cellular Network Infrastructure » Core Network (CN) » OsmoGGSN (former OpenGGSN) » Linux Kernel GTP-U

kbuild robot reported via sparse tool:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git main
head:   230b7c1e9db7fcb2b796845017ce64f35e7430f1
commit: cb1bf181ddcc4a7f28deb5cde00e14ed37b7c3d9 [8/11] gtp: remove IPv4 and IPv6 header from context object
config: i386-randconfig-062-20231014 (https://download.01.org/0day-ci/archive/20231014/202310142009.vKJll0Uf-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/202310142009.vKJll0Uf-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/202310142009.vKJll0Uf-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/net/gtp.c:1197: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:1197:45: sparse:     expected restricted __be32 [usertype] label
   drivers/net/gtp.c:1197:45: sparse:     got unsigned char [addressable] [usertype] tos

Problem is here:

  1192  #if IS_ENABLED(CONFIG_IPV6)
  1193                  udp_tunnel6_xmit_skb(&pktinfo.rt6->dst, pktinfo.sk, skb, dev,
  1194                                       &pktinfo.fl6.saddr, &pktinfo.fl6.daddr,
  1195                                       0,
  1196                                       ip6_dst_hoplimit(&pktinfo.rt->dst),
> 1197                                       pktinfo.tos,
  1198                                       pktinfo.gtph_port, pktinfo.gtph_port,
  1199                                       false);
  1200  #else
  1201                  goto tx_err;
  1202  #endif
  1203                  break;
  1204          }
  1205
  1206          return NETDEV_TX_OK;
  1207  tx_err:
  1208          dev->stats.tx_errors++;
  1209          dev_kfree_skb(skb);
  1210          return NETDEV_TX_OK;
  1211  }

This is a real bug in ("gtp: add IPv6 support"), I accidentally swapped ToS and flowlabel. This is the fix I have collapsed:

  1193                  udp_tunnel6_xmit_skb(&pktinfo.rt6->dst, pktinfo.sk, skb, dev,
  1194                                       &pktinfo.fl6.saddr, &pktinfo.fl6.daddr,
> 1195                                       pktinfo.tos,
  1196                                       ip6_dst_hoplimit(&pktinfo.rt->dst),
~ 1197                                       0,
  1198                                       pktinfo.gtph_port, pktinfo.gtph_port,
  1199                                       false);

This is now fixed in gtp.git, I have rebased and forced a push, please make sure you work with a fresh gtp.git clone when testing, thanks.

Hi pablo,

an initial version of IPv6 support has been merged into libgtpnl (the patches had to be reworked to address points brought up in code review): https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/commit/a295615229747e938f14514da14aabd595d8d1b5

To better understand how libgtpnl APIs should be used from osmo-ggsn for type-support v4v6, and to make sure that it works as expected without using osmo-ggsn, I've started adding test cases to libgtpnl. The tests build a small initrd with gtp-tunnel, gtp-link, busybox and test shell scripts, and run it in QEMU (so it can run against a kernel built from source).

libgtpnl.git branch: osmith/qemu-tests at https://gerrit.osmocom.org/libgtpnl (browse, also on gitea, but it takes a bit to sync)

$ autoreconf -fi
$ ./configure --enable-qemu-tests
$ make check

Here are the current test results (see tests/qemu/initrd-init.sh):

run_test 01_ms_ip4_sgsn_ip4.sh  # OK
# run_test 02_ms_ip4_sgsn_ip6.sh  # NOK: kernel panic
# run_test 03_ms_ip6_sgsn_ip4.sh  # NOK: ping doesn't work
# run_test 04_ms_ip6_sgsn_ip6.sh  # NOK: ping doesn't work
# run_test 05_ms_ip46_sgsn_ip4.sh  # TODO

Could you take a look at the kernel panic (see below)? I'm running HEAD of https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/log/ (da3924c0, "gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP").

For tests 03 and 04, could you check if I'm using it wrong, or if there might be a bug in the kernel?

Thanks!

PS: I found that I probably need to add gtp_tunnel_set_family() again (which was in your patch) to be able to properly delete a tunnel (otherwise, it is not clear if the IPv4 or IPv6 variant of a tunnel with the same i_tei and o_tei and MS + SGSN addr should be deleted, if MS has both IPv4 and IPv6 as it would be the case with type-support v4v6 in osmo-ggsn as I understand). For adding the tunnel it seemed not necessary, because the family would always be the same one as for MS, as I understood from your patches. So I will add it back in a follow-up patch.

02_ms_ip4_sgsn_ip6.sh kernel panic02_ms_ip4_sgsn_ip6.sh kernel panic

[    0.669400] Run /init as init process
Running initrd-init.sh
+ export 'HOME=/root'
+ export 'LD_LIBRARY_PATH=/usr/local/lib'
+ export 'PATH=/usr/local/bin:/usr/bin:/bin:/sbin:/usr/local/sbin:/usr/sbin'
+ export 'TERM=screen'
+ /bin/busybox --install -s
+ hostname qemu
+ mount -t proc proc /proc
+ mount -t sysfs sys /sys
+ mknod /dev/null c 1 3
+ . /cmd.sh
+ export 'LD_LIBRARY_PATH=/home/user/code/osmo-dev/src/libgtpnl/src/.libs:'
+ set +x

QEMU test: 02_ms_ip4_sgsn_ip6.sh

+ . /tests/00_test_functions.sh
+ alias 'ip=/bin/ip'
+ alias 'ggsn_side=ip netns exec ggsn_side'
+ MS=172.99.0.1
+ MS_PREFLEN=32
+ SGSN=fd00::1
+ SGSN_PREFLEN=7
+ GGSN=fd00::2
+ WEBSERVER=172.99.0.2
+ tunnel_start
+ /bin/ip netns add ggsn_side
[    0.680914] ip (726) used greatest stack depth: 13600 bytes left
+ /bin/ip link add veth_sgsn type veth peer name veth_ggsn
[    0.683186] ip (732) used greatest stack depth: 12656 bytes left
+ /bin/ip addr add fd00::1/7 dev veth_sgsn
+ /bin/ip link set veth_sgsn up
+ /bin/ip addr add 172.99.0.1/32 dev lo
+ /bin/ip link set lo up
+ sleep 1
+ gtp-link add gtp_sgsn --sgsn
WARNING: attaching dummy socket descriptors. Keep this process running for testing purposes.
[    1.080749] input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input3
+ gtp-tunnel add gtp_sgsn v1 200 100 172.99.0.1 fd00::2
+ /bin/ip route add 172.99.0.2/32 dev gtp_sgsn
+ /bin/ip link set veth_ggsn netns ggsn_side
+ /bin/ip netns exec ggsn_side ip addr add fd00::2/7 dev veth_ggsn
+ /bin/ip netns exec ggsn_side ip link set veth_ggsn up
+ /bin/ip netns exec ggsn_side ip addr add 172.99.0.2/32 dev lo
+ /bin/ip netns exec ggsn_side ip link set lo up
+ sleep 1
+ /bin/ip netns exec ggsn_side gtp-link add gtp_ggsn
WARNING: attaching dummy socket descriptors. Keep this process running for testing purposes.
+ /bin/ip netns exec ggsn_side gtp-tunnel add gtp_ggsn v1 100 200 172.99.0.1 fd00::1
+ /bin/ip netns exec ggsn_side ip route add 172.99.0.1/32 dev gtp_ggsn
+ gtp-tunnel list
version 1 tei 200/100 ms_addr 172.99.0.1 sgsn_addr fd00::2
+ /bin/ip netns exec ggsn_side gtp-tunnel list
version 1 tei 100/200 ms_addr 172.99.0.1 sgsn_addr fd00::1
+ tunnel_ping
+ ping -c 1 172.99.0.2
PING 172.99.0.2 (172.99.0.2): 56 data bytes
[    2.760129] BUG: kernel NULL pointer dereference, address: 0000000000000008
[    2.762133] #PF: supervisor read access in kernel mode
[    2.763597] #PF: error_code(0x0000) - not-present page
[    2.765064] PGD 4e3b067 P4D 4e3b067 PUD 4748067 PMD 0 
[    2.766548] Oops: 0000 [#1] PREEMPT SMP NOPTI
[    2.767808] CPU: 0 PID: 768 Comm: ping Not tainted 6.6.0-rc2-00276-gda3924c06688 #4
[    2.769985] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
[    2.772337] RIP: 0010:__gtp_build_skb_ip6+0x7f/0x290
[    2.773773] Code: 48 8d 54 24 08 31 c0 b9 0c 00 00 00 4d 8b 57 48 48 89 d7 4d 8b 5f 50 f3 48 ab 4c 89 ef 8b 46 14 4c 89 54 24 38 4c 89 5c 24 40 <4d> 8b 48 08 89 44 24 08 0f b7 86 04 02 00 00 4d 8b 00 4c 89 4c 24
[    2.779021] RSP: 0018:ffffc900001679a8 EFLAGS: 00010246
[    2.780515] RAX: 0000000000000000 RBX: ffff88800485b000 RCX: 0000000000000000
[    2.782547] RDX: ffffc900001679b0 RSI: ffff8880049f5680 RDI: ffffffff8350b1c0
[    2.784574] RBP: ffffc90000167a40 R08: 0000000000000000 R09: 0000000000000000
[    2.786610] R10: 00000000000000fd R11: 0200000000000000 R12: ffffc90000167a50
[    2.788640] R13: ffffffff8350b1c0 R14: ffff888004d9e100 R15: ffff888004198480
[    2.790680] FS:  0000000001a073c0(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
[    2.792976] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    2.794626] CR2: 0000000000000008 CR3: 0000000004f9a000 CR4: 00000000000006f0
[    2.796644] Call Trace:
[    2.797362]  <TASK>
[    2.798008]  ? __die+0x1f/0x70
[    2.798900]  ? page_fault_oops+0x156/0x420
[    2.800075]  ? search_exception_tables+0x37/0x50
[    2.801399]  ? fixup_exception+0x21/0x310
[    2.802561]  ? exc_page_fault+0x69/0x150
[    2.803688]  ? asm_exc_page_fault+0x26/0x30
[    2.804885]  ? __gtp_build_skb_ip6+0x7f/0x290
[    2.806156]  gtp_dev_xmit+0x288/0x390
[    2.807210]  ? crng_make_state+0xf0/0x120
[    2.808365]  dev_hard_start_xmit+0x8a/0x1d0
[    2.809566]  __dev_queue_xmit+0x7fd/0xcf0
[    2.810675]  ip_finish_output2+0x17b/0x4f0
[    2.811703]  ? __pfx_ip_finish_output+0x10/0x10
[    2.812846]  ip_push_pending_frames+0xa1/0xb0
[    2.813963]  raw_sendmsg+0x950/0xd10
[    2.814531]  ? asm_common_interrupt+0x26/0x40
[    2.814941]  ? avc_alloc_node+0x1f/0x160
[    2.815303]  ? selinux_sk_alloc_security+0x41/0x80
[    2.815748]  ? __kmem_cache_alloc_node+0x104/0x1f0
[    2.816192]  ? selinux_netlbl_sock_genattr+0x3a/0x100
[    2.816663]  ? kmem_cache_alloc+0xf0/0x240
[    2.817043]  ? __sys_sendto+0x1d7/0x210
[    2.817398]  __sys_sendto+0x1d7/0x210
[    2.817742]  ? kvm_clock_get_cycles+0x18/0x30
[    2.818147]  ? ktime_get_ts64+0x4b/0xf0
[    2.818510]  ? _copy_to_user+0x24/0x40
[    2.818861]  ? put_timespec64+0x39/0x60
[    2.819218]  __x64_sys_sendto+0x1f/0x30
[    2.819577]  do_syscall_64+0x3f/0x90
[    2.819913]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
[    2.820382] RIP: 0033:0x48b44a
[    2.820672] Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 41 89 ca 64 8b 04 25 18 00 00 00 85 c0 75 15 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 7e c3 0f 1f 44 00 00 41 54 48 83 ec 30 44 89
[    2.822368] RSP: 002b:00007fff74f83108 EFLAGS: 00000246 ORIG_RAX: 000000000000002c
[    2.823059] RAX: ffffffffffffffda RBX: 0000000000000040 RCX: 000000000048b44a
[    2.823710] RDX: 0000000000000040 RSI: 0000000001a09960 RDI: 0000000000000000
[    2.824366] RBP: 000000000053f20b R08: 000000000061c328 R09: 000000000000001c
[    2.825021] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000001a08844
[    2.825681] R13: 00007fff74f83230 R14: 000000000053df8b R15: 0000000000000000
[    2.826337]  </TASK>
[    2.826546] Modules linked in:
[    2.826837] CR2: 0000000000000008
[    2.827147] ---[ end trace 0000000000000000 ]---
[    2.827578] RIP: 0010:__gtp_build_skb_ip6+0x7f/0x290
[    2.828039] Code: 48 8d 54 24 08 31 c0 b9 0c 00 00 00 4d 8b 57 48 48 89 d7 4d 8b 5f 50 f3 48 ab 4c 89 ef 8b 46 14 4c 89 54 24 38 4c 89 5c 24 40 <4d> 8b 48 08 89 44 24 08 0f b7 86 04 02 00 00 4d 8b 00 4c 89 4c 24
[    2.829736] RSP: 0018:ffffc900001679a8 EFLAGS: 00010246
[    2.830218] RAX: 0000000000000000 RBX: ffff88800485b000 RCX: 0000000000000000
[    2.830873] RDX: ffffc900001679b0 RSI: ffff8880049f5680 RDI: ffffffff8350b1c0
[    2.831528] RBP: ffffc90000167a40 R08: 0000000000000000 R09: 0000000000000000
[    2.832184] R10: 00000000000000fd R11: 0200000000000000 R12: ffffc90000167a50
[    2.832840] R13: ffffffff8350b1c0 R14: ffff888004d9e100 R15: ffff888004198480
[    2.833498] FS:  0000000001a073c0(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
[    2.834240] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    2.834771] CR2: 0000000000000008 CR3: 0000000004f9a000 CR4: 00000000000006f0
[    2.835426] Kernel panic - not syncing: Fatal exception in interrupt
[    2.836115] Kernel Offset: disabled

osmith wrote in #note-6:

Hi pablo,

an initial version of IPv6 support has been merged into libgtpnl (the patches had to be reworked to address points brought up in code review): https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/commit/a295615229747e938f14514da14aabd595d8d1b5

Thank you.

It feels a bit strange that you are taking my patches you mangled original author? Looking at the repository, they look as they were done by you, why didn't you use 'git am' to apply patches?

Could you also explain what you mangled from the original patches? Otherwise I have to review all this. Why not simply apply incremental patches on top instead of better traceability?

Sorry, this is not nice.

To better understand how libgtpnl APIs should be used from osmo-ggsn for type-support v4v6, and to make sure that it works as expected without using osmo-ggsn, I've started adding test cases to libgtpnl. The tests build a small initrd with gtp-tunnel, gtp-link, busybox and test shell scripts, and run it in QEMU (so it can run against a kernel built from source).

libgtpnl.git branch: osmith/qemu-tests at https://gerrit.osmocom.org/libgtpnl (browse, also on gitea, but it takes a bit to sync)

[...]

Here are the current test results (see tests/qemu/initrd-init.sh):

[...]

Could you take a look at the kernel panic (see below)? I'm running HEAD of https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/log/ (da3924c0, "gtp: support for IPv4-in-IPv6-GTP and IPv6-in-IPv4-GTP").

For tests 03 and 04, could you check if I'm using it wrong, or if there might be a bug in the kernel?

Thanks!

PS: I found that I probably need to add gtp_tunnel_set_family() again (which was in your patch) to be able to properly delete a tunnel (otherwise, it is not clear if the IPv4 or IPv6 variant of a tunnel with the same i_tei and o_tei and MS + SGSN addr should be deleted, if MS has both IPv4 and IPv6 as it would be the case with type-support v4v6 in osmo-ggsn as I understand). For adding the tunnel it seemed not necessary, because the family would always be the same one as for MS, as I understood from your patches. So I will add it back in a follow-up patch.

I will have to review what you apply to master and compare it to my local branch to review what is going on.

I have to admit I did not test delete tunnel path with my scripts.

{{collapse(02_ms_ip4_sgsn_ip6.sh kernel panic)
[...]
}}

I will take a look at the kernel again, I really tested datapath, this very much means nothing is working which is strange. Maybe I have break anything with my little fixes to make kbuild robot happy. I will come back to you with a more detailed assesment on what is going on.

I will start reviewing with my scripts with containers, it is a lot more lightweight than this qemu environment you have created and they do not require osmo-ggsn, I would have suggested to pick up to those I sent, they are also much easier to integrate into kernel/selftests infrastructure upstream. Well, we can also end up with two test infrastructures, one with qemu and another with kernel/selftests using containers, but I really think we really need kernel/selftests for this submission.

Apologies if my feedback is not pleasant to read, but I have to say a few things I don't like.

Thanks for your work.

I suggest you revert what you applied and we start from scratch, apply my patches and then you incrementally make patches on top, writing a description on your intention and rationale, otherwise it is really very hard to track down things. Then I have a chance to discuss with you if what you apply makes also sense to me.

I can see you have also squashed my preparation patches, I really like to split initial preparation patches that make no functional changes, before adding new features. All that was really intentional on my side, it is really the way I have been working for two decades in upstream projects.

I am now looking at the kernel issue you reported, I will keep using my local libgtnl library by now for this kernel testing.

Apologies, previous comment was not very constructive, let's find a way of working that allows us to scrutinize each other work.

pablo wrote in #note-7:

It feels a bit strange that you are taking my patches you mangled original author? Looking at the repository, they look as they were done by you, why didn't you use 'git am' to apply patches?

Hello Pablo,

I'm sorry, it was not my intention to upset you with this. I started modifying the patches as they were, adding descriptions of changes I made to each of them, but then it became apparent that I had to rewrite a lot to make them fit what was suggested in code review. I started modifying the patches as they were, using "struct gtp_addr" as requested in code review, and iterating through all the patches, resolving all the conflicts. But it became apparent that the original patches implement two versions, first one where v4-in-v6 and vice versa does not work, and later the implementation gets changed to make it work. After spending a lot of time changing the patches as they were, it seemed to me to make more sense to just squash them together since in the end we only need one implementation. I spent many hours working on the patches, and in the end the resulting patch is very different from the original patches. I changed the primary author to indicate that this is not just a small fixup but rather a rewrite of the patches, but mentioned in the commit message that this is based on your patches, and added you as Co-developed-by.

I thought I was supposed to take care of getting the patches into libgtpnl and change them as needed, and that this would be a valid way to do it. In any case, I'm sure we can figure this out, and if needed, force push patches to master that changes the author back or change the patches as needed.

Could you also explain what you mangled from the original patches? Otherwise I have to review all this. Why not simply apply incremental patches on top instead of better traceability?

• using struct gtp_addr
• no extra ip4/ip6 arguments in gtp-tunnel, instead it is implict from the format of the address (if it can be parsed as ipv4, it is an ipv4)
• removed gtp_tunnel_set_family as mentioned (but this needs to be brought back, see the "PS" above)

Please do read the patch in its current form, it is not long.

I have to admit I did not test delete tunnel path with my scripts.

Where are your test scripts? (I was not aware that they exist.)

I will start reviewing with my scripts with containers, it is a lot more lightweight than this qemu environment you have created and they do not require osmo-ggsn, I would have suggested to pick up to those I sent, they are also much easier to integrate into kernel/selftests infrastructure upstream. Well, we can also end up with two test infrastructures, one with qemu and another with kernel/selftests using containers, but I really think we really need kernel/selftests for this submission.

The test scripts I wrote don't use osmo-ggsn either, it just tests libgtpnl with gtp-tunnel.

In addition to that there are ttcn3 tests that do use osmo-ggsn, but that is not what I mentioned above.

Apologies if my feedback is not pleasant to read, but I have to say a few things I don't like.

Thanks for your work.

Thank you too!

Hi Oliver,

On Fri, Oct 27, 2023 at 02:24:34PM +0000, osmith wrote:

I thought I was supposed to take care of getting the patches into libgtpnl and change them as needed, and that this would be a valid way to do it. In any case, I'm sure we can figure this out, and if needed, force push patches to master that changes the author back or change the patches as needed.

The primary task from my point of view was to perform the testing. The
fact that pablo send diff files by e-mail and didn't follow the (to us
normal) process for code review in gerrit directly could have been
understood in a way to transfer responsibility to you, basically "take
these patches and bring them through code review and upstream, I don't
want to have to deal with gerrit".

In the end, what I as Osmocom project lead and sysmocom evil dictator
care about is that things get done. It is generally my impressin that
Pablo is often busy with other and probably more important tasks (in
terms of netfilter, etc.) and is generally rather remote from what we're
doing in Osmocom (in terms of tools used, code review, etc). That's
fine, and I think that the sysmocom team should do as much as we can to
allow Pablo to focus on those parts that really are in his direct line
of expertise which is the Linux kernel networking code.

I also agree with osmith's rationale to squash the "v6 / v4 separately"
and later change that to support mixed v4/v6. That's an arbitrary
distinction which resulted from the development history. However, that
"v4/v6 separate" is not the way GTP is ever used, so it's an arbitrary
intermediate step, and it's cleaner to introduce it properly at once.

osmith wrote in #note-12:

Thanks, Harald.

Back to technical discussions:

pablo wrote in #note-10:

Where are your test scripts? (I was not aware that they exist.)

Attached. They require two patches for iproute2, which allows for standalone GTP tunnels (with no osmo-ggsn).

Thanks for attaching the tests, I've tried them out. Setting up the GTP tunnels works with them, but attempting to run iperf3 as in the commented lines does not work for me ("unable to send control message: Bad file descriptor", looks like the IPs there need to be adjusted and routes need to be added? I've tried it for a bit but left it for now).

Regarding the test that caused a kernel panic, it can also be executed without qemu:

• https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/osmith/qemu-tests/tests/qemu/02_ms_ip4_sgsn_ip6.sh
• https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/osmith/qemu-tests/tests/qemu/00_test_functions.sh

gtp-link also needs an update to set up a IPv6 socket for the gtpX device.

This patch would avoid a crash from control plane.

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 2d2f24eb05ca..8698a00708ec 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1780,6 +1780,9 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
                    info->attrs[GTPA_MS_ADDR6])
                        return ERR_PTR(-EINVAL);

+               if (sk->sk_family != AF_INET)
+                       return ERR_PTR(-EAFNOSUPPORT);
+
                ms_addr = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
                hash_ms = ipv4_hashfn(ms_addr) % gtp->hash_size;
                pctx = ipv4_pdp_find(gtp, ms_addr);
@@ -1789,6 +1792,9 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
                    info->attrs[GTPA_MS_ADDRESS])
                        return ERR_PTR(-EINVAL);

+               if (sk->sk_family != AF_INET6)
+                       return ERR_PTR(-EAFNOSUPPORT);
+
                ms_addr6 = nla_get_in6_addr(info->attrs[GTPA_MS_ADDR6]);
                hash_ms = ipv6_hashfn(&ms_addr6) % gtp->hash_size;
                pctx = ipv6_pdp_find(gtp, &ms_addr6);

Actually, fix is this to reject adding a peer address which has different socket family:

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 2d2f24eb05ca..0147939dbd6e 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1826,6 +1826,8 @@ static struct pdp_ctx *gtp_pdp_add(struct gtp_dev *gtp, struct sock *sk,
                        ipv6_pdp_fill(pctx, info);
                        break;
                }
+               if (pctx->peer_af != sk->sk_family)
+                       return ERR_PTR(-EAFNOSUPPORT);

                if (pctx->gtp_version == GTP_V0)
                        netdev_dbg(dev, "GTPv0-U: update tunnel id = %llx (pdp %p)\n",

pablo wrote in #note-13:

gtp-link also needs an update to set up a IPv6 socket for the gtpX device.

Thanks for explaining.

This patch would avoid a crash from control plane.

[...]

At this stage, the gtp device can either have an IPv4 address or IPv6 address, ie. two GTP devices would be needed, one for each outer protocol family. In other words, in an environment with both GTP over IPv4 and GTP over IPv6 (regardless the inner protocol family network header), two GTP devices are required.

Is there a need for a single device both GTP IPv4 and IPv6 sockets?

I looked quickly over the related osmo-ggsn code, and it looks like it should be fine to have a different device for IPv4 and IPv6.

But laforge is the expert, maybe he has an important reason why it should use the same device.

After this patch I am proposing, if gtp0 has an IPv6 socket, then this fails:

gtp-tunnel add gtp0 v1 1 1 ip 10.141.10.2 ip 192.168.10.11

because peer address needs to be IPv6.

Sounds good to me.

pablo: as 03_ms_ip6_sgsn_ip4.sh is very similar to 02_ms_ip4_sgsn_ip6.sh, do you happen to see why I wasn't able to ping in 03_ms_ip6_sgsn_ip4.sh? https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/osmith/qemu-tests/tests/qemu/03_ms_ip6_sgsn_ip4.sh

On Tue, Oct 31, 2023 at 10:48:15AM +0000, osmith wrote:

I looked quickly over the related osmo-ggsn code, and it looks like it should be fine to have a different tun device for IPv4 and IPv6.

well, both my and your response are technically correct. The 3GPP specs say nothing about "gtp net-devices".

The configuration we must support is a single tunnel (TEID) that carries both v4 and v6 inner IP packets.
Normally, one would expect those to be emerging from the same net-device upon decapsulation. However, if
it simplified things on the implementation side, one could of course let those packets appear on separate
net-devices. The key requirement is that a single tunnel must be able to carry both v4 and v6.

Regards,
Harald

Hi Pablo,

On Tue, Oct 31, 2023 at 10:24:36AM +0000, pablo wrote:

At this stage, the gtp device can either have an IPv4 address or IPv6 address, ie. two GTP devices would be needed, one for each outer protocol family. In other words, in an environment with both GTP over IPv4 and GTP over IPv6 (regardless the inner protocol family network header), two GTP devices are required.

Is there a need for a single device both GTP IPv4 and IPv6 sockets?

From the 3GPP usage point of view, there are the following three options, all of which are deployed in
production for many years:

• IPv4-only APN
  • The tunnel gets allocated a single IPv4 address only
  • All user-plane IP packets are IPv4
• IPv6-only APN
  • The tunnel gets allocated and IPv6 prefix (/64) only
  • All user-plane IP packets are IPv6
• IPv4v6 APN
  • The tunnel gets allocated both an IPv4 address and and IPv6 prefix (/64)
  • user-plane IP packets are alternating v4 and v6. Basically just like any normal dual-stacked host on the
    internet today
  • this is actually the default bearer type since the introduction of LTE in 3GPP Release 8 in 2010

So I think the answer is yes, as single GTP device / socket must support a combination of both AF as inner
protocol.

See also https://www.ietf.org/proceedings/77/slides/v6ops-10.ppt for a high-level intro that was apparently
presented at an IETF meeting long ago

osmith wrote in #note-15:

pablo: as 03_ms_ip6_sgsn_ip4.sh is very similar to 02_ms_ip4_sgsn_ip6.sh, do you happen to see why I wasn't able to ping in 03_ms_ip6_sgsn_ip4.sh? https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/osmith/qemu-tests/tests/qemu/03_ms_ip6_sgsn_ip4.sh

I suspect it is the same issue, showing up in a different way.

static int __gtp_build_skb_ip6(struct net *net, struct sk_buff *skb,             
                               struct net_device *dev,                           
                               struct gtp_pktinfo *pktinfo,                      
                               struct pdp_ctx *pctx, __u8 tos)                   
{                                                                                
        struct dst_entry *dst;                                                   
        struct rt6_info *rt;                                                     
        struct flowi6 fl6;                                                       
        int mtu;                                                                 

        rt = ip6_route_output_gtp(net, &fl6, pctx->sk, &pctx->ip6.peer_addr,     
                                  &inet6_sk(pctx->sk)->saddr);   

Kernel drivers uses the socket address to look up for the route when building the IPv6 GTP packet, see inet6_sk(), so kernel assume an IPv6 socket is in place.

But gtp-link is setting an IPv4 socket from userspace.

I can try to reproduce it here.

laforge wrote in #note-18:

So I think the answer is yes, as single GTP device / socket must support a combination of both AF as inner
protocol.

OK, I am going to explore a way to attach two sockets from netlink interface POV.

On Tue, Oct 31, 2023 at 11:18:54AM +0000, pablo wrote:

OK, I am going to explore a way to attach two sockets from netlink interface POV.

sorry, it's been ages since I looked at this code in detail. But why would we need two different
sockets? Maybe we should schedule a quick call with osmith, pespin, yourself and me? At least
I'm flexible today.

Notes from the call we did:

• overloading one device with more sockets is not needed
• next step is getting ipv4 and ipv6 traffic working at all
• then the kernel code needs to be adjusted to match on the /64 netmask prefix for ipv6, as each user gets a specific prefix

IIRC Pablo said he would look into the kernel code some more and then notify us.

Thanks for the summary. I'll pick up asap, on Friday I could hit EINVAL when adding tunnels, I need to follow up on this and debug the issue.

just thinking more about this in terms of expectations to the kernel GTP IPv6 support:

• all-routers-multicast destination should not be handled in the kernel at all (neither inside the normal kernel ICMPv6 code, nor in the GTP module. It should be passed to the UDP socket of the control plan, so it can handle the 3GPP-specific interpretation of those packets
• link-local source address with lower-64bits matching a PDP context should also pass into the control plane via the UDP socket
• only the source address upper 64 bit prefix matching a PDP context should be handled inside the kernel path for decapsulation (like we do in the IPv4 path

• we cannot let the normal kernel ICMPv6 code handle them (as it would send wrong responses as it doesn't know the per-UE/PDP specific prefixes)
• we don't want to put all this logic into the kernel either; different control planes might want to handle this differently. Technically it's possible to do it in the kernel, if somebody thinks this has some advantage. I can't really think of any right now.

One more requirement for the combined IPv4/v6 support.

• it is correct that one Socket (and hence gtpX net-device) can remain bound to one specific address family
• however, a PDP context can not just be v4 or v6, but also 'v4v6'. The latter is (as stated earlier) the default in modern cellular networks.

This means that within one TEID we might have IPv4 and IPv6 in alternating fashion.

1. allow a single tunnel / TEID to have both a (inner) v4 address and a (inner) v6 prefix, not just one of the two, or
2. allow the creation of two tunnels with identical TEID but different address families

From the user/application point of view the first approach would be more convenient, but I undertand that it might be less invasive to the kernel code to do the second variant. Either way is fine in terms of achieving the desired functionality.

laforge wrote in #note-26:

pablo wrote in #note-24:

Hi!

I have just refreshed the kernel GTP driver for IPv4-over-IPv6, IPv6-over-IPv6 and IPv4-over-IPv6.

excellent, thanks.

I had a very brief review of the patches. I think it still does a /128 compare for IPv6 addresses, right? the two memcmp in gtp_check_ms_ipv6 and ipv6_pdp_find still use sizeof(struct in6_addr). As discussed, we need a /64 prefix match.

Yes, this is pending. I am going to digest this and get back to you with a patch (or questions if any before such patch), thanks!

laforge wrote in #note-28:

One more requirement for the combined IPv4/v6 support.

• it is correct that one Socket (and hence gtpX net-device) can remain bound to one specific address family
• however, a PDP context can not just be v4 or v6, but also 'v4v6'. The latter is (as stated earlier) the default in modern cellular networks.

This means that within one TEID we might have IPv4 and IPv6 in alternating fashion.

There are two possible ways to represent this to the netlink user (control plane):

1. allow a single tunnel / TEID to have both a (inner) v4 address and a (inner) v6 prefix, not just one of the two, or
2. allow the creation of two tunnels with identical TEID but different address families

From the user/application point of view the first approach would be more convenient, but I undertand that it might be less invasive to the kernel code to do the second variant. Either way is fine in terms of achieving the desired functionality.

Thanks! I will pick up on this task too.

Hi, just adding my five cents here to clarify something:

on the signaling plane, the UE (== MS) gets allocated a /64 prefix
the MS/UE may send a router-solicitation to the GGSN/PGW
the GGSN/PGW answers with a router advertisement containing that /64 prefix

According to Harald's writing above, I have the impression he expresses the fact that the 64 prefix initially allocated to do the IPv6 SLAAC procedure is the same one being passed to the UE as the final prefix to be used after SLAAC. This is afaiu incorrect: The two prefixes don't have any requirement/restriction to be the same; The prefix initially allocated to the SLAAC procedure can be different that the one assigned later one during SLAAC.

We actually had this sort of problem in our implementations in the past and got eventually fixed: https://gitea.osmocom.org/cellular-infrastructure/osmo-ggsn/commit/04715d284f5abc6bce6cd6401c5700e3031662de
The fact that osmo-ggsn uses the same prefix for both is just an implementation detail.
See the commit link shared above how actually talks about the fact of 3 IP address entries required to match the 1 IPv4 and 2 IPv6 addresses, as opposted to the 2 entries described by Harald previously:

There are two possible ways to represent this to the netlink user (control plane):

    allow a single tunnel / TEID to have both a (inner) v4 address and a (inner) v6 prefix, not just one of the two, or

Hi Pau,

pespin wrote in #note-31:

See the commit link shared above how actually talks about the fact of 3 IP address entries required to match the 1 IPv4 and 2 IPv6 addresses, as opposted to the 2 entries described by Harald previously:
[...]

Do you mean that this needs to represent the following in the netlink control plane?

IPv4 + TEID
IPv6 /prefix + TEID
IPv6 link-local /prefix + TEID

I was expecting that SGSN allocates the IP address as described above, so no SLAAC is seen in the tunnel.

Commit above says GGSN is responsible to reply to SLAAC request?

* link-local: "fe80::IfId", where IfId is the Interface-Identifier
  received during Pdp Context Resp and which can be used to communicate
  with the nearest router (the GGSN).

SLAAC happens between UE and GGSN. SGSN is just relaying messages from left to right and vice versa.

OK, then link-local traffic is seen in the tunnel. This confirms up to three entries (one for IPv4 and at least two for IPv6) are needed.

There are two possible ways to represent this to the netlink user (control plane):

    1. allow a single tunnel / TEID to have both a (inner) v4 address and a (inner) v6 prefix, not just one of the two, or
    2. allow the creation of two tunnels with identical TEID but different address families

I will go for option number 2, it is more flexible since it also covered for the SLAAC case.

Hi Pablo,

pablo wrote in #note-32:

Do you mean that this needs to represent the following in the netlink control plane?

IPv4 + TEID
IPv6 /prefix + TEID
IPv6 link-local /prefix + TEID

I never really used nor looked into the gtp tun module (I'm doing it soon these days since I need to set it up in osmo-epdg), so I can't tell for sure right now, I may be saying dummy things starting from here. My understanding is that the gtp kernel module offloads regular user data traffic and only forwards a handful of selected packets to the fd which the controlling userspace app (SGSN/GGSN) can read and process?
If that's the case, then yes I guess you need to forward the "IPv6 link-local /prefix + TEID" to the GGSN userspace process. But my point is, bear in mind this /prefix is not necessarily the same as the one in "IPv6 /prefix + TEID" case. Because I guess you don't want to forward ICMPv6 for link local address outside of the tunnel? Or we want to inject it into the regular routing stack and then handle it using some sort of RAW ICMP socket?

• UE/SGSN -> GGSN sends CreatePDPContextReq with type IPv6
• UE/SGSN <- GGSN sends CreatePDPContextResp with an 128bit IPv6 address consisting of first 64 bits of trash and 64 bits containing the Interface-Identifier (need to check in sgsnemu/osmo-ggsn whether the first or last 64 bits are the trash).
• UE/SGSN -> GGSN sends over the GTPU tunnel an ICMPv6 Router Soliciation with src link-local address fe80:: + Interface-Identifier.
• UE/SGSN <- GGSN sends over the GTPU tunnel an ICMPv6 Router Advertisement back to fe80:: + Interface-Identifier, containing "ICMPv6 Option" "Prefix information" of 64 bits. This "Prefix information" may be different that the Interface-Identifier used before.
• UE/SGSN <-> GGSN transmit data over GTPU tunnel for that pdp context using IPv6 address consisting of first 64 bits "Prefix information" and then 64 bits can be any value.

As Harald mentioned, you can see it in actions here: https://jenkins.osmocom.org/jenkins/view/TTCN3/job/ttcn3-ggsn-test/lastSuccessfulBuild/artifact/logs/ggsn-tester/GGSN_Tests.TC_pdp6_act_deact_gtpu_access.pcap.gz
(wireshark filter: "!tcp && !gsmtap_log"). Our GGSN implementation there is returning a 128bit IPv6 address consisting of the 64bit Interface-Identifier copied twice, but one of them is actually trash and not really used.

Hi Pablo,

On Tue, Nov 28, 2023 at 11:48:09AM +0000, pespin wrote:

My understanding is that the gtp kernel module offloads regular user data traffic and only forwards a handful of selected packets to the fd which the controlling userspace app (SGSN/GGSN) can read and process?

It's technically the other way around: It treats those packets it can treat in the kernel and lets
everything else pass normally to userspace via the udp socket. So basically the UDP socket doesn't see
all packets anymore.

If that's the case, then yes I guess you need to forward the "IPv6 link-local /prefix + TEID" to the GGSN userspace process.

That is exactly what I proposed in #1952#note-27 12 days ago. Happy to hear we agree.

laforge wrote in #note-37:

On Tue, Nov 28, 2023 at 11:48:09AM +0000, pespin wrote:

If that's the case, then yes I guess you need to forward the "IPv6 link-local /prefix + TEID" to the GGSN userspace process.

That is exactly what I proposed in #1952#note-27 12 days ago. Happy to hear we agree.

OK, then I will update the GTP driver to pass up this IPv6 link-local packets to userspace :-)

pablo wrote in #note-39:

Attaching patch to pass up IPv6 link-local packets to userspace.

I am checking this from gtp_check_ms_ipv6(), which already passes up packets to the userspace daemon in case that a MS does not have a PDP context in place.

Hi pablo, both the v6-prefix and the link-local patches look fine to me.

To avoid having to collect individual patches from an issue tracker:
Do you have a branch somehwere with the entire series of GTPv6 patches? https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/ doesn't have any updates since November 15, AFAICT.

laforge wrote in #note-42:

Hi pablo, both the v6-prefix and the link-local patches look fine to me.

To avoid having to collect individual patches from an issue tracker:
Do you have a branch somehwere with the entire series of GTPv6 patches? https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/ doesn't have any updates since November 15, AFAICT.

All of the three patches (also the one I just posted to allow to use same TEID for IPv4 and IPv6) that I posted today are compile-tested only.

I am going to give them a test, then push them out to:

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

I will report once this done.

pablo wrote in #note-44:

laforge wrote in #note-42:

Hi pablo, both the v6-prefix and the link-local patches look fine to me.

To avoid having to collect individual patches from an issue tracker:
Do you have a branch somehwere with the entire series of GTPv6 patches? https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/ doesn't have any updates since November 15, AFAICT.

Last update happened at that time Nov 15 (see Note 25 in this ticket). Then, from Note 26 on it includes the three new requirements that I just made patches for.

I believe all pending requirements are addressed by my patches today.

Thanks.

Patches have been pushed out here as requested:

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

I had to fix a few glitches in the patches I posted here I detected with testing:

- Check for s6_addr32\[3\] and s6_addr32\[4\] to be zero, should be instead s6_addr32\[2\] and s6_addr32\[3\] respectively.
- update ipv6_hash() function to only consider the IPv6 prefix, otherwise PDP context lookup from packet path fails.

I could also test control plane to add two gtp devices (hence sockets, one IPv4 and another IPv6 respectively). Then, add tunnels with the same TEID. I only tested control plane path, I would need to extend my tested to have a dual stack scenario (I have simple container tests for IPv4, IPv6, IPv4 over IPv6 and IPv6 over IPv4 but not for both IPv4 AND IPv6).

I found two issues:
- gtp-link for IPv6 listens to both IPv4 and IPv6 because IPV6_V6ONLY is off by default these days. So far we discussed that two sockets are required for a setup with both IPv4 and IPv6 traffic (dual stack), but probably it is possible to support for this with a single gtp device and socket by checking from sk->sk_ipv6only from the kernel. I still have to check if this will work from GTP tx and rx packet path too. Currently control plane rejects this since the IPv6 support patch from Q4 2023 I made, I can revisit this.
- gtp-tunnel del command, which reports ENOENT, I suspect it is a userspace issue. I remember Oliver mentioned this already.

pablo wrote in #note-47:

I made a kernel patch to ensure that IPv6 socket that is received from userspace is always IPv6-only, see attachment.

Two more patches have been pushed out to:

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

Hi Pablo,

thank you very much for the updated patches, and sorry that it took me some time to get back to this.

I have tried the updated kernel tree and libgtpnl patches.

01_ms_ip4_sgsn_ip4.sh is passing again.

02_ms_ip4_sgsn_ip6.sh almost passes, traffic flows through the GTP tunnel, only in the teardown it fails here (find full test output below):

+ /bin/ip addr del fd00::1/7 dev veth_sgsn
Error: ipv6: address not found.

• 03_ms_ip6_sgsn_ip4.sh
• 04_ms_ip6_sgsn_ip6.sh

For both I get the following error, find the full log below.

+ gtp-tunnel add gtp_sgsn v1 200 100 fc00::1 fd00::2
genl_socket_talk: Cannot assign requested address

I'm not sure if it is a problem with my tests, or if I'm triggering an unexpected code path again.

The tests are here:
https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/osmith/wip/tests/qemu

Your workflow seems to be to just boot the kernel on a test box and then run test code. You could directly run these tests if you put gtp-tunnel and gtp-link in PATH.

I run the tests in qemu by selecting the test to run in tests/qemu/initrd-init.sh, and then:

$ cp bzImage tests/qemu/_linux
$ ./configure --enable-qemu-tests
$ make
$ make check

I've built a kernel from your tree with make defconfig and then enabled via menuconfig:

CONFIG_GTP=y
CONFIG_NET_NS=y
CONFIG_VETH=y

Regarding the libgtpnl patches, there seems to be a bug in gtp-link.c: "gtp-link del" shouldn't require 4 arguments (argc < 4).

• are the tests up-to-date? they seem to have e.g. old syntax for gtp-tunnel
• are the iputils2 patches still required (https://osmocom.org/issues/1952#note-10)? for the purpose of testing libgtpnl.git, I think it would be preferably to just use gtp-link and gtp-tunnel instead, so we know these tools and the related libgtpnl code they use, works as expected
• do you remove the comments infront of some of the commented out lines when running the tests?

Full test outputs:

output of 02_ms_ip4_sgsn_ip6.shoutput of 02_ms_ip4_sgsn_ip6.sh

+ . /tests/00_test_functions.sh
+ alias 'ip=/bin/ip'
+ alias 'ggsn_side=ip netns exec ggsn_side'
+ MS=172.99.0.1
+ MS_PREFLEN=32
+ SGSN_GGSN_PROTO=ip6
+ SGSN=fd00::1
+ SGSN_PREFLEN=7
+ GGSN=fd00::2
+ WEBSERVER=172.99.0.2
+ tunnel_start
+ test -n 172.99.0.1
+ test -n 32
+ test -n ip6
+ test -n fd00::1
+ test -n 7
+ test -n fd00::2
+ test -n 172.99.0.2
+ /bin/ip netns add ggsn_side
+ /bin/ip link add veth_sgsn type veth peer name veth_ggsn
+ /bin/ip addr add fd00::1/7 dev veth_sgsn
+ /bin/ip link set veth_sgsn up
+ /bin/ip addr add 172.99.0.1/32 dev lo
+ /bin/ip link set lo up
+ sleep 1
+ gtp-link add gtp_sgsn ip6 --sgsn
WARNING: attaching dummy socket descriptors. Keep this process running for testing purposes.
[    1.071015] input: ImExPS/2 Generic Explorer Mouse as /devices/platform/i8042/serio1/input/input3
+ gtp-tunnel add gtp_sgsn v1 200 100 172.99.0.1 fd00::2
+ /bin/ip route add 172.99.0.2/32 dev gtp_sgsn
+ /bin/ip link set veth_ggsn netns ggsn_side
+ /bin/ip netns exec ggsn_side ip addr add fd00::2/7 dev veth_ggsn
+ /bin/ip netns exec ggsn_side ip link set veth_ggsn up
+ /bin/ip netns exec ggsn_side ip addr add 172.99.0.2/32 dev lo
+ /bin/ip netns exec ggsn_side ip link set lo up
+ + /bin/ip netns exec ggsn_side gtp-link add gtp_ggsn ip6
sleep 1
WARNING: attaching dummy socket descriptors. Keep this process running for testing purposes.
+ /bin/ip netns exec ggsn_side gtp-tunnel add gtp_ggsn v1 100 200 172.99.0.1 fd00::1
+ /bin/ip netns exec ggsn_side ip route add 172.99.0.1/32 dev gtp_ggsn
+ gtp-tunnel list
gtp_sgsn version 1 tei 200/100 ms_addr 172.99.0.1 sgsn_addr fd00::2
+ /bin/ip netns exec ggsn_side gtp-tunnel list
gtp_ggsn version 1 tei 100/200 ms_addr 172.99.0.1 sgsn_addr fd00::1
+ tunnel_ping
+ /bin/ip addr show
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet 172.99.0.1/32 scope global lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host 
       valid_lft forever preferred_lft forever
2: sit0@NONE: <NOARP> mtu 1480 qdisc noop state DOWN group default qlen 1000
    link/sit 0.0.0.0 brd 0.0.0.0
4: veth_sgsn@if3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 9e:13:19:cf:80:47 brd ff:ff:ff:ff:ff:ff link-netns ggsn_side
    inet6 fd00::1/7 scope global tentative 
       valid_lft forever preferred_lft forever
    inet6 fe80::9c13:19ff:fecf:8047/64 scope link tentative 
       valid_lft forever preferred_lft forever
5: gtp_sgsn: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1452 qdisc noqueue state UNKNOWN group default qlen 1000
    link/none 
    inet6 fe80::f566:7ed:4b10:f1fa/64 scope link stable-privacy 
       valid_lft forever preferred_lft forever
+ ping -c 1 172.99.0.2
PING 172.99.0.2 (172.99.0.2): 56 data bytes
64 bytes from 172.99.0.2: seq=0 ttl=64 time=1044.078 ms

--- 172.99.0.2 ping statistics ---
1 packets transmitted, 1 packets received, 0% packet loss
round-trip min/avg/max = 1044.078/1044.078/1044.078 ms
+ /bin/ip netns exec ggsn_side ping -c 1 172.99.0.1
PING 172.99.0.1 (172.99.0.1): 56 data bytes
64 bytes from 172.99.0.1: seq=0 ttl=64 time=0.077 ms

--- 172.99.0.1 ping statistics ---
1 packets transmitted, 1 packets received, 0% packet loss
round-trip min/avg/max = 0.077/0.077/0.077 ms
+ tunnel_stop
+ killall gtp-link
+ /bin/ip addr del 172.99.0.1/32 dev lo
+ /bin/ip link set veth_sgsn down
+ /bin/ip addr del fd00::1/7 dev veth_sgsn
Error: ipv6: address not found.

output of 03_ms_ip6_sgsn_ip4.shoutput of 03_ms_ip6_sgsn_ip4.sh

+ . /tests/00_test_functions.sh
+ alias 'ip=/bin/ip'
+ alias 'ggsn_side=ip netns exec ggsn_side'
+ MS=fd00::1
+ MS_PREFLEN=7
+ SGSN_GGSN_PROTO=ip
+ SGSN=172.0.0.1
+ SGSN_PREFLEN=24
+ GGSN=172.0.0.2
+ WEBSERVER=fd00::2
+ tunnel_start
+ test -n fd00::1
+ test -n 7
+ test -n ip
+ test -n 172.0.0.1
+ test -n 24
+ test -n 172.0.0.2
+ test -n fd00::2
+ /bin/ip netns add ggsn_side
+ /bin/ip link add veth_sgsn type veth peer name veth_ggsn
+ /bin/ip addr add 172.0.0.1/24 dev veth_sgsn
+ /bin/ip link set veth_sgsn up
+ /bin/ip addr add fd00::1/7 dev lo
+ /bin/ip link set lo up
+ sleep 1
+ gtp-link add gtp_sgsn ip --sgsn
WARNING: attaching dummy socket descriptors. Keep this process running for testing purposes.
+ gtp-tunnel add gtp_sgsn v1 200 100 fd00::1 172.0.0.2
genl_socket_talk: Cannot assign requested address

output of 04_ms_ip6_sgsn_ip6.shoutput of 04_ms_ip6_sgsn_ip6.sh

+ . /tests/00_test_functions.sh
+ alias 'ip=/bin/ip'
+ alias 'ggsn_side=ip netns exec ggsn_side'
+ MS=fc00::1
+ MS_PREFLEN=7
+ SGSN_GGSN_PROTO=ip6
+ SGSN=fd00::1
+ SGSN_PREFLEN=7
+ GGSN=fd00::2
+ WEBSERVER=fc00::2
+ tunnel_start
+ test -n fc00::1
+ test -n 7
+ test -n ip6
+ test -n fd00::1
+ test -n 7
+ test -n fd00::2
+ test -n fc00::2
+ /bin/ip netns add ggsn_side
+ /bin/ip link add veth_sgsn type veth peer name veth_ggsn
+ /bin/ip addr add fd00::1/7 dev veth_sgsn
+ /bin/ip link set veth_sgsn up
+ /bin/ip addr add fc00::1/7 dev lo
+ /bin/ip link set lo up
+ sleep 1
+ gtp-link add gtp_sgsn ip6 --sgsn
WARNING: attaching dummy socket descriptors. Keep this process running for testing purposes.
+ gtp-tunnel add gtp_sgsn v1 200 100 fc00::1 fd00::2
genl_socket_talk: Cannot assign requested address

osmith wrote in #note-50:

Hi Pablo,

thank you very much for the updated patches, and sorry that it took me some time to get back to this.

no issue!

I have tried the updated kernel tree and libgtpnl patches.

01_ms_ip4_sgsn_ip4.sh is passing again.

02_ms_ip4_sgsn_ip6.sh almost passes, traffic flows through the GTP tunnel, only in the teardown it fails here (find full test output below):

+ /bin/ip addr del fd00::1/7 dev veth_sgsn
Error: ipv6: address not found.

It looks like an issue in your tests.

tunnel_stop() invokes: ip netns del ggsn_side

and tunnel_stop is called before: ip addr del fd00::1/7 dev veth_sgsn

+ tunnel_stop
+ killall gtp-link
+ /bin/ip addr del 172.99.0.1/32 dev lo
+ /bin/ip link set veth_sgsn down
+ /bin/ip addr del fd00::1/7 dev veth_sgsn
Error: ipv6: address not found.

And note that:

ip link add veth_sgsn type veth peer name veth_ggsn

and veth_sgns depends on the peer veth_ggsn, which is now gone.

Could you try to remove this address before the veth_ggsn is destroyed?

IIRC, the host size veth goes away as soon as the container is removed, since the peer veth on the container is gone, the kernel releases the veth as soon because the host veth now points nowhere.

And again, IIRC, the kernel uses a wait queue (see netdev_unregistering_wq), and it wakes up this kernel thread when there is garbage netdev interfaces, which might explain why you don't see this error earlier. So this looks like a race in the test scripts?

These two tests don't pass:

• 03_ms_ip6_sgsn_ip4.sh
• 04_ms_ip6_sgsn_ip6.sh

For both I get the following error, find the full log below.

+ gtp-tunnel add gtp_sgsn v1 200 100 fc00::1 fd00::2
genl_socket_talk: Cannot assign requested address

I'm not sure if it is a problem with my tests, or if I'm triggering an unexpected code path again.

I forgot to mention: ms address now needs to be IPv6 /64 prefix, ie:

+ gtp-tunnel add gtp_sgsn v1 200 100 fc00:: fd00::2

otherwise this hits EADDRNOTAVAIL, see:

commit 7bdd525d85db58862fb6bf52191af3b59490697d
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Tue Jan 30 11:24:24 2024 +0100

    gtp: use IPv6 address /64 prefix for UE/MS

and

commit dac0c642ed0f96d0b150bb5f6b1ea2e556c37f31 (HEAD -> main, orig
in/main)
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Wed Jan 31 19:35:51 2024 +0100

    gtp: report EADDRNOTAVAIL if userspace provides a non-IPv6 /64 
prefix for MS/UE

    Provide a different error reporting, in case userspace provides
 a
    non-IPv6 /64 prefix.

    Squash this patch to ("gtp: use IPv6 address /64 prefix for UE/
MS")

The tests are here:
https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/osmith/wip/tests/qemu

Your workflow seems to be to just boot the kernel on a test box and then run test code. You could directly run these tests if you put gtp-tunnel and gtp-link in PATH.

I run the tests in qemu by selecting the test to run in tests/qemu/initrd-init.sh, and then:
[...]

I've built a kernel from your tree with make defconfig and then enabled via menuconfig:
[...]

Regarding the libgtpnl patches, there seems to be a bug in gtp-link.c: "gtp-link del" shouldn't require 4 arguments (argc < 4).

Looking at this, let me get back to you.

I will following up in a different thread with a bit more details on my test scripts that could be using for kselftests.

osmith wrote in #note-50:

Regarding your tests, I could try to run them as well (and use them to narrow down why my tests don't work), but I would need some more information:

• are the tests up-to-date? they seem to have e.g. old syntax for gtp-tunnel

Tests are up to date and running fine here.

• are the iputils2 patches still required (https://osmocom.org/issues/1952#note-10)? for the purpose of testing libgtpnl.git, I think it would be preferably to just use gtp-link and gtp-tunnel instead, so we know these tools and the related libgtpnl code they use, works as expected

Not needed.

• do you remove the comments infront of some of the commented out lines when running the tests?

No, they can be removed. They are distracting leftovers. Although, last two lines give you a hint on to use iperf from ns1 and ns2 (the client and server that send traffic that uses nsr1 and nsr2 as routers (sgsn and ggsn respectively).

These scripts work for me almost out-of-the-box. I copied them to libgtnl/tools/, then run the script and from ''ns1'':

I have to do a few manual things to generate traffic:

ip netns exec ns1 ping X.X.X.X

where X.X.X.X is the IP address in ns2, to check ICMP traffic. Type 'exit' to leave ns1 container.

Then, run iperf server in ns2:

term2% ip netns exec ns2 iperf3 -s

and in a different terminal, run iperf client traffic generator:

term1% ip netns exec ns1 iperf3 -c 192.168.10.2 -n 100G

to test TCP traffic, where -n is the amount of traffic to generate. I have to press ctrl-c to stop it.

To automate this, it should be possible to check for the ping output results. With iperf, using -t (to limit maximum time to send data) in the client (ns1), and -D in the server (ns2).

It would be great to use this to include kselftests in the submission. Maybe extend the nftables ruleset to match for GTP traffic (simple rule with UDP and port), then check counters in the ruleset.

See linux/tools/testing/selftests/ for kselftest, there is one folder for netfilter.

pablo wrote in #note-53:

See linux/tools/testing/selftests/ for kselftest, there is one folder for netfilter.

Hm.

One issue when integrating this into kselftests is the availability of a fresh libgtnl version and the gtp-link and gtp-tunnel tools...

Another possibility can be to make ad-hoc libmnl programs to add gtp links and tunnels that can compile in the kernel tree, so this integrates into kselftest.

There is some recent progress to have NetDev tree CI:

https://netdev.bots.linux.dev/status.html

but to be honest, I have no idea how that works and how to request for GTP to be covered. I don't seem to find the netfilter tests on that page.

Thank you for the detailed feedback!

pablo wrote in #note-51:

02_ms_ip4_sgsn_ip6.sh almost passes, traffic flows through the GTP tunnel, only in the teardown it fails here (find full test output below):

+ /bin/ip addr del fd00::1/7 dev veth_sgsn
Error: ipv6: address not found.

It looks like an issue in your tests.

tunnel_stop() invokes: ip netns del ggsn_side

and tunnel_stop is called before: ip addr del fd00::1/7 dev veth_sgsn

+ tunnel_stop
+ killall gtp-link
+ /bin/ip addr del 172.99.0.1/32 dev lo
+ /bin/ip link set veth_sgsn down
+ /bin/ip addr del fd00::1/7 dev veth_sgsn
Error: ipv6: address not found.

The "ip addr del" line is part of "tunnel_stop", the shell (sh -x) prints "+ tunnel_stop" to indicate that it is now executing this function, and then each line it executes from this function. So this is not resolved yet, if you have another pointer it would be great.

And note that:

[...]

and veth_sgns depends on the peer veth_ggsn, which is now gone.

Could you try to remove this address before the veth_ggsn is destroyed?

IIRC, the host size veth goes away as soon as the container is removed, since the peer veth on the container is gone, the kernel releases the veth as soon because the host veth now points nowhere.

And again, IIRC, the kernel uses a wait queue (see netdev_unregistering_wq), and it wakes up this kernel thread when there is garbage netdev interfaces, which might explain why you don't see this error earlier. So this looks like a race in the test scripts?

I see it consistently failing with ip6, and never with ip.

These two tests don't pass:

• 03_ms_ip6_sgsn_ip4.sh
• 04_ms_ip6_sgsn_ip6.sh

For both I get the following error, find the full log below.

+ gtp-tunnel add gtp_sgsn v1 200 100 fc00::1 fd00::2
genl_socket_talk: Cannot assign requested address

I'm not sure if it is a problem with my tests, or if I'm triggering an unexpected code path again.

I forgot to mention: ms address now needs to be IPv6 /64 prefix, ie:

+ gtp-tunnel add gtp_sgsn v1 200 100 fc00:: fd00::2

Thanks a lot, I've adjusted the tests 03 and 04, and they pass now!

pablo wrote in #note-52:

osmith wrote in #note-50:

Regarding the libgtpnl patches, there seems to be a bug in gtp-link.c: "gtp-link del" shouldn't require 4 arguments (argc < 4).

Fix for git-link del, this needs to be collapsed to: ("gtp-link: add IPv6 support"), attached.

Thanks, the fix works. I have rebased my patch on top of your latest attached patches, squashed the gtp-link del fix as instructed, and submitted them to gerrit:

• https://gerrit.osmocom.org/c/libgtpnl/+/35985 gtp-link: add IPv6 support [NEW]
• https://gerrit.osmocom.org/c/libgtpnl/+/35986 gtp-link: set IPv6 socket only [NEW]
• https://gerrit.osmocom.org/c/libgtpnl/+/35987 gtp: provide interface to set family [NEW]
• https://gerrit.osmocom.org/c/libgtpnl/+/35988 gtp: add flags to gtp_tunnel object [NEW]
• https://gerrit.osmocom.org/c/libgtpnl/+/35989 gtp-genl: display gtp device in listing [NEW]
• https://gerrit.osmocom.org/c/libgtpnl/+/35990 gtp-tunnel: display i_tei in help [NEW]
• https://gerrit.osmocom.org/c/libgtpnl/+/35991 Add QEMU tests [NEW]

Also thanks for the instructions on how to use your tests. It would be great to have them in kselftests. Not sure what the most elegant way to do it is, but maybe it is feasible once a new libgtpnl version is released that has the new gtp-link and gtp-tunnel tools? (But FWIW, I think this is out-of-scope for the issue at hand... at least we have the tests for libgtpnl + kernel, and the ttcn-3 tests for osmo-ggsn + libgtpnl + kernel, so we should notice regressions.)

I have also done some regression testing with the TTCN-3 tests for osmo-ggsn and found that using your kernel tree with the old libgtpnl fails. I assume the kernel now requires the family to be set, and does not default to ipv4 as it was the case before. Does that make sense, and if so, could you fix it to avoid the regression?

What I have not tested yet is using the new libgtpnl, which sets the ip family, and using an old kernel which does not support setting the family. For best compatibility, using ipv4 should still work in that combination. Do you think there is anything that needs to be adjusted, or should this be working?

Pau has left some review comments on your patches on gerrit. Pablo, do you want to reply there, and possibly update the patches?

On Thu, Feb 15, 2024 at 02:25:15PM +0000, osmith wrote:

I have also done some regression testing with the TTCN-3 tests for osmo-ggsn and found that using your kernel tree with the old libgtpnl fails. I assume the kernel now requires the family to be set, and does not default to ipv4 as it was the case before. Does that make sense, and if so, could you fix it to avoid the regression?

if this is true, it would be an ABI breakage and must be resolved.
Old apps/libs must work against new kernels, of course.

osmith wrote in #note-57:

Pau has left some review comments on your patches on gerrit. Pablo, do you want to reply there, and possibly update the patches?

Replied to https://gerrit.osmocom.org/c/libgtpnl/+/35985/1 and related comments in the same patch series.

laforge wrote in #note-58:

On Thu, Feb 15, 2024 at 02:25:15PM +0000, osmith wrote:

I have also done some regression testing with the TTCN-3 tests for osmo-ggsn and found that using your kernel tree with the old libgtpnl fails. I assume the kernel now requires the family to be set, and does not default to ipv4 as it was the case before. Does that make sense, and if so, could you fix it to avoid the regression?

Hm. There is a fallback to AF_INET if the attribute is not specified.

if (info->attrs[GTPA_FAMILY])
      family = nla_get_u8(info->attrs[GTPA_FAMILY]);
else
      family = AF_INET;

if this is true, it would be an ABI breakage and must be resolved.
Old apps/libs must work against new kernels, of course.

I have just tested:

- new libgtnl with old stable 6.1 kernel, using my test2-netns-gtp.sh and it works fine.
- old libgtnl (1.2.5, not including any recent patch for the IPv6 support), I had modified test2-netns-gtp.sh so it uses old gtp-link syntax, with new kernel with GTP IPv6 patches, it also works fine.

I cannot reproduce this issue here.

pablo wrote in #note-60:

pablo wrote in #note-59:

osmith wrote in #note-57:

Pau has left some review comments on your patches on gerrit. Pablo, do you want to reply there, and possibly update the patches?

Replied to https://gerrit.osmocom.org/c/libgtpnl/+/35985/1 and related comments in the same patch series.

pespin said: "What If I want to, lets say bind to IPv4 address A.B.C.D and IPV6 address ::Z, and then announce those in CreateSessionReq/Resp in GTPv{1,2}C? AFAIU I need to create 2 gtp tun devices, binding one to A.B.C.D and one to ::Z."

I am attaching a patch to extend the gtp-link to allow to specify the listener address, I extended one of my container tests, I checked via ss -tu that the specified listener address is shown. It falls back to ANY_ADDRESS in case no address is specified.

Thanks! Submitted the patch to gerrit here:
https://gerrit.osmocom.org/c/libgtpnl/+/35997

Adjusted it slightly to make it pass checkpatch:

tools/gtp-link.c:146: WARNING:BRACES_NOT_NECESSARY: braces {} are not necessary for any arm of this statement
total: 0 errors, 1 warnings, 67 lines checked

EDIT: also done trivial fixes to have "[<address>]" in the usage help string

pablo wrote in #note-61:

laforge wrote in #note-58:

On Thu, Feb 15, 2024 at 02:25:15PM +0000, osmith wrote:

I have also done some regression testing with the TTCN-3 tests for osmo-ggsn and found that using your kernel tree with the old libgtpnl fails. I assume the kernel now requires the family to be set, and does not default to ipv4 as it was the case before. Does that make sense, and if so, could you fix it to avoid the regression?

Hm. There is a fallback to AF_INET if the attribute is not specified.

[...]

if this is true, it would be an ABI breakage and must be resolved.
Old apps/libs must work against new kernels, of course.

I have just tested:

- new libgtnl with old stable 6.1 kernel, using my test2-netns-gtp.sh and it works fine.
- old libgtnl (1.2.5, not including any recent patch for the IPv6 support), I had modified test2-netns-gtp.sh so it uses old gtp-link syntax, with new kernel with GTP IPv6 patches, it also works fine.

I cannot reproduce this issue here.

My bad, I ran the ttcn-3 tests with libgptnl nightly (current master) against your kernel tree. I can confirm that if using libgtpnl 1.2.5 against your kernel tree, the same tests pass as when running the testsuite against a standard debian kernel. So there is no regression.

osmith wrote in #note-64:

Hi Pablo,

thanks!

I'm in the process of testing everything, trying to get this finished up.

The following combinations show no regressions in our ttcn3 tests (EDIT: if the right configurations are used, jenkins shows a lot more errors right now because we use a new configuration with an old kernel; I've tested locally):

• old osmo-ggsn, old libgtpnl, new kernel
• old osmo-ggsn, new libgtpnl, old kernel
• old osmo-ggsn, new libgtpnl, new kernel
• (new osmo-ggsn, old libgtpnl is not valid and configure.ac will detect this in the next release)
• new osmo-ggsn, new libgtpnl, old kernel
• new osmo-ggsn, new libgtpnl, new kernel

("old" always means the previous released version)

I've also tested new osmo-ggsn, new libgtpnl and the current kernel (minus the patch you just added) on real hardware, and there is no regression in IPv4 traffic. I'm not getting IPv6 traffic working on real hardware yet, but that is probably an issue with my test setup as we do have some IPv6 related tests passing in the ttcn3 testsuite.

Thanks a lot for the detailed report and taking the time to careful test my recent work on the GTP kernel driver!

Since no regression was found: could you start the process to upstream your kernel patches, if you haven't already? I'm assuming, they would then show up at net-next at some point, and then get merged into mainline?

Sure, I will report progress on this issue.

One of the ttcn3 tests that doesn't pass yet (compared to not using kernel-gtpu) is TC_pdp6_act_deact_gtpu_access -> https://osmocom.org/issues/6382

Indeed, we are currently discussing this.

pablo wrote in #note-67:

I am currently rebasing on top of net-next to prepare for submission, I will refresh the gtp.git kernel tree in a few hours, in case you have a chance to give it another run to tests.

For the record, I have pushed out a rebase on top of net-next.git, including a fix for device maximum mtu for IPv6 and another to reject nonsense such as sk0 being AF_INET and sk1u being AF_INET6.

My rebase is made on top of three recent patches from Breno Leitao that have been already posted and that are flying to net-next now.