diff mbox series

[ovs-dev] datapath-windows: Update ct stats when packet is processed by conntrack

Message ID 20180425200616.4180-1-kumaranand@vmware.com
State Changes Requested
Headers show
Series [ovs-dev] datapath-windows: Update ct stats when packet is processed by conntrack | expand

Commit Message

Anand Kumar April 25, 2018, 8:06 p.m. UTC
When ct lookup returns a matching ct entry, increment ct stats
even if the zone information in conntrack entry does not match with
flowkey.

Signed-off-by: Anand Kumar <kumaranand@vmware.com>
---
 datapath-windows/ovsext/Conntrack.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Sairam Venugopal April 26, 2018, 10:57 p.m. UTC | #1
Anand,

Thanks for the patch. Can you update the commit message to better describe the underlying issue?

Prevent conntrack-counters from getting redundantly incremented for recirculated packets.  
The counters ought to be incremented only if it's a new lookup or if it's recirculated through a different zone for the first time.

Thanks,
Sairam

On 4/25/18, 1:06 PM, "ovs-dev-bounces@openvswitch.org on behalf of Anand Kumar" <ovs-dev-bounces@openvswitch.org on behalf of kumaranand@vmware.com> wrote:

    When ct lookup returns a matching ct entry, increment ct stats
    even if the zone information in conntrack entry does not match with
    flowkey.
    
    Signed-off-by: Anand Kumar <kumaranand@vmware.com>
    ---
     datapath-windows/ovsext/Conntrack.c | 7 ++++---
     1 file changed, 4 insertions(+), 3 deletions(-)
    
    diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
    index 678bedb..add1491 100644
    --- a/datapath-windows/ovsext/Conntrack.c
    +++ b/datapath-windows/ovsext/Conntrack.c
    @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
             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.
    +    /* Increment stats for the entry if it wasn't tracked previously or
    +     * if they are on different zones
          */
    -    if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) {
    +    if (entry && (entry->key.zone != key->ct.zone ||
    +           (!(key->ct.state & OVS_CS_F_TRACKED)))) {
             OvsCtIncrementCounters(entry, ctx.reply, curNbl);
         }
     
    -- 
    2.9.3.windows.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=0j1dB0xbZjupscaOcQmDzLRgDPo5kjVp8k_-HX4w7P8&s=HdqtDC96bCuR7w2P7qVPA5skEcXSEeVqVbqefbaA5pw&e=
Anand Kumar April 27, 2018, 4:58 p.m. UTC | #2
Hi Sairam,

Sure. Will update the commit message and send out V2.

Thanks,
Anand Kumar

On 4/26/18, 3:57 PM, "Sairam Venugopal" <vsairam@vmware.com> wrote:

    Anand,
    
    Thanks for the patch. Can you update the commit message to better describe the underlying issue?
    
    Prevent conntrack-counters from getting redundantly incremented for recirculated packets.  
    The counters ought to be incremented only if it's a new lookup or if it's recirculated through a different zone for the first time.
    
    Thanks,
    Sairam
    
    On 4/25/18, 1:06 PM, "ovs-dev-bounces@openvswitch.org on behalf of Anand Kumar" <ovs-dev-bounces@openvswitch.org on behalf of kumaranand@vmware.com> wrote:
    
        When ct lookup returns a matching ct entry, increment ct stats
        even if the zone information in conntrack entry does not match with
        flowkey.
        
        Signed-off-by: Anand Kumar <kumaranand@vmware.com>
        ---
         datapath-windows/ovsext/Conntrack.c | 7 ++++---
         1 file changed, 4 insertions(+), 3 deletions(-)
        
        diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
        index 678bedb..add1491 100644
        --- a/datapath-windows/ovsext/Conntrack.c
        +++ b/datapath-windows/ovsext/Conntrack.c
        @@ -886,10 +886,11 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
                 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.
        +    /* Increment stats for the entry if it wasn't tracked previously or
        +     * if they are on different zones
              */
        -    if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) {
        +    if (entry && (entry->key.zone != key->ct.zone ||
        +           (!(key->ct.state & OVS_CS_F_TRACKED)))) {
                 OvsCtIncrementCounters(entry, ctx.reply, curNbl);
             }
         
        -- 
        2.9.3.windows.1
        
        _______________________________________________
        dev mailing list
        dev@openvswitch.org
        https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=0j1dB0xbZjupscaOcQmDzLRgDPo5kjVp8k_-HX4w7P8&s=HdqtDC96bCuR7w2P7qVPA5skEcXSEeVqVbqefbaA5pw&e=
diff mbox series

Patch

diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index 678bedb..add1491 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -886,10 +886,11 @@  OvsCtExecute_(OvsForwardingContext *fwdCtx,
         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.
+    /* Increment stats for the entry if it wasn't tracked previously or
+     * if they are on different zones
      */
-    if (entry && (!(key->ct.state & OVS_CS_F_TRACKED))) {
+    if (entry && (entry->key.zone != key->ct.zone ||
+           (!(key->ct.state & OVS_CS_F_TRACKED)))) {
         OvsCtIncrementCounters(entry, ctx.reply, curNbl);
     }