From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <bernard.iremonger@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
 by dpdk.org (Postfix) with ESMTP id BA0731B75B
 for <dev@dpdk.org>; Fri, 13 Oct 2017 17:40:00 +0200 (CEST)
Received: from fmsmga005.fm.intel.com ([10.253.24.32])
 by fmsmga105.fm.intel.com with ESMTP; 13 Oct 2017 08:39:58 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.43,371,1503385200"; d="scan'208";a="162355218"
Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157])
 by fmsmga005.fm.intel.com with ESMTP; 13 Oct 2017 08:39:57 -0700
Received: from irsmsx108.ger.corp.intel.com ([169.254.11.167]) by
 IRSMSX103.ger.corp.intel.com ([169.254.3.49]) with mapi id 14.03.0319.002;
 Fri, 13 Oct 2017 16:39:56 +0100
From: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: "Singh, Jasvinder" <jasvinder.singh@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>, "Yigit, Ferruh" <ferruh.yigit@intel.com>, "Ananyev,
 Konstantin" <konstantin.ananyev@intel.com>, "Dumitrescu, Cristian"
 <cristian.dumitrescu@intel.com>, "adrien.mazarguil@6wind.com"
 <adrien.mazarguil@6wind.com>
CC: "Iremonger, Bernard" <bernard.iremonger@intel.com>
Thread-Topic: [dpdk-dev] [PATCH v7 1/4] librte_flow_classify: add
 librte_flow_classify library
Thread-Index: AQHTPrPovjzPq8rewE6rE0QpQ4lHtqLh9EhQ
Date: Fri, 13 Oct 2017 15:39:56 +0000
Message-ID: <8CEF83825BEC744B83065625E567D7C24E052811@IRSMSX108.ger.corp.intel.com>
References: <1506676737-23900-1-git-send-email-bernard.iremonger@intel.com>
 <1506936668-31197-2-git-send-email-bernard.iremonger@intel.com>
 <54CBAA185211B4429112C315DA58FF6D33264ABA@IRSMSX103.ger.corp.intel.com>
In-Reply-To: <54CBAA185211B4429112C315DA58FF6D33264ABA@IRSMSX103.ger.corp.intel.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzlhNmNiMjctMjBjZC00ZmYyLTliN2YtZjkwNGRjYmU0MWY5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImpNZ1BQVUpSWHVvTjQ0NlVwTWplTlJQbXQ0aWFlUlpaNmhvYjNUZEVyeDg9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
x-originating-ip: [163.33.239.180]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v7 1/4] librte_flow_classify:
	add	librte_flow_classify library
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 13 Oct 2017 15:40:01 -0000

Hi Jasvinder,

> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Friday, October 6, 2017 4:01 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org;
> Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; adrien.mazarguil@6wind.com
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v7 1/4] librte_flow_classify: add
> librte_flow_classify library
>=20
> Hi Bernard,
>=20
> <snip>
>=20
> > +struct rte_flow_classify *
> > +rte_flow_classify_create(void *table_handle,
> > +		uint32_t entry_size,
> > +		const struct rte_flow_attr *attr,
> > +		const struct rte_flow_item pattern[],
> > +		const struct rte_flow_action actions[],
> > +		struct rte_flow_error *error)
> > +{
> > +	struct rte_flow_classify *flow_classify;
> > +	int ret;
> > +
> > +	if (!error)
> > +		return NULL;
> > +
> > +	if (!table_handle) {
> > +		rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_HANDLE,
> > +				   NULL, "NULL table_handle.");
> > +		return NULL;
> > +	}
> > +
> > +	if (!pattern) {
> > +		rte_flow_error_set(error, EINVAL,
> > RTE_FLOW_ERROR_TYPE_ITEM_NUM,
> > +				   NULL, "NULL pattern.");
> > +		return NULL;
> > +	}
> > +
> > +	if (!actions) {
> > +		rte_flow_error_set(error, EINVAL,
> > +				   RTE_FLOW_ERROR_TYPE_ACTION_NUM,
> > +				   NULL, "NULL action.");
> > +		return NULL;
> > +	}
> > +
> > +	if (!attr) {
> > +		rte_flow_error_set(error, EINVAL,
> > +				   RTE_FLOW_ERROR_TYPE_ATTR,
> > +				   NULL, "NULL attribute.");
> > +		return NULL;
> > +	}
> > +
> > +	/* parse attr, pattern and actions */
> > +	ret =3D rte_flow_classify_validate(table_handle, attr, pattern,
> > +			actions, error);
> > +	if (ret < 0)
> > +		return NULL;
> > +
> > +	flow_classify =3D allocate_5tuple();
> > +	if (!flow_classify)
> > +		return NULL;
> > +
> > +	flow_classify->entry =3D malloc(entry_size);
> > +	if (!flow_classify->entry) {
> > +		free(flow_classify);
> > +		flow_classify =3D NULL;
> > +		return NULL;
> > +	}
> > +	memset(flow_classify->entry, 0, entry_size);
> > +	memmove(flow_classify->entry, &flow_classify->id,
> > sizeof(uint32_t));
> > +
> > +	ret =3D rte_table_acl_ops.f_add(table_handle, &flow_classify-
> > >key_add,
> > +			flow_classify->entry, &flow_classify->key_found,
> > +			&flow_classify->entry_ptr);
> > +	if (ret) {
> > +		free(flow_classify->entry);
> > +		free(flow_classify);
> > +		flow_classify =3D NULL;
> > +		return NULL;
> > +	}
> > +
> > +	return flow_classify;
> > +}
>=20
> The API in its current form creates the classifier object which will alwa=
ys use
> librte_acl based classification mechanism. This behavior imposes restrict=
ion
> on the application to always pass only ACL table related parameters for f=
low
> classification. In my opinion, API implementation should be agnostic to
> specific classification method and should be generic enough to allow
> application to select any of the available flow classification method (fo=
r e.g.
> acl, hash, LPM, etc.). Otherwise, this library will become another abstra=
ction
> of librte_acl for flow classification.
>=20
> Also, library allows table entries to be added while creating the classif=
ier
> object, not later. Is there any specific reason?

Thanks for reviewing this patchset.

I will rework the code so that the API's are table agnostic.
In the v7 patchset the application creates the table ACL and then classify =
rules can be added and deleted at will.

I will send a v8 patchset.

Regards,

Bernard.