> -----Original Message----- > From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > Sent: Monday, February 19, 2018 4:58 PM > To: Steve Wise > Cc: dledford@xxxxxxxxxx; leon@xxxxxxxxxx; yishaih@xxxxxxxxxxxx; linux- > rdma@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v2 rdma-next 3/9] RDMA/nldev: provide detailed CM_ID > information > > On Mon, Feb 19, 2018 at 08:16:34AM -0800, Steve Wise wrote: > > > +static int nldev_res_get_cm_id_dumpit(struct sk_buff *skb, > > + struct netlink_callback *cb) > > +{ > > + struct nlattr *tb[RDMA_NLDEV_ATTR_MAX]; > > + struct rdma_id_private *id_priv = NULL; > > + struct rdma_restrack_entry *res; > > + int err, ret = 0, idx = 0; > > + struct nlattr *table_attr; > > + struct ib_device *device; > > + int start = cb->args[0]; > > + struct nlmsghdr *nlh; > > + u32 index, port = 0; > > I think this function is looking like a lot of code duplication.. Can something > be done? > > Like a general function that accepts an ID and a callback function ?? I'll look into this. > > > @@ -350,6 +352,34 @@ enum rdma_nldev_attr { > > */ > > RDMA_NLDEV_ATTR_RES_KERN_NAME, /* string */ > > > > + RDMA_NLDEV_ATTR_RES_CM_ID, /* nested table */ > > + RDMA_NLDEV_ATTR_RES_CM_ID_ENTRY, /* nested table */ > > + /* > > + * rdma_cm_id port space. > > + */ > > + RDMA_NLDEV_ATTR_RES_PS, /* u32 */ > > + /* > > + * Source and destination IP address and port attributes. > > + */ > > + RDMA_NLDEV_ATTR_RES_IPV4_SADDR, /* u8[4] */ > > + RDMA_NLDEV_ATTR_RES_IPV4_DADDR, /* u8[4] */ > > + RDMA_NLDEV_ATTR_RES_IPV6_SADDR, /* u8[16] */ > > + RDMA_NLDEV_ATTR_RES_IPV6_DADDR, /* u8[16] */ > > Hum, that doesn't seem very netlink-like. Maybe this should just pass > a kernel_sockaddr_storage? I could do that I guess. > > > + /* > > + * ARPHRD_INFINIBAND, ARPHRD_ETHER, ... > > + */ > > + RDMA_NLDEV_ATTR_RES_DEV_TYPE, /* u8 */ > > + /* > > + * enum enum rdma_transport_type (IB, IWARP, ...) > > + */ > > + RDMA_NLDEV_ATTR_RES_TRANSPORT_TYPE, /* u8 */ > > + /* > > + * enum rdma_network_type (IB, IPv4, IPv6,...) > > + */ > > + RDMA_NLDEV_ATTR_RES_NETWORK_TYPE, /* u8 */ > > All of these enums are in a uapi header someplace, right? Yes. Include/uapi/rdma/rdma_netlink.h > > Not sure I like this - why so many ways to say almost the same thing? > I simply took the approach to add a NLDEV type for each field being passed up. That allows the user mode tool to parse the types into human-readable format. IE it will know what the valid values are for dev_type, transport_type, etc... Similar to what is there already for the qp resource. Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
