openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Question of phosphor-sel-logger
@ 2021-03-24 11:28 Duke Du (杜祥嘉)
  2021-03-24 13:20 ` Matt Spinler
  0 siblings, 1 reply; 6+ messages in thread
From: Duke Du (杜祥嘉) @ 2021-03-24 11:28 UTC (permalink / raw)
  To: openbmc
  Cc: vernon.mauery, jason.m.bills, Fran Hsu (徐誌謙),
	George Hung (洪忠敬)

Hi all,

     I used package phosphor-hwmon and phospor-sel-logger to monitor sensor and create log when sensor reading
   cross the threshold. I found after the commit 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger 
   would catch the signal "ThresholdAsserted" to create sensor threhold log in the journal, but the phosphor-hwmon 
   would not send the signal "ThresholdAsserted" when sensor reading is abnormal so that phosphor-sel-logger
   would not create the sensor threhold log, am I right ?

   If I'm right, can you give me some suggestion to fix this side effect, or what setting I have lost in the
   phosphor-hwmon or phosphor-sel-logger ? 

   phosphor-sel-logger commit 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
   https://github.com/openbmc/phosphor-sel-logger/commit/25b26e162bd109b51aa09b16f26f9aa3d9d940fa

   Thanks very much !
   Duke

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Question of phosphor-sel-logger
  2021-03-24 11:28 Question of phosphor-sel-logger Duke Du (杜祥嘉)
@ 2021-03-24 13:20 ` Matt Spinler
  2021-03-24 16:07   ` rgrs
  2021-03-25  8:22   ` Duke Du (杜祥嘉)
  0 siblings, 2 replies; 6+ messages in thread
From: Matt Spinler @ 2021-03-24 13:20 UTC (permalink / raw)
  To: Duke Du (杜祥嘉), openbmc
  Cc: vernon.mauery, jason.m.bills, Fran Hsu (徐誌謙),
	George Hung (洪忠敬)



On 3/24/2021 6:28 AM, Duke Du (杜祥嘉) wrote:
> Hi all,
>
>       I used package phosphor-hwmon and phospor-sel-logger to monitor sensor and create log when sensor reading
>     cross the threshold. I found after the commit 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger
>     would catch the signal "ThresholdAsserted" to create sensor threhold log in the journal, but the phosphor-hwmon
>     would not send the signal "ThresholdAsserted" when sensor reading is abnormal so that phosphor-sel-logger
>     would not create the sensor threhold log, am I right ?
>
>     If I'm right, can you give me some suggestion to fix this side effect, or what setting I have lost in the
>     phosphor-hwmon or phosphor-sel-logger ?

Hi,
That signal isn't defined in phosphor-dbus-interfaces, so phosphor-hwmon 
cannot use it.  When I tried to add it, it was rejected as-is with a 
recommendation to break it up into separate signals for each alarm 
property on each interface.  At that point I gave up and had the code I 
was working on at the time just look at propertiesChanged signals instead.

If you would like to take that up it would entail:
* Update 
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/39899 
as requested
* Update phosphor-hwmon to emit the new signals
* Update phosphor-sel-logger to also listen for these new signals in 
addition to the  current one, or change the dbus-sensors code to only 
emit the new signals.


>     phosphor-sel-logger commit 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
>     https://github.com/openbmc/phosphor-sel-logger/commit/25b26e162bd109b51aa09b16f26f9aa3d9d940fa
>
>     Thanks very much !
>     Duke


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Question of phosphor-sel-logger
  2021-03-24 13:20 ` Matt Spinler
@ 2021-03-24 16:07   ` rgrs
  2021-03-25  8:22   ` Duke Du (杜祥嘉)
  1 sibling, 0 replies; 6+ messages in thread
From: rgrs @ 2021-03-24 16:07 UTC (permalink / raw)
  To: Matt Spinler
  Cc: Duke Du, Fran Hsu, vernon.mauery, openbmc, jason.m.bills, George Hung

Hi,

On similar lines, phosphor-sel-logger doesn't add callout to logging entries.

How are the SEL entries parsed by phosphor-ipmi-hostd without respective sensor, inventory path?

Thanks,


‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, 24 March 2021 18:50, Matt Spinler <mspinler@linux.ibm.com> wrote:

> On 3/24/2021 6:28 AM, Duke Du (杜祥嘉) wrote:
>
> > Hi all,
> >
> >       I used package phosphor-hwmon and phospor-sel-logger to monitor sensor and create log when sensor reading
> >     cross the threshold. I found after the commit 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger
> >     would catch the signal "ThresholdAsserted" to create sensor threhold log in the journal, but the phosphor-hwmon
> >     would not send the signal "ThresholdAsserted" when sensor reading is abnormal so that phosphor-sel-logger
> >     would not create the sensor threhold log, am I right ?
> >
> >     If I'm right, can you give me some suggestion to fix this side effect, or what setting I have lost in the
> >     phosphor-hwmon or phosphor-sel-logger ?
> >
>
> Hi,
> That signal isn't defined in phosphor-dbus-interfaces, so phosphor-hwmon
> cannot use it.  When I tried to add it, it was rejected as-is with a
> recommendation to break it up into separate signals for each alarm
> property on each interface.  At that point I gave up and had the code I
> was working on at the time just look at propertiesChanged signals instead.
>
> If you would like to take that up it would entail:
>
> -   Update
>     https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/39899
>     as requested
>
> -   Update phosphor-hwmon to emit the new signals
> -   Update phosphor-sel-logger to also listen for these new signals in
>     addition to the  current one, or change the dbus-sensors code to only
>     emit the new signals.
>
>
> >     phosphor-sel-logger commit 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
> >     https://github.com/openbmc/phosphor-sel-logger/commit/25b26e162bd109b51aa09b16f26f9aa3d9d940fa
> >
> >     Thanks very much !
> >     Duke
> >



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: Question of phosphor-sel-logger
  2021-03-24 13:20 ` Matt Spinler
  2021-03-24 16:07   ` rgrs
@ 2021-03-25  8:22   ` Duke Du (杜祥嘉)
  2021-03-25 14:25     ` Matt Spinler
  1 sibling, 1 reply; 6+ messages in thread
From: Duke Du (杜祥嘉) @ 2021-03-25  8:22 UTC (permalink / raw)
  To: Matt Spinler, openbmc
  Cc: vernon.mauery, jason.m.bills, Fran Hsu (徐誌謙),
	George Hung (洪忠敬)

> -----Original Message-----
> From: Matt Spinler <mspinler@linux.ibm.com>
> Sent: Wednesday, March 24, 2021 9:20 PM
> To: Duke Du (杜祥嘉) <Duke.Du@quantatw.com>; openbmc@lists.ozlabs.org
> Cc: vernon.mauery@linux.intel.com; jason.m.bills@linux.intel.com; Fran Hsu
> (徐誌謙) <Fran.Hsu@quantatw.com>; George Hung (洪忠敬)
> <George.Hung@quantatw.com>
> Subject: Re: Question of phosphor-sel-logger
> 
> 
> 
> On 3/24/2021 6:28 AM, Duke Du (杜祥嘉) wrote:
> > Hi all,
> >
> >       I used package phosphor-hwmon and phospor-sel-logger to
> monitor sensor and create log when sensor reading
> >     cross the threshold. I found after the commit
> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger
> >     would catch the signal "ThresholdAsserted" to create sensor threhold
> log in the journal, but the phosphor-hwmon
> >     would not send the signal "ThresholdAsserted" when sensor reading
> is abnormal so that phosphor-sel-logger
> >     would not create the sensor threhold log, am I right ?
> >
> >     If I'm right, can you give me some suggestion to fix this side effect, or
> what setting I have lost in the
> >     phosphor-hwmon or phosphor-sel-logger ?
> 
> Hi,
> That signal isn't defined in phosphor-dbus-interfaces, so phosphor-hwmon
> cannot use it.  When I tried to add it, it was rejected as-is with a
> recommendation to break it up into separate signals for each alarm property
> on each interface.  At that point I gave up and had the code I was working
> on at the time just look at propertiesChanged signals instead.
> 
> If you would like to take that up it would entail:
> * Update
> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgerrit.
> openbmc-project.xyz%2Fc%2Fopenbmc%2Fphosphor-dbus-interfaces%2F%2B
> %2F39899&amp;data=04%7C01%7CDuke.Du%40quantatw.com%7Cc5bf4d3d1
> 6f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C1%7
> C0%7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp
> ;sdata=XoY4nKa3Go%2F9jt2coyzOcnXNrcMaw6XUtqnmK57k0ds%3D&amp;res
> erved=0
> as requested
> * Update phosphor-hwmon to emit the new signals
> * Update phosphor-sel-logger to also listen for these new signals in addition
> to the  current one, or change the dbus-sensors code to only emit the new
> signals.
> 
> 

Hi Matt,

   Thanks for your reply, I want to add a event monitor to listen "signal PropertyChanged" for 
   "phosphor-phosphor-hwmon" only, like watchdog event monitor 
   (https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-sel-logger/+/37774), 
   I think this is a simple way to fix this side effect, what do you think about my thought ?
   
   Please feel free to give me some suggestion, thanks very much !

Thanks
Duke 

> >     phosphor-sel-logger commit
> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
> >
> > https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> >
> ub.com%2Fopenbmc%2Fphosphor-sel-logger%2Fcommit%2F25b26e162bd10
> 9b51aa0
> >
> 9b16f26f9aa3d9d940fa&amp;data=04%7C01%7CDuke.Du%40quantatw.com%
> 7Cc5bf4
> >
> d3d16f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C
> 1%7C0
> >
> %7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQ
> >
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=rHxKqIk
> Vg7
> > yQXmqvjXal7I6eVBzw3ifl26gsZF8o4xo%3D&amp;reserved=0
> >
> >     Thanks very much !
> >     Duke


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Question of phosphor-sel-logger
  2021-03-25  8:22   ` Duke Du (杜祥嘉)
@ 2021-03-25 14:25     ` Matt Spinler
  2021-03-25 17:30       ` Bills, Jason M
  0 siblings, 1 reply; 6+ messages in thread
From: Matt Spinler @ 2021-03-25 14:25 UTC (permalink / raw)
  To: Duke Du (杜祥嘉), openbmc
  Cc: vernon.mauery, jason.m.bills, Fran Hsu (徐誌謙),
	George Hung (洪忠敬)



On 3/25/2021 3:22 AM, Duke Du (杜祥嘉) wrote:
>> -----Original Message-----
>> From: Matt Spinler <mspinler@linux.ibm.com>
>> Sent: Wednesday, March 24, 2021 9:20 PM
>> To: Duke Du (杜祥嘉) <Duke.Du@quantatw.com>; openbmc@lists.ozlabs.org
>> Cc: vernon.mauery@linux.intel.com; jason.m.bills@linux.intel.com; Fran Hsu
>> (徐誌謙) <Fran.Hsu@quantatw.com>; George Hung (洪忠敬)
>> <George.Hung@quantatw.com>
>> Subject: Re: Question of phosphor-sel-logger
>>
>>
>>
>> On 3/24/2021 6:28 AM, Duke Du (杜祥嘉) wrote:
>>> Hi all,
>>>
>>>        I used package phosphor-hwmon and phospor-sel-logger to
>> monitor sensor and create log when sensor reading
>>>      cross the threshold. I found after the commit
>> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger
>>>      would catch the signal "ThresholdAsserted" to create sensor threhold
>> log in the journal, but the phosphor-hwmon
>>>      would not send the signal "ThresholdAsserted" when sensor reading
>> is abnormal so that phosphor-sel-logger
>>>      would not create the sensor threhold log, am I right ?
>>>
>>>      If I'm right, can you give me some suggestion to fix this side effect, or
>> what setting I have lost in the
>>>      phosphor-hwmon or phosphor-sel-logger ?
>> Hi,
>> That signal isn't defined in phosphor-dbus-interfaces, so phosphor-hwmon
>> cannot use it.  When I tried to add it, it was rejected as-is with a
>> recommendation to break it up into separate signals for each alarm property
>> on each interface.  At that point I gave up and had the code I was working
>> on at the time just look at propertiesChanged signals instead.
>>
>> If you would like to take that up it would entail:
>> * Update
>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgerrit.
>> openbmc-project.xyz%2Fc%2Fopenbmc%2Fphosphor-dbus-interfaces%2F%2B
>> %2F39899&amp;data=04%7C01%7CDuke.Du%40quantatw.com%7Cc5bf4d3d1
>> 6f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C1%7
>> C0%7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
>> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp
>> ;sdata=XoY4nKa3Go%2F9jt2coyzOcnXNrcMaw6XUtqnmK57k0ds%3D&amp;res
>> erved=0
>> as requested
>> * Update phosphor-hwmon to emit the new signals
>> * Update phosphor-sel-logger to also listen for these new signals in addition
>> to the  current one, or change the dbus-sensors code to only emit the new
>> signals.
>>
>>
> Hi Matt,
>
>     Thanks for your reply, I want to add a event monitor to listen "signal PropertyChanged" for
>     "phosphor-phosphor-hwmon" only, like watchdog event monitor
>     (https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-sel-logger/+/37774),
>     I think this is a simple way to fix this side effect, what do you think about my thought ?
>     
>     Please feel free to give me some suggestion, thanks very much !

