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

Add vfNames in selectors to support VM case #205

Closed
pperiyasamy opened this issue Feb 12, 2020 · 8 comments
Closed

Add vfNames in selectors to support VM case #205

pperiyasamy opened this issue Feb 12, 2020 · 8 comments

Comments

@pperiyasamy
Copy link
Contributor

What would you like to be added?

I would like to provide vfNames field in selectors to enumerate matched devices into resource pool.

What is the use case for this feature / enhancement?

In case of OpenStack VM deployments, the virtio (or) VFs interfaces are created and attached inside the VM for applications usage. These devices can be either bound with net or dpdk driver.

When dpdk driver is bound with those devices, then there won’t be any kernel interface associated with it and can’t be mapped into a resource pool using its PCI Address in pfNames parameter. so it needs vfNames parameter to filter the devices at granular level.

The vfNames is provided with list of PCI Addresses of such devices.
It can be also made to support net devices.

@zshi-redhat
Copy link
Collaborator

@pperiyasamy thanks for trying to address dpdk usage in VM in another way.
I feel this looks better than the original fix to GetPfName as this is to add an optional selector to filter VF PCI address.

It sounds to me that VfName may not reflect the fact that it's going to use VF PCI address as element of VfNames, VfDevices might be more accurate.

Wrt supporting netdevice with this same VfName selector, it reminds me of rootDevices that we supported in v2.1.0 tagged version, which specifies PF PCI addresses to be discovered. I think we might want to go with VF devices first rather to support all netdevices as we are not intended to support exposing PFs directly as a resource.

When DP is enabled to support VF device in VM, we may also need to consider improving SR-IOV CNI in order to use VF in kernel mode inside VM. Which I don't think it works today. For example, it still needs to access PF when configuring VF. The ability to configure VF properties (such as vlan, spoof check, trusted vf) is not available inside OpenStack VM unless it has a way to communicate with Neutron.

@pperiyasamy
Copy link
Contributor Author

@zshi-redhat Thanks for your feedback. The vfDevices looks to be more relevant than vfNames. But doesn't it lead to a confusion with existing devices field ?

we may also need to consider improving SR-IOV CNI in order to use VF in kernel mode inside VM. Which I don't think it works today

Yes, It needed changes. I will work on it.

@zshi-redhat
Copy link
Collaborator

@zshi-redhat Thanks for your feedback. The vfDevices looks to be more relevant than vfNames. But doesn't it lead to a confusion with existing devices field ?

Yes, it does look similar to devices field. I cannot think of better names other than vfDevices or vfAddresses. Let's just use vfNames unless any other ideas.

we may also need to consider improving SR-IOV CNI in order to use VF in kernel mode inside VM. Which I don't think it works today

Yes, It needed changes. I will work on it.

@pperiyasamy
Copy link
Contributor Author

@zshi-redhat Thanks for your feedback. The vfDevices looks to be more relevant than vfNames. But doesn't it lead to a confusion with existing devices field ?

Yes, it does look similar to devices field. I cannot think of better names other than vfDevices or vfAddresses. Let's just use vfNames unless any other ideas.

we may also need to consider improving SR-IOV CNI in order to use VF in kernel mode inside VM. Which I don't think it works today

Yes, It needed changes. I will work on it.

@zshi-redhat I've updated PR #195 as per the discussion. Please have a look and let me know.
I will also change vfDevices into vfNames in the next commit.

@zshi-redhat
Copy link
Collaborator

@zshi-redhat Thanks for your feedback. The vfDevices looks to be more relevant than vfNames. But doesn't it lead to a confusion with existing devices field ?

Yes, it does look similar to devices field. I cannot think of better names other than vfDevices or vfAddresses. Let's just use vfNames unless any other ideas.

we may also need to consider improving SR-IOV CNI in order to use VF in kernel mode inside VM. Which I don't think it works today

Yes, It needed changes. I will work on it.

@zshi-redhat I've updated PR #195 as per the discussion. Please have a look and let me know.
I will also change vfDevices into vfNames in the next commit.

@pperiyasamy thanks for the quick implementation!
I'd like to have more people review on this proposal before commenting on the PR.
Let's see if any other comments on the VfNames selector.

@zshi-redhat
Copy link
Collaborator

@ahalim-intel PTAL, thanks!

@cswindle
Copy link

Should this issue be closed as I believe that #195 deals with it and has been merged?

@pperiyasamy
Copy link
Contributor Author

Yes, closing it.

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

No branches or pull requests

3 participants