[ofw] Re: [PATCH 2/6] [ib-diag] ibroute: add support for WinOF
Sasha Khapyorsky
sashak at voltaire.com
Thu Feb 26 13:02:19 PST 2009
On 12:07 Thu 26 Feb , Sean Hefty wrote:
> Both of your patches (ibnetdiscover and ibroute) build on winof. I replaced my
> 2 patches with yours, updated to the latest codebase, and pushed everything:
>
> git://git.openfabrics.org/~shefty/ib-mgmt.git master
>
> Were there changes to the other patches that you wanted (including saquery,
> which wasn't part of the numbered series)?
Thanks. I applied everything except ibsysstat.c and saquery.c. Wanted to
clarify some things there:
> From 49f28a63589be21dd7218922ed9d0b2b719a92c2 Mon Sep 17 00:00:00 2001
> From: Sean Hefty <sean.hefty at intel.com>
> Date: Thu, 26 Feb 2009 10:12:07 -0800
> Subject: [PATCH 1/2] [ib-diag] ibsysstat: add support for WinOF
>
> Signed-off-by: Sean Hefty <sean.hefty at intel.com>
> ---
> infiniband-diags/src/ibsysstat.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/infiniband-diags/src/ibsysstat.c b/infiniband-diags/src/ibsysstat.c
> index cc1418d..b9f2f85 100644
> --- a/infiniband-diags/src/ibsysstat.c
> +++ b/infiniband-diags/src/ibsysstat.c
> @@ -183,7 +183,7 @@ static char *ibsystat_serv(void)
>
> DEBUG("got packet: attr 0x%x mod 0x%x", attr, mod);
>
> - size = mk_reply(attr, mad + IB_VENDOR_RANGE2_DATA_OFFS,
> + size = mk_reply(attr, (char *) mad + IB_VENDOR_RANGE2_DATA_OFFS,
What is the reason for such void * to char * casting?
> sizeof(buf) - umad_size() - IB_VENDOR_RANGE2_DATA_OFFS);
>
> if (server_respond(umad, IB_VENDOR_RANGE2_DATA_OFFS + size) < 0)
> @@ -210,7 +210,7 @@ static char *ibsystat(ib_portid_t *portid, int attr)
> {
> ib_rpc_t rpc = { 0 };
> int fd, agent, timeout, len;
> - void *data = umad_get_mad(buf) + IB_VENDOR_RANGE2_DATA_OFFS;
> + void *data = (char *) umad_get_mad(buf) + IB_VENDOR_RANGE2_DATA_OFFS;
Ditto.
>
> DEBUG("Sysstat ping..");
>
> @@ -318,7 +318,7 @@ int main(int argc, char **argv)
> const struct ibdiag_opt opts[] = {
> { "oui", 'o', 1, NULL, "use specified OUI number" },
> { "Server", 'S', 0, NULL, "start in server mode" },
> - { }
> + { 0 }
> };
> char usage_args[] = "<dest lid|guid> [<op>]";
>
> --
> 1.6.1.2.319.gbd9e
>
>
> From 1b9685769339891670df6d9af66e9933794be8a0 Mon Sep 17 00:00:00 2001
> From: Sean Hefty <sean.hefty at intel.com>
> Date: Thu, 26 Feb 2009 10:12:29 -0800
> Subject: [PATCH 2/2] [ib-diag] saquery: add support for WinOF
>
> Signed-off-by: Sean Hefty <sean.hefty at intel.com>
> ---
> infiniband-diags/src/saquery.c | 80 ++++++++++++++++++++++------------------
> 1 files changed, 44 insertions(+), 36 deletions(-)
>
> diff --git a/infiniband-diags/src/saquery.c b/infiniband-diags/src/saquery.c
> index bcd1f61..4a5cfb8 100644
> --- a/infiniband-diags/src/saquery.c
> +++ b/infiniband-diags/src/saquery.c
> @@ -37,20 +37,25 @@
> *
> */
>
> +#if HAVE_CONFIG_H
> +# include <config.h>
> +#endif /* HAVE_CONFIG_H */
> +
> #include <unistd.h>
> #include <stdio.h>
> #include <arpa/inet.h>
> #include <ctype.h>
> #include <string.h>
> #include <errno.h>
> +#include <assert.h>
>
> #define _GNU_SOURCE
> #include <getopt.h>
>
> #include <infiniband/umad.h>
> #include <infiniband/mad.h>
> -#include <infiniband/iba/ib_types.h>
> -#include <infiniband/complib/cl_nodenamemap.h>
> +#include <iba/ib_types.h>
> +#include <complib/cl_nodenamemap.h>
>
> #include "ibdiag_common.h"
>
> @@ -170,7 +175,7 @@ recv_mad:
> if (ibdebug > 1)
> xdump(stdout, "SA Response:\n", mad, len);
>
> - method = mad_get_field(mad, 0, IB_MAD_METHOD_F);
> + method = (uint8_t) mad_get_field(mad, 0, IB_MAD_METHOD_F);
> offset = mad_get_field(mad, 0, IB_SA_ATTROFFS_F);
> result.status = mad_get_field(mad, 0, IB_MAD_STATUS_F);
> result.p_result_madw = mad;
> @@ -189,12 +194,12 @@ recv_mad:
> static void *get_query_rec(void *mad, unsigned i)
> {
> int offset = mad_get_field(mad, 0, IB_SA_ATTROFFS_F);
> - return mad + IB_SA_DATA_OFFS + i * (offset << 3);
> + return (char *) mad + IB_SA_DATA_OFFS + i * (offset << 3);
Ditto.
> }
>
> static unsigned valid_gid(ib_gid_t *gid)
> {
> - ib_gid_t zero_gid = { };
> + ib_gid_t zero_gid = { 0 };
> return memcmp(&zero_gid, gid, sizeof(*gid));
> }
>
> @@ -442,7 +447,7 @@ static void dump_multicast_member_record(void *data)
> char gid_str2[INET6_ADDRSTRLEN];
> ib_member_rec_t *p_mcmr = data;
> uint16_t mlid = cl_ntoh16(p_mcmr->mlid);
> - int i = 0;
> + unsigned i = 0;
> char *node_name = "<unknown>";
>
> /* go through the node records searching for a port guid which matches
> @@ -758,7 +763,7 @@ static void dump_one_mft_record(void *data)
>
> static void dump_results(struct query_res *r, void (*dump_func) (void *))
> {
> - int i;
> + unsigned i;
> for (i = 0; i < r->result_cnt; i++) {
> void *data = get_query_rec(r->p_result_madw, i);
> dump_func(data);
> @@ -768,7 +773,7 @@ static void dump_results(struct query_res *r, void (*dump_func) (void *))
> static void return_mad(void)
> {
> if (result.p_result_madw) {
> - free(result.p_result_madw - umad_size());
> + free((char *) result.p_result_madw - umad_size());
Ditto.
> result.p_result_madw = NULL;
> }
> }
> @@ -839,7 +844,8 @@ get_lid_from_name(bind_handle_t h, const char *name, uint16_t* lid)
> {
> ib_node_record_t *node_record = NULL;
> ib_node_info_t *p_ni = NULL;
> - int i = 0, ret;
> + unsigned i;
> + int ret;
>
> ret = get_all_records(h, IB_SA_ATTR_NODERECORD, 0);
> if (ret)
> @@ -869,7 +875,7 @@ static uint16_t get_lid(bind_handle_t h, const char *name)
> if (isalpha(name[0]))
> assert(get_lid_from_name(h, name, &rc_lid) == IB_SUCCESS);
> else
> - rc_lid = atoi(name);
> + rc_lid = (uint16_t) atoi(name);
> if (rc_lid == 0)
> fprintf(stderr, "Failed to find lid for \"%s\"\n", name);
> return rc_lid;
> @@ -917,8 +923,8 @@ static int parse_lid_and_ports(bind_handle_t h,
>
> #define cl_hton8(x) (x)
> #define CHECK_AND_SET_VAL(val, size, comp_with, target, name, mask) \
> - if (val > comp_with) { \
> - target = cl_hton##size(val); \
> + if ((uint##size##_t) val > (uint##size##_t) comp_with) { \
> + target = cl_hton##size((uint##size##_t) val); \
> comp_mask |= IB_##name##_COMPMASK_##mask; \
> }
>
> @@ -951,7 +957,8 @@ static int get_issm_records(bind_handle_t h, ib_net32_t capability_mask)
>
> static int print_node_records(bind_handle_t h)
> {
> - int i = 0, ret;
> + unsigned i;
> + int ret;
>
> ret = get_all_records(h, IB_SA_ATTR_NODERECORD, 0);
> if (ret)
> @@ -1027,7 +1034,7 @@ static int query_path_records(const struct query_cmd *q, bind_handle_t h,
> CHECK_AND_SET_VAL(p->dlid, 16, 0, pr.dlid, PR, DLID);
> CHECK_AND_SET_VAL(p->hop_limit, 32, -1, pr.hop_flow_raw, PR, HOPLIMIT);
> CHECK_AND_SET_VAL(p->flow_label, 8, 0, flow, PR, FLOWLABEL);
> - pr.hop_flow_raw |= cl_hton32(flow << 8);
> + pr.hop_flow_raw |= (uint8_t) cl_hton32(flow << 8);
Why this casting is needed? This should be uint32_t to uint32_t
assignment, no?
> CHECK_AND_SET_VAL(p->tclass, 8, 0, pr.tclass, PR, TCLASS);
> CHECK_AND_SET_VAL(p->reversible, 8, -1, reversible, PR, REVERSIBLE);
> CHECK_AND_SET_VAL(p->numb_path, 8, -1, pr.num_path, PR, NUMBPATH);
> @@ -1089,7 +1096,7 @@ static int print_multicast_member_records(bind_handle_t h)
>
> return_mc:
> if (mc_group_result.p_result_madw)
> - free(mc_group_result.p_result_madw - umad_size());
> + free((char *) mc_group_result.p_result_madw - umad_size());
void * to char * casting again.
>
> return ret;
> }
> @@ -1267,7 +1274,7 @@ static int query_pkey_tbl_records(const struct query_cmd *q,
> memset(&pktr, 0, sizeof(pktr));
> CHECK_AND_SET_VAL(lid, 16, 0, pktr.lid, PKEY, LID);
> CHECK_AND_SET_VAL(port, 8, -1, pktr.port_num, PKEY, PORT);
> - CHECK_AND_SET_VAL(block, 16, -1, pktr.port_num, PKEY, BLOCK);
> + CHECK_AND_SET_VAL(block, 16, -1, pktr.block_num, PKEY, BLOCK);
This fix is unrelated to porting, right?
The rest looks fine for me.
Sasha
>
> return get_and_dump_any_records(h, IB_SA_ATTR_PKEYTABLERECORD, 0,
> comp_mask, &pktr, smkey,
> @@ -1503,13 +1510,13 @@ static int process_opt(void *context, int ch, char *optarg)
> query_type = IB_SA_ATTR_LINKRECORD;
> break;
> case 5:
> - p->slid = strtoul(optarg, NULL, 0);
> + p->slid = (uint16_t) strtoul(optarg, NULL, 0);
> break;
> case 6:
> - p->dlid = strtoul(optarg, NULL, 0);
> + p->dlid = (uint16_t) strtoul(optarg, NULL, 0);
> break;
> case 7:
> - p->mlid = strtoul(optarg, NULL, 0);
> + p->mlid = (uint16_t) strtoul(optarg, NULL, 0);
> break;
> case 14:
> if (inet_pton(AF_INET6, optarg, &p->sgid) <= 0)
> @@ -1534,7 +1541,7 @@ static int process_opt(void *context, int ch, char *optarg)
> p->numb_path = strtoul(optarg, NULL, 0);
> break;
> case 18:
> - p->pkey = strtoul(optarg, NULL, 0);
> + p->pkey = (uint16_t) strtoul(optarg, NULL, 0);
> break;
> case 'Q':
> p->qos_class = strtoul(optarg, NULL, 0);
> @@ -1543,19 +1550,19 @@ static int process_opt(void *context, int ch, char *optarg)
> p->sl = strtoul(optarg, NULL, 0);
> break;
> case 'M':
> - p->mtu = strtoul(optarg, NULL, 0);
> + p->mtu = (uint8_t) strtoul(optarg, NULL, 0);
> break;
> case 'R':
> - p->rate = strtoul(optarg, NULL, 0);
> + p->rate = (uint8_t) strtoul(optarg, NULL, 0);
> break;
> case 20:
> - p->pkt_life = strtoul(optarg, NULL, 0);
> + p->pkt_life = (uint8_t) strtoul(optarg, NULL, 0);
> break;
> case 'q':
> p->qkey = strtoul(optarg, NULL, 0);
> break;
> case 'T':
> - p->tclass = strtoul(optarg, NULL, 0);
> + p->tclass = (uint8_t) strtoul(optarg, NULL, 0);
> break;
> case 'F':
> p->flow_label = strtoul(optarg, NULL, 0);
> @@ -1564,10 +1571,10 @@ static int process_opt(void *context, int ch, char *optarg)
> p->hop_limit = strtoul(optarg, NULL, 0);
> break;
> case 21:
> - p->scope = strtoul(optarg, NULL, 0);
> + p->scope = (uint8_t) strtoul(optarg, NULL, 0);
> break;
> case 'J':
> - p->join_state = strtoul(optarg, NULL, 0);
> + p->join_state = (uint8_t) strtoul(optarg, NULL, 0);
> break;
> case 'X':
> p->proxy_join = strtoul(optarg, NULL, 0);
> @@ -1582,14 +1589,7 @@ int main(int argc, char **argv)
> {
> char usage_args[1024];
> bind_handle_t h;
> - struct query_params params = {
> - .hop_limit = -1,
> - .reversible = -1,
> - .numb_path = -1,
> - .qos_class = -1,
> - .sl = -1,
> - .proxy_join = -1,
> - };
> + struct query_params params;
> const struct query_cmd *q;
> ib_api_status_t status;
> int n;
> @@ -1643,9 +1643,17 @@ int main(int argc, char **argv)
> { "scope", 21, 1, NULL, "Scope (MCMemberRecord)" },
> { "join_state", 'J', 1, NULL, "Join state (MCMemberRecord)" },
> { "proxy_join", 'X', 1, NULL, "Proxy join (MCMemberRecord)" },
> - {}
> + { 0 }
> };
>
> + memset(¶ms, 0, sizeof params);
> + params.hop_limit = -1;
> + params.reversible = -1;
> + params.numb_path = -1;
> + params.qos_class = -1;
> + params.sl = -1;
> + params.proxy_join = -1;
> +
> n = sprintf(usage_args, "[query-name] [<name> | <lid> | <guid>]\n"
> "\nSupported query names (and aliases):\n");
> for (q = query_cmds; q->name; q++) {
> @@ -1680,7 +1688,7 @@ int main(int argc, char **argv)
>
> if (argc) {
> if (node_print_desc == NAME_OF_LID) {
> - requested_lid = strtoul(argv[0], NULL, 0);
> + requested_lid = (uint16_t) strtoul(argv[0], NULL, 0);
> requested_lid_flag++;
> } else if (node_print_desc == NAME_OF_GUID) {
> requested_guid = strtoul(argv[0], NULL, 0);
> --
> 1.6.1.2.319.gbd9e
>
More information about the ofw
mailing list