Re: [PATCH v2 0/6] common/sfc_efx/base: add Medford4 VF support

On Wed, 12 Aug 2026 21:10:57 +0400
Ivan Malov <[email protected]> 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.