Re: [ovs-dev] [PATCH] Update netbsd install doc

[Takashi YAMAMOTO]

On Wed, Dec 21, 2016 at 3:25 AM, Hui Kang  wrote:

> - test ovs on netbsd 7.0.2
> - use gmake to compile and install
>
> Signed-off-by: Hui Kang 
> ---
>  Documentation/intro/install/netbsd.rst | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/intro/install/netbsd.rst
> b/Documentation/intro/install/netbsd.rst
> index b32da20..c2c23e5 100644
> --- a/Documentation/intro/install/netbsd.rst
> +++ b/Documentation/intro/install/netbsd.rst
> @@ -42,7 +42,7 @@ information.
>  Assuming you are running NetBSD/amd64 6.1.2, you can download and install
>  pre-built binary packages as the following::
>
> -$ PKG_PATH=http://ftp.netbsd.org/pub/pkgsrc/packages/
> NetBSD/amd64/6.1.2/All/
> +$ PKG_PATH=http://ftp.netbsd.org/pub/pkgsrc/packages/
> NetBSD/amd64/7.0.2/All/
>  $ export PKG_PATH
>  $ pkg_add automake libtool-base gmake python27 py27-six py27-xml \
>  pkg_alternatives
> @@ -55,7 +55,8 @@ NetBSD's ``/usr/bin/make`` is not GNU make.  GNU make is
> installed as
>  ``/usr/pkg/bin/gmake`` by the above mentioned ``gmake`` package.
>
>  As all executables installed with pkgsrc are placed in ``/usr/pkg/bin/``
> -directory, it might be a good idea to add it to your PATH.
> +directory, it might be a good idea to add it to your PATH. Or install ovs
> by
>

you mean And?


> +``gmake`` and ``gmake install``.
>
>  Open vSwitch on NetBSD is currently "userspace switch" implementation in
> the
>  sense described in :doc:`userspace` and :doc:`/topics/porting`.
> --
> 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 1/2] ovn: Document upgrade procedure.

[Ben Pfaff]

On Wed, Dec 21, 2016 at 10:56:35PM -0500, Russell Bryant wrote:
> On Wed, Dec 21, 2016 at 5:06 PM, Ben Pfaff  wrote:
> 
> > On Wed, Dec 07, 2016 at 02:28:13PM -0500, Russell Bryant wrote:
> > > Signed-off-by: Russell Bryant 
> >
> > This procedure isn't what I expected.  It recommends:
> >
> > 1. Upgrade SB/NB databases and northd.
> > 2. Upgrade ovn-controller.
> > 3. Upgrade integration.
> >
> > I think that this is likely to cause problems in the common case,
> > because the new northd is likely to try to start using features that are
> > not yet available on the hypervisors, such as new logical match fields
> > and actions.
> >
> > I expected:
> >
> > 1. Upgrade ovn-controller.
> > 2. Upgrade SB/NB databases and northd.
> > 3. Upgrade integration.
> >
> > (I don't think that it matters when the databases are upgraded, though,
> > as long as they're upgraded before northd.)
> >
> 
> Thanks for the feedback!
> 
> There's a possible problem in this case, too.  ovn-controller could try to
> start using new fields in the southbound database that aren't there yet.

I guess that I have always considered that ovn-controller needs to
tolerate some version-related variation in the database schema.  This is
a familiar problem, since ovs-vsctl and (to a lesser extent)
ovs-vswitchd also need to tolerate similar variation.

Different kinds of tolerance must be considered.  Missing columns
(because they were added in a later database schema) are one of the
easiest, and the IDL takes care of that for us; the column will just
appear empty.  Over in OVS, we normally design the schema so that an
empty column yields the default new behavior anyhow.

The related_datapaths column that had been proposed for optimizing
conditional monitoring was a tougher case for upgrade.  With this
proposal, an empty column or a missing one would break ovn-controller,
since it wouldn't get all of the rows that it needed.  Even if the
column was present, it still wouldn't help if ovn-northd was too old to
populate it correctly.  I was about to propose that ovn-controller
needed to figure out not just whether the column was present but whether
ovn-northd was new enough and properly populating it, when I came up
with the approach we're now using that doesn't need this column at all.

So, upgrading the databases earlier cannot hurt, but I doubt that it
solves many problems.

> Maybe it should be:
> 
> 1. Upgrade SB/NB databases.
> 2. Upgrade ovn-controller.
> 3. Upgrade ovn-northd.
> 4. Upgrade integration.

This ordering seems fine to me.

Thank you for writing down the upgrade procedure.  (Have we written down
the installation procedure?)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] ovn-ctl: add support for SSL nb/sb db connections

[Ben Pfaff]

On Wed, Dec 21, 2016 at 06:35:43PM -0500, Lance Richardson wrote:
> Add support for SSL connections to OVN northbound and/or
> southbound databases.
> 
> To improve security, the NB and SB ovsdb daemons no longer
> have open ptcp connections by default.  This is a change in
> behavior from previous versions, users wishing to use TCP
> connections to the NB/SB daemons can either request that
> a passive TCP connection be used via ovn-ctl command-line
> options (e.g. via OVN_CTL_OPTS/OVN_NORTHD_OPTS in startup
> scripts):
> 
> --db-sb-create-remote=yes
> --db-nb-create-remote=yes

Thanks for writing this, and for rebasing.

I don't yet understand the design choices for the --db-?b-create-remote
options.  The names seem odd to me, since these options are particularly
about adding insecure remotes, and so I would expect the names to say
something about "legacy" or "insecure".  I'm also puzzled why these
options, which I'd expect to be supplied time after time to ovn-ctl if
they are necessary at all, make a stateful database change.  I would
have guessed, instead, that they add another --remote option to daemon
invocations.

Can you help me understand better?

Thanks,

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

Re: [ovs-dev] [PATCH 1/2] ovn: Document upgrade procedure.

[Russell Bryant]

On Wed, Dec 21, 2016 at 5:06 PM, Ben Pfaff  wrote:

> On Wed, Dec 07, 2016 at 02:28:13PM -0500, Russell Bryant wrote:
> > Signed-off-by: Russell Bryant 
>
> This procedure isn't what I expected.  It recommends:
>
> 1. Upgrade SB/NB databases and northd.
> 2. Upgrade ovn-controller.
> 3. Upgrade integration.
>
> I think that this is likely to cause problems in the common case,
> because the new northd is likely to try to start using features that are
> not yet available on the hypervisors, such as new logical match fields
> and actions.
>
> I expected:
>
> 1. Upgrade ovn-controller.
> 2. Upgrade SB/NB databases and northd.
> 3. Upgrade integration.
>
> (I don't think that it matters when the databases are upgraded, though,
> as long as they're upgraded before northd.)
>

Thanks for the feedback!

There's a possible problem in this case, too.  ovn-controller could try to
start using new fields in the southbound database that aren't there yet.
Maybe it should be:

1. Upgrade SB/NB databases.
2. Upgrade ovn-controller.
3. Upgrade ovn-northd.
4. Upgrade integration.

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

Re: [ovs-dev] [PATCH v2] odp-execute: Optimize IP header modification in OVS datapath

[Daniele Di Proietto]

2016-12-13 9:27 GMT-08:00 Zoltán Balogh :
>
> Hi Daniele,
>
>> Have you tried avoiding also computing the new field if the mask is 0?
>> For example, if
>> mask->ipv4_src is 0, there's not reason to compute new_ip_src, or to
>> extract ip_src_nh.
>
> Yes, I have investigated this case in mid-October. My results was
> that execution of "output + dec_ttl" and "output + mod_nw_tos" became even
> faster, but execution of rest of the actions I investigated became slower
> compared to the patch I posted previously.
> So I rejected this modification.
>
> Now, I rebased the code to 9th Dec master and ran the tests with DPDK 16.11
> again. Please find the new rebased patch below.
> The results of my original and the new patch:
>
>| T | T | I | I |
>| T | O | P | P |  Vanilla OVS  ||  + old patch  |  + new patch
>| L | S | s | d | (nsec/packet) || (nsec/packet) | (nsec/packet)
> ---+---+---+---+---+---++---+---
> output |   |   |   |   |67.19  ||67.19  |67.19
>| X |   |   |   |74.48  ||69.53  |68.78(+)
>|   | X |   |   |74.42  ||71.20  |70.07(+)
>|   |   | X |   |84.62  ||78.70  |78.03(+)
>|   |   |   | X |84.25  ||78.51  |77.94(+)
>|   |   | X | X |97.46  ||91.98  |91.86(+)
>| X |   | X | X |   100.42  ||95.00  |96.00(-)
>| X | X | X | X |   102.80  ||   100.40  |   100.73(-)
>
> I ran each test five times. The values are the mean of the readings obtained.
>
> As you can see, there is an improvement in the first 5 cases. But, in case of
> "output + dec_ttl + mod_nw_src + mod_nw_dst" and
> "output + dec_ttl + mod_nw_tos + mod_nw_src + mod_nw_dst", the execution of
> actions became slower compared to the old patch. However, this is still faster
> than vanilla OVS.
>
> Could you please confirm the results?
> Are these values acceptable?
> Is there anything still to improve in the new patch?

Sorry for the delay and thanks for the details and the new version

I've tried to reproduce your tests (64 bytes UDP phy-phy throughput)
on my system (2 Ghz haswell with ixgbe).

Here are my results:

   | T | T | I | I |
   | T | O | P | P |  Vanilla OVS  ||  + old patch  |  + new patch
   | L | S | s | d | (nsec/packet) || (nsec/packet) | (nsec/packet)
---+---+---+---+---+---++---+---
output |   |   |   |   |67.20  ||67.20  |67.20
   | X |   |   |   |87.03  ||82.17  |80.19(+)
   |   | X |   |   |87.03  ||81.37  |81.50(-)
   |   |   | X |   |95.88  ||90.66  |87.26(+)
   |   |   |   | X |95.51  ||90.42  |87.18(+)
   |   |   | X | X |   107.53  ||   103.41  |   100.20(+)
   | X |   | X | X |   111.98  ||   108.23  |   105.49(+)
   | X | X | X | X |   116.01  ||   112.49  |   111.11(+)

There are no regressions compared to master in any case, which is good.

Both versions look good to me.  Since you did the original profiling, which
version do you prefer?

Thanks,

Daniele

>
> The patch was applied to: 7971b36c3acc279f8e931d360f16c200752a3be2
>
> Best regards,
> Zoltan
>
> Signed-off-by: Zoltán Balogh 
>
> ---
>
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 65a6fcd..cc555b8 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -33,6 +33,7 @@
>  #include "flow.h"
>  #include "unaligned.h"
>  #include "util.h"
> +#include "csum.h"
>
>  /* Masked copy of an ethernet address. 'src' is already properly masked. */
>  static void
> @@ -68,13 +69,50 @@ odp_set_ipv4(struct dp_packet *packet, const struct 
> ovs_key_ipv4 *key,
>   const struct ovs_key_ipv4 *mask)
>  {
>  struct ip_header *nh = dp_packet_l3(packet);
> +ovs_be32 ip_src_nh;
> +ovs_be32 ip_dst_nh;
> +ovs_be32 new_ip_src;
> +ovs_be32 new_ip_dst;
> +uint8_t new_tos;
> +uint8_t new_ttl;
> +
> +if (mask->ipv4_src) {
> +ip_src_nh = get_16aligned_be32(&nh->ip_src);
> +new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src);
> +
> +if (ip_src_nh != new_ip_src) {
> +packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src);
> +}
> +}
>
> -packet_set_ipv4(
> -packet,
> -key->ipv4_src | (get_16aligned_be32(&nh->ip_src) & ~mask->ipv4_src),
> -key->ipv4_dst | (get_16aligned_be32(&nh->ip_dst) & ~mask->ipv4_dst),
> -key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos),
> -key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl));
> +if (mask->ipv4_dst) {
> +ip_dst_nh = get_16aligned_be32(&nh->ip_dst);
> +new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst);
> +
> +if (ip_dst_nh != new_ip_dst) {
> +  

Re: [ovs-dev] [RFC-PATCH 2/2] lib/dpdk: No more deferred release

[Daniele Di Proietto]

On 21/12/2016 12:06, "Aaron Conole"  wrote:

>Aaron Conole  writes:
>
>> DPDK documentation is recently updated to reflect that DPDK does not
>> hold any references to, nor take ownership of, the argv/argc elements.
>> With that understanding, let's just release the memory asap.
>>
>> Signed-off-by: Aaron Conole 
>> ---
>> NOTE: if the patch at
>>   http://dpdk.org/ml/archives/dev/2016-December/051812.html is
>>   accepted, then consider this non-RFC.
>
>Quick note - the documentation patch was recently merged to DPDK; is it
>possible to get this reviewed?  I can rebase, retest, and resubmit if
>needed.

No need to resend, it still applies.

I pushed this to master, thanks!

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

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Remove OVS_UNUSED from upcall_unixctl_set_flow_limit() arg.

[Justin Pettit]

> On Dec 21, 2016, at 4:06 PM, Joe Stringer  wrote:
> 
> On 21 December 2016 at 14:03, Justin Pettit  wrote:
>> The 'argv' argument is used.
>> 
>> Signed-off-by: Justin Pettit 
> 
> Acked-by: Joe Stringer 

Thanks.  I pushed this to master and branch-2.6.

--Justin


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

Re: [ovs-dev] [PATCH 1/5] system-traffic: Introduce OVS_START_L7 macro.

[Joe Stringer]

On 21 December 2016 at 17:05, Daniele Di Proietto  wrote:
> 2016-12-20 13:28 GMT-08:00 Joe Stringer :
>> All of the commands starting L7 servers duplicate detailed specifics
>> which inhibits readability, and makes it difficult to ensure that the
>> servers are ready before the test proceeds. Add a new macro that
>> provides simpler semantics from the test perspective and hide the
>> details in the macro. A followup patch will extend this macro to ensure
>> that servers are ready to serve requests before the test proceeds.
>>
>> Signed-off-by: Joe Stringer 
>
> Makes sense, thanks.
>
> There are still a couple NETNS_DAEMONIZE in system-ovn.at that could
> be replaced.

I'll do another scan across the tests for any other instances before committing.

> One more comment below.
>
> Acked-by: Daniele Di Proietto 
>
>> ---
>>  tests/system-common-macros.at |  11 
>>  tests/system-ovn.at   |   6 +--
>>  tests/system-traffic.at   | 121 
>> +++---
>>  3 files changed, 79 insertions(+), 59 deletions(-)
>>
>> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
>> index d41de2366695..eb5478d38d1b 100644
>> --- a/tests/system-common-macros.at
>> +++ b/tests/system-common-macros.at
>> @@ -230,6 +230,17 @@ m4_define([NETNS_DAEMONIZE],
>>  m4_define([OVS_CHECK_FIREWALL],
>>  [AT_SKIP_IF([systemctl status firewalld 2>&1 | grep running > 
>> /dev/null])])
>>
>> +# OVS_START_L7([namespace], [protocol])
>> +#
>> +# Start a server serving 'protocol' within 'namespace'. The server will exit
>> +# when the test finishes.
>> +#
>> +m4_define([OVS_START_L7],
>> +   [PIDFILE=$(mktemp $2XXX.log)
>
> Should this be .pid?

Yes, yes it should ;)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-trace: Fix small error condition memory leak in trace().

[Justin Pettit]

> On Dec 21, 2016, at 2:12 PM, Ben Pfaff  wrote:
> 
> On Wed, Dec 21, 2016 at 02:04:32PM -0800, Justin Pettit wrote:
>> Signed-off-by: Justin Pettit 
>> ---
>> ovn/utilities/ovn-trace.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
>> index 71798f8..6d8e514 100644
>> --- a/ovn/utilities/ovn-trace.c
>> +++ b/ovn/utilities/ovn-trace.c
>> @@ -1447,7 +1447,9 @@ trace(const char *dp_s, const char *flow_s)
>> char *error = expr_parse_microflow(flow_s, &symtab, &address_sets,
>>ovntrace_lookup_port, dp, &uflow);
>> if (error) {
>> -return xasprintf("error parsing flow: %s\n", error);
>> +char *s = xasprintf("error parsing flow: %s\n", error);
>> +free(error);
>> +return s;
>> }
> 
> I've often wished for a %s-like format specifier that frees the string
> after it prints it.
> 
> Acked-by: Ben Pfaff 

Thanks.  I pushed this to master and branch-2.6.

--Justin



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

Re: [ovs-dev] [PATCH 2/5] system-traffic: Wait for L7 servers to start.

[Joe Stringer]

On 21 December 2016 at 17:05, Daniele Di Proietto  wrote:
> 2016-12-20 13:28 GMT-08:00 Joe Stringer :
>> Use OVS_WAIT_UNTIL() with netstat to ensure servers are listening before
>> sending requests to them. This allows us to drop --retry-connrefused on
>> wget, needed before we can switch to curl.
>
> This commit doesn't actually remove --retry-connrefused.  Should we
> remove --retry-connrefused or should we update the commit message
> (maybe I misunderstood)?  Also, the last patch introduces --retry 2 in
> CURL_OPT, so I'm assuming we should keep --retry-connrefused.

Ah, let me drop that from the commit message. This helps with that,
but the subsequent patch that updates that is missing from this series
;) (I've got it in a queue somewhere but I'd rather get these
refactoring patches in first)

>>
>> Signed-off-by: Joe Stringer 
>
> Thanks for doing that, it's a nice improvement!
>
> Acked-by: Daniele Di Proietto 

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

Re: [ovs-dev] [PATCH 5/5] lib: Add support for tftp ct helper.

[Daniele Di Proietto]

2016-12-20 13:28 GMT-08:00 Joe Stringer :
> The kernel datapath provides support for TFTP helpers, so add support
> for this ALG to the commandline and OpenFlow encoding/decoding.
>
> Signed-off-by: Joe Stringer 

Do you want to add a test to tests/ofp-actions.at to parse the new alg?

Do you want to add a test to tests/odp.at to parse the new alg? Maybe
it's not important because for odp flows it's just a string.

I think we should add a check in ofpact_check__() to enforce that
alg=tftp is only used on udp flows, like commit
c9f7de8c2b3f("ofp-actions: Check that 'alg=ftp' matches on TCP.")

I have a few more comments inline, otherwise:

Acked-by: Daniele Di Proietto 

> ---
>  Documentation/intro/install/general.rst |  2 +
>  NEWS|  1 +
>  Vagrantfile |  4 +-
>  include/sparse/netinet/in.h |  1 +
>  include/windows/netinet/in.h|  1 +
>  lib/ofp-actions.c   | 13 -
>  lib/ofp-parse.c |  4 ++
>  ofproto/ofproto-dpif-xlate.c| 10 +++-
>  tests/atlocal.in| 26 +++---
>  tests/system-traffic.at | 84 
> +++--
>  tests/test-l7.py| 28 +--
>  utilities/ovs-ofctl.8.in| 15 --
>  12 files changed, 163 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/intro/install/general.rst 
> b/Documentation/intro/install/general.rst
> index 28c458fc8e43..c44a339aab9a 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -120,6 +120,8 @@ The datapath tests for userspace and Linux datapaths also 
> rely upon:
>  - pyftpdlib. Version 1.2.0 is known to work. Earlier versions should
>also work.
>
> +- tftpy. Version 0.6.2 is known to work. Earlier versions should also work.
> +

Should we add curl here?

>  - GNU wget. Version 1.16 is known to work. Earlier versions should also
>work.
>
> diff --git a/NEWS b/NEWS
> index 3a08dbc70db6..b58eaf46c293 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -22,6 +22,7 @@ Post-v2.6.0
> "selection_method" and related options in ovs-ofctl(8) for
> details.
>   * The "sample" action now supports "ingress" and "egress" options.
> + * The "ct" action now supports the TFTP ALG.
> - ovs-ofctl:
>   * 'bundle' command now supports packet-out messages.
>   * New syntax for 'ovs-ofctl packet-out' command, which uses the
> diff --git a/Vagrantfile b/Vagrantfile
> index 1fe60ecf4791..f596322ca4aa 100644
> --- a/Vagrantfile
> +++ b/Vagrantfile
> @@ -11,7 +11,7 @@ dnf -y install autoconf automake openssl-devel libtool \
> python-twisted-core python-zope-interface \
> desktop-file-utils groff graphviz rpmdevtools nc \
> wget python-six pyftpdlib checkpolicy selinux-policy-devel \
> -   libcap-ng-devel kernel-devel-`uname -r` ethtool
> +   libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy

and here

>  echo "search extra update built-in" >/etc/depmod.d/search_path.conf
>  SCRIPT
>
> @@ -26,7 +26,7 @@ aptitude -y install -R \
> xdg-utils groff graphviz netcat \
> wget python-six ethtool \
> libcap-ng-dev libssl-dev python-dev openssl \
> -   python-pyftpdlib python-flake8 \
> +   python-pyftpdlib python-flake8 python-tftpy \

and here

> linux-headers-`uname -r`
>  SCRIPT
>
> diff --git a/include/sparse/netinet/in.h b/include/sparse/netinet/in.h
> index 8a5b887bd51d..6dba45876e16 100644
> --- a/include/sparse/netinet/in.h
> +++ b/include/sparse/netinet/in.h
> @@ -75,6 +75,7 @@ struct sockaddr_in6 {
>  #define IPPROTO_SCTP 132
>
>  #define IPPORT_FTP 21
> +#define IPPORT_TFTP 69
>
>  /* All the IP options documented in Linux ip(7). */
>  #define IP_ADD_MEMBERSHIP 35
> diff --git a/include/windows/netinet/in.h b/include/windows/netinet/in.h
> index e4169994b14f..bae9f8ceecc5 100644
> --- a/include/windows/netinet/in.h
> +++ b/include/windows/netinet/in.h
> @@ -19,5 +19,6 @@
>
>  #define IPPROTO_GRE 47
>  #define IPPORT_FTP 21
> +#define IPPORT_TFTP 69

This works for sparse and for windows but fails for FreeBSD.

IPPORT_FTP is also defined in include/openvswitch/ofp-actions.h.  Can
we define IPPORT_TFTP there as well?  I'm not sure if we want to leave
it also in windows and sparse headers.

>
>  #endif /* netinet/in.h */
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 41d06fa319b7..391089d5f4dd 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -5454,10 +5454,19 @@ parse_CT(char *arg, struct ofpbuf *ofpacts,
>  static void
>  format_alg(int port, struct ds *s)
>  {
> -if (port == IPPORT_FTP) {
> +switch(port) {
> +case IPPORT_FTP:
>  ds_put_format(s, "%salg=%sftp,", colors.param, colors.end);
> -} else if (port) {
> +break;
> +  

Re: [ovs-dev] [PATCH 4/5] system-traffic: Reorder and bannerize ct tests.

[Daniele Di Proietto]

2016-12-20 13:28 GMT-08:00 Joe Stringer :
> Signed-off-by: Joe Stringer 

I haven't looked at this line by line, but as long as you just moved
code around:

Acked-by: Daniele Di Proietto 

> ---
>  tests/system-traffic.at | 898 
> 
>  1 file changed, 449 insertions(+), 449 deletions(-)
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 14fbf4518fdf..8e424c56031c 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -649,84 +649,6 @@ 
> udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
> -AT_SETUP([conntrack - IPv4 HTTP])
> -CHECK_CONNTRACK()
> -OVS_TRAFFIC_VSWITCHD_START()
> -
> -ADD_NAMESPACES(at_ns0, at_ns1)
> -
> -ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> -ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> -
> -dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from 
> ns1->ns0.
> -AT_DATA([flows.txt], [dnl
> -priority=1,action=drop
> -priority=10,arp,action=normal
> -priority=10,icmp,action=normal
> -priority=100,in_port=1,tcp,action=ct(commit),2
> -priority=100,in_port=2,ct_state=-trk,tcp,action=ct(table=0)
> -priority=100,in_port=2,ct_state=+trk+est,tcp,action=1
> -])
> -
> -AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> -
> -OVS_START_L7([at_ns0], [http])
> -OVS_START_L7([at_ns1], [http])
> -
> -dnl HTTP requests from ns0->ns1 should work fine.
> -NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o 
> wget0.log])
> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> -tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),protoinfo=(state=)
> -])
> -
> -dnl HTTP requests from ns1->ns0 should fail due to network failure.
> -dnl Try 3 times, in 1 second intervals.
> -NS_CHECK_EXEC([at_ns1], [wget 10.1.1.1 -t 3 -T 1 -v -o wget1.log], [4])
> -
> -OVS_TRAFFIC_VSWITCHD_STOP
> -AT_CLEANUP
> -
> -AT_SETUP([conntrack - IPv6 HTTP])
> -CHECK_CONNTRACK()
> -OVS_TRAFFIC_VSWITCHD_START()
> -
> -ADD_NAMESPACES(at_ns0, at_ns1)
> -
> -ADD_VETH(p0, at_ns0, br0, "fc00::1/96")
> -ADD_VETH(p1, at_ns1, br0, "fc00::2/96")
> -
> -dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from 
> ns1->ns0.
> -AT_DATA([flows.txt], [dnl
> -priority=1,action=drop
> -priority=10,icmp6,action=normal
> -priority=100,in_port=1,tcp6,action=ct(commit),2
> -priority=100,in_port=2,ct_state=-trk,tcp6,action=ct(table=0)
> -priority=100,in_port=2,ct_state=+trk+est,tcp6,action=1
> -])
> -
> -AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> -
> -dnl Linux seems to take a little time to get its IPv6 stack in order. Without
> -dnl waiting, we get occasional failures due to the following error:
> -dnl "connect: Cannot assign requested address"
> -OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
> -
> -OVS_START_L7([at_ns0], [http6])
> -OVS_START_L7([at_ns1], [http6])
> -
> -dnl HTTP requests from ns0->ns1 should work fine.
> -NS_CHECK_EXEC([at_ns0], [wget http://[[fc00::2]] -t 3 -T 1 
> --retry-connrefused -v -o wget0.log])
> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
> -tcp,orig=(src=fc00::1,dst=fc00::2,sport=,dport=),reply=(src=fc00::2,dst=fc00::1,sport=,dport=),protoinfo=(state=)
> -])
> -
> -dnl HTTP requests from ns1->ns0 should fail due to network failure.
> -dnl Try 3 times, in 1 second intervals.
> -NS_CHECK_EXEC([at_ns1], [wget http://[[fc00::1]] -t 3 -T 1 -v -o wget1.log], 
> [4])
> -
> -OVS_TRAFFIC_VSWITCHD_STOP
> -AT_CLEANUP
> -
>  AT_SETUP([conntrack - IPv4 ping])
>  CHECK_CONNTRACK()
>  OVS_TRAFFIC_VSWITCHD_START()
> @@ -815,47 +737,6 @@ 
> icmpv6,orig=(src=fc00::1,dst=fc00::2,id=,type=128,code=0),reply=(src=fc
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
> -AT_SETUP([conntrack - commit, recirc])
> -CHECK_CONNTRACK()
> -OVS_TRAFFIC_VSWITCHD_START()
> -
> -ADD_NAMESPACES(at_ns0, at_ns1, at_ns2, at_ns3)
> -
> -ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> -ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> -ADD_VETH(p2, at_ns2, br0, "10.1.1.3/24")
> -ADD_VETH(p3, at_ns3, br0, "10.1.1.4/24")
> -
> -dnl Allow any traffic from ns0->ns1, ns2->ns3.
> -AT_DATA([flows.txt], [dnl
> -priority=1,action=drop
> -priority=10,arp,action=normal
> -priority=10,icmp,action=normal
> -priority=100,in_port=1,tcp,ct_state=-trk,action=ct(commit,table=0)
> -priority=100,in_port=1,tcp,ct_state=+trk,action=2
> -priority=100,in_port=2,tcp,ct_state=-trk,action=ct(table=0)
> -priority=100,in_port=2,tcp,ct_state=+trk,action=1
> -priority=100,in_port=3,tcp,ct_state=-trk,action=set_field:0->metadata,ct(table=0)
> -priority=100,in_port=3,tcp,ct_state=+trk,metadata=0,action=set_field:1->metadata,ct(commit,table=0)
> -priority=100,in_port=3,tcp,ct_state=+trk,metadata=1,action=4
> -priority=100,in_port=4,tcp,ct_state=-trk,action=ct(commit,table=0)
> -priority=100,in_port=4,tcp,ct_state=+trk,action=3
> -])
> -
> -AT_CHECK([ovs-ofctl --bundle a

Re: [ovs-dev] [PATCH 3/5] system-traffic: Add banners for ct sections.

[Daniele Di Proietto]

2016-12-20 13:28 GMT-08:00 Joe Stringer :
> Signed-off-by: Joe Stringer 

Acked-by: Daniele Di Proietto 

> ---
>  tests/system-ovn.at | 2 ++
>  tests/system-traffic.at | 3 +++
>  2 files changed, 5 insertions(+)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 7b35e84a43d2..022dc63f22c2 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -1,3 +1,5 @@
> +AT_BANNER([system-ovn])
> +
>  AT_SETUP([ovn -- 2 LRs connected via LS, gateway router, SNAT and DNAT])
>  AT_KEYWORDS([ovnnat])
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 803b00850511..14fbf4518fdf 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -604,6 +604,8 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep "in_port=4" | 
> ofctl_strip], [0], [dnl
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
> +AT_BANNER([conntrack])
> +
>  AT_SETUP([conntrack - controller])
>  CHECK_CONNTRACK()
>  OVS_TRAFFIC_VSWITCHD_START()
> @@ -2182,6 +2184,7 @@ NXST_FLOW reply:
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
> +AT_BANNER([conntrack - NAT])
>
>  AT_SETUP([conntrack - simple SNAT])
>  CHECK_CONNTRACK()
> --
> 2.10.2
>
> ___
> 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 2/5] system-traffic: Wait for L7 servers to start.

[Daniele Di Proietto]

2016-12-20 13:28 GMT-08:00 Joe Stringer :
> Use OVS_WAIT_UNTIL() with netstat to ensure servers are listening before
> sending requests to them. This allows us to drop --retry-connrefused on
> wget, needed before we can switch to curl.

This commit doesn't actually remove --retry-connrefused.  Should we
remove --retry-connrefused or should we update the commit message
(maybe I misunderstood)?  Also, the last patch introduces --retry 2 in
CURL_OPT, so I'm assuming we should keep --retry-connrefused.

>
> Signed-off-by: Joe Stringer 

Thanks for doing that, it's a nice improvement!

Acked-by: Daniele Di Proietto 

> ---
>  tests/system-common-macros.at | 4 
>  tests/system-traffic.at   | 4 
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
> index eb5478d38d1b..90f6065c1ba4 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -238,6 +238,10 @@ m4_define([OVS_CHECK_FIREWALL],
>  m4_define([OVS_START_L7],
> [PIDFILE=$(mktemp $2XXX.log)
>  NETNS_DAEMONIZE([$1], [[$PYTHON $srcdir/test-l7.py $2]], [$PIDFILE])
> +
> +dnl netstat doesn't print http over IPv6 as "http6"; drop the number.
> +PROTO=$(echo $2 | sed -e 's/\([[a-zA-Z]]*\).*/\1/')
> +OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -l | grep $PROTO])])
> ]
>  )
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 67e06a11805b..803b00850511 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -1642,7 +1642,6 @@ AT_CHECK([ovs-ofctl --bundle replace-flows br0 
> flows1.txt])
>
>  OVS_START_L7([at_ns0], [ftp])
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p1->p0 should fail due to network failure.
>  dnl Try 3 times, in 1 second intervals.
> @@ -1727,7 +1726,6 @@ dnl "connect: Cannot assign requested address"
>  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
>
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p0->p1 should work fine.
>  NS_CHECK_EXEC([at_ns0], [wget ftp://[[fc00::2]] -6 --no-passive-ftp -t 3 -T 
> 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log 
> -d])
> @@ -2534,7 +2532,6 @@ m4_define([CHECK_FTP_NAT],
>  AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
>
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p0->p1 should work fine.
>  NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 -4 --no-passive-ftp -t 3 -T 
> 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log 
> -d])
> @@ -2772,7 +2769,6 @@ dnl "connect: Cannot assign requested address"
>  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
>
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p0->p1 should work fine.
>  NS_CHECK_EXEC([at_ns0], [wget ftp://[[fc00::2]] -6 --no-passive-ftp -t 3 -T 
> 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log 
> -d])
> --
> 2.10.2
>
> ___
> 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 1/5] system-traffic: Introduce OVS_START_L7 macro.

[Daniele Di Proietto]

2016-12-20 13:28 GMT-08:00 Joe Stringer :
> All of the commands starting L7 servers duplicate detailed specifics
> which inhibits readability, and makes it difficult to ensure that the
> servers are ready before the test proceeds. Add a new macro that
> provides simpler semantics from the test perspective and hide the
> details in the macro. A followup patch will extend this macro to ensure
> that servers are ready to serve requests before the test proceeds.
>
> Signed-off-by: Joe Stringer 

Makes sense, thanks.

There are still a couple NETNS_DAEMONIZE in system-ovn.at that could
be replaced.

One more comment below.

Acked-by: Daniele Di Proietto 

> ---
>  tests/system-common-macros.at |  11 
>  tests/system-ovn.at   |   6 +--
>  tests/system-traffic.at   | 121 
> +++---
>  3 files changed, 79 insertions(+), 59 deletions(-)
>
> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
> index d41de2366695..eb5478d38d1b 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -230,6 +230,17 @@ m4_define([NETNS_DAEMONIZE],
>  m4_define([OVS_CHECK_FIREWALL],
>  [AT_SKIP_IF([systemctl status firewalld 2>&1 | grep running > 
> /dev/null])])
>
> +# OVS_START_L7([namespace], [protocol])
> +#
> +# Start a server serving 'protocol' within 'namespace'. The server will exit
> +# when the test finishes.
> +#
> +m4_define([OVS_START_L7],
> +   [PIDFILE=$(mktemp $2XXX.log)

Should this be .pid?

> +NETNS_DAEMONIZE([$1], [[$PYTHON $srcdir/test-l7.py $2]], [$PIDFILE])
> +   ]
> +)
> +
>  # OVS_CHECK_VXLAN()
>  #
>  # Do basic check for vxlan functionality, skip the test if it's not there.
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 9e323424a2a1..7b35e84a43d2 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -581,9 +581,9 @@ ovn-nbctl set load_balancer $uuid 
> vips:'"30.0.0.2:8000"'='"172.16.1.2:80,172.16.
>  OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | grep ct\(])
>
>  # Start webservers in 'bar1', 'bar2' and 'bar3'.
> -NETNS_DAEMONIZE([bar1], [[$PYTHON $srcdir/test-l7.py]], [http1.pid])
> -NETNS_DAEMONIZE([bar2], [[$PYTHON $srcdir/test-l7.py]], [http2.pid])
> -NETNS_DAEMONIZE([bar3], [[$PYTHON $srcdir/test-l7.py]], [http3.pid])
> +OVS_START_L7([bar1], [http])
> +OVS_START_L7([bar2], [http])
> +OVS_START_L7([bar3], [http])
>
>  dnl Should work with the virtual IP 30.0.0.1 address through NAT
>  for i in `seq 1 20`; do
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index d70c5c3f0b87..67e06a11805b 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -37,7 +37,7 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 
> | FORMAT_PING], [0],
>  3 packets transmitted, 3 received, 0% packet loss, time 0ms
>  ])
>
> -NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py]], [http0.pid])
> +OVS_START_L7([at_ns1], [http])
>  NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o 
> wget0.log])
>
>  OVS_TRAFFIC_VSWITCHD_STOP
> @@ -668,17 +668,17 @@ priority=100,in_port=2,ct_state=+trk+est,tcp,action=1
>
>  AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
>
> +OVS_START_L7([at_ns0], [http])
> +OVS_START_L7([at_ns1], [http])
> +
>  dnl HTTP requests from ns0->ns1 should work fine.
> -NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py]], [http0.pid])
>  NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o 
> wget0.log])
> -
>  AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
>  
> tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),protoinfo=(state=)
>  ])
>
>  dnl HTTP requests from ns1->ns0 should fail due to network failure.
>  dnl Try 3 times, in 1 second intervals.
> -NETNS_DAEMONIZE([at_ns0], [[$PYTHON $srcdir/test-l7.py]], [http1.pid])
>  NS_CHECK_EXEC([at_ns1], [wget 10.1.1.1 -t 3 -T 1 -v -o wget1.log], [4])
>
>  OVS_TRAFFIC_VSWITCHD_STOP
> @@ -709,18 +709,17 @@ dnl waiting, we get occasional failures due to the 
> following error:
>  dnl "connect: Cannot assign requested address"
>  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
>
> -dnl HTTP requests from ns0->ns1 should work fine.
> -NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py http6]], [http0.pid])
> +OVS_START_L7([at_ns0], [http6])
> +OVS_START_L7([at_ns1], [http6])
>
> +dnl HTTP requests from ns0->ns1 should work fine.
>  NS_CHECK_EXEC([at_ns0], [wget http://[[fc00::2]] -t 3 -T 1 
> --retry-connrefused -v -o wget0.log])
> -
>  AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
>  
> tcp,orig=(src=fc00::1,dst=fc00::2,sport=,dport=),reply=(src=fc00::2,dst=fc00::1,sport=,dport=),protoinfo=(state=)
>  ])
>
>  dnl HTTP requests from ns1->ns0 should fail due to network failure.
>  dnl Try 3 times, in 1 second intervals.
> -NETNS_DAEMONIZE([at_ns0], [[$PYTHON $srcdir/test-l7.py http6]], [http1.pid

Re: [ovs-dev] [PATCH] system-traffic: Fix clone test.

[Joe Stringer]

On 21 December 2016 at 14:22, William Tu  wrote:
> The existing clone test fails the system testsuite.  The patch provides
> fix, removes the unused at_ns2, and uses "ovs-ofctl monitor" to validate
> the packet contents after actions inside a clone.
>
> Signed-off-by: William Tu 

LGTM thanks! Applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Remove OVS_UNUSED from upcall_unixctl_set_flow_limit() arg.

[Joe Stringer]

On 21 December 2016 at 14:03, Justin Pettit  wrote:
> The 'argv' argument is used.
>
> Signed-off-by: Justin Pettit 

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

Re: [ovs-dev] [RFC PATCH v6 1/6] Add support for 802.1ad (QinQ tunneling)

[Ben Pfaff]

On Mon, Dec 12, 2016 at 11:29:49AM -0500, Eric Garver wrote:
> Flow key handling changes:
>  - Add VLAN header array in struct flow, to record multiple 802.1q VLAN
>headers.
>  - Add dpif multi-VLAN capability probing. If datapath supports
>multi-VLAN, increase the maximum depth of nested OVS_KEY_ATTR_ENCAP.
> 
> Refactor VLAN handling in dpif-xlate:
>  - Introduce 'xvlan' to track VLAN stack during flow processing.
>  - Input and output VLAN translation according to the xbundle type.
> 
> Push VLAN action support:
>  - Allow ethertype 0x88a8 in VLAN headers and push_vlan action.
>  - Support push_vlan on dot1q packets.
> 
> Add new port VLAN mode "dot1q-tunnel":
>  - Example:
>  ovs-vsctl set Port p1 vlan_mode=dot1q-tunnel tag=100
>Pushes another VLAN 100 header on packets (tagged and untagged) on
>ingress, and pops it on egress.
>  - Customer VLAN check:
>  ovs-vsctl set Port p1 vlan_mode=dot1q-tunnel tag=100 cvlans=10,20
>Only customer VLAN of 10 and 20 are allowed.
> 
> Use other_config:vlan-limit in table Open_vSwitch to limit maximum VLANs
> that can be matched. This allows us to preserve backwards compatibility.
> 
> Add test cases for VLAN depth limit, Multi-VLAN actions and QinQ VLAN
> handling
> 
> Co-authored-by: Thomas F Herbert 
> Co-authored-by: Xiao Liang 
> Co-authored-by: Eric Garver 
> Signed-off-by: Eric Garver 

I'm pretty happy with this.  I have only a few comments.

This should add some items to NEWS.

I'd prefer to see the dot1q-tunnel/cvlan changes split into a second
patch.  They are logically separate from pure OpenFlow support for QinQ,
and require additional thought.

I'll look forward to the first non-RFC version.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] splitting commits in Git (was: Re: Sync on PTAP, EXT-382 and NSH)

[Jan Scheurich]

Thanks Ben/Jarno,

I had previously seen some people using git add/commit -p but found it not very 
intuitive.
I had also used the Git GUI at some time but never realized that is offers the 
possibility to stage individual hunks or even lines in a file. That looks 
pretty neat.

Regards, Jan

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> 
> On Wed, Dec 21, 2016 at 10:06:29AM -0800, Jarno Rajahalme wrote:
> >
> > > On Dec 21, 2016, at 9:30 AM, Ben Pfaff  wrote:
> > >
> > > There was a question about splitting up large commits in Git.  I do
> > > this routinely and I volunteered to find some resources.
> > >
> > > The "git-rebase" manpage has a section titled "SPLITTING COMMITS"
> > > that is a good place to start.  It does not cover very well how to
> > > commit parts of a file rather than a whole file, which is something
> > > that I do often.  For this, I use "git citool" and then select lines
> > > to commit and right-click to "stage lines for commit".  I know that
> > > there's a mode for the "git commit" CLI that can do this too, but
> > > I've always found that hard to use compared to "git citool”.
> >
> > You mean “git add -p”? I’ve used that, but was not aware of “git
> > citool”…
> 
> I think I use "git rebase -i" and "git citool" more than any other git 
> commands.  (I use "git am" a lot too but I have a mutt shortcut for
> that.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-ovn] ovn:When configuring Logical switch port type by" lsp-set-type"command, Not Specify the type of Logical switch port.

[Ben Pfaff]

On Mon, Dec 12, 2016 at 11:00:38AM +0800, zhou.huij...@zte.com.cn wrote:
> commit 812f44a9cb6862f3b487e23154a9e114040c40a2 
> Author: zhaojingjing  
> Date:   Mon Dec 12 10:48:33 2016 +0800 
> 
> Specify the type of Logical switch port 
> 

This seems to have limited value, and it will be difficult to maintain
as we continue to add more kinds of ports over time, so I'd prefer not
to merge it.

The patch is badly formatted and I doubt I could apply it.  Please try
to use git-send-email in the future.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofp-parse: Allow match field names in actions and brackets in matches.

[Ben Pfaff]

On Fri, Dec 09, 2016 at 03:25:57PM -0800, Jarno Rajahalme wrote:
> Allow using match field names in addition to the canonical register
> names in actions (including 'load', 'move', 'push', 'pop', 'output',
> 'multipath', 'bundle_load', and 'learn').  Allow also leaving out the
> trailing '[]' to indicate full field.  These changes allow simpler
> syntax similar to 'set_field' to be used also elsewhere.
> 
> Correspondingly, allow the '[start..end]' syntax to be used in matches
> in addition to the more explicit 'value/mask' notation.  For example,
> to match on the value 2 of the bits 14..15 of NXM_NX_REG0, the match
> could include:
> 
> ... reg0[14..15]=2 ...
> 
> instead of
> 
> ... reg0=0x8000/0xc000 ...
> 
> Note that only contiguous masks can be specified with the bracket
> notation.
> 
> Signed-of-by: Jarno Rajahalme 
> Signed-off-by: Jarno Rajahalme 

This is much nicer than before.  Thank you.

The signed-off-by is doubled above.

Acked-by: Ben Pfaff 


Here are some suggestions.

diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index b091c1b..9f123ef 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -2069,6 +2069,7 @@ struct field_array {
 
 /* Finding mf_fields. */
 const struct mf_field *mf_from_name(const char *name);
+const struct mf_field *mf_from_name_len(const char *name, size_t len);
 
 static inline const struct mf_field *
 mf_from_id(enum mf_field_id id)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index e25adec..6fc8ff9 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -80,6 +80,17 @@ mf_from_name(const char *name)
 return shash_find_data(&mf_by_name, name);
 }
 
+/* Returns the field with the given 'name' (which is 'len' bytes long), or a
+ * null pointer if no field has that name. */
+const struct mf_field *
+mf_from_name_len(const char *name, size_t len)
+{
+nxm_init();
+
+struct shash_node *node = shash_find_len(&mf_by_name, name, len);
+return node ? node->data : NULL;
+}
+
 static void
 nxm_do_init(void)
 {
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 9c769c0..2fd31b9 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1825,17 +1825,7 @@ mf_parse_subfield__(struct mf_subfield *sf, const char 
**sp)
 name_len = strcspn(s, "[-");
 
 f = mf_parse_subfield_name(name, name_len, &wild);
-if (f) {
-field = mf_from_id(f->id);
-} else if (name_len < 256) {
-char name_buf[256];
-if (s[name_len] != '\0') {
-memcpy(name_buf, name, name_len);
-name_buf[name_len] = '\0';
-name = name_buf;
-}
-field = mf_from_name(name);
-}
+field = f ? mf_from_id(f->id) : mf_from_name_len(name, name_len);
 if (!field) {
 return xasprintf("%s: unknown field `%.*s'", *sp, name_len, s);
 }
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index c4685b7..48d000c 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1545,10 +1545,8 @@ is the packet's input port, the packet is not output.
 .
 .IP \fBoutput:\fIsrc\fB[\fIstart\fB..\fIend\fB]
 Outputs the packet to the OpenFlow port number read from \fIsrc\fR,
-which must be an NXM field as described above.  Also the match field
-name can be used, for example, instead of 'NXM_NX_REG0' the
-name 'reg0' can be used.  For example,
-\fBoutput:NXM_NX_REG0[16..31]\fR outputs to the OpenFlow port number
+which may be an NXM field name, as described above, or a match field name.
+\fBoutput:reg0[16..31]\fR outputs to the OpenFlow port number
 written in the upper half of register 0.  If the port number is the
 packet's input port, the packet is not output.
 .IP
@@ -2009,10 +2007,9 @@ bytes with value 0 to make the total number 6 more than 
a multiple of
 .
 .IP 
"\fBmove:\fIsrc\fB[\fIstart\fB..\fIend\fB]\->\fIdst\fB[\fIstart\fB..\fIend\fB]\fR"
 Copies the named bits from field \fIsrc\fR to field \fIdst\fR.
-\fIsrc\fR and \fIdst\fR must be NXM field names as defined in
-\fBnicira\-ext.h\fR, e.g. \fBNXM_OF_UDP_SRC\fR or \fBNXM_NX_REG0\fR.
-Also the corresponding match field names can be used, for example,
-instead of 'NXM_NX_REG0' the name 'reg0' can be used.  Each
+\fIsrc\fR and \fIdst\fR may be NXM field names as defined in
+\fBnicira\-ext.h\fR, e.g. \fBNXM_OF_UDP_SRC\fR or \fBNXM_NX_REG0\fR,
+or a match field name, e.g. \fBreg0\fR.  Each
 \fIstart\fR and \fIend\fR pair, which are inclusive, must specify the
 same number of bits and must fit within its respective field.
 Shorthands for \fB[\fIstart\fB..\fIend\fB]\fR exist: use
@@ -3288,10 +3285,10 @@ Implements a level 2 MAC learning switch using the 
learn.
 \fBovs\-ofctl add\-flow br0 'table=0,priority=0 
actions=load:3->NXM_NX_REG0[0..15],learn(table=0,priority=1,idle_timeout=10,NXM_OF_ETH_SRC[],NXM_OF_VLAN_TCI[0..11],output:NXM_NX_REG0[0..15]),output:2\fR
 In this use of a learn action, the first packet from each source MAC
 will be sent to port 2. Subsequent packets will be output t

[ovs-dev] [PATCH v3] ovn-ctl: add support for SSL nb/sb db connections

[Lance Richardson]

Add support for SSL connections to OVN northbound and/or
southbound databases.

To improve security, the NB and SB ovsdb daemons no longer
have open ptcp connections by default.  This is a change in
behavior from previous versions, users wishing to use TCP
connections to the NB/SB daemons can either request that
a passive TCP connection be used via ovn-ctl command-line
options (e.g. via OVN_CTL_OPTS/OVN_NORTHD_OPTS in startup
scripts):

--db-sb-create-remote=yes
--db-nb-create-remote=yes

Or configure a connection after the NB/SB daemons have been
started, e.g.:

ovn-sbctl set-connection ptcp:6642
ovn-nbctl set-connection ptcp:6641

Users desiring SSL database connections will need to generate certificates
and private key as described in INSTALL.SSL.rst and perform the following
one-time configuration steps:

   ovn-sbctl set-ssl   
   ovn-sbctl set-connection pssl:6642
   ovn-nbctl set-ssl   
   ovn-nbctl set-connection pssl:6641

On the ovn-controller and ovn-controller-vtep side, SSL configuration
must be provided on the command-line when the daemons are started, this
should be provided via the following command-line options (e.g. via
OVN_CTL_OPTS/OVN_CONTROLLER_OPTS in startup scripts):

   --ovn-controller-ssl-key=
   --ovn-controller-ssl-cert=
   --ovn-controller-ssl-ca-cert=

The SB database connection should also be configured to use SSL, e.g.:

ovs-vsctl set Open_vSwitch . \
  external-ids:ovn-remote=ssl:w.x.y.z:6642

Co-authored-by: Numan Siddique 
Signed-off-by: Numan Siddique 
Signed-off-by: Lance Richardson 
---
v3: - rebased
- s/db-sb-default-remote/db-sb-create-remote/ in man page
- s/db-nb-default-remote/db-nb-create-remote/ in man page

v2: - Changed DB_NB_DEFAULT_REMOTE to DB_NB_CREATE_REMOTE.
- Changed DB_SB_DEFAULT_REMOTE to DB_SB_CREATE_REMOTE.
- Create default remote configuration in db instead of
  via command-line options.

 NEWS|   5 +++
 manpages.mk |   4 ++
 ovn/utilities/ovn-ctl   | 106 +---
 ovn/utilities/ovn-ctl.8.xml |   7 +++
 4 files changed, 105 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index 882f611..e30273a 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,11 @@ Post-v2.6.0
  * ovn-trace can now trace put_dhcp_opts and put_dhcp_optsv6 actions.
  * Support for managing SSL and remote connection configuration in
northbound and southbound databases.
+ * TCP connections to northbound and southbound databases are no
+   longer enabled by default and must be explicitly configured.
+   See documentation for ovn-sbctl/ovn-nbctl "set-connection" command
+   or ovn-ctl "--db-sb-create-remote"/"--db-nb-create-remote"
+   options for information regarding enabling TCP connections.
- Fixed regression in table stats maintenance introduced in OVS
  2.3.0, wherein the number of OpenFlow table hits and misses was
  not accurate.
diff --git a/manpages.mk b/manpages.mk
index 742bd66..825e2bc 100644
--- a/manpages.mk
+++ b/manpages.mk
@@ -42,6 +42,8 @@ ovsdb/ovsdb-client.1: \
lib/vlog-syn.man \
lib/vlog.man \
ovsdb/remote-active.man \
+   ovsdb/remote-active.man \
+   ovsdb/remote-passive.man \
ovsdb/remote-passive.man
 ovsdb/ovsdb-client.1.in:
 lib/common-syn.man:
@@ -58,6 +60,8 @@ lib/table.man:
 lib/vlog-syn.man:
 lib/vlog.man:
 ovsdb/remote-active.man:
+ovsdb/remote-active.man:
+ovsdb/remote-passive.man:
 ovsdb/remote-passive.man:
 
 ovsdb/ovsdb-server.1: \
diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 73e78e5..f4526fd 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -50,7 +50,7 @@ stop_ovsdb () {
 
 demote_ovnnb() {
 if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
-echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+echo 
"$DB_NB_SYNC_FROM_PROTO:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
 fi
 
 if test -e $ovnnb_active_conf_file; then
@@ -64,7 +64,7 @@ demote_ovnnb() {
 
 demote_ovnsb() {
 if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
-echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+echo 
"$DB_SB_SYNC_FROM_PROTO:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
 fi
 
 if test -e $ovnsb_active_conf_file; then
@@ -93,15 +93,17 @@ start_ovsdb () {
 
 set ovsdb-server
 
-set "$@" --detach --monitor $OVN_NB_LOG \
---log-file=$OVN_NB_LOGFILE \
---remote=punix:$DB_NB_SOCK \
---remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR \
---pidfile=$DB_NB_PID \
---unixctl=ovnnb_db.ctl
+set "$@" --detach --monitor
+set "$@" $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE
+set "$@" --remote=punix:$DB_NB_SOCK --pidfile=$DB_NB_PID
+set "$@" --remote=db:OVN_Northbound,NB_Global,connections
+s

Re: [ovs-dev] [PATCH] Update netbsd install doc

[Ben Pfaff]

On Tue, Dec 20, 2016 at 01:25:07PM -0500, Hui Kang wrote:
> - test ovs on netbsd 7.0.2
> - use gmake to compile and install
> 
> Signed-off-by: Hui Kang 

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

[ovs-dev] [PATCH v4 2/2] ovn-trace: New --ovs option to also print OpenFlow flows.

[Ben Pfaff]

Sometimes seeing the OpenFlow flows that back a given logical flow can
provide additional insight.  This commit adds a new --ovs option to
ovn-trace that makes it connect to Open vSwitch over OpenFlow and retrieve
and print the OpenFlow flows behind each logical flow encountered during
a trace.

Signed-off-by: Ben Pfaff 
---
 NEWS  |   4 +-
 include/openvswitch/vconn.h   |  15 +++--
 lib/vconn.c   | 121 -
 ovn/utilities/ovn-trace.8.xml |  61 -
 ovn/utilities/ovn-trace.c |  91 -
 utilities/ovs-ofctl.c | 152 +-
 6 files changed, 315 insertions(+), 129 deletions(-)

diff --git a/NEWS b/NEWS
index 882f611..aead685 100644
--- a/NEWS
+++ b/NEWS
@@ -7,7 +7,9 @@ Post-v2.6.0
  * DSCP marking is now supported, via the new northbound QoS table.
  * IPAM now supports fixed MAC addresses.
  * Support for source IP address based routing.
- * ovn-trace can now trace put_dhcp_opts and put_dhcp_optsv6 actions.
+ * ovn-trace:
+   - New --ovs option to also print OpenFlow flows.
+   - put_dhcp_opts and put_dhcp_optsv6 actions may now be traced.
  * Support for managing SSL and remote connection configuration in
northbound and southbound databases.
- Fixed regression in table stats maintenance introduced in OVS
diff --git a/include/openvswitch/vconn.h b/include/openvswitch/vconn.h
index d2fbd16..acdc79c 100644
--- a/include/openvswitch/vconn.h
+++ b/include/openvswitch/vconn.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, 
Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,9 +18,10 @@
 #define OPENVSWITCH_VCONN_H 1
 
 #include 
-#include 
-#include 
-#include 
+#include "openvswitch/list.h"
+#include "openvswitch/types.h"
+#include "openvswitch/ofp-util.h"
+#include "openflow/openflow.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -31,6 +32,8 @@ struct pvconn;
 struct pvconn_class;
 struct vconn;
 struct vconn_class;
+struct ofputil_flow_stats;
+struct ofputil_flow_stats_request;
 
 void vconn_usage(bool active, bool passive, bool bootstrap);
 
@@ -56,6 +59,10 @@ int vconn_transact_noreply(struct vconn *, struct ofpbuf *, 
struct ofpbuf **);
 int vconn_transact_multiple_noreply(struct vconn *, struct ovs_list *requests,
 struct ofpbuf **replyp);
 
+int vconn_dump_flows(struct vconn *, const struct ofputil_flow_stats_request *,
+ enum ofputil_protocol,
+ struct ofputil_flow_stats **fsesp, size_t *n_fsesp);
+
 /* Bundle errors must be free()d by the caller. */
 struct vconn_bundle_error {
 struct ovs_list list_node;
diff --git a/lib/vconn.c b/lib/vconn.c
index d5a17f6..57cf429 100644
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008-2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -943,6 +943,125 @@ vconn_transact_multiple_noreply(struct vconn *vconn, 
struct ovs_list *requests,
 return 0;
 }
 
+static int
+recv_flow_stats_reply(struct vconn *vconn, ovs_be32 send_xid,
+  struct ofpbuf **replyp,
+  struct ofputil_flow_stats *fs, struct ofpbuf *ofpacts)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+struct ofpbuf *reply = *replyp;
+
+for (;;) {
+int retval;
+bool more;
+
+/* Get a flow stats reply message, if we don't already have one. */
+if (!reply) {
+enum ofptype type;
+enum ofperr error;
+
+do {
+error = vconn_recv_block(vconn, &reply);
+if (error) {
+return error;
+}
+} while (((struct ofp_header *) reply->data)->xid != send_xid);
+
+error = ofptype_decode(&type, reply->data);
+if (error || type != OFPTYPE_FLOW_STATS_REPLY) {
+VLOG_WARN_RL(&rl, "received bad reply: %s",
+ ofp_to_string(reply->data, reply->size, 1));
+return EPROTO;
+}
+}
+
+/* Pull an individual flow stats reply out of the message. */
+retval = ofputil_decode_flow_stats_reply(fs, reply, false, ofpacts);
+switch (retval) {
+case 0:
+*replyp = reply;
+return 0;
+
+case EOF:
+more = ofpmp_more(reply->header);
+ofpbuf_delete(reply);
+reply = NULL;
+if (!more) {
+*replyp = NULL;
+

[ovs-dev] [PATCH v4 1/2] ovn-controller: Tie OpenFlow and logical flows using OpenFlow cookie.

[Ben Pfaff]

This makes it easy to find the logical flow that generated a particular
OpenFlow flow, by running "ovn-sbctl dump-flows ".

Later, this can be refined (and automated for "ofproto/trace"), but this
is still a significant advance.

Signed-off-by: Ben Pfaff 
---
 lib/uuid.c   |  22 +++-
 lib/uuid.h   |   3 +-
 ovn/controller/lflow.c   |   6 +-
 ovn/controller/ofctrl.c  |   6 +-
 ovn/controller/ofctrl.h  |   2 +-
 ovn/controller/physical.c|  38 +++--
 ovn/utilities/ovn-sbctl.8.in |  13 -
 ovn/utilities/ovn-sbctl.c| 129 ---
 8 files changed, 172 insertions(+), 47 deletions(-)

diff --git a/lib/uuid.c b/lib/uuid.c
index 0f2a58e..bd98d40 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
+/* Copyright (c) 2008, 2009, 2010, 2011, 2013, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -210,6 +210,26 @@ error:
 uuid_zero(uuid);
 return false;
 }
+
+/* Returns the number of characters at the beginning of 's' that are valid for
+ * a UUID.  For example, the "123" at the beginning of "123xyzzy" could begin a
+ * UUID, so uuid_is_partial_string() would return 3; for "xyzzy", this function
+ * would return 0, since "x" can't start a UUID. */
+int
+uuid_is_partial_string(const char *s)
+{
+static const char tmpl[UUID_LEN] = "----";
+size_t i;
+for (i = 0; i < UUID_LEN; i++) {
+if (tmpl[i] == 'x'
+? hexit_value(s[i]) < 0
+: s[i] != '-') {
+break;
+}
+}
+return i;
+}
+
 
 static void
 sha1_update_int(struct sha1_ctx *sha1_ctx, uintmax_t x)
diff --git a/lib/uuid.h b/lib/uuid.h
index a5792b0..113574c 100644
--- a/lib/uuid.h
+++ b/lib/uuid.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2008, 2009, 2010 Nicira, Inc.
+/* Copyright (c) 2008, 2009, 2010, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -61,6 +61,7 @@ bool uuid_is_zero(const struct uuid *);
 int uuid_compare_3way(const struct uuid *, const struct uuid *);
 bool uuid_from_string(struct uuid *, const char *);
 bool uuid_from_string_prefix(struct uuid *, const char *);
+int uuid_is_partial_string(const char *);
 void uuid_set_bits_v4(struct uuid *);
 
 #endif /* uuid.h */
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index d913998..e3c99ad 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -266,7 +266,7 @@ consider_logical_flow(const struct lport_index *lports,
 }
 if (!m->n) {
 ofctrl_add_flow(flow_table, ptable, lflow->priority, &m->match,
-&ofpacts);
+lflow->header_.uuid.parts[0], &ofpacts);
 } else {
 uint64_t conj_stubs[64 / 8];
 struct ofpbuf conj;
@@ -282,7 +282,7 @@ consider_logical_flow(const struct lport_index *lports,
 dst->n_clauses = src->n_clauses;
 }
 ofctrl_add_flow(flow_table, ptable, lflow->priority, &m->match,
-&conj);
+0, &conj);
 ofpbuf_uninit(&conj);
 }
 }
@@ -350,7 +350,7 @@ consider_neighbor_flow(const struct lport_index *lports,
 uint64_t stub[1024 / 8];
 struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
 put_load(mac.ea, sizeof mac.ea, MFF_ETH_DST, 0, 48, &ofpacts);
-ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, &match, &ofpacts);
+ofctrl_add_flow(flow_table, OFTABLE_MAC_BINDING, 100, &match, 0, &ofpacts);
 ofpbuf_uninit(&ofpacts);
 }
 
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 1d8fbf3..7449293 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -57,6 +57,7 @@ struct ovn_flow {
 /* Data. */
 struct ofpact *ofpacts;
 size_t ofpacts_len;
+uint64_t cookie;
 };
 
 static uint32_t ovn_flow_hash(const struct ovn_flow *);
@@ -602,7 +603,8 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type)
 void
 ofctrl_add_flow(struct hmap *desired_flows,
 uint8_t table_id, uint16_t priority,
-const struct match *match, const struct ofpbuf *actions)
+const struct match *match, uint64_t cookie,
+const struct ofpbuf *actions)
 {
 struct ovn_flow *f = xmalloc(sizeof *f);
 f->table_id = table_id;
@@ -611,6 +613,7 @@ ofctrl_add_flow(struct hmap *desired_flows,
 f->ofpacts = xmemdup(actions->data, actions->size);
 f->ofpacts_len = actions->size;
 f->hmap_node.hash = ovn_flow_hash(f);
+f->cookie = cookie;
 
 if (ovn_flow_lookup(desired_flows, f)) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);

[ovs-dev] [PATCH v4 0/2] Clearly relate OVN and OpenFlow flows.

[Ben Pfaff]

v1->v2:
  - Patch 1 in v1 was committed and therefore dropped from the series.
  - Patch 2 in v2 is new.

v2->v3:
  - Rebase to fix patch rejects.

v3->v4:
  - Rebase to fix patch rejects.

Ben Pfaff (2):
  ovn-controller: Tie OpenFlow and logical flows using OpenFlow cookie.
  ovn-trace: New --ovs option to also print OpenFlow flows.

 NEWS  |   4 +-
 include/openvswitch/vconn.h   |  15 +++--
 lib/uuid.c|  22 +-
 lib/uuid.h|   3 +-
 lib/vconn.c   | 121 -
 ovn/controller/lflow.c|   6 +-
 ovn/controller/ofctrl.c   |   6 +-
 ovn/controller/ofctrl.h   |   2 +-
 ovn/controller/physical.c |  38 ++-
 ovn/utilities/ovn-sbctl.8.in  |  13 +++-
 ovn/utilities/ovn-sbctl.c | 129 +--
 ovn/utilities/ovn-trace.8.xml |  61 -
 ovn/utilities/ovn-trace.c |  91 -
 utilities/ovs-ofctl.c | 152 +-
 14 files changed, 487 insertions(+), 176 deletions(-)

-- 
2.10.2

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

Re: [ovs-dev] [PATCH] ovsdb-data: Add support for integer ranges in database commands

[Ben Pfaff]

On Wed, Dec 14, 2016 at 10:39:35PM +0100, Łukasz Rząsik wrote:
> I was not worrying about this problem because I assumed it was already
> there. A user could provide a very long list of integers and this would
> affect memory in similar way. There is a limit for how long the list could
> get, kernel's limit on size of arguments, so in reality the list would not
> get that long but this is outside of OVS control.
> 
> Of course with the change it's much easier for a user to specify something
> that will explode OVS memory.

That's the issue.  Of course a user could provide a long list before,
but it was obvious to him that he was doing so.  Now, it's trivial and
can be done without intent.

> I would propose three possible solutions, starting from the easiest and
> simplest:
> 1. Specify an arbitrary limit for size of a range. If a user specifies a
> range bigger than the limit, OVS will abort the command and inform the user
> about the limit. Actually most of the columns in the database already have
> a limit. Quickly looking through the schema I found only one without the
> limit: cfm_remote_mpids in Interface table.
> 2. Internally split very big ranges into smaller ones and commit each range
> separately.
> 3. Create a new struct, e.g. ovsdb_atom_range. Instead of adding every atom
> separately to datum, add the struct, send it to the database and handle it
> on the database side.
> 
> What do you think about it? Maybe I'm missing something? Do you have a
> solution in mind?

I think a simple way to start out would be to introduce an arbitrary
4096-element maximum.  That covers the common networking use case of a
set of VLANs.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: fix some typos

[Ben Pfaff]

On Wed, Dec 14, 2016 at 04:37:16PM -0500, Lance Richardson wrote:
> s/deamon/daemon/
> s/dependant/dependent/
> 
> Signed-off-by: Lance Richardson 

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

Re: [ovs-dev] [PATCH 5/5] datapath-windows: Conntrack disable type truncation warning

[Guru Shetty]

On 5 December 2016 at 07:39, Alin Serdean 
wrote:

> Compiling with the WDK 10 gave the following warning:
> Warning C4311   'type cast': pointer truncation from 'POVS_CT_ENTRY' to
> 'UINT32'
> ovsext (OVSExt\ovsext)  Conntrack.c 1139
>
> This patch disables the warning on the file Conntrack.c.
>
> Signed-off-by: Alin Gabriel Serdean 
>

Would you mind reposting this? It gives me rejects.


> ---
>  datapath-windows/ovsext/Conntrack.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/datapath-windows/ovsext/Conntrack.c
> b/datapath-windows/ovsext/Conntrack.c
> index 84c4091..bdcf42c 100644
> --- a/datapath-windows/ovsext/Conntrack.c
> +++ b/datapath-windows/ovsext/Conntrack.c
> @@ -25,6 +25,9 @@
>  #include "Debug.h"
>  #include "Event.h"
>
> +#pragma warning( push )
> +#pragma warning( disable:4311 )
> +
>  #define WINDOWS_TICK 1000
>  #define SEC_TO_UNIX_EPOCH 11644473600LL
>  #define SEC_TO_NANOSEC 10LL
> @@ -1266,3 +1269,5 @@ OvsCtDumpCmdHandler(POVS_USER_PARAMS_CONTEXT
> usrParamsCtx,
>
>  return STATUS_SUCCESS;
>  }
> +
> +#pragma warning( pop )
> --
> 2.10.2.windows.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 2/5] datapath-windows: Add Windows 10 family to solution

[Guru Shetty]

On 5 December 2016 at 07:39, Alin Serdean 
wrote:

> This patch adds two more compiling targets:
>   - one for Windows 10 release
>   - one for Windows 10 Debug
>
> The new targets are flagged properly to use the new Windows 10 kernel mode
> driver and its toolchain.
>
> Signed-off-by: Alin Gabriel Serdean 
>

Would you mind reposting this patch. It gives me rejects.


> ---
>  datapath-windows/Package/package.VcxProj  | 32 
>  datapath-windows/Package/package.VcxProj.user |  8 +++-
>  datapath-windows/ovsext.sln   | 18 -
>  datapath-windows/ovsext/ovsext.vcxproj| 54
> +++
>  datapath-windows/ovsext/ovsext.vcxproj.user   |  8 +++-
>  5 files changed, 116 insertions(+), 4 deletions(-)
>
> diff --git a/datapath-windows/Package/package.VcxProj
> b/datapath-windows/Package/package.VcxProj
> index 0d48163..1a0da97 100644
> --- a/datapath-windows/Package/package.VcxProj
> +++ b/datapath-windows/Package/package.VcxProj
> @@ -1,6 +1,14 @@
>  
>  http://schemas.
> microsoft.com/developer/msbuild/2003">
>
> +
> +  Win10 Debug
> +  x64
> +
> +
> +  Win10 Release
> +  x64
> +
>  
>Win8.1 Debug
>x64
> @@ -38,6 +46,13 @@
>  true
>  WindowsKernelModeDriver8.1
>
> +  
> +
> +
> +true
> +WindowsKernelModeDriver10.0
> +Desktop
> +  
>
>  Windows8
>  true
> @@ -48,6 +63,13 @@
>  false
>  WindowsKernelModeDriver8.1
>
> +  
> +
> +
> +false
> +WindowsKernelModeDriver10.0
> +Universal
> +  
>
>  Windows8
>  false
> @@ -88,6 +110,11 @@
>true
>  
>
> +  
> +
> +  true
> +
> +  
>
>  
>true
> @@ -98,6 +125,11 @@
>true
>  
>
> +  
> +
> +  true
> +
> +  
>
>  
>   Condition="'@(Inf)'!=''" />
> diff --git a/datapath-windows/Package/package.VcxProj.user
> b/datapath-windows/Package/package.VcxProj.user
> index 7169f02..891fbc0 100644
> --- a/datapath-windows/Package/package.VcxProj.user
> +++ b/datapath-windows/Package/package.VcxProj.user
> @@ -6,10 +6,16 @@
>
>  TestSign
>
> +  
> +TestSign
> +  
>
>  TestSign
>
>
>  TestSign
>
> -
> +  
> +TestSign
> +  
> +
> \ No newline at end of file
> diff --git a/datapath-windows/ovsext.sln b/datapath-windows/ovsext.sln
> index 60e9318..831db89 100644
> --- a/datapath-windows/ovsext.sln
> +++ b/datapath-windows/ovsext.sln
> @@ -1,6 +1,6 @@
>  Microsoft Visual Studio Solution File, Format Version 12.00
> -# Visual Studio 2013
> -VisualStudioVersion = 12.0.31101.0
> +# Visual Studio 14
> +VisualStudioVersion = 14.0.25420.1
>  MinimumVisualStudioVersion = 10.0.40219.1
>  Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Package",
> "Package", "{6BA8554E-AE50-49B0-9C98-4592447FEF8D}"
>  EndProject
> @@ -12,12 +12,20 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") =
> "ovsext", "ovsext\ovsext.vcx
>  EndProject
>  Global
> GlobalSection(SolutionConfigurationPlatforms) = preSolution
> +   Win10Debug|x64 = Win10Debug|x64
> +   Win10Release|x64 = Win10Release|x64
> Win8.1Debug|x64 = Win8.1Debug|x64
> Win8.1Release|x64 = Win8.1Release|x64
> Win8Debug|x64 = Win8Debug|x64
> Win8Release|x64 = Win8Release|x64
> EndGlobalSection
> GlobalSection(ProjectConfigurationPlatforms) = postSolution
> +   
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Debug|x64.ActiveCfg
> = Win10 Debug|x64
> +   {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Debug|x64.Build.0
> = Win10 Debug|x64
> +   {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Debug|x64.Deploy.0
> = Win10 Debug|x64
> +   
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Release|x64.ActiveCfg
> = Win10 Release|x64
> +   
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Release|x64.Build.0
> = Win10 Release|x64
> +   
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win10Release|x64.Deploy.0
> = Win10 Release|x64
> 
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Debug|x64.ActiveCfg
> = Win8.1 Debug|x64
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Debug|x64.Build.0
> = Win8.1 Debug|x64
> 
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8.1Release|x64.ActiveCfg
> = Win8.1 Release|x64
> @@ -26,6 +34,12 @@ Global
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Debug|x64.Build.0
> = Win8 Debug|x64
> 
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Release|x64.ActiveCfg
> = Win8 Release|x64
> {911D7389-3E61-449F-B8F3-14AD7EE9A0F2}.Win8Release|x64.Build.0
> = Win8 Release|x64
> +   
> {63FE215D-98BE-4440-8081-C6160EFB80FA}.Win10Debug|x64.ActiveCfg
> = Win10 Debug|x64
> +   {63FE215D-98BE-4440-8081-C6160EFB80FA}.Win10Debug|x64.Build.0
> = Win10 Deb

Re: [ovs-dev] [PATCH] ovn-northd: fix monitor process naming

[Ben Pfaff]

On Wed, Dec 14, 2016 at 11:16:53AM -0500, Lance Richardson wrote:
> Currently the ovn-northd monitor process and the ovn-northd process
> have the same name, e.g. ps -ef | grep northd shows (edited for space):
> 
> ... ovn-northd --detach --monitor --log-file=ovn-northd.log --pidfile
> ... ovn-northd --detach --monitor --log-file=ovn-northd.log --pidfile
> 
> With the call to ovs_cmdl_proctitle_init() added, we have:
> 
> ... ovn-northd: monitoring pid 15662 (healthy)
> ... ovn-northd --detach --monitor --log-file=ovn-northd.log --pidfile
> 
> Signed-off-by: Lance Richardson 

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

Re: [ovs-dev] [PATCH v2 0/3] ovn: support ssl connections to nb/sb dbs

[Ben Pfaff]

On Thu, Dec 08, 2016 at 01:26:41PM -0500, Lance Richardson wrote:
> > From: "Lance Richardson" 
> > To: d...@openvswitch.org, b...@ovn.org, russ...@ovn.org, nusid...@redhat.com
> > Sent: Thursday, December 8, 2016 1:12:22 PM
> > Subject: [ovs-dev] [PATCH v2 0/3] ovn: support ssl connections to nb/sb dbs
> > 
> > This series adds support for SSL connections to the northbound
> > and southbound OVN database servers and removes the previous
> > default TCP connection type.
> > 
> > 
> > v2: - Changed DB_NB_DEFAULT_REMOTE to DB_NB_CREATE_REMOTE.
> > - Changed DB_SB_DEFAULT_REMOTE to DB_SB_CREATE_REMOTE.
> > - Create default remote configuration in db instead of
> >   via command-line options.
> 
> Forgot to mention:
>   - Added support for specifying poll interval when creating
> remote via ovn-ctl.

Thanks.

I applied patches 1 and 2.  Patch 3 seems to have patch rejects, so
please rebase and repost.

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

Re: [ovs-dev] [PATCH] datapath-windows: Remove dead code from PacketIO

[Ben Pfaff]

On Thu, Dec 08, 2016 at 05:57:19PM +, Alin Serdean wrote:
> Assigning value to 'nativeNbls' has no effect outside the function and
> the variable is not used inside the function.
> 
> Signed-off-by: Alin Gabriel Serdean 

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

Re: [ovs-dev] [PATCH] nx-match: Only store significant bytes to stack.

[Ben Pfaff]

On Wed, Dec 07, 2016 at 04:49:00PM -0800, Jarno Rajahalme wrote:
> Always storing the maximum mf_value size wastes about 120 bytes for
> each stack entry.  This patch changes the stack from an mf_value array
> to a string of value-length pairs.
> 
> The lenght is stored after the value so that the stack pop may first

s/lenght/length/

> read the length and then the appropriate number of bytes.  Iterating
> the stack in the reverse order is not possible, so some code needs to
> reverse the stack first to then traverse the stack.  This could be
> avoided by storing the length of the value both before and after it.
> 
> Signed-off-by: Jarno Rajahalme 

I don't think it's worth reversing the stack to serialize for
continuations.  This is part of the "private" part of the continuation
that users are not supposed to look at, so we can put it in any order we
like.  Also, for the sake of printing it for debugging purposes, we can
also print it in any order we like; starting from top-of-stack is not a
problem, although it might be nice to put (top) and (bottom) indicators
in the output.

I'd be more comfortable if nx_stack_pop() had assertions to check for
underflow.

In ofputil_decode_packet_in_private(), it's probably worth checking the
format of the stack we pull from the payload, since a badly formatted
stack can segfault us (if we leave out assertions) or assert-fail us (if
we include them).

It'd be nice to document the stack format somewhere in a comment.

Thanks,

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

Re: [ovs-dev] [PATCH] odp: Use struct in6_addr for IPv6 addresses.

[Ben Pfaff]

On Wed, Dec 07, 2016 at 04:48:23PM -0800, Jarno Rajahalme wrote:
> Code is simplified when the ODP keys use the same type as the struct
> flow for the IPv6 addresses.  As the change is facilitated by
> extract-odp-netlink-h, this change only affects the userspace.  We
> already do the same for the ethernet addresses.
> 
> Signed-off-by: Jarno Rajahalme 

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

[ovs-dev] [PATCH] system-traffic: Fix clone test.

[William Tu]

The existing clone test fails the system testsuite.  The patch provides
fix, removes the unused at_ns2, and uses "ovs-ofctl monitor" to validate
the packet contents after actions inside a clone.

Signed-off-by: William Tu 
---
 tests/system-common-macros.at |  6 ++
 tests/system-traffic.at   | 25 ++---
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index d41de23..64640b4 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -204,6 +204,12 @@ m4_define([ADD_NATIVE_TUNNEL],
 #
 m4_define([FORMAT_PING], [grep "transmitted" | sed 's/time.*ms$/time 0ms/'])
 
+# STRIP_MONITOR_CSUM([])
+#
+# Strip the csum value from ovs-ofctl monitor.
+#
+m4_define([STRIP_MONITOR_CSUM], [grep "csum:" | sed 's/csum:.*/csum: /'])
+
 # FORMAT_CT([ip-addr])
 #
 # Strip content from the piped input which would differ from test to test
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index d70c5c3..7f55783 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -344,24 +344,27 @@ ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)
 
 ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
 ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
-ADD_VETH(p2, at_ns2, br0, "10.1.1.3/24")
 
-AT_CHECK([ovs-vsctl -- set interface ovs-p0 ofport_request=1])
-AT_CHECK([ovs-vsctl -- set interface ovs-p1 ofport_request=2])
-AT_CHECK([ovs-vsctl -- set interface ovs-p2 ofport_request=3])
+AT_CHECK([ovs-vsctl -- set interface ovs-p0 ofport_request=1 \
+-- set interface ovs-p1 ofport_request=2])
 
-dnl verify that the clone(...) won't affect the original packet, so ping still 
works OK
-dnl without 'output' in 'clone()'
-AT_CHECK([ovs-ofctl add-flow br0 
"in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
 output:2"])
-dnl with 'output' in 'clone()'
-AT_CHECK([ovs-ofctl add-flow br0 
"in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
 output:3), output:1"])
+AT_DATA([flows.txt], [dnl
+priority=1 actions=NORMAL
+priority=10 
in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
 output:2
+priority=10 
in_port=2,ip,actions=clone(mod_dl_src(ae:c6:7e:54:8d:4d),mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
 controller), output:1
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
+AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir 
--pidfile 2> ofctl_monitor.log])
 NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
[0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])
 
-NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py]], [http0.pid])
-NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o 
wget0.log])
+AT_CHECK([cat ofctl_monitor.log | STRIP_MONITOR_CSUM], [0], [dnl
+icmp,vlan_tci=0x,dl_src=ae:c6:7e:54:8d:4d,dl_dst=50:54:00:00:00:0b,nw_src=10.1.1.2,nw_dst=192.168.4.4,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=0,icmp_code=0
 icmp_csum: 
+icmp,vlan_tci=0x,dl_src=ae:c6:7e:54:8d:4d,dl_dst=50:54:00:00:00:0b,nw_src=10.1.1.2,nw_dst=192.168.4.4,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=0,icmp_code=0
 icmp_csum: 
+icmp,vlan_tci=0x,dl_src=ae:c6:7e:54:8d:4d,dl_dst=50:54:00:00:00:0b,nw_src=10.1.1.2,nw_dst=192.168.4.4,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=0,icmp_code=0
 icmp_csum: 
+])
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] ovn-trace: Fix small error condition memory leak in trace().

[Ben Pfaff]

On Wed, Dec 21, 2016 at 02:04:32PM -0800, Justin Pettit wrote:
> Signed-off-by: Justin Pettit 
> ---
>  ovn/utilities/ovn-trace.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> index 71798f8..6d8e514 100644
> --- a/ovn/utilities/ovn-trace.c
> +++ b/ovn/utilities/ovn-trace.c
> @@ -1447,7 +1447,9 @@ trace(const char *dp_s, const char *flow_s)
>  char *error = expr_parse_microflow(flow_s, &symtab, &address_sets,
> ovntrace_lookup_port, dp, &uflow);
>  if (error) {
> -return xasprintf("error parsing flow: %s\n", error);
> +char *s = xasprintf("error parsing flow: %s\n", error);
> +free(error);
> +return s;
>  }

I've often wished for a %s-like format specifier that frees the string
after it prints it.

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

Re: [ovs-dev] [PATCH 2/2] ovn: Move a couple files into ovn/docs/.

[Ben Pfaff]

On Wed, Dec 07, 2016 at 02:28:14PM -0500, Russell Bryant wrote:
> Signed-off-by: Russell Bryant 

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

Re: [ovs-dev] [PATCH 1/2] ovn: Document upgrade procedure.

[Ben Pfaff]

On Wed, Dec 07, 2016 at 02:28:13PM -0500, Russell Bryant wrote:
> Signed-off-by: Russell Bryant 

This procedure isn't what I expected.  It recommends:

1. Upgrade SB/NB databases and northd.
2. Upgrade ovn-controller.
3. Upgrade integration.

I think that this is likely to cause problems in the common case,
because the new northd is likely to try to start using features that are
not yet available on the hypervisors, such as new logical match fields
and actions.

I expected:

1. Upgrade ovn-controller.
2. Upgrade SB/NB databases and northd.
3. Upgrade integration.

(I don't think that it matters when the databases are upgraded, though,
as long as they're upgraded before northd.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovn-trace: Fix small error condition memory leak in trace().

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 ovn/utilities/ovn-trace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 71798f8..6d8e514 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1447,7 +1447,9 @@ trace(const char *dp_s, const char *flow_s)
 char *error = expr_parse_microflow(flow_s, &symtab, &address_sets,
ovntrace_lookup_port, dp, &uflow);
 if (error) {
-return xasprintf("error parsing flow: %s\n", error);
+char *s = xasprintf("error parsing flow: %s\n", error);
+free(error);
+return s;
 }
 
 uint32_t in_key = uflow.regs[MFF_LOG_INPORT - MFF_REG0];
-- 
1.9.1

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

[ovs-dev] [PATCH] ofproto-dpif-upcall: Remove OVS_UNUSED from upcall_unixctl_set_flow_limit() arg.

[Justin Pettit]

The 'argv' argument is used.

Signed-off-by: Justin Pettit 
---
 ofproto/ofproto-dpif-upcall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index df12882..32c7730 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2592,7 +2592,7 @@ upcall_unixctl_enable_ufid(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
 static void
 upcall_unixctl_set_flow_limit(struct unixctl_conn *conn,
   int argc OVS_UNUSED,
-  const char *argv[] OVS_UNUSED,
+  const char *argv[],
   void *aux OVS_UNUSED)
 {
 struct ds ds = DS_EMPTY_INITIALIZER;
-- 
1.9.1

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

Re: [ovs-dev] splitting commits in Git (was: Re: Sync on PTAP, EXT-382 and NSH)

[Ben Pfaff]

On Wed, Dec 21, 2016 at 10:06:29AM -0800, Jarno Rajahalme wrote:
> 
> > On Dec 21, 2016, at 9:30 AM, Ben Pfaff  wrote:
> > 
> > There was a question about splitting up large commits in Git.  I do this
> > routinely and I volunteered to find some resources.
> > 
> > The "git-rebase" manpage has a section titled "SPLITTING COMMITS" that
> > is a good place to start.  It does not cover very well how to commit
> > parts of a file rather than a whole file, which is something that I do
> > often.  For this, I use "git citool" and then select lines to commit and
> > right-click to "stage lines for commit".  I know that there's a mode for
> > the "git commit" CLI that can do this too, but I've always found that
> > hard to use compared to "git citool”.
> 
> You mean “git add -p”? I’ve used that, but was not aware of “git citool”…

I think I use "git rebase -i" and "git citool" more than any other git
commands.  (I use "git am" a lot too but I have a mutt shortcut for
that.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/4] doc: Rework DPDK guide

[Ben Pfaff]

On Wed, Dec 14, 2016 at 10:07:56AM +, Stephen Finucane wrote:
> As promised, this series reworks the largest of the installation guides
> to suit the new structure of the documentation. A couple of related
> patches are included to build upon this.
> 
> Changes since v2:
> - Rebase onto master
> 
> Stephen Finucane (4):
>   doc: Split dpdk, dpdk-advanced into multiple docs
>   doc: Move testing to testing section
>   doc: Document Patchwork instance
>   gitignore: Ignore venv

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

Re: [ovs-dev] [PATCH v3] ovn-controller: Fix duplicated flow add attempts in table 32.

[Darrell Ball]

There is another comment about the test below

From: Han Zhou 
Date: Wednesday, December 21, 2016 at 1:50 PM
To: Darrell Ball 
Cc: "d...@openvswitch.org" 
Subject: Re: [ovs-dev] [PATCH v3] ovn-controller: Fix duplicated flow add 
attempts in table 32.

oops, I updated the head line in v3, but forgot to update this message. I am 
very sorry about that.

On Wed, Dec 21, 2016 at 1:45 PM, Darrell Ball 
mailto:db...@vmware.com>> wrote:


On 12/21/16, 12:32 PM, 
"ovs-dev-boun...@openvswitch.org on 
behalf of Han Zhou" 
mailto:ovs-dev-boun...@openvswitch.org> on 
behalf of zhou...@gmail.com> wrote:

In commit 475f0a2c it introduced a priority 150 flow for filtering
the sending of traffic received from vxlan tunnels back out tunnels.
However, it added the flow for every remote port processing, which
results in continuous logs about duplicated flows. We only need to
install this flow once per physical_run() loop iteration.

I thought we covered this part in the last version.

s/However, it added the flow for every remote port processing, which
results in continuous logs about duplicated flows. We only need to
install this flow once per physical_run() loop iteration.
/
However, it attempted to add the flow for every remote port processing,
which results in continuous logs about duplicated flow add attempts. We
only need to attempt to install this flow once per physical_run() loop 
iteration.
/

Signed-off-by: Han Zhou mailto:zhou...@gmail.com>>
Acked-by: Darrell Ball mailto:db...@vmware.com>>
---

Notes:
v1 -> v2: update commit message according to Darrell's comments.
v2 -> v3: update test case.

 ovn/controller/physical.c | 47 
---
 
tests/ovn.at
  |  5 +
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 48adb78..3b653dd 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
 } else {
 /* Remote port connected by tunnel */

-/* Table 32, priority 150 and 100.
+/* Table 32, priority 100.
  * ===
  *
- * Priority 150 is for packets received from a VXLAN tunnel
- * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
- * lack of needed metadata in VXLAN, explicitly skip sending
- * back out any tunnels and resubmit to table 33 for local
- * delivery.
- *
- * Priority 100 is for all other traffic which need to be sent
- * to a remote hypervisor.  Each flow matches an output port
- * that includes a logical port on a remote hypervisor, and
- * tunnels the packet to that hypervisor.
+ * Priority 100 is for traffic that needs to be sent to a remote
+ * hypervisor.  Each flow matches an output port that includes a
+ * logical port on a remote hypervisor, and tunnels the packet to
+ * that hypervisor.
  */
 match_init_catchall(&match);
 ofpbuf_clear(ofpacts_p);
-match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
- MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
-
-/* Resubmit to table 33. */
-put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
-ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match,
-ofpacts_p);
-
-
-match_init_catchall(&match);
-ofpbuf_clear(ofpacts_p);

 /* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
 match_set_metadata(&match, htonll(dp_key));
@@ -870,12 +853,30 @@ physical_run(struct controller_ctx *ctx, enum 
mf_field_id mff_ovn_geneve,
 }
 }

+/* Table 32, priority 150.
+ * ===
+ *
+ * Priority 150 is for packets received from a VXLAN tunnel
+ * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
+ * lack of needed metadata in VXLAN, explicitly skip sending
+ * back out any tunnels and resubmit to table 33 for local
+ * delivery.
+ */
+struct match match;
+match_init_catchall(&match);
+ofpbuf_clear(&ofpacts);
+match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
+ MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
+
+/* Resubmit to table 33

Re: [ovs-dev] [PATCH v3] ovn-controller: Fix duplicated flow add attempts in table 32.

[Han Zhou]

oops, I updated the head line in v3, but forgot to update this message. I
am very sorry about that.

On Wed, Dec 21, 2016 at 1:45 PM, Darrell Ball  wrote:

>
>
> On 12/21/16, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of Han
> Zhou" 
> wrote:
>
> In commit 475f0a2c it introduced a priority 150 flow for filtering
> the sending of traffic received from vxlan tunnels back out tunnels.
> However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
>
> I thought we covered this part in the last version.
>
> s/However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
> /
> However, it attempted to add the flow for every remote port processing,
> which results in continuous logs about duplicated flow add attempts. We
> only need to attempt to install this flow once per physical_run() loop
> iteration.
> /
>
> Signed-off-by: Han Zhou 
> Acked-by: Darrell Ball 
> ---
>
> Notes:
> v1 -> v2: update commit message according to Darrell's comments.
> v2 -> v3: update test case.
>
>  ovn/controller/physical.c | 47 --
> -
>  tests/ovn.at  |  5 +
>  2 files changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index 48adb78..3b653dd 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id
> mff_ovn_geneve,
>  } else {
>  /* Remote port connected by tunnel */
>
> -/* Table 32, priority 150 and 100.
> +/* Table 32, priority 100.
>   * ===
>   *
> - * Priority 150 is for packets received from a VXLAN tunnel
> - * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due
> to
> - * lack of needed metadata in VXLAN, explicitly skip sending
> - * back out any tunnels and resubmit to table 33 for local
> - * delivery.
> - *
> - * Priority 100 is for all other traffic which need to be sent
> - * to a remote hypervisor.  Each flow matches an output port
> - * that includes a logical port on a remote hypervisor, and
> - * tunnels the packet to that hypervisor.
> + * Priority 100 is for traffic that needs to be sent to a
> remote
> + * hypervisor.  Each flow matches an output port that
> includes a
> + * logical port on a remote hypervisor, and tunnels the
> packet to
> + * that hypervisor.
>   */
>  match_init_catchall(&match);
>  ofpbuf_clear(ofpacts_p);
> -match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
> - MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
> -
> -/* Resubmit to table 33. */
> -put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
> -ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150,
> &match,
> -ofpacts_p);
> -
> -
> -match_init_catchall(&match);
> -ofpbuf_clear(ofpacts_p);
>
>  /* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
>  match_set_metadata(&match, htonll(dp_key));
> @@ -870,12 +853,30 @@ physical_run(struct controller_ctx *ctx, enum
> mf_field_id mff_ovn_geneve,
>  }
>  }
>
> +/* Table 32, priority 150.
> + * ===
> + *
> + * Priority 150 is for packets received from a VXLAN tunnel
> + * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
> + * lack of needed metadata in VXLAN, explicitly skip sending
> + * back out any tunnels and resubmit to table 33 for local
> + * delivery.
> + */
> +struct match match;
> +match_init_catchall(&match);
> +ofpbuf_clear(&ofpacts);
> +match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
> + MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
> +
> +/* Resubmit to table 33. */
> +put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
> +ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match,
> &ofpacts);
> +
>  /* Table 32, Priority 0.
>   * ===
>   *
>   * Resubmit packets that are not directed at tunnels or part of a
>   * multicast group to the local output table. */
> -struct match match;
>  match_init_catchall(&match);
>  ofpbuf_clear(&ofpacts)

Re: [ovs-dev] [PATCH 1/5] doc: Add vlan cookbook from website

[Ben Pfaff]

On Sat, Dec 17, 2016 at 10:24:04PM +, Stephen Finucane wrote:
> This is mostly copied verbatim.
> 
> Signed-off-by: Stephen Finucane 

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

Re: [ovs-dev] [PATCH 3/6] doc: Add info on building documentation

[Ben Pfaff]

On Sat, Dec 17, 2016 at 10:23:39PM +, Stephen Finucane wrote:
> I know how to do this, but does anyone else? Let's make it obvious and
> ease the cognitive load on the great folks writing docs.
> 
> Links to the various packaging guides, previously missing, are included
> on the main page.
> 
> Signed-off-by: Stephen Finucane 

I had to rewind the master branch to just before this patch, because it
adds a dependency on "requirements.txt" that isn't anywhere in the tree:

> @@ -68,7 +69,8 @@ EXTRA_DIST += \
>   Documentation/internals/contributing/coding-style.rst \
>   Documentation/internals/contributing/coding-style-windows.rst \
>   Documentation/internals/contributing/documentation-style.rst \
> - Documentation/internals/contributing/submitting-patches.rst
> + Documentation/internals/contributing/submitting-patches.rst \
> + requirements.txt

I'm happy to re-apply this patch and the rest of the series when that's
resolved.

Thanks,

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

Re: [ovs-dev] [PATCH v3] ovn-controller: Fix duplicated flow add attempts in table 32.

[Darrell Ball]

On 12/21/16, 12:32 PM, "ovs-dev-boun...@openvswitch.org on behalf of Han Zhou" 
 wrote:

In commit 475f0a2c it introduced a priority 150 flow for filtering
the sending of traffic received from vxlan tunnels back out tunnels.
However, it added the flow for every remote port processing, which
results in continuous logs about duplicated flows. We only need to
install this flow once per physical_run() loop iteration.

I thought we covered this part in the last version.

s/However, it added the flow for every remote port processing, which
results in continuous logs about duplicated flows. We only need to
install this flow once per physical_run() loop iteration.
/
However, it attempted to add the flow for every remote port processing,
which results in continuous logs about duplicated flow add attempts. We
only need to attempt to install this flow once per physical_run() loop 
iteration.
/

Signed-off-by: Han Zhou 
Acked-by: Darrell Ball 
---

Notes:
v1 -> v2: update commit message according to Darrell's comments.
v2 -> v3: update test case.

 ovn/controller/physical.c | 47 
---
 tests/ovn.at  |  5 +
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 48adb78..3b653dd 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
 } else {
 /* Remote port connected by tunnel */
 
-/* Table 32, priority 150 and 100.
+/* Table 32, priority 100.
  * ===
  *
- * Priority 150 is for packets received from a VXLAN tunnel
- * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
- * lack of needed metadata in VXLAN, explicitly skip sending
- * back out any tunnels and resubmit to table 33 for local
- * delivery.
- *
- * Priority 100 is for all other traffic which need to be sent
- * to a remote hypervisor.  Each flow matches an output port
- * that includes a logical port on a remote hypervisor, and
- * tunnels the packet to that hypervisor.
+ * Priority 100 is for traffic that needs to be sent to a remote
+ * hypervisor.  Each flow matches an output port that includes a
+ * logical port on a remote hypervisor, and tunnels the packet to
+ * that hypervisor.
  */
 match_init_catchall(&match);
 ofpbuf_clear(ofpacts_p);
-match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
- MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
-
-/* Resubmit to table 33. */
-put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
-ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match,
-ofpacts_p);
-
-
-match_init_catchall(&match);
-ofpbuf_clear(ofpacts_p);
 
 /* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
 match_set_metadata(&match, htonll(dp_key));
@@ -870,12 +853,30 @@ physical_run(struct controller_ctx *ctx, enum 
mf_field_id mff_ovn_geneve,
 }
 }
 
+/* Table 32, priority 150.
+ * ===
+ *
+ * Priority 150 is for packets received from a VXLAN tunnel
+ * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
+ * lack of needed metadata in VXLAN, explicitly skip sending
+ * back out any tunnels and resubmit to table 33 for local
+ * delivery.
+ */
+struct match match;
+match_init_catchall(&match);
+ofpbuf_clear(&ofpacts);
+match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
+ MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
+
+/* Resubmit to table 33. */
+put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
+ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match, 
&ofpacts);
+
 /* Table 32, Priority 0.
  * ===
  *
  * Resubmit packets that are not directed at tunnels or part of a
  * multicast group to the local output table. */
-struct match match;
 match_init_catchall(&match);
 ofpbuf_clear(&ofpacts);
 put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
diff --git a/tests/ovn.at b/tests/ovn.at
index 628d3c8..b852665 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1058,6 +1058,11 @@ ovn_populate_arp
 # XXX This should be more systematic.
 sleep 1
 
+# Make sure there is no attempt to adding duplicated f

Re: [ovs-dev] [PATCH 1/6] doc: Add some useful tools for doc editing

[Ben Pfaff]

On Sat, Dec 17, 2016 at 10:23:37PM +, Stephen Finucane wrote:
> This has come up on the mailing list. Let's document it!
> 
> Signed-off-by: Stephen Finucane 

Thank you very much!  I applied this series to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] doc: Resolve issues with Windows guide

[Ben Pfaff]

On Wed, Dec 21, 2016 at 08:39:52PM +, Stephen Finucane wrote:
> The formatting of this file was broken in a recent commit. Resolve this
> issue.
> 
> Signed-off-by: Stephen Finucane 
> Fixes: a0c03adff6c2 ("Windows: document multiple NIC support setup")
> Cc: Alin Gabriel Serdean 

Thanks for the patches!  I applied all of them to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] ovn-controller: Fix duplicated flow add attempts in table 32.

[Ben Pfaff]

On Wed, Dec 21, 2016 at 12:32:16PM -0800, Han Zhou wrote:
> In commit 475f0a2c it introduced a priority 150 flow for filtering
> the sending of traffic received from vxlan tunnels back out tunnels.
> However, it added the flow for every remote port processing, which
> results in continuous logs about duplicated flows. We only need to
> install this flow once per physical_run() loop iteration.
> 
> Signed-off-by: Han Zhou 
> Acked-by: Darrell Ball 
> ---
> 
> Notes:
> v1 -> v2: update commit message according to Darrell's comments.
> v2 -> v3: update test case.

Thanks, applied to master.

(I reformatted a couple of comments.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 32.

[Darrell Ball]

From: Han Zhou 
Date: Wednesday, December 21, 2016 at 12:45 PM
To: Darrell Ball 
Cc: "d...@openvswitch.org" 
Subject: Re: [ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 
32.

Hi Darrell,
Sorry for late response. I posted v3 just now.

On Wed, Dec 14, 2016 at 12:19 PM, Darrell Ball 
mailto:db...@vmware.com>> wrote:
>
> ERROR log level is not appropriate since we always filter adding the extra 
> flows anyways
>
> and it amount to extra processing and filtering.
>
> We anyways attempt to re-add every flow for every iteration of the 
> ovn-controller main loop,
>
> not just physical flows; we filter these attempts with “installed_flows”, so 
> we are doing lots
>
> of extra processing normally, at this time.
>

This is not true. There is a temporary flow_table constructed for desired 
flows, which is initiated and destructed in each iteration of main loop. The 
"duplicate flow" reported by the log is in fact for duplications in a single 
iteration. But I agree that ERROR log level is not necessary.
Which part was not true ? I don’t follow.
Here is the full context again in one paragraph.
Consider the case where there are no new flows added/removed from one iteration 
of the main loop
to another. For each iteration of the main loop, desired flows is initially 
empty, which means we add
each and every flow to desired_flows each iteration of the main loop.
The INFO log occurs because we try to add the same flow to desired_flows in one 
iteration
of the loop and we filter the attempt to add the new flow and log a message to 
that effect.
However, there is installed_flows as well, which keeps context across 
iterations.
When the desired_flows is checked against installed_flows which occurs every 
loop iteration,
we find out we already know about each and every flow in desired_flows (i.e. 
they are all
duplicates) so the attempted flow adds from desired_flows to installed_flows 
are all filtered.



>
>
> With the vtep test, the non-physical flows are not as many and diverse. So 
> checking
>
> for attempting to add the same flow in the same iteration of ovn-controller 
> main loop
>
> where the desired_flows log check is made (which is before the 
> installed_flows check)
>
> should help to catch the physical flow duplicate attempts in a given 
> iteration of the main loop.
>
>
>
> If we added the check there, we would be trying to catch some portion of the 
> unnecessary extra
>
> processing and filtering which seems valid and avoid noisy logs, which could 
> obscure other logs ?
>
>
I added the check for the 3 HVs test case, which I think is more appropriate 
since it is basic functionality.
Adding to any simple test is fine. I don’t care which one per se as long as you 
feel comfortable
with that test.
Thanks,
Han
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/4] make ofproto/trace output easier to read

[Ben Pfaff]

On Wed, Dec 21, 2016 at 02:54:06PM -0500, Lance Richardson wrote:
> The new output format is immensely easier to read and understand.
> 
> For the series:
> 
> Acked-by: Lance Richardson 

Thanks!  I've applied these acks to my tree, but I'm going to wait a bit
to see whether others have comments, for patches 2-4.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/4] ofproto: Update access rules for 'flow_cookie'.

[Ben Pfaff]

Thanks!  I applied this patch to master.

On Wed, Dec 21, 2016 at 10:40:23AM -0800, Jarno Rajahalme wrote:
> Acked-by: Jarno Rajahalme 
> 
> > On Dec 20, 2016, at 11:03 PM, Ben Pfaff  wrote:
> > 
> > The 'flow_cookie' member of struct rule is read during flow translation
> > without holding a mutex, breaking the documented OVS_GUARDED access rule.
> > However, the flow_cookie member is actually never modified after a rule is
> > initialized, so this commit changes the access rules to reflect that.
> > 
> > Signed-off-by: Ben Pfaff 
> > ---
> > ofproto/ofproto-provider.h | 4 ++--
> > ofproto/ofproto.c  | 5 +++--
> > 2 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> > index c515779..3739ebc 100644
> > --- a/ofproto/ofproto-provider.h
> > +++ b/ofproto/ofproto-provider.h
> > @@ -365,8 +365,8 @@ struct rule {
> > struct ovs_refcount ref_count;
> > 
> > /* A "flow cookie" is the OpenFlow name for a 64-bit value associated 
> > with
> > - * a flow.. */
> > -ovs_be64 flow_cookie OVS_GUARDED;
> > + * a flow. */
> > +const ovs_be64 flow_cookie; /* Immutable once rule is constructed. */
> > struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex);
> > 
> > enum ofputil_flow_mod_flags flags OVS_GUARDED;
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 4f43c45..58295a1 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -4859,7 +4859,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct 
> > cls_rule *cr,
> > 
> > ovs_mutex_init(&rule->mutex);
> > ovs_mutex_lock(&rule->mutex);
> > -rule->flow_cookie = new_cookie;
> > +*CONST_CAST(ovs_be64 *, &rule->flow_cookie) = new_cookie;
> > rule->created = rule->modified = time_msec();
> > rule->idle_timeout = idle_timeout;
> > rule->hard_timeout = hard_timeout;
> > @@ -5098,7 +5098,8 @@ replace_rule_start(struct ofproto *ofproto, struct 
> > ofproto_flow_mod *ofm,
> > new_rule->created = old_rule->created;
> > }
> > if (!change_cookie) {
> > -new_rule->flow_cookie = old_rule->flow_cookie;
> > +*CONST_CAST(ovs_be64 *, &new_rule->flow_cookie)
> > += old_rule->flow_cookie;
> > }
> > ovs_mutex_unlock(&old_rule->mutex);
> > ovs_mutex_unlock(&new_rule->mutex);
> > -- 
> > 2.10.2
> > 
> > ___
> > 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] ovn-controller: Fix duplicated flows in table 32.

