[ofw] RE: [PATCH] Add uverbs version
Leonid Keller
leonid at mellanox.co.il
Wed Mar 26 11:32:14 PDT 2008
> + ib_status = pfn_uvp_ifc( &p_vca_intf->user_verbs );
> + if (ib_status == IB_UNSUPPORTED)
You presume the only possible error return code from pfn_uvp_ifc.
It's in general not good. Someone can eventually change pfn_uvp_ifc ,
adding a new return code and you will regard it as success.
I'd suggest something like that:
if (ib_status != IB_SUCCESS)
{
al_unload_uvp( h_lib );
if (ib_status == IB_UNSUPPORTED)
AL_PRINT_EXIT(TRACE_LEVEL_ERROR ,AL_DBG_CA,
("Unsupported uvp interface version, expected =
0x%x\n", UVERBS_VERSION));
return ib_status;
}
________________________________
From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Fab Tillier
Sent: Wednesday, March 26, 2008 6:30 PM
To: Reuven Amitai; ofw at lists.openfabrics.org
Subject: [ofw] RE: [PATCH] Add uverbs version
That looks fine to me from a quick look at it.
-Fab
From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Reuven Amitai
Sent: Wednesday, March 26, 2008 9:27 AM
To: Fab Tillier; ofw at lists.openfabrics.org
Subject: [ofw] RE: [PATCH] Add uverbs version
Hi,
I have reversed the uvp version setting. How about it now ?
Reuven.
Index: core/al/user/ual_ca.c
===================================================================
--- core/al/user/ual_ca.c (revision 992)
+++ core/al/user/ual_ca.c (working copy)
@@ -64,6 +64,7 @@
ual_get_uvp_name_ioctl_t al_ioctl;
uintn_t bytes_ret;
cl_status_t cl_status;
+ ib_api_status_t ib_status;
void *h_lib;
uvp_get_interface_t pfn_uvp_ifc;
@@ -131,7 +132,17 @@
}
/* Query the vendor-supported user-mode functions */
- pfn_uvp_ifc( &p_vca_intf->user_verbs );
+ p_vca_intf->user_verbs.version = UVERBS_VERSION;
+
+ ib_status = pfn_uvp_ifc( &p_vca_intf->user_verbs );
+ if (ib_status == IB_UNSUPPORTED)
+ {
+ al_unload_uvp( h_lib );
+ AL_PRINT_EXIT(TRACE_LEVEL_ERROR ,AL_DBG_CA,
+ ("Unsupported uvp interface version, expected
= 0x%x\n", UVERBS_VERSION));
+ return IB_UNSUPPORTED;
+ }
+
p_vca_intf->h_uvp_lib = h_lib;
AL_EXIT( AL_DBG_CA );
return IB_SUCCESS;
Index: hw/mthca/user/mlnx_ual_main.c
===================================================================
--- hw/mthca/user/mlnx_ual_main.c (revision 992)
+++ hw/mthca/user/mlnx_ual_main.c (working copy)
@@ -143,7 +143,9 @@
/*
* Version of the header file this interface export can
handle
*/
- p_uvp->version = 0x101;
+ if (p_uvp->version != UVERBS_VERSION)
+ return IB_UNSUPPORTED;
+
p_uvp->guid = 0x12345678;
/*
Index: inc/user/iba/ib_uvp.h
===================================================================
--- inc/user/iba/ib_uvp.h (revision 992)
+++ inc/user/iba/ib_uvp.h (working copy)
@@ -116,6 +116,17 @@
*
*/
+/*
+ * Version that identifies this version of the header file for
interface
+ * definition.
+ */
+#define UVERBS_MAJOR_VER (0x0002)
+#define UVERBS_MINOR_VER (0x0000)
+
+#define UVERBS_VERSION (((UVERBS_MAJOR_VER) << 16)
| (UVERBS_MINOR_VER))
+#define MK_UVERBS_VERSION(maj,min) ((((maj) & 0xFFFF) << 16) |
\
+ ((min) &
0xFFFF))
+
/******/
/******/
@@ -3469,6 +3480,8 @@
* The registration is successful.
* IB_INSUFFICIENT_MEMORY
* Insufficient memory to satisfy request
+* IB_UNSUPPORTED
+* The interface version requested is not supported
*
* PORTABILITY
* User mode
________________________________
From: Fab Tillier [mailto:ftillier at windows.microsoft.com]
Sent: Wednesday, March 26, 2008 9:42 AM
To: Reuven Amitai; ofw at lists.openfabrics.org
Subject: RE: [PATCH] Add uverbs version
Hi Reuven,
The version needs to be supplied as input to the UVP, before it
fills the interface structure. Imagine a call into a version 2 UVP with
a version 1 interface that's smaller than the V2 interface. The UVP
would fill it as if it was a V2 interface and overrun the buffer.
Version should be set in the interface before calling
open_vendor_lib, and the UVP should check that it's an interface that it
supports and fill in the appropriate one. This also allows the UVP to
support multiple interfaces.
Cheers,
-Fab
From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Reuven Amitai
Sent: Wednesday, March 26, 2008 12:39 AM
To: ofw at lists.openfabrics.org
Subject: [ofw] [PATCH] Add uverbs version
Hi,
The patch adds version to uvp interface the same way the kvp has
( inc\iba\ib_ci.h ).
It also adds checking for the returned version of the uvp.
comments / suggestions are welcome.
Reuven.
Index: core/al/user/ual_ci_ca.c
===================================================================
--- core/al/user/ual_ci_ca.c (revision 992)
+++ core/al/user/ual_ci_ca.c (working copy)
@@ -162,6 +162,17 @@
add_ci_ca( p_ci_ca );
open_vendor_lib( ca_guid, &p_ci_ca->verbs );
+ /* Check vendor lib version */
+ if (p_ci_ca->verbs.user_verbs.version != UVERBS_VERSION)
+ {
+ p_ci_ca->obj.pfn_destroy( &p_ci_ca->obj, NULL );
+ AL_PRINT(TRACE_LEVEL_ERROR ,AL_DBG_ERROR,
+ ("Unsupported uvp interface version, "
+ "expected = 0x%x, actual = 0x%x.\n",
+ UVERBS_VERSION,
p_ci_ca->verbs.user_verbs.version) );
+ return IB_UNSUPPORTED;
+ }
+
/* Now open the UAL CA to be assigned to p_ci_ca */
status = ib_open_ca( h_al, ca_guid, ca_event_cb, p_ci_ca,
&p_ci_ca->h_ca );
Index: hw/mthca/user/mlnx_ual_main.c
===================================================================
--- hw/mthca/user/mlnx_ual_main.c (revision 992)
+++ hw/mthca/user/mlnx_ual_main.c (working copy)
@@ -143,7 +143,7 @@
/*
* Version of the header file this interface export can
handle
*/
- p_uvp->version = 0x101;
+ p_uvp->version = UVERBS_VERSION;
p_uvp->guid = 0x12345678;
/*
Index: inc/user/iba/ib_uvp.h
===================================================================
--- inc/user/iba/ib_uvp.h (revision 992)
+++ inc/user/iba/ib_uvp.h (working copy)
@@ -116,6 +116,17 @@
*
*/
+/*
+ * Version that identifies this version of the header file for
interface
+ * definition.
+ */
+#define UVERBS_MAJOR_VER (0x0002)
+#define UVERBS_MINOR_VER (0x0000)
+
+#define UVERBS_VERSION (((UVERBS_MAJOR_VER) << 16)
| (UVERBS_MINOR_VER))
+#define MK_UVERBS_VERSION(maj,min) ((((maj) & 0xFFFF) << 16) |
\
+ ((min) &
0xFFFF))
+
/******/
/******/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20080326/3d0149d1/attachment.html>
More information about the ofw
mailing list