[PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[Guilherme G. Piccoli]

Currently bnx2x ptp worker tries to read a register with timestamp
information in case of TX packet timestamping and in case it fails,
the routine reschedules itself indefinitely. This was reported as a
kworker always at 100% of CPU usage, which was narrowed down to be
bnx2x ptp_task.

By following the ioctl handler, we could narrow down the problem to
an NTP tool (chrony) requesting HW timestamping from bnx2x NIC with
RX filter zeroed; this isn't reproducible for example with ptp4l
(from linuxptp) since this tool requests a supported RX filter.
It seems NIC FW timestamp mechanism cannot work well with
RX_FILTER_NONE - driver's PTP filter init routine skips a register
write to the adapter if there's not a supported filter request.

This patch addresses the problem of bnx2x ptp thread's everlasting
reschedule by retrying the register read 10 times; between the read
attempts the thread sleeps for an increasing amount of time starting
in 50ms to give FW some time to perform the timestamping. If it still
fails after all retries, we bail out in order to prevent an unbound
resource consumption from bnx2x.

The patch also adds an ethtool statistic for accounting the skipped
TX timestamp packets and it reduces the priority of timestamping
error messages to prevent log flooding. The code was tested using
both linuxptp and chrony.

Reported-and-tested-by: Przemyslaw Hausman <przemyslaw.hausman@canonical.com>
Suggested-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
---

Sudarsana, thanks for the suggestion! I've tried to follow an identical
approach from [0], but still the ptp thread was consuming a lot of CPU
due to the good amount of reschedules.

I decided then to use the loop approach with small increasing delays,
in order to respect the time FW takes eventually to complete timestamping.

Also, I've dropped the PTP "outstanding, etc" messages to debug-level,
they're quite flooding my log. Cheers!

[0] git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=9adebac37e7d

 .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c   | 12 +++++--
 .../ethernet/broadcom/bnx2x/bnx2x_ethtool.c   |  4 ++-
 .../net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 36 ++++++++++++++-----
 .../net/ethernet/broadcom/bnx2x/bnx2x_stats.h |  3 ++
 4 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 008ad0ca89ba..6751cd04e8d8 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -3857,9 +3857,17 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
 		if (!(bp->flags & TX_TIMESTAMPING_EN)) {
-			BNX2X_ERR("Tx timestamping was not enabled, this packet will not be timestamped\n");
+			bp->eth_stats.ptp_skip_tx_ts++;
+			netdev_err_once(bp->dev,
+					"Tx timestamping isn't enabled, this packet won't be timestamped\n");
+			DP(BNX2X_MSG_PTP,
+			   "Tx timestamping isn't enabled, this packet won't be timestamped\n");
 		} else if (bp->ptp_tx_skb) {
-			BNX2X_ERR("The device supports only a single outstanding packet to timestamp, this packet will not be timestamped\n");
+			bp->eth_stats.ptp_skip_tx_ts++;
+			netdev_err_once(bp->dev,
+					"Device supports only a single outstanding packet to timestamp, this packet won't be timestamped\n");
+			DP(BNX2X_MSG_PTP,
+			   "Device supports only a single outstanding packet to timestamp, this packet won't be timestamped\n");
 		} else {
 			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
 			/* schedule check for Tx timestamp */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 51fc845de31a..4a0ba6801c9e 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -182,7 +182,9 @@ static const struct {
 	{ STATS_OFFSET32(driver_filtered_tx_pkt),
 				4, false, "driver_filtered_tx_pkt" },
 	{ STATS_OFFSET32(eee_tx_lpi),
-				4, true, "Tx LPI entry count"}
+				4, true, "Tx LPI entry count"},
+	{ STATS_OFFSET32(ptp_skip_tx_ts),
+				4, false, "ptp_skipped_tx_tstamp" },
 };
 
 #define BNX2X_NUM_STATS		ARRAY_SIZE(bnx2x_stats_arr)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 03ac10b1cd1e..066b24611890 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -15214,11 +15214,27 @@ static void bnx2x_ptp_task(struct work_struct *work)
 	u32 val_seq;
 	u64 timestamp, ns;
 	struct skb_shared_hwtstamps shhwtstamps;
+	bool bail = true;
+	int i;
 
-	/* Read Tx timestamp registers */
-	val_seq = REG_RD(bp, port ? NIG_REG_P1_TLLH_PTP_BUF_SEQID :
-			 NIG_REG_P0_TLLH_PTP_BUF_SEQID);
-	if (val_seq & 0x10000) {
+	/* FW may take a while to complete timestamping; try a bit and if it's
+	 * still not complete, may indicate an error state - bail out then.
+	 */
+	for (i = 0; i <= 10; i++) {
+		/* Read Tx timestamp registers */
+		val_seq = REG_RD(bp, port ? NIG_REG_P1_TLLH_PTP_BUF_SEQID :
+				 NIG_REG_P0_TLLH_PTP_BUF_SEQID);
+		if (val_seq & 0x10000) {
+			bail = false;
+			break;
+		}
+
+		if (!(i % 5)) /* Avoid log flood */
+			DP(BNX2X_MSG_PTP, "There's no valid Tx timestamp yet\n");
+		msleep(50 + 25 * i);
+	}
+
+	if (!bail) {
 		/* There is a valid timestamp value */
 		timestamp = REG_RD(bp, port ? NIG_REG_P1_TLLH_PTP_BUF_TS_MSB :
 				   NIG_REG_P0_TLLH_PTP_BUF_TS_MSB);
@@ -15233,16 +15249,18 @@ static void bnx2x_ptp_task(struct work_struct *work)
 		memset(&shhwtstamps, 0, sizeof(shhwtstamps));
 		shhwtstamps.hwtstamp = ns_to_ktime(ns);
 		skb_tstamp_tx(bp->ptp_tx_skb, &shhwtstamps);
-		dev_kfree_skb_any(bp->ptp_tx_skb);
-		bp->ptp_tx_skb = NULL;
 
 		DP(BNX2X_MSG_PTP, "Tx timestamp, timestamp cycles = %llu, ns = %llu\n",
 		   timestamp, ns);
 	} else {
-		DP(BNX2X_MSG_PTP, "There is no valid Tx timestamp yet\n");
-		/* Reschedule to keep checking for a valid timestamp value */
-		schedule_work(&bp->ptp_task);
+		DP(BNX2X_MSG_PTP,
+		   "Tx timestamp is not recorded (register read=%u)\n",
+		   val_seq);
+		bp->eth_stats.ptp_skip_tx_ts++;
 	}
+
+	dev_kfree_skb_any(bp->ptp_tx_skb);
+	bp->ptp_tx_skb = NULL;
 }
 
 void bnx2x_set_rx_ts(struct bnx2x *bp, struct sk_buff *skb)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
index b2644ed13d06..d55e63692cf3 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
@@ -207,6 +207,9 @@ struct bnx2x_eth_stats {
 	u32 driver_filtered_tx_pkt;
 	/* src: Clear-on-Read register; Will not survive PMF Migration */
 	u32 eee_tx_lpi;
+
+	/* PTP */
+	u32 ptp_skip_tx_ts;
 };
 
 struct bnx2x_eth_q_stats {
-- 
2.22.0

RE: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: Guilherme G. Piccoli <gpiccoli@canonical.com>
> Sent: Tuesday, June 25, 2019 3:54 AM
> To: GR-everest-linux-l2 <GR-everest-linux-l2@marvell.com>;
> netdev@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Ariel Elior <aelior@marvell.com>; gpiccoli@canonical.com;
> jay.vosburgh@canonical.com
> Subject: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled
> indefinitely
> 
> External Email
> 
> ----------------------------------------------------------------------
> Currently bnx2x ptp worker tries to read a register with timestamp
> information in case of TX packet timestamping and in case it fails, the routine
> reschedules itself indefinitely. This was reported as a kworker always at 100%
> of CPU usage, which was narrowed down to be bnx2x ptp_task.
> 
> By following the ioctl handler, we could narrow down the problem to an NTP
> tool (chrony) requesting HW timestamping from bnx2x NIC with RX filter
> zeroed; this isn't reproducible for example with ptp4l (from linuxptp) since
> this tool requests a supported RX filter.
> It seems NIC FW timestamp mechanism cannot work well with
> RX_FILTER_NONE - driver's PTP filter init routine skips a register write to the
> adapter if there's not a supported filter request.
> 
> This patch addresses the problem of bnx2x ptp thread's everlasting
> reschedule by retrying the register read 10 times; between the read
> attempts the thread sleeps for an increasing amount of time starting in 50ms
> to give FW some time to perform the timestamping. If it still fails after all
> retries, we bail out in order to prevent an unbound resource consumption
> from bnx2x.

Thanks for your changes and time on this. In general time-latching happens in couple or more milliseconds (even in some 100s of usec) under the normal traffic conditions. With this approach, there's a possibility that every packet has to wait for atleast 50ms for the timestamping. This in turn affects the wait-queue (of packets to be timestamped) at hardware as next TS recording happens only after the register is freed/read. And also, it incurs some latency for the ptp applications.

PTP thread is consuming time may be due to the debug messages in this error path, which you are planning address already (thanks!!).
   "Also, I've dropped the PTP "outstanding, etc" messages to debug-level, they're quite flooding my log.
Do you see cpu hog even after removing this message? In such case we may need to think of other alternatives such as sleep for 1 ms.

Just for the info, the approach continuous-poll-for-timestamp() is used ixgbe driver (ixgbe_ptp_tx_hwtstamp_work()) as well.

> 
> The patch also adds an ethtool statistic for accounting the skipped TX
> timestamp packets and it reduces the priority of timestamping error
> messages to prevent log flooding. The code was tested using both linuxptp
> and chrony.
> 
> Reported-and-tested-by: Przemyslaw Hausman
> <przemyslaw.hausman@canonical.com>
> Suggested-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
> 
> Sudarsana, thanks for the suggestion! I've tried to follow an identical
> approach from [0], but still the ptp thread was consuming a lot of CPU due to
> the good amount of reschedules.
> 
> I decided then to use the loop approach with small increasing delays, in order
> to respect the time FW takes eventually to complete timestamping.
> 
> Also, I've dropped the PTP "outstanding, etc" messages to debug-level,
> they're quite flooding my log. Cheers!
> 
> [0] git.kernel.org/pub/scm/linux/kernel/git/davem/net-
> next.git/commit/?id=9adebac37e7d
> 
>  .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c   | 12 +++++--
>  .../ethernet/broadcom/bnx2x/bnx2x_ethtool.c   |  4 ++-
>  .../net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 36 ++++++++++++++----
> -  .../net/ethernet/broadcom/bnx2x/bnx2x_stats.h |  3 ++
>  4 files changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 008ad0ca89ba..6751cd04e8d8 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -3857,9 +3857,17 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
> 
>  	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
>  		if (!(bp->flags & TX_TIMESTAMPING_EN)) {
> -			BNX2X_ERR("Tx timestamping was not enabled, this
> packet will not be timestamped\n");
> +			bp->eth_stats.ptp_skip_tx_ts++;
> +			netdev_err_once(bp->dev,
> +					"Tx timestamping isn't enabled, this
> packet won't be timestamped\n");
> +			DP(BNX2X_MSG_PTP,
> +			   "Tx timestamping isn't enabled, this packet won't
> be
> +timestamped\n");
>  		} else if (bp->ptp_tx_skb) {
> -			BNX2X_ERR("The device supports only a single
> outstanding packet to timestamp, this packet will not be timestamped\n");
> +			bp->eth_stats.ptp_skip_tx_ts++;
> +			netdev_err_once(bp->dev,
> +					"Device supports only a single
> outstanding packet to timestamp, this packet won't be timestamped\n");
> +			DP(BNX2X_MSG_PTP,
> +			   "Device supports only a single outstanding packet to
> timestamp,
> +this packet won't be timestamped\n");
>  		} else {
>  			skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>  			/* schedule check for Tx timestamp */ diff --git
> a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> index 51fc845de31a..4a0ba6801c9e 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> @@ -182,7 +182,9 @@ static const struct {
>  	{ STATS_OFFSET32(driver_filtered_tx_pkt),
>  				4, false, "driver_filtered_tx_pkt" },
>  	{ STATS_OFFSET32(eee_tx_lpi),
> -				4, true, "Tx LPI entry count"}
> +				4, true, "Tx LPI entry count"},
> +	{ STATS_OFFSET32(ptp_skip_tx_ts),
> +				4, false, "ptp_skipped_tx_tstamp" },
>  };
> 
>  #define BNX2X_NUM_STATS		ARRAY_SIZE(bnx2x_stats_arr)
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 03ac10b1cd1e..066b24611890 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -15214,11 +15214,27 @@ static void bnx2x_ptp_task(struct work_struct
> *work)
>  	u32 val_seq;
>  	u64 timestamp, ns;
>  	struct skb_shared_hwtstamps shhwtstamps;
> +	bool bail = true;
> +	int i;
> 
> -	/* Read Tx timestamp registers */
> -	val_seq = REG_RD(bp, port ? NIG_REG_P1_TLLH_PTP_BUF_SEQID :
> -			 NIG_REG_P0_TLLH_PTP_BUF_SEQID);
> -	if (val_seq & 0x10000) {
> +	/* FW may take a while to complete timestamping; try a bit and if it's
> +	 * still not complete, may indicate an error state - bail out then.
> +	 */
> +	for (i = 0; i <= 10; i++) {
> +		/* Read Tx timestamp registers */
> +		val_seq = REG_RD(bp, port ?
> NIG_REG_P1_TLLH_PTP_BUF_SEQID :
> +				 NIG_REG_P0_TLLH_PTP_BUF_SEQID);
> +		if (val_seq & 0x10000) {
> +			bail = false;
> +			break;
> +		}
> +
> +		if (!(i % 5)) /* Avoid log flood */
> +			DP(BNX2X_MSG_PTP, "There's no valid Tx timestamp
> yet\n");
> +		msleep(50 + 25 * i);
> +	}
> +
> +	if (!bail) {
>  		/* There is a valid timestamp value */
>  		timestamp = REG_RD(bp, port ?
> NIG_REG_P1_TLLH_PTP_BUF_TS_MSB :
>  				   NIG_REG_P0_TLLH_PTP_BUF_TS_MSB);
> @@ -15233,16 +15249,18 @@ static void bnx2x_ptp_task(struct work_struct
> *work)
>  		memset(&shhwtstamps, 0, sizeof(shhwtstamps));
>  		shhwtstamps.hwtstamp = ns_to_ktime(ns);
>  		skb_tstamp_tx(bp->ptp_tx_skb, &shhwtstamps);
> -		dev_kfree_skb_any(bp->ptp_tx_skb);
> -		bp->ptp_tx_skb = NULL;
> 
>  		DP(BNX2X_MSG_PTP, "Tx timestamp, timestamp cycles =
> %llu, ns = %llu\n",
>  		   timestamp, ns);
>  	} else {
> -		DP(BNX2X_MSG_PTP, "There is no valid Tx timestamp
> yet\n");
> -		/* Reschedule to keep checking for a valid timestamp value
> */
> -		schedule_work(&bp->ptp_task);
> +		DP(BNX2X_MSG_PTP,
> +		   "Tx timestamp is not recorded (register read=%u)\n",
> +		   val_seq);
> +		bp->eth_stats.ptp_skip_tx_ts++;
>  	}
> +
> +	dev_kfree_skb_any(bp->ptp_tx_skb);
> +	bp->ptp_tx_skb = NULL;
>  }
> 
>  void bnx2x_set_rx_ts(struct bnx2x *bp, struct sk_buff *skb) diff --git
> a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> index b2644ed13d06..d55e63692cf3 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> @@ -207,6 +207,9 @@ struct bnx2x_eth_stats {
>  	u32 driver_filtered_tx_pkt;
>  	/* src: Clear-on-Read register; Will not survive PMF Migration */
>  	u32 eee_tx_lpi;
> +
> +	/* PTP */
> +	u32 ptp_skip_tx_ts;
>  };
> 
>  struct bnx2x_eth_q_stats {
> --
> 2.22.0

Re: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[Guilherme Piccoli]

On Tue, Jun 25, 2019 at 1:02 AM Sudarsana Reddy Kalluru
<skalluru@marvell.com> wrote:
>
> Thanks for your changes and time on this. In general time-latching happens in couple or more milliseconds (even in some 100s of usec) under the normal traffic conditions. With this approach, there's a possibility that every packet has to wait for atleast 50ms for the timestamping. This in turn affects the wait-queue (of packets to be timestamped) at hardware as next TS recording happens only after the register is freed/read. And also, it incurs some latency for the ptp applications.
>
> PTP thread is consuming time may be due to the debug messages in this error path, which you are planning address already (thanks!!).
>    "Also, I've dropped the PTP "outstanding, etc" messages to debug-level, they're quite flooding my log.
> Do you see cpu hog even after removing this message? In such case we may need to think of other alternatives such as sleep for 1 ms.
> Just for the info, the approach continuous-poll-for-timestamp() is used ixgbe driver (ixgbe_ptp_tx_hwtstamp_work()) as well.
>

Thanks again for the good insights Sudarsana! I'll do some experiments
dropping all messages and checking
if the ptp thread is still consuming a lot of CPU (I believe so). In
this case, I'll rework the approach by starting
the delays in 1ms to avoid impacting the HW wait-queue and causing
delays in ptp applications.

Cheers,


Guilherme

Re: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[Guilherme Piccoli]

Sudarsana, let me ask you something: why does the register is reading
value 0x0 always in the
TX timestamp routine if the RX filter is set to None? This is the main
cause of the thread reschedule
thing.

Of course this thread thing is important to fix, but I was discussing
with my leader here and we
are curious on the reasoning the register is getting 0x0.

Thanks in advance,


Guilherme

RE: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: Guilherme Piccoli <gpiccoli@canonical.com>
> Sent: Wednesday, June 26, 2019 1:56 AM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> jay.vosburgh@canonical.com
> Cc: GR-everest-linux-l2 <GR-everest-linux-l2@marvell.com>;
> netdev@vger.kernel.org; Ariel Elior <aelior@marvell.com>
> Subject: Re: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled
> indefinitely
> 
> Sudarsana, let me ask you something: why does the register is reading value
> 0x0 always in the TX timestamp routine if the RX filter is set to None? This is
> the main cause of the thread reschedule thing.

The register value of zero indicates there is no pending Tx timestamp to be read by the driver.
FW writes/latches the Tx timestamp for PTP event packet in this register. And it does the latching only if the register is free.
In this case user/app look to be requesting  the Timestamp (via skb->tx_flags) for non-ptp Tx packet. In the Tx path, driver schedules a thread for reading the Tx timestamp,
   bnx2x_start_xmit()
   {
        if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) 
                        schedule_work(&bp->ptp_task);
   }
FW seem to be not timestamping the packet at all and driver is indefinitely waiting for it.

> 
> Of course this thread thing is important to fix, but I was discussing with my
> leader here and we are curious on the reasoning the register is getting 0x0.
> 
> Thanks in advance,
> 
> 
> Guilherme

Re: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely

[Guilherme Piccoli]

On Wed, Jun 26, 2019 at 5:25 AM Sudarsana Reddy Kalluru
<skalluru@marvell.com> wrote:
> > Sudarsana, let me ask you something: why does the register is reading value
> > 0x0 always in the TX timestamp routine if the RX filter is set to None? This is
> > the main cause of the thread reschedule thing.
>
> The register value of zero indicates there is no pending Tx timestamp to be read by the driver.
> FW writes/latches the Tx timestamp for PTP event packet in this register. And it does the latching only if the register is free.
> In this case user/app look to be requesting  the Timestamp (via skb->tx_flags) for non-ptp Tx packet. In the Tx path, driver schedules a thread for reading the Tx timestamp,
>    bnx2x_start_xmit()
>    {
>         if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
>                         schedule_work(&bp->ptp_task);
>    }
> FW seem to be not timestamping the packet at all and driver is indefinitely waiting for it.
>

Thanks Sudarsana! I've tried to implement the qede-like approach
again, with the 2s timeout before
bailing-out the thread reschedule. This time, I've remove _all_log
messages, including the DP() ones...
Unfortunately kthread is still consuming 100% of CPU, which makes
sense, since it reschedules itself
the most times it can in this 2s window...I think we really should
have small pauses before retrying to
read the registers. I've worked a V3, implementing 1ms-starting
pauses, which worked well:
https://marc.info/?l=linux-netdev&m=156158032618932

I hope this way we don't harm the PTP applications, nor introduce
delays in the FW wait-queue,
and at same time we can fix the indefinitely reschedule in bnx2x.
Thanks again for your support,


Guilherme