[dpdk-dev] [PATCH 1/2] config: remove explicit undefinition of unset values

[dpdk-dev] [PATCH 1/2] config: remove explicit undefinition of unset values

[Bruce Richardson]

Rather than explicitly clearing any setting of undefined values in our
rte_config.h file, it's better to instead just add a comment that the value
is not set. Using a comment allows the user to set the value using CFLAGS
or similar mechanism without the config file clearing the value again.

The text used "<VALUE> is not set" is modelled after the kernel approach of
doing the same thing.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/rte_config.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index 9bb915347c..1c5a86d6a7 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -85,17 +85,17 @@
 
 /* ip_fragmentation defines */
 #define RTE_LIBRTE_IP_FRAG_MAX_FRAG 4
-#undef RTE_LIBRTE_IP_FRAG_TBL_STAT
+// RTE_LIBRTE_IP_FRAG_TBL_STAT is not set
 
 /* rte_power defines */
 #define RTE_MAX_LCORE_FREQS 64
 
 /* rte_sched defines */
-#undef RTE_SCHED_RED
-#undef RTE_SCHED_COLLECT_STATS
-#undef RTE_SCHED_SUBPORT_TC_OV
+// RTE_SCHED_RED is not set
+// RTE_SCHED_COLLECT_STATS is not set
+// RTE_SCHED_SUBPORT_TC_OV is not set
 #define RTE_SCHED_PORT_N_GRINDERS 8
-#undef RTE_SCHED_VECTOR
+// RTE_SCHED_VECTOR is not set
 
 /* KNI defines */
 #define RTE_KNI_PREEMPT_DEFAULT 1
@@ -123,7 +123,7 @@
 
 /* i40e defines */
 #define RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC 1
-#undef RTE_LIBRTE_I40E_16BYTE_RX_DESC
+// RTE_LIBRTE_I40E_16BYTE_RX_DESC is not set
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF 64
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF 4
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM 4
-- 
2.25.1

[dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults

[Bruce Richardson]

In case a developer uses CFLAGS to set different default values for the
defines in the rte_config.h file, use #ifndef / #endif guards around the
setting of those values. For those lines just "defining" a macro without
assigning it a value to be used by code, drop the value argument (where
possible) to make it clearer that that is what is happening, since those
don't need the #ifdef guard.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/rte_config.h | 110 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 99 insertions(+), 11 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index 1c5a86d6a7..f39da76c13 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2017 Intel Corporation
+ * Copyright(c) 2017-2020 Intel Corporation
  */
 
 /**
@@ -20,10 +20,10 @@
 
 /* legacy defines */
 #ifdef RTE_EXEC_ENV_LINUX
-#define RTE_EXEC_ENV_LINUXAPP 1
+#define RTE_EXEC_ENV_LINUXAPP
 #endif
 #ifdef RTE_EXEC_ENV_FREEBSD
-#define RTE_EXEC_ENV_BSDAPP 1
+#define RTE_EXEC_ENV_BSDAPP
 #endif
 
 /* String that appears before the version number */
@@ -32,107 +32,195 @@
 /****** library defines ********/
 
 /* EAL defines */
+#define RTE_BACKTRACE
+#ifndef RTE_MAX_HEAPS
 #define RTE_MAX_HEAPS 32
+#endif
+#ifndef RTE_MAX_MEMSEG_LISTS
 #define RTE_MAX_MEMSEG_LISTS 128
+#endif
+#ifndef RTE_MAX_MEMSEG_PER_LIST
 #define RTE_MAX_MEMSEG_PER_LIST 8192
+#endif
+#ifndef RTE_MAX_MEM_MB_PER_LIST
 #define RTE_MAX_MEM_MB_PER_LIST 32768
+#endif
+#ifndef RTE_MAX_MEMSEG_PER_TYPE
 #define RTE_MAX_MEMSEG_PER_TYPE 32768
+#endif
+#ifndef RTE_MAX_MEM_MB_PER_TYPE
 #define RTE_MAX_MEM_MB_PER_TYPE 65536
+#endif
+#ifndef RTE_MAX_MEMZONE
 #define RTE_MAX_MEMZONE 2560
+#endif
+#ifndef RTE_MAX_TAILQ
 #define RTE_MAX_TAILQ 32
+#endif
+#ifndef RTE_LOG_DP_LEVEL
 #define RTE_LOG_DP_LEVEL RTE_LOG_INFO
-#define RTE_BACKTRACE 1
+#endif
+#ifndef RTE_MAX_VFIO_CONTAINERS
 #define RTE_MAX_VFIO_CONTAINERS 64
+#endif
 
 /* bsd module defines */
+#ifndef RTE_CONTIGMEM_MAX_NUM_BUFS
 #define RTE_CONTIGMEM_MAX_NUM_BUFS 64
+#endif
+#ifndef RTE_CONTIGMEM_DEFAULT_NUM_BUFS
 #define RTE_CONTIGMEM_DEFAULT_NUM_BUFS 1
+#endif
+#ifndef RTE_CONTIGMEM_DEFAULT_BUF_SIZE
 #define RTE_CONTIGMEM_DEFAULT_BUF_SIZE (512*1024*1024)
+#endif
 
 /* mempool defines */
+#ifndef RTE_MEMPOOL_CACHE_MAX_SIZE
 #define RTE_MEMPOOL_CACHE_MAX_SIZE 512
+#endif
 
 /* mbuf defines */
+#define RTE_MBUF_REFCNT_ATOMIC
+#ifndef RTE_MBUF_DEFAULT_MEMPOOL_OPS
 #define RTE_MBUF_DEFAULT_MEMPOOL_OPS "ring_mp_mc"
-#define RTE_MBUF_REFCNT_ATOMIC 1
+#endif
+#ifndef RTE_PKTMBUF_HEADROOM
 #define RTE_PKTMBUF_HEADROOM 128
+#endif
 
 /* ether defines */
+#define RTE_ETHDEV_RXTX_CALLBACKS
+#ifndef RTE_MAX_QUEUES_PER_PORT
 #define RTE_MAX_QUEUES_PER_PORT 1024
+#endif
+#ifndef RTE_ETHDEV_QUEUE_STAT_CNTRS
 #define RTE_ETHDEV_QUEUE_STAT_CNTRS 16
-#define RTE_ETHDEV_RXTX_CALLBACKS 1
+#endif
 
 /* cryptodev defines */
+#ifndef RTE_CRYPTO_MAX_DEVS
 #define RTE_CRYPTO_MAX_DEVS 64
+#endif
+#ifndef RTE_CRYPTODEV_NAME_LEN
 #define RTE_CRYPTODEV_NAME_LEN 64
+#endif
 
 /* compressdev defines */
+#ifndef RTE_COMPRESS_MAX_DEVS
 #define RTE_COMPRESS_MAX_DEVS 64
+#endif
 
 /* regexdev defines */
+#ifndef RTE_MAX_REGEXDEV_DEVS
 #define RTE_MAX_REGEXDEV_DEVS 32
+#endif
 
 /* eventdev defines */
+#ifndef RTE_EVENT_MAX_DEVS
 #define RTE_EVENT_MAX_DEVS 16
+#endif
+#ifndef RTE_EVENT_MAX_QUEUES_PER_DEV
 #define RTE_EVENT_MAX_QUEUES_PER_DEV 64
+#endif
+#ifndef RTE_EVENT_TIMER_ADAPTER_NUM_MAX
 #define RTE_EVENT_TIMER_ADAPTER_NUM_MAX 32
+#endif
+#ifndef RTE_EVENT_ETH_INTR_RING_SIZE
 #define RTE_EVENT_ETH_INTR_RING_SIZE 1024
+#endif
+#ifndef RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE
 #define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
+#endif
+#ifndef RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE
 #define RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
+#endif
 
 /* rawdev defines */
+#ifndef RTE_RAWDEV_MAX_DEVS
 #define RTE_RAWDEV_MAX_DEVS 64
+#endif
 
 /* ip_fragmentation defines */
+#ifndef RTE_LIBRTE_IP_FRAG_MAX_FRAG
 #define RTE_LIBRTE_IP_FRAG_MAX_FRAG 4
+#endif
 // RTE_LIBRTE_IP_FRAG_TBL_STAT is not set
 
 /* rte_power defines */
+#ifndef RTE_MAX_LCORE_FREQS
 #define RTE_MAX_LCORE_FREQS 64
+#endif
 
 /* rte_sched defines */
 // RTE_SCHED_RED is not set
 // RTE_SCHED_COLLECT_STATS is not set
 // RTE_SCHED_SUBPORT_TC_OV is not set
+#ifndef RTE_SCHED_PORT_N_GRINDERS
 #define RTE_SCHED_PORT_N_GRINDERS 8
+#endif
 // RTE_SCHED_VECTOR is not set
 
 /* KNI defines */
-#define RTE_KNI_PREEMPT_DEFAULT 1
+#define RTE_KNI_PREEMPT_DEFAULT
 
 /* rte_graph defines */
-#define RTE_GRAPH_BURST_SIZE 256
 #define RTE_LIBRTE_GRAPH_STATS 1
+#ifndef RTE_GRAPH_BURST_SIZE
+#define RTE_GRAPH_BURST_SIZE 256
+#endif
 
 /****** driver defines ********/
 
 /* QuickAssist device */
 /* Max. number of QuickAssist devices which can be attached */
+#ifndef RTE_PMD_QAT_MAX_PCI_DEVICES
 #define RTE_PMD_QAT_MAX_PCI_DEVICES 48
+#endif
+#ifndef RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS
 #define RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS 16
+#endif
+#ifndef RTE_PMD_QAT_COMP_IM_BUFFER_SIZE
 #define RTE_PMD_QAT_COMP_IM_BUFFER_SIZE 65536
+#endif
 
 /* virtio crypto defines */
+#ifndef RTE_MAX_VIRTIO_CRYPTO
 #define RTE_MAX_VIRTIO_CRYPTO 32
+#endif
 
 /* DPAA SEC max cryptodev devices*/
-#define RTE_LIBRTE_DPAA_MAX_CRYPTODEV	4
+#ifndef RTE_LIBRTE_DPAA_MAX_CRYPTODEV
+#define RTE_LIBRTE_DPAA_MAX_CRYPTODEV 4
+#endif
 
 /* fm10k defines */
-#define RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE 1
+#define RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE
 
 /* i40e defines */
-#define RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC 1
+#define RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC
 // RTE_LIBRTE_I40E_16BYTE_RX_DESC is not set
+#ifndef RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF 64
+#endif
+#ifndef RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF 4
+#endif
+#ifndef RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM 4
+#endif
 
 /* Ring net PMD settings */
+#ifndef RTE_PMD_RING_MAX_RX_RINGS
 #define RTE_PMD_RING_MAX_RX_RINGS 16
+#endif
+#ifndef RTE_PMD_RING_MAX_TX_RINGS
 #define RTE_PMD_RING_MAX_TX_RINGS 16
+#endif
 
 /* QEDE PMD defines */
+#ifndef RTE_LIBRTE_QEDE_FW
 #define RTE_LIBRTE_QEDE_FW ""
+#endif
 
 #endif /* _RTE_CONFIG_H_ */
-- 
2.25.1

Re: [dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults

[Ma, LihongX]

Tested-by: lihongx Ma<lihongx.ma@intel.com>
Before apply this patchset, set config like DRTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=8 will failed,
After apply this patchset, the the meson build can work find.
Cmd like below:
meson -Denable_kmods=True -Dlibdir=lib --default-library=static -Dexamples=vmdq_dcb -Dc_args='-DRTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=8' config-test1
ninja -C config-test1

Regards,
Ma,lihong

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> Sent: Tuesday, August 25, 2020 7:45 PM
> To: dev@dpdk.org
> Cc: bluca@debian.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH 2/2] config: allow overriding some build
> defaults
> 
> In case a developer uses CFLAGS to set different default values for the
> defines in the rte_config.h file, use #ifndef / #endif guards around the
> setting of those values. For those lines just "defining" a macro without
> assigning it a value to be used by code, drop the value argument (where
> possible) to make it clearer that that is what is happening, since those
> don't need the #ifdef guard.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  config/rte_config.h | 110 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 99 insertions(+), 11 deletions(-)
> 
> diff --git a/config/rte_config.h b/config/rte_config.h index

Re: [dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults

[Hemant Agrawal]

HI Bruce,
	Will you please also add similar command examples in docs so that it becomes easy for the developers to use meson?

Regards,
Hemant

-----Original Message-----
From: dev <dev-bounces@dpdk.org> On Behalf Of Ma, LihongX
Sent: Tuesday, September 1, 2020 10:48 AM
To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org
Cc: bluca@debian.org; Richardson, Bruce <bruce.richardson@intel.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults

Tested-by: lihongx Ma<lihongx.ma@intel.com> Before apply this patchset, set config like DRTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=8 will failed, After apply this patchset, the the meson build can work find.
Cmd like below:
meson -Denable_kmods=True -Dlibdir=lib --default-library=static -Dexamples=vmdq_dcb -Dc_args='-DRTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=8' config-test1 ninja -C config-test1

Regards,
Ma,lihong

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> Sent: Tuesday, August 25, 2020 7:45 PM
> To: dev@dpdk.org
> Cc: bluca@debian.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH 2/2] config: allow overriding some build 
> defaults
> 
> In case a developer uses CFLAGS to set different default values for 
> the defines in the rte_config.h file, use #ifndef / #endif guards 
> around the setting of those values. For those lines just "defining" a 
> macro without assigning it a value to be used by code, drop the value 
> argument (where
> possible) to make it clearer that that is what is happening, since 
> those don't need the #ifdef guard.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  config/rte_config.h | 110 
> +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 99 insertions(+), 11 deletions(-)
> 
> diff --git a/config/rte_config.h b/config/rte_config.h index

Re: [dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults

[Bruce Richardson]

On Tue, Sep 01, 2020 at 06:07:56AM +0000, Hemant Agrawal wrote:
> HI Bruce,
> 	Will you please also add similar command examples in docs so that it becomes easy for the developers to use meson?
> 
> Regards,
> Hemant
> 

I'll add a note in somewhere, but this is probably not something that we
want to be advertising too much. We are trying to move away from build-time
config so we want the defaults to be sane and try and avoid developers
asking the end-user to compile up DPDK with magic flags. That said, it
should be possible, and documented. :-)

/Bruce

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ma, LihongX
> Sent: Tuesday, September 1, 2020 10:48 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org
> Cc: bluca@debian.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults
> 
> Tested-by: lihongx Ma<lihongx.ma@intel.com> Before apply this patchset, set config like DRTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=8 will failed, After apply this patchset, the the meson build can work find.
> Cmd like below:
> meson -Denable_kmods=True -Dlibdir=lib --default-library=static -Dexamples=vmdq_dcb -Dc_args='-DRTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=8' config-test1 ninja -C config-test1
> 
> Regards,
> Ma,lihong
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> > Sent: Tuesday, August 25, 2020 7:45 PM
> > To: dev@dpdk.org
> > Cc: bluca@debian.org; Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: [dpdk-dev] [PATCH 2/2] config: allow overriding some build 
> > defaults
> > 
> > In case a developer uses CFLAGS to set different default values for 
> > the defines in the rte_config.h file, use #ifndef / #endif guards 
> > around the setting of those values. For those lines just "defining" a 
> > macro without assigning it a value to be used by code, drop the value 
> > argument (where
> > possible) to make it clearer that that is what is happening, since 
> > those don't need the #ifdef guard.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  config/rte_config.h | 110 
> > +++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 99 insertions(+), 11 deletions(-)
> > 
> > diff --git a/config/rte_config.h b/config/rte_config.h index
> 

Re: [dpdk-dev] [PATCH 2/2] config: allow overriding some build defaults

[Bruce Richardson]

On Tue, Sep 01, 2020 at 06:07:56AM +0000, Hemant Agrawal wrote:
> HI Bruce,
> 	Will you please also add similar command examples in docs so that it becomes easy for the developers to use meson?
> 
> Regards,
> Hemant
> 
Some doc updates made in V2.

[dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

A number of the more advanced DPDK build settings which are not expected to
be user modified are stored in config/rte_config.h. In some cases, for a
custom build a user may want to override those settings via CFLAGS, so we
need to ensure that the definitions do not override the user-provided
values.

Bruce Richardson (3):
  config: remove explicit undefinition of unset values
  config: allow overriding some build defaults
  doc: add notes on overriding extra config values

 config/rte_config.h                       | 122 +++++++++++++++++++---
 doc/guides/linux_gsg/build_dpdk.rst       |   8 ++
 doc/guides/prog_guide/build-sdk-meson.rst |   8 ++
 3 files changed, 121 insertions(+), 17 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH v2 1/3] config: remove explicit undefinition of unset values

[Bruce Richardson]

Rather than explicitly clearing any setting of undefined values in our
rte_config.h file, it's better to instead just add a comment that the value
is not set. Using a comment allows the user to set the value using CFLAGS
or similar mechanism without the config file clearing the value again.

The text used "<VALUE> is not set" is modelled after the kernel approach of
doing the same thing.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---

Although DPDK coding convention forbids use of "//" for comments, using
regular C comment style makes the config settings less clear, as they can
be confused with regular comments in the file. Using "//" makes them stand
out better, so I prefer it. However, if others feel strongly, they can be
changed to standard.
---
 config/rte_config.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index 9bb915347..1c5a86d6a 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -85,17 +85,17 @@
 
 /* ip_fragmentation defines */
 #define RTE_LIBRTE_IP_FRAG_MAX_FRAG 4
-#undef RTE_LIBRTE_IP_FRAG_TBL_STAT
+// RTE_LIBRTE_IP_FRAG_TBL_STAT is not set
 
 /* rte_power defines */
 #define RTE_MAX_LCORE_FREQS 64
 
 /* rte_sched defines */
-#undef RTE_SCHED_RED
-#undef RTE_SCHED_COLLECT_STATS
-#undef RTE_SCHED_SUBPORT_TC_OV
+// RTE_SCHED_RED is not set
+// RTE_SCHED_COLLECT_STATS is not set
+// RTE_SCHED_SUBPORT_TC_OV is not set
 #define RTE_SCHED_PORT_N_GRINDERS 8
-#undef RTE_SCHED_VECTOR
+// RTE_SCHED_VECTOR is not set
 
 /* KNI defines */
 #define RTE_KNI_PREEMPT_DEFAULT 1
@@ -123,7 +123,7 @@
 
 /* i40e defines */
 #define RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC 1
-#undef RTE_LIBRTE_I40E_16BYTE_RX_DESC
+// RTE_LIBRTE_I40E_16BYTE_RX_DESC is not set
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF 64
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF 4
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM 4
-- 
2.25.1

[dpdk-dev] [PATCH v2 2/3] config: allow overriding some build defaults

[Bruce Richardson]

In case a developer uses CFLAGS to set different default values for the
defines in the rte_config.h file, use #ifndef / #endif guards around the
setting of those values. For those lines just "defining" a macro without
assigning it a value to be used by code, drop the value argument (where
possible) to make it clearer that that is what is happening, since those
don't need the #ifdef guard.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Tested-by: Lihong Ma <lihongx.ma@intel.com>
---
 config/rte_config.h | 110 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 99 insertions(+), 11 deletions(-)

diff --git a/config/rte_config.h b/config/rte_config.h
index 1c5a86d6a..f39da76c1 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2017 Intel Corporation
+ * Copyright(c) 2017-2020 Intel Corporation
  */
 
 /**
@@ -20,10 +20,10 @@
 
 /* legacy defines */
 #ifdef RTE_EXEC_ENV_LINUX
-#define RTE_EXEC_ENV_LINUXAPP 1
+#define RTE_EXEC_ENV_LINUXAPP
 #endif
 #ifdef RTE_EXEC_ENV_FREEBSD
-#define RTE_EXEC_ENV_BSDAPP 1
+#define RTE_EXEC_ENV_BSDAPP
 #endif
 
 /* String that appears before the version number */
@@ -32,107 +32,195 @@
 /****** library defines ********/
 
 /* EAL defines */
+#define RTE_BACKTRACE
+#ifndef RTE_MAX_HEAPS
 #define RTE_MAX_HEAPS 32
+#endif
+#ifndef RTE_MAX_MEMSEG_LISTS
 #define RTE_MAX_MEMSEG_LISTS 128
+#endif
+#ifndef RTE_MAX_MEMSEG_PER_LIST
 #define RTE_MAX_MEMSEG_PER_LIST 8192
+#endif
+#ifndef RTE_MAX_MEM_MB_PER_LIST
 #define RTE_MAX_MEM_MB_PER_LIST 32768
+#endif
+#ifndef RTE_MAX_MEMSEG_PER_TYPE
 #define RTE_MAX_MEMSEG_PER_TYPE 32768
+#endif
+#ifndef RTE_MAX_MEM_MB_PER_TYPE
 #define RTE_MAX_MEM_MB_PER_TYPE 65536
+#endif
+#ifndef RTE_MAX_MEMZONE
 #define RTE_MAX_MEMZONE 2560
+#endif
+#ifndef RTE_MAX_TAILQ
 #define RTE_MAX_TAILQ 32
+#endif
+#ifndef RTE_LOG_DP_LEVEL
 #define RTE_LOG_DP_LEVEL RTE_LOG_INFO
-#define RTE_BACKTRACE 1
+#endif
+#ifndef RTE_MAX_VFIO_CONTAINERS
 #define RTE_MAX_VFIO_CONTAINERS 64
+#endif
 
 /* bsd module defines */
+#ifndef RTE_CONTIGMEM_MAX_NUM_BUFS
 #define RTE_CONTIGMEM_MAX_NUM_BUFS 64
+#endif
+#ifndef RTE_CONTIGMEM_DEFAULT_NUM_BUFS
 #define RTE_CONTIGMEM_DEFAULT_NUM_BUFS 1
+#endif
+#ifndef RTE_CONTIGMEM_DEFAULT_BUF_SIZE
 #define RTE_CONTIGMEM_DEFAULT_BUF_SIZE (512*1024*1024)
+#endif
 
 /* mempool defines */
+#ifndef RTE_MEMPOOL_CACHE_MAX_SIZE
 #define RTE_MEMPOOL_CACHE_MAX_SIZE 512
+#endif
 
 /* mbuf defines */
+#define RTE_MBUF_REFCNT_ATOMIC
+#ifndef RTE_MBUF_DEFAULT_MEMPOOL_OPS
 #define RTE_MBUF_DEFAULT_MEMPOOL_OPS "ring_mp_mc"
-#define RTE_MBUF_REFCNT_ATOMIC 1
+#endif
+#ifndef RTE_PKTMBUF_HEADROOM
 #define RTE_PKTMBUF_HEADROOM 128
+#endif
 
 /* ether defines */
+#define RTE_ETHDEV_RXTX_CALLBACKS
+#ifndef RTE_MAX_QUEUES_PER_PORT
 #define RTE_MAX_QUEUES_PER_PORT 1024
+#endif
+#ifndef RTE_ETHDEV_QUEUE_STAT_CNTRS
 #define RTE_ETHDEV_QUEUE_STAT_CNTRS 16
-#define RTE_ETHDEV_RXTX_CALLBACKS 1
+#endif
 
 /* cryptodev defines */
+#ifndef RTE_CRYPTO_MAX_DEVS
 #define RTE_CRYPTO_MAX_DEVS 64
+#endif
+#ifndef RTE_CRYPTODEV_NAME_LEN
 #define RTE_CRYPTODEV_NAME_LEN 64
+#endif
 
 /* compressdev defines */
+#ifndef RTE_COMPRESS_MAX_DEVS
 #define RTE_COMPRESS_MAX_DEVS 64
+#endif
 
 /* regexdev defines */
+#ifndef RTE_MAX_REGEXDEV_DEVS
 #define RTE_MAX_REGEXDEV_DEVS 32
+#endif
 
 /* eventdev defines */
+#ifndef RTE_EVENT_MAX_DEVS
 #define RTE_EVENT_MAX_DEVS 16
+#endif
+#ifndef RTE_EVENT_MAX_QUEUES_PER_DEV
 #define RTE_EVENT_MAX_QUEUES_PER_DEV 64
+#endif
+#ifndef RTE_EVENT_TIMER_ADAPTER_NUM_MAX
 #define RTE_EVENT_TIMER_ADAPTER_NUM_MAX 32
+#endif
+#ifndef RTE_EVENT_ETH_INTR_RING_SIZE
 #define RTE_EVENT_ETH_INTR_RING_SIZE 1024
+#endif
+#ifndef RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE
 #define RTE_EVENT_CRYPTO_ADAPTER_MAX_INSTANCE 32
+#endif
+#ifndef RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE
 #define RTE_EVENT_ETH_TX_ADAPTER_MAX_INSTANCE 32
+#endif
 
 /* rawdev defines */
+#ifndef RTE_RAWDEV_MAX_DEVS
 #define RTE_RAWDEV_MAX_DEVS 64
+#endif
 
 /* ip_fragmentation defines */
+#ifndef RTE_LIBRTE_IP_FRAG_MAX_FRAG
 #define RTE_LIBRTE_IP_FRAG_MAX_FRAG 4
+#endif
 // RTE_LIBRTE_IP_FRAG_TBL_STAT is not set
 
 /* rte_power defines */
+#ifndef RTE_MAX_LCORE_FREQS
 #define RTE_MAX_LCORE_FREQS 64
+#endif
 
 /* rte_sched defines */
 // RTE_SCHED_RED is not set
 // RTE_SCHED_COLLECT_STATS is not set
 // RTE_SCHED_SUBPORT_TC_OV is not set
+#ifndef RTE_SCHED_PORT_N_GRINDERS
 #define RTE_SCHED_PORT_N_GRINDERS 8
+#endif
 // RTE_SCHED_VECTOR is not set
 
 /* KNI defines */
-#define RTE_KNI_PREEMPT_DEFAULT 1
+#define RTE_KNI_PREEMPT_DEFAULT
 
 /* rte_graph defines */
-#define RTE_GRAPH_BURST_SIZE 256
 #define RTE_LIBRTE_GRAPH_STATS 1
+#ifndef RTE_GRAPH_BURST_SIZE
+#define RTE_GRAPH_BURST_SIZE 256
+#endif
 
 /****** driver defines ********/
 
 /* QuickAssist device */
 /* Max. number of QuickAssist devices which can be attached */
+#ifndef RTE_PMD_QAT_MAX_PCI_DEVICES
 #define RTE_PMD_QAT_MAX_PCI_DEVICES 48
+#endif
+#ifndef RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS
 #define RTE_PMD_QAT_COMP_SGL_MAX_SEGMENTS 16
+#endif
+#ifndef RTE_PMD_QAT_COMP_IM_BUFFER_SIZE
 #define RTE_PMD_QAT_COMP_IM_BUFFER_SIZE 65536
+#endif
 
 /* virtio crypto defines */
+#ifndef RTE_MAX_VIRTIO_CRYPTO
 #define RTE_MAX_VIRTIO_CRYPTO 32
+#endif
 
 /* DPAA SEC max cryptodev devices*/
-#define RTE_LIBRTE_DPAA_MAX_CRYPTODEV	4
+#ifndef RTE_LIBRTE_DPAA_MAX_CRYPTODEV
+#define RTE_LIBRTE_DPAA_MAX_CRYPTODEV 4
+#endif
 
 /* fm10k defines */
-#define RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE 1
+#define RTE_LIBRTE_FM10K_RX_OLFLAGS_ENABLE
 
 /* i40e defines */
-#define RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC 1
+#define RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC
 // RTE_LIBRTE_I40E_16BYTE_RX_DESC is not set
+#ifndef RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF 64
+#endif
+#ifndef RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF 4
+#endif
+#ifndef RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM
 #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM 4
+#endif
 
 /* Ring net PMD settings */
+#ifndef RTE_PMD_RING_MAX_RX_RINGS
 #define RTE_PMD_RING_MAX_RX_RINGS 16
+#endif
+#ifndef RTE_PMD_RING_MAX_TX_RINGS
 #define RTE_PMD_RING_MAX_TX_RINGS 16
+#endif
 
 /* QEDE PMD defines */
+#ifndef RTE_LIBRTE_QEDE_FW
 #define RTE_LIBRTE_QEDE_FW ""
+#endif
 
 #endif /* _RTE_CONFIG_H_ */
-- 
2.25.1

[dpdk-dev] [PATCH v2 3/3] doc: add notes on overriding extra config values

[Bruce Richardson]

Since it's possible to tweak the DPDK build a little further using CFLAGS,
we note that in the documentation.

Suggested-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/linux_gsg/build_dpdk.rst       | 8 ++++++++
 doc/guides/prog_guide/build-sdk-meson.rst | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/guides/linux_gsg/build_dpdk.rst b/doc/guides/linux_gsg/build_dpdk.rst
index c536e354e..9c242069e 100644
--- a/doc/guides/linux_gsg/build_dpdk.rst
+++ b/doc/guides/linux_gsg/build_dpdk.rst
@@ -117,6 +117,14 @@ dependencies are met on the current system are built.
 When `-Dexamples=all` is set as a meson option, meson will check each example application to see if it can be built,
 and add all which can be built to the list of tasks in the ninja build configuration file.
 
+.. note::
+
+    A number of buildtime constants are present in DPDK, listed in file ``config/rte_config.h``.
+    While these should not normally need to be changed,
+    they can be overridden by setting the new value of the constant in the ``CFLAGS`` environment variable,
+    or via ``c_args`` meson parameter.
+    For example: ``meson configure -Dc_args="-DRTE_PKTMBUF_HEADROOM=256"``
+
 Building Applications Using Installed DPDK
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/prog_guide/build-sdk-meson.rst b/doc/guides/prog_guide/build-sdk-meson.rst
index 44d1cafdf..3838871a3 100644
--- a/doc/guides/prog_guide/build-sdk-meson.rst
+++ b/doc/guides/prog_guide/build-sdk-meson.rst
@@ -68,6 +68,14 @@ built into meson, while others, such as ``max_lcores``, or the list of
 examples to build, are DPDK-specific. To have a list of all options
 available run ``meson configure`` in the build directory.
 
+.. note::
+
+    A number of buildtime constants are present in DPDK, listed in file ``config/rte_config.h``.
+    While these should not normally need to be changed,
+    they can be overridden by setting the new value of the constant in the ``CFLAGS`` environment variable,
+    or via ``c_args`` meson parameter.
+    For example: ``meson configure -Dc_args="-DRTE_PKTMBUF_HEADROOM=256"``
+
 Examples of adjusting the defaults when doing initial meson configuration.
 Project-specific options are passed used -Doption=value::
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2 3/3] doc: add notes on overriding extra config values

