Re: [v5 PATCH 11/14] xfrm: ipcomp: Use crypto_acomp interface

[Sabrina Dubroca <sd@queasysnail.net>]

Hi Herbert,

2025-03-15, 18:30:43 +0800, Herbert Xu wrote:
> +static int ipcomp_post_acomp(struct sk_buff *skb, int err, int hlen)
> +{
> +	struct acomp_req *req = ipcomp_cb(skb)->req;
> +	struct ipcomp_req_extra *extra;
> +	const int plen = skb->data_len;
> +	struct scatterlist *dsg;
> +	int len, dlen;
>  
> -	len = dlen - plen;
> -	if (len > skb_tailroom(skb))
> -		len = skb_tailroom(skb);
> +	if (unlikely(err))
> +		goto out_free_req;
>  
> -	__skb_put(skb, len);
> +	extra = acomp_request_extra(req);
> +	dsg = extra->sg;
> +	dlen = req->dlen;
>  
> -	len += plen;
> -	skb_copy_to_linear_data(skb, scratch, len);
> +	pskb_trim_unique(skb, 0);
> +	__skb_put(skb, hlen);
>  
> -	while ((scratch += len, dlen -= len) > 0) {
> +	/* Only update truesize on input. */
> +	if (!hlen)
> +		skb->truesize += dlen - plen;

Are you sure we need to subtract plen here? When I run fragmented
traffic with ipcomp, I'm hitting the WARN from skb_try_coalesce during
reassembly, ie truesize is too small:

    delta = from->truesize - SKB_TRUESIZE(skb_end_offset(from));
    WARN_ON_ONCE(delta < len);

The splat goes away with

 	/* Only update truesize on input. */
 	if (!hlen)
-		skb->truesize += dlen - plen;
+		skb->truesize += dlen;
 	skb->data_len = dlen;
 	skb->len += dlen;

pskb_trim_unique ends up calling skb_condense, which seems to adjust
the truesize to account for all frags being dropped.

Does that look like the right fix to you?

-- 
Sabrina