[dpdk-dev,00/15] preparing l2fwd for eventmode additions

Message ID 1528477766-15788-1-git-send-email-anoob.joseph@caviumnetworks.com (mailing list archive)
Headers
Series preparing l2fwd for eventmode additions |

Message

Anoob Joseph June 8, 2018, 5:09 p.m. UTC
  This patchset modularizes l2fwd application to prepare it for eventmode
additions. This patchset doesn't change the code flow or logic, except
for few minor improvements. Some of the newly added functions are used
in just one place, but is added for efficient usage with eventmode.

Anoob Joseph (15):
  examples/l2fwd: add new header to move common code
  examples/l2fwd: move macro definitions to common header
  examples/l2fwd: move structure definitions to common header
  examples/l2fwd: move globally accessed vars to common header
  examples/l2fwd: add missing space
  examples/l2fwd: fix lines exceeding 80 char limit
  examples/l2fwd: move dataplane code to new file
  examples/l2fwd: remove unused header includes
  examples/l2fwd: move drain buffers to new function
  examples/l2fwd: optimize check for master core
  examples/l2fwd: move periodic tasks to new function
  examples/l2fwd: skip timer updates for non master cores
  examples/l2fwd: move pkt send code to a new function
  examples/l2fwd: use fprint instead of printf for usage print
  examples/l2fwd: improvements to the usage print

 examples/l2fwd/Makefile       |   1 +
 examples/l2fwd/l2fwd_common.h |  62 ++++++++++
 examples/l2fwd/l2fwd_worker.c | 249 +++++++++++++++++++++++++++++++++++++
 examples/l2fwd/l2fwd_worker.h |  16 +++
 examples/l2fwd/main.c         | 276 ++++++------------------------------------
 5 files changed, 363 insertions(+), 241 deletions(-)
 create mode 100644 examples/l2fwd/l2fwd_common.h
 create mode 100644 examples/l2fwd/l2fwd_worker.c
 create mode 100644 examples/l2fwd/l2fwd_worker.h
  

Comments

Anoob Joseph June 14, 2018, 11:48 a.m. UTC | #1
This patchset modularizes l2fwd application to prepare it for eventmode
additions. This patchset doesn't change the code flow or logic, except
for few minor improvements. Some of the newly added functions are used
in just one place, but is added for efficient usage with eventmode.

v1:
* Fix all checkpatch reported issues

Anoob Joseph (15):
  examples/l2fwd: add new header to move common code
  examples/l2fwd: move macro definitions to common header
  examples/l2fwd: move structure definitions to common header
  examples/l2fwd: move globally accessed vars to common header
  examples/l2fwd: add missing space
  examples/l2fwd: fix lines exceeding 80 char limit
  examples/l2fwd: move dataplane code to new file
  examples/l2fwd: remove unused header includes
  examples/l2fwd: move drain buffers to new function
  examples/l2fwd: optimize check for master core
  examples/l2fwd: move periodic tasks to new function
  examples/l2fwd: skip timer updates for non master cores
  examples/l2fwd: move pkt send code to a new function
  examples/l2fwd: use fprint instead of printf for usage print
  examples/l2fwd: improvements to the usage print

 examples/l2fwd/Makefile       |   1 +
 examples/l2fwd/l2fwd_common.h |  63 ++++++++++
 examples/l2fwd/l2fwd_worker.c | 249 +++++++++++++++++++++++++++++++++++++
 examples/l2fwd/l2fwd_worker.h |  16 +++
 examples/l2fwd/main.c         | 276 ++++++------------------------------------
 5 files changed, 364 insertions(+), 241 deletions(-)
 create mode 100644 examples/l2fwd/l2fwd_common.h
 create mode 100644 examples/l2fwd/l2fwd_worker.c
 create mode 100644 examples/l2fwd/l2fwd_worker.h
  
Anoob Joseph June 19, 2018, 10:04 a.m. UTC | #2
Hi Bruce, Pablo,

Any comments on this series?

Thanks,
Anoob

On 14/06/18 17:18, Anoob Joseph wrote:
> This patchset modularizes l2fwd application to prepare it for eventmode
> additions. This patchset doesn't change the code flow or logic, except
> for few minor improvements. Some of the newly added functions are used
> in just one place, but is added for efficient usage with eventmode.
>
> v1:
> * Fix all checkpatch reported issues
>
> Anoob Joseph (15):
>    examples/l2fwd: add new header to move common code
>    examples/l2fwd: move macro definitions to common header
>    examples/l2fwd: move structure definitions to common header
>    examples/l2fwd: move globally accessed vars to common header
>    examples/l2fwd: add missing space
>    examples/l2fwd: fix lines exceeding 80 char limit
>    examples/l2fwd: move dataplane code to new file
>    examples/l2fwd: remove unused header includes
>    examples/l2fwd: move drain buffers to new function
>    examples/l2fwd: optimize check for master core
>    examples/l2fwd: move periodic tasks to new function
>    examples/l2fwd: skip timer updates for non master cores
>    examples/l2fwd: move pkt send code to a new function
>    examples/l2fwd: use fprint instead of printf for usage print
>    examples/l2fwd: improvements to the usage print
>
>   examples/l2fwd/Makefile       |   1 +
>   examples/l2fwd/l2fwd_common.h |  63 ++++++++++
>   examples/l2fwd/l2fwd_worker.c | 249 +++++++++++++++++++++++++++++++++++++
>   examples/l2fwd/l2fwd_worker.h |  16 +++
>   examples/l2fwd/main.c         | 276 ++++++------------------------------------
>   5 files changed, 364 insertions(+), 241 deletions(-)
>   create mode 100644 examples/l2fwd/l2fwd_common.h
>   create mode 100644 examples/l2fwd/l2fwd_worker.c
>   create mode 100644 examples/l2fwd/l2fwd_worker.h
>
  
Bruce Richardson June 19, 2018, 10:09 a.m. UTC | #3
On Tue, Jun 19, 2018 at 03:34:29PM +0530, Anoob Joseph wrote:
> Hi Bruce, Pablo,
> 
> Any comments on this series?
> 
> Thanks,
> Anoob
> 
> On 14/06/18 17:18, Anoob Joseph wrote:
> > This patchset modularizes l2fwd application to prepare it for eventmode
> > additions. This patchset doesn't change the code flow or logic, except
> > for few minor improvements. Some of the newly added functions are used
> > in just one place, but is added for efficient usage with eventmode.
> > 
> > v1:
> > * Fix all checkpatch reported issues
> > 
My main concern here is how much this eventmode addition is going to
complicate the l2fwd example. l2fwd has always been a pretty basic example
app to get users started on the basics of DPDK use, and I'm not sure how
much we want to move away from that. Is this eventmode-l2fwd better being a
separate app, to allow l2fwd to be kept as simple as it can be? 

Looking for more thoughts from others here, since it's a community decision
as to the scope of the examples.

/Bruce
  
Anoob Joseph June 19, 2018, 2:07 p.m. UTC | #4
Hi Bruce,


Thanks for the feedback. Please see inline.


+ Hemant, Nikhil, Sunil, Gage, Harry, Narender, Pavan, Thomas, Akhil


On 19/06/18 15:39, Bruce Richardson wrote:
> On Tue, Jun 19, 2018 at 03:34:29PM +0530, Anoob Joseph wrote:
>> Hi Bruce, Pablo,
>>
>> Any comments on this series?
>>
>> Thanks,
>> Anoob
>>
>> On 14/06/18 17:18, Anoob Joseph wrote:
>>> This patchset modularizes l2fwd application to prepare it for eventmode
>>> additions. This patchset doesn't change the code flow or logic, except
>>> for few minor improvements. Some of the newly added functions are used
>>> in just one place, but is added for efficient usage with eventmode.
>>>
>>> v1:
>>> * Fix all checkpatch reported issues
>>>
> My main concern here is how much this eventmode addition is going to
> complicate the l2fwd example. l2fwd has always been a pretty basic example
> app to get users started on the basics of DPDK use, and I'm not sure how
> much we want to move away from that. Is this eventmode-l2fwd better being a
> separate app, to allow l2fwd to be kept as simple as it can be?
>
> Looking for more thoughts from others here, since it's a community decision
> as to the scope of the examples.
>
> /Bruce
The eventmode helper abstracts most of the changes required by the 
application to run in eventmode. This was taken up following the 
comments on a patch submitted by Sunil(sunil.kori@nxp.com).
http://patches.dpdk.org/patch/37955/

With eventmode helper, an application can be moved to eventmode with 
minimal changes. For l2fwd, the key patch which enables eventmode is,

http://patches.dpdk.org/patch/40920/
[The aforementioned patch is dependent on this patch series]

The bulk of the code in this patch(40920) is adding multiple event mode 
worker functions.The existing init code and poll mode worker is barely 
touched. Multiple workers were introduced because a single event mode 
worker would not have made the best use of the varying capabilities of 
event devices.

Single event mode worker could've demonstrated how minimal the changes 
can be. But the ability to register multiple workers, fine tuned for 
varying capabilities, is a good feature to have since it will enable 
applications to utilize the full potential of the hardware.

Eventmode helper patch series:
http://patches.dpdk.org/project/dpdk/list/?series=61

The rules that were followed while drafting eventmode helper were very 
simple,
1. Move any code common to multiple applications to eventmode helper
2. Expose all capabilities of the devices involved (event & eth devs)
3. Minimize changes to the existing code

