* TCP output handling bug ?
@ 2014-08-21 18:02 Alan Cox
2014-08-22 0:24 ` Stephen Hemminger
0 siblings, 1 reply; 7+ messages in thread
From: Alan Cox @ 2014-08-21 18:02 UTC (permalink / raw)
To: netdev
tcp_send_syn_data:
TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN;
TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH);
the reporter has a point 8)
https://bugzilla.kernel.org/show_bug.cgi?id=82101
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: TCP output handling bug ? 2014-08-21 18:02 TCP output handling bug ? Alan Cox @ 2014-08-22 0:24 ` Stephen Hemminger 2014-08-22 2:13 ` Dave Jones 0 siblings, 1 reply; 7+ messages in thread From: Stephen Hemminger @ 2014-08-22 0:24 UTC (permalink / raw) To: Alan Cox; +Cc: netdev On Thu, 21 Aug 2014 19:02:08 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > > tcp_send_syn_data: > > TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; > TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH); > > the reporter has a point 8) > > > https://bugzilla.kernel.org/show_bug.cgi?id=82101 > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html I wonder if covertity or smatch could be smart enough to catch this kind of bug? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TCP output handling bug ? 2014-08-22 0:24 ` Stephen Hemminger @ 2014-08-22 2:13 ` Dave Jones 2014-08-22 13:56 ` Eric Dumazet 0 siblings, 1 reply; 7+ messages in thread From: Dave Jones @ 2014-08-22 2:13 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Alan Cox, netdev On Thu, Aug 21, 2014 at 05:24:45PM -0700, Stephen Hemminger wrote: > On Thu, 21 Aug 2014 19:02:08 +0100 > Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > > > > > tcp_send_syn_data: > > > > TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; > > TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH); > > > > the reporter has a point 8) > > > > https://bugzilla.kernel.org/show_bug.cgi?id=82101 > > I wonder if covertity or smatch could be smart enough to catch this kind of bug? For coverity: it should be, but isn't :) I've pointed them at the bugzilla, maybe they can add a check in a future update. I bet this isn't the only instance of a bug like this left in the tree. We've definitely had similar cases before. Dave ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TCP output handling bug ? 2014-08-22 2:13 ` Dave Jones @ 2014-08-22 13:56 ` Eric Dumazet 2014-08-22 17:41 ` David Miller 0 siblings, 1 reply; 7+ messages in thread From: Eric Dumazet @ 2014-08-22 13:56 UTC (permalink / raw) To: Dave Jones; +Cc: Stephen Hemminger, Alan Cox, netdev On Thu, 2014-08-21 at 22:13 -0400, Dave Jones wrote: > On Thu, Aug 21, 2014 at 05:24:45PM -0700, Stephen Hemminger wrote: > > On Thu, 21 Aug 2014 19:02:08 +0100 > > Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > > > > > > > > tcp_send_syn_data: > > > > > > TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; > > > TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH); > > > > > > the reporter has a point 8) > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=82101 > > > > I wonder if covertity or smatch could be smart enough to catch this kind of bug? > > For coverity: it should be, but isn't :) > I've pointed them at the bugzilla, maybe they can add a check in a > future update. > > I bet this isn't the only instance of a bug like this left in the tree. > We've definitely had similar cases before. > > Dave There is no bug here as a matter of fact. You can simply remove the first line, as it is useless. - TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; At the end of the day, data segment has ACK and PSH flags only. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TCP output handling bug ? 2014-08-22 13:56 ` Eric Dumazet @ 2014-08-22 17:41 ` David Miller 2014-08-22 18:18 ` Yuchung Cheng 2014-08-22 18:30 ` Eric Dumazet 0 siblings, 2 replies; 7+ messages in thread From: David Miller @ 2014-08-22 17:41 UTC (permalink / raw) To: eric.dumazet; +Cc: davej, stephen, alan, netdev, ycheng From: Eric Dumazet <eric.dumazet@gmail.com> Date: Fri, 22 Aug 2014 06:56:30 -0700 CC:'ing Yuchung Cheng > On Thu, 2014-08-21 at 22:13 -0400, Dave Jones wrote: >> On Thu, Aug 21, 2014 at 05:24:45PM -0700, Stephen Hemminger wrote: >> > On Thu, 21 Aug 2014 19:02:08 +0100 >> > Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: >> > >> > > >> > > tcp_send_syn_data: >> > > >> > > TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; >> > > TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH); >> > > >> > > the reporter has a point 8) >> > > >> > > https://bugzilla.kernel.org/show_bug.cgi?id=82101 >> > >> > I wonder if covertity or smatch could be smart enough to catch this kind of bug? >> >> For coverity: it should be, but isn't :) >> I've pointed them at the bugzilla, maybe they can add a check in a >> future update. >> >> I bet this isn't the only instance of a bug like this left in the tree. >> We've definitely had similar cases before. >> >> Dave > > There is no bug here as a matter of fact. > > You can simply remove the first line, as it is useless. > > - TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; > > At the end of the day, data segment has ACK and PSH flags only. What about ECE/CWR? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TCP output handling bug ? 2014-08-22 17:41 ` David Miller @ 2014-08-22 18:18 ` Yuchung Cheng 2014-08-22 18:30 ` Eric Dumazet 1 sibling, 0 replies; 7+ messages in thread From: Yuchung Cheng @ 2014-08-22 18:18 UTC (permalink / raw) To: David Miller; +Cc: Eric Dumazet, Dave Jones, Stephen Hemminger, alan, netdev On Fri, Aug 22, 2014 at 10:41 AM, David Miller <davem@davemloft.net> wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Fri, 22 Aug 2014 06:56:30 -0700 > > CC:'ing Yuchung Cheng > >> On Thu, 2014-08-21 at 22:13 -0400, Dave Jones wrote: >>> On Thu, Aug 21, 2014 at 05:24:45PM -0700, Stephen Hemminger wrote: >>> > On Thu, 21 Aug 2014 19:02:08 +0100 >>> > Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: >>> > >>> > > >>> > > tcp_send_syn_data: >>> > > >>> > > TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; >>> > > TCP_SKB_CB(data)->tcp_flags = (TCPHDR_ACK|TCPHDR_PSH); >>> > > >>> > > the reporter has a point 8) >>> > > >>> > > https://bugzilla.kernel.org/show_bug.cgi?id=82101 >>> > >>> > I wonder if covertity or smatch could be smart enough to catch this kind of bug? >>> >>> For coverity: it should be, but isn't :) >>> I've pointed them at the bugzilla, maybe they can add a check in a >>> future update. >>> >>> I bet this isn't the only instance of a bug like this left in the tree. >>> We've definitely had similar cases before. >>> >>> Dave >> >> There is no bug here as a matter of fact. >> >> You can simply remove the first line, as it is useless. >> >> - TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; >> >> At the end of the day, data segment has ACK and PSH flags only. > > What about ECE/CWR? sorry for the late response. my original intention was TCP_SKB_CB(data)->tcp_flags &= ~TCPHDR_SYN; TCP_SKB_CB(data)->tcp_flags |= (TCPHDR_ACK|TCPHDR_PSH); so that we can keep other flags intact b/c in the future we may have xmas tree packet :-) I will send a patch soon. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TCP output handling bug ? 2014-08-22 17:41 ` David Miller 2014-08-22 18:18 ` Yuchung Cheng @ 2014-08-22 18:30 ` Eric Dumazet 1 sibling, 0 replies; 7+ messages in thread From: Eric Dumazet @ 2014-08-22 18:30 UTC (permalink / raw) To: David Miller; +Cc: davej, stephen, alan, netdev, ycheng On Fri, 2014-08-22 at 10:41 -0700, David Miller wrote: > What about ECE/CWR? TCP fastopen begins its life with a fresh socket. We received no packet from the peer yet for this flow that we are building. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-08-22 18:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-21 18:02 TCP output handling bug ? Alan Cox 2014-08-22 0:24 ` Stephen Hemminger 2014-08-22 2:13 ` Dave Jones 2014-08-22 13:56 ` Eric Dumazet 2014-08-22 17:41 ` David Miller 2014-08-22 18:18 ` Yuchung Cheng 2014-08-22 18:30 ` Eric Dumazet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).