Re: [PATCH net-next RFC 3/3] virtio-net: conditionally enable tx interrupt
From: Eric Dumazet <hidden>
Date: 2014-10-11 14:48:23
Also in:
kvm, linux-api, lkml, virtualization
On Sat, 2014-10-11 at 15:16 +0800, Jason Wang wrote:
quoted hunk ↗ jump to hunk
We free transmitted packets in ndo_start_xmit() in the past to get better performance in the past. One side effect is that skb_orphan() needs to be called in ndo_start_xmit() which makes sk_wmem_alloc not accurate in fact. For TCP protocol, this means several optimization could not work well such as TCP small queue and auto corking. This can lead extra low throughput of small packets stream. Thanks to the urgent descriptor support. This patch tries to solve this issue by enable the tx interrupt selectively for stream packets. This means we don't need to orphan TCP stream packets in ndo_start_xmit() but enable tx interrupt for those packets. After we get tx interrupt, a tx napi was scheduled to free those packets. With this method, sk_wmem_alloc of TCP socket were more accurate than in the past which let TCP can batch more through TSQ and auto corking. Signed-off-by: Jason Wang <redacted> --- drivers/net/virtio_net.c | 164 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 128 insertions(+), 36 deletions(-)diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 5810841..b450fc4 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c@@ -72,6 +72,8 @@ struct send_queue { /* Name of the send queue: output.$index */ char name[40]; + + struct napi_struct napi; }; /* Internal representation of a receive virtqueue */@@ -217,15 +219,40 @@ static struct page *get_a_page(struct receive_queue *rq, gfp_t gfp_mask) return p; } +static int free_old_xmit_skbs(struct send_queue *sq, int budget) +{ + struct sk_buff *skb; + unsigned int len; + struct virtnet_info *vi = sq->vq->vdev->priv; + struct virtnet_stats *stats = this_cpu_ptr(vi->stats); + int sent = 0; + + while (sent < budget && + (skb = virtqueue_get_buf(sq->vq, &len)) != NULL) { + pr_debug("Sent skb %p\n", skb); + + u64_stats_update_begin(&stats->tx_syncp); + stats->tx_bytes += skb->len; + stats->tx_packets++; + u64_stats_update_end(&stats->tx_syncp); + + dev_kfree_skb_any(skb); + sent++; + } +
You could accumulate skb->len in a totlen var, and perform a single u64_stats_update_begin(&stats->tx_syncp); stats->tx_bytes += totlen; stats->tx_packets += sent; u64_stats_update_end(&stats->tx_syncp); after the loop.
+ return sent; +} +
...
+
+static bool virtnet_skb_needs_intr(struct sk_buff *skb)
+{
+ union {
+ unsigned char *network;
+ struct iphdr *ipv4;
+ struct ipv6hdr *ipv6;
+ } hdr;
+ struct tcphdr *th = tcp_hdr(skb);
+ u16 payload_len;
+
+ hdr.network = skb_network_header(skb);
+
+ /* Only IPv4/IPv6 with TCP is supported */Oh well, yet another packet flow dissector :) If most packets were caught by your implementation, you could use it for fast patj and fallback to skb_flow_dissect() for encapsulated traffic. struct flow_keys keys; if (!skb_flow_dissect(skb, &keys)) return false; if (keys.ip_proto != IPPROTO_TCP) return false; then check __skb_get_poff() how to get th, and check if there is some payload...
+ if ((skb->protocol == htons(ETH_P_IP)) &&
+ hdr.ipv4->protocol == IPPROTO_TCP) {
+ payload_len = ntohs(hdr.ipv4->tot_len) - hdr.ipv4->ihl * 4 -
+ th->doff * 4;
+ } else if ((skb->protocol == htons(ETH_P_IPV6) ||
+ hdr.ipv6->nexthdr == IPPROTO_TCP)) {
+ payload_len = ntohs(hdr.ipv6->payload_len) - th->doff * 4;
+ } else {
+ return false;
+ }
+
+ /* We don't want to dealy packet with PUSH bit and pure ACK packet */
+ if (!th->psh && payload_len)
+ return true;
+
+ return false;
}