For l2fwd we can opt for a new eventmode-l2fwd app, but this might not 
work for more complicated apps like l3fwd & ipsec-secgw. L2fwd app will 
stay the same even with the eventmode additions. It will still be a 
quick-start, easy-to-use app. In addition to demonstrating DPDK, it will 
also be able to demonstrate how easily an app can be made to run in 
eventmode, using the helper functions.

With more event adapters getting added (tx adapter, crypto adapter, 
timer adapter etc), the helper will prove useful in abstracting the 
complex configuration options exposed by adapters. Similar changes would 
be required in other example apps, and the additions in l2fwd is to 
finalize on the approach.

The current patch series just re-factors the code with couple of patches 
fixing preexisting checkpatch issues. The rest of the changes are split 
into individual patches for ease of review and testing. Hence the large 
number of patches.

Thanks,
Anoob
  
Anoob Joseph July 3, 2018, 1:16 p.m. UTC | #5
Hi,

Gentle reminder!

Thanks,
Anoob

On 19-06-2018 19:37, Anoob Joseph wrote:
> External Email
>
> Hi Bruce,
>
>
> Thanks for the feedback. Please see inline.
>
>
> + Hemant, Nikhil, Sunil, Gage, Harry, Narender, Pavan, Thomas, Akhil
>
>
> On 19/06/18 15:39, Bruce Richardson wrote:
>> On Tue, Jun 19, 2018 at 03:34:29PM +0530, Anoob Joseph wrote:
>>> Hi Bruce, Pablo,
>>>
>>> Any comments on this series?
>>>
>>> Thanks,
>>> Anoob
>>>
>>> On 14/06/18 17:18, Anoob Joseph wrote:
>>>> This patchset modularizes l2fwd application to prepare it for 
>>>> eventmode
>>>> additions. This patchset doesn't change the code flow or logic, except
>>>> for few minor improvements. Some of the newly added functions are used
>>>> in just one place, but is added for efficient usage with eventmode.
>>>>
>>>> v1:
>>>> * Fix all checkpatch reported issues
>>>>
>> My main concern here is how much this eventmode addition is going to
>> complicate the l2fwd example. l2fwd has always been a pretty basic 
>> example
>> app to get users started on the basics of DPDK use, and I'm not sure how
>> much we want to move away from that. Is this eventmode-l2fwd better 
>> being a
>> separate app, to allow l2fwd to be kept as simple as it can be?
>>
>> Looking for more thoughts from others here, since it's a community 
>> decision
>> as to the scope of the examples.
>>
>> /Bruce
> The eventmode helper abstracts most of the changes required by the
> application to run in eventmode. This was taken up following the
> comments on a patch submitted by Sunil(sunil.kori@nxp.com).
> http://patches.dpdk.org/patch/37955/
>
> With eventmode helper, an application can be moved to eventmode with
> minimal changes. For l2fwd, the key patch which enables eventmode is,
>
> http://patches.dpdk.org/patch/40920/
> [The aforementioned patch is dependent on this patch series]
>
> The bulk of the code in this patch(40920) is adding multiple event mode
> worker functions.The existing init code and poll mode worker is barely
> touched. Multiple workers were introduced because a single event mode
> worker would not have made the best use of the varying capabilities of
> event devices.
>
> Single event mode worker could've demonstrated how minimal the changes
> can be. But the ability to register multiple workers, fine tuned for
> varying capabilities, is a good feature to have since it will enable
> applications to utilize the full potential of the hardware.
>
> Eventmode helper patch series:
> http://patches.dpdk.org/project/dpdk/list/?series=61
>
> The rules that were followed while drafting eventmode helper were very
> simple,
> 1. Move any code common to multiple applications to eventmode helper
> 2. Expose all capabilities of the devices involved (event & eth devs)
> 3. Minimize changes to the existing code
>
> For l2fwd we can opt for a new eventmode-l2fwd app, but this might not
> work for more complicated apps like l3fwd & ipsec-secgw. L2fwd app will
> stay the same even with the eventmode additions. It will still be a
> quick-start, easy-to-use app. In addition to demonstrating DPDK, it will
> also be able to demonstrate how easily an app can be made to run in
> eventmode, using the helper functions.
>
> With more event adapters getting added (tx adapter, crypto adapter,
> timer adapter etc), the helper will prove useful in abstracting the
> complex configuration options exposed by adapters. Similar changes would
> be required in other example apps, and the additions in l2fwd is to
> finalize on the approach.
>
> The current patch series just re-factors the code with couple of patches
> fixing preexisting checkpatch issues. The rest of the changes are split
> into individual patches for ease of review and testing. Hence the large
> number of patches.
>
> Thanks,
> Anoob