Bug #1952
openIPv6 support for inner (user) IP layer missing
70%
Description
The 3GPP specs permit for both IPv4 and IPv6, but we only implement IPv4. This is embarrassing and should be changed. Contribution welcome!
Files
Related issues
Updated by laforge 5 months ago
- Status changed from New to In Progress
- Assignee set to pablo
- Priority changed from Normal to High
- % Done changed from 0 to 90
This has actually been implemented by pablo some time ago, the kernel code is in https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/ while the libgtpnl extension is in https://gitea.osmocom.org/cellular-infrastructure/libgtpnl/src/branch/pablo/ipv6
We need to adjust osmo-ggsn support this in order to run the existing IPv6 TTCN3 test suite against it.
Updated by laforge 5 months ago
- Related to Feature #6096: add support for kernel-GTP IPv6 added
Updated by pablo about 1 month ago
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.
Updated by osmith about 1 month ago
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.
Updated by pablo about 1 month ago
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.
Updated by pablo about 1 month ago
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.
Updated by osmith about 1 month ago
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.
I've tried to explain above why I did it that way. What I changed is mostly: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!
Updated by pablo about 1 month ago
- File test2-netns-gtp-ipv4-over-ipv6.sh test2-netns-gtp-ipv4-over-ipv6.sh added
- File test2-netns-gtp-ipv6.sh test2-netns-gtp-ipv6.sh added
- File test2-netns-gtp-ipv6-over-ipv4.sh test2-netns-gtp-ipv6-over-ipv4.sh added
- File 0001-uapi-cache-linux-gtp.h-header.patch 0001-uapi-cache-linux-gtp.h-header.patch added
- File 0002-link-gtp-add-support-for-local-listener-address.patch 0002-link-gtp-add-support-for-local-listener-address.patch added
osmith wrote in #note-9:
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 understand, thanks for explaining. I missed all that process. I made scripts to test both v4-in-v6 and v6-in-v4, they are all attached. We have used different tickets for this task, so it seems they got lost.
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.
I've tried to explain above why I did it that way. What I changed is mostly: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)
thanks for explaining.
Please do read the patch in its current form, it is not long.
yes, patch is small, I will take a look.
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.)
Attached. They require two patches for iproute2, which allows for standalone GTP tunnels (with no osmo-ggsn).
Thanks
Updated by laforge about 1 month ago
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.
Updated by osmith 30 days ago
- % Done changed from 90 to 70
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:Updated by pablo 29 days ago
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:
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);
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?
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.
Updated by pablo 29 days ago
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",
Updated by osmith 29 days ago
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
Updated by pablo 29 days ago
- File gtp-link-ipv4-ipv6.patch gtp-link-ipv4-ipv6.patch added
Updated by laforge 29 days ago
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
Updated by laforge 29 days ago
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
Updated by pablo 29 days 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.
Updated by laforge 29 days ago
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.
Updated by osmith 21 days ago
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.
Updated by pablo 13 days ago
Hi!
I have just refreshed the kernel GTP driver for IPv4-over-IPv6, IPv6-over-IPv6 and IPv4-over-IPv6.
Tree is available here:
https://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp.git/
This round:
- Fixes a crash due to missing sanity check on the socket type, before it was possible to attach an IPv4 socket (which determines the outer header) to a mismatching IPv6 GTP tunnel.
- Fixes a silly union I made at the very beginning of the previous series when preparing for the IPv6 support, resulting in proble, when outer and inner header families were different.
- Added a few default: cases to switches with WARN_ON_ONCE(1) and properly set error, just in case.
- Remove pctx->peer_af, use the socket family which already determines the outer header family. Outer IP header and socket families need to be the same as we discussed, that is, a GTP device supports a single outer header family, but inner headers can be either IPv4 or IPv6.
This also requires a patch for libgtnl/gtp-link to support for IPv6 sockets that is attached to this report!
This time, I tested this with gtp-link (not iproute standalone more). For datapath, I have generated basic icmp echo/request packets and TCP traffic with iperf3.
I use this firewall policy to clamp TCP MSS to path MTU on the routers:
table inet filter {
chain forward {
type filter hook forward priority 0; policy accept;
tcp flags syn tcp option maxseg size set rt mtu
}
}
Updated by pablo 13 days ago
- File test2-netns-gtp.sh test2-netns-gtp.sh added
- File test2-netns-gtp-ipv4-over-ipv6.sh test2-netns-gtp-ipv4-over-ipv6.sh added
- File test2-netns-gtp-ipv6.sh test2-netns-gtp-ipv6.sh added
- File test2-netns-gtp-ipv6-over-ipv4.sh test2-netns-gtp-ipv6-over-ipv4.sh added
Here are my test scripts. They live under libgtpnl/tools/ in my test box, so they can invoke ./gtp-link and ./gtp-tunnel.
Updated by laforge 13 days ago
- File GGSN_Tests.TC_pdp6_act_deact_gtpu_access.pcap.gz GGSN_Tests.TC_pdp6_act_deact_gtpu_access.pcap.gz added
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.
3GPP TS 29.060:
PDN Connection: the association between a MS represented by one IPv4 address and/or one IPv6 prefix and a PDN represented by an APN.
so this clearly states that IPv4 is a single address while IPv6 is a single prefix.
3GPP TS 29.061, Section 11.2.1.3:
For APNs that are configured for IPv6 address allocation, the GGSN/P-GW shall only use the Prefix part of the IPv6 address for forwarding of mobile terminated IP packets. The size of the prefix shall be according to the maximum prefix length for a global IPv6 address as specified in the IPv6 Addressing Architecture, see RFC 4291 [82].
RFC 4291 section 2.5.4 states
All Global Unicast addresses other than those that start with binary 000 have a 64-bit interface ID field (i.e., n + m = 64) ...
3GPP TS 29.61 Section 11.2.1.3.2a:
In the procedure in the cases of using GTP-based S5/S8, P-GW acts as an access router, and allocates to a UE a globally unique /64 IPv6 prefix if the PLMN allocates the prefix. ...
The situation in IPv6 beyond that is unfortunately is rather complex. My memory is blurred as I implemented it many years ago (2017) in osmo-ggsn.
The point is that- 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
The router-solicitation/advertisement is using link-local addresses. In order to avoid address conflicts, the interface identifier used by the MS/UE is set to the /64 prefix(!).
So initially, the UE uses a link-local address with the lower 64 bits (interface identifier) set to the /64 prefix, and later on (assigned via router advertisement) the UE uses the /64 prefix as upper 64 bits and uses a random lower 64 bits (interface identifier).
In theory, of course, also at any later point during the PDP context, the UE and/or GGSN may need to send packets using the link-local addresses.
See 3GPP TS 29.061 11.2.1.3.2
The attached pcap file GGSN_Tests.TC_pdp6_act_deact_gtpu_access.pcap.gz (generated by our test suite, last artefact can always be found at 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) you can see
| Packet | Protocol | Interpretation |
|---|---|---|
| 52 | GTP-C | control plane allocates 2001:780:44:2004/64 to the UE |
| 54 | GTP-U | UE sends router solicitation from fe80:2001:780:44:2004/128 to ff02::2 |
| 56 | GTP-U | GGSN sends router advertisement from its own link-local addr to fe80:2001:780:44:2004/128 |
| 57 | GTP-U | UE sends neighbor solicitation from fe80:2001:780:44:2004 |
| 63+66 | GPT-U | UE exchanges normal IPv6 traffic from any random source within 2001:780:44:2004/64 |
- https://gitea.osmocom.org/cellular-infrastructure/osmo-ggsn/src/branch/master/lib/icmpv6.c (handling of incoming packets to all-router multicast; generating router advertisement)
- https://gitea.osmocom.org/cellular-infrastructure/osmo-ggsn/src/commit/08bb5182a471463e3eaf503ba030948ec84c790d/ggsn/ggsn.c#L668 (dispatchg of incoming IPv6 messages, showing mem-compare depending on whether it's link-local or not
Updated by laforge 13 days ago
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.
Updated by laforge 13 days ago
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):- allow a single tunnel / TEID to have both a (inner) v4 address and a (inner) v6 prefix, not just one of the two, or
- 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.
Updated by pablo 13 days ago
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!
Updated by pablo 13 days ago
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):
- allow a single tunnel / TEID to have both a (inner) v4 address and a (inner) v6 prefix, not just one of the two, or
- 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.
Updated by pespin 2 days ago
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
Updated by pablo 1 day ago
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).
Updated by pablo 1 day ago
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.
Updated by pespin about 22 hours ago
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.
Updated by laforge about 21 hours ago
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.
