RE: [RFC 0/2] Eliminate zero length arrays in DPDK

RE: [RFC 0/2] Eliminate zero length arrays in DPDK

[Morten Brørup]

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

My answer is at the end.Sent from a smartphone. Please pardon brevity and spelling.
-------- Oprindelig besked --------Fra: Bruce Richardson <bruce.richardson@intel.com> Dato: 16/02/2022  11.15  (GMT+01:00) Til: Morten Brørup <mb@smartsharesystems.com> Cc: Stephen Hemminger <stephen@networkplumber.org>, dev@dpdk.org Emne: Re: [RFC 0/2] Eliminate zero length arrays in DPDK On Wed, Feb 16, 2022 at 11:05:09AM +0100, Morten Brørup wrote:> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]> > Sent: Wednesday, 16 February 2022 10.33> > > > On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:> > > Yet another case of applying Linux kernel best practices> > > to DPDK. Flexible arrays are supported by Clang, GCC and> > > Microsoft compilers (part of C99).> > >> > Do we need to start explicitly stating that DPDK uses C99 features, and> > adding -std=c99 to our build flags? Are we also requiring that> > applications> > are compiled with c99 features to use this (I would hope that they are,> > but> > I'm not sure we can mandate it).> > No to -std=c99. It's >= C99 for applications; we should not prevent them from using a newer C standard.Yes. For build flags, I was referring only to having it in the cflags for thebuild of DPDK itself, not for apps. We definitely need to minimise thebuild flags we expose to apps.> > Adding a note about the C standard version to the DPDK requirements> documentation would be very nice. It only mentions a certain compiler> version required. But I think that documenting the detailed build and> runtime requirements (and why they are that way) is another task.> Sure, we should do that. I am just wanting to be sure that if we specify aminimum of C99, we won't get complaints back from those with legacycodebasees which only support C89/C90. I am therefore wondering if we needto have our public headers C90-compliant?/BruceWe are publicly using C11 for atomics [1]. I'm not sure if that also implies that we are requiring C11 generally.Otherwise,  I agree with your concerns about old code bases.[1]https://www.dpdk.org/blog/2021/03/26/dpdk-adopts-the-c11-memory-model/-Morten

[-- Attachment #2: Type: text/html, Size: 3574 bytes --]

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Tyler Retzlaff]

On Wed, Feb 16, 2022 at 11:39:30AM +0100, Morten Brørup wrote:
> My answer is at the end.Sent from a smartphone. Please pardon brevity and spelling.

> I am therefore wondering if we needto have our public headers C90-compliant?/BruceWe are publicly using C11 for atomics [1]. I'm not sure if that also implies that we are requiring C11 generally. Otherwise,  I agree with your concerns about old code bases. [1]https://www.dpdk.org/blog/2021/03/26/dpdk-adopts-the-c11-memory-model

we aren't actually using C11 atomics. we are using gcc builtin atomics
that follow the C11 memory model for accessing atomics.

so strictly speaking we don't need to make the minimum a C11 compiler.

i suspect a lot of pushback would occur if we made C11 a minimum.  so
for atomics i would make the suggestion that we abstract atomics to
permit C11 atomics to be used when a C11 compiler with stdatomic is
available.

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Stephen Hemminger]

On Thu, 24 Feb 2022 22:51:31 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 17 February 2022 08.42
> > 
> > On Wed, Feb 16, 2022 at 10:10:01AM +0000, Bruce Richardson wrote:  
> > > On Wed, Feb 16, 2022 at 11:05:09AM +0100, Morten Brørup wrote:  
> > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > > Sent: Wednesday, 16 February 2022 10.33
> > > > >
> > > > > On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger  
> > wrote:  
> > > > > > Yet another case of applying Linux kernel best practices
> > > > > > to DPDK. Flexible arrays are supported by Clang, GCC and
> > > > > > Microsoft compilers (part of C99).
> > > > > >  
> > > > > Do we need to start explicitly stating that DPDK uses C99  
> > features, and  
> > > > > adding -std=c99 to our build flags? Are we also requiring that
> > > > > applications
> > > > > are compiled with c99 features to use this (I would hope that  
> > they are,  
> > > > > but
> > > > > I'm not sure we can mandate it).  
> > > >
> > > > No to -std=c99. It's >= C99 for applications; we should not prevent  
> > them from using a newer C standard.  
> > >
> > > Yes. For build flags, I was referring only to having it in the cflags  
> > for the  
> > > build of DPDK itself, not for apps. We definitely need to minimise  
> > the  
> > > build flags we expose to apps.
> > >  
> > > >
> > > > Adding a note about the C standard version to the DPDK requirements
> > > > documentation would be very nice. It only mentions a certain  
> > compiler  
> > > > version required. But I think that documenting the detailed build  
> > and  
> > > > runtime requirements (and why they are that way) is another task.
> > > >  
> > > Sure, we should do that. I am just wanting to be sure that if we  
> > specify a  
> > > minimum of C99, we won't get complaints back from those with legacy
> > > codebasees which only support C89/C90. I am therefore wondering if we  
> > need  
> > > to have our public headers C90-compliant?  
> > 
> > this seems to be the real question. what "minimum" C standard should be
> > documented as required to consume dpdk. we can obviously use any
> > standard
> > we wish to build/provide binaries. similarly we ought to document a
> > minimum C++ standard for consumption.
> > 
> > i would advocate for C99 however before setting that in stone it is
> > fair
> > to ask if there are any consumers using < C99.
> > 
> > we may also want to consider that the minimum required may differ
> > depending on the platform/port. though most differences in public
> > interface
> > i would hope could be trivially abstracted though.
> > 
> > ty  
> 
> Just read that the Linux kernel is moving towards C11, or at minimum C99, for version 5.18:
> https://lwn.net/SubscriberLink/885941/01fdc39df2ecc25f/
> 
> Let's be bold and push for the same for DPDK! :-)

Would be good, but still getting held back by legacy distros (RHEL)
and other compiler environments ICC, etc.

RE: [RFC 0/2] Eliminate zero length arrays in DPDK

[Morten Brørup]

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 17 February 2022 08.42
> 
> On Wed, Feb 16, 2022 at 10:10:01AM +0000, Bruce Richardson wrote:
> > On Wed, Feb 16, 2022 at 11:05:09AM +0100, Morten Brørup wrote:
> > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > Sent: Wednesday, 16 February 2022 10.33
> > > >
> > > > On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger
> wrote:
> > > > > Yet another case of applying Linux kernel best practices
> > > > > to DPDK. Flexible arrays are supported by Clang, GCC and
> > > > > Microsoft compilers (part of C99).
> > > > >
> > > > Do we need to start explicitly stating that DPDK uses C99
> features, and
> > > > adding -std=c99 to our build flags? Are we also requiring that
> > > > applications
> > > > are compiled with c99 features to use this (I would hope that
> they are,
> > > > but
> > > > I'm not sure we can mandate it).
> > >
> > > No to -std=c99. It's >= C99 for applications; we should not prevent
> them from using a newer C standard.
> >
> > Yes. For build flags, I was referring only to having it in the cflags
> for the
> > build of DPDK itself, not for apps. We definitely need to minimise
> the
> > build flags we expose to apps.
> >
> > >
> > > Adding a note about the C standard version to the DPDK requirements
> > > documentation would be very nice. It only mentions a certain
> compiler
> > > version required. But I think that documenting the detailed build
> and
> > > runtime requirements (and why they are that way) is another task.
> > >
> > Sure, we should do that. I am just wanting to be sure that if we
> specify a
> > minimum of C99, we won't get complaints back from those with legacy
> > codebasees which only support C89/C90. I am therefore wondering if we
> need
> > to have our public headers C90-compliant?
> 
> this seems to be the real question. what "minimum" C standard should be
> documented as required to consume dpdk. we can obviously use any
> standard
> we wish to build/provide binaries. similarly we ought to document a
> minimum C++ standard for consumption.
> 
> i would advocate for C99 however before setting that in stone it is
> fair
> to ask if there are any consumers using < C99.
> 
> we may also want to consider that the minimum required may differ
> depending on the platform/port. though most differences in public
> interface
> i would hope could be trivially abstracted though.
> 
> ty

Just read that the Linux kernel is moving towards C11, or at minimum C99, for version 5.18:
https://lwn.net/SubscriberLink/885941/01fdc39df2ecc25f/

Let's be bold and push for the same for DPDK! :-)

-Morten

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Tyler Retzlaff]

On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:
> Yet another case of applying Linux kernel best practices
> to DPDK. Flexible arrays are supported by Clang, GCC and
> Microsoft compilers (part of C99).

Series-Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

subject to the outcome of documenting C99 compiler as a minimum
requirement.

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Tyler Retzlaff]

