On Mon, Aug 13, 2018 at 09:29:01PM +0530, Joseph, Anoob wrote: > Hi Bruce, > > The reason why l2fwd was chosen was to allow everyone to chip in their ideas > while preparing the framework. > This framework would be extended to other applications, hence needed enough > inputs before expanding to complex applications. If your suggestion is to > make l3fwd event driven first, I'll start looking in that direction.
Seems good to me, if others don't have an issue with it. > > As for l2fwd, I'm fine with moving event-mode additions to a new app. But > with the present approach, the app would run in both event mode and poll > mode. > > Your thoughts on renaming the existing app to l2fwd-poll and the proposed > app as l2fwd? > > Thanks, > Anoob I'm not sure about the name "poll", I think "ethdev" and "eventdev" should be the suffixes, if we want to move in that direction. However, my preference would be to leave l2fwd as-is, and to have a comment at the top of the source file, and note in the documentation along the lines of: "This example demonstrates basic l2 forwarding using ethdev primitives. To see the same use-case implemented using event-based primitives, see the 'l2fwd-eventdev' example". As I said before, my main concern is to keep the basic examples short and readable. /Bruce > On 13-08-2018 14:57, Bruce Richardson wrote: > > External Email > > > > On Mon, Aug 13, 2018 at 12:52:19PM +0530, Joseph, Anoob wrote: > > > Hi Bruce, Pablo, > > > > > > If there are no more issues about the approach, can you review the patches > > > and give the feedback? > > > > > > Please do note that this series doesn't add any event mode specific code. > > > That will come as a different patch series after incorporating Jerin's > > > comments. > > > > > > Thanks, > > > Anoob > > My main concern is with l2fwd, rather than l3fwd, which is already fairly > > complicated. I could see l3fwd being updated to allow an eventmode without > > too many problems. > > > > With l2fwd, the only issue I have is with the volume of code involved. > > l2fwd is currently a very simple application which fits in a single file. > > With these updates it's no longer such a simple, approachable app, rather > > it becomes one which takes a bit of studying a switching between files to > > fully understand. The data path is only a very small part of the app, so by > > adding an event-based path to the same app we have very little code saving. > > Therefore, I think having a separate l2fwd-eventdev would be better for > > this case. Two simpler to understand apps is better than one more > > complicated on IMHO. > > > > My 2c. > > > > /Bruce > > > > > On 02-08-2018 13:49, Ananyev, Konstantin wrote: > > > > External Email > > > > > > > > Hi everyone, > > > > > > > > > > > > In order to get this series accepted, we need more discussions > > > > > > > > with more people involved. > > > > > > > > So it will miss 18.08. > > > > > > > > > > > > > > > > It can be discussed in a more global discussion about examples > > > > > > > > maintenance. > > > > > > > > If discussion does not happen, you can request it to the > > > > > > > > technical board. > > > > > > > > > > > > > > > Event dev framework and various adapters enable multiple packet > > > > > > > handling > > > > > > > schemes, as opposed to the traditional polling on queues. But > > > > > > > these > > > > > > > features are not integrated into any established example > > > > > > > application. > > > > > > > There are specific example applications for event dev etc, which > > > > > > > can be > > > > > > > used to analyze an event device or a particular eventdev adapter, > > > > > > > but > > > > > > > there is no standard application which can be used to compare the > > > > > > > real > > > > > > > world performance for a system when it's using event device for > > > > > > > packet > > > > > > > handling and when it's done via polling on queues. > > > > > > > > > > > > > > The following patch submitted by Sunil was looking to address > > > > > > > this issue > > > > > > > with l3fwd, > > > > > > > https://mails.dpdk.org/archives/dev/2018-March/093131.html > > > > > > > > > > > > > > Bruce & Jerin reviewed the patch and suggested the addition of > > > > > > > helper > > > > > > > functions to abstract the event mode additions in applications, > > > > > > > https://mails.dpdk.org/archives/dev/2018-April/096879.html > > > > > > > > > > > > > > This effort of adding helper functions for eventmode was taken up > > > > > > > following the above suggestion. The idea is to add eventmode > > > > > > > without > > > > > > > touching the existing code path. All the eventmode specific > > > > > > > additions > > > > > > > would go into library so that these need not be repeated for every > > > > > > > application. And since there is no change in the existing code > > > > > > > path, > > > > > > > performance for any vendor should not have any impact with the > > > > > > > additions. > > > > > > > > > > > > > > The scope of this effort has increased since the submission, as > > > > > > > now we > > > > > > > have Tx adapter as well. Sunil & Konstantin had clarified their > > > > > > > concerns, and gave green flag to this approach. > > > > > > > https://mails.dpdk.org/archives/dev/2018-June/105730.html > > > > > > > https://mails.dpdk.org/archives/dev/2018-July/106453.html > > > > > > > > > > > > > > I guess Bruce was opening this question to the community. For > > > > > > > compute > > > > > > > intense applications like ipsec-secgw, eventmode might be the > > > > > > > right > > > > > > > approach in the first place. Such complex applications would need > > > > > > > a > > > > > > > scheduler to perform dynamic load balancing. Addition of > > > > > > > eventmode in > > > > > > > l2fwd was to float around the idea which can then be scaled for > > > > > > > more > > > > > > > complex applications. > > > > > > > > > > > > > > If maintainers doesn't have any objection to this, I'm fine with > > > > > > > adding > > > > > > > this in the next release. > > > > > > > > > > > > > > Thanks, > > > > > > > Anoob > > > > > > It is important that DPDK has good examples of how to use existing > > > > > > frameworks and libraries. These applications are what most customers > > > > > > build their applications from and they provide basis for testing. > > > > > > > > > > > > The DPDK needs to continue to support multiple usage models. This > > > > > > is one of its strong points. I would rather leave existing l2fwd > > > > > > and l3fwd alone and instead make new examples that use the > > > > > > frameworks. > > > > > > If nothing else haveing l2fwd and l2fwd-eventdev would allow for > > > > > > performance comparisons. > > > > > Unlike other applications example, there wont be any change in packet > > > > > processing functions in eventdev vs poll mode case. Only worker > > > > > schematics will change and that can be moved to separated files. > > > > > something like worker_poll.c and worker_event.c and both of them > > > > > use common packet processing functions using mbuf. > > > > > > > > > > The only disadvantage of having separate application would be packet > > > > > processing code duplication. Which is non trivial for l3fwd, IPSec > > > > > application IMO. > > > > Personally I am ok with original design suggestion: > > > > keep packet processing code common, that would be used by both poll and > > > > event modes. > > > > We could just have a command-line parameter in which mode the app will > > > > run. > > > > Another alternative - generate two binaries l2fwd-poll, l2fwd-event (or > > > > so). > > > > Konstantin > > > > > # Are we fine with code duplication in example application like l3fwd > > > > > and > > > > > IPSec? > > > > > # if yes, Are we fine with keeping l2fwd _as is_ to reduce the > > > > > complexity and l2fwd-eventdev supports both modes wherever possible? > > > > > > > > > > > As the number of examples increases, probably also need to have > > > > > > a roadmap or decision chart to explain the advangage/disadvantage > > > > > > of each architecture. > > > > > > >
