Hello all,
Please have look on it and provide your feedback.
Regards
Sunil Kumar Kori
> -----Original Message-----
> From: sk...@marvell.com <sk...@marvell.com>
> Sent: Thursday, July 7, 2022 12:28 PM
> To: Cristian Dumitrescu <cristian.dumitre...@intel.com>; Aman Singh
> <aman.deep.si...@intel.com>; Yuying Zhang <yuying.zh...@intel.com>;
> Nithin Kumar Dabilpuram <ndabilpu...@marvell.com>; Kiran Kumar
> Kokkilagadda <kirankum...@marvell.com>; Sunil Kumar Kori
> <sk...@marvell.com>; Satha Koteswara Rao Kottidi
> <skotesh...@marvell.com>; Jasvinder Singh <jasvinder.si...@intel.com>;
> Thomas Monjalon <tho...@monjalon.net>; Ferruh Yigit
> <ferruh.yi...@xilinx.com>; Andrew Rybchenko
> <andrew.rybche...@oktetlabs.ru>
> Cc: dev@dpdk.org
> Subject: [PATCH] ethdev: add protocol param to color table update
>
> From: Sunil Kumar Kori <sk...@marvell.com>
>
> Using rte_mtr_color_in_protocol_set(), user can configure combination of
> protocol headers, like outer_vlan and outer_ip, can be enabled on given
> meter object.
>
> But rte_mtr_meter_vlan_table_update() and
> rte_mtr_meter_dscp_table_update() do not have information that which
> table needs to be updated corresponding to protocol header i.e. inner or
> outer.
>
> Adding protocol paramreter will allow user to provide required protocol
> information so that corresponding inner or outer table can be updated
> corresponding to protocol header.
>
> If user wishes to configure both inner and outer table then API must be
> called twice with correct protocol information.
>
> Depends-on: series-23647 ("common/cnxk: update precolor table setup for
> VLAN")
>
> Signed-off-by: Sunil Kumar Kori <sk...@marvell.com>
> ---
> app/test-pmd/cmdline_mtr.c | 42 ++++++++++++++++-----
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 ++--
> drivers/net/cnxk/cnxk_ethdev_mtr.c | 20 +++++++++-
> drivers/net/softnic/rte_eth_softnic_meter.c | 4 +-
> lib/ethdev/rte_mtr.c | 8 ++--
> lib/ethdev/rte_mtr.h | 7 +++-
> lib/ethdev/rte_mtr_driver.h | 4 +-
> 7 files changed, 70 insertions(+), 23 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_mtr.c b/app/test-pmd/cmdline_mtr.c
> index b92e66cedb..c3644ef443 100644
> --- a/app/test-pmd/cmdline_mtr.c
> +++ b/app/test-pmd/cmdline_mtr.c
> @@ -293,8 +293,8 @@ parse_meter_color_str(char *c_str, uint32_t
> *use_prev_meter_color, }
>
> static int
> -parse_multi_token_string(char *t_str, uint16_t *port_id,
> - uint32_t *mtr_id, enum rte_color **dscp_table)
> +parse_multi_token_string(char *t_str, uint16_t *port_id, uint32_t *mtr_id,
> + enum rte_mtr_color_in_protocol *proto, enum rte_color
> **dscp_table)
> {
> char *token;
> uint64_t val;
> @@ -322,6 +322,16 @@ parse_multi_token_string(char *t_str, uint16_t
> *port_id,
>
> *mtr_id = val;
>
> + /* Third token: protocol */
> + token = strtok_r(t_str, PARSE_DELIMITER, &t_str);
> + if (token == NULL)
> + return 0;
> +
> + if (strcmp(token, "outer_ip") == 0)
> + *proto = RTE_MTR_COLOR_IN_PROTO_OUTER_IP;
> + else if (strcmp(token, "inner_ip") == 0)
> + *proto = RTE_MTR_COLOR_IN_PROTO_INNER_IP;
> +
> ret = parse_dscp_table_entries(t_str, dscp_table);
> if (ret != 0)
> return -1;
> @@ -331,7 +341,7 @@ parse_multi_token_string(char *t_str, uint16_t
> *port_id,
>
> static int
> parse_multi_token_vlan_str(char *t_str, uint16_t *port_id, uint32_t
> *mtr_id,
> - enum rte_color **vlan_table)
> + enum rte_mtr_color_in_protocol *proto, enum rte_color
> **vlan_table)
> {
> uint64_t val;
> char *token;
> @@ -359,6 +369,16 @@ parse_multi_token_vlan_str(char *t_str, uint16_t
> *port_id, uint32_t *mtr_id,
>
> *mtr_id = val;
>
> + /* Third token: protocol */
> + token = strtok_r(t_str, PARSE_DELIMITER, &t_str);
> + if (token == NULL)
> + return 0;
> +
> + if (strcmp(token, "outer_vlan") == 0)
> + *proto = RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN;
> + else if (strcmp(token, "inner_vlan") == 0)
> + *proto = RTE_MTR_COLOR_IN_PROTO_INNER_VLAN;
> +
> ret = parse_vlan_table_entries(t_str, vlan_table);
> if (ret != 0)
> return -1;
> @@ -1384,6 +1404,7 @@ static void
> cmd_set_port_meter_dscp_table_parsed(void *parsed_result,
> __rte_unused void *data)
> {
> struct cmd_set_port_meter_dscp_table_result *res = parsed_result;
> + enum rte_mtr_color_in_protocol proto = 0;
> struct rte_mtr_error error;
> enum rte_color *dscp_table = NULL;
> char *t_str = res->token_string;
> @@ -1392,7 +1413,8 @@ static void
> cmd_set_port_meter_dscp_table_parsed(void *parsed_result,
> int ret;
>
> /* Parse string */
> - ret = parse_multi_token_string(t_str, &port_id, &mtr_id,
> &dscp_table);
> + ret = parse_multi_token_string(t_str, &port_id, &mtr_id, &proto,
> + &dscp_table);
> if (ret) {
> fprintf(stderr, " Multi token string parse error\n");
> return;
> @@ -1402,7 +1424,7 @@ static void
> cmd_set_port_meter_dscp_table_parsed(void *parsed_result,
> goto free_table;
>
> /* Update Meter DSCP Table*/
> - ret = rte_mtr_meter_dscp_table_update(port_id, mtr_id,
> + ret = rte_mtr_meter_dscp_table_update(port_id, mtr_id, proto,
> dscp_table, &error);
> if (ret != 0)
> print_err_msg(&error);
> @@ -1414,7 +1436,7 @@ static void
> cmd_set_port_meter_dscp_table_parsed(void *parsed_result,
> cmdline_parse_inst_t cmd_set_port_meter_dscp_table = {
> .f = cmd_set_port_meter_dscp_table_parsed,
> .data = NULL,
> - .help_str = "set port meter dscp table <port_id> <mtr_id> "
> + .help_str = "set port meter dscp table <port_id> <mtr_id> <proto> "
> "[<dscp_tbl_entry0> <dscp_tbl_entry1> ...
> <dscp_tbl_entry63>]",
> .tokens = {
> (void *)&cmd_set_port_meter_dscp_table_set,
> @@ -1457,6 +1479,7 @@ static void
> cmd_set_port_meter_vlan_table_parsed(void *parsed_result,
> __rte_unused void *data)
> {
> struct cmd_set_port_meter_vlan_table_result *res = parsed_result;
> + enum rte_mtr_color_in_protocol proto = 0;
> struct rte_mtr_error error;
> enum rte_color *vlan_table = NULL;
> char *t_str = res->token_string;
> @@ -1465,7 +1488,8 @@ static void
> cmd_set_port_meter_vlan_table_parsed(void *parsed_result,
> int ret;
>
> /* Parse string */
> - ret = parse_multi_token_vlan_str(t_str, &port_id, &mtr_id,
> &vlan_table);
> + ret = parse_multi_token_vlan_str(t_str, &port_id, &mtr_id, &proto,
> + &vlan_table);
> if (ret) {
> fprintf(stderr, " Multi token string parse error\n");
> return;
> @@ -1475,7 +1499,7 @@ static void
> cmd_set_port_meter_vlan_table_parsed(void *parsed_result,
> goto free_table;
>
> /* Update Meter VLAN Table*/
> - ret = rte_mtr_meter_vlan_table_update(port_id, mtr_id,
> + ret = rte_mtr_meter_vlan_table_update(port_id, mtr_id, proto,
> vlan_table, &error);
> if (ret != 0)
> print_err_msg(&error);
> @@ -1487,7 +1511,7 @@ static void
> cmd_set_port_meter_vlan_table_parsed(void *parsed_result,
> cmdline_parse_inst_t cmd_set_port_meter_vlan_table = {
> .f = cmd_set_port_meter_vlan_table_parsed,
> .data = NULL,
> - .help_str = "set port meter vlan table <port_id> <mtr_id> "
> + .help_str = "set port meter vlan table <port_id> <mtr_id> <proto> "
> "[<vlan_tbl_entry0> <vlan_tbl_entry1> ...
> <vlan_tbl_entry15>]",
> .tokens = {
> (void *)&cmd_set_port_meter_vlan_table_set,
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 330e34427d..ce40e3b6f2 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2597,15 +2597,15 @@ set port meter dscp table
>
> Set meter dscp table for the ethernet device::
>
> - testpmd> set port meter dscp table (port_id) (mtr_id) [(dscp_tbl_entry0) \
> - (dscp_tbl_entry1)...(dscp_tbl_entry63)]
> + testpmd> set port meter dscp table (port_id) (mtr_id) (proto) \
> + [(dscp_tbl_entry0) (dscp_tbl_entry1)...(dscp_tbl_entry63)]
>
> set port meter vlan table
> ~~~~~~~~~~~~~~~~~~~~~~~~~
> Set meter VLAN table for the Ethernet device::
>
> - testpmd> set port meter vlan table (port_id) (mtr_id) [(vlan_tbl_entry0) \
> - (vlan_tbl_entry1)...(vlan_tbl_entry15)]
> + testpmd> set port meter vlan table (port_id) (mtr_id) (proto) \
> + [(vlan_tbl_entry0) (vlan_tbl_entry1)...(vlan_tbl_entry15)]
>
> set port meter protocol
> ~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/drivers/net/cnxk/cnxk_ethdev_mtr.c
> b/drivers/net/cnxk/cnxk_ethdev_mtr.c
> index be2cb7d628..0fa18f01c7 100644
> --- a/drivers/net/cnxk/cnxk_ethdev_mtr.c
> +++ b/drivers/net/cnxk/cnxk_ethdev_mtr.c
> @@ -720,6 +720,7 @@ cnxk_nix_mtr_disable(struct rte_eth_dev *eth_dev,
> uint32_t mtr_id,
>
> static int
> cnxk_nix_mtr_dscp_table_update(struct rte_eth_dev *eth_dev, uint32_t
> mtr_id,
> + enum rte_mtr_color_in_protocol proto,
> enum rte_color *dscp_table,
> struct rte_mtr_error *error)
> {
> @@ -750,7 +751,7 @@ cnxk_nix_mtr_dscp_table_update(struct rte_eth_dev
> *eth_dev, uint32_t mtr_id,
>
> table.count = ROC_NIX_BPF_PRECOLOR_TBL_SIZE_DSCP;
>
> - switch (dev->proto) {
> + switch (proto) {
> case RTE_MTR_COLOR_IN_PROTO_OUTER_IP:
> table.mode = ROC_NIX_BPF_PC_MODE_DSCP_OUTER;
> break;
> @@ -764,6 +765,13 @@ cnxk_nix_mtr_dscp_table_update(struct
> rte_eth_dev *eth_dev, uint32_t mtr_id,
> goto exit;
> }
>
> + if (dev->proto != proto) {
> + rc = -rte_mtr_error_set(error, EINVAL,
> + RTE_MTR_ERROR_TYPE_UNSPECIFIED, NULL,
> + "input color protocol is not configured");
> + goto exit;
> + }
> +
> for (i = 0; i < ROC_NIX_BPF_PRECOLOR_TBL_SIZE_DSCP; i++)
> table.color[i] = nix_dscp_tbl[i];
>
> @@ -784,6 +792,7 @@ cnxk_nix_mtr_dscp_table_update(struct rte_eth_dev
> *eth_dev, uint32_t mtr_id,
>
> static int
> cnxk_nix_mtr_vlan_table_update(struct rte_eth_dev *eth_dev, uint32_t
> mtr_id,
> + enum rte_mtr_color_in_protocol proto,
> enum rte_color *vlan_table,
> struct rte_mtr_error *error)
> {
> @@ -814,7 +823,7 @@ cnxk_nix_mtr_vlan_table_update(struct rte_eth_dev
> *eth_dev, uint32_t mtr_id,
>
> table.count = ROC_NIX_BPF_PRECOLOR_TBL_SIZE_VLAN;
>
> - switch (dev->proto) {
> + switch (proto) {
> case RTE_MTR_COLOR_IN_PROTO_OUTER_VLAN:
> table.mode = ROC_NIX_BPF_PC_MODE_VLAN_OUTER;
> break;
> @@ -828,6 +837,13 @@ cnxk_nix_mtr_vlan_table_update(struct
> rte_eth_dev *eth_dev, uint32_t mtr_id,
> goto exit;
> }
>
> + if (dev->proto != proto) {
> + rc = -rte_mtr_error_set(error, EINVAL,
> + RTE_MTR_ERROR_TYPE_UNSPECIFIED, NULL,
> + "input color protocol is not configured");
> + goto exit;
> + }
> +
> for (i = 0; i < ROC_NIX_BPF_PRECOLOR_TBL_SIZE_VLAN; i++)
> table.color[i] = nix_vlan_tbl[i];
>
> diff --git a/drivers/net/softnic/rte_eth_softnic_meter.c
> b/drivers/net/softnic/rte_eth_softnic_meter.c
> index 6b02f43e31..3e635a3cfe 100644
> --- a/drivers/net/softnic/rte_eth_softnic_meter.c
> +++ b/drivers/net/softnic/rte_eth_softnic_meter.c
> @@ -636,7 +636,7 @@ pmd_mtr_meter_profile_update(struct rte_eth_dev
> *dev,
> /* MTR object meter DSCP table update */ static int
> pmd_mtr_meter_dscp_table_update(struct rte_eth_dev *dev,
> - uint32_t mtr_id,
> + uint32_t mtr_id, enum rte_mtr_color_in_protocol proto,
> enum rte_color *dscp_table,
> struct rte_mtr_error *error)
> {
> @@ -648,6 +648,8 @@ pmd_mtr_meter_dscp_table_update(struct
> rte_eth_dev *dev,
> uint32_t table_id, i;
> int status;
>
> + RTE_SET_USED(proto);
> +
> /* MTR object id must be valid */
> m = softnic_mtr_find(p, mtr_id);
> if (m == NULL)
> diff --git a/lib/ethdev/rte_mtr.c b/lib/ethdev/rte_mtr.c index
> c460e4f4e0..e4dff20f76 100644
> --- a/lib/ethdev/rte_mtr.c
> +++ b/lib/ethdev/rte_mtr.c
> @@ -197,25 +197,25 @@ rte_mtr_meter_policy_update(uint16_t port_id,
> /** MTR object meter DSCP table update */ int
> rte_mtr_meter_dscp_table_update(uint16_t port_id,
> - uint32_t mtr_id,
> + uint32_t mtr_id, enum rte_mtr_color_in_protocol proto,
> enum rte_color *dscp_table,
> struct rte_mtr_error *error)
> {
> struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> return RTE_MTR_FUNC(port_id, meter_dscp_table_update)(dev,
> - mtr_id, dscp_table, error);
> + mtr_id, proto, dscp_table, error);
> }
>
> /** MTR object meter VLAN table update */ int
> rte_mtr_meter_vlan_table_update(uint16_t port_id,
> - uint32_t mtr_id,
> + uint32_t mtr_id, enum rte_mtr_color_in_protocol proto,
> enum rte_color *vlan_table,
> struct rte_mtr_error *error)
> {
> struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> return RTE_MTR_FUNC(port_id, meter_vlan_table_update)(dev,
> - mtr_id, vlan_table, error);
> + mtr_id, proto, vlan_table, error);
> }
>
> /** Set the input color protocol on MTR object */ diff --git
> a/lib/ethdev/rte_mtr.h b/lib/ethdev/rte_mtr.h index
> 008bc84f0d..5e4f7ba73b 100644
> --- a/lib/ethdev/rte_mtr.h
> +++ b/lib/ethdev/rte_mtr.h
> @@ -913,6 +913,8 @@ rte_mtr_meter_policy_update(uint16_t port_id,
> * The port identifier of the Ethernet device.
> * @param[in] mtr_id
> * MTR object ID. Needs to be valid.
> + * @param[in] proto
> + * Input color protocol.
> * @param[in] dscp_table
> * When non-NULL: it points to a pre-allocated and pre-populated table
> with
> * exactly 64 elements providing the input color for each value of the
> @@ -927,7 +929,7 @@ rte_mtr_meter_policy_update(uint16_t port_id,
> __rte_experimental int rte_mtr_meter_dscp_table_update(uint16_t
> port_id,
> - uint32_t mtr_id,
> + uint32_t mtr_id, enum rte_mtr_color_in_protocol proto,
> enum rte_color *dscp_table,
> struct rte_mtr_error *error);
>
> @@ -938,6 +940,8 @@ rte_mtr_meter_dscp_table_update(uint16_t port_id,
> * The port identifier of the Ethernet device.
> * @param[in] mtr_id
> * MTR object ID. Needs to be valid.
> + * @param[in] proto
> + * Input color protocol.
> * @param[in] vlan_table
> * When non-NULL: it points to a pre-allocated and pre-populated table
> with
> * exactly 16 elements providing the input color for each value of the
> @@ -952,6 +956,7 @@ rte_mtr_meter_dscp_table_update(uint16_t port_id,
> __rte_experimental int rte_mtr_meter_vlan_table_update(uint16_t port_id,
> uint32_t mtr_id,
> + enum rte_mtr_color_in_protocol proto,
> enum rte_color *vlan_table,
> struct rte_mtr_error *error);
>
> diff --git a/lib/ethdev/rte_mtr_driver.h b/lib/ethdev/rte_mtr_driver.h index
> f7dca9a54c..a8b652a607 100644
> --- a/lib/ethdev/rte_mtr_driver.h
> +++ b/lib/ethdev/rte_mtr_driver.h
> @@ -93,13 +93,13 @@ typedef int (*rte_mtr_meter_policy_update_t)(struct
> rte_eth_dev *dev,
>
> /** @internal MTR object meter DSCP table update. */ typedef int
> (*rte_mtr_meter_dscp_table_update_t)(struct rte_eth_dev *dev,
> - uint32_t mtr_id,
> + uint32_t mtr_id, enum rte_mtr_color_in_protocol proto,
> enum rte_color *dscp_table,
> struct rte_mtr_error *error);
>
> /** @internal mtr object meter vlan table update. */ typedef int
> (*rte_mtr_meter_vlan_table_update_t)(struct rte_eth_dev *dev,
> - uint32_t mtr_id,
> + uint32_t mtr_id, enum rte_mtr_color_in_protocol proto,
> enum rte_color *vlan_table,
> struct rte_mtr_error *error);
>
> --
> 2.25.1
