| Message ID | 158824572816.2172139.1358700000273697123.stgit@firesoul |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [net-next,v2,01/33] xdp: add frame size to xdp_buff | expand |
On Thu, Apr 30, 2020 at 01:22:08PM +0200, Jesper Dangaard Brouer wrote: > The virtio_net driver is running inside the guest-OS. There are two > XDP receive code-paths in virtio_net, namely receive_small() and > receive_mergeable(). The receive_big() function does not support XDP. > > In receive_small() the frame size is available in buflen. The buffer > backing these frames are allocated in add_recvbuf_small() with same > size, except for the headroom, but tailroom have reserved room for > skb_shared_info. The headroom is encoded in ctx pointer as a value. > > In receive_mergeable() the frame size is more dynamic. There are two > basic cases: (1) buffer size is based on a exponentially weighted > moving average (see DECLARE_EWMA) of packet length. Or (2) in case > virtnet_get_headroom() have any headroom then buffer size is > PAGE_SIZE. The ctx pointer is this time used for encoding two values; > the buffer len "truesize" and headroom. In case (1) if the rx buffer > size is underestimated, the packet will have been split over more > buffers (num_buf info in virtio_net_hdr_mrg_rxbuf placed in top of > buffer area). If that happens the XDP path does a xdp_linearize_page > operation. > > Cc: Jason Wang <jasowang@redhat.com> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > drivers/net/virtio_net.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 11f722460513..1df3676da185 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -689,6 +689,7 @@ static struct sk_buff *receive_small(struct net_device *dev, > xdp.data_end = xdp.data + len; > xdp.data_meta = xdp.data; > xdp.rxq = &rq->xdp_rxq; > + xdp.frame_sz = buflen; > orig_data = xdp.data; > act = bpf_prog_run_xdp(xdp_prog, &xdp); > stats->xdp_packets++; > @@ -797,10 +798,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > int offset = buf - page_address(page); > struct sk_buff *head_skb, *curr_skb; > struct bpf_prog *xdp_prog; > - unsigned int truesize; > + unsigned int truesize = mergeable_ctx_to_truesize(ctx); > unsigned int headroom = mergeable_ctx_to_headroom(ctx); > - int err; > unsigned int metasize = 0; > + unsigned int frame_sz; > + int err; > > head_skb = NULL; > stats->bytes += len - vi->hdr_len; > @@ -821,6 +823,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > if (unlikely(hdr->hdr.gso_type)) > goto err_xdp; > > + /* Buffers with headroom use PAGE_SIZE as alloc size, > + * see add_recvbuf_mergeable() + get_mergeable_buf_len() > + */ > + frame_sz = headroom ? PAGE_SIZE : truesize; > + > /* This happens when rx buffer size is underestimated > * or headroom is not enough because of the buffer > * was refilled before XDP is set. This should only > @@ -834,6 +841,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > page, offset, > VIRTIO_XDP_HEADROOM, > &len); > + frame_sz = PAGE_SIZE; > + > if (!xdp_page) > goto err_xdp; > offset = VIRTIO_XDP_HEADROOM; > @@ -850,6 +859,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > xdp.data_end = xdp.data + (len - vi->hdr_len); > xdp.data_meta = xdp.data; > xdp.rxq = &rq->xdp_rxq; > + xdp.frame_sz = frame_sz; > > act = bpf_prog_run_xdp(xdp_prog, &xdp); > stats->xdp_packets++; > @@ -924,7 +934,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, > } > rcu_read_unlock(); > > - truesize = mergeable_ctx_to_truesize(ctx); > if (unlikely(len > truesize)) { > pr_debug("%s: rx error: len %u exceeds truesize %lu\n", > dev->name, len, (unsigned long)ctx); > >
On 2020/5/7 上午4:34, Michael S. Tsirkin wrote: > On Thu, Apr 30, 2020 at 01:22:08PM +0200, Jesper Dangaard Brouer wrote: >> The virtio_net driver is running inside the guest-OS. There are two >> XDP receive code-paths in virtio_net, namely receive_small() and >> receive_mergeable(). The receive_big() function does not support XDP. >> >> In receive_small() the frame size is available in buflen. The buffer >> backing these frames are allocated in add_recvbuf_small() with same >> size, except for the headroom, but tailroom have reserved room for >> skb_shared_info. The headroom is encoded in ctx pointer as a value. >> >> In receive_mergeable() the frame size is more dynamic. There are two >> basic cases: (1) buffer size is based on a exponentially weighted >> moving average (see DECLARE_EWMA) of packet length. Or (2) in case >> virtnet_get_headroom() have any headroom then buffer size is >> PAGE_SIZE. The ctx pointer is this time used for encoding two values; >> the buffer len "truesize" and headroom. In case (1) if the rx buffer >> size is underestimated, the packet will have been split over more >> buffers (num_buf info in virtio_net_hdr_mrg_rxbuf placed in top of >> buffer area). If that happens the XDP path does a xdp_linearize_page >> operation. >> >> Cc: Jason Wang<jasowang@redhat.com> >> Signed-off-by: Jesper Dangaard Brouer<brouer@redhat.com> > Acked-by: Michael S. Tsirkin<mst@redhat.com> Note that we do: xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len; So using PAGE_SIZE here is probably not correct. Thanks >
On Fri, 8 May 2020 10:05:46 +0800 Jason Wang <jasowang@redhat.com> wrote: > On 2020/5/7 上午4:34, Michael S. Tsirkin wrote: > > On Thu, Apr 30, 2020 at 01:22:08PM +0200, Jesper Dangaard Brouer wrote: > >> The virtio_net driver is running inside the guest-OS. There are two > >> XDP receive code-paths in virtio_net, namely receive_small() and > >> receive_mergeable(). The receive_big() function does not support XDP. > >> > >> In receive_small() the frame size is available in buflen. The buffer > >> backing these frames are allocated in add_recvbuf_small() with same > >> size, except for the headroom, but tailroom have reserved room for > >> skb_shared_info. The headroom is encoded in ctx pointer as a value. > >> > >> In receive_mergeable() the frame size is more dynamic. There are two > >> basic cases: (1) buffer size is based on a exponentially weighted > >> moving average (see DECLARE_EWMA) of packet length. Or (2) in case > >> virtnet_get_headroom() have any headroom then buffer size is > >> PAGE_SIZE. The ctx pointer is this time used for encoding two values; > >> the buffer len "truesize" and headroom. In case (1) if the rx buffer > >> size is underestimated, the packet will have been split over more > >> buffers (num_buf info in virtio_net_hdr_mrg_rxbuf placed in top of > >> buffer area). If that happens the XDP path does a xdp_linearize_page > >> operation. > >> > >> Cc: Jason Wang<jasowang@redhat.com> > >> Signed-off-by: Jesper Dangaard Brouer<brouer@redhat.com> > > Acked-by: Michael S. Tsirkin<mst@redhat.com> > > > Note that we do: > > xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len; > > So using PAGE_SIZE here is probably not correct. Yes, you are correct. I will fix this up in V3. We need to adjust/reduce xdp.frame_sz with these offsets, as frame_sz is an offset size from xdp.data_hard_start. Thanks for pointing this out again, I will fix.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 11f722460513..1df3676da185 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -689,6 +689,7 @@ static struct sk_buff *receive_small(struct net_device *dev, xdp.data_end = xdp.data + len; xdp.data_meta = xdp.data; xdp.rxq = &rq->xdp_rxq; + xdp.frame_sz = buflen; orig_data = xdp.data; act = bpf_prog_run_xdp(xdp_prog, &xdp); stats->xdp_packets++; @@ -797,10 +798,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, int offset = buf - page_address(page); struct sk_buff *head_skb, *curr_skb; struct bpf_prog *xdp_prog; - unsigned int truesize; + unsigned int truesize = mergeable_ctx_to_truesize(ctx); unsigned int headroom = mergeable_ctx_to_headroom(ctx); - int err; unsigned int metasize = 0; + unsigned int frame_sz; + int err; head_skb = NULL; stats->bytes += len - vi->hdr_len; @@ -821,6 +823,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, if (unlikely(hdr->hdr.gso_type)) goto err_xdp; + /* Buffers with headroom use PAGE_SIZE as alloc size, + * see add_recvbuf_mergeable() + get_mergeable_buf_len() + */ + frame_sz = headroom ? PAGE_SIZE : truesize; + /* This happens when rx buffer size is underestimated * or headroom is not enough because of the buffer * was refilled before XDP is set. This should only @@ -834,6 +841,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, page, offset, VIRTIO_XDP_HEADROOM, &len); + frame_sz = PAGE_SIZE; + if (!xdp_page) goto err_xdp; offset = VIRTIO_XDP_HEADROOM; @@ -850,6 +859,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, xdp.data_end = xdp.data + (len - vi->hdr_len); xdp.data_meta = xdp.data; xdp.rxq = &rq->xdp_rxq; + xdp.frame_sz = frame_sz; act = bpf_prog_run_xdp(xdp_prog, &xdp); stats->xdp_packets++; @@ -924,7 +934,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, } rcu_read_unlock(); - truesize = mergeable_ctx_to_truesize(ctx); if (unlikely(len > truesize)) { pr_debug("%s: rx error: len %u exceeds truesize %lu\n", dev->name, len, (unsigned long)ctx);
The virtio_net driver is running inside the guest-OS. There are two XDP receive code-paths in virtio_net, namely receive_small() and receive_mergeable(). The receive_big() function does not support XDP. In receive_small() the frame size is available in buflen. The buffer backing these frames are allocated in add_recvbuf_small() with same size, except for the headroom, but tailroom have reserved room for skb_shared_info. The headroom is encoded in ctx pointer as a value. In receive_mergeable() the frame size is more dynamic. There are two basic cases: (1) buffer size is based on a exponentially weighted moving average (see DECLARE_EWMA) of packet length. Or (2) in case virtnet_get_headroom() have any headroom then buffer size is PAGE_SIZE. The ctx pointer is this time used for encoding two values; the buffer len "truesize" and headroom. In case (1) if the rx buffer size is underestimated, the packet will have been split over more buffers (num_buf info in virtio_net_hdr_mrg_rxbuf placed in top of buffer area). If that happens the XDP path does a xdp_linearize_page operation. Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- drivers/net/virtio_net.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)