diff mbox series

rcu: add deprecation notice about limit on defer queue element size

Message ID 1747957021-2581-1-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series rcu: add deprecation notice about limit on defer queue element size |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing warning Testing issues
ci/iol-abi-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/aws-unit-testing success Unit Testing PASS

Commit Message

Andre Muezerie May 22, 2025, 11:37 p.m. UTC 
The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim establish
no limit on the size of each element in the defer queue. With DPDK 25.11 a
hard limit will be set (``RTE_QSBR_ESIZE_MAX``). This will allow fixed
C arrays to be used in the functions' implementations, avoiding VLAs and
use of alloca().

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 doc/guides/rel_notes/deprecation.rst | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

David Marchand July 1, 2025, 7:56 a.m. UTC | #1 
On Fri, May 23, 2025 at 1:37 AM Andre Muezerie
<andremue@linux.microsoft.com> wrote:
>
> The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim establish
> no limit on the size of each element in the defer queue. With DPDK 25.11 a
> hard limit will be set (``RTE_QSBR_ESIZE_MAX``). This will allow fixed
> C arrays to be used in the functions' implementations, avoiding VLAs and
> use of alloca().
>
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>

Acked-by: David Marchand <david.marchand@redhat.com>
Thomas Monjalon July 1, 2025, 2:17 p.m. UTC | #2 
23/05/2025 01:37, Andre Muezerie:
> The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim establish
> no limit on the size of each element in the defer queue.

Very good, we need more unlimited API in DPDK.

> With DPDK 25.11 a hard limit will be set (``RTE_QSBR_ESIZE_MAX``).

I think it is a step in the wrong direction.
I prefer having no limit.

> This will allow fixed C arrays to be used in the functions' implementations,
> avoiding VLAs and use of alloca().

I don't understand this justification.
Why trying to remove the 2 alloca() in the lib RCU?
Andre Muezerie July 10, 2025, 2:37 p.m. UTC | #3 
On Tue, Jul 01, 2025 at 04:17:20PM +0200, Thomas Monjalon wrote:
> 23/05/2025 01:37, Andre Muezerie:
> > The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim establish
> > no limit on the size of each element in the defer queue.
> 
> Very good, we need more unlimited API in DPDK.
> 
> > With DPDK 25.11 a hard limit will be set (``RTE_QSBR_ESIZE_MAX``).
> 
> I think it is a step in the wrong direction.
> I prefer having no limit.
> 
> > This will allow fixed C arrays to be used in the functions' implementations,
> > avoiding VLAs and use of alloca().
> 
> I don't understand this justification.
> Why trying to remove the 2 alloca() in the lib RCU?
> 

Only because other developer expressed concerns that using alloca() allows
ill-intended callers to cause a stack overflow.
I personally also prefer to have no hardcoded limits.
Thomas Monjalon July 11, 2025, 12:38 p.m. UTC | #4 
10/07/2025 16:37, Andre Muezerie:
> On Tue, Jul 01, 2025 at 04:17:20PM +0200, Thomas Monjalon wrote:
> > 23/05/2025 01:37, Andre Muezerie:
> > > The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim establish
> > > no limit on the size of each element in the defer queue.
> > 
> > Very good, we need more unlimited API in DPDK.
> > 
> > > With DPDK 25.11 a hard limit will be set (``RTE_QSBR_ESIZE_MAX``).
> > 
> > I think it is a step in the wrong direction.
> > I prefer having no limit.
> > 
> > > This will allow fixed C arrays to be used in the functions' implementations,
> > > avoiding VLAs and use of alloca().
> > 
> > I don't understand this justification.
> > Why trying to remove the 2 alloca() in the lib RCU?
> > 
> 
> Only because other developer expressed concerns that using alloca() allows
> ill-intended callers to cause a stack overflow.
> I personally also prefer to have no hardcoded limits.

Yes I vote for keeping alloca().
Konstantin Ananyev July 14, 2025, 9:01 a.m. UTC | #5 
> 
> 10/07/2025 16:37, Andre Muezerie:
> > On Tue, Jul 01, 2025 at 04:17:20PM +0200, Thomas Monjalon wrote:
> > > 23/05/2025 01:37, Andre Muezerie:
> > > > The functions rte_rcu_qsbr_dq_create and rte_rcu_qsbr_dq_reclaim establish
> > > > no limit on the size of each element in the defer queue.
> > >
> > > Very good, we need more unlimited API in DPDK.
> > >
> > > > With DPDK 25.11 a hard limit will be set (``RTE_QSBR_ESIZE_MAX``).
> > >
> > > I think it is a step in the wrong direction.
> > > I prefer having no limit.
> > >
> > > > This will allow fixed C arrays to be used in the functions' implementations,
> > > > avoiding VLAs and use of alloca().
> > >
> > > I don't understand this justification.
> > > Why trying to remove the 2 alloca() in the lib RCU?
> > >
> >
> > Only because other developer expressed concerns that using alloca() allows
> > ill-intended callers to cause a stack overflow.
> > I personally also prefer to have no hardcoded limits.
> 
> Yes I vote for keeping alloca().
> 

Probably it was me who expressed some concerns, sorry for late reply.
I can only repeat what I already replied to David:
 
For that particular case, my reasons are mostly conceptual:
using alloca() doesn't really differ from simply using VLA,
in fact it makes code looks uglier.
I understand that we do want MSVC enabled, and in many cases such mechanical
replacement is ok, but probably better to avoid  it whenever possible. 

 suppose we have 3 options:
1) use predefined max value (it could be quite big to fit any reasonable usage, let say 1KB or so).
2) use alloca().
3) come-up with some smarter approach.

For 3) - I don't have any good ideas.
One option would be to create that ring RING_F_MP_HTS_ENQ  flags,
then we can use peek API  for enqueue part too (rte_ring_enqueue_bulk_elem_start).
That would solve an issue, as in that case we wouldn't need to make temp copy of data on the stack.
My preference would be either 1) or 3), but I could leave with 2) too - specially that I don't really use that part of RCU lib.
Would be really good to hear opinion of RCU lib maintainer.

Konstantin
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 36489f6e68..78b2a27b52 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -17,6 +17,12 @@  Other API and ABI deprecation notices are to be posted below.
 Deprecation Notices
 -------------------
 
+* rcu: The functions ``rte_rcu_qsbr_dq_create`` and ``rte_rcu_qsbr_dq_reclaim``
+  establish no limit on the size of each element in the defer queue. With
+  DPDK 25.11 a hard limit will be set (``RTE_QSBR_ESIZE_MAX``). This will
+  allow fixed C arrays to be used in the functions' implementations,
+  avoiding VLAs and use of alloca().
+
 * build: The ``enable_kmods`` option is deprecated and will be removed in a future release.
   Setting/clearing the option has no impact on the build.
   Instead, kernel modules will be always built for OS's where out-of-tree kernel modules