net/octeontx2: fix dynamic registration for timestamp
Checks
Commit Message
Registration of timestamp field should be done once and
only when ptp kernel changes are successful.
Fixes: 67b8f3220393 ("net/octeontx2: switch Rx timestamp to dynamic mbuf field")
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
drivers/net/octeontx2/otx2_ethdev.c | 10 ----------
drivers/net/octeontx2/otx2_ptp.c | 17 +++++++++--------
2 files changed, 9 insertions(+), 18 deletions(-)
Comments
03/11/2020 16:20, Harman Kalra:
> Registration of timestamp field should be done once and
Why?
Next registrations does nothing, it is not a problem.
> only when ptp kernel changes are successful.
Rx timestamp can be useful without PTP.
> --- a/drivers/net/octeontx2/otx2_ethdev.c
> +++ b/drivers/net/octeontx2/otx2_ethdev.c
> @@ -2225,16 +2225,6 @@ otx2_nix_dev_start(struct rte_eth_dev *eth_dev)
> if (otx2_ethdev_is_ptp_en(dev) && otx2_dev_is_vf(dev))
> otx2_nix_ptp_enable_vf(eth_dev);
NIX_RX_OFFLOAD_TSTAMP_F is set in above function.
Is it useless? Or should we register timestamp below?
> - if (dev->rx_offload_flags & NIX_RX_OFFLOAD_TSTAMP_F) {
> - rc = rte_mbuf_dyn_rx_timestamp_register(
> - &dev->tstamp.tstamp_dynfield_offset,
> - &dev->tstamp.rx_tstamp_dynflag);
> - if (rc != 0) {
> - otx2_err("Failed to register Rx timestamp field/flag");
> - return -rte_errno;
> - }
> - }
On Tue, Nov 03, 2020 at 04:41:14PM +0100, Thomas Monjalon wrote:
> External Email
>
> ----------------------------------------------------------------------
> 03/11/2020 16:20, Harman Kalra:
> > Registration of timestamp field should be done once and
>
> Why?
> Next registrations does nothing, it is not a problem.
>
> > only when ptp kernel changes are successful.
>
> Rx timestamp can be useful without PTP.
Ack, you are correct, VF change is also required.
>
> > --- a/drivers/net/octeontx2/otx2_ethdev.c
> > +++ b/drivers/net/octeontx2/otx2_ethdev.c
> > @@ -2225,16 +2225,6 @@ otx2_nix_dev_start(struct rte_eth_dev *eth_dev)
> > if (otx2_ethdev_is_ptp_en(dev) && otx2_dev_is_vf(dev))
> > otx2_nix_ptp_enable_vf(eth_dev);
>
> NIX_RX_OFFLOAD_TSTAMP_F is set in above function.
> Is it useless? Or should we register timestamp below?
>
> > - if (dev->rx_offload_flags & NIX_RX_OFFLOAD_TSTAMP_F) {
> > - rc = rte_mbuf_dyn_rx_timestamp_register(
> > - &dev->tstamp.tstamp_dynfield_offset,
> > - &dev->tstamp.rx_tstamp_dynflag);
> > - if (rc != 0) {
> > - otx2_err("Failed to register Rx timestamp field/flag");
> > - return -rte_errno;
> > - }
> > - }
>
>
>
@@ -2225,16 +2225,6 @@ otx2_nix_dev_start(struct rte_eth_dev *eth_dev)
if (otx2_ethdev_is_ptp_en(dev) && otx2_dev_is_vf(dev))
otx2_nix_ptp_enable_vf(eth_dev);
- if (dev->rx_offload_flags & NIX_RX_OFFLOAD_TSTAMP_F) {
- rc = rte_mbuf_dyn_rx_timestamp_register(
- &dev->tstamp.tstamp_dynfield_offset,
- &dev->tstamp.rx_tstamp_dynflag);
- if (rc != 0) {
- otx2_err("Failed to register Rx timestamp field/flag");
- return -rte_errno;
- }
- }
-
rc = npc_rx_enable(dev);
if (rc) {
otx2_err("Failed to enable NPC rx %d", rc);
@@ -239,14 +239,6 @@ otx2_nix_timesync_enable(struct rte_eth_dev *eth_dev)
dev->tstamp.tx_tstamp_iova = ts->iova;
dev->tstamp.tx_tstamp = ts->addr;
- rc = rte_mbuf_dyn_rx_timestamp_register(
- &dev->tstamp.tstamp_dynfield_offset,
- &dev->tstamp.rx_tstamp_dynflag);
- if (rc != 0) {
- otx2_err("Failed to register Rx timestamp field/flag");
- return -rte_errno;
- }
-
/* System time should be already on by default */
nix_start_timecounters(eth_dev);
@@ -264,6 +256,15 @@ otx2_nix_timesync_enable(struct rte_eth_dev *eth_dev)
/* Setting up the function pointers as per new offload flags */
otx2_eth_set_rx_function(eth_dev);
otx2_eth_set_tx_function(eth_dev);
+
+ /* Registering dynamic mbuf timestamp field and flag */
+ rc = rte_mbuf_dyn_rx_timestamp_register(
+ &dev->tstamp.tstamp_dynfield_offset,
+ &dev->tstamp.rx_tstamp_dynflag);
+ if (rc != 0) {
+ otx2_err("Failed to register Rx timestamp field/flag");
+ return -rte_errno;
+ }
}
rc = otx2_nix_recalc_mtu(eth_dev);