| Message ID | 20221122074348.88601-2-hengqi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio_net: support multi buffer xdp | expand |
On Tue, Nov 22, 2022 at 3:44 PM Heng Qi <hengqi@linux.alibaba.com> wrote: > > XDP core assumes that the frame_size of xdp_buff and the length of > the frag are PAGE_SIZE. But before xdp is set, the length of the prefilled > buffer may exceed PAGE_SIZE, which may cause the processing of xdp to fail, > so we disable the hole mechanism when xdp is loaded. > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com> > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 9cce7dec7366..c5046d21b281 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -1419,8 +1419,11 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, > /* To avoid internal fragmentation, if there is very likely not > * enough space for another buffer, add the remaining space to > * the current buffer. > + * XDP core assumes that frame_size of xdp_buff and the length > + * of the frag are PAGE_SIZE, so we disable the hole mechanism. > */ > - len += hole; > + if (!vi->xdp_enabled) How is this synchronized with virtnet_xdp_set()? I think we need to use headroom here since it did: static unsigned int virtnet_get_headroom(struct virtnet_info *vi) { return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0; } Otherwise xdp_enabled could be re-read which may lead bugs. Thanks > + len += hole; > alloc_frag->offset += hole; > } > > -- > 2.19.1.6.gb485710b >
在 2022/12/6 下午1:20, Jason Wang 写道: > On Tue, Nov 22, 2022 at 3:44 PM Heng Qi <hengqi@linux.alibaba.com> wrote: >> XDP core assumes that the frame_size of xdp_buff and the length of >> the frag are PAGE_SIZE. But before xdp is set, the length of the prefilled >> buffer may exceed PAGE_SIZE, which may cause the processing of xdp to fail, >> so we disable the hole mechanism when xdp is loaded. >> >> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com> >> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> >> --- >> drivers/net/virtio_net.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index 9cce7dec7366..c5046d21b281 100644 >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c >> @@ -1419,8 +1419,11 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, >> /* To avoid internal fragmentation, if there is very likely not >> * enough space for another buffer, add the remaining space to >> * the current buffer. >> + * XDP core assumes that frame_size of xdp_buff and the length >> + * of the frag are PAGE_SIZE, so we disable the hole mechanism. >> */ >> - len += hole; >> + if (!vi->xdp_enabled) > How is this synchronized with virtnet_xdp_set()? > > I think we need to use headroom here since it did: > > static unsigned int virtnet_get_headroom(struct virtnet_info *vi) > { > return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0; > } > > Otherwise xdp_enabled could be re-read which may lead bugs. Yes, we should use headroom instead of using vi->xdp_enabled twice in the same position to avoid re-reading. Thanks for reminding. > > Thanks > >> + len += hole; >> alloc_frag->offset += hole; >> } >> >> -- >> 2.19.1.6.gb485710b >>
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 9cce7dec7366..c5046d21b281 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1419,8 +1419,11 @@ static int add_recvbuf_mergeable(struct virtnet_info *vi, /* To avoid internal fragmentation, if there is very likely not * enough space for another buffer, add the remaining space to * the current buffer. + * XDP core assumes that frame_size of xdp_buff and the length + * of the frag are PAGE_SIZE, so we disable the hole mechanism. */ - len += hole; + if (!vi->xdp_enabled) + len += hole; alloc_frag->offset += hole; }