netdev - Re: [PATCH net-next 1/4] cxgb4: Use mask & shift while returning vlan

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: <B7C06B31-B31A-453D-AA4E-4C836056832E@chelsio.com>
Date:	Wed, 16 Dec 2015 17:40:41 -0800
From:	Casey Leedom <leedom@...lsio.com>
To:	David Miller <davem@...emloft.net>
Cc:	hariprasad@...lsio.com, netdev@...r.kernel.org,
	nirranjan@...lsio.com
Subject: Re: [PATCH net-next 1/4] cxgb4: Use mask & shift while returning vlan_prio


> On Dec 16, 2015, at 4:07 PM, David Miller <davem@...emloft.net> wrote:
> 
> From: Hariprasad Shenai <hariprasad@...lsio.com>
> Date: Wed, 16 Dec 2015 13:16:25 +0530
> 
>> @@ -66,7 +66,7 @@ struct l2t_data {
>> 
>> static inline unsigned int vlan_prio(const struct l2t_entry *e)
>> {
>> -	return e->vlan >> 13;
>> +	return (e->vlan & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
> 
> e->vlan is a u16, the vlan priotity is the top 3 bits of the 16-bit
> value, and finally the right shift will be non-signed.
> 
> Therefore this change is absolutely not necessary.
> 
> Please remove this patch from the series and resend.

I assume that you only meant that the masking portion is unnecessary.  Doing the shift with the symbolic constant VLAN_PRIO_SHIFT instead of the literal constant “13” is still a reasonable change.  The masking was almost certainly from me because once one uses the symbolic constants, weren’t not supposed to “know” about the internal structure of the operation.  Modern compilers are of course free to optimize away the mask, etc.

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