* [PATCH 0/3] net/sfc: miscellaneous bug fixes
@ 2026-08-11 17:49 Ivan Malov
2026-08-11 17:49 ` [PATCH 1/3] net/sfc: set Rx queue type flags from scratch on queue setup Ivan Malov
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Ivan Malov @ 2026-08-11 17:49 UTC (permalink / raw)
To: dev
Cc: Andy Moreton, Viacheslav Galaktionov, Roman Zhukov,
Pieter Jansen van Vuuren, Stephen Hemminger, Andrew Rybchenko
Three independent fixes. The first ensures that Rx queue
type flags are derived from scratch on every queue setup,
preventing flags from a prior configuration persisting
when an offload is disabled.
The second patch removes an erroneous static qualifier
from a flow RSS iterator variable, which incorrectly
shared state between multiple interfaces.
The third patch corrects reading of the advertised
autoneg capability. When the user disables it, the
corresponding bit was re-added upon the next
link-state query.
Ivan Malov (3):
net/sfc: set Rx queue type flags from scratch on queue setup
net/sfc: drop wrong static qualifier from iterator variable
common/sfc_efx/base: fix reading advertised autoneg ability
drivers/common/sfc_efx/base/efx_np.c | 11 +++++------
drivers/common/sfc_efx/base/medford4_phy.c | 6 +++++-
drivers/net/sfc/sfc_flow_rss.c | 2 +-
drivers/net/sfc/sfc_rx.c | 2 +-
4 files changed, 12 insertions(+), 9 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] net/sfc: set Rx queue type flags from scratch on queue setup
2026-08-11 17:49 [PATCH 0/3] net/sfc: miscellaneous bug fixes Ivan Malov
@ 2026-08-11 17:49 ` Ivan Malov
2026-08-11 17:49 ` [PATCH 2/3] net/sfc: drop wrong static qualifier from iterator variable Ivan Malov
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Ivan Malov @ 2026-08-11 17:49 UTC (permalink / raw)
To: dev
Cc: Andy Moreton, Viacheslav Galaktionov, Roman Zhukov,
Pieter Jansen van Vuuren, Stephen Hemminger, Andrew Rybchenko,
stable
Derive queue type flags from scratch on every queue setup
so that flags left over from a previous configuration do
not persist when an offload is disabled.
Fixes: b8cf5ba549f2 ("net/sfc: support initialising different Rx queue types")
Cc: stable@dpdk.org
Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Andy Moreton <andy.moreton@amd.com>
---
drivers/net/sfc/sfc_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index d8b961ff7e..e049beced5 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -1183,7 +1183,7 @@ sfc_rx_qinit(struct sfc_adapter *sa, sfc_sw_index_t sw_index,
else
rxq_info->type = EFX_RXQ_TYPE_DEFAULT;
- rxq_info->type_flags |=
+ rxq_info->type_flags =
(offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ?
EFX_RXQ_FLAG_SCATTER : EFX_RXQ_FLAG_NONE;
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] net/sfc: drop wrong static qualifier from iterator variable
2026-08-11 17:49 [PATCH 0/3] net/sfc: miscellaneous bug fixes Ivan Malov
2026-08-11 17:49 ` [PATCH 1/3] net/sfc: set Rx queue type flags from scratch on queue setup Ivan Malov
@ 2026-08-11 17:49 ` Ivan Malov
2026-08-11 17:49 ` [PATCH 3/3] common/sfc_efx/base: fix reading advertised autoneg ability Ivan Malov
2026-08-11 20:24 ` [PATCH 0/3] net/sfc: miscellaneous bug fixes Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Ivan Malov @ 2026-08-11 17:49 UTC (permalink / raw)
To: dev
Cc: Andy Moreton, Viacheslav Galaktionov, Roman Zhukov,
Pieter Jansen van Vuuren, Stephen Hemminger, Andrew Rybchenko,
stable
Obviously, this should be per interface state.
Static wrongly turns it into global state.
Fixes: 6da67e706dc9 ("net/sfc: rework flow action RSS support")
Cc: stable@dpdk.org
Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
Reviewed-by: Andy Moreton <andy.moreton@amd.com>
---
| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--git a/drivers/net/sfc/sfc_flow_rss.c b/drivers/net/sfc/sfc_flow_rss.c
index 8e2749833b..f19d6f00c5 100644
--- a/drivers/net/sfc/sfc_flow_rss.c
+++ b/drivers/net/sfc/sfc_flow_rss.c
@@ -202,7 +202,7 @@ sfc_flow_rss_ctx_reuse(struct sfc_adapter *sa,
uint16_t sw_qid_min, const uint16_t *sw_qids)
{
struct sfc_flow_rss *flow_rss = &sa->flow_rss;
- static struct sfc_flow_rss_ctx *ctx;
+ struct sfc_flow_rss_ctx *ctx;
SFC_ASSERT(sfc_adapter_is_locked(sa));
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] common/sfc_efx/base: fix reading advertised autoneg ability
2026-08-11 17:49 [PATCH 0/3] net/sfc: miscellaneous bug fixes Ivan Malov
2026-08-11 17:49 ` [PATCH 1/3] net/sfc: set Rx queue type flags from scratch on queue setup Ivan Malov
2026-08-11 17:49 ` [PATCH 2/3] net/sfc: drop wrong static qualifier from iterator variable Ivan Malov
@ 2026-08-11 17:49 ` Ivan Malov
2026-08-11 20:24 ` [PATCH 0/3] net/sfc: miscellaneous bug fixes Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Ivan Malov @ 2026-08-11 17:49 UTC (permalink / raw)
To: dev
Cc: Andy Moreton, Viacheslav Galaktionov, Roman Zhukov,
Pieter Jansen van Vuuren, Stephen Hemminger, Andrew Rybchenko,
stable
The issue is that when the user disables auto-negotiation by removing
the capability bit from the 'advertised mask' (set method) and then
reads the resulting capabilities, which involves querying MCDI, the
bit reappears in the mask irrespective of the user's intent.
Fix this by remembering the user's intent before any link-state queries.
Fixes: 2a5cf77e6de8 ("common/sfc_efx/base: provide PHY link get method on Medford4")
Fixes: 06f569de6c06 ("common/sfc_efx/base: decode netport link state on probe path")
Cc: stable@dpdk.org
Suggested-by: Andy Moreton <andy.moreton@amd.com>
Signed-off-by: Ivan Malov <ivan.malov@arknetworks.am>
Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
Reviewed-by: Andy Moreton <andy.moreton@amd.com>
---
drivers/common/sfc_efx/base/efx_np.c | 11 +++++------
drivers/common/sfc_efx/base/medford4_phy.c | 6 +++++-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/common/sfc_efx/base/efx_np.c b/drivers/common/sfc_efx/base/efx_np.c
index 4ba3c7d260..a8396535bf 100644
--- a/drivers/common/sfc_efx/base/efx_np.c
+++ b/drivers/common/sfc_efx/base/efx_np.c
@@ -436,9 +436,6 @@ efx_np_link_state(
MCDI_OUT2(req, const uint8_t, LINK_STATE_OUT_ADVERTISED_ABILITIES),
&lsp->enls_adv_cap_mask);
- if (lsp->enls_an_supported != B_FALSE)
- lsp->enls_adv_cap_mask |= 1U << EFX_PHY_CAP_AN;
-
efx_np_cap_hw_data_to_sw_mask(
MCDI_OUT2(req, const uint8_t,
LINK_STATE_OUT_LINK_PARTNER_ABILITIES),
@@ -1020,11 +1017,13 @@ efx_np_attach(
if (rc != 0)
goto fail3;
- if (ls.enls_an_supported != B_FALSE)
- epp->ep_phy_cap_mask |= 1U << EFX_PHY_CAP_AN;
-
epp->ep_adv_cap_mask = ls.enls_adv_cap_mask;
+ if (ls.enls_an_supported != B_FALSE) {
+ epp->ep_adv_cap_mask |= 1U << EFX_PHY_CAP_AN;
+ epp->ep_phy_cap_mask |= 1U << EFX_PHY_CAP_AN;
+ }
+
#if EFSYS_OPT_LOOPBACK
efx_np_assign_loopback_props(enp);
#endif /* EFSYS_OPT_LOOPBACK */
diff --git a/drivers/common/sfc_efx/base/medford4_phy.c b/drivers/common/sfc_efx/base/medford4_phy.c
index 7b456c9b8a..4d2a954d4f 100644
--- a/drivers/common/sfc_efx/base/medford4_phy.c
+++ b/drivers/common/sfc_efx/base/medford4_phy.c
@@ -32,15 +32,19 @@ medford4_phy_get_link(
__out ef10_link_state_t *elsp)
{
efx_np_handle_t nph = enp->en_port.ep_np_handle;
+ const efx_port_t *port = &enp->en_port;
efx_np_link_state_t ls;
efx_np_mac_state_t ms;
+ uint32_t preserve_an;
efx_rc_t rc;
+ preserve_an = port->ep_adv_cap_mask & (1U << EFX_PHY_CAP_AN);
+
rc = efx_np_link_state(enp, nph, &ls);
if (rc != 0)
goto fail1;
- elsp->epls.epls_adv_cap_mask = ls.enls_adv_cap_mask;
+ elsp->epls.epls_adv_cap_mask = ls.enls_adv_cap_mask | preserve_an;
elsp->epls.epls_lp_cap_mask = ls.enls_lp_cap_mask;
elsp->epls.epls_lane_count = ls.enls_lane_count;
elsp->els_loopback = ls.enls_loopback;
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 0/3] net/sfc: miscellaneous bug fixes
2026-08-11 17:49 [PATCH 0/3] net/sfc: miscellaneous bug fixes Ivan Malov
` (2 preceding siblings ...)
2026-08-11 17:49 ` [PATCH 3/3] common/sfc_efx/base: fix reading advertised autoneg ability Ivan Malov
@ 2026-08-11 20:24 ` Stephen Hemminger
3 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2026-08-11 20:24 UTC (permalink / raw)
To: Ivan Malov
Cc: dev, Andy Moreton, Viacheslav Galaktionov, Roman Zhukov,
Pieter Jansen van Vuuren, Andrew Rybchenko
On Tue, 11 Aug 2026 21:49:10 +0400
Ivan Malov <ivan.malov@arknetworks.am> wrote:
> Three independent fixes. The first ensures that Rx queue
> type flags are derived from scratch on every queue setup,
> preventing flags from a prior configuration persisting
> when an offload is disabled.
>
> The second patch removes an erroneous static qualifier
> from a flow RSS iterator variable, which incorrectly
> shared state between multiple interfaces.
>
> The third patch corrects reading of the advertised
> autoneg capability. When the user disables it, the
> corresponding bit was re-added upon the next
> link-state query.
>
> Ivan Malov (3):
> net/sfc: set Rx queue type flags from scratch on queue setup
> net/sfc: drop wrong static qualifier from iterator variable
> common/sfc_efx/base: fix reading advertised autoneg ability
>
> drivers/common/sfc_efx/base/efx_np.c | 11 +++++------
> drivers/common/sfc_efx/base/medford4_phy.c | 6 +++++-
> drivers/net/sfc/sfc_flow_rss.c | 2 +-
> drivers/net/sfc/sfc_rx.c | 2 +-
> 4 files changed, 12 insertions(+), 9 deletions(-)
>
Detailed AI review found some issues here.
Review of [PATCH 0/3] SFC bug fixes (Ivan Malov)
Patch 1/3 - net/sfc: set Rx queue type flags from scratch on queue setup
Error: replacing "|=" with "=" discards extra type flags that callers
seed via sfc_rx_qinit_info() immediately before sfc_rx_qinit().
Two call sites do this today:
drivers/net/sfc/sfc_repr_proxy.c:556
sfc_rx_qinit_info(sa, rxq->sw_index, EFX_RXQ_FLAG_INGRESS_MPORT);
sfc_rx_qinit(sa, rxq->sw_index, ...); /* line 562 */
drivers/net/sfc/sfc_mae_counter.c:870
sfc_rx_qinit_info(sa, sa->counter_rxq.sw_index,
EFX_RXQ_FLAG_USER_MARK);
sfc_rx_qinit(sa, sa->counter_rxq.sw_index, ...); /* line 875 */
sfc_rx_qinit_info() does "rxq_info->type_flags = extra_efx_type_flags"
(sfc_rx.c:1663). The "|=" at sfc_rx.c:1186 was what preserved that
value; with "=" it is overwritten before anything reads it.
Two consequences:
- rxq_info->type_flags is passed straight to efx_rx_qcreate()
(sfc_rx.c:822 and :842), so the hardware Rx prefix no longer
carries ingress mport / user mark for those queues.
- sfc_rx.c:1240 derives SFC_RXQ_FLAG_INGRESS_MPORT from type_flags,
and sfc_ef100_rx.c:824 consumes it. Representor proxy demux
loses its mport field.
EFX_RXQ_FLAG_USER_MARK is re-added at sfc_rx.c:1203, but only when
RTE_ETH_RX_METADATA_USER_MARK was negotiated or flow tunnel is active
- neither holds for the MAE counter queue.
The underlying problem the patch describes is real: sfc_rx_configure()
only calls sfc_rx_qinit_info() for newly added queues (sfc_rx.c:1837,
inside "while (sas->ethdev_rxq_count < nb_rx_queues)"), so existing
ethdev queues keep stale flags across a reconfigure. The fix needs to
reset only the offload-derived bits. Suggested approach: record the
caller-supplied flags in a separate field, e.g.
/* sfc_rx_qinit_info() */
rxq_info->extra_type_flags = extra_efx_type_flags;
rxq_info->type_flags = extra_efx_type_flags;
/* sfc_rx_qinit() */
rxq_info->type_flags = rxq_info->extra_type_flags |
((offloads & RTE_ETH_RX_OFFLOAD_SCATTER) ?
EFX_RXQ_FLAG_SCATTER : EFX_RXQ_FLAG_NONE);
Patch 2/3 - net/sfc: drop wrong static qualifier from iterator variable
Info: the change is correct, but the commit message overstates the
impact. TAILQ_FOREACH() assigns the variable from TAILQ_FIRST()
before the first iteration, and every return path returns a value
produced inside the loop, so the static storage is never read stale.
There is no observable misbehaviour to backport a fix for. Consider
rewording as a cleanup (unnecessary global state, not thread-safe by
construction) and dropping the Cc: stable and Fixes: tags, or state
explicitly that no functional change is expected.
Patch 3/3 - common/sfc_efx/base: fix reading advertised autoneg ability
Error: the first efx_np.c hunk does not apply to main. The patch
expects the block
if (lsp->enls_an_supported != B_FALSE)
lsp->enls_adv_cap_mask |= 1U << EFX_PHY_CAP_AN;
to sit after the LINK_STATE_OUT_ADVERTISED_ABILITIES conversion, but
upstream has it before it (efx_np.c:432), and at the position the
patch expects upstream has
if (status_flags & (1U << MC_CMD_LINK_STATUS_FLAGS_AN_ABLE))
lsp->enls_lp_cap_mask |= 1U << EFX_PHY_CAP_AN;
from 70857163b72a ("common/sfc_efx/base: fix autoneg detection with
netport MCDI"). git am and git apply -3 both fail on that hunk; the
efx_np_attach() hunk and medford4_phy.c apply cleanly. Please rebase
on main and resend.
The logic itself checks out on the rebased placement. Removing the
AN bit from enls_adv_cap_mask leaves only two consumers, and both are
covered: efx_np_attach() (efx_np.c:1005) now sets the bit itself, and
medford4_phy_get_link() (medford4_phy.c:43) restores it from
ep_adv_cap_mask. medford4_mac_poll() writes the result back into
ep_adv_cap_mask, so the bit is self-sustaining, and clearing it via
efx_phy_adv_cap_set() sticks. Setting it back is gated on
ep_phy_cap_mask (efx_phy.c:264), which attach only populates when
AN is supported, so the preserved bit cannot outlive AN support.
Info: the added local
const efx_port_t *port = &enp->en_port;
is used once and the file otherwise reaches through enp->en_port
directly (line 33) or names the local "epp" (medford4_phy_reconfigure,
medford4_mac_poll). Suggest dropping it:
preserve_an = enp->en_port.ep_adv_cap_mask &
(1U << EFX_PHY_CAP_AN);
Fixes: tags in all three patches resolve to real commits with matching
subjects.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-11 20:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 17:49 [PATCH 0/3] net/sfc: miscellaneous bug fixes Ivan Malov
2026-08-11 17:49 ` [PATCH 1/3] net/sfc: set Rx queue type flags from scratch on queue setup Ivan Malov
2026-08-11 17:49 ` [PATCH 2/3] net/sfc: drop wrong static qualifier from iterator variable Ivan Malov
2026-08-11 17:49 ` [PATCH 3/3] common/sfc_efx/base: fix reading advertised autoneg ability Ivan Malov
2026-08-11 20:24 ` [PATCH 0/3] net/sfc: miscellaneous bug fixes Stephen Hemminger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.