[ovs-dev] [PATCH v4 2/2] ovn: Add address_set() support for ACLs.
Zong Kai LI
zealokii at gmail.com
Mon Jun 27 15:20:43 UTC 2016
>
> +static void
> +update_address_sets(struct controller_ctx *ctx)
> +{
> + /* Remember the names of all address sets currently in
> expr_address_sets
> + * so we can detect address sets that have been deleted. */
> + struct sset cur_addr_set_names =
> SSET_INITIALIZER(&cur_addr_set_names);
> +
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &local_address_sets) {
> + sset_add(&cur_addr_set_names, node->name);
> + }
> +
> + /* Iterate address sets in the southbound database. Create and
> update the
> + * corresponding symtab entries as necessary. */
> + const struct sbrec_address_set *addr_set_rec;
> + SBREC_ADDRESS_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
> + node = shash_find(&local_address_sets, addr_set_rec->name);
> + struct address_set *addr_set = node ? node->data : NULL;
> +
>
How about using shash_find_data here?
struct address_set *addr_set = shash_find_data(&local_address_sets,
addr_set_rec->name);
+
> + /* Anything remaining in cur_addr_set_names refers to an address set
> that
> + * has been deleted from the southbound database. We should delete
> + * the corresponding symtab entry. */
> + const char *cur_node, *next_node;
> + SSET_FOR_EACH_SAFE (cur_node, next_node, &cur_addr_set_names) {
> + expr_macros_remove(&expr_address_sets, cur_node);
> +
> + struct address_set *addr_set
> + = shash_find_and_delete(&local_address_sets, cur_node);
> + address_set_destroy(addr_set);
> +
> + struct sset_node *sset_node = SSET_NODE_FROM_NAME(cur_node);
> + sset_delete(&cur_addr_set_names, sset_node);
>
Trivial: this seems unnecessary, sset_destroy will call sset_clear, and
sset_clear will call sset_delete.
> + }
> +
> + sset_destroy(&cur_addr_set_names);
> +}
>
> +static void
> +sync_address_sets(struct northd_context *ctx)
> +{
> + struct shash sb_address_sets = SHASH_INITIALIZER(&sb_address_sets);
> +
> + const struct sbrec_address_set *sb_address_set;
> + SBREC_ADDRESS_SET_FOR_EACH (sb_address_set, ctx->ovnsb_idl) {
> + shash_add(&sb_address_sets, sb_address_set->name, sb_address_set);
> + }
> +
> + const struct nbrec_address_set *nb_address_set;
> + NBREC_ADDRESS_SET_FOR_EACH (nb_address_set, ctx->ovnnb_idl) {
> + sb_address_set = shash_find_and_delete(&sb_address_sets,
> + nb_address_set->name);
> + if (!sb_address_set) {
> + sb_address_set = sbrec_address_set_insert(ctx->ovnsb_txn);
> + sbrec_address_set_set_name(sb_address_set,
> nb_address_set->name);
> + }
> +
> + sbrec_address_set_set_addresses(sb_address_set,
> + /* "char **" is not compatible with "const char **" */
> + (const char **) nb_address_set->addresses,
> + nb_address_set->n_addresses);
> + }
> +
> + struct shash_node *node, *next;
> + SHASH_FOR_EACH_SAFE (node, next, &sb_address_sets) {
> + sbrec_address_set_delete(node->data);
>
Trivial: this seems unnecessary, shash_destroy will delete and free node in
sb_address_sets.
+ shash_delete(&sb_address_sets, node);
> + }
> + shash_destroy(&sb_address_sets);
> +}
>
>
> + <table name="Address_Set" title="Address Sets">
> + <p>
> + Each row in this table represents a named set of addresses.
> + An address set may contain MAC, IPv4, or IPv6 addresses and cidrs.
> + The address set will ultimately be used in ACLs, where a certain
> + type of field such as ip4.src or ip6.src will be compared with the
> + address set. So, a single address set must contain addresses of the
> + same type.
> + </p>
> +
>
Thanks for updating this.
More information about the dev
mailing list