Re: [ovs-dev] [PATCH ovn 3/3] northd: interoduce logical flow for localnet egress shaping
On Mon, Sep 23, 2019 at 5:09 PM Lorenzo Bianconi
wrote:
>
> Add set_queue() action for qos capable localnet port in
> S_SWITCH_OUT_PORT_SEC_L2 stage of logical swith pipeline
> Introduce build_lswitch_port_sec and refactor lswitch_port_security code
> in order to remove duplicated code
>
> Signed-off-by: Lorenzo Bianconi
There's a typo in the subject line: s/interoduce/Introduce.
> ---
> northd/ovn-northd.8.xml | 7 +-
> northd/ovn-northd.c | 199 +---
> 2 files changed, 110 insertions(+), 96 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 0f4f1c112..093b55438 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1150,10 +1150,15 @@ output;
>eth.dst are always accepted instead of being subject to
> the
>port security rules; this is implemented through a priority-100 flow
> that
>matches on eth.mcast with action output;.
> - Finally, to ensure that even broadcast and multicast packets are not
> + Moreover, to ensure that even broadcast and multicast packets are not
>delivered to disabled logical ports, a priority-150 flow for each
>disabled logical outport overrides the priority-100 flow
>with a drop; action.
> + Finally if egress qos has been enabled on a localnet port, the outgoing
> + queue id is set through set_queue action. Please remember
> to
> + mark the corresponding physical interface with
> + ovn-egress-iface set to true in + table="Interface" db="Open_vSwitch"/>
>
>
> Logical Router Datapaths
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 633fb502b..4bacad572 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3903,6 +3903,108 @@ has_stateful_acl(struct ovn_datapath *od)
> return false;
> }
>
> +static void
> +build_lswitch_port_sec(struct hmap *ports, struct hmap *datapaths,
> + struct hmap *lflows)
> +{
Isn't it better to split this function into two functions (one for
ingress and one for egress)? What do you think?
> +struct ds match = DS_EMPTY_INITIALIZER;
> +struct ds actions = DS_EMPTY_INITIALIZER;
> +struct ovn_datapath *od;
> +struct ovn_port *op;
> +
> +HMAP_FOR_EACH (op, key_node, ports) {
> +if (!op->nbsp) {
> +continue;
> +}
> +
> +if (lsp_is_external(op->nbsp)) {
> +continue;
> +}
> +
> +ds_clear();
> +ds_put_format(, "outport == %s", op->json_key);
> +
> +/* Egress table 8: Egress port security - IP (priorities 90 and 80)
> + * if port security enabled.
> + *
> + * Egress table 9: Egress port security - L2 (priorities 50 and 150).
> + *
> + * Priority 50 rules implement port security for enabled logical
> port.
> + *
> + * Priority 150 rules drop packets to disabled logical ports, so that
> + * they don't even receive multicast or broadcast packets.
> + */
> +if (!lsp_is_enabled(op->nbsp)) {
> +ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 150,
> + ds_cstr(), "drop;");
> +build_port_security_ip(P_OUT, op, lflows);
Do we still need to add port security flows for IP if the port is
disabled? Won't the drop flow above suffice?
Thanks,
Dumitru
> +continue;
> +}
> +
> +ds_clear();
> +if (!strcmp(op->nbsp->type, "localnet")) {
> +const char *queue_id = smap_get(>sb->options,
> +"qdisc_queue_id");
> +if (queue_id) {
> +ds_put_format(, "set_queue(%s); ", queue_id);
> +}
> +}
> +ds_put_cstr(, "output;");
> +build_port_security_l2("eth.dst", op->ps_addrs, op->n_ps_addrs,
> + );
> +ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 50,
> + ds_cstr(), ds_cstr());
> +
> +ds_clear();
> +ds_clear();
> +
> +/* Logical switch ingress table 0: Ingress port security - L2
> + * (priority 50).
> + * Ingress table 1: Ingress port security - IP (priority 90 and 80)
> + * Ingress table 2: Ingress port security - ND (priority 90 and 80)
> + */
> +ds_put_format(, "inport == %s", op->json_key);
> +build_port_security_l2("eth.src", op->ps_addrs, op->n_ps_addrs,
> + );
> +
> +const char *queue_id = smap_get(>sb->options, "qdisc_queue_id");
> +if (queue_id) {
> +ds_put_format(, "set_queue(%s); ", queue_id);
> +}
> +ds_put_cstr(, "next;");
> +ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2, 50,
> + ds_cstr(), ds_cstr());
> +
> +if (op->nbsp->n_port_security) {
> +
[ovs-dev] [PATCH ovn 3/3] northd: interoduce logical flow for localnet egress shaping
Add set_queue() action for qos capable localnet port in
S_SWITCH_OUT_PORT_SEC_L2 stage of logical swith pipeline
Introduce build_lswitch_port_sec and refactor lswitch_port_security code
in order to remove duplicated code
Signed-off-by: Lorenzo Bianconi
---
northd/ovn-northd.8.xml | 7 +-
northd/ovn-northd.c | 199 +---
2 files changed, 110 insertions(+), 96 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 0f4f1c112..093b55438 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1150,10 +1150,15 @@ output;
eth.dst are always accepted instead of being subject to the
port security rules; this is implemented through a priority-100 flow that
matches on eth.mcast with action output;.
- Finally, to ensure that even broadcast and multicast packets are not
+ Moreover, to ensure that even broadcast and multicast packets are not
delivered to disabled logical ports, a priority-150 flow for each
disabled logical outport overrides the priority-100 flow
with a drop; action.
+ Finally if egress qos has been enabled on a localnet port, the outgoing
+ queue id is set through set_queue action. Please remember to
+ mark the corresponding physical interface with
+ ovn-egress-iface set to true in
Logical Router Datapaths
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 633fb502b..4bacad572 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3903,6 +3903,108 @@ has_stateful_acl(struct ovn_datapath *od)
return false;
}
+static void
+build_lswitch_port_sec(struct hmap *ports, struct hmap *datapaths,
+ struct hmap *lflows)
+{
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ds actions = DS_EMPTY_INITIALIZER;
+struct ovn_datapath *od;
+struct ovn_port *op;
+
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbsp) {
+continue;
+}
+
+if (lsp_is_external(op->nbsp)) {
+continue;
+}
+
+ds_clear();
+ds_put_format(, "outport == %s", op->json_key);
+
+/* Egress table 8: Egress port security - IP (priorities 90 and 80)
+ * if port security enabled.
+ *
+ * Egress table 9: Egress port security - L2 (priorities 50 and 150).
+ *
+ * Priority 50 rules implement port security for enabled logical port.
+ *
+ * Priority 150 rules drop packets to disabled logical ports, so that
+ * they don't even receive multicast or broadcast packets.
+ */
+if (!lsp_is_enabled(op->nbsp)) {
+ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 150,
+ ds_cstr(), "drop;");
+build_port_security_ip(P_OUT, op, lflows);
+continue;
+}
+
+ds_clear();
+if (!strcmp(op->nbsp->type, "localnet")) {
+const char *queue_id = smap_get(>sb->options,
+"qdisc_queue_id");
+if (queue_id) {
+ds_put_format(, "set_queue(%s); ", queue_id);
+}
+}
+ds_put_cstr(, "output;");
+build_port_security_l2("eth.dst", op->ps_addrs, op->n_ps_addrs,
+ );
+ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 50,
+ ds_cstr(), ds_cstr());
+
+ds_clear();
+ds_clear();
+
+/* Logical switch ingress table 0: Ingress port security - L2
+ * (priority 50).
+ * Ingress table 1: Ingress port security - IP (priority 90 and 80)
+ * Ingress table 2: Ingress port security - ND (priority 90 and 80)
+ */
+ds_put_format(, "inport == %s", op->json_key);
+build_port_security_l2("eth.src", op->ps_addrs, op->n_ps_addrs,
+ );
+
+const char *queue_id = smap_get(>sb->options, "qdisc_queue_id");
+if (queue_id) {
+ds_put_format(, "set_queue(%s); ", queue_id);
+}
+ds_put_cstr(, "next;");
+ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2, 50,
+ ds_cstr(), ds_cstr());
+
+if (op->nbsp->n_port_security) {
+build_port_security_ip(P_IN, op, lflows);
+build_port_security_ip(P_OUT, op, lflows);
+build_port_security_nd(op, lflows);
+}
+}
+
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs) {
+continue;
+}
+
+/* Ingress table 1 and 2: Port security - IP and ND, by default
+ * goto next. (priority 0)
+ */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_ND, 0, "1", "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_IP, 0, "1", "next;");
+
+/* Egress tables 8: Egress port security - IP (priority 0)
+ * Egress table 9: Egress port
