Re: [ovs-dev] [PATCH v3] ovn-openstack.rst: Miscelaneous fixes.

[0-day Robot]

Bleep bloop.  Greetings Flavio Fernandes, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Line is 99 characters long (recommended limit is 79)
#37 FILE: Documentation/tutorials/ovn-openstack.rst:71:
   
https://github.com/openstack/devstack/blob/master/doc/source/guides/devstack-with-nested-kvm.rst

Lines checked: 174, Warnings: 1, Errors: 0


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

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

[ovs-dev] [PATCH v3] ovn-openstack.rst: Miscelaneous fixes.

[Flavio Fernandes]

Added a few clarifications to improve the tutorial:

  - VM needs to be at least 6 Gb on latest openstack (ouch!);
  - VM must support nesting;
  - local.conf may require HOST_IP depending on device name;
  - Omit --project from network create to be consistent with OS_PROJECT_NAME;
  - Use a temporary file to ensure datapath flow for recirc(0) is obtained;
  - Reword what to expect when tracing packet before logical router creation;
  - Fix minor mistake on the usage of netcat command;
  - Use MAC of logical router's IPv4 interface to make ACL example more 
realistic.

Signed-off-by: Flavio Fernandes 
---
v1->v2: Fix [most] issues found by 0-day robot. Bleep bloop. :)
The one lingering issue I don't know how to fix, bc I rather not break 
up url.
v2->v3: Fix backslash mistake introduced on v2.

Open issues that I will look into in parallel:
  - With OVN moving to its own repo, this patch may need to move as well.
  - I was not able to see NAT entries in a logical flow when following the 
Gateway section.
---
 Documentation/tutorials/ovn-openstack.rst | 64 +++
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/Documentation/tutorials/ovn-openstack.rst 
b/Documentation/tutorials/ovn-openstack.rst
index c6dff5e55..353ef209e 100644
--- a/Documentation/tutorials/ovn-openstack.rst
+++ b/Documentation/tutorials/ovn-openstack.rst
@@ -62,12 +62,16 @@ with the tutorial in full.
 Unless you have a spare computer laying about, it's easiest to install
 DevStacck in a virtual machine.  This tutorial was built using a VM
 implemented by KVM and managed by virt-manager.  I recommend
-configuring the VM configured for the x86-64 architecture, 4 GB RAM, 2
+configuring the VM configured for the x86-64 architecture, 6 GB RAM, 2
 VCPUs, and a 20 GB virtual disk.
 
 .. note::
+   Since we will be creating VMs inside this VM, it is important to have
+   nesting configured properly. See
+   
https://github.com/openstack/devstack/blob/master/doc/source/guides/devstack-with-nested-kvm.rst
+   for details on that.
 
-   If you happen to run your Linux-based host with 32-bit userspace,
+   Also, if you happen to run your Linux-based host with 32-bit userspace,
then you will have some special issues, even if you use a 64-bit
kernel:
 
@@ -174,6 +178,12 @@ Here are step-by-step instructions to get started:
 
.. note::
 
+  Depending on the name of the network device used by the VM, devstack
+  may be unable to automatically obtain its IP address. If that happens,
+  edit ``local.conf`` and explicitly provide it (X marks the spot)::
+
+HOST_IP=X
+
   If you installed a 32-bit i386 guest (against the advice above),
   at this point edit ``local.conf`` to add the following line::
 
@@ -430,7 +440,7 @@ Creating network ``n1``
 
 Let's start by creating the network::
 
-  $ openstack network create --project admin --provider-network-type geneve n1
+  $ openstack network create --provider-network-type geneve n1
 
 OpenStack needs to know the subnets that a network serves.  We inform
 it by creating subnet objects.  To keep it simple, let's give our
@@ -997,6 +1007,22 @@ this::
 
   
recirc_id(0),in_port(3),eth(src=fa:16:3e:f5:2a:90),eth_type(0x0800),ipv4(src=10.1.1.5,frag=no),
 packets:388, bytes:38024, used:0.977s, actions:ct(zone=8),recirc(0x18)
 
+.. note::
+
+  Flows in the datapath can expire quickly and the ``watch`` command
+  mentioned above may be too slow to catch it. If that is your
+  case, stop the ``ping 10.1.1.6`` session and re-start it a few
+  seconds after this command::
+
+$ sudo conntrack -F ; rm -f /tmp/flows.txt ; \
+ for _ in $(seq 100) ; do \
+ sudo ovs-dpctl dump-flows >> /tmp/flows.txt ; \
+ sleep 0.1 ; done
+
+  Then, look for ``recirc_id(0)`` in flows.txt after ping command was issued::
+
+$ sort --uniq /tmp/flows.txt | grep zone
+
 We can hand the first part of this (everything up to the first space)
 to ``ofproto/trace``, and it will tell us what happens::
 
@@ -1064,7 +1090,7 @@ tracing with ``recirc_id(0xc)``::
   ...
   Datapath actions: 4
 
-It was took multiple hops, but we finally came to the end of the line
+It took multiple hops, but we finally came to the end of the line
 where the packet was output to ``b`` after passing through both
 firewalls.  The port number here is a datapath port number, which is
 usually different from an OpenFlow port number.  To check that it is
@@ -1114,15 +1140,16 @@ works in OVN.
 There's nothing really new for the network and the VM so let's just go
 ahead and create them::
 
-  $ openstack network create --project admin --provider-network-type geneve n2
+  $ openstack network create --provider-network-type geneve n2
   $ openstack subnet create --subnet-range 10.1.2.0/24 --network n2 n2subnet
   $ openstack server create --nic net-id=n2,v4-fixed-ip=10.1.2.7 --flavor 
m1.nano --image $IMAGE_ID --key-name demo c
   $ openstack port set --name cp $(openstack 

Re: [ovs-dev] [PATCH v2] ovn-openstack.rst: Miscelaneous fixes.

[0-day Robot]

Bleep bloop.  Greetings Flavio Fernandes, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Line is 99 characters long (recommended limit is 79)
#37 FILE: Documentation/tutorials/ovn-openstack.rst:71:
   
https://github.com/openstack/devstack/blob/master/doc/source/guides/devstack-with-nested-kvm.rst

Lines checked: 174, Warnings: 1, Errors: 0


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

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

[ovs-dev] [PATCH v2] ovn-openstack.rst: Miscelaneous fixes.

[Flavio Fernandes]

Added a few clarifications to improve the tutorial:

  - VM needs to be at least 6 Gb on latest openstack (ouch!);
  - VM must support nesting;
  - local.conf may require HOST_IP depending on device name;
  - Omit --project from network create to be consistent with OS_PROJECT_NAME;
  - Use a temporary file to ensure datapath flow for recirc(0) is obtained;
  - Reword what to expect when tracing packet before logical router creation;
  - Fix minor mistake on the usage of netcat command;
  - Use MAC of logical router's IPv4 interface to make ACL example more 
realistic.

Signed-off-by: Flavio Fernandes 
---
v1->v2: Fix [most] issues found by 0-day robot. Bleep bloop. :)
The one lingering issue I don't know how to fix, bc I rather not break 
up url

Open issues that I will look into in parallel:
  - With OVN moving to its own repo, this patch may need to move as well.
  - I was not able to see NAT entries in a logical flow when following the 
Gateway section.


Documentation/tutorials/ovn-openstack.rst | 64 +++
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/Documentation/tutorials/ovn-openstack.rst 
b/Documentation/tutorials/ovn-openstack.rst
index c6dff5e55..06715af71 100644
--- a/Documentation/tutorials/ovn-openstack.rst
+++ b/Documentation/tutorials/ovn-openstack.rst
@@ -62,12 +62,16 @@ with the tutorial in full.
 Unless you have a spare computer laying about, it's easiest to install
 DevStacck in a virtual machine.  This tutorial was built using a VM
 implemented by KVM and managed by virt-manager.  I recommend
-configuring the VM configured for the x86-64 architecture, 4 GB RAM, 2
+configuring the VM configured for the x86-64 architecture, 6 GB RAM, 2
 VCPUs, and a 20 GB virtual disk.
 
 .. note::
+   Since we will be creating VMs inside this VM, it is important to have
+   nesting configured properly. See
+   
https://github.com/openstack/devstack/blob/master/doc/source/guides/devstack-with-nested-kvm.rst
+   for details on that.
 
-   If you happen to run your Linux-based host with 32-bit userspace,
+   Also, if you happen to run your Linux-based host with 32-bit userspace,
then you will have some special issues, even if you use a 64-bit
kernel:
 
@@ -174,6 +178,12 @@ Here are step-by-step instructions to get started:
 
.. note::
 
+  Depending on the name of the network device used by the VM, devstack
+  may be unable to automatically obtain its IP address. If that happens,
+  edit ``local.conf`` and explicitly provide it (X marks the spot)::
+
+HOST_IP=X
+
   If you installed a 32-bit i386 guest (against the advice above),
   at this point edit ``local.conf`` to add the following line::
 
@@ -430,7 +440,7 @@ Creating network ``n1``
 
 Let's start by creating the network::
 
-  $ openstack network create --project admin --provider-network-type geneve n1
+  $ openstack network create --provider-network-type geneve n1
 
 OpenStack needs to know the subnets that a network serves.  We inform
 it by creating subnet objects.  To keep it simple, let's give our
@@ -997,6 +1007,22 @@ this::
 
   
recirc_id(0),in_port(3),eth(src=fa:16:3e:f5:2a:90),eth_type(0x0800),ipv4(src=10.1.1.5,frag=no),
 packets:388, bytes:38024, used:0.977s, actions:ct(zone=8),recirc(0x18)
 
+.. note::
+
+  Flows in the datapath can expire quickly and the ``watch`` command
+  mentioned above may be too slow to catch it. If that is your
+  case, stop the ``ping 10.1.1.6`` session and re-start it a few
+  seconds after this command::
+
+$ sudo conntrack -F ; rm -f /tmp/flows.txt ; \
+ for _ in $(seq 100) ; do \
+ sudo ovs-dpctl dump-flows >> /tmp/flows.txt ; \
+ sleep 0.1 ; done
+
+  Then, look for ``recirc_id(0)`` in flows.txt after ping command was issued::
+
+$ sort --uniq /tmp/flows.txt | grep zone
+
 We can hand the first part of this (everything up to the first space)
 to ``ofproto/trace``, and it will tell us what happens::
 
@@ -1064,7 +1090,7 @@ tracing with ``recirc_id(0xc)``::
   ...
   Datapath actions: 4
 
-It was took multiple hops, but we finally came to the end of the line
+It took multiple hops, but we finally came to the end of the line
 where the packet was output to ``b`` after passing through both
 firewalls.  The port number here is a datapath port number, which is
 usually different from an OpenFlow port number.  To check that it is
@@ -1114,15 +1140,16 @@ works in OVN.
 There's nothing really new for the network and the VM so let's just go
 ahead and create them::
 
-  $ openstack network create --project admin --provider-network-type geneve n2
+  $ openstack network create --provider-network-type geneve n2
   $ openstack subnet create --subnet-range 10.1.2.0/24 --network n2 n2subnet
   $ openstack server create --nic net-id=n2,v4-fixed-ip=10.1.2.7 --flavor 
m1.nano --image $IMAGE_ID --key-name demo c
   $ openstack port set --name cp $(openstack port list --server c -f value -c 
ID)
   $ 

Re: [ovs-dev] [PATCH ovn v2] OVN: Fix learning of neighbors from ARP/ND packets.

[Han Zhou]

On Wed, Aug 7, 2019 at 8:12 AM Dumitru Ceara  wrote:

> On Mon, Aug 5, 2019 at 5:34 PM Han Zhou  wrote:
> >
> >
> >
> > On Mon, Aug 5, 2019 at 1:06 AM Dumitru Ceara  wrote:
> > >
> > > On Mon, Aug 5, 2019 at 1:41 AM Han Zhou  wrote:
> > > >
> > > >
> > > >
> > > > On Mon, Jul 29, 2019 at 2:30 AM Dumitru Ceara 
> wrote:
> > > > >
> > > > > Add a restriction on the target protocol addresses to match the
> > > > > configured subnets. All other ARP/ND packets are invalid in this
> > > > > context.
> > > > >
> > > > > One exception is for ARP replies that are received for route
> next-hops
> > > > > that are only reachable via a port but can't be directly resolved
> > > > > through route lookups. Such support was introduced by commit:
> > > > >
> > > > > 6b785fd8fe29 ("ovn-util: Allow /32 IP addresses for router ports.")
> > > > >
> > > > > Reported-at: https://bugzilla.redhat.com/1729846
> > > > > Reported-by: Haidong Li 
> > > > > CC: Han Zhou 
> > > > > CC: Guru Shetty 
> > > > > Fixes: b068454082f5 ("ovn-northd: Support learning neighbor from
> ARP request.")
> > > > > Signed-off-by: Dumitru Ceara 
> > > >
> > > > Hi Dumitru,
> > >
> > > Hi Han,
> > >
> > > Thanks for reviewing this.
> > >
> > > >
> > > > Sorry for my slow response, and thanks a lot for revising the patch
> for a bigger scope of validations. However, the exception of /32 address
> makes me thinking more about this patch. If ARP replies is allowed from any
> nexthop for a LR port with /32, at least ARP request for GARP purpose
> should also be allowed. But before asking for a v3, I'd hold on to rethink
> the purpose of this patch.
> > >
> > > Right, we should allow GARP requests too. If we decide to go ahead
> > > with this patch I'll add a function in v3 to handle all types of ARPs
> > > and call it both for unreachable static route next-hops and attached
> > > subnets.
> > >
> > > >
> > > > The nexthop specific flows are now from static routes. What if OVN
> support dynamical routing protocols in the future? Of course we can then
> take those dynamic nexthops into allowed peers. But then I am thinking what
> is the real benefit of all these restrictions? Why can't we just have
> simpler logic to handle all these situations without validation? I think
> the major benefit of the validation is to avoid handling the noise ARP/NDs
> when multiple subnets shares same L2, but most cases it is really not a big
> deal, right? For the CPU problem caused by ARP flooding as mentioned by the
> bug report, it is a real problem, but this patch seems not really helpful
> for that, because the attacker can just trigger the same CPU problem with
> *valid* packets. So I am not sure if the benefit of the change is worth the
> complexity it introduced. Please share your thought and correct me if I
> missed something.
> > > >
> > > > Thanks,
> > > > Han
> > >
> > > I assume the simpler logic to handle all these situations without
> > > validation is to add rate limiting for ARP packets that get punted to
> > > the controller. I agree that this should be implemented too.
> > >
> > > But I think rate limiting all ARP packets regardless of IP addresses
> > > is not enough. In the following scenario and if we would have a way to
> > > rate limit ARP packets:
> > > - Subnet 42.42.42.0/24 configured on the OVN
> > > - "Invalid" ARP packets are injected at high rate in the network for
> 41.41.41.41
> > > - Host 42.42.42.42 sends GARP.
> > > - Rate limiting of ARP packets towards controller at 100pps
> > >
> > > With the current code, ARP packets for 41.41.41.41 will be punted to
> > > controller at a rate of at most 100 per second. But the chances that
> > > the valid 42.42.42.42 GARP is dropped is really high.
> > >
> > > If we instead use the following approach:
> > > a. Punt only useful ARPs to controller.
> > > b. Rate limit ARPs that are sent to controller.
> > >
> > > Then ARP packets outside 42.42.42./24 are never punted to the
> > > controller and don't consume any rate limiting tokens. For the second
> > > case, when an attacker would flood with valid ARP packets we would
> > > have the rate limit in place to protect the controller CPU.
> > >
> > > My commit addresses point "a" above as I think point "b" should be
> > > done in a generic way for all protocol packets that need to reach the
> > > controller.
> > >
> > > For dynamic routing protocols on the other hand I think we should not
> > > install routes with next-hops that are unreachable through other
> > > direct or indirect routes.
> > >
> > > Thanks,
> > > Dumitru
> >
> > I agree that blindly ARP rate limit is not helpful, but a) is not really
> helpful in this case either. In your example, the attacker can just use any
> valid IP in 42.42.42.0/24 to send GARP flooding, which would result in
> exactly same result that a useful GARP from 42.42.42.42 is dropped because
> of blindly rate limiting all ARPs. To solve the problem properly, the ARP
> rate limiting must be done per IP.
>
> Ok, ideally ARP 

Re: [ovs-dev] [PATCH v2 2/9] ovs-vsctl: Add conntrack zone commands.

[Darrell Ball]

On Wed, Aug 7, 2019 at 1:37 PM William Tu  wrote:

> Thanks for the review.
>
> On Mon, Aug 05, 2019 at 04:12:02PM -0700, Darrell Ball wrote:
> > Thanks for the patch
> >
> > I noticed '--may-exist' and '--if-exists' are supported now for
> > add--zone-tp/del-zone-tp - thanks
> > The check for duplicate timeout policies now correctly checks all key and
> > values - thanks
>
> yes, thanks. Will do it in next version.
> >
> > Some more comments inline
> > I am trying to avoid duplicate comment from Justin, so I just won't
> comment
> > on some parts in this version
> > to avoid confusion.
> >
> >
> > On Thu, Aug 1, 2019 at 3:09 PM Yi-Hung Wei  wrote:
> >
> > > From: William Tu 
> > >
> > > The patch adds commands creating/deleting/listing conntrack zone
> > > timeout policies:
> > >   $ ovs-vsctl {add,del,list}-zone-tp zone=zone_id ...
> > >
> > > Signed-off-by: William Tu 
> > > ---
> > >  tests/ovs-vsctl.at   |  34 +++-
> > >  utilities/ovs-vsctl.8.in |  25 ++
> > >  utilities/ovs-vsctl.c| 204
> > > +++
> > >  3 files changed, 261 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > > index 46fa3c5b1a33..f0c5975edd0e 100644
> > > --- a/tests/ovs-vsctl.at
> > > +++ b/tests/ovs-vsctl.at
> > > @@ -805,6 +805,20 @@ AT_CHECK(
> > >[RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567
> > > "'])])
> > >  AT_CHECK(
> > >[RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
> > > +
> > > +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath
> datapath_version=0 --
> > > set Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
> > >
> >
> > What happens if we add a datapath type here and there are no bridges of
> > that type; meaning the datapath of that type does not even exist.
> > Seems like a contradiction.
> > Maybe we should check for that at least and raise an error.
> > Ideally, it is better if these 'datapaths' are auto-managed by bridge
> > creation/deletion with given datapath types,
> > but we can certainly defer that.
> >
> >
> > > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> > > icmp_reply=2])])
> > >
> >
> > I mentioned this in V1:
> > There is no filtering of bad timeout key; for example
> >
> > AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> > foo_bar=2])])
> >
> > is accepted as valid
> >
> > Even worse, a minor typo will go unnoticed - missing 'y' in 'icmp_reply'.
> >
> > AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> > icmp_repl=2])])
>
> agree.
> >
> >
> > > +AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=1
> > > icmp_first=1 icmp_reply=2])])
> > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> > > Policies: icmp_first=1 icmp_reply=2
> > >
> >
> > I mentioned in V1
> > We should check all possible timeout keys to make sure they work.
>
> OK.
> >
> >
> > > +])
> > > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> > > icmp_reply=3])])
> > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> > > Policies: icmp_first=1 icmp_reply=2
> > > +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
> > > +])
> > > +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> > > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
> > > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout
> > > Policies: icmp_first=2 icmp_reply=3
> > > +])
> > >  OVS_VSCTL_CLEANUP
> > >  AT_CLEANUP
> > >
> > > @@ -890,10 +904,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0
> > > flood_vlans=-1])],
> > >  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
> > >[1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid
> > > range 0 to 4095 (inclusive)
> > >  ])
> > > -AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
> > > +AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
> > >
> >
> > I mentioned in V1.
> > I don't think we should make unrelated changes in a feature patch
> > especially since it seems the author wanted to convey
> > short form syntax is valid
>
> This has to be there, otherwise test fails.
>


yep, I got the obvious reason after running the negative test.

+ovs-vsctl: multiple table names match "c"