[Hemant Agrawal]

Acked-by: Hemant Agrawal <Hemant.agrawal@nxp.com>

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Thu, Sep 03, 2020 at 03:49:39PM +0100, Bruce Richardson wrote:
> A number of the more advanced DPDK build settings which are not expected to
> be user modified are stored in config/rte_config.h. In some cases, for a
> custom build a user may want to override those settings via CFLAGS, so we
> need to ensure that the definitions do not override the user-provided
> values.
> 
> Bruce Richardson (3):
>   config: remove explicit undefinition of unset values
>   config: allow overriding some build defaults
>   doc: add notes on overriding extra config values
> 
Ping for any further reviews or comments on this set?

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Chen, BoX C]

Hi, Bruce

Can this patch be merged before RC1? It is very helpful for our verification.
Thanks.

Regards,
Chen Bo

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> Sent: October 14, 2020 22:20
> To: dev@dpdk.org
> Cc: Ma, LihongX <lihongx.ma@intel.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>
> Subject: Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time
> constants
> 
> On Thu, Sep 03, 2020 at 03:49:39PM +0100, Bruce Richardson wrote:
> > A number of the more advanced DPDK build settings which are not
> > expected to be user modified are stored in config/rte_config.h. In
> > some cases, for a custom build a user may want to override those
> > settings via CFLAGS, so we need to ensure that the definitions do not
> > override the user-provided values.
> >
> > Bruce Richardson (3):
> >   config: remove explicit undefinition of unset values
> >   config: allow overriding some build defaults
> >   doc: add notes on overriding extra config values
> >
> Ping for any further reviews or comments on this set?

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Thu, Oct 15, 2020 at 09:55:18AM +0100, Chen, BoX C wrote:
> Hi, Bruce
> 
> Can this patch be merged before RC1? It is very helpful for our verification.
> Thanks.
> 
> Regards,
> Chen Bo
> 

