| 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
| ||
|
Date: Thu, 05 Jan 2023 11:49:13 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Arun Ramadoss <arun.ramadoss@...rochip.com>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Cc: woojung.huh@...rochip.com, UNGLinuxDriver@...rochip.com,
andrew@...n.ch, vivien.didelot@...il.com, f.fainelli@...il.com,
olteanv@...il.com, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, linux@...linux.org.uk, Tristram.Ha@...rochip.com,
richardcochran@...il.com, ceggers@...i.de
Subject: Re: [Patch net-next v7 06/13] net: ptp: add helper for one-step P2P
clocks
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@...i.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@...i.de>
> > Co-developed-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@...rochip.com>
> > ---
> > v1 -> v2
> > - Fixed compilation issue when PTP_CLASSIFY not selected in menuconfig
> > as reported by kernel test robot <lkp@...el.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
Powered by blists - more mailing lists