Messages in this thread | | | Date | Tue, 17 May 2022 22:06:03 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH 1/3] net: macb: Fix PTP one step sync support |
| |
On Wed, 18 May 2022 09:53:29 +0530 Harini Katakam wrote: > On Wed, May 18, 2022 at 8:12 AM Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Tue, 17 May 2022 13:02:57 +0530 Harini Katakam wrote: > > > PTP one step sync packets cannot have CSUM padding and insertion in > > > SW since time stamp is inserted on the fly by HW. > > > In addition, ptp4l version 3.0 and above report an error when skb > > > timestamps are reported for packets that not processed for TX TS > > > after transmission. > > > Add a helper to identify PTP one step sync and fix the above two > > > errors. > > > Also reset ptp OSS bit when one step is not selected. > > > > > > Fixes: ab91f0a9b5f4 ("net: macb: Add hardware PTP support") > > > Fixes: 653e92a9175e ("net: macb: add support for padding and fcs computation") > > > > Please make sure to CC authors of the patches under fixes. > > ./scripts/get_maintainer should point them out. > > Thanks for the review. > Rafal Ozieblo <rafalo@cadence.com> is the author of the first Fixes > patch but that > address hasn't worked in the last ~4 yrs. > I have cced Claudiu and everyone else from the maintainers > (Eric Dumazet <edumazet@google.com> also doesn't work)
I see, thanks, added Rafal's email to the ignore list, I'm quite sure Eric's email address works.
> > > @@ -1158,13 +1192,14 @@ static int macb_tx_complete(struct macb_queue *queue, int budget) > > > > > > /* First, update TX stats if needed */ > > > if (skb) { > > > - 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 > > > - */ > > > - tx_skb->skb = NULL; > > > + if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) && > > > > ptp_oss already checks if HW_TSTAMP is set. > > The check for SKBTX_HW_TSTAMP is required here universally and not > just inside ptp_oss. > I will remove the redundant check in ptp_oss instead. Please see the > reply below.
But then you need to add this check in the padding/fcs call site and the place where NOCRC is set. If you wrap the check for SKBTX_HW_TSTAMP in the helper with likely() and remove the inline - will the compiler not split the function and inline just that check? And leave the rest as a functionname.part... thing?
| |