From patchwork Thu Mar 1 03:37:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 879530 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="FX712laL"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zsJ6p2SMHz9s1B for ; Thu, 1 Mar 2018 14:38:07 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 4DF21FBE; Thu, 1 Mar 2018 03:38:05 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 930DBF98 for ; Thu, 1 Mar 2018 03:38:03 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f52.google.com (mail-pg0-f52.google.com [74.125.83.52]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 08B9612F for ; Thu, 1 Mar 2018 03:38:00 +0000 (UTC) Received: by mail-pg0-f52.google.com with SMTP id l24so1811716pgc.5 for ; Wed, 28 Feb 2018 19:38:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=SBeNP6gxJRDfYMa+h4bSi4rUsTXzFMa+tvb6dWu/814=; b=FX712laLp/gtxDfuuwAOV5OPIiiE1w2vrlIxintXuHWjFVGL8liO3wk76Hir24w8N1 mwdJmfFtlXt47WbBNwaWVaWkVPc9jJCpdjEOJyoE17A3riU+dS80g5dSW5NR48PkFEeT C1E81E79PYINBa75bkLCbtWpwV8jDzoyNrfSxrvUOBBqh09MZFcJWd95poOd7jqocCGX AHqsrwjR773tkEbZJ4H4dg+UY6DsAGZWXdi1pM+z42tJlidiv8MbpqtOl04L2wwfMHqm rATCSmn2kQXHlYaf6QzW/iYLkTHowDbmiKqbjte1gURMu3R4FA+qi8HOHpB4pVyzXHlL amrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=SBeNP6gxJRDfYMa+h4bSi4rUsTXzFMa+tvb6dWu/814=; b=mUZQIPIYBvl7zrw6Zhypdw78q0ijtKayp+CkFKe6NjiI6ERDM3fAyGva3qF9B3KREk ybeFi1fy6NqcuWQr9ede5Y7kyJFHsUQE6gm3gvx+L3rAHY4zMuWLYMstfsupoUpa6ZXH qHPVlnTiOhdc4+Dlbd4i7Q/nfA5uy0ks51ZV5ry9G7KBnQBvyZia4bA5IwSl4pN5fwE1 cdDZToOywWWrZleo6qor44SfaCoiRvwJbf6Gbj6GgVVjF7RgRQ6hRzRBNFIMHz8dkfVb btKRW/e9aaUVStuf8Cnv+Cwv06RoNiKqLJxrcOnuC6/B1Pf1RaJ8jHBMgrwRdVLpwczN UI9w== X-Gm-Message-State: APf1xPAkl+5X9TDTI3kSU/AV2pX5rZSNCUMFQeh6NqQ+WCws897tWkmQ UwRZtSAHuB3voyI2DKmGNxoa6A== X-Google-Smtp-Source: AG47ELv7za9E0SyjH43nv35+p32eXUk2tVPt/4xOfEetK2kmjyAvGEhz1E2O1yGQ1y8++4Mmtyj7GA== X-Received: by 10.99.113.75 with SMTP id b11mr371406pgn.271.1519875479802; Wed, 28 Feb 2018 19:37:59 -0800 (PST) Received: from localhost.localdomain.localdomain ([216.113.160.70]) by smtp.gmail.com with ESMTPSA id c67sm6559610pfl.106.2018.02.28.19.37.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Feb 2018 19:37:59 -0800 (PST) From: Han Zhou X-Google-Original-From: Han Zhou To: dev@openvswitch.org Date: Wed, 28 Feb 2018 19:37:42 -0800 Message-Id: <1519875463-65690-1-git-send-email-hzhou8@ebay.com> X-Mailer: git-send-email 2.1.0 X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 1/2] ovn: Support port groups in ACLs X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This patch enables using port group names in ACL match conditions. Users can create a port group in northbound DB Port_Group table, and then use the name of the port group in ACL match conditions for "inport" or "outport". It can help reduce the number of ACLs for CMS clients such as OpenStack Neutron, for the use cases where a group of logical ports share same ACL rules except the "inport"/"outport" part. Without this patch, the clients have to create N (N = number of lports) ACLs, and this patch helps achieve the same goal with only one ACL. E.g.: to-lport 1000 "outport == @port_group1 && ip4.src == {IP1, IP2, ...}" allow-related There was a similar attempt by Zong Kai Li in 2016 [1]. This patch takes a slightly different approach by using weak refs instead of strings, which requires a new table instead of reusing the address set table. This way it will also benefit for a follow up patch that enables generating address sets automatically from port groups to avoid a lot a trouble from client perspective [2]. [1] https://mail.openvswitch.org/pipermail/ovs-dev/2016-August/077118.html [2] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046260.html Reported-by: Daniel Alvarez Sanchez Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046166.html Signed-off-by: Han Zhou Tested-By: Daniel Alvarez Reviewed-by: Daniel Alvarez --- NEWS | 2 + include/ovn/expr.h | 24 +++++--- include/ovn/lex.h | 1 + ovn/controller/lflow.c | 16 +++-- ovn/controller/lflow.h | 1 + ovn/controller/ofctrl.c | 6 +- ovn/controller/ofctrl.h | 3 +- ovn/controller/ovn-controller.c | 31 +++++++--- ovn/lib/actions.c | 3 +- ovn/lib/expr.c | 127 ++++++++++++++++++++++++++++------------ ovn/lib/lex.c | 20 +++++++ ovn/northd/ovn-northd.c | 47 +++++++++++++++ ovn/ovn-nb.ovsschema | 15 ++++- ovn/ovn-nb.xml | 30 ++++++++++ ovn/ovn-sb.ovsschema | 10 +++- ovn/ovn-sb.xml | 20 +++++++ ovn/utilities/ovn-trace.c | 27 +++++++-- tests/ovn.at | 34 +++++++++++ tests/test-ovn.c | 43 ++++++++++---- 19 files changed, 381 insertions(+), 79 deletions(-) diff --git a/NEWS b/NEWS index 4230189..da2c3ec 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,8 @@ Post-v2.9.0 * OFPT_ROLE_STATUS is now available in OpenFlow 1.3. - Linux kernel 4.14 * Add support for compiling OVS with the latest Linux 4.14 kernel + - OVN: + * Port_Group is supported in ACL match conditions. v2.9.0 - 19 Feb 2018 -------------------- diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 711713e..3995e62 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -382,9 +382,11 @@ expr_from_node(const struct ovs_list *node) void expr_format(const struct expr *, struct ds *); void expr_print(const struct expr *); struct expr *expr_parse(struct lexer *, const struct shash *symtab, - const struct shash *addr_sets); + const struct shash *addr_sets, + const struct shash *port_groups); struct expr *expr_parse_string(const char *, const struct shash *symtab, const struct shash *addr_sets, + const struct shash *port_groups, char **errorp); struct expr *expr_clone(struct expr *); @@ -404,6 +406,7 @@ bool expr_is_normalized(const struct expr *); char *expr_parse_microflow(const char *, const struct shash *symtab, const struct shash *addr_sets, + const struct shash *port_groups, bool (*lookup_port)(const void *aux, const char *port_name, unsigned int *portp), @@ -486,19 +489,22 @@ void expr_constant_set_format(const struct expr_constant_set *, struct ds *); void expr_constant_set_destroy(struct expr_constant_set *cs); -/* Address sets. +/* Constant sets. * - * Instead of referring to a set of value as: + * For example, instead of referring to a set of IP addresses as: * {addr1, addr2, ..., addrN} * You can register a set of values and refer to them as: * $name - * The address set entries should all have integer/masked-integer values. - * The values that don't qualify are ignored. + * + * If convert_to_integer is true, the set must contain + * integer/masked-integer values. The values that don't qualify + * are ignored. */ -void expr_addr_sets_add(struct shash *addr_sets, const char *name, - const char * const *values, size_t n_values); -void expr_addr_sets_remove(struct shash *addr_sets, const char *name); -void expr_addr_sets_destroy(struct shash *addr_sets); +void expr_const_sets_add(struct shash *const_sets, const char *name, + const char * const *values, size_t n_values, + bool convert_to_integer); +void expr_const_sets_remove(struct shash *const_sets, const char *name); +void expr_const_sets_destroy(struct shash *const_sets); #endif /* ovn/expr.h */ diff --git a/include/ovn/lex.h b/include/ovn/lex.h index afa4a23..8d55857 100644 --- a/include/ovn/lex.h +++ b/include/ovn/lex.h @@ -37,6 +37,7 @@ enum lex_type { LEX_T_INTEGER, /* 12345 or 1.2.3.4 or ::1 or 01:02:03:04:05 */ LEX_T_MASKED_INTEGER, /* 12345/10 or 1.2.0.0/16 or ::2/127 or... */ LEX_T_MACRO, /* $NAME */ + LEX_T_PORT_GROUP, /* @NAME */ LEX_T_ERROR, /* invalid input */ /* Bare tokens. */ diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index df125b1..cc2a020 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -70,6 +70,7 @@ static void consider_logical_flow(struct controller_ctx *ctx, struct hmap *nd_ra_opts, uint32_t *conj_id_ofs, const struct shash *addr_sets, + const struct shash *port_groups, struct hmap *flow_table, struct sset *active_tunnels, struct sset *local_lport_ids); @@ -149,6 +150,7 @@ add_logical_flows(struct controller_ctx *ctx, struct ovn_extend_table *meter_table, const struct sbrec_chassis *chassis, const struct shash *addr_sets, + const struct shash *port_groups, struct hmap *flow_table, struct sset *active_tunnels, struct sset *local_lport_ids) @@ -179,8 +181,8 @@ add_logical_flows(struct controller_ctx *ctx, lflow, local_datapaths, group_table, meter_table, chassis, &dhcp_opts, &dhcpv6_opts, &nd_ra_opts, - &conj_id_ofs, addr_sets, flow_table, - active_tunnels, local_lport_ids); + &conj_id_ofs, addr_sets, port_groups, + flow_table, active_tunnels, local_lport_ids); } dhcp_opts_destroy(&dhcp_opts); @@ -201,6 +203,7 @@ consider_logical_flow(struct controller_ctx *ctx, struct hmap *nd_ra_opts, uint32_t *conj_id_ofs, const struct shash *addr_sets, + const struct shash *port_groups, struct hmap *flow_table, struct sset *active_tunnels, struct sset *local_lport_ids) @@ -258,7 +261,8 @@ consider_logical_flow(struct controller_ctx *ctx, struct hmap matches; struct expr *expr; - expr = expr_parse_string(lflow->match, &symtab, addr_sets, &error); + expr = expr_parse_string(lflow->match, &symtab, addr_sets, port_groups, + &error); if (!error) { if (prereqs) { expr = expr_combine(EXPR_T_AND, expr, prereqs); @@ -450,13 +454,15 @@ lflow_run(struct controller_ctx *ctx, struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, const struct shash *addr_sets, + const struct shash *port_groups, struct hmap *flow_table, struct sset *active_tunnels, struct sset *local_lport_ids) { add_logical_flows(ctx, chassis_index, local_datapaths, - group_table, meter_table, chassis, addr_sets, flow_table, - active_tunnels, local_lport_ids); + group_table, meter_table, chassis, addr_sets, + port_groups, flow_table, active_tunnels, + local_lport_ids); add_neighbor_flows(ctx, flow_table); } diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h index 22bf534..dcf2fe7 100644 --- a/ovn/controller/lflow.h +++ b/ovn/controller/lflow.h @@ -69,6 +69,7 @@ void lflow_run(struct controller_ctx *, struct ovn_extend_table *group_table, struct ovn_extend_table *meter_table, const struct shash *addr_sets, + const struct shash *port_groups, struct hmap *flow_table, struct sset *active_tunnels, struct sset *local_lport_ids); diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index 8d6d1b6..e3ee6b1 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -1145,7 +1145,8 @@ ofctrl_lookup_port(const void *br_int_, const char *port_name, * must free(). */ char * ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, const char *flow_s, - const struct shash *addr_sets) + const struct shash *addr_sets, + const struct shash *port_groups) { int version = rconn_get_version(swconn); if (version < 0) { @@ -1154,7 +1155,8 @@ ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, const char *flow_s, struct flow uflow; char *error = expr_parse_microflow(flow_s, &symtab, addr_sets, - ofctrl_lookup_port, br_int, &uflow); + port_groups, ofctrl_lookup_port, + br_int, &uflow); if (error) { return error; } diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h index d53bc68..a8b3afc 100644 --- a/ovn/controller/ofctrl.h +++ b/ovn/controller/ofctrl.h @@ -47,7 +47,8 @@ struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source); void ofctrl_ct_flush_zone(uint16_t zone_id); char *ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, - const char *flow_s, const struct shash *addr_sets); + const char *flow_s, const struct shash *addr_sets, + const struct shash *port_groups); /* Flow table interfaces to the rest of ovn-controller. */ void ofctrl_add_flow(struct hmap *desired_flows, uint8_t table_id, diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 7592bda..492e4fa 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -288,9 +288,22 @@ addr_sets_init(struct controller_ctx *ctx, struct shash *addr_sets) { const struct sbrec_address_set *as; SBREC_ADDRESS_SET_FOR_EACH (as, ctx->ovnsb_idl) { - expr_addr_sets_add(addr_sets, as->name, - (const char *const *) as->addresses, - as->n_addresses); + expr_const_sets_add(addr_sets, as->name, + (const char *const *) as->addresses, + as->n_addresses, true); + } +} + +/* Iterate port groups in the southbound database. Create and update the + * corresponding symtab entries as necessary. */ +static void +port_groups_init(struct controller_ctx *ctx, struct shash *port_groups) +{ + const struct sbrec_port_group *pg; + SBREC_PORT_GROUP_FOR_EACH (pg, ctx->ovnsb_idl) { + expr_const_sets_add(port_groups, pg->name, + (const char *const *) pg->ports, + pg->n_ports, false); } } @@ -696,6 +709,8 @@ main(int argc, char *argv[]) if (br_int && chassis) { struct shash addr_sets = SHASH_INITIALIZER(&addr_sets); addr_sets_init(&ctx, &addr_sets); + struct shash port_groups = SHASH_INITIALIZER(&port_groups); + port_groups_init(&ctx, &port_groups); patch_run(&ctx, br_int, chassis); @@ -713,8 +728,8 @@ main(int argc, char *argv[]) struct hmap flow_table = HMAP_INITIALIZER(&flow_table); lflow_run(&ctx, chassis, &chassis_index, &local_datapaths, &group_table, - &meter_table, &addr_sets, &flow_table, - &active_tunnels, &local_lport_ids); + &meter_table, &addr_sets, &port_groups, + &flow_table, &active_tunnels, &local_lport_ids); if (chassis_id) { bfd_run(&ctx, br_int, chassis, &local_datapaths, @@ -740,7 +755,7 @@ main(int argc, char *argv[]) if (pending_pkt.conn) { char *error = ofctrl_inject_pkt(br_int, pending_pkt.flow_s, - &addr_sets); + &port_groups, &addr_sets); if (error) { unixctl_command_reply_error(pending_pkt.conn, error); free(error); @@ -754,8 +769,10 @@ main(int argc, char *argv[]) update_sb_monitors(ctx.ovnsb_idl, chassis, &local_lports, &local_datapaths); - expr_addr_sets_destroy(&addr_sets); + expr_const_sets_destroy(&addr_sets); shash_destroy(&addr_sets); + expr_const_sets_destroy(&port_groups); + shash_destroy(&port_groups); } /* If we haven't handled the pending packet insertion diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index fde3bff..8845e77 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -234,7 +234,8 @@ add_prerequisite(struct action_context *ctx, const char *prerequisite) struct expr *expr; char *error; - expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, &error); + expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL, + &error); ovs_assert(!error); ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr); } diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c index b0aa672..1a33269 100644 --- a/ovn/lib/expr.c +++ b/ovn/lib/expr.c @@ -464,6 +464,7 @@ struct expr_context { struct lexer *lexer; /* Lexer for pulling more tokens. */ const struct shash *symtab; /* Symbol table. */ const struct shash *addr_sets; /* Address set table. */ + const struct shash *port_groups; /* Port groups table. */ bool not; /* True inside odd number of NOT operators. */ }; @@ -752,6 +753,36 @@ parse_addr_sets(struct expr_context *ctx, struct expr_constant_set *cs, } static bool +parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs, + size_t *allocated_values) +{ + struct expr_constant_set *port_group + = (ctx->port_groups + ? shash_find_data(ctx->port_groups, ctx->lexer->token.s) + : NULL); + if (!port_group) { + lexer_syntax_error(ctx->lexer, "expecting port group name"); + return false; + } + + if (!assign_constant_set_type(ctx, cs, EXPR_C_STRING)) { + return false; + } + + size_t n_values = cs->n_values + port_group->n_values; + if (n_values >= *allocated_values) { + cs->values = xrealloc(cs->values, n_values * sizeof *cs->values); + *allocated_values = n_values; + } + for (size_t i = 0; i < port_group->n_values; i++) { + cs->values[cs->n_values++].string = + xstrdup(port_group->values[i].string); + } + + return true; +} + +static bool parse_constant(struct expr_context *ctx, struct expr_constant_set *cs, size_t *allocated_values) { @@ -788,6 +819,12 @@ parse_constant(struct expr_context *ctx, struct expr_constant_set *cs, } lexer_get(ctx->lexer); return true; + } else if (ctx->lexer->token.type == LEX_T_PORT_GROUP) { + if (!parse_port_group(ctx, cs, allocated_values)) { + return false; + } + lexer_get(ctx->lexer); + return true; } else { lexer_syntax_error(ctx->lexer, "expecting constant"); return false; @@ -939,65 +976,74 @@ expr_constant_set_destroy(struct expr_constant_set *cs) } } -/* Adds an address set named 'name' to 'addr_sets', replacing any existing - * address set entry with the given name. */ +/* Adds an constant set named 'name' to 'const_sets', replacing any existing + * constant set entry with the given name. */ void -expr_addr_sets_add(struct shash *addr_sets, const char *name, - const char *const *values, size_t n_values) +expr_const_sets_add(struct shash *const_sets, const char *name, + const char *const *values, size_t n_values, + bool convert_to_integer) { /* Replace any existing entry for this name. */ - expr_addr_sets_remove(addr_sets, name); + expr_const_sets_remove(const_sets, name); struct expr_constant_set *cs = xzalloc(sizeof *cs); - cs->type = EXPR_C_INTEGER; cs->in_curlies = true; cs->n_values = 0; cs->values = xmalloc(n_values * sizeof *cs->values); - for (size_t i = 0; i < n_values; i++) { - /* Use the lexer to convert each address set into the proper - * integer format. */ - struct lexer lex; - lexer_init(&lex, values[i]); - lexer_get(&lex); - if (lex.token.type != LEX_T_INTEGER - && lex.token.type != LEX_T_MASKED_INTEGER) { - VLOG_WARN("Invalid address set entry: '%s', token type: %d", - values[i], lex.token.type); - } else { - union expr_constant *c = &cs->values[cs->n_values++]; - c->value = lex.token.value; - c->format = lex.token.format; - c->masked = lex.token.type == LEX_T_MASKED_INTEGER; - if (c->masked) { - c->mask = lex.token.mask; + if (convert_to_integer) { + cs->type = EXPR_C_INTEGER; + for (size_t i = 0; i < n_values; i++) { + /* Use the lexer to convert each constant set into the proper + * integer format. */ + struct lexer lex; + lexer_init(&lex, values[i]); + lexer_get(&lex); + if (lex.token.type != LEX_T_INTEGER + && lex.token.type != LEX_T_MASKED_INTEGER) { + VLOG_WARN("Invalid constant set entry: '%s', token type: %d", + values[i], lex.token.type); + } else { + union expr_constant *c = &cs->values[cs->n_values++]; + c->value = lex.token.value; + c->format = lex.token.format; + c->masked = lex.token.type == LEX_T_MASKED_INTEGER; + if (c->masked) { + c->mask = lex.token.mask; + } } + lexer_destroy(&lex); + } + } else { + cs->type = EXPR_C_STRING; + for (size_t i = 0; i < n_values; i++) { + union expr_constant *c = &cs->values[cs->n_values++]; + c->string = xstrdup(values[i]); } - lexer_destroy(&lex); } - shash_add(addr_sets, name, cs); + shash_add(const_sets, name, cs); } void -expr_addr_sets_remove(struct shash *addr_sets, const char *name) +expr_const_sets_remove(struct shash *const_sets, const char *name) { - struct expr_constant_set *cs = shash_find_and_delete(addr_sets, name); + struct expr_constant_set *cs = shash_find_and_delete(const_sets, name); if (cs) { expr_constant_set_destroy(cs); free(cs); } } -/* Destroy all contents of 'addr_sets'. */ +/* Destroy all contents of 'const_sets'. */ void -expr_addr_sets_destroy(struct shash *addr_sets) +expr_const_sets_destroy(struct shash *const_sets) { struct shash_node *node, *next; - SHASH_FOR_EACH_SAFE (node, next, addr_sets) { + SHASH_FOR_EACH_SAFE (node, next, const_sets) { struct expr_constant_set *cs = node->data; - shash_delete(addr_sets, node); + shash_delete(const_sets, node); expr_constant_set_destroy(cs); free(cs); } @@ -1214,11 +1260,13 @@ expr_parse__(struct expr_context *ctx) * lexer->error is NULL. */ struct expr * expr_parse(struct lexer *lexer, const struct shash *symtab, - const struct shash *addr_sets) + const struct shash *addr_sets, + const struct shash *port_groups) { struct expr_context ctx = { .lexer = lexer, .symtab = symtab, - .addr_sets = addr_sets }; + .addr_sets = addr_sets, + .port_groups = port_groups }; return lexer->error ? NULL : expr_parse__(&ctx); } @@ -1230,13 +1278,15 @@ expr_parse(struct lexer *lexer, const struct shash *symtab, * error (with free()). */ struct expr * expr_parse_string(const char *s, const struct shash *symtab, - const struct shash *addr_sets, char **errorp) + const struct shash *addr_sets, + const struct shash *port_groups, + char **errorp) { struct lexer lexer; lexer_init(&lexer, s); lexer_get(&lexer); - struct expr *expr = expr_parse(&lexer, symtab, addr_sets); + struct expr *expr = expr_parse(&lexer, symtab, addr_sets, port_groups); lexer_force_end(&lexer); *errorp = lexer_steal_error(&lexer); if (*errorp) { @@ -1460,7 +1510,7 @@ expr_get_level(const struct expr *expr) static enum expr_level expr_parse_level(const char *s, const struct shash *symtab, char **errorp) { - struct expr *expr = expr_parse_string(s, symtab, NULL, errorp); + struct expr *expr = expr_parse_string(s, symtab, NULL, NULL, errorp); enum expr_level level = expr ? expr_get_level(expr) : EXPR_L_NOMINAL; expr_destroy(expr); return level; @@ -1618,7 +1668,7 @@ parse_and_annotate(const char *s, const struct shash *symtab, char *error; struct expr *expr; - expr = expr_parse_string(s, symtab, NULL, &error); + expr = expr_parse_string(s, symtab, NULL, NULL, &error); if (expr) { expr = expr_annotate__(expr, symtab, nesting, &error); } @@ -3277,6 +3327,7 @@ expr_parse_microflow__(struct lexer *lexer, char * OVS_WARN_UNUSED_RESULT expr_parse_microflow(const char *s, const struct shash *symtab, const struct shash *addr_sets, + const struct shash *port_groups, bool (*lookup_port)(const void *aux, const char *port_name, unsigned int *portp), @@ -3286,7 +3337,7 @@ expr_parse_microflow(const char *s, const struct shash *symtab, lexer_init(&lexer, s); lexer_get(&lexer); - struct expr *e = expr_parse(&lexer, symtab, addr_sets); + struct expr *e = expr_parse(&lexer, symtab, addr_sets, port_groups); lexer_force_end(&lexer); if (e) { diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c index 2f49af0..9463640 100644 --- a/ovn/lib/lex.c +++ b/ovn/lib/lex.c @@ -231,6 +231,10 @@ lex_token_format(const struct lex_token *token, struct ds *s) ds_put_format(s, "$%s", token->s); break; + case LEX_T_PORT_GROUP: + ds_put_format(s, "@%s", token->s); + break; + case LEX_T_LPAREN: ds_put_cstr(s, "("); break; @@ -573,6 +577,18 @@ lex_parse_addr_set(const char *p, struct lex_token *token) return lex_parse_id(p, LEX_T_MACRO, token); } +static const char * +lex_parse_port_group(const char *p, struct lex_token *token) +{ + p++; + if (!lex_is_id1(*p)) { + lex_error(token, "`$' must be followed by a valid identifier."); + return p; + } + + return lex_parse_id(p, LEX_T_PORT_GROUP, token); +} + /* Initializes 'token' and parses the first token from the beginning of * null-terminated string 'p' into 'token'. Stores a pointer to the start of * the token (after skipping white space and comments, if any) into '*startp'. @@ -747,6 +763,10 @@ next: p = lex_parse_addr_set(p, token); break; + case '@': + p = lex_parse_port_group(p, token); + break; + case ':': if (p[1] != ':') { token->type = LEX_T_COLON; diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index d83681c..2924d8f 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -6135,6 +6135,49 @@ sync_address_sets(struct northd_context *ctx) shash_destroy(&sb_address_sets); } +/* Each port group in Port_Group table in OVN_Northbound has a corresponding + * entry in Port_Group table in OVN_Southbound. In OVN_Northbound the entries + * contains lport uuids, while in OVN_Southbound we store the lport names. + */ +static void +sync_port_groups(struct northd_context *ctx) +{ + struct shash sb_port_groups = SHASH_INITIALIZER(&sb_port_groups); + + const struct sbrec_port_group *sb_port_group; + SBREC_PORT_GROUP_FOR_EACH (sb_port_group, ctx->ovnsb_idl) { + shash_add(&sb_port_groups, sb_port_group->name, sb_port_group); + } + + const struct nbrec_port_group *nb_port_group; + NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) { + sb_port_group = shash_find_and_delete(&sb_port_groups, + nb_port_group->name); + if (!sb_port_group) { + sb_port_group = sbrec_port_group_insert(ctx->ovnsb_txn); + sbrec_port_group_set_name(sb_port_group, nb_port_group->name); + } + + const char **nb_port_names = xcalloc(nb_port_group->n_ports, + sizeof *nb_port_names); + int i; + for (i = 0; i < nb_port_group->n_ports; i++) { + nb_port_names[i] = nb_port_group->ports[i]->name; + } + sbrec_port_group_set_ports(sb_port_group, + nb_port_names, + nb_port_group->n_ports); + free(nb_port_names); + } + + struct shash_node *node, *next; + SHASH_FOR_EACH_SAFE (node, next, &sb_port_groups) { + sbrec_port_group_delete(node->data); + shash_delete(&sb_port_groups, node); + } + shash_destroy(&sb_port_groups); +} + /* * struct 'dns_info' is used to sync the DNS records between OVN Northbound db * and Southbound db. @@ -6251,6 +6294,7 @@ ovnnb_db_run(struct northd_context *ctx, struct chassis_index *chassis_index, build_lflows(ctx, &datapaths, &ports); sync_address_sets(ctx); + sync_port_groups(ctx); sync_dns_entries(ctx, &datapaths); struct ovn_datapath *dp, *next_dp; @@ -6848,6 +6892,9 @@ main(int argc, char *argv[]) ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_addresses); + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_port_group); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_group_col_name); + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_group_col_ports); ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_dns); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_dns_col_datapaths); diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema index 32f9d5a..2d09282 100644 --- a/ovn/ovn-nb.ovsschema +++ b/ovn/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", "version": "5.10.0", - "cksum": "626737541 17810", + "cksum": "222987318 18430", "tables": { "NB_Global": { "columns": { @@ -114,6 +114,19 @@ "min": 0, "max": "unlimited"}}}, "indexes": [["name"]], "isRoot": true}, + "Port_Group": { + "columns": { + "name": {"type": "string"}, + "ports": {"type": {"key": {"type": "uuid", + "refTable": "Logical_Switch_Port", + "refType": "weak"}, + "min": 0, + "max": "unlimited"}}, + "external_ids": { + "type": {"key": "string", "value": "string", + "min": 0, "max": "unlimited"}}}, + "indexes": [["name"]], + "isRoot": true}, "Load_Balancer": { "columns": { "name": {"type": "string"}, diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index b7a5b6b..83727c5 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -922,6 +922,36 @@ + +

+ Each row in this table represents a named group of logical switch ports. +

+ +

+ Port groups may be used in the column + of the table. For syntax information, see the details + of the expression language used for the column in the table of the database. +

+ + + A name for the port group. Names are ASCII and must match + [a-zA-Z_.][a-zA-Z_.0-9]*. + + + + The logical switch ports belonging to the group in uuids. + + + + + See External IDs at the beginning of this document. + + +
+

Each row represents one load balancer. diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema index 2abcc6b..9e271d4 100644 --- a/ovn/ovn-sb.ovsschema +++ b/ovn/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", "version": "1.15.0", - "cksum": "70426956 13327", + "cksum": "1839738004 13639", "tables": { "SB_Global": { "columns": { @@ -55,6 +55,14 @@ "max": "unlimited"}}}, "indexes": [["name"]], "isRoot": true}, + "Port_Group": { + "columns": { + "name": {"type": "string"}, + "ports": {"type": {"key": "string", + "min": 0, + "max": "unlimited"}}}, + "indexes": [["name"]], + "isRoot": true}, "Logical_Flow": { "columns": { "logical_datapath": {"type": {"key": {"type": "uuid", diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index f000b16..2eac943 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -377,6 +377,18 @@

+ +

+ This table contains names for the logical switch ports in the + database that belongs to the same group + that is defined in + in the database. +

+ + + +
+

Each row in this table represents one logical flow. @@ -770,6 +782,14 @@ $set1.

+

+ You may refer to a group of logical switch ports stored in the + table by its . An with a name + of port_group1 can be referred to as + @port_group1. +

+

Miscellaneous

diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c index db39c49..5a9e73d 100644 --- a/ovn/utilities/ovn-trace.c +++ b/ovn/utilities/ovn-trace.c @@ -418,6 +418,9 @@ static struct shash symtab; /* Address sets. */ static struct shash address_sets; +/* Port groups. */ +static struct shash port_groups; + /* DHCP options. */ static struct hmap dhcp_opts; /* Contains "struct gen_opts_map"s. */ static struct hmap dhcpv6_opts; /* Contains "struct gen_opts_map"s. */ @@ -697,9 +700,22 @@ read_address_sets(void) const struct sbrec_address_set *sbas; SBREC_ADDRESS_SET_FOR_EACH (sbas, ovnsb_idl) { - expr_addr_sets_add(&address_sets, sbas->name, + expr_const_sets_add(&address_sets, sbas->name, (const char *const *) sbas->addresses, - sbas->n_addresses); + sbas->n_addresses, true); + } +} + +static void +read_port_groups(void) +{ + shash_init(&port_groups); + + const struct sbrec_port_group *sbpg; + SBREC_PORT_GROUP_FOR_EACH (sbpg, ovnsb_idl) { + expr_const_sets_add(&port_groups, sbpg->name, + (const char *const *) sbpg->ports, + sbpg->n_ports, false); } } @@ -796,7 +812,8 @@ read_flows(void) char *error; struct expr *match; - match = expr_parse_string(sblf->match, &symtab, &address_sets, &error); + match = expr_parse_string(sblf->match, &symtab, &address_sets, + &port_groups, &error); if (error) { VLOG_WARN("%s: parsing expression failed (%s)", sblf->match, error); @@ -937,6 +954,7 @@ read_db(void) read_ports(); read_mcgroups(); read_address_sets(); + read_port_groups(); read_gen_opts(); read_flows(); read_mac_bindings(); @@ -2009,7 +2027,8 @@ trace(const char *dp_s, const char *flow_s) struct flow uflow; char *error = expr_parse_microflow(flow_s, &symtab, &address_sets, - ovntrace_lookup_port, dp, &uflow); + &port_groups, ovntrace_lookup_port, + dp, &uflow); if (error) { char *s = xasprintf("error parsing flow: %s\n", error); free(error); diff --git a/tests/ovn.at b/tests/ovn.at index 8ee3bf0..c8ab7b7 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -304,6 +304,7 @@ inport = "eth0" => Syntax error at `=' expecting relational operator. 123 == 123 => Syntax error at `123' expecting field name. $name => Syntax error at `$name' expecting address set name. +@name => Syntax error at `@name' expecting port group name. 123 == xyzzy => Syntax error at `xyzzy' expecting field name. xyzzy == 1 => Syntax error at `xyzzy' expecting field name. @@ -657,6 +658,24 @@ ip,nw_src=8.0.0.0/8.0.0.0 ]) AT_CLEANUP +AT_SETUP([ovn -- converting expressions to flows -- port groups]) +AT_KEYWORDS([expression]) +expr_to_flow () { + echo "$1" | ovstest test-ovn expr-to-flows | sort +} +AT_CHECK([expr_to_flow 'outport == @pg1'], [0], [dnl +reg15=0x11 +reg15=0x12 +reg15=0x13 +]) +AT_CHECK([expr_to_flow 'outport == {@pg_empty}'], [0], [dnl +(no flows) +]) +AT_CHECK([expr_to_flow 'outport == {"lsp1", @pg_empty}'], [0], [dnl +reg15=0x11 +]) +AT_CLEANUP + AT_SETUP([ovn -- action parsing]) dnl Unindented text is input (a set of OVN logical actions). dnl Indented text is expected output. @@ -1194,6 +1213,13 @@ ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1236 && outport == "lp33"' d ovn-nbctl create Address_Set name=set1 addresses=\"f0:00:00:00:00:11\",\"f0:00:00:00:00:21\",\"f0:00:00:00:00:31\" ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1237 && eth.src == $set1 && outport == "lp33"' drop +get_lsp_uuid () { + ovn-nbctl lsp-list lsw0 | grep $1 | awk '{ print $1 }' +} + +ovn-nbctl create Port_Group name=pg1 ports=`get_lsp_uuid lp22`,`get_lsp_uuid lp33` +ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1238 && outport == @pg1' drop + # 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). @@ -1334,10 +1360,18 @@ for is in 1 2 3; do else acl4=$d fi + if test $d = $s || test $d = 22 || test $d = 33; then + # dest of 22 and 33 should be dropped + # due to the 5th ACL that uses port_group(pg1). + acl5= + else + acl5=$d + fi test_packet $s f000000000$d f000000000$s 1234 #7, acl1 test_packet $s f000000000$d f000000000$s 1235 $acl2 #7, acl2 test_packet $s f000000000$d f000000000$s 1236 $acl3 #7, acl3 test_packet $s f000000000$d f000000000$s 1237 $acl4 #7, acl4 + test_packet $s f000000000$d f000000000$s 1238 $acl5 #7, acl5 test_packet $s f000000000$d f00000000055 810000091234 #4 test_packet $s f000000000$d 0100000000$s $s$d #5 diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 7452753..d4a5d59 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -211,10 +211,24 @@ create_addr_sets(struct shash *addr_sets) }; static const char *const addrs4[] = { NULL }; - expr_addr_sets_add(addr_sets, "set1", addrs1, 3); - expr_addr_sets_add(addr_sets, "set2", addrs2, 3); - expr_addr_sets_add(addr_sets, "set3", addrs3, 3); - expr_addr_sets_add(addr_sets, "set4", addrs4, 0); + expr_const_sets_add(addr_sets, "set1", addrs1, 3, true); + expr_const_sets_add(addr_sets, "set2", addrs2, 3, true); + expr_const_sets_add(addr_sets, "set3", addrs3, 3, true); + expr_const_sets_add(addr_sets, "set4", addrs4, 0, true); +} + +static void +create_port_groups(struct shash *port_groups) +{ + shash_init(port_groups); + + static const char *const pg1[] = { + "lsp1", "lsp2", "lsp3", + }; + static const char *const pg2[] = { NULL }; + + expr_const_sets_add(port_groups, "pg1", pg1, 3, false); + expr_const_sets_add(port_groups, "pg_empty", pg2, 0, false); } static bool @@ -245,23 +259,29 @@ test_parse_expr__(int steps) { struct shash symtab; struct shash addr_sets; + struct shash port_groups; struct simap ports; struct ds input; create_symtab(&symtab); create_addr_sets(&addr_sets); + create_port_groups(&port_groups); simap_init(&ports); simap_put(&ports, "eth0", 5); simap_put(&ports, "eth1", 6); simap_put(&ports, "LOCAL", ofp_to_u16(OFPP_LOCAL)); + simap_put(&ports, "lsp1", 0x11); + simap_put(&ports, "lsp2", 0x12); + simap_put(&ports, "lsp3", 0x13); ds_init(&input); while (!ds_get_test_line(&input, stdin)) { struct expr *expr; char *error; - expr = expr_parse_string(ds_cstr(&input), &symtab, &addr_sets, &error); + expr = expr_parse_string(ds_cstr(&input), &symtab, &addr_sets, + &port_groups, &error); if (!error && steps > 0) { expr = expr_annotate(expr, &symtab, &error); } @@ -298,8 +318,10 @@ test_parse_expr__(int steps) simap_destroy(&ports); expr_symtab_destroy(&symtab); shash_destroy(&symtab); - expr_addr_sets_destroy(&addr_sets); + expr_const_sets_destroy(&addr_sets); shash_destroy(&addr_sets); + expr_const_sets_destroy(&port_groups); + shash_destroy(&port_groups); } static void @@ -373,7 +395,7 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx) ovn_init_symtab(&symtab); struct flow uflow; - char *error = expr_parse_microflow(ctx->argv[1], &symtab, NULL, + char *error = expr_parse_microflow(ctx->argv[1], &symtab, NULL, NULL, lookup_atoi_cb, NULL, &uflow); if (error) { ovs_fatal(0, "%s", error); @@ -383,7 +405,7 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx) while (!ds_get_test_line(&input, stdin)) { struct expr *expr; - expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, &error); + expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, NULL, &error); if (!error) { expr = expr_annotate(expr, &symtab, &error); } @@ -857,7 +879,8 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab, expr_format(expr, &s); char *error; - modified = expr_parse_string(ds_cstr(&s), symtab, NULL, &error); + modified = expr_parse_string(ds_cstr(&s), symtab, NULL, + NULL, &error); if (error) { fprintf(stderr, "%s fails to parse (%s)\n", ds_cstr(&s), error); @@ -1163,7 +1186,7 @@ test_expr_to_packets(struct ovs_cmdl_context *ctx OVS_UNUSED) while (!ds_get_test_line(&input, stdin)) { struct flow uflow; char *error = expr_parse_microflow(ds_cstr(&input), &symtab, NULL, - lookup_atoi_cb, NULL, &uflow); + NULL, lookup_atoi_cb, NULL, &uflow); if (error) { puts(error); free(error); From patchwork Thu Mar 1 03:37:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 879531 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="DaOqqgxw"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zsJ7c40lCz9s1B for ; Thu, 1 Mar 2018 14:38:52 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 86CA01011; Thu, 1 Mar 2018 03:38:10 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 1CCD3100B for ; Thu, 1 Mar 2018 03:38:09 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f54.google.com (mail-pg0-f54.google.com [74.125.83.54]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id EA10D12F for ; Thu, 1 Mar 2018 03:38:07 +0000 (UTC) Received: by mail-pg0-f54.google.com with SMTP id r26so1802389pgv.13 for ; Wed, 28 Feb 2018 19:38:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=k25o45gKybi1N6Ph8g2tA8i2we9SNHMPViWmaWn0iAc=; b=DaOqqgxwDIvXPXtWLRxTI2O1DSXXG99ugZsmKECxvq4LfS68iLCKUSzZZskswGh35C sWNuIxPgFdTsoIuVCLL2imoMLQ9Rdf2S+ooD2EmcAs6cXSLdb/MazyZEn6kxMNTOQNSv LC37tPMn7OhnG28t4NYRcqZBfwwosUeU2Hn9YJDzbIExIHIUPlsRE1Htyr94q9CDfR5t j3uJgXthZsGs8kNiST4mLD9k1/IPVbqZvXn/yRoZLQKsNKaWUlHay9xKaYXtVQEPJF0U oEMkPXtKXvamgaub5CUi9cLIis62GbA4czAKw2YtOMcVeK1SVun+i+3Xhh8lR/ZUFPlT gvvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=k25o45gKybi1N6Ph8g2tA8i2we9SNHMPViWmaWn0iAc=; b=ooyP8Aw0bbE3cOrMyFDRqmZxnJ+/AXko9k+rWaEONispzIfoYYHIJP2kDiE1Kv1G8Y J+Q/yjKRWPF9mvpf7O5l+/mom6GOo8SPXJFdnshlDgMTCCk7JrigAfHLaTDjCu0IxNrf rMknJM0ctVvmCYXJA6ZSy71CoGKuudOgDG/lIYIwkcfI8a0erQCVW3PK9KmdqbK6dUvi kfcui/gzWtVr1dqA0y1JXnVWq9A+HfaTihEtzGN1GeghmNduRfi4kRIgZXGSDUXq6OoV 9/SdRjdqzqg08gi0cUOWk/CZp6sNJZceEWQYkFJ1U2l+BeIOQEDFD78qvm3pTQk8WZrK FQvw== X-Gm-Message-State: APf1xPBqhD/xnGgZhs7pWWh/7oOvSsmELhvy81T87ZHmoUDl1gTu8MOh /MWbJLLlBWCkmTEZDXg49Rt5pQ== X-Google-Smtp-Source: AG47ELsmj6EyWmxB7ZElA46KSa8BTa4Bq5weax4FZ32eW0kmd2Kttwc3k+euXv6ZNNV5earicSZckg== X-Received: by 10.98.61.73 with SMTP id k70mr485843pfa.10.1519875486929; Wed, 28 Feb 2018 19:38:06 -0800 (PST) Received: from localhost.localdomain.localdomain ([216.113.160.70]) by smtp.gmail.com with ESMTPSA id c67sm6559610pfl.106.2018.02.28.19.38.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 28 Feb 2018 19:38:06 -0800 (PST) From: Han Zhou X-Google-Original-From: Han Zhou To: dev@openvswitch.org Date: Wed, 28 Feb 2018 19:37:43 -0800 Message-Id: <1519875463-65690-2-git-send-email-hzhou8@ebay.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1519875463-65690-1-git-send-email-hzhou8@ebay.com> References: <1519875463-65690-1-git-send-email-hzhou8@ebay.com> X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, LOTS_OF_MONEY, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH 2/2] ovn: Support address sets generated from port groups X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Address sets are automatically generated from corresponding port groups, and can be used directly in ACL match conditions. There are two address sets generated for each port group: _ip4 _ip6 For example, if port_group1 is created, we can directly use below match condition in ACL: "outport == @port_group1 && ip4.src == $port_group1_ip4" This will simplify OVN client implementation, and avoid some tricky problems such as race conditions when maintaining address set memberships as discussed in the link below. Reported-by: Lucas Alvares Gomes Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-February/046174.html Signed-off-by: Han Zhou Tested-By: Daniel Alvarez Acked-by: Daniel Alvarez --- NEWS | 3 +- ovn/northd/ovn-northd.c | 87 ++++++++++++++++--- ovn/ovn-nb.xml | 18 ++++ ovn/ovn-sb.xml | 23 ++++- tests/ovn.at | 226 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 340 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index da2c3ec..db98282 100644 --- a/NEWS +++ b/NEWS @@ -15,7 +15,8 @@ Post-v2.9.0 - Linux kernel 4.14 * Add support for compiling OVS with the latest Linux 4.14 kernel - OVN: - * Port_Group is supported in ACL match conditions. + * Port_Group and generated address sets are supported in ACL match + conditions. See ovn-nb(5) and ovn-sb(5) for details. v2.9.0 - 19 Feb 2018 -------------------- diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 2924d8f..11b9ab0 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -6098,9 +6098,32 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, hmap_destroy(&mcgroups); } -/* OVN_Northbound and OVN_Southbound have an identical Address_Set table. - * We always update OVN_Southbound to match the current data in - * OVN_Northbound, so that the address sets used in Logical_Flows in +static void +sync_address_set(struct northd_context *ctx, const char *name, + const char **addrs, size_t n_addrs, + struct shash *sb_address_sets) +{ + const struct sbrec_address_set *sb_address_set; + sb_address_set = shash_find_and_delete(sb_address_sets, + name); + if (!sb_address_set) { + sb_address_set = sbrec_address_set_insert(ctx->ovnsb_txn); + sbrec_address_set_set_name(sb_address_set, name); + } + + sbrec_address_set_set_addresses(sb_address_set, + addrs, n_addrs); +} + +/* OVN_Southbound Address_Set table contains same records as in north + * bound, plus the records generated from Port_Group table in north bound. + * + * There are 2 records generated from each port group, one for IPv4, and + * one for IPv6, named in the format: _ip4 and + * _ip6 respectively. MAC addresses are ignored. + * + * We always update OVN_Southbound to match the Address_Set and Port_Group + * in OVN_Northbound, so that the address sets used in Logical_Flows in * OVN_Southbound is checked against the proper set.*/ static void sync_address_sets(struct northd_context *ctx) @@ -6112,19 +6135,55 @@ sync_address_sets(struct northd_context *ctx) 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); + /* sync port group generated address sets first */ + const struct nbrec_port_group *nb_port_group; + NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) { + const char **ipv4_addrs = xcalloc(1, sizeof *ipv4_addrs); + size_t n_ipv4_addrs = 0; + const char **ipv6_addrs = xcalloc(1, sizeof *ipv6_addrs); + size_t n_ipv6_addrs = 0; + for (size_t i = 0; i < nb_port_group->n_ports; i++) { + for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) { + struct lport_addresses laddrs; + extract_lsp_addresses(nb_port_group->ports[i]->addresses[j], + &laddrs); + ipv4_addrs = xrealloc(ipv4_addrs, + (n_ipv4_addrs + laddrs.n_ipv4_addrs) + * sizeof *ipv4_addrs); + for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) { + ipv4_addrs[n_ipv4_addrs++] = + xstrdup(laddrs.ipv4_addrs[k].addr_s); + } + ipv6_addrs = xrealloc(ipv6_addrs, + (n_ipv6_addrs + laddrs.n_ipv6_addrs) + * sizeof *ipv6_addrs); + for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) { + ipv6_addrs[n_ipv6_addrs++] = + xstrdup(laddrs.ipv6_addrs[k].addr_s); + } + destroy_lport_addresses(&laddrs); + } } + char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name); + char *ipv6_addrs_name = xasprintf("%s_ip6", nb_port_group->name); + sync_address_set(ctx, ipv4_addrs_name, ipv4_addrs, n_ipv4_addrs, + &sb_address_sets); + sync_address_set(ctx, ipv6_addrs_name, ipv6_addrs, n_ipv6_addrs, + &sb_address_sets); + free(ipv4_addrs_name); + free(ipv6_addrs_name); + free(ipv4_addrs); + free(ipv6_addrs); + } - 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); + /* sync user defined address sets, which may overwrite port group + * generated address sets if same name is used */ + const struct nbrec_address_set *nb_address_set; + NBREC_ADDRESS_SET_FOR_EACH (nb_address_set, ctx->ovnnb_idl) { + sync_address_set(ctx, nb_address_set->name, + /* "char **" is not compatible with "const char **" */ + (const char **)nb_address_set->addresses, + nb_address_set->n_addresses, &sb_address_sets); } struct shash_node *node, *next; diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index 83727c5..11b3e2b 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -936,6 +936,24 @@ db="OVN_Southbound"/> database.

+

+ For each port group, there are two address sets generated to the + table of the + database, containing the IP addresses + of the group of ports, one for IPv4, and the other for IPv6, with + being + the + of the followed by + a suffix _ip4 for IPv4 and _ip6 for IPv6. + The generated address sets can be used in the same way as regular + address sets in the column + of the table. For syntax information, see the details + of the expression language used for the column in the table of the database. +

+ A name for the port group. Names are ASCII and must match [a-zA-Z_.][a-zA-Z_.0-9]*. diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 2eac943..702ebef 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -368,9 +368,17 @@

- See the documentation for the table in the database - for details. + and address sets generated from the table in the database. +

+ +

+ See the documentation for the table and table in the + database for details.

@@ -790,6 +798,17 @@ @port_group1.

+

+ Additionally, you may refer to the set of addresses belonging to a + group of logical switch ports stored in the + table by its followed by + a suffix '_ip4'/'_ip6'. The IPv4 address set of a + with a name of port_group1 + can be referred to as $port_group1_ip4, and the IPv6 + address set of the same can be referred to + as $port_group1_ip6 +

+

Miscellaneous

diff --git a/tests/ovn.at b/tests/ovn.at index c8ab7b7..16993de 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9363,3 +9363,229 @@ ra_test 000005dc 40 80 40 aef00000000000000000000000000000 30 fd0f00000000000000 OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP + +AT_SETUP([ovn -- Port Groups]) +AT_KEYWORDS([ovnpg]) +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 uses them in ACL. + +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 + +ovn-nbctl create Port_Group name=pg1 ports="$pg1_ports" +ovn-nbctl create Port_Group name=pg2 ports="$pg2_ports" + +# create ACLs on all lswitches to drop traffic from pg2 to pg1 +ovn-nbctl acl-add ls1 to-lport 1001 'outport == @pg1 && ip4.src == $pg2_ip4' drop +ovn-nbctl acl-add ls2 to-lport 1001 'outport == @pg1 && ip4.src == $pg2_ip4' drop +ovn-nbctl acl-add ls3 to-lport 1001 'outport == @pg1 && ip4.src == $pg2_ip4' drop + +# 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 ACL (pg2 to pg1) 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 ACL 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