netdev - Re: [PATCH net-next] selftests: drv-net: test drivers sleeping in ndo_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250106074104.7fdf837f@kernel.org>
Date: Mon, 6 Jan 2025 07:41:04 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, shuah@...nel.org, willemb@...gle.com, petrm@...dia.com,
 linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: drv-net: test drivers sleeping in
 ndo_get_stats64

On Sun, 05 Jan 2025 10:55:50 -0500 Willem de Bruijn wrote:
> Two tiny comments, neither cause for respin.

Let me respin, not much work since comment changes shouldn't need 
a re-test..

> > +@...t_disruptive
> > +def procfs_downup_hammer(cfg) -> None:
> > +    """
> > +    Reading stats via procfs only holds the RCU lock, drivers often try
> > +    to sleep when reading the stats, or don't protect against races.
> > +    """
> > +    # Max out the queues, we'll flip between max an 1  
> 
> s/an/and/
> 
> > +    channels = ethnl.channels_get({'header': {'dev-index': cfg.ifindex}})
> > +    if channels['combined-count'] == 0:
> > +        rx_type = 'rx'
> > +    else:
> > +        rx_type = 'combined'
> > +    cur_queue_cnt = channels[f'{rx_type}-count']
> > +    max_queue_cnt = channels[f'{rx_type}-max']
> > +
> > +    cmd(f"ethtool -L {cfg.ifname} {rx_type} {max_queue_cnt}")
> > +    defer(cmd, f"ethtool -L {cfg.ifname} {rx_type} {cur_queue_cnt}")
> > +
> > +    # Real test stats
> > +    stats = __run_inf_loop("cat /proc/net/dev")
> > +    defer(stats.kill)
> > +
> > +    ipset = f"ip link set dev {cfg.ifname}"
> > +    defer(ip, f"link set dev {cfg.ifname} up")  
> 
> unimportant: could reference ipset here too, as in below.

Ha, that's what I did initially, but then running it I discovered
that ip() adds the initial "ip", so we end up executing:

 ip ip link set...
-- 
pw-bot: cr