From: Jakub Kicinski <kuba@xxxxxxxxxx> Date: Fri, 15 Nov 2024 18:40:59 -0800 > On Wed, 13 Nov 2024 16:24:34 +0100 Alexander Lobakin wrote: >> +static inline bool __xdp_buff_add_frag(struct xdp_buff *xdp, struct page *page, >> + u32 offset, u32 size, u32 truesize, >> + bool try_coalesce) >> +{ >> + struct skb_shared_info *sinfo = xdp_get_shared_info_from_buff(xdp); >> + skb_frag_t *prev; >> + u32 nr_frags; >> + >> + if (!xdp_buff_has_frags(xdp)) { >> + xdp_buff_set_frags_flag(xdp); >> + >> + nr_frags = 0; >> + sinfo->xdp_frags_size = 0; >> + sinfo->xdp_frags_truesize = 0; >> + >> + goto fill; >> + } >> + >> + nr_frags = sinfo->nr_frags; >> + if (unlikely(nr_frags == MAX_SKB_FRAGS)) >> + return false; >> + >> + prev = &sinfo->frags[nr_frags - 1]; >> + if (try_coalesce && page == skb_frag_page(prev) && >> + offset == skb_frag_off(prev) + skb_frag_size(prev)) >> + skb_frag_size_add(prev, size); > > don't we have to release the reference if we coalesced? Correct. I realized that when replying to Ido =\ > >> + else >> +fill: >> + __skb_fill_page_desc_noacc(sinfo, nr_frags++, page, >> + offset, size); >> + >> + sinfo->nr_frags = nr_frags; >> + sinfo->xdp_frags_size += size; >> + sinfo->xdp_frags_truesize += truesize; >> + >> + return true; >> +} Thanks, Olek