needrestart icon indicating copy to clipboard operation
needrestart copied to clipboard

Fix AMD Microcode Check

Open fritz-fritz opened this issue 1 year ago • 8 comments

As discussed in #249, the default behavior for AMD was changed with pull request #226 and as a result, CPUs that do not have microcode patches (because they're up to date) were leading to NRM_UNKNOWN resulting in "Failed to check for processor microcode upgrades." which is inaccurate.

This sets the $vars{AVAIL} to 0 if there are microcode patches to scan and none apply. While this doesn't guarantee microcode is up to date or from a valid source, it restores the previous behavior while accounting for the edge case presented in the previous pull request.

fritz-fritz avatar Oct 29 '23 13:10 fritz-fritz

This should address some more of #274 but also might want to look into expected behavior for Intel as well?

fritz-fritz avatar Oct 29 '23 13:10 fritz-fritz

@liske would be great to have some feedback

fritz-fritz avatar Nov 13 '23 13:11 fritz-fritz

@fritz-fritz why is this marked as draft, do you plan to get anything else done on this PR? otherwise i suggest just marking it as ready. i'll test this on our fleet now.

anarcat avatar Nov 15 '23 19:11 anarcat

@fritz-fritz why is this marked as draft

@anarcat mostly just to try and encourage feedback before merging. I believe I have implemented the expected behavior here but am willing to rework it if need be. When all looks good I’ll be happy to finalize the PR

fritz-fritz avatar Nov 15 '23 19:11 fritz-fritz

the patch doesn't fix the issue for us. before the patch:

root@tb-build-03:~# needrestart -p
UNKN - Kernel: 6.1.0-13-amd64, Microcode: unknown, Services: 1 (!), Containers: none, Sessions: 1 (!!)|Kernel=0;0;;0;2 Services=1;;0;0 Containers=0;;0;0 Sessions=1;0;;0
Services:
- systemd-logind.service
Sessions:
- redacted @ user manager service

after the patch:

root@tb-build-03:~# dpkg -i needrestart_3.6-4+deb12u1_all.deb 
(Reading database ... 110354 files and directories currently installed.)
Preparing to unpack needrestart_3.6-4+deb12u1_all.deb ...
Unpacking needrestart (3.6-4+deb12u1) over (3.6-4) ...
Setting up needrestart (3.6-4+deb12u1) ...
Processing triggers for man-db (2.11.2-2) ...
root@tb-build-03:~# needrestart -p
UNKN - Kernel: 6.1.0-13-amd64, Microcode: unknown, Services: 1 (!), Containers: none, Sessions: 1 (!!)|Kernel=0;0;;0;2 Services=1;;0;0 Containers=0;;0;0 Sessions=1;0;;0
Services:
- systemd-logind.service
Sessions:
- redacted @ user manager service

anarcat avatar Nov 15 '23 20:11 anarcat

@anarcat can you try it with -w -v I’d like to get better visibility at what it sees when ran. Also to note, this PR doesn’t fix the intel microcode checking on AMD I have that in a separate PR. So that could be the issue?

fritz-fritz avatar Nov 15 '23 20:11 fritz-fritz

-v always succeeds, that's the whole problem here. I think i found (and fixed) the root cause, try #288.

anarcat avatar Nov 15 '23 20:11 anarcat

edit: moved this comment into the issue thread instead, in https://github.com/liske/needrestart/issues/249#issuecomment-181326728

anarcat avatar Nov 15 '23 21:11 anarcat

i think this should be closed in favor of #288 and #290, and in any case would require a rebase.

anarcat avatar Jul 31 '24 06:07 anarcat

@anarcat I admittedly have been away from this code for a while, but a quick review of your PRs and I think I agree.

Closing this PR.

fritz-fritz avatar Aug 02 '24 07:08 fritz-fritz