| Message ID | 20241014031234.7659-3-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio_net: enable premapped mode by default | expand |
On Mon, Oct 14, 2024 at 11:12 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > > Now, if we want to judge the rx work mode, we have to use such codes: > > 1. merge mode: vi->mergeable_rx_bufs > 2. big mode: vi->big_packets && !vi->mergeable_rx_bufs > 3. small: !vi->big_packets && !vi->mergeable_rx_bufs > > This is inconvenient and abstract, and we also have this use case: > > if (vi->mergeable_rx_bufs) > .... > else if (vi->big_packets) > .... > else > > For this case, I think switch-case is the better choice. > > So here I introduce vi->mode to record the virtio-net work mode. > That is helpful to judge the work mode and choose the branches. > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > drivers/net/virtio_net.c | 61 +++++++++++++++++++++++++++++++--------- > 1 file changed, 47 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 59a99bbaf852..14809b614d62 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -385,6 +385,12 @@ struct control_buf { > virtio_net_ctrl_ack status; > }; > > +enum virtnet_mode { > + VIRTNET_MODE_SMALL, > + VIRTNET_MODE_MERGE, > + VIRTNET_MODE_BIG > +}; I'm not sure if this can ease or not. [...] > + if (vi->mergeable_rx_bufs) > + vi->mode = VIRTNET_MODE_MERGE; > + else if (vi->big_packets) > + vi->mode = VIRTNET_MODE_BIG; Maybe we can just say big_packets doesn't mean big mode. > + else > + vi->mode = VIRTNET_MODE_SMALL; > + > if (vi->any_header_sg) > dev->needed_headroom = vi->hdr_len; Anyhow this seems not a fix so it should be a separate series than patch 1? Thanks > > -- > 2.32.0.3.g01195cf9f >
On Fri, 18 Oct 2024 15:48:38 +0800, Jason Wang <jasowang@redhat.com> wrote: > On Mon, Oct 14, 2024 at 11:12 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote: > > > > Now, if we want to judge the rx work mode, we have to use such codes: > > > > 1. merge mode: vi->mergeable_rx_bufs > > 2. big mode: vi->big_packets && !vi->mergeable_rx_bufs > > 3. small: !vi->big_packets && !vi->mergeable_rx_bufs > > > > This is inconvenient and abstract, and we also have this use case: > > > > if (vi->mergeable_rx_bufs) > > .... > > else if (vi->big_packets) > > .... > > else > > > > For this case, I think switch-case is the better choice. > > > > So here I introduce vi->mode to record the virtio-net work mode. > > That is helpful to judge the work mode and choose the branches. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > --- > > drivers/net/virtio_net.c | 61 +++++++++++++++++++++++++++++++--------- > > 1 file changed, 47 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > index 59a99bbaf852..14809b614d62 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -385,6 +385,12 @@ struct control_buf { > > virtio_net_ctrl_ack status; > > }; > > > > +enum virtnet_mode { > > + VIRTNET_MODE_SMALL, > > + VIRTNET_MODE_MERGE, > > + VIRTNET_MODE_BIG > > +}; > > I'm not sure if this can ease or not. > > [...] > > > + if (vi->mergeable_rx_bufs) > > + vi->mode = VIRTNET_MODE_MERGE; > > + else if (vi->big_packets) > > + vi->mode = VIRTNET_MODE_BIG; > > Maybe we can just say big_packets doesn't mean big mode. > > > + else > > + vi->mode = VIRTNET_MODE_SMALL; > > + > > if (vi->any_header_sg) > > dev->needed_headroom = vi->hdr_len; > > Anyhow this seems not a fix so it should be a separate series than patch 1? I think this series is not about fixing the problem, the feature was disabled in the last Linux version. This series is about turning the pre-mapped mode of rx back on. This commit tries to make things easier. The current code is very unintuitive when we try to switch or check the virtio-net rx mode Thanks. > > Thanks > > > > > -- > > 2.32.0.3.g01195cf9f > > >
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 59a99bbaf852..14809b614d62 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -385,6 +385,12 @@ struct control_buf { virtio_net_ctrl_ack status; }; +enum virtnet_mode { + VIRTNET_MODE_SMALL, + VIRTNET_MODE_MERGE, + VIRTNET_MODE_BIG +}; + struct virtnet_info { struct virtio_device *vdev; struct virtqueue *cvq; @@ -414,6 +420,8 @@ struct virtnet_info { /* Host will merge rx buffers for big packets (shake it! shake it!) */ bool mergeable_rx_bufs; + enum virtnet_mode mode; + /* Host supports rss and/or hash report */ bool has_rss; bool has_rss_hash_report; @@ -643,12 +651,15 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask) static void virtnet_rq_free_buf(struct virtnet_info *vi, struct receive_queue *rq, void *buf) { - if (vi->mergeable_rx_bufs) + switch (vi->mode) { + case VIRTNET_MODE_SMALL: + case VIRTNET_MODE_MERGE: put_page(virt_to_head_page(buf)); - else if (vi->big_packets) + break; + case VIRTNET_MODE_BIG: give_pages(rq, buf); - else - put_page(virt_to_head_page(buf)); + break; + } } static void enable_delayed_refill(struct virtnet_info *vi) @@ -1315,7 +1326,8 @@ static void virtnet_receive_xsk_buf(struct virtnet_info *vi, struct receive_queu flags = ((struct virtio_net_common_hdr *)(xdp->data - vi->hdr_len))->hdr.flags; - if (!vi->mergeable_rx_bufs) + /* We only support small and merge mode. */ + if (vi->mode == VIRTNET_MODE_SMALL) skb = virtnet_receive_xsk_small(dev, vi, rq, xdp, xdp_xmit, stats); else skb = virtnet_receive_xsk_merge(dev, vi, rq, xdp, xdp_xmit, stats); @@ -2389,13 +2401,20 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq, */ flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags; - if (vi->mergeable_rx_bufs) + switch (vi->mode) { + case VIRTNET_MODE_MERGE: skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit, stats); - else if (vi->big_packets) + break; + + case VIRTNET_MODE_BIG: skb = receive_big(dev, vi, rq, buf, len, stats); - else + break; + + case VIRTNET_MODE_SMALL: skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats); + break; + } if (unlikely(!skb)) return; @@ -2580,12 +2599,19 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq, } do { - if (vi->mergeable_rx_bufs) + switch (vi->mode) { + case VIRTNET_MODE_MERGE: err = add_recvbuf_mergeable(vi, rq, gfp); - else if (vi->big_packets) + break; + + case VIRTNET_MODE_BIG: err = add_recvbuf_big(vi, rq, gfp); - else + break; + + case VIRTNET_MODE_SMALL: err = add_recvbuf_small(vi, rq, gfp); + break; + } if (err) break; @@ -2703,7 +2729,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi, int packets = 0; void *buf; - if (!vi->big_packets || vi->mergeable_rx_bufs) { + if (vi->mode != VIRTNET_MODE_BIG) { void *ctx; while (packets < budget && (buf = virtnet_rq_get_buf(rq, &len, &ctx))) { @@ -5510,7 +5536,7 @@ static int virtnet_xsk_pool_enable(struct net_device *dev, /* In big_packets mode, xdp cannot work, so there is no need to * initialize xsk of rq. */ - if (vi->big_packets && !vi->mergeable_rx_bufs) + if (vi->mode == VIRTNET_MODE_BIG) return -ENOENT; if (qid >= vi->curr_queue_pairs) @@ -6007,7 +6033,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi) vqs_info = kcalloc(total_vqs, sizeof(*vqs_info), GFP_KERNEL); if (!vqs_info) goto err_vqs_info; - if (!vi->big_packets || vi->mergeable_rx_bufs) { + if (vi->mode != VIRTNET_MODE_BIG) { ctx = kcalloc(total_vqs, sizeof(*ctx), GFP_KERNEL); if (!ctx) goto err_ctx; @@ -6480,6 +6506,13 @@ static int virtnet_probe(struct virtio_device *vdev) virtnet_set_big_packets(vi, mtu); + if (vi->mergeable_rx_bufs) + vi->mode = VIRTNET_MODE_MERGE; + else if (vi->big_packets) + vi->mode = VIRTNET_MODE_BIG; + else + vi->mode = VIRTNET_MODE_SMALL; + if (vi->any_header_sg) dev->needed_headroom = vi->hdr_len;
Now, if we want to judge the rx work mode, we have to use such codes: 1. merge mode: vi->mergeable_rx_bufs 2. big mode: vi->big_packets && !vi->mergeable_rx_bufs 3. small: !vi->big_packets && !vi->mergeable_rx_bufs This is inconvenient and abstract, and we also have this use case: if (vi->mergeable_rx_bufs) .... else if (vi->big_packets) .... else For this case, I think switch-case is the better choice. So here I introduce vi->mode to record the virtio-net work mode. That is helpful to judge the work mode and choose the branches. Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/net/virtio_net.c | 61 +++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 14 deletions(-)