Hello, I'm running 13.1-RELEASE with ZFS on a Dell R340 with a "Dell HBA330 Adp (Embedded)" adapter (firmware version 16.17.01.00): chimay% sudo pciconf -lv mpr0 mpr0@pci0:2:0:0: class=0x010700 rev=0x02 hdr=0x00 vendor=0x1000 device=0x0097 subvendor=0x1028 subdevice=0x1f45 vendor = 'Broadcom / LSI' device = 'SAS3008 PCI-Express Fusion-MPT SAS-3' class = mass storage subclass = SAS It works well, but I noticed a lot of: mpr0: mpr_user_pass_thru: user reply buffer (64) smaller than returned buffer (68) in the kernel logs Is it an issue or is it harmless? Thanks
Created attachment 234887 [details] mprutil show all
It means that some userspace program is sending a command to the controller firmware, but provided too small a buffer for the response. The first step is to figure out which program (mprutil?) is triggering the problem.
Hi Mark, Thanks for replying, and you're right: mprutil is the culprit..!
(In reply to Julien Cigar from comment #3) Which subcommand is it that's triggering the warning?
show all, show adapters, ... (It's a new machine and I haven't used other subcommands yet)
Created attachment 235088 [details] proposed patch Patch for an issue pointed out by Scott: The problems is here in mps_get_iocfacts() snprintf(sysctlname, sizeof(sysctlname), "dev.%s.%d.msg_version", is_mps ? "mps" : "mpr", mps_unit); factslen = sizeof(MPI2_IOC_FACTS_REPLY); len = sizeof(msgver); error = sysctlbyname(sysctlname, msgver, &len, NULL, 0); if (error == 0) { if (strncmp(msgver, "2.6", sizeof(msgver)) == 0) factslen += 4; } I haven’t been keeping track, but I’ll guess that msgver has numerically advanced past “2.6” for this system.
Mark's guess is incorrect. I'm seeing this issue too, and msg_version is in fact 2.5. I can't find anywhere in the code that sizes a buffer based on msg_version. In fact, I can't find anywhere that even sets the MsgLength field, so I'm guessing that it gets set by the firmware. Unless somebody has accurate documentation for how the firmware handles MPI2_FUNCTION_IOC_FACTS, I think we should just unconditionally allocate sufficient space in mprutil for the larger reply. And we should do the same in mpr_get_iocfacts as well. BTW, this is the commit that originally added an extra four bytes to the reply buffer. https://github.com/freebsd/freebsd-src/commit/69e85eb8ae4919e0806bc2957cbc4a33f9138b54
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=7d154c4dc64e61af7ca536c4e9927fa07c675a83 commit 7d154c4dc64e61af7ca536c4e9927fa07c675a83 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2023-02-22 22:06:43 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2023-09-21 14:38:24 +0000 mprutil: "fix user reply buffer (64)..." warnings Depending on the card's firmware version, it may return different length responses for MPI2_FUNCTION_IOC_FACTS. But the first part of the response contains the length of the rest, so query it first to get the length and then use that to size the buffer for the full response. Also, correctly zero-initialize MPI2_IOC_FACTS_REQUEST. It only worked by luck before. PR: 264848 Reported by: Julien Cigar <julien@perdition.city> MFC after: 1 week Sponsored by: Axcient Reviewed by: scottl, imp Differential Revision: https://reviews.freebsd.org/D38739 sys/dev/mpr/mpr.c | 2 +- sys/dev/mpr/mpr_user.c | 4 ++-- sys/dev/mps/mps.c | 2 +- sys/dev/mps/mps_user.c | 4 ++-- usr.sbin/mpsutil/mps_cmd.c | 40 +++++++++++++++++++++++++++------------- 5 files changed, 33 insertions(+), 19 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=c70a4185c637754428f36536af3bdc9181fa5155 commit c70a4185c637754428f36536af3bdc9181fa5155 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2023-02-22 22:06:43 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2023-10-03 01:31:12 +0000 mprutil: "fix user reply buffer (64)..." warnings Depending on the card's firmware version, it may return different length responses for MPI2_FUNCTION_IOC_FACTS. But the first part of the response contains the length of the rest, so query it first to get the length and then use that to size the buffer for the full response. Also, correctly zero-initialize MPI2_IOC_FACTS_REQUEST. It only worked by luck before. PR: 264848 Reported by: Julien Cigar <julien@perdition.city> Sponsored by: Axcient Reviewed by: scottl, imp Differential Revision: https://reviews.freebsd.org/D38739 (cherry picked from commit 7d154c4dc64e61af7ca536c4e9927fa07c675a83) sys/dev/mpr/mpr.c | 2 +- sys/dev/mpr/mpr_user.c | 4 ++-- sys/dev/mps/mps.c | 2 +- sys/dev/mps/mps_user.c | 4 ++-- usr.sbin/mpsutil/mps_cmd.c | 40 +++++++++++++++++++++++++++------------- 5 files changed, 33 insertions(+), 19 deletions(-)
Can somebody push this commit to 12-STABLE, please? I have a bunch of LSI3008 controller and also see this warning sometime. I'm scared of "It only worked by luck before". I'll plan to stay on 12-STABLE till 14-REALLY-STABLE.
(In reply to ant2 from comment #10) Yes, I'll merge it there, too.
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=51426ad9c75b21424a05208a0c3d122beb08ddd7 commit 51426ad9c75b21424a05208a0c3d122beb08ddd7 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2023-02-22 22:06:43 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2023-10-05 17:35:27 +0000 mprutil: "fix user reply buffer (64)..." warnings Depending on the card's firmware version, it may return different length responses for MPI2_FUNCTION_IOC_FACTS. But the first part of the response contains the length of the rest, so query it first to get the length and then use that to size the buffer for the full response. Also, correctly zero-initialize MPI2_IOC_FACTS_REQUEST. It only worked by luck before. PR: 264848 Reported by: Julien Cigar <julien@perdition.city> Sponsored by: Axcient Reviewed by: scottl, imp Differential Revision: https://reviews.freebsd.org/D38739 (cherry picked from commit 7d154c4dc64e61af7ca536c4e9927fa07c675a83) sys/dev/mpr/mpr.c | 2 +- sys/dev/mpr/mpr_user.c | 4 ++-- sys/dev/mps/mps.c | 2 +- sys/dev/mps/mps_user.c | 4 ++-- usr.sbin/mpsutil/mps_cmd.c | 40 +++++++++++++++++++++++++++------------- 5 files changed, 33 insertions(+), 19 deletions(-)
(In reply to Alan Somers from comment #11) Thank you very much!
(In reply to ant2 from comment #13) Don't get too excited, ant2! I haven't merged to stable/12 yet. I need to update my VM first.
(In reply to Alan Somers from comment #14) Thanks in advance is a kind of manipulation to get things done. P.S. I trust in you.
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=73765a5fa7d0daa72387d41661c36922f7860bcb commit 73765a5fa7d0daa72387d41661c36922f7860bcb Author: Scott Long <scottl@FreeBSD.org> AuthorDate: 2020-02-07 12:15:39 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2023-10-05 20:36:00 +0000 Advertise the MPI Message Version that's contained in the IOCFacts message in the sysctl block for the driver. mpsutil/mprutil needs this so it can know how big of a buffer to allocate when requesting the IOCFacts from the controller. This eliminates the kernel console messages about wrong allocation sizes. Reported by: imp (cherry picked from commit 69e85eb8ae4919e0806bc2957cbc4a33f9138b54) mprutil: "fix user reply buffer (64)..." warnings Depending on the card's firmware version, it may return different length responses for MPI2_FUNCTION_IOC_FACTS. But the first part of the response contains the length of the rest, so query it first to get the length and then use that to size the buffer for the full response. Also, correctly zero-initialize MPI2_IOC_FACTS_REQUEST. It only worked by luck before. PR: 264848 Reported by: Julien Cigar <julien@perdition.city> Sponsored by: Axcient Reviewed by: scottl, imp Differential Revision: https://reviews.freebsd.org/D38739 (cherry picked from commit 7d154c4dc64e61af7ca536c4e9927fa07c675a83) sys/dev/mpr/mpr.c | 14 ++++++++++++-- sys/dev/mpr/mpr_user.c | 6 +++--- sys/dev/mpr/mprvar.h | 1 + sys/dev/mps/mps.c | 14 ++++++++++++-- sys/dev/mps/mps_user.c | 6 +++--- sys/dev/mps/mpsvar.h | 1 + usr.sbin/mpsutil/mps_cmd.c | 35 +++++++++++++++++++++++++++++++---- 7 files changed, 63 insertions(+), 14 deletions(-)