Re: [ovs-dev] [bug] failed to add ovs bridge

2017-05-17 Thread Ben Pfaff 
I don't plan to pursue this because it only triggers when you do
something that I don't think makes sense.  It could be a bug in the
Linux bridge, or in OVS, or in their interaction, or in libvirt.  If you
narrow down the problem, or find a fix, please let us know.

On Thu, May 18, 2017 at 12:47:51AM +, fukaige wrote:
> No. I just hit this in the situation. Do you think this is a bug? What's your 
> next plan?
> 
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Wednesday, May 17, 2017 11:55 PM
> > To: fukaige
> > Cc: d...@openvswitch.org
> > Subject: Re: [bug] failed to add ovs bridge
> > 
> > Do you still see failures when the Linux bridge is not involved at all?
> > 
> > On Wed, May 17, 2017 at 03:52:54AM +, fukaige wrote:
> > > I start a vm with its backend tap device "vnet0" attached to a Linux 
> > > bridge,
> > then delete the bridge and vm.
> > > Next time, vm will start failed. this is as expected. But, seeing that
> > > linux bridge was already delelted; there is no reason that the OVS bridge 
> > > with
> > the same name can not be created. What do you think?
> > >
> > > Thanks
> > > fukaige
> > >
> > > > -Original Message-
> > > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > > Sent: Wednesday, May 17, 2017 11:37 AM
> > > > To: fukaige
> > > > Cc: d...@openvswitch.org
> > > > Subject: Re: [bug] failed to add ovs bridge
> > > >
> > > > On Wed, May 17, 2017 at 01:16:11AM +, fukaige wrote:
> > > > > I am using ovs-2.5.2 and get error when creating a bridge using
> > > > > “ovs-vsctl
> > > > add-br br-eth”
> > > >
> > > > > brctl addbr br0;ifconfig br0 up
> > > >
> > > > [...]
> > > >
> > > > > ifconfig br0 down; brctl delbr br0
> > > > >
> > > > > ovs-vsctl add-br br0
> > > > >
> > > > > ovs-vsctl del-br br0
> > > >
> > > > Wait, what?  Why are you mixing brctl and ovs-vsctl?  That makes no
> > sense.
> > > > Use the Linux bridge *or* OVS, not a mix of them.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] packets: Do not initialize ct_orig_tuple.

2017-05-17 Thread Joe Stringer 
On 17 May 2017 at 16:26, Darrell Ball  wrote:
>
>
> On 5/17/17, 2:19 PM, "Joe Stringer"  wrote:
>
> On 16 May 2017 at 21:01, Darrell Ball  wrote:
> >
> >
> > On 5/15/17, 6:00 AM, "ovs-dev-boun...@openvswitch.org on behalf of 
> Bodireddy, Bhanuprakash"  bhanuprakash.bodire...@intel.com> wrote:
> >
> > >Commit daf4d3c18da4("odp: Support conntrack orig tuple key.") 
> introduced
> > >new fields in struct 'pkt_metadata'.  pkt_metadata_init() is 
> called for every
> > >packet in the userspace datapath.  When testing a simple single 
> flow case with
> > >DPDK, we observe a lower throughput after the above commit (it was 
> 14.88
> > >Mpps before, it is 13 Mpps after).
> > >
> > >This patch skips initializing ct_orig_tuple in pkt_metadata_init().
> > >It should be enough to initialize ct_state, because nobody should 
> look at
> > >ct_orig_tuple unless ct_state is != 0.
> > >
> > >CC: Jarno Rajahalme 
> > >Signed-off-by: Daniele Di Proietto 
> > >---
> > >I'm sending this as an RFC because I didn't check very carefully 
> if we can really
> > >avoid initializing ct_orig_tuple.
> > >
> > >Maybe there are better solutions to this problem.
> > >---
> > > lib/packets.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/lib/packets.h b/lib/packets.h index 
> a5a483bc8..6f1791c7a 100644
> > >--- a/lib/packets.h
> > >+++ b/lib/packets.h
> > >@@ -129,7 +129,7 @@ pkt_metadata_init(struct pkt_metadata *md,
> > >odp_port_t port)
> > > /* It can be expensive to zero out all of the tunnel 
> metadata. However,
> > >  * we can just zero out ip_dst and the rest of the data will 
> never be
> > >  * looked at. */
> > >-memset(md, 0, offsetof(struct pkt_metadata, in_port));
> > >+memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple));
> > > md->tunnel.ip_dst = 0;
> > > md->tunnel.ipv6_dst = in6addr_any;
> > >
> >
> > It's been a while this RFC patch has been submitted to fix 
> performance drop on Master. This indeed fixes the OvS-DPDK performance drop 
> that was introduced by the conntrack commit.
> > Is there a better fix than what is suggested above?
> >
> >
> > This affects both kernel and userspace.
> > I tested this on both datapaths.
> > LGTM
>
> How do we make sure that ct_orig_tuple isn't used uninitialized? Do we
> need to zero out the ct_orig_tuple proto?
>
> There are a couple places where we explicitly set or reset the pkt metadata 
> ct_orig_tuple;
> one is in pkt_metadata_from_flow().
>
> I know there is a check for ct_orig_tuple proto in odp_key_from_pkt_metadata()
> Can you find a case where can run this code without a set or reset of 
> ct_orig_tuple pkt md
> or you are not sure ?

I wasn't sure, and I didn't see a response to the question that
Daniele asked below the dashes in the original submission.

Is miniflow_extract() safe wrt this? Seems like plausibly if the
recirc_id is nonzero but there is no CT state (because, eg, bonds, or
MPLS pop to IP, etc) it could end up populating the miniflow with
garbage. In particular the path from emc_processing() seems suspect.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [bug] failed to add ovs bridge

2017-05-17 Thread fukaige 
No. I just hit this in the situation. Do you think this is a bug? What's your 
next plan?

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Wednesday, May 17, 2017 11:55 PM
> To: fukaige
> Cc: d...@openvswitch.org
> Subject: Re: [bug] failed to add ovs bridge
> 
> Do you still see failures when the Linux bridge is not involved at all?
> 
> On Wed, May 17, 2017 at 03:52:54AM +, fukaige wrote:
> > I start a vm with its backend tap device "vnet0" attached to a Linux bridge,
> then delete the bridge and vm.
> > Next time, vm will start failed. this is as expected. But, seeing that
> > linux bridge was already delelted; there is no reason that the OVS bridge 
> > with
> the same name can not be created. What do you think?
> >
> > Thanks
> > fukaige
> >
> > > -Original Message-
> > > From: Ben Pfaff [mailto:b...@ovn.org]
> > > Sent: Wednesday, May 17, 2017 11:37 AM
> > > To: fukaige
> > > Cc: d...@openvswitch.org
> > > Subject: Re: [bug] failed to add ovs bridge
> > >
> > > On Wed, May 17, 2017 at 01:16:11AM +, fukaige wrote:
> > > > I am using ovs-2.5.2 and get error when creating a bridge using
> > > > “ovs-vsctl
> > > add-br br-eth”
> > >
> > > > brctl addbr br0;ifconfig br0 up
> > >
> > > [...]
> > >
> > > > ifconfig br0 down; brctl delbr br0
> > > >
> > > > ovs-vsctl add-br br0
> > > >
> > > > ovs-vsctl del-br br0
> > >
> > > Wait, what?  Why are you mixing brctl and ovs-vsctl?  That makes no
> sense.
> > > Use the Linux bridge *or* OVS, not a mix of them.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH branch-2.7 2/2] NEWS: Describe libopenvswitch-X.Y change.

2017-05-17 Thread Joe Stringer 
The previous commit renamed libopenvswitch-X to libopenvswitch-X.Y to
allow ABI breakage to be expressed through libtool numbering. Document
this change.

Signed-off-by: Joe Stringer 
---
 NEWS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/NEWS b/NEWS
index c68ecd74c97b..bbff66af3735 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,8 @@ v2.7.1 - xx xxx 
- Fedora Packaging:
  * OVN services are no longer restarted automatically after upgrade.
- Add --cleanup option to command 'ovs-appctl exit' (see ovs-vswitchd(8)).
+   - libopenvswitch-2 was renamed to libopenvswitch-2.7. Applications built
+ against libopenvswitch must be recompiled against the newer library.
 
 v2.7.0 - 21 Feb 2017
 -
-- 
2.11.1

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

[ovs-dev] [PATCH branch-2.7 1/2] libopenvswitch: Rename to libfoo-X.Y.

2017-05-17 Thread Joe Stringer 
The current intent for Open vSwitch is to maintain libopenvswitch ABI
stability for minor versions, for example each release within the 2.7.z
series. According to the following documentation, no changes to exported
headers should be made.

http://docs.openvswitch.org/en/latest/internals/contributing/libopenvswitch-abi/

However, it is occasionally necessary to make changes to
{include/openvswitch,lib}/*.h headers to fix issues within a given
release series. The current libtool tagging mechanism in the build
system does not allow for this without creating a conflict between the
libtool 'current' version and the next minor release of OVS.

This patch modifies libopenvswitch build to include the MAJOR.MINOR
release version in the libX name, and include the libtool CURRENT and
OVS MICRO release in the libtool versioning tags to indicate library
stability. The resulting format is "libfoo-X.Y.so.CURRENT.0.Z" for OVS
release "X.Y.Z".

Developers should still attempt to avoid introducing ABI-breaking changes
within a particular OVS-X.Y release series, but if this is not possible
this patch introduced a mechanism to allow an ABI-breaking fix to be
introduced. In such a case, developers may update the libtool CURRENT
version to indicate this breakage to library users.

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
 .../internals/contributing/libopenvswitch-abi.rst| 16 +---
 m4/openvswitch.m4|  6 +++---
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/Documentation/internals/contributing/libopenvswitch-abi.rst 
b/Documentation/internals/contributing/libopenvswitch-abi.rst
index 845d90898859..8fa24db964cd 100644
--- a/Documentation/internals/contributing/libopenvswitch-abi.rst
+++ b/Documentation/internals/contributing/libopenvswitch-abi.rst
@@ -87,16 +87,18 @@ ABI Policy
 --
 
 Open vSwitch will export the ABI version at the time of release, such that the
-library name will be the major version, and the rest of the release version
-information will be conveyed with a libtool interface version.
+library name will be the major.minor version, and the rest of the release
+version information will be conveyed with a libtool interface version.
 
 The intent is for Open vSwitch to maintain an ABI stability for each minor
 revision only (so that Open vSwitch release 2.5 carries a guarantee for all
-2.5.ZZ micro-releases).  This means that any porting effort to stable branches
-must take not to disrupt the existing ABI.  Each new 'minor-level' release
-bumps the libtool 'current' version, which informs the linker of a backwards
-incompatible interface, signaling that libraries exposed by Open vSwitch 2.6
-will not maintain ABI stability with Open vSwitch 2.5.
+2.5.ZZ micro-releases). This means that any porting effort to stable branches
+must take not to disrupt the existing ABI.
+
+In the event that a bug must be fixed in a backwards-incompatible way,
+developers must bump the libtool 'current' version to inform the linker of the
+ABI breakage. This will signal that libraries exposed by the subsequent release
+will not maintain ABI stability with the previous version.
 
 Coding
 ---
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 07c61911aac9..435f8923f7be 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -610,9 +610,9 @@ AC_DEFUN([OVS_LIBTOOL_VERSIONS],
   OVS_MAJOR=`echo "$PACKAGE_VERSION" | sed -e 's/[[.]].*//'`
   OVS_MINOR=`echo "$PACKAGE_VERSION" | sed -e "s/^$OVS_MAJOR//" -e 's/^.//' -e 
's/[[.]].*//'`
   OVS_MICRO=`echo "$PACKAGE_VERSION" | sed -e "s/^$OVS_MAJOR.$OVS_MINOR//" -e 
's/^.//' -e 's/[[^0-9]].*//'`
-  OVS_LT_RELINFO="-release $OVS_MAJOR"
-  OVS_LT_VERINFO="-version-info $OVS_MINOR:$OVS_MICRO"
+  OVS_LT_RELINFO="-release $OVS_MAJOR.$OVS_MINOR"
+  OVS_LT_VERINFO="-version-info $LT_CURRENT:$OVS_MICRO"
   OVS_LTINFO="$OVS_LT_RELINFO $OVS_LT_VERINFO"
-  AC_MSG_RESULT([libX-$OVS_MAJOR.so.$OVS_MINOR.0.$OVS_MICRO)])
+  AC_MSG_RESULT([libX-$OVS_MAJOR.$OVS_MINOR.so.$LT_CURRENT.0.$OVS_MICRO)])
   AC_SUBST(OVS_LTINFO)
 ])
-- 
2.11.1

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

Re: [ovs-dev] [RFC 0/2] Rename libopenvswitch to include MAJOR.MINOR number.

2017-05-17 Thread Joe Stringer 
On 4 May 2017 at 13:14, Ben Pfaff  wrote:
> On Wed, Apr 26, 2017 at 04:08:01PM -0700, Joe Stringer wrote:
>> I have recently been looking into backporting a fix for variable-length
>> metadata fields from master to branch-2.7. Unfortunately, the fix makes 
>> several
>> changes to libopenvswitch APIs, in particular some files under
>> include/openvswitch/*.h and lib/*.h. From the perspective of the
>> openvswitch-dev packages, these are visible in
>> /usr/include/openvswitch/{,lib/}*.h.
>>
>> This is the series:
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329811.html
>
> Thanks for working on this.
>
> I don't know of a big downside to this approach, and it seems like it
> has valuable upsides.
>
> For both:
>
> Acked-by: Ben Pfaff 

Thanks for the feedback Aaron and Ben, applied to master. I'll prepare
it for branch-2.7 as well.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC] packets: Do not initialize ct_orig_tuple.

2017-05-17 Thread Darrell Ball 


On 5/17/17, 2:19 PM, "Joe Stringer"  wrote:

On 16 May 2017 at 21:01, Darrell Ball  wrote:
>
>
> On 5/15/17, 6:00 AM, "ovs-dev-boun...@openvswitch.org on behalf of 
Bodireddy, Bhanuprakash"  wrote:
>
> >Commit daf4d3c18da4("odp: Support conntrack orig tuple key.") 
introduced
> >new fields in struct 'pkt_metadata'.  pkt_metadata_init() is called 
for every
> >packet in the userspace datapath.  When testing a simple single flow 
case with
> >DPDK, we observe a lower throughput after the above commit (it was 
14.88
> >Mpps before, it is 13 Mpps after).
> >
> >This patch skips initializing ct_orig_tuple in pkt_metadata_init().
> >It should be enough to initialize ct_state, because nobody should 
look at
> >ct_orig_tuple unless ct_state is != 0.
> >
> >CC: Jarno Rajahalme 
> >Signed-off-by: Daniele Di Proietto 
> >---
> >I'm sending this as an RFC because I didn't check very carefully if 
we can really
> >avoid initializing ct_orig_tuple.
> >
> >Maybe there are better solutions to this problem.
> >---
> > lib/packets.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/packets.h b/lib/packets.h index 
a5a483bc8..6f1791c7a 100644
> >--- a/lib/packets.h
> >+++ b/lib/packets.h
> >@@ -129,7 +129,7 @@ pkt_metadata_init(struct pkt_metadata *md,
> >odp_port_t port)
> > /* It can be expensive to zero out all of the tunnel metadata. 
However,
> >  * we can just zero out ip_dst and the rest of the data will 
never be
> >  * looked at. */
> >-memset(md, 0, offsetof(struct pkt_metadata, in_port));
> >+memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple));
> > md->tunnel.ip_dst = 0;
> > md->tunnel.ipv6_dst = in6addr_any;
> >
>
> It's been a while this RFC patch has been submitted to fix 
performance drop on Master. This indeed fixes the OvS-DPDK performance drop 
that was introduced by the conntrack commit.
> Is there a better fix than what is suggested above?
>
>
> This affects both kernel and userspace.
> I tested this on both datapaths.
> LGTM

How do we make sure that ct_orig_tuple isn't used uninitialized? Do we
need to zero out the ct_orig_tuple proto?

There are a couple places where we explicitly set or reset the pkt metadata 
ct_orig_tuple;
one is in pkt_metadata_from_flow().

I know there is a check for ct_orig_tuple proto in odp_key_from_pkt_metadata()
Can you find a case where can run this code without a set or reset of 
ct_orig_tuple pkt md
or you are not sure ?






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

[ovs-dev] [PATCH] ovn-controller: Fix log conditions for unexpected openflow messages.

2017-05-17 Thread Han Zhou 
Currently in pinctrl.c and ofctrl.c there are similar logic to log
ignored messages, which is somehow inaccurate and confusing. For example,
OFPTYPE_PACKET_IN is handled only in pinctrl.c but in ofctrl.c it
is listed as expected input and not logged as "ignored" messages, while
it is in fact unexpected and ignored there. This patch clearup the
unnecessary "if" conditions and logs all messages that are not
expected/handled honestly, so that there will be logs for debugging
if such abnormal case really happens.

Signed-off-by: Han Zhou 
---
 ovn/controller/ofctrl.c  | 6 +-
 ovn/controller/pinctrl.c | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index 417fdc9..4b4277a 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -587,11 +587,7 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type)
 } else if (type == OFPTYPE_ERROR) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
 log_openflow_rl(, VLL_INFO, oh, "OpenFlow error");
-} else if (type != OFPTYPE_ECHO_REPLY &&
-   type != OFPTYPE_BARRIER_REPLY &&
-   type != OFPTYPE_PACKET_IN &&
-   type != OFPTYPE_PORT_STATUS &&
-   type != OFPTYPE_FLOW_REMOVED) {
+} else {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
 log_openflow_rl(, VLL_DBG, oh, "OpenFlow packet ignored");
 }
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 9ad4133..b93cf06 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -991,7 +991,7 @@ pinctrl_recv(const struct ofp_header *oh, enum ofptype type,
 set_switch_config(swconn, );
 } else if (type == OFPTYPE_PACKET_IN) {
 process_packet_in(oh, ctx);
-} else if (type != OFPTYPE_ECHO_REPLY && type != OFPTYPE_BARRIER_REPLY) {
+} else {
 if (VLOG_IS_DBG_ENABLED()) {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30, 300);
 
-- 
2.1.0

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

Re: [ovs-dev] [branch-2.7 0/4] Backport of variable length metaflow field fixes.

2017-05-17 Thread Joe Stringer 
On 15 March 2017 at 16:01, Joe Stringer  wrote:
> Commit 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs."), on
> branch-2.7 as 9554b03d6ab7, attempted to address incorrect encode and decode 
> of
> variable length metaflow fields where the OXM/NXM encoding of the variable
> length fields would incorrectly serialize the length. The patch addresses this
> by introducing a new per-bridge structure that adds additional metaflow fields
> for the variable-length fields on demand when the TLVs are configured by a
> controller.
>
> Unfortunately, in the original patch there was nothing ensuring that flows
> referring to variable length fields would retain valid field references when
> controllers reconfigure the TLVs. In practice, this could lead to a crash of
> ovs-vswitchd by configuring a TLV field, adding a flow which refers to it,
> removing the TLV field, then running some traffic that hit the configured 
> flow.
>
> This series looks to remedy the situation by reference counting the variable
> length fields and preventing a controller from reconfiguring TLV fields when
> there are active flows whose match or actions refer to the field.
>
> This series was applied to master, but given the size of the change and the
> minor changes necessary to apply to branch-2.7, I would feel more confident in
> backporting it if there was an extra round of review to ensure that nothing 
> was
> missed when this series was first applied to master.
>
> Yi-Hung Wei (4):
>   nx-match: Fix oxm decode.
>   nx-match: Use vl_mff_map to parse match field.
>   ofproto: Add ref counting for variable length mf_fields.
>   ofproto: Move tun_table and vl_mff_map deletion.

I applied the remainder of this series to branch-2.7.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch_v3] tests: Native Tunneling.

2017-05-17 Thread Joe Stringer 
On 10 May 2017 at 14:45, Darrell Ball  wrote:
> Add a test that checks that native tunneling flow
> matching is working. The test verifies that outer L2 and L3
> flow fields populated in the overlay bridge can be
> matched in the underlay bridge.
>
> Signed-off-by: William Tu 
> Signed-off-by: Joe Stringer 
> Signed-off-by: Darrell Ball 
> Co-Authored-by: Joe Stringer 
> Co-Authored-by: Darrell Ball 

Thanks for fixing this up Darrell, it's looking more comprehensive.

'Native tunneling' and 'tunnel_push_pop' from the tests perspective is
exactly the same thing. I renamed the test to "tunnel_push_pop -
underlay bridge match". It seems that William's original co-authorship
was dropped, so I added that back in as author.

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

Re: [ovs-dev] [RFC] packets: Do not initialize ct_orig_tuple.

2017-05-17 Thread Joe Stringer 
On 16 May 2017 at 21:01, Darrell Ball  wrote:
>
>
> On 5/15/17, 6:00 AM, "ovs-dev-boun...@openvswitch.org on behalf of Bodireddy, 
> Bhanuprakash"  bhanuprakash.bodire...@intel.com> wrote:
>
> >Commit daf4d3c18da4("odp: Support conntrack orig tuple key.") introduced
> >new fields in struct 'pkt_metadata'.  pkt_metadata_init() is called for 
> every
> >packet in the userspace datapath.  When testing a simple single flow 
> case with
> >DPDK, we observe a lower throughput after the above commit (it was 14.88
> >Mpps before, it is 13 Mpps after).
> >
> >This patch skips initializing ct_orig_tuple in pkt_metadata_init().
> >It should be enough to initialize ct_state, because nobody should look at
> >ct_orig_tuple unless ct_state is != 0.
> >
> >CC: Jarno Rajahalme 
> >Signed-off-by: Daniele Di Proietto 
> >---
> >I'm sending this as an RFC because I didn't check very carefully if we 
> can really
> >avoid initializing ct_orig_tuple.
> >
> >Maybe there are better solutions to this problem.
> >---
> > lib/packets.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/packets.h b/lib/packets.h index a5a483bc8..6f1791c7a 
> 100644
> >--- a/lib/packets.h
> >+++ b/lib/packets.h
> >@@ -129,7 +129,7 @@ pkt_metadata_init(struct pkt_metadata *md,
> >odp_port_t port)
> > /* It can be expensive to zero out all of the tunnel metadata. 
> However,
> >  * we can just zero out ip_dst and the rest of the data will never 
> be
> >  * looked at. */
> >-memset(md, 0, offsetof(struct pkt_metadata, in_port));
> >+memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple));
> > md->tunnel.ip_dst = 0;
> > md->tunnel.ipv6_dst = in6addr_any;
> >
>
> It's been a while this RFC patch has been submitted to fix performance 
> drop on Master. This indeed fixes the OvS-DPDK performance drop that was 
> introduced by the conntrack commit.
> Is there a better fix than what is suggested above?
>
>
> This affects both kernel and userspace.
> I tested this on both datapaths.
> LGTM

How do we make sure that ct_orig_tuple isn't used uninitialized? Do we
need to zero out the ct_orig_tuple proto?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Fix timing dependency bridge - multiple bridges share a controller

2017-05-17 Thread Andy Zhou 
On Wed, May 17, 2017 at 1:43 PM, Joe Stringer  wrote:
> On 17 May 2017 at 13:16, Andy Zhou  wrote:
>> On Wed, May 17, 2017 at 12:33 PM, Joe Stringer  wrote:
>>> On 17 May 2017 at 11:37, Andy Zhou  wrote:
 Without the fix, this test currently consistently fail when running
 on Travis CI. Connecting to the controller can take more time than
 running locally. Because the exact connecting time is variable, the
 exact output should not be used for correctness checking.

 Fixes: 85c55772a453(bridge: Fix controller status update)
 Signed-off-by: Andy Zhou 
 ---
>>>
>>> Thanks, this seems to improve the situation.
>>>
  tests/bridge.at | 23 +++
  1 file changed, 15 insertions(+), 8 deletions(-)

 diff --git a/tests/bridge.at b/tests/bridge.at
 index 58b27d445062..cc7619d3f035 100644
 --- a/tests/bridge.at
 +++ b/tests/bridge.at
 @@ -49,23 +49,30 @@ OVS_VSWITCHD_START(

  dnl Start ovs-testcontroller
  AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
 +OVS_WAIT_UNTIL([test -e controller])

  dnl Add the controller to both bridges, 5 seconds apart.
  AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
 +AT_CHECK([ovs-vsctl set-fail-mode br0 secure])
  AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
  AT_CHECK([ovs-vsctl set-controller br1 unix:controller])
 +AT_CHECK([ovs-vsctl set-fail-mode br1 secure])

 -dnl Wait for the controller connection to come up
 -for i in `seq 0 7`
 +dnl Wait for the controller connectionsi to be up
 +for i in `seq 0 19`
  do
 -AT_CHECK([ovs-appctl time/warp 10], [0], [ignore])
 +if ovs-vsctl --column=is_connected list controller |grep "false"; then
 +:
 +else
 +break
 +fi
 +ovs-appctl time/warp 1100
  done
>>>
>>> Can we substitute this for something like OVS_WAIT_WHILE([ovs-vsctl
>>> --column=is_connected list controller | grep "false"]) ?
>>
>> I have tested using OVS_WAIT_WHILE() and found some times 10 seconds
>> is not enough. The patch uses ovs-appctl time/warp instead wall clock, so it
>> should have less delay than using sleep.
>
> I see, just noticed this myself on Travis.
>
> My main concern is to get this fixed up on master, we can argue about
> how the test is layed out later.

May be we can consider having another kind of wait what only advances
ovs internal clock? At any rate, I agree that fix the master is a good
thing to do.

>
> Acked-by: Joe Stringer 
Thanks for the review, Pushed to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Fix timing dependency bridge - multiple bridges share a controller

2017-05-17 Thread Joe Stringer 
On 17 May 2017 at 11:37, Andy Zhou  wrote:
> Without the fix, this test currently consistently fail when running
> on Travis CI. Connecting to the controller can take more time than
> running locally. Because the exact connecting time is variable, the
> exact output should not be used for correctness checking.
>
> Fixes: 85c55772a453(bridge: Fix controller status update)
> Signed-off-by: Andy Zhou 
> ---

Thanks, this seems to improve the situation.

>  tests/bridge.at | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/tests/bridge.at b/tests/bridge.at
> index 58b27d445062..cc7619d3f035 100644
> --- a/tests/bridge.at
> +++ b/tests/bridge.at
> @@ -49,23 +49,30 @@ OVS_VSWITCHD_START(
>
>  dnl Start ovs-testcontroller
>  AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
> +OVS_WAIT_UNTIL([test -e controller])
>
>  dnl Add the controller to both bridges, 5 seconds apart.
>  AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
> +AT_CHECK([ovs-vsctl set-fail-mode br0 secure])
>  AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
>  AT_CHECK([ovs-vsctl set-controller br1 unix:controller])
> +AT_CHECK([ovs-vsctl set-fail-mode br1 secure])
>
> -dnl Wait for the controller connection to come up
> -for i in `seq 0 7`
> +dnl Wait for the controller connectionsi to be up
> +for i in `seq 0 19`
>  do
> -AT_CHECK([ovs-appctl time/warp 10], [0], [ignore])
> +if ovs-vsctl --column=is_connected list controller |grep "false"; then
> +:
> +else
> +break
> +fi
> +ovs-appctl time/warp 1100
>  done

Can we substitute this for something like OVS_WAIT_WHILE([ovs-vsctl
--column=is_connected list controller | grep "false"]) ?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Fix timing dependency bridge - multiple bridges share a controller

2017-05-17 Thread Joe Stringer 
On 17 May 2017 at 12:09, Greg Rose  wrote:
> On Wed, May 17, 2017 at 11:37 AM, Andy Zhou  wrote:
>> Without the fix, this test currently consistently fail when running
>> on Travis CI. Connecting to the controller can take more time than
>> running locally. Because the exact connecting time is variable, the
>> exact output should not be used for correctness checking.
>>
>> Fixes: 85c55772a453(bridge: Fix controller status update)
>> Signed-off-by: Andy Zhou 
>> ---
>>  tests/bridge.at | 23 +++
>>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> Is this the breakage in the TESTSUITE=1 builds that I'm seeing here:
>
> https://travis-ci.org/gvrose8192/ovs-experimental

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

Re: [ovs-dev] [PATCH] tests: Fix timing dependency bridge - multiple bridges share a controller

2017-05-17 Thread Greg Rose 
On Wed, May 17, 2017 at 11:37 AM, Andy Zhou  wrote:
> Without the fix, this test currently consistently fail when running
> on Travis CI. Connecting to the controller can take more time than
> running locally. Because the exact connecting time is variable, the
> exact output should not be used for correctness checking.
>
> Fixes: 85c55772a453(bridge: Fix controller status update)
> Signed-off-by: Andy Zhou 
> ---
>  tests/bridge.at | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)

Is this the breakage in the TESTSUITE=1 builds that I'm seeing here:

https://travis-ci.org/gvrose8192/ovs-experimental

- Greg

>
> diff --git a/tests/bridge.at b/tests/bridge.at
> index 58b27d445062..cc7619d3f035 100644
> --- a/tests/bridge.at
> +++ b/tests/bridge.at
> @@ -49,23 +49,30 @@ OVS_VSWITCHD_START(
>
>  dnl Start ovs-testcontroller
>  AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
> +OVS_WAIT_UNTIL([test -e controller])
>
>  dnl Add the controller to both bridges, 5 seconds apart.
>  AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
> +AT_CHECK([ovs-vsctl set-fail-mode br0 secure])
>  AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
>  AT_CHECK([ovs-vsctl set-controller br1 unix:controller])
> +AT_CHECK([ovs-vsctl set-fail-mode br1 secure])
>
> -dnl Wait for the controller connection to come up
> -for i in `seq 0 7`
> +dnl Wait for the controller connectionsi to be up
> +for i in `seq 0 19`
>  do
> -AT_CHECK([ovs-appctl time/warp 10], [0], [ignore])
> +if ovs-vsctl --column=is_connected list controller |grep "false"; then
> +:
> +else
> +break
> +fi
> +ovs-appctl time/warp 1100
>  done
>
> -dnl Make sure the connection status are different
> -AT_CHECK([ovs-vsctl --columns=status list controller | sort], [0], [dnl
> -
> -status  : {sec_since_connect="0", state=ACTIVE}
> -status  : {sec_since_connect="5", state=ACTIVE}
> +dnl Make sure the connection status have two records and they are different.
> +dnl (The exact output contains timing information that are machine 
> dependent.)
> +AT_CHECK([ovs-vsctl --column=status list controller | dnl
> +  grep "status" | sort -u |wc -l], [0], [2
>  ])
>
>  OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> --
> 1.8.3.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] datapath-windows: Report success for conntrack actions over frags

2017-05-17 Thread Anand Kumar 
Hi Alin,

Thank you for identifying and fixing issue in fragmenatation. 

Acked-by: Anand Kumar 

Regards,
Anand Kumar

On 5/17/17, 6:43 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin 
Serdean"  wrote:

When a conntrack action is applied over an IP fragment we pend the fragment
which will be consumed later. This should be transparent to the userspace.

Report that the action was applied successfully so it does not spam
the ovs-vswitchd log.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index ebfb8a3..31b4514 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2032,6 +2032,11 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT 
switchContext,
 if (status != NDIS_STATUS_PENDING) {
 OVS_LOG_ERROR("CT Action failed");
 dropReason = L"OVS-conntrack action failed";
+} else {
+/* We added a new pending NBL to be consumed later.
+ * Report to the userspace that the action applied
+ * successfully */
+status = NDIS_STATUS_SUCCESS;
 }
 goto dropit;
 } else if (oldNbl != ovsFwdCtx.curNbl) {
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us=Y5x5a2PGs19T3vg4xFJVJ7Shc9w2EwzDuDLwbuky5B8=xKibgF9Sm5mhbBW9MLFgBhslRtDQ1EQX-fyo6ILGPic=
 


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

[ovs-dev] [PATCH] tests: Fix timing dependency bridge - multiple bridges share a controller

2017-05-17 Thread Andy Zhou 
Without the fix, this test currently consistently fail when running
on Travis CI. Connecting to the controller can take more time than
running locally. Because the exact connecting time is variable, the
exact output should not be used for correctness checking.

Fixes: 85c55772a453(bridge: Fix controller status update)
Signed-off-by: Andy Zhou 
---
 tests/bridge.at | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tests/bridge.at b/tests/bridge.at
index 58b27d445062..cc7619d3f035 100644
--- a/tests/bridge.at
+++ b/tests/bridge.at
@@ -49,23 +49,30 @@ OVS_VSWITCHD_START(
 
 dnl Start ovs-testcontroller
 AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore])
+OVS_WAIT_UNTIL([test -e controller])
 
 dnl Add the controller to both bridges, 5 seconds apart.
 AT_CHECK([ovs-vsctl set-controller br0 unix:controller])
+AT_CHECK([ovs-vsctl set-fail-mode br0 secure])
 AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
 AT_CHECK([ovs-vsctl set-controller br1 unix:controller])
+AT_CHECK([ovs-vsctl set-fail-mode br1 secure])
 
-dnl Wait for the controller connection to come up
-for i in `seq 0 7`
+dnl Wait for the controller connectionsi to be up
+for i in `seq 0 19`
 do
-AT_CHECK([ovs-appctl time/warp 10], [0], [ignore])
+if ovs-vsctl --column=is_connected list controller |grep "false"; then
+:
+else
+break
+fi
+ovs-appctl time/warp 1100
 done
 
-dnl Make sure the connection status are different
-AT_CHECK([ovs-vsctl --columns=status list controller | sort], [0], [dnl
-
-status  : {sec_since_connect="0", state=ACTIVE}
-status  : {sec_since_connect="5", state=ACTIVE}
+dnl Make sure the connection status have two records and they are different.
+dnl (The exact output contains timing information that are machine dependent.)
+AT_CHECK([ovs-vsctl --column=status list controller | dnl
+  grep "status" | sort -u |wc -l], [0], [2
 ])
 
 OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH] datapath-windows: Fix alignment in actions

2017-05-17 Thread Anand Kumar 
Acked-by: Anand Kumar 
 
Thanks,
Anand Kumar

On 5/17/17, 5:57 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin 
Serdean"  wrote:

Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index e2eae9a..ebfb8a3 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2035,18 +2035,18 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT 
switchContext,
 }
 goto dropit;
 } else if (oldNbl != ovsFwdCtx.curNbl) {
-   /*
-* OvsIpv4Reassemble consumes the original NBL and creates a
-* new one and assigns it to the curNbl of ovsFwdCtx.
-*/
-   OvsInitForwardingCtx(,
-ovsFwdCtx.switchContext,
-ovsFwdCtx.curNbl,
-ovsFwdCtx.srcVportNo,
-ovsFwdCtx.sendFlags,
-
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),
-ovsFwdCtx.completionList,
-, FALSE);
+/*
+ * OvsIpv4Reassemble consumes the original NBL and creates 
a
+ * new one and assigns it to the curNbl of ovsFwdCtx.
+ */
+OvsInitForwardingCtx(,
+ ovsFwdCtx.switchContext,
+ ovsFwdCtx.curNbl,
+ ovsFwdCtx.srcVportNo,
+ ovsFwdCtx.sendFlags,
+ 
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),
+ ovsFwdCtx.completionList,
+ , FALSE);
 }
 break;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us=UsxSsO9WSAnknHPl2cBu_yKunEVS7VT_t782bL9cOmA=MuqK1pQyzFQzIfS9GbpUMBaefUMDGLoH5YmQqmzIdyw=
 






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

Re: [ovs-dev] [PATCH] datapath-windows: Fix possible null dereference in ipfragment

2017-05-17 Thread Anand Kumar 
Thanks for identifying and fixing this. 

Acked-by: Anand Kumar 

Regards,
Anand Kumar

On 5/17/17, 6:54 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin 
Serdean"  wrote:

Found using static analysis tools.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpFragment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/IpFragment.c 
b/datapath-windows/ovsext/IpFragment.c
index 675c32e..0874cb9 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -343,7 +343,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT 
switchContext,
 }
 POVS_FRAGMENT_LIST next = entry->head;
 POVS_FRAGMENT_LIST prev = entry->tail;
-if (prev != NULL || prev->offset < offset) {
+if (prev != NULL && prev->offset < offset) {
 next = NULL;
 goto found;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us=CWdGnCwQAqL4DBYA2RcH6SVyKhJMwEh0s9_AwzmByns=oNiDelVhtnHmgoVawTP5ysWACw1yvHXAtTxiQ-A6KnU=
 


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

Re: [ovs-dev] [PATCH] datapath-windows: Update the key after defragmentation

2017-05-17 Thread Anand Kumar 
Acked-by: Anand Kumar kumaran...@vmware.com

Thanks,
Anand Kumar

On 5/17/17, 6:49 AM, "ovs-dev-boun...@openvswitch.org on behalf of Alin 
Serdean"  wrote:

Update the key used by the actions which follow up defragmentation, with
no fragment set in the IP header.

Found while testing OVN with two VMs on the same host.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 31b4514..c3f0362 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2052,6 +2052,7 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
  
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),
  ovsFwdCtx.completionList,
  , FALSE);
+key->ipKey.nwFrag = OVS_FRAG_TYPE_NONE;
 }
 break;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us=oirVwU1t-01D6SSv7OmCQFD8r2FB7Ao2Nn0HV6ydu5E=9TihOd-FPgWRz7fhTe9WyqLiuREPZwz8UOz7ai2KCLQ=
 


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

Re: [ovs-dev] [PATCH v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

2017-05-17 Thread Joe Stringer 
On 17 May 2017 at 04:37, Zoltán Balogh  wrote:
> Hi Joe
>
> I started to rework my patch based on your comments and suggestions. I had 
> some
> difficulties with the last one. Let us focus on this below.
>
>> >> >  if (credit_counts) {
>> >> > +uint64_t stats_n_bytes = 0;
>> >> > +
>> >> > +if (rule->truncated_packet_size) {
>> >> > +stats_n_bytes = MIN(rule->truncated_packet_size, 
>> >> > stats->n_bytes);
>> >> > +} else {
>> >> > +stats_n_bytes = stats->n_bytes;
>> >> > +}
>> >>
>> >> Is this fixing a separate issue? I'm confused about whether truncated
>> >> packet stats are being correctly attributed today on master. If so, I
>> >> think this should split out into a separate patch. You might also
>> >> consider whether it makes more sense to modify 'stats' earlier in the
>> >> path so that each rule doesn't need to individually apply the stats
>> >> adjustment. I could imagine a store/restore of the stats plus
>> >> modifying for truncation during the xlate_output_trunc_action()
>> >> processing rather than pushing this complexity all the way into the
>> >> rule stats attribution.
>>
>> >
>> > Incorrect bytes statistics on the underlay bridge is an unwanted 
>> > side-effect of
>> > the original "Avoid recirculate" commit. By exluding recirculation, there 
>> > is no
>> > upcall for the tunnelled packets, and the packet size is not set by
>> > upcall_xlate() again:
>> >
>> >   static void
>> >   upcall_xlate(struct udpif *udpif, struct upcall *upcall,
>> >struct ofpbuf *odp_actions, struct flow_wildcards *wc)
>> >   {
>> >   struct dpif_flow_stats stats;
>> >   struct xlate_in xin;
>> >
>> >   stats.n_packets = 1;
>> >   stats.n_bytes = dp_packet_size(upcall->packet);
>> >   stats.used = time_msec();
>> >   stats.tcp_flags = ntohs(upcall->flow->tcp_flags);
>> >
>> >   xlate_in_init(, upcall->ofproto,
>> > ofproto_dpif_get_tables_version(upcall->ofproto),
>> > upcall->flow, upcall->in_port, NULL,
>> > stats.tcp_flags, upcall->packet, wc, odp_actions);
>> >
>> >   if (upcall->type == DPIF_UC_MISS) {
>> >   xin.resubmit_stats = 
>> >
>> > Here upcall_xlate() sets xin.resubmint_stats which will be used for 
>> > calculating
>> > statistic. These are const values, that's why I have not updated them 
>> > earlier.
>> >
>> > /* If nonnull, flow translation credits the specified statistics to 
>> > each
>> >  * rule reached through a resubmit or OFPP_TABLE action.
>> >  *
>> >  * This is normally null so the client has to set it manually after
>> >  * calling xlate_in_init(). */
>> > const struct dpif_flow_stats *resubmit_stats;
>> >
>> > If it does not harm, then the const modifier could be removed and stats 
>> > updated
>> > at earlier stage.
>>
>> I see, thanks for the explanation. Mostly I'm just trying to push the
>> question of "how can we make this code easier to read and
>> understand?". Const is nice because it tells you these fields won't
>> change. But I think it's probably a little easier if the truncation of
>> the statistics is performed where the truncation occurs, so that the
>> core stats attribution logic can be oblivious of this particular
>> feature.
>
> I removed the const qualifier from resubmit_stats, removed the truncation from
> rule_dpif_credit_stats__() and applied it to resubmit_stats during 
> translation.
> That works fine for a packet translated due to an upcall. At the end we get a
> sinlge datapath flow in the netdev datapath. It should look like this:
>
> netdev@ovs-netdev: hit:4 missed:10
> br-int:
> br-int 65534/1: (tap)
> br-int-ns1 1/3: (system)
> gre0 2/5: (gre: remote_ip=10.0.0.20)
> br-p:
> br-p 65534/2: (tap)
> br-p-ns2 3/4: (system)
>
> flow-dump from non-dpdk interfaces:
> recirc_id(0),in_port(3),eth_type(0x0800),ipv4(tos=0/0x3,ttl=64,frag=no), 
> packets:0, bytes:0, used:never, actions:trunc(200),clone(tnl_push(tnl_port(5)
> ,header(size=38,type=3,eth(dst=cc:cc:cc:dd:dd:de,src=02:3e:b1:dc:0e:42,dl_type=0x0800),ipv4(src=10.0.0.10,dst=10.0.0.20,proto=47,tos=0,ttl=64,frag=0x4
> 000),gre((flags=0x0,proto=0x6558))),out_port(2)),set(ipv4(tos=0/0x3,ttl=63)),4)
>
>
> At this point I get a problem, when a new packet arrives but there is no need
> for translation since we have the datapath flow in the netdev datapath.
> In this case the byte and packet statistics are calculated by the
> rule_dpif_credit_stats__() function as well.
>
> static void
> rule_dpif_credit_stats__(struct rule_dpif *rule,
>  const struct dpif_flow_stats *stats,
>  bool credit_counts)
> OVS_REQUIRES(rule->stats_mutex)
> {
> if (credit_counts) {
> rule->stats.n_packets += stats->n_packets;
> rule->stats.n_bytes

[ovs-dev] [PATCH] ovn-ctl: Fix help message for option ovn-controller-priority

2017-05-17 Thread Han Zhou 
Signed-off-by: Han Zhou 
---
 ovn/utilities/ovn-ctl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index a3bdad1..accf2ba 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -394,7 +394,7 @@ Commands:
 Options:
   --ovn-northd-priority=NICE set ovn-northd's niceness (default: 
$OVN_NORTHD_PRIORITY)
   --ovn-northd-wrapper=WRAPPER   run with a wrapper like valgrind for debugging
-  --ovn-controller-priority=NICE set ovn-northd's niceness (default: 
$OVN_CONTROLLER_PRIORITY)
+  --ovn-controller-priority=NICE set ovn-controller's niceness (default: 
$OVN_CONTROLLER_PRIORITY)
   --ovn-controller-wrapper=WRAPPER   run with a wrapper like valgrind for 
debugging
   --ovn-controller-ssl-key=KEY OVN Southbound SSL private key file
   --ovn-controller-ssl-cert=CERT OVN Southbound SSL certificate file
-- 
2.1.0

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

Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in conntrack

2017-05-17 Thread Sairam Venugopal 
Hi Yin,

Thanks for clarifying the comments. I will ack the next version.

@Alin - will you have sometime to review the changes?

Thanks,
Sairam




On 5/16/17, 5:11 PM, "Yin Lin"  wrote:

>Thanks Sai for the review! I fixed most of them and explained the remaining 
>ones in the inline comments.
>
>-Original Message-
>From: Sairam Venugopal 
>Sent: Tuesday, May 16, 2017 4:50 PM
>To: Yin Lin ; d...@openvswitch.org
>Subject: Re: [ovs-dev] [PATCH v7 2/4] datapath-windows: Add NAT module in 
>conntrack
>
>Hi Yin,
>
>Thanks for the patch. Please find my comments inline.
>
>Thanks,
>Sairam
>
>
>
>
>On 5/9/17, 3:59 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yin Lin" 
> wrote:
>
>>Signed-off-by: Yin Lin 
>>---
>> datapath-windows/automake.mk|   2 +
>> datapath-windows/ovsext/Conntrack-nat.c | 424 
>> 
>> datapath-windows/ovsext/Conntrack-nat.h |  39 +++
>> 3 files changed, 465 insertions(+)
>> create mode 100644 datapath-windows/ovsext/Conntrack-nat.c
>> create mode 100644 datapath-windows/ovsext/Conntrack-nat.h
>>
>>diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
>>index 4f7b55a..7177630 100644
>>--- a/datapath-windows/automake.mk
>>+++ b/datapath-windows/automake.mk
>>@@ -16,7 +16,9 @@ EXTRA_DIST += \
>>  datapath-windows/ovsext/Conntrack-icmp.c \
>>  datapath-windows/ovsext/Conntrack-other.c \
>>  datapath-windows/ovsext/Conntrack-related.c \
>>+datapath-windows/ovsext/Conntrack-nat.c \
>>  datapath-windows/ovsext/Conntrack-tcp.c \
>>+datapath-windows/ovsext/Conntrack-nat.h \
>>  datapath-windows/ovsext/Conntrack.c \
>>  datapath-windows/ovsext/Conntrack.h \
>>  datapath-windows/ovsext/Datapath.c \
>>diff --git a/datapath-windows/ovsext/Conntrack-nat.c 
>>b/datapath-windows/ovsext/Conntrack-nat.c
>>new file mode 100644
>>index 000..edf5f8f
>>--- /dev/null
>>+++ b/datapath-windows/ovsext/Conntrack-nat.c
>>@@ -0,0 +1,424 @@
>>+#include "Conntrack-nat.h"
>>+#include "Jhash.h"
>>+
>>+PLIST_ENTRY ovsNatTable = NULL;
>>+PLIST_ENTRY ovsUnNatTable = NULL;
>>+
>>+/*
>>+ *---
>>+ * OvsHashNatKey
>>+ * Hash NAT related fields in a Conntrack key.
>>+ *---
>>+ */
>>+static __inline UINT32
>>+OvsHashNatKey(const OVS_CT_KEY *key)
>>+{
>>+UINT32 hash = 0;
>>+#define HASH_ADD(field) \
>>+hash = OvsJhashBytes(>field, sizeof(key->field), hash)
>>+
>>+HASH_ADD(src.addr.ipv4_aligned);
>>+HASH_ADD(dst.addr.ipv4_aligned);
>>+HASH_ADD(src.port);
>>+HASH_ADD(dst.port);
>>+HASH_ADD(zone);
>>+#undef HASH_ADD
>>+return hash;
>>+}
>>+
>>+/*
>>+ *---
>>+ * OvsNatKeyAreSame
>>+ * Compare NAT related fields in a Conntrack key.
>>+ *---
>>+ */
>>+static __inline BOOLEAN
>>+OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
>>+{
>>+// XXX: Compare IPv6 key as well
>>+#define FIELD_COMPARE(field) \
>
>[Sai]: Nit: move return statement to next line. 
>[Yin]: This is a MACRO and I don't want to spread it over lines. (Note that I 
>don't even add a semicolon at the end to make it look like a function)
>
>>+if (key1->field != key2->field) return FALSE
>>+
>>+FIELD_COMPARE(src.addr.ipv4_aligned);
>>+FIELD_COMPARE(dst.addr.ipv4_aligned);
>>+FIELD_COMPARE(src.port);
>>+FIELD_COMPARE(dst.port);
>>+FIELD_COMPARE(zone);
>>+return TRUE;
>>+#undef FIELD_COMPARE
>>+}
>>+
>>+/*
>>+ *---
>
>[Sai]: Nit - OvsNatGetBucket
>[Yin]: Fixed
>
>>+ * OvsNaGetBucket
>>+ * Returns the row of NAT table that has the same hash as the given NAT
>>+ * hash key. If isReverse is TRUE, returns the row of reverse NAT table
>>+ * instead.
>>+ *---
>>+ */
>>+static __inline PLIST_ENTRY
>>+OvsNatGetBucket(const OVS_CT_KEY *key, BOOLEAN isReverse)
>>+{
>>+uint32_t hash = OvsHashNatKey(key);
>>+if (isReverse) {
>>+return [hash & NAT_HASH_TABLE_MASK];
>>+} else {
>>+return [hash & NAT_HASH_TABLE_MASK];
>>+}
>>+}
>>+
>>+/*
>>+ *---
>>+ * OvsNatInit
>>+ * Initialize NAT related resources.
>>+ *---
>>+ */
>>+NTSTATUS OvsNatInit()
>>+{
>>+ASSERT(ovsNatTable == NULL);
>>+
>>+/* Init the Hash Buffer */
>>+ovsNatTable = OvsAllocateMemoryWithTag(
>>+sizeof(LIST_ENTRY) * NAT_HASH_TABLE_SIZE,
>>+OVS_CT_POOL_TAG);
>>+if (ovsNatTable

Re: [ovs-dev] [PATCH v4 3/7] dpif-netlink: Support rtnetlink port creation.

2017-05-17 Thread Joe Stringer 
On 17 May 2017 at 09:55, Eric Garver  wrote:
> On Tue, May 16, 2017 at 06:16:03PM -0700, Joe Stringer wrote:
>> On 7 May 2017 at 06:43, Eric Garver  wrote:
>> > In order to be able to add those tunnels, we need to add code to create
>> > the tunnels and add them as NETDEV vports. And when there is no support
>> > to create them, we need to fallback to compatibility code and add them
>> > as tunnel vports.
>> >
>> > When removing those tunnels, we need to remove the interfaces as well,
>> > and detecting the right type might be important, at least to distinguish
>> > the tunnel vports that we should remove and the interfaces that we
>> > shouldn't.
>> >
>> > Co-authored-by: Thadeu Lima de Souza Cascardo 
>> > Signed-off-by: Thadeu Lima de Souza Cascardo 
>> > Signed-off-by: Eric Garver 
>> > ---
>> >  lib/automake.mk |   3 +
>> >  lib/dpif-netlink-rtnl.c | 227 
>> > 
>> >  lib/dpif-netlink-rtnl.h |  47 ++
>> >  lib/dpif-netlink.c  |  29 ++-
>> >  lib/dpif-netlink.h  |   2 +
>> >  5 files changed, 307 insertions(+), 1 deletion(-)
>> >  create mode 100644 lib/dpif-netlink-rtnl.c
>> >  create mode 100644 lib/dpif-netlink-rtnl.h
>> >
>> > diff --git a/lib/automake.mk b/lib/automake.mk
>> > index faace7993e79..f5baba27179f 100644
>> > --- a/lib/automake.mk
>> > +++ b/lib/automake.mk
>> > @@ -352,6 +352,8 @@ if LINUX
>> >  lib_libopenvswitch_la_SOURCES += \
>> > lib/dpif-netlink.c \
>> > lib/dpif-netlink.h \
>> > +   lib/dpif-netlink-rtnl.c \
>> > +   lib/dpif-netlink-rtnl.h \
>> > lib/if-notifier.c \
>> > lib/if-notifier.h \
>> > lib/netdev-linux.c \
>> > @@ -382,6 +384,7 @@ if WIN32
>> >  lib_libopenvswitch_la_SOURCES += \
>> > lib/dpif-netlink.c \
>> > lib/dpif-netlink.h \
>> > +   lib/dpif-netlink-rtnl.h \
>> > lib/netdev-windows.c \
>> > lib/netlink-conntrack.c \
>> > lib/netlink-conntrack.h \
>> > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
>> > new file mode 100644
>> > index ..906e05145190
>> > --- /dev/null
>> > +++ b/lib/dpif-netlink-rtnl.c
>> > @@ -0,0 +1,227 @@
>> > +/*
>> > + * Copyright (c) 2017 Red Hat, Inc.
>> > + *
>> > + * Licensed under the Apache License, Version 2.0 (the "License");
>> > + * you may not use this file except in compliance with the License.
>> > + * You may obtain a copy of the License at:
>> > + *
>> > + * http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
>> > implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + */
>> > +
>> > +#include 
>> > +
>> > +#include "dpif-netlink-rtnl.h"
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#include "dpif-netlink.h"
>> > +#include "netdev-vport.h"
>> > +#include "netlink-socket.h"
>> > +
>> > +static const struct nl_policy rtlink_policy[] = {
>> > +[IFLA_LINKINFO] = { .type = NL_A_NESTED },
>> > +};
>> > +static const struct nl_policy linkinfo_policy[] = {
>> > +[IFLA_INFO_KIND] = { .type = NL_A_STRING },
>> > +[IFLA_INFO_DATA] = { .type = NL_A_NESTED },
>> > +};
>> > +
>> > +static int
>> > +rtnl_transact(uint32_t type, uint32_t flags, const char *name,
>> > +  struct ofpbuf **reply)
>> > +{
>> > +struct ofpbuf request;
>> > +int err;
>> > +
>> > +ofpbuf_init(, 0);
>> > +nl_msg_put_nlmsghdr(, 0, type, flags);
>> > +ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
>> > +nl_msg_put_string(, IFLA_IFNAME, name);
>> > +
>> > +err = nl_transact(NETLINK_ROUTE, , reply);
>> > +ofpbuf_uninit();
>> > +
>> > +return err;
>> > +}
>> > +
>> > +static int
>> > +dpif_netlink_rtnl_destroy(const char *name)
>> > +{
>> > +return rtnl_transact(RTM_DELLINK, NLM_F_REQUEST | NLM_F_ACK, name, 
>> > NULL);
>> > +}
>> > +
>> > +static int OVS_UNUSED
>> > +dpif_netlink_rtnl_getlink(const char *name, struct ofpbuf **reply)
>> > +{
>> > +return rtnl_transact(RTM_GETLINK, NLM_F_REQUEST, name, reply);
>> > +}
>> > +
>> > +static int OVS_UNUSED
>> > +rtnl_policy_parse(const char *kind, struct ofpbuf *reply,
>> > +  const struct nl_policy *policy,
>> > +  struct nlattr *tnl_info[],
>> > +  size_t policy_size)
>> > +{
>> > +struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
>> > +struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
>> > +struct ifinfomsg *ifmsg;
>> > +int error = 0;
>> > +
>> > +ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg);
>> > +if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg,
>> > +

[ovs-dev] [PATCH 6/6] OF1.5/EXT-334 OXS/Flow Removal -2

2017-05-17 Thread satyavalli . rama 
From: Harivelam Lavanya 

Since FLOW_REMOVED is not supported in OF1.5 version,
testing has been done by adding test case in ofproto.at.
While doing make check after adding test case we are able to see
OFPT_FLOW_REMOVED (OF1.5) message has sent successfully.

But make check is failing because of Signal 15 termination,
so we've removed that test case from the patch.

Signed-off-by: Harivelam Lavanya  
Co-authored-by: Satya Valli  

---
 lib/ofp-util.c |  41 ++-
 lib/ox-stat.c  | 225 +
 2 files changed, 262 insertions(+), 4 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index f0c96db..a63c643 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3346,7 +3346,23 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed 
*fr,
 {
 struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length));
 enum ofpraw raw = ofpraw_pull_assert();
-if (raw == OFPRAW_OFPT11_FLOW_REMOVED) {
+if (raw == OFPRAW_OFPT15_FLOW_REMOVED) {
+const struct ofp15_flow_removed *ofr;
+enum ofperr error;
+
+ofr = ofpbuf_pull(, sizeof *ofr);
+
+error = ofputil_pull_ofp11_match(, NULL, NULL,  >match, NULL);
+if (error) {
+return error;
+}
+oxs_flow_removed_stat_pull(,fr);
+
+fr->priority = ntohs(ofr->priority);
+fr->cookie = ofr->cookie;
+fr->reason = ofr->reason;
+fr->table_id = ofr->table_id;
+} else if (raw == OFPRAW_OFPT11_FLOW_REMOVED) {
 const struct ofp12_flow_removed *ofr;
 enum ofperr error;
 
@@ -3429,12 +3445,29 @@ ofputil_encode_flow_removed(const struct 
ofputil_flow_removed *fr,
 }
 
 switch (protocol) {
+case OFPUTIL_P_OF15_OXM:
+case OFPUTIL_P_OF16_OXM: {
+struct ofp15_flow_removed *ofr;
+
+msg = ofpraw_alloc_xid(OFPRAW_OFPT15_FLOW_REMOVED,
+   ofputil_protocol_to_ofp_version(protocol),
+   htonl(0),
+   ofputil_match_typical_len(protocol));
+ofr = ofpbuf_put_zeros(msg, sizeof *ofr);
+ofr->cookie = fr->cookie;
+ofr->priority = htons(fr->priority);
+ofr->reason = reason;
+ofr->table_id = fr->table_id;
+ofputil_put_ofp11_match(msg, >match, protocol);
+/*Stats encoding in OXS TLV Format*/
+oxs_flow_removed_stat_put(msg,fr,protocol);
+break;
+}
+
 case OFPUTIL_P_OF11_STD:
 case OFPUTIL_P_OF12_OXM:
 case OFPUTIL_P_OF13_OXM:
-case OFPUTIL_P_OF14_OXM:
-case OFPUTIL_P_OF15_OXM:
-case OFPUTIL_P_OF16_OXM: {
+case OFPUTIL_P_OF14_OXM: {
 struct ofp12_flow_removed *ofr;
 
 msg = ofpraw_alloc_xid(OFPRAW_OFPT11_FLOW_REMOVED,
diff --git a/lib/ox-stat.c b/lib/ox-stat.c
index 4cfcff7..d83ba00 100644
--- a/lib/ox-stat.c
+++ b/lib/ox-stat.c
@@ -150,6 +150,11 @@ static enum ofperr oxs_pull_raw(const uint8_t *, unsigned 
int ,
 ovs_be64 * cookie, ovs_be64 * cookie_mask);
 static enum ofperr oxs_pull_agg_raw(const uint8_t * p, unsigned int stat_len,
 struct ofputil_aggregate_stats *fs);
+static int oxs_flow_rem_stat_fields_pull(const uint8_t *p,
+ unsigned int stat_len,
+ struct ofputil_flow_removed *fr);
+static int oxs_pull_flow_rem_stat_entry(struct ofpbuf *b,
+struct ofputil_flow_removed *fr);
 static void oxs_init(void);
 static void oxs_put_header__(struct ofpbuf *b, uint64_t header);
 static void oxs_put_header_len(struct ofpbuf *b,
@@ -157,6 +162,18 @@ static void oxs_put_header_len(struct ofpbuf *b,
enum ofp_version version);
 static int ox_put_agg_raw(struct ofpbuf *b, enum ofp_version oxs,
   const struct ofputil_aggregate_stats *fs);
+static void oxs_put_duration(struct ofpbuf *b,
+ const struct ofputil_flow_removed *fr,
+ enum ofp_version version);
+static void oxs_put_packet_count(struct ofpbuf *b,
+ const struct ofputil_flow_removed *fr,
+ enum ofp_version version);
+static void oxs_put_byte_count(struct ofpbuf *b,
+   const struct ofputil_flow_removed *fr,
+   enum ofp_version version);
+static int oxs_flow_rem_stat_fields_put(struct ofpbuf *b,
+const struct ofputil_flow_removed *fr,
+enum ofp_version version);
 
 static bool
 is_experimenter_oxs(uint64_t header)
@@ -765,3 +782,211 @@ oxs_put_agg_stat(struct ofpbuf *b, const struct 
ofputil_aggregate_stats *fs,
return stat_len;
 }
 
+static void
+oxs_put_duration(struct

[ovs-dev] [PATCH 5/6] OF1.5/EXT-334 OXS/Flow Removal -1

2017-05-17 Thread satyavalli . rama 
From: SatyaValli 

OXS support for FLOW REMOVED messages

Signed-off-by: Satya Valli  
Co-authored-by: Harivelam Lavanya 
---
 include/openflow/openflow-1.5.h | 11 +++
 include/openvswitch/ofp-msgs.h  |  3 +++
 lib/ox-stat.h   |  6 ++
 3 files changed, 20 insertions(+)

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 3b66f7f..f8a95dd 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -200,4 +200,15 @@ enum oxs_ofb_stat_fields {
 OFPXST_OFB_BYTE_COUNT = 4,   /* Number of bytes in flow entry.  */
 };
 
+/* Flow removed (datapath -> controller). */
+struct ofp15_flow_removed {
+ovs_be64 cookie;/* Opaque controller-issued identifier. */
+ovs_be16 priority;  /* Priority level of flow entry. */
+uint8_t reason; /* One of OFPRR_*. */
+uint8_t table_id;   /* ID of the table */
+uint8_t pad2[4];/* Align to 64-bits. */
+};
+
+OFP_ASSERT(sizeof (struct ofp15_flow_removed) == 16);
+
 #endif /* openflow/openflow-1.5.h */
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 7d49bc7..3763261 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -163,6 +163,8 @@ enum ofpraw {
 OFPRAW_OFPT10_FLOW_REMOVED,
 /* OFPT 1.1+ (11): struct ofp11_flow_removed, uint8_t[8][]. */
 OFPRAW_OFPT11_FLOW_REMOVED,
+/* OFPT 1.5+ (35): struct ofp15_flow_removed,uint8_t[8][]. */
+OFPRAW_OFPT15_FLOW_REMOVED,
 /* NXT 1.0+ (14): struct nx_flow_removed, uint8_t[8][]. */
 OFPRAW_NXT_FLOW_REMOVED,
 
@@ -561,6 +563,7 @@ enum ofptype {
   * OFPRAW_NXT_PACKET_IN. */
 OFPTYPE_FLOW_REMOVED,/* OFPRAW_OFPT10_FLOW_REMOVED.
   * OFPRAW_OFPT11_FLOW_REMOVED.
+  * OFPRAW_OFPT15_FLOW_REMOVED.
   * OFPRAW_NXT_FLOW_REMOVED. */
 OFPTYPE_PORT_STATUS, /* OFPRAW_OFPT10_PORT_STATUS.
   * OFPRAW_OFPT11_PORT_STATUS.
diff --git a/lib/ox-stat.h b/lib/ox-stat.h
index 3ff7f07..826bbe4 100644
--- a/lib/ox-stat.h
+++ b/lib/ox-stat.h
@@ -33,4 +33,10 @@ int oxs_pull_stat(struct ofpbuf *,struct ofputil_flow_stats 
*,
 int oxs_put_agg_stat(struct ofpbuf *, const struct ofputil_aggregate_stats *,
  enum ofp_version);
 int oxs_pull_agg_stat(struct ofpbuf , struct ofputil_aggregate_stats *);
+int oxs_flow_removed_stat_put(struct ofpbuf *b,
+  const struct ofputil_flow_removed *fr,
+  enum ofp_version version);
+int oxs_flow_removed_stat_pull(struct ofpbuf *b,
+   struct ofputil_flow_removed *ofr);
+
 #endif /* ox_stat.h */
-- 
1.9.1

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

[ovs-dev] [PATCH 4/6] OF1.5/EXT-334 OXS/Aggregate Flow Statistics -2

2017-05-17 Thread muttamsetty . surya 
From: Muttamsetty surya 

Signed-off-by:  Muttamsetty Surya 
Co-authored-by: Satya Valli  

---
 lib/ofp-util.c |  18 ++-
 lib/ox-stat.c  | 166 +
 lib/ox-stat.h  |   3 ++
 3 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index adb87eb..f0c96db 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3289,6 +3289,12 @@ ofputil_encode_aggregate_stats_reply(
 enum ofpraw raw;
 
 ofpraw_decode(, request);
+if (raw == OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST) {
+enum ofp_version version = request->version;
+
+msg = ofpraw_alloc_stats_reply(request, 0);
+oxs_put_agg_stat(msg, stats, version);
+} else {
 if (raw == OFPRAW_OFPST10_AGGREGATE_REQUEST) {
 packet_count = unknown_to_zero(stats->packet_count);
 byte_count = unknown_to_zero(stats->byte_count);
@@ -3302,6 +3308,7 @@ ofputil_encode_aggregate_stats_reply(
 put_32aligned_be64(>packet_count, htonll(packet_count));
 put_32aligned_be64(>byte_count, htonll(byte_count));
 asr->flow_count = htonl(stats->flow_count);
+}
 
 return msg;
 }
@@ -3311,12 +3318,21 @@ ofputil_decode_aggregate_stats_reply(struct 
ofputil_aggregate_stats *stats,
  const struct ofp_header *reply)
 {
 struct ofpbuf msg = ofpbuf_const_initializer(reply, ntohs(reply->length));
-ofpraw_pull_assert();
+enum ofperr error;
+enum ofpraw raw;
 
+raw = ofpraw_pull_assert();
+if (raw == OFPRAW_OFPST15_OXS_AGGREGATE_REPLY) {
+memset(stats, 0, sizeof (*stats));
+error = oxs_pull_agg_stat(msg, stats);
+if (error)
+return error;
+} else {
 struct ofp_aggregate_stats_reply *asr = msg.msg;
 stats->packet_count = ntohll(get_32aligned_be64(>packet_count));
 stats->byte_count = ntohll(get_32aligned_be64(>byte_count));
 stats->flow_count = ntohl(asr->flow_count);
+}
 
 return 0;
 }
diff --git a/lib/ox-stat.c b/lib/ox-stat.c
index 8bea083..4cfcff7 100644
--- a/lib/ox-stat.c
+++ b/lib/ox-stat.c
@@ -148,11 +148,15 @@ static enum ofperr oxs_pull_match_entry(struct ofpbuf *b,
 static enum ofperr oxs_pull_raw(const uint8_t *, unsigned int ,
 struct ofputil_flow_stats *fs,
 ovs_be64 * cookie, ovs_be64 * cookie_mask);
+static enum ofperr oxs_pull_agg_raw(const uint8_t * p, unsigned int stat_len,
+struct ofputil_aggregate_stats *fs);
 static void oxs_init(void);
 static void oxs_put_header__(struct ofpbuf *b, uint64_t header);
 static void oxs_put_header_len(struct ofpbuf *b,
enum oxs_ofb_stat_fields field,
enum ofp_version version);
+static int ox_put_agg_raw(struct ofpbuf *b, enum ofp_version oxs,
+  const struct ofputil_aggregate_stats *fs);
 
 static bool
 is_experimenter_oxs(uint64_t header)
@@ -400,6 +404,87 @@ oxs_pull_stat(struct ofpbuf *b, struct ofputil_flow_stats 
*fs,
 NULL);
 }
 
+static enum ofperr
+oxs_pull_agg_raw(const uint8_t * p, unsigned int stat_len,
+ struct ofputil_aggregate_stats *fs)
+{
+struct ofpbuf b = ofpbuf_const_initializer(p, stat_len);
+
+while (b.size) {
+
+uint64_t header;
+unsigned int payload_len;
+const struct oxs_field *field;
+const uint8_t *payload;
+
+oxs_pull_header__(, , );
+payload_len = oxs_payload_len(header);
+payload = ofpbuf_try_pull(, payload_len);
+
+if (fs && field) {
+switch (field->id) {
+case OFPXST_OFB_FLOW_COUNT:
+{
+uint32_t flow_count = 0;
+
+memcpy(_count, payload, sizeof (flow_count));
+fs->flow_count = ntohl(flow_count);
+}
+break;
+case OFPXST_OFB_PACKET_COUNT:
+{
+uint64_t packet_count;
+
+memcpy(_count, payload, sizeof (packet_count));
+fs->packet_count = ntohll(packet_count);
+}
+break;
+case OFPXST_OFB_BYTE_COUNT:
+{
+uint64_t byte_count;
+
+memcpy(_count, payload, sizeof (byte_count));
+fs->byte_count = ntohll(byte_count);
+}
+break;
+case OFPXST_OFB_DURATION:
+case OFPXST_OFB_IDLE_TIME:
+break;
+}
+
+}
+
+}
+return 0;
+}
+
+/* Retrieve struct ofp_oxs_stat from 'b', followed by the aggregate flow entry
+ * statistics in OXS format.
+ *
+ * Returns error if message parsing fails, otherwise returns zero . */
+int
+oxs_pull_agg_stat(struct ofpbuf b,

[ovs-dev] [PATCH 3/6] OF1.5/EXT-334 OXS/Aggregate Flow Statistics -1

2017-05-17 Thread satyavalli . rama 
From: SatyaValli 

OXS support Aggregate Multipart Statistiscs

Signed-off-by:  Satya Valli 
Co-authored-by: Muttamsetty Surya 
---
 include/openvswitch/ofp-msgs.h | 6 ++
 lib/ofp-print.c| 6 ++
 lib/ofp-util.c | 9 -
 lib/rconn.c| 2 ++
 ofproto/ofproto.c  | 4 
 5 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 089db47..7d49bc7 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -307,11 +307,15 @@ enum ofpraw {
 OFPRAW_OFPST10_AGGREGATE_REQUEST,
 /* OFPST 1.1+ (2): struct ofp11_flow_stats_request, uint8_t[8][]. */
 OFPRAW_OFPST11_AGGREGATE_REQUEST,
+/* OFPST 1.5+ (18): struct ofp15_oxs_flow_stats_request, uint8_t[8][]. */
+OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST,
 /* NXST 1.0 (1): struct nx_flow_stats_request, uint8_t[8][]. */
 OFPRAW_NXST_AGGREGATE_REQUEST,
 
 /* OFPST 1.0+ (2): struct ofp_aggregate_stats_reply. */
 OFPRAW_OFPST_AGGREGATE_REPLY,
+/* OFPST 1.5+ (18): uint8_t[] . */
+OFPRAW_OFPST15_OXS_AGGREGATE_REPLY,
 /* NXST 1.0 (1): struct ofp_aggregate_stats_reply. */
 OFPRAW_NXST_AGGREGATE_REPLY,
 
@@ -640,9 +644,11 @@ enum ofptype {
 OFPTYPE_AGGREGATE_STATS_REQUEST, /* OFPRAW_OFPST10_AGGREGATE_REQUEST.
   * OFPRAW_OFPST11_AGGREGATE_REQUEST.
   * OFPRAW_NXST_AGGREGATE_REQUEST. */
+OFPTYPE_OXS_AGGREGATE_STATS_REQUEST, /* 
OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST. */
 OFPTYPE_OXS_FLOW_STATS_REPLY,/* OFPRAW_OFPST15_OXS_FLOW_REPLY. */
 OFPTYPE_AGGREGATE_STATS_REPLY,   /* OFPRAW_OFPST_AGGREGATE_REPLY.
   * OFPRAW_NXST_AGGREGATE_REPLY. */
+OFPTYPE_OXS_AGGREGATE_STATS_REPLY, /* OFPRAW_OFPST15_OXS_AGGREGATE_REPLY. 
*/
 OFPTYPE_TABLE_STATS_REQUEST, /* OFPRAW_OFPST_TABLE_REQUEST. */
 OFPTYPE_TABLE_STATS_REPLY,   /* OFPRAW_OFPST10_TABLE_REPLY.
   * OFPRAW_OFPST11_TABLE_REPLY.
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 0f855fa..c209c19 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -3564,6 +3564,7 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw 
raw,
 break;
 
 case OFPTYPE_OXS_FLOW_STATS_REQUEST:
+case OFPTYPE_OXS_AGGREGATE_STATS_REQUEST:
 ofp_print_stats(string, oh);
 ofp_print_flow_stats_request(string, oh);
 break;
@@ -3597,6 +3598,11 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw 
raw,
 ofp_print_flow_stats_reply(string, oh);
 break;
 
+case OFPTYPE_OXS_AGGREGATE_STATS_REPLY:
+ofp_print_stats(string, oh);
+ofp_print_aggregate_stats_reply(string, oh);
+break;
+
 case OFPTYPE_QUEUE_STATS_REPLY:
 ofp_print_stats(string, oh);
 ofp_print_ofpst_queue_reply(string, oh, verbosity);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 4813d77..adb87eb 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2805,6 +2805,11 @@ ofputil_decode_flow_stats_request(struct 
ofputil_flow_stats_request *fsr,
 return ofputil_decode_ofpst15_flow_request(fsr, , false, tun_table,
vl_mff_map);
 
+case OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST:
+oxs_field_set = 0;
+return ofputil_decode_ofpst15_flow_request(fsr, , true, tun_table,
+   vl_mff_map);
+
 case OFPRAW_NXST_FLOW_REQUEST:
 return ofputil_decode_nxst_flow_request(fsr, , false, tun_table,
 vl_mff_map);
@@ -2835,7 +2840,7 @@ ofputil_encode_flow_stats_request(const struct 
ofputil_flow_stats_request *fsr,
 struct ofp15_oxs_flow_stats_request *ofsr;
 
 raw = (fsr->aggregate
-   ? OFPRAW_OFPST11_AGGREGATE_REQUEST
+   ? OFPRAW_OFPST15_OXS_AGGREGATE_REQUEST 
: OFPRAW_OFPST15_OXS_FLOW_REQUEST);
 msg = ofpraw_alloc(raw, ofputil_protocol_to_ofp_version(protocol),
ofputil_match_typical_len(protocol));
@@ -10267,6 +10272,7 @@ ofputil_is_bundlable(enum ofptype type)
 case OFPTYPE_FLOW_STATS_REQUEST:
 case OFPTYPE_OXS_FLOW_STATS_REQUEST:
 case OFPTYPE_AGGREGATE_STATS_REQUEST:
+case OFPTYPE_OXS_AGGREGATE_STATS_REQUEST:
 case OFPTYPE_TABLE_STATS_REQUEST:
 case OFPTYPE_TABLE_FEATURES_STATS_REQUEST:
 case OFPTYPE_TABLE_DESC_REQUEST:
@@ -10299,6 +10305,7 @@ ofputil_is_bundlable(enum ofptype type)
 case OFPTYPE_PORT_STATS_REPLY:
 case OFPTYPE_TABLE_STATS_REPLY:
 case OFPTYPE_AGGREGATE_STATS_REPLY:
+case OFPTYPE_OXS_AGGREGATE_STATS_REPLY:
 case OFPTYPE_PORT_DESC_STATS_REPLY:
 case OFPTYPE_ROLE_REPLY:
 case OFPTYPE_FLOW_MONITOR_PAUSED:

Re: [ovs-dev] [PATCH v4 3/7] dpif-netlink: Support rtnetlink port creation.

2017-05-17 Thread Eric Garver 
On Tue, May 16, 2017 at 06:16:03PM -0700, Joe Stringer wrote:
> On 7 May 2017 at 06:43, Eric Garver  wrote:
> > In order to be able to add those tunnels, we need to add code to create
> > the tunnels and add them as NETDEV vports. And when there is no support
> > to create them, we need to fallback to compatibility code and add them
> > as tunnel vports.
> >
> > When removing those tunnels, we need to remove the interfaces as well,
> > and detecting the right type might be important, at least to distinguish
> > the tunnel vports that we should remove and the interfaces that we
> > shouldn't.
> >
> > Co-authored-by: Thadeu Lima de Souza Cascardo 
> > Signed-off-by: Thadeu Lima de Souza Cascardo 
> > Signed-off-by: Eric Garver 
> > ---
> >  lib/automake.mk |   3 +
> >  lib/dpif-netlink-rtnl.c | 227 
> > 
> >  lib/dpif-netlink-rtnl.h |  47 ++
> >  lib/dpif-netlink.c  |  29 ++-
> >  lib/dpif-netlink.h  |   2 +
> >  5 files changed, 307 insertions(+), 1 deletion(-)
> >  create mode 100644 lib/dpif-netlink-rtnl.c
> >  create mode 100644 lib/dpif-netlink-rtnl.h
> >
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index faace7993e79..f5baba27179f 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -352,6 +352,8 @@ if LINUX
> >  lib_libopenvswitch_la_SOURCES += \
> > lib/dpif-netlink.c \
> > lib/dpif-netlink.h \
> > +   lib/dpif-netlink-rtnl.c \
> > +   lib/dpif-netlink-rtnl.h \
> > lib/if-notifier.c \
> > lib/if-notifier.h \
> > lib/netdev-linux.c \
> > @@ -382,6 +384,7 @@ if WIN32
> >  lib_libopenvswitch_la_SOURCES += \
> > lib/dpif-netlink.c \
> > lib/dpif-netlink.h \
> > +   lib/dpif-netlink-rtnl.h \
> > lib/netdev-windows.c \
> > lib/netlink-conntrack.c \
> > lib/netlink-conntrack.h \
> > diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> > new file mode 100644
> > index ..906e05145190
> > --- /dev/null
> > +++ b/lib/dpif-netlink-rtnl.c
> > @@ -0,0 +1,227 @@
> > +/*
> > + * Copyright (c) 2017 Red Hat, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + * http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#include 
> > +
> > +#include "dpif-netlink-rtnl.h"
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "dpif-netlink.h"
> > +#include "netdev-vport.h"
> > +#include "netlink-socket.h"
> > +
> > +static const struct nl_policy rtlink_policy[] = {
> > +[IFLA_LINKINFO] = { .type = NL_A_NESTED },
> > +};
> > +static const struct nl_policy linkinfo_policy[] = {
> > +[IFLA_INFO_KIND] = { .type = NL_A_STRING },
> > +[IFLA_INFO_DATA] = { .type = NL_A_NESTED },
> > +};
> > +
> > +static int
> > +rtnl_transact(uint32_t type, uint32_t flags, const char *name,
> > +  struct ofpbuf **reply)
> > +{
> > +struct ofpbuf request;
> > +int err;
> > +
> > +ofpbuf_init(, 0);
> > +nl_msg_put_nlmsghdr(, 0, type, flags);
> > +ofpbuf_put_zeros(, sizeof(struct ifinfomsg));
> > +nl_msg_put_string(, IFLA_IFNAME, name);
> > +
> > +err = nl_transact(NETLINK_ROUTE, , reply);
> > +ofpbuf_uninit();
> > +
> > +return err;
> > +}
> > +
> > +static int
> > +dpif_netlink_rtnl_destroy(const char *name)
> > +{
> > +return rtnl_transact(RTM_DELLINK, NLM_F_REQUEST | NLM_F_ACK, name, 
> > NULL);
> > +}
> > +
> > +static int OVS_UNUSED
> > +dpif_netlink_rtnl_getlink(const char *name, struct ofpbuf **reply)
> > +{
> > +return rtnl_transact(RTM_GETLINK, NLM_F_REQUEST, name, reply);
> > +}
> > +
> > +static int OVS_UNUSED
> > +rtnl_policy_parse(const char *kind, struct ofpbuf *reply,
> > +  const struct nl_policy *policy,
> > +  struct nlattr *tnl_info[],
> > +  size_t policy_size)
> > +{
> > +struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
> > +struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
> > +struct ifinfomsg *ifmsg;
> > +int error = 0;
> > +
> > +ifmsg = ofpbuf_at(reply, NLMSG_HDRLEN, sizeof *ifmsg);
> > +if (!nl_policy_parse(reply, NLMSG_HDRLEN + sizeof *ifmsg,
> > + rtlink_policy, rtlink, ARRAY_SIZE(rtlink_policy))
> > +|| !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
> > +linkinfo,

[ovs-dev] [PATCH 2/6] OF1.5/EXT-334 OXS/Individal Flow Statistics -2

2017-05-17 Thread satyavalli . rama 
From: Harivelam Lavanya 


Changes in ofproto.at and ofproto-dpif.at :-  

For 1.5 version "flags"  variable has been removed from flow_stats_reply 
structure.
For avoiding the make check failures for OF1.5 version add-flow with flags,
below test cases has been modified for 1.5 version in respective .at files

Files   testcase  
ofproto.at  0884
ofproto-dpif1126

In ofproto.at there is one test case numbered 0884 and 1126 in ofproto-dpif.at 
which is validating flagsfor OF1.5 this testcase is not valid, because
in stats reply flags are not explicitly communicated,I've removed this test 
case.
Before removing this test case is failing for 1.5 version.

Signed-off-by:  Lavanya Harivelam 
Co-authored-by: Satya Valli 

---
 lib/ofp-util.c| 125 -
 lib/ox-stat.c | 365 +-
 lib/ox-stat.h |   3 +-
 tests/ofproto-dpif.at |   1 -
 tests/ofproto.at  |   4 -
 5 files changed, 487 insertions(+), 11 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index bdf89b6..4813d77 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -50,9 +50,12 @@
 #include "unaligned.h"
 #include "util.h"
 #include "uuid.h"
+#include "ox-stat.h"
 
 VLOG_DEFINE_THIS_MODULE(ofp_util);
 
+extern uint8_t oxs_field_set;
+
 /* Rate limit for OpenFlow message parse errors.  These always indicate a bug
  * in the peer and so there's not much point in showing a lot of them. */
 static struct vlog_rate_limit bad_ofmsg_rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -2314,6 +2317,40 @@ ofputil_decode_ofpst11_flow_request(struct 
ofputil_flow_stats_request *fsr,
 }
 
 static enum ofperr
+ofputil_decode_ofpst15_flow_request(struct ofputil_flow_stats_request *fsr,
+struct ofpbuf *b, bool aggregate,
+const struct tun_table *tun_table,
+const struct vl_mff_map *vl_mff_map)
+{
+const struct ofp15_oxs_flow_stats_request *ofsr;
+enum ofperr error, stat_error;
+uint16_t statlen;
+
+ofsr = ofpbuf_pull(b, sizeof *ofsr);
+fsr->aggregate = aggregate;
+fsr->table_id = ofsr->table_id;
+
+error = ofputil_port_from_ofp11(ofsr->out_port, >out_port);
+if (error) {
+return error;
+}
+
+fsr->out_group = ntohl(ofsr->out_group);
+fsr->cookie = ofsr->cookie;
+fsr->cookie_mask = ofsr->cookie_mask;
+
+error = ofputil_pull_ofp11_match(b, tun_table, vl_mff_map, >match,
+ NULL);
+stat_error = oxs_pull_stat(b, NULL, );
+
+if (error || stat_error) {
+return error;
+}
+
+return 0;
+}
+
+static enum ofperr
 ofputil_decode_nxst_flow_request(struct ofputil_flow_stats_request *fsr,
  struct ofpbuf *b, bool aggregate,
  const struct tun_table *tun_table,
@@ -2763,6 +2800,11 @@ ofputil_decode_flow_stats_request(struct 
ofputil_flow_stats_request *fsr,
 return ofputil_decode_ofpst11_flow_request(fsr, , true, tun_table,
vl_mff_map);
 
+case OFPRAW_OFPST15_OXS_FLOW_REQUEST:
+oxs_field_set = 0;
+return ofputil_decode_ofpst15_flow_request(fsr, , false, tun_table,
+   vl_mff_map);
+
 case OFPRAW_NXST_FLOW_REQUEST:
 return ofputil_decode_nxst_flow_request(fsr, , false, tun_table,
 vl_mff_map);
@@ -2788,12 +2830,29 @@ ofputil_encode_flow_stats_request(const struct 
ofputil_flow_stats_request *fsr,
 enum ofpraw raw;
 
 switch (protocol) {
+case OFPUTIL_P_OF15_OXM:
+case OFPUTIL_P_OF16_OXM: {
+struct ofp15_oxs_flow_stats_request *ofsr;
+
+raw = (fsr->aggregate
+   ? OFPRAW_OFPST11_AGGREGATE_REQUEST
+   : OFPRAW_OFPST15_OXS_FLOW_REQUEST);
+msg = ofpraw_alloc(raw, ofputil_protocol_to_ofp_version(protocol),
+   ofputil_match_typical_len(protocol));
+ofsr = ofpbuf_put_zeros(msg, sizeof *ofsr);
+ofsr->table_id = fsr->table_id;
+ofsr->out_port = ofputil_port_to_ofp11(fsr->out_port);
+ofsr->out_group = htonl(fsr->out_group);
+ofsr->cookie = fsr->cookie;
+ofsr->cookie_mask = fsr->cookie_mask;
+ofputil_put_ofp11_match(msg, >match, protocol);
+oxs_put_stat(msg, NULL, ofputil_protocol_to_ofp_version(protocol));
+break;
+}
 case OFPUTIL_P_OF11_STD:
 case OFPUTIL_P_OF12_OXM:
 case OFPUTIL_P_OF13_OXM:
-case OFPUTIL_P_OF14_OXM:
-case OFPUTIL_P_OF15_OXM:
-case OFPUTIL_P_OF16_OXM: {
+case OFPUTIL_P_OF14_OXM: {
 struct ofp11_flow_stats_request *ofsr;
 
 raw = (fsr->aggregate
@@ -2893,6 +2952,46 @@ ofputil_decode_flow_stats_reply(struct 
ofputil_flow_stats *fs,

[ovs-dev] [PATCH 1/6] OF1.5/EXT-334 OXS/Individal Flow Statistics -1

2017-05-17 Thread satyavalli . rama 
From: SatyaValli 

OpenFlow 1.5 introduces the Extensible Statistics (OXS) by redefining the 
existing
flow entry statistics with OXS Fields.
This Patch provides implementation for OXS fields encoding in TLV format.

To support this implementation below two messages are newly added

OFPST_OXS_FLOW_STATS_REQUEST
OFPST_OXS_FLOW_STATS_REPLY
OFPST_OXS_AGGREGATE_STATS_REQUEST
OFPST_OXS_AGGREGATE_STATS_REPLY
OFPST_FLOW_REMOVED

As per the openflow specification-1.5, this enhancement should take place
on the existing flow entry statistics with the OXS fields on all the messages
that carries flow entry statistics.

The current commit adds support for the new feature in flow statistics 
multipart messages,
aggregate multipart messages and OXS flow statistics support for flow removal 
message.

Some more fields are added to ovs-ofctl dump-flows command to support 
OpenFlow15 OXS stats.
Below are Commands to display OXS stats field wise

Flow Statistics Multipart
ovs-ofctl dump-flows -O OpenFlow15  oxs-duration
ovs-ofctl dump-flows -O OpenFlow15  oxs-idle_time
ovs-ofctl dump-flows -O OpenFlow15  oxs-packet_count
ovs-ofctl dump-flows -O OpenFlow15  oxs-byte_count

Aggregate Flow Statistics Multipart
ovs-ofctl dump-aggregate -O OpenFlow15  oxs-packet_count
ovs-ofctl dump-aggregate -O OpenFlow15  oxs-byte_count
ovs-ofctl dump-aggregate -O OpenFlow15  oxs-flow_count

For feature vise verification please apply 
Patch 1 and 2 for OXS support for Flow multiplart stats support
Patch 3 and 4 for OXS support for Aggregate multipart stats support
Patch 5 and 6 for OXS support for Flow Removal   

Signed-off-by: Satya Valli 
Co-authored-by: Lavanya Harivelam 

---
 include/openflow/openflow-1.5.h |  50 +
 include/openvswitch/ofp-msgs.h  |   6 +
 lib/automake.mk |   2 +
 lib/ofp-parse.c |  45 +++-
 lib/ofp-print.c |  10 ++
 lib/ox-stat.c   | 238 
 lib/ox-stat.h   |  32 ++
 lib/rconn.c |   2 +
 ofproto/ofproto.c   |   4 +
 9 files changed, 388 insertions(+), 1 deletion(-)
 create mode 100644 lib/ox-stat.c
 create mode 100644 lib/ox-stat.h

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 3649e6c..3b66f7f 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -150,4 +150,54 @@ struct ofp15_group_desc_stats {
 };
 OFP_ASSERT(sizeof(struct ofp15_group_desc_stats) == 16);
 
+struct ofp_oxs_stat {
+ovs_be16 reserved;  /* One of OFPST_* */
+ovs_be16 length;/* Stats Length */
+};
+
+OFP_ASSERT(sizeof(struct ofp_oxs_stat) == 4);
+
+/* Body for ofp_multipart_request of type
+* OFPMP_FLOW_DESC & OFPMP_FLOW_STATS. */
+struct ofp15_oxs_flow_stats_request {
+uint8_t table_id;   /* ID of table to read (from ofp_table_desc),
+ * OFPTT_ALL for all tables. */
+uint8_t pad[3]; /* Align to 32 bits. */
+ovs_be32 out_port;  /* Require matching entries to include this as
+ * an output port. A value of OFP_ANY
+ * indicates no restriction. */
+ovs_be32 out_group; /* Require matching entries to include this as
+ * an output group. A value of OFPG_ANY
+ * indicates no restriction. */
+uint8_t pad2[4];/* Align to 64 bits. */
+ovs_be64 cookie;/* Require matching entries to contain this
+ * cookie value */
+ovs_be64 cookie_mask;   /* Mask used to restrict the cookie bits that
+ * must match. A value of 0 indicates no
+ * restriction. */
+};
+
+OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_request) == 32);
+
+/* Body of reply to OFPMP_FLOW_STATS request
+* and body for OFPIT_STAT_TRIGGER generated status. */
+struct ofp15_oxs_flow_stats_reply {
+ovs_be16 length;/* Length of this entry.  */
+uint8_t pad2[2];/* Align to 64-bits.  */
+uint8_t table_id;   /* ID of table flow came from. */
+uint8_t reason; /* One of OFPFSR_*.  */
+ovs_be16 priority;  /* Priority of the entry.  */
+};
+
+OFP_ASSERT(sizeof(struct ofp15_oxs_flow_stats_reply) == 8);
+
+/* OXS flow stat field types for OpenFlow basic class. */
+enum oxs_ofb_stat_fields {
+OFPXST_OFB_DURATION = 0, /* Time flow entry has been alive.  */
+OFPXST_OFB_IDLE_TIME = 1,/* Time flow entry has been idle.  */
+OFPXST_OFB_FLOW_COUNT = 2,   /* Number of aggregated flow entries. */
+OFPXST_OFB_PACKET_COUNT = 3, /* Number of packets in flow entry.  */
+OFPXST_OFB_BYTE_COUNT = 4,   /* Number of bytes in flow entry.  */
+};
+
 #endif /*

Re: [ovs-dev] [bug] failed to add ovs bridge

2017-05-17 Thread Ben Pfaff 
Do you still see failures when the Linux bridge is not involved at all?

On Wed, May 17, 2017 at 03:52:54AM +, fukaige wrote:
> I start a vm with its backend tap device "vnet0" attached to a Linux bridge, 
> then delete the bridge and vm.
> Next time, vm will start failed. this is as expected. But, seeing that linux 
> bridge was already delelted; 
> there is no reason that the OVS bridge with the same name can not be created. 
> What do you think?
> 
> Thanks
> fukaige
> 
> > -Original Message-
> > From: Ben Pfaff [mailto:b...@ovn.org]
> > Sent: Wednesday, May 17, 2017 11:37 AM
> > To: fukaige
> > Cc: d...@openvswitch.org
> > Subject: Re: [bug] failed to add ovs bridge
> > 
> > On Wed, May 17, 2017 at 01:16:11AM +, fukaige wrote:
> > > I am using ovs-2.5.2 and get error when creating a bridge using “ovs-vsctl
> > add-br br-eth”
> > 
> > > brctl addbr br0;ifconfig br0 up
> > 
> > [...]
> > 
> > > ifconfig br0 down; brctl delbr br0
> > >
> > > ovs-vsctl add-br br0
> > >
> > > ovs-vsctl del-br br0
> > 
> > Wait, what?  Why are you mixing brctl and ovs-vsctl?  That makes no sense.
> > Use the Linux bridge *or* OVS, not a mix of them.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Supporting ovn-northd service HA depend on OVNDB-HA

2017-05-17 Thread Ben Pfaff 
From: Zhengwei Gao 

As ovn-northd servcie  parse network element between ovnnb_db and
ovnsb_db, it ensures ovn-northd service connecting to ovnnb_db and
ovnsb_db. OVNDB-HA was supported with pacemaker, ovn-northd service
will be failover fllowing OVNDB-HA.
---
This was posted as a github pull request:
https://github.com/openvswitch/ovs/pull/176

 ovn/utilities/ovndb-servers.ocf | 4 
 1 file changed, 4 insertions(+)

diff --git a/ovn/utilities/ovndb-servers.ocf b/ovn/utilities/ovndb-servers.ocf
index 908cb3c17d84..40c55411e828 100755
--- a/ovn/utilities/ovndb-servers.ocf
+++ b/ovn/utilities/ovndb-servers.ocf
@@ -122,8 +122,12 @@ ovsdb_server_notify() {
 # the right thing at startup
 ocf_log debug "ovndb_server: $host_name is the master"
 ${CRM_ATTR_REPL_INFO} -v "$host_name"
+# Startup ovn-northd service
+${OVN_CTL} start_northd
 
 else
+# Stop ovn-northd service
+${OVN_CTL} stop_northd
 # Synchronize with the new master
 ocf_log debug "ovndb_server: Connecting to the new master 
${OCF_RESKEY_CRM_meta_notify_promote_uname}"
 ${OVN_CTL} demote_ovnnb --db-nb-sync-from-addr=${MASTER_IP} \
-- 
2.10.2

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

Re: [ovs-dev] [PATCH v2 1/3] netdev-dpdk: Fix double attaching of virtual devices.

2017-05-17 Thread Darrell Ball 


On 5/17/17, 4:16 AM, "Ilya Maximets"  wrote:

On 17.05.2017 08:53, Ilya Maximets wrote:
> Hi Darrell,
> 
> Good catch. Thanks. One comment inline.
> 
> Best regards, Ilya Maximets.
> 
> On 16.05.2017 20:14, Darrell Ball wrote:
>> I applied the following incremental to fix a missing free on error, but 
otherwise is fine 
>> Can you fold in the incremental.
>>
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 56b9d25..25d9c9b 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1128,7 +1128,7 @@ netdev_dpdk_process_devargs(const char *devargs, 
char **errp)
>>  } else {
>>  /* Attach unsuccessful */
>>  VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", 
devargs);
>> -return -1;
>> +new_port_id = UINT8_MAX;
> 
> 'rte_eth_dev_get_port_by_name' and 'rte_eth_dev_attach' set a valid
> 'new_port_id' only on success. So, we don't need to reinitialize it
> here. Just removing of return should be enough.

I just found that this change also was mistakenly moved to the third patch.
Oh.. Kind of bad luck with splitting of this patches.
Also, I think, I'll keep this reinitialization just to make code more clear.

In your previous response, you were arguing that removing the return was good
enough. Now, you are saying the re-initialization is ok since you had something 
similar in your 3rd patch – hmm, fine.


>>  }
>>  }
>>
>>
>> On 5/12/17, 8:10 AM, "Ilya Maximets"  wrote:
>>
>> 'devargs' for virtual devices contains not only name but
>> also a list of arguments like this:
>> 
>>  'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
>>  or
>>  'eth_af_packet0,iface=eth0'
>> 
>> We must cut off the arguments from this string before calling
>> 'rte_eth_dev_get_port_by_name()' to avoid double attaching of
>> the same device.
>> 
>> CC: Ciara Loftus 
>> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
(vdevs)")
>> Signed-off-by: Ilya Maximets 
>> ---
>> 
>>  lib/netdev-dpdk.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 609b8da..56b9d25 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1114,10 +1114,12 @@ netdev_dpdk_lookup_by_port_id(int port_id)
>>  static int
>>  netdev_dpdk_process_devargs(const char *devargs, char **errp)
>>  {
>> +/* Get the name up to the first comma. */
>> +char *name = xmemdup0(devargs, strcspn(devargs, ","));
>>  uint8_t new_port_id = UINT8_MAX;
>>  
>>  if (!rte_eth_dev_count()
>> -|| rte_eth_dev_get_port_by_name(devargs, _port_id)
>> +|| rte_eth_dev_get_port_by_name(name, _port_id)
>>  || !rte_eth_dev_is_valid_port(new_port_id)) {
>>  /* Device not found in DPDK, attempt to attach it */
>>  if (!rte_eth_dev_attach(devargs, _port_id)) {
>> @@ -1130,6 +1132,7 @@ netdev_dpdk_process_devargs(const char 
*devargs, char **errp)
>>  }
>>  }
>>  
>> +free(name);
>>  return new_port_id;
>>  }
>>  
>> -- 
>> 2.7.4
>> 
>> 
>>


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

Re: [ovs-dev] [PATCH 3/3] netdev-dpdk: Use uint8_t for port_id.

2017-05-17 Thread Ilya Maximets 
I guess, we need some more opinions about this.

My comments inline.

Best regards, Ilya Maximets.

On 13.05.2017 07:00, Darrell Ball wrote:
> 
> 
> On 5/12/17, 8:04 AM, "Ilya Maximets"  wrote:
> 
> On 05.04.2017 22:34, Darrell Ball wrote:
> > 
> > 
> > On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ilya 
> Maximets"  i.maxim...@samsung.com> wrote:
> > 
> > Currently, signed integer is used for 'port_id' variable and
> > '-1' as identifier of bad or uninitialized 'port_id'.
> > 
> > This inconsistent with dpdk library and, also, in few cases,
> > leads to passing '-1' to dpdk functions where uint8_t expected.
> > 
> > Such behaviour doesn't produce any issues, but it's better to
> > use same type as in dpdk library for consistency.
> > 
> > Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
> > macro.
> > 
> > Signed-off-by: Ilya Maximets 
> > ---
> >  lib/netdev-dpdk.c | 61 
> +++
> >  1 file changed, 35 insertions(+), 26 deletions(-)
> > 
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 658a454..216ced8 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
> >  #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by 
> guest and not
> >* yet mapped to another 
> queue. */
> >  
> > +#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
> > +
> >  #define VHOST_ENQ_RETRY_NUM 8
> >  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
> >  
> > @@ -309,7 +311,7 @@ struct dpdk_ring {
> >  struct rte_ring *cring_tx;
> >  struct rte_ring *cring_rx;
> >  unsigned int user_port_id; /* User given port no, parsed from 
> port name */
> > -int eth_port_id; /* ethernet device port id */
> > +uint8_t eth_port_id; /* ethernet device port id */
> > 
> > The rte does not have a typedef for port_id.
> > One optional change is for OVS to have a typedef for this external type.
> > /* The dpdk rte uses uint8_t for port_id. */
> > typedef uint8_t rte_port_id;
> 
> I don't think that it is a good change to do because we will have to
> create additional typedef at least for PRIu8. This will look ugly.
> 
> No, see my following diff
> 
> Also, if DPDK someday will create typedef with different name
> we will have typedef of the typedef?
> 
> I don’t follow this comment; see the diff below.
> 
> The reasons for the typedef here are:
> 1) Easier to maintain if the size of type changes in future

In case of future type change we will have to change all the "PRIu8"
format strings to something else. This is the half of all the changes.
So, maintainability is in question.

> 2) Serves as documentation that the origin of the type is the rte library and
> and originates externally to ovs

IMHO, using 'rte_' prefix for something defined not inside DPDK is a
bad style and may be misleading. We should remember that this type
defined locally in OVS.

> 
> Here is a few examples of typedefs in the OVS code base:
> 
> dpif-netdev.c:5587:typedef uint32_t map_type;
> stp.h:41:typedef uint64_t stp_identifier;
> timeval.c:45:typedef unsigned int clockid_t;
> 
> 
> dball@ubuntu:~/ovs$ git diff
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 609b8da..6667274 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -143,6 +143,9 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
>  #define VHOST_ENQ_RETRY_NUM 8
>  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>  
> +#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
> +typedef uint8_t rte_port_id;
> +
>  static const struct rte_eth_conf port_conf = {
>  .rxmode = {
>  .mq_mode = ETH_MQ_RX_RSS,
> @@ -309,7 +312,7 @@ struct dpdk_ring {
>  struct rte_ring *cring_tx;
>  struct rte_ring *cring_rx;
>  unsigned int user_port_id; /* User given port no, parsed from port name 
> */
> -int eth_port_id; /* ethernet device port id */
> +rte_port_id eth_port_id; /* ethernet device port id */
>  struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
>  };
>  
> @@ -325,7 +328,7 @@ enum dpdk_hw_ol_features {
>  
>  struct netdev_dpdk {
>  struct netdev up;
> -int port_id;
> +rte_port_id port_id;
>  int max_packet_len;
>  enum dpdk_dev_type type;
>  
> @@ -399,7 +402,7 @@ struct netdev_dpdk {
>  
>  struct netdev_rxq_dpdk {
>  struct netdev_rxq up;
> -int

[ovs-dev] [PATCH v3 3/3] netdev-dpdk: Use uint8_t for port_id.

2017-05-17 Thread Ilya Maximets 
Currently, signed integer is used for 'port_id' variable and
'-1' as identifier of bad or uninitialized 'port_id'.

This inconsistent with dpdk library and, also, in few cases,
leads to passing '-1' to dpdk functions where uint8_t expected.

Such behaviour doesn't produce any issues, but it's better to
use same type as in dpdk library for consistency.

Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID
macro.

Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 59 +++
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ddd3d50..1ed25b2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -140,6 +140,8 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / 
ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
 #define OVS_VHOST_QUEUE_DISABLED(-2) /* Queue was disabled by guest and not
   * yet mapped to another queue. */
 
+#define DPDK_ETH_PORT_ID_INVALIDRTE_MAX_ETHPORTS
+
 #define VHOST_ENQ_RETRY_NUM 8
 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
 
@@ -309,7 +311,7 @@ struct dpdk_ring {
 struct rte_ring *cring_tx;
 struct rte_ring *cring_rx;
 unsigned int user_port_id; /* User given port no, parsed from port name */
-int eth_port_id; /* ethernet device port id */
+uint8_t eth_port_id; /* ethernet device port id */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
 
@@ -325,7 +327,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 struct netdev up;
-int port_id;
+uint8_t port_id;
 int max_packet_len;
 enum dpdk_dev_type type;
 
@@ -402,7 +404,7 @@ struct netdev_dpdk {
 
 struct netdev_rxq_dpdk {
 struct netdev_rxq up;
-int port_id;
+uint8_t port_id;
 };
 
 static int netdev_dpdk_class_init(void);
@@ -603,12 +605,12 @@ check_link_status(struct netdev_dpdk *dev)
 dev->link_reset_cnt++;
 dev->link = link;
 if (dev->link.link_status) {
-VLOG_DBG_RL(, "Port %d Link Up - speed %u Mbps - %s",
+VLOG_DBG_RL(, "Port %"PRIu8" Link Up - speed %u Mbps - %s",
 dev->port_id, (unsigned) dev->link.link_speed,
 (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
  ("full-duplex") : ("half-duplex"));
 } else {
-VLOG_DBG_RL(, "Port %d Link Down", dev->port_id);
+VLOG_DBG_RL(, "Port %"PRIu8" Link Down", dev->port_id);
 }
 }
 }
@@ -726,8 +728,8 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
 if (rx_csum_ol_flag &&
 (info.rx_offload_capa & rx_chksm_offload_capa) !=
  rx_chksm_offload_capa) {
-VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
-   dev->port_id);
+VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8,
+   dev->port_id);
 dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
 return;
 }
@@ -738,7 +740,8 @@ static void
 dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex)
 {
 if (rte_eth_dev_flow_ctrl_set(dev->port_id, >fc_conf)) {
-VLOG_WARN("Failed to enable flow control on device %d", dev->port_id);
+VLOG_WARN("Failed to enable flow control on device %"PRIu8,
+  dev->port_id);
 }
 }
 
@@ -776,7 +779,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 memset(_addr, 0x0, sizeof(eth_addr));
 rte_eth_macaddr_get(dev->port_id, _addr);
-VLOG_INFO_RL(, "Port %d: "ETH_ADDR_FMT,
+VLOG_INFO_RL(, "Port %"PRIu8": "ETH_ADDR_FMT,
 dev->port_id, ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes));
 
 memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN);
@@ -788,7 +791,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 /* Get the Flow control configuration for DPDK-ETH */
 diag = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
 if (diag) {
-VLOG_DBG("cannot get flow control parameters on port=%d, err=%d",
+VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", err=%d",
  dev->port_id, diag);
 }
 
@@ -833,7 +836,7 @@ netdev_dpdk_alloc_txq(unsigned int n_txqs)
 }
 
 static int
-common_construct(struct netdev *netdev, unsigned int port_no,
+common_construct(struct netdev *netdev, uint8_t port_no,
  enum dpdk_dev_type type, int socket_id)
 OVS_REQUIRES(dpdk_mutex)
 {
@@ -918,7 +921,8 @@ vhost_common_construct(struct netdev *netdev)
 return ENOMEM;
 }
 
-return common_construct(netdev, -1, DPDK_DEV_VHOST, socket_id);
+return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
+DPDK_DEV_VHOST, socket_id);
 }
 
 static int
@@ -978,7 +982,8 @@ netdev_dpdk_construct(struct netdev *netdev)
 int err;
 
 ovs_mutex_lock(_mutex);
-err = common_construct(netdev,

[ovs-dev] [PATCH v3 2/3] netdev-dpdk: Fix device leak on port deletion.

2017-05-17 Thread Ilya Maximets 
Currently, once created device in dpdk will exist forever
even after del-port operation untill we manually call
'ovs-appctl netdev-dpdk/detach ', where  is not
the port's name but the name of dpdk eth device or pci address.

Few issues with current implementation:

1. Different API for usual (system) and DPDK devices.
   (We have to call 'ovs-appctl netdev-dpdk/detach' each
time after 'del-port' to actually free the device)
   This is a big issue mostly for virtual DPDK devices.

2. Follows from 1:
   For DPDK devices 'del-port' leads just to
   'rte_eth_dev_stop' and subsequent 'add-port' will
   just start the already existing device. Such behaviour
   will not reset the device to initial state as it could
   be expected. For example: virtual pcap pmd will continue
   reading input file instead of reading it from the beginning.

3. Follows from 2:
   After execution of the following commands 'port1' will be
   configured with the 'old-options' while 'ovs-vsctl show'
   will show us 'new-options' in dpdk-devargs field:

 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,
 ovs-vsctl del-port port1
 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,

4. Follows from 1:
   Not detached device consumes 'port_id'. Since we have very
   limited number of 'port_id's (32 in common case) this may
   lead to quick exhausting of id pool and inability to add any
   other port.

To avoid above issues we need to detach all the attached devices on
port destruction.
appctl 'netdev-dpdk/detach' removed because not needed anymore.

We need to use internal 'attached' variable to track ports on
which rte_eth_dev_attach() was called and returned successfully
to avoid closing and detaching devices that do not support hotplug or
by any other reason attached using the 'dpdk-extra' cmdline options.

CC: Ciara Loftus 
Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming")
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---
 Documentation/howto/dpdk.rst |  5 ++-
 lib/netdev-dpdk.c| 72 
 2 files changed, 22 insertions(+), 55 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index dc63f7d..20d8975 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -342,10 +342,9 @@ Then it can be attached to OVS::
 $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx type=dpdk \
 options:dpdk-devargs=:01:00.0
 
-It is also possible to detach a port from ovs, the user has to remove the
-port using the del-port command, then it can be detached using::
+Detaching will be performed while processing del-port command::
 
-$ ovs-appctl netdev-dpdk/detach :01:00.0
+$ ovs-vsctl del-port dpdkx
 
 This feature is not supported with VFIO and does not work with some NICs.
 For more information please refer to the `DPDK Port Hotplug Framework
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 25d9c9b..ddd3d50 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -359,6 +359,9 @@ struct netdev_dpdk {
 /* Device arguments for dpdk ports */
 char *devargs;
 
+/* If true, device was attached by rte_eth_dev_attach(). */
+bool attached;
+
 /* In dpdk_list. */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 
@@ -852,6 +855,7 @@ common_construct(struct netdev *netdev, unsigned int 
port_no,
 dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
 ovsrcu_index_init(>vid, -1);
 dev->vhost_reconfigured = false;
+dev->attached = false;
 
 ovsrcu_init(>qos_conf, NULL);
 
@@ -997,10 +1001,21 @@ static void
 netdev_dpdk_destruct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+char devname[RTE_ETH_NAME_MAX_LEN];
 
 ovs_mutex_lock(_mutex);
 
 rte_eth_dev_stop(dev->port_id);
+
+if (dev->attached) {
+rte_eth_dev_close(dev->port_id);
+if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
+VLOG_ERR("Device '%s' can not be detached", dev->devargs);
+} else {
+VLOG_INFO("Device '%s' detached", devname);
+}
+}
+
 free(dev->devargs);
 common_destruct(dev);
 
@@ -1112,7 +1127,8 @@ netdev_dpdk_lookup_by_port_id(int port_id)
 }
 
 static int
-netdev_dpdk_process_devargs(const char *devargs, char **errp)
+netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
+const char *devargs, char **errp)
 {
 /* Get the name up to the first comma. */
 char *name = xmemdup0(devargs, strcspn(devargs, ","));
@@ -1124,6 +1140,7 @@

[ovs-dev] [PATCH v3 1/3] netdev-dpdk: Fix double attaching of virtual devices.

2017-05-17 Thread Ilya Maximets 
'devargs' for virtual devices contains not only name but
also a list of arguments like this:

'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
or
'eth_af_packet0,iface=eth0'

We must cut off the arguments from this string before calling
'rte_eth_dev_get_port_by_name()' to avoid double attaching of
the same device.

CC: Ciara Loftus 
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets 
---
 lib/netdev-dpdk.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 609b8da..25d9c9b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1114,10 +1114,12 @@ netdev_dpdk_lookup_by_port_id(int port_id)
 static int
 netdev_dpdk_process_devargs(const char *devargs, char **errp)
 {
+/* Get the name up to the first comma. */
+char *name = xmemdup0(devargs, strcspn(devargs, ","));
 uint8_t new_port_id = UINT8_MAX;
 
 if (!rte_eth_dev_count()
-|| rte_eth_dev_get_port_by_name(devargs, _port_id)
+|| rte_eth_dev_get_port_by_name(name, _port_id)
 || !rte_eth_dev_is_valid_port(new_port_id)) {
 /* Device not found in DPDK, attempt to attach it */
 if (!rte_eth_dev_attach(devargs, _port_id)) {
@@ -1126,10 +1128,11 @@ netdev_dpdk_process_devargs(const char *devargs, char 
**errp)
 } else {
 /* Attach unsuccessful */
 VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", 
devargs);
-return -1;
+new_port_id = UINT8_MAX;
 }
 }
 
+free(name);
 return new_port_id;
 }
 
-- 
2.7.4

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

[ovs-dev] [PATCH v3 0/3] Hotplug fixes & port_id refactoring

2017-05-17 Thread Ilya Maximets 
Version 3:
* 1st patch: Fixed memory leak on error path. (Darrell Ball)
 (bad initial patch split)
* Discussion about patch #3 goes in replies to v1.
  Re-sent for now (rebased on modified patch #1).

Version 2:
* 1st patch: 'index' --> 'strcspn' (Ben Pfaff)
* One change was moved from patch 3 to patch 2.
  They were splitted in a wrong way.
  (passing 'dev' to 'netdev_dpdk_process_devargs()')


Ilya Maximets (3):
  netdev-dpdk: Fix double attaching of virtual devices.
  netdev-dpdk: Fix device leak on port deletion.
  netdev-dpdk: Use uint8_t for port_id.

 Documentation/howto/dpdk.rst |   5 +-
 lib/netdev-dpdk.c| 132 ++-
 2 files changed, 57 insertions(+), 80 deletions(-)

-- 
2.7.4

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

[ovs-dev] [PATCH] datapath-windows: Fix possible null dereference in ipfragment

2017-05-17 Thread Alin Serdean 
Found using static analysis tools.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/IpFragment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/IpFragment.c 
b/datapath-windows/ovsext/IpFragment.c
index 675c32e..0874cb9 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -343,7 +343,7 @@ OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
 }
 POVS_FRAGMENT_LIST next = entry->head;
 POVS_FRAGMENT_LIST prev = entry->tail;
-if (prev != NULL || prev->offset < offset) {
+if (prev != NULL && prev->offset < offset) {
 next = NULL;
 goto found;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] datapath-windows: Update the key after defragmentation

2017-05-17 Thread Alin Serdean 
Update the key used by the actions which follow up defragmentation, with
no fragment set in the IP header.

Found while testing OVN with two VMs on the same host.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 31b4514..c3f0362 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2052,6 +2052,7 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
  
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),
  ovsFwdCtx.completionList,
  , FALSE);
+key->ipKey.nwFrag = OVS_FRAG_TYPE_NONE;
 }
 break;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] datapath-windows: Report success for conntrack actions over frags

2017-05-17 Thread Alin Serdean 
When a conntrack action is applied over an IP fragment we pend the fragment
which will be consumed later. This should be transparent to the userspace.

Report that the action was applied successfully so it does not spam
the ovs-vswitchd log.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index ebfb8a3..31b4514 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2032,6 +2032,11 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 if (status != NDIS_STATUS_PENDING) {
 OVS_LOG_ERROR("CT Action failed");
 dropReason = L"OVS-conntrack action failed";
+} else {
+/* We added a new pending NBL to be consumed later.
+ * Report to the userspace that the action applied
+ * successfully */
+status = NDIS_STATUS_SUCCESS;
 }
 goto dropit;
 } else if (oldNbl != ovsFwdCtx.curNbl) {
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] datapath-windows: Fix alignment in actions

2017-05-17 Thread Alin Serdean 
Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index e2eae9a..ebfb8a3 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2035,18 +2035,18 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
 }
 goto dropit;
 } else if (oldNbl != ovsFwdCtx.curNbl) {
-   /*
-* OvsIpv4Reassemble consumes the original NBL and creates a
-* new one and assigns it to the curNbl of ovsFwdCtx.
-*/
-   OvsInitForwardingCtx(,
-ovsFwdCtx.switchContext,
-ovsFwdCtx.curNbl,
-ovsFwdCtx.srcVportNo,
-ovsFwdCtx.sendFlags,
-
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),
-ovsFwdCtx.completionList,
-, FALSE);
+/*
+ * OvsIpv4Reassemble consumes the original NBL and creates a
+ * new one and assigns it to the curNbl of ovsFwdCtx.
+ */
+OvsInitForwardingCtx(,
+ ovsFwdCtx.switchContext,
+ ovsFwdCtx.curNbl,
+ ovsFwdCtx.srcVportNo,
+ ovsFwdCtx.sendFlags,
+ 
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),
+ ovsFwdCtx.completionList,
+ , FALSE);
 }
 break;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] datapath-windows: Set Version correctly for OVSExt

2017-05-17 Thread Alin Serdean 
Just one small nit if you want to fix it (but not necessarily needed): no 
newline at the end of file.
This is because editing the file via the GUI strips down eol.

Acked-by: Alin Gabriel Serdean 

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Shashank Ram
> Sent: Tuesday, May 16, 2017 10:57 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v2] datapath-windows: Set Version correctly for
> OVSExt
> 
> - Previously, the 'Version' property passed to MSBuild
>   was not being passed to the RcComplile section. To
>   use the value of 'Version' property in the rc file,
>   it needs to be passed.
> 
> - Adds a macro to convert the Version to a string literal.
>   Previously, the Version was simply being converted
>   to a literal text 'Version' instead of the the version
>   number passed using the 'Version' property to MSBuild.
> 
> Signed-off-by: Shashank Ram 
> ---
> -
> \ No newline at end of file
> +
> --
> 2.6.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 v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

2017-05-17 Thread Zoltán Balogh 
Hi Joe

I started to rework my patch based on your comments and suggestions. I had some 
difficulties with the last one. Let us focus on this below.

> >> >  if (credit_counts) {
> >> > +uint64_t stats_n_bytes = 0;
> >> > +
> >> > +if (rule->truncated_packet_size) {
> >> > +stats_n_bytes = MIN(rule->truncated_packet_size, 
> >> > stats->n_bytes);
> >> > +} else {
> >> > +stats_n_bytes = stats->n_bytes;
> >> > +}
> >>
> >> Is this fixing a separate issue? I'm confused about whether truncated
> >> packet stats are being correctly attributed today on master. If so, I
> >> think this should split out into a separate patch. You might also
> >> consider whether it makes more sense to modify 'stats' earlier in the
> >> path so that each rule doesn't need to individually apply the stats
> >> adjustment. I could imagine a store/restore of the stats plus
> >> modifying for truncation during the xlate_output_trunc_action()
> >> processing rather than pushing this complexity all the way into the
> >> rule stats attribution.
>
> >
> > Incorrect bytes statistics on the underlay bridge is an unwanted 
> > side-effect of
> > the original "Avoid recirculate" commit. By exluding recirculation, there 
> > is no
> > upcall for the tunnelled packets, and the packet size is not set by
> > upcall_xlate() again:
> >
> >   static void
> >   upcall_xlate(struct udpif *udpif, struct upcall *upcall,
> >struct ofpbuf *odp_actions, struct flow_wildcards *wc)
> >   {
> >   struct dpif_flow_stats stats;
> >   struct xlate_in xin;
> >
> >   stats.n_packets = 1;
> >   stats.n_bytes = dp_packet_size(upcall->packet);
> >   stats.used = time_msec();
> >   stats.tcp_flags = ntohs(upcall->flow->tcp_flags);
> >
> >   xlate_in_init(, upcall->ofproto,
> > ofproto_dpif_get_tables_version(upcall->ofproto),
> > upcall->flow, upcall->in_port, NULL,
> > stats.tcp_flags, upcall->packet, wc, odp_actions);
> >
> >   if (upcall->type == DPIF_UC_MISS) {
> >   xin.resubmit_stats = 
> >
> > Here upcall_xlate() sets xin.resubmint_stats which will be used for 
> > calculating
> > statistic. These are const values, that's why I have not updated them 
> > earlier.
> >
> > /* If nonnull, flow translation credits the specified statistics to each
> >  * rule reached through a resubmit or OFPP_TABLE action.
> >  *
> >  * This is normally null so the client has to set it manually after
> >  * calling xlate_in_init(). */
> > const struct dpif_flow_stats *resubmit_stats;
> >
> > If it does not harm, then the const modifier could be removed and stats 
> > updated
> > at earlier stage.
> 
> I see, thanks for the explanation. Mostly I'm just trying to push the
> question of "how can we make this code easier to read and
> understand?". Const is nice because it tells you these fields won't
> change. But I think it's probably a little easier if the truncation of
> the statistics is performed where the truncation occurs, so that the
> core stats attribution logic can be oblivious of this particular
> feature.

I removed the const qualifier from resubmit_stats, removed the truncation from 
rule_dpif_credit_stats__() and applied it to resubmit_stats during translation.
That works fine for a packet translated due to an upcall. At the end we get a 
sinlge datapath flow in the netdev datapath. It should look like this:

netdev@ovs-netdev: hit:4 missed:10
br-int:
br-int 65534/1: (tap)
br-int-ns1 1/3: (system)
gre0 2/5: (gre: remote_ip=10.0.0.20)
br-p:
br-p 65534/2: (tap)
br-p-ns2 3/4: (system)

flow-dump from non-dpdk interfaces:
recirc_id(0),in_port(3),eth_type(0x0800),ipv4(tos=0/0x3,ttl=64,frag=no), 
packets:0, bytes:0, used:never, actions:trunc(200),clone(tnl_push(tnl_port(5)
,header(size=38,type=3,eth(dst=cc:cc:cc:dd:dd:de,src=02:3e:b1:dc:0e:42,dl_type=0x0800),ipv4(src=10.0.0.10,dst=10.0.0.20,proto=47,tos=0,ttl=64,frag=0x4
000),gre((flags=0x0,proto=0x6558))),out_port(2)),set(ipv4(tos=0/0x3,ttl=63)),4)


At this point I get a problem, when a new packet arrives but there is no need 
for translation since we have the datapath flow in the netdev datapath. 
In this case the byte and packet statistics are calculated by the 
rule_dpif_credit_stats__() function as well. 

static void
rule_dpif_credit_stats__(struct rule_dpif *rule,
 const struct dpif_flow_stats *stats,
 bool credit_counts)
OVS_REQUIRES(rule->stats_mutex)
{
if (credit_counts) {
rule->stats.n_packets += stats->n_packets;
rule->stats.n_bytes += stats->n_bytes;
}
rule->stats.used = MAX(rule->stats.used, stats->used);
}

However, in this case the 'stats' argument does not come from resubmit_stats. 
It is derived from dpif_flow_stats and udpif_flow stats

Re: [ovs-dev] [PATCH v2 1/3] netdev-dpdk: Fix double attaching of virtual devices.

2017-05-17 Thread Ilya Maximets 
On 17.05.2017 08:53, Ilya Maximets wrote:
> Hi Darrell,
> 
> Good catch. Thanks. One comment inline.
> 
> Best regards, Ilya Maximets.
> 
> On 16.05.2017 20:14, Darrell Ball wrote:
>> I applied the following incremental to fix a missing free on error, but 
>> otherwise is fine 
>> Can you fold in the incremental.
>>
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 56b9d25..25d9c9b 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1128,7 +1128,7 @@ netdev_dpdk_process_devargs(const char *devargs, char 
>> **errp)
>>  } else {
>>  /* Attach unsuccessful */
>>  VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", 
>> devargs);
>> -return -1;
>> +new_port_id = UINT8_MAX;
> 
> 'rte_eth_dev_get_port_by_name' and 'rte_eth_dev_attach' set a valid
> 'new_port_id' only on success. So, we don't need to reinitialize it
> here. Just removing of return should be enough.

I just found that this change also was mistakenly moved to the third patch.
Oh.. Kind of bad luck with splitting of this patches.
Also, I think, I'll keep this reinitialization just to make code more clear.

>>  }
>>  }
>>
>>
>> On 5/12/17, 8:10 AM, "Ilya Maximets"  wrote:
>>
>> 'devargs' for virtual devices contains not only name but
>> also a list of arguments like this:
>> 
>>  'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap'
>>  or
>>  'eth_af_packet0,iface=eth0'
>> 
>> We must cut off the arguments from this string before calling
>> 'rte_eth_dev_get_port_by_name()' to avoid double attaching of
>> the same device.
>> 
>> CC: Ciara Loftus 
>> Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
>> (vdevs)")
>> Signed-off-by: Ilya Maximets 
>> ---
>> 
>>  lib/netdev-dpdk.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 609b8da..56b9d25 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -1114,10 +1114,12 @@ netdev_dpdk_lookup_by_port_id(int port_id)
>>  static int
>>  netdev_dpdk_process_devargs(const char *devargs, char **errp)
>>  {
>> +/* Get the name up to the first comma. */
>> +char *name = xmemdup0(devargs, strcspn(devargs, ","));
>>  uint8_t new_port_id = UINT8_MAX;
>>  
>>  if (!rte_eth_dev_count()
>> -|| rte_eth_dev_get_port_by_name(devargs, _port_id)
>> +|| rte_eth_dev_get_port_by_name(name, _port_id)
>>  || !rte_eth_dev_is_valid_port(new_port_id)) {
>>  /* Device not found in DPDK, attempt to attach it */
>>  if (!rte_eth_dev_attach(devargs, _port_id)) {
>> @@ -1130,6 +1132,7 @@ netdev_dpdk_process_devargs(const char *devargs, 
>> char **errp)
>>  }
>>  }
>>  
>> +free(name);
>>  return new_port_id;
>>  }
>>  
>> -- 
>> 2.7.4
>> 
>> 
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Out-of-order packets when using userplance ovs with dpdkr interface

2017-05-17 Thread Junguk Cho 
Hi,

I have a simple setup in one machine and use userplane OVS-2.7 with
DPDK-16.11.1.

27afd0dc-cdac-4bb4-947c-65e13df66e99
Bridge "br0"
Controller "tcp:10.1.2.2"
is_connected: true
fail_mode: secure
Port "dpdkr2"
Interface "dpdkr2"
type: dpdkr
Port "dpdkr1"
Interface "dpdkr1"
type: dpdkr
Port "br0"
Interface "br0"
type: internal

Traffic is from dpdkr2 to dpdkr1.
I have two questions.

1. I observed one interesting behavior.
After running them for a while, sometimes,  dpdkr1 got out-of-order packets.
Is it a possible behavior even though I use two dpdkr in a same machine?

2. There are some hardcoded values for ring size and max & min num of
packets for mempool in netdev-dpdk.c.
enum { DPDK_RING_SIZE = 256 };
#define MAX_NB_MBUF  (4096 * 64)
#define MIN_NB_MBUF  (4096 * 4)

What are the intuition of these values?

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

Re: [ovs-dev] [PATCH] dpif-netdev: Fix insertion probability

2017-05-17 Thread Kevin Traynor 
On 05/17/2017 09:28 AM, Ciara Loftus wrote:
> emc_conditional_insert uses pmd->last_cycles and the packet's RSS hash
> to generate a random number used to determine whether or not an emc
> entry should be inserted. This works for single-packet bursts as
> last_cycles is updated for each burst. However, for bursts > 1 packet,
> where the packets in the batch generate the same RSS hash,
> pmd->last_cycles remains constant for the entire burst also, and thus
> cannot be used as a random number for each packet in the burst.
> 
> This commit replaces the use of pmd->last_cycles with random_uint32()
> for this purpose and subsequently fixes the behavior of the
> emc_insert_inv_prob setting for high-throughput (large bursts)
> single-flow cases.
> 
> Fixes: 4c30b24602c3 ("dpif-netdev: Conditional EMC insert")
> Reported-by: Kevin Traynor 
> Signed-off-by: Ciara Loftus 
> ---
>  lib/dpif-netdev.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index d215156..ab1e26e 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2037,11 +2037,7 @@ emc_probabilistic_insert(struct dp_netdev_pmd_thread 
> *pmd,
>  uint32_t min;
>  atomic_read_relaxed(>dp->emc_insert_min, );
>  
> -#ifdef DPDK_NETDEV
> -if (min && (key->hash ^ (uint32_t) pmd->last_cycles) <= min) {
> -#else
>  if (min && (key->hash ^ random_uint32()) <= min) {
> -#endif
>  emc_insert(>flow_cache, key, flow);
>  }
>  }
> 

Thanks for the fix.

Tested-by: Kevin Traynor 
Acked-by: Kevin Traynor 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure.

2017-05-17 Thread Chandran, Sugesh 


Regards
_Sugesh

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Wednesday, May 17, 2017 10:10 AM
> To: Chandran, Sugesh 
> Cc: d...@openvswitch.org
> Subject: Re: [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure.
> 
> On 05/16/2017 05:48 PM, Chandran, Sugesh wrote:
> > Hi Kevin,
> > Thank you for sending out this patch series.
> > Have you tested the tunneling decap usecase with checksum offload? I
> > am seeing weird behavior when I testing the tunneling with Rx checksum
> > offload ON and OFF.(Seeing the same behavior on master as well)
> >
> > Here is the summary of issue with the steps,
> >
> > 1) Send tunnel traffic to OVS to do the decap.
> > 2) Set & unset the checksum offload.
> > 3) I don't see any performance difference in both case.
> >
> > Now I went ahead and put some debug message to see what is happening
> >
> > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index
> > 2798324..49ca847 100644
> > --- a/lib/netdev-native-tnl.c
> > +++ b/lib/netdev-native-tnl.c
> > @@ -86,6 +86,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet
> *packet, struct flow_tnl *tnl,
> >  ovs_be32 ip_src, ip_dst;
> >
> >  if (OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) {
> > +VLOG_INFO("Checksum is not validated...");
> >  if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> >  VLOG_WARN_RL(_rl, "ip packet has invalid checksum");
> >  return NULL;
> > @@ -182,6 +183,7 @@ udp_extract_tnl_md(struct dp_packet *packet,
> > struct flow_tnl *tnl,
> >
> >  if (udp->udp_csum) {
> >  if (OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) {
> > +VLOG_INFO("Checksum is not validated...");
> >  uint32_t csum;
> >  if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> >  csum =
> > packet_csum_pseudoheader6(dp_packet_l3(packet));
> > sugeshch@silpixa00389816:~/repo/ovs_master$
> >
> > These debug messages are not showing at all when I am sending the
> > traffic. (I tried it with rx checksum ON and OFF)
> >
> > Looks like ol_flags are always reporting checksum is good.
> >
> > I am using DPDK 17.02 for the testing.
> > If I remember correctly it was reporting properly at the time of rx checksum
> offload.
> > Looks like DPDK is reporting checksum valid in all the cases even it is
> disabled.
> >
> > Any inputs on this?
> >
> 
> Hi Sugesh, I was trying to fix the reconfiguration code not applying the
> OVSDB value so that's all I tested. I guess it's best to roll back to your 
> original
> test and take it from there? You probably tested with DPDK
> 16.11.0 and I see some changes since then (e.g. below). Also, maybe you
> were testing enabled/disabled on first configure? It's the same configure
> code, but perhaps there is some different state in DPDK when the port is
> configured initially.
> 
Yes, I tried to configure initially as well as run time.
[Sugesh] Also,
At the time of Rx checksum offload implementation, one of the comments suggests 
not to use any configuration option at all.
Keep it ON for all the physical ports when it is supported. The reason being 
the configuration option is added is due to the vectorization.
In the earlier DPDK releases the vectorization will turned off when checksum 
offload is enabled. 
I feel that is not the case for the latest DPDK releases (Vectorization is ON 
with checksum offload).
If there is no impact of vectorization, then there is no usecase for having 
this configuration option at all.
This way there is one less thing for the user to worry about. What do you 
think? 
Do you think it makes any backward compatibility issues??

> commit f3a85f4ce04d9fb55ebdb392563f7c1711f3d943
> Author: Qi Zhang 
> Date:   Tue Jan 24 14:06:59 2017 -0500
> 
> net/i40e: fix checksum flag in x86 vector Rx
> 
> When no error reported in Rx descriptor, we should set
> CKSUM_GOOD flag before return.
> 
> Fixes: 9966a00a0688 ("net/i40e: enable bad checksum flags in vector Rx")
> Cc: sta...@dpdk.org
> 
> Signed-off-by: Qi Zhang 
> 
> 
> HTH,
> Kevin.
> 
> >
> >
> > Regards
> > _Sugesh
> >
> >
> >> -Original Message-
> >> From: Kevin Traynor [mailto:ktray...@redhat.com]
> >> Sent: Friday, May 12, 2017 6:22 PM
> >> To: d...@openvswitch.org
> >> Cc: Chandran, Sugesh ; Kevin Traynor
> >> 
> >> Subject: [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure.
> >>
> >> Rx checksum offload is enabled by default where available, with
> >> reconfiguration through OVSDB options:rx-checksum-offload.
> >> However, setting rx-checksum-offload does not result in a
> >> reconfiguration of the NIC.
> >>
> >> Fix that by checking if the requested port config features (e.g. rx
> >> checksum
> >> offload) are currently applied on the NIC and if not, perform a
> >>

Re: [ovs-dev] [PATCH 4.4-only] openvswitch: clear sender cpu before forwarding packets

2017-05-17 Thread Anoob Soman 

On 17/05/17 09:19, Greg KH wrote:

Why is this a non-upstream patch?  What commit in Linus's tree fixed
this?  Why not just backport that?

thanks,

greg k-h


Agreed, I think it is sensible to backport 52bd2d62ce67 "net: better 
skb->sender_cpu and skb->napi_id cohabitation" to 4.4, rather than 
having a different patch.


I think backport might be required for 4.1 as well, but I haven't checked.

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

[ovs-dev] Invitation: Good morning, @ Wed May 17, 2017 5:30am - 6:30am (EDT) (d...@openvswitch.org)

2017-05-17 Thread davidata39 

You have been invited to the following event.

Title: Good morning,
Good morning,

I hope you are fine today?

I don't know if you might be interested in a business proposal which am  
about to reveal to you considering that we haven't met in person.


Its all about US$(Ten Million, One Hundred Thousand Dollars) Â dormant fund  
in our bank coded account here..


I am the only person with the knowledge of the funds and its deposit, and  
will solicit for your partnership for us to have it. My position in the  
bank will guarantee easy and risk-free handling of the transaction as i  
have every details of it.


I will give your more details as soon as I hear from you, including the  
sharing ratio.Please ignore the proposition, if you don't seem to be  
interested.


Contact me through my private e-mail ( davidat...@yahoo.com) for more  
details.Otherwise, please delete this email and I say thanks for your time.


This transaction is 100% risky free.

Please keep confidential!!

Thanks,

Mr.David Ata,
When: Wed May 17, 2017 5:30am – 6:30am Eastern Time
Calendar: d...@openvswitch.org
Who:
(Guest list has been hidden at organizer's request)

Event details:  
https://www.google.com/calendar/event?action=VIEW=OXN2anJqMnAwcDZ1NG5zcTRtbTVoNGZrZjggZGV2QG9wZW52c3dpdGNoLm9yZw=MjAjZGF2aWRhdGEzOUBnbWFpbC5jb21hZGIxODJjNGUwYzQyZGNlMGIwZGVlMmVkOTZhOTQ2NGIxNzY5NjI2=America/New_York=en


Invitation from Google Calendar: https://www.google.com/calendar/

You are receiving this courtesy email at the account d...@openvswitch.org  
because you are an attendee of this event.


To stop receiving future updates for this event, decline this event.  
Alternatively you can sign up for a Google account at  
https://www.google.com/calendar/ and control your notification settings for  
your entire calendar.


Forwarding this invitation could allow any recipient to modify your RSVP  
response. Learn more at  
https://support.google.com/calendar/answer/37135#forwarding
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure.

2017-05-17 Thread Kevin Traynor 
On 05/16/2017 05:48 PM, Chandran, Sugesh wrote:
> Hi Kevin,
> Thank you for sending out this patch series.
> Have you tested the tunneling decap usecase with checksum offload? I am 
> seeing weird behavior when I testing the tunneling with Rx checksum offload 
> ON and OFF.(Seeing the same behavior on master as well)
> 
> Here is the summary of issue with the steps,
> 
> 1) Send tunnel traffic to OVS to do the decap.
> 2) Set & unset the checksum offload.
> 3) I don't see any performance difference in both case.
> 
> Now I went ahead and put some debug message to see what is happening
> 
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index 2798324..49ca847 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -86,6 +86,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, 
> struct flow_tnl *tnl,
>  ovs_be32 ip_src, ip_dst;
>  
>  if (OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) {
> +VLOG_INFO("Checksum is not validated...");
>  if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
>  VLOG_WARN_RL(_rl, "ip packet has invalid checksum");
>  return NULL;
> @@ -182,6 +183,7 @@ udp_extract_tnl_md(struct dp_packet *packet, struct 
> flow_tnl *tnl,
>  
>  if (udp->udp_csum) {
>  if (OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) {
> +VLOG_INFO("Checksum is not validated...");
>  uint32_t csum;
>  if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
>  csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> sugeshch@silpixa00389816:~/repo/ovs_master$
> 
> These debug messages are not showing at all when I am sending the traffic. (I 
> tried it with rx checksum ON and OFF)
> 
> Looks like ol_flags are always reporting checksum is good.
> 
> I am using DPDK 17.02 for the testing.
> If I remember correctly it was reporting properly at the time of rx checksum 
> offload.
> Looks like DPDK is reporting checksum valid in all the cases even it is 
> disabled.
> 
> Any inputs on this?
> 

Hi Sugesh, I was trying to fix the reconfiguration code not applying the
OVSDB value so that's all I tested. I guess it's best to roll back to
your original test and take it from there? You probably tested with DPDK
16.11.0 and I see some changes since then (e.g. below). Also, maybe you
were testing enabled/disabled on first configure? It's the same
configure code, but perhaps there is some different state in DPDK when
the port is configured initially.

commit f3a85f4ce04d9fb55ebdb392563f7c1711f3d943
Author: Qi Zhang 
Date:   Tue Jan 24 14:06:59 2017 -0500

net/i40e: fix checksum flag in x86 vector Rx

When no error reported in Rx descriptor, we should set
CKSUM_GOOD flag before return.

Fixes: 9966a00a0688 ("net/i40e: enable bad checksum flags in vector Rx")
Cc: sta...@dpdk.org

Signed-off-by: Qi Zhang 


HTH,
Kevin.

> 
> 
> Regards
> _Sugesh
> 
> 
>> -Original Message-
>> From: Kevin Traynor [mailto:ktray...@redhat.com]
>> Sent: Friday, May 12, 2017 6:22 PM
>> To: d...@openvswitch.org
>> Cc: Chandran, Sugesh ; Kevin Traynor
>> 
>> Subject: [PATCH v2 1/3] netdev-dpdk: Fix Rx checksum reconfigure.
>>
>> Rx checksum offload is enabled by default where available, with
>> reconfiguration through OVSDB options:rx-checksum-offload.
>> However, setting rx-checksum-offload does not result in a reconfiguration of
>> the NIC.
>>
>> Fix that by checking if the requested port config features (e.g. rx checksum
>> offload) are currently applied on the NIC and if not, perform a
>> reconfiguration.
>>
>> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature
>> on DPDK physical ports.")
>> Cc: Sugesh Chandran 
>> Signed-off-by: Kevin Traynor 
>> ---
>>  lib/netdev-dpdk.c | 14 +-
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 609b8da..d1688ce
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -374,4 +374,5 @@ struct netdev_dpdk {
>>  int requested_rxq_size;
>>  int requested_txq_size;
>> +uint32_t requested_hwol;
>>
>>  /* Number of rx/tx descriptors for physical devices */ @@ -648,5 +649,5
>> @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int
>> n_txq)
>>  conf.rxmode.max_rx_pkt_len = 0;
>>  }
>> -conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>> +conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
>>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>  /* A device may report more queues than it makes available (this has @@
>> -702,4 +703,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
>> int n_rxq, int n_txq)
>>  dev->up.n_rxq = n_rxq;
>>  dev->up.n_txq = n_txq;
>> +

Re: [ovs-dev] [PATCH 4.4-only] openvswitch: clear sender cpu before forwarding packets

2017-05-17 Thread Greg KH 
On Tue, May 16, 2017 at 03:25:10PM +0100, Anoob Soman wrote:
> Similar to commit c29390c6dfee ("xps: must clear sender_cpu before
> forwarding") the skb->sender_cpu needs to be cleared before forwarding
> packets.
> 
> Fixes: 2bd82484bb4c ("xps: fix xps for stacked devices")
> Signed-off-by: Anoob Soman 
> ---
>  net/openvswitch/vport.c | 1 +
>  1 file changed, 1 insertion(+)

Why is this a non-upstream patch?  What commit in Linus's tree fixed
this?  Why not just backport that?

thanks,

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

[ovs-dev] [PATCH v2] ovn: increase size of ingress and egress pipelines

2017-05-17 Thread Mickey Spiegel 
The OVN ingress pipeline for a logical switch is maxed out at 16 stages.

This patch takes the simple approach of starting the ingress pipeline at
table 8 rather than table 16, and starting the egress pipeline at
table 40 rather than table 48.

v1->v2:
Bumped range of Logical_Flow.table_id column in ovn/ovn-sb.ovsschema
from 0 to 15, to 0 to 23.
Ran automated tests with an extra noop table, pushing S_SWITCH_IN_L2_LKUP
to 16.

Signed-off-by: Mickey Spiegel 
---
 ovn/controller/lflow.h |  6 +++---
 ovn/ovn-architecture.7.xml | 27 ++-
 ovn/ovn-sb.ovsschema   |  6 +++---
 ovn/utilities/ovn-trace.c  |  2 +-
 tests/ovn.at   | 40 
 tests/test-ovn.c   |  6 +++---
 6 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index 8761b1e..a23cde0 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -49,17 +49,17 @@ struct uuid;
  * These are heavily documented in ovn-architecture(7), please update it if
  * you make any changes. */
 #define OFTABLE_PHY_TO_LOG0
-#define OFTABLE_LOG_INGRESS_PIPELINE 16 /* First of LOG_PIPELINE_LEN tables. */
+#define OFTABLE_LOG_INGRESS_PIPELINE  8 /* First of LOG_PIPELINE_LEN tables. */
 #define OFTABLE_REMOTE_OUTPUT32
 #define OFTABLE_LOCAL_OUTPUT 33
 #define OFTABLE_CHECK_LOOPBACK   34
-#define OFTABLE_LOG_EGRESS_PIPELINE  48 /* First of LOG_PIPELINE_LEN tables. */
+#define OFTABLE_LOG_EGRESS_PIPELINE  40 /* First of LOG_PIPELINE_LEN tables. */
 #define OFTABLE_SAVE_INPORT  64
 #define OFTABLE_LOG_TO_PHY   65
 #define OFTABLE_MAC_BINDING  66
 
 /* The number of tables for the ingress and egress pipelines. */
-#define LOG_PIPELINE_LEN 16
+#define LOG_PIPELINE_LEN 24
 
 void lflow_init(void);
 void lflow_run(struct controller_ctx *,
diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
index d8114f1..eb6744b 100644
--- a/ovn/ovn-architecture.7.xml
+++ b/ovn/ovn-architecture.7.xml
@@ -774,7 +774,7 @@
 VXLAN tunnels do not transmit the logical output port field.
 Since VXLAN tunnels do not carry a logical output port field in
 the tunnel key, when a packet is received from VXLAN tunnel by
-an OVN hypervisor, the packet is resubmitted to table 16 to
+an OVN hypervisor, the packet is resubmitted to table 8 to
 determine the output port(s);  when the packet reaches table 32,
 these packets are resubmitted to table 33 for local delivery by
 checking a MLF_RCV_FROM_VXLAN flag, which is set when the packet
@@ -835,7 +835,7 @@
 the packet's ingress port.  Its actions annotate the packet with
 logical metadata, by setting the logical datapath field to identify the
 logical datapath that the packet is traversing and the logical input
-port field to identify the ingress port.  Then it resubmits to table 16
+port field to identify the ingress port.  Then it resubmits to table 8
 to enter the logical ingress pipeline.
   
 
@@ -864,13 +864,13 @@
 
 
   
-OpenFlow tables 16 through 31 execute the logical ingress pipeline from
+OpenFlow tables 8 through 31 execute the logical ingress pipeline from
 the Logical_Flow table in the OVN Southbound database.
 These tables are expressed entirely in terms of logical concepts like
 logical ports and logical datapaths.  A big part of
 ovn-controller's job is to translate them into equivalent
 OpenFlow (in particular it translates the table numbers:
-Logical_Flow tables 0 through 15 become OpenFlow tables 16
+Logical_Flow tables 0 through 23 become OpenFlow tables 8
 through 31).
   
 
@@ -999,7 +999,7 @@
 and resubmit these packets to table 33 for local delivery. Packets
 received from VXLAN tunnels reach here because of a lack of logical
 output port field in the tunnel key and thus these packets needed to
-be submitted to table 16 to determine the output port.
+be submitted to table 8 to determine the output port.
   
 
   
@@ -1024,13 +1024,13 @@
   
 Table 34 matches and drops packets for which the logical input and
 output ports are the same and the MLF_ALLOW_LOOPBACK flag is not
-set.  It resubmits other packets to table 48.
+set.  It resubmits other packets to table 40.
   
 
 
 
   
-OpenFlow tables 48 through 63 execute the logical egress pipeline from
+OpenFlow tables 40 through 63 execute the logical egress pipeline from
 the Logical_Flow table in the OVN Southbound database.
 The egress pipeline can perform a final stage of validation before
 packet delivery.  Eventually, it may execute an output
@@ -1110,27 +1110,28 @@
 
 
   In OVS versions 2.7