[PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Paul Thomas]

Make sure SKBTX_HW_TSTAMP (i.e. SOF_TIMESTAMPING_TX_HARDWARE) has been
enabled for this skb It does fix the issue where normal socks that
aren't expecting a timestamp will not wake up on select. And when a
user does want a SOF_TIMESTAMPING_TX_HARDWARE this does work.

Signed-off-by: Paul Thomas <pthomas8589@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index ad099fd01b45..48fc45ab514f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -898,7 +898,8 @@ static void macb_tx_interrupt(struct macb_queue *queue)
 
 			/* First, update TX stats if needed */
 			if (skb) {
-				if (gem_ptp_do_txstamp(queue, skb, desc) == 0) {
+				if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+					gem_ptp_do_txstamp(queue, skb, desc) == 0) {
 					/* skb now belongs to timestamp buffer
 					 * and will be removed later
 					 */
-- 
2.17.1

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Paul Thomas]

Oh I notice that this is in the xilnx github repo so if it makes it in
from there that works too.
-Paul

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Harini Katakam]

Hi Paul,

On Tue, Mar 19, 2019 at 11:13 PM Paul Thomas <pthomas8589@gmail.com> wrote:
>
> Oh I notice that this is in the xilnx github repo so if it makes it in
> from there that works too.

No, I don't plan to send anything.
I just used your patch (retained your signed-off, of course) and
added the && check, because I wanted to pull in into our repo quickly.
Thanks

Regards,
Harini

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[David Miller]

Paul, please format your patch properly.

The Subject line should be of the form:

	[PATCH vX] $subsystem-prefix: Description.

Also please provide an accurate and proper Fixes: tag.

Thank you.

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Paul Thomas]

On Tue, Mar 19, 2019 at 1:07 PM Paul Thomas <pthomas8589@gmail.com> wrote:
>
> Make sure SKBTX_HW_TSTAMP (i.e. SOF_TIMESTAMPING_TX_HARDWARE) has been
> enabled for this skb It does fix the issue where normal socks that
> aren't expecting a timestamp will not wake up on select. And when a
> user does want a SOF_TIMESTAMPING_TX_HARDWARE this does work.
>
> Signed-off-by: Paul Thomas <pthomas8589@gmail.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index ad099fd01b45..48fc45ab514f 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -898,7 +898,8 @@ static void macb_tx_interrupt(struct macb_queue *queue)
>
>                         /* First, update TX stats if needed */
>                         if (skb) {
> -                               if (gem_ptp_do_txstamp(queue, skb, desc) == 0) {
> +                               if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
> +                                       gem_ptp_do_txstamp(queue, skb, desc) == 0) {
>                                         /* skb now belongs to timestamp buffer
>                                          * and will be removed later
>                                          */
> --
> 2.17.1
>

Hi All, just a reminder on this.

This is a bug fix that I'm sure others will hit at some point, I
imagine that it applies to the Microchip parts as well.

A quick recap on how this shows up. Without this and with HW
timestamps enabled ALL socks get notified through select() that the
timestamp is available (on the error queue). This causes applications
like ssh to break.

thanks,
Paul

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Harini Katakam]

Hi Paul,

On Thu, Apr 4, 2019 at 6:35 PM Paul Thomas <pthomas8589@gmail.com> wrote:
>
> On Tue, Mar 19, 2019 at 1:07 PM Paul Thomas <pthomas8589@gmail.com> wrote:
> >
<snip>
> Hi All, just a reminder on this.

Could you please resend the patch addressing Dave's review comments
about subject line and Fixes tag?

Regards,
Harini

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Paul Thomas]

> Could you please resend the patch addressing Dave's review comments
> about subject line and Fixes tag?
Yup, sorry about that. I'm not subscribed to netdev, but I thought
replies would have me in the to:line, but I see the comments on the
archive. I'll send this out shortly.

-Paul

Re: [PATCH] v2 Check for SKBTX_HW_TSTAMP in macb driver

[Paul Thomas]

On Thu, Apr 4, 2019 at 9:23 AM Paul Thomas <pthomas8589@gmail.com> wrote:
>
> > Could you please resend the patch addressing Dave's review comments
> > about subject line and Fixes tag?
> Yup, sorry about that. I'm not subscribed to netdev, but I thought
> replies would have me in the to:line, but I see the comments on the
> archive. I'll send this out shortly.
>
> -Paul

Quick question, what should the Fixes: tag be? On the submitting
patches page it says to use this "If your patch fixes a bug in a
specific commit", but I think this has been there since PTP support
was added:
commit: ab91f0a9b5f4b9b5b341fdc0ed457121e69c20e1
Is that would should be referenced?

thanks,
Paul