[ovs-dev] [bug10576 5/5] learn: Make it possible to parse "load" actions wider than 64 bits.

Ethan Jackson ethan at nicira.com
Sat Apr 14 01:06:47 UTC 2012


Looks good,

I would think that bitwise_is_all_zeros() belongs in it's own patch
just line bitwise_one(), but I don't think it matters much.

Ethan

On Wed, Apr 11, 2012 at 17:15, Ben Pfaff <blp at nicira.com> wrote:
> The implementation of the "learn" action now properly implements
> specifications such as 0x20010db885a308d313198a2e03707348->NXM_NX_IPV6_DST
> but the parser used in ovs-ofctl and elsewhere could not generate such
> specifications.  This commit adds that support.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/learn.c    |   91 +++++++++++++++++++++++++++++++++++++-------------------
>  lib/util.c     |   55 ++++++++++++++++++++++++++++++++++
>  lib/util.h     |    2 +
>  tests/learn.at |    9 ++++--
>  4 files changed, 123 insertions(+), 34 deletions(-)
>
> diff --git a/lib/learn.c b/lib/learn.c
> index d2db792..483442b 100644
> --- a/lib/learn.c
> +++ b/lib/learn.c
> @@ -317,13 +317,63 @@ struct learn_spec {
>
>     int src_type;
>     struct mf_subfield src;
> -    uint8_t src_imm[sizeof(union mf_value)];
> +    union mf_subvalue src_imm;
>
>     int dst_type;
>     struct mf_subfield dst;
>  };
>
>  static void
> +learn_parse_load_immediate(const char *s, struct learn_spec *spec)
> +{
> +    const char *full_s = s;
> +    const char *arrow = strstr(s, "->");
> +    struct mf_subfield dst;
> +    union mf_subvalue imm;
> +
> +    memset(&imm, 0, sizeof imm);
> +    if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X') && arrow) {
> +        const char *in = arrow - 1;
> +        uint8_t *out = imm.u8 + sizeof imm.u8 - 1;
> +        int n = arrow - (s + 2);
> +        int i;
> +
> +        for (i = 0; i < n; i++) {
> +            int hexit = hexit_value(in[-i]);
> +            if (hexit < 0) {
> +                ovs_fatal(0, "%s: bad hex digit in value", full_s);
> +            }
> +            out[-(i / 2)] |= i % 2 ? hexit << 4 : hexit;
> +        }
> +        s = arrow;
> +    } else {
> +        imm.be64[1] = htonll(strtoull(s, (char **) &s, 0));
> +    }
> +
> +    if (strncmp(s, "->", 2)) {
> +        ovs_fatal(0, "%s: missing `->' following value", full_s);
> +    }
> +    s += 2;
> +
> +    s = mf_parse_subfield(&dst, s);
> +    if (*s != '\0') {
> +        ovs_fatal(0, "%s: trailing garbage following destination", full_s);
> +    }
> +
> +    if (!bitwise_is_all_zeros(&imm, sizeof imm, dst.n_bits,
> +                              (8 * sizeof imm) - dst.n_bits)) {
> +        ovs_fatal(0, "%s: value does not fit into %u bits",
> +                  full_s, dst.n_bits);
> +    }
> +
> +    spec->n_bits = dst.n_bits;
> +    spec->src_type = NX_LEARN_SRC_IMMEDIATE;
> +    spec->src_imm = imm;
> +    spec->dst_type = NX_LEARN_DST_LOAD;
> +    spec->dst = dst;
> +}
> +
> +static void
>  learn_parse_spec(const char *orig, char *name, char *value,
>                  struct learn_spec *spec)
>  {
> @@ -340,7 +390,9 @@ learn_parse_spec(const char *orig, char *name, char *value,
>
>         spec->n_bits = dst->n_bits;
>         spec->src_type = NX_LEARN_SRC_IMMEDIATE;
> -        memcpy(spec->src_imm, &imm, dst->n_bytes);
> +        memset(&spec->src_imm, 0, sizeof spec->src_imm);
> +        memcpy(&spec->src_imm.u8[sizeof spec->src_imm - dst->n_bytes],
> +               &imm, dst->n_bytes);
>         spec->dst_type = NX_LEARN_DST_MATCH;
>         spec->dst.field = dst;
>         spec->dst.ofs = 0;
> @@ -372,23 +424,7 @@ learn_parse_spec(const char *orig, char *name, char *value,
>         spec->dst_type = NX_LEARN_DST_MATCH;
>     } else if (!strcmp(name, "load")) {
>         if (value[strcspn(value, "[-")] == '-') {
> -            struct nx_action_reg_load load;
> -            int nbits, imm_bytes;
> -            uint64_t imm;
> -            int i;
> -
> -            nxm_parse_reg_load(&load, value);
> -            nbits = nxm_decode_n_bits(load.ofs_nbits);
> -            imm_bytes = DIV_ROUND_UP(nbits, 8);
> -            imm = ntohll(load.value);
> -
> -            spec->n_bits = nbits;
> -            spec->src_type = NX_LEARN_SRC_IMMEDIATE;
> -            for (i = 0; i < imm_bytes; i++) {
> -                spec->src_imm[i] = imm >> ((imm_bytes - i - 1) * 8);
> -            }
> -            spec->dst_type = NX_LEARN_DST_LOAD;
> -            nxm_decode(&spec->dst, load.dst, load.ofs_nbits);
> +            learn_parse_load_immediate(value, spec);
>         } else {
>             struct nx_action_reg_move move;
>
> @@ -492,23 +528,16 @@ learn_parse(struct ofpbuf *b, char *arg, const struct flow *flow)
>             /* Update 'rule' to allow for satisfying destination
>              * prerequisites. */
>             if (spec.src_type == NX_LEARN_SRC_IMMEDIATE
> -                && spec.dst_type == NX_LEARN_DST_MATCH
> -                && spec.dst.ofs == 0
> -                && spec.n_bits == spec.dst.field->n_bytes * 8) {
> -                union mf_value imm;
> -
> -                memcpy(&imm, spec.src_imm, spec.dst.field->n_bytes);
> -                mf_set_value(spec.dst.field, &imm, &rule);
> +                && spec.dst_type == NX_LEARN_DST_MATCH) {
> +                mf_write_subfield(&spec.dst, &spec.src_imm, &rule);
>             }
>
>             /* Output the flow_mod_spec. */
>             put_u16(b, spec.n_bits | spec.src_type | spec.dst_type);
>             if (spec.src_type == NX_LEARN_SRC_IMMEDIATE) {
> -                int n_bytes = DIV_ROUND_UP(spec.n_bits, 8);
> -                if (n_bytes % 2) {
> -                    ofpbuf_put_zeros(b, 1);
> -                }
> -                ofpbuf_put(b, spec.src_imm, n_bytes);
> +                int n_bytes = DIV_ROUND_UP(spec.n_bits, 16) * 2;
> +                int ofs = sizeof spec.src_imm - n_bytes;
> +                ofpbuf_put(b, &spec.src_imm.u8[ofs], n_bytes);
>             } else {
>                 put_u32(b, spec.src.field->nxm_header);
>                 put_u16(b, spec.src.ofs);
> diff --git a/lib/util.c b/lib/util.c
> index 14a97f2..72d0988 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -913,6 +913,61 @@ bitwise_one(void *dst_, unsigned int dst_len, unsigned dst_ofs,
>     }
>  }
>
> +/* Scans the 'n_bits' bits starting from bit 'dst_ofs' in 'dst' for 1-bits.
> + * Returns false if any 1-bits are found, otherwise true.  'dst' is 'dst_len'
> + * bytes long.
> + *
> + * If you consider all of 'dst' to be a single unsigned integer in network byte
> + * order, then bit N is the bit with value 2**N.  That is, bit 0 is the bit
> + * with value 1 in dst[dst_len - 1], bit 1 is the bit with value 2, bit 2 is
> + * the bit with value 4, ..., bit 8 is the bit with value 1 in dst[dst_len -
> + * 2], and so on.
> + *
> + * Required invariant:
> + *   dst_ofs + n_bits <= dst_len * 8
> + */
> +bool
> +bitwise_is_all_zeros(const void *p_, unsigned int len, unsigned int ofs,
> +                     unsigned int n_bits)
> +{
> +    const uint8_t *p = p_;
> +
> +    if (!n_bits) {
> +        return true;
> +    }
> +
> +    p += len - (ofs / 8 + 1);
> +    ofs %= 8;
> +
> +    if (ofs) {
> +        unsigned int chunk = MIN(n_bits, 8 - ofs);
> +
> +        if (*p & (((1 << chunk) - 1) << ofs)) {
> +            return false;
> +        }
> +
> +        n_bits -= chunk;
> +        if (!n_bits) {
> +            return true;
> +        }
> +
> +        p--;
> +    }
> +
> +    while (n_bits >= 8) {
> +        if (*p) {
> +            return false;
> +        }
> +        n_bits -= 8;
> +    }
> +
> +    if (n_bits && *p & ((1 << n_bits) - 1)) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  /* Copies the 'n_bits' low-order bits of 'value' into the 'n_bits' bits
>  * starting at bit 'dst_ofs' in 'dst', which is 'dst_len' bytes long.
>  *
> diff --git a/lib/util.h b/lib/util.h
> index 63f4a24..e5d1c3a 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -230,6 +230,8 @@ void bitwise_zero(void *dst_, unsigned int dst_len, unsigned dst_ofs,
>                   unsigned int n_bits);
>  void bitwise_one(void *dst_, unsigned int dst_len, unsigned dst_ofs,
>                  unsigned int n_bits);
> +bool bitwise_is_all_zeros(const void *, unsigned int len, unsigned int ofs,
> +                          unsigned int n_bits);
>  void bitwise_put(uint64_t value,
>                  void *dst, unsigned int dst_len, unsigned int dst_ofs,
>                  unsigned int n_bits);
> diff --git a/tests/learn.at b/tests/learn.at
> index 8431937..0d1406c 100644
> --- a/tests/learn.at
> +++ b/tests/learn.at
> @@ -139,7 +139,9 @@ OVS_VSWITCHD_START(
>    add-port br0 eth1 -- set Interface eth1 type=dummy -- \
>    add-port br0 eth2 -- set Interface eth2 type=dummy])
>  # Set up flow table for TCPv6 port learning.
> -AT_CHECK([[ovs-ofctl add-flow br0 'table=0 tcp6 actions=learn(table=1, hard_timeout=60, eth_type=0x86dd, nw_proto=6, NXM_NX_IPV6_SRC[]=NXM_NX_IPV6_DST[], NXM_NX_IPV6_DST[]=NXM_NX_IPV6_SRC[], NXM_OF_TCP_SRC[]=NXM_OF_TCP_DST[], NXM_OF_TCP_DST[]=NXM_OF_TCP_SRC[]), flood']])
> +# Also add a 128-bit-wide "load" action and a 128-bit literal match to check
> +# that they work.
> +AT_CHECK([[ovs-ofctl add-flow br0 'table=0 tcp6 actions=learn(table=1, hard_timeout=60, eth_type=0x86dd, nw_proto=6, NXM_NX_IPV6_SRC[]=NXM_NX_IPV6_DST[], ipv6_dst=2001:0db8:85a3:0000:0000:8a2e:0370:7334, NXM_OF_TCP_SRC[]=NXM_OF_TCP_DST[], NXM_OF_TCP_DST[]=NXM_OF_TCP_SRC[], load(0x20010db885a308d313198a2e03707348->NXM_NX_IPV6_DST[])), flood']])
>
>  # Trace a TCPv6 packet arriving on port 3.
>  AT_CHECK([ovs-appctl ofproto/trace br0 'in_port(3),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:06),eth_type(0x86dd),ipv6(src=fec0::2,dst=fec0::1,label=0,proto=6,tclass=0,hlimit=255,frag=no),tcp(src=40000,dst=80)' -generate], [0], [stdout])
> @@ -147,8 +149,9 @@ AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2,0,1
>  ])
>
>  # Check for the learning entry.
> -AT_CHECK([ovs-ofctl dump-flows br0 table=1 | ofctl_strip | sort], [0], [dnl
> - table=1, hard_timeout=60,tcp6,ipv6_src=fec0::1,ipv6_dst=fec0::2,tp_src=80,tp_dst=40000 actions=drop
> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> + table=1, hard_timeout=60,tcp6,ipv6_src=fec0::1,ipv6_dst=2001:db8:85a3::8a2e:370:7334,tp_src=80,tp_dst=40000 actions=load:0x13198a2e03707348->NXM_NX_IPV6_DST[[0..63]],load:0x20010db885a308d3->NXM_NX_IPV6_DST[[64..127]]
> + tcp6 actions=learn(table=1,hard_timeout=60,eth_type=0x86dd,nw_proto=6,NXM_NX_IPV6_SRC[[]]=NXM_NX_IPV6_DST[[]],ipv6_dst=2001:db8:85a3::8a2e:370:7334,NXM_OF_TCP_SRC[[]]=NXM_OF_TCP_DST[[]],NXM_OF_TCP_DST[[]]=NXM_OF_TCP_SRC[[]],load:0x20010db885a308d313198a2e03707348->NXM_NX_IPV6_DST[[]]),FLOOD
>  NXST_FLOW reply:
>  ])
>  OVS_VSWITCHD_STOP
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list