[Han Zhou]

Hi Darrell,

Sorry for late response. I posted v3 just now.

On Wed, Dec 14, 2016 at 12:19 PM, Darrell Ball  wrote:
>
> ERROR log level is not appropriate since we always filter adding the
extra flows anyways
>
> and it amount to extra processing and filtering.
>
> We anyways attempt to re-add every flow for every iteration of the
ovn-controller main loop,
>
> not just physical flows; we filter these attempts with “installed_flows”,
so we are doing lots
>
> of extra processing normally, at this time.
>

This is not true. There is a temporary flow_table constructed for desired
flows, which is initiated and destructed in each iteration of main loop.
The "duplicate flow" reported by the log is in fact for duplications in a
single iteration. But I agree that ERROR log level is not necessary.

>
>
> With the vtep test, the non-physical flows are not as many and diverse.
So checking
>
> for attempting to add the same flow in the same iteration of
ovn-controller main loop
>
> where the desired_flows log check is made (which is before the
installed_flows check)
>
> should help to catch the physical flow duplicate attempts in a given
iteration of the main loop.
>
>
>
> If we added the check there, we would be trying to catch some portion of
the unnecessary extra
>
> processing and filtering which seems valid and avoid noisy logs, which
could obscure other logs ?
>
>

I added the check for the 3 HVs test case, which I think is more
appropriate since it is basic functionality.

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

[ovs-dev] [PATCH 2/3] doc: Misc Windows doc formatting fixes

[Stephen Finucane]

There are a couple of minor issues with this document:

- Some commands intended to be run in the MinGW shell are prefixed with
  '>', suggesting they are in fact PowerShell commands
- PowerShell syntax highlighting is not enabled
- Indentation is off for a couple of blocks

Resolve all of these through use of the 'code-block' element and a lot
of random fixes.

Signed-off-by: Stephen Finucane 
---
 Documentation/intro/install/windows.rst | 496 +++-
 1 file changed, 294 insertions(+), 202 deletions(-)

diff --git a/Documentation/intro/install/windows.rst 
b/Documentation/intro/install/windows.rst
index 6d65f06..1ba9b63 100644
--- a/Documentation/intro/install/windows.rst
+++ b/Documentation/intro/install/windows.rst
@@ -94,7 +94,7 @@ The following explains the steps in some detail.
   directory (e.g.: ``C:/pthread``). You should add the pthread-win32's dll path
   (e.g.: ``C:\pthread\dll\x86``) to the Windows' PATH environment variable.
 
-- OpenSSQL
+- OpenSSL
 
   To get SSL support for Open vSwitch on Windows, you will need to install
   `OpenSSL for Windows `__
@@ -102,6 +102,13 @@ The following explains the steps in some detail.
   Note down the directory where OpenSSL is installed (e.g.:
   ``C:/OpenSSL-Win32``) for later use.
 
+.. note::
+
+   Commands prefixed by ``$`` must be run in the Bash shell provided by MinGW.
+   Open vSwitch commands, such as ``ovs-dpctl`` are shown running under
+   PowerShell (``>`` prefix) but will also run under Bash. The remainder,
+   prefixed by ``>``, are PowerShell commands and must be run in PowerShell.
+
 Install Requirements
 
 
@@ -123,9 +130,11 @@ Bootstrapping
 This step is not needed if you have downloaded a released tarball. If
 you pulled the sources directly from an Open vSwitch Git tree or got a
 Git tree snapshot, then run boot.sh in the top source directory to build
-the "configure" script::
+the "configure" script:
+
+.. code-block:: bash
 
-> ./boot.sh
+   $ ./boot.sh
 
 .. _windows-configuring:
 
@@ -134,45 +143,53 @@ Configuring
 
 Configure the package by running the configure script.  You should provide some
 configure options to choose the right compiler, linker, libraries, Open vSwitch
-component installation directories, etc. For example::
+component installation directories, etc. For example:
 
-> ./configure CC=./build-aux/cccl LD="$(which link)" \
-LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
---prefix="C:/openvswitch/usr" \
---localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" \
---with-pthread="C:/pthread"
+.. code-block:: bash
+
+   $ ./configure CC=./build-aux/cccl LD="$(which link)" \
+   LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+   --prefix="C:/openvswitch/usr" \
+   --localstatedir="C:/openvswitch/var" \
+   --sysconfdir="C:/openvswitch/etc" \
+   --with-pthread="C:/pthread"
 
 .. note::
-  By default, the above enables compiler optimization for fast code.  For
-  default compiler optimization, pass the ``--with-debug`` configure option.
-
-To configure with SSL support, add the requisite additional options::
-
-> ./configure CC=./build-aux/cccl LD="`which link`"  \
-LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
---prefix="C:/openvswitch/usr" \
---localstatedir="C:/openvswitch/var"
---sysconfdir="C:/openvswitch/etc" \
---with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32"
-
-Finally, to the kernel module also::
-
-> ./configure CC=./build-aux/cccl LD="`which link`" \
-LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
---prefix="C:/openvswitch/usr" \
---localstatedir="C:/openvswitch/var" \
---sysconfdir="C:/openvswitch/etc" \
---with-pthread="C:/pthread" \
---enable-ssl --with-openssl="C:/OpenSSL-Win32" \
---with-vstudiotarget=""
+
+   By default, the above enables compiler optimization for fast code.  For
+   default compiler optimization, pass the ``--with-debug`` configure option.
+
+To configure with SSL support, add the requisite additional options:
+
+.. code-block:: bash
+
+   $ ./configure CC=./build-aux/cccl LD="`which link`"  \
+   LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+   --prefix="C:/openvswitch/usr" \
+   --localstatedir="C:/openvswitch/var"
+   --sysconfdir="C:/openvswitch/etc" \
+   --with-pthread="C:/pthread" \
+   --enable-ssl --with-openssl="C:/OpenSSL-Win32"
+
+Finally, to the kernel module also:
+
+.. code-block:: bash
+
+   $ ./configure CC=./build-aux/cccl LD="`which link`" \
+   LIBS="-lws2_32 -liphlpapi -lwbemuuid -lole32 -loleaut32" \
+   --prefix="C:/openvswitch/usr" \
+   --localstatedir="C:/openvswitch/var" \
+   --sysconfdir="C:/openvswitch/etc" \
+   --with-pthread="C:/pthr

[ovs-dev] [PATCH 1/3] doc: Resolve issues with Windows guide

[Stephen Finucane]

The formatting of this file was broken in a recent commit. Resolve this
issue.

Signed-off-by: Stephen Finucane 
Fixes: a0c03adff6c2 ("Windows: document multiple NIC support setup")
Cc: Alin Gabriel Serdean 
---
 Documentation/intro/install/windows.rst | 116 +---
 1 file changed, 62 insertions(+), 54 deletions(-)

diff --git a/Documentation/intro/install/windows.rst 
b/Documentation/intro/install/windows.rst
index 463f30f..6d65f06 100644
--- a/Documentation/intro/install/windows.rst
+++ b/Documentation/intro/install/windows.rst
@@ -498,64 +498,72 @@ found at technet_.
 
 .. _technet: 
https://technet.microsoft.com/en-us/library/jj553812%28v=wps.630%29.aspx
 
-I.e.::
-We will set up a switch team combined from ``Ethernet0 2`` and ``Ethernet1 2``
-named ``external``.
-
-PS > Get-NetAdapter
-Name  InterfaceDescription
-  
-br-intHyper-V Virtual Ethernet Adapter #3
-br-pifHyper-V Virtual Ethernet Adapter #2
-Ethernet3 2   Intel(R) 82574L Gigabit Network Co...#3
-Ethernet2 2   Intel(R) 82574L Gigabit Network Co...#4
-Ethernet1 2   Intel(R) 82574L Gigabit Network Co...#2
-Ethernet0 2   Intel(R) 82574L Gigabit Network Conn...
-
-PS > New-NetSwitchTeam -Name external -TeamMembers "Ethernet0 2","Ethernet1 2"
-PS > Get-NetSwitchTeam
-Name: external
-Members : {Ethernet1 2, Ethernet0 2}
-
-This will result in a new adapter bound to the host called ``external``
-
-PS > Get-NetAdapter
-
-Name  InterfaceDescription
-  
-br-test   Hyper-V Virtual Ethernet Adapter #4
-br-pifHyper-V Virtual Ethernet Adapter #2
-external  Microsoft Network Adapter Multiplexo...
-Ethernet3 2   Intel(R) 82574L Gigabit Network Co...#3
-Ethernet2 2   Intel(R) 82574L Gigabit Network Co...#4
-Ethernet1 2   Intel(R) 82574L Gigabit Network Co...#2
-Ethernet0 2   Intel(R) 82574L Gigabit Network Conn...
-
-Next we will set up the Hyper-V VMSwitch on the new adapter ``external``
-
-PS > New-VMSwitch -Name external -NetAdapterName external \
- -AllowManagementOS $false
+For example, to set up a switch team combined from ``Ethernet0 2`` and
+``Ethernet1 2`` named ``external``:
+
+.. code-block:: powershell
+
+   PS > Get-NetAdapter
+   Name  InterfaceDescription
+     
+   br-intHyper-V Virtual Ethernet Adapter #3
+   br-pifHyper-V Virtual Ethernet Adapter #2
+   Ethernet3 2   Intel(R) 82574L Gigabit Network Co...#3
+   Ethernet2 2   Intel(R) 82574L Gigabit Network Co...#4
+   Ethernet1 2   Intel(R) 82574L Gigabit Network Co...#2
+   Ethernet0 2   Intel(R) 82574L Gigabit Network Conn...
+
+   PS > New-NetSwitchTeam -Name external -TeamMembers "Ethernet0 2","Ethernet1 
2"
+   PS > Get-NetSwitchTeam
+   Name: external
+   Members : {Ethernet1 2, Ethernet0 2}
+
+This will result in a new adapter bound to the host called ``external``:
+
+.. code-block:: powershell
+
+   PS > Get-NetAdapter
+
+   Name  InterfaceDescription
+     
+   br-test   Hyper-V Virtual Ethernet Adapter #4
+   br-pifHyper-V Virtual Ethernet Adapter #2
+   external  Microsoft Network Adapter Multiplexo...
+   Ethernet3 2   Intel(R) 82574L Gigabit Network Co...#3
+   Ethernet2 2   Intel(R) 82574L Gigabit Network Co...#4
+   Ethernet1 2   Intel(R) 82574L Gigabit Network Co...#2
+   Ethernet0 2   Intel(R) 82574L Gigabit Network Conn...
+
+Next we will set up the Hyper-V VMSwitch on the new adapter ``external``:
+
+.. code-block:: powershell
+
+   PS > New-VMSwitch -Name external -NetAdapterName external \
+-AllowManagementOS $false
 
 Under OVS the adapters under the team ``external``, ``Ethernet0 2`` and
 ``Ethernet1 2``, can be added either under a bond device or separately.
 
