diff mbox series

ethdev: add extension keyword to statement expression macro

Message ID 1699560818-7453-1-git-send-email-roretzla@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series ethdev: add extension keyword to statement expression macro |

Checks

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

Commit Message

Tyler Retzlaff Nov. 9, 2023, 8:13 p.m. UTC 
  add missing __extension__ keyword to macros using gcc statement
expression extension.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/ethdev/rte_mtr.c | 10 +++++-----
 lib/ethdev/rte_tm.c  |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

Comments

Ferruh Yigit Nov. 10, 2023, 5:22 p.m. UTC | #1 
On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
> add missing __extension__ keyword to macros using gcc statement
> expression extension.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>


It seems there are a few more usage not marked in lib folder [1], and
more including drivers.

Is this compiler extension causing any problem for the Windows build or
MSVC toolchain?



[1]
$ grep -r '({' lib/ | grep -v __extension__
lib/port/rte_port_source_sink.c:({
lib/port/rte_port_source_sink.c:({
lib/pipeline/rte_swx_pipeline_internal.h:({
lib/pipeline/rte_pipeline.c:    ({ (p)->n_pkts_ah_drop =
lib/pipeline/rte_pipeline.c:    ({ (counter) += (p)->n_pkts_ah_drop;
lib/pipeline/rte_pipeline.c:    ({ (p)->pkts_drop_mask =
lib/pipeline/rte_pipeline.c:({
lib/ethdev/rte_mtr.c:({
lib/ethdev/rte_mtr.c:({
lib/ethdev/rte_tm.c:({
Ferruh Yigit Nov. 10, 2023, 8:32 p.m. UTC | #2 
On 11/10/2023 5:22 PM, Ferruh Yigit wrote:
> On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
>> add missing __extension__ keyword to macros using gcc statement
>> expression extension.
>>
>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 

Applied to dpdk-next-net/main, thanks.
Tyler Retzlaff Nov. 10, 2023, 8:49 p.m. UTC | #3 
On Fri, Nov 10, 2023 at 05:22:52PM +0000, Ferruh Yigit wrote:
> On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
> > add missing __extension__ keyword to macros using gcc statement
> > expression extension.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > 
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> 
> It seems there are a few more usage not marked in lib folder [1], and
> more including drivers.

yeah, sorry i'm not doing them all at once. though given the number of
instances left i'll see if i can find a few minutes maybe just clear
lib/* entirely drivers like for everything else are lower priority for
me right now (always open to help of course).

> 
> Is this compiler extension causing any problem for the Windows build or
> MSVC toolchain?

just the msvc toolchain, builds with gcc/clang targeting windows work fine.

i'm working internally with the msvc toolchain team to enable support
for the extension on the condition that it be marked with the
__extension__ keyword so i'm just making sure it is applied
consistently.

> 
> 
> 
> [1]
> $ grep -r '({' lib/ | grep -v __extension__
> lib/port/rte_port_source_sink.c:({
> lib/port/rte_port_source_sink.c:({
> lib/pipeline/rte_swx_pipeline_internal.h:({
> lib/pipeline/rte_pipeline.c:    ({ (p)->n_pkts_ah_drop =
> lib/pipeline/rte_pipeline.c:    ({ (counter) += (p)->n_pkts_ah_drop;
> lib/pipeline/rte_pipeline.c:    ({ (p)->pkts_drop_mask =
> lib/pipeline/rte_pipeline.c:({
> lib/ethdev/rte_mtr.c:({
> lib/ethdev/rte_mtr.c:({
> lib/ethdev/rte_tm.c:({
Ferruh Yigit Nov. 10, 2023, 11:25 p.m. UTC | #4 
On 11/10/2023 8:49 PM, Tyler Retzlaff wrote:
> On Fri, Nov 10, 2023 at 05:22:52PM +0000, Ferruh Yigit wrote:
>> On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
>>> add missing __extension__ keyword to macros using gcc statement
>>> expression extension.
>>>
>>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>>>
>>
>> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
>>
>>
>> It seems there are a few more usage not marked in lib folder [1], and
>> more including drivers.
> 
> yeah, sorry i'm not doing them all at once. though given the number of
> instances left i'll see if i can find a few minutes maybe just clear
> lib/* entirely drivers like for everything else are lower priority for
> me right now (always open to help of course).
> 

that is OK, I just want to remind

>>
>> Is this compiler extension causing any problem for the Windows build or
>> MSVC toolchain?
> 
> just the msvc toolchain, builds with gcc/clang targeting windows work fine.
> 
> i'm working internally with the msvc toolchain team to enable support
> for the extension on the condition that it be marked with the
> __extension__ keyword so i'm just making sure it is applied
> consistently.
> 

Nice, if just marking extensions is sufficient, that is easy to do.

>>
>>
>>
>> [1]
>> $ grep -r '({' lib/ | grep -v __extension__
>> lib/port/rte_port_source_sink.c:({
>> lib/port/rte_port_source_sink.c:({
>> lib/pipeline/rte_swx_pipeline_internal.h:({
>> lib/pipeline/rte_pipeline.c:    ({ (p)->n_pkts_ah_drop =
>> lib/pipeline/rte_pipeline.c:    ({ (counter) += (p)->n_pkts_ah_drop;
>> lib/pipeline/rte_pipeline.c:    ({ (p)->pkts_drop_mask =
>> lib/pipeline/rte_pipeline.c:({
>> lib/ethdev/rte_mtr.c:({
>> lib/ethdev/rte_mtr.c:({
>> lib/ethdev/rte_tm.c:({
Thomas Monjalon Nov. 11, 2023, 10:11 a.m. UTC | #5 
10/11/2023 18:22, Ferruh Yigit:
> On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
> > add missing __extension__ keyword to macros using gcc statement
> > expression extension.
> > 
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > 
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> 
> It seems there are a few more usage not marked in lib folder [1], and
> more including drivers.

Please let's mark all at once.

> Is this compiler extension causing any problem for the Windows build or
> MSVC toolchain?

I have the same question: is __extension__ supported in MSVC?

> [1]
> $ grep -r '({' lib/ | grep -v __extension__
> lib/port/rte_port_source_sink.c:({
> lib/port/rte_port_source_sink.c:({
> lib/pipeline/rte_swx_pipeline_internal.h:({
> lib/pipeline/rte_pipeline.c:    ({ (p)->n_pkts_ah_drop =
> lib/pipeline/rte_pipeline.c:    ({ (counter) += (p)->n_pkts_ah_drop;
> lib/pipeline/rte_pipeline.c:    ({ (p)->pkts_drop_mask =
> lib/pipeline/rte_pipeline.c:({
> lib/ethdev/rte_mtr.c:({
> lib/ethdev/rte_mtr.c:({
> lib/ethdev/rte_tm.c:({
Thomas Monjalon Nov. 11, 2023, 10:26 a.m. UTC | #6 
11/11/2023 00:25, Ferruh Yigit:
> On 11/10/2023 8:49 PM, Tyler Retzlaff wrote:
> > On Fri, Nov 10, 2023 at 05:22:52PM +0000, Ferruh Yigit wrote:
> >> On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
> >>> add missing __extension__ keyword to macros using gcc statement
> >>> expression extension.
> >>>
> >>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> >>>
> >>
> >> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> >>
> >>
> >> It seems there are a few more usage not marked in lib folder [1], and
> >> more including drivers.
> > 
> > yeah, sorry i'm not doing them all at once. though given the number of
> > instances left i'll see if i can find a few minutes maybe just clear
> > lib/* entirely drivers like for everything else are lower priority for
> > me right now (always open to help of course).
> > 
> 
> that is OK, I just want to remind

A commit on all libs would have more sense.
Here it is not even all ethdev.
I would not apply half a patch.
Ferruh Yigit Nov. 14, 2023, 2:49 p.m. UTC | #7 
On 11/11/2023 10:26 AM, Thomas Monjalon wrote:
> 11/11/2023 00:25, Ferruh Yigit:
>> On 11/10/2023 8:49 PM, Tyler Retzlaff wrote:
>>> On Fri, Nov 10, 2023 at 05:22:52PM +0000, Ferruh Yigit wrote:
>>>> On 11/9/2023 8:13 PM, Tyler Retzlaff wrote:
>>>>> add missing __extension__ keyword to macros using gcc statement
>>>>> expression extension.
>>>>>
>>>>> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
>>>>>
>>>>
>>>> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
>>>>
>>>>
>>>> It seems there are a few more usage not marked in lib folder [1], and
>>>> more including drivers.
>>>
>>> yeah, sorry i'm not doing them all at once. though given the number of
>>> instances left i'll see if i can find a few minutes maybe just clear
>>> lib/* entirely drivers like for everything else are lower priority for
>>> me right now (always open to help of course).
>>>
>>
>> that is OK, I just want to remind
> 
> A commit on all libs would have more sense.
> Here it is not even all ethdev.
> I would not apply half a patch.
> 
> 

Dropping patch from next-net, for the sake of more complete patch.

Patchwork status is now "change requested".
diff mbox series

Patch

diff --git a/lib/ethdev/rte_mtr.c b/lib/ethdev/rte_mtr.c
index 4e94af9..900837b 100644
--- a/lib/ethdev/rte_mtr.c
+++ b/lib/ethdev/rte_mtr.c
@@ -41,14 +41,14 @@ 
 }
 
 #define RTE_MTR_FUNC(port_id, func)			\
-({							\
+__extension__ ({					\
 	const struct rte_mtr_ops *ops =			\
-		rte_mtr_ops_get(port_id, error);		\
-	if (ops == NULL)					\
+		rte_mtr_ops_get(port_id, error);	\
+	if (ops == NULL)				\
 		return -rte_errno;			\
 							\
 	if (ops->func == NULL)				\
-		return -rte_mtr_error_set(error,		\
+		return -rte_mtr_error_set(error,	\
 			ENOSYS,				\
 			RTE_MTR_ERROR_TYPE_UNSPECIFIED,	\
 			NULL,				\
@@ -58,7 +58,7 @@ 
 })
 
 #define RTE_MTR_HNDL_FUNC(port_id, func)		\
-({							\
+__extension__ ({					\
 	const struct rte_mtr_ops *ops =			\
 		rte_mtr_ops_get(port_id, error);	\
 	if (ops == NULL)				\
diff --git a/lib/ethdev/rte_tm.c b/lib/ethdev/rte_tm.c
index 2d08141..d594fe0 100644
--- a/lib/ethdev/rte_tm.c
+++ b/lib/ethdev/rte_tm.c
@@ -40,11 +40,11 @@ 
 	return ops;
 }
 
-#define RTE_TM_FUNC(port_id, func)				\
-({							\
+#define RTE_TM_FUNC(port_id, func)			\
+__extension__ ({					\
 	const struct rte_tm_ops *ops =			\
 		rte_tm_ops_get(port_id, error);		\
-	if (ops == NULL)					\
+	if (ops == NULL)				\
 		return -rte_errno;			\
 							\
 	if (ops->func == NULL)				\