[dpdk-dev] [RFC v2 1/2] ethdev: add packet filter flow and new behavior switch to fdir

[Wu, Jingjing]

Hi, Rahul

> -----Original Message-----
> From: Rahul Lakkireddy [mailto:rahul.lakkireddy at chelsio.com]
> Sent: Wednesday, January 13, 2016 4:49 PM
> To: Wu, Jingjing
> Cc: dev at 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:
> > >
> > > 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?
> > 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