[dpdk-dev] [PATCH] net/ark: export ark timestamp dynfield offset

[dpdk-dev] [PATCH] net/ark: export ark timestamp dynfield offset

[Ed Czeck]

Access to mbuf dynfields in application code requires variable
export through header and dynamic library.

Fixes: a926951a606f ("net/ark: switch Rx timestamp to dynamic mbuf field")
Cc: thomas@monjalon.net

Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
---
 drivers/net/ark/ark_ethdev.c    | 8 ++++----
 drivers/net/ark/ark_ethdev_rx.c | 7 ++++---
 drivers/net/ark/ark_ethdev_rx.h | 3 ---
 drivers/net/ark/rte_pmd_ark.h   | 6 +++++-
 drivers/net/ark/version.map     | 2 ++
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index a65899351..661fca777 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -79,8 +79,8 @@ static int  eth_ark_set_mtu(struct rte_eth_dev *dev, uint16_t size);
 #define ARK_TX_MAX_QUEUE (4096 * 4)
 #define ARK_TX_MIN_QUEUE (256)
 
-uint64_t ark_timestamp_rx_dynflag;
-int ark_timestamp_dynfield_offset = -1;
+uint64_t rte_pmd_ark_timestamp_rx_dynflag;
+int rte_pmd_ark_timestamp_dynfield_offset = -1;
 
 int rte_pmd_ark_rx_userdata_dynfield_offset = -1;
 int rte_pmd_ark_tx_userdata_dynfield_offset = -1;
