Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

systemd: network: Avoid assigning IP addr from UE IP pool to ogstun interface #2975

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

pespin
Copy link
Contributor

@pespin pespin commented Feb 16, 2024

When running the open5gs package with systemd network config, the 1st IP address of the UE pool configured in open5gs-upfd config file for ogstun was being assigned to the interface. This causes routing problems when traffic from the UE which was assigned that same IP address is routed to/from the ogstun interface.

As a result, every time that open5gs-upfd systemd service was restarted, the following changed had to be applied:
ip addr del 10.45.0.1/16 dev ogstun
ip route add 10.45.0.0/16 dev ogstun

The IP address was set in order to get the routing entry for free. Instead, it should only add the routing entry.

Related: https://osmocom.org/issues/6361

@acetcom
Copy link
Member

acetcom commented Feb 17, 2024

@pespin

By default, open5gs assigns IPs to UEs except for 10.45.0.1. Do I still need to add the routing table when restarting UPF in this case?

Also, another issue is that if I modify systemd like that, the test code in open5gs doesn't work: all the codes that test GTP-U using ping packets don't seem to work. ./tests/registration/registration simple-test gets stuck where it is sending to and from GTP-U.

If I'm misunderstanding something, please let me know again.

Thanks a lot!
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Feb 18, 2024

Hi @acetcom ,

By default, open5gs assigns IPs to UEs except for 10.45.0.1. Do I still need to add the routing table when restarting UPF in this case?

I am not understanding you here. Can you rephrase or extend your question?
When using open5gs-smfd+upfd (in our setup where we are currently developing osmo-epdg), we have open5gs-smfd configured with the following:

  session:
    - subnet: 10.45.0.0/16

And the first time I send a GTPv2C CreateSessionReq, I get IP address 10.45.0.1 assigned. But you seem to say in your question that open5gs should not be assigning this IP address to UE. So according to what you say, you'd expect the first address in the pool to be reserved for PGW used and never assigned to UEs? Then maybe there's a bug in open5gs-smfd and we should get open5gs-smfd fixed so that it avoid assigning the first IP address to UEs.

Also, another issue is that if I modify systemd like that, the test code in open5gs doesn't work: all the codes that test GTP-U using ping packets don't seem to work. ./tests/registration/registration simple-test gets stuck where it is sending to and from GTP-U.

Are the open5gs tests under ./tests/ using systemd? I was unaware of that.
Are those simple-tests trying to ping the the IP address assigned at open5gs-upf? That may explain that they are not working anymore.

@acetcom
Copy link
Member

acetcom commented Feb 19, 2024

@pespin

Now that I know what happened, let me try to explain it again.

Open5GS defaults to the following settings.

  session:
    - subnet: 10.45.0.1/16

I'm using 10.45.0.1/16 instead of 10.45.0.0/16. I'm assuming that 10.45.0.1 is the IP set in the TUN, and I'm making sure that 10.45.0.1 is not set when setting the IP in the UE Pool.

The relevant code in lib/pfcp/context.c is shown below.

2003
2004                 /* Exclude TUN IP Address */
2005                 if (memcmp(ue_ip->addr, subnet->gw.sub, maxbytes) == 0)
2006                     continue;
2007

And using this TUN IP, the test program is using Ping (ICMP) to test Data Plane.

Open5GS is implemented so that any configuration file or program, including systemd, assumes that the TUN IP is set.

Of course, it is possible to implement this without TUN IP as you suggest, but it makes testing very difficult. In addition to the test program of Open5GS, I think the TUN IP is necessary for ease of installation during the actual deployment.

It may be a little more vulnerable, but I think TUN IPs are necessary nonetheless, and that's how Open5GS was designed.

Please let me know if you have any diffrent idea.

Thanks a lot!
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Feb 19, 2024

@pespin

Now that I know what happened, let me try to explain it again.

Open5GS defaults to the following settings.

  session:
    - subnet: 10.45.0.1/16

Ok I am also starting to understand the issue. IMHO specifying the subnet as 10.45.0.1/16 is really strange in here, since you'd be breaking the assumption that a subnet is a set of 1 followed by a set of 0 (a prefix).
See https://en.wikipedia.org/wiki/Subnet#Internet_Protocol_version_4 for more info.
One would in general expect the subnet to be 10.45.0.0.0/16, with the first (10.45.0.0.0) and last (10.45.0.255.255) addresses to be reserved as network address of the subnet and its broadcast address respectively.

