diff mbox series

examples/rxtx_callbacks: fix HW timestamp enable

Message ID 1564069902-3500-1-git-send-email-hkalra@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series examples/rxtx_callbacks: fix HW timestamp enable |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Harman Kalra July 25, 2019, 3:51 p.m. UTC 
  Offloafing Rx timestamp is a device capability than queue capability.
Hence the logic to enable HW timestamp via DEV_RX_OFFLOAD_TIMESTAMP
flag should be before device configuration.

Fixes: cd1dadeb9b2a ("examples/rxtx_callbacks: support HW timestamp")
Cc: barbette@kth.se

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 examples/rxtx_callbacks/main.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Jerin Jacob Kollanukkaran July 27, 2019, 5:14 p.m. UTC | #1 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Harman Kalra
> Sent: Thursday, July 25, 2019 9:22 PM
> To: John McNamara <john.mcnamara@intel.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>; Harry van Haaren
> <harry.van.haaren@intel.com>; Xiaoyun Li <xiaoyun.li@intel.com>
> Cc: dev@dpdk.org; Harman Kalra <hkalra@marvell.com>; barbette@kth.se
> Subject: [dpdk-dev] [PATCH] examples/rxtx_callbacks: fix HW timestamp
> enable
> 
> Offloafing Rx timestamp is a device capability than queue capability.
> Hence the logic to enable HW timestamp via DEV_RX_OFFLOAD_TIMESTAMP
> flag should be before device configuration.
> 
> Fixes: cd1dadeb9b2a ("examples/rxtx_callbacks: support HW timestamp")
> Cc: barbette@kth.se
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>

Reviewed-by: Jerin Jacob <jerinj@marvell.com>
Thomas Monjalon July 29, 2019, 10:18 p.m. UTC | #2 
25/07/2019 17:51, Harman Kalra:
> Offloafing Rx timestamp is a device capability than queue capability.

Why is it a device capability and not a queue capability?

> Hence the logic to enable HW timestamp via DEV_RX_OFFLOAD_TIMESTAMP
> flag should be before device configuration.
> 
> Fixes: cd1dadeb9b2a ("examples/rxtx_callbacks: support HW timestamp")
> Cc: barbette@kth.se
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
Harman Kalra July 30, 2019, 6:57 a.m. UTC | #3 
On Tue, Jul 30, 2019 at 12:18:59AM +0200, Thomas Monjalon wrote:
> External Email
> 
> ----------------------------------------------------------------------
> 25/07/2019 17:51, Harman Kalra:
> > Offloafing Rx timestamp is a device capability than queue capability.
> 
> Why is it a device capability and not a queue capability?

1. Since all PMDs doesn't implements per queue offload capabilities but
supports RX timestamping and also since rx_offload_capa includes all
rx_queue_offload_capa's. So we moved the DEV_RX_OFFLOAD_TIMESTAMP
configuration setting to device from queue, as its a test application
and should work with all PMDs.

2. Or we can have a test in this application, i.e. if PMD has
rx_queue_offload_capa implemented go with per queue configuration else
make it a device configuration.

If OK with this first approach, we will send V2 with reworded commit message 
explaining the reason as above.

> 
> > Hence the logic to enable HW timestamp via DEV_RX_OFFLOAD_TIMESTAMP
> > flag should be before device configuration.
> > 
> > Fixes: cd1dadeb9b2a ("examples/rxtx_callbacks: support HW timestamp")
> > Cc: barbette@kth.se
> > 
> > Signed-off-by: Harman Kalra <hkalra@marvell.com>
> 
> 
>
Thomas Monjalon July 30, 2019, 7:11 a.m. UTC | #4 
30/07/2019 08:57, Harman Kalra:
> On Tue, Jul 30, 2019 at 12:18:59AM +0200, Thomas Monjalon wrote:
> > External Email
> > 
> > ----------------------------------------------------------------------
> > 25/07/2019 17:51, Harman Kalra:
> > > Offloafing Rx timestamp is a device capability than queue capability.
> > 
> > Why is it a device capability and not a queue capability?
> 
> 1. Since all PMDs doesn't implements per queue offload capabilities but
> supports RX timestamping and also since rx_offload_capa includes all
> rx_queue_offload_capa's. So we moved the DEV_RX_OFFLOAD_TIMESTAMP
> configuration setting to device from queue, as its a test application
> and should work with all PMDs.
> 
> 2. Or we can have a test in this application, i.e. if PMD has
> rx_queue_offload_capa implemented go with per queue configuration else
> make it a device configuration.
> 
> If OK with this first approach, we will send V2 with reworded commit message 
> explaining the reason as above.

I'm fine with any logic if
1/ it works with all PMDs
2/ the real reason is explained
diff mbox series

Patch

diff --git a/examples/rxtx_callbacks/main.c b/examples/rxtx_callbacks/main.c
index c1abe9e1a..dbcd9f4fc 100644
--- a/examples/rxtx_callbacks/main.c
+++ b/examples/rxtx_callbacks/main.c
@@ -117,6 +117,15 @@  port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 		port_conf.txmode.offloads |=
 			DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 
+	if (hw_timestamping) {
+		if (!(dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TIMESTAMP)) {
+			printf("\nERROR: Port %u does not support hardware timestamping\n"
+					, port);
+			return -1;
+		}
+		port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_TIMESTAMP;
+	}
+
 	retval = rte_eth_dev_configure(port, rx_rings, tx_rings, &port_conf);
 	if (retval != 0)
 		return retval;
@@ -127,15 +136,6 @@  port_init(uint16_t port, struct rte_mempool *mbuf_pool)
 
 	rxconf = dev_info.default_rxconf;
 
-	if (hw_timestamping) {
-		if (!(dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TIMESTAMP)) {
-			printf("\nERROR: Port %u does not support hardware timestamping\n"
-					, port);
-			return -1;
-		}
-		rxconf.offloads |= DEV_RX_OFFLOAD_TIMESTAMP;
-	}
-
 	for (q = 0; q < rx_rings; q++) {
 		retval = rte_eth_rx_queue_setup(port, q, nb_rxd,
 			rte_eth_dev_socket_id(port), &rxconf, mbuf_pool);