Re: [ovs-dev] tc-conntrack: inconsistent behaviour with icmpv6

[Marcelo Leitner]

Hi there,

On Wed, Mar 10, 2021 at 12:06:52PM +0100, Ilya Maximets wrote:
> Hi, Louis.  Thanks for your report!
> 
> Marcelo, Paul, could you, please, take a look?

Thanks for the ping.
+wenxu

> 
> Best regards, Ilya Maximets.
> 
> On 3/10/21 8:51 AM, Louis Peens wrote:
> > Hi all
> > 
> > We've recently encountered an interesting situation with OVS conntrack
> > when offloading to the TC datapath, and would like some feedback. Sorry
> > about the longish wall of text, but I'm trying to explain the problem
> > as clearly as possible. The very short summary is that there is a mismatch

Details are very welcomed, thanks for them.

> > in behaviour between the OVS datapath and OVS+TC datapath, and we're
> > not sure how to resolve this. Here goes:
> > 
> > We have a set of rules looking like this:
> > ovs-ofctl add-flow br0 
> > "table=0,in_port=p1,ct_state=-trk,ipv6,actions=ct(table=1)"
> > ovs-ofctl add-flow br0 
> > "table=0,in_port=p2,ct_state=-trk,ipv6,actions=ct(table=1)"
> > #post_ct flows"
> > ovs-ofctl add-flow br0 
> > "table=1,in_port=p1,ct_state=+trk+new,ipv6,actions=ct(commit),output:p2"
> > ovs-ofctl add-flow br0 
> > "table=1,in_port=p2,ct_state=+trk+new,ipv6,actions=ct(commit),output:p1"
> > ovs-ofctl add-flow br0 
> > "table=1,in_port=p1,ct_state=+trk+est,ipv6,actions=output:p2"
> > ovs-ofctl add-flow br0 
> > "table=1,in_port=p2,ct_state=+trk+est,ipv6,actions=output:p1"
> > 
> > p1/p2 are the endpoints of two different veth pairs, just to keep this 
> > simple.
> > The rules above work well enough with UDP/TCP traffic, however ICMPv6 
> > packets
> > (08:56:39.984375 IP6 2001:db8:0:f101::1 > ff02::1:ff00:2: ICMP6, neighbor 
> > solicitation, who has 2001:db8:0:f101::2, length 32)
> > breaks this somewhat. With TC offload disabled:
> > 
> > ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=false
> > 
> > we get the following datapath rules:
> > 
> > ovs-appctl dpctl/dump-flows --names
> > recirc_id(0x1),in_port(p1),ct_state(-new-est+trk),eth(),eth_type(0x86dd),ipv6(frag=no),
> >  packets:2, bytes:172, used:1.329s, actions:drop
> > recirc_id(0),in_port(p1),ct_state(-trk),eth(),eth_type(0x86dd),ipv6(frag=no),
> >  packets:2, bytes:172, used:1.329s, actions:ct,recirc(0x1)
> > 
> > This part is still fine, we do not have a rule for just matching +trk, so 
> > the
> > the drop rule is to be expected. The problem however is when we enable TC
> > offload:
> > 
> > ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true
> > 
> > This is the result in the datapath:
> > 
> > ovs-appctl dpctl/dump-flows --names
> > ct_state(-trk),recirc_id(0),in_port(p1),eth_type(0x86dd),ipv6(frag=no), 
> > packets:2, bytes:144, used:0.920s, actions:ct,recirc(0x1)
> > recirc_id(0x1),in_port(p1),ct_state(-new-est-trk),eth(),eth_type(0x86dd),ipv6(frag=no),
> >  packets:1, bytes:86, used:0.928s, actions:drop
> > recirc_id(0x1),in_port(p1),ct_state(-new-est+trk),eth(),eth_type(0x86dd),ipv6(frag=no),
> >  packets:0, bytes:0, used:never, actions:drop
> > 
> > Notice the installation of the two recirc rules, one with -trk and one with 
> > +trk,
> > with the -trk one being the rule that handles all the next packets. Further
> > investigation reveals that something like the following is happening:
> > 
> > 1) The first packet arrives and is handled by the OVS datapath,

Hmm. This shouldn't happen if hw-offload=true, because the first rule
should be installed on tc datapath already. Or maybe you mean OVS
vswitchd when you referred to OVS datapath?

What does  dpctl/dump-flows --names -m  gives in this situation, are
all flows installed on dp:tc?

> >triggering the installation of the two rules like in the non-offloaded
> >case. So the recirc_id(0) rule gets installed into tc, and recirc_id(0x1)
> >gets installed into the ovs datapath. This bit of code in the OVS module
> >makes sure that +trk is set.
> > 
> > /* Update 'key' based on skb->_nfct.  If 'post_ct' is true, then OVS has
> >  * previously sent the packet to conntrack via the ct action.
> >  * /
> >static void ovs_ct_update_key(const struct sk_buff *skb,
> >   const struct ovs_conntrack_info *info,
> >   struct sw_flow_key *key, bool post_ct,
> >   bool keep_nat_flags)
> > {
> > ...
> > ct = nf_ct_get(skb, &ctinfo);
> > if (ct) {//tracked
> > ...
> > } else if (post_ct) {
> > state = OVS_CS_F_TRACKED | OVS_CS_F_INVALID;
> > if (info)
> > zone = &info->zone;
> > }
> > __ovs_ct_update_key(key, state, zone, ct);
> > 
> > }
> > Obviously this is not the case when the packet was sent to conntrack
> > via tc.
> > 
> > 2) The second packet arrives, and now hits the rule installed in
> >TC. However, TC does not handle ICMPv6 (Neighbor Solicitation), and 
> >

Re: [ovs-dev] [PATCH v6] python: Send notifications after the transaction ends

[Dumitru Ceara]

On 3/9/21 3:34 PM, Terry Wilson wrote:

The Python IDL notification mechanism was sending a notification
for each processed update in a transaction as it was processed.
This causes issues with multi-row changes that contain references
to each other.

For example, if a Logical_Router_Port is created along with a
Gateway_Chassis, and the LRP.gateway_chassis set to that GC, then
when the notify() passes the CREATE event for the LRP, the GC will
not yet have been processed, so __getattr__ when _uuid_to_row fails
to find the GC, will return the default value for LRP.gateway_chassis
which is [].

This patch has the process_update methods return the notifications
that would be produced when a row changes, so they can be queued
and sent after all rows have been processed.

Fixes: d7d417fcddf9 ("Allow subclasses of Idl to define a notification hook")
Signed-off-by: Terry Wilson 
---


I'm not too familiar with the python IDL code but the change looks good 
to me.


I also verified and the new tests exercises the scenario described 
above.  All other IDL tests also passed.


Acked-by: Dumitru Ceara 

Thanks,
Dumitru

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

Re: [ovs-dev] [PATCH ovn 1/2] Set release date for 21.03.0

[Mark Michelson]

On 3/12/21 12:31 PM, Numan Siddique wrote:

On Fri, Mar 12, 2021 at 6:57 AM Mark Michelson  wrote:


Signed-off-by: Mark Michelson 


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

I suppose the first patch should be applied to the main branch as well.

Thanks
Numan



Thanks Numan. I pushed patch 1 to master and branch-21.03. I tagged 
v21.03.0 and pushed it, and I pushed patch 2 to branch-21.03.



---
  NEWS | 2 +-
  debian/changelog | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 199a9266f..5372668bf 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-OVN v21.03.0 - 5 Mar 2020
+OVN v21.03.0 - 12 Mar 2021
  -
- Support ECMP multiple nexthops for reroute router policies.
 - BFD protocol support according to RFC880 [0]. Introduce next-hop BFD
diff --git a/debian/changelog b/debian/changelog
index aa5f6be9b..51f9bcc91 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,7 +2,7 @@ ovn (21.03.0-1) unstable; urgency=low

 * New upstream version

- -- OVN team   Fri, 05 Mar 2021 12:00:00 -0500
+ -- OVN team   Fri, 12 Mar 2021 12:00:00 -0500

  ovn (20.12.90-1) unstable; urgency=low

--
2.29.2

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





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

Re: [ovs-dev] [PATCH 1/1] tests: Add PMD auto load balance unit tests.

[Pai G, Sunil]

> >> Afaiu, from ovsdb pov, other_config is a simple map of strings with
> >> no constraint on the content of the strings.
> >> I understand the constraints expressed for sub fields like
> >> pmd-auto-lb-XX parameters in vswitchd/vswitchd.xml as only for
> documentation.
> >>
> >> I don't see the benefit of checking values 2 or 20001.
> >
> > I couldn't find the doc where the max value is mentioned.
> > Would you mind pointing it ?
> > Or its calculated somehow ?
> >
> 
> Thanks Sunil (and David). The max is mentioned here:
> https://github.com/openvswitch/ovs/blob/master/vswitchd/vswitch.xml#L6
> 77-L679

Thanks Kevin , I somehow overlooked the file mentioned by David.

> 
> We probably need to think a bit more about the units and max for this
> parameter in general, but that's what's currently documented.

I agree.

> 
> Kevin.

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

[ovs-dev] [PATCH] github: Fix handling of python packages.

[Ilya Maximets]

GitHub Actions doesn't have python locations in PATH and different
runners might have different configuration for default python
location and versions.  For example, on some runners python2 might
be installed or not.

Missing PATH causes weird situations on older branches where during
one run our scripts can locate just installed flake8 and can't do
that on a different run.  But this might also create other
unpredictable issues on all branches.

It's required to use actions/setup-python@v2 in order to have
predictable version of python installed and paths correctly configured.
Due to some bugs in GHA itself it doesn't set $HOME/.local/bin into
PATH, so we have to do that manually for now in order to use '--user'.
This might be fixed later in actions/setup-python or in base runners.
We already setting it for DPDK 20.11 (I think the issue was spotted
but not fully investigated).  Moving PATH updates to a separate step
to make them more explicit and avaialble for all steps of the job.

Unfortunately actions/setup-python@v2 also makes invisible python
packages installed from Ubuntu repositories.  Switching them to
'pip3 install'.

Fixes: 6cb2f5a630e3 ("github: Add GitHub Actions workflow.")
Reported-by: Numan Siddique 
Signed-off-by: Ilya Maximets 
---
 .ci/linux-prepare.sh |  3 ++-
 .github/workflows/build-and-test.yml | 26 +-
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 69a40011f..c55125cf7 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -20,7 +20,8 @@ cd sparse
 make -j4 HAVE_LLVM= HAVE_SQLITE= install
 cd ..
 
-pip3 install --disable-pip-version-check --user flake8 hacking
+pip3 install --disable-pip-version-check --user \
+flake8 hacking sphinx pyOpenSSL wheel setuptools
 pip3 install --user --upgrade docutils
 pip3 install --user  'meson==0.47.1'
 
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 1bb72bbb1..ce98a9f98 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -8,9 +8,7 @@ jobs:
   dependencies: |
 automake libtool gcc bc libjemalloc1 libjemalloc-dev\
 libssl-dev llvm-dev libelf-dev libnuma-dev libpcap-dev  \
-ninja-build python3-openssl python3-pip \
-python3-setuptools python3-sphinx python3-wheel \
-selinux-policy-dev
+ninja-build selinux-policy-dev
   deb_dependencies: |
 linux-headers-$(uname -r) build-essential fakeroot devscripts equivs
   AFXDP:   ${{ matrix.afxdp }}
@@ -115,6 +113,16 @@ jobs:
 - name: checkout
   uses: actions/checkout@v2
 
+- name: update PATH
+  run:  |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+- name: set up python
+  uses: actions/setup-python@v2
+  with:
+python-version: '3.x'
+
 - name: create ci signature file for the dpdk cache key
   if:   matrix.dpdk != '' || matrix.dpdk_shared != ''
   # This will collect most of DPDK related lines, so hash will be different
@@ -151,7 +159,7 @@ jobs:
   run:  ./.ci/linux-prepare.sh
 
 - name: build
-  run:  PATH="$PATH:$HOME/bin:$HOME/.local/bin" ./.ci/linux-build.sh
+  run:  ./.ci/linux-build.sh
 
 - name: upload deb packages
   if:   matrix.deb_package != ''
@@ -194,12 +202,20 @@ jobs:
 steps:
 - name: checkout
   uses: actions/checkout@v2
+- name: update PATH
+  run:  |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+- name: set up python
+  uses: actions/setup-python@v2
+  with:
+python-version: '3.x'
 - name: install dependencies
   run:  brew install automake libtool
 - name: prepare
   run:  ./.ci/osx-prepare.sh
 - name: build
-  run:  PATH="$PATH:$HOME/bin" ./.ci/osx-build.sh
+  run:  ./.ci/osx-build.sh
 - name: upload logs on failure
   if: failure()
   uses: actions/upload-artifact@v2
-- 
2.26.2

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

[ovs-dev] [PATCH ovn v2] ci: Fix handling of python packages.

[Ilya Maximets]

GitHub Actions doesn't have python locations in PATH and different
runners might have different configuration for default python
location and versions.  For example, on some runners python2 might
be installed or not.

Missing PATH causes weird situations where during one run our scripts
can locate just installed flake8 and can't do that on a different run.

Also, we're mistakenly installing python2 version of flake8.
On runners that able to locate installed flake8 this causes breakage
of a flake8-check build target because our python scripts written for
python3.  And runners that can't locate flake8 works just fine and
job succeeds.

It's required to use actions/setup-python@v2 in order to have
predictable version of python installed and paths correctly configured.
Due to some bugs in GHA itself it doesn't set $HOME/.local/bin into
PATH, so we have to do that manually for now in order to use '--user'.

Unfortunately actions/setup-python@v2 also makes invisible python
packages installed from Ubuntu repositories.  Switching them to
'pip3 install'.

'six' package is not needed, so it's dropped.

Fixes: ecdd790ecbff ("CI: Add github actions workflow.")
Reported-by: Numan Siddique 
Signed-off-by: Ilya Maximets 
---
 .ci/linux-prepare.sh   |  4 ++--
 .github/workflows/test.yml | 23 ---
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 0bb0ff096..83ad3958b 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -12,5 +12,5 @@ set -ev
 git clone git://git.kernel.org/pub/scm/devel/sparse/sparse.git
 cd sparse && make -j4 HAVE_LLVM= HAVE_SQLITE= install && cd ..
 
-pip install --disable-pip-version-check --user six flake8 hacking
-pip install --user --upgrade docutils
+pip3 install --disable-pip-version-check --user flake8 hacking sphinx pyOpenSSL
+pip3 install --upgrade --user docutils
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 26a8edb8f..6de63985d 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -13,7 +13,6 @@ jobs:
   dependencies: |
 automake libtool gcc bc libjemalloc1 libjemalloc-dev\
 libssl-dev llvm-dev libelf-dev libnuma-dev libpcap-dev  \
-python3-openssl python3-pip python3-sphinx  \
 selinux-policy-dev
   m32_dependecies: gcc-multilib
   CC:  ${{ matrix.compiler }}
@@ -97,11 +96,21 @@ jobs:
   if:   matrix.m32 != ''
   run:  sudo apt install -y ${{ env.m32_dependecies }}
 
+- name: update PATH
+  run:  |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+- name: set up python
+  uses: actions/setup-python@v2
+  with:
+python-version: '3.x'
+
 - name: prepare
   run:  ./.ci/linux-prepare.sh
 
 - name: build
-  run:  PATH="$PATH:$HOME/bin" ./.ci/linux-build.sh
+  run:  ./.ci/linux-build.sh
 
 - name: copy logs on failure
   if: failure() || cancelled()
@@ -154,10 +163,18 @@ jobs:
 ref: 'master'
 - name: install dependencies
   run:  brew install automake libtool
+- name: update PATH
+  run:  |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+- name: set up python
+  uses: actions/setup-python@v2
+  with:
+python-version: '3.x'
 - name: prepare
   run:  ./.ci/osx-prepare.sh
 - name: build
-  run:  PATH="$PATH:$HOME/bin" ./.ci/osx-build.sh
+  run:  ./.ci/osx-build.sh
 - name: upload logs on failure
   if: failure()
   uses: actions/upload-artifact@v2
-- 
2.26.2

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

Re: [ovs-dev] [PATCH 1/1] tests: Add PMD auto load balance unit tests.

[Kevin Traynor]

On 12/03/2021 13:21, Pai G, Sunil wrote:
> Hey Kevin, 
> 
> Tested the patch , works fine for me too :)
> 
>>
>> The ALB log messages follow a common format, so I'd suggest combining
>> those 3 helpers as a single.
>> That is with the hope we will conform to this format for new parameters
>> later.
>>
>> dnl CHECK_ALB_PARAM([param], [value], [+line]) dnl dnl Waits for ALB logs
>> for 'param' in logs and checks if value matches dnl 'value'. Checking starts
>> from line number 'line' in ovs-vswithd.log.
>> m4_define([CHECK_ALB_PARAM], [
>> line_st=$3
>> if [[ -z "$line_st" ]]
>> then
>> line_st="+0"
>> fi
>> OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "PMD auto load
>> balance $1 set to"])
>> AT_CHECK([tail -n $line_st ovs-vswitchd.log | sed -n "s#.*\(PMD auto load
>> balance $1 set to.*\)#\1#p" | tail -1], [0], [dnl PMD auto load balance $1 
>> set
>> to $2
>> ])
>> ])
>>
> 
> I agree with David on this one. It might be good to create a helper like 
> CHECK_LOAD_PARAM.
> 
>>
>>
>>> +AT_SETUP([ALB - interval param])
>>> +OVS_VSWITCHD_START
>>> +OVS_WAIT_UNTIL([grep "PMD auto load balance is disabled"
>>> +ovs-vswitchd.log])
>>> +
>>> +# Check default
>>> +CHECK_INTERVAL_PARAM([1], [])
>>
>> Here, it becomes:
>> CHECK_ALB_PARAM([interval], [1 mins], [])
>>
>> etc..
>>
>> [snip]
>>
>>> +# TODO 2 is documented as max value but it seems arbitary #
>>> +Setting to 20001 works, should it be checked and rejected?
>>> +# For now add a dummy test above the max value that accepts it
>>
>> Afaiu, from ovsdb pov, other_config is a simple map of strings with no
>> constraint on the content of the strings.
>> I understand the constraints expressed for sub fields like pmd-auto-lb-XX
>> parameters in vswitchd/vswitchd.xml as only for documentation.
>>
>> I don't see the benefit of checking values 2 or 20001.
> 
> I couldn't find the doc where the max value is mentioned.
> Would you mind pointing it ?
> Or its calculated somehow ? 
> 

Thanks Sunil (and David). The max is mentioned here:
https://github.com/openvswitch/ovs/blob/master/vswitchd/vswitch.xml#L677-L679

We probably need to think a bit more about the units and max for this
parameter in general, but that's what's currently documented.

Kevin.


> The rest looks to good to me!
> 
> 
> 

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

Re: [ovs-dev] [PATCH ovn v2 5/9] northd-ddlog: Update RBAC rules

[Numan Siddique]

On Fri, Mar 5, 2021 at 5:49 PM Frode Nordahl
 wrote:
>
> This patch summarizes a series of fixes to the C northd for missing
> or out of date RBAC rules and updates the DDlog version of Northd
> accordingly.
>
> Signed-off-by: Frode Nordahl 

Hi Frode,

Thanks for the patch series.

I applied the patches 1 to 5 of this series to master and backported
1-4 patches to
branch-21.03.

I have also backported some of the patches down to 20.03. I need to
apply a couple of
patches down to the 20.03 branch. I will do that in some time.

For the patches 6-9, I have not looked at them yet. I'd appreciate it
if others want to review them.

Thanks
Numan

> ---
>  northd/ovn_northd.dl | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 4482cffc0..8bc6dd9f6 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1257,7 +1257,8 @@ sb::Out_RBAC_Permission (
>  .authorization  = set_singleton("name"),
>  .insert_delete  = true,
>  .update = ["nb_cfg", "external_ids", "encaps",
> -   "vtep_logical_switches", "other_config"].to_set()
> +   "vtep_logical_switches", "other_config",
> +   "transport_zones"].to_set()
>  ).
>
>  sb::Out_RBAC_Permission (
> @@ -1281,7 +1282,7 @@ sb::Out_RBAC_Permission (
>  .table  = "Port_Binding",
>  .authorization  = set_singleton(""),
>  .insert_delete  = false,
> -.update = ["chassis", "up"].to_set()
> +.update = ["chassis", "encap", "up", "virtual_parent"].to_set()
>  ).
>
>  sb::Out_RBAC_Permission (
> @@ -1308,6 +1309,23 @@ sb::Out_RBAC_Permission (
>  .update = ["address", "chassis", "datapath", "ports"].to_set()
>  ).
>
> +sb::Out_RBAC_Permission (
> +._uuid  = 128'h2e5cbf3d_26f6_4f8a_9926_d6f77f61654f,
> +.table  = "Controller_Event",
> +.authorization  = set_singleton(""),
> +.insert_delete  = true,
> +.update = ["chassis", "event_info", "event_type",
> +   "seq_num"].to_set()
> +).
> +
> +sb::Out_RBAC_Permission (
> +._uuid  = 128'hb70964fc_322f_4ae5_aee4_ff6afadcc126,
> +.table  = "FDB",
> +.authorization  = set_singleton(""),
> +.insert_delete  = true,
> +.update = ["dp_key", "mac", "port_key"].to_set()
> +).
> +
>  /*
>   * RBAC_Role: fixed
>   */
> @@ -1317,7 +1335,9 @@ sb::Out_RBAC_Role (
>  .permissions = [
>  "Chassis" -> 128'h7df3749a_1754_4a78_afa4_3abf526fe510,
>  "Chassis_Private" -> 128'h07e623f7_137c_4a11_9084_3b3f89cb4a54,
> +"Controller_Event" -> 128'h2e5cbf3d_26f6_4f8a_9926_d6f77f61654f,
>  "Encap" -> 128'h94bec860_431e_4d95_82e7_3b75d8997241,
> +"FDB" -> 128'hb70964fc_322f_4ae5_aee4_ff6afadcc126,
>  "Port_Binding" -> 128'hd8ceff1a_2b11_48bd_802f_4a991aa4e908,
>  "MAC_Binding" -> 128'h6ffdc696_8bfb_4d82_b620_a00d39270b2f,
>  "Service_Monitor"-> 128'h39231c7e_4bf1_41d0_ada4_1d8a319c0da3]
> --
> 2.30.0
>
> ___
> 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 ovn] ci: Fix handling of python packages.

[Ilya Maximets]

On 3/12/21 5:35 PM, Ilya Maximets wrote:
> GitHub Actions doesn't have python locations in PATH and different
> runners might have different configuration for default python
> location and versions.  For example, on some runners python2 might
> be installed or not.
> 
> Missing PATH causes weird situations where during one run our scripts
> can locate just installed flake8 and can't do that on a different run.
> 
> Also, we're mistakenly installing python2 version of flake8.
> On runners that able to locate installed flake8 this causes breakage
> of a flake8-check build target because our python scripts written for
> python3.  And runners that can't locate flake8 works just fine and
> job succeeds.
> 
> It's required to use actions/setup-python@v2 in order to have
> predictable version of python installed and paths correctly configured.
> Due to some bugs in GHA itself it doesn't set $HOME/.local/bin into
> PATH, so we have to do that manually for now in order to use '--user'.
> 
> Fixes: ecdd790ecbff ("CI: Add github actions workflow.")
> Reported-by: Numan Siddique 
> Signed-off-by: Ilya Maximets 
> ---
> 
> Will also work on similar patch for OVS.
> 
>  .ci/linux-prepare.sh   |  4 ++--
>  .github/workflows/test.yml | 22 --
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
> index 0bb0ff096..55f419b63 100755
> --- a/.ci/linux-prepare.sh
> +++ b/.ci/linux-prepare.sh
> @@ -12,5 +12,5 @@ set -ev
>  git clone git://git.kernel.org/pub/scm/devel/sparse/sparse.git
>  cd sparse && make -j4 HAVE_LLVM= HAVE_SQLITE= install && cd ..
>  
> -pip install --disable-pip-version-check --user six flake8 hacking
> -pip install --user --upgrade docutils
> +pip3 install --disable-pip-version-check --user six flake8 hacking
> +pip3 install --upgrade --user docutils
> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
> index 26a8edb8f..251672748 100644
> --- a/.github/workflows/test.yml
> +++ b/.github/workflows/test.yml
> @@ -97,11 +97,21 @@ jobs:
>if:   matrix.m32 != ''
>run:  sudo apt install -y ${{ env.m32_dependecies }}
>  
> +- name: update PATH
> +  run:  |
> +echo "$HOME/bin">> $GITHUB_PATH
> +echo "$HOME/.local/bin" >> $GITHUB_PATH
> +
> +- name: set up python
> +  uses: actions/setup-python@v2
> +  with:
> +python-version: '3.x'

Ugh.  This change also makes invisible packages installed from
Ubuntu repositories.  Will move them to pip3 installation and
send v2.

> +
>  - name: prepare
>run:  ./.ci/linux-prepare.sh
>  
>  - name: build
> -  run:  PATH="$PATH:$HOME/bin" ./.ci/linux-build.sh
> +  run:  ./.ci/linux-build.sh
>  
>  - name: copy logs on failure
>if: failure() || cancelled()
> @@ -154,10 +164,18 @@ jobs:
>  ref: 'master'
>  - name: install dependencies
>run:  brew install automake libtool
> +- name: update PATH
> +  run:  |
> +echo "$HOME/bin">> $GITHUB_PATH
> +echo "$HOME/.local/bin" >> $GITHUB_PATH
> +- name: set up python
> +  uses: actions/setup-python@v2
> +  with:
> +python-version: '3.x'
>  - name: prepare
>run:  ./.ci/osx-prepare.sh
>  - name: build
> -  run:  PATH="$PATH:$HOME/bin" ./.ci/osx-build.sh
> +  run:  ./.ci/osx-build.sh
>  - name: upload logs on failure
>if: failure()
>uses: actions/upload-artifact@v2
> 

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

Re: [ovs-dev] [PATCH ovn 1/2] Set release date for 21.03.0

[Numan Siddique]

On Fri, Mar 12, 2021 at 6:57 AM Mark Michelson  wrote:
>
> Signed-off-by: Mark Michelson 

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

I suppose the first patch should be applied to the main branch as well.

Thanks
Numan

> ---
>  NEWS | 2 +-
>  debian/changelog | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 199a9266f..5372668bf 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,4 +1,4 @@
> -OVN v21.03.0 - 5 Mar 2020
> +OVN v21.03.0 - 12 Mar 2021
>  -
>- Support ECMP multiple nexthops for reroute router policies.
> - BFD protocol support according to RFC880 [0]. Introduce next-hop BFD
> diff --git a/debian/changelog b/debian/changelog
> index aa5f6be9b..51f9bcc91 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -2,7 +2,7 @@ ovn (21.03.0-1) unstable; urgency=low
>
> * New upstream version
>
> - -- OVN team   Fri, 05 Mar 2021 12:00:00 -0500
> + -- OVN team   Fri, 12 Mar 2021 12:00:00 -0500
>
>  ovn (20.12.90-1) unstable; urgency=low
>
> --
> 2.29.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule version to latest ovsdb-cs changes.

[Dmitry Yusupov]

> On Thu, Mar 11, 2021 at 6:11 PM Ben Pfaff  wrote:
> >
> > On Thu, Mar 11, 2021 at 08:07:58PM -0500, Mark Michelson wrote:
> > > On 3/11/21 5:58 PM, Ilya Maximets wrote:
> > > > I'd say that eventually OVN should become less feature hungry
> > > > or less dependent from OVS changes.  Then we should consider
> > > > to use only stable released OVS as a build dependency and
> > > > actually wait for the next major OVS release if some new features
> > > > required from it.  But right now we can't afford that.
> > >
> > > I'd say the vast majority of the time these days, the OVS changes OVN
> > > depends on are specifically OVSDB changes. One possible idea is to 
> > > separate
> > > OVSDB from OVS and semantically version the OVSDB API. Then OVS and OVN
> > > would be consumers of the same common project.
> > >
> > > You'd still have the potential for needing to update the OVS submodule to
> > > some non-released version. But hopefully the frequency of such updates 
> > > would
> > > be reduced. And if the frequency is reduced enough, it probably wouldn't 
> > > be
> > > outside the realm of reason to request backports of bug fixes and
> > > point-releases of OVS whenever OVN requires a submodule update.
> >
> > I'm adding Dmitry Yusupov to this thread because he expressed some
> > interest in separating OVSDB from OVS in the latest OVS Orbit episode
> > (https://ovsorbit.org/#e72).
>
> I think this is not only a dependency problem of compiling, but also of 
> runtime,
> if we want to continue supporting debian, because in debian packages dynamic 
> linked
> library is used instead of statically linked. If I understand correctly, the 
> submodule
> approach solves the compile time dependency but not runtime, for debian.
> So the only way out eventually might be separating OVSDB from OVS.
> (Please correct me if I am wrong)

Semantic versioning of OVSDB API would surely enable better compatibility 
between the projects.
As of right now, static linking in OVN RPM packages for instance can be easily 
disabled.
My tests showing that it can be beneficial in environments where we can  have 
controlled upgrades.
But without versioned OVSDB API it is just too dangerous to have that as a 
default.

As I also expressed in ovsorbit podcast, separation of OVSDB from OVS could 
potentially help us expand user base beyond just OVS use cases.
I've built OVSDB-KV prototype in Go and did some comparison to ETCD (see slides 
that Ben listed in podcast reference #e72) and found quite significant 
performance gains, granted after few changes to OVSDB that I proposed.

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

[ovs-dev] [PATCH ovn] ci: Fix handling of python packages.

[Ilya Maximets]

GitHub Actions doesn't have python locations in PATH and different
runners might have different configuration for default python
location and versions.  For example, on some runners python2 might
be installed or not.

Missing PATH causes weird situations where during one run our scripts
can locate just installed flake8 and can't do that on a different run.

Also, we're mistakenly installing python2 version of flake8.
On runners that able to locate installed flake8 this causes breakage
of a flake8-check build target because our python scripts written for
python3.  And runners that can't locate flake8 works just fine and
job succeeds.

It's required to use actions/setup-python@v2 in order to have
predictable version of python installed and paths correctly configured.
Due to some bugs in GHA itself it doesn't set $HOME/.local/bin into
PATH, so we have to do that manually for now in order to use '--user'.

Fixes: ecdd790ecbff ("CI: Add github actions workflow.")
Reported-by: Numan Siddique 
Signed-off-by: Ilya Maximets 
---

Will also work on similar patch for OVS.

 .ci/linux-prepare.sh   |  4 ++--
 .github/workflows/test.yml | 22 --
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh
index 0bb0ff096..55f419b63 100755
--- a/.ci/linux-prepare.sh
+++ b/.ci/linux-prepare.sh
@@ -12,5 +12,5 @@ set -ev
 git clone git://git.kernel.org/pub/scm/devel/sparse/sparse.git
 cd sparse && make -j4 HAVE_LLVM= HAVE_SQLITE= install && cd ..
 
-pip install --disable-pip-version-check --user six flake8 hacking
-pip install --user --upgrade docutils
+pip3 install --disable-pip-version-check --user six flake8 hacking
+pip3 install --upgrade --user docutils
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 26a8edb8f..251672748 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -97,11 +97,21 @@ jobs:
   if:   matrix.m32 != ''
   run:  sudo apt install -y ${{ env.m32_dependecies }}
 
+- name: update PATH
+  run:  |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+
+- name: set up python
+  uses: actions/setup-python@v2
+  with:
+python-version: '3.x'
+
 - name: prepare
   run:  ./.ci/linux-prepare.sh
 
 - name: build
-  run:  PATH="$PATH:$HOME/bin" ./.ci/linux-build.sh
+  run:  ./.ci/linux-build.sh
 
 - name: copy logs on failure
   if: failure() || cancelled()
@@ -154,10 +164,18 @@ jobs:
 ref: 'master'
 - name: install dependencies
   run:  brew install automake libtool
+- name: update PATH
+  run:  |
+echo "$HOME/bin">> $GITHUB_PATH
+echo "$HOME/.local/bin" >> $GITHUB_PATH
+- name: set up python
+  uses: actions/setup-python@v2
+  with:
+python-version: '3.x'
 - name: prepare
   run:  ./.ci/osx-prepare.sh
 - name: build
-  run:  PATH="$PATH:$HOME/bin" ./.ci/osx-build.sh
+  run:  ./.ci/osx-build.sh
 - name: upload logs on failure
   if: failure()
   uses: actions/upload-artifact@v2
-- 
2.26.2

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

Re: [ovs-dev] [PATCH] dpif-netdev: Allow PMD auto load balance with cross-numa.

[David Marchand]

On Thu, Mar 11, 2021 at 4:04 PM Kevin Traynor  wrote:
>
> Previously auto load balance did not trigger a reassignment when
> there was any cross-numa polling as an rxq could be polled from a
> different numa after reassign and it could impact estimates.
>
> In the case where there is only one numa with pmds available, the
> same numa will always poll before and after reassignment, so estimates
> are valid. Allow PMD auto load balance to trigger a reassignment in
> this case.
>
> Signed-off-by: Kevin Traynor 

Patch lgtm, I will test it next week, as my current system is mono numa..

-- 
David Marchand

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

Re: [ovs-dev] [PATCH v5] python: Send notifications after the transaction ends

[Flavio Fernandes]

Tested-by: Flavio Fernandes 

Used this change against Openstack's OVN Zuul :

 https://review.opendev.org/c/openstack/neutron/+/778595 





> On Mar 4, 2021, at 11:18 AM, Terry Wilson  wrote:
> 
> The Python IDL notification mechanism was sending a notification
> for each processed update in a transaction as it was processed.
> This causes issues with multi-row changes that contain references
> to each other.
> 
> For example, if a Logical_Router_Port is created along with a
> Gateway_Chassis, and the LRP.gateway_chassis set to that GC, then
> when the notify() passes the CREATE event for the LRP, the GC will
> not yet have been processed, so __getattr__ when _uuid_to_row fails
> to find the GC, will return the default value for LRP.gateway_chassis
> which is [].
> 
> This patch has the process_update methods return the notifications
> that would be produced when a row changes, so they can be queued
> and sent after all rows have been processed.
> 
> Fixes: d7d417fcddf9 ("Allow subclasses of Idl to define a notification hook")
> Signed-off-by: Terry Wilson 
> ---
> python/ovs/db/idl.py | 39 ---
> tests/ovsdb-idl.at   | 22 ++
> tests/test-ovsdb.py  |  7 +--
> 3 files changed, 51 insertions(+), 17 deletions(-)
> 

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

Re: [ovs-dev] [PATCH v2 2/2] testsuite: add test cases for ingress_policing parameters

[0-day Robot]

Bleep bloop.  Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Simon Horman , Louis Peens 

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


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

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

Re: [ovs-dev] [PATCH v2 1/2] netdev-linux: correct unit of burst parameter

[0-day Robot]

Bleep bloop.  Greetings Simon Horman, 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: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Simon Horman , Louis Peens 

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


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

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

Re: [ovs-dev] [PATCH 1/1] tests: Add PMD auto load balance unit tests.

[Pai G, Sunil]

Hey Kevin, 

Tested the patch , works fine for me too :)

> 
> The ALB log messages follow a common format, so I'd suggest combining
> those 3 helpers as a single.
> That is with the hope we will conform to this format for new parameters
> later.
> 
> dnl CHECK_ALB_PARAM([param], [value], [+line]) dnl dnl Waits for ALB logs
> for 'param' in logs and checks if value matches dnl 'value'. Checking starts
> from line number 'line' in ovs-vswithd.log.
> m4_define([CHECK_ALB_PARAM], [
> line_st=$3
> if [[ -z "$line_st" ]]
> then
> line_st="+0"
> fi
> OVS_WAIT_UNTIL([tail -n $line_st ovs-vswitchd.log | grep "PMD auto load
> balance $1 set to"])
> AT_CHECK([tail -n $line_st ovs-vswitchd.log | sed -n "s#.*\(PMD auto load
> balance $1 set to.*\)#\1#p" | tail -1], [0], [dnl PMD auto load balance $1 set
> to $2
> ])
> ])
> 

I agree with David on this one. It might be good to create a helper like 
CHECK_LOAD_PARAM.

> 
> 
> > +AT_SETUP([ALB - interval param])
> > +OVS_VSWITCHD_START
> > +OVS_WAIT_UNTIL([grep "PMD auto load balance is disabled"
> > +ovs-vswitchd.log])
> > +
> > +# Check default
> > +CHECK_INTERVAL_PARAM([1], [])
> 
> Here, it becomes:
> CHECK_ALB_PARAM([interval], [1 mins], [])
> 
> etc..
> 
> [snip]
> 
> > +# TODO 2 is documented as max value but it seems arbitary #
> > +Setting to 20001 works, should it be checked and rejected?
> > +# For now add a dummy test above the max value that accepts it
> 
> Afaiu, from ovsdb pov, other_config is a simple map of strings with no
> constraint on the content of the strings.
> I understand the constraints expressed for sub fields like pmd-auto-lb-XX
> parameters in vswitchd/vswitchd.xml as only for documentation.
> 
> I don't see the benefit of checking values 2 or 20001.

I couldn't find the doc where the max value is mentioned.
Would you mind pointing it ?
Or its calculated somehow ? 

The rest looks to good to me!


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

Re: [ovs-dev] [PATCH v2 2/2] ovsdb-idl: Preserve references for deleted rows.

[Dumitru Ceara]

On 3/10/21 9:33 AM, Han Zhou wrote:



On Tue, Mar 9, 2021 at 9:03 AM Dumitru Ceara > wrote:

 >
 > On 3/9/21 5:56 PM, Ilya Maximets wrote:
 > > On 3/9/21 5:54 PM, Ilya Maximets wrote:
 > >> On 3/9/21 5:26 PM, Dumitru Ceara wrote:
 > >>> On 3/9/21 4:07 PM, Ilya Maximets wrote:
 >  On 3/3/21 3:40 PM, Dumitru Ceara wrote:
 > > Considering two DB rows, 'a' from table A and 'b' from table B 
(with

 > > column 'ref_a' a reference to table A):
 > > a = {A._uuid=}
 > > b = {B._uuid=, B.ref_a=}
 > >
 > > Assuming both records are present in the IDL client's in-memory 
view of

 > > the database, depending whether row 'b' is also deleted in the same
 > > transaction or not, deletion of row 'a' should generate the 
following

 > > tracked changes:
 > >
 > > 1. only row 'a' is deleted:
 > > - for table A:
 > >     - deleted records: a = {A._uuid=}
 > > - for table B:
 > >     - updated records: b = {B._uuid=, B.ref_a=[]}
 > >
 > > 2. row 'a' and row 'b' are deleted in the same update:
 > > - for table A:
 > >     - deleted records: a = {A._uuid=}
 > > - for table B:
 > >     - deleted records: b = {B._uuid=, B.ref_a=}
 > >
 > > To ensure this, we now delay reparsing row backrefs until the 
row has

 > > been removed from the table's hmap and until the IDL client has
 > > processed all tracked changes (ovsdb_idl_track_clear() was called).
 > >
 > > Without this change, in scenario 2 above, the tracked changes 
for table

 > > B would be:
 > > - deleted records: b = {B._uuid=, B.ref_a=[]}
 > >
 > > In particular, for strong references, row 'a' can never be 
deleted in
 > > a transaction that happens strictly before row 'b' is deleted.  
In some

 > > cases [0] both rows are deleted in the same transaction and having
 > > B.ref_a=[] would violate the integrity of the database from client
 > > perspective.  This would force the client to always validate that
 > > strong reference fields are non-NULL.  This is not really an option
 > > because the information in the original reference is required for
 > > incrementally processing the record deletion.
 > >
 > > [0] with ovn-monitor-all=true, the following command triggers a 
crash
 > >       in ovn-controller because a strong reference field 
becomes NULL:
 > >       $ ovn-nbctl --wait=hv -- lr-add r -- lrp-add r rp 
00:00:00:00:00:01 1.0.0.1/24 

 > >       $ ovn-nbctl lr-del r
 > >
 > > Reported-at: https://bugzilla.redhat.com/1932642 

 > > Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for 
deleted rows.")
 > > Signed-off-by: Dumitru Ceara >

 > > ---
 > > v2:
 > >     - Added ovsdb-idl.at  test for strong 
references.

 > > ---
 > >    lib/ovsdb-idl.c    |    6 ++-
 > >    tests/ovsdb-idl.at  |  118 


 > >    tests/test-ovsdb.c |   45 
 > >    3 files changed, 167 insertions(+), 2 deletions(-)
 > >
 > > diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
 > > index 9e1e787..ecd4924 100644
 > > --- a/lib/ovsdb-idl.c
 > > +++ b/lib/ovsdb-idl.c
 > > @@ -163,6 +163,7 @@ static void ovsdb_idl_row_unparse(struct 
ovsdb_idl_row *);

 > >    static void ovsdb_idl_row_clear_old(struct ovsdb_idl_row *);
 > >    static void ovsdb_idl_row_clear_new(struct ovsdb_idl_row *);
 > >    static void ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *, 
bool destroy_dsts);
 > > +static void ovsdb_idl_row_reparse_backrefs(struct 
ovsdb_idl_row *row);

 > >      static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *);
 > >    static bool ovsdb_idl_txn_extract_mutations(struct 
ovsdb_idl_row *,

 > > @@ -367,6 +368,8 @@ ovsdb_idl_clear(struct ovsdb_idl *db)
 > >                    ovsdb_idl_row_unparse(row);
 > >                }
 > >                LIST_FOR_EACH_SAFE (arc, next_arc, src_node, 
&row->src_arcs) {

 > > +                ovs_list_remove(&arc->src_node);
 > > +                ovs_list_remove(&arc->dst_node);
 > >                    free(arc);
 > >                }
 > >                /* No need to do anything with dst_arcs: some 
node has those arcs
 > > @@ -1234,6 +1237,7 @@ ovsdb_idl_track_clear__(struct ovsdb_idl 
*idl, bool flush_all)

 > >                    ovs_list_remove(&row->track_node);
 > >                    ovs_list_init(&row->track_node);
 > >                    if (ovsdb_idl_row_is_orphan(row)) {
 > > +                    ovsdb_idl_row_reparse_backrefs(row);
 > >                        ovsdb_idl_row_unparse(row);
 > >                        if (row->tracked_old_datum) {
 > >                            cons

[ovs-dev] [PATCH v3 3/3] ovsdb-idl: Mark arc sources as updated when destination is deleted.

[Dumitru Ceara]

Considering two DB rows, 'a' from table A and 'b' from table B (with
column 'ref_a' a reference to table A):
a = {A._uuid=}
b = {B._uuid=, B.ref_a=}

When the IDL client processes an update that deletes row 'a', row 'b'
is also marked as 'updated' if change tracking is enabled for table B.

Fixes: 102781cc02c6 ("ovsdb-idl: Track changes for table references.")
Signed-off-by: Dumitru Ceara 
---
 lib/ovsdb-idl.c|3 +++
 tests/ovsdb-idl.at |1 +
 2 files changed, 4 insertions(+)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 1542da9..9f8378a 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -196,6 +196,8 @@ static void ovsdb_idl_remove_from_indexes(const struct 
ovsdb_idl_row *);
 static int ovsdb_idl_try_commit_loop_txn(struct ovsdb_idl_loop *loop,
  bool *may_need_wakeup);
 
+static void add_tracked_change_for_references(struct ovsdb_idl_row *);
+
 /* Creates and returns a connection to database 'remote', which should be in a
  * form acceptable to jsonrpc_session_open().  The connection will maintain an
  * in-memory replica of the remote database whose schema is described by
@@ -1380,6 +1382,7 @@ ovsdb_idl_process_orphans(struct ovsdb_idl *db)
 
 LIST_FOR_EACH_SAFE (row, next, track_node, &db->orphan_rows) {
 ovsdb_idl_row_untrack_change(row);
+add_tracked_change_for_references(row);
 ovsdb_idl_row_reparse_backrefs(row);
 
 /* Orphan rows that are still unreferenced or are part of tables that
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 7ac3c20..38f1020 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1286,6 +1286,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially 
populated, orphan rows, cond
 003: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] 
ba=[] sa=[] ua=[] uuid=<1>
 003: table simple: updated columns: s
 004: change conditions
+005: table simple6: name=first_row weak_ref=[] uuid=<0>
 005: table simple: deleted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] 
ba=[] sa=[] ua=[] uuid=<1>
 005: table simple: inserted row: i=0 r=0 b=false s=row1_s u=<2> ia=[] ra=[] 
ba=[] sa=[] ua=[] uuid=<3>
 005: table simple: updated columns: s

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

[ovs-dev] [PATCH v3 2/3] ovsdb-idl: Preserve references for deleted rows.

[Dumitru Ceara]

Considering two DB rows, 'a' from table A and 'b' from table B (with
column 'ref_a' a reference to table A):
a = {A._uuid=}
b = {B._uuid=, B.ref_a=}

Assuming both records are present in the IDL client's in-memory view of
the database, depending whether row 'b' is also deleted in the same
transaction or not, deletion of row 'a' should generate the following
tracked changes:

1. only row 'a' is deleted:
- for table A:
  - deleted records: a = {A._uuid=}
- for table B:
  - updated records: b = {B._uuid=, B.ref_a=[]}

2. row 'a' and row 'b' are deleted in the same update:
- for table A:
  - deleted records: a = {A._uuid=}
- for table B:
  - deleted records: b = {B._uuid=, B.ref_a=}

To ensure this, we now delay reparsing row backrefs for deleted rows
until all updates in the current run have been processed.

Without this change, in scenario 2 above, the tracked changes for table
B would be:
- deleted records: b = {B._uuid=, B.ref_a=[]}

In particular, for strong references, row 'a' can never be deleted in
a transaction that happens strictly before row 'b' is deleted.  In some
cases [0] both rows are deleted in the same transaction and having
B.ref_a=[] would violate the integrity of the database from client
perspective.  This would force the client to always validate that
strong reference fields are non-NULL.  This is not really an option
because the information in the original reference is required for
incrementally processing the record deletion.

[0] with ovn-monitor-all=true, the following command triggers a crash
in ovn-controller because a strong reference field becomes NULL:
$ ovn-nbctl --wait=hv -- lr-add r -- lrp-add r rp 00:00:00:00:00:01 
1.0.0.1/24
$ ovn-nbctl lr-del r

Reported-at: https://bugzilla.redhat.com/1932642
Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted rows.")
Signed-off-by: Dumitru Ceara 
---
 lib/ovsdb-idl.c |  135 ++
 tests/ovsdb-idl.at  |  203 +++
 tests/test-ovsdb.c  |   50 +
 tests/test-ovsdb.py |   14 
 4 files changed, 370 insertions(+), 32 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9e1e787..1542da9 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -92,6 +92,7 @@ struct ovsdb_idl {
 struct ovsdb_idl_txn *txn;
 struct hmap outstanding_txns;
 bool verify_write_only;
+struct ovs_list orphan_rows; /* All deleted but still referenced rows. */
 };
 
 static struct ovsdb_cs_ops ovsdb_idl_cs_ops;
