Two minor questions for clarification below, Jarno
Acked-by: Jarno Rajahalme <[email protected]> On Jul 25, 2014, at 10:25 PM, Ben Pfaff <[email protected]> wrote: > An upcoming commit will increase the number of fields beyond 64. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > lib/classifier.c | 9 +++------ > lib/meta-flow.h | 12 +++++++++--- > lib/ofp-print.c | 35 +++++++++++++++-------------------- > lib/ofp-util.c | 22 +++++++++------------- > lib/ofp-util.h | 9 +++++---- > 5 files changed, 41 insertions(+), 46 deletions(-) > > diff --git a/lib/classifier.c b/lib/classifier.c > index af28070..334d0da 100644 > --- a/lib/classifier.c > +++ b/lib/classifier.c > @@ -501,9 +501,6 @@ classifier_destroy(struct classifier *cls) > } > } > > -/* We use uint64_t as a set for the fields below. */ > -BUILD_ASSERT_DECL(MFF_N_IDS <= 64); > - > /* Set the fields for which prefix lookup should be performed. */ > bool > classifier_set_prefix_fields(struct classifier *cls, > @@ -511,8 +508,8 @@ classifier_set_prefix_fields(struct classifier *cls, > unsigned int n_fields) > OVS_EXCLUDED(cls->mutex) > { > - uint64_t fields = 0; > const struct mf_field * new_fields[CLS_MAX_TRIES]; > + struct mf_bitmap fields = MF_BITMAP_INITIALIZER; > int i, n_tries = 0; > bool changed = false; > > @@ -527,12 +524,12 @@ classifier_set_prefix_fields(struct classifier *cls, > continue; > } > > - if (fields & (UINT64_C(1) << trie_fields[i])) { > + if (bitmap_is_set(fields.bm, trie_fields[i])) { > /* Duplicate field, there is no need to build more than > * one index for any one field. */ > continue; > } > - fields |= UINT64_C(1) << trie_fields[i]; > + bitmap_set1(fields.bm, trie_fields[i]); > > new_fields[n_tries] = NULL; > if (n_tries >= cls->n_tries || field != cls->tries[n_tries].field) { > diff --git a/lib/meta-flow.h b/lib/meta-flow.h > index 3208137..f65ed20 100644 > --- a/lib/meta-flow.h > +++ b/lib/meta-flow.h > @@ -20,9 +20,9 @@ > #include <sys/types.h> > #include <netinet/in.h> > #include <netinet/ip6.h> > +#include "bitmap.h" > #include "flow.h" > #include "ofp-errors.h" > -#include "ofp-util.h" > #include "packets.h" > #include "util.h" > > @@ -134,6 +134,12 @@ enum OVS_PACKED_ENUM mf_field_id { > MFF_N_IDS > }; > > +/* A set of mf_field_ids. */ > +struct mf_bitmap { > + unsigned long bm[BITMAP_N_LONGS(MFF_N_IDS)]; > +}; > +#define MF_BITMAP_INITIALIZER { { [0] = 0 } } Is this different from just { { 0 } } ? > + > /* Use this macro as CASE_MFF_REGS: in a switch statement to choose all of the > * MFF_REGx cases. */ > #if FLOW_N_REGS == 8 > @@ -270,9 +276,9 @@ struct mf_field { > * Also, some field types are tranparently mapped to each other via the > * struct flow (like vlan and dscp/tos fields), so each variant supports > * all protocols. */ > - enum ofputil_protocol usable_protocols; /* If fully/cidr masked. */ > + uint32_t usable_protocols; /* If fully/cidr masked. */ > /* If partially/non-cidr masked. */ > - enum ofputil_protocol usable_protocols_bitwise; > + uint32_t usable_protocols_bitwise; These seem unrelated changes? > > int flow_be32ofs; /* Field's be32 offset in "struct flow", if prefix tree > * lookup is supported for the field, or -1. */ > diff --git a/lib/ofp-print.c b/lib/ofp-print.c > index a2c2434..25e0478 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -2555,15 +2555,11 @@ print_table_action_features(struct ds *s, > ds_put_char(s, '\n'); > > ds_put_cstr(s, " supported on Set-Field: "); > - if (taf->set_fields) { > + if (!bitmap_is_all_zeros(taf->set_fields.bm, MFF_N_IDS)) { > int i; > > - for (i = 0; i < MFF_N_IDS; i++) { > - uint64_t bit = UINT64_C(1) << i; > - > - if (taf->set_fields & bit) { > - ds_put_format(s, "%s,", mf_from_id(i)->name); > - } > + BITMAP_FOR_EACH_1 (i, MFF_N_IDS, taf->set_fields.bm) { > + ds_put_format(s, "%s,", mf_from_id(i)->name); > } > ds_chomp(s, ','); > } else { > @@ -2576,7 +2572,8 @@ static bool > table_action_features_equal(const struct ofputil_table_action_features *a, > const struct ofputil_table_action_features *b) > { > - return a->actions == b->actions && a->set_fields == b->set_fields; > + return (a->actions == b->actions > + && bitmap_equal(a->set_fields.bm, b->set_fields.bm, MFF_N_IDS)); > } > > static void > @@ -2679,18 +2676,16 @@ ofp_print_table_features(struct ds *s, const struct > ofp_header *oh) > } > > ds_put_cstr(s, " matching:\n"); > - for (i = 0; i < MFF_N_IDS; i++) { > - uint64_t bit = UINT64_C(1) << i; > - > - if (tf.match & bit) { > - const struct mf_field *f = mf_from_id(i); > - > - ds_put_format(s, " %s: %s\n", > - f->name, > - (tf.mask ? "arbitrary mask" > - : tf.wildcard ? "exact match or wildcard" > - : "must exact match")); > - } > + BITMAP_FOR_EACH_1 (i, MFF_N_IDS, tf.match.bm) { > + const struct mf_field *f = mf_from_id(i); > + bool mask = bitmap_is_set(tf.mask.bm, i); > + bool wildcard = bitmap_is_set(tf.wildcard.bm, i); > + > + ds_put_format(s, " %s: %s\n", > + f->name, > + (mask ? "arbitrary mask" > + : wildcard ? "exact match or wildcard" > + : "must exact match")); > } > } > } > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index 912def9..278349cb 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -4596,11 +4596,11 @@ parse_oxm(struct ofpbuf *b, bool loose, > > static enum ofperr > parse_oxms(struct ofpbuf *payload, bool loose, > - uint64_t *exactp, uint64_t *maskedp) > + struct mf_bitmap *exactp, struct mf_bitmap *maskedp) > { > - uint64_t exact, masked; > + struct mf_bitmap exact = MF_BITMAP_INITIALIZER; > + struct mf_bitmap masked = MF_BITMAP_INITIALIZER; > > - exact = masked = 0; > while (ofpbuf_size(payload) > 0) { > const struct mf_field *field; > enum ofperr error; > @@ -4608,23 +4608,19 @@ parse_oxms(struct ofpbuf *payload, bool loose, > > error = parse_oxm(payload, loose, &field, &hasmask); > if (!error) { > - if (hasmask) { > - masked |= UINT64_C(1) << field->id; > - } else { > - exact |= UINT64_C(1) << field->id; > - } > + bitmap_set1(hasmask ? masked.bm : exact.bm, field->id); > } else if (error != OFPERR_OFPBMC_BAD_FIELD || !loose) { > return error; > } > } > if (exactp) { > *exactp = exact; > - } else if (exact) { > + } else if (!bitmap_is_all_zeros(exact.bm, MFF_N_IDS)) { > return OFPERR_OFPBMC_BAD_MASK; > } > if (maskedp) { > *maskedp = masked; > - } else if (masked) { > + } else if (!bitmap_is_all_zeros(masked.bm, MFF_N_IDS)) { > return OFPERR_OFPBMC_BAD_MASK; > } > return 0; > @@ -4769,10 +4765,10 @@ ofputil_decode_table_features(struct ofpbuf *msg, > * > * - Turn on 'wildcard' bits that are set in 'mask', because a field > * that is arbitrarily maskable can be wildcarded entirely. */ > - tf->mask &= tf->match; > - tf->wildcard &= tf->match; > + bitmap_and(tf->mask.bm, tf->match.bm, MFF_N_IDS); > + bitmap_and(tf->wildcard.bm, tf->match.bm, MFF_N_IDS); > > - tf->wildcard |= tf->mask; > + bitmap_or(tf->wildcard.bm, tf->mask.bm, MFF_N_IDS); > > return 0; > } > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index 878bcf8..23396bb 100644 > --- a/lib/ofp-util.h > +++ b/lib/ofp-util.h > @@ -25,6 +25,7 @@ > #include "flow.h" > #include "list.h" > #include "match.h" > +#include "meta-flow.h" > #include "netdev.h" > #include "openflow/nicira-ext.h" > #include "openvswitch/types.h" > @@ -650,7 +651,7 @@ struct ofputil_table_features { > * instruction. */ > struct ofputil_table_action_features { > uint32_t actions; /* Bitmap of supported OFPAT*. */ > - uint64_t set_fields; /* Bitmap of MFF_* "set-field" supports. */ > + struct mf_bitmap set_fields; /* Fields for "set-field". */ > } write, apply; > } nonmiss, miss; > > @@ -673,9 +674,9 @@ struct ofputil_table_features { > * > * Other combinations do not make sense. > */ > - uint64_t match; /* Fields that may be matched. */ > - uint64_t mask; /* Subset of 'match' that may have masks. */ > - uint64_t wildcard; /* Subset of 'match' that may be wildcarded. > */ > + struct mf_bitmap match; /* Fields that may be matched. */ > + struct mf_bitmap mask; /* Subset of 'match' that may have masks. */ > + struct mf_bitmap wildcard; /* Subset of 'match' that may be wildcarded. > */ > }; > > int ofputil_decode_table_features(struct ofpbuf *, > -- > 1.9.1 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
