>-----Original Message-----
>From: Daniele Di Proietto [mailto:diproiet...@ovn.org]
>Sent: Tuesday, October 18, 2016 4:04 AM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com>
>Cc: dev@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v3 01/12] dpcls: Use 32 packet batches for
>lookups.
>
>
>
>2016-10-14 7:37 GMT-07:00 Bhanuprakash Bodireddy
><bhanuprakash.bodire...@intel.com>:
>This patch increases the number of packets processed in a batch during a
>lookup from 16 to 32. Processing batches of 32 packets improves
>performance and also one of the internal loops can be avoided here.
>
>Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
>Co-authored-by: Bhanuprakash Bodireddy
><bhanuprakash.bodire...@intel.com>
>Signed-off-by: Bhanuprakash Bodireddy
><bhanuprakash.bodire...@intel.com>
>
>I guess Co-authored-by should have Antonio?
My mistake, I am sending out the remaining 4 patches separately with
appropriate tags.
>Also, the (already existing code) is not trivial, I'd like to take another
>look at it
Sure, let us know if you have more comments.
Regards,
Bhanuprakash.
>Thanks,
>Daniele
>
>---
> lib/dpif-netdev.c | 110 ++++++++++++++++++++++-------------------------------
>-
> 1 file changed, 45 insertions(+), 65 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index eb9f764..0a4f338 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -4985,23 +4985,21 @@ dpcls_lookup(struct dpcls *cls, const struct
>netdev_flow_key keys[],
> int *num_lookups_p)
> {
> /* The received 'cnt' miniflows are the search-keys that will be processed
>- * in batches of 16 elements. N_MAPS will contain the number of these
>- * 16-elements batches. i.e. for 'cnt' = 32, N_MAPS will be 2. The batch
>- * size 16 was experimentally found faster than 8 or 32. */
>- typedef uint16_t map_type;
>+ * to find a matching entry into the available subtables.
>+ * The number of bits in map_type is equal to NETDEV_MAX_BURST. */
>+ typedef uint32_t map_type;
> #define MAP_BITS (sizeof(map_type) * CHAR_BIT)
>+ BUILD_ASSERT_DECL(MAP_BITS >= NETDEV_MAX_BURST);
>
>-#if !defined(__CHECKER__) && !defined(_WIN32)
>- const int N_MAPS = DIV_ROUND_UP(cnt, MAP_BITS);
>-#else
>- enum { N_MAPS = DIV_ROUND_UP(NETDEV_MAX_BURST, MAP_BITS) };
>-#endif
>- map_type maps[N_MAPS];
> struct dpcls_subtable *subtable;
>
>- memset(maps, 0xff, sizeof maps);
>- if (cnt % MAP_BITS) {
>- maps[N_MAPS - 1] >>= MAP_BITS - cnt % MAP_BITS; /* Clear extra bits.
>*/
>+ map_type keys_map = TYPE_MAXIMUM(map_type);
>+ map_type found_map;
>+ uint32_t hashes[MAP_BITS];
>+ const struct cmap_node *nodes[MAP_BITS];
>+
>+ if (cnt != NETDEV_MAX_BURST) {
>+ keys_map >>= NETDEV_MAX_BURST - cnt; /* Clear extra bits. */
> }
> memset(rules, 0, cnt * sizeof *rules);
>
>@@ -5015,61 +5013,43 @@ dpcls_lookup(struct dpcls *cls, const struct
>netdev_flow_key keys[],
> * search-key, the search for that key can stop because the rules are
> * non-overlapping. */
> PVECTOR_FOR_EACH (subtable, &cls->subtables) {
>- const struct netdev_flow_key *mkeys = keys;
>- struct dpcls_rule **mrules = rules;
>- map_type remains = 0;
>- int m;
>-
>- BUILD_ASSERT_DECL(sizeof remains == sizeof *maps);
>-
>- /* Loops on each batch of 16 search-keys. */
>- for (m = 0; m < N_MAPS; m++, mkeys += MAP_BITS, mrules +=
>MAP_BITS) {
>- uint32_t hashes[MAP_BITS];
>- const struct cmap_node *nodes[MAP_BITS];
>- unsigned long map = maps[m];
>- int i;
>-
>- if (!map) {
>- continue; /* Skip empty maps. */
>- }
>-
>- /* Compute hashes for the remaining keys. Each search-key is
>- * masked with the subtable's mask to avoid hashing the wildcarded
>- * bits. */
>- ULLONG_FOR_EACH_1(i, map) {
>- hashes[i] = netdev_flow_key_hash_in_mask(&mkeys[i],
>- &subtable->mask);
>- }
>- /* Lookup. */
>- map = cmap_find_batch(&subtable->rules, map, hashes, nodes);
>- /* Check results. When the i-th bit of map is set, it means that
>a
>- * set of nodes with a matching hash value was found for the i-th
>- * search-key. Due to possible hash collisions we need to check
>- * which of the found rules, if any, really matches our masked
>- * search-key. */
>- ULLONG_FOR_EACH_1(i, map) {
>- struct dpcls_rule *rule;
>-
>- CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
>- if (OVS_LIKELY(dpcls_rule_matches_key(rule, &mkeys[i]))) {
>- mrules[i] = rule;
>- /* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
>- * within one second optimization interval */
>- subtable->hit_cnt++;
>- lookups_match += subtable_pos;
>- goto next;
>- }
>+ int i;
>+
>+ /* Compute hashes for the remaining keys. Each search-key is
>+ * masked with the subtable's mask to avoid hashing the wildcarded
>+ * bits. */
>+ ULLONG_FOR_EACH_1(i, keys_map) {
>+ hashes[i] = netdev_flow_key_hash_in_mask(&keys[i],
>+ &subtable->mask);
>+ }
>+ /* Lookup. */
>+ found_map = cmap_find_batch(&subtable->rules, keys_map, hashes,
>nodes);
>+ /* Check results. When the i-th bit of found_map is set, it means
>+ * that a set of nodes with a matching hash value was found for the
>+ * i-th search-key. Due to possible hash collisions we need to check
>+ * which of the found rules, if any, really matches our masked
>+ * search-key. */
>+ ULLONG_FOR_EACH_1(i, found_map) {
>+ struct dpcls_rule *rule;
>+
>+ CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
>+ if (OVS_LIKELY(dpcls_rule_matches_key(rule, &keys[i]))) {
>+ rules[i] = rule;
>+ /* Even at 20 Mpps the 32-bit hit_cnt cannot wrap
>+ * within one second optimization interval. */
>+ subtable->hit_cnt++;
>+ lookups_match += subtable_pos;
>+ goto next;
> }
>- /* None of the found rules was a match. Reset the i-th bit to
>- * keep searching in the next subtable. */
>- ULLONG_SET0(map, i); /* Did not match. */
>- next:
>- ; /* Keep Sparse happy. */
> }
>- maps[m] &= ~map; /* Clear the found rules. */
>- remains |= maps[m];
>+ /* None of the found rules was a match. Reset the i-th bit to
>+ * keep searching this key in the next subtable. */
>+ ULLONG_SET0(found_map, i); /* Did not match. */
>+ next:
>+ ; /* Keep Sparse happy. */
> }
>- if (!remains) {
>+ keys_map &= ~found_map; /* Clear the found rules. */
>+ if (!keys_map) {
> if (num_lookups_p) {
> *num_lookups_p = lookups_match;
> }
>--
>2.4.11
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
