[PATCH net-next 00/13] sctp: refactor MTU handling

[PATCH net-next 00/13] sctp: refactor MTU handling

[Marcelo Ricardo Leitner]

Currently MTU handling is spread over SCTP stack. There are multiple
places doing same/similar calculations and updating them is error prone
as one spot can easily be left out.

This patchset converges it into a more concise and consistent code. In
general, it moves MTU handling from functions with bigger objectives,
such as sctp_assoc_add_peer(), to specific functions.

It's also a preparation for the next patchset, which removes the
duplication between sctp_make_op_error_space and
sctp_make_op_error_fixed and relies on sctp_mtu_payload introduced here.

More details on each patch.

Marcelo Ricardo Leitner (13):
  sctp: remove old and unused SCTP_MIN_PMTU
  sctp: move transport pathmtu calc away of sctp_assoc_add_peer
  sctp: remove an if() that is always true
  sctp: introduce sctp_assoc_set_pmtu
  sctp: introduce sctp_mtu_payload
  sctp: introduce sctp_assoc_update_frag_point
  sctp: remove sctp_assoc_pending_pmtu
  sctp: introduce sctp_dst_mtu
  sctp: remove sctp_transport_pmtu_check
  sctp: re-use sctp_transport_pmtu in sctp_transport_route
  sctp: honor PMTU_DISABLED when handling icmp
  sctp: consider idata chunks when setting SCTP_MAXSEG
  sctp: allow unsetting sockopt MAXSEG

 include/net/sctp/constants.h |  5 ++--
 include/net/sctp/sctp.h      | 52 ++++++++++++++------------------------
 include/net/sctp/structs.h   |  2 ++
 net/sctp/associola.c         | 60 +++++++++++++++++++++++---------------------
 net/sctp/chunk.c             | 12 +--------
 net/sctp/output.c            | 28 ++++++++-------------
 net/sctp/socket.c            | 43 ++++++++++++++-----------------
 net/sctp/transport.c         | 37 ++++++++++++++-------------
 8 files changed, 105 insertions(+), 134 deletions(-)

[PATCH net-next 00/13] sctp: refactor MTU handling

[Marcelo Ricardo Leitner]

Currently MTU handling is spread over SCTP stack. There are multiple
places doing same/similar calculations and updating them is error prone
as one spot can easily be left out.

This patchset converges it into a more concise and consistent code. In
general, it moves MTU handling from functions with bigger objectives,
such as sctp_assoc_add_peer(), to specific functions.

It's also a preparation for the next patchset, which removes the
duplication between sctp_make_op_error_space and
sctp_make_op_error_fixed and relies on sctp_mtu_payload introduced here.

More details on each patch.

Marcelo Ricardo Leitner (13):
  sctp: remove old and unused SCTP_MIN_PMTU
  sctp: move transport pathmtu calc away of sctp_assoc_add_peer
  sctp: remove an if() that is always true
  sctp: introduce sctp_assoc_set_pmtu
  sctp: introduce sctp_mtu_payload
  sctp: introduce sctp_assoc_update_frag_point
  sctp: remove sctp_assoc_pending_pmtu
  sctp: introduce sctp_dst_mtu
  sctp: remove sctp_transport_pmtu_check
  sctp: re-use sctp_transport_pmtu in sctp_transport_route
  sctp: honor PMTU_DISABLED when handling icmp
  sctp: consider idata chunks when setting SCTP_MAXSEG
  sctp: allow unsetting sockopt MAXSEG

 include/net/sctp/constants.h |  5 ++--
 include/net/sctp/sctp.h      | 52 ++++++++++++++------------------------
 include/net/sctp/structs.h   |  2 ++
 net/sctp/associola.c         | 60 +++++++++++++++++++++++---------------------
 net/sctp/chunk.c             | 12 +--------
 net/sctp/output.c            | 28 ++++++++-------------
 net/sctp/socket.c            | 43 ++++++++++++++-----------------
 net/sctp/transport.c         | 37 ++++++++++++++-------------
 8 files changed, 105 insertions(+), 134 deletions(-)

--
2.14.3

[PATCH net-next 01/13] sctp: remove old and unused SCTP_MIN_PMTU

[Marcelo Ricardo Leitner]

This value is not used anywhere in the code. In essence it is a
duplicate of SCTP_DEFAULT_MINSEGMENT, which is used by the stack.

SCTP_MIN_PMTU value makes more sense, but we should not change to it now
as it would risk breaking applications.

So this patch removes SCTP_MIN_PMTU and adjust the comment above it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/constants.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index 20ff237c5eb2ff358589b30bbd89d636e41b2d92..86f034b524d46178e4d26e868be3a2bf87acac4a 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -254,11 +254,10 @@ enum { SCTP_ARBITRARY_COOKIE_ECHO_LEN = 200 };
 #define SCTP_TSN_MAP_SIZE 4096
 
 /* We will not record more than this many duplicate TSNs between two
- * SACKs.  The minimum PMTU is 576.  Remove all the headers and there
- * is enough room for 131 duplicate reports.  Round down to the
+ * SACKs.  The minimum PMTU is 512.  Remove all the headers and there
+ * is enough room for 117 duplicate reports.  Round down to the
  * nearest power of 2.
  */
-enum { SCTP_MIN_PMTU = 576 };
 enum { SCTP_MAX_DUP_TSNS = 16 };
 enum { SCTP_MAX_GABS = 16 };
 
-- 
2.14.3

[PATCH net-next 01/13] sctp: remove old and unused SCTP_MIN_PMTU

[Marcelo Ricardo Leitner]

This value is not used anywhere in the code. In essence it is a
duplicate of SCTP_DEFAULT_MINSEGMENT, which is used by the stack.

SCTP_MIN_PMTU value makes more sense, but we should not change to it now
as it would risk breaking applications.

So this patch removes SCTP_MIN_PMTU and adjust the comment above it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/constants.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index 20ff237c5eb2ff358589b30bbd89d636e41b2d92..86f034b524d46178e4d26e868be3a2bf87acac4a 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -254,11 +254,10 @@ enum { SCTP_ARBITRARY_COOKIE_ECHO_LEN = 200 };
 #define SCTP_TSN_MAP_SIZE 4096
 
 /* We will not record more than this many duplicate TSNs between two
- * SACKs.  The minimum PMTU is 576.  Remove all the headers and there
- * is enough room for 131 duplicate reports.  Round down to the
+ * SACKs.  The minimum PMTU is 512.  Remove all the headers and there
+ * is enough room for 117 duplicate reports.  Round down to the
  * nearest power of 2.
  */
-enum { SCTP_MIN_PMTU = 576 };
 enum { SCTP_MAX_DUP_TSNS = 16 };
 enum { SCTP_MAX_GABS = 16 };
 
-- 
2.14.3

[PATCH net-next 02/13] sctp: move transport pathmtu calc away of sctp_assoc_add_peer

[Marcelo Ricardo Leitner]

There was only one case that sctp_assoc_add_peer couldn't handle, which
is when SPP_PMTUD_DISABLE is set and pathmtu not initialized.
So add this situation to sctp_transport_route and reuse what was
already in there.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/associola.c | 9 +--------
 net/sctp/transport.c | 8 ++++++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a8f3b088fcb2ac82f58b7d6fb8894fe712e2bc0a..b3aa95222bd52113295cb246c503c903bdd5c353 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -652,15 +652,8 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 	 */
 	peer->param_flags = asoc->param_flags;
 
-	sctp_transport_route(peer, NULL, sp);
-
 	/* Initialize the pmtu of the transport. */
-	if (peer->param_flags & SPP_PMTUD_DISABLE) {
-		if (asoc->pathmtu)
-			peer->pathmtu = asoc->pathmtu;
-		else
-			peer->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
-	}
+	sctp_transport_route(peer, NULL, sp);
 
 	/* If this is the first transport addr on this association,
 	 * initialize the association PMTU to the peer's PMTU.
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 47f82bd794d915188bad037463c2aa14175a55ef..c5fc3aed08a18658448b528b07dbb13900686313 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -307,11 +307,15 @@ void sctp_transport_route(struct sctp_transport *transport,
 		 * association's active path for getsockname().
 		 */
 		if (asoc && (!asoc->peer.primary_path ||
-				(transport == asoc->peer.active_path)))
+			     (transport == asoc->peer.active_path)))
 			opt->pf->to_sk_saddr(&transport->saddr,
 					     asoc->base.sk);
-	} else
+	} else if ((transport->param_flags & SPP_PMTUD_DISABLE) &&
+		   asoc && asoc->pathmtu) {
+		transport->pathmtu = asoc->pathmtu;
+	} else {
 		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
+	}
 }
 
 /* Hold a reference to a transport.  */
-- 
2.14.3

[PATCH net-next 02/13] sctp: move transport pathmtu calc away of sctp_assoc_add_peer

[Marcelo Ricardo Leitner]

There was only one case that sctp_assoc_add_peer couldn't handle, which
is when SPP_PMTUD_DISABLE is set and pathmtu not initialized.
So add this situation to sctp_transport_route and reuse what was
already in there.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/associola.c | 9 +--------
 net/sctp/transport.c | 8 ++++++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a8f3b088fcb2ac82f58b7d6fb8894fe712e2bc0a..b3aa95222bd52113295cb246c503c903bdd5c353 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -652,15 +652,8 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 	 */
 	peer->param_flags = asoc->param_flags;
 
-	sctp_transport_route(peer, NULL, sp);
-
 	/* Initialize the pmtu of the transport. */
-	if (peer->param_flags & SPP_PMTUD_DISABLE) {
-		if (asoc->pathmtu)
-			peer->pathmtu = asoc->pathmtu;
-		else
-			peer->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
-	}
+	sctp_transport_route(peer, NULL, sp);
 
 	/* If this is the first transport addr on this association,
 	 * initialize the association PMTU to the peer's PMTU.
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 47f82bd794d915188bad037463c2aa14175a55ef..c5fc3aed08a18658448b528b07dbb13900686313 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -307,11 +307,15 @@ void sctp_transport_route(struct sctp_transport *transport,
 		 * association's active path for getsockname().
 		 */
 		if (asoc && (!asoc->peer.primary_path ||
-				(transport = asoc->peer.active_path)))
+			     (transport = asoc->peer.active_path)))
 			opt->pf->to_sk_saddr(&transport->saddr,
 					     asoc->base.sk);
-	} else
+	} else if ((transport->param_flags & SPP_PMTUD_DISABLE) &&
+		   asoc && asoc->pathmtu) {
+		transport->pathmtu = asoc->pathmtu;
+	} else {
 		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
+	}
 }
 
 /* Hold a reference to a transport.  */
