Thanks Marcelo for the confirmation.
We'll try the patch you suggest.
Regards,
Chris
-----Original Message-----
From: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
Sent: 2020年2月18日 11:40
To: Fan, Jessie (NSB - CN/Qingdao) <jessie.fan@xxxxxxxxxxxxxxx>
Cc: linux-sctp@xxxxxxxxxxxxxxx; dajiang.zhang@xxxxxxxxx; piggy@xxxxxxx; karl@xxxxxxxxxxxxxxxxxxxx; chris@xxxxxxxxxxxxxxxxx; jgrimm@xxxxxxxxxx; xingang.guo@xxxxxxxxx; sri@xxxxxxxxxx; daisyc@xxxxxxxxxx; ardelle.fan@xxxxxxxxx; kevin.gao@xxxxxxxxx; Chen, Chris A. (NSB - CN/Qingdao) <chris.a.chen@xxxxxxxxxxxxxxx>
Subject: Re: v5.3.12 SCTP Stream Negotiation Problem
Hi,
On Tue, Feb 18, 2020 at 12:37:17AM +0000, Fan, Jessie (NSB - CN/Qingdao) wrote:
> Hi,
>
> I found the SCTP Stream negotiation doesn't work as expected, that is, the local outbound stream and the remote inbound stream comparison seems missing.
> For example, when the local outstream(16) is greater than the remote inbound stream(2), 16 is saved and used as the "OUTS", which is shown from /proc/pid/net/sctp/assocs below.
> Can anyone help comment?
>
> From local end point, 16 is set as both the outbound and inbound stream.
> From the remote end point, 2 is set for both the outbound and inbound
> stream
>
> However, after the association is up, the inbound and outbound stream is set as (2,16), which I think is unexpected.
> sh-4.2# cat 1/net/sctp/assocs
> ASSOC SOCK STY SST ST HBKT ASSOC-ID TX_QUEUE RX_QUEUE UID INODE LPORT
> RPORT LADDRS <-> RADDRS HBINT INS OUTS MAXRT T1X T2X RTXC wmema wmemq
> sndbuf rcvbuf 55dae5bb bb3dec72 0 7 3 0 2415 0 0 504 1223451 2905 3904
> xx.xx.xx.xx yy.yy.yy.yy <-> *zz.zz.zz.zz ww.ww.ww.ww 30000 2 16 10 0 0
> 0 1 0 262144 262144
>
> I further checked the kernel code and found in v5.0, there is still comparison logic: min(outcnt, stream->outcnt) and save the smaller one in the function sctp_stream_alloc_out().
> But the logic disappeared in v5.1, and if the "outcnt" is smaller, it's not saved locally.
> static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
> gfp_t gfp) {
> int ret;
>
> if (outcnt <= stream->outcnt) //Here is problematic, and the outcnt is not saved locally.
Makes sense.
Before 2075e50caf5e ("sctp: convert to genradix") it was updating
stream->outcnt with smaller values when applicable:
- if (outcnt > stream->outcnt)
- fa_zero(out, stream->outcnt, (outcnt - stream->outcnt));
+ if (outcnt <= stream->outcnt)
+ return 0;
- stream->out = out;
It also affects the input stream AFAICT.
Can you please try the following patch?
Thanks,
Marcelo
---8<---
diff --git a/net/sctp/stream.c b/net/sctp/stream.c index 67f7e71f9129..34f0b7312fe8 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -81,12 +81,13 @@ static int sctp_stream_alloc_out(struct sctp_stream *stream, __u16 outcnt,
int ret;
if (outcnt <= stream->outcnt)
- return 0;
+ goto out;
ret = genradix_prealloc(&stream->out, outcnt, gfp);
if (ret)
return ret;
+out:
stream->outcnt = outcnt;
return 0;
}
@@ -96,13 +97,14 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt, {
int ret;
- if (incnt <= stream->incnt)
- return 0;
+ if (incnt > stream->incnt)
+ goto out;
ret = genradix_prealloc(&stream->in, incnt, gfp);
if (ret)
return ret;
+out:
stream->incnt = incnt;
return 0;
}