Understood, however, it is not fully reviewed and acked, so if you have
tested this whole set, please provide a tested-by tag in an email.
Similarly if you, or others, have time to review the set please do and
provide feedback and/or acks on it.

/Bruce

> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> > Sent: October 14, 2020 22:20
> > To: dev@dpdk.org
> > Cc: Ma, LihongX <lihongx.ma@intel.com>; Hemant Agrawal
> > <hemant.agrawal@nxp.com>
> > Subject: Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time
> > constants
> >
> > On Thu, Sep 03, 2020 at 03:49:39PM +0100, Bruce Richardson wrote:
> > > A number of the more advanced DPDK build settings which are not
> > > expected to be user modified are stored in config/rte_config.h. In
> > > some cases, for a custom build a user may want to override those
> > > settings via CFLAGS, so we need to ensure that the definitions do not
> > > override the user-provided values.
> > >
> > > Bruce Richardson (3):
> > >   config: remove explicit undefinition of unset values
> > >   config: allow overriding some build defaults
> > >   doc: add notes on overriding extra config values
> > >
> > Ping for any further reviews or comments on this set?

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[David Marchand]

Hello Bruce,

On Thu, Sep 3, 2020 at 4:50 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> A number of the more advanced DPDK build settings which are not expected to
> be user modified are stored in config/rte_config.h. In some cases, for a
> custom build a user may want to override those settings via CFLAGS, so we
> need to ensure that the definitions do not override the user-provided
> values.
>
> Bruce Richardson (3):
>   config: remove explicit undefinition of unset values
>   config: allow overriding some build defaults
>   doc: add notes on overriding extra config values

