Re: [ovs-dev] [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

2020-08-06 Thread Eli Britstein



On 8/6/2020 8:28 PM, Stokes, Ian wrote:

On 8/6/2020 6:17 PM, Emma Finn wrote:

The following 2 commits introduced changes which caused a regression
for XL710 devices and functionality ceases for partial offload as a result.
864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for type
only.")
a79eae87abe4 ("netdev-offload-dpdk: Remove pre-validate of patterns
function.")

OVS is vendor agnostic. That kind of workaround belongs in intel PMD in dpdk,
not in OVS.

Hi Eli,

Yes OVS looks to be vendor agnostic, but this code I believe was already in 
place and working for this usecase. As such it's removal introduced a 
regression from an OVS point of view between the releases.

We have had examples in the past where workarounds are permissible if there is 
a clear path to fixing this in the future on the DPDK side (which is what I 
suggest here) (for example scatter gather support for MTUs in the past raised 
similar issue where we hand to handle specific NIC until the next DPDK LTS 
release).

So my suggestion is to re-instate the original workaround and remove its when 
fixed in the next DPDK LTS which supports the change for i40e at the PMD layer 
or if it's backported to the next 19.11 stable release which would be validated 
for use with OVS.


Hi,

There was a bug with this WA.

Please see 
https://patchwork.ozlabs.org/project/openvswitch/patch/1587180266-28632-1-git-send-email-jackerd...@gmail.com/.


Is it possible to address it in DPDK instead of reverting in OVS and 
later re-reverting?


Thanks,

Eli



I've included the i40e DPDK maintainers here for their thoughts also.

@Beilei/Jia Is this something we can look at to introduce in the i40e PMD?

Regards
Ian


Fixed by partial reversion of these changes.

Signed-off-by: Emma Finn

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


Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Tony Liu
Hi,

There are still some connection errors from ovn-controller.
Is that connection drop will cause flows to be deleted from vswitchd?

..
2020-08-07T03:55:22.269Z|03988|jsonrpc|WARN|tcp:127.0.0.1:6640: send error: 
Broken pipe
..
2020-08-07T03:55:31.551Z|03996|reconnect|WARN|tcp:127.0.0.1:6640: connection 
dropped (Broken pipe)



2020-08-07T03:55:22.268Z|03986|poll_loop|INFO|wakeup due to [POLLIN] on fd 14 
(127.0.0.1:49514<->127.0.0.1:6640) at lib/stream-fd.c:157 (99% CPU usage)
2020-08-07T03:55:22.268Z|03987|poll_loop|INFO|wakeup due to [POLLIN] on fd 19 
(10.6.20.91:42854<->10.6.20.84:6642) at lib/stream-fd.c:157 (99% CPU usage)
2020-08-07T03:55:22.269Z|03988|jsonrpc|WARN|tcp:127.0.0.1:6640: send error: 
Broken pipe
2020-08-07T03:55:31.549Z|03989|timeval|WARN|Unreasonably long 9280ms poll 
interval (9220ms user, 1ms system)
2020-08-07T03:55:31.550Z|03990|timeval|WARN|disk: 0 reads, 8 writes
2020-08-07T03:55:31.550Z|03991|timeval|WARN|context switches: 0 voluntary, 5 
involuntary
2020-08-07T03:55:31.550Z|03992|coverage|INFO|Dropped 4 log messages in last 47 
seconds (most recently, 9 seconds ago) due to excessive rate
2020-08-07T03:55:31.551Z|03993|coverage|INFO|Skipping details of duplicate 
event coverage for hash=824dd6ab
2020-08-07T03:55:31.551Z|03994|poll_loop|INFO|wakeup due to [POLLIN] on fd 20 
(<->/var/run/openvswitch/br-int.mgmt) at lib/stream-fd.c:157 (100% CPU usage)
2020-08-07T03:55:31.551Z|03995|poll_loop|INFO|wakeup due to [POLLIN] on fd 19 
(10.6.20.91:42854<->10.6.20.84:6642) at lib/stream-fd.c:157 (100% CPU usage)
2020-08-07T03:55:31.551Z|03996|reconnect|WARN|tcp:127.0.0.1:6640: connection 
dropped (Broken pipe)
2020-08-07T03:55:31.552Z|03997|poll_loop|INFO|wakeup due to 0-ms timeout at 
controller/ovn-controller.c:2123 (100% CPU usage)
2020-08-07T03:55:40.752Z|03998|timeval|WARN|Unreasonably long 9176ms poll 
interval (9118ms user, 0ms system)
2020-08-07T03:55:40.752Z|03999|timeval|WARN|context switches: 0 voluntary, 7 
involuntary
2020-08-07T03:55:40.753Z|04000|poll_loop|INFO|Dropped 2 log messages in last 10 
seconds (most recently, 10 seconds ago) due to excessive rate
2020-08-07T03:55:40.753Z|04001|poll_loop|INFO|wakeup due to 0-ms timeout at 
lib/reconnect.c:643 (99% CPU usage)
2020-08-07T03:55:40.754Z|04002|reconnect|INFO|tcp:127.0.0.1:6640: connecting...
2020-08-07T03:55:40.771Z|04003|reconnect|INFO|tcp:127.0.0.1:6640: connected


Thanks!

Tony
> -Original Message-
> From: discuss  On Behalf Of Tony
> Liu
> Sent: Thursday, August 6, 2020 8:23 PM
> To: Han Zhou ; Numan Siddique 
> Cc: ovs-dev ; ovs-discuss  disc...@openvswitch.org>
> Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> 
> Interesting...
> 
> with this configuration on gateway (chassis) node, 
> external_ids: {ovn-bridge-mappings="physnet1:br-ex", ovn-cms-
> options=enable-chassis-as-gw, ovn-encap-ip="10.6.30.91", ovn-encap-
> type=geneve, ovn-openflow-probe-interval="30", ovn-
> remote="tcp:10.6.20.84:6642,tcp:10.6.20.85:6642,tcp:10.6.20.86:6642",
> ovn-remote-probe-interval="3", system-id="gateway-1"}
> 
> 
> I still see error from ovn-controller.
> 
> 2020-08-07T03:17:48.186Z|02737|reconnect|ERR|tcp:127.0.0.1:6640: no
> response to inactivity probe after 8.74 seconds, disconnecting 
> That tcp:127.0.0.1:6640 is the connection between ovn-controller and
> local ovsdb-server.
> 
> Any settings I missed?
> 
> 
> Thanks!
> 
> Tony
> > -Original Message-
> > From: dev  On Behalf Of Tony Liu
> > Sent: Thursday, August 6, 2020 7:45 PM
> > To: Han Zhou ; Numan Siddique 
> > Cc: ovs-dev ; ovs-discuss  > disc...@openvswitch.org>
> > Subject: Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity
> > probe
> >
> > Hi Han and Numan,
> >
> > I'd like to have a few more clarifications.
> >
> > For inactivity probe:
> > From ovn-controller to ovn-sb-db: ovn-remote-probe-interval
> >
> > From ovn-controller to ovs-vswitchd: ovn-openflow-probe-interval
> >
> > From ovn-controller to local ovsdb: which interval?
> >
> > From local ovsdb to ovn-controller: which interval?
> >
> > From ovs-vswitchd to ovn-controller: which interval?
> >
> >
> > Regarding to the connection between ovn-controller and local ovsdb-
> > server, I recall that UNIX socket is lighter than TCP socket and UNIX
> > socket is recommended for local communication.
> > Is that right?
> >
> >
> > Thanks!
> >
> > Tony
> >
> > > -Original Message-
> > > From: Han Zhou 
> > > Sent: Thursday, August 6, 2020 12:42 PM
> > > To: Tony Liu 
> > > Cc: Han Zhou ; Numan Siddique ;
> > > ovs-dev ; ovs-discuss
> > > 
> > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > >
> > >
> > >
> > > On Thu, Aug 6, 2020 at 12:07 PM Tony Liu  > >  > wrote:
> > > >
> > > > Inline...
> > > >
> > > > Thanks!
> > > >
> > > > Tony
> > > > > -Original Message-
> > > > > From: 

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Tony Liu
Interesting...

with this configuration on gateway (chassis) node,

external_ids: {ovn-bridge-mappings="physnet1:br-ex", 
ovn-cms-options=enable-chassis-as-gw, ovn-encap-ip="10.6.30.91", 
ovn-encap-type=geneve, ovn-openflow-probe-interval="30", 
ovn-remote="tcp:10.6.20.84:6642,tcp:10.6.20.85:6642,tcp:10.6.20.86:6642", 
ovn-remote-probe-interval="3", system-id="gateway-1"}


I still see error from ovn-controller.

2020-08-07T03:17:48.186Z|02737|reconnect|ERR|tcp:127.0.0.1:6640: no response to 
inactivity probe after 8.74 seconds, disconnecting

That tcp:127.0.0.1:6640 is the connection between ovn-controller
and local ovsdb-server.

Any settings I missed?


Thanks!

Tony
> -Original Message-
> From: dev  On Behalf Of Tony Liu
> Sent: Thursday, August 6, 2020 7:45 PM
> To: Han Zhou ; Numan Siddique 
> Cc: ovs-dev ; ovs-discuss  disc...@openvswitch.org>
> Subject: Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity
> probe
> 
> Hi Han and Numan,
> 
> I'd like to have a few more clarifications.
> 
> For inactivity probe:
> From ovn-controller to ovn-sb-db: ovn-remote-probe-interval
> 
> From ovn-controller to ovs-vswitchd: ovn-openflow-probe-interval
> 
> From ovn-controller to local ovsdb: which interval?
> 
> From local ovsdb to ovn-controller: which interval?
> 
> From ovs-vswitchd to ovn-controller: which interval?
> 
> 
> Regarding to the connection between ovn-controller and local ovsdb-
> server, I recall that UNIX socket is lighter than TCP socket and UNIX
> socket is recommended for local communication.
> Is that right?
> 
> 
> Thanks!
> 
> Tony
> 
> > -Original Message-
> > From: Han Zhou 
> > Sent: Thursday, August 6, 2020 12:42 PM
> > To: Tony Liu 
> > Cc: Han Zhou ; Numan Siddique ; ovs-dev
> > ; ovs-discuss 
> > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> >
> >
> >
> > On Thu, Aug 6, 2020 at 12:07 PM Tony Liu  >  > wrote:
> > >
> > > Inline...
> > >
> > > Thanks!
> > >
> > > Tony
> > > > -Original Message-
> > > > From: Han Zhou mailto:hz...@ovn.org> >
> > > > Sent: Thursday, August 6, 2020 11:37 AM
> > > > To: Tony Liu  > > >  >
> > > > Cc: Han Zhou mailto:hz...@ovn.org> >; Numan
> > > > Siddique mailto:num...@ovn.org> >; ovs-dev
> > > > mailto:ovs-dev@openvswitch.org> >;
> > > > ovs-discuss  > > >  >
> > > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > > >
> > > >
> > > >
> > > > On Thu, Aug 6, 2020 at 11:11 AM Tony Liu  > > >   >  > > wrote:
> > > > >
> > > > > Inline... (please read with monospaced font:))
> > > > >
> > > > > Thanks!
> > > > >
> > > > > Tony
> > > > > > -Original Message-
> > > > > > From: Han Zhou mailto:hz...@ovn.org>
> > > > > >  > >
> > > > > > Sent: Wednesday, August 5, 2020 11:48 PM
> > > > > > To: Tony Liu  > > > > > 
> > > > > >  > > > > >  > >
> > > > > > Cc: Han Zhou mailto:hz...@ovn.org>
> > > > > >  > >; Numan
> > > > > > Siddique mailto:num...@ovn.org>
> > > > > >  > >; ovs-dev
> > > > > > mailto:ovs-dev@openvswitch.org>
> > > > > >  > > > > > 
> > > > > > > >; ovs-discuss  > > > > > 
> > > > > >  > > > > >  > >
> > > > > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity
> > > > > > probe
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, Aug 5, 2020 at 9:14 PM Tony Liu
> > > > > > mailto:tonyliu0...@hotmail.com>
> > > > > >  > > > > >  >
> > > > > >  > > > > > 
> > > >  >  > > > wrote:
> > > > > >
> > > > > >
> > > > > >   I set the connection target="ptcp:6641:10.6.20.84" for
> > > > > > ovn-nb-
> > > > db
> > > > > >   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the
> > > > > > first
> > > > node
> > > > > >   of cluster. Also ovn-openflow-probe-interval=30 on
> > > > > > compute
> > > > node.
> > > > > >   It seems helping. Not that many connect/drop/reconnect
> > > > > > in
> > > > logging.
> > > > > >   That "commit failure" is also gone.
> > > > > >   The issue I reported in another thread "packet drop"
> > > > > > seems
> > > > gone.
> > > > > >   And launching VM starts working.
> > > > > >
> > > > > >   How should I set connection table for all ovn-nb-db and
> > > > > > ovn-
> > > > sb-db
> 

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Tony Liu
Hi Han and Numan,

I'd like to have a few more clarifications.

For inactivity probe:
>From ovn-controller to ovn-sb-db: ovn-remote-probe-interval

>From ovn-controller to ovs-vswitchd: ovn-openflow-probe-interval

>From ovn-controller to local ovsdb: which interval?

>From local ovsdb to ovn-controller: which interval?

>From ovs-vswitchd to ovn-controller: which interval?


Regarding to the connection between ovn-controller and local
ovsdb-server, I recall that UNIX socket is lighter than TCP socket
and UNIX socket is recommended for local communication.
Is that right?


Thanks!

Tony

> -Original Message-
> From: Han Zhou 
> Sent: Thursday, August 6, 2020 12:42 PM
> To: Tony Liu 
> Cc: Han Zhou ; Numan Siddique ; ovs-dev
> ; ovs-discuss 
> Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> 
> 
> 
> On Thu, Aug 6, 2020 at 12:07 PM Tony Liu   > wrote:
> >
> > Inline...
> >
> > Thanks!
> >
> > Tony
> > > -Original Message-
> > > From: Han Zhou mailto:hz...@ovn.org> >
> > > Sent: Thursday, August 6, 2020 11:37 AM
> > > To: Tony Liu  > >  >
> > > Cc: Han Zhou mailto:hz...@ovn.org> >; Numan Siddique
> > > mailto:num...@ovn.org> >; ovs-dev
> > > mailto:ovs-dev@openvswitch.org> >;
> > > ovs-discuss  > >  >
> > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > >
> > >
> > >
> > > On Thu, Aug 6, 2020 at 11:11 AM Tony Liu  > >    > > wrote:
> > > >
> > > > Inline... (please read with monospaced font:))
> > > >
> > > > Thanks!
> > > >
> > > > Tony
> > > > > -Original Message-
> > > > > From: Han Zhou mailto:hz...@ovn.org>
> > > > >  > >
> > > > > Sent: Wednesday, August 5, 2020 11:48 PM
> > > > > To: Tony Liu  > > > >   > > > >  > >
> > > > > Cc: Han Zhou mailto:hz...@ovn.org>
> > > > >  > >; Numan Siddique
> > > > > mailto:num...@ovn.org>   > > > >  > >; ovs-dev  > > > > 
> > > > > 
> > > > > > >; ovs-discuss  > > > > 
> > > > >  > > > >  > >
> > > > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Aug 5, 2020 at 9:14 PM Tony Liu  > > > >   > > > >  >
> > > > > 
> > >   > > > wrote:
> > > > >
> > > > >
> > > > >   I set the connection target="ptcp:6641:10.6.20.84" for
> > > > > ovn-nb-
> > > db
> > > > >   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first
> > > node
> > > > >   of cluster. Also ovn-openflow-probe-interval=30 on compute
> > > node.
> > > > >   It seems helping. Not that many connect/drop/reconnect in
> > > logging.
> > > > >   That "commit failure" is also gone.
> > > > >   The issue I reported in another thread "packet drop" seems
> > > gone.
> > > > >   And launching VM starts working.
> > > > >
> > > > >   How should I set connection table for all ovn-nb-db and
> > > > > ovn-
> > > sb-db
> > > > >   nodes in the cluster to set inactivity_probe?
> > > > >   One row with address 0.0.0.0 seems not working.
> > > > >
> > > > > You can simply use 0.0.0.0 in the connection table, but don't
> > > > > specify the same connection method on the command line when
> > > > > starting
> > > > > ovsdb- server for NB/SB DB. Otherwise, these are conflicting and
> > > > > that's why you saw "Address already in use" error.
> > > >
> > > > Could you share a bit details how it works?
> > > > I thought the row in connection table only tells nbdb and sbdb the
> > > > probe interval. Isn't that right? Does nbdb and sbdb also create
> > > > socket based on target column?
> > >
> > > >
> > >
> > > In --remote option of ovsdb-server, you can specify either a
> > > connection method directly, or specify the db,table,column which
> > > contains the connection information.
> > > Please see manpage ovsdb-server(1).
> >
> > Here is how one of those 3 nbdb nodes invoked.
> > 
> > ovsdb-server -vconsole:off -vfile:info
> > --log-file=/var/log/kolla/openvswitch/ovn-sb-db.log
> > --remote=punix:/var/run/ovn/ovnsb_db.sock --
> pidfile=/run/ovn/ovnsb_db.pid --unixctl=/var/run/ovn/ovnsb_db.ctl --
> remote=db:OVN_Southbound,SB_Global,connections --private-
> key=db:OVN_Southbound,SSL,private_key --
> 

[ovs-dev] TNT Express delivery Consignment Notification

2020-08-06 Thread TNT EXPRESS


 Dear Customer,
A shipment has been arranged for you through TNT
 The shipment has been scheduled for delivery and has TNT consignment number: 
87993766478.
Attached is the documentation that relates to this Express Import Order
Print and sign all copies of the consignment note.
 

 
Note: For shipments outside of the EU, it is highly recommended to include the 
actual invoice of the goods. 
 - Make sure you have read the TNT Terms & Conditions attached to this email. 
By signing the consignment notes, you confirm that you agree to the TNT Terms & 
Conditions. 
 - Hand the signed copies of the consignment note to the TNT Express driver, 
keep the Sender copy for your records. 
 - Leave the shipment open for TNT Express driver to inspect. 
 As your order has been processed automatically, it is not necessary to contact 
TNT Express Customer Service by telephone.
 
If you would like to find out about the many ways TNT helps you to track your 
shipment, or if you would like to know more about the services provided by TNT, 
simply connect to www.tnt.com and select your location at any time.
  
 
---
This message and any attachment are confidential and may be privileged or 
otherwise protected from disclosure. 
If you are not the intended recipient, please telephone or email the sender and 
delete this message and any attachment from your system. 
If you are not the intended recipient you must not copy this message or 
attachment or disclose the contents to any other person.
Please consider the environmental impact before printing this document and its 
attachment(s). 
Print black and white and double-sided where possible.
---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

2020-08-06 Thread Xing, Beilei


