[PATCH for-next] RDMA/cma: Fix incorrect Packet Lifetime calculation

[PATCH for-next] RDMA/cma: Fix incorrect Packet Lifetime calculation

[Håkon Bugge]

An approximation for the PacketLifeTime is half the local ACK timeout.
The encoding for both timers are logarithmic. The PacketLifeTime
calculation is wrong when local ACK timeout is zero. In that case,
PacketLifeTime is set to the incorrect value 255.

Fixed by explicitly testing for timeout being zero.

Fixes: e1ee1e62bec4 ("RDMA/cma: Use ACK timeout for RoCE packetLifeTime")
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>

---

	* Note: This commit must be merged after ("RDMA/cma: Replace
          RMW with atomic bit-ops")
---
 drivers/infiniband/core/cma.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 6759889..b1512ca 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -3096,9 +3096,11 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
 	 * PacketLifeTime = local ACK timeout/2
 	 * as a reasonable approximation for RoCE networks.
 	 */
-	route->path_rec->packet_life_time =
-		test_bit(TIMEOUT_SET, &id_priv->flags) ?
-		id_priv->timeout - 1 : CMA_IBOE_PACKET_LIFETIME;
+	if (test_bit(TIMEOUT_SET, &id_priv->flags))
+		route->path_rec->packet_life_time =
+			id_priv->timeout ? id_priv->timeout - 1 : 0;
+	else
+		route->path_rec->packet_life_time = CMA_IBOE_PACKET_LIFETIME;
 
 	if (!route->path_rec->mtu) {
 		ret = -EINVAL;
-- 
1.8.3.1

Re: [PATCH for-next] RDMA/cma: Fix incorrect Packet Lifetime calculation

[Leon Romanovsky]

On Mon, Jun 21, 2021 at 03:18:57PM +0200, Håkon Bugge wrote:
> An approximation for the PacketLifeTime is half the local ACK timeout.
> The encoding for both timers are logarithmic. The PacketLifeTime
> calculation is wrong when local ACK timeout is zero. In that case,
> PacketLifeTime is set to the incorrect value 255.
> 
> Fixed by explicitly testing for timeout being zero.
> 
> Fixes: e1ee1e62bec4 ("RDMA/cma: Use ACK timeout for RoCE packetLifeTime")
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> 
> ---
> 
> 	* Note: This commit must be merged after ("RDMA/cma: Replace
>           RMW with atomic bit-ops")
> ---
>  drivers/infiniband/core/cma.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Re: [PATCH for-next] RDMA/cma: Fix incorrect Packet Lifetime calculation

[Haakon Bugge]

> On 22 Jun 2021, at 10:18, Leon Romanovsky <leon@kernel.org> wrote:
> 
> On Mon, Jun 21, 2021 at 03:18:57PM +0200, Håkon Bugge wrote:
>> An approximation for the PacketLifeTime is half the local ACK timeout.
>> The encoding for both timers are logarithmic. The PacketLifeTime
>> calculation is wrong when local ACK timeout is zero. In that case,
>> PacketLifeTime is set to the incorrect value 255.
>> 
>> Fixed by explicitly testing for timeout being zero.
>> 
>> Fixes: e1ee1e62bec4 ("RDMA/cma: Use ACK timeout for RoCE packetLifeTime")
>> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
>> 
>> ---
>> 
>> 	* Note: This commit must be merged after ("RDMA/cma: Replace
>>          RMW with atomic bit-ops")
>> ---
>> drivers/infiniband/core/cma.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>> 
> 
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>

Thanks for the review, Leon! I have to rebase on the tip of for-next, since the ("RDMA/cma: Replace RMW with atomic bit-ops") will not have the get_bit() stuff in cma_resolve_iboe_route() anymore. I assume I can retain your r-b after the rebase?


Thxs, Håkon

Re: [PATCH for-next] RDMA/cma: Fix incorrect Packet Lifetime calculation

[Leon Romanovsky]

On Tue, Jun 22, 2021 at 09:32:27AM +0000, Haakon Bugge wrote:
> 
> 
> > On 22 Jun 2021, at 10:18, Leon Romanovsky <leon@kernel.org> wrote:
> > 
> > On Mon, Jun 21, 2021 at 03:18:57PM +0200, Håkon Bugge wrote:
> >> An approximation for the PacketLifeTime is half the local ACK timeout.
> >> The encoding for both timers are logarithmic. The PacketLifeTime
> >> calculation is wrong when local ACK timeout is zero. In that case,
> >> PacketLifeTime is set to the incorrect value 255.
> >> 
> >> Fixed by explicitly testing for timeout being zero.
> >> 
> >> Fixes: e1ee1e62bec4 ("RDMA/cma: Use ACK timeout for RoCE packetLifeTime")
> >> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> >> 
> >> ---
> >> 
> >> 	* Note: This commit must be merged after ("RDMA/cma: Replace
> >>          RMW with atomic bit-ops")
> >> ---
> >> drivers/infiniband/core/cma.c | 8 +++++---
> >> 1 file changed, 5 insertions(+), 3 deletions(-)
> >> 
> > 
> > Thanks,
> > Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> 
> Thanks for the review, Leon! I have to rebase on the tip of for-next, since the ("RDMA/cma: Replace RMW with atomic bit-ops") will not have the get_bit() stuff in cma_resolve_iboe_route() anymore. I assume I can retain your r-b after the rebase?

Yes, please.

> 
> 
> Thxs, Håkon
> 
> 
> 
>