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

Support DeviceID on Auxiliary Bus #1003

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

adrianchiris
Copy link
Contributor

Device plugins may allocate network device on a bus different than PCI.

sriov-network-device-plugin supports the allocation of network devices over Auxiliary bus[1][2][3].

extend host-device CNI to support such devices if provided through runtime config.

  • Check if device provided by DeviceID runtime config is present on either PCI bus or Auxiliary bus
  • extend getLink method to support getting netdev link obj from auxiliary bus
  • add unit-test to cover the new flow

[1] https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/tree/master?tab=readme-ov-file#auxiliary-network-devices-selectors
[2] https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/tree/master/docs/subfunctions
[3] https://docs.kernel.org/networking/devlink/devlink-port.html

@@ -53,6 +56,9 @@ type NetConf struct {
RuntimeConfig struct {
DeviceID string `json:"deviceID,omitempty"`
} `json:"runtimeConfig,omitempty"`

// for internal use
AuxDevice string `json:"-"` // Auxiliary device name as appears on Auxiliary bus (/sys/bus/auxiliary)
Copy link
Member

Choose a reason for hiding this comment

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

If this is internal, it the first word should be lower-cased. Yay golang.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will align.

noticed that NetConf.DPDKMode may also need to be internal as its inferred from the system[1].
but anyhow, not related to this PR.

[1]

n.DPDKMode, err = hasDpdkDriver(n.PCIAddr)

@adrianchiris
Copy link
Contributor Author

@squeed golangci-lint version bumped in gh actions which causes lint failure (not related to this PR)

submitted #1009 to keep version fixed so that CI is stable (version bumps IMO should occur periodically in controlled manner, i.e submitting PR)

Copy link
Contributor

@s1061123 s1061123 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @adrianchiris !

@s1061123
Copy link
Contributor

@squeed PTAL again?

@s1061123
Copy link
Contributor

@adrianchiris hi, CI (Lint) has some error. Could you please take a look into it?

@adrianchiris
Copy link
Contributor Author

@adrianchiris hi, CI (Lint) has some error. Could you please take a look into it?

@s1061123 we should merge : #1009 then ill rebase this one and we should be good.

the lint issues not related to this PR but rather because linter version bumped in CI.

Device plugins may allocate network device on a bus
different than PCI.

sriov-network-device-plugin supports the allocation
of network devices over Auxiliary bus[1][2][3].

extend host-device CNI to support such devices if provided
through runtime config.

- Check if device provided by DeviceID runtime config
  is present on either PCI bus or Auxiliary bus
- extend getLink method to support getting netdev link obj
  from auxiliary bus
- add unit-test to cover the new flow

[1] https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/tree/master?tab=readme-ov-file#auxiliary-network-devices-selectors
[2] https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/tree/master/docs/subfunctions
[3] https://docs.kernel.org/networking/devlink/devlink-port.html

Signed-off-by: adrianc <adrianc@nvidia.com>
@adrianchiris
Copy link
Contributor Author

Rebased on top of main. CI is green.

@s1061123 @s1061123 PTAL on this one :)

@s1061123 s1061123 merged commit c860b78 into containernetworking:main Mar 12, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants