netdev - Re: [PATCH net] ice: Fix undersized tx

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20230509200910.2a149f24@kernel.org>
Date: Tue, 9 May 2023 20:09:10 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com,
 netdev@...r.kernel.org, Jan Sokolowski <jan.sokolowski@...el.com>,
 maciej.fijalkowski@...el.com, daniel@...earbox.net, Alexander Lobakin
 <aleksander.lobakin@...el.com>, Michal Schmidt <mschmidt@...hat.com>, Pucha
 Himasekhar Reddy <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net] ice: Fix undersized tx_flags variable

On Mon,  8 May 2023 10:42:25 -0700 Tony Nguyen wrote:
> From: Jan Sokolowski <jan.sokolowski@...el.com>
> 
> As not all ICE_TX_FLAGS_* fit in current 16-bit limited
> tx_flags field, some VLAN-related flags would not properly apply.

nit: this is a bit of a understatement. The vlan info is gone
completely, right? Maybe say something along the lines of:

"VLAN ID was stored on upper 16 bits of tx_flags, the commit under
Fixes reduced the size of tx_flags to 16 bits discarding vlan
information completely."

> Fix that by refactoring tx_flags variable into flags only and
> a separate variable that holds VLAN ID.

This sentence just describes what the patch does, it's not necessary.

> As there is some space left,
> type variable can fit between those two. Pahole reports no size
> change to ice_tx_buf struct.

You need to also describe user-visible misbehavior.

> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
> index fff0efe28373..76a34d435025 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.h
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
> @@ -127,10 +127,9 @@ static inline int ice_skb_pad(void)
>  #define ICE_TX_FLAGS_IPV6	BIT(6)
>  #define ICE_TX_FLAGS_TUNNEL	BIT(7)
>  #define ICE_TX_FLAGS_HW_OUTER_SINGLE_VLAN	BIT(8)
> -#define ICE_TX_FLAGS_VLAN_M	0xffff0000
> -#define ICE_TX_FLAGS_VLAN_PR_M	0xe0000000
> -#define ICE_TX_FLAGS_VLAN_PR_S	29
> -#define ICE_TX_FLAGS_VLAN_S	16
> +
> +#define ICE_TX_VLAN_PR_M	0xe000
> +#define ICE_TX_VLAN_PR_S	13

You can use VLAN_PRIO_MASK and VLAN_PRIO_SHIFT if you're storing
it in a normal-ish 16b field.
-- 
pw-bot: cr