diff mbox

[3/5] IB/ipoib: Make sendonly multicast joins create the mcast group

Message ID 1454706707-11239-4-git-send-email-tim.gardner@canonical.com
State New
Headers show

Commit Message

Tim Gardner Feb. 5, 2016, 9:11 p.m. UTC 
From: Doug Ledford <dledford@redhat.com>

BugLink: http://bugs.launchpad.net/bugs/1542444

Since IPoIB should, as much as possible, emulate how multicast
sends work on Ethernet for regular TCP/IP apps, there should be
no requirement to subscribe to a multicast group before your
sends are properly sent.  However, due to the difference in how
multicast is handled on InfiniBand, we must join the appropriate
multicast group before we can send to it.  Previously we tried
not to trigger the auto-create feature of the subnet manager when
doing this because we didn't have tracking of these sendonly
groups and the auto-creation might never get undone.  The previous
patch added timing to these sendonly joins and allows us to
leave them after a reasonable idle expiration time.  So supply
all of the information needed to auto-create group.

Signed-off-by: Doug Ledford <dledford@redhat.com>
(back ported from commit c3852ab0e606212de523c1fb1e15adbf9f431619)
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

 Conflicts:
	drivers/infiniband/ulp/ipoib/ipoib_multicast.c
---
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Andy Whitcroft Feb. 9, 2016, 6:01 p.m. UTC | #1 
On Fri, Feb 05, 2016 at 02:11:45PM -0700, tim.gardner@canonical.com wrote:
> From: Doug Ledford <dledford@redhat.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1542444
> 
> Since IPoIB should, as much as possible, emulate how multicast
> sends work on Ethernet for regular TCP/IP apps, there should be
> no requirement to subscribe to a multicast group before your
> sends are properly sent.  However, due to the difference in how
> multicast is handled on InfiniBand, we must join the appropriate
> multicast group before we can send to it.  Previously we tried
> not to trigger the auto-create feature of the subnet manager when
> doing this because we didn't have tracking of these sendonly
> groups and the auto-creation might never get undone.  The previous
> patch added timing to these sendonly joins and allows us to
> leave them after a reasonable idle expiration time.  So supply
> all of the information needed to auto-create group.
> 
> Signed-off-by: Doug Ledford <dledford@redhat.com>
> (back ported from commit c3852ab0e606212de523c1fb1e15adbf9f431619)
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> 
>  Conflicts:
> 	drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> index 4990572..180d10e 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> @@ -500,6 +500,24 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast,
>  		rec.sl		  = priv->broadcast->mcmember.sl;
>  		rec.flow_label	  = priv->broadcast->mcmember.flow_label;
>  		rec.hop_limit	  = priv->broadcast->mcmember.hop_limit;
> +
> +		/*
> +		 * Send-only IB Multicast joins do not work at the core
> +		 * IB layer yet, so we can't use them here.  However,
> +		 * we are emulating an Ethernet multicast send, which
> +		 * does not require a multicast subscription and will
> +		 * still send properly.  The most appropriate thing to
> +		 * do is to create the group if it doesn't exist as that
> +		 * most closely emulates the behavior, from a user space
> +		 * application perspecitive, of Ethernet multicast
> +		 * operation.  For now, we do a full join, maybe later
> +		 * when the core IB layers support send only joins we
> +		 * will use them.
> +		 */
> +#if 0
> +		if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags))
> +			rec.join_state = 4;
> +#endif
>  	}
>  
>  	multicast = ib_sa_join_multicast(&ipoib_sa_client, priv->ca, priv->port,

This patch seems to only add code which is commented out ?  I am not
sure this makes sense to me.

-apw
Tim Gardner Feb. 9, 2016, 8:30 p.m. UTC | #2 
On 02/09/2016 10:01 AM, Andy Whitcroft wrote:
> On Fri, Feb 05, 2016 at 02:11:45PM -0700, tim.gardner@canonical.com wrote:
>> From: Doug Ledford <dledford@redhat.com>
>>
>> BugLink: http://bugs.launchpad.net/bugs/1542444
>>
>> Since IPoIB should, as much as possible, emulate how multicast
>> sends work on Ethernet for regular TCP/IP apps, there should be
>> no requirement to subscribe to a multicast group before your
>> sends are properly sent.  However, due to the difference in how
>> multicast is handled on InfiniBand, we must join the appropriate
>> multicast group before we can send to it.  Previously we tried
>> not to trigger the auto-create feature of the subnet manager when
>> doing this because we didn't have tracking of these sendonly
>> groups and the auto-creation might never get undone.  The previous
>> patch added timing to these sendonly joins and allows us to
>> leave them after a reasonable idle expiration time.  So supply
>> all of the information needed to auto-create group.
>>
>> Signed-off-by: Doug Ledford <dledford@redhat.com>
>> (back ported from commit c3852ab0e606212de523c1fb1e15adbf9f431619)
>> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
>>
>>  Conflicts:
>> 	drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> ---
>>  drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> index 4990572..180d10e 100644
>> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> @@ -500,6 +500,24 @@ static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast,
>>  		rec.sl		  = priv->broadcast->mcmember.sl;
>>  		rec.flow_label	  = priv->broadcast->mcmember.flow_label;
>>  		rec.hop_limit	  = priv->broadcast->mcmember.hop_limit;
>> +
>> +		/*
>> +		 * Send-only IB Multicast joins do not work at the core
>> +		 * IB layer yet, so we can't use them here.  However,
>> +		 * we are emulating an Ethernet multicast send, which
>> +		 * does not require a multicast subscription and will
>> +		 * still send properly.  The most appropriate thing to
>> +		 * do is to create the group if it doesn't exist as that
>> +		 * most closely emulates the behavior, from a user space
>> +		 * application perspecitive, of Ethernet multicast
>> +		 * operation.  For now, we do a full join, maybe later
>> +		 * when the core IB layers support send only joins we
>> +		 * will use them.
>> +		 */
>> +#if 0
>> +		if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags))
>> +			rec.join_state = 4;
>> +#endif
>>  	}
>>  
>>  	multicast = ib_sa_join_multicast(&ipoib_sa_client, priv->ca, priv->port,
> 
> This patch seems to only add code which is commented out ?  I am not
> sure this makes sense to me.
> 
> -apw
> 

It does look a little weird, but the end result is correct. I'm happy
with dropping it.

rtg
diff mbox

Patch

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 4990572..180d10e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -500,6 +500,24 @@  static void ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast,
 		rec.sl		  = priv->broadcast->mcmember.sl;
 		rec.flow_label	  = priv->broadcast->mcmember.flow_label;
 		rec.hop_limit	  = priv->broadcast->mcmember.hop_limit;
+
+		/*
+		 * Send-only IB Multicast joins do not work at the core
+		 * IB layer yet, so we can't use them here.  However,
+		 * we are emulating an Ethernet multicast send, which
+		 * does not require a multicast subscription and will
+		 * still send properly.  The most appropriate thing to
+		 * do is to create the group if it doesn't exist as that
+		 * most closely emulates the behavior, from a user space
+		 * application perspecitive, of Ethernet multicast
+		 * operation.  For now, we do a full join, maybe later
+		 * when the core IB layers support send only joins we
+		 * will use them.
+		 */
+#if 0
+		if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags))
+			rec.join_state = 4;
+#endif
 	}
 
 	multicast = ib_sa_join_multicast(&ipoib_sa_client, priv->ca, priv->port,