@@ -144,6 +145,7 @@ static bool ovsdb_idl_modify_row(struct ovsdb_idl_row *,
  const struct shash *values, bool xor);
 static void ovsdb_idl_parse_update(struct ovsdb_idl *,
const struct ovsdb_cs_update_event *);
+static void ovsdb_idl_process_orphans(struct ovsdb_idl *);
 
 static void ovsdb_idl_txn_process_reply(struct ovsdb_idl *,
 const struct jsonrpc_msg *);
@@ -163,6 +165,10 @@ static void ovsdb_idl_row_unparse(struct ovsdb_idl_row *);
 static void ovsdb_idl_row_clear_old(struct ovsdb_idl_row *);
 static void ovsdb_idl_row_clear_new(struct ovsdb_idl_row *);
 static void ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *, bool 
destroy_dsts);
+static void ovsdb_idl_row_reparse_backrefs(struct ovsdb_idl_row *);
+static void ovsdb_idl_row_track_change(struct ovsdb_idl_row *,
+   enum ovsdb_idl_change);
+static void ovsdb_idl_row_untrack_change(struct ovsdb_idl_row *);
 
 static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *);
 static bool ovsdb_idl_txn_extract_mutations(struct ovsdb_idl_row *,
@@ -248,6 +254,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class 
*class,
 .txn = NULL,
 .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns),
 .verify_write_only = false,