On Wed, Feb 16, 2022 at 10:10:01AM +0000, Bruce Richardson wrote:
> On Wed, Feb 16, 2022 at 11:05:09AM +0100, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Wednesday, 16 February 2022 10.33
> > > 
> > > On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:
> > > > Yet another case of applying Linux kernel best practices
> > > > to DPDK. Flexible arrays are supported by Clang, GCC and
> > > > Microsoft compilers (part of C99).
> > > >
> > > Do we need to start explicitly stating that DPDK uses C99 features, and
> > > adding -std=c99 to our build flags? Are we also requiring that
> > > applications
> > > are compiled with c99 features to use this (I would hope that they are,
> > > but
> > > I'm not sure we can mandate it).
> > 
> > No to -std=c99. It's >= C99 for applications; we should not prevent them from using a newer C standard.
> 
> Yes. For build flags, I was referring only to having it in the cflags for the
> build of DPDK itself, not for apps. We definitely need to minimise the
> build flags we expose to apps.
> 
> > 
> > Adding a note about the C standard version to the DPDK requirements
> > documentation would be very nice. It only mentions a certain compiler
> > version required. But I think that documenting the detailed build and
> > runtime requirements (and why they are that way) is another task.
> > 
> Sure, we should do that. I am just wanting to be sure that if we specify a
> minimum of C99, we won't get complaints back from those with legacy
> codebasees which only support C89/C90. I am therefore wondering if we need
> to have our public headers C90-compliant?

this seems to be the real question. what "minimum" C standard should be
documented as required to consume dpdk. we can obviously use any standard
we wish to build/provide binaries. similarly we ought to document a
minimum C++ standard for consumption.

i would advocate for C99 however before setting that in stone it is fair
to ask if there are any consumers using < C99.

we may also want to consider that the minimum required may differ
depending on the platform/port. though most differences in public interface
i would hope could be trivially abstracted though.

ty

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Stephen Hemminger]

On Wed, 16 Feb 2022 09:33:02 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:
> > Yet another case of applying Linux kernel best practices
> > to DPDK. Flexible arrays are supported by Clang, GCC and
> > Microsoft compilers (part of C99).
> >  
> Do we need to start explicitly stating that DPDK uses C99 features, and
> adding -std=c99 to our build flags? Are we also requiring that applications
> are compiled with c99 features to use this (I would hope that they are, but
> I'm not sure we can mandate it).

There are already many uses of flex array in DPDK. This is nothing new.

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Bruce Richardson]

