netdev - Re: [PATCH net-next 1/3] sfc: extend bitfield macros to 19 fields

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0UciV2rSiNBHQOhqHkrx=XBLzOTdHmKXZ6fTxdt1D3c0Gg@mail.gmail.com>
Date:   Fri, 13 Nov 2020 11:06:03 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Edward Cree <ecree@...arflare.com>
Cc:     Solarflare linux maintainers <linux-net-drivers@...arflare.com>,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] sfc: extend bitfield macros to 19 fields

On Thu, Nov 12, 2020 at 7:23 AM Edward Cree <ecree@...arflare.com> wrote:
>
> Our TSO descriptors got even more fussy.
>
> Signed-off-by: Edward Cree <ecree@...arflare.com>
> ---
>  drivers/net/ethernet/sfc/bitfield.h | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/bitfield.h b/drivers/net/ethernet/sfc/bitfield.h
> index 64731eb5dd56..1f981dfe4bdc 100644
> --- a/drivers/net/ethernet/sfc/bitfield.h
> +++ b/drivers/net/ethernet/sfc/bitfield.h
> @@ -289,7 +289,9 @@ typedef union efx_oword {
>                                  field14, value14,                      \
>                                  field15, value15,                      \
>                                  field16, value16,                      \
> -                                field17, value17)                      \
> +                                field17, value17,                      \
> +                                field18, value18,                      \
> +                                field19, value19)                      \
>         (EFX_INSERT_FIELD_NATIVE((min), (max), field1, (value1)) |      \
>          EFX_INSERT_FIELD_NATIVE((min), (max), field2, (value2)) |      \
>          EFX_INSERT_FIELD_NATIVE((min), (max), field3, (value3)) |      \
> @@ -306,7 +308,9 @@ typedef union efx_oword {
>          EFX_INSERT_FIELD_NATIVE((min), (max), field14, (value14)) |    \
>          EFX_INSERT_FIELD_NATIVE((min), (max), field15, (value15)) |    \
>          EFX_INSERT_FIELD_NATIVE((min), (max), field16, (value16)) |    \
> -        EFX_INSERT_FIELD_NATIVE((min), (max), field17, (value17)))
> +        EFX_INSERT_FIELD_NATIVE((min), (max), field17, (value17)) |    \
> +        EFX_INSERT_FIELD_NATIVE((min), (max), field18, (value18)) |    \
> +        EFX_INSERT_FIELD_NATIVE((min), (max), field19, (value19)))
>
>  #define EFX_INSERT_FIELDS64(...)                               \
>         cpu_to_le64(EFX_INSERT_FIELDS_NATIVE(__VA_ARGS__))
> @@ -348,7 +352,11 @@ typedef union efx_oword {
>  #endif
>
>  /* Populate an octword field with various numbers of arguments */
> -#define EFX_POPULATE_OWORD_17 EFX_POPULATE_OWORD
> +#define EFX_POPULATE_OWORD_19 EFX_POPULATE_OWORD
> +#define EFX_POPULATE_OWORD_18(oword, ...) \
> +       EFX_POPULATE_OWORD_19(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
> +#define EFX_POPULATE_OWORD_17(oword, ...) \
> +       EFX_POPULATE_OWORD_18(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>  #define EFX_POPULATE_OWORD_16(oword, ...) \
>         EFX_POPULATE_OWORD_17(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>  #define EFX_POPULATE_OWORD_15(oword, ...) \
> @@ -391,7 +399,11 @@ typedef union efx_oword {
>                              EFX_DWORD_3, 0xffffffff)
>
>  /* Populate a quadword field with various numbers of arguments */
> -#define EFX_POPULATE_QWORD_17 EFX_POPULATE_QWORD
> +#define EFX_POPULATE_QWORD_19 EFX_POPULATE_QWORD
> +#define EFX_POPULATE_QWORD_18(qword, ...) \
> +       EFX_POPULATE_QWORD_19(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
> +#define EFX_POPULATE_QWORD_17(qword, ...) \
> +       EFX_POPULATE_QWORD_18(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>  #define EFX_POPULATE_QWORD_16(qword, ...) \
>         EFX_POPULATE_QWORD_17(qword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>  #define EFX_POPULATE_QWORD_15(qword, ...) \
> @@ -432,7 +444,11 @@ typedef union efx_oword {
>                              EFX_DWORD_1, 0xffffffff)
>
>  /* Populate a dword field with various numbers of arguments */
> -#define EFX_POPULATE_DWORD_17 EFX_POPULATE_DWORD
> +#define EFX_POPULATE_DWORD_19 EFX_POPULATE_DWORD
> +#define EFX_POPULATE_DWORD_18(dword, ...) \
> +       EFX_POPULATE_DWORD_19(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
> +#define EFX_POPULATE_DWORD_17(dword, ...) \
> +       EFX_POPULATE_DWORD_18(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>  #define EFX_POPULATE_DWORD_16(dword, ...) \
>         EFX_POPULATE_DWORD_17(dword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>  #define EFX_POPULATE_DWORD_15(dword, ...) \
>

Are all these macros really needed? It seems like this is adding a
bunch of noise in order to add support for a few additional fields.
Wouldn't it be possible to just define the ones that are actually
needed and add multiple dummy values to fill in the gaps instead of
defining every macro between zero and 19? For example this patch set
adds an option for setting 18 fields, but from what I can tell it is
never used.