* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-10-31 20:23 [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write() Nathan Chancellor
@ 2023-10-31 21:49 ` Nick Desaulniers
2023-11-01 4:49 ` Eric Dumazet
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2023-10-31 21:49 UTC (permalink / raw)
To: Nathan Chancellor
Cc: edumazet, davem, dsahern, kuba, pabeni, trix, 0x7f454c46,
fruggeri, noureddine, netdev, linux-kernel, llvm, patches
On Tue, Oct 31, 2023 at 1:23 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>
> net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> 663 | }
> | ^
> 1 error generated.
>
> On earlier releases (such as clang-11, the current minimum supported
> version for building the kernel) that do not support C23, this was a
> hard error unconditionally:
>
> net/ipv4/tcp_output.c:663:2: error: expected statement
> }
> ^
> 1 error generated.
>
> Add a semicolon after the label to create an empty statement, which
> resolves the warning or error for all compilers.
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
> Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Well, at least ISO fixed that in C23...I found it annoying. One day we
might have nice things. Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> net/ipv4/tcp_output.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f558c054cf6e..6064895daece 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -658,7 +658,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> memset(ptr, TCPOPT_NOP, sizeof(*ptr));
> ptr++;
> }
> -out_ao:
> +out_ao:;
> #endif
> }
> if (unlikely(opts->mss)) {
>
> ---
> base-commit: 55c900477f5b3897d9038446f72a281cae0efd86
> change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-10-31 20:23 [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write() Nathan Chancellor
2023-10-31 21:49 ` Nick Desaulniers
@ 2023-11-01 4:49 ` Eric Dumazet
2023-11-02 0:41 ` Palmer Dabbelt
2023-11-03 8:22 ` Christoph Hellwig
3 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2023-11-01 4:49 UTC (permalink / raw)
To: Nathan Chancellor
Cc: davem, dsahern, kuba, pabeni, ndesaulniers, trix, 0x7f454c46,
fruggeri, noureddine, netdev, linux-kernel, llvm, patches
On Tue, Oct 31, 2023 at 9:23 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>
> net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> 663 | }
> | ^
> 1 error generated.
>
> On earlier releases (such as clang-11, the current minimum supported
> version for building the kernel) that do not support C23, this was a
> hard error unconditionally:
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-10-31 20:23 [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write() Nathan Chancellor
2023-10-31 21:49 ` Nick Desaulniers
2023-11-01 4:49 ` Eric Dumazet
@ 2023-11-02 0:41 ` Palmer Dabbelt
2023-11-02 1:07 ` Nathan Chancellor
2023-11-03 8:22 ` Christoph Hellwig
3 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2023-11-02 0:41 UTC (permalink / raw)
To: nathan
Cc: edumazet, davem, dsahern, kuba, pabeni, ndesaulniers, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches, nathan
On Tue, 31 Oct 2023 13:23:35 PDT (-0700), nathan@kernel.org wrote:
> Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>
> net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> 663 | }
> | ^
> 1 error generated.
>
> On earlier releases (such as clang-11, the current minimum supported
> version for building the kernel) that do not support C23, this was a
> hard error unconditionally:
>
> net/ipv4/tcp_output.c:663:2: error: expected statement
> }
> ^
> 1 error generated.
>
> Add a semicolon after the label to create an empty statement, which
> resolves the warning or error for all compilers.
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
> Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
> net/ipv4/tcp_output.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f558c054cf6e..6064895daece 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -658,7 +658,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> memset(ptr, TCPOPT_NOP, sizeof(*ptr));
> ptr++;
> }
> -out_ao:
> +out_ao:;
> #endif
> }
> if (unlikely(opts->mss)) {
>
> ---
> base-commit: 55c900477f5b3897d9038446f72a281cae0efd86
> change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
>
> Best regards,
This gives me a
linux/net/ipv4/tcp_output.c:663:2: error: expected statement
}
on GCC for me. So I think something like
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f558c054cf6e..ca09763acaa8 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -659,6 +659,11 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
ptr++;
}
out_ao:
+ /*
+ * Labels at the end of compound statements are a C23 feature, so
+ * introduce a block to avoid a warning/error on strict toolchains.
+ */
+ {}
#endif
}
if (unlikely(opts->mss)) {
should do it (though it's still build testing...)
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-11-02 0:41 ` Palmer Dabbelt
@ 2023-11-02 1:07 ` Nathan Chancellor
2023-11-02 1:42 ` Palmer Dabbelt
0 siblings, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2023-11-02 1:07 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: edumazet, davem, dsahern, kuba, pabeni, ndesaulniers, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches
On Wed, Nov 01, 2023 at 05:41:10PM -0700, Palmer Dabbelt wrote:
> On Tue, 31 Oct 2023 13:23:35 PDT (-0700), nathan@kernel.org wrote:
> > Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
> >
> > net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> > 663 | }
> > | ^
> > 1 error generated.
> >
> > On earlier releases (such as clang-11, the current minimum supported
> > version for building the kernel) that do not support C23, this was a
> > hard error unconditionally:
> >
> > net/ipv4/tcp_output.c:663:2: error: expected statement
> > }
> > ^
> > 1 error generated.
> >
> > Add a semicolon after the label to create an empty statement, which
> > resolves the warning or error for all compilers.
> >
> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
> > Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> > net/ipv4/tcp_output.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index f558c054cf6e..6064895daece 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -658,7 +658,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> > memset(ptr, TCPOPT_NOP, sizeof(*ptr));
> > ptr++;
> > }
> > -out_ao:
> > +out_ao:;
> > #endif
> > }
> > if (unlikely(opts->mss)) {
> >
> > ---
> > base-commit: 55c900477f5b3897d9038446f72a281cae0efd86
> > change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
> >
> > Best regards,
>
> This gives me a
>
> linux/net/ipv4/tcp_output.c:663:2: error: expected statement
> }
>
> on GCC for me.
What GCC version?
I cannot reproduce that error with my patch applied. I tested mainline
at commit deefd5024f07 ("Merge tag 'vfio-v6.7-rc1' of
https://github.com/awilliam/linux-vfio") using GCC 6 from kernel.org and
I can reproduce a similar failure with ARCH=x86_64 allyesconfig:
net/ipv4/tcp_output.c: In function 'tcp_options_write':
net/ipv4/tcp_output.c:661:1: error: label at end of compound statement
out_ao:
^~~~~~
With this change applied, the error disappears for GCC 6 and GCC 13
continues to build without error. I can try the other supported versions
later, I just did an older and newer one for a quick test.
> So I think something like
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f558c054cf6e..ca09763acaa8 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -659,6 +659,11 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
> ptr++;
> }
> out_ao:
> + /*
> + * Labels at the end of compound statements are a C23 feature, so
> + * introduce a block to avoid a warning/error on strict toolchains.
> + */
> + {}
> #endif
> }
> if (unlikely(opts->mss)) {
>
> should do it (though it's still build testing...)
I am not opposed to this once we understand what versions are affected
by this so that we have some timeline of removing this workaround.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-11-02 1:07 ` Nathan Chancellor
@ 2023-11-02 1:42 ` Palmer Dabbelt
2023-11-02 20:23 ` Palmer Dabbelt
0 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2023-11-02 1:42 UTC (permalink / raw)
To: nathan
Cc: edumazet, davem, dsahern, kuba, pabeni, ndesaulniers, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches
On Wed, 01 Nov 2023 18:07:23 PDT (-0700), nathan@kernel.org wrote:
> On Wed, Nov 01, 2023 at 05:41:10PM -0700, Palmer Dabbelt wrote:
>> On Tue, 31 Oct 2023 13:23:35 PDT (-0700), nathan@kernel.org wrote:
>> > Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>> >
>> > net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
>> > 663 | }
>> > | ^
>> > 1 error generated.
>> >
>> > On earlier releases (such as clang-11, the current minimum supported
>> > version for building the kernel) that do not support C23, this was a
>> > hard error unconditionally:
>> >
>> > net/ipv4/tcp_output.c:663:2: error: expected statement
>> > }
>> > ^
>> > 1 error generated.
>> >
>> > Add a semicolon after the label to create an empty statement, which
>> > resolves the warning or error for all compilers.
>> >
>> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
>> > Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
>> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>> > ---
>> > net/ipv4/tcp_output.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>> > index f558c054cf6e..6064895daece 100644
>> > --- a/net/ipv4/tcp_output.c
>> > +++ b/net/ipv4/tcp_output.c
>> > @@ -658,7 +658,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
>> > memset(ptr, TCPOPT_NOP, sizeof(*ptr));
>> > ptr++;
>> > }
>> > -out_ao:
>> > +out_ao:;
>> > #endif
>> > }
>> > if (unlikely(opts->mss)) {
>> >
>> > ---
>> > base-commit: 55c900477f5b3897d9038446f72a281cae0efd86
>> > change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
>> >
>> > Best regards,
>>
>> This gives me a
>>
>> linux/net/ipv4/tcp_output.c:663:2: error: expected statement
>> }
>>
>> on GCC for me.
>
> What GCC version?
12.1, though I can't get a smaller reproducer so I'm going to roll back
to your change and double-check. Might take a bit...
> I cannot reproduce that error with my patch applied. I tested mainline
> at commit deefd5024f07 ("Merge tag 'vfio-v6.7-rc1' of
> https://github.com/awilliam/linux-vfio") using GCC 6 from kernel.org and
> I can reproduce a similar failure with ARCH=x86_64 allyesconfig:
>
> net/ipv4/tcp_output.c: In function 'tcp_options_write':
> net/ipv4/tcp_output.c:661:1: error: label at end of compound statement
> out_ao:
> ^~~~~~
>
> With this change applied, the error disappears for GCC 6 and GCC 13
> continues to build without error. I can try the other supported versions
> later, I just did an older and newer one for a quick test.
>
>> So I think something like
>>
>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>> index f558c054cf6e..ca09763acaa8 100644
>> --- a/net/ipv4/tcp_output.c
>> +++ b/net/ipv4/tcp_output.c
>> @@ -659,6 +659,11 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
>> ptr++;
>> }
>> out_ao:
>> + /*
>> + * Labels at the end of compound statements are a C23 feature, so
>> + * introduce a block to avoid a warning/error on strict toolchains.
>> + */
>> + {}
>> #endif
>> }
>> if (unlikely(opts->mss)) {
>>
>> should do it (though it's still build testing...)
>
> I am not opposed to this once we understand what versions are affected
> by this so that we have some timeline of removing this workaround.
>
> Cheers,
> Nathan
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-11-02 1:42 ` Palmer Dabbelt
@ 2023-11-02 20:23 ` Palmer Dabbelt
0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2023-11-02 20:23 UTC (permalink / raw)
To: nathan
Cc: edumazet, davem, dsahern, kuba, pabeni, ndesaulniers, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches
On Wed, 01 Nov 2023 18:42:10 PDT (-0700), Palmer Dabbelt wrote:
> On Wed, 01 Nov 2023 18:07:23 PDT (-0700), nathan@kernel.org wrote:
>> On Wed, Nov 01, 2023 at 05:41:10PM -0700, Palmer Dabbelt wrote:
>>> On Tue, 31 Oct 2023 13:23:35 PDT (-0700), nathan@kernel.org wrote:
>>> > Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>>> >
>>> > net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
>>> > 663 | }
>>> > | ^
>>> > 1 error generated.
>>> >
>>> > On earlier releases (such as clang-11, the current minimum supported
>>> > version for building the kernel) that do not support C23, this was a
>>> > hard error unconditionally:
>>> >
>>> > net/ipv4/tcp_output.c:663:2: error: expected statement
>>> > }
>>> > ^
>>> > 1 error generated.
>>> >
>>> > Add a semicolon after the label to create an empty statement, which
>>> > resolves the warning or error for all compilers.
>>> >
>>> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1953
>>> > Fixes: 1e03d32bea8e ("net/tcp: Add TCP-AO sign to outgoing packets")
>>> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>> > ---
>>> > net/ipv4/tcp_output.c | 2 +-
>>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >
>>> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>> > index f558c054cf6e..6064895daece 100644
>>> > --- a/net/ipv4/tcp_output.c
>>> > +++ b/net/ipv4/tcp_output.c
>>> > @@ -658,7 +658,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
>>> > memset(ptr, TCPOPT_NOP, sizeof(*ptr));
>>> > ptr++;
>>> > }
>>> > -out_ao:
>>> > +out_ao:;
>>> > #endif
>>> > }
>>> > if (unlikely(opts->mss)) {
>>> >
>>> > ---
>>> > base-commit: 55c900477f5b3897d9038446f72a281cae0efd86
>>> > change-id: 20231031-tcp-ao-fix-label-in-compound-statement-warning-ebd6c9978498
>>> >
>>> > Best regards,
>>>
>>> This gives me a
>>>
>>> linux/net/ipv4/tcp_output.c:663:2: error: expected statement
>>> }
>>>
>>> on GCC for me.
>>
>> What GCC version?
>
> 12.1, though I can't get a smaller reproducer so I'm going to roll back
> to your change and double-check. Might take a bit...
Looks like there was just some bug in my test scripts and the original
patch wasn't actually picked up for all the configs. It's working now,
so
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Sorry for the confusion!
>> I cannot reproduce that error with my patch applied. I tested mainline
>> at commit deefd5024f07 ("Merge tag 'vfio-v6.7-rc1' of
>> https://github.com/awilliam/linux-vfio") using GCC 6 from kernel.org and
>> I can reproduce a similar failure with ARCH=x86_64 allyesconfig:
>>
>> net/ipv4/tcp_output.c: In function 'tcp_options_write':
>> net/ipv4/tcp_output.c:661:1: error: label at end of compound statement
>> out_ao:
>> ^~~~~~
>>
>> With this change applied, the error disappears for GCC 6 and GCC 13
>> continues to build without error. I can try the other supported versions
>> later, I just did an older and newer one for a quick test.
>>
>>> So I think something like
>>>
>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>> index f558c054cf6e..ca09763acaa8 100644
>>> --- a/net/ipv4/tcp_output.c
>>> +++ b/net/ipv4/tcp_output.c
>>> @@ -659,6 +659,11 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
>>> ptr++;
>>> }
>>> out_ao:
>>> + /*
>>> + * Labels at the end of compound statements are a C23 feature, so
>>> + * introduce a block to avoid a warning/error on strict toolchains.
>>> + */
>>> + {}
>>> #endif
>>> }
>>> if (unlikely(opts->mss)) {
>>>
>>> should do it (though it's still build testing...)
>>
>> I am not opposed to this once we understand what versions are affected
>> by this so that we have some timeline of removing this workaround.
>>
>> Cheers,
>> Nathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-10-31 20:23 [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write() Nathan Chancellor
` (2 preceding siblings ...)
2023-11-02 0:41 ` Palmer Dabbelt
@ 2023-11-03 8:22 ` Christoph Hellwig
2023-11-03 16:53 ` Nathan Chancellor
2023-11-03 16:57 ` Nick Desaulniers
3 siblings, 2 replies; 11+ messages in thread
From: Christoph Hellwig @ 2023-11-03 8:22 UTC (permalink / raw)
To: Nathan Chancellor
Cc: edumazet, davem, dsahern, kuba, pabeni, ndesaulniers, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches
On Tue, Oct 31, 2023 at 01:23:35PM -0700, Nathan Chancellor wrote:
> Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
>
> net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> 663 | }
> | ^
> 1 error generated.
>
> On earlier releases (such as clang-11, the current minimum supported
> version for building the kernel) that do not support C23, this was a
> hard error unconditionally:
>
> net/ipv4/tcp_output.c:663:2: error: expected statement
> }
> ^
> 1 error generated.
>
> Add a semicolon after the label to create an empty statement, which
> resolves the warning or error for all compilers.
Can you please just split the A0 handlig into a separate helper, which
shuld make the whole thing a lot cleaner?
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-11-03 8:22 ` Christoph Hellwig
@ 2023-11-03 16:53 ` Nathan Chancellor
2023-11-06 7:49 ` Christoph Hellwig
2023-11-03 16:57 ` Nick Desaulniers
1 sibling, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2023-11-03 16:53 UTC (permalink / raw)
To: Christoph Hellwig
Cc: edumazet, davem, dsahern, kuba, pabeni, ndesaulniers, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches
Hi Christoph,
On Fri, Nov 03, 2023 at 01:22:05AM -0700, Christoph Hellwig wrote:
> On Tue, Oct 31, 2023 at 01:23:35PM -0700, Nathan Chancellor wrote:
> > Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
> >
> > net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> > 663 | }
> > | ^
> > 1 error generated.
> >
> > On earlier releases (such as clang-11, the current minimum supported
> > version for building the kernel) that do not support C23, this was a
> > hard error unconditionally:
> >
> > net/ipv4/tcp_output.c:663:2: error: expected statement
> > }
> > ^
> > 1 error generated.
> >
> > Add a semicolon after the label to create an empty statement, which
> > resolves the warning or error for all compilers.
>
> Can you please just split the A0 handlig into a separate helper, which
> shuld make the whole thing a lot cleaner?
Is something like this (I think I got all the pointer manipulation
correct...) what you had in mind? I am happy to send that as a v2 if the
netdev folks would prefer it over this small change (along with some
guidance about the function name, if it should be something different).
Cheers,
Nathan
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f558c054cf6e..6f2a5e3bb7b3 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -601,6 +601,43 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb,
}
#endif
+static void process_tcp_ao_options(struct tcp_sock *tp,
+ const struct tcp_request_sock *tcprsk,
+ struct tcp_out_options *opts,
+ struct tcp_key *key, __be32 **ptr)
+{
+#ifdef CONFIG_TCP_AO
+ u8 maclen = tcp_ao_maclen(key->ao_key);
+
+ if (tcprsk) {
+ u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
+
+ *(*ptr)++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
+ (tcprsk->ao_keyid << 8) |
+ (tcprsk->ao_rcv_next));
+ } else {
+ struct tcp_ao_key *rnext_key;
+ struct tcp_ao_info *ao_info;
+
+ ao_info = rcu_dereference_check(tp->ao_info,
+ lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
+ rnext_key = READ_ONCE(ao_info->rnext_key);
+ if (WARN_ON_ONCE(!rnext_key))
+ return;
+ *(*ptr)++ = htonl((TCPOPT_AO << 24) |
+ (tcp_ao_len(key->ao_key) << 16) |
+ (key->ao_key->sndid << 8) |
+ (rnext_key->rcvid));
+ }
+ opts->hash_location = (__u8 *)(*ptr);
+ *ptr += maclen / sizeof(**ptr);
+ if (unlikely(maclen % sizeof(**ptr))) {
+ memset(*ptr, TCPOPT_NOP, sizeof(**ptr));
+ (*ptr)++;
+ }
+#endif
+}
+
/* Write previously computed TCP options to the packet.
*
* Beware: Something in the Internet is very sensitive to the ordering of
@@ -629,37 +666,7 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
opts->hash_location = (__u8 *)ptr;
ptr += 4;
} else if (tcp_key_is_ao(key)) {
-#ifdef CONFIG_TCP_AO
- u8 maclen = tcp_ao_maclen(key->ao_key);
-
- if (tcprsk) {
- u8 aolen = maclen + sizeof(struct tcp_ao_hdr);
-
- *ptr++ = htonl((TCPOPT_AO << 24) | (aolen << 16) |
- (tcprsk->ao_keyid << 8) |
- (tcprsk->ao_rcv_next));
- } else {
- struct tcp_ao_key *rnext_key;
- struct tcp_ao_info *ao_info;
-
- ao_info = rcu_dereference_check(tp->ao_info,
- lockdep_sock_is_held(&tp->inet_conn.icsk_inet.sk));
- rnext_key = READ_ONCE(ao_info->rnext_key);
- if (WARN_ON_ONCE(!rnext_key))
- goto out_ao;
- *ptr++ = htonl((TCPOPT_AO << 24) |
- (tcp_ao_len(key->ao_key) << 16) |
- (key->ao_key->sndid << 8) |
- (rnext_key->rcvid));
- }
- opts->hash_location = (__u8 *)ptr;
- ptr += maclen / sizeof(*ptr);
- if (unlikely(maclen % sizeof(*ptr))) {
- memset(ptr, TCPOPT_NOP, sizeof(*ptr));
- ptr++;
- }
-out_ao:
-#endif
+ process_tcp_ao_options(tp, tcprsk, opts, key, &ptr);
}
if (unlikely(opts->mss)) {
*ptr++ = htonl((TCPOPT_MSS << 24) |
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-11-03 16:53 ` Nathan Chancellor
@ 2023-11-06 7:49 ` Christoph Hellwig
0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2023-11-06 7:49 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Christoph Hellwig, edumazet, davem, dsahern, kuba, pabeni,
ndesaulniers, trix, 0x7f454c46, fruggeri, noureddine, netdev,
linux-kernel, llvm, patches
Normally the kernel keeps the ifdef outside the function body and adds
a stub. But this already looks like a huge imrpovement over the
existing version to me.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH net] tcp: Fix -Wc23-extensions in tcp_options_write()
2023-11-03 8:22 ` Christoph Hellwig
2023-11-03 16:53 ` Nathan Chancellor
@ 2023-11-03 16:57 ` Nick Desaulniers
1 sibling, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2023-11-03 16:57 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Nathan Chancellor, edumazet, davem, dsahern, kuba, pabeni, trix,
0x7f454c46, fruggeri, noureddine, netdev, linux-kernel, llvm,
patches, Thorsten Leemhuis
On Fri, Nov 3, 2023 at 1:22 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Oct 31, 2023 at 01:23:35PM -0700, Nathan Chancellor wrote:
> > Clang warns (or errors with CONFIG_WERROR=y) when CONFIG_TCP_AO is set:
> >
> > net/ipv4/tcp_output.c:663:2: error: label at end of compound statement is a C23 extension [-Werror,-Wc23-extensions]
> > 663 | }
> > | ^
> > 1 error generated.
> >
> > On earlier releases (such as clang-11, the current minimum supported
> > version for building the kernel) that do not support C23, this was a
> > hard error unconditionally:
> >
> > net/ipv4/tcp_output.c:663:2: error: expected statement
> > }
> > ^
> > 1 error generated.
> >
> > Add a semicolon after the label to create an empty statement, which
> > resolves the warning or error for all compilers.
>
> Can you please just split the A0 handlig into a separate helper, which
> shuld make the whole thing a lot cleaner?
Just a note; mainline is currently red over this for us since
1e03d32bea8e spent all of ~3 days in linux-next before getting merged
into mainline.
Whatever the fix is, it would be great to get it into mainline ASAP.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread