Page MenuHomeFreeBSD

e1000: Improve device name strings
ClosedPublic

Authored by kbowling on Apr 20 2021, 8:59 PM.

Details

Summary

This is just clerical work to ease bug triage and may be used to set expectations around the ability for anyone in the community to perform testing and development on older parts (this driver covers over 20 years of silicon)

MFC: 2 weeks to stable/12 and stable/13

Test Plan

No functional change

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kbowling created this revision.
kbowling added a reviewer: jeb.

I consulted the PCI IDs database, and occasionally some data sheets, to produce this. I would like any feedback on the formatting, it can probably be slightly more regular. Tell me what to do and I'll do it :)

sys/dev/e1000/if_em.c
53–54

@erj @jeb I'm not sure if it's correct to include the family [MT, GT, XT etc] in some of these. Is that reserved for Intel add-in cards?

sys/dev/e1000/if_em.c
221

Going to add I347-AT4 for this one based on file:///tmp/mozilla_kev0090/ethernet-connection-i347-spec-update.pdf

kbowling marked an inline comment as not done.

Seems ok as a code change. I compared some of the changes with a pci id database but not exhaustively.

This revision is now accepted and ready to land.Apr 21 2021, 2:08 PM
sys/dev/e1000/if_em.c
53–54

Nah, I think it's fine. Though, I'm not sure how helpful it'll be since you helpfully put the MAC type and media type in the name.

105

You should be able to drop the "PRO/1000" part of the name for most of these. I don't know exactly which ones, but I picked this line because I know for certain that every I21x card doesn't have that.

113

If you want to do extra work, you could add the processor family (e.g. Skylake / Kaby Lake) to these I219 entries, so that they aren't just one list of 21 different MACs that only differ by number. Yoiu could do it to the other I21x entries, but I think it's easier to figure out which generation they are since there aren't very many.

203–242

I would also probably drop the PRO/1000 and PCI-Express Network Driver stuff, too. These are all going to be PCI-E, and like I said above, the PRO/1000 has been retired for a while, though there aren't really many "new" igb devices, unlike with em above.

This revision now requires review to proceed.Apr 21 2021, 9:17 PM

I'd like to commit this sometime in the next few days if there are no objections, can I get approval from a src committer? I will increase the MFC period to 1 month to allow more community feedback before it hits the stable branches. We have a while til the next minor releases so I'm not to worried in general.

This revision is now accepted and ready to land.Apr 25 2021, 9:04 PM

Just a Q: Are any of these changes likely to cause issues with respect to scripts / devd rules, or other 'automatic' things that might leverage these description? If so, is there anything in particular we might be able to do to mitigate that?

Just a Q: Are any of these changes likely to cause issues with respect to scripts / devd rules, or other 'automatic' things that might leverage these description? If so, is there anything in particular we might be able to do to mitigate that?

Hi, not that I know of. Because the names were all the same in a family, I predict most people would sooner script off of 'em', 'igb' before reaching for these unwieldy strings. It appears in dmesg during attach i.e. em0: <Intel(R) I211 (Copper)"> or sysctl dev.em.0.%desc. That could change now that they are more useful, but I would encourage folks to script off of PCI ID numbers before reaching for strings. Even the pci-ids database strings change frequently enough when corrections come in, but I do suspect people occasionally script off of those despite the lack of stability (i.e. pciconf -lv em0). Let me know if you find out anything different as I would like to MFC this after 1 month to stable/12 and stable/13.

Just a Q: Are any of these changes likely to cause issues with respect to scripts / devd rules, or other 'automatic' things that might leverage these description? If so, is there anything in particular we might be able to do to mitigate that?

Hi, not that I know of. Because the names were all the same in a family, I predict most people would sooner script off of 'em', 'igb' before reaching for these unwieldy strings. It appears in dmesg during attach i.e. em0: <Intel(R) I211 (Copper)"> or sysctl dev.em.0.%desc. That could change now that they are more useful, but I would encourage folks to script off of PCI ID numbers before reaching for strings. Even the pci-ids database strings change frequently enough when corrections come in, but I do suspect people occasionally script off of those despite the lack of stability (i.e. pciconf -lv em0). Let me know if you find out anything different as I would like to MFC this after 1 month to stable/12 and stable/13.

Agreed that dev ID's are better than strings. Do you think a RELNOTES (for future 12/13 at least) might be useful in anycase?

@koobs I don't have a strong opinion. I can set Relnotes: yes in the MFC so the decision can occur later on.