Aaah. Ira pointed me at a previous thread that provided some context. I thought the concern was with the display of mkey results in a query, not on the command line. I hadn't noticed the getpass() support for smkey. It makes sense that mkey should operate the same way.
Jim On Tue, 2012-03-13 at 05:31 -0700, Hal Rosenstock wrote: > On 3/9/2012 2:17 PM, Jim Foraker wrote: > > > > On Fri, 2012-03-09 at 05:00 -0800, Hal Rosenstock wrote: > >> On 3/6/2012 5:16 PM, Jim Foraker wrote: > >>> > >>> Displaying/changing are both supported for the > >>> MKey itself, plus lease and protect bits > >> > >> Don't we want to be careful about displaying mkey info ? I think by > >> default it shouldn't be displayed and at least require an additional > >> option to make it visible. > > Is your concern that mkey info may be fetched by people who should > > not be seeing it, or that "prying eyes" may glance the mkey over the > > shoulder of a fabric admin while they work? > > I think the first issue is covered sufficiently well via the > > protect bits. I can understand your point along the second line, > > although it didn't raise to the level where it seemed worth it to add > > another command line option to support it. If the feeling is that it > > is, I can add that to the patch. > > Yes, my concern is the latter. Note that a similar thing was done with > saquery in terms of the SA (nee SM) key. > > -- Hal > > > > > Jim > > > >>> Signed-off-by: Jim Foraker <forak...@llnl.gov> > >>> --- > >>> src/ibportstate.c | 64 > >>> +++++++++++++++++++++++++++++++++++++++++++++++----- > >>> 1 files changed, 57 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/src/ibportstate.c b/src/ibportstate.c > >>> index ec6b823..b5a1a98 100644 > >>> --- a/src/ibportstate.c > >>> +++ b/src/ibportstate.c > >>> @@ -64,6 +64,9 @@ enum port_ops { > >>> LID, > >>> SMLID, > >>> LMC, > >>> + MKEY, > >>> + MKEYLEASE, > >>> + MKEYPROT, > >>> }; > >>> > >>> struct ibmad_port *srcport; > >>> @@ -76,6 +79,10 @@ int smlid; > >>> int lmc; > >>> int mtu; > >>> int vls = 0; /* no state change */ > >>> +int mkeyfake; /* Just a placeholder */ > >>> +uint64_t mkey; > >>> +int mkeylease; > >>> +int mkeyprot; > >>> > >>> struct { > >>> const char *name; > >>> @@ -98,6 +105,9 @@ struct { > >>> {"lid", &lid, 0}, /* LID */ > >>> {"smlid", &smlid, 0}, /* SMLID */ > >>> {"lmc", &lmc, 0}, /* LMC */ > >>> + {"mkey", &mkeyfake, 0}, /* MKEY */ > >>> + {"mkeylease", &mkeylease, 0}, /* MKEY LEASE */ > >>> + {"mkeyprot", &mkeyprot, 0}, /* MKEY PROTECT BITS */ > >>> }; > >>> > >>> #define NPORT_ARGS (sizeof(port_args) / sizeof(port_args[0])) > >>> @@ -142,7 +152,7 @@ static int get_port_info(ib_portid_t * dest, uint8_t > >>> * data, int portnum, > >>> } > >>> > >>> static void show_port_info(ib_portid_t * dest, uint8_t * data, int > >>> portnum, > >>> - int espeed_cap) > >>> + int espeed_cap, int is_switch) > >>> { > >>> char buf[2300]; > >>> char val[64]; > >>> @@ -201,18 +211,32 @@ static void show_port_info(ib_portid_t * dest, > >>> uint8_t * data, int portnum, > >>> val); > >>> sprintf(buf + strlen(buf), "%s", "\n"); > >>> } > >>> + if (!is_switch || portnum == 0) { > >>> + mad_decode_field(data, IB_PORT_MKEY_F, val); > >>> + mad_dump_field(IB_PORT_MKEY_F, buf + strlen(buf), > >>> + sizeof buf - strlen(buf), val); > >>> + sprintf(buf+strlen(buf), "%s", "\n"); > >>> + mad_decode_field(data, IB_PORT_MKEY_LEASE_F, val); > >>> + mad_dump_field(IB_PORT_MKEY_LEASE_F, buf + strlen(buf), > >>> + sizeof buf - strlen(buf), val); > >>> + sprintf(buf+strlen(buf), "%s", "\n"); > >>> + mad_decode_field(data, IB_PORT_MKEY_PROT_BITS_F, val); > >>> + mad_dump_field(IB_PORT_MKEY_PROT_BITS_F, buf + strlen(buf), > >>> + sizeof buf - strlen(buf), val); > >>> + sprintf(buf+strlen(buf), "%s", "\n"); > >>> + } > >>> > >>> printf("# Port info: %s port %d\n%s", portid2str(dest), portnum, buf); > >>> } > >>> > >>> static void set_port_info(ib_portid_t * dest, uint8_t * data, int > >>> portnum, > >>> - int espeed_cap) > >>> + int espeed_cap, int is_switch) > >>> { > >>> if (!smp_set_via(data, dest, IB_ATTR_PORT_INFO, portnum, 0, srcport)) > >>> IBERROR("smp set portinfo failed"); > >>> > >>> printf("\nAfter PortInfo set:\n"); > >>> - show_port_info(dest, data, portnum, espeed_cap); > >>> + show_port_info(dest, data, portnum, espeed_cap, is_switch); > >>> } > >>> > >>> static void get_ext_port_info(ib_portid_t * dest, uint8_t * data, int > >>> portnum) > >>> @@ -351,7 +375,7 @@ int main(int argc, char **argv) > >>> long val; > >>> char usage_args[] = "<dest dr_path|lid|guid> <portnum> [<op>]\n" > >>> "\nSupported ops: enable, disable, reset, speed, width, query,\n" > >>> - "\tdown, arm, active, vls, mtu, lid, smlid, lmc\n"; > >>> + "\tdown, arm, active, vls, mtu, lid, smlid, lmc, mkey, mkeylease, > >>> mkeyprot\n"; > >>> const char *usage_examples[] = { > >>> "3 1 disable\t\t\t# by lid", > >>> "-G 0x2C9000100D051 1 enable\t# by guid", > >>> @@ -441,6 +465,18 @@ int main(int argc, char **argv) > >>> case LMC: > >>> if (val < 0 || val > 7) > >>> IBERROR("invalid lmc value %ld", val); > >>> + break; > >>> + case MKEY: > >>> + /* port_args is using ints, but we need uint64 > >>> */ > >>> + mkey = strtoll(argv[i], 0, 0); > >>> + break; > >>> + case MKEYLEASE: > >>> + if (val < 0 || val > 0xFFFF) > >>> + IBERROR("invalid mkey lease time %ld", > >>> val); > >>> + break; > >>> + case MKEYPROT: > >>> + if (val < 0 || val > 3) > >>> + IBERROR("invalid mkey protection bit > >>> setting %ld", val); > >> > >> Nit: bit -> bits > >> > >> -- Hal > >> > >>> } > >>> *port_args[j].val = (int)val; > >>> changed = 1; > >>> @@ -455,12 +491,16 @@ int main(int argc, char **argv) > >>> is_switch = get_node_info(&portid, data); > >>> devid = (uint16_t) mad_get_field(data, 0, IB_NODE_DEVID_F); > >>> > >>> + if ((port_args[MKEY].set || port_args[MKEYLEASE].set || > >>> + port_args[MKEYPROT].set) && is_switch && portnum != 0) > >>> + IBERROR("Can't set M_Key fields on switch port != 0"); > >>> + > >>> if (port_op != QUERY || changed) > >>> printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA"); > >>> else > >>> printf("%s PortInfo:\n", is_switch ? "Switch" : "CA"); > >>> espeed_cap = get_port_info(&portid, data, portnum, is_switch); > >>> - show_port_info(&portid, data, portnum, espeed_cap); > >>> + show_port_info(&portid, data, portnum, espeed_cap, is_switch); > >>> if (is_mlnx_ext_port_info_supported(devid)) { > >>> get_ext_port_info(&portid, data2, portnum); > >>> show_ext_port_info(&portid, data2, portnum); > >>> @@ -527,7 +567,17 @@ int main(int argc, char **argv) > >>> fdr10); > >>> set_ext_port_info(&portid, data2, portnum); > >>> } > >>> - set_port_info(&portid, data, portnum, is_switch); > >>> + > >>> + if (port_args[MKEY].set) > >>> + mad_set_field64(data, 0, IB_PORT_MKEY_F, mkey); > >>> + if (port_args[MKEYLEASE].set) > >>> + mad_set_field(data, 0, IB_PORT_MKEY_LEASE_F, > >>> + mkeylease); > >>> + if (port_args[MKEYPROT].set) > >>> + mad_set_field(data, 0, IB_PORT_MKEY_PROT_BITS_F, > >>> + mkeyprot); > >>> + > >>> + set_port_info(&portid, data, portnum, espeed_cap, is_switch); > >>> > >>> } else if (is_switch && portnum) { > >>> /* Now, make sure PortState is Active */ > >>> @@ -596,7 +646,7 @@ int main(int argc, char **argv) > >>> get_ext_port_info(&peerportid, data2, > >>> peerlocalportnum); > >>> show_port_info(&peerportid, data, peerlocalportnum, > >>> - peer_espeed_cap); > >>> + peer_espeed_cap, is_peer_switch); > >>> if (is_mlnx_ext_port_info_supported(rem_devid)) > >>> show_ext_port_info(&peerportid, data2, > >>> peerlocalportnum); > >> > > > > > -- 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