diff mbox

[libibverbs,V1] Align Flow Steering API to the extension verbs scheme

Message ID 1400159784-24423-1-git-send-email-ogerlitz@r-vnc04.mtr.labs.mlnx (mailing list archive)
State Rejected
Headers show

Commit Message

Or Gerlitz May 15, 2014, 1:16 p.m. UTC
From: Matan Barak <matanb@mellanox.com>

The receive flow steering API didn't conform with the extension
verbs:
1. It introduced VERBS_CONTEXT_CREATE_FLOW and
   VERBS_CONTEXT_DESTROY_FLOW, even though there isn't a
   non-extended structure it could be converted to.
2. It introduces drv_ functions, which was dropped in one
   of the design phases of the extension verbs.

This patch fixes the above. It drops VERBS_CONTEXT_CREATE_FLOW
and VERBS_CONTEXT_DESTROY_FLOW and replaces the drv_function with
a reserved stub.

Also add man page for the two new verbs

Signed-off-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---

Hi Roland -- this addresses all the comments by Jason who gave a
ack/reviewed-by note for both the libibverbs and libmlx4 fixes.

So you can move ahead and conduct point releases for both libs.

 Makefile.am                |    6 ++-
 include/infiniband/verbs.h |   29 +++++--------
 man/ibv_create_flow.3      |   99 ++++++++++++++++++++++++++++++++++++++++++++
 src/device.c               |    4 --
 4 files changed, 115 insertions(+), 23 deletions(-)
 create mode 100644 man/ibv_create_flow.3

Comments

Roland Dreier May 20, 2014, 1:05 a.m. UTC | #1
On Thu, May 15, 2014 at 6:16 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Hi Roland -- this addresses all the comments by Jason who gave a
> ack/reviewed-by note for both the libibverbs and libmlx4 fixes.
>
> So you can move ahead and conduct point releases for both libs.


OK... so I should do one more libmlx4 release before you take over?

 - R.
--
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
Or Gerlitz May 20, 2014, 6:59 a.m. UTC | #2
On 20/05/2014 04:05, Roland Dreier wrote:
> On Thu, May 15, 2014 at 6:16 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> Hi Roland -- this addresses all the comments by Jason who gave a
>> ack/reviewed-by note for both the libibverbs and libmlx4 fixes.
>>
>> So you can move ahead and conduct point releases for both libs.
>
> OK... so I should do one more libmlx4 release before you take over?

YES, please.


--
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
Or Gerlitz June 11, 2014, 9:53 a.m. UTC | #3
On 20/05/2014 09:59, Or Gerlitz wrote:
> On 20/05/2014 04:05, Roland Dreier wrote:
>> On Thu, May 15, 2014 at 6:16 AM, Or Gerlitz <ogerlitz@mellanox.com> 
>> wrote:
>>> Hi Roland -- this addresses all the comments by Jason who gave a
>>> ack/reviewed-by note for both the libibverbs and libmlx4 fixes.
>>>
>>> So you can move ahead and conduct point releases for both libs.
>>
>> OK... so I should do one more libmlx4 release before you take over?
>
> YES, please.
>

Hi Roland,

Can you please go and do this joint/point release of libibverbs (1.1.9) 
and libmlx4 (1.0.7)
which fixes the issues with flow steering?!

I got distro contact which was about to take now 1.1.8/1.0.6 and asked 
them to wait few days and take the proper bits, so...

Or.
--
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
Or Gerlitz June 11, 2014, 10:51 a.m. UTC | #4
On 11/06/2014 12:53, Or Gerlitz wrote:
> On 20/05/2014 09:59, Or Gerlitz wrote:
>> On 20/05/2014 04:05, Roland Dreier wrote:
>>> On Thu, May 15, 2014 at 6:16 AM, Or Gerlitz <ogerlitz@mellanox.com> 
>>> wrote:
>>>> Hi Roland -- this addresses all the comments by Jason who gave a
>>>> ack/reviewed-by note for both the libibverbs and libmlx4 fixes.
>>>>
>>>> So you can move ahead and conduct point releases for both libs.
>>>
>>> OK... so I should do one more libmlx4 release before you take over?
>>
>> YES, please.
>>
>
> Hi Roland,
>
> Can you please go and do this joint/point release of libibverbs 
> (1.1.9) and libmlx4 (1.0.7)
> which fixes the issues with flow steering?!

FWIW, if it helps you, these are the relevant patchwork entries

https://patchwork.kernel.org/patch/4182461/
https://patchwork.kernel.org/patch/4174891/


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

Patch

diff --git a/Makefile.am b/Makefile.am
index a6767de..3a40f3e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -58,7 +58,7 @@  man_MANS = man/ibv_asyncwatch.1 man/ibv_devices.1 man/ibv_devinfo.1	\
     man/ibv_query_srq.3 man/ibv_rate_to_mult.3 man/ibv_reg_mr.3		\
     man/ibv_req_notify_cq.3 man/ibv_resize_cq.3 man/ibv_rate_to_mbps.3  \
     man/ibv_create_qp_ex.3 man/ibv_create_srq_ex.3 man/ibv_open_xrcd.3  \
-    man/ibv_get_srq_num.3 man/ibv_open_qp.3
+    man/ibv_get_srq_num.3 man/ibv_open_qp.3 man/ibv_create_flow.3
 
 DEBIAN = debian/changelog debian/compat debian/control debian/copyright \
     debian/ibverbs-utils.install debian/libibverbs1.install \
@@ -94,6 +94,7 @@  install-data-hook:
 	$(RM) ibv_port_state_str.3 && \
 	$(RM) mbps_to_ibv_rate.3 && \
 	$(RM) ibv_close_xrcd.3 && \
+	$(RM) ibv_destroy_flow.3 && \
 	$(LN_S) ibv_get_async_event.3 ibv_ack_async_event.3 && \
 	$(LN_S) ibv_get_cq_event.3 ibv_ack_cq_events.3 && \
 	$(LN_S) ibv_open_device.3 ibv_close_device.3 && \
@@ -111,4 +112,5 @@  install-data-hook:
 	$(LN_S) ibv_event_type_str.3 ibv_node_type_str.3 && \
 	$(LN_S) ibv_event_type_str.3 ibv_port_state_str.3 && \
 	$(LN_S) ibv_rate_to_mbps.3 mbps_to_ibv_rate.3 && \
-	$(LN_S) ibv_open_xrcd.3 ibv_close_xrcd.3
+	$(LN_S) ibv_open_xrcd.3 ibv_close_xrcd.3 && \
+	$(LN_S) ibv_create_flow.3 ibv_destroy_flow.3
diff --git a/include/infiniband/verbs.h b/include/infiniband/verbs.h
index cfa1156..9826b72 100644
--- a/include/infiniband/verbs.h
+++ b/include/infiniband/verbs.h
@@ -944,21 +944,16 @@  enum verbs_context_mask {
 	VERBS_CONTEXT_XRCD	= 1 << 0,
 	VERBS_CONTEXT_SRQ	= 1 << 1,
 	VERBS_CONTEXT_QP	= 1 << 2,
-	VERBS_CONTEXT_CREATE_FLOW = 1 << 3,
-	VERBS_CONTEXT_DESTROY_FLOW = 1 << 4,
-	VERBS_CONTEXT_RESERVED	= 1 << 5
+	VERBS_CONTEXT_RESERVED	= 1 << 3
 };
 
 struct verbs_context {
 	/*  "grows up" - new fields go here */
-	int (*drv_ibv_destroy_flow) (struct ibv_flow *flow);
-	int (*lib_ibv_destroy_flow) (struct ibv_flow *flow);
-	struct ibv_flow * (*drv_ibv_create_flow) (struct ibv_qp *qp,
-						  struct ibv_flow_attr
-						  *flow_attr);
-	struct ibv_flow * (*lib_ibv_create_flow) (struct ibv_qp *qp,
-						  struct ibv_flow_attr
-						  *flow_attr);
+	void (*_reserved_2)(void);
+	int (*destroy_flow)(struct ibv_flow *flow);
+	void (*_reserved_1)(void);
+	struct ibv_flow * (*create_flow)(struct ibv_qp *qp,
+					 struct ibv_flow_attr *flow_attr);
 	struct ibv_qp *(*open_qp)(struct ibv_context *context,
 			struct ibv_qp_open_attr *attr);
 	struct ibv_qp *(*create_qp_ex)(struct ibv_context *context,
@@ -1111,20 +1106,20 @@  static inline struct ibv_flow *ibv_create_flow(struct ibv_qp *qp,
 					       struct ibv_flow_attr *flow)
 {
 	struct verbs_context *vctx = verbs_get_ctx_op(qp->context,
-						      lib_ibv_create_flow);
-	if (!vctx || !vctx->lib_ibv_create_flow)
+						      create_flow);
+	if (!vctx)
 		return NULL;
 
-	return vctx->lib_ibv_create_flow(qp, flow);
+	return vctx->create_flow(qp, flow);
 }
 
 static inline int ibv_destroy_flow(struct ibv_flow *flow_id)
 {
 	struct verbs_context *vctx = verbs_get_ctx_op(flow_id->context,
-						      lib_ibv_destroy_flow);
-	if (!vctx || !vctx->lib_ibv_destroy_flow)
+						      destroy_flow);
+	if (!vctx)
 		return -ENOSYS;
-	return vctx->lib_ibv_destroy_flow(flow_id);
+	return vctx->destroy_flow(flow_id);
 }
 
 /**
diff --git a/man/ibv_create_flow.3 b/man/ibv_create_flow.3
new file mode 100644
index 0000000..f1003d0
--- /dev/null
+++ b/man/ibv_create_flow.3
@@ -0,0 +1,99 @@ 
+.TH IBV_CREATE_FLOW 3 2013-08-21 libibverbs "Libibverbs Programmer's Manual"
+.SH "NAME"
+ibv_create_flow, ibv_destroy_flow \- create or destroy flow steering rules
+.SH "SYNOPSIS"
+.nf
+.B #include <infiniband/verbs.h>
+.sp
+.BI "struct ibv_flow *ibv_create_flow(struct ibv_qp " "*qp" ,
+.BI "                                 struct ibv_flow_attr " "*flow");
+.BI "int ibv_destroy_flow(struct ibv_flow " "*flow_id");
+.sp
+.fi
+.SH "DESCRIPTION"
+.SS ibv_create_flow()
+allows a user application QP
+.I qp
+to be attached into a specified flow
+.I flow
+which is defined in
+.I <infiniband/verbs.h>
+.PP
+.nf
+struct ibv_flow_attr {
+.in +8
+uint32_t comp_mask;						/* Future extendibility */
+enum ibv_flow_attr_type type;				/* Rule type - see below */
+uint16_t size;							/* Size of command */
+uint16_t priority;						/* Rule priority - See below */
+uint8_t num_of_specs;					/* Number of ibv_flow_spec_xxx */
+uint8_t port;							/* The uplink port number */
+uint32_t flags;						/* Extra flags for rule - see below */
+/* Following are the optional layers according to user request
+ * struct ibv_flow_spec_xxx
+ * struct ibv_flow_spec_yyy
+ */
+.in -8
+};
+.sp
+.nf
+enum ibv_flow_attr_type {
+.in +8
+IBV_FLOW_ATTR_NORMAL		= 0x0,		/* steering according to rule specifications */
+IBV_FLOW_ATTR_ALL_DEFAULT	= 0x1,		/* default unicast and multicast rule - receive all Eth traffic which isn't steered to any QP */
+IBV_FLOW_ATTR_MC_DEFAULT		= 0x2,		/* default multicast rule - receive all Eth multicast traffic which isn't steered to any QP */
+.in -8
+};
+.sp
+.nf
+enum ibv_flow_flags {
+.in +8
+IBV_FLOW_ATTR_FLAGS_ALLOW_LOOP_BACK = 1,	/* Apply the rules on packets that were sent from the attached QP through loopback. IB only.*/
+.in -8
+};
+.fi
+.PP
+Each header struct holds the relevant network layer parameters for matching.To enforce the match, the
+user sets a mask for each parameter. If the bit is set in the mask, the corresponding bit in the value should be matched.
+.br
+Note that most vendors support either full mask (all "1"s) or zero mask (all "0"s).
+.br
+.B Network paramters in the relevant network structs should be given in network order (big endian).
+
+.SS Flow domains and priority
+Flow steering defines the concept of domain and priority. Each domain represents a user agent that can attach a flow. The domains are prioritized. A higher priority domain will always supersede a lower priority domain when their flow specifications overlap. In addition to the domain, there is priority within each of the domains. Each domain has at most 2^12 priorities. A lower priority numeric value (higher priority) takes precedence over matching rules with higher numeric priority value (lower priority). It is important to note that the priority value of a flow spec is used not only to establish the precedence of conflicting flow matches but also as a way to abstract the order on which flow specs are tested for matches. Flows with higher priorities will be tested before flows with lower priorities.
+.br
+.B IB verbs have the higher priority domain.
+.PP
+.SS ibv_destroy_flow()
+destroys the flow
+.I flow_id\fR.
+.SH "RETURN VALUE"
+.B ibv_create_flow()
+returns a pointer to the flow, or NULL if the request fails. In case of an error, errno is updated.
+.PP
+.B ibv_destroy_flow()
+returns 0 on success, or the value of errno on failure (which indicates the failure reason).
+.SH "ERRORS"
+.SS EINVAL
+.B ibv_create_flow()
+flow specification, QP or priority are invalid
+.PP
+.B ibv_destroy_flow()
+flow_id is invalid
+.SS ENOMEM
+Couldn't create/destory flow, not enough memory
+.SS ENXIO
+Device managed flow steering isn't currently supported
+.SS EPERM
+No permissions to add the flow steering rule
+.SH "NOTES"
+These verbs are available only for devices supporting IBV_DEVICE_MANAGED_FLOW_STEERING and
+only for QPs of Transport Service Type
+.BR IBV_QPT_UD
+or
+.BR IBV_QPT_RAW_PACKET
+.PP
+.SH "AUTHORS"
+.TP
+Matan Barak <matanb@mellanox.com>         Hadar Hen Zion <hadarh@mellanox.com>
diff --git a/src/device.c b/src/device.c
index 9642ead..beb7b3c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -169,10 +169,6 @@  struct ibv_context *__ibv_open_device(struct ibv_device *device)
 		 * context_ex->lib_new_func1 = __verbs_new_func1;
 		 * context_ex->lib_new_func2 = __verbs_new_func2;
 		 */
-		 context_ex->lib_ibv_create_flow =
-			 context_ex->drv_ibv_create_flow;
-		 context_ex->lib_ibv_destroy_flow =
-			 context_ex->drv_ibv_destroy_flow;
 	}
 
 	context->device = device;