So that field should be fixed and configured by the user as 10.45.0.0/16, like we were doing in our osmo-epdg faulty setup.

Now, if you also want to assign an IP address within the subnet to the tundev (which is fine for me too, it has its utility like pinging the end of the tunnel), I see you have 2 ways:

  1. Implicitly take the 1st non-reserved IP address of the subnet (10.45.0.0.1 in this case) and avoid getting it allocated to any UE.
  2. Explicitly add a config option to set the tun IP address, so the user can pick whatever out of 10.45.0.1, and again avoid getting that IP address from the pool assigned to any UE.

I am fine with any of the 2 above. In osmo-ggsn we use approach number 2, which allows further flexibility (see "ip ifconfig" being used to assign the GGSN IP address within the tunnel):

 apn internet
  gtpu-mode tun
  tun-device tun4
  type-support v4
  ip prefix dynamic 172.16.222.0/24
  ip dns 0 8.8.8.8
  ip dns 1 8.8.4.4
  ip ifconfig 172.16.222.0/24
  no shutdown
 apn inet6
  gtpu-mode tun
  tun-device tun6
  type-support v6
  ipv6 prefix dynamic 2001:780:44:2000:0:0:0:0/56
  ipv6 dns 0 2001:4860:4860::8888
  ipv6 dns 1 2001:4860:4860::8844
  ipv6 ifconfig 2001:780:44:2000:0:0:0:0/56
  no shutdown
 apn inet46
  gtpu-mode tun
  tun-device tun46
  type-support v4v6
  ip prefix dynamic 172.16.46.0/24
  ip dns 0 8.8.8.8
  ip dns 1 8.8.4.4
  ip ifconfig 172.16.46.0/24
  ipv6 prefix dynamic 2001:780:44:2100:0:0:0:0/56
  ipv6 dns 0 2001:4860:4860::8888
  ipv6 dns 1 2001:4860:4860::8844
  ipv6 ifconfig 2001:780:44:2100:0:0:0:0/56
  no shutdown

I'm using 10.45.0.1/16 instead of 10.45.0.0/16. I'm assuming that 10.45.0.1 is the IP set in the TUN, and I'm making sure that 10.45.0.1 is not set when setting the IP in the UE Pool.

The relevant code in lib/pfcp/context.c is shown below.

2003
2004                 /* Exclude TUN IP Address */
2005                 if (memcmp(ue_ip->addr, subnet->gw.sub, maxbytes) == 0)
2006                     continue;
2007

That's fine, but then we need to fix the subnet stuff I mentioned above.

@pespin
Copy link
Contributor Author

pespin commented Feb 19, 2024

@acetcom see how in osmo-ggsn we actually allow allocating the network ip address (.0) and also probably the broadcast address (.255), since those are anyway no really used in the tunnel network created/managed by the GGSN.
And hence why we assign the first IP address (.0) to the GGSN, while letting 1-255 for the UEs.

So I'd propose doing the same here:

  • Set cfg file "subnet" to "10.45.0.0/16".
  • Set new cfg file "address" to " 10.45.0.0/16" or " 10.45.0.1/16" (depending on which IP address do you want to assign to the tun).
  • Update smf code to blacklist UE IP assignmen from the pool for the IP address in cfg file option "address" from above.

@acetcom
Copy link
Member

acetcom commented Feb 23, 2024

@pespin

Basically, I agree with you, using the name subnet and including the gateway(or router) information in there was probably a bad choice.

I want to add a new router directive to the subnet as below.

  session:
    - subnet: 10.45.0.0/16
       router: 10.45.0.1
    - subnet: 2001:db8:cafe::0/48
       router: 2001:db8:cafe::1

For backwards compatibility, I can set a gateway(or router) information to a subnet for a period of time as below.

  session:
    - subnet: 10.45.0.1/16
    - subnet: 2001:db8:cafe::1/48

And also, subnet will be used without router information.

  session:
    - subnet: 10.45.0.0/16
    - subnet: 2001:db8:cafe::0/48

