[PATCH net-next 0/5] SCTP updates

[PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

This set improves the SCTP socket API to be more in line with
RFC6458, Geir and myself have finalized it eventually. While
at it, the first patch also fixes two possible information
leaks that should go to net tree as well. For more details, I
refer you to the patches themselves.

Thanks a lot.

Daniel Borkmann (1):
  net: sctp: fix information leaks in ulpevent layer

Geir Ola Vaagland (4):
  net: sctp: implement rfc6458, 5.3.4. SCTP_SNDINFO cmsg support
  net: sctp: implement rfc6458, 5.3.5. SCTP_RCVINFO cmsg support
  net: sctp: implement rfc6458, 5.3.6. SCTP_NXTINFO cmsg support
  net: sctp: implement rfc6458, 8.1.31. SCTP_DEFAULT_SNDINFO support

 include/net/sctp/sctp.h     |   1 +
 include/net/sctp/structs.h  |   7 +-
 include/net/sctp/ulpevent.h |  14 +--
 include/uapi/linux/sctp.h   | 102 ++++++++++++----
 net/sctp/socket.c           | 285 ++++++++++++++++++++++++++++++++++++++------
 net/sctp/ulpevent.c         | 185 ++++++++++++----------------
 6 files changed, 423 insertions(+), 171 deletions(-)

-- 
1.7.11.7

[PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

This set improves the SCTP socket API to be more in line with
RFC6458, Geir and myself have finalized it eventually. While
at it, the first patch also fixes two possible information
leaks that should go to net tree as well. For more details, I
refer you to the patches themselves.

Thanks a lot.

Daniel Borkmann (1):
  net: sctp: fix information leaks in ulpevent layer

Geir Ola Vaagland (4):
  net: sctp: implement rfc6458, 5.3.4. SCTP_SNDINFO cmsg support
  net: sctp: implement rfc6458, 5.3.5. SCTP_RCVINFO cmsg support
  net: sctp: implement rfc6458, 5.3.6. SCTP_NXTINFO cmsg support
  net: sctp: implement rfc6458, 8.1.31. SCTP_DEFAULT_SNDINFO support

 include/net/sctp/sctp.h     |   1 +
 include/net/sctp/structs.h  |   7 +-
 include/net/sctp/ulpevent.h |  14 +--
 include/uapi/linux/sctp.h   | 102 ++++++++++++----
 net/sctp/socket.c           | 285 ++++++++++++++++++++++++++++++++++++++------
 net/sctp/ulpevent.c         | 185 ++++++++++++----------------
 6 files changed, 423 insertions(+), 171 deletions(-)

-- 
1.7.11.7

[PATCH net-next 1/5] net: sctp: fix information leaks in ulpevent layer

[Daniel Borkmann]

While working on some other SCTP code, I noticed that some
structures shared with user space are leaking uninitialized
stack or heap buffer. In particular, struct sctp_sndrcvinfo
has a 2 bytes hole between .sinfo_flags and .sinfo_ppid that
remains unfilled by us in sctp_ulpevent_read_sndrcvinfo() when
putting this into cmsg. But also struct sctp_remote_error
contains a 2 bytes hole that we don't fill but place into a skb
through skb_copy_expand() via sctp_ulpevent_make_remote_error().

Both structures are defined in RFC6458:

* Section 5.3.2. SCTP Header Information Structure:

  The sctp_sndrcvinfo structure is defined below:

  struct sctp_sndrcvinfo {
    uint16_t sinfo_stream;
    uint16_t sinfo_ssn;
    uint16_t sinfo_flags;
    <-- 2 bytes hole  -->
    uint32_t sinfo_ppid;
    uint32_t sinfo_context;
    uint32_t sinfo_timetolive;
    uint32_t sinfo_tsn;
    uint32_t sinfo_cumtsn;
    sctp_assoc_t sinfo_assoc_id;
  };

* 6.1.3. SCTP_REMOTE_ERROR:

  A remote peer may send an Operation Error message to its peer.
  This message indicates a variety of error conditions on an
  association. The entire ERROR chunk as it appears on the wire
  is included in an SCTP_REMOTE_ERROR event. Please refer to the
  SCTP specification [RFC4960] and any extensions for a list of
  possible error formats. An SCTP error notification has the
  following format:

  struct sctp_remote_error {
    uint16_t sre_type;
    uint16_t sre_flags;
    uint32_t sre_length;
    uint16_t sre_error;
    <-- 2 bytes hole  -->
    sctp_assoc_t sre_assoc_id;
    uint8_t  sre_data[];
  };

Fix this by setting both to 0 before filling them out. We also
have other structures shared between user and kernel space in
SCTP that contains holes (e.g. struct sctp_paddrthlds), but we
copy that buffer over from user space first and thus don't need
to care about it in that cases.

While at it, we can also remove lengthy comments copied from
the draft, instead, we update the comment with the correct RFC
number where one can look it up.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/ulpevent.c | 122 +++++++---------------------------------------------
 1 file changed, 15 insertions(+), 107 deletions(-)

diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
index 85c6465..b6842fd 100644
--- a/net/sctp/ulpevent.c
+++ b/net/sctp/ulpevent.c
@@ -366,9 +366,10 @@ fail:
  * specification [SCTP] and any extensions for a list of possible
  * error formats.
  */
-struct sctp_ulpevent *sctp_ulpevent_make_remote_error(
-	const struct sctp_association *asoc, struct sctp_chunk *chunk,
-	__u16 flags, gfp_t gfp)
+struct sctp_ulpevent *
+sctp_ulpevent_make_remote_error(const struct sctp_association *asoc,
+				struct sctp_chunk *chunk, __u16 flags,
+				gfp_t gfp)
 {
 	struct sctp_ulpevent *event;
 	struct sctp_remote_error *sre;
@@ -387,8 +388,7 @@ struct sctp_ulpevent *sctp_ulpevent_make_remote_error(
 	/* Copy the skb to a new skb with room for us to prepend
 	 * notification with.
 	 */
-	skb = skb_copy_expand(chunk->skb, sizeof(struct sctp_remote_error),
-			      0, gfp);
+	skb = skb_copy_expand(chunk->skb, sizeof(*sre), 0, gfp);
 
 	/* Pull off the rest of the cause TLV from the chunk.  */
 	skb_pull(chunk->skb, elen);
@@ -399,62 +399,21 @@ struct sctp_ulpevent *sctp_ulpevent_make_remote_error(
 	event = sctp_skb2event(skb);
 	sctp_ulpevent_init(event, MSG_NOTIFICATION, skb->truesize);
 
-	sre = (struct sctp_remote_error *)
-		skb_push(skb, sizeof(struct sctp_remote_error));
+	sre = (struct sctp_remote_error *) skb_push(skb, sizeof(*sre));
 
 	/* Trim the buffer to the right length.  */
-	skb_trim(skb, sizeof(struct sctp_remote_error) + elen);
+	skb_trim(skb, sizeof(*sre) + elen);
 
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_type:
-	 *   It should be SCTP_REMOTE_ERROR.
-	 */
+	/* RFC6458, Section 6.1.3. SCTP_REMOTE_ERROR */
+	memset(sre, 0, sizeof(*sre));
 	sre->sre_type = SCTP_REMOTE_ERROR;
-
-	/*
-	 * Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_flags: 16 bits (unsigned integer)
-	 *   Currently unused.
-	 */
 	sre->sre_flags = 0;
-
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_length: sizeof (__u32)
-	 *
-	 * This field is the total length of the notification data,
-	 * including the notification header.
-	 */
 	sre->sre_length = skb->len;
-
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_error: 16 bits (unsigned integer)
-	 * This value represents one of the Operational Error causes defined in
-	 * the SCTP specification, in network byte order.
-	 */
 	sre->sre_error = cause;
-
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_assoc_id: sizeof (sctp_assoc_t)
-	 *
-	 * The association id field, holds the identifier for the association.
-	 * All notifications for a given association have the same association
-	 * identifier.  For TCP style socket, this field is ignored.
-	 */
 	sctp_ulpevent_set_owner(event, asoc);
 	sre->sre_assoc_id = sctp_assoc2id(asoc);
 
 	return event;
-
 fail:
 	return NULL;
 }
@@ -899,7 +858,9 @@ __u16 sctp_ulpevent_get_notification_type(const struct sctp_ulpevent *event)
 	return notification->sn_header.sn_type;
 }
 
-/* Copy out the sndrcvinfo into a msghdr.  */
+/* RFC6458, Section 5.3.2. SCTP Header Information Structure
+ * (SCTP_SNDRCV, DEPRECATED)
+ */
 void sctp_ulpevent_read_sndrcvinfo(const struct sctp_ulpevent *event,
 				   struct msghdr *msghdr)
 {
@@ -908,74 +869,21 @@ void sctp_ulpevent_read_sndrcvinfo(const struct sctp_ulpevent *event,
 	if (sctp_ulpevent_is_notification(event))
 		return;
 
-	/* Sockets API Extensions for SCTP
-	 * Section 5.2.2 SCTP Header Information Structure (SCTP_SNDRCV)
-	 *
-	 * sinfo_stream: 16 bits (unsigned integer)
-	 *
-	 * For recvmsg() the SCTP stack places the message's stream number in
-	 * this value.
-	*/
+	memset(&sinfo, 0, sizeof(sinfo));
 	sinfo.sinfo_stream = event->stream;
-	/* sinfo_ssn: 16 bits (unsigned integer)
-	 *
-	 * For recvmsg() this value contains the stream sequence number that
-	 * the remote endpoint placed in the DATA chunk.  For fragmented
-	 * messages this is the same number for all deliveries of the message
-	 * (if more than one recvmsg() is needed to read the message).
-	 */
 	sinfo.sinfo_ssn = event->ssn;
-	/* sinfo_ppid: 32 bits (unsigned integer)
-	 *
-	 * In recvmsg() this value is
-	 * the same information that was passed by the upper layer in the peer
-	 * application.  Please note that byte order issues are NOT accounted
-	 * for and this information is passed opaquely by the SCTP stack from
-	 * one end to the other.
-	 */
 	sinfo.sinfo_ppid = event->ppid;
-	/* sinfo_flags: 16 bits (unsigned integer)
-	 *
-	 * This field may contain any of the following flags and is composed of
-	 * a bitwise OR of these values.
-	 *
-	 * recvmsg() flags:
-	 *
-	 * SCTP_UNORDERED - This flag is present when the message was sent
-	 *                 non-ordered.
-	 */
 	sinfo.sinfo_flags = event->flags;
-	/* sinfo_tsn: 32 bit (unsigned integer)
-	 *
-	 * For the receiving side, this field holds a TSN that was
-	 * assigned to one of the SCTP Data Chunks.
-	 */
 	sinfo.sinfo_tsn = event->tsn;
-	/* sinfo_cumtsn: 32 bit (unsigned integer)
-	 *
-	 * This field will hold the current cumulative TSN as
-	 * known by the underlying SCTP layer.  Note this field is
-	 * ignored when sending and only valid for a receive
-	 * operation when sinfo_flags are set to SCTP_UNORDERED.
-	 */
 	sinfo.sinfo_cumtsn = event->cumtsn;
-	/* sinfo_assoc_id: sizeof (sctp_assoc_t)
-	 *
-	 * The association handle field, sinfo_assoc_id, holds the identifier
-	 * for the association announced in the COMMUNICATION_UP notification.
-	 * All notifications for a given association have the same identifier.
-	 * Ignored for one-to-one style sockets.
-	 */
 	sinfo.sinfo_assoc_id = sctp_assoc2id(event->asoc);
-
-	/* context value that is set via SCTP_CONTEXT socket option. */
+	/* Context value that is set via SCTP_CONTEXT socket option. */
 	sinfo.sinfo_context = event->asoc->default_rcv_context;
-
 	/* These fields are not used while receiving. */
 	sinfo.sinfo_timetolive = 0;
 
 	put_cmsg(msghdr, IPPROTO_SCTP, SCTP_SNDRCV,
-		 sizeof(struct sctp_sndrcvinfo), (void *)&sinfo);
+		 sizeof(sinfo), &sinfo);
 }
 
 /* Do accounting for bytes received and hold a reference to the association
-- 
1.7.11.7

[PATCH net-next 1/5] net: sctp: fix information leaks in ulpevent layer

[Daniel Borkmann]

While working on some other SCTP code, I noticed that some
structures shared with user space are leaking uninitialized
stack or heap buffer. In particular, struct sctp_sndrcvinfo
has a 2 bytes hole between .sinfo_flags and .sinfo_ppid that
remains unfilled by us in sctp_ulpevent_read_sndrcvinfo() when
putting this into cmsg. But also struct sctp_remote_error
contains a 2 bytes hole that we don't fill but place into a skb
through skb_copy_expand() via sctp_ulpevent_make_remote_error().

Both structures are defined in RFC6458:

* Section 5.3.2. SCTP Header Information Structure:

  The sctp_sndrcvinfo structure is defined below:

  struct sctp_sndrcvinfo {
    uint16_t sinfo_stream;
    uint16_t sinfo_ssn;
    uint16_t sinfo_flags;
    <-- 2 bytes hole  -->
    uint32_t sinfo_ppid;
    uint32_t sinfo_context;
    uint32_t sinfo_timetolive;
    uint32_t sinfo_tsn;
    uint32_t sinfo_cumtsn;
    sctp_assoc_t sinfo_assoc_id;
  };

* 6.1.3. SCTP_REMOTE_ERROR:

  A remote peer may send an Operation Error message to its peer.
  This message indicates a variety of error conditions on an
  association. The entire ERROR chunk as it appears on the wire
  is included in an SCTP_REMOTE_ERROR event. Please refer to the
  SCTP specification [RFC4960] and any extensions for a list of
  possible error formats. An SCTP error notification has the
  following format:

  struct sctp_remote_error {
    uint16_t sre_type;
    uint16_t sre_flags;
    uint32_t sre_length;
    uint16_t sre_error;
    <-- 2 bytes hole  -->
    sctp_assoc_t sre_assoc_id;
    uint8_t  sre_data[];
  };

Fix this by setting both to 0 before filling them out. We also
have other structures shared between user and kernel space in
SCTP that contains holes (e.g. struct sctp_paddrthlds), but we
copy that buffer over from user space first and thus don't need
to care about it in that cases.

While at it, we can also remove lengthy comments copied from
the draft, instead, we update the comment with the correct RFC
number where one can look it up.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/sctp/ulpevent.c | 122 +++++++---------------------------------------------
 1 file changed, 15 insertions(+), 107 deletions(-)

diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
index 85c6465..b6842fd 100644
--- a/net/sctp/ulpevent.c
+++ b/net/sctp/ulpevent.c
@@ -366,9 +366,10 @@ fail:
  * specification [SCTP] and any extensions for a list of possible
  * error formats.
  */
-struct sctp_ulpevent *sctp_ulpevent_make_remote_error(
-	const struct sctp_association *asoc, struct sctp_chunk *chunk,
-	__u16 flags, gfp_t gfp)
+struct sctp_ulpevent *
+sctp_ulpevent_make_remote_error(const struct sctp_association *asoc,
+				struct sctp_chunk *chunk, __u16 flags,
+				gfp_t gfp)
 {
 	struct sctp_ulpevent *event;
 	struct sctp_remote_error *sre;
@@ -387,8 +388,7 @@ struct sctp_ulpevent *sctp_ulpevent_make_remote_error(
 	/* Copy the skb to a new skb with room for us to prepend
 	 * notification with.
 	 */
-	skb = skb_copy_expand(chunk->skb, sizeof(struct sctp_remote_error),
-			      0, gfp);
+	skb = skb_copy_expand(chunk->skb, sizeof(*sre), 0, gfp);
 
 	/* Pull off the rest of the cause TLV from the chunk.  */
 	skb_pull(chunk->skb, elen);
@@ -399,62 +399,21 @@ struct sctp_ulpevent *sctp_ulpevent_make_remote_error(
 	event = sctp_skb2event(skb);
 	sctp_ulpevent_init(event, MSG_NOTIFICATION, skb->truesize);
 
-	sre = (struct sctp_remote_error *)
-		skb_push(skb, sizeof(struct sctp_remote_error));
+	sre = (struct sctp_remote_error *) skb_push(skb, sizeof(*sre));
 
 	/* Trim the buffer to the right length.  */
-	skb_trim(skb, sizeof(struct sctp_remote_error) + elen);
+	skb_trim(skb, sizeof(*sre) + elen);
 
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_type:
-	 *   It should be SCTP_REMOTE_ERROR.
-	 */
+	/* RFC6458, Section 6.1.3. SCTP_REMOTE_ERROR */
+	memset(sre, 0, sizeof(*sre));
 	sre->sre_type = SCTP_REMOTE_ERROR;
-
-	/*
-	 * Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_flags: 16 bits (unsigned integer)
-	 *   Currently unused.
-	 */
 	sre->sre_flags = 0;
-
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_length: sizeof (__u32)
-	 *
-	 * This field is the total length of the notification data,
-	 * including the notification header.
-	 */
 	sre->sre_length = skb->len;
-
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_error: 16 bits (unsigned integer)
-	 * This value represents one of the Operational Error causes defined in
-	 * the SCTP specification, in network byte order.
-	 */
 	sre->sre_error = cause;
-
-	/* Socket Extensions for SCTP
-	 * 5.3.1.3 SCTP_REMOTE_ERROR
-	 *
-	 * sre_assoc_id: sizeof (sctp_assoc_t)
-	 *
-	 * The association id field, holds the identifier for the association.
-	 * All notifications for a given association have the same association
-	 * identifier.  For TCP style socket, this field is ignored.
-	 */
 	sctp_ulpevent_set_owner(event, asoc);
 	sre->sre_assoc_id = sctp_assoc2id(asoc);
 
 	return event;
-
 fail:
 	return NULL;
 }
@@ -899,7 +858,9 @@ __u16 sctp_ulpevent_get_notification_type(const struct sctp_ulpevent *event)
 	return notification->sn_header.sn_type;
 }
 
-/* Copy out the sndrcvinfo into a msghdr.  */
+/* RFC6458, Section 5.3.2. SCTP Header Information Structure
+ * (SCTP_SNDRCV, DEPRECATED)
+ */
 void sctp_ulpevent_read_sndrcvinfo(const struct sctp_ulpevent *event,
 				   struct msghdr *msghdr)
 {
@@ -908,74 +869,21 @@ void sctp_ulpevent_read_sndrcvinfo(const struct sctp_ulpevent *event,
 	if (sctp_ulpevent_is_notification(event))
 		return;
 
-	/* Sockets API Extensions for SCTP
-	 * Section 5.2.2 SCTP Header Information Structure (SCTP_SNDRCV)
-	 *
-	 * sinfo_stream: 16 bits (unsigned integer)
-	 *
-	 * For recvmsg() the SCTP stack places the message's stream number in
-	 * this value.
-	*/
+	memset(&sinfo, 0, sizeof(sinfo));
 	sinfo.sinfo_stream = event->stream;
-	/* sinfo_ssn: 16 bits (unsigned integer)
-	 *
-	 * For recvmsg() this value contains the stream sequence number that
-	 * the remote endpoint placed in the DATA chunk.  For fragmented
-	 * messages this is the same number for all deliveries of the message
-	 * (if more than one recvmsg() is needed to read the message).
-	 */
 	sinfo.sinfo_ssn = event->ssn;
-	/* sinfo_ppid: 32 bits (unsigned integer)
-	 *
-	 * In recvmsg() this value is
-	 * the same information that was passed by the upper layer in the peer
-	 * application.  Please note that byte order issues are NOT accounted
-	 * for and this information is passed opaquely by the SCTP stack from
-	 * one end to the other.
-	 */
 	sinfo.sinfo_ppid = event->ppid;
-	/* sinfo_flags: 16 bits (unsigned integer)
-	 *
-	 * This field may contain any of the following flags and is composed of
-	 * a bitwise OR of these values.
-	 *
-	 * recvmsg() flags:
-	 *
-	 * SCTP_UNORDERED - This flag is present when the message was sent
-	 *                 non-ordered.
-	 */
 	sinfo.sinfo_flags = event->flags;
-	/* sinfo_tsn: 32 bit (unsigned integer)
-	 *
-	 * For the receiving side, this field holds a TSN that was
-	 * assigned to one of the SCTP Data Chunks.
-	 */
 	sinfo.sinfo_tsn = event->tsn;
-	/* sinfo_cumtsn: 32 bit (unsigned integer)
-	 *
-	 * This field will hold the current cumulative TSN as
-	 * known by the underlying SCTP layer.  Note this field is
-	 * ignored when sending and only valid for a receive
-	 * operation when sinfo_flags are set to SCTP_UNORDERED.
-	 */
 	sinfo.sinfo_cumtsn = event->cumtsn;
-	/* sinfo_assoc_id: sizeof (sctp_assoc_t)
-	 *
-	 * The association handle field, sinfo_assoc_id, holds the identifier
-	 * for the association announced in the COMMUNICATION_UP notification.
-	 * All notifications for a given association have the same identifier.
-	 * Ignored for one-to-one style sockets.
-	 */
 	sinfo.sinfo_assoc_id = sctp_assoc2id(event->asoc);
-
-	/* context value that is set via SCTP_CONTEXT socket option. */
+	/* Context value that is set via SCTP_CONTEXT socket option. */
 	sinfo.sinfo_context = event->asoc->default_rcv_context;
-
 	/* These fields are not used while receiving. */
 	sinfo.sinfo_timetolive = 0;
 
 	put_cmsg(msghdr, IPPROTO_SCTP, SCTP_SNDRCV,
-		 sizeof(struct sctp_sndrcvinfo), (void *)&sinfo);
+		 sizeof(sinfo), &sinfo);
 }
 
 /* Do accounting for bytes received and hold a reference to the association
-- 
1.7.11.7

[PATCH net-next 5/5] net: sctp: implement rfc6458, 8.1.31. SCTP_DEFAULT_SNDINFO support

[Daniel Borkmann]

From: Geir Ola Vaagland <geirola@gmail.com>

This patch implements section 8.1.31. of RFC6458, which adds support
for setting/retrieving SCTP_DEFAULT_SNDINFO:

  Applications that wish to use the sendto() system call may wish
  to specify a default set of parameters that would normally be
  supplied through the inclusion of ancillary data. This socket
  option allows such an application to set the default sctp_sndinfo
  structure. The application that wishes to use this socket option
  simply passes the sctp_sndinfo structure (defined in Section 5.3.4)
  to this call. The input parameters accepted by this call include
  snd_sid, snd_flags, snd_ppid, and snd_context. The snd_flags
  parameter is composed of a bitwise OR of SCTP_UNORDERED, SCTP_EOF,
  and SCTP_SENDALL. The snd_assoc_id field specifies the association
  to which to apply the parameters. For a one-to-many style socket,
  any of the predefined constants are also allowed in this field.
  The field is ignored for one-to-one style sockets.

Joint work with Daniel Borkmann.

Signed-off-by: Geir Ola Vaagland <geirola@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 include/uapi/linux/sctp.h |   1 +
 net/sctp/socket.c         | 107 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 222f82f..ce70fe6 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -97,6 +97,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_PEER_ADDR_THLDS	31
 #define SCTP_RECVRCVINFO	32
 #define SCTP_RECVNXTINFO	33
+#define SCTP_DEFAULT_SNDINFO	34
 
 /* Internal Socket Options. Some of the sctp library functions are
  * implemented using these socket options.
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9bca87e..d95a50c 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2770,19 +2770,22 @@ static int sctp_setsockopt_default_send_param(struct sock *sk,
 					      char __user *optval,
 					      unsigned int optlen)
 {
-	struct sctp_sndrcvinfo info;
-	struct sctp_association *asoc;
 	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndrcvinfo info;
 
-	if (optlen != sizeof(struct sctp_sndrcvinfo))
+	if (optlen != sizeof(info))
 		return -EINVAL;
 	if (copy_from_user(&info, optval, optlen))
 		return -EFAULT;
+	if (info.sinfo_flags &
+	    ~(SCTP_UNORDERED | SCTP_ADDR_OVER |
+	      SCTP_ABORT | SCTP_EOF))
+		return -EINVAL;
 
 	asoc = sctp_id2assoc(sk, info.sinfo_assoc_id);
 	if (!asoc && info.sinfo_assoc_id && sctp_style(sk, UDP))
 		return -EINVAL;
-
 	if (asoc) {
 		asoc->default_stream = info.sinfo_stream;
 		asoc->default_flags = info.sinfo_flags;
@@ -2800,6 +2803,44 @@ static int sctp_setsockopt_default_send_param(struct sock *sk,
 	return 0;
 }
 
+/* RFC6458, Section 8.1.31. Set/get Default Send Parameters
+ * (SCTP_DEFAULT_SNDINFO)
+ */
+static int sctp_setsockopt_default_sndinfo(struct sock *sk,
+					   char __user *optval,
+					   unsigned int optlen)
+{
+	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndinfo info;
+
+	if (optlen != sizeof(info))
+		return -EINVAL;
+	if (copy_from_user(&info, optval, optlen))
+		return -EFAULT;
+	if (info.snd_flags &
+	    ~(SCTP_UNORDERED | SCTP_ADDR_OVER |
+	      SCTP_ABORT | SCTP_EOF))
+		return -EINVAL;
+
+	asoc = sctp_id2assoc(sk, info.snd_assoc_id);
+	if (!asoc && info.snd_assoc_id && sctp_style(sk, UDP))
+		return -EINVAL;
+	if (asoc) {
+		asoc->default_stream = info.snd_sid;
+		asoc->default_flags = info.snd_flags;
+		asoc->default_ppid = info.snd_ppid;
+		asoc->default_context = info.snd_context;
+	} else {
+		sp->default_stream = info.snd_sid;
+		sp->default_flags = info.snd_flags;
+		sp->default_ppid = info.snd_ppid;
+		sp->default_context = info.snd_context;
+	}
+
+	return 0;
+}
+
 /* 7.1.10 Set Primary Address (SCTP_PRIMARY_ADDR)
  *
  * Requests that the local SCTP stack use the enclosed peer address as
@@ -3725,6 +3766,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_setsockopt_default_send_param(sk, optval,
 							    optlen);
 		break;
+	case SCTP_DEFAULT_SNDINFO:
+		retval = sctp_setsockopt_default_sndinfo(sk, optval, optlen);
+		break;
 	case SCTP_PRIMARY_ADDR:
 		retval = sctp_setsockopt_primary_addr(sk, optval, optlen);
 		break;
@@ -5027,14 +5071,14 @@ static int sctp_getsockopt_default_send_param(struct sock *sk,
 					int len, char __user *optval,
 					int __user *optlen)
 {
-	struct sctp_sndrcvinfo info;
-	struct sctp_association *asoc;
 	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndrcvinfo info;
 
-	if (len < sizeof(struct sctp_sndrcvinfo))
+	if (len < sizeof(info))
 		return -EINVAL;
 
-	len = sizeof(struct sctp_sndrcvinfo);
+	len = sizeof(info);
 
 	if (copy_from_user(&info, optval, len))
 		return -EFAULT;
@@ -5042,7 +5086,6 @@ static int sctp_getsockopt_default_send_param(struct sock *sk,
 	asoc = sctp_id2assoc(sk, info.sinfo_assoc_id);
 	if (!asoc && info.sinfo_assoc_id && sctp_style(sk, UDP))
 		return -EINVAL;
-
 	if (asoc) {
 		info.sinfo_stream = asoc->default_stream;
 		info.sinfo_flags = asoc->default_flags;
@@ -5065,6 +5108,48 @@ static int sctp_getsockopt_default_send_param(struct sock *sk,
 	return 0;
 }
 
+/* RFC6458, Section 8.1.31. Set/get Default Send Parameters
+ * (SCTP_DEFAULT_SNDINFO)
+ */
+static int sctp_getsockopt_default_sndinfo(struct sock *sk, int len,
+					   char __user *optval,
+					   int __user *optlen)
+{
+	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndinfo info;
+
+	if (len < sizeof(info))
+		return -EINVAL;
+
+	len = sizeof(info);
+
+	if (copy_from_user(&info, optval, len))
+		return -EFAULT;
+
+	asoc = sctp_id2assoc(sk, info.snd_assoc_id);
+	if (!asoc && info.snd_assoc_id && sctp_style(sk, UDP))
+		return -EINVAL;
+	if (asoc) {
+		info.snd_sid = asoc->default_stream;
+		info.snd_flags = asoc->default_flags;
+		info.snd_ppid = asoc->default_ppid;
+		info.snd_context = asoc->default_context;
+	} else {
+		info.snd_sid = sp->default_stream;
+		info.snd_flags = sp->default_flags;
+		info.snd_ppid = sp->default_ppid;
+		info.snd_context = sp->default_context;
+	}
+
+	if (put_user(len, optlen))
+		return -EFAULT;
+	if (copy_to_user(optval, &info, len))
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  *
  * 7.1.5 SCTP_NODELAY
@@ -5924,6 +6009,10 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_getsockopt_default_send_param(sk, len,
 							    optval, optlen);
 		break;
+	case SCTP_DEFAULT_SNDINFO:
+		retval = sctp_getsockopt_default_sndinfo(sk, len,
+							 optval, optlen);
+		break;
 	case SCTP_PRIMARY_ADDR:
 		retval = sctp_getsockopt_primary_addr(sk, len, optval, optlen);
 		break;
-- 
1.7.11.7

[PATCH net-next 5/5] net: sctp: implement rfc6458, 8.1.31. SCTP_DEFAULT_SNDINFO support

[Daniel Borkmann]

From: Geir Ola Vaagland <geirola@gmail.com>

This patch implements section 8.1.31. of RFC6458, which adds support
for setting/retrieving SCTP_DEFAULT_SNDINFO:

  Applications that wish to use the sendto() system call may wish
  to specify a default set of parameters that would normally be
  supplied through the inclusion of ancillary data. This socket
  option allows such an application to set the default sctp_sndinfo
  structure. The application that wishes to use this socket option
  simply passes the sctp_sndinfo structure (defined in Section 5.3.4)
  to this call. The input parameters accepted by this call include
  snd_sid, snd_flags, snd_ppid, and snd_context. The snd_flags
  parameter is composed of a bitwise OR of SCTP_UNORDERED, SCTP_EOF,
  and SCTP_SENDALL. The snd_assoc_id field specifies the association
  to which to apply the parameters. For a one-to-many style socket,
  any of the predefined constants are also allowed in this field.
  The field is ignored for one-to-one style sockets.

Joint work with Daniel Borkmann.

Signed-off-by: Geir Ola Vaagland <geirola@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 include/uapi/linux/sctp.h |   1 +
 net/sctp/socket.c         | 107 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 99 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 222f82f..ce70fe6 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -97,6 +97,7 @@ typedef __s32 sctp_assoc_t;
 #define SCTP_PEER_ADDR_THLDS	31
 #define SCTP_RECVRCVINFO	32
 #define SCTP_RECVNXTINFO	33
+#define SCTP_DEFAULT_SNDINFO	34
 
 /* Internal Socket Options. Some of the sctp library functions are
  * implemented using these socket options.
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9bca87e..d95a50c 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2770,19 +2770,22 @@ static int sctp_setsockopt_default_send_param(struct sock *sk,
 					      char __user *optval,
 					      unsigned int optlen)
 {
-	struct sctp_sndrcvinfo info;
-	struct sctp_association *asoc;
 	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndrcvinfo info;
 
-	if (optlen != sizeof(struct sctp_sndrcvinfo))
+	if (optlen != sizeof(info))
 		return -EINVAL;
 	if (copy_from_user(&info, optval, optlen))
 		return -EFAULT;
+	if (info.sinfo_flags &
+	    ~(SCTP_UNORDERED | SCTP_ADDR_OVER |
+	      SCTP_ABORT | SCTP_EOF))
+		return -EINVAL;
 
 	asoc = sctp_id2assoc(sk, info.sinfo_assoc_id);
 	if (!asoc && info.sinfo_assoc_id && sctp_style(sk, UDP))
 		return -EINVAL;
-
 	if (asoc) {
 		asoc->default_stream = info.sinfo_stream;
 		asoc->default_flags = info.sinfo_flags;
@@ -2800,6 +2803,44 @@ static int sctp_setsockopt_default_send_param(struct sock *sk,
 	return 0;
 }
 
+/* RFC6458, Section 8.1.31. Set/get Default Send Parameters
+ * (SCTP_DEFAULT_SNDINFO)
+ */
+static int sctp_setsockopt_default_sndinfo(struct sock *sk,
+					   char __user *optval,
+					   unsigned int optlen)
+{
+	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndinfo info;
+
+	if (optlen != sizeof(info))
+		return -EINVAL;
+	if (copy_from_user(&info, optval, optlen))
+		return -EFAULT;
+	if (info.snd_flags &
+	    ~(SCTP_UNORDERED | SCTP_ADDR_OVER |
+	      SCTP_ABORT | SCTP_EOF))
+		return -EINVAL;
+
+	asoc = sctp_id2assoc(sk, info.snd_assoc_id);
+	if (!asoc && info.snd_assoc_id && sctp_style(sk, UDP))
+		return -EINVAL;
+	if (asoc) {
+		asoc->default_stream = info.snd_sid;
+		asoc->default_flags = info.snd_flags;
+		asoc->default_ppid = info.snd_ppid;
+		asoc->default_context = info.snd_context;
+	} else {
+		sp->default_stream = info.snd_sid;
+		sp->default_flags = info.snd_flags;
+		sp->default_ppid = info.snd_ppid;
+		sp->default_context = info.snd_context;
+	}
+
+	return 0;
+}
+
 /* 7.1.10 Set Primary Address (SCTP_PRIMARY_ADDR)
  *
  * Requests that the local SCTP stack use the enclosed peer address as
@@ -3725,6 +3766,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_setsockopt_default_send_param(sk, optval,
 							    optlen);
 		break;
+	case SCTP_DEFAULT_SNDINFO:
+		retval = sctp_setsockopt_default_sndinfo(sk, optval, optlen);
+		break;
 	case SCTP_PRIMARY_ADDR:
 		retval = sctp_setsockopt_primary_addr(sk, optval, optlen);
 		break;
@@ -5027,14 +5071,14 @@ static int sctp_getsockopt_default_send_param(struct sock *sk,
 					int len, char __user *optval,
 					int __user *optlen)
 {
-	struct sctp_sndrcvinfo info;
-	struct sctp_association *asoc;
 	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndrcvinfo info;
 
-	if (len < sizeof(struct sctp_sndrcvinfo))
+	if (len < sizeof(info))
 		return -EINVAL;
 
-	len = sizeof(struct sctp_sndrcvinfo);
+	len = sizeof(info);
 
 	if (copy_from_user(&info, optval, len))
 		return -EFAULT;
@@ -5042,7 +5086,6 @@ static int sctp_getsockopt_default_send_param(struct sock *sk,
 	asoc = sctp_id2assoc(sk, info.sinfo_assoc_id);
 	if (!asoc && info.sinfo_assoc_id && sctp_style(sk, UDP))
 		return -EINVAL;
-
 	if (asoc) {
 		info.sinfo_stream = asoc->default_stream;
 		info.sinfo_flags = asoc->default_flags;
@@ -5065,6 +5108,48 @@ static int sctp_getsockopt_default_send_param(struct sock *sk,
 	return 0;
 }
 
+/* RFC6458, Section 8.1.31. Set/get Default Send Parameters
+ * (SCTP_DEFAULT_SNDINFO)
+ */
+static int sctp_getsockopt_default_sndinfo(struct sock *sk, int len,
+					   char __user *optval,
+					   int __user *optlen)
+{
+	struct sctp_sock *sp = sctp_sk(sk);
+	struct sctp_association *asoc;
+	struct sctp_sndinfo info;
+
+	if (len < sizeof(info))
+		return -EINVAL;
+
+	len = sizeof(info);
+
+	if (copy_from_user(&info, optval, len))
+		return -EFAULT;
+
+	asoc = sctp_id2assoc(sk, info.snd_assoc_id);
+	if (!asoc && info.snd_assoc_id && sctp_style(sk, UDP))
+		return -EINVAL;
+	if (asoc) {
+		info.snd_sid = asoc->default_stream;
+		info.snd_flags = asoc->default_flags;
+		info.snd_ppid = asoc->default_ppid;
+		info.snd_context = asoc->default_context;
+	} else {
+		info.snd_sid = sp->default_stream;
+		info.snd_flags = sp->default_flags;
+		info.snd_ppid = sp->default_ppid;
+		info.snd_context = sp->default_context;
+	}
+
+	if (put_user(len, optlen))
+		return -EFAULT;
+	if (copy_to_user(optval, &info, len))
+		return -EFAULT;
+
+	return 0;
+}
+
 /*
  *
  * 7.1.5 SCTP_NODELAY
@@ -5924,6 +6009,10 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
 		retval = sctp_getsockopt_default_send_param(sk, len,
 							    optval, optlen);
 		break;
+	case SCTP_DEFAULT_SNDINFO:
+		retval = sctp_getsockopt_default_sndinfo(sk, len,
+							 optval, optlen);
+		break;
 	case SCTP_PRIMARY_ADDR:
 		retval = sctp_getsockopt_primary_addr(sk, len, optval, optlen);
 		break;
-- 
1.7.11.7

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Fri, Jul 04, 2014 at 11:05:03PM +0200, Daniel Borkmann wrote:
> This set improves the SCTP socket API to be more in line with
> RFC6458, Geir and myself have finalized it eventually. While
> at it, the first patch also fixes two possible information
> leaks that should go to net tree as well. For more details, I
> refer you to the patches themselves.
> 
> Thanks a lot.
> 
> Daniel Borkmann (1):
>   net: sctp: fix information leaks in ulpevent layer
> 
> Geir Ola Vaagland (4):
>   net: sctp: implement rfc6458, 5.3.4. SCTP_SNDINFO cmsg support
>   net: sctp: implement rfc6458, 5.3.5. SCTP_RCVINFO cmsg support
>   net: sctp: implement rfc6458, 5.3.6. SCTP_NXTINFO cmsg support
>   net: sctp: implement rfc6458, 8.1.31. SCTP_DEFAULT_SNDINFO support
> 
>  include/net/sctp/sctp.h     |   1 +
>  include/net/sctp/structs.h  |   7 +-
>  include/net/sctp/ulpevent.h |  14 +--
>  include/uapi/linux/sctp.h   | 102 ++++++++++++----
>  net/sctp/socket.c           | 285 ++++++++++++++++++++++++++++++++++++++------
>  net/sctp/ulpevent.c         | 185 ++++++++++++----------------
>  6 files changed, 423 insertions(+), 171 deletions(-)
> 
> -- 
> 1.7.11.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

since you're adding cmsg's from rfc6458, do you also want to add some
deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
schedule its eventual removal?

Neil

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Fri, Jul 04, 2014 at 11:05:03PM +0200, Daniel Borkmann wrote:
> This set improves the SCTP socket API to be more in line with
> RFC6458, Geir and myself have finalized it eventually. While
> at it, the first patch also fixes two possible information
> leaks that should go to net tree as well. For more details, I
> refer you to the patches themselves.
> 
> Thanks a lot.
> 
> Daniel Borkmann (1):
>   net: sctp: fix information leaks in ulpevent layer
> 
> Geir Ola Vaagland (4):
>   net: sctp: implement rfc6458, 5.3.4. SCTP_SNDINFO cmsg support
>   net: sctp: implement rfc6458, 5.3.5. SCTP_RCVINFO cmsg support
>   net: sctp: implement rfc6458, 5.3.6. SCTP_NXTINFO cmsg support
>   net: sctp: implement rfc6458, 8.1.31. SCTP_DEFAULT_SNDINFO support
> 
>  include/net/sctp/sctp.h     |   1 +
>  include/net/sctp/structs.h  |   7 +-
>  include/net/sctp/ulpevent.h |  14 +--
>  include/uapi/linux/sctp.h   | 102 ++++++++++++----
>  net/sctp/socket.c           | 285 ++++++++++++++++++++++++++++++++++++++------
>  net/sctp/ulpevent.c         | 185 ++++++++++++----------------
>  6 files changed, 423 insertions(+), 171 deletions(-)
> 
> -- 
> 1.7.11.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

since you're adding cmsg's from rfc6458, do you also want to add some
deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
schedule its eventual removal?

Neil

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/08/2014 01:14 PM, Neil Horman wrote:
...
> since you're adding cmsg's from rfc6458, do you also want to add some
> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> schedule its eventual removal?

Sure, we can do that. Do you want me to include it into the set?

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/08/2014 01:14 PM, Neil Horman wrote:
...
> since you're adding cmsg's from rfc6458, do you also want to add some
> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> schedule its eventual removal?

Sure, we can do that. Do you want me to include it into the set?

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> On 07/08/2014 01:14 PM, Neil Horman wrote:
> ...
> >since you're adding cmsg's from rfc6458, do you also want to add some
> >deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >schedule its eventual removal?
> 
> Sure, we can do that. Do you want me to include it into the set?
> 

If you're plan is to implement 6458, then yes, I think that would be good.
Neil

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> On 07/08/2014 01:14 PM, Neil Horman wrote:
> ...
> >since you're adding cmsg's from rfc6458, do you also want to add some
> >deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >schedule its eventual removal?
> 
> Sure, we can do that. Do you want me to include it into the set?
> 

If you're plan is to implement 6458, then yes, I think that would be good.
Neil

Re: [PATCH net-next 0/5] SCTP updates

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Fri,  4 Jul 2014 23:05:03 +0200

> This set improves the SCTP socket API to be more in line with
> RFC6458, Geir and myself have finalized it eventually. While
> at it, the first patch also fixes two possible information
> leaks that should go to net tree as well. For more details, I
> refer you to the patches themselves.
> 
> Thanks a lot.

Where are we with this series?  Will there be another respin?

Re: [PATCH net-next 0/5] SCTP updates

[David Miller]

From: Daniel Borkmann <dborkman@redhat.com>
Date: Fri,  4 Jul 2014 23:05:03 +0200

> This set improves the SCTP socket API to be more in line with
> RFC6458, Geir and myself have finalized it eventually. While
> at it, the first patch also fixes two possible information
> leaks that should go to net tree as well. For more details, I
> refer you to the patches themselves.
> 
> Thanks a lot.

Where are we with this series?  Will there be another respin?

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/08/2014 11:40 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Fri,  4 Jul 2014 23:05:03 +0200
>
>> This set improves the SCTP socket API to be more in line with
>> RFC6458, Geir and myself have finalized it eventually. While
>> at it, the first patch also fixes two possible information
>> leaks that should go to net tree as well. For more details, I
>> refer you to the patches themselves.
>>
>> Thanks a lot.
>
> Where are we with this series?  Will there be another respin?

I'll add a 6th patch into the series to address Neil's feedback
and will respin with the rest unchanged. I'll try to get to that
by today. Thanks.

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/08/2014 11:40 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Fri,  4 Jul 2014 23:05:03 +0200
>
>> This set improves the SCTP socket API to be more in line with
>> RFC6458, Geir and myself have finalized it eventually. While
>> at it, the first patch also fixes two possible information
>> leaks that should go to net tree as well. For more details, I
>> refer you to the patches themselves.
>>
>> Thanks a lot.
>
> Where are we with this series?  Will there be another respin?

I'll add a 6th patch into the series to address Neil's feedback
and will respin with the rest unchanged. I'll try to get to that
by today. Thanks.

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/08/2014 04:41 PM, Neil Horman wrote:
> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>> ...
>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>> schedule its eventual removal?
>>
>> Sure, we can do that. Do you want me to include it into the set?
>
> If you're plan is to implement 6458, then yes, I think that would be good.

Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
only, which is fine. What you're suggesting is to place similar ratelimited
warnings (due to different possible pids on the machine) into the 'fastpath'
where we get and set cmsg message headers.

While that may be fine for {set,get}sockopt(2) that's called once or very few
times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
to unnecessary spamming the klog since up to now this is the only way our
users can set or receive this info. I'm not sure we want to annoy users like
that ...

In how many years do you plan a removal ... I think we're stuck with uapi
basically forever as we don't want to break old binaries, no? ;/

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/08/2014 04:41 PM, Neil Horman wrote:
> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>> ...
>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>> schedule its eventual removal?
>>
>> Sure, we can do that. Do you want me to include it into the set?
>
> If you're plan is to implement 6458, then yes, I think that would be good.

Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
only, which is fine. What you're suggesting is to place similar ratelimited
warnings (due to different possible pids on the machine) into the 'fastpath'
where we get and set cmsg message headers.

While that may be fine for {set,get}sockopt(2) that's called once or very few
times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
to unnecessary spamming the klog since up to now this is the only way our
users can set or receive this info. I'm not sure we want to annoy users like
that ...

In how many years do you plan a removal ... I think we're stuck with uapi
basically forever as we don't want to break old binaries, no? ;/

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> On 07/08/2014 04:41 PM, Neil Horman wrote:
> >On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>...
> >>>since you're adding cmsg's from rfc6458, do you also want to add some
> >>>deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>schedule its eventual removal?
> >>
> >>Sure, we can do that. Do you want me to include it into the set?
> >
> >If you're plan is to implement 6458, then yes, I think that would be good.
> 
> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> only, which is fine. What you're suggesting is to place similar ratelimited
> warnings (due to different possible pids on the machine) into the 'fastpath'
> where we get and set cmsg message headers.
> 
> While that may be fine for {set,get}sockopt(2) that's called once or very few
> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> to unnecessary spamming the klog since up to now this is the only way our
> users can set or receive this info. I'm not sure we want to annoy users like
> that ...
> 
Then we wrap it in a ONCE macro so that it only triggers on the first use
instance.

> In how many years do you plan a removal ... I think we're stuck with uapi
> basically forever as we don't want to break old binaries, no? ;/
> 
I thought we could remove things on a schedule if we followed the deprecation
process, but that may just be for sysfs.  Regardless, it would still be nice to
inform people they are using an older api.

Neil

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> On 07/08/2014 04:41 PM, Neil Horman wrote:
> >On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>...
> >>>since you're adding cmsg's from rfc6458, do you also want to add some
> >>>deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>schedule its eventual removal?
> >>
> >>Sure, we can do that. Do you want me to include it into the set?
> >
> >If you're plan is to implement 6458, then yes, I think that would be good.
> 
> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> only, which is fine. What you're suggesting is to place similar ratelimited
> warnings (due to different possible pids on the machine) into the 'fastpath'
> where we get and set cmsg message headers.
> 
> While that may be fine for {set,get}sockopt(2) that's called once or very few
> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> to unnecessary spamming the klog since up to now this is the only way our
> users can set or receive this info. I'm not sure we want to annoy users like
> that ...
> 
Then we wrap it in a ONCE macro so that it only triggers on the first use
instance.

> In how many years do you plan a removal ... I think we're stuck with uapi
> basically forever as we don't want to break old binaries, no? ;/
> 
I thought we could remove things on a schedule if we followed the deprecation
process, but that may just be for sysfs.  Regardless, it would still be nice to
inform people they are using an older api.

Neil

RE: [PATCH net-next 0/5] SCTP updates

[David Laight]

From: Neil Horman
> > In how many years do you plan a removal ... I think we're stuck with uapi
> > basically forever as we don't want to break old binaries, no? ;/
> >
> I thought we could remove things on a schedule if we followed the deprecation
> process, but that may just be for sysfs.  Regardless, it would still be nice to
> inform people they are using an older api.

The issue here is that it is an application API, not a system programming one.
I'd guess that most sysfs stuff is only used by programs that are released
as part of a Linux distribution - and thus code that is likely to be
release with a matching kernel.

These sccp structures are much more likely to be used by 'proper' 3rd party
applications, targeted at multiple kernel versions - possibly in binary form.
We compile applications on the oldest linux version we think our customers
are using - otherwise we'd have to release multiple copies.

So I don't think they can be deprecated (with warnings from the kernel) for
a long time, perhaps 5 or 10 years!

The first stage would be (somehow) to generate warnings when applications
are compiled. Only when that has been in place for some time would it
make sense to generate kernel warnings.

		David

RE: [PATCH net-next 0/5] SCTP updates

[David Laight]

From: Neil Horman
> > In how many years do you plan a removal ... I think we're stuck with uapi
> > basically forever as we don't want to break old binaries, no? ;/
> >
> I thought we could remove things on a schedule if we followed the deprecation
> process, but that may just be for sysfs.  Regardless, it would still be nice to
> inform people they are using an older api.

The issue here is that it is an application API, not a system programming one.
I'd guess that most sysfs stuff is only used by programs that are released
as part of a Linux distribution - and thus code that is likely to be
release with a matching kernel.

These sccp structures are much more likely to be used by 'proper' 3rd party
applications, targeted at multiple kernel versions - possibly in binary form.
We compile applications on the oldest linux version we think our customers
are using - otherwise we'd have to release multiple copies.

So I don't think they can be deprecated (with warnings from the kernel) for
a long time, perhaps 5 or 10 years!

The first stage would be (somehow) to generate warnings when applications
are compiled. Only when that has been in place for some time would it
make sense to generate kernel warnings.

		David

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/09/2014 12:49 PM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>> ...
>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>> schedule its eventual removal?
>>>>
>>>> Sure, we can do that. Do you want me to include it into the set?
>>>
>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>
>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>> only, which is fine. What you're suggesting is to place similar ratelimited
>> warnings (due to different possible pids on the machine) into the 'fastpath'
>> where we get and set cmsg message headers.
>>
>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>> to unnecessary spamming the klog since up to now this is the only way our
>> users can set or receive this info. I'm not sure we want to annoy users like
>> that ...
>>
> Then we wrap it in a ONCE macro so that it only triggers on the first use
> instance.

I'm not convinced about this so far. The whole point is that we also provide the
pid just as we currently do, so that we give the user a chance to possibly pin
point the process that needs code change to not use the deprecated API anymore.

>> In how many years do you plan a removal ... I think we're stuck with uapi
>> basically forever as we don't want to break old binaries, no? ;/
>>
> I thought we could remove things on a schedule if we followed the deprecation
> process, but that may just be for sysfs.  Regardless, it would still be nice to
> inform people they are using an older api.

I think we rather might be stuck with also the deprecated stuff forever, just
as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
remove anything, there's actually also no point to spam the log about it, if
everyone can/should read it from the RFC anyway.

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/09/2014 12:49 PM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>> ...
>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>> schedule its eventual removal?
>>>>
>>>> Sure, we can do that. Do you want me to include it into the set?
>>>
>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>
>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>> only, which is fine. What you're suggesting is to place similar ratelimited
>> warnings (due to different possible pids on the machine) into the 'fastpath'
>> where we get and set cmsg message headers.
>>
>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>> to unnecessary spamming the klog since up to now this is the only way our
>> users can set or receive this info. I'm not sure we want to annoy users like
>> that ...
>>
> Then we wrap it in a ONCE macro so that it only triggers on the first use
> instance.

I'm not convinced about this so far. The whole point is that we also provide the
pid just as we currently do, so that we give the user a chance to possibly pin
point the process that needs code change to not use the deprecated API anymore.

>> In how many years do you plan a removal ... I think we're stuck with uapi
>> basically forever as we don't want to break old binaries, no? ;/
>>
> I thought we could remove things on a schedule if we followed the deprecation
> process, but that may just be for sysfs.  Regardless, it would still be nice to
> inform people they are using an older api.

I think we rather might be stuck with also the deprecated stuff forever, just
as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
remove anything, there's actually also no point to spam the log about it, if
everyone can/should read it from the RFC anyway.

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> On 07/09/2014 12:49 PM, Neil Horman wrote:
> >On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>...
> >>>>>since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>schedule its eventual removal?
> >>>>
> >>>>Sure, we can do that. Do you want me to include it into the set?
> >>>
> >>>If you're plan is to implement 6458, then yes, I think that would be good.
> >>
> >>Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>only, which is fine. What you're suggesting is to place similar ratelimited
> >>warnings (due to different possible pids on the machine) into the 'fastpath'
> >>where we get and set cmsg message headers.
> >>
> >>While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>to unnecessary spamming the klog since up to now this is the only way our
> >>users can set or receive this info. I'm not sure we want to annoy users like
> >>that ...
> >>
> >Then we wrap it in a ONCE macro so that it only triggers on the first use
> >instance.
> 
> I'm not convinced about this so far. The whole point is that we also provide the
> pid just as we currently do, so that we give the user a chance to possibly pin
> point the process that needs code change to not use the deprecated API anymore.
> 
> >>In how many years do you plan a removal ... I think we're stuck with uapi
> >>basically forever as we don't want to break old binaries, no? ;/
> >>
> >I thought we could remove things on a schedule if we followed the deprecation
> >process, but that may just be for sysfs.  Regardless, it would still be nice to
> >inform people they are using an older api.
> 
> I think we rather might be stuck with also the deprecated stuff forever, just
> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> remove anything, there's actually also no point to spam the log about it, if
> everyone can/should read it from the RFC anyway.
> 
So this begs the question as to why we have deprecation warnings to begin with.
At what point do we draw the line where we can change some aspect of the user
api.  I agree if the answer is never, then yeah we're stuck, but then, why
bother announcing deprecation warnings at all?

Neil

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> On 07/09/2014 12:49 PM, Neil Horman wrote:
> >On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>...
> >>>>>since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>schedule its eventual removal?
> >>>>
> >>>>Sure, we can do that. Do you want me to include it into the set?
> >>>
> >>>If you're plan is to implement 6458, then yes, I think that would be good.
> >>
> >>Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>only, which is fine. What you're suggesting is to place similar ratelimited
> >>warnings (due to different possible pids on the machine) into the 'fastpath'
> >>where we get and set cmsg message headers.
> >>
> >>While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>to unnecessary spamming the klog since up to now this is the only way our
> >>users can set or receive this info. I'm not sure we want to annoy users like
> >>that ...
> >>
> >Then we wrap it in a ONCE macro so that it only triggers on the first use
> >instance.
> 
> I'm not convinced about this so far. The whole point is that we also provide the
> pid just as we currently do, so that we give the user a chance to possibly pin
> point the process that needs code change to not use the deprecated API anymore.
> 
> >>In how many years do you plan a removal ... I think we're stuck with uapi
> >>basically forever as we don't want to break old binaries, no? ;/
> >>
> >I thought we could remove things on a schedule if we followed the deprecation
> >process, but that may just be for sysfs.  Regardless, it would still be nice to
> >inform people they are using an older api.
> 
> I think we rather might be stuck with also the deprecated stuff forever, just
> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> remove anything, there's actually also no point to spam the log about it, if
> everyone can/should read it from the RFC anyway.
> 
So this begs the question as to why we have deprecation warnings to begin with.
At what point do we draw the line where we can change some aspect of the user
api.  I agree if the answer is never, then yeah we're stuck, but then, why
bother announcing deprecation warnings at all?

Neil

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/09/2014 05:13 PM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
>> On 07/09/2014 12:49 PM, Neil Horman wrote:
>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>>>> ...
>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>>>> schedule its eventual removal?
>>>>>>
>>>>>> Sure, we can do that. Do you want me to include it into the set?
>>>>>
>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>>>
>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>>>> only, which is fine. What you're suggesting is to place similar ratelimited
>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
>>>> where we get and set cmsg message headers.
>>>>
>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>>>> to unnecessary spamming the klog since up to now this is the only way our
>>>> users can set or receive this info. I'm not sure we want to annoy users like
>>>> that ...
>>>>
>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
>>> instance.
>>
>> I'm not convinced about this so far. The whole point is that we also provide the
>> pid just as we currently do, so that we give the user a chance to possibly pin
>> point the process that needs code change to not use the deprecated API anymore.
>>
>>>> In how many years do you plan a removal ... I think we're stuck with uapi
>>>> basically forever as we don't want to break old binaries, no? ;/
>>>>
>>> I thought we could remove things on a schedule if we followed the deprecation
>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
>>> inform people they are using an older api.
>>
>> I think we rather might be stuck with also the deprecated stuff forever, just
>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
>> remove anything, there's actually also no point to spam the log about it, if
>> everyone can/should read it from the RFC anyway.
>>
> So this begs the question as to why we have deprecation warnings to begin with.
> At what point do we draw the line where we can change some aspect of the user
> api.  I agree if the answer is never, then yeah we're stuck, but then, why
> bother announcing deprecation warnings at all?

That's a good question, not sure when this was decided, Vlad? ;)

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/09/2014 05:13 PM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
>> On 07/09/2014 12:49 PM, Neil Horman wrote:
>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>>>> ...
>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>>>> schedule its eventual removal?
>>>>>>
>>>>>> Sure, we can do that. Do you want me to include it into the set?
>>>>>
>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>>>
>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>>>> only, which is fine. What you're suggesting is to place similar ratelimited
>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
>>>> where we get and set cmsg message headers.
>>>>
>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>>>> to unnecessary spamming the klog since up to now this is the only way our
>>>> users can set or receive this info. I'm not sure we want to annoy users like
>>>> that ...
>>>>
>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
>>> instance.
>>
>> I'm not convinced about this so far. The whole point is that we also provide the
>> pid just as we currently do, so that we give the user a chance to possibly pin
>> point the process that needs code change to not use the deprecated API anymore.
>>
>>>> In how many years do you plan a removal ... I think we're stuck with uapi
>>>> basically forever as we don't want to break old binaries, no? ;/
>>>>
>>> I thought we could remove things on a schedule if we followed the deprecation
>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
>>> inform people they are using an older api.
>>
>> I think we rather might be stuck with also the deprecated stuff forever, just
>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
>> remove anything, there's actually also no point to spam the log about it, if
>> everyone can/should read it from the RFC anyway.
>>
> So this begs the question as to why we have deprecation warnings to begin with.
> At what point do we draw the line where we can change some aspect of the user
> api.  I agree if the answer is never, then yeah we're stuck, but then, why
> bother announcing deprecation warnings at all?

That's a good question, not sure when this was decided, Vlad? ;)

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/09/2014 11:13 AM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
>> On 07/09/2014 12:49 PM, Neil Horman wrote:
>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>>>> ...
>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>>>> schedule its eventual removal?
>>>>>>
>>>>>> Sure, we can do that. Do you want me to include it into the set?
>>>>>
>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>>>
>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>>>> only, which is fine. What you're suggesting is to place similar ratelimited
>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
>>>> where we get and set cmsg message headers.
>>>>
>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>>>> to unnecessary spamming the klog since up to now this is the only way our
>>>> users can set or receive this info. I'm not sure we want to annoy users like
>>>> that ...
>>>>
>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
>>> instance.
>>
>> I'm not convinced about this so far. The whole point is that we also provide the
>> pid just as we currently do, so that we give the user a chance to possibly pin
>> point the process that needs code change to not use the deprecated API anymore.
>>
>>>> In how many years do you plan a removal ... I think we're stuck with uapi
>>>> basically forever as we don't want to break old binaries, no? ;/
>>>>
>>> I thought we could remove things on a schedule if we followed the deprecation
>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
>>> inform people they are using an older api.
>>
>> I think we rather might be stuck with also the deprecated stuff forever, just
>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
>> remove anything, there's actually also no point to spam the log about it, if
>> everyone can/should read it from the RFC anyway.
>>
> So this begs the question as to why we have deprecation warnings to begin with.
> At what point do we draw the line where we can change some aspect of the user
> api.  I agree if the answer is never, then yeah we're stuck, but then, why
> bother announcing deprecation warnings at all?

I think we can deprecate user API after a significantly long period of time
(like 5 or 6 years).  That's why we also have a deprecation schedule that can
be updated and hopefully that's something people pay attention to.

The problem here is deprecation of ancillary data and that's is a lot tougher
then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
spec.  Ancillary data should not have been enabled using even notification api,
as it is not an event, but we now have to live with it.

-vlad

> 
> Neil
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/09/2014 11:13 AM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
>> On 07/09/2014 12:49 PM, Neil Horman wrote:
>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>>>> ...
>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>>>> schedule its eventual removal?
>>>>>>
>>>>>> Sure, we can do that. Do you want me to include it into the set?
>>>>>
>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>>>
>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>>>> only, which is fine. What you're suggesting is to place similar ratelimited
>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
>>>> where we get and set cmsg message headers.
>>>>
>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>>>> to unnecessary spamming the klog since up to now this is the only way our
>>>> users can set or receive this info. I'm not sure we want to annoy users like
>>>> that ...
>>>>
>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
>>> instance.
>>
>> I'm not convinced about this so far. The whole point is that we also provide the
>> pid just as we currently do, so that we give the user a chance to possibly pin
>> point the process that needs code change to not use the deprecated API anymore.
>>
>>>> In how many years do you plan a removal ... I think we're stuck with uapi
>>>> basically forever as we don't want to break old binaries, no? ;/
>>>>
>>> I thought we could remove things on a schedule if we followed the deprecation
>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
>>> inform people they are using an older api.
>>
>> I think we rather might be stuck with also the deprecated stuff forever, just
>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
>> remove anything, there's actually also no point to spam the log about it, if
>> everyone can/should read it from the RFC anyway.
>>
> So this begs the question as to why we have deprecation warnings to begin with.
> At what point do we draw the line where we can change some aspect of the user
> api.  I agree if the answer is never, then yeah we're stuck, but then, why
> bother announcing deprecation warnings at all?

I think we can deprecate user API after a significantly long period of time
(like 5 or 6 years).  That's why we also have a deprecation schedule that can
be updated and hopefully that's something people pay attention to.

The problem here is deprecation of ancillary data and that's is a lot tougher
then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
spec.  Ancillary data should not have been enabled using even notification api,
as it is not an event, but we now have to live with it.

-vlad

> 
> Neil
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 05:25:15PM +0200, Daniel Borkmann wrote:
> On 07/09/2014 05:13 PM, Neil Horman wrote:
> >On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> >>On 07/09/2014 12:49 PM, Neil Horman wrote:
> >>>On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>>>On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>>>On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>>>On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>>>...
> >>>>>>>since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>>>deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>>>schedule its eventual removal?
> >>>>>>
> >>>>>>Sure, we can do that. Do you want me to include it into the set?
> >>>>>
> >>>>>If you're plan is to implement 6458, then yes, I think that would be good.
> >>>>
> >>>>Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>>>warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>>>only, which is fine. What you're suggesting is to place similar ratelimited
> >>>>warnings (due to different possible pids on the machine) into the 'fastpath'
> >>>>where we get and set cmsg message headers.
> >>>>
> >>>>While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>>>times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>>>to unnecessary spamming the klog since up to now this is the only way our
> >>>>users can set or receive this info. I'm not sure we want to annoy users like
> >>>>that ...
> >>>>
> >>>Then we wrap it in a ONCE macro so that it only triggers on the first use
> >>>instance.
> >>
> >>I'm not convinced about this so far. The whole point is that we also provide the
> >>pid just as we currently do, so that we give the user a chance to possibly pin
> >>point the process that needs code change to not use the deprecated API anymore.
> >>
> >>>>In how many years do you plan a removal ... I think we're stuck with uapi
> >>>>basically forever as we don't want to break old binaries, no? ;/
> >>>>
> >>>I thought we could remove things on a schedule if we followed the deprecation
> >>>process, but that may just be for sysfs.  Regardless, it would still be nice to
> >>>inform people they are using an older api.
> >>
> >>I think we rather might be stuck with also the deprecated stuff forever, just
> >>as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> >>remove anything, there's actually also no point to spam the log about it, if
> >>everyone can/should read it from the RFC anyway.
> >>
> >So this begs the question as to why we have deprecation warnings to begin with.
> >At what point do we draw the line where we can change some aspect of the user
> >api.  I agree if the answer is never, then yeah we're stuck, but then, why
> >bother announcing deprecation warnings at all?
> 
> That's a good question, not sure when this was decided, Vlad? ;)
Well, I did the deprecation warnings in SCTP a while ago, after we all discussed
them, and there are lots of deprecation warnings spread out through the kernel
that would adversely affect user space api if they were actually removed.  I
added the ones in the get/setsockopt paths under the impression that eventually
they could be removed.  If they can't ever be removed though, it really calls
into question the use of a deprecation warning.

Neil

> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 05:25:15PM +0200, Daniel Borkmann wrote:
> On 07/09/2014 05:13 PM, Neil Horman wrote:
> >On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> >>On 07/09/2014 12:49 PM, Neil Horman wrote:
> >>>On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>>>On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>>>On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>>>On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>>>...
> >>>>>>>since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>>>deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>>>schedule its eventual removal?
> >>>>>>
> >>>>>>Sure, we can do that. Do you want me to include it into the set?
> >>>>>
> >>>>>If you're plan is to implement 6458, then yes, I think that would be good.
> >>>>
> >>>>Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>>>warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>>>only, which is fine. What you're suggesting is to place similar ratelimited
> >>>>warnings (due to different possible pids on the machine) into the 'fastpath'
> >>>>where we get and set cmsg message headers.
> >>>>
> >>>>While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>>>times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>>>to unnecessary spamming the klog since up to now this is the only way our
> >>>>users can set or receive this info. I'm not sure we want to annoy users like
> >>>>that ...
> >>>>
> >>>Then we wrap it in a ONCE macro so that it only triggers on the first use
> >>>instance.
> >>
> >>I'm not convinced about this so far. The whole point is that we also provide the
> >>pid just as we currently do, so that we give the user a chance to possibly pin
> >>point the process that needs code change to not use the deprecated API anymore.
> >>
> >>>>In how many years do you plan a removal ... I think we're stuck with uapi
> >>>>basically forever as we don't want to break old binaries, no? ;/
> >>>>
> >>>I thought we could remove things on a schedule if we followed the deprecation
> >>>process, but that may just be for sysfs.  Regardless, it would still be nice to
> >>>inform people they are using an older api.
> >>
> >>I think we rather might be stuck with also the deprecated stuff forever, just
> >>as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> >>remove anything, there's actually also no point to spam the log about it, if
> >>everyone can/should read it from the RFC anyway.
> >>
> >So this begs the question as to why we have deprecation warnings to begin with.
> >At what point do we draw the line where we can change some aspect of the user
> >api.  I agree if the answer is never, then yeah we're stuck, but then, why
> >bother announcing deprecation warnings at all?
> 
> That's a good question, not sure when this was decided, Vlad? ;)
Well, I did the deprecation warnings in SCTP a while ago, after we all discussed
them, and there are lots of deprecation warnings spread out through the kernel
that would adversely affect user space api if they were actually removed.  I
added the ones in the get/setsockopt paths under the impression that eventually
they could be removed.  If they can't ever be removed though, it really calls
into question the use of a deprecation warning.

Neil

> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 11:36:07AM -0400, Vlad Yasevich wrote:
> On 07/09/2014 11:13 AM, Neil Horman wrote:
> > On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> >> On 07/09/2014 12:49 PM, Neil Horman wrote:
> >>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>>> ...
> >>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>>> schedule its eventual removal?
> >>>>>>
> >>>>>> Sure, we can do that. Do you want me to include it into the set?
> >>>>>
> >>>>> If you're plan is to implement 6458, then yes, I think that would be good.
> >>>>
> >>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>>> only, which is fine. What you're suggesting is to place similar ratelimited
> >>>> warnings (due to different possible pids on the machine) into the 'fastpath'
> >>>> where we get and set cmsg message headers.
> >>>>
> >>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>>> to unnecessary spamming the klog since up to now this is the only way our
> >>>> users can set or receive this info. I'm not sure we want to annoy users like
> >>>> that ...
> >>>>
> >>> Then we wrap it in a ONCE macro so that it only triggers on the first use
> >>> instance.
> >>
> >> I'm not convinced about this so far. The whole point is that we also provide the
> >> pid just as we currently do, so that we give the user a chance to possibly pin
> >> point the process that needs code change to not use the deprecated API anymore.
> >>
> >>>> In how many years do you plan a removal ... I think we're stuck with uapi
> >>>> basically forever as we don't want to break old binaries, no? ;/
> >>>>
> >>> I thought we could remove things on a schedule if we followed the deprecation
> >>> process, but that may just be for sysfs.  Regardless, it would still be nice to
> >>> inform people they are using an older api.
> >>
> >> I think we rather might be stuck with also the deprecated stuff forever, just
> >> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> >> remove anything, there's actually also no point to spam the log about it, if
> >> everyone can/should read it from the RFC anyway.
> >>
> > So this begs the question as to why we have deprecation warnings to begin with.
> > At what point do we draw the line where we can change some aspect of the user
> > api.  I agree if the answer is never, then yeah we're stuck, but then, why
> > bother announcing deprecation warnings at all?
> 
> I think we can deprecate user API after a significantly long period of time
> (like 5 or 6 years).  That's why we also have a deprecation schedule that can
> be updated and hopefully that's something people pay attention to.
> 
> The problem here is deprecation of ancillary data and that's is a lot tougher
> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> spec.  Ancillary data should not have been enabled using even notification api,
> as it is not an event, but we now have to live with it.
> 
Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
deprecate it.  Though that does call the question up as to how to differentiate
expectations of the data format for each cmsg, if they use the same type.  Does
the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
not checked myself yet).

Neil

> -vlad
> 
> > 
> > Neil
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 11:36:07AM -0400, Vlad Yasevich wrote:
> On 07/09/2014 11:13 AM, Neil Horman wrote:
> > On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> >> On 07/09/2014 12:49 PM, Neil Horman wrote:
> >>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>>> ...
> >>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>>> schedule its eventual removal?
> >>>>>>
> >>>>>> Sure, we can do that. Do you want me to include it into the set?
> >>>>>
> >>>>> If you're plan is to implement 6458, then yes, I think that would be good.
> >>>>
> >>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>>> only, which is fine. What you're suggesting is to place similar ratelimited
> >>>> warnings (due to different possible pids on the machine) into the 'fastpath'
> >>>> where we get and set cmsg message headers.
> >>>>
> >>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>>> to unnecessary spamming the klog since up to now this is the only way our
> >>>> users can set or receive this info. I'm not sure we want to annoy users like
> >>>> that ...
> >>>>
> >>> Then we wrap it in a ONCE macro so that it only triggers on the first use
> >>> instance.
> >>
> >> I'm not convinced about this so far. The whole point is that we also provide the
> >> pid just as we currently do, so that we give the user a chance to possibly pin
> >> point the process that needs code change to not use the deprecated API anymore.
> >>
> >>>> In how many years do you plan a removal ... I think we're stuck with uapi
> >>>> basically forever as we don't want to break old binaries, no? ;/
> >>>>
> >>> I thought we could remove things on a schedule if we followed the deprecation
> >>> process, but that may just be for sysfs.  Regardless, it would still be nice to
> >>> inform people they are using an older api.
> >>
> >> I think we rather might be stuck with also the deprecated stuff forever, just
> >> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> >> remove anything, there's actually also no point to spam the log about it, if
> >> everyone can/should read it from the RFC anyway.
> >>
> > So this begs the question as to why we have deprecation warnings to begin with.
> > At what point do we draw the line where we can change some aspect of the user
> > api.  I agree if the answer is never, then yeah we're stuck, but then, why
> > bother announcing deprecation warnings at all?
> 
> I think we can deprecate user API after a significantly long period of time
> (like 5 or 6 years).  That's why we also have a deprecation schedule that can
> be updated and hopefully that's something people pay attention to.
> 
> The problem here is deprecation of ancillary data and that's is a lot tougher
> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> spec.  Ancillary data should not have been enabled using even notification api,
> as it is not an event, but we now have to live with it.
> 
Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
deprecate it.  Though that does call the question up as to how to differentiate
expectations of the data format for each cmsg, if they use the same type.  Does
the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
not checked myself yet).

Neil

> -vlad
> 
> > 
> > Neil
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH net-next 0/5] SCTP updates

[David Laight]

From: Neil Horman
...
> > The problem here is deprecation of ancillary data and that's is a lot tougher
> > then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> > I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> > enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> > spec.  Ancillary data should not have been enabled using even notification api,
> > as it is not an event, but we now have to live with it.
> >
> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
> deprecate it.  Though that does call the question up as to how to differentiate
> expectations of the data format for each cmsg, if they use the same type.  Does
> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
> not checked myself yet).

Not from what I remember from when I read that RFC.
I think the lengths are different enough to determine which is which.

That RFC (I've forgotten the number) looks like an entire bag of poo
that should be ignored...

	David

RE: [PATCH net-next 0/5] SCTP updates

[David Laight]

From: Neil Horman
...
> > The problem here is deprecation of ancillary data and that's is a lot tougher
> > then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> > I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> > enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> > spec.  Ancillary data should not have been enabled using even notification api,
> > as it is not an event, but we now have to live with it.
> >
> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
> deprecate it.  Though that does call the question up as to how to differentiate
> expectations of the data format for each cmsg, if they use the same type.  Does
> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
> not checked myself yet).

Not from what I remember from when I read that RFC.
I think the lengths are different enough to determine which is which.

That RFC (I've forgotten the number) looks like an entire bag of poo
that should be ignored...

	David

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/09/2014 05:36 PM, Vlad Yasevich wrote:
...
> The problem here is deprecation of ancillary data and that's is a lot tougher
> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> spec.  Ancillary data should not have been enabled using even notification api,
> as it is not an event, but we now have to live with it.

Then the set could stay as is from my side.

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/09/2014 05:36 PM, Vlad Yasevich wrote:
...
> The problem here is deprecation of ancillary data and that's is a lot tougher
> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> spec.  Ancillary data should not have been enabled using even notification api,
> as it is not an event, but we now have to live with it.

Then the set could stay as is from my side.

Re: [PATCH net-next 0/5] SCTP updates

[Michael Tuexen]

On 09 Jul 2014, at 18:02, David Laight <David.Laight@ACULAB.COM> wrote:

> From: Neil Horman
> ...
>>> The problem here is deprecation of ancillary data and that's is a lot tougher
>>> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
>>> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
>>> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
I don't think this is true:
To request SCTP_SNDRCVINFO you use the SCTP_EVENTS option. See
http://tools.ietf.org/html/rfc6458#section-6.2.1
To request the SCTP_RCVINFO you use the SCTP_RECVRCVINFO option. See
http://tools.ietf.org/html/rfc6458#section-8.1.29
So the user does different things and the kernel can provide the requested
information.

Best regards
Michael
>>> spec.  Ancillary data should not have been enabled using even notification api,
>>> as it is not an event, but we now have to live with it.
>>> 
>> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
>> deprecate it.  Though that does call the question up as to how to differentiate
>> expectations of the data format for each cmsg, if they use the same type.  Does
>> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
>> not checked myself yet).
> 
> Not from what I remember from when I read that RFC.
> I think the lengths are different enough to determine which is which.
> 
> That RFC (I've forgotten the number) looks like an entire bag of poo
> that should be ignored...
> 
> 	David
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Michael Tuexen]

On 09 Jul 2014, at 18:02, David Laight <David.Laight@ACULAB.COM> wrote:

> From: Neil Horman
> ...
>>> The problem here is deprecation of ancillary data and that's is a lot tougher
>>> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
>>> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
>>> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
I don't think this is true:
To request SCTP_SNDRCVINFO you use the SCTP_EVENTS option. See
http://tools.ietf.org/html/rfc6458#section-6.2.1
To request the SCTP_RCVINFO you use the SCTP_RECVRCVINFO option. See
http://tools.ietf.org/html/rfc6458#section-8.1.29
So the user does different things and the kernel can provide the requested
information.

Best regards
Michael
>>> spec.  Ancillary data should not have been enabled using even notification api,
>>> as it is not an event, but we now have to live with it.
>>> 
>> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
>> deprecate it.  Though that does call the question up as to how to differentiate
>> expectations of the data format for each cmsg, if they use the same type.  Does
>> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
>> not checked myself yet).
> 
> Not from what I remember from when I read that RFC.
> I think the lengths are different enough to determine which is which.
> 
> That RFC (I've forgotten the number) looks like an entire bag of poo
> that should be ignored...
> 
> 	David
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/09/2014 12:12 PM, Michael Tuexen wrote:
> On 09 Jul 2014, at 18:02, David Laight <David.Laight@ACULAB.COM> wrote:
> 
>> From: Neil Horman
>> ...
>>>> The problem here is deprecation of ancillary data and that's is a lot tougher
>>>> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
>>>> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
>>>> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> I don't think this is true:
> To request SCTP_SNDRCVINFO you use the SCTP_EVENTS option. See
> http://tools.ietf.org/html/rfc6458#section-6.2.1
> To request the SCTP_RCVINFO you use the SCTP_RECVRCVINFO option. See
> http://tools.ietf.org/html/rfc6458#section-8.1.29
> So the user does different things and the kernel can provide the requested
> information.

This is not very clear, but yes.  I've re-re-read the section in question after
sending out that mail and yes this does seem to be the case.

-vlad

> 
> Best regards
> Michael
>>>> spec.  Ancillary data should not have been enabled using even notification api,
>>>> as it is not an event, but we now have to live with it.
>>>>
>>> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
>>> deprecate it.  Though that does call the question up as to how to differentiate
>>> expectations of the data format for each cmsg, if they use the same type.  Does
>>> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
>>> not checked myself yet).
>>
>> Not from what I remember from when I read that RFC.
>> I think the lengths are different enough to determine which is which.
>>
>> That RFC (I've forgotten the number) looks like an entire bag of poo
>> that should be ignored...
>>
>> 	David
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/09/2014 12:12 PM, Michael Tuexen wrote:
> On 09 Jul 2014, at 18:02, David Laight <David.Laight@ACULAB.COM> wrote:
> 
>> From: Neil Horman
>> ...
>>>> The problem here is deprecation of ancillary data and that's is a lot tougher
>>>> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
>>>> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
>>>> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> I don't think this is true:
> To request SCTP_SNDRCVINFO you use the SCTP_EVENTS option. See
> http://tools.ietf.org/html/rfc6458#section-6.2.1
> To request the SCTP_RCVINFO you use the SCTP_RECVRCVINFO option. See
> http://tools.ietf.org/html/rfc6458#section-8.1.29
> So the user does different things and the kernel can provide the requested
> information.

This is not very clear, but yes.  I've re-re-read the section in question after
sending out that mail and yes this does seem to be the case.

-vlad

> 
> Best regards
> Michael
>>>> spec.  Ancillary data should not have been enabled using even notification api,
>>>> as it is not an event, but we now have to live with it.
>>>>
>>> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
>>> deprecate it.  Though that does call the question up as to how to differentiate
>>> expectations of the data format for each cmsg, if they use the same type.  Does
>>> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
>>> not checked myself yet).
>>
>> Not from what I remember from when I read that RFC.
>> I think the lengths are different enough to determine which is which.
>>
>> That RFC (I've forgotten the number) looks like an entire bag of poo
>> that should be ignored...
>>
>> 	David
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/09/2014 11:44 AM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 11:36:07AM -0400, Vlad Yasevich wrote:
>> On 07/09/2014 11:13 AM, Neil Horman wrote:
>>> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
>>>> On 07/09/2014 12:49 PM, Neil Horman wrote:
>>>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>>>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>>>>>> ...
>>>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>>>>>> schedule its eventual removal?
>>>>>>>>
>>>>>>>> Sure, we can do that. Do you want me to include it into the set?
>>>>>>>
>>>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>>>>>
>>>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>>>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>>>>>> only, which is fine. What you're suggesting is to place similar ratelimited
>>>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
>>>>>> where we get and set cmsg message headers.
>>>>>>
>>>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>>>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>>>>>> to unnecessary spamming the klog since up to now this is the only way our
>>>>>> users can set or receive this info. I'm not sure we want to annoy users like
>>>>>> that ...
>>>>>>
>>>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
>>>>> instance.
>>>>
>>>> I'm not convinced about this so far. The whole point is that we also provide the
>>>> pid just as we currently do, so that we give the user a chance to possibly pin
>>>> point the process that needs code change to not use the deprecated API anymore.
>>>>
>>>>>> In how many years do you plan a removal ... I think we're stuck with uapi
>>>>>> basically forever as we don't want to break old binaries, no? ;/
>>>>>>
>>>>> I thought we could remove things on a schedule if we followed the deprecation
>>>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
>>>>> inform people they are using an older api.
>>>>
>>>> I think we rather might be stuck with also the deprecated stuff forever, just
>>>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
>>>> remove anything, there's actually also no point to spam the log about it, if
>>>> everyone can/should read it from the RFC anyway.
>>>>
>>> So this begs the question as to why we have deprecation warnings to begin with.
>>> At what point do we draw the line where we can change some aspect of the user
>>> api.  I agree if the answer is never, then yeah we're stuck, but then, why
>>> bother announcing deprecation warnings at all?
>>
>> I think we can deprecate user API after a significantly long period of time
>> (like 5 or 6 years).  That's why we also have a deprecation schedule that can
>> be updated and hopefully that's something people pay attention to.
>>
>> The problem here is deprecation of ancillary data and that's is a lot tougher
>> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
>> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
>> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
>> spec.  Ancillary data should not have been enabled using even notification api,
>> as it is not an event, but we now have to live with it.
>>
> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
> deprecate it.  Though that does call the question up as to how to differentiate
> expectations of the data format for each cmsg, if they use the same type.  Does
> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
> not checked myself yet).

No there is not direct overlap between the two.  However, as Michael pointed out,
there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.

-vlad

> 
> Neil
> 
>> -vlad
>>
>>>
>>> Neil
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/09/2014 11:44 AM, Neil Horman wrote:
> On Wed, Jul 09, 2014 at 11:36:07AM -0400, Vlad Yasevich wrote:
>> On 07/09/2014 11:13 AM, Neil Horman wrote:
>>> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
>>>> On 07/09/2014 12:49 PM, Neil Horman wrote:
>>>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
>>>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
>>>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
>>>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
>>>>>>>> ...
>>>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
>>>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
>>>>>>>>> schedule its eventual removal?
>>>>>>>>
>>>>>>>> Sure, we can do that. Do you want me to include it into the set?
>>>>>>>
>>>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
>>>>>>
>>>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
>>>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
>>>>>> only, which is fine. What you're suggesting is to place similar ratelimited
>>>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
>>>>>> where we get and set cmsg message headers.
>>>>>>
>>>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
>>>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
>>>>>> to unnecessary spamming the klog since up to now this is the only way our
>>>>>> users can set or receive this info. I'm not sure we want to annoy users like
>>>>>> that ...
>>>>>>
>>>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
>>>>> instance.
>>>>
>>>> I'm not convinced about this so far. The whole point is that we also provide the
>>>> pid just as we currently do, so that we give the user a chance to possibly pin
>>>> point the process that needs code change to not use the deprecated API anymore.
>>>>
>>>>>> In how many years do you plan a removal ... I think we're stuck with uapi
>>>>>> basically forever as we don't want to break old binaries, no? ;/
>>>>>>
>>>>> I thought we could remove things on a schedule if we followed the deprecation
>>>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
>>>>> inform people they are using an older api.
>>>>
>>>> I think we rather might be stuck with also the deprecated stuff forever, just
>>>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
>>>> remove anything, there's actually also no point to spam the log about it, if
>>>> everyone can/should read it from the RFC anyway.
>>>>
>>> So this begs the question as to why we have deprecation warnings to begin with.
>>> At what point do we draw the line where we can change some aspect of the user
>>> api.  I agree if the answer is never, then yeah we're stuck, but then, why
>>> bother announcing deprecation warnings at all?
>>
>> I think we can deprecate user API after a significantly long period of time
>> (like 5 or 6 years).  That's why we also have a deprecation schedule that can
>> be updated and hopefully that's something people pay attention to.
>>
>> The problem here is deprecation of ancillary data and that's is a lot tougher
>> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
>> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
>> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
>> spec.  Ancillary data should not have been enabled using even notification api,
>> as it is not an event, but we now have to live with it.
>>
> Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
> deprecate it.  Though that does call the question up as to how to differentiate
> expectations of the data format for each cmsg, if they use the same type.  Does
> the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
> not checked myself yet).

No there is not direct overlap between the two.  However, as Michael pointed out,
there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.

-vlad

> 
> Neil
> 
>> -vlad
>>
>>>
>>> Neil
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 12:32:55PM -0400, Vlad Yasevich wrote:
> On 07/09/2014 11:44 AM, Neil Horman wrote:
> > On Wed, Jul 09, 2014 at 11:36:07AM -0400, Vlad Yasevich wrote:
> >> On 07/09/2014 11:13 AM, Neil Horman wrote:
> >>> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> >>>> On 07/09/2014 12:49 PM, Neil Horman wrote:
> >>>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>>>>> ...
> >>>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>>>>> schedule its eventual removal?
> >>>>>>>>
> >>>>>>>> Sure, we can do that. Do you want me to include it into the set?
> >>>>>>>
> >>>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
> >>>>>>
> >>>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>>>>> only, which is fine. What you're suggesting is to place similar ratelimited
> >>>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
> >>>>>> where we get and set cmsg message headers.
> >>>>>>
> >>>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>>>>> to unnecessary spamming the klog since up to now this is the only way our
> >>>>>> users can set or receive this info. I'm not sure we want to annoy users like
> >>>>>> that ...
> >>>>>>
> >>>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
> >>>>> instance.
> >>>>
> >>>> I'm not convinced about this so far. The whole point is that we also provide the
> >>>> pid just as we currently do, so that we give the user a chance to possibly pin
> >>>> point the process that needs code change to not use the deprecated API anymore.
> >>>>
> >>>>>> In how many years do you plan a removal ... I think we're stuck with uapi
> >>>>>> basically forever as we don't want to break old binaries, no? ;/
> >>>>>>
> >>>>> I thought we could remove things on a schedule if we followed the deprecation
> >>>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
> >>>>> inform people they are using an older api.
> >>>>
> >>>> I think we rather might be stuck with also the deprecated stuff forever, just
> >>>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> >>>> remove anything, there's actually also no point to spam the log about it, if
> >>>> everyone can/should read it from the RFC anyway.
> >>>>
> >>> So this begs the question as to why we have deprecation warnings to begin with.
> >>> At what point do we draw the line where we can change some aspect of the user
> >>> api.  I agree if the answer is never, then yeah we're stuck, but then, why
> >>> bother announcing deprecation warnings at all?
> >>
> >> I think we can deprecate user API after a significantly long period of time
> >> (like 5 or 6 years).  That's why we also have a deprecation schedule that can
> >> be updated and hopefully that's something people pay attention to.
> >>
> >> The problem here is deprecation of ancillary data and that's is a lot tougher
> >> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> >> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> >> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> >> spec.  Ancillary data should not have been enabled using even notification api,
> >> as it is not an event, but we now have to live with it.
> >>
> > Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
> > deprecate it.  Though that does call the question up as to how to differentiate
> > expectations of the data format for each cmsg, if they use the same type.  Does
> > the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
> > not checked myself yet).
> 
> No there is not direct overlap between the two.  However, as Michael pointed out,
> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> 
Ok, so we should still consider deprecation warnings then.  Daniel, what about
ratelimited warnings with pids included then?
Neil

> -vlad
> 
> > 
> > Neil
> > 
> >> -vlad
> >>
> >>>
> >>> Neil
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> 
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Wed, Jul 09, 2014 at 12:32:55PM -0400, Vlad Yasevich wrote:
> On 07/09/2014 11:44 AM, Neil Horman wrote:
> > On Wed, Jul 09, 2014 at 11:36:07AM -0400, Vlad Yasevich wrote:
> >> On 07/09/2014 11:13 AM, Neil Horman wrote:
> >>> On Wed, Jul 09, 2014 at 03:28:03PM +0200, Daniel Borkmann wrote:
> >>>> On 07/09/2014 12:49 PM, Neil Horman wrote:
> >>>>> On Wed, Jul 09, 2014 at 11:57:37AM +0200, Daniel Borkmann wrote:
> >>>>>> On 07/08/2014 04:41 PM, Neil Horman wrote:
> >>>>>>> On Tue, Jul 08, 2014 at 04:05:26PM +0200, Daniel Borkmann wrote:
> >>>>>>>> On 07/08/2014 01:14 PM, Neil Horman wrote:
> >>>>>>>> ...
> >>>>>>>>> since you're adding cmsg's from rfc6458, do you also want to add some
> >>>>>>>>> deprecation warnings around the use of SCTP_SNDRCVINFO too, so we can start to
> >>>>>>>>> schedule its eventual removal?
> >>>>>>>>
> >>>>>>>> Sure, we can do that. Do you want me to include it into the set?
> >>>>>>>
> >>>>>>> If you're plan is to implement 6458, then yes, I think that would be good.
> >>>>>>
> >>>>>> Looking a bit closer at it, all our pr_warn_ratelimited(DEPRECATED ...)
> >>>>>> warnings in SCTP are being done in 'slowpath' {set,get}sockopt(2) operations
> >>>>>> only, which is fine. What you're suggesting is to place similar ratelimited
> >>>>>> warnings (due to different possible pids on the machine) into the 'fastpath'
> >>>>>> where we get and set cmsg message headers.
> >>>>>>
> >>>>>> While that may be fine for {set,get}sockopt(2) that's called once or very few
> >>>>>> times, I'm not sure this is a good idea in SCTP_SNDRCVINFO as it will yield
> >>>>>> to unnecessary spamming the klog since up to now this is the only way our
> >>>>>> users can set or receive this info. I'm not sure we want to annoy users like
> >>>>>> that ...
> >>>>>>
> >>>>> Then we wrap it in a ONCE macro so that it only triggers on the first use
> >>>>> instance.
> >>>>
> >>>> I'm not convinced about this so far. The whole point is that we also provide the
> >>>> pid just as we currently do, so that we give the user a chance to possibly pin
> >>>> point the process that needs code change to not use the deprecated API anymore.
> >>>>
> >>>>>> In how many years do you plan a removal ... I think we're stuck with uapi
> >>>>>> basically forever as we don't want to break old binaries, no? ;/
> >>>>>>
> >>>>> I thought we could remove things on a schedule if we followed the deprecation
> >>>>> process, but that may just be for sysfs.  Regardless, it would still be nice to
> >>>>> inform people they are using an older api.
> >>>>
> >>>> I think we rather might be stuck with also the deprecated stuff forever, just
> >>>> as AF_PACKET still has to carry around the old spkt stuff. :( So if we don't
> >>>> remove anything, there's actually also no point to spam the log about it, if
> >>>> everyone can/should read it from the RFC anyway.
> >>>>
> >>> So this begs the question as to why we have deprecation warnings to begin with.
> >>> At what point do we draw the line where we can change some aspect of the user
> >>> api.  I agree if the answer is never, then yeah we're stuck, but then, why
> >>> bother announcing deprecation warnings at all?
> >>
> >> I think we can deprecate user API after a significantly long period of time
> >> (like 5 or 6 years).  That's why we also have a deprecation schedule that can
> >> be updated and hopefully that's something people pay attention to.
> >>
> >> The problem here is deprecation of ancillary data and that's is a lot tougher
> >> then socket options.  In this particular case (SCTP_SNDRCVINFO vs SCTP_RCVINFO),
> >> I don't think there is any way to deprecate the SCTP_SNDRCVINFO since the event
> >> enabling it is the same as the one for SCTP_RCVINFO.  This was a mistake in the
> >> spec.  Ancillary data should not have been enabled using even notification api,
> >> as it is not an event, but we now have to live with it.
> >>
> > Ugh I didn't even consider cmsg type overlap.  Thats probably it then, we can't
> > deprecate it.  Though that does call the question up as to how to differentiate
> > expectations of the data format for each cmsg, if they use the same type.  Does
> > the SCTP_RCVINFO data struct overlay the SNDRCVINFO struct exactly?  (sorry I've
> > not checked myself yet).
> 
> No there is not direct overlap between the two.  However, as Michael pointed out,
> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> 
Ok, so we should still consider deprecation warnings then.  Daniel, what about
ratelimited warnings with pids included then?
Neil

> -vlad
> 
> > 
> > Neil
> > 
> >> -vlad
> >>
> >>>
> >>> Neil
> >>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>>
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> 
> 

RE: [PATCH net-next 0/5] SCTP updates

[David Laight]

From: Neil Horman
...
> > No there is not direct overlap between the two.  However, as Michael pointed out,
> > there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> > warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> > Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> >
> Ok, so we should still consider deprecation warnings then.  Daniel, what about
> ratelimited warnings with pids included then?

Can you defer any deprecation warnings for a few kernel versions?
This gives time for applications to be recoded.
Including argv[0] (even just the exec-time value) is much more use than the pid.

Actually this is 'right PITA' for an application.
A program binary that needs to work with old and new kernels will have to
try the new option, and if it fails fall back to the old one, and then
conditionally create/inspect the cmsg data.
I can't actually imagine anyone bothering!

Our sctp code is actually in a kernel module, so we can look at the kernel
version when (part of) the driver is compiled on the target system.

	David

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/10/2014 11:02 AM, David Laight wrote:
> From: Neil Horman
> ...
>>> No there is not direct overlap between the two.  However, as Michael pointed out,
>>> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
>>> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
>>> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
>>>
>> Ok, so we should still consider deprecation warnings then.  Daniel, what about
>> ratelimited warnings with pids included then?
>
> Can you defer any deprecation warnings for a few kernel versions?
> This gives time for applications to be recoded.

I am fine either way, adding the warning a bit later seems fine, too.

Right now, even in-kernel users like dlm would hit it if we include
it immediately.

Otherwise, I'll just add something like the below ...

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d95a50c..6a0e5a4 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user *optval,
  	if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
  		return -EFAULT;

-	/*
-	 * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
+	if (sctp_sk(sk)->subscribe.sctp_data_io_event)
+		pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
+				    "Requested SCTP_SNDRCVINFO event.\n"
+				    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
+				    current->comm, task_pid_nr(current));
+
+	/* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
  	 * if there is no data to be sent or retransmit, the stack will
  	 * immediately send up this notification.
  	 */

> Including argv[0] (even just the exec-time value) is much more use than the pid.
>
> Actually this is 'right PITA' for an application.
> A program binary that needs to work with old and new kernels will have to
> try the new option, and if it fails fall back to the old one, and then
> conditionally create/inspect the cmsg data.
> I can't actually imagine anyone bothering!
>
> Our sctp code is actually in a kernel module, so we can look at the kernel
> version when (part of) the driver is compiled on the target system.
>
> 	David
>
>
>

Re: [PATCH net-next 0/5] SCTP updates

[Daniel Borkmann]

On 07/10/2014 11:02 AM, David Laight wrote:
> From: Neil Horman
> ...
>>> No there is not direct overlap between the two.  However, as Michael pointed out,
>>> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
>>> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
>>> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
>>>
>> Ok, so we should still consider deprecation warnings then.  Daniel, what about
>> ratelimited warnings with pids included then?
>
> Can you defer any deprecation warnings for a few kernel versions?
> This gives time for applications to be recoded.

I am fine either way, adding the warning a bit later seems fine, too.

Right now, even in-kernel users like dlm would hit it if we include
it immediately.

Otherwise, I'll just add something like the below ...

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d95a50c..6a0e5a4 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user *optval,
  	if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
  		return -EFAULT;

-	/*
-	 * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
+	if (sctp_sk(sk)->subscribe.sctp_data_io_event)
+		pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
+				    "Requested SCTP_SNDRCVINFO event.\n"
+				    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
+				    current->comm, task_pid_nr(current));
+
+	/* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
  	 * if there is no data to be sent or retransmit, the stack will
  	 * immediately send up this notification.
  	 */

> Including argv[0] (even just the exec-time value) is much more use than the pid.
>
> Actually this is 'right PITA' for an application.
> A program binary that needs to work with old and new kernels will have to
> try the new option, and if it fails fall back to the old one, and then
> conditionally create/inspect the cmsg data.
> I can't actually imagine anyone bothering!
>
> Our sctp code is actually in a kernel module, so we can look at the kernel
> version when (part of) the driver is compiled on the target system.
>
> 	David
>
>
>

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Thu, Jul 10, 2014 at 09:02:23AM +0000, David Laight wrote:
> From: Neil Horman
> ...
> > > No there is not direct overlap between the two.  However, as Michael pointed out,
> > > there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> > > warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> > > Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> > >
> > Ok, so we should still consider deprecation warnings then.  Daniel, what about
> > ratelimited warnings with pids included then?
> 
> Can you defer any deprecation warnings for a few kernel versions?
> This gives time for applications to be recoded.
I'm not sure I understand the reasoning here?  Why should a deprecation warning
be delayed so you can recode your application?  What about all the people who
aren't subscribed this list?  That just furhter delays their finding out about
the deprecation.

> Including argv[0] (even just the exec-time value) is much more use than the pid.
> 
Thats fine, I think deprecation warnings usually have the form "[deprecated] pid
%d(%s)..." where %s is argv[0].

> Actually this is 'right PITA' for an application.
> A program binary that needs to work with old and new kernels will have to
> try the new option, and if it fails fall back to the old one, and then
> conditionally create/inspect the cmsg data.
> I can't actually imagine anyone bothering!
> 
A deprecation warning is just that, a warning, it doesn't cause the old
method to fail, its just a heads up to the application developer that they need
to make a change sometime in the next 5 years.  If you implement the new method
in your application, you can wait for a reasonable period of time until you are
confident that you application isn't being used on older kernels, or bump the
minimal kernel version to run your app, and cut over to the new method.

> Our sctp code is actually in a kernel module, so we can look at the kernel
> version when (part of) the driver is compiled on the target system.
> 
You can do that too if you would like.
Neil

> 	David
> 
> 
> 
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Thu, Jul 10, 2014 at 09:02:23AM +0000, David Laight wrote:
> From: Neil Horman
> ...
> > > No there is not direct overlap between the two.  However, as Michael pointed out,
> > > there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> > > warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> > > Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> > >
> > Ok, so we should still consider deprecation warnings then.  Daniel, what about
> > ratelimited warnings with pids included then?
> 
> Can you defer any deprecation warnings for a few kernel versions?
> This gives time for applications to be recoded.
I'm not sure I understand the reasoning here?  Why should a deprecation warning
be delayed so you can recode your application?  What about all the people who
aren't subscribed this list?  That just furhter delays their finding out about
the deprecation.

> Including argv[0] (even just the exec-time value) is much more use than the pid.
> 
Thats fine, I think deprecation warnings usually have the form "[deprecated] pid
%d(%s)..." where %s is argv[0].

> Actually this is 'right PITA' for an application.
> A program binary that needs to work with old and new kernels will have to
> try the new option, and if it fails fall back to the old one, and then
> conditionally create/inspect the cmsg data.
> I can't actually imagine anyone bothering!
> 
A deprecation warning is just that, a warning, it doesn't cause the old
method to fail, its just a heads up to the application developer that they need
to make a change sometime in the next 5 years.  If you implement the new method
in your application, you can wait for a reasonable period of time until you are
confident that you application isn't being used on older kernels, or bump the
minimal kernel version to run your app, and cut over to the new method.

> Our sctp code is actually in a kernel module, so we can look at the kernel
> version when (part of) the driver is compiled on the target system.
> 
You can do that too if you would like.
Neil

> 	David
> 
> 
> 
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Thu, Jul 10, 2014 at 11:37:19AM +0200, Daniel Borkmann wrote:
> On 07/10/2014 11:02 AM, David Laight wrote:
> >From: Neil Horman
> >...
> >>>No there is not direct overlap between the two.  However, as Michael pointed out,
> >>>there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> >>>warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> >>>Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> >>>
> >>Ok, so we should still consider deprecation warnings then.  Daniel, what about
> >>ratelimited warnings with pids included then?
> >
> >Can you defer any deprecation warnings for a few kernel versions?
> >This gives time for applications to be recoded.
> 
> I am fine either way, adding the warning a bit later seems fine, too.
> 
> Right now, even in-kernel users like dlm would hit it if we include
> it immediately.
> 
> Otherwise, I'll just add something like the below ...
> 
This works for me, I don't see any reason to wait, and its a pretty easy
changeover for most applications to make.

Thanks David.
Neil

> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d95a50c..6a0e5a4 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user *optval,
>  	if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
>  		return -EFAULT;
> 
> -	/*
> -	 * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
> +	if (sctp_sk(sk)->subscribe.sctp_data_io_event)
> +		pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
> +				    "Requested SCTP_SNDRCVINFO event.\n"
> +				    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
> +				    current->comm, task_pid_nr(current));
> +
> +	/* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
>  	 * if there is no data to be sent or retransmit, the stack will
>  	 * immediately send up this notification.
>  	 */
> 
> >Including argv[0] (even just the exec-time value) is much more use than the pid.
> >
> >Actually this is 'right PITA' for an application.
> >A program binary that needs to work with old and new kernels will have to
> >try the new option, and if it fails fall back to the old one, and then
> >conditionally create/inspect the cmsg data.
> >I can't actually imagine anyone bothering!
> >
> >Our sctp code is actually in a kernel module, so we can look at the kernel
> >version when (part of) the driver is compiled on the target system.
> >
> >	David
> >
> >
> >
> 

Re: [PATCH net-next 0/5] SCTP updates

[Neil Horman]

On Thu, Jul 10, 2014 at 11:37:19AM +0200, Daniel Borkmann wrote:
> On 07/10/2014 11:02 AM, David Laight wrote:
> >From: Neil Horman
> >...
> >>>No there is not direct overlap between the two.  However, as Michael pointed out,
> >>>there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
> >>>warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
> >>>Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
> >>>
> >>Ok, so we should still consider deprecation warnings then.  Daniel, what about
> >>ratelimited warnings with pids included then?
> >
> >Can you defer any deprecation warnings for a few kernel versions?
> >This gives time for applications to be recoded.
> 
> I am fine either way, adding the warning a bit later seems fine, too.
> 
> Right now, even in-kernel users like dlm would hit it if we include
> it immediately.
> 
> Otherwise, I'll just add something like the below ...
> 
This works for me, I don't see any reason to wait, and its a pretty easy
changeover for most applications to make.

Thanks David.
Neil

> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d95a50c..6a0e5a4 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user *optval,
>  	if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
>  		return -EFAULT;
> 
> -	/*
> -	 * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
> +	if (sctp_sk(sk)->subscribe.sctp_data_io_event)
> +		pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
> +				    "Requested SCTP_SNDRCVINFO event.\n"
> +				    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
> +				    current->comm, task_pid_nr(current));
> +
> +	/* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
>  	 * if there is no data to be sent or retransmit, the stack will
>  	 * immediately send up this notification.
>  	 */
> 
> >Including argv[0] (even just the exec-time value) is much more use than the pid.
> >
> >Actually this is 'right PITA' for an application.
> >A program binary that needs to work with old and new kernels will have to
> >try the new option, and if it fails fall back to the old one, and then
> >conditionally create/inspect the cmsg data.
> >I can't actually imagine anyone bothering!
> >
> >Our sctp code is actually in a kernel module, so we can look at the kernel
> >version when (part of) the driver is compiled on the target system.
> >
> >	David
> >
> >
> >
> 

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/10/2014 05:37 AM, Daniel Borkmann wrote:
> On 07/10/2014 11:02 AM, David Laight wrote:
>> From: Neil Horman
>> ...
>>>> No there is not direct overlap between the two.  However, as Michael pointed out,
>>>> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
>>>> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
>>>> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
>>>>
>>> Ok, so we should still consider deprecation warnings then.  Daniel, what about
>>> ratelimited warnings with pids included then?
>>
>> Can you defer any deprecation warnings for a few kernel versions?
>> This gives time for applications to be recoded.
> 
> I am fine either way, adding the warning a bit later seems fine, too.
> 
> Right now, even in-kernel users like dlm would hit it if we include
> it immediately.
> 
> Otherwise, I'll just add something like the below ...
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d95a50c..6a0e5a4 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user
> *optval,
>      if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
>          return -EFAULT;
> 
> -    /*
> -     * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
> +    if (sctp_sk(sk)->subscribe.sctp_data_io_event)
> +        pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
> +                    "Requested SCTP_SNDRCVINFO event.\n"
> +                    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
> +                    current->comm, task_pid_nr(current));
> +
> +    /* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
>       * if there is no data to be sent or retransmit, the stack will
>       * immediately send up this notification.
>       */
> 

This is OK for step 1.  Once we implement SCTP_EVENT, we can cover the rest of the events.

-vlad

>> Including argv[0] (even just the exec-time value) is much more use than the pid.
>>
>> Actually this is 'right PITA' for an application.
>> A program binary that needs to work with old and new kernels will have to
>> try the new option, and if it fails fall back to the old one, and then
>> conditionally create/inspect the cmsg data.
>> I can't actually imagine anyone bothering!
>>
>> Our sctp code is actually in a kernel module, so we can look at the kernel
>> version when (part of) the driver is compiled on the target system.
>>
>>     David
>>
>>
>>

Re: [PATCH net-next 0/5] SCTP updates

[Vlad Yasevich]

On 07/10/2014 05:37 AM, Daniel Borkmann wrote:
> On 07/10/2014 11:02 AM, David Laight wrote:
>> From: Neil Horman
>> ...
>>>> No there is not direct overlap between the two.  However, as Michael pointed out,
>>>> there is a new option to control SCTP_RCVINFO.  So would could add a deprecation
>>>> warning to the over SCTP_EVENTS option and carry SCTP_SNDRCVINFO with it.
>>>> Once SCTP_EVENTS goes away so can SCTP_SNDRCVINFO.
>>>>
>>> Ok, so we should still consider deprecation warnings then.  Daniel, what about
>>> ratelimited warnings with pids included then?
>>
>> Can you defer any deprecation warnings for a few kernel versions?
>> This gives time for applications to be recoded.
> 
> I am fine either way, adding the warning a bit later seems fine, too.
> 
> Right now, even in-kernel users like dlm would hit it if we include
> it immediately.
> 
> Otherwise, I'll just add something like the below ...
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d95a50c..6a0e5a4 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2205,8 +2205,13 @@ static int sctp_setsockopt_events(struct sock *sk, char __user
> *optval,
>      if (copy_from_user(&sctp_sk(sk)->subscribe, optval, optlen))
>          return -EFAULT;
> 
> -    /*
> -     * At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
> +    if (sctp_sk(sk)->subscribe.sctp_data_io_event)
> +        pr_warn_ratelimited(DEPRECATED "%s (pid %d) "
> +                    "Requested SCTP_SNDRCVINFO event.\n"
> +                    "Use SCTP_RCVINFO through SCTP_RECVRCVINFO option instead.\n",
> +                    current->comm, task_pid_nr(current));
> +
> +    /* At the time when a user app subscribes to SCTP_SENDER_DRY_EVENT,
>       * if there is no data to be sent or retransmit, the stack will
>       * immediately send up this notification.
>       */
> 

This is OK for step 1.  Once we implement SCTP_EVENT, we can cover the rest of the events.

-vlad

>> Including argv[0] (even just the exec-time value) is much more use than the pid.
>>
>> Actually this is 'right PITA' for an application.
>> A program binary that needs to work with old and new kernels will have to
>> try the new option, and if it fails fall back to the old one, and then
>> conditionally create/inspect the cmsg data.
>> I can't actually imagine anyone bothering!
>>
>> Our sctp code is actually in a kernel module, so we can look at the kernel
>> version when (part of) the driver is compiled on the target system.
>>
>>     David
>>
>>
>>

[PATCH net-next 0/5]: SCTP updates

[Vlad Yasevich]

[PATCH net-next 0/5]: SCTP updates

[Vlad Yasevich]

[PATCH net-next 0/5]: SCTP updates

[Vlad Yasevich]

Hi David

The following series is set of updates for net-next.  It is primarily
new API and cleanups.  However patch 04/05 (Avoid memory overflow while
FWD-TSN chunk is received with bad stream ID) probably needs to be pushed
to stable.

Please apply.  If you want me to respin 04/05 against net-2.6, let me know.

Thanks
-vlad

[PATCH net-next 0/5]: SCTP updates

[Vlad Yasevich]

Hi David

The following series is set of updates for net-next.  It is primarily
new API and cleanups.  However patch 04/05 (Avoid memory overflow while
FWD-TSN chunk is received with bad stream ID) probably needs to be pushed
to stable.

Please apply.  If you want me to respin 04/05 against net-2.6, let me know.

Thanks
-vlad