-- 
2.14.3

[PATCH net-next 03/13] sctp: remove an if() that is always true

[Marcelo Ricardo Leitner]

As noticed by Xin Long, the if() here is always true as PMTU can never
be 0.

Reported-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/associola.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 			pmtu = t->pathmtu;
 	}
 
-	if (pmtu) {
-		asoc->pathmtu = pmtu;
-		asoc->frag_point = sctp_frag_point(asoc, pmtu);
-	}
+	asoc->pathmtu = pmtu;
+	asoc->frag_point = sctp_frag_point(asoc, pmtu);
 
 	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
 		 asoc->pathmtu, asoc->frag_point);
-- 
2.14.3

[PATCH net-next 03/13] sctp: remove an if() that is always true

[Marcelo Ricardo Leitner]

As noticed by Xin Long, the if() here is always true as PMTU can never
be 0.

Reported-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/associola.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 			pmtu = t->pathmtu;
 	}
 
-	if (pmtu) {
-		asoc->pathmtu = pmtu;
-		asoc->frag_point = sctp_frag_point(asoc, pmtu);
-	}
+	asoc->pathmtu = pmtu;
+	asoc->frag_point = sctp_frag_point(asoc, pmtu);
 
 	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
 		 asoc->pathmtu, asoc->frag_point);
-- 
2.14.3

Re: [PATCH net-next 03/13] sctp: remove an if() that is always true

[Neil Horman]

On Thu, Apr 26, 2018 at 04:58:52PM -0300, Marcelo Ricardo Leitner wrote:
> As noticed by Xin Long, the if() here is always true as PMTU can never
> be 0.
> 
> Reported-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  net/sctp/associola.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
>  			pmtu = t->pathmtu;
>  	}
>  
> -	if (pmtu) {
> -		asoc->pathmtu = pmtu;
> -		asoc->frag_point = sctp_frag_point(asoc, pmtu);
> -	}
> +	asoc->pathmtu = pmtu;
> +	asoc->frag_point = sctp_frag_point(asoc, pmtu);
>  
Can you double check this?  Looking at it, it seems far fetched, but if someone
sends a crafted icmp dest unreach message to the host, pmtu_sending might be
able to get set for an association (which may have no transports established
yet), and if so, on the first packet send sctp_assoc_sync_pmtu can be called,
leading to a fall through in the loop over all transports, and pmtu being zero.
It seems like a far fetched set of circumstances, I know, but if it can happen,
I think you might see a crash in sctp_frag_point due to an underflow of the frag
value

Neil

>  	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
>  		 asoc->pathmtu, asoc->frag_point);
> -- 
> 2.14.3
> 
> 

Re: [PATCH net-next 03/13] sctp: remove an if() that is always true

[Neil Horman]

On Thu, Apr 26, 2018 at 04:58:52PM -0300, Marcelo Ricardo Leitner wrote:
> As noticed by Xin Long, the if() here is always true as PMTU can never
> be 0.
> 
> Reported-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  net/sctp/associola.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
>  			pmtu = t->pathmtu;
>  	}
>  
> -	if (pmtu) {
> -		asoc->pathmtu = pmtu;
> -		asoc->frag_point = sctp_frag_point(asoc, pmtu);
> -	}
> +	asoc->pathmtu = pmtu;
> +	asoc->frag_point = sctp_frag_point(asoc, pmtu);
>  
Can you double check this?  Looking at it, it seems far fetched, but if someone
sends a crafted icmp dest unreach message to the host, pmtu_sending might be
able to get set for an association (which may have no transports established
yet), and if so, on the first packet send sctp_assoc_sync_pmtu can be called,
leading to a fall through in the loop over all transports, and pmtu being zero.
It seems like a far fetched set of circumstances, I know, but if it can happen,
I think you might see a crash in sctp_frag_point due to an underflow of the frag
value

Neil

>  	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
>  		 asoc->pathmtu, asoc->frag_point);
> -- 
> 2.14.3
> 
> 

Re: [PATCH net-next 03/13] sctp: remove an if() that is always true

[Marcelo Ricardo Leitner]

On Fri, Apr 27, 2018 at 06:50:50AM -0400, Neil Horman wrote:
> On Thu, Apr 26, 2018 at 04:58:52PM -0300, Marcelo Ricardo Leitner wrote:
> > As noticed by Xin Long, the if() here is always true as PMTU can never
> > be 0.
> >
> > Reported-by: Xin Long <lucien.xin@gmail.com>
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> >  net/sctp/associola.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> > index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
> > --- a/net/sctp/associola.c
> > +++ b/net/sctp/associola.c
> > @@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
> >  			pmtu = t->pathmtu;
> >  	}
> >
> > -	if (pmtu) {
> > -		asoc->pathmtu = pmtu;
> > -		asoc->frag_point = sctp_frag_point(asoc, pmtu);
> > -	}
> > +	asoc->pathmtu = pmtu;
> > +	asoc->frag_point = sctp_frag_point(asoc, pmtu);
> >
> Can you double check this?  Looking at it, it seems far fetched, but if someone

Sure.

> sends a crafted icmp dest unreach message to the host, pmtu_sending might be
> able to get set for an association (which may have no transports established
> yet), and if so, on the first packet send sctp_assoc_sync_pmtu can be called,
> leading to a fall through in the loop over all transports, and pmtu being zero.
> It seems like a far fetched set of circumstances, I know, but if it can happen,
> I think you might see a crash in sctp_frag_point due to an underflow of the frag
> value

If I got you right, this situation would not happen because when
handling the icmp it will check if there is a transport and ignore it
otherwise.

  Marcelo

>
> Neil
>
> >  	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
> >  		 asoc->pathmtu, asoc->frag_point);
> > --
> > 2.14.3
> >
> >
> --
> 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
>

Re: [PATCH net-next 03/13] sctp: remove an if() that is always true

[Marcelo Ricardo Leitner]

On Fri, Apr 27, 2018 at 06:50:50AM -0400, Neil Horman wrote:
> On Thu, Apr 26, 2018 at 04:58:52PM -0300, Marcelo Ricardo Leitner wrote:
> > As noticed by Xin Long, the if() here is always true as PMTU can never
> > be 0.
> >
> > Reported-by: Xin Long <lucien.xin@gmail.com>
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> >  net/sctp/associola.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> > index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
> > --- a/net/sctp/associola.c
> > +++ b/net/sctp/associola.c
> > @@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
> >  			pmtu = t->pathmtu;
> >  	}
> >
> > -	if (pmtu) {
> > -		asoc->pathmtu = pmtu;
> > -		asoc->frag_point = sctp_frag_point(asoc, pmtu);
> > -	}
> > +	asoc->pathmtu = pmtu;
> > +	asoc->frag_point = sctp_frag_point(asoc, pmtu);
> >
> Can you double check this?  Looking at it, it seems far fetched, but if someone

Sure.

> sends a crafted icmp dest unreach message to the host, pmtu_sending might be
> able to get set for an association (which may have no transports established
> yet), and if so, on the first packet send sctp_assoc_sync_pmtu can be called,
> leading to a fall through in the loop over all transports, and pmtu being zero.
> It seems like a far fetched set of circumstances, I know, but if it can happen,
> I think you might see a crash in sctp_frag_point due to an underflow of the frag
> value

If I got you right, this situation would not happen because when
handling the icmp it will check if there is a transport and ignore it
otherwise.

  Marcelo

>
> Neil
>
> >  	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
> >  		 asoc->pathmtu, asoc->frag_point);
> > --
> > 2.14.3
> >
> >
> --
> 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
>

Re: [PATCH net-next 03/13] sctp: remove an if() that is always true

[Neil Horman]

On Fri, Apr 27, 2018 at 03:13:49PM -0300, Marcelo Ricardo Leitner wrote:
> On Fri, Apr 27, 2018 at 06:50:50AM -0400, Neil Horman wrote:
> > On Thu, Apr 26, 2018 at 04:58:52PM -0300, Marcelo Ricardo Leitner wrote:
> > > As noticed by Xin Long, the if() here is always true as PMTU can never
> > > be 0.
> > >
> > > Reported-by: Xin Long <lucien.xin@gmail.com>
> > > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > ---
> > >  net/sctp/associola.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> > > index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
> > > --- a/net/sctp/associola.c
> > > +++ b/net/sctp/associola.c
> > > @@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
> > >  			pmtu = t->pathmtu;
> > >  	}
> > >
> > > -	if (pmtu) {
> > > -		asoc->pathmtu = pmtu;
> > > -		asoc->frag_point = sctp_frag_point(asoc, pmtu);
> > > -	}
> > > +	asoc->pathmtu = pmtu;
> > > +	asoc->frag_point = sctp_frag_point(asoc, pmtu);
> > >
> > Can you double check this?  Looking at it, it seems far fetched, but if someone
> 
> Sure.
> 
> > sends a crafted icmp dest unreach message to the host, pmtu_sending might be
> > able to get set for an association (which may have no transports established
> > yet), and if so, on the first packet send sctp_assoc_sync_pmtu can be called,
> > leading to a fall through in the loop over all transports, and pmtu being zero.
> > It seems like a far fetched set of circumstances, I know, but if it can happen,
> > I think you might see a crash in sctp_frag_point due to an underflow of the frag
> > value
> 
> If I got you right, this situation would not happen because when
> handling the icmp it will check if there is a transport and ignore it
> otherwise.
> 
Yup, you're right.  sctp_err_lookup returns NULL if there is no transport
associated with the icmp message

Thanks
Neil

>   Marcelo
> 
> >
> > Neil
> >
> > >  	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
> > >  		 asoc->pathmtu, asoc->frag_point);
> > > --
> > > 2.14.3
> > >
> > >
> > --
> > 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
> >
> 

Re: [PATCH net-next 03/13] sctp: remove an if() that is always true

[Neil Horman]

