Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

[Roi Dayan]

On 05/04/2017 10:08, Joe Stringer wrote:

On 4 April 2017 at 21:18, Roi Dayan  wrote:



On 04/04/2017 23:53, Joe Stringer wrote:


On 3 April 2017 at 10:53, Joe Stringer  wrote:


On 3 April 2017 at 03:27, Roi Dayan  wrote:




On 29/03/2017 20:13, Joe Stringer wrote:



On 29 March 2017 at 04:50, Roi Dayan  wrote:





On 23/03/2017 09:01, Joe Stringer wrote:




I ran the make check-offloads tests on a recent net-next kernel and
it
failed, output was not as expected:

../../tests/system-offloaded-traffic.at:54
: ovs-appctl dpctl/dump-flows
|
grep "eth_type(0x0800)" | sed -e



's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac
s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
--- - 2017-03-22 16:43:37.598689692 -0700
+++



/home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout
2017-03-22 16:43:37.595628000 -0700
@@ -1,3 +1,3 @@
-in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
-in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
used:0.001s, actions:output
+in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output
+in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
bytes:882, used:0.001s, actions:output



Hi Joe,

can you tell me what kernel you used here?
maybe tc offloads were not supported and there was a fallback to OVS
dp.




I believe that it was a snapshot of net-next relatively recently,
01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
again with latest net-next? Or do you think there may be some
userspace dependency the test relies on?



I installed net-next kernel and make check-offloads pass for me.
The last commit I'm on is
397df70 Merge branch '40GbE' of
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
last tag is 4.11-rc3+
I'm thinking maybe the test fails for you from something else like a
second
openvswitch process running already?



I don't think that was the case, but let me try again with your latest
series and the above commit.



I saw the same behaviour with upstream net-next 397df7092a15. My host
is Ubuntu 14.04 with this kernel.

I thought it might be because I'm not running any of your hardware and
I assumed that the testsuite doesn't require hardware to run. Looking
at the test it seems that assumption was wrong, but when I tried to
configure the tc-policy to skip_hw with the following modification,
the OVSDB change didn't seem to propagate into OVS (there were no log
messages about changing the tc-policy):

diff --git a/tests/system-offloaded-traffic.at
b/tests/system-offloaded-traffic.at
index 7aec8a3f430e..3ddf23a939a8 100644
--- a/tests/system-offloaded-traffic.at
+++ b/tests/system-offloaded-traffic.at
@@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
offloads enabled])
OVS_TRAFFIC_VSWITCHD_START()

AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:tc-policy="skip_hw"])
AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])

ADD_NAMESPACES(at_ns0, at_ns1)

---

Looking again, my kernel config had CLS_FLOWER disabled so that's
probably what caused the issue. My ovs-vswitchd log from the test is
below.

2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file

/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
NUMA node 0
2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
and 2 CPU cores

2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connecting...

2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
connected
2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch)
2.7.90
2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
Datapath supports recirculation
2017-04-04T20:41:50.757Z|8|ofproto_dpif|INFO|system@ovs-system:
MPLS label stack length probed as 1
2017-04-04T20:41:50.757Z|9|ofproto_dpif|INFO|system@ovs-system:
Datapath supports truncate action
2017-04-04T20:41:50.757Z|00010|ofproto_dpif|INFO|system@ovs-system:
Datapath supports unique flow ids
2017-04-04T20:41:50.757Z|00011|ofproto_dpif|INFO|system@ovs-system:
Datapath does not support clone action
2017-04-04T20:41:50.757Z|00012|ofproto_dpif|INFO|system@ovs-system:
Max sample nesting level probed as 10
2017-04-04T20:41:50.757Z|00013|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_state
2017-04-04T20:41:50.757Z|00014|ofproto_dpif|INFO|system@ovs-system:
Datapath supports ct_zone
2017-04-04T20:41:50.757Z|00015|ofproto_dpif|INFO|system@ovs-system:
Datapath supports 

Re: [ovs-dev] [PATCH v6 0/5] datapath-windows: Add support for Ipv4 fragments

[Alin Serdean]

Thanks a lot for all the hard work Anand!



The patches are in good state overall with some small nits .



In the comments over the patches I said I will add an incremental for them.



I attached a diff to hopefully get you going.



Disregarding the changes to ` OvsApplySWChecksumOnNB` which I will send on a 
patch tomorrow,

I will copy the rest of things over here and go over them and added [AS] in 
front and bold it:

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c

index a68679c..42f0459 100644

--- a/datapath-windows/ovsext/Actions.c

+++ b/datapath-windows/ovsext/Actions.c

@@ -676,6 +676,32 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)

 }

 OVS_FWD_INFO switchFwdInfo = { 0 };

+POVS_BUFFER_CONTEXT ctx = 
(POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(ovsFwdCtx->curNbl);

+if (ctx->mru != 0) {

+/* Fragment nbl based on mru. If it returns NULL then the original

+ * reassembled NBL is sent out to the VIF which will be dropped if

+ * the packet size is more than VIF MTU.

+ */

+POVS_VPORT_ENTRY tx = ovsFwdCtx->tunnelTxNic;

+PNET_BUFFER_LIST fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,

+  ovsFwdCtx->curNbl,

+  &(ovsFwdCtx->layers),

+  ctx->mru, 0, TRUE);

+if (fragNbl != NULL) {

+OvsCompleteNBLForwardingCtx(ovsFwdCtx,

+L"Dropped since fragmenting NBL");

+status = OvsInitForwardingCtx(ovsFwdCtx,

+  ovsFwdCtx->switchContext,

+  fragNbl,

+  ovsFwdCtx->srcVportNo,

+  ovsFwdCtx->sendFlags,

+  
NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(fragNbl),

+  ovsFwdCtx->completionList,

+  >layers, FALSE);

+ovsFwdCtx->tunnelTxNic = tx;

+}

+}

+

[AS] When outputting a packet over a tunnel we need to make sure that the 
packet was fragmented prior of it being sent out via a type of tunnel. 
Otherwise the inner packet packet + headroom will be over the MTU of the port 
on which it will be outputted.

 /* Apply the encapsulation. The encapsulation will not consume the NBL. */

 switch(ovsFwdCtx->tunnelTxNic->ovsType) {

 case OVS_VPORT_TYPE_GRE:

@@ -890,14 +916,17 @@ OvsOutputForwardingCtx(OvsForwardingContext *ovsFwdCtx)

  */

 if (ovsFwdCtx->tunnelTxNic != NULL || ovsFwdCtx->tunnelRxNic != NULL) {

 nb = NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl);

+POVS_BUFFER_CONTEXT srcCtx = 
(POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(ovsFwdCtx->curNbl);

 newNbl = OvsPartialCopyNBL(ovsFwdCtx->switchContext, 
ovsFwdCtx->curNbl,

0, 0, TRUE /*copy NBL info*/);

+POVS_BUFFER_CONTEXT dstCtx = 
(POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(newNbl);

 if (newNbl == NULL) {

 status = NDIS_STATUS_RESOURCES;

 ovsActionStats.noCopiedNbl++;

 dropReason = L"Dropped due to failure to create NBL copy.";

 goto dropit;

 }

+dstCtx->mru = srcCtx->mru;

[AS] Just to copy over the mru after the PartialCopy

 }

 /* It does not seem like we'll get here unless 'portsToUpdate' > 0. */

@@ -2058,11 +2087,21 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,

 goto dropit;

 }

 }

-

+PNET_BUFFER_LIST bla = ovsFwdCtx.curNbl;

 status = OvsExecuteConntrackAction(switchContext, 
,

ovsFwdCtx.completionList,


ovsFwdCtx.fwdDetail->SourcePortId,

layers, key, (const PNL_ATTR)a);

+if (bla != ovsFwdCtx.curNbl) {

+OvsInitForwardingCtx(,

+ovsFwdCtx.switchContext,

+ovsFwdCtx.curNbl,

+ovsFwdCtx.srcVportNo,

+ovsFwdCtx.sendFlags,

+NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(ovsFwdCtx.curNbl),

+ovsFwdCtx.completionList,

+, FALSE);

+}

[AS] This is needed because in ` OvsIpv4Reassemble` you complete the original 
nbl and create a new one and assign it over to the curNbl of the ovsFwdCtx. A 
side effect of completing the nbl will be that it will remove the ` 

Re: [ovs-dev] [PATCH 3/3 v2] datapath-windows: NAT integration with conntrack

[Alin Serdean]

Hi Yin,

I'm back sorry it took a while .

When applying `OvsUpdateAddressAndPort` for some reason there still is a 
problem with the checksums.

There are two aspects which apparently are hit for some reason:
1. normal packet.
2. tcp segment.

If I disable tcp/udp checksums(on the NIC) normal packets have the right 
checksums. However, the tcp segments checksums are still wrong.

If I apply a full blown checksum using ` OvsApplySWChecksumOnNB` and ` 
OvsTcpSegmentNBL`(just for checksum computations) things are fine with tcp/udp 
checksum enabled/disabled and lso enabled/disabled.

From what I can think of the pseudochecksums are wrong when being sent down to 
a NIC.

I'll try to dig further into this issue since what you apply here is the same 
thing we apply on the set ip/tcp port actions and that works fine.

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Yin Lin
> Sent: Thursday, March 23, 2017 12:12 AM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH 3/3 v2] datapath-windows: NAT integration with
> conntrack
> 
> Signed-off-by: Yin Lin 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 5/5] datapath-windows: Fragment NBL based on MRU size

[Alin Serdean]

Mind creating a function for it? You will need to add the same for 
OvsTunnelPortTx as well, otherwise the packet won't be fragmented to the vif 
value before sending it via a tunnel type (i.e. vxlan).

I will try to explain more on patch 0.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Anand Kumar
> Sent: Friday, March 24, 2017 10:51 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v6 5/5] datapath-windows: Fragment NBL based
> on MRU size
> 
> This patch adds support for Fragmenting NBL based on the MRU value.
> MRU value is updated only for Ipv4 fragments, if it is non zero, then fragment
> the NBL and send out the new NBL to the vnic.
> 
> Signed-off-by: Anand Kumar 
> ---
> v5->v6: No Change
> v4->v5:
>   - Use MRU information in the _OVS_BUFFER_CONTEXT to fragment
> NBL.
> v3->v4: No Change
> v2->v3:
>   - Updated log message
> v1->v2: No change
> ---
>  datapath-windows/ovsext/Actions.c | 27
> +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-
> windows/ovsext/Actions.c
> index cbc7640..a68679c 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -34,6 +34,7 @@
>  #include "Vport.h"
>  #include "Vxlan.h"
>  #include "Geneve.h"
> +#include "IpFragment.h"
> 
>  #ifdef OVS_DBG_MOD
>  #undef OVS_DBG_MOD
> @@ -864,6 +865,8 @@ OvsOutputForwardingCtx(OvsForwardingContext
> *ovsFwdCtx)
>  NDIS_STATUS status = STATUS_SUCCESS;
>  POVS_SWITCH_CONTEXT switchContext = ovsFwdCtx->switchContext;
>  PCWSTR dropReason;
> +PNET_BUFFER_LIST fragNbl = NULL;
> +POVS_BUFFER_CONTEXT ctx;
> 
>  /*
>   * Handle the case where the some of the destination ports are tunneled
> @@ -909,6 +912,30 @@ OvsOutputForwardingCtx(OvsForwardingContext
> *ovsFwdCtx)
>  goto dropit;
>  }
> 
> +ctx =
> (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(ovsF
> wdCtx->curNbl);
> +if (ctx->mru != 0) {
> +/* Fragment nbl based on mru. If it returns NULL then the 
> original
> + * reassembled NBL is sent out to the VIF which will be dropped 
> if
> + * the packet size is more than VIF MTU.
> + */
> +fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,
> + ovsFwdCtx->curNbl,
> + &(ovsFwdCtx->layers),
> + ctx->mru, 0, TRUE);
> +if (fragNbl != NULL) {
> +OvsCompleteNBLForwardingCtx(ovsFwdCtx,
> +L"Dropped since fragmenting 
> NBL");
> +status = OvsInitForwardingCtx(ovsFwdCtx,
> +  ovsFwdCtx->switchContext,
> +  fragNbl,
> +  ovsFwdCtx->srcVportNo,
> +  ovsFwdCtx->sendFlags,
> +
> NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(fragNbl),
> +  ovsFwdCtx->completionList,
> +  >layers, FALSE);
> +}
> +}
> +
>  OvsSendNBLIngress(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl,
>ovsFwdCtx->sendFlags);
>  /* End this pipeline by resetting the corresponding context. */
> --
> 2.9.3.windows.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 4/5] datapath-windows: Updated OvsTcpSegmentNBL to handle IP fragments.

[Alin Serdean]

Since you split the functionality on `BOOLEAN isFragment` mind just creating 
new function and leaving FixSegmentHeader as is. I think it would look much 
cleaner.

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Anand Kumar
> Sent: Friday, March 24, 2017 10:51 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v6 4/5] datapath-windows: Updated
> OvsTcpSegmentNBL to handle IP fragments.
> 
> With this patch, OvsTcpSegmentNBL not only supports fragmenting NBL to
> TCP segments but also Ipv4 fragments.
> 
> To reflect the new changes, renamed function name from
> OvsTcpSegmentNBL to OvsFragmentNBL and created a wrapper for
> OvsTcpSegmentNBL.
> 
> Signed-off-by: Anand Kumar 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 2/5] datapath-windows: Added Ipv4 fragments support in Conntrack

[Alin Serdean]

Thanks a lot for the patch.

>From what I see you and Yin(https://patchwork.ozlabs.org/patch/742367/) both 
>benefit by using `ovsFwdCtx` (OvsForwardingContext) in the Conntrack pieces.
IMO it make sense just to fire a single patch just to add 
OvsExecuteConntrackAction/OvsCtExecute_ so both of you can benefit from it 
later.
What do you think Sai?

Also if you pass the forwarding context I don't think you will need the 
following, passing it over too multiple functions: 
> +PNET_BUFFER_LIST newNbl = NULL;
[Alin Serdean] and initialize the curNbl from ovsFwdCtx with the new allocated 
nbl in OvsIpv4Reassemble. I will explain more in patch 0 why reiniting the 
fwdcontext is needed.
>  NDIS_STATUS status;
> 
> -status = OvsDetectCtPacket(key);
> +status = OvsDetectCtPacket(switchContext, curNbl, completionList,
> +   sourcePort, key, );
>  if (status != NDIS_STATUS_SUCCESS) {
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] python: Allow tuning the session probe_interval from IDL

[Russell Bryant]

On Thu, Apr 6, 2017 at 6:25 AM, Lucas Alvares Gomes
 wrote:
> This patch is adding a new method set_probe_interval() to the Idl class
> which allows configuring the probe interval for the IDL session.
> Instances of Idl might want to tune this interval to be less agressive
> than the current 5s one.
>
> Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1680146
> Signed-off-by: Lucas Alvares Gomes 
> ---
>  python/ovs/db/idl.py | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index 079a03ba1..d3c0086c6 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -147,6 +147,15 @@ class Idl(object):
>  table.condition = [True]
>  table.cond_changed = False
>
> +def set_probe_interval(self, probe_interval):
> +"""Sets the probe interval for the IDL session, in milliseconds.
> +
> +If "probe_interval" is zero it disables the connection keepalive
> +feature. If non-zero the value will be forced to at least 1000
> +milliseconds.
> +"""
> +self._session.reconnect.set_probe_interval(probe_interval)
> +
>  def close(self):
>  """Closes the connection to the database.  The IDL will no longer
>  update."""

Thanks for the patch!

Nothing else in the Idl class is touching the reconnect object
directly, which makes me wonder if this should also be a method on the
session, or perhaps an option to Session.open().

I also wonder if it'd be better as an optional argument to the
constructor so that we can pass in the desired value before the
connection is opened and is used from the beginning.  The separate
method is tempting as far as writing code that can deal with versions
of this library that may or may not have support for the official way
of setting this, but it still feels like the "right" way is to be able
to set the value before the connection is even opened.  What do you
think?

I feel like I'm nit picking this, sorry for that.

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

Re: [ovs-dev] [PATCH 1/2] doc: Describe how docs.openvswitch.org works

[Russell Bryant]

On Thu, Apr 6, 2017 at 9:58 AM, Stephen Finucane  wrote:
> Signed-off-by: Stephen Finucane 
> ---
>  Documentation/index.rst|  3 +-
>  .../internals/contributing/documentation-style.rst |  6 ++
>  Documentation/internals/documentation.rst  | 78 
> ++
>  Documentation/internals/index.rst  |  1 +
>  4 files changed, 87 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/internals/documentation.rst

Thanks for the patches!  I applied one minor change to this patch and
applied both patches in this series to master.


diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 5a82431..1fd452b 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -77,6 +77,7 @@ DOC_SOURCE = \
Documentation/internals/bugs.rst \
Documentation/internals/committer-grant-revocation.rst \
Documentation/internals/committer-responsibilities.rst \
+   Documentation/internals/documentation.rst \
Documentation/internals/mailing-lists.rst \
Documentation/internals/maintainers.rst \
Documentation/internals/patchwork.rst \


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

Re: [ovs-dev] [PATCH] m4: Add hard requirements for python in "configure"

[Ben Pfaff]

On Thu, Apr 06, 2017 at 07:20:32PM +0200, Timothy M. Redaelli wrote:
> On 04/06/2017 12:16 AM, Ben Pfaff wrote:
> > On Wed, Mar 29, 2017 at 01:49:42PM +0200, Timothy Redaelli wrote:
> >> Since Python 2.7 and python-six are needed to build Open vSwitch,
> >> ./configure should return an error if they are missing or if they are too 
> >> old
> >>
> >> Signed-off-by: Timothy Redaelli 
> > 
> > Thanks for the patch.
> > 
> > I think that if we do it this way, then eventually we'll have to debug
> > the problem.  I suggest that we use the following instead, which does a
> > better job of dumping problems into config.log for troubleshooting.
> > 
> > What do you think?
> 
> LGTM
> 
> Tested cases:
>  - missing python
>  - python version < 2.7
>  - python version >= 2.7, without python-six
>  - python version >= 2.7, with old python-six
> 
>  - python version >= 2.7, with correct python-six
> 
> Thank you

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

Re: [ovs-dev] [PATCH v2] bundle: add nw_src/dst hash method

[Ben Pfaff]

On Thu, Mar 09, 2017 at 12:01:02PM +0800, we...@ucloud.cn wrote:
> From: wenxu 
> 
> Add only nw_src or nw_dst hash feature to bundle and multipath.
> 
> Signed-off-by: wenxu 

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

[ovs-dev] [datapath backport 10/10] datapath: Openvswitch: Refactor sample and recirc actions implementation

[Andy Zhou]

Upstream commit:
Openvswitch: Refactor sample and recirc actions implementation

Added clone_execute() that both the sample and the recirc
action implementation can use.

Signed-off-by: Andy Zhou 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Upstream: bef7f7567a10 ("Openvswitch: Refactor sample and recirc actions 
implementation")
Signed-off-by: Andy Zhou 
---
 datapath/actions.c | 172 +
 1 file changed, 93 insertions(+), 79 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 32c0c10e7c62..1f3b2fe4ab73 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -169,6 +169,12 @@ static bool is_flow_key_valid(const struct sw_flow_key 
*key)
return !(key->mac_proto & SW_FLOW_KEY_INVALID);
 }
 
+static int clone_execute(struct datapath *dp, struct sk_buff *skb,
+struct sw_flow_key *key,
+u32 recirc_id,
+const struct nlattr *actions, int len,
+bool last, bool clone_flow_key);
+
 static void update_ethertype(struct sk_buff *skb, struct ethhdr *hdr,
 __be16 ethertype)
 {
@@ -927,10 +933,9 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
 {
struct nlattr *actions;
struct nlattr *sample_arg;
-   struct sw_flow_key *orig_key = key;
int rem = nla_len(attr);
-   int err = 0;
const struct sample_arg *arg;
+   bool clone_flow_key;
 
/* The first action is always 'OVS_SAMPLE_ATTR_ARG'. */
sample_arg = nla_data(attr);
@@ -944,43 +949,9 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
return 0;
}
 
-   /* Unless the last action, sample works on the clone of SKB.  */
-   skb = last ? skb : skb_clone(skb, GFP_ATOMIC);
-   if (!skb) {
-   /* Out of memory, skip this sample action.
-*/
-   return 0;
-   }
-
-   /* In case the sample actions won't change 'key',
-* it can be used directly to execute sample actions.
-* Otherwise, allocate a new key from the
-* next recursion level of 'flow_keys'. If
-* successful, execute the sample actions without
-* deferring.
-*
-* Defer the sample actions if the recursion
-* limit has been reached.
-*/
-   if (!arg->exec) {
-   __this_cpu_inc(exec_actions_level);
-   key = clone_key(key);
-   }
-
-   if (key) {
-   err = do_execute_actions(dp, skb, key, actions, rem);
-   } else if (!add_deferred_actions(skb, orig_key, actions, rem)) {
-
-   if (net_ratelimit())
-   pr_warn("%s: deferred action limit reached, drop sample 
action\n",
-   ovs_dp_name(dp));
-   kfree_skb(skb);
-   }
-
-   if (!arg->exec)
-   __this_cpu_dec(exec_actions_level);
-
-   return err;
+   clone_flow_key = !arg->exec;
+   return clone_execute(dp, skb, key, 0, actions, rem, last,
+clone_flow_key);
 }
 
 static void execute_hash(struct sk_buff *skb, struct sw_flow_key *key,
@@ -1091,10 +1062,9 @@ static int execute_masked_set_action(struct sk_buff *skb,
 
 static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
  struct sw_flow_key *key,
- const struct nlattr *a, int rem)
+ const struct nlattr *a, bool last)
 {
-   struct sw_flow_key *recirc_key;
-   struct deferred_action *da;
+   u32 recirc_id;
 
if (!is_flow_key_valid(key)) {
int err;
@@ -1105,40 +1075,8 @@ static int execute_recirc(struct datapath *dp, struct 
sk_buff *skb,
}
BUG_ON(!is_flow_key_valid(key));
 
-   if (!nla_is_last(a, rem)) {
-   /* Recirc action is the not the last action
-* of the action list, need to clone the skb.
-*/
-   skb = skb_clone(skb, GFP_ATOMIC);
-
-   /* Skip the recirc action when out of memory, but
-* continue on with the rest of the action list.
-*/
-   if (!skb)
-   return 0;
-   }
-
-   /* If within the limit of 'OVS_DEFERRED_ACTION_THRESHOLD',
-* recirc immediately, otherwise, defer it for later execution.
-*/
-   recirc_key = clone_key(key);
-   if (recirc_key) {
-   recirc_key->recirc_id = nla_get_u32(a);
-   ovs_dp_process_packet(skb, recirc_key);
-   } else {
-   da = add_deferred_actions(skb, key, NULL, 0);
-   if (da) {
-   recirc_key = >pkt_key;
-   recirc_key->recirc_id = nla_get_u32(a);
-   } 

[ovs-dev] [datapath backport 09/10] datapath: openvswitch: Optimize sample action for the clone use cases

[Andy Zhou]

Upstream commit:
openvswitch: Optimize sample action for the clone use cases

With the introduction of open flow 'clone' action, the OVS user space
can now translate the 'clone' action into kernel datapath 'sample'
action, with 100% probability, to ensure that the clone semantics,
which is that the packet seen by the clone action is the same as the
packet seen by the action after clone, is faithfully carried out
in the datapath.

While the sample action in the datpath has the matching semantics,
its implementation is only optimized for its original use.
Specifically, there are two limitation: First, there is a 3 level of
nesting restriction, enforced at the flow downloading time. This
limit turns out to be too restrictive for the 'clone' use case.
Second, the implementation avoid recursive call only if the sample
action list has a single userspace action.

The main optimization implemented in this series removes the static
nesting limit check, instead, implement the run time recursion limit
check, and recursion avoidance similar to that of the 'recirc' action.
This optimization solve both #1 and #2 issues above.

One related optimization attempts to avoid copying flow key as
long as the actions enclosed does not change the flow key. The
detection is performed only once at the flow downloading time.

Another related optimization is to rewrite the action list
at flow downloading time in order to save the fast path from parsing
the sample action list in its original form repeatedly.

Signed-off-by: Andy Zhou 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Upstream: 798c166173ff ("openvswitch: Optimize sample action for the clone use 
cases")
Signed-off-by: Andy Zhou 
---
 datapath/actions.c| 112 +
 datapath/datapath.h   |   2 -
 datapath/flow_netlink.c   | 141 +++---
 datapath/linux/compat/include/linux/openvswitch.h |  15 +++
 4 files changed, 172 insertions(+), 98 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index f300307b422b..32c0c10e7c62 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -43,6 +43,11 @@
 #include "gso.h"
 #include "vport.h"
 
+/* U32_MAX was introduced in include/linux/kernel.h after version 3.14. */
+#ifndef U32_MAX
+#define U32_MAX((u32)~0U)
+#endif
+
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
  struct sw_flow_key *key,
  const struct nlattr *attr, int len);
@@ -912,73 +917,70 @@ static int output_userspace(struct datapath *dp, struct 
sk_buff *skb,
return err;
 }
 
+/* When 'last' is true, sample() should always consume the 'skb'.
+ * Otherwise, sample() should keep 'skb' intact regardless what
+ * actions are executed within sample().
+ */
 static int sample(struct datapath *dp, struct sk_buff *skb,
  struct sw_flow_key *key, const struct nlattr *attr,
- const struct nlattr *actions, int actions_len)
+ bool last)
 {
-   const struct nlattr *acts_list = NULL;
-   const struct nlattr *a;
-   int rem;
-   u32 cutlen = 0;
-
-   for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
-a = nla_next(a, )) {
-   u32 probability;
+   struct nlattr *actions;
+   struct nlattr *sample_arg;
+   struct sw_flow_key *orig_key = key;
+   int rem = nla_len(attr);
+   int err = 0;
+   const struct sample_arg *arg;
 
-   switch (nla_type(a)) {
-   case OVS_SAMPLE_ATTR_PROBABILITY:
-   probability = nla_get_u32(a);
-   if (!probability || prandom_u32() > probability)
-   return 0;
-   break;
+   /* The first action is always 'OVS_SAMPLE_ATTR_ARG'. */
+   sample_arg = nla_data(attr);
+   arg = nla_data(sample_arg);
+   actions = nla_next(sample_arg, );
 
-   case OVS_SAMPLE_ATTR_ACTIONS:
-   acts_list = a;
-   break;
-   }
+   if ((arg->probability != U32_MAX) &&
+   (!arg->probability || prandom_u32() > arg->probability)) {
+   if (last)
+   consume_skb(skb);
+   return 0;
}
 
-   rem = nla_len(acts_list);
-   a = nla_data(acts_list);
-
-   /* Actions list is empty, do nothing */
-   if (unlikely(!rem))
+   /* Unless the last action, sample works on the clone of SKB.  */
+   skb = last ? skb : skb_clone(skb, GFP_ATOMIC);
+   if (!skb) {
+   /* Out of memory, skip this sample action.
+*/
return 0;
+   }
 
-   /* The 

[ovs-dev] [datapath backport 08/10] datapath: openvswitch: Refactor recirc key allocation.

[Andy Zhou]

Upstream commit:
openvswitch: Refactor recirc key allocation.

The logic of allocating and copy key for each 'exec_actions_level'
was specific to execute_recirc(). However, future patches will reuse
as well.  Refactor the logic into its own function clone_key().

Signed-off-by: Andy Zhou 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Upstream: 4572ef52a00b ("openvswitch: Refactor recirc key allocation.")
Signed-off-by: Andy Zhou 
---
 datapath/actions.c | 66 +-
 1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 964d398655f8..f300307b422b 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007-2015 Nicira, Inc.
+ * Copyright (c) 2007-2017 Nicira, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of version 2 of the GNU General Public
@@ -81,14 +81,31 @@ struct action_fifo {
struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
 };
 
-struct recirc_keys {
+struct action_flow_keys {
struct sw_flow_key key[OVS_DEFERRED_ACTION_THRESHOLD];
 };
 
 static struct action_fifo __percpu *action_fifos;
-static struct recirc_keys __percpu *recirc_keys;
+static struct action_flow_keys __percpu *flow_keys;
 static DEFINE_PER_CPU(int, exec_actions_level);
 
+/* Make a clone of the 'key', using the pre-allocated percpu 'flow_keys'
+ * space. Return NULL if out of key spaces.
+ */
+static struct sw_flow_key *clone_key(const struct sw_flow_key *key_)
+{
+   struct action_flow_keys *keys = this_cpu_ptr(flow_keys);
+   int level = this_cpu_read(exec_actions_level);
+   struct sw_flow_key *key = NULL;
+
+   if (level <= OVS_DEFERRED_ACTION_THRESHOLD) {
+   key = >key[level - 1];
+   *key = *key_;
+   }
+
+   return key;
+}
+
 static void action_fifo_init(struct action_fifo *fifo)
 {
fifo->head = 0;
@@ -1074,8 +1091,8 @@ static int execute_recirc(struct datapath *dp, struct 
sk_buff *skb,
  struct sw_flow_key *key,
  const struct nlattr *a, int rem)
 {
+   struct sw_flow_key *recirc_key;
struct deferred_action *da;
-   int level;
 
if (!is_flow_key_valid(key)) {
int err;
@@ -1099,29 +1116,26 @@ static int execute_recirc(struct datapath *dp, struct 
sk_buff *skb,
return 0;
}
 
-   level = this_cpu_read(exec_actions_level);
-   if (level <= OVS_DEFERRED_ACTION_THRESHOLD) {
-   struct recirc_keys *rks = this_cpu_ptr(recirc_keys);
-   struct sw_flow_key *recirc_key = >key[level - 1];
-
-   *recirc_key = *key;
+   /* If within the limit of 'OVS_DEFERRED_ACTION_THRESHOLD',
+* recirc immediately, otherwise, defer it for later execution.
+*/
+   recirc_key = clone_key(key);
+   if (recirc_key) {
recirc_key->recirc_id = nla_get_u32(a);
ovs_dp_process_packet(skb, recirc_key);
-
-   return 0;
-   }
-
-   da = add_deferred_actions(skb, key, NULL, 0);
-   if (da) {
-   da->pkt_key.recirc_id = nla_get_u32(a);
} else {
-   kfree_skb(skb);
-
-   if (net_ratelimit())
-   pr_warn("%s: deferred action limit reached, drop recirc 
action\n",
-   ovs_dp_name(dp));
+   da = add_deferred_actions(skb, key, NULL, 0);
+   if (da) {
+   recirc_key = >pkt_key;
+   recirc_key->recirc_id = nla_get_u32(a);
+   } else {
+   /* Log an error in case action fifo is full.  */
+   kfree_skb(skb);
+   if (net_ratelimit())
+   pr_warn("%s: deferred action limit reached, 
drop recirc action\n",
+   ovs_dp_name(dp));
+   }
}
-
return 0;
 }
 
@@ -1311,8 +1325,8 @@ int action_fifos_init(void)
if (!action_fifos)
return -ENOMEM;
 
-   recirc_keys = alloc_percpu(struct recirc_keys);
-   if (!recirc_keys) {
+   flow_keys = alloc_percpu(struct action_flow_keys);
+   if (!flow_keys) {
free_percpu(action_fifos);
return -ENOMEM;
}
@@ -1323,5 +1337,5 @@ int action_fifos_init(void)
 void action_fifos_exit(void)
 {
free_percpu(action_fifos);
-   free_percpu(recirc_keys);
+   free_percpu(flow_keys);
 }
-- 
1.8.3.1

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

[ovs-dev] [datapath backport 06/10] datapath: openvswitch: Add missing case OVS_TUNNEL_KEY_ATTR_PAD

[Andy Zhou]

From: Kris Murphy 

openvswitch: Add missing case OVS_TUNNEL_KEY_ATTR_PAD

Added a case for OVS_TUNNEL_KEY_ATTR_PAD to the switch statement
in ip_tun_from_nlattr in order to prevent the default case
returning an error.

Fixes: b46f6ded906e ("libnl: nla_put_be64(): align on a 64-bit area")
Signed-off-by: Kris Murphy 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Upstream: 8f3dbfd79ed9("openvswitch: Add missing case OVS_TUNNEL_KEY_ATTR_PAD")
Signed-off-by: Andy Zhou 
---
 AUTHORS.rst | 1 +
 datapath/flow_netlink.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index c8adeadccd6f..77deb2ebbabd 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -179,6 +179,7 @@ Kevin Loke...@freebsd.org
 Kevin Traynor   kevin.tray...@intel.com
 Khem Rajraj.k...@gmail.com
 Kmindg Gkmi...@gmail.com
+Kris Murphy krisk...@linux.vnet.ibm.com
 Krishna Kondaka kkond...@vmware.com
 Kyle Mesterymest...@mestery.com
 Kyle Upton  kup...@baymicrosystems.com
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index e31852ebb421..704e2da5f7a6 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -667,6 +667,8 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
tun_flags |= TUNNEL_VXLAN_OPT;
opts_type = type;
break;
+   case OVS_TUNNEL_KEY_ATTR_PAD:
+   break;
default:
OVS_NLERR(log, "Unknown IP tunnel attribute %d",
  type);
-- 
1.8.3.1

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

[ovs-dev] [datapath backport 07/10] datapath: openvswitch: Deferred fifo API change.

[Andy Zhou]

Upstream commit:
openvswitch: Deferred fifo API change.

add_deferred_actions() API currently requires actions to be passed in
as a fully encoded netlink message. So far both 'sample' and 'recirc'
actions happens to carry actions as fully encoded netlink messages.
However, this requirement is more restrictive than necessary, future
patch will need to pass in action lists that are not fully encoded
by themselves.

Signed-off-by: Andy Zhou 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Upstream: 47c697aa2d07 ("openvswitch: Deferred fifo API change.")
Signed-off-by: Andy Zhou 
---
 datapath/actions.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index b2a0ae537e37..964d398655f8 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -50,6 +50,7 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
 struct deferred_action {
struct sk_buff *skb;
const struct nlattr *actions;
+   int actions_len;
 
/* Store pkt_key clone when creating deferred action. */
struct sw_flow_key pkt_key;
@@ -117,8 +118,9 @@ static struct deferred_action *action_fifo_put(struct 
action_fifo *fifo)
 
 /* Return queue entry if fifo is not full */
 static struct deferred_action *add_deferred_actions(struct sk_buff *skb,
-   const struct sw_flow_key 
*key,
-   const struct nlattr *attr)
+   const struct sw_flow_key *key,
+   const struct nlattr *actions,
+   const int actions_len)
 {
struct action_fifo *fifo;
struct deferred_action *da;
@@ -127,7 +129,8 @@ static struct deferred_action *add_deferred_actions(struct 
sk_buff *skb,
da = action_fifo_put(fifo);
if (da) {
da->skb = skb;
-   da->actions = attr;
+   da->actions = actions;
+   da->actions_len = actions_len;
da->pkt_key = *key;
}
 
@@ -950,7 +953,8 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
/* Skip the sample action when out of memory. */
return 0;
 
-   if (!add_deferred_actions(skb, key, a)) {
+   if (!add_deferred_actions(skb, key, nla_data(acts_list),
+ nla_len(acts_list))) {
if (net_ratelimit())
pr_warn("%s: deferred actions limit reached, dropping 
sample action\n",
ovs_dp_name(dp));
@@ -1107,7 +,7 @@ static int execute_recirc(struct datapath *dp, struct 
sk_buff *skb,
return 0;
}
 
-   da = add_deferred_actions(skb, key, NULL);
+   da = add_deferred_actions(skb, key, NULL, 0);
if (da) {
da->pkt_key.recirc_id = nla_get_u32(a);
} else {
@@ -1262,10 +1266,10 @@ static void process_deferred_actions(struct datapath 
*dp)
struct sk_buff *skb = da->skb;
struct sw_flow_key *key = >pkt_key;
const struct nlattr *actions = da->actions;
+   int actions_len = da->actions_len;
 
if (actions)
-   do_execute_actions(dp, skb, key, actions,
-  nla_len(actions));
+   do_execute_actions(dp, skb, key, actions, actions_len);
else
ovs_dp_process_packet(skb, key);
} while (!action_fifo_is_empty(fifo));
-- 
1.8.3.1

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

[ovs-dev] [datapath backport 05/10] datapath: net/openvswitch: Set the ipv6 source tunnel key address attribute correctly

[Andy Zhou]

From: Or Gerlitz 

Upstream commit:
net/openvswitch: Set the ipv6 source tunnel key address attribute correctly

When dealing with ipv6 source tunnel key address attribute
(OVS_TUNNEL_KEY_ATTR_IPV6_SRC) we are wrongly setting the tunnel
dst ip, fix that.

Fixes: 6b26ba3a7d95 ('openvswitch: netlink attributes for IPv6 tunneling')
Signed-off-by: Or Gerlitz 
Reported-by: Paul Blakey 
Acked-by: Jiri Benc 
Acked-by: Joe Stringer 
Signed-off-by: David S. Miller 

Upstream: 3d20f1f7bd575 ("net/openvswitch: Set the ipv6 source tunnel key 
address attribute correctly")

Signed-off-by: Andy Zhou 
---
 AUTHORS.rst | 1 +
 datapath/flow_netlink.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 5baee5839a2c..c8adeadccd6f 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -225,6 +225,7 @@ Nithin Raju nit...@vmware.com
 Niti Rohillaniti.rohi...@tcs.com
 Numan Siddique  nusid...@redhat.com
 Ofer Ben-Yacov  ofer.benya...@gmail.com
+Or Gerlitz  ogerl...@mellanox.com
 Ori Shoshan ori.shos...@guardicore.com
 Padmanabhan Krishnankpr...@yahoo.com
 Panu Matilainen pmati...@redhat.com
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index e2b17fb935ab..e31852ebb421 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -606,7 +606,7 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
ipv4 = true;
break;
case OVS_TUNNEL_KEY_ATTR_IPV6_SRC:
-   SW_FLOW_KEY_PUT(match, tun_key.u.ipv6.dst,
+   SW_FLOW_KEY_PUT(match, tun_key.u.ipv6.src,
nla_get_in6_addr(a), is_mask);
ipv6 = true;
break;
-- 
1.8.3.1

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

[ovs-dev] [datapath backport 04/10] datapath: actions: fixed a brace coding style warning.

[Andy Zhou]

From: Peter Downs 

Upstream commit:
openvswitch: actions: fixed a brace coding style warning

Fixed a brace coding style warning reported by checkpatch.pl

Signed-off-by: Peter Downs 
Signed-off-by: David S. Miller 

Upstream: f1304f7ba398 ("openvswitch: actions: fixed a brace coding style 
warning")
Signed-off-by: Joe Stringer 
---
 AUTHORS.rst| 1 +
 datapath/actions.c | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index cb3b9b0710b0..5baee5839a2c 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -234,6 +234,7 @@ Paul Fazzonepfazz...@nicira.com
 Paul Ingram p...@nicira.com
 Paul-Emmanuel Raoul sky...@skyplabs.net
 Pavithra Ramesh param...@vmware.com
+Peter Downs pado...@gmail.com
 Philippe Jung   phil.j...@free.fr
 Pim van den Bergp...@nethuis.nl
 pritesh pritesh.koth...@cisco.com
diff --git a/datapath/actions.c b/datapath/actions.c
index 06080b24ea5a..b2a0ae537e37 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -775,9 +775,8 @@ static void ovs_fragment(struct net *net, struct vport 
*vport,
unsigned long orig_dst;
struct rt6_info ovs_rt;
 
-   if (!v6ops) {
+   if (!v6ops)
goto err;
-   }
 
prepare_frag(vport, skb,
 ovs_key_mac_proto(key));
-- 
1.8.3.1

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

[ovs-dev] [datapath backport 03/10] compat: ipv6: orphan skbs in reassembly unit.

[Andy Zhou]

From: Eric Dumazet 

Upstream commit:
ipv6: orphan skbs in reassembly unit

Andrey reported a use-after-free in IPv6 stack.

Issue here is that we free the socket while it still has skb
in TX path and in some queues.

It happens here because IPv6 reassembly unit messes skb->truesize,
breaking skb_set_owner_w() badly.

We fixed a similar issue for IPV4 in commit 8282f27449bf ("inet: frag:
Always orphan skbs inside ip_defrag()")
Acked-by: Joe Stringer 

==
BUG: KASAN: use-after-free in sock_wfree+0x118/0x120
Read of size 8 at addr 880062da0060 by task a.out/4140

page:ea00018b6800 count:1 mapcount:0 mapping:  (null)
index:0x0 compound_mapcount: 0
flags: 0x1008100(slab|head)
raw: 01008100   000180130013
raw: dead0100 dead0200 88006741f140 
page dumped because: kasan: bad access detected

CPU: 0 PID: 4140 Comm: a.out Not tainted 4.10.0-rc3+ #59
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:15
 dump_stack+0x292/0x398 lib/dump_stack.c:51
 describe_address mm/kasan/report.c:262
 kasan_report_error+0x121/0x560 mm/kasan/report.c:370
 kasan_report mm/kasan/report.c:392
 __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:413
 sock_flag ./arch/x86/include/asm/bitops.h:324
 sock_wfree+0x118/0x120 net/core/sock.c:1631
 skb_release_head_state+0xfc/0x250 net/core/skbuff.c:655
 skb_release_all+0x15/0x60 net/core/skbuff.c:668
 __kfree_skb+0x15/0x20 net/core/skbuff.c:684
 kfree_skb+0x16e/0x4e0 net/core/skbuff.c:705
 inet_frag_destroy+0x121/0x290 net/ipv4/inet_fragment.c:304
 inet_frag_put ./include/net/inet_frag.h:133
 nf_ct_frag6_gather+0x1125/0x38b0 
net/ipv6/netfilter/nf_conntrack_reasm.c:617
 ipv6_defrag+0x21b/0x350 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68
 nf_hook_entry_hookfn ./include/linux/netfilter.h:102
 nf_hook_slow+0xc3/0x290 net/netfilter/core.c:310
 nf_hook ./include/linux/netfilter.h:212
 __ip6_local_out+0x52c/0xaf0 net/ipv6/output_core.c:160
 ip6_local_out+0x2d/0x170 net/ipv6/output_core.c:170
 ip6_send_skb+0xa1/0x340 net/ipv6/ip6_output.c:1722
 ip6_push_pending_frames+0xb3/0xe0 net/ipv6/ip6_output.c:1742
 rawv6_push_pending_frames net/ipv6/raw.c:613
 rawv6_sendmsg+0x2cff/0x4130 net/ipv6/raw.c:927
 inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
 sock_sendmsg_nosec net/socket.c:635
 sock_sendmsg+0xca/0x110 net/socket.c:645
 sock_write_iter+0x326/0x620 net/socket.c:848
 new_sync_write fs/read_write.c:499
 __vfs_write+0x483/0x760 fs/read_write.c:512
 vfs_write+0x187/0x530 fs/read_write.c:560
 SYSC_write fs/read_write.c:607
 SyS_write+0xfb/0x230 fs/read_write.c:599
 entry_SYSCALL_64_fastpath+0x1f/0xc2 arch/x86/entry/entry_64.S:203
RIP: 0033:0x7ff26e6f5b79
RSP: 002b:7ff268e0ed98 EFLAGS: 0206 ORIG_RAX: 0001
RAX: ffda RBX: 7ff268e0f9c0 RCX: 7ff26e6f5b79
RDX: 0010 RSI: 20f50fe1 RDI: 0003
RBP: 7ff26ebc1220 R08:  R09: 
R10:  R11: 0206 R12: 
R13: 7ff268e0f9c0 R14: 7ff26efec040 R15: 0003

The buggy address belongs to the object at 880062da
 which belongs to the cache RAWv6 of size 1504
The buggy address 880062da0060 is located 96 bytes inside
 of 1504-byte region [880062da, 880062da05e0)

Freed by task 4113:
 save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
 save_stack+0x43/0xd0 mm/kasan/kasan.c:502
 set_track mm/kasan/kasan.c:514
 kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:578
 slab_free_hook mm/slub.c:1352
 slab_free_freelist_hook mm/slub.c:1374
 slab_free mm/slub.c:2951
 kmem_cache_free+0xb2/0x2c0 mm/slub.c:2973
 sk_prot_free net/core/sock.c:1377
 __sk_destruct+0x49c/0x6e0 net/core/sock.c:1452
 sk_destruct+0x47/0x80 net/core/sock.c:1460
 __sk_free+0x57/0x230 net/core/sock.c:1468
 sk_free+0x23/0x30 net/core/sock.c:1479
 sock_put ./include/net/sock.h:1638
 sk_common_release+0x31e/0x4e0 net/core/sock.c:2782
 rawv6_close+0x54/0x80 net/ipv6/raw.c:1214
 inet_release+0xed/0x1c0 net/ipv4/af_inet.c:425
 inet6_release+0x50/0x70 net/ipv6/af_inet6.c:431
 sock_release+0x8d/0x1e0 net/socket.c:599
 sock_close+0x16/0x20 net/socket.c:1063
 __fput+0x332/0x7f0 fs/file_table.c:208
 fput+0x15/0x20 fs/file_table.c:244
 task_work_run+0x19b/0x270 kernel/task_work.c:116
 exit_task_work ./include/linux/task_work.h:21
 do_exit+0x186b/0x2800 kernel/exit.c:839
 do_group_exit+0x149/0x420 

[ovs-dev] [datapath backport 02/10] datapath: Pack struct sw_flow_key.

[Andy Zhou]

From: Jarno Rajahalme 

Upstream commit:
openvswitch: Pack struct sw_flow_key.

struct sw_flow_key has two 16-bit holes. Move the most matched
conntrack match fields there.  In some typical cases this reduces the
size of the key that needs to be hashed into half and into one cache
line.

Signed-off-by: Jarno Rajahalme 
Acked-by: Joe Stringer 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Upstream: 316d4d78cf9b ("openvswitch: Pack struct sw_flow_key.")
Signed-off-by: Joe Stringer 
---
 datapath/conntrack.c| 40 
 datapath/conntrack.h|  8 
 datapath/flow.h | 14 --
 datapath/flow_netlink.c | 11 +++
 4 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 413095955b1b..a47525355534 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -162,7 +162,7 @@ static void __ovs_ct_update_key_orig_tp(struct sw_flow_key 
*key,
const struct nf_conntrack_tuple *orig,
u8 icmp_proto)
 {
-   key->ct.orig_proto = orig->dst.protonum;
+   key->ct_orig_proto = orig->dst.protonum;
if (orig->dst.protonum == icmp_proto) {
key->ct.orig_tp.src = htons(orig->dst.u.icmp.type);
key->ct.orig_tp.dst = htons(orig->dst.u.icmp.code);
@@ -176,8 +176,8 @@ static void __ovs_ct_update_key(struct sw_flow_key *key, u8 
state,
const struct nf_conntrack_zone *zone,
const struct nf_conn *ct)
 {
-   key->ct.state = state;
-   key->ct.zone = zone->id;
+   key->ct_state = state;
+   key->ct_zone = zone->id;
key->ct.mark = ovs_ct_get_mark(ct);
ovs_ct_get_labels(ct, >ct.labels);
 
@@ -205,10 +205,10 @@ static void __ovs_ct_update_key(struct sw_flow_key *key, 
u8 state,
return;
}
}
-   /* Clear 'ct.orig_proto' to mark the non-existence of conntrack
+   /* Clear 'ct_orig_proto' to mark the non-existence of conntrack
 * original direction key fields.
 */
-   key->ct.orig_proto = 0;
+   key->ct_orig_proto = 0;
 }
 
 /* Update 'key' based on skb->_nfct.  If 'post_ct' is true, then OVS has
@@ -238,7 +238,7 @@ static void ovs_ct_update_key(const struct sk_buff *skb,
if (ct->master)
state |= OVS_CS_F_RELATED;
if (keep_nat_flags) {
-   state |= key->ct.state & OVS_CS_F_NAT_MASK;
+   state |= key->ct_state & OVS_CS_F_NAT_MASK;
} else {
if (ct->status & IPS_SRC_NAT)
state |= OVS_CS_F_SRC_NAT;
@@ -269,11 +269,11 @@ void ovs_ct_fill_key(const struct sk_buff *skb, struct 
sw_flow_key *key)
 int ovs_ct_put_key(const struct sw_flow_key *swkey,
   const struct sw_flow_key *output, struct sk_buff *skb)
 {
-   if (nla_put_u32(skb, OVS_KEY_ATTR_CT_STATE, output->ct.state))
+   if (nla_put_u32(skb, OVS_KEY_ATTR_CT_STATE, output->ct_state))
return -EMSGSIZE;
 
if (IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES) &&
-   nla_put_u16(skb, OVS_KEY_ATTR_CT_ZONE, output->ct.zone))
+   nla_put_u16(skb, OVS_KEY_ATTR_CT_ZONE, output->ct_zone))
return -EMSGSIZE;
 
if (IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) &&
@@ -285,14 +285,14 @@ int ovs_ct_put_key(const struct sw_flow_key *swkey,
>ct.labels))
return -EMSGSIZE;
 
-   if (swkey->ct.orig_proto) {
+   if (swkey->ct_orig_proto) {
if (swkey->eth.type == htons(ETH_P_IP)) {
struct ovs_key_ct_tuple_ipv4 orig = {
output->ipv4.ct_orig.src,
output->ipv4.ct_orig.dst,
output->ct.orig_tp.src,
output->ct.orig_tp.dst,
-   output->ct.orig_proto,
+   output->ct_orig_proto,
};
if (nla_put(skb, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4,
sizeof(orig), ))
@@ -303,7 +303,7 @@ int ovs_ct_put_key(const struct sw_flow_key *swkey,
IN6_ADDR_INITIALIZER(output->ipv6.ct_orig.dst),
output->ct.orig_tp.src,
output->ct.orig_tp.dst,
-   output->ct.orig_proto,
+   output->ct_orig_proto,
};
if (nla_put(skb, OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6,
sizeof(orig), ))
@@ -649,11 +649,11 

[ovs-dev] [datapath backport 01/10] dapatah: Fix missing CONFIG_NF_CONNTRACK_LABLES check

[Andy Zhou]

This config flag was not consistently checked. Fix it.

Signed-off-by: Andy Zhou 
---
 datapath/conntrack.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 4df73528bf69..413095955b1b 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -139,20 +139,23 @@ static u32 ovs_ct_get_mark(const struct nf_conn *ct)
 #endif
 }
 
+static void ovs_ct_get_labels(const struct nf_conn *ct,
+ struct ovs_key_ct_labels *labels)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS)
 /* Guard against conntrack labels max size shrinking below 128 bits. */
 #if NF_CT_LABELS_MAX_SIZE < 16
 #error NF_CT_LABELS_MAX_SIZE must be at least 16 bytes
 #endif
-
-static void ovs_ct_get_labels(const struct nf_conn *ct,
- struct ovs_key_ct_labels *labels)
-{
struct nf_conn_labels *cl = ct ? nf_ct_labels_find(ct) : NULL;
 
-   if (cl)
+   if (cl) {
memcpy(labels, cl->bits, OVS_CT_LABELS_LEN);
-   else
-   memset(labels, 0, OVS_CT_LABELS_LEN);
+   return;
+   }
+#endif /* IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS). */
+
+   memset(labels, 0, OVS_CT_LABELS_LEN);
 }
 
 static void __ovs_ct_update_key_orig_tp(struct sw_flow_key *key,
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH v2] ovn-controller: enable ssl config via local ovsdb

[Ben Pfaff]

On Thu, Apr 06, 2017 at 12:49:13PM -0400, Lance Richardson wrote:
> Allow ovn-controller to use SSL certificate and key configuration
> from local ovsdb. With this change, SSL configuration from the
> vswitchd database will be used if present, otherwise configuration
> can still be specified from the command line.
> 
> If SSL configuration is present in both locations, the configuration
> in the local ovsdb has precedence. This is consistent with how
> vswitchd is currently implemented.
> 
> The existing ovs-vsctl get-ssl/set-ssl/del-ssl commands can be used
> to manage the configuration in the vswitchd database.
> 
> Signed-off-by: Lance Richardson 
> ---
> v2: Add NEWS item, update ovn-controller(8) man page.

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

Re: [ovs-dev] [PATCH v6 1/5] datapath-windows: Added a new file to support Ipv4 fragments.

[Alin Serdean]

> +
> +/*
> +*--
> +--
> +* OvsIpv4Reassemble
> +* Reassemble the ipv4 fragments and return newNbl on success.
> +* Should be called after acquiring the lockObj for the entry.
> +*--
> +--
> +*/
> +NDIS_STATUS
[Alin Serdean] Another small nit pick:
Please change comments from form:
/*
*
*/
To 
/*
 *
 */

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

Re: [ovs-dev] [PATCH] route-table: Fix compatibility with pre-2.6.36 headers.

[Ben Pfaff]

On Thu, Apr 06, 2017 at 11:39:54AM -0700, Andy Zhou wrote:
> On Fri, Mar 17, 2017 at 2:21 PM, Ben Pfaff  wrote:
> > Reported-by: Timothy Redaelli 
> > Reported-at: 
> > https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329604.html
> > Signed-off-by: Ben Pfaff 
>  Acked-by: Andy Zhou 

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

Re: [ovs-dev] [PATCH 2/4] check-structs: struct eth_addr has alignment 2, not 1.

[Ben Pfaff]

On Thu, Apr 06, 2017 at 11:35:44AM -0700, Andy Zhou wrote:
> On Wed, Apr 5, 2017 at 11:11 PM, Ben Pfaff  wrote:
> > It consists of ovs_be16 elements, so it has 16-bit alignment.
> >
> > (This doesn't make a difference for any actual OpenFlow protocol elements.)
> >
> > Signed-off-by: Ben Pfaff 
> Acked-by: Andy Zhou 

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

Re: [ovs-dev] [PATCH v6 1/5] datapath-windows: Added a new file to support Ipv4 fragments.

[Alin Serdean]

Hi Anand,

Thanks a lot for the patch.

At first glance the code looks ok. Just a small nit pick, can you please use a 
new memory pool tag instead of using `OVS_MEMORY_TAG`.

Please define a new member after 
https://github.com/openvswitch/ovs/blob/master/datapath-windows/ovsext/Util.h#L41

This will help us debugging either using a windows specific debugger to look at 
that specific pool or use tools (i.e. 
https://msdn.microsoft.com/en-us/windows/hardware/drivers/devtest/poolmon)

Thanks,
Alin.

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Anand Kumar
> Sent: Friday, March 24, 2017 10:51 PM
> To: d...@openvswitch.org
> Subject: [ovs-dev] [PATCH v6 1/5] datapath-windows: Added a new file to
> support Ipv4 fragments.
> 
> This patch adds functionalities to support IPv4 fragments, which will be used
> by Conntrack module.
> 
> Added a new structure to hold the Ipv4 fragments and a hash table to hold
> Ipv4 datagram entries. Also added a clean up thread that runs every minute
> to delete the expired IPv4 datagram entries.
> 
> The individual fragments are ignored by the conntrack. Once all the
> fragments are recieved, a new NBL is created out of the reassembled
> fragments and conntrack executes actions on the new NBL.
> 
> Created new APIs OvsProcessIpv4Fragment() to process individual
> fragments,
> OvsIpv4Reassemble() to reassemble Ipv4 fragments.
> 
> Signed-off-by: Anand Kumar 
> ---
> v5->v6: No Change
> v4->v5:
>   - Modified Patch 3 to retain MRU in _OVS_BUFFER_CONTEXT instead of
>   using it in ovsForwardingContext with minor changes in rest of the
>   patches.
> v3->v4:
>   - Rebase
>   - Acquire read lock for read operations.
> v2->v3:
>   - using spinlock instead of RW lock.
>   - updated log messages, summary, fixed alignment issues.
> v1->v2:
>   - Patch 4 updated to make it compile for release mode.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH RFC] dpif-netdev: Add Cuckoo Distributor to Accelerate Megaflow Search

[yipeng1 . wang]

From: Yipeng Wang 

The Datapath Classifier uses tuple space search for flow classification.
The rules are arranged into a set of tuples/subtables (each with a
distinct mask).  Each subtable is implemented as a hash table and lookup
is done with flow keys formed by selecting the bits from the packet header
based on each subtable's mask. Tuple space search will sequentially search
each subtable until a match is found. With a large number of subtables, a
sequential search of the subtables could consume a lot of CPU cycles. In
a testbench with a uniform traffic pattern equally distributed across 20
subtables, we measured that up to 65% of total execution time is attributed
to the megaflow cache lookup.

This patch presents the idea of the two-layer hierarchical lookup, where a
low overhead first level of indirection is accessed first, we call this
level cuckoo distributor (CD). If a flow key has been inserted in the flow
table the first level will indicate with high probability that which
subtable to look into. A lookup is performed on the second level (the
target subtable) to retrieve the result. If the key doesn’t have a match,
then we revert back to the sequential search of subtables.

This patch can improve the already existing Subtable Ranking when traffic
data has high entropy. Subtable Ranking helps minimize the number of
traversed subtables when most of the traffic hit the same subtable.
However, in the case of high entropy traffic such as traffic coming from
a physical port, multiple subtables could be hit with a similar frequency.
In this case the average subtable lookups per hit would be much greater
than 1. In addition, CD can adaptively turn off when it finds the traffic
mostly hit one subtable. Thus, CD will not be an overhead when Subtable
Ranking works well.

Scheme:

 ---
|  CD   |
 ---
   \
\
 -  - -
|sub  ||sub  |...|sub  |
|table||table|   |table|
 -  - -

Evaluation:

We create set of rules with various src IP. We feed traffic containing 1
million flows with various src IP and dst IP. All the flows hit 10/20/30
rules creating 10/20/30 subtables.

The table below shows the preliminary continuous testing results (full line
speed test) we collected with a uni-directional port-to-port setup. The
machine we tested on is a Xeon E5 server running with 2.2GHz cores. OvS
runs with 1 PMD. We use Spirent as the hardware traffic generator.

no.subtable: 10  20  30
cd-ovs   3895961 3170530 2968555
orig-ovs 2683455 1646227 1240501
speedup  1.45x   1.92x   2.39x

Signed-off-by: Yipeng Wang 
Signed-off-by: Charlie Tai 
Co-authored-by: Charlie Tai 
Signed-off-by: Sameh Gobriel 
Co-authored-by: Sameh Gobriel 
Signed-off-by: Ren Wang 
Co-authored-by: Ren Wang 
Signed-off-by: Antonio Fischetti 
Co-authored-by: Antonio Fischetti 
---
 lib/dpif-netdev.c | 654 --
 tests/ofproto-dpif.at |   3 +-
 2 files changed, 633 insertions(+), 24 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a14a2eb..d9a883b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -79,11 +79,23 @@
 
 VLOG_DEFINE_THIS_MODULE(dpif_netdev);
 
+/* Length of Subtable table for cuckoo distributor to index subtables.
+ * The size of the table is at most 256 entires because the CD's entry only
+ * provides 1 byte for indexing.
+ */
+#define SUBTABLE_TABLE_LENGTH 256
+
 #define FLOW_DUMP_MAX_BATCH 50
 /* Use per thread recirc_depth to prevent recirculation loop. */
 #define MAX_RECIRC_DEPTH 5
 DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
 
+
+#define CD_DEBUG 0
+#define debug_print(...) \
+do { if (CD_DEBUG) fprintf(stderr, __VA_ARGS__); } while (0)
+
+
 /* Configuration parameters. */
 enum { MAX_FLOWS = 65536 }; /* Maximum number of flows in flow table. */
 enum { MAX_METERS = 65536 };/* Maximum number of meters. */
@@ -163,6 +175,44 @@ struct emc_cache {
 int sweep_idx;/* For emc_cache_slow_sweep(). */
 };
 
+
+/* Cuckoo distributor (CD) is a 2-hash function hash table.
+ * For now, the design does not allow desplacing items when bucket is full,
+ * which is different from the behavior of a cuckoo hash table.
+ * The advantage is that we do not need to store two sigantures so that
+ * the struct will be more compact. We use 16 entries per bucket for the
+ * usage of AVX.
+ *
+ * Each classifier has its own cuckoo distributor. It is NOT thread-safe
+ */
+#define CD_NUM_BUCKETS (1<<16)
+#define CD_BUCKET_MASK (CD_NUM_BUCKETS-1)
+#define CD_ENTRIES 16
+
+/* These two seeds are used for hashing two bucket locations */
+#define CD_PRIM_BUCKET_SEED 10
+#define CD_SEC_BUCKET_SEED 20
+
+/* This bit is used to choose which bucket to replace CD's entry in cd_insert*/
+#define CD_CHOOSE_SEC_BUCKT_BIT (1 << CD_ENTRIES)
+
+typedef uint16_t simple_sig_store_t;
+
+
+/* The bucket struct for 

[ovs-dev] RE

[Zalar Matija]

I have a proposal for you,  Kindly contact me via:  mrsstacyle...@gmail.com


Pravni pogoji / Legal disclaimer
Telekom Slovenije, d.d., Ljubljana 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] route-table: Fix compatibility with pre-2.6.36 headers.

[Andy Zhou]

On Fri, Mar 17, 2017 at 2:21 PM, Ben Pfaff  wrote:
> Reported-by: Timothy Redaelli 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329604.html
> Signed-off-by: Ben Pfaff 
 Acked-by: Andy Zhou 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/4] check-structs: struct eth_addr has alignment 2, not 1.

[Andy Zhou]

On Wed, Apr 5, 2017 at 11:11 PM, Ben Pfaff  wrote:
> It consists of ovs_be16 elements, so it has 16-bit alignment.
>
> (This doesn't make a difference for any actual OpenFlow protocol elements.)
>
> Signed-off-by: Ben Pfaff 
Acked-by: Andy Zhou 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/4] util: New macro ovs_strlcpy_arrays().

[Andy Zhou]

On Wed, Apr 5, 2017 at 11:11 PM, Ben Pfaff  wrote:
> When both arguments to ovs_strlcpy() are character arrays, it makes sense
> to just pass the smaller of their sizes as the overall size.  It's
> somewhat error-prone and definitely redundant to write that by hand, so
> this commit adds a new macro that does it automatically.
>
> Signed-off-by: Ben Pfaff 

Nice!
Acked-by: Andy Zhou 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] m4: Add hard requirements for python in "configure"

[Timothy M. Redaelli]

On 04/06/2017 12:16 AM, Ben Pfaff wrote:
> On Wed, Mar 29, 2017 at 01:49:42PM +0200, Timothy Redaelli wrote:
>> Since Python 2.7 and python-six are needed to build Open vSwitch,
>> ./configure should return an error if they are missing or if they are too old
>>
>> Signed-off-by: Timothy Redaelli 
> 
> Thanks for the patch.
> 
> I think that if we do it this way, then eventually we'll have to debug
> the problem.  I suggest that we use the following instead, which does a
> better job of dumping problems into config.log for troubleshooting.
> 
> What do you think?

LGTM

Tested cases:
 - missing python
 - python version < 2.7
 - python version >= 2.7, without python-six
 - python version >= 2.7, with old python-six

 - python version >= 2.7, with correct python-six

Thank you

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

Re: [ovs-dev] [PATCH v6 5/5] datapath-windows: Fragment NBL based on MRU size

[Sairam Venugopal]

Acked-by: Sairam Venugopal 





On 3/24/17, 1:51 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand Kumar" 
 wrote:

>This patch adds support for Fragmenting NBL based on the MRU value.
>MRU value is updated only for Ipv4 fragments, if it is non zero, then
>fragment the NBL and send out the new NBL to the vnic.
>
>Signed-off-by: Anand Kumar 
>---
>v5->v6: No Change
>v4->v5:
>   - Use MRU information in the _OVS_BUFFER_CONTEXT to fragment NBL.
>v3->v4: No Change
>v2->v3:
>   - Updated log message
>v1->v2: No change
>---
> datapath-windows/ovsext/Actions.c | 27 +++
> 1 file changed, 27 insertions(+)
>
>diff --git a/datapath-windows/ovsext/Actions.c 
>b/datapath-windows/ovsext/Actions.c
>index cbc7640..a68679c 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -34,6 +34,7 @@
> #include "Vport.h"
> #include "Vxlan.h"
> #include "Geneve.h"
>+#include "IpFragment.h"
> 
> #ifdef OVS_DBG_MOD
> #undef OVS_DBG_MOD
>@@ -864,6 +865,8 @@ OvsOutputForwardingCtx(OvsForwardingContext *ovsFwdCtx)
> NDIS_STATUS status = STATUS_SUCCESS;
> POVS_SWITCH_CONTEXT switchContext = ovsFwdCtx->switchContext;
> PCWSTR dropReason;
>+PNET_BUFFER_LIST fragNbl = NULL;
>+POVS_BUFFER_CONTEXT ctx;
> 
> /*
>  * Handle the case where the some of the destination ports are tunneled
>@@ -909,6 +912,30 @@ OvsOutputForwardingCtx(OvsForwardingContext *ovsFwdCtx)
> goto dropit;
> }
> 
>+ctx = 
>(POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(ovsFwdCtx->curNbl);
>+if (ctx->mru != 0) {
>+/* Fragment nbl based on mru. If it returns NULL then the original
>+ * reassembled NBL is sent out to the VIF which will be dropped if
>+ * the packet size is more than VIF MTU.
>+ */
>+fragNbl = OvsFragmentNBL(ovsFwdCtx->switchContext,
>+ ovsFwdCtx->curNbl,
>+ &(ovsFwdCtx->layers),
>+ ctx->mru, 0, TRUE);
>+if (fragNbl != NULL) {
>+OvsCompleteNBLForwardingCtx(ovsFwdCtx,
>+L"Dropped since fragmenting NBL");
>+status = OvsInitForwardingCtx(ovsFwdCtx,
>+  ovsFwdCtx->switchContext,
>+  fragNbl,
>+  ovsFwdCtx->srcVportNo,
>+  ovsFwdCtx->sendFlags,
>+  
>NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(fragNbl),
>+  ovsFwdCtx->completionList,
>+  >layers, FALSE);
>+}
>+}
>+
> OvsSendNBLIngress(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl,
>   ovsFwdCtx->sendFlags);
> /* End this pipeline by resetting the corresponding context. */
>-- 
>2.9.3.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=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=3sl4LJX8-HSAc_mfofbqDS1UYgla5hIj4zVdVv8Mz5c=CuBYGxQI9b-Q-vjVq6DY8KAqOu1TllZ5TWdmnea67go=
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6 4/5] datapath-windows: Updated OvsTcpSegmentNBL to handle IP fragments.

[Sairam Venugopal]

Acked-by: Sairam Venugopal 





On 3/24/17, 1:51 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand Kumar" 
 wrote:

>With this patch, OvsTcpSegmentNBL not only supports fragmenting NBL
>to TCP segments but also Ipv4 fragments.
>
>To reflect the new changes, renamed function name from OvsTcpSegmentNBL
>to OvsFragmentNBL and created a wrapper for OvsTcpSegmentNBL.
>
>Signed-off-by: Anand Kumar 
>---
>v5->v6: No Change
>v4->v5: Changed a variable mss to fragmentSize.
>v3->v4: No Change
>v2->v3:
>   - Updated log message and function summary
>v1->v2:
>   - Fix compile error for release mode.
>---
> datapath-windows/ovsext/BufferMgmt.c | 194 +--
> datapath-windows/ovsext/BufferMgmt.h |  10 +-
> datapath-windows/ovsext/Geneve.c |   2 +-
> datapath-windows/ovsext/Gre.c|   2 +-
> datapath-windows/ovsext/Stt.c|   2 +-
> datapath-windows/ovsext/User.c   |   2 +-
> datapath-windows/ovsext/Vxlan.c  |   2 +-
> 7 files changed, 152 insertions(+), 62 deletions(-)
>
>diff --git a/datapath-windows/ovsext/BufferMgmt.c 
>b/datapath-windows/ovsext/BufferMgmt.c
>index d99052d..0011c10 100644
>--- a/datapath-windows/ovsext/BufferMgmt.c
>+++ b/datapath-windows/ovsext/BufferMgmt.c
>@@ -1084,6 +1084,31 @@ nblcopy_error:
> return NULL;
> }
> 
>+NDIS_STATUS
>+GetIpHeaderInfo(PNET_BUFFER_LIST curNbl,
>+UINT32 *hdrSize)
>+{
>+CHAR *ethBuf[sizeof(EthHdr)];
>+EthHdr *eth;
>+IPHdr *ipHdr;
>+PNET_BUFFER curNb;
>+
>+curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
>+ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
>+
>+eth = (EthHdr *)NdisGetDataBuffer(curNb, ETH_HEADER_LENGTH,
>+  (PVOID), 1, 0);
>+if (eth == NULL) {
>+return NDIS_STATUS_INVALID_PACKET;
>+}
>+ipHdr = (IPHdr *)((PCHAR)eth + ETH_HEADER_LENGTH);
>+if (ipHdr == NULL) {
>+return NDIS_STATUS_INVALID_PACKET;
>+}
>+*hdrSize = (UINT32)(ETH_HEADER_LENGTH + (ipHdr->ihl * 4));
>+return NDIS_STATUS_SUCCESS;
>+}
>+
> /*
>  * --
>  * GetSegmentHeaderInfo
>@@ -1113,15 +1138,16 @@ GetSegmentHeaderInfo(PNET_BUFFER_LIST nbl,
> 
> /*
>  * --
>- * FixSegmentHeader
>+ * FixPacketHeader
>  *
>- *Fix IP length, IP checksum, TCP sequence number and TCP checksum
>- *in the segment.
>+ *Fix IP length, Offset, IP checksum, TCP sequence number and TCP checksum
>+ *in the netbuffer if applicable.
>  * --
>  */
> static NDIS_STATUS
>-FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 seqNumber,
>- BOOLEAN lastPacket, UINT16 packetCounter)
>+FixPacketHeader(PNET_BUFFER nb, UINT16 segmentSize, UINT32 seqNumber,
>+BOOLEAN lastPacket, UINT16 packetCounter, UINT16 offset,
>+BOOLEAN isFragment)
> {
> EthHdr *dstEth = NULL;
> TCPHdr *dstTCP = NULL;
>@@ -1140,41 +1166,55 @@ FixSegmentHeader(PNET_BUFFER nb, UINT16 segmentSize, 
>UINT32 seqNumber,
> case ETH_TYPE_IPV4_NBO:
> {
> IPHdr *dstIP = NULL;
>-
>-ASSERT((INT)MmGetMdlByteCount(mdl) - NET_BUFFER_CURRENT_MDL_OFFSET(nb)
>+if (!isFragment) {
>+ASSERT((INT)MmGetMdlByteCount(mdl) - 
>NET_BUFFER_CURRENT_MDL_OFFSET(nb)
> >= sizeof(EthHdr) + sizeof(IPHdr) + sizeof(TCPHdr));
>-dstIP = (IPHdr *)((PCHAR)dstEth + sizeof(*dstEth));
>-dstTCP = (TCPHdr *)((PCHAR)dstIP + dstIP->ihl * 4);
>-ASSERT((INT)MmGetMdlByteCount(mdl) - NET_BUFFER_CURRENT_MDL_OFFSET(nb)
>+dstIP = (IPHdr *)((PCHAR)dstEth + sizeof(*dstEth));
>+dstTCP = (TCPHdr *)((PCHAR)dstIP + dstIP->ihl * 4);
>+ASSERT((INT)MmGetMdlByteCount(mdl) - 
>NET_BUFFER_CURRENT_MDL_OFFSET(nb)
> >= sizeof(EthHdr) + dstIP->ihl * 4 + TCP_HDR_LEN(dstTCP));
> 
>-/* Fix IP length and checksum */
>-ASSERT(dstIP->protocol == IPPROTO_TCP);
>-dstIP->tot_len = htons(segmentSize + dstIP->ihl * 4 + 
>TCP_HDR_LEN(dstTCP));
>-dstIP->id += packetCounter;
>+/* Fix IP length and checksum */
>+ASSERT(dstIP->protocol == IPPROTO_TCP);
>+dstIP->tot_len = htons(segmentSize + dstIP->ihl * 4 + 
>TCP_HDR_LEN(dstTCP));
>+dstIP->id += packetCounter;
>+dstTCP->seq = htonl(seqNumber);
>+
>+/*
>+ * Set the TCP FIN and PSH bit only for the last packet
>+ * More information can be found under:
>+ * 

Re: [ovs-dev] [PATCH v6 3/5] datapath-windows: Retain MRU value in the _OVS_BUFFER_CONTEXT.

[Sairam Venugopal]

Thanks for incorporating this change.

Acked-by: Sairam Venugopal 





On 3/24/17, 1:51 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand Kumar" 
 wrote:

>This patch introduces a new field MRU(Maximum Recieved Unit) in the
>_OVS_BUFFER_CONTEXT and it is used only for Ip Fragments to retain MRU for
>the reassembled IP datagram when the packet is forwarded to userspace.
>
>Signed-off-by: Anand Kumar 
>---
>v5->v6: No Change
>v4->v5:
>   - Refactored the patch as MRU field is removed form 
> ovsForwardingContext.
>   - Added MRU field in _OVS_BUFFER_CONTEXT.
>   - Updated commit message.
>v3->v4: No Change
>v2->v3: No change
>v1->v2: No change
>---
> datapath-windows/ovsext/BufferMgmt.c |  1 +
> datapath-windows/ovsext/BufferMgmt.h |  1 +
> datapath-windows/ovsext/DpInternal.h |  2 +-
> datapath-windows/ovsext/IpFragment.c |  2 ++
> datapath-windows/ovsext/User.c   | 20 +++-
> 5 files changed, 24 insertions(+), 2 deletions(-)
>
>diff --git a/datapath-windows/ovsext/BufferMgmt.c 
>b/datapath-windows/ovsext/BufferMgmt.c
>index 6027c35..d99052d 100644
>--- a/datapath-windows/ovsext/BufferMgmt.c
>+++ b/datapath-windows/ovsext/BufferMgmt.c
>@@ -266,6 +266,7 @@ OvsInitNBLContext(POVS_BUFFER_CONTEXT ctx,
> ctx->flags = flags;
> ctx->srcPortNo = srcPortNo;
> ctx->origDataLength = origDataLength;
>+ctx->mru = 0;
> }
> 
> 
>diff --git a/datapath-windows/ovsext/BufferMgmt.h 
>b/datapath-windows/ovsext/BufferMgmt.h
>index 11a05b2..77b2854 100644
>--- a/datapath-windows/ovsext/BufferMgmt.h
>+++ b/datapath-windows/ovsext/BufferMgmt.h
>@@ -58,6 +58,7 @@ typedef union _OVS_BUFFER_CONTEXT {
> UINT32 origDataLength;
> UINT32 dataOffsetDelta;
> };
>+UINT16 mru;
> };
> 
> UINT64 value[MEM_ALIGN_SIZE(16) >> 3];
>diff --git a/datapath-windows/ovsext/DpInternal.h 
>b/datapath-windows/ovsext/DpInternal.h
>index f62fc55..9d1a783 100644
>--- a/datapath-windows/ovsext/DpInternal.h
>+++ b/datapath-windows/ovsext/DpInternal.h
>@@ -298,7 +298,7 @@ typedef struct _OVS_PACKET_INFO {
> typedef struct OvsPacketExecute {
>uint32_t dpNo;
>uint32_t inPort;
>-
>+   uint16 mru;
>uint32_t packetLen;
>uint32_t actionsLen;
>PNL_MSG_HDR nlMsgHdr;
>diff --git a/datapath-windows/ovsext/IpFragment.c 
>b/datapath-windows/ovsext/IpFragment.c
>index 37abcd8..b0294ce 100644
>--- a/datapath-windows/ovsext/IpFragment.c
>+++ b/datapath-windows/ovsext/IpFragment.c
>@@ -208,6 +208,8 @@ OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
> OvsCompleteNBL(switchContext, *curNbl, TRUE);
> }
> /* Store mru in the ovs buffer context. */
>+ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(*newNbl);
>+ctx->mru = entry->mru;
> *curNbl = *newNbl;
> return status;
> }
>diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
>index c7ac284..3154640 100644
>--- a/datapath-windows/ovsext/User.c
>+++ b/datapath-windows/ovsext/User.c
>@@ -283,7 +283,8 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT 
>usrParamsCtx,
> [OVS_PACKET_ATTR_ACTIONS] = {.type = NL_A_UNSPEC, .optional = FALSE},
> [OVS_PACKET_ATTR_USERDATA] = {.type = NL_A_UNSPEC, .optional = TRUE},
> [OVS_PACKET_ATTR_EGRESS_TUN_KEY] = {.type = NL_A_UNSPEC,
>-.optional = TRUE}
>+.optional = TRUE},
>+[OVS_PACKET_ATTR_MRU] = { .type = NL_A_U16, .optional = TRUE }
> };
> 
> RtlZeroMemory(, sizeof(OvsPacketExecute));
>@@ -381,6 +382,10 @@ _MapNlAttrToOvsPktExec(PNL_MSG_HDR nlMsgHdr, PNL_ATTR 
>*nlAttrs,
> ASSERT(keyAttrs[OVS_KEY_ATTR_IN_PORT]);
> execute->inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]);
> execute->keyAttrs = keyAttrs;
>+
>+if (nlAttrs[OVS_PACKET_ATTR_MRU]) {
>+execute->mru = NlAttrGetU16(nlAttrs[OVS_PACKET_ATTR_MRU]);
>+}
> }
> 
> NTSTATUS
>@@ -397,6 +402,7 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
> POVS_VPORT_ENTRYvport = NULL;
> PNL_ATTR tunnelAttrs[__OVS_TUNNEL_KEY_ATTR_MAX];
> OvsFlowKey tempTunKey = {0};
>+POVS_BUFFER_CONTEXT ctx;
> 
> if (execute->packetLen == 0) {
> status = STATUS_INVALID_PARAMETER;
>@@ -459,6 +465,9 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
> ndisStatus = OvsExtractFlow(pNbl, execute->inPort, , ,
>  tempTunKey.tunKey.dst == 0 ? NULL : );
> 
>+ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(pNbl);
>+ctx->mru = execute->mru;
>+
> if (ndisStatus == NDIS_STATUS_SUCCESS) {
> NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, , 0);
> ndisStatus = OvsActionsExecute(gOvsSwitchContext, NULL, pNbl,
>@@ -988,6 +997,7 @@ OvsCreateQueueNlPacket(PVOID userData,
> UINT32 nlMsgSize;
> NL_BUFFER nlBuf;
> 

Re: [ovs-dev] [PATCH v6 1/5] datapath-windows: Added a new file to support Ipv4 fragments.

[Sairam Venugopal]

Thanks for the patches!

Acked-by: Sairam Venugopal 






On 3/24/17, 1:51 PM, "ovs-dev-boun...@openvswitch.org on behalf of Anand Kumar" 
 wrote:

>This patch adds functionalities to support IPv4 fragments, which will be
>used by Conntrack module.
>
>Added a new structure to hold the Ipv4 fragments and a hash table to
>hold Ipv4 datagram entries. Also added a clean up thread that runs
>every minute to delete the expired IPv4 datagram entries.
>
>The individual fragments are ignored by the conntrack. Once all the
>fragments are recieved, a new NBL is created out of the reassembled
>fragments and conntrack executes actions on the new NBL.
>
>Created new APIs OvsProcessIpv4Fragment() to process individual fragments,
>OvsIpv4Reassemble() to reassemble Ipv4 fragments.
>
>Signed-off-by: Anand Kumar 
>---
>v5->v6: No Change
>v4->v5:
>  - Modified Patch 3 to retain MRU in _OVS_BUFFER_CONTEXT instead of
>   using it in ovsForwardingContext with minor changes in rest of the
>   patches.
>v3->v4:
>  - Rebase
>  - Acquire read lock for read operations.
>v2->v3:
>  - using spinlock instead of RW lock.
>  - updated log messages, summary, fixed alignment issues.
>v1->v2:
>  - Patch 4 updated to make it compile for release mode.
>---
> datapath-windows/automake.mk   |   2 +
> datapath-windows/ovsext/Debug.h|   3 +-
> datapath-windows/ovsext/IpFragment.c   | 500 +
> datapath-windows/ovsext/IpFragment.h   |  73 +
> datapath-windows/ovsext/Switch.c   |   9 +
> datapath-windows/ovsext/ovsext.vcxproj |   2 +
> 6 files changed, 588 insertions(+), 1 deletion(-)
> create mode 100644 datapath-windows/ovsext/IpFragment.c
> create mode 100644 datapath-windows/ovsext/IpFragment.h
>
>diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
>index 53983ae..4f7b55a 100644
>--- a/datapath-windows/automake.mk
>+++ b/datapath-windows/automake.mk
>@@ -32,6 +32,8 @@ EXTRA_DIST += \
>   datapath-windows/ovsext/Flow.h \
>   datapath-windows/ovsext/Gre.h \
>   datapath-windows/ovsext/Gre.c \
>+  datapath-windows/ovsext/IpFragment.c \
>+  datapath-windows/ovsext/IpFragment.h \
>   datapath-windows/ovsext/IpHelper.c \
>   datapath-windows/ovsext/IpHelper.h \
>   datapath-windows/ovsext/Jhash.c \
>diff --git a/datapath-windows/ovsext/Debug.h b/datapath-windows/ovsext/Debug.h
>index cae6ac9..6de1812 100644
>--- a/datapath-windows/ovsext/Debug.h
>+++ b/datapath-windows/ovsext/Debug.h
>@@ -42,8 +42,9 @@
> #define OVS_DBG_STT  BIT32(22)
> #define OVS_DBG_CONTRK   BIT32(23)
> #define OVS_DBG_GENEVE   BIT32(24)
>+#define OVS_DBG_IPFRAG   BIT32(25)
> 
>-#define OVS_DBG_LAST 24  /* Set this to the last defined module number. */
>+#define OVS_DBG_LAST 25  /* Set this to the last defined module number. */
> /* Please add above OVS_DBG_LAST. */
> 
> #define OVS_DBG_ERRORDPFLTR_ERROR_LEVEL
>diff --git a/datapath-windows/ovsext/IpFragment.c 
>b/datapath-windows/ovsext/IpFragment.c
>new file mode 100644
>index 000..37abcd8
>--- /dev/null
>+++ b/datapath-windows/ovsext/IpFragment.c
>@@ -0,0 +1,500 @@
>+/*
>+ * Copyright (c) 2017 VMware, 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:
>+ *
>+ * 
>https://urldefense.proofpoint.com/v2/url?u=http-3A__www.apache.org_licenses_LICENSE-2D2.0=DwICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=1abLNFAEjs9Cscb3AJMpOuBIqOdZ-_J4Gaaxv_J69Uc=8iji8m6jUpxnBC2ddLAT_24ix4jr_EMbFXX31y-p3h4=
> 
>+ *
>+ * 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 "Conntrack.h"
>+#include "Debug.h"
>+#include "IpFragment.h"
>+#include "Jhash.h"
>+#include "Offload.h"
>+#include "PacketParser.h"
>+
>+#ifdef OVS_DBG_MOD
>+#undef OVS_DBG_MOD
>+#endif
>+#define OVS_DBG_MOD OVS_DBG_IPFRAG
>+
>+/* Function declarations */
>+static VOID OvsIpFragmentEntryCleaner(PVOID data);
>+static VOID OvsIpFragmentEntryDelete(POVS_IPFRAG_ENTRY entry);
>+
>+/* Global and static variables */
>+static OVS_IPFRAG_THREAD_CTX ipFragThreadCtx;
>+static PNDIS_RW_LOCK_EX ovsIpFragmentHashLockObj;
>+static UINT64 ipTotalEntries;
>+static PLIST_ENTRY OvsIpFragTable;
>+
>+NDIS_STATUS
>+OvsInitIpFragment(POVS_SWITCH_CONTEXT context)
>+{
>+
>+NDIS_STATUS status;
>+HANDLE threadHandle = NULL;
>+
>+/* Init the sync-lock */
>+ovsIpFragmentHashLockObj = NdisAllocateRWLock(context->NdisFilterHandle);
>+if (ovsIpFragmentHashLockObj == NULL) {
>+return 

Re: [ovs-dev] [ovs-dev, RFC] ovn: Revised support for service function chaining

[Mickey Spiegel]

John,

On Thu, Apr 6, 2017 at 9:15 AM, John McDowall <
jmcdow...@paloaltonetworks.com> wrote:

> Mickey,
>
>
>
> Thanks, here is what I propose, based on your comments:
>
>
>
> 1.   Support for non-ip traffic : I will make the agreed on changes.
>
> 2.   Match Filter: Will make the changes
>
Not sure that there are any changes to make other than documentation,
unless you found some magic solution :-). Are you referring to
documentation?

> 3.   Support for VNF’s that change src/dst: Will leave as an open
> issue for now and look for relevant use cases.
>
OK.

> 4.   Multiple classifiers: Will limit each port(application ) to a
> single chain for now, to prevent any errors. If there are use cases for
> multiple chains we can revisit
>
Would you actually check and limit?
Or just document?

> 5.   Mac Learning Issue: I looked at using other tunnels but as you
> correctly point out it adds a lot of complexity for not a huge amount of
> value. I think it is better to use the current Geneve overlay in OVN. I
> will make the changes to add a new register flag for chaining and have the
> flows exit the chain from the original src so the mac addresses are correct.
>
If you advance to an ingress table after S_SWITCH_IN_CHAIN, then you do not
need the flag. I saw the ability to start at a later table after I wrote
the flag comment, and forgot that that makes the flag unnecessary.

Mickey

>
>
> Regards
>
>
>
> John
>
>
>
>
>
>
>
> *From: *Mickey Spiegel 
> *Date: *Wednesday, April 5, 2017 at 4:02 PM
> *To: *John McDowall 
> *Cc: *ovs dev 
> *Subject: *Re: [ovs-dev] [ovs-dev, RFC] ovn: Revised support for service
> function chaining
>
>
>
> John,
>
>
>
> On Tue, Apr 4, 2017 at 10:03 AM, John McDowall <
> jmcdow...@paloaltonetworks.com> wrote:
>
> Mickey,
>
>
>
> Here are the proposed changes to address issues you brought up,  if these
> changes are acceptable I will add them and re-submit the patch.
>
>
>
>1. Support for Non-IP Traffic: The current code uses ipv4.src and
>ipv4.dst to identify flow source and destination. Change this is eth.src
>and eth.dst, there is no reason to require IP. This also addresses IPv6
>issues. If a user wants to add additional IP filtering they can add it to
>the “match” filter.
>
> This would resolve the issue with non-IP traffic.
>
>
>
> Do you want to support VNFs that modify the source or destination ethernet
> address?
>
> If so, that would require further investigation and additional logic.
>
>
>
>
>1. Match filter: Can cause conflicts but not really any different from
>ACL where user can construct conflicting filters. I will try and add some
>light checking (warnings) to catch any major conflicts.
>
> Are you referring to this issue that I had mentioned?
>
>
>
> a) If two different logical port chain classifiers with different "match"
> were specified on the same port, with different logical port chains that
> share at least one logical port pair, then the IP address does not
> distinguish between the two logical port chains.
>
>
>
> This seems like a tricky issue. I don't see any way to resolve it unless
> you use a tunneling format that specifies a chain ID such as NSH.
>
>
>
> If it is not resolved, then people have to think carefully whenever they
> specific a "match" condition. IMO this is less obvious than ACL conflicts,
> since it involves interactions between the "match" conditions and the
> logical port chain definitions.
>
>
>
>
>1. End of the service chain (MAC Learning Issue): I will add a new
>rule at the end of the chain to return the flow to the original src to
>ensure that the packet header is “correct” when leaving the host. I think
>I can set the recirculate flag at the end of the chain and then detect the
>flag and send it to the original destination.This will ensure that no
>packet leaves OVN with an “incorrect” header.
>
> When you say "original src", I assume you mean the location where eth.src
> resides?
>
>
>
> I am not sure what you mean by the "recirculate flag"?
>
> Are you referring to "flags.loopback"? Lots of things set that including
> DHCP and ARP responder, so you cannot infer much from the fact that the
> flag is set. In any case, that just lets the outport be the same as the
> inport. It does not determine what the outport is.
>
>
>
> To do item #3 I need some help composing the rule to recirculate the flow
> back to the original host - can someone point me to a good example or
> sample code?
>
>
>
> I assume you want the logic in build_chain to be something like:
>
>
>
> +if (j == (lpc->n_port_pair_groups-1)) {
>
>  /* Not sure why you are using xasprintf rather than using
>
>   * OVS dynamic strings like everything else in ovn-northd.c */
>
> +lcc_action = xasprintf("outport=%s; output;",
>
> +   

Re: [ovs-dev] [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb

[Ben Pfaff]

On Mon, Mar 27, 2017 at 02:56:08PM -0400, Lance Richardson wrote:
> This series implements role-based access control infrastructure for
> ovsdb-server, and uses that infrastructure to apply role-based access
> controls to the OVN_Southbound database. This implementation follows
> the outline discussed at:
> 
>  https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html
> 
> With this series applied, enabling role-based ACLs is a matter of:
> 
> - Configuring southbound ovsdb-server and ovn-controller to use SSL,
>   configuring an ovn-controller "role" for SSL connections via e.g.:
>  ovn-sbctl set-connection role=ovn-controller pssl:6642
> - Using unique certificates for each ovn-controller with a unique
>   CN for each chassis, generated e.g. via:
>  ovs-pki -B 1024 req+sign chassis1 switch
>  ovs-pki -B 1024 req+sign chassis2 switch
>  ovs-pki -B 1024 req+sign chassis3 switch
> - Starting the southbound ovsdb-server with the "--rbac" command-line
>   option:
>  --rbac=db:OVN_Southbound,RBAC_Role

This series is promising.

I'm a little concerned about additional per-DB command-line options
because it makes it hard to add and remove databases at runtime.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 5/5] ovn-sbctl: support setting rbac role for remote connections

[Ben Pfaff]

On Mon, Mar 27, 2017 at 02:56:13PM -0400, Lance Richardson wrote:
> Add support for specifying rbac "role" when setting remote
> connection configuration in southbound database.
> 
> Signed-off-by: Lance Richardson 

Seems reasonable at first glance, thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 4/5] ovn: add rbac tables to ovn southbound schema

[Ben Pfaff]

On Thu, Apr 06, 2017 at 08:58:02AM -0700, Ben Pfaff wrote:
> On Mon, Mar 27, 2017 at 02:56:12PM -0400, Lance Richardson wrote:
> > Add rbac "roles" and "permissions" tables to ovn southbound
> > database schema.
> > 
> > Signed-off-by: Lance Richardson 
> 
> I see that master has changed a little and will require a respin here,
> but I doubt it's a big deal.
> 
> This is where it becomes important to have good documentation for how
> OVSDB RBAC works; I guess that this is not yet available (other than the
> commit message) in the previous patch.
> 
> I see that this adds some documentation for the ovn-sb RBAC tables, but
> there's a real need for a description of the overall design and its
> goals, because otherwise the users won't understand what's important
> here.  I'd expect that that would go into the ovn-architecture or
> possibly the ovn-northd manpage.

Also, Clang says:

../ovn/northd/ovn-northd.c:5567:10: error: missing field 'auth' initializer 
[-Werror,-Wmissing-field-initializers]

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

Re: [ovs-dev] [RFC 4/5] ovn: add rbac tables to ovn southbound schema

[Ben Pfaff]

On Mon, Mar 27, 2017 at 02:56:12PM -0400, Lance Richardson wrote:
> Add rbac "roles" and "permissions" tables to ovn southbound
> database schema.
> 
> Signed-off-by: Lance Richardson 

I see that master has changed a little and will require a respin here,
but I doubt it's a big deal.

This is where it becomes important to have good documentation for how
OVSDB RBAC works; I guess that this is not yet available (other than the
commit message) in the previous patch.

I see that this adds some documentation for the ovn-sb RBAC tables, but
there's a real need for a description of the overall design and its
goals, because otherwise the users won't understand what's important
here.  I'd expect that that would go into the ovn-architecture or
possibly the ovn-northd manpage.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 3/5] ovsdb: add support for role-based access controls

[Ben Pfaff]

On Mon, Mar 27, 2017 at 02:56:11PM -0400, Lance Richardson wrote:
> Add suport for ovsdb RBAC (role-based access control). This includes:
> 
>- Support for new "--rbac " command-line option to
>  ovsdb-server, to specify the RBAC roles table to be used.
> 
>  This table has one row per role, with each row having a
>  "name" column (role name) and a "permissions" column (map of
>  table name to UUID of row in separate permission table.) The
>  permission table has one row per access control configuration,
>  with columns:
>   "name" - name of table to which this row applies
>   "authorization" - set of column names and column:key pairs
> to be compared against client ID to
> determine authorization status
>   "insert_delete" - boolean, true if insertions and
> authorized deletions are allowed.
>   "update"- Set of columns and column:key pairs for
> which authorized updates are allowed.
>- Support for a new "role" column in the remote configuration
>  table.
>- Logic for applying the RBAC role and permission tables, in
>  combination with session role and client id, to determine
>  whether operations modifying database contents should be
>  permitted.
> 
> Signed-off-by: Lance Richardson 

Thank you for working on this.  I have some preliminary comments; I did
not scrutinize all the code.

"git am" says:

.git/rebase-apply/patch:892: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

There's a too-deep indentation in the new code in
ovsdb_execute_insert().

I've tried to make the OVSDB errors a little more conversational in the
past, so that they'd be formatted something more like "RBAC for client X
with role Y prohibits update to column Z in table T."  Not that it's
important.

I'm not sure that a syntax error is the best choice of errors to use.
Access denied errors are pretty different from syntax errors, and in
theory a client might want to react differently in each case.  OVSDB
does not have an existing tag for errors that is completely appropriate,
so it might make sense to create a "permission denied" tag and use
that.

I don't think it makes sense to invent a tag named "mutate execution
denied by RBAC"; it is too specific.

Maybe the distinction between tags and details for errors is not clear.
The tag is supposed to be a kind of category, perhaps roughly equivalent
to an errno value.  It should be a short and generally pretty generic
string, which a machine can recognize.  The details, on the other hand,
are freeform text that is supposed to help a human find out what went
wrong in detail.

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

Re: [ovs-dev] [PATCH ovs V6 00/24] Introducing HW offload support for openvswitch

[Joe Stringer]

On 5 April 2017 at 02:00, Paul Blakey  wrote:
> Actually we've based the code on odp_flow_key_from_flow__, we are doing the
> same thing, and looking at it again, it isn't foolproof as well and in the
> same way. We will do the said check and printing a generic msg in case we
> did (maybe with some DBG info, like offset) something.

OK. I think the difference in odp_flow_key_from_flow__() is that that
function is always updated with the latest features and so on, given
that the primary push for new features and so on is in the OVS netlink
API. For TC flower, it's playing catchup so the lack of support for,
eg, connection tracking needs to be handled carefully.

I look forward to the next version.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V5 00/24] Introducing HW offload support for openvswitch

[Joe Stringer]

On 5 April 2017 at 00:38, Roi Dayan  wrote:
>
>
> On 05/04/2017 07:18, Roi Dayan wrote:
>>
>>
>>
>> On 04/04/2017 23:53, Joe Stringer wrote:
>>>
>>> On 3 April 2017 at 10:53, Joe Stringer  wrote:

 On 3 April 2017 at 03:27, Roi Dayan  wrote:
>
>
>
> On 29/03/2017 20:13, Joe Stringer wrote:
>>
>>
>> On 29 March 2017 at 04:50, Roi Dayan  wrote:
>>>
>>>
>>>
>>>
>>> On 23/03/2017 09:01, Joe Stringer wrote:



 I ran the make check-offloads tests on a recent net-next kernel
 and it
 failed, output was not as expected:

 ../../tests/system-offloaded-traffic.at:54
 : ovs-appctl
 dpctl/dump-flows |
 grep "eth_type(0x0800)" | sed -e



 's/used:[0-9].[0-9]*s/used:0.001s/;s/eth(src=[a-z0-9:]*,dst=[a-z0-9:]*)/eth(mac

 s)/;s/actions:[0-9,]*/actions:output/;s/recirc_id(0),//' | sort
 --- - 2017-03-22 16:43:37.598689692 -0700
 +++



 /home/vagrant/ovs/_build-clang/tests/system-offloads-testsuite.dir/at-groups/2/stdout

 2017-03-22 16:43:37.595628000 -0700
 @@ -1,3 +1,3 @@
 -in_port(2),eth(macs),eth_type(0x0800), packets:9, bytes:756,
 used:0.001s, actions:output
 -in_port(3),eth(macs),eth_type(0x0800), packets:9, bytes:756,
 used:0.001s, actions:output
 +in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
 bytes:882, used:0.001s, actions:output
 +in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:9,
 bytes:882, used:0.001s, actions:output

>>>
>>> Hi Joe,
>>>
>>> can you tell me what kernel you used here?
>>> maybe tc offloads were not supported and there was a fallback to
>>> OVS dp.
>>
>>
>>
>> I believe that it was a snapshot of net-next relatively recently,
>> 01461abe62df ("Merge branch 'fib-notifications-cleanup'"). I could try
>> again with latest net-next? Or do you think there may be some
>> userspace dependency the test relies on?
>>
>
> I installed net-next kernel and make check-offloads pass for me.
> The last commit I'm on is
> 397df70 Merge branch '40GbE' of
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
> last tag is 4.11-rc3+
> I'm thinking maybe the test fails for you from something else like a
> second
> openvswitch process running already?


 I don't think that was the case, but let me try again with your latest
 series and the above commit.
>>>
>>>
>>> I saw the same behaviour with upstream net-next 397df7092a15. My host
>>> is Ubuntu 14.04 with this kernel.
>>>
>>> I thought it might be because I'm not running any of your hardware and
>>> I assumed that the testsuite doesn't require hardware to run. Looking
>>> at the test it seems that assumption was wrong, but when I tried to
>>> configure the tc-policy to skip_hw with the following modification,
>>> the OVSDB change didn't seem to propagate into OVS (there were no log
>>> messages about changing the tc-policy):
>>>
>>> diff --git a/tests/system-offloaded-traffic.at
>>> b/tests/system-offloaded-traffic.at
>>> index 7aec8a3f430e..3ddf23a939a8 100644
>>> --- a/tests/system-offloaded-traffic.at
>>> +++ b/tests/system-offloaded-traffic.at
>>> @@ -40,6 +40,7 @@ AT_SETUP([offloads - ping between two ports -
>>> offloads enabled])
>>> OVS_TRAFFIC_VSWITCHD_START()
>>>
>>> AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>>> +AT_CHECK([ovs-vsctl set Open_vSwitch .
>>> other_config:tc-policy="skip_hw"])
>>> AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>>>
>>> ADD_NAMESPACES(at_ns0, at_ns1)
>>>
>>> ---
>>>
>>> Looking again, my kernel config had CLS_FLOWER disabled so that's
>>> probably what caused the issue. My ovs-vswitchd log from the test is
>>> below.
>>>
>>> 2017-04-04T20:41:50.737Z|1|vlog|INFO|opened log file
>>>
>>> /home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/ovs-vswitchd.log
>>>
>>> 2017-04-04T20:41:50.737Z|2|ovs_numa|INFO|Discovered 2 CPU cores on
>>> NUMA node 0
>>> 2017-04-04T20:41:50.737Z|3|ovs_numa|INFO|Discovered 1 NUMA nodes
>>> and 2 CPU cores
>>>
>>> 2017-04-04T20:41:50.738Z|4|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
>>>
>>> connecting...
>>>
>>> 2017-04-04T20:41:50.738Z|5|reconnect|INFO|unix:/home/joe/git/openvswitch/_build-gcc/tests/system-offloads-testsuite.dir/2/db.sock:
>>>
>>> connected
>>> 2017-04-04T20:41:50.743Z|6|bridge|INFO|ovs-vswitchd (Open vSwitch)
>>> 2.7.90
>>> 2017-04-04T20:41:50.757Z|7|ofproto_dpif|INFO|system@ovs-system:
>>> Datapath supports recirculation
>>> 

Re: [ovs-dev] [RFC 2/5] ovsdb: refactor utility functions into separate file

[Ben Pfaff]

On Mon, Mar 27, 2017 at 02:56:10PM -0400, Lance Richardson wrote:
> Move local db access functions to a new file and make give them
> global scope so they can be included in the ovsdb library and used
> by other ovsdb library functions.
> 
> Signed-off-by: Lance Richardson 

Seems reasonable, though possibly they should have some new prefix like
ovsdb_util_.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 1/5] stream: store stream peer id with stream state

[Ben Pfaff]

On Thu, Apr 06, 2017 at 08:37:34AM -0700, Ben Pfaff wrote:
> On Mon, Mar 27, 2017 at 02:56:09PM -0400, Lance Richardson wrote:
> > Keep track of authenticated ID for stream peer. For SSL connections,
> > the authenticated ID is the CN (Common Name) field from the peer's
> > SSL certificate.
> > 
> > Signed-off-by: Lance Richardson 
> 
> Not all the new functions here follow the OVS convention that the
> function name should be at the beginning of a line.
> 
> It looks like the convention here is that the peer id is the common
> name, except that if the common name contains "id:..." then that and
> everything after it is not part of the peer id.  Probably, this
> convention should be documented somewhere, although that might only come
> with a later patch that actually makes this feature user-visible (I
> haven't read ahead yet).

Oh, and I get build failures because:

../lib/stream-ssl.c:452:24: error: 'ASN1_STRING_data' is deprecated 
[-Werror,-Wdeprecated-declarations]
/usr/include/openssl/asn1.h:553:35: note: 'ASN1_STRING_data' has been 
explicitly marked deprecated here
/usr/include/i386-linux-gnu/openssl/opensslconf.h:130:53: note: expanded 
from macro 'DEPRECATEDIN_1_1_0'
/usr/include/i386-linux-gnu/openssl/opensslconf.h:105:35: note: expanded 
from macro 'DECLARE_DEPRECATED'
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC 1/5] stream: store stream peer id with stream state

[Ben Pfaff]

On Mon, Mar 27, 2017 at 02:56:09PM -0400, Lance Richardson wrote:
> Keep track of authenticated ID for stream peer. For SSL connections,
> the authenticated ID is the CN (Common Name) field from the peer's
> SSL certificate.
> 
> Signed-off-by: Lance Richardson 

Not all the new functions here follow the OVS convention that the
function name should be at the beginning of a line.

It looks like the convention here is that the peer id is the common
name, except that if the common name contains "id:..." then that and
everything after it is not part of the peer id.  Probably, this
convention should be documented somewhere, although that might only come
with a later patch that actually makes this feature user-visible (I
haven't read ahead yet).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 3/3] ovn-nbctl: include table formatting options in man page

[Ben Pfaff]

On Tue, Apr 04, 2017 at 09:44:51AM -0400, Lance Richardson wrote:
> Include descriptions of table formatting optiosn in ovn-nbctl
> man page.
> 
> Signed-off-by: Lance Richardson 
> ---
>  v2: no changes

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

Re: [ovs-dev] [PATCH v2 1/3] ovsdb-client: improve formatting option description in man page

[Ben Pfaff]

On Tue, Apr 04, 2017 at 09:44:39AM -0400, Lance Richardson wrote:
> Use correct option name for "--no-headings", remove duplicate
> "--no-heading" option in synopsis.
> 
> Clarify description of "--data=json" option.
> 
> Signed-off-by: Lance Richardson 
> ---
> v2: all new

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

Re: [ovs-dev] [PATCH] table: provide table formatting option help at runtime

[Ben Pfaff]

On Fri, Mar 31, 2017 at 01:21:03PM -0400, Lance Richardson wrote:
> Show table formatting options with help output from
> ovn-nbctl, obn-sbctl, ovs-vsctl, and vtep-ctl commands.
> Include "--data" option in ovsdb-client help output.
> 
> Signed-off-by: Lance Richardson 

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

Re: [ovs-dev] [ovs-discuss] ovn: unsnat handling error for Distributed Gateway

[Guoshuai Li]

revese my topology:

 +-++
 |  VM  172.16.1.7  |
 +-++
   |
 +-++
 |  Logical Switch  |
 +-++
   |172.16.1.254
  10.157.142.3 +---++
  ++  Logical Router 1  +
  |++
+-++
|  Logical Switch  |
+--+
  |++
  ++  Logical Router 2  |
  10.157.142.1 ++



Hi All, I am having a problem for ovn and need help, thanks.


I created two logical routes and connected the two LogicalRoutes 
through a external LogicalSwitch (connected to the external network) .


And then LogicalRoute-1 connected to the VM through the internal 
LogicalSwitch .


my topology:

  10.157.142.3  172.16.1.254
   ++ 
+-++   +-++
  ++  Logical Router 1 +--|  
Logical Switch  +---+ VM 172.16.1.7   |
  |++ 
+--+   +--+

+-++
|  Logical Switch  |
+--+
  |++
  ++  Logical Router 2  |
   ++
  10.157.142.1

I tested the master and Branch2.7, it Can not be transferred from VM 
(172.16.1.7) to LogicaRouter-2 's port (10.157.142.
Sorry, The destination address is 10.157.142.1, And The SNAT/unSNAT 
address is 10.157.142.3.

) via ping.
My logical router is a distributed gateway, and the two logical router 
ports that connect external LogicalSwitch are on the same chassis.
If the two logical router ports are not on the same chassis ping is 
also OK, And ping from VM (172.16.1.7) to external network is also OK.


I looked at the openflow tables on gateway chassis,  I suspected 
unsnat handling error in Router1 input for icmp replay.
I think it is necessary to replace the destination address 
10.157.142.3 with 172.16.1.7 in Table 19 and route 172.16.1.7 in Table 
21, but now the route match is 10.157.142.0/24.


cookie=0x92bd0055, duration=68.468s, table=16, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=50,reg14=0x4,metadata=0x7,dl_dst=fa:16:3e:58:1c:8a 
actions=resubmit(,17)
cookie=0x45765344, duration=68.467s, table=17, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,metadata=0x7 actions=resubmit(,18)
cookie=0xaeaaed29, duration=68.479s, table=18, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,metadata=0x7 actions=resubmit(,19)
cookie=0xce785d51, duration=68.479s, table=19, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=100,ip,reg14=0x4,metadata=0x7,nw_dst=10.157.142.3 
actions=ct(table=20,zone=NXM_NX_REG12[0..15],nat)
cookie=0xbd994421, duration=68.481s, table=20, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,metadata=0x7 actions=resubmit(,21)
cookie=0xaea3a6ae, duration=68.479s, table=21, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=49,ip,metadata=0x7,nw_dst=10.157.142.0/24 
actions=dec_ttl(),move:NXM_OF_IP_DST[]->NXM_NX_XXREG0[96..127],load:0xa9d8e03->NXM_NX_XXREG0[64..95],mod_dl_src:fa:16:3e:58:1c:8a,load:0x4->NXM_NX_REG15[],load:0x1->NXM_NX_REG10[0],resubmit(,22)
cookie=0xce6e8d4e, duration=68.482s, table=22, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,ip,metadata=0x7 
actions=push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],pop:NXM_NX_REG0[],mod_dl_dst:00:00:00:00:00:00,resubmit(,66),pop:NXM_NX_REG0[],resubmit(,23)
cookie=0xce89c4ed, duration=68.481s, table=23, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=150,reg15=0x4,metadata=0x7,dl_dst=00:00:00:00:00:00 
actions=load:0x5->NXM_NX_REG15[],resubmit(,24)
cookie=0xb2d84350, duration=68.469s, table=24, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=100,ip,metadata=0x7,dl_dst=00:00:00:00:00:00


I do not know why and need help, thanks.

___
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss


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

Re: [ovs-dev] [ovs-discuss] ovn: unsnat handling error for Distributed Gateway

[Guoshuai Li]

revese my topology:
+-++
 |  VM  172.16.1.7  |
 +-++
   |
 +-++
 |  Logical Switch  |
 +-++
   |172.16.1.254
  10.157.142.3 +---++
  ++  Logical Router 1  +
  |++
+-++
|  Logical Switch  |
+--+
  |++
  ++  Logical Router 2  |
  10.157.142.3 ++



Hi All, I am having a problem for ovn and need help, thanks.


I created two logical routes and connected the two LogicalRoutes 
through a external LogicalSwitch (connected to the external network) .


And then LogicalRoute-1 connected to the VM through the internal 
LogicalSwitch .


my topology:

  10.157.142.3  172.16.1.254
   ++ 
+-++   +-++
  ++  Logical Router 1 +--|  
Logical Switch  +---+ VM 172.16.1.7   |
  |++ 
+--+   +--+

+-++
|  Logical Switch  |
+--+
  |++
  ++  Logical Router 2  |
   ++
  10.157.142.3

I tested the master and Branch2.7, it Can not be transferred from VM 
(172.16.1.7) to LogicaRouter-2 's port (10.157.142.3) via ping.
My logical router is a distributed gateway, and the two logical router 
ports that connect external LogicalSwitch are on the same chassis.
If the two logical router ports are not on the same chassis ping is 
also OK, And ping from VM (172.16.1.7) to external network is also OK.


I looked at the openflow tables on gateway chassis,  I suspected 
unsnat handling error in Router1 input for icmp replay.
I think it is necessary to replace the destination address 
10.157.142.3 with 172.16.1.7 in Table 19 and route 172.16.1.7 in Table 
21, but now the route match is 10.157.142.0/24.


cookie=0x92bd0055, duration=68.468s, table=16, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=50,reg14=0x4,metadata=0x7,dl_dst=fa:16:3e:58:1c:8a 
actions=resubmit(,17)
cookie=0x45765344, duration=68.467s, table=17, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,metadata=0x7 actions=resubmit(,18)
cookie=0xaeaaed29, duration=68.479s, table=18, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,metadata=0x7 actions=resubmit(,19)
cookie=0xce785d51, duration=68.479s, table=19, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=100,ip,reg14=0x4,metadata=0x7,nw_dst=10.157.142.3 
actions=ct(table=20,zone=NXM_NX_REG12[0..15],nat)
cookie=0xbd994421, duration=68.481s, table=20, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,metadata=0x7 actions=resubmit(,21)
cookie=0xaea3a6ae, duration=68.479s, table=21, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=49,ip,metadata=0x7,nw_dst=10.157.142.0/24 
actions=dec_ttl(),move:NXM_OF_IP_DST[]->NXM_NX_XXREG0[96..127],load:0xa9d8e03->NXM_NX_XXREG0[64..95],mod_dl_src:fa:16:3e:58:1c:8a,load:0x4->NXM_NX_REG15[],load:0x1->NXM_NX_REG10[0],resubmit(,22)
cookie=0xce6e8d4e, duration=68.482s, table=22, n_packets=1, 
n_bytes=98, idle_age=36, priority=0,ip,metadata=0x7 
actions=push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],pop:NXM_NX_REG0[],mod_dl_dst:00:00:00:00:00:00,resubmit(,66),pop:NXM_NX_REG0[],resubmit(,23)
cookie=0xce89c4ed, duration=68.481s, table=23, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=150,reg15=0x4,metadata=0x7,dl_dst=00:00:00:00:00:00 
actions=load:0x5->NXM_NX_REG15[],resubmit(,24)
cookie=0xb2d84350, duration=68.469s, table=24, n_packets=1, 
n_bytes=98, idle_age=36, 
priority=100,ip,metadata=0x7,dl_dst=00:00:00:00:00:00


I do not know why and need help, thanks.

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


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

[ovs-dev] [ovs-discuss] ovn: unsnat handling error for Distributed Gateway

[Guoshuai Li]

Hi All, I am having a problem for ovn and need help, thanks.


I created two logical routes and connected the two LogicalRoutes through 
a external LogicalSwitch (connected to the external network) .


And then LogicalRoute-1 connected to the VM through the internal 
LogicalSwitch .


my topology:

  10.157.142.3  172.16.1.254
   ++ 
+-++   +-++
  ++  Logical Router 1 +--|  
Logical Switch  +---+ VM 172.16.1.7   |
  |++ 
+--+   +--+

+-++
|  Logical Switch  |
+--+
  |++
  ++  Logical Router 2  |
   ++
  10.157.142.3

I tested the master and Branch2.7, it Can not be transferred from VM 
(172.16.1.7) to LogicaRouter-2 's port (10.157.142.3) via ping.
My logical router is a distributed gateway, and the two logical router 
ports that connect external LogicalSwitch are on the same chassis.
If the two logical router ports are not on the same chassis ping is also 
OK, And ping from VM (172.16.1.7) to external network is also OK.


I looked at the openflow tables on gateway chassis,  I suspected unsnat 
handling error in Router1 input for icmp replay.
I think it is necessary to replace the destination address 10.157.142.3 
with 172.16.1.7 in Table 19 and route 172.16.1.7 in Table 21, but now 
the route match is 10.157.142.0/24.


cookie=0x92bd0055, duration=68.468s, table=16, n_packets=1, n_bytes=98, 
idle_age=36, priority=50,reg14=0x4,metadata=0x7,dl_dst=fa:16:3e:58:1c:8a 
actions=resubmit(,17)
cookie=0x45765344, duration=68.467s, table=17, n_packets=1, n_bytes=98, 
idle_age=36, priority=0,metadata=0x7 actions=resubmit(,18)
cookie=0xaeaaed29, duration=68.479s, table=18, n_packets=1, n_bytes=98, 
idle_age=36, priority=0,metadata=0x7 actions=resubmit(,19)
cookie=0xce785d51, duration=68.479s, table=19, n_packets=1, n_bytes=98, 
idle_age=36, priority=100,ip,reg14=0x4,metadata=0x7,nw_dst=10.157.142.3 
actions=ct(table=20,zone=NXM_NX_REG12[0..15],nat)
cookie=0xbd994421, duration=68.481s, table=20, n_packets=1, n_bytes=98, 
idle_age=36, priority=0,metadata=0x7 actions=resubmit(,21)
cookie=0xaea3a6ae, duration=68.479s, table=21, n_packets=1, n_bytes=98, 
idle_age=36, priority=49,ip,metadata=0x7,nw_dst=10.157.142.0/24 
actions=dec_ttl(),move:NXM_OF_IP_DST[]->NXM_NX_XXREG0[96..127],load:0xa9d8e03->NXM_NX_XXREG0[64..95],mod_dl_src:fa:16:3e:58:1c:8a,load:0x4->NXM_NX_REG15[],load:0x1->NXM_NX_REG10[0],resubmit(,22)
cookie=0xce6e8d4e, duration=68.482s, table=22, n_packets=1, n_bytes=98, 
idle_age=36, priority=0,ip,metadata=0x7 
actions=push:NXM_NX_REG0[],push:NXM_NX_XXREG0[96..127],pop:NXM_NX_REG0[],mod_dl_dst:00:00:00:00:00:00,resubmit(,66),pop:NXM_NX_REG0[],resubmit(,23)
cookie=0xce89c4ed, duration=68.481s, table=23, n_packets=1, n_bytes=98, 
idle_age=36, 
priority=150,reg15=0x4,metadata=0x7,dl_dst=00:00:00:00:00:00 
actions=load:0x5->NXM_NX_REG15[],resubmit(,24)
cookie=0xb2d84350, duration=68.469s, table=24, n_packets=1, n_bytes=98, 
idle_age=36, priority=100,ip,metadata=0x7,dl_dst=00:00:00:00:00:00


I do not know why and need help, thanks.

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

[ovs-dev] [PATCH 2/2] doc: Update makefile target for docs

[Stephen Finucane]

Signed-off-by: Stephen Finucane 
---
 Documentation/intro/install/documentation.rst | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/intro/install/documentation.rst 
b/Documentation/intro/install/documentation.rst
index 1160e94..94af950 100644
--- a/Documentation/intro/install/documentation.rst
+++ b/Documentation/intro/install/documentation.rst
@@ -74,14 +74,14 @@ Building
 Once Sphinx installed, the documentation can be built using the provided
 Makefile targets::
 
-$ make htmldocs
+$ make htmldocs-check
 
 .. important::
 
-   The ``htmldocs`` target will fail if there are any syntax errors. However,
-   it won't catch more succint issues such as style or grammar issues. As a
-   result, you should always inspect changes visually to ensure the result is
-   as intended.
+   The ``htmldocs-check`` target will fail if there are any syntax errors.
+   However, it won't catch more succint issues such as style or grammar issues.
+   As a result, you should always inspect changes visually to ensure the result
+   is as intended.
 
 Once built, documentation is available in the ``/Documentation/_build`` folder.
 Open the root ``index.html`` to browse the documentation.
-- 
2.9.3

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

[ovs-dev] [PATCH 1/2] doc: Describe how docs.openvswitch.org works

[Stephen Finucane]

Signed-off-by: Stephen Finucane 
---
 Documentation/index.rst|  3 +-
 .../internals/contributing/documentation-style.rst |  6 ++
 Documentation/internals/documentation.rst  | 78 ++
 Documentation/internals/index.rst  |  1 +
 4 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/internals/documentation.rst

diff --git a/Documentation/index.rst b/Documentation/index.rst
index 6970dce..d81e9ba 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -108,7 +108,8 @@ Learn more about the Open vSwitch project and about how you 
can contribute:
   :doc:`internals/committer-grant-revocation`
 
 - **Documentation:** :doc:`internals/contributing/documentation-style` |
-  :doc:`Building Open vSwitch Documentation `
+  :doc:`Building Open vSwitch Documentation ` |
+  :doc:`internals/documentation`
 
 Getting Help
 -
diff --git a/Documentation/internals/contributing/documentation-style.rst 
b/Documentation/internals/contributing/documentation-style.rst
index f9da44c..9fe473c 100644
--- a/Documentation/internals/contributing/documentation-style.rst
+++ b/Documentation/internals/contributing/documentation-style.rst
@@ -32,6 +32,12 @@ Open vSwitch. Documentation includes any documents found in 
``Documentation``
 along with any ``README``, ``MAINTAINERS``, or generally ``rst`` suffixed
 documents found in the project tree.
 
+.. note::
+
+   This guide only applies to documentation for Open vSwitch v2.7. or greater.
+   Previous versions of Open vSwitch used a combination of Markdown and raw
+   plain text, and guidelines for these are not detailed here.
+
 reStructuredText vs. Sphinx
 ---
 
diff --git a/Documentation/internals/documentation.rst 
b/Documentation/internals/documentation.rst
new file mode 100644
index 000..eed98b4
--- /dev/null
+++ b/Documentation/internals/documentation.rst
@@ -0,0 +1,78 @@
+..
+  Copyright (c) 2017 Stephen Finucane 
+
+  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.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+==
+How Open vSwitch's Documentation Works
+==
+
+This document provides a brief overview on how the documentation build system
+within Open vSwitch works. This is intended to maximize the "bus factor" and
+share best practices with other projects.
+
+reStructuredText and Sphinx
+---
+
+Nearly all of Open vSwitch's documentation is written in `reStructuredText`__,
+with man pages being the sole exception. Of this documentation, most of it is
+fed into `Sphinx`__, which provides not only the ability to convert rST to a
+variety of other output formats but also allows for things like
+cross-referencing and indexing. for more information on the two, refer to the
+:doc:`contributing/documentation-style`.
+
+ovs-sphinx-theme
+
+
+The documentation uses its own theme, `ovs-sphinx-theme`, which can be found on
+GitHub__ and is published on pypi__. This is packaged separately from Open
+vSwitch itself to ensure all documentation gets the latest version of the theme
+(assuming there are no major version bumps in that package). If building
+locally and the package is installed, it will be used. If the package is not
+installed, Sphinx will fallback to the default theme.
+
+The package is currently maintained by Stephen Finucane and Russell Bryant.
+
+Read the Docs
+-
+
+The documentation is hosted on readthedocs.org and a CNAME redirect is in place
+to allow access from docs.openvswitch.org. *Read the Docs* provides a couple of
+nifty features for us, such as automatic building of docs whenever there are
+changes and versioning of documentation.
+
+The *Read the Docs* project is currently maintained by Stephen Finucane,
+Russell Bryant and Ben Pfaff.
+
+openvswitch.org
+---
+
+The sources for openvswitch.org are maintained separately from
+docs.openvswitch.org. For modifications to this site, refer to the `GitHub
+project`__.
+
+__ http://docutils.sourceforge.net/rst.html
+__ 

Re: [ovs-dev] IPFIX crash

[Joe Stringer]

On 4 April 2017 at 22:34, Neelakantam Gaddam  wrote:
> Hi All,
>
> While testing the IPFIX feature with openvswitch-2.6.1, observed the below
> crash when traffic is on.
>
> [  347.143417] BUG: unable to handle kernel NULL pointer dereference
> at   (null)
> [  347.143452] IP: [] dst_cache_get_ip4+0xc/0x50 
> [openvswitch]
> [  347.143484] PGD 0
> [  347.143493] Oops:  [#1] SMP
> [  347.143506] Modules linked in: vhost_net vhost macvtap macvlan
> vport_geneve(OE) vport_gre(OE) vport_vxlan(OE) openvswitch(OE)
> nf_conntrack_ipv6 nf_nat_ipv6 nf_defrag_ipv6 geneve gre libcrc32c
> xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4
> iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4
> xt_conntrack nf_conntrack ipt_REJECT tun bridge stp llc ebtable_filter
> ebtables ip6table_filter ip6_tables iptable_filter fuse dm_mirror
> dm_region_hash dm_log dm_mod intel_powerclamp coretemp intel_rapl
> kvm_intel ixgbe(OE) kvm iTCO_wdt crc32_pclmul ipmi_ssif ipmi_devintf
> iTCO_vendor_support ses ghash_clmulni_intel enclosure ipmi_si vxlan
> ip6_udp_tunnel udp_tunnel aesni_intel mei_me sg sb_edac dcdbas
> edac_core lpc_ich mei shpchp wmi mfd_core acpi_power_meter lrw
> gf128mul glue_helper
> [  347.143809]  ablk_helper cryptd pcspkr ipmi_msghandler nfsd
> auth_rpcgss nfs_acl lockd grace sunrpc ip_tables ext4 mbcache jbd2
> sr_mod cdrom sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea
> sysfillrect sysimgblt drm_kms_helper ttm crct10dif_pclmul drm
> crct10dif_common ahci crc32c_intel libahci igb dca libata i2c_algo_bit
> ptp i2c_core megaraid_sas pps_core [last unloaded: liquidio_ovs]
> [  347.143960] CPU: 4 PID: 3818 Comm: vhost-3811 Tainted: G
> OE     3.10.0-327.el7.x86_64 #1
> [  347.143993] Hardware name: Dell Inc. PowerEdge T630/0W9WXC, BIOS
> 1.1.4 11/04/2014
> [  347.144019] task: 880847ec1700 ti: 8808516f8000 task.ti:
> 8808516f8000
> [  347.144043] RIP: 0010:[]  []
> dst_cache_get_ip4+0xc/0x50 [openvswitch]
> [  347.144079] RSP: 0018:8808516fb5d8  EFLAGS: 00010246
> [  347.144097] RAX:  RBX: 880849023210 RCX: 
> 880849023208
> [  347.144120] RDX:  RSI: 880849023210 RDI: 
> 
> [  347.144144] RBP: 8808516fb5e8 R08: 0a04a8c0 R09: 
> 880849023210
> [  347.144167] R10: 880849023e00 R11: 8808516fb890 R12: 
> 
> [  347.144190] R13: 8800784d8c00 R14:  R15: 
> 8808514568f8
> [  347.144214] FS:  () GS:88085e48()
> knlGS:
> [  347.144240] CS:  0010 DS:  ES:  CR0: 80050033
> [  347.144259] CR2:  CR3: 00081c865000 CR4: 
> 001427e0
> [  347.144283] DR0:  DR1:  DR2: 
> 
> [  347.144306] DR3:  DR6: 0ff0 DR7: 
> 0400
> [  347.144329] Stack:
> [  347.144337]  880849023210  8808516fb658
> a04f6b0a
> [  347.144365]  0a04a8c04d5e1de0 8800 8808516fb658
> 81524289
> [  347.144393]  020e0e0e6400 030e0e0e 80da67a9
> 880849023208
> [  347.144421] Call Trace:
> [  347.144436]  []
> vxlan_get_route.isra.41+0xea/0x130 [openvswitch]
> [  347.144464]  [] ? __skb_get_hash+0x39/0x160
> [  347.144487]  []
> ovs_vxlan_fill_metadata_dst+0x170/0x1e0 [openvswitch]
> [  347.144517]  []
> ovs_dev_fill_metadata_dst+0x9e/0xd0 [openvswitch]
> [  347.144544]  [] output_userspace+0xfe/0x180 [openvswitch]
> [  347.144569]  [] do_execute_actions+0x63d/0x8f0
> [openvswitch]
> [  347.144594]  [] ovs_execute_actions+0x41/0x130
> [openvswitch]
> [  347.145578]  [] ovs_dp_process_packet+0x94/0x140
> [openvswitch]
> [  347.146549]  [] ovs_vport_receive+0x73/0xd0 [openvswitch]
> [  347.147510]  [] ? enqueue_task_fair+0x402/0x6c0
> [  347.148470]  [] ? sched_clock_cpu+0x85/0xc0
> [  347.149432]  [] ? check_preempt_curr+0x75/0xa0
> [  347.150378]  [] ? ttwu_do_wakeup+0x19/0xd0
> [  347.151316]  [] ? ttwu_do_activate.constprop.84+0x5d/0x70
> [  347.152249]  [] ? try_to_wake_up+0x1b6/0x300
> [  347.153168]  [] ? __wake_up+0x44/0x50
> [  347.154090]  [] internal_dev_xmit+0x24/0x60 [openvswitch]
> [  347.155012]  [] dev_hard_start_xmit+0x171/0x3b0
> [  347.155919]  [] sch_direct_xmit+0x104/0x200
> [  347.156800]  [] dev_queue_xmit+0x236/0x570
> [  347.157662]  [] macvlan_start_xmit+0x3c/0xc0 [macvlan]
> [  347.158503]  [] dev_hard_start_xmit+0x171/0x3b0
> [  347.159327]  [] sch_direct_xmit+0x104/0x200
> [  347.160127]  [] dev_queue_xmit+0x236/0x570
> [  347.160901]  [] macvtap_get_user+0x414/0x720 [macvtap]
> [  347.161658]  [] macvtap_sendmsg+0x2b/0x30 [macvtap]
> [  347.162391]  [] handle_tx+0x2fc/0x550 [vhost_net]
> [  347.163111]  [] handle_tx_kick+0x15/0x20 [vhost_net]
> [  347.163807]  [] vhost_worker+0xfb/0x1e0 [vhost]
> [  347.164489]  [] ? vhost_dev_reset_owner+0x50/0x50 [vhost]
> [  

[ovs-dev] [PATCH] compat: vxlan: Fix NULL dereference in dst_cache.

[Joe Stringer]

Neelakantam reports:

BUG: unable to handle kernel NULL pointer dereference
RIP: 0010:[]  [] dst_cache_get_ip4+0xc/0x50 
[openvswitch]
Call Trace:
 [] vxlan_get_route.isra.41+0xea/0x130 [openvswitch]
 [] ? __skb_get_hash+0x39/0x160
 [] ovs_vxlan_fill_metadata_dst+0x170/0x1e0 [openvswitch]
 [] ovs_dev_fill_metadata_dst+0x9e/0xd0 [openvswitch]
 [] output_userspace+0xfe/0x180 [openvswitch]
 [] do_execute_actions+0x63d/0x8f0 [openvswitch]
 [] ovs_execute_actions+0x41/0x130 [openvswitch]
 [] ovs_dp_process_packet+0x94/0x140 [openvswitch]
 [] ovs_vport_receive+0x73/0xd0 [openvswitch]
 [] ? enqueue_task_fair+0x402/0x6c0
 [] ? sched_clock_cpu+0x85/0xc0
 [] ? check_preempt_curr+0x75/0xa0
 [] ? ttwu_do_wakeup+0x19/0xd0
 [] ? ttwu_do_activate.constprop.84+0x5d/0x70
 [] ? try_to_wake_up+0x1b6/0x300
 [] ? __wake_up+0x44/0x50
 [] internal_dev_xmit+0x24/0x60 [openvswitch]
 [] dev_hard_start_xmit+0x171/0x3b0
 [] sch_direct_xmit+0x104/0x200
 [] dev_queue_xmit+0x236/0x570
 [] macvlan_start_xmit+0x3c/0xc0 [macvlan]
 [] dev_hard_start_xmit+0x171/0x3b0
 [] sch_direct_xmit+0x104/0x200
 [] dev_queue_xmit+0x236/0x570
 [] macvtap_get_user+0x414/0x720 [macvtap]
 [] macvtap_sendmsg+0x2b/0x30 [macvtap]
 [] handle_tx+0x2fc/0x550 [vhost_net]
 [] handle_tx_kick+0x15/0x20 [vhost_net]
 [] vhost_worker+0xfb/0x1e0 [vhost]
 [] ? vhost_dev_reset_owner+0x50/0x50 [vhost]
 [] kthread+0xcf/0xe0
 [] ? kthread_create_on_node+0x140/0x140
 [] ret_from_fork+0x58/0x90
 [] ? kthread_create_on_node+0x140/0x140

ovs_vxlan_fill_metadata_dst() calls vxlan_get_route() with no dst_cache,
handle this case and don't attempt to use dst_cache.

Reported-by: Neelakantam Gaddam 
Signed-off-by: Joe Stringer 
---
 datapath/linux/compat/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 3abcab1dcba7..d9da6f3d18c8 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -916,7 +916,7 @@ static struct rtable *vxlan_get_route(struct vxlan_dev 
*vxlan,
  struct dst_cache *dst_cache,
  const struct ip_tunnel_info *info)
 {
-   bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
+   bool use_cache = (dst_cache && ip_tunnel_dst_cache_usable(skb, info));
struct rtable *rt = NULL;
struct flowi4 fl4;
 
-- 
2.12.0

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

[ovs-dev] [PATCH] python: Allow tuning the session probe_interval from IDL

[Lucas Alvares Gomes]

This patch is adding a new method set_probe_interval() to the Idl class
which allows configuring the probe interval for the IDL session.
Instances of Idl might want to tune this interval to be less agressive
than the current 5s one.

Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1680146
Signed-off-by: Lucas Alvares Gomes 
---
 python/ovs/db/idl.py | 9 +
 1 file changed, 9 insertions(+)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 079a03ba1..d3c0086c6 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -147,6 +147,15 @@ class Idl(object):
 table.condition = [True]
 table.cond_changed = False

+def set_probe_interval(self, probe_interval):
+"""Sets the probe interval for the IDL session, in milliseconds.
+
+If "probe_interval" is zero it disables the connection keepalive
+feature. If non-zero the value will be forced to at least 1000
+milliseconds.
+"""
+self._session.reconnect.set_probe_interval(probe_interval)
+
 def close(self):
 """Closes the connection to the database.  The IDL will no longer
 update."""
-- 
2.12.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] OVN L3-HA request for feedback

[Miguel Angel Ajo Pelayo]

Hello everybody,

 First I'd like to say hello, because I'll be able to spend more time
working
with this community, and I'm sure it will be an enjoyable journey for what
I've
seen (silently watching) during the last few years.

 I'm planning to start work (together with Anil) on the L3 High
availability area of OVN. We've been reading [1], and it seems quite
reasonable.

 We're wondering to fast forward and skip the naive active/backup
implementation
in favor of the Active/Standby (per router) based on bfd +
bundle(active_backup)
output actions, since the proposal of having ovn-northd monitoring the
gateways
seems a bit unnatural, and the difference in effort (naive vs
active/standby) is
probably not very big (warning: I tend to be optimistic).

 I spent a couple of days looking at how L3 works now, and, very
naively, I would
propose either having the redirect-chassis option of Logical_Router_Ports
accept
multiple chassis with priorities.

For example:

ovn-nbctl lrp-add R1 alice 00:00:02:01:02:03 172.16.1.1/24 \
 -- set Logical_Router_Port alice
options:redirect-chassis=gw1:10,gw2:20,gw3:30

Or multiple chassis without priorities:

ovn-nbctl lrp-add R1 alice 00:00:02:01:02:03 172.16.1.1/24 \
 -- set Logical_Router_Port alice
options:redirect-chassis=gw1,gw2,gw3

(and in this case we let ovn decide -and may be rewrite the option-
how to balance
priorities between gateways to spread the load)


We may want to have another field in the Logical_Router_Port, to
let us know which
one(s) is(are) the active gateway(s)


   This logical model would also allow for Active/Active L3 when we
have that implemented,
for example by assigning the same priorities.


   Alternatively we could have two options:
  * ha-redirect-chassis=:[ ..
::]
  * ha-redirect-mode=active_standby/active_active


Best regards,
Miguel Ángel Ajo

[1]
https://github.com/openvswitch/ovs/blob/master/Documentation/topics/high-availability.rst
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Invitation: OVS-DPDK bi-weekly meeting...

[Kevin Traynor]

Meeting cancelled today. Next meeting 20th April.

thanks,
Kevin.

On 03/24/2017 04:28 PM, Kevin Traynor wrote:
> 24 March 2017
> ATTENDEES: Johann, Antonio, Rashid, Eelco, Flavio, Simon, Georg, Jan,
> Robin, Ian, Bhanu, Darren, Billy, Fikkie, Vinod, Rick, Niaz, Kevin
> 
> GENERAL
> 
> If anyone doesn't have the calendar invite and wants it, drop me a mail
> (Kevin)
> 
> User guide / blog location in OvS.org - where is the best place to keep
> this material? (Robin/Ian)
> - blogs tend to be scattered various places
> - planet blog aggregator suggested
> - Ian to send a proposal about adding info from blogs to OVS docs
> 
> Bug Tracking - some common place to share bugs info (Robin/Ian)
> - suggestion to use github as a bug tracker
> - Ian will send a note to the mailing list
> 
> Release Testing: who does what in terms of features and functionality?
> (Robin/Ian)
> - would be good to identify gaps in testing
> - ask is for people to gather summary of tests/areas covered and share
> in future meetings (as a starting point)
> 
> PERFORMANCE
> 
> Update on OVS conf presented perf patches? (Kevin)
> - Tx batching - improves throughput but affects latency (Bhanu)
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328363.html
> --- may need to do something like this for vhost ports also (Jan)
> - Combine actions
> --- 1/2 patch merged, 2/2 wip
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-February/328708.html
> 
> Uni-dir perf drops (Kevin)
> - can happen with vhost ports in test case if enqueue/dequeue are on
> different pmds and dequeue pmd has no pkts to process (from any port)
> --- proposed dpdk patch to fix http://dpdk.org/dev/patchwork/patch/22186/
> 
> Indirect descriptors (Billy)
> - perf gain for non-dpdk in VM, no significant impact when dpdk in VM
> 
> EMC probabilistic insertion (disable EMC completely) (Bhanu)
> - Idea of disabling EMC for certain PMDs
> --- feedback seemed to be that it would only suit artificial test case
> 
> HWOL
> 
> Document about use cases and some other ways it can help. Please review
> and comment in the
> doc.https://docs.google.com/document/d/1A8adu1xzg53bzcFKINhffYyC0nCQjqwqnI1jAFx2sck/edit?usp=sharing
> 
> FEATURES
> 
> Userspace conntrack/NAT (Darren)
> - reworked and pulled in some new patches. v7 is posted
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/330189.html
> 
>>>
> On 12/13/2016 05:47 PM, ktray...@redhat.com wrote:
>> You have been invited to the following event.
>>
>> Title: OVS-DPDK bi-weekly meeting
>> Hi All,
>>
>> At the OVS-DPDK Meetup after the OVS conference, it was suggested to
>> have a regular sync up call between people working on OVS-DPDK. Let's
>> keep it to status/plans and keep the technical discussion for the
>> mailing list.
>>
>> I'd suggest for a first meeting, we collect status for the items
>> identified at the meetup
>> https://mail.openvswitch.org/pipermail/ovs-dev/2016-November/325217.html
>> and discuss any new plans or opens anyone wants to talk about.
>>
>> You can connect to Bluejeans on a computer or through phone dial in, all
>> welcome.
>> https://bluejeans.com/393834449
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

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

Re: [ovs-dev] [PATCH v2] netdev-dpdk: Create separate memory pool for each port

[Robert Wojciechowicz]

On Wed, Apr 05, 2017 at 05:12:44PM +0100, Kevin Traynor wrote:
> On 02/24/2017 11:27 AM, Robert Wojciechowicz wrote:
> > Since it's possible to delete memory pool in DPDK
> > we can try to estimate better required memory size
> > when port is reconfigured, e.g. with different number
> > of rx queues.
> > 
> 
> Hi Robert,
> 
> The patch does not apply on master. I think this is probably a good idea
> because now that the mtu can be set you can end up with multiple large
> mempools per socket.

Thanks for looking into this patch.
Good to see that it makes sense, so I will rebase it and send once again.
> 
> It's very much down to the wire, I wonder if there should be some amount
> of additional buffers (e.g. +MIN_NB_MBUF) for any corner cases which are
> hard to find under basic testing. Also, there may be some impact to the
> local mbuf cache when it is provisioned so tightly.

Good point, I will think about it.

> 
> Kevin.
> 

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

[ovs-dev] [PATCH 4/4] Add support for OpenFlow 1.6 (draft) port status and port mod messages.

[Ben Pfaff]

OpenFlow 1.6 adds support for EUI-64 addresses for ports, and extends
the maximum length of OpenFlow port names from 16 to 64 bytes.

ONF-JIRA: EXT-566
Signed-off-by: Ben Pfaff 
---
 NEWS   |   4 +-
 include/openflow/automake.mk   |   1 +
 include/openflow/openflow-1.0.h|   7 +-
 include/openflow/openflow-1.1.h|   3 +-
 include/openflow/openflow-1.4.h|   4 +-
 include/openflow/openflow-1.6.h|  98 +++
 include/openflow/openflow-common.h |   1 -
 include/openflow/openflow.h|   3 +-
 include/openvswitch/ofp-msgs.h |  16 +-
 include/openvswitch/ofp-util.h |  15 +-
 lib/ofp-print.c|   9 +
 lib/ofp-util.c | 332 +++--
 ofproto/ofproto-dpif-xlate.c   |   2 +-
 ofproto/ofproto-dpif.c |   6 +-
 ofproto/ofproto.c  |   6 +-
 tests/ofp-print.at |  41 +
 tests/ofproto.at   |  66 
 utilities/ovs-ofctl.c  |   1 +
 18 files changed, 505 insertions(+), 110 deletions(-)
 create mode 100644 include/openflow/openflow-1.6.h

diff --git a/NEWS b/NEWS
index ec8572a7fc59..6ec1f2f0b7ef 100644
--- a/NEWS
+++ b/NEWS
@@ -10,13 +10,15 @@ Post-v2.7.0
Log level can be changed in a usual OVS way using
'ovs-appctl vlog' commands for 'dpdk' module. Lower bound
still can be configured via extra arguments for DPDK EAL.
-   - The "learn" action now supports a "limit" option (see ovs-ofctl(8)).
- New support for multiple VLANs (802.1ad or "QinQ"), including a new
  "dot1q-tunnel" port VLAN mode.
- OVN:
  * Make the DHCPv4 router setting optional.
  * Gratuitous ARP for NAT addresses on a distributed logical router.
- Add the command 'ovs-appctl stp/show' (see ovs-vswitchd(8)).
+   - OpenFlow:
+ * The "learn" action now supports a "limit" option (see ovs-ofctl(8)).
+ * Increased support for OpenFlow 1.6 (draft).
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/include/openflow/automake.mk b/include/openflow/automake.mk
index 18cc649899f8..9755e7c0396e 100644
--- a/include/openflow/automake.mk
+++ b/include/openflow/automake.mk
@@ -9,6 +9,7 @@ openflowinclude_HEADERS = \
include/openflow/openflow-1.3.h \
include/openflow/openflow-1.4.h \
include/openflow/openflow-1.5.h \
+   include/openflow/openflow-1.6.h \
include/openflow/openflow-common.h \
include/openflow/openflow.h
 
diff --git a/include/openflow/openflow-1.0.h b/include/openflow/openflow-1.0.h
index 68c79526efcb..ad06610da3a4 100644
--- a/include/openflow/openflow-1.0.h
+++ b/include/openflow/openflow-1.0.h
@@ -21,6 +21,11 @@
 
 #include 
 
+/* Maximum name of a port.
+ *
+ * OpenFlow 1.6 (draft) increases this to 64. */
+#define OFP10_MAX_PORT_NAME_LEN  16
+
 /* Port number(s)   meaning
  * ---  --
  * 0x   not assigned a meaning by OpenFlow 1.0
@@ -97,7 +102,7 @@ enum ofp10_port_features {
 struct ofp10_phy_port {
 ovs_be16 port_no;
 struct eth_addr hw_addr;
-char name[OFP_MAX_PORT_NAME_LEN]; /* Null-terminated */
+char name[OFP10_MAX_PORT_NAME_LEN]; /* Null-terminated */
 
 ovs_be32 config;/* Bitmap of OFPPC_* and OFPPC10_* flags. */
 ovs_be32 state; /* Bitmap of OFPPS_* and OFPPS10_* flags. */
diff --git a/include/openflow/openflow-1.1.h b/include/openflow/openflow-1.1.h
index de28475435fa..a29db8f3ef87 100644
--- a/include/openflow/openflow-1.1.h
+++ b/include/openflow/openflow-1.1.h
@@ -53,6 +53,7 @@
 #define OPENFLOW_11_H 1
 
 #include 
+#include 
 
 /* OpenFlow 1.1 uses 32-bit port numbers.  Open vSwitch, for now, uses OpenFlow
  * 1.0 port numbers internally.  We map them to OpenFlow 1.0 as follows:
@@ -112,7 +113,7 @@ struct ofp11_port {
 uint8_t pad[4];
 struct eth_addr hw_addr;
 uint8_t pad2[2];  /* Align to 64 bits. */
-char name[OFP_MAX_PORT_NAME_LEN]; /* Null-terminated */
+char name[OFP10_MAX_PORT_NAME_LEN]; /* Null-terminated */
 
 ovs_be32 config;/* Bitmap of OFPPC_* flags. */
 ovs_be32 state; /* Bitmap of OFPPS_* and OFPPS11_* flags. */
diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index fcebe4eb3eb6..9399950b2837 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2008, 2014 The Board of Trustees of The Leland Stanford
+/* Copyright (c) 2008, 2014, 2017 The Board of Trustees of The Leland Stanford
 * Junior University
 * Copyright (c) 2011, 2012 Open Networking Foundation
 *
@@ -80,7 +80,7 @@ struct ofp14_port {
 uint8_t pad[2];
 struct eth_addr hw_addr;
 uint8_t pad2[2];  /* Align to 64 bits. */
-char name[OFP_MAX_PORT_NAME_LEN]; /* Null-terminated */
+char name[OFP10_MAX_PORT_NAME_LEN]; /* Null-terminated */
 
 ovs_be32 

[ovs-dev] [PATCH 1/4] util: New macro ovs_strlcpy_arrays().

[Ben Pfaff]

When both arguments to ovs_strlcpy() are character arrays, it makes sense
to just pass the smaller of their sizes as the overall size.  It's
somewhat error-prone and definitely redundant to write that by hand, so
this commit adds a new macro that does it automatically.

Signed-off-by: Ben Pfaff 
---
 lib/netdev-vport.c   |  4 ++--
 lib/ofp-util.c   | 24 
 lib/ovs-rcu.c|  5 ++---
 lib/tnl-ports.c  |  4 ++--
 lib/util.h   |  9 -
 ovn/controller/pinctrl.c |  4 ++--
 6 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 2d0aa4367f6d..39093e81f424 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2017 Nicira, Inc.
  * Copyright (c) 2016 Red Hat, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -276,7 +276,7 @@ tunnel_check_status_change__(struct netdev_vport *netdev)
 
 if (strcmp(netdev->egress_iface, iface)
 || netdev->carrier_status != status) {
-ovs_strlcpy(netdev->egress_iface, iface, IFNAMSIZ);
+ovs_strlcpy_arrays(netdev->egress_iface, iface);
 netdev->carrier_status = status;
 
 return true;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 54c83fa6b6c1..fac82c40cc18 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4325,7 +4325,7 @@ ofputil_decode_ofp10_phy_port(struct ofputil_phy_port *pp,
 {
 pp->port_no = u16_to_ofp(ntohs(opp->port_no));
 pp->hw_addr = opp->hw_addr;
-ovs_strlcpy(pp->name, opp->name, OFP_MAX_PORT_NAME_LEN);
+ovs_strlcpy_arrays(pp->name, opp->name);
 
 pp->config = ntohl(opp->config) & OFPPC10_ALL;
 pp->state = ntohl(opp->state) & OFPPS10_ALL;
@@ -4352,7 +4352,7 @@ ofputil_decode_ofp11_port(struct ofputil_phy_port *pp,
 return error;
 }
 pp->hw_addr = op->hw_addr;
-ovs_strlcpy(pp->name, op->name, OFP_MAX_PORT_NAME_LEN);
+ovs_strlcpy_arrays(pp->name, op->name);
 
 pp->config = ntohl(op->config) & OFPPC11_ALL;
 pp->state = ntohl(op->state) & OFPPS11_ALL;
@@ -4451,7 +4451,7 @@ ofputil_encode_ofp10_phy_port(const struct 
ofputil_phy_port *pp,
 
 opp->port_no = htons(ofp_to_u16(pp->port_no));
 opp->hw_addr = pp->hw_addr;
-ovs_strlcpy(opp->name, pp->name, OFP_MAX_PORT_NAME_LEN);
+ovs_strlcpy_arrays(opp->name, pp->name);
 
 opp->config = htonl(pp->config & OFPPC10_ALL);
 opp->state = htonl(pp->state & OFPPS10_ALL);
@@ -4470,7 +4470,7 @@ ofputil_encode_ofp11_port(const struct ofputil_phy_port 
*pp,
 
 op->port_no = ofputil_port_to_ofp11(pp->port_no);
 op->hw_addr = pp->hw_addr;
-ovs_strlcpy(op->name, pp->name, OFP_MAX_PORT_NAME_LEN);
+ovs_strlcpy_arrays(op->name, pp->name);
 
 op->config = htonl(pp->config & OFPPC11_ALL);
 op->state = htonl(pp->state & OFPPS11_ALL);
@@ -5269,7 +5269,7 @@ ofputil_decode_table_features(struct ofpbuf *msg,
 return OFPERR_OFPTFFC_BAD_TABLE;
 }
 
-ovs_strlcpy(tf->name, otf->name, OFP_MAX_TABLE_NAME_LEN);
+ovs_strlcpy_arrays(tf->name, otf->name);
 tf->metadata_match = otf->metadata_match;
 tf->metadata_write = otf->metadata_write;
 tf->miss_config = OFPUTIL_TABLE_MISS_DEFAULT;
@@ -5480,7 +5480,7 @@ ofputil_append_table_features_reply(const struct 
ofputil_table_features *tf,
 
 otf = ofpbuf_put_zeros(reply, sizeof *otf);
 otf->table_id = tf->table_id;
-ovs_strlcpy(otf->name, tf->name, sizeof otf->name);
+ovs_strlcpy_arrays(otf->name, tf->name);
 otf->metadata_match = tf->metadata_match;
 otf->metadata_write = tf->metadata_write;
 if (version >= OFP14_VERSION) {
@@ -6257,7 +6257,7 @@ ofputil_put_ofp10_table_stats(const struct 
ofputil_table_stats *stats,
 
 out = ofpbuf_put_zeros(buf, sizeof *out);
 out->table_id = features->table_id;
-ovs_strlcpy(out->name, features->name, sizeof out->name);
+ovs_strlcpy_arrays(out->name, features->name);
 out->wildcards = mf_bitmap_to_of10();
 out->max_entries = htonl(features->max_entries);
 out->active_count = htonl(stats->active_count);
@@ -6328,7 +6328,7 @@ ofputil_put_ofp11_table_stats(const struct 
ofputil_table_stats *stats,
 
 out = ofpbuf_put_zeros(buf, sizeof *out);
 out->table_id = features->table_id;
-ovs_strlcpy(out->name, features->name, sizeof out->name);
+ovs_strlcpy_arrays(out->name, features->name);
 out->wildcards = mf_bitmap_to_of11();
 out->match = mf_bitmap_to_of11(>match);
 out->instructions = ovsinst_bitmap_to_openflow(
@@ -6353,7 +6353,7 @@ ofputil_put_ofp12_table_stats(const struct 
ofputil_table_stats *stats,
 
 out = ofpbuf_put_zeros(buf, sizeof *out);
 out->table_id = features->table_id;
-ovs_strlcpy(out->name, features->name, sizeof out->name);
+ovs_strlcpy_arrays(out->name, features->name);
 out->match = 

[ovs-dev] [PATCH 2/4] check-structs: struct eth_addr has alignment 2, not 1.

[Ben Pfaff]

It consists of ovs_be16 elements, so it has 16-bit alignment.

(This doesn't make a difference for any actual OpenFlow protocol elements.)

Signed-off-by: Ben Pfaff 
---
 build-aux/check-structs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/check-structs b/build-aux/check-structs
index f79f23513d7b..8de9defe2b36 100755
--- a/build-aux/check-structs
+++ b/build-aux/check-structs
@@ -15,7 +15,7 @@ types['ovs_be16'] = {"size": 2, "alignment": 2}
 types['ovs_be32'] = {"size": 4, "alignment": 4}
 types['ovs_be64'] = {"size": 8, "alignment": 8}
 types['ovs_32aligned_be64'] = {"size": 8, "alignment": 4}
-types['struct eth_addr'] = {"size": 6, "alignment": 1}
+types['struct eth_addr'] = {"size": 6, "alignment": 2}
 
 token = None
 line = ""
-- 
2.10.2

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