lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 28 Dec 2022 16:25:17 +0800 From: Heng Qi <hengqi@...ux.alibaba.com> To: Jason Wang <jasowang@...hat.com>, netdev@...r.kernel.org, bpf@...r.kernel.org Cc: "Michael S . Tsirkin" <mst@...hat.com>, Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>, John Fastabend <john.fastabend@...il.com>, "David S . Miller" <davem@...emloft.net>, Daniel Borkmann <daniel@...earbox.net>, Alexei Starovoitov <ast@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com> Subject: Re: [PATCH v2 6/9] virtio_net: transmit the multi-buffer xdp 在 2022/12/28 下午2:30, Jason Wang 写道: > > 在 2022/12/27 16:26, Heng Qi 写道: >> >> >> 在 2022/12/27 下午3:12, Jason Wang 写道: >>> >>> 在 2022/12/20 22:14, Heng Qi 写道: >>>> This serves as the basis for XDP_TX and XDP_REDIRECT >>>> to send a multi-buffer xdp_frame. >>>> >>>> Signed-off-by: Heng Qi <hengqi@...ux.alibaba.com> >>>> Reviewed-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com> >>>> --- >>>> drivers/net/virtio_net.c | 27 ++++++++++++++++++++++----- >>>> 1 file changed, 22 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>>> index 40bc58fa57f5..9f31bfa7f9a6 100644 >>>> --- a/drivers/net/virtio_net.c >>>> +++ b/drivers/net/virtio_net.c >>>> @@ -563,22 +563,39 @@ static int __virtnet_xdp_xmit_one(struct >>>> virtnet_info *vi, >>>> struct xdp_frame *xdpf) >>>> { >>>> struct virtio_net_hdr_mrg_rxbuf *hdr; >>>> - int err; >>>> + struct skb_shared_info *shinfo; >>>> + u8 nr_frags = 0; >>>> + int err, i; >>>> if (unlikely(xdpf->headroom < vi->hdr_len)) >>>> return -EOVERFLOW; >>>> - /* Make room for virtqueue hdr (also change xdpf->headroom?) */ >>>> + if (unlikely(xdp_frame_has_frags(xdpf))) { >>>> + shinfo = xdp_get_shared_info_from_frame(xdpf); >>>> + nr_frags = shinfo->nr_frags; >>>> + } >>>> + >>>> + /* Need to adjust this to calculate the correct postion >>>> + * for shinfo of the xdpf. >>>> + */ >>>> + xdpf->headroom -= vi->hdr_len; >>> >>> >>> Any reason we need to do this here? (Or if it is, is it only needed >>> for multibuffer XDP?) >> >> Going back to its wrapping function virtnet_xdp_xmit(), we need to >> free up the pending old buffers. >> If the "is_xdp_frame(ptr)" condition is met, then we need to >> calculate the position of skb_shared_info >> in xdp_get_frame_len() and xdp_return_frame(), which will involve to >> xdpf->data and xdpf->headroom. >> Therefore, we need to update the value of headroom synchronously here. > > > Let's tweak the comment above to something like this. Ok, thanks for your energy. > > With that fixed, > > Acked-by: Jason Wang <jasowang@...hat.com> > > Thanks > > >> >> Also, it's not necessary for single-buffer xdp, but we need to keep >> it because it's harmless and as it should be. >> >> Thanks. >> >>> >>> Other looks good. >>> >>> Thanks >>> >>> >>>> xdpf->data -= vi->hdr_len; >>>> /* Zero header and leave csum up to XDP layers */ >>>> hdr = xdpf->data; >>>> memset(hdr, 0, vi->hdr_len); >>>> xdpf->len += vi->hdr_len; >>>> - sg_init_one(sq->sg, xdpf->data, xdpf->len); >>>> + sg_init_table(sq->sg, nr_frags + 1); >>>> + sg_set_buf(sq->sg, xdpf->data, xdpf->len); >>>> + for (i = 0; i < nr_frags; i++) { >>>> + skb_frag_t *frag = &shinfo->frags[i]; >>>> + >>>> + sg_set_page(&sq->sg[i + 1], skb_frag_page(frag), >>>> + skb_frag_size(frag), skb_frag_off(frag)); >>>> + } >>>> - err = virtqueue_add_outbuf(sq->vq, sq->sg, 1, xdp_to_ptr(xdpf), >>>> - GFP_ATOMIC); >>>> + err = virtqueue_add_outbuf(sq->vq, sq->sg, nr_frags + 1, >>>> + xdp_to_ptr(xdpf), GFP_ATOMIC); >>>> if (unlikely(err)) >>>> return -ENOSPC; /* Caller handle free/refcnt */ >>
Powered by blists - more mailing lists