On Mon, Jul 13, 2020 at 12:57:34PM +0200, Kurt Kanzenbach wrote: > Hi Vladimir, > > On Mon Jul 13 2020, Vladimir Oltean wrote: > >> +/* Get a pointer to the PTP header in this skb */ > >> +static u8 *parse_ptp_header(struct sk_buff *skb, unsigned int type) > > > > Maybe this and the function from mv88e6xxx could share the same > > implementation somehow. > > Actually both functions are identical. Should it be moved to the ptp > core, maybe? Then, all drivers could use that. I guess we should also > define a PTP offset for the reserved field which is accessed in > hellcreek_get_reserved_field() just with 16 instead of a proper macro > constant. I support re-factoring the code that parses the PTP header. Last time I looked, each driver needed slightly different fields, and I didn't see an easy way to accommodate them all. > > I would like to get some clarification on whether "SKBTX_IN_PROGRESS" > > should be set in shtx->tx_flags or not. On one hand, it's asking for > > trouble, on the other hand, it's kind of required for proper compliance > > to API pre-SO_TIMESTAMPING... > > Hm. We actually oriented our code on the mv88e6xxx time stamping code base. Where in mv88e6xxx does the driver set SKBTX_IN_PROGRESS? I don't think it makes sense for DSA drivers to set this bit, as it serves no purpose in the DSA context. Thanks, Richard