From owner-svn-src-head@freebsd.org Thu May 18 15:13:17 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 024C1D73209; Thu, 18 May 2017 15:13:17 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (turbocat.net [IPv6:2a01:4f8:c17:6c4b::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C10151A8D; Thu, 18 May 2017 15:13:16 +0000 (UTC) (envelope-from hps@selasky.org) Received: from hps2016.home.selasky.org (unknown [62.141.129.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id E05AA26093C; Thu, 18 May 2017 17:13:14 +0200 (CEST) Subject: Re: svn commit: r305177 - head/sys/net To: Oleg Bulyzhin Cc: Sepherosa Ziehau , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, araujo@FreeBSD.org References: <201609010632.u816WZUj025186@repo.freebsd.org> <20170518140403.GA54405@lath.RINET.ru> <355478da-ed4d-60bb-bb4e-144a694e28b8@selasky.org> <20170518150030.GA55481@lath.RINET.ru> From: Hans Petter Selasky Message-ID: Date: Thu, 18 May 2017 17:11:16 +0200 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 MIME-Version: 1.0 In-Reply-To: <20170518150030.GA55481@lath.RINET.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 May 2017 15:13:17 -0000 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