[PATCHv2 net-next 0/5] sctp: update from rfc7829

[PATCHv2 net-next 0/5] sctp: update from rfc7829

[Xin Long]

SCTP-PF was implemented based on a Internet-Draft in 2012:

  https://tools.ietf.org/html/draft-nishida-tsvwg-sctp-failover-05

It's been updated quite a few by rfc7829 in 2016.

This patchset adds the following features:

  1. add SCTP_ADDR_POTENTIALLY_FAILED notification
  2. add pf_expose per netns/sock/asoc
  3. add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt
  4. add ps_retrans per netns/sock/asoc/transport
     (Primary Path Switchover)
  5. add spt_pathcpthld for SCTP_PEER_ADDR_THLDS sockopt

v1->v2:
  - See Patch 2/5 and Patch 5/5.

Xin Long (5):
  sctp: add SCTP_ADDR_POTENTIALLY_FAILED notification
  sctp: add pf_expose per netns and sock and asoc
  sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt
  sctp: add support for Primary Path Switchover
  sctp: add SCTP_PEER_ADDR_THLDS_V2 sockopt

 include/net/netns/sctp.h   |  13 +++++
 include/net/sctp/structs.h |  13 ++++-
 include/uapi/linux/sctp.h  |  13 +++++
 net/sctp/associola.c       |  26 ++++-----
 net/sctp/protocol.c        |   6 ++
 net/sctp/sm_sideeffect.c   |   5 ++
 net/sctp/socket.c          | 143 ++++++++++++++++++++++++++++++++++++++++-----
 net/sctp/sysctl.c          |  16 +++++
 8 files changed, 203 insertions(+), 32 deletions(-)

-- 
2.1.0

[PATCHv2 net-next 0/5] sctp: update from rfc7829

[Xin Long]

SCTP-PF was implemented based on a Internet-Draft in 2012:

  https://tools.ietf.org/html/draft-nishida-tsvwg-sctp-failover-05

It's been updated quite a few by rfc7829 in 2016.

This patchset adds the following features:

  1. add SCTP_ADDR_POTENTIALLY_FAILED notification
  2. add pf_expose per netns/sock/asoc
  3. add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt
  4. add ps_retrans per netns/sock/asoc/transport
     (Primary Path Switchover)
  5. add spt_pathcpthld for SCTP_PEER_ADDR_THLDS sockopt

v1->v2:
  - See Patch 2/5 and Patch 5/5.

Xin Long (5):
  sctp: add SCTP_ADDR_POTENTIALLY_FAILED notification
  sctp: add pf_expose per netns and sock and asoc
  sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt
  sctp: add support for Primary Path Switchover
  sctp: add SCTP_PEER_ADDR_THLDS_V2 sockopt

 include/net/netns/sctp.h   |  13 +++++
 include/net/sctp/structs.h |  13 ++++-
 include/uapi/linux/sctp.h  |  13 +++++
 net/sctp/associola.c       |  26 ++++-----
 net/sctp/protocol.c        |   6 ++
 net/sctp/sm_sideeffect.c   |   5 ++
 net/sctp/socket.c          | 143 ++++++++++++++++++++++++++++++++++++++++-----
 net/sctp/sysctl.c          |  16 +++++
 8 files changed, 203 insertions(+), 32 deletions(-)

-- 
2.1.0

[PATCHv2 net-next 1/5] sctp: add SCTP_ADDR_POTENTIALLY_FAILED notification

[Xin Long]

SCTP Quick failover draft section 5.1, point 5 has been removed
from rfc7829. Instead, "the sender SHOULD (i) notify the Upper
Layer Protocol (ULP) about this state transition", as said in
section 3.2, point 8.

So this patch is to add SCTP_ADDR_POTENTIALLY_FAILED, defined
in section 7.1, "which is reported if the affected address
becomes PF". Also remove transport cwnd's update when moving
from PF back to ACTIVE , which is no longer in rfc7829 either.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |  1 +
 net/sctp/associola.c      | 17 ++++-------------
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 6d5b164..45a85d7 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -410,6 +410,7 @@ enum sctp_spc_state {
 	SCTP_ADDR_ADDED,
 	SCTP_ADDR_MADE_PRIM,
 	SCTP_ADDR_CONFIRMED,
+	SCTP_ADDR_POTENTIALLY_FAILED,
 };
 
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index d2ffc9a..7278b7e 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -798,14 +798,6 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 			spc_state = SCTP_ADDR_CONFIRMED;
 		else
 			spc_state = SCTP_ADDR_AVAILABLE;
-		/* Don't inform ULP about transition from PF to
-		 * active state and set cwnd to 1 MTU, see SCTP
-		 * Quick failover draft section 5.1, point 5
-		 */
-		if (transport->state == SCTP_PF) {
-			ulp_notify = false;
-			transport->cwnd = asoc->pathmtu;
-		}
 		transport->state = SCTP_ACTIVE;
 		break;
 
@@ -814,19 +806,18 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		 * to inactive state.  Also, release the cached route since
 		 * there may be a better route next time.
 		 */
-		if (transport->state != SCTP_UNCONFIRMED)
+		if (transport->state != SCTP_UNCONFIRMED) {
 			transport->state = SCTP_INACTIVE;
-		else {
+			spc_state = SCTP_ADDR_UNREACHABLE;
+		} else {
 			sctp_transport_dst_release(transport);
 			ulp_notify = false;
 		}
-
-		spc_state = SCTP_ADDR_UNREACHABLE;
 		break;
 
 	case SCTP_TRANSPORT_PF:
 		transport->state = SCTP_PF;
-		ulp_notify = false;
+		spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
 		break;
 
 	default:
-- 
2.1.0

[PATCHv2 net-next 1/5] sctp: add SCTP_ADDR_POTENTIALLY_FAILED notification

[Xin Long]

SCTP Quick failover draft section 5.1, point 5 has been removed
from rfc7829. Instead, "the sender SHOULD (i) notify the Upper
Layer Protocol (ULP) about this state transition", as said in
section 3.2, point 8.

So this patch is to add SCTP_ADDR_POTENTIALLY_FAILED, defined
in section 7.1, "which is reported if the affected address
becomes PF". Also remove transport cwnd's update when moving
from PF back to ACTIVE , which is no longer in rfc7829 either.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |  1 +
 net/sctp/associola.c      | 17 ++++-------------
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 6d5b164..45a85d7 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -410,6 +410,7 @@ enum sctp_spc_state {
 	SCTP_ADDR_ADDED,
 	SCTP_ADDR_MADE_PRIM,
 	SCTP_ADDR_CONFIRMED,
+	SCTP_ADDR_POTENTIALLY_FAILED,
 };
 
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index d2ffc9a..7278b7e 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -798,14 +798,6 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 			spc_state = SCTP_ADDR_CONFIRMED;
 		else
 			spc_state = SCTP_ADDR_AVAILABLE;
-		/* Don't inform ULP about transition from PF to
-		 * active state and set cwnd to 1 MTU, see SCTP
-		 * Quick failover draft section 5.1, point 5
-		 */
-		if (transport->state = SCTP_PF) {
-			ulp_notify = false;
-			transport->cwnd = asoc->pathmtu;
-		}
 		transport->state = SCTP_ACTIVE;
 		break;
 
@@ -814,19 +806,18 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		 * to inactive state.  Also, release the cached route since
 		 * there may be a better route next time.
 		 */
-		if (transport->state != SCTP_UNCONFIRMED)
+		if (transport->state != SCTP_UNCONFIRMED) {
 			transport->state = SCTP_INACTIVE;
-		else {
+			spc_state = SCTP_ADDR_UNREACHABLE;
+		} else {
 			sctp_transport_dst_release(transport);
 			ulp_notify = false;
 		}
-
-		spc_state = SCTP_ADDR_UNREACHABLE;
 		break;
 
 	case SCTP_TRANSPORT_PF:
 		transport->state = SCTP_PF;
-		ulp_notify = false;
+		spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
 		break;
 
 	default:
-- 
2.1.0

[PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[Xin Long]

As said in rfc7829, section 3, point 12:

  The SCTP stack SHOULD expose the PF state of its destination
  addresses to the ULP as well as provide the means to notify the
  ULP of state transitions of its destination addresses from
  active to PF, and vice versa.  However, it is recommended that
  an SCTP stack implementing SCTP-PF also allows for the ULP to be
  kept ignorant of the PF state of its destinations and the
  associated state transitions, thus allowing for retention of the
  simpler state transition model of [RFC4960] in the ULP.

Not only does it allow to expose the PF state to ULP, but also
allow to ignore sctp-pf to ULP.

So this patch is to add pf_expose per netns, sock and asoc. And in
sctp_assoc_control_transport(), ulp_notify will be set to false if
asoc->expose is not set.

It also allows a user to change pf_expose per netns by sysctl, and
pf_expose per sock and asoc will be initialized with it.

Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
to not allow a user to query the state of a sctp-pf peer address
when pf_expose is not enabled, as said in section 7.3.

v1->v2:
  - Fix a build warning noticed by Nathan Chancellor.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/netns/sctp.h   |  7 +++++++
 include/net/sctp/structs.h |  2 ++
 include/uapi/linux/sctp.h  |  1 +
 net/sctp/associola.c       |  8 +++++++-
 net/sctp/protocol.c        |  3 +++
 net/sctp/socket.c          | 12 ++++++++++--
 net/sctp/sysctl.c          |  7 +++++++
 7 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
index bdc0f27..5234940c 100644
--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -97,6 +97,13 @@ struct netns_sctp {
 	int pf_enable;
 
 	/*
+	 * Disable Potentially-Failed state exposure, enabled by default
+	 * pf_expose	-  0  : disable pf state exposure
+	 *		- >0  : enable  pf state exposure
+	 */
+	int pf_expose;
+
+	/*
 	 * Policy for preforming sctp/socket accounting
 	 * 0   - do socket level accounting, all assocs share sk_sndbuf
 	 * 1   - do sctp accounting, each asoc may use sk_sndbuf bytes
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 503fbc3..c2d3317 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -215,6 +215,7 @@ struct sctp_sock {
 	__u32 adaptation_ind;
 	__u32 pd_point;
 	__u16	nodelay:1,
+		pf_expose:1,
 		reuse:1,
 		disable_fragments:1,
 		v4mapped:1,
@@ -2053,6 +2054,7 @@ struct sctp_association {
 
 	__u8 need_ecne:1,	/* Need to send an ECNE Chunk? */
 	     temp:1,		/* Is it a temporary association? */
+	     pf_expose:1,       /* Expose pf state? */
 	     force_delay:1;
 
 	__u8 strreset_enable;
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 45a85d7..b6649d6 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -920,6 +920,7 @@ struct sctp_paddrinfo {
 enum sctp_spinfo_state {
 	SCTP_INACTIVE,
 	SCTP_PF,
+#define	SCTP_POTENTIALLY_FAILED		SCTP_PF
 	SCTP_ACTIVE,
 	SCTP_UNCONFIRMED,
 	SCTP_UNKNOWN = 0xffff  /* Value used for transport state unknown */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 7278b7e..1c30fda 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -86,6 +86,7 @@ static struct sctp_association *sctp_association_init(
 	 */
 	asoc->max_retrans = sp->assocparams.sasoc_asocmaxrxt;
 	asoc->pf_retrans  = sp->pf_retrans;
+	asoc->pf_expose   = sp->pf_expose;
 
 	asoc->rto_initial = msecs_to_jiffies(sp->rtoinfo.srto_initial);
 	asoc->rto_max = msecs_to_jiffies(sp->rtoinfo.srto_max);
@@ -793,6 +794,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		 * to heartbeat success, report the SCTP_ADDR_CONFIRMED
 		 * state to the user, otherwise report SCTP_ADDR_AVAILABLE.
 		 */
+		if (transport->state == SCTP_PF && !asoc->pf_expose)
+			ulp_notify = false;
 		if (SCTP_UNCONFIRMED == transport->state &&
 		    SCTP_HEARTBEAT_SUCCESS == error)
 			spc_state = SCTP_ADDR_CONFIRMED;
@@ -817,7 +820,10 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 
 	case SCTP_TRANSPORT_PF:
 		transport->state = SCTP_PF;
-		spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
+		if (!asoc->pf_expose)
+			ulp_notify = false;
+		else
+			spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
 		break;
 
 	default:
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 08d14d8..a303011 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
 	/* Enable pf state by default */
 	net->sctp.pf_enable = 1;
 
+	/* Enable pf state exposure by default */
+	net->sctp.pf_expose = 1;
+
 	/* Association.Max.Retrans  - 10 attempts
 	 * Path.Max.Retrans         - 5  attempts (per destination address)
 	 * Max.Init.Retransmits     - 8  attempts
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 939b8d2..8d27434 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5041,6 +5041,7 @@ static int sctp_init_sock(struct sock *sk)
 	sp->hbinterval  = net->sctp.hb_interval;
 	sp->pathmaxrxt  = net->sctp.max_retrans_path;
 	sp->pf_retrans  = net->sctp.pf_retrans;
+	sp->pf_expose   = net->sctp.pf_expose;
 	sp->pathmtu     = 0; /* allow default discovery */
 	sp->sackdelay   = net->sctp.sack_timeout;
 	sp->sackfreq	= 2;
@@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
 
 	transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
 					   pinfo.spinfo_assoc_id);
-	if (!transport)
-		return -EINVAL;
+	if (!transport) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	if (transport->state == SCTP_PF && !transport->asoc->pf_expose) {
+		retval = -EACCES;
+		goto out;
+	}
 
 	pinfo.spinfo_assoc_id = sctp_assoc2id(transport->asoc);
 	pinfo.spinfo_state = transport->state;
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 238cf17..eacc9a1 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "pf_expose",
+		.data		= &init_net.sctp.pf_expose,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
 
 	{ /* sentinel */ }
 };
-- 
2.1.0

[PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[Xin Long]

As said in rfc7829, section 3, point 12:

  The SCTP stack SHOULD expose the PF state of its destination
  addresses to the ULP as well as provide the means to notify the
  ULP of state transitions of its destination addresses from
  active to PF, and vice versa.  However, it is recommended that
  an SCTP stack implementing SCTP-PF also allows for the ULP to be
  kept ignorant of the PF state of its destinations and the
  associated state transitions, thus allowing for retention of the
  simpler state transition model of [RFC4960] in the ULP.

Not only does it allow to expose the PF state to ULP, but also
allow to ignore sctp-pf to ULP.

So this patch is to add pf_expose per netns, sock and asoc. And in
sctp_assoc_control_transport(), ulp_notify will be set to false if
asoc->expose is not set.

It also allows a user to change pf_expose per netns by sysctl, and
pf_expose per sock and asoc will be initialized with it.

Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
to not allow a user to query the state of a sctp-pf peer address
when pf_expose is not enabled, as said in section 7.3.

v1->v2:
  - Fix a build warning noticed by Nathan Chancellor.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/netns/sctp.h   |  7 +++++++
 include/net/sctp/structs.h |  2 ++
 include/uapi/linux/sctp.h  |  1 +
 net/sctp/associola.c       |  8 +++++++-
 net/sctp/protocol.c        |  3 +++
 net/sctp/socket.c          | 12 ++++++++++--
 net/sctp/sysctl.c          |  7 +++++++
 7 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
index bdc0f27..5234940c 100644
--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -97,6 +97,13 @@ struct netns_sctp {
 	int pf_enable;
 
 	/*
+	 * Disable Potentially-Failed state exposure, enabled by default
+	 * pf_expose	-  0  : disable pf state exposure
+	 *		- >0  : enable  pf state exposure
+	 */
+	int pf_expose;
+
+	/*
 	 * Policy for preforming sctp/socket accounting
 	 * 0   - do socket level accounting, all assocs share sk_sndbuf
 	 * 1   - do sctp accounting, each asoc may use sk_sndbuf bytes
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 503fbc3..c2d3317 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -215,6 +215,7 @@ struct sctp_sock {
 	__u32 adaptation_ind;
 	__u32 pd_point;
 	__u16	nodelay:1,
+		pf_expose:1,
 		reuse:1,
 		disable_fragments:1,
 		v4mapped:1,
@@ -2053,6 +2054,7 @@ struct sctp_association {
 
 	__u8 need_ecne:1,	/* Need to send an ECNE Chunk? */
 	     temp:1,		/* Is it a temporary association? */
+	     pf_expose:1,       /* Expose pf state? */
 	     force_delay:1;
 
 	__u8 strreset_enable;
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 45a85d7..b6649d6 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -920,6 +920,7 @@ struct sctp_paddrinfo {
 enum sctp_spinfo_state {
 	SCTP_INACTIVE,
 	SCTP_PF,
+#define	SCTP_POTENTIALLY_FAILED		SCTP_PF
 	SCTP_ACTIVE,
 	SCTP_UNCONFIRMED,
 	SCTP_UNKNOWN = 0xffff  /* Value used for transport state unknown */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 7278b7e..1c30fda 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -86,6 +86,7 @@ static struct sctp_association *sctp_association_init(
 	 */
 	asoc->max_retrans = sp->assocparams.sasoc_asocmaxrxt;
 	asoc->pf_retrans  = sp->pf_retrans;
+	asoc->pf_expose   = sp->pf_expose;
 
 	asoc->rto_initial = msecs_to_jiffies(sp->rtoinfo.srto_initial);
 	asoc->rto_max = msecs_to_jiffies(sp->rtoinfo.srto_max);
@@ -793,6 +794,8 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		 * to heartbeat success, report the SCTP_ADDR_CONFIRMED
 		 * state to the user, otherwise report SCTP_ADDR_AVAILABLE.
 		 */
+		if (transport->state = SCTP_PF && !asoc->pf_expose)
+			ulp_notify = false;
 		if (SCTP_UNCONFIRMED = transport->state &&
 		    SCTP_HEARTBEAT_SUCCESS = error)
 			spc_state = SCTP_ADDR_CONFIRMED;
@@ -817,7 +820,10 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 
 	case SCTP_TRANSPORT_PF:
 		transport->state = SCTP_PF;
-		spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
+		if (!asoc->pf_expose)
+			ulp_notify = false;
+		else
+			spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
 		break;
 
 	default:
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 08d14d8..a303011 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
 	/* Enable pf state by default */
 	net->sctp.pf_enable = 1;
 
+	/* Enable pf state exposure by default */
+	net->sctp.pf_expose = 1;
+
 	/* Association.Max.Retrans  - 10 attempts
 	 * Path.Max.Retrans         - 5  attempts (per destination address)
 	 * Max.Init.Retransmits     - 8  attempts
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 939b8d2..8d27434 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5041,6 +5041,7 @@ static int sctp_init_sock(struct sock *sk)
 	sp->hbinterval  = net->sctp.hb_interval;
 	sp->pathmaxrxt  = net->sctp.max_retrans_path;
 	sp->pf_retrans  = net->sctp.pf_retrans;
+	sp->pf_expose   = net->sctp.pf_expose;
 	sp->pathmtu     = 0; /* allow default discovery */
 	sp->sackdelay   = net->sctp.sack_timeout;
 	sp->sackfreq	= 2;
@@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
 
 	transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
 					   pinfo.spinfo_assoc_id);
-	if (!transport)
-		return -EINVAL;
+	if (!transport) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	if (transport->state = SCTP_PF && !transport->asoc->pf_expose) {
+		retval = -EACCES;
+		goto out;
+	}
 
 	pinfo.spinfo_assoc_id = sctp_assoc2id(transport->asoc);
 	pinfo.spinfo_state = transport->state;
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 238cf17..eacc9a1 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "pf_expose",
+		.data		= &init_net.sctp.pf_expose,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
 
 	{ /* sentinel */ }
 };
-- 
2.1.0

[PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

This is a sockopt defined in section 7.3 of rfc7829: "Exposing
the Potentially Failed Path State", by which users can change
pf_expose per sock and asoc.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |  1 +
 net/sctp/socket.c         | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index b6649d6..a15cc28 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -137,6 +137,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_ASCONF_SUPPORTED	128
 #define SCTP_AUTH_SUPPORTED	129
 #define SCTP_ECN_SUPPORTED	130
+#define SCTP_EXPOSE_POTENTIALLY_FAILED_STATE	131
 
 /* PR-SCTP policies */
 #define SCTP_PR_SCTP_NONE	0x0000
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 8d27434..82faf78 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4589,6 +4589,37 @@ static int sctp_setsockopt_ecn_supported(struct sock *sk,
 	return retval;
 }
 
+static int sctp_setsockopt_pf_expose(struct sock *sk,
+				     char __user *optval,
+				     unsigned int optlen)
+{
+	struct sctp_assoc_value params;
+	struct sctp_association *asoc;
+	int retval = -EINVAL;
+
+	if (optlen != sizeof(params))
+		goto out;
+
+	if (copy_from_user(&params, optval, optlen)) {
+		retval = -EFAULT;
+		goto out;
+	}
+
+	asoc = sctp_id2assoc(sk, params.assoc_id);
+	if (!asoc && params.assoc_id != SCTP_FUTURE_ASSOC &&
+	    sctp_style(sk, UDP))
+		goto out;
+
+	if (asoc)
+		asoc->pf_expose = !!params.assoc_value;
+	else
+		sctp_sk(sk)->pf_expose = !!params.assoc_value;
+	retval = 0;
+
+out:
+	return retval;
+}
+
 /* API 6.2 setsockopt(), getsockopt()
  *
  * Applications use setsockopt() and getsockopt() to set or retrieve
@@ -4798,6 +4829,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 	case SCTP_ECN_SUPPORTED:
 		retval = sctp_setsockopt_ecn_supported(sk, optval, optlen);
 		break;
+	case SCTP_EXPOSE_POTENTIALLY_FAILED_STATE:
+		retval = sctp_setsockopt_pf_expose(sk, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
@@ -7908,6 +7942,45 @@ static int sctp_getsockopt_ecn_supported(struct sock *sk, int len,
 	return retval;
 }
 
+static int sctp_getsockopt_pf_expose(struct sock *sk, int len,
+				     char __user *optval,
+				     int __user *optlen)
+{
+	struct sctp_assoc_value params;
+	struct sctp_association *asoc;
+	int retval = -EFAULT;
+
+	if (len < sizeof(params)) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	len = sizeof(params);
+	if (copy_from_user(&params, optval, len))
+		goto out;
+
+	asoc = sctp_id2assoc(sk, params.assoc_id);
+	if (!asoc && params.assoc_id != SCTP_FUTURE_ASSOC &&
+	    sctp_style(sk, UDP)) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	params.assoc_value = asoc ? asoc->pf_expose
+				  : sctp_sk(sk)->pf_expose;
+
+	if (put_user(len, optlen))
+		goto out;
+
+	if (copy_to_user(optval, &params, len))
+		goto out;
+
+	retval = 0;
+
+out:
+	return retval;
+}
+
 static int sctp_getsockopt(struct sock *sk, int level, int optname,
 			   char __user *optval, int __user *optlen)
 {
@@ -8120,6 +8193,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 	case SCTP_ECN_SUPPORTED:
 		retval = sctp_getsockopt_ecn_supported(sk, len, optval, optlen);
 		break;
+	case SCTP_EXPOSE_POTENTIALLY_FAILED_STATE:
+		retval = sctp_getsockopt_pf_expose(sk, len, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
-- 
2.1.0

[PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

This is a sockopt defined in section 7.3 of rfc7829: "Exposing
the Potentially Failed Path State", by which users can change
pf_expose per sock and asoc.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h |  1 +
 net/sctp/socket.c         | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index b6649d6..a15cc28 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -137,6 +137,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_ASCONF_SUPPORTED	128
 #define SCTP_AUTH_SUPPORTED	129
 #define SCTP_ECN_SUPPORTED	130
+#define SCTP_EXPOSE_POTENTIALLY_FAILED_STATE	131
 
 /* PR-SCTP policies */
 #define SCTP_PR_SCTP_NONE	0x0000
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 8d27434..82faf78 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4589,6 +4589,37 @@ static int sctp_setsockopt_ecn_supported(struct sock *sk,
 	return retval;
 }
 
+static int sctp_setsockopt_pf_expose(struct sock *sk,
+				     char __user *optval,
+				     unsigned int optlen)
+{
+	struct sctp_assoc_value params;
+	struct sctp_association *asoc;
+	int retval = -EINVAL;
+
+	if (optlen != sizeof(params))
+		goto out;
+
+	if (copy_from_user(&params, optval, optlen)) {
+		retval = -EFAULT;
+		goto out;
+	}
+
+	asoc = sctp_id2assoc(sk, params.assoc_id);
+	if (!asoc && params.assoc_id != SCTP_FUTURE_ASSOC &&
+	    sctp_style(sk, UDP))
+		goto out;
+
+	if (asoc)
+		asoc->pf_expose = !!params.assoc_value;
+	else
+		sctp_sk(sk)->pf_expose = !!params.assoc_value;
+	retval = 0;
+
+out:
+	return retval;
+}
+
 /* API 6.2 setsockopt(), getsockopt()
  *
  * Applications use setsockopt() and getsockopt() to set or retrieve
@@ -4798,6 +4829,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 	case SCTP_ECN_SUPPORTED:
 		retval = sctp_setsockopt_ecn_supported(sk, optval, optlen);
 		break;
+	case SCTP_EXPOSE_POTENTIALLY_FAILED_STATE:
+		retval = sctp_setsockopt_pf_expose(sk, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
@@ -7908,6 +7942,45 @@ static int sctp_getsockopt_ecn_supported(struct sock *sk, int len,
 	return retval;
 }
 
+static int sctp_getsockopt_pf_expose(struct sock *sk, int len,
+				     char __user *optval,
+				     int __user *optlen)
+{
+	struct sctp_assoc_value params;
+	struct sctp_association *asoc;
+	int retval = -EFAULT;
+
+	if (len < sizeof(params)) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	len = sizeof(params);
+	if (copy_from_user(&params, optval, len))
+		goto out;
+
+	asoc = sctp_id2assoc(sk, params.assoc_id);
+	if (!asoc && params.assoc_id != SCTP_FUTURE_ASSOC &&
+	    sctp_style(sk, UDP)) {
+		retval = -EINVAL;
+		goto out;
+	}
+
+	params.assoc_value = asoc ? asoc->pf_expose
+				  : sctp_sk(sk)->pf_expose;
+
+	if (put_user(len, optlen))
+		goto out;
+
+	if (copy_to_user(optval, &params, len))
+		goto out;
+
+	retval = 0;
+
+out:
+	return retval;
+}
+
 static int sctp_getsockopt(struct sock *sk, int level, int optname,
 			   char __user *optval, int __user *optlen)
 {
@@ -8120,6 +8193,9 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 	case SCTP_ECN_SUPPORTED:
 		retval = sctp_getsockopt_ecn_supported(sk, len, optval, optlen);
 		break;
+	case SCTP_EXPOSE_POTENTIALLY_FAILED_STATE:
+		retval = sctp_getsockopt_pf_expose(sk, len, optval, optlen);
+		break;
 	default:
 		retval = -ENOPROTOOPT;
 		break;
-- 
2.1.0

[PATCHv2 net-next 4/5] sctp: add support for Primary Path Switchover

[Xin Long]

This is a new feature defined in section 5 of rfc7829: "Primary Path
Switchover". By introducing a new tunable parameter:

  Primary.Switchover.Max.Retrans (PSMR)

The primary path will be changed to another active path when the path
error counter on the old primary path exceeds PSMR, so that "the SCTP
sender is allowed to continue data transmission on a new working path
even when the old primary destination address becomes active again".

This patch is to add this tunable parameter, 'ps_retrans' per netns,
sock, asoc and transport. It also allows a user to change ps_retrans
per netns by sysctl, and ps_retrans per sock/asoc/transport will be
initialized with it.

The check will be done in sctp_do_8_2_transport_strike() when this
feature is enabled.

Note this feature is disabled by initializing 'ps_retrans' per netns
as 0xffff by default, and its value can't be less than 'pf_retrans'
when changing by sysctl.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/netns/sctp.h   |  6 ++++++
 include/net/sctp/structs.h | 11 ++++++++---
 net/sctp/associola.c       |  3 +++
 net/sctp/protocol.c        |  3 +++
 net/sctp/sm_sideeffect.c   |  5 +++++
 net/sctp/socket.c          |  1 +
 net/sctp/sysctl.c          |  9 +++++++++
 7 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
index 5234940c..cab0903 100644
--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -89,6 +89,12 @@ struct netns_sctp {
 	 */
 	int pf_retrans;
 
+	/* Primary.Switchover.Max.Retrans sysctl value
+	 * taken from:
+	 * https://tools.ietf.org/html/rfc7829
+	 */
+	int ps_retrans;
+
 	/*
 	 * Disable Potentially-Failed feature, the feature is enabled by default
 	 * pf_enable	-  0  : disable pf
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c2d3317..3680a93 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -184,7 +184,8 @@ struct sctp_sock {
 	__u32 flowlabel;
 	__u8  dscp;
 
-	int pf_retrans;
+	__u16 pf_retrans;
+	__u16 ps_retrans;
 
 	/* The initial Path MTU to use for new associations. */
 	__u32 pathmtu;
@@ -897,7 +898,9 @@ struct sctp_transport {
 	 * and will be initialized from the assocs value.  This can be changed
 	 * using the SCTP_PEER_ADDR_THLDS socket option
 	 */
-	int pf_retrans;
+	__u16 pf_retrans;
+	/* Used for primary path switchover. */
+	__u16 ps_retrans;
 	/* PMTU	      : The current known path MTU.  */
 	__u32 pathmtu;
 
@@ -1773,7 +1776,9 @@ struct sctp_association {
 	 * and will be initialized from the assocs value.  This can be
 	 * changed using the SCTP_PEER_ADDR_THLDS socket option
 	 */
-	int pf_retrans;
+	__u16 pf_retrans;
+	/* Used for primary path switchover. */
+	__u16 ps_retrans;
 
 	/* Maximum number of times the endpoint will retransmit INIT  */
 	__u16 max_init_attempts;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 1c30fda..8aaa7c3 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -86,6 +86,7 @@ static struct sctp_association *sctp_association_init(
 	 */
 	asoc->max_retrans = sp->assocparams.sasoc_asocmaxrxt;
 	asoc->pf_retrans  = sp->pf_retrans;
+	asoc->ps_retrans  = sp->ps_retrans;
 	asoc->pf_expose   = sp->pf_expose;
 
 	asoc->rto_initial = msecs_to_jiffies(sp->rtoinfo.srto_initial);
@@ -625,6 +626,8 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 
 	/* And the partial failure retrans threshold */
 	peer->pf_retrans = asoc->pf_retrans;
+	/* And the primary path switchover retrans threshold */
+	peer->ps_retrans = asoc->ps_retrans;
 
 	/* Initialize the peer's SACK delay timeout based on the
 	 * association configured value.
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index a303011..84a3d75 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1217,6 +1217,9 @@ static int __net_init sctp_defaults_init(struct net *net)
 	/* Max.Burst		    - 4 */
 	net->sctp.max_burst			= SCTP_DEFAULT_MAX_BURST;
 
+	/* Disable of Primary Path Switchover by default */
+	net->sctp.ps_retrans = 0xffff;
+
 	/* Enable pf state by default */
 	net->sctp.pf_enable = 1;
 
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index e52b212..acd737d 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -567,6 +567,11 @@ static void sctp_do_8_2_transport_strike(struct sctp_cmd_seq *commands,
 					     SCTP_FAILED_THRESHOLD);
 	}
 
+	if (transport->error_count > transport->ps_retrans &&
+	    asoc->peer.primary_path == transport &&
+	    asoc->peer.active_path != transport)
+		sctp_assoc_set_primary(asoc, asoc->peer.active_path);
+
 	/* E2) For the destination address for which the timer
 	 * expires, set RTO <- RTO * 2 ("back off the timer").  The
 	 * maximum value discussed in rule C7 above (RTO.max) may be
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 82faf78..7dfb2c5 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5075,6 +5075,7 @@ static int sctp_init_sock(struct sock *sk)
 	sp->hbinterval  = net->sctp.hb_interval;
 	sp->pathmaxrxt  = net->sctp.max_retrans_path;
 	sp->pf_retrans  = net->sctp.pf_retrans;
+	sp->ps_retrans  = net->sctp.ps_retrans;
 	sp->pf_expose   = net->sctp.pf_expose;
 	sp->pathmtu     = 0; /* allow default discovery */
 	sp->sackdelay   = net->sctp.sack_timeout;
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index eacc9a1..c9ebfc2 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -212,6 +212,15 @@ static struct ctl_table sctp_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
+		.extra2		= &init_net.sctp.ps_retrans,
+	},
+	{
+		.procname	= "ps_retrans",
+		.data		= &init_net.sctp.ps_retrans,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &init_net.sctp.pf_retrans,
 		.extra2		= SYSCTL_INT_MAX,
 	},
 	{
-- 
2.1.0

[PATCHv2 net-next 4/5] sctp: add support for Primary Path Switchover

[Xin Long]

This is a new feature defined in section 5 of rfc7829: "Primary Path
Switchover". By introducing a new tunable parameter:

  Primary.Switchover.Max.Retrans (PSMR)

The primary path will be changed to another active path when the path
error counter on the old primary path exceeds PSMR, so that "the SCTP
sender is allowed to continue data transmission on a new working path
even when the old primary destination address becomes active again".

This patch is to add this tunable parameter, 'ps_retrans' per netns,
sock, asoc and transport. It also allows a user to change ps_retrans
per netns by sysctl, and ps_retrans per sock/asoc/transport will be
initialized with it.

The check will be done in sctp_do_8_2_transport_strike() when this
feature is enabled.

Note this feature is disabled by initializing 'ps_retrans' per netns
as 0xffff by default, and its value can't be less than 'pf_retrans'
when changing by sysctl.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/netns/sctp.h   |  6 ++++++
 include/net/sctp/structs.h | 11 ++++++++---
 net/sctp/associola.c       |  3 +++
 net/sctp/protocol.c        |  3 +++
 net/sctp/sm_sideeffect.c   |  5 +++++
 net/sctp/socket.c          |  1 +
 net/sctp/sysctl.c          |  9 +++++++++
 7 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
index 5234940c..cab0903 100644
--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -89,6 +89,12 @@ struct netns_sctp {
 	 */
 	int pf_retrans;
 
+	/* Primary.Switchover.Max.Retrans sysctl value
+	 * taken from:
+	 * https://tools.ietf.org/html/rfc7829
+	 */
+	int ps_retrans;
+
 	/*
 	 * Disable Potentially-Failed feature, the feature is enabled by default
 	 * pf_enable	-  0  : disable pf
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c2d3317..3680a93 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -184,7 +184,8 @@ struct sctp_sock {
 	__u32 flowlabel;
 	__u8  dscp;
 
-	int pf_retrans;
+	__u16 pf_retrans;
+	__u16 ps_retrans;
 
 	/* The initial Path MTU to use for new associations. */
 	__u32 pathmtu;
@@ -897,7 +898,9 @@ struct sctp_transport {
 	 * and will be initialized from the assocs value.  This can be changed
 	 * using the SCTP_PEER_ADDR_THLDS socket option
 	 */
-	int pf_retrans;
+	__u16 pf_retrans;
+	/* Used for primary path switchover. */
+	__u16 ps_retrans;
 	/* PMTU	      : The current known path MTU.  */
 	__u32 pathmtu;
 
@@ -1773,7 +1776,9 @@ struct sctp_association {
 	 * and will be initialized from the assocs value.  This can be
 	 * changed using the SCTP_PEER_ADDR_THLDS socket option
 	 */
-	int pf_retrans;
+	__u16 pf_retrans;
+	/* Used for primary path switchover. */
+	__u16 ps_retrans;
 
 	/* Maximum number of times the endpoint will retransmit INIT  */
 	__u16 max_init_attempts;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 1c30fda..8aaa7c3 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -86,6 +86,7 @@ static struct sctp_association *sctp_association_init(
 	 */
 	asoc->max_retrans = sp->assocparams.sasoc_asocmaxrxt;
 	asoc->pf_retrans  = sp->pf_retrans;
+	asoc->ps_retrans  = sp->ps_retrans;
 	asoc->pf_expose   = sp->pf_expose;
 
 	asoc->rto_initial = msecs_to_jiffies(sp->rtoinfo.srto_initial);
@@ -625,6 +626,8 @@ struct sctp_transport *sctp_assoc_add_peer(struct sctp_association *asoc,
 
 	/* And the partial failure retrans threshold */
 	peer->pf_retrans = asoc->pf_retrans;
+	/* And the primary path switchover retrans threshold */
+	peer->ps_retrans = asoc->ps_retrans;
 
 	/* Initialize the peer's SACK delay timeout based on the
 	 * association configured value.
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index a303011..84a3d75 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1217,6 +1217,9 @@ static int __net_init sctp_defaults_init(struct net *net)
 	/* Max.Burst		    - 4 */
 	net->sctp.max_burst			= SCTP_DEFAULT_MAX_BURST;
 
+	/* Disable of Primary Path Switchover by default */
+	net->sctp.ps_retrans = 0xffff;
+
 	/* Enable pf state by default */
 	net->sctp.pf_enable = 1;
 
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index e52b212..acd737d 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -567,6 +567,11 @@ static void sctp_do_8_2_transport_strike(struct sctp_cmd_seq *commands,
 					     SCTP_FAILED_THRESHOLD);
 	}
 
+	if (transport->error_count > transport->ps_retrans &&
+	    asoc->peer.primary_path = transport &&
+	    asoc->peer.active_path != transport)
+		sctp_assoc_set_primary(asoc, asoc->peer.active_path);
+
 	/* E2) For the destination address for which the timer
 	 * expires, set RTO <- RTO * 2 ("back off the timer").  The
 	 * maximum value discussed in rule C7 above (RTO.max) may be
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 82faf78..7dfb2c5 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5075,6 +5075,7 @@ static int sctp_init_sock(struct sock *sk)
 	sp->hbinterval  = net->sctp.hb_interval;
 	sp->pathmaxrxt  = net->sctp.max_retrans_path;
 	sp->pf_retrans  = net->sctp.pf_retrans;
+	sp->ps_retrans  = net->sctp.ps_retrans;
 	sp->pf_expose   = net->sctp.pf_expose;
 	sp->pathmtu     = 0; /* allow default discovery */
 	sp->sackdelay   = net->sctp.sack_timeout;
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index eacc9a1..c9ebfc2 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -212,6 +212,15 @@ static struct ctl_table sctp_net_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= SYSCTL_ZERO,
+		.extra2		= &init_net.sctp.ps_retrans,
+	},
+	{
+		.procname	= "ps_retrans",
+		.data		= &init_net.sctp.ps_retrans,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &init_net.sctp.pf_retrans,
 		.extra2		= SYSCTL_INT_MAX,
 	},
 	{
-- 
2.1.0

[PATCHv2 net-next 5/5] sctp: add SCTP_PEER_ADDR_THLDS_V2 sockopt

[Xin Long]

Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
added to allow a user to change ps_retrans per sock/asoc/transport, as
other 2 paddrthlds: pf_retrans, pathmaxrxt.

Note: to not break the user's program, here to support pf_retrans dump
and setting by adding a new sockopt SCTP_PEER_ADDR_THLDS_V2, and a new
structure sctp_paddrthlds_v2 instead of extending sctp_paddrthlds.

Also, when setting ps_retrans, the value is not allowed to be greater
than pf_retrans.

v1->v2:
  - use SCTP_PEER_ADDR_THLDS_V2 to set/get pf_retrans instead,
    as Marcelo and David Laight suggested.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h | 10 +++++++++
 net/sctp/socket.c         | 54 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index a15cc28..7974257 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -105,6 +105,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_DEFAULT_SNDINFO	34
 #define SCTP_AUTH_DEACTIVATE_KEY	35
 #define SCTP_REUSE_PORT		36
+#define SCTP_PEER_ADDR_THLDS_V2	37
 
 /* Internal Socket Options. Some of the sctp library functions are
  * implemented using these socket options.
@@ -1071,6 +1072,15 @@ struct sctp_paddrthlds {
 	__u16 spt_pathpfthld;
 };
 
+/* Use a new structure with spt_pathcpthld for back compatibility */
+struct sctp_paddrthlds_v2 {
+	sctp_assoc_t spt_assoc_id;
+	struct sockaddr_storage spt_address;
+	__u16 spt_pathmaxrxt;
+	__u16 spt_pathpfthld;
+	__u16 spt_pathcpthld;
+};
+
 /*
  * Socket Option for Getting the Association/Stream-Specific PR-SCTP Status
  */
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 7dfb2c5..7cd9387 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3943,18 +3943,22 @@ static int sctp_setsockopt_auto_asconf(struct sock *sk, char __user *optval,
  */
 static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
 					    char __user *optval,
-					    unsigned int optlen)
+					    unsigned int optlen, bool v2)
 {
-	struct sctp_paddrthlds val;
+	struct sctp_paddrthlds_v2 val;
 	struct sctp_transport *trans;
 	struct sctp_association *asoc;
+	int len;
 
-	if (optlen < sizeof(struct sctp_paddrthlds))
+	len = v2 ? sizeof(val) : sizeof(struct sctp_paddrthlds);
+	if (optlen < len)
 		return -EINVAL;
-	if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval,
-			   sizeof(struct sctp_paddrthlds)))
+	if (copy_from_user(&val, optval, len))
 		return -EFAULT;
 
+	if (v2 && val.spt_pathpfthld > val.spt_pathcpthld)
+		return -EINVAL;
+
 	if (!sctp_is_any(sk, (const union sctp_addr *)&val.spt_address)) {
 		trans = sctp_addr_id2transport(sk, &val.spt_address,
 					       val.spt_assoc_id);
@@ -3963,6 +3967,8 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
 
 		if (val.spt_pathmaxrxt)
 			trans->pathmaxrxt = val.spt_pathmaxrxt;
+		if (v2)
+			trans->ps_retrans = val.spt_pathcpthld;
 		trans->pf_retrans = val.spt_pathpfthld;
 
 		return 0;
@@ -3978,17 +3984,23 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
 				    transports) {
 			if (val.spt_pathmaxrxt)
 				trans->pathmaxrxt = val.spt_pathmaxrxt;
+			if (v2)
+				trans->ps_retrans = val.spt_pathcpthld;
 			trans->pf_retrans = val.spt_pathpfthld;
 		}
 
 		if (val.spt_pathmaxrxt)
 			asoc->pathmaxrxt = val.spt_pathmaxrxt;
+		if (v2)
+			asoc->ps_retrans = val.spt_pathcpthld;
 		asoc->pf_retrans = val.spt_pathpfthld;
 	} else {
 		struct sctp_sock *sp = sctp_sk(sk);
 
 		if (val.spt_pathmaxrxt)
 			sp->pathmaxrxt = val.spt_pathmaxrxt;
+		if (v2)
+			sp->ps_retrans = val.spt_pathcpthld;
 		sp->pf_retrans = val.spt_pathpfthld;
 	}
 
@@ -4775,7 +4787,12 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_setsockopt_auto_asconf(sk, optval, optlen);
 		break;
 	case SCTP_PEER_ADDR_THLDS:
-		retval = sctp_setsockopt_paddr_thresholds(sk, optval, optlen);
+		retval = sctp_setsockopt_paddr_thresholds(sk, optval, optlen,
+							  false);
+		break;
+	case SCTP_PEER_ADDR_THLDS_V2:
+		retval = sctp_setsockopt_paddr_thresholds(sk, optval, optlen,
+							  true);
 		break;
 	case SCTP_RECVRCVINFO:
 		retval = sctp_setsockopt_recvrcvinfo(sk, optval, optlen);
@@ -7213,18 +7230,19 @@ static int sctp_getsockopt_assoc_ids(struct sock *sk, int len,
  * http://www.ietf.org/id/draft-nishida-tsvwg-sctp-failover-05.txt
  */
 static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
-					    char __user *optval,
-					    int len,
-					    int __user *optlen)
+					    char __user *optval, int len,
+					    int __user *optlen, bool v2)
 {
-	struct sctp_paddrthlds val;
+	struct sctp_paddrthlds_v2 val;
 	struct sctp_transport *trans;
 	struct sctp_association *asoc;
+	int min;
 
-	if (len < sizeof(struct sctp_paddrthlds))
+	min = v2 ? sizeof(val) : sizeof(struct sctp_paddrthlds);
+	if (len < min)
 		return -EINVAL;
-	len = sizeof(struct sctp_paddrthlds);
-	if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval, len))
+	len = min;
+	if (copy_from_user(&val, optval, len))
 		return -EFAULT;
 
 	if (!sctp_is_any(sk, (const union sctp_addr *)&val.spt_address)) {
@@ -7235,6 +7253,7 @@ static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
 
 		val.spt_pathmaxrxt = trans->pathmaxrxt;
 		val.spt_pathpfthld = trans->pf_retrans;
+		val.spt_pathcpthld = trans->ps_retrans;
 
 		goto out;
 	}
@@ -7247,11 +7266,13 @@ static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
 	if (asoc) {
 		val.spt_pathpfthld = asoc->pf_retrans;
 		val.spt_pathmaxrxt = asoc->pathmaxrxt;
+		val.spt_pathcpthld = asoc->ps_retrans;
 	} else {
 		struct sctp_sock *sp = sctp_sk(sk);
 
 		val.spt_pathpfthld = sp->pf_retrans;
 		val.spt_pathmaxrxt = sp->pathmaxrxt;
+		val.spt_pathcpthld = sp->ps_retrans;
 	}
 
 out:
@@ -8131,7 +8152,12 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_getsockopt_auto_asconf(sk, len, optval, optlen);
 		break;
 	case SCTP_PEER_ADDR_THLDS:
-		retval = sctp_getsockopt_paddr_thresholds(sk, optval, len, optlen);
+		retval = sctp_getsockopt_paddr_thresholds(sk, optval, len,
+							  optlen, false);
+		break;
+	case SCTP_PEER_ADDR_THLDS_V2:
+		retval = sctp_getsockopt_paddr_thresholds(sk, optval, len,
+							  optlen, true);
 		break;
 	case SCTP_GET_ASSOC_STATS:
 		retval = sctp_getsockopt_assoc_stats(sk, len, optval, optlen);
-- 
2.1.0

[PATCHv2 net-next 5/5] sctp: add SCTP_PEER_ADDR_THLDS_V2 sockopt

[Xin Long]

Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
added to allow a user to change ps_retrans per sock/asoc/transport, as
other 2 paddrthlds: pf_retrans, pathmaxrxt.

Note: to not break the user's program, here to support pf_retrans dump
and setting by adding a new sockopt SCTP_PEER_ADDR_THLDS_V2, and a new
structure sctp_paddrthlds_v2 instead of extending sctp_paddrthlds.

Also, when setting ps_retrans, the value is not allowed to be greater
than pf_retrans.

v1->v2:
  - use SCTP_PEER_ADDR_THLDS_V2 to set/get pf_retrans instead,
    as Marcelo and David Laight suggested.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/sctp.h | 10 +++++++++
 net/sctp/socket.c         | 54 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index a15cc28..7974257 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -105,6 +105,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_DEFAULT_SNDINFO	34
 #define SCTP_AUTH_DEACTIVATE_KEY	35
 #define SCTP_REUSE_PORT		36
+#define SCTP_PEER_ADDR_THLDS_V2	37
 
 /* Internal Socket Options. Some of the sctp library functions are
  * implemented using these socket options.
@@ -1071,6 +1072,15 @@ struct sctp_paddrthlds {
 	__u16 spt_pathpfthld;
 };
 
+/* Use a new structure with spt_pathcpthld for back compatibility */
+struct sctp_paddrthlds_v2 {
+	sctp_assoc_t spt_assoc_id;
+	struct sockaddr_storage spt_address;
+	__u16 spt_pathmaxrxt;
+	__u16 spt_pathpfthld;
+	__u16 spt_pathcpthld;
+};
+
 /*
  * Socket Option for Getting the Association/Stream-Specific PR-SCTP Status
  */
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 7dfb2c5..7cd9387 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3943,18 +3943,22 @@ static int sctp_setsockopt_auto_asconf(struct sock *sk, char __user *optval,
  */
 static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
 					    char __user *optval,
-					    unsigned int optlen)
+					    unsigned int optlen, bool v2)
 {
-	struct sctp_paddrthlds val;
+	struct sctp_paddrthlds_v2 val;
 	struct sctp_transport *trans;
 	struct sctp_association *asoc;
+	int len;
 
-	if (optlen < sizeof(struct sctp_paddrthlds))
+	len = v2 ? sizeof(val) : sizeof(struct sctp_paddrthlds);
+	if (optlen < len)
 		return -EINVAL;
-	if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval,
-			   sizeof(struct sctp_paddrthlds)))
+	if (copy_from_user(&val, optval, len))
 		return -EFAULT;
 
+	if (v2 && val.spt_pathpfthld > val.spt_pathcpthld)
+		return -EINVAL;
+
 	if (!sctp_is_any(sk, (const union sctp_addr *)&val.spt_address)) {
 		trans = sctp_addr_id2transport(sk, &val.spt_address,
 					       val.spt_assoc_id);
@@ -3963,6 +3967,8 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
 
 		if (val.spt_pathmaxrxt)
 			trans->pathmaxrxt = val.spt_pathmaxrxt;
+		if (v2)
+			trans->ps_retrans = val.spt_pathcpthld;
 		trans->pf_retrans = val.spt_pathpfthld;
 
 		return 0;
@@ -3978,17 +3984,23 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
 				    transports) {
 			if (val.spt_pathmaxrxt)
 				trans->pathmaxrxt = val.spt_pathmaxrxt;
+			if (v2)
+				trans->ps_retrans = val.spt_pathcpthld;
 			trans->pf_retrans = val.spt_pathpfthld;
 		}
 
 		if (val.spt_pathmaxrxt)
 			asoc->pathmaxrxt = val.spt_pathmaxrxt;
+		if (v2)
+			asoc->ps_retrans = val.spt_pathcpthld;
 		asoc->pf_retrans = val.spt_pathpfthld;
 	} else {
 		struct sctp_sock *sp = sctp_sk(sk);
 
 		if (val.spt_pathmaxrxt)
 			sp->pathmaxrxt = val.spt_pathmaxrxt;
+		if (v2)
+			sp->ps_retrans = val.spt_pathcpthld;
 		sp->pf_retrans = val.spt_pathpfthld;
 	}
 
@@ -4775,7 +4787,12 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_setsockopt_auto_asconf(sk, optval, optlen);
 		break;
 	case SCTP_PEER_ADDR_THLDS:
-		retval = sctp_setsockopt_paddr_thresholds(sk, optval, optlen);
+		retval = sctp_setsockopt_paddr_thresholds(sk, optval, optlen,
+							  false);
+		break;
+	case SCTP_PEER_ADDR_THLDS_V2:
+		retval = sctp_setsockopt_paddr_thresholds(sk, optval, optlen,
+							  true);
 		break;
 	case SCTP_RECVRCVINFO:
 		retval = sctp_setsockopt_recvrcvinfo(sk, optval, optlen);
@@ -7213,18 +7230,19 @@ static int sctp_getsockopt_assoc_ids(struct sock *sk, int len,
  * http://www.ietf.org/id/draft-nishida-tsvwg-sctp-failover-05.txt
  */
 static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
-					    char __user *optval,
-					    int len,
-					    int __user *optlen)
+					    char __user *optval, int len,
+					    int __user *optlen, bool v2)
 {
-	struct sctp_paddrthlds val;
+	struct sctp_paddrthlds_v2 val;
 	struct sctp_transport *trans;
 	struct sctp_association *asoc;
+	int min;
 
-	if (len < sizeof(struct sctp_paddrthlds))
+	min = v2 ? sizeof(val) : sizeof(struct sctp_paddrthlds);
+	if (len < min)
 		return -EINVAL;
-	len = sizeof(struct sctp_paddrthlds);
-	if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval, len))
+	len = min;
+	if (copy_from_user(&val, optval, len))
 		return -EFAULT;
 
 	if (!sctp_is_any(sk, (const union sctp_addr *)&val.spt_address)) {
@@ -7235,6 +7253,7 @@ static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
 
 		val.spt_pathmaxrxt = trans->pathmaxrxt;
 		val.spt_pathpfthld = trans->pf_retrans;
+		val.spt_pathcpthld = trans->ps_retrans;
 
 		goto out;
 	}
@@ -7247,11 +7266,13 @@ static int sctp_getsockopt_paddr_thresholds(struct sock *sk,
 	if (asoc) {
 		val.spt_pathpfthld = asoc->pf_retrans;
 		val.spt_pathmaxrxt = asoc->pathmaxrxt;
+		val.spt_pathcpthld = asoc->ps_retrans;
 	} else {
 		struct sctp_sock *sp = sctp_sk(sk);
 
 		val.spt_pathpfthld = sp->pf_retrans;
 		val.spt_pathmaxrxt = sp->pathmaxrxt;
+		val.spt_pathcpthld = sp->ps_retrans;
 	}
 
 out:
@@ -8131,7 +8152,12 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_getsockopt_auto_asconf(sk, len, optval, optlen);
 		break;
 	case SCTP_PEER_ADDR_THLDS:
-		retval = sctp_getsockopt_paddr_thresholds(sk, optval, len, optlen);
+		retval = sctp_getsockopt_paddr_thresholds(sk, optval, len,
+							  optlen, false);
+		break;
+	case SCTP_PEER_ADDR_THLDS_V2:
+		retval = sctp_getsockopt_paddr_thresholds(sk, optval, len,
+							  optlen, true);
 		break;
 	case SCTP_GET_ASSOC_STATS:
 		retval = sctp_getsockopt_assoc_stats(sk, len, optval, optlen);
-- 
2.1.0

RE: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[David Laight]

From: Xin Long
> Sent: 08 October 2019 12:25
> 
> This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> the Potentially Failed Path State", by which users can change
> pf_expose per sock and asoc.

If I read these patches correctly the default for this sockopt in 'enabled'.
Doesn't this mean that old application binaries will receive notifications
that they aren't expecting?

I'd have thought that applications would be required to enable it.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[David Laight]

From: Xin Long
> Sent: 08 October 2019 12:25
> 
> This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> the Potentially Failed Path State", by which users can change
> pf_expose per sock and asoc.

If I read these patches correctly the default for this sockopt in 'enabled'.
Doesn't this mean that old application binaries will receive notifications
that they aren't expecting?

I'd have thought that applications would be required to enable it.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long
> > Sent: 08 October 2019 12:25
> >
> > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > the Potentially Failed Path State", by which users can change
> > pf_expose per sock and asoc.
>
> If I read these patches correctly the default for this sockopt in 'enabled'.
> Doesn't this mean that old application binaries will receive notifications
> that they aren't expecting?
>
> I'd have thought that applications would be required to enable it.
If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long
> > Sent: 08 October 2019 12:25
> >
> > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > the Potentially Failed Path State", by which users can change
> > pf_expose per sock and asoc.
>
> If I read these patches correctly the default for this sockopt in 'enabled'.
> Doesn't this mean that old application binaries will receive notifications
> that they aren't expecting?
>
> I'd have thought that applications would be required to enable it.
If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Xin Long
> > > Sent: 08 October 2019 12:25
> > >
> > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > the Potentially Failed Path State", by which users can change
> > > pf_expose per sock and asoc.
> >
> > If I read these patches correctly the default for this sockopt in 'enabled'.
> > Doesn't this mean that old application binaries will receive notifications
> > that they aren't expecting?
> >
> > I'd have thought that applications would be required to enable it.
> If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> 
I don't think we can safely do either of these things.  Older
applications still need to behave as they did prior to the introduction
of this notification, and we shouldn't allow unexpected notifications to
be sent.

What if you added a check in get_peer_addr_info to only return -EACCESS
if pf_expose is 0 and the application isn't subscribed to the PF event?

Neil

> >
> >         David
> >
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)
> >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Xin Long
> > > Sent: 08 October 2019 12:25
> > >
> > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > the Potentially Failed Path State", by which users can change
> > > pf_expose per sock and asoc.
> >
> > If I read these patches correctly the default for this sockopt in 'enabled'.
> > Doesn't this mean that old application binaries will receive notifications
> > that they aren't expecting?
> >
> > I'd have thought that applications would be required to enable it.
> If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> 
I don't think we can safely do either of these things.  Older
applications still need to behave as they did prior to the introduction
of this notification, and we shouldn't allow unexpected notifications to
be sent.

What if you added a check in get_peer_addr_info to only return -EACCESS
if pf_expose is 0 and the application isn't subscribed to the PF event?

Neil

> >
> >         David
> >
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)
> >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Xin Long
> > > > Sent: 08 October 2019 12:25
> > > >
> > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > the Potentially Failed Path State", by which users can change
> > > > pf_expose per sock and asoc.
> > >
> > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > Doesn't this mean that old application binaries will receive notifications
> > > that they aren't expecting?
> > >
> > > I'd have thought that applications would be required to enable it.
> > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> >
> I don't think we can safely do either of these things.  Older
> applications still need to behave as they did prior to the introduction
> of this notification, and we shouldn't allow unexpected notifications to
> be sent.
>
> What if you added a check in get_peer_addr_info to only return -EACCESS
> if pf_expose is 0 and the application isn't subscribed to the PF event?
We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
events.

Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
are new, we should give 'expose' a default value that would disable both.
How do think if we set 'pf_expose = -1' by default. We send the pf event
only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().

>
> Neil
>
> > >
> > >         David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > >
> >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Xin Long
> > > > Sent: 08 October 2019 12:25
> > > >
> > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > the Potentially Failed Path State", by which users can change
> > > > pf_expose per sock and asoc.
> > >
> > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > Doesn't this mean that old application binaries will receive notifications
> > > that they aren't expecting?
> > >
> > > I'd have thought that applications would be required to enable it.
> > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> >
> I don't think we can safely do either of these things.  Older
> applications still need to behave as they did prior to the introduction
> of this notification, and we shouldn't allow unexpected notifications to
> be sent.
>
> What if you added a check in get_peer_addr_info to only return -EACCESS
> if pf_expose is 0 and the application isn't subscribed to the PF event?
We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
events.

Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
are new, we should give 'expose' a default value that would disable both.
How do think if we set 'pf_expose = -1' by default. We send the pf event
only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().

>
> Neil
>
> > >
> > >         David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > >
> >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > >
> > > > From: Xin Long
> > > > > Sent: 08 October 2019 12:25
> > > > >
> > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > the Potentially Failed Path State", by which users can change
> > > > > pf_expose per sock and asoc.
> > > >
> > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > Doesn't this mean that old application binaries will receive notifications
> > > > that they aren't expecting?
> > > >
> > > > I'd have thought that applications would be required to enable it.
> > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > >
> > I don't think we can safely do either of these things.  Older
> > applications still need to behave as they did prior to the introduction
> > of this notification, and we shouldn't allow unexpected notifications to
> > be sent.
> >
> > What if you added a check in get_peer_addr_info to only return -EACCESS
> > if pf_expose is 0 and the application isn't subscribed to the PF event?
> We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> events.
> 
> Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> are new, we should give 'expose' a default value that would disable both.
> How do think if we set 'pf_expose = -1' by default. We send the pf event
> only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> 
And if pf_expose = 0, we send the event, and return -EACCESS if we call
the socket option and find a PF assoc?  If so, yes, I think that makes
sense.

Neil

> >
> > Neil
> >
> > > >
> > > >         David
> > > >
> > > > -
> > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > Registration No: 1397386 (Wales)
> > > >
> > >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > >
> > > > From: Xin Long
> > > > > Sent: 08 October 2019 12:25
> > > > >
> > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > the Potentially Failed Path State", by which users can change
> > > > > pf_expose per sock and asoc.
> > > >
> > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > Doesn't this mean that old application binaries will receive notifications
> > > > that they aren't expecting?
> > > >
> > > > I'd have thought that applications would be required to enable it.
> > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > >
> > I don't think we can safely do either of these things.  Older
> > applications still need to behave as they did prior to the introduction
> > of this notification, and we shouldn't allow unexpected notifications to
> > be sent.
> >
> > What if you added a check in get_peer_addr_info to only return -EACCESS
> > if pf_expose is 0 and the application isn't subscribed to the PF event?
> We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> events.
> 
> Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> are new, we should give 'expose' a default value that would disable both.
> How do think if we set 'pf_expose = -1' by default. We send the pf event
> only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> 
And if pf_expose = 0, we send the event, and return -EACCESS if we call
the socket option and find a PF assoc?  If so, yes, I think that makes
sense.

Neil

> >
> > Neil
> >
> > > >
> > > >         David
> > > >
> > > > -
> > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > Registration No: 1397386 (Wales)
> > > >
> > >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > >
> > > > > From: Xin Long
> > > > > > Sent: 08 October 2019 12:25
> > > > > >
> > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > the Potentially Failed Path State", by which users can change
> > > > > > pf_expose per sock and asoc.
> > > > >
> > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > that they aren't expecting?
> > > > >
> > > > > I'd have thought that applications would be required to enable it.
> > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > >
> > > I don't think we can safely do either of these things.  Older
> > > applications still need to behave as they did prior to the introduction
> > > of this notification, and we shouldn't allow unexpected notifications to
> > > be sent.
> > >
> > > What if you added a check in get_peer_addr_info to only return -EACCESS
> > > if pf_expose is 0 and the application isn't subscribed to the PF event?
> > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> > events.
> >
> > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> > are new, we should give 'expose' a default value that would disable both.
> > How do think if we set 'pf_expose = -1' by default. We send the pf event
> > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> >
> And if pf_expose = 0, we send the event, and return -EACCESS if we call
> the socket option and find a PF assoc?  If so, yes, I think that makes
> sense.
pf_expose:
-1: compatible with old application (by default)
0: not expose PF to user
1: expose PF to user

So it should be:
if pf_expose == -1:  not send event, not return -EACCESS
if pf_expose == 0: not send event, return -EACCESS
if pf_expose > 0: sent event, not return -EACCESS

makes sense?

>
> Neil
>
> > >
> > > Neil
> > >
> > > > >
> > > > >         David
> > > > >
> > > > > -
> > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > > Registration No: 1397386 (Wales)
> > > > >
> > > >
> >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > >
> > > > > From: Xin Long
> > > > > > Sent: 08 October 2019 12:25
> > > > > >
> > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > the Potentially Failed Path State", by which users can change
> > > > > > pf_expose per sock and asoc.
> > > > >
> > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > that they aren't expecting?
> > > > >
> > > > > I'd have thought that applications would be required to enable it.
> > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > >
> > > I don't think we can safely do either of these things.  Older
> > > applications still need to behave as they did prior to the introduction
> > > of this notification, and we shouldn't allow unexpected notifications to
> > > be sent.
> > >
> > > What if you added a check in get_peer_addr_info to only return -EACCESS
> > > if pf_expose is 0 and the application isn't subscribed to the PF event?
> > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> > events.
> >
> > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> > are new, we should give 'expose' a default value that would disable both.
> > How do think if we set 'pf_expose = -1' by default. We send the pf event
> > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> >
> And if pf_expose = 0, we send the event, and return -EACCESS if we call
> the socket option and find a PF assoc?  If so, yes, I think that makes
> sense.
pf_expose:
-1: compatible with old application (by default)
0: not expose PF to user
1: expose PF to user

So it should be:
if pf_expose = -1:  not send event, not return -EACCESS
if pf_expose = 0: not send event, return -EACCESS
if pf_expose > 0: sent event, not return -EACCESS

makes sense?

>
> Neil
>
> > >
> > > Neil
> > >
> > > > >
> > > > >         David
> > > > >
> > > > > -
> > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > > Registration No: 1397386 (Wales)
> > > > >
> > > >
> >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Fri, Oct 11, 2019 at 11:57 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> > > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > > >
> > > > > > From: Xin Long
> > > > > > > Sent: 08 October 2019 12:25
> > > > > > >
> > > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > > the Potentially Failed Path State", by which users can change
> > > > > > > pf_expose per sock and asoc.
> > > > > >
> > > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > > that they aren't expecting?
> > > > > >
> > > > > > I'd have thought that applications would be required to enable it.
> > > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > > >
> > > > I don't think we can safely do either of these things.  Older
> > > > applications still need to behave as they did prior to the introduction
> > > > of this notification, and we shouldn't allow unexpected notifications to
> > > > be sent.
> > > >
> > > > What if you added a check in get_peer_addr_info to only return -EACCESS
> > > > if pf_expose is 0 and the application isn't subscribed to the PF event?
> > > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> > > events.
> > >
> > > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> > > are new, we should give 'expose' a default value that would disable both.
> > > How do think if we set 'pf_expose = -1' by default. We send the pf event
> > > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> > >
> > And if pf_expose = 0, we send the event, and return -EACCESS if we call
> > the socket option and find a PF assoc?  If so, yes, I think that makes
> > sense.
> pf_expose:
> -1: compatible with old application (by default)
> 0: not expose PF to user
> 1: expose PF to user
>
> So it should be:
> if pf_expose == -1:  not send event, not return -EACCESS
> if pf_expose == 0: not send event, return -EACCESS
> if pf_expose > 0: sent event, not return -EACCESS
>
> makes sense?
Oh, sorry, pf_expose is 1 bit only now in asoc/ep.
Maybe we should use 2 bits, and values could be:
2: compatible with old application (by default)
0: not expose PF to user
1: expose PF to user

>
> >
> > Neil
> >
> > > >
> > > > Neil
> > > >
> > > > > >
> > > > > >         David
> > > > > >
> > > > > > -
> > > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > > > Registration No: 1397386 (Wales)
> > > > > >
> > > > >
> > >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Fri, Oct 11, 2019 at 11:57 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> > > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > > >
> > > > > > From: Xin Long
> > > > > > > Sent: 08 October 2019 12:25
> > > > > > >
> > > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > > the Potentially Failed Path State", by which users can change
> > > > > > > pf_expose per sock and asoc.
> > > > > >
> > > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > > that they aren't expecting?
> > > > > >
> > > > > > I'd have thought that applications would be required to enable it.
> > > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > > >
> > > > I don't think we can safely do either of these things.  Older
> > > > applications still need to behave as they did prior to the introduction
> > > > of this notification, and we shouldn't allow unexpected notifications to
> > > > be sent.
> > > >
> > > > What if you added a check in get_peer_addr_info to only return -EACCESS
> > > > if pf_expose is 0 and the application isn't subscribed to the PF event?
> > > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> > > events.
> > >
> > > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> > > are new, we should give 'expose' a default value that would disable both.
> > > How do think if we set 'pf_expose = -1' by default. We send the pf event
> > > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> > >
> > And if pf_expose = 0, we send the event, and return -EACCESS if we call
> > the socket option and find a PF assoc?  If so, yes, I think that makes
> > sense.
> pf_expose:
> -1: compatible with old application (by default)
> 0: not expose PF to user
> 1: expose PF to user
>
> So it should be:
> if pf_expose = -1:  not send event, not return -EACCESS
> if pf_expose = 0: not send event, return -EACCESS
> if pf_expose > 0: sent event, not return -EACCESS
>
> makes sense?
Oh, sorry, pf_expose is 1 bit only now in asoc/ep.
Maybe we should use 2 bits, and values could be:
2: compatible with old application (by default)
0: not expose PF to user
1: expose PF to user

>
> >
> > Neil
> >
> > > >
> > > > Neil
> > > >
> > > > > >
> > > > > >         David
> > > > > >
> > > > > > -
> > > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > > > Registration No: 1397386 (Wales)
> > > > > >
> > > > >
> > >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Sat, Oct 12, 2019 at 12:25:27AM +0800, Xin Long wrote:
> On Fri, Oct 11, 2019 at 11:57 PM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> > > > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > >
> > > > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > > > >
> > > > > > > From: Xin Long
> > > > > > > > Sent: 08 October 2019 12:25
> > > > > > > >
> > > > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > > > the Potentially Failed Path State", by which users can change
> > > > > > > > pf_expose per sock and asoc.
> > > > > > >
> > > > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > > > that they aren't expecting?
> > > > > > >
> > > > > > > I'd have thought that applications would be required to enable it.
> > > > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > > > >
> > > > > I don't think we can safely do either of these things.  Older
> > > > > applications still need to behave as they did prior to the introduction
> > > > > of this notification, and we shouldn't allow unexpected notifications to
> > > > > be sent.
> > > > >
> > > > > What if you added a check in get_peer_addr_info to only return -EACCESS
> > > > > if pf_expose is 0 and the application isn't subscribed to the PF event?
> > > > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> > > > events.
> > > >
> > > > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> > > > are new, we should give 'expose' a default value that would disable both.
> > > > How do think if we set 'pf_expose = -1' by default. We send the pf event
> > > > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> > > >
> > > And if pf_expose = 0, we send the event, and return -EACCESS if we call
> > > the socket option and find a PF assoc?  If so, yes, I think that makes
> > > sense.
> > pf_expose:
> > -1: compatible with old application (by default)
> > 0: not expose PF to user
> > 1: expose PF to user
> >
> > So it should be:
> > if pf_expose == -1:  not send event, not return -EACCESS
> > if pf_expose == 0: not send event, return -EACCESS
> > if pf_expose > 0: sent event, not return -EACCESS
> >
> > makes sense?
> Oh, sorry, pf_expose is 1 bit only now in asoc/ep.
> Maybe we should use 2 bits, and values could be:
> 2: compatible with old application (by default)
> 0: not expose PF to user
> 1: expose PF to user
> 
Yes, this version makes sense to me
Best
Neil

> >
> > >
> > > Neil
> > >
> > > > >
> > > > > Neil
> > > > >
> > > > > > >
> > > > > > >         David
> > > > > > >
> > > > > > > -
> > > > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > > > > Registration No: 1397386 (Wales)
> > > > > > >
> > > > > >
> > > >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Sat, Oct 12, 2019 at 12:25:27AM +0800, Xin Long wrote:
> On Fri, Oct 11, 2019 at 11:57 PM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > On Thu, Oct 10, 2019 at 8:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Thu, Oct 10, 2019 at 05:28:34PM +0800, Xin Long wrote:
> > > > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > >
> > > > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > > > >
> > > > > > > From: Xin Long
> > > > > > > > Sent: 08 October 2019 12:25
> > > > > > > >
> > > > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > > > the Potentially Failed Path State", by which users can change
> > > > > > > > pf_expose per sock and asoc.
> > > > > > >
> > > > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > > > that they aren't expecting?
> > > > > > >
> > > > > > > I'd have thought that applications would be required to enable it.
> > > > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > > > >
> > > > > I don't think we can safely do either of these things.  Older
> > > > > applications still need to behave as they did prior to the introduction
> > > > > of this notification, and we shouldn't allow unexpected notifications to
> > > > > be sent.
> > > > >
> > > > > What if you added a check in get_peer_addr_info to only return -EACCESS
> > > > > if pf_expose is 0 and the application isn't subscribed to the PF event?
> > > > We can't subscribe to PF event only, but all the SCTP_PEER_ADDR_CHANGE
> > > > events.
> > > >
> > > > Now I'm thinking both PF event and "return -EACCES" in get_peer_addr_info
> > > > are new, we should give 'expose' a default value that would disable both.
> > > > How do think if we set 'pf_expose = -1' by default. We send the pf event
> > > > only if (asoc->pf_expose > 0) in sctp_assoc_control_transport().
> > > >
> > > And if pf_expose = 0, we send the event, and return -EACCESS if we call
> > > the socket option and find a PF assoc?  If so, yes, I think that makes
> > > sense.
> > pf_expose:
> > -1: compatible with old application (by default)
> > 0: not expose PF to user
> > 1: expose PF to user
> >
> > So it should be:
> > if pf_expose = -1:  not send event, not return -EACCESS
> > if pf_expose = 0: not send event, return -EACCESS
> > if pf_expose > 0: sent event, not return -EACCESS
> >
> > makes sense?
> Oh, sorry, pf_expose is 1 bit only now in asoc/ep.
> Maybe we should use 2 bits, and values could be:
> 2: compatible with old application (by default)
> 0: not expose PF to user
> 1: expose PF to user
> 
Yes, this version makes sense to me
Best
Neil

> >
> > >
> > > Neil
> > >
> > > > >
> > > > > Neil
> > > > >
> > > > > > >
> > > > > > >         David
> > > > > > >
> > > > > > > -
> > > > > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > > > > Registration No: 1397386 (Wales)
> > > > > > >
> > > > > >
> > > >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Xin Long
> > > > Sent: 08 October 2019 12:25
> > > >
> > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > the Potentially Failed Path State", by which users can change
> > > > pf_expose per sock and asoc.
> > >
> > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > Doesn't this mean that old application binaries will receive notifications
> > > that they aren't expecting?
> > >
> > > I'd have thought that applications would be required to enable it.
> > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> >
> I don't think we can safely do either of these things.  Older
> applications still need to behave as they did prior to the introduction
> of this notification, and we shouldn't allow unexpected notifications to
> be sent.
Hi, Neil

I think about again, and also talked with QE, we think to get unexpected
notifications shouldn't be a problem for user's applications.

RFC actually keeps adding new notifications, and a user shouldn't expect
the specific notifications coming in some exact orders. They should just
ignore it and wait until the ones they expect. I don't think some users
would abort its application when getting an unexpected notification.

We should NACK patchset v3 and go with v2. What do you think?

>
> What if you added a check in get_peer_addr_info to only return -EACCESS
> if pf_expose is 0 and the application isn't subscribed to the PF event?
>
> Neil
>
> > >
> > >         David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > >
> >

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Xin Long]

On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Xin Long
> > > > Sent: 08 October 2019 12:25
> > > >
> > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > the Potentially Failed Path State", by which users can change
> > > > pf_expose per sock and asoc.
> > >
> > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > Doesn't this mean that old application binaries will receive notifications
> > > that they aren't expecting?
> > >
> > > I'd have thought that applications would be required to enable it.
> > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> >
> I don't think we can safely do either of these things.  Older
> applications still need to behave as they did prior to the introduction
> of this notification, and we shouldn't allow unexpected notifications to
> be sent.
Hi, Neil

I think about again, and also talked with QE, we think to get unexpected
notifications shouldn't be a problem for user's applications.

RFC actually keeps adding new notifications, and a user shouldn't expect
the specific notifications coming in some exact orders. They should just
ignore it and wait until the ones they expect. I don't think some users
would abort its application when getting an unexpected notification.

We should NACK patchset v3 and go with v2. What do you think?

>
> What if you added a check in get_peer_addr_info to only return -EACCESS
> if pf_expose is 0 and the application isn't subscribed to the PF event?
>
> Neil
>
> > >
> > >         David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > >
> >

RE: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[David Laight]

From: Xin Long <lucien.xin@gmail.com>
> Sent: 14 October 2019 09:37
...
> RFC actually keeps adding new notifications,

That RFC keeps moving the goalposts.
Even the structures are guaranteed to have holes.

> and a user shouldn't expect
> the specific notifications coming in some exact orders. They should just
> ignore it and wait until the ones they expect. I don't think some users
> would abort its application when getting an unexpected notification.

I've an example of exactly 1 application.
It uses TCP-style sockets (and will work over TCP).
It does getsockopt(SCTP_EVENTS), sets sctp_association_event, then setsockopt().
Any MSG_NOTIFICATION is assumed to be the a connection reset (enabled above)
and treated as an inwards disconnect.

So any unexpected notification will kill the connection.

I suspect it isn't the only one..

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[David Laight]

RnJvbTogWGluIExvbmcgPGx1Y2llbi54aW5AZ21haWwuY29tPg0KPiBTZW50OiAxNCBPY3RvYmVy
IDIwMTkgMDk6MzcNCi4uLg0KPiBSRkMgYWN0dWFsbHkga2VlcHMgYWRkaW5nIG5ldyBub3RpZmlj
YXRpb25zLA0KDQpUaGF0IFJGQyBrZWVwcyBtb3ZpbmcgdGhlIGdvYWxwb3N0cy4NCkV2ZW4gdGhl
IHN0cnVjdHVyZXMgYXJlIGd1YXJhbnRlZWQgdG8gaGF2ZSBob2xlcy4NCg0KPiBhbmQgYSB1c2Vy
IHNob3VsZG4ndCBleHBlY3QNCj4gdGhlIHNwZWNpZmljIG5vdGlmaWNhdGlvbnMgY29taW5nIGlu
IHNvbWUgZXhhY3Qgb3JkZXJzLiBUaGV5IHNob3VsZCBqdXN0DQo+IGlnbm9yZSBpdCBhbmQgd2Fp
dCB1bnRpbCB0aGUgb25lcyB0aGV5IGV4cGVjdC4gSSBkb24ndCB0aGluayBzb21lIHVzZXJzDQo+
IHdvdWxkIGFib3J0IGl0cyBhcHBsaWNhdGlvbiB3aGVuIGdldHRpbmcgYW4gdW5leHBlY3RlZCBu
b3RpZmljYXRpb24uDQoNCkkndmUgYW4gZXhhbXBsZSBvZiBleGFjdGx5IDEgYXBwbGljYXRpb24u
DQpJdCB1c2VzIFRDUC1zdHlsZSBzb2NrZXRzIChhbmQgd2lsbCB3b3JrIG92ZXIgVENQKS4NCkl0
IGRvZXMgZ2V0c29ja29wdChTQ1RQX0VWRU5UUyksIHNldHMgc2N0cF9hc3NvY2lhdGlvbl9ldmVu
dCwgdGhlbiBzZXRzb2Nrb3B0KCkuDQpBbnkgTVNHX05PVElGSUNBVElPTiBpcyBhc3N1bWVkIHRv
IGJlIHRoZSBhIGNvbm5lY3Rpb24gcmVzZXQgKGVuYWJsZWQgYWJvdmUpDQphbmQgdHJlYXRlZCBh
cyBhbiBpbndhcmRzIGRpc2Nvbm5lY3QuDQoNClNvIGFueSB1bmV4cGVjdGVkIG5vdGlmaWNhdGlv
biB3aWxsIGtpbGwgdGhlIGNvbm5lY3Rpb24uDQoNCkkgc3VzcGVjdCBpdCBpc24ndCB0aGUgb25s
eSBvbmUuLg0KDQoJRGF2aWQNCg0KLQ0KUmVnaXN0ZXJlZCBBZGRyZXNzIExha2VzaWRlLCBCcmFt
bGV5IFJvYWQsIE1vdW50IEZhcm0sIE1pbHRvbiBLZXluZXMsIE1LMSAxUFQsIFVLDQpSZWdpc3Ry
YXRpb24gTm86IDEzOTczODYgKFdhbGVzKQ0K

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Mon, Oct 14, 2019 at 04:36:34PM +0800, Xin Long wrote:
> On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > >
> > > > From: Xin Long
> > > > > Sent: 08 October 2019 12:25
> > > > >
> > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > the Potentially Failed Path State", by which users can change
> > > > > pf_expose per sock and asoc.
> > > >
> > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > Doesn't this mean that old application binaries will receive notifications
> > > > that they aren't expecting?
> > > >
> > > > I'd have thought that applications would be required to enable it.
> > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > >
> > I don't think we can safely do either of these things.  Older
> > applications still need to behave as they did prior to the introduction
> > of this notification, and we shouldn't allow unexpected notifications to
> > be sent.
> Hi, Neil
> 
> I think about again, and also talked with QE, we think to get unexpected
> notifications shouldn't be a problem for user's applications.
> 
On principle, I disagree.  Regardless of what the RFC does, we shouldn't
send notifications that an application aren't subscribed to.  Just
because QE doesn't think it should be a problem (and for their uses it
may well not be an issue), we can't make that general assumption.

> RFC actually keeps adding new notifications, and a user shouldn't expect
> the specific notifications coming in some exact orders. They should just
> ignore it and wait until the ones they expect. I don't think some users
> would abort its application when getting an unexpected notification.
> 
To make that assertion is to discount the purpose of the SCTP_EVENTS
sockopt entirely.  the SCTP_EVENTS option is a whitelist operation, so
they expect to get what they subscribe to, and no more.

> We should NACK patchset v3 and go with v2. What do you think?
> 
No, we need to go with an option that maintains backwards compatibility
without relying on the assumption that applications will just ignore
events they didn't subscribe to.  Davids example is a case in point.

Neil

> >
> > What if you added a check in get_peer_addr_info to only return -EACCESS
> > if pf_expose is 0 and the application isn't subscribed to the PF event?
> >
> > Neil
> >
> > > >
> > > >         David
> > > >
> > > > -
> > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > Registration No: 1397386 (Wales)
> > > >
> > >
> 

Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[Neil Horman]

On Mon, Oct 14, 2019 at 04:36:34PM +0800, Xin Long wrote:
> On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > >
> > > > From: Xin Long
> > > > > Sent: 08 October 2019 12:25
> > > > >
> > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > the Potentially Failed Path State", by which users can change
> > > > > pf_expose per sock and asoc.
> > > >
> > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > Doesn't this mean that old application binaries will receive notifications
> > > > that they aren't expecting?
> > > >
> > > > I'd have thought that applications would be required to enable it.
> > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > >
> > I don't think we can safely do either of these things.  Older
> > applications still need to behave as they did prior to the introduction
> > of this notification, and we shouldn't allow unexpected notifications to
> > be sent.
> Hi, Neil
> 
> I think about again, and also talked with QE, we think to get unexpected
> notifications shouldn't be a problem for user's applications.
> 
On principle, I disagree.  Regardless of what the RFC does, we shouldn't
send notifications that an application aren't subscribed to.  Just
because QE doesn't think it should be a problem (and for their uses it
may well not be an issue), we can't make that general assumption.

> RFC actually keeps adding new notifications, and a user shouldn't expect
> the specific notifications coming in some exact orders. They should just
> ignore it and wait until the ones they expect. I don't think some users
> would abort its application when getting an unexpected notification.
> 
To make that assertion is to discount the purpose of the SCTP_EVENTS
sockopt entirely.  the SCTP_EVENTS option is a whitelist operation, so
they expect to get what they subscribe to, and no more.

> We should NACK patchset v3 and go with v2. What do you think?
> 
No, we need to go with an option that maintains backwards compatibility
without relying on the assumption that applications will just ignore
events they didn't subscribe to.  Davids example is a case in point.

Neil

> >
> > What if you added a check in get_peer_addr_info to only return -EACCESS
> > if pf_expose is 0 and the application isn't subscribed to the PF event?
> >
> > Neil
> >
> > > >
> > > >         David
> > > >
> > > > -
> > > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > > Registration No: 1397386 (Wales)
> > > >
> > >
> 

RE: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt

[David Laight]

From: Neil Horman <nhorman@tuxdriver.com>
> Sent: 14 October 2019 13:42
> To: Xin Long <lucien.xin@gmail.com>
> Cc: David Laight <David.Laight@ACULAB.COM>; network dev <netdev@vger.kernel.org>; linux-sctp@vger.kernel.org; Marcelo
> Ricardo Leitner <marcelo.leitner@gmail.com>; davem@davemloft.net
> Subject: Re: [PATCHv2 net-next 3/5] sctp: add SCTP_EXPOSE_POTENTIALLY_FAILED_STATE sockopt
> 
> On Mon, Oct 14, 2019 at 04:36:34PM +0800, Xin Long wrote:
> > On Thu, Oct 10, 2019 at 12:18 AM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Tue, Oct 08, 2019 at 11:28:32PM +0800, Xin Long wrote:
> > > > On Tue, Oct 8, 2019 at 9:02 PM David Laight <David.Laight@aculab.com> wrote:
> > > > >
> > > > > From: Xin Long
> > > > > > Sent: 08 October 2019 12:25
> > > > > >
> > > > > > This is a sockopt defined in section 7.3 of rfc7829: "Exposing
> > > > > > the Potentially Failed Path State", by which users can change
> > > > > > pf_expose per sock and asoc.
> > > > >
> > > > > If I read these patches correctly the default for this sockopt in 'enabled'.
> > > > > Doesn't this mean that old application binaries will receive notifications
> > > > > that they aren't expecting?
> > > > >
> > > > > I'd have thought that applications would be required to enable it.
> > > > If we do that, sctp_getsockopt_peer_addr_info() in patch 2/5 breaks.
> > > >
> > > I don't think we can safely do either of these things.  Older
> > > applications still need to behave as they did prior to the introduction
> > > of this notification, and we shouldn't allow unexpected notifications to
> > > be sent.
> > Hi, Neil
> >
> > I think about again, and also talked with QE, we think to get unexpected
> > notifications shouldn't be a problem for user's applications.
> >
> On principle, I disagree.  Regardless of what the RFC does, we shouldn't
> send notifications that an application aren't subscribed to.  Just
> because QE doesn't think it should be a problem (and for their uses it
> may well not be an issue), we can't make that general assumption.
> 
> > RFC actually keeps adding new notifications, and a user shouldn't expect
> > the specific notifications coming in some exact orders. They should just
> > ignore it and wait until the ones they expect. I don't think some users
> > would abort its application when getting an unexpected notification.
> >
> To make that assertion is to discount the purpose of the SCTP_EVENTS
> sockopt entirely.  the SCTP_EVENTS option is a whitelist operation, so
> they expect to get what they subscribe to, and no more.
> 
> > We should NACK patchset v3 and go with v2. What do you think?
> >
> No, we need to go with an option that maintains backwards compatibility
> without relying on the assumption that applications will just ignore
> events they didn't subscribe to.  Davids example is a case in point.