$ CFLAGS="-DRTE_MAX_MEMSEG_LISTS=64" meson setup
--default-library=shared --buildtype=debugoptimized
-Dprefix=/home/dmarchan/git/pub/dpdk.org/build/install build
$ ninja-build -C build -j4 install


librte_eal.so is indeed built with the 64 value:
$ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
die__process_function: tag not supported (INVALID)!
    struct rte_memseg_list     memsegs[64];          /*   136  8704 */


But no trace of the custom value for external applications:
$ grep -r RTE_MAX_MEMSEG_LISTS build/install
build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
Binary file build/install/lib64/librte_eal.a matches
Binary file build/install/lib64/librte_eal.so.21.0 matches

I can see the same using the meson option -Dc_args.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Fri, Oct 16, 2020 at 05:47:45PM +0200, David Marchand wrote:
> Hello Bruce,
> 
> On Thu, Sep 3, 2020 at 4:50 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > A number of the more advanced DPDK build settings which are not expected to
> > be user modified are stored in config/rte_config.h. In some cases, for a
> > custom build a user may want to override those settings via CFLAGS, so we
> > need to ensure that the definitions do not override the user-provided
> > values.
> >
> > Bruce Richardson (3):
> >   config: remove explicit undefinition of unset values
> >   config: allow overriding some build defaults
> >   doc: add notes on overriding extra config values
> 
> $ CFLAGS="-DRTE_MAX_MEMSEG_LISTS=64" meson setup
> --default-library=shared --buildtype=debugoptimized
> -Dprefix=/home/dmarchan/git/pub/dpdk.org/build/install build
> $ ninja-build -C build -j4 install
> 
> 
> librte_eal.so is indeed built with the 64 value:
> $ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
> die__process_function: tag not supported (INVALID)!
>     struct rte_memseg_list     memsegs[64];          /*   136  8704 */
> 
> 
> But no trace of the custom value for external applications:
> $ grep -r RTE_MAX_MEMSEG_LISTS build/install
> build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
> build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
> Binary file build/install/lib64/librte_eal.a matches
> Binary file build/install/lib64/librte_eal.so.21.0 matches
> 
> I can see the same using the meson option -Dc_args.
> 

