Re: [PATCH net-next 2/6] netdevsim: psp: remove unnecessary UDP checksum computation

[Willem de Bruijn]

Daniel Zahka wrote:
> 
> On 5/11/26 1:01 PM, Willem de Bruijn wrote:
> > Daniel Zahka wrote:
> >> The PSP spec requires the implementations accept 0 checksum in psp-udp
> >> header. Let's take advantage of that to trim netdevsim's psp code
> >> down. psp_dev_encapsulate() already sets uh->check to 0.
> >>
> >> Assisted-by: Claude:claude-opus-4.6
> >> Signed-off-by: Daniel Zahka <daniel.zahka@xxxxxxxxx>

Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>

> >> ---
> >>   drivers/net/netdevsim/psp.c | 32 --------------------------------
> >>   1 file changed, 32 deletions(-)
> >>
> >> diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
> >> index 5073bda60883..75740e2a731f 100644
> >> --- a/drivers/net/netdevsim/psp.c
> >> +++ b/drivers/net/netdevsim/psp.c
> >> @@ -1,8 +1,6 @@
> >>   // SPDX-License-Identifier: GPL-2.0
> >>   
> >> -#include <linux/ip.h>
> >>   #include <linux/skbuff.h>
> >> -#include <net/ip6_checksum.h>
> >>   #include <net/psp.h>
> >>   #include <net/sock.h>
> >>   
> >> @@ -81,36 +79,6 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
> >>   			      skb->len - skb_inner_transport_offset(skb));
> >>   		u64_stats_update_end(&ns->psp.syncp);
> >>   	} else {
> >> -		struct ipv6hdr *ip6h __maybe_unused;
> >> -		struct iphdr *iph;
> >> -		struct udphdr *uh;
> >> -		__wsum csum;
> >> -
> >> -		/* Do not decapsulate. Receive the skb with the udp and psp
> >> -		 * headers still there as if this is a normal udp packet.
> >> -		 * psp_dev_encapsulate() sets udp checksum to 0, so we need to
> >> -		 * provide a valid checksum here, so the skb isn't dropped.
> >> -		 */
> > Perhaps this was here as IPv6 does not allow zero checksums except for
> > tunneling in specific cases (RFC 6936)?
> 
> 
> Yes it was originally here for IPv6. It was needed to make a test case 
> pass that ultimately never got upstreamed (yet). The test basically used 
> the psp_dev_ops::set_config() function to turn psp rx off midflow, and 
> then tried to catch packets with a udp socket listening on port 1000. At 
> the time I didn't realize that I could probably make the test work by 
> opting the socket into a setting for RFC 6936 with setsockopt().

Just curious: which setsockopt is this?

> 
> > It seems benign enough to keep the code. Do you have a specific reason
> > to remove it, beyond reducing LoC?
> 
> 
> It was just to reduce LoC for the series :) I can put it back in if you 
> like,

No, fine to remove then

> though as I mentioned, none of the current tests cover that the 
> psp-udp csum is correct.

.. actually based on this, preferable even