From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 524CC45E3D; Thu, 5 Dec 2024 22:01:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D49F240B92; Thu, 5 Dec 2024 22:01:02 +0100 (CET) Received: from mail-vk1-f172.google.com (mail-vk1-f172.google.com [209.85.221.172]) by mails.dpdk.org (Postfix) with ESMTP id A312E40A73 for ; Thu, 5 Dec 2024 22:01:01 +0100 (CET) Received: by mail-vk1-f172.google.com with SMTP id 71dfb90a1353d-515285c340fso370218e0c.2 for ; Thu, 05 Dec 2024 13:01:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1733432461; x=1734037261; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=H96UeK7KpY69D1MGdxVAcW27MS8nRguF3FK5IIqEazg=; b=bSy3ASOCYvcgpr5HV18TgduR3DzNB6dJm8zON1b+Vx96rJgAngI0yrNV+7uoRACbuO 1lscila8yjvPYT+5lvGLNesTw+KhFTf1pRMxtyIT6XoC5FFgCH1p2K+0r8LDPXdc/3Zd 9AHhRRBnF4kuH4TeJeYNVSW/mvGK8UVLVFTEezomcG/5DSYAn0ONH/vtRBBWj+kRwHll SB7hDYk1XYE9L64OsPh3YtOjywTUq9Nh7BU+S5iC4FGeiqXPulzzd9qBIDe2dtLi1CkD g5quYEhMYKLyHUgeB+IzSzuk2wkG93fm7SeizrbrHkYWvG0ud0CO73jmXhWtncOg7NQ7 XGsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733432461; x=1734037261; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=H96UeK7KpY69D1MGdxVAcW27MS8nRguF3FK5IIqEazg=; b=L0l4j2RalVT3L0Xn3zoQlO0GJQybVpPP2zizF4SKYZpESybEbFdLfy4G2QmjijUy0W OF+tfaS1bJMcTs2ZhAd4O9HuOtOTfZFLtM04volkBm8sW90BUJfdnDxZSwRWwLVpI7w8 9tep+EF4o2CgPCEkxbpN2y8HFJ7Ulgp++zyxhRTXUmXS5+OPST+Fm1UUMilLFqC+FWoh PJ5Oe8MoITbMsU6Q9mrymFz3zt4Mq30hakxm3rwl/2kUxsm6kN5u5KzKn6lQxGrg3Cwr oTywQepfRk2nVGAAnFWcyz78B/3Xv4lXfL/wLQaM87HW3L3kLtQnlRKMHNQeVR4y85/q b0sA== X-Forwarded-Encrypted: i=1; AJvYcCUvrXJYtaWklVyS+iwhbgfKcC0zl8QAyu6GIMZLAWccZDjwJT/cpDC+Hj82oulsQiOWQqw=@dpdk.org X-Gm-Message-State: AOJu0YzY2ZZ6X23KYbdcTYgYCWDCZ8emBZkK2WrppOx+22/7vGfvvqG4 TDf5OVBIka6xqBVWwER/pAlbQjxdeHNIhVBPwBm5hWWx8j291VWMAA/WIkDNeBFOp/Inij9hKZb z4FlBn81+WbE42zIKVYsuHphURdO7DO7iDyA4UhrfiWal3ca7 X-Gm-Gg: ASbGnctIhLIx3mQfoUdopJsb9ChWtg3b/ihmKLhwqdLG05TdO6mNPIFTxP8WgC/Z7vz h3VDuXuQ9nA+EoEXnKxaNfSEErolFLZRpuPRPrv0zoE5TtyNytfeAH1d/Hb1QYzrc X-Google-Smtp-Source: AGHT+IHt6hovQkKof39K+eOmXdk7dpipDAW0CUexqRuxys6uA74onOoxM/rmuTGJipVxvbVN+BHufcdnKyzFdtm33vU= X-Received: by 2002:a05:6122:d1a:b0:50d:bfb2:4f2f with SMTP id 71dfb90a1353d-515fca06f27mr1079801e0c.2.1733432459394; Thu, 05 Dec 2024 13:00:59 -0800 (PST) MIME-Version: 1.0 References: <20240715221141.16153-1-wathsala.vithanage@arm.com> <20241021015246.304431-1-wathsala.vithanage@arm.com> <20241021015246.304431-3-wathsala.vithanage@arm.com> <20241203131322.41f3afb3@hermes.local> In-Reply-To: From: Stephen Hemminger Date: Thu, 5 Dec 2024 13:00:50 -0800 Message-ID: Subject: Re: [RFC v3 2/2] ethdev: introduce the cache stashing hints API To: David Marchand Cc: Wathsala Vithanage , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , dev , nd@arm.com, Honnappa Nagarahalli , Dhruv Tripathi Content-Type: multipart/alternative; boundary="000000000000fc1a9206288c310d" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --000000000000fc1a9206288c310d Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Your right my test was crude. Just do build and look at symbol table of static linked binary. I was confused since pointer is exposed but not data structure On Thu, Dec 5, 2024, 07:40 David Marchand wrote= : > On Tue, Dec 3, 2024 at 10:13=E2=80=AFPM Stephen Hemminger > wrote: > > > > On Mon, 21 Oct 2024 01:52:46 +0000 > > Wathsala Vithanage wrote: > > > > > Extend the ethdev library to enable the stashing of different data > > > objects, such as the ones listed below, into CPU caches directly > > > from the NIC. > > > > > > - Rx/Tx queue descriptors > > > - Rx packets > > > - Packet headers > > > - packet payloads > > > - Data of a packet at an offset from the start of the packet > > > > > > The APIs are designed in a hardware/vendor agnostic manner such that > > > supporting PMDs could use any capabilities available in the underlyin= g > > > hardware for fine-grained stashing of data objects into a CPU cache > > > (e.g., Steering Tags int PCIe TLP Processing Hints). > > > > > > The API provides an interface to query the availability of stashing > > > capabilities, i.e., platform/NIC support, stashable object types, etc= , > > > via the rte_eth_dev_stashing_capabilities_get interface. > > > > > > The function pair rte_eth_dev_stashing_rx_config_set and > > > rte_eth_dev_stashing_tx_config_set sets the stashing hint (the CPU, > > > cache level, and data object types) on the Rx and Tx queues. > > > > > > PMDs that support stashing must register their implementations with t= he > > > following eth_dev_ops callbacks, which are invoked by the ethdev > > > functions listed above. > > > > > > - stashing_capabilities_get > > > - stashing_rx_hints_set > > > - stashing_tx_hints_set > > > > > > Signed-off-by: Wathsala Vithanage > > > Reviewed-by: Honnappa Nagarahalli > > > Reviewed-by: Dhruv Tripathi > > > > > > --- > > > lib/ethdev/ethdev_driver.h | 66 +++++++++++++++ > > > lib/ethdev/rte_ethdev.c | 120 +++++++++++++++++++++++++++ > > > lib/ethdev/rte_ethdev.h | 161 +++++++++++++++++++++++++++++++++++= ++ > > > lib/ethdev/version.map | 4 + > > > 4 files changed, 351 insertions(+) > > > > > > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > > > index 1fd4562b40..7caaea54a8 100644 > > > --- a/lib/ethdev/ethdev_driver.h > > > +++ b/lib/ethdev/ethdev_driver.h > > > @@ -1367,6 +1367,68 @@ enum rte_eth_dev_operation { > > > typedef uint64_t (*eth_get_restore_flags_t)(struct rte_eth_dev *dev, > > > enum rte_eth_dev_operation > op); > > > > > > +/** > > > + * @internal > > > + * Set cache stashing hints in Rx queue. > > > + * > > > + * @param dev > > > + * Port (ethdev) handle. > > > + * @param queue_id > > > + * Rx queue. > > > + * @param config > > > + * Stashing hints configuration for the queue. > > > + * > > > + * @return > > > + * -ENOTSUP if the device or the platform does not support cache > stashing. > > > + * -ENOSYS if the underlying PMD hasn't implemented cache stashin= g > feature. > > > + * -EINVAL on invalid arguments. > > > + * 0 on success. > > > + */ > > > +typedef int (*eth_stashing_rx_hints_set_t)(struct rte_eth_dev *dev, > uint16_t queue_id, > > > + struct > rte_eth_stashing_config *config); > > > + > > > +/** > > > + * @internal > > > + * Set cache stashing hints in Tx queue. > > > + * > > > + * @param dev > > > + * Port (ethdev) handle. > > > + * @param queue_id > > > + * Tx queue. > > > + * @param config > > > + * Stashing hints configuration for the queue. > > > + * > > > + * @return > > > + * -ENOTSUP if the device or the platform does not support cache > stashing. > > > + * -ENOSYS if the underlying PMD hasn't implemented cache stashin= g > feature. > > > + * -EINVAL on invalid arguments. > > > + * 0 on success. > > > + */ > > > +typedef int (*eth_stashing_tx_hints_set_t)(struct rte_eth_dev *dev, > uint16_t queue_id, > > > + struct > rte_eth_stashing_config *config); > > > + > > > +/** > > > + * @internal > > > + * Get cache stashing object types supported in the ethernet device. > > > + * The return value indicates availability of stashing hints support > > > + * in the hardware and the PMD. > > > + * > > > + * @param dev > > > + * Port (ethdev) handle. > > > + * @param objects > > > + * PMD sets supported bits on return. > > > + * > > > + * @return > > > + * -ENOTSUP if the device or the platform does not support cache > stashing. > > > + * -ENOSYS if the underlying PMD hasn't implemented cache stashin= g > feature. > > > + * -EINVAL on NULL values for types or hints parameters. > > > + * On return, types and hints parameters will have bits set for > supported > > > + * object types and hints. > > > + * 0 on success. > > > + */ > > > +typedef int (*eth_stashing_capabilities_get_t)(struct rte_eth_dev > *dev, > > > + uint16_t *objects); > > > + > > > /** > > > * @internal A structure containing the functions exported by an > Ethernet driver. > > > */ > > > @@ -1393,6 +1455,10 @@ struct eth_dev_ops { > > > eth_mac_addr_remove_t mac_addr_remove; /**< Remove MAC > address */ > > > eth_mac_addr_add_t mac_addr_add; /**< Add a MAC addres= s > */ > > > eth_mac_addr_set_t mac_addr_set; /**< Set a MAC addres= s > */ > > > + eth_stashing_rx_hints_set_t stashing_rx_hints_set; /**< Set R= x > cache stashing*/ > > > + eth_stashing_tx_hints_set_t stashing_tx_hints_set; /**< Set T= x > cache stashing*/ > > > + /** Get supported stashing hints*/ > > > + eth_stashing_capabilities_get_t stashing_capabilities_get; > > > /** Set list of multicast addresses */ > > > eth_set_mc_addr_list_t set_mc_addr_list; > > > mtu_set_t mtu_set; /**< Set MTU */ > > > > Since eth_dev_ops is visible in application binary, it is part of the > ABI. > > Therefore it can not be changed until 25.11 release. > > The layout of eth_dev_ops is not exposed to applications as it is in a > private header. > Could you clarify where you see a breakage for an application? > > > I see an ABI breakage for out of tree drivers though. > This could be avoided by moving those added ops at the end of the struct? > > > -- > David Marchand > > --000000000000fc1a9206288c310d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Your right my test was crude. Just do build and look at s= ymbol table of static linked binary.
I was confused since = pointer is exposed but not data structure

On Thu, Dec 5, 2024, 07:40 D= avid Marchand <david.marcha= nd@redhat.com> wrote:
On Tue, Dec 3, 2024 at 10:13=E2=80=AFPM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 21 Oct 2024 01:52:46 +0000
> Wathsala Vithanage <wathsala.vithanage@arm.com> wrot= e:
>
> > Extend the ethdev library to enable the stashing of different dat= a
> > objects, such as the ones listed below, into CPU caches directly<= br> > > from the NIC.
> >
> > - Rx/Tx queue descriptors
> > - Rx packets
> > - Packet headers
> > - packet payloads
> > - Data of a packet at an offset from the start of the packet
> >
> > The APIs are designed in a hardware/vendor agnostic manner such t= hat
> > supporting PMDs could use any capabilities available in the under= lying
> > hardware for fine-grained stashing of data objects into a CPU cac= he
> > (e.g., Steering Tags int PCIe TLP Processing Hints).
> >
> > The API provides an interface to query the availability of stashi= ng
> > capabilities, i.e., platform/NIC support, stashable object types,= etc,
> > via the rte_eth_dev_stashing_capabilities_get interface.
> >
> > The function pair rte_eth_dev_stashing_rx_config_set and
> > rte_eth_dev_stashing_tx_config_set sets the stashing hint (the CP= U,
> > cache level, and data object types) on the Rx and Tx queues.
> >
> > PMDs that support stashing must register their implementations wi= th the
> > following eth_dev_ops callbacks, which are invoked by the ethdev<= br> > > functions listed above.
> >
> > - stashing_capabilities_get
> > - stashing_rx_hints_set
> > - stashing_tx_hints_set
> >
> > Signed-off-by: Wathsala Vithanage <wathsala.vithanage@= arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha= lli@arm.com>
> > Reviewed-by: Dhruv Tripathi <dhruv.tripathi@arm.com>= ;
> >
> > ---
> >=C2=A0 lib/ethdev/ethdev_driver.h |=C2=A0 66 +++++++++++++++
> >=C2=A0 lib/ethdev/rte_ethdev.c=C2=A0 =C2=A0 | 120 ++++++++++++++++= +++++++++++
> >=C2=A0 lib/ethdev/rte_ethdev.h=C2=A0 =C2=A0 | 161 ++++++++++++++++= +++++++++++++++++++++
> >=C2=A0 lib/ethdev/version.map=C2=A0 =C2=A0 =C2=A0|=C2=A0 =C2=A04 +=
> >=C2=A0 4 files changed, 351 insertions(+)
> >
> > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_drive= r.h
> > index 1fd4562b40..7caaea54a8 100644
> > --- a/lib/ethdev/ethdev_driver.h
> > +++ b/lib/ethdev/ethdev_driver.h
> > @@ -1367,6 +1367,68 @@ enum rte_eth_dev_operation {
> >=C2=A0 typedef uint64_t (*eth_get_restore_flags_t)(struct rte_eth_= dev *dev,
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0enum rte_eth_dev_operation op);
> >
> > +/**
> > + * @internal
> > + * Set cache stashing hints in Rx queue.
> > + *
> > + * @param dev
> > + *=C2=A0 =C2=A0Port (ethdev) handle.
> > + * @param queue_id
> > + *=C2=A0 =C2=A0Rx queue.
> > + * @param config
> > + *=C2=A0 =C2=A0Stashing hints configuration for the queue.
> > + *
> > + * @return
> > + *=C2=A0 =C2=A0-ENOTSUP if the device or the platform does not s= upport cache stashing.
> > + *=C2=A0 =C2=A0-ENOSYS=C2=A0 if the underlying PMD hasn't im= plemented cache stashing feature.
> > + *=C2=A0 =C2=A0-EINVAL=C2=A0 on invalid arguments.
> > + *=C2=A0 =C2=A00 on success.
> > + */
> > +typedef int (*eth_stashing_rx_hints_set_t)(struct rte_eth_dev *d= ev, uint16_t queue_id,
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 struct rte_eth_stashing_config *config);
> > +
> > +/**
> > + * @internal
> > + * Set cache stashing hints in Tx queue.
> > + *
> > + * @param dev
> > + *=C2=A0 =C2=A0Port (ethdev) handle.
> > + * @param queue_id
> > + *=C2=A0 =C2=A0Tx queue.
> > + * @param config
> > + *=C2=A0 =C2=A0Stashing hints configuration for the queue.
> > + *
> > + * @return
> > + *=C2=A0 =C2=A0-ENOTSUP if the device or the platform does not s= upport cache stashing.
> > + *=C2=A0 =C2=A0-ENOSYS=C2=A0 if the underlying PMD hasn't im= plemented cache stashing feature.
> > + *=C2=A0 =C2=A0-EINVAL=C2=A0 on invalid arguments.
> > + *=C2=A0 =C2=A00 on success.
> > + */
> > +typedef int (*eth_stashing_tx_hints_set_t)(struct rte_eth_dev *d= ev, uint16_t queue_id,
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 struct rte_eth_stashing_config *config);
> > +
> > +/**
> > + * @internal
> > + * Get cache stashing object types supported in the ethernet dev= ice.
> > + * The return value indicates availability of stashing hints sup= port
> > + * in the hardware and the PMD.
> > + *
> > + * @param dev
> > + *=C2=A0 =C2=A0Port (ethdev) handle.
> > + * @param objects
> > + *=C2=A0 =C2=A0PMD sets supported bits on return.
> > + *
> > + * @return
> > + *=C2=A0 =C2=A0-ENOTSUP if the device or the platform does not s= upport cache stashing.
> > + *=C2=A0 =C2=A0-ENOSYS=C2=A0 if the underlying PMD hasn't im= plemented cache stashing feature.
> > + *=C2=A0 =C2=A0-EINVAL=C2=A0 on NULL values for types or hints p= arameters.
> > + *=C2=A0 =C2=A0On return, types and hints parameters will have b= its set for supported
> > + *=C2=A0 =C2=A0object types and hints.
> > + *=C2=A0 =C2=A00 on success.
> > + */
> > +typedef int (*eth_stashing_capabilities_get_t)(struct rte_eth_de= v *dev,
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 uint16_t *objects);
> > +
> >=C2=A0 /**
> >=C2=A0 =C2=A0* @internal A structure containing the functions expo= rted by an Ethernet driver.
> >=C2=A0 =C2=A0*/
> > @@ -1393,6 +1455,10 @@ struct eth_dev_ops {
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0eth_mac_addr_remove_t=C2=A0 =C2=A0 =C2= =A0 mac_addr_remove; /**< Remove MAC address */
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0eth_mac_addr_add_t=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0mac_addr_add;=C2=A0 /**< Add a MAC address */
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0eth_mac_addr_set_t=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0mac_addr_set;=C2=A0 /**< Set a MAC address */
> > +=C2=A0 =C2=A0 =C2=A0eth_stashing_rx_hints_set_t=C2=A0 =C2=A0stas= hing_rx_hints_set; /**< Set Rx cache stashing*/
> > +=C2=A0 =C2=A0 =C2=A0eth_stashing_tx_hints_set_t=C2=A0 =C2=A0stas= hing_tx_hints_set; /**< Set Tx cache stashing*/
> > +=C2=A0 =C2=A0 =C2=A0/** Get supported stashing hints*/
> > +=C2=A0 =C2=A0 =C2=A0eth_stashing_capabilities_get_t stashing_cap= abilities_get;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0/** Set list of multicast addresses */<= br> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0eth_set_mc_addr_list_t=C2=A0 =C2=A0 =C2= =A0set_mc_addr_list;
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0mtu_set_t=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 mtu_set;=C2=A0 =C2=A0 =C2=A0 =C2=A0/**&l= t; Set MTU */
>
> Since eth_dev_ops is visible in application binary, it is part of the = ABI.
> Therefore it can not be changed until 25.11 release.

The layout of eth_dev_ops is not exposed to applications as it is in a
private header.
Could you clarify where you see a breakage for an application?


I see an ABI breakage for out of tree drivers though.
This could be avoided by moving those added ops at the end of the struct?

--
David Marchand

--000000000000fc1a9206288c310d--