Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan backed DVR

2019-05-02 Thread Numan Siddique
On Thu, May 2, 2019 at 6:40 AM Ankur Sharma 
wrote:

> Hi Numan,
>
> Appreciate your feedback.
> I tried removing the network_type  from the implementation and use
> localnet port’s presence as a criteria
>
> For differentiating between overlay and vlan/provider backed networking.
> This landed the implementation
>
> In a road block wrt N-S traffic.
>
> Hence, something like network_type is needed to clarify the absence of
> encap of any kind.
>
> I will make the changes as per your suggestion, and submit a V5 soon.
> However, I still think the PROVIDER may not be the right term.
>
> I am also considering the term “BRIDGED”. It aligns well with the
> documentation in ovn-nb.xml.
> “
>   There are two kinds of logical switches, that is, ones that fully
>
>   virtualize the network (overlay logical switches) and ones that
> provide
>
>   simple connectivity to a physical network (bridged logical switches).
>
>   They work in the same way when providing connectivity between logical
>
>   ports on same chasis, but differently when connecting remote logical
>
>   ports.  Overlay logical switches connect remote logical ports by
> tunnels,
>
>   while bridged logical switches provide connectivity to remote ports
> by
>
>   bridging the packets to directly connected physical L2 segment with
> the
>
>   help of localnet ports.  Each bridged logical switch has
>
>   one and only one localnet port, which has only one
> special
>
>   address unknown.
> “
>
> Let me know your thoughts. If you still want to use PROVIDER then I will
> use that in V5.
>

Hi Ankur,

The type = "BRIDGED" seems more appropriate to me.

Thanks
Numan


>
>
> Thanks again for review.
>
> Regards,
>
> Ankur
>
>
>
> *From:* Numan Siddique 
> *Sent:* Monday, April 29, 2019 3:55 AM
> *To:* Ankur Sharma 
> *Cc:* ovs-dev@openvswitch.org
> *Subject:* Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan
> backed DVR
>
>
>
>
>
>
>
> On Fri, Apr 26, 2019 at 5:47 AM Ankur Sharma 
> wrote:
>
> Hi Numan,
>
> Glad to see your review comments.
> Sorry, I missed your feedback on flat networks earlier.
>
> a. The idea behind network_type is just to indicate if packets will be
> encapsulated or not.
>
> b. Having type as “vlan” is a way of saying that packets will be tagged if
> there is a native vlan configured on the switch.
> Otherwise it will got out as untagged.
>
> c. Yes, comment regarding going through ports was not correct (will
> rectify it in v5), however I see value in having network_type.
> it will be helpful in debugging and coding. In code, checking
> network_type  and making some decisions looks better to me,
>
> rather than checking for localnet ports in the datapath.
>
>
>
> If you think its good to have network_type, then I would suggest to have 2
> values - "overload" and "provider" (instead of "vlan").
>
>
>
>
>
> d. Having said that, I agree that we can live without for now. If we do
> not hear much from other reviewers by early next week, then i
>
> will remove network_type related changes.
>
>
>
> My suggestion would be to have something like below in ovn-northd.c
>
>
>
> 
>
> enum ovn_datapath_nw_type {
>
> DP_NETWORK_OVERLAY,
>
> DP_NETWORK_PROVIDER
>
> };
>
>
>
> static void
>
> ovn_datapath_update_nw_type(struct ovn_datapath *od)
>
> {
>
> if (!od->nbs) {
>
> return;
>
> }
>
>
>
> if (!od->localnet_port) {
>
> od->network_type = DP_NETWORK_OVERLAY;
>
> } else {
>
> od->network_type = DP_NETWORK_PROVIDER;
>
> }
>
> }
>
> **
>
>
>
> and use ``od->network_type`` in rest of the code.
>
>
>
> Also the present patch displays the network_type in "ovn-nbctl show"
> command. It would be nice to still display that
>
> in case you remove the "network_type" from the Logical_Switch table.
>
>
>
> Thanks
>
> Numan
>
>
>
>
> Please find my responses on inline on rest of the comments. Will make
> these changes in v5.
>
> Thanks
>
> Regards,
> Ankur
>
>
>
>
>
> *From:* Numan Siddique 
> *Sent:* Thursday, April 25, 2019 1:40 AM
> *To:* Ankur Sharma 
> *Cc:* ovs-dev@openvswitch.org
> *Subject:* Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan
> backed DVR
>
>
>
>
>
>
>
> On Thu, Apr 25, 2019 at 5:46 AM Ankur Sharma 
> wrote:
>
> Background:
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
> [mail.openvswitch.org]
> 
> [2] 
> https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing
> [docs.google.com]
> 

Re: [ovs-dev] [PATCH 1/1] dpdk: Use DPDK 18.11.1 release.

2019-05-02 Thread Kevin Traynor
On 26/04/2019 12:07, Ilya Maximets wrote:
> On 25.04.2019 13:42, Ian Stokes wrote:
>> On 4/25/2019 11:09 AM, Ilya Maximets wrote:
>>> "On 24.04.2019 20:03, Ian Stokes wrote:
 Modify travis linux build script to use the latest
 DPDK stable release 18.11.1. Update docs for latest
 DPDK stable releases.

 Signed-off-by: Ian Stokes 
 ---
   .travis/linux-build.sh   | 2 +-
   Documentation/faq/releases.rst   | 4 ++--
   Documentation/intro/install/dpdk.rst | 8 
   Documentation/topics/dpdk/vhost-user.rst | 6 +++---
   4 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> Do we need a NEWS update for this change?
>>> 18.11.1 should not have any user-visible changes from the OVS
>>> point of view, however upcoming 18.11.2 will have in case we'll
>>> merge vhost destroy_connection related patch. The questions are:
>>> Are we going to add stable release updates to NEWS?
>> We can, if people think it's of use. It's now mentioned in the release notes 
>> of 18.11.1 that OVS DPDK was validated, so adding an entry into NEWS for the 
>> next OVS dot release could help push users to use the correct DPDK to OVS 
>> release mapping. To date we still see users on xx.yy.0 LTS releases which 
>> I'd like to encourage to move on.
> 
> OK. Sounds good.
> 

Good idea, +1

>>
>>> Do we need to mention stable releases that has no direct impact
>>> on OVS itself?
>> If it has no impact then we can just add an item into NEWS saying it's been 
>> validated and is supported.
> 
> OK.
> 
>> If there is a known impact then we should detail the nature of that impact, 
>> typically NEWS has an entry of 'various bug fixes' or something to that 
>> effect, are you thinking we should provide specifics of the bug fix that 
>> affects users as part of NEWS from now on? I don't see the harm once we are 
>> aware of it.
> 
> Yes, we can do that. Another option:
> * If there is no visible impact on OVS --> mention that new stable release 
> validated.
> * If there is some impact (feature depends on a bug fix in a new stable DPDK)
>   --> bump the minimal supported DPDK version.
> 
> Like this:
> NEWS:
>  * DPDK:
>- OVS validated with DPDK 18.11.1 which is recommended to use.
> 
>  * DPDK:
>- DPDK 18.11.2 is a new minimal supported version.
>- DPDK 18.11.1 and lower is no longer supported.
> 
> The key point here is that we relay on bug fixes in a new stable DPDK and able
> to merge new functionality/fixes that depends on them in OVS.
> 
> Thoughts?
> 

LGTM, hopefully the latter will be a rare event.

> Best regards, Ilya Maximets.
> 

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


Re: [ovs-dev] [PATCH 1/1] dpdk: Use DPDK 18.11.1 release.

2019-05-02 Thread Kevin Traynor
On 24/04/2019 18:03, Ian Stokes wrote:
> Modify travis linux build script to use the latest
> DPDK stable release 18.11.1. Update docs for latest
> DPDK stable releases.
> 
> Signed-off-by: Ian Stokes 
> ---
>  .travis/linux-build.sh   | 2 +-
>  Documentation/faq/releases.rst   | 4 ++--
>  Documentation/intro/install/dpdk.rst | 8 
>  Documentation/topics/dpdk/vhost-user.rst | 6 +++---
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 0cf5da6af..d869713f7 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -89,7 +89,7 @@ fi
>  
>  if [ "$DPDK" ] || [ "$DPDK_SHARED" ]; then
>  if [ -z "$DPDK_VER" ]; then
> -DPDK_VER="18.11"
> +DPDK_VER="18.11.1"

Looks like there's logic to handle the the dpdk-stable- dir prefix in
the travis prep code, and if it's been run through travis then it must
be ok.

>  fi
>  install_dpdk $DPDK_VER
>  if [ "$CC" = "clang" ]; then
> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
> index cd5aad162..6c5949b33 100644
> --- a/Documentation/faq/releases.rst
> +++ b/Documentation/faq/releases.rst
> @@ -173,11 +173,11 @@ Q: What DPDK version does each Open vSwitch release 
> work with?
>  2.4.x2.0
>  2.5.x2.2
>  2.6.x16.07.2
> -2.7.x16.11.8
> +2.7.x16.11.9

Not strictly part of this change, but maybe you could update the 17.11's
too (if they are ready) and add a generic comment in the commit message
to say updating docs for other new stable releases.

>  2.8.x17.05.2
>  2.9.x17.11.4
>  2.10.x   17.11.4
> -2.11.x   18.11
> +2.11.x   18.11.1
>   ===
>  
>  Q: Are all the DPDK releases that OVS versions work with maintained?
> diff --git a/Documentation/intro/install/dpdk.rst 
> b/Documentation/intro/install/dpdk.rst
> index 344d2b3a6..32b40c391 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -42,7 +42,7 @@ Build requirements
>  In addition to the requirements described in :doc:`general`, building Open
>  vSwitch with DPDK will require the following:
>  
> -- DPDK 18.11
> +- DPDK 18.11.1
>  
>  - A `DPDK supported NIC`_
>  
> @@ -71,9 +71,9 @@ Install DPDK
>  #. Download the `DPDK sources`_, extract the file and set ``DPDK_DIR``::
>  
> $ cd /usr/src/
> -   $ wget http://fast.dpdk.org/rel/dpdk-18.11.tar.xz
> -   $ tar xf dpdk-18.11.tar.xz
> -   $ export DPDK_DIR=/usr/src/dpdk-18.11
> +   $ wget http://fast.dpdk.org/rel/dpdk-18.11.1.tar.xz
> +   $ tar xf dpdk-18.11.1.tar.xz
> +   $ export DPDK_DIR=/usr/src/dpdk-18.11.1

