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.

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

Add support for Intel X710 Backplane and Base T #452

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Add support for Intel X710 Backplane and Base T #452

merged 1 commit into from
Jun 29, 2023

Conversation

vrindle
Copy link
Contributor

@vrindle vrindle commented Jun 5, 2023 •
edited
Loading

Closes #453

@github-actions
Copy link

github-actions bot commented Jun 5, 2023

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Jun 5, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 5406644637

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.07%) to 25.727%
Files with Coverage Reduction New Missed Lines %
controllers/sriovibnetwork_controller.go 2 64.15%
controllers/sriovnetwork_controller.go 2 65.71%
api/v1/helper.go 3 41.32%
Totals Coverage Status
Change from base Build 5268039487: -0.07%
Covered Lines: 1954
Relevant Lines: 7595
💛 - Coveralls

@vrindle vrindle force-pushed the add_support_Intel_X710_Nics branch from ef8d2aa to 2b316cd Compare June 5, 2023 18:28
@github-actions
Copy link

github-actions bot commented Jun 5, 2023

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@bn222
Copy link
Collaborator

bn222 commented Jun 5, 2023

/lgtm

@github-actions github-actions bot added the lgtm label Jun 5, 2023
@wizhaoredhat
Copy link
Contributor

Please follow: https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/supported-hardware.md#initial-support

@SchSeba
Copy link
Collaborator

SchSeba commented Jun 19, 2023

@Eoghan1232 please review as this are intel nics :)

Eoghan1232
Eoghan1232 reviewed Jun 20, 2023
View reviewed changes
doc/supported-hardware.md Outdated
Comment on lines 47 to 49
| Intel X710 10GbE Backplane | 8086 | 1581 | V | V | X |
| Intel X710 10Gbe Base T | 8086 | 15ff | V | V | X |
| Intel X550 Family | V | V | X |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| Intel X710 10GbE Backplane | 8086 | 1581 | V | V | X |
| Intel X710 10Gbe Base T | 8086 | 15ff | V | V | X |
| Intel X550 Family | V | V | X |
| Intel X710 10GbE Backplane | V | V | X |
| Intel X710 10Gbe Base T | V | V | X |
| Intel X550 Family | V | V | X |

please remove the vendor and device ID from this section.

Copy link
Collaborator

Choose a reason for hiding this comment

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

also please change both to GbE

@Eoghan1232
Copy link
Collaborator

Intel X710 10Gbe Base T
I valdiated this NIC, however I do not have the other NIC to test.

I assume these were validated @vrindle as you are currently using them :)

Can you address the comments above and we can get this merged for you :)

@vrindle vrindle force-pushed the add_support_Intel_X710_Nics branch from 2b316cd to 2416da1 Compare June 28, 2023 20:40
@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@vrindle
Copy link
Contributor Author

vrindle commented Jun 28, 2023

Intel X710 10Gbe Base T I valdiated this NIC, however I do not have the other NIC to test.

I assume these were validated @vrindle as you are currently using them :)

Can you address the comments above and we can get this merged for you :)

I validated both of these NIC just yesterday using this guide: https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/supported-hardware.md#initial-support. The comments are addressed.

@vrindle vrindle requested a review from Eoghan1232 June 28, 2023 21:00
@Eoghan1232
Copy link
Collaborator

Intel X710 10Gbe Base T I valdiated this NIC, however I do not have the other NIC to test.
I assume these were validated @vrindle as you are currently using them :)
Can you address the comments above and we can get this merged for you :)

I validated both of these NIC just yesterday using this guide: https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/supported-hardware.md#initial-support. The comments are addressed.

Hi @vrindle , you need to add this to the Supported features per hardware of the doc/supported-hardware

| Intel X710 10GbE Backplane | V | V | X |
| Intel X710 10Gbe Base T    | V | V | X |

I think you removed it by mistake as you had it in already :)

@vrindle vrindle force-pushed the add_support_Intel_X710_Nics branch from 2416da1 to f5371cf Compare June 28, 2023 23:00
@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@vrindle vrindle force-pushed the add_support_Intel_X710_Nics branch from f5371cf to 28df315 Compare June 28, 2023 23:01
@github-actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@vrindle
Copy link
Contributor Author

vrindle commented Jun 28, 2023

| Intel X710 10GbE Backplane | V | V | X |

Addressed

Eoghan1232
Eoghan1232 approved these changes Jun 29, 2023
View reviewed changes
Copy link
Collaborator

@Eoghan1232 Eoghan1232 left a comment

Choose a reason for hiding this comment

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

lgtm! thanks for fixing those :)

@adrianchiris adrianchiris merged commit b9102a7 into k8snetworkplumbingwg:master Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eoghan1232 Eoghan1232 Eoghan1232 approved these changes

@adrianchiris adrianchiris adrianchiris approved these changes

Assignees
No one assigned
Labels
lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Intel X710 Backplane and Base T
7 participants
@vrindle @coveralls @bn222 @wizhaoredhat @SchSeba @Eoghan1232 @adrianchiris