Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use same MTU as uplink for bridge port #6577

Merged

Conversation

antoninbas
Copy link
Contributor

In bridging mode (on Linux), when moving the physical adapter to the bridge, we explictly set the MTU for the bridge port to the same value as for the physical adapter. Without this change, the MTU may default to a different (lower) value if some existing container ports have a lower MTU value. For example, this occurs when first installing Antrea in encap mode, then re-installing Antrea in noEncap mode with bridging mode enabled.

We also do some minor documentation updates to indicate to users that they should consider restarting existing workloads when updating the Antrea datapath configuration.

Fixes #6456

Copy link
Contributor Author

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wenyingd do you think something like this would make sense for the bridge port on Windows? I am not sure it is needed as on Windows are always use "bridging mode", and this is not something that is configuration-dependent.

Comment on lines +457 to +462
if mtu > 0 {
if err := bridge.SetInterfaceMTU(ifaceName, mtu); err != nil {
return "", false, fmt.Errorf("failed to set bridge interface MTU: %w", err)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not enforced if the port already exists (as determined by the call to bridge.GetOFPort at the beginning of the function). My take is that this is consistent with how we handle other configurations at the moment (e.g., we don't call SetLinkUp unconditionally).

@antoninbas antoninbas force-pushed the use-same-mtu-as-uplink-for-bridge-port branch from 4f0ae28 to 71374be Compare July 31, 2024 01:06
@wenyingd
Copy link
Contributor

@wenyingd do you think something like this would make sense for the bridge port on Windows? I am not sure it is needed as on Windows are always use "bridging mode", and this is not something that is configuration-dependent.

I don't think it is necessary for Windows. This is because the host local interface is automatically created by Windows OS when we create the HNSNetwork (VMSwitch) , what antrea-agent does is to rename the vNIC (host local interface) and to attach it on OVS bridge. Windows OS has ensured the configurations on the vNIC is the same as the uplink (physical interface). Besides, setting mtu values on OVSDB on Windows doesn't really take effect on the network adapter, we shall modify the network adapter directly.

// We request the same MTU for the bridge interface as for the uplink adapter. If we don't,
// OVS will default to the lowest MTU among all existing bridge ports, including container
// ports. There may be some existing workloads with a lower MTU, and using that lower value
// may impact host connectivity.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add additional comment that big packet will be fragmented when it passthrough lower MTU interfaces (e.g. old antrea-gw0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always update the MTU of antrea-gw0 to the correct value:

if err := i.setInterfaceMTU(i.hostGateway, i.networkConfig.InterfaceMTU); err != nil {

Do you have another case in mind?

tnqn
tnqn previously approved these changes Aug 1, 2024
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas
Copy link
Contributor Author

/test-flexible-ipam-e2e

Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two nits

docs/noencap-hybrid-modes.md Show resolved Hide resolved
pkg/agent/util/net_linux.go Show resolved Hide resolved
In bridging mode (on Linux), when moving the physical adapter to the
bridge, we explictly set the MTU for the bridge port to the same value
as for the physical adapter. Without this change, the MTU may default to
a different (lower) value if some existing container ports have a lower
MTU value. For example, this occurs when first installing Antrea in
encap mode, then re-installing Antrea in noEncap mode with bridging mode
enabled.

We also do some minor documentation updates to indicate to users that
they should consider restarting existing workloads when updating the
Antrea datapath configuration.

Fixes antrea-io#6456

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
@antoninbas
Copy link
Contributor Author

/test-all
/test-flexible-ipam-e2e

@antoninbas antoninbas added kind/bug Categorizes issue or PR as related to a bug. action/release-note Indicates a PR that should be included in release notes. labels Aug 6, 2024
@antoninbas antoninbas merged commit ffa1af6 into antrea-io:main Aug 6, 2024
57 of 61 checks passed
@antoninbas antoninbas deleted the use-same-mtu-as-uplink-for-bridge-port branch August 6, 2024 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pod and uplink port MTU mismatched after a testbed is changed from encap to noEncap mode
5 participants