From: Maxime Coquelin
> Sent: Monday, February 3, 2020 1:00 PM
> To: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; Slava Ovsiienko
> <viachesl...@mellanox.com>
> Subject: Re: [PATCH v3 13/13] vdpa/mlx5: disable ROCE
> 
> 
> 
> On 2/3/20 10:27 AM, Maxime Coquelin wrote:
> > Hi Matan,
> >
> > On 2/2/20 5:03 PM, Matan Azrad wrote:
> >> In order to support virtio queue creation by the FW, ROCE mode should
> >> be disabled in the device.
> >>
> >> Do it by netlink which is like the devlink tool commands:
> >>    1. devlink dev param set pci/[pci] name enable_roce value false
> >>       cmode driverinit
> >>            2. devlink dev reload pci/[pci]
> >> Or by sysfs which is like:
> >>    echo 0 >  /sys/bus/pci/devices/[pci]/roce_enable
> >>
> >> The IB device is matched again after ROCE disabling.
> >>
> >> Signed-off-by: Matan Azrad <ma...@mellanox.com>
> >> Acked-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> >> Acked-by: Maxime Coquelin <maxime.coque...@redhat.com>
> >> ---
> >>  drivers/vdpa/mlx5/Makefile    |   2 +-
> >>  drivers/vdpa/mlx5/meson.build |   2 +-
> >>  drivers/vdpa/mlx5/mlx5_vdpa.c | 191
> >> ++++++++++++++++++++++++++++++++++--------
> >>  3 files changed, 160 insertions(+), 35 deletions(-)
> > ...
> >> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c
> >> b/drivers/vdpa/mlx5/mlx5_vdpa.c index 57619d2..710f305 100644
> >> --- a/drivers/vdpa/mlx5/mlx5_vdpa.c
> >> +++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
> >
> > ...
> >
> >> @@ -246,8 +389,7 @@
> >>  mlx5_vdpa_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
> >>                struct rte_pci_device *pci_dev __rte_unused)  {
> >> -  struct ibv_device **ibv_list;
> >> -  struct ibv_device *ibv_match = NULL;
> >> +  struct ibv_device *ibv;
> >>    struct mlx5_vdpa_priv *priv = NULL;
> >>    struct ibv_context *ctx = NULL;
> >>    struct mlx5_hca_attr attr;
> >> @@ -258,42 +400,25 @@
> >>                    " driver.");
> >>            return 1;
> >>    }
> >> -  errno = 0;
> >> -  ibv_list = mlx5_glue->get_device_list(&ret);
> >> -  if (!ibv_list) {
> >> -          rte_errno = ENOSYS;
> >> -          DRV_LOG(ERR, "Failed to get device list, is ib_uverbs
> loaded?");
> >> +  ibv = mlx5_vdpa_get_ib_device_match(&pci_dev->addr);
> >> +  if (!ibv) {
> >> +          DRV_LOG(ERR, "No matching IB device for PCI slot "
> >> +                  PCI_PRI_FMT ".", pci_dev->addr.domain,
> >> +                  pci_dev->addr.bus, pci_dev->addr.devid,
> >> +                  pci_dev->addr.function);
> >>            return -rte_errno;
> >> -  }
> >> -  while (ret-- > 0) {
> >> -          struct rte_pci_addr pci_addr;
> >> -
> >> -          DRV_LOG(DEBUG, "Checking device \"%s\"..", ibv_list[ret]-
> >name);
> >> -          if (mlx5_dev_to_pci_addr(ibv_list[ret]->ibdev_path,
> &pci_addr))
> >> -                  continue;
> >> -          if (pci_dev->addr.domain != pci_addr.domain ||
> >> -              pci_dev->addr.bus != pci_addr.bus ||
> >> -              pci_dev->addr.devid != pci_addr.devid ||
> >> -              pci_dev->addr.function != pci_addr.function)
> >> -                  continue;
> >> +  } else {
> >>            DRV_LOG(INFO, "PCI information matches for device
> \"%s\".",
> >> -                  ibv_list[ret]->name);
> >> -          ibv_match = ibv_list[ret];
> >> -          break;
> >> +                  ibv->name);
> >>    }
> >> -  mlx5_glue->free_device_list(ibv_list);
> >> -  if (!ibv_match) {
> >> -          DRV_LOG(ERR, "No matching IB device for PCI slot "
> >> -                  "%" SCNx32 ":%" SCNx8 ":%" SCNx8 ".%" SCNx8 ".",
> >> -                  pci_dev->addr.domain, pci_dev->addr.bus,
> >> -                  pci_dev->addr.devid, pci_dev->addr.function);
> >> -          rte_errno = ENOENT;
> >> -          return -rte_errno;
> >> +  if (mlx5_vdpa_roce_disable(&pci_dev->addr, &ibv) != 0) {
> >> +          DRV_LOG(WARNING, "Failed to disable ROCE for \"%s\".",
> >> +                  ibv->name);
> >> +          //return -rte_errno;
> >>    }
> >
> > Is that commented return expected?
> >
> 
> Please let me know if I should remove the comment, or remove the return.

Sorry, forgot the comment , good catch!
It should not be comment just need to remove "//".

Can you do it in integration?

> 
> Thanks,
> Maxime

Reply via email to