On Mon, 31 May 2021 14:10:55 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
>
> 在 2021/5/14 下午11:16, Xuan Zhuo 写道:
> > In the case of merge, the page passed into page_to_skb() may be a head
> > page, not the page where the current data is located.
>
>
> I don't get how this can happen?
>
> Maybe you can explain a little bit more?
>
> receive_mergeable() call page_to_skb() in two places:
>
> 1) XDP_PASS for linearized page , in this case we use xdp_page
> 2) page_to_skb() for "normal" page, in this case the page contains the data
The offset may be greater than PAGE_SIZE, because page is obtained by
virt_to_head_page(), not the page where buf is located. And "offset" is the offset
of buf relative to page.
tailroom = truesize - len - offset;
In this case, the tailroom must be less than 0. Although there may be enough
content on this page to save skb_shared_info.
Thanks.
>
> Thanks
>
>
> > So when trying to
> > get the buf where the data is located, you should directly use the
> > pointer(p) to get the address corresponding to the page.
> >
> > At the same time, the offset of the data in the page should also be
> > obtained using offset_in_page().
> >
> > This patch solves this problem. But if you don’t use this patch, the
> > original code can also run, because if the page is not the page of the
> > current data, the calculated tailroom will be less than 0, and will not
> > enter the logic of build_skb() . The significance of this patch is to
> > modify this logical problem, allowing more situations to use
> > build_skb().
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> > ---
> > drivers/net/virtio_net.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 3e46c12dde08..073fec4c0df1 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -407,8 +407,12 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > * see add_recvbuf_mergeable() + get_mergeable_buf_len()
> > */
> > truesize = PAGE_SIZE;
> > - tailroom = truesize - len - offset;
> > - buf = page_address(page);
> > +
> > + /* page maybe head page, so we should get the buf by p, not the
> > + * page
> > + */
> > + tailroom = truesize - len - offset_in_page(p);
> > + buf = (char *)((unsigned long)p & PAGE_MASK);
> > } else {
> > tailroom = truesize - len;
> > buf = p;
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
