On Fri, Apr 18, 2025 at 01:00:24AM +0200, chia-yu.chang@xxxxxxxxxxxxxxxxxxx wrote:
> From: Ilpo Järvinen <ij@xxxxxxxxxx>
>
> Instead of sending the option in every ACK, limit sending to
> those ACKs where the option is necessary:
> - Handshake
> - "Change-triggered ACK" + the ACK following it. The
> 2nd ACK is necessary to unambiguously indicate which
> of the ECN byte counters in increasing. The first
> ACK has two counters increasing due to the ecnfield
> edge.
> - ACKs with CE to allow CEP delta validations to take
> advantage of the option.
> - Force option to be sent every at least once per 2^22
> bytes. The check is done using the bit edges of the
> byte counters (avoids need for extra variables).
> - AccECN option beacon to send a few times per RTT even if
> nothing in the ECN state requires that. The default is 3
> times per RTT, and its period can be set via
> sysctl_tcp_ecn_option_beacon.
>
> Signed-off-by: Ilpo Järvinen <ij@xxxxxxxxxx>
> Co-developed-by: Chia-Yu Chang <chia-yu.chang@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@xxxxxxxxxxxxxxxxxxx>
> ---
> include/linux/tcp.h | 3 +++
> include/net/netns/ipv4.h | 1 +
> include/net/tcp.h | 1 +
> net/ipv4/sysctl_net_ipv4.c | 9 ++++++++
> net/ipv4/tcp.c | 5 ++++-
> net/ipv4/tcp_input.c | 36 +++++++++++++++++++++++++++++++-
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_minisocks.c | 2 ++
> net/ipv4/tcp_output.c | 42 ++++++++++++++++++++++++++++++--------
> 9 files changed, 90 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 0e032d9631ac..9619524d8901 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -309,7 +309,10 @@ struct tcp_sock {
> u8 received_ce_pending:4, /* Not yet transmit cnt of received_ce */
> unused2:4;
> u8 accecn_minlen:2,/* Minimum length of AccECN option sent */
> + prev_ecnfield:2,/* ECN bits from the previous segment */
> + accecn_opt_demand:2,/* Demand AccECN option for n next ACKs */
> est_ecnfield:2;/* ECN field for AccECN delivered estimates */
> + u64 accecn_opt_tstamp; /* Last AccECN option sent timestamp */
> u32 app_limited; /* limited until "delivered" reaches this val */
> u32 rcv_wnd; /* Current receiver window */
> /*
...
> @@ -5113,7 +5116,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, 122 + 6);
> + CACHELINE_ASSERT_GROUP_SIZE(struct tcp_sock, tcp_sock_write_txrx, 130 + 6);
Hi,
While this seems find on x86_64, x86_32 and arm64, it does not seem correct
on ARM (32-bit).
This is because the existing two byte hole after est_ecnfield grows
to 6 bytes. I assume this is because of alignment requirements.
But in any case, the result is that the overall group size increases
by 12 bytes rather than 8.
I believe that you can avoid the hole growing, and thus limit the overall
increase in size of the group to 12 bytes, by placing accecn_opt_tstamp
elsewhere, e.g. after app_limited rather than before it.
You can exercise this by cross compiling for ARM and examining
the structure layout using pahole.
Cross compilers available from [1] should be able to do that something
like this:
PATH=".../gcc-14.2.0-nolibc/arm-linux-gnueabi/bin:$PATH"
export ARCH=arm
export CROSS_COMPILE=arm-linux-gnueabi-
make allmodconfig
echo "CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y" >> .config
yes "" | make oldconfig
make net/ipv4/tcp.o
[1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/14.2.0/
