On Sun, Jul 26, 2026 at 6:37 PM Stephen Hemminger
<[email protected]> wrote:
>
> On Thu, 23 Jul 2026 11:35:40 +0200
> Maxime Leroy <[email protected]> wrote:
>
> > The RSS key only extracted the outer IP header. Tunnelled traffic whose
> > outer headers are fixed then carries no entropy for the hash, so every
> > flow lands on a single Rx queue.
> >
> > Extract both the outer IP (header index 0) and the innermost IP
> > instance (HDR_INDEX_LAST). Plain frames keep being hashed on their only
> > IP header; the inner extract resolves to nothing and adds no entropy.
> > Tunnelled frames are also hashed on their inner IP and spread across the
> > Rx queues.
> >
> > This is the PMD default hash: dpaa2 does not expose the ethdev RSS level
> > selector, so it applies to every RSS request. The hardware cannot hash
> > the inner IP alone, as HDR_INDEX_LAST only resolves when several IP
> > headers are stacked and a plain frame would hash to a constant. Folding
> > the outer IP into the key is therefore unavoidable, and two tunnelled
> > flows that share an inner IP but differ in their outer IP may hash to
> > different queues. This is documented in the dpaa2 guide.
> >
> > Signed-off-by: Maxime Leroy <[email protected]>
> > ---
>
> More detailed AI review
>
> Warning: RSS key size likely exceeds the DPNI key limit
>
> drivers/net/dpaa2/base/dpaa2_hw_dpni.c
>
> Doubling the IP extracts doubles the generated key, not just the
> extract count.  The driver's own accounting treats a NET_PROT_IP
> full-field address extract as 16 bytes (NH_FLD_IPV6_ADDR_SIZE, see
> dpaa2_flow_add_ipaddr_extract_rule() in dpaa2_flow.c), so:
>
>     before:  16 + 16 + 1                = 33 bytes  (+4 with L4 = 37)
>     after:   2 * (16 + 16 + 1)          = 66 bytes  (+4 with L4 = 70)
>
> DPNI_MAX_KEY_SIZE is 56, and dpni_attr.fs_key_size is documented as
> "Size, in bytes, of the flow steering look-up key.  Defining a key
> larger than this when composing the hash + FS key will result in an
> error."  The PMD never reads fs_key_size and there is no size check
> anywhere on this path, so the first sign of trouble would be
> dpni_set_rx_hash_dist() failing and rte_eth_dev_configure() returning
> an error for a plain RTE_ETH_RSS_IP request that works today.
>
> Has this been tested on hardware with RTE_ETH_RSS_IP (and IP|TCP|UDP)?

It has, on LX2160A, and with exactly those two requests. testpmd --rss-ip
sets rss_hf to RTE_ETH_RSS_IP and testpmd --rss-udp sets it to
RTE_ETH_RSS_IP | RTE_ETH_RSS_UDP, which compose six and eight extracts,
that is your 66 bytes and 70 bytes.

Our forwarding application goes further, requesting

 rss_hf = RTE_ETH_RSS_VLAN | RTE_ETH_RSS_IP | RTE_ETH_RSS_UDP |
          RTE_ETH_RSS_TCP

which composes nine extracts: one VLAN TCI, six IP (source, destination
and protocol, at index 0 and at HDR_INDEX_LAST) and two L4 ports, so 72
bytes with the 16 bytes per address you count, against 39 bytes before the
patch.

In all three cases dpni_set_rx_hash_dist() accepts the key and
rte_eth_dev_configure() succeeds.

Distribution was then checked per traffic type: plain IPv4, plain IPv6,
VLAN tagged frames and IPv6 tunnelled traffic all spread across the Rx
queues, the last one on its inner IP, which is the point of the patch.

So 56 does not bound this path. DPNI_MAX_KEY_SIZE and fs_key_size bound
the flow steering look-up key, that is the key which is stored in, and
compared against, the FS and QoS table entries. Its size is explicit in
the API (dpni_rule_cfg carries key_iova, mask_iova and key_size), and it
is the size dpaa2_flow.c accounts and checks: that is the path your 16
bytes figure comes from.

The hash distribution path installs no table entry and stores no key.
dpni_rx_dist_cfg has no key_size field at all, only dist_size and
key_cfg_iova, i.e. the key composition; the composed bytes are only fed
to the hash function which yields the queue index. The two budgets are
not the same.

For the extract count, the flib bound is DPKG_MAX_NUM_OF_EXTRACTS = 20,
and it is the only limit the driver checks, in dpkg_prepare_key_cfg().
The worst case, requesting every type the PMD advertises (ETH, PPPOE,
ESP, AH, VLAN, MPLS, IP and L4), is 16 extracts with this patch. It is 13
without it, and it was 15 in the code that shipped until the June cleanup
folded the SCTP ports into the L4 case (4b200f10de27).

> If MC accepts it, please say so in the commit message, because the
> arithmetic says it should not.  If the limit is real, the extract set
> needs to shrink -- dropping NH_FLD_IP_PROTO, or extracting only the
> inner address pair, would be candidates.
>
> Warning: the "inner extract resolves to nothing" claim needs backing
>
> Both the commit message and the new comment assert that on a plain
> frame the HDR_INDEX_LAST extract resolves to nothing and contributes no
> entropy.  The MC header documents hdr_index as "used for protocols that
> may have more than a single header, 0 indicates an outer header" with
> NET_PROT_IP taking (0, HDR_INDEX_LAST).  The natural reading is that
> for a single-IP frame the last header *is* the outer one, so index 0
> and HDR_INDEX_LAST select the same header and the fields are extracted
> twice.

That reading is excluded by measurement on LX2160A, with testpmd.

It is in fact the regression patch 1 of this series reverts. The reverted
commit programmed the IP extracts *only* at HDR_INDEX_LAST. Had
HDR_INDEX_LAST selected the outer header of a frame carrying a single IP
header, plain traffic would have kept hashing on that header and there
would have been nothing to fix. What we measured instead was every plain
frame getting the same hash and landing on one Rx queue, while tunnelled
traffic spread correctly on its inner IP. Patch 1 states this and carries
Hemant's ack.

Reading the driver leads to the same conclusion, though I can only offer
it as a reading. fsl_dpkg.h defines hdr_index for VLAN, MPLS and IP only,
and for those three the Rx annotation exposes one flag set per header
instance: L3_IPV4_1_PRESENT against L3_IPV4_N_PRESENT and so on in
base/dpaa2_hw_dpni_annot.h, the matching offset accessor being named
IP_N_OR_MIN_ENCAP_OFFSET(). Index 0 would then select the "1" instance
and HDR_INDEX_LAST the "N" one, and a frame with a single IP header has no
"N" instance for the extract to resolve to. Whatever the key generator
places in the key for such an extract, it is the same for every plain
frame, which is what the measurement shows: no entropy, and no second
extraction of the outer header.

> That is harmless for distribution but it is not what the patch
> says, and it changes the key size and hash values for every existing
> non-tunnelled user.
>
> Please confirm the actual behaviour with NXP and describe it
> accurately, one way or the other.  This also feeds directly into the
> key-size question above.
>
> Info: duplicate constant
>
> +#define DPAA2_DIST_HDR_INDEX_LAST 0xff
>
> mc/fsl_net.h already carries LAST_HDR_INDEX (0xFFFFFFFF), truncated to
> 0xff by the uint8_t hdr_index field.  Two differently-named constants
> for the same hardware encoding in one driver is confusing.  Either
> reuse the existing one with an explicit cast, or add a comment saying
> this is LAST_HDR_INDEX narrowed to the 8-bit command field.
>
> Info: placement
>
> The #define sits between two function definitions.  Move it up with the
> other file-scope definitions, or into dpaa2_ethdev.h next to the
> related driver constants.

Agreed on both. v3 derives the constant from LAST_HDR_INDEX, cast to the
8-bit field it feeds, and moves it up with the other file scope
definitions:

 /* LAST_HDR_INDEX narrowed to the 8-bit dpkg hdr_index field */
 #define DPAA2_DIST_HDR_INDEX_LAST ((uint8_t)LAST_HDR_INDEX)

-------------------------------
Maxime Leroy
[email protected]

Reply via email to