Skip to contentSkip to conversation

sctp: unify sctp_make_op_error_fixed and sctp_make_op_error_space

PATCH In review Marcelo Ricardo Leitner 4 participants 14 messages Apr 29, 2018 – May 23, 2018

Subsystems (3)
lore
Share & tools
lore

Marcelo Ricardo Leitner

author maintainer
#0 lore raw

cover sctp: unify sctp_make_op_error_fixed and sctp_make_op_error_space

These two variants are very close to each other and can be merged to avoid code duplication. That's what this patchset does.

First, we allow sctp_init_cause to return errors, which then allow us to add sctp_make_op_error_limited that handles both situations.

Marcelo Ricardo Leitner (2):
sctp: allow sctp_init_cause to return errors
sctp: add sctp_make_op_error_limited and reuse inner functions

 include/net/sctp/sm.h    |   2 +-
 net/sctp/sm_make_chunk.c | 134 ++++++++++++++++++-----------------------------
 2 files changed, 52 insertions(+), 84 deletions(-)

--
2.14.3

author maintainer
#1 lore raw

PATCH 1/2 sctp: allow sctp_init_cause to return errors

And do so if the skb doesn't have enough space for the payload. This is a preparation for the next patch.

---
 include/net/sctp/sm.h    |  2 +-
 net/sctp/sm_make_chunk.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 4 deletions(-)