I don't think we need to support blacklist because we can use range as shown below.

  session:
    - subnet: 10.45.0.0/16
      router: 10.45.0.1
      range:
        - 10.45.0.100-10.45.0.200
        - 10.45.1.100-
        - -10.45.0.200
    - subnet: 2001:db8:cafe::0/48
      router: 2001:db8:cafe::1
      range:
        - 2001:db8:cafe:a0::0-2001:db8:cafe:b0::0
        - 2001:db8:cafe:c0::0-2001:db8:cafe:d0::0

I'll use the subnet setting, which includes router, as the default for deploying packages, so I want to leave the systemd setting as well.

Let me know if you have any objections.
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Feb 23, 2024

I want to add a new router directive to the subnet as below.

I'd better name it simply "address" or "gateway", I think it will be clearer IMHO.

For backwards compatibility, I can set a gateway(or router) information to a subnet for a period of time as below.

Not sure what you mean here. You plan to keep the old behavior for a while? You mean that for a while if no "address" is specified you will still implicitly take .1 as "address"?

I don't think we need to support blacklist because we can use range as shown below.

Oh I didn't know about that "range" support, nice. AFAIU that configures explicitly the available IP addresses within the subnet which are available to be assigned to UEs?
Still, I think if the user sets the "address" config, then that IP address has to be implicitly excluded from the UE IP address pool, otherwise it will end up assigned to an UE and to the tundev, which causes problems as described in the initial comment of this ticket. Otherwise the user would need to explicitly set a range excluding that IP address, which is a bit redundant and prone to error (or prone to missing doing that and ending up with broken setup).

I'll use the subnet setting, which includes router, as the default for deploying packages, so I want to leave the systemd setting as well.
If you want to add the "address" in the config by default, I'm fine with it. So you change the "subnet" to ".0" and add the "address" as ".1" correct?
Still, I think it makes sense to update the .network systemd file with the "[Route]" sections I submitted here. Since you also want to add the "address", then also keep the section "[Network]" which I removed.

Let me now your thoughts @acetcom .

@acetcom
Copy link
Member

acetcom commented Feb 23, 2024

@pespin

If router is not appropriate, I would use gateway, so the default settings in the package distribution or on github would be.

  session:
    - subnet: 10.45.0.0/16
       gateway: 10.45.0.1
    - subnet: 2001:db8:cafe::0/48
       gateway: 2001:db8:cafe::1

I don't always implicitly use .1 for backwards compatibility: if we used subnet like this, we would exclude .5.

  session:
    - subnet: 10.45.0.5/16
    - subnet: 2001:db8:cafe::5/48

This means that for the time being, I will allow subnets to contain .x as before, without using gateway for backwards compatibility.

Please let me know if you still don't understand my intent.

Thanks a lot!
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Feb 23, 2024

@acetcom Ah ok, now I think I understand what you mean:

  • If subnet ends in all zeros (taking into account the prefix), then the new behavior is applied, where one must specify a "gateway" IP address which is the one being set on the tundev. This IP address is excluded from the UE IP address pool.
  • If subnet ends with something != 0 (taking into account the prefix), the the old behavior is assumed, where the tundev IP address is the exact same value passed in "subnet", and the UE IP adress pool range is assumed to be 0-255 (excluding the IP address aplied on the tundev).

Fine with me. If possible add a warning message if the backward-compatible way is used (the suffix is not all 0), telling the user to move the config to the new format.

@acetcom
Copy link
Member

acetcom commented Feb 23, 2024

@pespin

Now you're on the same page as me. Giving warning messages is a good idea.

However, I have a lot of other things on my plate, so I don't plan to work on this fix right now. When I have more time, and if someone hasn't already done so, I may attempt to fix this issue at that time.

Thank a lot!
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Feb 24, 2024

@acetcom I have more urgent topics to work on in order to get osmo-epdg working with open5gs, so I will prioritize those first.
For instance, over next days I plan to add support in open5gs-smfd for APCO IE in GTPv2C which is used in S2b interface to get DNS and P-CSCF addresses.
I already started adding a ttcn3 test for S2b interface against open5gs-smfd here: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/36075

acetcom added a commit that referenced this pull request Apr 13, 2024
The way subnet is set up has changed as shown below.

```
<OLD Format>
smf:
  session:
    - subnet: 10.45.0.1/16

<NEW Format>
smf:
  session:
    - subnet: 10.45.0.0/16
      gateway: 10.45.0.1
```

