[switchdev 8/x] Add support for switchdev configuration to sriov pkg #642

ykulazhenkov · 2024-02-22T07:47:21Z

Add support for switchdev configuration to sriov pkg

This PR introduce required logic to handle switchdev configuration by the sriov pkg.

This code is a "dead" code at the moment and will never run (except tests), because all switchdev devices are filtered by the upper layer.

Depends on #633 and #628.

Only two last commits are relevant to the PR.

cc @adrianchiris @SchSeba @zeeke

github-actions · 2024-02-22T07:47:31Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

coveralls · 2024-02-22T07:52:05Z

Pull Request Test Coverage Report for Build 8076878900

Details

-87 of 157 (44.59%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.7%) to 35.877%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
api/v1/helper.go	5	10	50.0%
pkg/host/internal/vdpa/vdpa.go	2	8	25.0%
pkg/host/internal/sriov/sriov.go	62	138	44.93%

Totals
Change from base Build 8067666954:	0.7%
Covered Lines:	4434
Relevant Lines:	12359

💛 - Coveralls

github-actions · 2024-02-22T10:35:27Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-02-23T09:22:56Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

pkg/host/internal/sriov/sriov.go

github-actions · 2024-02-27T08:45:26Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

e0ne

LGTM

zeeke

Left a few minor comments. Overall PR shape looks good to me.

This code is a "dead" code at the moment and will never run (except tests), because all switchdev devices are filtered by the upper layer.

what is the upper layer that filters the devices?

zeeke · 2024-02-27T16:28:04Z

pkg/host/internal/sriov/sriov.go

@@ -330,6 +342,45 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error {
 	return nil
 }

+func (s *sriov) configureHWOptionsForPF(iface *sriovnetworkv1.Interface) error {


nit: this function is all about settings for switchdev mode. right? if so, please rename to configureHWOptionsForSwitchdev

zeeke · 2024-02-27T16:56:52Z

pkg/host/internal/sriov/sriov.go

 	devLink, err := s.netlinkLib.DevLinkGetDeviceByName("pci", pciAddress)
 	if err != nil {
 		if errors.Is(err, syscall.ENODEV) {
-			// the device doesn't support devlink
-			return "", nil
+			log.Log.V(2).Info("GetNicSriovMode(): device doesn't support devlink, assume legacy mode", "device", pciAddress)


This can produce many log lines. We fixed it in:

Reduce config-daemon logging verbosity #468

I'd avoid logging here

ok, I removed this line

Signed-off-by: Yury Kulazhenkov <ykulazhenkov@nvidia.com>

github-actions · 2024-02-27T17:43:02Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-02-27T17:54:11Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

Signed-off-by: Yury Kulazhenkov <ykulazhenkov@nvidia.com>

github-actions · 2024-02-28T07:39:01Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

ykulazhenkov · 2024-02-28T07:46:30Z

@zeeke thx for the review

what is the upper layer that filters the devices?

I'm talking about skipVF logic in generic_plugin, which filters NICs with switchdev configurations
https://github.com/ykulazhenkov/sriov-network-operator/blob/d324d46240b8ea66318bdb2c3acf621e9158178b/pkg/plugins/generic/generic_plugin.go#L416

In this PR, skipVF logic is still there this means that NICs with switchdev config will not be passed to ConfigSriovInterfaces function.

adrianchiris

LGTM

ykulazhenkov · 2024-03-04T07:14:40Z

@zeeke Could you take another look at this PR? thx

zeeke

LGTM

ykulazhenkov force-pushed the pr-switchdev-configuration branch from 4cbf027 to 5282b87 Compare February 22, 2024 10:35

ykulazhenkov force-pushed the pr-switchdev-configuration branch from 5282b87 to d2c215f Compare February 23, 2024 09:22

ykulazhenkov mentioned this pull request Feb 26, 2024

[switchdev 9/9] Enable new switchdev implementation #643

Merged

adrianchiris reviewed Feb 26, 2024

View reviewed changes

pkg/host/internal/sriov/sriov.go Show resolved Hide resolved

pkg/host/internal/sriov/sriov.go Outdated Show resolved Hide resolved

ykulazhenkov force-pushed the pr-switchdev-configuration branch from d2c215f to 6a8c00d Compare February 27, 2024 08:45

e0ne approved these changes Feb 27, 2024

View reviewed changes

zeeke reviewed Feb 27, 2024

View reviewed changes

Add helper funcs to retrieve eswitch mode from interface spec and status

d6a284b

Signed-off-by: Yury Kulazhenkov <ykulazhenkov@nvidia.com>

ykulazhenkov force-pushed the pr-switchdev-configuration branch from 6a8c00d to 84963fb Compare February 27, 2024 17:42

ykulazhenkov force-pushed the pr-switchdev-configuration branch from 84963fb to 6d37264 Compare February 27, 2024 17:54

Add support for switchdev configs to sriov pkg

d324d46

Signed-off-by: Yury Kulazhenkov <ykulazhenkov@nvidia.com>

ykulazhenkov force-pushed the pr-switchdev-configuration branch from 6d37264 to d324d46 Compare February 28, 2024 07:38

adrianchiris approved these changes Feb 28, 2024

View reviewed changes

zeeke approved these changes Mar 5, 2024

View reviewed changes

zeeke merged commit efea23b into k8snetworkplumbingwg:master Mar 5, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[switchdev 8/x] Add support for switchdev configuration to sriov pkg #642

[switchdev 8/x] Add support for switchdev configuration to sriov pkg #642

ykulazhenkov commented Feb 22, 2024

github-actions bot commented Feb 22, 2024

coveralls commented Feb 22, 2024 •

edited

Loading

github-actions bot commented Feb 22, 2024

github-actions bot commented Feb 23, 2024

github-actions bot commented Feb 27, 2024

e0ne left a comment

zeeke left a comment

zeeke Feb 27, 2024

ykulazhenkov Feb 28, 2024

zeeke Feb 27, 2024

ykulazhenkov Feb 28, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 28, 2024

ykulazhenkov commented Feb 28, 2024

adrianchiris left a comment

ykulazhenkov commented Mar 4, 2024

zeeke left a comment

[switchdev 8/x] Add support for switchdev configuration to sriov pkg #642

[switchdev 8/x] Add support for switchdev configuration to sriov pkg #642

Conversation

ykulazhenkov commented Feb 22, 2024

github-actions bot commented Feb 22, 2024

coveralls commented Feb 22, 2024 • edited Loading

Pull Request Test Coverage Report for Build 8076878900

Details

💛 - Coveralls

github-actions bot commented Feb 22, 2024

github-actions bot commented Feb 23, 2024

github-actions bot commented Feb 27, 2024

e0ne left a comment

Choose a reason for hiding this comment

zeeke left a comment

Choose a reason for hiding this comment

zeeke Feb 27, 2024

Choose a reason for hiding this comment

ykulazhenkov Feb 28, 2024

Choose a reason for hiding this comment

zeeke Feb 27, 2024

Choose a reason for hiding this comment

ykulazhenkov Feb 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 28, 2024

ykulazhenkov commented Feb 28, 2024

adrianchiris left a comment

Choose a reason for hiding this comment

ykulazhenkov commented Mar 4, 2024

zeeke left a comment

Choose a reason for hiding this comment

coveralls commented Feb 22, 2024 •

edited

Loading