Re: [PATCH v2] app/testpmd: show output of commands read from file
From: Ferruh Yigit <hidden>
Date: 2024-08-22 16:53:37
On 8/22/2024 11:41 AM, Bruce Richardson wrote:
Testpmd supports the "--cmdline-file" parameter to read a set of initial commands from a file. However, the only indication that this has been done successfully on startup is a single-line message, no output from the commands is seen.
For user I think it makes sense to see the command [1], only concern is if someone parsing testpmd output may be impacted on this, although I expect it should be trivial to update the relevant parsing. [1] Btw, I can still see the command output, I assume because command does the printf itself, for example for 'show port summary 0' command: - before patch: ... Number of available ports: 2 Port MAC Address Name Driver Status Link 0 xx:xx:xx:xx:xx:xx xxxx:xx:xx.x aaaaaaaa up xxx Gbps ... - after patch ... testpmd> show port summary 0 Number of available ports: 2 Port MAC Address Name Driver Status Link 0 xx:xx:xx:xx:xx:xx xxxx:xx:xx.x aaaaaaaa up xxx Gbps ... Only difference above is, after patch the command itself also printed.
quoted hunk ↗ jump to hunk
To improve usability here, we can use cmdline_new rather than cmdline_file_new and have the output from the various commands sent to stdout, allowing the user to see better what is happening. Signed-off-by: Bruce Richardson <redacted> --- v2: use STDOUT_FILENO in place of hard-coded "1" --- app/test-pmd/cmdline.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index b7759e38a8..52e64430d9 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c@@ -6,6 +6,7 @@ #include <ctype.h> #include <stdarg.h> #include <errno.h> +#include <fcntl.h> #include <stdio.h> #include <stdint.h> #include <stdlib.h>@@ -13431,7 +13432,18 @@ cmdline_read_from_file(const char *filename) { struct cmdline *cl; - cl = cmdline_file_new(main_ctx, "testpmd> ", filename); + /* cmdline_file_new does not produce any output which is not ideal here. + * Much better to show output of the commands, so we open filename directly + * and then pass that to cmdline_new with stdout as the output path. + */ + int fd = open(filename, O_RDONLY); + if (fd < 0) { + fprintf(stderr, "Failed to open file %s: %s\n", + filename, strerror(errno)); + return; + } + + cl = cmdline_new(main_ctx, "testpmd> ", fd, STDOUT_FILENO);
Above is almost save as 'cmdline_file_new()' function with only difference that it uses '-1' for 's_out'. If this usecase may be required by others, do you think does it have a value to pass 's_out' to 'cmdline_file_new()' or have a new version of API that accepts 's_out' as parameter? btw, I recognized that 'cmdline' library doesn't have doxygen documentation, which is a gap to address. Next time when someone asks for entry level task, we can point this one.