vermaden / scripts

Various scripts I wrote when using FreeBSD/Linux/UNIX systems for 15+ years.
https://vermaden.wordpress.com
148 stars 26 forks source link

[FIXED] Please update sensors.sh for new NVMe / NVD / NDA devices #13

Open thesunexpress opened 11 months ago

thesunexpress commented 11 months ago

Hello there,

The FreeBSD devs have decided, in their infinite wisdom, that NVMe devices should now be considered NDA devices by default: https://man.freebsd.org/cgi/man.cgi?nda

This is instead of the previous standard of nvme / nvd: https://man.freebsd.org/cgi/man.cgi?nvme

As the manpages reflect, one can mess with sysctl knobs to force nda devices to report as nvme / nvd instead, but this is non-standard now. I am not sure if/when smartmontools devs will realize this change. Currently, I have your sensors.sh script hacked up, in a dirty way, to report temps from nvmecontrol -- which is not ideal. The fact that a piece of software, designed specifically for nvme devices, is now co-opted to do so for nda devices is one seriously proper operation to confuse & mislead everybody. I haven't a clue why it was decided this is the correct course of action... there must be some seriously eccentric folks writing FreeBSD base code. I am not experienced enough with shell scripting (zsh in particular), or the internal workings of smartmontools, or rather 'smartctl', to figure out how to make it report info of nda devices. But if you can figure it out, it would future-proof your excellent script.

vermaden commented 11 months ago

Hi and thank You for letting me know.

Do You thing this is the change that will help here?

-    # THE nvd(4) AND nvme(4) DEVICES NEED SPECIAL HANDLING
-    (nvd*)
-      I=$( echo ${I} | sed -e 's/nvd/nvme/g' )

+    # THE nvd(4)/nda(4)/nvme(4) DEVICES NEED SPECIAL HANDLING
+    (nvd*|nda*)
+      I=$( echo ${I} | sed -e 's/nvd/nvme/g' -e 's/nda/nvme/g' )

I do not have access to hardware with nda(4) devices so I can not test anything :(

Regards, vermaden

vermaden commented 11 months ago

... or this one instead:

-    # THE nvd(4) AND nvme(4) DEVICES NEED SPECIAL HANDLING
-    (nvd*)

+    # THE nvd(4)/nda(4)/nvme(4) DEVICES NEED SPECIAL HANDLING
+    (nvd*|nda*)
thesunexpress commented 11 months ago

Hi and thank You for letting me know.

Do You thing this is the change that will help here?

-    # THE nvd(4) AND nvme(4) DEVICES NEED SPECIAL HANDLING
-    (nvd*)
-      I=$( echo ${I} | sed -e 's/nvd/nvme/g' )

+    # THE nvd(4)/nda(4)/nvme(4) DEVICES NEED SPECIAL HANDLING
+    (nvd*|nda*)
+      I=$( echo ${I} | sed -e 's/nvd/nvme/g' -e 's/nda/nvme/g' )

I do not have access to hardware with nda(4) devices so I can not test anything :(

Regards, vermaden

This sort of works, but the temperature is not the same as reported by nvmecontrol -- the difference is off by 1 to 8 degrees at least. I am not sure how smartctl goes about its business gathering stats. I suspect their nda implementation is not fully baked just yet. Any FreeBSD-14 install on an NVMe SSD should now result in nda device enumeration. At least the last 7 installs I have done are like this, perhaps with the caveat that I only use -STABLE branch.

Note: Only adding (nvd*|nda*) is not enough. You definitely need the second line I=$( echo ${I} | sed -e 's/nvd/nvme/g' -e 's/nda/nvme/g' ) as well. I had in fact tried just adding nda* in my own dirty hack, but that didn't work. So as a quick work-around, I used nvmecontrol because I knew that for sure would do the trick.

I will keep an eye on both smartctl & nvmecontrol to see if I can find the reason for the discrepancy.

Attached is a screenshot of my laptop's output, as evidence your suggested changes work: sensors sh output

vermaden commented 11 months ago

Thank You for testing.

Added both in sensors and scripts repos:

vermaden commented 11 months ago

I assume it works OK now?