[ovs-dev] [PATCH] ofp-parse: Allow match field names in actions and brackets in matches.

Jarno Rajahalme jarno at ovn.org
Thu Jan 5 00:34:35 UTC 2017


Thanks for the review! Pushed to master with the suggested improvements,

  Jarno

> On Dec 21, 2016, at 3:42 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Fri, Dec 09, 2016 at 03:25:57PM -0800, Jarno Rajahalme wrote:
>> Allow using match field names in addition to the canonical register
>> names in actions (including 'load', 'move', 'push', 'pop', 'output',
>> 'multipath', 'bundle_load', and 'learn').  Allow also leaving out the
>> trailing '[]' to indicate full field.  These changes allow simpler
>> syntax similar to 'set_field' to be used also elsewhere.
>> 
>> Correspondingly, allow the '[start..end]' syntax to be used in matches
>> in addition to the more explicit 'value/mask' notation.  For example,
>> to match on the value 2 of the bits 14..15 of NXM_NX_REG0, the match
>> could include:
>> 
>> ... reg0[14..15]=2 ...
>> 
>> instead of
>> 
>> ... reg0=0x8000/0xc000 ...
>> 
>> Note that only contiguous masks can be specified with the bracket
>> notation.
>> 
>> Signed-of-by: Jarno Rajahalme <jarno at ovn.org>
>> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
> 
> This is much nicer than before.  Thank you.
> 
> The signed-off-by is doubled above.
> 
> Acked-by: Ben Pfaff <blp at ovn.org>
> 
> 
> Here are some suggestions.
> 
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index b091c1b..9f123ef 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -2069,6 +2069,7 @@ struct field_array {
> 
> /* Finding mf_fields. */
> const struct mf_field *mf_from_name(const char *name);
> +const struct mf_field *mf_from_name_len(const char *name, size_t len);
> 
> static inline const struct mf_field *
> mf_from_id(enum mf_field_id id)
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index e25adec..6fc8ff9 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -80,6 +80,17 @@ mf_from_name(const char *name)
>     return shash_find_data(&mf_by_name, name);
> }
> 
> +/* Returns the field with the given 'name' (which is 'len' bytes long), or a
> + * null pointer if no field has that name. */
> +const struct mf_field *
> +mf_from_name_len(const char *name, size_t len)
> +{
> +    nxm_init();
> +
> +    struct shash_node *node = shash_find_len(&mf_by_name, name, len);
> +    return node ? node->data : NULL;
> +}
> +
> static void
> nxm_do_init(void)
> {
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 9c769c0..2fd31b9 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -1825,17 +1825,7 @@ mf_parse_subfield__(struct mf_subfield *sf, const char **sp)
>     name_len = strcspn(s, "[-");
> 
>     f = mf_parse_subfield_name(name, name_len, &wild);
> -    if (f) {
> -        field = mf_from_id(f->id);
> -    } else if (name_len < 256) {
> -        char name_buf[256];
> -        if (s[name_len] != '\0') {
> -            memcpy(name_buf, name, name_len);
> -            name_buf[name_len] = '\0';
> -            name = name_buf;
> -        }
> -        field = mf_from_name(name);
> -    }
> +    field = f ? mf_from_id(f->id) : mf_from_name_len(name, name_len);
>     if (!field) {
>         return xasprintf("%s: unknown field `%.*s'", *sp, name_len, s);
>     }
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index c4685b7..48d000c 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -1545,10 +1545,8 @@ is the packet's input port, the packet is not output.
> .
> .IP \fBoutput:\fIsrc\fB[\fIstart\fB..\fIend\fB]
> Outputs the packet to the OpenFlow port number read from \fIsrc\fR,
> -which must be an NXM field as described above.  Also the match field
> -name can be used, for example, instead of 'NXM_NX_REG0' the
> -name 'reg0' can be used.  For example,
> -\fBoutput:NXM_NX_REG0[16..31]\fR outputs to the OpenFlow port number
> +which may be an NXM field name, as described above, or a match field name.
> +\fBoutput:reg0[16..31]\fR outputs to the OpenFlow port number
> written in the upper half of register 0.  If the port number is the
> packet's input port, the packet is not output.
> .IP
> @@ -2009,10 +2007,9 @@ bytes with value 0 to make the total number 6 more than a multiple of
> .
> .IP "\fBmove:\fIsrc\fB[\fIstart\fB..\fIend\fB]\->\fIdst\fB[\fIstart\fB..\fIend\fB]\fR"
> Copies the named bits from field \fIsrc\fR to field \fIdst\fR.
> -\fIsrc\fR and \fIdst\fR must be NXM field names as defined in
> -\fBnicira\-ext.h\fR, e.g. \fBNXM_OF_UDP_SRC\fR or \fBNXM_NX_REG0\fR.
> -Also the corresponding match field names can be used, for example,
> -instead of 'NXM_NX_REG0' the name 'reg0' can be used.  Each
> +\fIsrc\fR and \fIdst\fR may be NXM field names as defined in
> +\fBnicira\-ext.h\fR, e.g. \fBNXM_OF_UDP_SRC\fR or \fBNXM_NX_REG0\fR,
> +or a match field name, e.g. \fBreg0\fR.  Each
> \fIstart\fR and \fIend\fR pair, which are inclusive, must specify the
> same number of bits and must fit within its respective field.
> Shorthands for \fB[\fIstart\fB..\fIend\fB]\fR exist: use
> @@ -3288,10 +3285,10 @@ Implements a level 2 MAC learning switch using the learn.
> \fBovs\-ofctl add\-flow br0 'table=0,priority=0 actions=load:3->NXM_NX_REG0[0..15],learn(table=0,priority=1,idle_timeout=10,NXM_OF_ETH_SRC[],NXM_OF_VLAN_TCI[0..11],output:NXM_NX_REG0[0..15]),output:2\fR
> In this use of a learn action, the first packet from each source MAC
> will be sent to port 2. Subsequent packets will be output to port 3,
> -with an idle timeout of 10 seconds.  Also the match field names can be
> -used, for example, instead of 'NXM_NX_REG0' the name 'reg0' can be
> -used.  Empty brackets can also be left off.
> -.
> +with an idle timeout of 10 seconds.  NXM field names and match field
> +names are both accepted, e.g. \fBNXM_NX_REG0\fR or \fBreg0\fR for the
> +first register, and empty brackets may be omitted.
> +.IP
> Additional examples may be found documented as part of related sections.
> .
> .SH "SEE ALSO"



More information about the dev mailing list