Fri 02 Jun 2017 08:55:22 PM UTC, comment #12:
Patch committed in 3eaf976152d781a195caa80cf1e1ed34de631c95. This fixes the introduced regression in ABC changes
Simon,
I'm open to continuing the discussion on a better way to limit cwnd. Another thing I thought of was that our send window is also limited by TCP_SND_BUF, which is what we are using to initialize ssthresh to get our "arbitrarily high limit" recommended by the RFC.
The send window is going to be limited to the smallest of the following:
1) TCP_SND_BUF
2) snd_wnd_max (assuming empty window where snd_wnd == snd_wnd_max)
3) cwnd
So we could stop incrementing cwnd when it reaches TCP_SND_BUF, which should ensure there is no rollover. TCP_SND_BUF is a nice compile time constant, though I know there was talk about implementing SO_SNDBUF, which would mean the number could change during the pcb's lifetime
|
Thu 01 Jun 2017 05:44:12 PM UTC, comment #6:
I was able to reproduce the issue, there is integer promotion happening in the inequality that checks for rollover, meaning the check doesn't work.
Tim ran into the rollover because cwnd is 1080, which is smaller than the next segment (guessing based on 1460 SMSS), so sending stalls
This was evaluating to TRUE even though all variables are of type tcpwnd_size_t, then rollover was occurring for pcb->cwnd += increase;
The previous code did the following:
And the cast causes the rollover before the inequality comparison (done as an int I believe due to integer promotion rules)
I have attached a patch that introduces a new macro, TCP_WND_INC that uses the correct cast, but also sets the window to maximum value if it were to rollover. This should ensure we can fully utilize the tcpwnd_size_t number space for cwnd. Before, once the increment would cause a rollover, we wouldn't increment cwnd to the full value
Tim,
Can you try my patch?
(file #40837)
|
Thu 01 Jun 2017 03:54:28 PM UTC, comment #4:
Hi Joel,
LWIP_WND_SCALE = 0 (default)
TCP_RCV_SCALE = 0 (default)
TCP_SND_BUF = TCP_WND = 64k-1
I'm not sure why I set TCP_SND_BUF to TCP_WND precisely.
And the pcb in stuck state: I'll just give you the whole struct :)
local_ip = {
addr = xxx :O
},
remote_ip = {
addr = xxx :)
},
netif_idx = 0 '\000',
so_options = 4 '\004',
tos = 0 '\000',
ttl = 255 '\377',
next = 0x0 <>,
callback_arg = 0x362500,
state = ESTABLISHED,
prio = 64 '@',
local_port = 5002,
remote_port = 57240,
flags = 0,
polltmr = 1 '\001',
pollinterval = 2 '\002',
last_timer = 53 '5',
tmr = 7,
rcv_nxt = 3825489976,
rcv_wnd = 65535,
rcv_ann_wnd = 65535,
rcv_ann_right_edge = 3825555511,
rtime = -1,
mss = 1460,
rttest = 0,
rtseq = 2161524,
sa = 1,
sv = 3,
rto = 3,
nrtx = 0 '\000',
dupacks = 0 '\000',
lastack = 2165904,
cwnd = 1080,
ssthresh = 65535,
rto_end = 0,
snd_nxt = 2165904,
snd_wl1 = 3825489976,
snd_wl2 = 2165904,
snd_lbb = 2231439,
snd_wnd = 65535,
snd_wnd_max = 65535,
snd_buf = 0,
snd_queuelen = 45,
unsent_oversize = 165,
bytes_acked = 0,
unsent = 0x3620a0,
unacked = 0x0 <>,
ooseq = 0x0 <>,
refused_data = 0x0 <>,
listener = 0x34fea8,
sent = 0x1356ec <sent_tcp>,
recv = 0x1351f0 <recv_tcp>,
connected = 0x0 <>,
poll = 0x1350f0 <poll_tcp>,
errf = 0x135418 <err_tcp>,
keep_idle = 7200000,
persist_cnt = 0 '\000',
persist_backoff = 0 '\000',
persist_probe = 0 '\000',
keep_cnt_sent = 0 '\000'
|