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 f

Constantine A. Murenin mureninc at gmail.com
Tue Oct 16 16:20:51 PDT 2007


On 16/10/2007, Alexander Leidinger <netchild at freebsd.org> wrote:
> 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?

Alexander,

Your questions are very valid indeed. It is this kind of problem that
the OpenBSD sensors framework is trying to solve.

OpenBSD already has the ipmi(4) using the sensors framework, and it is
simply impossible for the system administrator to not use IPMI on an
OpenBSD machine -- all you have to do is type `sysctl hw.sensors` or
`systat sensors`, and you'll see all the readings momentarily --
unlike in some other systems, the values are already cached within the
framework.

Some googling revealed a nice web-site with some sensor readings:

http://monitor.cns.ualberta.ca/?details

http://monitor.cns.ualberta.ca/?startcarp1.srv.ualberta.ca_sensors

hw.sensors.ipmi0.temp0=18.00 degC (Temp), OK
hw.sensors.ipmi0.temp1=50.00 degC (Temp), OK
hw.sensors.ipmi0.temp2=25.00 degC (Ambient Temp), OK
hw.sensors.ipmi0.fan0=10875 RPM (FAN MOD 1A RPM), OK
hw.sensors.ipmi0.fan1=9675 RPM (FAN MOD 1C RPM), OK
hw.sensors.ipmi0.fan2=10650 RPM (FAN MOD 1B RPM), OK
hw.sensors.ipmi0.fan3=9525 RPM (FAN MOD 1D RPM), OK
hw.sensors.ipmi0.fan4=11100 RPM (FAN MOD 2A RPM), OK
hw.sensors.ipmi0.fan5=9600 RPM (FAN MOD 2C RPM), OK
hw.sensors.ipmi0.fan6=10575 RPM (FAN MOD 2B RPM), OK
hw.sensors.ipmi0.fan7=9600 RPM (FAN MOD 2D RPM), OK
hw.sensors.ipmi0.indicator0=Off (Intrusion), OK


>
> 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.

Exactly, I'd also like to point out that the i2c sensors on OpenBSD
are enabled by default and were tested on many systems. It is very
well known that improper probing of the smbus may result in certain
machines being no longer usable -- just checkout the Linux-based
lm-sensors README.thinkpad file, regarding ThinkPads.  This is
something that the OpenBSD project has been very serious about, and a
lot of work was put into making the i2c probing as safe as possible,
whilst still providing necessary functionality.

> 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.

I am not so certain about the extensive infrastructure argument.

So far, many FreeBSD committers have greeted me with total ignorance
in regards to the p4 infrastructure. I repeatedly asked if some of my
fixes could be committed from perforce into the CVS tree, and provided
links to the patches that were submitted into my perforce branch, only
to have to deal with the fact that people cannot apply those patches
to the tree, and simply do not know how to use the perforce.

It also remains to be seen how regular users can test the stuff from
the perforce repository. For the complete duration of this project, I
am not aware of any single person that has ever used my perforce
branch, although a lot of people have expressed interest in this
project.

> 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.


More information about the cvs-src mailing list