export DPDK_DIR=/usr/src/dpdk-stable-18.11.1

> $ cd $DPDK_DIR
>  
>  #. (Optional) Configure DPDK as a shared library
> diff --git a/Documentation/topics/dpdk/vhost-user.rst 
> b/Documentation/topics/dpdk/vhost-user.rst
> index 993797de5..483e228e4 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -320,9 +320,9 @@ To begin, instantiate a guest as described in 
> :ref:`dpdk-vhost-user` or
>  DPDK sources to VM and build DPDK::
>  
>  $ cd /root/dpdk/
> -$ wget http://fast.dpdk.org/rel/dpdk-18.11.tar.xz
> -$ tar xf dpdk-18.11.tar.xz
> -$ export DPDK_DIR=/root/dpdk/dpdk-18.11
> +$ wget http://fast.dpdk.org/rel/dpdk-18.11.1.tar.xz
> +$ tar xf dpdk-18.11.1.tar.xz
> +$ export DPDK_DIR=/root/dpdk/dpdk-18.11.1

export DPDK_DIR=/usr/src/dpdk-stable-18.11.1

>  $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
>  $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
>  $ cd $DPDK_DIR
> 

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


[ovs-dev] Technology Customers and MSPs Contacts

2019-05-02 Thread laura . bryant
Hi,



Would you like to own the data in excel file for unlimited usages with
quarterly updates?



We can provide you the contact information of:-

   - *CEOs*
   - *CXOs*
   - *COOs*
   - *CTOs list*
   - *Chief information security officers list*
   - *EVP/SVP/VP of IT executives list*
   - *IT directors list*
   - *IT Managers List*
   - *IT security executives list*
   - *IT Resellers/VARs list*
   - *MSPs/MSSPs list*
   - *Database Administrators list*
   - *Network Administrators list*
   - *Business Intelligence Administrators list*
   - *SME IT decision makers list*
   - *Fortune 1,000 companies IT decision makers list*
   - *SME Business owners list*
   - *AWS Customers/partners list*
   - *Microsoft Customers/partners list*
   - *IBM Customers/partners list*
   - *Oracle Customers/partners list*
   - *SAP Customers/partners list*
   - *CRM Customers list*
   - *ERP Customers list*
   - *VoIP Customers list*
   - *Backup and recovery customers list.*
   - *Business Intelligence, Networking software, IT security software,
   Database application users list.*

We provide data across the globe - North America, EMEA, Asia Pacific, and
LATAM.



Please review and let me know your current requirement, we will be more
than happy to share you counts and other details.



We await your response!



Thanks,

*Laura Bryant*

Database coordinator



To opt out please response Remove
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ovn: Added missing --wait in ovn tests

2019-05-02 Thread Leonid Ryzhyk
Several of the ovn tests did not use the `--wait` flag to to wait for a
configuration change to propagate through the system. As a result,
these tests fail when `ovn-northd` is slow.

Fixed by adding `--wait=hv` or `--wait=sb` as appropriate.

Signed-off-by: Leonid Ryzhyk 
---
tests/ovn-northd.at |  8 
tests/ovn.at| 12 ++--
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 9588c76c9..62e58fd0e 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -204,7 +204,7 @@ AT_SKIP_IF([test $HAVE_PYTHON = no])
 ovn_start

 ovn-nbctl ls-add S1
-ovn-nbctl lsp-add S1 S1-vm1
+ovn-nbctl --wait=sb lsp-add S1 S1-vm1
 AT_CHECK([test x`ovn-nbctl lsp-get-up S1-vm1` = xdown])

 ovn-sbctl chassis-add hv1 geneve 127.0.0.1
@@ -224,7 +224,7 @@ ovn-nbctl ls-add S1
 ovn-nbctl lsp-add S1 S1-R1
 ovn-nbctl lsp-set-type S1-R1 router
 ovn-nbctl lsp-set-addresses S1-R1 02:ac:10:01:00:01
-ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1
+ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1
 AT_CHECK([test x`ovn-nbctl lsp-get-up S1-R1` = xup])

 AT_CLEANUP
@@ -242,7 +242,7 @@ ovn-nbctl ls-add S1
 ovn-nbctl lsp-add S1 S1-R1
 ovn-nbctl lsp-set-type S1-R1 router
 ovn-nbctl lsp-set-addresses S1-R1 02:ac:10:01:00:01
-ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1
+ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1

 ovn-sbctl lsp-bind S1-R1 gw1
 AT_CHECK([test x`ovn-nbctl lsp-get-up S1-R1` = xup])
@@ -263,7 +263,7 @@ ovn-nbctl ls-add S1
 ovn-nbctl lsp-add S1 S1-R1
 ovn-nbctl lsp-set-type S1-R1 router
 ovn-nbctl lsp-set-addresses S1-R1 router
-ovn-nbctl lsp-set-options S1-R1 router-port=R1-S1
+ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1
 AT_CHECK([test x`ovn-nbctl lsp-get-up S1-R1` = xup])

 AT_CLEANUP
diff --git a/tests/ovn.at b/tests/ovn.at
index 592f491fd..34dba4edf 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1689,7 +1689,7 @@ for i in 1 2; do
 ovn-nbctl lsp-add lsw0 lp$i$j
 ip_addrs="192.168.0.$i$j"
 ovn-nbctl lsp-set-addresses lp$i$j "f0:00:00:00:00:$i$j $ip_addrs"
-ovn-nbctl lsp-set-port-security lp$i$j f0:00:00:00:00:$i$j
+ovn-nbctl --wait=hv lsp-set-port-security lp$i$j
f0:00:00:00:00:$i$j
 done
 done

@@ -7161,7 +7161,7 @@ ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==84'
allow-related
 ovn-nbctl --log acl-add lsw0 to-lport 1000 'tcp.dst==85' allow-related

 ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==86' reject
-ovn-nbctl --log --severity=alert --name=reject-flow acl-add lsw0 to-lport
1000 'tcp.dst==87' reject
+ovn-nbctl --wait=hv --log --severity=alert --name=reject-flow acl-add lsw0
to-lport 1000 'tcp.dst==87' reject

 ovn-sbctl dump-flows

@@ -7268,7 +7268,7 @@ ovn-nbctl --log --severity=alert --meter=http-rl2
--name=http-acl2 acl-add lsw0

 # Add an ACL that doesn't rate-limit logs.
 ovn-nbctl --log --severity=alert --name=http-acl3 acl-add lsw0 to-lport
1000 'tcp.dst==82' drop
-
+ovn-nbctl --wait=hv sync

 # For each ACL, send 100 packets.
 for i in `seq 1 100`; do
@@ -12058,7 +12058,7 @@ AT_CHECK([ovn-sbctl get Address_Set pg2_ip6
addresses],
  [0], [[["2001:db8:2::ff:fe00:2", "2001:db8:3::ff:fe00:3"]]
 ])

-ovn-nbctl set Logical_Switch ls1 \
+ovn-nbctl --wait=sb set Logical_Switch ls1 \
 other_config:subnet=10.11.0.0/24
other_config:ipv6_prefix="2001:db8:11::"

 dnl Check if updated address got propagated to the port group address sets
@@ -13370,7 +13370,7 @@ ovn-nbctl lsp-set-addresses sw1-p5 "unknown"
 ovn-nbctl list logical_switch_port

 # Now try to add duplicate addresses on a new port. These should all fail
-ovn-nbctl lsp-add sw1 sw1-p5
+ovn-nbctl --wait=sb lsp-add sw1 sw1-p5
 AT_CHECK([ovn-nbctl lsp-set-addresses sw1-p5 "00:00:00:00:00:04
10.0.0.1"], [1], [],
 [ovn-nbctl: Error on switch sw1: duplicate IPv4 address 10.0.0.1
 ])
@@ -13829,7 +13829,7 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p0
dynamic_addresses], [0],
 ["0a:00:00:a8:01:03 192.168.1.2"
 ])

-ovn-nbctl lsp-set-addresses p0 router
+ovn-nbctl --wait=sb lsp-set-addresses p0 router

 ovn-nbctl get Logical-Switch-Port p0 dynamic_addresses
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] ovn: Added missing --wait in ovn tests

2019-05-02 Thread Ben Pfaff
On Thu, May 02, 2019 at 10:37:57AM -0700, Leonid Ryzhyk wrote:
> Several of the ovn tests did not use the `--wait` flag to to wait for a
> configuration change to propagate through the system. As a result,
> these tests fail when `ovn-northd` is slow.
> 
> Fixed by adding `--wait=hv` or `--wait=sb` as appropriate.
> 
> Signed-off-by: Leonid Ryzhyk 

Thanks, applied to master.

I had to fix up word-wrapping in the patch, though.  Please try to use
"git send-email" for patches.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2] ovs-actions.xml: Better document the "bundle" and "bundle_load" actions.

2019-05-02 Thread Ben Pfaff
Signed-off-by: Ben Pfaff 
---
v1->v2: Add paragraph about liveness.

 lib/ovs-actions.xml | 53 +
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml
index cfd9b81be604..76aa5afb4d06 100644
--- a/lib/ovs-actions.xml
+++ b/lib/ovs-actions.xml
@@ -789,15 +789,15 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
actions=mod_nw_src:1.2.3.4
 
 
   The bundle and bundle_load actions
-  bundle(fields, 
basis, algorithm, 
ofport, slaves:port...)
-  bundle_load(fields, 
basis, algorithm, 
ofport, dst, 
slaves:port...)
+  bundle(fields, 
basis, algorithm, ofport, 
slaves:port...)
+  bundle_load(fields, 
basis, algorithm, ofport, 
dst, 
slaves:port...)
 
   
 These actions choose a port (``slave'') from a comma-separated OpenFlow
 port list.  After selecting the port, bundle
 outputs to it, whereas bundle_load writes its port number
-to dst, which must be a field or subfield in the syntax
-described under ``Field Specifications'' above.
+to dst, which must be a 16-bit or wider field or subfield in
+the syntax described under ``Field Specifications'' above.
   
 
   
@@ -847,6 +847,51 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0 
actions=mod_nw_src:1.2.3.4
 
   
 
