atlantic: guard rss_getkey/rss_gethashconfig with #ifdef RSS (fixes GENERIC link)#386
Conversation
aq_rss_prepare() called rss_getkey() and rss_gethashconfig()
unconditionally, but those symbols only exist when the kernel is built
with "options RSS" (net/rss_config.c is "optional inet rss"). The
atlantic driver enables hardware RSS whenever it has more than one RX
ring (aq_rss_enabled() checks rx_rings_count > 1), independent of the
kernel RSS framework, so these calls are reached in a stock GENERIC
kernel and the build fails to link:
ld: error: undefined symbol: rss_getkey
ld: error: undefined symbol: rss_gethashconfig
>>> referenced by aq_main.c ... aq_if_init
Guard the two calls with #ifdef RSS, mirroring dev/e1000/if_em.c: when
the RSS framework is absent, generate a random hash key with arc4rand()
and use the standard IPv4/IPv6 TCP and 2-tuple hash types (the RSS
subsystem default) so the NIC still distributes traffic across queues.
Fixes the GENERIC kernel link on builds without "options RSS".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds conditional compilation around RSS framework calls in the Atlantic driver so it builds without kernel RSS, and provides a fallback RSS key and hash configuration when RSS is not enabled. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates sys/dev/atlantic/aq_main.c to provide a fallback configuration when the kernel RSS framework is not defined, generating a random key and setting default hash types. The reviewer noted that when RSS is undefined, the RSS lookup table (softc->rss_lut) remains uninitialized, causing all traffic to route to queue 0. To fix this, the reviewer suggested adding a fallback block to initialize the lookup table with a default round-robin distribution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| *rss_hash_cfg = RSS_HASHTYPE_RSS_IPV4 | RSS_HASHTYPE_RSS_TCP_IPV4 | | ||
| RSS_HASHTYPE_RSS_IPV6 | RSS_HASHTYPE_RSS_TCP_IPV6 | | ||
| RSS_HASHTYPE_RSS_IPV6_EX | RSS_HASHTYPE_RSS_TCP_IPV6_EX; | ||
| #endif |
There was a problem hiding this comment.
While guarding rss_getkey and rss_gethashconfig with #ifdef RSS correctly fixes the link error on GENERIC kernels, there is a secondary issue: the subsequent #ifdef RSS block that populates the RSS lookup table (softc->rss_lut) will now be skipped entirely when RSS is not defined.
If softc->rss_lut is left uninitialized (all zeros), the hardware will route all incoming traffic to queue 0, effectively disabling multi-queue distribution and defeating the purpose of enabling RSS.
To fix this, you should also provide a fallback #else block for the LUT initialization to populate it with a default round-robin distribution. For example:
#ifdef RSS
if (rss_gethashalgo() == RSS_HASH_TOEPLITZ) {
uint32_t lut_size = hw_ops->get_rss_lut_size(softc->aq_hw);
for (i = 0; i < lut_size; i++) {
softc->rss_lut[i] = rss_get_indirection_to_bucket(i) %
softc->resources.num_rss_queues;
}
}
#else
uint32_t lut_size = hw_ops->get_rss_lut_size(softc->aq_hw);
for (i = 0; i < lut_size; i++) {
softc->rss_lut[i] = i % softc->resources.num_rss_queues;
}
#endifThere was a problem hiding this comment.
Thanks, but I believe this concern doesn't apply to this driver as written — the indirection table is already initialized in the non-RSS case.
The round-robin else block is deliberately placed outside the #ifdef RSS, so it is always compiled:
#ifdef RSS
if (rss_gethashalgo() == RSS_HASH_TOEPLITZ) {
for (i = 0; i < ARRAY_SIZE(softc->rss_table); i++)
softc->rss_table[i] =
(uint8_t)(rss_get_indirection_to_bucket(i) % qcnt);
} else
#endif
{
for (i = 0; i < ARRAY_SIZE(softc->rss_table); i++)
softc->rss_table[i] = (uint8_t)(i % qcnt); /* round-robin */
}When RSS is undefined the preprocessor removes only the #ifdef RSS … } else lines, leaving the trailing { … } round-robin loop intact. With qcnt == rx_rings_count, softc->rss_table[] is filled 0,1,…,qcnt-1,0,1,…, so the LUT is a valid round-robin distribution and traffic is spread across all RX queues — not pinned to queue 0.
(Note also that this driver's field is softc->rss_table, populated via ARRAY_SIZE(softc->rss_table) and qcnt; the suggested softc->rss_lut / get_rss_lut_size() / num_rss_queues don't exist here.)
So the only behavior dropped without the RSS framework is the Toeplitz indirection mapping, which correctly degrades to round-robin. No change needed here.
|
Claude finished @laffer1's task —— View job Code Review Complete ✅Reviewed atlantic driver RSS fix for correctness, style(9) compliance, and security issues:
Review SummaryOverall Assessment: ✅ APPROVED - This is a solid build fix with appropriate fallback logic. ✅ Correctness & Security
✅ Style(9) Compliance
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The non-RSS fallback hardcodes a specific set of RSS_HASHTYPE_* flags; consider centralizing or reusing an existing default (e.g., via a shared helper or macro) so behavior stays aligned if the system-wide RSS defaults change.
- It may be worth adding a brief note near the #ifdef RSS block explaining that the hash configuration and key choice must remain consistent with the hardware’s expectations and the RSS framework defaults to avoid subtle distribution differences.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The non-RSS fallback hardcodes a specific set of RSS_HASHTYPE_* flags; consider centralizing or reusing an existing default (e.g., via a shared helper or macro) so behavior stays aligned if the system-wide RSS defaults change.
- It may be worth adding a brief note near the #ifdef RSS block explaining that the hash configuration and key choice must remain consistent with the hardware’s expectations and the RSS framework defaults to avoid subtle distribution differences.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1 participant
Summary
aq_rss_prepare()in the atlantic (Aquantia/Marvell AQtion) driver callsrss_getkey()andrss_gethashconfig()unconditionally, but those symbols onlyexist when the kernel is built with
options RSS(net/rss_config.cisoptional inet rss). The driver enables hardware RSS whenever it has more thanone RX ring (
aq_rss_enabled()→rx_rings_count > 1), independent of the kernelRSS framework, so these calls are reached in a stock GENERIC kernel and the
build fails to link:
Fix
Guard the two calls with
#ifdef RSS, mirroring the established pattern indev/e1000/if_em.c. Without the RSS framework, generate a random hash key witharc4rand()and use the standard IPv4/IPv6 TCP + 2-tuple hash types (the RSSsubsystem default), so the NIC still distributes traffic across its queues. The
already-
#ifdef RSS-guarded indirection-table logic below is unchanged.Testing
aq_main.ocompiles clean under a no-RSS (GENERIC) config.symbols) — it previously failed at link as shown above.
This is an independent build fix (no functional change when
options RSSis set).🤖 Generated with Claude Code
Summary by Sourcery
Guard atlantic driver RSS configuration against builds without the kernel RSS framework to restore GENERIC kernel linkability.
Bug Fixes:
Enhancements: