[PATCH] sctp: rfc conformance fixes

[PATCH] sctp: rfc conformance fixes

[Vlad Yasevich]

Hi David

The following series is a set of conformance fixes from Wei Yongjun.
One of them addresses a small bug in pmtu process, and the other three
make the shutdown process conform to the latest spec.

Please apply.

Thanks
-vlad

[PATCH 1/4] sctp: Drop ICMP packet too big message with MTU larger than current PMTU

[Vlad Yasevich]

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If ICMP packet too big message is received with MTU larger than current
PMTU, SCTP will still accept this ICMP message and sync the PMTU of assoc
with the wrong MTU.

Endpoing A                 Endpoint B
(ESTABLISHED)              (ESTABLISHED)
ICMP         --------->
(packet too big, MTU too larger)
                           sync PMTU

This patch fixed the problem by drop that ICMP message.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/input.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index a49fa80..bf612d9 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -369,7 +369,7 @@ static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
 void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
 			   struct sctp_transport *t, __u32 pmtu)
 {
-	if (!t || (t->pathmtu == pmtu))
+	if (!t || (t->pathmtu <= pmtu))
 		return;
 
 	if (sock_owned_by_user(sk)) {
-- 
1.5.3.5

Re: [PATCH 1/4] sctp: Drop ICMP packet too big message with MTU larger than current PMTU

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 21 Oct 2008 22:08:44 -0400

> If ICMP packet too big message is received with MTU larger than current
> PMTU, SCTP will still accept this ICMP message and sync the PMTU of assoc
> with the wrong MTU.
> 
> Endpoing A                 Endpoint B
> (ESTABLISHED)              (ESTABLISHED)
> ICMP         --------->
> (packet too big, MTU too larger)
>                            sync PMTU
> 
> This patch fixed the problem by drop that ICMP message.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

[PATCH 2/4] sctp: Add check for the TSN field of the SHUTDOWN chunk

[Vlad Yasevich]

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If SHUTDOWN chunk is received Cumulative TSN Ack beyond the max tsn currently
send, SHUTDOWN chunk be accepted and the association will be broken. New data
is send, but after received SACK it will be drop because TSN in SACK is less
than the Cumulative TSN, data will be retrans again and again even if correct
SACK is received.

The packet sequence is like this:

Endpoint A                       Endpoint B       ULP
(ESTABLISHED)                    (ESTABLISHED)

               <-----------      DATA (TSN=x-1)

               <-----------      DATA (TSN=x)

  SHUTDOWN     ----------->      (Now Cumulative TSN=x+1000)
  (TSN=x+1000)
               <-----------      DATA (TSN=x+1)

  SACK         ----------->      drop the SACK
  (TSN=x+1)
               <-----------      DATA (TSN=x+1)(retrans)

This patch fix this problem by terminating the association and respond to
the sender with an ABORT.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_statefuns.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index d4c3fbc..12f6217 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2544,6 +2544,7 @@ sctp_disposition_t sctp_sf_do_9_2_shutdown(const struct sctp_endpoint *ep,
 	sctp_shutdownhdr_t *sdh;
 	sctp_disposition_t disposition;
 	struct sctp_ulpevent *ev;
+	__u32 ctsn;
 
 	if (!sctp_vtag_verify(chunk, asoc))
 		return sctp_sf_pdiscard(ep, asoc, type, arg, commands);
@@ -2558,6 +2559,14 @@ sctp_disposition_t sctp_sf_do_9_2_shutdown(const struct sctp_endpoint *ep,
 	sdh = (sctp_shutdownhdr_t *)chunk->skb->data;
 	skb_pull(chunk->skb, sizeof(sctp_shutdownhdr_t));
 	chunk->subh.shutdown_hdr = sdh;
+	ctsn = ntohl(sdh->cum_tsn_ack);
+
+	/* If Cumulative TSN Ack beyond the max tsn currently
+	 * send, terminating the association and respond to the
+	 * sender with an ABORT.
+	 */
+	if (!TSN_lt(ctsn, asoc->next_tsn))
+		return sctp_sf_violation_ctsn(ep, asoc, type, arg, commands);
 
 	/* API 5.3.1.5 SCTP_SHUTDOWN_EVENT
 	 * When a peer sends a SHUTDOWN, SCTP delivers this notification to
-- 
1.5.3.5

Re: [PATCH 2/4] sctp: Add check for the TSN field of the SHUTDOWN chunk

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 21 Oct 2008 22:08:45 -0400

> If SHUTDOWN chunk is received Cumulative TSN Ack beyond the max tsn currently
> send, SHUTDOWN chunk be accepted and the association will be broken. New data
> is send, but after received SACK it will be drop because TSN in SACK is less
> than the Cumulative TSN, data will be retrans again and again even if correct
> SACK is received.
> 
> The packet sequence is like this:
> 
> Endpoint A                       Endpoint B       ULP
> (ESTABLISHED)                    (ESTABLISHED)
> 
>                <-----------      DATA (TSN=x-1)
> 
>                <-----------      DATA (TSN=x)
> 
>   SHUTDOWN     ----------->      (Now Cumulative TSN=x+1000)
>   (TSN=x+1000)
>                <-----------      DATA (TSN=x+1)
> 
>   SACK         ----------->      drop the SACK
>   (TSN=x+1)
>                <-----------      DATA (TSN=x+1)(retrans)
> 
> This patch fix this problem by terminating the association and respond to
> the sender with an ABORT.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

[PATCH 3/4] sctp: Fix to handle SHUTDOWN in SHUTDOWN-PENDING state

[Vlad Yasevich]

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If SHUTDOWN is received in SHUTDOWN-PENDING state, enpoint should enter
the SHUTDOWN-RECEIVED state and check the Cumulative TSN Ack field of
the SHUTDOWN chunk (RFC 4960 Section 9.2). If the SHUTDOWN chunk can
acknowledge all of the send DATA chunks, SHUTDOWN-ACK should be sent.

But now endpoint just silently discarded the SHUTDOWN chunk.

SHUTDOWN received in SHUTDOWN-PENDING state can happend when the last
SACK is lost by network, or the SHUTDOWN chunk can acknowledge all of
the received DATA chunks. The packet sequence(SACK lost) is like this:

Endpoint A                       Endpoint B       ULP
(ESTABLISHED)                    (ESTABLISHED)

               <-----------      DATA
                                             <--- shutdown
                                 Enter SHUTDOWN-PENDING state
  SACK         ----lost---->

  SHUTDOWN(*1) ------------>

               <-----------      SHUTDOWN-ACK

 (*1) silently discarded now.

This patch fix to handle SHUTDOWN in SHUTDOWN-PENDING state as the same
as ESTABLISHED state.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_statetable.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sctp/sm_statetable.c b/net/sctp/sm_statetable.c
index dd4ddc4..a5b5590 100644
--- a/net/sctp/sm_statetable.c
+++ b/net/sctp/sm_statetable.c
@@ -266,7 +266,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
 	/* SCTP_STATE_ESTABLISHED */ \
 	TYPE_SCTP_FUNC(sctp_sf_do_9_2_shutdown), \
 	/* SCTP_STATE_SHUTDOWN_PENDING */ \
-	TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
+	TYPE_SCTP_FUNC(sctp_sf_do_9_2_shutdown), \
 	/* SCTP_STATE_SHUTDOWN_SENT */ \
 	TYPE_SCTP_FUNC(sctp_sf_do_9_2_shutdown_ack), \
 	/* SCTP_STATE_SHUTDOWN_RECEIVED */ \
-- 
1.5.3.5

Re: [PATCH 3/4] sctp: Fix to handle SHUTDOWN in SHUTDOWN-PENDING state

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 21 Oct 2008 22:08:46 -0400

> If SHUTDOWN is received in SHUTDOWN-PENDING state, enpoint should enter
> the SHUTDOWN-RECEIVED state and check the Cumulative TSN Ack field of
> the SHUTDOWN chunk (RFC 4960 Section 9.2). If the SHUTDOWN chunk can
> acknowledge all of the send DATA chunks, SHUTDOWN-ACK should be sent.
> 
> But now endpoint just silently discarded the SHUTDOWN chunk.
> 
> SHUTDOWN received in SHUTDOWN-PENDING state can happend when the last
> SACK is lost by network, or the SHUTDOWN chunk can acknowledge all of
> the received DATA chunks. The packet sequence(SACK lost) is like this:
> 
> Endpoint A                       Endpoint B       ULP
> (ESTABLISHED)                    (ESTABLISHED)
> 
>                <-----------      DATA
>                                              <--- shutdown
>                                  Enter SHUTDOWN-PENDING state
>   SACK         ----lost---->
> 
>   SHUTDOWN(*1) ------------>
> 
>                <-----------      SHUTDOWN-ACK
> 
>  (*1) silently discarded now.
> 
> This patch fix to handle SHUTDOWN in SHUTDOWN-PENDING state as the same
> as ESTABLISHED state.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

[PATCH 4/4] sctp: Fix to handle SHUTDOWN in SHUTDOWN_RECEIVED state

[Vlad Yasevich]

From: Wei Yongjun <yjwei@cn.fujitsu.com>

Once an endpoint has reached the SHUTDOWN-RECEIVED state,
it MUST NOT send a SHUTDOWN in response to a ULP request.
The Cumulative TSN Ack of the received SHUTDOWN chunk
MUST be processed.

This patch fix to process Cumulative TSN Ack of the received
SHUTDOWN chunk in SHUTDOWN_RECEIVED state.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/sm.h    |    1 +
 net/sctp/sm_statefuns.c  |   45 +++++++++++++++++++++++++++++++++++++++++++++
 net/sctp/sm_statetable.c |    2 +-
 3 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index 029a54a..c1dd893 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -125,6 +125,7 @@ sctp_state_fn_t sctp_sf_beat_8_3;
 sctp_state_fn_t sctp_sf_backbeat_8_3;
 sctp_state_fn_t sctp_sf_do_9_2_final;
 sctp_state_fn_t sctp_sf_do_9_2_shutdown;
+sctp_state_fn_t sctp_sf_do_9_2_shut_ctsn;
 sctp_state_fn_t sctp_sf_do_ecn_cwr;
 sctp_state_fn_t sctp_sf_do_ecne;
 sctp_state_fn_t sctp_sf_ootb;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 12f6217..a6a0ea7 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2608,6 +2608,51 @@ out:
 	return disposition;
 }
 
+/*
+ * sctp_sf_do_9_2_shut_ctsn
+ *
+ * Once an endpoint has reached the SHUTDOWN-RECEIVED state,
+ * it MUST NOT send a SHUTDOWN in response to a ULP request.
+ * The Cumulative TSN Ack of the received SHUTDOWN chunk
+ * MUST be processed.
+ */
+sctp_disposition_t sctp_sf_do_9_2_shut_ctsn(const struct sctp_endpoint *ep,
+					   const struct sctp_association *asoc,
+					   const sctp_subtype_t type,
+					   void *arg,
+					   sctp_cmd_seq_t *commands)
+{
+	struct sctp_chunk *chunk = arg;
+	sctp_shutdownhdr_t *sdh;
+
+	if (!sctp_vtag_verify(chunk, asoc))
+		return sctp_sf_pdiscard(ep, asoc, type, arg, commands);
+
+	/* Make sure that the SHUTDOWN chunk has a valid length. */
+	if (!sctp_chunk_length_valid(chunk,
+				      sizeof(struct sctp_shutdown_chunk_t)))
+		return sctp_sf_violation_chunklen(ep, asoc, type, arg,
+						  commands);
+
+	sdh = (sctp_shutdownhdr_t *)chunk->skb->data;
+
+	/* If Cumulative TSN Ack beyond the max tsn currently
+	 * send, terminating the association and respond to the
+	 * sender with an ABORT.
+	 */
+	if (!TSN_lt(ntohl(sdh->cum_tsn_ack), asoc->next_tsn))
+		return sctp_sf_violation_ctsn(ep, asoc, type, arg, commands);
+
+	/* verify, by checking the Cumulative TSN Ack field of the
+	 * chunk, that all its outstanding DATA chunks have been
+	 * received by the SHUTDOWN sender.
+	 */
+	sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_CTSN,
+			SCTP_BE32(sdh->cum_tsn_ack));
+
+	return SCTP_DISPOSITION_CONSUME;
+}
+
 /* RFC 2960 9.2
  * If an endpoint is in SHUTDOWN-ACK-SENT state and receives an INIT chunk
  * (e.g., if the SHUTDOWN COMPLETE was lost) with source and destination
diff --git a/net/sctp/sm_statetable.c b/net/sctp/sm_statetable.c
index a5b5590..5c8186d 100644
--- a/net/sctp/sm_statetable.c
+++ b/net/sctp/sm_statetable.c
@@ -270,7 +270,7 @@ const sctp_sm_table_entry_t *sctp_sm_lookup_event(sctp_event_t event_type,
 	/* SCTP_STATE_SHUTDOWN_SENT */ \
 	TYPE_SCTP_FUNC(sctp_sf_do_9_2_shutdown_ack), \
 	/* SCTP_STATE_SHUTDOWN_RECEIVED */ \
-	TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
+	TYPE_SCTP_FUNC(sctp_sf_do_9_2_shut_ctsn), \
 	/* SCTP_STATE_SHUTDOWN_ACK_SENT */ \
 	TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
 } /* TYPE_SCTP_SHUTDOWN */
-- 
1.5.3.5

Re: [PATCH 4/4] sctp: Fix to handle SHUTDOWN in SHUTDOWN_RECEIVED state

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Tue, 21 Oct 2008 22:08:47 -0400

> Once an endpoint has reached the SHUTDOWN-RECEIVED state,
> it MUST NOT send a SHUTDOWN in response to a ULP request.
> The Cumulative TSN Ack of the received SHUTDOWN chunk
> MUST be processed.
> 
> This patch fix to process Cumulative TSN Ack of the received
> SHUTDOWN chunk in SHUTDOWN_RECEIVED state.
> 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

Please CC: netdev on any and all patches you want applied.

That way it gets tracked properly at:

	http://patchwork.ozlabs.org/project/netdev/list/

otherwise, your patch "might" (read this as: will) get lost.
:-)

Re: [PATCH] sctp: rfc conformance fixes

[Maxime Bizon]

On Tue, 2008-10-21 at 21:05 -0700, David Miller wrote:

Hello David,

> Please CC: netdev on any and all patches you want applied.
> 
> That way it gets tracked properly at:
> 
> 	http://patchwork.ozlabs.org/project/netdev/list/

The feeder of patchwork does not seem to consider posts with subject
like '[PATCH/RFC' '[PATCH,RFC]', yet I see '[RFC]' is ok.

Should I repost with another subject or RFC patches are not supposed to
be in patchwork ?

-- 
Maxime

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Maxime Bizon <mbizon@freebox.fr>
Date: Wed, 22 Oct 2008 06:44:58 +0200

> On Tue, 2008-10-21 at 21:05 -0700, David Miller wrote:
> 
> > Please CC: netdev on any and all patches you want applied.
> > 
> > That way it gets tracked properly at:
> > 
> > 	http://patchwork.ozlabs.org/project/netdev/list/
> 
> The feeder of patchwork does not seem to consider posts with subject
> like '[PATCH/RFC' '[PATCH,RFC]', yet I see '[RFC]' is ok.
> 
> Should I repost with another subject or RFC patches are not supposed to
> be in patchwork ?

Hmmm, that would be very strange if it were true.

Jeremy, patchwork doesn't try to interpret the subject line
at all does it?  My impression is that it simply looked for
patches in the email posting content and that's it.

Re: [PATCH] sctp: rfc conformance fixes

[Jeremy Kerr]

Hi,

> Jeremy, patchwork doesn't try to interpret the subject line
> at all does it?  My impression is that it simply looked for
> patches in the email posting content and that's it.

That's correct - patchwork uses the content of the mail to determine if 
it contains a patch, not the subject.

Patchwork does make a few modifications to the subject though - mostly 
treatment of []-enclosed prefixes. For example, it removes [PATCH] from 
the subject line - if it's in patchwork, it's a patch :).

So, in this case, the following subjects:

 [RFC] fix breakage
 [PATCH,RFC] fix breakage
 [RFC,PATCH] fix breakage
 [PATCH/RFC] fix breakage

will all be normalised to:

 [RFC] fix breakage

Cheers,


Jeremy

Re: [PATCH] sctp: rfc conformance fixes

[Maxime Bizon]

On Wed, 2008-10-22 at 16:01 +1100, Jeremy Kerr wrote:

Hi Jeremy,

> That's correct - patchwork uses the content of the mail to determine
> if it contains a patch, not the subject.

Thanks for explaining.

I sent a patch sunday (using git-format-patch/git-send-email), and
patchwork seems to have missed it, I can't see it even with all filters
removed.

