* Re: About rwnd_press?
@ 2015-12-15 3:24 Vlad Yasevich
0 siblings, 0 replies; 2+ messages in thread
From: Vlad Yasevich @ 2015-12-15 3:24 UTC (permalink / raw)
To: linux-sctp
On 11/18/2015 10:49 AM, g.o.m.o.n.o.v.y.c.h wrote:
> Hello,
>
> I would like to ask about asoc->rwnd_press in compared to asoc->rwnd.
>
> sctp_assoc_rwnd_increase has next if case:
> if (asoc->rwnd_press && asoc->rwnd >= asoc->rwnd_press)
>
> I want ask community opinion about "asoc->rwnd >= asoc->rwnd_press"
> I observed situation when association buffer was empty but "asoc->rwnd
>> = asoc->rwnd_press"
> returned false.
> Next table shows this issue:
> 1. skb_payload = payload + skb; (skb=232)
> 2. If ulp will read all packets from buffer how much asoc->rwnd will be
> rwnd_r = (sk_rcvbuf / skb_payload) * payload;
> 3. for current payload size and sk_rcvbuf size how big initialization
> value will be for rwnd_press (in sctp_assoc_rwnd_decrease).
> rwnd_press = sk_rcvbuf/2 - rwnd_r
> On my laptop skb has sizeof(struct sk_buff) = 232 and sk_rcvbuf equal 212992.
>
> payload skb_payload rwnd_r rwnd_press
> ...
> 16 248 13744 92752
> //asoc->rwnd>=asoc->rwnd_press - false. but buffer empty.
> 17 249 14552 91944
> 18 250 15336 91160
> 19 251 16131 90365
> 20 252 16920 89576
> 21 253 17682 88814
> 22 254 18458 88038
> 23 255 19228 87268
> 24 256 19968 86528
> 25 257 20725 85771
> 26 258 21476 85020
> 27 259 22221 84275
> 28 260 22960 83536
> 29 261 23693 82803
> 30 262 24390 82106
> 31 263 25110 81386
> 32 264 25824 80672
> ...
> 64 296 46080 60416
> 65 297 46670 59826
> 66 298 47190 59306
> 67 299 47771 58725
> 68 300 48280 58216
> 69 301 48852 57644
> 70 302 49420 57076
> 71 303 49913 56583
> 72 304 50472 56024
> 73 305 51027 55469
> 74 306 51578 54918
> 75 307 52050 54446
> 76 308 52592 53904
> 77 309 53130 < 53366
> 78 310 53664 > 52832
> //asoc->rwnd>=asoc->rwnd_press - true
> 79 311 54115 > 52381
> 80 312 54640 > 51856
>
> Should it ("asoc->rwnd >= asoc->rwnd_press") stay false even when buffer empty?
I've been thinking about this and yes, it looks like we have an issue.
What you are describing is a condition where our receive buffer fills
up considerably faster then the receive window (due to small data size).
As a result, we might arrive at a situation, where we mark that our window
is under pressure the current available window is larger then consumed.
As a result, when we try to release consumed space, we never fully restore
it.
There is actually another problem with this code. Simply allowing the
receive to consume all the data will never completely relieve the pressure
condition.
>
> I tested on my laptop next solution:
>
> --- a/net/sctp/associola.c 2015-08-15 19:07:16.527696361 +0200
> +++ b/net/sctp/associola.c 2015-08-15 20:08:22.407812656 +0200
> @@ -1458,10 +1458,18 @@ void sctp_assoc_rwnd_increase(struct sct
> * threshold. The idea is to recover slowly, but up
> * to the initial advertised window.
> */
> - if (asoc->rwnd_press && asoc->rwnd >= asoc->rwnd_press) {
> - int change = min(asoc->pathmtu, asoc->rwnd_press);
> - asoc->rwnd += change;
> - asoc->rwnd_press -= change;
> + if (asoc->rwnd_press) {
> + int fr_count = asoc->base.sk->sk_rcvbuf;
> + if (asoc->ep->rcvbuf_policy)
> + fr_count -= atomic_read(&asoc->rmem_alloc);
> + else
> + fr_count -= atomic_read(&asoc->base.sk->sk_rmem_alloc);
> +
> + if (fr_count > 0 && ((fr_count >> 1) >= asoc->rwnd_press)) {
I am trying to understand this solution... You are computing the 'free count'
here by looking at the current value of allocated memory. However, since the skb
hasn't been freed yet, the allocated memory amount hasn't changed yet.
So, in effect, (sk_recvbuf - rmem_alloc) >> 1 should be the same as asoc->rwnd
at the time we are trying to increase rwnd.
The above section can be effectively simplified to:
int old_rwnd = asoc->rwnd;
... rwnd_over code...
if (asoc->rwnd_press && old_rwnd && old_rwnd >= asoc->rwnd_press) {
This still doesn't address all the problems. The correct way to address this
issue is to tie the window to free buffer space. This was attempted before
and had a very bad performance impact. At the time we reverted the change,
but the work got dropped.
My wish is for someone to pick that code up and figure out what went wrong.
-vlad
> + int change = min(asoc->pathmtu, asoc->rwnd_press);
> + asoc->rwnd += change;
> + asoc->rwnd_press -= change;
> + }
> }
>
> pr_debug("%s: asoc:%p rwnd increased by %d to (%u, %u) - %u\n",
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 2+ messages in thread* About rwnd_press?
@ 2015-11-18 15:49 g.o.m.o.n.o.v.y.c.h
0 siblings, 0 replies; 2+ messages in thread
From: g.o.m.o.n.o.v.y.c.h @ 2015-11-18 15:49 UTC (permalink / raw)
To: linux-sctp
Hello,
I would like to ask about asoc->rwnd_press in compared to asoc->rwnd.
sctp_assoc_rwnd_increase has next if case:
if (asoc->rwnd_press && asoc->rwnd >= asoc->rwnd_press)
I want ask community opinion about "asoc->rwnd >= asoc->rwnd_press"
I observed situation when association buffer was empty but "asoc->rwnd
>= asoc->rwnd_press"
returned false.
Next table shows this issue:
1. skb_payload = payload + skb; (skb=232)
2. If ulp will read all packets from buffer how much asoc->rwnd will be
rwnd_r = (sk_rcvbuf / skb_payload) * payload;
3. for current payload size and sk_rcvbuf size how big initialization
value will be for rwnd_press (in sctp_assoc_rwnd_decrease).
rwnd_press = sk_rcvbuf/2 - rwnd_r
On my laptop skb has sizeof(struct sk_buff) = 232 and sk_rcvbuf equal 212992.
payload skb_payload rwnd_r rwnd_press
...
16 248 13744 92752
//asoc->rwnd>=asoc->rwnd_press - false. but buffer empty.
17 249 14552 91944
18 250 15336 91160
19 251 16131 90365
20 252 16920 89576
21 253 17682 88814
22 254 18458 88038
23 255 19228 87268
24 256 19968 86528
25 257 20725 85771
26 258 21476 85020
27 259 22221 84275
28 260 22960 83536
29 261 23693 82803
30 262 24390 82106
31 263 25110 81386
32 264 25824 80672
...
64 296 46080 60416
65 297 46670 59826
66 298 47190 59306
67 299 47771 58725
68 300 48280 58216
69 301 48852 57644
70 302 49420 57076
71 303 49913 56583
72 304 50472 56024
73 305 51027 55469
74 306 51578 54918
75 307 52050 54446
76 308 52592 53904
77 309 53130 < 53366
78 310 53664 > 52832
//asoc->rwnd>=asoc->rwnd_press - true
79 311 54115 > 52381
80 312 54640 > 51856
Should it ("asoc->rwnd >= asoc->rwnd_press") stay false even when buffer empty?
I tested on my laptop next solution:
--- a/net/sctp/associola.c 2015-08-15 19:07:16.527696361 +0200
+++ b/net/sctp/associola.c 2015-08-15 20:08:22.407812656 +0200
@@ -1458,10 +1458,18 @@ void sctp_assoc_rwnd_increase(struct sct
* threshold. The idea is to recover slowly, but up
* to the initial advertised window.
*/
- if (asoc->rwnd_press && asoc->rwnd >= asoc->rwnd_press) {
- int change = min(asoc->pathmtu, asoc->rwnd_press);
- asoc->rwnd += change;
- asoc->rwnd_press -= change;
+ if (asoc->rwnd_press) {
+ int fr_count = asoc->base.sk->sk_rcvbuf;
+ if (asoc->ep->rcvbuf_policy)
+ fr_count -= atomic_read(&asoc->rmem_alloc);
+ else
+ fr_count -= atomic_read(&asoc->base.sk->sk_rmem_alloc);
+
+ if (fr_count > 0 && ((fr_count >> 1) >= asoc->rwnd_press)) {
+ int change = min(asoc->pathmtu, asoc->rwnd_press);
+ asoc->rwnd += change;
+ asoc->rwnd_press -= change;
+ }
}
pr_debug("%s: asoc:%p rwnd increased by %d to (%u, %u) - %u\n",
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-12-15 3:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-15 3:24 About rwnd_press? Vlad Yasevich
-- strict thread matches above, loose matches on Subject: below --
2015-11-18 15:49 g.o.m.o.n.o.v.y.c.h
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.