+.orphan_rows = OVS_LIST_INITIALIZER(&idl->orphan_rows),
 };
 
 uint8_t default_mode = (monitor_everything_by_default
@@ -351,6 +358,12 @@ ovsdb_idl_set_leader_only(struct ovsdb_idl *idl, bool 
leader_only)
 static void
 ovsdb_idl_clear(struct ovsdb_idl *db)
 {
+/* Process orphan rows, removing them from the 'orphan_rows' list. */
+ovsdb_idl_process_orphans(db);
+
+/* Cleanup all rows; each row gets added to its own table's
+ * 'track_list'.
+ */
 for (size_t i = 0; i < db->class_->n_tables; i++) {
 struct ovsdb_idl_table *table = &db->tables[i];
 struct ovsdb_idl_row *row, *next_row;
@@ -367,17 +380,26 @@ ovsdb_idl_clear(struct ovsdb_idl *db)
 ovsdb_idl_row_unparse(row);
 }
 LIST_FOR_EACH_SAFE (arc, next_arc, src_node, &row->src_arcs) {
+ovs_list_remove(&arc->src_node);
+ovs_list_remove(&arc->dst_node);
+free(arc);
+}
+LIST_FOR_EACH_SAFE (arc, next_arc, dst_node, &row->dst_arcs) {
+ovs_list_r

[ovs-dev] [PATCH v3 1/3] ovsdb-idl.at: Make test outputs more predictable.

[Dumitru Ceara]

IDL tests need predictable output from test-ovsdb.

This used to be done by first sorting the output of test-ovsdb and then
applying uuidfilt to predictably translate UUIDs.  This was not
reliable enough in case test-ovsdb processes two or more insert/delete
operations in the same iteration because the order of lines in the
output depends on the automatically generated UUID values.

To fix this we change the way test-ovsdb and test-ovsdb.py generate
outputs and prepend the table name and tracking information before
printing the contents of a row.

All existing ovsdb-idl.at and ovsdb-cluster.at tests are updated to
expect the new output format.

Signed-off-by: Dumitru Ceara 
---
Note: the old approach was enough for outputs of the existing tests but
the next patch in this series adds a new test that requires this
change.

v3:
- Changed expected output of ovsdb-cluster.at to reflect the new
  formatting in test-ovsdb output.
- Fixed typo in test-ovsdb.py.
v2:
- Reworked the patch and changed test-ovsdb.c and test-ovsdb.py to
  generate output that can be sorted predictably.
- Rephrased commit message.
---
 lib/ovsdb-idl.c|3 
 lib/ovsdb-idl.h|2 
 tests/ovsdb-cluster.at |2 
 tests/ovsdb-idl.at |  463 +++-
 tests/test-ovsdb.c |  186 +++
 tests/test-ovsdb.py|   87 +
 6 files changed, 390 insertions(+), 353 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 2c8a0c9..9e1e787 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -182,7 +182,6 @@ ovsdb_idl_table_from_class(const struct ovsdb_idl *,
 static struct ovsdb_idl_table *
 ovsdb_idl_table_from_class(const struct ovsdb_idl *,
const struct ovsdb_idl_table_class *);
-static bool ovsdb_idl_track_is_set(struct ovsdb_idl_table *table);
 static void ovsdb_idl_track_clear__(struct ovsdb_idl *, bool flush_all);
 
 static void ovsdb_idl_destroy_indexes(struct ovsdb_idl_table *);
@@ -1140,7 +1139,7 @@ ovsdb_idl_track_add_all(struct ovsdb_idl *idl)
 }
 
 /* Returns true if 'table' has any tracked column. */
-static bool
+bool
 ovsdb_idl_track_is_set(struct ovsdb_idl_table *table)
 {
 size_t i;
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 05bb48d..d934832 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -53,6 +53,7 @@ struct ovsdb_datum;
 struct ovsdb_idl_class;
 struct ovsdb_idl_row;
 struct ovsdb_idl_column;
+struct ovsdb_idl_table;
 struct ovsdb_idl_table_class;
 struct uuid;
 
@@ -217,6 +218,7 @@ unsigned int ovsdb_idl_row_get_seqno(
 void ovsdb_idl_track_add_column(struct ovsdb_idl *idl,
 const struct ovsdb_idl_column *column);
 void ovsdb_idl_track_add_all(struct ovsdb_idl *idl);
+bool ovsdb_idl_track_is_set(struct ovsdb_idl_table *table);
 const struct ovsdb_idl_row *ovsdb_idl_track_get_first(
 const struct ovsdb_idl *, const struct ovsdb_idl_table_class *);
 const struct ovsdb_idl_row *ovsdb_idl_track_get_next(const struct 
ovsdb_idl_row *);
diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index 92aa427..cf43e9c 100644
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -128,7 +128,7 @@ ovsdb_test_cluster_disconnect () {
"rows": [{"i": 1}]}]]' > test-ovsdb.log 2>&1 &
 echo $! > test-ovsdb.pid
 
-OVS_WAIT_UNTIL([grep "000: i=1" test-ovsdb.log])
+OVS_WAIT_UNTIL([grep "000: table simple: i=1" test-ovsdb.log])
 
 # Start collecting raft_is_connected logs for $target before shutting down
 # any servers.
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 4b4791a..610680c 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -355,28 +355,28 @@ OVSDB_CHECK_IDL([simple idl, initially empty, various 
ops],
 'reconnect']],
   [[000: empty
 001: {"error":null,"result":[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]}]}
-002: i=0 r=0 b=false s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
-002: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc 
def] ua=[<4> <5>] uuid=<0>
+002: table simple: i=0 r=0 b=false s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] 
uuid=<1>
+002: table simple: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] 
ba=[true] sa=[abc def] ua=[<4> <5>] uuid=<0>
 003: {"error":null,"result":[{"count":2}]}
-004: i=0 r=0 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
-004: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] sa=[abc 
def] ua=[<4> <5>] uuid=<0>
+004: table simple: i=0 r=0 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] 
uuid=<1>
+004: table simple: i=1 r=2 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] 
ba=[true] sa=[abc def] ua=[<4> <5>] uuid=<0>
 005: {"error":null,"result":[{"count":2}]}
-006: i=0 r=123.5 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1>
-006: i=1 r=123.5 b=true s=mystring u=<3> ia=[1 2 3] ra=[-0.5] ba=[true] 
sa=[abc def] ua=[<4> <5>] uuid=<0>
+006: table simple: i=0 r=123.5 b=true s= u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] 
u

[ovs-dev] [PATCH v3 0/3] ovsdb-idl: Preserve references for tracked deleted rows.

[Dumitru Ceara]

Patch 1/3 of the series makes the ovsdb-idl tests more future proof
by trying to ensure more predictable output from test-ovsdb.

Paches 2/3 and 3/3 fix problems in the IDL change tracking code.

Changes in v3:
- Patch 1/3:
  - Changed expected output of ovsdb-cluster.at to reflect the new
formatting in test-ovsdb output.
  - Fixed typo in test-ovsdb.py.
- Patch 2/3:
  - Rework based on the discussion with Ilya.
  - Added more tests.
