netdev - Re: [PATCH net-next 01/12] ice: Minor switchdev fixes

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: <05485b0b-27be-1595-a41a-7ff5a0297e93@intel.com>
Date:   Wed, 19 Apr 2023 16:35:43 +0200
From:   Alexander Lobakin <aleksander.lobakin@...el.com>
To:     Wojciech Drewek <wojciech.drewek@...el.com>
CC:     <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
        <alexandr.lobakin@...el.com>, <david.m.ertman@...el.com>,
        <michal.swiatkowski@...ux.intel.com>,
        <marcin.szycik@...ux.intel.com>, <pawel.chmielewski@...el.com>,
        <sridhar.samudrala@...el.com>
Subject: Re: [PATCH net-next 01/12] ice: Minor switchdev fixes

From: Wojciech Drewek <wojciech.drewek@...el.com>
Date: Mon, 17 Apr 2023 11:34:01 +0200

> Introduce a few fixes that are needed for bridge offload
> to work properly.
> 
> - Skip adv rule removal in ice_eswitch_disable_switchdev.
>   Advanced rules for ctrl VSI will be removed anyway when the
>   VSI will cleaned up, no need to do it explicitly.

[...]

> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 7c04057c524c..f198c845631f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -385,7 +385,7 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi)
>  	}
>  	err = 0;
>  	/* check for changes in promiscuous modes */
> -	if (changed_flags & IFF_ALLMULTI) {
> +	if (changed_flags & IFF_ALLMULTI && !ice_is_switchdev_running(pf)) {

Nit: pls enclose bitops into separate set of braces, i.e. in this case:

	if ((changed_flags & IFF_ALLMULTI) && ...

It's more safe and also more readable I'd say (clearly states that `&`
is intended, not a typoed `&&`).

>  		if (vsi->current_netdev_flags & IFF_ALLMULTI) {
>  			err = ice_set_promisc(vsi, ICE_MCAST_PROMISC_BITS);
>  			if (err) {

Thanks,
Olek