sctp: refactor MTU handling
Subsystems (3)
Marcelo Ricardo Leitner
maintainercover sctp: refactor MTU handling
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
Marcelo Ricardo Leitner
maintainerPATCH 1/13 sctp: remove old and unused SCTP_MIN_PMTU
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.
--- include/net/sctp/constants.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
include/net/sctp/constants.h +2 -3
@@ -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 };
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 2/13 sctp: move transport pathmtu calc away of sctp_assoc_add_peer
In reply to Marcelo Ricardo LeitnerThere 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.
--- net/sctp/associola.c | 9 +-------- net/sctp/transport.c | 8 ++++++-- 2 files changed, 7 insertions(+), 10 deletions(-)
net/sctp/associola.c +1 -8
@@ -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.
net/sctp/transport.c +6 -2
@@ -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. */
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 3/13 sctp: remove an if() that is always true
In reply to Marcelo Ricardo LeitnerAs noticed by Xin Long, the if() here is always true as PMTU can never be 0.
--- net/sctp/associola.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
net/sctp/associola.c +2 -4
@@ -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);
Signature · 1 line
2.14.3
Re: [PATCH net-next 03/13] sctp: remove an if() that is always true
Quoted 14 lines · Marcelo Ricardo Leitner
As noticed by Xin Long, the
if()here is always true as PMTU can never be 0.Reported-by Xin Long <[email protected]>Signed-off-by Marcelo Ricardo Leitner <[email protected]>--- net/sctp/associola.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)net/sctp/associola.c +0
-0
net/sctp/associola.c void sctp_assoc_sync_pmtu(struct sctp_association *asoc) · line 1402
Quoted 10 lines
net/sctp/associola.c +2
-4@@ -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
Quoted 6 lines
net/sctp/associola.c +0
-0pr_debug ("%s: asoc:%p, pmtu:%d, frag_point:%d\n" , __func__, asoc, asoc->pathmtu, asoc->frag_point);Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerRe: [PATCH net-next 03/13] sctp: remove an if() that is always true
Quoted 18 lines · Neil Horman
Quoted 17 lines · Marcelo Ricardo Leitner
As noticed by Xin Long, the
if()here is always true as PMTU can never be 0.Reported-by Xin Long <[email protected]>Signed-off-by Marcelo Ricardo Leitner <[email protected]>--- net/sctp/associola.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)net/sctp/associola.c +0
-0@@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc) pmtu = t->pathmtu; }
Quoted 8 lines
net/sctp/associola.c void sctp_assoc_sync_pmtu(struct sctp_association *asoc) · line 1402
Quoted 7 lines
net/sctp/associola.c +2
-4-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.
Quoted 7 lines
sends a crafted icmp dest unreach message to the host,
pmtu_sendingmight be able to get set for an association (which may have no transports established yet), and if so, on the first packet sendsctp_assoc_sync_pmtucan 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 insctp_frag_pointdue 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
Quoted 14 lines
Neil
Quoted 6 lines
net/sctp/associola.c +0
-1pr_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 [email protected]
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
Quoted 38 lines · Marcelo Ricardo Leitner
Quoted 18 lines · Neil Horman
Quoted 17 lines · Marcelo Ricardo Leitner
As noticed by Xin Long, the
if()here is always true as PMTU can never be 0.Reported-by Xin Long <[email protected]>Signed-off-by Marcelo Ricardo Leitner <[email protected]>--- net/sctp/associola.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)net/sctp/associola.c +0
-0@@ -1397,10 +1397,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc) pmtu = t->pathmtu; }Quoted 8 lines
net/sctp/associola.c void sctp_assoc_sync_pmtu(struct sctp_association *asoc) · line 1402
Quoted 7 lines
net/sctp/associola.c +2
-4-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.
Quoted 7 lines
sends a crafted icmp dest unreach message to the host,
pmtu_sendingmight be able to get set for an association (which may have no transports established yet), and if so, on the first packet sendsctp_assoc_sync_pmtucan 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 insctp_frag_pointdue to an underflow of the frag value
Quoted 4 lines
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
Quoted 17 lines
Marcelo
Quoted 14 lines
Neil
Quoted 6 lines
net/sctp/associola.c +0
-1pr_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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Marcelo Ricardo Leitner
maintainerPATCH 4/13 sctp: introduce sctp_assoc_set_pmtu
In reply to Marcelo Ricardo LeitnerAll changes to asoc PMTU should now go through this wrapper, making it easier to track them and to do other actions upon it.
--- include/net/sctp/structs.h | 1 + net/sctp/associola.c | 21 +++++++++++++-------- net/sctp/socket.c | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-)
include/net/sctp/structs.h +1 -0
@@ -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 );
net/sctp/associola.c +13 -8
@@ -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,
net/sctp/socket.c +1 -1
@@ -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; }
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 5/13 sctp: introduce sctp_mtu_payload
In reply to Marcelo Ricardo LeitnerWhen 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.
--- include/net/sctp/sctp.h | 19 +++++++++++++++++++ net/sctp/output.c | 25 ++++++++++--------------- net/sctp/socket.c | 7 ++----- 3 files changed, 31 insertions(+), 20 deletions(-)
include/net/sctp/sctp.h +19 -0
@@ -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)),
net/sctp/output.c +10 -15
@@ -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)) {
net/sctp/socket.c +2 -5
@@ -3234,11 +3234,8 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsignedif (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)
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 6/13 sctp: introduce sctp_assoc_update_frag_point
In reply to Marcelo Ricardo Leitnerand 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.
--- 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(-)
include/net/sctp/sctp.h +0 -20
@@ -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);
include/net/sctp/structs.h +1 -0
@@ -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 );
net/sctp/associola.c +17 -7
@@ -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? */
net/sctp/chunk.c +1 -11
@@ -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;
net/sctp/socket.c +1 -1
@@ -3251,7 +3251,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsignedsctp_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 ;
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 7/13 sctp: remove sctp_assoc_pending_pmtu
In reply to Marcelo Ricardo LeitnerNo need for this helper.
--- include/net/sctp/sctp.h | 6 ------ net/sctp/socket.c | 6 ++++-- 2 files changed, 4 insertions(+), 8 deletions(-)
include/net/sctp/sctp.h +0 -6
@@ -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);
net/sctp/socket.c +4 -2
@@ -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));
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 8/13 sctp: introduce sctp_dst_mtu
In reply to Marcelo Ricardo LeitnerWhich makes sure that the MTU respects the minimum value of SCTP_DEFAULT_MINSEGMENT and that it is correctly aligned.
--- include/net/sctp/sctp.h | 9 +++++++-- net/sctp/associola.c | 6 ++---- net/sctp/transport.c | 6 +++--- 3 files changed, 12 insertions(+), 9 deletions(-)
include/net/sctp/sctp.h +7 -2
@@ -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;
net/sctp/associola.c +2 -4
@@ -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))
net/sctp/transport.c +3 -3
@@ -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 ; }
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 9/13 sctp: remove sctp_transport_pmtu_check
In reply to Marcelo Ricardo LeitnerWe 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.
--- include/net/sctp/sctp.h | 12 ------------ net/sctp/output.c | 3 --- 2 files changed, 15 deletions(-)
include/net/sctp/sctp.h +0 -12
@@ -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__ */
net/sctp/output.c +0 -3
@@ -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
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 10/13 sctp: re-use sctp_transport_pmtu in sctp_transport_route
In reply to Marcelo Ricardo Leitnersctp_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.
--- net/sctp/socket.c | 6 ++---- net/sctp/transport.c | 35 +++++++++++++++++------------------ 2 files changed, 19 insertions(+), 22 deletions(-)
net/sctp/socket.c +2 -4
@@ -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)); }
net/sctp/transport.c +17 -18
@@ -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. */
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 11/13 sctp: honor PMTU_DISABLED when handling icmp
In reply to Marcelo Ricardo Leitnersctp_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.
--- net/sctp/socket.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
net/sctp/socket.c +5 -3
@@ -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)
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 12/13 sctp: consider idata chunks when setting SCTP_MAXSEG
In reply to Marcelo Ricardo LeitnerWhen 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.
--- net/sctp/socket.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
net/sctp/socket.c +6 -3
@@ -3233,18 +3233,21 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsignedreturn -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;
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH 13/13 sctp: allow unsetting sockopt MAXSEG
In reply to Marcelo Ricardo LeitnerRFC 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.
--- net/sctp/socket.c | 7 ------- 1 file changed, 7 deletions(-)
net/sctp/socket.c +0 -7
@@ -3211,7 +3211,6 @@ static int sctp_setsockopt_mappedv4(struct sock *sk, char __user *optval, unsignstatic 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 {
Signature · 1 line
2.14.3
Quoted 39 lines · 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_spaceandsctp_make_op_error_fixedand relies onsctp_mtu_payloadintroduced here.More details on each patch.
Marcelo Ricardo Leitner (13):
sctp: remove old and unusedSCTP_MIN_PMTU
sctp: move transport pathmtu calc away ofsctp_assoc_add_peer
sctp: remove anif()that is always true
sctp: introducesctp_assoc_set_pmtu
sctp: introducesctp_mtu_payload
sctp: introducesctp_assoc_update_frag_point
sctp: removesctp_assoc_pending_pmtu
sctp: introducesctp_dst_mtu
sctp: removesctp_transport_pmtu_check
sctp: re-usesctp_transport_pmtuinsctp_transport_route
sctp: honorPMTU_DISABLEDwhen handling icmp
sctp: consider idata chunks when settingSCTP_MAXSEG
sctp: allow unsetting sockopt MAXSEGinclude/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(-)
Quoted 3 lines
--
2.14.3
Series
David Miller
maintainerFrom: Marcelo Ricardo Leitner <[email protected]>
Date: Thu, 26 Apr 2018 16:58:49 -0300
Quoted 8 lines
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.
Quoted 5 lines
It's also a preparation for the next patchset, which removes the duplication between
sctp_make_op_error_spaceandsctp_make_op_error_fixedand relies onsctp_mtu_payloadintroduced here.More details on each patch.
Series applied, thanks!