[dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API calling

[Iremonger, Bernard]

Hi Wenzhuo,

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Thursday, January 12, 2017 1:09 AM
> To: Iremonger, Bernard <bernard.iremonger at intel.com>; dev at dpdk.org
> Cc: Wu, Jingjing <jingjing.wu at intel.com>; stable at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API calling
> 
> Hi Bernard,
> 
> > -----Original Message-----
> > From: Iremonger, Bernard
> > Sent: Wednesday, January 11, 2017 6:27 PM
> > To: Lu, Wenzhuo; dev at dpdk.org
> > Cc: Wu, Jingjing; stable at dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API
> > calling
> >
> > Hi Wenzhuo,
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu
> > > Sent: Wednesday, January 11, 2017 2:48 AM
> > > To: dev at dpdk.org
> > > Cc: Wu, Jingjing <jingjing.wu at intel.com>; Lu, Wenzhuo
> > > <wenzhuo.lu at intel.com>; stable at dpdk.org
> > > Subject: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API
> > > calling
> > >
> > > Some ixgbe private APIs are added to expose ixgbe specific functions.
> > > When they're used by testpmd, there's no check for if the NICs are ixgbe.
> > > Other NICs also have chance to  call these APIs.
> > > This patch add the check and the feedback print.
> >
> > I am not sure that testpmd is the right place to do this.
> > The rte_pmd_ixgbe_* functions are public API's which can be called by
> > other applications.
> > The checks should be in the rte_pmd_ixgbe_* API's
> To be safer, it's better to add a check in the APIs.

I have already sent a patch for ixgbe which Ferruh has reviewed and applied to dpdk-next-net
http://dpdk.org/dev/patchwork/patch/19151/

We should consider doing something similar for the i40e.

> But the APIs is so called private API, not really public.  Considering if we have
> the same function on different NICs, for example we have rte_pmd_ixgbe_a
> and rte_pmd_i40e_a.
> APP still need to call them one by one, like ret = rte_pmd_ixgbe_a; ret =
> rte_pmd_i40e_a;
> 
> then, why not add the check, like
> If (NIC is ixgbe)
> 	ret = rte_pmd_ixgbe_a;
> if (NIC is i40e)
> 	ret = rte_pmd_i40e_a;

There is already code like the above in testpmd in cases where there is a choice of rte_pmd_* functions to call.
 
> testpmd is an example to let the users to know how to use the APIs. They
> should follow the example.
> How about your opinion?

In the case where there is no choice of function to make. There is no need to check the NIC in testpmd (as it is now done in the rte_pmd_ixgbe_* API's).

Regards,

Bernard.