test/py: Detect timeout in phy negotiation

PATCH New Michal Simek 2 participants 9 messages May 17, 2016 – May 18, 2016

lore

Share & tools

lore

Michal Simek

author

2016-05-17 13:57 UTC #0 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

If timeout happen it should be reported as fault.

Signed-off-by Michal Simek <[email protected]>

---

 test/py/tests/test_net.py | 3 +++
 1 file changed, 3 insertions(+)

test/py/tests/test_net.py +3 -0

index 4f4a876c1335..833a8fd5b3e3 100644
@@ -81,6 +81,7 @@ def test_net_dhcp(u_boot_console):
 
     u_boot_console.run_command('setenv autoload no')
     output = u_boot_console.run_command('dhcp')
+    assert 'TIMEOUT' not in output
     assert 'DHCP client bound to address ' in output
 
     global net_set_up

@@ -117,6 +118,7 @@ def test_net_ping(u_boot_console):
         pytest.skip('Network not initialized')
 
     output = u_boot_console.run_command('ping $serverip')
+    assert 'TIMEOUT' not in output
     assert 'is alive' in output
 
 @pytest.mark.buildconfigspec('cmd_net')

@@ -152,6 +154,7 @@ def test_net_tftpboot(u_boot_console):
     sz = f.get('size', None)
     if sz:
         expected_text += '%d' % sz
+    assert 'TIMEOUT' not in output
     assert expected_text in output
 
     expected_crc = f.get('crc32', None)

Signature · 1 line

1.9.1

Stephen Warren

2016-05-17 16:50 UTC #1 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 1 line · Michal Simek

If timeout happen it should be reported as fault.

Presumably if a timeout occurs, the expected text does not appear, i.e. the existing assert fails anyway?

Anyway, it's useful to point out problems explicitly, so,

Acked-by Stephen Warren <[email protected]>

Michal Simek

author

2016-05-17 16:56 UTC #2 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Hi Stephen,

Quoted 8 lines · Stephen Warren

Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e. the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>

Unfortunately I found this issue when I was checking logs where I am getting this.

ethernet at e000b000 Waiting for PHY auto negotiation to complete......... TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms)
Zynq> .Zynq> setenv serverip 192.168.0.105

I haven't looked at the exact reason why it is failing but IMHO it is worth to check.

Thanks,
Michal

Stephen Warren

2016-05-17 17:00 UTC #3 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 23 lines · Michal Simek

Hi Stephen,
Quoted 8 lines · Stephen Warren
Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e. the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>
Unfortunately I found this issue when I was checking logs where I am getting this.
ethernet at e000b000 Waiting for PHY auto negotiation to complete......... TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms)
Zynq> .Zynq> setenv serverip 192.168.0.105

Quoted 2 lines

I haven't looked at the exact reason why it is failing but IMHO it is worth to check.

Oh, in that case I think I should withdraw my ack; in the log above, the operation completed successfully, so I'm not convinced the test should be marked a failure. I thought this change simply provided more detail
re: the cause of a test failure.

Michal Simek

author

2016-05-17 17:03 UTC #4 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 24 lines · Stephen Warren

Quoted 23 lines · Michal Simek
Hi Stephen,
Quoted 8 lines · Stephen Warren
Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e. the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>
Unfortunately I found this issue when I was checking logs where I am getting this.
ethernet at e000b000 Waiting for PHY auto negotiation to complete......... TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms)
Zynq> .Zynq> setenv serverip 192.168.0.105

Quoted 7 lines

Quoted 2 lines
I haven't looked at the exact reason why it is failing but IMHO it is worth to check.
Oh, in that case I think I should withdraw my ack; in the log above, the operation completed successfully, so I'm not convinced the test should be marked a failure. I thought this change simply provided more detail
re: the cause of a test failure.

Is there any other way how to run just phy negotiation and mark this test as fail?

Thanks
Michal

Stephen Warren

2016-05-17 17:36 UTC #5 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 28 lines · Michal Simek

Quoted 27 lines · Stephen Warren
Quoted 25 lines · Michal Simek
Hi Stephen,
Quoted 8 lines · Stephen Warren
Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e. the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>
Unfortunately I found this issue when I was checking logs where I am getting this.
ethernet at e000b000 Waiting for PHY auto negotiation to complete......... TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms)
Zynq> .Zynq> setenv serverip 192.168.0.105
I haven't looked at the exact reason why it is failing but IMHO it is worth to check.

Quoted 7 lines

Quoted 4 lines
Oh, in that case I think I should withdraw my ack; in the log above, the operation completed successfully, so I'm not convinced the test should be marked a failure. I thought this change simply provided more detail
re: the cause of a test failure.
Is there any other way how to run just phy negotiation and mark this test as fail?

I don't see anything obvious that will do that; I think that only happens when net_loop starts. It might be possible to add some new command to test just PHY startup, or a new mode for net_loop() that just waited for link up and did no protocol work.

Michal Simek

author

2016-05-17 18:00 UTC #6 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 35 lines · Stephen Warren

Quoted 34 lines · Michal Simek
Quoted 32 lines · Stephen Warren
Quoted 24 lines · Michal Simek
Hi Stephen,
Quoted 9 lines · Stephen Warren
Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e.
the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>
Unfortunately I found this issue when I was checking logs where I am getting this.
ethernet at e000b000 Waiting for PHY auto negotiation to complete......... TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms)
Zynq> .Zynq> setenv serverip 192.168.0.105
Quoted 2 lines
I haven't looked at the exact reason why it is failing but IMHO it is worth to check.
Oh, in that case I think I should withdraw my ack; in the log above, the operation completed successfully, so I'm not convinced the test should be marked a failure. I thought this change simply provided more detail
re: the cause of a test failure.

