netdev - Re: [PATCH net-next 2/7] ice: Support untagged VLAN traffic in br offload

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: <20230802193142.59fe5bf3@kernel.org>
Date: Wed, 2 Aug 2023 19:31:42 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiri Pirko <jiri@...nulli.us>, Ido Schimmel <idosch@...sch.org>,
 Vladimir Oltean <vladimir.oltean@....com>, Vlad Buslov <vladbu@...dia.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, netdev@...r.kernel.org,
 Wojciech Drewek <wojciech.drewek@...el.com>, Simon Horman
 <horms@...nel.org>, Sujai Buvaneswaran <sujai.buvaneswaran@...el.com>
Subject: Re: [PATCH net-next 2/7] ice: Support untagged VLAN traffic in br
 offload

On Tue,  1 Aug 2023 10:31:07 -0700 Tony Nguyen wrote:
> From: Wojciech Drewek <wojciech.drewek@...el.com>
> 
> When driver receives SWITCHDEV_FDB_ADD_TO_DEVICE notification
> with vid = 1, it means that we have to offload untagged traffic.
> This is achieved by adding vlan metadata lookup.

Paul already asked about this behavior but it's unclear to me from the
answer whether this is a local custom or legit switchdev behavior.
Could someone with switchdev knowledge glance over this?

> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch_br.c b/drivers/net/ethernet/intel/ice/ice_eswitch_br.c
> index 67bfd1f61cdd..5b425260b0eb 100644
> --- a/drivers/net/ethernet/intel/ice/ice_eswitch_br.c
> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch_br.c
> @@ -104,13 +104,18 @@ ice_eswitch_br_rule_delete(struct ice_hw *hw, struct ice_rule_query_data *rule)
>  static u16
>  ice_eswitch_br_get_lkups_cnt(u16 vid)
>  {
> -	return ice_eswitch_br_is_vid_valid(vid) ? 2 : 1;
> +	/* if vid == 0 then we need only one lookup (ICE_MAC_OFOS),
> +	 * otherwise we need both mac and vlan
> +	 */
> +	return vid == 0 ? 1 : 2;
>  }
>  
>  static void
>  ice_eswitch_br_add_vlan_lkup(struct ice_adv_lkup_elem *list, u16 vid)
>  {
> -	if (ice_eswitch_br_is_vid_valid(vid)) {
> +	if (vid == 1) {
> +		ice_rule_add_vlan_metadata(&list[1]);
> +	} else if (vid > 1) {
>  		list[1].type = ICE_VLAN_OFOS;
>  		list[1].h_u.vlan_hdr.vlan = cpu_to_be16(vid & VLAN_VID_MASK);
>  		list[1].m_u.vlan_hdr.vlan = cpu_to_be16(0xFFFF);
> @@ -400,7 +405,6 @@ ice_eswitch_br_fdb_entry_create(struct net_device *netdev,
>  	unsigned long event;
>  	int err;
>  
> -	/* untagged filtering is not yet supported */
>  	if (!(bridge->flags & ICE_ESWITCH_BR_VLAN_FILTERING) && vid)
>  		return;
>  
> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch_br.h b/drivers/net/ethernet/intel/ice/ice_eswitch_br.h
> index 85a8fadb2928..cf7b0e5acfcb 100644
> --- a/drivers/net/ethernet/intel/ice/ice_eswitch_br.h
> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch_br.h
> @@ -103,15 +103,6 @@ struct ice_esw_br_vlan {
>  		     struct ice_esw_br_fdb_work, \
>  		     work)
>  
> -static inline bool ice_eswitch_br_is_vid_valid(u16 vid)
> -{
> -	/* In trunk VLAN mode, for untagged traffic the bridge sends requests
> -	 * to offload VLAN 1 with pvid and untagged flags set. Since these
> -	 * flags are not supported, add a MAC filter instead.
> -	 */
> -	return vid > 1;
> -}
> -
>  void
>  ice_eswitch_br_offloads_deinit(struct ice_pf *pf);
>  int