netdev - Re: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode

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] [day] [month] [year] [list]

Date:	Wed, 19 Nov 2014 13:56:21 -0800
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	"Tantilov, Emil S" <emil.s.tantilov@...el.com>
Cc:	Vlad Yasevich <vyasevich@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Keller, Jacob E" <jacob.e.keller@...el.com>,
	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
Subject: Re: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode

On Wed, 2014-11-19 at 13:50 -0800, Tantilov, Emil S wrote:
> >-----Original Message-----
> >From: Vlad Yasevich [mailto:vyasevich@...il.com]
> >Sent: Wednesday, November 19, 2014 11:41 AM
> >To: Tantilov, Emil S; netdev@...r.kernel.org
> >Cc: Kirsher, Jeffrey T; Keller, Jacob E; Skidmore, Donald C
> >Subject: Re: [PATCH] ixgbe: Correctly disable vlan filter in
> >promiscuous mode
> >
> >On 11/18/2014 03:24 PM, Tantilov, Emil S wrote:
> >>> -----Original Message-----
> >>> From: netdev-owner@...r.kernel.org [mailto:netdev-
> >>> owner@...r.kernel.org] On Behalf Of Vladislav Yasevich
> >>> Sent: Tuesday, November 18, 2014 11:28 AM
> >>> To: netdev@...r.kernel.org
> >>> Cc: Vladislav Yasevich; Kirsher, Jeffrey T; Keller, Jacob E
> >>> Subject: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
> >>>
> >>> IXGBE adapater seems to require that vlan filtering be enabled if VMDQ
> >>> or SRIOV are enabled.  When those functions are disabled,
> >>> vlan filtering may be disabled in promiscuous mode.
> >>>
> >>> Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
> >>>    ixgbe: remove vlan_filter_disable and enable functions
> >>>
> >>> the logic was correct.  However, after the commit the logic
> >>> got reversed and vlan filtered in now turned on when VMDQ/SRIOV
> >>> is disabled.
> >>>
> >>> This patch changes the condition to enable hw vlan filtered
> >>> when VMDQ or SRIOV is enabled.
> >>>
> >>> Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe:
> >>> remove
> >>> vlan_filter_disable and enable functions)
> >>> CC: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> >>> CC: Jacob Keller <jacob.e.keller@...el.com>
> >>> Signed-off-by: Vladislav Yasevich <vyasevic@...hat.com>
> >>> ---
> >>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> index d2df4e3..3f81c7a 100644
> >>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> >>> @@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct
> >>> net_device *netdev)
> >>> 		 * if SR-IOV and VMDQ are disabled - otherwise ensure
> >>> 		 * that hardware VLAN filters remain enabled.
> >>> 		 */
> >>> -		if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
> >>> -					IXGBE_FLAG_SRIOV_ENABLED)))
> >>> +		if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
> >>> +				      IXGBE_FLAG_SRIOV_ENABLED))
> >>> 			vlnctrl |= (IXGBE_VLNCTRL_VFE |
> >>> IXGBE_VLNCTRL_CFIEN);
> >>> 	} else {
> >>> 		if (netdev->flags & IFF_ALLMULTI) {
> >>
> >> The current logic is correct and it's like this on purpose
> >> as it should be obvious by the comment preceding this check.
> >
> >Actually the comment right now does not match what the code
> >is doing.
> >
> >The comment states:
> >                /* Only disable hardware filter vlans in promiscuous mode
> >                 * if SR-IOV and VMDQ are disabled - otherwise ensure
> >                 * that hardware VLAN filters remain enabled.
> >                 */
> >
> >However, the code currently will _enable_ vlan filtering if VMDQ/SRIOV
> >is _disabled_ in promiscuous mode.
> 
> Actually you're right. Sorry - I misread the patch initially.
> This is indeed a bug in the code.
> 
> Acked-by: Emil Tantilov <emil.s.tantilov@...el.com>
> 
> Jeff should pick it up.

Consider it done, I have added Vlad's patch to my queue.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)