From:   Maxime Bizon <mbizon@freebox.fr>
To:     jgarzik@pobox.com
Cc:     linux-mips@linux-mips.org, netdev@vger.kernel.org, Maxime Bizon <mbizon@freebox.fr>
Message-Id: <1224382023-24388-1-git-send-email-mbizon@freebox.fr>
Subject: [PATCH/RFC v1 09/12] [MIPS] BCM63XX: Add integrated ethernet mac support.
Sender: netdev-owner@vger.kernel.org

Any idea ?

-- 
Maxime

Re: [PATCH] sctp: rfc conformance fixes

[Jeremy Kerr]

Hi Maxime,

> I sent a patch sunday (using git-format-patch/git-send-email), and
> patchwork seems to have missed it, I can't see it even with all
> filters removed.

There are some conditions where the parser will reject the patch 
outright, usually due to character encoding breakages (either in the 
email itself or the parser code).

If you could send me an mbox file containing the email, I'll see what's 
going on (and add a testcase to patchwork).

Cheers,


Jeremy

Re: [PATCH] sctp: rfc conformance fixes

[Maxime Bizon]

On Wed, 2008-10-22 at 16:35 +1100, Jeremy Kerr wrote:

Hi,

> If you could send me an mbox file containing the email, I'll see what's 
> going on (and add a testcase to patchwork).