Good point, I had not thought of external apps using these values.

They are mostly for internal use, so maybe its worthwhile looking to not
have them in a public header file. What do you think? Is it likely that
apps would be using some of these values, or needs to know the specifics?

/Bruce

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[David Marchand]

On Fri, Oct 16, 2020 at 5:56 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > librte_eal.so is indeed built with the 64 value:
> > $ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
> > die__process_function: tag not supported (INVALID)!
> >     struct rte_memseg_list     memsegs[64];          /*   136  8704 */
> >
> >
> > But no trace of the custom value for external applications:
> > $ grep -r RTE_MAX_MEMSEG_LISTS build/install
> > build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
> > build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
> > Binary file build/install/lib64/librte_eal.a matches
> > Binary file build/install/lib64/librte_eal.so.21.0 matches
> >
> > I can see the same using the meson option -Dc_args.
> >
>
> Good point, I had not thought of external apps using these values.
>
> They are mostly for internal use, so maybe its worthwhile looking to not
> have them in a public header file. What do you think? Is it likely that
> apps would be using some of these values, or needs to know the specifics?

Some are publicly exposed, like RTE_MEMPOOL_CACHE_MAX_SIZE,
RTE_PKTMBUF_HEADROOM, RTE_ETHDEV_RXTX_CALLBACKS,
For those, either we propagate the overriden value to the installed
rte_config.h or we refuse customisation.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Fri, Oct 16, 2020 at 06:46:12PM +0200, David Marchand wrote:
> On Fri, Oct 16, 2020 at 5:56 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > > librte_eal.so is indeed built with the 64 value:
> > > $ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
> > > die__process_function: tag not supported (INVALID)!
> > >     struct rte_memseg_list     memsegs[64];          /*   136  8704 */
> > >
> > >
> > > But no trace of the custom value for external applications:
> > > $ grep -r RTE_MAX_MEMSEG_LISTS build/install
> > > build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
> > > build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
> > > Binary file build/install/lib64/librte_eal.a matches
> > > Binary file build/install/lib64/librte_eal.so.21.0 matches
> > >
> > > I can see the same using the meson option -Dc_args.
> > >
> >
> > Good point, I had not thought of external apps using these values.
> >
> > They are mostly for internal use, so maybe its worthwhile looking to not
> > have them in a public header file. What do you think? Is it likely that
> > apps would be using some of these values, or needs to know the specifics?
> 
> Some are publicly exposed, like RTE_MEMPOOL_CACHE_MAX_SIZE,
> RTE_PKTMBUF_HEADROOM, RTE_ETHDEV_RXTX_CALLBACKS,
> For those, either we propagate the overriden value to the installed
> rte_config.h or we refuse customisation.
> 
I'd suggest the first 2 of those should possibly be global meson options.
Third should probably not be exposed at all.

