Re: [dpdk-dev] [PATCH v13 09/10] app/testpmd: fix unused function warnings

[Jie Zhou <jizh@linux.microsoft.com>]

On Thu, Jun 24, 2021 at 09:44:04PM +0300, Dmitry Kozlyuk wrote:
> 2021-06-24 08:45 (UTC-0700), Tyler Retzlaff:
> > On Wed, Jun 23, 2021 at 02:26:32PM -0700, Jie Zhou wrote:
> > > On Mon, Jun 21, 2021 at 02:30:53AM +0300, Dmitry Kozlyuk wrote:  
> > > > -
> > > >  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.  
> > 
> > i think i have to agree with jie here. there are limits to how many
> > defects we should have to correct which are unrelated change. if this is
> > critical i think it would be best if the maintainer provide a patch
> > cleaning up the code they own.
> > 
> > let's not hold this patch up over it because of it being a broad change
> > we lose a lot of time rebasing where either the maintainer or author
> > could follow up with a narrow change to correct this.
> 
> Fair enough, the patch doesn't add technical debt at least.
> 
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Thanks Dmitry. Can you please help Ack on V14 which I sent out yesterday? Otherwise, I will send out V15 to carry over this ack from this V13.