New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 814442 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task



Sign in to add a comment

Unify/Systematize how network interfaces and their properties are obtained in autotests

Project Member Reported by kirtika@google.com, Feb 21 2018

Issue description

OS: R66

A few people (or maybe just briannorris@ at different times) have noticed that we have many autotests that use home-cooked ways of figuring out what network interfaces a device is using, whether the link is up etc. 

This bug tracks cleaning that up. Specifically:

1. Try to avoid using sysfs reads (e.g. accessing /sys/class/net/*) 
wherever possible. Get the same info from `ip` instead.
If reading the sysfs is required, better do it in network/interface.py instead of every test having its own hack. 

2. *try* replacing ifconfig with `ip` where possible. 
Already tracked at  crbug.com/410601 


3. Don't use hacky ways to figure out if the link is available/up. 
e.g. network_EthernetStressPlug test currently does this: 


        # Linux can set network link state by frobbing "flags" bitfields.
        # Bit fields are documented in include/uapi/linux/if.h.
        # Bit 0 is IFF_UP (link up=1 or down=0).
        elif os.path.exists(self.eth_flagspath):
            try:
                fp = open(self.eth_flagspath, mode='r')
                val= int(fp.readline().strip(), 16)
                fp.close()
            except:
                raise error.TestError('Could not read %s' % self.eth_flagspath)

Candidate tests: 
1. power_LoadTest: Already fixed by Brian: https://chromium-review.googlesource.com/#/c/chromiumos/third_party/autotest/+/919703/

2. power_SuspendStress

3. network_EthernetStressPlug



 

Comment 1 by kirtika@google.com, Feb 21 2018

4. network_UdevRename uses this: 

def GetInterfaceList():
  """Gets the list of network interfaces on this host.

  @return List containing a string for each interface name
  """
  return os.listdir('/sys/class/net')


> 3. network_EthernetStressPlug

Remember this was written 3 or 4 years ago. I'm happy to take ownership of rewriting the hacks that are currently working just to make sure they are not copied as an example of "how to do this".

I'll look into using 'ip' command instead of trolling /sys/class/net directory.
If you'd like to handle some of this Grant, feel free to grab the bug :)

I'd just suggest getting people to use the interface.py library. e.g., the new interface.get_interfaces(). And that would also help people not to re-implement the Interface.is_up property, etc.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/6ce87431b086434c9747c722b3211e739cc215ce

commit 6ce87431b086434c9747c722b3211e739cc215ce
Author: Brian Norris <briannorris@chromium.org>
Date: Fri Feb 23 22:15:56 2018

power_SuspendStress: use 'interface' library

We don't need to reimplement the 'is_link_operational()' check.

BUG=chromium:814442
TEST=power_SuspendStress with cable removed and 'check_connection'
     forced on

Change-Id: I7b8c6f59529ecc6a718b86a1127a2c85e44e9a00
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/930021
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Todd Broch <tbroch@chromium.org>

[modify] https://crrev.com/6ce87431b086434c9747c722b3211e739cc215ce/client/site_tests/power_SuspendStress/power_SuspendStress.py

Project Member

Comment 6 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/aafceeea0fcfc4d4841ade440ce9e607d0cdba96

commit aafceeea0fcfc4d4841ade440ce9e607d0cdba96
Author: Brian Norris <briannorris@chromium.org>
Date: Fri Feb 23 22:15:55 2018

network/interface: factor out 'is_link_operational' helper

We want this for other tests.

BUG=chromium:814442
TEST=none

Change-Id: I803026c11cd8377edae42b5019e935d845a2f8f0
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/933286
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/aafceeea0fcfc4d4841ade440ce9e607d0cdba96/client/common_lib/cros/network/interface.py
[modify] https://crrev.com/aafceeea0fcfc4d4841ade440ce9e607d0cdba96/client/cros/backchannel.py

Sign in to add a comment