mbox series

[00/49] shared code update

Message ID 20190604054248.68510-1-leyi.rong@intel.com (mailing list archive)
Headers
Series shared code update |

Message

Leyi Rong June 4, 2019, 5:41 a.m. UTC
  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.

Leyi Rong (49):
  net/ice/base: add macro for rounding up
  net/ice/base: update standard extr seq to include DIR flag
  net/ice/base: add API to configure MIB
  net/ice/base: add more recipe commands
  net/ice/base: add funcs to create new switch recipe
  net/ice/base: programming a new switch recipe
  net/ice/base: replay advanced rule after reset
  net/ice/base: code for removing advanced rule
  net/ice/base: add lock around profile map list
  net/ice/base: save and post reset replay q bandwidth
  net/ice/base: rollback AVF RSS configurations
  net/ice/base: move RSS replay list
  net/ice/base: cache the data of set PHY cfg AQ in SW
  net/ice/base: refactor HW table init function
  net/ice/base: add compatibility check for package version
  net/ice/base: add API to init FW logging
  net/ice/base: use macro instead of magic 8
  net/ice/base: move and redefine ice debug cq API
  net/ice/base: separate out control queue lock creation
  net/ice/base: add helper functions for PHY caching
  net/ice/base: added sibling head to parse nodes
  net/ice/base: add and fix debuglogs
  net/ice/base: add support for reading REPC statistics
  net/ice/base: move VSI to VSI group
  net/ice/base: forbid VSI to remove unassociated ucast filter
  net/ice/base: add some minor features
  net/ice/base: call out dev/func caps when printing
  net/ice/base: add some minor features
  net/ice/base: cleanup update link info
  net/ice/base: add rd64 support
  net/ice/base: track HW stat registers past rollover
  net/ice/base: implement LLDP persistent settings
  net/ice/base: check new FD filter duplicate location
  net/ice/base: correct UDP/TCP PTYPE assignments
  net/ice/base: calculate rate limit burst size correctly
  net/ice/base: add lock around profile map list
  net/ice/base: fix Flow Director VSI count
  net/ice/base: use more efficient structures
  net/ice/base: slightly code update
  net/ice/base: code clean up
  net/ice/base: cleanup ice flex pipe files
  net/ice/base: change how VMDq capability is wrapped
  net/ice/base: refactor VSI node sched code
  net/ice/base: add some minor new defines
  net/ice/base: add 16-byte Flex Rx Descriptor
  net/ice/base: add vxlan/generic tunnel management
  net/ice/base: enable additional switch rules
  net/ice/base: allow forward to Q groups in switch rule
  net/ice/base: changes for reducing ice add adv rule time

 drivers/net/ice/base/ice_adminq_cmd.h    |  128 +-
 drivers/net/ice/base/ice_common.c        |  618 ++++--
 drivers/net/ice/base/ice_common.h        |   32 +-
 drivers/net/ice/base/ice_controlq.c      |  247 ++-
 drivers/net/ice/base/ice_controlq.h      |    4 +-
 drivers/net/ice/base/ice_dcb.c           |   74 +-
 drivers/net/ice/base/ice_dcb.h           |   12 +-
 drivers/net/ice/base/ice_fdir.c          |   11 +-
 drivers/net/ice/base/ice_fdir.h          |    1 -
 drivers/net/ice/base/ice_flex_pipe.c     | 1251 ++++++------
 drivers/net/ice/base/ice_flex_pipe.h     |   74 +-
 drivers/net/ice/base/ice_flex_type.h     |   54 +-
 drivers/net/ice/base/ice_flow.c          |  447 +++-
 drivers/net/ice/base/ice_flow.h          |   26 +-
 drivers/net/ice/base/ice_lan_tx_rx.h     |   31 +-
 drivers/net/ice/base/ice_nvm.c           |   18 +-
 drivers/net/ice/base/ice_osdep.h         |   23 +
 drivers/net/ice/base/ice_protocol_type.h |    7 +-
 drivers/net/ice/base/ice_sched.c         |  214 +-
 drivers/net/ice/base/ice_sched.h         |   24 +-
 drivers/net/ice/base/ice_switch.c        | 2382 +++++++++++++++++++++-
 drivers/net/ice/base/ice_switch.h        |   64 +-
 drivers/net/ice/base/ice_type.h          |   95 +-
 drivers/net/ice/ice_ethdev.c             |    4 +-
 24 files changed, 4478 insertions(+), 1363 deletions(-)
  

Comments

Maxime Coquelin June 4, 2019, 4:56 p.m. UTC | #1
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
  
Leyi Rong June 6, 2019, 5:44 a.m. UTC | #2
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Wednesday, June 5, 2019 12:56 AM
> To: Rong, Leyi <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> 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.


Best Regards,
Leyi Rong
  
Maxime Coquelin June 7, 2019, 12:53 p.m. UTC | #3
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 <leyi.rong@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
>> 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
>