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,
	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 2/2] net/sfc: provide cached dev info to use in secondary process
Date: Fri, 21 Aug 2026 12:10:18 -0700	[thread overview]
Message-ID: <20260821121018.6c8ba96f@phoenix.local> (raw)
In-Reply-To: <20260820130314.12251-3-ivan.malov@arknetworks.am>

On Thu, 20 Aug 2026 17:03:14 +0400
Ivan Malov <ivan.malov@arknetworks.am> 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 <ivan.malov@arknetworks.am>
> Reviewed-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> ---

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.

  reply	other threads:[~2026-08-21 19:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20 13:03 [PATCH 0/2] net/sfc: fix and extend secondary process support Ivan Malov
2026-08-20 13:03 ` [PATCH 1/2] net/sfc: fix shared adapter pointer set in secondary process Ivan Malov
2026-08-21 19:09   ` Stephen Hemminger
2026-08-20 13:03 ` [PATCH 2/2] net/sfc: provide cached dev info to use " Ivan Malov
2026-08-21 19:10   ` Stephen Hemminger [this message]
2026-08-21 23:45 ` [PATCH v2 0/2] net/sfc: fix and extend secondary process support Ivan Malov
2026-08-21 23:45   ` [PATCH v2 1/2] net/sfc: fix shared adapter pointer set in secondary process Ivan Malov
2026-08-21 23:45   ` [PATCH v2 2/2] net/sfc: provide cached dev info to use " Ivan Malov
2026-08-22  0:06 ` [PATCH v3 0/2] net/sfc: fix and extend secondary process support Ivan Malov
2026-08-22  0:06   ` [PATCH v3 1/2] net/sfc: fix shared adapter pointer set in secondary process Ivan Malov
2026-08-22  0:06   ` [PATCH v3 2/2] net/sfc: provide cached dev info to use " 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=20260821121018.6c8ba96f@phoenix.local \
    --to=stephen@networkplumber.org \
    --cc=Roman.Zhukov@arknetworks.am \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --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