On Fri, Apr 27, 2018 at 03:13:49PM -0300, Marcelo Ricardo Leitner wrote:
> On Fri, Apr 27, 2018 at 06:50:50AM -0400, Neil Horman wrote:
> > On Thu, Apr 26, 2018 at 04:58:52PM -0300, Marcelo Ricardo Leitner wrote:
> > > As noticed by Xin Long, the if() here is always true as PMTU can never
> > > be 0.
> > >
> > > Reported-by: Xin Long <lucien.xin@gmail.com>
> > > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > ---
> > >  net/sctp/associola.c | 6 ++----
> > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> > > index b3aa95222bd52113295cb246c503c903bdd5c353..c5ed09cfa8423b17546e3d45f6d06db03af66384 100644
> > > --- a/net/sctp/associola.c
> > > +++ b/net/sctp/associola.c
> > > @@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
> > >  			pmtu = t->pathmtu;
> > >  	}
> > >
> > > -	if (pmtu) {
> > > -		asoc->pathmtu = pmtu;
> > > -		asoc->frag_point = sctp_frag_point(asoc, pmtu);
> > > -	}
> > > +	asoc->pathmtu = pmtu;
> > > +	asoc->frag_point = sctp_frag_point(asoc, pmtu);
> > >
> > Can you double check this?  Looking at it, it seems far fetched, but if someone
> 
> Sure.
> 
> > sends a crafted icmp dest unreach message to the host, pmtu_sending might be
> > able to get set for an association (which may have no transports established
> > yet), and if so, on the first packet send sctp_assoc_sync_pmtu can be called,
> > leading to a fall through in the loop over all transports, and pmtu being zero.
> > It seems like a far fetched set of circumstances, I know, but if it can happen,
> > I think you might see a crash in sctp_frag_point due to an underflow of the frag
> > value
> 
> If I got you right, this situation would not happen because when
> handling the icmp it will check if there is a transport and ignore it
> otherwise.
> 
Yup, you're right.  sctp_err_lookup returns NULL if there is no transport
associated with the icmp message

Thanks
Neil

>   Marcelo
> 
> >
> > Neil
> >
> > >  	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
> > >  		 asoc->pathmtu, asoc->frag_point);
> > > --
> > > 2.14.3
> > >
> > >
> > --
> > 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
> >
> 

[PATCH net-next 04/13] sctp: introduce sctp_assoc_set_pmtu

[Marcelo Ricardo Leitner]

All changes to asoc PMTU should now go through this wrapper, making it
easier to track them and to do other actions upon it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/structs.h |  1 +
 net/sctp/associola.c       | 21 +++++++++++++--------
 net/sctp/socket.c          |  2 +-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 05594b248e5277790cca6542c3680510739ce18b..c5e244be8f1ea99118cac9ada0cfce871cb15d3f 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2097,6 +2097,7 @@ int sctp_assoc_update(struct sctp_association *old,
 
 __u32 sctp_association_get_next_tsn(struct sctp_association *);
 
+void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu);
 void sctp_assoc_sync_pmtu(struct sctp_association *asoc);
 void sctp_assoc_rwnd_increase(struct sctp_association *, unsigned int);
 void sctp_assoc_rwnd_decrease(struct sctp_association *, unsigned int);
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index c5ed09cfa8423b17546e3d45f6d06db03af66384..85b362324084e9ba139a39293a527ef06824e55b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -660,13 +660,9 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 	 * If not and the current association PMTU is higher than the new
 	 * peer's PMTU, reset the association PMTU to the new peer's PMTU.
 	 */
-	if (asoc->pathmtu)
-		asoc->pathmtu = min_t(int, peer->pathmtu, asoc->pathmtu);
-	else
-		asoc->pathmtu = peer->pathmtu;
-
-	pr_debug("%s: association:%p PMTU set to %d\n", __func__, asoc,
-		 asoc->pathmtu);
+	sctp_assoc_set_pmtu(asoc, asoc->pathmtu ?
+				  min_t(int, peer->pathmtu, asoc->pathmtu) :
+				  peer->pathmtu);
 
 	peer->pmtu_pending = 0;
 
@@ -1374,6 +1370,15 @@ sctp_assoc_choose_alter_transport(struct sctp_association *asoc,
 	}
 }
 
+void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu)
+{
+	if (asoc->pathmtu != pmtu)
+		asoc->pathmtu = pmtu;
+
+	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
+		 asoc->pathmtu, asoc->frag_point);
+}
+
 /* Update the association's pmtu and frag_point by going through all the
  * transports. This routine is called when a transport's PMTU has changed.
  */
@@ -1397,7 +1402,7 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 			pmtu = t->pathmtu;
 	}
 
-	asoc->pathmtu = pmtu;
+	sctp_assoc_set_pmtu(asoc, pmtu);
 	asoc->frag_point = sctp_frag_point(asoc, pmtu);
 
 	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 80835ac26d2c3ce6559f75aaaa0b315fb77d9adf..eeec81d5c485b86076f4e3a4f5dc01fc28fbb91b 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2539,7 +2539,7 @@ static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
 			trans->pathmtu = params->spp_pathmtu;
 			sctp_assoc_sync_pmtu(asoc);
 		} else if (asoc) {
-			asoc->pathmtu = params->spp_pathmtu;
+			sctp_assoc_set_pmtu(asoc, params->spp_pathmtu);
 		} else {
 			sp->pathmtu = params->spp_pathmtu;
 		}
-- 
2.14.3

[PATCH net-next 04/13] sctp: introduce sctp_assoc_set_pmtu

[Marcelo Ricardo Leitner]

All changes to asoc PMTU should now go through this wrapper, making it
easier to track them and to do other actions upon it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/structs.h |  1 +
 net/sctp/associola.c       | 21 +++++++++++++--------
 net/sctp/socket.c          |  2 +-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 05594b248e5277790cca6542c3680510739ce18b..c5e244be8f1ea99118cac9ada0cfce871cb15d3f 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2097,6 +2097,7 @@ int sctp_assoc_update(struct sctp_association *old,
 
 __u32 sctp_association_get_next_tsn(struct sctp_association *);
 
+void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu);
 void sctp_assoc_sync_pmtu(struct sctp_association *asoc);
 void sctp_assoc_rwnd_increase(struct sctp_association *, unsigned int);
 void sctp_assoc_rwnd_decrease(struct sctp_association *, unsigned int);
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index c5ed09cfa8423b17546e3d45f6d06db03af66384..85b362324084e9ba139a39293a527ef06824e55b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -660,13 +660,9 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 	 * If not and the current association PMTU is higher than the new
 	 * peer's PMTU, reset the association PMTU to the new peer's PMTU.
 	 */
-	if (asoc->pathmtu)
-		asoc->pathmtu = min_t(int, peer->pathmtu, asoc->pathmtu);
-	else
-		asoc->pathmtu = peer->pathmtu;
-
-	pr_debug("%s: association:%p PMTU set to %d\n", __func__, asoc,
-		 asoc->pathmtu);
+	sctp_assoc_set_pmtu(asoc, asoc->pathmtu ?
+				  min_t(int, peer->pathmtu, asoc->pathmtu) :
+				  peer->pathmtu);
 
 	peer->pmtu_pending = 0;
 
@@ -1374,6 +1370,15 @@ sctp_assoc_choose_alter_transport(struct sctp_association *asoc,
 	}
 }
 
+void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu)
+{
+	if (asoc->pathmtu != pmtu)
+		asoc->pathmtu = pmtu;
+
+	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
+		 asoc->pathmtu, asoc->frag_point);
+}
+
 /* Update the association's pmtu and frag_point by going through all the
  * transports. This routine is called when a transport's PMTU has changed.
  */
@@ -1397,7 +1402,7 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 			pmtu = t->pathmtu;
 	}
 
-	asoc->pathmtu = pmtu;
+	sctp_assoc_set_pmtu(asoc, pmtu);
 	asoc->frag_point = sctp_frag_point(asoc, pmtu);
 
 	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 80835ac26d2c3ce6559f75aaaa0b315fb77d9adf..eeec81d5c485b86076f4e3a4f5dc01fc28fbb91b 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2539,7 +2539,7 @@ static int sctp_apply_peer_addr_params(struct sctp_paddrparams *params,
 			trans->pathmtu = params->spp_pathmtu;
 			sctp_assoc_sync_pmtu(asoc);
 		} else if (asoc) {
-			asoc->pathmtu = params->spp_pathmtu;
+			sctp_assoc_set_pmtu(asoc, params->spp_pathmtu);
 		} else {
 			sp->pathmtu = params->spp_pathmtu;
 		}
-- 
2.14.3

[PATCH net-next 05/13] sctp: introduce sctp_mtu_payload

[Marcelo Ricardo Leitner]

When given a MTU, this function calculates how much payload we can carry
on it. Without a MTU, it calculates the amount of header overhead we
have.

So that when we have extra overhead, like the one added for IP options
on SELinux patches, it is easier to handle it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 19 +++++++++++++++++++
 net/sctp/output.c       | 25 ++++++++++---------------
 net/sctp/socket.c       |  7 ++-----
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 28b996d6349072fb031de91e10115b688d89b42d..0b98e4683f108670c65a0a7aceaf9a736d54bf29 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -607,6 +607,25 @@ static inline struct dst_entry *sctp_transport_dst_check(struct sctp_transport *
 	return t->dst;
 }
 
