RE: [PATCH v4 2/3] app/graph: add ethdev forward command

[Sunil Kumar Kori <skori@marvell.com>]

[-- Attachment #1: Type: text/plain, Size: 5541 bytes --]

> -----Original Message-----
> From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> Sent: Tuesday, December 5, 2023 2:57 PM
> To: Sunil Kumar Kori <skori@marvell.com>; Rakesh Kudurumalla
> <rkudurumalla@marvell.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nithin
> Kumar Dabilpuram <ndabilpuram@marvell.com>
> Subject: [PATCH v4 2/3] app/graph: add ethdev forward command
> 
> Adds a txport to forward packet for every rxport
> 
> Mapping will be used to forward packets to txport received on rxport
> 
> Following commands are exposed:
> 	- ethdev forward <tx_dev_name> <rx_dev_name>"
> 
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> ---
>  app/graph/cli.c         |  1 +
>  app/graph/ethdev.c      | 62
> +++++++++++++++++++++++++++++++++++++++++
>  app/graph/ethdev.h      |  1 +
>  app/graph/ethdev_priv.h |  8 ++++++
>  4 files changed, 72 insertions(+)
> 
> diff --git a/app/graph/cli.c b/app/graph/cli.c index
> 30b12312d6..76f5b8e670 100644
> --- a/app/graph/cli.c
> +++ b/app/graph/cli.c
> @@ -32,6 +32,7 @@ cmdline_parse_ctx_t modules_ctx[] = {
>  	(cmdline_parse_inst_t *)&ethdev_prom_mode_cmd_ctx,
>  	(cmdline_parse_inst_t *)&ethdev_ip4_cmd_ctx,
>  	(cmdline_parse_inst_t *)&ethdev_ip6_cmd_ctx,
> +	(cmdline_parse_inst_t *)&ethdev_forward_cmd_ctx,
>  	(cmdline_parse_inst_t *)&ethdev_cmd_ctx,
>  	(cmdline_parse_inst_t *)&ethdev_help_cmd_ctx,
>  	(cmdline_parse_inst_t *)&ethdev_rx_cmd_ctx, diff --git
> a/app/graph/ethdev.c b/app/graph/ethdev.c index
> c9b09168c1..bceee659a2 100644
> --- a/app/graph/ethdev.c
> +++ b/app/graph/ethdev.c
> @@ -38,6 +38,10 @@ cmd_ethdev_ip4_addr_help[] = "ethdev
> <ethdev_name> ip4 addr add <ip> netmask <ma  static const char
> cmd_ethdev_ip6_addr_help[] = "ethdev <ethdev_name> ip6 addr add <ip>
> netmask <mask>";
> 
> +static const char
> +cmd_ethdev_forward_help[] = "ethdev forward <tx_dev_name>
> +<rx_dev_name>";
> +
> +
Remove extra line.

>  static struct rte_eth_conf port_conf_default = {
>  	.link_speeds = 0,
>  	.rxmode = {
> @@ -888,3 +892,61 @@ cmdline_parse_inst_t ethdev_help_cmd_ctx = {
>  		NULL,
>  	},
>  };
> +
> +static int
> +ethdev_forward_config(char *tx_name, char *rx_name) {
Replace tx_name -> tx_dev and rx_name -> rx_dev

> +	struct ethdev *port;
> +	uint16_t portid_rx = 0;
> +	uint16_t portid_tx = 0;
> +	int rc = -EINVAL;
> +
Use reverse x-max tree method to declare the variables.

> +	rc = rte_eth_dev_get_port_by_name(tx_name, &portid_tx);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = rte_eth_dev_get_port_by_name(rx_name, &portid_rx);
> +	if (rc < 0)
> +		return rc;
> +
> +	port = ethdev_port_by_id(portid_rx);
> +	if (port) {
> +		port->config.tx_port_id = portid_tx;
> +		rc = 0;
> +	}
> +
If port is NULL then rc will be returned with value >= 0. Means in case of failure, success  is returned. 

> +	return rc;
> +}
> +
> +static void
> +cli_ethdev_forward(void *parsed_result, __rte_unused struct cmdline
> +*cl, void *data __rte_unused) {
> +	struct ethdev_fwd_cmd_tokens *res = parsed_result;
> +	int rc = -EINVAL;
> +
> +	rc = ethdev_forward_config(res->tx_dev, res->rx_dev);
> +	if (rc < 0)
> +		printf(MSG_CMD_FAIL, res->cmd);
> +}
> +
> +cmdline_parse_token_string_t ethdev_l2_cmd =
Better to rename as ethdev_forward_xyz to align with other's naming convention.

> +	TOKEN_STRING_INITIALIZER(struct ethdev_fwd_cmd_tokens, cmd,
> "ethdev");
> +cmdline_parse_token_string_t ethdev_fwd_cmd =
> +	TOKEN_STRING_INITIALIZER(struct ethdev_fwd_cmd_tokens, fwd,
> +"forward"); cmdline_parse_token_string_t ethdev_tx_device =
> +	TOKEN_STRING_INITIALIZER(struct ethdev_fwd_cmd_tokens,
> tx_dev, NULL);
> +cmdline_parse_token_string_t ethdev_rx_device =
> +	TOKEN_STRING_INITIALIZER(struct ethdev_fwd_cmd_tokens,
> rx_dev, NULL);
> +
> +cmdline_parse_inst_t ethdev_forward_cmd_ctx = {
> +	.f = cli_ethdev_forward,
> +	.data = NULL,
> +	.help_str = cmd_ethdev_forward_help,
> +	.tokens = {
> +	       (void *)&ethdev_l2_cmd,
> +	       (void *)&ethdev_fwd_cmd,
> +	       (void *)&ethdev_tx_device,
> +	       (void *)&ethdev_rx_device,
> +	       NULL,
> +	},
> +};
> diff --git a/app/graph/ethdev.h b/app/graph/ethdev.h index
> 94d3247a2c..836052046b 100644
> --- a/app/graph/ethdev.h
> +++ b/app/graph/ethdev.h
> @@ -15,6 +15,7 @@ extern cmdline_parse_inst_t ethdev_mtu_cmd_ctx;
> extern cmdline_parse_inst_t ethdev_prom_mode_cmd_ctx;  extern
> cmdline_parse_inst_t ethdev_ip4_cmd_ctx;  extern cmdline_parse_inst_t
> ethdev_ip6_cmd_ctx;
> +extern cmdline_parse_inst_t ethdev_forward_cmd_ctx;
>  extern cmdline_parse_inst_t ethdev_cmd_ctx;  extern
> cmdline_parse_inst_t ethdev_help_cmd_ctx;
> 
> diff --git a/app/graph/ethdev_priv.h b/app/graph/ethdev_priv.h index
> f231f3f3e1..e5e5fbc9ae 100644
> --- a/app/graph/ethdev_priv.h
> +++ b/app/graph/ethdev_priv.h
> @@ -61,6 +61,13 @@ struct ethdev_ip6_cmd_tokens {
>  	cmdline_fixed_string_t mask;
>  };
> 
> +struct ethdev_fwd_cmd_tokens {
> +	cmdline_fixed_string_t cmd;
> +	cmdline_fixed_string_t fwd;
> +	cmdline_fixed_string_t tx_dev;
> +	cmdline_fixed_string_t rx_dev;
> +};
> +
>  struct ethdev_cmd_tokens {
>  	cmdline_fixed_string_t cmd;
>  	cmdline_fixed_string_t dev;
> @@ -98,6 +105,7 @@ struct ethdev_config {
>  		uint32_t queue_size;
>  	} tx;
> 
> +	uint16_t tx_port_id;
Please move it to struct ethdev{}.

>  	int promiscuous;
>  	uint32_t mtu;
>  };
> --
> 2.25.1


[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 37229 bytes --]