if_gif(4): Support the NOCLAMP flag to change the MTU handling for IPv6
Needs ReviewPublic
Actions

Authored by meta on Mon, Jul 14, 8:25 AM.

Details

Reviewers

ae
pauamma_gundo.com

Group Reviewers

network
manpages

Summary

The gif(4) interface always used IPV6_MINMTU when the outer protocol was
IPv6 so that packets would not be dropped between the tunnel endpoints.
The interface MTU was ignored because path MTU discovery was too expensive.

This change adds NOCLAMP flag support to revert it to normal behavior,
and documents the backgrounds. While using IPV6_MINMTU is still
the safest option for IPv6, ignoring the configured interface MTU is
confuses the users, and using a larger MTU is sometimes useful.

This is originally developed by hrs@. I just made additional changes to
implement the feature via a named flag noclamp as ae@ suggested in D45854.
Also I updated the man page for the named flag. I named noclamp for the
flag but I'm open to other suggestions.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 65409
Build 62292: arc lint + arc unit

Event Timeline

meta created this revision.Mon, Jul 14, 8:25 AM

Herald added subscribers: ziaee, glebius, melifaro and 2 others. · View Herald TranscriptMon, Jul 14, 8:25 AM

meta requested review of this revision.Mon, Jul 14, 8:25 AM

Harbormaster completed remote builds in B65409: Diff 158455.Mon, Jul 14, 8:25 AM

meta added reviewers: network, manpages.Mon, Jul 14, 8:26 AM

meta added subscribers: emaste, zlei, kazubu_jtime.net, hrs.Mon, Jul 14, 8:29 AM

meta mentioned this in D45854: if_gif(4): Support the IFF_LINK0 flag to change the MTU handling for IPv6.Mon, Jul 14, 8:33 AM

LGTM.

This revision is now accepted and ready to land.Mon, Jul 14, 9:12 AM

I like the word noclamp. Well, there're other tunneling interfaces such as if_gre, if_vxlan and if_wg. Will we introduce too many XX_NOCLAP flags for other types of tunneling interfaces?

In D51297#1171478, @zlei wrote:

I like the word noclamp. Well, there're other tunneling interfaces such as if_gre, if_vxlan and if_wg. Will we introduce too many XX_NOCLAP flags for other types of tunneling interfaces?

Honestly, I'm not familiar with this area since I only have the ports commit bit. If we implement the NOCLAMP flag in a more common place that covers other tunneling types, such as IFCAPBITS, then we would not need to reintroduce NOCLAMP for each tunneling type individually. However, I'm not sure if that's considered good practice; probably not.

Some suggestions for the manual pages.

sbin/ifconfig/ifconfig.8
2892–2893
share/man/man4/gif.4
104	Missing word?
124–129
161
181

meta added inline comments.Thu, Jul 17, 2:01 AM

sbin/ifconfig/ifconfig.8
2892–2893	I choose "a" here for consistency with the `ignore_source` flag just below.
share/man/man4/gif.4
124–129	Regarding the man page gif.4, it comes from D45854, which is already approved. I just replaced the flag name, `link0` to `noclamp`. So, can we focus on the difference with D45854? I'll make the suggested corrections if they're essential though.

pauamma_gundo.com added inline comments.Thu, Jul 17, 2:32 AM

share/man/man4/gif.4
124–129	Regarding the man page gif.4, it comes from D45854, which is already approved. I just replaced the flag name, `link0` to `noclamp`. So, can we focus on the difference with D45854? I'll make the suggested corrections if they're essential though. I didn't see that review until just now. Had I seen it in time, I would have commented there instead of here. As for "essential", I think https://reviews.freebsd.org/D51297#inline-305594 at least is important to have for clarity, and the others would be good to have, perhaps in a follow-up review.

meta added inline comments.Thu, Jul 17, 4:54 AM

share/man/man4/gif.4
124–129	I see. Thank you for your time.

Reflect essential review comments on manpages

This revision now requires review to proceed.Thu, Jul 17, 6:09 AM

Harbormaster completed remote builds in B65503: Diff 158652.Thu, Jul 17, 6:09 AM

ziaee added inline comments.Thu, Jul 17, 12:08 PM

sbin/ifconfig/ifconfig.8
2892–2893	It's wrong there too. Consider "I would like to brush a bunny". This is any bunny. The most nonspecific article. Vs, "I would like to brush the bunny", implies that there is a specific bunny before us.

meta added inline comments.Thu, Jul 17, 11:57 PM

sbin/ifconfig/ifconfig.8
2892–2893	That makes sense, but I guessed there was a certain reason "a" was chosen when the description of `ignore source` was added. The existing one was also just wrong. Anyway, I will correct them. Thanks.