From: Numan Siddique <[email protected]>

Having zone id per datapath is more than sufficient, because the
CT tuple information will be unique anyway with in the logical
datapath.

In our testing we have observed that, the packet between two ports of
a datapath within the same chassis is sent to the CT twice (both in
ingress and egress pipeline) with (2 different zone ids) resulting in
some performance hit. With this patch, the packet will use the same
zone id. This doesn't improve the performace, but a future patch may
optimize this scenario by sending the packet to CT only once.

Signed-off-by: Numan Siddique <[email protected]>
---
 ovn/controller/ovn-controller.8.xml |  8 ++++----
 ovn/controller/ovn-controller.c     | 17 +++++++----------
 ovn/controller/physical.c           |  6 ++++--
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/ovn/controller/ovn-controller.8.xml 
b/ovn/controller/ovn-controller.8.xml
index c92fd55..b8bec6c 100644
--- a/ovn/controller/ovn-controller.8.xml
+++ b/ovn/controller/ovn-controller.8.xml
@@ -209,14 +209,14 @@
         <code>external_ids:ct-zone-*</code> in the <code>Bridge</code> table
       </dt>
       <dd>
-        Logical ports and gateway routers are assigned a connection
+        Logical switch and router datapaths are assigned a connection
         tracking zone by <code>ovn-controller</code> for stateful
         services.  To keep state across restarts of
         <code>ovn-controller</code>, these keys are stored in the
         integration bridge's Bridge table.  The name contains a prefix
         of <code>ct-zone-</code> followed by the name of the logical
-        port or gateway router's zone key.  The value for this key
-        identifies the zone used for this port.
+        datapath's zone key.  The value for this key identifies the zone used
+        for the datapath.
       </dd>
 
       <dt>
@@ -309,7 +309,7 @@
 
       <dt><code>ct-zone-list</code></dt>
       <dd>
-        Lists each local logical port and its connection tracking zone.
+        Lists each local logical datapath and its connection tracking zone.
       </dd>
 
       <dt><code>inject-pkt</code> <var>microflow</var></dt>
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index ea299da..696723d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -307,7 +307,7 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
 }
 
 static void
-update_ct_zones(struct sset *lports, const struct hmap *local_datapaths,
+update_ct_zones(const struct hmap *local_datapaths,
                 struct simap *ct_zones, unsigned long *ct_zone_bitmap,
                 struct shash *pending_ct_zones)
 {
@@ -316,15 +316,15 @@ update_ct_zones(struct sset *lports, const struct hmap 
*local_datapaths,
     const char *user;
     struct sset all_users = SSET_INITIALIZER(&all_users);
 
-    SSET_FOR_EACH(user, lports) {
-        sset_add(&all_users, user);
-    }
-
     /* Local patched datapath (gateway routers) need zones assigned. */
     const struct local_datapath *ld;
     HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
         /* XXX Add method to limit zone assignment to logical router
          * datapaths with NAT */
+        char *dp_user = xasprintf(UUID_FMT,
+                                  UUID_ARGS(&ld->datapath->header_.uuid));
+        sset_add(&all_users, dp_user);
+        free(dp_user);
         char *dnat = alloc_nat_zone_key(&ld->datapath->header_.uuid, "dnat");
         char *snat = alloc_nat_zone_key(&ld->datapath->header_.uuid, "snat");
         sset_add(&all_users, dnat);
@@ -350,10 +350,7 @@ update_ct_zones(struct sset *lports, const struct hmap 
*local_datapaths,
         }
     }
 
-    /* xxx This is wasteful to assign a zone to each port--even if no
-     * xxx security policy is applied. */
-
-    /* Assign a unique zone id for each logical port and two zones
+    /* Assign a unique zone id for each local datapath and two zones
      * to a gateway router. */
     SSET_FOR_EACH(user, &all_users) {
         int zone;
@@ -616,7 +613,7 @@ main(int argc, char *argv[])
                                                          &pending_ct_zones);
 
             pinctrl_run(&ctx, &lports, br_int, chassis, &local_datapaths);
-            update_ct_zones(&local_lports, &local_datapaths, &ct_zones,
+            update_ct_zones(&local_datapaths, &ct_zones,
                             ct_zone_bitmap, &pending_ct_zones);
             if (ctx.ovs_idl_txn) {
 
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 0f1aa63..7d44485 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -167,8 +167,10 @@ get_zone_ids(const struct sbrec_port_binding *binding,
 {
     struct zone_ids zone_ids;
 
-    zone_ids.ct = simap_get(ct_zones, binding->logical_port);
-
+    char *dp_uuid = xasprintf(UUID_FMT,
+                              UUID_ARGS(&binding->datapath->header_.uuid));
+    zone_ids.ct = simap_get(ct_zones, dp_uuid);
+    free(dp_uuid);
     const struct uuid *key = &binding->datapath->header_.uuid;
 
     char *dnat = alloc_nat_zone_key(key, "dnat");
-- 
2.9.3

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to