On Wed, Feb 16, 2022 at 11:05:09AM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Wednesday, 16 February 2022 10.33
> > 
> > On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:
> > > Yet another case of applying Linux kernel best practices
> > > to DPDK. Flexible arrays are supported by Clang, GCC and
> > > Microsoft compilers (part of C99).
> > >
> > Do we need to start explicitly stating that DPDK uses C99 features, and
> > adding -std=c99 to our build flags? Are we also requiring that
> > applications
> > are compiled with c99 features to use this (I would hope that they are,
> > but
> > I'm not sure we can mandate it).
> 
> No to -std=c99. It's >= C99 for applications; we should not prevent them from using a newer C standard.

Yes. For build flags, I was referring only to having it in the cflags for the
build of DPDK itself, not for apps. We definitely need to minimise the
build flags we expose to apps.

> 
> Adding a note about the C standard version to the DPDK requirements
> documentation would be very nice. It only mentions a certain compiler
> version required. But I think that documenting the detailed build and
> runtime requirements (and why they are that way) is another task.
> 
Sure, we should do that. I am just wanting to be sure that if we specify a
minimum of C99, we won't get complaints back from those with legacy
codebasees which only support C89/C90. I am therefore wondering if we need
to have our public headers C90-compliant?

/Bruce

RE: [RFC 0/2] Eliminate zero length arrays in DPDK

[Morten Brørup]

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Wednesday, 16 February 2022 10.33
> 
> On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:
> > Yet another case of applying Linux kernel best practices
> > to DPDK. Flexible arrays are supported by Clang, GCC and
> > Microsoft compilers (part of C99).
> >
> Do we need to start explicitly stating that DPDK uses C99 features, and
> adding -std=c99 to our build flags? Are we also requiring that
> applications
> are compiled with c99 features to use this (I would hope that they are,
> but
> I'm not sure we can mandate it).

No to -std=c99. It's >= C99 for applications; we should not prevent them from using a newer C standard.

Adding a note about the C standard version to the DPDK requirements documentation would be very nice. It only mentions a certain compiler version required. But I think that documenting the detailed build and runtime requirements (and why they are that way) is another task.

Re: [RFC 0/2] Eliminate zero length arrays in DPDK

[Bruce Richardson]

On Tue, Feb 15, 2022 at 03:00:56PM -0800, Stephen Hemminger wrote:
> Yet another case of applying Linux kernel best practices
> to DPDK. Flexible arrays are supported by Clang, GCC and
> Microsoft compilers (part of C99).
>
Do we need to start explicitly stating that DPDK uses C99 features, and
adding -std=c99 to our build flags? Are we also requiring that applications
are compiled with c99 features to use this (I would hope that they are, but
I'm not sure we can mandate it).

RE: [RFC 0/2] Eliminate zero length arrays in DPDK

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 16 February 2022 00.01
> 
> Yet another case of applying Linux kernel best practices
> to DPDK. Flexible arrays are supported by Clang, GCC and
> Microsoft compilers (part of C99).
> 
> Stephen Hemminger (2):
>   devtools: add script to check for zero length array
>   treewide: replace zero-length array with flex array
> 
>  app/test/test_table_tables.c                  |  2 +-
>  devtools/cocci/zerolengtharray.cocci          | 17 +++++++++++++++++
>  drivers/bus/dpaa/include/netcfg.h             |  4 ++--
>  drivers/common/cnxk/roc_se.h                  |  2 +-
>  drivers/common/dpaax/caamflib/desc/ipsec.h    |  2 +-
>  drivers/common/dpaax/dpaax_iova_table.h       |  2 +-
>  drivers/common/mlx5/mlx5_prm.h                |  8 ++++----
>  drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h     |  2 +-
>  drivers/crypto/ipsec_mb/ipsec_mb_private.h    |  4 ++--
>  drivers/crypto/virtio/virtio_ring.h           |  4 ++--
>  drivers/crypto/virtio/virtqueue.h             |  2 +-
>  drivers/net/cxgbe/clip_tbl.h                  |  2 +-
>  drivers/net/cxgbe/l2t.h                       |  2 +-
>  drivers/net/cxgbe/mps_tcam.h                  |  2 +-
>  drivers/net/cxgbe/smt.h                       |  2 +-
>  drivers/net/enic/base/vnic_devcmd.h           |  4 ++--
>  drivers/net/hinic/hinic_pmd_tx.h              |  2 +-
>  drivers/net/nfp/nfpcore/nfp_nsp.h             |  2 +-
>  drivers/net/virtio/virtio_ring.h              |  4 ++--
>  drivers/net/virtio/virtio_user/vhost_kernel.c |  2 +-
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   |  2 +-
>  drivers/net/virtio/virtqueue.h                |  2 +-
>  drivers/regex/mlx5/mlx5_rxp.h                 |  2 +-
>  examples/ip_reassembly/main.c                 |  2 +-
>  lib/cryptodev/cryptodev_pmd.h                 |  2 +-
>  lib/cryptodev/rte_cryptodev.h                 |  2 +-
>  lib/ip_frag/ip_reassembly.h                   |  2 +-
>  lib/ipsec/sa.h                                |  2 +-
>  lib/rib/rte_rib.c                             |  2 +-
>  lib/rib/rte_rib6.c                            |  2 +-
>  lib/table/rte_swx_table_learner.c             |  2 +-
>  lib/table/rte_table_hash_key16.c              |  4 ++--
>  lib/table/rte_table_hash_key32.c              |  4 ++--
>  lib/table/rte_table_hash_key8.c               |  4 ++--
>  lib/vhost/rte_vhost.h                         |  4 ++--
>  35 files changed, 63 insertions(+), 46 deletions(-)
>  create mode 100644 devtools/cocci/zerolengtharray.cocci
> 
> --
> 2.34.1

Thank you! We must all appreciate the DPDK housekeeping efforts, where it DPDK gets some real love, and not just added support for new bells and whistles in fancy NICs. :-)

Series-Acked-By: Morten Brørup <mb@smartsharesystems.com>

[RFC 0/2] Eliminate zero length arrays in DPDK

[Stephen Hemminger]

Yet another case of applying Linux kernel best practices
to DPDK. Flexible arrays are supported by Clang, GCC and
Microsoft compilers (part of C99).

Stephen Hemminger (2):
  devtools: add script to check for zero length array
  treewide: replace zero-length array with flex array

 app/test/test_table_tables.c                  |  2 +-
 devtools/cocci/zerolengtharray.cocci          | 17 +++++++++++++++++
 drivers/bus/dpaa/include/netcfg.h             |  4 ++--
 drivers/common/cnxk/roc_se.h                  |  2 +-
 drivers/common/dpaax/caamflib/desc/ipsec.h    |  2 +-
 drivers/common/dpaax/dpaax_iova_table.h       |  2 +-
 drivers/common/mlx5/mlx5_prm.h                |  8 ++++----
 drivers/crypto/dpaa2_sec/dpaa2_sec_priv.h     |  2 +-
 drivers/crypto/ipsec_mb/ipsec_mb_private.h    |  4 ++--
 drivers/crypto/virtio/virtio_ring.h           |  4 ++--
 drivers/crypto/virtio/virtqueue.h             |  2 +-
 drivers/net/cxgbe/clip_tbl.h                  |  2 +-
 drivers/net/cxgbe/l2t.h                       |  2 +-
 drivers/net/cxgbe/mps_tcam.h                  |  2 +-
 drivers/net/cxgbe/smt.h                       |  2 +-
 drivers/net/enic/base/vnic_devcmd.h           |  4 ++--
 drivers/net/hinic/hinic_pmd_tx.h              |  2 +-
 drivers/net/nfp/nfpcore/nfp_nsp.h             |  2 +-
 drivers/net/virtio/virtio_ring.h              |  4 ++--
 drivers/net/virtio/virtio_user/vhost_kernel.c |  2 +-
 drivers/net/virtio/virtio_user/vhost_vdpa.c   |  2 +-
 drivers/net/virtio/virtqueue.h                |  2 +-
 drivers/regex/mlx5/mlx5_rxp.h                 |  2 +-
 examples/ip_reassembly/main.c                 |  2 +-
 lib/cryptodev/cryptodev_pmd.h                 |  2 +-
 lib/cryptodev/rte_cryptodev.h                 |  2 +-
 lib/ip_frag/ip_reassembly.h                   |  2 +-
 lib/ipsec/sa.h                                |  2 +-
 lib/rib/rte_rib.c                             |  2 +-
 lib/rib/rte_rib6.c                            |  2 +-
 lib/table/rte_swx_table_learner.c             |  2 +-
 lib/table/rte_table_hash_key16.c              |  4 ++--
 lib/table/rte_table_hash_key32.c              |  4 ++--
 lib/table/rte_table_hash_key8.c               |  4 ++--
 lib/vhost/rte_vhost.h                         |  4 ++--
 35 files changed, 63 insertions(+), 46 deletions(-)
 create mode 100644 devtools/cocci/zerolengtharray.cocci

-- 
2.34.1