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

SpiderMultusConfig: Add mtu size support for macvlan / ipvlan / sriov #4636

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

cyclinder
Copy link
Collaborator

@cyclinder cyclinder commented Feb 10, 2025

Thanks for contributing!

Notice:

What issue(s) does this PR fix:

Fixes #4422

Special notes for your reviewer:

SpiderMultusConfig: Add mtu size support for macvlan and ipvlan

@cyclinder cyclinder added the release/feature-new release note for new feature label Feb 10, 2025
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.28%. Comparing base (edb0960) to head (1d90f42).
Report is 12 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4636      +/-   ##
==========================================
- Coverage   75.68%   75.28%   -0.40%     
==========================================
  Files          56       56              
  Lines        6765     6765              
==========================================
- Hits         5120     5093      -27     
- Misses       1431     1462      +31     
+ Partials      214      210       -4     
Flag Coverage Δ
unittests 75.28% <ø> (-0.40%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@cyclinder cyclinder force-pushed the spidermultusconfig/mtu branch 6 times, most recently from be8393c to 9b2d918 Compare February 11, 2025 05:53
@weizhoublue
Copy link
Collaborator

weizhoublue commented Feb 11, 2025

需要明确几点

  1. macvlan ipvlan 设置了特殊的 MTU, 如果 同 master 不同,本质是否 是 不同工作的 ? 因为 master 的 mtu 就会限制 它的 收发长度,一旦出现特殊长度的报文 ,子接口是不能收发的。所以,是否应该 与 master 接口的 mtu 要对其 。这导致 功能规划的偏差,是否根本不需要自定义子接口 mtu, 只要用户设置好 master mtu, cni 只需要 提供一个开关,保持子接口的 mtu 与 master 对其 即可

  2. 设置特殊 mtu 的 实际场景是什么? 目前看,是否在 rdma 场景下 设置 sriov 、macvlan 接口,与 master 对其 。那么也需要考虑到 sriov

@cyclinder
Copy link
Collaborator Author

需要明确几点

macvlan ipvlan 设置了特殊的 MTU, 如果 同 master 不同,本质是否 是 不同工作的 ? 因为 master 的 mtu 就会限制 它的 收发长度,一旦出现特殊长度的报文 ,子接口是不能收发的。所以,是否应该 与 master 接口的 mtu 要对其 。这导致 功能规划的偏差,是否根本不需要自定义子接口 mtu, 只要用户设置好 master mtu, cni 只需要 提供一个开关,保持子接口的 mtu 与 master 对其 即可

设置特殊 mtu 的 实际场景是什么? 目前看,是否在 rdma 场景下 设置 sriov 、macvlan 接口,与 master 对其 。那么也需要考虑到 sriov

  1. 对于 macvlan 和 ipvlan,mtu 配置建议应该是 <= master.mtu, 参考 https://github.com/containernetworking/plugins/blob/6e7fb60738998431c0be3f3b21b96cb1e7043f1a/plugins/main/macvlan/macvlan.go#L124, mtu 本是 macvlan 和 ipvlan 官方支持的配置,应该保持与它们保持原样

  2. sriov vf 设置 mtu 可能是没意义的,受 pf 控制,而且 sriov-cni 官方没看见支持这个

设置 macvlan 子接口的 MTU(Maximum Transmission Unit)通常是有意义的,尤其是在特定的网络环境中需要优化数据包传输效率的时候。以下是一些设置 MTU 的场景和理由:

网络性能优化:通过调整 MTU,可以优化网络性能。较大的 MTU 可以减少数据包的数量,从而降低 CPU 负载和网络开销。然而,过大的 MTU 可能导致数据包在网络中被分片,影响性能。
避免分片:在某些网络环境中,默认的 MTU 可能导致数据包被分片。通过设置合适的 MTU,可以避免分片,提高传输效率。
适配网络基础设施:不同的网络基础设施可能有不同的 MTU 要求。设置 macvlan 子接口的 MTU 可以确保与其他网络设备的兼容性。
应用需求:某些应用程序可能对 MTU 有特定的要求,以确保数据传输的可靠性和效率。

@weizhoublue
Copy link
Collaborator

weizhoublue commented Feb 11, 2025

  1. 换言之,我们真实需求,是要 设置 不同于 master 的 mtu, 还是 需要自动保持 与 master 一样的 mtu ,哪个会更加 贴近实际场景。 而 前者,即使 macvlan mtu 小于 master mtu,能顺利创建出来,是否也存在潜在风险,为什么 master mtu 也不进行调整,避免 master mtu 9000 收到一个 9000 的报文,导致无法转给 macvlan mtu 1500 的 丢包风险,难以排障。
    最佳实践的需求,是否是 调整好 master mtu 后, cni 层面提供 创建的 macvlan 与 master 对齐 ,即可

  2. pf 改变了,vf 是自动改 ?目前实际环境中,如果 pf 改为 9000 之类, vf 插入容器后 还是 1500 ? 是怎么解决的

@cyclinder
Copy link
Collaborator Author

cyclinder commented Feb 11, 2025

  1. 默认情况下,创建的子接口 mtu 跟 master.mtu 保持一致,跟这个 PR 无关 。相信大部分场景下都是默认即可,另有一些需求需要自定义 mtu,CNI 官方支持,ip link 也支持,内核也支持,我觉得我们跟社区同步支持它,并无不妥,用户既然要设置它,you它自己考量需求,风险,否则大部分场景也不需要去配置它。

这个 PR 只是在 spiderMultusConfig 中新增 mtu 字段,翻译到 Multus NAD, 具体是由 CNI(macvlan,ipvlan) 配置的

  1. see Ability to set MTU for the VF k8snetworkplumbingwg/sriov-cni#123, pf 9000 创建出来的 vf 仍然为 1500,考虑 dpdk 等场景,社区建议用 tuning 实现,没在 sriov-cni 中支持

@cyclinder cyclinder force-pushed the spidermultusconfig/mtu branch 6 times, most recently from e3cd132 to df578ed Compare February 26, 2025 06:54
@cyclinder cyclinder force-pushed the spidermultusconfig/mtu branch from df578ed to 1d90f42 Compare February 26, 2025 07:27
@@ -71,6 +71,7 @@ This is the SpiderReservedIP spec for users to configure.
| master | the Interfaces on your master, you could specify a single one Interface<br/> or multiple Interfaces to generate one bond Interface | list of strings | required | |
| vlanID | vlan ID | int | optional | [0,4094] |
| bond | expected bond Interface configurations | [BondConfig](./crd-spidermultusconfig.md#bondconfig) | optional | |
| mtu | mtu of the Interface | int | optional | the max mtu can't be over than the max mtu of the Interface |
Copy link
Collaborator

@weizhoublue weizhoublue Feb 27, 2025

Choose a reason for hiding this comment

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

可在 ai 部署文档中 的相关步骤中 提及,如何适配 mtu

否则 这个 等于没交付出去

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#4646 中提及了

@weizhoublue weizhoublue changed the title SpiderMultusConfig: Add mtu size support for macvlan and ipvlan SpiderMultusConfig: Add mtu size support for macvlan / ipvlan / sriov Feb 27, 2025
@weizhoublue weizhoublue merged commit 44bcd29 into spidernet-io:main Feb 27, 2025
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/feature-new release note for new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MTU size
2 participants