Re: [ovs-dev] [PATCH v15] Improved Packet Drop Statistics in OVS

[0-day Robot]

Bleep bloop.  Greetings Anju Thomas via dev, 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.


build:
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/nx-match.lo -MD -MP -MF $depbase.Tpo -c -o lib/nx-match.lo 
lib/nx-match.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/nx-match.lo -MD -MP -MF lib/.deps/nx-match.Tpo 
-c lib/nx-match.c -o lib/nx-match.o
depbase=`echo lib/object-collection.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/object-collection.lo -MD -MP -MF $depbase.Tpo -c -o 
lib/object-collection.lo lib/object-collection.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/object-collection.lo -MD -MP -MF 
lib/.deps/object-collection.Tpo -c lib/object-collection.c -o 
lib/object-collection.o
depbase=`echo lib/odp-execute.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/odp-execute.lo -MD -MP -MF $depbase.Tpo -c -o lib/odp-execute.lo 
lib/odp-execute.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/odp-execute.lo -MD -MP -MF 
lib/.deps/odp-execute.Tpo -c lib/odp-execute.c -o lib/odp-execute.o
depbase=`echo lib/odp-util.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. 
   -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall 
-Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security 
-Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align 
-Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes 
-Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g 
-O2 -MT lib/odp-util.lo -MD -MP -MF $depbase.Tpo -c -o lib/odp-util.lo 
lib/odp-util.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include 
-I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
-Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow 
-Werror -Werror -g -O2 -MT lib/odp-util.lo -MD -MP -MF lib/.deps/odp-util.Tpo 
-c lib/odp-util.c -o lib/odp-util.o
lib/odp-util.c: In function 'odp_actions_from_string':
lib/odp-util.c:2581:22: error: unused variable 'drop_action' 
[-Werror=unused-variable]
 enum xlate_error drop_action;
  ^
cc1: all warnings 

[ovs-dev] [PATCH ovn] docs: Add note about RBAC and remote ovn-northd connection

[Frode Nordahl]

Signed-off-by: Frode Nordahl 
Acked-by: Aliasgar Ginwala 
Submitted-at: https://github.com/ovn-org/ovn/pull/25
---
 .../topics/role-based-access-control.rst  |  7 ++
 Documentation/tutorials/ovn-rbac.rst  | 25 +++
 2 files changed, 32 insertions(+)

diff --git a/Documentation/topics/role-based-access-control.rst
b/Documentation/topics/role-based-access-control.rst
index 2acd1e88b..e13e2d5dc 100644
--- a/Documentation/topics/role-based-access-control.rst
+++ b/Documentation/topics/role-based-access-control.rst
@@ -82,6 +82,13 @@ command:

$ ovn-sbctl set-connection role=ovn-controller ssl:192.168.0.1:6642

+.. note::
+
+   There is currently no pre-defined role for ovn-northd. You must configure
+   a separate listener on the OVN southbound database that ovn-northd can
+   connect to if your deployment topology require ovn-northd to connect to a
+   OVN southbound database instance on a remote machine.
+
 Pre-defined Roles
 -
 This section describes roles that have been defined internally by OVS/OVN.
diff --git a/Documentation/tutorials/ovn-rbac.rst
b/Documentation/tutorials/ovn-rbac.rst
index 22b169d6d..fc2de5d5d 100644
--- a/Documentation/tutorials/ovn-rbac.rst
+++ b/Documentation/tutorials/ovn-rbac.rst
@@ -132,3 +132,28 @@ Configuring RBAC
 /path/to/chassis_2-cert.pem /path/to/cacert.pem
   $ ovs-vsctl set open_vswitch . \
 external_ids:ovn-remote=ssl:machine_3-ip:6642
+
+The OVN central control daemon and RBAC
+~~~
+
+The OVN central control daemon (`ovn-northd`) needs full write access to
+the southbound database. When you have one machine hosting the central
+components, `ovn-northd` can talk to the databases through a local unix
+socket, bypassing the `ovn-controller` RBAC configured for the listener
+at port '6642'. However, if you want to deploy multiple machines for
+hosting the central components, `ovn-northd` will require a remote
+connection to all of them.
+
+1. Configure the southbound database with a second SSL listener on a
+   separate port without RBAC enabled for use by `ovn-northd`.
+
+   In `machine_3`::
+
+  $ ovn-sbctl -- --id=@conn_uuid create Connection \
+  target="pssl\:16642" \
+  -- add  SB_Global . connections=@conn_uuid
+
+   .. note::
+
+ Care should be taken to restrict access to the above mentioned port
+ so that only trusted machines can connect to it.
--
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v15] Improved Packet Drop Statistics in OVS

[Anju Thomas via dev]

Currently OVS maintains explicit packet drop/error counters only on port level. 
Packets that are dropped as part of normal OpenFlow processing are counted in 
flow stats of “drop” flows or as table misses in table stats. These can only be 
interpreted by controllers that know the semantics of the configured OpenFlow 
pipeline.
Without that knowledge, it is impossible for an OVS user to obtain e.g. the 
total number of packets dropped due to OpenFlow rules.

Furthermore, there are numerous other reasons for which packets can be dropped 
by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further 
expensive upcalls to the slow path, but subsequent packets dropped by the 
datapath are not accounted anywhere.

Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult for OVS 
users to monitor packet drop in an OVS instance and to alert a management 
system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.

With this patch we implement following changes to address the issues mentioned 
above.

1. Identify and account all the silent packet drop scenarios

2. Display these drops in ovs-appctl coverage/show

Co-authored-by: Rohith Basavaraja 
Co-authored-by: Keshav Gupta 
Signed-off-by: Anju Thomas 
Signed-off-by: Rohith Basavaraja 
Signed-off-by: Keshav Gupta 
Acked-by: Eelco Chaudron size
 3. Add version history

 v12(Ben's comments)
 1. new dp action in kernel block in openvswitch.h
 2. change xlate_error enum to be used as u32 3. resetting xlate error in case 
of congestion drop
 and forwarding disabled
---
 datapath/linux/compat/include/linux/openvswitch.h |  18 ++
 lib/dpif-netdev.c |  47 +-
 lib/dpif.c|   7 +
 lib/dpif.h|   2 +
 lib/odp-execute.c |  78 +
 lib/odp-util.c|   7 +
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  38 -
 ofproto/ofproto-dpif-xlate.h  |   7 +-
 ofproto/ofproto-dpif.c|   8 +
 ofproto/ofproto-dpif.h|   8 +-
 tests/automake.mk |   3 +-
 tests/dpif-netdev.at  |   8 +
 tests/drop-stats.at   | 190 ++
 tests/ofproto-dpif.at |   2 +-
 tests/testsuite.at|   1 +
 tests/tunnel-push-pop.at  |  28 +++-
 tests/tunnel.at   |  16 +-
 19 files changed, 457 insertions(+), 13 deletions(-)
 create mode 100644 tests/drop-stats.at

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 7b16b1d..60525a2 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -403,6 +403,22 @@ enum ovs_tunnel_key_attr {
__OVS_TUNNEL_KEY_ATTR_MAX
 };
 
+enum xlate_error {
+XLATE_OK = 0,
+XLATE_BRIDGE_NOT_FOUND,
+XLATE_RECURSION_TOO_DEEP,
+XLATE_TOO_MANY_RESUBMITS,
+XLATE_STACK_TOO_DEEP,
+XLATE_NO_RECIRCULATION_CONTEXT,
+XLATE_RECIRCULATION_CONFLICT,
+XLATE_TOO_MANY_MPLS_LABELS,
+XLATE_INVALID_TUNNEL_METADATA,
+XLATE_UNSUPPORTED_PACKET_TYPE,
+XLATE_CONGESTION_DROP,
+XLATE_FORWARDING_DISABLED,
+XLATE_MAX,
+};
+
 #define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1)
 
 /**
@@ -961,6 +977,7 @@ struct check_pkt_len_arg {
  * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
  * of actions if greater than the specified packet length, else execute
  * another set of actions.
+ * @OVS_ACTION_ATTR_DROP: Explicit drop action.
  */
 
 enum ovs_action_attr {
@@ -993,6 +1010,7 @@ enum ovs_action_attr {
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+   OVS_ACTION_ATTR_DROP,  /* explicit drop action. */
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4720ba1..8835158 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of 
meters. */
 enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
 enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
 
+COVERAGE_DEFINE(datapath_drop_meter);

Re: [ovs-dev] BUG: MAX_LOCKDEP_ENTRIES too low!

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit:99a8efbb NFC: st21nfca: fix double free
git tree:   net
console output: https://syzkaller.appspot.com/x/log.txt?x=15ed70d8e0
kernel config:  https://syzkaller.appspot.com/x/.config?x=cbbed3e8d4eb64bf
dashboard link: https://syzkaller.appspot.com/bug?extid=cd0ec5211ac07c18c049
compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=13cf5594e0
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1036c762e0

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+cd0ec5211ac07c18c...@syzkaller.appspotmail.com

device 5580n entered promiscuous mode
BUG: MAX_LOCKDEP_ENTRIES too low!
turning off the locking correctness validator.
CPU: 0 PID: 14197 Comm: syz-executor527 Not tainted 5.4.0-rc5+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011

Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x172/0x1f0 lib/dump_stack.c:113
 alloc_list_entry.cold+0x11/0x18 kernel/locking/lockdep.c:1292
 add_lock_to_list kernel/locking/lockdep.c:1313 [inline]
 check_prev_add kernel/locking/lockdep.c:2528 [inline]
 check_prevs_add kernel/locking/lockdep.c:2581 [inline]
 validate_chain kernel/locking/lockdep.c:2971 [inline]
 __lock_acquire+0x2a15/0x4a00 kernel/locking/lockdep.c:3955
 lock_acquire+0x190/0x410 kernel/locking/lockdep.c:4487
 __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
 _raw_spin_lock_bh+0x33/0x50 kernel/locking/spinlock.c:175
 spin_lock_bh include/linux/spinlock.h:343 [inline]
 netif_addr_lock_bh include/linux/netdevice.h:4055 [inline]
 dev_set_rx_mode+0x20/0x40 net/core/dev.c:7808
 dev_set_promiscuity+0xbf/0xe0 net/core/dev.c:7716
 internal_dev_create+0x387/0x550 net/openvswitch/vport-internal_dev.c:196
 ovs_vport_add+0x150/0x500 net/openvswitch/vport.c:199
 new_vport+0x1b/0x1d0 net/openvswitch/datapath.c:194
 ovs_dp_cmd_new+0x5e5/0xe30 net/openvswitch/datapath.c:1644
 genl_family_rcv_msg+0x74b/0xf90 net/netlink/genetlink.c:629
 genl_rcv_msg+0xca/0x170 net/netlink/genetlink.c:654
 netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
 genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
 netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
 netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
 netlink_sendmsg+0x8a5/0xd60 net/netlink/af_netlink.c:1917
 sock_sendmsg_nosec net/socket.c:637 [inline]
 sock_sendmsg+0xd7/0x130 net/socket.c:657
 ___sys_sendmsg+0x803/0x920 net/socket.c:2311
 __sys_sendmsg+0x105/0x1d0 net/socket.c:2356
 __do_sys_sendmsg net/socket.c:2365 [inline]
 __se_sys_sendmsg net/socket.c:2363 [inline]
 __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2363
 do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x441779
Code: e8 9c ad 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 1b 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00

RSP: 002b:7ffea7e5fcc8 EFLAGS: 0246 ORIG_RAX: 002e
RAX: ffda RBX: 0003 RCX: 00441779
RDX:  RSI: 2240 RDI: 0003
RBP: 00058f66 R08: 7ffe0025 R09: 7ffe0025
R10: 0004 R11: 0246 R12: 006cdbc0
R13: 0013 R14:  R15: 

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

Re: [ovs-dev] [PATCH] netdev-afxdp: add afxdp specific maximum MTU check

[William Tu]

On Thu, Nov 07, 2019 at 03:01:18PM +0100, Eelco Chaudron wrote:
> Any feedback on this?
> 
> 
> On 1 Oct 2019, at 11:55, Eelco Chaudron wrote:
> 
> >Drivers natively supporting AF_XDP will check that a configured MTU size
> >will not exceed the allowed size for AF_XDP. However, when the skb
> >compatibility mode is used there is no check and any value is accepted.
> >This, for example, is the case when using the TAP interface.
> >
> >This fix adds a check to make sure only AF_XDP valid values are excepted.
> >
> >Signed-off-by: Eelco Chaudron 
> >---
> > lib/netdev-afxdp.c |   17 +
> > lib/netdev-afxdp.h |1 +
> > lib/netdev-linux.c |9 +
> > 3 files changed, 27 insertions(+)
> >
> >diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> >index 6e0180327..140150f29 100644
> >--- a/lib/netdev-afxdp.c
> >+++ b/lib/netdev-afxdp.c
> >@@ -1001,6 +1001,23 @@ netdev_afxdp_destruct(struct netdev *netdev)
> > ovs_mutex_destroy(>mutex);
> > }
> >
> >+int
> >+netdev_afxdp_verify_mtu_size(const struct netdev *netdev, int mtu)
> >+{
> >+/*
> >+ * If a device is used in xdpmode skb, no driver-specific MTU size is
> >+ * checked and any value is allowed resulting in packet drops.
> >+ * This check will verify the maximum supported value based on the
> >+ * buffer size allocated and the additional headroom required.
> >+ */
> >+if (netdev == NULL || mtu <= 0
> >+|| mtu > (FRAME_SIZE - OVS_XDP_HEADROOM - XDP_PACKET_HEADROOM)) {

I remember XDP max MTU = 3520 bytes,
and it's (page_size(4096) - headroom(256) - shinfo(320))
so here we should also subtract shinfo?

Regards,
William

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

[ovs-dev] L2 acl on the logical switch

[venugopal iyer via dev]

[Pardon the length of the mail; have left out the ofctl flows, but if it helps 
i can send them too]
Assuming :        logical Switch (ls1) with                                   
ls1_vm1 : 02:ac:10:ff:00:11/172.16.255.11                                  
ls1_vm2 : 02:ac:10:ff:00:22/172.16.255.22                                  
ls1_vm3 : 02:ac:10:ff:00:33/172.16.255.33
Looking to :        have MAC ACLs between vm1 and vm2 so they can't talk to 
each other.
Using:        # ovn-nbctl  create Address_Set name=macs 
addresses=\"02:ac:10:ff:00:11\",\"02:ac:10:ff:00:22\"        # ovn-nbctl create 
Port_Group name=l2pg        # ovn-nbctl add Port_Group l2pg ports      
   # ovn-nbctl add Port_Group l2pg ports         # ovn-nbctl acl-add 
ls1 to-lport 32767  "outport == @l2pg && eth.src == \$macs" drop
What we are seeing:      The above, by itself, will limit L3 and L2 between vm1 
and vm2, but not vm3 (as expected)
lflow:          table=4 (ls_out_acl         ), 
priority=33767, match=(outport == @l2pg && eth.src == $macs), action=(/* drop 
*/)
dpflow (sending a arbit packet of ether type 0x7a05):--     
   
(0),in_port(vm1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x7a05),
 packets:0, bytes:0, used:never, actions:drop
However, if there is a stateful rule, e.g.:        # ovn-nbctl acl-add ls1 
from-lport 2000  "inport == \"ls1-vm3\" && ip" allow-related
Then, the behavior differs
lflow:---          table=6 (ls_in_acl          ), priority=3000 
, match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 0 && (inport == 
"ls1-vm3" && ip)), action=(next;)          table=6 (ls_in_acl          ), 
priority=3000 , match=(((ct.new && !ct.est) || (!ct.new && ct.est && !ct.rpl && 
ct_label.blocked == 1)) && (inport == "ls1-vm3" && ip)), action=(reg0[1] = 1; 
next;)          table=4 (ls_out_acl         ), priority=33767, match=((!ct.est 
|| (ct.est && ct_label.blocked == 1)) && (outport == @l2pg && eth.src == 
$macs)), action=(/* drop */)          table=4 (ls_out_acl         ), 
priority=33767, match=(ct.est && ct_label.blocked == 0 && (outport == @l2pg && 
eth.src == $macs)), action=(ct_commit(ct_label=1/1); /* drop */)
*Note: the condition !ct.new && ct.est && !ct.rpl && ct_label.blocked == 
0**
This does block L3 traffic:
dpctl flow for ICMP-           
recirc_id(0),in_port(vm1),eth(src=02:ac:10:ff:00:11),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0xf8),
 packets:6, bytes:588, used:0.864s, actions:ct(zone=1),recirc(0x1)           
recirc_id(0x1),in_port(vm1),ct_state(+new-est-rel-rpl-inv+trk),ct_label(0/0x1),eth(dst=02:ac:10:ff:00:22),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0xf8),
 packets:6, bytes:588, used:0.864s, 
actions:ct(commit,zone=1,label=0/0x1),ct(zone=4),recirc(0x2)           
recirc_id(0x2),in_port(vm1),ct_state(+new-est-rel-rpl-inv+trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11),eth_type(0x0800),ipv4(frag=no),
 packets:6, bytes:588, used:0.864s, actions:drop           
recirc_id(0),in_port(vm1),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x0806),arp(sha=02:ac:10:ff:00:11),
 packets:0, bytes:0, used:never, actions:vm2           
recirc_id(0),in_port(vm2),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:22,dst=02:ac:10:ff:00:11),eth_type(0x0806),arp(sha=02:ac:10:ff:00:22),
 packets:0, bytes:0, used:never, actions:vm1

But, it doesn't block L2, see the ARP going through, without the stateful rule 
the ARP would have been dropped too.
Using the arbit packet:
           
recirc_id(0),in_port(vm1),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x7a05),
 packets:0, bytes:0, used:never, actions:vm2


From what i understand it is because of the condition "!ct.new && ct.est && 
!ct.rpl && ct_label.blocked == 0", which implies trk, I suppose, which makes 
sense for IP packets, but probably not for non-IP packets, i.e. the  rules 
above have "-trk". Hence, I believe it gets through

Proposed changes:
If my understanding is right (and objective to remove the inconsistency between 
when there are stateful rules and not, w.r.t. L2 packets), we couldhave the 
condition as:
# git diffdiff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.cindex 
6c6de2afd..b0f524531 100644--- a/ovn/northd/ovn-northd.c+++ 
b/ovn/northd/ovn-northd.c@@ -4191,10 +4191,13 @@ 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 part of an 
established connection, then+             * we can simply reject/drop it.+      

Re: [ovs-dev] [RFC ovn PATCH 0/5] Separate pinctrl to its own process

[Mark Michelson]

Hi Han, I had some time to get back to this. See my comments below.

On 10/21/19 4:01 PM, Han Zhou wrote:

Hi Mark,

Thanks for the patch. We had a brief discussion during last OVN meeting. 
Let me put my points inlined.


On Fri, Oct 18, 2019 at 1:43 PM Mark Michelson > wrote:

 >
 > This proposes a set of patches to move pinctrl operations out of the
 > ovn-controller process and into its own.
 >
 > The main reasons for doing this are the following:
 > 1) Separating pinctrl makes it so that receiving a packet-in can't wake
 > up ovn-controller.

To avoid waking up ovn-controller, it doesn't have to be in a separate 
process. A thread with its own OVSDB IDL to SB DB can achieve the same, 
as what this old patch did: 
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332887.html


However, the problem of a separate SB connection introduced the concern 
for scalability. There were discussions and thoughts for a separate 
thread without introducing new SB connection, but once the two threads 
share same SB connection, there has to be some synchronization between 
the threads that ends up waking up or blocking each other whenever there 
is a pinctrl processing that requires read/write SB data. The current 
multi-thread implementation from Numan is a trade off that avoids new SB 
connection but syncing with the main thread when SB data is needed. It 
is perfect for pinctrl handling that doesn't require SB data, and then 
wakes up ovn-controller for updating SB data.


I followed the discussions that resulted from the patch you sent. It 
looks like the concerns are that you either have to


1) Have two separate connections to the SB database, resulting in double 
the connections (this is what my patch does)
2) Have one connection to the SB database but synchronise the efforts of 
the different concerns of ovn-controller (namely logical flow processing 
and pinctrl processing).


1 is easy but resource intensive, and 2 is difficult but has the 
potential for not having the same bottlenecks.


But this has me thinking. The current IDL code assumes that one IDL 
client == one database connection. I suppose it may be possible to alter 
the OVSDB IDL code so that a single connection could be shared by 
multiple IDL clients. In other words, you could create the SB 
connection, then create a separate thread. Each thread would then create 
an IDL that makes use of the same connection. The OVSDB client code 
would need to be altered to be able to notify multiple IDLs about 
changes. The client would also need to be modified to be thread safe, in 
the case that multiple threads want to write to the database at once.


This would allow for multithreading, and the controller code wouldn't 
need to worry about synchronization. Each thread would have its own data 
it manipulates, and the synchronization would be handled by the IDL itself.


Having said all this, though, it would not be a trivial task to 
implement this. And so the question becomes, is it worth it?




Today (2.12) there were improvements on both ovn-controller and OVSDB 
server, that alleviated the scale problems on both side.
- For ovn-controller, with incremental processing, when there is no 
input change, it doesn't trigger flow recomputing, even when pinctrl 
wakes up the main thread. The major concern may be when main thread does 
need a recompute, it could block pinctrl processing for messages that 
requires SB data accessing, such as ARP handling.

- For SB DB
   - Active-active cluster alleviates the burden of a single server and 
spread to 3 or 5. However, RAFT is not designed for scale. Write always 
happen on the leader node, and the cost of cluster sync between leader 
and follower becomes higher when number of nodes increases.
   - The fast-resync feature (requiring active-active clustered mode) 
avoids the slowness of data resync to all clients after DB 
restart/failover. However, it doesn't help if ovsdb-server is overloaded 
for regular updates and notifications during normal operations, given 
that it is single threaded. Also, there are corner cases that 
fast-resync doesn't help, e.g. when DB restart/failover happened just 
after a compress, when all the transaction history is lost.


I'd suggest to reconsider these scalability concerns, the pros and cons 
for a dedicated SB connection for pinctrl, before moving forward to this 
approach.


 > 2) Separating pinctrl allows for manipulating the southbound database
 > directly while handling packets in, thus minimizing the need for storing
 > local copies of data

This is true, but similar as point 1), it doesn't necessarily need a 
separate process. The point is whether pinctrl (thread or process) 
should use a dedicated SB connection.


Yep, you're definitely correct here. I guess my thought here was that if 
the two threads have no need to share any memory or state, then it makes 
more sense for them to exist as two processes instead.


However, what I 

[ovs-dev] Investment opportunity

[Peter Wong]

Greetings,

Find attached email very confidential. reply for more details

Thanks.
Peter Wong





This email was sent by the shareware version of Postman Professional.

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

[ovs-dev] Determinación de metas y periodos de cálculo

[Medición de indicadores y mejora de procesos]

28 de Noviembre | Horario de 10:00 a 17:00 hrs.  |  (hora del centro de México) 

Experto en Indicadores Clave de Desempeño

Este webinar está dirigido al personal de las distintas áreas de la 
organización que requieren
medir el desempeño de sus procesos y profesionales dedicados a la mejora 
continua. Está
diseñado para explicar la importancia de la utilización de indicadores de 
desempeño, identificar
sus diferentes tipos y determinar los mecanismos de monitoreo para evaluar el 
cumplimiento y la
alineación a los objetivos clave de la empresa, pudiendo generar acciones de 
mejora.
Utilizaremos una metodología práctica y ordenada para la generación de 
indicadores de
desempeño que pueda ser aplicable a cualquier tipo de organización y le permita 
mejorarlos.

Solicita información respondiendo a este correo con la palabra Indicadores, 
junto con los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Dirigido a: Personal del área de recursos humanos, calidad, gerencias de área, 
profesionales de las áreas operativas y administrativas de la organización.

Números de Atención: (045) 55 15 54 66 30 - (045) 55 85567293 - (045) 
5530167085 

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder con la palabra baja o 
enviar un correo a bajas@ innovalearn.net



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

Re: [ovs-dev] [PATCH ovn] Remove python directory

[Numan Siddique]

On Wed, Nov 6, 2019, 11:50 PM Mark Michelson  wrote:

> Acked-by: Mark Michelson 
>

Thanks for the review. Looks like the patch failed in CI.

Below changes in Makefile.am fixed it.

I applied this patch to master with the below additional changes.

Thanks
Numan

diff --git a/Makefile.am b/Makefile.am
index 88ede2d82..1e41e49ea 100644
--- a/Makefile.am
+++ b/Makefile.am
 ALL_LOCAL =
@@ -165,7 +165,7 @@ ro_shell = printf '\043 Generated automatically -- do
not modify!-*- buffer-

 SUFFIXES += .in
 .in:
-   $(AM_V_GEN)PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON)
$(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $< | \
+
$(AM_V_GEN)PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python
$(PYTHON) $(srcdir)/build-aux/soexpand.py -I$(srcdir) -I$(OVS_SRCDIR) < $<
| \
  $(PYTHON) $(srcdir)/build-aux/dpdkstrip.py $(DPDKSTRIP_FLAGS) | \
  sed \
-e 's,[@]PKIDIR[@],$(PKIDIR),g' \
@@ -424,8 +424,8 @@ endif
 CLEANFILES += flake8-check

 include $(srcdir)/manpages.mk
-$(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py
python/build/soutil.py
-   @PYTHONPATH=$$PYTHONPATH$(psep)$(srcdir)/python $(PYTHON)
$(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR)
$(MAN_ROOTS) >$(@F).tmp
+$(srcdir)/manpages.mk: $(MAN_ROOTS) build-aux/sodepends.py
$(OVS_SRCDIR)/python/build/soutil.py
+
@PYTHONPATH=$(OVS_SRCDIR)/python$(psep)$$PYTHONPATH$(psep)$(srcdir)/python
$(PYTHON) $(srcdir)/build-aux/sodepends.py -I. -I$(srcdir) -I$(OVS_MANDIR)
$(MAN_ROOTS) >$(@F).tmp
@if cmp -s $(@F).tmp $@; then \
  touch $@; \
  rm -f $(@F).tmp; \


> On 11/6/19 6:52 AM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > The python/ directory belongs to Open vSwitch repo.
> > This patch uses the python utils required for building OVN from
> > the configured OVS source directory and deletes the python directory.
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   Makefile.am   |5 +-
> >   python/.gitignore |2 -
> >   python/README.rst |1 -
> >   python/automake.mk|  123 -
> >   python/build/__init__.py  |0
> >   python/build/nroff.py |  398 ---
> >   python/build/soutil.py|   56 -
> >   python/ovs/.gitignore |1 -
> >   python/ovs/__init__.py|1 -
> >   python/ovs/_json.c|  269 --
> >   python/ovs/compat/__init__.py |0
> >   python/ovs/compat/sortedcontainers/LICENSE|   13 -
> >   .../ovs/compat/sortedcontainers/__init__.py   |   52 -
> >   .../ovs/compat/sortedcontainers/sorteddict.py |  741 -
> >   .../ovs/compat/sortedcontainers/sortedlist.py | 2508 -
> >   .../ovs/compat/sortedcontainers/sortedset.py  |  327 ---
> >   python/ovs/daemon.py  |  652 -
> >   python/ovs/db/__init__.py |1 -
> >   python/ovs/db/custom_index.py |  154 -
> >   python/ovs/db/data.py |  585 
> >   python/ovs/db/error.py|   34 -
> >   python/ovs/db/idl.py  | 2030 -
> >   python/ovs/db/parser.py   |  118 -
> >   python/ovs/db/schema.py   |  304 --
> >   python/ovs/db/types.py|  647 -
> >   python/ovs/dirs.py|   31 -
> >   python/ovs/dirs.py.template   |   31 -
> >   python/ovs/fatal_signal.py|  183 --
> >   python/ovs/fcntl_win.py   |   46 -
> >   python/ovs/json.py|  531 
> >   python/ovs/jsonrpc.py |  616 
> >   python/ovs/ovsuuid.py |   70 -
> >   python/ovs/poller.py  |  290 --
> >   python/ovs/process.py |   41 -
> >   python/ovs/reconnect.py   |  608 
> >   python/ovs/socket_util.py |  335 ---
> >   python/ovs/stream.py  |  831 --
> >   python/ovs/timeval.py |   81 -
> >   python/ovs/unixctl/__init__.py|   91 -
> >   python/ovs/unixctl/client.py  |   68 -
> >   python/ovs/unixctl/server.py  |  260 --
> >   python/ovs/util.py|   95 -
> >   python/ovs/vlog.py|  475 
> >   python/ovs/winutils.py|  266 --
> >   python/ovstest/__init__.py|1 -
> >   python/ovstest/args.py|  283 --
> >   python/ovstest/rpcserver.py   |  383 ---
> >   python/ovstest/tcp.py |  120 -
> >   

Re: [ovs-dev] [PATCH ovn v5] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

[Dumitru Ceara]

On Thu, Nov 7, 2019 at 6:02 PM Han Zhou  wrote:
>
>
>
> On Thu, Nov 7, 2019 at 7:43 AM Dumitru Ceara  wrote:
> >
> > On Thu, Nov 7, 2019 at 3:51 AM Han Zhou  wrote:
> > >
> > >
> > >
> > > On Mon, Nov 4, 2019 at 11:32 AM Numan Siddique  wrote:
> > > >
> > > > On Mon, Nov 4, 2019 at 8:37 PM Dumitru Ceara  wrote:
> > > > >
> > > > > ARP request and ND NS packets for router owned IPs were being
> > > > > flooded in the complete L2 domain (using the MC_FLOOD multicast 
> > > > > group).
> > > > > However this creates a scaling issue in scenarios where aggregation
> > > > > logical switches are connected to more logical routers (~350). The
> > > > > logical pipelines of all routers would have to be executed before the
> > > > > packet is finally replied to by a single router, the owner of the IP
> > > > > address.
> > > > >
> > > > > This commit limits the broadcast domain by bypassing the L2 Lookup 
> > > > > stage
> > > > > for ARP requests that will be replied by a single router. The packets
> > > > > are still flooded in the L2 domain but not on any of the other patch
> > > > > ports towards other routers connected to the switch. This restricted
> > > > > flooding is done by using a new multicast group (MC_ARP_ND_FLOOD).
> > > > >
> > > > > IPs that are owned by the routers and for which this fix applies are:
> > > > > - IP addresses configured on the router ports.
> > > > > - VIPs.
> > > > > - NAT IPs.
> > > > >
> > > > > This commit also fixes function get_router_load_balancer_ips() which
> > > > > was incorrectly returning a single address_family even though the
> > > > > IP set could contain a mix of IPv4 and IPv6 addresses.
> > > > >
> > > > > Reported-at: https://bugzilla.redhat.com/1756945
> > > > > Reported-by: Anil Venkata 
> > > > > Signed-off-by: Dumitru Ceara 
> > > >
> > > > Thanks Dumitru, for addressing the review comments.
> > > >
> > > > Acked-by: Numan Siddique 
> > > >
> > > > Han, if you can take a look in this patch and provide your comments,
> > > > that would be great.
> > > >
> >
> > Hi Han,
> >
> > >
> > > Sorry for delayed response :(
> >
> > No worries and thanks for reviewing this change.
> >
> > >
> > > The patch looks good to me except:
> > >
> > > 1. It changes the behavior that when an ARP request for a LRP's IP is 
> > > coming to the logical switch, other routers will no longer learn the 
> > > MAC-IP binding of the ARP sender. This has been discussed and I tend to 
> > > agree with Dumitru that it should not cause real issue. I think it just 
> > > worth to be documented clearly in the ovn-architecture, probably in the 
> > > section: Logical Routers and Logical Patch Ports, because this is 
> > > something different from a traditional switch's behavior, and network 
> > > administrators may get suprised.
> >
> > Ack, I'll add this in v6.
> >
> > >
> > > 2. Since we are anyway changing the behavior of ARP request handling and 
> > > bypassing logical router ports, and we think it should not cause real 
> > > problem, then I wonder why adding the MC_ARP_ND group to still flood to 
> > > the non-router ports. Is it really useful? Maybe it is not a big deal, 
> > > but I think the extra MC group would add some performance cost and it is 
> > > almost redundant with the regular MC_FLOOD except that there is no router 
> > > ports in it - it is a lot of redundancy considering that most LSPs are 
> > > regular VIFs in typical large scale environments. I would suggest to 
> > > simplify this unless there is clear concerns of not flooding to other 
> > > ports.
> >
> > If I skip flooding to non-router ports the following test fails
> > because one of the things checked by the test is that we flood
> > broadcasts (including ARPs) in the broadcast domain:
> >  36: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR   FAILED 
> > (ovs-macros.at:220)
> >
> > So my concern was that people might expect the ARP requests to be
> > flooded within the L2 broadcast domain.
> > However, we could add configuration knob to change the behavior and
> > only flood them on localnet ports. Like this we could maintain the
> > current L2 flooding but in deployments where this is not necessary the
> > users could disable the flooding to VIF ports and this would remove
> > the MC_ARP_ND group unless there's a localnet port in the datapath.
> >
> > What do you think?
> >
>
> Adding configuration knob is a valid option, but I hoped we could simplify 
> the change instead of making it even more complex :)
>
> My point is that since we changed the behavior, it might be better to change 
> it with a more straightforward philosophy: for ARP request for an IP that is 
> owned by a LRP, it is not flooded. This is in fact comply with our ARP 
> responder behavior in LS: for a known IP-MAC binding, the ARP request is not 
> flooded but directly responded to the sender. Now with your change we are 
> just extending this behavior for IPs owned by LRPs behind the LSPs with type 
> "router".

Sounds good to me.

>
> I 

[ovs-dev] Cómo elaborar estados financieros

[Excel]

Nuestro webinar le permitirá al participante realizar un análisis profundo de 
la situación económica y financiera de su organización en un momento 
determinado así como aplicar
 las principales herramientas de análisis e interpretación de los indicadores 
económicos y financieros para obtener conclusiones válidas acerca de la 
solvencia y liquidez de la 
organización.

Curso en Línea "Taller de elaboración de estados financieros con Excel"
Martes 10 de Diciembre con un horario de 10:00 a 17:00 hrs. (hora del centro de 
México)


Objetivos Específicos:

- El participante elaborará estados financieros con la herramienta Excel.
- El participante analizará las situaciones económica financiera de su 
organización.
- El participante analizará las causas de los resultadosobtenidos, ya sean 
positivos o negativos y ser capaces
de entender la motivación a fin de tomar decisiones.

Teléfonos: (045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 

Solicita información respondiendo a este correo con la palabra Excel, junto con 
los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Qué tengas un excelente día

Si desea dejar de recibir nuestra promoción favor de responder con la palabra 
baja o enviar un correo a bajas@ innovalearn.net


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

Re: [ovs-dev] [PATCH ovn v5] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

[Han Zhou]

On Thu, Nov 7, 2019 at 7:43 AM Dumitru Ceara  wrote:
>
> On Thu, Nov 7, 2019 at 3:51 AM Han Zhou  wrote:
> >
> >
> >
> > On Mon, Nov 4, 2019 at 11:32 AM Numan Siddique  wrote:
> > >
> > > On Mon, Nov 4, 2019 at 8:37 PM Dumitru Ceara 
wrote:
> > > >
> > > > ARP request and ND NS packets for router owned IPs were being
> > > > flooded in the complete L2 domain (using the MC_FLOOD multicast
group).
> > > > However this creates a scaling issue in scenarios where aggregation
> > > > logical switches are connected to more logical routers (~350). The
> > > > logical pipelines of all routers would have to be executed before
the
> > > > packet is finally replied to by a single router, the owner of the IP
> > > > address.
> > > >
> > > > This commit limits the broadcast domain by bypassing the L2 Lookup
stage
> > > > for ARP requests that will be replied by a single router. The
packets
> > > > are still flooded in the L2 domain but not on any of the other patch
> > > > ports towards other routers connected to the switch. This restricted
> > > > flooding is done by using a new multicast group (MC_ARP_ND_FLOOD).
> > > >
> > > > IPs that are owned by the routers and for which this fix applies
are:
> > > > - IP addresses configured on the router ports.
> > > > - VIPs.
> > > > - NAT IPs.
> > > >
> > > > This commit also fixes function get_router_load_balancer_ips() which
> > > > was incorrectly returning a single address_family even though the
> > > > IP set could contain a mix of IPv4 and IPv6 addresses.
> > > >
> > > > Reported-at: https://bugzilla.redhat.com/1756945
> > > > Reported-by: Anil Venkata 
> > > > Signed-off-by: Dumitru Ceara 
> > >
> > > Thanks Dumitru, for addressing the review comments.
> > >
> > > Acked-by: Numan Siddique 
> > >
> > > Han, if you can take a look in this patch and provide your comments,
> > > that would be great.
> > >
>
> Hi Han,
>
> >
> > Sorry for delayed response :(
>
> No worries and thanks for reviewing this change.
>
> >
> > The patch looks good to me except:
> >
> > 1. It changes the behavior that when an ARP request for a LRP's IP is
coming to the logical switch, other routers will no longer learn the MAC-IP
binding of the ARP sender. This has been discussed and I tend to agree with
Dumitru that it should not cause real issue. I think it just worth to be
documented clearly in the ovn-architecture, probably in the section:
Logical Routers and Logical Patch Ports, because this is something
different from a traditional switch's behavior, and network administrators
may get suprised.
>
> Ack, I'll add this in v6.
>
> >
> > 2. Since we are anyway changing the behavior of ARP request handling
and bypassing logical router ports, and we think it should not cause real
problem, then I wonder why adding the MC_ARP_ND group to still flood to the
non-router ports. Is it really useful? Maybe it is not a big deal, but I
think the extra MC group would add some performance cost and it is almost
redundant with the regular MC_FLOOD except that there is no router ports in
it - it is a lot of redundancy considering that most LSPs are regular VIFs
in typical large scale environments. I would suggest to simplify this
unless there is clear concerns of not flooding to other ports.
>
> If I skip flooding to non-router ports the following test fails
> because one of the things checked by the test is that we flood
> broadcasts (including ARPs) in the broadcast domain:
>  36: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR   FAILED (
ovs-macros.at:220)
>
> So my concern was that people might expect the ARP requests to be
> flooded within the L2 broadcast domain.
> However, we could add configuration knob to change the behavior and
> only flood them on localnet ports. Like this we could maintain the
> current L2 flooding but in deployments where this is not necessary the
> users could disable the flooding to VIF ports and this would remove
> the MC_ARP_ND group unless there's a localnet port in the datapath.
>
> What do you think?
>

Adding configuration knob is a valid option, but I hoped we could simplify
the change instead of making it even more complex :)

My point is that since we changed the behavior, it might be better to
change it with a more straightforward philosophy: for ARP request for an IP
that is owned by a LRP, it is not flooded. This is in fact comply with our
ARP responder behavior in LS: for a known IP-MAC binding, the ARP request
is not flooded but directly responded to the sender. Now with your change
we are just extending this behavior for IPs owned by LRPs behind the LSPs
with type "router".

I don't think we need to worry about the test case "36: ovn -- 3 HVs, 3 LS,
3 lports/LS, 1 LR". In fact the test already considered a case where
flooding is not expected:
# 192.168.33.254 is configured to the switch patch port for
lrp33,
# so no ARP flooding expected for it.
Now we can just change the test case a little more to exclude the IPs owned
by LRPs, since this is new 

Re: [ovs-dev] can OVS conntrack support IP list like this: actions=ct(commit, table=0, zone=1, nat(dst=220.0.0.3, 220.0.0.7, 220.0.0.123))?

[Darrell Ball]

On Thu, Nov 7, 2019 at 8:05 AM Darrell Ball  wrote:

>
>
> On Wed, Nov 6, 2019 at 5:01 PM Yi Yang (杨燚)-云服务集团 
> wrote:
>
>> Thanks Darrell, I didn’t receive your second reply, I saw it in
>> mail.openvswitch.org.
>>
>>
>>
>> “
>>
>> probably, you should give an example of what you mean by above
>>
>> I am not sure you are meaning to say that you want to specify an L4 port
>> in
>>
>> your
>>
>> snat action rule or not; you will want to use ephemeral ports by not
>>
>> specifying a
>>
>> specific port in most cases
>>
>> “
>>
>>
>>
>> For SNAT, we don’t specify port, just use default port range
>> “1024-65535”), but for internal source IPs, i.e. floating IPs, they are
>> discrete in most cases because some floating IPs needn’t access Internet,
>> for public IPs, so are they. For public IPs, maybe they are from different
>> telecom carriers, we prefer egress traffic can be distributed on several
>> BGP lines.
>>
>
>>
>>
>> table=0,ip,nw_src=172.18.0.67,…,actions=ct(commit,table=0,zone=1,nat(src=220.0.0.3,230.0.0.7,240.0.0.123))
>>
>>
>> table=0,ip,nw_src=172.18.0.80,…,actions=ct(commit,table=0,zone=1,nat(src=220.0.0.3,230.0.0.7,240.0.0.123))
>>
>>
>> table=0,ip,nw_src=172.19.0.23,…,actions=ct(commit,table=0,zone=1,nat(src=220.0.0.3,230.0.0.7,240.0.0.123))
>>
>>
>>
>> Ideally, we hope, for different traffic types from the same internal IP
>> (say 172.18.0.67), some can SNAT to 220.0.0.3, some can SNAT to 230.0.0.7,
>> some can SNAT to
>>
>> 240.0.0.123, that way, they can leverage total bandwidth of several BGP
>> lines.
>>
>>
>>
>> I know current OVS can’t support the above IP list for snat, but it is
>> indeed required in reality, I don’t understand why OVS can’t do in this
>> way, is it linux conntrack limitation or what else reason? I think it is
>> similar to IP range which can be supported.
>>
>
> Presently, the limitation is both at the Openflow layer and implementation
> details at datapath
> The layer above (a controller or even a script) can do the mapping taking
> into account the desired distribution
> A controller can/will often do this and similar types of configuration
> specification.
>

btw, in terms of controller config support, Openflow Groups with select
type may help here


>
>
>>
>>
>>
>> *发件人:* Darrell Ball [mailto:dlu...@gmail.com]
>> *发送时间:* 2019年11月6日 9:38
>> *收件人:* Yi Yang (杨燚)-云服务集团 
>> *抄送:* ovs-disc...@openvswitch.org; ovs-dev@openvswitch.org
>> *主题:* Re: [ovs-dev] can OVS conntrack support IP list like this:
>> actions=ct(commit, table=0, zone=1, nat(dst=220.0.0.3, 220.0.0.7,
>> 220.0.0.123))?
>>
>>
>>
>>
>>
>>
>>
>> On Tue, Nov 5, 2019 at 4:32 PM Yi Yang (杨燚)-云服务集团 
>> wrote:
>>
>> Hi, folks
>>
>>
>>
>> We need to do SNAT for many internal IPs by just using several public IPs,
>> we also need to do DNAT by some other public IPs for exposing webservice,
>> openflow rules look like the below:
>>
>>
>>
>> table=0,ip,nw_src=172.17.0.0/16,
>> …,actions=ct(commit,table=0,zone=1,nat(src=
>> 220.0.0.3,220.0.0.7,220.0.0.123))
>>
>>
>> table=0,ip,nw_src=172.18.0.67,…,actions=ct(commit,table=0,zone=1,nat(src=22
>> 0.0.0.3,220.0.0.7,220.0.0.123))
>>
>>
>>
>> for snat, you can map some subset of private IPs to a given public IP and
>> so on
>>
>>
>>
>>
>>
>>
>> table=0,ip,tcp,nw_dst=220.0.0.11,tp_dst=80,…,actions=ct(commit,table=0,zone
>> =2,nat(dst=172.16.0.100:80))
>>
>> table=0,ip,tcp,nw_dst=220.0.0.11,
>> tp_dst=443,…,actions=ct(commit,table=0,zone=2,nat(dst=172.16.0.100:443))
>>
>>
>>
>> you are mapping 'to' private IPs, so you have control over the range
>>
>>
>>
>>
>>
>>
>>
>>
>> From ct document, it seems it can’t support IP list for nat, anybody knows
>> how we can handle such cases in some kind feasible way?
>>
>>
>>
>> In addition, is it ok if multiple openflow rules use the same NAT IP:PORT
>> combination? I’m not sure if it will result in some conflicts for SNAT,
>> because all of them need to do dynamic source port mapping, per my test,
>> it
>> seems this isn’t a problem.
>>
>>
>>
>> IIUC, as long as tuples are unique, it should be fine
>>
>>
>>
>>
>>
>>
>> Thank you all in advance and appreciate your help sincerely.
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-afxdp: add afxdp specific maximum MTU check

[Ilya Maximets]

On 07.11.2019 15:01, Eelco Chaudron wrote:

Any feedback on this?


On 1 Oct 2019, at 11:55, Eelco Chaudron wrote:


Drivers natively supporting AF_XDP will check that a configured MTU size
will not exceed the allowed size for AF_XDP. However, when the skb
compatibility mode is used there is no check and any value is accepted.


This sounds like a kernel issue.
So, maybe it's better to fix it there?


This, for example, is the case when using the TAP interface.

This fix adds a check to make sure only AF_XDP valid values are excepted.

Signed-off-by: Eelco Chaudron 
---
 lib/netdev-afxdp.c |   17 +
 lib/netdev-afxdp.h |    1 +
 lib/netdev-linux.c |    9 +
 3 files changed, 27 insertions(+)

diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index 6e0180327..140150f29 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -1001,6 +1001,23 @@ netdev_afxdp_destruct(struct netdev *netdev)
 ovs_mutex_destroy(>mutex);
 }

+int
+netdev_afxdp_verify_mtu_size(const struct netdev *netdev, int mtu)
+{
+    /*
+ * If a device is used in xdpmode skb, no driver-specific MTU size is
+ * checked and any value is allowed resulting in packet drops.
+ * This check will verify the maximum supported value based on the
+ * buffer size allocated and the additional headroom required.
+ */
+    if (netdev == NULL || mtu <= 0


I don't really think that we need to check above things.
netdev can't be NULL here.  You may put an assert here if you worried.

mtu shuld be validated by db schema, and it can not be < 1.


+    || mtu > (FRAME_SIZE - OVS_XDP_HEADROOM - XDP_PACKET_HEADROOM)) {


mtu doesn't usually include ethernet header, vlans.  So, these should be
excluded too.  Not sure about FCS, if it passed to XDP program in generic
mode or it's stripped by the driver.


+    return EINVAL;
+    }
+
+    return 0;
+}
+
 int
 netdev_afxdp_get_custom_stats(const struct netdev *netdev,
   struct netdev_custom_stats *custom_stats)
diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
index e2f400b72..ee6939fce 100644
--- a/lib/netdev-afxdp.h
+++ b/lib/netdev-afxdp.h
@@ -39,6 +39,7 @@ int netdev_afxdp_rxq_construct(struct netdev_rxq *rxq_);
 void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_);
 int netdev_afxdp_construct(struct netdev *netdev_);
 void netdev_afxdp_destruct(struct netdev *netdev_);
+int netdev_afxdp_verify_mtu_size(const struct netdev *netdev, int mtu);

 int netdev_afxdp_rxq_recv(struct netdev_rxq *rxq_,
   struct dp_packet_batch *batch,
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index f48192373..89d0d9a9d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1593,6 +1593,15 @@ netdev_linux_set_mtu(struct netdev *netdev_, int mtu)
 goto exit;
 }

+#ifdef HAVE_AF_XDP
+    if (!strcmp(netdev_get_type(netdev_), "afxdp")) {


It's better to compare netdev class with _afxdp_calss
as it done for tap.


+    error = netdev_afxdp_verify_mtu_size(netdev_, mtu);
+    if (error) {
+    goto exit;
+    }
+    }
+#endif
+
 if (netdev->cache_valid & VALID_MTU) {
 error = netdev->netdev_mtu_error;
 if (error || netdev->mtu == mtu) {

___
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] can OVS conntrack support IP list like this: actions=ct(commit, table=0, zone=1, nat(dst=220.0.0.3, 220.0.0.7, 220.0.0.123))?

[Darrell Ball]

On Wed, Nov 6, 2019 at 5:01 PM Yi Yang (杨燚)-云服务集团 
wrote:

> Thanks Darrell, I didn’t receive your second reply, I saw it in
> mail.openvswitch.org.
>
>
>
> “
>
> probably, you should give an example of what you mean by above
>
> I am not sure you are meaning to say that you want to specify an L4 port in
>
> your
>
> snat action rule or not; you will want to use ephemeral ports by not
>
> specifying a
>
> specific port in most cases
>
> “
>
>
>
> For SNAT, we don’t specify port, just use default port range
> “1024-65535”), but for internal source IPs, i.e. floating IPs, they are
> discrete in most cases because some floating IPs needn’t access Internet,
> for public IPs, so are they. For public IPs, maybe they are from different
> telecom carriers, we prefer egress traffic can be distributed on several
> BGP lines.
>

>
>
> table=0,ip,nw_src=172.18.0.67,…,actions=ct(commit,table=0,zone=1,nat(src=220.0.0.3,230.0.0.7,240.0.0.123))
>
>
> table=0,ip,nw_src=172.18.0.80,…,actions=ct(commit,table=0,zone=1,nat(src=220.0.0.3,230.0.0.7,240.0.0.123))
>
>
> table=0,ip,nw_src=172.19.0.23,…,actions=ct(commit,table=0,zone=1,nat(src=220.0.0.3,230.0.0.7,240.0.0.123))
>
>
>
> Ideally, we hope, for different traffic types from the same internal IP
> (say 172.18.0.67), some can SNAT to 220.0.0.3, some can SNAT to 230.0.0.7,
> some can SNAT to
>
> 240.0.0.123, that way, they can leverage total bandwidth of several BGP
> lines.
>
>
>
> I know current OVS can’t support the above IP list for snat, but it is
> indeed required in reality, I don’t understand why OVS can’t do in this
> way, is it linux conntrack limitation or what else reason? I think it is
> similar to IP range which can be supported.
>

Presently, the limitation is both at the Openflow layer and implementation
details at datapath
The layer above (a controller or even a script) can do the mapping taking
into account the desired distribution
A controller can/will often do this and similar types of configuration
specification.


>
>
>
> *发件人:* Darrell Ball [mailto:dlu...@gmail.com]
> *发送时间:* 2019年11月6日 9:38
> *收件人:* Yi Yang (杨燚)-云服务集团 
> *抄送:* ovs-disc...@openvswitch.org; ovs-dev@openvswitch.org
> *主题:* Re: [ovs-dev] can OVS conntrack support IP list like this:
> actions=ct(commit, table=0, zone=1, nat(dst=220.0.0.3, 220.0.0.7,
> 220.0.0.123))?
>
>
>
>
>
>
>
> On Tue, Nov 5, 2019 at 4:32 PM Yi Yang (杨燚)-云服务集团 
> wrote:
>
> Hi, folks
>
>
>
> We need to do SNAT for many internal IPs by just using several public IPs,
> we also need to do DNAT by some other public IPs for exposing webservice,
> openflow rules look like the below:
>
>
>
> table=0,ip,nw_src=172.17.0.0/16,
> …,actions=ct(commit,table=0,zone=1,nat(src=
> 220.0.0.3,220.0.0.7,220.0.0.123))
>
> table=0,ip,nw_src=172.18.0.67,…,actions=ct(commit,table=0,zone=1,nat(src=22
> 0.0.0.3,220.0.0.7,220.0.0.123))
>
>
>
> for snat, you can map some subset of private IPs to a given public IP and
> so on
>
>
>
>
>
> table=0,ip,tcp,nw_dst=220.0.0.11,tp_dst=80,…,actions=ct(commit,table=0,zone
> =2,nat(dst=172.16.0.100:80))
>
> table=0,ip,tcp,nw_dst=220.0.0.11,
> tp_dst=443,…,actions=ct(commit,table=0,zone=2,nat(dst=172.16.0.100:443))
>
>
>
> you are mapping 'to' private IPs, so you have control over the range
>
>
>
>
>
>
>
>
> From ct document, it seems it can’t support IP list for nat, anybody knows
> how we can handle such cases in some kind feasible way?
>
>
>
> In addition, is it ok if multiple openflow rules use the same NAT IP:PORT
> combination? I’m not sure if it will result in some conflicts for SNAT,
> because all of them need to do dynamic source port mapping, per my test, it
> seems this isn’t a problem.
>
>
>
> IIUC, as long as tuples are unique, it should be fine
>
>
>
>
>
>
> Thank you all in advance and appreciate your help sincerely.
>
> ___
> 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] ovn-northd: Validate dnat_and_snat external_mac/logical_ip.

[Dumitru Ceara]

On Thu, Nov 7, 2019 at 3:44 PM Numan Siddique  wrote:
>
> On Thu, Nov 7, 2019 at 5:52 PM Dumitru Ceara  wrote:
> >
> > When dnat_and_snat NAT rules are configured, if the user tries to set
> > external_mac in the NAT rule record without setting logical_ip
> > ovn-northd crashes as there's no validation in place.
> >
> > Add checks for valid ethernet address in NAT.external_mac and for
> > non-null NAT.logical_ip where applicable.
> >
> > Reported-by: Daniel Alvarez Sanchez 
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1769709
> > Signed-off-by: Dumitru Ceara 
>
> Thanks Dumitru.
> The fix looks good to me. I applied this to master.
>
> Thanks
> Numan

Thanks Numan!

>
> > ---
> >  northd/ovn-northd.c | 14 +++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index c23c270..2f0f501 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6032,9 +6032,12 @@ add_distributed_nat_routes(struct hmap *lflows, 
> > const struct ovn_port *op)
> >  for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
> >  const struct nbrec_nat *nat = op->od->nbr->nat[i];
> >  bool found = false;
> > +struct eth_addr mac;
> >
> >  if (strcmp(nat->type, "dnat_and_snat") ||
> > -!nat->external_mac  || !nat->external_ip) {
> > +!nat->external_mac ||
> > +!eth_addr_from_string(nat->external_mac, ) ||
> > +!nat->external_ip || !nat->logical_port) {
> >  continue;
> >  }
> >
> > @@ -6083,10 +6086,14 @@ add_distributed_nat_routes(struct hmap *lflows, 
> > const struct ovn_port *op)
> >
> >  for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
> >  const struct nbrec_nat *nat2 = op->od->nbr->nat[j];
> > +struct eth_addr mac2;
> >
> >  if (nat == nat2 || strcmp(nat2->type, "dnat_and_snat") ||
> > -!nat2->external_mac || !nat2->external_ip)
> > +!nat2->external_mac ||
> > +!eth_addr_from_string(nat2->external_mac, ) ||
> > +!nat2->external_ip) {
> >  continue;
> > +}
> >
> >  family = AF_INET;
> >  if (!ip_parse(nat2->external_ip, ) || !ip) {
> > @@ -7785,7 +7792,8 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> > hmap *ports,
> >  if (od->l3dgw_port) {
> >  /* Distributed router. */
> >  if (!strcmp(nat->type, "dnat_and_snat") &&
> > -nat->external_mac && nat->external_ip) {
> > +nat->external_mac && nat->external_ip &&
> > +eth_addr_from_string(nat->external_mac, )) {
> >  for (int j = 0; j < od->nbr->n_nat; j++) {
> >  const struct nbrec_nat *nat2 = od->nbr->nat[j];
> >
> > --
> > 1.8.3.1
> >
> > ___
> > 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 v5] ovn-northd: Limit ARP/ND broadcast domain whenever possible.

[Dumitru Ceara]

On Thu, Nov 7, 2019 at 3:51 AM Han Zhou  wrote:
>
>
>
> On Mon, Nov 4, 2019 at 11:32 AM Numan Siddique  wrote:
> >
> > On Mon, Nov 4, 2019 at 8:37 PM Dumitru Ceara  wrote:
> > >
> > > ARP request and ND NS packets for router owned IPs were being
> > > flooded in the complete L2 domain (using the MC_FLOOD multicast group).
> > > However this creates a scaling issue in scenarios where aggregation
> > > logical switches are connected to more logical routers (~350). The
> > > logical pipelines of all routers would have to be executed before the
> > > packet is finally replied to by a single router, the owner of the IP
> > > address.
> > >
> > > This commit limits the broadcast domain by bypassing the L2 Lookup stage
> > > for ARP requests that will be replied by a single router. The packets
> > > are still flooded in the L2 domain but not on any of the other patch
> > > ports towards other routers connected to the switch. This restricted
> > > flooding is done by using a new multicast group (MC_ARP_ND_FLOOD).
> > >
> > > IPs that are owned by the routers and for which this fix applies are:
> > > - IP addresses configured on the router ports.
> > > - VIPs.
> > > - NAT IPs.
> > >
> > > This commit also fixes function get_router_load_balancer_ips() which
> > > was incorrectly returning a single address_family even though the
> > > IP set could contain a mix of IPv4 and IPv6 addresses.
> > >
> > > Reported-at: https://bugzilla.redhat.com/1756945
> > > Reported-by: Anil Venkata 
> > > Signed-off-by: Dumitru Ceara 
> >
> > Thanks Dumitru, for addressing the review comments.
> >
> > Acked-by: Numan Siddique 
> >
> > Han, if you can take a look in this patch and provide your comments,
> > that would be great.
> >

Hi Han,

>
> Sorry for delayed response :(

No worries and thanks for reviewing this change.

>
> The patch looks good to me except:
>
> 1. It changes the behavior that when an ARP request for a LRP's IP is coming 
> to the logical switch, other routers will no longer learn the MAC-IP binding 
> of the ARP sender. This has been discussed and I tend to agree with Dumitru 
> that it should not cause real issue. I think it just worth to be documented 
> clearly in the ovn-architecture, probably in the section: Logical Routers and 
> Logical Patch Ports, because this is something different from a traditional 
> switch's behavior, and network administrators may get suprised.

Ack, I'll add this in v6.

>
> 2. Since we are anyway changing the behavior of ARP request handling and 
> bypassing logical router ports, and we think it should not cause real 
> problem, then I wonder why adding the MC_ARP_ND group to still flood to the 
> non-router ports. Is it really useful? Maybe it is not a big deal, but I 
> think the extra MC group would add some performance cost and it is almost 
> redundant with the regular MC_FLOOD except that there is no router ports in 
> it - it is a lot of redundancy considering that most LSPs are regular VIFs in 
> typical large scale environments. I would suggest to simplify this unless 
> there is clear concerns of not flooding to other ports.

If I skip flooding to non-router ports the following test fails
because one of the things checked by the test is that we flood
broadcasts (including ARPs) in the broadcast domain:
 36: ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR   FAILED (ovs-macros.at:220)

So my concern was that people might expect the ARP requests to be
flooded within the L2 broadcast domain.
However, we could add configuration knob to change the behavior and
only flood them on localnet ports. Like this we could maintain the
current L2 flooding but in deployments where this is not necessary the
users could disable the flooding to VIF ports and this would remove
the MC_ARP_ND group unless there's a localnet port in the datapath.

What do you think?

>
> 3. Not a big thing, but the name of the functions 
> build_lswitch_rport_arp_responders() and build_lswitch_rport_arp_flow() are a 
> little bit confusing. It is not for arp-responder, but for bypassing router 
> ports. Would it be better to be something like: 
> build_lswitch_rport_arp_req_flows() and 
> build_lswitch_rport_arp_request_flow_for_ip()?

Sure, I'll change them.
>
> In addition, not for the patch, but for the problem itself, I think we'd 
> better quantify the number of router ports we support with current resubmit 
> limit (as suggested by Ben), and document somewhere the limit and the impact 
> if user exceeds the limit. If the limit is easily exceed in very common 
> deployments, we probably should consider increasing the resubmit limit.

Agreed, that should be done too.

Thanks,
Dumitru

>
> Thanks,
> Han

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

[ovs-dev] Loan Available.

[Harry Smith]

Dear Sir,
 
We work with a reliable and efficient company in England that specialized on 
bank loan at a very reasonable rate of 4% per-annul which are far much better 
 than securing financial instrument such as Bank Guarantee (BG) or Standby 
Letter of Credit (SBLC).  The whole process from signing of contract and the 
 application process and approval of loan can be completed as early as ten (10) 
banking days or less.
 Contact me for further directives / information.
 Sincerely.
 Mr. Harry Smith
 Email: harrysmith1...@yahoo.com

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

Re: [ovs-dev] [PATCH ovn] ovn-northd: Validate dnat_and_snat external_mac/logical_ip.

[Numan Siddique]

On Thu, Nov 7, 2019 at 5:52 PM Dumitru Ceara  wrote:
>
> When dnat_and_snat NAT rules are configured, if the user tries to set
> external_mac in the NAT rule record without setting logical_ip
> ovn-northd crashes as there's no validation in place.
>
> Add checks for valid ethernet address in NAT.external_mac and for
> non-null NAT.logical_ip where applicable.
>
> Reported-by: Daniel Alvarez Sanchez 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1769709
> Signed-off-by: Dumitru Ceara 

Thanks Dumitru.
The fix looks good to me. I applied this to master.

Thanks
Numan

> ---
>  northd/ovn-northd.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index c23c270..2f0f501 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6032,9 +6032,12 @@ add_distributed_nat_routes(struct hmap *lflows, const 
> struct ovn_port *op)
>  for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
>  const struct nbrec_nat *nat = op->od->nbr->nat[i];
>  bool found = false;
> +struct eth_addr mac;
>
>  if (strcmp(nat->type, "dnat_and_snat") ||
> -!nat->external_mac  || !nat->external_ip) {
> +!nat->external_mac ||
> +!eth_addr_from_string(nat->external_mac, ) ||
> +!nat->external_ip || !nat->logical_port) {
>  continue;
>  }
>
> @@ -6083,10 +6086,14 @@ add_distributed_nat_routes(struct hmap *lflows, const 
> struct ovn_port *op)
>
>  for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
>  const struct nbrec_nat *nat2 = op->od->nbr->nat[j];
> +struct eth_addr mac2;
>
>  if (nat == nat2 || strcmp(nat2->type, "dnat_and_snat") ||
> -!nat2->external_mac || !nat2->external_ip)
> +!nat2->external_mac ||
> +!eth_addr_from_string(nat2->external_mac, ) ||
> +!nat2->external_ip) {
>  continue;
> +}
>
>  family = AF_INET;
>  if (!ip_parse(nat2->external_ip, ) || !ip) {
> @@ -7785,7 +7792,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
> *ports,
>  if (od->l3dgw_port) {
>  /* Distributed router. */
>  if (!strcmp(nat->type, "dnat_and_snat") &&
> -nat->external_mac && nat->external_ip) {
> +nat->external_mac && nat->external_ip &&
> +eth_addr_from_string(nat->external_mac, )) {
>  for (int j = 0; j < od->nbr->n_nat; j++) {
>  const struct nbrec_nat *nat2 = od->nbr->nat[j];
>
> --
> 1.8.3.1
>
> ___
> 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 ] travis: support ppc64le builds

[Yanqin Wei (Arm Technology China)]

Hi David, 

I am sorry to reply this late. Yes, if travis support this configuration, it 
should be a good solution for me.

Best regards,
Wei Yanqin

-Original Message-
From: dwilder  
Sent: Wednesday, November 6, 2019 4:32 AM
To: Yanqin Wei (Arm Technology China) 
Cc: Ilya Maximets ; ovs-dev@openvswitch.org; 
wil...@us.ibm.com; nd 
Subject: Re: RE: RE: [ovs-dev] [PATCH ] travis: support ppc64le builds

Hi Wei

If I change my matrix:include to use "arch: ppc64le" rather than "os: 
linux-ppc64le", will it eliminate your concern?

matrix:
   include:
-   - os: linux-ppc64le
+   - arch: ppc64le
   compiler: gcc
   env: OPTS="--disable-ssl"


Later when we want to enable the full matrix on all arch we would add:

1) arch:
   - amd64
   - ppc64le
   - arm64

2) eliminate the include: -arch: ppc64le

3) Add any exclude: - arch:XXX sections for any tests that dont apply.

For example I would add:

  exclude:
- arch: ppc64le
  env: M32=1 OPTS="--disable-ssl"

Regards,
   David

On 2019-11-02 02:09, Yanqin Wei (Arm Technology China) wrote:
> Hi David,
> 
> Thanks for your reply.  Yes, my concern is how to define arch and os 
> in .travis.yml after we cover all builds and cases for arm and ppc.
> This pattern can enable all builds and testsuits for x86 and arm
> arch:
>   - amd64
>   - arm64
> os:
>   - linux
> 
> This can enable all jobs for x86 and ppc.
> arch:
>   - amd64 //default
> os:
>   - linux
>   - linux-ppc64le
> 
> But it does not work to combine them.  This means four kinds of
> arch+os combinations in all.   Arm64+linux-ppc64le is invalid.
> arch:
>   - amd64
>   - arm64
> os:
>   - linux
>   - linux-ppc64le
> 
> So if we finally cover all the builds and cases for arm/ppc,  we have 
> to duplicate all jobs for different cpu arch in the matrix include.
> matrix:
>   include:
> - os: linux-ppc64le
>   env: job1
> - os: linux-ppc64le
>   env: job2
> ...
> - arch: arm64
>   env: job1
> - arch: arm64
>   env: job2
> ...
> 
> But currently either arm or ppc has not cover all the cases, so they 
> can coexist in build-matrix.  And there is no conflict in 
> build/prepare scripts, because both of them use TRAVIS_ARCH variable 
> to indicate cpu arch.
> The patch series to enable arm CI is under internal review. It will be 
> submitted when ready.
> 
> Best Regards,
> Wei Yanqin
> 
> 
> -Original Message-
> From: dwilder 
> Sent: Saturday, November 2, 2019 1:09 AM
> To: Yanqin Wei (Arm Technology China) 
> Cc: Ilya Maximets ; ovs-dev@openvswitch.org; 
> wil...@us.ibm.com; nd 
> Subject: Re: RE: [ovs-dev] [PATCH ] travis: support ppc64le builds
> 
> On 2019-10-30 19:04, Yanqin Wei (Arm Technology China) wrote:
>> Hi,
>> 
>> We are working to support arm64 build for ovs travis CI. It is indeed 
>> to use arch: arm64 to choose cpu architecture, because travis has 
>> provided native arm64 option now.
>> But in this patch it seems ppc64 builds run on the ppc-VM + x86 
>> native machine.
>> Currently arm only select a part of jobs to run, which is defined in 
>> matrix:include. But the final object is to run all jobs. It means 
>> that
>>  arch: arm64 will be moved out of marix. If ppc plans to do the same 
>> in the future, it will conflict with arm jobs.
>> 
>> Best Regards,
>> Wei Yanqin
> 
> Hi,
> I have added a build only test for ppc64le following the model used 
> for osx. I think this is a good start for getting multi-arch support 
> into Ci.
> 
> I agree that running all jobs on the matrix on every arch is good goal.
> I dont completely understand your issue, is your concern the use of os:
> vs arch: ?
> 
> I am glad to work with you to find a solution. Can you share your
> arm64 changes?  We can discuss off-list if you prefer.
> 
> 
>> 
>> -Original Message-
>> From: ovs-dev-boun...@openvswitch.org 
>>  On Behalf Of dwilder
>> Sent: Wednesday, October 30, 2019 1:55 AM
>> To: Ilya Maximets 
>> Cc: ovs-dev@openvswitch.org; wil...@us.ibm.com
>> Subject: Re: [ovs-dev] [PATCH ] travis: support ppc64le builds
>> 
>> On 2019-10-29 09:52, Ilya Maximets wrote:
>>> On 28.10.2019 22:22, David Wilder wrote:
 Add support for travis-ci ppc64le builds.
 
 - Updated matrix in .travis.yml to include a ppc64le build.
 - Added support to install packages needed by specific 
 architectures.
 
 To keep the total build time at an acceptable level only a single 
 build job is included in the matrix for ppc64le.
 
 A build report example can be found here [1] [0] 
 https://urldefense.proofpoint.com/v2/url?u=http-3A__travis-2Dci.org
 _& 
 d=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=7ndxyKjH9UrBD68Us2WP1wI4BwEBQbe
 Ay 
 z8i_vwCCaI=6JANehIfGoxUMtwHhe4yob4UPeby0Y8ovgzRDIyJZFo=UMYL8rzJ
 Np h87seC0oJLBiWoe-sUSL80AJy0RMTgBzQ=
 [1]
 https://urldefense.proofpoint.com/v2/url?u=https-3A__travis-2Dci.or
 g_ 
 djlwilder_ovs_builds_604098141=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=
 7n 
 

Re: [ovs-dev] [PATCH] netdev-afxdp: add afxdp specific maximum MTU check

[Eelco Chaudron]

Any feedback on this?


On 1 Oct 2019, at 11:55, Eelco Chaudron wrote:

Drivers natively supporting AF_XDP will check that a configured MTU 
size

will not exceed the allowed size for AF_XDP. However, when the skb
compatibility mode is used there is no check and any value is 
accepted.

This, for example, is the case when using the TAP interface.

This fix adds a check to make sure only AF_XDP valid values are 
excepted.


Signed-off-by: Eelco Chaudron 
---
 lib/netdev-afxdp.c |   17 +
 lib/netdev-afxdp.h |1 +
 lib/netdev-linux.c |9 +
 3 files changed, 27 insertions(+)

diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index 6e0180327..140150f29 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -1001,6 +1001,23 @@ netdev_afxdp_destruct(struct netdev *netdev)
 ovs_mutex_destroy(>mutex);
 }

+int
+netdev_afxdp_verify_mtu_size(const struct netdev *netdev, int mtu)
+{
+/*
+ * If a device is used in xdpmode skb, no driver-specific MTU 
size is

+ * checked and any value is allowed resulting in packet drops.
+ * This check will verify the maximum supported value based on 
the

+ * buffer size allocated and the additional headroom required.
+ */
+if (netdev == NULL || mtu <= 0
+|| mtu > (FRAME_SIZE - OVS_XDP_HEADROOM - 
XDP_PACKET_HEADROOM)) {

+return EINVAL;
+}
+
+return 0;
+}
+
 int
 netdev_afxdp_get_custom_stats(const struct netdev *netdev,
   struct netdev_custom_stats 
*custom_stats)

diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
index e2f400b72..ee6939fce 100644
--- a/lib/netdev-afxdp.h
+++ b/lib/netdev-afxdp.h
@@ -39,6 +39,7 @@ int netdev_afxdp_rxq_construct(struct netdev_rxq 
*rxq_);

 void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_);
 int netdev_afxdp_construct(struct netdev *netdev_);
 void netdev_afxdp_destruct(struct netdev *netdev_);
+int netdev_afxdp_verify_mtu_size(const struct netdev *netdev, int 
mtu);


 int netdev_afxdp_rxq_recv(struct netdev_rxq *rxq_,
   struct dp_packet_batch *batch,
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index f48192373..89d0d9a9d 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1593,6 +1593,15 @@ netdev_linux_set_mtu(struct netdev *netdev_, 
int mtu)

 goto exit;
 }

+#ifdef HAVE_AF_XDP
+if (!strcmp(netdev_get_type(netdev_), "afxdp")) {
+error = netdev_afxdp_verify_mtu_size(netdev_, mtu);
+if (error) {
+goto exit;
+}
+}
+#endif
+
 if (netdev->cache_valid & VALID_MTU) {
 error = netdev->netdev_mtu_error;
 if (error || netdev->mtu == mtu) {

___
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 v2] Fix ha chassis failover issues for stale ha chassis entries

[Numan Siddique]

On Thu, Nov 7, 2019 at 5:57 PM Dumitru Ceara  wrote:
>
> On Thu, Nov 7, 2019 at 10:37 AM  wrote:
> >
> > From: Numan Siddique 
> >
> > If ha chassis rows of an HA chassis group become stale i.e the 
> > HA_Chassis.chassis
> > column is empty (because ovn-controller is not running in that chassis)
> > except one row and when ha_chassis_group_is_active()
> > is called on that ovn-controller, then it returns false. Ideally it should
> > become active since its the only active chassis. This patch fixes this 
> > issue.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1762777
> > Reported-by: Daniel Alvarez 
> >
> > Signed-off-by: Numan Siddique 
>
> Hi Numan,
>
> Looks good to me.
>
> Thanks,
> Dumitru
>
> Acked-by: Dumitru Ceara 

Thanks. I applied this to master.


Numan

>
> > ---
> >
> > v1 -> v2
> > --
> >   * Addresses Dumitru's comments.
> >
> >  controller/ha-chassis.c | 25 +
> >  tests/ovn.at| 20 +++-
> >  2 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/ha-chassis.c b/controller/ha-chassis.c
> > index 6d9426a5c..d6ec7b658 100644
> > --- a/controller/ha-chassis.c
> > +++ b/controller/ha-chassis.c
> > @@ -142,6 +142,27 @@ ha_chassis_destroy_ordered(struct ha_chassis_ordered 
> > *ordered_ha_ch)
> >  }
> >  }
> >
> > +/* Returns true if there is only one active ha chassis in the chassis group
> > + * (i.e HA_Chassis.chassis column is set) and that active ha chassis is
> > + * local chassis.
> > + * Returns false otherwise. */
> > +static bool
> > +is_local_chassis_only_candidate(const struct sbrec_ha_chassis_group 
> > *ha_ch_grp,
> > +const struct sbrec_chassis *local_chassis)
> > +{
> > +size_t n_active_ha_chassis = 0;
> > +bool local_chassis_present = false;
> > +for (size_t i = 0; i < ha_ch_grp->n_ha_chassis; i++) {
> > +if (ha_ch_grp->ha_chassis[i]->chassis) {
> > +n_active_ha_chassis++;
> > +if (ha_ch_grp->ha_chassis[i]->chassis == local_chassis) {
> > +local_chassis_present = true;
> > +}
> > +}
> > +}
> > +
> > +return (local_chassis_present && n_active_ha_chassis == 1);
> > +}
> >
> >  /* Returns true if the local_chassis is the master of
> >   * the HA chassis group, false otherwise. */
> > @@ -159,6 +180,10 @@ ha_chassis_group_is_active(
> >  return (ha_ch_grp->ha_chassis[0]->chassis == local_chassis);
> >  }
> >
> > +if (is_local_chassis_only_candidate(ha_ch_grp, local_chassis)) {
> > +return true;
> > +}
> > +
> >  if (sset_is_empty(active_tunnels)) {
> >  /* If active tunnel sset is empty, it means it has lost
> >   * connectivity with other chassis. */
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 410f4b514..cb7903db8 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -13413,7 +13413,25 @@ OVS_WAIT_UNTIL(
> >  logical_port=ls1-lp_ext1`
> >  test "$chassis" = "$hv1_uuid"])
> >
> > -OVN_CLEANUP([hv1],[hv2],[hv3])
> > +# Stop ovn-controllers on hv1 and hv3.
> > +as hv1 ovn-appctl -t ovn-controller exit
> > +as hv3 ovn-appctl -t ovn-controller exit
> > +
> > +# hv2 should be master and claim ls1-lp_ext1
> > +OVS_WAIT_UNTIL(
> > +[chassis=`ovn-sbctl --bare --columns chassis find port_binding \
> > +logical_port=ls1-lp_ext1`
> > +test "$chassis" = "$hv2_uuid"])
> > +
> > +as hv1
> > +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +
> > +as hv3
> > +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +
> > +OVN_CLEANUP([hv2])
> >  AT_CLEANUP
> >
> >  AT_SETUP([ovn -- Address Set Incremental Processing])
> > --
> > 2.23.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
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] netdev-afxdp: Best-effort configuration of XDP mode.

