[PATCH net-next 0/2] sctp: unify sctp_make_op_error_fixed and sctp_make_op_error_space

[PATCH net-next 0/2] sctp: unify sctp_make_op_error_fixed and sctp_make_op_error_space

[Marcelo Ricardo Leitner]

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(-)

[PATCH net-next 1/2] sctp: allow sctp_init_cause to return errors

[Marcelo Ricardo Leitner]

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

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 include/net/sctp/sm.h    |  2 +-
 net/sctp/sm_make_chunk.c | 12 +++++++++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index f4b657478a304050851f33d92c71162a4a4a2e50..5ef1bad81ef54906b375dbfd0e4fd897d078abce 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -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);
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index db93eabd6ef500ab20be6e091ee06bd3b60923c9..e518eb64ccf3578f7892da050c160a56cf3cc833 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -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
-- 
2.14.3

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

[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.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/sm_make_chunk.c | 130 +++++++++++++++++------------------------------
 1 file changed, 46 insertions(+), 84 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index e518eb64ccf3578f7892da050c160a56cf3cc833..4d7b3ccea0789f3a695f710046b50855e4cc41fc 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -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;
-- 
2.14.3

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28784 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2055 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29738 bytes --]

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

[Marcelo Ricardo Leitner]

On Mon, Apr 30, 2018 at 10:14:06AM +0800, kbuild test robot wrote:
> 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.

> 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

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

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

[Ye Xiaolong]

Sorry for the late response.


On 04/29, Marcelo Ricardo Leitner wrote:
>On Mon, Apr 30, 2018 at 10:14:06AM +0800, kbuild test robot wrote:
>> 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.

>
>> 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
>
>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

>
>$ 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
>kbuild-all@lists.01.org
>https://lists.01.org/mailman/listinfo/kbuild-all

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

[Marcelo Ricardo Leitner]

On Mon, May 14, 2018 at 03:40:53PM +0800, Ye Xiaolong wrote:
> >> 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
> >
> >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

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

[Ye Xiaolong]

On 05/14, Marcelo Ricardo Leitner wrote:
>On Mon, May 14, 2018 at 03:40:53PM +0800, Ye Xiaolong wrote:
>> >> 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
>> >
>> >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
>
>Thanks,
>Marcelo

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

[Marcelo Ricardo Leitner]

On Mon, May 14, 2018 at 07:47:20PM +0800, Ye Xiaolong wrote:
> On 05/14, Marcelo Ricardo Leitner wrote:
> >On Mon, May 14, 2018 at 03:40:53PM +0800, Ye Xiaolong wrote:
> >> >> 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
> >> >
> >> >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.

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

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

[Ye Xiaolong]

On 05/14, Marcelo Ricardo Leitner wrote:
>On Mon, May 14, 2018 at 07:47:20PM +0800, Ye Xiaolong wrote:
>> On 05/14, Marcelo Ricardo Leitner wrote:
>> >On Mon, May 14, 2018 at 03:40:53PM +0800, Ye Xiaolong wrote:
>> >> >> 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
>> >> >
>> >> >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.
>
>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
>
>  Marcelo

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

[Marcelo Ricardo Leitner]

On Tue, May 15, 2018 at 09:23:08AM +0800, Ye Xiaolong wrote:
> On 05/14, Marcelo Ricardo Leitner wrote:
> >On Mon, May 14, 2018 at 07:47:20PM +0800, Ye Xiaolong wrote:
> >> On 05/14, Marcelo Ricardo Leitner wrote:
> >> >On Mon, May 14, 2018 at 03:40:53PM +0800, Ye Xiaolong wrote:
> >> >> >> 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
> >> >> >
> >> >> >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.
> >
> >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/netdev@vger.kernel.org/msg231756.html
this one.

Thanks,
Marcelo

> 
> Thanks,
> Xiaolong
> >
> >  Marcelo
> --
> 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/2] sctp: unify sctp_make_op_error_fixed and sctp_make_op_error_space

[David Miller]

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Sun, 29 Apr 2018 12:56:30 -0300

> 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.

Re: [PATCH net-next 0/2] sctp: unify sctp_make_op_error_fixed and sctp_make_op_error_space

[Marcelo Ricardo Leitner]

On Tue, May 01, 2018 at 12:13:53PM -0400, David Miller wrote:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Date: Sun, 29 Apr 2018 12:56:30 -0300
>
> > 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.

>
> 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