Quoted 7 lines

Quoted 2 lines
Is there any other way how to run just phy negotiation and mark this test as fail?
I don't see anything obvious that will do that; I think that only happens when net_loop starts. It might be possible to add some new command to test just PHY startup, or a new mode for net_loop() that just waited for link up and did no protocol work.

ok. It means you are saying that this is bug and we are not able to test it by single test. That's why I think this should be report as a bug when this is happening just to hands up there is a problem.

Not sure if there is any way to report is as a warning instead of bug that test case will pass with warning.

Thanks,
Michal

Stephen Warren

2016-05-17 19:09 UTC #7 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 44 lines · Michal Simek

Quoted 35 lines · Stephen Warren
Quoted 34 lines · Michal Simek
Quoted 32 lines · Stephen Warren
Quoted 24 lines · Michal Simek
Hi Stephen,
Quoted 9 lines · Stephen Warren
Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e.
the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>
Unfortunately I found this issue when I was checking logs where I am getting this.
ethernet at e000b000 Waiting for PHY auto negotiation to complete......... TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms)
Zynq> .Zynq> setenv serverip 192.168.0.105
Quoted 2 lines
I haven't looked at the exact reason why it is failing but IMHO it is worth to check.
Oh, in that case I think I should withdraw my ack; in the log above, the operation completed successfully, so I'm not convinced the test should be marked a failure. I thought this change simply provided more detail
re: the cause of a test failure.
Quoted 7 lines
Quoted 2 lines
Is there any other way how to run just phy negotiation and mark this test as fail?
I don't see anything obvious that will do that; I think that only happens when net_loop starts. It might be possible to add some new command to test just PHY startup, or a new mode for net_loop() that just waited for link up and did no protocol work.

Quoted 6 lines

ok. It means you are saying that this is bug and we are not able to test it by single test. That's why I think this should be report as a bug when this is happening just to hands up there is a problem.
Not sure if there is any way to report is as a warning instead of bug that test case will pass with warning.

At least my only-marginally-old version of pytest (2.5.1) doesn't appear to have any concept of warnings. Some quite new versions of pytest do, although I'm not sure how they're reported. Integrating some form of warning system into the tests could be useful though.

I wonder if the correct fix for this issue isn't to make the PHY code actively return an error whenever there is a TIMEOUT raised, rather than presumably ignoring it as it does now (since the network code goes on to actually work correctly)?

Michal Simek

author

2016-05-18 10:56 UTC #8 lore raw

PATCH 1/1 test/py: Detect timeout in phy negotiation

Quoted 51 lines · Stephen Warren

Quoted 50 lines · Michal Simek
Quoted 37 lines · Stephen Warren
Quoted 36 lines · Michal Simek
Quoted 34 lines · Stephen Warren
Quoted 25 lines · Michal Simek
Hi Stephen,
Quoted 9 lines · Stephen Warren
Quoted 1 line · Michal Simek
If timeout happen it should be reported as fault.
Presumably if a timeout occurs, the expected text does not appear, i.e.
the existing assert fails anyway?
Anyway, it's useful to point out problems explicitly, so,
Acked-by Stephen Warren <[email protected]>
Unfortunately I found this issue when I was checking logs where I am getting this.
ethernet at e000b000 Waiting for PHY auto negotiation to complete.........
TIMEOUT !
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
DHCP client bound to address 192.168.0.107 (882 ms) Zynq> .Zynq> setenv serverip 192.168.0.105
Quoted 2 lines
I haven't looked at the exact reason why it is failing but IMHO it is worth to check.
Oh, in that case I think I should withdraw my ack; in the log above, the
operation completed successfully, so I'm not convinced the test should be marked a failure. I thought this change simply provided more detail
re: the cause of a test failure.
Quoted 7 lines
Quoted 2 lines
Is there any other way how to run just phy negotiation and mark this test as fail?
I don't see anything obvious that will do that; I think that only happens when net_loop starts. It might be possible to add some new command to test just PHY startup, or a new mode for net_loop() that just waited for link up and did no protocol work.
ok. It means you are saying that this is bug and we are not able to test it by single test. That's why I think this should be report as a bug when this is happening just to hands up there is a problem.

Quoted 7 lines

Quoted 2 lines
Not sure if there is any way to report is as a warning instead of bug that test case will pass with warning.
At least my only-marginally-old version of pytest (2.5.1) doesn't appear to have any concept of warnings. Some quite new versions of pytest do, although I'm not sure how they're reported. Integrating some form of warning system into the tests could be useful though.

ok.

Quoted 5 lines

I wonder if the correct fix for this issue isn't to make the PHY code actively return an error whenever there is a TIMEOUT raised, rather than presumably ignoring it as it does now (since the network code goes on to actually work correctly)?

I will send some patches to fix this problem that transfers not start when TIMEOUT happens.

All these boards should be fixed to return error codes properly.
drivers/net/cpsw.c:612: phy_startup(phy);
drivers/net/keystone_net.c:581: phy_startup(phy_dev);
drivers/net/keystone_net.c:768: phy_startup(priv->phydev);
drivers/net/lpc32xx_eth.c:605: phy_startup(phydev);
drivers/net/mvgbe.c:697: phy_startup(phydev);
drivers/net/mvneta.c:1478: phy_startup(phydev);
drivers/net/mvpp2.c:3384: phy_startup(phy_dev);
drivers/net/pic32_eth.c:343: phy_startup(priv->phydev);

But again if we have visible errors and code is not handling it properly I do agree that code should be fixed first but still testing should be able to capture this problem for others to avoid that this problem happens again.

Thanks,
Michal