custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Pavel Machek]

On Wed 2016-02-24 10:01:49, Johannes Berg wrote:
> On Tue, 2016-02-23 at 22:45 +0100, Pavel Machek wrote:
> 
> > Well "the airplane mode" is well defined. It means no intentional
> > transmitting at radio frequencies.
> > 
> > The fact that you are allowed to use WIFI on certain flights does not
> > change anything.
> 
> Nope, not that simple. Pick up any (contemporary) smartphone and watch
> what happens with the airplane mode indicator (the little airplane
> icon) when you enable wifi after enabling airplane mode. It stays
> there.
> 
> Clearly the same logic could then apply to an actual LED (on a system
> that's less size-constrained, e.g. a laptop or tablet.)
> 
> Thus, the display of "in airplane mode" *does* have policy, and clearly
> there's precedent for not disabling the icon or LED when wifi is
> enabled, but the kernel shouldn't really impose that. Now, the kernel
> has a "safe" default which does what you thought was the "well defined"
> airplane mode, but at the same time it's obviously not good enough.

If you want different trigger, implement different trigger. If you
want to indicate all but wifi, implement all but wifi, and then
userspace can select it by writing trigger name. If you want complete
userspace control, fine, but we have standard interface and it is not
ioctl.

> > (Besides, finding all LEDs with given trigger is trivial
> > operation. Besides... there should never be more than one).
> 
> That *might* actually work. But once a tool has detached the trigger
> the information is gone; and tools would have to do that to control the
> LED, making recovery from any kind of error difficult.

If you allow userspace to control the LED, well, then the LED may no
longer display the "airplane mode" status. Debugging "wrong trigger is
set" will certainly be less tricky than figuring out that "airplane
mode trigger" is actually "no trigger" based on some obscure ioctls.

> In any case, I've applied those patches already.

Well, you can still revert them before it becomes ABI. David does
not have to pull from you and Linus does not have to pull from
Linus. Besides, the series really should have been Cc-ed to LED
people, too.

Having custom, ioctl-based interface to control the LED is just too
ugly. (And I did not see it documented, which should be another reason
not to merge it.)  

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Johannes Berg]

On Wed, 2016-02-24 at 11:46 +0100, Pavel Machek wrote:

> If you want different trigger, implement different trigger. If you
> want to indicate all but wifi, implement all but wifi, and then
> userspace can select it by writing trigger name. 

This is still mostly a strawman, since userspace cannot have a database
of LEDs that indicate airplane mode.

I'm sure you'd also not like to see 2**7 triggers implemented in rfkill
to cover all the possibilities.

> If you want complete
> userspace control, fine, but we have standard interface and it is not
> ioctl.

The "standard interface" is usable if you really just want to driver a
single LED and you know which one.

I think you're looking at this the wrong way, focusing too much on the
LED aspect.

Really what you have here is a concept of "airplane mode", and that
concept is specific to the rfkill subsystem. This happens to affect
mostly an LED trigger, today, but as a concept it's something that
*should* be managed within the rfkill subsystem.

> Besides, the series really should have been Cc-ed to LED
> people, too.

That's simply unreasonable, you're essentially saying that any user of
any kernel infrastructure should be Cc'ed to the implementer of that
infrastructure... 9/10 patches in this series aren't even LED specific,
only the *previous* patch, the one that tied the "airplane mode"
concept to an LED trigger in a very standard way had anything to do
with LED triggers at all.

johannes

Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Pavel Machek]

On Wed 2016-02-24 12:01:37, Johannes Berg wrote:
> On Wed, 2016-02-24 at 11:46 +0100, Pavel Machek wrote:
> 
> > If you want different trigger, implement different trigger. If you
> > want to indicate all but wifi, implement all but wifi, and then
> > userspace can select it by writing trigger name. 
> 
> This is still mostly a strawman, since userspace cannot have a database
> of LEDs that indicate airplane mode.

Why would it need to? It could look at default triggers for the led if
it really wanted to.

> I'm sure you'd also not like to see 2**7 triggers implemented in rfkill
> to cover all the possibilities.

Are all the possibilities useful? I assumed about 2 are. If you need
2**7 of them, LED triggers can have parameters.

> > If you want complete
> > userspace control, fine, but we have standard interface and it is not
> > ioctl.
> 
> The "standard interface" is usable if you really just want to driver a
> single LED and you know which one.
> 
> I think you're looking at this the wrong way, focusing too much on the
> LED aspect.
> 
> Really what you have here is a concept of "airplane mode", and that
> concept is specific to the rfkill subsystem. This happens to affect
> mostly an LED trigger, today, but as a concept it's something that
> *should* be managed within the rfkill subsystem.

How is that concept used outside the LEDs? What semantics does
"airplane mode" have? You tried to explain "airplane mode" is not well
defined up in this thread...

> > Besides, the series really should have been Cc-ed to LED
> > people, too.
> 
> That's simply unreasonable, you're essentially saying that any user of
> any kernel infrastructure should be Cc'ed to the implementer of that
> infrastructure... 9/10 patches in this series aren't even LED
> > specific,

I'm not saying that. I'm saying that LED maintainers should be Cced,
to keep the interfaces consistent.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Johannes Berg]

On Wed, 2016-02-24 at 13:14 +0100, Pavel Machek wrote:
> 
> Why would it need to? It could look at default triggers for the led
> if it really wanted to.

And then it needs to change them; if anything goes wrong error recovery
is practically impossible since the trigger information is lost
forever.

> > I'm sure you'd also not like to see 2**7 triggers implemented in
> > rfkill
> > to cover all the possibilities.
> 
> Are all the possibilities useful? I assumed about 2 are. If you need
> 2**7 of them, LED triggers can have parameters.

It's not my position to decide which combinations some system
integrator or userspace developer might find useful.

Even when we add parameters to a trigger (I don't see a generic way to
do that, but please do enlighten me), you're now ignoring the issue of
the userspace controlled GSM modem...

> > Really what you have here is a concept of "airplane mode", and that
> > concept is specific to the rfkill subsystem. This happens to affect
> > mostly an LED trigger, today, but as a concept it's something that
> > *should* be managed within the rfkill subsystem.
> 
> How is that concept used outside the LEDs? What semantics does
> "airplane mode" have? You tried to explain "airplane mode" is not
> well defined up in this thread...

I'd say it's well-defined for any given set of system software, so if
e.g. NetworkManager decides to define it one way, and connman another
way, there's a definition but the kernel need not interfere with it.

> > > Besides, the series really should have been Cc-ed to LED
> > > people, too.
> > 
> > That's simply unreasonable, you're essentially saying that any user
> > of any kernel infrastructure should be Cc'ed to the implementer of
> > that infrastructure... 9/10 patches in this series aren't even LED
> > specific,
> 
> I'm not saying that. I'm saying that LED maintainers should be Cced,
> to keep the interfaces consistent.

I pretty much have to read it that way, since the LED API is in no way
impacted by these changes. Here's a new trigger, with some magic inner
working. No impact on the LED API.

johannes

Re: custom ioctl-based interface to control LED in networking (was Re: [PATCHv2 09/10] rfkill: Userspace control for airplane mode)

[Pavel Machek]

On Wed 2016-02-24 14:31:33, Johannes Berg wrote:
> On Wed, 2016-02-24 at 13:14 +0100, Pavel Machek wrote:
> > 
> > Why would it need to? It could look at default triggers for the led
> > if it really wanted to.
> 
> And then it needs to change them; if anything goes wrong error recovery
> is practically impossible since the trigger information is lost
> forever.

Well, conventional way to solve this is to simply name the led
"acer::airplane"... that way it is persistent. We already do that for
display/keyboard backlights.

> It's not my position to decide which combinations some system
> integrator or userspace developer might find useful.
> 
> Even when we add parameters to a trigger (I don't see a generic way to
> do that, but please do enlighten me), you're now ignoring the issue of
> the userspace controlled GSM modem...

Take a look at the blinking triggers.

> > > Really what you have here is a concept of "airplane mode", and that
> > > concept is specific to the rfkill subsystem. This happens to affect
> > > mostly an LED trigger, today, but as a concept it's something that
> > > *should* be managed within the rfkill subsystem.
> > 
> > How is that concept used outside the LEDs? What semantics does
> > "airplane mode" have? You tried to explain "airplane mode" is not
> > well defined up in this thread...
> 
> I'd say it's well-defined for any given set of system software, so if
> e.g. NetworkManager decides to define it one way, and connman another
> way, there's a definition but the kernel need not interfere with it.

So... the LED changes meaning during boot? That's surely not a nice
solution.

So... you rather store bit in a kernel with unclear semantics,
explaining "oh I need to leave the flexibility to userland"? Sorry,
that's just ugly.

> > I'm not saying that. I'm saying that LED maintainers should be Cced,
> > to keep the interfaces consistent.
> 
> I pretty much have to read it that way, since the LED API is in no way
> impacted by these changes. Here's a new trigger, with some magic inner
> working. No impact on the LED API.

New LED trigger and new ioctl for LED control... not matching how LEDs
are normally controlled.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html