Page MenuHomeFreeBSD

first cut at reading igb vf stat support
ClosedPublic

Authored by jmg on Sep 1 2025, 10:04 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 11, 6:46 AM
Unknown Object (File)
Sat, Oct 11, 6:46 AM
Unknown Object (File)
Fri, Oct 10, 11:31 PM
Unknown Object (File)
Fri, Oct 10, 11:29 PM
Unknown Object (File)
Fri, Oct 10, 11:29 PM
Unknown Object (File)
Fri, Oct 10, 11:29 PM
Unknown Object (File)
Fri, Oct 10, 5:09 PM
Unknown Object (File)
Sat, Oct 4, 5:04 AM

Details

Reviewers
kbowling
kgalazka
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rGd2518851f2f9: e1000: fix igb VF stats
rG3c60ea77649d: e1000: fix igb VF stats
Summary

compile tested, but not run tested

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Sep 1 2025, 10:04 PM
jmg requested review of this revision.Sep 1 2025, 10:04 PM
jmg retitled this revision from first cut at readding igb stat support to first cut at reading igb vf stat support.Sep 1 2025, 10:06 PM
jmg added a reviewer: kbowling.

Note that this is only compiled tested. This was mainly to get a preliminary test patch out as it might be a couple days before I can do my own testing, and w/ 15 branch coming up, thought if someone had time to test/look at it in the meantime, we can compress the timeline.

overall looks fine

sys/dev/e1000/e1000_vf.h
165 ↗(On Diff #161346)

? Is this the right patch?

sys/dev/e1000/if_em.c
4789

style(9)

sys/dev/e1000/e1000_vf.h
165 ↗(On Diff #161346)

This header isn't referenced anywhere, so I'm thinking of deleting it. And it's been made mostly redundant by definitions in other headers.

sys/dev/e1000/if_em.c
4789

you mean the space between * and stats;? yeah, I probably copied that from stable/11's version. I'll fix.

sys/dev/e1000/e1000_vf.h
165 ↗(On Diff #161346)

That should be done in a separate review and you shouldn't commit this #error b change

sys/dev/e1000/if_em.c
4789

yes

address comments, add comment about union usage

jmg marked 4 inline comments as done.Sep 4 2025, 11:32 PM

fixed comments, commit message to be:

e1000: fix igb VF stats

igb VF must not read normal stat registers and only read a limited
set of registers.  The PF registers also don't make since as the VF
is an internal port, and there is no PHY to collect stats like CRC
errors from.

PR:     282309
Obtained from:  Juniper Networks, Inc.
Differential Revision:  https://reviews.freebsd.org/D52326

@jmg thank you for the patch! I started working on that issue, but a family emergency put me AFK for almost 2 weeks. To avoid further delay I think we should proceed with your version. It looks good to me, but let me run some basic tests on Monday. I'll approve this review as soon as I'm done.

@jmg thank you for the patch! I started working on that issue, but a family emergency put me AFK for almost 2 weeks. To avoid further delay I think we should proceed with your version. It looks good to me, but let me run some basic tests on Monday. I'll approve this review as soon as I'm done.

Thanks.

I've tested VF, but haven't been able to test PF as it's an involved process. I assume you'll do this testing on my behalf then and once verified, approve for me to commit?

@kgalazka

ping, any update on this?

Thanks.

We have tested PF, and verified that the sysctls are still there and have not changed.

In my testing it also looks good, both on PF and VF.

Once again thank you and my apologies for the delay.

This revision is now accepted and ready to land.Sep 12 2025, 12:44 PM
This revision was automatically updated to reflect the committed changes.