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

Use Devlink when possible for NewPciNetDevice #399

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SchSeba
Copy link
Collaborator

@SchSeba SchSeba commented Nov 23, 2021

Try to use devlink api to configure the linkType to be ether.
If the card doesn't support this we fall back to netlink.

This commit is a partial fix for #392
There are still cards the doesn't support the netlink api like intel xv710

Signed-off-by: Sebastian Sch sebassch@gmail.com

@@ -82,7 +82,13 @@ func NewPciNetDevice(dev *ghw.PCIDevice, rFactory types.ResourceFactory, rc *typ
}

linkType := ""
if len(ifName) > 0 {
if _, err = utils.GetNetlinkProvider().GetDevLinkDevice(pciAddr); err == nil {
linkType = "ether"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@adrianchiris question if the nic is InfiniBand will it get reported in devlink dev command?

I don't have a card to test this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a linkType field in devlink device ?
I only see the port Type field in devlink port attribute, but not in devlink device.
I was thinking we can reliably get the linkType from devlink interface for devlink device.

Copy link
Contributor

Choose a reason for hiding this comment

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

@adrianchiris question if the nic is InfiniBand will it get reported in devlink dev command?

it will report a devlink device. (tested on cx5)

however you need devlink port as Zenghui mentioned.
Now, for infiniband port, i dont think mlx driver has implemented the feature :(.

running on my system i didnt see devlink port for infiniband PF

// PF 03:00.1 is infiniband
$ devlink dev show
pci/0000:03:00.0
pci/0000:03:00.1
$ devlink port show
pci/0000:03:00.0/65535: type eth netdev enp3s0f0 flavour physical port 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the comment I have to say this is not working for me.

I try it on my system

lspci | grep Mellanox
5e:00.0 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5]
5e:00.1 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5]
5e:00.2 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:00.3 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:00.4 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:00.5 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:00.6 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:00.7 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:01.0 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:01.1 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:01.2 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]
5e:01.3 Ethernet controller: Mellanox Technologies MT27800 Family [ConnectX-5 Virtual Function]

I try the devlink port show and it's empty on my system do I miss something?

[root@test core]# devlink port show
[root@test core]# 

Copy link
Contributor

Choose a reason for hiding this comment

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

I try the devlink port show and it's empty on my system do I miss something?

hmmm, maybe you need a newer kernel ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

your version is a bit newer

[core@cnfdt04 ~]$ uname -a
Linux cnfdt04 4.18.0-305.19.1.el8_4.x86_64 #1 SMP Tue Sep 7 07:07:31 EDT 2021 x86_64 x86_64 x86_64 GNU/Linux
[core@cnfdt04 ~]$ devlink port
[core@cnfdt04 ~]$ 

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @bn222 I try it with the same kernel you did and I am still not able to get the information

[root@cnfdd8 core]# devlink dev
pci/0000:19:00.0
pci/0000:19:00.1
pci/0000:3b:00.0
pci/0000:3b:00.1
pci/0000:86:00.0
pci/0000:86:00.1
[root@cnfdd8 core]# devlink port
[root@cnfdd8 core]# 
[root@cnfdd8 core]# 
[root@cnfdd8 core]# uname -a
Linux cnfdd8 4.18.0-305.28.1.el8_4.x86_64 #1 SMP Mon Nov 8 07:45:47 EST 2021 x86_64 x86_64 x86_64 GNU/Linux

Copy link
Contributor

@bn222 bn222 Dec 9, 2021

Choose a reason for hiding this comment

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

Hi @SchSeba , I just checked this again. I was confused by other NICs on my system. I can confirm that I also don't have it showing up in devlink port.
I'm using a slightly different ConnectX-5:

d8:00.0 Ethernet controller: Mellanox Technologies MT28800 Family [ConnectX-5 Ex] d8:00.1 Ethernet controller: Mellanox Technologies MT28800 Family [ConnectX-5 Ex]

I also tried the latest RHEL kernel, also not showing anything there.

However, I also just built an upstream kernel, and there it works. I get:

pci/0000:d8:00.0/65535: type eth netdev ens8f0np0 flavour physical port 0 splittable false pci/0000:d8:00.1/131071: type eth netdev ens8f1np1 flavour physical port 1 splittable false

I suspect RHEL is missing some patches. @adrianchiris Do you have an idea what patches could be missing?

I could start a bisecting effort to pinpoint the patches if we have no clue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bn222 will the type by ib if the nic is in InfiniBand mode?

I must say I have a problem needing a specific kernel and the solution working only for some time of network nics.

Is it possible to drop this one now that k8snetworkplumbingwg/sriov-network-operator#203 was merged?

Or we are going to have a problem with we want to use InfiniBand?

Copy link
Contributor

@bn222 bn222 Dec 14, 2021

Choose a reason for hiding this comment

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

@SchSeba It's not that we need a specific kernel. Rather, I think we just miss a fix/patch in the kernel. Once I can pinpoint what changes we need, we would be in a better place to decide.

Regarding k8snetworkplumbingwg/sriov-network-operator#203 which removes the selector, I'm just wondering if there are other issues elsewhere with having an empty link type.

I think that having an additional way to query the link type is still useful. That, or the question becomes what the real validity is of having the link type stored in the first place.

Try to use devlink api to configure the linkType to be `ether`.
If the card doesn't support this we fall back to netlink.

This commit is a partial fix for k8snetworkplumbingwg#392
There are still cards the doesn't support the netlink api like intel xv710

Signed-off-by: Sebastian Sch <sebassch@gmail.com>
@zshi-redhat
Copy link
Collaborator

As we are adding devlink support in various functional calls, it would good to have a generic isDevlinkEnabled function to distinguish devlink call and non-devlink call.

@zshi-redhat
Copy link
Collaborator

As we are adding devlink support in various functional calls, it would good to have a generic isDevlinkEnabled function to distinguish devlink call and non-devlink call.

This is being added through #387

@adrianchiris
Copy link
Contributor

@SchSeba are there plans to work on this one ?

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.

None yet

5 participants