On 10/3/24 12:23, Paul Barker 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) >>> * The last 2 bytes are the protocol checksum status which will be zero >>> * if the checksum has been validated. >>> */ >>> - if (unlikely(skb->len < sizeof(__sum16) * 2)) >>> + csum_len = sizeof(*hw_csum) * 2; >> >> Could've been done by an initializer instead? > > So, if I move this to the initializers at the start of the function, > csum_len must be declared after hw_csum which breaks reverse Christmas > tree ordering: > > struct skb_shared_info *shinfo = skb_shinfo(skb); > u16 *hw_csum; > size_t csum_len = sizeof(*hw_csum) * 2; Could use sizeof(u16) instead, but it's OK to ignore me on this matter. :-) > Thanks, MBR, Sergey