/Bruce

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Thomas Monjalon]

19/10/2020 12:21, Bruce Richardson:
> On Fri, Oct 16, 2020 at 06:46:12PM +0200, David Marchand wrote:
> > On Fri, Oct 16, 2020 at 5:56 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > > > librte_eal.so is indeed built with the 64 value:
> > > > $ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
> > > > die__process_function: tag not supported (INVALID)!
> > > >     struct rte_memseg_list     memsegs[64];          /*   136  8704 */
> > > >
> > > >
> > > > But no trace of the custom value for external applications:
> > > > $ grep -r RTE_MAX_MEMSEG_LISTS build/install
> > > > build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
> > > > build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
> > > > Binary file build/install/lib64/librte_eal.a matches
> > > > Binary file build/install/lib64/librte_eal.so.21.0 matches
> > > >
> > > > I can see the same using the meson option -Dc_args.
> > >
> > > Good point, I had not thought of external apps using these values.
> > >
> > > They are mostly for internal use, so maybe its worthwhile looking to not
> > > have them in a public header file. What do you think? Is it likely that
> > > apps would be using some of these values, or needs to know the specifics?
> > 
> > Some are publicly exposed, like RTE_MEMPOOL_CACHE_MAX_SIZE,
> > RTE_PKTMBUF_HEADROOM, RTE_ETHDEV_RXTX_CALLBACKS,
> > For those, either we propagate the overriden value to the installed
> > rte_config.h or we refuse customisation.
> > 
> I'd suggest the first 2 of those should possibly be global meson options.

How the application is reading the meson options?

> Third should probably not be exposed at all.

I think everything should be exposed.
The application may need to know whether a feature is enabled or not,
and what is a specific tuning value.

I suspect it is the last blocker for meson adoption.
Now that we removed the makefiles, we need to fill this gap urgently.

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Mon, Oct 19, 2020 at 11:04:54PM +0200, Thomas Monjalon wrote:
> 19/10/2020 12:21, Bruce Richardson:
> > On Fri, Oct 16, 2020 at 06:46:12PM +0200, David Marchand wrote:
> > > On Fri, Oct 16, 2020 at 5:56 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > > > librte_eal.so is indeed built with the 64 value:
> > > > > $ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
> > > > > die__process_function: tag not supported (INVALID)!
> > > > >     struct rte_memseg_list     memsegs[64];          /*   136  8704 */
> > > > >
> > > > >
> > > > > But no trace of the custom value for external applications:
> > > > > $ grep -r RTE_MAX_MEMSEG_LISTS build/install
> > > > > build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
> > > > > build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
> > > > > Binary file build/install/lib64/librte_eal.a matches
> > > > > Binary file build/install/lib64/librte_eal.so.21.0 matches
> > > > >
> > > > > I can see the same using the meson option -Dc_args.
> > > >
> > > > Good point, I had not thought of external apps using these values.
> > > >
> > > > They are mostly for internal use, so maybe its worthwhile looking to not
> > > > have them in a public header file. What do you think? Is it likely that
> > > > apps would be using some of these values, or needs to know the specifics?
> > > 
> > > Some are publicly exposed, like RTE_MEMPOOL_CACHE_MAX_SIZE,
> > > RTE_PKTMBUF_HEADROOM, RTE_ETHDEV_RXTX_CALLBACKS,
> > > For those, either we propagate the overriden value to the installed
> > > rte_config.h or we refuse customisation.
> > > 
> > I'd suggest the first 2 of those should possibly be global meson options.
> 
> How the application is reading the meson options?
> 
The meson options are reflected in the rte_build_config.h file. It's not
automatic, but they should be set there by the config/meson.build file. If
some are missed, they can be added, but I disagree that all meson options
should always be passed through to apps. It makes them part of the API,
perhaps unnecessarily, and therefore harder to change or adjust.
Furthermore why should an app ever need to care if a DPDK build included
the docs, or the kmods?

> > Third should probably not be exposed at all.
> 
> I think everything should be exposed.
> The application may need to know whether a feature is enabled or not,
> and what is a specific tuning value.
> 
> I suspect it is the last blocker for meson adoption.  Now that we removed
> the makefiles, we need to fill this gap urgently.
> 
I really don't see this as a gap. With "make" we struggled with massive
amounts of build config, and we tried to remove as much as we can. While
this is reporting what's there rather than tweaking it, surely many of
these settings are just better as #defines in the individual header files -
if they need to be exposed at all.

/Bruce

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Thomas Monjalon]

