DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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>
---
 drivers/net/sfc/sfc_flow_rss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox