Re: [ovs-dev] [patch_v7 2/9] dpdk: Remove batch sorting in userspace conntrack.

[Darrell Ball]

Thanks for pointing that out; it was on my todo list from before and I
forgot about it.

On 5/1/17, 10:54 AM, "ovs-dev-boun...@openvswitch.org on behalf of Jarno 
Rajahalme"  wrote:

Would be nice to have a commit message, with the motivation for the change.

  Jarno

> On Mar 24, 2017, at 2:15 AM, Darrell Ball  wrote:
> 
> Signed-off-by: Darrell Ball 
> Acked-by: Flavio Leitner 
> ---
> lib/conntrack.c | 58 
+++--
> 1 file changed, 11 insertions(+), 47 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 4f490fb..9a0763e 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -318,22 +318,9 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
> {
> struct dp_packet **pkts = pkt_batch->packets;
> size_t cnt = pkt_batch->count;
> -#if !defined(__CHECKER__) && !defined(_WIN32)
> -const size_t KEY_ARRAY_SIZE = cnt;
> -#else
> -enum { KEY_ARRAY_SIZE = NETDEV_MAX_BURST };
> -#endif
> -struct conn_lookup_ctx ctxs[KEY_ARRAY_SIZE];
> -int8_t bucket_list[CONNTRACK_BUCKETS];
> -struct {
> -unsigned bucket;
> -unsigned long maps;
> -} arr[KEY_ARRAY_SIZE];
> +struct conn_lookup_ctx ctx;
> long long now = time_msec();
> size_t i = 0;
> -uint8_t arrcnt = 0;
> -
> -BUILD_ASSERT_DECL(sizeof arr[0].maps * CHAR_BIT >= NETDEV_MAX_BURST);
> 
> if (helper) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -342,48 +329,25 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
> /* Continue without the helper */
> }
> 
> -memset(bucket_list, INT8_C(-1), sizeof bucket_list);
> for (i = 0; i < cnt; i++) {
> -unsigned bucket;
> 
> -if (!conn_key_extract(ct, pkts[i], dl_type, [i], zone)) {
> +if (!conn_key_extract(ct, pkts[i], dl_type, , zone)) {
> write_ct_md(pkts[i], CS_INVALID, zone, NULL, NULL);
> continue;
> }
> 
> -bucket = hash_to_bucket(ctxs[i].hash);
> -if (bucket_list[bucket] == INT8_C(-1)) {
> -bucket_list[bucket] = arrcnt;
> -
> -arr[arrcnt].maps = 0;
> -ULLONG_SET1(arr[arrcnt].maps, i);
> -arr[arrcnt++].bucket = bucket;
> -} else {
> -ULLONG_SET1(arr[bucket_list[bucket]].maps, i);
> -}
> -}
> -
> -for (i = 0; i < arrcnt; i++) {
> -struct conntrack_bucket *ctb = >buckets[arr[i].bucket];
> -size_t j;
> -
> +struct conntrack_bucket *ctb = >buckets[i];
> ct_lock_lock(>lock);
> +conn_key_lookup(ctb, , now);
> +struct conn *conn = process_one(ct, pkts[i], , zone,
> +force, commit, now);
> 
> -ULLONG_FOR_EACH_1(j, arr[i].maps) {
> -struct conn *conn;
> -
> -conn_key_lookup(ctb, [j], now);
> -
> -conn = process_one(ct, pkts[j], [j], zone, force, 
commit,
> -   now);
> -
> -if (conn && setmark) {
> -set_mark(pkts[j], conn, setmark[0], setmark[1]);
> -}
> +if (conn && setmark) {
> +set_mark(pkts[i], conn, setmark[0], setmark[1]);
> +}
> 
> -if (conn && setlabel) {
> -set_label(pkts[j], conn, [0], [1]);
> -}
> +if (conn && setlabel) {
> +set_label(pkts[i], conn, [0], [1]);
> }
> ct_lock_unlock(>lock);
> }
> -- 
> 1.9.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> 
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=6g0A5S-JcYSJgz5d0NvkCE57V_TZhpdjImW7b6GhnIY=mtFK9-RPvtX_F1wWJM0SgoFhEWQ2c7v3iiUpUIQiA_A=
 

___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=6g0A5S-JcYSJgz5d0NvkCE57V_TZhpdjImW7b6GhnIY=mtFK9-RPvtX_F1wWJM0SgoFhEWQ2c7v3iiUpUIQiA_A=
 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch_v7 2/9] dpdk: Remove batch sorting in userspace conntrack.

[Jarno Rajahalme]

Would be nice to have a commit message, with the motivation for the change.

  Jarno

> On Mar 24, 2017, at 2:15 AM, Darrell Ball  wrote:
> 
> Signed-off-by: Darrell Ball 
> Acked-by: Flavio Leitner 
> ---
> lib/conntrack.c | 58 +++--
> 1 file changed, 11 insertions(+), 47 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 4f490fb..9a0763e 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -318,22 +318,9 @@ conntrack_execute(struct conntrack *ct, struct 
> dp_packet_batch *pkt_batch,
> {
> struct dp_packet **pkts = pkt_batch->packets;
> size_t cnt = pkt_batch->count;
> -#if !defined(__CHECKER__) && !defined(_WIN32)
> -const size_t KEY_ARRAY_SIZE = cnt;
> -#else
> -enum { KEY_ARRAY_SIZE = NETDEV_MAX_BURST };
> -#endif
> -struct conn_lookup_ctx ctxs[KEY_ARRAY_SIZE];
> -int8_t bucket_list[CONNTRACK_BUCKETS];
> -struct {
> -unsigned bucket;
> -unsigned long maps;
> -} arr[KEY_ARRAY_SIZE];
> +struct conn_lookup_ctx ctx;
> long long now = time_msec();
> size_t i = 0;
> -uint8_t arrcnt = 0;
> -
> -BUILD_ASSERT_DECL(sizeof arr[0].maps * CHAR_BIT >= NETDEV_MAX_BURST);
> 
> if (helper) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -342,48 +329,25 @@ conntrack_execute(struct conntrack *ct, struct 
> dp_packet_batch *pkt_batch,
> /* Continue without the helper */
> }
> 
> -memset(bucket_list, INT8_C(-1), sizeof bucket_list);
> for (i = 0; i < cnt; i++) {
> -unsigned bucket;
> 
> -if (!conn_key_extract(ct, pkts[i], dl_type, [i], zone)) {
> +if (!conn_key_extract(ct, pkts[i], dl_type, , zone)) {
> write_ct_md(pkts[i], CS_INVALID, zone, NULL, NULL);
> continue;
> }
> 
> -bucket = hash_to_bucket(ctxs[i].hash);
> -if (bucket_list[bucket] == INT8_C(-1)) {
> -bucket_list[bucket] = arrcnt;
> -
> -arr[arrcnt].maps = 0;
> -ULLONG_SET1(arr[arrcnt].maps, i);
> -arr[arrcnt++].bucket = bucket;
> -} else {
> -ULLONG_SET1(arr[bucket_list[bucket]].maps, i);
> -}
> -}
> -
> -for (i = 0; i < arrcnt; i++) {
> -struct conntrack_bucket *ctb = >buckets[arr[i].bucket];
> -size_t j;
> -
> +struct conntrack_bucket *ctb = >buckets[i];
> ct_lock_lock(>lock);
> +conn_key_lookup(ctb, , now);
> +struct conn *conn = process_one(ct, pkts[i], , zone,
> +force, commit, now);
> 
> -ULLONG_FOR_EACH_1(j, arr[i].maps) {
> -struct conn *conn;
> -
> -conn_key_lookup(ctb, [j], now);
> -
> -conn = process_one(ct, pkts[j], [j], zone, force, commit,
> -   now);
> -
> -if (conn && setmark) {
> -set_mark(pkts[j], conn, setmark[0], setmark[1]);
> -}
> +if (conn && setmark) {
> +set_mark(pkts[i], conn, setmark[0], setmark[1]);
> +}
> 
> -if (conn && setlabel) {
> -set_label(pkts[j], conn, [0], [1]);
> -}
> +if (conn && setlabel) {
> +set_label(pkts[i], conn, [0], [1]);
> }
> ct_lock_unlock(>lock);
> }
> -- 
> 1.9.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch_v7 2/9] dpdk: Remove batch sorting in userspace conntrack.

[Daniele Di Proietto]

Thanks for doing this

Acked-by: Daniele Di Proietto 

2017-03-24 2:15 GMT-07:00 Darrell Ball :
> Signed-off-by: Darrell Ball 
> Acked-by: Flavio Leitner 
> ---
>  lib/conntrack.c | 58 
> +++--
>  1 file changed, 11 insertions(+), 47 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 4f490fb..9a0763e 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -318,22 +318,9 @@ conntrack_execute(struct conntrack *ct, struct 
> dp_packet_batch *pkt_batch,
>  {
>  struct dp_packet **pkts = pkt_batch->packets;
>  size_t cnt = pkt_batch->count;
> -#if !defined(__CHECKER__) && !defined(_WIN32)
> -const size_t KEY_ARRAY_SIZE = cnt;
> -#else
> -enum { KEY_ARRAY_SIZE = NETDEV_MAX_BURST };
> -#endif
> -struct conn_lookup_ctx ctxs[KEY_ARRAY_SIZE];
> -int8_t bucket_list[CONNTRACK_BUCKETS];
> -struct {
> -unsigned bucket;
> -unsigned long maps;
> -} arr[KEY_ARRAY_SIZE];
> +struct conn_lookup_ctx ctx;
>  long long now = time_msec();
>  size_t i = 0;
> -uint8_t arrcnt = 0;
> -
> -BUILD_ASSERT_DECL(sizeof arr[0].maps * CHAR_BIT >= NETDEV_MAX_BURST);
>
>  if (helper) {
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -342,48 +329,25 @@ conntrack_execute(struct conntrack *ct, struct 
> dp_packet_batch *pkt_batch,
>  /* Continue without the helper */
>  }
>
> -memset(bucket_list, INT8_C(-1), sizeof bucket_list);
>  for (i = 0; i < cnt; i++) {
> -unsigned bucket;
>
> -if (!conn_key_extract(ct, pkts[i], dl_type, [i], zone)) {
> +if (!conn_key_extract(ct, pkts[i], dl_type, , zone)) {
>  write_ct_md(pkts[i], CS_INVALID, zone, NULL, NULL);
>  continue;
>  }
>
> -bucket = hash_to_bucket(ctxs[i].hash);
> -if (bucket_list[bucket] == INT8_C(-1)) {
> -bucket_list[bucket] = arrcnt;
> -
> -arr[arrcnt].maps = 0;
> -ULLONG_SET1(arr[arrcnt].maps, i);
> -arr[arrcnt++].bucket = bucket;
> -} else {
> -ULLONG_SET1(arr[bucket_list[bucket]].maps, i);
> -}
> -}
> -
> -for (i = 0; i < arrcnt; i++) {
> -struct conntrack_bucket *ctb = >buckets[arr[i].bucket];
> -size_t j;
> -
> +struct conntrack_bucket *ctb = >buckets[i];
>  ct_lock_lock(>lock);
> +conn_key_lookup(ctb, , now);
> +struct conn *conn = process_one(ct, pkts[i], , zone,
> +force, commit, now);
>
> -ULLONG_FOR_EACH_1(j, arr[i].maps) {
> -struct conn *conn;
> -
> -conn_key_lookup(ctb, [j], now);
> -
> -conn = process_one(ct, pkts[j], [j], zone, force, commit,
> -   now);
> -
> -if (conn && setmark) {
> -set_mark(pkts[j], conn, setmark[0], setmark[1]);
> -}
> +if (conn && setmark) {
> +set_mark(pkts[i], conn, setmark[0], setmark[1]);
> +}
>
> -if (conn && setlabel) {
> -set_label(pkts[j], conn, [0], [1]);
> -}
> +if (conn && setlabel) {
> +set_label(pkts[i], conn, [0], [1]);
>  }
>  ct_lock_unlock(>lock);
>  }
> --
> 1.9.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [patch_v7 2/9] dpdk: Remove batch sorting in userspace conntrack.

[Darrell Ball]

Signed-off-by: Darrell Ball 
Acked-by: Flavio Leitner 
---
 lib/conntrack.c | 58 +++--
 1 file changed, 11 insertions(+), 47 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 4f490fb..9a0763e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -318,22 +318,9 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
 {
 struct dp_packet **pkts = pkt_batch->packets;
 size_t cnt = pkt_batch->count;
-#if !defined(__CHECKER__) && !defined(_WIN32)
-const size_t KEY_ARRAY_SIZE = cnt;
-#else
-enum { KEY_ARRAY_SIZE = NETDEV_MAX_BURST };
-#endif
-struct conn_lookup_ctx ctxs[KEY_ARRAY_SIZE];
-int8_t bucket_list[CONNTRACK_BUCKETS];
-struct {
-unsigned bucket;
-unsigned long maps;
-} arr[KEY_ARRAY_SIZE];
+struct conn_lookup_ctx ctx;
 long long now = time_msec();
 size_t i = 0;
-uint8_t arrcnt = 0;
-
-BUILD_ASSERT_DECL(sizeof arr[0].maps * CHAR_BIT >= NETDEV_MAX_BURST);
 
 if (helper) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
@@ -342,48 +329,25 @@ conntrack_execute(struct conntrack *ct, struct 
dp_packet_batch *pkt_batch,
 /* Continue without the helper */
 }
 
-memset(bucket_list, INT8_C(-1), sizeof bucket_list);
 for (i = 0; i < cnt; i++) {
-unsigned bucket;
 
-if (!conn_key_extract(ct, pkts[i], dl_type, [i], zone)) {
+if (!conn_key_extract(ct, pkts[i], dl_type, , zone)) {
 write_ct_md(pkts[i], CS_INVALID, zone, NULL, NULL);
 continue;
 }
 
-bucket = hash_to_bucket(ctxs[i].hash);
-if (bucket_list[bucket] == INT8_C(-1)) {
-bucket_list[bucket] = arrcnt;
-
-arr[arrcnt].maps = 0;
-ULLONG_SET1(arr[arrcnt].maps, i);
-arr[arrcnt++].bucket = bucket;
-} else {
-ULLONG_SET1(arr[bucket_list[bucket]].maps, i);
-}
-}
-
-for (i = 0; i < arrcnt; i++) {
-struct conntrack_bucket *ctb = >buckets[arr[i].bucket];
-size_t j;
-
+struct conntrack_bucket *ctb = >buckets[i];
 ct_lock_lock(>lock);
+conn_key_lookup(ctb, , now);
+struct conn *conn = process_one(ct, pkts[i], , zone,
+force, commit, now);
 
-ULLONG_FOR_EACH_1(j, arr[i].maps) {
-struct conn *conn;
-
-conn_key_lookup(ctb, [j], now);
-
-conn = process_one(ct, pkts[j], [j], zone, force, commit,
-   now);
-
-if (conn && setmark) {
-set_mark(pkts[j], conn, setmark[0], setmark[1]);
-}
+if (conn && setmark) {
+set_mark(pkts[i], conn, setmark[0], setmark[1]);
+}
 
-if (conn && setlabel) {
-set_label(pkts[j], conn, [0], [1]);
-}
+if (conn && setlabel) {
+set_label(pkts[i], conn, [0], [1]);
 }
 ct_lock_unlock(>lock);
 }
-- 
1.9.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev