On Mon, Apr 30, 2018 at 2:43 PM, Guru Shetty <g...@ovn.org> wrote: > > > On 22 April 2018 at 09:52, Han Zhou <zhou...@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/p >> ipermail/ovs-dev/2018-March/344856.html >> Requested-by: Guru Shetty <g...@ovn.org> >> Requested-by: Daniel Alvarez Sanchez <dalva...@redhat.com> >> Signed-off-by: Han Zhou <hzh...@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? > > Hi Guru, yes, you are 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 >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev