Re: [PATCH net-next 1/2] sctp: add fair capacity stream scheduler

On Thu, Mar 9, 2023 at 5:31 AM Paolo Abeni [off-list ref] wrote:

On Tue, 2023-03-07 at 16:23 -0500, Xin Long wrote:

quoted

diff --git a/net/sctp/stream_sched_fc.c b/net/sctp/stream_sched_fc.c
new file mode 100644
index 000000000000..b336c2f5486b
--- /dev/null
+++ b/net/sctp/stream_sched_fc.c

@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* SCTP kernel implementation
+ * (C) Copyright Red Hat Inc. 2022
+ *
+ * This file is part of the SCTP kernel implementation
+ *
+ * These functions manipulate sctp stream queue/scheduling.
+ *
+ * Please send any bug reports or fixes you make to the
+ * email addresched(es):
+ *    lksctp developers <linux-sctp@vger.kernel.org>
+ *
+ * Written or modified by:
+ *    Xin Long <lucien.xin@gmail.com>
+ */
+
+#include <linux/list.h>
+#include <net/sctp/sctp.h>

Note that the above introduces a new compile warning:

../net/sctp/stream_sched_fc.c: note: in included file (through ../include/net/sctp/sctp.h):
../include/net/sctp/structs.h:335:41: warning: array of flexible structures

that is not really a fault of the new code, it's due to:

        struct sctp_init_chunk peer_init[];

struct sctp_init_chunk {
        struct sctp_chunkhdr chunk_hdr;
        struct sctp_inithdr init_hdr;
};

struct sctp_inithdr {
        __be32 init_tag;
        __be32 a_rwnd;
        __be16 num_outbound_streams;
        __be16 num_inbound_streams;
        __be32 initial_tsn;
        __u8  params[]; // <- this!
};

Any chance a future patch could remove the 'params' field from the
struct included by sctp_init_chunk?

e.g.

struct sctp_inithdr_base {
        __be32 init_tag;
        __be32 a_rwnd;
        __be16 num_outbound_streams;
        __be16 num_inbound_streams;
        __be32 initial_tsn;
};

struct sctp_init_chunk {
        struct sctp_chunkhdr chunk_hdr;
        struct sctp_inithdr_base init_hdr;
};

and then cast 'init_hdr' to 'struct sctp_inithdr' if/where needed.

In any case, the above is not blocking this series.

Hi, Paolo,

There are actually quite some warnings like this in SCTP:

# make C=1 CF="-Wflexible-array-nested" M=./net/sctp/ 2> /tmp/warnings
# grep "nested flexible array" /tmp/warnings |grep sctp |sort |uniq

./include/linux/sctp.h:230:29: warning: nested flexible array
./include/linux/sctp.h:278:29: warning: nested flexible array
./include/linux/sctp.h:348:29: warning: nested flexible array
./include/linux/sctp.h:393:29: warning: nested flexible array
./include/linux/sctp.h:451:28: warning: nested flexible array
./include/linux/sctp.h:611:32: warning: nested flexible array
./include/linux/sctp.h:628:33: warning: nested flexible array
./include/linux/sctp.h:675:30: warning: nested flexible array
./include/linux/sctp.h:735:29: warning: nested flexible array
./include/net/sctp/structs.h:1145:41: warning: nested flexible array
./include/net/sctp/structs.h:1588:28: warning: nested flexible array
./include/net/sctp/structs.h:343:28: warning: nested flexible array
./include/uapi/linux/sctp.h:641:34: warning: nested flexible array
./include/uapi/linux/sctp.h:643:34: warning: nested flexible array
./include/uapi/linux/sctp.h:644:33: warning: nested flexible array
./include/uapi/linux/sctp.h:650:40: warning: nested flexible array
./include/uapi/linux/sctp.h:653:39: warning: nested flexible array

Other than the uapis, I can try to give others a cleanup by deleting
these flexible array members and casting it by (struct xxx *) (hdr + 1)
when accessing it.

This warning is reported if a structure having a flexible array member is
included by other structures, and I also noticed there is the same problem
on some core structures like:

struct ip_options
struct nft_set_ext

which are included in many other structures, and can also cause such warnings.
I guess we'll just leave it as it is, right?

Thanks.