On Mon, Apr 14, 2025 at 03:13:05PM +0200, [email protected]
wrote:
> From: Ilpo Järvinen <[email protected]>
>
> These counters track IP ECN field payload byte sums for all
> arriving (acceptable) packets. The AccECN option (added by
> a later patch in the series) echoes these counters back to
> sender side.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> Signed-off-by: Neal Cardwell <[email protected]>
> Signed-off-by: Chia-Yu Chang <[email protected]>
> ---
> include/linux/tcp.h | 1 +
> include/net/tcp.h | 18 +++++++++++++++++-
> net/ipv4/tcp.c | 3 ++-
> net/ipv4/tcp_input.c | 13 +++++++++----
> net/ipv4/tcp_minisocks.c | 3 ++-
> 5 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index af38fff24aa4..9cbfefd693e3 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -303,6 +303,7 @@ struct tcp_sock {
> u32 delivered; /* Total data packets delivered incl. rexmits */
> u32 delivered_ce; /* Like the above but only ECE marked packets */
> u32 received_ce; /* Like the above but for rcvd CE marked pkts */
> + u32 received_ecn_bytes[3];
> u8 received_ce_pending:4, /* Not yet transmit cnt of received_ce */
> unused2:4;
> u32 app_limited; /* limited until "delivered" reaches this val */
...
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 73f8cc715bff..278990dba721 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -5092,6 +5092,7 @@ static void __init tcp_struct_check(void)
> CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> delivered);
> CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> delivered_ce);
> CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> received_ce);
> + CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> received_ecn_bytes);
> CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> app_limited);
> CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> rcv_wnd);
> CACHELINE_ASSERT_GROUP_MEMBER(struct tcp_sock, tcp_sock_write_txrx,
> rx_opt);
> @@ -5099,7 +5100,7 @@ static void __init tcp_struct_check(void)
> /* 32bit arches with 8byte alignment on u64 fields might need padding
> * before tcp_clock_cache.
> */
> - CACHELINE_ASSERT_GROUP_SIZE(struct tcp_sock, tcp_sock_write_txrx, 97 +
> 7);
> + CACHELINE_ASSERT_GROUP_SIZE(struct tcp_sock, tcp_sock_write_txrx, 109 +
> 3);
Hi Ilpo,
I think that incrementing 97 to 109 is correct, as 12 bytes have been
added to this group.
However, I do not think it is correct to decrement 7 to 3.
On, at least, x86_64, x86_32 and arm64 that decrement does not
cause any problems. (I assume it is also fine without the rest of this
patch).
But on (32-bit) ARM, this causes the assertion to fail.
This is because on ARM an extra 4-byte hole is added just after pred_flags.
And the assertion checks an upper bound on the size of the group.
I assume this extra hole is due to alignment requirements.
In any case on ARM, with this patch applied, pahole shows
the group looking like this:
__u8
__cacheline_group_begin__tcp_sock_write_txrx[0]; /* 1869 0 */
/* XXX 3 bytes hole, try to pack */
__be32 pred_flags; /* 1872 4 */
/* XXX 4 bytes hole, try to pack */
u64 tcp_clock_cache; /* 1880 8 */
u64 tcp_mstamp; /* 1888 8 */
u32 rcv_nxt; /* 1896 4 */
u32 snd_nxt; /* 1900 4 */
u32 snd_una; /* 1904 4 */
u32 window_clamp; /* 1908 4 */
u32 srtt_us; /* 1912 4 */
u32 packets_out; /* 1916 4 */
/* --- cacheline 30 boundary (1920 bytes) --- */
u32 snd_up; /* 1920 4 */
u32 delivered; /* 1924 4 */
u32 delivered_ce; /* 1928 4 */
u32 received_ce; /* 1932 4 */
u32 received_ecn_bytes[3]; /* 1936 12 */
u8 received_ce_pending:4; /* 1948: 0 1 */
u8 unused2:4; /* 1948: 4 1 */
/* XXX 3 bytes hole, try to pack */
u32 app_limited; /* 1952 4 */
u32 rcv_wnd; /* 1956 4 */
struct tcp_options_received rx_opt; /* 1960 24 */
/* --- cacheline 31 boundary (1984 bytes) --- */
u8 nonagle:4; /* 1984: 0 1 */
u8 rate_app_limited:1; /* 1984: 4 1 */
/* XXX 3 bits hole, try to pack */
__u8
__cacheline_group_end__tcp_sock_write_txrx[0]; /* 1985 0 */
So we are now at 3 cache lines. And perhaps it is worth trying to pack
things a bit. Or perhaps that becomes tricky to get right across
different architectures. I didn't explore that.
But, taking the naive approach: with the following update, tcp.c compiles
compiles with allmodconfig on x86_64, x86_32, ARM and arm64 (I did not test
others).
CACHELINE_ASSERT_GROUP_SIZE(struct tcp_sock, tcp_sock_write_txrx, 109 +
7);
For the record, gcc 14.2.0 reports this problem as:
CC net/ipv4/tcp.o
In file included from <command-line>:
In function 'tcp_struct_check',
inlined from 'tcp_init' at net/ipv4/tcp.c:5133:2:
././include/linux/compiler_types.h:557:45: error: call to '__compiletime_assert_
1403' declared with attribute error: BUILD_BUG_ON failed: offsetof(struct
tcp_sock, __cacheline_group_end__tcp_sock_write_txrx) - offsetofend(struct
tcp_sock, __cacheline_group_begin__tcp_sock_write_txrx) > 109 + 3
557 | _compiletime_assert(condition, msg, __compiletime_assert_,
__COUNTER__)
| ^
././include/linux/compiler_types.h:538:25: note: in definition of macro
'__compiletime_assert'
538 | prefix ## suffix();
\
| ^~~~~~
././include/linux/compiler_types.h:557:9: note: in expansion of macro
'_compiletime_assert'
557 | _compiletime_assert(condition, msg, __compiletime_assert_,
__COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro
'compiletime_assert'
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^~~~~~~~~~~~~~~~
./include/linux/cache.h:160:9: note: in expansion of macro 'BUILD_BUG_ON'
160 | BUILD_BUG_ON(offsetof(TYPE, __cacheline_group_end__##GROUP) - \
| ^~~~~~~~~~~~
net/ipv4/tcp.c:5103:9: note: in expansion of macro 'CACHELINE_ASSERT_GROUP_SIZE'
5103 | CACHELINE_ASSERT_GROUP_SIZE(struct tcp_sock,
tcp_sock_write_txrx, 109 + 3);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
...
--
pw-bot: changes-requested
