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 mlx5 vf as PF1 proxy #614

Merged
merged 11 commits into from
Oct 17, 2024
Merged

Use mlx5 vf as PF1 proxy #614

merged 11 commits into from
Oct 17, 2024

Conversation

PlagueCZ
Copy link
Contributor

@PlagueCZ PlagueCZ commented Oct 1, 2024

Given TAP device's limitations, we implemented the PF1-proxy using a VF bound to MLX5 driver (as opposed to VFIO). This VF is created on PF1 to make it easier to set up.

This also enables offloading of the proxy path. For normal operation this is easy, for OSC vith virtual services, there is a little bit more work, but nothing major.

I tried to reorder and squash commits into blocks of work, so this PR should be nicely readable by going through it commit-by-commit.

I had to change the prepare script, but it should be backwards-compatible. There is one reordering (setting switchdev before setting the number of VFs) that is needed for multiport mode. OSC is also now provisioning the HW differently, so I only tested the prepare script's HW setup by hand locally.

Connected to #606

@github-actions github-actions bot added size/XXL documentation Improvements or additions to documentation enhancement New feature or request labels Oct 1, 2024
@PlagueCZ PlagueCZ marked this pull request as ready for review October 1, 2024 16:38
@PlagueCZ PlagueCZ requested a review from a team as a code owner October 1, 2024 16:38
@byteocean
Copy link
Contributor

byteocean commented Oct 2, 2024

@PlagueCZ Thanks for providing this enhancement of using VF as the proxy node, which is very interesting & nicely done. I would also suggest to add the tx node of the proxy to the node graph in the documentation, but as a separate one. The prep script works on my testing machine.

@PlagueCZ
Copy link
Contributor Author

PlagueCZ commented Oct 2, 2024

As for the graph schema, I thought about it, but given that virtual services are also not there because they are in an ifdef, I dod not do it.

But if it's wanted I could create two copies of the shema and create a virtual-services-specific one and pf1-proxy-specific one.

@PlagueCZ PlagueCZ force-pushed the feature/pf1_proxy_mlx5 branch 2 times, most recently from bcad48b to 2aecca3 Compare October 3, 2024 13:35
docs/sys_design/README.md Outdated Show resolved Hide resolved
docs/sys_design/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@byteocean byteocean left a comment

Choose a reason for hiding this comment

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

LGTM.

@guvenc
Copy link
Collaborator

guvenc commented Oct 16, 2024

@PlagueCZ
I am still going through the changes but one thing which popped into my mind is that clang build which is run anyways and produces throw away binary during the CI/CD can include the compiler flag of pf1 proxy so that this newly introduced code path gets compiled at least once during CI/CD.

@PlagueCZ
Copy link
Contributor Author

We should really run a tester in a job in OSC, but unfortunately that is still not being done.
We should then add another compilation run though, because I already had clang catch a warning about unused variable, which was unused due to pf1-proxy not being there

Copy link
Collaborator

@guvenc guvenc left a comment

Choose a reason for hiding this comment

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

@PlagueCZ
I think this PR is in good shape. Thanks also for providing the updated dataplane graph schema.

  • Let's add a (temporary) compilation step in Dockerfile where this newly introduced feature is enabled, as discussed.

  • And also the clarification whether the RoCE needs to be disabled on firmware level to get a good performance when mpesw is enabled. If so, documenting it in the docs with a sentence or so.

  • And also please rebase to main.

Apart from these things, in pretty good shape. Thanks !

@guvenc guvenc merged commit 17bb85c into main Oct 17, 2024
6 checks passed
@guvenc guvenc deleted the feature/pf1_proxy_mlx5 branch October 17, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants