On 2023/5/10 16:40, Simon Horman wrote:
> + XDP people and ML
>
> On Tue, May 09, 2023 at 07:41:45PM +0800, Yunsheng Lin wrote:
>> Most users use __skb_frag_set_page()/skb_frag_off_set()/
>> skb_frag_size_set() to fill the page desc for a skb frag.
>>
>> Introduce skb_frag_fill_page_desc() to do that.
>>
>> net/bpf/test_run.c does not call skb_frag_off_set() to
>> set the offset, "copy_from_user(page_address(page), ...)"
>> suggest that it is assuming offset to be initialized as
>> zero, so call skb_frag_fill_page_desc() with offset being
>> zero for this case.
>
> I think the question is, what is the value of bv_offset before this patch.
sinfo seems to be part of the 'data' kzalloced in
bpf_test_init(), so bv_offset should be zero too.
>
> Lorenzo and Stanislav, do you have any insight here?
>
>>
>> Also, skb_frag_set_page() is not used anymore, so remove
>> it.
>>
>> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx>
>
> ...
>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 738776ab8838..30be21c7d05f 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -2411,6 +2411,15 @@ static inline unsigned int skb_pagelen(const struct sk_buff *skb)
>> return skb_headlen(skb) + __skb_pagelen(skb);
>> }
>>
>> +static inline void skb_frag_fill_page_desc(skb_frag_t *frag,
>> + struct page *page,
>> + int off, int size)
>> +{
>> + frag->bv_page = page;
>> + frag->bv_offset = off;
>
> Maybe it is slightly nicer to use skb_frag_off_set() here.
Yes, that is good idea.
But we need to move the definition of skb_frag_off_set() before
skb_frag_fill_page_desc in order to use it, I try to keep the
patch simple for reviewing for now, so I perfer to not do it
now if that is ok for you.
>
