linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Denis Kenzior <denkenz@gmail.com>
To: Johannes Berg <johannes@sipsolutions.net>,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH] nl80211: Support mgmt frame unregistrations
Date: Wed, 11 Sep 2019 08:33:34 -0500	[thread overview]
Message-ID: <fcb56c09-d9de-c086-ff27-75e3bd1da6a8@gmail.com> (raw)
In-Reply-To: <7a089d5ec94da92fc0c1255a2afa6ae0d8b8aaa4.camel@sipsolutions.net>

Hi Johannes,

On 9/11/19 3:53 AM, Johannes Berg wrote:
> On Wed, 2019-09-04 at 11:22 -0500, Denis Kenzior wrote:
>> To state another way, it is
>> currently not possible to write a userspace application that utilizes a
>> single nl80211 genl socket, instead it must open multiple genl sockets
>> for multiple wdevs on multiple phys.
> 
> I don't see how this is too onerous for the application, every
> application is basically going to have an event loop anyway.

What does having an event loop have to do with this? :)

> 
> Thus, I don't really see any reason for us to add a bunch of code just
> to make an application track fewer file descriptors - we need to have
> the cleanup on close already anyway, so why not actually exercise those
> code paths?

I just find this super wasteful.  We have instances where we need to 
register to a single management frame temporarily.  So opening and 
closing a socket just for that is just bloat.

> 
> I do note that with the "unregister on iftype change" patch you could
> switch to an unsupported type and reach this, but I don't think you'd
> want to rely on that :-)
> 

Not sure I understand?

> Possibly I could imagine a reason for this if you needed a single socket
> for functional reason, but you're not really giving any such reason. I
> could imagine that there might be races, but I'm having a hard time
> coming up with a scenario where they actually matter ... if you really
> really get a race between e.g. RX-AUTH and INTERFACE-DEL you'll try to
> do some operations that will just fail, but so what?

- Waste on the userspace side.  Typically userspace uses some sort of 
abstraction for tracking genl sockets.  So it has to allocate buffers, 
etc.  Can we get around that? Sure, but you're not winning any arguments 
that the nl80211 is 'nice to use' that way.

- Waste on the kernel side.  Each socket costs something for the kernel, 
makes things harder to audit, etc, etc.  And we now have people trying 
to stuff 15+ cards into a single system.  Each card might have multiple 
netdevs.  Each netdev might need multiple file descriptors open.  So 
we're ending up needing 30-60, or whatever file descriptors when we 
could just as easily use 1.  Extreme case? Sure, but I like to remove 
bloat whenever / wherever I can.

Regards,
-Denis

      reply	other threads:[~2019-09-11 15:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04 16:22 [PATCH] nl80211: Support mgmt frame unregistrations Denis Kenzior
2019-09-11  8:53 ` Johannes Berg
2019-09-11 13:33   ` Denis Kenzior [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fcb56c09-d9de-c086-ff27-75e3bd1da6a8@gmail.com \
    --to=denkenz@gmail.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).