Although I don't enable the SCTP_PEER_ADDR_CHANGE indications.
But rfc 6458 doesn't say that the list might be extended.

Aren't there 3 separate items here:
1) The SCTP protocol changes (to better handle primary path failure).
2) The SCTP_GET_PEER_ADDR_INFO sockopt.
3) The MSG_NOTIFICATION indication for SCTP_ADDR_POTENTIALLY_FAILED.

Looking at RFC 7829 section 7.3.
7.3 defines SCTP_EXPOSE_POTENTIALLY_FAILED_STATE.
For compatibility this must default to 'disabled'.
This is even true if the application has set the SCTP_PEER_ADDR_THLDS.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[David Laight]

From: Xin Long
> Sent: 08 October 2019 12:25
> As said in rfc7829, section 3, point 12:
> 
>   The SCTP stack SHOULD expose the PF state of its destination
>   addresses to the ULP as well as provide the means to notify the
>   ULP of state transitions of its destination addresses from
>   active to PF, and vice versa.  However, it is recommended that
>   an SCTP stack implementing SCTP-PF also allows for the ULP to be
>   kept ignorant of the PF state of its destinations and the
>   associated state transitions, thus allowing for retention of the
>   simpler state transition model of [RFC4960] in the ULP.
> 
> Not only does it allow to expose the PF state to ULP, but also
> allow to ignore sctp-pf to ULP.
> 
> So this patch is to add pf_expose per netns, sock and asoc. And in
> sctp_assoc_control_transport(), ulp_notify will be set to false if
> asoc->expose is not set.
> 
> It also allows a user to change pf_expose per netns by sysctl, and
> pf_expose per sock and asoc will be initialized with it.
> 
> Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
> to not allow a user to query the state of a sctp-pf peer address
> when pf_expose is not enabled, as said in section 7.3.
...
> index 08d14d8..a303011 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
>  	/* Enable pf state by default */
>  	net->sctp.pf_enable = 1;
> 
> +	/* Enable pf state exposure by default */
> +	net->sctp.pf_expose = 1;
> +

