From: Oleksandr Natalenko <oleksandr@natalenko.name>
To: Neal Cardwell <ncardwell@google.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [bbr3] Suspicious use of bbr_param
Date: Tue, 24 Dec 2024 22:04:53 +0100 [thread overview]
Message-ID: <4616579.LvFx2qVVIh@natalenko.name> (raw)
[-- Attachment #1: Type: text/plain, Size: 3071 bytes --]
Hello Neal.
One of my users reports [1] that BBRv3 from [2] cannot be built with LLVM=1 and WERROR=y because of the following warnings:
net/ipv4/tcp_bbr.c:1079:48: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
1079 | if (!bbr->ecn_eligible && bbr_can_use_ecn(sk) &&
| ^
1080 | bbr_param(sk, ecn_factor) &&
| ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1079:48: note: use '&' for a bitwise operation
1079 | if (!bbr->ecn_eligible && bbr_can_use_ecn(sk) &&
| ^~
| &
net/ipv4/tcp_bbr.c:1079:48: note: remove constant to silence this warning
1079 | if (!bbr->ecn_eligible && bbr_can_use_ecn(sk) &&
| ^~
1080 | bbr_param(sk, ecn_factor) &&
| ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1187:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
1187 | bbr->ecn_eligible && bbr_param(sk, ecn_thresh)) {
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1187:24: note: use '&' for a bitwise operation
1187 | bbr->ecn_eligible && bbr_param(sk, ecn_thresh)) {
| ^~
| &
net/ipv4/tcp_bbr.c:1187:24: note: remove constant to silence this warning
1187 | bbr->ecn_eligible && bbr_param(sk, ecn_thresh)) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1385:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
1385 | if (bbr->ecn_in_round && bbr_param(sk, ecn_factor)) {
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
net/ipv4/tcp_bbr.c:1385:24: note: use '&' for a bitwise operation
1385 | if (bbr->ecn_in_round && bbr_param(sk, ecn_factor)) {
| ^~
| &
net/ipv4/tcp_bbr.c:1385:24: note: remove constant to silence this warning
1385 | if (bbr->ecn_in_round && bbr_param(sk, ecn_factor)) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
The usage of bbr_param() with ecn_thresh and ecn_factor here does indeed look suspicious. In both cases, the bbr_param() macro gets evaluated to `static const u32` values, and those get &&'ed in the if statements. The consts are positive, so they do not have any impact in the conditional expressions. FWIW, the sk argument is dropped by the macro altogether, so I'm not sure what was the intention here.
Interestingly, unlike Clang, GCC stays silent.
Could you please comment on this?
Appreciate your time, and looking forward to your reply.
Thank you.
[1] https://codeberg.org/pf-kernel/linux/issues/11
[2] https://github.com/google/bbr
--
Oleksandr Natalenko, MSE
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next reply other threads:[~2024-12-24 21:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-24 21:04 Oleksandr Natalenko [this message]
2024-12-25 12:40 ` [bbr3] Suspicious use of bbr_param Holger Hoffstätte
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4616579.LvFx2qVVIh@natalenko.name \
--to=oleksandr@natalenko.name \
--cc=linux-kernel@vger.kernel.org \
--cc=ncardwell@google.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox