DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Ivan Malov <ivan.malov@arknetworks.am>
Cc: dev@dpdk.org, Andy Moreton <andy.moreton@amd.com>,
	Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>,
	Roman Zhukov <Roman.Zhukov@arknetworks.am>,
	Pieter Jansen van Vuuren <pieter.jansen-van-vuuren@amd.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v2 0/6] common/sfc_efx/base: add Medford4 VF support
Date: Sat, 15 Aug 2026 20:27:22 -0700	[thread overview]
Message-ID: <20260815202722.1abe35ed@phoenix.local> (raw)
In-Reply-To: <20260812171103.8553-1-ivan.malov@arknetworks.am>

On Wed, 12 Aug 2026 21:10:57 +0400
Ivan Malov <ivan.malov@arknetworks.am> wrote:

> This series enables DPDK to use the sfc driver
> on a Medford4 VF alongside the PF.
> 
> The first patch wires EVB switch operations into the Medford4
> libefx implementation, allowing the PF to manage VFs.
> 
> Starting with MCFW 1.4.0.8, VFs may use the netport MCDI for basic port
> configuration, though several operations remain restricted. The
> remaining four patches address each restriction: dummy fixed
> port properties, suppressed event subscription, denied FCS
> and flow control, and ENOTSUP for periodic MAC stats DMA.
> 
> This series depends on the VADAPTER statistics series.

I decided to hold off on this part, need to look at these AI nits.

Reviewed v3 applied on top of series-38976 plus the net/sfc bugfix
series. check-git-log passes. Two issues below; no Reviewed-by yet.

1. Patch 2/6: PERM derived from the current advertisement

	epp->ep_phy_cap_mask = epp->ep_adv_cap_mask;

ep_adv_cap_mask comes from LINK_STATE_OUT_ADVERTISED_ABILITIES, so
EFX_PHY_CAP_PERM now tracks what the port happens to advertise at
probe rather than what it can support. If that mask has no speed bit
-- no module, or link down at probe -- then in net/sfc

	port->phy_adv_cap_mask = PERM;                 (sfc_port.c:450)
	phy_adv_cap = caps_from_link_speeds(speeds) & phy_adv_cap_mask;
	if ((phy_adv_cap & ~(1 << EFX_PHY_CAP_AN)) == 0)
		return EINVAL;                        (sfc.c:164)

and rte_eth_dev_configure() fails with "No link speeds from mask ...
are supported". The VF stays unusable until it is re-probed with the
link up. A PF is not exposed to this because its PERM comes from
GET_FIXED_PORT_PROPERTIES, which does not vary with link state. The
snapshot is also never refreshed, so a later PF-side advertisement
change leaves the VF's PERM stale.

The comment says link state gives a clue to "which link speed is de
facto active", but the code reads the advertised mask, not
ls.enls_speed / LINK_TECHNOLOGY.

Since medford4_phy_reconfigure() returns early on a VF (no LINK or
ADMIN privilege, so efx_np_link_ctrl() is unreachable), nothing on a
VF can program the PHY anyway and a permissive PERM costs nothing.
Seeding it from the speeds efx_np_cap_map_tech can express would avoid
both problems.

2. Patch 6/6: VF restrictions undocumented

The bare "SR-IOV VF" bullet documents none of what 2/6 to 5/6 encode:

 - flow_ctrl_set with autoneg=0 returns -ENOTSUP (4/6)
 - RTE_ETH_RX_OFFLOAD_KEEP_CRC returns -ENOTSUP (4/6)
 - link status change events are never delivered (3/6)
 - periodic MAC stats DMA unavailable; one-shot uploads instead (5/6)
 - no PHY configuration at all: link_speeds, FEC mode and lane count
   are accepted and silently ignored

The last is the one I would most want written down -- a fixed link
speed on a VF gets no error and no effect. sfc_efx.rst:137 already has
a Limitations section, which looks like the right home, along with the
MCFW 1.4.0.8 floor from 2/6's commit message (older firmware just
fails to probe).

Minor

2/6: efx_np_get_fixed_port_props() now returns 0 without writing
sup_cap_rawp, sup_cap_maskp or loopback_cap_maskp, which are still
__out_bcount_opt / __out_opt. Harmless today, but it is the same
pattern 07/14 of the annotation series changes to __inout. Either make
these __inout_opt or zero them on the VF path.

