Page MenuHomeFreeBSD

dhclient: allow to supersede interface-mtu option
ClosedPublic

Authored by novel on May 18 2018, 7:22 PM.

Details

Summary

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.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

novel created this revision.May 18 2018, 7:22 PM
cem accepted this revision.May 18 2018, 8:58 PM
cem added inline comments.
dhclient/dhclient.c
839–841 ↗(On Diff #42708)

Maybe we should still warn if the server sends zero, vs the user configuring it. But I don't feel strongly about it.

dhclient/dhclient.conf.5
234–235 ↗(On Diff #42708)

It doesn't seem clear to me from the text that the special zero value only applies if supersede is specified; also it seems a little weird to put it in the "OPTION MODIFIERS" section.

I'd suggest:

.Pp
Some options have special values:
.Bl -tag -width indent
.It Ar interface-mtu
Any server-supplied interface MTU is ignored by the client if a
.Ic supersede
zero value is configured.

But maybe get someone from docs to offer advice.

This revision is now accepted and ready to land.May 18 2018, 8:58 PM
novel updated this revision to Diff 42730.May 19 2018, 6:08 AM
novel added a reviewer: wblock.
This revision now requires review to proceed.May 19 2018, 6:08 AM
cem accepted this revision.May 19 2018, 6:11 AM
cem added inline comments.
dhclient/dhclient.c
843 ↗(On Diff #42730)

All of these parentheses seem excessive -- just if (!supersede || mtu == 0) would do.

This revision is now accepted and ready to land.May 19 2018, 6:11 AM
novel added a comment.May 19 2018, 6:31 AM

@cem I'd appreciate if you could get this committed for me. I guess it needs to be properly MFCed so it gets into 11.2, and given that timeline is tight and I'm not very familiar with the process, it'll be better to get it done right from the first attempt.

dhclient/dhclient.c
843 ↗(On Diff #42730)

This way it'll still print a warning for superseded value because 'mtu == 0' will be true.

novel updated this revision to Diff 42731.May 19 2018, 6:35 AM
This revision now requires review to proceed.May 19 2018, 6:35 AM
cem accepted this revision.May 19 2018, 7:00 AM

@cem I'd appreciate if you could get this committed for me.

Sure, no problem. I'd like to see some manpages/doc team feedback; hopefully that happens soon.

I guess it needs to be properly MFCed so it gets into 11.2, and given that timeline is tight and I'm not very familiar with the process, it'll be better to get it done right from the first attempt.

I don't MFC, but maybe someone can be recruited to do it.

dhclient/dhclient.c
843 ↗(On Diff #42730)

Ah, sorry, I meant '!= 0' and I see you got that right :-). LGTM.

This revision is now accepted and ready to land.May 19 2018, 7:00 AM
This revision was automatically updated to reflect the committed changes.