Re: [dpdk-dev] [PATCH v3 09/16] net/octeontx2: switch Rx timestamp to dynamic mbuf field

[Harman Kalra <hkalra@marvell.com>]

On Tue, Nov 03, 2020 at 01:14:00AM +0100, Thomas Monjalon wrote:
> The mbuf timestamp is moved to a dynamic field
> in order to allow removal of the deprecated static field.
> The related mbuf flag is also replaced.
> 
> The dynamic offset and flag are stored in struct otx2_timesync_info
> to favor cache locality.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Hi Thomas,

   With the following changes, ptpclient and testpmd(ieee1588 mode) is
   crashing for us. I am debugging the issue and will update soon.
  ------------------
   Steps to reproduce:
   1. Testpmd:
      ./dpdk-testpmd -c 0xffff01 -n 4 -w 0002:05:00.0 -- -i
      --port-topology=loop
      testpmd> set fwd ieee1588
      testpmd> set port 0 ptype_mask 0xf
      testpmd> start

      I am sending ptp packets using scapy from the peer:
      >>> p = Ether(src='98:03:9b:67:b0:d0', dst='FA:62:0C:27:AD:BC',
		      >>> type=35063)/Raw(load='\x00\x02')
      >>> sendp (p, iface="p5p2")

      I am observing seg fault even for 1 packet.

    2. ./ptpclient -l 1 -n 4 -w 0002:05:00.0 -- -p 0xf    -- on board
       ptp4l -E -2 -H -i p5p2  -m -q -p /dev/ptp4     ... on peer

Thanks
Harman

> ---
>  drivers/net/octeontx2/otx2_ethdev.c | 10 ++++++++++
>  drivers/net/octeontx2/otx2_rx.h     | 19 ++++++++++++++++---
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/octeontx2/otx2_ethdev.c b/drivers/net/octeontx2/otx2_ethdev.c
> index cfb733a4b5..f6962be9b2 100644
> --- a/drivers/net/octeontx2/otx2_ethdev.c
> +++ b/drivers/net/octeontx2/otx2_ethdev.c
> @@ -2219,6 +2219,16 @@ otx2_nix_dev_start(struct rte_eth_dev *eth_dev)
>  	else
>  		otx2_nix_timesync_disable(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;
> +		}
> +	}
> +
>  	/* Update VF about data off shifted by 8 bytes if PTP already
>  	 * enabled in PF owning this VF
>  	 */
> diff --git a/drivers/net/octeontx2/otx2_rx.h b/drivers/net/octeontx2/otx2_rx.h
> index 61a5c436dd..926f614a4e 100644
> --- a/drivers/net/octeontx2/otx2_rx.h
> +++ b/drivers/net/octeontx2/otx2_rx.h
> @@ -49,6 +49,8 @@ struct otx2_timesync_info {
>  	uint64_t	rx_tstamp;
>  	rte_iova_t	tx_tstamp_iova;
>  	uint64_t	*tx_tstamp;
> +	uint64_t	rx_tstamp_dynflag;
> +	int		tstamp_dynfield_offset;
>  	uint8_t		tx_ready;
>  	uint8_t		rx_ready;
>  } __rte_cache_aligned;
> @@ -63,6 +65,14 @@ union mbuf_initializer {
>  	uint64_t value;
>  };
>  
> +static inline rte_mbuf_timestamp_t *
> +otx2_timestamp_dynfield(struct rte_mbuf *mbuf,
> +		struct otx2_timesync_info *info)
> +{
> +	return RTE_MBUF_DYNFIELD(mbuf,
> +		info->tstamp_dynfield_offset, rte_mbuf_timestamp_t *);
> +}
> +
>  static __rte_always_inline void
>  otx2_nix_mbuf_to_tstamp(struct rte_mbuf *mbuf,
>  			struct otx2_timesync_info *tstamp, const uint16_t flag,
> @@ -77,15 +87,18 @@ otx2_nix_mbuf_to_tstamp(struct rte_mbuf *mbuf,
>  		/* Reading the rx timestamp inserted by CGX, viz at
>  		 * starting of the packet data.
>  		 */
> -		mbuf->timestamp = rte_be_to_cpu_64(*tstamp_ptr);
> +		*otx2_timestamp_dynfield(mbuf, tstamp) =
> +				rte_be_to_cpu_64(*tstamp_ptr);
>  		/* PKT_RX_IEEE1588_TMST flag needs to be set only in case
>  		 * PTP packets are received.
>  		 */
>  		if (mbuf->packet_type == RTE_PTYPE_L2_ETHER_TIMESYNC) {
> -			tstamp->rx_tstamp = mbuf->timestamp;
> +			tstamp->rx_tstamp =
> +					*otx2_timestamp_dynfield(mbuf, tstamp);
>  			tstamp->rx_ready = 1;
>  			mbuf->ol_flags |= PKT_RX_IEEE1588_PTP |
> -				PKT_RX_IEEE1588_TMST | PKT_RX_TIMESTAMP;
> +				PKT_RX_IEEE1588_TMST |
> +				tstamp->rx_tstamp_dynflag;
>  		}
>  	}
>  }
> -- 
> 2.28.0
>