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

DHCP: No longer set interface mtu #346

Merged
merged 1 commit into from
Jul 29, 2024
Merged

DHCP: No longer set interface mtu #346

merged 1 commit into from
Jul 29, 2024

Conversation

rsmarples
Copy link
Member

We've been enforcing an interface MTU that is slightly larger than the minimum for some time.
Instead, log an error than the MTU is smaller than the minimum to send a BOOTP message.

The DHCP MTU is only used when adding routes as setting the interface MTU can cause a PHY reset which is bad.

Fixes #345

We've been enforcing an interface MTU that is slightly larger
than the minimum for some time.
Instead, log an error than the MTU is smaller than the minimum
to send a BOOTP message.

The DHCP MTU is only used when adding routes as setting the
interface MTU can cause a PHY reset which is bad.

Fixes #345
@rsmarples rsmarples merged commit 5ac1235 into master Jul 29, 2024
2 of 5 checks passed
@rsmarples rsmarples deleted the dhcp_min_mtu branch July 29, 2024 15:17
@mtremer
Copy link

mtremer commented Sep 5, 2024

Hello Roy,

thank you for your work on this. At IPFire, we are happy long-term users of dhcpcd, but I am not sure if we need to make some changes due to this patch.

The DHCP MTU is only used when adding routes as setting the interface MTU can cause a PHY reset which is bad.

I agree that resetting the PHY is a problem. We have seen this here and there over time, but it has not been a major problem for our user base in a while.

