Re: [PATCH 2/2] net/sfc: provide cached dev info to use in secondary process

On Thu, 20 Aug 2026 17:03:14 +0400
Ivan Malov <[email protected]> wrote:

> Secondary process support in the 'test-pmd' application now requires that
> the driver expose the 'dev_infos_get' method within that context. Use the
> cached dev info from the primary process in order to meet the requirement.
> 
> Signed-off-by: Ivan Malov <[email protected]>
> Reviewed-by: Viacheslav Galaktionov <[email protected]>
> ---

This patch has issues.



Patch 2/2: net/sfc: provide cached dev info to use in secondary process

Error: the cached snapshot is missing the defaults that
rte_eth_dev_info_get() fills in before it calls the driver callback,
so the secondary process reports zero for several fields.

rte_eth_dev_info_get() pre-populates the struct and then calls
.dev_infos_get(), so a PMD callback only has to set the fields it
actually knows about.  Besides switch_info.domain_id and device
(both handled by this patch) it pre-sets:

  rx_desc_lim.nb_seg_max      = UINT16_MAX
  rx_desc_lim.nb_mtu_seg_max  = UINT16_MAX
  tx_desc_lim.nb_seg_max      = UINT16_MAX
  tx_desc_lim.nb_mtu_seg_max  = UINT16_MAX
  rss_algo_capa               = RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT)
  max_rx_bufsize              = UINT32_MAX

sfc_dev_infos_get() never writes any of these, and neither do the
datapath get_dev_info() helpers (sfc_ef100_rx_get_dev_info() and
sfc_ef100_get_dev_info() only touch nb_min and nb_align).  In the
primary process that is fine because the ethdev layer supplied the
values.  Here the cache is filled by calling sfc_dev_infos_get()
directly on a zeroed structure, so those fields stay zero, and
sfc_dev_infos_get_secondary() then overwrites the ethdev pre-fill
wholesale with

	*dev_info = sfc_adapter_shared_by_eth_dev(dev)->dev_info_cache;

A secondary process therefore sees nb_seg_max = 0,
nb_mtu_seg_max = 0, max_rx_bufsize = 0 and rss_algo_capa = 0, which
differs from what the same call returns in the primary.  Applications
that validate multi-segment Tx against tx_desc_lim.nb_seg_max, or
that check the RSS hash algorithm capability mask, will get wrong
answers.

Suggested fix: seed the cache with the same defaults before the
snapshot is taken, e.g.

	static const struct rte_eth_desc_lim lim = {
		.nb_max = UINT16_MAX,
		.nb_min = 0,
		.nb_align = 1,
		.nb_seg_max = UINT16_MAX,
		.nb_mtu_seg_max = UINT16_MAX,
	};

	sas->dev_info_cache.rx_desc_lim = lim;
	sas->dev_info_cache.tx_desc_lim = lim;
	sas->dev_info_cache.max_rx_bufsize = UINT32_MAX;
	sas->dev_info_cache.rss_algo_capa =
		RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT);
	sas->dev_info_cache.switch_info.domain_id =
		RTE_ETH_DEV_SWITCH_DOMAIN_ID_INVALID;
	(void)sfc_dev_infos_get(dev, &sas->dev_info_cache);

This duplicates ethdev knowledge in the driver and will drift when
new pre-filled fields are added.  An alternative that avoids the
duplication is to have sfc_dev_infos_get_secondary() copy only the
fields the PMD owns, or to keep the caller's pre-filled struct and
merge the cached values into it.

Info: switch_info.name is left pointing at the primary process copy
of dev->device->driver->name.  The comment in
sfc_dev_infos_get_secondary() only mentions the device pointer, but
this is the same class of problem; the string lives in the driver
image rather than in per-process heap, so it happens to work under
the usual multi-process assumptions, but it would be more consistent
to re-derive it next to the device pointer:

	if (dev_info->switch_info.name != NULL)
		dev_info->switch_info.name = dev->device->driver->name;

Info: the cache is a snapshot taken at the end of sfc_eth_dev_init().
Everything sfc_dev_infos_get() reports is fixed at attach time today
(NIC config, rxq_max/txq_max, offload capabilities, MAE status), so
the snapshot is accurate.  Worth a note in the sfc.h comment that any
future dev_info field derived from post-attach state must not be
served from this cache.