-
Notifications
You must be signed in to change notification settings - Fork 121
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
Conversation
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
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.
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:
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, |
Hi Michael
Glad you're long term uses, always happy to make changes.
OK, before we talk about these points, dhcpcd has not set the interface MTU for almost 10 years now.
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.
The user can set a static mtu as well, which should be applied to the routes dhcpcd configures.
That sounds like a CAKE issue?
See above for CAKE.
This change just removed changing the MTU to the "minimum" size needed for DHCP.
I don't think your patch is needed anymore as we now accept UINT_MAX MTU which is bigger than what you changed to. |
Currently we would not require anything as dhcpcd has been working like a charm :)
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...
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).
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.
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. |
You should see old and new values for mtu when changed by DHCP.
And that corridor has been removed, the user is free to pick whatever.
Welcome. |
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