On 4/22/25 5:35 PM, [email protected] wrote:
> @@ -298,6 +298,9 @@ struct tcp_sock {
> u32 snd_up; /* Urgent pointer */
> 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 */
> + u8 received_ce_pending:4, /* Not yet transmit cnt of received_ce */
> + unused2:4;
AFAICS this uses a 4 bytes hole present prior to this patch after
"rcv_wnd", leaving a 3 bytes hole after 'unused2'. Possibly should be
worth mentioning the hole presence.
@Eric: would it make sense use this hole for
'noneagle'/'rate_app_limited' and shrink the 'tcp_sock_write_txrx' group
a bit?
[...]
> @@ -5095,7 +5097,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, 92 +
> 4);
> + CACHELINE_ASSERT_GROUP_SIZE(struct tcp_sock, tcp_sock_write_txrx, 97 +
> 7);
Really? I *think* the change here should not move the cacheline end
around, due to holes. Could you please include the relevant pahole
(trimmed) output prior to this patch and after in the commit message?
[...]
> @@ -384,17 +387,16 @@ static void tcp_data_ecn_check(struct sock *sk, const
> struct sk_buff *skb)
> if (tcp_ca_needs_ecn(sk))
> tcp_ca_event(sk, CA_EVENT_ECN_IS_CE);
>
> - if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
> + if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR) &&
> + tcp_ecn_mode_rfc3168(tp)) {
> /* Better not delay acks, sender can have a very low
> cwnd */
> tcp_enter_quickack_mode(sk, 2);
> tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
> }
> - tp->ecn_flags |= TCP_ECN_SEEN;
At this point is not entirely clear to me why the removal of the above
line is needed/correct.
[...]
> @@ -4056,6 +4118,11 @@ static int tcp_ack(struct sock *sk, const struct
> sk_buff *skb, int flag)
>
> tcp_rack_update_reo_wnd(sk, &rs);
>
> + if (tcp_ecn_mode_accecn(tp))
> + ecn_count = tcp_accecn_process(sk, skb,
> + tp->delivered - delivered,
> + &flag);
AFAICS the above could set FLAG_ECE in flags, menaning the previous
tcp_clean_rtx_queue() will run with such flag cleared and the later
function checking such flag will not. I wondering if this inconsistency
could cause problems?
/P
