From: "Zhao1, Wei" <wei.zhao1@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Subject: Re: [dpdk-dev] [PATCH 02/18] net/ixgbe: store flow director filter
Date: Mon, 26 Dec 2016 02:50:31 +0000 [thread overview]
Message-ID: <A2573D2ACFCADC41BB3BE09C6DE313CA02018DB2@PGSMSX103.gar.corp.intel.com> (raw)
In-Reply-To: <71288c3d-2abb-d99d-45a4-f26d399194c2@intel.com>
Hi, Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 21, 2016 12:58 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 02/18] net/ixgbe: store flow director filter
>
> On 12/2/2016 10:42 AM, Wei Zhao wrote:
> > From: wei zhao1 <wei.zhao1@intel.com>
> >
> > Add support for storing flow director filter in SW.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > Signed-off-by: wei zhao1 <wei.zhao1@intel.com>
> > ---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 48 ++++++++++++++++++
> > drivers/net/ixgbe/ixgbe_ethdev.h | 19 ++++++-
> > drivers/net/ixgbe/ixgbe_fdir.c | 105
> ++++++++++++++++++++++++++++++++++++++-
> > 3 files changed, 169 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 7f10cca..f8e5fe1 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> <...>
> > @@ -1289,6 +1302,25 @@ eth_ixgbe_dev_init(struct rte_eth_dev
> *eth_dev)
> >
> > /* initialize SYN filter */
> > filter_info->syn_info = 0;
> > + /* initialize flow director filter list & hash */
> > + TAILQ_INIT(&fdir_info->fdir_list);
> > + snprintf(fdir_hash_name, RTE_HASH_NAMESIZE,
> > + "fdir_%s", eth_dev->data->name);
> > + fdir_info->hash_handle = rte_hash_create(&fdir_hash_params);
>
> Do we really create a hash table in device init? Is there a way to do this if we
> know user will use it?
By now, we have no idea about whether user wiil use flow director or not.
So, our strategy is init all types of filter and give option to users.
>
> > + if (!fdir_info->hash_handle) {
> > + PMD_INIT_LOG(ERR, "Failed to create fdir hash table!");
> > + return -EINVAL;
>
> And should we exit here? What if user will not use flow director at all?
Maybe, we can delete "return -EINVAL;" and only print error log as a warning.
But some uer maybe do not pay attention to warning error log sometimes, so we chose to
exit if any init fail.
>
> > + }
> > + fdir_info->hash_map = rte_zmalloc("ixgbe",
> > + sizeof(struct ixgbe_fdir_filter *) *
> > + IXGBE_MAX_FDIR_FILTER_NUM,
> > + 0);
> > + if (!fdir_info->hash_map) {
> > + PMD_INIT_LOG(ERR,
> > + "Failed to allocate memory for fdir hash map!");
> > + return -ENOMEM;
> > + }
> > +
>
> Can you please extract these into functions, to have more clear init/uninit
> functions?
ok, that seems like a good idea to creat two new functions to do initialize of flow director and l2 tunnel fliter.
That will make dev init function more clear. I wiil add two init function for this purpose in v2.
>
> > return 0;
> > }
> >
> > @@ -1297,6 +1329,9 @@ eth_ixgbe_dev_uninit(struct rte_eth_dev
> > *eth_dev) {
> > struct rte_pci_device *pci_dev;
> > struct ixgbe_hw *hw;
> > + struct ixgbe_hw_fdir_info *fdir_info =
> > + IXGBE_DEV_PRIVATE_TO_FDIR_INFO(eth_dev->data-
> >dev_private);
> > + struct ixgbe_fdir_filter *fdir_filter;
> >
> > PMD_INIT_FUNC_TRACE();
> >
> > @@ -1330,6 +1365,19 @@ eth_ixgbe_dev_uninit(struct rte_eth_dev
> *eth_dev)
> > rte_free(eth_dev->data->hash_mac_addrs);
> > eth_dev->data->hash_mac_addrs = NULL;
> >
> > + /* remove all the fdir filters & hash */
> > + if (fdir_info->hash_map)
> > + rte_free(fdir_info->hash_map);
> > + if (fdir_info->hash_handle)
> > + rte_hash_free(fdir_info->hash_handle);
>
> All rte_hash_xxx() APIs gives build error for shared library build, [1] needs to
> be added into Makefile.
>
> But, this makes hash library a dependency to the PMD, do you think is there
> a way to escape from this?
>
> [1]
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_net
> +lib/librte_hash
>
Ok, I will change as your suggestion in v2.
> > +
> > + while ((fdir_filter = TAILQ_FIRST(&fdir_info->fdir_list))) {
> > + TAILQ_REMOVE(&fdir_info->fdir_list,
> > + fdir_filter,
> > + entries);
> > + rte_free(fdir_filter);
> > + }
> > +
> > return 0;
> > }
> >
> <...>
next prev parent reply other threads:[~2016-12-26 2:50 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-02 10:42 [dpdk-dev] [PATCH 00/18] net/ixgbe: Consistent filter API Wei Zhao
2016-12-02 10:42 ` [dpdk-dev] [PATCH 01/18] net/ixgbe: store SYN filter Wei Zhao
2016-12-20 16:55 ` Ferruh Yigit
2016-12-22 9:48 ` Zhao1, Wei
2017-01-06 16:26 ` Ferruh Yigit
2017-01-10 6:48 ` Zhao1, Wei
2016-12-26 1:47 ` Zhao1, Wei
2016-12-02 10:42 ` [dpdk-dev] [PATCH 02/18] net/ixgbe: store flow director filter Wei Zhao
2016-12-20 16:58 ` Ferruh Yigit
2016-12-26 2:50 ` Zhao1, Wei [this message]
2016-12-02 10:42 ` [dpdk-dev] [PATCH 03/18] net/ixgbe: store L2 tunnel filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 04/18] net/ixgbe: restore n-tuple filter Wei Zhao
2016-12-20 16:58 ` Ferruh Yigit
2016-12-26 3:32 ` Zhao1, Wei
2016-12-02 10:43 ` [dpdk-dev] [PATCH 05/18] net/ixgbe: restore ether type filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 06/18] net/ixgbe: restore SYN filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 07/18] net/ixgbe: restore flow director filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 08/18] net/ixgbe: restore L2 tunnel filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 09/18] net/ixgbe: store and restore L2 tunnel configuration Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 10/18] net/ixgbe: flush all the filters Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 11/18] net/ixgbe: parse n-tuple filter Wei Zhao
2016-12-20 17:23 ` Ferruh Yigit
2016-12-02 10:43 ` [dpdk-dev] [PATCH 12/18] net/ixgbe: parse ethertype filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 13/18] net/ixgbe: parse SYN filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 14/18] net/ixgbe: parse L2 tunnel filter Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 15/18] net/ixgbe: parse flow director filter Wei Zhao
2016-12-20 17:00 ` Ferruh Yigit
2016-12-22 9:19 ` Zhao1, Wei
2016-12-22 10:44 ` Ferruh Yigit
2016-12-23 8:13 ` Adrien Mazarguil
2016-12-27 3:31 ` Zhao1, Wei
2016-12-02 10:43 ` [dpdk-dev] [PATCH 16/18] net/ixgbe: create consistent filter Wei Zhao
2016-12-20 17:25 ` Ferruh Yigit
2016-12-23 6:26 ` Zhao1, Wei
2016-12-02 10:43 ` [dpdk-dev] [PATCH 17/18] net/ixgbe: destroy " Wei Zhao
2016-12-02 10:43 ` [dpdk-dev] [PATCH 18/18] net/ixgbe: flush " Wei Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=A2573D2ACFCADC41BB3BE09C6DE313CA02018DB2@PGSMSX103.gar.corp.intel.com \
--to=wei.zhao1@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=wenzhuo.lu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).