[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89i+9USaOthY3yaJPT-cbfAcP0re2bbGbWU7SqOSYEW2CMw@mail.gmail.com>
Date: Thu, 7 Nov 2024 13:40:58 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: chia-yu.chang@...ia-bell-labs.com
Cc: netdev@...r.kernel.org, dsahern@...il.com, davem@...emloft.net,
dsahern@...nel.org, pabeni@...hat.com, joel.granados@...nel.org,
kuba@...nel.org, andrew+netdev@...n.ch, horms@...nel.org, pablo@...filter.org,
kadlec@...filter.org, netfilter-devel@...r.kernel.org, coreteam@...filter.org,
ij@...nel.org, ncardwell@...gle.com, koen.de_schepper@...ia-bell-labs.com,
g.white@...lelabs.com, ingemar.s.johansson@...csson.com,
mirja.kuehlewind@...csson.com, cheshire@...le.com, rs.ietf@....at,
Jason_Livingood@...cast.com, vidhi_goel@...le.com
Subject: Re: [PATCH v5 net-next 09/13] gro: prevent ACE field corruption &
better AccECN handling
On Tue, Nov 5, 2024 at 11:07 AM <chia-yu.chang@...ia-bell-labs.com> wrote:
>
> From: Ilpo Järvinen <ij@...nel.org>
>
> 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@...nel.org>
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
> ---
> 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));
> 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;
> }
> EXPORT_SYMBOL(tcp_gro_complete);
>
I do not really understand this patch. How a GRO engine can know which
ECN variant the peers are using ?
SKB_GSO_TCP_ECN is also used from other points, what is your plan ?
git grep -n SKB_GSO_TCP_ECN
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:3888:
skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1291:
skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:1312:
skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
include/linux/netdevice.h:5061: BUILD_BUG_ON(SKB_GSO_TCP_ECN !=
(NETIF_F_TSO_ECN >> NETIF_F_GSO_SHIFT));
include/linux/skbuff.h:664: SKB_GSO_TCP_ECN = 1 << 2,
include/linux/virtio_net.h:88: gso_type |= SKB_GSO_TCP_ECN;
include/linux/virtio_net.h:161: switch (gso_type & ~SKB_GSO_TCP_ECN) {
include/linux/virtio_net.h:226: if (sinfo->gso_type & SKB_GSO_TCP_ECN)
net/ipv4/tcp_offload.c:404: shinfo->gso_type |= SKB_GSO_TCP_ECN;
net/ipv4/tcp_output.c:389:
skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
Powered by blists - more mailing lists