From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id DA54CA0096 for ; Fri, 7 Jun 2019 14:53:33 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8499F1BBDA; Fri, 7 Jun 2019 14:53:33 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id E6EF01BBD3 for ; Fri, 7 Jun 2019 14:53:31 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 24F5F308626C; Fri, 7 Jun 2019 12:53:25 +0000 (UTC) Received: from [10.36.112.53] (ovpn-112-53.ams2.redhat.com [10.36.112.53]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 06D0B7D54C; Fri, 7 Jun 2019 12:53:20 +0000 (UTC) To: "Rong, Leyi" , "Zhang, Qi Z" , "Stillwell Jr, Paul M" Cc: "dev@dpdk.org" References: <20190604054248.68510-1-leyi.rong@intel.com> <47ACC7359E973C41ACB0C2477632BC72518B7F28@SHSMSX103.ccr.corp.intel.com> From: Maxime Coquelin Message-ID: <65242236-aba7-0b14-9bdc-685fa1e5d6d8@redhat.com> Date: Fri, 7 Jun 2019 14:53:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <47ACC7359E973C41ACB0C2477632BC72518B7F28@SHSMSX103.ccr.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Fri, 07 Jun 2019 12:53:31 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 00/49] shared code update X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Leyi, On 6/6/19 7:44 AM, Rong, Leyi wrote: > >> -----Original Message----- >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >> Sent: Wednesday, June 5, 2019 12:56 AM >> To: Rong, Leyi ; Zhang, Qi Z >> Cc: dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 00/49] shared code update >> >> Hi Leyi, >> >> On 6/4/19 7:41 AM, Leyi Rong wrote: >>> Main changes: >>> 1. Advanced switch rule support. >>> 2. Add more APIs for tunnel management. >>> 3. Add some minor features. >>> 4. Code clean and bug fix. >> >> In order to ease the review process, I think it would be much better to split this series in multiple ones, by features. >> Otherwise, it is more difficult to keep track if comments are taken into account in the next revision. >> >> Also, it is suggested to put the fixes first in the series to ease the backporting. >> >> Thanks, >> Maxime > > +Paul, > > Hello Maxime, > Thanks for all your constructive comments, but we do the same process for the CVL shared code update to DPDK upstream on the previous release. > This series of patches are extracted/reorganized/squashed from the ND released packages, which the shared code difference between 1905 and 1908 can be more than 200 commits from the original shared code repo. > > IMHO, there might be some reasons for take all these patches into one patchset. > - the patchset try to keeps the history order as the commits in the original shared code repo. > - the relatively behind patch in the patchset may have dependency on the front patches. > - it's difficult to split this series into multiple ones, since the patches are irregular and squashed. On the other hand, it means we should just apply the series without even reviewing it as it would not be taken into account. I personally think this is not a sane practice. This is not the case of this series, which is almost good to me except some missing errors handling, but imagine there is a security issue in it, should we just apply it as-is not to diverge from your internal code base and wait for the next shared code release to get the fix? Note that my comment is not intended at Intel drivers specifically, as it seems a common practice to have the base driver not reviewed and applied as-is. Best regards, Maxime > > > Best Regards, > Leyi Rong >