diff mbox

[rdma-next,3/4] RDMA/netink: Export lids and sm_lids

Message ID 20170628133445.16550-4-leon@kernel.org (mailing list archive)
State Superseded
Headers show

Commit Message

Leon Romanovsky June 28, 2017, 1:34 p.m. UTC 
From: Leon Romanovsky <leonro@mellanox.com>

According to the IB specification, the LID and SM_LID
are 16-bit wide, but to support OmniPath users, export
it as 32-bit value from the beginning.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c  | 7 ++++++-
 include/uapi/rdma/rdma_netlink.h | 8 ++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

--
2.13.1

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

Comments

Jason Gunthorpe June 28, 2017, 4:05 p.m. UTC | #1 
On Wed, Jun 28, 2017 at 04:34:44PM +0300, Leon Romanovsky wrote:
> +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_LID, attr.lid))
> +		return -EMSGSIZE;
> +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
> +		return -EMSGSIZE;

Is this careful to only stuff attributes if they actuall exists? Eg
these should not be present for iwarp/roce/etc

Same comment for all attributes. Only present if they are valid.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky June 28, 2017, 4:16 p.m. UTC | #2 
On Wed, Jun 28, 2017 at 10:05:13AM -0600, Jason Gunthorpe wrote:
> On Wed, Jun 28, 2017 at 04:34:44PM +0300, Leon Romanovsky wrote:
> > +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_LID, attr.lid))
> > +		return -EMSGSIZE;
> > +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
> > +		return -EMSGSIZE;
>
> Is this careful to only stuff attributes if they actuall exists? Eg
> these should not be present for iwarp/roce/etc
>
> Same comment for all attributes. Only present if they are valid.

It will be zero if it is not exist, but you are right it is better to
avoid to fill unneeded fields.

>
> Jason
Jason Gunthorpe June 28, 2017, 4:19 p.m. UTC | #3 
On Wed, Jun 28, 2017 at 07:16:16PM +0300, Leon Romanovsky wrote:
> On Wed, Jun 28, 2017 at 10:05:13AM -0600, Jason Gunthorpe wrote:
> > On Wed, Jun 28, 2017 at 04:34:44PM +0300, Leon Romanovsky wrote:
> > > +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_LID, attr.lid))
> > > +		return -EMSGSIZE;
> > > +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
> > > +		return -EMSGSIZE;
> >
> > Is this careful to only stuff attributes if they actuall exists? Eg
> > these should not be present for iwarp/roce/etc
> >
> > Same comment for all attributes. Only present if they are valid.
> 
> It will be zero if it is not exist, but you are right it is better to
> avoid to fill unneeded fields.

Not existing is the netlink way. Same comment for everything
attributes. Many of the guids do not apply to iwarp, IIRC..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky June 28, 2017, 4:24 p.m. UTC | #4 
On Wed, Jun 28, 2017 at 10:19:03AM -0600, Jason Gunthorpe wrote:
> On Wed, Jun 28, 2017 at 07:16:16PM +0300, Leon Romanovsky wrote:
> > On Wed, Jun 28, 2017 at 10:05:13AM -0600, Jason Gunthorpe wrote:
> > > On Wed, Jun 28, 2017 at 04:34:44PM +0300, Leon Romanovsky wrote:
> > > > +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_LID, attr.lid))
> > > > +		return -EMSGSIZE;
> > > > +	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
> > > > +		return -EMSGSIZE;
> > >
> > > Is this careful to only stuff attributes if they actuall exists? Eg
> > > these should not be present for iwarp/roce/etc
> > >
> > > Same comment for all attributes. Only present if they are valid.
> >
> > It will be zero if it is not exist, but you are right it is better to
> > avoid to fill unneeded fields.
>
> Not existing is the netlink way. Same comment for everything
> attributes. Many of the guids do not apply to iwarp, IIRC..

I got it, this is why I prefer to have basic RDMAtool with minimal
prints (device name + caps) be accepted and only after that start work
on every field in serial manner.

Thanks

>
> Jason
diff mbox

Patch

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 755b8167280b..6b290dada589 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -45,6 +45,8 @@  static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_GUID]		= { .type = NLA_U64 },
 	[RDMA_NLDEV_ATTR_SYS_IMAGE_GUID] = { .type = NLA_U64 },
 	[RDMA_NLDEV_ATTR_SUBNET_PREFIX]	= { .type = NLA_U64 },
+	[RDMA_NLDEV_ATTR_LID]		= { .type = NLA_U32 },
+	[RDMA_NLDEV_ATTR_SM_LID]	= { .type = NLA_U32 },
 };

 static int fill_dev_info(struct sk_buff *msg, struct ib_device *device)
@@ -99,7 +101,10 @@  static int fill_port_info(struct sk_buff *msg,
 	if (nla_put_u64_64bit(msg, RDMA_NLDEV_ATTR_SUBNET_PREFIX,
 			      attr.subnet_prefix, 0))
 		return -EMSGSIZE;
-
+	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_LID, attr.lid))
+		return -EMSGSIZE;
+	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_SM_LID, attr.sm_lid))
+		return -EMSGSIZE;
 	return 0;
 }

diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 5fec88038e6e..22e4870f2e31 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -284,6 +284,14 @@  enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_SUBNET_PREFIX,		/* u64 */

+	/*
+	 * Local Identifier (LID),
+	 * According to IB specification, It is 16-bit address assigned
+	 * by the Subnet Manager. Extended to be 32-bit for OmniPath users.
+	 */
+	RDMA_NLDEV_ATTR_LID,			/* u32 */
+	RDMA_NLDEV_ATTR_SM_LID,			/* u32 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */