netdev - Re: [PATCH bpf-next v7 11/18] ice: use VLAN proto from ring packet context in skb path

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: <ZVeDcaZGyeyMrWlK@boxer>
Date: Fri, 17 Nov 2023 16:14:57 +0100
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Larysa Zaremba <larysa.zaremba@...el.com>
CC: <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>,
	<andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>, <yhs@...com>,
	<john.fastabend@...il.com>, <kpsingh@...nel.org>, <sdf@...gle.com>,
	<haoluo@...gle.com>, <jolsa@...nel.org>, David Ahern <dsahern@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, Willem de Bruijn <willemb@...gle.com>,
	Jesper Dangaard Brouer <hawk@...nel.org>, Anatoly Burakov
	<anatoly.burakov@...el.com>, Alexander Lobakin <alexandr.lobakin@...el.com>,
	Magnus Karlsson <magnus.karlsson@...il.com>, Maryam Tahhan
	<mtahhan@...hat.com>, <xdp-hints@...-project.net>, <netdev@...r.kernel.org>,
	Willem de Bruijn <willemdebruijn.kernel@...il.com>, Alexei Starovoitov
	<alexei.starovoitov@...il.com>, Tariq Toukan <tariqt@...lanox.com>, "Saeed
 Mahameed" <saeedm@...lanox.com>
Subject: Re: [PATCH bpf-next v7 11/18] ice: use VLAN proto from ring packet
 context in skb path

On Fri, Nov 17, 2023 at 03:07:37PM +0100, Larysa Zaremba wrote:
> On Fri, Nov 17, 2023 at 09:10:44AM +0100, Maciej Fijalkowski wrote:
> > On Wed, Nov 15, 2023 at 06:52:53PM +0100, Larysa Zaremba wrote:
> > > VLAN proto, used in ice XDP hints implementation is stored in ring packet
> > > context. Utilize this value in skb VLAN processing too instead of checking
> > > netdev features.
> > > 
> > > At the same time, use vlan_tci instead of vlan_tag in touched code,
> > > because VLAN tag often refers to VLAN proto and VLAN TCI combined,
> > > while in the code we clearly store only VLAN TCI.
> > > 
> > > Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
> > > ---
> > >  drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 14 +++++---------
> > >  drivers/net/ethernet/intel/ice/ice_txrx_lib.h |  2 +-
> > >  2 files changed, 6 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> > > index 63bf9f882363..a1f5243299c5 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c
> > > @@ -246,21 +246,17 @@ ice_process_skb_fields(struct ice_rx_ring *rx_ring,
> > >   * ice_receive_skb - Send a completed packet up the stack
> > >   * @rx_ring: Rx ring in play
> > >   * @skb: packet to send up
> > > - * @vlan_tag: VLAN tag for packet
> > > + * @vlan_tci: VLAN TCI for packet
> > >   *
> > >   * This function sends the completed packet (via. skb) up the stack using
> > >   * gro receive functions (with/without VLAN tag)
> > >   */
> > >  void
> > > -ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tag)
> > > +ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tci)
> > >  {
> > > -	netdev_features_t features = rx_ring->netdev->features;
> > > -	bool non_zero_vlan = !!(vlan_tag & VLAN_VID_MASK);
> > > -
> > > -	if ((features & NETIF_F_HW_VLAN_CTAG_RX) && non_zero_vlan)
> > > -		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
> > > -	else if ((features & NETIF_F_HW_VLAN_STAG_RX) && non_zero_vlan)
> > > -		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021AD), vlan_tag);
> > > +	if ((vlan_tci & VLAN_VID_MASK) && rx_ring->vlan_proto)
> > > +		__vlan_hwaccel_put_tag(skb, rx_ring->vlan_proto,
> > 
> > Umm... pkt_ctx.vlan_proto ?
> > 
> 
> This compiles both ways. I have put pkt_ctx into a union.

Right, missed that.

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>

> 
> > > +				       vlan_tci);
> > >  
> > >  	napi_gro_receive(&rx_ring->q_vector->napi, skb);
> > >  }
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
> > > index 3893af1c11f3..762047508619 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
> > > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.h
> > > @@ -150,7 +150,7 @@ ice_process_skb_fields(struct ice_rx_ring *rx_ring,
> > >  		       union ice_32b_rx_flex_desc *rx_desc,
> > >  		       struct sk_buff *skb);
> > >  void
> > > -ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tag);
> > > +ice_receive_skb(struct ice_rx_ring *rx_ring, struct sk_buff *skb, u16 vlan_tci);
> > >  
> > >  static inline void
> > >  ice_xdp_meta_set_desc(struct xdp_buff *xdp,
> > > -- 
> > > 2.41.0
> > >