Re: [ovs-dev] [PATCH] ovsdb-server: Forbid user-specified databases with reserved names.

2017-12-22 Thread Justin Pettit


> On Dec 22, 2017, at 12:49 PM, Ben Pfaff  wrote:
> 
> On Thu, Dec 21, 2017 at 04:18:05PM -0800, Justin Pettit wrote:
>> 
>> 
>>> On Dec 8, 2017, at 12:53 PM, Ben Pfaff  wrote:
>>> 
>>> +if (argc > 2 && argv[1][0] == '_') {
>>> +unixctl_command_reply_error(conn, "cannot compact built-in 
>>> databases");
>>> +return;
>>> +}
>> 
>> This error condition seems a little odd to me.  I think it will only provide 
>> this warning if multiple databases are specified, and only complain if the 
>> first one is built-in.
> 
> You're right, there's a bug here, and I didn't test it.  Shame on me.
> 
> (However, the command supports at most one named database on the command
> line, which is part of the reason the test is somewhat odd.)

Oh, yeah, I see that now when I look at the command registration.  I was thrown 
off since I think the man page indicates multiple are supported:

   ovsdb-server/compact [db]...
  Compacts each database db in-place.  If no db is specified, com‐
  pacts every database in-place. 

I sent out a patch to update the man page.

Thanks for the incremental patch; it looks good to me.

--Justin


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


[ovs-dev] [PATCH 2/2] ovsdb-client: Remove extraneous markup from man page.

2017-12-22 Thread Justin Pettit
Signed-off-by: Justin Pettit 
---
 ovsdb/ovsdb-client.1.in | 68 -
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
index 203d70cfac2d..5dbd49f25263 100644
--- a/ovsdb/ovsdb-client.1.in
+++ b/ovsdb/ovsdb-client.1.in
@@ -14,43 +14,43 @@ ovsdb\-client \- command-line interface to 
\fBovsdb-server\fR(1)
 .
 .SH SYNOPSIS
 .IP "Server-Level Commands:"
-\fBovsdb\-client \fR[\fIoptions\fR] \fBlist\-dbs \fR[\fIserver\fR]
+\fBovsdb\-client\fR [\fIoptions\fR] \fBlist\-dbs\fR [\fIserver\fR]
 .IP "Database Schema Commands:"
-\fBovsdb\-client \fR[\fIoptions\fR] \fBget\-schema \fR[\fIserver\fR] 
\fR[\fIdatabase\fR]
+\fBovsdb\-client\fR [\fIoptions\fR] \fBget\-schema\fR [\fIserver\fR] 
[\fIdatabase\fR]
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBlist\-tables\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR]
+\fBovsdb\-client\fR [\fIoptions\fR] \fBlist\-tables\fR [\fIserver\fR] 
[\fIdatabase\fR]
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBlist\-columns\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR] [\fItable\fR]
+\fBovsdb\-client\fR [\fIoptions\fR] \fBlist\-columns\fR [\fIserver\fR] 
[\fIdatabase\fR] [\fItable\fR]
 .IP "Database Version Management Commands:"
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBget\-schema\-version\fI 
\fR[\fIserver\fR] \fR[\fIdatabase\fR]
+\fBovsdb\-client\fR [\fIoptions\fR] \fBget\-schema\-version\fR [\fIserver\fR] 
[\fIdatabase\fR]
 .IP "Data Management Commands:"
-\fBovsdb\-client \fR[\fIoptions\fR] \fBtransact\fI \fR[\fIserver\fR] 
\fItransaction\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBtransact\fR [\fIserver\fR] 
\fItransaction\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBquery \fR[\fIserver\fR] 
\fItransaction\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBquery\fR [\fIserver\fR] 
\fItransaction\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBdump\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR]\fR [\fItable\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBdump\fR [\fIserver\fR] [\fIdatabase\fR] 
[\fItable\fR
 [\fIcolumn\fR...]]
 .br
-\fBovsdb\-client \fR[\fIoptions\fR]
-\fBbackup \fR[\fIserver\fR] [\fIdatabase\fR] \fB> \fIsnapshot\fR
+\fBovsdb\-client\fR [\fIoptions\fR]
+\fBbackup\fR [\fIserver\fR] [\fIdatabase\fR] \fB> \fIsnapshot\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] [\fB\-\-force\fR]
-\fBrestore \fR[\fIserver\fR] [\fIdatabase\fR] \fB< \fIsnapshot\fR
+\fBovsdb\-client\fR [\fIoptions\fR] [\fB\-\-force\fR]
+\fBrestore\fR [\fIserver\fR] [\fIdatabase\fR] \fB< \fIsnapshot\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBmonitor\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR] \fItable\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBmonitor\fR [\fIserver\fR] 
[\fIdatabase\fR] \fItable\fR
 [\fIcolumn\fR[\fB,\fIcolumn\fR]...]...
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBmonitor\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR] \fBALL\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBmonitor\fR [\fIserver\fR] 
[\fIdatabase\fR] \fBALL\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBmonitor\-cond\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR] \fIconditions
+\fBovsdb\-client\fR [\fIoptions\fR] \fBmonitor\-cond\fR [\fIserver\fR] 
[\fIdatabase\fR] \fIconditions
 \fItable\fR [\fIcolumn\fR[\fB,\fIcolumn\fR]...]...
 .IP "Testing Commands:"
-\fBovsdb\-client \fR[\fIoptions\fR] \fBlock\fI \fR[\fIserver\fR] \fIlock\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBlock\fR [\fIserver\fR] \fIlock\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBsteal\fI \fR[\fIserver\fR] \fIlock\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBsteal\fR [\fIserver\fR] \fIlock\fR
 .br
-\fBovsdb\-client \fR[\fIoptions\fR] \fBunlock\fI \fR[\fIserver\fR] \fIlock\fR
+\fBovsdb\-client\fR [\fIoptions\fR] \fBunlock\fR [\fIserver\fR] \fIlock\fR
 .br
 .IP "Other Commands:"
 \fBovsdb\-client help\fR
@@ -86,7 +86,7 @@ supports.
 Most \fBovsdb\-client\fR commands work with an individual database,
 but these commands apply to an entire database server.
 .
-.IP "\fBlist\-dbs \fR[\fIserver\fR]"
+.IP "\fBlist\-dbs\fR [\fIserver\fR]"
 Connects to \fIserver\fR, retrieves the list of known databases, and
 prints them one per line.  These database names are the ones that
 other commands may use for \fIdatabase\fR.
@@ -96,16 +96,16 @@ other commands may use for \fIdatabase\fR.
 These commands obtain the schema from a database and print it or part
 of it.
 .
-.IP "\fBget\-schema \fR[\fIserver\fR] \fR[\fIdatabase\fR]"
+.IP "\fBget\-schema\fR [\fIserver\fR] [\fIdatabase\fR]"
 Connects to \fIserver\fR, retrieves the schema for \fIdatabase\fR, and
 prints it in JSON format.
 .
-.IP "\fBlist\-tables\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR]"
+.IP "\fBlist\-tables\fR [\fIserver\fR] [\fIdatabase\fR]"
 Connects to \fIserver\fR, retrieves the schema for \fIdatabase\fR, and
 prints a table listing the name of each table
 within the database.
 .
-.IP "\fBlist\-columns\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] \fItable\fR"
+.IP "\fBlist\-columns\fR [\fIserver\fR] [\fIdatabase\fR] \fItable\fR"
 

[ovs-dev] [PATCH 1/2] ovsdb-server: Update description for "compact" command in man page.

2017-12-22 Thread Justin Pettit
The man page indicated that multiple databases could be specified, but
only one is allowed.

Signed-off-by:Justin Pettit 
---
 ovsdb/ovsdb-server.1.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index 870459e5d9ac..15ff77fd28aa 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -181,8 +181,8 @@ described below.
 These commands are specific to \fBovsdb\-server\fR.
 .IP "\fBexit\fR"
 Causes \fBovsdb\-server\fR to gracefully terminate.
-.IP "\fBovsdb\-server/compact\fR [\fIdb\fR]\&..."
-Compacts each database \fIdb\fR in-place.  If no \fIdb\fR is
+.IP "\fBovsdb\-server/compact\fR [\fIdb\fR]"
+Compacts database \fIdb\fR in-place.  If \fIdb\fR is not
 specified, compacts every database in-place.  A database is also
 compacted automatically when a transaction is logged if it is over 4
 times as large as its previous compacted size (and at least 10 MB),
-- 
2.7.4

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


Re: [ovs-dev] [PATCH 09/13] log: Support using /dev/stdin for opening logs read-only.

2017-12-22 Thread Justin Pettit

> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> On Unix-like systems, usually /dev/stdin opens a duplicate of fd 0, and
> this will be convenient in a few places later on.  This commit makes this
> support universal.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 08/13] log: New function ovsdb_log_compose_record().

2017-12-22 Thread Justin Pettit

> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> This will acquire a new user later on.
> 
> Signed-off-by: Ben Pfaff 

I think this patch has already been reviewed and merged.

--Justin


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


Re: [ovs-dev] [PATCH 07/13] log: New functions for replacing a log's contents.

2017-12-22 Thread Justin Pettit


> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> These functions will acquire users in future commits.
> 
> This new functionality made the internal state machine of the log hard
> enough to understand that I thought that it was best to make it explicit
> with a 'state' variable, so this commit introduces one.
> 
> This commit duplicates code and logic from ovsdb_rename() and
> ovsdb_compact() in ovsdb/file.c.  A future commit removes this duplication.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 06/13] log: Make reading in writing mode less of an error.

2017-12-22 Thread Justin Pettit

> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> Clients are intended to use the OVSDB log code by reading zero or more
> records and then writing zero or more records, but not reading after any
> write has occurred.  Until now, ovsdb_log_read() has signaled the latter
> behavior as an error.  Upcoming changes to OVSDB are going to make it an
> expected behavior in some cases, so this commit changes it so that it
> just becomes an empty read.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 05/13] log: Add new open mode OVSDB_LOG_CREATE_EXCL.

2017-12-22 Thread Justin Pettit


> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> Until now, OVSDB_LOG_CREATE implied EXCL, but this commit breaks them
> apart.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 04/13] log: Log write errors.

2017-12-22 Thread Justin Pettit


> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> This saves all the callers from logging them separately.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin




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


Re: [ovs-dev] [PATCH 03/13] log: Make json parameter to ovsdb_log_write() const.

2017-12-22 Thread Justin Pettit

> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 02/13] log: Require log entries to be JSON objects.

2017-12-22 Thread Justin Pettit

> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> The current and upcoming users of the OVSDB logging module only use
> JSON objects as records.  This commit simplifies the users slightly
> by allowing them to always assume that the records are JSON objects.
> 
> Unfortunately this resulted in a large number of updates to tests,
> which didn't always use JSON objects.
> 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 01/13] log: Allow client to specify magic.

2017-12-22 Thread Justin Pettit


> On Dec 7, 2017, at 4:12 PM, Ben Pfaff  wrote:
> 
> diff --git a/ovsdb/log.c b/ovsdb/log.c
> index 380f5e93d464..e6f66e668fe5 100644
> --- a/ovsdb/log.c
> +++ b/ovsdb/log.c
> ...
> @@ -53,12 +54,17 @@ struct ovsdb_log {
>  * the new log into '*filep' and returns NULL; otherwise returns NULL and
>  * stores NULL into '*filep'.
>  *
> + * 'magic' is a short text string put at the beginning of every record and 
> used
> + * to distinguish one kind of log file from another.  For a conventional 
> OVSDB
> + * log file, use OVSDB_MAGIC.

Since 'magic' is a string, it might be good to be explicit that "OVSDB_MAGIC" 
is a #define.

> struct ovsdb_error *
> -ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode,
> +ovsdb_log_open(const char *name, const char *magic,
> +   enum ovsdb_log_open_mode open_mode,
>int locking, struct ovsdb_log **filep)
> {
> struct lockfile *lockfile;
> @@ -118,10 +124,30 @@ ovsdb_log_open(const char *name, enum 
> ovsdb_log_open_mode open_mode,
> goto error_unlock;
> }
> 
> +if (!fstat(fd, )) {
> +if (s.st_size == 0) {
> +/* It's (probably) a new file so fsync() its parent directory to
> + * ensure that its directory entry is committed to disk. */
> +fsync_parent_dir(name);
> +} else if (s.st_size >= strlen(magic) && S_ISREG(s.st_mode)) {
> +/* Try to read the magic from the first log record.  If it's not
> + * the magic we expect, this is the wrong kind of file, so reject
> + * it immediately. */
> +size_t magic_len = strlen(magic);
> +char *buf = xzalloc(magic_len + 1);
> +bool err = (read(fd, buf, magic_len) == magic_len
> +&& strcmp(buf, magic));

It's probably hard to do much about it at this point, but if the new magic is a 
substring of existing one (e.g., just "OVSDB"), then this will not catch it.  
Probably not a big deal in practice, though.

> @@ -132,6 +158,7 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode 
> open_mode,
> 
> file = xmalloc(sizeof *file);
> file->name = xstrdup(name);
> +file->magic = xstrdup(magic);

I don't think this gets freed.

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH v2] openvswitch: Trim off padding before L3+ netfilter processing

2017-12-22 Thread Ed Swierk via dev
On Fri, Dec 22, 2017 at 3:31 PM, Pravin Shelar  wrote:
> On Thu, Dec 21, 2017 at 7:17 AM, Ed Swierk  wrote:
>> IPv4 and IPv6 packets may arrive with lower-layer padding that is not
>> included in the L3 length. For example, a short IPv4 packet may have
>> up to 6 bytes of padding following the IP payload when received on an
>> Ethernet device. In the normal IPv4 receive path, ip_rcv() trims the
>> packet to ip_hdr->tot_len before invoking netfilter hooks (including
>> conntrack and nat).
>>
>> In the IPv6 receive path, ip6_rcv() does the same using
>> ipv6_hdr->payload_len. Similarly in the br_netfilter receive path,
>> br_validate_ipv4() and br_validate_ipv6() trim the packet to the L3
>> length before invoking NF_INET_PRE_ROUTING hooks.
>>
>> In the OVS conntrack receive path, ovs_ct_execute() pulls the skb to
>> the L3 header but does not trim it to the L3 length before calling
>> nf_conntrack_in(NF_INET_PRE_ROUTING). When nf_conntrack_proto_tcp
>> encounters a packet with lower-layer padding, nf_checksum() fails and
>> logs "nf_ct_tcp: bad TCP checksum". While extra zero bytes don't
>> affect the checksum, the length in the IP pseudoheader does. That
>> length is based on skb->len, and without trimming, it doesn't match
>> the length the sender used when computing the checksum.
>>
>> The assumption throughout nf_conntrack and nf_nat is that skb->len
>> reflects the length of the L3 header and payload, so there is no need
>> to refer back to ip_hdr->tot_len or ipv6_hdr->payload_len.
>>
>> This change brings OVS into line with other netfilter users, trimming
>> IPv4 and IPv6 packets prior to L3+ netfilter processing.
>>
>> Signed-off-by: Ed Swierk 
>> ---
>> v2:
>> - Trim packet in nat receive path as well as conntrack
>> - Free skb on error
>> ---
>>  net/openvswitch/conntrack.c | 34 ++
>>  1 file changed, 34 insertions(+)
>>
>> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
>> index b27c5c6..1bdc78f 100644
>> --- a/net/openvswitch/conntrack.c
>> +++ b/net/openvswitch/conntrack.c
>> @@ -703,6 +703,33 @@ static bool skb_nfct_cached(struct net *net,
>> return ct_executed;
>>  }
>>
>> +/* Trim the skb to the L3 length. Assumes the skb is already pulled to
>> + * the L3 header. The skb is freed on error.
>> + */
>> +static int skb_trim_l3(struct sk_buff *skb)
>> +{
>> +   unsigned int nh_len;
>> +   int err;
>> +
>> +   switch (skb->protocol) {
>> +   case htons(ETH_P_IP):
>> +   nh_len = ntohs(ip_hdr(skb)->tot_len);
>> +   break;
>> +   case htons(ETH_P_IPV6):
>> +   nh_len = ntohs(ipv6_hdr(skb)->payload_len)
>> +   + sizeof(struct ipv6hdr);
>> +   break;
>> +   default:
>> +   nh_len = skb->len;
>> +   }
>> +
>> +   err = pskb_trim_rcsum(skb, nh_len);
>> +   if (err)
> This should is unlikely.

I'll add unlikely().

>> +   kfree_skb(skb);
>> +
>> +   return err;
>> +}
>> +
> This looks like a generic function, it probably does not belong to OVS
> code base.

Indeed. I'll move it to skbuff.c, unless you have a better idea.

>>  #ifdef CONFIG_NF_NAT_NEEDED
>>  /* Modelled after nf_nat_ipv[46]_fn().
>>   * range is only used for new, uninitialized NAT state.
>> @@ -715,8 +742,12 @@ static int ovs_ct_nat_execute(struct sk_buff *skb, 
>> struct nf_conn *ct,
>>  {
>> int hooknum, nh_off, err = NF_ACCEPT;
>>
>> +   /* The nat module expects to be working at L3. */
>> nh_off = skb_network_offset(skb);
>> skb_pull_rcsum(skb, nh_off);
>> +   err = skb_trim_l3(skb);
>> +   if (err)
>> +   return err;
>>
> ct-nat is executed within ct action, so I do not see why you you call
> skb-trim again from ovs_ct_nat_execute().
> ovs_ct_execute() trim should take care of the skb.

I see. Doesn't that mean that skb_pull_rcsum() is also unnecessary in
ovs_ct_nat_execute(), as ovs_ct_execute() has already pulled the skb
to the L3 header?

>> /* See HOOK2MANIP(). */
>> if (maniptype == NF_NAT_MANIP_SRC)
>> @@ -,6 +1142,9 @@ int ovs_ct_execute(struct net *net, struct sk_buff 
>> *skb,
>> /* The conntrack module expects to be working at L3. */
>> nh_ofs = skb_network_offset(skb);
>> skb_pull_rcsum(skb, nh_ofs);
>> +   err = skb_trim_l3(skb);
>> +   if (err)
>> +   return err;
>>
>> if (key->ip.frag != OVS_FRAG_TYPE_NONE) {
>> err = handle_fragments(net, key, info->zone.id, skb);
>> --
>> 1.9.1
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v4 0/3] dpif-netdev: Detailed PMD performance metrics and supervision

2017-12-22 Thread Wang, Yipeng1
Hi, Jan

I know performance is not a concern for this patch that people can always turn 
it off.  I just run some p2p test with the stats on for a reference point. 
Basically  it introduces around 10 cycles overhead per packet. In real pvp use 
cases, the performance influence should be small.

flow-subtable cnt   Throughput influence
10k-1t  -3.51%
10k-3t  -3.33%
10k-5t  -2.84%
10k-10t -2.30%
10k-20t -3.32%
100k-1t -3.02%
100k-3t -2.83%
100k-5t -2.11%
100k-10t-1.33%
100k-20t-3.81%
1M-1t   -3.25%
1M-3t   -2.75%
1M-5t   -1.98%
1M-10t  -1.78%
1M-20t  -0.60%

Tested-by:  Yipeng Wang 

Thanks
Yipeng

>-Original Message-
>From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
>boun...@openvswitch.org] On Behalf Of Jan Scheurich
>Sent: Monday, December 18, 2017 6:14 AM
>To: d...@openvswitch.org
>Subject: [ovs-dev] [PATCH v4 0/3] dpif-netdev: Detailed PMD performance
>metrics and supervision
>
>The run-time performance of PMDs is often difficult to understand and
>trouble-shoot. The existing PMD statistics counters only provide a coarse
>grained average picture. At packet rates of several Mpps sporadic drops of
>packet bursts happen at sub-millisecond time scales and are impossible to
>capture and analyze with existing tools.
>
>This patch set refactors the existing PMD statistics into a dedicated submodule
>and collects a large number of important PMD performance metrics per PMD
>iteration, maintaining histograms and circular histories for iteration metrics
>and millisecond averages. To capture sporadic drop events, the patch set can
>be configured to monitor iterations for suspicious metrics and to log the
>neighborhood of such iterations for off-line analysis.
>
>The extra cost for the performance metric collection and the supervision has
>been measured to be in the order of 1% compared to the base commit in a
>PVP setup with L3 pipeline over VXLAN tunnels. For that reason the metrics
>collection is disabled by default and can be enabled at run-time through
>configuration.
>
>The first patch in the series fully includes the changes proposed in Darrel's
>earlier "[patch_v5 0/3] dpif-netdev: Fix and refactor pmd stats"
>(https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337686.html).
>
>
>v3->v4:
>Rebased to master (commit 4d0a31b)
>Reverting changes to struct dp_netdev_pmd_thread
>Make metrics collection configurable
>Several bugfixes
>
>v2->v3:
>Rebased to OVS master (commit 3728b3b)
>Non-trivial adaptation to struct dp_netdev_pmd_thread
>refactored in commit a807c157 (Bhanu)
>No other changes compared to v2
>
>v1 -> v2:
>Rebased to OVS master (commit 7468ec788)
>No other changes compared to v1
>
>
>Jan Scheurich (3):
>  dpif-netdev: Refactor PMD performance into dpif-netdev-perf
>  dpif-netdev: Detailed performance stats for PMDs
>  dpif-netdev: Detection and logging of suspicious PMD iterations
>
> lib/automake.mk|   2 +
> lib/dp-packet.h|   2 +
> lib/dpif-netdev-perf.c | 515
>+
> lib/dpif-netdev-perf.h | 322 +++
> lib/dpif-netdev.c  | 471 ++--
> lib/netdev-dpdk.c  |  23 ++-
> lib/netdev-dpdk.h  |  14 ++
> ofproto/ofproto-dpif.c |   3 +-
> tests/pmd.at   |  22 ++-
> 9 files changed, 1168 insertions(+), 206 deletions(-)
> create mode 100644 lib/dpif-netdev-perf.c
> create mode 100644 lib/dpif-netdev-perf.h
>
>--
>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 v2] ofproto: Delete all groups and meters when (un)configuring a controller.

2017-12-22 Thread Ben Pfaff
On Tue, Nov 07, 2017 at 07:04:02PM -0800, Ben Pfaff wrote:
> Open vSwitch has always deleted all flows from the flow table whenever a
> controller is configured or whenever all the controllers are unconfigured.
> After this commit, OVS additionally deletes all OpenFlow groups and meters.
> 
> Suggested-by: Periyasamy Palanisamy 
> Suggested-by: Jan Scheurich 
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2: Clear the meter table too (thanks Jan).

Anyone want to review this?

Thanks,

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


Re: [ovs-dev] [PATCH] [RFC] coding-style: Remove caveats about mixing statements and declarations.

2017-12-22 Thread Ben Pfaff
On Wed, Dec 20, 2017 at 03:04:48PM -0800, Justin Pettit wrote:
> 
> 
> > On Nov 28, 2017, at 4:42 PM, Ben Pfaff  wrote:
> > 
> > OVS practice has evolved over time to mix code and data more freely.
> > 
> > See also:
> > https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341032.html
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] how to use ofctl/packet-out command???

2017-12-22 Thread Ben Pfaff
On Fri, Dec 22, 2017 at 09:04:41AM +, lin huang wrote:
>  I want to redirect packet-out message to a specific port. Is there 
> an example for me??
> Or someone who could teach me to use this feature??

To output to a port, use that port number as the action.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovs-vsctl: show DPDK version

2017-12-22 Thread Ben Pfaff
On Fri, Dec 22, 2017 at 11:53:43AM +0100, Matteo Croce wrote:
> Show DPDK version if Open vSwitch is compiled with DPDK support.
> Add a `dpdk_version` column in the datamodel and also edit ovs-dev.py to
> populate the field with the right value.
> 
> Signed-off-by: Matteo Croce 

I could see this being useful.  Thank you for the patch.

But nothing here populates the new column, except in the narrow case of
a developer running ovs-vswitchd himself using the ovs-dev.py script.
I guess that ovs-ctl or ovs-vswitchd itself should populate it.

Since this adds a new column to the schema, it should increment the
middle part of the version number, instead of the final part.

Thanks,

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


Re: [ovs-dev] [PATCH] sparse: Add guards to prevent FreeBSD-incompatible #include order.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 04:50:28PM -0800, Justin Pettit wrote:
> 
> > On Nov 6, 2017, at 2:42 PM, Ben Pfaff  wrote:
> 
> > diff --git a/include/sparse/arpa/inet.h b/include/sparse/arpa/inet.h
> > index dd64e61764e8..3634b230c8bb 100644
> > --- a/include/sparse/arpa/inet.h
> > +++ b/include/sparse/arpa/inet.h
> > @@ -18,4 +18,8 @@
> > #error "Use this header only with sparse.  It is not a correct 
> > implementation."
> > #endif
> > 
> > +#ifndef NETINET_IN_H_INCLUDED
> > +#error "Must include  before  for FreeBSD 
> > support"
> > +#endif
> > +
> > #include 
> 
> This method was clever.

Thanks!

> > diff --git a/lib/lldp/lldpd-structs.h b/lib/lldp/lldpd-structs.h
> > index 15e5ce8fa75d..6a3ffb8d33f0 100644
> > --- a/lib/lldp/lldpd-structs.h
> > +++ b/lib/lldp/lldpd-structs.h
> > @@ -20,11 +20,9 @@
> > #define _LLDPD_STRUCTS_H
> > 
> > #include 
> > -#ifndef _WIN32
> > +#include 
> > #include 
> > -#endif
> 
> Based on other changes, I assume including "" on Windows is 
> fine.  I just wanted to make sure, since it seems like there were some hoops 
> jumped for Windows.

I think it's OK.

> Acked-by: Justin Pettit 

Thank you for the review.  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] Información actualizada a su alcance

2017-12-22 Thread Criptomonedas
Nuevos esquemas tecnológicos para la conservación y transmisión de valor 

Criptomonedas: innovación, aplicación e impacto en el sector financiero
19 de enero- Lic. Jonathan H. Stahl Ducker - 9am-3pm

No puede perderse la oportunidad de acudir a este seminario en el que conocerá 
el sistema financiero actual y sus distintas áreas de oportunidad, analizará 
las características y aplicaciones de nuevos protocolos para la conservación, 
distribución de “dinero” y medios de pago. También aprenderá sobre la 
aplicación de estos nuevos protocolos de comunicación, a través del estudio de 
casos prácticos que pondrán de manifiesto el aspecto financiero, regulatorio y 
técnico de distintos proyectos. 

BENEFICIOS DE ASISTIR: 

- Aprenderá las generalidades del Sistema Financiero Mexicano. 
- Dominará el lenguaje utilizado en el ecosistema de las criptomonedas. 
- Analizará casos y experiencias internacionales. 
- Comprenderá el estado jurídico y regulatorio nacional e internacional. 
- Obtendrá herramientas y modelos de análisis. 

¿Requiere la información a la Brevedad? responda este email con la palabra: 
Criptomonedas + nombre - teléfono - correo.


centro telefónico:018002120744


 


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


[ovs-dev] [PATCH 2/3] Conntrack: Add initial support for new SIP Alg.

2017-12-22 Thread Tiago Lam
Handle_sip, in conntrack.c, is the new function handler that handles the
SIP messages received in the userspace conntrack (detected in
process_one).  It parses the SIP messages (and SDPs) using the newly
introduced SIP API (conntrack-sip.h) and accordingly creates the
expectations necessary to let the RTP traffic through.
To identify the new SIP Alg, a new IPPROTO_SIP has also been introduced,
which maps to port 5060, the SIP assigned port.

