On Tue, May 12, 2020 at 01:29:18AM +0300, Kamal Heib wrote:
> Avoid disabling device management for devices that don't support
> Management datagrams (MADs) by checking if the "mad_agent" pointer is
> initialized before calling ib_modify_port, also change the error message
> to a warning and make it more informative.
>
> Fixes: 09f8a1486dca ("RDMA/srpt: Fix handling of SR-IOV and iWARP ports")
> Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> drivers/infiniband/ulp/srpt/ib_srpt.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index 7ed38d1cb997..7b21792ab6f7 100644
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -625,14 +625,18 @@ static void srpt_unregister_mad_agent(struct srpt_device *sdev)
> .clr_port_cap_mask = IB_PORT_DEVICE_MGMT_SUP,
> };
> struct srpt_port *sport;
> + int ret;
> int i;
>
> for (i = 1; i <= sdev->device->phys_port_cnt; i++) {
> sport = &sdev->port[i - 1];
> WARN_ON(sport->port != i);
> - if (ib_modify_port(sdev->device, i, 0, &port_modify) < 0)
> - pr_err("disabling MAD processing failed.\n");
> if (sport->mad_agent) {
> + ret = ib_modify_port(sdev->device, i, 0, &port_modify);
> + if (ret < 0)
> + pr_warn("%s-%d: disabling device management failed (%d). Note: this is expected if SR-IOV is enabled.\n",
> + dev_name(&sport->sdev->device->dev),
> + sport->port, ret);
This ib_modify_port needs to be strictly paired with the
ib_modify_port that turns on IB_PORT_DEVICE_MGMT_SUP.
If the call during register fails then we should not try to do it
during unregister.
So this is sort of the right approach, but the error unwind in
srpt_refresh_port() needs to be fixed too so that sport->mad_agent
indicates if modify_port is needed on unregister.
And the print is wrong, if something fails here it means the driver is
busted up as we should always be able to undo setting
IB_PORT_DEVICE_MGMT_SUP if it was successfully set.
Jason