- Add patch 3/3:
  - Mark reference sources as "udpated" when destinations are deleted.

Changes in v2:
- Patch 1/2:
  - reworked the patch to improve the output of test-ovsdb.c and
test-ovsdb.py themselves.
- Patch 2/2:
  - added a test for strong references.

Dumitru Ceara (3):
  ovsdb-idl.at: Make test outputs more predictable.
  ovsdb-idl: Preserve references for deleted rows.
  ovsdb-idl: Mark arc sources as updated when destination is deleted.


 lib/ovsdb-idl.c|  141 --
 lib/ovsdb-idl.h|2 
 tests/ovsdb-cluster.at |2 
 tests/ovsdb-idl.at |  667 +++-
 tests/test-ovsdb.c |  236 -
 tests/test-ovsdb.py|  101 +--
 6 files changed, 764 insertions(+), 385 deletions(-)

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

[ovs-dev] [PATCH v2 2/2] testsuite: add test cases for ingress_policing parameters

[Simon Horman]

From: Tianyu Yuan 

* Exercise OVS setting of ingress_policing parameters using ovs-vsctl and
  verify that the correct values are stored on OVSDB.
* Verify the ingress_policing parameters with tc command. Also check
  offload and non-offload in tc software datapath based on tc filter type
  (matchall and basic).