> -Original Message-
> From: Stokes, Ian 
> Sent: Friday, August 7, 2020 1:29 AM
> To: Eli Britstein ; Finn, Emma ;
> d...@openvswitch.org; Xing, Beilei ; Guo, Jia
> 
> Cc: i.maxim...@ovn.org
> Subject: RE: [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching
> HWOL for XL710 NIC
> 
> > On 8/6/2020 6:17 PM, Emma Finn wrote:
> > > The following 2 commits introduced changes which caused a regression
> > > for XL710 devices and functionality ceases for partial offload as a 
> > > result.
> > > 864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for type
> > > only.")
> > > a79eae87abe4 ("netdev-offload-dpdk: Remove pre-validate of patterns
> > > function.")
> > OVS is vendor agnostic. That kind of workaround belongs in intel PMD
> > in dpdk, not in OVS.
> 
> Hi Eli,
> 
> Yes OVS looks to be vendor agnostic, but this code I believe was already in
> place and working for this usecase. As such it's removal introduced a
> regression from an OVS point of view between the releases.
> 
> We have had examples in the past where workarounds are permissible if there
> is a clear path to fixing this in the future on the DPDK side (which is what I
> suggest here) (for example scatter gather support for MTUs in the past raised
> similar issue where we hand to handle specific NIC until the next DPDK LTS
> release).
> 
> So my suggestion is to re-instate the original workaround and remove its when
> fixed in the next DPDK LTS which supports the change for i40e at the PMD
> layer or if it's backported to the next 19.11 stable release which would be
> validated for use with OVS.
> 
> I've included the i40e DPDK maintainers here for their thoughts also.
> 
> @Beilei/Jia Is this something we can look at to introduce in the i40e PMD?

Hi Ian,

Sorry I'm not very clear about what's the relationship between the three OVS 
patches
(864852a0624a, a79eae87abe4, this fix patch) and DPDK issue. What's the i40e 
issue?
Did you create a JIRA to track it? 

BR,
Beilei

> 
> Regards
> Ian
> 
> > >
> > > Fixed by partial reversion of these changes.
> > >
> > > Signed-off-by: Emma Finn

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


Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Tony Liu
I think I have everything clarified here.
I will make necessary tunes and rerun the test.


Thanks!

Tony

> -Original Message-
> From: Han Zhou 
> Sent: Thursday, August 6, 2020 12:42 PM
> To: Tony Liu 
> Cc: Han Zhou ; Numan Siddique ; ovs-dev
> ; ovs-discuss 
> Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> 
> 
> 
> On Thu, Aug 6, 2020 at 12:07 PM Tony Liu   > wrote:
> >
> > Inline...
> >
> > Thanks!
> >
> > Tony
> > > -Original Message-
> > > From: Han Zhou mailto:hz...@ovn.org> >
> > > Sent: Thursday, August 6, 2020 11:37 AM
> > > To: Tony Liu  > >  >
> > > Cc: Han Zhou mailto:hz...@ovn.org> >; Numan Siddique
> > > mailto:num...@ovn.org> >; ovs-dev
> > > mailto:ovs-dev@openvswitch.org> >;
> > > ovs-discuss  > >  >
> > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > >
> > >
> > >
> > > On Thu, Aug 6, 2020 at 11:11 AM Tony Liu  > >    > > wrote:
> > > >
> > > > Inline... (please read with monospaced font:))
> > > >
> > > > Thanks!
> > > >
> > > > Tony
> > > > > -Original Message-
> > > > > From: Han Zhou mailto:hz...@ovn.org>
> > > > >  > >
> > > > > Sent: Wednesday, August 5, 2020 11:48 PM
> > > > > To: Tony Liu  > > > >   > > > >  > >
> > > > > Cc: Han Zhou mailto:hz...@ovn.org>
> > > > >  > >; Numan Siddique
> > > > > mailto:num...@ovn.org>   > > > >  > >; ovs-dev  > > > > 
> > > > > 
> > > > > > >; ovs-discuss  > > > > 
> > > > >  > > > >  > >
> > > > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > > > >
> > > > >
> > > > >
> > > > > On Wed, Aug 5, 2020 at 9:14 PM Tony Liu  > > > >   > > > >  >
> > > > > 
> > >   > > > wrote:
> > > > >
> > > > >
> > > > >   I set the connection target="ptcp:6641:10.6.20.84" for
> > > > > ovn-nb-
> > > db
> > > > >   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first
> > > node
> > > > >   of cluster. Also ovn-openflow-probe-interval=30 on compute
> > > node.
> > > > >   It seems helping. Not that many connect/drop/reconnect in
> > > logging.
> > > > >   That "commit failure" is also gone.
> > > > >   The issue I reported in another thread "packet drop" seems
> > > gone.
> > > > >   And launching VM starts working.
> > > > >
> > > > >   How should I set connection table for all ovn-nb-db and
> > > > > ovn-
> > > sb-db
> > > > >   nodes in the cluster to set inactivity_probe?
> > > > >   One row with address 0.0.0.0 seems not working.
> > > > >
> > > > > You can simply use 0.0.0.0 in the connection table, but don't
> > > > > specify the same connection method on the command line when
> > > > > starting
> > > > > ovsdb- server for NB/SB DB. Otherwise, these are conflicting and
> > > > > that's why you saw "Address already in use" error.
> > > >
> > > > Could you share a bit details how it works?
> > > > I thought the row in connection table only tells nbdb and sbdb the
> > > > probe interval. Isn't that right? Does nbdb and sbdb also create
> > > > socket based on target column?
> > >
> > > >
> > >
> > > In --remote option of ovsdb-server, you can specify either a
> > > connection method directly, or specify the db,table,column which
> > > contains the connection information.
> > > Please see manpage ovsdb-server(1).
> >
> > Here is how one of those 3 nbdb nodes invoked.
> > 
> > ovsdb-server -vconsole:off -vfile:info
> > --log-file=/var/log/kolla/openvswitch/ovn-sb-db.log
> > --remote=punix:/var/run/ovn/ovnsb_db.sock --
> pidfile=/run/ovn/ovnsb_db.pid --unixctl=/var/run/ovn/ovnsb_db.ctl --
> remote=db:OVN_Southbound,SB_Global,connections --private-
> key=db:OVN_Southbound,SSL,private_key --
> certificate=db:OVN_Southbound,SSL,certificate --ca-
> cert=db:OVN_Southbound,SSL,ca_cert --ssl-
> protocols=db:OVN_Southbound,SSL,ssl_protocols --ssl-
> ciphers=db:OVN_Southbound,SSL,ssl_ciphers --remote=ptcp:6642:10.6.20.84
> /var/lib/openvswitch/ovn-sb/ov sb.db  It creates UNIX and TCP
> sockets, and takes configuration from DB.
> > Does that look ok?
> > Given that, what the target column should be for all nodes of the
> cluster?
> > And whatever target is set, ovsdb-server will 

[ovs-dev] Loan Offer

2020-08-06 Thread Mr. Philip Walter
 - This mail is in HTML. Some elements may be ommited in plain text. -

Attention, Do you need Max Loan,  Do you need a loan to clear your debts ? Are 
you going to increase your finances ? You are a business person who wants to 
expand his / her company. We give loan to individual and cooperate bodies 
ranging from ?100,000 euros to ?1,000,000.00 euros, we also offer personal 
loans from the tune of  ?5,000.00 euros to ?50 Million euros for a period of 1 
to 20 years. Please get back to us if you are interested.

Contact person: Mr. Philip Walter
Whatsappp +447787831965
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v2] dpif-netlink: distribute polling to discreet handlers

2020-08-06 Thread Flavio Leitner


Hi Aaron,

Thanks for the patch. I ran some basic tests here
and they passed. I could see only one handler thread
becoming active with a single upcall.

See my comment below.

On Tue, Jul 21, 2020 at 07:27:41PM -0400, Aaron Conole wrote:
> Currently, the channel handlers are polled globally.  On some
> systems, this causes a thundering herd issue where multiple
> handler threads become active, only to do no work and immediately
> sleep.
> 
> The approach here is to push the netlink socket channels to discreet
> handler threads to process, rather than polling on every thread.
> This will eliminate the need to wake multiple threads.
> 
> To check:
> 
>   ip netns add left
>   ip netns add right
>   ip link add center-left type veth peer name left0
>   ip link add center-right type veth peer name right0
>   ip link set left0 netns left
>   ip link set right0 netns right
>   ip link set center-left up
>   ip link set center-right up
>   ip -n left ip link set left0 up
>   ip -n left ip addr add 172.31.110.10/24 dev left0
>   ip -n right ip link set right0 up
>   ip -n right ip addr add 172.31.110.11/24 dev right0
> 
>   ovs-vsctl add-br br0
>   ovs-vsctl add-port br0 center-right
>   ovs-vsctl add-port br0 center-left
> 
>   # in one terminal
>   perf record -e sched:sched_wakeup,irq:softirq_entry -ag
> 
>   # in a separate terminal
>   ip netns exec left arping -I left0 -c 1 172.31.110.11
> 
>   # in the perf terminal after exiting
>   perf script
> 
> Look for the number of 'handler' threads which were made active.
> 
> Suggested-by: Ben Pfaff 
> Reported-by: David Ahern 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-December/365857.html
> Cc: Matteo Croce 
> Cc: Flavio Leitner 
> Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink sockets")
> Signed-off-by: Aaron Conole 
> ---
> v2: Oops - forgot to commit my whitespace cleanups.
> 
> lib/dpif-netlink.c | 289 -
>  1 file changed, 179 insertions(+), 110 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 7da4fb54d9..71d2805427 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -183,6 +183,10 @@ struct dpif_handler {
>  int n_events; /* Num events returned by epoll_wait(). */
>  int event_offset; /* Offset into 'epoll_events'. */
>  
> +struct dpif_channel *channels; /* Array of channels for each port. */
> +uint32_t *port_idx_map;/* Port idx to channel idx. */
> +size_t n_channels;
> +
>  #ifdef _WIN32
>  /* Pool of sockets. */
>  struct dpif_windows_vport_sock *vport_sock_pool;
> @@ -202,8 +206,6 @@ struct dpif_netlink {
>  struct dpif_handler *handlers;
>  uint32_t n_handlers;   /* Num of upcall handlers. */
>  struct dpif_channel *channels; /* Array of channels for each port. */


Shouldn't the above channels be removed?

> -int uc_array_size; /* Size of 'handler->channels' and */
> -   /* 'handler->epoll_events'. */
>  
>  /* Change notification. */
>  struct nl_sock *port_notifier; /* vport multicast group subscriber. */
> @@ -287,6 +289,55 @@ close_nl_sock(struct nl_sock *sock)
>  #endif
>  }
>  
> +static uint32_t
> +dpif_handler_port_idx_max(const struct dpif_handler *handler,
> +  struct dpif_channel **out_chn)

Looks like there is no caller passing out_chn, so why
that is needed?


> +{
> +uint32_t max = 0;
> +size_t i;
> +
> +for (i = 0; i < handler->n_channels; ++i) {

OVS does have some code using '++i', but the majority of
it uses 'i++', can you please swap them?


> +if (handler->channels[i].sock &&
> +handler->port_idx_map[i] > max) {
> +max = handler->port_idx_map[i];
> +if (out_chn) {
> +*out_chn = >channels[i];
> +}
> +}
> +}
> +
> +return max;
> +}
> +
> +static size_t
> +dpif_handler_active_channels(const struct dpif_handler *handler)
> +{
> +size_t i, ret = 0;

Perhaps 'count' or 'num'  would be a better name.

> +
> +for (i = 0; i < handler->n_channels; ++i) {
> +if (handler->channels[i].sock) {
> +ret++;
> +}
> +}
> +
> +return ret;
> +}
> +
> +static struct dpif_channel *
> +dpif_handler_has_port_idx(const struct dpif_handler *handler, uint32_t idx)
> +{
> +size_t i;
> +
> +for (i = 0; i < handler->n_channels; ++i) {
> +if (handler->channels[i].sock &&
> +handler->port_idx_map[i] == idx) {
> +return >channels[i];
> +}
> +}
> +
> +return NULL;
> +}
> +
>  static struct dpif_netlink *
>  dpif_netlink_cast(const struct dpif *dpif)
>  {
> @@ -452,90 +503,86 @@ static bool
>  vport_get_pid(struct dpif_netlink *dpif, uint32_t port_idx,
>uint32_t *upcall_pid)
>  {
> -/* Since the nl_sock can only be assigned in either all
> - * or 

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Han Zhou
On Thu, Aug 6, 2020 at 12:07 PM Tony Liu  wrote:
>
> Inline...
>
> Thanks!
>
> Tony
> > -Original Message-
> > From: Han Zhou 
> > Sent: Thursday, August 6, 2020 11:37 AM
> > To: Tony Liu 
> > Cc: Han Zhou ; Numan Siddique ; ovs-dev
> > ; ovs-discuss 
> > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> >
> >
> >
> > On Thu, Aug 6, 2020 at 11:11 AM Tony Liu  >  > wrote:
> > >
> > > Inline... (please read with monospaced font:))
> > >
> > > Thanks!
> > >
> > > Tony
> > > > -Original Message-
> > > > From: Han Zhou mailto:hz...@ovn.org> >
> > > > Sent: Wednesday, August 5, 2020 11:48 PM
> > > > To: Tony Liu  > > >  >
> > > > Cc: Han Zhou mailto:hz...@ovn.org> >; Numan Siddique
> > > > mailto:num...@ovn.org> >; ovs-dev
> > > > mailto:ovs-dev@openvswitch.org> >;
> > > > ovs-discuss  > > >  >
> > > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > > >
> > > >
> > > >
> > > > On Wed, Aug 5, 2020 at 9:14 PM Tony Liu  > > >   >  > > wrote:
> > > >
> > > >
> > > >   I set the connection target="ptcp:6641:10.6.20.84" for ovn-nb-
> > db
> > > >   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first
> > node
> > > >   of cluster. Also ovn-openflow-probe-interval=30 on compute
> > node.
> > > >   It seems helping. Not that many connect/drop/reconnect in
> > logging.
> > > >   That "commit failure" is also gone.
> > > >   The issue I reported in another thread "packet drop" seems
> > gone.
> > > >   And launching VM starts working.
> > > >
> > > >   How should I set connection table for all ovn-nb-db and ovn-
> > sb-db
> > > >   nodes in the cluster to set inactivity_probe?
> > > >   One row with address 0.0.0.0 seems not working.
> > > >
> > > > You can simply use 0.0.0.0 in the connection table, but don't
> > > > specify the same connection method on the command line when starting
> > > > ovsdb- server for NB/SB DB. Otherwise, these are conflicting and
> > > > that's why you saw "Address already in use" error.
> > >
> > > Could you share a bit details how it works?
> > > I thought the row in connection table only tells nbdb and sbdb the
> > > probe interval. Isn't that right? Does nbdb and sbdb also create
> > > socket based on target column?
> >
> > >
> >
> > In --remote option of ovsdb-server, you can specify either a connection
> > method directly, or specify the db,table,column which contains the
> > connection information.
> > Please see manpage ovsdb-server(1).
>
> Here is how one of those 3 nbdb nodes invoked.
> 
> ovsdb-server -vconsole:off -vfile:info
--log-file=/var/log/kolla/openvswitch/ovn-sb-db.log
--remote=punix:/var/run/ovn/ovnsb_db.sock --pidfile=/run/ovn/ovnsb_db.pid
--unixctl=/var/run/ovn/ovnsb_db.ctl
--remote=db:OVN_Southbound,SB_Global,connections
--private-key=db:OVN_Southbound,SSL,private_key
--certificate=db:OVN_Southbound,SSL,certificate
--ca-cert=db:OVN_Southbound,SSL,ca_cert
--ssl-protocols=db:OVN_Southbound,SSL,ssl_protocols
--ssl-ciphers=db:OVN_Southbound,SSL,ssl_ciphers
--remote=ptcp:6642:10.6.20.84 /var/lib/openvswitch/ovn-sb/ov sb.db
> 
> It creates UNIX and TCP sockets, and takes configuration from DB.
> Does that look ok?
> Given that, what the target column should be for all nodes of the cluster?
> And whatever target is set, ovsdb-server will create socket, right?
> Oh... Should I do "--remote=ptcp:6642:0.0.0.0"? Then I can set the same
> in connection table, and it won't cause conflict?
> If --remote and connection target are the same, whoever comes in later
> will be ignored, right?
> In coding, does ovsdb-server create a connection object for each of
> --remote and connection target, or it's one single connection object
> for both of them because method:port:address is the same? I'd expect
> the single object.
>

--remote=ptcp:6642:10.6.20.84 should be removed from the command.
You already specifies --remote=db:OVN_Southbound,SB_Global,connections
which should contain ptcp:6642:0.0.0.0
If you set both, it will result in conflict when ovsdb-server tries to
create both sockets with same port.

> > > >   Is "external_ids:ovn-remote-probe-interval" in ovsdb-server on
> > > >   compute node for ovn-controller to probe ovn-sb-db?
> > > >
> > > > OVSDB probe is bidirectional, so you need to set this value, too, if
> > > > you don't want too many probes handled by the SB server. (setting
> > > > the connection table for SB only changes the server side).
> > >
> > > In that case, how do I set probe interval for ovn-controller?
> > > My understanding is that, ovn-controller reads configuration from
> > > ovsdb-server on the local compute node. Isn't that right?
> >
> > >
> >
> > The configuration you mentioned "external_ids:ovn-remote-probe-interval"
> > is exactly the way 

Re: [ovs-dev] [PATCH] meta-flow: fix a typo in "MPLS Bottom of Stack Field" paragraph

2020-08-06 Thread Gregory Rose




On 8/6/2020 9:33 AM, Timothy Redaelli wrote:

In the ovs-fields.7 manual page, the "MPLS Bottom of Stack Field" paragraph
says:
  * When mpls_bos is 1, there is another MPLS label following this one,
so the Ethertype passed to pop_mpls should be an MPLS Ethertype. [...]

  * When mpls_bos is 0, this MPLS label is the last one, so the Ethertype
passed to pop_mpls should be a non-MPLS Ethertype such as IPv4. [...]

The values 0 and 1 have been swapped: when BOS is 1,
then no more label stack entries follows.

Fixes: 96fee5e0a2a0 ("ovs-fields: New manpage to document Open vSwitch and OpenFlow 
fields.")
Cc: b...@ovn.org
Reported-at: https://bugzilla.redhat.com/1842032
Reported-by: Guillaume Nault 
Signed-off-by: Timothy Redaelli 


Thanks for fixing the documentation.

Acked-by: Greg Rose 


---
  lib/meta-flow.xml | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
index 154675874..e72ba52ec 100644
--- a/lib/meta-flow.xml
+++ b/lib/meta-flow.xml
@@ -3920,18 +3920,18 @@ r r c c c.
  


  
-  When  is 1, there is another MPLS label
+  When  is 0, there is another MPLS label
following this one, so the Ethertype passed to pop_mpls
should be an MPLS Ethertype.  For example: table=0,
-  dl_type=0x8847, mpls_bos=1, actions=pop_mpls:0x8847,
+  dl_type=0x8847, mpls_bos=0, actions=pop_mpls:0x8847,
goto_table:1
  
  
  

-  When  is 0, this MPLS label is the last one,
+  When  is 1, this MPLS label is the last one,
so the Ethertype passed to pop_mpls should be a 
non-MPLS
Ethertype such as IPv4.  For example: table=1, dl_type=0x8847,
-  mpls_bos=0, actions=pop_mpls:0x0800, goto_table:2
+  mpls_bos=1, actions=pop_mpls:0x0800, goto_table:2
  

  


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


Re: [ovs-dev] [ovs-discuss] packet drop

2020-08-06 Thread Tony Liu
Inline...

Thanks!

Tony
> -Original Message-
> From: Numan Siddique 
> Sent: Thursday, August 6, 2020 11:49 AM
> To: Tony Liu 
> Cc: ovs-dev@openvswitch.org; ovs-disc...@openvswitch.org
> Subject: Re: [ovs-discuss] [ovs-dev] packet drop
> 
> On Fri, Aug 7, 2020 at 12:10 AM Tony Liu  wrote:
> >
> > Inline...
> >
> > Thanks!
> >
> > Tony
> > > -Original Message-
> > > From: Numan Siddique 
> > > Sent: Thursday, August 6, 2020 10:03 AM
> > > To: Tony Liu 
> > > Cc: ovs-disc...@openvswitch.org; ovs-dev@openvswitch.org
> > > Subject: Re: [ovs-dev] packet drop
> > >
> > >
> > >
> > > On Thu, Aug 6, 2020 at 4:05 AM Tony Liu  > >  > wrote:
> > >
> > >
> > >
> > >   The drop is caused by flow change.
> > >
> > >   When packet is dropped.
> > >   
> > >
> > > recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,ge
> > > neve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-
> > > df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:0
> > > df+csum+0:00
> > > /01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type
> > > =8/0 xf8), packets:14, bytes:1372, used:0.846s, actions:drop
> > >
> recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:
> > >
> 1e:85),eth_type(0x0800),ipv4(src=192.168.236.152/255.255.255.252,dst=10.
> > > 6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no
> > >  > > 0x3, ttl=64,frag=no> ),icmp(type=0), packets:6, bytes:588,
> > > used:8.983s, actions:drop
> > >   
> > >
> > >   When packet goes through.
> > >   
> > >
> > > recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,ge
> > > neve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-
> > > df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:0
> > > df+csum+0:00
> > > /01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type
> > > =8/0 xf8), packets:3, bytes:294, used:0.104s, actions:12
> > >
> recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:
> > >
> 1e:85),eth_type(0x0800),ipv4(src=192.168.236.152/255.255.255.252,dst=10.
> > > 6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no
> > >  > > 0x3, ttl=64,frag=no> ),icmp(type=0), packets:3, bytes:294,
> > > used:0.103s,
> > > actions:ct_clear,set(tunnel(tun_id=0x1a8ee,dst=10.6.30.92,ttl=64,tp_
> > > dst=
> > > 6081,geneve({class=0x102,type=0x80,len=4,0x1000b}),flags(df|csum|key
> > > ))),
> > > set(eth(src=fa:16:3e:75:b7:e5,dst=52:54:00:0c:ef:b9)),set(ipv4(ttl=6
> > > 3)),
> > > 3
> > >   
> > >
> > >   Is that flow programmed by ovn-controller via ovs-vswitchd?
> > >
> > > What version of OVN and OVS are you using ?
> >
> > ovn-20.03.0-2.el8.x86_64
> > openvswitch-2.12.0-1.el8.x86_64
> >
> > > Can you share your OVN NB DB ?
> >
> > Yes, I can. Let me know how.
> >
> > > If I understand correctly the packet is received from the patch port
> > > to br-int on the gateway node and then tunnelled to the compute node
> right ?
> > > And the packet is dropped on the compute node ?
> >
> > Yes and yes.
> >
> > > If you could share your NB DB if it's fine with you and tell the
> > > destination logical port, I can try it out locally.
> >
> > Here is what I had.
> > On compute node, ovn-controller is very busy. It keeps saying "commit
> > failed".
> > 
> > 2020-08-05T02:44:23.927Z|04125|reconnect|INFO|tcp:10.6.20.84:6642:
> > connected 2020-08-05T02:44:23.936Z|04126|main|INFO|OVNSB commit failed,
> force recompute next time.
> > 2020-08-05T02:44:23.938Z|04127|ovsdb_idl|INFO|tcp:10.6.20.84:6642:
> > clustered database server is disconnected from cluster; trying another
> > server
> > 2020-08-05T02:44:23.939Z|04128|reconnect|INFO|tcp:10.6.20.84:6642:
> > connection attempt timed out
> > 2020-08-05T02:44:23.939Z|04129|reconnect|INFO|tcp:10.6.20.84:6642:
> > waiting 2 seconds before reconnect 
> >
> > The connection to local OVSDB keeps being dropped, because no probe
> > response.
> > 
> > 2020-08-05T02:47:15.437Z|04351|poll_loop|INFO|wakeup due to [POLLIN]
> > on fd 20 (10.6.20.22:42362<->10.6.20.86:6642) at lib/stream-fd.c:157
> > (100% CPU usage)
> > 2020-08-05T02:47:15.438Z|04352|reconnect|WARN|tcp:127.0.0.1:6640:
> > connection dropped (Broken pipe)
> > 2020-08-
> 05T02:47:15.438Z|04353|rconn|INFO|unix:/var/run/openvswitch/br-int.mgmt:
> connecting...
> > 2020-08-05T02:47:15.449Z|04354|rconn|INFO|unix:/var/run/openvswitch/br
> > -int.mgmt: connected 
> >
> > After set probe interval to 30s, the problem seems gone. I mentioned
> > this in another thread "[OVN] no response to inactivity probe".
> >
> > I can restore probe interval back to default 5s, see if the problem
> > can be reproduced. For me, it's important to understand what happens
> > behind it.
> 
> Ok. This makes sense. Since the 

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Tony Liu
Inline...

Thanks!

Tony
> -Original Message-
> From: Han Zhou 
> Sent: Thursday, August 6, 2020 11:37 AM
> To: Tony Liu 
> Cc: Han Zhou ; Numan Siddique ; ovs-dev
> ; ovs-discuss 
> Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> 
> 
> 
> On Thu, Aug 6, 2020 at 11:11 AM Tony Liu   > wrote:
> >
> > Inline... (please read with monospaced font:))
> >
> > Thanks!
> >
> > Tony
> > > -Original Message-
> > > From: Han Zhou mailto:hz...@ovn.org> >
> > > Sent: Wednesday, August 5, 2020 11:48 PM
> > > To: Tony Liu  > >  >
> > > Cc: Han Zhou mailto:hz...@ovn.org> >; Numan Siddique
> > > mailto:num...@ovn.org> >; ovs-dev
> > > mailto:ovs-dev@openvswitch.org> >;
> > > ovs-discuss  > >  >
> > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > >
> > >
> > >
> > > On Wed, Aug 5, 2020 at 9:14 PM Tony Liu  > >    > > wrote:
> > >
> > >
> > >   I set the connection target="ptcp:6641:10.6.20.84" for ovn-nb-
> db
> > >   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first
> node
> > >   of cluster. Also ovn-openflow-probe-interval=30 on compute
> node.
> > >   It seems helping. Not that many connect/drop/reconnect in
> logging.
> > >   That "commit failure" is also gone.
> > >   The issue I reported in another thread "packet drop" seems
> gone.
> > >   And launching VM starts working.
> > >
> > >   How should I set connection table for all ovn-nb-db and ovn-
> sb-db
> > >   nodes in the cluster to set inactivity_probe?
> > >   One row with address 0.0.0.0 seems not working.
> > >
> > > You can simply use 0.0.0.0 in the connection table, but don't
> > > specify the same connection method on the command line when starting
> > > ovsdb- server for NB/SB DB. Otherwise, these are conflicting and
> > > that's why you saw "Address already in use" error.
> >
> > Could you share a bit details how it works?
> > I thought the row in connection table only tells nbdb and sbdb the
> > probe interval. Isn't that right? Does nbdb and sbdb also create
> > socket based on target column?
> 
> >
> 
> In --remote option of ovsdb-server, you can specify either a connection
> method directly, or specify the db,table,column which contains the
> connection information.
> Please see manpage ovsdb-server(1).

Here is how one of those 3 nbdb nodes invoked.

ovsdb-server -vconsole:off -vfile:info 
--log-file=/var/log/kolla/openvswitch/ovn-sb-db.log 
--remote=punix:/var/run/ovn/ovnsb_db.sock --pidfile=/run/ovn/ovnsb_db.pid 
--unixctl=/var/run/ovn/ovnsb_db.ctl 
--remote=db:OVN_Southbound,SB_Global,connections 
--private-key=db:OVN_Southbound,SSL,private_key 
--certificate=db:OVN_Southbound,SSL,certificate 
--ca-cert=db:OVN_Southbound,SSL,ca_cert 
--ssl-protocols=db:OVN_Southbound,SSL,ssl_protocols 
--ssl-ciphers=db:OVN_Southbound,SSL,ssl_ciphers --remote=ptcp:6642:10.6.20.84 
/var/lib/openvswitch/ovn-sb/ov sb.db

It creates UNIX and TCP sockets, and takes configuration from DB.
Does that look ok?
Given that, what the target column should be for all nodes of the cluster?
And whatever target is set, ovsdb-server will create socket, right?
Oh... Should I do "--remote=ptcp:6642:0.0.0.0"? Then I can set the same
in connection table, and it won't cause conflict?
If --remote and connection target are the same, whoever comes in later
will be ignored, right?
In coding, does ovsdb-server create a connection object for each of
--remote and connection target, or it's one single connection object
for both of them because method:port:address is the same? I'd expect
the single object.

> > >   Is "external_ids:ovn-remote-probe-interval" in ovsdb-server on
> > >   compute node for ovn-controller to probe ovn-sb-db?
> > >
> > > OVSDB probe is bidirectional, so you need to set this value, too, if
> > > you don't want too many probes handled by the SB server. (setting
> > > the connection table for SB only changes the server side).
> >
> > In that case, how do I set probe interval for ovn-controller?
> > My understanding is that, ovn-controller reads configuration from
> > ovsdb-server on the local compute node. Isn't that right?
> 
> >
> 
> The configuration you mentioned "external_ids:ovn-remote-probe-interval"
> is exactly the way to set the ovn-controller -> SB probe interval.
> (SB -> ovn-controller probe is set in the connection table of SB)
> 
> 
> You are right that ovn-controller reads configuration from the local
> ovsdb-server. This setting is in local ovsdb-server.
> 
> 
> > >   Is "external_ids:ovn-openflow-probe-interval" in ovsdb-server
> on
> > >   compute node for ovn-controller to probe ovsdb-server?
> > >
> > > It is for the OpenFlow connection between ovn-controller and ovs-
> > > vswitchd, which is part of the OpenFlow protocol.

[ovs-dev] [PATCH ovn v2] Allow force_snat options to work for dual-stack routers.

2020-08-06 Thread Mark Michelson
The lb_force_snat and dnat_force_snat options could accept only a single
IP address. For routers that only route traffic of a single IP address
family, this is fine. However, if a router routes both IPv4 and IPv6
traffic, then this limitation is a problem.

This patch addresses this problem by allowing for these options to
specify both an IPv4 and IPv6 address.

Signed-off-by: Mark Michelson 
---
 northd/ovn-northd.c | 187 ---
 ovn-nb.xml  |  24 +-
 tests/system-ovn.at | 541 
 3 files changed, 657 insertions(+), 95 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index b7102b2bf..171e420df 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8001,44 +8001,37 @@ op_put_v6_networks(struct ds *ds, const struct ovn_port 
*op)
 ds_put_cstr(ds, "}");
 }
 
-static const char *
+static bool
 get_force_snat_ip(struct ovn_datapath *od, const char *key_type,
-  struct v46_ip *ip)
+  struct lport_addresses *laddrs)
 {
 char *key = xasprintf("%s_force_snat_ip", key_type);
-const char *ip_address = smap_get(>nbr->options, key);
+const char *addresses = smap_get(>nbr->options, key);
 free(key);
 
-if (ip_address) {
-ovs_be32 mask;
-ip->family = AF_INET;
-char *error = ip_parse_masked(ip_address, >ipv4, );
-if (error || mask != OVS_BE32_MAX) {
-free(error);
-struct in6_addr mask_v6, v6_exact = IN6ADDR_EXACT_INIT;
-ip->family = AF_INET6;
-error = ipv6_parse_masked(ip_address, >ipv6, _v6);
-if (error || memcmp(_v6, _exact, sizeof(mask_v6))) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(, "bad ip %s in options of router "UUID_FMT"",
- ip_address, UUID_ARGS(>key));
-memset(ip, 0, sizeof *ip);
-ip->family = AF_UNSPEC;
-return NULL;
-}
-}
-return ip_address;
+if (!addresses) {
+return false;
 }
 
-memset(ip, 0, sizeof *ip);
-ip->family = AF_UNSPEC;
-return NULL;
+if (!extract_ip_addresses(addresses, laddrs) ||
+laddrs->n_ipv4_addrs > 1 ||
+laddrs->n_ipv6_addrs > 1 ||
+(laddrs->n_ipv4_addrs && laddrs->ipv4_addrs[0].plen != 32) ||
+(laddrs->n_ipv6_addrs && laddrs->ipv6_addrs[0].plen != 128)) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+VLOG_WARN_RL(, "bad ip %s in options of router "UUID_FMT"",
+ addresses, UUID_ARGS(>key));
+destroy_lport_addresses(laddrs);
+return false;
+}
+
+return true;
 }
 
 static void
 add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
struct ds *match, struct ds *actions, int priority,
-   const char *lb_force_snat_ip, struct lb_vip *lb_vip,
+   bool lb_force_snat_ip, struct lb_vip *lb_vip,
const char *proto, struct nbrec_load_balancer *lb,
struct shash *meter_groups, struct sset *nat_entries)
 {
@@ -8359,6 +8352,32 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct 
ovn_port *op,
 ds_destroy();
 }
 
+static void
+build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od,
+   const char *ip_version, const char *ip_addr,
+   const char *context)
+{
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ds actions = DS_EMPTY_INITIALIZER;
+ds_put_format(, "ip%s && ip%s.dst == %s",
+  ip_version, ip_version, ip_addr);
+ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 110,
+  ds_cstr(), "ct_snat;");
+
+/* Higher priority rules to force SNAT with the IP addresses
+ * configured in the Gateway router.  This only takes effect
+ * when the packet has already been DNATed or load balanced once. */
+ds_clear();
+ds_put_format(, "flags.force_snat_for_%s == 1 && ip%s",
+  context, ip_version);
+ds_put_format(, "ct_snat(%s);", ip_addr);
+ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 100,
+  ds_cstr(), ds_cstr());
+
+ds_destroy();
+ds_destroy();
+}
+
 static void
 build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 struct hmap *lflows, struct shash *meter_groups,
@@ -8871,24 +8890,37 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 }
 }
 