This is exactly what I gave to git-send-email:

http://88.191.35.171/bcm63xx/patches/linux-mips-bcm63xx/v1/0009-MIPS-BCM63XX-Add-integrated-ethernet-mac-support.patch

Thanks for helping !

-- 
Maxime

Re: [PATCH] sctp: rfc conformance fixes

[Vlad Yasevich]

Hi Jeremy

Jeremy Kerr wrote:
> Hi Maxime,
> 
>> I sent a patch sunday (using git-format-patch/git-send-email), and
>> patchwork seems to have missed it, I can't see it even with all
>> filters removed.
> 
> There are some conditions where the parser will reject the patch 
> outright, usually due to character encoding breakages (either in the 
> email itself or the parser code).
> 
> If you could send me an mbox file containing the email, I'll see what's 
> going on (and add a testcase to patchwork).
> 

Oddly enough, none of this series made it to patchwork.

I've put all the patches here:

http://www.kernel.org/pub/linux/kernel/people/vxy/for_jkerr/

These were generated used git-format-patch and send via git-send-email
with git version 1.5.3.5.

-vlad

Re: [PATCH] sctp: rfc conformance fixes

[Jeremy Kerr]

Hi Vlad,

> Oddly enough, none of this series made it to patchwork.

I'll chase this up then - could you formward a copy of one of the 
messages after it has come through the list?

Cheers,


Jeremy

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

