> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@xilinx.com>
> Sent: Monday, September 5, 2022 11:42 PM
> To: Chaoyong He <chaoyong...@corigine.com>; dev@dpdk.org
> Cc: oss-drivers <oss-driv...@corigine.com>; Niklas Soderlund
> <niklas.soderl...@corigine.com>
> Subject: Re: [PATCH v7 05/12] net/nfp: add flower PF setup and mempool
> init logic
>
> On 8/12/2022 11:22 AM, Chaoyong He wrote:
> > Adds the vNIC initialization logic for the flower PF vNIC. The flower
> > firmware exposes this vNIC for the purposes of fallback traffic in the
> > switchdev use-case. The logic of setting up this vNIC is similar to
> > the logic seen in nfp_net_init() and nfp_net_start().
> >
> > Adds minimal dev_ops for this PF device. Because the device is being
> > exposed externally to DPDK it should also be configured using DPDK
> > helpers like rte_eth_configure(). For these helpers to work the flower
> > logic needs to implements a minimal set of dev_ops. The Rx and Tx
> > logic for this vNIC will be added in a subsequent commit.
> >
> > OVS expects incoming packets coming into the OVS datapath to be
> > allocated from a mempool that contains objects of type "struct
> > dp_packet". For the PF handling the slowpath into OVS it should use a
> > mempool that is compatible with OVS. This commit adds the logic to
> > create the OVS compatible mempool. It adds certain OVS specific
> > structs to be able to instantiate the mempool.
> >
>
> Can you please elaborate what is OVS compatible mempool?
>
> <...>
>
> > +static inline struct nfp_app_flower * nfp_app_flower_priv_get(struct
> > +nfp_pf_dev *pf_dev) {
> > + if (pf_dev == NULL)
> > + return NULL;
> > + else if (pf_dev->app_id != NFP_APP_FLOWER_NIC)
> > + return NULL;
> > + else
> > + return (struct nfp_app_flower *)pf_dev->app_priv; }
> > +
>
> What do you think to unify functions to get private data, instead of having a
> function for each FW, it can be possible to have single one?
>
At first, we use two macros for this, and Andrew advice change them to
functions.
```
#define NFP_APP_PRIV_TO_APP_NIC(app_priv)\
((struct nfp_app_nic *)app_priv)
#define NFP_APP_PRIV_TO_APP_FLOWER(app_priv)\
((struct nfp_app_flower *)app_priv)
```
So your advice is we unify the functions into:
```
static inline struct nfp_app_nic *
nfp_app_priv_get(struct nfp_pf_dev *pf_dev)
{
if (pf_dev == NULL)
return NULL;
else if (pf_dev->app_id == NFP_APP_CORE_NIC ||
pf_dev->app_id == NFP_APP_FLOWER_NIC)
return pf_dev->app_priv;
else
return NULL;
}
```
and convert the pointer type at where this function been called?
