| Message ID | cde3385c115ddf64fe14725f57d88a2a089f23e1.1635977622.git.cdleonard@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tcp: Use BIT() for OPTION_* constants | expand |
On 11/3/21 3:17 PM, Leonard Crestez wrote: > Extending these flags using the existing (1 << x) pattern triggers > complaints from checkpatch. Instead of ignoring checkpatch modify the > existing values to use BIT(x) style in a separate commit. > > Signed-off-by: Leonard Crestez <cdleonard@gmail.com> > Yes, I guess checkpatch does not know that we currently use at most 16 bits :) u16 options = opts->options; Anyway, this seems fine. Reviewed-by: Eric Dumazet <edumazet@google.com>
From: Eric Dumazet > Sent: 03 November 2021 22:50 > > On 11/3/21 3:17 PM, Leonard Crestez wrote: > > Extending these flags using the existing (1 << x) pattern triggers > > complaints from checkpatch. Instead of ignoring checkpatch modify the > > existing values to use BIT(x) style in a separate commit. > > > > Signed-off-by: Leonard Crestez <cdleonard@gmail.com> > > > > Yes, I guess checkpatch does not know that we currently use at most 16 bits :) > > u16 options = opts->options; > > Anyway, this seems fine. Doesn't BIT() have a nasty habit of generating 64bit constants that just cause a different set of issues when inverted? It may be safe here - but who knows. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On 11/4/21 2:17 AM, David Laight wrote: > From: Eric Dumazet >> Sent: 03 November 2021 22:50 >> >> On 11/3/21 3:17 PM, Leonard Crestez wrote: >>> Extending these flags using the existing (1 << x) pattern triggers >>> complaints from checkpatch. Instead of ignoring checkpatch modify the >>> existing values to use BIT(x) style in a separate commit. >>> >>> Signed-off-by: Leonard Crestez <cdleonard@gmail.com> >>> >> >> Yes, I guess checkpatch does not know that we currently use at most 16 bits :) >> >> u16 options = opts->options; >> >> Anyway, this seems fine. > > Doesn't BIT() have a nasty habit of generating 64bit constants > that just cause a different set of issues when inverted? > It may be safe here - but who knows. BIT() does not use/force 64bit constants, plain "unsigned long" ones. Really this patch looks a nop to me.
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 6867e5db3e35..96f16386f50e 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -406,17 +406,17 @@ static void tcp_init_nondata_skb(struct sk_buff *skb, u32 seq, u8 flags) static inline bool tcp_urg_mode(const struct tcp_sock *tp) { return tp->snd_una != tp->snd_up; } -#define OPTION_SACK_ADVERTISE (1 << 0) -#define OPTION_TS (1 << 1) -#define OPTION_MD5 (1 << 2) -#define OPTION_WSCALE (1 << 3) -#define OPTION_FAST_OPEN_COOKIE (1 << 8) -#define OPTION_SMC (1 << 9) -#define OPTION_MPTCP (1 << 10) +#define OPTION_SACK_ADVERTISE BIT(0) +#define OPTION_TS BIT(1) +#define OPTION_MD5 BIT(2) +#define OPTION_WSCALE BIT(3) +#define OPTION_FAST_OPEN_COOKIE BIT(8) +#define OPTION_SMC BIT(9) +#define OPTION_MPTCP BIT(10) static void smc_options_write(__be32 *ptr, u16 *options) { #if IS_ENABLED(CONFIG_SMC) if (static_branch_unlikely(&tcp_have_smc)) {
Extending these flags using the existing (1 << x) pattern triggers complaints from checkpatch. Instead of ignoring checkpatch modify the existing values to use BIT(x) style in a separate commit. Signed-off-by: Leonard Crestez <cdleonard@gmail.com> --- This was split away from a longer series implementing RFC5925 Authentication Option for TCP. Link: https://lore.kernel.org/netdev/dc9dca0006fa1b586da44dcd54e29eb4300fe773.1635784253.git.cdleonard@gmail.com/ net/ipv4/tcp_output.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) base-commit: d4a07dc5ac34528f292a4f328cf3c65aba312e1b