net, passt: Rm optimization requirements #933

nirdothan · 2025-09-25T14:27:46Z

What this PR does / why we need it:
Remove redundant node optimization requirements which are no longer required, and update some stale information that is no longer relevant.
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note:

NONE

nirdothan · 2025-09-25T14:28:40Z

/cc @orelmisan @frenzyfriday @sbrivio-rh

kubevirt-bot · 2025-09-25T14:28:46Z

@nirdothan: GitHub didn't allow me to request PR reviews from the following users: sbrivio-rh.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @orelmisan @frenzyfriday @sbrivio-rh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

sbrivio-rh · 2025-09-25T20:38:54Z

It all looks good to me!

Just a note about wmem_max and rmem_max tuning: the current status is that we haven't assessed performance benefits and drawbacks with those in a long time. We have limited evidence that they're not necessary because TCP auto-tuning appears to do the trick nowadays (especially in combination with the vhost-user interface), but we didn't really run a thorough comparison. We have https://bugs.passt.top/show_bug.cgi?id=138 filed for that.

For the moment being, I think it makes sense to drop those from the user guide, and re-introduce them (with whatever value) in the unlikely case we find they still give us a substantial throughput advantage.

orelmisan

Thank you for the PR @nirdothan!
Just one small comment.

orelmisan · 2025-09-28T14:46:10Z

docs/network/net_binding_plugins/passt.md

 The relevant sidecar image needs to be accessible by the cluster and
 specified in the Kubevirt CR when registering the network binding plugin.

-### Feature Gate


This section needs to be retained for backward compatibility with KubeVirt versions prior to v1.5, which are still within our support window. Additionally, this change appears unrelated to the optimizations described in the commit message.

you might want to see conflicts about #926

imo keep the PR focusing only on the optimization req, nothing else

nirdothan · 2025-09-28T15:06:49Z

Restored FG section fot the benefit of legacy versions <1.15 users.

oshoval · 2025-09-28T15:10:03Z

docs/network/net_binding_plugins/passt.md

                    "computeResourceOverhead": {
                      "requests": {
-                        "memory": "500Mi",
+                        "memory": "250Mi",


regarding memory changes
there is lgtmed PR that already does those
#916

if something is missing, it is better to comment there instead please

Won't the merge take care of that?

if 916 is merged first then yes, otherwise, it doesnt make sense to include it in this PR
so in both cases it doesnt need to be in this PR, moreover it is not related to PR desc

I don't see how this can cause a problem, and 916 should be merged.

orelmisan · 2025-09-29T12:46:11Z

docs/network/net_binding_plugins/passt.md

-### Feature Gate
-If not already set, add the `NetworkBindingPlugins` FG.
+### Feature Gate - prior to v1.15
+For KubeVirt versions prior to 1.15, make sure to enable the `NetworkBindingPlugins` FG.


There is a typo in the version number (twice).
Please consider dropping the prior to v1.15 suffix from the title.

Remove redundant node optimization requirements which are no longer required, and update some stale information that is no longer relevant. Signed-off-by: Nir Dothan <[email protected]>

nirdothan · 2025-09-29T14:58:51Z

Fixed typos.

orelmisan

Thank you @nirdothan!
/approve

kubevirt-bot · 2025-09-29T16:28:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: orelmisan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~docs/network/OWNERS~~ [orelmisan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

frenzyfriday

/lgtm

Thanks!

kubevirt-bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Sep 25, 2025

kubevirt-bot requested review from EdDev and RamLavi September 25, 2025 14:27

kubevirt-bot added the size/S label Sep 25, 2025

kubevirt-bot requested review from frenzyfriday and orelmisan September 25, 2025 14:28

orelmisan suggested changes Sep 28, 2025

View reviewed changes

kubevirt-bot assigned orelmisan Sep 28, 2025

nirdothan force-pushed the passt-alpha branch from 0d9be45 to 68f997d Compare September 28, 2025 15:04

oshoval reviewed Sep 28, 2025

View reviewed changes

orelmisan suggested changes Sep 29, 2025

View reviewed changes

net, passt: Rm optimization requirements

7c37e6d

Remove redundant node optimization requirements which are no longer required, and update some stale information that is no longer relevant. Signed-off-by: Nir Dothan <[email protected]>

nirdothan force-pushed the passt-alpha branch from 68f997d to 7c37e6d Compare September 29, 2025 14:58

orelmisan reviewed Sep 29, 2025

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 29, 2025

frenzyfriday reviewed Sep 30, 2025

View reviewed changes

kubevirt-bot assigned frenzyfriday Sep 30, 2025

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2025

kubevirt-bot merged commit 79896bf into kubevirt:main Sep 30, 2025
6 checks passed

orelmisan mentioned this pull request Sep 30, 2025

passt: Fix docs #916

Merged

8 tasks

net, passt: Rm optimization requirements #933

net, passt: Rm optimization requirements #933

Uh oh!

Conversation

nirdothan commented Sep 25, 2025

Uh oh!

nirdothan commented Sep 25, 2025

Uh oh!

kubevirt-bot commented Sep 25, 2025

Uh oh!

sbrivio-rh commented Sep 25, 2025

Uh oh!

orelmisan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nirdothan commented Sep 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nirdothan commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orelmisan left a comment

Choose a reason for hiding this comment

Uh oh!

kubevirt-bot commented Sep 29, 2025

Uh oh!

frenzyfriday left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nirdothan commented Sep 29, 2025 •

edited

Loading