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

Sabrina Dubroca
2025-04-25T13:13:55Z

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