On 21/5/2014 11:10 PM, Jason Gunthorpe wrote:
On Sun, May 18, 2014 at 12:38:48PM +0300, Or Gerlitz wrote:
+enum ibv_query_port_ex_attr_mask {
+ IBV_QUERY_PORT_EX_STATE = 1 << 0,
+ IBV_QUERY_PORT_EX_MAX_MTU = 1 << 1,
+ IBV_QUERY_PORT_EX_ACTIVE_MTU = 1 << 2,
+ IBV_QUERY_PORT_EX_GID_TBL_LEN = 1 << 3,
+ IBV_QUERY_PORT_EX_CAP_FLAGS = 1 << 4,
+ IBV_QUERY_PORT_EX_MAX_MSG_SZ = 1 << 5,
+ IBV_QUERY_PORT_EX_BAD_PKEY_CNTR = 1 << 6,
+ IBV_QUERY_PORT_EX_QKEY_VIOL_CNTR = 1 << 7,
+ IBV_QUERY_PORT_EX_PKEY_TBL_LEN = 1 << 8,
+ IBV_QUERY_PORT_EX_LID = 1 << 9,
+ IBV_QUERY_PORT_EX_SM_LID = 1 << 10,
+ IBV_QUERY_PORT_EX_LMC = 1 << 11,
+ IBV_QUERY_PORT_EX_MAX_VL_NUM = 1 << 12,
+ IBV_QUERY_PORT_EX_SM_SL = 1 << 13,
+ IBV_QUERY_PORT_EX_SUBNET_TIMEOUT = 1 << 14,
+ IBV_QUERY_PORT_EX_INIT_TYPE_REPLY = 1 << 15,
+ IBV_QUERY_PORT_EX_ACTIVE_WIDTH = 1 << 16,
+ IBV_QUERY_PORT_EX_ACTIVE_SPEED = 1 << 17,
+ IBV_QUERY_PORT_EX_PHYS_STATE = 1 << 18,
+ IBV_QUERY_PORT_EX_LINK_LAYER = 1 << 19,
+ /* mask of the fields that exists in the standard query_port_command */
+ IBV_QUERY_PORT_EX_STD_MASK = (1 << 20) - 1,
+ /* mask of all supported fields */
+ IBV_QUERY_PORT_EX_MASK = IBV_QUERY_PORT_EX_STD_MASK,
+};
OK
+struct ibv_port_attr_ex {
+ enum ibv_port_state state;
+ enum ibv_mtu max_mtu;
+ enum ibv_mtu active_mtu;
+ int gid_tbl_len;
+ uint32_t port_cap_flags;
+ uint32_t max_msg_sz;
+ uint32_t bad_pkey_cntr;
+ uint32_t qkey_viol_cntr;
+ uint16_t pkey_tbl_len;
+ uint16_t lid;
+ uint16_t sm_lid;
+ uint8_t lmc;
+ uint8_t max_vl_num;
+ uint8_t sm_sl;
+ uint8_t subnet_timeout;
+ uint8_t init_type_reply;
+ uint8_t active_width;
+ uint8_t active_speed;
+ uint8_t phys_state;
+ uint8_t link_layer;
+ uint8_t reserved;
OK
+ uint32_t comp_mask;
This uses the first 20 bits already, should comp_mask just be 64 bits
off the bat?
First of all, I think that comp_mask should be the same type for all
extension verbs and since ibv_flow_attr already uses a 32bit comp_mask,
so should this verb.
Moreover, I don't think we expect to reach 32 values anytime soon.
In term of future scalability, I understand that the last bit is
reserved for comp_mask2 field.
@@ -998,6 +1050,8 @@ enum verbs_context_mask {
struct verbs_context {
/* "grows up" - new fields go here */
+ int (*query_port_ex)(struct ibv_context *context, uint8_t port_num,
+ struct ibv_port_attr_ex *port_attr);
OK
+static inline int ibv_query_port_ex(struct ibv_context *context,
+ uint8_t port_num,
+ struct ibv_port_attr_ex *port_attr)
+{
+ struct verbs_context *vctx;
+
+ port_attr->link_layer = IBV_LINK_LAYER_UNSPECIFIED;
+ port_attr->reserved = 0;
We don't need this. All the calls to ibv_query_port already set these
values and we can simply require that all implementations of
ibv_query_port_ex set them too.
Correct
+ if (0 == port_attr->comp_mask)
+ return ibv_query_port(context, port_num,
+ (struct ibv_port_attr *)port_attr);
I'm confused what this is doing? Why is 0 ever a valid comp_mask?
I'll remove this.
+ /* Check that only valid flags were given */
+ if (port_attr->comp_mask & ~IBV_QUERY_PORT_EX_MASK) {
+ errno = EINVAL;
+ return -errno;
+ }
And this doesn't seem to make sense either.
Sanity check - the user should provide a combination of
ibv_query_port_ex_attr_mask flags.
+ vctx = verbs_get_ctx_op(context, query_port_ex);
+
+ if (!vctx) {
+ /* Fallback to legacy mode */
+ if (!(port_attr->comp_mask & ~IBV_QUERY_PORT_EX_STD_MASK))
+ return ibv_query_port(context, port_num,
+ (struct ibv_port_attr
*)port_attr);
+
+ /* Unsupported field was requested */
+ errno = ENOSYS;
+ return -errno;
+ }
+
+ return vctx->query_port_ex(context, port_num, port_attr);
+}
+
#define ibv_query_port(context, port_num, port_attr) \
___ibv_query_port(context, port_num, port_attr)
diff --git a/man/ibv_query_port_ex.3 b/man/ibv_query_port_ex.3
new file mode 100644
index 0000000..07073fd
+++ b/man/ibv_query_port_ex.3
@@ -0,0 +1,97 @@
+.\" -*- nroff -*-
+.\"
+.TH IBV_QUERY_PORT_EX 3 2006-10-31 libibverbs "Libibverbs Programmer's Manual"
+.SH "NAME"
+ibv_query_port_ex \- query an RDMA port's attributes
+.SH "SYNOPSIS"
+.nf
+.B #include <infiniband/verbs.h>
+.sp
+.BI "int ibv_query_port_ex(struct ibv_context " "*context" ", uint8_t "
"port_num" ,
+.BI " struct ibv_port_attr_ex " "*port_attr" ");
+.fi
+.SH "DESCRIPTION"
+.B ibv_query_port_ex()
I feel it would be nicer to just patch the existing ibv_query_port man
page to have the new call and a paragraph about the extra field.
Similar to how 'man accept' works with accept and accept4
Ok
+returns the attributes of port
+.I port_num
+for device context
+.I context
+through the pointer
+.I port_attr\fR.
+The argument
+.I port_attr
+is an ibv_port_attr struct, as defined in <infiniband/verbs.h>.
^^^^^^^
No it isn't. Please proof-read everything.
Jason
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
