Hi Paolo,
Thanks for the review comment.
On Thu, 2023-01-05 at 11:49 +0100, Paolo Abeni wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you
know the content is safe

quoted

quoted

+/**
+ * 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-

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.

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.

Do I need to update the checksum only if ip_sum is not equal to
CHECKSUM_COMPLETE or CHECKSUM_PARTIAL.

if ( skb->ip_summed == CHECKSUM_COMPLETE ||
     skb->ip_summed == CHECKSUM_PARTIAL) {
	warn_on_once(1);
	return;
}

Kindly suggest.

Thanks,

Paolo