Re: [PATCH rdma-next 1/2] IB/core: Query IBoE link speed with a new driver API

From: Jason Gunthorpe <jgg@nvidia.com>
To: Mark Zhang <markzhang@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>, linux-rdma@vger.kernel.org
Subject: Re: [PATCH rdma-next 1/2] IB/core: Query IBoE link speed with a new driver API
Date: Tue, 11 Apr 2023 08:20:39 -0300	[thread overview]
Message-ID: <ZDVCh34IIvv8PNCB@nvidia.com> (raw)
In-Reply-To: <a7be2148-b17b-816e-b28b-1837ebcb11a1@nvidia.com>

On Tue, Apr 11, 2023 at 07:42:37AM +0800, Mark Zhang wrote:
> On 4/11/2023 7:23 AM, Jason Gunthorpe wrote:
> > On Mon, Apr 10, 2023 at 04:12:06PM +0300, Leon Romanovsky wrote:
> > > From: Mark Zhang <markzhang@nvidia.com>
> > > 
> > > Currently the ethtool API is used to get IBoE link speed, which must be
> > > protected with the rtnl lock. This becomes a bottleneck when try to setup
> > > many rdma-cm connections at the same time, especially with multiple
> > > processes.
> > > 
> > > In order to avoid it, a new driver API is introduced to query the IBoE
> > > rate. It will be used firstly, and back to ethtool way if it fails.
> > > 
> > > Signed-off-by: Mark Zhang <markzhang@nvidia.com>
> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > ---
> > >   drivers/infiniband/core/cma.c    |  6 ++++--
> > >   drivers/infiniband/core/device.c |  1 +
> > >   include/rdma/ib_addr.h           | 31 ++++++++++++++++++++-----------
> > >   include/rdma/ib_verbs.h          |  3 +++
> > >   4 files changed, 28 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> > > index 9c7d26a7d243..ff706d2e39c6 100644
> > > --- a/drivers/infiniband/core/cma.c
> > > +++ b/drivers/infiniband/core/cma.c
> > > @@ -3296,7 +3296,8 @@ static int cma_resolve_iboe_route(struct rdma_id_private *id_priv)
> > >   	route->path_rec->traffic_class = tos;
> > >   	route->path_rec->mtu = iboe_get_mtu(ndev->mtu);
> > >   	route->path_rec->rate_selector = IB_SA_EQ;
> > > -	route->path_rec->rate = iboe_get_rate(ndev);
> > > +	route->path_rec->rate = iboe_get_rate(ndev, id_priv->id.device,
> > > +					      id_priv->id.port_num);
> > >   	dev_put(ndev);
> > >   	route->path_rec->packet_life_time_selector = IB_SA_EQ;
> > >   	/* In case ACK timeout is set, use this value to calculate
> > > @@ -4962,7 +4963,8 @@ static int cma_iboe_join_multicast(struct rdma_id_private *id_priv,
> > >   	if (!ndev)
> > >   		return -ENODEV;
> > > -	ib.rec.rate = iboe_get_rate(ndev);
> > > +	ib.rec.rate = iboe_get_rate(ndev, id_priv->id.device,
> > > +				    id_priv->id.port_num);
> > >   	ib.rec.hop_limit = 1;
> > >   	ib.rec.mtu = iboe_get_mtu(ndev->mtu);
> > 
> > What do we even use rate for in roce mode in the first place?
> > 
> In mlx5 it is set to "address_path.stat_rate", but I believe we always set 0
> for roce actually. Not sure about other devices?

"rate" is to reduce the packet rate of connections, it should always
be 0 for roce, AFAIK.

Maybe we should look into making that the case instead of this?

Jason