Re: [ovs-dev] [PATCH] datapath-windows: Increment ct packet counters based on ct_state.

[Shashank Ram]

Comments inline.



________________________________________
Subject: [ovs-dev] [PATCH] datapath-windows: Increment ct packet counters       
based on ct_state.

For a given packet, packet counters in conntrack should be accounted only
once, even if the packet is processed multiple times by conntrack.

When a packet is processed by conntrack, ct_state flag is set to
OVS_CS_F_TRACKED. Use this state to identify if a packet has been
processed previously by conntrack.

Also update the ct packet counters when ct entry is created.

With this patch, the conntrack's packet counters behavior is similar
to linux

Signed-off-by: Anand Kumar <kumaran...@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 8bcda05..0adb6d5 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -169,6 +169,18 @@ OvsPostCtEventEntry(POVS_CT_ENTRY entry, UINT8 type)
     OvsPostCtEvent(&ctEventEntry);
 }

+static __inline VOID
+OvsCtIncrementCounters(POVS_CT_ENTRY entry, BOOLEAN reply, PNET_BUFFER_LIST 
nbl)
Function parameters should be on separate lines

+{
+    if (reply) {
+        entry->rev_key.byteCount+= OvsPacketLenNBL(nbl);
+        entry->rev_key.packetCount++;
+    } else {
+        entry->key.byteCount += OvsPacketLenNBL(nbl);
+        entry->key.packetCount++;
+    }
+}
+
 static __inline BOOLEAN
 OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx,
               PNAT_ACTION_INFO natInfo, UINT64 now)
@@ -287,6 +299,9 @@ OvsCtEntryCreate(OvsForwardingContext *fwdCtx,
     }

     OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
+    if (entry) {
+        OvsCtIncrementCounters(entry, ctx->reply, curNbl);
+    }
     return entry;
 }

@@ -382,18 +397,6 @@ OvsCtKeyAreSame(OVS_CT_KEY ctxKey, OVS_CT_KEY entryKey)
             (ctxKey.zone == entryKey.zone));
 }

-static __inline VOID
-OvsCtIncrementCounters(POVS_CT_ENTRY entry, BOOLEAN reply, PNET_BUFFER_LIST 
nbl)
-{
-    if (reply) {
-        entry->rev_key.byteCount+= OvsPacketLenNBL(nbl);
-        entry->rev_key.packetCount++;
-    } else {
-        entry->key.byteCount += OvsPacketLenNBL(nbl);
-        entry->key.packetCount++;
-    }
-}
-
 POVS_CT_ENTRY
 OvsCtLookup(OvsConntrackKeyLookupCtx *ctx)
 {
@@ -730,6 +733,12 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
         NdisReleaseRWLock(ovsConntrackLockObj, &lockState);
         OVS_LOG_ERROR("Conntrack Limit hit: %lu", ctTotalEntries);
         return NDIS_STATUS_RESOURCES;
+
+    /* Increment the counters soon after the lookup, since we set ct.state
+     * to OVS_CS_F_TRACKED after processing the ct entry.
+     */
Comments should be styled as follows
/*
  * ----
  */
+    if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) {
+        OvsCtIncrementCounters(entry, ctx.reply, curNbl);
     }

     if (!entry) {
@@ -740,7 +749,6 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
                                  &entryCreated);
     } else {
         /* Process the entry and update CT flags */
-        OvsCtIncrementCounters(entry, ctx.reply, curNbl);
         entry = OvsProcessConntrackEntry(fwdCtx, layers->l4Offset, &ctx, key,
                                          zone, natInfo, commit, currentTime,
                                          &entryCreated);
--
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev