Re: [PATCH v5 00/40] support setting and querying RSS algorithms

["lihuisong (C)" <lihuisong@huawei.com>]

在 2023/10/17 22:26, Ferruh Yigit 写道:
> 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.
IMO, it is more better to report RSS algorithm capability.
It can avoid the later ABI break successfully as Stephen said.
>
>
>
> .