[Eelco Chaudron]

On 7 Nov 2019, at 14:21, Ilya Maximets wrote:


On 07.11.2019 13:39, Eelco Chaudron wrote:



On 7 Nov 2019, at 12:36, Ilya Maximets wrote:

Until now there was only two options for XDP mode in OVS: SKB or 
DRV.

i.e. 'generic XDP' or 'native XDP with zero-copy enabled'.

Devices like 'veth' interfaces in Linux supports native XDP, but
doesn't support zero-copy mode.  This case can not be covered by
existing API and we have to use slower generic XDP for such devices.
There are few more issues, e.g. TCP is not supported in generic XDP
mode for veth interfaces due to kernel limitations, however it is
supported in native mode.

This change introduces ability to use native XDP without zero-copy
along with best-effort configuration option that enabled by default.
In best-effort case OVS will sequentially try different modes 
starting
from the fastest one and will choose the first acceptable for 
current

interface.  This will guarantee the best possible performance.

If user will want to choose specific mode, it's still possible by
setting the 'options:xdp-mode'.

This change additionally changes the API by renaming the 
configuration

knob from 'xdpmode' to 'xdp-mode' and also renaming the modes
themselves to be more user-friendly.

The full list of currently supported modes:
  * native-with-zerocopy - former DRV
  * native   - new one, DRV without 
