address illegal SR-IOV VF MTU configuration by kanlkan · Pull Request #213 · kubernetes-sigs/dranet

kanlkan · 2026-06-01T00:06:03Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

MTU can be configured via opaque.parameters.interface.mtu.
However, for SR-IOV VFs, it is currently possible to set an MTU larger than the parent PF’s MTU due to the lack of validation.

Which issue(s) this PR is related to:

Fixes #178

Special notes for your reviewer:

The example output below shows the case where validation fails. An error is logged, and the Pod remains in the ContainerCreating state.

Events:
  Type     Reason                         Age   From               Message
  ----     ------                         ----  ----               -------
  Normal   Scheduled                      8s    default-scheduler  Successfully assigned default/pod-a-2-mlnx-nomac to eagle04
  Warning  FailedPrepareDynamicResources  7s    kubelet            Failed to prepare dynamic resources: prepare dynamic resources: NodePrepareResources failed for ResourceClaim vf-rc-a-2: claim 21c6d996-0dfe-43d7-a88d-846db605d4d8 contain errors: requested MTU 1600 for SR-IOV VF ens1f0v1 exceeds parent PF ens1f0np0 MTU 1500
kanda-masaharu@eagle04:~/work/repo/dci-community-team/k8s_demo/sriov_vf_test_dranet/pods$ k get pod pod-a-2-mlnx-nomac
NAME                 READY   STATUS              RESTARTS   AGE
pod-a-2-mlnx-nomac   0/1     ContainerCreating   0          55s

Does this PR introduce a user-facing change?

Fixed an issue where SR-IOV VF MTU could be configured larger than the MTU of its parent PF. Such configurations are now rejected.

Signed-off-by: Masaharu Kanda <kanlkan.naklnak@gmail.com>

netlify · 2026-06-01T00:06:09Z

✅ Deploy Preview for dranet canceled.

Name	Link
🔨 Latest commit	`5f8645a`
🔍 Latest deploy log	https://app.netlify.com/projects/dranet/deploys/6a21111fa07d2c000883859e

Copilot

Pull request overview

This PR adds validation to prevent configuring an SR-IOV Virtual Function (VF) MTU larger than its parent Physical Function (PF) MTU, avoiding invalid network setups that can leave pods stuck during resource preparation.

Changes:

Add sysfs helper(s) to resolve a VF’s parent PF interface name.
Add unit tests for PF-name resolution via a temporary sysfs-like directory tree.
Enforce VF MTU ≤ PF MTU during NodePrepareResources / claim preparation (rejecting invalid claims).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pkg/inventory/sysfs.go	Adds PF interface lookup helper for a given VF using sysfs.
pkg/inventory/sysfs_test.go	Adds tests covering PF lookup success and failure scenarios.
pkg/driver/dra_hooks.go	Adds claim-time MTU validation for SR-IOV VFs against parent PF MTU.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kanlkan · 2026-06-02T05:43:57Z

+		// For SR-IOV VFs, check that the requested MTU does not exceed the parent PF's MTU.
+		// If it does, log an error and fail the Pod creation to avoid silent misconfiguration.
+		if deviceCfg.NetworkInterfaceConfigInPod.Interface.MTU != nil {
+			if pfName, err := inventory.GetPFInterfaceName(ifName); err == nil {
+				if pfLink, err := nlHandle.LinkByName(pfName); err == nil {
+					pfMTU := pfLink.Attrs().MTU
+					requestedMTU := int(*deviceCfg.NetworkInterfaceConfigInPod.Interface.MTU)
+					if requestedMTU > pfMTU {
+						klog.Errorf("requested MTU %d for SR-IOV VF %s exceeds parent PF %s MTU %d",
+							requestedMTU, ifName, pfName, pfMTU)
+						errorList = append(errorList, fmt.Errorf("requested MTU %d for SR-IOV VF %s exceeds parent PF %s MTU %d",
+							requestedMTU, ifName, pfName, pfMTU))
+						continue
+					}
+				}
+			}
+		}


I fix the behavior when an error happened for each branch of this validation. b4c5e84

kanlkan · 2026-06-02T05:41:31Z

