[PATCH V2] sctp: not send SCTP_PEER_ADDR_CHANGE notifications with failed probe

Hi, Vlad && Deepak && Michael && David && Daniel

V2: Following the advice from Daniel Borkmann, I modified the comments and short log.

From Michael && Deepak

"
 lxr SCTP implementation, doesn't transit the path state to INACTIVE, if it was never confirmed. this leads to SCTP_PEER_ADDRESS_CHANGE notification after each failed probe from this time.
 Is there any specific reason to have same notification to SCTP User with each probe in RTO time period ?
 806 case SCTP_TRANSPORT_DOWN:
 807 /* If the transport was never confirmed, do not transition it
 808 * to inactive state. Also, release the cached route since
 809 * there may be a better route next time.
 810 */
 811 if (transport->state != SCTP_UNCONFIRMED)

 812 transport->state = SCTP_INACTIVE;

 http://lxr.free-electrons.com/source/net/sctp/associola.c#L806

 ......

 I would also only expect a  SCTP_PEER_ADDRESS_CHANGE notification when a path state changes, not on every  try.

"

I made a patch to disable sending SCTP_PEER_ADDRESS_CHANGE notification every try. Now the patch is in the attachment. Please check it.

Zhu Yanjun


Zhu Yanjun (1):
  sctp: not send SCTP_PEER_ADDR_CHANGE notifications with failed probe

 net/sctp/associola.c | 1 +
 1 file changed, 1 insertion(+)

-- 
1.9.1

Since the transport has always been in state SCTP_UNCONFIRMED, it
therefore wasn't active before and hasn't been used before, and it
always has been, so it is unnecessary to bug the user with a 
notification.

Reported-by: Deepak Khandelwal <redacted>  
Suggested-by: Vlad Yasevich <redacted> 
Suggested-by: Michael Tuexen <redacted>
Suggested-by: Daniel Borkmann <redacted>
Signed-off-by: Zhu Yanjun <redacted>
---
 net/sctp/associola.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 9de23a2..2e23f6b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c

@@ -813,6 +813,7 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		else {
 			dst_release(transport->dst);
 			transport->dst = NULL;
+			ulp_notify = false;
 		}
 
 		spc_state = SCTP_ADDR_UNREACHABLE;

-- 
1.9.1

On 08/20/2014 05:31 AM, Zhu Yanjun wrote:

Since the transport has always been in state SCTP_UNCONFIRMED, it
therefore wasn't active before and hasn't been used before, and it
always has been, so it is unnecessary to bug the user with a 
notification.

Reported-by: Deepak Khandelwal <redacted>  
Suggested-by: Vlad Yasevich <redacted> 
Suggested-by: Michael Tuexen <redacted>
Suggested-by: Daniel Borkmann <redacted>
Signed-off-by: Zhu Yanjun <redacted>

Acked-by: Vlad Yasevich <redacted>

Thanks
-vlad

On 08/20/2014 11:31 AM, Zhu Yanjun wrote:

Since the transport has always been in state SCTP_UNCONFIRMED, it
therefore wasn't active before and hasn't been used before, and it
always has been, so it is unnecessary to bug the user with a
notification.

Reported-by: Deepak Khandelwal <redacted>
Suggested-by: Vlad Yasevich <redacted>
Suggested-by: Michael Tuexen <redacted>
Suggested-by: Daniel Borkmann <redacted>
Signed-off-by: Zhu Yanjun <redacted>

Looks a bit better, thanks!

Acked-by: Daniel Borkmann <redacted>

From: Zhu Yanjun <zyjzyj2000@gmail.com>
Date: Wed, 20 Aug 2014 17:31:43 +0800

Since the transport has always been in state SCTP_UNCONFIRMED, it
therefore wasn't active before and hasn't been used before, and it
always has been, so it is unnecessary to bug the user with a 
notification.

Reported-by: Deepak Khandelwal <redacted>  
Suggested-by: Vlad Yasevich <redacted> 
Suggested-by: Michael Tuexen <redacted>
Suggested-by: Daniel Borkmann <redacted>
Signed-off-by: Zhu Yanjun <redacted>

Applied, thanks.