Enforce the CT limit protection, it ensures that
any CT limit value that was set by forced operation,
currently the DB CT limit, will be protected against
overwrite from other sources, e.g. the dpctl command.

Signed-off-by: Ales Musil <amu...@redhat.com>
---
 lib/conntrack.c         | 51 ++++++++++++++++++++++++++---------------
 lib/conntrack.h         |  5 ++--
 lib/ct-dpif.c           |  9 ++++----
 lib/ct-dpif.h           |  5 ++--
 lib/dpctl.c             |  4 ++--
 lib/dpif-netdev.c       | 12 ++++++----
 lib/dpif-netlink.c      | 37 ++++++++++++++++++++++++++----
 lib/dpif-provider.h     | 13 +++++++----
 ofproto/ofproto-dpif.c  |  6 +++--
 tests/system-traffic.at | 28 ++++++++++++++++++++++
 10 files changed, 126 insertions(+), 44 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 47a443fba..b5b5d4a4c 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -85,6 +85,7 @@ enum ct_alg_ctl_type {
 struct zone_limit {
     struct cmap_node node;
     struct conntrack_zone_limit czl;
+    bool limit_protected;
 };
 
 static bool conn_key_extract(struct conntrack *, struct dp_packet *,
@@ -344,17 +345,13 @@ zone_limit_get(struct conntrack *ct, int32_t zone)
 }
 
 static int
-zone_limit_create(struct conntrack *ct, int32_t zone, uint32_t limit)
+zone_limit_create(struct conntrack *ct, int32_t zone, uint32_t limit,
+                  bool limit_protected)
     OVS_REQUIRES(ct->ct_lock)
 {
-    struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
-
-    if (zl) {
-        return 0;
-    }
-
     if (zone >= DEFAULT_ZONE && zone <= MAX_ZONE) {
-        zl = xzalloc(sizeof *zl);
+        struct zone_limit *zl = xzalloc(sizeof *zl);
+        zl->limit_protected = limit_protected;
         zl->czl.limit = limit;
         zl->czl.zone = zone;
         zl->czl.zone_limit_seq = ct->zone_limit_seq++;
@@ -366,18 +363,28 @@ zone_limit_create(struct conntrack *ct, int32_t zone, 
uint32_t limit)
     }
 }
 
+static inline bool
+can_update_zone_limit(struct zone_limit *zl, bool force)
+{
+    return !(zl && zl->limit_protected && !force);
+}
+
 int
-zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit)
+zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit,
+                  bool force)
 {
     int err = 0;
-    struct zone_limit *zl = zone_limit_lookup(ct, zone);
-    if (zl) {
+    ovs_mutex_lock(&ct->ct_lock);
+
+    struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
+    if (!can_update_zone_limit(zl, force)) {
+        err = EPERM;
+    } else if (zl) {
         zl->czl.limit = limit;
+        zl->limit_protected = force;
         VLOG_INFO("Changed zone limit of %u for zone %d", limit, zone);
     } else {
-        ovs_mutex_lock(&ct->ct_lock);
-        err = zone_limit_create(ct, zone, limit);
-        ovs_mutex_unlock(&ct->ct_lock);
+        err = zone_limit_create(ct, zone, limit, force);
         if (!err) {
             VLOG_INFO("Created zone limit of %u for zone %d", limit, zone);
         } else {
@@ -385,6 +392,8 @@ zone_limit_update(struct conntrack *ct, int32_t zone, 
uint32_t limit)
                       zone);
         }
     }
+
+    ovs_mutex_unlock(&ct->ct_lock);
     return err;
 }
 
@@ -398,20 +407,24 @@ zone_limit_clean(struct conntrack *ct, struct zone_limit 
*zl)
 }
 
 int
-zone_limit_delete(struct conntrack *ct, uint16_t zone)
+zone_limit_delete(struct conntrack *ct, uint16_t zone, bool force)
 {
+    int err = 0;
     ovs_mutex_lock(&ct->ct_lock);
+
     struct zone_limit *zl = zone_limit_lookup_protected(ct, zone);
-    if (zl) {
+    if (!can_update_zone_limit(zl, force)) {
+        err = EPERM;
+    } else if (zl) {
         zone_limit_clean(ct, zl);
-        ovs_mutex_unlock(&ct->ct_lock);
         VLOG_INFO("Deleted zone limit for zone %d", zone);
     } else {
-        ovs_mutex_unlock(&ct->ct_lock);
         VLOG_INFO("Attempted delete of non-existent zone limit: zone %d",
                   zone);
     }
-    return 0;
+
+    ovs_mutex_unlock(&ct->ct_lock);
+    return err;
 }
 
 static void
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 57d5159b6..a58a800f9 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -153,7 +153,8 @@ bool conntrack_get_tcp_seq_chk(struct conntrack *ct);
 struct ipf *conntrack_ipf_ctx(struct conntrack *ct);
 struct conntrack_zone_limit zone_limit_get(struct conntrack *ct,
                                            int32_t zone);
-int zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit);
-int zone_limit_delete(struct conntrack *ct, uint16_t zone);
+int zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit,
+                      bool force);
+int zone_limit_delete(struct conntrack *ct, uint16_t zone, bool force);
 
 #endif /* conntrack.h */
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index f59c6e560..335ba09f9 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -399,11 +399,11 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
 
 int
 ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
-                   const struct ovs_list *zone_limits)
+                   const struct ovs_list *zone_limits, bool force)
 {
     return (dpif->dpif_class->ct_set_limits
             ? dpif->dpif_class->ct_set_limits(dpif, default_limit,
-                                              zone_limits)
+                                              zone_limits, force)
             : EOPNOTSUPP);
 }
 
@@ -420,10 +420,11 @@ ct_dpif_get_limits(struct dpif *dpif, uint32_t 
*default_limit,
 }
 
 int
-ct_dpif_del_limits(struct dpif *dpif, const struct ovs_list *zone_limits)
+ct_dpif_del_limits(struct dpif *dpif, const struct ovs_list *zone_limits,
+                   bool force)
 {
     return (dpif->dpif_class->ct_del_limits
-            ? dpif->dpif_class->ct_del_limits(dpif, zone_limits)
+            ? dpif->dpif_class->ct_del_limits(dpif, zone_limits, force)
             : EOPNOTSUPP);
 }
 
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 0b728b529..0b74ec463 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -308,10 +308,11 @@ int ct_dpif_get_nconns(struct dpif *dpif, uint32_t 
*nconns);
 int ct_dpif_set_tcp_seq_chk(struct dpif *dpif, bool enabled);
 int ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled);
 int ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
-                       const struct ovs_list *);
+                       const struct ovs_list *, bool enforced);
 int ct_dpif_get_limits(struct dpif *dpif, uint32_t *default_limit,
                        const struct ovs_list *, struct ovs_list *);
-int ct_dpif_del_limits(struct dpif *dpif, const struct ovs_list *);
+int ct_dpif_del_limits(struct dpif *dpif, const struct ovs_list *,
+                       bool enforced);
 int ct_dpif_sweep(struct dpif *, uint32_t *ms);
 int ct_dpif_ipf_set_enabled(struct dpif *, bool v6, bool enable);
 int ct_dpif_ipf_set_min_frag(struct dpif *, bool v6, uint32_t min_frag);
diff --git a/lib/dpctl.c b/lib/dpctl.c
index cd12625a1..e498c29ce 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -2233,7 +2233,7 @@ dpctl_ct_set_limits(int argc, const char *argv[],
         ct_dpif_push_zone_limit(&zone_limits, zone, limit, 0);
     }
 
-    error = ct_dpif_set_limits(dpif, p_default_limit, &zone_limits);
+    error = ct_dpif_set_limits(dpif, p_default_limit, &zone_limits, false);
     if (!error) {
         ct_dpif_free_zone_limits(&zone_limits);
         dpif_close(dpif);
@@ -2300,7 +2300,7 @@ dpctl_ct_del_limits(int argc, const char *argv[],
         goto error;
     }
 
-    error = ct_dpif_del_limits(dpif, &zone_limits);
+    error = ct_dpif_del_limits(dpif, &zone_limits, false);
     if (!error) {
         goto out;
     } else {
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 157694bcf..90a48baa6 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -9447,12 +9447,14 @@ dpif_netdev_ct_get_sweep_interval(struct dpif *dpif, 
uint32_t *ms)
 static int
 dpif_netdev_ct_set_limits(struct dpif *dpif,
                            const uint32_t *default_limits,
-                           const struct ovs_list *zone_limits)
+                           const struct ovs_list *zone_limits,
+                           bool force)
 {
     int err = 0;
     struct dp_netdev *dp = get_dp_netdev(dpif);
     if (default_limits) {
-        err = zone_limit_update(dp->conntrack, DEFAULT_ZONE, *default_limits);
+        err = zone_limit_update(dp->conntrack, DEFAULT_ZONE, *default_limits,
+                                force);
         if (err != 0) {
             return err;
         }
@@ -9461,7 +9463,7 @@ dpif_netdev_ct_set_limits(struct dpif *dpif,
     struct ct_dpif_zone_limit *zone_limit;
     LIST_FOR_EACH (zone_limit, node, zone_limits) {
         err = zone_limit_update(dp->conntrack, zone_limit->zone,
-                                zone_limit->limit);
+                                zone_limit->limit, force);
         if (err != 0) {
             break;
         }
@@ -9512,13 +9514,13 @@ dpif_netdev_ct_get_limits(struct dpif *dpif,
 
 static int
 dpif_netdev_ct_del_limits(struct dpif *dpif,
-                           const struct ovs_list *zone_limits)
+                          const struct ovs_list *zone_limits, bool force)
 {
     int err = 0;
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct ct_dpif_zone_limit *zone_limit;
     LIST_FOR_EACH (zone_limit, node, zone_limits) {
-        err = zone_limit_delete(dp->conntrack, zone_limit->zone);
+        err = zone_limit_delete(dp->conntrack, zone_limit->zone, force);
         if (err != 0) {
             break;
         }
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 9194971d3..8ef5ce87f 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -250,6 +250,10 @@ static int ovs_ct_limit_family;
  * Initialized by dpif_netlink_init(). */
 static unsigned int ovs_vport_mcgroup;
 
+/* CT limit protection, must be global for all 'struct dpif_netlink'
+ * instances. */
+static unsigned long ct_limit_protection[BITMAP_N_LONGS(UINT16_MAX)] = {0};
+
 /* If true, tunnel devices are created using OVS compat/genetlink.
  * If false, tunnel devices are created with rtnetlink and using light weight
  * tunnels. If we fail to create the tunnel the rtnetlink+LWT, then we fallback
@@ -3358,15 +3362,35 @@ dpif_netlink_ct_flush(struct dpif *dpif OVS_UNUSED, 
const uint16_t *zone,
     }
 }
 
+static int
+update_zone_limit_protection(const struct ovs_list *limits, bool force)
+{
+    struct ct_dpif_zone_limit *zone_limit;
+    LIST_FOR_EACH (zone_limit, node, limits) {
+        if (bitmap_is_set(ct_limit_protection, zone_limit->zone) &&
+            !force) {
+            return EPERM;
+        }
+        bitmap_set(ct_limit_protection, zone_limit->zone, force);
+    }
+
+    return 0;
+}
+
 static int
 dpif_netlink_ct_set_limits(struct dpif *dpif OVS_UNUSED,
                            const uint32_t *default_limits,
-                           const struct ovs_list *zone_limits)
+                           const struct ovs_list *zone_limits, bool force)
 {
     if (ovs_ct_limit_family < 0) {
         return EOPNOTSUPP;
     }
 
+    int err = update_zone_limit_protection(zone_limits, force);
+    if (err) {
+        return err;
+    }
+
     struct ofpbuf *request = ofpbuf_new(NL_DUMP_BUFSIZE);
     nl_msg_put_genlmsghdr(request, 0, ovs_ct_limit_family,
                           NLM_F_REQUEST | NLM_F_ECHO, OVS_CT_LIMIT_CMD_SET,
@@ -3399,7 +3423,7 @@ dpif_netlink_ct_set_limits(struct dpif *dpif OVS_UNUSED,
     }
     nl_msg_end_nested(request, opt_offset);
 
-    int err = nl_transact(NETLINK_GENERIC, request, NULL);
+    err = nl_transact(NETLINK_GENERIC, request, NULL);
     ofpbuf_delete(request);
     return err;
 }
@@ -3508,12 +3532,17 @@ out:
 
 static int
 dpif_netlink_ct_del_limits(struct dpif *dpif OVS_UNUSED,
-                           const struct ovs_list *zone_limits)
+                           const struct ovs_list *zone_limits, bool force)
 {
     if (ovs_ct_limit_family < 0) {
         return EOPNOTSUPP;
     }
 
+    int err = update_zone_limit_protection(zone_limits, force);
+    if (err) {
+        return err;
+    }
+
     struct ofpbuf *request = ofpbuf_new(NL_DUMP_BUFSIZE);
     nl_msg_put_genlmsghdr(request, 0, ovs_ct_limit_family,
             NLM_F_REQUEST | NLM_F_ECHO, OVS_CT_LIMIT_CMD_DEL,
@@ -3537,7 +3566,7 @@ dpif_netlink_ct_del_limits(struct dpif *dpif OVS_UNUSED,
         nl_msg_end_nested(request, opt_offset);
     }
 
-    int err = nl_transact(NETLINK_GENERIC, request, NULL);
+    err = nl_transact(NETLINK_GENERIC, request, NULL);
 
     ofpbuf_delete(request);
     return err;
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 1b822cb07..6c292856f 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -521,9 +521,11 @@ struct dpif_class {
     /* Sets the max connections allowed per zone according to 'zone_limits',
      * a list of 'struct ct_dpif_zone_limit' entries (the 'count' member
      * is not used when setting limits).  If 'default_limit' is not NULL,
-     * modifies the default limit to '*default_limit'. */
+     * modifies the default limit to '*default_limit'. If 'force' is set
+     * to 'true' it will overwrite current configuration, otherwise it can
+     * return 'EPERM' if the limit is already enforced for this zone. */
     int (*ct_set_limits)(struct dpif *, const uint32_t *default_limit,
-                         const struct ovs_list *zone_limits);
+                         const struct ovs_list *zone_limits, bool force);
 
     /* Looks up the default per zone limit and stores that in
      * 'default_limit'.  Look up the per zone limits for all zones in
@@ -536,8 +538,11 @@ struct dpif_class {
                          struct ovs_list *zone_limits_out);
 
     /* Deletes per zone limit of all zones specified in 'zone_limits', a
-     * list of 'struct ct_dpif_zone_limit' entries. */
-    int (*ct_del_limits)(struct dpif *, const struct ovs_list *zone_limits);
+     * list of 'struct ct_dpif_zone_limit' entries. If 'force' is set
+     * to 'true' it will remove current configuration, otherwise it
+     * returns 'EPERM' if the limit is already enforced for this zone.*/
+    int (*ct_del_limits)(struct dpif *, const struct ovs_list *zone_limits,
+                         bool force);
 
     /* Connection tracking timeout policy */
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 55eaeefa3..96149ad8d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5653,12 +5653,14 @@ static void
 ct_zone_limits_commit(struct dpif_backer *backer)
 {
     if (!ovs_list_is_empty(&backer->ct_zone_limits_to_add)) {
-        ct_dpif_set_limits(backer->dpif, NULL, &backer->ct_zone_limits_to_add);
+        ct_dpif_set_limits(backer->dpif, NULL, &backer->ct_zone_limits_to_add,
+                           true);
         ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_add);
     }
 
     if (!ovs_list_is_empty(&backer->ct_zone_limits_to_del)) {
-        ct_dpif_del_limits(backer->dpif, &backer->ct_zone_limits_to_del);
+        ct_dpif_del_limits(backer->dpif, &backer->ct_zone_limits_to_del,
+                           true);
         ct_dpif_free_zone_limits(&backer->ct_zone_limits_to_del);
     }
 }
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 537db66e0..8eb609675 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5238,6 +5238,34 @@ OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/ct-get-limits], 
[dnl
 default limit=10
 zone=0,limit=3,count=0])
 
+dnl Try to overwrite the zone limit via dpctl command.
+AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=0,limit=5], [2], [ignore], 
[ignore])
+
+AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl
+default limit=10
+zone=0,limit=3,count=0
+])
+
+AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=0], [2], [ignore], [ignore])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl
+default limit=10
+zone=0,limit=3,count=0
+])
+
+dnl Set limit for zone that is not in DB via dpctl command.
+AT_CHECK([ovs-appctl dpctl/ct-set-limits zone=1,limit=5])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl
+default limit=10
+zone=0,limit=3,count=0
+zone=1,limit=5,count=0
+])
+
+AT_CHECK([ovs-appctl dpctl/ct-del-limits zone=1])
+AT_CHECK([ovs-appctl dpctl/ct-get-limits], [0], [dnl
+default limit=10
+zone=0,limit=3,count=0
+])
+
 OVS_TRAFFIC_VSWITCHD_STOP(["dnl
 /could not create datapath/d
 /(Cannot allocate memory) on packet/d
-- 
2.41.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to