On Tue, Aug 04, 2020 at 11:56:12PM +0300, Grygorii Strashko wrote: > > > On 31/07/2020 13:06, Kurt Kanzenbach wrote: > > On Thu Jul 30 2020, Petr Machata wrote: > > > Kurt Kanzenbach <k...@linutronix.de> writes: > > > > > > > @@ -107,6 +107,37 @@ unsigned int ptp_classify_raw(const struct sk_buff > > > > *skb) > > > > } > > > > EXPORT_SYMBOL_GPL(ptp_classify_raw); > > > > +struct ptp_header *ptp_parse_header(struct sk_buff *skb, unsigned int > > > > type) > > > > +{ > > > > + u8 *data = skb_mac_header(skb); > > > > + u8 *ptr = data; > > > > > > One of the "data" and "ptr" variables is superfluous. > > > > Yeah. Can be shortened to u8 *ptr = skb_mac_header(skb); > > Actually usage of skb_mac_header(skb) breaks CPTS RX time-stamping on > am571x platform PATCH 6. > > The CPSW RX timestamp requested after full packet put in SKB, but > before calling eth_type_trans(). > > So, skb->data pints on Eth header, but skb_mac_header() return garbage. > > Below diff fixes it for me.
However, that's likely to break everyone else. For example, anyone calling this from the mii_timestamper rxtstamp() method, the skb will have been classified with the MAC header pushed and restored, so skb->data points at the network header. Your change means that ptp_parse_header() expects the MAC header to also be pushed. Is it possible to adjust CPTS? Looking at: drivers/net/ethernet/ti/cpsw.c... yes. drivers/net/ethernet/ti/cpsw_new.c... yes. drivers/net/ethernet/ti/netcp_core.c... unclear. If not, maybe cpts should remain unconverted - I don't see any reason to provide a generic function for one user. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!