Hi, yigit

-----Original Message-----
From: Yigit, Ferruh
Sent: Saturday, January 7, 2017 12:26 AM
To: Zhao1, Wei <redacted>; dev@dpdk.org
Cc: Lu, Wenzhuo <redacted>
Subject: Re: [dpdk-dev] [PATCH 01/18] net/ixgbe: store SYN filter

On 12/22/2016 9:48 AM, Zhao1, Wei wrote:

quoted

Hi, Yigit

quoted

-----Original Message-----
From: Yigit, Ferruh
Sent: Wednesday, December 21, 2016 12:56 AM
To: Zhao1, Wei <redacted>; dev@dpdk.org
Cc: Lu, Wenzhuo <redacted>
Subject: Re: [dpdk-dev] [PATCH 01/18] net/ixgbe: store SYN filter

On 12/2/2016 10:42 AM, Wei Zhao wrote:

quoted

From: wei zhao1 <redacted>

Add support for storing SYN filter in SW.

Do you think does it makes more clear to refer as TCP SYN filter? Or
SYN filter is clear enough?

Ok, I will change to " TCP SYN filter " to make it more clear

quoted

quoted

Signed-off-by: Wenzhuo Lu <redacted>
Signed-off-by: wei zhao1 <redacted>

Can you please update sign-off to your actual name?

Ok, I will change to " Signed-off-by: Wei Zhao [off-list ref]"

quoted

quoted

---
 drivers/net/ixgbe/ixgbe_ethdev.c | 12 ++++++++++--
drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c

b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..7f10cca 100644

--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c

@@ -1287,6 +1287,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev

*eth_dev)

quoted

quoted

quoted

 	memset(filter_info->fivetuple_mask, 0,
 	       sizeof(uint32_t) * IXGBE_5TUPLE_ARRAY_SIZE);

+	/* initialize SYN filter */
+	filter_info->syn_info = 0;

can it be an option to memset all filter_info? (and of course move
list init after memset)

Maybe, change to the following code?

memset(filter_info, 0, sizeof(struct ixgbe_filter_info));
TAILQ_INIT(&filter_info->fivetuple_list);

But that wiil mix /* initialize ether type filter */ and /* initialize
5tuple filter list */ two process together, Because  struct ixgbe_filter_info

store two type info of ether and 5tuple.

I see filter info consist of different filter types, but as far as I can see they are
not used before this memset, so what is the problem of cleaning all struct?

Currently memset a sub-set of struct, and assigns zero to other field explicitly,
and rest remains undefined and unused. I am suggesting memset whole
structure and get rid of zero assignment.

Ok, do as your suggestion in v3.

quoted

So, not to change ?

struct ixgbe_filter_info {
	uint8_t ethertype_mask;  /* Bit mask for every used ethertype filter

*/

quoted

	/* store used ethertype filters*/
	struct ixgbe_ethertype_filter

ethertype_filters[IXGBE_MAX_ETQF_FILTERS];

quoted

	/* Bit mask for every used 5tuple filter */
	uint32_t fivetuple_mask[IXGBE_5TUPLE_ARRAY_SIZE];
	struct ixgbe_5tuple_filter_list fivetuple_list;
	/* store the SYN filter info */
	uint32_t syn_info;
};

<...>