On 9/30/24 22:11, Sergey Shtylyov wrote: [...] >> From: Paul Barker <paul.barker.ct@xxxxxxxxxxxxxx> >> >> The HW checksum value is used as a 16-bit flag, it is zero when the > > I think I prefer s/HW/hardware/ but there's no hard feelings... :-) > >> checksum has been validated and non-zero otherwise. Therefore we don't >> need to treat this as an actual __wsum type or call csum_unfold(), we >> can just use a u16 pointer. >> >> Signed-off-by: Paul Barker <paul.barker.ct@xxxxxxxxxxxxxx> > [...] >> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c >> index 1dd2152734b0..9350ca10ab22 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] >> @@ -762,23 +761,22 @@ static void ravb_rx_csum_gbeth(struct sk_buff *skb) [...] >> + if (unlikely(skb->len < csum_len)) >> return; >> >> if (skb_is_nonlinear(skb)) { >> - last_frag = &shinfo->frags[shinfo->nr_frags - 1]; >> - hw_csum = skb_frag_address(last_frag) + >> - skb_frag_size(last_frag); >> - skb_frag_size_sub(last_frag, 2 * sizeof(__sum16)); >> + skb_frag_t *last_frag = &shinfo->frags[shinfo->nr_frags - 1]; > > Could've been done in the previous patch... Even fit better there, I think... >> + >> + hw_csum = (u16 *)(skb_frag_address(last_frag) + >> + skb_frag_size(last_frag)); >> + skb_frag_size_sub(last_frag, csum_len); >> } else { >> - hw_csum = skb_tail_pointer(skb); >> - skb_trim(skb, skb->len - 2 * sizeof(__sum16)); >> + hw_csum = (u16 *)skb_tail_pointer(skb); >> + skb_trim(skb, skb->len - csum_len); >> } >> >> - hw_csum -= sizeof(__sum16); >> - csum_proto = csum_unfold((__force __sum16)get_unaligned_le16(hw_csum)); >> - >> - if (!csum_proto) >> + if (!*--hw_csum) > > Hm, you lost get_unaligned_le16() here. The checksum can be anywhere, > unaligned too... No need to keep using get_unaligned_le16() itself but you should then switch to using get_unaligned(). [...] MBR, Sergey