Hi Sugesh,

Please see below.

On Wed, Jul 20, 2016 at 04:32:50PM +0000, Chandran, Sugesh wrote:
[...]

quoted

quoted

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.

quoted

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.

quoted

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.

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.

Are we on the same page?

-- 
Adrien Mazarguil
6WIND