zero-copy

  * generic  - former SKB
  * best-effort  - new one, chooses the best 
available from

   3 above modes

Since 'best-effort' is a default mode, users will not need to
explicitely set 'xdp-mode' in most cases.

TCP related tests enabled back in system afxdp testsuite, because
'best-effort' will choose 'native' mode for veth interfaces
and this mode has no issues with TCP.

Signed-off-by: Ilya Maximets 


No review, I was running some performance tests for the OVS 
conference presentation so quickly tried this patch (assuming 
performance would increase :)…


So with the native option (default for tap) I see a decrease, :(,  
in performance over the skb mode (this is with my usual ovs_perf PVP 
test setup).


With the patch applied, and default configuration 
(xdp-mode-in-use=native-with-zerocopy for my tap):


Hmm. tap supports zero-copy? Interesting.
Have you tried forcing 'native' mode without zero-copy?
Maybe it will make sense to enable/disable need-wakeup feature.


Oops, wrong cut/paste: xdp-mode-in-use=native

All my the tests where with use-need-wakeup=true

  port 2: tapVM (afxdp: n_rxq=1, use-need-wakeup=true, 
xdp-mode=best-effort, xdp-mode-in-use=native)






"Physical to Virtual to Physical test, L3 flows[port redirect]"
, Packet size
Number of flows,  64
    1, 711581
  100, 673152
1000, 617733

Here you will even see a performance drop with multiple IP flows 
(this is a single queue config).


This is strange..



With SKB mode (xdp-mode-in-use=generic):


I'm curious, does TCP work via tap interfaces in generic mode?



"Physical to Virtual to Physical test, L3 flows[port redirect]"
, Packet size
Number of flows,  64
    1, 800796
  100, 800912
1000, 800133

Cheers,

Eelco



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

Re: [ovs-dev] [PATCH v2] netdev-afxdp: Best-effort configuration of XDP mode.

[Ilya Maximets]

On 07.11.2019 13:39, Eelco Chaudron wrote:



On 7 Nov 2019, at 12:36, Ilya Maximets wrote:


Until now there was only two options for XDP mode in OVS: SKB or DRV.
i.e. 'generic XDP' or 'native XDP with zero-copy enabled'.

Devices like 'veth' interfaces in Linux supports native XDP, but
doesn't support zero-copy mode.  This case can not be covered by
existing API and we have to use slower generic XDP for such devices.
There are few more issues, e.g. TCP is not supported in generic XDP
mode for veth interfaces due to kernel limitations, however it is
supported in native mode.

This change introduces ability to use native XDP without zero-copy
along with best-effort configuration option that enabled by default.
In best-effort case OVS will sequentially try different modes starting
from the fastest one and will choose the first acceptable for current
interface.  This will guarantee the best possible performance.

If user will want to choose specific mode, it's still possible by
setting the 'options:xdp-mode'.

This change additionally changes the API by renaming the configuration
knob from 'xdpmode' to 'xdp-mode' and also renaming the modes
themselves to be more user-friendly.

The full list of currently supported modes:
  * native-with-zerocopy - former DRV
  * native   - new one, DRV without zero-copy
  * generic  - former SKB
  * best-effort  - new one, chooses the best available from
   3 above modes

Since 'best-effort' is a default mode, users will not need to
explicitely set 'xdp-mode' in most cases.

TCP related tests enabled back in system afxdp testsuite, because
'best-effort' will choose 'native' mode for veth interfaces
and this mode has no issues with TCP.

Signed-off-by: Ilya Maximets 


No review, I was running some performance tests for the OVS conference 
presentation so quickly tried this patch (assuming performance would increase 
:)…

So with the native option (default for tap) I see a decrease, :(,  in 
performance over the skb mode (this is with my usual ovs_perf PVP test setup).

With the patch applied, and default configuration 
(xdp-mode-in-use=native-with-zerocopy for my tap):


Hmm. tap supports zero-copy? Interesting.
Have you tried forcing 'native' mode without zero-copy?
Maybe it will make sense to enable/disable need-wakeup feature.



"Physical to Virtual to Physical test, L3 flows[port redirect]"
, Packet size
Number of flows,  64
    1, 711581
  100, 673152
1000, 617733

Here you will even see a performance drop with multiple IP flows (this is a 
single queue config).


This is strange..



With SKB mode (xdp-mode-in-use=generic):


I'm curious, does TCP work via tap interfaces in generic mode?



"Physical to Virtual to Physical test, L3 flows[port redirect]"
, Packet size
Number of flows,  64
    1, 800796
  100, 800912
1000, 800133

Cheers,

Eelco


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

Re: [ovs-dev] [PATCH v2] netdev-afxdp: Best-effort configuration of XDP mode.

[Eelco Chaudron]

On 7 Nov 2019, at 12:36, Ilya Maximets wrote:


Until now there was only two options for XDP mode in OVS: SKB or DRV.
i.e. 'generic XDP' or 'native XDP with zero-copy enabled'.

Devices like 'veth' interfaces in Linux supports native XDP, but
doesn't support zero-copy mode.  This case can not be covered by
existing API and we have to use slower generic XDP for such devices.
There are few more issues, e.g. TCP is not supported in generic XDP
mode for veth interfaces due to kernel limitations, however it is
supported in native mode.

This change introduces ability to use native XDP without zero-copy
along with best-effort configuration option that enabled by default.
In best-effort case OVS will sequentially try different modes starting
from the fastest one and will choose the first acceptable for current
interface.  This will guarantee the best possible performance.

If user will want to choose specific mode, it's still possible by
setting the 'options:xdp-mode'.

This change additionally changes the API by renaming the configuration
knob from 'xdpmode' to 'xdp-mode' and also renaming the modes
themselves to be more user-friendly.

The full list of currently supported modes:
  * native-with-zerocopy - former DRV
  * native   - new one, DRV without zero-copy
  * generic  - former SKB
  * best-effort  - new one, chooses the best available from
   3 above modes

Since 'best-effort' is a default mode, users will not need to
explicitely set 'xdp-mode' in most cases.

TCP related tests enabled back in system afxdp testsuite, because
'best-effort' will choose 'native' mode for veth interfaces
and this mode has no issues with TCP.

Signed-off-by: Ilya Maximets 


No review, I was running some performance tests for the OVS conference 
presentation so quickly tried this patch (assuming performance would 
increase :)…


So with the native option (default for tap) I see a decrease, :(,  in 
performance over the skb mode (this is with my usual ovs_perf PVP test 
setup).


With the patch applied, and default configuration 
(xdp-mode-in-use=native-with-zerocopy for my tap):


"Physical to Virtual to Physical test, L3 flows[port redirect]"
, Packet size
Number of flows,  64
   1, 711581
 100, 673152
1000, 617733

Here you will even see a performance drop with multiple IP flows (this 
is a single queue config).


With SKB mode (xdp-mode-in-use=generic):

"Physical to Virtual to Physical test, L3 flows[port redirect]"
, Packet size
Number of flows,  64
   1, 800796
 100, 800912
1000, 800133

Cheers,

Eelco

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

Re: [ovs-dev] [PATCH ovn v2] Fix ha chassis failover issues for stale ha chassis entries

[Dumitru Ceara]

On Thu, Nov 7, 2019 at 10:37 AM  wrote:
>
> From: Numan Siddique 
>
> If ha chassis rows of an HA chassis group become stale i.e the 
> HA_Chassis.chassis
> column is empty (because ovn-controller is not running in that chassis)
> except one row and when ha_chassis_group_is_active()
> is called on that ovn-controller, then it returns false. Ideally it should
> become active since its the only active chassis. This patch fixes this issue.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1762777
> Reported-by: Daniel Alvarez 
>
> Signed-off-by: Numan Siddique 

Hi Numan,

Looks good to me.

Thanks,
Dumitru

Acked-by: Dumitru Ceara 

> ---
>
> v1 -> v2
> --
>   * Addresses Dumitru's comments.
>
>  controller/ha-chassis.c | 25 +
>  tests/ovn.at| 20 +++-
>  2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/controller/ha-chassis.c b/controller/ha-chassis.c
> index 6d9426a5c..d6ec7b658 100644
> --- a/controller/ha-chassis.c
> +++ b/controller/ha-chassis.c
> @@ -142,6 +142,27 @@ ha_chassis_destroy_ordered(struct ha_chassis_ordered 
> *ordered_ha_ch)
>  }
>  }
>
> +/* Returns true if there is only one active ha chassis in the chassis group
> + * (i.e HA_Chassis.chassis column is set) and that active ha chassis is
> + * local chassis.
> + * Returns false otherwise. */
> +static bool
> +is_local_chassis_only_candidate(const struct sbrec_ha_chassis_group 
> *ha_ch_grp,
> +const struct sbrec_chassis *local_chassis)
> +{
> +size_t n_active_ha_chassis = 0;
> +bool local_chassis_present = false;
> +for (size_t i = 0; i < ha_ch_grp->n_ha_chassis; i++) {
> +if (ha_ch_grp->ha_chassis[i]->chassis) {
> +n_active_ha_chassis++;
> +if (ha_ch_grp->ha_chassis[i]->chassis == local_chassis) {
> +local_chassis_present = true;
> +}
> +}
> +}
> +
> +return (local_chassis_present && n_active_ha_chassis == 1);
> +}
>
>  /* Returns true if the local_chassis is the master of
>   * the HA chassis group, false otherwise. */
> @@ -159,6 +180,10 @@ ha_chassis_group_is_active(
>  return (ha_ch_grp->ha_chassis[0]->chassis == local_chassis);
>  }
>
> +if (is_local_chassis_only_candidate(ha_ch_grp, local_chassis)) {
> +return true;
> +}
> +
>  if (sset_is_empty(active_tunnels)) {
>  /* If active tunnel sset is empty, it means it has lost
>   * connectivity with other chassis. */
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 410f4b514..cb7903db8 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -13413,7 +13413,25 @@ OVS_WAIT_UNTIL(
>  logical_port=ls1-lp_ext1`
>  test "$chassis" = "$hv1_uuid"])
>
> -OVN_CLEANUP([hv1],[hv2],[hv3])
> +# Stop ovn-controllers on hv1 and hv3.
> +as hv1 ovn-appctl -t ovn-controller exit
> +as hv3 ovn-appctl -t ovn-controller exit
> +
> +# hv2 should be master and claim ls1-lp_ext1
> +OVS_WAIT_UNTIL(
> +[chassis=`ovn-sbctl --bare --columns chassis find port_binding \
> +logical_port=ls1-lp_ext1`
> +test "$chassis" = "$hv2_uuid"])
> +
> +as hv1
> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as hv3
> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +OVN_CLEANUP([hv2])
>  AT_CLEANUP
>
>  AT_SETUP([ovn -- Address Set Incremental Processing])
> --
> 2.23.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] [ovs-discuss] gso packet is failing with af_packet socket with packet_vnet_hdr

[Flavio Leitner]

On Wed, 6 Nov 2019 01:59:41 +0530
Ramana Reddy  wrote:

> Hi Flavio,
> As per your inputs, I modified the gso_size, and now
> skb_gso_validate_mtu(skb, mtu) is returning true, and
> ip_finish_output2(sk, skb)  and dst_neigh_output(dst, neigh, skb); are
> getting called. But still, I am seeing the large packets getting
> dropped somewhere in the kernel
> down the line and retransmission happening.

The gso_size is the size of the data payload, so it doesn't account the
headers. Usually this depends on the iface MTU like I pointed before and
that MTU should account for the encapsulation later on. For example:

veth0(1450)veth1(1450) -- VXLAN(64k) --- eth0(1500)

Perhaps you can use ``dropwatch´´ to find out where the packet is
dropped.

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

[ovs-dev] [PATCH ovn] ovn-northd: Validate dnat_and_snat external_mac/logical_ip.

[Dumitru Ceara]

When dnat_and_snat NAT rules are configured, if the user tries to set
external_mac in the NAT rule record without setting logical_ip
ovn-northd crashes as there's no validation in place.

Add checks for valid ethernet address in NAT.external_mac and for
non-null NAT.logical_ip where applicable.

Reported-by: Daniel Alvarez Sanchez 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1769709
Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c23c270..2f0f501 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6032,9 +6032,12 @@ add_distributed_nat_routes(struct hmap *lflows, const 
struct ovn_port *op)
 for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
 const struct nbrec_nat *nat = op->od->nbr->nat[i];
 bool found = false;
+struct eth_addr mac;
 
 if (strcmp(nat->type, "dnat_and_snat") ||
-!nat->external_mac  || !nat->external_ip) {
+!nat->external_mac ||
+!eth_addr_from_string(nat->external_mac, ) ||
+!nat->external_ip || !nat->logical_port) {
 continue;
 }
 
@@ -6083,10 +6086,14 @@ add_distributed_nat_routes(struct hmap *lflows, const 
struct ovn_port *op)
 
 for (size_t j = 0; j < op->od->nbr->n_nat; j++) {
 const struct nbrec_nat *nat2 = op->od->nbr->nat[j];
+struct eth_addr mac2;
 
 if (nat == nat2 || strcmp(nat2->type, "dnat_and_snat") ||
-!nat2->external_mac || !nat2->external_ip)
+!nat2->external_mac ||
+!eth_addr_from_string(nat2->external_mac, ) ||
+!nat2->external_ip) {
 continue;
+}
 
 family = AF_INET;
 if (!ip_parse(nat2->external_ip, ) || !ip) {
@@ -7785,7 +7792,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 if (od->l3dgw_port) {
 /* Distributed router. */
 if (!strcmp(nat->type, "dnat_and_snat") &&
-nat->external_mac && nat->external_ip) {
+nat->external_mac && nat->external_ip &&
+eth_addr_from_string(nat->external_mac, )) {
 for (int j = 0; j < od->nbr->n_nat; j++) {
 const struct nbrec_nat *nat2 = od->nbr->nat[j];
 
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH v5] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event

[Ilya Maximets]

On 06.11.2019 14:43, Eelco Chaudron wrote:

Currently, OVS does not register and therefore not handle the
interface reset event from the DPDK framework. This would cause a
problem in cases where a VF is used as an interface, and its
configuration changes.

As an example in the following scenario the MAC change is not
detected/acted upon until OVS is restarted without the patch applied:

   $ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
   $ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
 set Interface dpdk0 type=dpdk -- \
 set Interface dpdk0 options:dpdk-devargs=:05:0a.0

   $ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33

Requires patch "bridge: Allow manual notifications about interfaces' updates."

Signed-off-by: Eelco Chaudron 
---
v4 -> v5:
   Using new if_notifier_manual_report() API

v3 -> v4:
   Add support for if-notification to make sure set_config()
   gets called

v2 -> v3:
v1 -> v2:
   Fixed Ilya's comments

lib/netdev-dpdk.c |   56 +++--
  1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4805783..b744589 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -46,6 +46,7 @@
  #include "dpdk.h"
  #include "dpif-netdev.h"
  #include "fatal-signal.h"
+#include "if-notifier.h"
  #include "netdev-provider.h"
  #include "netdev-vport.h"
  #include "odp-util.h"
@@ -396,6 +397,8 @@ struct netdev_dpdk {
  bool attached;
  /* If true, rte_eth_dev_start() was successfully called */
  bool started;
+bool reset_needed;
+/* 1 pad byte here. */
  struct eth_addr hwaddr;
  int mtu;
  int socket_id;
@@ -1173,6 +1176,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
  ovsrcu_index_init(>vid, -1);
  dev->vhost_reconfigured = false;
  dev->attached = false;
+dev->started = false;
+dev->reset_needed = false;
  
  ovsrcu_init(>qos_conf, NULL);
  
@@ -1769,6 +1774,36 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,

  return new_port_id;
  }
  
+static int

+dpdk_eth_event_callback(dpdk_port_t port_id, enum rte_eth_event_type type,
+void *param OVS_UNUSED, void *ret_param OVS_UNUSED)
+{
+struct netdev_dpdk *dev;
+
+switch ((int) type) {
+case RTE_ETH_EVENT_INTR_RESET:
+ovs_mutex_lock(_mutex);
+dev = netdev_dpdk_lookup_by_port_id(port_id);
+if (dev) {
+ovs_mutex_lock(>mutex);
+dev->reset_needed = true;
+netdev_request_reconfigure(>up);
+VLOG_DBG_RL(, "%s: Device reset requested.",
+netdev_get_name(>up));
+ovs_mutex_unlock(>mutex);
+}
+ovs_mutex_unlock(_mutex);
+
+if_notifier_manual_report();
+break;
+
+default:
+/* Ignore all other types. */
+break;
+   }
+   return 0;
+}
+
  static void
  dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
  OVS_REQUIRES(dev->mutex)
@@ -3807,6 +3842,8 @@ netdev_dpdk_class_init(void)
  /* This function can be called for different classes.  The initialization
   * needs to be done only once */
  if (ovsthread_once_start()) {
+int ret;
+
  ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
  unixctl_command_register("netdev-dpdk/set-admin-state",
   "[netdev] up|down", 1, 2,
@@ -3820,6 +3857,15 @@ netdev_dpdk_class_init(void)
   "[netdev]", 0, 1,
   netdev_dpdk_get_mempool_info, NULL);
  
+ret = rte_eth_dev_callback_register(RTE_ETH_ALL,

+RTE_ETH_EVENT_INTR_RESET,
+dpdk_eth_event_callback, NULL);
+


This empty line could be removed.  There is already enough empty space.


+if (ret != 0) {
+VLOG_ERR("Ethernet device callback register error: %s",
+ rte_strerror(-ret));
+}
+
  ovsthread_once_done();
  }
  
@@ -4180,13 +4226,19 @@ netdev_dpdk_reconfigure(struct netdev *netdev)

  && dev->rxq_size == dev->requested_rxq_size
  && dev->txq_size == dev->requested_txq_size
  && dev->socket_id == dev->requested_socket_id
-&& dev->started) {
+&& dev->started && !dev->reset_needed) {
  /* Reconfiguration is unnecessary */
  
  goto out;

  }
  
-rte_eth_dev_stop(dev->port_id);

+if (dev->reset_needed) {
+rte_eth_dev_reset(dev->port_id);


Thinking more about the configuration sequence it seems that call
if_notifier_manual_report() should be here and not in the callback
to be sure that settings will be applied after reset.

BTW, even if this will be always called from the main thread, I
still think that notifier itself should be thread-safe as we could
use it later 

[ovs-dev] [PATCH v2] netdev-afxdp: Best-effort configuration of XDP mode.

[Ilya Maximets]

Until now there was only two options for XDP mode in OVS: SKB or DRV.
i.e. 'generic XDP' or 'native XDP with zero-copy enabled'.

Devices like 'veth' interfaces in Linux supports native XDP, but
doesn't support zero-copy mode.  This case can not be covered by
existing API and we have to use slower generic XDP for such devices.
There are few more issues, e.g. TCP is not supported in generic XDP
mode for veth interfaces due to kernel limitations, however it is
supported in native mode.

This change introduces ability to use native XDP without zero-copy
along with best-effort configuration option that enabled by default.
In best-effort case OVS will sequentially try different modes starting
from the fastest one and will choose the first acceptable for current
interface.  This will guarantee the best possible performance.

If user will want to choose specific mode, it's still possible by
setting the 'options:xdp-mode'.

This change additionally changes the API by renaming the configuration
knob from 'xdpmode' to 'xdp-mode' and also renaming the modes
themselves to be more user-friendly.

The full list of currently supported modes:
  * native-with-zerocopy - former DRV
  * native   - new one, DRV without zero-copy
  * generic  - former SKB
  * best-effort  - new one, chooses the best available from
   3 above modes

Since 'best-effort' is a default mode, users will not need to
explicitely set 'xdp-mode' in most cases.

TCP related tests enabled back in system afxdp testsuite, because
'best-effort' will choose 'native' mode for veth interfaces
and this mode has no issues with TCP.

Signed-off-by: Ilya Maximets 
---

With this patch I modified the user-visible API, but I think it's OK
since it's still an experimental netdev.  Comments are welcome.

Version 2:
  * Rebased on current master.

 Documentation/intro/install/afxdp.rst |  54 ---
 NEWS  |  12 +-
 lib/netdev-afxdp.c| 223 --
 lib/netdev-afxdp.h|   9 ++
 lib/netdev-linux-private.h|   8 +-
 tests/system-afxdp-macros.at  |   7 -
 vswitchd/vswitch.xml  |  38 +++--
 7 files changed, 227 insertions(+), 124 deletions(-)

diff --git a/Documentation/intro/install/afxdp.rst 
b/Documentation/intro/install/afxdp.rst
index a136db0c9..937770ad0 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -153,9 +153,8 @@ To kick start end-to-end autotesting::
   make check-afxdp TESTSUITEFLAGS='1'
 
 .. note::
-   Not all test cases pass at this time. Currenly all TCP related
-   tests, ex: using wget or http, are skipped due to XDP limitations
-   on veth. cvlan test is also skipped.
+   Not all test cases pass at this time. Currenly all cvlan tests are skipped
+   due to kernel issues.
 
 If a test case fails, check the log at::
 
@@ -177,33 +176,35 @@ in :doc:`general`::
   ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
 
 Make sure your device driver support AF_XDP, netdev-afxdp supports
-the following additional options (see man ovs-vswitchd.conf.db for
+the following additional options (see ``man ovs-vswitchd.conf.db`` for
 more details):
 
- * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
+ * ``xdp-mode``: ``best-effort``, ``native-with-zerocopy``,
+   ``native`` or ``generic``.  Defaults to ``best-effort``, i.e. best of
+   supported modes, so in most cases you don't need to change it.
 
- * **use-need-wakeup**: default "true" if libbpf supports it, otherwise false.
+ * ``use-need-wakeup``: default ``true`` if libbpf supports it,
+   otherwise ``false``.
 
 For example, to use 1 PMD (on core 4) on 1 queue (queue 0) device,
-configure these options: **pmd-cpu-mask, pmd-rxq-affinity, and n_rxq**.
-The **xdpmode** can be "drv" or "skb"::
+configure these options: ``pmd-cpu-mask``, ``pmd-rxq-affinity``, and
+``n_rxq``::
 
   ethtool -L enp2s0 combined 1
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
   ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 type="afxdp" \
-options:n_rxq=1 options:xdpmode=drv \
-other_config:pmd-rxq-affinity="0:4"
+   other_config:pmd-rxq-affinity="0:4"
 
 Or, use 4 pmds/cores and 4 queues by doing::
 
   ethtool -L enp2s0 combined 4
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x36
   ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 type="afxdp" \
-options:n_rxq=4 options:xdpmode=drv \
-other_config:pmd-rxq-affinity="0:1,1:2,2:3,3:4"
+options:n_rxq=4 other_config:pmd-rxq-affinity="0:1,1:2,2:3,3:4"
 
 .. note::
-   pmd-rxq-affinity is optional. If not specified, system will auto-assign.
+   ``pmd-rxq-affinity`` is optional. If not specified, system will auto-assign.
+   ``n_rxq`` equals ``1`` by default.
 
 To validate that the bridge has successfully instantiated, you can use the::
 
@@ 

Re: [ovs-dev] [PATCH] netdev-afxdp: Best-effort configuration of XDP mode.

[Ilya Maximets]

On 07.11.2019 11:55, 0-day Robot wrote:

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

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


git-am:
fatal: sha1 information is lacking or useless (NEWS).


Oops. Mistakenly formatted the patch from the development branch.
Will rebase and re-send.

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

Re: [ovs-dev] [PATCH] netdev-afxdp: Best-effort configuration of XDP mode.

[0-day Robot]

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

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


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


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

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

[ovs-dev] [PATCH] netdev-afxdp: Best-effort configuration of XDP mode.

[Ilya Maximets]

Until now there was only two options for XDP mode in OVS: SKB or DRV.
i.e. 'generic XDP' or 'native XDP with zero-copy enabled'.

Devices like 'veth' interfaces in Linux supports native XDP, but
doesn't support zero-copy mode.  This case can not be covered by
existing API and we have to use slower generic XDP for such devices.
There are few more issues, e.g. TCP is not supported in generic XDP
mode for veth interfaces due to kernel limitations, however it is
supported in native mode.

This change introduces ability to use native XDP without zero-copy
along with best-effort configuration option that enabled by default.
In best-effort case OVS will sequentially try different modes starting
from the fastest one and will choose the first acceptable for current
interface.  This will guarantee the best possible performance.

If user will want to choose specific mode, it's still possible by
setting the 'options:xdp-mode'.

This change additionally changes the API by renaming the configuration
knob from 'xdpmode' to 'xdp-mode' and also renaming the modes
themselves to be more user-friendly.

The full list of currently supported modes:
  * native-with-zerocopy - former DRV
  * native   - new one, DRV without zero-copy
  * generic  - former SKB
  * best-effort  - new one, chooses the best available from
   3 above modes

Since 'best-effort' is a default mode, users will not need to
explicitely set 'xdp-mode' in most cases.

TCP related tests enabled back in system afxdp testsuite, because
'best-effort' will choose 'native' mode for veth interfaces
and this mode has no issues with TCP.

Signed-off-by: Ilya Maximets 
---

With this patch I modified the user-visible API, but I think it's OK
since it's still an experimental netdev.  Comments are welcome.

 Documentation/intro/install/afxdp.rst |  54 ---
 NEWS  |  12 +-
 lib/netdev-afxdp.c| 223 --
 lib/netdev-afxdp.h|   9 ++
 lib/netdev-linux-private.h|   8 +-
 tests/system-afxdp-macros.at  |   7 -
 vswitchd/vswitch.xml  |  38 +++--
 7 files changed, 227 insertions(+), 124 deletions(-)

diff --git a/Documentation/intro/install/afxdp.rst 
b/Documentation/intro/install/afxdp.rst
index a136db0c9..937770ad0 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -153,9 +153,8 @@ To kick start end-to-end autotesting::
   make check-afxdp TESTSUITEFLAGS='1'
 
 .. note::
-   Not all test cases pass at this time. Currenly all TCP related
-   tests, ex: using wget or http, are skipped due to XDP limitations
-   on veth. cvlan test is also skipped.
+   Not all test cases pass at this time. Currenly all cvlan tests are skipped
+   due to kernel issues.
 
 If a test case fails, check the log at::
 
@@ -177,33 +176,35 @@ in :doc:`general`::
   ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
 
 Make sure your device driver support AF_XDP, netdev-afxdp supports
-the following additional options (see man ovs-vswitchd.conf.db for
+the following additional options (see ``man ovs-vswitchd.conf.db`` for
 more details):
 
- * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
+ * ``xdp-mode``: ``best-effort``, ``native-with-zerocopy``,
+   ``native`` or ``generic``.  Defaults to ``best-effort``, i.e. best of
+   supported modes, so in most cases you don't need to change it.
 
- * **use-need-wakeup**: default "true" if libbpf supports it, otherwise false.
+ * ``use-need-wakeup``: default ``true`` if libbpf supports it,
+   otherwise ``false``.
 
 For example, to use 1 PMD (on core 4) on 1 queue (queue 0) device,
-configure these options: **pmd-cpu-mask, pmd-rxq-affinity, and n_rxq**.
-The **xdpmode** can be "drv" or "skb"::
+configure these options: ``pmd-cpu-mask``, ``pmd-rxq-affinity``, and
+``n_rxq``::
 
   ethtool -L enp2s0 combined 1
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
   ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 type="afxdp" \
-options:n_rxq=1 options:xdpmode=drv \
-other_config:pmd-rxq-affinity="0:4"
+   other_config:pmd-rxq-affinity="0:4"
 
 Or, use 4 pmds/cores and 4 queues by doing::
 
   ethtool -L enp2s0 combined 4
   ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x36
   ovs-vsctl add-port br0 enp2s0 -- set interface enp2s0 type="afxdp" \
-options:n_rxq=4 options:xdpmode=drv \
-other_config:pmd-rxq-affinity="0:1,1:2,2:3,3:4"
+options:n_rxq=4 other_config:pmd-rxq-affinity="0:1,1:2,2:3,3:4"
 
 .. note::
-   pmd-rxq-affinity is optional. If not specified, system will auto-assign.
+   ``pmd-rxq-affinity`` is optional. If not specified, system will auto-assign.
+   ``n_rxq`` equals ``1`` by default.
 
 To validate that the bridge has successfully instantiated, you can use the::
 
@@ -214,12 +215,21 @@ Should show something like::
 

[ovs-dev] [PATCH ovn v2] Fix ha chassis failover issues for stale ha chassis entries

[numans]

From: Numan Siddique 

If ha chassis rows of an HA chassis group become stale i.e the 
HA_Chassis.chassis
column is empty (because ovn-controller is not running in that chassis)
except one row and when ha_chassis_group_is_active()
is called on that ovn-controller, then it returns false. Ideally it should
become active since its the only active chassis. This patch fixes this issue.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1762777
Reported-by: Daniel Alvarez 

Signed-off-by: Numan Siddique 
---

v1 -> v2
--
  * Addresses Dumitru's comments.

 controller/ha-chassis.c | 25 +
 tests/ovn.at| 20 +++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/controller/ha-chassis.c b/controller/ha-chassis.c
index 6d9426a5c..d6ec7b658 100644
--- a/controller/ha-chassis.c
+++ b/controller/ha-chassis.c
@@ -142,6 +142,27 @@ ha_chassis_destroy_ordered(struct ha_chassis_ordered 
*ordered_ha_ch)
 }
 }
 
+/* Returns true if there is only one active ha chassis in the chassis group
+ * (i.e HA_Chassis.chassis column is set) and that active ha chassis is
+ * local chassis.
+ * Returns false otherwise. */
+static bool
+is_local_chassis_only_candidate(const struct sbrec_ha_chassis_group *ha_ch_grp,
+const struct sbrec_chassis *local_chassis)
+{
+size_t n_active_ha_chassis = 0;
+bool local_chassis_present = false;
+for (size_t i = 0; i < ha_ch_grp->n_ha_chassis; i++) {
+if (ha_ch_grp->ha_chassis[i]->chassis) {
+n_active_ha_chassis++;
+if (ha_ch_grp->ha_chassis[i]->chassis == local_chassis) {
+local_chassis_present = true;
+}
+}
+}
+
+return (local_chassis_present && n_active_ha_chassis == 1);
+}
 
 /* Returns true if the local_chassis is the master of
  * the HA chassis group, false otherwise. */
@@ -159,6 +180,10 @@ ha_chassis_group_is_active(
 return (ha_ch_grp->ha_chassis[0]->chassis == local_chassis);
 }
 
+if (is_local_chassis_only_candidate(ha_ch_grp, local_chassis)) {
+return true;
+}
+
 if (sset_is_empty(active_tunnels)) {
 /* If active tunnel sset is empty, it means it has lost
  * connectivity with other chassis. */
diff --git a/tests/ovn.at b/tests/ovn.at
index 410f4b514..cb7903db8 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -13413,7 +13413,25 @@ OVS_WAIT_UNTIL(
 logical_port=ls1-lp_ext1`
 test "$chassis" = "$hv1_uuid"])
 
-OVN_CLEANUP([hv1],[hv2],[hv3])
+# Stop ovn-controllers on hv1 and hv3.
+as hv1 ovn-appctl -t ovn-controller exit
+as hv3 ovn-appctl -t ovn-controller exit
+
+# hv2 should be master and claim ls1-lp_ext1
+OVS_WAIT_UNTIL(
+[chassis=`ovn-sbctl --bare --columns chassis find port_binding \
+logical_port=ls1-lp_ext1`
+test "$chassis" = "$hv2_uuid"])
+
+as hv1
+OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as hv3
+OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+OVN_CLEANUP([hv2])
 AT_CLEANUP
 
 AT_SETUP([ovn -- Address Set Incremental Processing])
-- 
2.23.0

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

[ovs-dev] ovs does not forward igmp to the interface which set mcast-snooping-flood-reports=true

[王培辉]

Hi All



I’m using ovs 2.11.3 on CentOS, I enabled multicast snooping on the bridge, I 
have a vm, and a nic with  mcast-snooping-flood-reports=true on this bridge, vm 
sends igmp packet, but ovs doesn’t forward igmp to the nic. So ovs of other 
nodes cannot learn multicast table entries.

Please feel free to correct me if I understand something wrong, looking forward 
to your reply.

Thanks.



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

[ovs-dev] [PATCH 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 | 66 -
 ovn-nb.xml  | 10 +++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c23c270dc..e1af9828b 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -2588,6 +2588,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". */
@@ -2710,6 +2712,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();
 
@@ -2759,6 +2767,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)) {
@@ -2808,6 +2822,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 {
@@ -7242,7 +7262,38 @@ 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;
+}
+
+bool prefix_delegation = smap_get_bool(>nbrp->options,
+   "prefix_delegation", false);
+if (!prefix_delegation) {
+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(, "inport == %s && ip6.dst == %s"
+  " && udp.src == 547 && udp.dst == 546",
+  op->json_key, lrp_networks.ipv6_addrs[i].addr_s);
+ds_put_format(, "reg0 = 0; dhcp6_server_pkt { "
+  "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;
@@ -8037,6 +8088,19 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 continue;
 }
 
+/* enable IPv6 prefix delegation */
+bool prefix_delegation = smap_get_bool(>nbrp->options,
+   "prefix_delegation", false);
+if (prefix_delegation) {
+struct smap options;
+
+smap_clone(, >sb->options);
+smap_add(, "ipv6_prefix_delegation", "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 d8f3237fc..7468d37ec 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2048,6 +2048,16 @@
   to true.
 
   
+
+  
+
+  If set to true, enable IPv6 prefix delegation state
+  machine on this logical router port (RFC3633). IPv6 prefix
+  delegation is available just on a gateway router or on a gateway
+  router port.
+
+  
 
 
 
-- 

[ovs-dev] [PATCH ovn 1/2] controller: add ipv6 prefix delegation state machine

[Lorenzo Bianconi]

Introduce IPv6 Prefix delegation state machine according to RFC 3633
https://tools.ietf.org/html/rfc3633.
Add dhcp6_server_pkt controller action to parse advertise/reply from
IPv6 delegation server. Advertise/reply are parsed running respectively:
- pinctrl_parse_dhcv6_advt
- pinctrl_parse_dhcv6_reply
The IPv6 requesting router starts sending dhcpv6 solicit through the logical
router ported marked with ipv6_prefix_delegationn set to true.
Save IPv6 prefix received by IPv6 delegation router in the options columns of
SB port binding table in order to be reused by Router Advertisement framework
run by ovn logical router pipeline.
IPv6 Prefix delegation state machine is enabled on Gateway Router or on
a Gateway Router Port

Signed-off-by: Lorenzo Bianconi 
---
 controller/pinctrl.c  | 553 ++
 include/ovn/actions.h |   9 +-
 lib/actions.c |  22 ++
 lib/ovn-l7.h  |  19 ++
 tests/ovn.at  |   6 +
 utilities/ovn-trace.c |   2 +
 6 files changed, 610 insertions(+), 1 deletion(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index a90ee73d6..b15c9e530 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -269,6 +269,19 @@ static void pinctrl_ip_mcast_handle_igmp(
 const struct match *md,
 struct ofpbuf *userdata);
 
+static void init_ipv6_prefixd(void);
+static void destroy_ipv6_prefixd(void);
+static void ipv6_prefixd_wait(long long int timeout);
+static void
+prepare_ipv6_prefix_req(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
+struct ovsdb_idl_index *sbrec_port_binding_by_name,
+const struct hmap *local_datapaths,
+const struct sbrec_chassis *chassis,
+const struct sset *active_tunnels)
+OVS_REQUIRES(pinctrl_mutex);
+static void
+send_ipv6_prefix_msg(struct rconn *swconn, long long int *send_prefixd_time)
+OVS_REQUIRES(pinctrl_mutex);
 static bool may_inject_pkts(void);
 
 static void init_put_vport_bindings(void);
@@ -440,6 +453,7 @@ pinctrl_init(void)
 init_put_mac_bindings();
 init_send_garps_rarps();
 init_ipv6_ras();
+init_ipv6_prefixd();
 init_buffered_packets_map();
 init_event_table();
 ip_mcast_snoop_init();
@@ -526,6 +540,55 @@ set_actions_and_enqueue_msg(struct rconn *swconn,
 ofpbuf_uninit();
 }
 
+static struct shash ipv6_prefixd;
+
+enum {
+PREFIX_SOLICIT,
+PREFIX_PENDING,
+PREFIX_DONE,
+};
+
+struct ipv6_prefixd_state {
+long long int next_announce;
+struct in6_addr ipv6_addr;
+struct eth_addr ea;
+int64_t port_key;
+int64_t metadata;
+struct in6_addr prefix;
+unsigned plife_time;
+unsigned vlife_time;
+unsigned t1;
+unsigned t2;
+int8_t plen;
+int state;
+};
+
+static void
+init_ipv6_prefixd(void)
+{
+shash_init(_prefixd);
+}
+
+static void
+ipv6_prefixd_delete(struct ipv6_prefixd_state *pfd)
+{
+if (pfd) {
+free(pfd);
+}
+}
+
+static void
+destroy_ipv6_prefixd(void)
+{
+struct shash_node *iter, *next;
+SHASH_FOR_EACH_SAFE (iter, next, _prefixd) {
+struct ipv6_prefixd_state *pfd = iter->data;
+ipv6_prefixd_delete(pfd);
+shash_delete(_prefixd, iter);
+}
+shash_destroy(_prefixd);
+}
+
 struct buffer_info {
 struct ofpbuf ofpacts;
 struct dp_packet *p;
@@ -949,6 +1012,251 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const 
struct flow *ip_flow,
 dp_packet_uninit();
 }
 
+static void
+pinctrl_parse_dhcv6_advt(struct rconn *swconn, const struct flow *ip_flow,
+ struct dp_packet *pkt_in, const struct match *md,
+ struct ofpbuf *userdata)
+{
+struct udp_header *udp_in = dp_packet_l4(pkt_in);
+unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1);
+size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in));
+uint8_t *data, *end = (uint8_t *)udp_in + dlen;
+int len = 0;
+
+data = xmalloc(dlen);
+if (!data) {
+return;
+}
+
+in_dhcpv6_data += 4;
+
+while (in_dhcpv6_data < end) {
+struct dhcpv6_opt_header *in_opt =
+ (struct dhcpv6_opt_header *)in_dhcpv6_data;
+int opt_len = sizeof *in_opt + ntohs(in_opt->len);
+
+if (dlen < opt_len + len) {
+goto out;
+}
+
+switch (ntohs(in_opt->code)) {
+case DHCPV6_OPT_IA_PD: {
+int orig_len = len, hdr_len = 0, size = sizeof *in_opt + 12;
+
+memcpy([len], in_opt, size);
+in_opt = (struct dhcpv6_opt_header *)(in_dhcpv6_data + size);
+len += size;
+
+while (size < opt_len) {
+int flen = sizeof *in_opt + ntohs(in_opt->len);
+
+if (dlen < flen + len) {
+goto out;
+}
+
+if (ntohs(in_opt->code) == DHCPV6_OPT_IA_PREFIX) {
+memcpy([len], in_opt, flen);
+  

[ovs-dev] [PATCH 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 dhcp6_server_pkt 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

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

 controller/pinctrl.c  | 553 ++
 include/ovn/actions.h |   9 +-
 lib/actions.c |  22 ++
 lib/ovn-l7.h  |  19 ++
 northd/ovn-northd.c   |  66 -
 ovn-nb.xml|  10 +
 tests/ovn.at  |   6 +
 utilities/ovn-trace.c |   2 +
 8 files changed, 685 insertions(+), 2 deletions(-)

-- 
2.21.0

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