[PATCH net v2 0/2] Fix skb length accounting after XDP frag adjustment

Hi,

This series fixes skb length accounting after XDP fragment adjustment in
generic XDP and veth.

v1:
https://lore.kernel.org/bpf/[email protected]/

Changes in v2:
- Move the veth fragment accounting before the linear tail adjustment, so
  __skb_put() observes a linear skb after a shrink removes all fragments.
- Fold the skb->len update into the xdp_buff_has_frags() branch, as suggested
  by Lorenzo Bianconi.
- Clarify why the old data_len contribution must be removed before adding the
  updated one, as requested by Maciej Fijalkowski.

Sun Jian (2):
  net: fix skb length accounting after generic XDP frag adjustment
  veth: fix skb length accounting after XDP frag adjustment

 drivers/net/veth.c | 23 +++++++++++++++--------
 net/core/dev.c     | 10 +++++++---
 2 files changed, 22 insertions(+), 11 deletions(-)

Range-diff against v1:
1:  b914fb40f348 ! 1:  2d716a1bd570 net: fix skb length accounting after generic XDP frag adjustment
    @@ Commit message
     
         Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
         Cc: [email protected]
    -    Link: https://lore.kernel.org/r/[email protected]
         Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
         Signed-off-by: Sun Jian <[email protected]>
     
    @@ net/core/dev.c: u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buf
      
      	/* XDP frag metadata (e.g. nr_frags) are updated in eBPF helpers
     -	 * (e.g. bpf_xdp_adjust_tail), we need to update data_len here.
    -+	 * (e.g. bpf_xdp_adjust_tail), update skb length fields here.
    ++	 * (e.g. bpf_xdp_adjust_tail). Remove the old fragment contribution
    ++	 * from skb->len before updating data_len, then add the new one back.
      	 */
    +-	if (xdp_buff_has_frags(xdp))
     +	skb->len -= skb->data_len;
    - 	if (xdp_buff_has_frags(xdp))
    ++	if (xdp_buff_has_frags(xdp)) {
      		skb->data_len = skb_shinfo(skb)->xdp_frags_size;
    - 	else
    +-	else
    ++		skb->len += skb->data_len;
    ++	} else {
      		skb->data_len = 0;
    -+	skb->len += skb->data_len;
    ++	}
      
      	/* check if XDP changed eth hdr such SKB needs update */
      	eth = (struct ethhdr *)xdp->data;
2:  59c79966c84a ! 2:  e5b9383bc46e veth: fix skb length accounting after XDP frag adjustment
    @@ Commit message
         Subtract the old data_len before replacing it and add the new data_len
         afterwards, keeping skb->len and skb->data_len synchronized.
     
    +    The fragment accounting must run before the linear tail adjustment:
    +    when bpf_xdp_adjust_tail() shrinks the packet into the linear area it
    +    releases all fragments, and __skb_put() requires skb->data_len == 0
    +    by that point.
    +
         A 60000-byte UDP datagram on a veth pair with MTU 64000 was shortened by
         1024 bytes from its fragment area. Before the fix, all 10 runs produced
         corrupted payloads. After the fix, all 10 runs matched the expected
    @@ Commit message
     
         Fixes: 718a18a0c8a6 ("veth: Rework veth_xdp_rcv_skb in order to accept non-linear skb")
         Cc: [email protected]
    -    Link: https://lore.kernel.org/r/[email protected]
         Link: https://lore.kernel.org/bpf/al9T9Eto%2FhRIzP5W@boxer/
         Signed-off-by: Sun Jian <[email protected]>
     
      ## drivers/net/veth.c ##
     @@ drivers/net/veth.c: static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
    - 		__skb_put(skb, off); /* positive on grow, negative on shrink */
      
    + 	skb_reset_mac_header(skb);
    + 
    +-	/* check if bpf_xdp_adjust_tail was used */
    +-	off = xdp->data_end - orig_data_end;
    +-	if (off != 0)
    +-		__skb_put(skb, off); /* positive on grow, negative on shrink */
    +-
      	/* XDP frag metadata (e.g. nr_frags) are updated in eBPF helpers
     -	 * (e.g. bpf_xdp_adjust_tail), we need to update data_len here.
    -+	 * (e.g. bpf_xdp_adjust_tail), update skb length fields here.
    ++	 * (e.g. bpf_xdp_adjust_tail). Remove the old fragment contribution
    ++	 * from skb->len before updating data_len, then add the new one back.
    ++	 * This must precede the linear tail adjustment below: a changed
    ++	 * data_end implies that no fragments remain, and __skb_put() requires
    ++	 * a linear skb.
      	 */
    +-	if (xdp_buff_has_frags(xdp))
     +	skb->len -= skb->data_len;
    - 	if (xdp_buff_has_frags(xdp))
    ++	if (xdp_buff_has_frags(xdp)) {
      		skb->data_len = skb_shinfo(skb)->xdp_frags_size;
    - 	else
    +-	else
    ++		skb->len += skb->data_len;
    ++	} else {
      		skb->data_len = 0;
    -+	skb->len += skb->data_len;
    ++	}
    ++
    ++	/* check if bpf_xdp_adjust_tail was used */
    ++	off = xdp->data_end - orig_data_end;
    ++	if (off != 0)
    ++		__skb_put(skb, off); /* positive on grow, negative on shrink */
      
      	skb->protocol = eth_type_trans(skb, rq->dev);
      

base-commit: 97ac08560d236ca17f6606d9e671118e5eae5721
-- 
2.43.0