-/* A gateway router can have 2 SNAT IP addresses to force DNATed and
+/* A gateway router can have 4 SNAT IP addresses to force DNATed and
  * LBed traffic respectively to be SNATed.  In addition, there can be
  * a number of SNAT rules in the NAT table. */
 struct v46_ip *snat_ips = xmalloc(sizeof *snat_ips
-   

Re: [ovs-dev] [ovs-discuss] packet drop

2020-08-06 Thread Numan Siddique
On Fri, Aug 7, 2020 at 12:10 AM Tony Liu  wrote:
>
> Inline...
>
> Thanks!
>
> Tony
> > -Original Message-
> > From: Numan Siddique 
> > Sent: Thursday, August 6, 2020 10:03 AM
> > To: Tony Liu 
> > Cc: ovs-disc...@openvswitch.org; ovs-dev@openvswitch.org
> > Subject: Re: [ovs-dev] packet drop
> >
> >
> >
> > On Thu, Aug 6, 2020 at 4:05 AM Tony Liu  >  > wrote:
> >
> >
> >
> >   The drop is caused by flow change.
> >
> >   When packet is dropped.
> >   
> >   recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,ge
> > neve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-
> > df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:00:00
> > /01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0
> > xf8), packets:14, bytes:1372, used:0.846s, actions:drop
> >   recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:
> > 1e:85),eth_type(0x0800),ipv4(src=192.168.236.152/255.255.255.252,dst=10.
> > 6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no
> >  > ttl=64,frag=no> ),icmp(type=0), packets:6, bytes:588, used:8.983s,
> > actions:drop
> >   
> >
> >   When packet goes through.
> >   
> >   recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,ge
> > neve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-
> > df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:00:00
> > /01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0
> > xf8), packets:3, bytes:294, used:0.104s, actions:12
> >   recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:
> > 1e:85),eth_type(0x0800),ipv4(src=192.168.236.152/255.255.255.252,dst=10.
> > 6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no
> >  > ttl=64,frag=no> ),icmp(type=0), packets:3, bytes:294, used:0.103s,
> > actions:ct_clear,set(tunnel(tun_id=0x1a8ee,dst=10.6.30.92,ttl=64,tp_dst=
> > 6081,geneve({class=0x102,type=0x80,len=4,0x1000b}),flags(df|csum|key))),
> > set(eth(src=fa:16:3e:75:b7:e5,dst=52:54:00:0c:ef:b9)),set(ipv4(ttl=63)),
> > 3
> >   
> >
> >   Is that flow programmed by ovn-controller via ovs-vswitchd?
> >
> > What version of OVN and OVS are you using ?
>
> ovn-20.03.0-2.el8.x86_64
> openvswitch-2.12.0-1.el8.x86_64
>
> > Can you share your OVN NB DB ?
>
> Yes, I can. Let me know how.
>
> > If I understand correctly the packet is received from the patch port to
> > br-int on the gateway node and then tunnelled to the compute node right ?
> > And the packet is dropped on the compute node ?
>
> Yes and yes.
>
> > If you could share your NB DB if it's fine with you and tell the
> > destination logical port, I can try it out locally.
>
> Here is what I had.
> On compute node, ovn-controller is very busy. It keeps saying
> "commit failed".
> 
> 2020-08-05T02:44:23.927Z|04125|reconnect|INFO|tcp:10.6.20.84:6642: connected
> 2020-08-05T02:44:23.936Z|04126|main|INFO|OVNSB commit failed, force recompute 
> next time.
> 2020-08-05T02:44:23.938Z|04127|ovsdb_idl|INFO|tcp:10.6.20.84:6642: clustered 
> database server is disconnected from cluster; trying another server
> 2020-08-05T02:44:23.939Z|04128|reconnect|INFO|tcp:10.6.20.84:6642: connection 
> attempt timed out
> 2020-08-05T02:44:23.939Z|04129|reconnect|INFO|tcp:10.6.20.84:6642: waiting 2 
> seconds before reconnect
> 
>
> The connection to local OVSDB keeps being dropped, because no probe
> response.
> 
> 2020-08-05T02:47:15.437Z|04351|poll_loop|INFO|wakeup due to [POLLIN] on fd 20 
> (10.6.20.22:42362<->10.6.20.86:6642) at lib/stream-fd.c:157 (100% CPU usage)
> 2020-08-05T02:47:15.438Z|04352|reconnect|WARN|tcp:127.0.0.1:6640: connection 
> dropped (Broken pipe)
> 2020-08-05T02:47:15.438Z|04353|rconn|INFO|unix:/var/run/openvswitch/br-int.mgmt:
>  connecting...
> 2020-08-05T02:47:15.449Z|04354|rconn|INFO|unix:/var/run/openvswitch/br-int.mgmt:
>  connected
> 
>
> After set probe interval to 30s, the problem seems gone. I mentioned
> this in another thread "[OVN] no response to inactivity probe".
>
> I can restore probe interval back to default 5s, see if the problem
> can be reproduced. For me, it's important to understand what happens
> behind it.

Ok. This makes sense. Since the connection between ovn-controller and
ovs-vswitchd is braking, the flows are deleted and reprogrammed
and this results in a loop. And the packet gets dropped if flows are missing.

I think increasing the probe interval - ovn-openflow-probe-interval
seems fine to me. It indicates that ovn-controller is taking more than
5 seconds to do a full recompute.

If you take ovn-20.06.01 then ovn-controller should not break often as
there are many optimizations in the latest version to handle the DB
changes more
efficiently.

Thanks

Re: [ovs-dev] [PATCH ovn v2 3/3] ovn-northd.c: Support optionally disabling neighbor learning from ARP request/NS.

2020-08-06 Thread Han Zhou
On Thu, Aug 6, 2020 at 11:22 AM Numan Siddique  wrote:
>
> On Thu, Aug 6, 2020 at 11:45 PM Han Zhou  wrote:
> >
> > On Thu, Aug 6, 2020 at 11:07 AM Numan Siddique  wrote:
> > >
> > > On Wed, Aug 5, 2020 at 12:36 PM Han Zhou  wrote:
> > > >
> > > > Support a new logical router option "always_learn_from_arp_request"
> > that controls
> > > > behavior when handling ARP requests or IPv4 ND-NS packets.
> > > >
> > > > "true" - Always learn the MAC/IP binding and add a new MAC_Binding
entry
> > > > (default behavior)
> > > >
> > > > "false" - If there is a MAC_binding for that IP and the MAC is
> > different, or,
> > > > if TPA of ARP request belongs to any router port on this router,
then
> > > > update/add that MAC/IP binding. Otherwise, don't update/add entries.
> > > >
> > > > Reported-by: Girish Moodalbail 
> > > > Reported-at:
> > https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
> > > > Signed-off-by: Han Zhou 
> > >
> > > Hi Han,
> > >
> > > I have few small nits. Please see below. With those addressed
> > >
> > > Acked-by: Numan Siddique  for all the 3 patches in the
> > series.
> > >
> > >
> > > > ---
> > > >  northd/ovn-northd.8.xml | 109
> > 
> > > >  northd/ovn-northd.c |  96
> > +++---
> > > >  ovn-nb.xml  |  27 
> > > >  tests/ovn.at|  18 
> > > >  4 files changed, 217 insertions(+), 33 deletions(-)
> > > >
> > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > > > index 508123e..b08293f 100644
> > > > --- a/northd/ovn-northd.8.xml
> > > > +++ b/northd/ovn-northd.8.xml
> > > > @@ -1537,58 +1537,126 @@ output;
> > > >  
> > > >For each router port P that owns IP address
> > A,
> > > >which belongs to subnet S with prefix length
> > L,
> > > > -  a priority-100 flow is added which matches
> > > > -  inport == P 
> > > > -  arp.spa == S/L  arp.op ==
> > 1
> > > > -  (ARP request) with the
> > > > -  following actions:
> > > > +  if the option always_learn_from_arp_request
is
> > > > +  true for this router, a priority-100 flow is
> > added which
> > > > +  matches inport == P  arp.spa
==
> > > > +  S/L  arp.op == 1
(ARP
> > request)
> > > > +  with the following actions:
> > > > +
> > > > +
> > > > +
> > > > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > > +next;
> > > > +
> > > > +
> > > > +
> > > > +  If the option always_learn_from_arp_request
is
> > > > +  false, the following two flows are added.
> > > > +
> > > > +
> > > > +
> > > > +  A priority-110 flow is added which matches inport
==
> > > > +  P  arp.spa ==
S/L
> > > > +   arp.tpa == A  arp.op ==
> > 1
> > > > +  (ARP request) with the following actions:
> > > >  
> > > >
> > > >  
> > > >  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > > +reg9[3] = 1;
> > > > +next;
> > > > +
> > > > +
> > > > +
> > > > +  A priority-100 flow is added which matches inport
==
> > > > +  P  arp.spa ==
S/L
> > > > +   arp.op == 1 (ARP request) with the
> > following
> > > > +  actions:
> > > > +
> > > > +
> > > > +
> > > > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > > +reg9[3] = lookup_arp_ip(inport, arp.spa);
> > > >  next;
> > > >  
> > > >
> > > >  
> > > >If the logical router port P is a distributed
> > gateway
> > > >router port, additional match
> > > > -  is_chassis_resident(cr-P) is
added
> > so that
> > > > -  the resident gateway chassis handles the neighbor lookup.
> > > > +  is_chassis_resident(cr-P) is
added
> > for all
> > > > +  these flows.
> > > >  
> > > >
> > > >
> > > >
> > > >  
> > > >A priority-100 flow which matches on ARP reply packets
and
> > applies
> > > > -  the actions:
> > > > +  the actions if the option
> > always_learn_from_arp_request
> > > > +  is true:
> > > >  
> > > >
> > > >  
> > > >  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > >  next;
> > > >  
> > > > +
> > > > +
> > > > +  If the option always_learn_from_arp_request
> > > > +  is false, the above actions will be:
> > > > +
> > > > +
> > > > +
> > > > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > > +reg9[3] = 1;
> > > > +next;
> > > > +
> > > > +
> > > >
> > > >
> > > >
> > > >  
> > > >A priority-100 flow which matches on IPv6 Neighbor
Discovery
> > > > -  advertisement packet and applies the actions:
> > > > +  advertisement packet and applies the actions if the
option
> > > > +  

