[ovs-dev] [PATCH v2] ovn-northd: Apply pre ACLs when using Port Groups
Han Zhou
zhouhan at gmail.com
Fri Jun 22 02:38:16 UTC 2018
On Tue, Jun 19, 2018 at 7:18 PM, Daniel Alvarez <dalvarez at redhat.com> wrote:
>
> When using Port Groups, the pre ACLs were not applied so the
> conntrack action was not performed. This patch takes Port Groups
> into account when processing the pre ACLs.
>
> As a follow up, we could enhance this patch by creating an index
> from lswitch to port groups.
>
> Signed-off-by: Daniel Alvarez <dalvarez at redhat.com>
> ---
> ovn/northd/ovn-northd.c | 100
+++++++++++++++++++++++++++---------------------
> tests/ovn.at | 2 +-
> 2 files changed, 57 insertions(+), 45 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 72fe4e795..818ac59fa 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2835,8 +2835,47 @@ build_dhcpv6_action(struct ovn_port *op, struct
in6_addr *offer_ip,
> return true;
> }
>
> +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 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 bool
> -has_stateful_acl(struct ovn_datapath *od)
> +has_stateful_acl(struct ovn_datapath *od, struct hmap *port_groups)
> {
> for (size_t i = 0; i < od->nbs->n_acls; i++) {
> struct nbrec_acl *acl = od->nbs->acls[i];
> @@ -2845,13 +2884,25 @@ has_stateful_acl(struct ovn_datapath *od)
> }
> }
>
> + 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++) {
> + struct nbrec_acl *acl = pg->acls[i];
> + if (!strcmp(acl->action, "allow-related")) {
> + return true;
> + }
> + }
> + }
> + }
> return false;
> }
>
> static void
> -build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
> +build_pre_acls(struct ovn_datapath *od, struct hmap *lflows,
> + struct hmap *port_groups)
> {
> - bool has_stateful = has_stateful_acl(od);
> + bool has_stateful = has_stateful_acl(od, port_groups);
>
> /* Ingress and Egress Pre-ACL Table (Priority 0): Packets are
> * allowed by default. */
> @@ -3309,21 +3360,6 @@ consider_acl(struct hmap *lflows, struct
ovn_datapath *od,
> 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)
> @@ -3338,30 +3374,6 @@ ovn_port_group_create(struct hmap *pgs,
> 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)
> {
> @@ -3416,7 +3428,7 @@ static void
> build_acls(struct ovn_datapath *od, struct hmap *lflows,
> struct hmap *port_groups)
> {
> - bool has_stateful = has_stateful_acl(od);
> + bool has_stateful = has_stateful_acl(od, port_groups);
>
> /* Ingress and Egress ACL Table (Priority 0): Packets are allowed by
> * default. A related rule at priority 1 is added below if there
> @@ -3769,7 +3781,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
> continue;
> }
>
> - build_pre_acls(od, lflows);
> + build_pre_acls(od, lflows, port_groups);
> build_pre_lb(od, lflows);
> build_pre_stateful(od, lflows);
> build_acls(od, lflows, port_groups);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 6553d17c6..93644b023 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9981,7 +9981,7 @@ ovn-nbctl create Port_Group name=pg2
ports="$pg2_ports"
> # create ACLs on pg1 to drop traffic from pg2 to pg1
> ovn-nbctl acl-add pg1 to-lport 1001 'outport == @pg1' drop
> ovn-nbctl --type=port-group acl-add pg1 to-lport 1002 \
> - 'outport == @pg1 && ip4.src == $pg2_ip4' allow-related
> + 'outport == @pg1 && ip4.src == $pg2_ip4' allow
>
> # Physical network:
> #
> --
> 2.15.1 (Apple Git-101)
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Acked-by: Han Zhou <hzhou8 at ebay.com>
p.s. I added a follow up patch to ensure the test coverage for the bug:
https://patchwork.ozlabs.org/patch/933050/
Please review it, too.
More information about the dev
mailing list