[ovs-dev] [PATCH] datapath-windows: Trigger conntrack event after setting mark and label

[Sairam Venugopal]

Hi Shashank,

Can you explain this? The intent of the patch is not to confirm if an entry exists. Instead it is to track if an entry is created as part of the current CT action.

Not all CT actions trigger an Entry Creation. If an entry is created, then we need to delay the event generation until Mark and Label are present.

Thanks,
Sairam

From: Shashank Ram
Date: Thursday, March 2, 2017 at 2:05 PM
To: Sairam Venugopal, "dev at openvswitch.org<mailto:dev at openvswitch.org>"
Subject: Re: [ovs-dev] [PATCH] datapath-windows: Trigger conntrack event after setting mark and label


Thanks Sai for the change.


I was wondering if you could add a helper function to lookup the OVS_CT_ENTRY that got added to the OvsConntrackKeyLookupCtx list, instead of passing a boolean around to functions.

i.e replace "entryCreated" with "IsEntryCreated()".


Thanks,

Shashank

________________________________
From: ovs-dev-bounces at openvswitch.org<mailto:ovs-dev-bounces at openvswitch.org> <ovs-dev-bounces at openvswitch.org<mailto:ovs-dev-bounces at openvswitch.org>> on behalf of Sairam Venugopal <vsairam at vmware.com<mailto:vsairam at vmware.com>>
Sent: Thursday, March 2, 2017 1:41:03 PM
To: dev at openvswitch.org<mailto:dev at openvswitch.org>
Subject: [ovs-dev] [PATCH] datapath-windows: Trigger conntrack event after setting mark and label

New Conntrack Entry event should be triggered after setting the mark and
label fields. The current RW lock implementation prevents Event Handler
from reading the entry until mark/label is set.

Fixing the workflow to trigger the event after setting mark/label.

Signed-off-by: Sairam Venugopal <vsairam at vmware.com<mailto:vsairam at vmware.com>>
---
 datapath-windows/ovsext/Conntrack.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index d1be480..9f41861 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -168,7 +168,7 @@ OvsCtAddEntry(POVS_CT_ENTRY entry, OvsConntrackKeyLookupCtx *ctx, UINT64 now)
     entry->timestampStart = now;
     InsertHeadList(&ovsConntrackTable[ctx->hash & CT_HASH_TABLE_MASK],
                    &entry->link);
-    OvsPostCtEventEntry(entry, OVS_EVENT_CT_NEW);
+
     ctTotalEntries++;
 }

@@ -179,9 +179,11 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
                  OvsConntrackKeyLookupCtx *ctx,
                  OvsFlowKey *key,
                  BOOLEAN commit,
-                 UINT64 currentTime)
+                 UINT64 currentTime,
+                 BOOLEAN *entryCreated)
 {
     POVS_CT_ENTRY entry = NULL;
+    *entryCreated = FALSE;
     UINT32 state = 0;
     switch (ipProto)
     {
@@ -211,6 +213,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
                     entry->parent = parentEntry;
                 }
                 OvsCtAddEntry(entry, ctx, currentTime);
+                *entryCreated = TRUE;
             }

             OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
@@ -232,6 +235,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
                     return NULL;
                 }
                 OvsCtAddEntry(entry, ctx, currentTime);
+                *entryCreated = TRUE;
             }

             OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
@@ -246,6 +250,7 @@ OvsCtEntryCreate(PNET_BUFFER_LIST curNbl,
                     return NULL;
                 }
                 OvsCtAddEntry(entry, ctx, currentTime);
+                *entryCreated = TRUE;
             }

             OvsCtUpdateFlowKey(key, state, ctx->key.zone, 0, NULL);
@@ -525,10 +530,12 @@ OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl,
                          OvsFlowKey *key,
                          UINT16 zone,
                          BOOLEAN commit,
-                         UINT64 currentTime)
+                         UINT64 currentTime,
+                         BOOLEAN *entryCreated)
 {
     POVS_CT_ENTRY entry = ctx->entry;
     UINT32 state = 0;
+    *entryCreated = FALSE;

     /* If an entry was found, update the state based on TCP flags */
     if (ctx->related) {
@@ -555,7 +562,8 @@ OvsProcessConntrackEntry(PNET_BUFFER_LIST curNbl,
             OvsCtEntryDelete(ctx->entry);
             ctx->entry = NULL;
             entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto, l4Offset,
-                                     ctx, key, commit, currentTime);
+                                     ctx, key, commit, currentTime,
+                                     entryCreated);
             if (!entry) {
                 return NULL;
             }
@@ -644,17 +652,19 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,

     /* Lookup Conntrack entries for a matching entry */
     entry = OvsCtLookup(&ctx);
-
+    BOOLEAN entryCreated = FALSE;
     if (!entry) {
         /* If no matching entry was found, create one and add New state */
         entry = OvsCtEntryCreate(curNbl, key->ipKey.nwProto,
                                  layers->l4Offset, &ctx,
-                                 key, commit, currentTime);
+                                 key, commit, currentTime,
+                                 &entryCreated);
     } else {
         /* Process the entry and update CT flags */
         OvsCtIncrementCounters(entry, ctx.reply, curNbl);
         entry = OvsProcessConntrackEntry(curNbl, layers->l4Offset, &ctx, key,
-                                         zone, commit, currentTime);
+                                         zone, commit, currentTime,
+                                         &entryCreated);
     }

     if (entry && mark) {
@@ -676,6 +686,10 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
         }
     }

+    if (entryCreated && entry) {
+        OvsPostCtEventEntry(entry, OVS_EVENT_CT_NEW);
+    }
+
     NdisReleaseRWLock(ovsConntrackLockObj, &lockState);

     return status;
--
2.9.0.windows.1

_______________________________________________
dev mailing list
dev at openvswitch.org<mailto:dev at openvswitch.org>
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=6OuVHk-mnufSWzkKa74UkQ&m=YbTu7StQZmOxMTQuBv5LEbdDy9ps5zsmOdxKgCDjqNE&s=ujIWU7NjUI4GmgsI2JkdNCZhbZHw70sg5xYTgTdvgQg&e=