netdev - RE: [PATCH net-next 08/12] net: enetc: save the parsed information of PTP packet to skb->cb

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:
 <PAXPR04MB85101AA274D7B11B3E31EC078854A@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Mon, 14 Jul 2025 03:07:25 +0000
From: Wei Fang <wei.fang@....com>
To: Vadim Fedorenko <vfedorenko@...ek.ru>
CC: "F.S. Peng" <fushi.peng@....com>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"richardcochran@...il.com" <richardcochran@...il.com>, Claudiu Manoil
	<claudiu.manoil@....com>, Vladimir Oltean <vladimir.oltean@....com>, Clark
 Wang <xiaoning.wang@....com>, "andrew+netdev@...n.ch"
	<andrew+netdev@...n.ch>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>
Subject: RE: [PATCH net-next 08/12] net: enetc: save the parsed information of
 PTP packet to skb->cb

> > @@ -225,13 +225,12 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >   {
> >   	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
> >   	struct enetc_ndev_priv *priv = netdev_priv(tx_ring->ndev);
> > +	struct enetc_skb_cb *enetc_cb = ENETC_SKB_CB(skb);
> >   	struct enetc_hw *hw = &priv->si->hw;
> >   	struct enetc_tx_swbd *tx_swbd;
> >   	int len = skb_headlen(skb);
> >   	union enetc_tx_bd temp_bd;
> > -	u8 msgtype, twostep, udp;
> >   	union enetc_tx_bd *txbd;
> > -	u16 offset1, offset2;
> >   	int i, count = 0;
> >   	skb_frag_t *frag;
> >   	unsigned int f;
> > @@ -280,16 +279,10 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >   	count++;
> >
> >   	do_vlan = skb_vlan_tag_present(skb);
> > -	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> > -		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep, &offset1,
> > -				    &offset2) ||
> > -		    msgtype != PTP_MSGTYPE_SYNC || twostep)
> > -			WARN_ONCE(1, "Bad packet for one-step timestamping\n");
> > -		else
> > -			do_onestep_tstamp = true;
> > -	} else if (skb->cb[0] & ENETC_F_TX_TSTAMP) {
> > +	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP)
> > +		do_onestep_tstamp = true;
> > +	else if (enetc_cb->flag & ENETC_F_TX_TSTAMP)
> >   		do_twostep_tstamp = true;
> > -	}
> >
> >   	tx_swbd->do_twostep_tstamp = do_twostep_tstamp;
> >   	tx_swbd->qbv_en = !!(priv->active_offloads & ENETC_F_QBV); @@
> > -333,6 +326,8 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring,
> struct sk_buff *skb)
> >   		}
> >
> >   		if (do_onestep_tstamp) {
> > +			u16 tstamp_off = enetc_cb->origin_tstamp_off;
> > +			u16 corr_off = enetc_cb->correction_off;
> >   			__be32 new_sec_l, new_nsec;
> >   			u32 lo, hi, nsec, val;
> >   			__be16 new_sec_h;
> > @@ -362,32 +357,32 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >   			new_sec_h = htons((sec >> 32) & 0xffff);
> >   			new_sec_l = htonl(sec & 0xffffffff);
> >   			new_nsec = htonl(nsec);
> > -			if (udp) {
> > +			if (enetc_cb->udp) {
> >   				struct udphdr *uh = udp_hdr(skb);
> >   				__be32 old_sec_l, old_nsec;
> >   				__be16 old_sec_h;
> >
> > -				old_sec_h = *(__be16 *)(data + offset2);
> > +				old_sec_h = *(__be16 *)(data + tstamp_off);
> >   				inet_proto_csum_replace2(&uh->check, skb, old_sec_h,
> >   							 new_sec_h, false);
> >
> > -				old_sec_l = *(__be32 *)(data + offset2 + 2);
> > +				old_sec_l = *(__be32 *)(data + tstamp_off + 2);
> >   				inet_proto_csum_replace4(&uh->check, skb, old_sec_l,
> >   							 new_sec_l, false);
> >
> > -				old_nsec = *(__be32 *)(data + offset2 + 6);
> > +				old_nsec = *(__be32 *)(data + tstamp_off + 6);
> >   				inet_proto_csum_replace4(&uh->check, skb, old_nsec,
> >   							 new_nsec, false);
> >   			}
> >
> > -			*(__be16 *)(data + offset2) = new_sec_h;
> > -			*(__be32 *)(data + offset2 + 2) = new_sec_l;
> > -			*(__be32 *)(data + offset2 + 6) = new_nsec;
> > +			*(__be16 *)(data + tstamp_off) = new_sec_h;
> > ++			*(__be32 *)(data + tstamp_off + 2) = new_sec_l;
> > ++			*(__be32 *)(data + tstamp_off + 6) = new_nsec;
> 
> This looks like merge conflict artifact...

No, I have changed the variable names (see below) , which are more easier to
understand what the variables mean. I should add a description to the commit
message to state this change.

-	u16 offset1, offset2;
+			u16 tstamp_off = enetc_cb->origin_tstamp_off;
+			u16 corr_off = enetc_cb->correction_off;

> 
> >
> >   			/* Configure single-step register */
> >   			val = ENETC_PM0_SINGLE_STEP_EN;
> > -			val |= ENETC_SET_SINGLE_STEP_OFFSET(offset1);
> > -			if (udp)
> > +			val |= ENETC_SET_SINGLE_STEP_OFFSET(corr_off);
> > +			if (enetc_cb->udp)
> >   				val |= ENETC_PM0_SINGLE_STEP_CH;
> >
> >   			enetc_port_mac_wr(priv->si, ENETC_PM0_SINGLE_STEP, @@