3/6: the call site comment still reads "Subscribe to link change
events." with no hint it is a no-op for VFs.

5/6: base returns ENOTSUP, net/sfc tests EOPNOTSUPP (sfc_port.c:322).
Equal on Linux and FreeBSD, so fine, just worth knowing.

No findings on 1/6 or 4/6. 4/6's fail label renumbering is correct
(encounter order, chain reaches fail1 from every entry), fcntl is
assigned before the new test, and the net/sfc defaults
(flow_ctrl_autoneg B_TRUE, KEEP_CRC off) trip neither denial.

Not built on my side.

  parent reply	other threads:[~2026-08-16  3:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 17:50 [PATCH 0/6] common/sfc_efx/base: add Medford4 VF support Ivan Malov
2026-08-11 17:50 ` [PATCH 1/6] common/sfc_efx/base: let Medford4 PF manage VFs Ivan Malov
2026-08-11 17:50 ` [PATCH 2/6] common/sfc_efx/base: indicate dummy netport properties on VF Ivan Malov
2026-08-11 17:50 ` [PATCH 3/6] common/sfc_efx/base: skip netport event subscriptions on VFs Ivan Malov
2026-08-11 17:50 ` [PATCH 4/6] common/sfc_efx/base: deny tuning FCS and flow control to VFs Ivan Malov
2026-08-11 17:50 ` [PATCH 5/6] common/sfc_efx/base: deny periodic MAC stats delivery " Ivan Malov
2026-08-11 17:50 ` [PATCH 6/6] doc: announce VF support of AMD Solarflare X45xx family NICs Ivan Malov
2026-08-11 21:05 ` [PATCH 0/6] common/sfc_efx/base: add Medford4 VF support Stephen Hemminger
2026-08-12 17:10 ` [PATCH v2 " Ivan Malov
2026-08-12 17:10   ` [PATCH v2 1/6] common/sfc_efx/base: let Medford4 PF manage VFs Ivan Malov
2026-08-12 17:10   ` [PATCH v2 2/6] common/sfc_efx/base: indicate dummy netport properties on VF Ivan Malov
2026-08-12 17:11   ` [PATCH v2 3/6] common/sfc_efx/base: skip netport event subscriptions on VFs Ivan Malov
2026-08-12 17:11   ` [PATCH v2 4/6] common/sfc_efx/base: deny tuning FCS and flow control to VFs Ivan Malov
2026-08-12 17:11   ` [PATCH v2 5/6] common/sfc_efx/base: deny periodic MAC stats delivery " Ivan Malov
2026-08-12 17:11   ` [PATCH v2 6/6] doc: announce VF support of AMD Solarflare X45xx family NICs Ivan Malov
2026-08-16  3:27   ` Stephen Hemminger [this message]
2026-08-16  6:20 ` [PATCH v4 0/6] common/sfc_efx/base: add Medford4 VF support Ivan Malov
2026-08-16  6:20   ` [PATCH v4 1/6] common/sfc_efx/base: let Medford4 PF manage VFs Ivan Malov
2026-08-16  6:20   ` [PATCH v4 2/6] common/sfc_efx/base: indicate dummy netport properties on VF Ivan Malov
2026-08-16  6:20   ` [PATCH v4 3/6] common/sfc_efx/base: skip netport event subscriptions on VFs Ivan Malov
2026-08-16  6:20   ` [PATCH v4 4/6] common/sfc_efx/base: deny tuning FCS and flow control to VFs Ivan Malov
2026-08-16  6:20   ` [PATCH v4 5/6] common/sfc_efx/base: deny periodic MAC stats delivery " Ivan Malov
2026-08-16  6:20   ` [PATCH v4 6/6] doc: announce VF support of AMD Solarflare X45xx family NICs Ivan Malov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260815202722.1abe35ed@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=Roman.Zhukov@arknetworks.am \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=andy.moreton@amd.com \
    --cc=dev@dpdk.org \
    --cc=ivan.malov@arknetworks.am \
    --cc=pieter.jansen-van-vuuren@amd.com \
    --cc=viacheslav.galaktionov@arknetworks.am \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox