* [PATCH v2 1/4] bitops: fix build for GCC without experimental API
2024-10-16 11:38 ` [PATCH v2 0/4] Enhance headers check David Marchand
@ 2024-10-16 11:38 ` David Marchand
2024-10-16 11:38 ` [PATCH v2 2/4] bitset: " David Marchand
` (3 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2024-10-16 11:38 UTC (permalink / raw)
To: dev
Cc: thomas, bruce.richardson, ktraynor, Morten Brørup,
Jack Bond-Preston, Tyler Retzlaff, Mattias Rönnblom
Building OVS against current DPDK fails with following warnings:
In file included from .../ovs/dpdk-dir/include/rte_memory.h:18,
from .../ovs/dpdk-dir/include/rte_ring_core.h:29,
from .../ovs/dpdk-dir/include/rte_ring.h:37,
from .../ovs/dpdk-dir/include/rte_mempool.h:49,
from .../ovs/dpdk-dir/include/rte_mbuf.h:38,
from lib/dp-packet.h:25,
from lib/ofp-packet.c:20:
.../ovs/dpdk-dir/include/rte_bitops.h: In function ‘__rte_bit_assign32’:
.../ovs/dpdk-dir/include/rte_bitops.h:528:1: error:
‘__rte_bit_set32’ is deprecated: Symbol is not yet part of
stable ABI [-Werror=deprecated-declarations]
...
This comes from the fact that some (experimental) inline helpers
are calling other experimental API.
Hide those calls.
Fixes: 471de107ae23 ("bitops: add new bit manipulation API")
Reported-by: Kevin Traynor <ktraynor@redhat.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
lib/eal/include/rte_bitops.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/eal/include/rte_bitops.h b/lib/eal/include/rte_bitops.h
index e08e41199a..deb1fd43f2 100644
--- a/lib/eal/include/rte_bitops.h
+++ b/lib/eal/include/rte_bitops.h
@@ -525,8 +525,10 @@ __rte_bit_ ## variant ## flip ## size(qualifier uint ## size ## _t *addr, unsign
__RTE_GEN_BIT_OPS(,, size) \
__RTE_GEN_BIT_OPS(v_, volatile, size)
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_GEN_BIT_OPS_SIZE(32)
__RTE_GEN_BIT_OPS_SIZE(64)
+#endif
#define __RTE_GEN_BIT_ATOMIC_TEST(variant, qualifier, size) \
__rte_experimental \
@@ -651,8 +653,10 @@ __rte_bit_atomic_ ## variant ## test_and_assign ## size( \
__RTE_GEN_BIT_ATOMIC_OPS(,, size) \
__RTE_GEN_BIT_ATOMIC_OPS(v_, volatile, size)
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_GEN_BIT_ATOMIC_OPS_SIZE(32)
__RTE_GEN_BIT_ATOMIC_OPS_SIZE(64)
+#endif
/*------------------------ 32-bit relaxed operations ------------------------*/
@@ -1481,6 +1485,7 @@ rte_bit_ ## family ## fun(qualifier uint ## size ## _t *addr, arg1_type arg1_nam
__RTE_BIT_OVERLOAD_SZ_4R(family, fun, qualifier, 64, ret_type, arg1_type, arg1_name, \
arg2_type, arg2_name, arg3_type, arg3_name)
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BIT_OVERLOAD_2R(, test, const, bool, unsigned int, nr)
__RTE_BIT_OVERLOAD_2(, set,, unsigned int, nr)
__RTE_BIT_OVERLOAD_2(, clear,, unsigned int, nr)
@@ -1496,6 +1501,7 @@ __RTE_BIT_OVERLOAD_3R(atomic_, test_and_set,, bool, unsigned int, nr, int, memor
__RTE_BIT_OVERLOAD_3R(atomic_, test_and_clear,, bool, unsigned int, nr, int, memory_order)
__RTE_BIT_OVERLOAD_4R(atomic_, test_and_assign,, bool, unsigned int, nr, bool, value,
int, memory_order)
+#endif
#endif
--
2.46.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 11:38 ` [PATCH v2 0/4] Enhance headers check David Marchand
2024-10-16 11:38 ` [PATCH v2 1/4] bitops: fix build for GCC without experimental API David Marchand
@ 2024-10-16 11:38 ` David Marchand
2024-10-16 14:14 ` Mattias Rönnblom
2024-10-16 11:38 ` [PATCH v2 3/4] vhost: remove internal vDPA API description from public header David Marchand
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2024-10-16 11:38 UTC (permalink / raw)
To: dev
Cc: thomas, bruce.richardson, ktraynor, Morten Brørup,
Mattias Rönnblom, Tyler Retzlaff
For a reason similar to the change on bitops header, hide bitset
implementation relying on experimental API.
Fixes: 99a1197647d8 ("eal: add bitset type")
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
lib/eal/include/rte_bitset.h | 123 +++++++++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/lib/eal/include/rte_bitset.h b/lib/eal/include/rte_bitset.h
index 74c643a72a..8ae8425fc2 100644
--- a/lib/eal/include/rte_bitset.h
+++ b/lib/eal/include/rte_bitset.h
@@ -255,7 +255,13 @@ __rte_experimental
static inline bool
rte_bitset_test(const uint64_t *bitset, size_t bit_num)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ return false;
+#endif
}
/**
@@ -277,7 +283,12 @@ __rte_experimental
static inline void
rte_bitset_set(uint64_t *bitset, size_t bit_num)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE(rte_bit_set, bitset, bit_num);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+#endif
}
/**
@@ -299,7 +310,12 @@ __rte_experimental
static inline void
rte_bitset_clear(uint64_t *bitset, size_t bit_num)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE(rte_bit_clear, bitset, bit_num);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+#endif
}
/**
@@ -323,7 +339,13 @@ __rte_experimental
static inline void
rte_bitset_assign(uint64_t *bitset, size_t bit_num, bool bit_value)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE_N(rte_bit_assign, bitset, bit_num, bit_value);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ RTE_SET_USED(bit_value);
+#endif
}
/**
@@ -345,7 +367,12 @@ __rte_experimental
static inline void
rte_bitset_flip(uint64_t *bitset, size_t bit_num)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE(rte_bit_flip, bitset, bit_num);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+#endif
}
/**
@@ -370,7 +397,14 @@ __rte_experimental
static inline bool
rte_bitset_atomic_test(const uint64_t *bitset, size_t bit_num, int memory_order)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __RTE_BITSET_DELEGATE_N(rte_bit_atomic_test, bitset, bit_num, memory_order);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ RTE_SET_USED(memory_order);
+ return false;
+#endif
}
/**
@@ -399,7 +433,13 @@ __rte_experimental
static inline void
rte_bitset_atomic_set(uint64_t *bitset, size_t bit_num, int memory_order)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE_N(rte_bit_atomic_set, bitset, bit_num, memory_order);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ RTE_SET_USED(memory_order);
+#endif
}
/**
@@ -428,7 +468,13 @@ __rte_experimental
static inline void
rte_bitset_atomic_clear(uint64_t *bitset, size_t bit_num, int memory_order)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE_N(rte_bit_atomic_clear, bitset, bit_num, memory_order);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ RTE_SET_USED(memory_order);
+#endif
}
/**
@@ -459,7 +505,14 @@ __rte_experimental
static inline void
rte_bitset_atomic_assign(uint64_t *bitset, size_t bit_num, bool bit_value, int memory_order)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE_N(rte_bit_atomic_assign, bitset, bit_num, bit_value, memory_order);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ RTE_SET_USED(bit_value);
+ RTE_SET_USED(memory_order);
+#endif
}
/**
@@ -488,7 +541,13 @@ __rte_experimental
static inline void
rte_bitset_atomic_flip(uint64_t *bitset, size_t bit_num, int memory_order)
{
+#ifdef ALLOW_EXPERIMENTAL_API
__RTE_BITSET_DELEGATE_N(rte_bit_atomic_flip, bitset, bit_num, memory_order);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(bit_num);
+ RTE_SET_USED(memory_order);
+#endif
}
/**
@@ -524,7 +583,12 @@ __rte_experimental
static inline void
rte_bitset_clear_all(uint64_t *bitset, size_t size)
{
+#ifdef ALLOW_EXPERIMENTAL_API
rte_bitset_init(bitset, size);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+#endif
}
/**
@@ -576,7 +640,13 @@ __rte_experimental
static inline size_t
rte_bitset_count_clear(const uint64_t *bitset, size_t size)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return size - rte_bitset_count_set(bitset, size);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ return 0;
+#endif
}
#define __RTE_BITSET_FIND_FLAG_FIND_CLEAR (1U << 0)
@@ -638,6 +708,7 @@ static inline ssize_t
__rte_bitset_find(const uint64_t *bitset, size_t size, size_t start_bit, size_t len,
unsigned int flags)
{
+#ifdef ALLOW_EXPERIMENTAL_API
bool find_clear = flags & __RTE_BITSET_FIND_FLAG_FIND_CLEAR;
bool may_wrap = flags & __RTE_BITSET_FIND_FLAG_WRAP;
bool does_wrap = (start_bit + len) > size;
@@ -658,6 +729,14 @@ __rte_bitset_find(const uint64_t *bitset, size_t size, size_t start_bit, size_t
rc = __rte_bitset_find_nowrap(bitset, size, start_bit, len, find_clear);
return rc;
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ RTE_SET_USED(start_bit);
+ RTE_SET_USED(len);
+ RTE_SET_USED(flags);
+ return 0;
+#endif
}
/**
@@ -681,7 +760,13 @@ __rte_experimental
static inline ssize_t
rte_bitset_find_first_set(const uint64_t *bitset, size_t size)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __rte_bitset_find(bitset, size, 0, size, 0);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ return 0;
+#endif
}
/**
@@ -711,7 +796,15 @@ __rte_experimental
static inline ssize_t
rte_bitset_find_set(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __rte_bitset_find(bitset, size, start_bit, len, 0);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ RTE_SET_USED(start_bit);
+ RTE_SET_USED(len);
+ return 0;
+#endif
}
/**
@@ -742,7 +835,15 @@ __rte_experimental
static inline ssize_t
rte_bitset_find_set_wrap(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __rte_bitset_find(bitset, size, start_bit, len, __RTE_BITSET_FIND_FLAG_WRAP);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ RTE_SET_USED(start_bit);
+ RTE_SET_USED(len);
+ return 0;
+#endif
}
/**
@@ -766,7 +867,13 @@ __rte_experimental
static inline ssize_t
rte_bitset_find_first_clear(const uint64_t *bitset, size_t size)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __rte_bitset_find(bitset, size, 0, size, __RTE_BITSET_FIND_FLAG_FIND_CLEAR);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ return 0;
+#endif
}
/**
@@ -796,7 +903,15 @@ __rte_experimental
static inline ssize_t
rte_bitset_find_clear(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __rte_bitset_find(bitset, size, start_bit, len, __RTE_BITSET_FIND_FLAG_FIND_CLEAR);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ RTE_SET_USED(start_bit);
+ RTE_SET_USED(len);
+ return 0;
+#endif
}
/**
@@ -827,8 +942,16 @@ __rte_experimental
static inline ssize_t
rte_bitset_find_clear_wrap(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
{
+#ifdef ALLOW_EXPERIMENTAL_API
return __rte_bitset_find(bitset, size, start_bit, len,
__RTE_BITSET_FIND_FLAG_FIND_CLEAR | __RTE_BITSET_FIND_FLAG_WRAP);
+#else
+ RTE_SET_USED(bitset);
+ RTE_SET_USED(size);
+ RTE_SET_USED(start_bit);
+ RTE_SET_USED(len);
+ return 0;
+#endif
}
/**
--
2.46.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 11:38 ` [PATCH v2 2/4] bitset: " David Marchand
@ 2024-10-16 14:14 ` Mattias Rönnblom
2024-10-16 15:36 ` David Marchand
0 siblings, 1 reply; 23+ messages in thread
From: Mattias Rönnblom @ 2024-10-16 14:14 UTC (permalink / raw)
To: David Marchand, dev
Cc: thomas, bruce.richardson, ktraynor, Morten Brørup,
Mattias Rönnblom, Tyler Retzlaff
On 2024-10-16 13:38, David Marchand wrote:
> For a reason similar to the change on bitops header, hide bitset
> implementation relying on experimental API.
>
> Fixes: 99a1197647d8 ("eal: add bitset type")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
> lib/eal/include/rte_bitset.h | 123 +++++++++++++++++++++++++++++++++++
> 1 file changed, 123 insertions(+)
>
> diff --git a/lib/eal/include/rte_bitset.h b/lib/eal/include/rte_bitset.h
> index 74c643a72a..8ae8425fc2 100644
> --- a/lib/eal/include/rte_bitset.h
> +++ b/lib/eal/include/rte_bitset.h
> @@ -255,7 +255,13 @@ __rte_experimental
> static inline bool
> rte_bitset_test(const uint64_t *bitset, size_t bit_num)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + return false;
This is no RTE_VERIFY(0) here, because this is just dummy code, that
will never be run. Is that correct?
> +#endif
> }
>
> /**
> @@ -277,7 +283,12 @@ __rte_experimental
> static inline void
> rte_bitset_set(uint64_t *bitset, size_t bit_num)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE(rte_bit_set, bitset, bit_num);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> +#endif
> }
>
> /**
> @@ -299,7 +310,12 @@ __rte_experimental
> static inline void
> rte_bitset_clear(uint64_t *bitset, size_t bit_num)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE(rte_bit_clear, bitset, bit_num);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> +#endif
> }
>
> /**
> @@ -323,7 +339,13 @@ __rte_experimental
> static inline void
> rte_bitset_assign(uint64_t *bitset, size_t bit_num, bool bit_value)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE_N(rte_bit_assign, bitset, bit_num, bit_value);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + RTE_SET_USED(bit_value);
> +#endif
> }
>
> /**
> @@ -345,7 +367,12 @@ __rte_experimental
> static inline void
> rte_bitset_flip(uint64_t *bitset, size_t bit_num)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE(rte_bit_flip, bitset, bit_num);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> +#endif
> }
>
> /**
> @@ -370,7 +397,14 @@ __rte_experimental
> static inline bool
> rte_bitset_atomic_test(const uint64_t *bitset, size_t bit_num, int memory_order)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __RTE_BITSET_DELEGATE_N(rte_bit_atomic_test, bitset, bit_num, memory_order);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + RTE_SET_USED(memory_order);
> + return false;
> +#endif
> }
>
> /**
> @@ -399,7 +433,13 @@ __rte_experimental
> static inline void
> rte_bitset_atomic_set(uint64_t *bitset, size_t bit_num, int memory_order)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE_N(rte_bit_atomic_set, bitset, bit_num, memory_order);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + RTE_SET_USED(memory_order);
> +#endif
> }
>
> /**
> @@ -428,7 +468,13 @@ __rte_experimental
> static inline void
> rte_bitset_atomic_clear(uint64_t *bitset, size_t bit_num, int memory_order)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE_N(rte_bit_atomic_clear, bitset, bit_num, memory_order);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + RTE_SET_USED(memory_order);
> +#endif
> }
>
> /**
> @@ -459,7 +505,14 @@ __rte_experimental
> static inline void
> rte_bitset_atomic_assign(uint64_t *bitset, size_t bit_num, bool bit_value, int memory_order)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE_N(rte_bit_atomic_assign, bitset, bit_num, bit_value, memory_order);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + RTE_SET_USED(bit_value);
> + RTE_SET_USED(memory_order);
> +#endif
> }
>
> /**
> @@ -488,7 +541,13 @@ __rte_experimental
> static inline void
> rte_bitset_atomic_flip(uint64_t *bitset, size_t bit_num, int memory_order)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> __RTE_BITSET_DELEGATE_N(rte_bit_atomic_flip, bitset, bit_num, memory_order);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(bit_num);
> + RTE_SET_USED(memory_order);
> +#endif
> }
>
> /**
> @@ -524,7 +583,12 @@ __rte_experimental
> static inline void
> rte_bitset_clear_all(uint64_t *bitset, size_t size)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> rte_bitset_init(bitset, size);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> +#endif
> }
>
> /**
> @@ -576,7 +640,13 @@ __rte_experimental
> static inline size_t
> rte_bitset_count_clear(const uint64_t *bitset, size_t size)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return size - rte_bitset_count_set(bitset, size);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + return 0;
> +#endif
> }
>
> #define __RTE_BITSET_FIND_FLAG_FIND_CLEAR (1U << 0)
> @@ -638,6 +708,7 @@ static inline ssize_t
> __rte_bitset_find(const uint64_t *bitset, size_t size, size_t start_bit, size_t len,
> unsigned int flags)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> bool find_clear = flags & __RTE_BITSET_FIND_FLAG_FIND_CLEAR;
> bool may_wrap = flags & __RTE_BITSET_FIND_FLAG_WRAP;
> bool does_wrap = (start_bit + len) > size;
> @@ -658,6 +729,14 @@ __rte_bitset_find(const uint64_t *bitset, size_t size, size_t start_bit, size_t
> rc = __rte_bitset_find_nowrap(bitset, size, start_bit, len, find_clear);
>
> return rc;
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + RTE_SET_USED(start_bit);
> + RTE_SET_USED(len);
> + RTE_SET_USED(flags);
> + return 0;
> +#endif
> }
>
> /**
> @@ -681,7 +760,13 @@ __rte_experimental
> static inline ssize_t
> rte_bitset_find_first_set(const uint64_t *bitset, size_t size)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __rte_bitset_find(bitset, size, 0, size, 0);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + return 0;
> +#endif
> }
>
> /**
> @@ -711,7 +796,15 @@ __rte_experimental
> static inline ssize_t
> rte_bitset_find_set(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __rte_bitset_find(bitset, size, start_bit, len, 0);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + RTE_SET_USED(start_bit);
> + RTE_SET_USED(len);
> + return 0;
> +#endif
> }
>
> /**
> @@ -742,7 +835,15 @@ __rte_experimental
> static inline ssize_t
> rte_bitset_find_set_wrap(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __rte_bitset_find(bitset, size, start_bit, len, __RTE_BITSET_FIND_FLAG_WRAP);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + RTE_SET_USED(start_bit);
> + RTE_SET_USED(len);
> + return 0;
> +#endif
> }
>
> /**
> @@ -766,7 +867,13 @@ __rte_experimental
> static inline ssize_t
> rte_bitset_find_first_clear(const uint64_t *bitset, size_t size)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __rte_bitset_find(bitset, size, 0, size, __RTE_BITSET_FIND_FLAG_FIND_CLEAR);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + return 0;
> +#endif
> }
>
> /**
> @@ -796,7 +903,15 @@ __rte_experimental
> static inline ssize_t
> rte_bitset_find_clear(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __rte_bitset_find(bitset, size, start_bit, len, __RTE_BITSET_FIND_FLAG_FIND_CLEAR);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + RTE_SET_USED(start_bit);
> + RTE_SET_USED(len);
> + return 0;
> +#endif
> }
>
> /**
> @@ -827,8 +942,16 @@ __rte_experimental
> static inline ssize_t
> rte_bitset_find_clear_wrap(const uint64_t *bitset, size_t size, size_t start_bit, size_t len)
> {
> +#ifdef ALLOW_EXPERIMENTAL_API
> return __rte_bitset_find(bitset, size, start_bit, len,
> __RTE_BITSET_FIND_FLAG_FIND_CLEAR | __RTE_BITSET_FIND_FLAG_WRAP);
> +#else
> + RTE_SET_USED(bitset);
> + RTE_SET_USED(size);
> + RTE_SET_USED(start_bit);
> + RTE_SET_USED(len);
> + return 0;
> +#endif
> }
>
> /**
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 14:14 ` Mattias Rönnblom
@ 2024-10-16 15:36 ` David Marchand
2024-10-16 15:42 ` Morten Brørup
2024-10-16 15:51 ` Mattias Rönnblom
0 siblings, 2 replies; 23+ messages in thread
From: David Marchand @ 2024-10-16 15:36 UTC (permalink / raw)
To: Mattias Rönnblom
Cc: dev, thomas, bruce.richardson, ktraynor, Morten Brørup,
Mattias Rönnblom, Tyler Retzlaff
On Wed, Oct 16, 2024 at 4:14 PM Mattias Rönnblom <hofors@lysator.liu.se> wrote:
>
> On 2024-10-16 13:38, David Marchand wrote:
> > For a reason similar to the change on bitops header, hide bitset
> > implementation relying on experimental API.
> >
> > Fixes: 99a1197647d8 ("eal: add bitset type")
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > ---
> > lib/eal/include/rte_bitset.h | 123 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 123 insertions(+)
> >
> > diff --git a/lib/eal/include/rte_bitset.h b/lib/eal/include/rte_bitset.h
> > index 74c643a72a..8ae8425fc2 100644
> > --- a/lib/eal/include/rte_bitset.h
> > +++ b/lib/eal/include/rte_bitset.h
> > @@ -255,7 +255,13 @@ __rte_experimental
> > static inline bool
> > rte_bitset_test(const uint64_t *bitset, size_t bit_num)
> > {
> > +#ifdef ALLOW_EXPERIMENTAL_API
> > return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
> > +#else
> > + RTE_SET_USED(bitset);
> > + RTE_SET_USED(bit_num);
> > + return false;
>
> This is no RTE_VERIFY(0) here, because this is just dummy code, that
> will never be run. Is that correct?
Adding a RTE_VERIFY(false) is an interesting idea.
It is not supposed to be run, indeed.
Do you prefer I respin with this?
--
David Marchand
^ permalink raw reply [flat|nested] 23+ messages in thread* RE: [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 15:36 ` David Marchand
@ 2024-10-16 15:42 ` Morten Brørup
2024-10-16 16:03 ` Mattias Rönnblom
2024-10-16 15:51 ` Mattias Rönnblom
1 sibling, 1 reply; 23+ messages in thread
From: Morten Brørup @ 2024-10-16 15:42 UTC (permalink / raw)
To: David Marchand, Mattias Rönnblom
Cc: dev, thomas, bruce.richardson, ktraynor, Mattias Rönnblom,
Tyler Retzlaff
> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, 16 October 2024 17.37
>
> On Wed, Oct 16, 2024 at 4:14 PM Mattias Rönnblom
> <hofors@lysator.liu.se> wrote:
> >
> > On 2024-10-16 13:38, David Marchand wrote:
> > > For a reason similar to the change on bitops header, hide bitset
> > > implementation relying on experimental API.
> > >
> > > Fixes: 99a1197647d8 ("eal: add bitset type")
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > > ---
> > > lib/eal/include/rte_bitset.h | 123
> +++++++++++++++++++++++++++++++++++
> > > 1 file changed, 123 insertions(+)
> > >
> > > diff --git a/lib/eal/include/rte_bitset.h
> b/lib/eal/include/rte_bitset.h
> > > index 74c643a72a..8ae8425fc2 100644
> > > --- a/lib/eal/include/rte_bitset.h
> > > +++ b/lib/eal/include/rte_bitset.h
> > > @@ -255,7 +255,13 @@ __rte_experimental
> > > static inline bool
> > > rte_bitset_test(const uint64_t *bitset, size_t bit_num)
> > > {
> > > +#ifdef ALLOW_EXPERIMENTAL_API
> > > return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
> > > +#else
> > > + RTE_SET_USED(bitset);
> > > + RTE_SET_USED(bit_num);
> > > + return false;
> >
> > This is no RTE_VERIFY(0) here, because this is just dummy code, that
> > will never be run. Is that correct?
>
> Adding a RTE_VERIFY(false) is an interesting idea.
> It is not supposed to be run, indeed.
Great idea.
>
> Do you prefer I respin with this?
No need.
Instead, create a ticket in Bugzilla so RTE_VERIFY(false) goes in everywhere there is dummy code, not just here.
>
>
> --
> David Marchand
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 15:42 ` Morten Brørup
@ 2024-10-16 16:03 ` Mattias Rönnblom
2024-10-16 16:17 ` Thomas Monjalon
0 siblings, 1 reply; 23+ messages in thread
From: Mattias Rönnblom @ 2024-10-16 16:03 UTC (permalink / raw)
To: Morten Brørup, David Marchand
Cc: dev, thomas, bruce.richardson, ktraynor, Mattias Rönnblom,
Tyler Retzlaff
On 2024-10-16 17:42, Morten Brørup wrote:
>> From: David Marchand [mailto:david.marchand@redhat.com]
>> Sent: Wednesday, 16 October 2024 17.37
>>
>> On Wed, Oct 16, 2024 at 4:14 PM Mattias Rönnblom
>> <hofors@lysator.liu.se> wrote:
>>>
>>> On 2024-10-16 13:38, David Marchand wrote:
>>>> For a reason similar to the change on bitops header, hide bitset
>>>> implementation relying on experimental API.
>>>>
>>>> Fixes: 99a1197647d8 ("eal: add bitset type")
>>>>
>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>> ---
>>>> lib/eal/include/rte_bitset.h | 123
>> +++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 123 insertions(+)
>>>>
>>>> diff --git a/lib/eal/include/rte_bitset.h
>> b/lib/eal/include/rte_bitset.h
>>>> index 74c643a72a..8ae8425fc2 100644
>>>> --- a/lib/eal/include/rte_bitset.h
>>>> +++ b/lib/eal/include/rte_bitset.h
>>>> @@ -255,7 +255,13 @@ __rte_experimental
>>>> static inline bool
>>>> rte_bitset_test(const uint64_t *bitset, size_t bit_num)
>>>> {
>>>> +#ifdef ALLOW_EXPERIMENTAL_API
>>>> return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
>>>> +#else
>>>> + RTE_SET_USED(bitset);
>>>> + RTE_SET_USED(bit_num);
>>>> + return false;
>>>
>>> This is no RTE_VERIFY(0) here, because this is just dummy code, that
>>> will never be run. Is that correct?
>>
>> Adding a RTE_VERIFY(false) is an interesting idea.
>> It is not supposed to be run, indeed.
>
> Great idea.
>
>>
>> Do you prefer I respin with this?
>
> No need.
> Instead, create a ticket in Bugzilla so RTE_VERIFY(false) goes in everywhere there is dummy code, not just here.
>
No experimental function is supposed to be invoked, if
ALLOW_EXPERIMENTAL_API is not set, right? So RTE_VERIFY(), or its
compile-time equivalent, should be in every such function, not just the
broken ones.
>>
>>
>> --
>> David Marchand
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 16:03 ` Mattias Rönnblom
@ 2024-10-16 16:17 ` Thomas Monjalon
0 siblings, 0 replies; 23+ messages in thread
From: Thomas Monjalon @ 2024-10-16 16:17 UTC (permalink / raw)
To: Morten Brørup, David Marchand, Mattias Rönnblom
Cc: dev, bruce.richardson, ktraynor, Mattias Rönnblom, Tyler Retzlaff
16/10/2024 18:03, Mattias Rönnblom:
> On 2024-10-16 17:42, Morten Brørup wrote:
> >> From: David Marchand [mailto:david.marchand@redhat.com]
> >> Sent: Wednesday, 16 October 2024 17.37
> >>
> >> On Wed, Oct 16, 2024 at 4:14 PM Mattias Rönnblom
> >> <hofors@lysator.liu.se> wrote:
> >>>
> >>> On 2024-10-16 13:38, David Marchand wrote:
> >>>> For a reason similar to the change on bitops header, hide bitset
> >>>> implementation relying on experimental API.
> >>>>
> >>>> Fixes: 99a1197647d8 ("eal: add bitset type")
> >>>>
> >>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> >>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>>> ---
> >>>> lib/eal/include/rte_bitset.h | 123
> >> +++++++++++++++++++++++++++++++++++
> >>>> 1 file changed, 123 insertions(+)
> >>>>
> >>>> diff --git a/lib/eal/include/rte_bitset.h
> >> b/lib/eal/include/rte_bitset.h
> >>>> index 74c643a72a..8ae8425fc2 100644
> >>>> --- a/lib/eal/include/rte_bitset.h
> >>>> +++ b/lib/eal/include/rte_bitset.h
> >>>> @@ -255,7 +255,13 @@ __rte_experimental
> >>>> static inline bool
> >>>> rte_bitset_test(const uint64_t *bitset, size_t bit_num)
> >>>> {
> >>>> +#ifdef ALLOW_EXPERIMENTAL_API
> >>>> return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
> >>>> +#else
> >>>> + RTE_SET_USED(bitset);
> >>>> + RTE_SET_USED(bit_num);
> >>>> + return false;
> >>>
> >>> This is no RTE_VERIFY(0) here, because this is just dummy code, that
> >>> will never be run. Is that correct?
> >>
> >> Adding a RTE_VERIFY(false) is an interesting idea.
> >> It is not supposed to be run, indeed.
> >
> > Great idea.
> >
> >>
> >> Do you prefer I respin with this?
> >
> > No need.
> > Instead, create a ticket in Bugzilla so RTE_VERIFY(false) goes in everywhere there is dummy code, not just here.
> >
>
> No experimental function is supposed to be invoked, if
> ALLOW_EXPERIMENTAL_API is not set, right? So RTE_VERIFY(), or its
> compile-time equivalent, should be in every such function, not just the
> broken ones.
There is no other experimental dummy branch in the current version.
I think respin is a good option.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/4] bitset: fix build for GCC without experimental API
2024-10-16 15:36 ` David Marchand
2024-10-16 15:42 ` Morten Brørup
@ 2024-10-16 15:51 ` Mattias Rönnblom
1 sibling, 0 replies; 23+ messages in thread
From: Mattias Rönnblom @ 2024-10-16 15:51 UTC (permalink / raw)
To: David Marchand
Cc: dev, thomas, bruce.richardson, ktraynor, Morten Brørup,
Mattias Rönnblom, Tyler Retzlaff
On 2024-10-16 17:36, David Marchand wrote:
> On Wed, Oct 16, 2024 at 4:14 PM Mattias Rönnblom <hofors@lysator.liu.se> wrote:
>>
>> On 2024-10-16 13:38, David Marchand wrote:
>>> For a reason similar to the change on bitops header, hide bitset
>>> implementation relying on experimental API.
>>>
>>> Fixes: 99a1197647d8 ("eal: add bitset type")
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>> ---
>>> lib/eal/include/rte_bitset.h | 123 +++++++++++++++++++++++++++++++++++
>>> 1 file changed, 123 insertions(+)
>>>
>>> diff --git a/lib/eal/include/rte_bitset.h b/lib/eal/include/rte_bitset.h
>>> index 74c643a72a..8ae8425fc2 100644
>>> --- a/lib/eal/include/rte_bitset.h
>>> +++ b/lib/eal/include/rte_bitset.h
>>> @@ -255,7 +255,13 @@ __rte_experimental
>>> static inline bool
>>> rte_bitset_test(const uint64_t *bitset, size_t bit_num)
>>> {
>>> +#ifdef ALLOW_EXPERIMENTAL_API
>>> return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
>>> +#else
>>> + RTE_SET_USED(bitset);
>>> + RTE_SET_USED(bit_num);
>>> + return false;
>>
>> This is no RTE_VERIFY(0) here, because this is just dummy code, that
>> will never be run. Is that correct?
>
> Adding a RTE_VERIFY(false) is an interesting idea.
> It is not supposed to be run, indeed.
>
> Do you prefer I respin with this?
>
>
I reminded myself how ALLOW_EXPERIMENTAL_APIs works. This code may
indeed be run. Experimental just generates a compiler warning.
So RTE_VERIFY(0); is needed. You should be able to remove the "return
false;" statement.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 3/4] vhost: remove internal vDPA API description from public header
2024-10-16 11:38 ` [PATCH v2 0/4] Enhance headers check David Marchand
2024-10-16 11:38 ` [PATCH v2 1/4] bitops: fix build for GCC without experimental API David Marchand
2024-10-16 11:38 ` [PATCH v2 2/4] bitset: " David Marchand
@ 2024-10-16 11:38 ` David Marchand
2024-10-16 11:47 ` Maxime Coquelin
2024-10-16 11:38 ` [PATCH v2 4/4] buildtools/chkincs: check headers with stable API only David Marchand
2024-10-16 20:40 ` [PATCH v2 0/4] Enhance headers check David Marchand
4 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2024-10-16 11:38 UTC (permalink / raw)
To: dev
Cc: thomas, bruce.richardson, ktraynor, Maxime Coquelin, Chenbo Xia,
Adrian Moreno
rte_vdpa_relay_vring_used() is an internal symbol which was left behind
in the application header when doing the application vs driver API
split.
Fixes: a49f758d1170 ("vhost: split vDPA header file")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
lib/vhost/rte_vdpa.h | 17 -----------------
1 file changed, 17 deletions(-)
diff --git a/lib/vhost/rte_vdpa.h b/lib/vhost/rte_vdpa.h
index 18e273c20f..3a7755d82d 100644
--- a/lib/vhost/rte_vdpa.h
+++ b/lib/vhost/rte_vdpa.h
@@ -108,23 +108,6 @@ rte_vdpa_get_features(struct rte_vdpa_device *dev, uint64_t *features);
int
rte_vdpa_get_protocol_features(struct rte_vdpa_device *dev, uint64_t *features);
-/**
- * Synchronize the used ring from mediated ring to guest, log dirty
- * page for each writeable buffer, caller should handle the used
- * ring logging before device stop.
- *
- * @param vid
- * vhost device id
- * @param qid
- * vhost queue id
- * @param vring_m
- * mediated virtio ring pointer
- * @return
- * number of synced used entries on success, -1 on failure
- */
-int
-rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
-
/**
* Retrieve names of statistics of a vDPA device.
*
--
2.46.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 3/4] vhost: remove internal vDPA API description from public header
2024-10-16 11:38 ` [PATCH v2 3/4] vhost: remove internal vDPA API description from public header David Marchand
@ 2024-10-16 11:47 ` Maxime Coquelin
0 siblings, 0 replies; 23+ messages in thread
From: Maxime Coquelin @ 2024-10-16 11:47 UTC (permalink / raw)
To: David Marchand, dev
Cc: thomas, bruce.richardson, ktraynor, Chenbo Xia, Adrian Moreno
On 10/16/24 13:38, David Marchand wrote:
> rte_vdpa_relay_vring_used() is an internal symbol which was left behind
> in the application header when doing the application vs driver API
> split.
>
> Fixes: a49f758d1170 ("vhost: split vDPA header file")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> lib/vhost/rte_vdpa.h | 17 -----------------
> 1 file changed, 17 deletions(-)
>
Good catch!
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/4] buildtools/chkincs: check headers with stable API only
2024-10-16 11:38 ` [PATCH v2 0/4] Enhance headers check David Marchand
` (2 preceding siblings ...)
2024-10-16 11:38 ` [PATCH v2 3/4] vhost: remove internal vDPA API description from public header David Marchand
@ 2024-10-16 11:38 ` David Marchand
2024-10-16 20:40 ` [PATCH v2 0/4] Enhance headers check David Marchand
4 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2024-10-16 11:38 UTC (permalink / raw)
To: dev; +Cc: thomas, bruce.richardson, ktraynor
An exported header should be usable w/ and w/o ALLOW_EXPERIMENTAL_API so
that an application that only wants stable API may include it.
Plus, the widely common case is that an application will not use
internal API.
Cover those cases but keep the original test.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
buildtools/chkincs/meson.build | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 8da221fc33..787d70272b 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -12,8 +12,6 @@ gen_c_files = generator(gen_c_file_for_header,
arguments: ['@INPUT@', '@OUTPUT@'])
cflags = machine_args
-cflags += '-DALLOW_EXPERIMENTAL_API'
-cflags += '-DALLOW_INTERNAL_API'
sources = files('main.c')
sources += gen_c_files.process(dpdk_chkinc_headers)
@@ -32,6 +30,18 @@ executable('chkincs', sources,
dependencies: deps,
install: false)
+executable('chkincs-exp', sources,
+ c_args: [cflags, '-DALLOW_EXPERIMENTAL_API'],
+ include_directories: includes,
+ dependencies: deps,
+ install: false)
+
+executable('chkincs-all', sources,
+ c_args: [cflags, '-DALLOW_EXPERIMENTAL_API', '-DALLOW_INTERNAL_API'],
+ include_directories: includes,
+ dependencies: deps,
+ install: false)
+
# run tests for c++ builds also
if not add_languages('cpp', required: false)
subdir_done()
@@ -49,3 +59,16 @@ executable('chkincs-cpp', cpp_sources,
include_directories: includes,
dependencies: deps,
install: false)
+
+executable('chkincs-cpp-exp', cpp_sources,
+ cpp_args: ['-include', 'rte_config.h', cflags, '-DALLOW_EXPERIMENTAL_API'],
+ include_directories: includes,
+ dependencies: deps,
+ install: false)
+
+executable('chkincs-cpp-all', cpp_sources,
+ cpp_args: ['-include', 'rte_config.h', cflags, '-DALLOW_EXPERIMENTAL_API',
+ '-DALLOW_INTERNAL_API'],
+ include_directories: includes,
+ dependencies: deps,
+ install: false)
--
2.46.2
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 0/4] Enhance headers check
2024-10-16 11:38 ` [PATCH v2 0/4] Enhance headers check David Marchand
` (3 preceding siblings ...)
2024-10-16 11:38 ` [PATCH v2 4/4] buildtools/chkincs: check headers with stable API only David Marchand
@ 2024-10-16 20:40 ` David Marchand
4 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2024-10-16 20:40 UTC (permalink / raw)
To: dev; +Cc: thomas, bruce.richardson, ktraynor
On Wed, Oct 16, 2024 at 1:38 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> We currently check that exported headers are fine with
> -DALLOW_EXPERIMENTAL_API and -DALLOW_INTERNAL_API.
>
> Such a check won't catch issues like the one fixed in patch 1, where OVS
> compilation is broken by the additional of experimental API in a header
> commonly included in other parts of DPDK.
>
> Ideally, I would like to merge patch 1 in rc1.
> The patch 2 is not a real issue (more like the enhanced check would
> complain about this header).
> In any case, I think it would not hurt merging everything now.
Applied patch 1 and 3.
I will respin the series (or come up with a different fix for patch 2) for rc2.
--
David Marchand
^ permalink raw reply [flat|nested] 23+ messages in thread