This message generated a parse failure. Raw output follows here. Please use 'back' to navigate. From devnull@lkml.org Wed Apr 24 12:12:15 2024 Envelope-to: j@jasper.es Delivery-date: Wed, 20 Feb 2013 19:07:37 +0100 Received: from localhost ([127.0.0.1] helo=squeeze.vs19.net) by squeeze.vs19.net with esmtp (Exim 4.72) (envelope-from ) id 1U8E4v-0003gQ-6Y for j@jasper.es; Wed, 20 Feb 2013 19:07:37 +0100 Received: from pop3.telfort.nl [213.205.33.252] by squeeze.vs19.net with POP3 (fetchmail-6.3.18) for (single-drop); Wed, 20 Feb 2013 19:07:37 +0100 (CET) Received: from cmgw-nl-2 (10.39.115.162) by mda-nl-1.mail.tiscali.sys (8.5.142) id 4FC4EF72069B944A for jasper@telfort.nl; Wed, 20 Feb 2013 19:06:28 +0100 Received: from cpsmtpb-ews10.kpnxchange.com ([213.75.39.15]) by cmgw-nl-2 with id 2i6T1l0030Kd0Rx01i6TmR; Wed, 20 Feb 2013 19:06:27 +0100 X-CNFS-Analysis: v=2.0 cv=OLKlLFmB c=1 sm=2 a=UK1r566ZdBxH71SXbqIOeA==:17 a=0wiOde9d68gA:10 a=CA8DF7HueB4A:10 a=nDghuxUhq_wA:10 a=IkcTkHD0fZMA:10 a=pGLkceISAAAA:8 a=1v0RR-UgpuMA:10 a=cH6R9-kdAAAA:8 a=VwQbUJbxAAAA:8 a=W0vUJOdyAAAA:8 a=JKAEfWV9HlijubxxB68A:9 a=QEXdDO2ut3YA Received: from cpsps-ews13.kpnxchange.com ([10.94.84.180]) by cpsmtpb-ews10.kpnxchange.com with Microsoft SMTPSVC(7.5.7601.17514); Wed, 20 Feb 2013 19:05:06 +0100 Received: from vger.kernel.org ([209.132.180.67]) by cpsps-ews13.kpnxchange.com with Microsoft SMTPSVC(7.5.7601.17514); Wed, 20 Feb 2013 19:05:06 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934480Ab3BTSGL convert rfc822-to-quoted-printable (ORCPT ); Wed, 20 Feb 2013 13:06:11 -0500 Received: from mail-qe0-f49.google.com ([209.85.128.49]:56191 "EHLO mail-qe0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751640Ab3BTSGJ (ORCPT ); Wed, 20 Feb 2013 13:06:09 -0500 Received: by mail-qe0-f49.google.com with SMTP id q19so229067qeb.36 for ; Wed, 20 Feb 2013 10:06:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=dUON9BtzkNVav4 X-Received: by 10.224.216.65 with SMTP id hh1mr9564876qab.43.1361383568025; Wed, 20 Feb 2013 10:06:08 -0800 (PST) Received: from [192.168.98.106] (pool-70-109-140-46.cncdnh.east.myfairpoint.net. [70.109.140.46]) by mx.google.com with ESMTPS id t2sm30927245qav.1.2013.02.20.10.06.06 (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 20 Feb 2013 10:06:07 -080 Message-Id: <5125108D.3010508@gmail.com> Date: Wed, 20 Feb 2013 13:06:05 -0500 From: Vlad Yasevich User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/17.0 Thunderbird/17.0 Mime-Version: 1.0 To: "Roberts, Lee A." Cc: "linux-sctp@vger.kernel.org" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 3/3] sctp: fix association hangs due to reassembly/ordering logic References: <1361374996.3450.3.camel@laptop.lroberts> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-Id: X-Mailing-List: linux-kernel@vger.kernel.org X-OriginalArrivalTime: 20 Feb 2013 18:05:06.0134 (UTC) FILETIME=[D01B2760:01CE0F94] X-RcptDomain: telfort.nl Hi Lee On 02/20/2013 10:56 AM, Roberts, Lee A. wrote: > From: Lee A. Roberts > > Resolve SCTP association hangs observed during SCTP stress > testing. Observable symptoms include communications hangs > with data being held in the association reassembly and/or lobby > (ordering) queues. Close examination of reassembly queue shows > missing packets. As a general note for this patch series, you could really benefit from a cover letter that describes the symptoms and all the different issues you found. Also, please title your patches based on the context of the patch. Giving them all the same title is very confusing and at quick glance makes appear that the same patch was applied 3 times. > > In sctp_eat_data(), enter partial delivery mode only if the > data on the head of the reassembly queue is at or before the > cumulative TSN ACK point. > > In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(), > correct message reassembly logic for SCTP partial delivery. > Change logic to ensure that as much data as possible is sent > with the initial partial delivery and that following partial > deliveries contain all available data. > > In sctp_ulpq_renege(), adjust logic to enter partial delivery > only if the incoming chunk remains on the reassembly queue > after processing by sctp_ulpq_tail_data(). Remove call to > sctp_tsnmap_mark(), as this is handled correctly in call to > sctp_ulpq_tail_data(). > > Patch applies to linux-3.8 kernel. > > Signed-off-by: Lee A. Roberts > --- > net/sctp/sm_statefuns.c | 12 ++++++++++-- > net/sctp/ulpqueue.c | 33 ++++++++++++++++++++++++++------- > 2 files changed, 36 insertions(+), 9 deletions(-) > > diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP > +2/net/sctp/sm_statefuns.c linux-3.8-SCTP+3/net/sctp/sm_statefuns.c > --- linux-3.8-SCTP+2/net/sctp/sm_statefuns.c 2013-02-18 > 16:58:34.000000000 -0700 > +++ linux-3.8-SCTP+3/net/sctp/sm_statefuns.c 2013-02-20 > 08:31:51.092132884 -0700 > @@ -6090,7 +6090,8 @@ static int sctp_eat_data(const struct sc > size_t datalen; > sctp_verb_t deliver; > int tmp; > - __u32 tsn; > + __u32 tsn, ctsn; > + struct sk_buff *skb; > struct sctp_tsnmap *map =3D (struct sctp_tsnmap *)&asoc->peer.tsn_= map; > struct sock *sk =3D asoc->base.sk; > struct net *net =3D sock_net(sk); > @@ -6160,7 +6161,14 @@ static int sctp_eat_data(const struct sc > /* Even if we don't accept this chunk there is > * memory pressure. > */ > - sctp_add_cmd_sf(commands, SCTP_CMD_PART_DELIVER, SCTP_NULL()); > + skb =3D skb_peek(&asoc->ulpq.reasm); > + if (skb !=3D NULL) { > + ctsn =3D sctp_skb2event(skb)->tsn; > + if (TSN_lte(ctsn, > + sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map))) > + sctp_add_cmd_sf(commands, > + SCTP_CMD_PART_DELIVER, SCTP_NULL()); > + } What if the tsn you are currently processing will advance the CUM TSN? You may still need to eneter PD. This is why sctp_eat_data() is not the right place to place this check. The more I look at the SCTP_CMD_PART_DELIVERY the more I am thinking=20 that it has to come after the current TSN has been delivered to the=20 queue. That way, if we are currently processing the first fragment,=20 we'll queue it, then enter PD and pull it off the queue properly. If we are processing the middle or last, it will get queued first, and then PD will be entered to fetch anything that we can fetch. In fact, the above is what happens when we issue a RENEGE command, but=20 the order is reversed is RENEGE is skipped for some reason. I am still trying to figure out if it's possible to enter PD without RENEGE, but I= =20 did notice the above aberration. > } > > /* Spill over rwnd a little bit. Note: While allowed, this spill = over > diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP > +2/net/sctp/ulpqueue.c linux-3.8-SCTP+3/net/sctp/ulpqueue.c > --- linux-3.8-SCTP+2/net/sctp/ulpqueue.c 2013-02-20 08:17:53.67923336= 5 > -0700 > +++ linux-3.8-SCTP+3/net/sctp/ulpqueue.c 2013-02-20 08:27:02.78504274= 4 > -0700 > @@ -540,14 +540,19 @@ static struct sctp_ulpevent *sctp_ulpq_r > ctsn =3D cevent->tsn; > > switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) { > + case SCTP_DATA_FIRST_FRAG: > + if (!first_frag) > + return NULL; > + goto done; > case SCTP_DATA_MIDDLE_FRAG: > if (!first_frag) { > first_frag =3D pos; > next_tsn =3D ctsn + 1; > last_frag =3D pos; > - } else if (next_tsn =3D=3D ctsn) > + } else if (next_tsn =3D=3D ctsn) { > next_tsn++; > - else > + last_frag =3D pos; > + } else > goto done; > break; > case SCTP_DATA_LAST_FRAG: This may still allow you to skip over a gap if the first middle fragmen= t=20 in the queue starts after the gap. We need to make sure that TSN of the current chunk is less then equal to sctp_tsnmap_get_ctsn(map= ). > @@ -651,6 +656,14 @@ static struct sctp_ulpevent *sctp_ulpq_r > } else > goto done; > break; > + > + case SCTP_DATA_LAST_FRAG: > + if (!first_frag) > + return NULL; > + else > + goto done; > + break; > + > default: > return NULL; Same thing here. Both, FIRST and MIDDLE fragments need to be validated against sctp_tsnmap_get_ctsn(), otherwise you may be stepping over a ga= p. > } > @@ -1054,6 +1067,7 @@ void sctp_ulpq_renege(struct sctp_ulpq * > gfp_t gfp) > { > struct sctp_association *asoc; > + struct sk_buff *skb; > __u16 needed, freed; > > asoc =3D ulpq->asoc; > @@ -1074,12 +1088,17 @@ void sctp_ulpq_renege(struct sctp_ulpq * > } > /* If able to free enough room, accept this chunk. */ > if (chunk && (freed >=3D needed)) { > - __u32 tsn; > + __u32 tsn, ctsn; > tsn =3D ntohl(chunk->subh.data_hdr->tsn); > - sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport); > - sctp_ulpq_tail_data(ulpq, chunk, gfp); > - > - sctp_ulpq_partial_delivery(ulpq, gfp); > + if (sctp_ulpq_tail_data(ulpq, chunk, gfp) =3D=3D 0) { > + skb =3D skb_peek(&ulpq->reasm); > + if (skb !=3D NULL) { > + ctsn =3D sctp_skb2event(skb)->tsn; > + if (TSN_lte(ctsn, tsn)) > + sctp_ulpq_partial_delivery(ulpq, chunk, > + gfp); > + } > + } > } I am not sure this hunk is really needed. You are trying to use this code make sure that you start PD with=20 something to deliver, but the PD code already takes care of that. You also get some basic cum_tsn checking for the current chunk because of how renege is called, but you still don't do the checks for=20 subsequent chunks in the queue as I stated above, so you are still subject to possible hangs. -vlad > > sk_mem_reclaim(asoc->base.sk); > > N=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDr=EF=BF=BD=EF=BF=BDy=EF= =BF=BD=EF=BF=BD=EF=BF=BDb=EF=BF=BDX=EF=BF=BD=EF=BF=BD=C7=A7v=EF=BF=BD^=EF= =BF=BD)=DE=BA{.n=EF=BF=BD+=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD{=EF=BF=BD= =EF=BF=BD=EF=BF=BDi=EF=BF=BD{ay=EF=BF=BD=1D=CA=87=DA=99=EF=BF=BD,j=07=EF= =BF=BD=EF=BF=BDf=EF=BF=BD=EF=BF=BD=EF=BF=BDh=EF=BF=BD=EF=BF=BD=EF=BF=BD= z=EF=BF=BD=1E=EF=BF=BDw=EF=BF=BD=EF=BF=BD=EF=BF=BD=0C=EF=BF=BD=EF=BF=BD= =EF=BF=BDj:+v=EF=BF=BD=EF=BF=BD=EF=BF=BDw=EF=BF=BDj=EF=BF=BDm=EF=BF=BD=EF= =BF=BD=EF=BF=BD=EF=BF=BD=07=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDzZ+=EF=BF= =BD=EF=BF=BD=DD=A2j"=EF=BF=BD=EF=BF=BD!tml=3D > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel"= in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/