On Mon, Jun 21, 2021 at 02:30:53AM +0300, Dmitry Kozlyuk wrote:
> 2021-05-05 12:12 (UTC-0700), Jie Zhou:
> > Function print_fdir_mask and print_fdir_flex_payload is only called
> > when either i40e or ixgbe presents. Add #if defined to remove
> > "unused function" compilation warning.
> >
> > Signed-off-by: Jie Zhou <j...@microsoft.com>
> > Signed-off-by: Jie Zhou <j...@linux.microsoft.com>
> > Acked-by: Tal Shnaiderman <tal...@nvidia.com>
> > ---
> > app/test-pmd/config.c | 82 +++++++++++++++++++++----------------------
> > 1 file changed, 41 insertions(+), 41 deletions(-)
>
> Code inside #ifdef isn't compile-checked, it's better to avoid.
> The only case we can't is when i40e or ixgbe API is called directly.
> I'd rather remove #ifdef whenever possible and mark maybe-unused entities,
> like this:
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 3723317ab4..97a577fec0 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -4488,8 +4488,6 @@ flowtype_to_str(uint16_t flow_type)
> return NULL;
> }
>
> -#if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE)
> -
> static inline void
> print_fdir_mask(struct rte_eth_fdir_masks *mask)
> {
> @@ -4590,6 +4588,9 @@ get_fdir_info(portid_t port_id, struct
> rte_eth_fdir_info *fdir_info,
> {
> int ret = -ENOTSUP;
>
> + RTE_SET_USED(fdir_info);
> + RTE_SET_USED(fdir_stat);
> +
> #ifdef RTE_NET_I40E
> if (ret == -ENOTSUP) {
> ret = rte_pmd_i40e_get_fdir_info(port_id, fdir_info);
> @@ -4686,8 +4687,6 @@ fdir_get_infos(portid_t port_id)
> fdir_stats_border, fdir_stats_border);
> }
>
> -#endif /* RTE_NET_I40E || RTE_NET_IXGBE */
> -
> void
> fdir_set_flex_mask(portid_t port_id, struct rte_eth_fdir_flex_mask *cfg)
> {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index d61a055bdd..a40ee902e8 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -917,9 +917,7 @@ int all_ports_stopped(void);
> int port_is_stopped(portid_t port_id);
> int port_is_started(portid_t port_id);
> void pmd_test_exit(void);
> -#if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE)
> void fdir_get_infos(portid_t port_id);
> -#endif
> void fdir_set_flex_mask(portid_t port_id,
> struct rte_eth_fdir_flex_mask *cfg);
> void fdir_set_flex_payload(portid_t port_id,
Hi Dmitry, I agree that should avoid the #ifdef as much as possible. But in
this case, I am not quite sure if I followed your comment correctly. Someone
originally introduced these i40e and ixgbe related fdir functions
(print_fdir_mask, print_fdir_flex_payload, print_fdir_flex_mask,
print_fdir_flow_type, get_fdir_info, fdir_get_infos) into testpmd with adding
the #if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE) for 4 out of 6
functions and left 2 of them outside the #ifdef which caused compilation
"unused function" warning. What I did here is just move the starting point of
#ifdef to also include those 2 missed functions (print_fdir_mask and
print_fdir_flex_payload). IMO the original author would be in better place to
reducing the unneccary #ifdef in a proper way.
