[dpdk-dev] [PATCH v1 1/6] librte_table: move structure to header file

[Iremonger, Bernard]

Hi Cristian,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Iremonger, Bernard
> Sent: Wednesday, August 23, 2017 3:32 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu at intel.com>; dev at dpdk.org;
> Yigit, Ferruh <ferruh.yigit at intel.com>; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>; adrien.mazarguil at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH v1 1/6] librte_table: move structure to
> header file
> 
> Hi Cristian,
> 
> <snip>
> 
> > > Subject: [PATCH v1 1/6] librte_table: move structure to header file
> > >
> > > Move struct librte_table from the rte_table_acl.c to the
> > > rte_table_acl.h file.
> > >
> > > Signed-off-by: Bernard Iremonger <bernard.iremonger at intel.com>
> > > ---
> > >  lib/librte_table/rte_table_acl.c | 24 ------------------------
> > > lib/librte_table/rte_table_acl.h | 24 ++++++++++++++++++++++++
> > >  2 files changed, 24 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/lib/librte_table/rte_table_acl.c
> > > b/lib/librte_table/rte_table_acl.c
> > > index 3c05e4a..900f658 100644
> > > --- a/lib/librte_table/rte_table_acl.c
> > > +++ b/lib/librte_table/rte_table_acl.c
> > > @@ -57,30 +57,6 @@
> > >
> > >  #endif
> > >
> > > -struct rte_table_acl {
> > > -	struct rte_table_stats stats;
> > > -
> > > -	/* Low-level ACL table */
> > > -	char name[2][RTE_ACL_NAMESIZE];
> > > -	struct rte_acl_param acl_params; /* for creating low level acl table */
> > > -	struct rte_acl_config cfg; /* Holds the field definitions (metadata) */
> > > -	struct rte_acl_ctx *ctx;
> > > -	uint32_t name_id;
> > > -
> > > -	/* Input parameters */
> > > -	uint32_t n_rules;
> > > -	uint32_t entry_size;
> > > -
> > > -	/* Internal tables */
> > > -	uint8_t *action_table;
> > > -	struct rte_acl_rule **acl_rule_list; /* Array of pointers to rules */
> > > -	uint8_t *acl_rule_memory; /* Memory to store the rules */
> > > -
> > > -	/* Memory to store the action table and stack of free entries */
> > > -	uint8_t memory[0] __rte_cache_aligned;
> > > -};
> > > -
> > > -
> > >  static void *
> > >  rte_table_acl_create(
> > >  	void *params,
> > > diff --git a/lib/librte_table/rte_table_acl.h
> > > b/lib/librte_table/rte_table_acl.h
> > > index a9cc032..1370b12 100644
> > > --- a/lib/librte_table/rte_table_acl.h
> > > +++ b/lib/librte_table/rte_table_acl.h
> > > @@ -55,6 +55,30 @@
> > >
> > >  #include "rte_table.h"
> > >
> > > +
> > > +struct rte_table_acl {
> > > +	struct rte_table_stats stats;
> > > +
> > > +	/* Low-level ACL table */
> > > +	char name[2][RTE_ACL_NAMESIZE];
> > > +	struct rte_acl_param acl_params; /* for creating low level acl table */
> > > +	struct rte_acl_config cfg; /* Holds the field definitions (metadata) */
> > > +	struct rte_acl_ctx *ctx;
> > > +	uint32_t name_id;
> > > +
> > > +	/* Input parameters */
> > > +	uint32_t n_rules;
> > > +	uint32_t entry_size;
> > > +
> > > +	/* Internal tables */
> > > +	uint8_t *action_table;
> > > +	struct rte_acl_rule **acl_rule_list; /* Array of pointers to rules */
> > > +	uint8_t *acl_rule_memory; /* Memory to store the rules */
> > > +
> > > +	/* Memory to store the action table and stack of free entries */
> > > +	uint8_t memory[0] __rte_cache_aligned; };
> > > +
> > >  /** ACL table parameters */
> > >  struct rte_table_acl_params {
> > >  	/** Name */
> > > --
> > > 1.9.1
> >
> >
> > Hi Bernard,
> >
> > Strong objection here:
> > - This data structure contains the internal data needed to run the ACL
> > table. It is implementation dependent, it is not part of the API.
> > Therefore, it must not be exposed as part of the API, so it has to
> > stay in the .c file as opposed to the .h file.
> > - Users should handle the ACL table through the handle returned by the
> > create function as opposed to accessing this structure directly.
> >
> > Regards,
> > Cristian
> 
> I will revisit this to see if there is another way.
> 
> Regards,
> 
> Bernard.
> 

This patch has been dropped from the v2 patch set.
The functionality needed has been implemented in a different way.

Regards,

Bernard.