Date: Fri, 30 Nov 2018 23:10:39 +0300 From: Andrew Rybchenko <arybchik@FreeBSD.org> To: Philip Paeps <philip@freebsd.org>, John Baldwin <jhb@FreeBSD.org> Cc: gnn@FreeBSD.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r341327 - head/sys/dev/sfxge Message-ID: <cd231ea2-1001-184b-cb45-eb7e4fd41e2f@oktetlabs.ru> In-Reply-To: <0A8EBFBB-37DA-439F-A8D3-099A13128AD9@freebsd.org> References: <201811300711.wAU7B5R6084752@repo.freebsd.org> <d60f54ae-a1a6-bad5-73f0-c898d0579089@FreeBSD.org> <7545b110-4930-d49f-f99f-7ec17db7e7a7@oktetlabs.ru> <be19ca93-6703-e329-b070-f338ec264f0b@FreeBSD.org> <0A8EBFBB-37DA-439F-A8D3-099A13128AD9@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <Ivan.Malov at oktetlabs.ru> >>>>> 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?cd231ea2-1001-184b-cb45-eb7e4fd41e2f>