Bug 1640356

Nice catch. The problem origins from where we get link names (ip -o link):

# ip -o link
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000\    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000\    link/ether 00:1a:4a:22:39:27 brd ff:ff:ff:ff:ff:ff
3: br0.100@eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000\    link/ether 00:1a:4a:22:39:27 brd ff:ff:ff:ff:ff:ff
4: br0.10@test@eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000\    link/ether 00:1a:4a:22:39:27 brd ff:ff:ff:ff:ff:ff
#

Here we take 2nd column without trailing @NONE (no idea now why that).

We must identify the link names by some different means..

(see in above example that vlan device can have '@' in its name, as well as (I think) eth name, so we cant say for sure what br0.100@eth0 or br0.10@test@eth0 is - eth name or vlan name with appended eth name?)

Any idea how to list interface names uniquely and precisely is welcomed.

Created attachment 1495228 [details]
example patch to use sysfs to collect interface names

Hehe I looked at the networking module and its get_eth_interfaces() function and kinda cried a little bit.

For me, a much cleaner and fool-proof way to get a list of net devs is to just look at the /sys/class/net/ directory contents. You will not have to do any goofy substring matching or splits or anything like that...

$ ls -1 /sys/class/net
br0
enp0s31f6
lo
tun0
virbr0
virbr0-nic
virbr1
virbr1-nic
virbr4
virbr4-nic
virbr5
virbr5-nic
vnet0
vnet1
vnet2
wlp58s0


I am not so good at python but how about this?

(In reply to Patrick Talbert from comment #4)
> Created attachment 1495228 [details]
> example patch to use sysfs to collect interface names
> 
> Hehe I looked at the networking module and its get_eth_interfaces() function
> and kinda cried a little bit.
> 
> For me, a much cleaner and fool-proof way to get a list of net devs is to
> just look at the /sys/class/net/ directory contents. You will not have to do
> any goofy substring matching or splits or anything like that...
> 
> $ ls -1 /sys/class/net
> br0
> enp0s31f6
> lo
> tun0
> virbr0
> virbr0-nic
> virbr1
> virbr1-nic
> virbr4
> virbr4-nic
> virbr5
> virbr5-nic
> vnet0
> vnet1
> vnet2
> wlp58s0
> 
> 
> I am not so good at python but how about this?

That is easy to implement, but is there a similar way to get equivalent list of interfaces per given namespace?

Since we use the same get_eth_interfaces method also for generating list of commands like:

ip netns exec $namespace ethtool -k $eth

(for each $eth in $namespace)


(I am networking noob so might ask basic question here)


Thanks in advance for advice.

Created attachment 1495545 [details]
amended patch to exclude bonding_masters

Hey Pavel,

Yeah, the patch handles that as well:

224             # Devices that exist in a namespace use less ethtool
225             # parameters. Run this per namespace.
226             for namespace in self.get_ip_netns(ip_netns_file):
227                 ns_cmd_prefix = cmd_prefix + namespace + " "
228                 netns_netdev_list = self.call_ext_prog(ns_cmd_prefix +
229                                                        "ls -1 /sys/class/net/")
230                 for eth in netns_netdev_list['output'].splitlines():
231                     self.add_cmd_output([
232                         ns_cmd_prefix + "ethtool " + eth,
233                         ns_cmd_prefix + "ethtool -i " + eth,
234                         ns_cmd_prefix + "ethtool -k " + eth,
235                         ns_cmd_prefix + "ethtool -S " + eth
236                     ])



One thing I did realize is that if the bonding module is loaded, it creates a *file* called 'bonding_masters' under /sys/class/net/. So we want to exclude that from the list (in both the default and extra namespaces).

I submit here a second patch which skips add_cmd_output if it sees 'bonding_masters' .


What do you think? At least on my system, it creates a valid report with the expected ethtool output for both the default namespace and custom namespaces.

Patrick

Indeed the patch works well there also.

Thanks for the prompt feedback and definitely also for patch - after adding some comments, I propose it to upstream:

https://github.com/sosreport/sos/pull/1458


(devel_ack+ for 7.7, now it is tooo late for 7.6 in any case)

POSTed to upstream

POSTed to upstream

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2019:2295