[PATCH 1/2] infiniband-diags/src/ibqueryerrors.c: fix segv due to CA port 0 reference

[PATCH 1/2] infiniband-diags/src/ibqueryerrors.c: fix segv due to CA port 0 reference

[Marcus R Epperson]

Signed-off-by: Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 src/ibqueryerrors.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/ibqueryerrors.c b/src/ibqueryerrors.c
index fa116a9..230f6b2 100644
--- a/src/ibqueryerrors.c
+++ b/src/ibqueryerrors.c
@@ -427,8 +427,9 @@ static int print_results(ib_portid_t * portid, char *node_name,
 		}
 
 		if (portnum == 0xFF)
-			printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
-			       node->ports[0]->guid, str);
+			if (node->type == IB_NODE_SWITCH)
+				printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
+				       node->ports[0]->guid, str);
 		else
 			printf("   GUID 0x%" PRIx64 " port %d:%s\n",
 			       node->ports[portnum]->guid, portnum, str);
-- 
1.7.4.1

--
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

[PATCH 2/2] infiniband-diags/src/ibqueryerrors.c: don't increment bad_ports for port ALL

[Marcus R Epperson]

Signed-off-by: Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 src/ibqueryerrors.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/ibqueryerrors.c b/src/ibqueryerrors.c
index 230f6b2..d37d268 100644
--- a/src/ibqueryerrors.c
+++ b/src/ibqueryerrors.c
@@ -426,17 +426,17 @@ static int print_results(ib_portid_t * portid, char *node_name,
 			summary.bad_nodes++;
 		}
 
-		if (portnum == 0xFF)
+		if (portnum == 0xFF) {
 			if (node->type == IB_NODE_SWITCH)
 				printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
 				       node->ports[0]->guid, str);
-		else
+		} else {
 			printf("   GUID 0x%" PRIx64 " port %d:%s\n",
 			       node->ports[portnum]->guid, portnum, str);
-		if (portnum != 0xFF && port_config)
-			print_port_config(node_name, node, portnum);
-
-		summary.bad_ports++;
+			if (port_config)
+				print_port_config(node_name, node, portnum);
+			summary.bad_ports++;
+		}
 	}
 	return (n);
 }
-- 
1.7.4.1

--
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

Re: [PATCH 1/2] infiniband-diags/src/ibqueryerrors.c: fix segv due to CA port 0 reference

[Ira Weiny]

On Tue, 27 Sep 2011 15:52:30 -0700
Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> 
> Signed-off-by: Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

I am curious if you have actually hit this bug.  I have not run into any non-switches which set AllPortSelect which would lead to this condition.[*]  In this case was it your intention to suppress the errors for "all" ports and allow the individual ports to print?  I believe this is the correct behaviour.

Also, I assume this patch and 2/2 must go together.  However, this patch should include the braces from patch 2 to disambiguate the else statement.  Could you redo the series for that?

Thanks,
Ira

[*] but apparently the spec does not limit AllPortSelect to switches...

> ---
>  src/ibqueryerrors.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ibqueryerrors.c b/src/ibqueryerrors.c
> index fa116a9..230f6b2 100644
> --- a/src/ibqueryerrors.c
> +++ b/src/ibqueryerrors.c
> @@ -427,8 +427,9 @@ static int print_results(ib_portid_t * portid, char *node_name,
>  		}
>  
>  		if (portnum == 0xFF)
> -			printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
> -			       node->ports[0]->guid, str);
> +			if (node->type == IB_NODE_SWITCH)
> +				printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
> +				       node->ports[0]->guid, str);
>  		else
>  			printf("   GUID 0x%" PRIx64 " port %d:%s\n",
>  			       node->ports[portnum]->guid, portnum, str);
> -- 
> 1.7.4.1
> 


-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
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

Re: [PATCH 1/2] infiniband-diags/src/ibqueryerrors.c: fix segv due to CA port 0 reference

[Marcus Epperson]

On Tue, Sep 27, 2011 at 5:29 PM, Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org> wrote:
> On Tue, 27 Sep 2011 15:52:30 -0700
> Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>>
>> Signed-off-by: Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> I am curious if you have actually hit this bug.  I have not run into any non-switches which set AllPortSelect which would lead to this condition.[*]  In this case was it your intention to suppress the errors for "all" ports and allow the individual ports to print?  I believe this is the correct behaviour.

