[PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

Subsystems: broadcom bnxt_en 50 gigabit ethernet driver, networking drivers, the rest · Maintainers: Michael Chan, Pavan Chebbi, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

STALE3247d

7 messages, 3 authors, 2017-12-07 · open the first message on its own page

[PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: Thomas Bogendoerfer <hidden>
Date: 2017-12-05 12:33:55 

bnxt driver spams logfiles with

[  541.003065] bnxt_en 0000:5d:00.1 eth5: Link speed -1 no longer supported

if a direct attached cable (DAC) is plugged into the bnxt card and is
unplugged on the other side. This patch removes the code printing this
message, since it doesn't provide any useful information.

Signed-off-by: Thomas Bogendoerfer <redacted>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8c1dd60eab6f..8a2319ed79dc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1701,18 +1701,9 @@ static int bnxt_async_event_process(struct bnxt *bp,
 	/* TODO CHIMP_FW: Define event id's for link change, error etc */
 	switch (event_id) {
 	case ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE: {
-		u32 data1 = le32_to_cpu(cmpl->event_data1);
-		struct bnxt_link_info *link_info = &bp->link_info;
-
 		if (BNXT_VF(bp))
 			goto async_event_process_exit;
-		if (data1 & 0x20000) {
-			u16 fw_speed = link_info->force_link_speed;
-			u32 speed = bnxt_fw_to_ethtool_speed(fw_speed);
 
-			netdev_warn(bp->dev, "Link speed %d no longer supported\n",
-				    speed);
-		}
 		set_bit(BNXT_LINK_SPEED_CHNG_SP_EVENT, &bp->sp_event);
 		/* fall thru */
 	}
-- 
2.12.3

Re: [PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: David Miller <davem@davemloft.net>
Date: 2017-12-06 18:49:13 

From: Thomas Bogendoerfer <redacted>
Date: Tue,  5 Dec 2017 13:33:40 +0100

bnxt driver spams logfiles with

[  541.003065] bnxt_en 0000:5d:00.1 eth5: Link speed -1 no longer supported

if a direct attached cable (DAC) is plugged into the bnxt card and is
unplugged on the other side. This patch removes the code printing this
message, since it doesn't provide any useful information.

Signed-off-by: Thomas Bogendoerfer <redacted>
Michael, I really need you to review this.

Thank you.

Re: [PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: Michael Chan <michael.chan@broadcom.com>
Date: 2017-12-06 19:27:36 

On Tue, Dec 5, 2017 at 4:33 AM, Thomas Bogendoerfer
[off-list ref] wrote:
quoted hunk
bnxt driver spams logfiles with

[  541.003065] bnxt_en 0000:5d:00.1 eth5: Link speed -1 no longer supported

if a direct attached cable (DAC) is plugged into the bnxt card and is
unplugged on the other side. This patch removes the code printing this
message, since it doesn't provide any useful information.

Signed-off-by: Thomas Bogendoerfer <redacted>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8c1dd60eab6f..8a2319ed79dc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1701,18 +1701,9 @@ static int bnxt_async_event_process(struct bnxt *bp,
        /* TODO CHIMP_FW: Define event id's for link change, error etc */
        switch (event_id) {
        case ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE: {
-               u32 data1 = le32_to_cpu(cmpl->event_data1);
-               struct bnxt_link_info *link_info = &bp->link_info;
-
                if (BNXT_VF(bp))
                        goto async_event_process_exit;
-               if (data1 & 0x20000) {
-                       u16 fw_speed = link_info->force_link_speed;
-                       u32 speed = bnxt_fw_to_ethtool_speed(fw_speed);

-                       netdev_warn(bp->dev, "Link speed %d no longer supported\n",
-                                   speed);
-               }
This is supposed to provide useful information to the user under some
conditions.  In your particular situation, it is not useful since the
speed is -1.  Let me try to modify this code a bit to reduce the spam.
I will post a revised patch in the next 2 hours.

Thanks.

                set_bit(BNXT_LINK_SPEED_CHNG_SP_EVENT, &bp->sp_event);
                /* fall thru */
        }
--
2.12.3

Re: [PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: Thomas Bogendoerfer <hidden>
Date: 2017-12-07 09:14:45 

On Wed, 6 Dec 2017 11:27:31 -0800
Michael Chan [off-list ref] wrote:

On Tue, Dec 5, 2017 at 4:33 AM, Thomas Bogendoerfer
[off-list ref] wrote:
quoted
bnxt driver spams logfiles with

[  541.003065] bnxt_en 0000:5d:00.1 eth5: Link speed -1 no longer supported

if a direct attached cable (DAC) is plugged into the bnxt card and is
unplugged on the other side. This patch removes the code printing this
message, since it doesn't provide any useful information.

Signed-off-by: Thomas Bogendoerfer <redacted>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8c1dd60eab6f..8a2319ed79dc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1701,18 +1701,9 @@ static int bnxt_async_event_process(struct bnxt *bp,
        /* TODO CHIMP_FW: Define event id's for link change, error etc */
        switch (event_id) {
        case ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE: {
-               u32 data1 = le32_to_cpu(cmpl->event_data1);
-               struct bnxt_link_info *link_info = &bp->link_info;
-
                if (BNXT_VF(bp))
                        goto async_event_process_exit;
-               if (data1 & 0x20000) {
-                       u16 fw_speed = link_info->force_link_speed;
-                       u32 speed = bnxt_fw_to_ethtool_speed(fw_speed);

-                       netdev_warn(bp->dev, "Link speed %d no longer supported\n",
-                                   speed);
-               }
This is supposed to provide useful information to the user under some
conditions. 
well, it will print the forced rate, if there is one configured and -1 otherwise,
if the link is lost or will not come up because of a cable problem. I don't see much
value in that...

Thomas.

-- 
SUSE Linux GmbH
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: Michael Chan <michael.chan@broadcom.com>
Date: 2017-12-07 09:24:49 

On Thu, Dec 7, 2017 at 1:14 AM, Thomas Bogendoerfer
[off-list ref] wrote:
On Wed, 6 Dec 2017 11:27:31 -0800
Michael Chan [off-list ref] wrote:
quoted
On Tue, Dec 5, 2017 at 4:33 AM, Thomas Bogendoerfer
[off-list ref] wrote:
quoted
bnxt driver spams logfiles with

[  541.003065] bnxt_en 0000:5d:00.1 eth5: Link speed -1 no longer supported

if a direct attached cable (DAC) is plugged into the bnxt card and is
unplugged on the other side. This patch removes the code printing this
message, since it doesn't provide any useful information.

Signed-off-by: Thomas Bogendoerfer <redacted>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 8c1dd60eab6f..8a2319ed79dc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1701,18 +1701,9 @@ static int bnxt_async_event_process(struct bnxt *bp,
        /* TODO CHIMP_FW: Define event id's for link change, error etc */
        switch (event_id) {
        case ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE: {
-               u32 data1 = le32_to_cpu(cmpl->event_data1);
-               struct bnxt_link_info *link_info = &bp->link_info;
-
                if (BNXT_VF(bp))
                        goto async_event_process_exit;
-               if (data1 & 0x20000) {
-                       u16 fw_speed = link_info->force_link_speed;
-                       u32 speed = bnxt_fw_to_ethtool_speed(fw_speed);

-                       netdev_warn(bp->dev, "Link speed %d no longer supported\n",
-                                   speed);
-               }
This is supposed to provide useful information to the user under some

6 more quoted lines

The main purpose is to tell the user that the speed he selected for a
port is no longer supported due to an incompatible speed configured on
the other port.  This is useful for the user so that he can either
take action to change the speed or do nothing as he sees fit.

Re: [PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: Thomas Bogendoerfer <hidden>
Date: 2017-12-07 09:39:07 

On Thu, 7 Dec 2017 01:24:43 -0800
Michael Chan [off-list ref] wrote:

On Thu, Dec 7, 2017 at 1:14 AM, Thomas Bogendoerfer
[off-list ref] wrote:
quoted
well, it will print the forced rate, if there is one configured and -1 otherwise,
if the link is lost or will not come up because of a cable problem. I don't see much
value in that...
The main purpose is to tell the user that the speed he selected for a
port is no longer supported due to an incompatible speed configured on
the other port.  This is useful for the user so that he can either
take action to change the speed or do nothing as he sees fit.
just out of curiosity what's meant my imcompatible speed on the other port ?
Does the message show up continously like in the problem case I already know ?

Thomas.

-- 
SUSE Linux GmbH
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH net-next] bnxt: Don't print message, if DAC isn't connected on both ends

From: Michael Chan <michael.chan@broadcom.com>
Date: 2017-12-07 09:47:37 

On Thu, Dec 7, 2017 at 1:39 AM, Thomas Bogendoerfer
[off-list ref] wrote:
On Thu, 7 Dec 2017 01:24:43 -0800
Michael Chan [off-list ref] wrote:
quoted
On Thu, Dec 7, 2017 at 1:14 AM, Thomas Bogendoerfer
[off-list ref] wrote:
quoted
well, it will print the forced rate, if there is one configured and -1 otherwise,
if the link is lost or will not come up because of a cable problem. I don't see much
value in that...
The main purpose is to tell the user that the speed he selected for a
port is no longer supported due to an incompatible speed configured on
the other port.  This is useful for the user so that he can either
take action to change the speed or do nothing as he sees fit.
just out of curiosity what's meant my imcompatible speed on the other port ?
Does the message show up continously like in the problem case I already know ?
On some dual-port cards, 10G and 25G are not compatible.  If one port
links up at 25G, for example, the other port will never link up if it
is set to 10G.  In this case the 10G port gets a message from the
firmware and the driver will print the "10G no longer supported"
message.  It's supposed to be a one time message.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help