[ovs-dev] [PATCH 1/2] ovn: support applying ACLs to port groups
Guru Shetty
guru at ovn.org
Mon Apr 30 21:43:57 UTC 2018
On 22 April 2018 at 09:52, Han Zhou <zhouhan at gmail.com> wrote:
> Although port group can be used in match conditions of ACLs, it is
> still inconvenient for clients to figure out the lswitches that each
> ACL should be applied to.
>
> This patch supports applying ACLs to port groups directly instead of
> applying to each related lswitch individually. It provides convenience
> for clients such as k8s and OpenStack Neutron.
>
> Requested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/
> 344856.html
> Requested-by: Guru Shetty <guru at ovn.org>
> Requested-by: Daniel Alvarez Sanchez <dalvarez at redhat.com>
> Signed-off-by: Han Zhou <hzhou8 at ebay.com>
>
Thanks for the patch. I have not tested it, but just looked through the
code and it looks like what I had in mind.
So, you can technically use address sets or port groups in the ACLs inside
a port group, right?
> ---
> NEWS | 3 +-
> ovn/northd/ovn-northd.c | 422 ++++++++++++++++++++++++++++++
> +-----------------
> ovn/ovn-nb.ovsschema | 9 +-
> ovn/ovn-nb.xml | 19 ++-
> tests/ovn.at | 229 ++++++++++++++++++++++++++
> 5 files changed, 526 insertions(+), 156 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index cd4ffbb..8601cfc 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -22,7 +22,8 @@ Post-v2.9.0
> and reply with a RST for TCP or ICMPv4/ICMPv6 unreachable message
> for
> other IPv4/IPv6-based protocols whenever a reject ACL rule is hit.
> * ACL match conditions can now match on Port_Groups as well as
> address
> - sets that are automatically generated by Port_Groups.
> + sets that are automatically generated by Port_Groups. ACLs can be
> + applied directly to Port_Groups as well.
>
> v2.9.0 - 19 Feb 2018
> --------------------
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index ce472a5..cd01756 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -3162,7 +3162,259 @@ build_reject_acl_rules(struct ovn_datapath *od,
> struct hmap *lflows,
> }
>
> static void
> -build_acls(struct ovn_datapath *od, struct hmap *lflows)
> +consider_acl(struct hmap *lflows, struct ovn_datapath *od,
> + struct nbrec_acl *acl, bool has_stateful)
> +{
> + bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
> + enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL : S_SWITCH_OUT_ACL;
> +
> + char *stage_hint = xasprintf("%08x", acl->header_.uuid.parts[0]);
> + if (!strcmp(acl->action, "allow")
> + || !strcmp(acl->action, "allow-related")) {
> + /* If there are any stateful flows, we must even commit "allow"
> + * actions. This is because, while the initiater's
> + * direction may not have any stateful rules, the server's
> + * may and then its return traffic would not have an
> + * associated conntrack entry and would return "+invalid". */
> + if (!has_stateful) {
> + struct ds actions = DS_EMPTY_INITIALIZER;
> + build_acl_log(&actions, acl);
> + ds_put_cstr(&actions, "next;");
> + ovn_lflow_add_with_hint(lflows, od, stage,
> + acl->priority + OVN_ACL_PRI_OFFSET,
> + acl->match, ds_cstr(&actions),
> + stage_hint);
> + ds_destroy(&actions);
> + } else {
> + struct ds match = DS_EMPTY_INITIALIZER;
> + struct ds actions = DS_EMPTY_INITIALIZER;
> +
> + /* Commit the connection tracking entry if it's a new
> + * connection that matches this ACL. After this commit,
> + * the reply traffic is allowed by a flow we create at
> + * priority 65535, defined earlier.
> + *
> + * It's also possible that a known connection was marked for
> + * deletion after a policy was deleted, but the policy was
> + * re-added while that connection is still known. We catch
> + * that case here and un-set ct_label.blocked (which will be
> done
> + * by ct_commit in the "stateful" stage) to indicate that the
> + * connection should be allowed to resume.
> + */
> + ds_put_format(&match, "((ct.new && !ct.est)"
> + " || (!ct.new && ct.est && !ct.rpl "
> + "&& ct_label.blocked == 1)) "
> + "&& (%s)", acl->match);
> + ds_put_cstr(&actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
> + build_acl_log(&actions, acl);
> + ds_put_cstr(&actions, "next;");
> + ovn_lflow_add_with_hint(lflows, od, stage,
> + acl->priority + OVN_ACL_PRI_OFFSET,
> + ds_cstr(&match),
> + ds_cstr(&actions),
> + stage_hint);
> +
> + /* Match on traffic in the request direction for an
> established
> + * connection tracking entry that has not been marked for
> + * deletion. There is no need to commit here, so we can just
> + * proceed to the next table. We use this to ensure that this
> + * connection is still allowed by the currently defined
> + * policy. */
> + ds_clear(&match);
> + ds_clear(&actions);
> + ds_put_format(&match,
> + "!ct.new && ct.est && !ct.rpl"
> + " && ct_label.blocked == 0 && (%s)",
> + acl->match);
> +
> + build_acl_log(&actions, acl);
> + ds_put_cstr(&actions, "next;");
> + ovn_lflow_add_with_hint(lflows, od, stage,
> + acl->priority + OVN_ACL_PRI_OFFSET,
> + ds_cstr(&match), ds_cstr(&actions),
> + stage_hint);
> +
> + ds_destroy(&match);
> + ds_destroy(&actions);
> + }
> + } else if (!strcmp(acl->action, "drop")
> + || !strcmp(acl->action, "reject")) {
> + struct ds match = DS_EMPTY_INITIALIZER;
> + struct ds actions = DS_EMPTY_INITIALIZER;
> +
> + /* 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 reject/drop it. */
> + ds_put_cstr(&match,
> + "(!ct.est || (ct.est && ct_label.blocked == 1))");
> + if (!strcmp(acl->action, "reject")) {
> + build_reject_acl_rules(od, lflows, stage, acl, &match,
> + &actions);
> + } else {
> + ds_put_format(&match, " && (%s)", acl->match);
> + build_acl_log(&actions, acl);
> + 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
> + * this connection and we committed the connection tracking
> + * entry. Current policy says that we should drop this
> + * connection. First, we set bit 0 of ct_label to indicate
> + * that this connection is set for deletion. By not
> + * 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
> + * rejecting/dropping the packet, we go ahead and do it here.
> + */
> + ds_clear(&match);
> + ds_clear(&actions);
> + ds_put_cstr(&match, "ct.est && ct_label.blocked == 0");
> + ds_put_cstr(&actions, "ct_commit(ct_label=1/1); ");
> + if (!strcmp(acl->action, "reject")) {
> + build_reject_acl_rules(od, lflows, stage, acl, &match,
> + &actions);
> + } else {
> + ds_put_format(&match, " && (%s)", acl->match);
> + build_acl_log(&actions, acl);
> + 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 "reject/drop" ACL is simply the "reject/drop"
> + * logical flow action in all cases. */
> + if (!strcmp(acl->action, "reject")) {
> + build_reject_acl_rules(od, lflows, stage, acl, &match,
> + &actions);
> + } else {
> + build_acl_log(&actions, acl);
> + ds_put_cstr(&actions, "/* drop */");
> + ovn_lflow_add(lflows, od, stage,
> + acl->priority + OVN_ACL_PRI_OFFSET,
> + acl->match, ds_cstr(&actions));
> + }
> + }
> + ds_destroy(&match);
> + ds_destroy(&actions);
> + }
> + free(stage_hint);
> +}
> +
> +struct ovn_port_group_ls {
> + struct hmap_node key_node; /* Index on 'key'. */
> + struct uuid key; /* nb_ls->header_.uuid. */
> + const struct nbrec_logical_switch *nb_ls;
> +};
> +
> +struct ovn_port_group {
> + struct hmap_node key_node; /* Index on 'key'. */
> + struct uuid key; /* nb_pg->header_.uuid. */
> + const struct nbrec_port_group *nb_pg;
> + struct hmap nb_lswitches; /* NB lswitches related to the port group
> */
> + size_t n_acls; /* Number of ACLs applied to the port
> group */
> + struct nbrec_acl **acls; /* ACLs applied to the port group */
> +};
> +
> +static struct ovn_port_group *
> +ovn_port_group_create(struct hmap *pgs,
> + const struct nbrec_port_group *nb_pg)
> +{
> + struct ovn_port_group *pg = xzalloc(sizeof *pg);
> + pg->key = nb_pg->header_.uuid;
> + pg->nb_pg = nb_pg;
> + pg->n_acls = nb_pg->n_acls;
> + pg->acls = nb_pg->acls;
> + hmap_init(&pg->nb_lswitches);
> + hmap_insert(pgs, &pg->key_node, uuid_hash(&pg->key));
> + return pg;
> +}
> +
> +static void
> +ovn_port_group_ls_add(struct ovn_port_group *pg,
> + const struct nbrec_logical_switch *nb_ls)
> +{
> + struct ovn_port_group_ls *pg_ls = xzalloc(sizeof *pg_ls);
> + pg_ls->key = nb_ls->header_.uuid;
> + pg_ls->nb_ls = nb_ls;
> + hmap_insert(&pg->nb_lswitches, &pg_ls->key_node,
> uuid_hash(&pg_ls->key));
> +}
> +
> +static struct ovn_port_group_ls *
> +ovn_port_group_ls_find(struct ovn_port_group *pg, const struct uuid
> *ls_uuid)
> +{
> + struct ovn_port_group_ls *pg_ls;
> +
> + HMAP_FOR_EACH_WITH_HASH (pg_ls, key_node, uuid_hash(ls_uuid),
> + &pg->nb_lswitches) {
> + if (uuid_equals(ls_uuid, &pg_ls->key)) {
> + return pg_ls;
> + }
> + }
> + return NULL;
> +}
> +
> +static void
> +ovn_port_group_destroy(struct hmap *pgs, struct ovn_port_group *pg)
> +{
> + if (pg) {
> + hmap_remove(pgs, &pg->key_node);
> + struct ovn_port_group_ls *ls;
> + HMAP_FOR_EACH_POP (ls, key_node, &pg->nb_lswitches) {
> + free(ls);
> + }
> + hmap_destroy(&pg->nb_lswitches);
> + free(pg);
> + }
> +}
> +
> +static void
> +build_port_group_lswitches(struct northd_context *ctx, struct hmap *pgs,
> + struct hmap *ports)
> +{
> + hmap_init(pgs);
> +
> + const struct nbrec_port_group *nb_pg;
> + NBREC_PORT_GROUP_FOR_EACH (nb_pg, ctx->ovnnb_idl) {
> + struct ovn_port_group *pg = ovn_port_group_create(pgs, nb_pg);
> + for (size_t i = 0; i < nb_pg->n_ports; i++) {
> + struct ovn_port *op = ovn_port_find(ports,
> nb_pg->ports[i]->name);
> + if (!op) {
> + static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_ERR_RL(&rl, "lport %s in port group %s not found.",
> + nb_pg->ports[i]->name,
> + nb_pg->name);
> + continue;
> + }
> +
> + if (!op->od->nbs) {
> + static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "lport %s in port group %s has no
> lswitch.",
> + nb_pg->ports[i]->name,
> + nb_pg->name);
> + continue;
> + }
> +
> + struct ovn_port_group_ls *pg_ls =
> + ovn_port_group_ls_find(pg, &op->od->nbs->header_.uuid);
> + if (!pg_ls) {
> + ovn_port_group_ls_add(pg, op->od->nbs);
> + }
> + }
> + }
> +}
> +
> +static void
> +build_acls(struct ovn_datapath *od, struct hmap *lflows,
> + struct hmap *port_groups)
> {
> bool has_stateful = has_stateful_acl(od);
>
> @@ -3263,148 +3515,15 @@ build_acls(struct ovn_datapath *od, struct hmap
> *lflows)
> /* Ingress or Egress ACL Table (Various priorities). */
> for (size_t i = 0; i < od->nbs->n_acls; i++) {
> struct nbrec_acl *acl = od->nbs->acls[i];
> - bool ingress = !strcmp(acl->direction, "from-lport") ? true
> :false;
> - enum ovn_stage stage = ingress ? S_SWITCH_IN_ACL :
> S_SWITCH_OUT_ACL;
> -
> - char *stage_hint = xasprintf("%08x", acl->header_.uuid.parts[0]);
> - if (!strcmp(acl->action, "allow")
> - || !strcmp(acl->action, "allow-related")) {
> - /* If there are any stateful flows, we must even commit
> "allow"
> - * actions. This is because, while the initiater's
> - * direction may not have any stateful rules, the server's
> - * may and then its return traffic would not have an
> - * associated conntrack entry and would return "+invalid". */
> - if (!has_stateful) {
> - struct ds actions = DS_EMPTY_INITIALIZER;
> - build_acl_log(&actions, acl);
> - ds_put_cstr(&actions, "next;");
> - ovn_lflow_add_with_hint(lflows, od, stage,
> - acl->priority +
> OVN_ACL_PRI_OFFSET,
> - acl->match, ds_cstr(&actions),
> - stage_hint);
> - ds_destroy(&actions);
> - } else {
> - struct ds match = DS_EMPTY_INITIALIZER;
> - struct ds actions = DS_EMPTY_INITIALIZER;
> -
> - /* Commit the connection tracking entry if it's a new
> - * connection that matches this ACL. After this commit,
> - * the reply traffic is allowed by a flow we create at
> - * priority 65535, defined earlier.
> - *
> - * It's also possible that a known connection was marked
> for
> - * deletion after a policy was deleted, but the policy was
> - * re-added while that connection is still known. We
> catch
> - * that case here and un-set ct_label.blocked (which will
> be done
> - * by ct_commit in the "stateful" stage) to indicate that
> the
> - * connection should be allowed to resume.
> - */
> - ds_put_format(&match, "((ct.new && !ct.est)"
> - " || (!ct.new && ct.est && !ct.rpl "
> - "&& ct_label.blocked == 1)) "
> - "&& (%s)", acl->match);
> - ds_put_cstr(&actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
> - build_acl_log(&actions, acl);
> - ds_put_cstr(&actions, "next;");
> - ovn_lflow_add_with_hint(lflows, od, stage,
> - acl->priority +
> OVN_ACL_PRI_OFFSET,
> - ds_cstr(&match),
> - ds_cstr(&actions),
> - stage_hint);
> -
> - /* Match on traffic in the request direction for an
> established
> - * connection tracking entry that has not been marked for
> - * deletion. There is no need to commit here, so we can
> just
> - * proceed to the next table. We use this to ensure that
> this
> - * connection is still allowed by the currently defined
> - * policy. */
> - ds_clear(&match);
> - ds_clear(&actions);
> - ds_put_format(&match,
> - "!ct.new && ct.est && !ct.rpl"
> - " && ct_label.blocked == 0 && (%s)",
> - acl->match);
> -
> - build_acl_log(&actions, acl);
> - ds_put_cstr(&actions, "next;");
> - ovn_lflow_add_with_hint(lflows, od, stage,
> - acl->priority +
> OVN_ACL_PRI_OFFSET,
> - ds_cstr(&match),
> ds_cstr(&actions),
> - stage_hint);
> -
> - ds_destroy(&match);
> - ds_destroy(&actions);
> - }
> - } else if (!strcmp(acl->action, "drop")
> - || !strcmp(acl->action, "reject")) {
> - struct ds match = DS_EMPTY_INITIALIZER;
> - struct ds actions = DS_EMPTY_INITIALIZER;
> -
> - /* 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 reject/drop it. */
> - ds_put_cstr(&match,
> - "(!ct.est || (ct.est && ct_label.blocked ==
> 1))");
> - if (!strcmp(acl->action, "reject")) {
> - build_reject_acl_rules(od, lflows, stage, acl, &match,
> - &actions);
> - } else {
> - ds_put_format(&match, " && (%s)", acl->match);
> - build_acl_log(&actions, acl);
> - 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
> - * this connection and we committed the connection
> tracking
> - * entry. Current policy says that we should drop this
> - * connection. First, we set bit 0 of ct_label to
> indicate
> - * that this connection is set for deletion. By not
> - * 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
> - * rejecting/dropping the packet, we go ahead and do it
> here.
> - */
> - ds_clear(&match);
> - ds_clear(&actions);
> - ds_put_cstr(&match, "ct.est && ct_label.blocked == 0");
> - ds_put_cstr(&actions, "ct_commit(ct_label=1/1); ");
> - if (!strcmp(acl->action, "reject")) {
> - build_reject_acl_rules(od, lflows, stage, acl, &match,
> - &actions);
> - } else {
> - ds_put_format(&match, " && (%s)", acl->match);
> - build_acl_log(&actions, acl);
> - 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 "reject/drop" ACL is simply the "reject/drop"
> - * logical flow action in all cases. */
> - if (!strcmp(acl->action, "reject")) {
> - build_reject_acl_rules(od, lflows, stage, acl, &match,
> - &actions);
> - } else {
> - build_acl_log(&actions, acl);
> - ds_put_cstr(&actions, "/* drop */");
> - ovn_lflow_add(lflows, od, stage,
> - acl->priority + OVN_ACL_PRI_OFFSET,
> - acl->match, ds_cstr(&actions));
> - }
> + consider_acl(lflows, od, acl, has_stateful);
> + }
> + struct ovn_port_group *pg;
> + HMAP_FOR_EACH (pg, key_node, port_groups) {
> + if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
> + for (size_t i = 0; i < pg->n_acls; i++) {
> + consider_acl(lflows, od, pg->acls[i], has_stateful);
> }
> - ds_destroy(&match);
> - ds_destroy(&actions);
> }
> - free(stage_hint);
> }
>
> /* Add 34000 priority flow to allow DHCP reply from ovn-controller to
> all
> @@ -3633,7 +3752,8 @@ build_stateful(struct ovn_datapath *od, struct hmap
> *lflows)
>
> static void
> build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
> - struct hmap *lflows, struct hmap *mcgroups)
> + struct hmap *port_groups, struct hmap *lflows,
> + struct hmap *mcgroups)
> {
> /* This flow table structure is documented in ovn-northd(8), so please
> * update ovn-northd.8.xml if you change anything. */
> @@ -3652,7 +3772,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
> build_pre_acls(od, lflows);
> build_pre_lb(od, lflows);
> build_pre_stateful(od, lflows);
> - build_acls(od, lflows);
> + build_acls(od, lflows, port_groups);
> build_qos(od, lflows);
> build_lb(od, lflows);
> build_stateful(od, lflows);
> @@ -6069,12 +6189,12 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> * constructing their contents based on the OVN_NB database. */
> static void
> build_lflows(struct northd_context *ctx, struct hmap *datapaths,
> - struct hmap *ports)
> + struct hmap *ports, struct hmap *port_groups)
> {
> struct hmap lflows = HMAP_INITIALIZER(&lflows);
> struct hmap mcgroups = HMAP_INITIALIZER(&mcgroups);
>
> - build_lswitch_flows(datapaths, ports, &lflows, &mcgroups);
> + build_lswitch_flows(datapaths, ports, port_groups, &lflows,
> &mcgroups);
> build_lrouter_flows(datapaths, ports, &lflows);
>
> /* Push changes to the Logical_Flow table to database. */
> @@ -6421,6 +6541,7 @@ sync_dns_entries(struct northd_context *ctx, struct
> hmap *datapaths)
> hmap_destroy(&dns_map);
> }
>
> +
>
> static void
> ovnnb_db_run(struct northd_context *ctx, struct chassis_index
> *chassis_index,
> @@ -6429,16 +6550,23 @@ ovnnb_db_run(struct northd_context *ctx, struct
> chassis_index *chassis_index,
> if (!ctx->ovnsb_txn || !ctx->ovnnb_txn) {
> return;
> }
> - struct hmap datapaths, ports;
> + struct hmap datapaths, ports, port_groups;
> build_datapaths(ctx, &datapaths);
> build_ports(ctx, &datapaths, chassis_index, &ports);
> build_ipam(&datapaths, &ports);
> - build_lflows(ctx, &datapaths, &ports);
> + build_port_group_lswitches(ctx, &port_groups, &ports);
> + build_lflows(ctx, &datapaths, &ports, &port_groups);
>
> sync_address_sets(ctx);
> sync_port_groups(ctx);
> sync_dns_entries(ctx, &datapaths);
>
> + struct ovn_port_group *pg, *next_pg;
> + HMAP_FOR_EACH_SAFE (pg, next_pg, key_node, &port_groups) {
> + ovn_port_group_destroy(&port_groups, pg);
> + }
> + hmap_destroy(&port_groups);
> +
> struct ovn_datapath *dp, *next_dp;
> HMAP_FOR_EACH_SAFE (dp, next_dp, key_node, &datapaths) {
> ovn_datapath_destroy(&datapaths, dp);
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 2d09282..8e6ddec 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "5.10.0",
> - "cksum": "222987318 18430",
> + "version": "5.11.0",
> + "cksum": "1149260021 18713",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -122,6 +122,11 @@
> "refType": "weak"},
> "min": 0,
> "max": "unlimited"}},
> + "acls": {"type": {"key": {"type": "uuid",
> + "refTable": "ACL",
> + "refType": "strong"},
> + "min": 0,
> + "max": "unlimited"}},
> "external_ids": {
> "type": {"key": "string", "value": "string",
> "min": 0, "max": "unlimited"}}},
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 62d5a07..6aed610 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -963,6 +963,12 @@
> The logical switch ports belonging to the group in uuids.
> </column>
>
> + <column name="acls">
> + Access control rules that apply to the port group. Applying an ACL
> + to a port group has the same effect as applying the ACL to all
> logical
> + lswitches that the ports of the port group belong to.
> + </column>
> +
> <group title="Common Columns">
> <column name="external_ids">
> See <em>External IDs</em> at the beginning of this document.
> @@ -1030,12 +1036,13 @@
> <table name="ACL" title="Access Control List (ACL) rule">
> <p>
> Each row in this table represents one ACL rule for a logical switch
> - that points to it through its <ref column="acls"/> column. The <ref
> - column="action"/> column for the highest-<ref column="priority"/>
> - matching row in this table determines a packet's treatment. If no
> row
> - matches, packets are allowed by default. (Default-deny treatment is
> - possible: add a rule with <ref column="priority"/> 0,
> <code>0</code> as
> - <ref column="match"/>, and <code>deny</code> as <ref
> column="action"/>.)
> + or a port group that points to it through its <ref column="acls"/>
> + column. The <ref column="action"/> column for the
> + highest-<ref column="priority"/> matching row in this table
> determines a
> + packet's treatment. If no row matches, packets are allowed by
> default.
> + (Default-deny treatment is possible: add a rule with
> + <ref column="priority"/> 0, <code>0</code> as <ref column="match"/>,
> + and <code>deny</code> as <ref column="action"/>.)
> </p>
>
> <column name="priority">
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4a53165..95f747a 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9798,3 +9798,232 @@ done
> # Gracefully terminate daemons
> OVN_CLEANUP([hv1], [hv2], [hv3])
> AT_CLEANUP
> +
> +AT_SETUP([ovn -- ACLs on Port Groups])
> +AT_KEYWORDS([ovnpg_acl])
> +AT_SKIP_IF([test $HAVE_PYTHON = no])
> +ovn_start
> +
> +# Logical network:
> +#
> +# Three logical switches ls1, ls2, ls3.
> +# One logical router lr0 connected to ls[123],
> +# with nine subnets, three per logical switch:
> +#
> +# lrp11 on ls1 for subnet 192.168.11.0/24
> +# lrp12 on ls1 for subnet 192.168.12.0/24
> +# lrp13 on ls1 for subnet 192.168.13.0/24
> +# ...
> +# lrp33 on ls3 for subnet 192.168.33.0/24
> +#
> +# 27 VIFs, 9 per LS, 3 per subnet: lp[123][123][123], where the first two
> +# digits are the subnet and the last digit distinguishes the VIF.
> +#
> +# This test will create two port groups and ACLs will be applied on them.
> +
> +get_lsp_uuid () {
> + ovn-nbctl lsp-list ls${1%??} | grep lp$1 | awk '{ print $1 }'
> +}
> +
> +pg1_ports=
> +pg2_ports=
> +for i in 1 2 3; do
> + ovn-nbctl ls-add ls$i
> + for j in 1 2 3; do
> + for k in 1 2 3; do
> + ovn-nbctl \
> + -- lsp-add ls$i lp$i$j$k \
> + -- lsp-set-addresses lp$i$j$k "f0:00:00:00:0$i:$j$k \
> + 192.168.$i$j.$k"
> + # logical ports lp[12]?1 belongs to port group pg1
> + if test $i != 3 && test $k == 1; then
> + pg1_ports="$pg1_ports `get_lsp_uuid $i$j$k`"
> + fi
> + # logical ports lp[23]?2 belongs to port group pg2
> + if test $i != 1 && test $k == 2; then
> + pg2_ports="$pg2_ports `get_lsp_uuid $i$j$k`"
> + fi
> + done
> + done
> +done
> +
> +ovn-nbctl lr-add lr0
> +for i in 1 2 3; do
> + for j in 1 2 3; do
> + ovn-nbctl lrp-add lr0 lrp$i$j 00:00:00:00:ff:$i$j
> 192.168.$i$j.254/24
> + ovn-nbctl \
> + -- lsp-add ls$i lrp$i$j-attachment \
> + -- set Logical_Switch_Port lrp$i$j-attachment type=router \
> + options:router-port=lrp$i$j \
> + addresses='"00:00:00:00:ff:'$i$j'"'
> + done
> +done
> +
> +pg1_uuid=`ovn-nbctl create Port_Group name=pg1 ports="$pg1_ports"`
> +ovn-nbctl create Port_Group name=pg2 ports="$pg2_ports"
> +
> +# create ACLs on pg1 to drop traffic from pg2 to pg1
> +ovn-nbctl --id=@acl1 create acl priority=1001 direction=to-lport \
> + match='"outport == @pg1"' action=drop \
> + -- add port_group $pg1_uuid acls @acl1
> +ovn-nbctl --id=@acl2 create acl priority=1002 direction=to-lport \
> + match='"outport == @pg1 && ip4.src == $pg2_ip4"'
> action=allow-related \
> + -- add port_group $pg1_uuid acls @acl2
> +
> +# Physical network:
> +#
> +# Three hypervisors hv[123].
> +# lp?1[123] spread across hv[123]: lp?11 on hv1, lp?12 on hv2, lp?13 on
> hv3.
> +# lp?2[123] spread across hv[23]: lp?21 and lp?22 on hv2, lp?23 on hv3.
> +# lp?3[123] all on hv3.
> +
> +# Given the name of a logical port, prints the name of the hypervisor
> +# on which it is located.
> +vif_to_hv() {
> + case $1 in dnl (
> + ?11) echo 1 ;; dnl (
> + ?12 | ?21 | ?22) echo 2 ;; dnl (
> + ?13 | ?23 | ?3?) echo 3 ;;
> + esac
> +}
> +
> +# Given the name of a logical port, prints the name of its logical router
> +# port, e.g. "vif_to_lrp 123" yields 12.
> +vif_to_lrp() {
> + echo ${1%?}
> +}
> +
> +# Given the name of a logical port, prints the name of its logical
> +# switch, e.g. "vif_to_ls 123" yields 1.
> +vif_to_ls() {
> + echo ${1%??}
> +}
> +
> +net_add n1
> +for i in 1 2 3; do
> + sim_add hv$i
> + as hv$i
> + ovs-vsctl add-br br-phys
> + ovn_attach n1 br-phys 192.168.0.$i
> +done
> +for i in 1 2 3; do
> + for j in 1 2 3; do
> + for k in 1 2 3; do
> + hv=`vif_to_hv $i$j$k`
> + as hv$hv ovs-vsctl \
> + -- add-port br-int vif$i$j$k \
> + -- set Interface vif$i$j$k \
> + external-ids:iface-id=lp$i$j$k \
> + options:tx_pcap=hv$hv/vif$i$j$k-tx.pcap \
> + options:rxq_pcap=hv$hv/vif$i$j$k-rx.pcap \
> + ofport-request=$i$j$k
> + done
> + done
> +done
> +
> +# Pre-populate the hypervisors' ARP tables so that we don't lose any
> +# packets for ARP resolution (native tunneling doesn't queue packets
> +# for ARP resolution).
> +OVN_POPULATE_ARP
> +
> +# Allow some time for ovn-northd and ovn-controller to catch up.
> +# XXX This should be more systematic.
> +sleep 1
> +
> +# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
> +#
> +# This shell function causes a packet to be received on INPORT. The
> packet's
> +# content has Ethernet destination DST and source SRC (each exactly 12 hex
> +# digits) and Ethernet type ETHTYPE (4 hex digits). The OUTPORTs (zero or
> +# more) list the VIFs on which the packet should be received. INPORT and
> the
> +# OUTPORTs are specified as logical switch port numbers, e.g. 123 for
> vif123.
> +for i in 1 2 3; do
> + for j in 1 2 3; do
> + for k in 1 2 3; do
> + : > $i$j$k.expected
> + done
> + done
> +done
> +test_ip() {
> + # This packet has bad checksums but logical L3 routing doesn't check.
> + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
> + local packet=${dst_mac}${src_mac}08004500001c0000000040110000${
> src_ip}${dst_ip}0035111100080000
> + shift; shift; shift; shift; shift
> + hv=hv`vif_to_hv $inport`
> + as $hv ovs-appctl netdev-dummy/receive vif$inport $packet
> + #as $hv ovs-appctl ofproto/trace br-int in_port=$inport $packet
> + in_ls=`vif_to_ls $inport`
> + in_lrp=`vif_to_lrp $inport`
> + for outport; do
> + out_ls=`vif_to_ls $outport`
> + if test $in_ls = $out_ls; then
> + # Ports on the same logical switch receive exactly the same
> packet.
> + echo $packet
> + else
> + # Routing decrements TTL and updates source and dest MAC
> + # (and checksum).
> + out_lrp=`vif_to_lrp $outport`
> + echo f00000000${outport}00000000ff$
> {out_lrp}08004500001c00000000"3f1101"00${src_ip}${dst_ip}0035111100080000
> + fi >> $outport.expected
> + done
> +}
> +
> +as hv1 ovs-vsctl --columns=name,ofport list interface
> +as hv1 ovn-sbctl list port_binding
> +as hv1 ovn-sbctl list datapath_binding
> +as hv1 ovn-sbctl list port_group
> +as hv1 ovn-sbctl list address_set
> +as hv1 ovn-sbctl dump-flows
> +as hv1 ovs-ofctl dump-flows br-int
> +
> +# Send IP packets between all pairs of source and destination ports,
> +# packets matches ACL1 but not ACL2 should be dropped
> +ip_to_hex() {
> + printf "%02x%02x%02x%02x" "$@"
> +}
> +for is in 1 2 3; do
> + for js in 1 2 3; do
> + for ks in 1 2 3; do
> + bcast=
> + s=$is$js$ks
> + smac=f00000000$s
> + sip=`ip_to_hex 192 168 $is$js $ks`
> + for id in 1 2 3; do
> + for jd in 1 2 3; do
> + for kd in 1 2 3; do
> + d=$id$jd$kd
> + dip=`ip_to_hex 192 168 $id$jd $kd`
> + if test $is = $id; then dmac=f00000000$d; else
> dmac=00000000ff$is$js; fi
> + if test $d != $s; then unicast=$d; else unicast=; fi
> +
> + # packets matches ACL1 but not ACL2 should be dropped
> + if test $id != 3 && test $kd == 1; then
> + if test $is == 1 || test $ks != 2; then
> + unicast=
> + fi
> + fi
> + test_ip $s $smac $dmac $sip $dip $unicast #1
> + done
> + done
> + done
> + done
> + done
> +done
> +
> +# Allow some time for packet forwarding.
> +# XXX This can be improved.
> +sleep 1
> +
> +# Now check the packets actually received against the ones expected.
> +for i in 1 2 3; do
> + for j in 1 2 3; do
> + for k in 1 2 3; do
> + OVN_CHECK_PACKETS([hv`vif_to_hv $i$j$k`/vif$i$j$k-tx.pcap],
> + [$i$j$k.expected])
> + done
> + done
> +done
> +
> +# Gracefully terminate daemons
> +OVN_CLEANUP([hv1], [hv2], [hv3])
> +AT_CLEANUP
> --
> 2.1.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list