From patchwork Thu Apr 4 04:09:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tuong Lien X-Patchwork-Id: 1076771 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=dektech.com.au Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=dektech.com.au header.i=@dektech.com.au header.b="japCztdk"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 44ZVB82zw9z9sSP for ; Thu, 4 Apr 2019 15:21:04 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726223AbfDDEUX (ORCPT ); Thu, 4 Apr 2019 00:20:23 -0400 Received: from f0-dek.dektech.com.au ([210.10.221.142]:50902 "EHLO mail.dektech.com.au" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725919AbfDDEUX (ORCPT ); Thu, 4 Apr 2019 00:20:23 -0400 Received: from localhost (localhost [127.0.0.1]) by mail.dektech.com.au (Postfix) with ESMTP id 735EBF9746; Thu, 4 Apr 2019 15:10:05 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=dektech.com.au; h=references:in-reply-to:x-mailer:message-id:date:date:subject :subject:from:from:received:received:received; s=mail_dkim; t= 1554351005; bh=OPmAWPCjfSMK5m78JQLkyXkg2dvWg8+XZHkgk9+Pza4=; b=j apCztdkrykM1ia0UjGZ0tQAg1B92u7ka4Cl4nT0GRB5UJnMDMVCU07hM50x/o5cS jVTgJVI3COt57Hqm0AHeTdPZzu8idi5nHiEnvxpAAn3lDHZBdpeDWb6ZvQdeUiiU vssT0B9gXJniX8KuQ4TcUx13hJqsH8kKA1qYY1oG1A= X-Virus-Scanned: amavisd-new at dektech.com.au Received: from mail.dektech.com.au ([127.0.0.1]) by localhost (mail2.dektech.com.au [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id xTs6r3kaLacs; Thu, 4 Apr 2019 15:10:05 +1100 (AEDT) Received: from mail.dektech.com.au (localhost [127.0.0.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.dektech.com.au (Postfix) with ESMTPS id 54F0EF974B; Thu, 4 Apr 2019 15:10:05 +1100 (AEDT) Received: from localhost.localdomain (unknown [14.161.14.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail.dektech.com.au (Postfix) with ESMTPSA id CFB0DF9746; Thu, 4 Apr 2019 15:10:03 +1100 (AEDT) From: Tuong Lien To: davem@davemloft.net, jon.maloy@ericsson.com, maloy@donjonn.com, ying.xue@windriver.com, netdev@vger.kernel.org Cc: tipc-discussion@lists.sourceforge.net Subject: [net-next 2/3] tipc: reduce duplicate packets for unicast traffic Date: Thu, 4 Apr 2019 11:09:52 +0700 Message-Id: <20190404040953.1569-3-tuong.t.lien@dektech.com.au> X-Mailer: git-send-email 2.13.7 In-Reply-To: <20190404040953.1569-1-tuong.t.lien@dektech.com.au> References: <20190404040953.1569-1-tuong.t.lien@dektech.com.au> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org For unicast transmission, the current NACK sending althorithm is over- active that forces the sending side to retransmit a packet that is not really lost but just arrived at the receiving side with some delay, or even retransmit same packets that have already been retransmitted before. As a result, many duplicates are observed also under normal condition, ie. without packet loss. One example case is: node1 transmits 1 2 3 4 10 5 6 7 8 9, when node2 receives packet #10, it puts into the deferdq. When the packet #5 comes it sends NACK with gap [6 - 9]. However, shortly after that, when packet #6 arrives, it pulls out packet #10 from the deferfq, but it is still out of order, so it makes another NACK with gap [7 - 9] and so on ... Finally, node1 has to retransmit the packets 5 6 7 8 9 a number of times, but in fact all the packets are not lost at all, so duplicates! This commit reduces duplicates by changing the condition to send NACK, also restricting the retransmissions on individual packets via a timer of about 1ms. However, it also needs to say that too tricky condition for NACKs or too long timeout value for retransmissions will result in performance reducing! The criterias in this commit are found to be effective for both the requirements to reduce duplicates but not affect performance. The tipc_link_rcv() is also improved to only dequeue skb from the link deferdq if it is expected (ie. its seqno <= rcv_nxt). Acked-by: Ying Xue Acked-by: Jon Maloy Signed-off-by: Tuong Lien --- net/tipc/link.c | 26 ++++++++++++++++---------- net/tipc/msg.h | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/net/tipc/link.c b/net/tipc/link.c index 5aee1ed23ba9..1f2cde0d025f 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -209,6 +209,7 @@ enum { }; #define TIPC_BC_RETR_LIM msecs_to_jiffies(10) /* [ms] */ +#define TIPC_UC_RETR_TIME (jiffies + msecs_to_jiffies(1)) /* * Interval between NACKs when packets arrive out of order @@ -1305,6 +1306,10 @@ static void tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap, kfree_skb(skb); } else if (less_eq(seqno, acked + gap)) { /* retransmit skb */ + if (time_before(jiffies, TIPC_SKB_CB(skb)->nxt_retr)) + continue; + TIPC_SKB_CB(skb)->nxt_retr = TIPC_UC_RETR_TIME; + _skb = __pskb_copy(skb, MIN_H_SIZE, GFP_ATOMIC); if (!_skb) continue; @@ -1380,6 +1385,7 @@ static int tipc_link_build_nack_msg(struct tipc_link *l, struct sk_buff_head *xmitq) { u32 def_cnt = ++l->stats.deferred_recv; + u32 defq_len = skb_queue_len(&l->deferdq); int match1, match2; if (link_is_bc_rcvlink(l)) { @@ -1390,7 +1396,7 @@ static int tipc_link_build_nack_msg(struct tipc_link *l, return 0; } - if ((skb_queue_len(&l->deferdq) == 1) || !(def_cnt % TIPC_NACK_INTV)) + if (defq_len >= 3 && !((defq_len - 3) % 16)) tipc_link_build_proto_msg(l, STATE_MSG, 0, 0, 0, 0, 0, xmitq); return 0; } @@ -1404,29 +1410,29 @@ int tipc_link_rcv(struct tipc_link *l, struct sk_buff *skb, struct sk_buff_head *xmitq) { struct sk_buff_head *defq = &l->deferdq; - struct tipc_msg *hdr; + struct tipc_msg *hdr = buf_msg(skb); u16 seqno, rcv_nxt, win_lim; int rc = 0; + /* Verify and update link state */ + if (unlikely(msg_user(hdr) == LINK_PROTOCOL)) + return tipc_link_proto_rcv(l, skb, xmitq); + + /* Don't send probe at next timeout expiration */ + l->silent_intv_cnt = 0; + do { hdr = buf_msg(skb); seqno = msg_seqno(hdr); rcv_nxt = l->rcv_nxt; win_lim = rcv_nxt + TIPC_MAX_LINK_WIN; - /* Verify and update link state */ - if (unlikely(msg_user(hdr) == LINK_PROTOCOL)) - return tipc_link_proto_rcv(l, skb, xmitq); - if (unlikely(!link_is_up(l))) { if (l->state == LINK_ESTABLISHING) rc = TIPC_LINK_UP_EVT; goto drop; } - /* Don't send probe at next timeout expiration */ - l->silent_intv_cnt = 0; - /* Drop if outside receive window */ if (unlikely(less(seqno, rcv_nxt) || more(seqno, win_lim))) { l->stats.duplicates++; @@ -1457,7 +1463,7 @@ int tipc_link_rcv(struct tipc_link *l, struct sk_buff *skb, rc |= tipc_link_build_state_msg(l, xmitq); if (unlikely(rc & ~TIPC_LINK_SND_STATE)) break; - } while ((skb = __skb_dequeue(defq))); + } while ((skb = __tipc_skb_dequeue(defq, l->rcv_nxt))); return rc; drop: diff --git a/net/tipc/msg.h b/net/tipc/msg.h index ec5c511a9c9c..8de02ad6e352 100644 --- a/net/tipc/msg.h +++ b/net/tipc/msg.h @@ -1151,4 +1151,25 @@ static inline void tipc_skb_queue_splice_tail_init(struct sk_buff_head *list, tipc_skb_queue_splice_tail(&tmp, head); } +/* __tipc_skb_dequeue() - dequeue the head skb according to expected seqno + * @list: list to be dequeued from + * @seqno: seqno of the expected msg + * + * returns skb dequeued from the list if its seqno is less than or equal to + * the expected one, otherwise the skb is still hold + * + * Note: must be used with appropriate locks held only + */ +static inline struct sk_buff *__tipc_skb_dequeue(struct sk_buff_head *list, + u16 seqno) +{ + struct sk_buff *skb = skb_peek(list); + + if (skb && less_eq(buf_seqno(skb), seqno)) { + __skb_unlink(skb, list); + return skb; + } + return NULL; +} + #endif