From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <jasvinder.singh@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id 8B127DE3;
 Mon, 18 Sep 2017 17:29:52 +0200 (CEST)
Received: from orsmga002.jf.intel.com ([10.7.209.21])
 by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 18 Sep 2017 08:29:51 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.42,413,1500966000"; d="scan'208";a="136611640"
Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59])
 by orsmga002.jf.intel.com with ESMTP; 18 Sep 2017 08:29:50 -0700
Received: from irsmsx103.ger.corp.intel.com ([169.254.3.49]) by
 IRSMSX151.ger.corp.intel.com ([169.254.4.108]) with mapi id 14.03.0319.002;
 Mon, 18 Sep 2017 16:29:49 +0100
From: "Singh, Jasvinder" <jasvinder.singh@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>
CC: "Iremonger, Bernard" <bernard.iremonger@intel.com>, "stable@dpdk.org"
 <stable@dpdk.org>
Thread-Topic: [dpdk-dev] [PATCH v5 1/6] librte_table: fix acl entry add and
 delete functions
Thread-Index: AQHTJ/iX3VkPXfHcBku0Imgmd0M9q6K60Eyw
Date: Mon, 18 Sep 2017 15:29:48 +0000
Message-ID: <54CBAA185211B4429112C315DA58FF6D33253912@IRSMSX103.ger.corp.intel.com>
References: <1504693668-2062-1-git-send-email-bernard.iremonger@intel.com>
 <1504802598-27296-2-git-send-email-bernard.iremonger@intel.com>
In-Reply-To: <1504802598-27296-2-git-send-email-bernard.iremonger@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYmY0NWU0NjMtZjhiYy00NjQ5LTg4NTMtMzJhMzA5NmE2MWZhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImZCY25LZWtDU2tWenU5MmMzSURTWWVoazc0cmE1TUVkYUxQNVJuRHBveUE9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v5 1/6] librte_table: fix acl entry add
	and	delete functions
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: Mon, 18 Sep 2017 15:29:53 -0000

Hi Bernard,

<snip>

> --- a/lib/librte_table/rte_table_acl.c
> +++ b/lib/librte_table/rte_table_acl.c
> @@ -316,8 +316,7 @@ struct rte_table_acl {
>  		if (status =3D=3D 0) {
>  			*key_found =3D 1;
>  			*entry_ptr =3D &acl->memory[i * acl->entry_size];
> -			memcpy(*entry_ptr, entry, acl->entry_size);
> -
> +			memcpy(entry, *entry_ptr, acl->entry_size);
>  			return 0;
>  		}
>  	}

In this case, table entry which is being added already presents in the tabl=
e. So, first the pointer to that entry from the memory[] that stores the  p=
ipeline table data(struct rte_pipeline_table_entry) associated with key is =
retrieved and the changes (action and metadara) are stored in the internal =
table pointed by action_table. So, above fix doesn't seem correct.

> @@ -353,8 +352,8 @@ struct rte_table_acl {
>  		rte_acl_free(acl->ctx);
>  	acl->ctx =3D ctx;
>  	*key_found =3D 0;
> -	*entry_ptr =3D &acl->memory[free_pos * acl->entry_size];
> -	memcpy(*entry_ptr, entry, acl->entry_size);
> +	*entry_ptr =3D &acl->acl_rule_memory[free_pos * acl->entry_size];
> +	memcpy(entry, *entry_ptr, acl->entry_size);
>=20
>  	return 0;
>  }
> @@ -435,7 +434,7 @@ struct rte_table_acl {
>  	acl->ctx =3D ctx;
>  	*key_found =3D 1;
>  	if (entry !=3D NULL)
> -		memcpy(entry, &acl->memory[pos * acl->entry_size],
> +		memcpy(entry, &acl->acl_rule_memory[pos * acl-
> >entry_size],
>  			acl->entry_size);


Above fixes also seems not correct. As per documentation, *entry_ptr is int=
ended to store the handle to the pipeline table entry containing the data a=
ssociated with the current key instead of pointing to memory used to store =
the acl rules, etc. Please refer rte_table_acl_create() where memory is ini=
tialized and organized to stores different types of internal tables (pointe=
d by action_table, acl_rule_list and acl_rule_memory).