[dpdk-dev,v2,03/17] net/i40e: store flow director filter

Message ID 1482819984-14120-4-git-send-email-beilei.xing@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation fail Compilation issues

Commit Message

Xing, Beilei Dec. 27, 2016, 6:26 a.m. UTC
  Currently there's no flow director filter stored in SW. This
patch stores flow director filters in SW with cuckoo hash,
also adds protection if a flow director filter has been added.

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 48 +++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h | 12 ++++++
 drivers/net/i40e/i40e_fdir.c   | 98 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+)
  

Comments

Tiwei Bie Dec. 28, 2016, 3:38 a.m. UTC | #1
On Tue, Dec 27, 2016 at 02:26:10PM +0800, Beilei Xing wrote:
> Currently there's no flow director filter stored in SW. This
> patch stores flow director filters in SW with cuckoo hash,
> also adds protection if a flow director filter has been added.
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 48 +++++++++++++++++++++
>  drivers/net/i40e/i40e_ethdev.h | 12 ++++++
>  drivers/net/i40e/i40e_fdir.c   | 98 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 158 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index c012d5d..427ebdc 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
[...]
> @@ -1342,6 +1379,17 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
>  		rte_free(p_tunnel);
>  	}
>  
> +	/* Remove all flow director rules and hash */
> +	if (fdir_info->hash_map)
> +		rte_free(fdir_info->hash_map);
> +	if (fdir_info->hash_table)
> +		rte_hash_free(fdir_info->hash_table);
> +
> +	while ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {

There is a redundant pair of parentheses, or you should compare with
NULL.

> +		TAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules);
> +		rte_free(p_fdir);
> +	}
> +
>  	dev->dev_ops = NULL;
>  	dev->rx_pkt_burst = NULL;
>  	dev->tx_pkt_burst = NULL;
[...]
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 335bf15..faa2495 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
[...]
> +/* Check if there exists the flow director filter */
> +static struct i40e_fdir_filter *
> +i40e_sw_fdir_filter_lookup(struct i40e_fdir_info *fdir_info,
> +			const struct rte_eth_fdir_input *input)
> +{
> +	int ret = 0;
> +

The initialization is meaningless, as it will be written by the below
assignment unconditionally.

> +	ret = rte_hash_lookup(fdir_info->hash_table, (const void *)input);
> +	if (ret < 0)
> +		return NULL;
> +
> +	return fdir_info->hash_map[ret];
> +}
> +
> +/* Add a flow director filter into the SW list */
> +static int
> +i40e_sw_fdir_filter_insert(struct i40e_pf *pf, struct i40e_fdir_filter *filter)
> +{
> +	struct i40e_fdir_info *fdir_info = &pf->fdir;
> +	int ret = 0;
> +

Same here.

> +	ret = rte_hash_add_key(fdir_info->hash_table,
> +			       &filter->fdir.input);
> +	if (ret < 0)
> +		PMD_DRV_LOG(ERR,
> +			    "Failed to insert fdir filter to hash table %d!",
> +			    ret);

Function should return when ret < 0.

> +	fdir_info->hash_map[ret] = filter;
> +
> +	TAILQ_INSERT_TAIL(&fdir_info->fdir_list, filter, rules);
> +
> +	return 0;
> +}
> +
> +/* Delete a flow director filter from the SW list */
> +static int
> +i40e_sw_fdir_filter_del(struct i40e_pf *pf, struct i40e_fdir_filter *filter)
> +{
> +	struct i40e_fdir_info *fdir_info = &pf->fdir;
> +	int ret = 0;
> +

Same here.

> +	ret = rte_hash_del_key(fdir_info->hash_table,
> +			       &filter->fdir.input);
> +	if (ret < 0)
> +		PMD_DRV_LOG(ERR,
> +			    "Failed to delete fdir filter to hash table %d!",
> +			    ret);

Function should return when ret < 0.

> +	fdir_info->hash_map[ret] = NULL;
> +
> +	TAILQ_REMOVE(&fdir_info->fdir_list, filter, rules);
> +	rte_free(filter);
> +
> +	return 0;
> +}
> +
>  /*
>   * i40e_add_del_fdir_filter - add or remove a flow director filter.
>   * @pf: board private structure
> @@ -1032,6 +1105,8 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	unsigned char *pkt = (unsigned char *)pf->fdir.prg_pkt;
>  	enum i40e_filter_pctype pctype;
> +	struct i40e_fdir_info *fdir_info = &pf->fdir;
> +	struct i40e_fdir_filter *fdir_filter, *node;
>  	int ret = 0;
>  
>  	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT) {
> @@ -1054,6 +1129,21 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
>  		return -EINVAL;
>  	}
>  
> +	fdir_filter = rte_zmalloc("fdir_filter", sizeof(*fdir_filter), 0);
> +	i40e_fdir_filter_convert(filter, fdir_filter);
> +	node = i40e_sw_fdir_filter_lookup(fdir_info, &fdir_filter->fdir.input);
> +	if (add && node) {
> +		PMD_DRV_LOG(ERR,
> +			    "Conflict with existing flow director rules!");
> +		rte_free(fdir_filter);
> +		return -EINVAL;
> +	} else if (!add && !node) {

When `if (add && node)' is true, function will return. There is no need
to use `else' here.

Best regards,
Tiwei Bie
  
Xing, Beilei Dec. 28, 2016, 7:10 a.m. UTC | #2
> -----Original Message-----

> From: Bie, Tiwei

> Sent: Wednesday, December 28, 2016 11:39 AM

> To: Xing, Beilei <beilei.xing@intel.com>

> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin

> <helin.zhang@intel.com>; dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v2 03/17] net/i40e: store flow director filter

> 

> On Tue, Dec 27, 2016 at 02:26:10PM +0800, Beilei Xing wrote:

> > Currently there's no flow director filter stored in SW. This patch

> > stores flow director filters in SW with cuckoo hash, also adds

> > protection if a flow director filter has been added.

> >

> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>

> > ---

> >  drivers/net/i40e/i40e_ethdev.c | 48 +++++++++++++++++++++

> > drivers/net/i40e/i40e_ethdev.h | 12 ++++++

> >  drivers/net/i40e/i40e_fdir.c   | 98

> ++++++++++++++++++++++++++++++++++++++++++

> >  3 files changed, 158 insertions(+)

> >

> > diff --git a/drivers/net/i40e/i40e_ethdev.c

> > b/drivers/net/i40e/i40e_ethdev.c index c012d5d..427ebdc 100644

> > --- a/drivers/net/i40e/i40e_ethdev.c

> > +++ b/drivers/net/i40e/i40e_ethdev.c

> [...]

> > @@ -1342,6 +1379,17 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)

> >  		rte_free(p_tunnel);

> >  	}

> >

> > +	/* Remove all flow director rules and hash */