As of now, this preliminary support mainly handles the following happy
case, exchanged between a UAC and UAS:
- UAC > UAS | SIP INVITE (SDP offer);
- UAC < UAS | SIP 200(SDP answer);
- UAC > UAS | SIP ACK;
- UAC > UAS | SIP BYE;
- UAC < UAS | SIP 200;
It also assumes the SIP messages are coming over TCP and IPv4 (although
SIP's main usage is over UDP).

This integration has been tested manually by setting the following
flows, which let's tcp traffic from port 1 to flow to port 0, but only
+ESTABLISHED or +RELATED traffic on the opposite direction (from port 0
to port 1). It also allows UDP +RELATED to pass through in both
directions (which is needed for the RTP packets):
- table=0,priority=1,action=drop
- table=0,priority=10,arp,action=normal
- table=0,priority=10,icmp,action=normal
- table=0,priority=100,in_port=1,tcp,action=ct(alg=sip,commit),0
- table=0,priority=100,in_port=0,tcp,action=ct(table=1)
- table=0,priority=100,in_port=1,udp,action=ct(table=2)
- table=0,priority=100,in_port=0,udp,action=ct(table=2)
- table=1,in_port=0,tcp,ct_state=+trk+est,action=1
- table=1,in_port=0,tcp,ct_state=+trk+rel,action=1
- table=2,in_port=0,udp,ct_state=+rel,action=1
- table=2,in_port=1,udp,ct_state=+rel,action=0

Signed-off-by: Tiago Lam 
---
 include/openvswitch/ofp-actions.h |   4 +
 lib/conntrack-private.h   |   2 +
 lib/conntrack-sip.c   |  14 +++
 lib/conntrack-sip.h   |  11 ++
 lib/conntrack.c   | 228 +-
 lib/ofp-parse.c   |   5 +
 ofproto/ofproto-dpif-xlate.c  |   3 +
 7 files changed, 266 insertions(+), 1 deletion(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 25f61ef93..fccd3a61c 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -604,6 +604,10 @@ enum nx_conntrack_flags {
 #define IPPORT_TFTP  69
 #endif
 
+#if !defined(IPPORT_SIP)
+#define IPPORT_SIP  5060
+#endif
+
 /* OFPACT_CT.
  *
  * Used for NXAST_CT. */
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index ac0198f34..5205252d0 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -105,6 +105,8 @@ struct conn {
 uint8_t conn_type;
 /* TCP sequence skew due to NATTing of FTP control messages. */
 uint8_t seq_skew_dir;
+/* XXX: Move SIP specific state to a more appropriate place. */
+struct sip_state *sip_state;
 /* True if alg data connection. */
 uint8_t alg_related;
 };
diff --git a/lib/conntrack-sip.c b/lib/conntrack-sip.c
index 7239bdad6..4c850b5c0 100644
--- a/lib/conntrack-sip.c
+++ b/lib/conntrack-sip.c
@@ -22,6 +22,20 @@
 #include "dp-packet.h"
 #include "util.h"
 
+void free_sip_state(struct sip_state *sip_state) {
+free(sip_state->peer[0].sdp);
+free(sip_state->peer[1].sdp);
+free(sip_state);
+}
+
+void sip_state_init(struct sip_state *sip_state) {
+sip_state->peer[0].sdp = NULL;
+sip_state->peer[0].was_bye = false;
+
+sip_state->peer[1].sdp = NULL;
+sip_state->peer[1].was_bye = false;
+}
+
 enum sip_mthd sip_mthd_to_enum(char *mthd) {
 if (mthd == NULL) {
 return INVALID;
diff --git a/lib/conntrack-sip.h b/lib/conntrack-sip.h
index 134f35f3a..37abaad10 100644
--- a/lib/conntrack-sip.h
+++ b/lib/conntrack-sip.h
@@ -59,6 +59,15 @@ struct sip_strt_ln {
 enum sip_strt_type type;
 };
 
+struct sip_peer {
+struct sip_sdp *sdp;
+bool was_bye;
+};
+
+struct sip_state {
+struct sip_peer peer[2];
+};
+
 enum sip_mthd sip_mthd_to_enum(char *sip_mthd);
 int sip_skip_to_body(char **sip_hdr, size_t *len);
 uint32_t sip_body_len(char *sip_hdr);
@@ -68,6 +77,8 @@ bool sip_ln_cpy_nxt_word(char **sip, size_t *len, char **dst);
 bool is_valid_status(uint32_t status);
 void sip_ln_skip(char **sdp, size_t *len);
 
+void free_sip_state(struct sip_state *sip_state);
+void sip_state_init(struct sip_state *sip_state);
 char * sdp_get_origin(char *orig, size_t len);
 char * sdp_get_conn(char *conn, size_t len);
 char * sdp_get_media(char *media, size_t len);
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 6d078f5a8..65d10dd08 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -25,6 +25,7 @@
 #include "bitmap.h"
 #include "conntrack.h"
 #include "conntrack-private.h"
+#include "conntrack-sip.h"
 #include "coverage.h"
 #include "csum.h"
 #include "ct-dpif.h"
@@ -40,6 +41,7 @@
 #include "random.h"
 #include "timeval.h"
 
+
 VLOG_DEFINE_THIS_MODULE(conntrack);
 
 

[ovs-dev] [PATCH 3/3] Conntrack: Support asymmetric RTP port for SIP.

2017-12-22 Thread Tiago Lam
Previously, when creating the expecations, SIP Alg would assume the RTP
packets are sent from the same IP address + port which had been
announced in the SDP offer + answer. As specified in rfc4961, this might
not be the case, as the RTP packets might be sent from different IP
addresses and / or port.

This commit adds extra logic, by creating two expectations, one in each
direction, to support the "port case", i.e., the RTP packets are sent
from a different port than the one the host is listenning on.

Signed-off-by: Tiago Lam 
---
 lib/conntrack.c | 76 ++---
 1 file changed, 51 insertions(+), 25 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 65d10dd08..0db3447f2 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2610,32 +2610,31 @@ expectation_lookup(struct hmap *alg_expectations,
 
 static void
 sip_expectation_create(struct conntrack *ct,
-   const ovs_be32 offer_addr,
-   const ovs_be32 answer_addr,
-   const ovs_be16 offer_port,
+   const ovs_be32 src_addr,
+   const ovs_be32 dst_addr,
+   const ovs_be16 dst_port,
const long long now,
const struct conn *master_conn)
 {
-/* Set src address coming from answer SDP 'c' */
-struct ct_addr src_addr;
-memset(_addr, 0, sizeof src_addr);
-src_addr.ipv4_aligned = answer_addr;
-/* Set dst address coming from offer SDP 'c' */
-struct ct_addr dst_addr;
-memset(_addr, 0, sizeof dst_addr);
-dst_addr.ipv4_aligned = offer_addr;
+/* Set src address (from SDP's 'c') */
+struct ct_addr ct_src_addr;
+memset(_src_addr, 0, sizeof ct_src_addr);
+ct_src_addr.ipv4_aligned = src_addr;
+/* Set dst address (from SDP's 'c') */
+struct ct_addr ct_dst_addr;
+memset(_dst_addr, 0, sizeof ct_dst_addr);
+ct_dst_addr.ipv4_aligned = dst_addr;
 
 struct alg_exp_node *alg_exp_node =
 xzalloc(sizeof *alg_exp_node);
 alg_exp_node->key.dl_type = master_conn->key.dl_type;
-/* nw_proto might won't be the same as SIP, since RTP is over UDP - hence
- * set it to UDP explicitly */
+/* RTP is over UDP - hence set nw_proto to UDP explicitly */
 alg_exp_node->key.nw_proto = IPPROTO_UDP;
 alg_exp_node->key.zone = master_conn->key.zone;
-alg_exp_node->key.src.addr = src_addr;
-alg_exp_node->key.dst.addr = dst_addr;
+alg_exp_node->key.src.addr = ct_src_addr;
+alg_exp_node->key.dst.addr = ct_dst_addr;
 alg_exp_node->key.src.port = ALG_WC_SRC_PORT;
-alg_exp_node->key.dst.port = offer_port;
+alg_exp_node->key.dst.port = dst_port;
 alg_exp_node->master_mark = master_conn->mark;
 alg_exp_node->master_label = master_conn->label;
 alg_exp_node->master_key = master_conn->key;
@@ -2664,6 +2663,39 @@ sip_expectation_create(struct conntrack *ct,
 ct_rwlock_unlock(>resources_lock);
 }
 
+/* Setups two expectations for the RTP connections, one in each direction.
+ * Example: If an SDP answer announces it is listenning at IP address 10.0.1.
+ * 10:6000 and an the SDP offer at 10.0.2.10:6000, then:
+ * - Sets a UDP expectation going from 10.0.1.10:0 to 10.0.2.10:6000:
+ * - Sets another UDP expectation going from 10.0.2.10:0 to 10.0.1.10:6000;
+ *
+ * Thus, this allows traffic from a different port than the one the host is
+ * listenning to (the one it announced in the SDP).
+ *
+ * XXX According to rfc4961, hosts might send RTP packets from a different IP
+ * address and / or port (i.e. asymmetrically). Since this function supports
+ * the sending of packets from a different port, consider supporting the
+ * sending from a different IP address.
+ */
+static void
+sip_expectations_setup(struct conntrack *ct,
+   const struct sip_sdp *offer_sdp,
+   const struct sip_sdp *answer_sdp,
+   const long long now,
+   const struct conn *conn) {
+ovs_be32 offer_addr = htonl(offer_sdp->conn);
+ovs_be16 offer_port = htons(offer_sdp->port);
+ovs_be32 answer_addr = htonl(answer_sdp->conn);
+ovs_be16 answer_port = htons(answer_sdp->port);
+
+/* Set expectation from SDP answer -> SDP offer */
+sip_expectation_create(ct, answer_addr, offer_addr, offer_port, now,
+   conn);
+/* Set expectation from SDP offer -> SDP answer */
+sip_expectation_create(ct, offer_addr, answer_addr, answer_port, now,
+   conn);
+}
+
 static void
 expectation_create(struct conntrack *ct,
ovs_be16 dst_port,
@@ -3383,9 +3415,6 @@ handle_sip(struct conntrack *ct,
 
 /* Check if this SIP reply has an SDP. */
 struct sip_sdp *sdp = sip_parse_sdp(msg_bdy, sip_len);
-ovs_be32 offer_addr;
-ovs_be32 offer_port;
-ovs_be32 

[ovs-dev] [PATCH 1/3] Conntrack: Add new API for future SIP Alg.

2017-12-22 Thread Tiago Lam
The new API (in conntrack-sip.h and conntrack-sip.c) defines preliminary
functions, helpers and strucures to move through and parse SIP messages,
including SDP in the message bodies, if present.

Note that this is still a preliminary version of the API, and future
work is still needed to improve the SIP and SDP handling. When parsing
an SDP, for example, the order of the 'm', 'c' and 'o' sections is not
enforced, and only the ports and IP addresses are stored at the moment.

This API will be used by a future commit in order to implement the new
SIP Alg in userspace conntrack.

Signed-off-by: Tiago Lam 
---
 lib/automake.mk |   2 +
 lib/conntrack-sip.c | 477 
 lib/conntrack-sip.h | 112 
 3 files changed, 591 insertions(+)
 create mode 100644 lib/conntrack-sip.c
 create mode 100644 lib/conntrack-sip.h

diff --git a/lib/automake.mk b/lib/automake.mk
index effe5b5c2..0dc068b9e 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -51,6 +51,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/connectivity.h \
lib/conntrack-icmp.c \
lib/conntrack-private.h \
+   lib/conntrack-sip.c \
+   lib/conntrack-sip.h \
lib/conntrack-tcp.c \
lib/conntrack-other.c \
lib/conntrack.c \
diff --git a/lib/conntrack-sip.c b/lib/conntrack-sip.c
new file mode 100644
index 0..7239bdad6
--- /dev/null
+++ b/lib/conntrack-sip.c
@@ -0,0 +1,477 @@
+/*
+ * Copyright (c) 2017 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include 
+#include 
+
+#include "conntrack-sip.h"
+#include "dp-packet.h"
+#include "util.h"
+
+enum sip_mthd sip_mthd_to_enum(char *mthd) {
+if (mthd == NULL) {
+return INVALID;
+}
+
+if (!strcmp(mthd, "INVITE")) {
+return INVITE;
+} else if (!strcmp(mthd, "REGISTER")) {
+return REGISTER;
+} else if (!strcmp(mthd, "ACK")) {
+return ACK;
+} else if (!strcmp(mthd, "CANCEL")) {
+return CANCEL;
+} else if (!strcmp(mthd, "BYE")) {
+return BYE;
+} else if (!strcmp(mthd, "OPTIONS")) {
+return OPTIONS;
+}
+
+return INVALID;
+}
+/* A valid SIP status falls in between the following ranges (rfc3261):
+ * - 1xx: Provisional;
+ * - 2xx: Success;
+ * - 3xx: Redirection;
+ * - 4xx: Client Error;
+ * - 5xx: Server Error;
+ * - 6xx: Global Failure.
+ *
+ * Returns false to anything that falls outside of the ranges above, and true
+ * otherwise. */
+bool is_valid_status(uint32_t status) {
+if ((status >= 100 && status < 200) ||
+(status >= 200 && status < 300) ||
+(status >= 300 && status < 400) ||
+(status >= 400 && status < 500) ||
+(status >= 500 && status < 600) ||
+(status >= 600 && status < 700)) {
+return true;
+}
+
+return false;
+}
+
+/* Find and validate the beginning of the CRLF ending, returning a pointer to
+ * the found CR, or NULL otherwise. */
+char * sip_ln_find_crlf(char *sip, size_t len) {
+if (sip == NULL) {
+return NULL;
+}
+
+char *cr = memchr(sip, '\r', len);
+if (cr == NULL) {
+return NULL;
+}
+char *lf = memchr(sip, '\n', len);
+if (lf == NULL) {
+return NULL;
+}
+
+if (lf - cr != 1) {
+return NULL;
+}
+
+return cr;
+}
+
+/* Returns the size of the line until the next CRLF ending (not inclusive). If
+ * CRLF is not found it returns -1. */
+size_t sip_ln_len(char *sip, size_t len) {
+char *cr = sip_ln_find_crlf(sip, len);
+if (cr == NULL) {
+return -1;
+}
+
+return cr - sip;
+}
+
+/* Ignores the next CRLF ending and moves to CRLF + 1. After returning, 'sip'
+ * is modified to point to the next line after CRLF (CRLF + 1), and 'len' has
+ * been updated accordingly. If CRLF is not found, 'sip' and 'len' are
+ * untouched */
+void sip_ln_skip(char **sip, size_t *len) {
+char *cr = sip_ln_find_crlf(*sip, *len);
+if (cr == NULL) {
+return;
+}
+size_t sz = cr - *sip;
+*sip += sz + 2;
+*len -= sz + 2;
+}
+
+/* Iterate through each word in the line at 'sip', where each word is divided
+ * by the space char ' ', and store the resulting word in 'dst', if not NULL.
+ *
+ * After returning 'sip' is pointing to the next word in the line, and 'len'
+ * has been updated accordingly. */
+bool sip_ln_cpy_nxt_word(char **sip, size_t *len, char **dst) {
+char 

[ovs-dev] [PATCH 0/3] Initial support for new SIP Alg.

2017-12-22 Thread Tiago Lam
This patch-set is an initial approach at implementing the new SIP Alg,
mentioned by Aaron at [1].

I'm mostly interested in getting to know your thoughts of how this is
headed. There are a couple of points that are worth bringing up:
- As mentioned in patches 1/3 and 2/3, this is still a preliminary
  implementation, and some work will be needed to move away from some
  assuptions, like assuming the SIP traffic is always going over IPv4
  and TCP;
- At the moment, the sip state is being stored in the conn struct. I
  followed the example of seq_skew_dir here, which is also stored there,
  but realise this is not ideal. It seems storing it somewhere agnostic
  will be ideal in the future, to avoid polluting that struct with
  different Alg's details;
- The SIP helpers functions and structures are in conntrack-sip.h and
  conntrack-sip.c. This can create confusion when comparing to
  conntrack-tcp.c and other protocols since SIP is an Alg and is at a
  different level.

With regards to testing, for now, this has been tested manually, by
setting up the flows mentioned in patch 2/3 and having two VMs connected
to OvS, both using SIPp to simulate real traffic both ways. I'm going to
have a look at how this can be automated and added to
tests/system-traffic.at, together with the rest of the already existing
tests.

[1] [CONNTRACK] Discussions at OvS 2017:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341089.html

Tiago Lam (3):
  Conntrack: Add new API for future SIP Alg.
  Conntrack: Add initial support for new SIP Alg.
  Conntrack: Support asymmetric RTP port for SIP.

 include/openvswitch/ofp-actions.h |   4 +
 lib/automake.mk   |   2 +
 lib/conntrack-private.h   |   2 +
 lib/conntrack-sip.c   | 491 ++
 lib/conntrack-sip.h   | 123 ++
 lib/conntrack.c   | 254 +++-
 lib/ofp-parse.c   |   5 +
 ofproto/ofproto-dpif-xlate.c  |   3 +
 8 files changed, 883 insertions(+), 1 deletion(-)
 create mode 100644 lib/conntrack-sip.c
 create mode 100644 lib/conntrack-sip.h

-- 
2.14.3

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


Re: [ovs-dev] [PATCH] table: Add --max-column-width option.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 03:49:17PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 8, 2017, at 12:54 PM, Ben Pfaff  wrote:
> > 
> > This can make it easier to read tables that contain wide data in some
> > columns.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovsdb-client: Show even constraint-breaking data in "dump" output.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 04:22:37PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 8, 2017, at 12:37 PM, Ben Pfaff  wrote:
> > 
> > The ovsdb-client "dump" command is a fairly low-level tool that can be
> > used, among other purposes, to debug the OVSDB protocol.  It's better if
> > it just prints what the server sends without being too judgmental about it.
> > Thus, we might as well ignore constraints for the purpose of dumping
> > tables.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovsdb-server: Forbid user-specified databases with reserved names.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 04:18:05PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 8, 2017, at 12:53 PM, Ben Pfaff  wrote:
> > 
> > +if (argc > 2 && argv[1][0] == '_') {
> > +unixctl_command_reply_error(conn, "cannot compact built-in 
> > databases");
> > +return;
> > +}
> 
> This error condition seems a little odd to me.  I think it will only provide 
> this warning if multiple databases are specified, and only complain if the 
> first one is built-in.

You're right, there's a bug here, and I didn't test it.  Shame on me.

(However, the command supports at most one named database on the command
line, which is part of the reason the test is somewhat odd.)

> Acked-by: Justin Pettit 

I fixed the bug and added a test by folding in the following, and
applied this to master.   Thank you for the review and pointing out the
bug.

--8<--cut here-->8--

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index ff2ada76df4b..7f2d19ef568b 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -1136,13 +1136,14 @@ static void
 ovsdb_server_compact(struct unixctl_conn *conn, int argc,
  const char *argv[], void *dbs_)
 {
+const char *db_name = argc < 2 ? NULL : argv[1];
 struct shash *all_dbs = dbs_;
 struct ds reply;
 struct db *db;
 struct shash_node *node;
 int n = 0;
 
-if (argc > 2 && argv[1][0] == '_') {
+if (db_name && db_name[0] == '_') {
 unixctl_command_reply_error(conn, "cannot compact built-in databases");
 return;
 }
@@ -1150,9 +1151,9 @@ ovsdb_server_compact(struct unixctl_conn *conn, int argc,
 ds_init();
 SHASH_FOR_EACH(node, all_dbs) {
 db = node->data;
-if (argc < 2
-? node->name[0] != '_'
-: !strcmp(argv[1], node->name)) {
+if (db_name
+? !strcmp(node->name, db_name)
+: node->name[0] != '_') {
 struct ovsdb_error *error;
 
 VLOG_INFO("compacting %s database by user request", node->name);
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index b830be0c1f4d..968356781604 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -689,6 +689,11 @@ _uuidname  number
 dnl Now compact the database in-place.
 AT_CHECK([[ovs-appctl -t ovsdb-server ovsdb-server/compact]],
   [0], [], [ignore], [test ! -e pid || kill `cat pid`])
+dnl Negative test.
+AT_CHECK([[ovs-appctl -t ovsdb-server ovsdb-server/compact _Server]],
+  [2], [], [cannot compact built-in databases
+ovs-appctl: ovsdb-server: server returned an error
+])
 dnl Make sure that "db" is still a symlink to dir/db instead of getting
 dnl replaced by a regular file, ditto for .db.~lock~.
 if test "$IS_WIN32" = "no"; then
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] odp-util: Use flexible sized buffer to hold Geneve options.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 04:30:05PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 4, 2017, at 11:51 AM, Ben Pfaff  wrote:
> > 
> > The 'mask' buffer in parse_odp_action() is supposed to always be big
> > enough:
> >/* 'mask' is big enough to hold any key. */
> > 
> > Geneve options can be really big and the comment was wrong.  In addition,
> > the user might supply more options than can really fit in any case, so
> > we might as well just use a stub.
> > 
> > Found by libfuzzer.
> > 
> > Reported-by: Bhargava Shastry 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master and backported as far as 2.4.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] netdev-tc-offloads: Use customary types for buffer.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 04:39:04PM -0800, Justin Pettit wrote:
> 
> 
> > On Nov 27, 2017, at 1:53 PM, Ben Pfaff  wrote:
> > 
> > This function uses local array set_buff[] to store Netlink attributes.
> > It declares set_buff as an array of character pointers, which is a strange
> > type for a buffer of non-character-pointer objects.  In OVS it is
> > customary to use an ofpbuf with a stub of uint64_t objecs (to ensure
> > proper alignment, otherwise uint8_t would be more usual).  This commit
> > changes to that more usual form.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] jsonrpc-server: Enforce uniqueness of monitor IDs.

2017-12-22 Thread Ben Pfaff
On Thu, Dec 21, 2017 at 04:54:28PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 21, 2017, at 4:30 PM, Ben Pfaff  wrote:
> > 
> > What do you think of this version?
> 
> Yes, it's much clearer to me.  Thanks!
> 
> > @@ -1458,10 +1466,12 @@ ovsdb_jsonrpc_monitor_cond_change(struct 
> > ovsdb_jsonrpc_session *s,
> > }
> > 
> > /* Change monitor id */
> > -hmap_remove(>monitors, >node);
> > -json_destroy(m->monitor_id);
> > -m->monitor_id = json_clone(params->u.array.elems[1]);
> > -hmap_insert(>monitors, >node, json_hash(m->monitor_id, 0));
> > +if (changing_id) {
> > +hmap_remove(>monitors, >node);
> > +json_destroy(m->monitor_id);
> > +m->monitor_id = json_clone(new_monitor_id);
> > +hmap_insert(>monitors, >node, json_hash(m->monitor_id, 0));
> > +}
> 
> It doesn't matter, but you can probably drop the comment, since the "if" 
> statement is pretty clear.
> 
> Acked-by: Justin Pettit 

Thanks, I removed the comment and applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v3 7/8] ovsdb-client: Add new "query" command.

2017-12-22 Thread Ben Pfaff
On Wed, Dec 20, 2017 at 02:45:36PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 13, 2017, at 3:10 PM, Ben Pfaff  wrote:
> > 
> > 
> > diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
> > index 30de9c536600..70529a1e6600 100644
> > --- a/ovsdb/ovsdb-client.1.in
> > +++ b/ovsdb/ovsdb-client.1.in
> > @@ -27,6 +27,8 @@ ovsdb\-client \- command-line interface to 
> > \fBovsdb-server\fR(1)
> > .IP "Data Management Commands:"
> > \fBovsdb\-client \fR[\fIoptions\fR] \fBtransact\fI \fR[\fIserver\fR] 
> > \fItransaction\fR
> > .br
> > +\fBovsdb\-client \fR[\fIoptions\fR] \fBquery\fI \fR[\fIserver\fR] 
> > \fItransaction\fR
> > +.br
> 
> Same comment about unnecessary "\f*".

Thanks, fixed.

> > @@ -137,6 +139,15 @@ which must be a JSON array appropriate for use as the 
> > \fBparams\fR to
> > a JSON-RPC \fBtransact\fR request, and prints the received reply on
> > stdout.
> > .
> > +.IP "\fBquery\fI \fR[\fIserver\fR] \fItransaction\fR"
> 
> Ditto.

Thanks, fixed.

> > +Connects to \fIserver\fR, sends it the specified \fItransaction\fR,
> > +which must be a JSON array appropriate for use as the \fBparams\fR to
> > +a JSON-RPC \fBtransact\fR request, and prints the received reply on
> > +stdout.  To ensure that the transaction does not modify the database,
> > +this command appends an \fBabort\fR operation to the set of operations
> > +included in \fItransaction\fR before sending it to the database, and
> > +then removes the \fBabort\fR result from the reply (if it is present).
> 
> It seems that the only difference between "transact" and "query" is that 
> "query" is read-only.  The man page descriptions are slightly different 
> between the commands, so it wasn't immediately obvious that was the only 
> difference.  It might be worth making it a bit more concrete--possibly even 
> by directly referencing "transact" from this description.

That is a good idea.  I added: "This commands acts like a read-only
version of \fBtransact\fR."

> Acked-by: Justin Pettit 

Thank you for the review.  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v5 0/5] OVS-DPDK flow offload with rte_flow

2017-12-22 Thread Chandran, Sugesh
Hi Yuanhan,

Thank you for working on this.
I will be reviewing this patch when I am back office after my Xmas/New Year 
vacation.
I do have some comments as well as the questions on how this proposal is going 
to work with the different hardwares, full acceleration, and other partial 
offload options. Lets have that conversation after the holidays and see whats 
the best way to get it work for most of the cases.





Regards
_Sugesh


> -Original Message-
> From: Finn Christensen [mailto:f...@napatech.com]
> Sent: Friday, December 22, 2017 11:29 AM
> To: Yuanhan Liu ; d...@openvswitch.org
> Cc: Darrell Ball ; Chandran, Sugesh
> ; Simon Horman
> ; Stokes, Ian 
> Subject: RE: [PATCH v5 0/5] OVS-DPDK flow offload with rte_flow
> 
> Hi,
> 
> The addition of the offload thread is an great addition, furthermore, RSS 
> action
> usage for target queues, works now well with our NICs, I just had to do some
> additional drive work, to make it all work.
> 
> The support for RSS removed the problem with rte-flow queue selection, but RSS
> in general added an additional challenge as now multiple pmd's may request the
> same flow offload (as Yuanhan pointed out). The solution has worked without
> issues for me. I have run tests with 1 and 2 queues in a PV and PVP setup 
> going
> over virtio.
> Yuanhan mentioned tests in a P2P setup with high throughput acceleration. I
> have focused on a PVP scenario, which, in addition, will crosses virtio, and 
> most
> importantly, has acceleration in Rx direction only (seen from VM). It still 
> gives
> fairly good performance improvements.
> 
> Here are my initial test results.
> 
> Percentage improvements:
> 1 RX/Tx queue:
> 1 megaflow - 10K flowsPV: 73%
> 10  megaflows - 100K flowsPVP: 50%PV: 56%
> 100 megaflows - 1M flowsPVP: 42%PV: 49%
> 1000 megaflows - 10M flowsPVP: 40%PV: 42%
> 
> With RSS: 2 Rx/Tx queue pairs:
> 1 megaflow - 10K flowsPV: 55%
> 10  megaflows - 100K flowsPVP: 36%PV: 38%
> 100 megaflows - 1M flowsPVP: 27%PV: 24%
> 1000 megaflows - 10M flowsPVP: 21%PV: 24%
> 
> PVP for 1 megaflow offload is omitted because my VM-app imposed a bottle-
> neck.
> 
> Regards,
> Finn
> 
> -Original Message-
> From: Yuanhan Liu [mailto:y...@fridaylinux.org]
> Sent: 20. december 2017 15:45
> To: d...@openvswitch.org
> Cc: Finn Christensen ; Darrell Ball
> ; Chandran Sugesh ;
> Simon Horman ; Ian Stokes
> ; Yuanhan Liu 
> Subject: [PATCH v5 0/5] OVS-DPDK flow offload with rte_flow
> 
> Hi,
> 
> Here is a joint work from Mellanox and Napatech, to enable the flow hw
> offload with the DPDK generic flow interface (rte_flow).
> 
> The basic idea is to associate the flow with a mark id (a unit32_t 
> number).
> Later, we then get the flow directly from the mark id, which could bypass
> some heavy CPU operations, including but not limiting to mini flow 
> extract,
> emc lookup, dpcls lookup, etc.
> 
> The association is done with CMAP in patch 1. The CPU workload bypassing
> is done in patch 2. The flow offload is done in patch 3, which mainly does
> two things:
> 
> - translate the ovs match to DPDK rte flow patterns
> - bind those patterns with a RSS + MARK action.
> 
> Patch 5 makes the offload work happen in another thread, for leaving the
> datapath as light as possible.
> 
> A PHY-PHY forwarding with 1000 mega flows (udp,tp_src=1000-1999) and 1
> million streams (tp_src=1000-1999, tp_dst=2000-2999) show more than
> 260% performance boost.
> 
> Note that it's disabled by default, which can be enabled by:
> 
> $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
> 
> 
> v5: - fixed an issue that it took too long if we do flow add/remove
>   repeatedly.
> - removed an unused mutex lock
> - turned most of the log level to DBG
> - rebased on top of the latest code
> 
> v4: - use RSS action instead of QUEUE action with MARK
> - make it work with multiple queue (see patch 1)
> - rebased on top of latest code
> 
> v3: - The mark and id association is done with array instead of CMAP.
> - Added a thread to do hw offload operations
> - Removed macros completely
> - dropped the patch to set FDIR_CONF, which is a workround some
>   Intel NICs.
> - Added a debug patch to show all flow patterns we have created.
> - Misc fixes
> 
> v2: - workaround the queue action issue
> - fixed the tcp_flags being skipped issue, which also fixed the
>   build warnings
> - fixed l2 patterns for Intel nic
> - Converted some macros to functions
> - did not hardcode the max 

[ovs-dev] [PATCH 3/3] rhel: add "force-reload-kmod" support in "ovs-systemd-reload"

2017-12-22 Thread Timothy Redaelli
Since you can't use "ovs-ctl force-reload-kmod" on Fedora/RHEL, due to
systemd dependencies, this commit adds the "force-reload-kmod" feature on
ovs-systemd-reload.

Signed-off-by: Timothy Redaelli 
---
 rhel/usr_share_openvswitch_scripts_ovs-systemd-reload | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload 
b/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload
index 5d2efc621..894df0427 100755
--- a/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload
+++ b/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload
@@ -40,6 +40,10 @@ add_managers() {
 :
 }
 
-restart
+if [ "$1" = "force-reload-kmod" ]; then
+force_reload_kmod
+else
+restart
+fi
 
 exit 0
-- 
2.14.3

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


[ovs-dev] [PATCH 2/3] rhel: use the functions in ovs-lib.in in ovs-systemd-reload

2017-12-22 Thread Timothy Redaelli
To avoid code duplication use the functions from ovs-lib.in

Signed-off-by: Timothy Redaelli 
---
 ...sr_share_openvswitch_scripts_ovs-systemd-reload | 37 ++
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload 
b/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload
index 3ac1a46c6..5d2efc621 100755
--- a/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload
+++ b/rhel/usr_share_openvswitch_scripts_ovs-systemd-reload
@@ -14,23 +14,32 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# Save flows
-bridges=$(ovs-vsctl -- --real list-br)
-flows=$(/usr/share/openvswitch/scripts/ovs-save save-flows $bridges)
+case $0 in
+*/*) dir0=`echo "$0" | sed 's,/[^/]*$,,'` ;;
+*) dir0=./ ;;
+esac
+. "$dir0/ovs-lib" || exit 1
 
-# Restart the database first, since a large database may take a
-# while to load, and we want to minimize forwarding disruption.
-systemctl --job-mode=ignore-dependencies restart ovsdb-server
+stop_ovsdb() {
+systemctl --job-mode=ignore-dependencies stop ovsdb-server
+}
 
-# Stop ovs-vswitchd.
-systemctl --job-mode=ignore-dependencies stop ovs-vswitchd
+start_ovsdb() {
+systemctl --job-mode=ignore-dependencies start ovsdb-server
+}
 
-# Start vswitchd by asking it to wait till flow restore is finished.
-ovs-vsctl --no-wait set open_vswitch . other_config:flow-restore-wait=true
-systemctl --job-mode=ignore-dependencies start ovs-vswitchd
+stop_forwarding() {
+systemctl --job-mode=ignore-dependencies stop ovs-vswitchd
+}
 
-# Restore saved flows and inform vswitchd that we are done.
-eval "$flows"
-ovs-vsctl --if-exists remove open_vswitch . other_config flow-restore-wait=true
+start_forwarding() {
+systemctl --job-mode=ignore-dependencies start ovs-vswitchd
+}
+
+add_managers() {
+:
+}
+
+restart
 
 exit 0
-- 
2.14.3

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


[ovs-dev] [PATCH 1/3] utilities: move some functions from ovs-ctl.in to ovs-lib.in

2017-12-22 Thread Timothy Redaelli
Move the functions related to "force-reload-kmod" and "restart" from
ovs-ctl.in to ovs-lib.in in order to permit other scripts to use them.

Signed-off-by: Timothy Redaelli 
---
 utilities/ovs-ctl.in | 173 ---
 utilities/ovs-lib.in | 173 +++
 2 files changed, 173 insertions(+), 173 deletions(-)

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index f1b01d1d3..1df56c4a5 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -63,10 +63,6 @@ insert_mod_if_required () {
 insert_mods
 }
 
-ovs_vsctl () {
-ovs-vsctl --no-wait "$@"
-}
-
 set_hostname () {
 # 'hostname -f' needs network connectivity to work.  So we should
 # call this only after ovs-vswitchd is running.
@@ -270,175 +266,6 @@ stop_forwarding () {
 fi
 }
 
-## - ##
-## force-reload-kmod ##
-## - ##
-
-internal_interfaces () {
-# Outputs a list of internal interfaces:
-#
-#   - There is an internal interface for every bridge, whether it
-# has an Interface record or not and whether the Interface
-# record's 'type' is properly set or not.
-#
-#   - There is an internal interface for each Interface record whose
-# 'type' is 'internal'.
-#
-# But ignore interfaces that don't really exist.
-for d in `(ovs_vsctl --bare \
--- --columns=name find Interface type=internal \
-   -- list-br) | sort -u`
-do
-if test -e "/sys/class/net/$d"; then
-   printf "%s " "$d"
-   fi
-done
-}
-
-ovs_save () {
-bridges=`ovs_vsctl -- --real list-br`
-if [ -n "${bridges}" ] && \
-"$datadir/scripts/ovs-save" "$1" ${bridges} > "$2"; then
-chmod +x "$2"
-return 0
-fi
-[ -z "${bridges}" ] && return 0
-}
-
-save_flows_if_required () {
-if test X"$DELETE_BRIDGES" != Xyes; then
-action "Saving flows" ovs_save save-flows "${script_flows}"
-fi
-}
-
-save_interfaces () {
-"$datadir/scripts/ovs-save" save-interfaces ${ifaces} \
-> "${script_interfaces}"
-}
-
-flow_restore_wait () {
-if test X"$OVS_VSWITCHD" = Xyes; then
-ovs_vsctl set open_vswitch . other_config:flow-restore-wait="true"
-fi
-}
-
-flow_restore_complete () {
-if test X"$OVS_VSWITCHD" = Xyes; then
-ovs_vsctl --if-exists remove open_vswitch . other_config \
-  flow-restore-wait="true"
-fi
-}
-
-restore_flows () {
-[ -x "${script_flows}" ] && \
-action "Restoring saved flows" "${script_flows}"
-}
-
-restore_interfaces () {
-[ ! -x "${script_interfaces}" ] && return 0
-action "Restoring interface configuration" "${script_interfaces}"
-rc=$?
-if test $rc = 0; then
-level=debug
-else
-level=err
-fi
-log="logger -p daemon.$level -t ovs-save"
-$log "interface restore script exited with status $rc:"
-$log -f "$script_interfaces"
-}
-
-init_restore_scripts () {
-script_interfaces=`mktemp`
-script_flows=`mktemp`
-trap 'rm -f "${script_interfaces}" "${script_flows}"' 0
-}
-
-force_reload_kmod () {
-
-if test X"$OVS_VSWITCHD" != Xyes; then
-log_failure_msg "Reloading of kmod without ovs-vswitchd is an error"
-exit 1
-fi
-
-ifaces=`internal_interfaces`
-action "Detected internal interfaces: $ifaces" true
-
-init_restore_scripts
-save_flows_if_required
-
-# Restart the database first, since a large database may take a
-# while to load, and we want to minimize forwarding disruption.
-stop_ovsdb
-start_ovsdb || return 1
-
-stop_forwarding
-
-if action "Saving interface configuration" save_interfaces; then
-:
-else
-log_warning_msg "Failed to save configuration, not replacing kernel 
module"
-start_forwarding
-add_managers
-exit 1
-fi
-chmod +x "$script_interfaces"
-
-for dp in `ovs-dpctl dump-dps`; do
-action "Removing datapath: $dp" ovs-dpctl del-dp "$dp"
-done
-
-for vport in `awk '/^vport_/ { print $1 }' /proc/modules`; do
-action "Removing $vport module" rmmod $vport
-done
-
-if test -e /sys/module/openvswitch; then
-action "Removing openvswitch module" rmmod openvswitch
-fi
-
-# Start vswitchd by asking it to wait till flow restore is finished.
-flow_restore_wait
-start_forwarding || return 1
-
-# Restore saved flows and inform vswitchd that we are done.
-restore_flows
-flow_restore_complete
-add_managers
-
-restore_interfaces
-
-"$datadir/scripts/ovs-check-dead-ifs"
-}
-
-## --- ##
-## restart ##
-## --- ##
-
-restart () {
-if daemon_is_running ovsdb-server && daemon_is_running ovs-vswitchd; then
-init_restore_scripts
-if test X"$OVS_VSWITCHD" = Xyes; then
-save_flows_if_required
-fi
-fi
-

[ovs-dev] [PATCH 0/3] rhel: Add force-reload-kmod support in ovs-systemd-reload

2017-12-22 Thread Timothy Redaelli
On Fedora / RHEL7 "ovs-ctl force-reload-mod" doesn't work due to systemd
dependencies so this series adds support for "force-reload-mod" inside the
rhel-only "ovs-systemd-reload" script.

Timothy Redaelli (3):
  utilities: move some functions from ovs-ctl.in to ovs-lib.in
  rhel: use the functions in ovs-lib.in in ovs-systemd-reload
  rhel: add "force-reload-kmod" support in "ovs-systemd-reload"

 ...sr_share_openvswitch_scripts_ovs-systemd-reload |  41 +++--
 utilities/ovs-ctl.in   | 173 -
 utilities/ovs-lib.in   | 173 +
 3 files changed, 200 insertions(+), 187 deletions(-)

-- 
2.14.3

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


[ovs-dev] [PATCH] tests: Kill the daemons at cleanup only if pidfile exists.

2017-12-22 Thread Jakub Sitnicki
Remembering the PIDs of started daemon processes and killing them during
cleanup can interfere with other tests.

When the tests are running in parallel (i.e., -jX was passed in
TESTSUITEFLAGS), PIDs of daemons that have terminated before the cleanup
are subject to reuse by processes, or threads, spawned in another test.

This means that, while executing its 'cleanup' script, one test can
accidentally send a SIGTERM to a process running as a part of another
test and influence its outcome.

Retrieve the PID of the process we intend to kill at the last possible
moment to narrow down the window of opportunity for interfering.

This approach has a downside that if the daemon's pidfile has
disappeared but the process has not terminated, we will not clean it up
at the end of the test.

Signed-off-by: Jakub Sitnicki 
---
 tests/ofproto-macros.at | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 64cba44..ec5c892 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -55,8 +55,8 @@ m4_define([PARSE_LISTENING_PORT],
 
 start_daemon () {
 "$@" -vconsole:off --detach --no-chdir --pidfile --log-file
-pid=`cat "$OVS_RUNDIR"/$1.pid`
-on_exit "kill $pid"
+pidfile="$OVS_RUNDIR"/$1.pid
+on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`"
 }
 
 # sim_add SANDBOX
-- 
2.9.5

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


Re: [ovs-dev] [PATCH] ovs-vsctl: show DPDK version

2017-12-22 Thread Eelco Chaudron

On 22/12/17 11:53, Matteo Croce wrote:

Show DPDK version if Open vSwitch is compiled with DPDK support.
Add a `dpdk_version` column in the datamodel and also edit ovs-dev.py to
populate the field with the right value.

Signed-off-by: Matteo Croce 
---
  utilities/ovs-dev.py   | 15 ---
  utilities/ovs-vsctl.c  |  7 +++
  vswitchd/vswitch.ovsschema |  7 +--
  vswitchd/vswitch.xml   |  4 
  4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index 9ce0f04c7..1bc0ba8c7 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -270,11 +270,20 @@ def run():
  " %s/ovsclient-cert.pem %s/vswitchd.cacert"
  % (pki_dir, pki_dir, pki_dir))
  version = _sh("ovs-vsctl --no-wait --version", capture=True)
-version = version[0].decode().strip().split()[3]
  root_uuid = _sh("ovs-vsctl --no-wait --bare list Open_vSwitch",
  capture=True)[0].decode().strip()
-_sh("ovs-vsctl --no-wait set Open_vSwitch %s ovs_version=%s"
-% (root_uuid, version))
+
+ovs_version = [s for s in version if "Open vSwitch" in s]
+if ovs_version:
+ovs_version = ovs_version[0].decode().strip().split()[3]
+_sh("ovs-vsctl --no-wait set Open_vSwitch %s ovs_version=%s"
+% (root_uuid, ovs_version))
+
+dpdk_version = [s for s in version if "DPDK" in s]
+if dpdk_version:
+dpdk_version = dpdk_version[0].decode().strip().split()[1]
+_sh("ovs-vsctl --no-wait set Open_vSwitch %s dpdk_version=%s"
+% (root_uuid, dpdk_version))
  
  build = BUILD_CLANG if options.clang else BUILD_GCC

  cmd = [build + "/vswitchd/ovs-vswitchd"]
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 7b909431d..f830120ad 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -27,6 +27,10 @@
  #include 
  #include 
  
+#ifdef DPDK_NETDEV

+#include 
+#endif
+
  #include "db-ctl-base.h"
  
  #include "command-line.h"

@@ -304,6 +308,9 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
  case 'V':
  ovs_print_version(0, 0);
  printf("DB Schema %s\n", ovsrec_get_db_version());
+#ifdef DPDK_NETDEV
+printf("%s\n", rte_version());
+#endif
  exit(EXIT_SUCCESS);
  
  case 't':

diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 90e50b626..757d8ec17 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
  {"name": "Open_vSwitch",
- "version": "7.15.1",
- "cksum": "3682332033 23608",
+ "version": "7.15.2",
+ "cksum": "3876676959 23718",
   "tables": {
 "Open_vSwitch": {
   "columns": {
@@ -36,6 +36,9 @@
 "db_version": {
   "type": {"key": {"type": "string"},
"min": 0, "max": 1}},
+   "dpdk_version": {
+ "type": {"key": {"type": "string"},
+  "min": 0, "max": 1}},
 "system_type": {
   "type": {"key": {"type": "string"},
"min": 0, "max": 1}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 37d04b7cf..74890f72b 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -611,6 +611,10 @@
  

  
+  

+DPDK version number, e.g. 17.11.
+  
+

  
An identifier for the type of system on top of which Open vSwitch


Changes look good to me.

Reviewed-by: Eelco Chaudron 

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


Re: [ovs-dev] [PATCH v5 0/5] OVS-DPDK flow offload with rte_flow

2017-12-22 Thread Finn Christensen
Hi,

The addition of the offload thread is an great addition, furthermore, RSS 
action usage for target queues, works now well with our NICs, I just had to do 
some additional drive work, to make it all work.

The support for RSS removed the problem with rte-flow queue selection, but RSS 
in general added an additional challenge as now multiple pmd's may request the 
same flow offload (as Yuanhan pointed out). The solution has worked without 
issues for me. I have run tests with 1 and 2 queues in a PV and PVP setup going 
over virtio. 
Yuanhan mentioned tests in a P2P setup with high throughput acceleration. I 
have focused on a PVP scenario, which, in addition, will crosses virtio, and 
most importantly, has acceleration in Rx direction only (seen from VM). It 
still gives fairly good performance improvements.

Here are my initial test results.

Percentage improvements:
1 RX/Tx queue:
1 megaflow - 10K flows  PV: 73%
10  megaflows - 100K flows  PVP: 50%PV: 56%
100 megaflows - 1M flowsPVP: 42%PV: 49%
1000 megaflows - 10M flows  PVP: 40%PV: 42%

With RSS: 2 Rx/Tx queue pairs:
1 megaflow - 10K flows  PV: 55%
10  megaflows - 100K flows  PVP: 36%PV: 38%
100 megaflows - 1M flowsPVP: 27%PV: 24%
1000 megaflows - 10M flows  PVP: 21%PV: 24%

PVP for 1 megaflow offload is omitted because my VM-app imposed a bottle-neck.

Regards,
Finn

-Original Message-
From: Yuanhan Liu [mailto:y...@fridaylinux.org]
Sent: 20. december 2017 15:45
To: d...@openvswitch.org
Cc: Finn Christensen ; Darrell Ball
; Chandran Sugesh ;
Simon Horman ; Ian Stokes
; Yuanhan Liu 
Subject: [PATCH v5 0/5] OVS-DPDK flow offload with rte_flow

Hi,

Here is a joint work from Mellanox and Napatech, to enable the flow hw
offload with the DPDK generic flow interface (rte_flow).

The basic idea is to associate the flow with a mark id (a unit32_t number).
Later, we then get the flow directly from the mark id, which could bypass
some heavy CPU operations, including but not limiting to mini flow extract,
emc lookup, dpcls lookup, etc.

The association is done with CMAP in patch 1. The CPU workload bypassing
is done in patch 2. The flow offload is done in patch 3, which mainly does
two things:

- translate the ovs match to DPDK rte flow patterns
- bind those patterns with a RSS + MARK action.

Patch 5 makes the offload work happen in another thread, for leaving the
datapath as light as possible.

A PHY-PHY forwarding with 1000 mega flows (udp,tp_src=1000-1999) and 1
million streams (tp_src=1000-1999, tp_dst=2000-2999) show more than
260% performance boost.

Note that it's disabled by default, which can be enabled by:

$ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true


v5: - fixed an issue that it took too long if we do flow add/remove
  repeatedly.
- removed an unused mutex lock
- turned most of the log level to DBG
- rebased on top of the latest code

v4: - use RSS action instead of QUEUE action with MARK
- make it work with multiple queue (see patch 1)
- rebased on top of latest code

v3: - The mark and id association is done with array instead of CMAP.
- Added a thread to do hw offload operations
- Removed macros completely
- dropped the patch to set FDIR_CONF, which is a workround some
  Intel NICs.
- Added a debug patch to show all flow patterns we have created.
- Misc fixes

v2: - workaround the queue action issue
- fixed the tcp_flags being skipped issue, which also fixed the
  build warnings
- fixed l2 patterns for Intel nic
- Converted some macros to functions
- did not hardcode the max number of flow/action
- rebased on top of the latest code

Thanks.

--yliu


---
Finn Christensen (1):
  netdev-dpdk: implement flow offload with rte flow

Yuanhan Liu (4):
  dpif-netdev: associate flow with a mark id
  dpif-netdev: retrieve flow directly from the flow mark
  netdev-dpdk: add debug for rte flow patterns
  dpif-netdev: do hw flow offload in a thread

 lib/dp-packet.h   |  13 +
 lib/dpif-netdev.c | 473 ++-
 lib/flow.c| 155 +---
 lib/flow.h|   1 +
 lib/netdev-dpdk.c | 732
+-
 lib/netdev.h  |   6 +
 6 files changed, 1341 insertions(+), 39 deletions(-)

--
2.7.4

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

[ovs-dev] [PATCH] ovs-vsctl: show DPDK version

2017-12-22 Thread Matteo Croce
Show DPDK version if Open vSwitch is compiled with DPDK support.
Add a `dpdk_version` column in the datamodel and also edit ovs-dev.py to
populate the field with the right value.

Signed-off-by: Matteo Croce 
---
 utilities/ovs-dev.py   | 15 ---
 utilities/ovs-vsctl.c  |  7 +++
 vswitchd/vswitch.ovsschema |  7 +--
 vswitchd/vswitch.xml   |  4 
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index 9ce0f04c7..1bc0ba8c7 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -270,11 +270,20 @@ def run():
 " %s/ovsclient-cert.pem %s/vswitchd.cacert"
 % (pki_dir, pki_dir, pki_dir))
 version = _sh("ovs-vsctl --no-wait --version", capture=True)
-version = version[0].decode().strip().split()[3]
 root_uuid = _sh("ovs-vsctl --no-wait --bare list Open_vSwitch",
 capture=True)[0].decode().strip()
-_sh("ovs-vsctl --no-wait set Open_vSwitch %s ovs_version=%s"
-% (root_uuid, version))
+
+ovs_version = [s for s in version if "Open vSwitch" in s]
+if ovs_version:
+ovs_version = ovs_version[0].decode().strip().split()[3]
+_sh("ovs-vsctl --no-wait set Open_vSwitch %s ovs_version=%s"
+% (root_uuid, ovs_version))
+
+dpdk_version = [s for s in version if "DPDK" in s]
+if dpdk_version:
+dpdk_version = dpdk_version[0].decode().strip().split()[1]
+_sh("ovs-vsctl --no-wait set Open_vSwitch %s dpdk_version=%s"
+% (root_uuid, dpdk_version))
 
 build = BUILD_CLANG if options.clang else BUILD_GCC
 cmd = [build + "/vswitchd/ovs-vswitchd"]
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 7b909431d..f830120ad 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -27,6 +27,10 @@
 #include 
 #include 
 
+#ifdef DPDK_NETDEV
+#include 
+#endif
+
 #include "db-ctl-base.h"
 
 #include "command-line.h"
@@ -304,6 +308,9 @@ parse_options(int argc, char *argv[], struct shash 
*local_options)
 case 'V':
 ovs_print_version(0, 0);
 printf("DB Schema %s\n", ovsrec_get_db_version());
+#ifdef DPDK_NETDEV
+printf("%s\n", rte_version());
+#endif
 exit(EXIT_SUCCESS);
 
 case 't':
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 90e50b626..757d8ec17 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "7.15.1",
- "cksum": "3682332033 23608",
+ "version": "7.15.2",
+ "cksum": "3876676959 23718",
  "tables": {
"Open_vSwitch": {
  "columns": {
@@ -36,6 +36,9 @@
"db_version": {
  "type": {"key": {"type": "string"},
   "min": 0, "max": 1}},
+   "dpdk_version": {
+ "type": {"key": {"type": "string"},
+  "min": 0, "max": 1}},
"system_type": {
  "type": {"key": {"type": "string"},
   "min": 0, "max": 1}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 37d04b7cf..74890f72b 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -611,6 +611,10 @@
 
   
 
+  
+DPDK version number, e.g. 17.11.
+  
+
   
 
   An identifier for the type of system on top of which Open vSwitch
-- 
2.14.3

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


[ovs-dev] how to use ofctl/packet-out command???

2017-12-22 Thread lin huang
Hi guys,

 I want to redirect packet-out message to a specific port. Is there an 
example for me??
Or someone who could teach me to use this feature??

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