-The following example shows how the bridges look with the NICs being 
separated::
-
-> ovs-vsctl show
-
-6cd9481b-c249-4ee3-8692-97b399dd29d8
-Bridge br-test
-Port br-test
-Interface br-test
-type: internal
-Port "Ethernet1 2"
-Interface "Ethernet1 2"
-Bridge br-pif
-Port "Ethernet0 2"
-Interface "Ethernet0 2"
-Port br-pif
-Interface br-pif
-type: internal
+The following example shows how the bridges look with the NICs being
+separated:
+
+.. code-block:: powershell
+
+   PS > ovs-vsctl show
+
+   6cd9481b-c249-4ee3-8692-97b399dd29d8
+   Bridge br-test
+   Port br-test
+   Interface br-test
+  

[ovs-dev] [PATCH 3/3] doc: Prefer use of 'code-block' directive

[Stephen Finucane]

The '::' element previously preferred is shorter, but does not allow for
non-Python syntax highlighting. New documentation should make use of
syntax highlighting wherever possible.

Signed-off-by: Stephen Finucane 
---
 Documentation/internals/contributing/documentation-style.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/internals/contributing/documentation-style.rst 
b/Documentation/internals/contributing/documentation-style.rst
index c32921e..318cc81 100644
--- a/Documentation/internals/contributing/documentation-style.rst
+++ b/Documentation/internals/contributing/documentation-style.rst
@@ -111,7 +111,8 @@ Code
 
 
 - Use ``::``, the ``code`` role or the ``code-block:: `` role to prefix
-  code.
+  code. The ``code-block:: `` format is preferred as this provides
+  syntax highlighting for non-Python languages, such as Bash or PowerShell.
 
 - Prefix commands with ``$``.
 
-- 
2.9.3

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

[ovs-dev] [PATCH v3] ovn-controller: Fix duplicated flow add attempts in table 32.

[Han Zhou]

In commit 475f0a2c it introduced a priority 150 flow for filtering
the sending of traffic received from vxlan tunnels back out tunnels.
However, it added the flow for every remote port processing, which
results in continuous logs about duplicated flows. We only need to
install this flow once per physical_run() loop iteration.

Signed-off-by: Han Zhou 
Acked-by: Darrell Ball 
---

Notes:
v1 -> v2: update commit message according to Darrell's comments.
v2 -> v3: update test case.

 ovn/controller/physical.c | 47 ---
 tests/ovn.at  |  5 +
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 48adb78..3b653dd 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -465,33 +465,16 @@ consider_port_binding(enum mf_field_id mff_ovn_geneve,
 } else {
 /* Remote port connected by tunnel */
 
-/* Table 32, priority 150 and 100.
+/* Table 32, priority 100.
  * ===
  *
- * Priority 150 is for packets received from a VXLAN tunnel
- * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
- * lack of needed metadata in VXLAN, explicitly skip sending
- * back out any tunnels and resubmit to table 33 for local
- * delivery.
- *
- * Priority 100 is for all other traffic which need to be sent
- * to a remote hypervisor.  Each flow matches an output port
- * that includes a logical port on a remote hypervisor, and
- * tunnels the packet to that hypervisor.
+ * Priority 100 is for traffic that needs to be sent to a remote
+ * hypervisor.  Each flow matches an output port that includes a
+ * logical port on a remote hypervisor, and tunnels the packet to
+ * that hypervisor.
  */
 match_init_catchall(&match);
 ofpbuf_clear(ofpacts_p);
-match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
- MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
-
-/* Resubmit to table 33. */
-put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_p);
-ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match,
-ofpacts_p);
-
-
-match_init_catchall(&match);
-ofpbuf_clear(ofpacts_p);
 
 /* Match MFF_LOG_DATAPATH, MFF_LOG_OUTPORT. */
 match_set_metadata(&match, htonll(dp_key));
@@ -870,12 +853,30 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id 
mff_ovn_geneve,
 }
 }
 
+/* Table 32, priority 150.
+ * ===
+ *
+ * Priority 150 is for packets received from a VXLAN tunnel
+ * which get resubmitted to OFTABLE_LOG_INGRESS_PIPELINE due to
+ * lack of needed metadata in VXLAN, explicitly skip sending
+ * back out any tunnels and resubmit to table 33 for local
+ * delivery.
+ */
+struct match match;
+match_init_catchall(&match);
+ofpbuf_clear(&ofpacts);
+match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
+ MLF_RCV_FROM_VXLAN, MLF_RCV_FROM_VXLAN);
+
+/* Resubmit to table 33. */
+put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
+ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 150, &match, &ofpacts);
+
 /* Table 32, Priority 0.
  * ===
  *
  * Resubmit packets that are not directed at tunnels or part of a
  * multicast group to the local output table. */
-struct match match;
 match_init_catchall(&match);
 ofpbuf_clear(&ofpacts);
 put_resubmit(OFTABLE_LOCAL_OUTPUT, &ofpacts);
diff --git a/tests/ovn.at b/tests/ovn.at
index 628d3c8..b852665 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1058,6 +1058,11 @@ ovn_populate_arp
 # XXX This should be more systematic.
 sleep 1
 
