re: Fix IPoIB to conform to ethtool definitions

re: Fix IPoIB to conform to ethtool definitions

[Or Gerlitz]

Hi Eli, can this patch of yours which you placed in ofed be pushed
upstream? Or.

>From 4237a1fbc1bae6bb86665f81cd93cfac37b216d2 Mon Sep 17 00:00:00 2001
From: Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
Date: Wed, 3 Nov 2010 10:56:38 +0200
Subject: [PATCH] IPoIB: Fix IPoIB to conform to ethtool definitions

Ethtool documentation states that when once of the parameters,
rx_coalesce_usecs or rx_max_coalesced_frames are set to zero while the other
has a none zero value, the none zero parameter should still be operative. For
example, if rx_max_coalesced_frames is set to zero while rx_coalesce_usecs is >
0, the rate of events is limited to not exceed (1 / rx_coalesce_usecs). In the
opposite case, an event is generated only after rx_max_coalesced_frames have
arrived. The documentation also states that setting both to zero is invalid.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
index e9795f6..e602b7f 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
@@ -78,6 +78,13 @@ static int ipoib_set_coalesce(struct net_device *dev,
 	    coal->rx_max_coalesced_frames > 0xffff)
 		return -EINVAL;

+	if (coal->rx_max_coalesced_frames | coal->rx_coalesce_usecs) {
+		if (!coal->rx_max_coalesced_frames)
+			coal->rx_max_coalesced_frames = 0xffff;
+		else if (!coal->rx_coalesce_usecs)
+			coal->rx_coalesce_usecs = 0xffff;
+	}
+
 	ret = ib_modify_cq(priv->recv_cq, coal->rx_max_coalesced_frames,
 			   coal->rx_coalesce_usecs);
 	if (ret && ret != -ENOSYS) {
-- 
1.7.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Fix IPoIB to conform to ethtool definitions

[Eli Cohen]

Sure, I was going to. I will send later today.

-----Original Message-----
From: Or Gerlitz [mailto:ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org] 
Sent: Monday, November 08, 2010 9:38 AM
To: Eli Cohen; Roland Dreier
Cc: linux-rdma
Subject: re: Fix IPoIB to conform to ethtool definitions

Hi Eli, can this patch of yours which you placed in ofed be pushed
upstream? Or.

>From 4237a1fbc1bae6bb86665f81cd93cfac37b216d2 Mon Sep 17 00:00:00 2001
From: Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
Date: Wed, 3 Nov 2010 10:56:38 +0200
Subject: [PATCH] IPoIB: Fix IPoIB to conform to ethtool definitions

Ethtool documentation states that when once of the parameters,
rx_coalesce_usecs or rx_max_coalesced_frames are set to zero while the other
has a none zero value, the none zero parameter should still be operative. For
example, if rx_max_coalesced_frames is set to zero while rx_coalesce_usecs is >
0, the rate of events is limited to not exceed (1 / rx_coalesce_usecs). In the
opposite case, an event is generated only after rx_max_coalesced_frames have
arrived. The documentation also states that setting both to zero is invalid.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
index e9795f6..e602b7f 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c
@@ -78,6 +78,13 @@ static int ipoib_set_coalesce(struct net_device *dev,
 	    coal->rx_max_coalesced_frames > 0xffff)
 		return -EINVAL;

+	if (coal->rx_max_coalesced_frames | coal->rx_coalesce_usecs) {
+		if (!coal->rx_max_coalesced_frames)
+			coal->rx_max_coalesced_frames = 0xffff;
+		else if (!coal->rx_coalesce_usecs)
+			coal->rx_coalesce_usecs = 0xffff;
+	}
+
 	ret = ib_modify_cq(priv->recv_cq, coal->rx_max_coalesced_frames,
 			   coal->rx_coalesce_usecs);
 	if (ret && ret != -ENOSYS) {
-- 
1.7.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Fix IPoIB to conform to ethtool definitions

[Or Gerlitz]

Eli Cohen wrote:
> Sure, I was going to. I will send later today.

I saw that you've dropped and implementation of inline/blue-flame sending 
for kernel space, what was the motivation is it sdp, rds or alike or something else?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Fix IPoIB to conform to ethtool definitions

[Eli Cohen]

Will send those too. The idea is to let kernel consumers enjoy the improvements to latency that blue flame gives. And yes, SDP is motivating us but I am going to push to IPoIB too.
I want to take the opportunity that you raised the issue to hear others opinion about changing the bitmap allocator maintain an "avail" variable that will count the number of available UARs. I want to use this to limit the number of UARs that a kernel consumer can allocate so that there will always be some available for userspace such that we will never fail ibv_devinfo due to lack of UARs. Thoughts?

-----Original Message-----
From: Or Gerlitz [mailto:ogerlitz-hKgKHo2Ms0FWk0Htik3J/w@public.gmane.org] 
Sent: Monday, November 08, 2010 9:51 AM
To: Eli Cohen
Cc: Roland Dreier; linux-rdma
Subject: Re: Fix IPoIB to conform to ethtool definitions

Eli Cohen wrote:
> Sure, I was going to. I will send later today.

I saw that you've dropped and implementation of inline/blue-flame sending 
for kernel space, what was the motivation is it sdp, rds or alike or something else?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Or Gerlitz]

Eli Cohen wrote:
> The idea is to let kernel consumers enjoy the improvements to latency that blue flame gives. And yes, SDP is motivating us but I am going to push to IPoIB too.
>   
 From my recollection of numbers, for user space apps, using inline 
accounts for about 1us improvement in the latency, if this is indeed the 
case, I'm sure if there's great value here for kernel consumers, do you 
have any numbers to support this patch?

> I want to take the opportunity that you raised the issue to hear others opinion about changing the bitmap allocator maintain an "avail" variable that will count the number of available UARs. I want to use this to limit the number of UARs that a kernel consumer can allocate so that there will always be some available for userspace 
Is this correct that today all the kernels QPs use the same UAR, any 
problem with that?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Eli Cohen]