> >
> >
> > >[1], [], [[ovs-vsctl: constraint violation: xyz is not one of the
> > > allowed values ([in-band, out-of-band])
> > >  ]])
> > > -AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
> > > +AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
> > >[1], [], [ovs-vsctl: cannot specify key to set for non-map column
> > > connection_mode
> > >  ])
> > >  AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
> > > @@ -929,6 +943,22 @@ AT_CHECK([RUN_OVS_VSCTL([remove bridge br1
> > > flood-vlans true])],
> > >  AT_CHECK([RUN_OVS_VSCTL([clear bridge br1 name])],
> > >[1], [], [ovs-vsctl: cannot modify read-only 

Re: [ovs-dev] [PATCH 1/1] ovn-openstack.rst: Miscelaneous fixes.

[0-day Robot]

Bleep bloop.  Greetings Flavio Fernandes, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Line is 99 characters long (recommended limit is 79)
#37 FILE: Documentation/tutorials/ovn-openstack.rst:71:
   
https://github.com/openstack/devstack/blob/master/doc/source/guides/devstack-with-nested-kvm.rst

WARNING: Line is 80 characters long (recommended limit is 79)
#51 FILE: Documentation/tutorials/ovn-openstack.rst:183:
  to you, edit ``local.conf`` and explicitly provide it (X marks the spot)::

WARNING: Line has non-spaces leading whitespace
#80 FILE: Documentation/tutorials/ovn-openstack.rst:1019:
 sudo ovs-dpctl dump-flows >> /tmp/flows.txt ; \

WARNING: Line has non-spaces leading whitespace
#81 FILE: Documentation/tutorials/ovn-openstack.rst:1020:
 sleep 0.1 ; done

WARNING: Line is 103 characters long (recommended limit is 79)
#135 FILE: Documentation/tutorials/ovn-openstack.rst:1772:
$ N1SUBNET4_MAC=$(ovn-nbctl --bare --columns=mac find logical_router_port 
networks=\"10.1.1.1/24\")

WARNING: Line is 197 characters long (recommended limit is 79)
#140 FILE: Documentation/tutorials/ovn-openstack.rst:1776:
$ ovn-trace --ct new --ct new --minimal n1 'inport == "ap" && eth.src == 
'$AP_MAC' && eth.dst == '$N1SUBNET4_MAC' && ip4.src == 10.1.1.5 && ip4.dst == 
10.1.2.7 && ip.ttl == 64 && tcp.dst == 22'

WARNING: Line is 197 characters long (recommended limit is 79)
#149 FILE: Documentation/tutorials/ovn-openstack.rst:1791:
$ ovn-trace --ct new --ct new --minimal n1 'inport == "ap" && eth.src == 
'$AP_MAC' && eth.dst == '$N1SUBNET4_MAC' && ip4.src == 10.1.1.5 && ip4.dst == 
10.1.2.7 && ip.ttl == 64 && tcp.dst == 23'

WARNING: Line is 205 characters long (recommended limit is 79)
#158 FILE: Documentation/tutorials/ovn-openstack.rst:1804:
$ ovn-trace --ct est,rpl --ct est,rpl --minimal n1 'inport == "ap" && 
eth.src == '$AP_MAC' && eth.dst == '$N1SUBNET4_MAC' && ip4.src == 10.1.1.5 && 
ip4.dst == 10.1.2.7 && ip.ttl == 64 && tcp.dst == 23'

Lines checked: 164, Warnings: 8, Errors: 0


build:
-e 's,[@]PYTHON[@],/bin/python2,g' \
-e 's,[@]RUNDIR[@],/usr/local/var/run/openvswitch,g' \
-e 's,[@]VERSION[@],2.12.90,g' \
-e 's,[@]localstatedir[@],/usr/local/var,g' \
-e 's,[@]pkgdatadir[@],/usr/local/share/openvswitch,g' \
-e 's,[@]sysconfdir[@],/usr/local/etc,g' \
-e 's,[@]bindir[@],/usr/local/bin,g' \
-e 's,[@]sbindir[@],/usr/local/sbin,g' \
-e 
's,[@]abs_builddir[@],/var/lib/jenkins/jobs/upstream_build_from_pw/workspace,g' 
\
-e 
's,[@]abs_top_srcdir[@],/var/lib/jenkins/jobs/upstream_build_from_pw/workspace,g'
 \
  > rhel/ovn-fedora.spec.tmp
mv rhel/ovn-fedora.spec.tmp rhel/ovn-fedora.spec
(printf '\043 Generated automatically -- do not modify!-*- 
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.12.90,g') \
< ./xenserver/openvswitch-xen.spec.in > openvswitch-xen.spec.tmp || 
exit 1; \
if cmp -s openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; then touch 
xenserver/openvswitch-xen.spec; rm openvswitch-xen.spec.tmp; else mv 
openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; fi
make[3]: Entering directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/datapath'
make[3]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/datapath'
Documentation/tutorials/ovn-openstack.rst
See above for files that use tabs for indentation.
Please use spaces instead.
make[2]: *** [check-tabs] Error 1
make[2]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory 
`/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


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

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

[ovs-dev] [PATCH 1/1] ovn-openstack.rst: Miscelaneous fixes.

[Flavio Fernandes]

Added a few clarifications to improve the tutorial:

  - VM needs to be at least 6 Gb on latest openstack (ouch!);
  - VM must support nesting;
  - local.conf may require HOST_IP depending on device name;
  - Omit --project from network create to be consistent with OS_PROJECT_NAME;
  - Use a temporary file to ensure datapath flow for recirc(0) is obtained;
  - Reword what to expect when tracing packet before logical router creation;
  - Fix minor mistake on the usage of netcat command;
  - Use MAC of logical router's IPv4 interface to make ACL example more 
realistic.

Signed-off-by: Flavio Fernandes 
---
Open issues that I will look into in parallel:
  - With OVN moving to its own repo, this patch may need to move as well.
  - I was not able to see NAT entries in a logical flow when following the 
Gateway section.


 Documentation/tutorials/ovn-openstack.rst | 54 ++-
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/Documentation/tutorials/ovn-openstack.rst 
b/Documentation/tutorials/ovn-openstack.rst
index c6dff5e55..b5d776dd8 100644
--- a/Documentation/tutorials/ovn-openstack.rst
+++ b/Documentation/tutorials/ovn-openstack.rst
@@ -62,12 +62,16 @@ with the tutorial in full.
 Unless you have a spare computer laying about, it's easiest to install
 DevStacck in a virtual machine.  This tutorial was built using a VM
 implemented by KVM and managed by virt-manager.  I recommend
-configuring the VM configured for the x86-64 architecture, 4 GB RAM, 2
+configuring the VM configured for the x86-64 architecture, 6 GB RAM, 2
 VCPUs, and a 20 GB virtual disk.
 
 .. note::
+   Since we will be creating VMs inside this VM, it is important to have
+   nesting configured properly. See
+   
https://github.com/openstack/devstack/blob/master/doc/source/guides/devstack-with-nested-kvm.rst
+   for details on that.
 
-   If you happen to run your Linux-based host with 32-bit userspace,
+   Also, if you happen to run your Linux-based host with 32-bit userspace,
then you will have some special issues, even if you use a 64-bit
kernel:
 
@@ -174,6 +178,12 @@ Here are step-by-step instructions to get started:
 
.. note::
 
+  Depending on the name of the network device used by the VM, devstack
+  may be unable to automatically obtain its IP address. If that happens
+  to you, edit ``local.conf`` and explicitly provide it (X marks the 
spot)::
+
+HOST_IP=X
+
   If you installed a 32-bit i386 guest (against the advice above),
   at this point edit ``local.conf`` to add the following line::
 
@@ -430,7 +440,7 @@ Creating network ``n1``
 
 Let's start by creating the network::
 
-  $ openstack network create --project admin --provider-network-type geneve n1
+  $ openstack network create --provider-network-type geneve n1
 
 OpenStack needs to know the subnets that a network serves.  We inform
 it by creating subnet objects.  To keep it simple, let's give our
@@ -997,6 +1007,22 @@ this::
 
   
recirc_id(0),in_port(3),eth(src=fa:16:3e:f5:2a:90),eth_type(0x0800),ipv4(src=10.1.1.5,frag=no),
 packets:388, bytes:38024, used:0.977s, actions:ct(zone=8),recirc(0x18)
 
+.. note::
+
+  Flows in the datapath can expire quickly and the ``watch`` command
+  mentioned above may be too slow to catch it. If that is your
+  case, stop the ``ping 10.1.1.6`` session and re-start it a few
+  seconds after this command::
+
+$ sudo conntrack -F ; rm -f /tmp/flows.txt ; \
+ for _ in $(seq 100) ; do \
+sudo ovs-dpctl dump-flows >> /tmp/flows.txt ; \
+sleep 0.1 ; done
+
+  Then, look for ``recirc_id(0)`` in flows.txt after ping command was issued::
+
+$ sort --uniq /tmp/flows.txt | grep zone
+
 We can hand the first part of this (everything up to the first space)
 to ``ofproto/trace``, and it will tell us what happens::
 
@@ -1064,7 +1090,7 @@ tracing with ``recirc_id(0xc)``::
   ...
   Datapath actions: 4
 
-It was took multiple hops, but we finally came to the end of the line
+It took multiple hops, but we finally came to the end of the line
 where the packet was output to ``b`` after passing through both
 firewalls.  The port number here is a datapath port number, which is
 usually different from an OpenFlow port number.  To check that it is
@@ -1114,15 +1140,16 @@ works in OVN.
 There's nothing really new for the network and the VM so let's just go
 ahead and create them::
 
-  $ openstack network create --project admin --provider-network-type geneve n2
+  $ openstack network create --provider-network-type geneve n2
   $ openstack subnet create --subnet-range 10.1.2.0/24 --network n2 n2subnet
   $ openstack server create --nic net-id=n2,v4-fixed-ip=10.1.2.7 --flavor 
m1.nano --image $IMAGE_ID --key-name demo c
   $ openstack port set --name cp $(openstack port list --server c -f value -c 
ID)
   $ CP_MAC=$(openstack port show -f value -c mac_address cp)
 
 The new network ``n2`` is not yet connected to ``n1`` in any way.  You
-can try tracing a 

Re: [ovs-dev] [PATCH v2] datapath: compat: Backports bugfixes for nf_conncount

[Yifeng Sun]

Thanks YiHung for reviewing. I've sent out a new version.

Best,
Yifeng

On Wed, Aug 7, 2019 at 10:24 AM Yi-Hung Wei  wrote:
>
> On Tue, Aug 6, 2019 at 5:06 PM Yifeng Sun  wrote:
> >
> > This patch backports several critical bug fixes related to
> > locking and data consistency in nf_conncount code.
> >
> > This backport is based on the following upstream net-next upstream commits.
> > 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is 
> > negative")
> > d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of 
> > rb_link_node()")
> > 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> > 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.")
> > 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> > fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
> >
> > This patch also added additional compat code so that it can build on
> > all supported kernel versions.
> >
> > Travis tests are at
> > https://travis-ci.org/yifsun/ovs-travis/builds/568603796
> >
> > VMware-BZ: #2396471
> >
> > CC: Taehee Yoo 
> > Signed-off-by: Yifeng Sun 
> > ---
>
> Hi Yifeng,
>
> Thanks for the patch. This backport looks good in general.
>
> I found that there are couple of fixes on nf_conncount in upstream
> kernel.  Since we would like to use df4a90250976 ("netfilter:
> nf_conncount: merge lookup and add functions") to determine if the
> kernel has all the fixes. It would be good to bring other commits on
> the same date to our datapath compat code base as well. That is to
> squash all the following commits.
>
> One more suggestion is to squash the other patch that you sent
> ("datapath: Apply bug fixes of nf_conncount for different kernel
> versions") into this one, since updating "HAVE_UPSTREAM_NF_CONNCOUNT"
> is only useful with this patch.
>
>
> a007232066f6 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> c80f10bc973a ("netfilter: nf_conncount: speculative garbage collection
> on empty lists")
> 2f971a8f4255 ("netfilter: nf_conncount: move all list iterations under
> spinlock")
> df4a90250976 ("netfilter: nf_conncount: merge lookup and add functions")
> e8cfb372b38a ("netfilter: nf_conncount: restart search when nodes have
> been erased")
> f7fcc98dfc2d ("netfilter: nf_conncount: split gc in two phases")
> 4cd273bb91b3 ("netfilter: nf_conncount: don't skip eviction when age
> is negative")
> c78e7818f16f ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS
> with CONNCOUNT_SLOTS")
> d4e7df16567b ("netfilter: nf_conncount: use rb_link_node_rcu() instead
> of rb_link_node()")
> 53ca0f2fec39 ("netfilter: nf_conncount: remove wrong condition check routine")
> 3c5cdb17c3be ("netfilter: nf_conncount: fix unexpected permanent node of 
> list.")
> 31568ec09ea0 ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> fd3e71a9f71e ("netfilter: nf_conncount: use spin_lock_bh instead of 
> spin_lock")
>
> Thanks,
>
> -Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount

[Yifeng Sun]

This patch backports several critical bug fixes related to
locking and data consistency in nf_conncount code.

This backport is based on the following upstream net-next upstream commits.
a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
c80f10b ("netfilter: nf_conncount: speculative garbage collection on empty 
lists")
2f971a8 ("netfilter: nf_conncount: move all list iterations under spinlock")
df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been erased")
f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
4cd273b ("netfilter: nf_conncount: don't skip eviction when age is negative")
c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with 
CONNCOUNT_SLOTS")
d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of 
rb_link_node()")
53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.")
31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")

This patch adds additional compat code so that it can build on
all supported kernel versions.

In addition, this patch helps OVS datapath to always choose bug-fixed
nf_conncount code. If kernel already has these fixes, then kernel's
nf_conncount is being used. Otherwise, OVS falls back to use compat
nf_conncount functions.

Travis tests are at
https://travis-ci.org/yifsun/ovs-travis/builds/569056850
On latest RHEL kernel, 'make check-kmod' runs good.

VMware-BZ: #2396471

Signed-off-by: Yifeng Sun 
---
v1->v2: Add fixes to support old kernel versions, as suggested by YiHung.
v2->v3: Backport more upstream patches. Thanks YiHung for reviewing.
 acinclude.m4   |   6 +-
 datapath/linux/Modules.mk  |   3 +-
 datapath/linux/compat/include/linux/rbtree.h   |  19 ++
 .../include/net/netfilter/nf_conntrack_count.h |   7 -
 datapath/linux/compat/nf_conncount.c   | 290 ++---
 5 files changed, 161 insertions(+), 164 deletions(-)
 create mode 100644 datapath/linux/compat/include/linux/rbtree.h

diff --git a/acinclude.m4 b/acinclude.m4
index 116ffcf9096d..f0e38898b17a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -713,7 +713,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_nat.h], [nf_nat_range2])
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_seqadj.h], 
[nf_ct_seq_adjust])
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_count.h], 
[nf_conncount_gc_list],
-  [OVS_DEFINE([HAVE_UPSTREAM_NF_CONNCOUNT])])
+  
[OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_count.h],
+   [int nf_conncount_add],
+   [], 
[OVS_DEFINE([HAVE_UPSTREAM_NF_CONNCOUNT])])])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/random.h], [prandom_u32])
   OVS_GREP_IFELSE([$KSRC/include/linux/random.h], [prandom_u32_max])
@@ -1012,6 +1014,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   [OVS_DEFINE([HAVE_GRE_CALC_HLEN])])
   OVS_GREP_IFELSE([$KSRC/include/net/gre.h], [ip_gre_calc_hlen],
   [OVS_DEFINE([HAVE_IP_GRE_CALC_HLEN])])
+  OVS_GREP_IFELSE([$KSRC/include/linux/rbtree.h], [rb_link_node_rcu],
+  [OVS_DEFINE([HAVE_RBTREE_RB_LINK_NODE_RCU])])
 
   if cmp -s datapath/linux/kcompat.h.new \
 datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index cbb29f1c69d0..69d7faeac414 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -116,5 +116,6 @@ openvswitch_headers += \
linux/compat/include/uapi/linux/netfilter.h \
linux/compat/include/linux/mm.h \
linux/compat/include/linux/netfilter.h \
-   linux/compat/include/linux/overflow.h
+   linux/compat/include/linux/overflow.h \
+   linux/compat/include/linux/rbtree.h
 EXTRA_DIST += linux/compat/build-aux/export-check-whitelist
diff --git a/datapath/linux/compat/include/linux/rbtree.h 
b/datapath/linux/compat/include/linux/rbtree.h
new file mode 100644
index ..dbf20ff0e0b8
--- /dev/null
+++ b/datapath/linux/compat/include/linux/rbtree.h
@@ -0,0 +1,19 @@
+#ifndef __LINUX_RBTREE_WRAPPER_H
+#define __LINUX_RBTREE_WRAPPER_H 1
+
+#include_next 
+
+#ifndef HAVE_RBTREE_RB_LINK_NODE_RCU
+#include 
+
+static inline void rb_link_node_rcu(struct rb_node *node, struct rb_node 
*parent,
+   struct rb_node **rb_link)
+{
+   node->__rb_parent_color = (unsigned long)parent;
+   node->rb_left = node->rb_right = NULL;
+
+   rcu_assign_pointer(*rb_link, node);
+}
+#endif
+
+#endif /* __LINUX_RBTREE_WRAPPER_H */
diff --git 

[ovs-dev] Видеокурс "Как воспитать ребёнка, чтобы он вырос богатым, счастливым и успешным лидером" - подробно и с практическими рекомендациями. 13_05_2019 02_03 198707

[Роман Колунов via dev]

ВИДЕОКУРС КАК ВОСПИТАТЬ РЕБЁНКА, ЧТОБЫ ОН ВЫРОС БОГАТЫМ, СЧАСТЛИВЫМ И УСПЕШНЫМ 
ЛИДЕРОМ
В помощь молодым родителям в воспитании у детей правильных убеждений и ценностей

Наверно нет такого родителя, который не хочет, чтобы его ребенок был самым 
умным, счастливым, здоровым и успешным. Каждый старается дать своему ребенку 
только самое лучшее. Но, бывает и так, что наши усилия не оправдывают себя и 
все результаты рушатся школьной системой или же ребенок, попадает в «плохую» 
компанию и попадает под "дурное влияние". Каким вырастет Ваш ребёнок – зависит 
только от Вас! Ребёнку необязательно расти в богатой семье, чтобы стать богатым 
и успешным. Ни Билл Гейтс, ни Стив Джобс, ни Марк Цукерберг и многие другие 
сильные мира сего не росли в богатых семьях. Но вашему ребенку обязательно 
нужно получить правильные убеждения и ценности. Чему учить, что рекомендовать и 
что советовать детям, чтобы они смогли зарабатывать значительные суммы денег - 
либо в бизнесе, либо на топовых должностях в хороших компаниях. И всё - своим 
умом, без помощи родственников. В предлагаемом видеокурсе, мы пошагово, 
максимально подробно и с практическими рекомендациями рассматриваем стратегии 
финансового успеха в двух направлениях - свой бизнес или крутая карьера. 
Рассматриваются все сопутствующие аспекты. Авторы курса - практикующие 
профессионалы: психологи, в том числе детские, коучи и тренеры бизнес-тематик. 
Кроме самого видеокурса, записано много полезной информации, в частности: 
учебные пособия, актуальные статьи, даны задания для проведения самостоятельных 
работ по изученным темам, а также рекомендации по их выполнению. В качестве 
дополнительного бонуса, записана аудио-версия курса, по всем темам, 
замечательная возможность закрепить изученный материал. Полученные в результате 
изучения видеокурса знания, позволят детям и Вам сэкономить уже в недалёком 
будущем много времени, сил, нервов и денег, так что не стоит упускать такую 
возможность.

Список тем рассматриваемых в курсе:

Кто и почему становится альфа-лидером в детском коллективе. Как научить вашего 
ребенка быть ведущим для других. Как воспитывать лидерские качества. Внутренняя 
сила лидера и твёрдый характер.

Как ребенку наработать стрессоустойчивость. Как научить ребенка управлять 
своими эмоциями. Техники влияния на людей, доступные вашему ребёнку. Как 
сформировать адекватно высокую самооценку у вашего ребенка. Как избежать 
дурного влияния и плохих компаний. Что вам нужно сделать, чтобы ребенок легко 
смог отказаться от предложенных наркотиков и правонарушений.

В чем разница между активным харизматичным лидером и "серым кардиналом". Как 
научить ребенка быть лидером и активного, и скрытого типа. Как обучать ребенка 
эффективным переговорам. Как ребенку перестать конфликтовать с учителями, но не 
уступить им при этом.

Как эффективно наработать умение ребенка. Ценности лидера - как передать 
ребенку этически правильные нормы морали. Какие знания и навыки необходимы 
вашему ребенку для успеха. Как все эти навыки отработать в детском возрасте и 
реализовать в карьере. Как максимально быстро и эффективно дать ему эти знания.

Как развивать эмоциональный интеллект вашего ребенка. Как ребенку научиться 
быть умным и знающим. Как научить ребенка эффективно действовать, достигать 
поставленные цели и быть успешным. Чему учить ребенка, чтобы он был успешен во 
взрослой жизни. Какие знания и навыки необходимы вашему ребенку для успеха. Как 
максимально быстро и эффективно дать ребёнку эти знания.

Как научить ребёнка не терять время зря. На что надо обратить внимание 
родителям для правильного формирования личности ребёнка. Как раскрыть потенциал 
вашего ребенка. Какие способы самостоятельного честного заработка есть у наших 
детей и подростков. Какие финансовые навыки надо сформировать, чтобы ваш 
ребенок никогда не нуждался в деньгах.

Карманные деньги. Зачем давать ребёнку карманные деньги. Сколько давать ребенку 
денег и почему столько. Стоит ли платить за хорошую учёбу? Платить ли ребёнку 
за выполнение работы по дому и хозяйству? Как уберечь ребенка от проблем с 
долгами и транжирством. Как правильно научить ребенка распоряжаться деньгами. 
Как у ребенка развить способность к самообразованию.

Как усилить познавательную активность ребенка. Как отдать ответственность за 
учёбу ребёнку. Как помочь вашему ребёнку быть успешным в школе и ВУЗе. Как 
помочь ребенку поверить в свои силы и способности.

Как сформировать адекватно высокую самооценку у вашего ребенка. Воспитание 
лидерских качеств у ребёнка. Формирование уверенности в своих силах. Как 
вызвать у подростка желание работать и зарабатывать самому вместо: «Мама, купи!»

Как вам стать уверенным в финансовом будущем вашего ребенка. Как воспитывать 
детей уверенными в себе. Как помочь ребенку стремиться к его мечтам. Как дать 
ребенку возможность учиться на ошибках и максимально использовать полученный 
опыт.

Как вырастить ребёнка богатым и успешным. Какие знания и навыки необходимо дать 
ребенку, чтобы он 

Re: [ovs-dev] [PATCH v4] Remove OVN.

[Zhou, Han via dev]

Remove the huge body of the email so that conversation can be easier through 
the email thread (

On 8/7/19, 3:06 PM, "ovs-dev-boun...@openvswitch.org on behalf of Han Zhou" 
 wrote:

Hi Mark,

Does it remove the ovsdb-cluster.at and related files? I think they are
important as part of OVSDB. Maybe we need to rewrite those tests without
OVN tools before removing OVN?

Thanks,
  Han
  
On Mon, Jul 29, 2019 at 12:43 PM Mark Michelson  wrote:

> OVN is separated into its own repo. This commit removes the OVN source,
> OVN tests, and OVN documentation. It also removes mentions of OVN from
> most documentation. The only place where OVN has been left is in
> changelogs/NEWS, since we shouldn't mess with the history of the
> project.
>
> Signed-off-by: Mark Michelson 
> ---
> v3 -> v4 Fixed 0-day Robot problems with NEWS and documentation
> v2 -> v3 Added NEWS entry for removal of OVN
> v1 -> v2 Rebase
> ---
>  Documentation/automake.mk  |12 -
>  Documentation/faq/index.rst| 1 -
>  Documentation/faq/ovn.rst  |90 -
>  Documentation/howto/docker.rst |   326 -
>  Documentation/howto/firewalld.rst  |   107 -
>  Documentation/howto/index.rst  | 9 -
>  Documentation/howto/openstack-containers.rst   |   135 -
>  Documentation/index.rst|20 +-
>  Documentation/intro/install/fedora.rst |12 -
>  Documentation/intro/install/index.rst  | 8 -
>  Documentation/intro/install/ovn-upgrades.rst   |   115 -
>  Documentation/ref/ovs-sim.1.rst|   100 -
>  Documentation/topics/high-availability.rst |   440 -
>  Documentation/topics/index.rst |18 +-
>  Documentation/topics/ovn-news-2.8.rst  |   278 -
>  Documentation/topics/role-based-access-control.rst |   101 -
>  Documentation/tutorials/index.rst  | 7 +-
>  Documentation/tutorials/ovn-ipsec.rst  |   146 -
>  Documentation/tutorials/ovn-openstack.rst  |  1922 ---
>  Documentation/tutorials/ovn-rbac.rst   |   134 -
>  Documentation/tutorials/ovn-sandbox.rst|   177 -
>  Makefile.am| 1 -
>  NEWS   | 5 +-
>  configure.ac   | 3 -
>  debian/.gitignore  | 5 -
>  debian/automake.mk |22 -
>  debian/control |78 -
>  debian/ovn-central.dirs| 1 -
>  debian/ovn-central.init|60 -
>  debian/ovn-central.install | 3 -
>  debian/ovn-central.manpages| 1 -
>  debian/ovn-central.postinst|49 -
>  debian/ovn-central.postrm  |48 -
>  debian/ovn-central.template| 5 -
>  debian/ovn-common.install  | 7 -
>  debian/ovn-common.manpages | 8 -
>  debian/ovn-common.postinst |24 -
>  debian/ovn-common.postrm   |23 -
>  debian/ovn-controller-vtep.init|54 -
>  debian/ovn-controller-vtep.install | 1 -
>  debian/ovn-controller-vtep.manpages| 1 -
>  debian/ovn-docker.install  | 2 -
>  debian/ovn-host.dirs   | 1 -
>  debian/ovn-host.init   |54 -
>  debian/ovn-host.install| 1 -
>  debian/ovn-host.manpages   | 1 -
>  debian/ovn-host.postinst   |49 -
>  debian/ovn-host.postrm |44 -
>  debian/ovn-host.template   | 5 -
>  debian/rules   | 6 -
>  include/automake.mk| 1 -
>  include/ovn/actions.h  |   622 -
>  include/ovn/automake.mk| 6 -
>  include/ovn/expr.h |   518 -
>  include/ovn/lex.h  |   152 -
>  include/ovn/logical-fields.h   |   130 -
>  lib/db-ctl-base.xml|12 +-
>  manpages.mk|28 -
>  ovn/.gitignore

Re: [ovs-dev] [PATCH v2 5/9] ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables

[Darrell Ball]

On Wed, Aug 7, 2019 at 11:51 AM Darrell Ball  wrote:

>
>
> On Wed, Aug 7, 2019 at 11:40 AM Darrell Ball  wrote:
>
>>
>>
>> On Tue, Aug 6, 2019 at 9:57 PM Justin Pettit  wrote:
>>
>>>
>>> > On Aug 5, 2019, at 8:07 PM, Darrell Ball  wrote:
>>> >
>>> > On Thu, Aug 1, 2019 at 3:10 PM Yi-Hung Wei 
>>> wrote:
>>> >
>>> >> +struct ct_timeout_policy {
>>> >> +struct uuid uuid;
>>> >> +unsigned int last_used_seqno;
>>> >> +struct ct_dpif_timeout_policy cdtp;
>>> >> +struct cmap_node node;  /* Element in struct
>>> dpif_backer's
>>> >> + * "ct_tps" cmap. */
>>> >>
>>> >
>>> >
>>> > This looks like a layering violation
>>> > should be in dpif-netlink or netlink-conntrack for kernel side
>>>
>>> Hi, Darrell.  As I mentioned in my code review, I had my own concerns
>>> about layering, but mine were from the top-down.  Yi-Hung and I didn't
>>> understand your concern here, since these seem to be structures that would
>>> be useful regardless of the implementation.  Can you explain a bit more
>>> about your layering concerns?
>>>
>>
>>
>> I was off yesterday afternoon.
>>
>> There are 3 behaviors with the patchset that are datapath specific
>>
>> 1/ Unwildcarding of commit flows with timeout policies
>> As we discussed, the userspace conntrack does not need to do this and
>> would not since it is suboptimal
>> since unnecessary flows are generated.
>> This is because userspace conntrack would use a single shared profile
>> across all dl_types and ip_proto
>> rather than expanding to 6 profiles as in the case of kernel across
>> dl_types and ip_protos.
>>
>> 2/ Userspace datepath/conntrack can easily manage cleanup of deleted
>> profiles using a refcount approach.
>> For userspace conntrack, we don't need to read all the timeouts
>> profiles continually and to continually try to
>> delete them from top down hoping to catch a window when the profile
>> is not referenced by a flow.
>>
>> 3/ In terms of timeout profile naming in userspace conntrack, we don't
>> need to manage a separate profile ID space for
>> userspace conntrack. We can simply use the uuid directly. This
>> simplifies the management of profiles and always
>>  keeps knowledge of the profile name in sync across layers.
>>
>
I think '3' is not that important for the userspace datapath, since when
vswitchd restarts we loose everything in the datapath
and will need to reallocate u32 profile IDs and re-associate them, anyways.
So we never loose the association per se.
Also, we would need to increase the size of a datastructure field to use
the uuid directly.

Also, '1' and '2' can be implemented later if it delays things too much. I
can submit a followup patch(es) if needed.


>
> Hence, the comments for this patch center around where the implementation
> code is.
> I think the code should live in datapath type specific code/files.
>
> Of course, wrappers are needed at higher layers to call the datapath
> specific implementations.
>
>
>>
>> Thanks Darrell
>>
>>
>>
>>
>>>
>>> Thanks,
>>>
>>> --Justin
>>>
>>>
>>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/9] ovs-vsctl: Add conntrack zone commands.

[William Tu]

Thanks for the review.

On Mon, Aug 05, 2019 at 04:12:02PM -0700, Darrell Ball wrote:
> Thanks for the patch
> 
> I noticed '--may-exist' and '--if-exists' are supported now for
> add--zone-tp/del-zone-tp - thanks
> The check for duplicate timeout policies now correctly checks all key and
> values - thanks

yes, thanks. Will do it in next version.
> 
> Some more comments inline
> I am trying to avoid duplicate comment from Justin, so I just won't comment
> on some parts in this version
> to avoid confusion.
> 
> 
> On Thu, Aug 1, 2019 at 3:09 PM Yi-Hung Wei  wrote:
> 
> > From: William Tu 
> >
> > The patch adds commands creating/deleting/listing conntrack zone
> > timeout policies:
> >   $ ovs-vsctl {add,del,list}-zone-tp zone=zone_id ...
> >
> > Signed-off-by: William Tu 
> > ---
> >  tests/ovs-vsctl.at   |  34 +++-
> >  utilities/ovs-vsctl.8.in |  25 ++
> >  utilities/ovs-vsctl.c| 204
> > +++
> >  3 files changed, 261 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> > index 46fa3c5b1a33..f0c5975edd0e 100644
> > --- a/tests/ovs-vsctl.at
> > +++ b/tests/ovs-vsctl.at
> > @@ -805,6 +805,20 @@ AT_CHECK(
> >[RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567
> > "'])])
> >  AT_CHECK(
> >[RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 --
> > set Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
> >
> 
> What happens if we add a datapath type here and there are no bridges of
> that type; meaning the datapath of that type does not even exist.
> Seems like a contradiction.
> Maybe we should check for that at least and raise an error.
> Ideally, it is better if these 'datapaths' are auto-managed by bridge
> creation/deletion with given datapath types,
> but we can certainly defer that.
> 
> 
> > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> > icmp_reply=2])])
> >
> 
> I mentioned this in V1:
> There is no filtering of bad timeout key; for example
> 
> AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> foo_bar=2])])
> 
> is accepted as valid
> 
> Even worse, a minor typo will go unnoticed - missing 'y' in 'icmp_reply'.
> 
> AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> icmp_repl=2])])

agree. 
> 
> 
> > +AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=1
> > icmp_first=1 icmp_reply=2])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> > Policies: icmp_first=1 icmp_reply=2
> >
> 
> I mentioned in V1
> We should check all possible timeout keys to make sure they work.

OK.
> 
> 
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> > icmp_reply=3])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> > Policies: icmp_first=1 icmp_reply=2
> > +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> > +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
> > +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout
> > Policies: icmp_first=2 icmp_reply=3
> > +])
> >  OVS_VSCTL_CLEANUP
> >  AT_CLEANUP
> >
> > @@ -890,10 +904,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0
> > flood_vlans=-1])],
> >  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
> >[1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid
> > range 0 to 4095 (inclusive)
> >  ])
> > -AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
> > +AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
> >
> 
> I mentioned in V1.
> I don't think we should make unrelated changes in a feature patch
> especially since it seems the author wanted to convey
> short form syntax is valid

This has to be there, otherwise test fails.
> 
> 
> >[1], [], [[ovs-vsctl: constraint violation: xyz is not one of the
> > allowed values ([in-band, out-of-band])
> >  ]])
> > -AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
> > +AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
> >[1], [], [ovs-vsctl: cannot specify key to set for non-map column
> > connection_mode
> >  ])
> >  AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
> > @@ -929,6 +943,22 @@ AT_CHECK([RUN_OVS_VSCTL([remove bridge br1
> > flood-vlans true])],
> >  AT_CHECK([RUN_OVS_VSCTL([clear bridge br1 name])],
> >[1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge
> >  ])
> > +
> > +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 --
> > set Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
> > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1 icmp_first=1
> > icmp_reply=2])],
> > +  [1], [], [ovs-vsctl: datapath: netdevxx record not found
> > +])
> > +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 

Re: [ovs-dev] memory leak in internal_dev_create

[Pravin Shelar]

On Tue, Aug 6, 2019 at 5:00 AM Hillf Danton  wrote:
>
>
> On Tue, 06 Aug 2019 01:58:05 -0700
> > Hello,
> >
> > syzbot found the following crash on:
> >

...
> > BUG: memory leak
> > unreferenced object 0x8881228ca500 (size 128):
> >comm "syz-executor032", pid 7015, jiffies 4294944622 (age 7.880s)
> >hex dump (first 32 bytes):
> >  00 f0 27 18 81 88 ff ff 80 ac 8c 22 81 88 ff ff  ..'"
> >  40 b7 23 17 81 88 ff ff 00 00 00 00 00 00 00 00  @.#.
> >backtrace:
> >  [<0eb78212>] kmemleak_alloc_recursive  
> > include/linux/kmemleak.h:43 [inline]
> >  [<0eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline]
> >  [<0eb78212>] slab_alloc mm/slab.c:3319 [inline]
> >  [<0eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
> >  [<006ea6c6>] kmalloc include/linux/slab.h:552 [inline]
> >  [<006ea6c6>] kzalloc include/linux/slab.h:748 [inline]
> >  [<006ea6c6>] ovs_vport_alloc+0x37/0xf0  
> > net/openvswitch/vport.c:130
> >  [] internal_dev_create+0x24/0x1d0  
> > net/openvswitch/vport-internal_dev.c:164
> >  [<56ee7c13>] ovs_vport_add+0x81/0x190  
> > net/openvswitch/vport.c:199
> >  [<5434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194
> >  [] ovs_dp_cmd_new+0x22f/0x410  
> > net/openvswitch/datapath.c:1614
> >  [] genl_family_rcv_msg+0x2ab/0x5b0  
> > net/netlink/genetlink.c:629
> >  [] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654
> >  [<6694b647>] netlink_rcv_skb+0x61/0x170  
> > net/netlink/af_netlink.c:2477
> >  [<88381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
> >  [] netlink_unicast_kernel  
> > net/netlink/af_netlink.c:1302 [inline]
> >  [] netlink_unicast+0x1ec/0x2d0  
> > net/netlink/af_netlink.c:1328
> >  [<67e6b079>] netlink_sendmsg+0x270/0x480  
> > net/netlink/af_netlink.c:1917
> >  [] sock_sendmsg_nosec net/socket.c:637 [inline]
> >  [] sock_sendmsg+0x54/0x70 net/socket.c:657
> >  [<4cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311
> >  [] __sys_sendmsg+0x80/0xf0 net/socket.c:2356
> >  [] __do_sys_sendmsg net/socket.c:2365 [inline]
> >  [] __se_sys_sendmsg net/socket.c:2363 [inline]
> >  [] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363
>
>
> Always free vport manually unless register_netdevice() succeeds.
>
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -137,7 +137,7 @@ static void do_setup(struct net_device *
> netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
>   IFF_NO_QUEUE;
> netdev->needs_free_netdev = true;
> -   netdev->priv_destructor = internal_dev_destructor;
> +   netdev->priv_destructor = NULL;
> netdev->ethtool_ops = _dev_ethtool_ops;
> netdev->rtnl_link_ops = _dev_link_ops;
>
> @@ -159,7 +159,6 @@ static struct vport *internal_dev_create
> struct internal_dev *internal_dev;
> struct net_device *dev;
> int err;
> -   bool free_vport = true;
>
> vport = ovs_vport_alloc(0, _internal_vport_ops, parms);
> if (IS_ERR(vport)) {
> @@ -190,10 +189,9 @@ static struct vport *internal_dev_create
>
> rtnl_lock();
> err = register_netdevice(vport->dev);
> -   if (err) {
> -   free_vport = false;
> +   if (err)
> goto error_unlock;
> -   }
> +   vport->dev->priv_destructor = internal_dev_destructor;
>
I am not sure why have you moved this assignment out of do_setup().

Otherwise patch looks good to me.

Thanks.
> dev_set_promiscuity(vport->dev, 1);
> rtnl_unlock();
> @@ -207,8 +205,7 @@ error_unlock:
>  error_free_netdev:
> free_netdev(dev);
>  error_free_vport:
> -   if (free_vport)
> -   ovs_vport_free(vport);
> +   ovs_vport_free(vport);
>  error:
> return ERR_PTR(err);
>  }
> --
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/9] ovs-vsctl: Add conntrack zone commands.

[William Tu]

On Fri, Aug 02, 2019 at 05:42:16PM -0700, Justin Pettit wrote:
> 
> > On Aug 1, 2019, at 3:07 PM, Yi-Hung Wei  wrote:
> > 
> > From: William Tu 
> > 
> > The patch adds commands creating/deleting/listing conntrack zone
> > timeout policies:
> >  $ ovs-vsctl {add,del,list}-zone-tp zone=zone_id ...
> 
> I think the command also requires a datapath argument.

Thanks, I will fix it in next version.

> 
> > Signed-off-by: William Tu 
> > ---
> > tests/ovs-vsctl.at   |  34 +++-
> > utilities/ovs-vsctl.8.in |  25 ++
> > utilities/ovs-vsctl.c| 204 
> > +++
> > 3 files changed, 261 insertions(+), 2 deletions(-)
> 
> > diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> > index 7c09df79bd29..0925d4d97b39 100644
> > --- a/utilities/ovs-vsctl.8.in
> > +++ b/utilities/ovs-vsctl.8.in
> > @@ -353,6 +353,31 @@ list.
> > Prints the name of the bridge that contains \fIiface\fR on standard
> > output.
> > .
> > +.SS "Conntrack Zone Commands"
> > +These commands query and modify datapath CT zones and Timeout Policies.
> > +.
> > +.IP "\fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id \fIpolicies\fR"
> > +Creates a conntrack zone with \fIzone_id\fR under the datapath 
> > \fIdatapath\fR.
> > +Associate the conntrack timeout policies to it by a list of
> > +\fIkey\fB=\fIvalue\fR pairs, separeted by space.  For example, specifying
> > +30-second timeout policy for first icmp packet, and 60-second for icmp 
> > reply packet
> > +by doing \fBicmp_first=30 icmp_reply=60\fR.  See CT_Timeout_Policy TABLE
> > +at \fBovs-vswitchd.conf.db\fR(5) for all available configurations.
> > +.IP
> > +Without \fB\-\-may\-exist\fR, attempting to add a \fIzone_id\fR that
> > +already exists is an error.  With \fB\-\-may\-exist\fR,
> > +this command does nothing if \fIzone_id\fR is already created\fR.
> > +.
> > +.IP "\fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR"
> > +Delete a zone under \fIdatapath\fR by specifying its zone ID.
> > +.IP
> > +Without \fB\-\-if\-exists\fR, attempting to delete a zone that
> > +does not exist is an error.  With \fB\-\-if\-exists\fR, attempting to
> > +delete a zone that does not exist has no effect.
> 
> I think "--may-exist" and "--if-exists" should be included in the line 
> describing the command.  I have a few other suggested changes in the attached 
> patch.
> 
> > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> > index 4948137efe8c..16578edfc331 100644
> > --- a/utilities/ovs-vsctl.c
> > +++ b/utilities/ovs-vsctl.c
> > 
> > +static struct ovsrec_ct_timeout_policy *
> > +create_timeout_policy(struct ctl_context *ctx, char **argv, int n_tps)
> > +{
> > +const struct ovsrec_ct_timeout_policy_table *tp_table;
> > +const struct ovsrec_ct_timeout_policy *row;
> > +struct ovsrec_ct_timeout_policy *tp = NULL;
> > +const char **key_timeouts;
> > +int64_t *value_timeouts;
> > +struct simap new_tp, s;
> > +int i, j;
> 
> I think 'i' and 'j' can be declared in the for loop definitions (and you 
> don't actually need 'j' since it's not nested).
> 
> > +/* Parse timeout arguments. */
> > +for (i = 0; i < n_tps; i++) {
> > +char *key, *value, *pos, *copy;
> 
> 'copy' doesn't appear to be used.
> 
> > +
> > +pos = copy = xstrdup(argv[i]);
> 
> I think 'pos' is leaked.  I had to go through some contortions to make it 
> work without modifying 'argv'; let me know if you think the logic in the 
> attached diff is correct.
> 
> > +free(key_timeouts);
> > +free(value_timeouts);
> > +return tp;
> 
> I think you also need to destroy 'new_tp'.
> 
> > +static void
> > +cmd_add_zone(struct ctl_context *ctx)
> 
> I think these functions might be more accurately described with "_tp" at the 
> end.
> 
> > +{
> > +struct vsctl_context *vsctl_ctx = vsctl_context_cast(ctx);
> > +struct ovsrec_ct_timeout_policy *tp;
> > +struct ovsrec_ct_zone *zone;
> > +struct ovsrec_datapath *dp;
> > +const char *dp_name;
> > +int64_t zone_id;
> > +bool may_exist;
> > +int n_tps;
> 
> Minor style nit, but we usually declare the variables closer to their use 
> now.  I've updated some of them in the attached diff.
> 
> > +dp_name = ctx->argv[1];
> > +ovs_scan(ctx->argv[2], "zone=%"SCNi64, _id);
> > +may_exist = shash_find(>options, "--may-exist") != NULL;
> > +
> > +dp = find_datapath(vsctl_ctx, dp_name);
> > +if (!dp) {
> > +ctl_fatal("datapath: %s record not found", dp_name);
> 
> This error message is a bit out of style, the other code usually is more like 
> "datapath %s does not exit".
> 
> > +return;
> 
> I don't think you need to return after a call to ctl_fatal().
> 
> > +zone = find_ct_zone(dp, zone_id);
> > +if (zone) {
> > +if (!may_exist) {
> > +ctl_fatal("zone id %"PRIu64" alread exists", zone_id);
> 
> s/alread/already/
> 
> > +static void
> > +cmd_del_zone(struct ctl_context *ctx)
> > +{
> > +

Re: [ovs-dev] [PATCH v2 5/9] ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables

[Darrell Ball]

On Wed, Aug 7, 2019 at 11:40 AM Darrell Ball  wrote:

>
>
> On Tue, Aug 6, 2019 at 9:57 PM Justin Pettit  wrote:
>
>>
>> > On Aug 5, 2019, at 8:07 PM, Darrell Ball  wrote:
>> >
>> > On Thu, Aug 1, 2019 at 3:10 PM Yi-Hung Wei 
>> wrote:
>> >
>> >> +struct ct_timeout_policy {
>> >> +struct uuid uuid;
>> >> +unsigned int last_used_seqno;
>> >> +struct ct_dpif_timeout_policy cdtp;
>> >> +struct cmap_node node;  /* Element in struct dpif_backer's
>> >> + * "ct_tps" cmap. */
>> >>
>> >
>> >
>> > This looks like a layering violation
>> > should be in dpif-netlink or netlink-conntrack for kernel side
>>
>> Hi, Darrell.  As I mentioned in my code review, I had my own concerns
>> about layering, but mine were from the top-down.  Yi-Hung and I didn't
>> understand your concern here, since these seem to be structures that would
>> be useful regardless of the implementation.  Can you explain a bit more
>> about your layering concerns?
>>
>
>
> I was off yesterday afternoon.
>
> There are 3 behaviors with the patchset that are datapath specific
>
> 1/ Unwildcarding of commit flows with timeout policies
> As we discussed, the userspace conntrack does not need to do this and
> would not since it is suboptimal
> since unnecessary flows are generated.
> This is because userspace conntrack would use a single shared profile
> across all dl_types and ip_proto
> rather than expanding to 6 profiles as in the case of kernel across
> dl_types and ip_protos.
>
> 2/ Userspace datepath/conntrack can easily manage cleanup of deleted
> profiles using a refcount approach.
> For userspace conntrack, we don't need to read all the timeouts
> profiles continually and to continually try to
> delete them from top down hoping to catch a window when the profile is
> not referenced by a flow.
>
> 3/ In terms of timeout profile naming in userspace conntrack, we don't
> need to manage a separate profile ID space for
> userspace conntrack. We can simply use the uuid directly. This
> simplifies the management of profiles and always
>  keeps knowledge of the profile name in sync across layers.
>

Hence, the comments for this patch center around where the implementation
code is.
I think the code should live in datapath type specific code/files.

Of course, wrappers are needed at higher layers to call the datapath
specific implementations.


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

Re: [ovs-dev] [PATCH v2 5/9] ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables

[Darrell Ball]

On Tue, Aug 6, 2019 at 9:57 PM Justin Pettit  wrote:

>
> > On Aug 5, 2019, at 8:07 PM, Darrell Ball  wrote:
> >
> > On Thu, Aug 1, 2019 at 3:10 PM Yi-Hung Wei  wrote:
> >
> >> +struct ct_timeout_policy {
> >> +struct uuid uuid;
> >> +unsigned int last_used_seqno;
> >> +struct ct_dpif_timeout_policy cdtp;
> >> +struct cmap_node node;  /* Element in struct dpif_backer's
> >> + * "ct_tps" cmap. */
> >>
> >
> >
> > This looks like a layering violation
> > should be in dpif-netlink or netlink-conntrack for kernel side
>
> Hi, Darrell.  As I mentioned in my code review, I had my own concerns
> about layering, but mine were from the top-down.  Yi-Hung and I didn't
> understand your concern here, since these seem to be structures that would
> be useful regardless of the implementation.  Can you explain a bit more
> about your layering concerns?
>


I was off yesterday afternoon.

There are 3 behaviors with the patchset that are datapath specific

1/ Unwildcarding of commit flows with timeout policies
As we discussed, the userspace conntrack does not need to do this and
would not since it is suboptimal
since unnecessary flows are generated.
This is because userspace conntrack would use a single shared profile
across all dl_types and ip_proto
rather than expanding to 6 profiles as in the case of kernel across
dl_types and ip_protos.

2/ Userspace datepath/conntrack can easily manage cleanup of deleted
profiles using a refcount approach.
For userspace conntrack, we don't need to read all the timeouts
profiles continually and to continually try to
delete them from top down hoping to catch a window when the profile is
not referenced by a flow.

3/ In terms of timeout profile naming in userspace conntrack, we don't need
to manage a separate profile ID space for
userspace conntrack. We can simply use the uuid directly. This
simplifies the management of profiles and always
 keeps knowledge of the profile name in sync across layers.

Thanks Darrell




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

Re: [ovs-dev] [PATCH ovn] Enable OVN in tutorial/ovs-sandbox by default

[Numan Siddique]

On Wed, Aug 7, 2019 at 11:27 PM Mark Michelson  wrote:

> Acked-by: Mark Michelson 
>
>
Thanks for the review. I applied this to master.

Numan


> On 8/7/19 11:06 AM, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > The patch removs the --ovn option and enables OVN by default.
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   tutorial/ovs-sandbox | 6 +-
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
> > index 9b4c3e4f5..47032b499 100755
> > --- a/tutorial/ovs-sandbox
> > +++ b/tutorial/ovs-sandbox
> > @@ -67,7 +67,7 @@ srcdir=
> >   schema=
> >   installed=false
> >   built=false
> > -ovn=false
> > +ovn=true
> >   ovnsb_schema=
> >   ovnnb_schema=
> >   ovn_rbac=true
> > @@ -129,7 +129,6 @@ General options:
> > -S, --schema=FILEuse FILE as vswitch.ovsschema
> >
> >   OVN options:
> > -  -o, --ovnenable OVN
> > --no-ovn-rbacdisable role-based access control for OVN
> > --n-northds=NUMBER   run NUMBER copies of northd (default: 1)
> > --nbdb-model=standalone|backup|clusterednorthbound database model
> > @@ -201,9 +200,6 @@ EOF
> >   --gdb-ovn-controller-vtep)
> >   gdb_ovn_controller_vtep=true
> >   ;;
> > --o|--ovn)
> > -ovn=true
> > -;;
> >   --no-ovn-rbac)
> >   ovn_rbac=false
> >   ;;
> >
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] Enable OVN in tutorial/ovs-sandbox by default

[Mark Michelson]

Acked-by: Mark Michelson 

On 8/7/19 11:06 AM, nusid...@redhat.com wrote:

From: Numan Siddique 

The patch removs the --ovn option and enables OVN by default.

Signed-off-by: Numan Siddique 
---
  tutorial/ovs-sandbox | 6 +-
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index 9b4c3e4f5..47032b499 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -67,7 +67,7 @@ srcdir=
  schema=
  installed=false
  built=false
-ovn=false
+ovn=true
  ovnsb_schema=
  ovnnb_schema=
  ovn_rbac=true
@@ -129,7 +129,6 @@ General options:
-S, --schema=FILEuse FILE as vswitch.ovsschema
  
  OVN options:

-  -o, --ovnenable OVN
--no-ovn-rbacdisable role-based access control for OVN
--n-northds=NUMBER   run NUMBER copies of northd (default: 1)
--nbdb-model=standalone|backup|clusterednorthbound database model
@@ -201,9 +200,6 @@ EOF
  --gdb-ovn-controller-vtep)
  gdb_ovn_controller_vtep=true
  ;;
--o|--ovn)
-ovn=true
-;;
  --no-ovn-rbac)
  ovn_rbac=false
  ;;



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

Re: [ovs-dev] [PATCH v2] datapath: compat: Backports bugfixes for nf_conncount

[Yi-Hung Wei]

On Tue, Aug 6, 2019 at 5:06 PM Yifeng Sun  wrote:
>
> This patch backports several critical bug fixes related to
> locking and data consistency in nf_conncount code.
>
> This backport is based on the following upstream net-next upstream commits.
> 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is negative")
> d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of 
> rb_link_node()")
> 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.")
> 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
>
> This patch also added additional compat code so that it can build on
> all supported kernel versions.
>
> Travis tests are at
> https://travis-ci.org/yifsun/ovs-travis/builds/568603796
>
> VMware-BZ: #2396471
>
> CC: Taehee Yoo 
> Signed-off-by: Yifeng Sun 
> ---

Hi Yifeng,

Thanks for the patch. This backport looks good in general.

I found that there are couple of fixes on nf_conncount in upstream
kernel.  Since we would like to use df4a90250976 ("netfilter:
nf_conncount: merge lookup and add functions") to determine if the
kernel has all the fixes. It would be good to bring other commits on
the same date to our datapath compat code base as well. That is to
squash all the following commits.

One more suggestion is to squash the other patch that you sent
("datapath: Apply bug fixes of nf_conncount for different kernel
versions") into this one, since updating "HAVE_UPSTREAM_NF_CONNCOUNT"
is only useful with this patch.


a007232066f6 ("netfilter: nf_conncount: fix argument order to find_next_bit")
c80f10bc973a ("netfilter: nf_conncount: speculative garbage collection
on empty lists")
2f971a8f4255 ("netfilter: nf_conncount: move all list iterations under
spinlock")
df4a90250976 ("netfilter: nf_conncount: merge lookup and add functions")
e8cfb372b38a ("netfilter: nf_conncount: restart search when nodes have
been erased")
f7fcc98dfc2d ("netfilter: nf_conncount: split gc in two phases")
4cd273bb91b3 ("netfilter: nf_conncount: don't skip eviction when age
is negative")
c78e7818f16f ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS
with CONNCOUNT_SLOTS")
d4e7df16567b ("netfilter: nf_conncount: use rb_link_node_rcu() instead
of rb_link_node()")
53ca0f2fec39 ("netfilter: nf_conncount: remove wrong condition check routine")
3c5cdb17c3be ("netfilter: nf_conncount: fix unexpected permanent node of list.")
31568ec09ea0 ("netfilter: nf_conncount: fix list_del corruption in conn_free")
fd3e71a9f71e ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")

Thanks,

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

[ovs-dev] OVS-DPDK public meeting 7th August

[Stokes, Ian]

Hi all,

I don’t see anything on the agenda for today’s OVS DPDK community sync meeting. 
Unless I hear otherwise I propose we cancel today’s meeting and instead meet in 
two weeks when we’re closer to the release?

If there are any issues for the agenda today please let me know and I’ll be 
happy to facilitate the call today, otherwise I think it’s safe to cancel as 
there does not seem to be much to be discussed.

Regards
Ian

  -Original Appointment-
  From: ktray...@redhat.com [mailto:ktray...@redhat.com]
  Sent: Friday, June 28, 2019 12:34 PM
  To: ktray...@redhat.com; Giller, Robin; mcr...@redhat.com; Ferriter, 
Cian; chihc...@yahoo.com; Zongwei Zhou; Murali R; niazk...@vmware.com; 
indra...@chelsio.com; i.maxim...@samsung.com; acon...@redhat.com; 
m...@napatech.com; frikkie.scho...@netronome.com; Weirong Jiang; 
od...@mellanox.com; wz...@marvell.com; srinivas chowdhary Jampala; Wang, 
Yipeng1; therb...@redhat.com; as...@mellanox.com; Wang, Ren; 
fhal...@redhat.com; karen.schr...@broadcom.com; Bie, Tiwei; 
jorge.garcia_rodrig...@nokia.com; Bhavesh Davda; 
georg.schmueck...@ericsson.com; don.wallw...@broadcom.com; 
pieter.jansenvanvuu...@netronome.com; William Tu; ke...@bitperfect.org; 
echau...@redhat.com; ro...@mellanox.com; dmarc...@redhat.com; Gobriel, Sameh; 
Liew, Irene; simon.hor...@netronome.com; fleit...@redhat.com; 
jason.vanaa...@netronome.com; scott.sm...@broadcom.com; 
frederick.bo...@netronome.com; treda...@redhat.com; ys...@mellanox.com; 
pshe...@ovn.org; sriram.narasim...@hpe.com; tho...@monjalon.net; satish 
kondapalli; ol...@mellanox.com; f...@napatech.com; Loftus, Ciara; Mark 
Guinther; rk...@redhat.com; ralo...@redhat.com; Vinod Chegu; 
romain.leng...@berabera.info; Alejandro Lucero; jan.scheur...@ericsson.com; 
peter.suskov...@ericsson.com; jpettit@gmail.com; Christian Ehrhardt; 
Muhammad Shahbaz; b...@ovn.org; db...@vmware.com; pw...@hotmail.com; Stokes, 
Ian; Gregory Elkinbard; Love, Robert W; Carl Tung; 
mattias.g.johans...@ericsson.com; john.hur...@netronome.com; 
er...@mellanox.com; n...@mellanox.com; hemant.agra...@nxp.com; 
manish.awas...@cavium.com; jtons...@netronome.com; nick.vilj...@netronome.com; 
bobdu...@chelsio.com; atrag...@redhat.com
  Subject: Updated invitation with note: OVS-DPDK public meeting @ Every 2 
weeks from 5pm to 6pm on Wednesday from Wed Jul 10 to Wed Nov 27 (IST) 
(ian.sto...@intel.com)
  When: Wednesday, August 7, 2019 5:00 PM-6:00 PM Europe/Dublin.
  Where: https://bluejeans.com/139318596


This event has been changed with this note:
"Restoring the July 10th meeting"
more details 
»

OVS-DPDK public meeting
When
Changed: Every 2 weeks from 5pm to 6pm on Wednesday from Wed Jul 10 to 
Wed Nov 27 Ireland Time

Where
https://bluejeans.com/139318596 
(map)

Calendar
ian.sto...@intel.com

Who
•
ktray...@redhat.com - organizer

•
robin.gil...@intel.com

•
mcr...@redhat.com

•
cian.ferri...@intel.com

•
chihc...@yahoo.com

•
Zongwei Zhou

•
Murali R

•
niazk...@vmware.com

•
indra...@chelsio.com

•
i.maxim...@samsung.com

•
acon...@redhat.com

•
m...@napatech.com

•
frikkie.scho...@netronome.com

•
Weirong Jiang

•
od...@mellanox.com

•
wz...@marvell.com

•
srinivas chowdhary Jampala

•
yipeng1.w...@intel.com

•
therb...@redhat.com

•
as...@mellanox.com

•
ren.w...@intel.com

•
fhal...@redhat.com

•
karen.schr...@broadcom.com

•
tiwei@intel.com

•
jorge.garcia_rodrig...@nokia.com

•
Bhavesh Davda

•
georg.schmueck...@ericsson.com

•
don.wallw...@broadcom.com

•
pieter.jansenvanvuu...@netronome.com

•
William Tu

•
ke...@bitperfect.org

•
echau...@redhat.com

•
ro...@mellanox.com

•
dmarc...@redhat.com

•
sameh.gobr...@intel.com

•
irene.l...@intel.com

•
simon.hor...@netronome.com

•
fleit...@redhat.com

•
jason.vanaa...@netronome.com

•
scott.sm...@broadcom.com

•
frederick.bo...@netronome.com

•
treda...@redhat.com

•
ys...@mellanox.com

•
pshe...@ovn.org

•
sriram.narasim...@hpe.com

•
tho...@monjalon.net

•
satish kondapalli

•
ol...@mellanox.com

•
f...@napatech.com

•
ciara.lof...@intel.com

•
Mark Guinther

•
rk...@redhat.com

•
ralo...@redhat.com

•
Vinod Chegu

•
 

Re: [ovs-dev] [PATCH ovn v2] OVN: Fix learning of neighbors from ARP/ND packets.

[Dumitru Ceara]

On Mon, Aug 5, 2019 at 5:34 PM Han Zhou  wrote:
>
>
>
> On Mon, Aug 5, 2019 at 1:06 AM Dumitru Ceara  wrote:
> >
> > On Mon, Aug 5, 2019 at 1:41 AM Han Zhou  wrote:
> > >
> > >
> > >
> > > On Mon, Jul 29, 2019 at 2:30 AM Dumitru Ceara  wrote:
> > > >
> > > > Add a restriction on the target protocol addresses to match the
> > > > configured subnets. All other ARP/ND packets are invalid in this
> > > > context.
> > > >
> > > > One exception is for ARP replies that are received for route next-hops
> > > > that are only reachable via a port but can't be directly resolved
> > > > through route lookups. Such support was introduced by commit:
> > > >
> > > > 6b785fd8fe29 ("ovn-util: Allow /32 IP addresses for router ports.")
> > > >
> > > > Reported-at: https://bugzilla.redhat.com/1729846
> > > > Reported-by: Haidong Li 
> > > > CC: Han Zhou 
> > > > CC: Guru Shetty 
> > > > Fixes: b068454082f5 ("ovn-northd: Support learning neighbor from ARP 
> > > > request.")
> > > > Signed-off-by: Dumitru Ceara 
> > >
> > > Hi Dumitru,
> >
> > Hi Han,
> >
> > Thanks for reviewing this.
> >
> > >
> > > Sorry for my slow response, and thanks a lot for revising the patch for a 
> > > bigger scope of validations. However, the exception of /32 address makes 
> > > me thinking more about this patch. If ARP replies is allowed from any 
> > > nexthop for a LR port with /32, at least ARP request for GARP purpose 
> > > should also be allowed. But before asking for a v3, I'd hold on to 
> > > rethink the purpose of this patch.
> >
> > Right, we should allow GARP requests too. If we decide to go ahead
> > with this patch I'll add a function in v3 to handle all types of ARPs
> > and call it both for unreachable static route next-hops and attached
> > subnets.
> >
> > >
> > > The nexthop specific flows are now from static routes. What if OVN 
> > > support dynamical routing protocols in the future? Of course we can then 
> > > take those dynamic nexthops into allowed peers. But then I am thinking 
> > > what is the real benefit of all these restrictions? Why can't we just 
> > > have simpler logic to handle all these situations without validation? I 
> > > think the major benefit of the validation is to avoid handling the noise 
> > > ARP/NDs when multiple subnets shares same L2, but most cases it is really 
> > > not a big deal, right? For the CPU problem caused by ARP flooding as 
> > > mentioned by the bug report, it is a real problem, but this patch seems 
> > > not really helpful for that, because the attacker can just trigger the 
> > > same CPU problem with *valid* packets. So I am not sure if the benefit of 
> > > the change is worth the complexity it introduced. Please share your 
> > > thought and correct me if I missed something.
> > >
> > > Thanks,
> > > Han
> >
> > I assume the simpler logic to handle all these situations without
> > validation is to add rate limiting for ARP packets that get punted to
> > the controller. I agree that this should be implemented too.
> >
> > But I think rate limiting all ARP packets regardless of IP addresses
> > is not enough. In the following scenario and if we would have a way to
> > rate limit ARP packets:
> > - Subnet 42.42.42.0/24 configured on the OVN
> > - "Invalid" ARP packets are injected at high rate in the network for 
> > 41.41.41.41
> > - Host 42.42.42.42 sends GARP.
> > - Rate limiting of ARP packets towards controller at 100pps
> >
> > With the current code, ARP packets for 41.41.41.41 will be punted to
> > controller at a rate of at most 100 per second. But the chances that
> > the valid 42.42.42.42 GARP is dropped is really high.
> >
> > If we instead use the following approach:
> > a. Punt only useful ARPs to controller.
> > b. Rate limit ARPs that are sent to controller.
> >
> > Then ARP packets outside 42.42.42./24 are never punted to the
> > controller and don't consume any rate limiting tokens. For the second
> > case, when an attacker would flood with valid ARP packets we would
> > have the rate limit in place to protect the controller CPU.
> >
> > My commit addresses point "a" above as I think point "b" should be
> > done in a generic way for all protocol packets that need to reach the
> > controller.
> >
> > For dynamic routing protocols on the other hand I think we should not
> > install routes with next-hops that are unreachable through other
> > direct or indirect routes.
> >
> > Thanks,
> > Dumitru
>
> I agree that blindly ARP rate limit is not helpful, but a) is not really 
> helpful in this case either. In your example, the attacker can just use any 
> valid IP in 42.42.42.0/24 to send GARP flooding, which would result in 
> exactly same result that a useful GARP from 42.42.42.42 is dropped because of 
> blindly rate limiting all ARPs. To solve the problem properly, the ARP rate 
> limiting must be done per IP.

Ok, ideally ARP rate limiting should be done per IP but it would take
quite a lot of resources to keep that information per host.

Any idea how to 

[ovs-dev] [PATCH ovn] Enable OVN in tutorial/ovs-sandbox by default

[nusiddiq]

From: Numan Siddique 

The patch removs the --ovn option and enables OVN by default.

Signed-off-by: Numan Siddique 
---
 tutorial/ovs-sandbox | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index 9b4c3e4f5..47032b499 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -67,7 +67,7 @@ srcdir=
 schema=
 installed=false
 built=false
-ovn=false
+ovn=true
 ovnsb_schema=
 ovnnb_schema=
 ovn_rbac=true
@@ -129,7 +129,6 @@ General options:
   -S, --schema=FILEuse FILE as vswitch.ovsschema
 
 OVN options:
-  -o, --ovnenable OVN
   --no-ovn-rbacdisable role-based access control for OVN
   --n-northds=NUMBER   run NUMBER copies of northd (default: 1)
   --nbdb-model=standalone|backup|clusterednorthbound database model
@@ -201,9 +200,6 @@ EOF
 --gdb-ovn-controller-vtep)
 gdb_ovn_controller_vtep=true
 ;;
--o|--ovn)
-ovn=true
-;;
 --no-ovn-rbac)
 ovn_rbac=false
 ;;
-- 
2.21.0

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

Re: [ovs-dev] 2.12.0 branch snapshot - test 2618 failure on s390x

[Numan Siddique]

On Wed, Aug 7, 2019 at 4:20 PM James Page  wrote:

> Just tripped on this one whilst building for s390x on Ubuntu:
>
> ##  ##
>
> ## Summary of the failures. ##
> ##  ##
> Failed tests:
> openvswitch 2.12.0 test suite test groups:
>
>  NUM: FILE-NAME:LINE TEST-GROUP-NAME
>   KEYWORDS
>
>  2618: ovn.at:789 ovn -- action parsing
>
> ## -- ##
> ## Detailed failed tests. ##
> ## -- ##
>
> # -*- compilation -*-
> 2618. ovn.at:789: testing ovn -- action parsing ...
> ../../tests/ovn.at:1403: ovstest test-ovn parse-actions < input.txt
> --- expout  2019-08-07 10:40:15.539670085 +
> +++ /<>/_debian/tests/testsuite.dir/at-groups/2618/stdout
> 2019-08-07
> 10:40:15.539670085 +
> @@ -579,7 +579,7 @@
>  # bind_vport
>  # lsp1's port key is 0x11.
>  bind_vport("lsp1", inport);
> -encodes as controller(userdata=00.00.00.11.00.00.00.00.11.00.00.00)
> +encodes as controller(userdata=00.00.00.11.00.00.00.00.00.00.00.11)
>  # lsp2 doesn't exist. So it should be encoded as drop.
>  bind_vport("lsp2", inport);
>  encodes as drop
> 2618. ovn.at:789: 2618. ovn -- action parsing (ovn.at:789): FAILED (
> ovn.at:1403)
>
> Looks like something endian-y (s390x is the only BE arch in Ubuntu).
>

Hi James,

This patch https://patchwork.ozlabs.org/patch/1142628/ fixes the issue.

@Guru - Can you please take  a look at this patch and apply to branch 2.12.

Thanks
Numan

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

Re: [ovs-dev] [PATCH 1/1] ovn-northd: fix memory leak in add_distributed_nat_routes() function

[Numan Siddique]

On Wed, Aug 7, 2019 at 4:59 PM Damijan Skvarc 
wrote:

> Within this function actions & match dynamic strings are used as helper
> variables for adding entries into logical flow table. Variables are
> used several times in order to optimize number of memory allocations,
> however at the end memory was forgotten to be deallocated.
>
> Signed-off-by: Damijan Skvarc 
>

Hi Damijan,

This patch is already applied to ovn master repo a couple of days back -
https://github.com/ovn-org/ovn/commit/c1ba3f68a78af3b852aa5709a0d96f001b63b243

Would you please submit the patch to OVS branch 2.12 so that it can be
backported  (with the subject prefix - "branch 2.12")

Thanks
Numan



> ---
>  northd/ovn-northd.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 979dea4..8588961 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5667,6 +5667,8 @@ add_distributed_nat_routes(struct hmap *lflows,
> const struct ovn_port *op)
>  ds_clear();
>  }
>  }
> +ds_destroy();
> +ds_destroy();
>  }
>
>  static void
> --
> 2.7.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 ovn] logical-fields: fix memory leak while initializing ovnfield_by_name

[0-day Robot]

Bleep bloop.  Greetings Damijan Skvarc, 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:
Failed to merge in the changes.
Patch failed at 0001 logical-fields: fix memory leak while initializing 
ovnfield_by_name
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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

Re: [ovs-dev] [PATCH 1/1] ovn-northd: fix memory leak in add_distributed_nat_routes() function

[0-day Robot]

Bleep bloop.  Greetings Damijan Skvarc, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


git-am:
fatal: sha1 information is lacking or useless (northd/ovn-northd.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 ovn-northd: fix memory leak in 
add_distributed_nat_routes() function
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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

Re: [ovs-dev] [PATCH ovn] logical-fields: fix memory leak while initializing ovnfield_by_name

[Dumitru Ceara]

On Wed, Aug 7, 2019 at 12:50 PM Damijan Skvarc  wrote:
>
> ovnfield_by_name is a hash table, intended to quicky find ovn_field by
> name from a list of supported ovn_fields. This hash table is initialized
> in ovn_init_symtab() function based on ovn_fields const array. In case
> ovn_init_symtab() function is called multiple times then hash table is
> reinitialized multiple times without deallocating previously allocated
> memory. This is actually what happens in ovn_controller by initializing
> lflow.symtab and ofctrl.symtab tables.
>
> Since ovnfield_by_name is initialized twice with same values I have
> introduced a flag indicating ovnfield_by_name is already initialized
> or not and based on this flag hash table is prevented to be initialized
> multiple times.
>
> Note that currently ovn_fields array is constituted from one single
> entry and thus searching a list of one entry by using helper hash table
> is somehow useless. Memory leak could be solved by simply removing
> ovnfield_by_name table and do a linear search through a list of single
> entry. However I want to keep previous functionality in case ovn_fields
> array will be extended somewhen in the future.
>
> Signed-off-by: Damijan Skvarc 
> ---
>  lib/logical-fields.c | 25 +++--
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/lib/logical-fields.c b/lib/logical-fields.c
> index 4ad5bf4..62b9a71 100644
> --- a/lib/logical-fields.c
> +++ b/lib/logical-fields.c
> @@ -57,6 +57,23 @@ add_ct_bit(const char *name, int index, struct shash 
> *symtab)
>  free(expansion);
>  }
>
> +
> +static void
> +init_ovnfield_by_name()
> +{

This should actually be init_ovnfield_by_name(void)

> +static bool initialized = 0;
> +
> +if (0 == initialized) {
> +shash_init(_by_name);
> +for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
> +const struct ovn_field *of = _fields[i];
> +ovs_assert(of->id == i); /* Fields must be in the enum order. */
> +shash_add_once(_by_name, of->name, of);
> +}
> +initialized=1;

I'd use "initialized = true/false" and if "(!initialized)" instead of
using 0 and 1.

> +}
> +}

Instead of adding this check here isn't it cleaner to just make sure
that we call init_ovnfield_by_name only once? I see there's
OVS_CONSTRUCTOR which should allow exactly what you're trying to do
with the check.

Thanks,
Dumitru

> +
>  void
>  ovn_init_symtab(struct shash *symtab)
>  {
> @@ -218,12 +235,8 @@ ovn_init_symtab(struct shash *symtab)
>  expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false);
>  expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
>
> -shash_init(_by_name);
> -for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
> -const struct ovn_field *of = _fields[i];
> -ovs_assert(of->id == i); /* Fields must be in the enum order. */
> -shash_add_once(_by_name, of->name, of);
> -}
> +init_ovnfield_by_name();
> +
>  expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU);
>  }
>
> --
> 2.7.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

[ovs-dev] [PATCH v2 ovn] logical-fields: fix memory leak while initializing ovnfield_by_name

[Damijan Skvarc]

fixed 0-day Robot issues 

Signed-off-by: Damijan Skvarc 
---
 lib/logical-fields.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/logical-fields.c b/lib/logical-fields.c
index 62b9a71..fdc78cf 100644
--- a/lib/logical-fields.c
+++ b/lib/logical-fields.c
@@ -57,9 +57,8 @@ add_ct_bit(const char *name, int index, struct shash *symtab)
 free(expansion);
 }
 
-
 static void
-init_ovnfield_by_name()
+init_ovnfield_by_name(void)
 {
 static bool initialized = 0;
 
@@ -70,7 +69,7 @@ init_ovnfield_by_name()
 ovs_assert(of->id == i); /* Fields must be in the enum order. */
 shash_add_once(_by_name, of->name, of);
 }
-initialized=1;
+initialized = 1;
 }
 }
 
-- 
2.7.4

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

[ovs-dev] [PATCH 1/1] ovn-northd: fix memory leak in add_distributed_nat_routes() function

[Damijan Skvarc]

Within this function actions & match dynamic strings are used as helper
variables for adding entries into logical flow table. Variables are
used several times in order to optimize number of memory allocations,
however at the end memory was forgotten to be deallocated.

Signed-off-by: Damijan Skvarc 
---
 northd/ovn-northd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 979dea4..8588961 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5667,6 +5667,8 @@ add_distributed_nat_routes(struct hmap *lflows, const 
struct ovn_port *op)
 ds_clear();
 }
 }
+ds_destroy();
+ds_destroy();
 }
 
 static void
-- 
2.7.4

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

Re: [ovs-dev] [PATCH ovn] logical-fields: fix memory leak while initializing ovnfield_by_name

[0-day Robot]

Bleep bloop.  Greetings Damijan Skvarc, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#52 FILE: lib/logical-fields.c:73:
initialized=1;

Lines checked: 76, Warnings: 2, Errors: 0


build:
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I 
./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I ./ovs -I ./ovs -I 
./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/mcast-group-index.lo -MD -MP -MF 
lib/.deps/mcast-group-index.Tpo -c lib/mcast-group-index.c -o 
lib/mcast-group-index.o
depbase=`echo lib/lex.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
  -I ./include  -I ./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I 
./ovs -I ./ovs -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra 
-Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum 
-Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes 
-Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers 
-fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/lex.lo -MD -MP 
-MF $depbase.Tpo -c -o lib/lex.lo lib/lex.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I 
./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I ./ovs -I ./ovs -I 
./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/lex.lo -MD -MP -MF lib/.deps/lex.Tpo -c 
lib/lex.c -o lib/lex.o
depbase=`echo lib/ovn-util.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
  -I ./include  -I ./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I 
./ovs -I ./ovs -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra 
-Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum 
-Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes 
-Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers 
-fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/ovn-util.lo -MD 
-MP -MF $depbase.Tpo -c -o lib/ovn-util.lo lib/ovn-util.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I 
./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I ./ovs -I ./ovs -I 
./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/ovn-util.lo -MD -MP -MF lib/.deps/ovn-util.Tpo 
-c lib/ovn-util.c -o lib/ovn-util.o
depbase=`echo lib/logical-fields.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
  -I ./include  -I ./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I 
./ovs -I ./ovs -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra 
-Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum 
-Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes 
-Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers 
-fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT 
lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o lib/logical-fields.lo 
lib/logical-fields.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I 
./ovs/include -I ./ovs/include -I ./ovs/lib -I ./ovs/lib -I ./ovs -I ./ovs -I 
./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/logical-fields.lo -MD -MP -MF 
lib/.deps/logical-fields.Tpo -c lib/logical-fields.c -o lib/logical-fields.o
lib/logical-fields.c:62:1: error: function declaration isn't a prototype 
[-Werror=strict-prototypes]
 init_ovnfield_by_name()
 ^

[ovs-dev] [PATCH ovn] logical-fields: fix memory leak while initializing ovnfield_by_name

[Damijan Skvarc]

ovnfield_by_name is a hash table, intended to quicky find ovn_field by
name from a list of supported ovn_fields. This hash table is initialized
in ovn_init_symtab() function based on ovn_fields const array. In case
ovn_init_symtab() function is called multiple times then hash table is
reinitialized multiple times without deallocating previously allocated
memory. This is actually what happens in ovn_controller by initializing
lflow.symtab and ofctrl.symtab tables.

Since ovnfield_by_name is initialized twice with same values I have
introduced a flag indicating ovnfield_by_name is already initialized
or not and based on this flag hash table is prevented to be initialized
multiple times.

Note that currently ovn_fields array is constituted from one single
entry and thus searching a list of one entry by using helper hash table
is somehow useless. Memory leak could be solved by simply removing
ovnfield_by_name table and do a linear search through a list of single
entry. However I want to keep previous functionality in case ovn_fields
array will be extended somewhen in the future.

Signed-off-by: Damijan Skvarc 
---
 lib/logical-fields.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/logical-fields.c b/lib/logical-fields.c
index 4ad5bf4..62b9a71 100644
--- a/lib/logical-fields.c
+++ b/lib/logical-fields.c
@@ -57,6 +57,23 @@ add_ct_bit(const char *name, int index, struct shash *symtab)
 free(expansion);
 }
 
+
+static void
+init_ovnfield_by_name()
+{
+static bool initialized = 0;
+
+if (0 == initialized) {
+shash_init(_by_name);
+for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
+const struct ovn_field *of = _fields[i];
+ovs_assert(of->id == i); /* Fields must be in the enum order. */
+shash_add_once(_by_name, of->name, of);
+}
+initialized=1;
+}
+}
+
 void
 ovn_init_symtab(struct shash *symtab)
 {
@@ -218,12 +235,8 @@ ovn_init_symtab(struct shash *symtab)
 expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false);
 expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false);
 
-shash_init(_by_name);
-for (int i = 0; i < OVN_FIELD_N_IDS; i++) {
-const struct ovn_field *of = _fields[i];
-ovs_assert(of->id == i); /* Fields must be in the enum order. */
-shash_add_once(_by_name, of->name, of);
-}
+init_ovnfield_by_name();
+
 expr_symtab_add_ovn_field(symtab, "icmp4.frag_mtu", OVN_ICMP4_FRAG_MTU);
 }
 
-- 
2.7.4

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

[ovs-dev] 2.12.0 branch snapshot - test 2618 failure on s390x

[James Page]

Just tripped on this one whilst building for s390x on Ubuntu:

##  ##

## Summary of the failures. ##
##  ##
Failed tests:
openvswitch 2.12.0 test suite test groups:

 NUM: FILE-NAME:LINE TEST-GROUP-NAME
  KEYWORDS

 2618: ovn.at:789 ovn -- action parsing

## -- ##
## Detailed failed tests. ##
## -- ##

# -*- compilation -*-
2618. ovn.at:789: testing ovn -- action parsing ...
../../tests/ovn.at:1403: ovstest test-ovn parse-actions < input.txt
--- expout  2019-08-07 10:40:15.539670085 +
+++ /<>/_debian/tests/testsuite.dir/at-groups/2618/stdout  
2019-08-07
10:40:15.539670085 +
@@ -579,7 +579,7 @@
 # bind_vport
 # lsp1's port key is 0x11.
 bind_vport("lsp1", inport);
-encodes as controller(userdata=00.00.00.11.00.00.00.00.11.00.00.00)
+encodes as controller(userdata=00.00.00.11.00.00.00.00.00.00.00.11)
 # lsp2 doesn't exist. So it should be encoded as drop.
 bind_vport("lsp2", inport);
 encodes as drop
2618. ovn.at:789: 2618. ovn -- action parsing (ovn.at:789): FAILED (ovn.at:1403)

Looks like something endian-y (s390x is the only BE arch in Ubuntu).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn] ovn-northd: Add IGMP Relay support