+/* Calculate max payload size given a MTU, or the total overhead if
+ * given MTU is zero
+ */
+static inline __u32 sctp_mtu_payload(const struct sctp_sock *sp,
+				     __u32 mtu, __u32 extra)
+{
+	__u32 overhead = sizeof(struct sctphdr) + extra;
+
+	if (sp)
+		overhead += sp->pf->af->net_header_len;
+	else
+		overhead += sizeof(struct ipv6hdr);
+
+	if (WARN_ON_ONCE(mtu && mtu <= overhead))
+		mtu = overhead;
+
+	return mtu ? mtu - overhead : overhead;
+}
+
 static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
 {
 	__u32 pmtu = max_t(size_t, SCTP_TRUNC4(dst_mtu(t->dst)),
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 690d8557bb7bfe20a0a209b6c937c85c764a3bd0..bf4226c3cc1de79f2041d55a68ea53f4fd600b25 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -90,8 +90,8 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
 {
 	struct sctp_transport *tp = packet->transport;
 	struct sctp_association *asoc = tp->asoc;
+	struct sctp_sock *sp = NULL;
 	struct sock *sk;
-	size_t overhead = sizeof(struct ipv6hdr) + sizeof(struct sctphdr);
 
 	pr_debug("%s: packet:%p vtag:0x%x\n", __func__, packet, vtag);
 	packet->vtag = vtag;
@@ -102,25 +102,20 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
 
 	/* set packet max_size with pathmtu, then calculate overhead */
 	packet->max_size = tp->pathmtu;
+
 	if (asoc) {
-		struct sctp_sock *sp = sctp_sk(asoc->base.sk);
-		struct sctp_af *af = sp->pf->af;
-
-		overhead = af->net_header_len +
-			   af->ip_options_len(asoc->base.sk);
-		overhead += sizeof(struct sctphdr);
-		packet->overhead = overhead;
-		packet->size = overhead;
-	} else {
-		packet->overhead = overhead;
-		packet->size = overhead;
-		return;
+		sk = asoc->base.sk;
+		sp = sctp_sk(sk);
 	}
+	packet->overhead = sctp_mtu_payload(sp, 0, 0);
+	packet->size = packet->overhead;
+
+	if (!asoc)
+		return;
 
 	/* update dst or transport pathmtu if in need */
-	sk = asoc->base.sk;
 	if (!sctp_transport_dst_check(tp)) {
-		sctp_transport_route(tp, NULL, sctp_sk(sk));
+		sctp_transport_route(tp, NULL, sp);
 		if (asoc->param_flags & SPP_PMTUD_ENABLE)
 			sctp_assoc_sync_pmtu(asoc);
 	} else if (!sctp_transport_pmtu_check(tp)) {
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index eeec81d5c485b86076f4e3a4f5dc01fc28fbb91b..b9d14f57146b8e551c28379e379158231fb40ae7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3234,11 +3234,8 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 	if (val) {
 		int min_len, max_len;
 
-		min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len;
-		min_len -= af->ip_options_len(sk);
-		min_len -= sizeof(struct sctphdr) +
-			   sizeof(struct sctp_data_chunk);
-
+		min_len = sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT,
+					   sizeof(struct sctp_data_chunk));
 		max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
 
 		if (val < min_len || val > max_len)
-- 
2.14.3

[PATCH net-next 05/13] sctp: introduce sctp_mtu_payload

[Marcelo Ricardo Leitner]

When given a MTU, this function calculates how much payload we can carry
on it. Without a MTU, it calculates the amount of header overhead we
have.

So that when we have extra overhead, like the one added for IP options
on SELinux patches, it is easier to handle it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 19 +++++++++++++++++++
 net/sctp/output.c       | 25 ++++++++++---------------
 net/sctp/socket.c       |  7 ++-----
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 28b996d6349072fb031de91e10115b688d89b42d..0b98e4683f108670c65a0a7aceaf9a736d54bf29 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -607,6 +607,25 @@ static inline struct dst_entry *sctp_transport_dst_check(struct sctp_transport *
 	return t->dst;
 }
 
+/* Calculate max payload size given a MTU, or the total overhead if
+ * given MTU is zero
+ */
+static inline __u32 sctp_mtu_payload(const struct sctp_sock *sp,
+				     __u32 mtu, __u32 extra)
+{
+	__u32 overhead = sizeof(struct sctphdr) + extra;
+
+	if (sp)
+		overhead += sp->pf->af->net_header_len;
+	else
+		overhead += sizeof(struct ipv6hdr);
+
+	if (WARN_ON_ONCE(mtu && mtu <= overhead))
+		mtu = overhead;
+
+	return mtu ? mtu - overhead : overhead;
+}
+
 static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
 {
 	__u32 pmtu = max_t(size_t, SCTP_TRUNC4(dst_mtu(t->dst)),
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 690d8557bb7bfe20a0a209b6c937c85c764a3bd0..bf4226c3cc1de79f2041d55a68ea53f4fd600b25 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -90,8 +90,8 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
 {
 	struct sctp_transport *tp = packet->transport;
 	struct sctp_association *asoc = tp->asoc;
+	struct sctp_sock *sp = NULL;
 	struct sock *sk;
-	size_t overhead = sizeof(struct ipv6hdr) + sizeof(struct sctphdr);
 
 	pr_debug("%s: packet:%p vtag:0x%x\n", __func__, packet, vtag);
 	packet->vtag = vtag;
@@ -102,25 +102,20 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
 
 	/* set packet max_size with pathmtu, then calculate overhead */
 	packet->max_size = tp->pathmtu;
+
 	if (asoc) {
-		struct sctp_sock *sp = sctp_sk(asoc->base.sk);
-		struct sctp_af *af = sp->pf->af;
-
-		overhead = af->net_header_len +
-			   af->ip_options_len(asoc->base.sk);
-		overhead += sizeof(struct sctphdr);
-		packet->overhead = overhead;
-		packet->size = overhead;
-	} else {
-		packet->overhead = overhead;
-		packet->size = overhead;
-		return;
+		sk = asoc->base.sk;
+		sp = sctp_sk(sk);
 	}
+	packet->overhead = sctp_mtu_payload(sp, 0, 0);
+	packet->size = packet->overhead;
+
+	if (!asoc)
+		return;
 
 	/* update dst or transport pathmtu if in need */
-	sk = asoc->base.sk;
 	if (!sctp_transport_dst_check(tp)) {
-		sctp_transport_route(tp, NULL, sctp_sk(sk));
+		sctp_transport_route(tp, NULL, sp);
 		if (asoc->param_flags & SPP_PMTUD_ENABLE)
 			sctp_assoc_sync_pmtu(asoc);
 	} else if (!sctp_transport_pmtu_check(tp)) {
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index eeec81d5c485b86076f4e3a4f5dc01fc28fbb91b..b9d14f57146b8e551c28379e379158231fb40ae7 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3234,11 +3234,8 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 	if (val) {
 		int min_len, max_len;
 
-		min_len = SCTP_DEFAULT_MINSEGMENT - af->net_header_len;
-		min_len -= af->ip_options_len(sk);
-		min_len -= sizeof(struct sctphdr) +
-			   sizeof(struct sctp_data_chunk);
-
+		min_len = sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT,
+					   sizeof(struct sctp_data_chunk));
 		max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
 
 		if (val < min_len || val > max_len)
-- 
2.14.3

[PATCH net-next 06/13] sctp: introduce sctp_assoc_update_frag_point

[Marcelo Ricardo Leitner]

and avoid the open-coded versions of it.

Now sctp_datamsg_from_user can just re-use asoc->frag_point as it will
always be updated.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h    | 20 --------------------
 include/net/sctp/structs.h |  1 +
 net/sctp/associola.c       | 24 +++++++++++++++++-------
 net/sctp/chunk.c           | 12 +-----------
 net/sctp/socket.c          |  2 +-
 5 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 0b98e4683f108670c65a0a7aceaf9a736d54bf29..350c65620a4eb35ba2af0d59fef70e2b8b461e44 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -428,26 +428,6 @@ static inline int sctp_list_single_entry(struct list_head *head)
 	return (head->next != head) && (head->next == head->prev);
 }
 
-/* Break down data chunks at this point.  */
-static inline int sctp_frag_point(const struct sctp_association *asoc, int pmtu)
-{
-	struct sctp_sock *sp = sctp_sk(asoc->base.sk);
-	struct sctp_af *af = sp->pf->af;
-	int frag = pmtu;
-
-	frag -= af->ip_options_len(asoc->base.sk);
-	frag -= af->net_header_len;
-	frag -= sizeof(struct sctphdr) + sctp_datachk_len(&asoc->stream);
-
-	if (asoc->user_frag)
-		frag = min_t(int, frag, asoc->user_frag);
-
-	frag = SCTP_TRUNC4(min_t(int, frag, SCTP_MAX_CHUNK_LEN -
-					    sctp_datachk_len(&asoc->stream)));
-
-	return frag;
-}
-
 static inline void sctp_assoc_pending_pmtu(struct sctp_association *asoc)
 {
 	sctp_assoc_sync_pmtu(asoc);
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c5e244be8f1ea99118cac9ada0cfce871cb15d3f..ebf809eed33add7905ddd13abf98712a833ae0e0 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2097,6 +2097,7 @@ int sctp_assoc_update(struct sctp_association *old,
 
 __u32 sctp_association_get_next_tsn(struct sctp_association *);
 
+void sctp_assoc_update_frag_point(struct sctp_association *asoc);
 void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu);
 void sctp_assoc_sync_pmtu(struct sctp_association *asoc);
 void sctp_assoc_rwnd_increase(struct sctp_association *, unsigned int);
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 85b362324084e9ba139a39293a527ef06824e55b..a29025418b96015dd0240013b0a7c636194da67b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -666,8 +666,6 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 
 	peer->pmtu_pending = 0;
 
-	asoc->frag_point = sctp_frag_point(asoc, asoc->pathmtu);
-
 	/* The asoc->peer.port might not be meaningful yet, but
 	 * initialize the packet structure anyway.
 	 */
@@ -1370,10 +1368,26 @@ sctp_assoc_choose_alter_transport(struct sctp_association *asoc,
 	}
 }
 
+void sctp_assoc_update_frag_point(struct sctp_association *asoc)
+{
+	int frag = sctp_mtu_payload(sctp_sk(asoc->base.sk), asoc->pathmtu,
+				    sctp_datachk_len(&asoc->stream));
+
+	if (asoc->user_frag)
+		frag = min_t(int, frag, asoc->user_frag);
+
+	frag = min_t(int, frag, SCTP_MAX_CHUNK_LEN -
+				sctp_datachk_len(&asoc->stream));
+
+	asoc->frag_point = SCTP_TRUNC4(frag);
+}
+
 void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu)
 {
-	if (asoc->pathmtu != pmtu)
+	if (asoc->pathmtu != pmtu) {
 		asoc->pathmtu = pmtu;
+		sctp_assoc_update_frag_point(asoc);
+	}
 
 	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
 		 asoc->pathmtu, asoc->frag_point);
@@ -1403,10 +1417,6 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 	}
 
 	sctp_assoc_set_pmtu(asoc, pmtu);
-	asoc->frag_point = sctp_frag_point(asoc, pmtu);
-
-	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
-		 asoc->pathmtu, asoc->frag_point);
 }
 
 /* Should we send a SACK to update our peer? */
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index be296d633e951c8df322d560150973b5b2bdce5b..79daa98208c391c780440144d69bc7be875c3476 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -172,8 +172,6 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 	struct list_head *pos, *temp;
 	struct sctp_chunk *chunk;
 	struct sctp_datamsg *msg;
-	struct sctp_sock *sp;
-	struct sctp_af *af;
 	int err;
 
 	msg = sctp_datamsg_new(GFP_KERNEL);
@@ -192,12 +190,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 	/* This is the biggest possible DATA chunk that can fit into
 	 * the packet
 	 */
-	sp = sctp_sk(asoc->base.sk);
-	af = sp->pf->af;
-	max_data = asoc->pathmtu - af->net_header_len -
-		   sizeof(struct sctphdr) - sctp_datachk_len(&asoc->stream) -
-		   af->ip_options_len(asoc->base.sk);
-	max_data = SCTP_TRUNC4(max_data);
+	max_data = asoc->frag_point;
 
 	/* If the the peer requested that we authenticate DATA chunks
 	 * we need to account for bundling of the AUTH chunks along with
@@ -222,9 +215,6 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 		}
 	}
 
-	/* Check what's our max considering the above */
-	max_data = min_t(size_t, max_data, asoc->frag_point);
-
 	/* Set first_len and then account for possible bundles on first frag */
 	first_len = max_data;
 
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b9d14f57146b8e551c28379e379158231fb40ae7..21bf457be3ea321457107256c12ccb93e1fbab06 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3251,7 +3251,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 			       sctp_datachk_len(&asoc->stream);
 		}
 		asoc->user_frag = val;
-		asoc->frag_point = sctp_frag_point(asoc, asoc->pathmtu);
+		sctp_assoc_update_frag_point(asoc);
 	} else {
 		if (params.assoc_id && sctp_style(sk, UDP))
 			return -EINVAL;
-- 
2.14.3

[PATCH net-next 06/13] sctp: introduce sctp_assoc_update_frag_point

[Marcelo Ricardo Leitner]

and avoid the open-coded versions of it.

Now sctp_datamsg_from_user can just re-use asoc->frag_point as it will
always be updated.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h    | 20 --------------------
 include/net/sctp/structs.h |  1 +
 net/sctp/associola.c       | 24 +++++++++++++++++-------
 net/sctp/chunk.c           | 12 +-----------
 net/sctp/socket.c          |  2 +-
 5 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 0b98e4683f108670c65a0a7aceaf9a736d54bf29..350c65620a4eb35ba2af0d59fef70e2b8b461e44 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -428,26 +428,6 @@ static inline int sctp_list_single_entry(struct list_head *head)
 	return (head->next != head) && (head->next = head->prev);
 }
 
-/* Break down data chunks at this point.  */
-static inline int sctp_frag_point(const struct sctp_association *asoc, int pmtu)
-{
-	struct sctp_sock *sp = sctp_sk(asoc->base.sk);
-	struct sctp_af *af = sp->pf->af;
-	int frag = pmtu;
-
-	frag -= af->ip_options_len(asoc->base.sk);
-	frag -= af->net_header_len;
-	frag -= sizeof(struct sctphdr) + sctp_datachk_len(&asoc->stream);
-
-	if (asoc->user_frag)
-		frag = min_t(int, frag, asoc->user_frag);
-
-	frag = SCTP_TRUNC4(min_t(int, frag, SCTP_MAX_CHUNK_LEN -
-					    sctp_datachk_len(&asoc->stream)));
-
-	return frag;
-}
-
 static inline void sctp_assoc_pending_pmtu(struct sctp_association *asoc)
 {
 	sctp_assoc_sync_pmtu(asoc);
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c5e244be8f1ea99118cac9ada0cfce871cb15d3f..ebf809eed33add7905ddd13abf98712a833ae0e0 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2097,6 +2097,7 @@ int sctp_assoc_update(struct sctp_association *old,
 
 __u32 sctp_association_get_next_tsn(struct sctp_association *);
 
+void sctp_assoc_update_frag_point(struct sctp_association *asoc);
 void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu);
 void sctp_assoc_sync_pmtu(struct sctp_association *asoc);
 void sctp_assoc_rwnd_increase(struct sctp_association *, unsigned int);
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 85b362324084e9ba139a39293a527ef06824e55b..a29025418b96015dd0240013b0a7c636194da67b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -666,8 +666,6 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 
 	peer->pmtu_pending = 0;
 
-	asoc->frag_point = sctp_frag_point(asoc, asoc->pathmtu);
-
 	/* The asoc->peer.port might not be meaningful yet, but
 	 * initialize the packet structure anyway.
 	 */
@@ -1370,10 +1368,26 @@ sctp_assoc_choose_alter_transport(struct sctp_association *asoc,
 	}
 }
 
+void sctp_assoc_update_frag_point(struct sctp_association *asoc)
+{
+	int frag = sctp_mtu_payload(sctp_sk(asoc->base.sk), asoc->pathmtu,
+				    sctp_datachk_len(&asoc->stream));
+
+	if (asoc->user_frag)
+		frag = min_t(int, frag, asoc->user_frag);
+
+	frag = min_t(int, frag, SCTP_MAX_CHUNK_LEN -
+				sctp_datachk_len(&asoc->stream));
+
+	asoc->frag_point = SCTP_TRUNC4(frag);
+}
+
 void sctp_assoc_set_pmtu(struct sctp_association *asoc, __u32 pmtu)
 {
-	if (asoc->pathmtu != pmtu)
+	if (asoc->pathmtu != pmtu) {
 		asoc->pathmtu = pmtu;
+		sctp_assoc_update_frag_point(asoc);
+	}
 
 	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
 		 asoc->pathmtu, asoc->frag_point);
@@ -1403,10 +1417,6 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 	}
 
 	sctp_assoc_set_pmtu(asoc, pmtu);
-	asoc->frag_point = sctp_frag_point(asoc, pmtu);
-
-	pr_debug("%s: asoc:%p, pmtu:%d, frag_point:%d\n", __func__, asoc,
-		 asoc->pathmtu, asoc->frag_point);
 }
 
 /* Should we send a SACK to update our peer? */
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index be296d633e951c8df322d560150973b5b2bdce5b..79daa98208c391c780440144d69bc7be875c3476 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -172,8 +172,6 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 	struct list_head *pos, *temp;
 	struct sctp_chunk *chunk;
 	struct sctp_datamsg *msg;
-	struct sctp_sock *sp;
-	struct sctp_af *af;
 	int err;
 
 	msg = sctp_datamsg_new(GFP_KERNEL);
@@ -192,12 +190,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 	/* This is the biggest possible DATA chunk that can fit into
 	 * the packet
 	 */
-	sp = sctp_sk(asoc->base.sk);
-	af = sp->pf->af;
-	max_data = asoc->pathmtu - af->net_header_len -
-		   sizeof(struct sctphdr) - sctp_datachk_len(&asoc->stream) -
-		   af->ip_options_len(asoc->base.sk);
-	max_data = SCTP_TRUNC4(max_data);
+	max_data = asoc->frag_point;
 
 	/* If the the peer requested that we authenticate DATA chunks
 	 * we need to account for bundling of the AUTH chunks along with
@@ -222,9 +215,6 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 		}
 	}
 
-	/* Check what's our max considering the above */
-	max_data = min_t(size_t, max_data, asoc->frag_point);
-
 	/* Set first_len and then account for possible bundles on first frag */
 	first_len = max_data;
 
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index b9d14f57146b8e551c28379e379158231fb40ae7..21bf457be3ea321457107256c12ccb93e1fbab06 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3251,7 +3251,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 			       sctp_datachk_len(&asoc->stream);
 		}
 		asoc->user_frag = val;
-		asoc->frag_point = sctp_frag_point(asoc, asoc->pathmtu);
+		sctp_assoc_update_frag_point(asoc);
 	} else {
 		if (params.assoc_id && sctp_style(sk, UDP))
 			return -EINVAL;
-- 
2.14.3

[PATCH net-next 07/13] sctp: remove sctp_assoc_pending_pmtu

[Marcelo Ricardo Leitner]

No need for this helper.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 6 ------
 net/sctp/socket.c       | 6 ++++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 350c65620a4eb35ba2af0d59fef70e2b8b461e44..e327acad8e7d0710e30f32d0829116593c1171cf 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -428,12 +428,6 @@ static inline int sctp_list_single_entry(struct list_head *head)
 	return (head->next != head) && (head->next == head->prev);
 }
 
-static inline void sctp_assoc_pending_pmtu(struct sctp_association *asoc)
-{
-	sctp_assoc_sync_pmtu(asoc);
-	asoc->pmtu_pending = 0;
-}
-
 static inline bool sctp_chunk_pending(const struct sctp_chunk *chunk)
 {
 	return !list_empty(&chunk->list);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 21bf457be3ea321457107256c12ccb93e1fbab06..a93b60a28cc5fff0ebe299905fb31cb37de43d88 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1918,8 +1918,10 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 		goto err;
 	}
 
-	if (asoc->pmtu_pending)
-		sctp_assoc_pending_pmtu(asoc);
+	if (asoc->pmtu_pending) {
+		sctp_assoc_sync_pmtu(asoc);
+		asoc->pmtu_pending = 0;
+	}
 
 	if (sctp_wspace(asoc) < msg_len)
 		sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc));
-- 
2.14.3

[PATCH net-next 07/13] sctp: remove sctp_assoc_pending_pmtu

[Marcelo Ricardo Leitner]

No need for this helper.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 6 ------
 net/sctp/socket.c       | 6 ++++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 350c65620a4eb35ba2af0d59fef70e2b8b461e44..e327acad8e7d0710e30f32d0829116593c1171cf 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -428,12 +428,6 @@ static inline int sctp_list_single_entry(struct list_head *head)
 	return (head->next != head) && (head->next = head->prev);
 }
 
-static inline void sctp_assoc_pending_pmtu(struct sctp_association *asoc)
-{
-	sctp_assoc_sync_pmtu(asoc);
-	asoc->pmtu_pending = 0;
-}
-
 static inline bool sctp_chunk_pending(const struct sctp_chunk *chunk)
 {
 	return !list_empty(&chunk->list);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 21bf457be3ea321457107256c12ccb93e1fbab06..a93b60a28cc5fff0ebe299905fb31cb37de43d88 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1918,8 +1918,10 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 		goto err;
 	}
 
-	if (asoc->pmtu_pending)
-		sctp_assoc_pending_pmtu(asoc);
+	if (asoc->pmtu_pending) {
+		sctp_assoc_sync_pmtu(asoc);
+		asoc->pmtu_pending = 0;
+	}
 
 	if (sctp_wspace(asoc) < msg_len)
 		sctp_prsctp_prune(asoc, sinfo, msg_len - sctp_wspace(asoc));
-- 
2.14.3

[PATCH net-next 08/13] sctp: introduce sctp_dst_mtu

[Marcelo Ricardo Leitner]

Which makes sure that the MTU respects the minimum value of
SCTP_DEFAULT_MINSEGMENT and that it is correctly aligned.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 9 +++++++--
 net/sctp/associola.c    | 6 ++----
 net/sctp/transport.c    | 6 +++---
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index e327acad8e7d0710e30f32d0829116593c1171cf..4965cbfa7d92c0f60a76dd6e03571f78209dda5a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -600,10 +600,15 @@ static inline __u32 sctp_mtu_payload(const struct sctp_sock *sp,
 	return mtu ? mtu - overhead : overhead;
 }
 
