[PATCH] libibverbs: Allow arbitrary int values for MTU
Keep IBV_MTU_* enums values as they are, but pass MTU values around as
a struct containing a single int. Per lengthy discusson on the
linux-rdma list, this patch introdces a source code incompatibility.
Although legacy applications can continue to use the enum values, they
will need to be updated to use the struct. Newer applications are
encouraged to use arbitrary int values, not the MTU enums (e.g., 1024,
1500, 9000).
(if people like the idea of this patch, I will send the corresponding
kernel patch)
Signed-off-by: Jeff Squyres
---
Makefile.am| 3 ++-
examples/devinfo.c | 20 +++
examples/pingpong.c| 12 -
examples/pingpong.h| 1 -
examples/rc_pingpong.c | 10
examples/srq_pingpong.c| 10
examples/uc_pingpong.c | 10
examples/ud_pingpong.c | 2 +-
include/infiniband/verbs.h | 61 +++---
man/ibv_modify_qp.3| 2 +-
man/ibv_query_port.3 | 4 +--
man/ibv_query_qp.3 | 2 +-
src/cmd.c | 8 +++---
src/marshall.c | 2 +-
14 files changed, 89 insertions(+), 58 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 40e83be..1159e55 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -54,7 +54,8 @@ man_MANS = man/ibv_asyncwatch.1 man/ibv_devices.1
man/ibv_devinfo.1 \
man/ibv_post_srq_recv.3 man/ibv_query_device.3 man/ibv_query_gid.3 \
man/ibv_query_pkey.3 man/ibv_query_port.3 man/ibv_query_qp.3 \
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_req_notify_cq.3 man/ibv_resize_cq.3 man/ibv_rate_to_mbps.3 \
+man/ibv_mtu_to_num.3
DEBIAN = debian/changelog debian/compat debian/control debian/copyright \
debian/ibverbs-utils.install debian/libibverbs1.install \
diff --git a/examples/devinfo.c b/examples/devinfo.c
index ff078e4..e8fb27e 100644
--- a/examples/devinfo.c
+++ b/examples/devinfo.c
@@ -111,18 +111,6 @@ static const char *atomic_cap_str(enum ibv_atomic_cap
atom_cap)
}
}
-static const char *mtu_str(enum ibv_mtu max_mtu)
-{
- switch (max_mtu) {
- case IBV_MTU_256: return "256";
- case IBV_MTU_512: return "512";
- case IBV_MTU_1024: return "1024";
- case IBV_MTU_2048: return "2048";
- case IBV_MTU_4096: return "4096";
- default: return "invalid MTU";
- }
-}
-
static const char *width_str(uint8_t width)
{
switch (width) {
@@ -301,10 +289,10 @@ static int print_hca_cap(struct ibv_device *ib_dev,
uint8_t ib_port)
printf("\t\tport:\t%d\n", port);
printf("\t\t\tstate:\t\t\t%s (%d)\n",
port_state_str(port_attr.state), port_attr.state);
- printf("\t\t\tmax_mtu:\t\t%s (%d)\n",
- mtu_str(port_attr.max_mtu), port_attr.max_mtu);
- printf("\t\t\tactive_mtu:\t\t%s (%d)\n",
- mtu_str(port_attr.active_mtu), port_attr.active_mtu);
+ printf("\t\t\tmax_mtu:\t\t%d (%d)\n",
+ ibv_mtu_to_num(port_attr.max_mtu),
port_attr.max_mtu.mtu);
+ printf("\t\t\tactive_mtu:\t\t%d (%d)\n",
+ ibv_mtu_to_num(port_attr.active_mtu),
port_attr.active_mtu.mtu);
printf("\t\t\tsm_lid:\t\t\t%d\n", port_attr.sm_lid);
printf("\t\t\tport_lid:\t\t%d\n", port_attr.lid);
printf("\t\t\tport_lmc:\t\t0x%02x\n", port_attr.lmc);
diff --git a/examples/pingpong.c b/examples/pingpong.c
index 90732ef..d1c22c9 100644
--- a/examples/pingpong.c
+++ b/examples/pingpong.c
@@ -36,18 +36,6 @@
#include
#include
-enum ibv_mtu pp_mtu_to_enum(int mtu)
-{
- switch (mtu) {
- case 256: return IBV_MTU_256;
- case 512: return IBV_MTU_512;
- case 1024: return IBV_MTU_1024;
- case 2048: return IBV_MTU_2048;
- case 4096: return IBV_MTU_4096;
- default: return -1;
- }
-}
-
uint16_t pp_get_local_lid(struct ibv_context *context, int port)
{
struct ibv_port_attr attr;
diff --git a/examples/pingpong.h b/examples/pingpong.h
index 9cdc03e..91d217b 100644
--- a/examples/pingpong.h
+++ b/examples/pingpong.h
@@ -35,7 +35,6 @@
#include
-enum ibv_mtu pp_mtu_to_enum(int mtu);
uint16_t pp_get_local_lid(struct ibv_context *context, int port);
int pp_get_port_info(struct ibv_context *context, int port,
struct ibv_port_attr *attr);
diff --git a/examples/rc_pingpong.c b/examples/rc_pingpong.c
index 15494a1..a7e1836 100644
--- a/examples/rc_pingpong.c
+++ b/examples/rc_pingpong.c
@@ -78,7 +78,7 @@ struct pingpong_dest {
};
static int pp_connect_ctx(struct pingpong_context *ctx, int port, int my_psn,
- enum ibv_mtu mtu, int sl,
+ struct ibv_mtu_t mtu, int sl,
struct
Re: [PATCH] libibverbs: Allow arbitrary int values for MTU
On Jun 20, 2013, at 4:40 PM, Doug Ledford wrote:
>> {
>> static char str[16];
>> snprintf(str, sizeof(str), "%d", ibv_mtu_to_num(max_mtu));
>>return str;
>> }
>
> That is not, however, multi-thread safe nor advisable unless you clearly
> indicate in the man page to the function that subsequent calls to the
> function wipe out the result of previous calls. It's not even single
> thread safe if you have more than one interface and don't know that
> later calls wipe this buffer out. Best to avoid library routines such
> as this.
This is in the devinfo.c program (which is single-threaded), not in the library
itself.
But regardless, this whole function went away in V2 of the patch.
--
Jeff Squyres
jsquy...@cisco.com
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libibverbs: Allow arbitrary int values for MTU
On 06/18/2013 02:49 PM, Jason Gunthorpe wrote:
> This is simpler:
>
> {
> static char str[16];
> snprintf(str, sizeof(str), "%d", ibv_mtu_to_num(max_mtu));
> return str;
> }
That is not, however, multi-thread safe nor advisable unless you clearly
indicate in the man page to the function that subsequent calls to the
function wipe out the result of previous calls. It's not even single
thread safe if you have more than one interface and don't know that
later calls wipe this buffer out. Best to avoid library routines such
as this.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libibverbs: Allow arbitrary int values for MTU
On Jun 20, 2013, at 1:09 PM, "Hefty, Sean" wrote: >> int ibv_rate_to_mult(enum ibv_rate rate); >> enum ibv_rate mult_to_ibv_rate(int mult); >> >> int ibv_rate_to_mbps(enum ibv_rate rate); >> enum ibv_rate mbps_to_ibv_rate(int mbps); > > libibverbs uses the "ibv_" prefix for pretty much everything. ...except for those 2 functions above (mbps_to_ibv_rate and mult_to_ibv_rate). See: https://git.kernel.org/cgit/libs/infiniband/libibverbs.git/tree/include/infiniband/verbs.h#n392 and https://git.kernel.org/cgit/libs/infiniband/libibverbs.git/tree/include/infiniband/verbs.h#n379 respectively. -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/ -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] libibverbs: Allow arbitrary int values for MTU
> I used the name "num_to_ibv_mtu" because it is in the spirit of the other > enum- > to-int/int-to-enum function pair naming conventions: > > int ibv_rate_to_mult(enum ibv_rate rate); > enum ibv_rate mult_to_ibv_rate(int mult); > > int ibv_rate_to_mbps(enum ibv_rate rate); > enum ibv_rate mbps_to_ibv_rate(int mbps); libibverbs uses the "ibv_" prefix for pretty much everything. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libibverbs: Allow arbitrary int values for MTU
On Jun 18, 2013, at 2:49 PM, Jason Gunthorpe wrote: >> +int num_to_ibv_mtu(int num); > > Probably should be ibv_num_to_mtu() to keep with the naming pattern.. New patch coming momentarily, but I wanted to comment on this one: I used the name "num_to_ibv_mtu" because it is in the spirit of the other enum-to-int/int-to-enum function pair naming conventions: int ibv_rate_to_mult(enum ibv_rate rate); enum ibv_rate mult_to_ibv_rate(int mult); int ibv_rate_to_mbps(enum ibv_rate rate); enum ibv_rate mbps_to_ibv_rate(int mbps); -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/ -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libibverbs: Allow arbitrary int values for MTU
On Mon, Jun 17, 2013 at 02:00:07PM -0700, Jeff Squyres wrote:
> Keep IBV_MTU_* enums values as they are, but pass MTU values around as
> int's. This is an ABI-compatible change; legacy applications will use
> the enum values, but newer applications can use an int for values that
> do not currently exist in the enum set (e.g., 1500, 9000).
>
> (if people like the idea of this patch, I will send the corresponding
> kernel patch)
>
> Signed-off-by: Jeff Squyres
> examples/devinfo.c | 11 +--
> examples/pingpong.c| 12
> examples/pingpong.h| 1 -
> examples/rc_pingpong.c | 8
> examples/srq_pingpong.c| 8
> examples/uc_pingpong.c | 8
> examples/ud_pingpong.c | 2 +-
> include/infiniband/verbs.h | 20 +---
> man/ibv_modify_qp.3| 2 +-
> man/ibv_query_port.3 | 4 ++--
> man/ibv_query_qp.3 | 2 +-
> src/libibverbs.map | 3 +++
> src/verbs.c| 24
> 13 files changed, 70 insertions(+), 35 deletions(-)
>
> diff --git a/examples/devinfo.c b/examples/devinfo.c
> index ff078e4..b856c82 100644
> +++ b/examples/devinfo.c
> @@ -111,15 +111,22 @@ static const char *atomic_cap_str(enum ibv_atomic_cap
> atom_cap)
> }
> }
>
> -static const char *mtu_str(enum ibv_mtu max_mtu)
> +static const char *mtu_str(int max_mtu)
This is simpler:
{
static char str[16];
snprintf(str, sizeof(str), "%d", ibv_mtu_to_num(max_mtu));
return str;
}
You may want to replace 'enum ibv_mtu' with 'ibv_mtu_t' to make it
more clear that it is not an integer.
> - enum ibv_mtu mtu = IBV_MTU_1024;
> + int mtu = 1024;
No, you must keep using IBV_MTU_1024 for all cases requesting a 1024
byte MTU, new libraries will accept both, old libraries will only
accept IBV_MTU_1024, so we must stick with IBV_MTU_1024 in
applications.
So:
int mtu = num_to_ibv_mtu(1024);
> - mtu = pp_mtu_to_enum(strtol(optarg, NULL, 0));
> + mtu = strtol(optarg, NULL, 0);
> if (mtu < 0) {
> usage(argv[0]);
> return 1;
Same deal, add 'mtu = num_to_ibv_mtu(mtu);'
Same two comments repeated many times.
> +/**
> + * ibv_num_to_mtu - If an MTU enum value for "num" is available,
> + * return it (e.g., 1024 -> IBV_MTU_1024). Otherwise, return num
> + * (e.g., 1500 -> 1500).
> + */
> +int num_to_ibv_mtu(int num);
Probably should be ibv_num_to_mtu() to keep with the naming pattern..
> +
> +/**
> + * ibv_mtu_to_num - Return the numeric value of a symbolic enum
> + * ibv_mtu name (e.g., IBV_MTU_1024 -> 1024). If a non-symbolic enum
> + * value is passed, just return the same value (e.g., 1500 -> 1500).
> + */
> +int ibv_mtu_to_num(int mtu);
These should be static inlines, not externs. There is a desire to not
add new symbols to libibverbs.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libibverbs: Allow arbitrary int values for MTU
Keep IBV_MTU_* enums values as they are, but pass MTU values around as
int's. This is an ABI-compatible change; legacy applications will use
the enum values, but newer applications can use an int for values that
do not currently exist in the enum set (e.g., 1500, 9000).
(if people like the idea of this patch, I will send the corresponding
kernel patch)
Signed-off-by: Jeff Squyres
---
examples/devinfo.c | 11 +--
examples/pingpong.c| 12
examples/pingpong.h| 1 -
examples/rc_pingpong.c | 8
examples/srq_pingpong.c| 8
examples/uc_pingpong.c | 8
examples/ud_pingpong.c | 2 +-
include/infiniband/verbs.h | 20 +---
man/ibv_modify_qp.3| 2 +-
man/ibv_query_port.3 | 4 ++--
man/ibv_query_qp.3 | 2 +-
src/libibverbs.map | 3 +++
src/verbs.c| 24
13 files changed, 70 insertions(+), 35 deletions(-)
diff --git a/examples/devinfo.c b/examples/devinfo.c
index ff078e4..b856c82 100644
--- a/examples/devinfo.c
+++ b/examples/devinfo.c
@@ -111,15 +111,22 @@ static const char *atomic_cap_str(enum ibv_atomic_cap
atom_cap)
}
}
-static const char *mtu_str(enum ibv_mtu max_mtu)
+static const char *mtu_str(int max_mtu)
{
+ static char str[16];
+
switch (max_mtu) {
case IBV_MTU_256: return "256";
case IBV_MTU_512: return "512";
case IBV_MTU_1024: return "1024";
case IBV_MTU_2048: return "2048";
case IBV_MTU_4096: return "4096";
- default: return "invalid MTU";
+ default:
+ if (max_mtu > 0) {
+ snprintf(str, sizeof(str), "%d", max_mtu);
+ return str;
+ }
+ return "invalid MTU";
}
}
diff --git a/examples/pingpong.c b/examples/pingpong.c
index 90732ef..d1c22c9 100644
--- a/examples/pingpong.c
+++ b/examples/pingpong.c
@@ -36,18 +36,6 @@
#include
#include
-enum ibv_mtu pp_mtu_to_enum(int mtu)
-{
- switch (mtu) {
- case 256: return IBV_MTU_256;
- case 512: return IBV_MTU_512;
- case 1024: return IBV_MTU_1024;
- case 2048: return IBV_MTU_2048;
- case 4096: return IBV_MTU_4096;
- default: return -1;
- }
-}
-
uint16_t pp_get_local_lid(struct ibv_context *context, int port)
{
struct ibv_port_attr attr;
diff --git a/examples/pingpong.h b/examples/pingpong.h
index 9cdc03e..91d217b 100644
--- a/examples/pingpong.h
+++ b/examples/pingpong.h
@@ -35,7 +35,6 @@
#include
-enum ibv_mtu pp_mtu_to_enum(int mtu);
uint16_t pp_get_local_lid(struct ibv_context *context, int port);
int pp_get_port_info(struct ibv_context *context, int port,
struct ibv_port_attr *attr);
diff --git a/examples/rc_pingpong.c b/examples/rc_pingpong.c
index 15494a1..8a5318b 100644
--- a/examples/rc_pingpong.c
+++ b/examples/rc_pingpong.c
@@ -78,7 +78,7 @@ struct pingpong_dest {
};
static int pp_connect_ctx(struct pingpong_context *ctx, int port, int my_psn,
- enum ibv_mtu mtu, int sl,
+ int mtu, int sl,
struct pingpong_dest *dest, int sgid_idx)
{
struct ibv_qp_attr attr = {
@@ -209,7 +209,7 @@ out:
}
static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
-int ib_port, enum ibv_mtu mtu,
+int ib_port, int mtu,
int port, int sl,
const struct pingpong_dest
*my_dest,
int sgid_idx)
@@ -547,7 +547,7 @@ int main(int argc, char *argv[])
int port = 18515;
int ib_port = 1;
int size = 4096;
- enum ibv_mtu mtu = IBV_MTU_1024;
+ int mtu = 1024;
int rx_depth = 500;
int iters = 1000;
int use_event = 0;
@@ -608,7 +608,7 @@ int main(int argc, char *argv[])
break;
case 'm':
- mtu = pp_mtu_to_enum(strtol(optarg, NULL, 0));
+ mtu = strtol(optarg, NULL, 0);
if (mtu < 0) {
usage(argv[0]);
return 1;
diff --git a/examples/srq_pingpong.c b/examples/srq_pingpong.c
index 6e00f8c..f1eb879 100644
--- a/examples/srq_pingpong.c
+++ b/examples/srq_pingpong.c
@@ -81,7 +81,7 @@ struct pingpong_dest {
union ibv_gid gid;
};
-static int pp_connect_ctx(struct pingpong_context *ctx, int port, enum ibv_mtu
mtu,
+static int pp_connect_ctx(struct pingpong_context *ctx, int port, int mtu,
