Re: [RFC v2] ethdev: an API for cache stashing hints

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 7/15/2024 11:11 PM, Wathsala Vithanage wrote:
> An application provides cache stashing hints to the ethernet devices to
> improve memory access latencies from the CPU and the NIC. This patch
> introduces three distinct hints for this purpose.
> 
> The RTE_ETH_DEV_STASH_HINT_HOST_WILLNEED hint indicates that the host
> (CPU) requires the data written by the NIC immediately. This implies
> that the CPU expects to read data from its local cache rather than LLC
> or main memory if possible. This would improve memory access latency in
> the Rx path. For PCI devices with TPH capability, these hints translate
> into DWHR (Device Writes Host Reads) access pattern. This hint is only
> valid for receive queues.
> 
> The RTE_ETH_DEV_STASH_HINT_BI_DIR_DATA hint indicates that the host and
> the device access the data structure equally. Rx/Tx queue descriptors
> fit the description of such data. This hint applies to both Rx and Tx
> directions.  In the PCI TPH context, this hint translates into a
> Bi-Directional access pattern.
> 
> RTE_ETH_DEV_STASH_HINT_DEV_ONLY hint indicates that the CPU is not
> involved in a given device's receive or transmit paths. This implies
> that only devices are involved in the IO path. Depending on the
> implementation, this hint may result in data getting placed in a cache
> close to the device or not cached at all. For PCI devices with TPH
> capability, this hint translates into D*D* (DWDR, DRDW, DWDW, DRDR)
> access patterns. This is a bidirectional hint, and it can be applied to
> both Rx and Tx queues.  
> 
> The RTE_ETH_DEV_STASH_HINT_HOST_DONTNEED hint indicates that the device
> reads data written by the host (CPU) that may still be in the host's
> local cache but is not required by the host anytime soon. This hint is
> intended to prevent unnecessary cache invalidations that cause
> interconnect latencies when a device writes to a buffer already in host
> cache memory. In DPDK, this could happen with the recycling of mbufs
> where a mbuf is placed in the Tx queue that then gets back into mempool
> and gets recycled back into the Rx queue, all while a copy is being held
> in the CPU's local cache unnecessarily. By using this hint on supported
> platforms, the mbuf will be invalidated after the device completes the
> buffer reading, but it will be well before the buffer gets recycled and
> updated in the Rx path. This hint is only valid for transmit queues. 
> 
> Applications use three main interfaces in the ethdev library to discover
> and set cache stashing hints. rte_eth_dev_stashing_hints_tx interface is
> used to set hints on a Tx queue. rte_eth_dev_stashing_hints_rx interface
> is used to set hints on an Rx queue. Both of these functions take the
> following parameters as inputs: a port_id (the id of the ethernet
> device), a cpu_id (the target CPU), a cache_level (the level of the
> cache hierarchy the data should be stashed into), a queue_id (the queue
> the hints are applied to). In addition to the above list of parameters,
> a type parameter indicates the type of the object the application
> expects to be stashed by the hardware. Depending on the hardware, these
> may vary. Intel E810 NICs support the stashing of Rx/Tx descriptors,
> packet headers, and packet payloads. These are indicated by the macros
> RTE_ETH_DEV_STASH_TYPE_DESC, RTE_ETH_DEV_STASH_TYPE_HEADER,
> RTE_ETH_DEV_STASH_TYPE_PAYLOAD. Hardware capable of stashing data at any
> given offset into a packet can use the RTE_ETH_DEV_STASH_TYPE_OFFSET
> type. When an offset is used, the offset parameter in the above two
> functions should be set appropriately.
> 
> rte_eth_dev_stashing_hints_discover is used to discover the object types
> and hints supported in the platform and the device. The function takes
> types and hints pointers used as a bit vector to indicate hints and
> types supported by the NIC. An application that intends to use stashing
> hints should first discover supported hints and types and then use the
> functions rte_eth_dev_stashing_hints_tx and
> rte_eth_dev_stashing_hints_rx as required to set stashing hints
> accordingly. eth_dev_ops structure has been updated with two new ops
> that a PMD should implement to support cache stashing hints. A PMD that
> intends to support cache stashing hints should initialize the
> set_stashing_hints function pointer to a function that issues hints to
> the underlying hardware in compliance with platform capabilities. The
> same PMD should also implement a function that can return two-bit fields
> indicating supported types and hints and then initialize the
> discover_stashing_hints function pointer with it. If the NIC supports
> cache stashing hints, the NIC should always set the
> RTE_ETH_DEV_CAPA_CACHE_STASHING device capability.
> 
> Signed-off-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
> Reviewed-by: Dhruv Tripathi <dhruv.tripathi@arm.com>
> 

This is a fine grained config for performance improvement, it may help
to see the performance impact and driver implementation complexity,
before deciding how practical it is.
As ethdev API, as long as it is separate set of APIs, I don't see much
concern to have them.

In existing FEC APIs, and now in the speed lane patchset [1], we are
following similar design with three APIs:
rte_eth_X_set()
rte_eth_X_get()
rte_eth_X_get_capability()

Instead of adding RTE_ETH_DEV_CAPA_ macro and contaminating
'rte_eth_dev_info' with this edge use case, what do you think follow
above design and have dedicated get capability API?

And I can see set() has two different APIs,
'rte_eth_dev_stashing_hints_rx' & 'rte_eth_dev_stashing_hints_tx', is
there a reason to have two separate APIs instead of having one which
gets RX & TX as argument, as done in internal device ops?



[1]
https://patches.dpdk.org/project/dpdk/list/?series=32450&state=*