[dpdk-dev] [PATCH v5 01/51] net/bnxt: add basic infrastructure for VF reps

[Ferruh Yigit]

On 7/6/2020 3:14 PM, Ajit Khaparde wrote:
> On Mon, Jul 6, 2020 at 7:05 AM Somnath Kotur <somnath.kotur at broadcom.com
> <mailto:somnath.kotur at broadcom.com>> wrote:
> 
>     On Mon, Jul 6, 2020 at 3:37 PM Ferruh Yigit <ferruh.yigit at intel.com
>     <mailto:ferruh.yigit at intel.com>> wrote:
>     >
>     > On 7/3/2020 10:01 PM, Ajit Khaparde wrote:
>     > > From: Somnath Kotur <somnath.kotur at broadcom.com
>     <mailto:somnath.kotur at broadcom.com>>
>     > >
>     > > Defines data structures and code to init/uninit
>     > > VF representors during pci_probe and pci_remove
>     > > respectively.
>     > > Most of the dev_ops for the VF representor are just
>     > > stubs for now and will be will be filled out in next patch.
>     > >
>     > > To create a representor using testpmd:
>     > > testpmd -c 0xff -wB:D.F,representor=1 -- -i
>     > > testpmd -c 0xff -w05:02.0,representor=[1] -- -i
>     > >
>     > > To create a representor using ovs-dpdk:
>     > > 1. Firt add the trusted VF port to a bridge
>     > > ovs-vsctl add-port ovsbr0 vf_rep1 -- set Interface vf_rep1 type=dpdk
>     > > options:dpdk-devargs=0000:06:02.0
>     > > 2. Add the representor port to the bridge
>     > > ovs-vsctl add-port ovsbr0 vf_rep1 -- set Interface vf_rep1 type=dpdk
>     > > options:dpdk-devargs=0000:06:02.0,representor=1
>     > >
>     > > Signed-off-by: Somnath Kotur <somnath.kotur at broadcom.com
>     <mailto:somnath.kotur at broadcom.com>>
>     > > Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru at broadcom.com
>     <mailto:venkatkumar.duvvuru at broadcom.com>>
>     > > Reviewed-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com
>     <mailto:kalesh-anakkur.purayil at broadcom.com>>
>     > > Reviewed-by: Ajit Khaparde <ajit.khaparde at broadcom.com
>     <mailto:ajit.khaparde at broadcom.com>>
>     >
>     > <...>
>     >
>     > >  static int bnxt_pci_remove(struct rte_pci_device *pci_dev)
>     > >  {
>     > > -     if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>     > > -             return rte_eth_dev_pci_generic_remove(pci_dev,
>     > > -                             bnxt_dev_uninit);
>     > > -     else
>     > > +     struct rte_eth_dev *eth_dev;
>     > > +
>     > > +     eth_dev = rte_eth_dev_allocated(pci_dev->device.name
>     <http://device.name>);
>     > > +     if (!eth_dev)
>     > > +             return ENODEV; /* Invoked typically only by OVS-DPDK, by the
>     > > +                             * time it comes here the eth_dev is already
>     > > +                             * deleted by rte_eth_dev_close(), so returning
>     > > +                             * +ve value will atleast help in proper
>     cleanup
>     > > +                             */
>     >
>     > Why returning a positive error value? It hides the error since the caller
>     of the
>     > function does a "< 0" check.
>     > Better to be more explicit and return '0' if an error is not intendent in
>     this case.
>     >
>     Sure, makes sense Ferruh
> 
> Ferruh,
> Do you want Som to send a fresh patchset?
> Or send a fix just this with a new patch?

I will update this as "return 0;" while merging, no new patchset required.