> -----Original Message-----
> From: Jerin Jacob <jerinjac...@gmail.com>
> Sent: Monday, October 16, 2023 2:31 PM
> To: Sunil Kumar Kori <sk...@marvell.com>
> Cc: Thomas Monjalon <tho...@monjalon.net>; Rakesh Kudurumalla
> <rkuduruma...@marvell.com>; dev@dpdk.org
> Subject: [EXT] Re: [PATCH v8 01/12] app/graph: add application framework
> to read CLI
>
> External Email
>
> ----------------------------------------------------------------------
> On Fri, Sep 29, 2023 at 3:28 PM <sk...@marvell.com> wrote:
> >
> > From: Sunil Kumar Kori <sk...@marvell.com>
> >
> > It adds base framework to read a given .cli file as a command line
> > parameter "-s".
> >
> > Example:
> > # ./dpdk-graph -c 0xff -- -s ./app/graph/examples/dummy.cli
> >
> > Each .cli file will contain commands to configure different module
> > like mempool, ethdev, lookup tables, graph etc. Command parsing is
> > backed by commandline library.
> >
> > Each module needs to expose its supported commands & corresponding
> > callback functions to commandline library to get them parsed.
> >
> > Signed-off-by: Sunil Kumar Kori <sk...@marvell.com>
> > Signed-off-by: Rakesh Kudurumalla <rkuduruma...@marvell.com>
>
> > +cli_script_process(const char *file_name, size_t msg_in_len_max,
> > +size_t msg_out_len_max, void *obj) {
> > + char *msg_in = NULL, *msg_out = NULL;
> > + FILE *f = NULL;
> > +
> > + /* Check input arguments */
> > + if ((file_name == NULL) || (strlen(file_name) == 0) ||
> > (msg_in_len_max
> == 0) ||
> > + (msg_out_len_max == 0))
> > + return -EINVAL;
> > +
> > + msg_in = malloc(msg_in_len_max + 1);
> > + msg_out = malloc(msg_out_len_max + 1);
> > + if ((msg_in == NULL) || (msg_out == NULL)) {
>
> Use goto to avod dupliciting this section.
>
Will fix in next version.
> > + free(msg_out);
> > + free(msg_in);
> > + return -ENOMEM;
> > + }
> > +
> > + /* Open input file */
> > + f = fopen(file_name, "r");
> > + if (f == NULL) {
> > + free(msg_out);
> > + free(msg_in);
> > + return -EIO;
> > + }
> > +
> > + /* Read file */
> > + while (fgets(msg_in, msg_in_len_max, f) != NULL) {
> > + msg_out[0] = 0;
> > +
> > + cli_process(msg_in, msg_out, msg_out_len_max, obj);
> > +
> > + if (strlen(msg_out))
> > + printf("%s", msg_out);
> > + }
> > +
> > + /* Close file */
> > + fclose(f);
>
> See above:
>
Will fix in next version.
> > + free(msg_out);
> > + free(msg_in);
> > + return 0;
> > +}
> >
> new file mode 100644
Will fix in next version.
> > index 0000000000..08d0a48cd9
> > --- /dev/null
> > +++ b/app/graph/meson.build
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2023 Marvell.
> > +
> > +# override default name to drop the hyphen name = 'graph'
> > +build = cc.has_header('sys/epoll.h')
>
> Is this required?
>
Yes. It is needed to compiled the application for FreeBSD.
> > +if not build
> > + subdir_done()
> > +endif
> > +
> > +deps += ['bus_pci', 'graph', 'eal', 'lpm', 'ethdev', 'node',
> > +'cmdline']
>
> Check bus_pci really required?
>
Will check.
> > +sources = files(
> > + 'cli.c',
> > + 'main.c',
> > +)
> > diff --git a/app/graph/module_api.h b/app/graph/module_api.h new file
> > mode 100644 index 0000000000..372aeae7e3
> > --- /dev/null
> > +++ b/app/graph/module_api.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2023 Marvell.
> > + */
> > +
> > +#ifndef APP_GRAPH_MODULE_API_H
> > +#define APP_GRAPH_MODULE_API_H
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
>
> Have a new line here.
>
Will fix in next version.
> > +#include "cli.h"
> > +/*
> > + * Externs
> > + */
> > new file mode 100644
> > index 0000000000..271a85896d
> > --- /dev/null
> > +++ b/doc/guides/tools/graph.rst
> > @@ -0,0 +1,82 @@
> > +.. SPDX-License-Identifier: BSD-3-Clause
> > + Copyright(c) 2023 Marvell.
> > +
> > +dpdk-graph Application
> > +======================
> > +
> > +The ``dpdk-graph`` tool is a Data Plane Development Kit (DPDK)
> > +application that allows exercising various graph use cases.
> > +This application has a generic framework to add new graph based use
> > +cases to verify functionality. Each use case is defined as a
> > ``<usecase>.cli``
> file.
> > +Based on the input file, application creates a graph to cater the use case.
>
> Could you add some detailed text to describe the fact that, due to modular
> nature this app framework can be used by other graph based application OR
> SO.
>
> > +
> > +Supported Use cases
> > +-------------------
> > + *
>
> This * can be removed in this patch. Add it when l3fwd adds it.
>
Will fix in next version.
>
> > +
> > +Running the Application
> > +-----------------------
> > +
> > +The application has a number of command line options which can be
> > +provided in following syntax
> > +
> > +.. code-block:: console
> > +
> > + dpdk-graph [EAL Options] -- [application options]
> > +
> > +EAL Options
> > +~~~~~~~~~~~
> > +
> > +Following are the EAL command-line options that can be used in
> > +conjunction with the ``dpdk-graph`` application.
> > +See the DPDK Getting Started Guides for more information on these
> options.
> > +
> > +* ``-c <COREMASK>`` or ``-l <CORELIST>``
> > +
> > + Set the hexadecimal bit mask of the cores to run on. The CORELIST
> > is
> a
> > + list of cores to be used.
> > +
> > +Application Options
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +Following are the application command-line options:
> > +
> > +* ``-s``
> > +
> > + Script name with absolute path which specifies the use case. It is
> > + a mandatory parameter which will be used to create desired graph
> > + for a given use case.
> > +
> > +* ``--help``
> > +
> > + Dumps application usage
> > +
> > +Supported CLI commands
> > +----------------------
> > +
> > +This section provides details on commands which can be used in
> > +``<usecase>.cli`` file to express the requested use case configuration.
> > +
> > +.. list-table:: Exposed CLIs
> > + :widths: 40 40 10 10
> > + :header-rows: 1
> > + :class: longtable
> > +
> > + * - Command
> > + - Description
> > + - Dynamic
> > + - Optional
> > + * - Dummy command
> > + - Dummy command description
> > + - No
> > + - No
>
> Is it possible to make it as more vertical placement ? More horizontal
> placement is not PDF friendly.
Will check it.