Re: [ovs-dev] [PATCH ovn] Allow force_snat options to work for dual-stack routers.

2020-08-06 Thread Numan Siddique
On Thu, Jul 16, 2020 at 11:37 PM Mark Michelson  wrote:
>
> The lb_force_snat and dnat_force_snat options could accept only a single
> IP address. For routers that only route traffic of a single IP address
> family, this is fine. However, if a router routes both IPv4 and IPv6
> traffic, then this limitation is a problem.
>
> This patch addresses this problem by allowing for these options to
> specify both an IPv4 and IPv6 address.
>
> Signed-off-by: Mark Michelson 

Hi Mark,

A couple of  comments below.

Thanks
Numan



> ---
>  northd/ovn-northd.c | 179 ---
>  ovn-nb.xml  |  24 +-
>  tests/system-ovn.at | 541 
>  3 files changed, 649 insertions(+), 95 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 192198272..2c05d1c2a 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -7801,44 +7801,37 @@ op_put_v6_networks(struct ds *ds, const struct 
> ovn_port *op)
>  ds_put_cstr(ds, "}");
>  }
>
> -static const char *
> +static bool
>  get_force_snat_ip(struct ovn_datapath *od, const char *key_type,
> -  struct v46_ip *ip)
> +  struct lport_addresses *laddrs)
>  {
>  char *key = xasprintf("%s_force_snat_ip", key_type);
> -const char *ip_address = smap_get(>nbr->options, key);
> +const char *addresses = smap_get(>nbr->options, key);
>  free(key);
>
> -if (ip_address) {
> -ovs_be32 mask;
> -ip->family = AF_INET;
> -char *error = ip_parse_masked(ip_address, >ipv4, );
> -if (error || mask != OVS_BE32_MAX) {
> -free(error);
> -struct in6_addr mask_v6, v6_exact = IN6ADDR_EXACT_INIT;
> -ip->family = AF_INET6;
> -error = ipv6_parse_masked(ip_address, >ipv6, _v6);
> -if (error || memcmp(_v6, _exact, sizeof(mask_v6))) {
> -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 
> 1);
> -VLOG_WARN_RL(, "bad ip %s in options of router 
> "UUID_FMT"",
> - ip_address, UUID_ARGS(>key));
> -memset(ip, 0, sizeof *ip);
> -ip->family = AF_UNSPEC;
> -return NULL;
> -}
> -}
> -return ip_address;
> +if (!addresses) {
> +return false;
>  }
>
> -memset(ip, 0, sizeof *ip);
> -ip->family = AF_UNSPEC;
> -return NULL;
> +if (!extract_ip_addresses(addresses, laddrs) ||
> +laddrs->n_ipv4_addrs > 1 ||
> +laddrs->n_ipv6_addrs > 1 ||
> +(laddrs->n_ipv4_addrs && laddrs->ipv4_addrs[0].plen != 32) ||
> +(laddrs->n_ipv6_addrs && laddrs->ipv6_addrs[0].plen != 128)) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +VLOG_WARN_RL(, "bad ip %s in options of router "UUID_FMT"",
> + addresses, UUID_ARGS(>key));
> +destroy_lport_addresses(laddrs);
> +return false;
> +}
> +
> +return true;
>  }
>
>  static void
>  add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
> struct ds *match, struct ds *actions, int priority,
> -   const char *lb_force_snat_ip, struct lb_vip *lb_vip,
> +   bool lb_force_snat_ip, struct lb_vip *lb_vip,
> const char *proto, struct nbrec_load_balancer *lb,
> struct shash *meter_groups, struct sset *nat_entries)
>  {
> @@ -8159,6 +8152,29 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct 
> ovn_port *op,
>  ds_destroy();
>  }
>
> +static void
> +build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od,
> +   const char *ip_version, const char *ip_addr,
> +   const char *context)
> +{
> +struct ds match = DS_EMPTY_INITIALIZER;
> +struct ds actions = DS_EMPTY_INITIALIZER;
> +ds_put_format(, "ip%s && ip%s.dst == %s",
> +  ip_version, ip_version, ip_addr);
> +ovn_lflow_add(lflows, od, S_ROUTER_IN_UNSNAT, 110,
> +  ds_cstr(), "ct_snat;");
> +
> +/* Higher priority rules to force SNAT with the IP addresses
> + * configured in the Gateway router.  This only takes effect
> + * when the packet has already been DNATed or load balanced once. */
> +ds_clear();
> +ds_put_format(, "flags.force_snat_for_%s == 1 && ip%s",
> +  context, ip_version);
> +ds_put_format(, "ct_snat(%s);", ip_addr);
> +ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 100,
> +  ds_cstr(), ds_cstr());

There is a memory leak here. I think you need to call ds_destroy for
both 'match' and 'actions'.


> +}
> +
>  static void
>  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>  struct hmap *lflows, struct shash *meter_groups,
> @@ -8609,24 +8625,37 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>

Re: [ovs-dev] packet drop

2020-08-06 Thread Tony Liu
Inline...

Thanks!

Tony
> -Original Message-
> From: Numan Siddique 
> Sent: Thursday, August 6, 2020 10:03 AM
> To: Tony Liu 
> Cc: ovs-disc...@openvswitch.org; ovs-dev@openvswitch.org
> Subject: Re: [ovs-dev] packet drop
> 
> 
> 
> On Thu, Aug 6, 2020 at 4:05 AM Tony Liu   > wrote:
> 
> 
> 
>   The drop is caused by flow change.
> 
>   When packet is dropped.
>   
>   recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,ge
> neve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-
> df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:00:00
> /01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0
> xf8), packets:14, bytes:1372, used:0.846s, actions:drop
>   recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:
> 1e:85),eth_type(0x0800),ipv4(src=192.168.236.152/255.255.255.252,dst=10.
> 6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no
>  ttl=64,frag=no> ),icmp(type=0), packets:6, bytes:588, used:8.983s,
> actions:drop
>   
> 
>   When packet goes through.
>   
>   recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,ge
> neve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-
> df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:00:00
> /01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0
> xf8), packets:3, bytes:294, used:0.104s, actions:12
>   recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:
> 1e:85),eth_type(0x0800),ipv4(src=192.168.236.152/255.255.255.252,dst=10.
> 6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no
>  ttl=64,frag=no> ),icmp(type=0), packets:3, bytes:294, used:0.103s,
> actions:ct_clear,set(tunnel(tun_id=0x1a8ee,dst=10.6.30.92,ttl=64,tp_dst=
> 6081,geneve({class=0x102,type=0x80,len=4,0x1000b}),flags(df|csum|key))),
> set(eth(src=fa:16:3e:75:b7:e5,dst=52:54:00:0c:ef:b9)),set(ipv4(ttl=63)),
> 3
>   
> 
>   Is that flow programmed by ovn-controller via ovs-vswitchd?
> 
> What version of OVN and OVS are you using ?

ovn-20.03.0-2.el8.x86_64
openvswitch-2.12.0-1.el8.x86_64

> Can you share your OVN NB DB ?

Yes, I can. Let me know how.

> If I understand correctly the packet is received from the patch port to
> br-int on the gateway node and then tunnelled to the compute node right ?
> And the packet is dropped on the compute node ?

Yes and yes.

> If you could share your NB DB if it's fine with you and tell the
> destination logical port, I can try it out locally.

Here is what I had.
On compute node, ovn-controller is very busy. It keeps saying
"commit failed".

2020-08-05T02:44:23.927Z|04125|reconnect|INFO|tcp:10.6.20.84:6642: connected
2020-08-05T02:44:23.936Z|04126|main|INFO|OVNSB commit failed, force recompute 
next time.
2020-08-05T02:44:23.938Z|04127|ovsdb_idl|INFO|tcp:10.6.20.84:6642: clustered 
database server is disconnected from cluster; trying another server
2020-08-05T02:44:23.939Z|04128|reconnect|INFO|tcp:10.6.20.84:6642: connection 
attempt timed out
2020-08-05T02:44:23.939Z|04129|reconnect|INFO|tcp:10.6.20.84:6642: waiting 2 
seconds before reconnect


The connection to local OVSDB keeps being dropped, because no probe
response.

2020-08-05T02:47:15.437Z|04351|poll_loop|INFO|wakeup due to [POLLIN] on fd 20 
(10.6.20.22:42362<->10.6.20.86:6642) at lib/stream-fd.c:157 (100% CPU usage)
2020-08-05T02:47:15.438Z|04352|reconnect|WARN|tcp:127.0.0.1:6640: connection 
dropped (Broken pipe)
2020-08-05T02:47:15.438Z|04353|rconn|INFO|unix:/var/run/openvswitch/br-int.mgmt:
 connecting...
2020-08-05T02:47:15.449Z|04354|rconn|INFO|unix:/var/run/openvswitch/br-int.mgmt:
 connected


After set probe interval to 30s, the problem seems gone. I mentioned
this in another thread "[OVN] no response to inactivity probe".

I can restore probe interval back to default 5s, see if the problem
can be reproduced. For me, it's important to understand what happens
behind it.

> 
> Thanks
> Numan
> 
> 
> 
> 
> 
>   Thanks!
> 
>   Tony
> 
>   > -Original Message-
>   > From: discuss mailto:ovs-
> discuss-boun...@openvswitch.org> > On Behalf Of Tony
>   > Liu
>   > Sent: Wednesday, August 5, 2020 2:48 PM
>   > To: ovs-disc...@openvswitch.org  disc...@openvswitch.org> ; ovs-dev@openvswitch.org  d...@openvswitch.org>
>   > Subject: [ovs-discuss] packet drop
>   >
>   > Hi,
>   >
>   > I am running ping from external to VM via OVN gateway.
>   > On the compute node, ICMP request packet is consistently coming
> into
>   > interface "ovn-gatewa-1". But there is about 10 out of 25 packet
> loss on
>   > tap interface. It's like the switch pauses 10s after every 15s.
>   >
>   

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Han Zhou
On Thu, Aug 6, 2020 at 11:11 AM Tony Liu  wrote:
>
> Inline... (please read with monospaced font:))
>
> Thanks!
>
> Tony
> > -Original Message-
> > From: Han Zhou 
> > Sent: Wednesday, August 5, 2020 11:48 PM
> > To: Tony Liu 
> > Cc: Han Zhou ; Numan Siddique ; ovs-dev
> > ; ovs-discuss 
> > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> >
> >
> >
> > On Wed, Aug 5, 2020 at 9:14 PM Tony Liu  >  > wrote:
> >
> >
> >   I set the connection target="ptcp:6641:10.6.20.84" for ovn-nb-db
> >   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first node
> >   of cluster. Also ovn-openflow-probe-interval=30 on compute node.
> >   It seems helping. Not that many connect/drop/reconnect in logging.
> >   That "commit failure" is also gone.
> >   The issue I reported in another thread "packet drop" seems gone.
> >   And launching VM starts working.
> >
> >   How should I set connection table for all ovn-nb-db and ovn-sb-db
> >   nodes in the cluster to set inactivity_probe?
> >   One row with address 0.0.0.0 seems not working.
> >
> > You can simply use 0.0.0.0 in the connection table, but don't specify
> > the same connection method on the command line when starting ovsdb-
> > server for NB/SB DB. Otherwise, these are conflicting and that's why you
> > saw "Address already in use" error.
>
> Could you share a bit details how it works?
> I thought the row in connection table only tells nbdb and sbdb the
> probe interval. Isn't that right? Does nbdb and sbdb also create
> socket based on target column?
>

In --remote option of ovsdb-server, you can specify either a connection
method directly, or specify the db,table,column which contains the
connection information.
Please see manpage ovsdb-server(1).

> >
> >   Is "external_ids:ovn-remote-probe-interval" in ovsdb-server on
> >   compute node for ovn-controller to probe ovn-sb-db?
> >
> > OVSDB probe is bidirectional, so you need to set this value, too, if you
> > don't want too many probes handled by the SB server. (setting the
> > connection table for SB only changes the server side).
>
> In that case, how do I set probe interval for ovn-controller?
> My understanding is that, ovn-controller reads configuration from
> ovsdb-server on the local compute node. Isn't that right?
>

The configuration you mentioned "external_ids:ovn-remote-probe-interval" is
exactly the way to set the ovn-controller -> SB probe interval.
(SB -> ovn-controller probe is set in the connection table of SB)

You are right that ovn-controller reads configuration from the local
ovsdb-server. This setting is in local ovsdb-server.

> >   Is "external_ids:ovn-openflow-probe-interval" in ovsdb-server on
> >   compute node for ovn-controller to probe ovsdb-server?
> >
> > It is for the OpenFlow connection between ovn-controller and ovs-
> > vswitchd, which is part of the OpenFlow protocol.
> >
> >   What's probe interval for ovsdb-server to probe ovn-controller?
> >
> > The local ovsdb connection uses unix socket, which doesn't send probe by
> > default (if I remember correctly).
>
> Here is how ovsdb-server and ovn-controller is invoked on compute node.
> 
> root 41129  0.0  0.0 157556 20532 ?SJul30   1:51
/usr/sbin/ovsdb-server /var/lib/openvswitch/conf.db -vconsole:emer
-vsyslog:err -vfile:info --remote=punix:/run/openvswitch/db.sock
--remote=ptcp:6640:127.0.0.1
--remote=db:Open_vSwitch,Open_vSwitch,manager_options
--log-file=/var/log/kolla/openvswitch/ovsdb-server.log --pidfile
>
> root 63775 55.9  0.4 1477796 1224324 ? Sl   Aug04 1360:55
/usr/bin/ovn-controller --pidfile=/run/ovn/ovn-controller.pid
--log-file=/var/log/kolla/openvswitch/ovn-controller.log tcp:127.0.0.1:6640
> 
> Is that OK? Or UNIX socket method is recommended for ovn-controller
> to connect to ovsdb-server?

If using TCP, by default it is 5s probe interval. I think it is better to
use unix socket. (but maybe it doesn't matter that much)

>
> Here is the configuration in open_vswitch table in ovsdb-server.
> 
> external_ids: {ovn-encap-ip="10.6.30.22", ovn-encap-type=geneve,
ovn-openflow-probe-interval="30", ovn-remote="tcp:10.6.20.84:6642,tcp:
10.6.20.85:6642,tcp:10.6.20.86:6642", ovn-remote-probe-interval="6",
system-id="compute-3"}
> 
> ovn-controller connects to ovsdb-server and reads this configuration,
> so it knows how to connect to all sbdb nodes, right?
>
Yes, you are right.

> If it's TCP between ovn-controller and ovsdb-server, is that probe
> interval setting will also apply to the probe from ovn-controller to
> ovsdb-server?
>
No. They are not related.

> ovn-controller connects to ovs-vswitchd by UNIX socket to program
> open-flow. ovs-vswitchd and ovsdb-server are connected by UNIX too.
> So, is that ovn-openflow-probe-interval for the probe from ovn-controller
> to ovs-vswitchd via UNIX?
>
> As a summary for the probe 

Re: [ovs-dev] [PATCH ovn v2 1/2] ovn-northd: Don't send the pkt to conntrack if it is to be routed in egress stage.

2020-08-06 Thread Dumitru Ceara
On 8/4/20 9:19 AM, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> If there is a logical port 'P1' with the IP - 10.0.0.3 and a logical port 
> 'P2' with
> the IP 20.0.0.3 and if the logical switch of 'P1' has atleast one load 
> balancer
> associated with it and atleast one ACL with allow-related action associated 
> with it.
> Then for every packet from 'P1' to 'P2' after the TCP connection
> is established we see a total of 4 recirculations in the datapath on the 
> chassis
> claiming 'P1'. This is because,
> 
> In the ingress logical switch pipeline, below logical flows are hit
>   - table=9 (ls_in_lb   ), priority=65535, match=(ct.est && !ct.rel 
> && !ct.new && !ct.inv), action=(reg0[2] = 1; next;)
>   - table=10(ls_in_stateful ), priority=100  , match=(reg0[2] == 1), 
> action=(ct_lb;)
> 
> And in the egress logical switch pipeline, below logical flows are hit
>  - table=0 (ls_out_pre_lb  ), priority=100  , match=(ip), action=(reg0[0] 
> = 1; next;)
>  - table=2 (ls_out_pre_stateful), priority=100  , match=(reg0[0] == 1), 
> action=(ct_next;)
>  - table=3 (ls_out_lb  ), priority=65535, match=(ct.est && !ct.rel && 
> !ct.new && !ct.inv), action=(reg0[2] = 1; next;)
>  - table=7 (ls_out_stateful), priority=100  , match=(reg0[2] == 1), 
> action=(ct_lb;)
> 
> In the above example, when the packet enters the egress pipeline and since it 
> needs to
> enter the router pipeline, we can skip setting reg0[0] if outport is peer 
> port of
> logical router port. There is no need to send the packet to conntrack in this 
> case.
> 
> This patch handles this case for router ports. Next patch in the series 
> avoids sending to
> conntrack with the action - ct_lb if the packet is not destined to the LB VIP.
> 
> With the present master for the above example, we see total of 4 
> recirculations on the
> chassis claiming the lport 'P1'. With this patch we see only 2 recirculations.
> 
> Signed-off-by: Numan Siddique 

Hi Numan,

Looks good to me:

Acked-by: Dumitru Ceara 

Thanks,
Dumitru

> ---
> 
> v1 -> v2
> 
>  * No change.
> 
>  northd/ovn-northd.8.xml | 33 -
>  northd/ovn-northd.c | 39 ++-
>  2 files changed, 62 insertions(+), 10 deletions(-)
> 
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index ed1cd58e70..b741f49347 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -366,6 +366,15 @@
>db="OVN_Northbound"/> table.
>  
>  
> +
> +  This table also has a priority-110 flow with the match
> +  inport == I for all logical switch
> +  datapaths to move traffic to the next table. Where I
> +  is the peer of a logical router port. This flow is added to
> +  skip the connection tracking of packets which enter from
> +  logical router datapath to logical switch datapath.
> +
> +
>  Ingress Table 5: Pre-stateful
>  
>  
> @@ -533,7 +542,20 @@
>  
>  
>It contains a priority-0 flow that simply moves traffic to the next
> -  table.  For established connections a priority 100 flow matches on
> +  table.
> +
> +
> +
> +  A priority-65535 flow with the match
> +  inport == I for all logical switch
> +  datapaths to move traffic to the next table. Where I
> +  is the peer of a logical router port. This flow is added to
> +  skip the connection tracking of packets which enter from
> +  logical router datapath to logical switch datapath.
> +
> +
> +
> +  For established connections a priority 65534 flow matches on
>ct.est  !ct.rel  !ct.new 
>!ct.inv and sets an action reg0[2] = 1; next; to 
> act
>as a hint for table Stateful to send packets through
> @@ -1359,6 +1381,15 @@ output;
>db="OVN_Northbound"/> table.
>  
>  
> +
> +  This table also has a priority-110 flow with the match
> +  outport == I for all logical switch
> +  datapaths to move traffic to the next table. Where I
> +  is the peer of a logical router port. This flow is added to
> +  skip the connection tracking of packets which will be entering
> +  logical router datapath from logical switch datapath for routing.
> +
> +
>  Egress Table 2: Pre-stateful
>  
>  
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 03c62bafaa..c7b1239adf 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4850,8 +4850,9 @@ build_lswitch_output_port_sec(struct hmap *ports, 
> struct hmap *datapaths,
>  }
>  
>  static void
> -build_pre_acl_flows(struct ovn_datapath *od, struct ovn_port *op,
> -struct hmap *lflows)
> +skip_port_from_conntrack(struct ovn_datapath *od, struct ovn_port *op,
> + enum ovn_stage in_stage, enum ovn_stage out_stage,
> + uint16_t priority, struct hmap *lflows)
>  {
>  /* Can't use ct() for router ports. Consider the 

Re: [ovs-dev] [PATCH ovn v2 3/3] ovn-northd.c: Support optionally disabling neighbor learning from ARP request/NS.

2020-08-06 Thread Numan Siddique
On Thu, Aug 6, 2020 at 11:45 PM Han Zhou  wrote:
>
> On Thu, Aug 6, 2020 at 11:07 AM Numan Siddique  wrote:
> >
> > On Wed, Aug 5, 2020 at 12:36 PM Han Zhou  wrote:
> > >
> > > Support a new logical router option "always_learn_from_arp_request"
> that controls
> > > behavior when handling ARP requests or IPv4 ND-NS packets.
> > >
> > > "true" - Always learn the MAC/IP binding and add a new MAC_Binding entry
> > > (default behavior)
> > >
> > > "false" - If there is a MAC_binding for that IP and the MAC is
> different, or,
> > > if TPA of ARP request belongs to any router port on this router, then
> > > update/add that MAC/IP binding. Otherwise, don't update/add entries.
> > >
> > > Reported-by: Girish Moodalbail 
> > > Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
> > > Signed-off-by: Han Zhou 
> >
> > Hi Han,
> >
> > I have few small nits. Please see below. With those addressed
> >
> > Acked-by: Numan Siddique  for all the 3 patches in the
> series.
> >
> >
> > > ---
> > >  northd/ovn-northd.8.xml | 109
> 
> > >  northd/ovn-northd.c |  96
> +++---
> > >  ovn-nb.xml  |  27 
> > >  tests/ovn.at|  18 
> > >  4 files changed, 217 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > > index 508123e..b08293f 100644
> > > --- a/northd/ovn-northd.8.xml
> > > +++ b/northd/ovn-northd.8.xml
> > > @@ -1537,58 +1537,126 @@ output;
> > >  
> > >For each router port P that owns IP address
> A,
> > >which belongs to subnet S with prefix length
> L,
> > > -  a priority-100 flow is added which matches
> > > -  inport == P 
> > > -  arp.spa == S/L  arp.op ==
> 1
> > > -  (ARP request) with the
> > > -  following actions:
> > > +  if the option always_learn_from_arp_request is
> > > +  true for this router, a priority-100 flow is
> added which
> > > +  matches inport == P  arp.spa ==
> > > +  S/L  arp.op == 1 (ARP
> request)
> > > +  with the following actions:
> > > +
> > > +
> > > +
> > > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > +next;
> > > +
> > > +
> > > +
> > > +  If the option always_learn_from_arp_request is
> > > +  false, the following two flows are added.
> > > +
> > > +
> > > +
> > > +  A priority-110 flow is added which matches inport ==
> > > +  P  arp.spa == S/L
> > > +   arp.tpa == A  arp.op ==
> 1
> > > +  (ARP request) with the following actions:
> > >  
> > >
> > >  
> > >  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > +reg9[3] = 1;
> > > +next;
> > > +
> > > +
> > > +
> > > +  A priority-100 flow is added which matches inport ==
> > > +  P  arp.spa == S/L
> > > +   arp.op == 1 (ARP request) with the
> following
> > > +  actions:
> > > +
> > > +
> > > +
> > > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > +reg9[3] = lookup_arp_ip(inport, arp.spa);
> > >  next;
> > >  
> > >
> > >  
> > >If the logical router port P is a distributed
> gateway
> > >router port, additional match
> > > -  is_chassis_resident(cr-P) is added
> so that
> > > -  the resident gateway chassis handles the neighbor lookup.
> > > +  is_chassis_resident(cr-P) is added
> for all
> > > +  these flows.
> > >  
> > >
> > >
> > >
> > >  
> > >A priority-100 flow which matches on ARP reply packets and
> applies
> > > -  the actions:
> > > +  the actions if the option
> always_learn_from_arp_request
> > > +  is true:
> > >  
> > >
> > >  
> > >  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > >  next;
> > >  
> > > +
> > > +
> > > +  If the option always_learn_from_arp_request
> > > +  is false, the above actions will be:
> > > +
> > > +
> > > +
> > > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > > +reg9[3] = 1;
> > > +next;
> > > +
> > > +
> > >
> > >
> > >
> > >  
> > >A priority-100 flow which matches on IPv6 Neighbor Discovery
> > > -  advertisement packet and applies the actions:
> > > +  advertisement packet and applies the actions if the option
> > > +  always_learn_from_arp_request is
> true:
> > >  
> > >
> > >  
> > >  reg9[2] = lookup_nd(inport, nd.target, nd.tll);
> > >  next;
> > >  
> > > +
> > > +
> > > +  If the option always_learn_from_arp_request
> > > +  is false, the above actions will be:
> > > +
> > > +
> > > +
> > > +reg9[2] = 

Re: [ovs-dev] [PATCH ovn v2 3/3] ovn-northd.c: Support optionally disabling neighbor learning from ARP request/NS.

2020-08-06 Thread Han Zhou
On Thu, Aug 6, 2020 at 11:07 AM Numan Siddique  wrote:
>
> On Wed, Aug 5, 2020 at 12:36 PM Han Zhou  wrote:
> >
> > Support a new logical router option "always_learn_from_arp_request"
that controls
> > behavior when handling ARP requests or IPv4 ND-NS packets.
> >
> > "true" - Always learn the MAC/IP binding and add a new MAC_Binding entry
> > (default behavior)
> >
> > "false" - If there is a MAC_binding for that IP and the MAC is
different, or,
> > if TPA of ARP request belongs to any router port on this router, then
> > update/add that MAC/IP binding. Otherwise, don't update/add entries.
> >
> > Reported-by: Girish Moodalbail 
> > Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
> > Signed-off-by: Han Zhou 
>
> Hi Han,
>
> I have few small nits. Please see below. With those addressed
>
> Acked-by: Numan Siddique  for all the 3 patches in the
series.
>
>
> > ---
> >  northd/ovn-northd.8.xml | 109

> >  northd/ovn-northd.c |  96
+++---
> >  ovn-nb.xml  |  27 
> >  tests/ovn.at|  18 
> >  4 files changed, 217 insertions(+), 33 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 508123e..b08293f 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1537,58 +1537,126 @@ output;
> >  
> >For each router port P that owns IP address
A,
> >which belongs to subnet S with prefix length
L,
> > -  a priority-100 flow is added which matches
> > -  inport == P 
> > -  arp.spa == S/L  arp.op ==
1
> > -  (ARP request) with the
> > -  following actions:
> > +  if the option always_learn_from_arp_request is
> > +  true for this router, a priority-100 flow is
added which
> > +  matches inport == P  arp.spa ==
> > +  S/L  arp.op == 1 (ARP
request)
> > +  with the following actions:
> > +
> > +
> > +
> > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > +next;
> > +
> > +
> > +
> > +  If the option always_learn_from_arp_request is
> > +  false, the following two flows are added.
> > +
> > +
> > +
> > +  A priority-110 flow is added which matches inport ==
> > +  P  arp.spa == S/L
> > +   arp.tpa == A  arp.op ==
1
> > +  (ARP request) with the following actions:
> >  
> >
> >  
> >  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > +reg9[3] = 1;
> > +next;
> > +
> > +
> > +
> > +  A priority-100 flow is added which matches inport ==
> > +  P  arp.spa == S/L
> > +   arp.op == 1 (ARP request) with the
following
> > +  actions:
> > +
> > +
> > +
> > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > +reg9[3] = lookup_arp_ip(inport, arp.spa);
> >  next;
> >  
> >
> >  
> >If the logical router port P is a distributed
gateway
> >router port, additional match
> > -  is_chassis_resident(cr-P) is added
so that
> > -  the resident gateway chassis handles the neighbor lookup.
> > +  is_chassis_resident(cr-P) is added
for all
> > +  these flows.
> >  
> >
> >
> >
> >  
> >A priority-100 flow which matches on ARP reply packets and
applies
> > -  the actions:
> > +  the actions if the option
always_learn_from_arp_request
> > +  is true:
> >  
> >
> >  
> >  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> >  next;
> >  
> > +
> > +
> > +  If the option always_learn_from_arp_request
> > +  is false, the above actions will be:
> > +
> > +
> > +
> > +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> > +reg9[3] = 1;
> > +next;
> > +
> > +
> >
> >
> >
> >  
> >A priority-100 flow which matches on IPv6 Neighbor Discovery
> > -  advertisement packet and applies the actions:
> > +  advertisement packet and applies the actions if the option
> > +  always_learn_from_arp_request is
true:
> >  
> >
> >  
> >  reg9[2] = lookup_nd(inport, nd.target, nd.tll);
> >  next;
> >  
> > +
> > +
> > +  If the option always_learn_from_arp_request
> > +  is false, the above actions will be:
> > +
> > +
> > +
> > +reg9[2] = lookup_nd(inport, nd.target, nd.tll);
> > +reg9[3] = 1;
> > +next;
> > +
> >
> >
> >
> >  
> >A priority-100 flow which matches on IPv6 Neighbor Discovery
> > -  solicitation packet and applies the actions:
> > +  solicitation packet and applies the actions if the option
> > +  always_learn_from_arp_request is
true:
> > +  

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Tony Liu
Inline... (please read with monospaced font:))

Thanks!

Tony
> -Original Message-
> From: Han Zhou 
> Sent: Wednesday, August 5, 2020 11:48 PM
> To: Tony Liu 
> Cc: Han Zhou ; Numan Siddique ; ovs-dev
> ; ovs-discuss 
> Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> 
> 
> 
> On Wed, Aug 5, 2020 at 9:14 PM Tony Liu   > wrote:
> 
> 
>   I set the connection target="ptcp:6641:10.6.20.84" for ovn-nb-db
>   and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first node
>   of cluster. Also ovn-openflow-probe-interval=30 on compute node.
>   It seems helping. Not that many connect/drop/reconnect in logging.
>   That "commit failure" is also gone.
>   The issue I reported in another thread "packet drop" seems gone.
>   And launching VM starts working.
> 
>   How should I set connection table for all ovn-nb-db and ovn-sb-db
>   nodes in the cluster to set inactivity_probe?
>   One row with address 0.0.0.0 seems not working.
> 
> You can simply use 0.0.0.0 in the connection table, but don't specify
> the same connection method on the command line when starting ovsdb-
> server for NB/SB DB. Otherwise, these are conflicting and that's why you
> saw "Address already in use" error.

Could you share a bit details how it works?
I thought the row in connection table only tells nbdb and sbdb the
probe interval. Isn't that right? Does nbdb and sbdb also create
socket based on target column?

> 
>   Is "external_ids:ovn-remote-probe-interval" in ovsdb-server on
>   compute node for ovn-controller to probe ovn-sb-db?
> 
> OVSDB probe is bidirectional, so you need to set this value, too, if you
> don't want too many probes handled by the SB server. (setting the
> connection table for SB only changes the server side).

In that case, how do I set probe interval for ovn-controller?
My understanding is that, ovn-controller reads configuration from
ovsdb-server on the local compute node. Isn't that right?

>   Is "external_ids:ovn-openflow-probe-interval" in ovsdb-server on
>   compute node for ovn-controller to probe ovsdb-server?
> 
> It is for the OpenFlow connection between ovn-controller and ovs-
> vswitchd, which is part of the OpenFlow protocol.
> 
>   What's probe interval for ovsdb-server to probe ovn-controller?
> 
> The local ovsdb connection uses unix socket, which doesn't send probe by
> default (if I remember correctly).

Here is how ovsdb-server and ovn-controller is invoked on compute node.

root 41129  0.0  0.0 157556 20532 ?SJul30   1:51 
/usr/sbin/ovsdb-server /var/lib/openvswitch/conf.db -vconsole:emer -vsyslog:err 
-vfile:info --remote=punix:/run/openvswitch/db.sock 
--remote=ptcp:6640:127.0.0.1 
--remote=db:Open_vSwitch,Open_vSwitch,manager_options 
--log-file=/var/log/kolla/openvswitch/ovsdb-server.log --pidfile

root 63775 55.9  0.4 1477796 1224324 ? Sl   Aug04 1360:55 
/usr/bin/ovn-controller --pidfile=/run/ovn/ovn-controller.pid 
--log-file=/var/log/kolla/openvswitch/ovn-controller.log tcp:127.0.0.1:6640

Is that OK? Or UNIX socket method is recommended for ovn-controller
to connect to ovsdb-server?

Here is the configuration in open_vswitch table in ovsdb-server.

external_ids: {ovn-encap-ip="10.6.30.22", ovn-encap-type=geneve, 
ovn-openflow-probe-interval="30", 
ovn-remote="tcp:10.6.20.84:6642,tcp:10.6.20.85:6642,tcp:10.6.20.86:6642", 
ovn-remote-probe-interval="6", system-id="compute-3"}

ovn-controller connects to ovsdb-server and reads this configuration,
so it knows how to connect to all sbdb nodes, right?

If it's TCP between ovn-controller and ovsdb-server, is that probe
interval setting will also apply to the probe from ovn-controller to
ovsdb-server?

ovn-controller connects to ovs-vswitchd by UNIX socket to program
open-flow. ovs-vswitchd and ovsdb-server are connected by UNIX too.
So, is that ovn-openflow-probe-interval for the probe from ovn-controller
to ovs-vswitchd via UNIX?

As a summary for the probe setting,

+--+  driver configuration
|  ovn-driver  |
+--+
^|
|v
+--+  inactivity_probe in table "Connection"
|  ovn-nb-db   |
+--+
^|
|v
+--+  options:northd_probe_interval in table "NB_Global"
|  ovn-northd  |  in nbdb.
+--+
^|
|v
+--+  inactivity_probe in table "Connection"
|  ovn-sb-db   |
+--+
^|
|v
++  in table "Open_vSwitch" in ovsdb-server
|ovn-controller  |  ovn-remote-probe-interval for TCP
++  probe to ovsdb-server,
^|^|ovn-openflow-probe-interval for UNIX
|v TCP|v UNIX   probe to ovs-vswitchd
+--+  +--+
| ovsdb-server |  | ovs-vswitchd |
+--+  +--+

Is that 

Re: [ovs-dev] [PATCH ovn v2 3/3] ovn-northd.c: Support optionally disabling neighbor learning from ARP request/NS.

2020-08-06 Thread Numan Siddique
On Wed, Aug 5, 2020 at 12:36 PM Han Zhou  wrote:
>
> Support a new logical router option "always_learn_from_arp_request" that 
> controls
> behavior when handling ARP requests or IPv4 ND-NS packets.
>
> "true" - Always learn the MAC/IP binding and add a new MAC_Binding entry
> (default behavior)
>
> "false" - If there is a MAC_binding for that IP and the MAC is different, or,
> if TPA of ARP request belongs to any router port on this router, then
> update/add that MAC/IP binding. Otherwise, don't update/add entries.
>
> Reported-by: Girish Moodalbail 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-May/049995.html
> Signed-off-by: Han Zhou 

Hi Han,

I have few small nits. Please see below. With those addressed

Acked-by: Numan Siddique  for all the 3 patches in the series.


> ---
>  northd/ovn-northd.8.xml | 109 
> 
>  northd/ovn-northd.c |  96 +++---
>  ovn-nb.xml  |  27 
>  tests/ovn.at|  18 
>  4 files changed, 217 insertions(+), 33 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 508123e..b08293f 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1537,58 +1537,126 @@ output;
>  
>For each router port P that owns IP address 
> A,
>which belongs to subnet S with prefix length 
> L,
> -  a priority-100 flow is added which matches
> -  inport == P 
> -  arp.spa == S/L  arp.op == 1
> -  (ARP request) with the
> -  following actions:
> +  if the option always_learn_from_arp_request is
> +  true for this router, a priority-100 flow is added 
> which
> +  matches inport == P  arp.spa ==
> +  S/L  arp.op == 1 (ARP 
> request)
> +  with the following actions:
> +
> +
> +
> +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> +next;
> +
> +
> +
> +  If the option always_learn_from_arp_request is
> +  false, the following two flows are added.
> +
> +
> +
> +  A priority-110 flow is added which matches inport ==
> +  P  arp.spa == S/L
> +   arp.tpa == A  arp.op == 1
> +  (ARP request) with the following actions:
>  
>
>  
>  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> +reg9[3] = 1;
> +next;
> +
> +
> +
> +  A priority-100 flow is added which matches inport ==
> +  P  arp.spa == S/L
> +   arp.op == 1 (ARP request) with the following
> +  actions:
> +
> +
> +
> +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> +reg9[3] = lookup_arp_ip(inport, arp.spa);
>  next;
>  
>
>  
>If the logical router port P is a distributed gateway
>router port, additional match
> -  is_chassis_resident(cr-P) is added so that
> -  the resident gateway chassis handles the neighbor lookup.
> +  is_chassis_resident(cr-P) is added for all
> +  these flows.
>  
>
>
>
>  
>A priority-100 flow which matches on ARP reply packets and applies
> -  the actions:
> +  the actions if the option 
> always_learn_from_arp_request
> +  is true:
>  
>
>  
>  reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
>  next;
>  
> +
> +
> +  If the option always_learn_from_arp_request
> +  is false, the above actions will be:
> +
> +
> +
> +reg9[2] = lookup_arp(inport, arp.spa, arp.sha);
> +reg9[3] = 1;
> +next;
> +
> +
>
>
>
>  
>A priority-100 flow which matches on IPv6 Neighbor Discovery
> -  advertisement packet and applies the actions:
> +  advertisement packet and applies the actions if the option
> +  always_learn_from_arp_request is true:
>  
>
>  
>  reg9[2] = lookup_nd(inport, nd.target, nd.tll);
>  next;
>  
> +
> +
> +  If the option always_learn_from_arp_request
> +  is false, the above actions will be:
> +
> +
> +
> +reg9[2] = lookup_nd(inport, nd.target, nd.tll);
> +reg9[3] = 1;
> +next;
> +
>
>
>
>  
>A priority-100 flow which matches on IPv6 Neighbor Discovery
> -  solicitation packet and applies the actions:
> +  solicitation packet and applies the actions if the option
> +  always_learn_from_arp_request is true:
> +
> +
> +
> +reg9[2] = lookup_nd(inport, ip6.src, nd.sll);
> +next;
> +
> +
> +
> +  If the option always_learn_from_arp_request
> +  is false, the above actions will be:
>  
>
>  
>  reg9[2] = lookup_nd(inport, ip6.src, nd.sll);
> +reg9[3] = lookup_nd_ip(inport, ip6.src);

Re: [ovs-dev] [PATCH ovn v2 2/2] ovn-northd: Don't send the pkt to conntrack for NAT if its not destined for LB VIP.

2020-08-06 Thread Dumitru Ceara
On 8/4/20 9:19 AM, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Presently when a logical switch has load balancer(s) associated to it, then 
> the
> packet is still sent to conntrack with the action ct_lb on both the ingress
> and egress logical switch pipeline even if the destination IP is not LB VIP.
> 
> This is because below logical flows are hit:
> 
> In the ingress logical switch pipeline:
>   - table=9 (ls_in_lb   ), priority=65535, match=(ct.est && !ct.rel && 
> !ct.new && !ct.inv), action=(reg0[2] = 1; next;)
>   - table=10(ls_in_stateful ), priority=100  , match=(reg0[2] == 1), 
> action=(ct_lb;)
> 
> In the egress logical switch pipeline:
>   - table=3 (ls_out_lb  ), priority=65535, match=(ct.est && !ct.rel && 
> !ct.new && !ct.inv), action=(reg0[2] = 1; next;)
>   - table=7 (ls_out_stateful), priority=100  , match=(reg0[2] == 1), 
> action=(ct_lb;)
> 
> This patch avoid unnecessary ct actions by setting the ct_mark to 0x1/0x1 
> when the ct_lb(backends=...) action
> is applied for NEW connections and updating the above logical flows to check 
> for this mark:
> 
>  - table=9 (ls_in_lb), priority=65535, match=(ct.est && !ct.rel && !ct.new && 
> !ct.inv && ct.mark == 1/1),
>action=(reg0[2] = 1; next;)
> 
>  - table=3 (ls_out_lb), priority=65535, match=(ct.est && !ct.rel && !ct.new 
> && !ct.inv && ct.mark == 1/1),
>action=(reg0[2] = 1; next;)
> 
> Signed-off-by: Numan Siddique 

Hi Numan,

The patch looks good to me. One minor comment inline. Otherwise:

Acked-by: Dumitru Ceara 

Thanks,
Dumitru

> ---
> v1 -> v2
> --
>  * Rebased to latest master and resolved merge conflicts.
> 
>  lib/actions.c|   3 +-
>  lib/logical-fields.c |   6 ++-
>  northd/ovn-northd.c  |   6 ++-
>  tests/ovn.at |  17 +++---
>  tests/system-ovn.at  | 122 +--
>  5 files changed, 80 insertions(+), 74 deletions(-)
> 
> diff --git a/lib/actions.c b/lib/actions.c
> index 05fa44b601..1f2520c808 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1086,7 +1086,8 @@ encode_CT_LB(const struct ovnact_ct_lb *cl,
>  if (dst->port) {
>  ds_put_format(, ":%"PRIu16, dst->port);
>  }
> -ds_put_format(, "),commit,table=%d,zone=NXM_NX_REG%d[0..15])",
> +ds_put_format(, "),commit,table=%d,zone=NXM_NX_REG%d[0..15],"
> +  "exec(set_field:2/3->ct_label))",

This could be "exec(set_field:2/3->ct_label)".

>recirc_table, zone_reg);
>  }
>  
> diff --git a/lib/logical-fields.c b/lib/logical-fields.c
> index 15342ddedf..bf61df7719 100644
> --- a/lib/logical-fields.c
> +++ b/lib/logical-fields.c
> @@ -126,10 +126,12 @@ ovn_init_symtab(struct shash *symtab)
>  expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false,
>   WR_CT_COMMIT);
>  
> -expr_symtab_add_field_scoped(symtab, "ct_label", MFF_CT_LABEL, NULL, 
> false,
> - WR_CT_COMMIT);
> +expr_symtab_add_field_scoped(symtab, "ct_label", MFF_CT_LABEL, NULL,
> + false, WR_CT_COMMIT);
>  expr_symtab_add_subfield_scoped(symtab, "ct_label.blocked", NULL,
>  "ct_label[0]", WR_CT_COMMIT);
> +expr_symtab_add_subfield_scoped(symtab, "ct_label.natted", NULL,
> +"ct_label[1]", WR_CT_COMMIT);
>  expr_symtab_add_subfield_scoped(symtab, "ct_label.ecmp_reply_eth", NULL,
>  "ct_label[32..79]", WR_CT_COMMIT);
>  expr_symtab_add_subfield_scoped(symtab, "ct_label.ecmp_reply_port", NULL,
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index c7b1239adf..293abbff3d 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5750,10 +5750,12 @@ build_lb(struct ovn_datapath *od, struct hmap *lflows)
>   *
>   * Send established traffic through conntrack for just NAT. */
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_LB, UINT16_MAX - 1,
> -  "ct.est && !ct.rel && !ct.new && !ct.inv",
> +  "ct.est && !ct.rel && !ct.new && !ct.inv && "
> +  "ct_label.natted == 1",
>REGBIT_CONNTRACK_NAT" = 1; next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_LB, UINT16_MAX - 1,
> -  "ct.est && !ct.rel && !ct.new && !ct.inv",
> +  "ct.est && !ct.rel && !ct.new && !ct.inv && "
> +  "ct_label.natted == 1",
>REGBIT_CONNTRACK_NAT" = 1; next;");
>  }
>  }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index b0179a8db1..7adc835966 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -197,6 +197,7 @@ ct_label = NXM_NX_CT_LABEL
>  ct_label.blocked = ct_label[0]
>  ct_label.ecmp_reply_eth = ct_label[32..79]
>  ct_label.ecmp_reply_port = ct_label[80..95]
> +ct_label.natted = ct_label[1]
>  ct_mark = NXM_NX_CT_MARK
>  

Re: [ovs-dev] [PATCH v2] Allow bare ct_commits when no nested actions are required.

2020-08-06 Thread Mark Michelson

On 8/6/20 1:47 PM, Numan Siddique wrote:

On Thu, Aug 6, 2020 at 11:03 PM Mark Michelson  wrote:


On 8/6/20 12:16 PM, Numan Siddique wrote:



On Thu, Aug 6, 2020 at 8:22 PM Mark Michelson mailto:mmich...@redhat.com>> wrote:

 In the fixes commit below, ct_commit was changed to use nested actions.
 This requires that curly braces be present for all ct_commits. When
 adjusting ovn-northd, some ct_commits were not updated to have them.
 This commit changes the behavior of the ct_commit action not to require
 curly braces if there are no nested actions required.

 Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
 Signed-off-by: Mark Michelson mailto:mmich...@redhat.com>>


Thanks for the fix.
Acked-by: Numan Siddique mailto:num...@ovn.org>>

The system test case - 29: ovn --Test packet drops due to incorrect
flows in physical table 33 FAILED (system-ovn.at:4538
) is failing
on my setup. But this patch is not the issue. I see failure with the
master too. Just FYI.


OK. Just to throw out another data point, that test passes on my setup.



It's a timing issue and adding the below before the check fixes the
issue for me and tests passes all the
time when I run in a loop.

OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1-f) = xup])
OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2-f) = xup])
OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p3-f) = xup])

I'll submit the patch to fix it later.

Thanks
Numan


OK, cool.

In the meantime, I pushed this patch to master.








Thanks
Numan

 ---
   lib/actions.c | 20 
   tests/ovn.at   |  5 -
   2 files changed, 20 insertions(+), 5 deletions(-)

 diff --git a/lib/actions.c b/lib/actions.c
 index 05fa44b60..4afc23d66 100644
 --- a/lib/actions.c
 +++ b/lib/actions.c
 @@ -630,15 +630,27 @@ ovnact_ct_next_free(struct ovnact_ct_next *a
 OVS_UNUSED)
   static void
   parse_CT_COMMIT(struct action_context *ctx)
   {
 -
 -parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
 -WR_CT_COMMIT);
 +if (ctx->lexer->token.type == LEX_T_LCURLY) {
 +parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
 +WR_CT_COMMIT);
 +} else {
 +/* Add an empty nested action to allow for "ct_commit;"
 syntax */
 +add_prerequisite(ctx, "ip");
 +struct ovnact_nest *on = ovnact_put(ctx->ovnacts,
 OVNACT_CT_COMMIT,
 +OVNACT_ALIGN(sizeof *on));
 +on->nested_len = 0;
 +on->nested = NULL;
 +}
   }

   static void
   format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
   {
 -format_nested_action(on, "ct_commit", s);
 +if (on->nested_len) {
 +format_nested_action(on, "ct_commit", s);
 +} else {
 +ds_put_cstr(s, "ct_commit;");
 +}
   }

   static void
 diff --git a/tests/ovn.at  b/tests/ovn.at 
 index b0179a8db..7236eeb8e 100644
 --- a/tests/ovn.at 
 +++ b/tests/ovn.at 
 @@ -1050,8 +1050,11 @@ ct_next;
   has prereqs ip

   # ct_commit
 +ct_commit;
 +encodes as ct(commit,zone=NXM_NX_REG13[0..15])
 +has prereqs ip
   ct_commit { };
 -formats as ct_commit { drop; };
 +formats as ct_commit;
   encodes as ct(commit,zone=NXM_NX_REG13[0..15])
   has prereqs ip
   ct_commit { ct_mark=1; };
 --
 2.25.4

 ___
 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




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


Re: [ovs-dev] [PATCH v2] Allow bare ct_commits when no nested actions are required.

2020-08-06 Thread Numan Siddique
On Thu, Aug 6, 2020 at 11:03 PM Mark Michelson  wrote:
>
> On 8/6/20 12:16 PM, Numan Siddique wrote:
> >
> >
> > On Thu, Aug 6, 2020 at 8:22 PM Mark Michelson  > > wrote:
> >
> > In the fixes commit below, ct_commit was changed to use nested actions.
> > This requires that curly braces be present for all ct_commits. When
> > adjusting ovn-northd, some ct_commits were not updated to have them.
> > This commit changes the behavior of the ct_commit action not to require
> > curly braces if there are no nested actions required.
> >
> > Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
> > Signed-off-by: Mark Michelson  > >
> >
> >
> > Thanks for the fix.
> > Acked-by: Numan Siddique mailto:num...@ovn.org>>
> >
> > The system test case - 29: ovn --Test packet drops due to incorrect
> > flows in physical table 33 FAILED (system-ovn.at:4538
> > ) is failing
> > on my setup. But this patch is not the issue. I see failure with the
> > master too. Just FYI.
>
> OK. Just to throw out another data point, that test passes on my setup.


It's a timing issue and adding the below before the check fixes the
issue for me and tests passes all the
time when I run in a loop.

OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p1-f) = xup])
OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p2-f) = xup])
OVS_WAIT_UNTIL([test x$(ovn-nbctl lsp-get-up sw0-p3-f) = xup])

I'll submit the patch to fix it later.

Thanks
Numan

>
>
> >
> > Thanks
> > Numan
> >
> > ---
> >   lib/actions.c | 20 
> >   tests/ovn.at   |  5 -
> >   2 files changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/actions.c b/lib/actions.c
> > index 05fa44b60..4afc23d66 100644
> > --- a/lib/actions.c
> > +++ b/lib/actions.c
> > @@ -630,15 +630,27 @@ ovnact_ct_next_free(struct ovnact_ct_next *a
> > OVS_UNUSED)
> >   static void
> >   parse_CT_COMMIT(struct action_context *ctx)
> >   {
> > -
> > -parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
> > -WR_CT_COMMIT);
> > +if (ctx->lexer->token.type == LEX_T_LCURLY) {
> > +parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
> > +WR_CT_COMMIT);
> > +} else {
> > +/* Add an empty nested action to allow for "ct_commit;"
> > syntax */
> > +add_prerequisite(ctx, "ip");
> > +struct ovnact_nest *on = ovnact_put(ctx->ovnacts,
> > OVNACT_CT_COMMIT,
> > +OVNACT_ALIGN(sizeof *on));
> > +on->nested_len = 0;
> > +on->nested = NULL;
> > +}
> >   }
> >
> >   static void
> >   format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
> >   {
> > -format_nested_action(on, "ct_commit", s);
> > +if (on->nested_len) {
> > +format_nested_action(on, "ct_commit", s);
> > +} else {
> > +ds_put_cstr(s, "ct_commit;");
> > +}
> >   }
> >
> >   static void
> > diff --git a/tests/ovn.at  b/tests/ovn.at 
> > index b0179a8db..7236eeb8e 100644
> > --- a/tests/ovn.at 
> > +++ b/tests/ovn.at 
> > @@ -1050,8 +1050,11 @@ ct_next;
> >   has prereqs ip
> >
> >   # ct_commit
> > +ct_commit;
> > +encodes as ct(commit,zone=NXM_NX_REG13[0..15])
> > +has prereqs ip
> >   ct_commit { };
> > -formats as ct_commit { drop; };
> > +formats as ct_commit;
> >   encodes as ct(commit,zone=NXM_NX_REG13[0..15])
> >   has prereqs ip
> >   ct_commit { ct_mark=1; };
> > --
> > 2.25.4
> >
> > ___
> > 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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] OVN Meeting Logs 06 August, 2020

2020-08-06 Thread mmichels
Here is the IRC log for the OVN meeting for 06 August, 2020

http://eavesdrop.openstack.org/meetings//ovn_community_development_discussion/2020/ovn_community_development_discussion.2020-08-06-17.15.log.html

If you are interested in attending this meeting, it happens every
Thursday in the #openvswitch channel on the Freenode IRC server. The
next one is at 13:15 EDT (18:15 BST, 10:15 PDT) on 13 August, 2020.

Mark Michelson

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


Re: [ovs-dev] [PATCH v2] Allow bare ct_commits when no nested actions are required.

2020-08-06 Thread Mark Michelson

On 8/6/20 12:16 PM, Numan Siddique wrote:



On Thu, Aug 6, 2020 at 8:22 PM Mark Michelson > wrote:


In the fixes commit below, ct_commit was changed to use nested actions.
This requires that curly braces be present for all ct_commits. When
adjusting ovn-northd, some ct_commits were not updated to have them.
This commit changes the behavior of the ct_commit action not to require
curly braces if there are no nested actions required.

Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
Signed-off-by: Mark Michelson mailto:mmich...@redhat.com>>


Thanks for the fix.
Acked-by: Numan Siddique mailto:num...@ovn.org>>

The system test case - 29: ovn --Test packet drops due to incorrect 
flows in physical table 33 FAILED (system-ovn.at:4538 
) is failing
on my setup. But this patch is not the issue. I see failure with the 
master too. Just FYI.


OK. Just to throw out another data point, that test passes on my setup.



Thanks
Numan

---
  lib/actions.c | 20 
  tests/ovn.at   |  5 -
  2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/actions.c b/lib/actions.c
index 05fa44b60..4afc23d66 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -630,15 +630,27 @@ ovnact_ct_next_free(struct ovnact_ct_next *a
OVS_UNUSED)
  static void
  parse_CT_COMMIT(struct action_context *ctx)
  {
-
-    parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
-                        WR_CT_COMMIT);
+    if (ctx->lexer->token.type == LEX_T_LCURLY) {
+        parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
+                            WR_CT_COMMIT);
+    } else {
+        /* Add an empty nested action to allow for "ct_commit;"
syntax */
+        add_prerequisite(ctx, "ip");
+        struct ovnact_nest *on = ovnact_put(ctx->ovnacts,
OVNACT_CT_COMMIT,
+                                            OVNACT_ALIGN(sizeof *on));
+        on->nested_len = 0;
+        on->nested = NULL;
+    }
  }

  static void
  format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
  {
-    format_nested_action(on, "ct_commit", s);
+    if (on->nested_len) {
+        format_nested_action(on, "ct_commit", s);
+    } else {
+        ds_put_cstr(s, "ct_commit;");
+    }
  }

  static void
diff --git a/tests/ovn.at  b/tests/ovn.at 
index b0179a8db..7236eeb8e 100644
--- a/tests/ovn.at 
+++ b/tests/ovn.at 
@@ -1050,8 +1050,11 @@ ct_next;
      has prereqs ip

  # ct_commit
+ct_commit;
+    encodes as ct(commit,zone=NXM_NX_REG13[0..15])
+    has prereqs ip
  ct_commit { };
-    formats as ct_commit { drop; };
+    formats as ct_commit;
      encodes as ct(commit,zone=NXM_NX_REG13[0..15])
      has prereqs ip
  ct_commit { ct_mark=1; };
-- 
2.25.4


___
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] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

2020-08-06 Thread Stokes, Ian
> On 8/6/2020 6:17 PM, Emma Finn wrote:
> > The following 2 commits introduced changes which caused a regression
> > for XL710 devices and functionality ceases for partial offload as a result.
> > 864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for type
> > only.")
> > a79eae87abe4 ("netdev-offload-dpdk: Remove pre-validate of patterns
> > function.")
> OVS is vendor agnostic. That kind of workaround belongs in intel PMD in dpdk,
> not in OVS.

Hi Eli, 

Yes OVS looks to be vendor agnostic, but this code I believe was already in 
place and working for this usecase. As such it's removal introduced a 
regression from an OVS point of view between the releases.

We have had examples in the past where workarounds are permissible if there is 
a clear path to fixing this in the future on the DPDK side (which is what I 
suggest here) (for example scatter gather support for MTUs in the past raised 
similar issue where we hand to handle specific NIC until the next DPDK LTS 
release).

So my suggestion is to re-instate the original workaround and remove its when 
fixed in the next DPDK LTS which supports the change for i40e at the PMD layer 
or if it's backported to the next 19.11 stable release which would be validated 
for use with OVS.

I've included the i40e DPDK maintainers here for their thoughts also.

@Beilei/Jia Is this something we can look at to introduce in the i40e PMD?

Regards
Ian

> >
> > Fixed by partial reversion of these changes.
> >
> > Signed-off-by: Emma Finn
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] CONTACT OUR INTERNATIONAL DIPLOMATIC AGENT, MR. JOHN BENDER TO RECEIVE YOUR ATM CARD WORTH $12.8MILLION US DOLLARS, This delivery was approved today, 06/08/2020

2020-08-06 Thread David Mark
Attn,Dear.
GOODNEWS FOR YOU.
CONTACT OUR INTERNATIONAL DIPLOMATIC AGENT, MR. JOHN BENDER TO RECEIVE
YOUR ATM CARD WORTH $12.8MILLION US DOLLARS, This delivery was
approved today, 06/08/2020
Contact Person, AGENT, MR. JOHN BENDER
Email: john.b...@yahoo.com
Phone number (408) 650-6103, call or Text Him once you receive this message.
Remember to send to MR. JOHN BENDER your house address i listed below
Your Full Name
House Address
Your Phone Numbers___
Let me know once you receive your delivery ok.
Delivery fee to your address is $75.00, ask where to send the fee to Him today.
Thanks and may God bless you.
David Mark
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH 1/1] dpdk: Deprecate vhost-user dequeue zero-copy.

2020-08-06 Thread Ian Stokes
Dequeue zero-copy is no longer supported for vhost-user client mode
in DPDK due to commit [1].

In addition to this, zero-copy mode has been proposed to be marked
deprecated in [2] with removal in the next DPDK LTS release.

This commit deprecates support for vhost-user dequeue zero-copy in OVS
with its removal expected in the next OVS release.

[1] 715070ea10e6 ("vhost: prevent zero-copy with incompatible client
mode")
[2] http://mails.dpdk.org/archives/dev/2020-August/177236.html

Signed-off-by: Ian Stokes 
---
 Documentation/topics/dpdk/vhost-user.rst | 5 +
 NEWS | 2 ++
 lib/netdev-dpdk.c| 2 ++
 3 files changed, 9 insertions(+)

diff --git a/Documentation/topics/dpdk/vhost-user.rst 
b/Documentation/topics/dpdk/vhost-user.rst
index b1eb5d9da..4af738d11 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -556,6 +556,11 @@ shown with::
 vhost-user Dequeue Zero Copy (experimental)
 ---
 
+.. warning::
+
+   vhost-user Dequeue Zero Copy is deprecated in OVS and will be removed in
+   the next release.
+
 Normally when dequeuing a packet from a vHost User device, a memcpy operation
 must be used to copy that packet from guest address space to host address
 space. This memcpy can be removed by enabling dequeue zero-copy like so::
diff --git a/NEWS b/NEWS
index dceda95a3..5d6489f26 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,8 @@ v2.14.0 - xx xxx 
CVE-2020-10726, this DPDK version is strongly recommended to be used.
  * New 'ovs-appctl dpdk/log-list' and 'ovs-appctl dpdk/log-set' commands
to list and change log levels in DPDK components.
+ * Vhost-user Dequeue zero-copy support is deprecated and will be removed
+   in the next release.
- Linux datapath:
  * Support for kernel versions up to 5.5.x.
- AF_XDP:
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 44ebf96da..27bf3b6ed 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -5079,6 +5079,8 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
   dev->up.name, dev->vhost_id);
 if (zc_enabled) {
 VLOG_INFO("Zero copy enabled for vHost port %s", dev->up.name);
+VLOG_WARN("Zero copy support is deprecated and will be "
+  "removed in the next OVS release.");
 }
 }
 
-- 
2.13.6

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


Re: [ovs-dev] packet drop

2020-08-06 Thread Numan Siddique
On Thu, Aug 6, 2020 at 4:05 AM Tony Liu  wrote:

>
> The drop is caused by flow change.
>
> When packet is dropped.
> 
> recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,geneve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:00:00/01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0xf8),
> packets:14, bytes:1372, used:0.846s, actions:drop
>
> recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:1e:85),eth_type(0x0800),ipv4(src=
> 192.168.236.152/255.255.255.252,dst=10.6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no),icmp(type=0),
> packets:6, bytes:588, used:8.983s, actions:drop
> 
>
> When packet goes through.
> 
> recirc_id(0),tunnel(tun_id=0x19aca,src=10.6.30.92,dst=10.6.30.22,geneve({class=0x102,type=0x80,len=4,0x20003/0x7fff}),flags(-df+csum+key)),in_port(3),eth(src=fa:16:3e:df:1e:85,dst=00:00:00:00:00:00/01:00:00:00:00:00),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0xf8),
> packets:3, bytes:294, used:0.104s, actions:12
>
> recirc_id(0),in_port(12),eth(src=fa:16:3e:7d:bb:85,dst=fa:16:3e:df:1e:85),eth_type(0x0800),ipv4(src=
> 192.168.236.152/255.255.255.252,dst=10.6.40.9,proto=1,tos=0/0x3,ttl=64,frag=no),icmp(type=0),
> packets:3, bytes:294, used:0.103s,
> actions:ct_clear,set(tunnel(tun_id=0x1a8ee,dst=10.6.30.92,ttl=64,tp_dst=6081,geneve({class=0x102,type=0x80,len=4,0x1000b}),flags(df|csum|key))),set(eth(src=fa:16:3e:75:b7:e5,dst=52:54:00:0c:ef:b9)),set(ipv4(ttl=63)),3
> 
>
> Is that flow programmed by ovn-controller via ovs-vswitchd?
>
>
What version of OVN and OVS are you using ?

Can you share your OVN NB DB ?

If I understand correctly the packet is received from the patch port to
br-int on the gateway node and then tunnelled to the compute node right ?
And the packet is dropped on the compute node ?

If you could share your NB DB if it's fine with you and tell the
destination logical port, I can try it out locally.

Thanks
Numan




