[dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[David Harton]

Some compilers generate warnings for duplicate symbols for the
set of filter lists current defined in ixgbe_ethdev.h.
This commits moves the defintion and declaration to the source
file that actually uses them and provides a function to
initialize the values akin to its flush function.

Signed-off-by: David Harton <dharton@cisco.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
 drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
 drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 1ec5aaf..ed21af5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
 	/* initialize l2 tunnel filter list & hash */
 	ixgbe_l2_tn_filter_init(eth_dev);
 
-	TAILQ_INIT(&filter_ntuple_list);
-	TAILQ_INIT(&filter_ethertype_list);
-	TAILQ_INIT(&filter_syn_list);
-	TAILQ_INIT(&filter_fdir_list);
-	TAILQ_INIT(&filter_l2_tunnel_list);
-	TAILQ_INIT(&ixgbe_flow_list);
+	/* initialize flow filter lists */
+	ixgbe_filterlist_init();
 
 	/* initialize bandwidth configuration info */
 	memset(bw_conf, 0, sizeof(struct ixgbe_bw_conf));
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index caa50c8..3f1aeb5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -396,17 +396,11 @@ struct ixgbe_flow_mem {
 };
 
 TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele);
-struct ixgbe_ntuple_filter_list filter_ntuple_list;
 TAILQ_HEAD(ixgbe_ethertype_filter_list, ixgbe_ethertype_filter_ele);
-struct ixgbe_ethertype_filter_list filter_ethertype_list;
 TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele);
-struct ixgbe_syn_filter_list filter_syn_list;
 TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele);
-struct ixgbe_fdir_rule_filter_list filter_fdir_list;
 TAILQ_HEAD(ixgbe_l2_tunnel_filter_list, ixgbe_eth_l2_tunnel_conf_ele);
-struct ixgbe_l2_tunnel_filter_list filter_l2_tunnel_list;
 TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem);
-struct ixgbe_flow_mem_list ixgbe_flow_list;
 
 /*
  * Statistics counters collected by the MACsec
@@ -692,6 +686,7 @@ int ixgbe_syn_filter_set(struct rte_eth_dev *dev,
 int
 ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
 			       struct rte_eth_l2_tunnel_conf *l2_tunnel);
+void ixgbe_filterlist_init(void);
 void ixgbe_filterlist_flush(void);
 /*
  * Flow director function prototypes
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index d679608..6dbfd25 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -79,6 +79,13 @@
 #define IXGBE_MAX_N_TUPLE_PRIO 7
 #define IXGBE_MAX_FLX_SOURCE_OFF 62
 
+static struct ixgbe_ntuple_filter_list filter_ntuple_list;
+static struct ixgbe_ethertype_filter_list filter_ethertype_list;
+static struct ixgbe_syn_filter_list filter_syn_list;
+static struct ixgbe_fdir_rule_filter_list filter_fdir_list;
+static struct ixgbe_l2_tunnel_filter_list filter_l2_tunnel_list;
+static struct ixgbe_flow_mem_list ixgbe_flow_list;
+
 /**
  * Endless loop will never happen with below assumption
  * 1. there is at least one no-void item(END)
@@ -2600,6 +2607,17 @@ static inline uint8_t signature_match(const struct rte_flow_item pattern[])
 }
 
 void
+ixgbe_filterlist_init(void)
+{
+	TAILQ_INIT(&filter_ntuple_list);
+	TAILQ_INIT(&filter_ethertype_list);
+	TAILQ_INIT(&filter_syn_list);
+	TAILQ_INIT(&filter_fdir_list);
+	TAILQ_INIT(&filter_l2_tunnel_list);
+	TAILQ_INIT(&ixgbe_flow_list);
+}
+
+void
 ixgbe_filterlist_flush(void)
 {
 	struct ixgbe_ntuple_filter_ele *ntuple_filter_ptr;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[Ferruh Yigit]

On 8/25/2017 8:21 PM, David Harton wrote:
> Some compilers generate warnings for duplicate symbols for the
> set of filter lists current defined in ixgbe_ethdev.h.
> This commits moves the defintion and declaration to the source
> file that actually uses them and provides a function to
> initialize the values akin to its flush function.
> 
> Signed-off-by: David Harton <dharton@cisco.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
>  drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
>  drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
>  3 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 1ec5aaf..ed21af5 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
>  	/* initialize l2 tunnel filter list & hash */
>  	ixgbe_l2_tn_filter_init(eth_dev);
>  
> -	TAILQ_INIT(&filter_ntuple_list);
> -	TAILQ_INIT(&filter_ethertype_list);
> -	TAILQ_INIT(&filter_syn_list);
> -	TAILQ_INIT(&filter_fdir_list);
> -	TAILQ_INIT(&filter_l2_tunnel_list);
> -	TAILQ_INIT(&ixgbe_flow_list);
> +	/* initialize flow filter lists */
> +	ixgbe_filterlist_init();

Will it work if list initialization converted to the static
initialization? That would remove requirement of this function call.

something like:

diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index c8645f056..6184eb4af 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -79,6 +79,24 @@
 #define IXGBE_MAX_N_TUPLE_PRIO 7
 #define IXGBE_MAX_FLX_SOURCE_OFF 62

+static TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele)
+       filter_ntuple_list = TAILQ_HEAD_INITIALIZER(filter_ntuple_list);
+
+static TAILQ_HEAD(ixgbe_ethertype_filter_list, ixgbe_ethertype_filter_ele)
+       filter_ethertype_list =
TAILQ_HEAD_INITIALIZER(filter_ethertype_list);
+
+static TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele)
+       filter_syn_list = TAILQ_HEAD_INITIALIZER(filter_syn_list);
+
+static TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele)
+       filter_fdir_list = TAILQ_HEAD_INITIALIZER(filter_fdir_list);
+
+static TAILQ_HEAD(ixgbe_l2_tunnel_filter_list,
ixgbe_eth_l2_tunnel_conf_ele)
+       filter_l2_tunnel_list =
TAILQ_HEAD_INITIALIZER(filter_l2_tunnel_list);
+
+static TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem)
+       ixgbe_flow_list = TAILQ_HEAD_INITIALIZER(ixgbe_flow_list);
+

Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[David Harton (dharton)]

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, August 28, 2017 9:27 AM
> To: David Harton (dharton) <dharton@cisco.com>;
> konstantin.ananyev@intel.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist
> symbols
> 
> On 8/25/2017 8:21 PM, David Harton wrote:
> > Some compilers generate warnings for duplicate symbols for the set of
> > filter lists current defined in ixgbe_ethdev.h.
> > This commits moves the defintion and declaration to the source file
> > that actually uses them and provides a function to initialize the
> > values akin to its flush function.
> >
> > Signed-off-by: David Harton <dharton@cisco.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
> > drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
> >  drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
> >  3 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 1ec5aaf..ed21af5 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
> >  	/* initialize l2 tunnel filter list & hash */
> >  	ixgbe_l2_tn_filter_init(eth_dev);
> >
> > -	TAILQ_INIT(&filter_ntuple_list);
> > -	TAILQ_INIT(&filter_ethertype_list);
> > -	TAILQ_INIT(&filter_syn_list);
> > -	TAILQ_INIT(&filter_fdir_list);
> > -	TAILQ_INIT(&filter_l2_tunnel_list);
> > -	TAILQ_INIT(&ixgbe_flow_list);
> > +	/* initialize flow filter lists */
> > +	ixgbe_filterlist_init();
> 
> Will it work if list initialization converted to the static
> initialization? That would remove requirement of this function call.

I was trying to keep the behavior the same:
  .probe -> eth_ixgbe_dev_init -> ixgbe_filterlist_init
  .remove -> eth_ixgbe_dev_uninit -> ixgbe_filterlist_flush

While I think the below would work I always like mirror/parallel relationships like ctor/dtor for objects.

Is that reasonable?

Thanks,
Dave

> 
> something like:
> 
> diff --git a/drivers/net/ixgbe/ixgbe_flow.c
> b/drivers/net/ixgbe/ixgbe_flow.c index c8645f056..6184eb4af 100644
> --- a/drivers/net/ixgbe/ixgbe_flow.c
> +++ b/drivers/net/ixgbe/ixgbe_flow.c
> @@ -79,6 +79,24 @@
>  #define IXGBE_MAX_N_TUPLE_PRIO 7
>  #define IXGBE_MAX_FLX_SOURCE_OFF 62
> 
> +static TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele)
> +       filter_ntuple_list = TAILQ_HEAD_INITIALIZER(filter_ntuple_list);
> +
> +static TAILQ_HEAD(ixgbe_ethertype_filter_list,
> ixgbe_ethertype_filter_ele)
> +       filter_ethertype_list =
> TAILQ_HEAD_INITIALIZER(filter_ethertype_list);
> +
> +static TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele)
> +       filter_syn_list = TAILQ_HEAD_INITIALIZER(filter_syn_list);
> +
> +static TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele)
> +       filter_fdir_list = TAILQ_HEAD_INITIALIZER(filter_fdir_list);
> +
> +static TAILQ_HEAD(ixgbe_l2_tunnel_filter_list,
> ixgbe_eth_l2_tunnel_conf_ele)
> +       filter_l2_tunnel_list =
> TAILQ_HEAD_INITIALIZER(filter_l2_tunnel_list);
> +
> +static TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem)
> +       ixgbe_flow_list = TAILQ_HEAD_INITIALIZER(ixgbe_flow_list);
> +

Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[Ferruh Yigit]

On 8/28/2017 6:13 PM, David Harton (dharton) wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Monday, August 28, 2017 9:27 AM
>> To: David Harton (dharton) <dharton@cisco.com>;
>> konstantin.ananyev@intel.com
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist
>> symbols
>>
>> On 8/25/2017 8:21 PM, David Harton wrote:
>>> Some compilers generate warnings for duplicate symbols for the set of
>>> filter lists current defined in ixgbe_ethdev.h.
>>> This commits moves the defintion and declaration to the source file
>>> that actually uses them and provides a function to initialize the
>>> values akin to its flush function.
>>>
>>> Signed-off-by: David Harton <dharton@cisco.com>
>>> ---
>>>  drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
>>> drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
>>>  drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
>>>  3 files changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> index 1ec5aaf..ed21af5 100644
>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> @@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
>>>  	/* initialize l2 tunnel filter list & hash */
>>>  	ixgbe_l2_tn_filter_init(eth_dev);
>>>
>>> -	TAILQ_INIT(&filter_ntuple_list);
>>> -	TAILQ_INIT(&filter_ethertype_list);
>>> -	TAILQ_INIT(&filter_syn_list);
>>> -	TAILQ_INIT(&filter_fdir_list);
>>> -	TAILQ_INIT(&filter_l2_tunnel_list);
>>> -	TAILQ_INIT(&ixgbe_flow_list);
>>> +	/* initialize flow filter lists */
>>> +	ixgbe_filterlist_init();
>>
>> Will it work if list initialization converted to the static
>> initialization? That would remove requirement of this function call.
> 
> I was trying to keep the behavior the same:
>   .probe -> eth_ixgbe_dev_init -> ixgbe_filterlist_init
>   .remove -> eth_ixgbe_dev_uninit -> ixgbe_filterlist_flush
> 
> While I think the below would work I always like mirror/parallel relationships like ctor/dtor for objects.

I would agree if ixgbe_filterlist_init() initializes something more than
list itself, but for this case initialization can be avoided completely
which I would prefer.

Driver maintainer also cc'ed for more comment.

> 
> Is that reasonable?
> 
> Thanks,
> Dave
> 
>>
>> something like:
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_flow.c
>> b/drivers/net/ixgbe/ixgbe_flow.c index c8645f056..6184eb4af 100644
>> --- a/drivers/net/ixgbe/ixgbe_flow.c
>> +++ b/drivers/net/ixgbe/ixgbe_flow.c
>> @@ -79,6 +79,24 @@
>>  #define IXGBE_MAX_N_TUPLE_PRIO 7
>>  #define IXGBE_MAX_FLX_SOURCE_OFF 62
>>
>> +static TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele)
>> +       filter_ntuple_list = TAILQ_HEAD_INITIALIZER(filter_ntuple_list);
>> +
>> +static TAILQ_HEAD(ixgbe_ethertype_filter_list,
>> ixgbe_ethertype_filter_ele)
>> +       filter_ethertype_list =
>> TAILQ_HEAD_INITIALIZER(filter_ethertype_list);
>> +
>> +static TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele)
>> +       filter_syn_list = TAILQ_HEAD_INITIALIZER(filter_syn_list);
>> +
>> +static TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele)
>> +       filter_fdir_list = TAILQ_HEAD_INITIALIZER(filter_fdir_list);
>> +
>> +static TAILQ_HEAD(ixgbe_l2_tunnel_filter_list,
>> ixgbe_eth_l2_tunnel_conf_ele)
>> +       filter_l2_tunnel_list =
>> TAILQ_HEAD_INITIALIZER(filter_l2_tunnel_list);
>> +
>> +static TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem)
>> +       ixgbe_flow_list = TAILQ_HEAD_INITIALIZER(ixgbe_flow_list);
>> +

Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[Ferruh Yigit]

On 8/29/2017 5:23 PM, Ferruh Yigit wrote:
> On 8/28/2017 6:13 PM, David Harton (dharton) wrote:
>>
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>> Sent: Monday, August 28, 2017 9:27 AM
>>> To: David Harton (dharton) <dharton@cisco.com>;
>>> konstantin.ananyev@intel.com
>>> Cc: dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist
>>> symbols
>>>
>>> On 8/25/2017 8:21 PM, David Harton wrote:
>>>> Some compilers generate warnings for duplicate symbols for the set of
>>>> filter lists current defined in ixgbe_ethdev.h.
>>>> This commits moves the defintion and declaration to the source file
>>>> that actually uses them and provides a function to initialize the
>>>> values akin to its flush function.
>>>>
>>>> Signed-off-by: David Harton <dharton@cisco.com>
>>>> ---
>>>>  drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
>>>> drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
>>>>  drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
>>>>  3 files changed, 21 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>> index 1ec5aaf..ed21af5 100644
>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>> @@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
>>>>  	/* initialize l2 tunnel filter list & hash */
>>>>  	ixgbe_l2_tn_filter_init(eth_dev);
>>>>
>>>> -	TAILQ_INIT(&filter_ntuple_list);
>>>> -	TAILQ_INIT(&filter_ethertype_list);
>>>> -	TAILQ_INIT(&filter_syn_list);
>>>> -	TAILQ_INIT(&filter_fdir_list);
>>>> -	TAILQ_INIT(&filter_l2_tunnel_list);
>>>> -	TAILQ_INIT(&ixgbe_flow_list);
>>>> +	/* initialize flow filter lists */
>>>> +	ixgbe_filterlist_init();
>>>
>>> Will it work if list initialization converted to the static
>>> initialization? That would remove requirement of this function call.
>>
>> I was trying to keep the behavior the same:
>>   .probe -> eth_ixgbe_dev_init -> ixgbe_filterlist_init
>>   .remove -> eth_ixgbe_dev_uninit -> ixgbe_filterlist_flush
>>
>> While I think the below would work I always like mirror/parallel relationships like ctor/dtor for objects.
> 
> I would agree if ixgbe_filterlist_init() initializes something more than
> list itself, but for this case initialization can be avoided completely
> which I would prefer.
> 
> Driver maintainer also cc'ed for more comment.

It seems there is no more comment. So, I suggest lets focus on your fix,
my suggestion to convert to static initialization can be done later if
desired.

Still, what do you think about moving declaration of structs as well to
the ixgbe_flow.c? After your change, only ixgbe_flow.c will be using
them, no need to keep them in ixgbe_ethdev.h.

Thanks,
ferruh

> 
>>
>> Is that reasonable?
>>
>> Thanks,
>> Dave
>>
>>>
>>> something like:
>>>
>>> diff --git a/drivers/net/ixgbe/ixgbe_flow.c
>>> b/drivers/net/ixgbe/ixgbe_flow.c index c8645f056..6184eb4af 100644
>>> --- a/drivers/net/ixgbe/ixgbe_flow.c
>>> +++ b/drivers/net/ixgbe/ixgbe_flow.c
>>> @@ -79,6 +79,24 @@
>>>  #define IXGBE_MAX_N_TUPLE_PRIO 7
>>>  #define IXGBE_MAX_FLX_SOURCE_OFF 62
>>>
>>> +static TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele)
>>> +       filter_ntuple_list = TAILQ_HEAD_INITIALIZER(filter_ntuple_list);
>>> +
>>> +static TAILQ_HEAD(ixgbe_ethertype_filter_list,
>>> ixgbe_ethertype_filter_ele)
>>> +       filter_ethertype_list =
>>> TAILQ_HEAD_INITIALIZER(filter_ethertype_list);
>>> +
>>> +static TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele)
>>> +       filter_syn_list = TAILQ_HEAD_INITIALIZER(filter_syn_list);
>>> +
>>> +static TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele)
>>> +       filter_fdir_list = TAILQ_HEAD_INITIALIZER(filter_fdir_list);
>>> +
>>> +static TAILQ_HEAD(ixgbe_l2_tunnel_filter_list,
>>> ixgbe_eth_l2_tunnel_conf_ele)
>>> +       filter_l2_tunnel_list =
>>> TAILQ_HEAD_INITIALIZER(filter_l2_tunnel_list);
>>> +
>>> +static TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem)
>>> +       ixgbe_flow_list = TAILQ_HEAD_INITIALIZER(ixgbe_flow_list);
>>> +
> 

Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[David Harton (dharton)]

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, September 14, 2017 6:10 AM
> To: David Harton (dharton) <dharton@cisco.com>;
> konstantin.ananyev@intel.com
> Cc: dev@dpdk.org; Wenzhuo Lu <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist
> symbols
> 
> On 8/29/2017 5:23 PM, Ferruh Yigit wrote:
> > On 8/28/2017 6:13 PM, David Harton (dharton) wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >>> Sent: Monday, August 28, 2017 9:27 AM
> >>> To: David Harton (dharton) <dharton@cisco.com>;
> >>> konstantin.ananyev@intel.com
> >>> Cc: dev@dpdk.org
> >>> Subject: Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate
> >>> filterlist symbols
> >>>
> >>> On 8/25/2017 8:21 PM, David Harton wrote:
> >>>> Some compilers generate warnings for duplicate symbols for the set
> >>>> of filter lists current defined in ixgbe_ethdev.h.
> >>>> This commits moves the defintion and declaration to the source file
> >>>> that actually uses them and provides a function to initialize the
> >>>> values akin to its flush function.
> >>>>
> >>>> Signed-off-by: David Harton <dharton@cisco.com>
> >>>> ---
> >>>>  drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
> >>>> drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
> >>>>  drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
> >>>>  3 files changed, 21 insertions(+), 12 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>> index 1ec5aaf..ed21af5 100644
> >>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>>> @@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
> >>>>  	/* initialize l2 tunnel filter list & hash */
> >>>>  	ixgbe_l2_tn_filter_init(eth_dev);
> >>>>
> >>>> -	TAILQ_INIT(&filter_ntuple_list);
> >>>> -	TAILQ_INIT(&filter_ethertype_list);
> >>>> -	TAILQ_INIT(&filter_syn_list);
> >>>> -	TAILQ_INIT(&filter_fdir_list);
> >>>> -	TAILQ_INIT(&filter_l2_tunnel_list);
> >>>> -	TAILQ_INIT(&ixgbe_flow_list);
> >>>> +	/* initialize flow filter lists */
> >>>> +	ixgbe_filterlist_init();
> >>>
> >>> Will it work if list initialization converted to the static
> >>> initialization? That would remove requirement of this function call.
> >>
> >> I was trying to keep the behavior the same:
> >>   .probe -> eth_ixgbe_dev_init -> ixgbe_filterlist_init
> >>   .remove -> eth_ixgbe_dev_uninit -> ixgbe_filterlist_flush
> >>
> >> While I think the below would work I always like mirror/parallel
> relationships like ctor/dtor for objects.
> >
> > I would agree if ixgbe_filterlist_init() initializes something more
> > than list itself, but for this case initialization can be avoided
> > completely which I would prefer.
> >
> > Driver maintainer also cc'ed for more comment.
> 
> It seems there is no more comment. So, I suggest lets focus on your fix,
> my suggestion to convert to static initialization can be done later if
> desired.
> 
> Still, what do you think about moving declaration of structs as well to
> the ixgbe_flow.c? After your change, only ixgbe_flow.c will be using them,
> no need to keep them in ixgbe_ethdev.h.

Works for me.  Let me make the changes and test.

Regards,
Dave

> 
> Thanks,
> ferruh
> 
> >
> >>
> >> Is that reasonable?
> >>
> >> Thanks,
> >> Dave
> >>
> >>>
> >>> something like:
> >>>
> >>> diff --git a/drivers/net/ixgbe/ixgbe_flow.c
> >>> b/drivers/net/ixgbe/ixgbe_flow.c index c8645f056..6184eb4af 100644
> >>> --- a/drivers/net/ixgbe/ixgbe_flow.c
> >>> +++ b/drivers/net/ixgbe/ixgbe_flow.c
> >>> @@ -79,6 +79,24 @@
> >>>  #define IXGBE_MAX_N_TUPLE_PRIO 7
> >>>  #define IXGBE_MAX_FLX_SOURCE_OFF 62
> >>>
> >>> +static TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele)
> >>> +       filter_ntuple_list =
> >>> +TAILQ_HEAD_INITIALIZER(filter_ntuple_list);
> >>> +
> >>> +static TAILQ_HEAD(ixgbe_ethertype_filter_list,
> >>> ixgbe_ethertype_filter_ele)
> >>> +       filter_ethertype_list =
> >>> TAILQ_HEAD_INITIALIZER(filter_ethertype_list);
> >>> +
> >>> +static TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele)
> >>> +       filter_syn_list = TAILQ_HEAD_INITIALIZER(filter_syn_list);
> >>> +
> >>> +static TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele)
> >>> +       filter_fdir_list = TAILQ_HEAD_INITIALIZER(filter_fdir_list);
> >>> +
> >>> +static TAILQ_HEAD(ixgbe_l2_tunnel_filter_list,
> >>> ixgbe_eth_l2_tunnel_conf_ele)
> >>> +       filter_l2_tunnel_list =
> >>> TAILQ_HEAD_INITIALIZER(filter_l2_tunnel_list);
> >>> +
> >>> +static TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem)
> >>> +       ixgbe_flow_list = TAILQ_HEAD_INITIALIZER(ixgbe_flow_list);
> >>> +
> >

Re: [dpdk-dev] [PATCH] ixgbe: eliminate duplicate filterlist symbols

[David Harton (dharton)]

Hi Konstantin,

A gentle reminder to weigh in on this patch and the 
discussion between Ferruh and myself below.

Thanks,
Dave

> -----Original Message-----
> From: David Harton (dharton)
> 
> > -----Original Message-----
> > From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >
> > On 8/25/2017 8:21 PM, David Harton wrote:
> > > Some compilers generate warnings for duplicate symbols for the set
> > > of filter lists current defined in ixgbe_ethdev.h.
> > > This commits moves the defintion and declaration to the source file
> > > that actually uses them and provides a function to initialize the
> > > values akin to its flush function.
> > >
> > > Signed-off-by: David Harton <dharton@cisco.com>
> > > ---
> > >  drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++------
> > > drivers/net/ixgbe/ixgbe_ethdev.h |  7 +------
> > >  drivers/net/ixgbe/ixgbe_flow.c   | 18 ++++++++++++++++++
> > >  3 files changed, 21 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > index 1ec5aaf..ed21af5 100644
> > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > @@ -1332,12 +1332,8 @@ struct rte_ixgbe_xstats_name_off {
> > >  	/* initialize l2 tunnel filter list & hash */
> > >  	ixgbe_l2_tn_filter_init(eth_dev);
> > >
> > > -	TAILQ_INIT(&filter_ntuple_list);
> > > -	TAILQ_INIT(&filter_ethertype_list);
> > > -	TAILQ_INIT(&filter_syn_list);
> > > -	TAILQ_INIT(&filter_fdir_list);
> > > -	TAILQ_INIT(&filter_l2_tunnel_list);
> > > -	TAILQ_INIT(&ixgbe_flow_list);
> > > +	/* initialize flow filter lists */
> > > +	ixgbe_filterlist_init();
> >
> > Will it work if list initialization converted to the static
> > initialization? That would remove requirement of this function call.
> 
> I was trying to keep the behavior the same:
>   .probe -> eth_ixgbe_dev_init -> ixgbe_filterlist_init
>   .remove -> eth_ixgbe_dev_uninit -> ixgbe_filterlist_flush
> 
> While I think the below would work I always like mirror/parallel
> relationships like ctor/dtor for objects.
> 
> Is that reasonable?
> 
> Thanks,
> Dave
> 
> >
> > something like:
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_flow.c
> > b/drivers/net/ixgbe/ixgbe_flow.c index c8645f056..6184eb4af 100644
> > --- a/drivers/net/ixgbe/ixgbe_flow.c
> > +++ b/drivers/net/ixgbe/ixgbe_flow.c
> > @@ -79,6 +79,24 @@
> >  #define IXGBE_MAX_N_TUPLE_PRIO 7
> >  #define IXGBE_MAX_FLX_SOURCE_OFF 62
> >
> > +static TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele)
> > +       filter_ntuple_list =
> > +TAILQ_HEAD_INITIALIZER(filter_ntuple_list);
> > +
> > +static TAILQ_HEAD(ixgbe_ethertype_filter_list,
> > ixgbe_ethertype_filter_ele)
> > +       filter_ethertype_list =
> > TAILQ_HEAD_INITIALIZER(filter_ethertype_list);
> > +
> > +static TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele)
> > +       filter_syn_list = TAILQ_HEAD_INITIALIZER(filter_syn_list);
> > +
> > +static TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele)
> > +       filter_fdir_list = TAILQ_HEAD_INITIALIZER(filter_fdir_list);
> > +
> > +static TAILQ_HEAD(ixgbe_l2_tunnel_filter_list,
> > ixgbe_eth_l2_tunnel_conf_ele)
> > +       filter_l2_tunnel_list =
> > TAILQ_HEAD_INITIALIZER(filter_l2_tunnel_list);
> > +
> > +static TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem)
> > +       ixgbe_flow_list = TAILQ_HEAD_INITIALIZER(ixgbe_flow_list);
> > +

[dpdk-dev] [PATCH v2] ixgbe: eliminate duplicate filterlist symbols

[David C Harton]

From: David Harton <dharton@cisco.com>

Some compilers generate warnings for duplicate symbols for the
set of filter lists current defined in ixgbe_ethdev.h.
This commits moves the defintion and declaration to the source
file that actually uses them and provides a function to
initialize the values akin to its flush function.

Signed-off-by: David Harton <dharton@cisco.com>
---

v2
* Moved flow filter structs to ixgbe_flow.c.

v1
* Moved flow filter definitions and initialization to ixgbe_flow.c.

 drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++----
 drivers/net/ixgbe/ixgbe_ethdev.h | 44 +------------------------------
 drivers/net/ixgbe/ixgbe_flow.c   | 56 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 1ec5aaf..ed21af5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1332,12 +1332,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 	/* initialize l2 tunnel filter list & hash */
 	ixgbe_l2_tn_filter_init(eth_dev);
 
-	TAILQ_INIT(&filter_ntuple_list);
-	TAILQ_INIT(&filter_ethertype_list);
-	TAILQ_INIT(&filter_syn_list);
-	TAILQ_INIT(&filter_fdir_list);
-	TAILQ_INIT(&filter_l2_tunnel_list);
-	TAILQ_INIT(&ixgbe_flow_list);
+	/* initialize flow filter lists */
+	ixgbe_filterlist_init();
 
 	/* initialize bandwidth configuration info */
 	memset(bw_conf, 0, sizeof(struct ixgbe_bw_conf));
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index caa50c8..e28c856 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -364,49 +364,6 @@ struct rte_flow {
 	enum rte_filter_type filter_type;
 	void *rule;
 };
-/* ntuple filter list structure */
-struct ixgbe_ntuple_filter_ele {
-	TAILQ_ENTRY(ixgbe_ntuple_filter_ele) entries;
-	struct rte_eth_ntuple_filter filter_info;
-};
-/* ethertype filter list structure */
-struct ixgbe_ethertype_filter_ele {
-	TAILQ_ENTRY(ixgbe_ethertype_filter_ele) entries;
-	struct rte_eth_ethertype_filter filter_info;
-};
-/* syn filter list structure */
-struct ixgbe_eth_syn_filter_ele {
-	TAILQ_ENTRY(ixgbe_eth_syn_filter_ele) entries;
-	struct rte_eth_syn_filter filter_info;
-};
-/* fdir filter list structure */
-struct ixgbe_fdir_rule_ele {
-	TAILQ_ENTRY(ixgbe_fdir_rule_ele) entries;
-	struct ixgbe_fdir_rule filter_info;
-};
-/* l2_tunnel filter list structure */
-struct ixgbe_eth_l2_tunnel_conf_ele {
-	TAILQ_ENTRY(ixgbe_eth_l2_tunnel_conf_ele) entries;
-	struct rte_eth_l2_tunnel_conf filter_info;
-};
-/* ixgbe_flow memory list structure */
-struct ixgbe_flow_mem {
-	TAILQ_ENTRY(ixgbe_flow_mem) entries;
-	struct rte_flow *flow;
-};
-
-TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele);
-struct ixgbe_ntuple_filter_list filter_ntuple_list;
-TAILQ_HEAD(ixgbe_ethertype_filter_list, ixgbe_ethertype_filter_ele);
-struct ixgbe_ethertype_filter_list filter_ethertype_list;
-TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele);
-struct ixgbe_syn_filter_list filter_syn_list;
-TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele);
-struct ixgbe_fdir_rule_filter_list filter_fdir_list;
-TAILQ_HEAD(ixgbe_l2_tunnel_filter_list, ixgbe_eth_l2_tunnel_conf_ele);
-struct ixgbe_l2_tunnel_filter_list filter_l2_tunnel_list;
-TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem);
-struct ixgbe_flow_mem_list ixgbe_flow_list;
 
 /*
  * Statistics counters collected by the MACsec
@@ -692,6 +649,7 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
 int
 ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
 			       struct rte_eth_l2_tunnel_conf *l2_tunnel);
+void ixgbe_filterlist_init(void);
 void ixgbe_filterlist_flush(void);
 /*
  * Flow director function prototypes
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index d679608..6216c54 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -79,6 +79,51 @@
 #define IXGBE_MAX_N_TUPLE_PRIO 7
 #define IXGBE_MAX_FLX_SOURCE_OFF 62
 
+/* ntuple filter list structure */
+struct ixgbe_ntuple_filter_ele {
+	TAILQ_ENTRY(ixgbe_ntuple_filter_ele) entries;
+	struct rte_eth_ntuple_filter filter_info;
+};
+/* ethertype filter list structure */
+struct ixgbe_ethertype_filter_ele {
+	TAILQ_ENTRY(ixgbe_ethertype_filter_ele) entries;
+	struct rte_eth_ethertype_filter filter_info;
+};
+/* syn filter list structure */
+struct ixgbe_eth_syn_filter_ele {
+	TAILQ_ENTRY(ixgbe_eth_syn_filter_ele) entries;
+	struct rte_eth_syn_filter filter_info;
+};
+/* fdir filter list structure */
+struct ixgbe_fdir_rule_ele {
+	TAILQ_ENTRY(ixgbe_fdir_rule_ele) entries;
+	struct ixgbe_fdir_rule filter_info;
+};
+/* l2_tunnel filter list structure */
+struct ixgbe_eth_l2_tunnel_conf_ele {
+	TAILQ_ENTRY(ixgbe_eth_l2_tunnel_conf_ele) entries;
+	struct rte_eth_l2_tunnel_conf filter_info;
+};
+/* ixgbe_flow memory list structure */
+struct ixgbe_flow_mem {
+	TAILQ_ENTRY(ixgbe_flow_mem) entries;
+	struct rte_flow *flow;
+};
+
+TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele);
+TAILQ_HEAD(ixgbe_ethertype_filter_list, ixgbe_ethertype_filter_ele);
+TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele);
+TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele);
+TAILQ_HEAD(ixgbe_l2_tunnel_filter_list, ixgbe_eth_l2_tunnel_conf_ele);
+TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem);
+
+static struct ixgbe_ntuple_filter_list filter_ntuple_list;
+static struct ixgbe_ethertype_filter_list filter_ethertype_list;
+static struct ixgbe_syn_filter_list filter_syn_list;
+static struct ixgbe_fdir_rule_filter_list filter_fdir_list;
+static struct ixgbe_l2_tunnel_filter_list filter_l2_tunnel_list;
+static struct ixgbe_flow_mem_list ixgbe_flow_list;
+
 /**
  * Endless loop will never happen with below assumption
  * 1. there is at least one no-void item(END)
@@ -2600,6 +2645,17 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
 }
 
 void
+ixgbe_filterlist_init(void)
+{
+	TAILQ_INIT(&filter_ntuple_list);
+	TAILQ_INIT(&filter_ethertype_list);
+	TAILQ_INIT(&filter_syn_list);
+	TAILQ_INIT(&filter_fdir_list);
+	TAILQ_INIT(&filter_l2_tunnel_list);
+	TAILQ_INIT(&ixgbe_flow_list);
+}
+
+void
 ixgbe_filterlist_flush(void)
 {
 	struct ixgbe_ntuple_filter_ele *ntuple_filter_ptr;
-- 
2.10.3.dirty

[dpdk-dev] [PATCH v3] ixgbe: eliminate duplicate filterlist symbols

[David C Harton]

From: David Harton <dharton@cisco.com>

Some compilers generate warnings for duplicate symbols for the
set of filter lists current defined in ixgbe_ethdev.h.
This commits moves the definition and declaration to the source
file that actually uses them and provides a function to
initialize the values akin to its flush function.

Signed-off-by: David Harton <dharton@cisco.com>
---

v3
* Fixed spelling in commit message.  Not sure why running
  checkpatches.sh locally didn't catch it. :(

v2
* Moved flow filter structs to ixgbe_flow.c.

v1
* Moved flow filter definitions and initialization to ixgbe_flow.c.

 drivers/net/ixgbe/ixgbe_ethdev.c |  8 ++----
 drivers/net/ixgbe/ixgbe_ethdev.h | 44 +------------------------------
 drivers/net/ixgbe/ixgbe_flow.c   | 56 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 49 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 1ec5aaf..ed21af5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1332,12 +1332,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
 	/* initialize l2 tunnel filter list & hash */
 	ixgbe_l2_tn_filter_init(eth_dev);
 
-	TAILQ_INIT(&filter_ntuple_list);
-	TAILQ_INIT(&filter_ethertype_list);
-	TAILQ_INIT(&filter_syn_list);
-	TAILQ_INIT(&filter_fdir_list);
-	TAILQ_INIT(&filter_l2_tunnel_list);
-	TAILQ_INIT(&ixgbe_flow_list);
+	/* initialize flow filter lists */
+	ixgbe_filterlist_init();
 
 	/* initialize bandwidth configuration info */
 	memset(bw_conf, 0, sizeof(struct ixgbe_bw_conf));
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index caa50c8..e28c856 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -364,49 +364,6 @@ struct rte_flow {
 	enum rte_filter_type filter_type;
 	void *rule;
 };
-/* ntuple filter list structure */
-struct ixgbe_ntuple_filter_ele {
-	TAILQ_ENTRY(ixgbe_ntuple_filter_ele) entries;
-	struct rte_eth_ntuple_filter filter_info;
-};
-/* ethertype filter list structure */
-struct ixgbe_ethertype_filter_ele {
-	TAILQ_ENTRY(ixgbe_ethertype_filter_ele) entries;
-	struct rte_eth_ethertype_filter filter_info;
-};
-/* syn filter list structure */
-struct ixgbe_eth_syn_filter_ele {
-	TAILQ_ENTRY(ixgbe_eth_syn_filter_ele) entries;
-	struct rte_eth_syn_filter filter_info;
-};
-/* fdir filter list structure */
-struct ixgbe_fdir_rule_ele {
-	TAILQ_ENTRY(ixgbe_fdir_rule_ele) entries;
-	struct ixgbe_fdir_rule filter_info;
-};
-/* l2_tunnel filter list structure */
-struct ixgbe_eth_l2_tunnel_conf_ele {
-	TAILQ_ENTRY(ixgbe_eth_l2_tunnel_conf_ele) entries;
-	struct rte_eth_l2_tunnel_conf filter_info;
-};
-/* ixgbe_flow memory list structure */
-struct ixgbe_flow_mem {
-	TAILQ_ENTRY(ixgbe_flow_mem) entries;
-	struct rte_flow *flow;
-};
-
-TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele);
-struct ixgbe_ntuple_filter_list filter_ntuple_list;
-TAILQ_HEAD(ixgbe_ethertype_filter_list, ixgbe_ethertype_filter_ele);
-struct ixgbe_ethertype_filter_list filter_ethertype_list;
-TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele);
-struct ixgbe_syn_filter_list filter_syn_list;
-TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele);
-struct ixgbe_fdir_rule_filter_list filter_fdir_list;
-TAILQ_HEAD(ixgbe_l2_tunnel_filter_list, ixgbe_eth_l2_tunnel_conf_ele);
-struct ixgbe_l2_tunnel_filter_list filter_l2_tunnel_list;
-TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem);
-struct ixgbe_flow_mem_list ixgbe_flow_list;
 
 /*
  * Statistics counters collected by the MACsec
@@ -692,6 +649,7 @@ ixgbe_dev_l2_tunnel_filter_add(struct rte_eth_dev *dev,
 int
 ixgbe_dev_l2_tunnel_filter_del(struct rte_eth_dev *dev,
 			       struct rte_eth_l2_tunnel_conf *l2_tunnel);
+void ixgbe_filterlist_init(void);
 void ixgbe_filterlist_flush(void);
 /*
  * Flow director function prototypes
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index d679608..6216c54 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -79,6 +79,51 @@
 #define IXGBE_MAX_N_TUPLE_PRIO 7
 #define IXGBE_MAX_FLX_SOURCE_OFF 62
 
+/* ntuple filter list structure */
+struct ixgbe_ntuple_filter_ele {
+	TAILQ_ENTRY(ixgbe_ntuple_filter_ele) entries;
+	struct rte_eth_ntuple_filter filter_info;
+};
+/* ethertype filter list structure */
+struct ixgbe_ethertype_filter_ele {
+	TAILQ_ENTRY(ixgbe_ethertype_filter_ele) entries;
+	struct rte_eth_ethertype_filter filter_info;
+};
+/* syn filter list structure */
+struct ixgbe_eth_syn_filter_ele {
+	TAILQ_ENTRY(ixgbe_eth_syn_filter_ele) entries;
+	struct rte_eth_syn_filter filter_info;
+};
+/* fdir filter list structure */
+struct ixgbe_fdir_rule_ele {
+	TAILQ_ENTRY(ixgbe_fdir_rule_ele) entries;
+	struct ixgbe_fdir_rule filter_info;
+};
+/* l2_tunnel filter list structure */
+struct ixgbe_eth_l2_tunnel_conf_ele {
+	TAILQ_ENTRY(ixgbe_eth_l2_tunnel_conf_ele) entries;
+	struct rte_eth_l2_tunnel_conf filter_info;
+};
+/* ixgbe_flow memory list structure */
+struct ixgbe_flow_mem {
+	TAILQ_ENTRY(ixgbe_flow_mem) entries;
+	struct rte_flow *flow;
+};
+
+TAILQ_HEAD(ixgbe_ntuple_filter_list, ixgbe_ntuple_filter_ele);
+TAILQ_HEAD(ixgbe_ethertype_filter_list, ixgbe_ethertype_filter_ele);
+TAILQ_HEAD(ixgbe_syn_filter_list, ixgbe_eth_syn_filter_ele);
+TAILQ_HEAD(ixgbe_fdir_rule_filter_list, ixgbe_fdir_rule_ele);
+TAILQ_HEAD(ixgbe_l2_tunnel_filter_list, ixgbe_eth_l2_tunnel_conf_ele);
+TAILQ_HEAD(ixgbe_flow_mem_list, ixgbe_flow_mem);
+
+static struct ixgbe_ntuple_filter_list filter_ntuple_list;
+static struct ixgbe_ethertype_filter_list filter_ethertype_list;
+static struct ixgbe_syn_filter_list filter_syn_list;
+static struct ixgbe_fdir_rule_filter_list filter_fdir_list;
+static struct ixgbe_l2_tunnel_filter_list filter_l2_tunnel_list;
+static struct ixgbe_flow_mem_list ixgbe_flow_list;
+
 /**
  * Endless loop will never happen with below assumption
  * 1. there is at least one no-void item(END)
@@ -2600,6 +2645,17 @@ ixgbe_parse_fdir_filter(struct rte_eth_dev *dev,
 }
 
 void
+ixgbe_filterlist_init(void)
+{
+	TAILQ_INIT(&filter_ntuple_list);
+	TAILQ_INIT(&filter_ethertype_list);
+	TAILQ_INIT(&filter_syn_list);
+	TAILQ_INIT(&filter_fdir_list);
+	TAILQ_INIT(&filter_l2_tunnel_list);
+	TAILQ_INIT(&ixgbe_flow_list);
+}
+
+void
 ixgbe_filterlist_flush(void)
 {
 	struct ixgbe_ntuple_filter_ele *ntuple_filter_ptr;
-- 
2.10.3.dirty

Re: [dpdk-dev] [PATCH v3] ixgbe: eliminate duplicate filterlist symbols

[Ferruh Yigit]

On 9/14/2017 1:50 PM, David C Harton wrote:
> From: David Harton <dharton@cisco.com>
> 
> Some compilers generate warnings for duplicate symbols for the
> set of filter lists current defined in ixgbe_ethdev.h.
> This commits moves the definition and declaration to the source
> file that actually uses them and provides a function to
> initialize the values akin to its flush function.
> 
> Signed-off-by: David Harton <dharton@cisco.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

(title needs to be "net/ixgbe: ...", but this can be fixed while applying)

Re: [dpdk-dev] [PATCH v3] ixgbe: eliminate duplicate filterlist symbols

[Radu Nicolau]

On 9/14/2017 2:38 PM, Ferruh Yigit wrote:
> On 9/14/2017 1:50 PM, David C Harton wrote:
>> From: David Harton <dharton@cisco.com>
>>
>> Some compilers generate warnings for duplicate symbols for the
>> set of filter lists current defined in ixgbe_ethdev.h.
>> This commits moves the definition and declaration to the source
>> file that actually uses them and provides a function to
>> initialize the values akin to its flush function.
>>
>> Signed-off-by: David Harton <dharton@cisco.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
> (title needs to be "net/ixgbe: ...", but this can be fixed while applying)
>
Reviewed-by:  Radu Nicolau <radu.nicolau@intel.com>

Re: [dpdk-dev] [PATCH v3] ixgbe: eliminate duplicate filterlist symbols

[Ferruh Yigit]

On 9/18/2017 2:13 PM, Radu Nicolau wrote:
> 
> On 9/14/2017 2:38 PM, Ferruh Yigit wrote:
>> On 9/14/2017 1:50 PM, David C Harton wrote:
>>> From: David Harton <dharton@cisco.com>
>>>
>>> Some compilers generate warnings for duplicate symbols for the
>>> set of filter lists current defined in ixgbe_ethdev.h.
>>> This commits moves the definition and declaration to the source
>>> file that actually uses them and provides a function to
>>> initialize the values akin to its flush function.
>>>
>>> Signed-off-by: David Harton <dharton@cisco.com>
>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> (title needs to be "net/ixgbe: ...", but this can be fixed while applying)
>>
> Reviewed-by:  Radu Nicolau <radu.nicolau@intel.com>

Applied to dpdk-next-net/master, thanks.