* Re: [PATCH 00/13] ENA release 2.15.0
2026-10-13 15:06 [PATCH 00/13] ENA release 2.15.0 Shai Brandes
@ 2026-10-13 15:33 ` Stephen Hemminger
0 siblings, 0 replies; 2+ messages in thread
From: Stephen Hemminger @ 2026-10-13 15:33 UTC (permalink / raw)
To: Shai Brandes; +Cc: dev
On Tue, 13 Oct 2026 18:06:11 +0300
Shai Brandes <shaibran@amazon.com> wrote:
> Hello all,
> This series upgrades HAL and fixes two PHC correctness issues,
> an LLQ feature version not communicated to the device, and
> a failed LLQ configuration zeroing the negotiated tx_max_header_size.
> Additionally it adds n-tuple Rx flow steering support through the rte_flow API.
> Review and comments are welcome
>
> Best regards,
> Shai Brandes
Running AI review spots several issues:
Review of [PATCH 00/13] net/ena: 2.15.0 (bundle 2177)
Applied to main at 7f615af; every commit built with -Dwerror=true
(gcc, x86-64).
Series level
------------
Error: patch 04 breaks the build (see below).
Warning: patch 11 does not apply to main. doc/guides/nics/features/
ena.ini now ends with "Usage doc" and "Design doc"; rebase.
Info: the stable fixes (02, 03, 06, 10) sit behind refactors, and
06 and 10 depend on context from 01, 05 and 09. Put the fixes first
so they backport without rework.
[PATCH 01/13] net/ena: avoid using un-needed admin response buffers
-------------------------------------------------------------------
Warning: undocumented behaviour change. Besides the move, the
missing-interrupt fallback in
ena_com_wait_and_process_admin_cq_interrupts() changes from
admin_queue->polling = true;
to
ena_com_set_admin_polling_mode(admin_queue->ena_dev, true);
which also writes ENA_REGS_ADMIN_INTR_MASK to the device. Probably
the right thing, but the commit message only talks about response
buffers. Say so in the commit message, or split the wait path
rework into its own patch.
[PATCH 03/13] net/ena/base: fix missing barriers in PHC timestamp read
---------------------------------------------------------------------
Warning: wrong Fixes tag. The req_id poll loop without a barrier
comes from f73f53f7dc7a (v23.11); a14be4dd8693 (v25.11) only added
the statistics block. As tagged, 23.11 and 24.11 will not pick this
up. Use:
Fixes: f73f53f7dc7a ("net/ena: upgrade HAL")
[PATCH 04/13] net/ena/base: rename bounce buffer references
----------------------------------------------------------
Error: build break. The member is renamed in ena_com.h but
ena_eth_com.c still uses the old name until patch 05:
ena_eth_com.c:160: error: 'struct ena_com_llq_pkt_ctrl' has no
member named 'curr_bounce_buf'
Move the ena_eth_com.c renames (curr_bounce_buf, plus
write_header_to_bounce and close_bounce_buffer, which 05 also
renames without mentioning it) into this patch. 05 is then only
the ena_com_sq_is_llq() helper, matching its subject.
[PATCH 08/13] net/ena/base: deduplicate meta caching config flag
---------------------------------------------------------------
Info: removing the field leaves two consecutive blank lines in
struct ena_com_io_sq after mem_queue_type. Drop one.
[PATCH 09/13] net/ena/base: split LLQ info into datapath and config
------------------------------------------------------------------
Warning: this is not "No functional change". The burst limit
divisor changes:
- llq_default_cfg->llq_ring_entry_size_value;
+ llq_info->common.desc_list_entry_size;
They differ when the device does not support the default entry
size and the fallback above picks another one. With a 128 byte
default and a 256 byte fallback, the old code allows twice as many
entries per burst as the device accepts. That is a real bug, present
since the LIMIT_TX_BURST support went in. Split it out:
net/ena/base: fix Tx burst limit with fallback entry size
Fixes: b68309be44c0 ("net/ena/base: update communication layer
for the ENAv2")
Cc: stable@dpdk.org
and keep 09 a pure refactor.
Info: continuation lines of the descs_num_before_header assignments
are not indented:
llq_info->common.descs_num_before_header =
llq_default_cfg->llq_num_decs_before_header;
Indent the second line one more tab, or keep it on one line.
[PATCH 11/13] net/ena: support flow API for n-tuple Rx steering
--------------------------------------------------------------
Warning: an item without spec is rejected for IPV4, IPV6, TCP and
UDP ("IPv4 item requires spec"). In rte_flow a NULL spec means
"match any packet of this protocol". testpmd passes spec == NULL
for items given without fields, so the obvious rule
flow create 0 ingress pattern eth / ipv4 / tcp dst is 80 / end
actions queue index 1 / end
fails. Treat NULL spec as protocol presence with zero masks, e.g.:
case RTE_FLOW_ITEM_TYPE_IPV4:
...
has_ipv4 = true;
if (spec == NULL)
break;
if (mask == NULL)
mask = &rte_flow_item_ipv4_mask;
Same for IPV6, TCP and UDP.
Warning: secondary process. All other admin commands (MTU, RSS,
stats) go through ENA_PROXY because only the primary owns the admin
queue. The flow ops call ena_com_flow_steering_*() directly, so
rte_flow from a secondary submits admin commands from the wrong
process. Proxy them, or simply refuse in ena_flow_ops_get():
if (rte_eal_process_type() != RTE_PROC_PRIMARY)
return -E_RTE_SECONDARY;
Warning: features/ena.ini adds the [rte_flow items] and
[rte_flow actions] tables but not "Flow API = Y" under [Features].
Info:
- ena_infos_get(): the two "&= ~" lines are no-ops (ENA never sets
RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP or RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE)
and dev_infos_get should not write dev->data. Drop them and the
comments.
- ena_flow_create() parses the pattern twice, once inside
ena_flow_validate(). Split the attr/started checks into a helper
and call ena_flow_parse() once.
- adapter->flow_rule_count duplicates
ena_dev->flow_steering.active_rules_cnt. Use the base counter.
- ena_flow_ops_get() logs WARNING when steering is unsupported.
testpmd calls rte_flow_flush() on port close, so this fires on
every close. Use DEBUG or no log.
- ena_flow_query() only returns ENOTSUP; leave .query NULL and
ethdev returns ENOSYS.
- ena_com_flow_steering_remove_rule() and _remove_all_rules() pass
a response buffer they never read. Use
ena_com_execute_admin_command_no_resp() from patch 01.
- ena_com_flow_steering_destroy() leaves dangling pointers. End it
with memset(flow_steering, 0, sizeof(*flow_steering)) like
ena_com_rss_destroy().
[PATCH 12/13] net/ena/base: remove x86 Tx LLQ write barrier
----------------------------------------------------------
Warning: arm_wmb() is a no-op on every non-ARM architecture, not
just x86. ppc64le, riscv and loongarch are weakly ordered and the
driver still builds there. The argument for dropping the barrier
(UC doorbell store flushes WC buffers) is x86 specific, so make the
test x86 specific:
#if defined(RTE_ARCH_X86)
#define ena_llq_wmb() do { } while (0)
#else
#define ena_llq_wmb() rte_wmb()
#endif
(rte_io_wmb() everywhere is the other option, if Graviton testing
shows dmb oshst is enough; on x86 it is a compiler barrier.)
Info: the new comment in ena_com_write_bounce_buffer_to_dev() is
20 lines, half of it history of the old wmb(). History belongs in
the commit message; keep the comment to what the barrier orders and
why x86 does not need it.
^ permalink raw reply [flat|nested] 2+ messages in thread