I don't really know the best way to go here other than what I already 
suggested.  sel-logger used to look at PropertiesChanged, but was 
changed to use ThresholdAsserted I think so that it could capture the 
sensor value within the signal.  But applications that need 
phosphor-dbus-interfaces bindings for their operations, like 
phosphor-hwmon, can't use ThresholdAsserted because it failed the PDI 
review.

Jason or Vernon, what do you suggest?


>
> Thanks
> Duke
>
>>>      phosphor-sel-logger commit
>> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
>>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>>>
>> ub.com%2Fopenbmc%2Fphosphor-sel-logger%2Fcommit%2F25b26e162bd10
>> 9b51aa0
>> 9b16f26f9aa3d9d940fa&amp;data=04%7C01%7CDuke.Du%40quantatw.com%
>> 7Cc5bf4
>> d3d16f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C
>> 1%7C0
>> %7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>> wMDAiLCJQ
>> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=rHxKqIk
>> Vg7
>>> yQXmqvjXal7I6eVBzw3ifl26gsZF8o4xo%3D&amp;reserved=0
>>>
>>>      Thanks very much !
>>>      Duke


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Question of phosphor-sel-logger
  2021-03-25 14:25     ` Matt Spinler
@ 2021-03-25 17:30       ` Bills, Jason M
  0 siblings, 0 replies; 6+ messages in thread
From: Bills, Jason M @ 2021-03-25 17:30 UTC (permalink / raw)
  To: openbmc



