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.
>

Reply via email to