+  
+algorithm must be one of the following:
+  
+
+  
+active_backup
+
+  Chooses the first live port listed in slaves.
+
+
+hrw (Highest Random Weight)
+
+  
+Computes the following, considering only the live ports in
+slaves:
+  
+
+  
+for i in [1,n_slaves]:
+weights[i] = hash(flow, i)
+slave = { i such that weights[i] 
>= weights[j] for all j != i }
+  
+
+  
+This algorithm is specified by RFC 2992.
+  
+
+  
+
+  
+The algorithms take port liveness into account when selecting slaves.
+The definition of whether a port is live is subject to change.  It
+currently takes into account carrier status and link monitoring
+protocols such as BFD and CFM.  If none of the slaves is live,
+bundle does not output the packet and
+bundle_load stores OFPP_NONE (65535) in the
+output field.
+  
+
+  
+Example: bundle(eth_src,0,hrw,ofport,slaves:4,8) uses an
+Ethernet source hash with basis 0, to select between OpenFlow ports 4
+and 8 using the Highest Random Weight algorithm.
+  
+
   
 Open vSwitch 1.2 introduced the bundle and
 bundle_load OpenFlow extension actions.
-- 
2.20.1

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


Re: [ovs-dev] [PATCH] ovn: Added missing --wait in ovn tests

2019-05-02 Thread 0-day Robot
Bleep bloop.  Greetings Leonid Ryzhyk, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
fatal: patch fragment without header at line 7: @@ -7161,7 +7161,7 @@ ovn-nbctl 
acl-add lsw0 to-lport 1000 'tcp.dst==84'
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ovn: Added missing --wait in ovn tests
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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


Re: [ovs-dev] [PATCH v2] ovs-actions.xml: Better document the "bundle" and "bundle_load" actions.

2019-05-02 Thread 0-day Robot
Bleep bloop.  Greetings Ben Pfaff, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 188 characters long (recommended limit is 79)
#21 FILE: lib/ovs-actions.xml:792:
  bundle(fields, 
basis, algorithm, ofport, 
slaves:port...)

WARNING: Line is 222 characters long (recommended limit is 79)
#22 FILE: lib/ovs-actions.xml:793:
  bundle_load(fields, 
basis, algorithm, ofport, 
dst, 
slaves:port...)

WARNING: Line is 154 characters long (recommended limit is 79)
#59 FILE: lib/ovs-actions.xml:870:
slave = { i such that weights[i] 
>= weights[j] for all j != i }

Lines checked: 89, Warnings: 3, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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


[ovs-dev] [PATCH net-next] net: openvswitch: return an error instead of doing BUG_ON()

2019-05-02 Thread Eelco Chaudron
For all other error cases in queue_userspace_packet() the error is
returned, so it makes sense to do the same for these two error cases.

Reported-by: Davide Caratti 
Signed-off-by: Eelco Chaudron 
---
 net/openvswitch/datapath.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index b95015c..dc9ff93 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -455,7 +455,8 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
upcall->dp_ifindex = dp_ifindex;
 
err = ovs_nla_put_key(key, key, OVS_PACKET_ATTR_KEY, false, user_skb);
-   BUG_ON(err);
+   if (err)
+   goto out;
 
if (upcall_info->userdata)
__nla_put(user_skb, OVS_PACKET_ATTR_USERDATA,
@@ -471,7 +472,9 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
}
err = ovs_nla_put_tunnel_info(user_skb,
  upcall_info->egress_tun_info);
-   BUG_ON(err);
+   if (err)
+   goto out;
+
nla_nest_end(user_skb, nla);
}
 

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


Re: [ovs-dev] [PATCH net-next] net: openvswitch: return an error instead of doing BUG_ON()

2019-05-02 Thread Flavio Leitner via dev
On Thu, May 02, 2019 at 04:12:38PM -0400, Eelco Chaudron wrote:
> For all other error cases in queue_userspace_packet() the error is
> returned, so it makes sense to do the same for these two error cases.
> 
> Reported-by: Davide Caratti 
> Signed-off-by: Eelco Chaudron 
> ---

LGTM
Acked-by: Flavio Leitner 

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


[ovs-dev] [PATCH v5 0/2] OVN: Distributed Virtual Router for Vlan Backed Networks

2019-05-02 Thread Ankur Sharma
This series is about enhancing the logical router functionality in OVN to work
with vlan backed logical switches.

Intial proposal was discused here:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This series covers following:
a. L2:
   Associate a type with logical switches. Type value could be vlan or bridged.

b. L3 E-W:
   In the absence of encapsulation, we cannot use router port mac as source mac
   (since it is distributed), hence replace the same with a chassis unique mac.

c. L3 N-S:
   Use gateway-chassis construct to respond to ARP requests for router port,
   so that it becomes entry point for all chassis bound traffic coming from
   "external" network.
   Some additional changes, like no need to redirect south to north traffic
   in the absence of NAT etc.

This series does not cover following:
(will be sent out for review in follow up series once this series is 
reviewed/committed)
a. Network Address Translation.
b. Ensuring VMs mac is learnt in underlay network to avoid flooding of L3 flows.

Ankur Sharma (2):
  OVN: Enable E-W Traffic, Vlan backed DVR
  OVN: Enable N-S Traffic, Vlan backed DVR

 ovn/controller/binding.c|  12 +-
 ovn/controller/chassis.c|  66 -
 ovn/controller/chassis.h|   4 +
 ovn/controller/ovn-controller.8.xml |  10 +
 ovn/controller/ovn-controller.c |   2 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   | 114 
 ovn/controller/pinctrl.c| 205 +--
 ovn/controller/pinctrl.h|   6 +
 ovn/lib/ovn-util.c  |  31 +++
 ovn/lib/ovn-util.h  |   6 +
 ovn/northd/ovn-northd.c |  81 +-
 ovn/ovn-architecture.7.xml  |  12 +
 ovn/ovn-nb.ovsschema|  10 +-
 ovn/ovn-nb.xml  |  18 ++
 ovn/ovn-sb.xml  |  15 ++
 ovn/utilities/ovn-nbctl.c   |  49 +++-
 tests/ovn-nbctl.at  |  48 +++-
 tests/ovn-northd.at |  22 ++
 tests/ovn.at| 502 
 20 files changed, 1147 insertions(+), 71 deletions(-)

-- 
1.8.3.1

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


[ovs-dev] [PATCH v5 2/2] OVN: Enable N-S Traffic, Vlan backed DVR

2019-05-02 Thread Ankur Sharma
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This Series:
Layer 2, Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.

This patch:
For North-South traffic, we need a chassis which will respond to
ARP requests for router port coming from outside. For this purpose,
we will reply upon gateway-chassis construct in OVN, on a logical
router port, we will associate one or more chassis as gateway chassis.

One of these chassis would be active at a point and will become
entry point to traffic, bound for end points behind logical router
coming from outside network (North to South).

This patch make some enhancements to gateway chassis implementation
to manage above used case.

A.
Do not replace router port mac with chassis mac on gateway
chassis.
This is done, because:
i. Chassisredirect port is NOT a distributed port, hence
   we need not replace its mac address
  (which same as router port mac).

   ii. ARP cache will be consistent everywhere, i.e just like
   endpoints on OVN chassis will see configured router port
   mac as resolved mac for router port ip, outside endpoints
   will see that as well.

  iii. For implementing Network Address Translation. Although
   not a part of this series. But, follow up series would
   be having this feature and approach would rely upon
   sending packets to redirect chassis using chassis redirect
   router port mac as dest mac.

B.
Advertise router port GARP on gateway chassis.
This is needed, especially if a failover happens and
chassisredirect port moves to a new gateway chassis.
Otherwise, there would be packet drops till outside
router ARPs for router port ip again.

Intention of this GARP is to update top of the rack (TOR)
to direct router port mac to new hypervisor.

Hence, we could have done the same using RARP as well, but
because ovn-controller has implementation for GARP already,
hence it did not look like worthy to add a RARP implementation
just for this.

C.
For South to North traffic, we need not pass through gateway
chassis, if there is no address transalation needed.

For overlay networks, NATing is a must to talk to outside networks.
However, for vlan backed networks, NATing is not a must, and hence
in the absence of NATing configuration we need redirect the packet
to gateway chassis.

Signed-off-by: Ankur Sharma 
---
 ovn/controller/physical.c |  24 +++-
 ovn/controller/pinctrl.c  | 205 +++
 ovn/controller/pinctrl.h  |   6 +
 ovn/lib/ovn-util.c|  31 +
 ovn/lib/ovn-util.h|   6 +
 ovn/northd/ovn-northd.c   |  43 +--
 tests/ovn.at  | 307 +-
 7 files changed, 579 insertions(+), 43 deletions(-)

diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index d689e89..2069de1 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -21,6 +21,7 @@
 #include "lflow.h"
 #include "lport.h"
 #include "chassis.h"
+#include "pinctrl.h"
 #include "lib/bundle.h"
 #include "openvswitch/poll-loop.h"
 #include "lib/uuid.h"
@@ -235,9 +236,12 @@ get_zone_ids(const struct sbrec_port_binding *binding,
 }
 
 static void
-put_replace_router_port_mac_flows(const struct
+put_replace_router_port_mac_flows(struct ovsdb_idl_index
+  *sbrec_port_binding_by_name,
+  const struct
   sbrec_port_binding *localnet_port,
   const struct sbrec_chassis *chassis,
+  const struct sset *active_tunnels,
   const struct hmap *local_datapaths,
   struct ofpbuf *ofpacts_p,
   ofp_port_t ofport,
@@ -278,8 +282,21 @@ put_replace_router_port_mac_flows(const struct
 char *err_str = NULL;
 struct match match;
 struct ofpact_mac *replace_mac;
+char *cr_peer_name = xasprintf("cr-%s", rport_binding->logical_port);
 
-/* Table 65, priority 150.
+
+if (pinctrl_is_chassis_resident(sbrec_port_binding_by_name,
+chassis, active_tunnels,
+cr_peer_name)) {
+/* If a router port's chassisredirect port is
+ * resident on this chassis, then we need not do mac replace. */
+free(cr_peer_name);
+continue;
+}
+
+free(cr_peer_name);
+
+   /* Table 65, priority 150.
  * ===
  *
  * Implements output to localnet port.
@@ -792,7 +809,8 @@ consider_port_binding(struct ovsdb_idl_index 
*sbrec_port_binding_by_name,
 &match, ofpacts_p);
 
 if (!strcmp(binding->type,

[ovs-dev] [PATCH v5 1/2] OVN: Enable E-W Traffic, Vlan backed DVR

2019-05-02 Thread Ankur Sharma
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing

This Series:
Layer 3 E-W and Layer 3 N-S (NO NAT) changes for vlan
backed distributed logical router.

This patch:

A.
Key difference between an overlay logical switch and
vlan backed logical switch is that for vlan logical switches
packets are not encapsulated.

Hence, if a distributed router port is connected to vlan type
logical switch, then router port mac as source mac could be
seen from multiple hypervisors. Same  pairs coming
from multiple ports from a top of the rack switch (TOR) perspective
could be seen as a security threat and it could send alarms, drop
the packets or block the ports etc.

This patch addresses the same by introducing the concept of chassis mac.
A chassis mac is CMS provisioned unique mac per chassis. For any routed packet
(i.e source mac is router port mac) going on the wire on a vlan type
logical switch, we will replace its source mac with chassis mac.

This replacing of source mac with chassis mac will happen in table=65
of the logical switch datapath. A flow is added at priority 150, which
matches the source mac and replaces it with chassis mac if the value
is a router port mac.

Example flow:
cookie=0x0, duration=67765.830s, table=65, n_packets=0, n_bytes=0,
idle_age=65534, hard_age=65534, priority=150,reg15=0x1,metadata=0x4,
dl_src=00:00:01:01:02:03 actions=mod_dl_src:aa:bb:cc:dd:ee:ff,
mod_vlan_vid:1000,output:16

Here, 00:00:01:01:02:03 is router port mac and aa:bb:cc:dd:ee:ff
is chassis mac.

B.
This patch adds one more change of associating "types" with logical
switches. i.e a logical switch could be of type "overlay" or "bridged".
This is done to explicitly call out that on a bridged logical
switch there will no encapsulation.
Just a localnet port's presence is not sufficient, as we do
encap while redirecting the packet to gateway chassis.
By marking the logical switch as bridged, we can either
avoid redirection totally (if there is no NAT) or do redirection
based on router port mac, rather than encap over a tunnel.

Signed-off-by: Ankur Sharma 
---
 ovn/controller/binding.c|  12 +--
 ovn/controller/chassis.c|  66 +++-
 ovn/controller/chassis.h|   4 +
 ovn/controller/ovn-controller.8.xml |  10 ++
 ovn/controller/ovn-controller.c |   2 +-
 ovn/controller/ovn-controller.h |   5 +-
 ovn/controller/physical.c   |  96 ++
 ovn/northd/ovn-northd.c |  38 +++
 ovn/ovn-architecture.7.xml  |  12 +++
 ovn/ovn-nb.ovsschema|  10 +-
 ovn/ovn-nb.xml  |  18 
 ovn/ovn-sb.xml  |  15 +++
 ovn/utilities/ovn-nbctl.c   |  49 +++--
 tests/ovn-nbctl.at  |  48 ++---
 tests/ovn-northd.at |  22 
 tests/ovn.at| 197 
 16 files changed, 572 insertions(+), 32 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 404f0e7..e9461ef 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -159,13 +159,11 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
  sbrec_port_binding_by_name,
  peer->datapath, false,
  depth + 1, local_datapaths);
-ld->n_peer_dps++;
-ld->peer_dps = xrealloc(
-ld->peer_dps,
-ld->n_peer_dps * sizeof *ld->peer_dps);
-ld->peer_dps[ld->n_peer_dps - 1] = datapath_lookup_by_key(
-sbrec_datapath_binding_by_key,
-peer->datapath->tunnel_key);
+ld->n_peer_ports++;
+ld->peer_ports = xrealloc(ld->peer_ports,
+  ld->n_peer_ports *
+  sizeof *ld->peer_ports);
+ld->peer_ports[ld->n_peer_ports - 1] = peer;
 }
 }
 }
diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
index 0f537f1..617a7e1 100644
--- a/ovn/controller/chassis.c
+++ b/ovn/controller/chassis.c
@@ -23,6 +23,7 @@
 #include "lib/vswitch-idl.h"
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/ofp-parse.h"
 #include "ovn/lib/chassis-index.h"
 #include "ovn/lib/ovn-sb-idl.h"
 #include "ovn-controller.h"
@@ -69,6 +70,12 @@ get_bridge_mappings(const struct smap *ext_ids)
 }
 
 static const char *
+get_chassis_mac_mappings(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
+}
+
+static const char *
 get_cms_options(const struct smap *ext_ids)
 {
 r

Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan backed DVR

2019-05-02 Thread Ankur Sharma
Hi Numan,

Thanks for the feedback.
Just submitted the V5, please take a look.

Regards,
Ankur

From: Numan Siddique 
Sent: Thursday, May 2, 2019 1:54 AM
To: Ankur Sharma 
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan backed DVR



On Thu, May 2, 2019 at 6:40 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
Hi Numan,

Appreciate your feedback.
I tried removing the network_type  from the implementation and use localnet 
port’s presence as a criteria
For differentiating between overlay and vlan/provider backed networking. This 
landed the implementation
In a road block wrt N-S traffic.

Hence, something like network_type is needed to clarify the absence of encap of 
any kind.

I will make the changes as per your suggestion, and submit a V5 soon.
However, I still think the PROVIDER may not be the right term.

I am also considering the term “BRIDGED”. It aligns well with the documentation 
in ovn-nb.xml.
“
  There are two kinds of logical switches, that is, ones that fully
  virtualize the network (overlay logical switches) and ones that provide
  simple connectivity to a physical network (bridged logical switches).
  They work in the same way when providing connectivity between logical
  ports on same chasis, but differently when connecting remote logical
  ports.  Overlay logical switches connect remote logical ports by tunnels,
  while bridged logical switches provide connectivity to remote ports by
  bridging the packets to directly connected physical L2 segment with the
  help of localnet ports.  Each bridged logical switch has
  one and only one localnet port, which has only one special
  address unknown.
“

Let me know your thoughts. If you still want to use PROVIDER then I will use 
that in V5.

Hi Ankur,

The type = "BRIDGED" seems more appropriate to me.

Thanks
Numan



Thanks again for review.

Regards,
Ankur

From: Numan Siddique mailto:nusid...@redhat.com>>
Sent: Monday, April 29, 2019 3:55 AM
To: Ankur Sharma mailto:ankur.sha...@nutanix.com>>
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan backed DVR



On Fri, Apr 26, 2019 at 5:47 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
Hi Numan,

Glad to see your review comments.
Sorry, I missed your feedback on flat networks earlier.

a. The idea behind network_type is just to indicate if packets will be 
encapsulated or not.

b. Having type as “vlan” is a way of saying that packets will be tagged if 
there is a native vlan configured on the switch.
Otherwise it will got out as untagged.

c. Yes, comment regarding going through ports was not correct (will rectify it 
in v5), however I see value in having network_type.
it will be helpful in debugging and coding. In code, checking network_type  
and making some decisions looks better to me,
rather than checking for localnet ports in the datapath.

If you think its good to have network_type, then I would suggest to have 2 
values - "overload" and "provider" (instead of "vlan").


d. Having said that, I agree that we can live without for now. If we do not 
hear much from other reviewers by early next week, then i
will remove network_type related changes.

My suggestion would be to have something like below in ovn-northd.c


enum ovn_datapath_nw_type {
DP_NETWORK_OVERLAY,
DP_NETWORK_PROVIDER
};

static void
ovn_datapath_update_nw_type(struct ovn_datapath *od)
{
if (!od->nbs) {
return;
}

if (!od->localnet_port) {
od->network_type = DP_NETWORK_OVERLAY;
} else {
od->network_type = DP_NETWORK_PROVIDER;
}
}
**

and use ``od->network_type`` in rest of the code.

Also the present patch displays the network_type in "ovn-nbctl show" command. 
It would be nice to still display that
in case you remove the "network_type" from the Logical_Switch table.

Thanks
Numan


Please find my responses on inline on rest of the comments. Will make these 
changes in v5.

Thanks

Regards,
Ankur



From: Numan Siddique mailto:nusid...@redhat.com>>
Sent: Thursday, April 25, 2019 1:40 AM
To: Ankur Sharma mailto:ankur.sha...@nutanix.com>>
Cc: ovs-dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v4 1/2] OVN: Enable E-W Traffic, Vlan backed DVR



On Thu, Apr 25, 2019 at 5:46 AM Ankur Sharma 
mailto:ankur.sha...@nutanix.com>> wrote:
Background:
[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/353066.html 
[mail.openvswitch.org]
[2] 
https://docs.google.com/document/d/1uoQH478wM1OZ16HrxzbOUvk5LvFnfNEWbkPT6Zmm9OU/edit?usp=sharing
 
[doc

[ovs-dev] [branch-2.9 1/3] conntrack: Fix race for NAT cleanup.

2019-05-02 Thread Darrell Ball
Reference lists are not fully protected during cleanup of
NAT connections where the bucket lock is transiently not held during
list traversal.  This can lead to referencing freed memory during
cleaning from multiple contexts.  Fix this by protecting with
the existing 'cleanup' mutex in the missed cases where 'conn_clean()'
is called.  'conntrack_flush()' is converted to expiry list traversal
to support the proper bucket level protection with the 'cleanup' mutex.

The NAT exhaustion case cleanup in 'conn_not_found()' is also modified
to avoid the same issue.

Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
Reported-by: solomon 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357056.html
Tested-by: solomon 
Signed-off-by: Darrell Ball 
---
 lib/conntrack.c | 129 +++-
 1 file changed, 90 insertions(+), 39 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 7616758..d552e3b 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -353,7 +353,7 @@ conntrack_destroy(struct conntrack *ct)
 struct conntrack_bucket *ctb = &ct->buckets[i];
 struct conn *conn;
 
-ovs_mutex_destroy(&ctb->cleanup_mutex);
+ovs_mutex_lock(&ctb->cleanup_mutex);
 ct_lock_lock(&ctb->lock);
 HMAP_FOR_EACH_POP (conn, node, &ctb->connections) {
 if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
@@ -363,7 +363,9 @@ conntrack_destroy(struct conntrack *ct)
 }
 hmap_destroy(&ctb->connections);
 ct_lock_unlock(&ctb->lock);
+ovs_mutex_unlock(&ctb->cleanup_mutex);
 ct_lock_destroy(&ctb->lock);
+ovs_mutex_destroy(&ctb->cleanup_mutex);
 }
 ct_rwlock_wrlock(&ct->resources_lock);
 struct nat_conn_key_node *nat_conn_key_node;
@@ -754,6 +756,27 @@ conn_lookup(struct conntrack *ct, const struct conn_key 
*key, long long now)
 return ctx.conn;
 }
 
+/* Only used when looking up 'CT_CONN_TYPE_DEFAULT' conns. */
+static struct conn *
+conn_lookup_def(const struct conn_key *key,
+const struct conntrack_bucket *ctb, uint32_t hash)
+OVS_REQUIRES(ctb->lock)
+{
+struct conn *conn = NULL;
+
+HMAP_FOR_EACH_WITH_HASH (conn, node, hash, &ctb->connections) {
+if (!conn_key_cmp(&conn->key, key)
+&& conn->conn_type == CT_CONN_TYPE_DEFAULT) {
+break;
+}
+if (!conn_key_cmp(&conn->rev_key, key)
+&& conn->conn_type == CT_CONN_TYPE_DEFAULT) {
+break;
+}
+}
+return conn;
+}
+
 static void
 conn_seq_skew_set(struct conntrack *ct, const struct conn_key *key,
   long long now, int seq_skew, bool seq_skew_dir)
@@ -821,6 +844,22 @@ conn_clean(struct conntrack *ct, struct conn *conn,
 }
 }
 
+/* Only called for 'CT_CONN_TYPE_DEFAULT' conns; must be called with no
+ * locks held and upon return no locks are held. */
+static void
+conn_clean_safe(struct conntrack *ct, struct conn *conn,
+struct conntrack_bucket *ctb, uint32_t hash)
+{
+ovs_mutex_lock(&ctb->cleanup_mutex);
+ct_lock_lock(&ctb->lock);
+conn = conn_lookup_def(&conn->key, ctb, hash);
+if (conn) {
+conn_clean(ct, conn, ctb);
+}
+ct_lock_unlock(&ctb->lock);
+ovs_mutex_unlock(&ctb->cleanup_mutex);
+}
+
 static bool
 ct_verify_helper(const char *helper, enum ct_alg_ctl_type ct_alg_ctl)
 {
@@ -852,6 +891,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
enum ct_alg_ctl_type ct_alg_ctl)
 {
 struct conn *nc = NULL;
+struct conn connl;
 
 if (!valid_new(pkt, &ctx->key)) {
 pkt->md.ct_state = CS_INVALID;
@@ -874,8 +914,9 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
 }
 
 unsigned bucket = hash_to_bucket(ctx->hash);
-nc = new_conn(&ct->buckets[bucket], pkt, &ctx->key, now);
-ctx->conn = nc;
+nc = &connl;
+memset(nc, 0, sizeof *nc);
+memcpy(&nc->key, &ctx->key, sizeof nc->key);
 nc->rev_key = nc->key;
 conn_key_reverse(&nc->rev_key);
 
@@ -919,6 +960,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
 ct_rwlock_wrlock(&ct->resources_lock);
 bool nat_res = nat_select_range_tuple(ct, nc,
   conn_for_un_nat_copy);
+ct_rwlock_unlock(&ct->resources_lock);
 
 if (!nat_res) {
 goto nat_res_exhaustion;
@@ -927,14 +969,24 @@ conn_not_found(struct conntrack *ct, struct dp_packet 
*pkt,
 /* Update nc with nat adjustments made to
  * conn_for_un_nat_copy by nat_select_range_tuple(). */
 *nc = *conn_for_un_nat_copy;
-ct_rwlock_unlock(&ct->resources_lock);
 }
 conn_for_un_nat_copy->conn_type = CT_CONN_TYPE_UN_NAT;
 conn_for_un_nat_copy->nat_info = NUL

[ovs-dev] [branch-2.9 2/3] conntrack: Lookup only 'UNNAT conns' in 'nat_clean()'.

2019-05-02 Thread Darrell Ball
When freeing 'UNNAT conns', lookup only 'UNNAT conns' to
protect against possible address overlap with 'default
conns' during a DOS attempt.  This is very unlikely, but
protection is simple.

Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
Signed-off-by: Darrell Ball 
---
 lib/conntrack.c | 25 +
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index d552e3b..042b94e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -777,6 +777,22 @@ conn_lookup_def(const struct conn_key *key,
 return conn;
 }
 
+static struct conn *
+conn_lookup_unnat(const struct conn_key *key,
+  const struct conntrack_bucket *ctb, uint32_t hash)
+OVS_REQUIRES(ctb->lock)
+{
+struct conn *conn = NULL;
+
+HMAP_FOR_EACH_WITH_HASH (conn, node, hash, &ctb->connections) {
+if (!conn_key_cmp(&conn->key, key)
+&& conn->conn_type == CT_CONN_TYPE_UN_NAT) {
+break;
+}
+}
+return conn;
+}
+
 static void
 conn_seq_skew_set(struct conntrack *ct, const struct conn_key *key,
   long long now, int seq_skew, bool seq_skew_dir)
@@ -800,12 +816,13 @@ nat_clean(struct conntrack *ct, struct conn *conn,
 nat_conn_keys_remove(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis);
 ct_rwlock_unlock(&ct->resources_lock);
 ct_lock_unlock(&ctb->lock);
-unsigned bucket_rev_conn =
-hash_to_bucket(conn_key_hash(&conn->rev_key, ct->hash_basis));
+uint32_t hash = conn_key_hash(&conn->rev_key, ct->hash_basis);
+unsigned bucket_rev_conn = hash_to_bucket(hash);
 ct_lock_lock(&ct->buckets[bucket_rev_conn].lock);
 ct_rwlock_wrlock(&ct->resources_lock);
-long long now = time_msec();
-struct conn *rev_conn = conn_lookup(ct, &conn->rev_key, now);
+struct conn *rev_conn = conn_lookup_unnat(&conn->rev_key,
+  &ct->buckets[bucket_rev_conn],
+  hash);
 struct nat_conn_key_node *nat_conn_key_node =
 nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key,
  ct->hash_basis);
-- 
1.9.1

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


[ovs-dev] [branch-2.9 3/3] conntrack: Replace structure copy by memcpy().

2019-05-02 Thread Darrell Ball
There are a few cases where structure copy can be replaced by
memcpy(), for possible portability benefit.  This is because
the structures involved have padding and elements of the
structure are used to generate hashes.

Signed-off-by: Darrell Ball 
---
 lib/conntrack.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 042b94e..0d71195 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -934,7 +934,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
 nc = &connl;
 memset(nc, 0, sizeof *nc);
 memcpy(&nc->key, &ctx->key, sizeof nc->key);
-nc->rev_key = nc->key;
+memcpy(&nc->rev_key, &nc->key, sizeof nc->rev_key);
 conn_key_reverse(&nc->rev_key);
 
 if (ct_verify_helper(helper, ct_alg_ctl)) {
@@ -985,7 +985,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
 
 /* Update nc with nat adjustments made to
  * conn_for_un_nat_copy by nat_select_range_tuple(). */
-*nc = *conn_for_un_nat_copy;
+memcpy(nc, conn_for_un_nat_copy, sizeof *nc);
 }
 conn_for_un_nat_copy->conn_type = CT_CONN_TYPE_UN_NAT;
 conn_for_un_nat_copy->nat_info = NULL;
@@ -1076,8 +1076,8 @@ create_un_nat_conn(struct conntrack *ct, struct conn 
*conn_for_un_nat_copy,
long long now, bool alg_un_nat)
 {
 struct conn *nc = xmemdup(conn_for_un_nat_copy, sizeof *nc);
-nc->key = conn_for_un_nat_copy->rev_key;
-nc->rev_key = conn_for_un_nat_copy->key;
+memcpy(&nc->key, &conn_for_un_nat_copy->rev_key, sizeof nc->key);
+memcpy(&nc->rev_key, &conn_for_un_nat_copy->key, sizeof nc->rev_key);
 uint32_t un_nat_hash = conn_key_hash(&nc->key, ct->hash_basis);
 unsigned un_nat_conn_bucket = hash_to_bucket(un_nat_hash);
 ct_lock_lock(&ct->buckets[un_nat_conn_bucket].lock);
@@ -1266,7 +1266,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
 
 struct conn_lookup_ctx ctx2;
 ctx2.conn = NULL;
-ctx2.key = conn->rev_key;
+memcpy(&ctx2.key, &conn->rev_key, sizeof ctx2.key);
 ctx2.hash = conn_key_hash(&conn->rev_key, ct->hash_basis);
 
 ct_lock_unlock(&ct->buckets[bucket].lock);
@@ -1352,7 +1352,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
 
 struct conn conn_for_expectation;
 if (OVS_UNLIKELY((ct_alg_ctl != CT_ALG_CTL_NONE) && conn)) {
-conn_for_expectation = *conn;
+memcpy(&conn_for_expectation, conn, sizeof conn_for_expectation);
 }
 
 ct_lock_unlock(&ct->buckets[bucket].lock);
@@ -2334,8 +2334,10 @@ nat_conn_keys_insert(struct hmap *nat_conn_keys, const 
struct conn *nat_conn,
 
 if (!nat_conn_key_node) {
 struct nat_conn_key_node *nat_conn_key = xzalloc(sizeof *nat_conn_key);
-nat_conn_key->key = nat_conn->rev_key;
-nat_conn_key->value = nat_conn->key;
+memcpy(&nat_conn_key->key, &nat_conn->rev_key,
+   sizeof nat_conn_key->key);
+memcpy(&nat_conn_key->value, &nat_conn->key,
+   sizeof nat_conn_key->value);
 hmap_insert(nat_conn_keys, &nat_conn_key->node,
 conn_key_hash(&nat_conn_key->key, basis));
 return true;
@@ -2741,7 +2743,8 @@ expectation_create(struct conntrack *ct, ovs_be16 
dst_port,
 alg_exp_node->key.dst.port = dst_port;
 alg_exp_node->master_mark = master_conn->mark;
 alg_exp_node->master_label = master_conn->label;
-alg_exp_node->master_key = master_conn->key;
+memcpy(&alg_exp_node->master_key, &master_conn->key,
+   sizeof alg_exp_node->master_key);
 /* Take the write lock here because it is almost 100%
  * likely that the lookup will fail and
  * expectation_create() will be called below. */
-- 
1.9.1

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


[ovs-dev] [patch v4 0/2] conntrack: Optimize and refactor.

2019-05-02 Thread Darrell Ball
Patch 1 is not an optimization per se, but aimed at eliminating
the exporting of internal conntrack infra.

Patch 2 Add RCU support.  This is mainly done for performance 
reasons, but also simplifies the code.

v4: Just include first 2 patches as the other patches are moved to a 
subsequent series.

Reinstated support for running multiple userpace datapaths at the
same time.

Fixed a bug where 'ovs_list_remove()' was misplaced.

Cleaned up some unneeded code.

v3: Dropped Patch 3 from the series since it was broken/incomplete
and not worth the fix, since there are some disadvantages in
terms of maintainability, including treating UDP/ICMP different
than TCP, while the performance benefit was borderline.

Fixed bug in 'nat_res_exhaustion' exception code.

Cleaned up a few APIs - 'conn_clean*'/'delete_conn*' and 
removed an unneeded one, 'conn_available()'.  Added more
comments.

Removed non-essential 'nat' field from struct conn_lookup_ctx.

Fixed some splicing issues b/w Patch 1 and Patch 2, where some
aspects of Patch 2 were now moved to Patch 1.

v2: Put back somehow deleted '&' in Patch 1 in call to
check_orig_tuple() for bucket parameter (which is
removed in Patch 2).

Darrell Ball (2):
  conntrack: Stop exporting internal datastructures.
  conntrack: Add rcu support.

 lib/conntrack-icmp.c|  25 +-
 lib/conntrack-other.c   |  12 +-
 lib/conntrack-private.h | 126 +--
 lib/conntrack-tcp.c |  20 +-
 lib/conntrack.c | 934 
 lib/conntrack.h | 188 +-
 lib/dpif-netdev.c   |  32 +-
 tests/test-conntrack.c  |  14 +-
 8 files changed, 461 insertions(+), 890 deletions(-)

-- 
1.9.1

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


[ovs-dev] [patch v4 1/2] conntrack: Stop exporting internal datastructures.

2019-05-02 Thread Darrell Ball
Stop the exporting of the main internal conntrack datastructure.

Signed-off-by: Darrell Ball 
---
 lib/conntrack-private.h | 174 +
 lib/conntrack.c |   8 ++-
 lib/conntrack.h | 184 +---
 lib/dpif-netdev.c   |  32 -
 tests/test-conntrack.c  |  14 ++--
 5 files changed, 205 insertions(+), 207 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 96a6a9f..059af9e 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -119,6 +119,180 @@ enum ct_conn_type {
 CT_CONN_TYPE_UN_NAT,
 };
 
+/* 'struct ct_lock' is a wrapper for an adaptive mutex.  It's useful to try
+ * different types of locks (e.g. spinlocks) */
+
+struct OVS_LOCKABLE ct_lock {
+struct ovs_mutex lock;
+};
+
+static inline void ct_lock_init(struct ct_lock *lock)
+{
+ovs_mutex_init_adaptive(&lock->lock);
+}
+
+static inline void ct_lock_lock(struct ct_lock *lock)
+OVS_ACQUIRES(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_mutex_lock(&lock->lock);
+}
+
+static inline void ct_lock_unlock(struct ct_lock *lock)
+OVS_RELEASES(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_mutex_unlock(&lock->lock);
+}
+
+static inline void ct_lock_destroy(struct ct_lock *lock)
+{
+ovs_mutex_destroy(&lock->lock);
+}
+
+struct OVS_LOCKABLE ct_rwlock {
+struct ovs_rwlock lock;
+};
+
+static inline void ct_rwlock_init(struct ct_rwlock *lock)
+{
+ovs_rwlock_init(&lock->lock);
+}
+
+
+static inline void ct_rwlock_wrlock(struct ct_rwlock *lock)
+OVS_ACQ_WRLOCK(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_rwlock_wrlock(&lock->lock);
+}
+
+static inline void ct_rwlock_rdlock(struct ct_rwlock *lock)
+OVS_ACQ_RDLOCK(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_rwlock_rdlock(&lock->lock);
+}
+
+static inline void ct_rwlock_unlock(struct ct_rwlock *lock)
+OVS_RELEASES(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_rwlock_unlock(&lock->lock);
+}
+
+static inline void ct_rwlock_destroy(struct ct_rwlock *lock)
+{
+ovs_rwlock_destroy(&lock->lock);
+}
+
+/* Timeouts: all the possible timeout states passed to update_expiration()
+ * are listed here. The name will be prefix by CT_TM_ and the value is in
+ * milliseconds */
+#define CT_TIMEOUTS \
+CT_TIMEOUT(TCP_FIRST_PACKET, 30 * 1000) \
+CT_TIMEOUT(TCP_OPENING, 30 * 1000) \
+CT_TIMEOUT(TCP_ESTABLISHED, 24 * 60 * 60 * 1000) \
+CT_TIMEOUT(TCP_CLOSING, 15 * 60 * 1000) \
+CT_TIMEOUT(TCP_FIN_WAIT, 45 * 1000) \
+CT_TIMEOUT(TCP_CLOSED, 30 * 1000) \
+CT_TIMEOUT(OTHER_FIRST, 60 * 1000) \
+CT_TIMEOUT(OTHER_MULTIPLE, 60 * 1000) \
+CT_TIMEOUT(OTHER_BIDIR, 30 * 1000) \
+CT_TIMEOUT(ICMP_FIRST, 60 * 1000) \
+CT_TIMEOUT(ICMP_REPLY, 30 * 1000)
+
+/* The smallest of the above values: it is used as an upper bound for the
+ * interval between two rounds of cleanup of expired entries */
+#define CT_TM_MIN (30 * 1000)
+
+#define CT_TIMEOUT(NAME, VAL) BUILD_ASSERT_DECL(VAL >= CT_TM_MIN);
+CT_TIMEOUTS
+#undef CT_TIMEOUT
+
+enum ct_timeout {
+#define CT_TIMEOUT(NAME, VALUE) CT_TM_##NAME,
+CT_TIMEOUTS
+#undef CT_TIMEOUT
+N_CT_TM
+};
+
+
+/* Locking:
+ *
+ * The connections are kept in different buckets, which are completely
+ * independent. The connection bucket is determined by the hash of its key.
+ *
+ * Each bucket has two locks. Acquisition order is, from outermost to
+ * innermost:
+ *
+ *cleanup_mutex
+ *lock
+ *
+ * */
+struct conntrack_bucket {
+/* Protects 'connections' and 'exp_lists'.  Used in the fast path */
+struct ct_lock lock;
+/* Contains the connections in the bucket, indexed by 'struct conn_key' */
+struct hmap connections OVS_GUARDED;
+/* For each possible timeout we have a list of connections. When the
+ * timeout of a connection is updated, we move it to the back of the list.
+ * Since the connection in a list have the same relative timeout, the list
+ * will be ordered, with the oldest connections to the front. */
+struct ovs_list exp_lists[N_CT_TM] OVS_GUARDED;
+
+/* Protects 'next_cleanup'. Used to make sure that there's only one thread
+ * performing the cleanup. */
+struct ovs_mutex cleanup_mutex;
+long long next_cleanup OVS_GUARDED;
+};
+
+#define CONNTRACK_BUCKETS_SHIFT 8
+#define CONNTRACK_BUCKETS (1 << CONNTRACK_BUCKETS_SHIFT)
+
+struct conntrack {
+/* Independent buckets containing the connections */
+struct conntrack_bucket buckets[CONNTRACK_BUCKETS];
+
+/* Salt for hashing a connection key. */
+uint32_t hash_basis;
+/* The thread performing periodic cleanup of the connection
+ * tracker. */
+pthread_t clean_thread;
+/* Latch to destroy the 'clean_thread' */
+struct latch clean_thread_exit;
+
+/* Number of connections currently in the connection tracker. */
+atomic_count n_conn;
+/* Connections limit. When this limit is reached, no 

[ovs-dev] [patch v4 2/2] conntrack: Add rcu support.

2019-05-02 Thread Darrell Ball
For performance and code simplification reasons, add rcu support for
conntrack. The array of hmaps is replaced by a cmap as part of this
conversion.  Using a single map also simplifies the handling of NAT
and allows the removal of the nat_conn map and friends.  Per connection
entry locks are introduced, which are needed in a few code paths.

Signed-off-by: Darrell Ball 
---
 lib/conntrack-icmp.c|  25 +-
 lib/conntrack-other.c   |  12 +-
 lib/conntrack-private.h | 186 +++---
 lib/conntrack-tcp.c |  20 +-
 lib/conntrack.c | 928 
 lib/conntrack.h |   4 +-
 6 files changed, 374 insertions(+), 801 deletions(-)

diff --git a/lib/conntrack-icmp.c b/lib/conntrack-icmp.c
index 40fd1d8..f00a4c4 100644
--- a/lib/conntrack-icmp.c
+++ b/lib/conntrack-icmp.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015-2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -46,16 +46,12 @@ conn_icmp_cast(const struct conn *conn)
 }
 
 static enum ct_update_res
-icmp_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
- struct dp_packet *pkt OVS_UNUSED, bool reply, long long now)
+icmp_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
+ bool reply, long long now)
 {
 struct conn_icmp *conn = conn_icmp_cast(conn_);
-
-if (reply && conn->state != ICMPS_REPLY) {
-conn->state = ICMPS_REPLY;
-}
-
-conn_update_expiration(ctb, &conn->up, icmp_timeouts[conn->state], now);
+conn->state = reply ? ICMPS_REPLY : ICMPS_FIRST;
+conn_update_expiration(&conn->up, icmp_timeouts[conn->state], now);
 
 return CT_UPDATE_VALID;
 }
@@ -79,15 +75,12 @@ icmp6_valid_new(struct dp_packet *pkt)
 }
 
 static struct conn *
-icmp_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt OVS_UNUSED,
-   long long now)
+icmp_new_conn(struct conntrack *ct, struct dp_packet *pkt OVS_UNUSED,
+  long long now)
 {
-struct conn_icmp *conn;
-
-conn = xzalloc(sizeof *conn);
+struct conn_icmp *conn = xzalloc(sizeof *conn);
 conn->state = ICMPS_FIRST;
-
-conn_init_expiration(ctb, &conn->up, icmp_timeouts[conn->state], now);
+conn_init_expiration(ct, &conn->up, icmp_timeouts[conn->state], now);
 
 return &conn->up;
 }
diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
index 2920889..a539f18 100644
--- a/lib/conntrack-other.c
+++ b/lib/conntrack-other.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015-2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,8 +43,8 @@ conn_other_cast(const struct conn *conn)
 }
 
 static enum ct_update_res
-other_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
-  struct dp_packet *pkt OVS_UNUSED, bool reply, long long now)
+other_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
+  bool reply, long long now)
 {
 struct conn_other *conn = conn_other_cast(conn_);
 
@@ -54,7 +54,7 @@ other_conn_update(struct conn *conn_, struct conntrack_bucket 
*ctb,
 conn->state = OTHERS_MULTIPLE;
 }
 
-conn_update_expiration(ctb, &conn->up, other_timeouts[conn->state], now);
+conn_update_expiration(&conn->up, other_timeouts[conn->state], now);
 
 return CT_UPDATE_VALID;
 }
@@ -66,7 +66,7 @@ other_valid_new(struct dp_packet *pkt OVS_UNUSED)
 }
 
 static struct conn *
-other_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt OVS_UNUSED,
+other_new_conn(struct conntrack *ct, struct dp_packet *pkt OVS_UNUSED,
long long now)
 {
 struct conn_other *conn;
@@ -74,7 +74,7 @@ other_new_conn(struct conntrack_bucket *ctb, struct dp_packet 
*pkt OVS_UNUSED,
 conn = xzalloc(sizeof *conn);
 conn->state = OTHERS_FIRST;
 
-conn_init_expiration(ctb, &conn->up, other_timeouts[conn->state], now);
+conn_init_expiration(ct, &conn->up, other_timeouts[conn->state], now);
 
 return &conn->up;
 }
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 059af9e..b60094d 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
+ * Copyright (c) 2015-2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,8 +21,10 @@
 #include 
 #include 
 
+#include "cmap.h"
 #include "conntrack.h"
 #include "ct-dpif.h"
+#include "ipf.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/list.h"
 #include "openvswitch/types.h"
@@ -57,12 +59,6 @@ struct conn_key {
 uint8_t nw_proto;
 };
 
-struct nat_conn_key_node {
-struct hmap_node node;
-struc

[ovs-dev] Home Office

2019-05-02 Thread Supervisión y control del trabajo desde casa
Webinar Interactivo – Viernes 17 de Mayo

Guía para la implementación del Home Office

Hace algunos años era impensable trabajar desde casa. Ahora esta tendencia 
incluso podría representar utilidades
a la organización, reducir costos y potenciar la productividad de nuestros 
empleados. 

Revisaremos los conceptos del Home Offce así como el proceso deimplementación 
en una empresa. 
 

Ejes Temáticos:

• Definición de conceptos básicos.

• Evaluación de situación actual y potencial de la organización para ésta 
práctica.

• Proceso a seguir para la correcta implementación.

• Supervisión y control del trabajo desde casa.


Para mayor información, responder sobre este correo con la palabra HOME + los 
siguientes datos:


NOMBRE:

TELÉFONO:

EMPRESA:

CORREO ALTERNO: 


 Llamanos al (045) 55 1554 6630
www.Innovalearn.mx 


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


Re: [ovs-dev] [patch v4 2/2] conntrack: Add rcu support.

2019-05-02 Thread Darrell Ball
ignore this version of this patch - I will resend with a small incremental
later today

Darrell

On Thu, May 2, 2019 at 5:37 PM Darrell Ball  wrote:

> For performance and code simplification reasons, add rcu support for
> conntrack. The array of hmaps is replaced by a cmap as part of this
> conversion.  Using a single map also simplifies the handling of NAT
> and allows the removal of the nat_conn map and friends.  Per connection
> entry locks are introduced, which are needed in a few code paths.
>
> Signed-off-by: Darrell Ball 
> ---
>  lib/conntrack-icmp.c|  25 +-
>  lib/conntrack-other.c   |  12 +-
>  lib/conntrack-private.h | 186 +++---
>  lib/conntrack-tcp.c |  20 +-
>  lib/conntrack.c | 928
> 
>  lib/conntrack.h |   4 +-
>  6 files changed, 374 insertions(+), 801 deletions(-)
>
> diff --git a/lib/conntrack-icmp.c b/lib/conntrack-icmp.c
> index 40fd1d8..f00a4c4 100644
> --- a/lib/conntrack-icmp.c
> +++ b/lib/conntrack-icmp.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2015, 2016 Nicira, Inc.
> + * Copyright (c) 2015-2019 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -46,16 +46,12 @@ conn_icmp_cast(const struct conn *conn)
>  }
>
>  static enum ct_update_res
> -icmp_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
> - struct dp_packet *pkt OVS_UNUSED, bool reply, long long
> now)
> +icmp_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
> + bool reply, long long now)
>  {
>  struct conn_icmp *conn = conn_icmp_cast(conn_);
> -
> -if (reply && conn->state != ICMPS_REPLY) {
> -conn->state = ICMPS_REPLY;
> -}
> -
> -conn_update_expiration(ctb, &conn->up, icmp_timeouts[conn->state],
> now);
> +conn->state = reply ? ICMPS_REPLY : ICMPS_FIRST;
> +conn_update_expiration(&conn->up, icmp_timeouts[conn->state], now);
>
>  return CT_UPDATE_VALID;
>  }
> @@ -79,15 +75,12 @@ icmp6_valid_new(struct dp_packet *pkt)
>  }
>
>  static struct conn *
> -icmp_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt
> OVS_UNUSED,
> -   long long now)
> +icmp_new_conn(struct conntrack *ct, struct dp_packet *pkt OVS_UNUSED,
> +  long long now)
>  {
> -struct conn_icmp *conn;
> -
> -conn = xzalloc(sizeof *conn);
> +struct conn_icmp *conn = xzalloc(sizeof *conn);
>  conn->state = ICMPS_FIRST;
> -
> -conn_init_expiration(ctb, &conn->up, icmp_timeouts[conn->state], now);
> +conn_init_expiration(ct, &conn->up, icmp_timeouts[conn->state], now);
>
>  return &conn->up;
>  }
> diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
> index 2920889..a539f18 100644
> --- a/lib/conntrack-other.c
> +++ b/lib/conntrack-other.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2015, 2016 Nicira, Inc.
> + * Copyright (c) 2015-2019 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -43,8 +43,8 @@ conn_other_cast(const struct conn *conn)
>  }
>
>  static enum ct_update_res
> -other_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
> -  struct dp_packet *pkt OVS_UNUSED, bool reply, long long
> now)
> +other_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
> +  bool reply, long long now)
>  {
>  struct conn_other *conn = conn_other_cast(conn_);
>
> @@ -54,7 +54,7 @@ other_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>  conn->state = OTHERS_MULTIPLE;
>  }
>
> -conn_update_expiration(ctb, &conn->up, other_timeouts[conn->state],
> now);
> +conn_update_expiration(&conn->up, other_timeouts[conn->state], now);
>
>  return CT_UPDATE_VALID;
>  }
> @@ -66,7 +66,7 @@ other_valid_new(struct dp_packet *pkt OVS_UNUSED)
>  }
>
>  static struct conn *
> -other_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt
> OVS_UNUSED,
> +other_new_conn(struct conntrack *ct, struct dp_packet *pkt OVS_UNUSED,
> long long now)
>  {
>  struct conn_other *conn;
> @@ -74,7 +74,7 @@ other_new_conn(struct conntrack_bucket *ctb, struct
> dp_packet *pkt OVS_UNUSED,
>  conn = xzalloc(sizeof *conn);
>  conn->state = OTHERS_FIRST;
>
> -conn_init_expiration(ctb, &conn->up, other_timeouts[conn->state],
> now);
> +conn_init_expiration(ct, &conn->up, other_timeouts[conn->state], now);
>
>  return &conn->up;
>  }
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index 059af9e..b60094d 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
> + * Copyright (c) 2015-2019 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use t

[ovs-dev] [patch v5 0/2] conntrack: Optimize and refactor.

2019-05-02 Thread Darrell Ball
Patch 1 is not an optimization per se, but aimed at eliminating
the exporting of internal conntrack infra.

Patch 2 Add RCU support.  This is mainly done for performance 
reasons, but also simplifies the code.

v5: Removed a wayward ovs_list_remove() from conn_update_expiration().

Merged some conn level locking that was missed b4 and adjusted const
specifier for conn, as a result.

v4: Just include first 2 patches as the other patches are moved to a 
subsequent series.

Reinstated support for running multiple userpace datapaths at the
same time.

Fixed a bug where 'ovs_list_remove()' was misplaced.

Cleaned up some unneeded code.

v3: Dropped Patch 3 from the series since it was broken/incomplete
and not worth the fix, since there are some disadvantages in
terms of maintainability, including treating UDP/ICMP different
than TCP, while the performance benefit was borderline.

Fixed bug in 'nat_res_exhaustion' exception code.

Cleaned up a few APIs - 'conn_clean*'/'delete_conn*' and 
removed an unneeded one, 'conn_available()'.  Added more
comments.

Removed non-essential 'nat' field from struct conn_lookup_ctx.

Fixed some splicing issues b/w Patch 1 and Patch 2, where some
aspects of Patch 2 were now moved to Patch 1.

v2: Put back somehow deleted '&' in Patch 1 in call to
check_orig_tuple() for bucket parameter (which is
removed in Patch 2).

Darrell Ball (2):
  conntrack: Stop exporting internal datastructures.
  conntrack: Add rcu support.

 lib/conntrack-icmp.c|  25 +-
 lib/conntrack-other.c   |  12 +-
 lib/conntrack-private.h | 127 +--
 lib/conntrack-tcp.c |  20 +-
 lib/conntrack.c | 973 +---
 lib/conntrack.h | 188 +-
 lib/dpif-netdev.c   |  32 +-
 tests/test-conntrack.c  |  14 +-
 8 files changed, 484 insertions(+), 907 deletions(-)

-- 
1.9.1

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


[ovs-dev] [patch v5 1/2] conntrack: Stop exporting internal datastructures.

2019-05-02 Thread Darrell Ball
Stop the exporting of the main internal conntrack datastructure.

Signed-off-by: Darrell Ball 
---
 lib/conntrack-private.h | 174 +
 lib/conntrack.c |   8 ++-
 lib/conntrack.h | 184 +---
 lib/dpif-netdev.c   |  32 -
 tests/test-conntrack.c  |  14 ++--
 5 files changed, 205 insertions(+), 207 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 96a6a9f..059af9e 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -119,6 +119,180 @@ enum ct_conn_type {
 CT_CONN_TYPE_UN_NAT,
 };
 
+/* 'struct ct_lock' is a wrapper for an adaptive mutex.  It's useful to try
+ * different types of locks (e.g. spinlocks) */
+
+struct OVS_LOCKABLE ct_lock {
+struct ovs_mutex lock;
+};
+
+static inline void ct_lock_init(struct ct_lock *lock)
+{
+ovs_mutex_init_adaptive(&lock->lock);
+}
+
+static inline void ct_lock_lock(struct ct_lock *lock)
+OVS_ACQUIRES(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_mutex_lock(&lock->lock);
+}
+
+static inline void ct_lock_unlock(struct ct_lock *lock)
+OVS_RELEASES(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_mutex_unlock(&lock->lock);
+}
+
+static inline void ct_lock_destroy(struct ct_lock *lock)
+{
+ovs_mutex_destroy(&lock->lock);
+}
+
+struct OVS_LOCKABLE ct_rwlock {
+struct ovs_rwlock lock;
+};
+
+static inline void ct_rwlock_init(struct ct_rwlock *lock)
+{
+ovs_rwlock_init(&lock->lock);
+}
+
+
+static inline void ct_rwlock_wrlock(struct ct_rwlock *lock)
+OVS_ACQ_WRLOCK(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_rwlock_wrlock(&lock->lock);
+}
+
+static inline void ct_rwlock_rdlock(struct ct_rwlock *lock)
+OVS_ACQ_RDLOCK(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_rwlock_rdlock(&lock->lock);
+}
+
+static inline void ct_rwlock_unlock(struct ct_rwlock *lock)
+OVS_RELEASES(lock)
+OVS_NO_THREAD_SAFETY_ANALYSIS
+{
+ovs_rwlock_unlock(&lock->lock);
+}
+
+static inline void ct_rwlock_destroy(struct ct_rwlock *lock)
+{
+ovs_rwlock_destroy(&lock->lock);
+}
+
+/* Timeouts: all the possible timeout states passed to update_expiration()
+ * are listed here. The name will be prefix by CT_TM_ and the value is in
+ * milliseconds */
+#define CT_TIMEOUTS \
+CT_TIMEOUT(TCP_FIRST_PACKET, 30 * 1000) \
+CT_TIMEOUT(TCP_OPENING, 30 * 1000) \
+CT_TIMEOUT(TCP_ESTABLISHED, 24 * 60 * 60 * 1000) \
+CT_TIMEOUT(TCP_CLOSING, 15 * 60 * 1000) \
+CT_TIMEOUT(TCP_FIN_WAIT, 45 * 1000) \
+CT_TIMEOUT(TCP_CLOSED, 30 * 1000) \
+CT_TIMEOUT(OTHER_FIRST, 60 * 1000) \
+CT_TIMEOUT(OTHER_MULTIPLE, 60 * 1000) \
+CT_TIMEOUT(OTHER_BIDIR, 30 * 1000) \
+CT_TIMEOUT(ICMP_FIRST, 60 * 1000) \
+CT_TIMEOUT(ICMP_REPLY, 30 * 1000)
+
+/* The smallest of the above values: it is used as an upper bound for the
+ * interval between two rounds of cleanup of expired entries */
+#define CT_TM_MIN (30 * 1000)
+
+#define CT_TIMEOUT(NAME, VAL) BUILD_ASSERT_DECL(VAL >= CT_TM_MIN);
+CT_TIMEOUTS
+#undef CT_TIMEOUT
+
+enum ct_timeout {
+#define CT_TIMEOUT(NAME, VALUE) CT_TM_##NAME,
+CT_TIMEOUTS
+#undef CT_TIMEOUT
+N_CT_TM
+};
+
+
+/* Locking:
+ *
+ * The connections are kept in different buckets, which are completely
+ * independent. The connection bucket is determined by the hash of its key.
+ *
+ * Each bucket has two locks. Acquisition order is, from outermost to
+ * innermost:
+ *
+ *cleanup_mutex
+ *lock
+ *
+ * */
+struct conntrack_bucket {
+/* Protects 'connections' and 'exp_lists'.  Used in the fast path */
+struct ct_lock lock;
+/* Contains the connections in the bucket, indexed by 'struct conn_key' */
+struct hmap connections OVS_GUARDED;
+/* For each possible timeout we have a list of connections. When the
+ * timeout of a connection is updated, we move it to the back of the list.
+ * Since the connection in a list have the same relative timeout, the list
+ * will be ordered, with the oldest connections to the front. */
+struct ovs_list exp_lists[N_CT_TM] OVS_GUARDED;
+
+/* Protects 'next_cleanup'. Used to make sure that there's only one thread
+ * performing the cleanup. */
+struct ovs_mutex cleanup_mutex;
+long long next_cleanup OVS_GUARDED;
+};
+
+#define CONNTRACK_BUCKETS_SHIFT 8
+#define CONNTRACK_BUCKETS (1 << CONNTRACK_BUCKETS_SHIFT)
+
+struct conntrack {
+/* Independent buckets containing the connections */
+struct conntrack_bucket buckets[CONNTRACK_BUCKETS];
+
+/* Salt for hashing a connection key. */
+uint32_t hash_basis;
+/* The thread performing periodic cleanup of the connection
+ * tracker. */
+pthread_t clean_thread;
+/* Latch to destroy the 'clean_thread' */
+struct latch clean_thread_exit;
+
+/* Number of connections currently in the connection tracker. */
+atomic_count n_conn;
+/* Connections limit. When this limit is reached, no 

[ovs-dev] [patch v5 2/2] conntrack: Add rcu support.

2019-05-02 Thread Darrell Ball
For performance and code simplification reasons, add rcu support for
conntrack. The array of hmaps is replaced by a cmap as part of this
conversion.  Using a single map also simplifies the handling of NAT
and allows the removal of the nat_conn map and friends.  Per connection
entry locks are introduced, which are needed in a few code paths.

Signed-off-by: Darrell Ball 
---
 lib/conntrack-icmp.c|  25 +-
 lib/conntrack-other.c   |  12 +-
 lib/conntrack-private.h | 187 ++
 lib/conntrack-tcp.c |  20 +-
 lib/conntrack.c | 967 +---
 lib/conntrack.h |   4 +-
 6 files changed, 397 insertions(+), 818 deletions(-)

diff --git a/lib/conntrack-icmp.c b/lib/conntrack-icmp.c
index 40fd1d8..f00a4c4 100644
--- a/lib/conntrack-icmp.c
+++ b/lib/conntrack-icmp.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015-2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -46,16 +46,12 @@ conn_icmp_cast(const struct conn *conn)
 }
 
 static enum ct_update_res
-icmp_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
- struct dp_packet *pkt OVS_UNUSED, bool reply, long long now)
+icmp_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
+ bool reply, long long now)
 {
 struct conn_icmp *conn = conn_icmp_cast(conn_);
-
-if (reply && conn->state != ICMPS_REPLY) {
-conn->state = ICMPS_REPLY;
-}
-
-conn_update_expiration(ctb, &conn->up, icmp_timeouts[conn->state], now);
+conn->state = reply ? ICMPS_REPLY : ICMPS_FIRST;
+conn_update_expiration(&conn->up, icmp_timeouts[conn->state], now);
 
 return CT_UPDATE_VALID;
 }
@@ -79,15 +75,12 @@ icmp6_valid_new(struct dp_packet *pkt)
 }
 
 static struct conn *
-icmp_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt OVS_UNUSED,
-   long long now)
+icmp_new_conn(struct conntrack *ct, struct dp_packet *pkt OVS_UNUSED,
+  long long now)
 {
-struct conn_icmp *conn;
-
-conn = xzalloc(sizeof *conn);
+struct conn_icmp *conn = xzalloc(sizeof *conn);
 conn->state = ICMPS_FIRST;
-
-conn_init_expiration(ctb, &conn->up, icmp_timeouts[conn->state], now);
+conn_init_expiration(ct, &conn->up, icmp_timeouts[conn->state], now);
 
 return &conn->up;
 }
diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
index 2920889..a539f18 100644
--- a/lib/conntrack-other.c
+++ b/lib/conntrack-other.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2015-2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,8 +43,8 @@ conn_other_cast(const struct conn *conn)
 }
 
 static enum ct_update_res
-other_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
-  struct dp_packet *pkt OVS_UNUSED, bool reply, long long now)
+other_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
+  bool reply, long long now)
 {
 struct conn_other *conn = conn_other_cast(conn_);
 
@@ -54,7 +54,7 @@ other_conn_update(struct conn *conn_, struct conntrack_bucket 
*ctb,
 conn->state = OTHERS_MULTIPLE;
 }
 
-conn_update_expiration(ctb, &conn->up, other_timeouts[conn->state], now);
+conn_update_expiration(&conn->up, other_timeouts[conn->state], now);
 
 return CT_UPDATE_VALID;
 }
@@ -66,7 +66,7 @@ other_valid_new(struct dp_packet *pkt OVS_UNUSED)
 }
 
 static struct conn *
-other_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt OVS_UNUSED,
+other_new_conn(struct conntrack *ct, struct dp_packet *pkt OVS_UNUSED,
long long now)
 {
 struct conn_other *conn;
@@ -74,7 +74,7 @@ other_new_conn(struct conntrack_bucket *ctb, struct dp_packet 
*pkt OVS_UNUSED,
 conn = xzalloc(sizeof *conn);
 conn->state = OTHERS_FIRST;
 
-conn_init_expiration(ctb, &conn->up, other_timeouts[conn->state], now);
+conn_init_expiration(ct, &conn->up, other_timeouts[conn->state], now);
 
 return &conn->up;
 }
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 059af9e..35d8928 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2016, 2017 Nicira, Inc.
+ * Copyright (c) 2015-2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,8 +21,10 @@
 #include 
 #include 
 
+#include "cmap.h"
 #include "conntrack.h"
 #include "ct-dpif.h"
+#include "ipf.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/list.h"
 #include "openvswitch/types.h"
@@ -57,12 +59,6 @@ struct conn_key {
 uint8_t nw_proto;
 };
 
-struct nat_conn_key_node {
-struct hmap_node node;
-struc