# \[PATCH\] tcp: fix setting csum\_start in tcp\_gso\_segment - Subsystems: NETWORKING \[GENERAL\], THE REST - Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds - List: netdev - 8 messages, 5 authors - Page: https://ratatoskr.run/netdev/2014/06/4280607/t ## \[PATCH\] tcp: fix setting csum\_start in tcp\_gso\_segment - From: Tom Herbert \ - Date: 2014-06-25 04:03:54 - Message: https://ratatoskr.run/netdev/2014/06/4280607.md ```text Dave Jones reported that a crash is occuring in csum_partial tcp_gso_segment inet_gso_segment ? update_dl_migration skb_mac_gso_segment __skb_gso_segment dev_hard_start_xmit sch_direct_xmit __dev_queue_xmit ? dev_hard_start_xmit dev_queue_xmit ip_finish_output ? ip_output ip_output ip_forward_finish ip_forward ip_rcv_finish ip_rcv __netif_receive_skb_core ? __netif_receive_skb_core ? trace_hardirqs_on __netif_receive_skb netif_receive_skb_internal napi_gro_complete ? napi_gro_complete dev_gro_receive ? dev_gro_receive napi_gro_receive It looks like a likely culprit is that SKB_GSO_CB()->csum_start is not set correctly when doing non-scatter gather. We are using offset as opposed to doffset. Reported-by: Dave Jones Signed-off-by: Tom Herbert --- net/core/skbuff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 9cd5344..c1a3303 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -2993,7 +2993,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, skb_put(nskb, len), len, 0); SKB_GSO_CB(nskb)->csum_start = - skb_headroom(nskb) + offset; + skb_headroom(nskb) + doffset; continue; } -- 2.0.0.526.g5318336 ``` ## Re: \[PATCH\] tcp: fix setting csum\_start in tcp\_gso\_segment - From: Linus Torvalds \ - Date: 2014-06-25 04:17:28 - Message: https://ratatoskr.run/netdev/2014/06/4280609.md ```text On Tue, Jun 24, 2014 at 9:03 PM, Tom Herbert [ref] wrote: > > It looks like a likely culprit is that SKB_GSO_CB()->csum_start is > not set correctly when doing non-scatter gather. We are using > offset as opposed to doffset. > > Reported-by: Dave Jones DaveJ, I think you triggered this in five minutes on your box, and I don't recall seeing anybody else reporting the oops (and google doesn't find anything in the last month). So it's presumably somewhat hw-specific. Does this fix the problem? Linus ``` ## Re: \[PATCH\] tcp: fix setting csum\_start in tcp\_gso\_segment - From: Eric Dumazet \ - Date: 2014-06-25 07:19:12 - Message: https://ratatoskr.run/netdev/2014/06/4280693.md ```text On Tue, 2014-06-24 at 21:03 -0700, Tom Herbert wrote: > > It looks like a likely culprit is that SKB_GSO_CB()->csum_start is > not set correctly when doing non-scatter gather. We are using > offset as opposed to doffset. > > Reported-by: Dave Jones > Signed-off-by: Tom Herbert > --- > net/core/skbuff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 9cd5344..c1a3303 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -2993,7 +2993,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, > skb_put(nskb, len), > len, 0); > SKB_GSO_CB(nskb)->csum_start = > - skb_headroom(nskb) + offset; > + skb_headroom(nskb) + doffset; > continue; > } > Yes, seems an obvious typo, but please change patch title. This is not "tcp: fix setting csum_start in tcp_gso_segment" Maybe "net: fix setting csum_start in skb_segment()" ``` ## Re: \[PATCH\] tcp: fix setting csum\_start in tcp\_gso\_segment - From: Dave Jones \ - Date: 2014-06-25 14:11:54 - Message: https://ratatoskr.run/netdev/2014/06/4281035.md ```text On Tue, Jun 24, 2014 at 09:17:25PM -0700, Linus Torvalds wrote: > On Tue, Jun 24, 2014 at 9:03 PM, Tom Herbert [ref] wrote: > > > > It looks like a likely culprit is that SKB_GSO_CB()->csum_start is > > not set correctly when doing non-scatter gather. We are using > > offset as opposed to doffset. > > > > Reported-by: Dave Jones [ref] > > DaveJ, I think you triggered this in five minutes on your box, and I > don't recall seeing anybody else reporting the oops (and google > doesn't find anything in the last month). So it's presumably somewhat > hw-specific. Does this fix the problem? It's survived routing ~1GB of packets overnight, so I'd call this good. thanks Tom. Dave ``` ## Re: \[PATCH\] tcp: fix setting csum\_start in tcp\_gso\_segment - From: David Miller \ - Date: 2014-06-25 18:53:04 - Message: https://ratatoskr.run/netdev/2014/06/4281282.md ```text From: Dave Jones Date: Wed, 25 Jun 2014 10:10:52 -0400 > On Tue, Jun 24, 2014 at 09:17:25PM -0700, Linus Torvalds wrote: > > On Tue, Jun 24, 2014 at 9:03 PM, Tom Herbert [ref] wrote: > > > > > > It looks like a likely culprit is that SKB_GSO_CB()->csum_start is > > > not set correctly when doing non-scatter gather. We are using > > > offset as opposed to doffset. > > > > > > Reported-by: Dave Jones [ref] > > > > DaveJ, I think you triggered this in five minutes on your box, and I > > don't recall seeing anybody else reporting the oops (and google > > doesn't find anything in the last month). So it's presumably somewhat > > hw-specific. Does this fix the problem? > > It's survived routing ~1GB of packets overnight, so I'd call this good. Tom, please adjust the Subject line as suggested by Eric Dumazet and add a Tested-by: for Dave. Thanks! ``` ## \[PATCH\] net: fix setting csum\_start in skb\_segment() - From: Eric Dumazet \ - Date: 2014-06-25 19:51:08 - Message: https://ratatoskr.run/netdev/2014/06/4281315.md ```text From: Tom Herbert Dave Jones reported that a crash is occurring in csum_partial tcp_gso_segment inet_gso_segment ? update_dl_migration skb_mac_gso_segment __skb_gso_segment dev_hard_start_xmit sch_direct_xmit __dev_queue_xmit ? dev_hard_start_xmit dev_queue_xmit ip_finish_output ? ip_output ip_output ip_forward_finish ip_forward ip_rcv_finish ip_rcv __netif_receive_skb_core ? __netif_receive_skb_core ? trace_hardirqs_on __netif_receive_skb netif_receive_skb_internal napi_gro_complete ? napi_gro_complete dev_gro_receive ? dev_gro_receive napi_gro_receive It looks like a likely culprit is that SKB_GSO_CB()->csum_start is not set correctly when doing non-scatter gather. We are using offset as opposed to doffset. Reported-by: Dave Jones Tested-by: Dave Jones Signed-off-by: Tom Herbert Signed-off-by: Eric Dumazet Fixes: 7e2b10c1e52ca ("net: Support for multiple checksums with gso") --- net/core/skbuff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 9cd5344fad73..c1a33033cbe2 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -2993,7 +2993,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, skb_put(nskb, len), len, 0); SKB_GSO_CB(nskb)->csum_start = - skb_headroom(nskb) + offset; + skb_headroom(nskb) + doffset; continue; } ``` ## Re: \[PATCH\] net: fix setting csum\_start in skb\_segment() - From: Tom Herbert \ - Date: 2014-06-26 02:15:55 - Message: https://ratatoskr.run/netdev/2014/06/4281485.md ```text On Wed, Jun 25, 2014 at 12:51 PM, Eric Dumazet [ref] wrote: > From: Tom Herbert > > Dave Jones reported that a crash is occurring in > > csum_partial > tcp_gso_segment > inet_gso_segment > ? update_dl_migration > skb_mac_gso_segment > __skb_gso_segment > dev_hard_start_xmit > sch_direct_xmit > __dev_queue_xmit > ? dev_hard_start_xmit > dev_queue_xmit > ip_finish_output > ? ip_output > ip_output > ip_forward_finish > ip_forward > ip_rcv_finish > ip_rcv > __netif_receive_skb_core > ? __netif_receive_skb_core > ? trace_hardirqs_on > __netif_receive_skb > netif_receive_skb_internal > napi_gro_complete > ? napi_gro_complete > dev_gro_receive > ? dev_gro_receive > napi_gro_receive > > It looks like a likely culprit is that SKB_GSO_CB()->csum_start is > not set correctly when doing non-scatter gather. We are using > offset as opposed to doffset. > Acked-by: Tom Herbert > Reported-by: Dave Jones > Tested-by: Dave Jones > Signed-off-by: Tom Herbert > Signed-off-by: Eric Dumazet > Fixes: 7e2b10c1e52ca ("net: Support for multiple checksums with gso") > --- > net/core/skbuff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 9cd5344fad73..c1a33033cbe2 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -2993,7 +2993,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb, > skb_put(nskb, len), > len, 0); > SKB_GSO_CB(nskb)->csum_start = > - skb_headroom(nskb) + offset; > + skb_headroom(nskb) + doffset; > continue; > } > > > ``` ## Re: \[PATCH\] net: fix setting csum\_start in skb\_segment() - From: David Miller \ - Date: 2014-06-26 03:46:15 - Message: https://ratatoskr.run/netdev/2014/06/4281502.md ```text From: Eric Dumazet Date: Wed, 25 Jun 2014 12:51:01 -0700 > From: Tom Herbert > > Dave Jones reported that a crash is occurring in ... > It looks like a likely culprit is that SKB_GSO_CB()->csum_start is > not set correctly when doing non-scatter gather. We are using > offset as opposed to doffset. > > Reported-by: Dave Jones > Tested-by: Dave Jones > Signed-off-by: Tom Herbert > Signed-off-by: Eric Dumazet > Fixes: 7e2b10c1e52ca ("net: Support for multiple checksums with gso") Applied, thanks everyone. ```