[Dumitru Ceara]

Add a new configuration option 'mcast_relay' to the Logical_Router:options
in the OVN Northbound database.

If a router is configured with 'mcast_relay' enabled then ovn-northd
will install Logical_Flows to allow IP multicast traffic to be routed
between Logical_Switches. The logical router will aggregate all IGMP
groups from attached logical switches and modify the routing pipeline in
the following way:
- Table S_ROUTER_IN_IP_INPUT: add flow matching the group address and
  set outport= associated with the IGMP group. Continue
  to next table.
- Table S_ROUTER_IN_IP_ROUTING: bypass route lookup for IP multicast
  traffic and just decrement TTL. If the packet reached this table then
  it must have matched a flow in S_ROUTER_IN_IP_INPUT and had outport
  set. Continue to next table.
- Table S_ROUTER_IN_ARP_RESOLVE: bypass ARP resolve for IP multicast
  traffic and continue to next table.
- Table S_ROUTER_OUT_DELIVERY: add flow matching IP multicast traffic
  and set ETH.SRC to the MAC address of the logical port on which
  traffic is forwarded.

Signed-off-by: Dumitru Ceara 
---
 NEWS|   1 +
 lib/mcast-group-index.h |   7 +-
 northd/ovn-northd.c | 506 
 ovn-nb.xml  |   6 +
 tests/ovn.at| 199 +--
 5 files changed, 580 insertions(+), 139 deletions(-)