>
> Thanks!
>
> Tony
>
> > -Original Message-
> > From: discuss  On Behalf Of Tony
> > Liu
> > Sent: Wednesday, August 5, 2020 2:48 PM
> > To: ovs-disc...@openvswitch.org; ovs-dev@openvswitch.org
> > Subject: [ovs-discuss] packet drop
> >
> > Hi,
> >
> > I am running ping from external to VM via OVN gateway.
> > On the compute node, ICMP request packet is consistently coming into
> > interface "ovn-gatewa-1". But there is about 10 out of 25 packet loss on
> > tap interface. It's like the switch pauses 10s after every 15s.
> >
> > Has anyone experiences such issue?
> > Any advice how to look into it?
> >
> > 
> > 21fed09f-909e-4efc-b117-f5d5fcb636c9
> > Bridge br-int
> > fail_mode: secure
> > datapath_type: system
> > Port "ovn-gatewa-0"
> > Interface "ovn-gatewa-0"
> > type: geneve
> > options: {csum="true", key=flow, remote_ip="10.6.30.91"}
> > bfd_status: {diagnostic="No Diagnostic", flap_count="1",
> > forwarding="true", remote_diagnostic="No Diagnostic", remote_state=up,
> > state=up}
> > Port "tap2588bb4e-35"
> > Interface "tap2588bb4e-35"
> > Port "ovn-gatewa-1"
> > Interface "ovn-gatewa-1"
> > type: geneve
> > options: {csum="true", key=flow, remote_ip="10.6.30.92"}
> > bfd_status: {diagnostic="No Diagnostic", flap_count="1",
> > forwarding="true", remote_diagnostic="No Diagnostic", remote_state=up,
> > state=up}
> > Port "tap37f6b2d7-cc"
> > Interface "tap37f6b2d7-cc"
> > Port "tap2c4b3b0f-8b"
> > Interface "tap2c4b3b0f-8b"
> > Port "tap23245491-a4"
> > Interface "tap23245491-a4"
> > Port "tap51660269-2c"
> > Interface "tap51660269-2c"
> > Port "tap276cd1ef-e1"
> > Interface "tap276cd1ef-e1"
> > Port "tap138526d3-b3"
> > Interface "tap138526d3-b3"
> > Port "tapd1ae48a1-2d"
> > Interface "tapd1ae48a1-2d"
> > Port br-int
> > Interface br-int
> > type: internal
> > Port "tapdd08f476-94"
> > Interface "tapdd08f476-94"
> > 
> >
> >
> > Thanks!
> >
> > Tony
> >
> > ___
> > 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
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] meta-flow: fix a typo in "MPLS Bottom of Stack Field" paragraph

2020-08-06 Thread Timothy Redaelli
In the ovs-fields.7 manual page, the "MPLS Bottom of Stack Field" paragraph
says:
 * When mpls_bos is 1, there is another MPLS label following this one,
   so the Ethertype passed to pop_mpls should be an MPLS Ethertype. [...]

 * When mpls_bos is 0, this MPLS label is the last one, so the Ethertype
   passed to pop_mpls should be a non-MPLS Ethertype such as IPv4. [...]

The values 0 and 1 have been swapped: when BOS is 1,
then no more label stack entries follows.

Fixes: 96fee5e0a2a0 ("ovs-fields: New manpage to document Open vSwitch and 
OpenFlow fields.")
Cc: b...@ovn.org
Reported-at: https://bugzilla.redhat.com/1842032
Reported-by: Guillaume Nault 
Signed-off-by: Timothy Redaelli 
---
 lib/meta-flow.xml | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
index 154675874..e72ba52ec 100644
--- a/lib/meta-flow.xml
+++ b/lib/meta-flow.xml
@@ -3920,18 +3920,18 @@ r r c c c.
 
   
 
-  When  is 1, there is another MPLS label
+  When  is 0, there is another MPLS label
   following this one, so the Ethertype passed to pop_mpls
   should be an MPLS Ethertype.  For example: table=0,
-  dl_type=0x8847, mpls_bos=1, actions=pop_mpls:0x8847,
+  dl_type=0x8847, mpls_bos=0, actions=pop_mpls:0x8847,
   goto_table:1
 
 
 
-  When  is 0, this MPLS label is the last one,
+  When  is 1, this MPLS label is the last one,
   so the Ethertype passed to pop_mpls should be a non-MPLS
   Ethertype such as IPv4.  For example: table=1, dl_type=0x8847,
-  mpls_bos=0, actions=pop_mpls:0x0800, goto_table:2
+  mpls_bos=1, actions=pop_mpls:0x0800, goto_table:2
 
   
 
-- 
2.26.2

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


Re: [ovs-dev] [PATCH ovn] Add missing curly braces to bare ct_commits

2020-08-06 Thread Flavio Fernandes


Verified this fix on top of commit acd38429638c01afe1b2a1d15404e4724232ec1d .

Tested-by: Flavio Fernandes 



> On Aug 6, 2020, at 9:38 AM, Mark Michelson  wrote:
> 
> In the fixes commit below, ct_commit was changed to use nested actions.
> This requires that curly braces be present for all ct_commits. When
> adjusting ovn-northd, some ct_commits were not updated to have them.
> This commit fixes the issue.
> 
> Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
> Signed-off-by: Mark Michelson 
> ---
> northd/ovn-northd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 03c62bafa..d2312f020 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5590,7 +5590,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
> if (server_id && server_mac && lease_time) {
> struct ds match = DS_EMPTY_INITIALIZER;
> const char *actions =
> -has_stateful ? "ct_commit; next;" : "next;";
> +has_stateful ? "ct_commit { }; next;" : "next;";
> ds_put_format(, "outport == \"%s\" && eth.src == %s "
>   "&& ip4.src == %s && udp && udp.src == 67 "
>   "&& udp.dst == 68", od->nbs->ports[i]->name,
> @@ -5616,7 +5616,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
> ipv6_string_mapped(server_ip, );
> 
> struct ds match = DS_EMPTY_INITIALIZER;
> -const char *actions = has_stateful ? "ct_commit; next;" :
> +const char *actions = has_stateful ? "ct_commit { }; next;" :
> "next;";
> ds_put_format(, "outport == \"%s\" && eth.src == %s "
>   "&& ip6.src == %s && udp && udp.src == 547 "
> @@ -5634,7 +5634,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
>  * if the CMS has configured DNS records for the datapath.
>  */
> if (ls_has_dns_records(od->nbs)) {
> -const char *actions = has_stateful ? "ct_commit; next;" : "next;";
> +const char *actions = has_stateful ? "ct_commit { }; next;" : 
> "next;";
> ovn_lflow_add(
> lflows, od, S_SWITCH_OUT_ACL, 34000, "udp.src == 53",
> actions);
> --
> 2.25.4
> 
> ___
> 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] Allow bare ct_commits when no nested actions are required.

2020-08-06 Thread Numan Siddique
On Thu, Aug 6, 2020 at 8:22 PM Mark Michelson  wrote:

> In the fixes commit below, ct_commit was changed to use nested actions.
> This requires that curly braces be present for all ct_commits. When
> adjusting ovn-northd, some ct_commits were not updated to have them.
> This commit changes the behavior of the ct_commit action not to require
> curly braces if there are no nested actions required.
>
> Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
> Signed-off-by: Mark Michelson 
>

Thanks for the fix.
Acked-by: Numan Siddique 

The system test case - 29: ovn --Test packet drops due to incorrect flows
in physical table 33 FAILED (system-ovn.at:4538) is failing
on my setup. But this patch is not the issue. I see failure with the master
too. Just FYI.

Thanks
Numan


> ---
>  lib/actions.c | 20 
>  tests/ovn.at  |  5 -
>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/lib/actions.c b/lib/actions.c
> index 05fa44b60..4afc23d66 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -630,15 +630,27 @@ ovnact_ct_next_free(struct ovnact_ct_next *a
> OVS_UNUSED)
>  static void
>  parse_CT_COMMIT(struct action_context *ctx)
>  {
> -
> -parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
> -WR_CT_COMMIT);
> +if (ctx->lexer->token.type == LEX_T_LCURLY) {
> +parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
> +WR_CT_COMMIT);
> +} else {
> +/* Add an empty nested action to allow for "ct_commit;" syntax */
> +add_prerequisite(ctx, "ip");
> +struct ovnact_nest *on = ovnact_put(ctx->ovnacts,
> OVNACT_CT_COMMIT,
> +OVNACT_ALIGN(sizeof *on));
> +on->nested_len = 0;
> +on->nested = NULL;
> +}
>  }
>
>  static void
>  format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
>  {
> -format_nested_action(on, "ct_commit", s);
> +if (on->nested_len) {
> +format_nested_action(on, "ct_commit", s);
> +} else {
> +ds_put_cstr(s, "ct_commit;");
> +}
>  }
>
>  static void
> diff --git a/tests/ovn.at b/tests/ovn.at
> index b0179a8db..7236eeb8e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1050,8 +1050,11 @@ ct_next;
>  has prereqs ip
>
>  # ct_commit
> +ct_commit;
> +encodes as ct(commit,zone=NXM_NX_REG13[0..15])
> +has prereqs ip
>  ct_commit { };
> -formats as ct_commit { drop; };
> +formats as ct_commit;
>  encodes as ct(commit,zone=NXM_NX_REG13[0..15])
>  has prereqs ip
>  ct_commit { ct_mark=1; };
> --
> 2.25.4
>
> ___
> 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] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

2020-08-06 Thread Eli Britstein



On 8/6/2020 6:17 PM, Emma Finn wrote:

The following 2 commits introduced changes which caused a regression
for XL710 devices and functionality ceases for partial offload as a result.
864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for type only.")
a79eae87abe4 ("netdev-offload-dpdk: Remove pre-validate of patterns function.")
OVS is vendor agnostic. That kind of workaround belongs in intel PMD in 
dpdk, not in OVS.


Fixed by partial reversion of these changes.

Signed-off-by: Emma Finn

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


Re: [ovs-dev] [PATCH v2] tc: Use skip_hw flag when probing tc features

2020-08-06 Thread Simon Horman
On Tue, Aug 04, 2020 at 05:33:33PM +0800, Tonghao Zhang wrote:
> On Tue, Aug 4, 2020 at 2:37 PM Roi Dayan  wrote:
> >
> > There is no need to pass tc rules to hw when just probing
> > for tc features. this will avoid redundant errors from hw drivers
> > that may happen.
> >
> > Signed-off-by: Roi Dayan 
> > Acked-By: Vlad Buslov 
>
> Reviewed-by: Tonghao Zhang 

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


[ovs-dev] [PATCH branch-2.13] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

2020-08-06 Thread Emma Finn
The following commit introduced changes which caused a regression
for XL710 devices and functionality ceases for partial offload as a result.
864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for type only.")

Fixed by reversion of these changes.

Signed-off-by: Emma Finn 
---
 lib/netdev-offload-dpdk.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index f97b8f8..c7f25b2 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -565,8 +565,7 @@ parse_flow_match(struct flow_patterns *patterns,
 uint8_t proto = 0;
 
 /* Eth */
-if (match->wc.masks.dl_type ||
-!eth_addr_is_zero(match->wc.masks.dl_src) ||
+if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
 !eth_addr_is_zero(match->wc.masks.dl_dst)) {
 struct rte_flow_item_eth *spec, *mask;
 
@@ -582,6 +581,15 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->type = match->wc.masks.dl_type;
 
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
+} else {
+/*
+ * If user specifies a flow (like UDP flow) without L2 patterns,
+ * OVS will at least set the dl_type. Normally, it's enough to
+ * create an eth pattern just with it. Unluckily, some Intel's
+ * NIC (such as XL710) doesn't support that. Below is a workaround,
+ * which simply matches any L2 pkts.
+ */
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN, NULL, NULL);
 }
 
 /* VLAN */
-- 
2.7.4

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


[ovs-dev] [PATCH] netdev-offload-dpdk: Fix for broken ethernet matching HWOL for XL710 NIC

2020-08-06 Thread Emma Finn
The following 2 commits introduced changes which caused a regression
for XL710 devices and functionality ceases for partial offload as a result.
864852a0624a ("netdev-offload-dpdk: Fix Ethernet matching for type only.")
a79eae87abe4 ("netdev-offload-dpdk: Remove pre-validate of patterns function.")

Fixed by partial reversion of these changes.

Signed-off-by: Emma Finn 
---
 lib/netdev-offload-dpdk.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index de6101e..b300884 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -691,8 +691,7 @@ parse_flow_match(struct flow_patterns *patterns,
 consumed_masks->packet_type = 0;
 
 /* Eth */
-if (match->wc.masks.dl_type ||
-!eth_addr_is_zero(match->wc.masks.dl_src) ||
+if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
 !eth_addr_is_zero(match->wc.masks.dl_dst)) {
 struct rte_flow_item_eth *spec, *mask;
 
@@ -712,6 +711,16 @@ parse_flow_match(struct flow_patterns *patterns,
 consumed_masks->dl_type = 0;
 
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
+} else {
+/*
+* If user specifies a flow (like UDP flow) without L2 patterns,
+* OVS will at least set the dl_type. Normally, it's enough to
+* create an eth pattern just with it. Unluckily, some Intel's
+* NIC (such as XL710) doesn't support that. Below is a workaround,
+* which simply matches any L2 pkts.
+*/
+consumed_masks->dl_type = 0;
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, NULL, NULL);
 }
 
 /* VLAN */
-- 
2.7.4

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


Re: [ovs-dev] [PATCH v2] Allow bare ct_commits when no nested actions are required.

2020-08-06 Thread 0-day Robot
Bleep bloop.  Greetings Mark Michelson, 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:
error: sha1 information is lacking or useless (lib/actions.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 Allow bare ct_commits when no nested actions are required.
When you have resolved this problem, run "git am --continue".
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...@redhat.com

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


[ovs-dev] [PATCH v2] Allow bare ct_commits when no nested actions are required.

2020-08-06 Thread Mark Michelson
In the fixes commit below, ct_commit was changed to use nested actions.
This requires that curly braces be present for all ct_commits. When
adjusting ovn-northd, some ct_commits were not updated to have them.
This commit changes the behavior of the ct_commit action not to require
curly braces if there are no nested actions required.

Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
Signed-off-by: Mark Michelson 
---
 lib/actions.c | 20 
 tests/ovn.at  |  5 -
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/lib/actions.c b/lib/actions.c
index 05fa44b60..4afc23d66 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -630,15 +630,27 @@ ovnact_ct_next_free(struct ovnact_ct_next *a OVS_UNUSED)
 static void
 parse_CT_COMMIT(struct action_context *ctx)
 {
-
-parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
-WR_CT_COMMIT);
+if (ctx->lexer->token.type == LEX_T_LCURLY) {
+parse_nested_action(ctx, OVNACT_CT_COMMIT, "ip",
+WR_CT_COMMIT);
+} else {
+/* Add an empty nested action to allow for "ct_commit;" syntax */
+add_prerequisite(ctx, "ip");
+struct ovnact_nest *on = ovnact_put(ctx->ovnacts, OVNACT_CT_COMMIT,
+OVNACT_ALIGN(sizeof *on));
+on->nested_len = 0;
+on->nested = NULL;
+}
 }
 
 static void
 format_CT_COMMIT(const struct ovnact_nest *on, struct ds *s)
 {
-format_nested_action(on, "ct_commit", s);
+if (on->nested_len) {
+format_nested_action(on, "ct_commit", s);
+} else {
+ds_put_cstr(s, "ct_commit;");
+}
 }
 
 static void
diff --git a/tests/ovn.at b/tests/ovn.at
index b0179a8db..7236eeb8e 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1050,8 +1050,11 @@ ct_next;
 has prereqs ip
 
 # ct_commit
+ct_commit;
+encodes as ct(commit,zone=NXM_NX_REG13[0..15])
+has prereqs ip
 ct_commit { };
-formats as ct_commit { drop; };
+formats as ct_commit;
 encodes as ct(commit,zone=NXM_NX_REG13[0..15])
 has prereqs ip
 ct_commit { ct_mark=1; };
-- 
2.25.4

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


[ovs-dev] [PATCH ovn] Add missing curly braces to bare ct_commits

2020-08-06 Thread Mark Michelson
In the fixes commit below, ct_commit was changed to use nested actions.
This requires that curly braces be present for all ct_commits. When
adjusting ovn-northd, some ct_commits were not updated to have them.
This commit fixes the issue.

Fixes: 6cfb44a76c61("Used nested actions in ct_commit")
Signed-off-by: Mark Michelson 
---
 northd/ovn-northd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 03c62bafa..d2312f020 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5590,7 +5590,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
 if (server_id && server_mac && lease_time) {
 struct ds match = DS_EMPTY_INITIALIZER;
 const char *actions =
-has_stateful ? "ct_commit; next;" : "next;";
+has_stateful ? "ct_commit { }; next;" : "next;";
 ds_put_format(, "outport == \"%s\" && eth.src == %s "
   "&& ip4.src == %s && udp && udp.src == 67 "
   "&& udp.dst == 68", od->nbs->ports[i]->name,
@@ -5616,7 +5616,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
 ipv6_string_mapped(server_ip, );
 
 struct ds match = DS_EMPTY_INITIALIZER;
-const char *actions = has_stateful ? "ct_commit; next;" :
+const char *actions = has_stateful ? "ct_commit { }; next;" :
 "next;";
 ds_put_format(, "outport == \"%s\" && eth.src == %s "
   "&& ip6.src == %s && udp && udp.src == 547 "
@@ -5634,7 +5634,7 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
  * if the CMS has configured DNS records for the datapath.
  */
 if (ls_has_dns_records(od->nbs)) {
-const char *actions = has_stateful ? "ct_commit; next;" : "next;";
+const char *actions = has_stateful ? "ct_commit { }; next;" : "next;";
 ovn_lflow_add(
 lflows, od, S_SWITCH_OUT_ACL, 34000, "udp.src == 53",
 actions);
-- 
2.25.4

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


[ovs-dev] [PATCH ovn 2/2] pinctrl: Avoid flushing of non-local IGMP_Groups.

2020-08-06 Thread Dumitru Ceara
With ovn-monitor-all enabled, all ovn-controllers get updates about all
SB records. This means that an ovn-controller might receive updates about
IGMP_Groups inserted by other chassis on datapaths that are not local to
the local chassis. These entries are valid and are not owned by the local
chassis and should not be flushed.

Fixes: 70ff8243040f ("OVN: Add IGMP SB definitions and ovn-controller support")
Signed-off-by: Dumitru Ceara 
---
 controller/pinctrl.c |   10 ++
 tests/ovn.at |   43 ++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index e37f210..f72ab70 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -4561,7 +4561,17 @@ ip_mcast_sync(struct ovsdb_idl_txn *ovnsb_idl_txn,
 continue;
 }
 
+/* Skip non-local records. */
+if (sbrec_igmp->chassis != chassis) {
+continue;
+}
+
+/* Skip non-local datapaths. */
 int64_t dp_key = sbrec_igmp->datapath->tunnel_key;
+if (!get_local_datapath(local_datapaths, dp_key)) {
+continue;
+}
+
 struct ip_mcast_snoop *ip_ms = ip_mcast_snoop_find(dp_key);
 
 /* If the datapath doesn't exist anymore or IGMP snooping was disabled
diff --git a/tests/ovn.at b/tests/ovn.at
index 2235a19..d641075 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -16997,7 +16997,48 @@ OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [expected_empty])
 OVN_CHECK_PACKETS([hv2/vif3-tx.pcap], [expected_empty])
 OVN_CHECK_PACKETS([hv2/vif4-tx.pcap], [expected_empty])
 
-OVN_CLEANUP([hv1], [hv2])
+# With ovn-monitor-all=true, make sure ovn-controllers don't delete each
+# other's IGMP_Group records.
+
+# Add a new chassis with no local datapaths.
+net_add n3
+sim_add hv3
+as hv3
+ovs-vsctl add-br br-phys
+ovn_attach n3 br-phys 192.168.0.3
+
+# Enable ovn-monitor-all on all chassis.
+as hv1 ovs-vsctl set open . external_ids:ovn-monitor-all=true
+as hv2 ovs-vsctl set open . external_ids:ovn-monitor-all=true
+as hv3 ovs-vsctl set open . external_ids:ovn-monitor-all=true
+
+# Wait until ovn-monitor-all is processed by ovn-controller.
+OVS_WAIT_UNTIL([
+test $(ovn-sbctl get chassis hv1 other_config:ovn-monitor-all) = '"true"'
+])
+OVS_WAIT_UNTIL([
+test $(ovn-sbctl get chassis hv2 other_config:ovn-monitor-all) = '"true"'
+])
+OVS_WAIT_UNTIL([
+test $(ovn-sbctl get chassis hv3 other_config:ovn-monitor-all) = '"true"'
+])
+
+# Inject a fake IGMP_Group entry.
+dp=$(ovn-sbctl --bare --columns _uuid list Datapath_Binding sw3)
+ch=$(ovn-sbctl --bare --columns _uuid list Chassis hv3)
+ovn-sbctl create IGMP_Group address="239.0.1.42" datapath=$dp chassis=$ch
+
+ovn-nbctl --wait=hv sync
+OVS_WAIT_UNTIL([
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.68" -c`
+test "${total_entries}" = "1"
+])
+OVS_WAIT_UNTIL([
+total_entries=`ovn-sbctl find IGMP_Group | grep "239.0.1.42" -c`
+test "${total_entries}" = "1"
+])
+
+OVN_CLEANUP([hv1], [hv2], [hv3])
 AT_CLEANUP
 
 AT_SETUP([ovn -- MLD snoop/querier/relay])

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


[ovs-dev] [PATCH ovn 0/2] Fix IGMP when ovn-monitor-all=true.

2020-08-06 Thread Dumitru Ceara
With ovn-monitor-all=true all chassis get updates about all SB DB records
regardless if they're of interest to the local chassis or not.

There's a bug in the IGMP code in ovn-controller which causes IGMP_Group
records not owned by the local chassis to be flushed. The second patch in
this series fixes the issue.

The first patch propagates the value of ovn-monitor-all to the
Chassis:other_config field in order to make unit tests that use
ovn-monitor-all more reliable.

CC: Han Zhou 
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (2):
  chassis: Propagate ovn-monitor-all external-id to Chassis:other_config.
  pinctrl: Avoid flushing of non-local IGMP_Groups.


 controller/chassis.c |   23 +--
 controller/pinctrl.c |   10 ++
 tests/ovn.at |   51 +-
 3 files changed, 81 insertions(+), 3 deletions(-)


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


[ovs-dev] [PATCH ovn 1/2] chassis: Propagate ovn-monitor-all external-id to Chassis:other_config.

2020-08-06 Thread Dumitru Ceara
At least for avoiding races in the unit tests it's useful to store the value
of the OVS ovn-monitor-all external-id in the Chassis record other_config
field. This allows us to know for sure when an ovn-controller has processed
the update to the OVS Open_vSwitch DB table.

Signed-off-by: Dumitru Ceara 
---
 controller/chassis.c |   23 +--
 tests/ovn.at |8 
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index eec270e..3440d5a 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -84,6 +84,7 @@ struct ovs_chassis_cfg {
 const char *datapath_type;
 const char *encap_csum;
 const char *cms_options;
+const char *monitor_all;
 const char *chassis_macs;
 
 /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
@@ -159,6 +160,12 @@ get_cms_options(const struct smap *ext_ids)
 }
 
 static const char *
+get_monitor_all(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-monitor-all", "false");
+}
+
+static const char *
 get_encap_csum(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-encap-csum", "true");
@@ -277,6 +284,7 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
 ovs_cfg->datapath_type = get_datapath_type(br_int);
 ovs_cfg->encap_csum = get_encap_csum(>external_ids);
 ovs_cfg->cms_options = get_cms_options(>external_ids);
+ovs_cfg->monitor_all = get_monitor_all(>external_ids);
 ovs_cfg->chassis_macs = get_chassis_mac_mappings(>external_ids);
 
 if (!chassis_parse_ovs_encap_type(encap_type, _cfg->encap_type_set)) {
@@ -303,12 +311,13 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
 static void
 chassis_build_other_config(struct smap *config, const char *bridge_mappings,
const char *datapath_type, const char *cms_options,
-   const char *chassis_macs, const char *iface_types,
-   bool is_interconn)
+   const char *monitor_all, const char *chassis_macs,
+   const char *iface_types, bool is_interconn)
 {
 smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
 smap_replace(config, "datapath-type", datapath_type);
 smap_replace(config, "ovn-cms-options", cms_options);
+smap_replace(config, "ovn-monitor-all", monitor_all);
 smap_replace(config, "iface-types", iface_types);
 smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
 smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
@@ -321,6 +330,7 @@ static bool
 chassis_other_config_changed(const char *bridge_mappings,
  const char *datapath_type,
  const char *cms_options,
+ const char *monitor_all,
  const char *chassis_macs,
  const struct ds *iface_types,
  bool is_interconn,
@@ -347,6 +357,13 @@ chassis_other_config_changed(const char *bridge_mappings,
 return true;
 }
 
+const char *chassis_monitor_all =
+get_monitor_all(_rec->other_config);
+
+if (strcmp(monitor_all, chassis_monitor_all)) {
+return true;
+}
+
 const char *chassis_mac_mappings =
 get_chassis_mac_mappings(_rec->other_config);
 if (strcmp(chassis_macs, chassis_mac_mappings)) {
@@ -554,6 +571,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
 if (chassis_other_config_changed(ovs_cfg->bridge_mappings,
  ovs_cfg->datapath_type,
  ovs_cfg->cms_options,
+ ovs_cfg->monitor_all,
  ovs_cfg->chassis_macs,
  _cfg->iface_types,
  ovs_cfg->is_interconn,
@@ -564,6 +582,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
 chassis_build_other_config(_config, ovs_cfg->bridge_mappings,
ovs_cfg->datapath_type,
ovs_cfg->cms_options,
+   ovs_cfg->monitor_all,
ovs_cfg->chassis_macs,
ds_cstr_ro(_cfg->iface_types),
ovs_cfg->is_interconn);
diff --git a/tests/ovn.at b/tests/ovn.at
index b0179a8..2235a19 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -20777,6 +20777,14 @@ as hv2 ovs-vsctl del-port hv2-vif1
 as hv1 ovs-vsctl set open . external_ids:ovn-monitor-all=true
 as hv2 ovs-vsctl set open . external_ids:ovn-monitor-all=true
 
+# Wait until ovn-monitor-all is processed by ovn-controller.
+OVS_WAIT_UNTIL([
+test $(ovn-sbctl get chassis hv1 other_config:ovn-monitor-all) = '"true"'
+])

[ovs-dev] Performance drop with conntrack flows

2020-08-06 Thread K Venkata Kiran via dev
Hi,

We see 40% traffic drop with UDP traffic over VxLAN and 20% traffic drop with 
UDP traffic over MPLSoGRE between OVS 2.8.2 & OVS 2.12.1.

We narrowed the drop in performance in our test is due to below commit and 
backing out the commit fixed the performance drop problem.

The commit of concern is :
https://github.com/openvswitch/ovs/commit/967bb5c5cd9070112138d74a2f4394c50ae48420
commit 967bb5c5cd9070112138d74a2f4394c50ae48420
Author: Darrell Ball mailto:dlu...@gmail.com>>
Date:   Thu May 9 08:15:07 2019 -0700
 conntrack: Add rcu support.
We suspect 'ct->ct_lock' lock taken to do 'conn_update_state' and for 
conn_key_lookup could be causing the issue.
Anyone noticed the issue and any pointers on fix? We could not get any obvious 
commit that could solve the issue. Any guidance in solving this issue helps?
Thanks
Kiran

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


Re: [ovs-dev] [ovs-discuss] [OVN] ovn-northd takes much CPU when no configuration update

2020-08-06 Thread Numan Siddique
On Tue, Aug 4, 2020 at 11:31 PM Han Zhou  wrote:

> On Tue, Aug 4, 2020 at 12:38 AM Numan Siddique  wrote:
>
> >
> >
> > On Tue, Aug 4, 2020 at 9:02 AM Tony Liu  wrote:
> >
> >> The probe awakes recomputing?
> >> There is probe every 5 seconds. Without any connection up/down or
> >> failover,
> >> ovn-northd will recompute everything every 5 seconds, no matter what?
> >> Really?
> >>
> >> Anyways, I will increase the probe interval for now, see if that helps.
> >>
> >
> > I think we should optimise this case. I am planning to look into this.
> >
> > Thanks
> > Numan
> >
>
> Thanks Numan.
> I'd like to discuss more on this before we move forward to change anything.
>
> 1) Regarding the problem itself, the CPU cost triggered by OVSDB IDLE probe
> when there is no configuration change to compute, I don't think it matters
> that much in real production. It simply wastes CPU cycles when there is
> nothing to do, so what harm would it do here? For ovn-northd, since it is
> the centralized component, we would always ensure there is enough CPU
> available for ovn-north when computing is needed, and this reservation will
> be wasted anyway when there is no change to compute. So, I'd avoid making
> any change specifically only to address this issue. I could be wrong,
> though. I'd like to hear what would be the real concern if this is not
> addressed.
>

Agree with you.


>
> 2) ovn-northd incremental processing would avoid this CPU problem
> naturally. So let's discuss how to move forward for incremental processing,
> which is much more important because it also solves the CPU efficiency when
> handling the changes, and the IDLE probe problem is just a byproduct. I
> believe the DDlog branch would have solved this problem. However, it seems
> we are not sure about the current status of DDlog. As you proposed at the
> last OVN meeting, an alternative is to implement partial
> incremental-processing using the I-P engine like ovn-controller. While I
> have no objection to this, we'd better check with Ben and Leonid on the
> plan to avoid overlapping and waste of work. @Ben @Leonid, would you mind
> sharing the status here since you were not at the meeting last week?
>
>
My idea is not to address this issue specifically. But add a very
minimalistic I-P support that
avoids unnecessary recomputation to start with. I don't want to spend too
much time or
make it complicated. But start with a simple one which will trigger full
recompute for any NB DB or SB DB change.


Thanks
Numan




Thanks,
> Han
>
> >
> >
> >>
> >>
> >> Thanks!
> >>
> >> Tony
> >>
> >> > -Original Message-
> >> > From: Han Zhou 
> >> > Sent: Monday, August 3, 2020 8:22 PM
> >> > To: Tony Liu 
> >> > Cc: Han Zhou ; ovs-discuss <
> ovs-disc...@openvswitch.org
> >> >;
> >> > ovs-dev 
> >> > Subject: Re: [ovs-discuss] [OVN] ovn-northd takes much CPU when no
> >> > configuration update
> >> >
> >> > Sorry that I didn't make it clear enough. The OVSDB probe itself
> doesn't
> >> > take much CPU, but the probe awakes ovn-northd main loop, which
> >> > recompute everything, which is why you see CPU spike.
> >> > It will be solved by incremental-processing, when only delta is
> >> > processed, and in case of probe handling, there is no change in
> >> > configuration, so the delta is zero.
> >> > For now, please follow the steps to adjust probe interval, if the CPU
> of
> >> > ovn-northd (when there is no configuration change) is a concern for
> you.
> >> > But please remember that this has no impact to the real CPU usage for
> >> > handling configuration changes.
> >> >
> >> >
> >> > Thanks,
> >> > Han
> >> >
> >> >
> >> > On Mon, Aug 3, 2020 at 8:11 PM Tony Liu  >> >  > wrote:
> >> >
> >> >
> >> >   Health check (5 sec internal) taking 30%-100% CPU is definitely
> >> not
> >> > acceptable,
> >> >   if that's really the case. There must be some blocking (and not
> >> > yielding CPU)
> >> >   in coding, which is not supposed to be there.
> >> >
> >> >   Could you point me to the coding for such health check?
> >> >   Is it single thread? Does it use any event library?
> >> >
> >> >
> >> >   Thanks!
> >> >
> >> >   Tony
> >> >
> >> >   > -Original Message-
> >> >   > From: Han Zhou mailto:hz...@ovn.org> >
> >> >   > Sent: Saturday, August 1, 2020 9:11 PM
> >> >   > To: Tony Liu  >> >  >
> >> >   > Cc: ovs-discuss mailto:ovs-
> >> > disc...@openvswitch.org> >; ovs-dev  >> >   > d...@openvswitch.org  >
> >> >   > Subject: Re: [ovs-discuss] [OVN] ovn-northd takes much CPU
> when
> >> > no
> >> >   > configuration update
> >> >   >
> >> >   >
> >> >   >
> >> >   > On Fri, Jul 31, 2020 at 4:14 PM Tony Liu <
> >> tonyliu0...@hotmail.com
> >> > 
> >> >   >  >> >  > > wrote:
> >> >   >

Re: [ovs-dev] [PATCH ovn v2] Fix the routing for external logical ports of bridged logical switches.

2020-08-06 Thread Numan Siddique
Thanks Ankur for the lengthy reply.

Please see below for some comments.

Thanks
Numan


On Thu, Jul 30, 2020 at 9:23 AM Ankur Sharma 
wrote:

> Hi Numan, Daniel, Lucas,
>
> Thank  you so much for the feedback and providing your inputs.
> I went the through the bug that was being referenced and Numan's inputs
> and following is my take on it.
> Its a lengthy email, if you just want to look at the suggestions for fix
> then please scroll to the second section :).
>
> Just taking a summary of the input/feedback points and the ones that are
> most pertinent to the discussion.
>
> a. "Hard to colocate the the gateway ports of External port and Router
> port"
> [ANKUR]:  Looking at https://bugzilla.redhat.com/show_bug.cgi?id=1829762.
> I do not see a reference to an attempt for using "HA Chassis group" (or
> may be i missed it).
> OVN has a primitive which allows a HA chassis cluster to be shared across
> multiple entities. I like this primitive, i think it can be easily used
> here.
> And the whole purpose of having a separate table for HA chassis group is
> so that it can be shared.
> Other than openstack implementation, if there are other hindrances in
> adopting it then we should try to address the same in OVN.
>
> b. " if you see the logical swithes ls_vlan4 and ls_vlan5 in this example
> don't provide any N/S connectivity and hence it may not be accurate
> to consider them as having gateway ports."
> [ANKUR]: Its just the terminology and its interpretation, i.e what is our
> interpretation of NS traffic and gateway ports.
> From the router's perspective, all we are trying to say is that for a
> distributed router port, if we receive traffic from non OVN endpoint
> (i.e entry point to chassis is localnet port), then we need to have a
> specific designated chassis which is the entry point for ARP and Unicast
> traffic.
>
> Now, the existing gateway port primitive does exactly what i have
> described above. However, if we want to use some other
> term for discussion/documentation, i am totally fine with it.
>
>
> c. "ideally there should be no external entity sharing the IP address from
> these subnets and using the same VLAN. Although practically it's possible
> to do so"
> [ANKUR]: Even in an ideal world its a valid requirement. Especially for
> vlan backed subnets, it is a valid and practical use case where OVN does
> not own
> the whole ip space of a CIDR. For example, there could be baremetal
> endpoints or there is a transition phase of migrating virtual non ovn
> endpoints
> to ovn, or to avoid the pain of migration there are both ovn and non ovn
> virtual endpoints in same subnet, similarly there could be
> stretched subnet across sites (hybrid cloud use case), where both sites
> (and both of them need not be OVN run)
> manage non overlapping ip pools for their respective endpoints.
>
>
> d.  " If there is such a requirement, I think CMS should consider using
> 'external' port for this"
> [ANKUR]: Use of external port is that we leverage on DHCP service by OVN
> for non ovn endpoints.
> Which is definitely good to have, but not mandatory (infact it MUST not be
> mandatory).
> A deployment could have its own external DHCP server even for ovn
> endpoints or
> external dhcp server for non ovn endpoints or non ovn endpoints have
> static IPs.
> Neither of which mandates that non ovn endpoint has to be marked as
> external.
> Infact, for hybrid connectivity cases like (like stretched subnets across
> sites/AZs as mentioned in c. above),
> mandating DHCP service for whole CIDR by OVN would be non desirable.
> TOR gateway is also a non ovn endpoint :).
>
>
> ==
>
> ALTERNATIVE PROPOSALS:
> There are multiple ways the scenario called out in this thread can be
> addressed.
>
> a. Remove the "ls_in_external_port" stage in pipeline. For example, in the
> topology mentioned here:
>
> https://docs.google.com/document/d/117yskeP1S3qHmkNrBrZ0PxXCvMJCCVJhcPG4phw1Qls/edit?usp=sharing
>
> I see following flows in "ls_in_external_port":
>   table=18(ls_in_external_port), priority=100  , match=(inport ==
> "ls-underlay-physnet1" && eth.src == 50:6b:8d:cc:17:7d &&
> !is_chassis_resident("lsp-external") && arp.tpa == 10.15.24.135 && arp.op
> == 1), action=(drop;)
>   table=18(ls_in_external_port), priority=100  , match=(inport ==
> "ls-underlay-physnet1" && eth.src == 50:6b:8d:cc:17:7d &&
> !is_chassis_resident("lsp-external") && nd_ns && ip6.dst ==
> {fe80::200:1ff:fe01:204, ff02::1:ff01:204} && nd.target ==
> fe80::200:1ff:fe01:204), action=(drop;)
>   table=18(ls_in_external_port), priority=0, match=(1), action=(next;)
>
> My opinion/understanding is that it is on router to decide where it wants
> router port ARP to be resolved. A logical switch port cannot/should not
> decide, where it want the attached router port ip to be resolved.
> If we do not have above flow, then it implies that significance of
> HA_Chassis group for external port is to call out which chassis will behave

Re: [ovs-dev] [ovs-discuss] [OVN] no response to inactivity probe

2020-08-06 Thread Han Zhou
On Wed, Aug 5, 2020 at 9:14 PM Tony Liu  wrote:

> I set the connection target="ptcp:6641:10.6.20.84" for ovn-nb-db
> and "ptcp:6642:10.6.20.84" for ovn-sb-db. .84 is the first node
> of cluster. Also ovn-openflow-probe-interval=30 on compute node.
> It seems helping. Not that many connect/drop/reconnect in logging.
> That "commit failure" is also gone.
> The issue I reported in another thread "packet drop" seems gone.
> And launching VM starts working.
>
> How should I set connection table for all ovn-nb-db and ovn-sb-db
> nodes in the cluster to set inactivity_probe?
> One row with address 0.0.0.0 seems not working.
>

You can simply use 0.0.0.0 in the connection table, but don't specify the
same connection method on the command line when starting ovsdb-server for
NB/SB DB. Otherwise, these are conflicting and that's why you saw "Address
already in use" error.


> Is "external_ids:ovn-remote-probe-interval" in ovsdb-server on
> compute node for ovn-controller to probe ovn-sb-db?
>
> OVSDB probe is bidirectional, so you need to set this value, too, if you
don't want too many probes handled by the SB server. (setting the
connection table for SB only changes the server side).



> Is "external_ids:ovn-openflow-probe-interval" in ovsdb-server on
> compute node for ovn-controller to probe ovsdb-server?
>
> It is for the OpenFlow connection between ovn-controller and ovs-vswitchd,
which is part of the OpenFlow protocol.


> What's probe interval for ovsdb-server to probe ovn-controller?
>
> The local ovsdb connection uses unix socket, which doesn't send probe by
default (if I remember correctly).

For ovn-controller, since it is implemented with incremental-processing,
even if there are probes from openflow or local ovsdb, it doesn't matter.
If there is no configuration change, ovn-controller simply replies the
probe and there is no extra cost.


> Thanks!
>
> Tony
> > -Original Message-
> > From: discuss  On Behalf Of Tony
> > Liu
> > Sent: Wednesday, August 5, 2020 4:29 PM
> > To: Han Zhou 
> > Cc: ovs-dev ; ovs-discuss  > disc...@openvswitch.org>
> > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> >
> > Hi Han,
> >
> > After setting connection target="ptcp:6642:0.0.0.0" for ovn-sb-db, I see
> > this error.
> > 
> > 2020-08-
> > 05T23:01:26.819Z|06799|ovsdb_jsonrpc_server|ERR|ptcp:6642:0.0.0.0:
> > listen failed: Address already in use  Anything I am missing
> > here?
> >
> >
> > Thanks!
> >
> > Tony
> > > -Original Message-
> > > From: Han Zhou 
> > > Sent: Tuesday, August 4, 2020 4:44 PM
> > > To: Tony Liu 
> > > Cc: Numan Siddique ; Han Zhou ; ovs-
> > > discuss ; ovs-dev
> > > 
> > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > >
> > >
> > >
> > > On Tue, Aug 4, 2020 at 2:50 PM Tony Liu  > >  > wrote:
> > >
> > >
> > > Hi,
> > >
> > > Since I have 3 OVN DB nodes, should I add 3 rows in connection
> > table
> > > for the inactivity_probe? Or put 3 addresses into one row?
> > >
> > > "set-connection" set one row only, and there is no
> "add-connection".
> > > How should I add 3 rows into the table connection?
> > >
> > >
> > >
> > >
> > > You only need to set one row. Try this command:
> > >
> > > ovn-nbctl -- --id=@conn_uuid create Connection
> > > target="ptcp\:6641\:0.0.0.0" inactivity_probe=0 -- set NB_Global .
> > > connections=@conn_uuid
> > >
> > >
> > >
> > > Thanks!
> > >
> > > Tony
> > >
> > > > -Original Message-
> > > > From: Numan Siddique mailto:num...@ovn.org> >
> > > > Sent: Tuesday, August 4, 2020 12:36 AM
> > > > To: Tony Liu  > >  >
> > > > Cc: ovs-discuss mailto:ovs-
> > > disc...@openvswitch.org> >; ovs-dev  > > > d...@openvswitch.org  >
> > > > Subject: Re: [ovs-discuss] [OVN] no response to inactivity probe
> > > >
> > > >
> > > >
> > > > On Tue, Aug 4, 2020 at 9:12 AM Tony Liu  > > 
> > > >  > >  > > wrote:
> > > >
> > > >
> > > >   In my deployment, on each Neutron server, there are 13
> > > Neutron
> > > > server processes.
> > > >   I see 12 of them (monitor, maintenance, RPC, API) connect
> > > to both
> > > > ovn-nb-db
> > > >   and ovn-sb-db. With 3 Neutron server nodes, that's 36 OVSDB
> > > clients.
> > > >   Is so many clients OK?
> > > >
> > > >   Any suggestions how to figure out which side doesn't
> > > respond the
> > > > probe,
> > > >   if it's bi-directional? I don't see any activities from
> > > logging,
> > > > other than
> > > >   connect/drop and reconnect...
> > > >
> > > >   BTW, please let me know if this is not the right place to
> > > discuss
> > > > Neutron OVN
> > > >   ML2 driver.
> > > >
> > > >
>