For more information, please refer to Pull Request #2975.
acetcom added a commit that referenced this pull request Apr 13, 2024
The way subnet is set up has changed as shown below.

```
<OLD Format>
smf:
  session:
    - subnet: 10.45.0.1/16

<NEW Format>
smf:
  session:
    - subnet: 10.45.0.0/16
      gateway: 10.45.0.1
```

For more information, please refer to Pull Request #2975.
@acetcom
Copy link
Member

acetcom commented Apr 13, 2024

@pespin

As you suggested, I've changed the way we set up subnets on the main branch as follows.

<OLD Format>
smf:
  session:
    - subnet: 10.45.0.1/16

<NEW Format>
smf:
  session:
    - subnet: 10.45.0.0/16
      gateway: 10.45.0.1

Please let me know if you have any different idea.

Thanks a lot!
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Apr 13, 2024

@acetcom I may be wrong, but from looking only at your changes, I think it is still not possible to define a pool of addresses with no address assigned to the tundev?

See 3 examples:

1- "subnet: 10.45.0.1/16"
Since subnet mask "10.45.0.1 & ~255.255.0.0 != 0", then 10.45.0.0/16 (10.45.0.1 & 255.255.0.0) is the subnet defining the pool of EUAs, and "10.45.0.1" is the gateway IP address.

2- "subnet: 10.45.0.0/16
gateway: 10.45.0.1"
Since subnet mask "10.45.0.0 & ~255.255.0.0 == 0", then 10.45.0.0/16 is the subnet defining the pool of EUAs, and "10.45.0.1" (from the "gateway" attribute) is the gateway IP address.

3- "subnet: 10.45.0.0/16"
Since subnet mask "10.45.0.0 & ~255.255.0.0 == 0", then 10.45.0.0/16 is the subnet defining the pool of EUAs, and since there's no gateway IP address set (no "gateway" attribute), then no IP address is reserved from the pool of addresses.

I have the feeling, with your changes, you now support cases "1" and "2", but most probably case "3" is still being handled as case "1" where IP address "10.45.0.0" is assigned to the tundev.

Remember the initial point of this ticket was to allow setting up open5gs PGW without need to assign one IP address from the pool to the tundev. Splitting the "subnet" and "gateway" attribute is a first correct step, but that alone is not enough.

acetcom added a commit that referenced this pull request Apr 14, 2024
@acetcom
Copy link
Member

acetcom commented Apr 14, 2024

@pespin

Here are the results of my experiment. Note that Open5GS runs can be traced with the -t option.

  1. subnet 10.45.0.1/16
smf:
 session:
    - subnet: 10.45.0.1/16

$ open5gs-smfd -t
...
04/14 09:29:01.671: [sock] DEBUG: addr:127.0.0.7, port:8805 (../lib/core/ogs-sockaddr.c:143)
04/14 09:29:01.671: [pfcp] WARNING: Please change the configuration files of smf.yaml and upf.yaml as below. (../lib/pfcp/context.c:2199)

<OLD Format>
smf:
  session:
    - subnet: 10.45.0.1/16

<NEW Format>
smf:
  session:
    - subnet: 10.45.0.0/16
      gateway: 10.45.0.1


