Re: [PATCH -next] RDMA/srpt: Fix error return code in srpt_cm_req_recv()
On 4/8/21 4:31 AM, Wang Wensheng wrote: > Fix to return a negative error code from the error handling > case instead of 0, as done elsewhere in this function. > > Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") > Reported-by: Hulk Robot > Signed-off-by: Wang Wensheng > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c > b/drivers/infiniband/ulp/srpt/ib_srpt.c > index 98a393d..ea44780 100644 > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const > sdev, > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not > enabled\n", > dev_name(>device->dev), port_num); > mutex_unlock(>mutex); > + ret = -EINVAL; > goto reject; > } Reviewed-by: Bart Van Assche
Re: [PATCH -next] RDMA/srpt: Fix error return code in srpt_cm_req_recv()
On 4/12/21 11:09 AM, Jason Gunthorpe wrote: > On Thu, Apr 08, 2021 at 09:50:30AM -0700, Bart Van Assche wrote: >> On 4/8/21 4:31 AM, Wang Wensheng wrote: >>> Fix to return a negative error code from the error handling >>> case instead of 0, as done elsewhere in this function. >>> >>> Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") >>> Reported-by: Hulk Robot >>> Signed-off-by: Wang Wensheng >>> drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c >>> b/drivers/infiniband/ulp/srpt/ib_srpt.c >>> index 98a393d..ea44780 100644 >>> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c >>> @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const >>> sdev, >>> pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not >>> enabled\n", >>> dev_name(>device->dev), port_num); >>> mutex_unlock(>mutex); >>> + ret = -EINVAL; >>> goto reject; >>> } >> >> Please fix the Hulk Robot. The following code occurs three lines above >> the modified code: >> >> ret = -EINVAL; > > That is a different if. > > The patch looks correct to me, please check again: > > ret = srpt_create_ch_ib(ch); > if (ret) { > // Ret is proven to be 0 > > [..] > > if (!sport->enabled) { > rej->reason = cpu_to_be32( > SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES); > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not > enabled\n", > dev_name(>device->dev), port_num); > goto reject; // Ret is 0 > > If there is an assignment to ret between those two blocks it is hidden.. Right, I was looking at another if-statement in the same function. Bart.
Re: [PATCH -next] RDMA/srpt: Fix error return code in srpt_cm_req_recv()
On Thu, Apr 08, 2021 at 09:50:30AM -0700, Bart Van Assche wrote: > On 4/8/21 4:31 AM, Wang Wensheng wrote: > > Fix to return a negative error code from the error handling > > case instead of 0, as done elsewhere in this function. > > > > Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") > > Reported-by: Hulk Robot > > Signed-off-by: Wang Wensheng > > drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c > > b/drivers/infiniband/ulp/srpt/ib_srpt.c > > index 98a393d..ea44780 100644 > > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > > @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const > > sdev, > > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not > > enabled\n", > > dev_name(>device->dev), port_num); > > mutex_unlock(>mutex); > > + ret = -EINVAL; > > goto reject; > > } > > Please fix the Hulk Robot. The following code occurs three lines above > the modified code: > > ret = -EINVAL; That is a different if. The patch looks correct to me, please check again: ret = srpt_create_ch_ib(ch); if (ret) { // Ret is proven to be 0 [..] if (!sport->enabled) { rej->reason = cpu_to_be32( SRP_LOGIN_REJ_INSUFFICIENT_RESOURCES); pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not enabled\n", dev_name(>device->dev), port_num); goto reject; // Ret is 0 If there is an assignment to ret between those two blocks it is hidden.. Jason
Re: [PATCH -next] RDMA/srpt: Fix error return code in srpt_cm_req_recv()
On 4/8/21 4:31 AM, Wang Wensheng wrote: > Fix to return a negative error code from the error handling > case instead of 0, as done elsewhere in this function. > > Fixes: db7683d7deb2 ("IB/srpt: Fix login-related race conditions") > Reported-by: Hulk Robot > Signed-off-by: Wang Wensheng > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c > b/drivers/infiniband/ulp/srpt/ib_srpt.c > index 98a393d..ea44780 100644 > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > @@ -2382,6 +2382,7 @@ static int srpt_cm_req_recv(struct srpt_device *const > sdev, > pr_info("rejected SRP_LOGIN_REQ because target %s_%d is not > enabled\n", > dev_name(>device->dev), port_num); > mutex_unlock(>mutex); > + ret = -EINVAL; > goto reject; > } Please fix the Hulk Robot. The following code occurs three lines above the modified code: ret = -EINVAL; Thanks, Bart.