In theory we can live with setting the MTU when creating the routes, but I wanted to check with you whether there are some follow-up problems that might need addressing:

  • Path MTU discovery and clamp MSS to Path MTU are two things that we are using here or there. I always assumed that the kernel will use the interface's MTU and make according decisions. Since it now has to look up the route (which it would have to do anyways to find the egress interface) this will work fine (https://git.ipfire.org/?p=thirdparty/kernel/linux.git;a=blob;f=net/netfilter/xt_TCPMSS.c;h=116a885adb3cd6e93f82ee13f988e2dc0bee29f1;hb=c763c43396883456ef57e5e78b64d3c259c4babc#l62). In user space (i.e. OpenVPN) I don't know. We are not a big user of this as Path MTU discovery doesn't work well in practise, but I wanted to flag that this might potentially become a problem.
  • Additionally we allow users to configure static routes. Those might point at a gateway on the network that is configured with DHCP. Since we don't know the MTU, we can only configure them in the classic way without the MTU and we might send out packets where this route matches and therefore the requested MTU is not applied. Setting the MTU on there interface guaranteed before that we will never send any packets larger than the MTU no matter what.
  • Last but not least, we use QoS in form of CAKE and fq_codel. Both set up themselves using the device MTU which will now be different from what is actually being used. The crucial function is psched_mtu (https://git.ipfire.org/?p=thirdparty/kernel/linux.git;a=blob;f=include/net/pkt_sched.h;hb=c763c43396883456ef57e5e78b64d3c259c4babc#l137) which for fq_codel will set the quantum parameter and for CAKE will be used to compute the packet overhead due to fragmentation. Miscalculating those values will make QoS less efficient the more the route MTU differs from the interface MTU.
  • Maybe there are more things. I could think of IPsec which also encapsulates packets and therefore wants to know the MTU, but since that is living in the kernel, this should be fine using the route option. I did not check this.

I would like to see if we can either revert this change, make it configurable or at least export the value to the shell script so that we can potentially adjust anything manually. I agree that the PHY reset is undesirable, but is that not rather a driver problem that needs to be fixed instead of implementing this workaround which has these disadvantages?

I am happy to work on the implementation but before I start, I would like to hear your thoughts on this.

Up to the latest release, we had this patch in place (https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/patches/dhcpcd-10.0.2-Allow-free-selection-of-MTU-by-the-user.patch;h=69a35daf5036ad388204f010dcb950d49e4bcc93;hb=4c672e3b9692927d4d3319cb25283098b9075a46).

Best,
-Michael

@rsmarples
Copy link
Member Author

Hi Michael

thank you for your work on this. At IPFire, we are happy long-term users of dhcpcd, but I am not sure if we need to make some changes due to this patch.

Glad you're long term uses, always happy to make changes.

The DHCP MTU is only used when adding routes as setting the interface MTU can cause a PHY reset which is bad.

I agree that resetting the PHY is a problem. We have seen this here and there over time, but it has not been a major problem for our user base in a while.

In theory we can live with setting the MTU when creating the routes, but I wanted to check with you whether there are some follow-up problems that might need addressing:

OK, before we talk about these points, dhcpcd has not set the interface MTU for almost 10 years now.
ca6cdf5

* Path MTU discovery and clamp MSS to Path MTU are two things that we are using here or there. I always assumed that the kernel will use the interface's MTU and make according decisions. Since it now has to look up the route (which it would have to do anyways to find the egress interface) this will work fine (https://git.ipfire.org/?p=thirdparty/kernel/linux.git;a=blob;f=net/netfilter/xt_TCPMSS.c;h=116a885adb3cd6e93f82ee13f988e2dc0bee29f1;hb=c763c43396883456ef57e5e78b64d3c259c4babc#l62). In user space (i.e. OpenVPN) I don't know. We are not a big user of this  as Path MTU discovery doesn't work well in practise, but I wanted to flag that this might potentially become a problem.

I can't speak for Linux, but in BSD when we need to look at any MTU we always use the lowest of the outbound route or the interface MTU. We always have a route and an interface, so there is no extra lookup needed.

* Additionally we allow users to configure static routes. Those might point at a gateway on the network that is configured with DHCP. Since we don't know the MTU, we can only configure them in the classic way without the MTU and we might send out packets where this route matches and therefore the requested MTU is not applied. Setting the MTU on there interface guaranteed before that we will never send any packets larger than the MTU no matter what.

The user can set a static mtu as well, which should be applied to the routes dhcpcd configures.

* Last but not least, we use QoS in form of CAKE and fq_codel. Both set up themselves using the device MTU which will now be different from what is actually being used. The crucial function is psched_mtu (https://git.ipfire.org/?p=thirdparty/kernel/linux.git;a=blob;f=include/net/pkt_sched.h;hb=c763c43396883456ef57e5e78b64d3c259c4babc#l137) which for fq_codel will set the quantum parameter and for CAKE will be used to compute the packet overhead due to fragmentation. Miscalculating those values will make QoS less efficient the more the route MTU differs from the interface MTU.

That sounds like a CAKE issue?
Anyway, the important thing is that DHCP MTU now behaves exactly like IPv6 in-kernel RA MTU which only sets MTU on routes.
So if it is a CAKE issue then it needs to be resolved outside of dhcpcd for kernel IPv6 RA MTU anyway.

* Maybe there are more things. I could think of IPsec which also encapsulates packets and therefore wants to know the MTU, but since that is living in the kernel, this should be fine using the route option. I did not check this.

See above for CAKE.

I would like to see if we can either revert this change, make it configurable or at least export the value to the shell script so that we can potentially adjust anything manually. I agree that the PHY reset is undesirable, but is that not rather a driver problem that needs to be fixed instead of implementing this workaround which has these disadvantages?

I am happy to work on the implementation but before I start, I would like to hear your thoughts on this.

This change just removed changing the MTU to the "minimum" size needed for DHCP.
It never changed the interface MTU for the MTU from a DHCP message. That was removed over 10 years ago as I said earlier.

Up to the latest release, we had this patch in place (https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/patches/dhcpcd-10.0.2-Allow-free-selection-of-MTU-by-the-user.patch;h=69a35daf5036ad388204f010dcb950d49e4bcc93;hb=4c672e3b9692927d4d3319cb25283098b9075a46).

I don't think your patch is needed anymore as we now accept UINT_MAX MTU which is bigger than what you changed to.

@mtremer
Copy link

mtremer commented Sep 5, 2024

Glad you're long term uses, always happy to make changes.

Currently we would not require anything as dhcpcd has been working like a charm :)

OK, before we talk about these points, dhcpcd has not set the interface MTU for almost 10 years now. ca6cdf5

Oh, this is a lot older than I remembered. I do recall that this change gave us some trouble and I thought that we still had some sort of workaround that we would employ to change the interface MTU.

Granted, I don't think that there are many networks that run on anything else but 1500. There might be some fibre modems that encapsulate packets later on and might want 1492 or even smaller, but I don't believe these things are very common. Even then, connections should just work even with 1500 if a router further down the line performs any fragmentation.

I must have misread the commit then. Regardless, we have discovered some very minor problem here...

  • Additionally we allow users to configure static routes. Those might point at a gateway on the network that is configured with DHCP. Since we don't know the MTU, we can only configure them in the classic way without the MTU and we might send out packets where this route matches and therefore the requested MTU is not applied. Setting the MTU on there interface guaranteed before that we will never send any packets larger than the MTU no matter what.

The user can set a static mtu as well, which should be applied to the routes dhcpcd configures.

Yes, we have an option for that and I do remember testing this with the changes from ~10 years ago. The static routes are just set by a different script in a different route table. This can all be changed since in this case we know the MTU from the user. My problem is that I don't seem to have a way to find the MTU when it has been changed by the DHCP server. The variable interface_mtu= is empty in the script (or I am too dumb to test things right or I have a cable modem that does not specify a MTU).

That sounds like a CAKE issue? Anyway, the important thing is that DHCP MTU now behaves exactly like IPv6 in-kernel RA MTU which only sets MTU on routes. So if it is a CAKE issue then it needs to be resolved outside of dhcpcd for kernel IPv6 RA MTU anyway.

This is a very strong point that I did not even consider. Thank you for this, I will reach out to the CAKE people and ask if they think that this might cause a problem.

I would like to see if we can either revert this change, make it configurable or at least export the value to the shell script so that we can potentially adjust anything manually. I agree that the PHY reset is undesirable, but is that not rather a driver problem that needs to be fixed instead of implementing this workaround which has these disadvantages?
I am happy to work on the implementation but before I start, I would like to hear your thoughts on this.

This change just removed changing the MTU to the "minimum" size needed for DHCP. It never changed the interface MTU for the MTU from a DHCP message. That was removed over 10 years ago as I said earlier.

We needed this because there was only a small corridor for the MTU and I remember that this patch was needed to virtually allow the user to pick whatever they want.

So, thank you for your help for now. I will reach out to the CAKE people and see if this is actually a problem here or if there is a way to get around this.

@rsmarples
Copy link
Member Author

Yes, we have an option for that and I do remember testing this with the changes from ~10 years ago. The static routes are just set by a different script in a different route table. This can all be changed since in this case we know the MTU from the user. My problem is that I don't seem to have a way to find the MTU when it has been changed by the DHCP server. The variable interface_mtu= is empty in the script (or I am too dumb to test things right or I have a cable modem that does not specify a MTU).

You should see old and new values for mtu when changed by DHCP.
I see this in my testing.

reason=BOUND
interface=vtnet0
interface_mtu=1492

This change just removed changing the MTU to the "minimum" size needed for DHCP. It never changed the interface MTU for the MTU from a DHCP message. That was removed over 10 years ago as I said earlier.

We needed this because there was only a small corridor for the MTU and I remember that this patch was needed to virtually allow the user to pick whatever they want.

And that corridor has been removed, the user is free to pick whatever.

So, thank you for your help for now. I will reach out to the CAKE people and see if this is actually a problem here or if there is a way to get around this.

Welcome.
But open a new issue if needed.
You can reference this if needed.

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.

MTU_MIN, MTU_MAX
2 participants