20/10/2020 10:34, Bruce Richardson:
> On Mon, Oct 19, 2020 at 11:04:54PM +0200, Thomas Monjalon wrote:
> > 19/10/2020 12:21, Bruce Richardson:
> > > On Fri, Oct 16, 2020 at 06:46:12PM +0200, David Marchand wrote:
> > > > On Fri, Oct 16, 2020 at 5:56 PM Bruce Richardson
> > > > <bruce.richardson@intel.com> wrote:
> > > > > > librte_eal.so is indeed built with the 64 value:
> > > > > > $ pahole -C rte_mem_config build/install/lib64/librte_eal.so |grep memsegs
> > > > > > die__process_function: tag not supported (INVALID)!
> > > > > >     struct rte_memseg_list     memsegs[64];          /*   136  8704 */
> > > > > >
> > > > > >
> > > > > > But no trace of the custom value for external applications:
> > > > > > $ grep -r RTE_MAX_MEMSEG_LISTS build/install
> > > > > > build/install/include/rte_config.h:#ifndef RTE_MAX_MEMSEG_LISTS
> > > > > > build/install/include/rte_config.h:#define RTE_MAX_MEMSEG_LISTS 128
> > > > > > Binary file build/install/lib64/librte_eal.a matches
> > > > > > Binary file build/install/lib64/librte_eal.so.21.0 matches
> > > > > >
> > > > > > I can see the same using the meson option -Dc_args.
> > > > >
> > > > > Good point, I had not thought of external apps using these values.
> > > > >
> > > > > They are mostly for internal use, so maybe its worthwhile looking to not
> > > > > have them in a public header file. What do you think? Is it likely that
> > > > > apps would be using some of these values, or needs to know the specifics?
> > > > 
> > > > Some are publicly exposed, like RTE_MEMPOOL_CACHE_MAX_SIZE,
> > > > RTE_PKTMBUF_HEADROOM, RTE_ETHDEV_RXTX_CALLBACKS,
> > > > For those, either we propagate the overriden value to the installed
> > > > rte_config.h or we refuse customisation.
> > > > 
> > > I'd suggest the first 2 of those should possibly be global meson options.
> > 
> > How the application is reading the meson options?
> > 
> The meson options are reflected in the rte_build_config.h file. It's not
> automatic, but they should be set there by the config/meson.build file. If
> some are missed, they can be added, but I disagree that all meson options
> should always be passed through to apps. It makes them part of the API,
> perhaps unnecessarily, and therefore harder to change or adjust.
> Furthermore why should an app ever need to care if a DPDK build included
> the docs, or the kmods?
> 
> > > Third should probably not be exposed at all.
> > 
> > I think everything should be exposed.
> > The application may need to know whether a feature is enabled or not,
> > and what is a specific tuning value.
> > 
> > I suspect it is the last blocker for meson adoption.  Now that we removed
> > the makefiles, we need to fill this gap urgently.
> > 
> I really don't see this as a gap. With "make" we struggled with massive
> amounts of build config, and we tried to remove as much as we can. While
> this is reporting what's there rather than tweaking it, surely many of
> these settings are just better as #defines in the individual header files -
> if they need to be exposed at all.

I agree with the goal of moving these #defines internally.
I just feel having wrong values in rte_config.h looks to be a bug.

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Tue, Oct 20, 2020 at 12:04:56PM +0200, Thomas Monjalon wrote:
> 20/10/2020 10:34, Bruce Richardson:
> > On Mon, Oct 19, 2020 at 11:04:54PM +0200, Thomas Monjalon wrote:
> > > 19/10/2020 12:21, Bruce Richardson:
> > > > On Fri, Oct 16, 2020 at 06:46:12PM +0200, David Marchand wrote:
> > > > > On Fri, Oct 16, 2020 at 5:56 PM Bruce Richardson
> > > > > <bruce.richardson@intel.com> wrote:
> > > > > > > librte_eal.so is indeed built with the 64 value: $ pahole -C
> > > > > > > rte_mem_config build/install/lib64/librte_eal.so |grep
> > > > > > > memsegs die__process_function: tag not supported (INVALID)!
> > > > > > > struct rte_memseg_list     memsegs[64];          /*   136
> > > > > > > 8704 */
> > > > > > >
> > > > > > >
> > > > > > > But no trace of the custom value for external applications: $
> > > > > > > grep -r RTE_MAX_MEMSEG_LISTS build/install
> > > > > > > build/install/include/rte_config.h:#ifndef
> > > > > > > RTE_MAX_MEMSEG_LISTS
> > > > > > > build/install/include/rte_config.h:#define
> > > > > > > RTE_MAX_MEMSEG_LISTS 128 Binary file
> > > > > > > build/install/lib64/librte_eal.a matches Binary file
> > > > > > > build/install/lib64/librte_eal.so.21.0 matches
> > > > > > >
> > > > > > > I can see the same using the meson option -Dc_args.
> > > > > >
> > > > > > Good point, I had not thought of external apps using these
> > > > > > values.
> > > > > >
> > > > > > They are mostly for internal use, so maybe its worthwhile
> > > > > > looking to not have them in a public header file. What do you
> > > > > > think? Is it likely that apps would be using some of these
> > > > > > values, or needs to know the specifics?
> > > > > 
> > > > > Some are publicly exposed, like RTE_MEMPOOL_CACHE_MAX_SIZE,
> > > > > RTE_PKTMBUF_HEADROOM, RTE_ETHDEV_RXTX_CALLBACKS, For those,
> > > > > either we propagate the overriden value to the installed
> > > > > rte_config.h or we refuse customisation.
> > > > > 
> > > > I'd suggest the first 2 of those should possibly be global meson
> > > > options.
> > > 
> > > How the application is reading the meson options?
> > > 
> > The meson options are reflected in the rte_build_config.h file. It's
> > not automatic, but they should be set there by the config/meson.build
> > file. If some are missed, they can be added, but I disagree that all
> > meson options should always be passed through to apps. It makes them
> > part of the API, perhaps unnecessarily, and therefore harder to change
> > or adjust.  Furthermore why should an app ever need to care if a DPDK
> > build included the docs, or the kmods?
> > 
> > > > Third should probably not be exposed at all.
> > > 
> > > I think everything should be exposed.  The application may need to
> > > know whether a feature is enabled or not, and what is a specific
> > > tuning value.
> > > 
> > > I suspect it is the last blocker for meson adoption.  Now that we
> > > removed the makefiles, we need to fill this gap urgently.
> > > 
> > I really don't see this as a gap. With "make" we struggled with massive
> > amounts of build config, and we tried to remove as much as we can.
> > While this is reporting what's there rather than tweaking it, surely
> > many of these settings are just better as #defines in the individual
> > header files - if they need to be exposed at all.
> 
> I agree with the goal of moving these #defines internally.  I just feel
> having wrong values in rte_config.h looks to be a bug.
> 
Oh agree, absolutely!  The other attitude to take (and document) here is
that if someone is going to the trouble to override the default DPDK value
when building DPDK they also need to provide that CFLAG when building their
app too. If they don't want that, they need to adjust the value in the code
directly.

Re: [dpdk-dev] [PATCH v2 0/3] Allow overriding of build-time constants

[Bruce Richardson]

On Thu, Sep 03, 2020 at 03:49:39PM +0100, Bruce Richardson wrote:
> A number of the more advanced DPDK build settings which are not expected to
> be user modified are stored in config/rte_config.h. In some cases, for a
> custom build a user may want to override those settings via CFLAGS, so we
> need to ensure that the definitions do not override the user-provided
> values.
> 
> Bruce Richardson (3):
>   config: remove explicit undefinition of unset values
>   config: allow overriding some build defaults
>   doc: add notes on overriding extra config values

Since there are issues flagged on this set, and I don't have a good
solution to fix them right now, I think the best approach is to drop this
set for consideration for merging in 20.11 release. As such, I'm going to
mark this set as rejected in patchwork.

If we still have a problem here in future, we can look at this again, but I
don't see the current patchset going ahead in its current form.

Regards,
/Bruce