[ovs-dev] [PATCH v2] ovn-northd: Apply pre ACLs when using Port Groups

Lucas Alvares Gomes lucasagomes at gmail.com
Wed Jun 20 09:35:16 UTC 2018


On Wed, Jun 20, 2018 at 3:18 AM, 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: Lucas Alvares Gomes <lucasagomes at gmail.com>

Thanks for the patch, I just pulled it in a test environment along
with the port group support series for networking-ovn [0], gave it a
go and it works.

[stack at host11 devstack]$ sudo ip net e
ovnmeta-7b7909ae-b000-4196-9433-6a6dea0f352e ssh cirros at 10.0.0.3
The authenticity of host '10.0.0.3 (10.0.0.3)' can't be established.
RSA key fingerprint is SHA256:hECvgpUHbfO+hrwfOFmh6DYZxTL0p8gjBPkl8BwSNxA.
RSA key fingerprint is MD5:5c:12:10:b4:ca:a6:33:cc:52:8b:9b:06:c1:28:4f:2c.
Are you sure you want to continue connecting (yes/no)? yes
Warning: Permanently added '10.0.0.3' (RSA) to the list of known hosts.
cirros at 10.0.0.3's password:
$ curl 169.254.169.254
1.0
2007-01-19
2007-03-01
2007-08-29
2007-10-10
2007-12-15
2008-02-01
2008-09-01
2009-04-04
latest$

[0] https://review.openstack.org/#/c/570186/


More information about the dev mailing list