dhclient: add support for interface-mtu (26)

[novel]

Summary

Make dhclient set interface MTU if it was provided.

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

Also, to be able to test this with devd, I've modified if_vtnet(4) to send LINK_(UP|DOWN) (inl. the case when MTU is being changed): https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209427

So far it works well in my setup...

[• ian]

dhclient-script

263 ↗

(On Diff #14425)

I think this line should probably be:

if [ -n "$new_interface_mtu" -a "$old_interface_mtu" != "$new_interface_mtu" ]; then

That should prevent spamming syslog with non-changes on lease renewal (but I don't have any easy way to test the change).

[novel]

Update the check to not set MTU if it didn't change on renewal.

[novel]

dhclient-script

263 ↗

(On Diff #14425)

Yeah, you're right. I've updated the diff and now it doesn't re-set MTU if not needed. I'm not sure how to trigger renewal with MTU change in my env either.

[ae]

When I tested similar patch I found the problem described here https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206721
I tested with em(4).

[novel]

In D5675#123498, @ae wrote:

When I tested similar patch I found the problem described here https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206721
I tested with em(4).

Hm... I'm testing this with vtnet(4). It looks something like this:

Mar 30 10:35:52 host-10-20-0-5 dhclient: New MTU (vtnet0): 1458
Mar 30 10:35:52 host-10-20-0-5 dhclient: New IP Address (vtnet0): 10.20.0.5
Mar 30 10:35:52 host-10-20-0-5 dhclient: New Subnet Mask (vtnet0): 255.255.255.0
Mar 30 10:35:52 host-10-20-0-5 dhclient: New Broadcast Address (vtnet0): 10.20.0.255
Mar 30 10:35:52 host-10-20-0-5 dhclient: New Routers (vtnet0): 10.20.0.1
Mar 30 10:35:52 host-10-20-0-5 dhclient: New Classless Static Routes (vtnet0):  169.254.169.254/32 10.20.0.1 default 10.20.0.1

I do see interface going up and down for a moment, however it does not cause dhclient restart.

E.g., you can see that pids are the same:

$ date
Wed Mar 30 10:41:43 UTC 2016
$ ps aux|grep dhclient|grep -v grep
root    45306   0.0  0.0   10588   2480  -  Ss   10:35       0:00.06 dhclient: vtnet0 [priv] (dhclient)
_dhcp   45328   0.0  0.0   10588   2660  -  Ss   10:35       0:00.01 dhclient: vtnet0 (dhclient)
$ date
Wed Mar 30 10:42:59 UTC 2016
$ ps aux|grep dhclient|grep -v grep
root    45306   0.0  0.0   10588   2480  -  Is   10:35       0:00.07 dhclient: vtnet0 [priv] (dhclient)
_dhcp   45328   0.0  0.0   10588   2660  -  Is   10:35       0:00.01 dhclient: vtnet0 (dhclient)
$

I don't see new records in logs either.

I run it either like "dhclient -d vtnet0" or "dhclient vtnet0". Maybe some additional steps are required to reproduce that?

[ae]

I think this is specific to each network driver. if_em(4) does hardware reset on SIOCSIFMTU. As I can see, ixgbe(4) also does the same and changing MTU initiates link state change.

Mar 30 15:16:46 test25 devd: Processing event '!system=IFNET subsystem=ix0 type=LINK_DOWN'
Mar 30 15:16:46 test25 devd: Pushing table
Mar 30 15:16:46 test25 devd: Processing notify event
Mar 30 15:16:46 test25 devd: Popping table
Mar 30 15:16:46 test25 devd: Processing event '!system=IFNET subsystem=ix0 type=LINK_UP'

[novel]

In D5675#123508, @ae wrote:

I think this is specific to each network driver. if_em(4) does hardware reset on SIOCSIFMTU. As I can see, ixgbe(4) also does the same and changing MTU initiates link state change.

Mar 30 15:16:46 test25 devd: Processing event '!system=IFNET subsystem=ix0 type=LINK_DOWN'
Mar 30 15:16:46 test25 devd: Pushing table
Mar 30 15:16:46 test25 devd: Processing notify event
Mar 30 15:16:46 test25 devd: Popping table
Mar 30 15:16:46 test25 devd: Processing event '!system=IFNET subsystem=ix0 type=LINK_UP'

Hm, yes, I don't see events like that from devd, though I can see ping interrupts for a moment:

64 bytes from 10.20.0.1: icmp_seq=2 ttl=64 time=0.435 ms
64 bytes from 10.20.0.1: icmp_seq=3 ttl=64 time=0.453 ms
ping: sendto: No route to host
ping: sendto: No route to host
64 bytes from 10.20.0.1: icmp_seq=6 ttl=64 time=0.467 ms
64 bytes from 10.20.0.1: icmp_seq=7 ttl=64 time=0.352 ms

BTW, I'm wondering if the issue you're seeing is caused by dhclient alone or because devd contains the dhclient start rule when interface goes up?

Anyway, I need to find some way to reproduce it myself...

On a related note, I'm wondering if vtnet(4) should spawn LINK_(UP|DOWN) events?

[cem]

LGTM other than some nits.

sbin/dhclient/dhclient.c

809–810

mtu = be16dec(opt->data); ?

811

magic number?

[novel]

Thanks for taking a look, Conrad. I've updated the diff based on your suggestions.

[cem]

sbin/dhclient/dhclient.c

8–9

mismerge?

16–17

int void looks like a mis-merge

29

(unsigned)mtu to match the %u format.

[cem]

Ignore the mismerge comments. I think phabricator is just confused by the lack of context. For reference, phabricator does better with full context (diff -U999999999).

[novel]

New diff fixing the %u (unsinged)mtu bit and preserving context.

[novel]

@ae could you please test the latest patch in your environment?

I've tested it, but I only can test it with vtnet, and also I have to use additional patch to make it send devctl events (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209427).

[novel]

Now when HEAD appears to be open, this change could probably go in. However, as I don't have src commit bit, it seems that I either need to get a explicit approval or somebody else committing this.

Anyway, now when bhyve got e1000 support, I can test thing change not only with my patches virtio-net, so I'll try to test it to re-confirm it addresses concerns outlined by @ae.

[allanjude]

In D5675#149197, @novel wrote:

Now when HEAD appears to be open, this change could probably go in. However, as I don't have src commit bit, it seems that I either need to get a explicit approval or somebody else committing this.

Anyway, now when bhyve got e1000 support, I can test thing change not only with my patches virtio-net, so I'll try to test it to re-confirm it addresses concerns outlined by @ae.

e1000 support went in yesterday: https://svnweb.freebsd.org/base?view=revision&revision=302504

[novel]

In D5675#149198, @allanjude wrote:

In D5675#149197, @novel wrote:

Now when HEAD appears to be open, this change could probably go in. However, as I don't have src commit bit, it seems that I either need to get a explicit approval or somebody else committing this.

Anyway, now when bhyve got e1000 support, I can test thing change not only with my patches virtio-net, so I'll try to test it to re-confirm it addresses concerns outlined by @ae.

e1000 support went in yesterday: https://svnweb.freebsd.org/base?view=revision&revision=302504

yeah, that's what I'm planning to use to test the devctl thing, because I had to patch virtio to send these events (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209427), but that's probably not the best testing environment...

[cem]

I can commit it for you, but I'd like wider review (i.e. someone other than me signing off on it) before I do so.

[Petz_nisshoko.net]

In D5675#149247, @cem wrote:

I can commit it for you, but I'd like wider review (i.e. someone other than me signing off on it) before I do so.

Hi,

This is something I'm just looking into again now. I have been after this since the ixv driver started working on AWS EC2
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206721

If testing by me would help get this in for 11, please let me know.

Regards Jarrod

[jhb]

I think the issues ae@ was referring to were resolved by having dhclient not drop the interface's configuration when the link goes down. Thus, during a link flap the interface remains configured and continues to work. (See r239564)

[cem]

I think we've missed the boat on 11.0. 12 is the current branch I would land this in. It could be MFCed for 11.1 by somebody interested. I'd still like someone else's Approval to commit, though :-).

[allanjude]

[novel]

Thanks!