Skip to content

handle: enable VF info collection by default for pkgHandle - #1173

Open
zeeke wants to merge 1 commit into
vishvananda:mainfrom
zeeke:main
Open

handle: enable VF info collection by default for pkgHandle#1173
zeeke wants to merge 1 commit into
vishvananda:mainfrom
zeeke:main

Conversation

@zeeke

@zeeke zeeke commented Mar 18, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

The package-level handle (pkgHandle) was initialized without collectVFInfo, causing package-level functions like LinkByName to no longer collect Virtual Function information. This restores the previous default behavior.

Summary by CodeRabbit

  • Improvements
    • Virtual Function (VF) information collection is now enabled by default, providing enhanced visibility into network interface details. The existing option to disable this collection remains available if needed.

@zeeke
handle: enable VF info collection by default for pkgHandle
c9b22c9 
The package-level handle (pkgHandle) was initialized without
collectVFInfo, causing package-level functions like LinkByName to
no longer collect Virtual Function information. This restores the
previous default behavior.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
@coderabbitai

coderabbitai Bot commented Mar 18, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9c89483-0e02-4762-96a0-c99e17eed775

📥 Commits

Reviewing files that changed from the base of the PR and between ddba687 and c9b22c9.

📒 Files selected for processing (1)
  • handle_linux.go
📝 Walkthrough

Walkthrough

The default initialization of the package-level pkgHandle variable is modified to enable VF information collection by default instead of using an empty Handle. This single-line change affects the default behavior of VF info gathering for netlink handle operations.

Changes

Cohort / File(s) Summary
VF Info Collection Default
handle_linux.go		 Changed default pkgHandle initialization to enable collectVFInfo: true, modifying the default behavior of VF information collection across netlink handle usage.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A tiny change, but mighty in its way,
Default VF info now collected each day,
One line adjusted, behavior refined,
Better data gathering, peace of mind! 💫

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: enabling VF info collection by default for the package-level handle variable.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can enable review details to help with troubleshooting, context usage and more.

Enable the reviews.review_details setting to include review details such as the model used, the time taken for each step and more in the review comments.

@zeeke zeeke mentioned this pull request Mar 18, 2026
Merged
@ti-mo

ti-mo commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

I just filed #1174 which should address this by making the default value useful/backwards-compatible. Handle.DisableVFInfoCollection() is pretty painful to use as well. Note that it's pending the merge of #1170 so it may take a bit longer to land.

@aboch

aboch commented Mar 20, 2026

Copy link
Copy Markdown
Collaborator

Agree with @ti-mo, lets wait for his PR to move this extra flag under the options

@zeeke

zeeke commented Mar 26, 2026

Copy link
Copy Markdown
Author

hi! Thanks for the feedback. The solution proposed in #1174 looks way more robust than my changes.
BTW, since this PR is small and fixes a backward-compatibility break, I'd suggest moving forward with it so whoever is consuming the library from the main branch has a working dependency soon.

cc @ashokpariya0

@ashokpariya0

ashokpariya0 commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

@aboch can we please merge this pr , and we can then proceed with the more complete fix #1174 (currently in draft state)

@ashokpariya0

ashokpariya0 commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

@ti-mo Could you please share the current status of PR #1174?

@ashokpariya0

ashokpariya0 commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Agree with @ti-mo, lets wait for his PR to move this extra flag under the options

Hi @aboch @ti-mo

Right now, we’re unable to move forward in sriov-cni due to this dependency, blocked on this since few weeks,
Could you please help move this forward by merging this PR or #1174?
cc @SchSeba

@SchSeba SchSeba mentioned this pull request Jun 24, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zeeke @ti-mo @aboch @ashokpariya0