Page MenuHomeFreeBSD

vxlan: correct interface MTU when using hw offloads
ClosedPublic

Authored by kib on Mar 30 2021, 7:19 PM.

Details

Summary

Otherwise it breaks when offloading like checksum or TSO are used, because second (encapsulated) ip_output() processing passes fragments of the encapsulated packet down to the hardware interface.

Diagnosed by: hselasky

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

kib requested review of this revision.Mar 30 2021, 7:19 PM

I fear I don't know enough about vxlan internals. It seems to me as we'd want to document the reduced mtu somewhere in man page or code and the reason for it though.

I wonder if the VXLAN_MAX_MTU / ioctl code path are doing the correct thing in all cases if you have to remove hdrlen in some?

bz added a subscriber: bz.
np requested changes to this revision.Mar 30 2021, 9:23 PM

vxlan_setup_interface_hdrlen() can be called repeatedly and if the vxlan interface is down it will call vxlan_ctrl_set_remote_addr(). if_mtu will get decremented every time and that's not correct.

This revision now requires changes to proceed.Mar 30 2021, 9:23 PM
In D29501#661249, @np wrote:

vxlan_setup_interface_hdrlen() can be called repeatedly and if the vxlan interface is down it will call vxlan_ctrl_set_remote_addr(). if_mtu will get decremented every time and that's not correct.

I meant this the other way around of course. vxlan_ctrl_set_remote_addr() can be called repeatedly and it calls vxlan_setup_interface_hdrlen() if the ifnet is down.

Do not decrement, set directly when address is selected and hdrlen is known.

Do not override user-set MTU value.

Lock sc around vxlan_setup_interface_hdrlen() to get consistent MTU/flag setting.

sys/net/if_vxlan.c
3216

Note that at this point ifp is externally visible already. Doing vxlan_setup_interface_hdrlen() there without the lock is simply unsafe since other thread might start modifying this vxlan parameters.

This revision is now accepted and ready to land.Mar 31 2021, 12:49 AM

By the way there's an "MTU" section in the vxlan(4) man page and you might want to update it with this new behavior.

Document auto MTU adjustment.

This revision now requires review to proceed.Mar 31 2021, 1:13 AM
share/man/man4/vxlan.4
180–181 ↗(On Diff #86594)

Minor nit: "sets its MTU" instead of "sets it MTU"

187 ↗(On Diff #86594)

Maybe use "set" instead of "fixate"?

kib marked an inline comment as done.Mar 31 2021, 1:26 AM
kib added inline comments.
share/man/man4/vxlan.4
187 ↗(On Diff #86594)

I wrote it as used to set the fixed MTU, I want to highlight that it is non-adjustable there, in addition to the sentence after.

This revision is now accepted and ready to land.Mar 31 2021, 2:12 AM