[-- Attachment #1: Type: Text/Plain, Size: 275 bytes --]

From: Jeremy Kerr <jk@ozlabs.org>
Date: Thu, 23 Oct 2008 09:54:02 +1100

> > Oddly enough, none of this series made it to patchwork.
> 
> I'll chase this up then - could you formward a copy of one of the 
> messages after it has come through the list?

Here is one of them.


[-- Attachment #2: x.txt --]
[-- Type: Text/Plain, Size: 2339 bytes --]

Return-Path: <vladislav.yasevich@hp.com>
X-Original-To: davem@davemloft.net
Delivered-To: davem@davemloft.net
Received: from g5t0008.atlanta.hp.com (g5t0008.atlanta.hp.com [15.192.0.45])
	by sunset.davemloft.net (Postfix) with ESMTP id B6A09C8C192
	for <davem@davemloft.net>; Tue, 21 Oct 2008 19:08:29 -0700 (PDT)
Received: from smtp2.fc.hp.com (smtp.cnd.hp.com [15.11.136.114])
	by g5t0008.atlanta.hp.com (Postfix) with ESMTP id 7C98524554;
	Wed, 22 Oct 2008 02:08:50 +0000 (UTC)
Received: from localhost.localdomain (squirrel.fc.hp.com [15.11.146.57])
	by smtp2.fc.hp.com (Postfix) with ESMTP id 3B1CC2892FA;
	Wed, 22 Oct 2008 01:55:45 +0000 (UTC)
From: Vlad Yasevich <vladislav.yasevich@hp.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org,
	Wei Yongjun <yjwei@cn.fujitsu.com>,
	Vlad Yasevich <vladislav.yasevich@hp.com>
Subject: [PATCH 1/4] sctp: Drop ICMP packet too big message with MTU larger than current PMTU
Date: Tue, 21 Oct 2008 22:08:44 -0400
Message-Id: <1224641327-11913-2-git-send-email-vladislav.yasevich@hp.com>
X-Mailer: git-send-email 1.5.3.5
In-Reply-To: <1224641327-11913-1-git-send-email-vladislav.yasevich@hp.com>
References: <1224641327-11913-1-git-send-email-vladislav.yasevich@hp.com>

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If ICMP packet too big message is received with MTU larger than current
PMTU, SCTP will still accept this ICMP message and sync the PMTU of assoc
with the wrong MTU.

Endpoing A                 Endpoint B
(ESTABLISHED)              (ESTABLISHED)
ICMP         --------->
(packet too big, MTU too larger)
                           sync PMTU

This patch fixed the problem by drop that ICMP message.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/input.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sctp/input.c b/net/sctp/input.c
index a49fa80..bf612d9 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -369,7 +369,7 @@ static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
 void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
 			   struct sctp_transport *t, __u32 pmtu)
 {
-	if (!t || (t->pathmtu == pmtu))
+	if (!t || (t->pathmtu <= pmtu))
 		return;
 
 	if (sock_owned_by_user(sk)) {
-- 
1.5.3.5

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Wed, 22 Oct 2008 16:46:36 -0700 (PDT)

> From: Jeremy Kerr <jk@ozlabs.org>
> Date: Thu, 23 Oct 2008 09:54:02 +1100
> 
> > > Oddly enough, none of this series made it to patchwork.
> > 
> > I'll chase this up then - could you formward a copy of one of the 
> > messages after it has come through the list?
> 
> Here is one of them.

Sorry, I'm a bozo, that was a copy that made it privately to me.

I don't have any copies in my inbox that came via vger.

Re: [PATCH] sctp: rfc conformance fixes

[Vlad Yasevich]

David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 22 Oct 2008 16:46:36 -0700 (PDT)
> 
>> From: Jeremy Kerr <jk@ozlabs.org>
>> Date: Thu, 23 Oct 2008 09:54:02 +1100
>>
>>>> Oddly enough, none of this series made it to patchwork.
>>> I'll chase this up then - could you formward a copy of one of the 
>>> messages after it has come through the list?
>> Here is one of them.
> 
> Sorry, I'm a bozo, that was a copy that made it privately to me.
> 
> I don't have any copies in my inbox that came via vger.
> 

Really?  They made it to vger and back out.  They are also in
the archive or marc.   I wonder which black hole some of them
disappeared to... :)

-vlad

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Wed, 22 Oct 2008 21:24:47 -0400

> David Miller wrote:
> > From: David Miller <davem@davemloft.net>
> > Date: Wed, 22 Oct 2008 16:46:36 -0700 (PDT)
> > 
> >> From: Jeremy Kerr <jk@ozlabs.org>
> >> Date: Thu, 23 Oct 2008 09:54:02 +1100
> >>
> >>>> Oddly enough, none of this series made it to patchwork.
> >>> I'll chase this up then - could you formward a copy of one of the 
> >>> messages after it has come through the list?
> >> Here is one of them.
> > 
> > Sorry, I'm a bozo, that was a copy that made it privately to me.
> > 
> > I don't have any copies in my inbox that came via vger.
> > 
> 
> Really?  They made it to vger and back out.  They are also in
> the archive or marc.   I wonder which black hole some of them
> disappeared to... :)

I meant that I didn't save any such copies.

When I read my email I always get 2, 3, 4 copies of every damn
patch, so I delete all but one of those and I happened to save only
the instances that were directly sent to me this time.  It's just
random what happens.

Re: [PATCH] sctp: rfc conformance fixes

[Simon Horman]

On Wed, Oct 22, 2008 at 09:06:49PM -0700, David Miller wrote:
> From: Vlad Yasevich <vladislav.yasevich@hp.com>
> Date: Wed, 22 Oct 2008 21:24:47 -0400
> 
> > David Miller wrote:
> > > From: David Miller <davem@davemloft.net>
> > > Date: Wed, 22 Oct 2008 16:46:36 -0700 (PDT)
> > > 
> > >> From: Jeremy Kerr <jk@ozlabs.org>
> > >> Date: Thu, 23 Oct 2008 09:54:02 +1100
> > >>
> > >>>> Oddly enough, none of this series made it to patchwork.
> > >>> I'll chase this up then - could you formward a copy of one of the 
> > >>> messages after it has come through the list?
> > >> Here is one of them.
> > > 
> > > Sorry, I'm a bozo, that was a copy that made it privately to me.
> > > 
> > > I don't have any copies in my inbox that came via vger.
> > > 
> > 
> > Really?  They made it to vger and back out.  They are also in
> > the archive or marc.   I wonder which black hole some of them
> > disappeared to... :)
> 
> I meant that I didn't save any such copies.
> 
> When I read my email I always get 2, 3, 4 copies of every damn
> patch, so I delete all but one of those and I happened to save only
> the instances that were directly sent to me this time.  It's just
> random what happens.

