Hi Bin,

On Thu, Sep 10, 2015 at 4:46 AM, Bin Meng bmeng.cn@gmail.com wrote:

Hi Joe,

I found under some circumstances the eth rotation functionality is broken.

Test scenario:

1). Say we have two ethernet devices, eth0 and eth1. Both devices are not probed yet (or probe failed due to ethaddr and eth1addr not set)

Now we set the eth1addr to a valid MAC address, set ethact to NULL. Test a "ping 10.10.0.100", it fails.

The supposed eth rotation should switch to eth1 for 'ping', right? But debugging shows that eth1's probe routine never gets called (see log below).

It seems like we should be skipping a device that fails probe. I guess there may be more smarts that can be added to the eth_init() function that you were already looking at.

=> ping 10.10.0.100

Error: eth_designware#0 address not set.

Error: eth_designware#0 address not set.

Error: eth_designware#0 address not set.

Error: eth_designware#0 address not set. No ethernet found.

Error: eth_designware#0 address not set. ping failed; host 10.10.0.100 is not alive

2). The same test scenario above, but set ethact to eth1, we should get "ping 10.10.0.100" successfully pinged. Unfortunately, it still fails, reporting 'No ethernet found".

=> ping 10.10.0.100

Error: eth_designware#0 address not set.

Error: eth_designware#0 address not set.

Error: eth_designware#0 address not set.

Error: eth_designware#1 address not set.

Error: eth_designware#0 address not set.

Error: eth_designware#0 address not set. No ethernet found.

Error: eth_designware#0 address not set. ping failed; host 10.10.0.100 is not alive

This time eth1's probe routine gets called, but it still failed (see log "Error: eth_designware#1 address not set.")

This is because with eth0 probe failure before, when eth1 get probed, eth1's dev->seq will be set to 0 as it is the first ethernet device seen by dm eth, so its corresponding MAC address is ethaddr, NOT eth1addr. So its probe phase will still fail in eth_post_probe(), and rotation does not work as expected. I think this "dev->seq" match to "eth%daddr" is something we should think about with driver model conversion.

The second case is expected. It seems clear that your device tree does not have aliases defined for your network adapters. If you ask DM to assign a dynamic sequence number, then you get this behavior. Also, it doesn't seem like a real use-case to be assigning only the 2nd MAC address.

So far the eth rotation seems worked pretty well if everything is good (i.e. probe OK), but on some exception path like above two it does not function as expected :(

There are test cases for the typical situations, so I would expect that they work pretty well. :)

Maybe we need to add a test case for the first situation and extend the capability. I think the second case need not be adjusted.

Cheers, -Joe