Hi everyone,

First of all thanks Owen for stepping forward with this RFC.
Few thoughts on this subject below.
Konstantin   

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, April 14, 2022 12:59 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: FW: [PATCH v1 0/4] [RFC] Testpmd RPC API
>
>
>
> From: Owen Hilyard <ohilyard@iol.unh.edu>
> Sent: Wednesday, April 13, 2022 1:47 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>
> Cc: dpdk-dev <dev@dpdk.org>; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [PATCH v1 0/4] [RFC] Testpmd RPC API
>
> If so, I think, gRPC service would be along with existing testpmd functions, like start_packet_forwarding().
>
> It was my intention to re-use existing functions. I used the ACL tests as an example because they are more self-contained then Testpmd,
> which made creating the proof of concept much easier.
>
> Also, We don't need to rewrite the existing testpmd, Instead, RPC service, we can add in existing app/test-pmd/
>
> The reason that I split out the services is that there doesn't seem to be a way to produce multiple binaries without re-writing that section of
> the build system. I wanted to avoid the hard requirement of having a C++ compiler available in order to be able to use testpmd, since that
> may affect what platforms Testpmd can run on and I want to avoid this being any kind of breaking change. If we decide to go the route of
> putting it all in a single application, we would need to conditionally enable the gRPC service at build time. Meson's current lack of support
> for conditionally detecting compilers causes issues here.
>
> I think, DPDK has only one interactive test case which is testpmd,
>
> Could you point me to that test case? Either invocation or source is ok. I can't see anything that would lead me to assume use of testpmd in
> "meson test --list". To my knowledge, all of the test cases that use testpmd are in DTS. If there is a test that uses testpmd but is not part of
> DTS, I think it would be a candidate for moving into DTS assuming it's not a unit test.
>
> How you are planning to do noninteractive test cases?
>
> I'm not planning to make any change to unit testing, you can read more about how testing is currently conducted
> here: https://www.dpdk.org/blog/2021/07/05/dpdk-testing-approaches/
>
> If there is a unit test that involves testpmd, there are two possibilities.
> 1. If we are making a separate application for Testpmd with the gRPC api, then nothing changes except for possibly changing where some
> of the testpmd source lives in order to enable code reuse between the two applications.
> 2. If gRPC is being added to Testpmd, then the unit test should still function as it previously did if I do any necessary refactoring as correctly.
>
> I think, key should be leveraging existing test cases as much as possible and make easy for developers to add new test cases.
>
> That is part of the reason why I want to be able to do this. Adding a new test in DTS is very easy if the functionality needed already exists in
> Testpmd. If the functionality does not exist, then adding the test becomes difficult, due to the required modifications to the Testpmd lexer
> and parser to accommodate the new command. My plan is to leave unit testing in C, but help make it easier to expose C functions to Python
> for integration testing. This gives us the best of both worlds in terms of access to DPDK and the ability to use a high-level language to write
> the tests.
>
> On Tue, Apr 12, 2022 at 2:07 AM Jerin Jacob <mailto:jerinjacobk@gmail.com> wrote:
> On Mon, Apr 11, 2022 at 11:19 PM Owen Hilyard <mailto:ohilyard@iol.unh.edu> wrote:
> >>
> >> scheme is probably over-engineered
> >
> >
> > I tried my hardest to keep this as simple as possible. The requirements imposed by DTS being a distributed system in Python restricted
> what I could do a lot. Needing to be compatible with DPDK's license also got rid of a lot of options. Binding generators are made for simple
> projects, and DPDK is not a simple project. There were some other options related to choice in the RPC framework, but very few RPC
> protocols seem to work well with C and be usable from Python, which is why I ended up using C++ with gRPC. Most of the code in
> api_impl.cc is taken from /app/test-acl/main.c, and the new part is mostly the C++ class at the bottom. Overall, this proposal comes out to
> ~100 lines of new C++, 9 lines of C, 12 lines of gRPC Protobuf and 100 lines of Meson. gRPC may be able to do a lot more than I need it to
> for the proof of concept, but many of the features that are not used, like bi-directional streaming, become very useful in writing more
> complicated tests. Overall, this solution is probably more capable than we need it to be, but I think that those extra capabilities don't come
> at a very large cost.
>
>
> Now it is clear, I was carried away with the POC test application and
> I was not knowing existing DTS tests are based on python
>
> Is below a fair summary?
>
> 1) DPDK has interactive test cases and no interactive test cases.
>
> For The interactive test case like testpmd, I agree that we can enable
> RPC service via gRPC server in C++ as  and client in Python, and
> something along the lines of exposing the existing test-pmd command
> line function as service
> to avoid command line parsing and reuse the existing python test suite.
>
> If so, I think, gRPC service would be along with existing testpmd
> functions, like start_packet_forwarding(). Also, We don't need to
> rewrite the existing testpmd,
> Instead, RPC service, we can add in existing app/test-pmd/ and hook to
> existing core testpmd functions to bypass the command-line parsing in
> C and control from python client as needed as service.
>
> Also, I agree that pulling in gRPC C++ server boilerplate and hooking
> to C functions is a good idea as it is the best C-based RPC scheme
> available today.
>
> 2)I think, DPDK has only one interactive test case which is testpmd,
> Remaining test cases are non-interactive, non-interactive test cases
> can simply run over ssh with passwordless login. Right?
> Do we need gRPC for that? Will the following scheme suffice? If not,
> How you are planning to do noninteractive test cases?
> i.e
> a)Copy test to target
> b) result=`ssh username@IP /path/to/testapp/in/target`
>
> I think, key should be leveraging existing test cases as much as
> possible and make easy for developers to add new test cases.
>
>
> >>
> >> Now that, Test code is also part of DPDK.
> >
> >
> > DTS is pure python. I tried to use FFI to call directly into DPDK from Python and then use xmlrpc from the python standard library. As
> mentioned in the writeup, I couldn't find a binding generator that would properly handle DPDK's allocators, which made it so that anything
> passed to DPDK using python was allocated using the system malloc. I don't think it is wise to attempt to programmatically re-write the
> generated code to allow for custom allocators. The original reason for needing to have DTS and DPDK in the same repository was so that
> tests could be committed and run alongside the feature patch.
> >
> >> Interactive - Testpmd one, I believe, Feeding stdin programmatically would suffice to test all the combinations.
> >
> >
> > One of the issues this is trying to address is that human-readable strings are a poor way to pass complex information between two
> programs. DTS is a distributed system, and it can have up to 3 physical servers involved in any given test. This means that it's not stdin via a
> pipe, it's an entire SSH session. This adds a noticeable amount of overhead when trying to send and verify the result of sending 1,000+
> packets, since the lack of structured output means each packet must be checked before the next can be sent. This might be solvable by
> adding a structured output mode to testpmd, but that would involve committing to writing output twice for every function in testpmd
> forever.
> >
> >> We need to add all test cases in this model and we need to maintain two sets of programs.(Traditional tests and gRPC model-based
> tests).
> >
> >
> > Assuming by traditional tests you mean the unit tests run by Meson, I would argue that we are already maintaining 2 kinds of tests. The
> unit tests, and the python-based DTS tests. My intention is to create a thin wrapper around DPDK that would be exposed via gRPC, like you
> see here, and use that as midware. Then, we would have two front-ends. Testpmd, which takes text and then calls midware as it does now,
> and the gRPC frontend, which parses messages from the RPC server and runs the midware. This would enable testpmd to still be used to
> sanity check a DPDK installation, but we would not need to continually expand Testpmd. The primary issue is that, right now, anything not
> included in Testpmd is not really testable by DTS. This includes portions of the RTE Flow API, which was part of my reason for proposing this.
> The RTE Flow API would, in my estimation, if fully implemented into Testpmd, probably add at least another 10,000 lines of code. As
> mentioned in my proposal, Testpmd already does more parsing and lexing than it does interaction with DPDK by line count. Also, since I am
> proposing making this a separate application, we would be able to gradually migrate the tests inside of DTS. This would have no effect on
> anything except for Testpmd, the new application and the addition of a flag to toggle the use of a C++ compiler.
> >
> > I'm not sure exactly what you mean by gRPC model-based tests. gRPC uses classes to model services, but for this usecase we are
> essentially using it to transfer function arguments across the internet and then pass the return value back. Any RPC framework would
> function similarly if I ignored the restrictions of which languages to use, and the choice is not important to how tests are conducted. Put
> another way, how you write a test for DTS will not change much if you are using this or testpmd, it's just how you transfer data and get it
> back that I want to change.

- In general I think it is a good idea to adding gRPC binding to testpmd to expose/improve testing automation.
  Though I think we shouldn’t remove existing CLI interface.
  Ideally I’d like to have both – CLI and gRPC for all commands.
  Don’t know how realistic is that, but at least for major commands -  port/queue configure, start/stop, etc.
- Conditional compilation (new meson flag or so) is probably good enough for this case.   
- About RFC itself - I understand that you choose testacl for simplicity, but in fact, it is a standalone application
  that has not much common with testpmd itself and the problems that you mentioned:
  interactive commands, parameter and results parsing, etc.
  Would it be possible to try implement something more realistic with testpmd itself,
  like simple test-pmd port/queue configure, start,  result collection, etc.?
  To get a better idea how it is going to work and how complicated it would be.