diff mbox series

[net-next] TCP: make seq # error messages more readable

Message ID 48586224-eb36-2489-735c-4946cb0b1c2b@infradead.org
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series [net-next] TCP: make seq # error messages more readable | expand

Commit Message

Randy Dunlap July 13, 2018, 12:48 a.m. UTC 
From: Randy Dunlap <rdunlap@infradead.org>

Attempt to make cryptic TCP seq number error messages clearer by
(1) adding the function name, (2) identifying the errors as "seq # bug",
and (3) grouping the field identifiers and values by separating them
with commas.

E.g., the following message is changed from:

recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0
WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:1881 tcp_recvmsg+0x649/0xb90

to:

tcp_recvmsg: TCP recvmsg seq # bug 2: copied 73BCB6CD, seq 70F17CBE, rcvnxt 73BCB9AA, fl 0
WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:2011 tcp_recvmsg+0x694/0xba0

Suggested-by: 積丹尼 Dan Jacobson <jidanni@jidanni.org>
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
 net/ipv4/tcp.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Eric Dumazet July 13, 2018, 1:38 p.m. UTC | #1 
On 07/12/2018 05:48 PM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Attempt to make cryptic TCP seq number error messages clearer by
> (1) adding the function name, (2) identifying the errors as "seq # bug",
> and (3) grouping the field identifiers and values by separating them
> with commas.
> 
> E.g., the following message is changed from:
> 
> recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0
> WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:1881 tcp_recvmsg+0x649/0xb90
> 
> to:
> 
> tcp_recvmsg: TCP recvmsg seq # bug 2: copied 73BCB6CD, seq 70F17CBE, rcvnxt 73BCB9AA, fl 0
> WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:2011 tcp_recvmsg+0x694/0xba0
> 

Hi Randy

It is not clear what this patch improves.
Do we really to mention tcp twice ?

Thanks.
Randy Dunlap July 13, 2018, 4:48 p.m. UTC | #2 
On 07/13/2018 06:38 AM, Eric Dumazet wrote:
> 
> 
> On 07/12/2018 05:48 PM, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Attempt to make cryptic TCP seq number error messages clearer by
>> (1) adding the function name, (2) identifying the errors as "seq # bug",
>> and (3) grouping the field identifiers and values by separating them
>> with commas.
>>
>> E.g., the following message is changed from:
>>
>> recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0
>> WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:1881 tcp_recvmsg+0x649/0xb90
>>
>> to:
>>
>> tcp_recvmsg: TCP recvmsg seq # bug 2: copied 73BCB6CD, seq 70F17CBE, rcvnxt 73BCB9AA, fl 0
>> WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:2011 tcp_recvmsg+0x694/0xba0
>>
> 
> Hi Randy
> 
> It is not clear what this patch improves.
> Do we really to mention tcp twice ?

I will gladly drop the __func__: string.  Doing so would change this:

recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0

to:

TCP recvmsg seq # bug 2: copied 73BCB6CD, seq 70F17CBE, rcvnxt 73BCB9AA, fl 0


The (new) message identifies that it is TCP and that it is a sequence # bug.
Both of those are helpful in my view, whereas the original message isn't
helpful at all.


> 
> Thanks.
diff mbox series

Patch

--- linux-next-20180712.orig/net/ipv4/tcp.c
+++ linux-next-20180712/net/ipv4/tcp.c
@@ -1994,9 +1994,9 @@  int tcp_recvmsg(struct sock *sk, struct
 			 * shouldn't happen.
 			 */
 			if (WARN(before(*seq, TCP_SKB_CB(skb)->seq),
-				 "recvmsg bug: copied %X seq %X rcvnxt %X fl %X\n",
-				 *seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt,
-				 flags))
+				 "%s: TCP recvmsg seq # bug: copied %X, seq %X, rcvnxt %X, fl %X\n",
+				 __func__, *seq,
+				 TCP_SKB_CB(skb)->seq, tp->rcv_nxt, flags))
 				break;
 
 			offset = *seq - TCP_SKB_CB(skb)->seq;
@@ -2009,8 +2009,9 @@  int tcp_recvmsg(struct sock *sk, struct
 			if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
 				goto found_fin_ok;
 			WARN(!(flags & MSG_PEEK),
-			     "recvmsg bug 2: copied %X seq %X rcvnxt %X fl %X\n",
-			     *seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt, flags);
+			     "%s: TCP recvmsg seq # bug 2: copied %X, seq %X, rcvnxt %X, fl %X\n",
+			     __func__, *seq,
+			     TCP_SKB_CB(skb)->seq, tp->rcv_nxt, flags);
 		}
 
 		/* Well, if we have backlog, try to process it now yet. */