netdev - Re: [Patch net-next v7 06/13] net: ptp: add helper for one-step P2P clocks

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] [day] [month] [year] [list]

Date:   Mon, 9 Jan 2023 04:12:25 +0000
From:   <Arun.Ramadoss@...rochip.com>
To:     <pabeni@...hat.com>, <edumazet@...gle.com>
CC:     <andrew@...n.ch>, <linux-kernel@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>, <vivien.didelot@...il.com>,
        <olteanv@...il.com>, <linux@...linux.org.uk>, <ceggers@...i.de>,
        <Tristram.Ha@...rochip.com>, <f.fainelli@...il.com>,
        <kuba@...nel.org>, <richardcochran@...il.com>,
        <netdev@...r.kernel.org>, <Woojung.Huh@...rochip.com>,
        <davem@...emloft.net>
Subject: Re: [Patch net-next v7 06/13] net: ptp: add helper for one-step P2P
 clocks

Hi Eric,
On Thu, 2023-01-05 at 12:27 +0100, Eric Dumazet wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Jan 5, 2023 at 11:09 AM Paolo Abeni <pabeni@...hat.com>
> wrote:
> > 
> > 
> > > +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.
> 
> Presumably skb->ip_summed could be forced to CHECKSUM_NONE
> 
> Note: if IPV4 UDP checksum is zero, we are not supposed to change it.
> 
> (Not sure if this point is already checked in caller)

This function is called only for the Pdelay_Req/Resp packet processing
from the hardware where correction field is updated & checksum is
recomputed. 
As per the recommendation, Can I set the skb->ip_summed = CHECKSUM_NONE
in the function after recomputing the checksum and resubmit the patch.
Kindly suggest. 

> 
> > 
> > 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).
> > 
> > Thanks!
> > 
> > Paolo
> >