I did hit this bug unfortunately. It happened with a Voltaire 4036E IO
node. My first inclination was to print the NodeGUID instead, but it
looked like you didn't want NodeGUIDs showing up anymore (based on
12748135). So the only other thing I could think of was to suppress
the "all" line for non-switches.

>
> Also, I assume this patch and 2/2 must go together.  However, this patch should include the braces from patch 2 to disambiguate the else statement.  Could you redo the series for that?

Sure thing.

-Marcus

>
> Thanks,
> Ira
>
> [*] but apparently the spec does not limit AllPortSelect to switches...
>
>> ---
>>  src/ibqueryerrors.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/ibqueryerrors.c b/src/ibqueryerrors.c
>> index fa116a9..230f6b2 100644
>> --- a/src/ibqueryerrors.c
>> +++ b/src/ibqueryerrors.c
>> @@ -427,8 +427,9 @@ static int print_results(ib_portid_t * portid, char *node_name,
>>               }
>>
>>               if (portnum == 0xFF)
>> -                     printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
>> -                            node->ports[0]->guid, str);
>> +                     if (node->type == IB_NODE_SWITCH)
>> +                             printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
>> +                                    node->ports[0]->guid, str);
>>               else
>>                       printf("   GUID 0x%" PRIx64 " port %d:%s\n",
>>                              node->ports[portnum]->guid, portnum, str);
>> --
>> 1.7.4.1
>>
>
>
> --
> Ira Weiny
> Math Programmer/Computer Scientist
> Lawrence Livermore National Lab
> 925-423-8008
> weiny2-i2BcT+NCU+M@public.gmane.org
>
--
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

Re: [PATCH 1/2] infiniband-diags/src/ibqueryerrors.c: fix segv due to CA port 0 reference

[Ira Weiny]

On Tue, 27 Sep 2011 17:50:13 -0700
Marcus Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Tue, Sep 27, 2011 at 5:29 PM, Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org> wrote:
> > On Tue, 27 Sep 2011 15:52:30 -0700
> > Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> >>
> >> Signed-off-by: Marcus R Epperson <marcus.r.epperson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >
> > I am curious if you have actually hit this bug.  I have not run into any non-switches which set AllPortSelect which would lead to this condition.[*]  In this case was it your intention to suppress the errors for "all" ports and allow the individual ports to print?  I believe this is the correct behaviour.
> 
> I did hit this bug unfortunately. It happened with a Voltaire 4036E IO
> node. My first inclination was to print the NodeGUID instead, but it
> looked like you didn't want NodeGUIDs showing up anymore (based on
> 12748135). So the only other thing I could think of was to suppress
> the "all" line for non-switches.

This is very interesting.  I wonder what ALL means for this node?  If you have 2 ports on the node and each is in a different fabric will it return errors for the port in the other fabric???

Anyway, I think it would be an error to report "ALL" in this case.

Thanks applied both v2 patches
Ira

> 
> >
> > Also, I assume this patch and 2/2 must go together.  However, this patch should include the braces from patch 2 to disambiguate the else statement.  Could you redo the series for that?
> 
> Sure thing.
> 
> -Marcus
> 
> >
> > Thanks,
> > Ira
> >
> > [*] but apparently the spec does not limit AllPortSelect to switches...
> >
> >> ---
> >>  src/ibqueryerrors.c |    5 +++--
> >>  1 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/ibqueryerrors.c b/src/ibqueryerrors.c
> >> index fa116a9..230f6b2 100644
> >> --- a/src/ibqueryerrors.c
> >> +++ b/src/ibqueryerrors.c
> >> @@ -427,8 +427,9 @@ static int print_results(ib_portid_t * portid, char *node_name,
> >>               }
> >>
> >>               if (portnum == 0xFF)
> >> -                     printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
> >> -                            node->ports[0]->guid, str);
> >> +                     if (node->type == IB_NODE_SWITCH)
> >> +                             printf("   GUID 0x%" PRIx64 " port ALL:%s\n",
> >> +                                    node->ports[0]->guid, str);
> >>               else
> >>                       printf("   GUID 0x%" PRIx64 " port %d:%s\n",
> >>                              node->ports[portnum]->guid, portnum, str);
> >> --
> >> 1.7.4.1
> >>
> >
> >
> > --
> > Ira Weiny
> > Math Programmer/Computer Scientist
> > Lawrence Livermore National Lab
> > 925-423-8008
> > weiny2-i2BcT+NCU+M@public.gmane.org
> >


-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
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