+		// For SR-IOV VFs, check that the requested MTU does not exceed the parent PF's MTU.
+		// If it does, log an error and fail the Pod creation to avoid silent misconfiguration.
+		if deviceCfg.NetworkInterfaceConfigInPod.Interface.MTU != nil {
+			if pfName, err := inventory.GetPFInterfaceName(ifName); err == nil {
+				if pfLink, err := nlHandle.LinkByName(pfName); err == nil {


I added an unit test for this validation. b4c5e84

Signed-off-by: Masaharu Kanda <kanlkan.naklnak@gmail.com>

kanlkan · 2026-06-02T05:57:07Z

The e2e IPv4 test failed, but it does not seem to be related to my changes.
I would like to re-run this test, but I do not have permission to trigger it manually. The only workaround I can think of is to push an empty commit, but that is not ideal because it would make the commit history less clean.
Is there another way to re-run the test?

gauravkghildiyal · 2026-06-02T10:35:32Z

Try retest? (Edit: Doesn't work, tests don't use prow)

/retest

kanlkan · 2026-06-02T23:41:28Z

@gauravkghildiyal
Thank you for the advice. Your retest command worked perfectly.
I had previously tried using retest on another PR, but it didn't seem to trigger a re-run, so I didn't try it this time.
I'll make use of it in the future when tests fail due to transient or unrelated issues.

gauravkghildiyal

Thanks @kanlkan. Minor suggestions.

gauravkghildiyal · 2026-06-04T03:41:40Z

+		if deviceCfg.NetworkInterfaceConfigInPod.Interface.MTU != nil {
+			pfName, err := inventory.GetPFInterfaceName(ifName)
+			if err != nil {
+				// Not an SR-IOV VF, or the parent PF cannot be determined. This is


Can we only perform the check if this is an actual VF? We have an isSriovVf function to check that.

Thank you for your feedback. I've added that check. 5f8645a

gauravkghildiyal · 2026-06-04T03:48:31Z

+				if err == nil {
+					t.Fatalf("validateVFMTU() expected error, got nil")
+				}
+				want := fmt.Sprintf("requested MTU %d for SR-IOV VF eth1 exceeds parent PF eth0 MTU %d", tc.requestedMTU, tc.pfMTU)


nit: valudateVFMTU is a simple function with only one single kind of error. Let's not try to unnecessarily validate the return string. We can simplify the logic to check if err != tc.WantErr

Thank you for your feedback. I simplyfied the test. 5f8645a

gauravkghildiyal · 2026-06-04T04:10:04Z

Please also add a release note

Signed-off-by: Masaharu Kanda <kanlkan.naklnak@gmail.com>

gauravkghildiyal · 2026-06-04T06:40:33Z

@kanlkan Please remember a release note as well :) (Unhold once done)

/lgtm
/approve
/hold

k8s-ci-robot · 2026-06-04T06:40:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauravkghildiyal, kanlkan, tamilmani1989

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [gauravkghildiyal]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kanlkan · 2026-06-04T08:57:56Z

Please remember a release note as well :) (Unhold once done)

@gauravkghildiyal
Sorry. I added it.

gauravkghildiyal · 2026-06-04T16:15:43Z

/unhold

address illegal SR-IOV VF MTU configuration

f8fa173

Signed-off-by: Masaharu Kanda <kanlkan.naklnak@gmail.com>

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 1, 2026

k8s-ci-robot requested review from MikeZappa87 and tamilmani1989 June 1, 2026 00:06

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 1, 2026

kanlkan mentioned this pull request Jun 1, 2026

Missing validation for SR-IOV VF MTU exceeding parent PF MTU #178

Closed

aojea requested a review from Copilot June 1, 2026 11:09

Copilot started reviewing on behalf of aojea June 1, 2026 11:09 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

tamilmani1989 reviewed Jun 1, 2026

View reviewed changes

Comment thread pkg/driver/dra_hooks.go Outdated

Comment thread pkg/driver/dra_hooks.go Outdated

Log or fail on each branch of SR-IOV VF MTU validation, add unit test

b4c5e84

Signed-off-by: Masaharu Kanda <kanlkan.naklnak@gmail.com>

kanlkan requested a review from tamilmani1989 June 2, 2026 05:51

tamilmani1989 approved these changes Jun 3, 2026

View reviewed changes

gauravkghildiyal reviewed Jun 4, 2026

View reviewed changes

gauravkghildiyal self-assigned this Jun 4, 2026

Check SR-IOV VF before validating VF MTU, simplify unit test

5f8645a

Signed-off-by: Masaharu Kanda <kanlkan.naklnak@gmail.com>

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jun 4, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 4, 2026

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 4, 2026

k8s-ci-robot merged commit 4fcedee into kubernetes-sigs:main Jun 4, 2026
13 checks passed

kanlkan deleted the fix/vf-mtu branch June 5, 2026 00:20

Conversation

kanlkan commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR is related to:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

netlify Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for dranet canceled.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

kanlkan Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

kanlkan Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kanlkan commented Jun 2, 2026

Uh oh!

gauravkghildiyal commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kanlkan commented Jun 2, 2026

Uh oh!

gauravkghildiyal left a comment

Choose a reason for hiding this comment

Uh oh!

gauravkghildiyal Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

kanlkan Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gauravkghildiyal Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

kanlkan Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

gauravkghildiyal commented Jun 4, 2026

Uh oh!

gauravkghildiyal commented Jun 4, 2026

Uh oh!

k8s-ci-robot commented Jun 4, 2026

Uh oh!

kanlkan commented Jun 4, 2026

Uh oh!

gauravkghildiyal commented Jun 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kanlkan commented Jun 1, 2026 •

edited

Loading

netlify Bot commented Jun 1, 2026 •

edited

Loading

gauravkghildiyal commented Jun 2, 2026 •

edited

Loading

kanlkan Jun 4, 2026 •

edited

Loading