Thanks for reviewing the patch. I have sent a V2 with the default and assert.
Thanks, Sairam On 2/29/16, 9:35 AM, "Alin Serdean" <aserd...@cloudbasesolutions.com> wrote: >Small nit can you please add a default to the switch and put an assert. > > > >Acked-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> > > > > > >> -----Mesaj original----- > >> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Sairam > >> Venugopal > >> Trimis: Friday, February 26, 2016 1:07 AM > >> Către: dev@openvswitch.org > >> Subiect: [ovs-dev] [PATCH] datapath-windows: Support for IPv6 in TCP > >> segmentation > >> > >> When a packet which needs segmentation is received, the header for each > >> segment is being calculated, i.e. IP length, checksum, TCP seq, TCP > >> checksum. > >> > >> The problem with the current code is that it wrongly assumes that the > >> Ethernet frame payload is always an IPv4 packet. > >> > >> This patch checks the EtherType field of the Ethernet frame to see which > >> protocol is encapsulated in its payload, IPv4 or IPv6, and calculates >>the > >> segment's header accordingly. > >> > >> Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> > >> Co-authored-by: Sairam Venugopal <vsai...@vmware.com> > >> Reported-by: Sairam Venugopal <vsai...@vmware.com> > >> Reported-at: >>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswit >>ch_ovs-2Dissues_issues_105&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVM >>NtXt-uEs&r=Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=R0OVkoezG3iOMuMM >>Yve_RKIeIuZyUJ_kyTmQG4MFJj8&s=QVgByAwt3U18L-QpR-xv0sBJGt9K1oP-ndC6WQsyuRg >>&e= > >> --- > >> datapath-windows/ovsext/BufferMgmt.c | 116 > >> +++++++++++++++++++++++------------ > >> 1 file changed, 77 insertions(+), 39 deletions(-) > >> > >> diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath- > >> windows/ovsext/BufferMgmt.c > >> index 3189e9b..3fcbaad 100644 > >> --- a/datapath-windows/ovsext/BufferMgmt.c > >> +++ b/datapath-windows/ovsext/BufferMgmt.c > >> @@ -1122,11 +1122,10 @@ static NDIS_STATUS > >> FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 > >> seqNumber, > >> BOOLEAN lastPacket, UINT16 packetCounter) { > >> - EthHdr *dstEth; > >> - IPHdr *dstIP; > >> - TCPHdr *dstTCP; > >> - PMDL mdl; > >> - PUINT8 bufferStart; > >> + EthHdr *dstEth = NULL; > >> + TCPHdr *dstTCP = NULL; > >> + PMDL mdl = NULL; > >> + PUINT8 bufferStart = NULL; > >> > >> mdl = NET_BUFFER_FIRST_MDL(nb); > >> > >> @@ -1135,44 +1134,83 @@ FixSegmentHeader(PNET_BUFFER nb, UINT16 > >> segmentSize, UINT32 seqNumber, > >> return NDIS_STATUS_RESOURCES; > >> } > >> dstEth = (EthHdr *)(bufferStart + > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb)); > >> - ASSERT((INT)MmGetMdlByteCount(mdl) - > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb) > >> - >= sizeof(EthHdr) + sizeof(IPHdr) + sizeof(TCPHdr)); > >> - dstIP = (IPHdr *)((PCHAR)dstEth + sizeof *dstEth); > >> - dstTCP = (TCPHdr *)((PCHAR)dstIP + dstIP->ihl * 4); > >> - ASSERT((INT)MmGetMdlByteCount(mdl) - > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb) > >> - >= sizeof(EthHdr) + dstIP->ihl * 4 + TCP_HDR_LEN(dstTCP)); > >> - > >> - /* Fix IP length and checksum */ > >> - ASSERT(dstIP->protocol == IPPROTO_TCP); > >> - dstIP->tot_len = htons(segmentSize + dstIP->ihl * 4 + > >> TCP_HDR_LEN(dstTCP)); > >> - dstIP->id += packetCounter; > >> - dstIP->check = 0; > >> - dstIP->check = IPChecksum((UINT8 *)dstIP, dstIP->ihl * 4, 0); > >> - > >> - /* Fix TCP checksum */ > >> - dstTCP->seq = htonl(seqNumber); > >> > >> - /* > >> - * Set the TCP FIN and PSH bit only for the last packet > >> - * More information can be found under: > >> - * >>https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_e >>n-2D&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dcruz40PROJ >>40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=R0OVkoezG3iOMuMMYve_RKIeIuZyUJ_kyTmQG4 >>MFJj8&s=Y34uZn9AlyGqeEE32G5qh2qFTeV8s9Kq6nNORiMgQMc&e= > >> us/library/windows/hardware/ff568840%28v=vs.85%29.aspx > >> - */ > >> - if (dstTCP->fin) { > >> - dstTCP->fin = lastPacket; > >> + switch (dstEth->Type) { > >> + case ETH_TYPE_IPV4_NBO: > >> + { > >> + IPHdr *dstIP = NULL; > >> + > >> + ASSERT((INT)MmGetMdlByteCount(mdl) - > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb) > >> + >= sizeof(EthHdr) + sizeof(IPHdr) + sizeof(TCPHdr)); > >> + dstIP = (IPHdr *)((PCHAR)dstEth + sizeof(*dstEth)); > >> + dstTCP = (TCPHdr *)((PCHAR)dstIP + dstIP->ihl * 4); > >> + ASSERT((INT)MmGetMdlByteCount(mdl) - > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb) > >> + >= sizeof(EthHdr) + dstIP->ihl * 4 + > >> + TCP_HDR_LEN(dstTCP)); > >> + > >> + /* Fix IP length and checksum */ > >> + ASSERT(dstIP->protocol == IPPROTO_TCP); > >> + dstIP->tot_len = htons(segmentSize + dstIP->ihl * 4 + > >> TCP_HDR_LEN(dstTCP)); > >> + dstIP->id += packetCounter; > >> + dstIP->check = 0; > >> + dstIP->check = IPChecksum((UINT8 *)dstIP, dstIP->ihl * 4, 0); > >> + dstTCP->seq = htonl(seqNumber); > >> + > >> + /* > >> + * Set the TCP FIN and PSH bit only for the last packet > >> + * More information can be found under: > >> + * >>https://urldefense.proofpoint.com/v2/url?u=https-3A__msdn.microsoft.com_e >>n-2D&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dcruz40PROJ >>40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=R0OVkoezG3iOMuMMYve_RKIeIuZyUJ_kyTmQG4 >>MFJj8&s=Y34uZn9AlyGqeEE32G5qh2qFTeV8s9Kq6nNORiMgQMc&e= > >> us/library/windows/hardware/ff568840%28v=vs.85%29.aspx > >> + */ > >> + if (dstTCP->fin) { > >> + dstTCP->fin = lastPacket; > >> + } > >> + if (dstTCP->psh) { > >> + dstTCP->psh = lastPacket; > >> + } > >> + UINT16 csumLength = segmentSize + TCP_HDR_LEN(dstTCP); > >> + dstTCP->check = IPPseudoChecksum(&dstIP->saddr, > >> + &dstIP->daddr, > >> + IPPROTO_TCP, > >> + csumLength); > >> + dstTCP->check = CalculateChecksumNB(nb, > >> + csumLength, > >> + sizeof(*dstEth) + >>dstIP->ihl * 4); > >> + break; > >> + } > >> + case ETH_TYPE_IPV6_NBO: > >> + { > >> + IPv6Hdr *dstIP = NULL; > >> + > >> + ASSERT((INT)MmGetMdlByteCount(mdl) - > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb) > >> + >= sizeof(EthHdr) + sizeof(IPv6Hdr) + sizeof(TCPHdr)); > >> + dstIP = (IPv6Hdr *)((PCHAR)dstEth + sizeof(*dstEth)); > >> + dstTCP = (TCPHdr *)((PCHAR)dstIP + sizeof(IPv6Hdr)); > >> + ASSERT((INT)MmGetMdlByteCount(mdl) - > >> NET_BUFFER_CURRENT_MDL_OFFSET(nb) > >> + >= sizeof(EthHdr) + sizeof(IPv6Hdr) + TCP_HDR_LEN(dstTCP)); > >> + > >> + /* Fix IP length */ > >> + ASSERT(dstIP->nexthdr == IPPROTO_TCP); > >> + dstIP->payload_len = htons(segmentSize + sizeof(IPv6Hdr) + > >> + TCP_HDR_LEN(dstTCP)); > >> + > >> + dstTCP->seq = htonl(seqNumber); > >> + if (dstTCP->fin) { > >> + dstTCP->fin = lastPacket; > >> + } > >> + if (dstTCP->psh) { > >> + dstTCP->psh = lastPacket; > >> + } > >> + > >> + UINT16 csumLength = segmentSize + TCP_HDR_LEN(dstTCP); > >> + dstTCP->check = IPv6PseudoChecksum((UINT32*)&dstIP->saddr, > >> + (UINT32*)&dstIP->daddr, > >> + IPPROTO_TCP, > >> + csumLength); > >> + dstTCP->check = CalculateChecksumNB(nb, > >> + csumLength, > >> + sizeof(*dstEth) + >>sizeof(IPv6Hdr)); > >> + break; > >> } > >> - if (dstTCP->psh) { > >> - dstTCP->psh = lastPacket; > >> } > >> > >> - UINT16 csumLength = segmentSize + TCP_HDR_LEN(dstTCP); > >> - dstTCP->check = IPPseudoChecksum(&dstIP->saddr, > >> - &dstIP->daddr, > >> - IPPROTO_TCP, > >> - csumLength); > >> - dstTCP->check = CalculateChecksumNB(nb, > >> - csumLength, > >> - sizeof *dstEth + dstIP->ihl * >>4); > >> - > >> return STATUS_SUCCESS; > >> } > >> > >> -- > >> 2.5.0.windows.1 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> >>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm >>an_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r= >>Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=R0OVkoezG3iOMuMMYve_RKIeIuZ >>yUJ_kyTmQG4MFJj8&s=i2dODoh8zm-QD_m9kBp6WZW3_JHbgNiESA7Yvg7u9_k&e= > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
