Re: [ovs-dev] [PATCH ovn v6 2/3] northd: make default drops explicit
On Mon, Nov 21, 2022 at 11:13 AM Adrian Moreno wrote:
>
> By default, traffic that doesn't match any configured flow will be dropped.
> But having that behavior implicit makes those drops more difficult to
> visualize.
>
> Make default drops explicit both as default logical flows and as default
> openflow flows (e.g: for physical tables). The only exceptions are
> physical tables 68 and 70 that are used to implement chk_lb_hairpin and
> ct_snat_to_vip actions and don't drop traffic.
>
> Signed-off-by: Adrian Moreno
Acked-by: Numan Siddique
This patch series needs a rebase. Can you please spin up v7 ?
Numan
> ---
> controller/physical.c | 32
> northd/northd.c | 34 +++-
> northd/ovn-northd.8.xml | 40 +-
> tests/ovn-northd.at | 84
> tests/ovn.at| 169 +++-
> 5 files changed, 349 insertions(+), 10 deletions(-)
>
> diff --git a/controller/physical.c b/controller/physical.c
> index 705146316..58c4e1f05 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -833,6 +833,17 @@ put_zones_ofpacts(const struct zone_ids *zone_ids,
> struct ofpbuf *ofpacts_p)
> }
> }
>
> +static void
> +add_default_drop_flow(uint8_t table_id,
> + struct ovn_desired_flow_table *flow_table)
> +{
> +struct match match = MATCH_CATCHALL_INITIALIZER;
> +struct ofpbuf ofpacts;
> +ofpbuf_init(, 0);
> +ofctrl_add_flow(flow_table, table_id, 0, 0, ,
> +, hc_uuid);
> +}
> +
> static void
> put_local_common_flows(uint32_t dp_key,
> const struct sbrec_port_binding *pb,
> @@ -2114,6 +2125,13 @@ physical_run(struct physical_ctx *p_ctx,
> }
> }
>
> +/* Table 0, priority 0.
> + * ==
> + *
> + * Drop packets tha do not match any tunnel in_port.
> + */
> +add_default_drop_flow(OFTABLE_PHY_TO_LOG, flow_table);
> +
> /* Table 37, priority 150.
> * ===
> *
> @@ -2159,6 +2177,13 @@ physical_run(struct physical_ctx *p_ctx,
> ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, 0, ,
> , hc_uuid);
>
> +/* Table 38, priority 0.
> + * ==
> + *
> + * Drop packets that do not match previous flows.
> + */
> +add_default_drop_flow(OFTABLE_LOCAL_OUTPUT, flow_table);
> +
> /* Table 39, Priority 0.
> * ===
> *
> @@ -2185,5 +2210,12 @@ physical_run(struct physical_ctx *p_ctx,
> ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 0, 0, ,
> , hc_uuid);
>
> +/* Table 65, priority 0.
> + * ==
> + *
> + * Drop packets that do not match previous flows.
> + */
> +add_default_drop_flow(OFTABLE_LOG_TO_PHY, flow_table);
> +
> ofpbuf_uninit();
> }
> diff --git a/northd/northd.c b/northd/northd.c
> index e1f3bace8..133df 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -5155,6 +5155,16 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct
> ovn_datapath *od,
> io_port, ctrl_meter, stage_hint, where, hash);
> }
>
> +static void
> +__ovn_lflow_add_default_drop(struct hmap *lflow_map,
> + struct ovn_datapath *od,
> + enum ovn_stage stage,
> + const char *where)
> +{
> +ovn_lflow_add_at(lflow_map, od, stage, 0, "1", "drop;",
> + NULL, NULL, NULL, where );
> +}
> +
> /* Adds a row with the specified contents to the Logical_Flow table. */
> #define ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
>ACTIONS, IN_OUT_PORT, CTRL_METER, \
> @@ -5167,6 +5177,10 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct
> ovn_datapath *od,
> ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
> NULL, NULL, STAGE_HINT, OVS_SOURCE_LOCATOR)
>
> +#define ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE)\
> +__ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE, OVS_SOURCE_LOCATOR)
> +
> +
> /* This macro is similar to ovn_lflow_add_with_hint, except that it requires
> * the IN_OUT_PORT argument, which tells the lport name that appears in the
> * MATCH, which helps ovn-controller to bypass lflows parsing when the lport
> is
> @@ -10975,6 +10989,9 @@ build_adm_ctrl_flows_for_lrouter(
> * Broadcast/multicast source address is invalid. */
> ovn_lflow_add(lflows, od, S_ROUTER_IN_ADMISSION, 100,
>"vlan.present || eth.src[40]", "drop;");
> +
> +/* Default action for L2 security is to drop. */
> +ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_ADMISSION);
> }
> }
>
> @@ -11216,6 +11233,8 @@ build_neigh_learning_flows_for_lrouter(
>"nd_ns",
[ovs-dev] [PATCH ovn v6 2/3] northd: make default drops explicit
By default, traffic that doesn't match any configured flow will be dropped.
But having that behavior implicit makes those drops more difficult to
visualize.
Make default drops explicit both as default logical flows and as default
openflow flows (e.g: for physical tables). The only exceptions are
physical tables 68 and 70 that are used to implement chk_lb_hairpin and
ct_snat_to_vip actions and don't drop traffic.
Signed-off-by: Adrian Moreno
---
controller/physical.c | 32
northd/northd.c | 34 +++-
northd/ovn-northd.8.xml | 40 +-
tests/ovn-northd.at | 84
tests/ovn.at| 169 +++-
5 files changed, 349 insertions(+), 10 deletions(-)
diff --git a/controller/physical.c b/controller/physical.c
index 705146316..58c4e1f05 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -833,6 +833,17 @@ put_zones_ofpacts(const struct zone_ids *zone_ids, struct
ofpbuf *ofpacts_p)
}
}
+static void
+add_default_drop_flow(uint8_t table_id,
+ struct ovn_desired_flow_table *flow_table)
+{
+struct match match = MATCH_CATCHALL_INITIALIZER;
+struct ofpbuf ofpacts;
+ofpbuf_init(, 0);
+ofctrl_add_flow(flow_table, table_id, 0, 0, ,
+, hc_uuid);
+}
+
static void
put_local_common_flows(uint32_t dp_key,
const struct sbrec_port_binding *pb,
@@ -2114,6 +2125,13 @@ physical_run(struct physical_ctx *p_ctx,
}
}
+/* Table 0, priority 0.
+ * ==
+ *
+ * Drop packets tha do not match any tunnel in_port.
+ */
+add_default_drop_flow(OFTABLE_PHY_TO_LOG, flow_table);
+
/* Table 37, priority 150.
* ===
*
@@ -2159,6 +2177,13 @@ physical_run(struct physical_ctx *p_ctx,
ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, 0, ,
, hc_uuid);
+/* Table 38, priority 0.
+ * ==
+ *
+ * Drop packets that do not match previous flows.
+ */
+add_default_drop_flow(OFTABLE_LOCAL_OUTPUT, flow_table);
+
/* Table 39, Priority 0.
* ===
*
@@ -2185,5 +2210,12 @@ physical_run(struct physical_ctx *p_ctx,
ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 0, 0, ,
, hc_uuid);
+/* Table 65, priority 0.
+ * ==
+ *
+ * Drop packets that do not match previous flows.
+ */
+add_default_drop_flow(OFTABLE_LOG_TO_PHY, flow_table);
+
ofpbuf_uninit();
}
diff --git a/northd/northd.c b/northd/northd.c
index e1f3bace8..133df 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5155,6 +5155,16 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct
ovn_datapath *od,
io_port, ctrl_meter, stage_hint, where, hash);
}
+static void
+__ovn_lflow_add_default_drop(struct hmap *lflow_map,
+ struct ovn_datapath *od,
+ enum ovn_stage stage,
+ const char *where)
+{
+ovn_lflow_add_at(lflow_map, od, stage, 0, "1", "drop;",
+ NULL, NULL, NULL, where );
+}
+
/* Adds a row with the specified contents to the Logical_Flow table. */
#define ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
ACTIONS, IN_OUT_PORT, CTRL_METER, \
@@ -5167,6 +5177,10 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct
ovn_datapath *od,
ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
NULL, NULL, STAGE_HINT, OVS_SOURCE_LOCATOR)
+#define ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE)\
+__ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE, OVS_SOURCE_LOCATOR)
+
+
/* This macro is similar to ovn_lflow_add_with_hint, except that it requires
* the IN_OUT_PORT argument, which tells the lport name that appears in the
* MATCH, which helps ovn-controller to bypass lflows parsing when the lport is
@@ -10975,6 +10989,9 @@ build_adm_ctrl_flows_for_lrouter(
* Broadcast/multicast source address is invalid. */
ovn_lflow_add(lflows, od, S_ROUTER_IN_ADMISSION, 100,
"vlan.present || eth.src[40]", "drop;");
+
+/* Default action for L2 security is to drop. */
+ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_ADMISSION);
}
}
@@ -11216,6 +11233,8 @@ build_neigh_learning_flows_for_lrouter(
"nd_ns", "put_nd(inport, ip6.src, nd.sll); next;",
copp_meter_get(COPP_ND_NS, od->nbr->copp,
meter_groups));
+
+ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_LEARN_NEIGHBOR);
}
}
@@ -11492,6 +11511,8 @@ build_static_route_flows_for_lrouter(
const struct hmap *bfd_connections)
{
if (od->nbr) {
+
