examples/rxtx_callbacks: fix HW timestamp enable
Checks
Commit Message
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
> -----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>
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>
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>
>
>
>
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
@@ -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);