For what it is worth, I use the following procmail rule to elimintate
duplicate messages based on Message-Id.

	PROCMAIL_ROOT=${HOME}/.procmail

        :0
	* ? /usr/bin/formail -D 819200 ${PROCMAIL_ROOT}/log/message.cache
	/dev/null

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Simon Horman <horms@verge.net.au>
Date: Thu, 23 Oct 2008 17:15:16 +1100

> For what it is worth, I use the following procmail rule to elimintate
> duplicate messages based on Message-Id.

Thanks Simon, I'll check it out.

Re: [PATCH] sctp: rfc conformance fixes

[Jeremy Kerr]

Hi Vlad,

> I've attached one of the messages here.  Is the format ok?

That's great, thanks.

Looks like the email is fine, I'll see what's happening on the patchwork 
side.

Cheers,


Jeremy

Re: [PATCH] sctp: rfc conformance fixes

[Jeremy Kerr]

Hi David. Maxime & Vlad,

> Looks like the email is fine, I'll see what's happening on the
> patchwork side.

OK, patchwork didn't like that the mail had no Content-Type header, so 
has no character set encoding defined. I've just checked-in change to 
assume a utf-8 encoding by default:

http://git.ozlabs.org/?p=patchwork;a=commitdiff;h=d45218b2e3894211e11313820bea9f59677c4bf2

Which should mean that future patches will be accepted.

(if you'd like me to manually re-inject that series of patches, just 
forward the mboxes over to me)

Cheers,


Jeremy

 

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Jeremy Kerr <jk@ozlabs.org>
Date: Thu, 23 Oct 2008 14:34:47 +1100

> OK, patchwork didn't like that the mail had no Content-Type header, so 
> has no character set encoding defined. I've just checked-in change to 
> assume a utf-8 encoding by default:
> 
> http://git.ozlabs.org/?p=patchwork;a=commitdiff;h=d45218b2e3894211e11313820bea9f59677c4bf2
> 
> Which should mean that future patches will be accepted.

Thanks a lot for fixing this Jeremy.  I bet some other patches got
eaten by this too.

