From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Audykowicz Subject: Re: [PATCH net] sctp: always set frag_point on pmtu change Date: Tue, 4 Dec 2018 20:30:22 +0100 Message-ID: <84d5c4e6-0e3b-a323-48c1-0bde73a89453@gmail.com> References: <20181118204736.7178-1-jakub.audykowicz@gmail.com> <20181128020838.GJ3601@localhost.localdomain> <20181128112600.GK3601@localhost.localdomain> <20cbe95f-8858-c571-ed23-e90a18ca9d19@gmail.com> <20181204174539.GA10987@localhost.localdomain> <20181204185832.GB10987@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Xin Long , linux-sctp@vger.kernel.org, Vlad Yasevich , Neil Horman , davem , network dev To: Marcelo Ricardo Leitner Return-path: Received: from mail-lj1-f194.google.com ([209.85.208.194]:42029 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725831AbeLDTa3 (ORCPT ); Tue, 4 Dec 2018 14:30:29 -0500 In-Reply-To: <20181204185832.GB10987@localhost.localdomain> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018-12-04 19:58, Marcelo Ricardo Leitner wrote: > On Tue, Dec 04, 2018 at 07:51:54PM +0100, Jakub Audykowicz wrote: > ... >> Thanks, I've taken your remarks into account and ended up with this >> simple solution: > LGTM! Thanks > >> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h >> index ab9242e51d9e..3487686f2cf5 100644 >> --- a/include/net/sctp/sctp.h >> +++ b/include/net/sctp/sctp.h >> @@ -620,4 +620,9 @@ static inline bool sctp_transport_pmtu_check(struct sctp_transport *t) >> return false; >> } >> >> +static inline __u32 sctp_min_frag_point(struct sctp_sock *sp, __u16 datasize) >> +{ >> + return sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT, datasize); > Not sure how final the patch is but be sure to run checkpatch.pl on > it before submitting it officially. It flagged some issues like the > above indentation using spaces, for example. Should be fine now. I have checkpatch as a post-commit hook but I was not committing when creating these quick diffs :). > >> +} >> + >> #endif /* __net_sctp_h__ */ >> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c >> index ce8087846f05..dc12c2ba487f 100644 >> --- a/net/sctp/chunk.c >> +++ b/net/sctp/chunk.c >> @@ -191,6 +191,12 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc, >> * the packet >> */ >> max_data = asoc->frag_point; >> + if (unlikely(!max_data)) { >> + pr_warn_ratelimited("%s: asoc:%p frag_point is zero, forcing max_data to default minimum", >> + __func__, asoc); >> + max_data = sctp_min_frag_point( >> + sctp_sk(asoc->base.sk), sctp_datachk_len(&asoc->stream)); >> + } >> >> /* If the the peer requested that we authenticate DATA chunks >> * we need to account for bundling of the AUTH chunks along with >> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >> index bf618d1b41fd..b8cebd5a87e5 100644 >> --- a/net/sctp/socket.c >> +++ b/net/sctp/socket.c >> @@ -3324,8 +3324,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned >> __u16 datasize = asoc ? sctp_datachk_len(&asoc->stream) : >> sizeof(struct sctp_data_chunk); >> >> - min_len = sctp_mtu_payload(sp, SCTP_DEFAULT_MINSEGMENT, >> - datasize); >> + min_len = sctp_min_frag_point(sp, datasize); >> max_len = SCTP_MAX_CHUNK_LEN - datasize; >> >> if (val < min_len || val > max_len) >> >>   >> >>