Re: [PATCH v1 1/1] ethdev: add flow item structure size query

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 23 Oct 2026 13:15:46 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> Add RTE_FLOW_CONV_OP_ITEM_SIZE to retrieve the fixed specification
> structure size for a flow item type using the existing descriptor table.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

I am not sure about the details AI is complaining about here,
but it does look like it has a point. Also it should be possible
to get some code coverage for this in a standalone test.

[PATCH v1 1/1] ethdev: add flow item structure size query

Warning

RTE_FLOW_ITEM_TYPE_MACSEC has no entry in rte_flow_desc_item[]. The
table uses designated initializers, so the MACSEC slot is zero filled.
With this patch a size query for MACSEC fails with EINVAL "unknown
item type", although it is a public item with a spec structure
(struct rte_flow_item_macsec). The doc comment says only unknown and
PMD-private types fail.

The hole predates this patch and hurts existing ops too:
 - RTE_FLOW_CONV_OP_ITEM_NAME on MACSEC calls strlcpy(dst, NULL, size);
   rte_flow_conv_name() has no NULL name check.
 - rte_flow_conv_copy() sees desc[type].size == 0, so rte_flow_copy()
   and RTE_FLOW_CONV_OP_PATTERN silently drop MACSEC spec/last/mask.

Please send a separate fix ahead of this patch, in rte_flow_desc_item[]:

	MK_FLOW_ITEM(MACSEC, sizeof(struct rte_flow_item_macsec)),

	Fixes: 0aaf097585a6 ("ethdev: add MACsec flow item")
	Cc: stable@dpdk.org

Missing release note. RTE_FLOW_CONV_OP_ITEM_DEFAULT_MASK_PTR, also new
in 26.11, has one. Extend that entry in release_26_11.rst:

	* **Added flow item default mask and size lookup.**

	  Added ``RTE_FLOW_CONV_OP_ITEM_DEFAULT_MASK_PTR`` and
	  ``RTE_FLOW_CONV_OP_ITEM_SIZE`` conversion operations
	  to ``rte_flow_conv()`` to retrieve the default mask
	  and the spec structure size of a flow item type.

No user and no test. The commit message does not say what needs this.
If the consumer is flow_graph or a driver parser, send it as part of
that series. app/test has no rte_flow_conv() coverage at all; a short
test checking END -> 0, ETH -> sizeof(struct rte_flow_item_eth),
MACSEC -> sizeof(struct rte_flow_item_macsec), a negative type ->
EINVAL, and size < sizeof(size_t) -> dst untouched would have caught
the MACSEC hole.

Info

The doc text "Writes the size of rte_flow_item_{name}" is wrong for
GRE_KEY (rte_be32_t), GTPC/GTPU (struct rte_flow_item_gtp) and
ICMP6_ECHO_REQUEST/REPLY (struct rte_flow_item_icmp6_echo). Suggest:

	 * Writes the size of the item specification structure to
	 * @p dst, or zero for known item types without one.

Every other op dispatches to a static helper. Move the braced case
block next to rte_flow_conv_item_default_mask():

	/** Retrieve the spec structure size of a flow item type. */
	static int
	rte_flow_conv_item_size(void *dst, const size_t size,
				const void *src,
				struct rte_flow_error *error)
	{
		unsigned int type = (uintptr_t)src;

		if (type >= RTE_DIM(rte_flow_desc_item) ||
		    rte_flow_desc_item[type].name == NULL)
			return rte_flow_error_set(error, EINVAL,
				RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
				"unknown item type to retrieve the size of");
		if (size >= sizeof(size_t))
			*(size_t *)dst = rte_flow_desc_item[type].size;
		return sizeof(size_t);
	}

	case RTE_FLOW_CONV_OP_ITEM_SIZE:
		ret = rte_flow_conv_item_size(dst, size, src, error);
		break;

The name == NULL check here is good; the same check belongs in
rte_flow_conv_name() and rte_flow_conv_item_default_mask().