[dpdk-dev] [PATCH v1 03/17] ethdev: replace library debug flag with global one
Ananyev, Konstantin
konstantin.ananyev at intel.com
Mon Apr 20 15:37:12 CEST 2020
>
>
>
> > -----Original Message-----
> > From: Lukasz Wojciechowski <l.wojciechow at partner.samsung.com>
> > Sent: Friday, April 17, 2020 10:57 PM
> > To: Thomas Monjalon <thomas at monjalon.net>; Igor Russkikh
> > <igor.russkikh at aquantia.com>; Pavel Belous <pavel.belous at aquantia.com>;
> > Lu, Wenzhuo <wenzhuo.lu at intel.com>; Marcin Wojtas
> > <mw at semihalf.com>; Michal Krawczyk <mk at semihalf.com>; Guy Tzalik
> > <gtzalik at amazon.com>; Evgeny Schemeilin <evgenys at amazon.com>; Igor
> > Chauskin <igorch at amazon.com>; John Daley <johndale at cisco.com>; Hyong
> > Youb Kim <hyonkim at cisco.com>; Zhang, Qi Z <qi.z.zhang at intel.com>;
> > Wang, Xiao W <xiao.w.wang at intel.com>; Ziyang Xuan
> > <xuanziyang2 at huawei.com>; Xiaoyun Wang
> > <cloud.wangxiaoyun at huawei.com>; Guoyang Zhou
> > <zhouguoyang at huawei.com>; Wei Hu (Xavier)
> > <xavier.huwei at huawei.com>; Min Hu (Connor) <humin29 at huawei.com>;
> > Yisen Zhuang <yisen.zhuang at huawei.com>; Xing, Beilei
> > <beilei.xing at intel.com>; Wu, Jingjing <jingjing.wu at intel.com>; Yang, Qiming
> > <qiming.yang at intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev at intel.com>; Rasesh Mody <rmody at marvell.com>;
> > Shahed Shaikh <shshaikh at marvell.com>; Singh, Jasvinder
> > <jasvinder.singh at intel.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu at intel.com>; Maxime Coquelin
> > <maxime.coquelin at redhat.com>; Wang, Zhihong
> > <zhihong.wang at intel.com>; Ye, Xiaolong <xiaolong.ye at intel.com>; Yong
> > Wang <yongwang at vmware.com>; Yigit, Ferruh <ferruh.yigit at intel.com>;
> > Andrew Rybchenko <arybchenko at solarflare.com>; Olivier Matz
> > <olivier.matz at 6wind.com>
> > Cc: dev at dpdk.org
> > Subject: [PATCH v1 03/17] ethdev: replace library debug flag with global one
> >
> > Use global debug flag RTE_DEBUG instead of RTE_LIBRTE_ETHDEV_DEBUG.
> > The old define is completely removed from source code and config.
> > The changes were applied also to all drivers using this flag.
> >
> > Signed-off-by: Lukasz Wojciechowski
> > <l.wojciechow at partner.samsung.com>
> > ---
> > config/common_base | 1 -
> > drivers/net/atlantic/atl_rxtx.c | 2 +-
> > drivers/net/e1000/em_rxtx.c | 2 +-
> > drivers/net/e1000/igb_rxtx.c | 2 +-
> > drivers/net/ena/ena_ethdev.c | 2 +-
> > drivers/net/enic/enic_rxtx.c | 2 +-
> > drivers/net/fm10k/fm10k_rxtx.c | 2 +-
> > drivers/net/hinic/hinic_pmd_tx.c | 2 +-
> > drivers/net/hns3/hns3_rxtx.c | 2 +-
> > drivers/net/i40e/i40e_rxtx.c | 2 +-
> > drivers/net/iavf/iavf_rxtx.c | 2 +-
> > drivers/net/ice/ice_rxtx.c | 2 +-
> > drivers/net/ixgbe/ixgbe_rxtx.c | 2 +-
> > drivers/net/qede/qede_rxtx.c | 4 ++--
> > drivers/net/softnic/rte_eth_softnic.c | 2 +-
> > drivers/net/softnic/rte_eth_softnic_thread.c | 2 +-
> > drivers/net/virtio/virtio_rxtx.c | 2 +-
> > drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
> > lib/librte_ethdev/rte_ethdev.h | 16 ++++++++--------
> > lib/librte_net/rte_net.h | 4 ++--
> > 20 files changed, 28 insertions(+), 29 deletions(-)
> >
> > diff --git a/config/common_base b/config/common_base
> > index c5be57f11..16a8f09b6 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -149,7 +149,6 @@ CONFIG_RTE_LIBRTE_KVARGS=y
> > # Compile generic ethernet library
> > #
> > CONFIG_RTE_LIBRTE_ETHER=y
> > -CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > CONFIG_RTE_MAX_ETHPORTS=32
> > CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > CONFIG_RTE_LIBRTE_IEEE1588=n
> > diff --git a/drivers/net/atlantic/atl_rxtx.c b/drivers/net/atlantic/atl_rxtx.c
> > index 449ffd454..eae54df22 100644
> > --- a/drivers/net/atlantic/atl_rxtx.c
> > +++ b/drivers/net/atlantic/atl_rxtx.c
> > @@ -821,7 +821,7 @@ atl_prep_pkts(__rte_unused void *tx_queue, struct
> > rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
> > index 49c53712a..c4083ff00 100644
> > --- a/drivers/net/e1000/em_rxtx.c
> > +++ b/drivers/net/e1000/em_rxtx.c
> > @@ -626,7 +626,7 @@ eth_em_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
> > index 684fa4ad8..6a78f26e6 100644
> > --- a/drivers/net/e1000/igb_rxtx.c
> > +++ b/drivers/net/e1000/igb_rxtx.c
> > @@ -641,7 +641,7 @@ eth_igb_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> > index 665afee4f..b9855e91b 100644
> > --- a/drivers/net/ena/ena_ethdev.c
> > +++ b/drivers/net/ena/ena_ethdev.c
> > @@ -2145,7 +2145,7 @@ eth_ena_prep_pkts(void *tx_queue, struct
> > rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/enic/enic_rxtx.c b/drivers/net/enic/enic_rxtx.c
> > index 6a8718c08..c42d563b4 100644
> > --- a/drivers/net/enic/enic_rxtx.c
> > +++ b/drivers/net/enic/enic_rxtx.c
> > @@ -414,7 +414,7 @@ uint16_t enic_prep_pkts(void *tx_queue, struct
> > rte_mbuf **tx_pkts,
> > rte_errno = ENOTSUP;
> > return i;
> > }
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> > b/drivers/net/fm10k/fm10k_rxtx.c
> > index 4accaa2cd..43d773f08 100644
> > --- a/drivers/net/fm10k/fm10k_rxtx.c
> > +++ b/drivers/net/fm10k/fm10k_rxtx.c
> > @@ -710,7 +710,7 @@ fm10k_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/hinic/hinic_pmd_tx.c
> > b/drivers/net/hinic/hinic_pmd_tx.c
> > index 64ec2c119..41d5a25b6 100644
> > --- a/drivers/net/hinic/hinic_pmd_tx.c
> > +++ b/drivers/net/hinic/hinic_pmd_tx.c
> > @@ -804,7 +804,7 @@ hinic_tx_offload_pkt_prepare(struct rte_mbuf *m,
> > !(ol_flags & PKT_TX_TUNNEL_VXLAN))
> > return -ENOTSUP;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > if (rte_validate_tx_offload(m) != 0)
> > return -EINVAL;
> > #endif
> > diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> > index ec6d19f58..45aa64b70 100644
> > --- a/drivers/net/hns3/hns3_rxtx.c
> > +++ b/drivers/net/hns3/hns3_rxtx.c
> > @@ -2296,7 +2296,7 @@ hns3_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > index 5e7c86ed8..282baf514 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -1499,7 +1499,7 @@ i40e_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
> > index 85d9a8e3b..8122d35be 100644
> > --- a/drivers/net/iavf/iavf_rxtx.c
> > +++ b/drivers/net/iavf/iavf_rxtx.c
> > @@ -1689,7 +1689,7 @@ iavf_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> > index 1c9f31efd..fd8ed2573 100644
> > --- a/drivers/net/ice/ice_rxtx.c
> > +++ b/drivers/net/ice/ice_rxtx.c
> > @@ -3037,7 +3037,7 @@ ice_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> > index 2e20e18c7..6964c4e52 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > @@ -992,7 +992,7 @@ ixgbe_prep_pkts(void *tx_queue, struct rte_mbuf
> > **tx_pkts, uint16_t nb_pkts)
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c
> > index b81788ca4..646eb2275 100644
> > --- a/drivers/net/qede/qede_rxtx.c
> > +++ b/drivers/net/qede/qede_rxtx.c
> > @@ -2156,7 +2156,7 @@ qede_xmit_prep_pkts(__rte_unused void *p_txq,
> > struct rte_mbuf **tx_pkts,
> > uint64_t ol_flags;
> > struct rte_mbuf *m;
> > uint16_t i;
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > int ret;
> > #endif
> >
> > @@ -2196,7 +2196,7 @@ qede_xmit_prep_pkts(__rte_unused void *p_txq,
> > struct rte_mbuf **tx_pkts,
> > break;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/drivers/net/softnic/rte_eth_softnic.c
> > b/drivers/net/softnic/rte_eth_softnic.c
> > index 11723778f..b5b169ff7 100644
> > --- a/drivers/net/softnic/rte_eth_softnic.c
> > +++ b/drivers/net/softnic/rte_eth_softnic.c
> > @@ -704,7 +704,7 @@ rte_pmd_softnic_manage(uint16_t port_id)
> > struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > struct pmd_internals *softnic;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > #endif
> >
> > diff --git a/drivers/net/softnic/rte_eth_softnic_thread.c
> > b/drivers/net/softnic/rte_eth_softnic_thread.c
> > index d610b1617..2f7c3a838 100644
> > --- a/drivers/net/softnic/rte_eth_softnic_thread.c
> > +++ b/drivers/net/softnic/rte_eth_softnic_thread.c
> > @@ -3093,7 +3093,7 @@ rte_pmd_softnic_run(uint16_t port_id)
> > {
> > struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > #endif
> >
> > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> > index 752faa0f6..02eaf38e3 100644
> > --- a/drivers/net/virtio/virtio_rxtx.c
> > +++ b/drivers/net/virtio/virtio_rxtx.c
> > @@ -1979,7 +1979,7 @@ virtio_xmit_pkts_prepare(void *tx_queue
> > __rte_unused, struct rte_mbuf **tx_pkts,
> > for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) {
> > struct rte_mbuf *m = tx_pkts[nb_tx];
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > error = rte_validate_tx_offload(m);
> > if (unlikely(error)) {
> > rte_errno = -error;
> > diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> > b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> > index dd99684be..a801290ff 100644
> > --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> > +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> > @@ -373,7 +373,7 @@ vmxnet3_prep_pkts(__rte_unused void *tx_queue,
> > struct rte_mbuf **tx_pkts,
> > return i;
> > }
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > ret = rte_validate_tx_offload(m);
> > if (ret != 0) {
> > rte_errno = -ret;
> > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> > index e9e3a1699..f314b57c7 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -4375,7 +4375,7 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t
> > queue_id,
> > struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > uint16_t nb_rx;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_pkt_burst, 0);
> >
> > @@ -4498,11 +4498,11 @@ rte_eth_rx_descriptor_status(uint16_t port_id,
> > uint16_t queue_id,
> > struct rte_eth_dev *dev;
> > void *rxq;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > #endif
> > dev = &rte_eth_devices[port_id];
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > if (queue_id >= dev->data->nb_rx_queues)
> > return -ENODEV;
> > #endif
> > @@ -4555,11 +4555,11 @@ static inline int
> > rte_eth_tx_descriptor_status(uint16_t port_id,
> > struct rte_eth_dev *dev;
> > void *txq;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> > #endif
> > dev = &rte_eth_devices[port_id];
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > if (queue_id >= dev->data->nb_tx_queues)
> > return -ENODEV;
> > #endif
> > @@ -4641,7 +4641,7 @@ rte_eth_tx_burst(uint16_t port_id, uint16_t
> > queue_id,
> > {
> > struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > RTE_FUNC_PTR_OR_ERR_RET(*dev->tx_pkt_burst, 0);
> >
> > @@ -4727,7 +4727,7 @@ rte_eth_tx_prepare(uint16_t port_id, uint16_t
> > queue_id,
> > {
> > struct rte_eth_dev *dev;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > if (!rte_eth_dev_is_valid_port(port_id)) {
> > RTE_ETHDEV_LOG(ERR, "Invalid TX port_id=%u\n", port_id);
> > rte_errno = EINVAL;
> > @@ -4737,7 +4737,7 @@ rte_eth_tx_prepare(uint16_t port_id, uint16_t
> > queue_id,
> >
> > dev = &rte_eth_devices[port_id];
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > if (queue_id >= dev->data->nb_tx_queues) {
> > RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n",
> > queue_id);
> > rte_errno = EINVAL;
> > diff --git a/lib/librte_net/rte_net.h b/lib/librte_net/rte_net.h
> > index 1560ecfa4..9a9732189 100644
> > --- a/lib/librte_net/rte_net.h
> > +++ b/lib/librte_net/rte_net.h
> > @@ -120,7 +120,7 @@ rte_net_intel_cksum_flags_prepare(struct rte_mbuf
> > *m, uint64_t ol_flags)
> > struct rte_udp_hdr *udp_hdr;
> > uint64_t inner_l3_offset = m->l2_len;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > /*
> > * Does packet set any of available offloads?
> > * Mainly it is required to avoid fragmented headers check if
> > @@ -133,7 +133,7 @@ rte_net_intel_cksum_flags_prepare(struct rte_mbuf
> > *m, uint64_t ol_flags)
> > if (ol_flags & (PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IPV6))
> > inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> >
> > -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#ifdef RTE_DEBUG
> > /*
> > * Check if headers are fragmented.
> > * The check could be less strict depending on which offloads are
> > --
> > 2.17.1
>
> The approach in this series does not make any sense to me: what if you want to debug just this library instead of the entire DPDK? We need
> to be able to enable debug for a single library at a time, as opposed to the user being flooded with unwanted debug messages from all the
> libraries.
>
> NAK from me.
I am agree with Cristian concern here:
that patch removes ability to enable/disable debug on particular library/PMD.
If the purpose is to minimize number of config compile options,
I wonder can't it be done in a slightly different way:
1. introduce gloabal RTE_DEBUG
2. keep actual .[c,h] files intact
3. In actual librte_xxx/meson.build file check if RTE_DEBUG is enabled,
If yes, then enable particular debug flag for these libs.
Something like:
If dpdk_conf.get('RTE_DEBUG') == true
dpdk_conf.set('RTE_LIBRTE_XXX_DEBUG ', 1)
defines that are used by multiple libs, probably can be
set in upper layer meson.build.
That way will have global 'debug' flag, but users will still
have an ability to enable/disable debug flags on a per lib basis
via CFLAGS="-D..."
Konstantin
More information about the dev
mailing list