Re: [ovs-dev] iperf tcp issue on veth using afxdp

[Yifeng Sun]

This seems to be related to netdev-afxdp's batch size bigger than
kernel's xdp batch size.
I created a patch to fix it.

https://patchwork.ozlabs.org/patch/1214397/

Could anyone take a look at this patch?

Thanks,
Yifeng

On Fri, Nov 22, 2019 at 9:52 AM William Tu  wrote:
>
> Hi Ilya and Eelco,
>
> Yiyang reports very poor TCP performance on his setup and I can
> also reproduce it on my machine. Somehow I think this might be a
> kernel issue, but I don't know where to debug this. Need your suggestion
> about how to debug.
>
> So the setup is like the system-traffic, creating 2 namespaces and
> veth devices and attach to OVS. I do remember to turn off tx offload
> and ping, UDP, nc (tcp-mode) works fine.
>
> TCP using iperf drops to 0Mbps after 4 seconds.
> At server side:
> root@osboxes:~/ovs# ip netns exec at_ns0 iperf -s
> 
> Server listening on TCP port 5001
> TCP window size:  128 KByte (default)
> 
> [  4] local 10.1.1.1 port 5001 connected with 10.1.1.2 port 40384
> Waiting for server threads to complete. Interrupt again to force quit.
>
> At client side
> root@osboxes:~/bpf-next# ip netns exec at_ns1 iperf -c 10.1.1.1 -i 1 -t 10
> 
> Client connecting to 10.1.1.1, TCP port 5001
> TCP window size: 85.0 KByte (default)
> 
> [  3] local 10.1.1.2 port 40384 connected with 10.1.1.1 port 5001
> [ ID] Interval   Transfer Bandwidth
> [  3]  0.0- 1.0 sec  17.0 MBytes   143 Mbits/sec
> [  3]  1.0- 2.0 sec  9.62 MBytes  80.7 Mbits/sec
> [  3]  2.0- 3.0 sec  6.75 MBytes  56.6 Mbits/sec
> [  3]  3.0- 4.0 sec  11.0 MBytes  92.3 Mbits/sec
> [  3]  5.0- 6.0 sec  0.00 Bytes  0.00 bits/sec
> [  3]  6.0- 7.0 sec  0.00 Bytes  0.00 bits/sec
> [  3]  7.0- 8.0 sec  0.00 Bytes  0.00 bits/sec
> [  3]  8.0- 9.0 sec  0.00 Bytes  0.00 bits/sec
> [  3]  9.0-10.0 sec  0.00 Bytes  0.00 bits/sec
> [  3] 10.0-11.0 sec  0.00 Bytes  0.00 bits/sec
>
> (after this, even ping stops working)
>
> Script to reproduce
> -
> ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
>
> ip netns add at_ns0
> ip link add p0 type veth peer name afxdp-p0
> ip link set p0 netns at_ns0
> ip link set dev afxdp-p0 up
> ovs-vsctl add-port br0 afxdp-p0
>
> ovs-vsctl -- set interface afxdp-p0 options:n_rxq=1 type="afxdp"
> options:xdp-mode=native
> ip netns exec at_ns0 sh << NS_EXEC_HEREDOC
> ip addr add "10.1.1.1/24" dev p0
> ip link set dev p0 up
> NS_EXEC_HEREDOC
>
> ip netns add at_ns1
> ip link add p1 type veth peer name afxdp-p1
> ip link set p1 netns at_ns1
> ip link set dev afxdp-p1 up
> ovs-vsctl add-port br0 afxdp-p1 -- \
>set interface afxdp-p1 options:n_rxq=1 type="afxdp"
> options:xdp-mode=native
>
> ip netns exec at_ns1 sh << NS_EXEC_HEREDOC
> ip addr add "10.1.1.2/24" dev p1
> ip link set dev p1 up
> NS_EXEC_HEREDOC
>
> ethtool -K afxdp-p0 tx off
> ethtool -K afxdp-p1 tx off
> ip netns exec at_ns0 ethtool -K p0 tx off
> ip netns exec at_ns1 ethtool -K p1 tx off
>
> ip netns exec at_ns0 ping  -c 10 -i .2 10.1.1.2
> echo "ip netns exec at_ns1 iperf -c 10.1.1.1 -i 1 -t 10"
> ip netns exec at_ns0 iperf -s
>
> Thank you
> William
> ___
> 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] afxdp: Reduce afxdp's batch size to match kernel's xdp batch size

[Yifeng Sun]

William reported that there is iperf TCP issue between two afxdp ports:

[  3] local 10.1.1.2 port 40384 connected with 10.1.1.1 port 5001
[ ID] Interval   Transfer Bandwidth
[  3]  0.0- 1.0 sec  17.0 MBytes   143 Mbits/sec
[  3]  1.0- 2.0 sec  9.62 MBytes  80.7 Mbits/sec
[  3]  2.0- 3.0 sec  6.75 MBytes  56.6 Mbits/sec
[  3]  3.0- 4.0 sec  11.0 MBytes  92.3 Mbits/sec
[  3]  5.0- 6.0 sec  0.00 Bytes  0.00 bits/sec
[  3]  6.0- 7.0 sec  0.00 Bytes  0.00 bits/sec
[  3]  7.0- 8.0 sec  0.00 Bytes  0.00 bits/sec
[  3]  8.0- 9.0 sec  0.00 Bytes  0.00 bits/sec
[  3]  9.0-10.0 sec  0.00 Bytes  0.00 bits/sec
[  3] 10.0-11.0 sec  0.00 Bytes  0.00 bits/sec

The reason is, currently, netdev-afxdp's batch size is 32 while kernel's
xdp batch size is only 16. This can result in exhausting of sock wmem if
netdev-afxdp keeps sending large number of packets. Later on, when ARP
expires at one side of TCP connection, ARP packets can be delayed or
even dropped because sock wmen is already full.

This patch fixes this issue by reducing netdev-afxdp's batch size so
as to match kernel's xdp batch size. Now iperf TCP works correctly.

[  3] local 10.1.1.2 port 57770 connected with 10.1.1.1 port 5001
[ ID] Interval   Transfer Bandwidth
[  3]  0.0- 1.0 sec   262 MBytes  2.20 Gbits/sec
[  3]  1.0- 2.0 sec   299 MBytes  2.51 Gbits/sec
[  3]  2.0- 3.0 sec   271 MBytes  2.27 Gbits/sec
[  3]  3.0- 4.0 sec   247 MBytes  2.07 Gbits/sec
[  3]  4.0- 5.0 sec   290 MBytes  2.43 Gbits/sec
[  3]  5.0- 6.0 sec   292 MBytes  2.45 Gbits/sec
[  3]  6.0- 7.0 sec   223 MBytes  1.87 Gbits/sec
[  3]  7.0- 8.0 sec   243 MBytes  2.04 Gbits/sec
[  3]  8.0- 9.0 sec   234 MBytes  1.97 Gbits/sec
[  3]  9.0-10.0 sec   238 MBytes  2.00 Gbits/sec

Reported-by: William Tu 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-dev/2019-November/365076.html
Signed-off-by: Yifeng Sun 
---
 lib/netdev-afxdp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index 58365ed483e3..38bbbeb055cc 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -82,7 +82,7 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 
20);
  * enough for most corner cases.
  */
 #define NUM_FRAMES  (4 * (PROD_NUM_DESCS + CONS_NUM_DESCS))
-#define BATCH_SIZE  NETDEV_MAX_BURST
+#define BATCH_SIZE  16
 
 BUILD_ASSERT_DECL(IS_POW2(NUM_FRAMES));
 BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
-- 
2.7.4

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

Re: [ovs-dev] [PATCH] Use batch process recv for tap and raw socket in netdev datapath

[William Tu]

On Wed, Dec 18, 2019 at 10:44:21AM +0800, yang_y_yi wrote:
> Hi, William
> 
> 
> I used OVS DPDK to test it, you shouldn't add tap interface to ovs DPDK 
> bridge if you use vdev to add tap, virtio_user is just for it, but that won't 
> use this receive function to receive packets.

Right.
I mean if you already use OVS-DPDK, you can create tap device using s.t like

ovs-vsctl -- set interface dpdk-p0 type=dpdk \
 options:dpdk-devargs=vdev:net_af_packet0,iface=dpdk-p0

Then you can get better veth performance around 2.3Gbps, without your patch.

William

> 
> At 2019-12-17 02:55:50, "William Tu"  wrote:
> >On Fri, Dec 06, 2019 at 02:09:24AM -0500, yang_y...@163.com wrote:
> >> From: Yi Yang 
> >> 
> >> Current netdev_linux_rxq_recv_tap and netdev_linux_rxq_recv_sock
> >> just receive single packet, that is very inefficient, per my test
> >> case which adds two tap ports or veth ports into OVS bridge
> >> (datapath_type=netdev) and use iperf3 to do performance test
> >> between two ports (they are set into different network name space).
> >> 
> >> The result is as below:
> >> 
> >>   tap:  295 Mbits/sec
> >>   veth: 207 Mbits/sec
> >> 
> >> After I change netdev_linux_rxq_recv_tap and
> >> netdev_linux_rxq_recv_sock to use batch process, the performance
> >> is boosted by about 7 times, here is the result:
> >> 
> >>   tap:  1.96 Gbits/sec
> >>   veth: 1.47 Gbits/sec
> >> 
> >> Undoubtedly this is a huge improvement although it can't match
> >> OVS kernel datapath yet.
> >> 
> >> FYI: here is thr result for OVS kernel datapath:
> >> 
> >>   tap:  37.2 Gbits/sec
> >>   veth: 36.3 Gbits/sec
> >> 
> >> Note: performance result is highly related with your test machine
> >> , you shouldn't expect the same results on your test machine.
> >> 
> >> Signed-off-by: Yi Yang 
> >
> >Hi Yi Yang,
> >
> >Are you testing this using OVS-DPDK?
> >If you're using OVS-DPDK, then you should use DPDK's vdev to
> >open and attach tap/veth device to OVS. I think you'll see much
> >better performance.
> >
> >The performance issue you pointed out only happens when using
> >userspace datapath without DPDK library, where afxdp is used.
> >I'm still looking for a better solutions for faster interface
> >for veth (af_packet) and tap.
> >
> >Thanks
> >William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC] WIP: netdev-tpacket: Add AF_PACKET v3 support.

[William Tu]

On Fri, Dec 20, 2019 at 01:14:37PM -0800, Ben Pfaff wrote:
> On Fri, Dec 20, 2019 at 09:49:44AM -0800, William Tu wrote:
> > On Thu, Dec 19, 2019 at 08:44:30PM -0800, Ben Pfaff wrote:
> > > On Thu, Dec 19, 2019 at 04:41:25PM -0800, William Tu wrote:
> > > > Currently the performance of sending packets from userspace
> > > > ovs to kernel veth device is pretty bad as reported from YiYang[1].
> > > > The patch adds AF_PACKET v3, tpacket v3, as another way to
> > > > tx/rx packet to linux device, hopefully showing better performance.
> > > > 
> > > > AF_PACKET v3 should get closed to 1Mpps, as shown[2]. However,
> > > > my current patch using iperf tcp shows only 1.4Gbps, maybe
> > > > I'm doing something wrong.  Also DPDK has similar implementation
> > > > using AF_PACKET v2[3].  This is still work-in-progress but any
> > > > feedbacks are welcome.
> > > 
> > > Is there a good reason that this is implemented as a new kind of netdev
> > > rather than just a new way for the existing netdev implementation to do
> > > packet i/o?
> > 
> > The AF_PACKET v3 is more like PMD mode driver (the netdev-afxdp and
> > other dpdk netdev), which has its own memory mgmt, ring structure, and
> > polling the descriptors. So I implemented it as a new kind. I feel its
> > pretty different than tap or existing af_packet netdev.
> > 
> > But integrate it to the existing netdev (lib/netdev-linux.c) is also OK.
> 
> Do you think it's sufficiently different from a user's point of view?  I
> think that's probably an important point of view here.  It's great if
> the user can just suddenly get better performance without having to do
> anything else.
> 
> On the other hand, if the user might need to know that tpacket is in
> use, like maybe if there is some downside or tradeoff to using it (for
> example, it needs a ring--does that use a lot of memory and would that
> be regrettable sometimes?), then that argues toward making it
> configurable.

Good point.
Let me spend more time on optimizing the patch's performance so I can
better answer this question.

> 
> You say it's more like afxdp.  Maybe it should be a fallback for afxdp,
> so that if afxdp isn't available for some reason then it automatically
> uses tpacket itself.  Or maybe that's a ridiculous idea for some
> reason, I don't know.
> 
> Do you think it is likely that a system supports tpacket but not afxdp?

It's possible, before 5.0 kernel, there is no afxdp.
So tpacket could be a good fallbck case.
But the performance differs a lot based on this patch.

William

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

Re: [ovs-dev] [PATCH ovn v3] ovn-northd: ls_*_acl behavior not consistent for untracked flows

[venugopal iyer via dev]

 Thanks, Numan! Please let me know what I should be monitoring to check for 
issues, if any,resulting from this change.
thanks,
-venu

On Friday, December 20, 2019, 11:36:30 AM PST, Numan Siddique 
 wrote:  
 
 On Fri, Dec 20, 2019 at 12:15 AM  wrote:
>
> From: venu iyer 
>
> If one creates a port group and a MAC address set, and an
> ACL that prevents packets being output to a port in that Port Group from
> any MAC address in that address set, the outcome is not consistent.
>
> The outcome depends on whether there is a stateful rule on the switch or not.
>
> Specifically:
>
> Assuming 'l2pg' is a port group with a list of ports and 'macs' is an Address
> Set with a list of MAC addresses and the intent is to drop all packets
> with source MAC address in 'macs' to any port in 'l2pg' using:
>
> ovn-nbctl acl-add  to-lport 5000 \
>        "outport == @l2pg && eth.src == $macs" drop
>
> Without any stateful rule on the logical switch, the corresponding
> logical flow looks like:
>        table=4 (ls_out_acl        ), priority=6000,\
>                match=(outport == @l2pg && eth.src == $macs), \
>                action=(/* drop */)
>
> Based on this rule, any packet destined to the ports in 'l2pg' with source
> Address in 'macs' will be dropped - as is expected from the ACL above.
>
> While with a Stateful rule on the switch (any stateful rule will do),
> the same rule looks like:
>        table=4 (ls_out_acl        ), priority=6000, \
>                match=((!ct.est || (ct.est && ct_label.blocked == 1)) && \
>                (outport == @l2pg && eth.src == $macs)), action=(/* drop */)
>
> With this, however, only packets that are tracked will match the rule
> and be dropped, e.g. IP packets will be dropped, but ARP etc., will go
> through - this is not expected.
>
> Based on whether there are stateful rules or not on the switch,
> untracked packets will see different behavior.
>
> The fix is to make the rule in the stateful case comprehensive, i.e.
> instead of just looking for flows that are not established (or not new),
> we should also look for flows that are not tracked.
>
> The fix was tested in the above scenario. Additionally, the following
> ACL was added to test the change in the "allow" case (i.e. to drop
> all the packets based on the above ACL, but have a higher priority
> rule that selectively allow ARP).
>
> ovn-nbctl acl-add ls1 to-lport 6000
>        "outport == @l2pg && eth.type == 0x806" allow
>
> with and without the stateful rule to make sure the behavior is the
> same.  The test suite has been enhanced to add the above test cases
> (with different ethertype) for drop and allow.
>
> OVN test cases were run with this fix and no failures were seen.
>
> Signed-off-by: venu iyer 

Thanks for rebasing the patch. I applied this patch to master. I also added your
name in the AUTHORS.rst in the same commit.

Thanks
Numan

> ---
>  northd/ovn-northd.c |  13 +--
>  tests/ovn.at        | 205 
>  2 files changed, 212 insertions(+), 6 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3a5cb7c91..d91a008b7 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4900,12 +4900,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
> *od,
>              * deletion.  There is no need to commit here, so we can just
>              * proceed to the next table. We use this to ensure that this
>              * connection is still allowed by the currently defined
> -            * policy. */
> +            * policy. Match untracked packets too. */
>              ds_clear();
>              ds_clear();
>              ds_put_format(,
> -                          "!ct.new && ct.est && !ct.rpl"
> -                          " && ct_label.blocked == 0 && (%s)",
> +                          "(!ct.trk || (!ct.new && ct.est && !ct.rpl"
> +                          " && ct_label.blocked == 0)) && (%s)",
>                            acl->match);
>
>              build_acl_log(, acl);
> @@ -4928,10 +4928,11 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
> *od,
>          * depending on whether the connection was previously committed
>          * to the connection tracker with ct_commit. */
>          if (has_stateful) {
> -            /* If the packet is not part of an established connection, then
> -            * we can simply reject/drop it. */
> +            /* If the packet is not tracked or not part of an established
> +            * connection, then we can simply reject/drop it. */
>              ds_put_cstr(,
> -                        "(!ct.est || (ct.est && ct_label.blocked == 1))");
> +                        "(!ct.trk || !ct.est"
> +                        " || (ct.est && ct_label.blocked == 1))");
>              if (!strcmp(acl->action, "reject")) {
>                  build_reject_acl_rules(od, lflows, stage, acl, ,
>                                        );
> diff --git a/tests/ovn.at b/tests/ovn.at
> 

Re: [ovs-dev] [PATCH] conntrack: Fix conntrack new state

[Ben Pfaff]

On Fri, Dec 20, 2019 at 09:51:08AM -0800, Yi-Hung Wei wrote:
> In connection tracking system, a connection is established if we
> see packets from both directions.  However, in userspace datapath's
> conntrack, if we send a connection setup packet in one direction
> twice, it will make the connection to be in established state.
> 
> This patch fixes the aforementioned issue, and adds a system traffic
> test for UDP and TCP traffic to avoid regression.
> 
> Fixes: a489b16854b59 ("conntrack: New userspace connection tracker.")
> Signed-off-by: Yi-Hung Wei 
> ---
> Travis CI: https://travis-ci.org/YiHungWei/ovs/builds/627518780

Good catch!

Darrell, will you review this?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH RFC] WIP: netdev-tpacket: Add AF_PACKET v3 support.

[Ben Pfaff]

On Fri, Dec 20, 2019 at 09:49:44AM -0800, William Tu wrote:
> On Thu, Dec 19, 2019 at 08:44:30PM -0800, Ben Pfaff wrote:
> > On Thu, Dec 19, 2019 at 04:41:25PM -0800, William Tu wrote:
> > > Currently the performance of sending packets from userspace
> > > ovs to kernel veth device is pretty bad as reported from YiYang[1].
> > > The patch adds AF_PACKET v3, tpacket v3, as another way to
> > > tx/rx packet to linux device, hopefully showing better performance.
> > > 
> > > AF_PACKET v3 should get closed to 1Mpps, as shown[2]. However,
> > > my current patch using iperf tcp shows only 1.4Gbps, maybe
> > > I'm doing something wrong.  Also DPDK has similar implementation
> > > using AF_PACKET v2[3].  This is still work-in-progress but any
> > > feedbacks are welcome.
> > 
> > Is there a good reason that this is implemented as a new kind of netdev
> > rather than just a new way for the existing netdev implementation to do
> > packet i/o?
> 
> The AF_PACKET v3 is more like PMD mode driver (the netdev-afxdp and
> other dpdk netdev), which has its own memory mgmt, ring structure, and
> polling the descriptors. So I implemented it as a new kind. I feel its
> pretty different than tap or existing af_packet netdev.
> 
> But integrate it to the existing netdev (lib/netdev-linux.c) is also OK.

Do you think it's sufficiently different from a user's point of view?  I
think that's probably an important point of view here.  It's great if
the user can just suddenly get better performance without having to do
anything else.

On the other hand, if the user might need to know that tpacket is in
use, like maybe if there is some downside or tradeoff to using it (for
example, it needs a ring--does that use a lot of memory and would that
be regrettable sometimes?), then that argues toward making it
configurable.

You say it's more like afxdp.  Maybe it should be a fallback for afxdp,
so that if afxdp isn't available for some reason then it automatically
uses tpacket itself.  Or maybe that's a ridiculous idea for some
reason, I don't know.

Do you think it is likely that a system supports tpacket but not afxdp?

Thanks,

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

Re: [ovs-dev] compiling on v5.4 kernel

[Gregory Rose]

On 12/20/2019 11:53 AM, David Ahern wrote:

On 12/20/19 11:14 AM, Gregory Rose wrote:

On 12/19/2019 7:20 PM, Gregory Rose wrote:

Bisecting is hard when you're doing it across 10s of thousands of
Linux revisions but after a few fitsand starts I think it's this
patch:

commit 9b9a3f20cbe0ba9269cde6fff9f9c69907e150cf
Author: Masahiro Yamada 
Date:   Thu Aug 15 01:06:23 2019 +0900

     kbuild: split final module linking out into Makefile.modfinal

     I think splitting the modpost and linking modules into separate
     Makefiles will be useful especially when more complex build steps
     come in. The main motivation of this commit is to integrate the
     proposed klp-convert feature cleanly.

     I moved the logging 'Building modules, stage 2.' to Makefile.modpost
     to avoid the code duplication although I do not know whether or not
     this message is needed in the first place.

     Signed-off-by: Masahiro Yamada 

I don't understand why it's causing the problem.  I'm no big specialist
in Linux kernel kbuild foo but I will investigate and see what I come
up with.  Whatever the case it is a bifurcation between kernels
earlier than 5.4.  We'll need to adjust our out of treee builds
accordingly I suspect.  I have no scope on that yet.

- Greg



Confirmed this is the patch that causes our out of tree kernel module
builds to fail on 5.4 based kernels.  I reset my tree to upstream
master, ran 'make clean'.  I then checked out commit 2a7f77c
'xprtrdma: Clean up xprt_rdma_set_connect_timeout()' which is the
commit just before the above commit.  I built that kernel and was able
to successfully build our out of tree kernel modules against that
build.

I then checked out the very next patch, the one above, and rebuilt the
kernel.  Then our OOT kernel module builds fail during the modpost
stage as I've stated before emitting thousands of lines of errors.

I'm looking at the patch and trying to figure out what we need to do
to fix our build to work against 5.4.

Using your branch as of

commit 1672395d571b34090fac14956d423092fe22051a (HEAD -> 5.4-support,
gregrose/5.4-support)
Author: Greg Rose 
Date:   Tue Dec 10 14:16:41 2019 -0800

 compat: Remove flex_array code

I get a lot of compile warnings and failures on Ubuntu 18.04:

   CC [M]  /home/dahern/oss/ovs.git/datapath/linux/conntrack.o
/home/dahern/oss/ovs.git/datapath/linux/conntrack.c: In function
'ovs_ct_nat_execute':
/home/dahern/oss/ovs.git/datapath/linux/conntrack.c:831:13: warning:
this statement may fall through [-Wimplicit-fallthrough=]
} else if (IS_ENABLED(CONFIG_NF_NAT_IPV6) &&
  ^
/home/dahern/oss/ovs.git/datapath/linux/conntrack.c:849:2: note: here
   case IP_CT_NEW:
   ^~~~


This just needs the fall through comment fixed if I am not mistaken.



...


In file included from ../include/uapi/linux/posix_types.h:5:0,
  from ../include/uapi/linux/types.h:14,
  from ../include/linux/types.h:6,
  from
/home/dahern/oss/ovs.git/datapath/linux/compat/include/linux/types.h:4,
  from ../include/linux/thread_info.h:11,
  from ../arch/x86/include/asm/elf.h:8,
  from ../include/linux/elf.h:5,
  from ../include/linux/elfnote.h:62,
  from ../include/linux/build-salt.h:4,
  from
/home/dahern/oss/ovs.git/datapath/linux/openvswitch.mod.c:1:
/home/dahern/oss/ovs.git/datapath/linux/compat/include/linux/stddef.h:10:2:
error: redeclaration of enumerator 'false'
   false   = 0,
   ^

Lots more like this for redinitions.


Right, those are the modpost errors caused by commit 9b9a3f2
'kbuild: split final module linking out into Makefile.modfinal'

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

Re: [ovs-dev] [PATCH ovn 4/4] extend-table: Fix reusing group/meter by multiple logical flows.

[Dumitru Ceara]

On Fri, Dec 6, 2019 at 11:29 PM Han Zhou  wrote:
>
> A meter/group can be used by multiple logical flows. However, current
> code didn't handle this properly. Each table_info item has a lflow_uuid
> field, which can keep track of only a single lflow.
>
> In most cases this doesn't create problems because multiple table_info
> entries are created even for same "name".
>
> However, when multiple lflows are added in the same main loop iteration
> using the same "name" (i.e. when the new_table_id == true), the function
> ovn_extend_table_assign_id() will return the old id without creating a
> new entry, and the reference by the second lflow is untracked. Later
> with incremental processing, if the old lflow is deleted, the table_info
> will be deleted, which results in the deletion of group/meter in OVS,
> even when it is still used by the second lflow.
>
> This patch fixes the problem by adding a hmap in each desired table_info
> item to keep track of multiple lflow references. A test case is added.
> The test case would fail without this fix.
>
> At the same time, this patch adds an index that maps from lflow_uuid to
> a list of desired table_info items used by the lflow, so that the
> ovn_extend_table_remove_desired() is more efficient, without having
> to do a O(N) iteration every time.
>
> Fixes: ca278d98a4f5 ("ovn-controller: Initial use of incremental engine - 
> quiet mode.")
> Signed-off-by: Han Zhou 

Nice catch! Looks good to me.

Acked-by: Dumitru Ceara 

> ---
>  lib/extend-table.c | 180 
> ++---
>  lib/extend-table.h |  31 -
>  tests/ovn.at   |  55 
>  3 files changed, 241 insertions(+), 25 deletions(-)
>
> diff --git a/lib/extend-table.c b/lib/extend-table.c
> index 82dfcfa..b102e44 100644
> --- a/lib/extend-table.c
> +++ b/lib/extend-table.c
> @@ -31,9 +31,37 @@ ovn_extend_table_init(struct ovn_extend_table *table)
>  table->table_ids = bitmap_allocate(MAX_EXT_TABLE_ID);
>  bitmap_set1(table->table_ids, 0); /* table id 0 is invalid. */
>  hmap_init(>desired);
> +hmap_init(>lflow_to_desired);
>  hmap_init(>existing);
>  }
>
> +static struct ovn_extend_table_info *
> +ovn_extend_table_info_alloc(const char *name, uint32_t id, bool is_new_id,
> +uint32_t hash)
> +{
> +struct ovn_extend_table_info *e = xmalloc(sizeof *e);
> +e->name = xstrdup(name);
> +e->table_id = id;
> +e->new_table_id = is_new_id;
> +e->hmap_node.hash = hash;
> +hmap_init(>references);
> +return e;
> +}
> +
> +static void
> +ovn_extend_table_info_destroy(struct ovn_extend_table_info *e)
> +{
> +free(e->name);
> +struct ovn_extend_table_lflow_ref *r, *r_next;
> +HMAP_FOR_EACH_SAFE (r, r_next, hmap_node, >references) {
> +hmap_remove(>references, >hmap_node);
> +ovs_list_remove(>list_node);
> +free(r);
> +}
> +hmap_destroy(>references);
> +free(e);
> +}
> +
>  /* Finds and returns a group_info in 'existing' whose key is identical
>   * to 'target''s key, or NULL if there is none. */
>  struct ovn_extend_table_info *
> @@ -51,6 +79,89 @@ ovn_extend_table_lookup(struct hmap *exisiting,
>  return NULL;
>  }
>
> +static struct ovn_extend_table_lflow_to_desired *
> +ovn_extend_table_find_desired_by_lflow(struct ovn_extend_table *table,
> +   const struct uuid *lflow_uuid)
> +{
> +struct ovn_extend_table_lflow_to_desired *l;
> +HMAP_FOR_EACH_WITH_HASH (l, hmap_node, uuid_hash(lflow_uuid),
> + >lflow_to_desired) {
> +if (uuid_equals(>lflow_uuid, lflow_uuid)) {
> +return l;
> +}
> +}
> +return NULL;
> +}
> +
> +/* Add a reference to the list of items that  uses.
> + * If the  entry doesn't exist in lflow_to_desired mapping, add
> + * the  entry first. */
> +static void
> +ovn_extend_table_add_desired_to_lflow(struct ovn_extend_table *table,
> +  const struct uuid *lflow_uuid,
> +  struct ovn_extend_table_lflow_ref *r)
> +{
> +struct ovn_extend_table_lflow_to_desired *l =
> +ovn_extend_table_find_desired_by_lflow(table, lflow_uuid);
> +if (!l) {
> +l = xmalloc(sizeof *l);
> +l->lflow_uuid = *lflow_uuid;
> +ovs_list_init(>desired);
> +hmap_insert(>lflow_to_desired, >hmap_node,
> +uuid_hash(lflow_uuid));
> +VLOG_DBG("%s: add new lflow_to_desired entry "UUID_FMT,
> + __func__, UUID_ARGS(lflow_uuid));
> +}
> +
> +ovs_list_insert(>desired, >list_node);
> +VLOG_DBG("%s: lflow "UUID_FMT" use new item %s, id %"PRIu32,
> + __func__, UUID_ARGS(lflow_uuid), r->desired->name,
> + r->desired->table_id);
> +}
> +
> +static struct ovn_extend_table_lflow_ref *
> +ovn_extend_info_find_lflow_ref(struct ovn_extend_table_info *e,
> +   

Re: [ovs-dev] [PATCH 2/2] ovsdb raft: Fix the problem when cluster restarted after DB compaction.

[Ben Pfaff]

On Tue, Dec 03, 2019 at 05:57:20PM -0800, Han Zhou wrote:
> Cluster doesn't work after all nodes restarted after DB compaction,
> unless there is any transaction after DB compaction before the restart.
> 
> Error log is like:
> raft|ERR|internal error: deferred vote_request message completed but not ready
> to send because message index 9 is past last synced index 0: s2 vote_request:
> term=6 last_log_index=9 last_log_term=4
> 
> The root cause is that the log_synced member is not initialized when
> reading the raft header. This patch fixes it and remove the XXX
> from the test case.
> 
> Signed-off-by: Han Zhou 

Thank you for finding this bug!  It must have been subtle.

I applied both of these patches to master and branch-2.12.

Thanks again,

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

Re: [ovs-dev] [PATCH ovn 2/4] ovn-controller: Fix meter-table-list and group-table-list commands.

[Dumitru Ceara]

On Fri, Dec 6, 2019 at 11:29 PM Han Zhou  wrote:
>
> These commands are supposed to print existing items of the tables,
> but they actually print only items that is in existing table but not
> in desired table, which is useless because this would print nothing
> in normal conditions. The patch fixes it so that they behave as
> what the document says.
>
> Signed-off-by: Han Zhou 

Looks good to me.

Acked-by: Dumitru Ceara 

> ---
>  controller/ovn-controller.c | 6 +++---
>  tests/ovn.at| 5 -
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index f836ffb..97be360 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2432,9 +2432,9 @@ extend_table_list(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  struct ds ds = DS_EMPTY_INITIALIZER;
>  struct simap items = SIMAP_INITIALIZER();
>
> -struct ovn_extend_table_info *installed, *next;
> -EXTEND_TABLE_FOR_EACH_INSTALLED (installed, next, extend_table) {
> -simap_put(, installed->name, installed->table_id);
> +struct ovn_extend_table_info *item;
> +HMAP_FOR_EACH (item, hmap_node, _table->existing) {
> +simap_put(, item->name, item->table_id);
>  }
>
>  const struct simap_node **nodes = simap_sort();
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 8f4d9a4..5dc7af2 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -7363,7 +7363,10 @@ echo "Meter duration: $d_secs"
>  AT_SKIP_IF([test $d_secs -gt 9])
>
>  # Print some information that may help debugging.
> -as hv ovs-appctl -t ovn-controller meter-table-list
> +AT_CHECK([as hv ovs-appctl -t ovn-controller meter-table-list], [0], [dnl
> +http-rl1: 1
> +http-rl2: 2
> +])
>  as hv ovs-ofctl -O OpenFlow13 meter-stats br-int
>
>  n_acl1=$(grep -c 'http-acl1' hv/ovn-controller.log)
> --
> 2.1.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 3/4] extend-table.c: Refactor code.

[Dumitru Ceara]

On Fri, Dec 6, 2019 at 11:29 PM Han Zhou  wrote:
>
> Reuse xxx_clear() function in xxx_destroy() and remove redundant code.
>
> Signed-off-by: Han Zhou 

Looks good to me.

Acked-by: Dumitru Ceara 

> ---
>  lib/extend-table.c | 31 ++-
>  1 file changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/lib/extend-table.c b/lib/extend-table.c
> index 77208fe..82dfcfa 100644
> --- a/lib/extend-table.c
> +++ b/lib/extend-table.c
> @@ -34,27 +34,6 @@ ovn_extend_table_init(struct ovn_extend_table *table)
>  hmap_init(>existing);
>  }
>
> -static void
> -ovn_extend_table_info_destroy(struct hmap *target)
> -{
> -struct ovn_extend_table_info *e, *next;
> -HMAP_FOR_EACH_SAFE (e, next, hmap_node, target) {
> -hmap_remove(target, >hmap_node);
> -free(e->name);
> -free(e);
> -}
> -hmap_destroy(target);
> -}
> -
> -void
> -ovn_extend_table_destroy(struct ovn_extend_table *table)
> -{
> -bitmap_free(table->table_ids);
> -
> -ovn_extend_table_info_destroy(>desired);
> -ovn_extend_table_info_destroy(>existing);
> -}
> -
>  /* Finds and returns a group_info in 'existing' whose key is identical
>   * to 'target''s key, or NULL if there is none. */
>  struct ovn_extend_table_info *
> @@ -91,6 +70,16 @@ ovn_extend_table_clear(struct ovn_extend_table *table, 
> bool existing)
>  }
>  }
>
> +void
> +ovn_extend_table_destroy(struct ovn_extend_table *table)
> +{
> +ovn_extend_table_clear(table, false);
> +hmap_destroy(>desired);
> +ovn_extend_table_clear(table, true);
> +hmap_destroy(>existing);
> +bitmap_free(table->table_ids);
> +}
> +
>  /* Remove an entry from existing table */
>  void
>  ovn_extend_table_remove_existing(struct ovn_extend_table *table,
> --
> 2.1.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 1/4] ovn-controller.c: Refactor meter-table-list and meter-group-list commands.

[Dumitru Ceara]

On Fri, Dec 6, 2019 at 11:29 PM Han Zhou  wrote:
>
> Remove redundant code.
>
> Signed-off-by: Han Zhou 

Looks good to me.

Acked-by: Dumitru Ceara 

> ---
>  controller/ovn-controller.c | 54 
> +++--
>  1 file changed, 13 insertions(+), 41 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 5874776..f836ffb 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -69,8 +69,7 @@ VLOG_DEFINE_THIS_MODULE(main);
>
>  static unixctl_cb_func ovn_controller_exit;
>  static unixctl_cb_func ct_zone_list;
> -static unixctl_cb_func meter_table_list;
> -static unixctl_cb_func group_table_list;
> +static unixctl_cb_func extend_table_list;
>  static unixctl_cb_func inject_pkt;
>  static unixctl_cb_func ovn_controller_conn_show;
>  static unixctl_cb_func engine_recompute_cmd;
> @@ -1975,11 +1974,11 @@ main(int argc, char *argv[])
>  get_ofctrl_probe_interval(ovs_idl_loop.idl));
>
>  unixctl_command_register("group-table-list", "", 0, 0,
> - group_table_list,
> + extend_table_list,
>   _output_data->group_table);
>
>  unixctl_command_register("meter-table-list", "", 0, 0,
> - meter_table_list,
> + extend_table_list,
>   _output_data->meter_table);
>
>  unixctl_command_register("ct-zone-list", "", 0, 0,
> @@ -2426,54 +2425,27 @@ ct_zone_list(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  }
>
>  static void
> -meter_table_list(struct unixctl_conn *conn, int argc OVS_UNUSED,
> - const char *argv[] OVS_UNUSED, void *meter_table_)
> +extend_table_list(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *extend_table_)
>  {
> -struct ovn_extend_table *meter_table = meter_table_;
> +struct ovn_extend_table *extend_table = extend_table_;
>  struct ds ds = DS_EMPTY_INITIALIZER;
> -struct simap meters = SIMAP_INITIALIZER();
> +struct simap items = SIMAP_INITIALIZER();
>
> -struct ovn_extend_table_info *m_installed, *next_meter;
> -EXTEND_TABLE_FOR_EACH_INSTALLED (m_installed, next_meter, meter_table) {
> -simap_put(, m_installed->name, m_installed->table_id);
> +struct ovn_extend_table_info *installed, *next;
> +EXTEND_TABLE_FOR_EACH_INSTALLED (installed, next, extend_table) {
> +simap_put(, installed->name, installed->table_id);
>  }
>
> -const struct simap_node **nodes = simap_sort();
> -size_t n_nodes = simap_count();
> +const struct simap_node **nodes = simap_sort();
> +size_t n_nodes = simap_count();
>  for (size_t i = 0; i < n_nodes; i++) {
>  const struct simap_node *node = nodes[i];
>  ds_put_format(, "%s: %d\n", node->name, node->data);
>  }
>
>  free(nodes);
> -simap_destroy();
> -
> -unixctl_command_reply(conn, ds_cstr());
> -ds_destroy();
> -}
> -
> -static void
> -group_table_list(struct unixctl_conn *conn, int argc OVS_UNUSED,
> - const char *argv[] OVS_UNUSED, void *group_table_)
> -{
> -struct ovn_extend_table *group_table = group_table_;
> -struct ds ds = DS_EMPTY_INITIALIZER;
> -struct simap groups = SIMAP_INITIALIZER();
> -
> -struct ovn_extend_table_info *m_installed, *next_group;
> -EXTEND_TABLE_FOR_EACH_INSTALLED (m_installed, next_group, group_table) {
> -simap_put(, m_installed->name, m_installed->table_id);
> -}
> -
> -const struct simap_node **nodes = simap_sort();
> -size_t n_nodes = simap_count();
> -for (size_t i = 0; i < n_nodes; i++) {
> -const struct simap_node *node = nodes[i];
> -ds_put_format(, "%s: %d\n", node->name, node->data);
> -}
> -
> -free(nodes);
> -simap_destroy();
> +simap_destroy();
>
>  unixctl_command_reply(conn, ds_cstr());
>  ds_destroy();
> --
> 2.1.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 v2] ovs container build: Make kernel module configurable

[Ben Pfaff]

On Thu, Dec 19, 2019 at 04:50:47PM -0800, amgin...@gmail.com wrote:
> From: Aliasgar Ginwala 
> 
> --with-linux can be made configurable while building containers
> for leveraging kernel modules installed on host.
> KERNEL_VERSION=host should be used in env variable for the same.
> 
> Signed-off-by: Aliasgar Ginwala 

I applied this to master without really reviewing it.  Since you're the
only contributor to utilities/docker so far, I'll take your word for it
that you know how it should work!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 答复: [PATCH] socket-util: Introduce emulation and wrapper for recvmmsg().

[Ben Pfaff]

On Fri, Dec 20, 2019 at 01:25:29AM +, Yi Yang (杨�D)-云服务集团 wrote:
> Current ovs matser has included sendmmsg declaration in
> include/sparse/sys/socket.h

I believe you are mistaken.

> int sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
> 
> I saw  "+^L" in your patch.

Yes, OVS uses page breaks to separate logical sections of code.  The
coding style document mentions this.

> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -1283,3 +1283,59 @@ wrap_sendmmsg(int fd, struct mmsghdr *msgs, unsigned
> int n, unsigned int flags)
>  }
>  #endif
>  #endif
> +^L
> +#ifndef _WIN32 /* Avoid using recvmsg on Windows entirely. */
> 
> +#undef recvmmsg
> +int
> +wrap_recvmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
> +  int flags, struct timespec *timeout)
> +{
> +ovs_assert(!timeout);   /* XXX not emulated */
> +
> +static bool recvmmsg_broken = false;
> +if (!recvmmsg_broken) {
> +int save_errno = errno;
> +int retval = recvmmsg(fd, msgs, n, flags, timeout);
> +if (retval >= 0 || errno != ENOSYS) {
> +return retval;
> +}
> +recvmmsg_broken = true;
> +errno = save_errno;
> +}
> +return emulate_recvmmsg(fd, msgs, n, flags, timeout);
> +}
> +#endif
> 
> I don't understand why call recvmmsg here although we have known recvmmsg
> isn't defined,

Can you explain that comment?  I don't believe that the code tries to
call recvmmsg() when it is not defined.  The code inside #ifndef
HAVE_SENDMMSG only uses emulate_recvmmsg(), which itself only calls
recvmsg(), not recvmmsg().

> I don't think "static bool recvmmsg_broken" is thread-safe. 

It is thread-safe enough, because it is merely an optimization: if the
value is wrong, then at most the code gets a little bit slower.

> I think we can completely remove the below part if we do know recvmmsg
> isn't defined (I think autoconf can detect it very precisely, we
> needn't to do runtime check for this)
> +static bool recvmmsg_broken = false;
> +if (!recvmmsg_broken) {
> +int save_errno = errno;
> +int retval = recvmmsg(fd, msgs, n, flags, timeout);
> +if (retval >= 0 || errno != ENOSYS) {
> +return retval;
> +}
> +recvmmsg_broken = true;
> +errno = save_errno;
> +}

There are three cases:

1. The C library does not have recvmmsg().  Then we cannot call it at
   all.  In this case, HAVE_SENDMMSG is false and the "#ifndef
   HAVE_SENDMMSG" fork will use emulate_recvmmsg().

2. The C library has recvmmsg() but the kernel does not, because it is
   too old.  Then wrap_recvmmsg() will receive an ENOSYS error from the
   kernel, call emulate_recvmmsg(), and set recvmmsg_broken so that
   future calls don't have to bother going into the kernel at all.

3. The C library and the kernel both have recvmmsg().  Then
   wrap_recvmmsg() will call recvmmsg() and either succeed or get back
   some error other than ENOSYS.  recvmmsg_broken will remain false, and
   all future calls to recvmmsg() will also take the kernel fast path.

Autoconf cannot distinguish cases 2 and 3, nor can anything that runs at
build time, because there is no way to guess whether the runtime kernel
matches the build time kernel.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/2] Remove dependency on python3-six

[Ben Pfaff]

