DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ori Kam <orika@mellanox.com>
To: Guy Kaneti <guyk@marvell.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	"xiang.w.wang@intel.com" <xiang.w.wang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
	Opher Reviv <opher@mellanox.com>,
	Alex Rosenbaum <alexr@mellanox.com>,
	Dovrat Zifroni <dovrat@marvell.com>,
	Prasun Kapoor <pkapoor@marvell.com>,
	"nipun.gupta@nxp.com" <nipun.gupta@nxp.com>,
	"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
	"yang.a.hong@intel.com" <yang.a.hong@intel.com>,
	"harry.chang@intel.com" <harry.chang@intel.com>,
	"gu.jian1@zte.com.cn" <gu.jian1@zte.com.cn>,
	"shanjiangh@chinatelecom.cn" <shanjiangh@chinatelecom.cn>,
	"zhangy.yun@chinatelecom.cn" <zhangy.yun@chinatelecom.cn>,
	"lixingfu@huachentel.com" <lixingfu@huachentel.com>,
	"wushuai@inspur.com" <wushuai@inspur.com>,
	"yuyingxia@yxlink.com" <yuyingxia@yxlink.com>,
	"fanchenggang@sunyainfo.com" <fanchenggang@sunyainfo.com>,
	"davidfgao@tencent.com" <davidfgao@tencent.com>,
	"liuzhong1@chinaunicom.cn" <liuzhong1@chinaunicom.cn>,
	"zhaoyong11@huawei.com" <zhaoyong11@huawei.com>,
	"oc@yunify.com" <oc@yunify.com>,
	"jim@netgate.com" <jim@netgate.com>,
	"hongjun.ni@intel.com" <hongjun.ni@intel.com>,
	"j.bromhead@titan-ic.com" <j.bromhead@titan-ic.com>,
	"deri@ntop.org" <deri@ntop.org>,
	"fc@napatech.com" <fc@napatech.com>,
	"arthur.su@lionic.com" <arthur.su@lionic.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v2 4/4] regexdev: implement regex rte	level	functions
Date: Wed, 22 Apr 2020 20:33:36 +0000
Message-ID: <AM6PR05MB5176F40DF3F428E199C0B575DBD20@AM6PR05MB5176.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <DM6PR18MB2410538D42BBA2880842016CD8D50@DM6PR18MB2410.namprd18.prod.outlook.com>

Hi Guy,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Guy Kaneti
> Sent: Tuesday, April 21, 2020 2:36 PM
> To: Ori Kam <orika@mellanox.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; xiang.w.wang@intel.com
> Cc: dev@dpdk.org; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Shahaf Shuler <shahafs@mellanox.com>;
> hemant.agrawal@nxp.com; Opher Reviv <opher@mellanox.com>; Alex
> Rosenbaum <alexr@mellanox.com>; Dovrat Zifroni <dovrat@marvell.com>;
> Prasun Kapoor <pkapoor@marvell.com>; nipun.gupta@nxp.com;
> bruce.richardson@intel.com; yang.a.hong@intel.com; harry.chang@intel.com;
> gu.jian1@zte.com.cn; shanjiangh@chinatelecom.cn;
> zhangy.yun@chinatelecom.cn; lixingfu@huachentel.com; wushuai@inspur.com;
> yuyingxia@yxlink.com; fanchenggang@sunyainfo.com;
> davidfgao@tencent.com; liuzhong1@chinaunicom.cn;
> zhaoyong11@huawei.com; oc@yunify.com; jim@netgate.com;
> hongjun.ni@intel.com; j.bromhead@titan-ic.com; deri@ntop.org;
> fc@napatech.com; arthur.su@lionic.com; Thomas Monjalon
> <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v2 4/4] regexdev: implement regex rte level
> functions
> 
> Hi,
> 
> > +int
> > +rte_regexdev_configure(uint8_t dev_id, const struct rte_regexdev_config
> > +*cfg) {
> > +	struct rte_regexdev *dev;
> > +	struct rte_regexdev_info dev_info;
> > +	int ret;
> > +
> > +	RTE_REGEXDEV_VALID_DEV_ID_OR_ERR_RET(dev_id, -EINVAL);
> > +	if (cfg == NULL)
> > +		return -EINVAL;
> > +	dev = &rte_regex_devices[dev_id];
> > +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -
> > ENOTSUP);
> > +	if (dev->data->dev_started) {
> > +		RTE_REGEXDEV_LOG
> > +			(ERR, "Dev %u must be stopped to allow
> > configuration\n",
> > +			 dev_id);
> > +		return -EBUSY;
> > +	}
> > +	ret = regexdev_info_get(dev_id, &dev_info);
> > +	if (ret < 0)
> > +		return ret;
> > +	if ((cfg->dev_cfg_flags &
> > RTE_REGEXDEV_CFG_CROSS_BUFFER_SCAN_F) &&
> > +	    !(dev_info.regexdev_capa &
> > RTE_REGEXDEV_SUPP_CROSS_BUFFER_F)) {
> > +		RTE_REGEXDEV_LOG(ERR,
> > +				 "Dev %u doesn't support cross buffer
> > scan\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if ((cfg->dev_cfg_flags & RTE_REGEXDEV_CFG_MATCH_AS_END_F)
> > &&
> > +	    !(dev_info.regexdev_capa &
> > RTE_REGEXDEV_SUPP_MATCH_AS_END_F)) {
> > +		RTE_REGEXDEV_LOG(ERR,
> > +				 "Dev %u doesn't support match as end\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if ((cfg->dev_cfg_flags & RTE_REGEXDEV_CFG_MATCH_ALL_F) &&
> > +	    !(dev_info.regexdev_capa &
> > RTE_REGEXDEV_SUPP_MATCH_ALL_F)) {
> > +		RTE_REGEXDEV_LOG(ERR,
> > +				 "Dev %u doesn't support match all\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if (cfg->nb_groups == 0) {
> > +		RTE_REGEXDEV_LOG(ERR, "Dev %u num of groups must be >
> > 0\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if (cfg->nb_groups >= dev_info.max_groups) {
> > +		RTE_REGEXDEV_LOG(ERR, "Dev %u num of groups %d >
> > %d\n",
> > +				 dev_id, cfg->nb_groups,
> > dev_info.max_groups);
> > +		return -EINVAL;
> > +	}
> 
> The comparison should be > and not >=
> 

Yes, you are correct will fix.

> > +	if (cfg->nb_max_matches == 0) {
> > +		RTE_REGEXDEV_LOG(ERR, "Dev %u num of matches must be
> > > 0\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if (cfg->nb_max_matches >= dev_info.max_matches) {
> > +		RTE_REGEXDEV_LOG(ERR, "Dev %u num of matches %d >
> > %d\n",
> > +				 dev_id, cfg->nb_max_matches,
> > +				 dev_info.max_matches);
> > +		return -EINVAL;
> > +	}
> 
> The comparison should be > and not >=
> 

Yes, will fix.

> > +	if (cfg->nb_queue_pairs == 0) {
> > +		RTE_REGEXDEV_LOG(ERR, "Dev %u num of queues must be
> > > 0\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if (cfg->nb_queue_pairs >= dev_info.max_queue_pairs) {
> > +		RTE_REGEXDEV_LOG(ERR, "Dev %u num of queues %d >
> > %d\n",
> > +				 dev_id, cfg->nb_queue_pairs,
> > +				 dev_info.max_queue_pairs);
> > +		return -EINVAL;
> > +	}
> 
> The comparison should be > and not >=
> 

Will fix.

> > +	if (cfg->nb_rules_per_group == 0) {
> > +		RTE_REGEXDEV_LOG(ERR,
> > +				 "Dev %u num of rules per group must be >
> > 0\n",
> > +				 dev_id);
> > +		return -EINVAL;
> > +	}
> > +	if (cfg->nb_rules_per_group >= dev_info.max_rules_per_group) {
> > +		RTE_REGEXDEV_LOG(ERR,
> > +				 "Dev %u num of rules per group %d > %d\n",
> > +				 dev_id, cfg->nb_rules_per_group,
> > +				 dev_info.max_rules_per_group);
> > +		return -EINVAL;
> > +	}
> 
> The comparison should be > and not >=
> 

Will fix.

> > +	ret = (*dev->dev_ops->dev_configure)(dev, cfg);
> > +	if (ret == 0)
> > +		dev->data->dev_conf = *cfg;
> > +	return ret;
> > +}
> 
> In general I think that the validation of the cfg values should be done by the
> PMD

This was done in the first version.
after comments from the community, I changed it.
As much as I like the idea that PMD should handle everything by itself.
there is no point of code duplication, all PMD will require to do those test,
and there is no advantage of doing it inside the PMD.
Also it is common practice in DPDK to assume that the input was tested in the above 
layer. (you can see ethdev)


  reply	other threads:[~2020-04-22 20:33 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-29  6:47 [dpdk-dev] [PATCH v1 0/4] add RegEx class Ori Kam
2020-03-29  6:47 ` [dpdk-dev] [PATCH v1 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-04-04 15:04   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2020-04-05 15:03     ` Ori Kam
2020-03-29  6:47 ` [dpdk-dev] [PATCH v1 2/4] regexdev: add regex core h file Ori Kam
     [not found]   ` <DM5PR18MB221411AAEAFFA9D9F373292BC6C20@DM5PR18MB2214.namprd18.prod.outlook.com>
2020-04-07  8:53     ` Guy Kaneti
2020-04-07 16:16       ` Ori Kam
2020-04-07 16:27         ` Jerin Jacob
2020-04-08  7:37           ` Ori Kam
2020-04-08  7:48             ` Jerin Jacob
2020-04-08  8:31               ` Ori Kam
2020-04-08  8:38                 ` Jerin Jacob
2020-04-08  9:51                   ` Ori Kam
2020-03-29  6:47 ` [dpdk-dev] [PATCH v1 3/4] regexdev: add regexdev core functions Ori Kam
2020-04-04 15:01   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2020-04-05 15:05     ` Ori Kam
2020-04-05 17:10       ` Pavan Nikhilesh Bhagavatula
2020-04-05 20:02         ` Ori Kam
2020-04-06 12:48           ` Pavan Nikhilesh Bhagavatula
2020-04-06 13:29             ` Thomas Monjalon
2020-04-06 13:38               ` Jerin Jacob
2020-04-06 19:11                 ` Ori Kam
2020-04-07  5:49                   ` Jerin Jacob
2020-04-07  6:46                     ` Ori Kam
2020-04-07  7:22                       ` Jerin Jacob
2020-04-07 12:27                     ` Thomas Monjalon
2020-04-07 12:54                       ` Jerin Jacob
2020-04-07 14:21                   ` Guy Kaneti
2020-04-07 16:28                     ` Ori Kam
2020-04-07 16:37                       ` Guy Kaneti
2020-04-08  6:52                         ` Ori Kam
2020-04-08  8:39               ` Ori Kam
2020-04-19 10:38                 ` Guy Kaneti
2020-04-22 21:36                   ` Ori Kam
2020-04-08  9:41             ` Ori Kam
2020-03-29  6:47 ` [dpdk-dev] [PATCH v1 4/4] regexdev: implement regex rte level functions Ori Kam
2020-04-04 14:27   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2020-04-05 15:04     ` Ori Kam
2020-04-05 16:48       ` Pavan Nikhilesh Bhagavatula
2020-04-05 19:46         ` Ori Kam
2020-04-06 11:16       ` Thomas Monjalon
2020-04-06 12:33         ` Pavan Nikhilesh Bhagavatula
2020-04-06 13:14           ` Thomas Monjalon
2020-04-06 13:20             ` Jerin Jacob
2020-04-06 13:22             ` Pavan Nikhilesh Bhagavatula
2020-04-06 13:36               ` Thomas Monjalon
2020-04-06 13:50                 ` Pavan Nikhilesh Bhagavatula
2020-04-06 14:00                   ` Thomas Monjalon
2020-04-06 18:53                     ` Ori Kam
2020-04-04 13:06 ` [dpdk-dev] [EXT] [PATCH v1 0/4] add RegEx class Pavan Nikhilesh Bhagavatula
2020-04-05 15:03   ` Ori Kam
2020-04-17 12:43 ` [dpdk-dev] [PATCH v2 " Ori Kam
2020-04-17 12:43   ` [dpdk-dev] [PATCH v2 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-04-17 12:43   ` [dpdk-dev] [PATCH v2 2/4] regexdev: add regex core h file Ori Kam
2020-04-20 10:48     ` Guy Kaneti
2020-04-20 15:49       ` Ori Kam
2020-04-17 12:43   ` [dpdk-dev] [PATCH v2 3/4] regexdev: add regexdev core functions Ori Kam
2020-04-17 12:43   ` [dpdk-dev] [PATCH v2 4/4] regexdev: implement regex rte level functions Ori Kam
2020-04-21 11:12     ` Guy Kaneti
2020-04-21 11:20       ` Ori Kam
2020-04-21 11:36     ` Guy Kaneti
2020-04-22 20:33       ` Ori Kam [this message]
2020-05-07  9:45 ` [dpdk-dev] [PATCH v3 0/4] add RegEx class Ori Kam
2020-05-07  9:45   ` [dpdk-dev] [PATCH v3 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-06-21 11:18     ` Ori Kam
2020-06-30 19:57       ` Ori Kam
2020-05-07  9:45   ` [dpdk-dev] [PATCH v3 2/4] regexdev: add regex core h file Ori Kam
2020-06-03  6:47     ` [dpdk-dev] [EXT] " Guy Kaneti
2020-05-07  9:45   ` [dpdk-dev] [PATCH v3 3/4] regexdev: add regexdev core functions Ori Kam
2020-06-03  6:57     ` [dpdk-dev] [EXT] " Guy Kaneti
2020-05-07  9:45   ` [dpdk-dev] [PATCH v3 4/4] regexdev: implement regex rte level functions Ori Kam
2020-06-03  6:59     ` [dpdk-dev] [EXT] " Guy Kaneti
2020-06-28 13:45     ` Guy Kaneti
2020-06-28 14:10       ` Ori Kam
2020-05-24 20:24   ` [dpdk-dev] [PATCH v3 0/4] add RegEx class Ori Kam
2020-07-02  7:45 ` [dpdk-dev] [PATCH v4 " Ori Kam
2020-07-02  7:46   ` [dpdk-dev] [PATCH v4 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-07-05 21:18     ` Thomas Monjalon
2020-07-06  7:02       ` Ori Kam
2020-07-02  7:46   ` [dpdk-dev] [PATCH v4 2/4] regexdev: add regex core h file Ori Kam
2020-07-02  7:46   ` [dpdk-dev] [PATCH v4 3/4] regexdev: add regexdev core functions Ori Kam
2020-07-05 21:08     ` Thomas Monjalon
2020-07-06  6:07       ` Ori Kam
2020-07-06  7:03         ` Thomas Monjalon
2020-07-06  8:00           ` Bruce Richardson
2020-07-02  7:46   ` [dpdk-dev] [PATCH v4 4/4] regexdev: implement regex rte level functions Ori Kam
2020-07-05 21:21   ` [dpdk-dev] [PATCH v4 0/4] add RegEx class Thomas Monjalon
2020-07-06  7:03     ` Ori Kam
2020-07-06 17:36 ` [dpdk-dev] [PATCH v5 " Ori Kam
2020-07-06 17:36   ` [dpdk-dev] [PATCH v5 1/4] regexdev: introduce regexdev subsystem Ori Kam
2020-07-06 19:38     ` Thomas Monjalon
2020-09-11 12:46     ` David Marchand
2020-07-06 17:36   ` [dpdk-dev] [PATCH v5 2/4] regexdev: add regex core h file Ori Kam
2020-07-06 17:36   ` [dpdk-dev] [PATCH v5 3/4] regexdev: add regexdev core functions Ori Kam
2020-07-06 17:36   ` [dpdk-dev] [PATCH v5 4/4] regexdev: implement regex rte level functions Ori Kam
2020-07-06 22:30   ` [dpdk-dev] [PATCH v5 0/4] add RegEx class Thomas Monjalon

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=AM6PR05MB5176F40DF3F428E199C0B575DBD20@AM6PR05MB5176.eurprd05.prod.outlook.com \
    --to=orika@mellanox.com \
    --cc=alexr@mellanox.com \
    --cc=arthur.su@lionic.com \
    --cc=bruce.richardson@intel.com \
    --cc=davidfgao@tencent.com \
    --cc=deri@ntop.org \
    --cc=dev@dpdk.org \
    --cc=dovrat@marvell.com \
    --cc=fanchenggang@sunyainfo.com \
    --cc=fc@napatech.com \
    --cc=gu.jian1@zte.com.cn \
    --cc=guyk@marvell.com \
    --cc=harry.chang@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=hongjun.ni@intel.com \
    --cc=j.bromhead@titan-ic.com \
    --cc=jerinj@marvell.com \
    --cc=jim@netgate.com \
    --cc=liuzhong1@chinaunicom.cn \
    --cc=lixingfu@huachentel.com \
    --cc=nipun.gupta@nxp.com \
    --cc=oc@yunify.com \
    --cc=opher@mellanox.com \
    --cc=pbhagavatula@marvell.com \
    --cc=pkapoor@marvell.com \
    --cc=shahafs@mellanox.com \
    --cc=shanjiangh@chinatelecom.cn \
    --cc=thomas@monjalon.net \
    --cc=wushuai@inspur.com \
    --cc=xiang.w.wang@intel.com \
    --cc=yang.a.hong@intel.com \
    --cc=yuyingxia@yxlink.com \
    --cc=zhangy.yun@chinatelecom.cn \
    --cc=zhaoyong11@huawei.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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git