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

netkvm: enhancing host throughput by combining virtio header and data in a single descriptor, where possible #1089

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zjmletang
Copy link
Contributor

… in a single memory block

for the problem description
please visit #1078

Signed-off-by: Zhang JianMing [email protected]

… in a single memory block

for the problem description
please visit virtio-win#1078
@JonKohler
Copy link
Contributor

cc @sb-ntnx

@jumbi77
Copy link

jumbi77 commented Jul 7, 2024

What the status of this PR?

@YanVugenfirer
Copy link
Collaborator

@zjmletang Do you want to rebase and fix the CI issues?

@zjmletang
Copy link
Contributor Author

@zjmletang Do you want to rebase and fix the CI issues?

@YanVugenfirer Yes, I would like to rebase and fix the CI issues. However, I am currently away and may need a few more days to address this. Thank you for your understanding!

@YanVugenfirer
Copy link
Collaborator

@zjmletang No problem.

@zjmletang zjmletang changed the title netkvm: enhancing host throughput by combining virtio header and data in a single memory block netkvm: enhancing host throughput by combining virtio header and data in a single descriptor, where possible Dec 14, 2024
@zjmletang
Copy link
Contributor Author

@YanVugenfirer @ybendito
hi,I've taken some time to revisit this patch and have some concerns regarding its compatibility with the latest design principles in the master branch. Specifically, I see potential conflicts that I’d like to discuss regarding whether this patch can be merged.

To recap, this patch was originally submitted to facilitate discussion around the issue, and the consensus at that time was that the changes were too sensitive to be integrated into the upstream community for now.

Here are the specific points of conflict I’ve identified with the latest master:

The commit ec9e1f5
appears to focus on memory efficiency by employing a header block layout: virtio header + indirect area + tail . My patch, however, would disrupt this layout. I propose a structure that combines the virtio header and data, aiming to represent them with a single descriptor, while either allocating the indirect area separately or placing it at the end of the combined virtio header and data.. The goal of my change is to utilize a single descriptor to represent a receive buffer, which could reduce DMA requests on the host side (as detailed in the related issue).
I’m not sure if the community is open to considering this kind of change. I would appreciate your insights on whether this direction could be acceptable.

@JonKohler
Copy link
Contributor

If we can do it in a way that doesn't make things unstable, I would absolutely be open to this. Sounds like a great idea

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