Hi Anatoly,
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.bura...@intel.com>
> Sent: Wednesday, May 6, 2020 18:27
> To: Wang, Haiyue <haiyue.w...@intel.com>; dev@dpdk.org; tho...@monjalon.net;
> jer...@marvell.com;
> david.march...@redhat.com
> Subject: Re: [PATCH v12 2/2] eal: support for VFIO-PCI VF token
>
> On 05-May-20 11:34 AM, Haiyue Wang wrote:
> > The kernel module vfio-pci introduces the VF token to enable SR-IOV
> > support since 5.7.
> >
> > The VF token can be set by a vfio-pci based PF driver and must be known
> > by the vfio-pci based VF driver in order to gain access to the device.
> >
> > Signed-off-by: Haiyue Wang <haiyue.w...@intel.com>
> > ---
>
> <snip>
>
> > diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
> > index 540b7d38c..00b02855e 100644
> > --- a/lib/librte_eal/freebsd/eal.c
> > +++ b/lib/librte_eal/freebsd/eal.c
> > @@ -1002,6 +1002,11 @@ rte_eal_vfio_intr_mode(void)
> > return RTE_INTR_MODE_NONE;
> > }
> >
> > +void rte_eal_vfio_vf_token(rte_uuid_t vf_token)
> > +{
> > + memset(vf_token, 0, sizeof(rte_uuid_t));
> > +}
>
> What's the purpose of memset(0) here? Presumably, if the API is not
> supposed to be supported, the function should have no effect?
>
Yes, originally I meant to no parse the input, then return zero UUID.
I changed it to the same as Linux:
rte_uuid_copy(vf_token, internal_config.vfio_vf_token);
Since it is inited to zero.
> > +
> > int rte_vfio_setup_device(__rte_unused const char *sysfs_base,
> > __rte_unused const char *dev_addr,
> > __rte_unused int *vfio_dev_fd,
> > diff --git a/lib/librte_eal/include/rte_eal.h
> > b/lib/librte_eal/include/rte_eal.h
> > index 2f9ed298d..d9040ce14 100644
> > --- a/lib/librte_eal/include/rte_eal.h
> > +++ b/lib/librte_eal/include/rte_eal.h
> > @@ -21,6 +21,7 @@
> > #include <rte_bus.h>
> >
> > #include <rte_pci_dev_feature_defs.h>
> > +#include <rte_uuid.h>
> >
> > #ifdef __cplusplus
> > extern "C" {
> > @@ -438,6 +439,17 @@ int rte_eal_create_uio_dev(void);
> > */
> > enum rte_intr_mode rte_eal_vfio_intr_mode(void);
> >
> > +
> > +/**
> > + * The user-configured vfio VF token.
>
> The description is unclear. Suggested rewording:
>
> Copy the user-configured vfio VF token.
>
+1.
> > + *
> > + * @param vf_token
> > + * vfio VF token configured with the command line is copied
> > + * into this parameter, zero uuid by default.
> > + */
> > +__rte_experimental
> > +void rte_eal_vfio_vf_token(rte_uuid_t vf_token);
>
> Maybe rte_eal_vfio_get_vf_token()? I imagine this would be a more
> descriptive name for this API.
>
Yes, this looks better. Fixed.
> Once all of the above is addressed (or explained),
>
> Acked-by: Anatoly Burakov <anatoly.bura...@intel.com>
>
> --
> Thanks,
> Anatoly
