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

Message-ID: <87618083B2453E4A8714035B62D67992501A1011@FMSMSX105.amr.corp.intel.com>
Date:	Wed, 19 Nov 2014 21:50:56 +0000
From:	"Tantilov, Emil S" <emil.s.tantilov@...el.com>
To:	Vlad Yasevich <vyasevich@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"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

>-----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.

Thanks,
Emil

--
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