From owner-svn-src-head@freebsd.org Fri Nov 30 20:10:42 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6125F114F230; Fri, 30 Nov 2018 20:10:42 +0000 (UTC) (envelope-from arybchik@FreeBSD.org) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [84.52.114.115]) (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 D445B75A39; Fri, 30 Nov 2018 20:10:41 +0000 (UTC) (envelope-from arybchik@FreeBSD.org) Received: from [192.168.1.192] (unknown [188.242.181.57]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 4AE347FFCF; Fri, 30 Nov 2018 23:10:40 +0300 (MSK) Subject: Re: svn commit: r341327 - head/sys/dev/sfxge To: Philip Paeps , John Baldwin Cc: gnn@FreeBSD.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201811300711.wAU7B5R6084752@repo.freebsd.org> <7545b110-4930-d49f-f99f-7ec17db7e7a7@oktetlabs.ru> <0A8EBFBB-37DA-439F-A8D3-099A13128AD9@freebsd.org> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: Date: Fri, 30 Nov 2018 23:10:39 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <0A8EBFBB-37DA-439F-A8D3-099A13128AD9@freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Rspamd-Queue-Id: D445B75A39 X-Spamd-Result: default: False [0.82 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-0.16)[-0.157,0]; NEURAL_SPAM_LONG(0.15)[0.155,0]; ASN(0.00)[asn:25408, ipnet:84.52.64.0/18, country:RU]; NEURAL_SPAM_SHORT(0.82)[0.818,0] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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: Fri, 30 Nov 2018 20:10:42 -0000 On 30.11.2018 22:01, Philip Paeps wrote: > On 2018-11-30 19:36:27 (+0100), John Baldwin wrote: >> On 11/30/18 10:15 AM, Andrew Rybchenko wrote: >>> On 30.11.2018 20:30, John Baldwin wrote: >>>> On 11/29/18 11:11 PM, Andrew Rybchenko wrote: >>>>> Author: arybchik >>>>> Date: Fri Nov 30 07:11:05 2018 >>>>> New Revision: 341327 >>>>> URL: https://svnweb.freebsd.org/changeset/base/341327 >>>>> >>>>> Log: >>>>>    sfxge(4): rollback last seen VLAN TCI if Tx packet is dropped >>>>> >>>>>    Early processing of a packet on transmit may change last seen >>>>>    VLAN TCI in the queue context. If such a packet is eventually >>>>>    dropped, last seen VLAN TCI must be set to its previous value. >>>>> >>>>>    Submitted by:   Ivan Malov >>>>>    Sponsored by:   Solarflare Communications, Inc. >>>>>    MFC after:      1 week >>>>>    Differential Revision: https://reviews.freebsd.org/D18288 >>>> >>>> Just as a general comment.  There's no point in creating a review >>>> in phabricator if you aren't going to get any actual review >>>> feedback via the tool.  That just adds noise.  (I've spotchecked a >>>> few of the recent sfxge commits and they all seem to create a >>>> review that then gets committed a few hours later without any >>>> feedback, etc.) >>> >>> All these changesets is the result of development in Solarflare.  >>> All these changesets were reviewed internally and in fact many have >>> later fixes which are simply squashed in. >>> >>> We have discussed it with George (gnn@) some time ago and he asked >>> to submit reviews anyway and wait at least a day or two before >>> commit. Yes in this particular case these 2 hundreds of patches is >>> the result of 2 years of development. So, I'd waited some time and >>> started to commit in blocks. >>> >>> This time I've not included np@ and bz@ in reviewers since I've not >>> got reviewed before and it would be too much spam. >>> >>> We have discussed it with Philip (philip@) shortly. As I understand >>> he has no time now to review it. >>> >>> Basically I'm ready to follow any sensible policy. I don't think it >>> makes to wait forever. If there are any volunteers I'll be happy to >>> include more people in reviewers. >> >> I don't think you have to wait forever, and if the changes were >> reviewed internally that counts for review, I just don't want to add >> noise and clutter to phabricator and commit logs. > > I think it makes sense to have Phabricator reviews for the > FreeBSD-specific parts of sfxge(4). Yes, I think it would be very-very useful. > The internal review at Solarflare is definitely good enough to commit > without waiting for review -- certainly on the parts of the driver > that are generated from the common source -- but there is some value > to having the FreeBSD-specific bits sit in Phabricator for a few days. Makes sense. Two FreeBSD-specific are waiting right now :) > The storm of commits in the last week is exceptional because it > represents two years of changes.  With hindsight, just bulk-committing > those to Subversion would have been a better idea. I think it is still useful for the project to have more granular changes. At least it easier to understand in the future motivation of these changes etc. Not a strong opinion as well, but I'd prefer to keep these changes as is (except squashing fixes as I do to avoid known breakages in the middle). > In the future though, and when in the steady state of commits > trickling in rather than flooding in, I still appreciate the reviews > in Phabricator.  Particularly for the FreeBSD-specific parts of the > driver. I can't promise, but I'll try to avoid so huge delays in the future. I'm trying to submit when I'm more confident that changes are stable and probability of problems is less. Andrew.