On Mon, Dec 29, 2025 at 6:05 AM Michal Berger <[email protected]> wrote:
> >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. :) > I debated about this a little, and I didn't consider LAN channels properly. And the spec leaves a lot of room for creative interpretation. But I did mess this up. First, it's the low bit, not the high bit. Second, the source address on a LAN channel comes from the message header, like an IPMB message. But it can be a SWID, so masking the top bit is wrong. I'll remove that and setting the high bit on system interfaces. Thanks, -corey > 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
