From patchwork Mon Aug 8 16:14:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 656855 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3s7MwD5Nqyz9syB for ; Tue, 9 Aug 2016 02:15:56 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 46A18109C8; Mon, 8 Aug 2016 09:14:48 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id F2D2D109AA for ; Mon, 8 Aug 2016 09:14:46 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id 88C84162686 for ; Mon, 8 Aug 2016 10:14:46 -0600 (MDT) X-ASG-Debug-ID: 1470672885-0b323721d7492400001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar6.cudamail.com with ESMTP id mXW6Q5cH4Y3ZGCYj (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 08 Aug 2016 10:14:46 -0600 (MDT) X-Barracuda-Envelope-From: blp@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO relay5-d.mail.gandi.net) (217.70.183.197) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 8 Aug 2016 16:14:45 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at ovn.org designates 217.70.183.197 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.197 X-Barracuda-RBL-IP: 217.70.183.197 Received: from mfilter16-d.gandi.net (mfilter16-d.gandi.net [217.70.178.144]) by relay5-d.mail.gandi.net (Postfix) with ESMTP id 37B3A41C08A; Mon, 8 Aug 2016 18:14:44 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter16-d.gandi.net Received: from relay5-d.mail.gandi.net ([IPv6:::ffff:217.70.183.197]) by mfilter16-d.gandi.net (mfilter16-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id VGcKideKpG1W; Mon, 8 Aug 2016 18:14:42 +0200 (CEST) X-Originating-IP: 173.228.112.241 Received: from sigabrt.gateway.sonic.net (173-228-112-241.dsl.dynamic.fusionbroadband.com [173.228.112.241]) (Authenticated sender: blp@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id D26E241C08E; Mon, 8 Aug 2016 18:14:41 +0200 (CEST) X-CudaMail-Envelope-Sender: blp@ovn.org From: Ben Pfaff To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E2-807035273 X-CudaMail-DTE: 080816 X-CudaMail-Originating-IP: 217.70.183.197 Date: Mon, 8 Aug 2016 09:14:19 -0700 X-ASG-Orig-Subj: [##CM-E2-807035273##][PATCH v2 08/21] expr: Give a subfield a direct pointer to its parent in struct expr_symbol. Message-Id: <1470672872-19450-9-git-send-email-blp@ovn.org> X-Mailer: git-send-email 2.1.3 In-Reply-To: <1470672872-19450-1-git-send-email-blp@ovn.org> References: <1470672872-19450-1-git-send-email-blp@ovn.org> X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1470672885 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH v2 08/21] expr: Give a subfield a direct pointer to its parent in struct expr_symbol. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Until now, symbols that represent subfields and predicates were both implemented as the same string member, named 'expansion', inside struct expr. This makes it a little inconvenient to find the parent of a subfield for two reasons. First, one must actually parse the string, e.g. to convert "vlan.tci[13..15]" into a pointer to a struct. Second, and more importantly, to parse the string it's necessary to have access to the symbol table, which isn't always convenient to pass around. This commit avoids the problem by breaking apart subfields and predicates and giving the former a direct pointer to the parent symbol. We could do the same thing for predicates by storing a pointer to a pre-built struct expr, but so far it's not necessary. Signed-off-by: Ben Pfaff Acked-by: Ryan Moats --- include/ovn/expr.h | 34 ++++++++++++++------------ ovn/lib/expr.c | 70 +++++++++++++++++++++--------------------------------- 2 files changed, 46 insertions(+), 58 deletions(-) diff --git a/include/ovn/expr.h b/include/ovn/expr.h index 569c524..011685d 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -106,20 +106,23 @@ const char *expr_level_to_string(enum expr_level); * Fields: * * One might, for example, define a field named "vlan.tci" to refer to - * MFF_VLAN_TCI. For integer fields, 'field' specifies the referent; for - * string fields, 'field' is NULL. + * MFF_VLAN_TCI. 'field' specifies the field. * - * 'expansion' is NULL. + * 'parent' and 'predicate' are NULL, and 'parent_ofs' is 0. * * Integer fields can be nominal or ordinal (see below). String fields are * always nominal. * * Subfields: * - * 'expansion' is a string that specifies a subfield of some larger field, - * e.g. "vlan.tci[0..11]" for a field that represents a VLAN VID. + * 'parent' specifies the field (which may itself be a subfield, + * recursively) in which the subfield is embedded, and 'parent_ofs' a + * bitwise offset from the least-significant bit of the parent. The + * subfield can contain a subset of the bits of the parent or all of them + * (in the latter case the subfield is really just a synonym for the + * parent). * - * 'field' is NULL. + * 'field' and 'predicate' are NULL. * * Only ordinal fields (see below) may have subfields, and subfields are * always ordinal. @@ -127,16 +130,15 @@ const char *expr_level_to_string(enum expr_level); * Predicates: * * A predicate is an arbitrary Boolean expression that can be used in an - * expression much like a 1-bit field. 'expansion' specifies the Boolean + * expression much like a 1-bit field. 'predicate' specifies the Boolean * expression, e.g. "ip4" might expand to "eth.type == 0x800". The - * expansion of a predicate might refer to other predicates, e.g. "icmp4" - * might expand to "ip4 && ip4.proto == 1". + * epxression might refer to other predicates, e.g. "icmp4" might expand to + * "ip4 && ip4.proto == 1". * - * 'field' is NULL. + * 'field' and 'parent' are NULL, and 'parent_ofs' is 0. * - * A predicate whose expansion refers to any nominal field or predicate - * (see below) is nominal; other predicates have Boolean level of - * measurement. + * A predicate that refers to any nominal field or predicate (see below) is + * nominal; other predicates have Boolean level of measurement. * * * Level of Measurement @@ -239,8 +241,10 @@ struct expr_symbol { char *name; int width; - const struct mf_field *field; - char *expansion; + const struct mf_field *field; /* Fields only, otherwise NULL. */ + const struct expr_symbol *parent; /* Subfields only, otherwise NULL. */ + int parent_ofs; /* Subfields only, otherwise 0. */ + char *predicate; /* Predicates only, otherwise NULL. */ enum expr_level level; diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c index 771765a..7ad990b 100644 --- a/ovn/lib/expr.c +++ b/ovn/lib/expr.c @@ -592,7 +592,7 @@ make_cmp(struct expr_context *ctx, } if (f->symbol->level == EXPR_L_NOMINAL) { - if (f->symbol->expansion) { + if (f->symbol->predicate) { ovs_assert(f->symbol->width > 0); for (size_t i = 0; i < cs->n_values; i++) { const union mf_subvalue *value = &cs->values[i].value; @@ -1209,7 +1209,8 @@ expr_symtab_add_subfield(struct shash *symtab, const char *name, symbol = add_symbol(symtab, name, f.n_bits, prereqs, level, false, f.symbol->rw); - symbol->expansion = xstrdup(subfield); + symbol->parent = f.symbol; + symbol->parent_ofs = f.ofs; return symbol; } @@ -1285,7 +1286,7 @@ expr_symtab_add_predicate(struct shash *symtab, const char *name, } symbol = add_symbol(symtab, name, 1, NULL, level, false, false); - symbol->expansion = xstrdup(expansion); + symbol->predicate = xstrdup(expansion); return symbol; } @@ -1301,7 +1302,7 @@ expr_symtab_destroy(struct shash *symtab) shash_delete(symtab, node); free(symbol->name); free(symbol->prereqs); - free(symbol->expansion); + free(symbol->predicate); free(symbol); } } @@ -1444,36 +1445,27 @@ expr_annotate_cmp(struct expr *expr, const struct shash *symtab, } } - if (symbol->expansion) { - if (symbol->level == EXPR_L_ORDINAL) { - struct expr_field field; + if (symbol->parent) { + expr->cmp.symbol = symbol->parent; + mf_subvalue_shift(&expr->cmp.value, symbol->parent_ofs); + mf_subvalue_shift(&expr->cmp.mask, symbol->parent_ofs); + } else if (symbol->predicate) { + struct expr *predicate; - if (!parse_field_from_string(symbol->expansion, symtab, - &field, errorp)) { - goto error; - } - - expr->cmp.symbol = field.symbol; - mf_subvalue_shift(&expr->cmp.value, field.ofs); - mf_subvalue_shift(&expr->cmp.mask, field.ofs); - } else { - struct expr *expansion; - - expansion = parse_and_annotate(symbol->expansion, symtab, - nesting, errorp); - if (!expansion) { - goto error; - } - - bool positive = (expr->cmp.value.integer & htonll(1)) != 0; - positive ^= expr->cmp.relop == EXPR_R_NE; - if (!positive) { - expr_not(expansion); - } + predicate = parse_and_annotate(symbol->predicate, symtab, + nesting, errorp); + if (!predicate) { + goto error; + } - expr_destroy(expr); - expr = expansion; + bool positive = (expr->cmp.value.integer & htonll(1)) != 0; + positive ^= expr->cmp.relop == EXPR_R_NE; + if (!positive) { + expr_not(predicate); } + + expr_destroy(expr); + expr = predicate; } *errorp = NULL; @@ -2707,7 +2699,7 @@ expand_symbol(struct expr_context *ctx, bool rw, { const struct expr_symbol *orig_symbol = f->symbol; - if (f->symbol->expansion && f->symbol->level != EXPR_L_ORDINAL) { + if (f->symbol->predicate) { expr_error(ctx, "Predicate symbol %s used where lvalue required.", f->symbol->name); return false; @@ -2730,21 +2722,13 @@ expand_symbol(struct expr_context *ctx, bool rw, } /* If there's no expansion, we're done. */ - if (!f->symbol->expansion) { + if (!f->symbol->parent) { break; } /* Expand. */ - struct expr_field expansion; - char *error; - if (!parse_field_from_string(f->symbol->expansion, ctx->symtab, - &expansion, &error)) { - expr_error(ctx, "%s", error); - free(error); - return false; - } - f->symbol = expansion.symbol; - f->ofs += expansion.ofs; + f->ofs += f->symbol->parent_ofs; + f->symbol = f->symbol->parent; } if (rw && !f->symbol->field->writable) {