mbox series

[v3,0/7] Refactor the NFP PMD

Message ID 20210729134711.35870-1-heinrich.kuhn@netronome.com (mailing list archive)
Headers show
Series Refactor the NFP PMD | expand

Message

Heinrich Kuhn July 29, 2021, 1:47 p.m. UTC
This patch set restructures the NFP PMD, aligning it more with the
common layout adopted by most other PMD's. Although the changes look
fairly large, functionally nothing is added or removed from the driver
and the existing code is mostly just reorganized into the familiar
structure seen in other PMD's. Apart form adopting the common PMD layout
this change should also aid in future feature development to the NFP
PMD. The previous layout where most of the logic resided in a single
file (nfp_net.c) would have become tedious to support going forward.

v3:
* Avoid squashing the new firmware loader helper added in: 
  https://git.dpdk.org/dpdk/commit/?id=40edb9c0d36b781
* Add dependency to patch-93299

v2:
* Added missing sign-off's

---
Depends-on: patch-93299 ("net/nfp: remove compile time log")

Heinrich Kuhn (7):
  net/nfp: split rxtx headers into separate file
  net/nfp: move rxtx functions to their own file
  net/nfp: move CPP bridge to a separate file
  net/nfp: prototype common functions in header file
  net/nfp: move VF functions into new file
  net/nfp: move PF functions into new file
  net/nfp: batch file rename for consistency

 drivers/net/nfp/meson.build                   |    6 +-
 drivers/net/nfp/nfp_common.c                  | 1322 ++++++
 drivers/net/nfp/nfp_common.h                  |  413 ++
 drivers/net/nfp/nfp_cpp_bridge.c              |  392 ++
 drivers/net/nfp/nfp_cpp_bridge.h              |   36 +
 .../net/nfp/{nfp_net_ctrl.h => nfp_ctrl.h}    |    6 +-
 drivers/net/nfp/nfp_ethdev.c                  | 1067 +++++
 drivers/net/nfp/nfp_ethdev_vf.c               |  504 +++
 .../net/nfp/{nfp_net_logs.h => nfp_logs.h}    |    6 +-
 drivers/net/nfp/nfp_net.c                     | 3889 -----------------
 drivers/net/nfp/nfp_rxtx.c                    | 1002 +++++
 drivers/net/nfp/{nfp_net_pmd.h => nfp_rxtx.h} |  279 +-
 12 files changed, 4783 insertions(+), 4139 deletions(-)
 create mode 100644 drivers/net/nfp/nfp_common.c
 create mode 100644 drivers/net/nfp/nfp_common.h
 create mode 100644 drivers/net/nfp/nfp_cpp_bridge.c
 create mode 100644 drivers/net/nfp/nfp_cpp_bridge.h
 rename drivers/net/nfp/{nfp_net_ctrl.h => nfp_ctrl.h} (99%)
 create mode 100644 drivers/net/nfp/nfp_ethdev.c
 create mode 100644 drivers/net/nfp/nfp_ethdev_vf.c
 rename drivers/net/nfp/{nfp_net_logs.h => nfp_logs.h} (94%)
 delete mode 100644 drivers/net/nfp/nfp_net.c
 create mode 100644 drivers/net/nfp/nfp_rxtx.c
 rename drivers/net/nfp/{nfp_net_pmd.h => nfp_rxtx.h} (54%)

Comments

Heinrich Kuhn Aug. 13, 2021, 12:46 p.m. UTC | #1
On 2021/07/29 15:47, Heinrich Kuhn wrote:
> This patch set restructures the NFP PMD, aligning it more with the
> common layout adopted by most other PMD's. Although the changes look
> fairly large, functionally nothing is added or removed from the driver
> and the existing code is mostly just reorganized into the familiar
> structure seen in other PMD's. Apart form adopting the common PMD layout
> this change should also aid in future feature development to the NFP
> PMD. The previous layout where most of the logic resided in a single
> file (nfp_net.c) would have become tedious to support going forward.
> 
> v3:
> * Avoid squashing the new firmware loader helper added in: 
>   https://git.dpdk.org/dpdk/commit/?id=40edb9c0d36b781
> * Add dependency to patch-93299
> 
> v2:
> * Added missing sign-off's
> 
> ---

I think this refactor is a step in the right direction for the NFP PMD.
I do have a question/concern regarding future bug fixes. If this is
merged, back-porting any bug fixes will require a little bit more effort
since the code base will differ quite substantially for some time.

If there is a strong preference to avoid a situation like this we can
certainly live without this refactor

Regards,
Heinrich
Ferruh Yigit Aug. 13, 2021, 2:47 p.m. UTC | #2
On 8/13/2021 1:46 PM, Heinrich Kuhn wrote:
> 
> 
> On 2021/07/29 15:47, Heinrich Kuhn wrote:
>> This patch set restructures the NFP PMD, aligning it more with the
>> common layout adopted by most other PMD's. Although the changes look
>> fairly large, functionally nothing is added or removed from the driver
>> and the existing code is mostly just reorganized into the familiar
>> structure seen in other PMD's. Apart form adopting the common PMD layout
>> this change should also aid in future feature development to the NFP
>> PMD. The previous layout where most of the logic resided in a single
>> file (nfp_net.c) would have become tedious to support going forward.
>>
>> v3:
>> * Avoid squashing the new firmware loader helper added in: 
>>   https://git.dpdk.org/dpdk/commit/?id=40edb9c0d36b781
>> * Add dependency to patch-93299
>>
>> v2:
>> * Added missing sign-off's
>>
>> ---
> 
> I think this refactor is a step in the right direction for the NFP PMD.
> I do have a question/concern regarding future bug fixes. If this is
> merged, back-porting any bug fixes will require a little bit more effort
> since the code base will differ quite substantially for some time.
> 
> If there is a strong preference to avoid a situation like this we can
> certainly live without this refactor
> 

I didn't check the set yet, but as generic rule when there is a conflict in
backporting, stable tree maintainers ask support from patch authors, for this
case from you.
If you are OK to provide this support when needed, there should be no problem.

We have rejected only a few refactoring patches in the past, they were doing
batch syntax updates or batch renames etc.. The benefit was small comparing the
conflict/noise it brings. But I am for having meaningful refactoring.
Ferruh Yigit Aug. 17, 2021, 4:29 p.m. UTC | #3
On 7/29/2021 2:47 PM, Heinrich Kuhn wrote:
> This patch set restructures the NFP PMD, aligning it more with the
> common layout adopted by most other PMD's. Although the changes look
> fairly large, functionally nothing is added or removed from the driver
> and the existing code is mostly just reorganized into the familiar
> structure seen in other PMD's. Apart form adopting the common PMD layout
> this change should also aid in future feature development to the NFP
> PMD. The previous layout where most of the logic resided in a single
> file (nfp_net.c) would have become tedious to support going forward.
> 
> v3:
> * Avoid squashing the new firmware loader helper added in: 
>   https://git.dpdk.org/dpdk/commit/?id=40edb9c0d36b781
> * Add dependency to patch-93299
> 
> v2:
> * Added missing sign-off's
> 
> ---
> Depends-on: patch-93299 ("net/nfp: remove compile time log")
> 
> Heinrich Kuhn (7):
>   net/nfp: split rxtx headers into separate file
>   net/nfp: move rxtx functions to their own file
>   net/nfp: move CPP bridge to a separate file
>   net/nfp: prototype common functions in header file
>   net/nfp: move VF functions into new file
>   net/nfp: move PF functions into new file
>   net/nfp: batch file rename for consistency
> 

Series applied to dpdk-next-net/main, thanks.