netdev - Re: [PATCH v1 wl-next 1/3] wifi: wext: Move wext

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-next>] [day] [month] [year] [list]

Message-ID: <f677361da9e1e4bb032e62301255ab705252e016.camel@sipsolutions.net>
Date: Wed, 16 Oct 2024 10:56:44 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: alexandre.ferrieux@...il.com, kuni1840@...il.com, 
	linux-wireless@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v1 wl-next 1/3] wifi: wext: Move wext_nlevents to
 net->gen[].

+netdev, I think we're starting to discuss more general things :)

On Tue, 2024-10-15 at 17:49 -0700, Kuniyuki Iwashima wrote:
> From: Johannes Berg <johannes@...solutions.net>
> Date: Tue, 15 Oct 2024 08:36:24 +0200
> > On Mon, 2024-10-14 at 13:55 -0700, Kuniyuki Iwashima wrote:
> > > CONFIG_WEXT_CORE cannot be built as a module
> > 
> > Isn't that precisely an argument for _not_ using net->gen[] with all the
> > additional dynamic allocations that implies?
> 
> Exactly...
> 
> Recently I was thinking most of the structs in struct net (except for
> first-class citizens like ipv4/ipv6) should use net->gen[] given the
> distro kernel enables most configs.

Wait I'm confused, to me it seems you're contradicting yourself? :)

If we agree that making it use net->gen[] is more overhead since it
requires additional allocations (which necessarily require more memory
due to alignment etc., but even without that because now you needed
wext_net->net too) ...

Then why do you think more should use net->gen[] if it's built-in?

> But yes, WEXT is always built-in.

I can see an argument for things that aren't always present, obviously,
like bonding and pktgen, but I don't see much of an argument for things
like wext that are either present or not?

> Probably because wext_nlevents was just before a cacheline
> on my setup ?
> 
> $ pahole -EC net vmlinux | grep net_generic -C 30
> ...
> 	} wext_nlevents; /*  2536    24 */
> 	/* --- cacheline 40 boundary (2560 bytes) --- */
> 	struct net_generic *       gen;                                                  /*  2560     8 */

I'd argue that doesn't really mean it makes sense to pull it into
net->gen (where it gets accessed via two indirect pointers)?

That's an argument for reordering things there perhaps, but in struct
net that's probably not too much of an issue unless it shares a
cacheline with something that's used all the time?

johannes