Message-ID: <20190422123133.161fb1a1@cakuba.netronome.com>
Date:   Mon, 22 Apr 2019 12:31:33 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com,
        dsahern@...il.com
Subject: Re: [patch net-next v2 15/16] netdevsim: move netdev
 creation/destruction to dev probe

On Sat, 20 Apr 2019 12:29:21 +0200, Jiri Pirko wrote:
> diff --git a/tools/testing/selftests/bpf/test_offload.py b/tools/testing/selftests/bpf/test_offload.py
> index 5f2e4f9e70e4..a0566dcf064a 100755
> --- a/tools/testing/selftests/bpf/test_offload.py
> +++ b/tools/testing/selftests/bpf/test_offload.py
> @@ -1,6 +1,7 @@
>  #!/usr/bin/python3
>  
>  # Copyright (C) 2017 Netronome Systems, Inc.
> +# Copyright (c) 2019 Mellanox Technologies. All rights reserved

What's your guiding principle with adding those copyright lines
everywhere?

>  # This software is licensed under the GNU General License Version 2,
>  # June 1991 as shown in the file COPYING in the top-level directory of this
> @@ -24,6 +25,7 @@ import struct
>  import subprocess
>  import time
>  import traceback
> +import errno

aphabetic order.

>  
>  logfile = None
>  log_level = 1
> @@ -323,42 +325,85 @@ class DebugfsDir:
>  
>          return dfs
>  
> -class NetdevSim:
> +class NetdevSimDev:
>      """
> -    Class for netdevsim netdevice and its attributes.
> +    Class for netdevsim bus device and its attributes.
>      """
>  
> -    def __init__(self, link=None):
> -        self.link = link
> +    def __init__(self, port_count=1):
> +        addr = 0
> +        while True:
> +            try:
> +                with open("/sys/bus/netdevsim/new_device","w") as f:

space after ,

> +                    f.write("%u %u" % (addr, port_count))
> +            except OSError as e:
> +                if e.errno == errno.ENOSPC:
> +                    addr += 1
> +                    continue
> +                raise e
> +            break
>  
> -        self.dev = self._netdevsim_create()
>          devs.append(self)
> +        self.addr = addr
> +        self.dfs_dir = "/sys/kernel/debug/netdevsim/netdevsim%u/" % addr
>  
> -        self.ns = ""
> +        self.nsims = []
> +        for port_index in range(port_count):
> +            self.nsims.append(NetdevSim(self, port_index))
>  
> -        self.dfs_dir = '/sys/kernel/debug/netdevsim/netdevsim0/ports/0/'
> -        self.dev_dir = self.dfs_dir + '/dev/'
> -        self.dfs_refresh()
> +    def dfs_num_bound_progs(self):
> +        path = os.path.join(self.dfs_dir, "bpf_bound_progs")
> +        _, progs = cmd('ls %s' % (path))
> +        return len(progs.split())
>  
> -    def __getitem__(self, key):
> -        return self.dev[key]
> +    def dfs_get_bound_progs(self, expected):
> +        progs = DebugfsDir(os.path.join(self.dfs_dir, "bpf_bound_progs"))
> +        if expected is not None:
> +            if len(progs) != expected:
> +                fail(True, "%d BPF programs bound, expected %d" %
> +                     (len(progs), expected))
> +        return progs
>  
> -    def _netdevsim_create(self):
> -        link = "" if self.link is None else "link " + self.link.dev['ifname']
> -        _, old  = ip("link show")
> -        ip("link add sim%d {link} type netdevsim".format(link=link))
> -        _, new  = ip("link show")
> +    def remove(self):
> +        with open("/sys/bus/netdevsim/del_device","w") as f:

and here, etc.

> +            f.write("%u" % self.addr)
> +        devs.remove(self)
>  
> -        for dev in new:
> -            f = filter(lambda x: x["ifname"] == dev["ifname"], old)
> -            if len(list(f)) == 0:
> -                return dev
> +    def remove_nsim(self, nsim):
> +        self.nsims.remove(nsim)
> +        with open("/sys/bus/netdevsim/devices/netdevsim%u/del_port" % self.addr ,"w") as f:
> +            f.write("%u" % nsim.port_index)
>  
> -        raise Exception("failed to create netdevsim device")
> +class NetdevSim:
> +    """
> +    Class for netdevsim netdevice and its attributes.
> +    """
> +
> +    def __init__(self, nsimdev, port_index):
> +        self.nsimdev = nsimdev
> +        self.port_index = port_index
> +        self.ns = ""
> +        self.dfs_dir = "%s/ports/%u/" % (nsimdev.dfs_dir, port_index)
> +        self.dfs_refresh()
> +
> +        ifname = "eni%unp%u" % (nsimdev.addr, port_index + 1)
> +        timeout = 0.5
> +        timeout_start = time.time()
> +
> +        while True:
> +            try:
> +                _, [self.dev] = ip("link show dev %s" % ifname)
> +            except Exception as e:
> +                if time.time() < timeout_start + timeout:
> +                    continue
> +                raise e
> +            break

udevadm settle?  The reliance on latest systemd is a real bad idea,
could you fall back to netdevsimX names if eni$SOME$CUFT is not found?

> +    def __getitem__(self, key):
> +        return self.dev[key]
>  
>      def remove(self):
> -        devs.remove(self)
> -        ip("link del dev %s" % (self.dev["ifname"]), ns=self.ns)
> +        self.nsimdev.remove_nsim(self)
>  
>      def dfs_refresh(self):
>          self.dfs = DebugfsDir(self.dfs_dir)

Powered by blists - more mailing lists