svn commit: r341327 - head/sys/dev/sfxge
Philip Paeps
philip at freebsd.org
Fri Nov 30 19:01:11 UTC 2018
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).
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
More information about the svn-src-all
mailing list