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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 9, 2023 at 5:31 AM Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
>
> On Tue, 2023-03-07 at 16:23 -0500, Xin Long wrote:
> > 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@xxxxxxxxxxxxxxx>
> > + *
> > + * Written or modified by:
> > + *    Xin Long <lucien.xin@xxxxxxxxx>
> > + */
> > +
> > +#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.




[Index of Archives]     [Linux Networking Development]     [Linux OMAP]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     SCTP

  Powered by Linux