Example invocation:
make check TESTSUITEFLAGS='-k ingress_policing'
make check-offloads TESTSUITEFLAGS='-k ingress_policing'

Signed-off-by: Tianyu Yuan 
Signed-off-by: Simon Horman 
Signed-off-by: Louis Peens 
---
 tests/ovs-vsctl.at   | 58 
 tests/system-offloads-traffic.at | 47 ++
 2 files changed, 105 insertions(+)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index d2cb41403..790d46f18 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1659,3 +1659,61 @@ AT_CHECK([grep "server name" ovsdb-server.log], [0],
 
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
+
+dnl --
+AT_BANNER([set ingress policing test])
+
+AT_SETUP([set ingress_policing_rate and ingress_policing_burst])
+AT_KEYWORDS([ingress_policing])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
+   [add-br a],
+   [add-port a a1],
+   [set interface a1 ingress_policing_rate=100],
+   [set interface a1 ingress_policing_burst=10],
+   [list interface a1 > configured_interface_a1.txt])],
+   [0], [])
+AT_CHECK(
+[uuidfilt configured_interface_a1.txt],
+[0],
+[[
+
+
+
+_uuid   : <0>
+admin_state : []
+bfd : {}
+bfd_status  : {}
+cfm_fault   : []
+cfm_fault_status: []
+cfm_flap_count  : []
+cfm_health  : []
+cfm_mpid: []
+cfm_remote_mpids: []
+cfm_remote_opstate  : []
+duplex  : []
+error   : []
+external_ids: {}
+ifindex : []
+ingress_policing_burst: 10
+ingress_policing_rate: 100
+lacp_current: []
+link_resets : []
+link_speed  : []
+link_state  : []
+lldp: {}
+mac : []
+mac_in_use  : []
+mtu : []
+mtu_request : []
+name: a1
+ofport  : []
+ofport_request  : []
+options : {}
+other_config: {}
+statistics  : {}
+status  : {}
+type: ""
+]])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index 4f601ef93..b9cd29c95 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -70,3 +70,50 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows 
: [[1-9]]"], [0], [i
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - 
offloads disabled])
+AT_KEYWORDS([ingress_policing])
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=false])
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+ADD_NAMESPACES(at_ns0)
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+AT_CHECK([ovs-vsctl set interface ovs-p0 ingress_policing_rate=100])
+AT_CHECK([ovs-vsctl set interface ovs-p0 ingress_policing_burst=10])
+AT_CHECK([ovs-vsctl list open | grep other_config > other_config.txt])
+AT_CHECK([cat other_config.txt],[0],
+[other_config: {hw-offload="false"}
+])
+AT_CHECK([tc -s -d filter show dev ovs-p0 ingress |grep rate| awk 
'{a=index($0,"rate");b=index($0,"mtu");print substr($0,a,b-a-1)}' > 
tc_ovs-p0.txt ],[0],[])
+AT_CHECK([cat tc_ovs-p0.txt],[0],
+[rate 100Kbit burst 1280b
+])
+AT_CHECK([tc -s -d filter show dev ovs-p0 ingress | grep basic | head -n 1 | 
awk '{a=index($0,"basic");print substr($0,a,5)}'| head -n 1],[0],
+[basic
+])
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
+
+AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - 
offloads enabled])
+AT_KEYWORDS([ingress_policing])
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+ADD_NAMESPACES(at_ns0)
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+AT_CHECK([ovs-vsctl set interface ovs-p0 ingress_policing_rate=100])
+AT_CHECK([ovs-vsctl set interface ovs-p0 ingress_policing_burst=10])
+AT_CHECK([ovs-vsctl list open | grep other_config > other_config.txt])
+AT_CHECK([cat other_config.txt],[0],
+[other_config: {hw-offload="true"}
+])
+AT_CHECK([tc -s -d filter show dev ovs-p0 ingress | grep rate| awk 
'{a=index($0,"rate");b=index($0,"mtu");print substr($0,a,b-a-1)}' > 
tc_ovs-p0.txt ],[0],[])
+AT_CHECK([cat tc_ovs-p0.txt],[0],
+[rate 100Kbit burst 1280b
+])
+AT_CHECK([tc -s -d filter show dev ovs-p0 ingress | grep matchall | head -n 1 
| awk '{a=index($0,"matchall");print substr($0,a,8)}'| head -n 1],[0],
+[matchall
+])
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
-- 
2.20.1

