Bug 206721

Summary: FreeBSDs DHCP client(dhclient) does not support the interface-mtu option(option 26).
Product: Base System Reporter: Jarrod Petz <jlpetz>
Component: binAssignee: Conrad Meyer <cem>
Status: Closed FIXED    
Severity: Affects Some People CC: ae, cem, jhb, jimp, koobs, mmpestorich, net, novel, re
Priority: --- Keywords: feature
Version: CURRENTFlags: koobs: mfc-stable11+
koobs: mfc-stable10-
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D5675
See Also: https://reviews.freebsd.org/D4788
Attachments:
Description Flags
allow to supersede interface-mtu none

Description Jarrod Petz 2016-01-29 03:35:08 UTC
After getting the Intel ixv driver working for AWS instances. I noticed there was some severe packet loss in iPerf tests. Looking at ifconfig I suspected there was an MTU blackhole between the virtual ethernet interface(ixv) with MTU of 1500 and the physical card MTU 9001. Confirmed by manually changing it and a few ping tests with 'Don't fragment' flags set.

Changing the MTU manually corrected the packet loss. But then I was wondering why this wasn't automatically done like it is for other operating systems which I have run on AWS EC2.

Turn out it looks like EC2 sends the MTU which should be configured for the interface as part of the DHCP response and FreeBSDs default client.
1. Doesn't request the interface-mtu option(option 26). Needed for it to be in the response
2. Even if it did request it this option and got it in the response. I don't think it would process it and configure the MTU on the NIC. Based on the source code I looked at.

I was able to work around this by using the isc-dhcp43-client-4.3.3 package/port. But am logging this as a bug/feature-request as I feel this should be part of the base system. MTU blackholes are quite common when using jumbo frame(~9000 MTU) systems and this seems like a good way of avoiding them which a lot of other operating systems are doing by default.

More details about this are in the phabricator review which has my testing details(Link below). Make sure you click on the "Show Older Changes" to see all the text on the page below.
https://reviews.freebsd.org/D4788
Comment 1 Andrey V. Elsukov freebsd_committer freebsd_triage 2016-02-11 11:30:21 UTC
It is easy to add support of "interface-mtu" option to dhclient-script. But the one problem is exists. When you are changing the MTU of interface, this initiates its reset and it goes down, then up. This by turn initiates restart of dhclient. So, we get an endless loop.
Comment 2 Jarrod Petz 2016-02-15 06:25:07 UTC
(In reply to Andrey V. Elsukov from comment #1)

Perhaps that's part of the reason OpenBSD do this as an ioctl system call from the C dhclient program and not the shell script? Though Linux does it via the shell script so somehow they made it work.
Comment 3 John Baldwin freebsd_committer freebsd_triage 2016-03-30 16:31:32 UTC
(In reply to Andrey V. Elsukov from comment #1)
An ioctl won't make a difference as the issue is the call to if_init in the driver.  However, I thought that r239564 handled this case as dhclient should leave the interface configured while the link goes down but reuse that configuration (as it should still be valid) when the link is restored.
Comment 4 commit-hook freebsd_committer freebsd_triage 2016-09-02 21:14:50 UTC
A commit references this bug:

Author: cem
Date: Fri Sep  2 21:14:30 UTC 2016
New revision: 305306
URL: https://svnweb.freebsd.org/changeset/base/305306

Log:
  dhclient: add support for interface-mtu (26)

  Make dhclient set interface MTU if it was provided.

  This version implements MTU setting in dhclient itself before it runs
  dhclient-script.

  PR:		206721
  Submitted by:	novel@
  Reported by:	Jarrod Petz <jlpetz at gmail.com>
  Reviewed by:	cem, allanjude
  Differential Revision:	https://reviews.freebsd.org/D5675

Changes:
  head/sbin/dhclient/clparse.c
  head/sbin/dhclient/dhclient.c
  head/sbin/dhclient/dhcpd.h
  head/sbin/dhclient/dispatch.c
  head/sbin/dhclient/privsep.c
  head/sbin/dhclient/privsep.h
Comment 5 Conrad Meyer freebsd_committer freebsd_triage 2016-09-02 21:18:44 UTC
FreeBSD's dhclient now supports interface-mtu :-).
Comment 6 Ben Woods freebsd_committer freebsd_triage 2016-09-02 23:04:40 UTC
*** Bug 208392 has been marked as a duplicate of this bug. ***
Comment 7 commit-hook freebsd_committer freebsd_triage 2018-03-19 04:16:51 UTC
A commit references this bug:

Author: eadler
Date: Mon Mar 19 04:16:38 UTC 2018
New revision: 331179
URL: https://svnweb.freebsd.org/changeset/base/331179

Log:
  MFC r305306:

  dhclient: add support for interface-mtu (26)

  Make dhclient set interface MTU if it was provided.

  This version implements MTU setting in dhclient itself before it runs
  dhclient-script.

  PR:		206721

Changes:
_U  stable/11/
  stable/11/sbin/dhclient/clparse.c
  stable/11/sbin/dhclient/dhclient.c
  stable/11/sbin/dhclient/dhcpd.h
  stable/11/sbin/dhclient/dispatch.c
  stable/11/sbin/dhclient/privsep.c
  stable/11/sbin/dhclient/privsep.h
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2018-03-19 11:40:38 UTC
Assign to committer that resolved. Thanks Conrad!

Also track MFC's correctly, currently only to stable/11

Can stable/10 not receive it?
Comment 9 Rodney W. Grimes freebsd_committer freebsd_triage 2018-03-19 13:56:37 UTC
Put back on freebsd-net@ notification list
Comment 10 jimp 2018-05-18 15:24:55 UTC
There does not appear to be a way to disable or override this behavior, even using supersede in the dhclient.conf doesn't let you turn it off or set an alternate value. My ISP sends a bogus MTU value of 576 which was ignored before, but now gets set and leads to major connectivity issues since the MTU should be 1500.

The link bounces when the MTU is set as well, at least on e1000. I see dhclient was changed to help cope with that but it also affects other things that key off link up/down events.

Any chance this can be reopened and the change backed out until it can be made optional, perhaps by a command line parameter to dhclient? This will probably impact a number of people unexpectedly on upgrade.
Comment 11 Roman Bogorodskiy freebsd_committer freebsd_triage 2018-05-18 17:10:46 UTC
Created attachment 193518 [details]
allow to supersede interface-mtu
Comment 12 Roman Bogorodskiy freebsd_committer freebsd_triage 2018-05-18 17:13:15 UTC
(In reply to jimp from comment #10)

I've attached a quick patch (for stable/11) that allows to use supersede for interface-mtu.

Works for me with config like:

interface "vtnet0" {                                                                                                                                                                                               
    supersede interface-mtu 1496;                                                                                                                                                                                  
}
Comment 13 jimp 2018-05-18 17:23:36 UTC
Thanks, I'll look into giving that a try but it won't be until next week most likely. From the look of it, if I set a supersede of 0 it should disable setting the MTU entirely, right?
Comment 14 Conrad Meyer freebsd_committer freebsd_triage 2018-05-18 17:24:49 UTC
(In reply to jimp from comment #13)
Yeah, but unfortunately it will produce that MIN_MTU warning.  Ideally we provide a sentinel value that doesn't warn but also doesn't override.
Comment 15 Roman Bogorodskiy freebsd_committer freebsd_triage 2018-05-18 17:43:25 UTC
(In reply to Conrad Meyer from comment #14)

One more possible workaround for this situation is to specify exact fields a client wants to receive using 'request' in /etc/dhclient.conf, OTOH, server could ignore that and force some options.

We can probably treat 0 as such an sentinel value, but I guess it doesn't look obvious for users.
Comment 16 jimp 2018-05-18 17:45:40 UTC
(In reply to Roman Bogorodskiy from comment #15)
I tried that first, but the server sends it unsolicited in my case. That makes it a pain since it's still respected in the client even when it wasn't asked to retrieve that value.
Comment 17 Conrad Meyer freebsd_committer freebsd_triage 2018-05-18 18:11:29 UTC
(In reply to Roman Bogorodskiy from comment #15)
Yep.  I think 0 is an ok sentinel, just add it to appropriate manual page(s).  I am happy to review or commit such a change promptly, just ping me.
Comment 18 Roman Bogorodskiy freebsd_committer freebsd_triage 2018-05-18 19:33:30 UTC
(In reply to Conrad Meyer from comment #17)

I've created a review request: https://reviews.freebsd.org/D15484
That's an early version before going to bed, I'll re-check it tomorrow.
Comment 19 jimp 2018-05-21 12:22:23 UTC
The patch is working great, thanks! I set a supersede of 0 and everything is back to normal.

One additional question: Since the new DHCP MTU behavior will be active for everyone, should the default dhclient configuration set a supersede of 0 to minimize the impact for users on upgrade?

Someone performing a remote upgrade could end up losing access to their installation in this situation.
Comment 20 Roman Bogorodskiy freebsd_committer freebsd_triage 2018-05-22 15:15:10 UTC
(In reply to jimp from comment #19)

Overall, I'd prefer the current behavior by default: there are environments where it's necessary to respect MTU value supplied by the (properly configured) DHCP server. So we basically either support properly configured DHCP servers or add a workaround for misconfigured DHCP servers. I think it's better to support proper configurations rather than trying to solve problems created elsewhere.

But loosing remote access because of that could be unfortunate indeed. I'm wondering how common is this type of misconfiguration? Maybe we can add a release note about that?
Comment 21 jimp 2018-05-22 15:29:05 UTC
(In reply to Roman Bogorodskiy from comment #20)
I see it less as "a workaround for misconfigured DHCP servers" and more as preserving current expected behavior. I didn't even know my ISP DHCP server was sending broken information until this change was active by default. I'm not sure how many others are in the same boat, but having them find out after the fact during an upgrade seems troublesome.

Ultimately, this is a new feature and not a bug fix. This new feature could potentially break current installations in unexpected ways (it certainly astonished me to find it broken). Preserving the existing and expected behavior is safe. If someone needs the new feature, IMO, they should have to make the change to activate it. It's a bit awkward that it has to be superseded to disable the change instead of going out of your way to enable it, but we're limited by what the dhclient configuration supports.

If it were entirely up to me, I would leave it out of the default request list and supersede it to 0, and if someone wants to use the DHCP MTU they can add it to the request list and remove the supersede value. Since it is not up to me, having an entry in the release notes would be OK as an alternative as long as it's prominent. It may even warrant a HEADS UP to lists to call attention to it for wider testing before 11.2-RELEASE.
Comment 22 commit-hook freebsd_committer freebsd_triage 2018-05-31 19:37:01 UTC
A commit references this bug:

Author: cem
Date: Thu May 31 19:36:25 UTC 2018
New revision: 334443
URL: https://svnweb.freebsd.org/changeset/base/334443

Log:
  dhclient(8): allow to supersede interface-mtu option

  In some cases broken DHCP servers might send invalid MTU value, so allow to
  use 'supersede' in dhclient.conf to override this. When superseded value is
  0, MTU value is not updated at all.

  PR:		206721
  Submitted by:	novel@
  Reported by:	<jimp AT pfsense.org>
  MFC after:	37 minutes (if you care about 11, please MFC to 11.2)
  Relnotes:	yes (potentially surprising behavior change w/ broken dhcpd mtu)
  Differential Revision:	https://reviews.freebsd.org/D15484

Changes:
  head/sbin/dhclient/dhclient.c
  head/sbin/dhclient/dhclient.conf.5
Comment 23 Kubilay Kocak freebsd_committer freebsd_triage 2018-06-01 03:52:46 UTC
Comment on attachment 193518 [details]
allow to supersede interface-mtu

Request merge to releng/11.2
Comment 24 Kubilay Kocak freebsd_committer freebsd_triage 2018-06-01 03:54:22 UTC
Comment on attachment 193518 [details]
allow to supersede interface-mtu

Remove request, this attachment/patch doesn't look like the second (followup) commit (base r334443)
Comment 25 Roman Bogorodskiy freebsd_committer freebsd_triage 2018-06-02 18:06:05 UTC
(In reply to Kubilay Kocak from comment #24)

I think it still makes sense to request MFC for r334443 as it fixes problem described in comment #10 (probably there's no separate PR for that issue).
Comment 26 commit-hook freebsd_committer freebsd_triage 2018-06-07 15:04:06 UTC
A commit references this bug:

Author: marius
Date: Thu Jun  7 15:03:48 UTC 2018
New revision: 334787
URL: https://svnweb.freebsd.org/changeset/base/334787

Log:
  MFC: r334443 (by cem@)

  dhclient(8): allow to supersede interface-mtu option

  In some cases broken DHCP servers might send invalid MTU value, so allow to
  use 'supersede' in dhclient.conf to override this. When superseded value is
  0, MTU value is not updated at all.

  PR:		206721
  Submitted by:	novel@
  Reported by:	<jimp AT pfsense.org>
  Relnotes:	yes (potentially surprising behavior change w/ broken dhcpd mtu)
  Differential Revision:	https://reviews.freebsd.org/D15484

Changes:
_U  stable/11/
  stable/11/sbin/dhclient/dhclient.c
  stable/11/sbin/dhclient/dhclient.conf.5
Comment 27 commit-hook freebsd_committer freebsd_triage 2018-06-07 15:51:45 UTC
A commit references this bug:

Author: marius
Date: Thu Jun  7 15:51:24 UTC 2018
New revision: 334789
URL: https://svnweb.freebsd.org/changeset/base/334789

Log:
  MFC: r334443 (by cem@) MF stable/11: r334787

  dhclient(8): allow to supersede interface-mtu option

  In some cases broken DHCP servers might send invalid MTU value, so allow to
  use 'supersede' in dhclient.conf to override this. When superseded value is
  0, MTU value is not updated at all.

  PR:		206721
  Submitted by:	novel@
  Reported by:	<jimp AT pfsense.org>
  Approved by:	re (gjb)
  Relnotes:	yes (potentially surprising behavior change w/ broken dhcpd mtu)
  Differential Revision:	https://reviews.freebsd.org/D15484

Changes:
_U  releng/11.2/
  releng/11.2/sbin/dhclient/dhclient.c
  releng/11.2/sbin/dhclient/dhclient.conf.5
Comment 28 Kubilay Kocak freebsd_committer freebsd_triage 2018-06-08 09:37:08 UTC
All commits and merges have been recorded, thank you Conrad & Marius.

This change will be included in 11.2-RELEASE (per base r334789)