[dpdk-dev,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@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(-)
  

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Wednesday, August 23, 2017 2:51 PM
> To: 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: [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@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
  

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@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.
  

Hi Cristian,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Iremonger, Bernard
> Sent: Wednesday, August 23, 2017 3:32 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org;
> Yigit, Ferruh <ferruh.yigit@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; adrien.mazarguil@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@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.