On 04/20/2015 06:29 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> v1->v2: Dropped remainder of series because it was committed.
>
> ovn/northd/automake.mk | 6 +-
> ovn/northd/ovn-northd.c | 345
> +++++++++++++++++++++++++++++++++++++++++++++++
> ovn/ovn-nb.xml | 14 +-
> 3 files changed, 358 insertions(+), 7 deletions(-)
>
Only minor comments. This looks great to me.
Acked-by: Russell Bryant <rbry...@redhat.com>
> + /* No such Pipeline row. Add one. */
> + const struct sbrec_pipeline *pipeline;
> + pipeline = sbrec_pipeline_insert(ctx->ovnsb_txn);
> + sbrec_pipeline_set_logical_datapath(pipeline,
> + logical_datapath->header_.uuid);
> + sbrec_pipeline_set_table_id(pipeline, table_id);
> + sbrec_pipeline_set_priority(pipeline, priority);
> + sbrec_pipeline_set_match(pipeline, match);
> + sbrec_pipeline_set_actions(pipeline, actions);
> +
> + VLOG_INFO("%s, %d, %d, %s, %s\n",
> + logical_datapath->name, table_id, priority, match, actions);
This may get a bit noisy. Maybe it should be just a debug message?
> + if (t) {
> + if (t->type == LEX_T_END) {
> + *t = lexer.token;
> + } else {
> + VLOG_INFO("%s: port_security has duplicate %s address",
> + ps, lex_format_to_string(lexer.token.format));
> + }
> + lexer_get(&lexer);
> + lexer_match(&lexer, LEX_T_COMMA);
> + continue;
> + }
> + }
> +
> + VLOG_INFO("%s: syntax error in port_security", ps);
> + lexer_destroy(&lexer);
> + return false;
> + } while (lexer.token.type != LEX_T_END);
> + lexer_destroy(&lexer);
> +
> + return true;
> +}
> +
> +/* Appends port security constraints on L2 address field 'eth_addr_field'
> + * (e.g. "eth.src" or "eth.dst") to 'match'. 'port_security', with
> + * 'n_port_security' elements, is the collection of port_security contraints
typo on "constraints"
> + * from an OVN_NB Logical_Port row.
> + *
> + * (This is naive; it's not yet possible to express complete L2 and L3 port
> + * security constraints as a single Boolean expression.) */
> +static void
> +build_port_security(const char *eth_addr_field,
> + char **port_security, size_t n_port_security,
> + struct ds *match)
> +{
> + size_t base_len = match->length;
> + ds_put_format(match, " && %s == {", eth_addr_field);
> +
> + size_t n = 0;
> + for (size_t i = 0; i < n_port_security; i++) {
> + struct ps_constraint c;
> + if (parse_port_security(port_security[i], &c)
> + && c.eth.type != LEX_T_END) {
> + lex_token_format(&c.eth, match);
> + ds_put_char(match, ' ');
> + n++;
> + }
> + }
> + ds_put_cstr(match, "}");
>
> + if (!n) {
> + match->length = base_len;
> + }
> +}
> +
> +/* Updates the Pipeline table in the OVN_SB database, constructing its
> contents
> + * based on the OVN_NB database. */
> +static void
> +build_pipeline(struct northd_context *ctx)
> +{
> + struct pipeline_ctx pc = {
> + .ovnsb_idl = ctx->ovnsb_idl,
> + .ovnsb_txn = ctx->ovnsb_txn,
> + .pipeline_hmap = HMAP_INITIALIZER(&pc.pipeline_hmap)
> + };
> +
> + /* Add all the Pipeline entries currently in the southbound database to
> + * 'pc.pipeline_hmap'. We remove entries that we generate from the hmap,
> + * thus by the time we're done only entries that need to be removed
> + * remain. */
> + const struct sbrec_pipeline *pipeline;
> + SBREC_PIPELINE_FOR_EACH (pipeline, ctx->ovnsb_idl) {
> + struct pipeline_hash_node *hash_node = xzalloc(sizeof *hash_node);
> + hash_node->pipeline = pipeline;
> + hmap_insert(&pc.pipeline_hmap, &hash_node->node,
> + pipeline_hash_rec(pipeline));
> + }
> +
> + /* Table 0: Admission control framework. */
> + const struct nbrec_logical_switch *lswitch;
> + NBREC_LOGICAL_SWITCH_FOR_EACH (lswitch, ctx->ovnnb_idl) {
> + /* Logical VLANs not supported. */
> + pipeline_add(&pc, lswitch, 0, 100, "vlan.present", "drop");
Child port traffic will have a VLAN tag. Do you want to make the
updates for that use case later? I could take a stab at it.
> index 6985f5e..a1b3a07 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -144,12 +144,14 @@
> </p>
>
> <p>
> - Exact syntax is TBD. One could simply use comma- or
> - space-separated L2 and L3 addresses in each set member, or
> - replace this by a subset of the general-purpose expression
> - language used for the <ref column="match" table="Pipeline"
> - db="OVN_Southbound"/> column in the OVN Southbound database's
> - <ref table="Pipeline" db="OVN_Southbound"/> table.
> + Each member of the set is a comma- or space-separated list. A single
> + set member may have an Ethernet address, an IPv4 address, and an IPv6
> + address, or any subset. Order is not significant.
> + </p>
> +
> + <p>
> + TBD: exact semantics. For now only Ethernet port security is
> + implemented.
> </p>
> </column>
>
>
You still have the tabs here instead of spaces for indentation.
--
Russell Bryant
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
