[PATCH V2 for-next 0/4] Support SendOnlyFullMember join

[PATCH V2 for-next 0/4] Support SendOnlyFullMember join

[Erez Shitrit]

There are four types of multicast join: FullMember, NonMember,
SendOnlyNonMember and SendOnlyFullMember.

This patch-set introduces the ability to send multicast join requests of
type SendOnlyFullMember (until now only the first three types were
supported).
Host that sends such join request, does not require a multicast
subscription and will still send properly, and in addition will not get
other multicast packets from that group.

It adds a new SA query (for classport_info) to check that the sm
supports sending such requests and it modifies multicast processing in
the IB/core module to handle SendOnlyFullMember multicast join requests.

In addition, the patch set modifies IPoIB so that if the sm does support
SendOnlyFullMember join requests, IPoIB will send such a request when
joining a sendonly multicast group.

Changes from V0:
PORTCLASSINFO_REC_FIELD changed to CLASSPORTINFO_REC_FIELD
2 fields in ib_class_port_info instead of one
"sa" changed to SA
"sa" in the headline changed to SA agent
No need to indicates NUM_JOIN_MEMBERSHIP_TYPES = 4
Change the place the ipoib driver calls the new sa_query, does it once after the broadcast is joined.

Changes from V1:
1. add define to sendonly_full_member join state, instead of "8"
2. change commit message to introduce the capmask2
3. number name instead of digit in commit message
4. SA agent instead of SA in commit message header.
 
Erez Shitrit (4):
  IB/core: Introduce capabilitymask2 field in ClassPortInfo mad
  IB/SA Agent: Add support for SA agent get ClassPortInfo
  IB/core: Support new type of join-state for multicast
  IB/ipoib: Support SendOnlyFullMember MCG for SendOnly join

 drivers/infiniband/core/multicast.c            |  23 +++-
 drivers/infiniband/core/sa_query.c             | 178 +++++++++++++++++++++++++
 drivers/infiniband/hw/qib/qib_mad.c            |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib.h           |   2 +
 drivers/infiniband/ulp/ipoib/ipoib_main.c      |  74 ++++++++++
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  38 ++++--
 include/rdma/ib_mad.h                          |   2 +-
 include/rdma/ib_sa.h                           |  12 ++
 8 files changed, 313 insertions(+), 20 deletions(-)

-- 
1.7.11.3

--
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

[PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Erez Shitrit]

Change struct ib_class_port_info to conform to IB Spec 1.3
That in order to get specific capability mask from ClassPortInfo mad.

>From the IB Spec, ClassPortInfo section:
        "CapabilityMask2 Bits 0-26: Additional class-specific capabilities...
         RespTimeValue the rest 5 bits"

The new struct now has one field for capabilitymask2 (previously was the
reserved field) and the resp_time field.

And it fixes up qib use of the field repurposed to be used as capabilitymask2:
IB/qib: Change pma_get_classportinfo

Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/qib/qib_mad.c | 4 +++-
 include/rdma/ib_mad.h               | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
index 0bd1837..c5f6248 100644
--- a/drivers/infiniband/hw/qib/qib_mad.c
+++ b/drivers/infiniband/hw/qib/qib_mad.c
@@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
 	struct ib_class_port_info *p =
 		(struct ib_class_port_info *)pmp->data;
 	struct qib_devdata *dd = dd_from_ibdev(ibdev);
+	char *p_cap_mask2;
 
 	memset(pmp->data, 0, sizeof(pmp->data));
 
@@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
 	 * Set the most significant bit of CM2 to indicate support for
 	 * congestion statistics
 	 */
-	p->reserved[0] = dd->psxmitwait_supported << 7;
+	p_cap_mask2 = (char *)&p->capability_mask2;
+	p_cap_mask2[0] = dd->psxmitwait_supported << 7;
 	/*
 	 * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
 	 */
diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
index 37dd534c..0761ab9 100644
--- a/include/rdma/ib_mad.h
+++ b/include/rdma/ib_mad.h
@@ -243,7 +243,7 @@ struct ib_class_port_info {
 	u8			base_version;
 	u8			class_version;
 	__be16			capability_mask;
-	u8			reserved[3];
+	__be32			capability_mask2;
 	u8			resp_time_value;
 	u8			redirect_gid[16];
 	__be32			redirect_tcslfl;
-- 
1.7.11.3

--
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

[PATCH V2 for-next 2/4] IB/SA Agent: Add support for SA agent get ClassPortInfo

[Erez Shitrit]

New SA query function to return the ClassPortInfo struct from the SA.
If the SM supports FullMemberSendOnly mode for MCG's, it sets a
capability bit in the capability_mask2 field of the response.

Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/sa_query.c | 178 +++++++++++++++++++++++++++++++++++++
 include/rdma/ib_sa.h               |  12 +++
 2 files changed, 190 insertions(+)

diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 8a09c0f..73d7b86 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -119,6 +119,12 @@ struct ib_sa_guidinfo_query {
 	struct ib_sa_query sa_query;
 };
 
+struct ib_sa_classport_info_query {
+	void (*callback)(int, struct ib_class_port_info *, void *);
+	void *context;
+	struct ib_sa_query sa_query;
+};
+
 struct ib_sa_mcmember_query {
 	void (*callback)(int, struct ib_sa_mcmember_rec *, void *);
 	void *context;
@@ -392,6 +398,87 @@ static const struct ib_field service_rec_table[] = {
 	  .size_bits    = 2*64 },
 };
 
+#define CLASSPORTINFO_REC_FIELD(field) \
+	.struct_offset_bytes = offsetof(struct ib_class_port_info, field),	\
+	.struct_size_bytes   = sizeof((struct ib_class_port_info *)0)->field,	\
+	.field_name          = "ib_class_port_info:" #field
+
+static const struct ib_field classport_info_rec_table[] = {
+	{ CLASSPORTINFO_REC_FIELD(base_version),
+	  .offset_words = 0,
+	  .offset_bits  = 0,
+	  .size_bits    = 8 },
+	{ CLASSPORTINFO_REC_FIELD(class_version),
+	  .offset_words = 0,
+	  .offset_bits  = 8,
+	  .size_bits    = 8 },
+	{ CLASSPORTINFO_REC_FIELD(capability_mask),
+	  .offset_words = 0,
+	  .offset_bits  = 16,
+	  .size_bits    = 16 },
+	{ CLASSPORTINFO_REC_FIELD(capability_mask2),
+	  .offset_words = 1,
+	  .offset_bits  = 0,
+	  .size_bits    = 27 },
+	{ CLASSPORTINFO_REC_FIELD(resp_time_value),
+	  .offset_words = 1,
+	  .offset_bits  = 27,
+	  .size_bits    = 5 },
+	{ CLASSPORTINFO_REC_FIELD(redirect_gid),
+	  .offset_words = 2,
+	  .offset_bits  = 0,
+	  .size_bits    = 128 },
+	{ CLASSPORTINFO_REC_FIELD(redirect_tcslfl),
+	  .offset_words = 6,
+	  .offset_bits  = 0,
+	  .size_bits    = 32 },
+
+	{ CLASSPORTINFO_REC_FIELD(redirect_lid),
+	  .offset_words = 7,
+	  .offset_bits  = 0,
+	  .size_bits    = 16 },
+	{ CLASSPORTINFO_REC_FIELD(redirect_pkey),
+	  .offset_words = 7,
+	  .offset_bits  = 16,
+	  .size_bits    = 16 },
+
+	{ CLASSPORTINFO_REC_FIELD(redirect_qp),
+	  .offset_words = 8,
+	  .offset_bits  = 0,
+	  .size_bits    = 32 },
+	{ CLASSPORTINFO_REC_FIELD(redirect_qkey),
+	  .offset_words = 9,
+	  .offset_bits  = 0,
+	  .size_bits    = 32 },
+
+	{ CLASSPORTINFO_REC_FIELD(trap_gid),
+	  .offset_words = 10,
+	  .offset_bits  = 0,
+	  .size_bits    = 128 },
+	{ CLASSPORTINFO_REC_FIELD(trap_tcslfl),
+	  .offset_words = 14,
+	  .offset_bits  = 0,
+	  .size_bits    = 32 },
+
+	{ CLASSPORTINFO_REC_FIELD(trap_lid),
+	  .offset_words = 15,
+	  .offset_bits  = 0,
+	  .size_bits    = 16 },
+	{ CLASSPORTINFO_REC_FIELD(trap_pkey),
+	  .offset_words = 15,
+	  .offset_bits  = 16,
+	  .size_bits    = 16 },
+
+	{ CLASSPORTINFO_REC_FIELD(trap_hlqp),
+	  .offset_words = 16,
+	  .offset_bits  = 0,
+	  .size_bits    = 32 },
+	{ CLASSPORTINFO_REC_FIELD(trap_qkey),
+	  .offset_words = 17,
+	  .offset_bits  = 0,
+	  .size_bits    = 32 },
+};
+
 #define GUIDINFO_REC_FIELD(field) \
 	.struct_offset_bytes = offsetof(struct ib_sa_guidinfo_rec, field),	\
 	.struct_size_bytes   = sizeof((struct ib_sa_guidinfo_rec *) 0)->field,	\
@@ -1645,6 +1732,97 @@ err1:
 }
 EXPORT_SYMBOL(ib_sa_guid_info_rec_query);
 
+/* Support get SA ClassPortInfo */
+static void ib_sa_classport_info_rec_callback(struct ib_sa_query *sa_query,
+					      int status,
+					      struct ib_sa_mad *mad)
+{
+	struct ib_sa_classport_info_query *query =
+		container_of(sa_query, struct ib_sa_classport_info_query, sa_query);
+
+	if (mad) {
+		struct ib_class_port_info rec;
+
+		ib_unpack(classport_info_rec_table,
+			  ARRAY_SIZE(classport_info_rec_table),
+			  mad->data, &rec);
+		query->callback(status, &rec, query->context);
+	} else {
+		query->callback(status, NULL, query->context);
+	}
+}
+
+static void ib_sa_portclass_info_rec_release(struct ib_sa_query *sa_query)
+{
+	kfree(container_of(sa_query, struct ib_sa_classport_info_query,
+			   sa_query));
+}
+
+int ib_sa_classport_info_rec_query(struct ib_sa_client *client,
+				   struct ib_device *device, u8 port_num,
+				   int timeout_ms, gfp_t gfp_mask,
+				   void (*callback)(int status,
+						    struct ib_class_port_info *resp,
+						    void *context),
+				   void *context,
+				   struct ib_sa_query **sa_query)
+{
+	struct ib_sa_classport_info_query *query;
+	struct ib_sa_device *sa_dev = ib_get_client_data(device, &sa_client);
+	struct ib_sa_port *port;
+	struct ib_mad_agent *agent;
+	struct ib_sa_mad *mad;
+	int ret;
+
+	if (!sa_dev)
+		return -ENODEV;
+
+	port  = &sa_dev->port[port_num - sa_dev->start_port];
+	agent = port->agent;
+
+	query = kzalloc(sizeof(*query), gfp_mask);
+	if (!query)
+		return -ENOMEM;
+
+	query->sa_query.port = port;
+	ret = alloc_mad(&query->sa_query, gfp_mask);
+	if (ret)
+		goto err1;
+
+	ib_sa_client_get(client);
+	query->sa_query.client = client;
+	query->callback        = callback;
+	query->context         = context;
+
+	mad = query->sa_query.mad_buf->mad;
+	init_mad(mad, agent);
+
+	query->sa_query.callback = callback ? ib_sa_classport_info_rec_callback : NULL;
+
+	query->sa_query.release  = ib_sa_portclass_info_rec_release;
+	/* support GET only */
+	mad->mad_hdr.method	 = IB_MGMT_METHOD_GET;
+	mad->mad_hdr.attr_id	 = cpu_to_be16(IB_SA_ATTR_CLASS_PORTINFO);
+	mad->sa_hdr.comp_mask	 = 0;
+	*sa_query = &query->sa_query;
+
+	ret = send_mad(&query->sa_query, timeout_ms, gfp_mask);
+	if (ret < 0)
+		goto err2;
+
+	return ret;
+
+err2:
+	*sa_query = NULL;
+	ib_sa_client_put(query->sa_query.client);
+	free_mad(&query->sa_query);
+
+err1:
+	kfree(query);
+	return ret;
+}
+EXPORT_SYMBOL(ib_sa_classport_info_rec_query);
+
 static void send_handler(struct ib_mad_agent *agent,
 			 struct ib_mad_send_wc *mad_send_wc)
 {
diff --git a/include/rdma/ib_sa.h b/include/rdma/ib_sa.h
index cdc1c81..3840416 100644
--- a/include/rdma/ib_sa.h
+++ b/include/rdma/ib_sa.h
@@ -94,6 +94,8 @@ enum ib_sa_selector {
 	IB_SA_BEST = 3
 };
 
+#define IB_SA_CAP_MASK2_SENDONLY_FULL_MEM_SUPPORT	BIT(12)
+
 /*
  * Structures for SA records are named "struct ib_sa_xxx_rec."  No
  * attempt is made to pack structures to match the physical layout of
@@ -439,4 +441,14 @@ int ib_sa_guid_info_rec_query(struct ib_sa_client *client,
 			      void *context,
 			      struct ib_sa_query **sa_query);
 
+/* Support get SA ClassPortInfo */
+int ib_sa_classport_info_rec_query(struct ib_sa_client *client,
+				   struct ib_device *device, u8 port_num,
+				   int timeout_ms, gfp_t gfp_mask,
+				   void (*callback)(int status,
+						    struct ib_class_port_info *resp,
+						    void *context),
+				   void *context,
+				   struct ib_sa_query **sa_query);
+
 #endif /* IB_SA_H */
-- 
1.7.11.3

--
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

[PATCH V2 for-next 3/4] IB/core: Support new type of join-state for multicast

[Erez Shitrit]

There are four types for MCG, FullMember, NonMember, SendOnlyNonMember,
and the new added type: SendOnlyFullMember.
Add support for the new SendOnlyFullMember join state.

The new type allows host to send join request as sendonly, it will cause the
group to be created but without getting packets from this multicast back to the
host.

Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/multicast.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/multicast.c b/drivers/infiniband/core/multicast.c
index 250937c..a83ec28 100644
--- a/drivers/infiniband/core/multicast.c
+++ b/drivers/infiniband/core/multicast.c
@@ -93,6 +93,18 @@ enum {
 
 struct mcast_member;
 
+/*
+* There are 4 types of join states:
+* FullMember, NonMember, SendOnlyNonMember, SendOnlyFullMember.
+*/
+enum {
+	FULLMEMBER_JOIN,
+	NONMEMBER_JOIN,
+	SENDONLY_NONMEBER_JOIN,
+	SENDONLY_FULLMEMBER_JOIN,
+	NUM_JOIN_MEMBERSHIP_TYPES,
+};
+
 struct mcast_group {
 	struct ib_sa_mcmember_rec rec;
 	struct rb_node		node;
@@ -102,7 +114,7 @@ struct mcast_group {
 	struct list_head	pending_list;
 	struct list_head	active_list;
 	struct mcast_member	*last_join;
-	int			members[3];
+	int			members[NUM_JOIN_MEMBERSHIP_TYPES];
 	atomic_t		refcount;
 	enum mcast_group_state	state;
 	struct ib_sa_query	*query;
@@ -220,8 +232,9 @@ static void queue_join(struct mcast_member *member)
 }
 
 /*
- * A multicast group has three types of members: full member, non member, and
- * send only member.  We need to keep track of the number of members of each
+ * A multicast group has four types of members: full member, non member,
+ * sendonly non member and sendonly full member.
+ * We need to keep track of the number of members of each
  * type based on their join state.  Adjust the number of members the belong to
  * the specified join states.
  */
@@ -229,7 +242,7 @@ static void adjust_membership(struct mcast_group *group, u8 join_state, int inc)
 {
 	int i;
 
-	for (i = 0; i < 3; i++, join_state >>= 1)
+	for (i = 0; i < NUM_JOIN_MEMBERSHIP_TYPES; i++, join_state >>= 1)
 		if (join_state & 0x1)
 			group->members[i] += inc;
 }
@@ -245,7 +258,7 @@ static u8 get_leave_state(struct mcast_group *group)
 	u8 leave_state = 0;
 	int i;
 
-	for (i = 0; i < 3; i++)
+	for (i = 0; i < NUM_JOIN_MEMBERSHIP_TYPES; i++)
 		if (!group->members[i])
 			leave_state |= (0x1 << i);
 
-- 
1.7.11.3

--
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

[PATCH V2 for-next 4/4] IB/ipoib: Support SendOnlyFullMember MCG for SendOnly join

[Erez Shitrit]

Check (via an SA query) if the SM supports the new option for SendOnly
multicast joins.
If the SM supports that option it will use the new join state to create
such multicast group.
If SendOnlyFullMember is supported, we wouldn't use faked FullMember state
join for SendOnly MCG, use the correct state if supported.

This check is performed at every invocation of mcast_restart task, to be
sure that the driver stays in sync with the current state of the SM.

Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib.h           |  2 +
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 74 ++++++++++++++++++++++++++
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 38 ++++++++-----
 3 files changed, 101 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index caec8e9..c51f618 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -392,6 +392,7 @@ struct ipoib_dev_priv {
 	struct ipoib_ethtool_st ethtool;
 	struct timer_list poll_timer;
 	unsigned max_send_sge;
+	bool sm_fullmember_sendonly_support;
 };
 
 struct ipoib_ah {
@@ -476,6 +477,7 @@ void ipoib_reap_ah(struct work_struct *work);
 
 void ipoib_mark_paths_invalid(struct net_device *dev);
 void ipoib_flush_paths(struct net_device *dev);
+int ipoib_check_sm_sendonly_fullmember_support(struct ipoib_dev_priv *priv);
 struct ipoib_dev_priv *ipoib_intf_alloc(const char *format);
 
 int ipoib_ib_dev_init(struct net_device *dev, struct ib_device *ca, int port);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 3b630e5..40b3b43 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -117,6 +117,8 @@ int ipoib_open(struct net_device *dev)
 
 	set_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags);
 
+	priv->sm_fullmember_sendonly_support = false;
+
 	if (ipoib_ib_dev_open(dev)) {
 		if (!test_bit(IPOIB_PKEY_ASSIGNED, &priv->flags))
 			return 0;
@@ -629,6 +631,78 @@ void ipoib_mark_paths_invalid(struct net_device *dev)
 	spin_unlock_irq(&priv->lock);
 }
 
+struct classport_info_context {
+	struct ipoib_dev_priv	*priv;
+	struct completion	done;
+	struct ib_sa_query	*sa_query;
+};
+
+static void classport_info_query_cb(int status, struct ib_class_port_info *rec,
+				    void *context)
+{
+	struct classport_info_context *cb_ctx = context;
+	struct ipoib_dev_priv *priv;
+
+	WARN_ON(!context);
+
+	priv = cb_ctx->priv;
+
+	if (status) {
+		pr_debug("device: %s failed query classport_info status: %d\n",
+			 priv->dev->name, status);
+		/* keeps the default, will try next mcast_restart */
+		priv->sm_fullmember_sendonly_support = false;
+		goto out;
+	}
+
+	/* take out resp_time that located in the last 5 bits */
+	if (be32_to_cpu(rec->capability_mask2) &
+	    IB_SA_CAP_MASK2_SENDONLY_FULL_MEM_SUPPORT) {
+		pr_debug("device: %s enabled fullmember-sendonly for sendonly MCG\n",
+			 priv->dev->name);
+		priv->sm_fullmember_sendonly_support = true;
+	} else {
+		pr_debug("device: %s disabled fullmember-sendonly for sendonly MCG\n",
+			 priv->dev->name);
+		priv->sm_fullmember_sendonly_support = false;
+	}
+
+out:
+	complete(&cb_ctx->done);
+}
+
+int ipoib_check_sm_sendonly_fullmember_support(struct ipoib_dev_priv *priv)
+{
+	struct classport_info_context *callback_context;
+	int ret;
+
+	callback_context = kmalloc(sizeof(*callback_context), GFP_KERNEL);
+	if (!callback_context)
+		return -ENOMEM;
+
+	callback_context->priv = priv;
+	init_completion(&callback_context->done);
+
+	ret = ib_sa_classport_info_rec_query(&ipoib_sa_client,
+					     priv->ca, priv->port, 3000,
+					     GFP_KERNEL,
+					     classport_info_query_cb,
+					     callback_context,
+					     &callback_context->sa_query);
+	if (ret < 0) {
+		pr_info("%s failed to send ib_sa_classport_info query, ret: %d\n",
+			priv->dev->name, ret);
+		kfree(callback_context);
+		return ret;
+	}
+
+	/* waiting for the callback to finish before returnning */
+	wait_for_completion(&callback_context->done);
+	kfree(callback_context);
+
+	return ret;
+}
+
 void ipoib_flush_paths(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 2588931..fc3e50e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -64,6 +64,9 @@ struct ipoib_mcast_iter {
 	unsigned int       send_only;
 };
 
+/* join state that allows creating mcg with sendonly member request */
+#define SENDONLY_FULLMEMBER_JOIN	8
+
 /*
  * This should be called with the priv->lock held
  */
@@ -326,12 +329,23 @@ void ipoib_mcast_carrier_on_task(struct work_struct *work)
 	struct ipoib_dev_priv *priv = container_of(work, struct ipoib_dev_priv,
 						   carrier_on_task);
 	struct ib_port_attr attr;
+	int ret;
 
 	if (ib_query_port(priv->ca, priv->port, &attr) ||
 	    attr.state != IB_PORT_ACTIVE) {
 		ipoib_dbg(priv, "Keeping carrier off until IB port is active\n");
 		return;
 	}
+	/*
+	 * Check if can send sendonly MCG's with sendonly-fullmember join state.
+	 * It done here after the successfully join to the broadcast group,
+	 * because the broadcast group must always be joined first and is always
+	 * re-joined if the SM changes substantially.
+	 */
+	ret = ipoib_check_sm_sendonly_fullmember_support(priv);
+	if (ret < 0)
+		pr_debug("%s failed query sm support for sendonly-fullmember (ret: %d)\n",
+			 priv->dev->name, ret);
 
 	/*
 	 * Take rtnl_lock to avoid racing with ipoib_stop() and
@@ -515,22 +529,20 @@ static int ipoib_mcast_join(struct net_device *dev, struct ipoib_mcast *mcast)
 		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
+		 * Send-only IB Multicast joins work at the core IB layer but
+		 * require specific SM support.
+		 * We can use such joins here only if the current SM supports that feature.
+		 * However, if not, we emulate 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.
+		 * application perspective, of Ethernet multicast operation.
 		 */
-#if 0
-		if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags))
-			rec.join_state = 4;
-#endif
+		if (test_bit(IPOIB_MCAST_FLAG_SENDONLY, &mcast->flags) &&
+		    priv->sm_fullmember_sendonly_support)
+			/* SM supports sendonly-fullmember, otherwise fallback to full-member */
+			rec.join_state = SENDONLY_FULLMEMBER_JOIN;
 	}
 	spin_unlock_irq(&priv->lock);
 
-- 
1.7.11.3

--
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: [PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Christoph Lameter]

Reviewed-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

--
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: [PATCH V2 for-next 3/4] IB/core: Support new type of join-state for multicast

[Christoph Lameter]

Reviewed-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

--
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: [PATCH V2 for-next 4/4] IB/ipoib: Support SendOnlyFullMember MCG for SendOnly join

[Christoph Lameter]

On Thu, 5 May 2016, Erez Shitrit wrote:

> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> index 2588931..fc3e50e 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> @@ -64,6 +64,9 @@ struct ipoib_mcast_iter {
>  	unsigned int       send_only;
>  };
>
> +/* join state that allows creating mcg with sendonly member request */
> +#define SENDONLY_FULLMEMBER_JOIN	8

What are the other join states etc? Should all the join states not be
defined with struct ib_sa_mcmbember_rec in include/rdma/ib_sa.h?
--
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: [PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Hal Rosenstock]

On 5/5/2016 7:36 AM, Erez Shitrit wrote:
> Change struct ib_class_port_info to conform to IB Spec 1.3
> That in order to get specific capability mask from ClassPortInfo mad.
> 
>>From the IB Spec, ClassPortInfo section:
>         "CapabilityMask2 Bits 0-26: Additional class-specific capabilities...
>          RespTimeValue the rest 5 bits"
> 
> The new struct now has one field for capabilitymask2 (previously was the
> reserved field) and the resp_time field.
> 
> And it fixes up qib use of the field repurposed to be used as capabilitymask2:
> IB/qib: Change pma_get_classportinfo
> 
> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/qib/qib_mad.c | 4 +++-
>  include/rdma/ib_mad.h               | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
> index 0bd1837..c5f6248 100644
> --- a/drivers/infiniband/hw/qib/qib_mad.c
> +++ b/drivers/infiniband/hw/qib/qib_mad.c
> @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>  	struct ib_class_port_info *p =
>  		(struct ib_class_port_info *)pmp->data;
>  	struct qib_devdata *dd = dd_from_ibdev(ibdev);
> +	char *p_cap_mask2;
>  
>  	memset(pmp->data, 0, sizeof(pmp->data));
>  
> @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>  	 * Set the most significant bit of CM2 to indicate support for
>  	 * congestion statistics
>  	 */
> -	p->reserved[0] = dd->psxmitwait_supported << 7;
> +	p_cap_mask2 = (char *)&p->capability_mask2;
> +	p_cap_mask2[0] = dd->psxmitwait_supported << 7;
>  	/*
>  	 * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
>  	 */
> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
> index 37dd534c..0761ab9 100644
> --- a/include/rdma/ib_mad.h
> +++ b/include/rdma/ib_mad.h
> @@ -243,7 +243,7 @@ struct ib_class_port_info {
>  	u8			base_version;
>  	u8			class_version;
>  	__be16			capability_mask;
> -	u8			reserved[3];
> +	__be32			capability_mask2;
>  	u8			resp_time_value;

This looks wrong to me as CapabilityMask2 is 27 bits and RespTimeValue
is the remaining 5 bits in a 32 bit field in ClassPortInfo attribute.

-- Hal

>  	u8			redirect_gid[16];
>  	__be32			redirect_tcslfl;
> 
--
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: [PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Erez Shitrit]

On Fri, May 6, 2016 at 3:14 PM, Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> On 5/5/2016 7:36 AM, Erez Shitrit wrote:
>> Change struct ib_class_port_info to conform to IB Spec 1.3
>> That in order to get specific capability mask from ClassPortInfo mad.
>>
>>>From the IB Spec, ClassPortInfo section:
>>         "CapabilityMask2 Bits 0-26: Additional class-specific capabilities...
>>          RespTimeValue the rest 5 bits"
>>
>> The new struct now has one field for capabilitymask2 (previously was the
>> reserved field) and the resp_time field.
>>
>> And it fixes up qib use of the field repurposed to be used as capabilitymask2:
>> IB/qib: Change pma_get_classportinfo
>>
>> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/infiniband/hw/qib/qib_mad.c | 4 +++-
>>  include/rdma/ib_mad.h               | 2 +-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
>> index 0bd1837..c5f6248 100644
>> --- a/drivers/infiniband/hw/qib/qib_mad.c
>> +++ b/drivers/infiniband/hw/qib/qib_mad.c
>> @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>>       struct ib_class_port_info *p =
>>               (struct ib_class_port_info *)pmp->data;
>>       struct qib_devdata *dd = dd_from_ibdev(ibdev);
>> +     char *p_cap_mask2;
>>
>>       memset(pmp->data, 0, sizeof(pmp->data));
>>
>> @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>>        * Set the most significant bit of CM2 to indicate support for
>>        * congestion statistics
>>        */
>> -     p->reserved[0] = dd->psxmitwait_supported << 7;
>> +     p_cap_mask2 = (char *)&p->capability_mask2;
>> +     p_cap_mask2[0] = dd->psxmitwait_supported << 7;
>>       /*
>>        * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
>>        */
>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
>> index 37dd534c..0761ab9 100644
>> --- a/include/rdma/ib_mad.h
>> +++ b/include/rdma/ib_mad.h
>> @@ -243,7 +243,7 @@ struct ib_class_port_info {
>>       u8                      base_version;
>>       u8                      class_version;
>>       __be16                  capability_mask;
>> -     u8                      reserved[3];
>> +     __be32                  capability_mask2;
>>       u8                      resp_time_value;
>
> This looks wrong to me as CapabilityMask2 is 27 bits and RespTimeValue
> is the remaining 5 bits in a 32 bit field in ClassPortInfo attribute.
>
> -- Hal

Can you please elaborate more on that?
In V0 I kept both fields in the same field in the struct and you
suggested to keep them each by its own field, now we are going back
again ?

>
>>       u8                      redirect_gid[16];
>>       __be32                  redirect_tcslfl;
>>
> --
> 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
--
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: [PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Hal Rosenstock]

On 5/6/2016 8:28 AM, Erez Shitrit wrote:
> On Fri, May 6, 2016 at 3:14 PM, Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>> On 5/5/2016 7:36 AM, Erez Shitrit wrote:
>>> Change struct ib_class_port_info to conform to IB Spec 1.3
>>> That in order to get specific capability mask from ClassPortInfo mad.
>>>
>>> >From the IB Spec, ClassPortInfo section:
>>>         "CapabilityMask2 Bits 0-26: Additional class-specific capabilities...
>>>          RespTimeValue the rest 5 bits"
>>>
>>> The new struct now has one field for capabilitymask2 (previously was the
>>> reserved field) and the resp_time field.
>>>
>>> And it fixes up qib use of the field repurposed to be used as capabilitymask2:
>>> IB/qib: Change pma_get_classportinfo
>>>
>>> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  drivers/infiniband/hw/qib/qib_mad.c | 4 +++-
>>>  include/rdma/ib_mad.h               | 2 +-
>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
>>> index 0bd1837..c5f6248 100644
>>> --- a/drivers/infiniband/hw/qib/qib_mad.c
>>> +++ b/drivers/infiniband/hw/qib/qib_mad.c
>>> @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>>>       struct ib_class_port_info *p =
>>>               (struct ib_class_port_info *)pmp->data;
>>>       struct qib_devdata *dd = dd_from_ibdev(ibdev);
>>> +     char *p_cap_mask2;
>>>
>>>       memset(pmp->data, 0, sizeof(pmp->data));
>>>
>>> @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>>>        * Set the most significant bit of CM2 to indicate support for
>>>        * congestion statistics
>>>        */
>>> -     p->reserved[0] = dd->psxmitwait_supported << 7;
>>> +     p_cap_mask2 = (char *)&p->capability_mask2;
>>> +     p_cap_mask2[0] = dd->psxmitwait_supported << 7;
>>>       /*
>>>        * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
>>>        */
>>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
>>> index 37dd534c..0761ab9 100644
>>> --- a/include/rdma/ib_mad.h
>>> +++ b/include/rdma/ib_mad.h
>>> @@ -243,7 +243,7 @@ struct ib_class_port_info {
>>>       u8                      base_version;
>>>       u8                      class_version;
>>>       __be16                  capability_mask;
>>> -     u8                      reserved[3];
>>> +     __be32                  capability_mask2;
>>>       u8                      resp_time_value;
>>
>> This looks wrong to me as CapabilityMask2 is 27 bits and RespTimeValue
>> is the remaining 5 bits in a 32 bit field in ClassPortInfo attribute.
>>
>> -- Hal
> 
> Can you please elaborate more on that?
> In V0 I kept both fields in the same field in the struct and you
> suggested to keep them each by its own field, now we are going back
> again ?

It's the field widths that looks problematic to me. Everything starting
with resp_time_value is moved down/off by a byte since capability mask2
is 32 bits here followed by 8 bits of response time value rather than
both of them fitting into 32 bits.

> 
>>
>>>       u8                      redirect_gid[16];
>>>       __be32                  redirect_tcslfl;
>>>
>> --
>> 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
> 
--
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: [PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 4697 bytes --]

On Fri, May 06, 2016 at 08:32:30AM -0400, Hal Rosenstock wrote:
> On 5/6/2016 8:28 AM, Erez Shitrit wrote:
> > On Fri, May 6, 2016 at 3:14 PM, Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> >> On 5/5/2016 7:36 AM, Erez Shitrit wrote:
> >>> Change struct ib_class_port_info to conform to IB Spec 1.3
> >>> That in order to get specific capability mask from ClassPortInfo mad.
> >>>
> >>> >From the IB Spec, ClassPortInfo section:
> >>>         "CapabilityMask2 Bits 0-26: Additional class-specific capabilities...
> >>>          RespTimeValue the rest 5 bits"
> >>>
> >>> The new struct now has one field for capabilitymask2 (previously was the
> >>> reserved field) and the resp_time field.
> >>>
> >>> And it fixes up qib use of the field repurposed to be used as capabilitymask2:
> >>> IB/qib: Change pma_get_classportinfo
> >>>
> >>> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >>> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >>> ---
> >>>  drivers/infiniband/hw/qib/qib_mad.c | 4 +++-
> >>>  include/rdma/ib_mad.h               | 2 +-
> >>>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
> >>> index 0bd1837..c5f6248 100644
> >>> --- a/drivers/infiniband/hw/qib/qib_mad.c
> >>> +++ b/drivers/infiniband/hw/qib/qib_mad.c
> >>> @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
> >>>       struct ib_class_port_info *p =
> >>>               (struct ib_class_port_info *)pmp->data;
> >>>       struct qib_devdata *dd = dd_from_ibdev(ibdev);
> >>> +     char *p_cap_mask2;
> >>>
> >>>       memset(pmp->data, 0, sizeof(pmp->data));
> >>>
> >>> @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
> >>>        * Set the most significant bit of CM2 to indicate support for
> >>>        * congestion statistics
> >>>        */
> >>> -     p->reserved[0] = dd->psxmitwait_supported << 7;
> >>> +     p_cap_mask2 = (char *)&p->capability_mask2;
> >>> +     p_cap_mask2[0] = dd->psxmitwait_supported << 7;
> >>>       /*
> >>>        * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
> >>>        */
> >>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
> >>> index 37dd534c..0761ab9 100644
> >>> --- a/include/rdma/ib_mad.h
> >>> +++ b/include/rdma/ib_mad.h
> >>> @@ -243,7 +243,7 @@ struct ib_class_port_info {
> >>>       u8                      base_version;
> >>>       u8                      class_version;
> >>>       __be16                  capability_mask;
> >>> -     u8                      reserved[3];
> >>> +     __be32                  capability_mask2;
> >>>       u8                      resp_time_value;
> >>
> >> This looks wrong to me as CapabilityMask2 is 27 bits and RespTimeValue
> >> is the remaining 5 bits in a 32 bit field in ClassPortInfo attribute.
> >>
> >> -- Hal
> > 
> > Can you please elaborate more on that?
> > In V0 I kept both fields in the same field in the struct and you
> > suggested to keep them each by its own field, now we are going back
> > again ?
> 
> It's the field widths that looks problematic to me. Everything starting
> with resp_time_value is moved down/off by a byte since capability mask2
> is 32 bits here followed by 8 bits of response time value rather than
> both of them fitting into 32 bits.

V0 patch which I reviewed had the following declaration (32 bits
were replaced by 32 bits field):
@@ -243,8 +243,8 @@ struct ib_class_port_info {
        u8                      base_version;
        u8                      class_version;
        __be16                  capability_mask;
-       u8                      reserved[3];
-       u8                      resp_time_value;
+         /* 27 bits for cap_mask2, 5 bits for resp_time */
+       __be32			cap_mask2_resp_time;
	u8			redirect_gid[16];
	__be32			redirect_tcslfl;
	__be16			redirect_lid;


> 
> > 
> >>
> >>>       u8                      redirect_gid[16];
> >>>       __be32                  redirect_tcslfl;
> >>>
> >> --
> >> 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
> > 
> --
> 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 for-next 1/4] IB/core: Introduce capabilitymask2 field in ClassPortInfo mad

[Hal Rosenstock]

On 5/7/2016 7:53 AM, Leon Romanovsky wrote:
> On Fri, May 06, 2016 at 08:32:30AM -0400, Hal Rosenstock wrote:
>> On 5/6/2016 8:28 AM, Erez Shitrit wrote:
>>> On Fri, May 6, 2016 at 3:14 PM, Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
>>>> On 5/5/2016 7:36 AM, Erez Shitrit wrote:
>>>>> Change struct ib_class_port_info to conform to IB Spec 1.3
>>>>> That in order to get specific capability mask from ClassPortInfo mad.
>>>>>
>>>>> >From the IB Spec, ClassPortInfo section:
>>>>>         "CapabilityMask2 Bits 0-26: Additional class-specific capabilities...
>>>>>          RespTimeValue the rest 5 bits"
>>>>>
>>>>> The new struct now has one field for capabilitymask2 (previously was the
>>>>> reserved field) and the resp_time field.
>>>>>
>>>>> And it fixes up qib use of the field repurposed to be used as capabilitymask2:
>>>>> IB/qib: Change pma_get_classportinfo
>>>>>
>>>>> Signed-off-by: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>>> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>>  drivers/infiniband/hw/qib/qib_mad.c | 4 +++-
>>>>>  include/rdma/ib_mad.h               | 2 +-
>>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
>>>>> index 0bd1837..c5f6248 100644
>>>>> --- a/drivers/infiniband/hw/qib/qib_mad.c
>>>>> +++ b/drivers/infiniband/hw/qib/qib_mad.c
>>>>> @@ -1158,6 +1158,7 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>>>>>       struct ib_class_port_info *p =
>>>>>               (struct ib_class_port_info *)pmp->data;
>>>>>       struct qib_devdata *dd = dd_from_ibdev(ibdev);
>>>>> +     char *p_cap_mask2;
>>>>>
>>>>>       memset(pmp->data, 0, sizeof(pmp->data));
>>>>>
>>>>> @@ -1172,7 +1173,8 @@ static int pma_get_classportinfo(struct ib_pma_mad *pmp,
>>>>>        * Set the most significant bit of CM2 to indicate support for
>>>>>        * congestion statistics
>>>>>        */
>>>>> -     p->reserved[0] = dd->psxmitwait_supported << 7;
>>>>> +     p_cap_mask2 = (char *)&p->capability_mask2;
>>>>> +     p_cap_mask2[0] = dd->psxmitwait_supported << 7;
>>>>>       /*
>>>>>        * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
>>>>>        */
>>>>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
>>>>> index 37dd534c..0761ab9 100644
>>>>> --- a/include/rdma/ib_mad.h
>>>>> +++ b/include/rdma/ib_mad.h
>>>>> @@ -243,7 +243,7 @@ struct ib_class_port_info {
>>>>>       u8                      base_version;
>>>>>       u8                      class_version;
>>>>>       __be16                  capability_mask;
>>>>> -     u8                      reserved[3];
>>>>> +     __be32                  capability_mask2;
>>>>>       u8                      resp_time_value;
>>>>
>>>> This looks wrong to me as CapabilityMask2 is 27 bits and RespTimeValue
>>>> is the remaining 5 bits in a 32 bit field in ClassPortInfo attribute.
>>>>
>>>> -- Hal
>>>
>>> Can you please elaborate more on that?
>>> In V0 I kept both fields in the same field in the struct and you
>>> suggested to keep them each by its own field, now we are going back
>>> again ?
>>
>> It's the field widths that looks problematic to me. Everything starting
>> with resp_time_value is moved down/off by a byte since capability mask2
>> is 32 bits here followed by 8 bits of response time value rather than
>> both of them fitting into 32 bits.
> 
> V0 patch which I reviewed had the following declaration (32 bits
> were replaced by 32 bits field):
> @@ -243,8 +243,8 @@ struct ib_class_port_info {
>         u8                      base_version;
>         u8                      class_version;
>         __be16                  capability_mask;
> -       u8                      reserved[3];
> -       u8                      resp_time_value;
> +         /* 27 bits for cap_mask2, 5 bits for resp_time */
> +       __be32			cap_mask2_resp_time;

Layout here is correct in that it takes 4 8 bit values and makes a
single 32 value. In my comment, I was asking whether the cap mask and
resp time fields could be split but it looks like that is difficult (I
don't see way to do it and if so, just ignore my comment and sorry for
the confusion).

> 	u8			redirect_gid[16];
> 	__be32			redirect_tcslfl;
> 	__be16			redirect_lid; 
>>
>>>
>>>>
>>>>>       u8                      redirect_gid[16];
>>>>>       __be32                  redirect_tcslfl;
>>>>>
>>>> --
>>>> 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
>>>
>> --
>> 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
--
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: [PATCH V2 for-next 4/4] IB/ipoib: Support SendOnlyFullMember MCG for SendOnly join

[Erez Shitrit]

On Fri, May 6, 2016 at 5:29 AM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, 5 May 2016, Erez Shitrit wrote:
>
>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> index 2588931..fc3e50e 100644
>> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> @@ -64,6 +64,9 @@ struct ipoib_mcast_iter {
>>       unsigned int       send_only;
>>  };
>>
>> +/* join state that allows creating mcg with sendonly member request */
>> +#define SENDONLY_FULLMEMBER_JOIN     8
>
> What are the other join states etc? Should all the join states not be
> defined with struct ib_sa_mcmbember_rec in include/rdma/ib_sa.h?

In the current code there is no define/enum for all the types, all the
code that use them (cma.c, multicast.c, ipoib_multicast.c) use a
specific digit ("1" for full-member in most cases)
probably we need to send a new patch (out of that patchset) that will
enum all of them.

> --
> 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
--
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