On Fri, Dec 20, 2019 at 06:35:07PM +0100, Timothy Redaelli wrote:
> Since Python 2 support was removed in 1ca0323e7c29 ("Require Python 3 and
> remove support for Python 2."), python3-six is not needed anymore.
> 
> Moreover python3-six is not available on RHEL/CentOS7 without using EPEL
> and so this patch is needed in order to release OVS 2.13 on RHEL7.

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

Re: [ovs-dev] compiling on v5.4 kernel

[David Ahern]

On 12/20/19 11:14 AM, Gregory Rose wrote:
> 
> On 12/19/2019 7:20 PM, Gregory Rose wrote:
>>
>> Bisecting is hard when you're doing it across 10s of thousands of
>> Linux revisions but after a few fitsand starts I think it's this
>> patch:
>>
>> commit 9b9a3f20cbe0ba9269cde6fff9f9c69907e150cf
>> Author: Masahiro Yamada 
>> Date:   Thu Aug 15 01:06:23 2019 +0900
>>
>>     kbuild: split final module linking out into Makefile.modfinal
>>
>>     I think splitting the modpost and linking modules into separate
>>     Makefiles will be useful especially when more complex build steps
>>     come in. The main motivation of this commit is to integrate the
>>     proposed klp-convert feature cleanly.
>>
>>     I moved the logging 'Building modules, stage 2.' to Makefile.modpost
>>     to avoid the code duplication although I do not know whether or not
>>     this message is needed in the first place.
>>
>>     Signed-off-by: Masahiro Yamada 
>>
>> I don't understand why it's causing the problem.  I'm no big specialist
>> in Linux kernel kbuild foo but I will investigate and see what I come
>> up with.  Whatever the case it is a bifurcation between kernels
>> earlier than 5.4.  We'll need to adjust our out of treee builds
>> accordingly I suspect.  I have no scope on that yet.
>>
>> - Greg
>>
>>
> 
> Confirmed this is the patch that causes our out of tree kernel module
> builds to fail on 5.4 based kernels.  I reset my tree to upstream
> master, ran 'make clean'.  I then checked out commit 2a7f77c
> 'xprtrdma: Clean up xprt_rdma_set_connect_timeout()' which is the
> commit just before the above commit.  I built that kernel and was able
> to successfully build our out of tree kernel modules against that
> build.
> 
> I then checked out the very next patch, the one above, and rebuilt the
> kernel.  Then our OOT kernel module builds fail during the modpost
> stage as I've stated before emitting thousands of lines of errors.
> 
> I'm looking at the patch and trying to figure out what we need to do
> to fix our build to work against 5.4.

Using your branch as of

commit 1672395d571b34090fac14956d423092fe22051a (HEAD -> 5.4-support,
gregrose/5.4-support)
Author: Greg Rose 
Date:   Tue Dec 10 14:16:41 2019 -0800

compat: Remove flex_array code

I get a lot of compile warnings and failures on Ubuntu 18.04:

  CC [M]  /home/dahern/oss/ovs.git/datapath/linux/conntrack.o
/home/dahern/oss/ovs.git/datapath/linux/conntrack.c: In function
'ovs_ct_nat_execute':
/home/dahern/oss/ovs.git/datapath/linux/conntrack.c:831:13: warning:
this statement may fall through [-Wimplicit-fallthrough=]
   } else if (IS_ENABLED(CONFIG_NF_NAT_IPV6) &&
 ^
/home/dahern/oss/ovs.git/datapath/linux/conntrack.c:849:2: note: here
  case IP_CT_NEW:
  ^~~~


...


In file included from ../include/uapi/linux/posix_types.h:5:0,
 from ../include/uapi/linux/types.h:14,
 from ../include/linux/types.h:6,
 from
/home/dahern/oss/ovs.git/datapath/linux/compat/include/linux/types.h:4,
 from ../include/linux/thread_info.h:11,
 from ../arch/x86/include/asm/elf.h:8,
 from ../include/linux/elf.h:5,
 from ../include/linux/elfnote.h:62,
 from ../include/linux/build-salt.h:4,
 from
/home/dahern/oss/ovs.git/datapath/linux/openvswitch.mod.c:1:
/home/dahern/oss/ovs.git/datapath/linux/compat/include/linux/stddef.h:10:2:
error: redeclaration of enumerator 'false'
  false   = 0,
  ^

Lots more like this for redinitions.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v3] ovn-northd: ls_*_acl behavior not consistent for untracked flows

[Numan Siddique]

On Fri, Dec 20, 2019 at 12:15 AM  wrote:
>
> From: venu iyer 
>
> If one creates a port group and a MAC address set, and an
> ACL that prevents packets being output to a port in that Port Group from
> any MAC address in that address set, the outcome is not consistent.
>
> The outcome depends on whether there is a stateful rule on the switch or not.
>
> Specifically:
>
> Assuming 'l2pg' is a port group with a list of ports and 'macs' is an Address
> Set with a list of MAC addresses and the intent is to drop all packets
> with source MAC address in 'macs' to any port in 'l2pg' using:
>
> ovn-nbctl acl-add  to-lport 5000 \
> "outport == @l2pg && eth.src == $macs" drop
>
> Without any stateful rule on the logical switch, the corresponding
> logical flow looks like:
> table=4 (ls_out_acl), priority=6000,\
> match=(outport == @l2pg && eth.src == $macs), \
> action=(/* drop */)
>
> Based on this rule, any packet destined to the ports in 'l2pg' with source
> Address in 'macs' will be dropped - as is expected from the ACL above.
>
> While with a Stateful rule on the switch (any stateful rule will do),
> the same rule looks like:
> table=4 (ls_out_acl), priority=6000, \
> match=((!ct.est || (ct.est && ct_label.blocked == 1)) && \
> (outport == @l2pg && eth.src == $macs)), action=(/* drop */)
>
> With this, however, only packets that are tracked will match the rule
> and be dropped, e.g. IP packets will be dropped, but ARP etc., will go
> through - this is not expected.
>
> Based on whether there are stateful rules or not on the switch,
> untracked packets will see different behavior.
>
> The fix is to make the rule in the stateful case comprehensive, i.e.
> instead of just looking for flows that are not established (or not new),
> we should also look for flows that are not tracked.
>
> The fix was tested in the above scenario. Additionally, the following
> ACL was added to test the change in the "allow" case (i.e. to drop
> all the packets based on the above ACL, but have a higher priority
> rule that selectively allow ARP).
>
> ovn-nbctl acl-add ls1 to-lport 6000
> "outport == @l2pg && eth.type == 0x806" allow
>
> with and without the stateful rule to make sure the behavior is the
> same.  The test suite has been enhanced to add the above test cases
> (with different ethertype) for drop and allow.
>
> OVN test cases were run with this fix and no failures were seen.
>
> Signed-off-by: venu iyer 

Thanks for rebasing the patch. I applied this patch to master. I also added your
name in the AUTHORS.rst in the same commit.

Thanks
Numan

> ---
>  northd/ovn-northd.c |  13 +--
>  tests/ovn.at| 205 
>  2 files changed, 212 insertions(+), 6 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3a5cb7c91..d91a008b7 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4900,12 +4900,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
> *od,
>   * deletion.  There is no need to commit here, so we can just
>   * proceed to the next table. We use this to ensure that this
>   * connection is still allowed by the currently defined
> - * policy. */
> + * policy. Match untracked packets too. */
>  ds_clear();
>  ds_clear();
>  ds_put_format(,
> -  "!ct.new && ct.est && !ct.rpl"
> -  " && ct_label.blocked == 0 && (%s)",
> +  "(!ct.trk || (!ct.new && ct.est && !ct.rpl"
> +  " && ct_label.blocked == 0)) && (%s)",
>acl->match);
>
>  build_acl_log(, acl);
> @@ -4928,10 +4928,11 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
> *od,
>   * depending on whether the connection was previously committed
>   * to the connection tracker with ct_commit. */
>  if (has_stateful) {
> -/* If the packet is not part of an established connection, then
> - * we can simply reject/drop it. */
> +/* If the packet is not tracked or not part of an established
> + * connection, then we can simply reject/drop it. */
>  ds_put_cstr(,
> -"(!ct.est || (ct.est && ct_label.blocked == 1))");
> +"(!ct.trk || !ct.est"
> +" || (ct.est && ct_label.blocked == 1))");
>  if (!strcmp(acl->action, "reject")) {
>  build_reject_acl_rules(od, lflows, stage, acl, ,
> );
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 1d5369341..213a1b1f9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -12357,6 +12357,211 @@ AT_CHECK([cat 2.packets], [0], [])
>
>  AT_CLEANUP
>
> +# 3 hypervisors, one logical switch, 3 

Re: [ovs-dev] [PATCH v8 2/2] netdev-afxdp: NUMA-aware memory allocation for XSK related memory

[William Tu]

On Wed, Dec 18, 2019 at 12:31:06PM -0800, Yi-Hung Wei wrote:
> Currently, the AF_XDP socket (XSK) related memory are allocated by main
> thread in the main thread's NUMA domain.  With the patch that detects
> netdev-linux's NUMA node id, the PMD thread of AF_XDP port will be run on
> the AF_XDP netdev's NUMA domain.  If the net device's NUMA domain
> is different from the main thread's NUMA domain, we will have two
> cross-NUMA memory accesses (netdev <-> memory, memory <-> CPU).
> 
> This patch addresses the aforementioned issue by allocating
> the memory in the net device's NUMA domain.
> 
> Signed-off-by: Yi-Hung Wei 

LGTM, Thanks for working on this!
(I wasn't able to test NIC on different NUMA id, becuase I don't
have physical access to hardware. So only make sure numa id=0 works)

Tested-by: William Tu 

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

Re: [ovs-dev] compiling on v5.4 kernel

[Gregory Rose]

On 12/19/2019 7:20 PM, Gregory Rose wrote:


Bisecting is hard when you're doing it across 10s of thousands of
Linux revisions but after a few fitsand starts I think it's this
patch:

commit 9b9a3f20cbe0ba9269cde6fff9f9c69907e150cf
Author: Masahiro Yamada 
Date:   Thu Aug 15 01:06:23 2019 +0900

    kbuild: split final module linking out into Makefile.modfinal

    I think splitting the modpost and linking modules into separate
    Makefiles will be useful especially when more complex build steps
    come in. The main motivation of this commit is to integrate the
    proposed klp-convert feature cleanly.

    I moved the logging 'Building modules, stage 2.' to Makefile.modpost
    to avoid the code duplication although I do not know whether or not
    this message is needed in the first place.

    Signed-off-by: Masahiro Yamada 

I don't understand why it's causing the problem.  I'm no big specialist
in Linux kernel kbuild foo but I will investigate and see what I come
up with.  Whatever the case it is a bifurcation between kernels
earlier than 5.4.  We'll need to adjust our out of treee builds
accordingly I suspect.  I have no scope on that yet.

- Greg




Confirmed this is the patch that causes our out of tree kernel module
builds to fail on 5.4 based kernels.  I reset my tree to upstream
master, ran 'make clean'.  I then checked out commit 2a7f77c
'xprtrdma: Clean up xprt_rdma_set_connect_timeout()' which is the
commit just before the above commit.  I built that kernel and was able
to successfully build our out of tree kernel modules against that
build.

I then checked out the very next patch, the one above, and rebuilt the
kernel.  Then our OOT kernel module builds fail during the modpost
stage as I've stated before emitting thousands of lines of errors.

I'm looking at the patch and trying to figure out what we need to do
to fix our build to work against 5.4.

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

Re: [ovs-dev] thundering herd wakeup of handler threads

[David Ahern]

On 12/16/19 2:42 PM, Aaron Conole wrote:
> Can you try the following and see if your scalability issue is
> addressed?  I think it could be better integrated, but this is a
> different quick 'n dirty.

your patch reduces the number of threads awakened, but it is still
really high - 43 out of 71 handler threads in one test.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] conntrack: Fix conntrack new state

[Yi-Hung Wei]

In connection tracking system, a connection is established if we
see packets from both directions.  However, in userspace datapath's
conntrack, if we send a connection setup packet in one direction
twice, it will make the connection to be in established state.

This patch fixes the aforementioned issue, and adds a system traffic
test for UDP and TCP traffic to avoid regression.

Fixes: a489b16854b59 ("conntrack: New userspace connection tracker.")
Signed-off-by: Yi-Hung Wei 
---
Travis CI: https://travis-ci.org/YiHungWei/ovs/builds/627518780

---
 lib/conntrack-other.c   |  4 +++-
 lib/conntrack-private.h |  1 +
 lib/conntrack-tcp.c | 15 ++-
 lib/conntrack.c |  3 +++
 tests/system-traffic.at | 41 +
 5 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
index 932f2f4ad9c6..de22ef87cc19 100644
--- a/lib/conntrack-other.c
+++ b/lib/conntrack-other.c
@@ -47,16 +47,18 @@ other_conn_update(struct conntrack *ct, struct conn *conn_,
   struct dp_packet *pkt OVS_UNUSED, bool reply, long long now)
 {
 struct conn_other *conn = conn_other_cast(conn_);
+enum ct_update_res ret = CT_UPDATE_VALID;
 
 if (reply && conn->state != OTHERS_BIDIR) {
 conn->state = OTHERS_BIDIR;
 } else if (conn->state == OTHERS_FIRST) {
 conn->state = OTHERS_MULTIPLE;
+ret = CT_UPDATE_VALID_NEW;
 }
 
 conn_update_expiration(ct, >up, other_timeouts[conn->state], now);
 
-return CT_UPDATE_VALID;
+return ret;
 }
 
 static bool
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index b04e4cd77542..9a8ca3910157 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -124,6 +124,7 @@ enum ct_update_res {
 CT_UPDATE_INVALID,
 CT_UPDATE_VALID,
 CT_UPDATE_NEW,
+CT_UPDATE_VALID_NEW,
 };
 
 /* Timeouts: all the possible timeout states passed to update_expiration()
diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
index 47eb8e20346f..416cb769d22f 100644
--- a/lib/conntrack-tcp.c
+++ b/lib/conntrack-tcp.c
@@ -181,11 +181,16 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
 return CT_UPDATE_INVALID;
 }
 
-if (((tcp_flags & (TCP_SYN | TCP_ACK)) == TCP_SYN)
-&& dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
-&& src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
-src->state = dst->state = CT_DPIF_TCPS_CLOSED;
-return CT_UPDATE_NEW;
+if ((tcp_flags & (TCP_SYN | TCP_ACK)) == TCP_SYN) {
+if (dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
+&& src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
+src->state = dst->state = CT_DPIF_TCPS_CLOSED;
+return CT_UPDATE_NEW;
+} else if (src->state <= CT_DPIF_TCPS_SYN_SENT) {
+src->state = CT_DPIF_TCPS_SYN_SENT;
+conn_update_expiration(ct, >up, CT_TM_TCP_FIRST_PACKET, now);
+return CT_UPDATE_NEW;
+}
 }
 
 if (src->wscale & CT_WSCALE_FLAG
diff --git a/lib/conntrack.c b/lib/conntrack.c
index b80080e72bf8..eb3790a9aecb 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1110,6 +1110,9 @@ conn_update_state(struct conntrack *ct, struct dp_packet 
*pkt,
 ovs_mutex_unlock(>ct_lock);
 create_new_conn = true;
 break;
+case CT_UPDATE_VALID_NEW:
+pkt->md.ct_state |= CS_NEW;
+break;
 default:
 OVS_NOT_REACHED();
 }
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 0fb7aacfa14e..4a39c929c207 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2290,6 +2290,47 @@ 
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - new connections])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows1.txt], [dnl
+table=0, priority=1,action=drop
+table=0, priority=10,arp,action=normal
+table=0, priority=100,tcp,action=ct(table=1)
+table=0, priority=100,udp,action=ct(table=1)
+table=1, priority=100,in_port=1,tcp,ct_state=+trk+new,action=ct(commit)
+table=1, priority=100,in_port=1,udp,ct_state=+trk+new,action=ct(commit)
+table=1, priority=100,in_port=1,ct_state=+trk+est,action=2
+table=1, priority=100,in_port=2,ct_state=+trk+est,action=1
+])
+
+ovs-appctl vlog/set dbg
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows1.txt])
+
+dnl TCP traffic from ns0 to ns1 should fail.
+OVS_START_L7([at_ns1], [http])
+NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o 
wget0.log], [4])
+
+dnl Send UDP packet on port 1 twice.
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 
packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
 actions=resubmit(,0)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out 

Re: [ovs-dev] [PATCH RFC] WIP: netdev-tpacket: Add AF_PACKET v3 support.

[William Tu]

On Thu, Dec 19, 2019 at 08:44:30PM -0800, Ben Pfaff wrote:
> On Thu, Dec 19, 2019 at 04:41:25PM -0800, William Tu wrote:
> > Currently the performance of sending packets from userspace
> > ovs to kernel veth device is pretty bad as reported from YiYang[1].
> > The patch adds AF_PACKET v3, tpacket v3, as another way to
> > tx/rx packet to linux device, hopefully showing better performance.
> > 
> > AF_PACKET v3 should get closed to 1Mpps, as shown[2]. However,
> > my current patch using iperf tcp shows only 1.4Gbps, maybe
> > I'm doing something wrong.  Also DPDK has similar implementation
> > using AF_PACKET v2[3].  This is still work-in-progress but any
> > feedbacks are welcome.
> 
> Is there a good reason that this is implemented as a new kind of netdev
> rather than just a new way for the existing netdev implementation to do
> packet i/o?

The AF_PACKET v3 is more like PMD mode driver (the netdev-afxdp and
other dpdk netdev), which has its own memory mgmt, ring structure, and
polling the descriptors. So I implemented it as a new kind. I feel its
pretty different than tap or existing af_packet netdev.

But integrate it to the existing netdev (lib/netdev-linux.c) is also OK.

William

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

Re: [ovs-dev] 答复: [PATCH RFC] WIP: netdev-tpacket: Add AF_PACKET v3 support.

[William Tu]

On Fri, Dec 20, 2019 at 06:09:08AM +, Yi Yang (杨燚)-云服务集团 wrote:
> Hi, William
> 
> What kernel version can support AF_PACKET v3? I can try it with your patch.

Hi Yiyang,

Kernel +4.0 should have v3 support.

I'm also reading this doc:
https://www.kernel.org/doc/Documentation/networking/packet_mmap.txt

---
+ AF_PACKET TPACKET_V3 example
---

AF_PACKET's TPACKET_V3 ring buffer can be configured to use non-static frame
sizes by doing it's own memory management. It is based on blocks where polling
works on a per block basis instead of per ring as in TPACKET_V2 and predecessor.

It is said that TPACKET_V3 brings the following benefits:
 *) ~15 - 20% reduction in CPU-usage
 *) ~20% increase in packet capture rate
 *) ~2x increase in packet density
 *) Port aggregation analysis
 *) Non static frame size to capture entire packet payload

So it seems to be a good candidate to be used with packet fanout.

DPDK library is using TPACKET_V2, and V3 is better due to:
TPACKET_V2 --> TPACKET_V3:
- Flexible buffer implementation for RX_RING:
1. Blocks can be configured with non-static frame-size
2. Read/poll is at a block-level (as opposed to packet-level)
3. Added poll timeout to avoid indefinite user-space wait
   on idle links
4. Added user-configurable knobs:
4.1 block::timeout
4.2 tpkt_hdr::sk_rxhash
- RX Hash data available in user space
- TX_RING semantics are conceptually similar to TPACKET_V2;

Thanks
William

> 
> -邮件原件-
> 发件人: William Tu [mailto:u9012...@gmail.com] 
> 发送时间: 2019年12月20日 8:41
> 收件人: d...@openvswitch.org
> 抄送: i.maxim...@ovn.org; Yi Yang (杨燚)-云服务集团 ;
> b...@ovn.org; echau...@redhat.com
> 主题: [PATCH RFC] WIP: netdev-tpacket: Add AF_PACKET v3 support.
> 
> Currently the performance of sending packets from userspace ovs to kernel
> veth device is pretty bad as reported from YiYang[1].
> The patch adds AF_PACKET v3, tpacket v3, as another way to tx/rx packet to
> linux device, hopefully showing better performance.
> 
> AF_PACKET v3 should get closed to 1Mpps, as shown[2]. However, my current
> patch using iperf tcp shows only 1.4Gbps, maybe I'm doing something wrong.
> Also DPDK has similar implementation using AF_PACKET v2[3].  This is still
> work-in-progress but any feedbacks are welcome.
> 
> [1] https://patchwork.ozlabs.org/patch/1204939/
> [2] slide 18, https://www.netdevconf.info/2.2/slides/karlsson-afpacket-talk.
> pdf
> [3] dpdk/drivers/net/af_packet/rte_eth_af_packet.c
> ---
>  lib/automake.mk|   2 +
>  lib/netdev-linux-private.h |  23 +++
>  lib/netdev-linux.c |  24 ++-
>  lib/netdev-provider.h  |   1 +
>  lib/netdev-tpacket.c   | 487
> +
>  lib/netdev-tpacket.h   |  43 
>  lib/netdev.c   |   1 +
>  7 files changed, 580 insertions(+), 1 deletion(-)  create mode 100644
> lib/netdev-tpacket.c  create mode 100644 lib/netdev-tpacket.h
> 

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

[ovs-dev] [PATCH v2 1/2] Remove dependency on python3-six

[Timothy Redaelli]

Since Python 2 support was removed in 1ca0323e7c29 ("Require Python 3 and
remove support for Python 2."), python3-six is not needed anymore.

Moreover python3-six is not available on RHEL/CentOS7 without using EPEL
and so this patch is needed in order to release OVS 2.13 on RHEL7.

Signed-off-by: Timothy Redaelli 
---
 .cirrus.yml   |  2 +-
 .travis/linux-prepare.sh  |  2 +-
 .travis/osx-prepare.sh|  1 -
 Documentation/intro/install/general.rst   |  6 +-
 Documentation/intro/install/netbsd.rst|  3 +-
 Documentation/intro/install/rhel.rst  |  2 +-
 Documentation/intro/install/windows.rst   |  5 +-
 Vagrantfile   |  7 ++-
 Vagrantfile-FreeBSD   |  2 +-
 appveyor.yml  |  2 +-
 configure.ac  |  1 -
 debian/control|  4 +-
 m4/openvswitch.m4 | 12 
 python/ovs/db/data.py | 30 +
 python/ovs/db/idl.py  | 60 +-
 python/ovs/db/parser.py   |  8 +--
 python/ovs/db/schema.py   | 34 +-
 python/ovs/db/types.py| 29 -
 python/ovs/json.py| 18 ++
 python/ovs/jsonrpc.py | 11 ++--
 python/ovs/ovsuuid.py |  7 +--
 python/ovs/socket_util.py |  7 +--
 python/ovs/stream.py  |  6 +-
 python/ovs/unixctl/__init__.py|  9 +--
 python/ovs/unixctl/client.py  |  7 +--
 python/ovs/unixctl/server.py  | 15 ++---
 python/ovs/vlog.py|  7 +--
 python/ovstest/rpcserver.py   |  7 +--
 python/ovstest/util.py|  6 +-
 rhel/openvswitch-fedora.spec.in   |  1 -
 tests/test-json.py| 13 +---
 tests/test-ovsdb.py   | 41 +---
 tests/test-vlog.py|  2 -
 utilities/bugtool/ovs-bugtool.in  |  3 -
 .../docker/debian/build-kernel-modules.sh |  3 +-
 utilities/gdb/ovs_gdb.py  | 62 ---
 vtep/ovs-vtep.in  | 23 +++
 ...sr_share_openvswitch_scripts_ovs-xapi-sync | 10 ++-
 38 files changed, 181 insertions(+), 287 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index ec48202dd..f7a625f01 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -9,7 +9,7 @@ freebsd_build_task:
 
   env:
 DEPENDENCIES: automake libtool gmake gcc wget openssl
-  python3 py36-six py36-openssl py36-sphinx
+  python3 py36-openssl py36-sphinx
 matrix:
   COMPILER: gcc
   COMPILER: clang
diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh
index 13390afc0..fda13e7d2 100755
--- a/.travis/linux-prepare.sh
+++ b/.travis/linux-prepare.sh
@@ -12,7 +12,7 @@ cd sparse
 make -j4 HAVE_LLVM= install
 cd ..
 
-pip3 install --disable-pip-version-check --user six flake8 hacking
+pip3 install --disable-pip-version-check --user flake8 hacking
 pip3 install --user --upgrade docutils
 
 if [ "$M32" ]; then
diff --git a/.travis/osx-prepare.sh b/.travis/osx-prepare.sh
index 78d5bb579..b6447aba1 100755
--- a/.travis/osx-prepare.sh
+++ b/.travis/osx-prepare.sh
@@ -1,4 +1,3 @@
 #!/bin/bash
 set -ev
-pip3 install --user six
 pip3 install --user --upgrade docutils
diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index e62501be7..09f2c13f1 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -90,8 +90,7 @@ need the following software:
   If libcap-ng is installed, then Open vSwitch will automatically build with
   support for it.
 
-- Python 3.4 or later. You must also have the Python ``six`` library
-  version 1.4.0 or later.
+- Python 3.4 or later.
 
 - Unbound library, from http://www.unbound.net, is optional but recommended if
   you want to enable ovs-vswitchd and other utilities to use DNS names when
@@ -203,8 +202,7 @@ simply install and run Open vSwitch you require the 
following software:
   from iproute2 (part of all major distributions and available at
   https://wiki.linuxfoundation.org/networking/iproute2).
 
-- Python 3.4 or later. You must also have the Python six library
-  version 1.4.0 or later.
+- Python 3.4 or later.
 
 On Linux you should ensure that ``/dev/urandom`` exists. To support TAP
 devices, you must also ensure that ``/dev/net/tun`` exists.
diff --git a/Documentation/intro/install/netbsd.rst 
b/Documentation/intro/install/netbsd.rst
index 4f60dad86..d5bd3660a 100644
--- a/Documentation/intro/install/netbsd.rst
+++ b/Documentation/intro/install/netbsd.rst
@@ -32,7 +32,6 @@ you need at least the 

[ovs-dev] [PATCH v2 2/2] Revert "docs: To build OVS on RHEL7 EPEL is needed"

[Timothy Redaelli]

This reverts commit 9e334d91b3ea95e2b96f7b3edcb2ba9c3353288a.

This commit is not needed since OVS doesn't use six anymore.

Signed-off-by: Timothy Redaelli 
---
 Documentation/intro/install/fedora.rst | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/intro/install/fedora.rst 
b/Documentation/intro/install/fedora.rst
index c6d1d83ae..6fe1fb5b2 100644
--- a/Documentation/intro/install/fedora.rst
+++ b/Documentation/intro/install/fedora.rst
@@ -63,12 +63,11 @@ The command below will create a temporary SPEC file::
   > /tmp/ovs.spec
 
 And to install specific dependencies, use the corresponding tool below.
-For some of the dependencies on RHEL 7 you may need to add three additional
+For some of the dependencies on RHEL you may need to add two additional
 repositories to help yum-builddep, e.g.::
 
 $ subscription-manager repos --enable=rhel-7-server-extras-rpms
 $ subscription-manager repos --enable=rhel-7-server-optional-rpms
-$ yum install 
https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm
 
 DNF::
 
-- 
2.23.0

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

[ovs-dev] [PATCH v2 0/2] Remove dependency on python3-six

[Timothy Redaelli]

Since Python 2 support was removed in 1ca0323e7c29 ("Require Python 3 and
remove support for Python 2."), python3-six is not needed anymore.

Moreover python3-six is not available on RHEL/CentOS7 without using EPEL
and so this patch is needed in order to release OVS 2.13 on RHEL7.


Changes in v2:

- Fixed a typo in commit message "Moveover" => "Moreover"
- Added missing "Signed-off-by" in Revert "docs: To build OVS on RHEL7 EPEL
  is needed"

Timothy Redaelli (2):
  Remove dependency on python3-six
  Revert "docs: To build OVS on RHEL7 EPEL is needed"

 .cirrus.yml   |  2 +-
 .travis/linux-prepare.sh  |  2 +-
 .travis/osx-prepare.sh|  1 -
 Documentation/intro/install/fedora.rst|  3 +-
 Documentation/intro/install/general.rst   |  6 +-
 Documentation/intro/install/netbsd.rst|  3 +-
 Documentation/intro/install/rhel.rst  |  2 +-
 Documentation/intro/install/windows.rst   |  5 +-
 Vagrantfile   |  7 ++-
 Vagrantfile-FreeBSD   |  2 +-
 appveyor.yml  |  2 +-
 configure.ac  |  1 -
 debian/control|  4 +-
 m4/openvswitch.m4 | 12 
 python/ovs/db/data.py | 30 +
 python/ovs/db/idl.py  | 60 +-
 python/ovs/db/parser.py   |  8 +--
 python/ovs/db/schema.py   | 34 +-
 python/ovs/db/types.py| 29 -
 python/ovs/json.py| 18 ++
 python/ovs/jsonrpc.py | 11 ++--
 python/ovs/ovsuuid.py |  7 +--
 python/ovs/socket_util.py |  7 +--
 python/ovs/stream.py  |  6 +-
 python/ovs/unixctl/__init__.py|  9 +--
 python/ovs/unixctl/client.py  |  7 +--
 python/ovs/unixctl/server.py  | 15 ++---
 python/ovs/vlog.py|  7 +--
 python/ovstest/rpcserver.py   |  7 +--
 python/ovstest/util.py|  6 +-
 rhel/openvswitch-fedora.spec.in   |  1 -
 tests/test-json.py| 13 +---
 tests/test-ovsdb.py   | 41 +---
 tests/test-vlog.py|  2 -
 utilities/bugtool/ovs-bugtool.in  |  3 -
 .../docker/debian/build-kernel-modules.sh |  3 +-
 utilities/gdb/ovs_gdb.py  | 62 ---
 vtep/ovs-vtep.in  | 23 +++
 ...sr_share_openvswitch_scripts_ovs-xapi-sync | 10 ++-
 39 files changed, 182 insertions(+), 289 deletions(-)

-- 
2.23.0

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

Re: [ovs-dev] [PATCH 2/2] Revert "docs: To build OVS on RHEL7 EPEL is needed"

[0-day Robot]

Bleep bloop.  Greetings Timothy Redaelli, 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:
ERROR: Author Timothy Redaelli  needs to sign off.
Lines checked: 34, Warnings: 0, Errors: 1


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

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

[ovs-dev] [PATCH 1/2] Remove dependency on python3-six

[Timothy Redaelli]

Since Python 2 support was removed in 1ca0323e7c29 ("Require Python 3 and
remove support for Python 2."), python3-six is not needed anymore.

Moveover python3-six is not available on RHEL/CentOS7 without using EPEL
and so this patch is needed in order to release OVS 2.13 on RHEL7.

Signed-off-by: Timothy Redaelli 
---
 .cirrus.yml   |  2 +-
 .travis/linux-prepare.sh  |  2 +-
 .travis/osx-prepare.sh|  1 -
 Documentation/intro/install/general.rst   |  6 +-
 Documentation/intro/install/netbsd.rst|  3 +-
 Documentation/intro/install/rhel.rst  |  2 +-
 Documentation/intro/install/windows.rst   |  5 +-
 Vagrantfile   |  7 ++-
 Vagrantfile-FreeBSD   |  2 +-
 appveyor.yml  |  2 +-
 configure.ac  |  1 -
 debian/control|  4 +-
 m4/openvswitch.m4 | 12 
 python/ovs/db/data.py | 30 +
 python/ovs/db/idl.py  | 60 +-
 python/ovs/db/parser.py   |  8 +--
 python/ovs/db/schema.py   | 34 +-
 python/ovs/db/types.py| 29 -
 python/ovs/json.py| 18 ++
 python/ovs/jsonrpc.py | 11 ++--
 python/ovs/ovsuuid.py |  7 +--
 python/ovs/socket_util.py |  7 +--
 python/ovs/stream.py  |  6 +-
 python/ovs/unixctl/__init__.py|  9 +--
 python/ovs/unixctl/client.py  |  7 +--
 python/ovs/unixctl/server.py  | 15 ++---
 python/ovs/vlog.py|  7 +--
 python/ovstest/rpcserver.py   |  7 +--
 python/ovstest/util.py|  6 +-
 rhel/openvswitch-fedora.spec.in   |  1 -
 tests/test-json.py| 13 +---
 tests/test-ovsdb.py   | 41 +---
 tests/test-vlog.py|  2 -
 utilities/bugtool/ovs-bugtool.in  |  3 -
 .../docker/debian/build-kernel-modules.sh |  3 +-
 utilities/gdb/ovs_gdb.py  | 62 ---
 vtep/ovs-vtep.in  | 23 +++
 ...sr_share_openvswitch_scripts_ovs-xapi-sync | 10 ++-
 38 files changed, 181 insertions(+), 287 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index ec48202dd..f7a625f01 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -9,7 +9,7 @@ freebsd_build_task:
 
   env:
 DEPENDENCIES: automake libtool gmake gcc wget openssl
-  python3 py36-six py36-openssl py36-sphinx
+  python3 py36-openssl py36-sphinx
 matrix:
   COMPILER: gcc
   COMPILER: clang
diff --git a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh
index 13390afc0..fda13e7d2 100755
--- a/.travis/linux-prepare.sh
+++ b/.travis/linux-prepare.sh
@@ -12,7 +12,7 @@ cd sparse
 make -j4 HAVE_LLVM= install
 cd ..
 
-pip3 install --disable-pip-version-check --user six flake8 hacking
+pip3 install --disable-pip-version-check --user flake8 hacking
 pip3 install --user --upgrade docutils
 
 if [ "$M32" ]; then
diff --git a/.travis/osx-prepare.sh b/.travis/osx-prepare.sh
index 78d5bb579..b6447aba1 100755
--- a/.travis/osx-prepare.sh
+++ b/.travis/osx-prepare.sh
@@ -1,4 +1,3 @@
 #!/bin/bash
 set -ev
-pip3 install --user six
 pip3 install --user --upgrade docutils
diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index e62501be7..09f2c13f1 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -90,8 +90,7 @@ need the following software:
   If libcap-ng is installed, then Open vSwitch will automatically build with
   support for it.
 
-- Python 3.4 or later. You must also have the Python ``six`` library
-  version 1.4.0 or later.
+- Python 3.4 or later.
 
 - Unbound library, from http://www.unbound.net, is optional but recommended if
   you want to enable ovs-vswitchd and other utilities to use DNS names when
@@ -203,8 +202,7 @@ simply install and run Open vSwitch you require the 
following software:
   from iproute2 (part of all major distributions and available at
   https://wiki.linuxfoundation.org/networking/iproute2).
 
-- Python 3.4 or later. You must also have the Python six library
-  version 1.4.0 or later.
+- Python 3.4 or later.
 
 On Linux you should ensure that ``/dev/urandom`` exists. To support TAP
 devices, you must also ensure that ``/dev/net/tun`` exists.
diff --git a/Documentation/intro/install/netbsd.rst 
b/Documentation/intro/install/netbsd.rst
index 4f60dad86..d5bd3660a 100644
--- a/Documentation/intro/install/netbsd.rst
+++ b/Documentation/intro/install/netbsd.rst
@@ -32,7 +32,6 @@ you need at least the 

[ovs-dev] [PATCH 2/2] Revert "docs: To build OVS on RHEL7 EPEL is needed"

[Timothy Redaelli]

This reverts commit 9e334d91b3ea95e2b96f7b3edcb2ba9c3353288a.

This commit is not needed since OVS doesn't use six anymore.
---
 Documentation/intro/install/fedora.rst | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/intro/install/fedora.rst 
b/Documentation/intro/install/fedora.rst
index c6d1d83ae..6fe1fb5b2 100644
--- a/Documentation/intro/install/fedora.rst
+++ b/Documentation/intro/install/fedora.rst
@@ -63,12 +63,11 @@ The command below will create a temporary SPEC file::
   > /tmp/ovs.spec
 
 And to install specific dependencies, use the corresponding tool below.
-For some of the dependencies on RHEL 7 you may need to add three additional
+For some of the dependencies on RHEL you may need to add two additional
 repositories to help yum-builddep, e.g.::
 
 $ subscription-manager repos --enable=rhel-7-server-extras-rpms
 $ subscription-manager repos --enable=rhel-7-server-optional-rpms
-$ yum install 
https://dl.fedoraproject.org/pub/epel/epel-release-latest-7.noarch.rpm
 
 DNF::
 
-- 
2.23.0

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

[ovs-dev] [PATCH 0/2] Remove dependency on python3-six

[Timothy Redaelli]

Since Python 2 support was removed in 1ca0323e7c29 ("Require Python 3 and
remove support for Python 2."), python3-six is not needed anymore.

Moveover python3-six is not available on RHEL/CentOS7 without using EPEL
and so this patch is needed in order to release OVS 2.13 on RHEL7.

Tested locally and with travis:
https://travis-ci.org/drizzt/ovs/builds/627763532

Timothy Redaelli (2):
  Remove dependency on python3-six
  Revert "docs: To build OVS on RHEL7 EPEL is needed"

 .cirrus.yml   |  2 +-
 .travis/linux-prepare.sh  |  2 +-
 .travis/osx-prepare.sh|  1 -
 Documentation/intro/install/fedora.rst|  3 +-
 Documentation/intro/install/general.rst   |  6 +-
 Documentation/intro/install/netbsd.rst|  3 +-
 Documentation/intro/install/rhel.rst  |  2 +-
 Documentation/intro/install/windows.rst   |  5 +-
 Vagrantfile   |  7 ++-
 Vagrantfile-FreeBSD   |  2 +-
 appveyor.yml  |  2 +-
 configure.ac  |  1 -
 debian/control|  4 +-
 m4/openvswitch.m4 | 12 
 python/ovs/db/data.py | 30 +
 python/ovs/db/idl.py  | 60 +-
 python/ovs/db/parser.py   |  8 +--
 python/ovs/db/schema.py   | 34 +-
 python/ovs/db/types.py| 29 -
 python/ovs/json.py| 18 ++
 python/ovs/jsonrpc.py | 11 ++--
 python/ovs/ovsuuid.py |  7 +--
 python/ovs/socket_util.py |  7 +--
 python/ovs/stream.py  |  6 +-
 python/ovs/unixctl/__init__.py|  9 +--
 python/ovs/unixctl/client.py  |  7 +--
 python/ovs/unixctl/server.py  | 15 ++---
 python/ovs/vlog.py|  7 +--
 python/ovstest/rpcserver.py   |  7 +--
 python/ovstest/util.py|  6 +-
 rhel/openvswitch-fedora.spec.in   |  1 -
 tests/test-json.py| 13 +---
 tests/test-ovsdb.py   | 41 +---
 tests/test-vlog.py|  2 -
 utilities/bugtool/ovs-bugtool.in  |  3 -
 .../docker/debian/build-kernel-modules.sh |  3 +-
 utilities/gdb/ovs_gdb.py  | 62 ---
 vtep/ovs-vtep.in  | 23 +++
 ...sr_share_openvswitch_scripts_ovs-xapi-sync | 10 ++-
 39 files changed, 182 insertions(+), 289 deletions(-)

-- 
2.23.0

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

[ovs-dev] [v1] dpif-netdev: Add ovs-appctl dpif-netdev/subtable-show.

[Emma Finn]

Add an ovs-appctl command to iterate through the dpcls
and for each subtable output the miniflow bits for any
existing table.

$ ovs-appctl dpif-netdev/subtable-show
pmd thread numa_id 0
  dpcls port 2:
subtable:
  unit_0: 2 (0x5)
  unit_1: 1 (0x1)
pmd thread numa_id 1
  dpcls port 3:
subtable:
  unit_0: 2 (0x5)
  unit_1: 1 (0x1)

Signed-off-by: Emma Finn 

---

RFC -> v1

* Changed revision from RFC to v1
* Reformatted based on comments
* Fixed same classifier being dumped multiple times
  flagged by Ilya
* Fixed print of subtables flagged by William
* Updated print count of bits as well as bits themselves
---
 NEWS|  2 ++
 lib/dpif-netdev-unixctl.man |  4 
 lib/dpif-netdev.c   | 53 -
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 2acde3f..2b7b0cc 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,8 @@ Post-v2.12.0
  * DPDK ring ports (dpdkr) are deprecated and will be removed in next
releases.
  * Add support for DPDK 19.11.
+ * New "ovs-appctl dpif-netdev/subtable-show" command for userspace
+   datapath to show subtable miniflow bits.
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
index 6c54f6f..c443465 100644
--- a/lib/dpif-netdev-unixctl.man
+++ b/lib/dpif-netdev-unixctl.man
@@ -217,3 +217,7 @@ with port names, which this thread polls.
 .
 .IP "\fBdpif-netdev/pmd-rxq-rebalance\fR [\fIdp\fR]"
 Reassigns rxqs to pmds in the datapath \fIdp\fR based on their current usage.
+.
+.IP "\fBdpif-netdev/subtable-show\fR [\fB-pmd\fR \fIcore\fR] [\fIdp\fR]"
+For one or all pmd threads of the datapath \fIdp\fR show the list of miniflow
+bits for each subtable in the datapath classifier.
\ No newline at end of file
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8485b54..a166b9e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -857,6 +857,8 @@ static inline bool
 pmd_perf_metrics_enabled(const struct dp_netdev_pmd_thread *pmd);
 static void queue_netdev_flow_del(struct dp_netdev_pmd_thread *pmd,
   struct dp_netdev_flow *flow);
+static void pmd_info_show_subtable(struct ds *reply,
+   struct dp_netdev_pmd_thread *pmd);
 
 static void
 emc_cache_init(struct emc_cache *flow_cache)
@@ -979,6 +981,7 @@ enum pmd_info_type {
 PMD_INFO_CLEAR_STATS, /* Set the cycles count to 0. */
 PMD_INFO_SHOW_RXQ,/* Show poll lists of pmd threads. */
 PMD_INFO_PERF_SHOW,   /* Show pmd performance details. */