For compatibility with existing applications pf_expose MUST default to 0.
I'm not even sure it makes sense to have a sysctl for it.

...
> @@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
> 
>  	transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
>  					   pinfo.spinfo_assoc_id);
> -	if (!transport)
> -		return -EINVAL;
> +	if (!transport) {
> +		retval = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (transport->state == SCTP_PF && !transport->asoc->pf_expose) {
> +		retval = -EACCES;
> +		goto out;
> +	}

Ugg...
To avoid reporting the unexpected 'SCTP_PF' state you probable need
to lie about the state (probably reporting 'working' - or whatever state
it would be in if PF detection wasn't enabled.

...
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec,
>  	},
> +	{
> +		.procname	= "pf_expose",
> +		.data		= &init_net.sctp.pf_expose,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec,
> +	},

Setting this will break existing applications.
So I don't think the default should be settable.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[David Laight]

From: Xin Long
> Sent: 08 October 2019 12:25
> As said in rfc7829, section 3, point 12:
> 
>   The SCTP stack SHOULD expose the PF state of its destination
>   addresses to the ULP as well as provide the means to notify the
>   ULP of state transitions of its destination addresses from
>   active to PF, and vice versa.  However, it is recommended that
>   an SCTP stack implementing SCTP-PF also allows for the ULP to be
>   kept ignorant of the PF state of its destinations and the
>   associated state transitions, thus allowing for retention of the
>   simpler state transition model of [RFC4960] in the ULP.
> 
> Not only does it allow to expose the PF state to ULP, but also
> allow to ignore sctp-pf to ULP.
> 
> So this patch is to add pf_expose per netns, sock and asoc. And in
> sctp_assoc_control_transport(), ulp_notify will be set to false if
> asoc->expose is not set.
> 
> It also allows a user to change pf_expose per netns by sysctl, and
> pf_expose per sock and asoc will be initialized with it.
> 
> Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
> to not allow a user to query the state of a sctp-pf peer address
> when pf_expose is not enabled, as said in section 7.3.
...
> index 08d14d8..a303011 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
>  	/* Enable pf state by default */
>  	net->sctp.pf_enable = 1;
> 
> +	/* Enable pf state exposure by default */
> +	net->sctp.pf_expose = 1;
> +

For compatibility with existing applications pf_expose MUST default to 0.
I'm not even sure it makes sense to have a sysctl for it.

...
> @@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
> 
>  	transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
>  					   pinfo.spinfo_assoc_id);
> -	if (!transport)
> -		return -EINVAL;
> +	if (!transport) {
> +		retval = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (transport->state = SCTP_PF && !transport->asoc->pf_expose) {
> +		retval = -EACCES;
> +		goto out;
> +	}

Ugg...
To avoid reporting the unexpected 'SCTP_PF' state you probable need
to lie about the state (probably reporting 'working' - or whatever state
it would be in if PF detection wasn't enabled.

...
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec,
>  	},
> +	{
> +		.procname	= "pf_expose",
> +		.data		= &init_net.sctp.pf_expose,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec,
> +	},

Setting this will break existing applications.
So I don't think the default should be settable.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[Xin Long]

On Fri, Oct 18, 2019 at 11:34 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long
> > Sent: 08 October 2019 12:25
> > As said in rfc7829, section 3, point 12:
> >
> >   The SCTP stack SHOULD expose the PF state of its destination
> >   addresses to the ULP as well as provide the means to notify the
> >   ULP of state transitions of its destination addresses from
> >   active to PF, and vice versa.  However, it is recommended that
> >   an SCTP stack implementing SCTP-PF also allows for the ULP to be
> >   kept ignorant of the PF state of its destinations and the
> >   associated state transitions, thus allowing for retention of the
> >   simpler state transition model of [RFC4960] in the ULP.
> >
> > Not only does it allow to expose the PF state to ULP, but also
> > allow to ignore sctp-pf to ULP.
> >
> > So this patch is to add pf_expose per netns, sock and asoc. And in
> > sctp_assoc_control_transport(), ulp_notify will be set to false if
> > asoc->expose is not set.
> >
> > It also allows a user to change pf_expose per netns by sysctl, and
> > pf_expose per sock and asoc will be initialized with it.
> >
> > Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
> > to not allow a user to query the state of a sctp-pf peer address
> > when pf_expose is not enabled, as said in section 7.3.
> ...
> > index 08d14d8..a303011 100644
> > --- a/net/sctp/protocol.c
> > +++ b/net/sctp/protocol.c
> > @@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
> >       /* Enable pf state by default */
> >       net->sctp.pf_enable = 1;
> >
> > +     /* Enable pf state exposure by default */
> > +     net->sctp.pf_expose = 1;
> > +
>
> For compatibility with existing applications pf_expose MUST default to 0.
> I'm not even sure it makes sense to have a sysctl for it.
You're reivewing v2, pls go and check v3 where it's:

net->sctp.pf_expose = SCTP_PF_EXPOSE_UNUSED

>
> ...
> > @@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
> >
> >       transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
> >                                          pinfo.spinfo_assoc_id);
> > -     if (!transport)
> > -             return -EINVAL;
> > +     if (!transport) {
> > +             retval = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     if (transport->state == SCTP_PF && !transport->asoc->pf_expose) {
> > +             retval = -EACCES;
> > +             goto out;
> > +     }
>
> Ugg...
> To avoid reporting the unexpected 'SCTP_PF' state you probable need
> to lie about the state (probably reporting 'working' - or whatever state
> it would be in if PF detection wasn't enabled.
return EACCES is from RFC. see v3 where it's become:

+       if (transport->state == SCTP_PF &&
+           transport->asoc->pf_expose == SCTP_PF_EXPOSE_DISABLE) {
+               retval = -EACCES;
+               goto out;
+       }

no more compatibility issue.

>
> ...
> > --- a/net/sctp/sysctl.c
> > +++ b/net/sctp/sysctl.c
> > @@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
> >               .mode           = 0644,
> >               .proc_handler   = proc_dointvec,
> >       },
> > +     {
> > +             .procname       = "pf_expose",
> > +             .data           = &init_net.sctp.pf_expose,
> > +             .maxlen         = sizeof(int),
> > +             .mode           = 0644,
> > +             .proc_handler   = proc_dointvec,
> > +     },
>
> Setting this will break existing applications.
> So I don't think the default should be settable.
If the user sets this new sysctl, he must have realized what's going to happen.
I don't think this will cause "compatibility issue".

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

Re: [PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[Xin Long]

On Fri, Oct 18, 2019 at 11:34 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long
> > Sent: 08 October 2019 12:25
> > As said in rfc7829, section 3, point 12:
> >
> >   The SCTP stack SHOULD expose the PF state of its destination
> >   addresses to the ULP as well as provide the means to notify the
> >   ULP of state transitions of its destination addresses from
> >   active to PF, and vice versa.  However, it is recommended that
> >   an SCTP stack implementing SCTP-PF also allows for the ULP to be
> >   kept ignorant of the PF state of its destinations and the
> >   associated state transitions, thus allowing for retention of the
> >   simpler state transition model of [RFC4960] in the ULP.
> >
> > Not only does it allow to expose the PF state to ULP, but also
> > allow to ignore sctp-pf to ULP.
> >
> > So this patch is to add pf_expose per netns, sock and asoc. And in
> > sctp_assoc_control_transport(), ulp_notify will be set to false if
> > asoc->expose is not set.
> >
> > It also allows a user to change pf_expose per netns by sysctl, and
> > pf_expose per sock and asoc will be initialized with it.
> >
> > Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
> > to not allow a user to query the state of a sctp-pf peer address
> > when pf_expose is not enabled, as said in section 7.3.
> ...
> > index 08d14d8..a303011 100644
> > --- a/net/sctp/protocol.c
> > +++ b/net/sctp/protocol.c
> > @@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
> >       /* Enable pf state by default */
> >       net->sctp.pf_enable = 1;
> >
> > +     /* Enable pf state exposure by default */
> > +     net->sctp.pf_expose = 1;
> > +
>
> For compatibility with existing applications pf_expose MUST default to 0.
> I'm not even sure it makes sense to have a sysctl for it.
You're reivewing v2, pls go and check v3 where it's:

net->sctp.pf_expose = SCTP_PF_EXPOSE_UNUSED

>
> ...
> > @@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
> >
> >       transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
> >                                          pinfo.spinfo_assoc_id);
> > -     if (!transport)
> > -             return -EINVAL;
> > +     if (!transport) {
> > +             retval = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     if (transport->state = SCTP_PF && !transport->asoc->pf_expose) {
> > +             retval = -EACCES;
> > +             goto out;
> > +     }
>
> Ugg...
> To avoid reporting the unexpected 'SCTP_PF' state you probable need
> to lie about the state (probably reporting 'working' - or whatever state
> it would be in if PF detection wasn't enabled.
return EACCES is from RFC. see v3 where it's become:

+       if (transport->state = SCTP_PF &&
+           transport->asoc->pf_expose = SCTP_PF_EXPOSE_DISABLE) {
+               retval = -EACCES;
+               goto out;
+       }

no more compatibility issue.

>
> ...
> > --- a/net/sctp/sysctl.c
> > +++ b/net/sctp/sysctl.c
> > @@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
> >               .mode           = 0644,
> >               .proc_handler   = proc_dointvec,
> >       },
> > +     {
> > +             .procname       = "pf_expose",
> > +             .data           = &init_net.sctp.pf_expose,
> > +             .maxlen         = sizeof(int),
> > +             .mode           = 0644,
> > +             .proc_handler   = proc_dointvec,
> > +     },
>
> Setting this will break existing applications.
> So I don't think the default should be settable.
If the user sets this new sysctl, he must have realized what's going to happen.
I don't think this will cause "compatibility issue".

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

RE: [PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[David Laight]

From: Xin Long <lucien.xin@gmail.com>
> Sent: 19 October 2019 09:45
> On Fri, Oct 18, 2019 at 11:34 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Xin Long
> > > Sent: 08 October 2019 12:25
> > > As said in rfc7829, section 3, point 12:
> > >
> > >   The SCTP stack SHOULD expose the PF state of its destination
> > >   addresses to the ULP as well as provide the means to notify the
> > >   ULP of state transitions of its destination addresses from
> > >   active to PF, and vice versa.  However, it is recommended that
> > >   an SCTP stack implementing SCTP-PF also allows for the ULP to be
> > >   kept ignorant of the PF state of its destinations and the
> > >   associated state transitions, thus allowing for retention of the
> > >   simpler state transition model of [RFC4960] in the ULP.
> > >
> > > Not only does it allow to expose the PF state to ULP, but also
> > > allow to ignore sctp-pf to ULP.
> > >
> > > So this patch is to add pf_expose per netns, sock and asoc. And in
> > > sctp_assoc_control_transport(), ulp_notify will be set to false if
> > > asoc->expose is not set.
> > >
> > > It also allows a user to change pf_expose per netns by sysctl, and
> > > pf_expose per sock and asoc will be initialized with it.
> > >
> > > Note that pf_expose also works for SCTP_GET_PEER_ADDR_INFO sockopt,
> > > to not allow a user to query the state of a sctp-pf peer address
> > > when pf_expose is not enabled, as said in section 7.3.
> > ...
> > > index 08d14d8..a303011 100644
> > > --- a/net/sctp/protocol.c
> > > +++ b/net/sctp/protocol.c
> > > @@ -1220,6 +1220,9 @@ static int __net_init sctp_defaults_init(struct net *net)
> > >       /* Enable pf state by default */
> > >       net->sctp.pf_enable = 1;
> > >
> > > +     /* Enable pf state exposure by default */
> > > +     net->sctp.pf_expose = 1;
> > > +
> >
> > For compatibility with existing applications pf_expose MUST default to 0.
> > I'm not even sure it makes sense to have a sysctl for it.
> You're reivewing v2, pls go and check v3 where it's:
> 
> net->sctp.pf_expose = SCTP_PF_EXPOSE_UNUSED

I'll dig out that tri-state logic again later.

> > ...
> > > @@ -5521,8 +5522,15 @@ static int sctp_getsockopt_peer_addr_info(struct sock *sk, int len,
> > >
> > >       transport = sctp_addr_id2transport(sk, &pinfo.spinfo_address,
> > >                                          pinfo.spinfo_assoc_id);
> > > -     if (!transport)
> > > -             return -EINVAL;
> > > +     if (!transport) {
> > > +             retval = -EINVAL;
> > > +             goto out;
> > > +     }
> > > +
> > > +     if (transport->state == SCTP_PF && !transport->asoc->pf_expose) {
> > > +             retval = -EACCES;
> > > +             goto out;
> > > +     }
> >
> > Ugg...
> > To avoid reporting the unexpected 'SCTP_PF' state you probable need
> > to lie about the state (probably reporting 'working' - or whatever state
> > it would be in if PF detection wasn't enabled.
> return EACCES is from RFC. see v3 where it's become:
> 
> +       if (transport->state == SCTP_PF &&
> +           transport->asoc->pf_expose == SCTP_PF_EXPOSE_DISABLE) {
> +               retval = -EACCES;
> +               goto out;
> +       }
> 
> no more compatibility issue.

Hmmm....
Never mind what the RFC says about returning EACCESS, that
is still an API change.

> > > --- a/net/sctp/sysctl.c
> > > +++ b/net/sctp/sysctl.c
> > > @@ -318,6 +318,13 @@ static struct ctl_table sctp_net_table[] = {
> > >               .mode           = 0644,
> > >               .proc_handler   = proc_dointvec,
> > >       },
> > > +     {
> > > +             .procname       = "pf_expose",
> > > +             .data           = &init_net.sctp.pf_expose,
> > > +             .maxlen         = sizeof(int),
> > > +             .mode           = 0644,
> > > +             .proc_handler   = proc_dointvec,
> > > +     },
> >
> > Setting this will break existing applications.
> > So I don't think the default should be settable.
> If the user sets this new sysctl, he must have realized what's going to happen.
> I don't think this will cause "compatibility issue".

The problem is that support is application dependant, not system dependant.
All it takes is a distro to decide to default to enabling it and all old apps break.

Given the application has to enable other things there is no reason not to
require this to be enabled by every application that wants to see the events (etc).

Note that this is different from doing the protocol part of PF - which is likely
to help applications when the 'primary' path is dodgy.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCHv2 net-next 2/5] sctp: add pf_expose per netns and sock and asoc

[David Laight]

DQpGcm9tOiBYaW4gTG9uZyA8bHVjaWVuLnhpbkBnbWFpbC5jb20+DQo+IFNlbnQ6IDE5IE9jdG9i
ZXIgMjAxOSAwOTo0NQ0KPiBPbiBGcmksIE9jdCAxOCwgMjAxOSBhdCAxMTozNCBQTSBEYXZpZCBM
YWlnaHQgPERhdmlkLkxhaWdodEBhY3VsYWIuY29tPiB3cm90ZToNCj4gPg0KPiA+IEZyb206IFhp
biBMb25nDQo+ID4gPiBTZW50OiAwOCBPY3RvYmVyIDIwMTkgMTI6MjUNCj4gPiA+IEFzIHNhaWQg
aW4gcmZjNzgyOSwgc2VjdGlvbiAzLCBwb2ludCAxMjoNCj4gPiA+DQo+ID4gPiAgIFRoZSBTQ1RQ
IHN0YWNrIFNIT1VMRCBleHBvc2UgdGhlIFBGIHN0YXRlIG9mIGl0cyBkZXN0aW5hdGlvbg0KPiA+
ID4gICBhZGRyZXNzZXMgdG8gdGhlIFVMUCBhcyB3ZWxsIGFzIHByb3ZpZGUgdGhlIG1lYW5zIHRv
IG5vdGlmeSB0aGUNCj4gPiA+ICAgVUxQIG9mIHN0YXRlIHRyYW5zaXRpb25zIG9mIGl0cyBkZXN0
aW5hdGlvbiBhZGRyZXNzZXMgZnJvbQ0KPiA+ID4gICBhY3RpdmUgdG8gUEYsIGFuZCB2aWNlIHZl
cnNhLiAgSG93ZXZlciwgaXQgaXMgcmVjb21tZW5kZWQgdGhhdA0KPiA+ID4gICBhbiBTQ1RQIHN0
YWNrIGltcGxlbWVudGluZyBTQ1RQLVBGIGFsc28gYWxsb3dzIGZvciB0aGUgVUxQIHRvIGJlDQo+
ID4gPiAgIGtlcHQgaWdub3JhbnQgb2YgdGhlIFBGIHN0YXRlIG9mIGl0cyBkZXN0aW5hdGlvbnMg
YW5kIHRoZQ0KPiA+ID4gICBhc3NvY2lhdGVkIHN0YXRlIHRyYW5zaXRpb25zLCB0aHVzIGFsbG93
aW5nIGZvciByZXRlbnRpb24gb2YgdGhlDQo+ID4gPiAgIHNpbXBsZXIgc3RhdGUgdHJhbnNpdGlv
biBtb2RlbCBvZiBbUkZDNDk2MF0gaW4gdGhlIFVMUC4NCj4gPiA+DQo+ID4gPiBOb3Qgb25seSBk
b2VzIGl0IGFsbG93IHRvIGV4cG9zZSB0aGUgUEYgc3RhdGUgdG8gVUxQLCBidXQgYWxzbw0KPiA+
ID4gYWxsb3cgdG8gaWdub3JlIHNjdHAtcGYgdG8gVUxQLg0KPiA+ID4NCj4gPiA+IFNvIHRoaXMg
cGF0Y2ggaXMgdG8gYWRkIHBmX2V4cG9zZSBwZXIgbmV0bnMsIHNvY2sgYW5kIGFzb2MuIEFuZCBp
bg0KPiA+ID4gc2N0cF9hc3NvY19jb250cm9sX3RyYW5zcG9ydCgpLCB1bHBfbm90aWZ5IHdpbGwg
YmUgc2V0IHRvIGZhbHNlIGlmDQo+ID4gPiBhc29jLT5leHBvc2UgaXMgbm90IHNldC4NCj4gPiA+
DQo+ID4gPiBJdCBhbHNvIGFsbG93cyBhIHVzZXIgdG8gY2hhbmdlIHBmX2V4cG9zZSBwZXIgbmV0
bnMgYnkgc3lzY3RsLCBhbmQNCj4gPiA+IHBmX2V4cG9zZSBwZXIgc29jayBhbmQgYXNvYyB3aWxs
IGJlIGluaXRpYWxpemVkIHdpdGggaXQuDQo+ID4gPg0KPiA+ID4gTm90ZSB0aGF0IHBmX2V4cG9z
ZSBhbHNvIHdvcmtzIGZvciBTQ1RQX0dFVF9QRUVSX0FERFJfSU5GTyBzb2Nrb3B0LA0KPiA+ID4g
dG8gbm90IGFsbG93IGEgdXNlciB0byBxdWVyeSB0aGUgc3RhdGUgb2YgYSBzY3RwLXBmIHBlZXIg
YWRkcmVzcw0KPiA+ID4gd2hlbiBwZl9leHBvc2UgaXMgbm90IGVuYWJsZWQsIGFzIHNhaWQgaW4g
c2VjdGlvbiA3LjMuDQo+ID4gLi4uDQo+ID4gPiBpbmRleCAwOGQxNGQ4Li5hMzAzMDExIDEwMDY0
NA0KPiA+ID4gLS0tIGEvbmV0L3NjdHAvcHJvdG9jb2wuYw0KPiA+ID4gKysrIGIvbmV0L3NjdHAv
cHJvdG9jb2wuYw0KPiA+ID4gQEAgLTEyMjAsNiArMTIyMCw5IEBAIHN0YXRpYyBpbnQgX19uZXRf
aW5pdCBzY3RwX2RlZmF1bHRzX2luaXQoc3RydWN0IG5ldCAqbmV0KQ0KPiA+ID4gICAgICAgLyog
RW5hYmxlIHBmIHN0YXRlIGJ5IGRlZmF1bHQgKi8NCj4gPiA+ICAgICAgIG5ldC0+c2N0cC5wZl9l
bmFibGUgPSAxOw0KPiA+ID4NCj4gPiA+ICsgICAgIC8qIEVuYWJsZSBwZiBzdGF0ZSBleHBvc3Vy
ZSBieSBkZWZhdWx0ICovDQo+ID4gPiArICAgICBuZXQtPnNjdHAucGZfZXhwb3NlID0gMTsNCj4g
PiA+ICsNCj4gPg0KPiA+IEZvciBjb21wYXRpYmlsaXR5IHdpdGggZXhpc3RpbmcgYXBwbGljYXRp
b25zIHBmX2V4cG9zZSBNVVNUIGRlZmF1bHQgdG8gMC4NCj4gPiBJJ20gbm90IGV2ZW4gc3VyZSBp
dCBtYWtlcyBzZW5zZSB0byBoYXZlIGEgc3lzY3RsIGZvciBpdC4NCj4gWW91J3JlIHJlaXZld2lu
ZyB2MiwgcGxzIGdvIGFuZCBjaGVjayB2MyB3aGVyZSBpdCdzOg0KPiANCj4gbmV0LT5zY3RwLnBm
X2V4cG9zZSA9IFNDVFBfUEZfRVhQT1NFX1VOVVNFRA0KDQpJJ2xsIGRpZyBvdXQgdGhhdCB0cmkt
c3RhdGUgbG9naWMgYWdhaW4gbGF0ZXIuDQoNCj4gPiAuLi4NCj4gPiA+IEBAIC01NTIxLDggKzU1
MjIsMTUgQEAgc3RhdGljIGludCBzY3RwX2dldHNvY2tvcHRfcGVlcl9hZGRyX2luZm8oc3RydWN0
IHNvY2sgKnNrLCBpbnQgbGVuLA0KPiA+ID4NCj4gPiA+ICAgICAgIHRyYW5zcG9ydCA9IHNjdHBf
YWRkcl9pZDJ0cmFuc3BvcnQoc2ssICZwaW5mby5zcGluZm9fYWRkcmVzcywNCj4gPiA+ICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcGluZm8uc3BpbmZvX2Fzc29jX2lk
KTsNCj4gPiA+IC0gICAgIGlmICghdHJhbnNwb3J0KQ0KPiA+ID4gLSAgICAgICAgICAgICByZXR1
cm4gLUVJTlZBTDsNCj4gPiA+ICsgICAgIGlmICghdHJhbnNwb3J0KSB7DQo+ID4gPiArICAgICAg
ICAgICAgIHJldHZhbCA9IC1FSU5WQUw7DQo+ID4gPiArICAgICAgICAgICAgIGdvdG8gb3V0Ow0K
PiA+ID4gKyAgICAgfQ0KPiA+ID4gKw0KPiA+ID4gKyAgICAgaWYgKHRyYW5zcG9ydC0+c3RhdGUg
PT0gU0NUUF9QRiAmJiAhdHJhbnNwb3J0LT5hc29jLT5wZl9leHBvc2UpIHsNCj4gPiA+ICsgICAg
ICAgICAgICAgcmV0dmFsID0gLUVBQ0NFUzsNCj4gPiA+ICsgICAgICAgICAgICAgZ290byBvdXQ7
DQo+ID4gPiArICAgICB9DQo+ID4NCj4gPiBVZ2cuLi4NCj4gPiBUbyBhdm9pZCByZXBvcnRpbmcg
dGhlIHVuZXhwZWN0ZWQgJ1NDVFBfUEYnIHN0YXRlIHlvdSBwcm9iYWJsZSBuZWVkDQo+ID4gdG8g
bGllIGFib3V0IHRoZSBzdGF0ZSAocHJvYmFibHkgcmVwb3J0aW5nICd3b3JraW5nJyAtIG9yIHdo
YXRldmVyIHN0YXRlDQo+ID4gaXQgd291bGQgYmUgaW4gaWYgUEYgZGV0ZWN0aW9uIHdhc24ndCBl
bmFibGVkLg0KPiByZXR1cm4gRUFDQ0VTIGlzIGZyb20gUkZDLiBzZWUgdjMgd2hlcmUgaXQncyBi
ZWNvbWU6DQo+IA0KPiArICAgICAgIGlmICh0cmFuc3BvcnQtPnN0YXRlID09IFNDVFBfUEYgJiYN
Cj4gKyAgICAgICAgICAgdHJhbnNwb3J0LT5hc29jLT5wZl9leHBvc2UgPT0gU0NUUF9QRl9FWFBP
U0VfRElTQUJMRSkgew0KPiArICAgICAgICAgICAgICAgcmV0dmFsID0gLUVBQ0NFUzsNCj4gKyAg
ICAgICAgICAgICAgIGdvdG8gb3V0Ow0KPiArICAgICAgIH0NCj4gDQo+IG5vIG1vcmUgY29tcGF0
aWJpbGl0eSBpc3N1ZS4NCg0KSG1tbS4uLi4NCk5ldmVyIG1pbmQgd2hhdCB0aGUgUkZDIHNheXMg
YWJvdXQgcmV0dXJuaW5nIEVBQ0NFU1MsIHRoYXQNCmlzIHN0aWxsIGFuIEFQSSBjaGFuZ2UuDQoN
Cj4gPiA+IC0tLSBhL25ldC9zY3RwL3N5c2N0bC5jDQo+ID4gPiArKysgYi9uZXQvc2N0cC9zeXNj
dGwuYw0KPiA+ID4gQEAgLTMxOCw2ICszMTgsMTMgQEAgc3RhdGljIHN0cnVjdCBjdGxfdGFibGUg
c2N0cF9uZXRfdGFibGVbXSA9IHsNCj4gPiA+ICAgICAgICAgICAgICAgLm1vZGUgICAgICAgICAg
ID0gMDY0NCwNCj4gPiA+ICAgICAgICAgICAgICAgLnByb2NfaGFuZGxlciAgID0gcHJvY19kb2lu
dHZlYywNCj4gPiA+ICAgICAgIH0sDQo+ID4gPiArICAgICB7DQo+ID4gPiArICAgICAgICAgICAg
IC5wcm9jbmFtZSAgICAgICA9ICJwZl9leHBvc2UiLA0KPiA+ID4gKyAgICAgICAgICAgICAuZGF0
YSAgICAgICAgICAgPSAmaW5pdF9uZXQuc2N0cC5wZl9leHBvc2UsDQo+ID4gPiArICAgICAgICAg
ICAgIC5tYXhsZW4gICAgICAgICA9IHNpemVvZihpbnQpLA0KPiA+ID4gKyAgICAgICAgICAgICAu
bW9kZSAgICAgICAgICAgPSAwNjQ0LA0KPiA+ID4gKyAgICAgICAgICAgICAucHJvY19oYW5kbGVy
ICAgPSBwcm9jX2RvaW50dmVjLA0KPiA+ID4gKyAgICAgfSwNCj4gPg0KPiA+IFNldHRpbmcgdGhp
cyB3aWxsIGJyZWFrIGV4aXN0aW5nIGFwcGxpY2F0aW9ucy4NCj4gPiBTbyBJIGRvbid0IHRoaW5r
IHRoZSBkZWZhdWx0IHNob3VsZCBiZSBzZXR0YWJsZS4NCj4gSWYgdGhlIHVzZXIgc2V0cyB0aGlz
IG5ldyBzeXNjdGwsIGhlIG11c3QgaGF2ZSByZWFsaXplZCB3aGF0J3MgZ29pbmcgdG8gaGFwcGVu
Lg0KPiBJIGRvbid0IHRoaW5rIHRoaXMgd2lsbCBjYXVzZSAiY29tcGF0aWJpbGl0eSBpc3N1ZSIu
DQoNClRoZSBwcm9ibGVtIGlzIHRoYXQgc3VwcG9ydCBpcyBhcHBsaWNhdGlvbiBkZXBlbmRhbnQs
IG5vdCBzeXN0ZW0gZGVwZW5kYW50Lg0KQWxsIGl0IHRha2VzIGlzIGEgZGlzdHJvIHRvIGRlY2lk
ZSB0byBkZWZhdWx0IHRvIGVuYWJsaW5nIGl0IGFuZCBhbGwgb2xkIGFwcHMgYnJlYWsuDQoNCkdp
dmVuIHRoZSBhcHBsaWNhdGlvbiBoYXMgdG8gZW5hYmxlIG90aGVyIHRoaW5ncyB0aGVyZSBpcyBu
byByZWFzb24gbm90IHRvDQpyZXF1aXJlIHRoaXMgdG8gYmUgZW5hYmxlZCBieSBldmVyeSBhcHBs
aWNhdGlvbiB0aGF0IHdhbnRzIHRvIHNlZSB0aGUgZXZlbnRzIChldGMpLg0KDQpOb3RlIHRoYXQg
dGhpcyBpcyBkaWZmZXJlbnQgZnJvbSBkb2luZyB0aGUgcHJvdG9jb2wgcGFydCBvZiBQRiAtIHdo
aWNoIGlzIGxpa2VseQ0KdG8gaGVscCBhcHBsaWNhdGlvbnMgd2hlbiB0aGUgJ3ByaW1hcnknIHBh
dGggaXMgZG9kZ3kuDQoNCglEYXZpZA0KDQotDQpSZWdpc3RlcmVkIEFkZHJlc3MgTGFrZXNpZGUs
IEJyYW1sZXkgUm9hZCwgTW91bnQgRmFybSwgTWlsdG9uIEtleW5lcywgTUsxIDFQVCwgVUsNClJl
Z2lzdHJhdGlvbiBObzogMTM5NzM4NiAoV2FsZXMpDQo