[Qemu-devel] [ RFC Patch v6 1/3] virtio-net rsc: support coalescing ipv4 tcp traffic
From: Wei Xu All the data packets in a tcp connection will be cached to a big buffer in every receive interval, and will be sent out via a timer, the 'virtio_net_rsc_timeout' controls the interval, the value will influent the performance and response of tcp connection extremely, 5(50us) is a experience value to gain a performance improvement, since the whql test sends packets every 100us, so '30(300us)' can pass the test case, this is also the default value, it's tunable via the command line parameter 'rsc_interval' with 'virtio-net-pci' device, for example, below parameter is to launch a guest with interval set as '50'. 'virtio-net-pci,netdev=hostnet1,bus=pci.0,id=net1,mac=00,rsc_interval=50' will The timer will only be triggered if the packets pool is not empty, and it'll drain off all the cached packets. 'NetRscChain' is used to save the segments of different protocols in a VirtIONet device. The main handler of TCP includes TCP window update, duplicated ACK check and the real data coalescing if the new segment passed sanity check and is identified as an 'wanted' one. An 'wanted' segment means: 1. Segment is within current window and the sequence is the expected one. 2. 'ACK' of the segment is in the valid window. Sanity check includes: 1. Incorrect version in IP header 2. IP options & IP fragment 3. Not a TCP packets 4. Sanity size check to prevent buffer overflow attack. There maybe more cases should be considered such as ip identification other flags, while it broke the test because windows set it to the same even it's not a fragment. Normally it includes 2 typical ways to handle a TCP control flag, 'bypass' and 'finalize', 'bypass' means should be sent out directly, and 'finalize' means the packets should also be bypassed, and this should be done after searching for the same connection packets in the pool and sending all of them out, this is to avoid out of data. All the 'SYN' packets will be bypassed since this always begin a new' connection, other flags such 'FIN/RST' will trigger a finalization, because this normally happens upon a connection is going to be closed, an 'URG' packet also finalize current coalescing unit. Statistics can be used to monitor the basic coalescing status, the 'out of order' and 'out of window' means how many retransmitting packets, thus describe the performance intuitively. Signed-off-by: Wei Xu --- hw/net/virtio-net.c | 498 +++- include/hw/virtio/virtio-net.h | 2 + include/hw/virtio/virtio.h | 75 + include/standard-headers/linux/virtio_net.h | 1 + 4 files changed, 575 insertions(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 5798f87..b3bb63b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -15,10 +15,12 @@ #include "qemu/iov.h" #include "hw/virtio/virtio.h" #include "net/net.h" +#include "net/eth.h" #include "net/checksum.h" #include "net/tap.h" #include "qemu/error-report.h" #include "qemu/timer.h" +#include "qemu/sockets.h" #include "hw/virtio/virtio-net.h" #include "net/vhost_net.h" #include "hw/virtio/virtio-bus.h" @@ -38,6 +40,25 @@ #define endof(container, field) \ (offsetof(container, field) + sizeof(((container *)0)->field)) +#define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ +#define VIRTIO_NET_TCP_PORT_SIZE 4/* sport + dport */ + +#define VIRTIO_NET_TCP_FLAG 0x3F +#define VIRTIO_NET_TCP_HDR_LENGTH 0xF000 + +/* IPv4 max payload, 16 bits in the header */ +#define VIRTIO_NET_MAX_IP4_PAYLOAD (65535 - sizeof(struct ip_header)) +#define VIRTIO_NET_MAX_TCP_PAYLOAD 65535 + +/* header length value in ip header without option */ +#define VIRTIO_NET_IP4_HEADER_LENGTH 5 + +/* Purge coalesced packets timer interval, This value affects the performance + a lot, and should be tuned carefully, '30'(300us) is the recommended + value to pass the WHQL test, '5' can gain 2x netperf throughput with + tso/gso/gro 'off'. */ +#define VIRTIO_NET_RSC_INTERVAL 30 + typedef struct VirtIOFeature { uint32_t flags; size_t end; @@ -1089,7 +1110,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size) return 0; } -static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t size) +static ssize_t virtio_net_do_receive(NetClientState *nc, + const uint8_t *buf, size_t size) { VirtIONet *n = qemu_get_nic_opaque(nc); VirtIONetQueue *q = virtio_net_get_subqueue(nc); @@ -1685,6 +1707,474 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, return 0; } +static void virtio_net_rsc_extract_unit4(NetRscChain *chain, + const uint8_t *buf, NetRscUnit* unit) +{ +uint16_t hdr_len; +uint16_t ip_hdrlen; +struct ip_header *ip; + +hdr_len = chain->n->guest_hdr_len; +ip = (struct ip_hea
Re: [Qemu-devel] [ RFC Patch v6 1/3] virtio-net rsc: support coalescing ipv4 tcp traffic
On 2016年05月29日 00:37, w...@redhat.com wrote: From: Wei Xu All the data packets in a tcp connection will be cached to a big buffer in every receive interval, and will be sent out via a timer, the 'virtio_net_rsc_timeout' controls the interval, the value will influent the performance and response of tcp connection extremely, 5(50us) is a experience value to gain a performance improvement, since the whql test sends packets every 100us, so '30(300us)' can pass the test case, this is also the default value, it's tunable via the command line parameter 'rsc_interval' with 'virtio-net-pci' device, for example, below parameter is to launch a guest with interval set as '50'. Does the value make sense if it was smaller than 1us? If not, why not just make the unit to be 1us? 'virtio-net-pci,netdev=hostnet1,bus=pci.0,id=net1,mac=00,rsc_interval=50' will The timer will only be triggered if the packets pool is not empty, and it'll drain off all the cached packets. 'NetRscChain' is used to save the segments of different protocols in a VirtIONet device. The main handler of TCP includes TCP window update, duplicated ACK check and the real data coalescing if the new segment passed sanity check and is identified as an 'wanted' one. An 'wanted' segment means: 1. Segment is within current window and the sequence is the expected one. 2. 'ACK' of the segment is in the valid window. Sanity check includes: 1. Incorrect version in IP header 2. IP options & IP fragment 3. Not a TCP packets 4. Sanity size check to prevent buffer overflow attack. There maybe more cases should be considered such as ip identification other flags, while it broke the test because windows set it to the same even it's not a fragment. Normally it includes 2 typical ways to handle a TCP control flag, 'bypass' and 'finalize', 'bypass' means should be sent out directly, and 'finalize' means the packets should also be bypassed, and this should be done after searching for the same connection packets in the pool and sending all of them out, this is to avoid out of data. All the 'SYN' packets will be bypassed since this always begin a new' connection, other flags such 'FIN/RST' will trigger a finalization, because this normally happens upon a connection is going to be closed, an 'URG' packet also finalize current coalescing unit. Statistics can be used to monitor the basic coalescing status, the 'out of order' and 'out of window' means how many retransmitting packets, thus describe the performance intuitively. We usually don't add device specific monitor command. Maybe a new control vq command ethtool -S in guest in the future. I was thinking of removing those counters since it was never used in this series. Signed-off-by: Wei Xu --- hw/net/virtio-net.c | 498 +++- include/hw/virtio/virtio-net.h | 2 + include/hw/virtio/virtio.h | 75 + include/standard-headers/linux/virtio_net.h | 1 + For RFC, it's ok. But for formal patch, this is not the correct way to modify Linux headers. There's a script in scripts/update-linux-headers.sh which was used to sync it from Linux source. This means, it must be merged in Linux or at least in maintainer's tree first. 4 files changed, 575 insertions(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 5798f87..b3bb63b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -15,10 +15,12 @@ #include "qemu/iov.h" #include "hw/virtio/virtio.h" #include "net/net.h" +#include "net/eth.h" #include "net/checksum.h" #include "net/tap.h" #include "qemu/error-report.h" #include "qemu/timer.h" +#include "qemu/sockets.h" #include "hw/virtio/virtio-net.h" #include "net/vhost_net.h" #include "hw/virtio/virtio-bus.h" @@ -38,6 +40,25 @@ #define endof(container, field) \ (offsetof(container, field) + sizeof(((container *)0)->field)) +#define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ +#define VIRTIO_NET_TCP_PORT_SIZE 4/* sport + dport */ + +#define VIRTIO_NET_TCP_FLAG 0x3F +#define VIRTIO_NET_TCP_HDR_LENGTH 0xF000 + +/* IPv4 max payload, 16 bits in the header */ +#define VIRTIO_NET_MAX_IP4_PAYLOAD (65535 - sizeof(struct ip_header)) +#define VIRTIO_NET_MAX_TCP_PAYLOAD 65535 Is this still true if window scaling is enabled? And need to make sure your ack/seq processing can work for window scaling. + +/* header length value in ip header without option */ +#define VIRTIO_NET_IP4_HEADER_LENGTH 5 + +/* Purge coalesced packets timer interval, This value affects the performance + a lot, and should be tuned carefully, '30'(300us) is the recommended + value to pass the WHQL test, '5' can gain 2x netperf throughput with + tso/gso/gro 'off'. */ +#define VIRTIO_NET_RSC_INTERVAL 30 Do we need a new property for this value? + typedef struct VirtIOFeature { uint32_t flags; siz
Re: [Qemu-devel] [ RFC Patch v6 1/3] virtio-net rsc: support coalescing ipv4 tcp traffic
On 2016年05月30日 12:20, Jason Wang wrote: On 2016年05月29日 00:37, w...@redhat.com wrote: From: Wei Xu All the data packets in a tcp connection will be cached to a big buffer in every receive interval, and will be sent out via a timer, the 'virtio_net_rsc_timeout' controls the interval, the value will influent the performance and response of tcp connection extremely, 5(50us) is a experience value to gain a performance improvement, since the whql test sends packets every 100us, so '30(300us)' can pass the test case, this is also the default value, it's tunable via the command line parameter 'rsc_interval' with 'virtio-net-pci' device, for example, below parameter is to launch a guest with interval set as '50'. Does the value make sense if it was smaller than 1us? If not, why not just make the unit to be 1us? It's an experience value, 500us is a good candidate for netperf throughput test, too short interval less than 50 us will gain an obvious penalty AFAIK, this is caused only few packets can be coalesced and the cycles wasted for rsc itself. 'virtio-net-pci,netdev=hostnet1,bus=pci.0,id=net1,mac=00,rsc_interval=50' will The timer will only be triggered if the packets pool is not empty, and it'll drain off all the cached packets. 'NetRscChain' is used to save the segments of different protocols in a VirtIONet device. The main handler of TCP includes TCP window update, duplicated ACK check and the real data coalescing if the new segment passed sanity check and is identified as an 'wanted' one. An 'wanted' segment means: 1. Segment is within current window and the sequence is the expected one. 2. 'ACK' of the segment is in the valid window. Sanity check includes: 1. Incorrect version in IP header 2. IP options & IP fragment 3. Not a TCP packets 4. Sanity size check to prevent buffer overflow attack. There maybe more cases should be considered such as ip identification other flags, while it broke the test because windows set it to the same even it's not a fragment. Normally it includes 2 typical ways to handle a TCP control flag, 'bypass' and 'finalize', 'bypass' means should be sent out directly, and 'finalize' means the packets should also be bypassed, and this should be done after searching for the same connection packets in the pool and sending all of them out, this is to avoid out of data. All the 'SYN' packets will be bypassed since this always begin a new' connection, other flags such 'FIN/RST' will trigger a finalization, because this normally happens upon a connection is going to be closed, an 'URG' packet also finalize current coalescing unit. Statistics can be used to monitor the basic coalescing status, the 'out of order' and 'out of window' means how many retransmitting packets, thus describe the performance intuitively. We usually don't add device specific monitor command. Maybe a new control vq command ethtool -S in guest in the future. I was thinking of removing those counters since it was never used in this series. Yes, that's a good idea, actually i'm doubting whether this feature should be a guest feature or a host feature, the spec says it should be more like a guest feature, but it's provided as a host built-in feature, so how and where to examine it is a problem. I'm using gdb to debugging it currently, normally i would check this counter directly via debug command, and the statistics is quite useful for troubleshooting, it'll be optional when this feature is geting more and more robust, can we keep it and leave if should keep it or where to display it along with qa's testing? Signed-off-by: Wei Xu --- hw/net/virtio-net.c | 498 +++- include/hw/virtio/virtio-net.h | 2 + include/hw/virtio/virtio.h | 75 + include/standard-headers/linux/virtio_net.h | 1 + For RFC, it's ok. But for formal patch, this is not the correct way to modify Linux headers. There's a script in scripts/update-linux-headers.sh which was used to sync it from Linux source. This means, it must be merged in Linux or at least in maintainer's tree first. 4 files changed, 575 insertions(+), 1 deletion(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 5798f87..b3bb63b 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -15,10 +15,12 @@ #include "qemu/iov.h" #include "hw/virtio/virtio.h" #include "net/net.h" +#include "net/eth.h" #include "net/checksum.h" #include "net/tap.h" #include "qemu/error-report.h" #include "qemu/timer.h" +#include "qemu/sockets.h" #include "hw/virtio/virtio-net.h" #include "net/vhost_net.h" #include "hw/virtio/virtio-bus.h" @@ -38,6 +40,25 @@ #define endof(container, field) \ (offsetof(container, field) + sizeof(((container *)0)->field)) +#define VIRTIO_NET_IP4_ADDR_SIZE 8/* ipv4 saddr + daddr */ +#define VIRTIO_NET_TCP_PORT_SIZE 4/* sport + dport */ + +#define VIRTIO_NET_TCP_FL