From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Singh, Jasvinder" <jasvinder.singh@intel.com>
Subject: Re: [PATCH v7 1/4] librte_flow_classify:
	add	librte_flow_classify library
Date: Fri, 6 Oct 2017 15:00:54 +0000
Message-ID: <54CBAA185211B4429112C315DA58FF6D33264ABA@IRSMSX103.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>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Cc: "Iremonger, Bernard" <bernard.iremonger@intel.com>
To: "Iremonger, Bernard" <bernard.iremonger@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>
Return-path: <dev-bounces@dpdk.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id 76116199B6
 for <dev@dpdk.org>; Fri,  6 Oct 2017 17:00:59 +0200 (CEST)
In-Reply-To: <1506936668-31197-2-git-send-email-bernard.iremonger@intel.com>
Content-Language: en-US
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Bernard,

<snip>

> +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;
> +}

The API in its current form creates the classifier object which will always=
 use librte_acl based classification mechanism. This behavior
imposes restriction on the application to always pass only ACL table relate=
d parameters for flow 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 classificat=
ion method (for e.g. acl, hash, LPM, etc.). Otherwise, this library will be=
come another abstraction of librte_acl for flow classification.

Also, library allows table entries to be added while creating the classifie=
r object, not later. Is there any specific reason?=20