Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: add private APIs

["Di, ChenxuX" <chenxux.di@intel.com>]

> -----Original Message-----
> From: Yang, Qiming
> Sent: Tuesday, June 30, 2020 6:25 PM
> To: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>
> Subject: RE: [PATCH v2 1/5] net/i40e: add private APIs
> 
> 
> 
> > -----Original Message-----
> > From: Di, ChenxuX <chenxux.di@intel.com>
> > Sent: Monday, June 15, 2020 10:19
> > To: dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Di, ChenxuX <chenxux.di@intel.com>
> > Subject: [PATCH v2 1/5] net/i40e: add private APIs
> >
> > The legacy filter API will be superseded by rte_flow.
> > There are also several small features which can not be implemented in
> > rte_flow. This patch re-implemented these features as private API.
> > Three APIs are added:
> > rte_pmd_i40e_get_fdir_info.
> > rte_pmd_i40e_get_fdir_stats.
> > rte_pmd_i40e_set_gre_key_len.
> >
> > Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> > ---
> >  doc/guides/rel_notes/release_20_08.rst    | 14 +++++++
> >  drivers/net/i40e/i40e_ethdev.c            |  2 +-
> >  drivers/net/i40e/i40e_ethdev.h            |  5 +++
> >  drivers/net/i40e/i40e_fdir.c              |  4 +-
> >  drivers/net/i40e/rte_pmd_i40e.c           | 51 +++++++++++++++++++++++
> >  drivers/net/i40e/rte_pmd_i40e.h           | 49 ++++++++++++++++++++++
> >  drivers/net/i40e/rte_pmd_i40e_version.map |  3 ++
> >  7 files changed, 125 insertions(+), 3 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/release_20_08.rst
> > b/doc/guides/rel_notes/release_20_08.rst
> > index dee4ccbb5..550ad33aa 100644
> > --- a/doc/guides/rel_notes/release_20_08.rst
> > +++ b/doc/guides/rel_notes/release_20_08.rst
> > @@ -62,6 +62,20 @@ New Features
> >
> >    * Added new PMD devarg ``reclaim_mem_mode``.
> >
> > +* **Updated the Intel i40e driver.**
> > +
> > +  Updated the Intel i40e driver with new features and improvements,
> > including:
> > +
> > +  * Re-implemented get_fdir_info and get_fdir_stat in private API.
> > +  * Re-implemented set_gre_key_len in private API.
> > +  * Added support for flow query RSS.
> > +
> > +* **Updated the Intel ixgbe driver.**
> > +
> > +  Updated the Intel ixgbe driver with new features and improvements,
> > including:
> > +
> > +  * Re-implemented get_fdir_info and get_fdir_stat in private API.
> > +
> >
> >  Removed Items
> >  -------------
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 970a31cb2..674b711d9 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -8787,7 +8787,7 @@ i40e_tunnel_filter_param_check(struct i40e_pf
> > *pf,
> >
> >  #define I40E_GL_PRS_FVBM_MSK_ENA 0x80000000
> >  #define I40E_GL_PRS_FVBM(_i)     (0x00269760 + ((_i) * 4))
> > -static int
> > +int
> >  i40e_dev_set_gre_key_len(struct i40e_hw *hw, uint8_t len)  {  struct
> > i40e_pf *pf = &((struct i40e_adapter *)hw->back)->pf; diff -- git
> > a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> > index e5d0ce53f..192636927 100644
> > --- a/drivers/net/i40e/i40e_ethdev.h
> > +++ b/drivers/net/i40e/i40e_ethdev.h
> > @@ -1270,6 +1270,11 @@ enum i40e_filter_pctype  uint16_t flow_type);
> > uint16_t i40e_pctype_to_flowtype(const struct i40e_adapter *adapter,
> >   enum i40e_filter_pctype pctype);
> > +int i40e_dev_set_gre_key_len(struct i40e_hw *hw, uint8_t len); void
> > +i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info
> > +*fdir); void i40e_fdir_stats_get(struct rte_eth_dev *dev,  struct
> > +rte_eth_fdir_stats *stat);
> >  int i40e_fdir_ctrl_func(struct rte_eth_dev *dev,
> >    enum rte_filter_op filter_op,
> >    void *arg);
> 
> It doesn't make sense to add the definition here as these functions only called by
> private API.
> I think it better to move these function to private also
> 

For the reason the legacy filter API and function is not really removed, it will call the function.
It may not be the right time to move the functions to private.

The same as ixgbe.

> [...]
> 
> 
> > --
> > 2.17.1
>