Macb PTP updates to handle PTP one step sync support and other minor
enhancements.
Harini Katakam (3):
net: macb: Fix PTP one step sync support
net: macb: Enable PTP unicast
net: macb: Optimize reading HW timestamp
drivers/net/ethernet/cadence/macb.h | 4 ++
drivers/net/ethernet/cadence/macb_main.c | 61 +++++++++++++++++++-----
drivers/net/ethernet/cadence/macb_ptp.c | 12 +++--
3 files changed, 63 insertions(+), 14 deletions(-)
--
2.17.1
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")
Signed-off-by: Harini Katakam <[email protected]>
Reviewed-by: Radhey Shyam Pandey <[email protected]>
---
Notes:
-> Added the macb pad and fcs fixes tag because strictly speaking the PTP support
patch precedes the fcs patch in timeline.
-> FYI, the error observed with setting HW TX timestamp for one step sync packets:
ptp4l[405.292]: port 1: unexpected socket error
drivers/net/ethernet/cadence/macb_main.c | 54 ++++++++++++++++++++----
drivers/net/ethernet/cadence/macb_ptp.c | 4 +-
2 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e993616308f8..e23a03e8badf 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -37,6 +37,7 @@
#include <linux/phy/phy.h>
#include <linux/pm_runtime.h>
#include <linux/reset.h>
+#include <linux/ptp_classify.h>
#include "macb.h"
/* This structure is only used for MACB on SiFive FU540 devices */
@@ -98,6 +99,9 @@ struct sifive_fu540_macb_mgmt {
#define MACB_MDIO_TIMEOUT 1000000 /* in usecs */
+/* IEEE1588 PTP flag field values */
+#define PTP_FLAG_TWOSTEP 0x2
+
/* DMA buffer descriptor might be different size
* depends on hardware configuration:
*
@@ -1122,6 +1126,36 @@ static void macb_tx_error_task(struct work_struct *work)
napi_enable(&queue->napi_tx);
}
+static inline bool ptp_oss(struct sk_buff *skb)
+{
+ struct ptp_header *hdr;
+ unsigned int ptp_class;
+ u8 msgtype;
+
+ /* No need to parse packet if PTP TS is not involved */
+ if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
+ goto not_oss;
+
+ /* Identify and return whether PTP one step sync is being processed */
+ ptp_class = ptp_classify_raw(skb);
+ if (ptp_class == PTP_CLASS_NONE)
+ goto not_oss;
+
+ hdr = ptp_parse_header(skb, ptp_class);
+ if (!hdr)
+ goto not_oss;
+
+ if (hdr->flag_field[0] & PTP_FLAG_TWOSTEP)
+ goto not_oss;
+
+ msgtype = ptp_get_msgtype(hdr, ptp_class);
+ if (msgtype == PTP_MSGTYPE_SYNC)
+ return true;
+
+not_oss:
+ return false;
+}
+
static int macb_tx_complete(struct macb_queue *queue, int budget)
{
struct macb *bp = queue->bp;
@@ -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(skb)) {
+ if (gem_ptp_do_txstamp(queue, skb, desc) == 0) {
+ /* skb now belongs to timestamp buffer
+ * and will be removed later
+ */
+ tx_skb->skb = NULL;
+ }
}
netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
macb_tx_ring_wrap(bp, tail),
@@ -2063,7 +2098,7 @@ static unsigned int macb_tx_map(struct macb *bp,
ctrl |= MACB_BF(TX_LSO, lso_ctrl);
ctrl |= MACB_BF(TX_TCP_SEQ_SRC, seq_ctrl);
if ((bp->dev->features & NETIF_F_HW_CSUM) &&
- skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl)
+ skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl && !ptp_oss(skb))
ctrl |= MACB_BIT(TX_NOCRC);
} else
/* Only set MSS/MFS on payload descriptors
@@ -2159,9 +2194,10 @@ static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
struct sk_buff *nskb;
u32 fcs;
+ /* Not available for GSO and PTP one step sync */
if (!(ndev->features & NETIF_F_HW_CSUM) ||
!((*skb)->ip_summed != CHECKSUM_PARTIAL) ||
- skb_shinfo(*skb)->gso_size) /* Not available for GSO */
+ skb_shinfo(*skb)->gso_size || ptp_oss(*skb))
return 0;
if (padlen <= 0) {
diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
index fb6b27f46b15..9559c16078f9 100644
--- a/drivers/net/ethernet/cadence/macb_ptp.c
+++ b/drivers/net/ethernet/cadence/macb_ptp.c
@@ -470,8 +470,10 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
case HWTSTAMP_TX_ONESTEP_SYNC:
if (gem_ptp_set_one_step_sync(bp, 1) != 0)
return -ERANGE;
- fallthrough;
+ tx_bd_control = TSTAMP_ALL_FRAMES;
+ break;
case HWTSTAMP_TX_ON:
+ gem_ptp_set_one_step_sync(bp, 0);
tx_bd_control = TSTAMP_ALL_FRAMES;
break;
default:
--
2.17.1
On Tue, May 17, 2022 at 01:02:56PM +0530, Harini Katakam wrote:
> Macb PTP updates to handle PTP one step sync support and other minor
> enhancements.
>
> Harini Katakam (3):
> net: macb: Fix PTP one step sync support
> net: macb: Enable PTP unicast
> net: macb: Optimize reading HW timestamp
>
> drivers/net/ethernet/cadence/macb.h | 4 ++
> drivers/net/ethernet/cadence/macb_main.c | 61 +++++++++++++++++++-----
> drivers/net/ethernet/cadence/macb_ptp.c | 12 +++--
> 3 files changed, 63 insertions(+), 14 deletions(-)
For the series:
Acked-by: Richard Cochran <[email protected]>
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.
> Signed-off-by: Harini Katakam <[email protected]>
> Reviewed-by: Radhey Shyam Pandey <[email protected]>
If this is a fix it needs to be posted as [PATCH net] separately first
so we can get it into stable as well. The trees get merged together
every Thursday, if you're quick you may still make it this week.
Then after trees get merged you should send send the remaining patches.
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> Notes:
> -> Added the macb pad and fcs fixes tag because strictly speaking the PTP support
> patch precedes the fcs patch in timeline.
> -> FYI, the error observed with setting HW TX timestamp for one step sync packets:
> ptp4l[405.292]: port 1: unexpected socket error
>
> drivers/net/ethernet/cadence/macb_main.c | 54 ++++++++++++++++++++----
> drivers/net/ethernet/cadence/macb_ptp.c | 4 +-
> 2 files changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index e993616308f8..e23a03e8badf 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -37,6 +37,7 @@
> #include <linux/phy/phy.h>
> #include <linux/pm_runtime.h>
> #include <linux/reset.h>
> +#include <linux/ptp_classify.h>
Please keep the includes in alphabetical order.
> #include "macb.h"
>
> /* This structure is only used for MACB on SiFive FU540 devices */
> @@ -98,6 +99,9 @@ struct sifive_fu540_macb_mgmt {
>
> #define MACB_MDIO_TIMEOUT 1000000 /* in usecs */
>
> +/* IEEE1588 PTP flag field values */
> +#define PTP_FLAG_TWOSTEP 0x2
Shouldn't this go into the PTP header?
> /* DMA buffer descriptor might be different size
> * depends on hardware configuration:
> *
> @@ -1122,6 +1126,36 @@ static void macb_tx_error_task(struct work_struct *work)
> napi_enable(&queue->napi_tx);
> }
>
> +static inline bool ptp_oss(struct sk_buff *skb)
Please spell out then name more oss == open source software.
No inline here, please, let the compiler decide if the function is
small enough. One step timestamp may be a rare use case so inlining
this twice is not necessarily the right choice.
> +{
> + struct ptp_header *hdr;
> + unsigned int ptp_class;
> + u8 msgtype;
> +
> + /* No need to parse packet if PTP TS is not involved */
> + if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
> + goto not_oss;
> +
> + /* Identify and return whether PTP one step sync is being processed */
> + ptp_class = ptp_classify_raw(skb);
> + if (ptp_class == PTP_CLASS_NONE)
> + goto not_oss;
> +
> + hdr = ptp_parse_header(skb, ptp_class);
> + if (!hdr)
> + goto not_oss;
> +
> + if (hdr->flag_field[0] & PTP_FLAG_TWOSTEP)
> + goto not_oss;
> +
> + msgtype = ptp_get_msgtype(hdr, ptp_class);
> + if (msgtype == PTP_MSGTYPE_SYNC)
> + return true;
> +
> +not_oss:
> + return false;
> +}
> +
> static int macb_tx_complete(struct macb_queue *queue, int budget)
> {
> struct macb *bp = queue->bp;
> @@ -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.
> + !ptp_oss(skb)) {
> + if (gem_ptp_do_txstamp(queue, skb, desc) == 0) {
Why convert the gem_ptp_do_txstamp check from a && in the condition to
a separate if?
> + /* skb now belongs to timestamp buffer
> + * and will be removed later
> + */
> + tx_skb->skb = NULL;
> + }
> }
> netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
> macb_tx_ring_wrap(bp, tail),
> @@ -2063,7 +2098,7 @@ static unsigned int macb_tx_map(struct macb *bp,
> ctrl |= MACB_BF(TX_LSO, lso_ctrl);
> ctrl |= MACB_BF(TX_TCP_SEQ_SRC, seq_ctrl);
> if ((bp->dev->features & NETIF_F_HW_CSUM) &&
> - skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl)
> + skb->ip_summed != CHECKSUM_PARTIAL && !lso_ctrl && !ptp_oss(skb))
> ctrl |= MACB_BIT(TX_NOCRC);
> } else
> /* Only set MSS/MFS on payload descriptors
> @@ -2159,9 +2194,10 @@ static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
> struct sk_buff *nskb;
> u32 fcs;
>
> + /* Not available for GSO and PTP one step sync */
If the functions are named right this comment should not be needed.
> if (!(ndev->features & NETIF_F_HW_CSUM) ||
> !((*skb)->ip_summed != CHECKSUM_PARTIAL) ||
> - skb_shinfo(*skb)->gso_size) /* Not available for GSO */
> + skb_shinfo(*skb)->gso_size || ptp_oss(*skb))
> return 0;
>
> if (padlen <= 0) {
> diff --git a/drivers/net/ethernet/cadence/macb_ptp.c b/drivers/net/ethernet/cadence/macb_ptp.c
> index fb6b27f46b15..9559c16078f9 100644
> --- a/drivers/net/ethernet/cadence/macb_ptp.c
> +++ b/drivers/net/ethernet/cadence/macb_ptp.c
> @@ -470,8 +470,10 @@ int gem_set_hwtst(struct net_device *dev, struct ifreq *ifr, int cmd)
> case HWTSTAMP_TX_ONESTEP_SYNC:
> if (gem_ptp_set_one_step_sync(bp, 1) != 0)
> return -ERANGE;
> - fallthrough;
> + tx_bd_control = TSTAMP_ALL_FRAMES;
> + break;
> case HWTSTAMP_TX_ON:
> + gem_ptp_set_one_step_sync(bp, 0);
> tx_bd_control = TSTAMP_ALL_FRAMES;
> break;
> default:
Enable transmission and reception of PTP unicast packets by
updating PTP unicast config bit and setting current HW mac
address as allowed address in PTP unicast filter registers.
Signed-off-by: Harini Katakam <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Radhey Shyam Pandey <[email protected]>
---
drivers/net/ethernet/cadence/macb.h | 4 ++++
drivers/net/ethernet/cadence/macb_main.c | 7 +++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 7ca077b65eaa..d245fd78ec51 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -95,6 +95,8 @@
#define GEM_SA4B 0x00A0 /* Specific4 Bottom */
#define GEM_SA4T 0x00A4 /* Specific4 Top */
#define GEM_WOL 0x00b8 /* Wake on LAN */
+#define GEM_RXPTPUNI 0x00D4 /* PTP RX Unicast address */
+#define GEM_TXPTPUNI 0x00D8 /* PTP TX Unicast address */
#define GEM_EFTSH 0x00e8 /* PTP Event Frame Transmitted Seconds Register 47:32 */
#define GEM_EFRSH 0x00ec /* PTP Event Frame Received Seconds Register 47:32 */
#define GEM_PEFTSH 0x00f0 /* PTP Peer Event Frame Transmitted Seconds Register 47:32 */
@@ -245,6 +247,8 @@
#define MACB_TZQ_OFFSET 12 /* Transmit zero quantum pause frame */
#define MACB_TZQ_SIZE 1
#define MACB_SRTSM_OFFSET 15 /* Store Receive Timestamp to Memory */
+#define MACB_PTPUNI_OFFSET 20 /* PTP Unicast packet enable */
+#define MACB_PTPUNI_SIZE 1
#define MACB_OSSMODE_OFFSET 24 /* Enable One Step Synchro Mode */
#define MACB_OSSMODE_SIZE 1
#define MACB_MIIONRGMII_OFFSET 28 /* MII Usage on RGMII Interface */
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e23a03e8badf..19276583811e 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -290,6 +290,9 @@ static void macb_set_hwaddr(struct macb *bp)
top = cpu_to_le16(*((u16 *)(bp->dev->dev_addr + 4)));
macb_or_gem_writel(bp, SA1T, top);
+ gem_writel(bp, RXPTPUNI, bottom);
+ gem_writel(bp, TXPTPUNI, bottom);
+
/* Clear unused address register sets */
macb_or_gem_writel(bp, SA2B, 0);
macb_or_gem_writel(bp, SA2T, 0);
@@ -723,8 +726,8 @@ static void macb_mac_link_up(struct phylink_config *config,
spin_unlock_irqrestore(&bp->lock, flags);
- /* Enable Rx and Tx */
- macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));
+ /* Enable Rx and Tx; Enable PTP unicast */
+ macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE) | MACB_BIT(PTPUNI));
netif_tx_wake_all_queues(ndev);
}
--
2.17.1
Hi Jakub,
On Wed, May 18, 2022 at 8:12 AM Jakub Kicinski <[email protected]> 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 <[email protected]> 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 <[email protected]> also doesn't work)
<snip>
> > +/* IEEE1588 PTP flag field values */
> > +#define PTP_FLAG_TWOSTEP 0x2
>
> Shouldn't this go into the PTP header?
Let me add it to ptp_classify where the relevant helpers are present.
<snip>
> > +static inline bool ptp_oss(struct sk_buff *skb)
>
> Please spell out then name more oss == open source software.
Will change to ptp_one_step_sync
>
> No inline here, please, let the compiler decide if the function is
> small enough. One step timestamp may be a rare use case so inlining
> this twice is not necessarily the right choice.
One step is a rare case but the check happens on every PTP packet in the
transmit data path and hence I wanted to explicitly inline it.
<snip>
> > @@ -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.
>
> > + !ptp_oss(skb)) {
> > + if (gem_ptp_do_txstamp(queue, skb, desc) == 0) {
>
> Why convert the gem_ptp_do_txstamp check from a && in the condition to
> a separate if?
The intention is that ptp_oss should only be evaluated when
SKBTX_HW_TSTAMP is set and
gem_ptp_do_txstamp should only be called if ptp_oss is false. Since
compiler follows the order
of evaluation, I'll simplify this to:
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) && !ptp_oss(skb) &&
gem_ptp_do_txstamp(queue, skb, desc) == 0) {
...
}
Regards,
Harini
On Wed, 18 May 2022 09:53:29 +0530 Harini Katakam wrote:
> On Wed, May 18, 2022 at 8:12 AM Jakub Kicinski <[email protected]> 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 <[email protected]> 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 <[email protected]> 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?
Hi Jakub,
<snip>
>
> > > > @@ -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?
Yes, I checked the disassembly and this is what's happening. This
should be good for
the non-PTP packet (going to "likely" branch) and the rest of ptp_oss
is evaluated for
PTP packets.
Regards,
Harini