diff mbox series

IB/ipoib: improve latency in ipoib/cm connection formation

Message ID 1611251403-12810-1-git-send-email-manjunath.b.patil@oracle.com (mailing list archive)
State Superseded
Delegated to: Jason Gunthorpe
Headers show
Series IB/ipoib: improve latency in ipoib/cm connection formation | expand

Commit Message

Manjunath Patil Jan. 21, 2021, 5:50 p.m. UTC
ipoib connected mode presently queries the device[HCA] to get P_Key
table entry during connection formation. This will increase the time
taken to form the connection, especially when limited P_Keys are in use.
This gets worse when multiple connection attempts are done in parallel.
Improve this by using the cached version of ib_find_pkey().

This improved the latency from 500ms to 3ms on an internal setup.

Suggested-by: Haakon Bugge <haakon.bugge@oracle.com>
Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
---
 drivers/infiniband/ulp/ipoib/ipoib_cm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Comments

Jason Gunthorpe Jan. 21, 2021, 6:16 p.m. UTC | #1
On Thu, Jan 21, 2021 at 09:50:03AM -0800, Manjunath Patil wrote:
> ipoib connected mode presently queries the device[HCA] to get P_Key
> table entry during connection formation. This will increase the time
> taken to form the connection, especially when limited P_Keys are in use.
> This gets worse when multiple connection attempts are done in parallel.
> Improve this by using the cached version of ib_find_pkey().
> 
> This improved the latency from 500ms to 3ms on an internal setup.
> 
> Suggested-by: Haakon Bugge <haakon.bugge@oracle.com>
> Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
>  drivers/infiniband/ulp/ipoib/ipoib_cm.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> index 8f0b598..013a1d8 100644
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> @@ -40,6 +40,7 @@
>  #include <linux/moduleparam.h>
>  #include <linux/sched/signal.h>
>  #include <linux/sched/mm.h>
> +#include <rdma/ib_cache.h>
>  
>  #include "ipoib.h"
>  
> @@ -1122,7 +1123,8 @@ static int ipoib_cm_modify_tx_init(struct net_device *dev,
>  	struct ipoib_dev_priv *priv = ipoib_priv(dev);
>  	struct ib_qp_attr qp_attr;
>  	int qp_attr_mask, ret;
> -	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey, &qp_attr.pkey_index);
> +	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey,
> +						&qp_attr.pkey_index);

ipoib interfaces are locked to a single pkey, you should be able to
get the pkey index that was determined at link up time and use it here
instead of searching anything

Jason
Manjunath Patil Jan. 25, 2021, 6:49 p.m. UTC | #2
> On Thu, Jan 21, 2021 at 09:50:03AM -0800, Manjunath Patil wrote:
> > ipoib connected mode presently queries the device[HCA] to get P_Key
> > table entry during connection formation. This will increase the time
> > taken to form the connection, especially when limited P_Keys are in use.
> > This gets worse when multiple connection attempts are done in parallel.
> > Improve this by using the cached version of ib_find_pkey().
> >
> > This improved the latency from 500ms to 3ms on an internal setup.
> >
> > Suggested-by: Haakon Bugge <haakon.bugge@oracle.com>
> > Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
> >  drivers/infiniband/ulp/ipoib/ipoib_cm.c |    4 +++-
> >  1 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > index 8f0b598..013a1d8 100644
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > @@ -40,6 +40,7 @@
> >  #include <linux/moduleparam.h>
> >  #include <linux/sched/signal.h>
> >  #include <linux/sched/mm.h>
> > +#include <rdma/ib_cache.h>
> >
> >  #include "ipoib.h"
> >
> > @@ -1122,7 +1123,8 @@ static int ipoib_cm_modify_tx_init(struct
> net_device *dev,
> >  	struct ipoib_dev_priv *priv = ipoib_priv(dev);
> >  	struct ib_qp_attr qp_attr;
> >  	int qp_attr_mask, ret;
> > -	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey,
> &qp_attr.pkey_index);
> > +	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey,
> > +						&qp_attr.pkey_index);
> 
> ipoib interfaces are locked to a single pkey, you should be able to get the
> pkey index that was determined at link up time and use it here instead of
> searching anything
> 
> Jason

Thank you Jason for your input. I will explore and get back to you.

-Manjunath
Haakon Bugge Jan. 26, 2021, 8:01 p.m. UTC | #3
> On 25 Jan 2021, at 19:49, Manjunath Patil <manjunath.b.patil@oracle.com> wrote:
> 
>> On Thu, Jan 21, 2021 at 09:50:03AM -0800, Manjunath Patil wrote:
>>> ipoib connected mode presently queries the device[HCA] to get P_Key
>>> table entry during connection formation. This will increase the time
>>> taken to form the connection, especially when limited P_Keys are in use.
>>> This gets worse when multiple connection attempts are done in parallel.
>>> Improve this by using the cached version of ib_find_pkey().
>>> 
>>> This improved the latency from 500ms to 3ms on an internal setup.
>>> 
>>> Suggested-by: Haakon Bugge <haakon.bugge@oracle.com>
>>> Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
>>> drivers/infiniband/ulp/ipoib/ipoib_cm.c |    4 +++-
>>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>>> b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>>> index 8f0b598..013a1d8 100644
>>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>>> @@ -40,6 +40,7 @@
>>> #include <linux/moduleparam.h>
>>> #include <linux/sched/signal.h>
>>> #include <linux/sched/mm.h>
>>> +#include <rdma/ib_cache.h>
>>> 
>>> #include "ipoib.h"
>>> 
>>> @@ -1122,7 +1123,8 @@ static int ipoib_cm_modify_tx_init(struct
>> net_device *dev,
>>> 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
>>> 	struct ib_qp_attr qp_attr;
>>> 	int qp_attr_mask, ret;
>>> -	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey,
>> &qp_attr.pkey_index);
>>> +	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey,
>>> +						&qp_attr.pkey_index);
>> 
>> ipoib interfaces are locked to a single pkey, you should be able to get the
>> pkey index that was determined at link up time and use it here instead of
>> searching anything

Isn't possible to:

# ip link add DEVICE name NAME type ipoib [ pkey PKEY ] 

?


Thxs, Håkon



>> 
>> Jason
> 
> Thank you Jason for your input. I will explore and get back to you.
> 
> -Manjunath
Haakon Bugge Jan. 26, 2021, 8:28 p.m. UTC | #4
> On 25 Jan 2021, at 19:49, Manjunath Patil <manjunath.b.patil@oracle.com> wrote:
> 
>> On Thu, Jan 21, 2021 at 09:50:03AM -0800, Manjunath Patil wrote:
>>> ipoib connected mode presently queries the device[HCA] to get P_Key
>>> table entry during connection formation. This will increase the time
>>> taken to form the connection, especially when limited P_Keys are in use.
>>> This gets worse when multiple connection attempts are done in parallel.
>>> Improve this by using the cached version of ib_find_pkey().
>>> 
>>> This improved the latency from 500ms to 3ms on an internal setup.
>>> 
>>> Suggested-by: Haakon Bugge <haakon.bugge@oracle.com>
>>> Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com>
>>> drivers/infiniband/ulp/ipoib/ipoib_cm.c |    4 +++-
>>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>>> b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>>> index 8f0b598..013a1d8 100644
>>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
>>> @@ -40,6 +40,7 @@
>>> #include <linux/moduleparam.h>
>>> #include <linux/sched/signal.h>
>>> #include <linux/sched/mm.h>
>>> +#include <rdma/ib_cache.h>
>>> 
>>> #include "ipoib.h"
>>> 
>>> @@ -1122,7 +1123,8 @@ static int ipoib_cm_modify_tx_init(struct
>> net_device *dev,
>>> 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
>>> 	struct ib_qp_attr qp_attr;
>>> 	int qp_attr_mask, ret;
>>> -	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey,
>> &qp_attr.pkey_index);
>>> +	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey,
>>> +						&qp_attr.pkey_index);
>> 
>> ipoib interfaces are locked to a single pkey, you should be able to get the
>> pkey index that was determined at link up time and use it here instead of
>> searching anything

Isn't possible to:

# ip link add DEVICE name NAME type ipoib [ pkey PKEY ] 

?


Thxs, Håkon



>> 
>> Jason
> 
> Thank you Jason for your input. I will explore and get back to you.
> 
> -Manjunath
Jason Gunthorpe Jan. 27, 2021, 12:16 a.m. UTC | #5
On Tue, Jan 26, 2021 at 08:28:18PM +0000, Haakon Bugge wrote: 
> >>> @@ -1122,7 +1123,8 @@ static int ipoib_cm_modify_tx_init(struct
> >> net_device *dev,
> >>> 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
> >>> 	struct ib_qp_attr qp_attr;
> >>> 	int qp_attr_mask, ret;
> >>> -	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey,
> >> &qp_attr.pkey_index);
> >>> +	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey,
> >>> +						&qp_attr.pkey_index);
> >> 
> >> ipoib interfaces are locked to a single pkey, you should be able to get the
> >> pkey index that was determined at link up time and use it here instead of
> >> searching anything
> 
> Isn't possible to:
> 
> # ip link add DEVICE name NAME type ipoib [ pkey PKEY ] 
> 
> ?

Yes, and each new netdev that spawns has a fixed pkey that doesn't
change for the life of the netdev

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 8f0b598..013a1d8 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -40,6 +40,7 @@ 
 #include <linux/moduleparam.h>
 #include <linux/sched/signal.h>
 #include <linux/sched/mm.h>
+#include <rdma/ib_cache.h>
 
 #include "ipoib.h"
 
@@ -1122,7 +1123,8 @@  static int ipoib_cm_modify_tx_init(struct net_device *dev,
 	struct ipoib_dev_priv *priv = ipoib_priv(dev);
 	struct ib_qp_attr qp_attr;
 	int qp_attr_mask, ret;
-	ret = ib_find_pkey(priv->ca, priv->port, priv->pkey, &qp_attr.pkey_index);
+	ret = ib_find_cached_pkey(priv->ca, priv->port, priv->pkey,
+						&qp_attr.pkey_index);
 	if (ret) {
 		ipoib_warn(priv, "pkey 0x%x not found: %d\n", priv->pkey, ret);
 		return ret;