On 10/21/24 23:59, chia-yu.chang@xxxxxxxxxxxxxxxxxxx wrote: > From: Ilpo Järvinen <ij@xxxxxxxxxx> > > There are important differences in how the CWR field behaves > in RFC3168 and AccECN. With AccECN, CWR flag is part of the > ACE counter and its changes are important so adjust the flags > changed mask accordingly. > > Also, if CWR is there, set the Accurate ECN GSO flag to avoid > corrupting CWR flag somewhere. > > Signed-off-by: Ilpo Järvinen <ij@xxxxxxxxxx> > Signed-off-by: Chia-Yu Chang <chia-yu.chang@xxxxxxxxxxxxxxxxxxx> > --- > net/ipv4/tcp_offload.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > index 0b05f30e9e5f..f59762d88c38 100644 > --- a/net/ipv4/tcp_offload.c > +++ b/net/ipv4/tcp_offload.c > @@ -329,7 +329,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb, > th2 = tcp_hdr(p); > flush = (__force int)(flags & TCP_FLAG_CWR); > flush |= (__force int)((flags ^ tcp_flag_word(th2)) & > - ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH)); > + ~(TCP_FLAG_FIN | TCP_FLAG_PSH)); If I read correctly, if the peer is using RFC3168 and TSO_ECN, GRO will now pump into the stack twice the number of packets it was doing prior to this patch, am I correct? That is likely causing measurable performance regressions. > flush |= (__force int)(th->ack_seq ^ th2->ack_seq); > for (i = sizeof(*th); i < thlen; i += 4) > flush |= *(u32 *)((u8 *)th + i) ^ > @@ -405,7 +405,7 @@ void tcp_gro_complete(struct sk_buff *skb) > shinfo->gso_segs = NAPI_GRO_CB(skb)->count; > > if (th->cwr) > - shinfo->gso_type |= SKB_GSO_TCP_ECN; > + shinfo->gso_type |= SKB_GSO_TCP_ACCECN; If this packet is forwarded, it will not leverage TSO anymore - with current H/W. I think we need a way to enable this feature conditionally, but I fear another sysctl will be ugly and the additional conditionals will not be good for GRO. Smarter suggestions welcome ;) Cheers, Paolo > } > EXPORT_SYMBOL(tcp_gro_complete); >
