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 <[email protected]> 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 <[email protected]> > --- > 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 > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
