[ovs-dev] [RFC 3/3] OVN: add acl reject rule support using icmp4 action

[Lorenzo Bianconi]

Whenever the acl reject rule is hit send back an ICMPv4 destination
unreachable packet and do not handle reject rule as drop one

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
 ovn/northd/ovn-northd.c | 76 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 26 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 63ed97efb..9b7063f45 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2890,13 +2890,18 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
 
         /* Ingress and Egress Pre-ACL Table (Priority 110).
          *
-         * Not to do conntrack on ND packets. */
+         * Not to do conntrack on ND and ICMP destination
+         * unreachable packets. */
         ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "nd", "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "(nd_rs || nd_ra)",
                       "next;");
+        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+                      "ip4 && icmp4.type == 3", "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, "nd", "next;");
         ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
                       "(nd_rs || nd_ra)", "next;");
+        ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+                      "ip4 && icmp4.type == 3", "next;");
 
         /* Ingress and Egress Pre-ACL Table (Priority 100).
          *
@@ -3256,28 +3261,30 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
             struct ds match = DS_EMPTY_INITIALIZER;
             struct ds actions = DS_EMPTY_INITIALIZER;
 
-            /* XXX Need to support "reject", treat it as "drop;" for now. */
-            if (!strcmp(acl->action, "reject")) {
-                VLOG_INFO("reject is not a supported action");
-            }
-
             /* The implementation of "drop" differs if stateful ACLs are in
              * use for this datapath.  In that case, the actions differ
              * depending on whether the connection was previously committed
              * to the connection tracker with ct_commit. */
             if (has_stateful) {
                 /* If the packet is not part of an established connection, then
-                 * we can simply drop it. */
+                 * we can simply reject/drop it. */
                 ds_put_format(&match,
                               "(!ct.est || (ct.est && ct_label.blocked == 1)) "
                               "&& (%s)",
                               acl->match);
                 build_acl_log(&actions, acl);
-                ds_put_cstr(&actions, "/* drop */");
-                ovn_lflow_add_with_hint(lflows, od, stage,
-                                        acl->priority + OVN_ACL_PRI_OFFSET,
-                                        ds_cstr(&match), ds_cstr(&actions),
-                                        stage_hint);
+                if (!strcmp(acl->action, "reject")) {
+                    ds_put_format(&actions, "reg0 = 0; "
+                                  "eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
+                                  "icmp4 { outport <-> inport; %s };",
+                                  ingress ? "output;"
+                                          : "next(pipeline=ingress,table=0);");
+                } else {
+                    ds_put_cstr(&actions, "/* drop */");
+                }
+                ovn_lflow_add(lflows, od, stage,
+                              acl->priority + OVN_ACL_PRI_OFFSET,
+                              ds_cstr(&match), ds_cstr(&actions));
 
                 /* For an existing connection without ct_label set, we've
                  * encountered a policy change. ACLs previously allowed
@@ -3288,7 +3295,8 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
                  * specifying "next;", we implicitly drop the packet after
                  * updating conntrack state.  We would normally defer
                  * ct_commit() to the "stateful" stage, but since we're
-                 * dropping the packet, we go ahead and do it here. */
+                 * rejecting/dropping the packet, we go ahead and do it here.
+                 */
                 ds_clear(&match);
                 ds_clear(&actions);
                 ds_put_format(&match,
@@ -3296,22 +3304,38 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
                               acl->match);
                 ds_put_cstr(&actions, "ct_commit(ct_label=1/1); ");
                 build_acl_log(&actions, acl);
-                ds_put_cstr(&actions, "/* drop */");
-                ovn_lflow_add_with_hint(lflows, od, stage,
-                                        acl->priority + OVN_ACL_PRI_OFFSET,
-                                        ds_cstr(&match), ds_cstr(&actions),
-                                        stage_hint);
-
+                if (!strcmp(acl->action, "reject")) {
+                    ds_put_format(&match, " && ip4");
+                    ds_put_format(&actions, "reg0 = 0; "
+                                  "eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
+                                  "icmp4 { outport <-> inport; %s };",
+                                  ingress ? "output;"
+                                          : "next(pipeline=ingress,table=0);");
+                } else {
+                    ds_put_cstr(&actions, "/* drop */");
+                }
+                ovn_lflow_add(lflows, od, stage,
+                              acl->priority + OVN_ACL_PRI_OFFSET,
+                              ds_cstr(&match), ds_cstr(&actions));
             } else {
                 /* There are no stateful ACLs in use on this datapath,
-                 * so a "drop" ACL is simply the "drop" logical flow action
-                 * in all cases. */
+                 * so a "reject/drop" ACL is simply the "reject/drop"
+                 * logical flow action in all cases. */
                 build_acl_log(&actions, acl);
-                ds_put_cstr(&actions, "/* drop */");
-                ovn_lflow_add_with_hint(lflows, od, stage,
-                                        acl->priority + OVN_ACL_PRI_OFFSET,
-                                        acl->match, ds_cstr(&actions),
-                                        stage_hint);
+                if (!strcmp(acl->action, "reject")) {
+                    ds_put_format(&match, "(%s) && ip4", acl->match);
+                    ds_put_format(&actions,
+                                  "eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
+                                  "icmp4 { outport <-> inport; %s };",
+                                  ingress ? "output;"
+                                          : "next(pipeline=ingress,table=0);");
+                } else {
+                    ds_put_cstr(&match, acl->match);
+                    ds_put_cstr(&actions, "/* drop */");
+               }
+               ovn_lflow_add(lflows, od, stage,
+                             acl->priority + OVN_ACL_PRI_OFFSET,
+                             ds_cstr(&match), ds_cstr(&actions));
             }
             ds_destroy(&match);
             ds_destroy(&actions);
-- 
2.13.6