On 3/25/2021 7:25 AM, Matt Spinler wrote:
> 
> 
> On 3/25/2021 3:22 AM, Duke Du (杜祥嘉) wrote:
>>> -----Original Message-----
>>> From: Matt Spinler <mspinler@linux.ibm.com>
>>> Sent: Wednesday, March 24, 2021 9:20 PM
>>> To: Duke Du (杜祥嘉) <Duke.Du@quantatw.com>; openbmc@lists.ozlabs.org
>>> Cc: vernon.mauery@linux.intel.com; jason.m.bills@linux.intel.com; 
>>> Fran Hsu
>>> (徐誌謙) <Fran.Hsu@quantatw.com>; George Hung (洪忠敬)
>>> <George.Hung@quantatw.com>
>>> Subject: Re: Question of phosphor-sel-logger
>>>
>>>
>>>
>>> On 3/24/2021 6:28 AM, Duke Du (杜祥嘉) wrote:
>>>> Hi all,
>>>>
>>>>        I used package phosphor-hwmon and phospor-sel-logger to
>>> monitor sensor and create log when sensor reading
>>>>      cross the threshold. I found after the commit
>>> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger
>>>>      would catch the signal "ThresholdAsserted" to create sensor 
>>>> threhold
>>> log in the journal, but the phosphor-hwmon
>>>>      would not send the signal "ThresholdAsserted" when sensor reading
>>> is abnormal so that phosphor-sel-logger
>>>>      would not create the sensor threhold log, am I right ?
>>>>
>>>>      If I'm right, can you give me some suggestion to fix this side 
>>>> effect, or
>>> what setting I have lost in the
>>>>      phosphor-hwmon or phosphor-sel-logger ?
>>> Hi,
>>> That signal isn't defined in phosphor-dbus-interfaces, so phosphor-hwmon
>>> cannot use it.  When I tried to add it, it was rejected as-is with a
>>> recommendation to break it up into separate signals for each alarm 
>>> property
>>> on each interface.  At that point I gave up and had the code I was 
>>> working
>>> on at the time just look at propertiesChanged signals instead.
>>>
>>> If you would like to take that up it would entail:
>>> * Update
>>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgerrit. 
>>>
>>> openbmc-project.xyz%2Fc%2Fopenbmc%2Fphosphor-dbus-interfaces%2F%2B
>>> %2F39899&amp;data=04%7C01%7CDuke.Du%40quantatw.com%7Cc5bf4d3d1
>>> 6f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C1%7
>>> C0%7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
>>> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp
>>> ;sdata=XoY4nKa3Go%2F9jt2coyzOcnXNrcMaw6XUtqnmK57k0ds%3D&amp;res
>>> erved=0
>>> as requested
>>> * Update phosphor-hwmon to emit the new signals
>>> * Update phosphor-sel-logger to also listen for these new signals in 
>>> addition
>>> to the  current one, or change the dbus-sensors code to only emit the 
>>> new
>>> signals.
>>>
>>>
>> Hi Matt,
>>
>>     Thanks for your reply, I want to add a event monitor to listen 
>> "signal PropertyChanged" for
>>     "phosphor-phosphor-hwmon" only, like watchdog event monitor
>>     
>> (https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-sel-logger/+/37774), 
>>
>>     I think this is a simple way to fix this side effect, what do you 
>> think about my thought ?
>>     Please feel free to give me some suggestion, thanks very much !
> 
> I don't really know the best way to go here other than what I already 
> suggested.  sel-logger used to look at PropertiesChanged, but was 
> changed to use ThresholdAsserted I think so that it could capture the 
> sensor value within the signal.  But applications that need 
> phosphor-dbus-interfaces bindings for their operations, like 
> phosphor-hwmon, can't use ThresholdAsserted because it failed the PDI 
> review.
> 
> Jason or Vernon, what do you suggest?
With just PropertiesChanged we were getting events but when we read the 
current sensor value it had changed, so we were getting cases where an 
event showed a threshold was crossed but the value in that event was 
below the threshold.

As Matt noted, we attempted to fix this issue by creating the 
ThresholdAsserted signal so the value that triggered the threshold event 
could be sent with it and logged correctly.

I think the best options are
1. Find a new solution to the original problem where we can report 
accurate threshold crossed value with just PropertiesChanged.
2. Pursue Matt's proposed change to phosphor-dbus-interfaces to get the 
signal officially defined to include the sensor value in the signal.

Thanks,
-Jason

> 
> 
>>
>> Thanks
>> Duke
>>
>>>>      phosphor-sel-logger commit
>>> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
>>>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>>>>
>>> ub.com%2Fopenbmc%2Fphosphor-sel-logger%2Fcommit%2F25b26e162bd10
>>> 9b51aa0
>>> 9b16f26f9aa3d9d940fa&amp;data=04%7C01%7CDuke.Du%40quantatw.com%
>>> 7Cc5bf4
>>> d3d16f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C
>>> 1%7C0
>>> %7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>>> wMDAiLCJQ
>>> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=rHxKqIk
>>> Vg7
>>>> yQXmqvjXal7I6eVBzw3ifl26gsZF8o4xo%3D&amp;reserved=0
>>>>
>>>>      Thanks very much !
>>>>      Duke
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-25 17:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 11:28 Question of phosphor-sel-logger Duke Du (杜祥嘉)
2021-03-24 13:20 ` Matt Spinler
2021-03-24 16:07   ` rgrs
2021-03-25  8:22   ` Duke Du (杜祥嘉)
2021-03-25 14:25     ` Matt Spinler
2021-03-25 17:30       ` Bills, Jason M

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