On 10/17/2023 3:06 PM, Thomas Monjalon wrote: > Hello, > > 11/10/2023 11:27, Jie Hai: >> app/proc-info/main.c | 32 ++++++++++----- >> app/test-pmd/cmdline.c | 29 ++++++++++--- >> app/test-pmd/config.c | 38 ++++++++--------- >> app/test-pmd/testpmd.h | 2 +- >> doc/guides/rel_notes/release_23_11.rst | 2 + >> drivers/net/atlantic/atl_ethdev.c | 2 + >> drivers/net/axgbe/axgbe_ethdev.c | 9 +++++ >> drivers/net/bnx2x/bnx2x_ethdev.c | 4 ++ >> drivers/net/bnxt/bnxt_ethdev.c | 6 +++ >> drivers/net/bonding/rte_eth_bond_pmd.c | 6 +++ >> drivers/net/cnxk/cnxk_ethdev.c | 5 +++ >> drivers/net/cnxk/cnxk_ethdev_ops.c | 3 ++ >> drivers/net/cpfl/cpfl_ethdev.c | 6 +++ >> drivers/net/cxgbe/cxgbe_ethdev.c | 9 ++++- >> drivers/net/dpaa/dpaa_ethdev.c | 7 ++++ >> drivers/net/dpaa2/dpaa2_ethdev.c | 7 ++++ >> drivers/net/ena/ena_rss.c | 3 ++ >> drivers/net/enic/enic_ethdev.c | 1 + >> drivers/net/enic/enic_main.c | 3 ++ >> drivers/net/fm10k/fm10k_ethdev.c | 9 ++++- >> drivers/net/hinic/hinic_pmd_ethdev.c | 3 ++ >> drivers/net/hinic/hinic_pmd_rx.c | 3 ++ >> drivers/net/hns3/hns3_rss.c | 47 ++++++++++++--------- >> drivers/net/i40e/i40e_ethdev.c | 7 ++++ >> drivers/net/iavf/iavf_ethdev.c | 6 +++ >> drivers/net/ice/ice_dcf.c | 3 ++ >> drivers/net/ice/ice_dcf_ethdev.c | 3 ++ >> drivers/net/ice/ice_ethdev.c | 7 ++++ >> drivers/net/idpf/idpf_ethdev.c | 6 +++ >> drivers/net/igc/igc_ethdev.c | 4 ++ >> drivers/net/igc/igc_txrx.c | 5 +++ >> drivers/net/ionic/ionic_ethdev.c | 6 +++ >> drivers/net/ixgbe/ixgbe_ethdev.c | 12 +++++- >> drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++ >> drivers/net/mana/mana.c | 11 ++++- >> drivers/net/mlx5/mlx5_ethdev.c | 4 ++ >> drivers/net/mlx5/mlx5_rss.c | 3 +- >> drivers/net/mvpp2/mrvl_ethdev.c | 3 ++ >> drivers/net/netvsc/hn_ethdev.c | 6 +++ >> drivers/net/nfp/nfp_common.c | 9 ++++- >> drivers/net/ngbe/ngbe_ethdev.c | 6 ++- >> drivers/net/ngbe/ngbe_rxtx.c | 3 ++ >> drivers/net/null/rte_eth_null.c | 8 ++++ >> drivers/net/qede/qede_ethdev.c | 9 ++++- >> drivers/net/sfc/sfc_ethdev.c | 3 ++ >> drivers/net/sfc/sfc_rx.c | 3 ++ >> drivers/net/tap/rte_eth_tap.c | 8 ++++ >> drivers/net/thunderx/nicvf_ethdev.c | 10 ++++- >> drivers/net/txgbe/txgbe_ethdev.c | 7 +++- >> drivers/net/txgbe/txgbe_ethdev_vf.c | 7 +++- >> drivers/net/txgbe/txgbe_rxtx.c | 3 ++ >> lib/ethdev/rte_ethdev.c | 17 ++++++++ >> lib/ethdev/rte_ethdev.h | 56 +++++++++++++++++++------- >> lib/ethdev/rte_flow.c | 1 - >> lib/ethdev/rte_flow.h | 25 +----------- >> 55 files changed, 395 insertions(+), 106 deletions(-) > > Changing all drivers is suspicious. > It shows that something is missing in ethdev. > Please can you add the checks in ethdev only? >
That is kind of request from me, let me try to summarize what is going on, there is a new config item added to "struct rte_eth_rss_conf" introduced in this set, which is RSS hashing algorithm to use. Problem is none of the existing drivers are taking account this new config item, so application will request it but drivers silently ignore it. This is a generic problem when adding a new config item to existing config struct. So my request was if drivers not supporting it, and it is requested by the application, driver should return an error to let application know that it is not supported, that is why bunch of drivers updated. One option can be adding a new, specific API and dev_ops for this, for this case new config item is related to the existing RSS API, so I think it can be part of the existing API. Other can be to have some kind of capability reporting by drivers, and application will detect it and won't request this new config item, I think Stephen already suggested something like this. This capability flag is again a generic requirement, and `rte_eth_dev_info_get()` partially used for this purpose. I think it will be odd from application perspective to have a capability for just one config item of a feature set. Anyway, I think updating drivers to report they are not supporting new config item is best option to me, but also I think we should discuss this capability reporting in ethdev in a wider context.
