| Message ID | 1624281537-5573-1-git-send-email-haakon.bugge@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-next] RDMA/cma: Fix incorrect Packet Lifetime calculation | expand |
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>
> 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
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 > > > >
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;
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(-)