+static inline __u32 sctp_dst_mtu(const struct dst_entry *dst)
+{
+	return SCTP_TRUNC4(max_t(__u32, dst_mtu(dst),
+				 SCTP_DEFAULT_MINSEGMENT));
+}
+
 static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
 {
-	__u32 pmtu = max_t(size_t, SCTP_TRUNC4(dst_mtu(t->dst)),
-			   SCTP_DEFAULT_MINSEGMENT);
+	__u32 pmtu = sctp_dst_mtu(t->dst);
 
 	if (t->pathmtu == pmtu)
 		return true;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a29025418b96015dd0240013b0a7c636194da67b..039fdb862b1797686bf7e5e911379b5b47ea58f7 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1405,11 +1405,9 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 		return;
 
 	/* Get the lowest pmtu of all the transports. */
-	list_for_each_entry(t, &asoc->peer.transport_addr_list,
-				transports) {
+	list_for_each_entry(t, &asoc->peer.transport_addr_list, transports) {
 		if (t->pmtu_pending && t->dst) {
-			sctp_transport_update_pmtu(
-					t, SCTP_TRUNC4(dst_mtu(t->dst)));
+			sctp_transport_update_pmtu(t, sctp_dst_mtu(t->dst));
 			t->pmtu_pending = 0;
 		}
 		if (!pmtu || (t->pathmtu < pmtu))
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index c5fc3aed08a18658448b528b07dbb13900686313..ed73a9d91b83eadd3eb8d681fda104fe71562c3f 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -242,9 +242,9 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 						&transport->fl, sk);
 	}
 
-	if (transport->dst) {
-		transport->pathmtu = SCTP_TRUNC4(dst_mtu(transport->dst));
-	} else
+	if (transport->dst)
+		transport->pathmtu = sctp_dst_mtu(transport->dst);
+	else
 		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
 }
 
-- 
2.14.3

[PATCH net-next 08/13] sctp: introduce sctp_dst_mtu

[Marcelo Ricardo Leitner]

Which makes sure that the MTU respects the minimum value of
SCTP_DEFAULT_MINSEGMENT and that it is correctly aligned.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 9 +++++++--
 net/sctp/associola.c    | 6 ++----
 net/sctp/transport.c    | 6 +++---
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index e327acad8e7d0710e30f32d0829116593c1171cf..4965cbfa7d92c0f60a76dd6e03571f78209dda5a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -600,10 +600,15 @@ static inline __u32 sctp_mtu_payload(const struct sctp_sock *sp,
 	return mtu ? mtu - overhead : overhead;
 }
 
+static inline __u32 sctp_dst_mtu(const struct dst_entry *dst)
+{
+	return SCTP_TRUNC4(max_t(__u32, dst_mtu(dst),
+				 SCTP_DEFAULT_MINSEGMENT));
+}
+
 static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
 {
-	__u32 pmtu = max_t(size_t, SCTP_TRUNC4(dst_mtu(t->dst)),
-			   SCTP_DEFAULT_MINSEGMENT);
+	__u32 pmtu = sctp_dst_mtu(t->dst);
 
 	if (t->pathmtu = pmtu)
 		return true;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a29025418b96015dd0240013b0a7c636194da67b..039fdb862b1797686bf7e5e911379b5b47ea58f7 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1405,11 +1405,9 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
 		return;
 
 	/* Get the lowest pmtu of all the transports. */
-	list_for_each_entry(t, &asoc->peer.transport_addr_list,
-				transports) {
+	list_for_each_entry(t, &asoc->peer.transport_addr_list, transports) {
 		if (t->pmtu_pending && t->dst) {
-			sctp_transport_update_pmtu(
-					t, SCTP_TRUNC4(dst_mtu(t->dst)));
+			sctp_transport_update_pmtu(t, sctp_dst_mtu(t->dst));
 			t->pmtu_pending = 0;
 		}
 		if (!pmtu || (t->pathmtu < pmtu))
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index c5fc3aed08a18658448b528b07dbb13900686313..ed73a9d91b83eadd3eb8d681fda104fe71562c3f 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -242,9 +242,9 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 						&transport->fl, sk);
 	}
 
-	if (transport->dst) {
-		transport->pathmtu = SCTP_TRUNC4(dst_mtu(transport->dst));
-	} else
+	if (transport->dst)
+		transport->pathmtu = sctp_dst_mtu(transport->dst);
+	else
 		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
 }
 
-- 
2.14.3

[PATCH net-next 09/13] sctp: remove sctp_transport_pmtu_check

[Marcelo Ricardo Leitner]

We are now keeping the MTU information synced between asoc, transport
and dst, which makes the check at sctp_packet_config() not needed
anymore. As it was the sole caller to this function, lets remove it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 12 ------------
 net/sctp/output.c       |  3 ---
 2 files changed, 15 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 4965cbfa7d92c0f60a76dd6e03571f78209dda5a..f66d4435000799f523fbaa34b0a57dbdeebda040 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -606,16 +606,4 @@ static inline __u32 sctp_dst_mtu(const struct dst_entry *dst)
 				 SCTP_DEFAULT_MINSEGMENT));
 }
 
-static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
-{
-	__u32 pmtu = sctp_dst_mtu(t->dst);
-
-	if (t->pathmtu == pmtu)
-		return true;
-
-	t->pathmtu = pmtu;
-
-	return false;
-}
-
 #endif /* __net_sctp_h__ */
diff --git a/net/sctp/output.c b/net/sctp/output.c
index bf4226c3cc1de79f2041d55a68ea53f4fd600b25..e672dee302c7092433a64ed3ed8bfcd183e1f9c8 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -118,9 +118,6 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
 		sctp_transport_route(tp, NULL, sp);
 		if (asoc->param_flags & SPP_PMTUD_ENABLE)
 			sctp_assoc_sync_pmtu(asoc);
-	} else if (!sctp_transport_pmtu_check(tp)) {
-		if (asoc->param_flags & SPP_PMTUD_ENABLE)
-			sctp_assoc_sync_pmtu(asoc);
 	}
 
 	/* If there a is a prepend chunk stick it on the list before
-- 
2.14.3

[PATCH net-next 09/13] sctp: remove sctp_transport_pmtu_check

[Marcelo Ricardo Leitner]

We are now keeping the MTU information synced between asoc, transport
and dst, which makes the check at sctp_packet_config() not needed
anymore. As it was the sole caller to this function, lets remove it.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sctp.h | 12 ------------
 net/sctp/output.c       |  3 ---
 2 files changed, 15 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 4965cbfa7d92c0f60a76dd6e03571f78209dda5a..f66d4435000799f523fbaa34b0a57dbdeebda040 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -606,16 +606,4 @@ static inline __u32 sctp_dst_mtu(const struct dst_entry *dst)
 				 SCTP_DEFAULT_MINSEGMENT));
 }
 
-static inline bool sctp_transport_pmtu_check(struct sctp_transport *t)
-{
-	__u32 pmtu = sctp_dst_mtu(t->dst);
-
-	if (t->pathmtu = pmtu)
-		return true;
-
-	t->pathmtu = pmtu;
-
-	return false;
-}
-
 #endif /* __net_sctp_h__ */
diff --git a/net/sctp/output.c b/net/sctp/output.c
index bf4226c3cc1de79f2041d55a68ea53f4fd600b25..e672dee302c7092433a64ed3ed8bfcd183e1f9c8 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -118,9 +118,6 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
 		sctp_transport_route(tp, NULL, sp);
 		if (asoc->param_flags & SPP_PMTUD_ENABLE)
 			sctp_assoc_sync_pmtu(asoc);
-	} else if (!sctp_transport_pmtu_check(tp)) {
-		if (asoc->param_flags & SPP_PMTUD_ENABLE)
-			sctp_assoc_sync_pmtu(asoc);
 	}
 
 	/* If there a is a prepend chunk stick it on the list before
-- 
2.14.3

[PATCH net-next 10/13] sctp: re-use sctp_transport_pmtu in sctp_transport_route

[Marcelo Ricardo Leitner]

sctp_transport_route currently is very similar to sctp_transport_pmtu plus
a few other bits.

This patch reuses sctp_transport_pmtu in sctp_transport_route and removes
the duplicated code.

Also, as all calls to sctp_transport_route were forcing the dst release
before calling it, let's just include such release too.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c    |  6 ++----
 net/sctp/transport.c | 35 +++++++++++++++++------------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index a93b60a28cc5fff0ebe299905fb31cb37de43d88..bb08d44b838bbbb0ffbf80b194a1291fed4069e4 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -644,16 +644,15 @@ static int sctp_send_asconf_add_ip(struct sock		*sk,
 
 			list_for_each_entry(trans,
 			    &asoc->peer.transport_addr_list, transports) {
-				/* Clear the source and route cache */
-				sctp_transport_dst_release(trans);
 				trans->cwnd = min(4*asoc->pathmtu, max_t(__u32,
 				    2*asoc->pathmtu, 4380));
 				trans->ssthresh = asoc->peer.i.a_rwnd;
 				trans->rto = asoc->rto_initial;
 				sctp_max_rto(asoc, trans);
 				trans->rtt = trans->srtt = trans->rttvar = 0;
+				/* Clear the source and route cache */
 				sctp_transport_route(trans, NULL,
-				    sctp_sk(asoc->base.sk));
+						     sctp_sk(asoc->base.sk));
 			}
 		}
 		retval = sctp_send_asconf(asoc, chunk);
@@ -896,7 +895,6 @@ static int sctp_send_asconf_del_ip(struct sock		*sk,
 		 */
 		list_for_each_entry(transport, &asoc->peer.transport_addr_list,
 					transports) {
-			sctp_transport_dst_release(transport);
 			sctp_transport_route(transport, NULL,
 					     sctp_sk(asoc->base.sk));
 		}
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index ed73a9d91b83eadd3eb8d681fda104fe71562c3f..4a95e260b674b3dffe2ffc4c54cf9a330dfac18c 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -242,6 +242,15 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 						&transport->fl, sk);
 	}
 
+	if (transport->param_flags & SPP_PMTUD_DISABLE) {
+		struct sctp_association *asoc = transport->asoc;
+
+		if (!transport->pathmtu && asoc && asoc->pathmtu)
+			transport->pathmtu = asoc->pathmtu;
+		if (transport->pathmtu)
+			return;
+	}
+
 	if (transport->dst)
 		transport->pathmtu = sctp_dst_mtu(transport->dst);
 	else
