cvs commit: src/etc Makefile sensorsd.conf src/etc/defaults rc.conf src/etc/rc.d Makefile sensorsd src/lib/libc/gen sysctl.3 src/sbin/sysctl sysctl.8 sysctl.c src/share/man/man5 rc.conf.5 src/share/man/man9 Makefile sensor_attach.9 src/sys/conf files ...

Tue Oct 16 09:34:28 PDT 2007

Quoting Poul-Henning Kamp <phk at phk.freebsd.dk> (from Mon, 15 Oct 2007  
19:52:07 +0000):

> In message <20071015210909.1b6b693b at deskjail>, Alexander Leidinger writes:
>> Quoting "Poul-Henning Kamp" <phk at phk.freebsd.dk> (Mon, 15 Oct 2007   
>> 15:01:47 +0000):
>
>>> >And I responded that we need to have a way to export data from the
>>> >kernel to userland in an unified way.
>>>
>>> I can't seem to find the supposed requirement for unification here,
>>
>> You didn't comment on what I wrote about reducing the work of
>> reinventing a way to export the data we want to export again and again
>> and again and again...
>
> Yes, that is the abstract argument, but the very same argument can
> be made for every other single kind of entity which consumes or
> produces bytes, from fingerprint readers to 9-track tape stations.

Why do we have a common linked list API? It's easy enough to do it  
again and again and again... We have it because we don't want to do it  
again and again... And with the sensors API we gain something similar.  
Sure, we can do it with sysctls in every place. But with the sensors  
API we reduce the code to write for such things like with the linked  
lists API.

> My beef with this code is that it unifies a lot of non-alike
> measurements and indications in a new, and pretty bogus IMO, api,
> but adding no value above a plain old sysctl while doing so.

It adds meta-data which can be used in an automated way. This is done  
with a consistent and documented API. Sure, we can do it with sysctls  
by hand, but see above.

>>> and in fact, it is exactly that a lot of crap gets bundled up
>>> under the wildcard term of "sensor" that I object to.
>>
>> Exporting temperature readings / voltage measurement, system status is
>> not crap.
>
> Actually it is, if it can be done as well (or better) from userland
> by exporting the underlying communications paths.

See below for the mbmon example. And for some things we already had  
the export of this via sysctl (temperature of Intel core processors),  
the sensors API "just" groups this together so that people don't have  
to hunt such info down, adds meta data to it, so that tools can do  
some automation, and unifies the API to export this info from the  
kernel.

>> And when you write some monitoring probes in a large server
>> farm, you don't want to hunt down every interesting data value in a lot
>> of places.
>
> Ahh, so this brings me to the next problem with sensors.
>
> If by "monitoring" you mean "plot MRTG graphs", then I guess sensors
> could possibly make sense, although, I'd have to point out that all
> that can be done just as well from ports/sysutils/whatever.

With elevated privileges for those tools. See below.

> But if you're talking real world monitoring, then sensors are
> pointless, because there is no way to derive necessary machine
> readable contextual information, such as alarm limits, resolution,
> calibration constants, hysteresis, polling periods etc.

Normally you configure this in the tools which do the monitoring, not  
in the tools which acquire the data. I'm not talking about doing this  
in the sensors framework, but I talk about to get rid of the need to  
hunt down such information from everywhere. I makes it easier to write  
monitoring probes. It is not supposed to make the monitoring itself  
easier.

> Or, for that matter, machine-readable information about what is
> actually being measured and where it is being measured and what it
> means.

A human being still has to interprete the measurements. No doubts. But  
with the framework you don't have to hunt down where to read the  
sensor data, and how to name it. You can write a probe which takes  
everything in the the sensors mib and let it produce names and values  
for the probed things automatically.

> There are frameworks for doing that sort of stuff in professional
> server hardware, but since OpenBSD doesn't support those, IPMI,
> ACPI etc, they have come up with this VAX-mentality junkheap instead.

IPMI is intended to handle situations when the OS is not booted or the  
system is not powered on. Yes, it allows some features when the OS is  
booted too. Now... how much hardware out there supports IPMI, or  
better... how much in production use doesn't use IPMI? I would say  
quite a lot (we all know the managers, "do everything with no money at  
all"). The sensors framework allows to monitor those systems in an  
easy (because you don't need to invest that much work with the sensors  
framework) way. Have you identified some parts in the sensors  
framework which make it impossible to play together with IPMI? If yes,  
would you please so kind and tell us what prevents them to play  
together?

Regarding ACPI... Nate (our ACPI committer) asked after the commit why  
acpi_thermal was not modified to play together with the sensors  
framework (the reason: ACPI was a little bit too much to do in the SoC  
for Constantine, Nate agreed to work together (after some other work  
in ACPI he wants to finish first) if desired (put on hold from the  
sensors side until this discussion comes tp an conclusion), and also  
suggested some improvements for the sensors framework). He didn't told  
us about something in the framework, which prevents the use of it  
together with ACPI.

>>> >I already told you last time
>>> >that the current way (access to the i2c or smbus) needs more access
>>> >rights than using the userland parts of the sensors framework.
>>>
>>> More rights than what exactly ?
>>
>> One popular userland temperature/voltage reading tool (as it supports a
>> lot of popular devices) is mbmon. It is currently a SUID root
>> application.
>
> Let me get this straight, you're telling me:
>
> 	"I'm worried about this code running as root, so I'm putting
> 	it in the kernel instead."
>
> Can anybody here spot the logic flaw of this argument ?
>
> Anybody ?
>
> Right, I thought so.  Lets pretend you didn't say that.

You missed the point. In our architecture we can not do it without  
executing some code with elevated privileges. What I suggested was to  
replace SUID root code with unknown security heritage and potential  
complete access to the machine (direct access to ISA I/O ports and the  
smbus), not only with code (which runs with elevated privileges) from  
a known security heritage (from people which strongly care about  
security), but also with an access method which doesn't need any  
elevated privileges at all. I assume you think that for example  
expoting some predefined numbers with "sysctl hw.sensors >outout" is  
not something we need to care that much about from a security point of  
view. Feel free to tell me if I'm wrong with my assumption. I at least  
feel safer to export an int over sysctl, than to run the SUID root  
mbmon tool.

There may be other ways to accomplish the same. Maybe a daemon which  
runs with elevated privileges and just outputs the temperature/voltage  
as a number to a tool which runs without elevated privileges and  
collects the data. So far we don't have such a tool, and in the  
beginning I'm not sure we can trust such a tool to the same extend as  
we can trust sysctl.

And this would only affect sensors like lm (which is not part of the  
sensors framework, but uses the sensors framework to export it's data,  
and was used as an example to show how the sensors framework can be  
used). What about other such data which lives in subsystems we don't  
want to let an userland application mess around with? I don't know if  
we want to let an userland application give that much access to ACPI.  
ACPI already exports some data with sysctl. ATM it does it by hand.  
The sensors framework provides an unified API to not do everything by  
hand anymore.

>>> No, I have yet to see why we need this framework.
>>
>> Several committers which voted for this project in the soc webinterface
>> seem to think otherwise, else they wouldn't have voted for it.
>
> I repeat: The SoC interface is not the gateway to -current.

It provides an idea in what people are interested in. And several  
committers here in the thread also showed interest in this framework  
(maybe not in the current implementation, but at least in the idea  
behind it). Just because you do not see how such a framework can be  
useful to you (so far I have the impression from your mails, that you  
object to the idea of this framework), it doesn't mean other people  
are not interested in it. So it is not a gateway for concrete  
implementations into -current, but it shows what people are interested  
in for -current. The current implementation of the sensors framework  
may not use the best way of doing something in some places, but it is  
not as bad as you give people the impression. In the mean time  
Constantine had a look at the constructive critics we got so far, and  
thinks that moving to the taskqueue API is not a bug problem. For the  
newbus suggestion so far he asked for more information, as the man  
pages didn't gave him enough input. As far as I know, the man pages of  
newbus are known to be in a suboptimal shape. But as you seem to be  
not interested in the idea of the sensors framework, one can have the  
impression, that you are not interested to hear about improvements of  
the implementation.

>>> >>    C) How it integrates into FreeBSD and for the places where
>>> >>       it does not (newbus ?) why it doesn't.
>>> >
>>
>> We're discussing it right now. And I was willing to discuss this even
>> back when you talked about it the first time.
>
> It should have been dicussed and fixed before being committed to
> -current.  Current is not where you start, it is where you end, these

We where willing to listed to your concerns. You stopped talking about  
them back then.

> kind of things are not details to be hashed out in CVS.

Constantine asked for review several times on -current. He got some  
reviews several times for commits to perforce. He incorporated  
suggestions from those reviews, or explained why it is like it is and  
why he can not switch (with no replies with suggestions how to solve  
the problems he sees with the suggestions). Now you come and ask why  
nobody pointed out some flaws before (without telling us which  
technical flaws you talk about).

> Ten years ago when we didn't have P4 and the _extensive_ infrastructure
> for making it easy for people to work out of the tree, we had to do
> stuff like that, but there is no excuse for it today.

Nobody is perfect. There will always be some bugs when something is  
committed to -current. You don't talk about obvious problems here.  
There's no destabilized system, there are no panics. You talk about  
not using an underdocumented API and not using a generic framework for  
creating tasks (so basically we talk about doing something by hand  
what can be done with less work by using an API... sounds like what  
the sensors framework tries to do for some kinds of data export).

Note: the commit of the code from the sensors project was done in 3  
steps on purpose, the sensors framework, 2 temperature/voltage chip  
drivers as examples how to use the sensors framework (and the benefit  
of getting rid of a SUID root application in the userland), and  
converting the existing CPU temperature sysctl (for some specific  
Intel CPUs) from doing a sysctl by hand to the sensors framework (as  
an example how to convert an existing part to the sensors framework).  
So far we mixed the discussion about various parts of this together  
under the umbrella of "sensors framework", which is not ok.

>>> >The code we have currently doesn't harm the system, [...]
>>>
>>> This is where I disagree: it does harm the system.
>>>
>>> It dumps a load of badly thought out code into our source tree, and
>>> that will effectively be in the way of any well thought out solution
>>> that might ever appear.
>>>
>>> This stuff should be backed out, and forced to live outside the tree
>>> until it satisfies our quality and architectural requirements.
>>
>> How does this compare to your attitude of bringing stuff into the tree
>> and letting other people fixup collateral damage in the past? But I
>> drift away from the topic... so back to the sensors framework.
>
> It does not compare, please answer the question instead of trying

Which question? I don't see a question from you in the quoted part  
above. And without mentioning the bad code, we can not proceed  
discussing about it. The constructive responses so far don't give the  
impression that there code is that bad as you suggest here.

> to insult me, because you're no good at it.  I have been insulted

I don't try to insult you.

>> To be able to address our quality and architectural requirements, we
>> need first to know which quality and architectural requirements are
>> violated by which part of the code in question. As you seem to have
>> identified them, would you please so kind and share your concrete
>> findings?
>
> Would you be so kind as to read the emails I've sent you ?  Maybe
> this time expending some effort to understand what people tell you in
> them, rather than pretending they contained nothing ?

So far I see from you that you are not interested in the idea of the  
framework. And I see mails where people are interested in the idea of  
the framework. And I see private mails which point out some  
improvements. According specific quality problems, I haven't seen a  
concrete mail from you, like saying "you use X, but this is should be  
done with Y, see at place A how to do it better". I have seen such  
mails from other people, but not from you.

>>> Forcing it to stay out of -current, means that the motivational
>>> pressure is on the people who thing we badly need this featureset,
>>> and gives them reason to improve it.
>>>
>>> Leaving it in the tree, is the sure fire way for it to become an
>>> unmaintained lump of code that slowly rots away.
>>
>> You did a psycho-analysis of Constantine so that you are able to tell
>> that he doesn't fix the issues while he agreed to improve the framework?
>
> No, it's far worse.
>
> I have 15 years of experience with this kind of code being slammed
> into the tree in an unfinished and badly thought out shape.
>
> And I have wasted far more time cleaning it up afterwards, than you
> or constantine will have spent on FreeBSD five years from now.

So would you please share your experience and tell us where parts are  
wrong because of what reason and what kind of technique is better  
suited to do it? So far I've only seen that you don't like the idea  
itself and that you for this reason haven't looked at concrete problems.

If someone reading this does not share this point of view, please,  
tell me about it. So far I haven't got any mail to my similar question  
in a previous mail, so please tell me about it if you think that I'm  
wrong when I say that Poul gives the impression he doesn't like the  
idea (while several other committers see benefit in it) and that it  
seems that he didn't provided concrete pointers to problems in the  
implementation.

Bye,
Alexander.

-- 
http://www.Leidinger.net  Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org     netchild @ FreeBSD.org  : PGP ID = 72077137
Who on earth would eat a charred caterpillar!?
No, no, you SINGE 'em!  You SINGE 'em and eat 'em!