On Mon, Nov 08, 2010 at 10:27:15AM +0200, Or Gerlitz wrote:
> Is this correct that today all the kernels QPs use the same UAR, any
> problem with that?
> 
Yes, today all kernel consumers use the same UAR. And if you want to
use blue flame then you need to use different UARs.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Or Gerlitz]

Or Gerlitz wrote:
> Eli Cohen wrote:
>>   
> From my recollection of numbers, for user space apps, using inline
> accounts for about 1us improvement in the latency, if this is indeed the
> case, I'm sure if there's great value here for kernel consumers, do you
> have any numbers to support this patch?

I wanted to say that I'm NOT sure, sorry for the spam

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Eli Cohen]

On Mon, Nov 08, 2010 at 10:45:42AM +0200, Or Gerlitz wrote:
> Or Gerlitz wrote:
> > Eli Cohen wrote:
> >>   
> > From my recollection of numbers, for user space apps, using inline
> > accounts for about 1us improvement in the latency, if this is indeed the
> > case, I'm sure if there's great value here for kernel consumers, do you
> > have any numbers to support this patch?
> 
> I wanted to say that I'm NOT sure, sorry for the spam
> 
It indeed improves SDP's latency - I don't have exact numbers.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Or Gerlitz]

Eli Cohen wrote:
> It indeed improves SDP's latency - I don't have exact numbers.

the SDP number is very interesting (Amir, do you have it?) but irrelevant for upstream, any IPoIB numbers?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Amir Vadai]

I am working on it, will notify when have numbers.

On 11/08/2010 10:51 AM, Or Gerlitz wrote:
> Eli Cohen wrote:
>> It indeed improves SDP's latency - I don't have exact numbers.
> the SDP number is very interesting (Amir, do you have it?) but irrelevant for upstream, any IPoIB numbers?
>
> Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Eli Cohen]

On Mon, Nov 08, 2010 at 10:51:30AM +0200, Or Gerlitz wrote:
> Eli Cohen wrote:
> > It indeed improves SDP's latency - I don't have exact numbers.
> 
> the SDP number is very interesting (Amir, do you have it?) but irrelevant for upstream, any IPoIB numbers?
> 

For IPoIB it gives ~1 usec for improvement in latency.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kernel inline sending / UARs / etc

[Or Gerlitz]

Eli Cohen wrote:
> For IPoIB it gives ~1 usec for improvement in latency.

yep, this is what I expected, so over your testbed from what value to what value? also it 
would be important to note the change in the cpu utilization (e.g few "vmstat 1" output
lines before/after the change, while running IPoIB traffic)

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html