From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38182) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQAo5-0000Id-33 for qemu-devel@nongnu.org; Mon, 01 Feb 2016 04:30:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQAnx-0007E7-I8 for qemu-devel@nongnu.org; Mon, 01 Feb 2016 04:29:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34908) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQAnx-0007E0-A3 for qemu-devel@nongnu.org; Mon, 01 Feb 2016 04:29:53 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 45DEC804EB for ; Mon, 1 Feb 2016 09:29:50 +0000 (UTC) References: <1454264009-24094-1-git-send-email-wexu@redhat.com> <1454264009-24094-5-git-send-email-wexu@redhat.com> <56AEF951.4010506@redhat.com> <56AF175F.5060302@redhat.com> From: Jason Wang Message-ID: <56AF257F.3090805@redhat.com> Date: Mon, 1 Feb 2016 17:29:35 +0800 MIME-Version: 1.0 In-Reply-To: <56AF175F.5060302@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC Patch v2 04/10] virtio-net rsc: Detailed IPv4 and General TCP data coalescing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wei Xu , qemu-devel@nongnu.org Cc: Wei Xu , victork@redhat.com, mst@redhat.com, yvugenfi@redhat.com, marcel@redhat.com, dfleytma@redhat.com On 02/01/2016 04:29 PM, Wei Xu wrote: > > On 02/01/2016 02:21 PM, Jason Wang wrote: > >> >> On 02/01/2016 02:13 AM, wexu@redhat.com wrote: >>> From: Wei Xu >>> >>> Since this feature also needs to support IPv6, and there are >>> some protocol specific differences difference for IPv4/6 in the header, >>> so try to make the interface to be general. >>> >>> IPv4/6 should set up both the new and old IP/TCP header before invoking >>> TCP coalescing, and should also tell the real payload. >>> >>> The main handler of TCP includes TCP window update, duplicated ACK >>> check >>> and the real data coalescing if the new segment passed invalid filter >>> and is identified as an expected one. >>> >>> An expected 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. >>> 3. If the ACK in the segment is a duplicated one, then it must less >>> than 2, >>> this is to notify upper layer TCP starting retransmission due to >>> the spec. >>> >>> Signed-off-by: Wei Xu >>> --- >>> hw/net/virtio-net.c | 127 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++-- >>> 1 file changed, 124 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c >>> index cfbac6d..4f77fbe 100644 >>> --- a/hw/net/virtio-net.c >>> +++ b/hw/net/virtio-net.c >>> @@ -41,6 +41,10 @@ >>> #define VIRTIO_HEADER 12 /* Virtio net header size */ >>> #define IP_OFFSET (VIRTIO_HEADER + sizeof(struct eth_header)) >>> +#define TCP_WINDOW 65535 >> The name is confusing, how about TCP_MAX_WINDOW_SIZE ? > > Sounds better, will take it in. > >> >>> + >>> +/* IPv4 max payload, 16 bits in the header */ >>> +#define MAX_IP4_PAYLOAD (65535 - sizeof(struct ip_header)) >>> #define MAX_VIRTIO_IP_PAYLOAD (65535 + IP_OFFSET) >>> @@ -1670,13 +1674,130 @@ out: >>> return 0; >>> } >>> +static int32_t virtio_net_rsc_handle_ack(NetRscChain *chain, >>> NetRscSeg *seg, >>> + const uint8_t *buf, struct >>> tcp_header *n_tcp, >>> + struct tcp_header *o_tcp) >>> +{ >>> + uint32_t nack, oack; >>> + uint16_t nwin, owin; >>> + >>> + nack = htonl(n_tcp->th_ack); >>> + nwin = htons(n_tcp->th_win); >>> + oack = htonl(o_tcp->th_ack); >>> + owin = htons(o_tcp->th_win); >>> + >>> + if ((nack - oack) >= TCP_WINDOW) { >>> + return RSC_FINAL; >>> + } else if (nack == oack) { >>> + /* duplicated ack or window probe */ >>> + if (nwin == owin) { >>> + /* duplicated ack, add dup ack count due to whql test >>> up to 1 */ >>> + >>> + if (seg->dup_ack_count == 0) { >>> + seg->dup_ack_count++; >>> + return RSC_COALESCE; >>> + } else { >>> + /* Spec says should send it directly */ >>> + return RSC_FINAL; >>> + } >>> + } else { >>> + /* Coalesce window update */ >> Need we flush this immediately consider it was a window update? > > The flowchart in the spec says this can be coalesced as normal. > > https://msdn.microsoft.com/en-us/library/windows/hardware/jj853325%28v=vs.85%29.aspx I see. > > >> >>> + o_tcp->th_win = n_tcp->th_win; >>> + return RSC_COALESCE; >>> + } >>> + } else { >> What if nack < oack here? > > That should happen, the modulo-232 arithmetic check at the begin of > this function will keep the ack is in the current window. Ok. Thanks