| Message ID | 1476455835-77641-3-git-send-email-bhanuprakash.bodireddy@intel.com |
|---|---|
| State | Superseded |
| Delegated to: | Daniele Di Proietto |
| Headers | show |
2016-10-14 7:37 GMT-07:00 Bhanuprakash Bodireddy < bhanuprakash.bodireddy@intel.com>: > This patch adds comments to mf_get_next_in_map() to make it more > comprehensible. > > Signed-off-by: Jarno Rajahalme <jarno@ovn.org> > Acked-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> > Acked-by: Antonio Fischetti <antonio.fischetti@intel.com> > The tags here look weird, at least your signoff is missing. > --- > lib/flow.h | 32 +++++++++++++++++++++++++++----- > 1 file changed, 27 insertions(+), 5 deletions(-) > > diff --git a/lib/flow.h b/lib/flow.h > index ea24e28..5a14941 100644 > --- a/lib/flow.h > +++ b/lib/flow.h > @@ -564,12 +564,27 @@ flow_values_get_next_in_maps(struct > flow_for_each_in_maps_aux *aux, > flow_values_get_next_in_maps(&aux__, &(VALUE));) > > struct mf_for_each_in_map_aux { > - size_t unit; > - struct flowmap fmap; > - struct flowmap map; > - const uint64_t *values; > + size_t unit; /* Current 64-bit unit of the flowmaps > + being processed. */ > + struct flowmap fmap; /* Remaining 1-bits corresponding to the > + 64-bit words in ‘values’ */ > + struct flowmap map; /* Remaining 1-bits corresponding to the > + 64-bit words of interest. */ > + const uint64_t *values; /* 64-bit words corresponding to the > + 1-bits in ‘fmap’. */ > }; > > +/* Get the data from ‘aux->values’ corresponding to the next lowest 1-bit > + * in ‘aux->map’, given that ‘aux->values’ points to an array of 64-bit > + * words corresponding to the 1-bits in ‘aux->fmap’, starting from the > + * rightmost 1-bit. > + * > + * Returns ’true’ if the traversal is incomplete, ‘false’ otherwise. > + * ‘aux’ is prepared for the next iteration after each call. > + * > + * This is used to traverse through, for example, the values in a miniflow > + * representation of a flow key selected by non-zero 64-bit words in a > + * corresponding subtable mask. */ > static inline bool > mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, > uint64_t *value) > @@ -577,8 +592,10 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, > map_t *map, *fmap; > map_t rm1bit; > > + /* Skip empty map units. */ > while (OVS_UNLIKELY(!*(map = &aux->map.bits[aux->unit]))) { > - /* Skip remaining data in the previous unit. */ > + /* Skip remaining data in the current unit before advancing > + * to the next. */ > aux->values += count_1bits(aux->fmap.bits[aux->unit]); > if (++aux->unit == FLOWMAP_UNITS) { > return false; > @@ -589,7 +606,12 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, > *map -= rm1bit; > fmap = &aux->fmap.bits[aux->unit]; > > + /* If the rightmost 1-bit found from the current unit in ‘aux->map’ > + * (‘rm1bit’) is also present in ‘aux->fmap’, store the corresponding > + * value from ‘aux->values’ to ‘*value', otherwise store 0. */ > if (OVS_LIKELY(*fmap & rm1bit)) { > + /* Skip all 64-bit words in ‘values’ preceding the one > corresponding > + * to ‘rm1bit’. */ > map_t trash = *fmap & (rm1bit - 1); > > *fmap -= trash; > -- > 2.4.11 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
> On Oct 17, 2016, at 8:06 PM, Daniele Di Proietto <diproiettod@ovn.org> wrote: > > 2016-10-14 7:37 GMT-07:00 Bhanuprakash Bodireddy < > bhanuprakash.bodireddy@intel.com <mailto:bhanuprakash.bodireddy@intel.com>>: > >> This patch adds comments to mf_get_next_in_map() to make it more >> comprehensible. >> >> Signed-off-by: Jarno Rajahalme <jarno@ovn.org <mailto:jarno@ovn.org>> >> Acked-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com <mailto:bhanuprakash.bodireddy@intel.com>> >> Acked-by: Antonio Fischetti <antonio.fischetti@intel.com <mailto:antonio.fischetti@intel.com>> >> > > The tags here look weird, at least your signoff is missing. > I guess this was based on all the changes in the patch being written by me. I’m OK with these tags, if this makes sense. Jarno > >> --- >> lib/flow.h | 32 +++++++++++++++++++++++++++----- >> 1 file changed, 27 insertions(+), 5 deletions(-) >> >> diff --git a/lib/flow.h b/lib/flow.h >> index ea24e28..5a14941 100644 >> --- a/lib/flow.h >> +++ b/lib/flow.h >> @@ -564,12 +564,27 @@ flow_values_get_next_in_maps(struct >> flow_for_each_in_maps_aux *aux, >> flow_values_get_next_in_maps(&aux__, &(VALUE));) >> >> struct mf_for_each_in_map_aux { >> - size_t unit; >> - struct flowmap fmap; >> - struct flowmap map; >> - const uint64_t *values; >> + size_t unit; /* Current 64-bit unit of the flowmaps >> + being processed. */ >> + struct flowmap fmap; /* Remaining 1-bits corresponding to the >> + 64-bit words in ‘values’ */ >> + struct flowmap map; /* Remaining 1-bits corresponding to the >> + 64-bit words of interest. */ >> + const uint64_t *values; /* 64-bit words corresponding to the >> + 1-bits in ‘fmap’. */ >> }; >> >> +/* Get the data from ‘aux->values’ corresponding to the next lowest 1-bit >> + * in ‘aux->map’, given that ‘aux->values’ points to an array of 64-bit >> + * words corresponding to the 1-bits in ‘aux->fmap’, starting from the >> + * rightmost 1-bit. >> + * >> + * Returns ’true’ if the traversal is incomplete, ‘false’ otherwise. >> + * ‘aux’ is prepared for the next iteration after each call. >> + * >> + * This is used to traverse through, for example, the values in a miniflow >> + * representation of a flow key selected by non-zero 64-bit words in a >> + * corresponding subtable mask. */ >> static inline bool >> mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, >> uint64_t *value) >> @@ -577,8 +592,10 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, >> map_t *map, *fmap; >> map_t rm1bit; >> >> + /* Skip empty map units. */ >> while (OVS_UNLIKELY(!*(map = &aux->map.bits[aux->unit]))) { >> - /* Skip remaining data in the previous unit. */ >> + /* Skip remaining data in the current unit before advancing >> + * to the next. */ >> aux->values += count_1bits(aux->fmap.bits[aux->unit]); >> if (++aux->unit == FLOWMAP_UNITS) { >> return false; >> @@ -589,7 +606,12 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, >> *map -= rm1bit; >> fmap = &aux->fmap.bits[aux->unit]; >> >> + /* If the rightmost 1-bit found from the current unit in ‘aux->map’ >> + * (‘rm1bit’) is also present in ‘aux->fmap’, store the corresponding >> + * value from ‘aux->values’ to ‘*value', otherwise store 0. */ >> if (OVS_LIKELY(*fmap & rm1bit)) { >> + /* Skip all 64-bit words in ‘values’ preceding the one >> corresponding >> + * to ‘rm1bit’. */ >> map_t trash = *fmap & (rm1bit - 1); >> >> *fmap -= trash; >> -- >> 2.4.11 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org <mailto:dev@openvswitch.org> >> http://openvswitch.org/mailman/listinfo/dev <http://openvswitch.org/mailman/listinfo/dev> >> > _______________________________________________ > dev mailing list > dev@openvswitch.org <mailto:dev@openvswitch.org> > http://openvswitch.org/mailman/listinfo/dev <http://openvswitch.org/mailman/listinfo/dev>
diff --git a/lib/flow.h b/lib/flow.h index ea24e28..5a14941 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -564,12 +564,27 @@ flow_values_get_next_in_maps(struct flow_for_each_in_maps_aux *aux, flow_values_get_next_in_maps(&aux__, &(VALUE));) struct mf_for_each_in_map_aux { - size_t unit; - struct flowmap fmap; - struct flowmap map; - const uint64_t *values; + size_t unit; /* Current 64-bit unit of the flowmaps + being processed. */ + struct flowmap fmap; /* Remaining 1-bits corresponding to the + 64-bit words in ‘values’ */ + struct flowmap map; /* Remaining 1-bits corresponding to the + 64-bit words of interest. */ + const uint64_t *values; /* 64-bit words corresponding to the + 1-bits in ‘fmap’. */ }; +/* Get the data from ‘aux->values’ corresponding to the next lowest 1-bit + * in ‘aux->map’, given that ‘aux->values’ points to an array of 64-bit + * words corresponding to the 1-bits in ‘aux->fmap’, starting from the + * rightmost 1-bit. + * + * Returns ’true’ if the traversal is incomplete, ‘false’ otherwise. + * ‘aux’ is prepared for the next iteration after each call. + * + * This is used to traverse through, for example, the values in a miniflow + * representation of a flow key selected by non-zero 64-bit words in a + * corresponding subtable mask. */ static inline bool mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, uint64_t *value) @@ -577,8 +592,10 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, map_t *map, *fmap; map_t rm1bit; + /* Skip empty map units. */ while (OVS_UNLIKELY(!*(map = &aux->map.bits[aux->unit]))) { - /* Skip remaining data in the previous unit. */ + /* Skip remaining data in the current unit before advancing + * to the next. */ aux->values += count_1bits(aux->fmap.bits[aux->unit]); if (++aux->unit == FLOWMAP_UNITS) { return false; @@ -589,7 +606,12 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, *map -= rm1bit; fmap = &aux->fmap.bits[aux->unit]; + /* If the rightmost 1-bit found from the current unit in ‘aux->map’ + * (‘rm1bit’) is also present in ‘aux->fmap’, store the corresponding + * value from ‘aux->values’ to ‘*value', otherwise store 0. */ if (OVS_LIKELY(*fmap & rm1bit)) { + /* Skip all 64-bit words in ‘values’ preceding the one corresponding + * to ‘rm1bit’. */ map_t trash = *fmap & (rm1bit - 1); *fmap -= trash;