netdev - Re: [PATCH] ixgbe: fix vlan filtering

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: <20080826165004.6467918d.akpm@linux-foundation.org>
Date:	Tue, 26 Aug 2008 16:50:04 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Cc:	jeff@...zik.org, netdev@...r.kernel.org, davem@...emloft.net,
	alexander.h.duyck@...el.com, jesse.brandeburg@...el.com,
	jeffrey.t.kirsher@...el.com
Subject: Re: [PATCH] ixgbe: fix vlan filtering

On Thu, 21 Aug 2008 18:01:28 -0700
Jeff Kirsher <jeffrey.t.kirsher@...el.com> wrote:

> From: Alexander Duyck <alexander.h.duyck@...el.com>
> 
> there are not vlan filter settings in the fctrl register.

Guys, this is a crap changelog.

> Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> ---
> 
>  drivers/net/ixgbe/ixgbe_main.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index 34bca16..53f41b6 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -1636,16 +1636,17 @@ static void ixgbe_set_multi(struct net_device *netdev)
>  	struct ixgbe_hw *hw = &adapter->hw;
>  	struct dev_mc_list *mc_ptr;
>  	u8 *mta_list;
> -	u32 fctrl;
> +	u32 fctrl, vlnctrl;
>  	int i;
>  
>  	/* Check for Promiscuous and All Multicast modes */
>  
>  	fctrl = IXGBE_READ_REG(hw, IXGBE_FCTRL);
> +	vlnctrl = IXGBE_READ_REG(hw, IXGBE_VLNCTRL);
>  
>  	if (netdev->flags & IFF_PROMISC) {
>  		fctrl |= (IXGBE_FCTRL_UPE | IXGBE_FCTRL_MPE);
> -		fctrl &= ~IXGBE_VLNCTRL_VFE;
> +		vlnctrl &= ~IXGBE_VLNCTRL_VFE;
>  	} else {
>  		if (netdev->flags & IFF_ALLMULTI) {
>  			fctrl |= IXGBE_FCTRL_MPE;
> @@ -1653,10 +1654,11 @@ static void ixgbe_set_multi(struct net_device *netdev)
>  		} else {
>  			fctrl &= ~(IXGBE_FCTRL_UPE | IXGBE_FCTRL_MPE);
>  		}
> -		fctrl |= IXGBE_VLNCTRL_VFE;
> +		vlnctrl |= IXGBE_VLNCTRL_VFE;
>  	}
>  
>  	IXGBE_WRITE_REG(hw, IXGBE_FCTRL, fctrl);
> +	IXGBE_WRITE_REG(hw, IXGBE_VLNCTRL, vlnctrl);
>  
>  	if (netdev->mc_count) {
>  		mta_list = kcalloc(netdev->mc_count, ETH_ALEN, GFP_ATOMIC);
> 

There's no way for me (at least) to judge the importance of this patch.
Maybe Jeff happens to know enough about this driver and the proposed
change to be able to work this out, but why risk it?

How long does it take to write a halfway respectable changelog?  Sixty
seconds?

A patch which purportedly fixes a bug should at the very least explain
the user-visible consequences of the bug.  That helps people to understand
the priority of the fix, and helps them to decide which kernel versions
it should be merged into.

Also, the fix _appears_ to be applicable to 2.6.26.x, 2.6.25.x and
perhaps earlier.  But for some reason which I didn't look into it does
not apply there.  It is valuable if the patch originator can provide
advice about which stable kernels (if any) they think the patch should
be backported into.

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