Re: [dpdk-dev] [PATCH] ethdev: add namespace

On 6/30/2021 7:29 AM, David Marchand wrote:

Hello Ferruh,

On Tue, Jun 29, 2021 at 3:46 PM Ferruh Yigit [off-list ref] wrote:

quoted

Add 'RTE_ETH' namespace to all enums & macros in a backward compatible
way. The macros for backward compatibility can be removed in next LTS.

Signed-off-by: Ferruh Yigit <redacted>

- I did not do a full check but I noticed that ETH_RSS compat macro at
least is removed.
Is this intentional?

Yes, two groups of macros was remaining from 2013 for backward compatibility,
with this patch it would be two layer of redicrection, so I moved to old ones.
The only changes in the examples/app are because of these removed macros, since
rest are all backward compatible.

Removed ones:
/**
 * for rx mq mode backward compatible
 */
#define ETH_RSS                       ETH_MQ_RX_RSS
#define VMDQ_DCB                      ETH_MQ_RX_VMDQ_DCB
#define ETH_DCB_RX                    ETH_MQ_RX_DCB

/**
 * for tx mq mode backward compatible
 */
#define ETH_DCB_NONE                ETH_MQ_TX_NONE
#define ETH_VMDQ_DCB_TX             ETH_MQ_TX_VMDQ_DCB
#define ETH_DCB_TX                  ETH_MQ_TX_DCB

- libabigail is not happy because of enum names changes.
Example:

  [C] 'function int rte_eth_dev_configure(uint16_t, uint16_t,
uint16_t, const rte_eth_conf*)' at rte_ethdev.c:1326:1 has some
indirect sub-type changes:
    parameter 4 of type 'const rte_eth_conf*' has sub-type changes:
      in pointed to type 'const rte_eth_conf':
        in unqualified underlying type 'struct rte_eth_conf' at
rte_ethdev.h:1491:1:
          type size hasn't changed
          5 data member changes (1 filtered):
            type of 'rte_eth_rxmode rxmode' changed:
              type size hasn't changed
              1 data member change:
                type of 'rte_eth_rx_mq_mode mq_mode' changed:
                  type size hasn't changed
                  8 enumerator deletions:
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_NONE' value '0'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_RSS' value '1'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_DCB' value '2'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_DCB_RSS' value '3'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_VMDQ_ONLY' value '4'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_VMDQ_RSS' value '5'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_VMDQ_DCB' value '6'
                    'rte_eth_rx_mq_mode::ETH_MQ_RX_VMDQ_DCB_RSS' value '7'
                  8 enumerator insertions:
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_NONE' value '0'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_RSS' value '1'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_DCB' value '2'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_DCB_RSS' value '3'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_VMDQ_ONLY' value '4'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_VMDQ_RSS' value '5'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_VMDQ_DCB' value '6'
                    'rte_eth_rx_mq_mode::RTE_ETH_MQ_RX_VMDQ_DCB_RSS' value '7'
[snip]


I guess libabigail is lost because the symbol
rte_eth_rx_mq_mode::ETH_MQ_RX_NONE simply disappeared (because we used
a macro to wrap to the new name).

Yes.

Maybe we could go the other way: leave the current enums defined as is
and put in place wrappers for new names pointing as old names. 
The rest of the code in DPDK would use the new names only.

It works to prevent libabigail warnings but I think it can be confusing for
users on figuring out which ones are the correct ones to use.

This comment applies if we want to merge this change in 21.08 and/or
we want to backport this change.

This won't be a problem if we merge this patch in 21.11.

OK to have it on v21.11. In that case I assume all internal components also
needs to be updated to use new macros/enums during v21.11 release.

Let me send a deprecation notice for it.

Meanwhile, we can use this patch to discuss the prefix/namespace, if it should
be 'RTE_ETH_' or 'RTE_ETH_DEV_', or mix of both (as done now).

And if mixed prefix used, we can try to define when to have DEV_ part and when
not to have it.

quoted

---
We can get the update on v21.11 and remove backward compatibility macros
on v22.11.