[ovs-dev] [PATCH v2 1/2] netdev-linux: correct unit of burst parameter

[Simon Horman]

From: "Yong.Xu" 

Correct calculation of burst parameter used when configuring TC policer
action for ingress port-based policing in the case where TC offload is in
use. This now matches the value calculated for the case where TC offload is
not in use.

The division by 8 is to convert from bits to bytes.
Its unclear why 64 was previously used.

Fixes: e7f6ba220 ("lib/tc: add ingress ratelimiting support for tc-offload")
Signed-off-by: Yong.Xu 
[simon: reworked changelog]
Signed-off-by: Simon Horman 
Signed-off-by: Louis Peens 
---
 lib/netdev-linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 15b25084b..f87a20075 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2572,7 +2572,7 @@ exit:
 static struct tc_police
 tc_matchall_fill_police(uint32_t kbits_rate, uint32_t kbits_burst)
 {
-unsigned int bsize = MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 64;
+unsigned int bsize = MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8;
 unsigned int bps = ((uint64_t) kbits_rate * 1000) / 8;
 struct tc_police police;
 struct tc_ratespec rate;
-- 
2.20.1

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

[ovs-dev] [PATCH v2 0/2] netdev-linux: correct unit of burst parameter

[Simon Horman]

Hi,

this short series corrects the unit of the burst parameter used to
configure TC policer actions in the case case where ingress rate limiting
is configured when TC flow offload is enabled.

Patch: 1/2
Change since v1: minor update to changelog

Adds tests for the configuration of rate limiting.

Patches 2/2
New in v2


Tianyu Yuan (1):
  testsuite: add test cases for ingress_policing parameters

Yong.Xu (1):
  netdev-linux: correct unit of burst parameter

 lib/netdev-linux.c   |  2 +-
 tests/ovs-vsctl.at   | 58 
 tests/system-offloads-traffic.at | 47 ++
 3 files changed, 106 insertions(+), 1 deletion(-)

-- 
2.20.1

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

Re: [ovs-dev] [PATCH ovn] ovs: Bump submodule version to latest ovsdb-cs changes.

[Ilya Maximets]

On 3/12/21 7:42 AM, Han Zhou wrote:
> 
> 
> On Thu, Mar 11, 2021 at 6:11 PM Ben Pfaff  > wrote:
>>
>> On Thu, Mar 11, 2021 at 08:07:58PM -0500, Mark Michelson wrote:
>> > On 3/11/21 5:58 PM, Ilya Maximets wrote:
>> > > I'd say that eventually OVN should become less feature hungry
>> > > or less dependent from OVS changes.  Then we should consider
>> > > to use only stable released OVS as a build dependency and
>> > > actually wait for the next major OVS release if some new features
>> > > required from it.  But right now we can't afford that.
>> >
>> > I'd say the vast majority of the time these days, the OVS changes OVN
>> > depends on are specifically OVSDB changes. One possible idea is to separate
>> > OVSDB from OVS and semantically version the OVSDB API. Then OVS and OVN
>> > would be consumers of the same common project.
>> >
>> > You'd still have the potential for needing to update the OVS submodule to
>> > some non-released version. But hopefully the frequency of such updates 
>> > would
>> > be reduced. And if the frequency is reduced enough, it probably wouldn't be
>> > outside the realm of reason to request backports of bug fixes and
>> > point-releases of OVS whenever OVN requires a submodule update.
>>
>> I'm adding Dmitry Yusupov to this thread because he expressed some
>> interest in separating OVSDB from OVS in the latest OVS Orbit episode
>> (https://ovsorbit.org/#e72 ).
> 
> I think this is not only a dependency problem of compiling, but also of 
> runtime, if we want to continue supporting debian, because in debian packages 
> dynamic linked library is used instead of statically linked. If I understand 
> correctly, the submodule approach solves the compile time dependency but not 
> runtime, for debian. So the only way out eventually might be separating OVSDB 
> from OVS. (Please correct me if I am wrong)
> 

OVS doesn't export too much in public libraries.  However, splitting
out OVSDB is not just splitting OVSDB itself.  It's he same as  splitting
OVN.  OVSDB parts just like OVN depends on all the internal non-exported
data structures and utility functions. So, there are several ways to
handle that:

a. OVN way: Keep data structures and stuff inside OVS sources and have
   OVS as a build dependency for both OVN and OVSDB.
   This way we'll have all the same issues with OVSDB as we have with
   OVN.  We'll end up creating an OVS submodule for OVSDB or something
   similar.  Will have to synchronize releases of 3 separate projects
   at the same time.

b. Split out all libraries to a 4th project and maintain it separately.
   In this case we will have to maintain public API/ABI compatibility
   for libraries, version all the APIs and waste lots of time maintaining
   while loosing a flexibility and speed of modification or addition of
   new functions or libraries.

c. Copy out all the libraries to all dependent projects.
   This will bring lots of code duplication across 3 different projects
   and consequent requirement to track all changes/fixes and port to other
   projects.

All 3 options above looks to me more like a maintainers' hell.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev