Hi, Rahul

-----Original Message-----
From: Rahul Lakkireddy [mailto:rahul.lakkireddy@chelsio.com]
Sent: Wednesday, January 13, 2016 4:49 PM
To: Wu, Jingjing
Cc: dev@dpdk.org; Felix Marti; Kumar A S; Nirranjan Kirubaharan
Subject: Re: [dpdk-dev] [RFC v2 1/2] ethdev: add packet filter flow and new behavior
switch to fdir

Hi Jingjing,

On Tuesday, January 01/12/16, 2016 at 17:12:47 -0800, Wu, Jingjing wrote:

quoted

quoted

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index ce224ad..5cc22a0 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h

@@ -74,7 +74,11 @@ extern "C" {
 #define RTE_ETH_FLOW_IPV6_EX            15
 #define RTE_ETH_FLOW_IPV6_TCP_EX        16
 #define RTE_ETH_FLOW_IPV6_UDP_EX        17
-#define RTE_ETH_FLOW_MAX                18
+#define RTE_ETH_FLOW_PKT_FILTER_IPV4_TCP 18 #define
+RTE_ETH_FLOW_PKT_FILTER_IPV4_UDP 19 #define
+RTE_ETH_FLOW_PKT_FILTER_IPV6_TCP 20 #define
+RTE_ETH_FLOW_PKT_FILTER_IPV6_UDP 21
+#define RTE_ETH_FLOW_MAX                22

How to distinguish RTE_ETH_FLOW_PKT_FILTER_IPV4_XX with

RTE_ETH_FLOW_NONFRAG_IPV4_XX, what is the difference?

The packet filter flow is basically a superset containing Ethernet,
vlan, ipv4/ipv6 and tcp/udp flows whose fields can all be matched at
the same time, unlike in case of the current flow director which seems
to match only one of the flows at any given time.  Additionally, it also
allows specifying masks.  I separated the two to make this meaning
explicit.  If this is not necessary, then I will merge them.

Thanks for clarification, now I understand. How about just define one to indicate using pkt_filter?
And move the IPV4_XX info to the structure rte_eth_pkt_filter?

quoted

There is also a patch http://dpdk.org/dev/patchwork/patch/9661/ which added these

fields. Maybe we can merge them together.

Yes, we can merge them.  Would you like me to merge your patch here?

The i40e driver implementation is done based on the change. If you'd like to merge,
maybe other patches in the patch set also need to be merged. Anyway, I think
maintainer can deal with it. 

The current rte_eth_XX_flow only allow matching _one_ of the flows
because of the union.  In contrast, rte_eth_pkt_filter_flow can match
several flows at the same time; i.e. it can match ethernet, vlan, ip,
and tcp/udp all at the same time.  rte_eth_pkt_filter_flow is basically
a superset containing several flows that can be matched at the same
time.

In our Chelsio T5 hardware, it's possible to have several flows that can
be matched in a single rule. This is why I've created a superset that
can match several flows in the same rule.

Thanks,
Rahul

Thanks for clarification, it's clear. And it's great to have this feature.
Even it is a superset containing several flows, we still can use the
existing structs like
struct rte_eth_ipv4_flow {
	struct rte_eth_l2_flow ether;
	struct rte_eth_vlan_flow vlan;
	uint32_t src_ip;
	uint32_t dst_ip;
};
What do you think?

Thanks
Jingjing