Re: [dpdk-dev] [PATCH v1 00/15] preparing l2fwd for eventmode additions

[Anoob Joseph <Anoob.Joseph@caviumnetworks.com>]

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