>First your mailer is munging up the patches pretty badly. I have to >hand apply them, which is a big pain.
Ah, apologies, I didn't check the formatting before I sent these emails (was rushing a bit, won't happen again). > If you could check it, that would be great. Tested, everything seems to be working fine. That said, there's one tiny thing I am a bit torn about: sel_data[4] = msg->saddr & 0x7f; For the case where the event is not received over the system interface, e.g., LAN channel, the msg->saddr is set to 0x81. So the above sets the Byte 1 of the generator ID to 1. With 0x1 LAN channel, we end up with 0x1001 as the generator ID for that event. So that indicates the generator holds an actual SWID of value 0x0. If we cross-reference that with the Table 5-4, System Software IDs it essentially says "BIOS generated this message" which sounds a bit odd. Initially, having 0x1000 made somewhat more sense to me as the way I read this was "no swid, the i2c address is 0h but the channel number is != 0h - this is definitely not a system software event, but one which still fits under IPMB messaging umbrella (i.e. LAN, as per Table 29-4, Platform Event (Event Message) Command)". That said, I don't feel very strongly about it, I am just more curious what the "right" interpretation of the spec should be here. Regardless, code LGTM. :) Regards, Michal pon., 29 gru 2025 o 01:24 Corey Minyard <[email protected]> napisał(a): > > On Sun, Dec 28, 2025 at 10:38:03PM +0100, Michal Berger wrote: > > Right now channel number is being unconditionally included in the > > Generator ID field. However, as per the SEL Event Records it should > > remain 0h depending on what medium/interface the event msg is > > received over. > > You are correct, there was a lot to be desired in this funciton. > > There were, unfortunately, a lot of issues with this patch. > > First your mailer is munging up the patches pretty badly. I have to > hand apply them, which is a big pain. > > On the chagne itself, you still weren't handling all cases for the > address in the SEL Event Records. > > And for an IPMB messages, the wrong data was copied and it was indexing > beyond the end of the message data when filling out the data. > > I've heavily modified the patch and pushed it up. If you could check > it, that would be great. > > Thanks, > > -corey > > > > > So have this: > > > > # ipmitool sel list > > SEL has no entries > > # ipmitool event 1 > > Sending SAMPLE event: Temperature - Upper Critical - Going High > > # ipmitool sel get 0x1 | grep "Generator ID" > > Generator ID : 0041 > > > > Instead of: > > > > # ipmitool event 1 > > Sending SAMPLE event: Temperature - Upper Critical - Going High > > # ipmitool sel get 0x2 | grep "Generator ID" > > Generator ID : f041 > > > > As we are at it adjust the msg length of the platform event - as per the > > Table 29-4, Platform Event (Event Message) Command, the 8 bytes is > > a msg length dedicated for the System Interface which must include > > the Generator ID. Case in point, when the event is sent over the > > LAN channel, ipmi_sim rejects it due to invalid length of the > > request (e.g. ipmitool does not include the extra software ID in the > > event data hence sending only 7 bytes): > > > > $ ipmitool -Ilanplus -Hlocalhost -p4242 -UXXXXX -PXXXXX event 1 > > Sending SAMPLE event: Temperature - Upper Critical - Going High > > Platform Event Message command failed: Request data length invalid > > > > Signed-off-by: Michal Berger <[email protected]> > > --- > > lanserv/bmc_sensor.c | 22 +++++++++++++++++++++- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/lanserv/bmc_sensor.c b/lanserv/bmc_sensor.c > > index be6c537f..cc21bf9c 100644 > > --- a/lanserv/bmc_sensor.c > > +++ b/lanserv/bmc_sensor.c > > @@ -58,6 +58,7 @@ > > #include <sys/stat.h> > > #include <fcntl.h> > > > > +#include <OpenIPMI/ipmi_mc.h> > > #include <OpenIPMI/ipmi_err.h> > > #include <OpenIPMI/ipmi_msgbits.h> > > #include <OpenIPMI/ipmi_bits.h> > > @@ -113,16 +114,35 @@ handle_platform_event(lmc_data_t *mc, > > void *cb_data) > > { > > unsigned char sel_data[13]; > > + uint8_t msg_length = 7; // IPMB MESSAGING > > > > - if (check_msg_length(msg, 8, rdata, rdata_len)) > > + if (msg->orig_channel->channel_num == 15 || > > + msg->orig_channel->medium_type == > > IPMI_CHANNEL_MEDIUM_SYS_INTF) > > + msg_length = 8; // SYSTEM INTERFACE MESSAGING > > + > > + if (check_msg_length(msg, msg_length, rdata, rdata_len)) > > return; > > > > sel_data[0] = 0; > > sel_data[1] = 0; > > sel_data[2] = 0; > > sel_data[3] = 0; > > + /* > > + From Table 32-1, SEL Event Records: > > + Byte 1 > > + [7:1] - 7-bit I2C . Slave Address, or 7-bit system software ID > > + [0] 0b = ID is IPMB Slave Address > > + 1b = system software ID > > + Byte 2 > > + [7:4] - Channel number. Channel that event message was > > received over. 0h if the > > + event message was received via the system > > interface, primary IPMB, or > > + internally generated by the BMC. > > + */ > > sel_data[4] = msg->data[0]; > > sel_data[5] = msg->orig_channel->channel_num << 4; > > + if (msg->orig_channel->channel_num == 15 || > > + msg->orig_channel->medium_type == > > IPMI_CHANNEL_MEDIUM_SYS_INTF) > > + sel_data[5] = 0; > > sel_data[6] = msg->data[1]; > > sel_data[7] = msg->data[2]; > > sel_data[8] = msg->data[3]; > > -- > > 2.43.0 > > > > > > _______________________________________________ > > Openipmi-developer mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/openipmi-developer _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
