netdev - Re: [PATCH 7/7] bnx2x: expose HW RX VLAN stripping toggle

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: <CAHXqBFLgTHvxL34eR1qX7nEmntyOBXYrg-7LQvW=d8dnDpbhWg@mail.gmail.com>
Date:	Tue, 30 Aug 2011 20:27:28 +0200
From:	Michał Mirosław <mirqus@...il.com>
To:	Michal Schmidt <mschmidt@...hat.com>
Cc:	netdev@...r.kernel.org, vladz@...adcom.com, dmitry@...adcom.com,
	eilong@...adcom.com
Subject: Re: [PATCH 7/7] bnx2x: expose HW RX VLAN stripping toggle

2011/8/30 Michal Schmidt <mschmidt@...hat.com>:
> Allow disabling of HW RX VLAN stripping with ethtool.
>
> [v2: Store the flag in the fp to ensure that pending packets are
>     handled correctly during a switch.]
[...]
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -66,6 +66,9 @@ static inline void bnx2x_bz_fp(struct bnx2x *bp, int index)
>         */
>        if ((bp->flags & TPA_ENABLE_FLAG) && !IS_FCOE_FP(fp))
>                fp->flags = FP_TPA;
> +
> +       if (bp->flags & RX_VLAN_STRIP_FLAG)
> +               fp->flags |= FP_VLAN_STRIP;
>  }
[...]
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -2719,9 +2719,8 @@ static unsigned long bnx2x_get_q_flags(struct bnx2x *bp,
>                __set_bit(BNX2X_Q_FLG_MCAST, &flags);
>        }
>
> -       /* Always set HW VLAN stripping */
> -       __set_bit(BNX2X_Q_FLG_VLAN, &flags);
> -
> +       if (fp->flags & FP_VLAN_STRIP)
> +               __set_bit(BNX2X_Q_FLG_VLAN, &flags);
>
>        return flags | bnx2x_get_common_flags(bp, fp, true);
>  }


It seems rather convoluted and unnecessary that you mirror
NETIF_F_HW_VLAN_RX in bp->flags and then also in fp->flags. Are the
fp->flags strictly mirroring hardware state (as in: there is no way
the states can differ in any point in time where the flags are
tested)? For this to be true, the two functions above need to be
called only without releasing a lock between them that is also taken
by receive handler. Isn't there a flag in the rx descriptor of a
packet that says if VLAN was stripped? (All that flag keeping would be
unnecessary then.)

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html