+PMD_INFO_SHOW_SUBTABLE, /* Show subtable miniflow bits. */
 };
 
 static void
@@ -1334,6 +1337,8 @@ dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, 
const char *argv[],
 pmd_info_show_stats(, pmd);
 } else if (type == PMD_INFO_PERF_SHOW) {
 pmd_info_show_perf(, pmd, (struct pmd_perf_params *)aux);
+} else if (type == PMD_INFO_SHOW_SUBTABLE) {
+pmd_info_show_subtable(, pmd);
 }
 }
 free(pmd_list);
@@ -1391,7 +1396,8 @@ dpif_netdev_init(void)
 {
 static enum pmd_info_type show_aux = PMD_INFO_SHOW_STATS,
   clear_aux = PMD_INFO_CLEAR_STATS,
-  poll_aux = PMD_INFO_SHOW_RXQ;
+  poll_aux = PMD_INFO_SHOW_RXQ,
+  subtable_aux = PMD_INFO_SHOW_SUBTABLE;
 
 unixctl_command_register("dpif-netdev/pmd-stats-show", "[-pmd core] [dp]",
  0, 3, dpif_netdev_pmd_info,
@@ -1416,6 +1422,9 @@ dpif_netdev_init(void)
  "[-us usec] [-q qlen]",
  0, 10, pmd_perf_log_set_cmd,
  NULL);
+unixctl_command_register("dpif-netdev/subtable-show", "[-pmd core] [dp]",
+ 0, 3, dpif_netdev_pmd_info,
+ (void *)_aux);
 return 0;
 }
 
