From: Li Zhang <lizh@nvidia.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
"dekelp@nvidia.com" <dekelp@nvidia.com>,
Ori Kam <orika@nvidia.com>,
Slava Ovsiienko <viacheslavo@nvidia.com>,
Matan Azrad <matan@nvidia.com>,
Shahaf Shuler <shahafs@nvidia.com>,
"lironh@marvell.com" <lironh@marvell.com>,
"jerinj@marvell.com" <jerinj@marvell.com>,
"Yigit, Ferruh" <ferruh.yigit@intel.com>,
"ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>,
Wisam Monther <wisamm@nvidia.com>,
"Li, Xiaoyun" <xiaoyun.li@intel.com>,
"Singh, Jasvinder" <jasvinder.singh@intel.com>,
NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
Ray Kinsella <mdr@ashroe.eu>, Neil Horman <nhorman@tuxdriver.com>,
Jerin Jacob <jerinjacobk@gmail.com>,
Hemant Agrawal <hemant.agrawal@nxp.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
Raslan Darawsheh <rasland@nvidia.com>,
Roni Bar Yanai <roniba@nvidia.com>,
Haifei Luo <haifeil@nvidia.com>,
"Jiawei(Jonny) Wang" <jiaweiw@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH v6 1/2] ethdev: add pre-defined meter policy API
Date: Thu, 15 Apr 2021 01:59:52 +0000 [thread overview]
Message-ID: <DM6PR12MB4090AD25680A4F936D744DF4BF4D9@DM6PR12MB4090.namprd12.prod.outlook.com> (raw)
In-Reply-To: <DM6PR11MB2796F2CB58FF2EA3C98DCA00EB4E9@DM6PR11MB2796.namprd11.prod.outlook.com>
Thanks Cristian.
Will change it in V7 patch.
Regards,
Li Zhang
> -----Original Message-----
> From: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Sent: Thursday, April 15, 2021 12:16 AM
> To: Li Zhang <lizh@nvidia.com>; dekelp@nvidia.com; Ori Kam
> <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Matan
> Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
> lironh@marvell.com; jerinj@marvell.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>; ajit.khaparde@broadcom.com; Wisam Monther
> <wisamm@nvidia.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Singh,
> Jasvinder <jasvinder.singh@intel.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Ray Kinsella <mdr@ashroe.eu>; Neil
> Horman <nhorman@tuxdriver.com>; Jerin Jacob <jerinjacobk@gmail.com>;
> Hemant Agrawal <hemant.agrawal@nxp.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>; Roni Bar Yanai
> <roniba@nvidia.com>; Haifei Luo <haifeil@nvidia.com>; Jiawei(Jonny) Wang
> <jiaweiw@nvidia.com>
> Subject: RE: [PATCH v6 1/2] ethdev: add pre-defined meter policy API
>
> External email: Use caution opening links or attachments
>
>
> Hi Li,
>
> Following the API changes, there are lots of changes in the drivers, as
> expected, so we'll have to take the necessary time to review them.
>
> Here are just a few comments below, please expect more during the next
> few days.
>
> <snip>
>
> > +
> > +/* MTR meter policy add */
> > +static int
> > +pmd_mtr_meter_policy_add(struct rte_eth_dev *dev,
> > + uint32_t meter_policy_id,
> > + struct rte_mtr_meter_policy_params *policy,
> > + struct rte_mtr_error *error)
> > +{
> > + struct pmd_internals *p = dev->data->dev_private;
> > + struct softnic_mtr_meter_policy_list *mpl = &p-
> > >mtr.meter_policies;
> > + struct softnic_mtr_meter_policy *mp;
> > + const struct rte_flow_action *act;
> > + const struct rte_flow_action_meter_color *recolor;
> > + uint32_t i;
> > +
> > + /* Meter policy ID must be valid. */
> > + if (meter_policy_id == UINT32_MAX)
> > + return -rte_mtr_error_set(error,
> > + EINVAL,
> > + RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
> > + NULL,
> > + "Meter policy id not valid");
>
> This is obviously not correct, we need to check whether the meter_policy_id
> provided by the user is already in use (by a policy previously added) or not.
> You can do this with the policy find function that you have already
> implemented.
>
> > +
> > + for (i = 0; i < RTE_COLORS; i++) {
> > + act = policy->actions[i];
> > + if (act && act->type !=
> > RTE_FLOW_ACTION_TYPE_METER_COLOR &&
> > + act->type != RTE_FLOW_ACTION_TYPE_DROP)
> > + return -rte_mtr_error_set(error,
> > + EINVAL,
> > + RTE_MTR_ERROR_TYPE_METER_POLICY,
> > + NULL,
> > + "Action invalid");
> > + }
>
> This check does not look right either: obviously we cannot accept a null
> action list for any color, plus the action list should contain only those action
> types we support (RTE_FLOW_ACTION_TYPE_METER_COLOR or
> RTE_FLOW_ACTION_TYPE_DROP).
>
> I agree, fist you need to check that the linked list of policy actions for each
> color is non-NULL, then you need to traverse it until you meet the END
> action, skip any PAD actions, then check that exactly one (and one only) of
> METER_COLOR or DROP exist, but not both at the same time, and also we
> don't have the same action showing up multiple times. Makes sense?
>
> Regards,
> Cristian
next prev parent reply other threads:[~2021-04-15 1:59 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-18 8:58 [dpdk-dev] [PATCH 1/2] [RFC]: " Li Zhang
2021-03-18 8:58 ` [dpdk-dev] [PATCH 2/2] [RFC]: ethdev: manage meter API object handles by the drivers Li Zhang
2021-03-23 21:33 ` Dumitrescu, Cristian
2021-03-25 8:21 ` Matan Azrad
2021-03-25 23:16 ` Ajit Khaparde
2021-03-29 19:56 ` Matan Azrad
2021-03-27 13:15 ` Jerin Jacob
2021-03-29 20:10 ` Matan Azrad
2021-03-31 10:22 ` Jerin Jacob
2021-03-23 21:02 ` [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined meter policy API Dumitrescu, Cristian
2021-03-25 6:56 ` Matan Azrad
2021-03-29 9:23 ` Ori Kam
2021-03-29 16:24 ` Dumitrescu, Cristian
2021-04-01 13:13 ` Ori Kam
2021-04-01 13:35 ` Dumitrescu, Cristian
2021-04-01 14:22 ` Ori Kam
2021-03-29 16:08 ` Dumitrescu, Cristian
2021-03-29 20:43 ` Matan Azrad
2021-03-31 15:46 ` Dumitrescu, Cristian
2021-04-04 13:48 ` Matan Azrad
2021-03-29 10:38 ` Jerin Jacob
2021-03-29 20:31 ` Matan Azrad
2021-03-31 10:50 ` Jerin Jacob
2021-04-13 0:14 ` [dpdk-dev] [PATCH v3 0/2] Support " Li Zhang
2021-04-13 0:14 ` [dpdk-dev] [PATCH v3 1/2] ethdev: add pre-defined " Li Zhang
2021-04-13 14:19 ` Dumitrescu, Cristian
2021-04-14 3:23 ` Li Zhang
2021-04-13 14:59 ` Dumitrescu, Cristian
2021-04-14 4:55 ` Li Zhang
2021-04-14 8:02 ` Thomas Monjalon
2021-04-14 8:31 ` Matan Azrad
2021-04-14 8:47 ` Asaf Penso
2021-04-14 8:59 ` Li Zhang
2021-04-14 9:04 ` Thomas Monjalon
2021-04-14 14:00 ` Dumitrescu, Cristian
2021-04-14 16:21 ` Li Zhang
2021-04-13 16:25 ` Kinsella, Ray
2021-04-13 0:14 ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-14 3:12 ` [dpdk-dev] [PATCH v4 0/2] Support meter policy API Li Zhang
2021-04-14 3:12 ` [dpdk-dev] [PATCH v4 1/2] ethdev: add pre-defined " Li Zhang
2021-04-14 3:12 ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-14 6:32 ` [dpdk-dev] [PATCH v5 0/2] Support meter policy API Li Zhang
2021-04-14 6:32 ` [dpdk-dev] [PATCH v5 1/2] ethdev: add pre-defined " Li Zhang
2021-04-14 6:32 ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-14 8:57 ` [dpdk-dev] [PATCH v6 0/2] Support meter policy API Li Zhang
2021-04-14 8:57 ` [dpdk-dev] [PATCH v6 1/2] ethdev: add pre-defined " Li Zhang
2021-04-14 16:16 ` Dumitrescu, Cristian
2021-04-15 1:59 ` Li Zhang [this message]
2021-04-14 22:21 ` Singh, Jasvinder
2021-04-15 2:00 ` Li Zhang
2021-04-14 8:58 ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-15 4:54 ` [dpdk-dev] [PATCH v7 0/2] Support meter policy API Li Zhang
2021-04-15 4:54 ` [dpdk-dev] [PATCH v7 1/2] ethdev: add pre-defined " Li Zhang
2021-04-15 4:54 ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-15 9:20 ` [dpdk-dev] [PATCH v8 0/2] Support meter policy API Li Zhang
2021-04-15 9:20 ` [dpdk-dev] [PATCH v8 1/2] ethdev: add pre-defined " Li Zhang
2021-04-15 15:13 ` Ori Kam
2021-04-19 12:34 ` Singh, Jasvinder
2021-04-19 16:13 ` Jiawei(Jonny) Wang
2021-04-15 9:20 ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: support policy actions per color Li Zhang
2021-04-19 16:08 ` [dpdk-dev] [PATCH v9 0/2] Support meter policy API Jiawei Wang
2021-04-19 16:08 ` [dpdk-dev] [PATCH v9 1/2] ethdev: add pre-defined " Jiawei Wang
2021-04-20 11:18 ` Dumitrescu, Cristian
2021-04-20 12:55 ` Asaf Penso
2021-04-20 21:01 ` Dumitrescu, Cristian
2021-04-19 16:08 ` [dpdk-dev] [PATCH v9 2/2] app/testpmd: support policy actions per color Jiawei Wang
2021-04-20 11:36 ` [dpdk-dev] [PATCH v9 0/2] Support meter policy API Ferruh Yigit
2021-04-20 14:08 ` Jiawei(Jonny) Wang
2021-04-20 14:04 ` [dpdk-dev] [PATCH v10 " Jiawei Wang
2021-04-20 14:04 ` [dpdk-dev] [PATCH v10 1/2] ethdev: add pre-defined " Jiawei Wang
2021-04-20 17:12 ` Ajit Khaparde
2021-04-21 19:43 ` Thomas Monjalon
2021-04-22 1:29 ` Li Zhang
2021-04-20 14:04 ` [dpdk-dev] [PATCH v10 2/2] app/testpmd: support policy actions per color Jiawei Wang
2021-04-20 17:14 ` Ajit Khaparde
2021-04-21 10:23 ` [dpdk-dev] [PATCH v10 0/2] Support meter policy API Ferruh Yigit
2021-04-20 17:56 ` [dpdk-dev] [PATCH 1/2] [RFC]: ethdev: add pre-defined " Stephen Hemminger
2021-04-21 2:49 ` Li Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR12MB4090AD25680A4F936D744DF4BF4D9@DM6PR12MB4090.namprd12.prod.outlook.com \
--to=lizh@nvidia.com \
--cc=ajit.khaparde@broadcom.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=cristian.dumitrescu@intel.com \
--cc=dekelp@nvidia.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=haifeil@nvidia.com \
--cc=hemant.agrawal@nxp.com \
--cc=jasvinder.singh@intel.com \
--cc=jerinj@marvell.com \
--cc=jerinjacobk@gmail.com \
--cc=jiaweiw@nvidia.com \
--cc=lironh@marvell.com \
--cc=matan@nvidia.com \
--cc=mdr@ashroe.eu \
--cc=nhorman@tuxdriver.com \
--cc=orika@nvidia.com \
--cc=rasland@nvidia.com \
--cc=roniba@nvidia.com \
--cc=shahafs@nvidia.com \
--cc=thomas@monjalon.net \
--cc=viacheslavo@nvidia.com \
--cc=wisamm@nvidia.com \
--cc=xiaoyun.li@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).