26/03/2021 16:37, Ferruh Yigit:

On 3/25/2021 10:20 AM, Thomas Monjalon wrote:

quoted

25/03/2021 11:00, Ferruh Yigit:

quoted

On 3/25/2021 5:53 AM, Andrew Rybchenko wrote:

quoted

On 3/24/21 11:00 PM, Thomas Monjalon wrote:

quoted

24/03/2021 19:08, Ferruh Yigit:

quoted

On 3/21/2021 9:00 AM, Thomas Monjalon wrote:

quoted

The header file rte_eth_ctrl.h should not be needed because
this legacy filtering API is completely replaced with the rte_flow API.
However some definitions from this file are still used by some drivers,
but such usage is already covered by an implicit include via rte_ethdev.h.

Signed-off-by: Thomas Monjalon <redacted>
Acked-by: Rosen Xu <redacted>
Acked-by: Hemant Agrawal <redacted>
---
    drivers/net/dpaa2/dpaa2_ptp.c       | 1 -
    drivers/net/iavf/iavf_hash.c        | 1 -
    drivers/net/ice/ice_acl_filter.c    | 1 -
    drivers/net/ice/ice_hash.c          | 1 -
    drivers/net/ice/ice_switch_filter.c | 1 -
    drivers/net/igc/igc_filter.h        | 1 -
    drivers/net/ipn3ke/ipn3ke_flow.c    | 1 -

Although this will work, if the above drives are using the defines from the
header file, isn't it better to include it explicitly?

What is the benefit of including the header implicitly?

The benefit is to progressively remove rte_eth_ctrl.h.
I want it to disappear.

+1

This is just hiding its usage, the patch is not making it less used as a step
forward to remove it.

Yes you're right. The only step forward is esthetic:
hiding something which should be removed.
And maybe some of these files don't need the include at all.

quoted

But anyway I guess it doesn't worth spending more time to discuss it ...

Feel free to reject if you feel it is not a good step.

What do you think doing exact opposite,

remove "#include <rte_eth_ctrl.h>" from 'rte_ethdev.h',
and include 'rte_eth_ctrl.h' explicitly where ever it is required,

this can make more clear what components use the 'rte_eth_ctrl.h' and why, which 
may help clearing them to remove the header. Plus it highlights if a new PMD 
wants to use the header, we can catch it easier.

When I tried above approach, it highlighted that not only drivers, libraries 
also using this header, 'librte_ehtdev' & 'librte_flow_classify'.
And for 'ethdev', the structs defined in the 'rte_eth_ctrl.h' are part of public 
structs, so it is hard to remove them.
Some PMD specific APIs also needs 'rte_eth_ctrl.h' header, but that is hidden 
because of the implicit include, but again some structs in the 'rte_eth_ctrl.h' 
are part of public APIs (although they are experimental).

Also, it turned out that same required headers in the drivers are hidden because 
of this implicit include in 'rte_ethdev.h', I will send a fix for it soon.

OK thanks