Skip to content

Add support for passing in a virtual function index to assign pci device #1163

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.

Sign up for GitHub

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

Jump to bottom
Merged
katiewasnothere merged 1 commit into from
Sep 24, 2021
Merged

Add support for passing in a virtual function index to assign pci device #1163

katiewasnothere merged 1 commit into from
Sep 24, 2021

Conversation

@katiewasnothere
Copy link

@katiewasnothere katiewasnothere commented Sep 18, 2021

This PR plumbs through support for passing in a virtual function index when assigning PCI devices.

To specify a device with a virtual function index, add the index to the end of the device instance ID path like so to the CRI spec for the container:

"devices": [
{"host_path":"vpci-location-path://PCIP\VEN_10DE&DEV_15F8&SUBSYS_118F10DE&REV_A1\6&17F903&0&00400010\2"}
],

Where 2 is the virtual function index.

Signed-off-by: Kathryn Baldauf [email protected]

@katiewasnothere katiewasnothere requested a review from a team as a code owner September 18, 2021 02:31
@dcantah
Copy link
Contributor

dcantah commented Sep 22, 2021 •
edited
Loading

Just to make sure, is there any behavioral difference between supplying a vf index of 0, and simply not providing one?

"devices": [
{"host_path":"vpci-location-path://PCIP\VEN_10DE&DEV_15F8&SUBSYS_118F10DE&REV_A1\6&17F903&0&00400010\0"}
],

vs.

"devices": [
{"host_path":"vpci-location-path://PCIP\VEN_10DE&DEV_15F8&SUBSYS_118F10DE&REV_A1\6&17F903&0&00400010"}
],

I guess you always set the index now on the schema, and if one isn't provided you return 0 for the index and set this so seemingly no

@katiewasnothere
Copy link
Author

katiewasnothere commented Sep 22, 2021

@dcantah there's no difference as of now

dcantah
dcantah reviewed Sep 22, 2021
View reviewed changes
dcantah
dcantah approved these changes Sep 22, 2021
View reviewed changes
Copy link
Contributor

@dcantah dcantah left a comment

Choose a reason for hiding this comment

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

lgtm!

@dcantah dcantah self-assigned this Sep 22, 2021
@katiewasnothere katiewasnothere merged commit 2608ae2 into microsoft:master Sep 24, 2021
@katiewasnothere katiewasnothere deleted the virtual_function_vpci branch September 24, 2021 17:46
anmaxvl pushed a commit to anmaxvl/hcsshim that referenced this pull request Nov 17, 2021
@ambarve
Merged PR 5137749: update hcsshim to 057bebe
a0b83d5 
Related work items: microsoft#1062, microsoft#1087, microsoft#1089, microsoft#1095, microsoft#1104, microsoft#1112, microsoft#1117, microsoft#1118, microsoft#1125, microsoft#1137, microsoft#1139, microsoft#1140, microsoft#1141, microsoft#1142, microsoft#1143, microsoft#1145, microsoft#1146, microsoft#1150, microsoft#1151, microsoft#1153, microsoft#1154, microsoft#1155, microsoft#1156, microsoft#1157, microsoft#1158, microsoft#1159, microsoft#1161, microsoft#1162, microsoft#1163, microsoft#1164, microsoft#1165, microsoft#1166, microsoft#1167, microsoft#1168, microsoft#1169, microsoft#1171, microsoft#1172, microsoft#1173, microsoft#1174, microsoft#1178
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
Merge pull request microsoft#1163 from katiewasnothere/virtual_functi…
9b14d0e 
…on_vpci

Add support for passing in a virtual function index to assign pci device
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anmaxvl anmaxvl anmaxvl approved these changes

+1 more reviewer

@dcantah dcantah dcantah approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dcantah dcantah

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@katiewasnothere @dcantah @anmaxvl