linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] net: macb: Process tx timestamp only on ptp packets
@ 2021-08-24 10:12 Harini Katakam
  2021-08-24 14:05 ` Richard Cochran
  0 siblings, 1 reply; 4+ messages in thread
From: Harini Katakam @ 2021-08-24 10:12 UTC (permalink / raw)
  To: nicolas.ferre, davem, richardcochran, claudiu.beznea,
	andrei.pistirica, kuba
  Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux, harini.katakam

The current implementation timestamps all packets and also processes
the BD timestamp for the same. While it is true that HWTSTAMP_TX_ON
enables timestamps for outgoing packets, the sender of the packet
i.e. linuxptp enables timestamp for PTP or PTP event packets. Cadence
GEM IP has a provision to enable this in HW only for PTP packets.
Enable this option in DMA BD settings register to decrease overhead.

Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Acked-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/net/ethernet/cadence/macb_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index c2e1f163bb14..e4f26d972219 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -471,7 +471,7 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
 			return -ERANGE;
 		fallthrough;
 	case HWTSTAMP_TX_ON:
-		tx_bd_control = TSTAMP_ALL_FRAMES;
+		tx_bd_control = TSTAMP_ALL_PTP_FRAMES;
 		break;
 	default:
 		return -ERANGE;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] net: macb: Process tx timestamp only on ptp packets
  2021-08-24 10:12 [RFC PATCH] net: macb: Process tx timestamp only on ptp packets Harini Katakam
@ 2021-08-24 14:05 ` Richard Cochran
  2021-08-24 15:29   ` Harini Katakam
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Cochran @ 2021-08-24 14:05 UTC (permalink / raw)
  To: Harini Katakam
  Cc: nicolas.ferre, davem, claudiu.beznea, andrei.pistirica, kuba,
	netdev, linux-kernel, michal.simek, harinikatakamlinux

On Tue, Aug 24, 2021 at 03:42:38PM +0530, Harini Katakam wrote:
> The current implementation timestamps all packets and also processes
> the BD timestamp for the same. While it is true that HWTSTAMP_TX_ON
> enables timestamps for outgoing packets, the sender of the packet
> i.e. linuxptp enables timestamp for PTP or PTP event packets. Cadence
> GEM IP has a provision to enable this in HW only for PTP packets.
> Enable this option in DMA BD settings register to decrease overhead.

NAK, because the HWTSTAMP_TX_ON means to time stamp any frame marked
by user space, not just PTP frames.

This patch does not "decrease overhead" because the code tests whether
time stamping was request per packet:

drivers/net/ethernet/cadence/macb_main.c line 1202

	if (unlikely(skb_shinfo(skb)->tx_flags &
		     SKBTX_HW_TSTAMP) &&
	    gem_ptp_do_txstamp(queue, skb, desc) == 0) {
		...
	}

Thanks,
Richard

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] net: macb: Process tx timestamp only on ptp packets
  2021-08-24 14:05 ` Richard Cochran
@ 2021-08-24 15:29   ` Harini Katakam
  2021-08-25  2:08     ` Richard Cochran
  0 siblings, 1 reply; 4+ messages in thread
From: Harini Katakam @ 2021-08-24 15:29 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Harini Katakam, Nicolas Ferre, David Miller, Claudiu Beznea,
	Andrei Pistirica, kuba, netdev, linux-kernel, Michal Simek

Hi Richard,

On Tue, Aug 24, 2021 at 7:35 PM Richard Cochran
<richardcochran@gmail.com> wrote:
>
> On Tue, Aug 24, 2021 at 03:42:38PM +0530, Harini Katakam wrote:
> > The current implementation timestamps all packets and also processes
> > the BD timestamp for the same. While it is true that HWTSTAMP_TX_ON
> > enables timestamps for outgoing packets, the sender of the packet
> > i.e. linuxptp enables timestamp for PTP or PTP event packets. Cadence
> > GEM IP has a provision to enable this in HW only for PTP packets.
> > Enable this option in DMA BD settings register to decrease overhead.
>
> NAK, because the HWTSTAMP_TX_ON means to time stamp any frame marked
> by user space, not just PTP frames.
>
> This patch does not "decrease overhead" because the code tests whether
> time stamping was request per packet:
>

Thanks for the review.
Yes, there is no SW overhead because the  skb check ensures timestamp
post processing is done only on requested packets. But the IP
timestamps all packets
because this is a register level setting, not per packet. That's the
overhead I was referring to.
But based on your explanation, it looks like we have no option but to enable
TSTAMP_ALL_FRAMES. Thanks.

Regards,
Harini

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] net: macb: Process tx timestamp only on ptp packets
  2021-08-24 15:29   ` Harini Katakam
@ 2021-08-25  2:08     ` Richard Cochran
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Cochran @ 2021-08-25  2:08 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Harini Katakam, Nicolas Ferre, David Miller, Claudiu Beznea,
	Andrei Pistirica, kuba, netdev, linux-kernel, Michal Simek

On Tue, Aug 24, 2021 at 08:59:20PM +0530, Harini Katakam wrote:
> Yes, there is no SW overhead because the  skb check ensures timestamp
> post processing is done only on requested packets. But the IP
> timestamps all packets
> because this is a register level setting, not per packet. That's the
> overhead I was referring to.

But the IP block time stamps the frames in silicon, no?

I don't see how that is "overhead" in any sense of the word.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-08-25  2:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 10:12 [RFC PATCH] net: macb: Process tx timestamp only on ptp packets Harini Katakam
2021-08-24 14:05 ` Richard Cochran
2021-08-24 15:29   ` Harini Katakam
2021-08-25  2:08     ` Richard Cochran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).