diff --git a/NEWS b/NEWS
index f476984..73045d6 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,7 @@ Post-v2.11.0
logical groups which results in tunnels only been formed between
members of the same transport zone(s).
  * Support for new logical switch port type - 'virtual'.
+ * Support for IGMP Snooping/Querier and Relay.
- New QoS type "linux-netem" on Linux.
- Added support for TLS Server Name Indication (SNI).
 
diff --git a/lib/mcast-group-index.h b/lib/mcast-group-index.h
index 15a1592..1ec46c1 100644
--- a/lib/mcast-group-index.h
+++ b/lib/mcast-group-index.h
@@ -20,8 +20,11 @@ struct ovsdb_idl;
 
 struct sbrec_datapath_binding;
 
-#define OVN_MCAST_FLOOD_TUNNEL_KEY   65535
-#define OVN_MCAST_UNKNOWN_TUNNEL_KEY (OVN_MCAST_FLOOD_TUNNEL_KEY - 1)
+#define OVN_MCAST_FLOOD_TUNNEL_KEY 65535
+#define OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY \
+(OVN_MCAST_FLOOD_TUNNEL_KEY - 1)
+#define OVN_MCAST_UNKNOWN_TUNNEL_KEY \
+(OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY - 1)
 
 struct ovsdb_idl_index *mcast_group_index_create(struct ovsdb_idl *);
 const struct sbrec_multicast_group *
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index e6953a4..d40944c 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -444,21 +444,49 @@ BUILD_ASSERT_DECL(OVN_MAX_IP_MULTICAST >= 
OVN_MIN_MULTICAST);
 /*
  * Multicast snooping and querier per datapath configuration.
  */
