* [PATCH infiniband-diags] vendstat: mad_rpc_close_port not called in corner cases
@ 2015-04-21 14:01 Hal Rosenstock
[not found] ` <5536584C.8070505-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
0 siblings, 1 reply; 2+ messages in thread
From: Hal Rosenstock @ 2015-04-21 14:01 UTC (permalink / raw
To: Ira Weiny
Cc: Vladimir Koushnir,
linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
From: Vladimir Koushnir <vladimirk-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Date: Thu, 16 Apr 2015 18:27:39 +0300
Call mad_rpc_close_port in all corner cases.
This is needed so fds are not leaked with ibsim.
Signed-off-by: Vladimir Koushnir <vladimirk-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
src/vendstat.c | 82 ++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 53 insertions(+), 29 deletions(-)
diff --git a/src/vendstat.c b/src/vendstat.c
index 11ee73e..baade3e 100644
--- a/src/vendstat.c
+++ b/src/vendstat.c
@@ -172,13 +172,14 @@ static int do_vendor(ib_portid_t *portid, struct ibmad_port *srcport,
call.attrid = attr_id;
call.mod = attr_mod;
- if (!ib_vendor_call_via(data, portid, &call, srcport))
- IBEXIT("vendstat: method %u, attribute %u", method, attr_id);
-
+ if (!ib_vendor_call_via(data, portid, &call, srcport)) {
+ fprintf(stderr,"vendstat: method %u, attribute %u failure\n", method, attr_id);
+ return -1;
+ }
return 0;
}
-static void do_config_space_records(ib_portid_t *portid, unsigned set,
+static int do_config_space_records(ib_portid_t *portid, unsigned set,
is3_config_space_t *cs, unsigned records)
{
unsigned i;
@@ -194,17 +195,19 @@ static void do_config_space_records(ib_portid_t *portid, unsigned set,
if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS,
set ? IB_MAD_METHOD_SET : IB_MAD_METHOD_GET,
IB_MLX_IS3_CONFIG_SPACE_ACCESS, 2 << 22 | records << 16,
- cs))
- IBEXIT("cannot %s config space records", set ? "set" : "get");
-
+ cs)) {
+ fprintf(stderr,"cannot %s config space records\n", set ? "set" : "get");
+ return -1;
+ }
for (i = 0; i < records; i++) {
printf("Config space record at 0x%x: 0x%x\n",
ntohl(cs->record[i].address),
ntohl(cs->record[i].data & cs->record[i].mask));
}
+ return 0;
}
-static void counter_groups_info(ib_portid_t * portid, int port)
+static int counter_groups_info(ib_portid_t * portid, int port)
{
char buf[1024];
is4_counter_group_info_t *cg_info;
@@ -213,15 +216,17 @@ static void counter_groups_info(ib_portid_t * portid, int port)
/* Counter Group Info */
memset(&buf, 0, sizeof(buf));
if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
- IB_MLX_IS4_COUNTER_GROUP_INFO, port, buf))
- IBEXIT("counter group info query");
-
+ IB_MLX_IS4_COUNTER_GROUP_INFO, port, buf)) {
+ fprintf(stderr,"counter group info query failure\n");
+ return -1;
+ }
cg_info = (is4_counter_group_info_t *) & buf;
num_cg = cg_info->num_of_counter_groups;
printf("counter_group_info:\n");
printf("%d counter groups\n", num_cg);
for (i = 0; i < num_cg; i++)
printf("group%d mask %#x\n", i, ntohl(cg_info->group_masks[i]));
+ return 0;
}
/* Group0 counter config values */
@@ -236,7 +241,7 @@ static void counter_groups_info(ib_portid_t * portid, int port)
static int cg0, cg1;
-static void config_counter_groups(ib_portid_t * portid, int port)
+static int config_counter_groups(ib_portid_t * portid, int port)
{
char buf[1024];
is4_config_counter_groups_t *cg_config;
@@ -251,15 +256,19 @@ static void config_counter_groups(ib_portid_t * portid, int port)
cg_config->group_selects[1].group_select = (uint8_t) cg1;
if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_SET,
- IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf))
- IBEXIT("config counter group set");
-
+ IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf)) {
+ fprintf(stderr, "config counter group set failure\n");
+ return -1;
+ }
/* get config counter groups */
memset(&buf, 0, sizeof(buf));
if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
- IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf))
- IBEXIT("config counter group query");
+ IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf)) {
+ fprintf(stderr, "config counter group query failure\n");
+ return -1;
+ }
+ return 0;
}
static int general_info, xmit_wait, counter_group_info, config_counter_group;
@@ -364,20 +373,26 @@ int main(int argc, char **argv)
if (argc) {
if (resolve_portid_str(ibd_ca, ibd_ca_port, &portid, argv[0],
- ibd_dest_type, ibd_sm_id, srcport) < 0)
+ ibd_dest_type, ibd_sm_id, srcport) < 0) {
+ mad_rpc_close_port(srcport);
IBEXIT("can't resolve destination port %s", argv[0]);
+ }
} else {
- if (resolve_self(ibd_ca, ibd_ca_port, &portid, &port, 0) < 0)
+ if (resolve_self(ibd_ca, ibd_ca_port, &portid, &port, 0) < 0) {
+ mad_rpc_close_port(srcport);
IBEXIT("can't resolve self port %s", argv[0]);
+ }
}
if (counter_group_info) {
counter_groups_info(&portid, port);
+ mad_rpc_close_port(srcport);
exit(0);
}
if (config_counter_group) {
config_counter_groups(&portid, port);
+ mad_rpc_close_port(srcport);
exit(0);
}
@@ -388,6 +403,7 @@ int main(int argc, char **argv)
if (write_cs_records)
do_config_space_records(&portid, 1, &write_cs,
write_cs_records);
+ mad_rpc_close_port(srcport);
exit(0);
}
@@ -395,23 +411,27 @@ int main(int argc, char **argv)
/* but vendors change the VendorId so how know for sure ? */
/* Only General Info and Port Xmit Wait Counters */
/* queries are currently supported */
- if (!general_info && !xmit_wait)
+ if (!general_info && !xmit_wait) {
+ mad_rpc_close_port(srcport);
IBEXIT("at least one of -N and -w must be specified");
-
+ }
/* Would need a list of these and it might not be complete */
/* so for right now, punt on this */
/* vendor ClassPortInfo is required attribute if class supported */
memset(&buf, 0, sizeof(buf));
if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
- CLASS_PORT_INFO, 0, buf))
+ CLASS_PORT_INFO, 0, buf)) {
+ mad_rpc_close_port(srcport);
IBEXIT("classportinfo query");
-
+ }
memset(&buf, 0, sizeof(buf));
gi_is3 = (is3_general_info_t *) &buf;
if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
- IB_MLX_IS3_GENERAL_INFO, 0, gi_is3))
+ IB_MLX_IS3_GENERAL_INFO, 0, gi_is3)) {
+ mad_rpc_close_port(srcport);
IBEXIT("generalinfo query");
+ }
if (is_ext_fw_info_supported(ntohs(gi_is3->hw_info.device_id))) {
gi_is4 = (is4_general_info_t *) &buf;
@@ -452,10 +472,11 @@ int main(int argc, char **argv)
is3_config_space_t *cs;
unsigned i;
- if (ntohs(gi_is3->hw_info.device_id) != IS3_DEVICE_ID)
+ if (ntohs(gi_is3->hw_info.device_id) != IS3_DEVICE_ID) {
+ mad_rpc_close_port(srcport);
IBEXIT("Unsupported device ID 0x%x",
ntohs(gi_is3->hw_info.device_id));
-
+ }
memset(&buf, 0, sizeof(buf));
/* Set record addresses for each port */
cs = (is3_config_space_t *) & buf;
@@ -464,9 +485,10 @@ int main(int argc, char **argv)
htonl(IB_MLX_IS3_PORT_XMIT_WAIT + ((i + 1) << 12));
if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS,
IB_MAD_METHOD_GET, IB_MLX_IS3_CONFIG_SPACE_ACCESS,
- 2 << 22 | 16 << 16, cs))
+ 2 << 22 | 16 << 16, cs)) {
+ mad_rpc_close_port(srcport);
IBEXIT("vendstat");
-
+ }
for (i = 0; i < 16; i++)
if (cs->record[i].data) /* PortXmitWait is 32 bit counter */
printf("Port %d: PortXmitWait 0x%x\n", i + 4, ntohl(cs->record[i].data)); /* port 4 is first port */
@@ -480,8 +502,10 @@ int main(int argc, char **argv)
htonl(IB_MLX_IS3_PORT_XMIT_WAIT + ((i + 17) << 12));
if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS,
IB_MAD_METHOD_GET, IB_MLX_IS3_CONFIG_SPACE_ACCESS,
- 2 << 22 | 8 << 16, cs))
+ 2 << 22 | 8 << 16, cs)) {
+ mad_rpc_close_port(srcport);
IBEXIT("vendstat");
+ }
for (i = 0; i < 8; i++)
if (cs->record[i].data) /* PortXmitWait is 32 bit counter */
--
1.7.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH infiniband-diags] vendstat: mad_rpc_close_port not called in corner cases
[not found] ` <5536584C.8070505-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
@ 2015-04-22 15:26 ` ira.weiny
0 siblings, 0 replies; 2+ messages in thread
From: ira.weiny @ 2015-04-22 15:26 UTC (permalink / raw
To: Hal Rosenstock
Cc: Vladimir Koushnir,
linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
On Tue, Apr 21, 2015 at 10:01:48AM -0400, Hal Rosenstock wrote:
> From: Vladimir Koushnir <vladimirk-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Date: Thu, 16 Apr 2015 18:27:39 +0300
>
> Call mad_rpc_close_port in all corner cases.
> This is needed so fds are not leaked with ibsim.
>
> Signed-off-by: Vladimir Koushnir <vladimirk-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
> src/vendstat.c | 82 ++++++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 53 insertions(+), 29 deletions(-)
>
> diff --git a/src/vendstat.c b/src/vendstat.c
> index 11ee73e..baade3e 100644
> --- a/src/vendstat.c
> +++ b/src/vendstat.c
> @@ -172,13 +172,14 @@ static int do_vendor(ib_portid_t *portid, struct ibmad_port *srcport,
> call.attrid = attr_id;
> call.mod = attr_mod;
>
> - if (!ib_vendor_call_via(data, portid, &call, srcport))
> - IBEXIT("vendstat: method %u, attribute %u", method, attr_id);
> -
> + if (!ib_vendor_call_via(data, portid, &call, srcport)) {
> + fprintf(stderr,"vendstat: method %u, attribute %u failure\n", method, attr_id);
We lose the function information when not calling IBEXIT.
Can you create a new MACRO in ibdiag_comon.h called IBERROR? And preserve this
information.
Same comment throughout,
Ira
> + return -1;
> + }
> return 0;
> }
>
> -static void do_config_space_records(ib_portid_t *portid, unsigned set,
> +static int do_config_space_records(ib_portid_t *portid, unsigned set,
> is3_config_space_t *cs, unsigned records)
> {
> unsigned i;
> @@ -194,17 +195,19 @@ static void do_config_space_records(ib_portid_t *portid, unsigned set,
> if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS,
> set ? IB_MAD_METHOD_SET : IB_MAD_METHOD_GET,
> IB_MLX_IS3_CONFIG_SPACE_ACCESS, 2 << 22 | records << 16,
> - cs))
> - IBEXIT("cannot %s config space records", set ? "set" : "get");
> -
> + cs)) {
> + fprintf(stderr,"cannot %s config space records\n", set ? "set" : "get");
> + return -1;
> + }
> for (i = 0; i < records; i++) {
> printf("Config space record at 0x%x: 0x%x\n",
> ntohl(cs->record[i].address),
> ntohl(cs->record[i].data & cs->record[i].mask));
> }
> + return 0;
> }
>
> -static void counter_groups_info(ib_portid_t * portid, int port)
> +static int counter_groups_info(ib_portid_t * portid, int port)
> {
> char buf[1024];
> is4_counter_group_info_t *cg_info;
> @@ -213,15 +216,17 @@ static void counter_groups_info(ib_portid_t * portid, int port)
> /* Counter Group Info */
> memset(&buf, 0, sizeof(buf));
> if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
> - IB_MLX_IS4_COUNTER_GROUP_INFO, port, buf))
> - IBEXIT("counter group info query");
> -
> + IB_MLX_IS4_COUNTER_GROUP_INFO, port, buf)) {
> + fprintf(stderr,"counter group info query failure\n");
> + return -1;
> + }
> cg_info = (is4_counter_group_info_t *) & buf;
> num_cg = cg_info->num_of_counter_groups;
> printf("counter_group_info:\n");
> printf("%d counter groups\n", num_cg);
> for (i = 0; i < num_cg; i++)
> printf("group%d mask %#x\n", i, ntohl(cg_info->group_masks[i]));
> + return 0;
> }
>
> /* Group0 counter config values */
> @@ -236,7 +241,7 @@ static void counter_groups_info(ib_portid_t * portid, int port)
>
> static int cg0, cg1;
>
> -static void config_counter_groups(ib_portid_t * portid, int port)
> +static int config_counter_groups(ib_portid_t * portid, int port)
> {
> char buf[1024];
> is4_config_counter_groups_t *cg_config;
> @@ -251,15 +256,19 @@ static void config_counter_groups(ib_portid_t * portid, int port)
> cg_config->group_selects[1].group_select = (uint8_t) cg1;
>
> if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_SET,
> - IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf))
> - IBEXIT("config counter group set");
> -
> + IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf)) {
> + fprintf(stderr, "config counter group set failure\n");
> + return -1;
> + }
> /* get config counter groups */
> memset(&buf, 0, sizeof(buf));
>
> if (do_vendor(portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
> - IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf))
> - IBEXIT("config counter group query");
> + IB_MLX_IS4_CONFIG_COUNTER_GROUP, port, buf)) {
> + fprintf(stderr, "config counter group query failure\n");
> + return -1;
> + }
> + return 0;
> }
>
> static int general_info, xmit_wait, counter_group_info, config_counter_group;
> @@ -364,20 +373,26 @@ int main(int argc, char **argv)
>
> if (argc) {
> if (resolve_portid_str(ibd_ca, ibd_ca_port, &portid, argv[0],
> - ibd_dest_type, ibd_sm_id, srcport) < 0)
> + ibd_dest_type, ibd_sm_id, srcport) < 0) {
> + mad_rpc_close_port(srcport);
> IBEXIT("can't resolve destination port %s", argv[0]);
> + }
> } else {
> - if (resolve_self(ibd_ca, ibd_ca_port, &portid, &port, 0) < 0)
> + if (resolve_self(ibd_ca, ibd_ca_port, &portid, &port, 0) < 0) {
> + mad_rpc_close_port(srcport);
> IBEXIT("can't resolve self port %s", argv[0]);
> + }
> }
>
> if (counter_group_info) {
> counter_groups_info(&portid, port);
> + mad_rpc_close_port(srcport);
> exit(0);
> }
>
> if (config_counter_group) {
> config_counter_groups(&portid, port);
> + mad_rpc_close_port(srcport);
> exit(0);
> }
>
> @@ -388,6 +403,7 @@ int main(int argc, char **argv)
> if (write_cs_records)
> do_config_space_records(&portid, 1, &write_cs,
> write_cs_records);
> + mad_rpc_close_port(srcport);
> exit(0);
> }
>
> @@ -395,23 +411,27 @@ int main(int argc, char **argv)
> /* but vendors change the VendorId so how know for sure ? */
> /* Only General Info and Port Xmit Wait Counters */
> /* queries are currently supported */
> - if (!general_info && !xmit_wait)
> + if (!general_info && !xmit_wait) {
> + mad_rpc_close_port(srcport);
> IBEXIT("at least one of -N and -w must be specified");
> -
> + }
> /* Would need a list of these and it might not be complete */
> /* so for right now, punt on this */
>
> /* vendor ClassPortInfo is required attribute if class supported */
> memset(&buf, 0, sizeof(buf));
> if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
> - CLASS_PORT_INFO, 0, buf))
> + CLASS_PORT_INFO, 0, buf)) {
> + mad_rpc_close_port(srcport);
> IBEXIT("classportinfo query");
> -
> + }
> memset(&buf, 0, sizeof(buf));
> gi_is3 = (is3_general_info_t *) &buf;
> if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS, IB_MAD_METHOD_GET,
> - IB_MLX_IS3_GENERAL_INFO, 0, gi_is3))
> + IB_MLX_IS3_GENERAL_INFO, 0, gi_is3)) {
> + mad_rpc_close_port(srcport);
> IBEXIT("generalinfo query");
> + }
>
> if (is_ext_fw_info_supported(ntohs(gi_is3->hw_info.device_id))) {
> gi_is4 = (is4_general_info_t *) &buf;
> @@ -452,10 +472,11 @@ int main(int argc, char **argv)
> is3_config_space_t *cs;
> unsigned i;
>
> - if (ntohs(gi_is3->hw_info.device_id) != IS3_DEVICE_ID)
> + if (ntohs(gi_is3->hw_info.device_id) != IS3_DEVICE_ID) {
> + mad_rpc_close_port(srcport);
> IBEXIT("Unsupported device ID 0x%x",
> ntohs(gi_is3->hw_info.device_id));
> -
> + }
> memset(&buf, 0, sizeof(buf));
> /* Set record addresses for each port */
> cs = (is3_config_space_t *) & buf;
> @@ -464,9 +485,10 @@ int main(int argc, char **argv)
> htonl(IB_MLX_IS3_PORT_XMIT_WAIT + ((i + 1) << 12));
> if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS,
> IB_MAD_METHOD_GET, IB_MLX_IS3_CONFIG_SPACE_ACCESS,
> - 2 << 22 | 16 << 16, cs))
> + 2 << 22 | 16 << 16, cs)) {
> + mad_rpc_close_port(srcport);
> IBEXIT("vendstat");
> -
> + }
> for (i = 0; i < 16; i++)
> if (cs->record[i].data) /* PortXmitWait is 32 bit counter */
> printf("Port %d: PortXmitWait 0x%x\n", i + 4, ntohl(cs->record[i].data)); /* port 4 is first port */
> @@ -480,8 +502,10 @@ int main(int argc, char **argv)
> htonl(IB_MLX_IS3_PORT_XMIT_WAIT + ((i + 17) << 12));
> if (do_vendor(&portid, srcport, IB_MLX_VENDOR_CLASS,
> IB_MAD_METHOD_GET, IB_MLX_IS3_CONFIG_SPACE_ACCESS,
> - 2 << 22 | 8 << 16, cs))
> + 2 << 22 | 8 << 16, cs)) {
> + mad_rpc_close_port(srcport);
> IBEXIT("vendstat");
> + }
>
> for (i = 0; i < 8; i++)
> if (cs->record[i].data) /* PortXmitWait is 32 bit counter */
> --
> 1.7.8.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-04-22 15:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-21 14:01 [PATCH infiniband-diags] vendstat: mad_rpc_close_port not called in corner cases Hal Rosenstock
[not found] ` <5536584C.8070505-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
2015-04-22 15:26 ` ira.weiny
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.