[ovs-dev] [PATCH] Revert "datapath-windows: Support attribute OVS_KEY_ATTR_TCP_FLAGS"

Gurucharan Shetty shettyg at nicira.com
Tue Oct 27 20:22:17 UTC 2015


On Tue, Oct 27, 2015 at 1:07 PM, Sairam Venugopal <vsairam at vmware.com> wrote:
> Acked-by: Sairam Venugopal <vsairam at vmware.com>
Applied, thanks.

>
>
> On 10/27/15, 12:50 PM, "Alin Serdean" <aserdean at cloudbasesolutions.com>
> wrote:
>
>>This reverts commit a26b2023ce33fed1ef962012dc2c03765d2e92cb.
>>
>>This patch punishes performance without the implementation of
>>megaflows on Windows.
>>
>>Once megaflows is implemented in the flow logic this patch will be
>>revisited.
>>
>>Signed-off-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
>>---
>> datapath-windows/ovsext/DpInternal.h   |  7 +++---
>> datapath-windows/ovsext/Flow.c         | 40
>>++++++++--------------------------
>> datapath-windows/ovsext/Flow.h         |  2 +-
>> datapath-windows/ovsext/PacketParser.c |  1 -
>> datapath-windows/ovsext/PacketParser.h |  3 +++
>> 5 files changed, 16 insertions(+), 37 deletions(-)
>>
>>diff --git a/datapath-windows/ovsext/DpInternal.h
>>b/datapath-windows/ovsext/DpInternal.h
>>index 0405b8e..8de48a2 100644
>>--- a/datapath-windows/ovsext/DpInternal.h
>>+++ b/datapath-windows/ovsext/DpInternal.h
>>@@ -65,8 +65,6 @@ typedef struct _OVS_VPORT_EXT_INFO {
>> typedef struct L4Key {
>>     ovs_be16 tpSrc;              /* TCP/UDP/SCTP source port. */
>>     ovs_be16 tpDst;              /* TCP/UDP/SCTP destination port. */
>>-    ovs_be16 flags;              /* TCP flags */
>>-    uint8_t  pad[2];
>> } L4Key;
>>
>> typedef struct Ipkey {
>>@@ -77,7 +75,7 @@ typedef struct Ipkey {
>>     uint8_t nwTtl;               /* IP TTL/Hop Limit. */
>>     uint8_t nwFrag;              /* FLOW_FRAG_* flags. */
>>     L4Key   l4;
>>-} IpKey;  /* Size of 20 byte. */
>>+} IpKey;  /* Size of 16 byte. */
>>
>> typedef struct ArpKey {
>>     ovs_be32 nwSrc;              /* IPv4 source address. */
>>@@ -97,6 +95,7 @@ typedef struct Ipv6Key {
>>     uint8_t nwTtl;               /* IP TTL/Hop Limit. */
>>     uint8_t nwFrag;              /* FLOW_FRAG_* flags. */
>>     L4Key  l4;
>>+    uint32_t pad;
>> } Ipv6Key;  /* Size of 48 byte. */
>>
>> typedef struct Icmp6Key {
>>@@ -111,7 +110,7 @@ typedef struct Icmp6Key {
>>     uint8_t arpSha[6];           /* ARP/ND source hardware address. */
>>     uint8_t arpTha[6];           /* ARP/ND target hardware address. */
>>     struct in6_addr ndTarget;    /* IPv6 neighbor discovery (ND) target.
>>*/
>>-} Icmp6Key; /* Size of 76 byte. */
>>+} Icmp6Key; /* Size of 72 byte. */
>>
>> typedef struct L2Key {
>>     uint32_t inPort;             /* Port number of input port. */
>>diff --git a/datapath-windows/ovsext/Flow.c
>>b/datapath-windows/ovsext/Flow.c
>>index ca6a353..b629c93 100644
>>--- a/datapath-windows/ovsext/Flow.c
>>+++ b/datapath-windows/ovsext/Flow.c
>>@@ -980,7 +980,6 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey
>>*ipv4FlowPutKey)
>>     switch (ipv4Key.ipv4_proto) {
>>         case IPPROTO_TCP: {
>>             struct ovs_key_tcp tcpKey;
>>-            UINT16 tcpFlags = 0;
>>             tcpKey.tcp_src = ipv4FlowPutKey->l4.tpSrc;
>>             tcpKey.tcp_dst = ipv4FlowPutKey->l4.tpDst;
>>             if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP,
>>@@ -989,13 +988,6 @@ _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey
>>*ipv4FlowPutKey)
>>                 rc = STATUS_UNSUCCESSFUL;
>>                 goto done;
>>             }
>>-            tcpFlags = ipv4FlowPutKey->l4.flags;
>>-            if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
>>-                                   (PCHAR)(&tcpFlags),
>>-                                    sizeof(tcpFlags))) {
>>-                rc = STATUS_UNSUCCESSFUL;
>>-                goto done;
>>-            }
>>             break;
>>         }
>>
>>@@ -1082,7 +1074,6 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key
>>*ipv6FlowPutKey,
>>     switch (ipv6Key.ipv6_proto) {
>>         case IPPROTO_TCP: {
>>             struct ovs_key_tcp tcpKey;
>>-            UINT16 tcpFlags = 0;
>>             tcpKey.tcp_src = ipv6FlowPutKey->l4.tpSrc;
>>             tcpKey.tcp_dst = ipv6FlowPutKey->l4.tpDst;
>>             if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP,
>>@@ -1091,13 +1082,6 @@ _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key
>>*ipv6FlowPutKey,
>>                 rc = STATUS_UNSUCCESSFUL;
>>                 goto done;
>>             }
>>-            tcpFlags = ipv6FlowPutKey->l4.flags;
>>-            if (!NlMsgPutTailUnspec(nlBuf, OVS_KEY_ATTR_TCP_FLAGS,
>>-                                   (PCHAR)(&tcpFlags),
>>-                                   sizeof(tcpFlags))) {
>>-                rc = STATUS_UNSUCCESSFUL;
>>-                goto done;
>>-            }
>>             break;
>>         }
>>
>>@@ -1373,12 +1357,6 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
>>                 ipv4FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
>>             }
>>
>>-            if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
>>-                const UINT16 *flags;
>>-                flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
>>-                ipv4FlowPutKey->l4.flags = *flags;
>>-            }
>>-
>>             if (keyAttrs[OVS_KEY_ATTR_UDP]) {
>>                 const struct ovs_key_udp *udpKey;
>>                 udpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_UDP]);
>>@@ -1423,12 +1401,6 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
>>                 ipv6FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
>>             }
>>
>>-            if (keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]) {
>>-                const UINT16 *flags;
>>-                flags = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP_FLAGS]);
>>-                ipv6FlowPutKey->l4.flags = *flags;
>>-            }
>>-
>>             if (keyAttrs[OVS_KEY_ATTR_UDP]) {
>>                 const struct ovs_key_udp *udpKey;
>>                 udpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_UDP]);
>>@@ -1471,6 +1443,8 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
>>
>>                 destKey->l2.keyLen += OVS_IPV6_KEY_SIZE;
>>             }
>>+
>>+            ipv6FlowPutKey->pad = 0;
>>         }
>>         break;
>>     }
>>@@ -1491,6 +1465,9 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
>>             /* Kernel datapath assumes 'arpFlowPutKey->nwProto' to be in
>>host
>>              * order. */
>>             arpFlowPutKey->nwProto = (UINT8)ntohs((arpKey->arp_op));
>>+            arpFlowPutKey->pad[0] = 0;
>>+            arpFlowPutKey->pad[1] = 0;
>>+            arpFlowPutKey->pad[2] = 0;
>>             destKey->l2.keyLen += OVS_ARP_KEY_SIZE;
>>             break;
>>         }
>>@@ -1658,7 +1635,7 @@ OvsFlowUsed(OvsFlow *flow,
>>     flow->used = tickCount.QuadPart * ovsTimeIncrementPerTick;
>>     flow->packetCount++;
>>     flow->byteCount += OvsPacketLenNBL(packet);
>>-    flow->tcpFlags |= OvsGetTcpFlags(packet, layers);
>>+    flow->tcpFlags |= OvsGetTcpFlags(packet, &flow->key, layers);
>> }
>>
>>
>>@@ -1819,7 +1796,6 @@ OvsExtractFlow(const NET_BUFFER_LIST *packet,
>>             ipKey->nwTtl = nh->ttl;
>>             ipKey->l4.tpSrc = 0;
>>             ipKey->l4.tpDst = 0;
>>-            ipKey->l4.flags = 0;
>>
>>             if (!(nh->frag_off & htons(IP_OFFSET))) {
>>                 if (ipKey->nwProto == SOCKET_IPPROTO_TCP) {
>>@@ -1853,7 +1829,7 @@ OvsExtractFlow(const NET_BUFFER_LIST *packet,
>>         layers->isIPv6 = 1;
>>         flow->ipv6Key.l4.tpSrc = 0;
>>         flow->ipv6Key.l4.tpDst = 0;
>>-        flow->ipv6Key.l4.flags = 0;
>>+        flow->ipv6Key.pad = 0;
>>
>>         if (flow->ipv6Key.nwProto == SOCKET_IPPROTO_TCP) {
>>             OvsParseTcp(packet, &(flow->ipv6Key.l4), layers);
>>@@ -1934,7 +1910,9 @@ AddFlow(OVS_DATAPATH *datapath, OvsFlow *flow)
>>      */
>>     KeMemoryBarrier();
>>
>>+    //KeAcquireSpinLock(&FilterDeviceExtension->NblQueueLock, &oldIrql);
>>     InsertTailList(head, &flow->ListEntry);
>>+    //KeReleaseSpinLock(&FilterDeviceExtension->NblQueueLock, oldIrql);
>>
>>     datapath->nFlows++;
>>
>>diff --git a/datapath-windows/ovsext/Flow.h
>>b/datapath-windows/ovsext/Flow.h
>>index 5d74ca8..e817bcf 100644
>>--- a/datapath-windows/ovsext/Flow.h
>>+++ b/datapath-windows/ovsext/Flow.h
>>@@ -28,7 +28,7 @@ typedef struct _OvsFlow {
>>     OvsFlowKey key;
>>     UINT64 hash;
>>     UINT32 actionsLen;
>>-    UINT8  tcpFlags;
>>+    UINT8 tcpFlags;
>>     UINT64 used;
>>     UINT64 packetCount;
>>     UINT64 byteCount;
>>diff --git a/datapath-windows/ovsext/PacketParser.c
>>b/datapath-windows/ovsext/PacketParser.c
>>index 219ddac..e01be17 100644
>>--- a/datapath-windows/ovsext/PacketParser.c
>>+++ b/datapath-windows/ovsext/PacketParser.c
>>@@ -186,7 +186,6 @@ OvsParseTcp(const NET_BUFFER_LIST *packet,
>>     if (tcp) {
>>         flow->tpSrc = tcp->source;
>>         flow->tpDst = tcp->dest;
>>-        flow->flags = OvsGetTcpFlags(packet, layers);
>>         layers->isTcp = 1;
>>         layers->l7Offset = layers->l4Offset + 4 * tcp->doff;
>>     }
>>diff --git a/datapath-windows/ovsext/PacketParser.h
>>b/datapath-windows/ovsext/PacketParser.h
>>index 765819a..55d110f 100644
>>--- a/datapath-windows/ovsext/PacketParser.h
>>+++ b/datapath-windows/ovsext/PacketParser.h
>>@@ -75,8 +75,11 @@ OvsGetTcpCtl(const NET_BUFFER_LIST *packet, // IN
>>
>> static UINT8
>> OvsGetTcpFlags(const NET_BUFFER_LIST *packet,    // IN
>>+               const OvsFlowKey *key,   // IN
>>                const POVS_PACKET_HDR_INFO layers) // IN
>> {
>>+    UNREFERENCED_PARAMETER(key); // should be removed later
>>+
>>     if (layers->isTcp) {
>>         return TCP_FLAGS(OvsGetTcpCtl(packet, layers));
>>     } else {
>>--
>>1.9.5.msysgit.0
>>_______________________________________________
>>dev mailing list
>>dev at openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=PpEXaHKvxOEM9VE7QT2sG2bdjbOL-s
>>DlNxCp44KuFxQ&s=7ontvSn37R6yJC1SDPYz1ZeC1bTUM2KDXUKJNgV1t5o&e=
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list