k8snetworkplumbingwg / sriov-network-operator

Operator for provisioning and configuring SR-IOV CNI plugin and device plugin
Apache License 2.0
76 stars 106 forks source link

Discussion: Extending NIC selectors #60

Open martinkennelly opened 3 years ago

martinkennelly commented 3 years ago

I propose we extend the sriovNetworkNicSelector fields to include additional optional selectors as defined in SRIOV DP. I want to generate discussion which additional selectors we would like to include.

For my customer use case, I want the SRIOV Network Operator to be able to group VFs by DDP Profile. This would involve exposing the ddpProfiles as an optional selector in this Operator. The provisioning of DDP on a NIC will be a pre-requisite outside the scope of this Operator.

I raised this topic at the additional network & resource community meeting and got positive feedback of adding new optional NIC selectors.

zshi-redhat commented 3 years ago

/cc @pliurh

pliurh commented 3 years ago

@martinkennelly I'm open to add this ddp selector to the SRIOV operator. However, we also need to think about how to discover the ddp profiles on different NIC models.

ipatrykx commented 3 years ago

Hi @zshi-redhat @pliurh I would like to continue the topic started by @martinkennelly. I have prepared small overview of what we would like to achieve and what changes would be required.

What could be added:

Support to use DDP profile name as a selector, what is currently supported by SRIOV Network Device Plugin itself: https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/tree/master/docs/ddp

Justification:

This feature is currently supported by DP. However it seems that SRIOV Network Operator does not support it as the team does not want to incorporate ‘DDPTool’ binary into Network Operator. However, we will soon introduce the changes to the DP community that will make it possible to read the DDP Package name using devlink directly for Intel’s E810 (Columbiaville) NICs (and other NICs that support’s devlink). Hopefully those changes will be accepted by the community.

Proposed workflow:

  1. Add support for devlink in SRIOV Network Device Plugin (implementation done, minor tweaks needed). Two possible ways of doing this (done simultaneously):

    • Add Devlink related logic to SRIOV DP
    • Add Devlink related logic to vishvananda/netlink library (preferred) For this, we I would like to get those changes merged into DP and as soon as the changes would be accepted to netlink library, get rid of the code in the DP and use the functions in the library instead.
  2. Add proper NIC selector to SRIOV Network Operator

    • Add ddpProfiles to SriovNetworkNicSelector
    • util.go -> validateSelector - validate if vendor == Intel && card == CVL
  3. This should be automatically parsed to sriov-device-plugin configmap. Add tests.

Out of scope:

Provisioning of the DDP package to the NIC (flashing) is out of scope for this changes. It would be nice to implement that, so node daemon would do that simply using SriovNetworkNodeState but that would require a way to find package file on the node. As I believe it would be possible to achieve that, I would like to discuss this after the selector itself will be supported.

adrianchiris commented 3 years ago

Great news shifting to devlink to query ddp profile !

@ipatrykx can you share some more info on the devlink work ? are those devlink device parameters ?

ipatrykx commented 3 years ago

@adrianchiris I think I will be able to create PR on DP in few days. Anyway, it uses devlink-info API, which general overview is provided in Kernel's docs and CVL specific implementation can be found here.

From DP point of view it's just about issuing devlink's DEVLINK_CMD_INFO_GET command and parsing the response to get the value of fw.app.name.

ipatrykx commented 2 years ago

Hi all, just to refresh this thread a bit -I already have this feature implemented, however I am waiting for the internal QA process to be finished.

ognjen011 commented 8 months ago

Just to find out has this ever been done can we use ddpProfiles?

SchSeba commented 7 months ago

Hi @ognjen011 this was not implemented in the operator because we didn't get any requirement for it.

be aware that we will need to first revive https://github.com/k8snetworkplumbingwg/sriov-network-device-plugin/pull/387 before we can add support in the operator