--- v4
+++ v3
@@ -1,185 +1,79 @@
-Now there are two types of "skb" and "xdp frame" during recycling old
-xmit.
+After testing, the performance of calling kick every time is not stable.
+And if all the packets are sent and kicked again, the performance is not
+good. So add a module parameter to specify how many packets are sent to
+call a kick.
-There are two completely similar and independent implementations. This
-is inconvenient for the subsequent addition of new types. So extract a
-function from this piece of code and call this function uniformly to
-recover old xmit ptr.
-
-Rename free_old_xmit_skbs() to free_old_xmit().
+8 is a relatively stable value with the best performance.
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
+Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
---
- drivers/net/virtio_net.c | 86 ++++++++++++++++++----------------------
- 1 file changed, 38 insertions(+), 48 deletions(-)
+ drivers/net/virtio_net.c | 23 ++++++++++++++++++++---
+ 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
-index 52653e234a20..f3752b254965 100644
+index 259fafcf6028..d7e95f55478d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
-@@ -264,6 +264,30 @@ static struct xdp_frame *ptr_to_xdp(void *ptr)
- return (struct xdp_frame *)((unsigned long)ptr & ~VIRTIO_XDP_FLAG);
- }
+@@ -29,10 +29,12 @@ module_param(napi_weight, int, 0444);
-+static void __free_old_xmit(struct send_queue *sq, bool in_napi,
-+ struct virtnet_sq_stats *stats)
-+{
-+ unsigned int len;
-+ void *ptr;
+ static bool csum = true, gso = true, napi_tx = true;
+ static int xsk_budget = 32;
++static int xsk_kick_thr = 8;
+ module_param(csum, bool, 0444);
+ module_param(gso, bool, 0444);
+ module_param(napi_tx, bool, 0644);
+ module_param(xsk_budget, int, 0644);
++module_param(xsk_kick_thr, int, 0644);
+
+ /* FIXME: MTU in config. */
+ #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
+@@ -2612,6 +2614,8 @@ static int virtnet_xsk_xmit_batch(struct send_queue *sq,
+ struct xdp_desc desc;
+ int err, packet = 0;
+ int ret = -EAGAIN;
++ int need_kick = 0;
++ int kicks = 0;
+
+ if (sq->xsk.last_desc.addr) {
+ err = virtnet_xsk_xmit(sq, pool, &sq->xsk.last_desc);
+@@ -2619,6 +2623,7 @@ static int virtnet_xsk_xmit_batch(struct send_queue *sq,
+ return -EBUSY;
+
+ ++packet;
++ ++need_kick;
+ --budget;
+ sq->xsk.last_desc.addr = 0;
+ }
+@@ -2642,13 +2647,25 @@ static int virtnet_xsk_xmit_batch(struct send_queue *sq,
+ }
+
+ ++packet;
++ ++need_kick;
++ if (need_kick > xsk_kick_thr) {
++ if (virtqueue_kick_prepare(sq->vq) &&
++ virtqueue_notify(sq->vq))
++ ++kicks;
+
-+ while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
-+ if (likely(!is_xdp_frame(ptr))) {
-+ struct sk_buff *skb = ptr;
-+
-+ pr_debug("Sent skb %p\n", skb);
-+
-+ stats->bytes += skb->len;
-+ napi_consume_skb(skb, in_napi);
-+ } else {
-+ struct xdp_frame *frame = ptr_to_xdp(ptr);
-+
-+ stats->bytes += frame->len;
-+ xdp_return_frame(frame);
++ need_kick = 0;
+ }
-+ stats->packets++;
-+ }
-+}
-+
- /* Converting between virtqueue no. and kernel tx/rx queue no.
- * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
- */
-@@ -525,15 +549,12 @@ static int virtnet_xdp_xmit(struct net_device *dev,
- int n, struct xdp_frame **frames, u32 flags)
- {
- struct virtnet_info *vi = netdev_priv(dev);
-+ struct virtnet_sq_stats stats = {};
- struct receive_queue *rq = vi->rq;
- struct bpf_prog *xdp_prog;
- struct send_queue *sq;
-- unsigned int len;
-- int packets = 0;
-- int bytes = 0;
- int nxmit = 0;
- int kicks = 0;
-- void *ptr;
- int ret;
- int i;
-
-@@ -552,20 +573,7 @@ static int virtnet_xdp_xmit(struct net_device *dev,
}
- /* Free up any pending old buffers before queueing new ones. */
-- while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
-- if (likely(is_xdp_frame(ptr))) {
-- struct xdp_frame *frame = ptr_to_xdp(ptr);
--
-- bytes += frame->len;
-- xdp_return_frame(frame);
-- } else {
-- struct sk_buff *skb = ptr;
--
-- bytes += skb->len;
-- napi_consume_skb(skb, false);
-- }
-- packets++;
-- }
-+ __free_old_xmit(sq, false, &stats);
+ if (packet) {
+- if (virtqueue_kick_prepare(sq->vq) &&
+- virtqueue_notify(sq->vq)) {
++ if (need_kick) {
++ if (virtqueue_kick_prepare(sq->vq) &&
++ virtqueue_notify(sq->vq))
++ ++kicks;
++ }
++ if (kicks) {
+ u64_stats_update_begin(&sq->stats.syncp);
+- sq->stats.kicks += 1;
++ sq->stats.kicks += kicks;
+ u64_stats_update_end(&sq->stats.syncp);
+ }
- for (i = 0; i < n; i++) {
- struct xdp_frame *xdpf = frames[i];
-@@ -582,8 +590,8 @@ static int virtnet_xdp_xmit(struct net_device *dev,
- }
- out:
- u64_stats_update_begin(&sq->stats.syncp);
-- sq->stats.bytes += bytes;
-- sq->stats.packets += packets;
-+ sq->stats.bytes += stats.bytes;
-+ sq->stats.packets += stats.packets;
- sq->stats.xdp_tx += n;
- sq->stats.xdp_tx_drops += n - nxmit;
- sq->stats.kicks += kicks;
-@@ -1406,39 +1414,21 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
- return stats.packets;
- }
-
--static void free_old_xmit_skbs(struct send_queue *sq, bool in_napi)
-+static void free_old_xmit(struct send_queue *sq, bool in_napi)
- {
-- unsigned int len;
-- unsigned int packets = 0;
-- unsigned int bytes = 0;
-- void *ptr;
--
-- while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
-- if (likely(!is_xdp_frame(ptr))) {
-- struct sk_buff *skb = ptr;
--
-- pr_debug("Sent skb %p\n", skb);
-+ struct virtnet_sq_stats stats = {};
-
-- bytes += skb->len;
-- napi_consume_skb(skb, in_napi);
-- } else {
-- struct xdp_frame *frame = ptr_to_xdp(ptr);
--
-- bytes += frame->len;
-- xdp_return_frame(frame);
-- }
-- packets++;
-- }
-+ __free_old_xmit(sq, in_napi, &stats);
-
- /* Avoid overhead when no packets have been processed
- * happens when called speculatively from start_xmit.
- */
-- if (!packets)
-+ if (!stats.packets)
- return;
-
- u64_stats_update_begin(&sq->stats.syncp);
-- sq->stats.bytes += bytes;
-- sq->stats.packets += packets;
-+ sq->stats.bytes += stats.bytes;
-+ sq->stats.packets += stats.packets;
- u64_stats_update_end(&sq->stats.syncp);
- }
-
-@@ -1463,7 +1453,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq)
- return;
-
- if (__netif_tx_trylock(txq)) {
-- free_old_xmit_skbs(sq, true);
-+ free_old_xmit(sq, true);
- __netif_tx_unlock(txq);
- }
-
-@@ -1548,7 +1538,7 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
-
- txq = netdev_get_tx_queue(vi->dev, index);
- __netif_tx_lock(txq, raw_smp_processor_id());
-- free_old_xmit_skbs(sq, true);
-+ free_old_xmit(sq, true);
- __netif_tx_unlock(txq);
-
- virtqueue_napi_complete(napi, sq->vq, 0);
-@@ -1617,7 +1607,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
- bool use_napi = sq->napi.weight;
-
- /* Free up any pending old buffers before queueing new ones. */
-- free_old_xmit_skbs(sq, false);
-+ free_old_xmit(sq, false);
-
- if (use_napi && kick)
- virtqueue_enable_cb_delayed(sq->vq);
-@@ -1661,7 +1651,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
- if (!use_napi &&
- unlikely(!virtqueue_enable_cb_delayed(sq->vq))) {
- /* More just got used, free them then recheck. */
-- free_old_xmit_skbs(sq, false);
-+ free_old_xmit(sq, false);
- if (sq->vq->num_free >= 2+MAX_SKB_FRAGS) {
- netif_start_subqueue(dev, qnum);
- virtqueue_disable_cb(sq->vq);
--
2.31.0