It is preferred to have the point of view from upstream as well. Let's wait and see. 🙏
s3nt fr0m a $martph0ne, excuse typ0s On Tue, 28 Jan 2025, 16:17 Louis Sautier, <sautier.lo...@gmail.com> wrote: > On 28/01/2025 09:22, Ritesh Raj Sarraf wrote: > > Opening an issue is worthwhile. The reason is because emails may fall > under spam classification while opening an issue guarantees that it is > reported. > > s3nt fr0m a $martph0ne, excuse typ0s > > On Tue, 28 Jan 2025, 12:30 Louis Sautier, <sautier.lo...@gmail.com> wrote: > >> On 28/01/2025 06:44, Ritesh Raj Sarraf wrote: >> >> Have you reported it upstream ? That'd be the very first thing to do >> >> s3nt fr0m a $martph0ne, excuse typ0s >> >> On Tue, 28 Jan 2025, 02:21 Louis Sautier, <sautier.lo...@gmail.com> >> wrote: >> >>> Package: smp-utils >>> Version: 0.99-1 >>> >>> Hi, >>> smp_discover 1.62 20190124 from smp-utils 0.99-1 has a bug when the >>> -p/--phy options is used; -n/--num is also broken. >>> The main problem is that the -p option is handled incorrectly, e.g. >>> smp_discover -p 5 >>> not only starts at phy 5 but also skips the last 5 phys. >>> >>> For instance, on a system with 30 phys on expander-1:1: >>> # smp_discover -m /dev/bsg/expander-1:1 # This works fine and is only >>> shown to showcase the bugs >>> phy 0:S:attached:[500605b009574bd1:07 i(SSP+STP+SMP)] 6 Gbps >>> >>> phy 1:S:attached:[500605b009574bd1:06 i(SSP+STP+SMP)] 6 Gbps >>> phy 2:S:attached:[500605b009574bd1:05 i(SSP+STP+SMP)] 6 Gbps >>> phy 3:S:attached:[500605b009574bd1:04 i(SSP+STP+SMP)] 6 Gbps >>> phy 4:U:attached:[0000000000000000:00] >>> phy 5:U:attached:[0000000000000000:00] >>> phy 6:U:attached:[0000000000000000:00] >>> phy 7:U:attached:[0000000000000000:00] >>> phy 8:D:disabled >>> phy 9:D:disabled >>> phy 10:D:disabled >>> phy 11:D:disabled >>> phy 12:U:attached:[5000cca23c0b38a5:00 t(SSP)] 6 Gbps >>> phy 13:U:attached:[5000cca23c0e74fd:00 t(SSP)] 6 Gbps >>> phy 14:U:attached:[5000cca098215285:00 t(SSP)] 6 Gbps >>> phy 15:U:attached:[5000cca25552e5f9:00 t(SSP)] 6 Gbps >>> phy 16:U:attached:[5000cca2710aab6d:00 t(SSP)] 6 Gbps >>> phy 17:U:attached:[5000cca23c0e75b9:00 t(SSP)] 6 Gbps >>> phy 18:U:attached:[5000cca23c0ebaad:00 t(SSP)] 6 Gbps >>> phy 19:U:attached:[5000cca23c0f9fe5:00 t(SSP)] 6 Gbps >>> phy 20:U:attached:[5000cca232695239:00 t(SSP)] 6 Gbps >>> phy 21:U:attached:[5000cca23c0dd615:00 t(SSP)] 6 Gbps >>> phy 22:U:attached:[5000cca232717d11:00 t(SSP)] 6 Gbps >>> phy 23:U:attached:[5000cca23c10e5c9:00 t(SSP)] 6 Gbps >>> phy 24:D:disabled >>> phy 25:D:disabled >>> phy 26:D:disabled >>> phy 27:D:disabled >>> phy 28:D:attached:[5003048017874b3d:00 V i(SSP+SMP) t(SSP)] 6 Gbps >>> phy 29:D:attached:[0000000000000000:00] >>> # smp_discover -m /dev/bsg/expander-1:1 -p 10 # Not only are the first >>> 10 phys skipped, the last 10 are skipped too >>> phy 10:D:disabled >>> phy 11:D:disabled >>> phy 12:U:attached:[5000cca23c0b38a5:00 t(SSP)] 6 Gbps >>> phy 13:U:attached:[5000cca23c0e74fd:00 t(SSP)] 6 Gbps >>> phy 14:U:attached:[5000cca098215285:00 t(SSP)] 6 Gbps >>> phy 15:U:attached:[5000cca25552e5f9:00 t(SSP)] 6 Gbps >>> phy 16:U:attached:[5000cca2710aab6d:00 t(SSP)] 6 Gbps >>> phy 17:U:attached:[5000cca23c0e75b9:00 t(SSP)] 6 Gbps >>> phy 18:U:attached:[5000cca23c0ebaad:00 t(SSP)] 6 Gbps >>> phy 19:U:attached:[5000cca23c0f9fe5:00 t(SSP)] 6 Gbps >>> # smp_discover -m /dev/bsg/expander-1:1 -p 10 -n 12 # Instead of >>> displaying 12 phys starting with phy 10, it only displays phys leading >>> up to phy 12 >>> phy 10:D:disabled >>> phy 11:D:disabled >>> # smp_discover -m /dev/bsg/expander-1:1 -p 10 -n 1 # This returns nothing >>> >>> >>> On Debian 10 (I'm not showing the result on Debian 11 where smp-utils is >>> the same version), with smp_discover 1.46 20140526 from smp-utils 0.98-2: >>> # smp_discover -m /dev/bsg/expander-1:1 -p 10 >>> phy 10:D:disabled >>> phy 11:D:disabled >>> phy 12:U:attached:[5000cca23c0b38a5:00 t(SSP)] 6 Gbps >>> phy 13:U:attached:[5000cca23c0e74fd:00 t(SSP)] 6 Gbps >>> phy 14:U:attached:[5000cca098215285:00 t(SSP)] 6 Gbps >>> phy 15:U:attached:[5000cca25552e5f9:00 t(SSP)] 6 Gbps >>> phy 16:U:attached:[5000cca2710aab6d:00 t(SSP)] 6 Gbps >>> phy 17:U:attached:[5000cca23c0e75b9:00 t(SSP)] 6 Gbps >>> phy 18:U:attached:[5000cca23c0ebaad:00 t(SSP)] 6 Gbps >>> phy 19:U:attached:[5000cca23c0f9fe5:00 t(SSP)] 6 Gbps >>> phy 20:U:attached:[5000cca232695239:00 t(SSP)] 6 Gbps >>> phy 21:U:attached:[5000cca23c0dd615:00 t(SSP)] 6 Gbps >>> phy 22:U:attached:[5000cca232717d11:00 t(SSP)] 6 Gbps >>> phy 23:U:attached:[5000cca23c10e5c9:00 t(SSP)] 6 Gbps >>> phy 24:D:disabled >>> phy 25:D:disabled >>> phy 26:D:disabled >>> phy 27:D:disabled >>> phy 28:D:attached:[5003048017874b3d:00 V i(SSP+SMP) t(SSP)] 6 Gbps >>> phy 29:D:attached:[0000000000000000:00] >>> # smp_discover -m /dev/bsg/expander-1:1 -p 10 -n 12 >>> phy 10:D:disabled >>> phy 11:D:disabled >>> phy 12:U:attached:[5000cca23c0b38a5:00 t(SSP)] 6 Gbps >>> phy 13:U:attached:[5000cca23c0e74fd:00 t(SSP)] 6 Gbps >>> phy 14:U:attached:[5000cca098215285:00 t(SSP)] 6 Gbps >>> phy 15:U:attached:[5000cca25552e5f9:00 t(SSP)] 6 Gbps >>> phy 16:U:attached:[5000cca2710aab6d:00 t(SSP)] 6 Gbps >>> phy 17:U:attached:[5000cca23c0e75b9:00 t(SSP)] 6 Gbps >>> phy 18:U:attached:[5000cca23c0ebaad:00 t(SSP)] 6 Gbps >>> phy 19:U:attached:[5000cca23c0f9fe5:00 t(SSP)] 6 Gbps >>> phy 20:U:attached:[5000cca232695239:00 t(SSP)] 6 Gbps >>> phy 21:U:attached:[5000cca23c0dd615:00 t(SSP)] 6 Gbps >>> # smp_discover -m /dev/bsg/expander-1:1 -p 10 -n 1 >>> phy 10:D:disabled >>> >>> >>> This bug was introduced in git commit >>> 5e9dd3dbab9f7fc85e2816ab2eb62bc92b068cc1, specifically this block, >>> starting at line 923: >>> >>> https://github.com/doug-gilbert/smp_utils/commit/5e9dd3dbab9f7fc85e2816ab2eb62bc92b068cc1#diff-853a65985ccfe20502945ddfbc340cbedde6cda3b15a3b09c64424634cd53c74R923 >>> < >>> https://github.com/doug-gilbert/smp_utils/commit/5e9dd3dbab9f7fc85e2816ab2eb62bc92b068cc1#diff-853a65985ccfe20502945ddfbc340cbedde6cda3b15a3b09c64424634cd53c74R923 >>> > >>> The working code was: >>> num = op->do_num ? (op->phy_id + op->do_num) : MAX_PHY_ID; >>> >>> The new broken code is: >>> num = get_num_phys(top, op, &has_t2t); >>> if (num <= 0) >>> num = op->do_num ? (op->phy_id + op->do_num) : MAX_PHY_ID; >>> else { >>> if (op->phy_id >= num) { >>> printf("Given phy_id=%d at or beyond number of phys (%d)\n", >>> op->phy_id, num); >>> return 0; /* nothing to do */ >>> } >>> num -= op->phy_id; >>> if (op->do_num) >>> num = (num > op->do_num) ? op->do_num : num; >>> } >>> I don't really understand what upstream was trying to do here, possibly >>> attempting to improve the error messages? >>> I emailed them as they don't really seem to have a bug tracker. This was >>> on 2025-01-17 and I haven't received a reply yet. I don't know if the >>> project is still actively maintained as upstream's doesn't seem to be >>> public, but the GitHub mirror hasn't been updated for about 2 years. >>> >>> I have opened a trivial merge request at >>> https://salsa.debian.org/linux-blocks-team/smp-utils/-/merge_requests/4 >>> <https://salsa.debian.org/linux-blocks-team/smp-utils/-/merge_requests/4 >>> > >>> which is restoring the old code to make option parsing work again. >>> >> Hi, >> >> As I explained at the end of the bug report, I contacted Douglas Gilbert >> 10 days ago, using the email provided at the bottom of >> https://sg.danny.cz/sg/index.html. I haven't heard from him since. I >> could open an issue at https://github.com/doug-gilbert/smp_utils/issues >> but I doubt it would help as the author "still prefers to use a subversion >> repository on his own equipment (a Raspberry Pi) for development" according >> to https://sg.danny.cz/sg/smp_utils.html. If you have another way to >> reach him, please let me know. >> > It's now reported at https://github.com/doug-gilbert/smp_utils/issues/4 > Have you looked at > https://salsa.debian.org/linux-blocks-team/smp-utils/-/merge_requests/4? > Do you think it could still be applied to fix the Debian package or would > you rather wait for a potential response from upstream? >