> (if you'd like me to manually re-inject that series of patches, just 
> forward the mboxes over to me)

Not needed, I'll apply Vlad's patches since I have them in my inbox.

Re: [PATCH] sctp: rfc conformance fixes

[Benjamin Herrenschmidt]

On Wed, 2008-10-22 at 21:11 -0700, David Miller wrote:
> > OK, patchwork didn't like that the mail had no Content-Type header, so 
> > has no character set encoding defined. I've just checked-in change to 
> > assume a utf-8 encoding by default:
> > 
> > http://git.ozlabs.org/?p=patchwork;a=commitdiff;h=d45218b2e3894211e11313820bea9f59677c4bf2
> > 
> > Which should mean that future patches will be accepted.
> 
> Thanks a lot for fixing this Jeremy.  I bet some other patches got
> eaten by this too.
> 
> > (if you'd like me to manually re-inject that series of patches, just 
> > forward the mboxes over to me)
> 
> Not needed, I'll apply Vlad's patches since I have them in my inbox.

Since patchwork2 is new and might still occasionally have such a
problem, I though of a way to recover these...

If we keep an archive of the list, we can use patchwork hashing feature
to basically generate hashes for all patches in the archive over the
last N month, and then use the patchwork client interface to check if
they are all indeed referenced there. If not, we can then re-inject
them.

Typically a tool that could be used after we fix that sort of bug.
(There have been a couple of other similar ones related to weird
encodings in the past).

Cheers,
Ben.

Re: [PATCH] sctp: rfc conformance fixes

[Jeremy Kerr]

Ben,

> If we keep an archive of the list, we can use patchwork hashing
> feature to basically generate hashes for all patches in the archive
> over the last N month, and then use the patchwork client interface to
> check if they are all indeed referenced there. If not, we can then
> re-inject them.

Simpler way - I just get the patchwork parser to store any mail that it 
rejects.

It's next on the patchwork todo list.

Cheers,


Jeremy

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Maxime Bizon <mbizon@freebox.fr>
Date: Wed, 22 Oct 2008 07:32:09 +0200

> I sent a patch sunday (using git-format-patch/git-send-email), and
> patchwork seems to have missed it, I can't see it even with all filters
> removed.
 ...
> Any idea ?

I think I might have accidently marked it as "not relevant" thinking
the MIPS tree would pick it up.

Once the patch gets turned down in some way, it doesn't get archived
due to current settings, maybe we should change that.

Re: [PATCH] sctp: rfc conformance fixes

[Maxime Bizon]

On Tue, 2008-10-21 at 23:12 -0700, David Miller wrote:

Hi David,

> I think I might have accidently marked it as "not relevant" thinking
> the MIPS tree would pick it up.

At least you did not think "that crap belongs to -staging" :)

Ralf told me to sent it to Jeff Garzik, so I did. If you think this
should go through the mips tree instead please tell me, or else I can
repost it.

-- 
Maxime

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Maxime Bizon <mbizon@freebox.fr>
Date: Wed, 22 Oct 2008 18:48:15 +0200

> 
> On Tue, 2008-10-21 at 23:12 -0700, David Miller wrote:
> 
> Hi David,
> 
> > I think I might have accidently marked it as "not relevant" thinking
> > the MIPS tree would pick it up.
> 
> At least you did not think "that crap belongs to -staging" :)
> 
> Ralf told me to sent it to Jeff Garzik, so I did. If you think this
> should go through the mips tree instead please tell me, or else I can
> repost it.

Going through Jeff was correct then :-)

Re: [PATCH] sctp: rfc conformance fixes

[Vlad Yasevich]

David Miller wrote:
> Please CC: netdev on any and all patches you want applied.
> 
> That way it gets tracked properly at:
> 
> 	http://patchwork.ozlabs.org/project/netdev/list/
> 
> otherwise, your patch "might" (read this as: will) get lost.
> :-)
> 

David

Do you want me to resubmit?

-vlad

Re: [PATCH] sctp: rfc conformance fixes

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Wed, 22 Oct 2008 19:42:57 -0400

> Do you want me to resubmit?

No need.