On 3/17/20 6:55 AM, Tuong Lien Tong wrote:
Hi Jon,

For the "variable window congestion control" patch, if I remember correctly,
it is for unicast link only? Why did you apply it for broadcast link, a
mistake or ...?
I did it so the code would be the same everywhere. Then, by setting both
min_win and max_win to the same value BC_LINK_WIN_DEFAULT (==50)
in the broadcast send link this window should never change.

It now causes user messages disordered on the receiving side, because on the
sending side, the broadcast link's window is suddenly increased to 300 (i.e.
max_t(u16, l->window / 2, 300)) at a packet retransmission, leaving some
gaps between the link's 'transmq' & 'backlogq' unexpectedly... Will we fix
this by removing it?
That is clearly a bug that breaks the above stated limitation.
It should be sufficient to check that also l->ssthresh never exceeds
l->max_win to remedy this.

///jon


@@ -1160,7 +1224,6 @@ static int tipc_link_bc_retrans(struct tipc_link *l,
struct tipc_link *r,
                        continue;
                if (more(msg_seqno(hdr), to))
                        break;
-
                if (time_before(jiffies, TIPC_SKB_CB(skb)->nxt_retr))
                        continue;
                TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
@@ -1173,11 +1236,12 @@ static int tipc_link_bc_retrans(struct tipc_link *l,
struct tipc_link *r,
                _skb->priority = TC_PRIO_CONTROL;
                __skb_queue_tail(xmitq, _skb);
                l->stats.retransmitted++;
-
+               retransmitted++;
                /* Increase actual retrans counter & mark first time */
                if (!TIPC_SKB_CB(skb)->retr_cnt++)
                        TIPC_SKB_CB(skb)->retr_stamp = jiffies;
        }
+       tipc_link_update_cwin(l, 0, retransmitted);             // ???
        return 0;
  }

+static void tipc_link_update_cwin(struct tipc_link *l, int released,
+                                 bool retransmitted)
+{
+       int bklog_len = skb_queue_len(&l->backlogq);
+       struct sk_buff_head *txq = &l->transmq;
+       int txq_len = skb_queue_len(txq);
+       u16 cwin = l->window;
+
+       /* Enter fast recovery */
+       if (unlikely(retransmitted)) {
+               l->ssthresh = max_t(u16, l->window / 2, 300);
+               l->window = l->ssthresh;
+               return;
+       }

BR/Tuong

-----Original Message-----
From: Jon Maloy <[email protected]>
Sent: Monday, December 2, 2019 7:33 AM
To: Jon Maloy <[email protected]>; Jon Maloy <[email protected]>
Cc: [email protected];
[email protected]; [email protected];
[email protected]; [email protected];
[email protected]; [email protected];
[email protected]
Subject: [net-next 3/3] tipc: introduce variable window congestion control

We introduce a simple variable window congestion control for links.
The algorithm is inspired by the Reno algorithm, covering both 'slow
start', 'congestion avoidance', and 'fast recovery' modes.

- We introduce hard lower and upper window limits per link, still
   different and configurable per bearer type.

- We introduce as 'slow start theshold' variable, initially set to
   the maximum window size.

- We let a link start at the minimum congestion window, i.e. in slow
   start mode, and then let is grow rapidly (+1 per rceived ACK) until
   it reaches the slow start threshold and enters congestion avoidance
   mode.

- In congestion avoidance mode we increment the congestion window for
   each window_size number of acked packets, up to a possible maximum
   equal to the configured maximum window.

- For each non-duplicate NACK received, we drop back to fast recovery
   mode, by setting the both the slow start threshold to and the
   congestion window to (current_congestion_window / 2).

- If the timeout handler finds that the transmit queue has not moved
   timeout, it drops the link back to slow start and forces a probe
   containing the last sent sequence number to the sent to the peer.

This change does in reality have effect only on unicast ethernet
transport, as we have seen that there is no room whatsoever for
increasing the window max size for the UDP bearer.
For now, we also choose to keep the limits for the broadcast link
unchanged and equal.

This algorithm seems to give a 50-100% throughput improvement for
messages larger than MTU.

Suggested-by: Xin Long <[email protected]>
Acked-by: Xin Long <[email protected]>
Signed-off-by: Jon Maloy <[email protected]>
---
  net/tipc/bcast.c     |  11 ++--
  net/tipc/bearer.c    |  11 ++--
  net/tipc/bearer.h    |   6 +-
  net/tipc/eth_media.c |   3 +-
  net/tipc/ib_media.c  |   5 +-
  net/tipc/link.c      | 175
+++++++++++++++++++++++++++++++++++----------------
  net/tipc/link.h      |   9 +--
  net/tipc/node.c      |  16 ++---
  net/tipc/udp_media.c |   3 +-
  9 files changed, 160 insertions(+), 79 deletions(-)







_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion




_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to