| 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: <4c82705d-b05c-4ee8-88ed-42f944a023ac@kernel.org>
Date: Mon, 19 Feb 2024 12:59:55 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Chintan Vankar <c-vankar@...com>, Dan Carpenter
<dan.carpenter@...aro.org>, Siddharth Vadapalli <s-vadapalli@...com>,
Richard Cochran <richardcochran@...il.com>, Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>,
"David S. Miller" <davem@...emloft.net>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
Pekka Varis <p-varis@...com>
Subject: Re: [PATCH net-next 2/2] net: ethernet: ti: am65-cpsw: Enable RX HW
timestamp only for PTP packets
Hi,
On 15/02/2024 13:09, Chintan Vankar wrote:
> The CPSW peripherals on J7AHP, J7VCL, J7AEP, J7ES, AM64 SoCs have
> an errata i2401 "CPSW: Host Timestamps Cause CPSW Port to Lock up".
Does this affect all platforms using am65-cpsw?
If this affects only some platforms then you need to restrict the
workaround to those platforms only.
>
> As a workaround, Disable timestamping on all RX packets and timestamp
> only PTP packets.
This does not exactly reflect what you are doing here.
I think you need to clarify that you are disabling in-band RX timestamp
mechanism on all packets and using out of band mechanism to get the
RX timestamps only for certain type of packets.
>
> Set Time Sync Receive bits in Time Sync control register so that
> packets can be determined as a valid Ethernet Receive Event for time
> synchronization.
>
> Update the RX filter configuration to indicate Hardware Timestamping
> support only for PTP packets.
>
> Replace "am65_cpsw_rx_ts()" function with "am65_cpts_rx_timestamp()"
> function which timestamps only PTP packets.
>
> Fixes: b1f66a5bee07 ("net: ethernet: ti: am65-cpsw-nuss: enable packet timestamping support")
> Signed-off-by: Chintan Vankar <c-vankar@...com>
> ---
> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 51 +++++++++++-------------
> 1 file changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 9d2f4ac783e4..ab843fb64b93 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -101,6 +101,12 @@
> #define AM65_CPSW_PN_TS_CTL_TX_HOST_TS_EN BIT(11)
> #define AM65_CPSW_PN_TS_CTL_MSG_TYPE_EN_SHIFT 16
>
> +#define AM65_CPSW_PN_TS_CTL_RX_ANX_F_EN BIT(0)
> +#define AM65_CPSW_PN_TS_CTL_RX_VLAN_LT1_EN BIT(1)
> +#define AM65_CPSW_PN_TS_CTL_RX_VLAN_LT2_EN BIT(2)
> +#define AM65_CPSW_PN_TS_CTL_RX_ANX_D_EN BIT(3)
> +#define AM65_CPSW_PN_TS_CTL_RX_ANX_E_EN BIT(9)
> +
> /* AM65_CPSW_PORTN_REG_TS_SEQ_LTYPE_REG register fields */
> #define AM65_CPSW_PN_TS_SEQ_ID_OFFSET_SHIFT 16
>
> @@ -124,6 +130,11 @@
> AM65_CPSW_PN_TS_CTL_TX_ANX_E_EN | \
> AM65_CPSW_PN_TS_CTL_TX_ANX_F_EN)
>
> +#define AM65_CPSW_TS_RX_ANX_ALL_EN \
> + (AM65_CPSW_PN_TS_CTL_RX_ANX_D_EN | \
> + AM65_CPSW_PN_TS_CTL_RX_ANX_E_EN | \
> + AM65_CPSW_PN_TS_CTL_RX_ANX_F_EN)
> +
> #define AM65_CPSW_ALE_AGEOUT_DEFAULT 30
> /* Number of TX/RX descriptors */
> #define AM65_CPSW_MAX_TX_DESC 500
> @@ -749,18 +760,6 @@ static int am65_cpsw_nuss_ndo_slave_open(struct net_device *ndev)
> return ret;
> }
>
> -static void am65_cpsw_nuss_rx_ts(struct sk_buff *skb, u32 *psdata)
> -{
> - struct skb_shared_hwtstamps *ssh;
> - u64 ns;
> -
> - ns = ((u64)psdata[1] << 32) | psdata[0];
> -
> - ssh = skb_hwtstamps(skb);
> - memset(ssh, 0, sizeof(*ssh));
> - ssh->hwtstamp = ns_to_ktime(ns);
> -}
> -
> /* RX psdata[2] word format - checksum information */
> #define AM65_CPSW_RX_PSD_CSUM_ADD GENMASK(15, 0)
> #define AM65_CPSW_RX_PSD_CSUM_ERR BIT(16)
> @@ -841,9 +840,6 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common *common,
> skb->dev = ndev;
>
> psdata = cppi5_hdesc_get_psdata(desc_rx);
> - /* add RX timestamp */
> - if (port->rx_ts_enabled)
> - am65_cpsw_nuss_rx_ts(skb, psdata);
> csum_info = psdata[2];
> dev_dbg(dev, "%s rx csum_info:%#x\n", __func__, csum_info);
>
> @@ -856,6 +852,9 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common *common,
> ndev_priv = netdev_priv(ndev);
> am65_cpsw_nuss_set_offload_fwd_mark(skb, ndev_priv->offload_fwd_mark);
> skb_put(skb, pkt_len);
> + skb_reset_mac_header(skb);
Why do you need to add skb_reset_mac_header here?
> + if (port->rx_ts_enabled)
> + am65_cpts_rx_timestamp(common->cpts, skb);
The timestamp should be added before you do skb_put().
> skb->protocol = eth_type_trans(skb, ndev);
> am65_cpsw_nuss_rx_csum(skb, csum_info);
> napi_gro_receive(&common->napi_rx, skb);
> @@ -1334,7 +1333,6 @@ static int am65_cpsw_nuss_ndo_slave_set_mac_address(struct net_device *ndev,
> static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
> struct ifreq *ifr)
> {
> - struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
> struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
> u32 ts_ctrl, seq_id, ts_ctrl_ltype2, ts_vlan_ltype;
> struct hwtstamp_config cfg;
> @@ -1358,11 +1356,6 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
> case HWTSTAMP_FILTER_NONE:
> port->rx_ts_enabled = false;
> break;
> - case HWTSTAMP_FILTER_ALL:
> - case HWTSTAMP_FILTER_SOME:
> - case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> - case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> - case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
Can't PTP_V1 be supported?
> case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> @@ -1372,10 +1365,13 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
> case HWTSTAMP_FILTER_PTP_V2_EVENT:
> case HWTSTAMP_FILTER_PTP_V2_SYNC:
> case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> - case HWTSTAMP_FILTER_NTP_ALL:
> port->rx_ts_enabled = true;
> - cfg.rx_filter = HWTSTAMP_FILTER_ALL;
> + cfg.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> break;
> + case HWTSTAMP_FILTER_ALL:
> + case HWTSTAMP_FILTER_SOME:
> + case HWTSTAMP_FILTER_NTP_ALL:
> + return -EOPNOTSUPP;
> default:
> return -ERANGE;
> }
> @@ -1405,6 +1401,10 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
> ts_ctrl |= AM65_CPSW_TS_TX_ANX_ALL_EN |
> AM65_CPSW_PN_TS_CTL_TX_VLAN_LT1_EN;
>
> + if (port->rx_ts_enabled)
> + ts_ctrl |= AM65_CPSW_TS_RX_ANX_ALL_EN |
> + AM65_CPSW_PN_TS_CTL_RX_VLAN_LT1_EN;
> +
> writel(seq_id, port->port_base + AM65_CPSW_PORTN_REG_TS_SEQ_LTYPE_REG);
> writel(ts_vlan_ltype, port->port_base +
> AM65_CPSW_PORTN_REG_TS_VLAN_LTYPE_REG);
> @@ -1412,9 +1412,6 @@ static int am65_cpsw_nuss_hwtstamp_set(struct net_device *ndev,
> AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2);
> writel(ts_ctrl, port->port_base + AM65_CPSW_PORTN_REG_TS_CTL);
>
> - /* en/dis RX timestamp */
> - am65_cpts_rx_enable(common->cpts, port->rx_ts_enabled);
> -
> return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
> }
>
> @@ -1431,7 +1428,7 @@ static int am65_cpsw_nuss_hwtstamp_get(struct net_device *ndev,
> cfg.tx_type = port->tx_ts_enabled ?
> HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
> cfg.rx_filter = port->rx_ts_enabled ?
> - HWTSTAMP_FILTER_ALL : HWTSTAMP_FILTER_NONE;
> + HWTSTAMP_FILTER_PTP_V2_EVENT : HWTSTAMP_FILTER_NONE;
>
> return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
> }
--
cheers,
-roger
Powered by blists - more mailing lists