From owner-svn-src-all@freebsd.org Fri Nov 30 19:01:11 2018 Return-Path: Delivered-To: svn-src-all@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 982BA114CF73; Fri, 30 Nov 2018 19:01:11 +0000 (UTC) (envelope-from philip@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4061971434; Fri, 30 Nov 2018 19:01:11 +0000 (UTC) (envelope-from philip@freebsd.org) Received: from weatherwax.trouble.is (weatherwax.trouble.is [IPv6:2a00:dd80:3c::62e]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "weatherwax.trouble.is", Issuer "Let's Encrypt Authority X3" (verified OK)) (Authenticated sender: philip/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id EBEF31721D; Fri, 30 Nov 2018 19:01:10 +0000 (UTC) (envelope-from philip@freebsd.org) Received: from rincewind.trouble.is (rincewind.trouble.is [IPv6:2a01:4f9:2a:1715::1:1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "rincewind.trouble.is", Issuer "Let's Encrypt Authority X3" (verified OK)) by weatherwax.trouble.is (Postfix) with ESMTPS id 4363dK1kYGz4fYY; Fri, 30 Nov 2018 19:01:09 +0000 (UTC) Received: by rincewind.trouble.is (Postfix, authenticated sender philip) id 4363dG13dzz1mnL; Fri, 30 Nov 2018 19:01:06 +0000 (UTC) From: "Philip Paeps" To: "John Baldwin" Cc: "Andrew Rybchenko" , 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 Date: Fri, 30 Nov 2018 20:01:01 +0100 X-Clacks-Overhead: GNU Terry Pratchett X-Mailer: MailMate (1.12.2r5571) Message-ID: <0A8EBFBB-37DA-439F-A8D3-099A13128AD9@freebsd.org> In-Reply-To: References: <201811300711.wAU7B5R6084752@repo.freebsd.org> <7545b110-4930-d49f-f99f-7ec17db7e7a7@oktetlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; format=flowed; markup=markdown X-Rspamd-Queue-Id: 4061971434 X-Spamd-Result: default: False [0.16 / 15.00]; local_wl_from(0.00)[freebsd.org]; NEURAL_HAM_MEDIUM(-0.25)[-0.251,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_SPAM_SHORT(0.46)[0.460,0]; NEURAL_HAM_LONG(-0.05)[-0.049,0] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Nov 2018 19:01:11 -0000 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). 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. 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. 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. Philip -- Philip Paeps Senior Reality Engineer Ministry of Information