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 support for vDPA devices management #939

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

ykulazhenkov
Copy link
Contributor

@ykulazhenkov ykulazhenkov commented Dec 27, 2023

vDPA - virtio Data Path Acceleration

Following functions are supported by the implementation:

VDPANewDev - adds new VDPA device
Equivalent to: vdpa dev add name <name> mgmtdev <mgmtBus>/<mgmtName> [params]

VDPADelDev removes VDPA device
Equivalent to: vdpa dev del <name>

VDPAGetDevList returns list of VDPA devices
Equivalent to: vdpa dev show

VDPAGetDevByName returns VDPA device selected by name
Equivalent to: vdpa dev show <name>

VDPAGetDevConfigList returns list of VDPA devices configurations
Equivalent to: vdpa dev config show

VDPAGetDevConfigByName returns VDPA device configuration selected by name
Equivalent to: vdpa dev config show <name>

VDPAGetDevVStats returns vstats for VDPA device
Equivalent to: vdpa dev vstats show <name> qidx <queueIndex>

VDPAGetMGMTDevList returns list of mgmt devices
Equivalent to: vdpa mgmtdev show

VDPAGetMGMTDevByBusAndName returns mgmt devices selected by bus and name
Equivalent to: vdpa mgmtdev show <bus>/<name>

Copy link
Collaborator

@adrianchiris adrianchiris 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 !

@amorenoz this work can simplify govdpa lib

could you take a look ? as this work is a superset of what was done for kernel vdpa in k8snetworkplumbingwg/govdpa

nl.NewRtAttr(nl.VDPA_ATTR_MGMTDEV_DEV_NAME, nl.ZeroTerminated(mgmtName)),
}
if mgmtBus != "" {
attrs = append(attrs, nl.NewRtAttr(nl.VDPA_ATTR_MGMTDEV_BUS_NAME, nl.ZeroTerminated(mgmtBus)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

mgmt bus is not mandatory ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, for some devices it can be not set

Copy link
Collaborator

Choose a reason for hiding this comment

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

ack, thx.

attrs = append(attrs,
nl.NewRtAttr(nl.VDPA_ATTR_MGMTDEV_DEV_NAME, nl.ZeroTerminated(*dev)),
)
if bus != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

bus is not mandatory if dev is provided ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, example is vdpa_sim device. It doesn't have a bus

@amorenoz
Copy link

@lmilleri PTAL

@aboch
Copy link
Collaborator

aboch commented Jan 22, 2024

Unless each commit brings in a independent feature/fix, please have one commit per PR. We have been following this logic in this repo.

Current implementation support
following functions:
- VDPANewDev
- VDPADelDev
- VDPAGetDevList
- VDPAGetDevByName
- VDPAGetDevConfigList
- VDPAGetDevConfigByName
- VDPAGetDevVStats
- VDPAGetMGMTDevList
- VDPAGetMGMTDevByBusAndName

Signed-off-by: Yury Kulazhenkov <[email protected]>
@ykulazhenkov
Copy link
Contributor Author

Unless each commit brings in a independent feature/fix, please have one commit per PR. We have been following this logic in this repo.

Thank you @aboch,
I squashed all commits to single one

@adrianchiris
Copy link
Collaborator

@aboch @vishvananda could you take a look at this PR ? :)

@aboch
Copy link
Collaborator

aboch commented Jan 29, 2024

LGTM

@aboch aboch merged commit 857968a into vishvananda:main Jan 29, 2024
2 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.

4 participants