include/net/sctp/sm.h +1 -1
index f4b657478a304050851f33d92c71162a4a4a2e50..5ef1bad81ef54906b375dbfd0e4fd897d078abce 100644
@@ -215,7 +215,7 @@ struct sctp_chunk *sctp_make_shutdown_ack(const struct sctp_association *asoc,
 struct sctp_chunk *sctp_make_shutdown_complete(
 					const struct sctp_association *asoc,
 					const struct sctp_chunk *chunk);
-void sctp_init_cause(struct sctp_chunk *chunk, __be16 cause, size_t paylen);
+int sctp_init_cause(struct sctp_chunk *chunk, __be16 cause, size_t paylen);
 struct sctp_chunk *sctp_make_abort(const struct sctp_association *asoc,
 				   const struct sctp_chunk *chunk,
 				   const size_t hint);
net/sctp/sm_make_chunk.c +9 -3
index db93eabd6ef500ab20be6e091ee06bd3b60923c9..e518eb64ccf3578f7892da050c160a56cf3cc833 100644
@@ -158,8 +158,8 @@ static const struct sctp_paramhdr prsctp_param = {
  * provided chunk, as most cause codes will be embedded inside an
  * abort chunk.
  */
-void sctp_init_cause(struct sctp_chunk *chunk, __be16 cause_code,
-		     size_t paylen)
+int sctp_init_cause(struct sctp_chunk *chunk, __be16 cause_code,
+		    size_t paylen)
 {
 	struct sctp_errhdr err;
 	__u16 len;
@@ -167,8 +167,14 @@ void sctp_init_cause(struct sctp_chunk *chunk, __be16 cause_code,
 	/* Cause code constants are now defined in network order.  */
 	err.cause = cause_code;
 	len = sizeof(err) + paylen;
-	err.length  = htons(len);
+	err.length = htons(len);
+
+	if (skb_tailroom(chunk->skb) < len)
+		return -ENOSPC;
+
 	chunk->subh.err_hdr = sctp_addto_chunk(chunk, sizeof(err), &err);
+
+	return 0;
 }
 
 /* A helper to initialize an op error inside a
Signature · 1 line
2.14.3
author maintainer
#2 lore raw

PATCH 2/2 sctp: add sctp_make_op_error_limited and reuse inner functions

In reply to Marcelo Ricardo Leitner

The idea is quite similar to the old functions, but note that the _fixed function wasn't "fixed" as in that it would generate a packet with a fixed size, but rather limited/bounded to PMTU.

Also, now with sctp_mtu_payload(), we have a more accurate limit.

---
 net/sctp/sm_make_chunk.c | 130 +++++++++++++++++------------------------------
 1 file changed, 46 insertions(+), 84 deletions(-)
net/sctp/sm_make_chunk.c +46 -83
index e518eb64ccf3578f7892da050c160a56cf3cc833..4d7b3ccea0789f3a695f710046b50855e4cc41fc 100644
@@ -81,8 +81,6 @@ static int sctp_process_param(struct sctp_association *asoc,
 			      gfp_t gfp);
 static void *sctp_addto_param(struct sctp_chunk *chunk, int len,
 			      const void *data);
-static void  *sctp_addto_chunk_fixed(struct sctp_chunk *, int len,
-				     const void *data);
 
 /* Control chunk destructor */
 static void sctp_control_release_owner(struct sk_buff *skb)
@@ -154,9 +152,8 @@ static const struct sctp_paramhdr prsctp_param = {
 	cpu_to_be16(sizeof(struct sctp_paramhdr)),
 };
 
-/* A helper to initialize an op error inside a
- * provided chunk, as most cause codes will be embedded inside an
- * abort chunk.
+/* A helper to initialize an op error inside a provided chunk, as most
+ * cause codes will be embedded inside an abort chunk.
  */
 int sctp_init_cause(struct sctp_chunk *chunk, __be16 cause_code,
 		    size_t paylen)
@@ -177,29 +174,6 @@ int sctp_init_cause(struct sctp_chunk *chunk, __be16 cause_code,
 	return 0;
 }
 
-/* A helper to initialize an op error inside a
- * provided chunk, as most cause codes will be embedded inside an
- * abort chunk.  Differs from sctp_init_cause in that it won't oops
- * if there isn't enough space in the op error chunk
- */
-static int sctp_init_cause_fixed(struct sctp_chunk *chunk, __be16 cause_code,
-				 size_t paylen)
-{
-	struct sctp_errhdr err;
-	__u16 len;
-
-	/* Cause code constants are now defined in network order.  */
-	err.cause = cause_code;
-	len = sizeof(err) + paylen;
-	err.length  = htons(len);
-
-	if (skb_tailroom(chunk->skb) < len)
-		return -ENOSPC;
-
-	chunk->subh.err_hdr = sctp_addto_chunk_fixed(chunk, sizeof(err), &err);
-
-	return 0;
-}
 /* 3.3.2 Initiation (INIT) (1)
  *
  * This chunk is used to initiate a SCTP association between two
@@ -1263,20 +1237,26 @@ static struct sctp_chunk *sctp_make_op_error_space(
 	return retval;
 }
 
-/* Create an Operation Error chunk of a fixed size,
- * specifically, max(asoc->pathmtu, SCTP_DEFAULT_MAXSEGMENT)
- * This is a helper function to allocate an error chunk for
- * for those invalid parameter codes in which we may not want
- * to report all the errors, if the incoming chunk is large
+/* Create an Operation Error chunk of a fixed size, specifically,
+ * min(asoc->pathmtu, SCTP_DEFAULT_MAXSEGMENT) - overheads.
+ * This is a helper function to allocate an error chunk for for those
+ * invalid parameter codes in which we may not want to report all the
+ * errors, if the incoming chunk is large. If it can't fit in a single
+ * packet, we ignore it.
  */
-static inline struct sctp_chunk *sctp_make_op_error_fixed(
+static inline struct sctp_chunk *sctp_make_op_error_limited(
 					const struct sctp_association *asoc,
 					const struct sctp_chunk *chunk)
 {
-	size_t size = asoc ? asoc->pathmtu : 0;
+	size_t size = SCTP_DEFAULT_MAXSEGMENT;
+	struct sctp_sock *sp = NULL;
+
+	if (asoc) {
+		size = min_t(size_t, size, asoc->pathmtu);
+		sp = sctp_sk(asoc->base.sk);
+	}
 
-	if (!size)
-		size = SCTP_DEFAULT_MAXSEGMENT;
+	size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
 
 	return sctp_make_op_error_space(asoc, chunk, size);
 }
@@ -1528,18 +1508,6 @@ void *sctp_addto_chunk(struct sctp_chunk *chunk, int len, const void *data)
 	return target;
 }
 
-/* Append bytes to the end of a chunk. Returns NULL if there isn't sufficient
- * space in the chunk
- */
-static void *sctp_addto_chunk_fixed(struct sctp_chunk *chunk,
-				    int len, const void *data)
-{
-	if (skb_tailroom(chunk->skb) >= len)
-		return sctp_addto_chunk(chunk, len, data);
-	else
-		return NULL;
-}
-
 /* Append bytes from user space to the end of a chunk.  Will panic if
  * chunk is not big enough.
  * Returns a kernel err value.
@@ -1834,6 +1802,9 @@ struct sctp_association *sctp_unpack_cookie(
 		kt = ktime_get_real();
 
 	if (!asoc && ktime_before(bear_cookie->expiration, kt)) {
+		suseconds_t usecs = ktime_to_us(ktime_sub(kt, bear_cookie->expiration));
+		__be32 n = htonl(usecs);
+
 		/*
 		 * Section 3.3.10.3 Stale Cookie Error (3)
 		 *
@@ -1842,17 +1813,12 @@ struct sctp_association *sctp_unpack_cookie(
 		 * Stale Cookie Error:  Indicates the receipt of a valid State
 		 * Cookie that has expired.
 		 */
-		len = ntohs(chunk->chunk_hdr->length);
-		*errp = sctp_make_op_error_space(asoc, chunk, len);
-		if (*errp) {
-			suseconds_t usecs = ktime_to_us(ktime_sub(kt, bear_cookie->expiration));
-			__be32 n = htonl(usecs);
-
-			sctp_init_cause(*errp, SCTP_ERROR_STALE_COOKIE,
-					sizeof(n));
-			sctp_addto_chunk(*errp, sizeof(n), &n);
+		*errp = sctp_make_op_error(asoc, chunk,
+					   SCTP_ERROR_STALE_COOKIE, &n,
+					   sizeof(n), 0);
+		if (*errp)
 			*error = -SCTP_IERROR_STALE_COOKIE;
-		} else
+		else
 			*error = -SCTP_IERROR_NOMEM;
 
 		goto fail;
@@ -2003,12 +1969,8 @@ static int sctp_process_hn_param(const struct sctp_association *asoc,
 	if (*errp)
 		sctp_chunk_free(*errp);
 
-	*errp = sctp_make_op_error_space(asoc, chunk, len);
-
-	if (*errp) {
-		sctp_init_cause(*errp, SCTP_ERROR_DNS_FAILED, len);
-		sctp_addto_chunk(*errp, len, param.v);
-	}
+	*errp = sctp_make_op_error(asoc, chunk, SCTP_ERROR_DNS_FAILED,
+				   param.v, len, 0);
 
 	/* Stop processing this chunk. */
 	return 0;
@@ -2133,23 +2095,23 @@ static enum sctp_ierror sctp_process_unk_param(
 		/* Make an ERROR chunk, preparing enough room for
 		 * returning multiple unknown parameters.
 		 */
-		if (NULL = *errp)
-			*errp = sctp_make_op_error_fixed(asoc, chunk);
-
-		if (*errp) {
-			if (!sctp_init_cause_fixed(*errp, SCTP_ERROR_UNKNOWN_PARAM,
-					SCTP_PAD4(ntohs(param.p->length))))
-				sctp_addto_chunk_fixed(*errp,
-						SCTP_PAD4(ntohs(param.p->length)),
-						param.v);
-		} else {
-			/* If there is no memory for generating the ERROR
-			 * report as specified, an ABORT will be triggered
-			 * to the peer and the association won't be
-			 * established.
-			 */
-			retval = SCTP_IERROR_NOMEM;
+		if (!*errp) {
+			*errp = sctp_make_op_error_limited(asoc, chunk);
+			if (!*errp) {
+				/* If there is no memory for generating the
+				 * ERROR report as specified, an ABORT will be
+				 * triggered to the peer and the association
+				 * won't be established.
+				 */
+				retval = SCTP_IERROR_NOMEM;
+				break;
+			}
 		}
+
+		if (!sctp_init_cause(*errp, SCTP_ERROR_UNKNOWN_PARAM,
+				     ntohs(param.p->length)))
+			sctp_addto_chunk(*errp, ntohs(param.p->length),
+					 param.v);
 		break;
 	default:
 		break;
@@ -2225,10 +2187,10 @@ static enum sctp_ierror sctp_verify_param(struct net *net,
 		 * MUST be aborted.  The ABORT chunk SHOULD contain the error
 		 * cause 'Protocol Violation'.
 		 */
-		if (SCTP_AUTH_RANDOM_LENGTH !-			ntohs(param.p->length) - sizeof(struct sctp_paramhdr)) {
+		if (SCTP_AUTH_RANDOM_LENGTH != ntohs(param.p->length) -
+					       sizeof(struct sctp_paramhdr)) {
 			sctp_process_inv_paramlength(asoc, param.p,
-							chunk, err_chunk);
+						     chunk, err_chunk);
 			retval = SCTP_IERROR_ABORT;
 		}
 		break;
Signature · 1 line
2.14.3
#3 lore raw

Re: [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Thank you for the patch! Yet something to improve:

Hi Marcelo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Marcelo-Ricardo-Leitner/sctp-allow-sctp_init_cause-to-return-errors/20180430-073613
config: i386-randconfig-s1-201817 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce:

        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

net/sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

>> net/sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload' [-Werror=implicit-function-declaration]
     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/sctp_mtu_payload +1260 net/sctp/sm_make_chunk.c

1240
1241 /* Create an Operation Error chunk of a fixed size, specifically,
1242 * min(asoc->pathmtu, SCTP_DEFAULT_MAXSEGMENT) - overheads.
1243 * This is a helper function to allocate an error chunk for for those
1244 * invalid parameter codes in which we may not want to report all the
1245 * errors, if the incoming chunk is large. If it can't fit in a single
1246 * packet, we ignore it.
1247 */
1248 static inline struct sctp_chunk *sctp_make_op_error_limited(
1249 const struct sctp_association *asoc,
1250 const struct sctp_chunk *chunk)
1251 {
1252 size_t size = SCTP_DEFAULT_MAXSEGMENT;
1253 struct sctp_sock *sp = NULL;
1254
1255 if (asoc) {
1256 size = min_t(size_t, size, asoc->pathmtu);
1257 sp = sctp_sk(asoc->base.sk);
1258 }
1259

> 1260		size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));

1261
1262 return sctp_make_op_error_space(asoc, chunk, size);
1263 }
1264

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#4 lore raw

Re: [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

In reply to Marcelo Ricardo Leitner
Thank you for the patch! Yet something to improve:

Hi Marcelo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Marcelo-Ricardo-Leitner/sctp-allow-sctp_init_cause-to-return-errors/20180430-073613
config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

>> net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]
     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

vim +1260 net//sctp/sm_make_chunk.c

1240
1241 /* Create an Operation Error chunk of a fixed size, specifically,
1242 * min(asoc->pathmtu, SCTP_DEFAULT_MAXSEGMENT) - overheads.
1243 * This is a helper function to allocate an error chunk for for those
1244 * invalid parameter codes in which we may not want to report all the
1245 * errors, if the incoming chunk is large. If it can't fit in a single
1246 * packet, we ignore it.
1247 */
1248 static inline struct sctp_chunk *sctp_make_op_error_limited(
1249 const struct sctp_association *asoc,
1250 const struct sctp_chunk *chunk)
1251 {
1252 size_t size = SCTP_DEFAULT_MAXSEGMENT;
1253 struct sctp_sock *sp = NULL;
1254
1255 if (asoc) {
1256 size = min_t(size_t, size, asoc->pathmtu);
1257 sp = sctp_sk(asoc->base.sk);
1258 }
1259

> 1260		size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));

1261
1262 return sctp_make_op_error_space(asoc, chunk, size);
1263 }
1264

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author maintainer
#5 lore raw

Re: [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Quoted 7 lines · kbuild test robot

Hi Marcelo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Marcelo-Ricardo-Leitner/sctp-allow-sctp_init_cause-to-return-errors/20180430-073613

This URL doesn't work, 404.

Quoted 6 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64
Quoted 8 lines

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Seems the test didn't pick up the MTU refactor patchset yet.

$ grep sctp/sctp -- net/sctp/sm_make_chunk.c
#include <net/sctp/sctp.h>
$ git grep sctp_mtu_payload -- include/
include/net/sctp/sctp.h:static inline __u32 sctp_mtu_payload(const
struct sctp_sock *sp,

it should be reachable.

Marcelo

#6 lore raw

Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Sorry for the late response.

Quoted 10 lines · Marcelo Ricardo Leitner
Quoted 7 lines · kbuild test robot

Hi Marcelo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Marcelo-Ricardo-Leitner/sctp-allow-sctp_init_cause-to-return-errors/20180430-073613

This URL doesn't work, 404.

Our fault, 0day service failed to push related changes to github recently, we're fixing it.

Quoted 9 lines
Quoted 9 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

Quoted 8 lines
Quoted 6 lines

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Seems the test didn't pick up the MTU refactor patchset yet.

Do you mean your patchset require MTU refactor patchset as prerequisites?

Thanks,
Xiaolong

Quoted 15 lines
$ grep sctp/sctp -- net/sctp/sm_make_chunk.c
#include <net/sctp/sctp.h>
$ git grep sctp_mtu_payload -- include/
include/net/sctp/sctp.h:static inline __u32 sctp_mtu_payload(const
struct sctp_sock *sp,

it should be reachable.

Marcelo
_______________________________________________
kbuild-all mailing list
[email protected]
https://lists.01.org/mailman/listinfo/kbuild-all

author maintainer
#7 lore raw

Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Quoted 10 lines · Ye Xiaolong
Quoted 10 lines
Quoted 10 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Seems the test didn't pick up the MTU refactor patchset yet.

Do you mean your patchset require MTU refactor patchset as prerequisites?

Yes.

Thanks,
Marcelo

#8 lore raw

Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Quoted 16 lines · Marcelo Ricardo Leitner
Quoted 15 lines · Ye Xiaolong
Quoted 15 lines
Quoted 14 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Seems the test didn't pick up the MTU refactor patchset yet.

Do you mean your patchset require MTU refactor patchset as prerequisites?

Yes.

Then it is recommended to use '--base' option of git format-patch, it would record the base tree info in the first patch or cover letter, 0day bot would apply your patchset to right base according to it.

Thanks,
Xiaolong

Quoted 3 lines

Thanks,
Marcelo

author maintainer
#9 lore raw

Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Quoted 19 lines · Ye Xiaolong
Quoted 18 lines · Marcelo Ricardo Leitner
Quoted 17 lines · Ye Xiaolong
Quoted 16 lines
Quoted 6 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64
Quoted 8 lines

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Seems the test didn't pick up the MTU refactor patchset yet.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Do you mean your patchset require MTU refactor patchset as prerequisites?

Yes.

Then it is recommended to use '--base' option of git format-patch, it would record the base tree info in the first patch or cover letter, 0day bot would apply your patchset to right base according to it.

Nice. I wasn't aware of it. Thanks.

Considering that the MTU refactor patchset was already applied on net-next when the bot did the test, why should I have to specify the base?

Marcelo

#10 lore raw

Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Quoted 28 lines · Marcelo Ricardo Leitner
Quoted 19 lines · Ye Xiaolong
Quoted 18 lines · Marcelo Ricardo Leitner
Quoted 17 lines · Ye Xiaolong
Quoted 16 lines
Quoted 6 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64
Quoted 8 lines

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Seems the test didn't pick up the MTU refactor patchset yet.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Do you mean your patchset require MTU refactor patchset as prerequisites?

Yes.

Then it is recommended to use '--base' option of git format-patch, it would record the base tree info in the first patch or cover letter, 0day bot would apply your patchset to right base according to it.

Quoted 5 lines

Nice. I wasn't aware of it. Thanks.

Considering that the MTU refactor patchset was already applied on net-next when the bot did the test, why should I have to specify the base?

Could you share me the subjects or commits of MTU refactor patcheset, I'll double check what was wrong.

Thanks,
Xiaolong

Quoted 2 lines

Marcelo

author maintainer
#11 lore raw

Re: [kbuild-all] [PATCH net-next 2/2] sctp: add sctp_make_op_error_limited and reuse inner functions

Quoted 29 lines · Ye Xiaolong
Quoted 28 lines · Marcelo Ricardo Leitner
Quoted 26 lines · Ye Xiaolong
Quoted 16 lines · Marcelo Ricardo Leitner
Quoted 15 lines · Ye Xiaolong
Quoted 15 lines
Quoted 14 lines

config: x86_64-randconfig-x006-201817 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

net//sctp/sm_make_chunk.c: In function 'sctp_make_op_error_limited':

Quoted 1 line
Quoted 1 line

net//sctp/sm_make_chunk.c:1260:9: error: implicit declaration of function 'sctp_mtu_payload'; did you mean 'sctp_do_peeloff'? [-Werror=implicit-function-declaration]

     size = sctp_mtu_payload(sp, size, sizeof(struct sctp_errhdr));
            ^~~~~~~~~~~~~~~~
            sctp_do_peeloff

cc1: some warnings being treated as errors

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Seems the test didn't pick up the MTU refactor patchset yet.

Do you mean your patchset require MTU refactor patchset as prerequisites?

Yes.

Then it is recommended to use '--base' option of git format-patch, it would record the base tree info in the first patch or cover letter, 0day bot would apply your patchset to right base according to it.

Quoted 8 lines
Quoted 5 lines

Nice. I wasn't aware of it. Thanks.

Considering that the MTU refactor patchset was already applied on net-next when the bot did the test, why should I have to specify the base?

Could you share me the subjects or commits of MTU refactor patcheset, I'll double check what was wrong.

https://www.mail-archive.com/[email protected]/msg231756.html
this one.

Thanks,
Marcelo

Quoted 10 lines

Thanks,
Xiaolong

Quoted 2 lines

Marcelo

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

maintainer
#12 lore raw
In reply to Marcelo Ricardo Leitner

From: Marcelo Ricardo Leitner <[email protected]>
Date: Sun, 29 Apr 2018 12:56:30 -0300

Quoted 5 lines

These two variants are very close to each other and can be merged to avoid code duplication. That's what this patchset does.

First, we allow sctp_init_cause to return errors, which then allow us to add sctp_make_op_error_limited that handles both situations.

Series applied.

But generally, there are a lot of smtp_init_cause() call sites with non-zero payload length that should start checking the return value now.

author maintainer
#13 lore raw
Quoted 10 lines · David Miller

From: Marcelo Ricardo Leitner <[email protected]>
Date: Sun, 29 Apr 2018 12:56:30 -0300

Quoted 5 lines

These two variants are very close to each other and can be merged to avoid code duplication. That's what this patchset does.

First, we allow sctp_init_cause to return errors, which then allow us to add sctp_make_op_error_limited that handles both situations.

Series applied.

Thanks.

Quoted 3 lines

But generally, there are a lot of smtp_init_cause() call sites with non-zero payload length that should start checking the return value now.

They are safe as is, because they follow the pattern:
- sctp_make_abort(...., somesize)
- sctp_init_cause(size)
where size is considered in somesize, so sctp_init_cause cannot fail in there.

This new usage in sctp_make_op_error_limited is the only one where it allocates a buffer without knowing how much data will actually be pushed into it.

Marcelo