linux-kernel - Re: [PATCH v2 net-next 13/14] net: enetc: don't update sync packet checksum if checksum offload is used

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aHgThBhKSqlLnX0D@lizhi-Precision-Tower-5810>
Date: Wed, 16 Jul 2025 17:03:00 -0400
From: Frank Li <Frank.li@....com>
To: Wei Fang <wei.fang@....com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	richardcochran@...il.com, claudiu.manoil@....com,
	vladimir.oltean@....com, xiaoning.wang@....com,
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, vadim.fedorenko@...ux.dev,
	shawnguo@...nel.org, s.hauer@...gutronix.de, festevam@...il.com,
	fushi.peng@....com, devicetree@...r.kernel.org,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	imx@...ts.linux.dev, kernel@...gutronix.de
Subject: Re: [PATCH v2 net-next 13/14] net: enetc: don't update sync packet
 checksum if checksum offload is used

On Wed, Jul 16, 2025 at 03:31:10PM +0800, Wei Fang wrote:
> For ENETC v4, the hardware has the capability to support Tx checksum
> offload. so the enetc driver does not need to update the UDP checksum
> of PTP sync packets if Tx checksum offload is enabled.
>
> Signed-off-by: Wei Fang <wei.fang@....com>

Reviewed-by: Frank Li <Frank.Li@....com>
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 6e04dd825a95..cf72d50246a9 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -247,7 +247,7 @@ static void enetc4_set_one_step_ts(struct enetc_si *si, bool udp, int offset)
>  }
>
>  static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
> -				     struct sk_buff *skb)
> +				     struct sk_buff *skb, bool csum_offload)
>  {
>  	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
>  	u16 tstamp_off = enetc_cb->origin_tstamp_off;
> @@ -269,18 +269,17 @@ static u32 enetc_update_ptp_sync_msg(struct enetc_ndev_priv *priv,
>  	 * - 48 bits seconds field
>  	 * - 32 bits nanseconds field
>  	 *
> -	 * In addition, the UDP checksum needs to be updated
> -	 * by software after updating originTimestamp field,
> -	 * otherwise the hardware will calculate the wrong
> -	 * checksum when updating the correction field and
> -	 * update it to the packet.
> +	 * In addition, if csum_offload is false, the UDP checksum needs
> +	 * to be updated by software after updating originTimestamp field,
> +	 * otherwise the hardware will calculate the wrong checksum when
> +	 * updating the correction field and update it to the packet.
>  	 */
>
>  	data = skb_mac_header(skb);
>  	new_sec_h = htons((sec >> 32) & 0xffff);
>  	new_sec_l = htonl(sec & 0xffffffff);
>  	new_nsec = htonl(nsec);
> -	if (enetc_cb->udp) {
> +	if (enetc_cb->udp && !csum_offload) {
>  		struct udphdr *uh = udp_hdr(skb);
>  		__be32 old_sec_l, old_nsec;
>  		__be16 old_sec_h;
> @@ -319,6 +318,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  	struct enetc_tx_swbd *tx_swbd;
>  	int len = skb_headlen(skb);
>  	union enetc_tx_bd temp_bd;
> +	bool csum_offload = false;
>  	union enetc_tx_bd *txbd;
>  	int i, count = 0;
>  	skb_frag_t *frag;
> @@ -345,6 +345,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
>  							    ENETC_TXBD_L4T_UDP);
>  			flags |= ENETC_TXBD_FLAGS_CSUM_LSO | ENETC_TXBD_FLAGS_L4CS;
> +			csum_offload = true;
>  		} else if (skb_checksum_help(skb)) {
>  			return 0;
>  		}
> @@ -352,7 +353,7 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>
>  	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
>  		do_onestep_tstamp = true;
> -		tstamp = enetc_update_ptp_sync_msg(priv, skb);
> +		tstamp = enetc_update_ptp_sync_msg(priv, skb, csum_offload);
>  	} else if (enetc_cb->flag & ENETC_F_TX_TSTAMP) {
>  		do_twostep_tstamp = true;
>  	}
> --
> 2.34.1
>