| Message ID | 20230427092309.3122-1-jan.sokolowski@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net,v2] ice: Fix undersized tx_flags variable | expand |
On Thu, Apr 27, 2023 at 11:19 AM Jan Sokolowski <jan.sokolowski@intel.com> wrote: > > As not all ICE_TX_FLAGS_* fit in current 16-bit limited > tx_flags field, some VLAN-related flags would not properly apply. > > Fix that by refactoring tx_flags variable into flags only and > a separate variable that holds VLAN ID. As there is some space left, > type variable can fit between those two. Pahole reports no size > change to ice_tx_buf struct. > > Fixes: aa1d3faf71a6 ("ice: Robustify cleaning/completing XDP Tx buffers") > Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com> > Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> I tested that with this patch the kernel now passes VLAN tests in our lab again (a test based on LNST VlansRecipe, https://lnst.readthedocs.io/en/latest/vlans_recipe.html). Tested-by: Michal Schmidt <mschmidt@redhat.com>
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Sokolowski, Jan > Sent: Thursday, April 27, 2023 2:53 PM > To: intel-wired-lan@lists.osuosl.org > Subject: [Intel-wired-lan] [PATCH net v2] ice: Fix undersized tx_flags variable > > As not all ICE_TX_FLAGS_* fit in current 16-bit limited tx_flags field, some VLAN-related flags would not properly apply. > > Fix that by refactoring tx_flags variable into flags only and a separate variable that holds VLAN ID. As there is some space left, type variable can fit between those two. Pahole reports no size change to ice_tx_buf struct. > > Fixes: aa1d3faf71a6 ("ice: Robustify cleaning/completing XDP Tx buffers") > Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com> > Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> > --- > v2: Applied review tips received on IWL, instead of changing the size of tx_flags now it works by explicit split of flags and vid variable > --- > drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 6 +++--- > drivers/net/ethernet/intel/ice/ice_txrx.c | 8 +++----- > drivers/net/ethernet/intel/ice/ice_txrx.h | 12 ++++++------ > 3 files changed, 12 insertions(+), 14 deletions(-) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c index c6d4926f0fcf..6c212d0dbef3 100644 --- a/drivers/net/ethernet/intel/ice/ice_dcb_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_dcb_lib.c @@ -932,10 +932,10 @@ ice_tx_prepare_vlan_flags_dcb(struct ice_tx_ring *tx_ring, if ((first->tx_flags & ICE_TX_FLAGS_HW_VLAN || first->tx_flags & ICE_TX_FLAGS_HW_OUTER_SINGLE_VLAN) || skb->priority != TC_PRIO_CONTROL) { - first->tx_flags &= ~ICE_TX_FLAGS_VLAN_PR_M; + first->vid &= ~ICE_TX_VLAN_PR_M; /* Mask the lower 3 bits to set the 802.1p priority */ - first->tx_flags |= (skb->priority & 0x7) << - ICE_TX_FLAGS_VLAN_PR_S; + first->vid |= (skb->priority << ICE_TX_VLAN_PR_S) & + ICE_TX_VLAN_PR_M; /* if this is not already set it means a VLAN 0 + priority needs * to be offloaded */ diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c index 4fcf2d07eb85..059bd911c51d 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c @@ -1664,8 +1664,7 @@ ice_tx_map(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first, if (first->tx_flags & ICE_TX_FLAGS_HW_VLAN) { td_cmd |= (u64)ICE_TX_DESC_CMD_IL2TAG1; - td_tag = (first->tx_flags & ICE_TX_FLAGS_VLAN_M) >> - ICE_TX_FLAGS_VLAN_S; + td_tag = first->vid; } dma = dma_map_single(tx_ring->dev, skb->data, size, DMA_TO_DEVICE); @@ -1998,7 +1997,7 @@ ice_tx_prepare_vlan_flags(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first) * VLAN offloads exclusively so we only care about the VLAN ID here */ if (skb_vlan_tag_present(skb)) { - first->tx_flags |= skb_vlan_tag_get(skb) << ICE_TX_FLAGS_VLAN_S; + first->vid = skb_vlan_tag_get(skb); if (tx_ring->flags & ICE_TX_FLAGS_RING_VLAN_L2TAG2) first->tx_flags |= ICE_TX_FLAGS_HW_OUTER_SINGLE_VLAN; else @@ -2388,8 +2387,7 @@ ice_xmit_frame_ring(struct sk_buff *skb, struct ice_tx_ring *tx_ring) offload.cd_qw1 |= (u64)(ICE_TX_DESC_DTYPE_CTX | (ICE_TX_CTX_DESC_IL2TAG2 << ICE_TXD_CTX_QW1_CMD_S)); - offload.cd_l2tag2 = (first->tx_flags & ICE_TX_FLAGS_VLAN_M) >> - ICE_TX_FLAGS_VLAN_S; + offload.cd_l2tag2 = first->vid; } /* set up TSO offload */ 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 #define ICE_XDP_PASS 0 #define ICE_XDP_CONSUMED BIT(0) @@ -182,8 +181,9 @@ struct ice_tx_buf { unsigned int gso_segs; unsigned int nr_frags; /* used for mbuf XDP */ }; - u32 type:16; /* &ice_tx_buf_type */ - u32 tx_flags:16; + u32 tx_flags:12; + u32 type:4; /* &ice_tx_buf_type */ + u32 vid:16; DEFINE_DMA_UNMAP_LEN(len); DEFINE_DMA_UNMAP_ADDR(dma); };
As not all ICE_TX_FLAGS_* fit in current 16-bit limited tx_flags field, some VLAN-related flags would not properly apply. Fix that by refactoring tx_flags variable into flags only and a separate variable that holds VLAN ID. As there is some space left, type variable can fit between those two. Pahole reports no size change to ice_tx_buf struct. Fixes: aa1d3faf71a6 ("ice: Robustify cleaning/completing XDP Tx buffers") Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com> --- v2: Applied review tips received on IWL, instead of changing the size of tx_flags now it works by explicit split of flags and vid variable --- drivers/net/ethernet/intel/ice/ice_dcb_lib.c | 6 +++--- drivers/net/ethernet/intel/ice/ice_txrx.c | 8 +++----- drivers/net/ethernet/intel/ice/ice_txrx.h | 12 ++++++------ 3 files changed, 12 insertions(+), 14 deletions(-)