[v3,09/16] net/octeontx2: switch Rx timestamp to dynamic mbuf field

Message ID 20201103001407.2931963-10-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series remove mbuf timestamp |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Thomas Monjalon Nov. 3, 2020, 12:14 a.m. UTC
  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>
---
 drivers/net/octeontx2/otx2_ethdev.c | 10 ++++++++++
 drivers/net/octeontx2/otx2_rx.h     | 19 ++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)
  

Comments

Harman Kalra Nov. 3, 2020, 10:52 a.m. UTC | #1
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
>
  
Thomas Monjalon Nov. 3, 2020, 11:22 a.m. UTC | #2
03/11/2020 11:52, Harman Kalra:
>    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.

Where is the crash? Could you provide a backtrace?
Is the field well registered?
  
Thomas Monjalon Nov. 3, 2020, 12:21 p.m. UTC | #3
03/11/2020 12:22, Thomas Monjalon:
> 03/11/2020 11:52, Harman Kalra:
> >    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.
> 
> Where is the crash? Could you provide a backtrace?
> Is the field well registered?

Sorry Harman, without any more explanation, we must move forward.
I am going to send a v4 without any change for octeontx2.
It should be merged today for -rc2.
  
Harman Kalra Nov. 3, 2020, 2:23 p.m. UTC | #4
On Tue, Nov 03, 2020 at 01:21:01PM +0100, Thomas Monjalon wrote:
> External Email
> 
> ----------------------------------------------------------------------
> 03/11/2020 12:22, Thomas Monjalon:
> > 03/11/2020 11:52, Harman Kalra:
> > >    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.
> > 
> > Where is the crash? Could you provide a backtrace?
> > Is the field well registered?
> 
> Sorry Harman, without any more explanation, we must move forward.
> I am going to send a v4 without any change for octeontx2.
> It should be merged today for -rc2.
> 

Hi Thomas,

   I have fixed the issue and sent a patch on top of your changes,
   kindly apply it:
   https://patches.dpdk.org/patch/83611/

   Once you will squash both the patches, I will abondon my patch.
   
Thanks
Harman

>
  

Patch

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;
 		}
 	}
 }