[dpdk-dev] [RFC] Generic flow director/filtering/classification API

[Chandran, Sugesh]

Hi Adrien,
Please find my comments below

Regards
_Sugesh


> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil at 6wind.com]
> Sent: Wednesday, July 20, 2016 6:11 PM
> To: Chandran, Sugesh <sugesh.chandran at intel.com>
> Cc: dev at dpdk.org; Thomas Monjalon <thomas.monjalon at 6wind.com>;
> Zhang, Helin <helin.zhang at intel.com>; Wu, Jingjing
> <jingjing.wu at intel.com>; Rasesh Mody <rasesh.mody at qlogic.com>; Ajit
> Khaparde <ajit.khaparde at broadcom.com>; Rahul Lakkireddy
> <rahul.lakkireddy at chelsio.com>; Lu, Wenzhuo <wenzhuo.lu at intel.com>;
> Jan Medala <jan at semihalf.com>; John Daley <johndale at cisco.com>; Chen,
> Jing D <jing.d.chen at intel.com>; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>; Matej Vido <matejvido at gmail.com>;
> Alejandro Lucero <alejandro.lucero at netronome.com>; Sony Chacko
> <sony.chacko at qlogic.com>; Jerin Jacob
> <jerin.jacob at caviumnetworks.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; Olga Shern <olgas at mellanox.com>;
> Chilikin, Andrey <andrey.chilikin at intel.com>
> Subject: Re: [dpdk-dev] [RFC] Generic flow director/filtering/classification
> API
> 
> Hi Sugesh,
> 
> Please see below.
> 
> On Wed, Jul 20, 2016 at 04:32:50PM +0000, Chandran, Sugesh wrote:
> [...]
> > > > How about a hardware flow flag in packet descriptor that set when
> > > > the packets hits any hardware rule. This way software doesn’t
> > > > worry /blocked by a hardware rule . Even though there is an
> > > > additional overhead of validating this flag, software datapath can
> > > > identify the
> > > hardware processed packets easily.
> > > > This way the packets traverses the software fallback path until
> > > > the rule configuration is complete. This flag avoids setting ID
> > > > action for every
> > > hardware flow that are configuring.
> > >
> > > That makes sense. I see it as a sort of single bit ID but it could
> > > be implemented through a different action for less capable devices.
> > > PMDs that support 32 bit IDs could reuse the same code for both
> features.
> > >
> > > I understand you'd prefer having this feature always present,
> > > however we already know that not all PMDs/devices support it, and
> > > like everything else this is a kind of offload that needs to be
> > > explicitly requested by the application as it may not be needed.
> > >
> > > If we go with the separate action, then perhaps it would make sense
> > > to rename "ID" to "MARK" to make things clearer:
> > >
> > >  RTE_FLOW_ACTION_TYPE_FLAG /* Flag packets processed by flow rule.
> > > */
> > >
> > >  RTE_FLOW_ACTION_TYPE_MARK /* Attach a 32 bit value to a packet. */
> > >
> > > I guess the result of the FLAG action would be something in ol_flag.
> > >
> > [Sugesh] This looks fine for me.
> 
> Great, I will update the specification accordingly.
[Sugesh] Thank you!
> 
> > > Thoughts?
> > >
> > [Sugesh] Two more queries that I missed out in the earlier comments
> > are, Support for PTYPE :- Intel NICs can report packet type in mbuf.
> > This can be used by software for the packet processing. Is generic API
> > capable of handling that as well?
> 
> Yes, however no PTYPE action has been defined for this (yet). It is only a
> matter of adding one.
[Sugesh] Thank you for confirming. Its fine for me
> 
> Currently packet type recognition is enabled per port using a separate API, so
> correct me if I'm wrong but I am not aware of any adapter with the ability to
> enable it per flow rule, so I do not think such an action needs to be defined
> from the start. We may add it later.
> 
> > RSS hashing support :- Just to confirm, the RSS flow action allows
> > application to decide the header fields to produce the hash. This
> > gives programmability on load sharing across different queues. The
> > application can program the NIC to calculate the RSS hash only using
> > mac or mac+ ip or ip only using this.
> 
> I'd say yes but from your summary, I'm not sure we share the same idea of
> what the RSS action is supposed to do, so here is mine.
> 
> Like all flow rules, the pattern part of the RSS action only filters the packets
> on which the action will be performed.
> 
> The rss_conf parameter (struct rte_eth_rss_conf) only provides a key and a
> RSS hash function to use (ETH_RSS_IPV4, ETH_RSS_NONFRAG_IPV6_UDP,
> etc).
> 
> Nothing prevents the RSS hash function from being applied to protocol
> headers which are not necessarily present in the flow rule pattern. These are
> two independent things, e.g. you could have a pattern matching IPv4 packets
> yet perform RSS hashing only on UDP headers.
> 
> Finally, the RSS action configuration only affects packets coming from this
> flow rule. It is not performed on the device globally so packets which are not
> matched are not affected by RSS processing. As a result it might not be
> possible to configure two flow rules specifying incompatible RSS actions
> simultaneously if the underlying device supports only a single global RSS
> context.
> 
[Sugesh] thank you for the explanation. This means I can have a rule that matches on
Every incoming packets(all field wild card rule) and does RSS hash on selected fields,
MAC only, IP only or IP & MAC? This can be useful to do a packet lookup in software by just using
Only hash. 
> Are we on the same page?
> 
> --
> Adrien Mazarguil
> 6WIND