netdev - [net-next 2/3] tipc: eliminate more unnecessary nacks and retransmissions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1575935566-18786-3-git-send-email-jon.maloy@ericsson.com>
Date:   Tue, 10 Dec 2019 00:52:45 +0100
From:   Jon Maloy <jon.maloy@...csson.com>
To:     <davem@...emloft.net>, <netdev@...r.kernel.org>
CC:     <tung.q.nguyen@...tech.com.au>, <hoang.h.le@...tech.com.au>,
        <jon.maloy@...csson.com>, <lxin@...hat.com>, <shuali@...hat.com>,
        <ying.xue@...driver.com>, <edumazet@...gle.com>,
        <tipc-discussion@...ts.sourceforge.net>
Subject: [net-next 2/3] tipc: eliminate more unnecessary nacks and retransmissions

When we increase the link tranmsit window we often observe the following
scenario:

1) A STATE message bypasses a sequence of traffic packets and arrives
   far ahead of those to the receiver. STATE messages contain a
   'peers_nxt_snt' field to indicate which was the last packet sent
   from the peer. This mechanism is intended as a last resort for the
   receiver to detect missing packets, e.g., during very low traffic
   when there is no packet flow to help early loss detection.
3) The receiving link compares the 'peer_nxt_snt' field to its own
   'rcv_nxt', finds that there is a gap, and immediately sends a
   NACK message back to the peer.
4) When this NACKs arrives at the sender, all the requested
   retransmissions are performed, since it is a first-time request.

Just like in the scenario described in the previous commit this leads
to many redundant retransmissions, with decreased throughput as a
consequence.

We fix this by adding two more conditions before we send a NACK in
this sitution. First, the deferred queue must be empty, so we cannot
assume that the potential packet loss has already been detected by
other means. Second, we check the 'peers_snd_nxt' field only in probe/
probe_reply messages, thus turning this into a true mechanism of last
resort as it was really meant to be.

Acked-by: Ying Xue <ying.xue@...driver.com>
Signed-off-by: Jon Maloy <jon.maloy@...csson.com>
---
 net/tipc/link.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 6d86446..3528181 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -2079,8 +2079,12 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 			     &l->mon_state, l->bearer_id);

 		/* Send NACK if peer has sent pkts we haven't received yet */
-		if (more(peers_snd_nxt, rcv_nxt) && !tipc_link_is_synching(l))
+		if ((reply || msg_is_keepalive(hdr)) &&
+		    more(peers_snd_nxt, rcv_nxt) &&
+		    !tipc_link_is_synching(l) &&
+		    skb_queue_empty(&l->deferdq))
 			rcvgap = peers_snd_nxt - l->rcv_nxt;
+
 		if (rcvgap || reply)
 			tipc_link_build_proto_msg(l, STATE_MSG, 0, reply,
 						  rcvgap, 0, 0, xmitq);
-- 
2.1.4