Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: sctp: Fix negotiation of the number of data streams.
@ 2020-08-18 14:36 David Laight
  2020-08-18 19:58 ` David Miller
  2020-08-18 21:46 ` 'Marcelo Ricardo Leitner'
  0 siblings, 2 replies; 5+ messages in thread
From: David Laight @ 2020-08-18 14:36 UTC (permalink / raw)
  To: 'netdev@vger.kernel.org', 'linux-sctp@vger.kernel.org'
  Cc: 'Marcelo Ricardo Leitner'

The number of streams offered by the remote system was being ignored.
Any data sent on those streams would get discarded by the remote system.

Fixes 2075e50caf5ea.

Signed-off-by: David Laight <david.laight@aculab.com>
---
 net/sctp/stream.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

This needs backporting to 5.1 and all later kernels.

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index bda2536dd740..6dc95dcc0ff4 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -88,12 +88,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;
 }
@@ -104,12 +105,13 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt,
 	int ret;
 
 	if (incnt <= stream->incnt)
-		return 0;
+		goto out;
 
 	ret = genradix_prealloc(&stream->in, incnt, gfp);
 	if (ret)
 		return ret;
 
+out:
 	stream->incnt = incnt;
 	return 0;
 }
-- 
2.25.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: sctp: Fix negotiation of the number of data streams.
  2020-08-18 14:36 [PATCH] net: sctp: Fix negotiation of the number of data streams David Laight
@ 2020-08-18 19:58 ` David Miller
  2020-08-18 21:28   ` David Laight
  2020-08-18 21:46 ` 'Marcelo Ricardo Leitner'
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2020-08-18 19:58 UTC (permalink / raw)
  To: David.Laight; +Cc: netdev, linux-sctp, marcelo.leitner

From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 18 Aug 2020 14:36:58 +0000

> Fixes 2075e50caf5ea.
> 
> Signed-off-by: David Laight <david.laight@aculab.com>

This is not the correct format for a Fixes tag, also it should not
be separated by empty lines from other tags such as Signed-off-by
and Acked-by.

As someone who reads netdev frequently, these patterns should be
etched into your brain by now. :-)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] net: sctp: Fix negotiation of the number of data streams.
  2020-08-18 19:58 ` David Miller
@ 2020-08-18 21:28   ` David Laight
  0 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2020-08-18 21:28 UTC (permalink / raw)
  To: 'David Miller'; +Cc: netdev, linux-sctp, marcelo.leitner

From: David Miller
> Sent: 18 August 2020 20:58
> 
> From: David Laight <David.Laight@ACULAB.COM>
> Date: Tue, 18 Aug 2020 14:36:58 +0000
> 
> > Fixes 2075e50caf5ea.
> >
> > Signed-off-by: David Laight <david.laight@aculab.com>
> 
> This is not the correct format for a Fixes tag, also it should not
> be separated by empty lines from other tags such as Signed-off-by
> and Acked-by.
> 
> As someone who reads netdev frequently, these patterns should be
> etched into your brain by now. :-)

Sorry I don't often write patches - I'm only paid to support
our drivers, not fix the Linux kernel :-)
(We don't ship a kernel, our drivers have to work in disto kernels.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net: sctp: Fix negotiation of the number of data streams.
  2020-08-18 14:36 [PATCH] net: sctp: Fix negotiation of the number of data streams David Laight
  2020-08-18 19:58 ` David Miller
@ 2020-08-18 21:46 ` 'Marcelo Ricardo Leitner'
  2020-08-18 22:10   ` David Laight
  1 sibling, 1 reply; 5+ messages in thread
From: 'Marcelo Ricardo Leitner' @ 2020-08-18 21:46 UTC (permalink / raw)
  To: David Laight
  Cc: 'netdev@vger.kernel.org', 'linux-sctp@vger.kernel.org'

On Tue, Aug 18, 2020 at 02:36:58PM +0000, David Laight wrote:
> The number of streams offered by the remote system was being ignored.
> Any data sent on those streams would get discarded by the remote system.

That's quite brief and not accurate: it was only ignored if 'Xcnt <=
stream->Xcnt'.

Other than this and the Fixes tag, LGTM. Passes the tests here. I'll
ack the v2 then.

> 
> Fixes 2075e50caf5ea.
> 
> Signed-off-by: David Laight <david.laight@aculab.com>
> ---
>  net/sctp/stream.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> This needs backporting to 5.1 and all later kernels.

A 'net' tree tag in patch tags is welcomed.

> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index bda2536dd740..6dc95dcc0ff4 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -88,12 +88,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;
>  }
> @@ -104,12 +105,13 @@ static int sctp_stream_alloc_in(struct sctp_stream *stream, __u16 incnt,
>  	int ret;
>  
>  	if (incnt <= stream->incnt)
> -		return 0;
> +		goto out;
>  
>  	ret = genradix_prealloc(&stream->in, incnt, gfp);
>  	if (ret)
>  		return ret;
>  
> +out:
>  	stream->incnt = incnt;
>  	return 0;
>  }
> -- 
> 2.25.1
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] net: sctp: Fix negotiation of the number of data streams.
  2020-08-18 21:46 ` 'Marcelo Ricardo Leitner'
@ 2020-08-18 22:10   ` David Laight
  0 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2020-08-18 22:10 UTC (permalink / raw)
  To: 'Marcelo Ricardo Leitner'
  Cc: 'netdev@vger.kernel.org', 'linux-sctp@vger.kernel.org'

From: 'Marcelo Ricardo Leitner'
> Sent: 18 August 2020 22:47
> 
> On Tue, Aug 18, 2020 at 02:36:58PM +0000, David Laight wrote:
> > The number of streams offered by the remote system was being ignored.
> > Any data sent on those streams would get discarded by the remote system.
> 
> That's quite brief and not accurate: it was only ignored if 'Xcnt <=
> stream->Xcnt'.

The number of streams (esp out ones) received from the remote
system in an INIT or INIT_ACK was ignored.
So it would always send data chunks using the number of streams
requested by the local user.
I managed to tweak our M3UA config to get invalid stream numbers
sent on both inwards and outwards connections.

I only noticed because of testing a (slightly horrid) workaround
for no longer being able to use kernel_getsockopt() to retrieve
the number of ostreams.
The number of ostreams was about the only thing we didn't trace :-(
At least my code can now obtain the correct value even for buggy
kernels.

> Other than this and the Fixes tag, LGTM. Passes the tests here. I'll
> ack the v2 then.

I wasn't sure whether DM actually wanted a V2 with the fixes
tag fixed.
I can send one tomorrow.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-08-18 22:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 14:36 [PATCH] net: sctp: Fix negotiation of the number of data streams David Laight
2020-08-18 19:58 ` David Miller
2020-08-18 21:28   ` David Laight
2020-08-18 21:46 ` 'Marcelo Ricardo Leitner'
2020-08-18 22:10   ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).