| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e22c53fb-35aa-356f-3a2d-65042dd298dd@microchip.com>
Date: Thu, 19 May 2022 08:27:22 +0000
From: <Claudiu.Beznea@...rochip.com>
To: <harini.katakam@...inx.com>, <Nicolas.Ferre@...rochip.com>,
<davem@...emloft.net>, <richardcochran@...il.com>,
<kuba@...nel.org>, <edumazet@...gle.com>, <pabeni@...hat.com>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<michal.simek@...inx.com>, <harinikatakamlinux@...il.com>,
<radhey.shyam.pandey@...inx.com>
Subject: Re: [PATCH net v3] net: macb: Fix PTP one step sync support
On 18.05.2022 20:07, Harini Katakam wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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. Add a common mask for PTP header flag field "twoStepflag".
> 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 <harini.katakam@...inx.com>
> Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@...inx.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea@...rochip.com>
> ---
> v3:
> - Rebase on net branch
> - Squash both commits on macb driver and ptp_classify.h
>
> v2:
> - Separate fix for net branch
> (Split from "Macb PTP updates" series
> https://lore.kernel.org/netdev/20220517135525.GC3344@hoboy.vegasvil.org/T/)
> - Fix include order
> - ptp_oss -> ptp_one_step_sync
> - Remove inline and add "likely" on SKB_HWTSTAMP check in ptp helper
> - Dont split gem_ptp_do_tx_tstamp from if condition as order of
> evaluation will take care of intent
> - Remove redundant comments in macb_pad_and_fcs
> - Add PTP flag to ptp_classify header for common use
> (Dint add Richard's ACK as the patch changed and there's a minor addition
> in ptp_classify.h as well)
>
> v1 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 | 40 +++++++++++++++++++++---
> drivers/net/ethernet/cadence/macb_ptp.c | 4 ++-
> include/linux/ptp_classify.h | 3 ++
> 3 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 61284baa0496..3a1b5ac48ca5 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -36,6 +36,7 @@
> #include <linux/iopoll.h>
> #include <linux/phy/phy.h>
> #include <linux/pm_runtime.h>
> +#include <linux/ptp_classify.h>
> #include <linux/reset.h>
> #include "macb.h"
>
> @@ -1124,6 +1125,36 @@ static void macb_tx_error_task(struct work_struct *work)
> spin_unlock_irqrestore(&bp->lock, flags);
> }
>
> +static bool ptp_one_step_sync(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 (likely(!(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 void macb_tx_interrupt(struct macb_queue *queue)
> {
> unsigned int tail;
> @@ -1168,8 +1199,8 @@ static void macb_tx_interrupt(struct macb_queue *queue)
>
> /* First, update TX stats if needed */
> if (skb) {
> - if (unlikely(skb_shinfo(skb)->tx_flags &
> - SKBTX_HW_TSTAMP) &&
> + if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
> + !ptp_one_step_sync(skb) &&
> gem_ptp_do_txstamp(queue, skb, desc) == 0) {
> /* skb now belongs to timestamp buffer
> * and will be removed later
> @@ -1999,7 +2030,8 @@ 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_one_step_sync(skb))
> ctrl |= MACB_BIT(TX_NOCRC);
> } else
> /* Only set MSS/MFS on payload descriptors
> @@ -2097,7 +2129,7 @@ static int macb_pad_and_fcs(struct sk_buff **skb, struct net_device *ndev)
>
> 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_one_step_sync(*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:
> diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
> index fefa7790dc46..2b6ea36ad162 100644
> --- a/include/linux/ptp_classify.h
> +++ b/include/linux/ptp_classify.h
> @@ -43,6 +43,9 @@
> #define OFF_PTP_SOURCE_UUID 22 /* PTPv1 only */
> #define OFF_PTP_SEQUENCE_ID 30
>
> +/* PTP header flag fields */
> +#define PTP_FLAG_TWOSTEP BIT(1)
> +
> /* Below defines should actually be removed at some point in time. */
> #define IP6_HLEN 40
> #define UDP_HLEN 8
> --
> 2.17.1
>
Powered by blists - more mailing lists