04/14 09:29:01.671: [sock] DEBUG: addr:127.0.0.4, port:7777 (../lib/core/ogs-sockaddr.c:143)
04/14 09:29:01.671: [sock] DEBUG: addr:127.0.0.200, port:7777 (../lib/core/ogs-sockaddr.c:143)
04/14 09:29:01.671: [sock] DEBUG: addr:127.0.0.4, port:9090 (../lib/core/ogs-sockaddr.c:143)
04/14 09:29:01.671: [pfcp] TRACE: [0] - 2002d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:29:01.671: [pfcp] TRACE: [1] - 3002d0a:0:0:0 (../lib/pfcp/context.c:2022)
...
04/14 09:29:01.714: [pfcp] TRACE: [4093] - ff0f2d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:29:01.714: [pfcp] TRACE: [4094] - 102d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:29:01.714: [pfcp] TRACE: [4095] - 1102d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:29:01.714: [metrics] INFO: metrics_server() [http://127.0.0.4]:9090 (../lib/metrics/prometheus/context.c:299)
04/14 09:29:01.714: [diam] TRACE: [1] (Thread 0x750c23455980 named 'Main')
...
  1. subnet 10.45.0.0/16 with gateway 10.45.0.1
smf:
 session:
    - subnet: 10.45.0.0/16
      gateway: 10.45.0.1

$ open5gs-smfd -t
...
04/14 09:30:40.883: [sock] DEBUG: addr:127.0.0.200, port:7777 (../lib/core/ogs-sockaddr.c:143)
04/14 09:30:40.883: [sock] DEBUG: addr:127.0.0.4, port:9090 (../lib/core/ogs-sockaddr.c:143)
04/14 09:30:40.883: [pfcp] TRACE: [0] - 2002d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:30:40.883: [pfcp] TRACE: [1] - 3002d0a:0:0:0 (../lib/pfcp/context.c:2022)
...
04/14 09:30:40.927: [pfcp] TRACE: [4092] - fe0f2d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:30:40.927: [pfcp] TRACE: [4093] - ff0f2d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:30:40.927: [pfcp] TRACE: [4094] - 102d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:30:40.927: [pfcp] TRACE: [4095] - 1102d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:30:40.927: [metrics] INFO: metrics_server() [http://127.0.0.4]:9090 (../lib/metrics/prometheus/context.c:299)
...
  1. subnet 10.45.0.0/16
smf:
 session:
    - subnet: 10.45.0.0/16

$ open5gs-smfd -t
...
04/14 09:25:50.301: [sock] DEBUG: addr:127.0.0.4, port:9090 (../lib/core/ogs-sockaddr.c:143)
04/14 09:25:50.301: [pfcp] TRACE: [0] - 1002d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:25:50.301: [pfcp] TRACE: [1] - 2002d0a:0:0:0 (../lib/pfcp/context.c:2022)
...
04/14 09:27:36.318: [pfcp] TRACE: [4094] - ff0f2d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:27:36.318: [pfcp] TRACE: [4095] - 102d0a:0:0:0 (../lib/pfcp/context.c:2022)
04/14 09:27:36.318: [metrics] INFO: metrics_server() [http://127.0.0.4]:9090 (../lib/metrics/prometheus/context.c:299)
...

The numbers 1 and 2 are TRACE: [0] - 2002d0a:0:0:0:0:0:0, which is assigned a UE IP from 10.45.0.2, number 3 is TRACE: [0] - 1002d0a:0:0:0:0:0, which is assigned a UE IP starting at 10.45.0.1.

Please let me know if you have any concerns.

Thanks a lot!
Sukchan

@pespin
Copy link
Contributor Author

pespin commented Apr 15, 2024

@acetcom Hi, that looks good then, thanks for confirming.

I am thinking about submitting a new version of this PR, where we add explicit routing rules, but this time keeping the setting of the IP address which you mentioned you want to have as a default setup.

The point of doing so is that if somebody wants to get rid of the allocated IP address for the tundev, one can simply comment out the "Address=" lines in the systemd file.

With the current systemd .network file, if one removes the "Address=" lines and does not do more stuff (the lines being added in this PR), then open5gs PGW will not work properly because routing will not be set.

Right now it is working because when an IP address is set, the kernel implicitly sets local network routing rules.
So we can add those local routing rules explicitly like I'm doing in this PR.
Are you fine with this? Feel free to test it yourself if you want (apply the PR but without the changes removing lines).

@acetcom
Copy link
Member

acetcom commented Apr 16, 2024

@pespin

That's a really good idea. Please resubmit the PR with the added routing table settings

Thanks a lot!
Sukchan

…ddresses

When running the open5gs package with systemd network config, the 1st IP address
of the UE pool configured in open5gs-upfd config file for ogstun is
being assigned to the interface through this file.
That was discussed as being a desirable default setup.

However, in the event a user wants a setup where no IP address is
assigned to the tundev, then it's not enough removing the IP address,
because then the implicit routing rules regarding the subnet of the IP
address added automatically by the kernel are also removed.

This patch adds config sections to set up the routing explicitly, with
the aim to get the routing still applied if the user decides to comment
out the IP address, so that packets are still forwarded properly in that
case.

Related: https://osmocom.org/issues/6361
@acetcom acetcom merged commit c0a520f into open5gs:main Apr 18, 2024
5 checks passed
@pespin pespin deleted the pespin/systemd branch April 22, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants