Re: [ovs-discuss] [OVN][RFC] ovn-northd simple optimization converting uuid from string
Nice finding!
I don't think it's necessary to inline this into the header file to get
the speedup, since its caller along the uuid_from_string() call chain is
in the same file as hexit_value().
I sent out a patch that does something similar:
https://patchwork.ozlabs.org/patch/868826/
On Fri, Feb 02, 2018 at 07:24:59PM +0100, Daniel Alvarez Sanchez wrote:
> Hi folks,
>
> While running rally in OpenStack we found out that ovn-northd was
> at 100% CPU most of the time. It doesn't have to be necessarily
> a problem but I wanted to do a simple profiling by running a rally task
> which creates a network (Logical Switch) and creates 6 ports on it,
> repeating the whole operation 1000 times. The ports are networks
> are also deleted.
>
> I used master branch and compiled it with -O1:
>
> CFLAGS="-O1 -g" ./configure --prefix=/usr/local/
> --with-linux=/usr/lib/modules/`ls /usr/lib/modules/ | tail -n 1`/build
>
> gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
>
> What I saw is that ~ 15% of the execution time was spent in
> uuid_from_string() function in util/uuid.c module which calls hexits_value()
> which ends up calling hexit_value(). This last function gets called >1M
> times.
>
> I thought that it was worth to inline hexit_value() and use a lookup table
> instead the switch/case [0] so I did it (find the patch below) and the gain
> is
> that instead of a 14%, uuid_from_string() now takes a 9% of the total
> execution time. See [1].
>
> [0]
> https://github.com/openvswitch/ovs/blob/79feb3b0de83932c6cbf761d70051330db4d07f7/lib/util.c#L844
> [1] https://imgur.com/a/3gzDF
>
> Patch:
> Note that we could make the table smaller to optimize the data cache usage
> but then we would have to accommodate the argument and include extra
> checks.
>
>
> ---
>
> diff --git a/lib/util.c b/lib/util.c
> index a4d22df0c..a24472690 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -839,38 +839,6 @@ str_to_double(const char *s, double *d)
> }
> }
>
> -/* Returns the value of 'c' as a hexadecimal digit. */
> -int
> -hexit_value(int c)
> -{
> -switch (c) {
> -case '0': case '1': case '2': case '3': case '4':
> -case '5': case '6': case '7': case '8': case '9':
> -return c - '0';
> -
> -case 'a': case 'A':
> -return 0xa;
> -
> -case 'b': case 'B':
> -return 0xb;
> -
> -case 'c': case 'C':
> -return 0xc;
> -
> -case 'd': case 'D':
> -return 0xd;
> -
> -case 'e': case 'E':
> -return 0xe;
> -
> -case 'f': case 'F':
> -return 0xf;
> -
> -default:
> -return -1;
> -}
> -}
> -
> /* Returns the integer value of the 'n' hexadecimal digits starting at
> 's', or
> * UINTMAX_MAX if one of those "digits" is not really a hex digit. Sets
> '*ok'
> * to true if the conversion succeeds or to false if a non-hex digit is
> diff --git a/lib/util.h b/lib/util.h
> index b6639b8b8..f41e2a030 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -217,7 +217,28 @@ bool ovs_scan_len(const char *s, int *n, const char
> *format, ...);
>
> bool str_to_double(const char *, double *);
>
> -int hexit_value(int c);
> +
> +static const char hextable[] = {
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1, 0,1,2,3,4,5,6,7,8,9,-1,-1,-1,-1,-1,-1,-1,10,11,12,13,14,15,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,10,11,12,13,14,15,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
> +-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1
> +};
> +
> +/* Returns the value of 'c' as a hexadecimal digit. */
> +static inline int
> +hexit_value(int c)
> +{
> +return hextable[c & 0xff];
> +}
> +
> uintmax_t hexits_value(const char *s, size_t n, bool *ok);
>
> int parse_int_string(const char *s, uint8_t *valuep, int field_width,
>
>
> ---
> ___
> discuss mailing list
> disc...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
[ovs-discuss] [OVN][RFC] ovn-northd simple optimization converting uuid from string
Hi folks,
While running rally in OpenStack we found out that ovn-northd was
at 100% CPU most of the time. It doesn't have to be necessarily
a problem but I wanted to do a simple profiling by running a rally task
which creates a network (Logical Switch) and creates 6 ports on it,
repeating the whole operation 1000 times. The ports are networks
are also deleted.
I used master branch and compiled it with -O1:
CFLAGS="-O1 -g" ./configure --prefix=/usr/local/
--with-linux=/usr/lib/modules/`ls /usr/lib/modules/ | tail -n 1`/build
gcc version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)
What I saw is that ~ 15% of the execution time was spent in
uuid_from_string() function in util/uuid.c module which calls hexits_value()
which ends up calling hexit_value(). This last function gets called >1M
times.
I thought that it was worth to inline hexit_value() and use a lookup table
instead the switch/case [0] so I did it (find the patch below) and the gain
is
that instead of a 14%, uuid_from_string() now takes a 9% of the total
execution time. See [1].
[0]
https://github.com/openvswitch/ovs/blob/79feb3b0de83932c6cbf761d70051330db4d07f7/lib/util.c#L844
[1] https://imgur.com/a/3gzDF
Patch:
Note that we could make the table smaller to optimize the data cache usage
but then we would have to accommodate the argument and include extra
checks.
---
diff --git a/lib/util.c b/lib/util.c
index a4d22df0c..a24472690 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -839,38 +839,6 @@ str_to_double(const char *s, double *d)
}
}
-/* Returns the value of 'c' as a hexadecimal digit. */
-int
-hexit_value(int c)
-{
-switch (c) {
-case '0': case '1': case '2': case '3': case '4':
-case '5': case '6': case '7': case '8': case '9':
-return c - '0';
-
-case 'a': case 'A':
-return 0xa;
-
-case 'b': case 'B':
-return 0xb;
-
-case 'c': case 'C':
-return 0xc;
-
-case 'd': case 'D':
-return 0xd;
-
-case 'e': case 'E':
-return 0xe;
-
-case 'f': case 'F':
-return 0xf;
-
-default:
-return -1;
-}
-}
-
/* Returns the integer value of the 'n' hexadecimal digits starting at
's', or
* UINTMAX_MAX if one of those "digits" is not really a hex digit. Sets
'*ok'
* to true if the conversion succeeds or to false if a non-hex digit is
diff --git a/lib/util.h b/lib/util.h
index b6639b8b8..f41e2a030 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -217,7 +217,28 @@ bool ovs_scan_len(const char *s, int *n, const char
*format, ...);
bool str_to_double(const char *, double *);
-int hexit_value(int c);
+
+static const char hextable[] = {
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1, 0,1,2,3,4,5,6,7,8,9,-1,-1,-1,-1,-1,-1,-1,10,11,12,13,14,15,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,10,11,12,13,14,15,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,
+-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1
+};
+
+/* Returns the value of 'c' as a hexadecimal digit. */
+static inline int
+hexit_value(int c)
+{
+return hextable[c & 0xff];
+}
+
uintmax_t hexits_value(const char *s, size_t n, bool *ok);
int parse_int_string(const char *s, uint8_t *valuep, int field_width,
---
___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
Re: [ovs-discuss] Running ovs-appctl to trace packet
On Fri, Feb 02, 2018 at 10:29:24AM -0500, Myra Sh wrote: > Hello, > > I want to use the same idea as the following link: > > https://techandtrains.com/2014/02/08/running-open-vswitch-in-network-namespace/ > > > But I still have a problem to trace packets. Is there any other way to > trace packets in OVS? No. ___ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
[ovs-discuss] About ovs2.8.1 & meter & iperf
Hi， excuse me,when I used the ovs2.8.1 and implemented “ovs-ofctl -O OpenFlow13 add-meter s2 meter=1,kbps,band=type=drop,rate=1” the CLI output “OFPMMFC_INVALID_METER”. If I typed "ovs-vsctl set bridge s2 datapath_type=netdev" before, it was succeed. But if ovs was ”netdev“, the iperf test was failture. ryu-manager simple_switch_13.py mn --topo linear,3 --mac --switch ovs,protocols=OpenFlow13 --controller remote The result is as follows： The server did not show the result. So I really want to know "netdev" model and the meter has what kind of relationship？ What to do to use meter in ovs2.8.1 and test success？ I hope you can answer it. Thank you very much!___ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
Re: [ovs-discuss] Running ovs-appctl to trace packet
Hello, I want to use the same idea as the following link: https://techandtrains.com/2014/02/08/running-open-vswitch-in-network-namespace/ But I still have a problem to trace packets. Is there any other way to trace packets in OVS? Thanks On Thu, Feb 1, 2018 at 12:23 PM, Ben Pfaff wrote: > Oh, and why are you running multiple instances of OVS? It's unusual. > > On Thu, Feb 01, 2018 at 09:22:31AM -0800, Ben Pfaff wrote: > > The rundir for OVS and for ovs-appctl should match. You can do that by > > defining OVS_RUNDIR in the environment for each one, otherwise you end > > up having to give full paths. > > > > On Thu, Feb 01, 2018 at 11:09:42AM -0500, Myra Sh wrote: > > > Thank you for your reply. > > > > > > I tried: > > > OVS_RUNDIR=$(pwd) > > > exportOVS_RUNDIR > > > in all nodes. > > > > > > > > > After that,when I run for example: *"ovs-appctl ofproto/trace br1 > > > in_port=1"* on bridge1, I receive the following message: > > > > > > *WARN|failed to connect to .../n1.conf/ovs-vswitchd.50.ctl* > > > > > > > > > > > > there is no ovs-vswitch.50.ctl in $(pwd) directory. > > > > > > I found *ovs-vswitchd.50.ctl* in */usr/local/var/run/openvswitch/ > *directory > > > and after that: > > > > > > *ovs-appctl -t /usr/local/var/run/openvswitch/ ovs-vswitchd.50.ctl > > > bridge/dump-flows br1 * > > > > > > result: > > > > > > *unknown bridge* > > > *ovs-appctl: /usr/local/var/run/openvswitch/ ovs-vswitchd.50.ctl > :server > > > returend an error* > > > > > > > > > > > > the mentioned command only works on a specific bridge(br3). > > > > > > > > > So I guess I should try to find the correct ovs-vswitchd.*.ctl for > each > > > bridge but it's not reasonable when I have several OVSs. > > > > > > Do you have any suggestion in this case? > > > > > > Is there any other way to trace the packets in OVS? > > > > > > > > > > > > > > > > > > On Wed, Jan 31, 2018 at 4:15 PM, Ben Pfaff wrote: > > > > > > > On Wed, Jan 31, 2018 at 02:53:25PM -0500, Myra Sh wrote: > > > > > Hello, > > > > > > > > > > I run several instances of OVS and I am using the following > command and > > > > > path to configure them: > > > > > > > > > > ovs-vsctl --db=unix:$(pwd)/db.sock add-br br0 > > > > > > > > > > The problem in having several instances is that when I want to run > > > > > "ovs-appctl ..." to trace packets, I have to define a path or > target for > > > > > this command. > > > > > > > > > > Do you have any suggestion that how I can define this target path > for > > > > each > > > > > OVS in the mentioned scenario? > > > > > > > > > > Should I check for any ovs-vswitch.*.ctl file and put that in the > target > > > > > part? > > > > > > > > It sounds like you should run something like "OVS_RUNDIR=$(pwd); > export > > > > OVS_RUNDIR". > > > > > ___ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
