Hi Harald,
On 12/12/2020 12:22, Harald Welte wrote:
Hi Jonas,
thanks again for your patches, they are very much appreciated.
However, I don't think that it is "that easy".
PDP contexts (at least) in GPRS/EDGE and UMTS come in three flavors:
* IPv4 only
* IPv6 only
* IPv4v6 (i.e. both an IPv4 and an IPv6 address within the same tunnel)
See for example osmo-ggsn at https://git.osmocom.org/osmo-ggsn
for an userspace implementation that covers all three cases,
as well as a related automatic test suite containing cases
for all three flavors at
https://git.osmocom.org/osmo-ttcn3-hacks/tree/ggsn_tests
If I read your patch correctly
On Fri, Dec 11, 2020 at 01:26:10PM +0100, Jonas Bonn wrote:
- struct in_addr ms_addr_ip4;
- struct in_addr peer_addr_ip4;
+ struct in6_addr ms_addr;
+ struct in6_addr peer_addr;
this simply replaces the (inner) IPv4 "MS addr" with an IPv6 "MS addr".
Yes, that's correct. It's currently an either-or proposition for the UE
address.
I actually wanted to proceed a bit carefully here because this patch
series is already quite large. I do plan on adding support for multiple
MS addresses, but already just juggling mixed inner and outer tunnels
seemed challenging enough to review.
Sure, it is an improvement over v4-only. But IMHO any follow-up
change to introduce v4v6 PDP contexts would require significant changes,
basically re-introducing the ms_add_ip4 member which you are removing here.
I think we ultimately need to take the MS (UE address) member out of the
PDP context struct altogether. A single PDP context can apply to even
more than two addresses as the UE can/should be provisioned with
multiple IPv6 addresses (as I understand it, superficially).
Therefore, I argue very much in favor of intrducing proper IPv6 support
(including v4v6) in one go.
For provisioning contexts via Netlink, I agree that we should try to
avoid pathological intermediate steps. For the actual packet handling
of outer and inner IPv6 tunnels, I think it's moot whether or not the
PDP contexts support multiple addresses or not: the subsequent step to
extend to multiple IP's is a bit of churn, but doesn't immediately seem
overly intrusive.
Anyway, I'll look into making this change...
Thanks for the review!
/Jonas
Regards,
Harald