> > +	if (fdir_info->hash_map)

> > +		rte_free(fdir_info->hash_map);

> > +	if (fdir_info->hash_table)

> > +		rte_hash_free(fdir_info->hash_table);

> > +

> > +	while ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {

> 

> There is a redundant pair of parentheses, or you should compare with NULL.


I think the another parentheses is used to compare with NULL. In fact there's similar using in PMD.

> 

> > +		TAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules);

> > +		rte_free(p_fdir);

> > +	}

> > +

> >  	dev->dev_ops = NULL;

> >  	dev->rx_pkt_burst = NULL;

> >  	dev->tx_pkt_burst = NULL;

> [...]

> > diff --git a/drivers/net/i40e/i40e_fdir.c

> > b/drivers/net/i40e/i40e_fdir.c index 335bf15..faa2495 100644

> > --- a/drivers/net/i40e/i40e_fdir.c

> > +++ b/drivers/net/i40e/i40e_fdir.c

> [...]

> > +/* Check if there exists the flow director filter */ static struct

> > +i40e_fdir_filter * i40e_sw_fdir_filter_lookup(struct i40e_fdir_info

> > +*fdir_info,

> > +			const struct rte_eth_fdir_input *input) {

> > +	int ret = 0;

> > +

> 

> The initialization is meaningless, as it will be written by the below

> assignment unconditionally.


Yes, you're right.

> 

> > +	ret = rte_hash_lookup(fdir_info->hash_table, (const void *)input);

> > +	if (ret < 0)

> > +		return NULL;

> > +

> > +	return fdir_info->hash_map[ret];

> > +}

> > +

> > +/* Add a flow director filter into the SW list */

> > +static int

> > +i40e_sw_fdir_filter_insert(struct i40e_pf *pf, struct i40e_fdir_filter *filter)

> > +{

> > +	struct i40e_fdir_info *fdir_info = &pf->fdir;

> > +	int ret = 0;

> > +

> 

> Same here.

> 

> > +	ret = rte_hash_add_key(fdir_info->hash_table,

> > +			       &filter->fdir.input);

> > +	if (ret < 0)

> > +		PMD_DRV_LOG(ERR,

> > +			    "Failed to insert fdir filter to hash table %d!",

> > +			    ret);

> 

> Function should return when ret < 0.


Thanks for catching it.

> 

> > +	fdir_info->hash_map[ret] = filter;

> > +

> > +	TAILQ_INSERT_TAIL(&fdir_info->fdir_list, filter, rules);

> > +

> > +	return 0;

> > +}

> > +

> > +/* Delete a flow director filter from the SW list */

> > +static int

> > +i40e_sw_fdir_filter_del(struct i40e_pf *pf, struct i40e_fdir_filter *filter)

> > +{

> > +	struct i40e_fdir_info *fdir_info = &pf->fdir;

> > +	int ret = 0;

> > +

> 

> Same here.

> 

> > +	ret = rte_hash_del_key(fdir_info->hash_table,

> > +			       &filter->fdir.input);

> > +	if (ret < 0)

> > +		PMD_DRV_LOG(ERR,

> > +			    "Failed to delete fdir filter to hash table %d!",

> > +			    ret);

> 

> Function should return when ret < 0.

> 

> > +	fdir_info->hash_map[ret] = NULL;

> > +

> > +	TAILQ_REMOVE(&fdir_info->fdir_list, filter, rules);

> > +	rte_free(filter);

> > +

> > +	return 0;

> > +}

> > +

> >  /*

> >   * i40e_add_del_fdir_filter - add or remove a flow director filter.

> >   * @pf: board private structure

> > @@ -1032,6 +1105,8 @@ i40e_add_del_fdir_filter(struct rte_eth_dev *dev,

> >  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-

> >dev_private);

> >  	unsigned char *pkt = (unsigned char *)pf->fdir.prg_pkt;

> >  	enum i40e_filter_pctype pctype;

> > +	struct i40e_fdir_info *fdir_info = &pf->fdir;

> > +	struct i40e_fdir_filter *fdir_filter, *node;

> >  	int ret = 0;

> >

> >  	if (dev->data->dev_conf.fdir_conf.mode !=

> RTE_FDIR_MODE_PERFECT) {

> > @@ -1054,6 +1129,21 @@ i40e_add_del_fdir_filter(struct rte_eth_dev

> *dev,

> >  		return -EINVAL;

> >  	}

> >

> > +	fdir_filter = rte_zmalloc("fdir_filter", sizeof(*fdir_filter), 0);

> > +	i40e_fdir_filter_convert(filter, fdir_filter);

> > +	node = i40e_sw_fdir_filter_lookup(fdir_info, &fdir_filter->fdir.input);

> > +	if (add && node) {

> > +		PMD_DRV_LOG(ERR,

> > +			    "Conflict with existing flow director rules!");

> > +		rte_free(fdir_filter);

> > +		return -EINVAL;

> > +	} else if (!add && !node) {

> 

> When `if (add && node)' is true, function will return. There is no need

> to use `else' here.

> 

> Best regards,

> Tiwei Bie
  
Tiwei Bie Dec. 28, 2016, 7:14 a.m. UTC | #3
On Wed, Dec 28, 2016 at 03:10:39PM +0800, Xing, Beilei wrote:
> 
> 
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Wednesday, December 28, 2016 11:39 AM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 03/17] net/i40e: store flow director filter
> > 
> > On Tue, Dec 27, 2016 at 02:26:10PM +0800, Beilei Xing wrote:
> > > Currently there's no flow director filter stored in SW. This patch
> > > stores flow director filters in SW with cuckoo hash, also adds
> > > protection if a flow director filter has been added.
> > >
> > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c | 48 +++++++++++++++++++++
> > > drivers/net/i40e/i40e_ethdev.h | 12 ++++++
> > >  drivers/net/i40e/i40e_fdir.c   | 98
> > ++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 158 insertions(+)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index c012d5d..427ebdc 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > [...]
> > > @@ -1342,6 +1379,17 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
> > >  		rte_free(p_tunnel);
> > >  	}
> > >
> > > +	/* Remove all flow director rules and hash */
> > > +	if (fdir_info->hash_map)
> > > +		rte_free(fdir_info->hash_map);
> > > +	if (fdir_info->hash_table)
> > > +		rte_hash_free(fdir_info->hash_table);
> > > +
> > > +	while ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {
> > 
> > There is a redundant pair of parentheses, or you should compare with NULL.
> 
> I think the another parentheses is used to compare with NULL. In fact there's similar using in PMD.
> 

The outer parentheses are redundant unless you compare it with NULL explicitly.
Any way, you could just follow the existing coding style.

Best regards,
Tiwei Bie
  
Tiwei Bie Dec. 28, 2016, 7:36 a.m. UTC | #4
On Wed, Dec 28, 2016 at 03:14:55PM +0800, Tiwei Bie wrote:
> On Wed, Dec 28, 2016 at 03:10:39PM +0800, Xing, Beilei wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Bie, Tiwei
> > > Sent: Wednesday, December 28, 2016 11:39 AM
> > > To: Xing, Beilei <beilei.xing@intel.com>
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin
> > > <helin.zhang@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 03/17] net/i40e: store flow director filter
> > > 
> > > On Tue, Dec 27, 2016 at 02:26:10PM +0800, Beilei Xing wrote:
> > > > Currently there's no flow director filter stored in SW. This patch
> > > > stores flow director filters in SW with cuckoo hash, also adds
> > > > protection if a flow director filter has been added.
> > > >
> > > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > > ---
> > > >  drivers/net/i40e/i40e_ethdev.c | 48 +++++++++++++++++++++
> > > > drivers/net/i40e/i40e_ethdev.h | 12 ++++++
> > > >  drivers/net/i40e/i40e_fdir.c   | 98
> > > ++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 158 insertions(+)
> > > >
> > > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > > b/drivers/net/i40e/i40e_ethdev.c index c012d5d..427ebdc 100644
> > > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > [...]
> > > > @@ -1342,6 +1379,17 @@ eth_i40e_dev_uninit(struct rte_eth_dev *dev)
> > > >  		rte_free(p_tunnel);
> > > >  	}
> > > >
> > > > +	/* Remove all flow director rules and hash */
> > > > +	if (fdir_info->hash_map)
> > > > +		rte_free(fdir_info->hash_map);
> > > > +	if (fdir_info->hash_table)
> > > > +		rte_hash_free(fdir_info->hash_table);
> > > > +
> > > > +	while ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {
> > > 
> > > There is a redundant pair of parentheses, or you should compare with NULL.
> > 
> > I think the another parentheses is used to compare with NULL. In fact there's similar using in PMD.
> > 
> 
> The outer parentheses are redundant unless you compare it with NULL explicitly.
> Any way, you could just follow the existing coding style.
> 

Sorry, I was wrong here. I just did a quick check and noticed that DPDK
has enabled the below option:

-Werror=parentheses

The outer parentheses are NOT redundant even if you don't compare it with
NULL explicitly.

Best regards,
Tiwei Bie
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index c012d5d..427ebdc 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -962,6 +962,7 @@  eth_i40e_dev_init(struct rte_eth_dev *dev)
 	uint8_t aq_fail = 0;
 	struct i40e_ethertype_rule *ethertype_rule = &pf->ethertype;
 	struct i40e_tunnel_rule *tunnel_rule = &pf->tunnel;
+	struct i40e_fdir_info *fdir_info = &pf->fdir;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -981,6 +982,14 @@  eth_i40e_dev_init(struct rte_eth_dev *dev)
 		.hash_func = rte_hash_crc,
 	};
 
+	char fdir_hash_name[RTE_HASH_NAMESIZE];
+	struct rte_hash_parameters fdir_hash_params = {
+		.name = fdir_hash_name,
+		.entries = I40E_MAX_FDIR_FILTER_NUM,
+		.key_len = sizeof(struct rte_eth_fdir_input),
+		.hash_func = rte_hash_crc,
+	};
+
 	dev->dev_ops = &i40e_eth_dev_ops;
 	dev->rx_pkt_burst = i40e_recv_pkts;
 	dev->tx_pkt_burst = i40e_xmit_pkts;
@@ -1262,8 +1271,33 @@  eth_i40e_dev_init(struct rte_eth_dev *dev)
 		goto err_tunnel_hash_map_alloc;
 	}
 
+	/* Initialize flow director filter rule list and hash */
+	TAILQ_INIT(&fdir_info->fdir_list);
+	snprintf(fdir_hash_name, RTE_HASH_NAMESIZE,
+		 "fdir_%s", dev->data->name);
+	fdir_info->hash_table = rte_hash_create(&fdir_hash_params);
+	if (!fdir_info->hash_table) {
+		PMD_INIT_LOG(ERR, "Failed to create fdir hash table!");
+		ret = -EINVAL;
+		goto err_fdir_hash_table_create;
+	}
+	fdir_info->hash_map = rte_zmalloc("i40e_fdir_hash_map",
+					  sizeof(struct i40e_fdir_filter *) *
+					  I40E_MAX_FDIR_FILTER_NUM,
+					  0);
+	if (!fdir_info->hash_map) {
+		PMD_INIT_LOG(ERR,
+			     "Failed to allocate memory for fdir hash map!");
+		ret = -ENOMEM;
+		goto err_fdir_hash_map_alloc;
+	}
+
 	return 0;
 
+err_fdir_hash_map_alloc:
+	rte_hash_free(fdir_info->hash_table);
+err_fdir_hash_table_create:
+	rte_free(tunnel_rule->hash_map);
 err_tunnel_hash_map_alloc:
 	rte_hash_free(tunnel_rule->hash_table);
 err_tunnel_hash_table_create:
@@ -1300,10 +1334,12 @@  eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 	struct i40e_filter_control_settings settings;
 	struct i40e_ethertype_filter *p_ethertype;
 	struct i40e_tunnel_filter *p_tunnel;
+	struct i40e_fdir_filter *p_fdir;
 	int ret;
 	uint8_t aq_fail = 0;
 	struct i40e_ethertype_rule *ethertype_rule;
 	struct i40e_tunnel_rule *tunnel_rule;
+	struct i40e_fdir_info *fdir_info;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1315,6 +1351,7 @@  eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 	pci_dev = dev->pci_dev;
 	ethertype_rule = &pf->ethertype;
 	tunnel_rule = &pf->tunnel;
+	fdir_info = &pf->fdir;
 
 	if (hw->adapter_stopped == 0)
 		i40e_dev_close(dev);
@@ -1342,6 +1379,17 @@  eth_i40e_dev_uninit(struct rte_eth_dev *dev)
 		rte_free(p_tunnel);
 	}
 
+	/* Remove all flow director rules and hash */
+	if (fdir_info->hash_map)
+		rte_free(fdir_info->hash_map);
+	if (fdir_info->hash_table)
+		rte_hash_free(fdir_info->hash_table);
+
+	while ((p_fdir = TAILQ_FIRST(&fdir_info->fdir_list))) {
+		TAILQ_REMOVE(&fdir_info->fdir_list, p_fdir, rules);
+		rte_free(p_fdir);
+	}
+
 	dev->dev_ops = NULL;
 	dev->rx_pkt_burst = NULL;
 	dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index c05436c..71756ae 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -377,6 +377,14 @@  struct i40e_fdir_flex_mask {
 };
 
 #define I40E_FILTER_PCTYPE_MAX 64
+#define I40E_MAX_FDIR_FILTER_NUM (1024 * 8)
+
+struct i40e_fdir_filter {
+	TAILQ_ENTRY(i40e_fdir_filter) rules;
+	struct rte_eth_fdir_filter fdir;
+};
+
+TAILQ_HEAD(i40e_fdir_filter_list, i40e_fdir_filter);
 /*
  *  A structure used to define fields of a FDIR related info.
  */
@@ -395,6 +403,10 @@  struct i40e_fdir_info {
 	 */
 	struct i40e_fdir_flex_pit flex_set[I40E_MAX_FLXPLD_LAYER * I40E_MAX_FLXPLD_FIED];
 	struct i40e_fdir_flex_mask flex_mask[I40E_FILTER_PCTYPE_MAX];
+
+	struct i40e_fdir_filter_list fdir_list;
+	struct i40e_fdir_filter **hash_map;
+	struct rte_hash *hash_table;
 };
 
 /* Ethertype filter number HW supports */
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 335bf15..faa2495 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -121,6 +121,16 @@  static int i40e_fdir_filter_programming(struct i40e_pf *pf,
 			bool add);
 static int i40e_fdir_flush(struct rte_eth_dev *dev);
 
+static int i40e_fdir_filter_convert(const struct rte_eth_fdir_filter *input,
+			 struct i40e_fdir_filter *filter);
+static struct i40e_fdir_filter *
+i40e_sw_fdir_filter_lookup(struct i40e_fdir_info *fdir_info,
+			const struct rte_eth_fdir_input *input);
+static int i40e_sw_fdir_filter_insert(struct i40e_pf *pf,
+				   struct i40e_fdir_filter *filter);
+static int i40e_sw_fdir_filter_del(struct i40e_pf *pf,
+				struct i40e_fdir_filter *filter);
+
 static int
 i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq)
 {
@@ -1017,6 +1027,69 @@  i40e_check_fdir_programming_status(struct i40e_rx_queue *rxq)
 	return ret;
 }
 
+static int
+i40e_fdir_filter_convert(const struct rte_eth_fdir_filter *input,
+			 struct i40e_fdir_filter *filter)
+{
+	rte_memcpy(&filter->fdir, input, sizeof(struct rte_eth_fdir_filter));
+	return 0;
+}
+
+/* Check if there exists the flow director filter */
+static struct i40e_fdir_filter *
+i40e_sw_fdir_filter_lookup(struct i40e_fdir_info *fdir_info,
+			const struct rte_eth_fdir_input *input)
+{
+	int ret = 0;
+
+	ret = rte_hash_lookup(fdir_info->hash_table, (const void *)input);
+	if (ret < 0)
+		return NULL;
+
+	return fdir_info->hash_map[ret];
+}
+
+/* Add a flow director filter into the SW list */
+static int
+i40e_sw_fdir_filter_insert(struct i40e_pf *pf, struct i40e_fdir_filter *filter)
+{
+	struct i40e_fdir_info *fdir_info = &pf->fdir;
+	int ret = 0;
+
+	ret = rte_hash_add_key(fdir_info->hash_table,
+			       &filter->fdir.input);
+	if (ret < 0)
+		PMD_DRV_LOG(ERR,
+			    "Failed to insert fdir filter to hash table %d!",
+			    ret);
+	fdir_info->hash_map[ret] = filter;
+
+	TAILQ_INSERT_TAIL(&fdir_info->fdir_list, filter, rules);
+
+	return 0;
+}
+
+/* Delete a flow director filter from the SW list */
+static int
+i40e_sw_fdir_filter_del(struct i40e_pf *pf, struct i40e_fdir_filter *filter)
+{
+	struct i40e_fdir_info *fdir_info = &pf->fdir;
+	int ret = 0;
+
+	ret = rte_hash_del_key(fdir_info->hash_table,
+			       &filter->fdir.input);
+	if (ret < 0)
+		PMD_DRV_LOG(ERR,
+			    "Failed to delete fdir filter to hash table %d!",
+			    ret);
+	fdir_info->hash_map[ret] = NULL;
+
+	TAILQ_REMOVE(&fdir_info->fdir_list, filter, rules);
+	rte_free(filter);
+
+	return 0;
+}
+
 /*
  * i40e_add_del_fdir_filter - add or remove a flow director filter.
  * @pf: board private structure
@@ -1032,6 +1105,8 @@  i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	unsigned char *pkt = (unsigned char *)pf->fdir.prg_pkt;
 	enum i40e_filter_pctype pctype;
+	struct i40e_fdir_info *fdir_info = &pf->fdir;
+	struct i40e_fdir_filter *fdir_filter, *node;
 	int ret = 0;
 
 	if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_PERFECT) {
@@ -1054,6 +1129,21 @@  i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	fdir_filter = rte_zmalloc("fdir_filter", sizeof(*fdir_filter), 0);
+	i40e_fdir_filter_convert(filter, fdir_filter);
+	node = i40e_sw_fdir_filter_lookup(fdir_info, &fdir_filter->fdir.input);
+	if (add && node) {
+		PMD_DRV_LOG(ERR,
+			    "Conflict with existing flow director rules!");
+		rte_free(fdir_filter);
+		return -EINVAL;
+	} else if (!add && !node) {
+		PMD_DRV_LOG(ERR,
+			    "There's no corresponding flow firector filter!");
+		rte_free(fdir_filter);
+		return -EINVAL;
+	}
+
 	memset(pkt, 0, I40E_FDIR_PKT_LEN);
 
 	ret = i40e_fdir_construct_pkt(pf, &filter->input, pkt);
@@ -1077,6 +1167,14 @@  i40e_add_del_fdir_filter(struct rte_eth_dev *dev,
 			    pctype);
 		return ret;
 	}
+
+	if (add) {
+		ret = i40e_sw_fdir_filter_insert(pf, fdir_filter);
+	} else {
+		ret = i40e_sw_fdir_filter_del(pf, node);
+		rte_free(fdir_filter);
+	}
+
 	return ret;
 }