diff mbox

[v3,infiniband-diags] ibdiag_common.c: Disambiguate link speed when QDR and could be FDR10

Message ID b3114ca9-18da-f640-7ab1-f05cc364bc27@dev.mellanox.co.il (mailing list archive)
State Accepted, archived
Delegated to: Ira Weiny
Headers show

Commit Message

Hal Rosenstock Dec. 20, 2016, 4:28 p.m. UTC
Rather than printing "Could be FDR10", print
"Could be FDR10 (Found link at QDR but expected speed is FDR10)"
for this case.

Confusion is with words "Could be" and fact that both
QDR and FDR10 are both 10.0 Gbps.

Signed-off-by: Hal Rosenstock <hal@mellanox.com>
---
Change since v2:
Added Active comment to clarify value of 4 for PortInfo:LinkSpeedActive

Change since v1:
Preserved original "Could be FDR10" so can grep for "Could be"
along with adding new language.

 src/ibdiag_common.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ira Weiny Dec. 20, 2016, 5:02 p.m. UTC | #1
On Tue, Dec 20, 2016 at 11:28:38AM -0500, Hal Rosenstock wrote:
> 
> Rather than printing "Could be FDR10", print
> "Could be FDR10 (Found link at QDR but expected speed is FDR10)"
> for this case.
> 
> Confusion is with words "Could be" and fact that both
> QDR and FDR10 are both 10.0 Gbps.
> 
> Signed-off-by: Hal Rosenstock <hal@mellanox.com>

Thanks applied,
Ira

> ---
> Change since v2:
> Added Active comment to clarify value of 4 for PortInfo:LinkSpeedActive
> 
> Change since v1:
> Preserved original "Could be FDR10" so can grep for "Could be"
> along with adding new language.
> 
>  src/ibdiag_common.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ibdiag_common.c b/src/ibdiag_common.c
> index f74eed4..8cad03a 100644
> --- a/src/ibdiag_common.c
> +++ b/src/ibdiag_common.c
> @@ -814,8 +814,14 @@ check_ext_speed:
>  
>  check_fdr10_active:
>  	if ((mad_get_field(port->ext_info, 0,
> -			   IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0)
> -		snprintf(speed_msg, msg_size, "Could be FDR10");
> +			   IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0) {
> +		/* Special case QDR to try to avoid confusion with FDR10 */
> +		if (mad_get_field(port->info, 0, IB_PORT_LINK_SPEED_ACTIVE_F) == 4)	/* QDR (10.0 Gbps) */
> +			snprintf(speed_msg, msg_size,
> +				 "Could be FDR10 (Found link at QDR but expected speed is FDR10)");
> +		else
> +			snprintf(speed_msg, msg_size, "Could be FDR10");
> +	}
>  }
>  
>  int vsnprint_field(char *buf, size_t n, enum MAD_FIELDS f, int spacing,
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yuval Shaia Dec. 20, 2016, 8:55 p.m. UTC | #2
Reviewed-By: Yuval Shaia <yuval.shaia@oracle.com>

On Tue, Dec 20, 2016 at 11:28:38AM -0500, Hal Rosenstock wrote:
> 
> Rather than printing "Could be FDR10", print
> "Could be FDR10 (Found link at QDR but expected speed is FDR10)"
> for this case.
> 
> Confusion is with words "Could be" and fact that both
> QDR and FDR10 are both 10.0 Gbps.
> 
> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
> ---
> Change since v2:
> Added Active comment to clarify value of 4 for PortInfo:LinkSpeedActive
> 
> Change since v1:
> Preserved original "Could be FDR10" so can grep for "Could be"
> along with adding new language.
> 
>  src/ibdiag_common.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ibdiag_common.c b/src/ibdiag_common.c
> index f74eed4..8cad03a 100644
> --- a/src/ibdiag_common.c
> +++ b/src/ibdiag_common.c
> @@ -814,8 +814,14 @@ check_ext_speed:
>  
>  check_fdr10_active:
>  	if ((mad_get_field(port->ext_info, 0,
> -			   IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0)
> -		snprintf(speed_msg, msg_size, "Could be FDR10");
> +			   IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0) {
> +		/* Special case QDR to try to avoid confusion with FDR10 */
> +		if (mad_get_field(port->info, 0, IB_PORT_LINK_SPEED_ACTIVE_F) == 4)	/* QDR (10.0 Gbps) */
> +			snprintf(speed_msg, msg_size,
> +				 "Could be FDR10 (Found link at QDR but expected speed is FDR10)");
> +		else
> +			snprintf(speed_msg, msg_size, "Could be FDR10");
> +	}
>  }
>  
>  int vsnprint_field(char *buf, size_t n, enum MAD_FIELDS f, int spacing,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/ibdiag_common.c b/src/ibdiag_common.c
index f74eed4..8cad03a 100644
--- a/src/ibdiag_common.c
+++ b/src/ibdiag_common.c
@@ -814,8 +814,14 @@  check_ext_speed:
 
 check_fdr10_active:
 	if ((mad_get_field(port->ext_info, 0,
-			   IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0)
-		snprintf(speed_msg, msg_size, "Could be FDR10");
+			   IB_MLNX_EXT_PORT_LINK_SPEED_ACTIVE_F) & FDR10) == 0) {
+		/* Special case QDR to try to avoid confusion with FDR10 */
+		if (mad_get_field(port->info, 0, IB_PORT_LINK_SPEED_ACTIVE_F) == 4)	/* QDR (10.0 Gbps) */
+			snprintf(speed_msg, msg_size,
+				 "Could be FDR10 (Found link at QDR but expected speed is FDR10)");
+		else
+			snprintf(speed_msg, msg_size, "Could be FDR10");
+	}
 }
 
 int vsnprint_field(char *buf, size_t n, enum MAD_FIELDS f, int spacing,