@@ -290,6 +299,7 @@ void sctp_transport_route(struct sctp_transport *transport,
 	struct sctp_association *asoc = transport->asoc;
 	struct sctp_af *af = transport->af_specific;
 
+	sctp_transport_dst_release(transport);
 	af->get_dst(transport, saddr, &transport->fl, sctp_opt2sk(opt));
 
 	if (saddr)
@@ -297,25 +307,14 @@ void sctp_transport_route(struct sctp_transport *transport,
 	else
 		af->get_saddr(opt, transport, &transport->fl);
 
-	if ((transport->param_flags & SPP_PMTUD_DISABLE) && transport->pathmtu) {
-		return;
-	}
-	if (transport->dst) {
-		transport->pathmtu = SCTP_TRUNC4(dst_mtu(transport->dst));
+	sctp_transport_pmtu(transport, sctp_opt2sk(opt));
 
-		/* Initialize sk->sk_rcv_saddr, if the transport is the
-		 * association's active path for getsockname().
-		 */
-		if (asoc && (!asoc->peer.primary_path ||
-			     (transport == asoc->peer.active_path)))
-			opt->pf->to_sk_saddr(&transport->saddr,
-					     asoc->base.sk);
-	} else if ((transport->param_flags & SPP_PMTUD_DISABLE) &&
-		   asoc && asoc->pathmtu) {
-		transport->pathmtu = asoc->pathmtu;
-	} else {
-		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
-	}
+	/* Initialize sk->sk_rcv_saddr, if the transport is the
+	 * association's active path for getsockname().
+	 */
+	if (transport->dst && asoc &&
+	    (!asoc->peer.primary_path || transport == asoc->peer.active_path))
+		opt->pf->to_sk_saddr(&transport->saddr, asoc->base.sk);
 }
 
 /* Hold a reference to a transport.  */
-- 
2.14.3

[PATCH net-next 10/13] sctp: re-use sctp_transport_pmtu in sctp_transport_route

[Marcelo Ricardo Leitner]

sctp_transport_route currently is very similar to sctp_transport_pmtu plus
a few other bits.

This patch reuses sctp_transport_pmtu in sctp_transport_route and removes
the duplicated code.

Also, as all calls to sctp_transport_route were forcing the dst release
before calling it, let's just include such release too.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c    |  6 ++----
 net/sctp/transport.c | 35 +++++++++++++++++------------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index a93b60a28cc5fff0ebe299905fb31cb37de43d88..bb08d44b838bbbb0ffbf80b194a1291fed4069e4 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -644,16 +644,15 @@ static int sctp_send_asconf_add_ip(struct sock		*sk,
 
 			list_for_each_entry(trans,
 			    &asoc->peer.transport_addr_list, transports) {
-				/* Clear the source and route cache */
-				sctp_transport_dst_release(trans);
 				trans->cwnd = min(4*asoc->pathmtu, max_t(__u32,
 				    2*asoc->pathmtu, 4380));
 				trans->ssthresh = asoc->peer.i.a_rwnd;
 				trans->rto = asoc->rto_initial;
 				sctp_max_rto(asoc, trans);
 				trans->rtt = trans->srtt = trans->rttvar = 0;
+				/* Clear the source and route cache */
 				sctp_transport_route(trans, NULL,
-				    sctp_sk(asoc->base.sk));
+						     sctp_sk(asoc->base.sk));
 			}
 		}
 		retval = sctp_send_asconf(asoc, chunk);
@@ -896,7 +895,6 @@ static int sctp_send_asconf_del_ip(struct sock		*sk,
 		 */
 		list_for_each_entry(transport, &asoc->peer.transport_addr_list,
 					transports) {
-			sctp_transport_dst_release(transport);
 			sctp_transport_route(transport, NULL,
 					     sctp_sk(asoc->base.sk));
 		}
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index ed73a9d91b83eadd3eb8d681fda104fe71562c3f..4a95e260b674b3dffe2ffc4c54cf9a330dfac18c 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -242,6 +242,15 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 						&transport->fl, sk);
 	}
 
+	if (transport->param_flags & SPP_PMTUD_DISABLE) {
+		struct sctp_association *asoc = transport->asoc;
+
+		if (!transport->pathmtu && asoc && asoc->pathmtu)
+			transport->pathmtu = asoc->pathmtu;
+		if (transport->pathmtu)
+			return;
+	}
+
 	if (transport->dst)
 		transport->pathmtu = sctp_dst_mtu(transport->dst);
 	else
@@ -290,6 +299,7 @@ void sctp_transport_route(struct sctp_transport *transport,
 	struct sctp_association *asoc = transport->asoc;
 	struct sctp_af *af = transport->af_specific;
 
+	sctp_transport_dst_release(transport);
 	af->get_dst(transport, saddr, &transport->fl, sctp_opt2sk(opt));
 
 	if (saddr)
@@ -297,25 +307,14 @@ void sctp_transport_route(struct sctp_transport *transport,
 	else
 		af->get_saddr(opt, transport, &transport->fl);
 
-	if ((transport->param_flags & SPP_PMTUD_DISABLE) && transport->pathmtu) {
-		return;
-	}
-	if (transport->dst) {
-		transport->pathmtu = SCTP_TRUNC4(dst_mtu(transport->dst));
+	sctp_transport_pmtu(transport, sctp_opt2sk(opt));
 
-		/* Initialize sk->sk_rcv_saddr, if the transport is the
-		 * association's active path for getsockname().
-		 */
-		if (asoc && (!asoc->peer.primary_path ||
-			     (transport = asoc->peer.active_path)))
-			opt->pf->to_sk_saddr(&transport->saddr,
-					     asoc->base.sk);
-	} else if ((transport->param_flags & SPP_PMTUD_DISABLE) &&
-		   asoc && asoc->pathmtu) {
-		transport->pathmtu = asoc->pathmtu;
-	} else {
-		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
-	}
+	/* Initialize sk->sk_rcv_saddr, if the transport is the
+	 * association's active path for getsockname().
+	 */
+	if (transport->dst && asoc &&
+	    (!asoc->peer.primary_path || transport = asoc->peer.active_path))
+		opt->pf->to_sk_saddr(&transport->saddr, asoc->base.sk);
 }
 
 /* Hold a reference to a transport.  */
-- 
2.14.3

[PATCH net-next 11/13] sctp: honor PMTU_DISABLED when handling icmp

[Marcelo Ricardo Leitner]

sctp_sendmsg() could trigger PMTU updates even when PMTU_DISABLED was
set, as pmtu_pending could be set unconditionally during icmp handling
if the socket was in use by the application.

This patch fixes it by checking for PMTU_DISABLED when handling such
deferred updates.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index bb08d44b838bbbb0ffbf80b194a1291fed4069e4..ad8965835d8d51d4eda857b91dfca140710bf7da 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1893,6 +1893,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 				struct sctp_sndrcvinfo *sinfo)
 {
 	struct sock *sk = asoc->base.sk;
+	struct sctp_sock *sp = sctp_sk(sk);
 	struct net *net = sock_net(sk);
 	struct sctp_datamsg *datamsg;
 	bool wait_connect = false;
@@ -1911,13 +1912,14 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 			goto err;
 	}
 