+struct mcast_switch_info {
+
+bool enabled;   /* True if snooping enabled. */
+bool querier;   /* True if querier enabled. */
+bool flood_unregistered;/* True if unregistered multicast should be
+ * flooded.
+ */
+bool flood_relay;   /* True if the switch is connected to a
+ * multicast router and unregistered multicast
+ * should be flooded to the mrouter. Only
+ * applicable if flood_unregistered == false.
+ */
+
+int64_t table_size; /* Max number of IP multicast groups. */
+int64_t idle_timeout;   /* Timeout after which an idle group is
+ * flushed.
+ */
+int64_t query_interval; /* Interval between multicast queries. */
+char *eth_src;  /* ETH src address of the multicast queries. */
+char *ipv4_src; /* IP src address of the multicast queries. */
+int64_t query_max_response; /* Expected time after which reports should
+ * be received for queries that were sent out.
+ */
+
+uint32_t active_flows;  /* Current number of active IP multicast
+ * flows.
+ */
+};
+
+struct mcast_router_info {
+bool relay; /* True if the router should relay IP multicast. */
+};
+
 struct mcast_info {
-bool enabled;
-bool querier;
-bool flood_unregistered;
-
-int64_t table_size;
-int64_t idle_timeout;
-int64_t query_interval;
-char *eth_src;
-char *ipv4_src;
-int64_t  query_max_response;
-
-struct hmap group_tnlids;
-uint32_t group_tnlid_hint;
-uint32_t active_flows;
+
+struct hmap group_tnlids;  /* Group tunnel IDs in use on this DP. */
+uint32_t group_tnlid_hint; /* Hint for allocating next group tunnel ID. */
+struct ovs_list groups;/* List of groups learnt on this DP. */
+
+union {
+struct mcast_switch_info sw;  /* 

[ovs-dev] Payment for Invoice(Confirm Bank Details And Amount)

[Accounts Department]

Greetings,

We have been directed by our European District Company to send payment to your 
Bank Account for an outstanding invoice.

We wish to reconfirm from you, Your correct Bank Details And Invoice Amount.

We will not bear any lost if you fail to reconfirm details as attached as we 
intend to send payment tomorrow.

Hope To Read From You Soon.

Kathleen Combs
Accounts Payable
513-642-2621
4100 Port Union Road
Fairfield, Ohio 45014___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Do RCU synchronization at fixed interval in PMD main loop.

[Nitin Katiyar]

Each PMD updates the global sequence number for RCU synchronization
purpose with other OVS threads. This is done at every 1025th iteration
in PMD main loop.

If the PMD thread is responsible for polling large number of queues
that are carrying traffic, it spends a lot of time processing packets
and this results in significant delay in performing the housekeeping
activities.

If the OVS main thread is waiting to synchronize with the PMD threads
and if those threads delay performing housekeeping activities for
more than 3 sec then LACP processing will be impacted and it will lead
to LACP flaps. Similarly, other controls protocols run by OVS main
thread are impacted.

For e.g. a PMD thread polling 200 ports/queues with average of 1600
processing cycles per packet with batch size of 32 may take 1024
(200 * 1600 * 32) cycles per iteration. In system with 2.0 GHz CPU
it means more than 5 ms per iteration. So, for 1024 iterations to
complete it would be more than 5 seconds.

This gets worse when there are PMD threads which are less loaded.
It reduces possibility of getting mutex lock in ovsrcu_try_quiesce()
by heavily loaded PMD and next attempt to quiesce would be after 1024
iterations.

With this patch, PMD RCU synchronization will be performed after fixed
interval instead after a fixed number of iterations. This will ensure
that even if the packet processing load is high the RCU synchronization
will not be delayed long.

Co-authored-by: Anju Thomas 

Signed-off-by: Nitin Katiyar 
Signed-off-by: Anju Thomas 
---
 lib/dpif-netdev-perf.c | 16 
 lib/dpif-netdev-perf.h | 17 +
 lib/dpif-netdev.c  | 27 +++
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c
index e7ed49e..c888e5d 100644
--- a/lib/dpif-netdev-perf.c
+++ b/lib/dpif-netdev-perf.c
@@ -43,22 +43,6 @@ uint64_t iter_cycle_threshold;
 
 static struct vlog_rate_limit latency_rl = VLOG_RATE_LIMIT_INIT(600, 600);
 
-#ifdef DPDK_NETDEV
-static uint64_t
-get_tsc_hz(void)
-{
-return rte_get_tsc_hz();
-}
-#else
-/* This function is only invoked from PMD threads which depend on DPDK.
- * A dummy function is sufficient when building without DPDK_NETDEV. */
-static uint64_t
-get_tsc_hz(void)
-{
-return 1;
-}
-#endif
-
 /* Histogram functions. */
 
 static void
diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h
index 244813f..3f2ee1c 100644
--- a/lib/dpif-netdev-perf.h
+++ b/lib/dpif-netdev-perf.h
@@ -187,6 +187,23 @@ struct pmd_perf_stats {
 char *log_reason;
 };
 
+#ifdef DPDK_NETDEV
+static inline uint64_t
+get_tsc_hz(void)
+{
+return rte_get_tsc_hz();
+}
+#else
+/* This function is only invoked from PMD threads which depend on DPDK.
+ * A dummy function is sufficient when building without DPDK_NETDEV. */
+static inline uint64_t
+get_tsc_hz(void)
+{
+return 1;
+}
+#endif
+
+
 #ifdef __linux__
 static inline uint64_t
 rdtsc_syscall(struct pmd_perf_stats *s)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d0a1c58..c3d6835 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -751,6 +751,9 @@ struct dp_netdev_pmd_thread {
 
 /* Set to true if the pmd thread needs to be reloaded. */
 bool need_reload;
+
+/* Last time (in tsc) when PMD was last quiesced */
+uint64_t last_rcu_quiesced;
 };
 
 /* Interface to netdev-based datapath. */
@@ -5445,6 +5448,7 @@ pmd_thread_main(void *f_)
 int poll_cnt;
 int i;
 int process_packets = 0;
+uint64_t rcu_quiesce_interval = 0;
 
 poll_list = NULL;
 
@@ -5486,6 +5490,13 @@ reload:
 pmd->intrvl_tsc_prev = 0;
 atomic_store_relaxed(>intrvl_cycles, 0);
 cycles_counter_update(s);
+
+if (get_tsc_hz() > 1) {
+/* Calculate ~10 ms interval. */
+rcu_quiesce_interval = get_tsc_hz() / 100;
+pmd->last_rcu_quiesced = cycles_counter_get(s);
+}
+
 /* Protect pmd stats from external clearing while polling. */
 ovs_mutex_lock(>perf_stats.stats_mutex);
 for (;;) {
@@ -5493,6 +5504,19 @@ reload:
 
 pmd_perf_start_iteration(s);
 
+/* Do RCU synchronization at fixed interval instead of doing it
+ * at fixed number of iterations. This ensures that synchronization
+ * would not be delayed long even at high load of packet
+ * processing. */
+
+if (rcu_quiesce_interval &&
+((cycles_counter_get(s) - pmd->last_rcu_quiesced) >
+ rcu_quiesce_interval)) {
+if (!ovsrcu_try_quiesce()) {
+pmd->last_rcu_quiesced = cycles_counter_get(s);
+}
+}
+
 for (i = 0; i < poll_cnt; i++) {
 
 if (!poll_list[i].rxq_enabled) {
@@ -5527,6 +5551,9 @@ reload:
 dp_netdev_pmd_try_optimize(pmd, poll_list, poll_cnt);
 if (!ovsrcu_try_quiesce()) {
 emc_cache_slow_sweep(&((pmd->flow_cache).emc_cache));
+if (rcu_quiesce_interval) {
+