Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 May 2017 17:11:16 +0200
From:      Hans Petter Selasky <hps@selasky.org>
To:        Oleg Bulyzhin <oleg@FreeBSD.org>
Cc:        Sepherosa Ziehau <sephe@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, araujo@FreeBSD.org
Subject:   Re: svn commit: r305177 - head/sys/net
Message-ID:  <c2dc3966-bf61-e240-4a4f-5057ff94d22f@selasky.org>
In-Reply-To: <20170518150030.GA55481@lath.RINET.ru>
References:  <201609010632.u816WZUj025186@repo.freebsd.org> <20170518140403.GA54405@lath.RINET.ru> <355478da-ed4d-60bb-bb4e-144a694e28b8@selasky.org> <20170518150030.GA55481@lath.RINET.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
On 05/18/17 17:00, Oleg Bulyzhin wrote:
> On Thu, May 18, 2017 at 04:25:01PM +0200, Hans Petter Selasky wrote:
>> On 05/18/17 16:04, Oleg Bulyzhin wrote:
>>> On Thu, Sep 01, 2016 at 06:32:35AM +0000, Sepherosa Ziehau wrote:
>>>> Author: sephe
>>>> Date: Thu Sep  1 06:32:35 2016
>>>> New Revision: 305177
>>>> URL: https://svnweb.freebsd.org/changeset/base/305177
>>>>
>>>> Log:
>>>>     net/vlan: Shift for pri is 13 (pri mask 0xe000) not 1.
>>>>     
>>>>     Reviewed by:	araujo, hps
>>>>     MFC after:	1 week
>>>>     Sponsored by:	Microsoft
>>>>     Differential Revision:	https://reviews.freebsd.org/D7710
>>>>
>>>> Modified:
>>>>     head/sys/net/ethernet.h
>>>>
>>>> Modified: head/sys/net/ethernet.h
>>>> ==============================================================================
>>>> --- head/sys/net/ethernet.h	Thu Sep  1 06:05:08 2016	(r305176)
>>>> +++ head/sys/net/ethernet.h	Thu Sep  1 06:32:35 2016	(r305177)
>>>> @@ -92,7 +92,7 @@ struct ether_vlan_header {
>>>>    #define	EVL_PRIOFTAG(tag)	(((tag) >> 13) & 7)
>>>>    #define	EVL_CFIOFTAG(tag)	(((tag) >> 12) & 1)
>>>>    #define	EVL_MAKETAG(vlid, pri, cfi)					\
>>>> -	((((((pri) & 7) << 1) | ((cfi) & 1)) << 12) | ((vlid) & EVL_VLID_MASK))
>>>> +	((((((pri) & 7) << 13) | ((cfi) & 1)) << 12) | ((vlid) & EVL_VLID_MASK))
>>>>    
>>>>    /*
>>>>     *  NOTE: 0x0000-0x05DC (0..1500) are generally IEEE 802.3 length fields.
>>>
>>> Please revert this one. It's just plain wrong and previous one was ok.
>>>
>>
>> Hi,
>>
>> Can you explain a bit more what is wrong?
>>
>>> If you care about readability it should be:
>>> ((((pri) & 7) << 13) | (((cfi) & 1) << 12) | ((vlid) & EVL_VLID_MASK))
>>
>> Isn't this exactly what the patch is doing? -R ???
> 
> Current version is shifting pri out of uint16. If you examine parentheses:
> pri is shifted left 13, then 12.
> Original version did it right (shift 1, then 12 (total 13)).
> 

Hi Oleg,

I see. The VLAN priority is then always zero after this change.

I'll let Sepherosa handle the revert and/or readability update.

--HPS



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?c2dc3966-bf61-e240-4a4f-5057ff94d22f>