[ovs-dev] [PATCH 1/2] ovn: support applying ACLs to port groups
Han Zhou
zhouhan at gmail.com
Mon Apr 30 22:32:11 UTC 2018
On Mon, Apr 30, 2018 at 2:43 PM, Guru Shetty <guru at ovn.org> wrote:
>
>
> 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/p
>> ipermail/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?
>
> 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
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
More information about the dev
mailing list