Re: [PATCH net-next] net: broadcom: use ethtool string helpers

[Rosen Penev]

On Wed, Oct 30, 2024 at 1:31 PM Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
>
>
>
> On 10/29/2024 5:37 PM, Jakub Kicinski wrote:
> > On Tue, 29 Oct 2024 16:43:15 -0700 Rosen Penev wrote:
> >>>> -             memcpy(buf, bnx2x_tests_str_arr + start,
> >>>> -                    ETH_GSTRING_LEN * BNX2X_NUM_TESTS(bp));
> >>>> +             for (i = start; i < BNX2X_NUM_TESTS(bp); i++)
> >>>> +                     ethtool_puts(&buf, bnx2x_tests_str_arr[i]);
> >>>
> >>> I don't think this is equivalent.
> >> What's wrong here?
> >
> > We used to copy ETH_GSTRING_LEN * BNX2X_NUM_TESTS(bp)
> > but i will no only go from start to BNX2X_NUM_TESTS(bp)
> > IOW the copied length is ETH_GSTRING_LEN * (BNX2X_NUM_TESTS(bp) - start)
> > No?
>
> Hmm. Yea that's right. I'm guessing BNX2X_NUM_TESTS(bp) changes the
> number of tests based on what start would be...
makes sense now.

Loop iteration variable should be ARRAY_SIZE(bnx2x_tests_str_arr),
which is BNX2X_NUM_TESTS_SF.
>
> Probably we could use a static size of the string array instead of
> BNX2X_NUM_TESTS(bp), and that would fix things, but this specific change
> needs more careful review of the surrounding code.