Messages in this thread | | | Subject | Re: [Patch net-next v7 06/13] net: ptp: add helper for one-step P2P clocks | From | Paolo Abeni <> | Date | Thu, 05 Jan 2023 11:49:13 +0100 |
| |
On Thu, 2023-01-05 at 11:09 +0100, Paolo Abeni wrote: > Hi, > > On Wed, 2023-01-04 at 14:13 +0530, Arun Ramadoss wrote: > > From: Christian Eggers <ceggers@arri.de> > > > > For P2P delay measurement, the ingress time stamp of the PDelay_Req is > > required for the correction field of the PDelay_Resp. The application > > echoes back the correction field of the PDelay_Req when sending the > > PDelay_Resp. > > > > Some hardware (like the ZHAW InES PTP time stamping IP core) subtracts > > the ingress timestamp autonomously from the correction field, so that > > the hardware only needs to add the egress timestamp on tx. Other > > hardware (like the Microchip KSZ9563) reports the ingress time stamp via > > an interrupt and requires that the software provides this time stamp via > > tail-tag on tx. > > > > In order to avoid introducing a further application interface for this, > > the driver can simply emulate the behavior of the InES device and > > subtract the ingress time stamp in software from the correction field. > > > > On egress, the correction field can either be kept as it is (and the > > time stamp field in the tail-tag is set to zero) or move the value from > > the correction field back to the tail-tag. > > > > Changing the correction field requires updating the UDP checksum (if UDP > > is used as transport). > > > > Signed-off-by: Christian Eggers <ceggers@arri.de> > > Co-developed-by: Arun Ramadoss <arun.ramadoss@microchip.com> > > Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com> > > --- > > v1 -> v2 > > - Fixed compilation issue when PTP_CLASSIFY not selected in menuconfig > > as reported by kernel test robot <lkp@intel.com> > > --- > > include/linux/ptp_classify.h | 71 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 71 insertions(+) > > > > diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h > > index 2b6ea36ad162..6e5869c2504c 100644 > > --- a/include/linux/ptp_classify.h > > +++ b/include/linux/ptp_classify.h > > @@ -10,8 +10,12 @@ > > #ifndef _PTP_CLASSIFY_H_ > > #define _PTP_CLASSIFY_H_ > > > > +#include <asm/unaligned.h> > > #include <linux/ip.h> > > +#include <linux/ktime.h> > > #include <linux/skbuff.h> > > +#include <linux/udp.h> > > +#include <net/checksum.h> > > > > #define PTP_CLASS_NONE 0x00 /* not a PTP event message */ > > #define PTP_CLASS_V1 0x01 /* protocol version 1 */ > > @@ -129,6 +133,67 @@ static inline u8 ptp_get_msgtype(const struct ptp_header *hdr, > > return msgtype; > > } > > > > +/** > > + * ptp_check_diff8 - Computes new checksum (when altering a 64-bit field) > > + * @old: old field value > > + * @new: new field value > > + * @oldsum: previous checksum > > + * > > + * This function can be used to calculate a new checksum when only a single > > + * field is changed. Similar as ip_vs_check_diff*() in ip_vs.h. > > + * > > + * Return: Updated checksum > > + */ > > +static inline __wsum ptp_check_diff8(__be64 old, __be64 new, __wsum oldsum) > > +{ > > + __be64 diff[2] = { ~old, new }; > > + > > + return csum_partial(diff, sizeof(diff), oldsum); > > +} > > + > > +/** > > + * ptp_header_update_correction - Update PTP header's correction field > > + * @skb: packet buffer > > + * @type: type of the packet (see ptp_classify_raw()) > > + * @hdr: ptp header > > + * @correction: new correction value > > + * > > + * This updates the correction field of a PTP header and updates the UDP > > + * checksum (if UDP is used as transport). It is needed for hardware capable of > > + * one-step P2P that does not already modify the correction field of Pdelay_Req > > + * event messages on ingress. > > + */ > > +static inline > > +void ptp_header_update_correction(struct sk_buff *skb, unsigned int type, > > + struct ptp_header *hdr, s64 correction) > > +{ > > + __be64 correction_old; > > + struct udphdr *uhdr; > > + > > + /* previous correction value is required for checksum update. */ > > + memcpy(&correction_old, &hdr->correction, sizeof(correction_old)); > > + > > + /* write new correction value */ > > + put_unaligned_be64((u64)correction, &hdr->correction); > > + > > + switch (type & PTP_CLASS_PMASK) { > > + case PTP_CLASS_IPV4: > > + case PTP_CLASS_IPV6: > > + /* locate udp header */ > > + uhdr = (struct udphdr *)((char *)hdr - sizeof(struct udphdr)); > > + break; > > + default: > > + return; > > + } > > + > > + /* update checksum */ > > + uhdr->check = csum_fold(ptp_check_diff8(correction_old, > > + hdr->correction, > > + ~csum_unfold(uhdr->check))); > > + if (!uhdr->check) > > + uhdr->check = CSUM_MANGLED_0; > > AFAICS the above works under the assumption that skb->ip_summed != > CHECKSUM_COMPLETE, and such assumption is true for the existing DSA > devices. > > Still the new helper is a generic one, so perhaps it should take care > of CHECKSUM_COMPLETE, too? Or at least add a big fat warning in the > helper documentation and/or a warn_on_once(CHECKSUM_COMPLETE).
I see this helper is used later even in the tx path, so even packet with ip_summed == CHECKSUM_PARTIAL could reach here and should be accomodated accordingly.
Thanks,
Paolo
| |