@@ -8139,3 +8148,45 @@ dpcls_lookup(struct dpcls *cls, const struct 
netdev_flow_key *keys[],
 }
 return false;
 }
+
+/* Iterate through all dpcls instances and dump out all subtable
+ * miniflow bits. */
+static void
+pmd_info_show_subtable(struct ds *reply, struct dp_netdev_pmd_thread *pmd)
+OVS_REQUIRES(dp_netdev_mutex)
+{
+ if (pmd->core_id != NON_PMD_CORE_ID) {
+struct dp_netdev_port *port = NULL;
+struct dp_netdev *dp = pmd->dp;
+
+ ovs_mutex_lock(>port_mutex);
+ HMAP_FOR_EACH (port, node, >ports) {
+ odp_port_t in_port = port->port_no;
+
+struct dpcls *cls = dp_netdev_pmd_lookup_dpcls(pmd, in_port);
+if (!cls) {
+ continue;
+} else {
+struct pvector *pvec = >subtables;
+ds_put_format(reply, "pmd thread numa_id %d "
+  "core_id %u: \n",
+  

[ovs-dev] Open Positions

[Curran, Able]

Hi,



I don't know whether you are the right person, so I am hoping you can point me 
in the right direction.

I see on the internet that you have some open positions within your company, 
and I have some candidates that may interest you.



It doesn't cost anything to look at resumes, so please let me know if I should 
send them to you or to someone else.


Look forward to hearing from you.
Regards
Able Curran | Talent Acquisition Manager
Talent Logic, Inc.
*O:281 358 1858  X 753


  IMPORTANT MESSAGE AND 
DISCLAIMER

Internet communications are not secure, and therefore Talent Logic, Inc.. does 
not accept legal responsibility for the contents of this message. However, 
Talent Logic, Inc... reserves the right to monitor the transmission of this 
message and to take corrective action against any misuse or abuse of its e-mail 
system or other components of its network. The information contained in this 
e-mail is confidential and may be legally privileged. It is intended solely for 
the addressee. If you are not the intended recipient, any disclosure, copying, 
distribution, or any action or act of forbearance taken in reliance on it, is 
prohibited and may be unlawful. Any views expressed in this e-mail are those of 
the individual sender, except where the sender specifically states them to be 
the views of Talent Logic, Inc.. or any of its affiliates or subsidiaries.




 END OF 
DISCLAIMER

If you wish to receive no further emails from this company, you 
can opt-out from future correspondence at any time by clicking 
Unsubscribe
 or Reply to unsubscr...@talentlogic.net at 
any time.


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

Re: [ovs-dev] [ovs-dev, ovn, 1/2] ovn-nbctl.c: Add "name" column of QoS table.

[Numan Siddique]

On Fri, Dec 20, 2019 at 1:39 PM taoyunxi...@cmss.chinamobile.com
 wrote:
>
> Hi, Could you review this patch。It is an important QoS feature for plugin of 
> OVN.

Hi Taoyunxiang,

I am sorry for the delay. Looks like I missed these patches.

The patch doesn't apply to the present master. Can you please rebase
the series and submit v2 ?

Thanks
Numan

>
> Thanks,
> Yun
>
>
>
>
> From: taoyunxi...@cmss.chinamobile.com
> Date: 2019-11-26 15:57
> To: ovs-dev
> Subject: [ovs-dev,ovn,1/2] ovn-nbctl.c: Add "name" column of QoS table.
> Add "name" column of QoS table
>
> Add "name" column of QoS table and make qos could be list
> by command "ovn-nbctl list qos "name"".
>
> Signed-off-by: Yunxiang Tao 
>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 12999a466..02ae48e73 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
>  "version": "5.18.0",
> -"cksum": "2806349485 24196",
> +"cksum": "80177565 24240",
>  "tables": {
>  "NB_Global": {
>  "columns": {
> @@ -204,6 +204,7 @@
>  "isRoot": false},
>  "QoS": {
>  "columns": {
> +"name": {"type": "string"},
>  "priority": {"type": {"key": {"type": "integer",
>"minInteger": 0,
>"maxInteger": 32767}}},
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 4a93d2f4a..61e77397e 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1662,6 +1662,12 @@
>
>  
>
> +
> +  
> +A name for the logical router.
> +  
> +
> +
>  
>
>  The value of this field is similar to  diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 8188948fa..93e37d169 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -5346,6 +5346,8 @@ static const struct ctl_table_class 
> tables[NBREC_N_TABLES] = {
>
>  [NBREC_TABLE_HA_CHASSIS_GROUP].row_ids[0]
>  = {_ha_chassis_group_col_name, NULL, NULL},
> +
> +[NBREC_TABLE_QOS].row_ids[0] = {_qos_col_name, NULL, NULL},
>  };
>
>  static char *
>
>
>
> taoyunxi...@cmss.chinamobile.com
> ___
> 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 v3] ovsdb replication: Provide option to configure probe interval.

[Numan Siddique]

On Fri, Dec 20, 2019 at 12:28 AM Ben Pfaff  wrote:
>
> On Fri, Dec 13, 2019 at 03:15:36PM -0500, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > When ovsdb-server is in backup mode and connects to the active
> > ovsdb-server for replication, and if takes more than 5 seconds to
> > get the dump of the whole database, it will drop the connection
> > soon after as the default probe interval is 5 seconds. This
> > results in a snowball effect of reconnections to the active
> > ovsdb-server.
> >
> > This patch handles or mitigates this issue by setting the
> > default probe interval value to 60 seconds and provide the option to
> > configure this value from the unixctl command.
> >
> > Other option could be increase the value of 
> > 'RECONNECT_DEFAULT_PROBE_INTERVAL'
> > to a higher value.
> >
> > Acked-by: Mark Michelson 
> > Signed-off-by: Numan Siddique 
>
> Thanks for the patch!  I have a couple of comments.
>
> In the documentation, please put \ before - consistently in the .IP.
>
> When it's possible, I like settings to take effect immediately, so that
> users don't have to restart things.  I think that this should be
> possible here, by calling jsonrpc_session_set_probe_interval() in the
> right place.  Did you look at what it would take?
>

Thanks  for the review. I have submitted v4 addressing the comments -
https://patchwork.ozlabs.org/patch/1214081/

I thought about that, but I wasn't sure if it's a good idea to set the
probe interval
in the unixctl command handler. v4 which I just submitted, sets the
probe interval in the command
handler though.

Thanks
Numan


> Thanks,
>
> Ben.
> ___
> 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 v4] ovsdb replication: Provide option to configure probe interval.

[numans]

From: Numan Siddique 

When ovsdb-server is in backup mode and connects to the active
ovsdb-server for replication, and if takes more than 5 seconds to
get the dump of the whole database, it will drop the connection
soon after as the default probe interval is 5 seconds. This
results in a snowball effect of reconnections to the active
ovsdb-server.

This patch handles or mitigates this issue by setting the
default probe interval value to 60 seconds and provide the option to
configure this value from the unixctl command.

Other option could be increase the value of 'RECONNECT_DEFAULT_PROBE_INTERVAL'
to a higher value.

Acked-by: Mark Michelson 
Signed-off-by: Numan Siddique 
Acked-by: Dumitru Ceara 
---
v3 -> v4
-
  * Addressed review comments from Ben


 ovsdb/ovsdb-server.1.in |  5 +
 ovsdb/ovsdb-server.c| 49 +++--
 ovsdb/replication.c | 12 +-
 ovsdb/replication.h |  5 -
 4 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index 21f527bc6..338f3bc29 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -288,6 +288,11 @@ Switches the server to an active role.  The server stops 
synchronizing
 its databases with an active server and closes all existing client
 connections, which requires clients to reconnect.
 .
+.IP "\fBovsdb\-server/set\-active\-ovsdb\-server\-probe\-interval \fIprobe 
interval"
+Sets  the probe interval (in milli seconds) for the connection to
+active \fIserver\fR.
+.
+.
 .IP "\fBovsdb\-server/set\-sync\-exclude\-tables 
\fIdb\fB:\fItable\fR[\fB,\fIdb\fB:\fItable\fR]..."
 Sets the \fItable\fR within \fIdb\fR that will be excluded from 
synchronization.
 This overrides the \fB\-\-sync\-exclude-tables\fR command-line option.
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 9827320ec..b6957d730 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -86,6 +86,7 @@ static unixctl_cb_func ovsdb_server_set_active_ovsdb_server;
 static unixctl_cb_func ovsdb_server_get_active_ovsdb_server;
 static unixctl_cb_func ovsdb_server_connect_active_ovsdb_server;
 static unixctl_cb_func ovsdb_server_disconnect_active_ovsdb_server;
+static unixctl_cb_func ovsdb_server_set_active_ovsdb_server_probe_interval;
 static unixctl_cb_func ovsdb_server_set_sync_exclude_tables;
 static unixctl_cb_func ovsdb_server_get_sync_exclude_tables;
 static unixctl_cb_func ovsdb_server_get_sync_status;
@@ -97,6 +98,7 @@ struct server_config {
 char **sync_from;
 char **sync_exclude;
 bool *is_backup;
+int *replication_probe_interval;
 struct ovsdb_jsonrpc_server *jsonrpc;
 };
 static unixctl_cb_func ovsdb_server_add_remote;
@@ -144,9 +146,10 @@ static void load_config(FILE *config_file, struct sset 
*remotes,
 
 static void
 ovsdb_replication_init(const char *sync_from, const char *exclude,
-   struct shash *all_dbs, const struct uuid *server_uuid)
+   struct shash *all_dbs, const struct uuid *server_uuid,
+   int probe_interval)
 {
-replication_init(sync_from, exclude, server_uuid);
+replication_init(sync_from, exclude, server_uuid, probe_interval);
 struct shash_node *node;
 SHASH_FOR_EACH (node, all_dbs) {
 struct db *db = node->data;
@@ -304,6 +307,7 @@ main(int argc, char *argv[])
 struct server_config server_config;
 struct shash all_dbs;
 struct shash_node *node, *next;
+int replication_probe_interval = REPLICATION_DEFAULT_PROBE_INTERVAL;
 
 ovs_cmdl_proctitle_init(argc, argv);
 set_program_name(argv[0]);
@@ -351,6 +355,7 @@ main(int argc, char *argv[])
 server_config.sync_from = _from;
 server_config.sync_exclude = _exclude;
 server_config.is_backup = _backup;
+server_config.replication_probe_interval = _probe_interval;
 
 perf_counters_init();
 
@@ -436,6 +441,9 @@ main(int argc, char *argv[])
 unixctl_command_register("ovsdb-server/disconnect-active-ovsdb-server", "",
  0, 0, ovsdb_server_disconnect_active_ovsdb_server,
  _config);
+unixctl_command_register(
+"ovsdb-server/set-active-ovsdb-server-probe-interval", "", 1, 1,
+ovsdb_server_set_active_ovsdb_server_probe_interval, _config);
 unixctl_command_register("ovsdb-server/set-sync-exclude-tables", "",
  0, 1, ovsdb_server_set_sync_exclude_tables,
  _config);
@@ -454,7 +462,8 @@ main(int argc, char *argv[])
 if (is_backup) {
 const struct uuid *server_uuid;
 server_uuid = ovsdb_jsonrpc_server_get_uuid(jsonrpc);
-ovsdb_replication_init(sync_from, sync_exclude, _dbs, server_uuid);
+ovsdb_replication_init(sync_from, sync_exclude, _dbs, server_uuid,
+   replication_probe_interval);
 }
 
 main_loop(_config, jsonrpc, _dbs, unixctl, ,
@@ -1317,7 +1326,8 

Re: [ovs-dev] [PATCH net-next v2] openvswitch: add TTL decrement action

[Matteo Croce]

On Tue, Dec 17, 2019 at 5:30 PM Nikolay Aleksandrov
 wrote:
>
> On 17/12/2019 17:51, Matteo Croce wrote:
> > New action to decrement TTL instead of setting it to a fixed value.
> > This action will decrement the TTL and, in case of expired TTL, drop it
> > or execute an action passed via a nested attribute.
> > The default TTL expired action is to drop the packet.
> >
> > Supports both IPv4 and IPv6 via the ttl and hop_limit fields, respectively.
> >
> > Tested with a corresponding change in the userspace:
> >
> > # ovs-dpctl dump-flows
> > in_port(2),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
> > actions:dec_ttl{ttl<=1 action:(drop)},1,1
> > in_port(1),eth(),eth_type(0x0800), packets:0, bytes:0, used:never, 
> > actions:dec_ttl{ttl<=1 action:(drop)},1,2
> > in_port(1),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, 
> > actions:2
> > in_port(2),eth(),eth_type(0x0806), packets:0, bytes:0, used:never, 
> > actions:1
> >
> > # ping -c1 192.168.0.2 -t 42
> > IP (tos 0x0, ttl 41, id 61647, offset 0, flags [DF], proto ICMP (1), 
> > length 84)
> > 192.168.0.1 > 192.168.0.2: ICMP echo request, id 386, seq 1, length 
> > 64
> > # ping -c1 192.168.0.2 -t 120
> > IP (tos 0x0, ttl 119, id 62070, offset 0, flags [DF], proto ICMP (1), 
> > length 84)
> > 192.168.0.1 > 192.168.0.2: ICMP echo request, id 388, seq 1, length 
> > 64
> > # ping -c1 192.168.0.2 -t 1
> > #
> >
> > Co-authored-by: Bindiya Kurle 
> > Signed-off-by: Bindiya Kurle 
> > Signed-off-by: Matteo Croce 
> > ---
> >  include/uapi/linux/openvswitch.h |  22 +++
> >  net/openvswitch/actions.c|  71 +
> >  net/openvswitch/flow_netlink.c   | 105 +++
> >  3 files changed, 198 insertions(+)
> >
>
> Hi Matteo,
>
> [snip]
> > +}
> > +
> >  /* When 'last' is true, sample() should always consume the 'skb'.
> >   * Otherwise, sample() should keep 'skb' intact regardless what
> >   * actions are executed within sample().
> > @@ -1176,6 +1201,44 @@ static int execute_check_pkt_len(struct datapath 
> > *dp, struct sk_buff *skb,
> >nla_len(actions), last, clone_flow_key);
> >  }
> >
> > +static int execute_dec_ttl(struct sk_buff *skb, struct sw_flow_key *key)
> > +{
> > + int err;
> > +
> > + if (skb->protocol == htons(ETH_P_IPV6)) {
> > + struct ipv6hdr *nh = ipv6_hdr(skb);
> > +
> > + err = skb_ensure_writable(skb, skb_network_offset(skb) +
> > +   sizeof(*nh));
>
> skb_ensure_writable() calls pskb_may_pull() which may reallocate so nh might 
> become invalid.
> It seems the IPv4 version below is ok as the ptr is reloaded.
>

Right

> One q as I don't know ovs that much - can this action be called only with
> skb->protocol ==  ETH_P_IP/IPV6 ? I.e. Are we sure that if it's not v6, then 
> it must be v4 ?
>

I'm adding a check in validate_and_copy_dec_ttl() so only ipv4/ipv6
packet will pass.

Thanks,

-- 
Matteo Croce
per aspera ad upstream

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

[ovs-dev] [PATCH v4 ovn 2/2] northd: add logical flows for dhcpv6 pfd parsing

[Lorenzo Bianconi]

Introduce logical flows in ovn router pipeline in order to parse dhcpv6
advertise/reply from IPv6 prefix delegation router.
Do not overwrite ipv6_ra_pd_list info in options column of SB port_binding
table written by ovn-controller

Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.c |  69 +++-
 ovn-nb.xml  |  17 ++
 tests/atlocal.in|   5 +-
 tests/system-ovn.at | 124 
 4 files changed, 213 insertions(+), 2 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3a5cb7c91..cc3d950bc 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2653,6 +2653,8 @@ ovn_port_update_sbrec(struct northd_context *ctx,
   struct sset *active_ha_chassis_grps)
 {
 sbrec_port_binding_set_datapath(op->sb, op->od->sb);
+const char *ipv6_pd_list = NULL;
+
 if (op->nbrp) {
 /* If the router is for l3 gateway, it resides on a chassis
  * and its port type is "l3gateway". */
@@ -2775,6 +2777,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 smap_add(, "l3gateway-chassis", chassis_name);
 }
 }
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
 sbrec_port_binding_set_options(op->sb, );
 smap_destroy();
 
@@ -2824,6 +2832,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 smap_add_format(,
 "qdisc_queue_id", "%d", queue_id);
 }
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
 sbrec_port_binding_set_options(op->sb, );
 smap_destroy();
 if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
@@ -2873,6 +2887,12 @@ ovn_port_update_sbrec(struct northd_context *ctx,
 if (chassis) {
 smap_add(, "l3gateway-chassis", chassis);
 }
+
+ipv6_pd_list = smap_get(>sb->options, "ipv6_ra_pd_list");
+if (ipv6_pd_list) {
+smap_add(, "ipv6_ra_pd_list", ipv6_pd_list);
+}
+
 sbrec_port_binding_set_options(op->sb, );
 smap_destroy();
 } else {
@@ -7842,7 +7862,36 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 free(snat_ips);
 }
 
-/* Logical router ingress table 3: IP Input for IPv6. */
+/* DHCPv6 reply handling */
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbrp) {
+continue;
+}
+
+if (op->derived) {
+continue;
+}
+
+struct lport_addresses lrp_networks;
+if (!extract_lrp_networks(op->nbrp, _networks)) {
+continue;
+}
+
+for (size_t i = 0; i < lrp_networks.n_ipv6_addrs; i++) {
+ds_clear();
+ds_clear();
+ds_put_format(, "ip6.dst == %s && udp.src == 547 &&"
+  " udp.dst == 546",
+  lrp_networks.ipv6_addrs[i].addr_s);
+ds_put_format(, "reg0 = 0; handle_dhcpv6_reply { "
+  "eth.dst <-> eth.src; ip6.dst <-> ip6.src; "
+  "outport <-> inport; output; };");
+ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 100,
+  ds_cstr(), ds_cstr());
+}
+}
+
+/* Logical router ingress table 1: IP Input for IPv6. */
 HMAP_FOR_EACH (op, key_node, ports) {
 if (!op->nbrp) {
 continue;
@@ -8643,6 +8692,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+struct smap options;
+/* enable IPv6 prefix delegation */
+bool prefix_delegation = smap_get_bool(>nbrp->options,
+   "prefix_delegation", false);
+if (prefix_delegation) {
+smap_clone(, >sb->options);
+smap_add(, "ipv6_prefix_delegation", "true");
+sbrec_port_binding_set_options(op->sb, );
+smap_destroy();
+}
+
+if (smap_get_bool(>nbrp->options, "prefix", false)) {
+smap_clone(, >sb->options);
+smap_add(, "ipv6_prefix", "true");
+sbrec_port_binding_set_options(op->sb, );
+smap_destroy();
+}
+
 const char *address_mode = smap_get(
 >nbrp->ipv6_ra_configs, "address_mode");
 
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 5ae52bbde..d7fddcae2 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2142,6 +2142,23 @@
   to true.
 
   
+
+  
+
+  If set to true, enable IPv6 prefix delegation state
+  machine on this logical router port (RFC3633). 

[ovs-dev] [PATCH v4 ovn 0/2] Add IPv6 Prefix delegation (RFC3633)

[Lorenzo Bianconi]

Introduce IPv6 Prefix delegation state machine according to RFC 3633
https://tools.ietf.org/html/rfc3633.
Add handle_dhcpv6_reply controller action to parse advertise/reply from
IPv6 delegation server.
Introduce logical flows in ovn router pipeline in order to parse dhcpv6
advertise/reply from IPv6 prefix delegation router.
This series relies on the following OVS commit:
https://github.com/openvswitch/ovs/commit/cec89046f72cb044b068ba6a4e30dbcc4292c4c1

Changes since v3:
- cosmetics
- add a provider bridge in the unit-test deployment and add a localnet
  port to the deployment to access the underlay network
- request IPv6 prefix even for bar router logical port in the unit-test
  deployment

Changes since v2:
- add unitest support in system-ovn.at

Changes since v1:
- rebase on top of ovn master branch
- request an IPv6 prefix for each 'downstream' logical router port marked with
  prefix set to true
- add missing documentation
- rename dhcp6_server_pkt in handle_dhcpv6_reply

Lorenzo Bianconi (2):
  controller: add ipv6 prefix delegation state machine
  northd: add logical flows for dhcpv6 pfd parsing

 controller/pinctrl.c  | 597 ++
 include/ovn/actions.h |   8 +-
 lib/actions.c |  22 ++
 lib/ovn-l7.h  |  19 ++
 northd/ovn-northd.c   |  69 -
 ovn-nb.xml|  17 ++
 ovn-sb.xml|   8 +
 tests/atlocal.in  |   5 +-
 tests/ovn.at  |   6 +
 tests/system-ovn.at   | 124 +
 utilities/ovn-trace.c |   3 +
 11 files changed, 875 insertions(+), 3 deletions(-)

-- 
2.21.0

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

[ovs-dev] Predzalobna vyzva 20.12.2019-0177-58

[Klara Dostalova]

Dobry den,

Do dnesniho dne jste nesplnili Vasi zakonnou povinnost a nevratili mi 
zaplacenou financni castku, kterou jste byli povinni mi vratit.

V opacnem pripade budu nucen obratit se se svym narokem na soud s navrhem na 
vydani platebniho rozkazu. Verim vsak, ze to nebude nutne.

Stahnout prilohu.

S pozdravem, Klara Dostalova.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, ovn, 1/2] ovn-nbctl.c: Add "name" column of QoS table.

[taoyunxi...@cmss.chinamobile.com]

Hi, Could you review this patch。It is an important QoS feature for plugin of 
OVN.  

Thanks,
Yun




From: taoyunxi...@cmss.chinamobile.com
Date: 2019-11-26 15:57
To: ovs-dev
Subject: [ovs-dev,ovn,1/2] ovn-nbctl.c: Add "name" column of QoS table.
Add "name" column of QoS table

Add "name" column of QoS table and make qos could be list
by command "ovn-nbctl list qos "name"".
 
Signed-off-by: Yunxiang Tao 

diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 12999a466..02ae48e73 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
 "version": "5.18.0",
-"cksum": "2806349485 24196",
+"cksum": "80177565 24240",
 "tables": {
 "NB_Global": {
 "columns": {
@@ -204,6 +204,7 @@
 "isRoot": false},
 "QoS": {
 "columns": {
+"name": {"type": "string"},
 "priority": {"type": {"key": {"type": "integer",
   "minInteger": 0,
   "maxInteger": 32767}}},
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4a93d2f4a..61e77397e 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1662,6 +1662,12 @@
   
 
 
+
+  
+A name for the logical router.
+  
+
+
 
   
 The value of this field is similar to https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, ovn, 2/2] ovn-nbctl.c: Add a way to delete QoS by its name or uuid.

[taoyunxi...@cmss.chinamobile.com]

Hi, Could you review this patch。It is an important QoS feature for plugin of 
OVN.  

Thanks,
Yun





From: taoyunxi...@cmss.chinamobile.com
Date: 2019-11-26 15:57
To: ovs-dev
Subject: [ovs-dev,ovn,2/2] ovn-nbctl.c: Add a way to delete QoS by its name or 
uuid.
Add a way to delete QoS by its name or uuid

Currently, qos can only be deleted by indirect way which must designate
switch or more parameters. This patch change the original "qos-del" to
"ls-qos-del", and add a new "qos-del" command. By the new command, you
can delete qos by uuid or name of the qos. It is very import to support
a way to delete a table by direct way. For example, we can associate
the qos table in neutron and OVN by "name" of qos in OVN, so neutron
could find and easily delete the corresponding qos in OVN.
Signed-off-by: Yunxiang Tao 
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 93e37d169..f1d64208d 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -604,8 +604,9 @@ ACL commands:\n\
 QoS commands:\n\
   qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] 
[dscp=DSCP]\n\
 add an QoS rule to SWITCH\n\
-  qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
+  ls-qos-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
 remove QoS rules from SWITCH\n\
+  qos-del QOS   remove QoS rules by name or UUID\n\
   qos-list SWITCH   print QoS rules for SWITCH\n\
 \n\
 Meter commands:\n\
@@ -2490,7 +2491,7 @@ nbctl_qos_add(struct ctl_context *ctx)
 }
 
 static void
-nbctl_qos_del(struct ctl_context *ctx)
+nbctl_ls_qos_del(struct ctl_context *ctx)
 {
 const struct nbrec_logical_switch *ls;
 char *error = ls_by_name_or_uuid(ctx, ctx->argv[1], true, );
@@ -2564,6 +2565,96 @@ nbctl_qos_del(struct ctl_context *ctx)
 }
 }
 
+/* Remove qos*/
+static void
+remove_qos(const struct nbrec_logical_switch *ls, size_t idx)
+{
+
+/* First remove 'qos' from the array of qos_rules.  This is what will
+ * actually cause the qos to be deleted when the transaction is
+ * sent to the database server (due to garbage collection). */
+struct nbrec_qos **new_qos_rules
+= xmemdup(ls->qos_rules, sizeof *new_qos_rules * ls->n_qos_rules);
+new_qos_rules[idx] = new_qos_rules[ls->n_qos_rules - 1];
+nbrec_logical_switch_verify_qos_rules(ls);
+nbrec_logical_switch_set_qos_rules(ls, new_qos_rules, \
+  ls->n_qos_rules - 1);
+free(new_qos_rules);
+
+/* Delete 'qos' from the IDL.This won't have a real effect on the
+ * database server (the IDL will suppress it in fact) but it means that it
+ * won't show up when we iterate with NBREC_LOGICAL_QOS_FOR_EACH later. */
+}
+
+static char * OVS_WARN_UNUSED_RESULT
+qos_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
+   const struct nbrec_qos **qos_p)
+{
+const struct nbrec_qos *qos = NULL;
+*qos_p = NULL;
+
+struct uuid qos_uuid;
+bool is_uuid = uuid_from_string(_uuid, id);
+if (is_uuid) {
+qos = nbrec_qos_get_for_uuid(ctx->idl, _uuid);
+}
+
+if (!qos) {
+const struct nbrec_qos *iter;
+
+NBREC_QOS_FOR_EACH(iter, ctx->idl) {
+if (strcmp(iter->name, id)) {
+continue;
+}
+if (qos) {
+return xasprintf("Multiple qos named '%s'.  "
+ "Use a UUID.", id);
+}
+qos = iter;
+}
+}
+
+if (!qos && must_exist) {
+return xasprintf("%s: qos %s not found",
+ id, is_uuid ? "UUID" : "name");
+}
+
+*qos_p = qos;
+return NULL;
+}
+
+static void
+nbctl_qos_del(struct ctl_context *ctx)
+{
+bool must_exist = !shash_find(>options, "--if-exists");
+const char *id = ctx->argv[1];
+const struct nbrec_qos *qos = NULL;
+
+char *error = qos_by_name_or_uuid(ctx, id, must_exist, );
+if (error) {
+ctx->error = error;
+return;
+}
+if (!qos) {
+return;
+}
+
+/* Find the switch that contains 'qos_rules', then delete it. */
+const struct nbrec_logical_switch *ls;
+NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->idl) {
+for (size_t i = 0; i < ls->n_qos_rules; i++) {
+if (ls->qos_rules[i] == qos) {
+remove_qos(ls, i);
+return;
+}
+}
+}
+
+/* Can't happen because of the database schema. */
+ctl_error(ctx, "qos %s is not part of any logical switch",
+  ctx->argv[1]);
+}
+
 static int
 meter_cmp(const void *meter1_, const void *meter2_)
 {
@@ -5652,8 +5743,9 @@ static const struct ctl_command_syntax nbctl_commands[] = 
{
 { "qos-add", 5, 7,
   "SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP]",
   NULL, nbctl_qos_add, NULL, "--may-exist", RW },
-{ "qos-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]",