| Message ID | 20240426065143.4667-4-nbd@nbd.name (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add TCP fraglist GRO support | expand |
On Fri, Apr 26, 2024 at 8:51 AM Felix Fietkau <nbd@nbd.name> wrote: > > This implements fraglist GRO similar to how it's handled in UDP, however > no functional changes are added yet. The next change adds a heuristic for > using fraglist GRO instead of regular GRO. > > Signed-off-by: Felix Fietkau <nbd@nbd.name> > --- > net/ipv4/tcp_offload.c | 22 ++++++++++++++++++++++ > net/ipv6/tcpv6_offload.c | 9 +++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > index c493e95e09a5..ffd6b7a4163a 100644 > --- a/net/ipv4/tcp_offload.c > +++ b/net/ipv4/tcp_offload.c > @@ -332,6 +332,19 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb) > flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq); > flush |= skb_cmp_decrypted(p, skb); > > + if (NAPI_GRO_CB(p)->is_flist) { Please add unlikely() for all NAPI_GRO_CB(p)->is_flist checks added in this patch. > + flush |= (__force int)(flags ^ tcp_flag_word(th2)); > + flush |= skb->ip_summed != p->ip_summed; > + flush |= skb->csum_level != p->csum_level; > + flush |= !pskb_may_pull(skb, skb_gro_offset(skb)); > + flush |= NAPI_GRO_CB(p)->count >= 64; > + > + if (flush || skb_gro_receive_list(p, skb)) > + mss = 1; > + > + goto out_check_final; > + } > + > if (flush || skb_gro_receive(p, skb)) { > mss = 1; > goto out_check_final; > @@ -398,6 +411,15 @@ INDIRECT_CALLABLE_SCOPE int tcp4_gro_complete(struct sk_buff *skb, int thoff) > const struct iphdr *iph = ip_hdr(skb); > struct tcphdr *th = tcp_hdr(skb); > > + if (NAPI_GRO_CB(skb)->is_flist) { > + skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST | SKB_GSO_TCPV4; > + skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count; > + > + __skb_incr_checksum_unnecessary(skb); > + > + return 0; > + } > + > th->check = ~tcp_v4_check(skb->len - thoff, iph->saddr, > iph->daddr, 0); > > diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c > index b3b8e1f6b92a..c97d55cf036f 100644 > --- a/net/ipv6/tcpv6_offload.c > +++ b/net/ipv6/tcpv6_offload.c > @@ -32,6 +32,15 @@ INDIRECT_CALLABLE_SCOPE int tcp6_gro_complete(struct sk_buff *skb, int thoff) > const struct ipv6hdr *iph = ipv6_hdr(skb); > struct tcphdr *th = tcp_hdr(skb); > > + if (NAPI_GRO_CB(skb)->is_flist) { > + skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST | SKB_GSO_TCPV6; > + skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count; > + > + __skb_incr_checksum_unnecessary(skb); > + > + return 0; > + } > + > th->check = ~tcp_v6_check(skb->len - thoff, &iph->saddr, > &iph->daddr, 0); > skb_shinfo(skb)->gso_type |= SKB_GSO_TCPV6; > -- > 2.44.0 >
On Fri, 2024-04-26 at 08:51 +0200, Felix Fietkau wrote: > This implements fraglist GRO similar to how it's handled in UDP, however > no functional changes are added yet. The next change adds a heuristic for > using fraglist GRO instead of regular GRO. > > Signed-off-by: Felix Fietkau <nbd@nbd.name> > --- > net/ipv4/tcp_offload.c | 22 ++++++++++++++++++++++ > net/ipv6/tcpv6_offload.c | 9 +++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c > index c493e95e09a5..ffd6b7a4163a 100644 > --- a/net/ipv4/tcp_offload.c > +++ b/net/ipv4/tcp_offload.c > @@ -332,6 +332,19 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb) > flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq); > flush |= skb_cmp_decrypted(p, skb); > > + if (NAPI_GRO_CB(p)->is_flist) { > + flush |= (__force int)(flags ^ tcp_flag_word(th2)); > + flush |= skb->ip_summed != p->ip_summed; > + flush |= skb->csum_level != p->csum_level; > + flush |= !pskb_may_pull(skb, skb_gro_offset(skb)); I'm sorry, I'm lagging behind. I think the TCP flags handling here is correct - preserving the original ones should work. The question a made WRT 2 above checks being non necessary/redundant: flush |= (__force int)(flags ^ tcp_flag_word(th2)); flush |= !pskb_may_pull(skb, skb_gro_offset(skb)); still stands, I think. Thanks, Paolo
On 26.04.24 10:21, Paolo Abeni wrote: > On Fri, 2024-04-26 at 08:51 +0200, Felix Fietkau wrote: >> This implements fraglist GRO similar to how it's handled in UDP, however >> no functional changes are added yet. The next change adds a heuristic for >> using fraglist GRO instead of regular GRO. >> >> Signed-off-by: Felix Fietkau <nbd@nbd.name> >> --- >> net/ipv4/tcp_offload.c | 22 ++++++++++++++++++++++ >> net/ipv6/tcpv6_offload.c | 9 +++++++++ >> 2 files changed, 31 insertions(+) >> >> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c >> index c493e95e09a5..ffd6b7a4163a 100644 >> --- a/net/ipv4/tcp_offload.c >> +++ b/net/ipv4/tcp_offload.c >> @@ -332,6 +332,19 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb) >> flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq); >> flush |= skb_cmp_decrypted(p, skb); >> >> + if (NAPI_GRO_CB(p)->is_flist) { >> + flush |= (__force int)(flags ^ tcp_flag_word(th2)); >> + flush |= skb->ip_summed != p->ip_summed; >> + flush |= skb->csum_level != p->csum_level; >> + flush |= !pskb_may_pull(skb, skb_gro_offset(skb)); > > I'm sorry, I'm lagging behind. I think the TCP flags handling here is > correct - preserving the original ones should work. > > The question a made WRT 2 above checks being non necessary/redundant: > > flush |= (__force int)(flags ^ tcp_flag_word(th2)); This one is not redundant, because the earlier flags check includes this part: & ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH)) > flush |= !pskb_may_pull(skb, skb_gro_offset(skb)); This one looks like a redundant leftover, I will remove it in the next version. Thanks, - Felix
On 26.04.24 09:47, Eric Dumazet wrote: > On Fri, Apr 26, 2024 at 8:51 AM Felix Fietkau <nbd@nbd.name> wrote: >> >> This implements fraglist GRO similar to how it's handled in UDP, however >> no functional changes are added yet. The next change adds a heuristic for >> using fraglist GRO instead of regular GRO. >> >> Signed-off-by: Felix Fietkau <nbd@nbd.name> >> --- >> net/ipv4/tcp_offload.c | 22 ++++++++++++++++++++++ >> net/ipv6/tcpv6_offload.c | 9 +++++++++ >> 2 files changed, 31 insertions(+) >> >> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c >> index c493e95e09a5..ffd6b7a4163a 100644 >> --- a/net/ipv4/tcp_offload.c >> +++ b/net/ipv4/tcp_offload.c >> @@ -332,6 +332,19 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb) >> flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq); >> flush |= skb_cmp_decrypted(p, skb); >> >> + if (NAPI_GRO_CB(p)->is_flist) { > > > Please add unlikely() for all NAPI_GRO_CB(p)->is_flist checks added in > this patch. Will do, thanks. - Felix
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c index c493e95e09a5..ffd6b7a4163a 100644 --- a/net/ipv4/tcp_offload.c +++ b/net/ipv4/tcp_offload.c @@ -332,6 +332,19 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb) flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq); flush |= skb_cmp_decrypted(p, skb); + if (NAPI_GRO_CB(p)->is_flist) { + flush |= (__force int)(flags ^ tcp_flag_word(th2)); + flush |= skb->ip_summed != p->ip_summed; + flush |= skb->csum_level != p->csum_level; + flush |= !pskb_may_pull(skb, skb_gro_offset(skb)); + flush |= NAPI_GRO_CB(p)->count >= 64; + + if (flush || skb_gro_receive_list(p, skb)) + mss = 1; + + goto out_check_final; + } + if (flush || skb_gro_receive(p, skb)) { mss = 1; goto out_check_final; @@ -398,6 +411,15 @@ INDIRECT_CALLABLE_SCOPE int tcp4_gro_complete(struct sk_buff *skb, int thoff) const struct iphdr *iph = ip_hdr(skb); struct tcphdr *th = tcp_hdr(skb); + if (NAPI_GRO_CB(skb)->is_flist) { + skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST | SKB_GSO_TCPV4; + skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count; + + __skb_incr_checksum_unnecessary(skb); + + return 0; + } + th->check = ~tcp_v4_check(skb->len - thoff, iph->saddr, iph->daddr, 0); diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c index b3b8e1f6b92a..c97d55cf036f 100644 --- a/net/ipv6/tcpv6_offload.c +++ b/net/ipv6/tcpv6_offload.c @@ -32,6 +32,15 @@ INDIRECT_CALLABLE_SCOPE int tcp6_gro_complete(struct sk_buff *skb, int thoff) const struct ipv6hdr *iph = ipv6_hdr(skb); struct tcphdr *th = tcp_hdr(skb); + if (NAPI_GRO_CB(skb)->is_flist) { + skb_shinfo(skb)->gso_type |= SKB_GSO_FRAGLIST | SKB_GSO_TCPV6; + skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count; + + __skb_incr_checksum_unnecessary(skb); + + return 0; + } + th->check = ~tcp_v6_check(skb->len - thoff, &iph->saddr, &iph->daddr, 0); skb_shinfo(skb)->gso_type |= SKB_GSO_TCPV6;
This implements fraglist GRO similar to how it's handled in UDP, however no functional changes are added yet. The next change adds a heuristic for using fraglist GRO instead of regular GRO. Signed-off-by: Felix Fietkau <nbd@nbd.name> --- net/ipv4/tcp_offload.c | 22 ++++++++++++++++++++++ net/ipv6/tcpv6_offload.c | 9 +++++++++ 2 files changed, 31 insertions(+)