Subject: Re: [PATCH v0 03/12] mlxsw: core: Add core environment module for port temperature reading

From: Andrew Lunn <[email-masked]>
Date: Thu, 21 Jun 2018 20:34:40 +0200

> Hi Andrew,

Adding Guenter Roeck, the HWMON maintainer.
 
> The temperature of each individual module can be obtained
> through ethtool.

You mean via --module-info?

FYI: I plan to add hwmon support to the kernel SFP code. So if you
ever decide to swap to the kernel SFP code, not your own, the raw
temperatures will be exported.

> The worst temperature is necessary for the system cooling
> control decision.

I would expect the system cooling would understand that.

> Up to 64 SFP/QSFP modules could be connected to the system.
> Some of them could cooper modules, which doesn't provide
> temperature measurement.

SFP modules are hot-plugable. So i would also expect the hwmon devices
to hotplug. If there is no sensor, then there is no hwmon device... If
there is no hwmon device, it plays no part in the thermal control
loop.

> Some of them could be optical modules, providing untrusted
> temperature measurement, which could impact thermal
> control of the system.

Why would you not trust it? Are you saying some modules simply have
broken temperature sensors? Do you have a whitelist/blacklist of
modules?

> Also optical modules could be from the different vendors,  and
> this is real situation, when, f.e. one module has the warning and
> critical thresholds 75C and 85C, while another 70C and 80C.

But hwmon exports both the actual temperature and the alarm
temperatures. I would expect the thermal control code to use all this
information when making its decisions, not just the current
temperature.

> So, nominal temperature is not the case here, we should know the
> "worst" value for the thermal control decision.

What it sounds like to me is you are working around problems in the
thermal control by fudging the raw temperatures. That is the wrong
thing to do. hwmon should export the raw data, and you should fix the
thermal control code to use it correctly.

	Andrew