-	if (sctp_sk(sk)->disable_fragments && msg_len > asoc->frag_point) {
+	if (sp->disable_fragments && msg_len > asoc->frag_point) {
 		err = -EMSGSIZE;
 		goto err;
 	}
 
 	if (asoc->pmtu_pending) {
-		sctp_assoc_sync_pmtu(asoc);
+		if (sp->param_flags & SPP_PMTUD_ENABLE)
+			sctp_assoc_sync_pmtu(asoc);
 		asoc->pmtu_pending = 0;
 	}
 
@@ -1936,7 +1938,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 		if (err)
 			goto err;
 
-		if (sctp_sk(sk)->strm_interleave) {
+		if (sp->strm_interleave) {
 			timeo = sock_sndtimeo(sk, 0);
 			err = sctp_wait_for_connect(asoc, &timeo);
 			if (err)
-- 
2.14.3

[PATCH net-next 11/13] sctp: honor PMTU_DISABLED when handling icmp

[Marcelo Ricardo Leitner]

sctp_sendmsg() could trigger PMTU updates even when PMTU_DISABLED was
set, as pmtu_pending could be set unconditionally during icmp handling
if the socket was in use by the application.

This patch fixes it by checking for PMTU_DISABLED when handling such
deferred updates.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index bb08d44b838bbbb0ffbf80b194a1291fed4069e4..ad8965835d8d51d4eda857b91dfca140710bf7da 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1893,6 +1893,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 				struct sctp_sndrcvinfo *sinfo)
 {
 	struct sock *sk = asoc->base.sk;
+	struct sctp_sock *sp = sctp_sk(sk);
 	struct net *net = sock_net(sk);
 	struct sctp_datamsg *datamsg;
 	bool wait_connect = false;
@@ -1911,13 +1912,14 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 			goto err;
 	}
 
-	if (sctp_sk(sk)->disable_fragments && msg_len > asoc->frag_point) {
+	if (sp->disable_fragments && msg_len > asoc->frag_point) {
 		err = -EMSGSIZE;
 		goto err;
 	}
 
 	if (asoc->pmtu_pending) {
-		sctp_assoc_sync_pmtu(asoc);
+		if (sp->param_flags & SPP_PMTUD_ENABLE)
+			sctp_assoc_sync_pmtu(asoc);
 		asoc->pmtu_pending = 0;
 	}
 
@@ -1936,7 +1938,7 @@ static int sctp_sendmsg_to_asoc(struct sctp_association *asoc,
 		if (err)
 			goto err;
 
-		if (sctp_sk(sk)->strm_interleave) {
+		if (sp->strm_interleave) {
 			timeo = sock_sndtimeo(sk, 0);
 			err = sctp_wait_for_connect(asoc, &timeo);
 			if (err)
-- 
2.14.3

[PATCH net-next 12/13] sctp: consider idata chunks when setting SCTP_MAXSEG

[Marcelo Ricardo Leitner]

When setting SCTP_MAXSEG sock option, it should consider which kind of
data chunk is being used if the asoc is already available, so that the
limit better reflect reality.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index ad8965835d8d51d4eda857b91dfca140710bf7da..2d35c8ea2470e7f5481bb9675ffd233eb3424d91 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3233,18 +3233,21 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 		return -EINVAL;
 	}
 
+	asoc = sctp_id2assoc(sk, params.assoc_id);
+
 	if (val) {
 		int min_len, max_len;
+		__u16 datasize = asoc ? sctp_datachk_len(&asoc->stream) :
+				 sizeof(struct sctp_data_chunk);
 
 		min_len = sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT,
-					   sizeof(struct sctp_data_chunk));
-		max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
+					   datasize);
+		max_len = SCTP_MAX_CHUNK_LEN - datasize;
 
 		if (val < min_len || val > max_len)
 			return -EINVAL;
 	}
 
-	asoc = sctp_id2assoc(sk, params.assoc_id);
 	if (asoc) {
 		if (val == 0) {
 			val = asoc->pathmtu - af->net_header_len;
-- 
2.14.3

[PATCH net-next 12/13] sctp: consider idata chunks when setting SCTP_MAXSEG

[Marcelo Ricardo Leitner]

When setting SCTP_MAXSEG sock option, it should consider which kind of
data chunk is being used if the asoc is already available, so that the
limit better reflect reality.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index ad8965835d8d51d4eda857b91dfca140710bf7da..2d35c8ea2470e7f5481bb9675ffd233eb3424d91 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3233,18 +3233,21 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 		return -EINVAL;
 	}
 
+	asoc = sctp_id2assoc(sk, params.assoc_id);
+
 	if (val) {
 		int min_len, max_len;
+		__u16 datasize = asoc ? sctp_datachk_len(&asoc->stream) :
+				 sizeof(struct sctp_data_chunk);
 
 		min_len = sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT,
-					   sizeof(struct sctp_data_chunk));
-		max_len = SCTP_MAX_CHUNK_LEN - sizeof(struct sctp_data_chunk);
+					   datasize);
+		max_len = SCTP_MAX_CHUNK_LEN - datasize;
 
 		if (val < min_len || val > max_len)
 			return -EINVAL;
 	}
 
-	asoc = sctp_id2assoc(sk, params.assoc_id);
 	if (asoc) {
 		if (val = 0) {
 			val = asoc->pathmtu - af->net_header_len;
-- 
2.14.3

[PATCH net-next 13/13] sctp: allow unsetting sockopt MAXSEG

[Marcelo Ricardo Leitner]

RFC 6458 Section 8.1.16 says that setting MAXSEG as 0 means that the user
is not limiting it, and not that it should set to the *current* maximum,
as we are doing.

This patch thus allow setting it as 0, effectively removing the user
limit.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2d35c8ea2470e7f5481bb9675ffd233eb3424d91..1b4593b842b001903f518e90484c763d9d3698f3 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3211,7 +3211,6 @@ static int sctp_setsockopt_mappedv4(struct sock *sk, char __user *optval, unsign
 static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned int optlen)
 {
 	struct sctp_sock *sp = sctp_sk(sk);
-	struct sctp_af *af = sp->pf->af;
 	struct sctp_assoc_value params;
 	struct sctp_association *asoc;
 	int val;
@@ -3249,12 +3248,6 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 	}
 
 	if (asoc) {
-		if (val == 0) {
-			val = asoc->pathmtu - af->net_header_len;
-			val -= af->ip_options_len(sk);
-			val -= sizeof(struct sctphdr) +
-			       sctp_datachk_len(&asoc->stream);
-		}
 		asoc->user_frag = val;
 		sctp_assoc_update_frag_point(asoc);
 	} else {
-- 
2.14.3

[PATCH net-next 13/13] sctp: allow unsetting sockopt MAXSEG

[Marcelo Ricardo Leitner]

RFC 6458 Section 8.1.16 says that setting MAXSEG as 0 means that the user
is not limiting it, and not that it should set to the *current* maximum,
as we are doing.

This patch thus allow setting it as 0, effectively removing the user
limit.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/socket.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 2d35c8ea2470e7f5481bb9675ffd233eb3424d91..1b4593b842b001903f518e90484c763d9d3698f3 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3211,7 +3211,6 @@ static int sctp_setsockopt_mappedv4(struct sock *sk, char __user *optval, unsign
 static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned int optlen)
 {
 	struct sctp_sock *sp = sctp_sk(sk);
-	struct sctp_af *af = sp->pf->af;
 	struct sctp_assoc_value params;
 	struct sctp_association *asoc;
 	int val;
@@ -3249,12 +3248,6 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
 	}
 
 	if (asoc) {
-		if (val = 0) {
-			val = asoc->pathmtu - af->net_header_len;
-			val -= af->ip_options_len(sk);
-			val -= sizeof(struct sctphdr) +
-			       sctp_datachk_len(&asoc->stream);
-		}
 		asoc->user_frag = val;
 		sctp_assoc_update_frag_point(asoc);
 	} else {
-- 
2.14.3

Re: [PATCH net-next 00/13] sctp: refactor MTU handling

[Xin Long]

On Fri, Apr 27, 2018 at 3:58 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> Currently MTU handling is spread over SCTP stack. There are multiple
> places doing same/similar calculations and updating them is error prone
> as one spot can easily be left out.
>
> This patchset converges it into a more concise and consistent code. In
> general, it moves MTU handling from functions with bigger objectives,
> such as sctp_assoc_add_peer(), to specific functions.
>
> It's also a preparation for the next patchset, which removes the
> duplication between sctp_make_op_error_space and
> sctp_make_op_error_fixed and relies on sctp_mtu_payload introduced here.
>
> More details on each patch.
>
> Marcelo Ricardo Leitner (13):
>   sctp: remove old and unused SCTP_MIN_PMTU
>   sctp: move transport pathmtu calc away of sctp_assoc_add_peer
>   sctp: remove an if() that is always true
>   sctp: introduce sctp_assoc_set_pmtu
>   sctp: introduce sctp_mtu_payload
>   sctp: introduce sctp_assoc_update_frag_point
>   sctp: remove sctp_assoc_pending_pmtu
>   sctp: introduce sctp_dst_mtu
>   sctp: remove sctp_transport_pmtu_check
>   sctp: re-use sctp_transport_pmtu in sctp_transport_route
>   sctp: honor PMTU_DISABLED when handling icmp
>   sctp: consider idata chunks when setting SCTP_MAXSEG
>   sctp: allow unsetting sockopt MAXSEG
>
>  include/net/sctp/constants.h |  5 ++--
>  include/net/sctp/sctp.h      | 52 ++++++++++++++------------------------
>  include/net/sctp/structs.h   |  2 ++
>  net/sctp/associola.c         | 60 +++++++++++++++++++++++---------------------
>  net/sctp/chunk.c             | 12 +--------
>  net/sctp/output.c            | 28 ++++++++-------------
>  net/sctp/socket.c            | 43 ++++++++++++++-----------------
>  net/sctp/transport.c         | 37 ++++++++++++++-------------
>  8 files changed, 105 insertions(+), 134 deletions(-)
>
> --
> 2.14.3
>
Series
Reviewed-by: Xin Long <lucien.xin@gmail.com>

Re: [PATCH net-next 00/13] sctp: refactor MTU handling

[Xin Long]

On Fri, Apr 27, 2018 at 3:58 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> Currently MTU handling is spread over SCTP stack. There are multiple
> places doing same/similar calculations and updating them is error prone
> as one spot can easily be left out.
>
> This patchset converges it into a more concise and consistent code. In
> general, it moves MTU handling from functions with bigger objectives,
> such as sctp_assoc_add_peer(), to specific functions.
>
> It's also a preparation for the next patchset, which removes the
> duplication between sctp_make_op_error_space and
> sctp_make_op_error_fixed and relies on sctp_mtu_payload introduced here.
>
> More details on each patch.
>
> Marcelo Ricardo Leitner (13):
>   sctp: remove old and unused SCTP_MIN_PMTU
>   sctp: move transport pathmtu calc away of sctp_assoc_add_peer
>   sctp: remove an if() that is always true
>   sctp: introduce sctp_assoc_set_pmtu
>   sctp: introduce sctp_mtu_payload
>   sctp: introduce sctp_assoc_update_frag_point
>   sctp: remove sctp_assoc_pending_pmtu
>   sctp: introduce sctp_dst_mtu
>   sctp: remove sctp_transport_pmtu_check
>   sctp: re-use sctp_transport_pmtu in sctp_transport_route
>   sctp: honor PMTU_DISABLED when handling icmp
>   sctp: consider idata chunks when setting SCTP_MAXSEG
>   sctp: allow unsetting sockopt MAXSEG
>
>  include/net/sctp/constants.h |  5 ++--
>  include/net/sctp/sctp.h      | 52 ++++++++++++++------------------------
>  include/net/sctp/structs.h   |  2 ++
>  net/sctp/associola.c         | 60 +++++++++++++++++++++++---------------------
>  net/sctp/chunk.c             | 12 +--------
>  net/sctp/output.c            | 28 ++++++++-------------
>  net/sctp/socket.c            | 43 ++++++++++++++-----------------
>  net/sctp/transport.c         | 37 ++++++++++++++-------------
>  8 files changed, 105 insertions(+), 134 deletions(-)
>
> --
> 2.14.3
>
Series
Reviewed-by: Xin Long <lucien.xin@gmail.com>

Re: [PATCH net-next 00/13] sctp: refactor MTU handling

[David Miller]

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 26 Apr 2018 16:58:49 -0300

> Currently MTU handling is spread over SCTP stack. There are multiple
> places doing same/similar calculations and updating them is error prone
> as one spot can easily be left out.
> 
> This patchset converges it into a more concise and consistent code. In
> general, it moves MTU handling from functions with bigger objectives,
> such as sctp_assoc_add_peer(), to specific functions.
> 
> It's also a preparation for the next patchset, which removes the
> duplication between sctp_make_op_error_space and
> sctp_make_op_error_fixed and relies on sctp_mtu_payload introduced here.
> 
> More details on each patch.

Series applied, thanks!

Re: [PATCH net-next 00/13] sctp: refactor MTU handling

[David Miller]

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 26 Apr 2018 16:58:49 -0300

> Currently MTU handling is spread over SCTP stack. There are multiple
> places doing same/similar calculations and updating them is error prone
> as one spot can easily be left out.
> 
> This patchset converges it into a more concise and consistent code. In
> general, it moves MTU handling from functions with bigger objectives,
> such as sctp_assoc_add_peer(), to specific functions.
> 
> It's also a preparation for the next patchset, which removes the
> duplication between sctp_make_op_error_space and
> sctp_make_op_error_fixed and relies on sctp_mtu_payload introduced here.
> 
> More details on each patch.

Series applied, thanks!