@@ -559,8 +559,8 @@ eth_ark_dev_configure(struct rte_eth_dev *dev)
 
 	if (dev->data->dev_conf.rxmode.offloads & DEV_RX_OFFLOAD_TIMESTAMP) {
 		ret = rte_mbuf_dyn_rx_timestamp_register(
-				&ark_timestamp_dynfield_offset,
-				&ark_timestamp_rx_dynflag);
+				&rte_pmd_ark_timestamp_dynfield_offset,
+				&rte_pmd_ark_timestamp_rx_dynflag);
 		if (ret != 0) {
 			ARK_PMD_LOG(ERR,
 				"Failed to register Rx timestamp field/flag\n");
diff --git a/drivers/net/ark/ark_ethdev_rx.c b/drivers/net/ark/ark_ethdev_rx.c
index d29d3db78..2021e33d0 100644
--- a/drivers/net/ark/ark_ethdev_rx.c
+++ b/drivers/net/ark/ark_ethdev_rx.c
@@ -273,10 +273,11 @@ eth_ark_recv_pkts(void *rx_queue,
 		mbuf->pkt_len = meta->pkt_len;
 		mbuf->data_len = meta->pkt_len;
 		/* set timestamp if enabled at least on one device */
-		if (ark_timestamp_rx_dynflag > 0) {
-			*RTE_MBUF_DYNFIELD(mbuf, ark_timestamp_dynfield_offset,
+		if (rte_pmd_ark_timestamp_rx_dynflag > 0) {
+			*RTE_MBUF_DYNFIELD(mbuf,
+				rte_pmd_ark_timestamp_dynfield_offset,
 				rte_mbuf_timestamp_t *) = meta->timestamp;
-			mbuf->ol_flags |= ark_timestamp_rx_dynflag;
+			mbuf->ol_flags |= rte_pmd_ark_timestamp_rx_dynflag;
 		}
 		rte_pmd_ark_mbuf_rx_userdata_set(mbuf, meta->user_data);
 
diff --git a/drivers/net/ark/ark_ethdev_rx.h b/drivers/net/ark/ark_ethdev_rx.h
index 001fa9bdf..00687a711 100644
--- a/drivers/net/ark/ark_ethdev_rx.h
+++ b/drivers/net/ark/ark_ethdev_rx.h
@@ -11,9 +11,6 @@
 #include <rte_mempool.h>
 #include <rte_ethdev_driver.h>
 
-extern uint64_t ark_timestamp_rx_dynflag;
-extern int ark_timestamp_dynfield_offset;
-
 int eth_ark_dev_rx_queue_setup(struct rte_eth_dev *dev,
 			       uint16_t queue_idx,
 			       uint16_t nb_desc,
diff --git a/drivers/net/ark/rte_pmd_ark.h b/drivers/net/ark/rte_pmd_ark.h
index 6f26d66b1..55be29fc5 100644
--- a/drivers/net/ark/rte_pmd_ark.h
+++ b/drivers/net/ark/rte_pmd_ark.h
@@ -27,6 +27,9 @@ typedef uint64_t rte_pmd_ark_rx_userdata_t;
 extern int rte_pmd_ark_tx_userdata_dynfield_offset;
 extern int rte_pmd_ark_rx_userdata_dynfield_offset;
 
+extern int rte_pmd_ark_timestamp_dynfield_offset;
+extern uint64_t rte_pmd_ark_timestamp_rx_dynflag;
+
 /** mbuf dynamic field for custom Tx ARK data */
 #define RTE_PMD_ARK_TX_USERDATA_DYNFIELD_NAME "rte_net_ark_dynfield_tx_userdata"
 /** mbuf dynamic field for custom Rx ARK data */
@@ -46,7 +49,8 @@ static inline rte_pmd_ark_tx_userdata_t
 rte_pmd_ark_mbuf_tx_userdata_get(const struct rte_mbuf *mbuf)
 {
 #if RTE_PMD_ARK_TX_USERDATA_ENABLE
-	return *RTE_MBUF_DYNFIELD(mbuf, rte_pmd_ark_tx_userdata_dynfield_offset,
+	return *RTE_MBUF_DYNFIELD(mbuf,
+				  rte_pmd_ark_tx_userdata_dynfield_offset,
 				  rte_pmd_ark_tx_userdata_t *);
 #else
 	RTE_SET_USED(mbuf);
diff --git a/drivers/net/ark/version.map b/drivers/net/ark/version.map
index 954bea679..8da6771d0 100644
--- a/drivers/net/ark/version.map
+++ b/drivers/net/ark/version.map
@@ -7,4 +7,6 @@ EXPERIMENTAL {
 
 	rte_pmd_ark_tx_userdata_dynfield_offset;
 	rte_pmd_ark_rx_userdata_dynfield_offset;
+	rte_pmd_ark_timestamp_dynfield_offset;
+	rte_pmd_ark_timestamp_rx_dynflag;
 };
-- 
2.17.1

Re: [dpdk-dev] [PATCH] net/ark: export ark timestamp dynfield offset

[Stephen Hemminger]

On Fri,  4 Dec 2020 15:27:45 -0500
Ed Czeck <ed.czeck@atomicrules.com> wrote:

> Access to mbuf dynfields in application code requires variable
> export through header and dynamic library.
> 
> Fixes: a926951a606f ("net/ark: switch Rx timestamp to dynamic mbuf field")
> Cc: thomas@monjalon.net
> 
> Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>

Not happy with having offsets exposed from driver.

Expopsing weird offsets for their special fields
is worse than the old model.

Why not use standardized field name and have the application lookup that?
There is an API already for that.

Re: [dpdk-dev] [PATCH] net/ark: export ark timestamp dynfield offset

[Thomas Monjalon]

04/12/2020 23:31, Stephen Hemminger:
> On Fri,  4 Dec 2020 15:27:45 -0500
> Ed Czeck <ed.czeck@atomicrules.com> wrote:
> 
> > Access to mbuf dynfields in application code requires variable
> > export through header and dynamic library.
> > 
> > Fixes: a926951a606f ("net/ark: switch Rx timestamp to dynamic mbuf field")
> > Cc: thomas@monjalon.net
> > 
> > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> 
> Not happy with having offsets exposed from driver.
> 
> Expopsing weird offsets for their special fields
> is worse than the old model.
> 
> Why not use standardized field name and have the application lookup that?
> There is an API already for that.

Yes lookup should be used.

Re: [dpdk-dev] [PATCH] net/ark: export ark timestamp dynfield offset

[Ed Czeck]

On Sun, Dec 6, 2020 at 10:53 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 04/12/2020 23:31, Stephen Hemminger:
> > On Fri,  4 Dec 2020 15:27:45 -0500
> > Ed Czeck <ed.czeck@atomicrules.com> wrote:
> >
> > > Access to mbuf dynfields in application code requires variable
> > > export through header and dynamic library.
> > >
> > > Fixes: a926951a606f ("net/ark: switch Rx timestamp to dynamic mbuf field")
> > > Cc: thomas@monjalon.net
> > >
> > > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> >
> > Not happy with having offsets exposed from driver.
> >
> > Expopsing weird offsets for their special fields
> > is worse than the old model.
> >
> > Why not use standardized field name and have the application lookup that?
> > There is an API already for that.
>
> Yes lookup should be used.
>
>
So we lookup and may a copy of that offset in application code -- no
changes to PMD are then required.
Got it, thanks for your comments.