+# Make sure there is no attempt to adding duplicated flows by ovn-controller
+AT_FAIL_IF([test -n "`grep duplicate hv1/ovn-controller.log`"])
+AT_FAIL_IF([test -n "`grep duplicate hv2/ovn-controller.log`"])
+AT_FAIL_IF([test -n "`grep duplicate hv3/ovn-controller.log`"])
+
 # Given the name of a logical port, prints the name of the hypervisor
 # on which it is located.
 vif_to_hv() {
-- 
2.1.0

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

Re: [ovs-dev] [RFC-PATCH 2/2] lib/dpdk: No more deferred release

[Aaron Conole]

Aaron Conole  writes:

> DPDK documentation is recently updated to reflect that DPDK does not
> hold any references to, nor take ownership of, the argv/argc elements.
> With that understanding, let's just release the memory asap.
>
> Signed-off-by: Aaron Conole 
> ---
> NOTE: if the patch at
>   http://dpdk.org/ml/archives/dev/2016-December/051812.html is
>   accepted, then consider this non-RFC.

Quick note - the documentation patch was recently merged to DPDK; is it
possible to get this reviewed?  I can rebase, retest, and resubmit if
needed.

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

Re: [ovs-dev] [PATCH v2 0/4] make ofproto/trace output easier to read

[Lance Richardson]

- Original Message -
> From: "Ben Pfaff" 
> To: d...@openvswitch.org
> Cc: "Ben Pfaff" 
> Sent: Wednesday, December 21, 2016 2:03:05 AM
> Subject: [ovs-dev] [PATCH v2 0/4] make ofproto/trace output easier to read
> 
> This series seeks to make the output of ofproto/trace easier to read,
> especially in complicated situations.
> 
> v1->v2:
> - Patch 4 extended and revised to make it suitable for review
>   and no longer RFC.
> 
> I hope that this series pleases many users.
> 
> Ben Pfaff (4):
>   ofproto: Update access rules for 'flow_cookie'.
>   ofproto-dpif: Unhide structure contents.
>   ofproto-dpif: Break trace functionality into a separate source file.
>   ofproto-dpif: Make ofproto/trace output easier to read.
> 
>  Documentation/tutorials/ovs-advanced.rst | 268 ++
>  lib/nx-match.c   |  29 +-
>  lib/nx-match.h   |   6 +-
>  ofproto/automake.mk  |   2 +
>  ofproto/bond.c   |   7 +-
>  ofproto/ofproto-dpif-rid.c   |   5 +-
>  ofproto/ofproto-dpif-trace.c | 485 +
>  ofproto/ofproto-dpif-trace.h |  45 ++
>  ofproto/ofproto-dpif-upcall.c|  10 +-
>  ofproto/ofproto-dpif-xlate-cache.c   |   8 +-
>  ofproto/ofproto-dpif-xlate.c | 869
>  +++
>  ofproto/ofproto-dpif-xlate.h |  26 +-
>  ofproto/ofproto-dpif.c   | 800 +---
>  ofproto/ofproto-dpif.h   | 349 -
>  ofproto/ofproto-provider.h   |   4 +-
>  ofproto/ofproto.c|   5 +-
>  tests/ofproto-dpif.at|  41 +-
>  tests/ovn.at |   4 +-
>  tests/rstp.at|   4 +-
>  tests/stp.at |   4 +-
>  20 files changed, 1531 insertions(+), 1440 deletions(-)
>  create mode 100644 ofproto/ofproto-dpif-trace.c
>  create mode 100644 ofproto/ofproto-dpif-trace.h
> 
> --

The new output format is immensely easier to read and understand.

For the series:

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

Re: [ovs-dev] [PATCH v3 05/16] ofp-actions: Add clone action.

[William Tu]

thanks. I will submit v4 patch.

On Wed, Dec 21, 2016 at 10:46 AM, Joe Stringer  wrote:
> On 21 December 2016 at 10:28, William Tu  wrote:
>> sorry, I forgot to add priority. Below lower the priority of the NORMAL 
>> action.
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index d70c5c3..321a901 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -340,6 +340,7 @@ AT_CLEANUP
>>  AT_SETUP([datapath - clone action])
>>  OVS_TRAFFIC_VSWITCHD_START()
>>
>> +AT_CHECK([ovs-ofctl add-flow br0 "priority=1 actions=normal"])
>>  ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)
>>
>>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
>> @@ -352,9 +353,9 @@ AT_CHECK([ovs-vsctl -- set interface ovs-p2
>> ofport_request=3])
>>
>>  dnl verify that the clone(...) won't affect the original packet, so
>> ping still works OK
>>  dnl without 'output' in 'clone()'
>> -AT_CHECK([ovs-ofctl add-flow br0
>> "in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
>> output:2"])
>> +AT_CHECK([ovs-ofctl add-flow br0 "priority=99
>> in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
>> output:2"])
>>  dnl with 'output' in 'clone()'
>> -AT_CHECK([ovs-ofctl add-flow br0
>> "in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
>> output:3), output:1"])
>> +AT_CHECK([ovs-ofctl add-flow br0 "priority=99
>> in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
>> output:3), output:1"])
>>
>>  NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
>> FORMAT_PING], [0], [dnl
>>  3 packets transmitted, 3 received, 0% packet loss, time 0ms
>
> I think it's sufficient to set the priority of the "normal" flow to 1,
> or just restrict it to ARP.
>
> A couple of other pieces of feedback on the test:
> * It's better if there is one ovs-ofctl call for all flows (look for
> flows.txt examples in the test file)
> * Similarly for the ofport_request setup, there doesn't need to be
> three separate ovs-vsctl calls, the commands can be separated by "--".
> (although since you're reliably adding the ports in order, I don't
> think it shouldn't be necessary to request the particular port numbers
> at all; OVS will allocate the numbers in the order you add them)
> * at_ns2 isn't used at all. Please come up with a way to ensure that
> the packets going to that port are modified as you would expect.
> * You might consider adding a controller action in the clone action,
> then using "ovs-ofctl monitor" to observe the traffic
>
> Please submit a patch the usual way addressing this feedback.
>
> Thanks,
> Joe
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/3] netdev-dpdk: Arbitrary 'dpdk' port naming

[Daniele Di Proietto]

2016-12-21 10:18 GMT-08:00 Kevin Traynor :
> On 12/21/2016 03:02 AM, Daniele Di Proietto wrote:
>> 2016-12-20 14:08 GMT-08:00 Kevin Traynor :
>>> On 12/15/2016 11:54 AM, Ciara Loftus wrote:
 'dpdk' ports no longer have naming restrictions. Now, instead of
 specifying the dpdk port ID as part of the name, the PCI address of the
 device must be specified via the 'dpdk-devargs' option. eg.

 ovs-vsctl add-port br0 my-port
 ovs-vsctl set Interface my-port type=dpdk
 ovs-vsctl set Interface my-port options:dpdk-devargs=:06:00.3
>>>
>>> I wouldn't encourage people to split up commands like above as they'll
>>> see errors and warnings.
>>
>> Good point
>>
>>>
>>> If you use the old command (which people surely will), it's not
>>> intuitive that it's now still a valid cmd but incomplete for setting up
>>> the port:
>>>
>>> []# ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
>>> 2016-12-20T19:53:54Z|00051|netdev|WARN|dpdk0: could not set
>>> configuration (Invalid argument)
>>> ovs-vsctl: Error detected while setting up 'dpdk0'.  See ovs-vswitchd
>>> log for details.
>>>
>>> It would be nice if this was just a warning and more informative - this
>>> could be an incremental change also.
>>
>> You're right, vsctl errors are pretty obscure in this case. I think a first
>> step is to improve what ovs-vsctl reports to the user. I sent a patch here:
>>
>> https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326542.html
>>
>> The second step would be to allow netdev_dpdk_set_config() to return an error
>> string, that can be printed by ovs-vsctl.  I'm fine with doing that later.
>> What do you guys think?
>
> sounds like a good plan to me.
>
> I've done some testing with this patch today and I can't seem to get
> hotplug working after applying 2/3. It works with 1/3 but I'm seeing
> hangs with the new scheme in 2/3 and a dpdk seg fault with 3/3. I think
> maybe my dpdk build is bad or my steps are wrong but it would be good if
> someone else could test too.
>
> - dpdk-devbind.py -u :01:00.0 :01:00.1
> - start vswitchd and add bridge
> - dpdk-devbind.py -b igb_uio :01:00.0 :01:00.1
> - ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
> options:dpdk-devargs=:01:00.0
>

I think there's a bug in DPDK 16.11 that should be fixed by this commit:

f9ae888b1e19("ethdev: fix port lookup if none").

Does it work if you include the fix in your DPDK build?

>
>>
>>>

 The user must no longer hotplug attach DPDK ports by issuing the
 specific ovs-appctl netdev-dpdk/attach command. The hotplug is now
 automatically invoked when a valid PCI address is set in the
 dpdk-devargs. The format for ovs-appctl netdev-dpdk/detach command
 has changed in that the user now must specify the relevant PCI address
 as input instead of the port name.

 Signed-off-by: Ciara Loftus 
 Signed-off-by: Daniele Di Proietto 
 Co-authored-by: Daniele Di Proietto 
>>
>> I think netdev_dpdk_set_config() should compare new_devargs with dev->devargs
>> and avoid calling netdev_dpdk_process_devargs() if they're equal and
>> if the port_id
>> is valid.  I've noticed that rte_eth_dev_get_port_by_name() sometimes
>> doesn't find
>> an existing device and I think comparing the strings will fix the problem.
>>
>> Also, could you add a log message if rte_eth_dev_attach fails?
>>
>> I've been playing with this for a while and I guess I'm fine with the
>> rest of the series.
>>
>> Thanks,
>>
>> Daniele
>>
 ---
 Changelog:
 * Keep port-detach appctl function - use PCI as input arg
 * Add requires_mutex to devargs processing functions
 * use reconfigure infrastructure for devargs changes
 * process devargs even if valid portid ie. device already configured
 * report err if dpdk-devargs is not specified
 * Add Daniele's incremental patch & Sign-off + Co-author tags
 * Update details of detach command to reflect PCI is needed instead of
   port name
 * Update NEWS to mention that the new naming scheme is not backwards
   compatible
 * Use existing DPDk functions to get port ID from PCI address/devname
 * Merged process_devargs and process_pdevargs functions
 * Removed unnecessary requires_mutex from devargs processing fn
 * Fix case where port is re-attached after detach
 * Add note to documentation that devices won't work until devargs set.

  Documentation/intro/install/dpdk-advanced.rst |   5 +-
  Documentation/intro/install/dpdk.rst  |  15 ++-
  NEWS  |   5 +
  lib/netdev-dpdk.c | 169 
 +-
  vswitchd/vswitch.xml  |   8 ++
  5 files changed, 138 insertions(+), 64 deletions(-)
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovs-vsctl: Print error when add-port fails.

[Daniele Di Proietto]

On 20/12/2016 21:37, "Ben Pfaff"  wrote:

>On Tue, Dec 20, 2016 at 06:49:24PM -0800, Daniele Di Proietto wrote:
>> When the add-port command fails, vsctl reports the failure and just
>> suggests to check the logs for more details.
>> 
>> ovs-vswitchd fills the error column in the Interface table with a
>> description of the error, so it might be helpful to print that.
>> 
>> This is useful especially for dpdk devices, because the port naming
>> change could use a better error reporting.
>> 
>> I'm planning another patch to make sure that ovs-vswitch writes
>> appropriates information in the error column, after the dpdk port naming
>> changes are merged.
>> 
>> CC: Ben Pfaff 
>> Signed-off-by: Daniele Di Proietto 
>
>I have another idea for improving the output here.  Quite often, when
>someone posts an error, they haven't looked in the log, even though they
>quoted the error message recommending that.  Then, when I ask them to
>look in the log, the next question is always "where's the log?"  So,
>what if we added something like "The default log directory is %s.",
>where %s is ovs_logdir(), to the error message?  Perhaps it will help at
>least one person find the log.

Good idea, I've added that

>
>Acked-by: Ben Pfaff 

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

Re: [ovs-dev] [PATCH v3 05/16] ofp-actions: Add clone action.

[Joe Stringer]

On 21 December 2016 at 10:28, William Tu  wrote:
> sorry, I forgot to add priority. Below lower the priority of the NORMAL 
> action.
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index d70c5c3..321a901 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -340,6 +340,7 @@ AT_CLEANUP
>  AT_SETUP([datapath - clone action])
>  OVS_TRAFFIC_VSWITCHD_START()
>
> +AT_CHECK([ovs-ofctl add-flow br0 "priority=1 actions=normal"])
>  ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)
>
>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
> @@ -352,9 +353,9 @@ AT_CHECK([ovs-vsctl -- set interface ovs-p2
> ofport_request=3])
>
>  dnl verify that the clone(...) won't affect the original packet, so
> ping still works OK
>  dnl without 'output' in 'clone()'
> -AT_CHECK([ovs-ofctl add-flow br0
> "in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
> output:2"])
> +AT_CHECK([ovs-ofctl add-flow br0 "priority=99
> in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
> output:2"])
>  dnl with 'output' in 'clone()'
> -AT_CHECK([ovs-ofctl add-flow br0
> "in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
> output:3), output:1"])
> +AT_CHECK([ovs-ofctl add-flow br0 "priority=99
> in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
> output:3), output:1"])
>
>  NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
> FORMAT_PING], [0], [dnl
>  3 packets transmitted, 3 received, 0% packet loss, time 0ms

I think it's sufficient to set the priority of the "normal" flow to 1,
or just restrict it to ARP.

A couple of other pieces of feedback on the test:
* It's better if there is one ovs-ofctl call for all flows (look for
flows.txt examples in the test file)
* Similarly for the ofport_request setup, there doesn't need to be
three separate ovs-vsctl calls, the commands can be separated by "--".
(although since you're reliably adding the ports in order, I don't
think it shouldn't be necessary to request the particular port numbers
at all; OVS will allocate the numbers in the order you add them)
* at_ns2 isn't used at all. Please come up with a way to ensure that
the packets going to that port are modified as you would expect.
* You might consider adding a controller action in the clone action,
then using "ovs-ofctl monitor" to observe the traffic

Please submit a patch the usual way addressing this feedback.

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

Re: [ovs-dev] [PATCH v2 1/4] ofproto: Update access rules for 'flow_cookie'.

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme 

> On Dec 20, 2016, at 11:03 PM, Ben Pfaff  wrote:
> 
> The 'flow_cookie' member of struct rule is read during flow translation
> without holding a mutex, breaking the documented OVS_GUARDED access rule.
> However, the flow_cookie member is actually never modified after a rule is
> initialized, so this commit changes the access rules to reflect that.
> 
> Signed-off-by: Ben Pfaff 
> ---
> ofproto/ofproto-provider.h | 4 ++--
> ofproto/ofproto.c  | 5 +++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index c515779..3739ebc 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -365,8 +365,8 @@ struct rule {
> struct ovs_refcount ref_count;
> 
> /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with
> - * a flow.. */
> -ovs_be64 flow_cookie OVS_GUARDED;
> + * a flow. */
> +const ovs_be64 flow_cookie; /* Immutable once rule is constructed. */
> struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex);
> 
> enum ofputil_flow_mod_flags flags OVS_GUARDED;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 4f43c45..58295a1 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -4859,7 +4859,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct 
> cls_rule *cr,
> 
> ovs_mutex_init(&rule->mutex);
> ovs_mutex_lock(&rule->mutex);
> -rule->flow_cookie = new_cookie;
> +*CONST_CAST(ovs_be64 *, &rule->flow_cookie) = new_cookie;
> rule->created = rule->modified = time_msec();
> rule->idle_timeout = idle_timeout;
> rule->hard_timeout = hard_timeout;
> @@ -5098,7 +5098,8 @@ replace_rule_start(struct ofproto *ofproto, struct 
> ofproto_flow_mod *ofm,
> new_rule->created = old_rule->created;
> }
> if (!change_cookie) {
> -new_rule->flow_cookie = old_rule->flow_cookie;
> +*CONST_CAST(ovs_be64 *, &new_rule->flow_cookie)
> += old_rule->flow_cookie;
> }
> ovs_mutex_unlock(&old_rule->mutex);
> ovs_mutex_unlock(&new_rule->mutex);
> -- 
> 2.10.2
> 
> ___
> 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 v3 05/16] ofp-actions: Add clone action.

[William Tu]

sorry, I forgot to add priority. Below lower the priority of the NORMAL action.

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index d70c5c3..321a901 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -340,6 +340,7 @@ AT_CLEANUP
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()

+AT_CHECK([ovs-ofctl add-flow br0 "priority=1 actions=normal"])
 ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)

 ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
@@ -352,9 +353,9 @@ AT_CHECK([ovs-vsctl -- set interface ovs-p2
ofport_request=3])

 dnl verify that the clone(...) won't affect the original packet, so
ping still works OK
 dnl without 'output' in 'clone()'
-AT_CHECK([ovs-ofctl add-flow br0
"in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
output:2"])
+AT_CHECK([ovs-ofctl add-flow br0 "priority=99
in_port=1,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0a),set_field:192.168.3.3->ip_dst),
output:2"])
 dnl with 'output' in 'clone()'
-AT_CHECK([ovs-ofctl add-flow br0
"in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
output:3), output:1"])
+AT_CHECK([ovs-ofctl add-flow br0 "priority=99
in_port=2,ip,actions=clone(mod_dl_dst(50:54:00:00:00:0b),set_field:192.168.4.4->ip_dst,
output:3), output:1"])

 NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
FORMAT_PING], [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms

Regards,
William

On Wed, Dec 21, 2016 at 10:21 AM, Joe Stringer  wrote:
> On 21 December 2016 at 07:45, William Tu  wrote:
>> Hi Joe,
>>
>> I think we're missing the normal action for handling the non-IP
>> traffic. Can you try this patch on top of the clone?
>>
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -340,6 +340,7 @@ AT_CLEANUP
>>  AT_SETUP([datapath - clone action])
>>  OVS_TRAFFIC_VSWITCHD_START()
>>
>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>>  ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)
>>
>>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
>
> If you force all traffic to be handled with this flow, then the test
> will pass but it will not be testing anything.
>
> Looking at the test, I don't really follow how it is supposed to be
> guaranteeing that the expected actions are executed on both copies of
> the packet. Could you explain it?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 05/16] ofp-actions: Add clone action.

[Joe Stringer]

On 21 December 2016 at 07:45, William Tu  wrote:
> Hi Joe,
>
> I think we're missing the normal action for handling the non-IP
> traffic. Can you try this patch on top of the clone?
>
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -340,6 +340,7 @@ AT_CLEANUP
>  AT_SETUP([datapath - clone action])
>  OVS_TRAFFIC_VSWITCHD_START()
>
> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>  ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)
>
>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")

If you force all traffic to be handled with this flow, then the test
will pass but it will not be testing anything.

Looking at the test, I don't really follow how it is supposed to be
guaranteeing that the expected actions are executed on both copies of
the packet. Could you explain it?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/3] netdev-dpdk: Arbitrary 'dpdk' port naming

[Kevin Traynor]

On 12/21/2016 03:02 AM, Daniele Di Proietto wrote:
> 2016-12-20 14:08 GMT-08:00 Kevin Traynor :
>> On 12/15/2016 11:54 AM, Ciara Loftus wrote:
>>> 'dpdk' ports no longer have naming restrictions. Now, instead of
>>> specifying the dpdk port ID as part of the name, the PCI address of the
>>> device must be specified via the 'dpdk-devargs' option. eg.
>>>
>>> ovs-vsctl add-port br0 my-port
>>> ovs-vsctl set Interface my-port type=dpdk
>>> ovs-vsctl set Interface my-port options:dpdk-devargs=:06:00.3
>>
>> I wouldn't encourage people to split up commands like above as they'll
>> see errors and warnings.
> 
> Good point
> 
>>
>> If you use the old command (which people surely will), it's not
>> intuitive that it's now still a valid cmd but incomplete for setting up
>> the port:
>>
>> []# ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
>> 2016-12-20T19:53:54Z|00051|netdev|WARN|dpdk0: could not set
>> configuration (Invalid argument)
>> ovs-vsctl: Error detected while setting up 'dpdk0'.  See ovs-vswitchd
>> log for details.
>>
>> It would be nice if this was just a warning and more informative - this
>> could be an incremental change also.
> 
> You're right, vsctl errors are pretty obscure in this case. I think a first
> step is to improve what ovs-vsctl reports to the user. I sent a patch here:
> 
> https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326542.html
> 
> The second step would be to allow netdev_dpdk_set_config() to return an error
> string, that can be printed by ovs-vsctl.  I'm fine with doing that later.
> What do you guys think?

sounds like a good plan to me.

I've done some testing with this patch today and I can't seem to get
hotplug working after applying 2/3. It works with 1/3 but I'm seeing
hangs with the new scheme in 2/3 and a dpdk seg fault with 3/3. I think
maybe my dpdk build is bad or my steps are wrong but it would be good if
someone else could test too.

- dpdk-devbind.py -u :01:00.0 :01:00.1
- start vswitchd and add bridge
- dpdk-devbind.py -b igb_uio :01:00.0 :01:00.1
- ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk
options:dpdk-devargs=:01:00.0


> 
>>
>>>
>>> The user must no longer hotplug attach DPDK ports by issuing the
>>> specific ovs-appctl netdev-dpdk/attach command. The hotplug is now
>>> automatically invoked when a valid PCI address is set in the
>>> dpdk-devargs. The format for ovs-appctl netdev-dpdk/detach command
>>> has changed in that the user now must specify the relevant PCI address
>>> as input instead of the port name.
>>>
>>> Signed-off-by: Ciara Loftus 
>>> Signed-off-by: Daniele Di Proietto 
>>> Co-authored-by: Daniele Di Proietto 
> 
> I think netdev_dpdk_set_config() should compare new_devargs with dev->devargs
> and avoid calling netdev_dpdk_process_devargs() if they're equal and
> if the port_id
> is valid.  I've noticed that rte_eth_dev_get_port_by_name() sometimes
> doesn't find
> an existing device and I think comparing the strings will fix the problem.
> 
> Also, could you add a log message if rte_eth_dev_attach fails?
> 
> I've been playing with this for a while and I guess I'm fine with the
> rest of the series.
> 
> Thanks,
> 
> Daniele
> 
>>> ---
>>> Changelog:
>>> * Keep port-detach appctl function - use PCI as input arg
>>> * Add requires_mutex to devargs processing functions
>>> * use reconfigure infrastructure for devargs changes
>>> * process devargs even if valid portid ie. device already configured
>>> * report err if dpdk-devargs is not specified
>>> * Add Daniele's incremental patch & Sign-off + Co-author tags
>>> * Update details of detach command to reflect PCI is needed instead of
>>>   port name
>>> * Update NEWS to mention that the new naming scheme is not backwards
>>>   compatible
>>> * Use existing DPDk functions to get port ID from PCI address/devname
>>> * Merged process_devargs and process_pdevargs functions
>>> * Removed unnecessary requires_mutex from devargs processing fn
>>> * Fix case where port is re-attached after detach
>>> * Add note to documentation that devices won't work until devargs set.
>>>
>>>  Documentation/intro/install/dpdk-advanced.rst |   5 +-
>>>  Documentation/intro/install/dpdk.rst  |  15 ++-
>>>  NEWS  |   5 +
>>>  lib/netdev-dpdk.c | 169 
>>> +-
>>>  vswitchd/vswitch.xml  |   8 ++
>>>  5 files changed, 138 insertions(+), 64 deletions(-)

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

Re: [ovs-dev] [PATCH 1/5] system-traffic: Introduce OVS_START_L7 macro.

[Joe Stringer]

On 20 December 2016 at 13:28, Joe Stringer  wrote:
> All of the commands starting L7 servers duplicate detailed specifics
> which inhibits readability, and makes it difficult to ensure that the
> servers are ready before the test proceeds. Add a new macro that
> provides simpler semantics from the test perspective and hide the
> details in the macro. A followup patch will extend this macro to ensure
> that servers are ready to serve requests before the test proceeds.
>
> Signed-off-by: Joe Stringer 

Just a reminder for me, this needs to be applied to the new clone test too.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] splitting commits in Git (was: Re: Sync on PTAP, EXT-382 and NSH)

[Jarno Rajahalme]

> On Dec 21, 2016, at 9:30 AM, Ben Pfaff  wrote:
> 
> There was a question about splitting up large commits in Git.  I do this
> routinely and I volunteered to find some resources.
> 
> The "git-rebase" manpage has a section titled "SPLITTING COMMITS" that
> is a good place to start.  It does not cover very well how to commit
> parts of a file rather than a whole file, which is something that I do
> often.  For this, I use "git citool" and then select lines to commit and
> right-click to "stage lines for commit".  I know that there's a mode for
> the "git commit" CLI that can do this too, but I've always found that
> hard to use compared to "git citool”.

You mean “git add -p”? I’ve used that, but was not aware of “git citool”…

  Jarno

> 
> I do this often so I'm happy to answer questions.

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

[ovs-dev] please help me for senario

[Reza Safaei]

hi

I attached the  used to create the queue but does not work according to my
wishes.
please guide me
Thanks
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] reg: OpenVswitch Fields Implementation

[Ben Pfaff]

I'm trying to understand you here.  You're saying that you added some
code to Open vSwitch.  It didn't work.  Now, you're asking what's wrong
with it.  How can we know?  We didn't write the code.

On Wed, Dec 21, 2016 at 10:54:17PM +0530, Satyavalli Rama wrote:
>  
>   Could you please let us know, because of which this type of issues can come 
> or we missing anything.
> 
> Thanks
> Satya Valli
> 
>  -Ben Pfaff  wrote: -
> 
>  ===
>  To: Satyavalli Rama 
>  From: Ben Pfaff 
>  Date: 12/21/2016 10:44PM 
>  Cc: ovs-dev@openvswitch.org
>  Subject: Re: [ovs-dev] reg: OpenVswitch Fields Implementation
>  ===
>Why are you telling us this?
> 
> On Wed, Dec 21, 2016 at 10:38:08PM +0530, Satyavalli Rama wrote:
> >  Hi Ben,
> > 
> > Currently we are implementing OXS as an independent infrastructure.  
> > While sending OXS Flow Stat Request,  we are facing the below issue.
> > We have dumped and verified the packet is in proper format only, but we are 
> > not receiving the Reply.
> > 
> > 2016-12-21T06:46:56Z|00016|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
> >  sent (Success): OFPST_OXS_FLOW request (OF1.5) (xid=0x2): 
> > 2016-12-21T06:46:56Z|00017|poll_loop|DBG|wakeup due to 0-ms timeout
> > 2016-12-21T06:47:55Z|00018|poll_loop|DBG|wakeup due to [POLLIN] on fd 4 
> > (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
> > 2016-12-21T06:47:55Z|00019|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
> >  received: OFPT_ECHO_REQUEST (OF1.5) (xid=0x0): 0 bytes of payload
> > 2016-12-21T06:47:55Z|00020|ofctl|DBG|received reply with xid  != 
> > expected 0200
> > 2016-12-21T06:48:55Z|00021|poll_loop|DBG|wakeup due to [POLLIN][POLLHUP] on 
> > fd 4 (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
> > ovs-ofctl: OpenFlow packet receive failed (End of file)
> > 
> > Thanks and Regards
> > Satya Valli
> >   
> > 
> >  -Ben Pfaff  wrote: -
> > 
> >  ===
> >  To: Satyavalli Rama 
> >  From: Ben Pfaff 
> >  Date: 12/16/2016 10:23PM 
> >  Cc: ovs-dev@openvswitch.org
> >  Subject: Re: [ovs-dev] reg: OpenVswitch Fields Implementation
> >  ===
> >On Fri, Dec 16, 2016 at 03:33:11PM +0530, Satyavalli Rama wrote:
> > > We are working on Openflow 15 EXT-334 i.e OpenFlow Extensible Flow Entry 
> > > Statistics,  as per OpenvSwitch fields implementation in the meta-flow.h, 
> > > the key-value pairs for "OXS fields" to be added/declared separately or 
> > > do 
> > > we need to follow the existing NXM/OXM  key-value pairs.
> > > 
> > > Please clarify us, and please refer us some guidelines that we need to 
> > > follow for implementing this Openvswitch fields.
> > 
> > I don't think that OXS defines fields at all, so I'd expect their
> > implementation to be independent of existing infrastructure.
> > 
> > =-=-=
> > Notice: The information contained in this e-mail
> > message and/or attachments to it may contain 
> > confidential or privileged information. If you are 
> > not the intended recipient, any dissemination, use, 
> > review, distribution, printing or copying of the 
> > information contained in this e-mail message 
> > and/or attachments to it are strictly prohibited. If 
> > you have received this communication in error, 
> > please notify us by reply e-mail or telephone and 
> > immediately and permanently delete the message 
> > and any attachments. Thank you
> > 
> > 
> > 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] splitting commits in Git (was: Re: Sync on PTAP, EXT-382 and NSH)

[Ben Pfaff]

There was a question about splitting up large commits in Git.  I do this
routinely and I volunteered to find some resources.

The "git-rebase" manpage has a section titled "SPLITTING COMMITS" that
is a good place to start.  It does not cover very well how to commit
parts of a file rather than a whole file, which is something that I do
often.  For this, I use "git citool" and then select lines to commit and
right-click to "stage lines for commit".  I know that there's a mode for
the "git commit" CLI that can do this too, but I've always found that
hard to use compared to "git citool".

I do this often so I'm happy to answer questions.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] lacp: Select a may-enable IF as the lead IF

[Ben Pfaff]

On Thu, Dec 01, 2016 at 02:01:59PM +0100, Torgny Lindberg wrote:
> A reboot of one switch in an MC-LAG bond makes all bond links
> to go down, causing a total connectivity loss for 3 seconds.
> 
> Packet capture shows that spurious LACP PDUs are sent to OVS with
> a different MAC address (partner system id) during the final
> stages of the MC-LAG switch reboot.
> 
> The current code selects a lead interface based on information
> in the LACP PDU, regardless of its synchronization state. If a
> non-synchronized interface is selected as the OVS lead interface
> then all other interfaces are forced down as their stored partner
> system id differs and the bond ends up with no working interface.
> The bond recovers within three seconds after the last spurious
> message.
> 
> To avoid the problem, this commit requires a lead interface
> to be synchronized. In case no synchronized interface exists,
> the selection of lead interface is done as in the current code.
> 
> Signed-off-by: Torgny Lindberg 

I think I understand what's going on here now.  I made some changes that
better reflect my understanding:
https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326567.html
Does this work for you?  If so, I'll apply it.

Thanks,

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

[ovs-dev] [PATCH v2] lacp: Select a may-enable IF as the lead IF

[Ben Pfaff]

A reboot of one switch in an MC-LAG bond makes all bond links
to go down, causing a total connectivity loss for 3 seconds.

Packet capture shows that spurious LACP PDUs are sent to OVS with
a different MAC address (partner system id) during the final
stages of the MC-LAG switch reboot.

The current code selects a lead interface based on information
in the LACP PDU, regardless of its synchronization state. If a
non-synchronized interface is selected as the OVS lead interface
then all other interfaces are forced down as their stored partner
system id differs and the bond ends up with no working interface.
The bond recovers within three seconds after the last spurious
message.

To avoid the problem, this commit requires a lead interface
to be synchronized. In case no synchronized interface exists,
the selection of lead interface is done as in the current code.

Signed-off-by: Torgny Lindberg 
Signed-off-by: Ben Pfaff 
---
v1->v2: Rewrite preference for a synchronized interface in terms of a
comparison inside the loop.

 lib/lacp.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index ad6ef8e..84b2cf6 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+/* Copyright (c) 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -596,11 +596,13 @@ lacp_update_attached(struct lacp *lacp) 
OVS_REQUIRES(mutex)
 {
 struct slave *lead, *slave;
 struct lacp_info lead_pri;
+bool lead_enable;
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 10);
 
 lacp->update = false;
 
 lead = NULL;
+lead_enable = false;
 HMAP_FOR_EACH (slave, node, &lacp->slaves) {
 struct lacp_info pri;
 
@@ -623,9 +625,14 @@ lacp_update_attached(struct lacp *lacp) OVS_REQUIRES(mutex)
 
 slave->attached = true;
 slave_get_priority(slave, &pri);
+bool enable = slave_may_enable__(slave);
 
-if (!lead || memcmp(&pri, &lead_pri, sizeof pri) < 0) {
+if (!lead
+|| enable > lead_enable
+|| (enable == lead_enable
+&& memcmp(&pri, &lead_pri, sizeof pri) < 0)) {
 lead = slave;
+lead_enable = enable;
 lead_pri = pri;
 }
 }
-- 
2.10.2

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

Re: [ovs-dev] reg: OpenVswitch Fields Implementation

[Satyavalli Rama]

 
  Could you please let us know, because of which this type of issues can come 
or we missing anything.

Thanks
Satya Valli

 -Ben Pfaff  wrote: -

 ===
 To: Satyavalli Rama 
 From: Ben Pfaff 
 Date: 12/21/2016 10:44PM 
 Cc: ovs-dev@openvswitch.org
 Subject: Re: [ovs-dev] reg: OpenVswitch Fields Implementation
 ===
   Why are you telling us this?

On Wed, Dec 21, 2016 at 10:38:08PM +0530, Satyavalli Rama wrote:
>  Hi Ben,
> 
> Currently we are implementing OXS as an independent infrastructure.  
> While sending OXS Flow Stat Request,  we are facing the below issue.
> We have dumped and verified the packet is in proper format only, but we are 
> not receiving the Reply.
> 
> 2016-12-21T06:46:56Z|00016|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
>  sent (Success): OFPST_OXS_FLOW request (OF1.5) (xid=0x2): 
> 2016-12-21T06:46:56Z|00017|poll_loop|DBG|wakeup due to 0-ms timeout
> 2016-12-21T06:47:55Z|00018|poll_loop|DBG|wakeup due to [POLLIN] on fd 4 
> (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
> 2016-12-21T06:47:55Z|00019|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
>  received: OFPT_ECHO_REQUEST (OF1.5) (xid=0x0): 0 bytes of payload
> 2016-12-21T06:47:55Z|00020|ofctl|DBG|received reply with xid  != 
> expected 0200
> 2016-12-21T06:48:55Z|00021|poll_loop|DBG|wakeup due to [POLLIN][POLLHUP] on 
> fd 4 (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
> ovs-ofctl: OpenFlow packet receive failed (End of file)
> 
> Thanks and Regards
> Satya Valli
>   
> 
>  -Ben Pfaff  wrote: -
> 
>  ===
>  To: Satyavalli Rama 
>  From: Ben Pfaff 
>  Date: 12/16/2016 10:23PM 
>  Cc: ovs-dev@openvswitch.org
>  Subject: Re: [ovs-dev] reg: OpenVswitch Fields Implementation
>  ===
>On Fri, Dec 16, 2016 at 03:33:11PM +0530, Satyavalli Rama wrote:
> > We are working on Openflow 15 EXT-334 i.e OpenFlow Extensible Flow Entry 
> > Statistics,  as per OpenvSwitch fields implementation in the meta-flow.h, 
> > the key-value pairs for "OXS fields" to be added/declared separately or do 
> > we need to follow the existing NXM/OXM  key-value pairs.
> > 
> > Please clarify us, and please refer us some guidelines that we need to 
> > follow for implementing this Openvswitch fields.
> 
> I don't think that OXS defines fields at all, so I'd expect their
> implementation to be independent of existing infrastructure.
> 
> =-=-=
> Notice: The information contained in this e-mail
> message and/or attachments to it may contain 
> confidential or privileged information. If you are 
> not the intended recipient, any dissemination, use, 
> review, distribution, printing or copying of the 
> information contained in this e-mail message 
> and/or attachments to it are strictly prohibited. If 
> you have received this communication in error, 
> please notify us by reply e-mail or telephone and 
> immediately and permanently delete the message 
> and any attachments. Thank you
> 
> 
> 

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

Re: [ovs-dev] reg: OpenVswitch Fields Implementation

[Ben Pfaff]

Why are you telling us this?

On Wed, Dec 21, 2016 at 10:38:08PM +0530, Satyavalli Rama wrote:
>  Hi Ben,
> 
> Currently we are implementing OXS as an independent infrastructure.  
> While sending OXS Flow Stat Request,  we are facing the below issue.
> We have dumped and verified the packet is in proper format only, but we are 
> not receiving the Reply.
> 
> 2016-12-21T06:46:56Z|00016|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
>  sent (Success): OFPST_OXS_FLOW request (OF1.5) (xid=0x2): 
> 2016-12-21T06:46:56Z|00017|poll_loop|DBG|wakeup due to 0-ms timeout
> 2016-12-21T06:47:55Z|00018|poll_loop|DBG|wakeup due to [POLLIN] on fd 4 
> (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
> 2016-12-21T06:47:55Z|00019|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
>  received: OFPT_ECHO_REQUEST (OF1.5) (xid=0x0): 0 bytes of payload
> 2016-12-21T06:47:55Z|00020|ofctl|DBG|received reply with xid  != 
> expected 0200
> 2016-12-21T06:48:55Z|00021|poll_loop|DBG|wakeup due to [POLLIN][POLLHUP] on 
> fd 4 (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
> ovs-ofctl: OpenFlow packet receive failed (End of file)
> 
> Thanks and Regards
> Satya Valli
>   
> 
>  -Ben Pfaff  wrote: -
> 
>  ===
>  To: Satyavalli Rama 
>  From: Ben Pfaff 
>  Date: 12/16/2016 10:23PM 
>  Cc: ovs-dev@openvswitch.org
>  Subject: Re: [ovs-dev] reg: OpenVswitch Fields Implementation
>  ===
>On Fri, Dec 16, 2016 at 03:33:11PM +0530, Satyavalli Rama wrote:
> > We are working on Openflow 15 EXT-334 i.e OpenFlow Extensible Flow Entry 
> > Statistics,  as per OpenvSwitch fields implementation in the meta-flow.h, 
> > the key-value pairs for "OXS fields" to be added/declared separately or do 
> > we need to follow the existing NXM/OXM  key-value pairs.
> > 
> > Please clarify us, and please refer us some guidelines that we need to 
> > follow for implementing this Openvswitch fields.
> 
> I don't think that OXS defines fields at all, so I'd expect their
> implementation to be independent of existing infrastructure.
> 
> =-=-=
> Notice: The information contained in this e-mail
> message and/or attachments to it may contain 
> confidential or privileged information. If you are 
> not the intended recipient, any dissemination, use, 
> review, distribution, printing or copying of the 
> information contained in this e-mail message 
> and/or attachments to it are strictly prohibited. If 
> you have received this communication in error, 
> please notify us by reply e-mail or telephone and 
> immediately and permanently delete the message 
> and any attachments. Thank you
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] reg: OpenVswitch Fields Implementation

[Satyavalli Rama]

 Hi Ben,

Currently we are implementing OXS as an independent infrastructure.  
While sending OXS Flow Stat Request,  we are facing the below issue.
We have dumped and verified the packet is in proper format only, but we are not 
receiving the Reply.

2016-12-21T06:46:56Z|00016|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
 sent (Success): OFPST_OXS_FLOW request (OF1.5) (xid=0x2): 
2016-12-21T06:46:56Z|00017|poll_loop|DBG|wakeup due to 0-ms timeout
2016-12-21T06:47:55Z|00018|poll_loop|DBG|wakeup due to [POLLIN] on fd 4 
(<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
2016-12-21T06:47:55Z|00019|vconn|DBG|unix:/usr/local/var/run/openvswitch/br0.mgmt:
 received: OFPT_ECHO_REQUEST (OF1.5) (xid=0x0): 0 bytes of payload
2016-12-21T06:47:55Z|00020|ofctl|DBG|received reply with xid  != 
expected 0200
2016-12-21T06:48:55Z|00021|poll_loop|DBG|wakeup due to [POLLIN][POLLHUP] on fd 
4 (<->/usr/local/var/run/openvswitch/br0.mgmt) at lib/stream-fd.c:155
ovs-ofctl: OpenFlow packet receive failed (End of file)

Thanks and Regards
Satya Valli
  

 -Ben Pfaff  wrote: -

 ===
 To: Satyavalli Rama 
 From: Ben Pfaff 
 Date: 12/16/2016 10:23PM 
 Cc: ovs-dev@openvswitch.org
 Subject: Re: [ovs-dev] reg: OpenVswitch Fields Implementation
 ===
   On Fri, Dec 16, 2016 at 03:33:11PM +0530, Satyavalli Rama wrote:
> We are working on Openflow 15 EXT-334 i.e OpenFlow Extensible Flow Entry 
> Statistics,  as per OpenvSwitch fields implementation in the meta-flow.h, 
> the key-value pairs for "OXS fields" to be added/declared separately or do 
> we need to follow the existing NXM/OXM  key-value pairs.
> 
> Please clarify us, and please refer us some guidelines that we need to 
> follow for implementing this Openvswitch fields.

I don't think that OXS defines fields at all, so I'd expect their
implementation to be independent of existing infrastructure.

=-=-=
Notice: The information contained in this e-mail
message and/or attachments to it may contain 
confidential or privileged information. If you are 
not the intended recipient, any dissemination, use, 
review, distribution, printing or copying of the 
information contained in this e-mail message 
and/or attachments to it are strictly prohibited. If 
you have received this communication in error, 
please notify us by reply e-mail or telephone and 
immediately and permanently delete the message 
and any attachments. Thank you



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

Re: [ovs-dev] [PATCH] hash: Update murmurhash repo link in comments

[Ben Pfaff]

On Wed, Dec 21, 2016 at 04:39:16PM +, Cian Ferriter wrote:
> The MurmurHash code repo has moved from code.google to github. Update
> the link to reflect this.
> 
> Signed-off-by: Cian Ferriter 

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

[ovs-dev] [PATCH] hash: Update murmurhash repo link in comments

[Cian Ferriter]

The MurmurHash code repo has moved from code.google to github. Update
the link to reflect this.

Signed-off-by: Cian Ferriter 
---
 lib/hash.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/hash.h b/lib/hash.h
index f2dd510..d68ed75 100644
--- a/lib/hash.h
+++ b/lib/hash.h
@@ -51,7 +51,7 @@ static inline uint32_t hash_pointer(const void *, uint32_t 
basis);
 static inline uint32_t hash_string(const char *, uint32_t basis);
 
 /* Murmurhash by Austin Appleby,
- * from http://code.google.com/p/smhasher/source/browse/trunk/MurmurHash3.cpp.
+ * from https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp
  *
  * The upstream license there says:
  *
-- 
1.7.0.7

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

[ovs-dev] Recursos Humanos - 12 Temas

[Compras - 12 Conferencias inéditas]

 

12 conferencias pregrabadas / Durante 3 meses 

Póliza Indispensable para 
los Responsables de RECURSOS HUMANOS
12 conferencias en cada póliza, pregrabadas, inéditas, 
para capacitar a todo su personal   
 
Al adquirir la Póliza de Capacitación, usted obtiene acceso a 12 temas 
enfocados al área de Recursos Humanos; temas especializados que han sido 
cuidadosamente seleccionados por nuestros expertos en entrenamiento ejecutivo 
con el ?n de brindar a las empresas la actualización que necesitan para llevar 
su competitividad a su máximo potencial. Su póliza le brinda acceso a TODA la 
programación, para aprovecharla cuando quiera, todas las veces que quiera 
durante sus tres meses de acceso.  
"Pregunte por nuestra Promoción Navideña en Pólizas"

TEMAS: 

1. Recursos Humanos para principiantes y sus principales funciones.

2. Planeación, organización y control del área de Recursos Humanos.


3. Elaboración de políticas y manuales de inducción para una excelente 
integración.

4. Reclutamiento: dónde localizar talento humano ideal para la Empresa (método 
tradicional vs. redes sociales).

5. Diversas técnicas de entrevista para la selección del personal.

- Y mucho más.


 
¿Requiere la información a la Brevedad?
responda este email con la palabra: 
Info - Recursos.
centro telefónico: 018002129393


 
¿Demasiados mensajes en su cuenta? Responda este mensaje indicando que solo 
desea recibir CALENDARIO y sólo recibirá un correo al mes. Si desea cancelar la 
suscripción, solicite su BAJA..
 

 

 

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

Re: [ovs-dev] [RFC v2] lib/table: add new flexible formatting

[Ben Pfaff]

On Wed, Dec 21, 2016 at 10:07:51AM -0500, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > On Tue, Dec 13, 2016 at 04:43:32PM -0500, Aaron Conole wrote:
> >> Ben Pfaff  writes:
> >> 
> >> > On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote:
> >> >> Ben Pfaff  writes:
> >> >> 
> >> >> > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote:
> >> >> >> Ben Pfaff  writes:
> >> >> >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote:
> >> >> >> >> Ben Pfaff  writes:
> >> >> >> >> To accomplish this, I'm going down the following route:
> >> >> >> >> 
> >> >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can
> >> >> >> >>still express the existing output.
> >> >> >> >> 
> >> >> >> >> 2. Change ofp-print to use a table for flow dumping, with a 
> >> >> >> >> default
> >> >> >> >>format that preserves the current format, and the option of 
> >> >> >> >> changing
> >> >> >> >>the format.
> >> >> >> >
> >> >> >> > #2 is interesting.
> >> >> >> 
> >> >> >> I don't currently see how to implement #2, while preserving the 
> >> >> >> existing
> >> >> >> output, unless I implement #1.  Is there a way to accomplish this 
> >> >> >> that
> >> >> >> I'm missing?  If not, is #2 compelling enough to accept #1?
> >> >> >
> >> >> > I don't understand how you are going to preserve the default format 
> >> >> > even
> >> >> > with #1.  I assumed you were going to need to write a separate code 
> >> >> > path
> >> >> > to do that.
> >> >> 
> >> >> No - that's my last resort.
> >> >> 
> >> >> > Can you explain your plan?
> >> >> 
> >> >> I was thinking of creating a table which had column headings that were,
> >> >> example:
> >> >> 
> >> >>   actions, in_port, priority, table number, etc.
> >> >> 
> >> >> Then a format string such as:
> >> >> 
> >> >>   [priority={priority},][table={tableno},]...
> >> >> 
> >> >> where things in the [] would only be printed if all elements in the row
> >> >> were filled in, could preserve the original output (without the full
> >> >> working code set it might be difficult to see what I mean), while also
> >> >> offering the existing --table=XXX output.
> >> >
> >> > This may be challenging because of the number of columns.  in_port is
> >> > easy enough, but OVS has dozens (at least) of possible fields.  It also
> >> > has a nonuniform way to display fields.  For example, it shows the
> >> > Ethernet type in a general form as "dl_type=0x1234", but if it's an IP
> >> > packet then it just says "ip" instead, and if it's TCP then it just says
> >> > "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on.
> >> 
> >> I enjoy a challenge from time to time ;-)
> >> 
> >> Moreover, I'm willing to do the work.  Agreed, there's lots of it
> >> (dozens is right), but I think it's an elegant solution.
> >
> > Do you have a basic plan for how to do it?  There's definitely room for
> > better display here, but I want the code to be clean and maintainable.
> 
> So, every plan that I've gone through starts with a separate code path
> for the table approach if I want to stage it.
> 
> It probably makes sense to start there, and add the fields I've been
> requested to add, first, and as part of the same series hook up
> ovs-ofctl dump-flows to call that new code path.
> 
> Does that make sense?

Sure.  I'd prefer to make the whole thing a single series because in
this case I don't want to add library code that won't be used, but it
sounds like you already plan to do it that way.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v8] netdev-dpdk: Increase pmd thread priority

[Bhanuprakash Bodireddy]

Increase the DPDK pmd thread scheduling priority by lowering the nice
value. This will advise the kernel scheduler to prioritize pmd thread
over other processes.

Signed-off-by: Bhanuprakash Bodireddy 
---
v7->v8:
* Rebase
* Update the documentation file @Documentation/intro/install/dpdk-advanced.rst

v6->v7:
* Remove realtime scheduling policy logic.
* Increase pmd thread scheduling priority by lowering nice value to -20.
* Update doc accordingly.

v5->v6:
* Prohibit spawning pmd thread on the lowest core in dpdk-lcore-mask if
  lcore-mask and pmd-mask affinity are identical.
* Updated Note section in INSTALL.DPDK-ADVANCED doc.
* Tested below cases to verify system stability with pmd priority patch

v4->v5:
* Reword Note section in DPDK-ADVANCED.md

v3->v4:
* Document update
* Use ovs_strerror for reporting errors in lib-numa.c

v2->v3:
* Move set_priority() function to lib/ovs-numa.c
* Apply realtime scheduling policy and priority to pmd thread only if
  pmd-cpu-mask is passed.
* Update INSTALL.DPDK-ADVANCED.

v1->v2:
* Removed #ifdef and introduced dummy function "pmd_thread_setpriority"
  in netdev-dpdk.h
* Rebase

 Documentation/intro/install/dpdk-advanced.rst |  8 +++-
 lib/dpif-netdev.c |  4 
 lib/ovs-numa.c| 19 +++
 lib/ovs-numa.h|  1 +
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/intro/install/dpdk-advanced.rst 
b/Documentation/intro/install/dpdk-advanced.rst
index 44d1cd7..67815ac 100644
--- a/Documentation/intro/install/dpdk-advanced.rst
+++ b/Documentation/intro/install/dpdk-advanced.rst
@@ -238,7 +238,8 @@ affinitized accordingly.
   to be affinitized to isolated cores for optimum performance.
 
   By setting a bit in the mask, a pmd thread is created and pinned to the
-  corresponding CPU core. e.g. to run a pmd thread on core 2::
+  corresponding CPU core and the nice value set to '-20'.
+  e.g. to run a pmd thread on core 2::
 
   $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x4
 
@@ -278,6 +279,11 @@ improvements as there will be more total CPU occupancy 
available::
 
 NIC port0 <-> OVS <-> VM <-> OVS <-> NIC port 1
 
+  .. note::
+It is recommended that the OVS control thread and pmd thread shouldn't be
+pinned to the same core i.e 'dpdk-lcore-mask' and 'pmd-cpu-mask' cpu mask
+settings should be non-overlapping.
+
 DPDK Physical Port Rx Queues
 
 
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0b73056..f58a855 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3120,6 +3120,10 @@ pmd_thread_main(void *f_)
 ovs_numa_thread_setaffinity_core(pmd->core_id);
 dpdk_set_lcore_id(pmd->core_id);
 poll_cnt = pmd_load_queues_and_ports(pmd, &poll_list);
+
+/* Set pmd thread's nice value to -20 */
+#define MIN_NICE -20
+ovs_numa_thread_setpriority(MIN_NICE);
 reload:
 emc_cache_init(&pmd->flow_cache);
 
diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index e1e7068..d52c517 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #endif /* __linux__ */
@@ -620,3 +621,21 @@ int ovs_numa_thread_setaffinity_core(unsigned core_id 
OVS_UNUSED)
 return EOPNOTSUPP;
 #endif /* __linux__ */
 }
+
+void
+ovs_numa_thread_setpriority(int nice OVS_UNUSED)
+{
+if (dummy_numa) {
+return;
+}
+
+#ifndef _WIN32
+int err;
+err = setpriority(PRIO_PROCESS, 0, nice);
+if (err) {
+VLOG_ERR("Thread priority error %s",ovs_strerror(err));
+}
+#else
+return EOPNOTSUPP;
+#endif
+}
diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h
index be836b2..3ce13c7 100644
--- a/lib/ovs-numa.h
+++ b/lib/ovs-numa.h
@@ -56,6 +56,7 @@ void ovs_numa_unpin_core(unsigned core_id);
 struct ovs_numa_dump *ovs_numa_dump_cores_on_numa(int numa_id);
 void ovs_numa_dump_destroy(struct ovs_numa_dump *);
 int ovs_numa_thread_setaffinity_core(unsigned core_id);
+void ovs_numa_thread_setpriority(int nice);
 
 #define FOR_EACH_CORE_ON_NUMA(ITER, DUMP)\
 LIST_FOR_EACH((ITER), list_node, &(DUMP)->dump)
-- 
2.4.11

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

Re: [ovs-dev] [PATCH v3 05/16] ofp-actions: Add clone action.

[William Tu]

Hi Joe,

I think we're missing the normal action for handling the non-IP
traffic. Can you try this patch on top of the clone?

--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -340,6 +340,7 @@ AT_CLEANUP
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()

+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
 ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)

 ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")

Thanks
William

On Tue, Dec 20, 2016 at 4:39 PM, William Tu  wrote:
> Hi Joe,
> Thanks I will take a look.
> William
>
> On Tue, Dec 20, 2016 at 10:25 AM, Joe Stringer  wrote:
>> On 18 December 2016 at 00:18, Ben Pfaff  wrote:
>>> From: William Tu 
>>>
>>> This patch adds OpenFlow clone action with syntax as below:
>>> "clone([action][,action...])".  The clone() action makes a copy of the
>>> current packet and executes the list of actions against the packet,
>>> without affecting the packet after the "clone(...)" action.  In other
>>> word, the packet before the clone() and after the clone() is the same,
>>> no matter what actions executed inside the clone().
>>>
>>> Use case 1:
>>> Set different fields and output to different ports without unset
>>> actions=
>>>   clone(mod_dl_src:, output:1), clone(mod_dl_dst:, output:2), 
>>> output:3
>>> Since each clone() has independent packet, output:1 has only dl_src 
>>> modified,
>>> output:2 has only dl_dst modified, output:3 has original packet.
>>>
>>> Similar to case1
>>> actions=
>>>   push_vlan(...), output:2, pop_vlan, push_vlan(...), output:3
>>> can be changed to
>>> actions=
>>>   clone(push_vlan(...), output:2),clone(push_vlan(...), output:3)
>>> without having to add pop_vlan.
>>>
>>> case 2: resubmit to another table without worrying packet being modified
>>>   actions=clone(resubmit(1,2)), ...
>>>
>>> Signed-off-by: William Tu 
>>> [b...@ovn.org revised this to omit the "sample" action]
>>> Signed-off-by: Ben Pfaff 
>>
>> It looks like the newly added system-traffic tests are failing on all
>> platforms with this patch applied.. William, will you take a look?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC PATCH] netdev-dpdk: Add OVS DPDK keep-alive functionality.

[Bhanuprakash Bodireddy]

This patch is aimed at achieving Fastpath Service Assurance in
OVS-DPDK deployments. This commit adds support for monitoring the packet
processing cores(pmd thread cores) by dispatching heartbeats at regular
intervals. Incase of heartbeat miss the failure shall be detected &
reported to higher level fault management systems/frameworks.

The implementation uses POSIX shared memory object for storing the
events that will be read by monitoring framework. keep-alive feature
can be enabled through below OVSDB settings.

dpdk-keepalive=true
   - Keepalive feature is disabled by default

dpdk-keepalive-interval="50"
   - Timer interval in milliseconds for monitoring the packet
 processing cores.

dpdk-keepalive-shm-name="/dpdk_keepalive_shm_name"
   - Shared memory block name where the events shall be updated.

When KA is enabled, 'ovs-keepalive' thread shall be spawned that wakes
up at regular intervals to update the timestamp and status of pmd cores
in shared memory region.

An external monitoring framework like collectd(with dpdk plugin support)
can read the status updates from shared memory. On a missing heartbeat,
the collectd shall relay the status to ceilometer service in the
controller. Below is the very high level overview of deployment model.

Compute Node   Controller

 Collectd  <-> Ceilometer

 OVS DPDK

   +-+
   | VM  |
   +--+--+
  \---+---/
  |
   +--+---+   ++--+ +--+---+
   | OVS  |-> |  collectd DPDK plugin | --> |   collectd   |
   +--+---+   ++--+ +--+---+

 +--+-+ +---++
 | Ceilometer | <-- | collectd ceilometer plugin |  <
 +--+-+ +---++

Signed-off-by: Bhanuprakash Bodireddy 
---
This patch is based on commit '8b6987d799fb0bc530ebb7f767767b1c661548c9'
and [PATCH v2 00/19] DPDK/pmd reconfiguration refactor and bugfixes

Will post v1 of this patch with documentation updates once "v2 of pmd
reconfiguration" and the documentation patches are upstreamed.

 lib/dpdk-stub.c  |  24 ++
 lib/dpdk.c   |  83 
 lib/dpdk.h   |  10 ++-
 lib/dpif-netdev.c|   9 +++
 lib/netdev-dpdk.c| 218 ++-
 lib/netdev-dpdk.h|   7 +-
 lib/process.c|  57 ++
 lib/process.h|  10 +++
 lib/util.c   |  12 +++
 lib/util.h   |   1 +
 vswitchd/vswitch.xml |  39 +
 11 files changed, 467 insertions(+), 3 deletions(-)

diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
index bd981bb..0b28c97 100644
--- a/lib/dpdk-stub.c
+++ b/lib/dpdk-stub.c
@@ -48,3 +48,27 @@ dpdk_get_vhost_sock_dir(void)
 {
 return NULL;
 }
+
+void
+dpdk_ka_register_core(unsigned core_id OVS_UNUSED)
+{
+/* Nothing */
+}
+
+void
+dpdk_ka_mark_core_alive(void)
+{
+/* Nothing */
+}
+
+void
+dpdk_ka_shm_store_tid(unsigned core_id OVS_UNUSED)
+{
+/* Nothing */
+}
+
+bool
+dpdk_is_ka_enabled(void)
+{
+return false;
+}
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 49a589a..cd1b485 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -15,12 +15,14 @@
  */
 
 #include 
+#include 
 #include "dpdk.h"
 
 #include 
 #include 
 #include 
 
+#include 
 #include 
 #ifdef DPDK_PDUMP
 #include 
@@ -38,6 +40,10 @@ VLOG_DEFINE_THIS_MODULE(dpdk);
 
 static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets */
 
+bool ovs_keepalive_enable = false;/* KA feature disabled by default */
+static int keepalive_timer_interval;  /* OvS-DPDK keepalive timer interval */
+static const char *keepalive_shm_blk = NULL;
+
 static int
 process_vhost_flags(char *flag, char *default_val, int size,
 const struct smap *ovs_other_config,
@@ -63,6 +69,35 @@ process_vhost_flags(char *flag, char *default_val, int size,
 return changed;
 }
 
+/* Retrieve and return the keepalive timer interval from OVSDB. */
+static int
+get_ka_timer_interval(const struct smap *ovs_other_config)
+{
+#define OVS_KEEPALIVE_TIMEOUT 100/* Default timeout set to 100ms */
+int ka_interval;
+
+/* Timer granularity in milliseconds
+ * Defaults to OVS_KEEPALIVE_TIMEOUT(ms) if not set */
+ka_interval = smap_get_int(ovs_other_config, "dpdk-keepalive-interval",
+  OVS_KEEPALIVE_TIMEOUT);
+
+VLOG_INFO("The keepalive timer interval: %d\n", ka_interval);
+return ka_interval;
+}
+
+static const char *
+get_ka_shm_block(const struct smap *ovs_other_config)
+{
+#define OVS_KEEPALIVE_SHM_NAME /dpdk_keepalive_shm_name  /* Shared mem block. 
*/
+keepalive_shm_blk = smap_get(ovs_other_config, "dpdk-keepalive-shm-name");
+if (!keepalive_shm_blk) {
+keepalive_shm_blk = OVS_STRINGIZE(OVS_KEEPALIVE_SHM_NAME);
+}
+
+VLOG_INFO("The keepalive Shared Memory block: %s\n", keepalive_shm_blk);
+return keepalive_shm_blk;
+}
+
 static char **
 grow

Re: [ovs-dev] [RFC v2] lib/table: add new flexible formatting

[Aaron Conole]

Ben Pfaff  writes:

> On Tue, Dec 13, 2016 at 04:43:32PM -0500, Aaron Conole wrote:
>> Ben Pfaff  writes:
>> 
>> > On Mon, Dec 12, 2016 at 08:11:16PM -0500, Aaron Conole wrote:
>> >> Ben Pfaff  writes:
>> >> 
>> >> > On Wed, Dec 07, 2016 at 02:15:12PM -0500, Aaron Conole wrote:
>> >> >> Ben Pfaff  writes:
>> >> >> > On Wed, Dec 07, 2016 at 01:08:43PM -0500, Aaron Conole wrote:
>> >> >> >> Ben Pfaff  writes:
>> >> >> >> To accomplish this, I'm going down the following route:
>> >> >> >> 
>> >> >> >> 1. Add formatting option to the lib/table.{c,h}, such that we can
>> >> >> >>still express the existing output.
>> >> >> >> 
>> >> >> >> 2. Change ofp-print to use a table for flow dumping, with a default
>> >> >> >>format that preserves the current format, and the option of 
>> >> >> >> changing
>> >> >> >>the format.
>> >> >> >
>> >> >> > #2 is interesting.
>> >> >> 
>> >> >> I don't currently see how to implement #2, while preserving the 
>> >> >> existing
>> >> >> output, unless I implement #1.  Is there a way to accomplish this that
>> >> >> I'm missing?  If not, is #2 compelling enough to accept #1?
>> >> >
>> >> > I don't understand how you are going to preserve the default format even
>> >> > with #1.  I assumed you were going to need to write a separate code path
>> >> > to do that.
>> >> 
>> >> No - that's my last resort.
>> >> 
>> >> > Can you explain your plan?
>> >> 
>> >> I was thinking of creating a table which had column headings that were,
>> >> example:
>> >> 
>> >>   actions, in_port, priority, table number, etc.
>> >> 
>> >> Then a format string such as:
>> >> 
>> >>   [priority={priority},][table={tableno},]...
>> >> 
>> >> where things in the [] would only be printed if all elements in the row
>> >> were filled in, could preserve the original output (without the full
>> >> working code set it might be difficult to see what I mean), while also
>> >> offering the existing --table=XXX output.
>> >
>> > This may be challenging because of the number of columns.  in_port is
>> > easy enough, but OVS has dozens (at least) of possible fields.  It also
>> > has a nonuniform way to display fields.  For example, it shows the
>> > Ethernet type in a general form as "dl_type=0x1234", but if it's an IP
>> > packet then it just says "ip" instead, and if it's TCP then it just says
>> > "tcp" instead of "dl_type=0x0800,nw_proto=6", and so on.
>> 
>> I enjoy a challenge from time to time ;-)
>> 
>> Moreover, I'm willing to do the work.  Agreed, there's lots of it
>> (dozens is right), but I think it's an elegant solution.
>
> Do you have a basic plan for how to do it?  There's definitely room for
> better display here, but I want the code to be clean and maintainable.

So, every plan that I've gone through starts with a separate code path
for the table approach if I want to stage it.

It probably makes sense to start there, and add the fields I've been
requested to add, first, and as part of the same series hook up
ovs-ofctl dump-flows to call that new code path.

Does that make sense?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev