> -----Original Message-----
> From: Paolo Abeni <pabeni@xxxxxxxxxx>
> Sent: Monday, July 14, 2025 3:34 PM
> To: Chia-Yu Chang (Nokia) <chia-yu.chang@xxxxxxxxxxxxxxxxxxx>; edumazet@xxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; corbet@xxxxxxx; horms@xxxxxxxxxx; dsahern@xxxxxxxxxx; kuniyu@xxxxxxxxxx; bpf@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; dave.taht@xxxxxxxxx; jhs@xxxxxxxxxxxx; kuba@xxxxxxxxxx; stephen@xxxxxxxxxxxxxxxxxx; xiyou.wangcong@xxxxxxxxx; jiri@xxxxxxxxxxx; davem@xxxxxxxxxxxxx; andrew+netdev@xxxxxxx; donald.hunter@xxxxxxxxx; ast@xxxxxxxxxxx; liuhangbin@xxxxxxxxx; shuah@xxxxxxxxxx; linux-kselftest@xxxxxxxxxxxxxxx; ij@xxxxxxxxxx; ncardwell@xxxxxxxxxx; Koen De Schepper (Nokia) <koen.de_schepper@xxxxxxxxxxxxxxxxxxx>; g.white@xxxxxxxxxxxxx; ingemar.s.johansson@xxxxxxxxxxxx; mirja.kuehlewind@xxxxxxxxxxxx; cheshire@xxxxxxxxx; rs.ietf@xxxxxx; Jason_Livingood@xxxxxxxxxxx; vidhi_goel@xxxxxxxxx
> Subject: Re: [PATCH v12 net-next 09/15] tcp: accecn: AccECN needs to know delivered bytes
>
>
> CAUTION: This is an external email. Please be very careful when clicking links or opening attachments. See the URL nok.it/ext for additional information.
>
>
>
> On 7/4/25 10:53 AM, chia-yu.chang@xxxxxxxxxxxxxxxxxxx wrote:
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index
> > eea790295e54..f7d7649612a2 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -1050,6 +1050,7 @@ struct tcp_sacktag_state {
> > u64 last_sackt;
> > u32 reord;
> > u32 sack_delivered;
> > + u32 delivered_bytes;
>
> Explicitly mentioning in the commit message that the above fills a 4 bytes hole could be helpful for reviewers.
>
Hi Paolo,
Just want to ask to clarify on "the above fills a 4 bytes hole".
Now I see if I move the delivered_bytes to the end of this struct, the pahole results of ARM32 bit compilation:
[BEFORE PATCH]
struct tcp_sacktag_state {
u64 first_sackt; /* 0 8 */
u64 last_sackt; /* 8 8 */
u32 reord; /* 16 4 */
u32 sack_delivered; /* 20 4 */
int flag; /* 24 4 */
unsigned int mss_now; /* 28 4 */
struct rate_sample * rate; /* 32 4 */
/* size: 40, cachelines: 1, members: 7 */
/* padding: 4 */
/* last cacheline: 40 bytes */
};
[AFTER PATCH]
struct tcp_sacktag_state {
u64 first_sackt; /* 0 8 */
u64 last_sackt; /* 8 8 */
u32 reord; /* 16 4 */
u32 sack_delivered; /* 20 4 */
int flag; /* 24 4 */
unsigned int mss_now; /* 28 4 */
struct rate_sample * rate; /* 32 4 */
u32 delivered_bytes; /* 36 4 */
/* size: 40, cachelines: 1, members: 8 */
/* last cacheline: 40 bytes */
};
And the 64 bit results are:
[BEFORE PATCH]
struct tcp_sacktag_state {
u64 first_sackt; /* 0 8 */
u64 last_sackt; /* 8 8 */
u32 reord; /* 16 4 */
u32 sack_delivered; /* 20 4 */
int flag; /* 24 4 */
unsigned int mss_now; /* 28 4 */
struct rate_sample * rate; /* 32 8 */
/* size: 40, cachelines: 1, members: 7 */
/* last cacheline: 40 bytes */
};
[AFTER PATCH]
struct tcp_sacktag_state {
u64 first_sackt; /* 0 8 */
u64 last_sackt; /* 8 8 */
u32 reord; /* 16 4 */
u32 sack_delivered; /* 20 4 */
int flag; /* 24 4 */
unsigned int mss_now; /* 28 4 */
struct rate_sample * rate; /* 32 8 */
u32 delivered_bytes; /* 40 4 */
/* size: 48, cachelines: 1, members: 8 */
/* padding: 4 */
/* last cacheline: 48 bytes */
};
I see this patch does not create any extra hole and it can reuse the existing padding 4 Bytes for ARM32 architecture.
Does it match when you mentioned?
BRs,
Chia-Yu
