Thread (19 messages) 19 messages, 3 authors, 2023-01-09

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

From: <Arun.Ramadoss@microchip.com>
Date: 2023-01-09 04:31:46
Also in: lkml

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 [off-list ref]
wrote:
quoted
quoted
+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-
quoted
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. 
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help