Ping.
> -----Original Message-----
> From: Sunil Kumar Kori <sk...@marvell.com>
> Sent: Thursday, July 21, 2022 1:02 PM
> To: Sunil Kumar Kori <sk...@marvell.com>; 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>; 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: RE: [PATCH] ethdev: add protocol param to color table update
>
> 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
