[ovs-dev] [PATCH V4 09/12] netdev: Allow storing dpif type into netdev structure.

[Eli Britstein]

From: Ilya Maximets 

Storing of the dpif type of the owning datapath interface will allow
us to easily distinguish, for example, userspace tunneling ports from
the system ones.  This is required in terms of HW offloading to avoid
offloading of userspace flows to kernel interfaces that doesn't belong
to userspace datapath, but have same dpif_port names.

Signed-off-by: Ilya Maximets 
Acked-by: Eli Britstein 
Acked-by: Roni Bar Yanai 
Acked-by: Ophir Munk 
Signed-off-by: Eli Britstein 
---
 lib/netdev-provider.h |  3 ++-
 lib/netdev.c  | 16 
 lib/netdev.h  |  2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index d9503adb0..73dce2fca 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -96,7 +96,8 @@ struct netdev {
 
 /* Functions to control flow offloading. */
 OVSRCU_TYPE(const struct netdev_flow_api *) flow_api;
-struct netdev_hw_info hw_info; /* offload-capable netdev info */
+const char *dpif_type;  /* Type of dpif this netdev belongs to. */
+struct netdev_hw_info hw_info;  /* Offload-capable netdev info. */
 };
 
 static inline void
diff --git a/lib/netdev.c b/lib/netdev.c
index 90962eec6..91e91955c 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -1984,6 +1984,22 @@ netdev_get_class(const struct netdev *netdev)
 return netdev->netdev_class;
 }
 
+/* Set the type of 'dpif' this 'netdev' belongs to. */
+void
+netdev_set_dpif_type(struct netdev *netdev, const char *type)
+{
+netdev->dpif_type = type;
+}
+
+/* Returns the type of 'dpif' this 'netdev' belongs to.
+ *
+ * The caller must not free the returned value. */
+const char *
+netdev_get_dpif_type(const struct netdev *netdev)
+{
+return netdev->dpif_type;
+}
+
 /* Returns the netdev with 'name' or NULL if there is none.
  *
  * The caller must free the returned netdev with netdev_close(). */
diff --git a/lib/netdev.h b/lib/netdev.h
index fdbe0e1f5..fb5073056 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -179,6 +179,8 @@ bool netdev_mtu_is_user_config(struct netdev *);
 int netdev_get_ifindex(const struct netdev *);
 int netdev_set_tx_multiq(struct netdev *, unsigned int n_txq);
 enum netdev_pt_mode netdev_get_pt_mode(const struct netdev *);
+void netdev_set_dpif_type(struct netdev *, const char *);
+const char *netdev_get_dpif_type(const struct netdev *);
 
 /* Packet reception. */
 int netdev_rxq_open(struct netdev *, struct netdev_rxq **, int id);
-- 
2.14.5

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

[ovs-dev] [PATCH V4 06/12] netdev-offload-dpdk: Remove pre-validate of patterns function

[Eli Britstein]

The function of adding patterns by requested matches checks that it
consumed all the required matches, and err if not. For functional
purpose there is no need for pre-validation. For performance such
validation may decrease the time spent for failing flows, but at the
expense of increasing the time spent for the good flows, and code
complexity. Remove the pre-validation function.

Signed-off-by: Eli Britstein 
Reviewed-by: Roni Bar Yanai 
Acked-by: Sriharsha Basavapatna 
---
 lib/netdev-offload-dpdk.c | 130 ++
 1 file changed, 51 insertions(+), 79 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 9d12d553d..d115e0260 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -544,11 +544,22 @@ free_flow_actions(struct flow_actions *actions)
 
 static int
 parse_flow_match(struct flow_patterns *patterns,
- const struct match *match)
+ struct match *match)
 {
 uint8_t *next_proto_mask = NULL;
+struct flow *consumed_masks;
 uint8_t proto = 0;
 
+consumed_masks = >wc.masks;
+
+memset(_masks->in_port, 0, sizeof consumed_masks->in_port);
+/* recirc id must be zero. */
+if (match->wc.masks.recirc_id & match->flow.recirc_id) {
+return -1;
+}
+consumed_masks->recirc_id = 0;
+consumed_masks->packet_type = 0;
+
 /* Eth */
 if (match->wc.masks.dl_type ||
 !eth_addr_is_zero(match->wc.masks.dl_src) ||
@@ -566,6 +577,10 @@ parse_flow_match(struct flow_patterns *patterns,
 memcpy(>src, >wc.masks.dl_src, sizeof mask->src);
 mask->type = match->wc.masks.dl_type;
 
+memset(_masks->dl_dst, 0, sizeof consumed_masks->dl_dst);
+memset(_masks->dl_src, 0, sizeof consumed_masks->dl_src);
+consumed_masks->dl_type = 0;
+
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
 }
 
@@ -584,6 +599,11 @@ parse_flow_match(struct flow_patterns *patterns,
 
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN, spec, mask);
 }
+/* For untagged matching match->wc.masks.vlans[0].tci is 0x and
+ * match->flow.vlans[0].tci is 0. Consuming is needed outside of the if
+ * scope to handle that.
+ */
+memset(_masks->vlans[0], 0, sizeof consumed_masks->vlans[0]);
 
 /* IP v4 */
 if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
@@ -604,6 +624,12 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->hdr.src_addr= match->wc.masks.nw_src;
 mask->hdr.dst_addr= match->wc.masks.nw_dst;
 
+consumed_masks->nw_tos = 0;
+consumed_masks->nw_ttl = 0;
+consumed_masks->nw_proto = 0;
+consumed_masks->nw_src = 0;
+consumed_masks->nw_dst = 0;
+
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_IPV4, spec, mask);
 
 /* Save proto for L4 protocol setup. */
@@ -611,6 +637,11 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->hdr.next_proto_id;
 next_proto_mask = >hdr.next_proto_id;
 }
+/* If fragmented, then don't HW accelerate - for now. */
+if (match->wc.masks.nw_frag & match->flow.nw_frag) {
+return -1;
+}
+consumed_masks->nw_frag = 0;
 
 if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP  &&
 proto != IPPROTO_SCTP && proto != IPPROTO_TCP  &&
@@ -637,6 +668,10 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->hdr.data_off  = ntohs(match->wc.masks.tcp_flags) >> 8;
 mask->hdr.tcp_flags = ntohs(match->wc.masks.tcp_flags) & 0xff;
 
+consumed_masks->tp_src = 0;
+consumed_masks->tp_dst = 0;
+consumed_masks->tcp_flags = 0;
+
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_TCP, spec, mask);
 
 /* proto == TCP and ITEM_TYPE_TCP, thus no need for proto match. */
@@ -655,6 +690,9 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->hdr.src_port = match->wc.masks.tp_src;
 mask->hdr.dst_port = match->wc.masks.tp_dst;
 
+consumed_masks->tp_src = 0;
+consumed_masks->tp_dst = 0;
+
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_UDP, spec, mask);
 
 /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto match. */
@@ -673,6 +711,9 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->hdr.src_port = match->wc.masks.tp_src;
 mask->hdr.dst_port = match->wc.masks.tp_dst;
 
+consumed_masks->tp_src = 0;
+consumed_masks->tp_dst = 0;
+
 add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_SCTP, spec, mask);
 
 /* proto == SCTP and ITEM_TYPE_SCTP, thus no need for proto match. */
@@ -691,6 +732,9 @@ parse_flow_match(struct flow_patterns *patterns,
 mask->hdr.icmp_type = (uint8_t) ntohs(match->wc.masks.tp_src);
 mask->hdr.icmp_code = (uint8_t) ntohs(match->wc.masks.tp_dst);
 
+consumed_masks->tp_src = 0;
+consumed_masks->tp_dst = 0;
+
 

[ovs-dev] [PATCH V4 04/12] netdev-offload-dpdk: Fix Ethernet matching for type only

[Eli Britstein]

For OVS rule of the form "eth type is 0x1234 / end", rule is offloaded
in the form of "eth / end", which is incorrect. Fix it.

Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Eli Britstein 
Reviewed-by: Roni Bar Yanai 
Acked-by: Sriharsha Basavapatna 
---
 lib/netdev-offload-dpdk.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

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

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

[ovs-dev] [PATCH V4 03/12] dpif-netdev: Don't use zero flow mark

[Eli Britstein]

Zero flow mark is used to indicate the HW to remove the mark. A packet
marked with zero mark is received in SW without a mark at all, so it
cannot be used as a valid mark. Change the pool range to fix it.

Fixes: 241bad15d99a ("dpif-netdev: associate flow with a mark id")
Signed-off-by: Eli Britstein 
Reviewed-by: Roni Bar Yanai 
Acked-by: Sriharsha Basavapatna 
---
 lib/dpif-netdev.c|  8 ++--
 tests/dpif-netdev.at | 18 +-
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 86a5e63ba..6462b1652 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2247,7 +2247,11 @@ dp_netdev_pmd_find_dpcls(struct dp_netdev_pmd_thread 
*pmd,
 }
 
 #define MAX_FLOW_MARK   (UINT32_MAX - 1)
-#define INVALID_FLOW_MARK   (UINT32_MAX)
+#define INVALID_FLOW_MARK   0
+/* Zero flow mark is used to indicate the HW to remove the mark. A packet
+ * marked with zero mark is received in SW without a mark at all, so it
+ * cannot be used as a valid mark.
+ */
 
 struct megaflow_to_mark_data {
 const struct cmap_node node;
@@ -2273,7 +2277,7 @@ flow_mark_alloc(void)
 
 if (!flow_mark.pool) {
 /* Haven't initiated yet, do it here */
-flow_mark.pool = id_pool_create(0, MAX_FLOW_MARK);
+flow_mark.pool = id_pool_create(1, MAX_FLOW_MARK);
 }
 
 if (id_pool_alloc_id(flow_mark.pool, )) {
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index ff173677a..ec5ffc290 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -395,7 +395,7 @@ 
skb_priority(0),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),recirc
# Check that flow successfully offloaded.
OVS_WAIT_UNTIL([grep "succeed to add netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
-p1: flow put[[create]]: flow match: 
recirc_id=0,eth,ip,in_port=1,vlan_tci=0x,nw_frag=no, mark: 0
+p1: flow put[[create]]: flow match: 
recirc_id=0,eth,ip,in_port=1,vlan_tci=0x,nw_frag=no, mark: 1
 ])
# Check that datapath flow installed successfully.
AT_CHECK([filter_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
@@ -406,7 +406,7 @@ 
recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), a
 
# Check for succesfull packet matching with installed offloaded flow.
AT_CHECK([filter_hw_packet_netdev_dummy < ovs-vswitchd.log | strip_xout], 
[0], [dnl
-p1: packet: 
ip,vlan_tci=0x,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=127.0.0.1,nw_dst=127.0.0.1,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=64
 matches with flow: recirc_id=0,eth,ip,vlan_tci=0x,nw_frag=no with mark: 0
+p1: packet: 
ip,vlan_tci=0x,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=127.0.0.1,nw_dst=127.0.0.1,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=64
 matches with flow: recirc_id=0,eth,ip,vlan_tci=0x,nw_frag=no with mark: 1
 ])
 
ovs-appctl revalidator/wait
@@ -423,7 +423,7 @@ 
recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), p
# Check that flow successfully deleted from HW.
OVS_WAIT_UNTIL([grep "succeed to delete netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_del < ovs-vswitchd.log | strip_xout], [0], [dnl
-p1: flow del: mark: 0
+p1: flow del: mark: 1
 ])
OVS_VSWITCHD_STOP
AT_CLEANUP])
@@ -462,7 +462,7 @@ 
packet_type(ns=0,id=0),eth(src=00:06:07:08:09:0a,dst=00:01:02:03:04:05),eth_type
# Check that flow successfully offloaded.
OVS_WAIT_UNTIL([grep "succeed to add netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
-p1: flow put[[create]]: flow match: 
recirc_id=0,eth,udp,in_port=1,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82,
 mark: 0
+p1: flow put[[create]]: flow match: 
recirc_id=0,eth,udp,in_port=1,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82,
 mark: 1
 ])
# Check that datapath flow installed successfully.
AT_CHECK([filter_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
@@ -474,7 +474,7 @@ 
recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x8100),vlan(vid=99,pcp=
# Check for succesfull packet matching with installed offloaded flow.
AT_CHECK([filter_hw_packet_netdev_dummy < ovs-vswitchd.log | strip_xout], 
[0], [dnl
 p1: packet: 
udp,dl_vlan=99,dl_vlan_pcp=7,vlan_tci1=0x,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=127.0.0.1,nw_dst=127.0.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=81,tp_dst=82
 dnl
-matches with flow: 
recirc_id=0,eth,udp,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82
 with mark: 0
+matches with flow: 
recirc_id=0,eth,udp,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82
 with mark: 1
 ])
 
ovs-appctl revalidator/wait
@@ -492,7 +492,7 @@ packets:1, bytes:64, used:0.0s, 
actions:set(ipv4(src=192.168.0.7)),set(udp(dst=3
# Check that flow successfully deleted from HW.
OVS_WAIT_UNTIL([grep 

[ovs-dev] [PATCH V4 10/12] netdev-offload: Use dpif type instead of class.

[Eli Britstein]

From: Ilya Maximets 

There is no real difference between the 'class' and 'type' in the
context of common lookup operations inside netdev-offload module
because it only checks the value of pointers without using the
value itself.  However, 'type' has some meaning and can be used by
offload provides on the initialization phase to check if this type
of Flow API in pair with the netdev type could be used in particular
datapath type.  For example, this is needed to check if Linux flow
API could be used for current tunneling vport because it could be
used only if tunneling vport belongs to system datapath, i.e. has
backing linux interface.

This is needed to unblock tunneling offloads in userspace datapath
with DPDK flow API.

Signed-off-by: Ilya Maximets 
Acked-by: Eli Britstein 
Acked-by: Roni Bar Yanai 
Acked-by: Ophir Munk 
Signed-off-by: Eli Britstein 
---
 lib/dpif-netdev.c | 15 +++--
 lib/dpif-netlink.c| 23 ++-
 lib/dpif.c| 21 +
 lib/netdev-offload-dpdk.c | 17 ++
 lib/netdev-offload-tc.c   |  3 ++-
 lib/netdev-offload.c  | 52 +--
 lib/netdev-offload.h  | 16 ++---
 ofproto/ofproto-dpif-upcall.c |  5 ++---
 8 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6462b1652..c80ca877d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2378,10 +2378,11 @@ static int
 mark_to_flow_disassociate(struct dp_netdev_pmd_thread *pmd,
   struct dp_netdev_flow *flow)
 {
-int ret = 0;
-uint32_t mark = flow->mark;
+const char *dpif_type_str = dpif_normalize_type(pmd->dp->class->type);
 struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,
  >mark_node);
+uint32_t mark = flow->mark;
+int ret = 0;
 
 cmap_remove(_mark.mark_to_flow, mark_node, hash_int(mark, 0));
 flow->mark = INVALID_FLOW_MARK;
@@ -2394,7 +2395,7 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread 
*pmd,
 struct netdev *port;
 odp_port_t in_port = flow->flow.in_port.odp_port;
 
-port = netdev_ports_get(in_port, pmd->dp->class);
+port = netdev_ports_get(in_port, dpif_type_str);
 if (port) {
 /* Taking a global 'port_mutex' to fulfill thread safety
  * restrictions for the netdev-offload-dpdk module. */
@@ -2502,9 +2503,9 @@ static int
 dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
 {
 struct dp_netdev_pmd_thread *pmd = offload->pmd;
-const struct dpif_class *dpif_class = pmd->dp->class;
 struct dp_netdev_flow *flow = offload->flow;
 odp_port_t in_port = flow->flow.in_port.odp_port;
+const char *dpif_type_str = dpif_normalize_type(pmd->dp->class->type);
 bool modification = offload->op == DP_NETDEV_FLOW_OFFLOAD_OP_MOD;
 struct offload_info info;
 struct netdev *port;
@@ -2540,9 +2541,8 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item 
*offload)
 }
 }
 info.flow_mark = mark;
-info.dpif_class = dpif_class;
 
-port = netdev_ports_get(in_port, pmd->dp->class);
+port = netdev_ports_get(in_port, dpif_type_str);
 if (!port || netdev_vport_is_vport_class(port->netdev_class)) {
 netdev_close(port);
 goto err_free;
@@ -3156,7 +3156,8 @@ dpif_netdev_get_flow_offload_status(const struct 
dp_netdev *dp,
 return false;
 }
 
-netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, dp->class);
+netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port,
+  dpif_normalize_type(dp->class->type));
 if (!netdev) {
 return false;
 }
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 18322e879..7da4fb54d 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1120,6 +1120,7 @@ dpif_netlink_port_get_pid(const struct dpif *dpif_, 
odp_port_t port_no)
 static int
 dpif_netlink_flow_flush(struct dpif *dpif_)
 {
+const char *dpif_type_str = dpif_normalize_type(dpif_type(dpif_));
 const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
 struct dpif_netlink_flow flow;
 
@@ -1128,7 +1129,7 @@ dpif_netlink_flow_flush(struct dpif *dpif_)
 flow.dp_ifindex = dpif->dp_ifindex;
 
 if (netdev_is_flow_api_enabled()) {
-netdev_ports_flow_flush(dpif_->dpif_class);
+netdev_ports_flow_flush(dpif_type_str);
 }
 
 return dpif_netlink_flow_transact(, NULL, NULL);
@@ -1445,7 +1446,7 @@ start_netdev_dump(const struct dpif *dpif_,
 ovs_mutex_lock(>netdev_lock);
 dump->netdev_current_dump = 0;
 dump->netdev_dumps
-= netdev_ports_flow_dump_create(dpif_->dpif_class,
+= netdev_ports_flow_dump_create(dpif_normalize_type(dpif_type(dpif_)),
 >netdev_dumps_num,
 

[ovs-dev] [PATCH V4 02/12] dpif-netdev: Add mega ufid in flow add/del log

[Eli Britstein]

As offload is done using the mega ufid of a flow, for better
debugability, add it in the log message.

Signed-off-by: Eli Britstein 
Reviewed-by: Roni Bar Yanai 
---
 lib/dpif-netdev.c   | 13 +
 tests/dpif-netdev.at|  2 ++
 tests/ofproto-macros.at |  3 ++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 1086efd47..86a5e63ba 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2351,7 +2351,8 @@ mark_to_flow_associate(const uint32_t mark, struct 
dp_netdev_flow *flow)
 hash_int(mark, 0));
 flow->mark = mark;
 
-VLOG_DBG("Associated dp_netdev flow %p with mark %u\n", flow, mark);
+VLOG_DBG("Associated dp_netdev flow %p with mark %u mega_ufid "UUID_FMT
+ "\n",flow, mark, UUID_ARGS((struct uuid *) >mega_ufid));
 }
 
 static bool
@@ -2400,7 +2401,8 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread 
*pmd,
 }
 
 flow_mark_free(mark);
-VLOG_DBG("Freed flow mark %u\n", mark);
+VLOG_DBG("Freed flow mark %u mega_ufid "UUID_FMT"\n", mark,
+ UUID_ARGS((struct uuid *) >mega_ufid));
 
 megaflow_to_mark_disassociate(>mega_ufid);
 }
@@ -2607,8 +2609,9 @@ dp_netdev_flow_offload_main(void *data OVS_UNUSED)
 OVS_NOT_REACHED();
 }
 
-VLOG_DBG("%s to %s netdev flow\n",
- ret == 0 ? "succeed" : "failed", op);
+VLOG_DBG("%s to %s netdev flow "UUID_FMT"\n",
+ ret == 0 ? "succeed" : "failed", op,
+ UUID_ARGS((struct uuid *) >flow->mega_ufid));
 dp_netdev_free_flow_offload(offload);
 ovsrcu_quiesce();
 }
@@ -3479,6 +3482,8 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
 
 ds_put_cstr(, "flow_add: ");
 odp_format_ufid(ufid, );
+ds_put_cstr(, " mega_");
+odp_format_ufid(>mega_ufid, );
 ds_put_cstr(, " ");
 odp_flow_format(key_buf.data, key_buf.size,
 mask_buf.data, mask_buf.size,
diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index 9c0a42d00..ff173677a 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -13,6 +13,7 @@ strip_timers () {
 
 strip_xout () {
 sed '
+s/mega_ufid:[-0-9a-f]* //
 s/ufid:[-0-9a-f]* //
 s/used:[0-9]*\.[0-9]*/used:0.0/
 s/actions:.*/actions: /
@@ -23,6 +24,7 @@ strip_xout () {
 
 strip_xout_keep_actions () {
 sed '
+s/mega_ufid:[-0-9a-f]* //
 s/ufid:[-0-9a-f]* //
 s/used:[0-9]*\.[0-9]*/used:0.0/
 s/packets:[0-9]*/packets:0/
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index b2b17eed3..87f9ae280 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -131,7 +131,8 @@ strip_duration () {
 # Strips 'ufid:...' from output, to make it easier to compare.
 # (ufids are random.)
 strip_ufid () {
-sed 's/ufid:[[-0-9a-f]]* //'
+sed 's/mega_ufid:[[-0-9a-f]]* //
+s/ufid:[[-0-9a-f]]* //'
 }
 m4_divert_pop([PREPARE_TESTS])
 
-- 
2.14.5

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

[ovs-dev] [PATCH V4 07/12] netdev-offload-dpdk: Add IPv6 pattern matching

[Eli Britstein]

Add support for IPv6 pattern matching for offloading flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Roni Bar Yanai 
Acked-by: Sriharsha Basavapatna 
---
 Documentation/howto/dpdk.rst |  2 +-
 NEWS |  1 +
 lib/netdev-offload-dpdk.c| 77 
 3 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index c40fcafcb..ebde9aeb9 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -385,7 +385,7 @@ The validated NICs are:
 Supported protocols for hardware offload matches are:
 
 - L2: Ethernet, VLAN
-- L3: IPv4
+- L3: IPv4, IPv6
 - L4: TCP, UDP, SCTP, ICMP
 
 Supported actions for hardware offload are:
diff --git a/NEWS b/NEWS
index 0116b3ea0..056e9a67e 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,7 @@ Post-v2.13.0
  * Deprecated DPDK pdump packet capture support removed.
  * Deprecated DPDK ring ports (dpdkr) are no longer supported.
  * Add hardware offload support for VLAN Push/Pop actions (experimental).
+ * Add hardware offload support for matching IPv6 protocol (experimental).
- Linux datapath:
  * Support for kernel versions up to 5.5.x.
- AF_XDP:
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index d115e0260..ef6105ddb 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -16,6 +16,8 @@
  */
 #include 
 
+#include 
+#include 
 #include 
 
 #include "cmap.h"
@@ -292,6 +294,43 @@ dump_flow_pattern(struct ds *s, const struct rte_flow_item 
*item)
   tcp_mask1->hdr.tcp_flags);
 }
 ds_put_cstr(s, "/ ");
+} else if (item->type == RTE_FLOW_ITEM_TYPE_IPV6) {
+const struct rte_flow_item_ipv6 *ipv6_spec = item->spec;
+const struct rte_flow_item_ipv6 *ipv6_mask = item->mask;
+
+char src_addr_str[INET6_ADDRSTRLEN];
+char dst_addr_str[INET6_ADDRSTRLEN];
+
+ds_put_cstr(s, "ipv6 ");
+if (ipv6_spec) {
+const struct rte_flow_item_ipv6 *ipv6_mask1 = ipv6_mask
+? ipv6_mask : _flow_item_ipv6_mask;
+char mask1_src_addr_str[INET6_ADDRSTRLEN];
+char mask1_dst_addr_str[INET6_ADDRSTRLEN];
+
+ipv6_string_mapped(src_addr_str,
+   (struct in6_addr *)_spec->hdr.src_addr);
+ipv6_string_mapped(dst_addr_str,
+   (struct in6_addr *)_spec->hdr.dst_addr);
+ipv6_string_mapped(mask1_src_addr_str,
+   (struct in6_addr *)_mask1->hdr.src_addr);
+ipv6_string_mapped(mask1_dst_addr_str,
+   (struct in6_addr *)_mask1->hdr.dst_addr);
+
+DUMP_TESTPMD_ITEM(ipv6_mask1->hdr.src_addr, "src", "%s",
+  src_addr_str, mask1_src_addr_str);
+DUMP_TESTPMD_ITEM(ipv6_mask1->hdr.dst_addr, "dst", "%s",
+  dst_addr_str, mask1_dst_addr_str);
+DUMP_TESTPMD_ITEM(ipv6_mask1->hdr.proto, "proto", "%"PRIu8,
+  ipv6_spec->hdr.proto, ipv6_mask1->hdr.proto);
+DUMP_TESTPMD_ITEM(ipv6_mask1->hdr.vtc_flow, "tc", "0x%"PRIx32,
+  ntohl(ipv6_spec->hdr.vtc_flow),
+  ntohl(ipv6_mask1->hdr.vtc_flow));
+DUMP_TESTPMD_ITEM(ipv6_mask1->hdr.hop_limits, "hop", "%"PRIu8,
+  ipv6_spec->hdr.hop_limits,
+  ipv6_mask1->hdr.hop_limits);
+}
+ds_put_cstr(s, "/ ");
 } else {
 ds_put_format(s, "unknown rte flow pattern (%d)\n", item->type);
 }
@@ -643,6 +682,44 @@ parse_flow_match(struct flow_patterns *patterns,
 }
 consumed_masks->nw_frag = 0;
 
+/* IP v6 */
+if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) {
+struct rte_flow_item_ipv6 *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
+
+spec->hdr.proto = match->flow.nw_proto;
+spec->hdr.hop_limits = match->flow.nw_ttl;
+spec->hdr.vtc_flow =
+htonl((uint32_t)match->flow.nw_tos << RTE_IPV6_HDR_TC_SHIFT);
+memcpy(spec->hdr.src_addr, >flow.ipv6_src,
+   sizeof spec->hdr.src_addr);
+memcpy(spec->hdr.dst_addr, >flow.ipv6_dst,
+   sizeof spec->hdr.dst_addr);
+
+mask->hdr.proto = match->wc.masks.nw_proto;
+mask->hdr.hop_limits = match->wc.masks.nw_ttl;
+mask->hdr.vtc_flow =
+htonl((uint32_t)match->wc.masks.nw_tos << RTE_IPV6_HDR_TC_SHIFT);
+memcpy(mask->hdr.src_addr, >wc.masks.ipv6_src,
+   sizeof mask->hdr.src_addr);
+memcpy(mask->hdr.dst_addr, >wc.masks.ipv6_dst,
+   sizeof mask->hdr.dst_addr);
+
+consumed_masks->nw_proto = 0;
+consumed_masks->nw_ttl = 0;
+consumed_masks->nw_tos = 0;
+

[ovs-dev] [PATCH V4 05/12] netdev-offload-dpdk: Support partial TCP/UDP port matching

[Eli Britstein]

The cited commit failed partial matching of TCP/UDP port matching,
preventing such offload of supporting HWs. Remove this failure.

Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Eli Britstein 
Reviewed-by: Roni Bar Yanai 
Acked-by: Sriharsha Basavapatna 
---
 lib/netdev-offload-dpdk.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 613e7e3ea..9d12d553d 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -621,11 +621,6 @@ parse_flow_match(struct flow_patterns *patterns,
 return -1;
 }
 
-if ((match->wc.masks.tp_src && match->wc.masks.tp_src != OVS_BE16_MAX) ||
-(match->wc.masks.tp_dst && match->wc.masks.tp_dst != OVS_BE16_MAX)) {
-return -1;
-}
-
 if (proto == IPPROTO_TCP) {
 struct rte_flow_item_tcp *spec, *mask;
 
-- 
2.14.5

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

Re: [ovs-dev] [PATCH v4 5/7] dpif-lookup: add avx512 gather implementation.

[William Tu]

On Tue, Jun 30, 2020 at 3:01 AM Van Haaren, Harry
 wrote:
>
> > -Original Message-
> > From: William Tu 
> > Sent: Saturday, June 27, 2020 7:27 PM
> > To: Van Haaren, Harry 
> > Cc: ovs-dev ; Stokes, Ian ;
> > Ilya Maximets ; Federico Iezzi 
> > Subject: Re: [PATCH v4 5/7] dpif-lookup: add avx512 gather implementation.
> >
> > On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
> >  wrote:
> > >
> > > This commit adds an AVX-512 dpcls lookup implementation.
> > > It uses the AVX-512 SIMD ISA to perform multiple miniflow
> > > operations in parallel.
> > >
> > > To run this implementation, the "avx512f" and "bmi2" ISAs are
> > > required. These ISA checks are performed at runtime while
> > > probing the subtable implementation. If a CPU does not provide
> > > both "avx512f" and "bmi2", then this code does not execute.
> > >
> > > The avx512 code is built as a seperate static library, with added
> > > CFLAGS to enable the required ISA features. By building only this
> > > static library with avx512 enabled, it is ensured that the main OVS
> > > core library is *not* using avx512, and that OVS continues to run
> > > as before on CPUs that do not support avx512.
> > >
> > > The approach taken in this implementation is to use the
> > > gather instruction to access the packet miniflow, allowing
> > > any miniflow blocks to be loaded into an AVX-512 register.
> > > This maximises the usefulness of the register, and hence this
> > > implementation handles any subtable with up to miniflow 8 bits.
> > >
> > > Note that specialization of these avx512 lookup routines
> > > still provides performance value, as the hashing of the
> > > resulting data is performed in scalar code, and compile-time
> > > loop unrolling occurs when specialized to miniflow bits.
> > >
> > > Signed-off-by: Harry van Haaren 
> > >
> > > ---
> > >
> > > v4:
> > > - Remove TODO comment on prio-set command (was accidentally
> > >   added to this commit in v3)
> > > - Fixup v3 changlog to not include #warning comment (William Tu)
> > > - Remove #define for debugging in lookup.h
> > > - Fix builds on older gcc versions that don't support -mavx512f.
> > >   Solution involves CC_CHECK and #ifdefs in code (OVS Robot, William Tu)
> > >
> > > v3:
> > > - Improve function name for _any subtable lookup
> > > - Use "" include not <> for immintrin.h
> > > - Add checks for SSE42 instructions in core OVS for CRC32 based hashing
> > >   If not available, disable AVX512 lookup implementation as it requires
> > >   uses CRC32 for hashing, and the hashing algorithm must match core OVS.
> > > - Rework ovs_asserts() into function selection time check
> > > - Add #define for magic number 8, number of u64 blocks in AVX512 register
> > > - Add #if CHECKER around AVX code, sparse doesn't like checking it
> > > - Simplify avx512 enabled building, fixes builds with --enable-shared
> > > ---
> > >  configure.ac   |   2 +
> > >  lib/automake.mk|  17 ++
> > >  lib/dpif-netdev-lookup-avx512-gather.c | 265 +
> > >  lib/dpif-netdev-lookup.c   |  17 ++
> > >  lib/dpif-netdev-lookup.h   |   4 +
> > >  5 files changed, 305 insertions(+)
> > >  create mode 100644 lib/dpif-netdev-lookup-avx512-gather.c
> > >
> > > diff --git a/configure.ac b/configure.ac
> > > index 81893e56e..1367c868b 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -178,6 +178,8 @@ OVS_ENABLE_OPTION([-Wno-null-pointer-arithmetic])
> > >  OVS_ENABLE_OPTION([-Warray-bounds-pointer-arithmetic])
> > >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused], [HAVE_WNO_UNUSED])
> > >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter],
> > [HAVE_WNO_UNUSED_PARAMETER])
> > > +OVS_CONDITIONAL_CC_OPTION([-mavx512f], [HAVE_AVX512F])
> > > +OVS_CHECK_CC_OPTION([-mavx512f], [CFLAGS="$CFLAGS -DHAVE_AVX512F"])
> >
> > Do you need both checks?
> > I thought the first one OVS_CONDITIONAL_CC_OPTION([-mavx512f],
> > [HAVE_AVX512F])
> > is good enough since at lib/automake.mk, you add the -mavx512f to CFLAGS.
>
> From testing during development, both are required.
> CONDITIONAL_CC_OPTION adds a build-system flag, indicating its present, but 
> doesn't
> seem to add a C #define for it, that can be used for conditional compilation?
>
> The CHECK_CC_OPTION is used to manually add a #define via command-line -D 
> parameter, it is used to add the avx512_gather probe function in the 
> available lookup function struct.
>
> There may be a more elegant way to achieve both in the same line, my AC-fu is 
> somewhat outdated, suggestions welcome if you know of a better method :)
>
I see, thanks. I don't know any better way.

> 
>
> > > +#include "immintrin.h"
> > > +
> > > +/* Each AVX512 register (zmm register in assembly notation) can contain 
> > > up
> > to
> > > + * 512 bits, which is equivelent to 8 uint64_t variables. This is the 
> > > maximum
> >
> > typo: equivalent
>
> Will fix.
>
>
> > > + * number of miniflow blocks that can be 

Re: [ovs-dev] [PATCH v4 1/7] dpif-netdev: implement subtable lookup validation.

[William Tu]

On Tue, Jun 30, 2020 at 4:37 AM Van Haaren, Harry
 wrote:
>
> > -Original Message-
> > From: Van Haaren, Harry
> > Sent: Tuesday, June 30, 2020 11:01 AM
> > To: William Tu 
> > Cc: ovs-dev ; Stokes, Ian ; 
> > Ilya
> > Maximets ; Federico Iezzi 
> > Subject: RE: [PATCH v4 1/7] dpif-netdev: implement subtable lookup 
> > validation.
> >
> > > -Original Message-
> > > From: William Tu 
> > > Sent: Saturday, June 27, 2020 7:27 PM
> > > To: Van Haaren, Harry 
> > > Cc: ovs-dev ; Stokes, Ian ;
> > > Ilya Maximets ; Federico Iezzi 
> > > Subject: Re: [PATCH v4 1/7] dpif-netdev: implement subtable lookup 
> > > validation.
> > >
> > > On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
> > >  wrote:
>
> 
>
> > > > - Add check to ensure autovalidator is 0th item in lookup array
> > >
> > > What's your concern here? Users can only change the prio, not
> > > the .probe function. So autovalidator is always at 0th item.
> > > If you worry about code being changed accidentally in the future,
> > > how about using BUILD_ASSERT_DECL?
> >
> > Yes - good improvement.
>
> Indeed the goal is to ensure the autovalidator is the 0th lookup, and
> that code is not changed accidentally. It seems like using a BUILD_ASSERT_DECL
> would achieve that, however the compiler doesn't agree with me:
>
> lib/dpif-netdev-lookup.c:67:45: error: expression in static assertion is not 
> constant
>  BUILD_ASSERT_DECL(subtable_lookups[0].probe == 
> dpcls_subtable_autovalidator_probe);
>~~^~
>
> In this case, the subtable_lookups[] is static, but not const. It cannot be 
> const as
> the user must be able to change the .priority field of each lookup 
> implementation.
> Doing a build time assert is (correctly) flagging that we're asserting 
> non-const data,
> which could be changed during runtime.
>
> The autovalidator component has a runtime check using ovs_assert() today, so a
> check is already being done:
> ovs_assert(lookup_funcs[0].probe(0, 0) == dpcls_subtable_autovalidator);
>
> Summary:
> - The existing check in autovalidator is enough to ensure its element 0 in 
> the lookups array.
> - Adding a BUILD_ASSERT_DECL isn't allowed by the compiler, as its not const 
> data being validated.
>
Thanks for trying it out. Then the existing checking way looks ok.
Thank you
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ctags: Include new annotations to ctags ignore list.

[William Tu]

On Tue, Jun 30, 2020 at 8:33 AM Flavio Leitner  wrote:
>
> On Sat, Jun 27, 2020 at 05:11:15PM -0700, William Tu wrote:
> > On Wed, Jun 10, 2020 at 04:49:45PM -0300, Flavio Leitner wrote:
> > > The annotation OVS_NO_THREAD_SAFETY_ANALYSIS and OVS_LOCKABLE are
> > > not part of the list, so ctags can't find functions using them.
> > >
> > > The annotation list comes from a regex and to include more items
> > > make the regex more difficult to read and maintain. Convert to a
> > > static list because it isn't supposed to change much and there
> > > is no standard names.
> > >
> > > Also add a comment to remind to keep the list up-to-date.
> > >
> > > Signed-off-by: Flavio Leitner 
> >
> > Hi Flavio,
> >
> > Instead of a static list, how about adding
> > sed -n -e 's/^#define \(OVS_[A-Z_]\+\)(\.\.\.)$/\1+/p' \
> >-e 's/^#define \(OVS_[A-Z_]\+\)$/\1+/p' 
> > include/openvswitch/compiler.h
> >
> > So with the 2nd sed command, it creates
> > OVS_NO_RETURN+
> > OVS_UNUSED+
> > OVS_WARN_UNUSED_RESULT+
> > OVS_LOCKABLE+
> > OVS_GUARDED+
> > OVS_NO_THREAD_SAFETY_ANALYSIS+
> > OVS_PACKED_ENUM+
> >
> > Which covers the OVS_NO_THREAD_SAFETY_ANALYSIS+ and others.
>
> The '+' is required only when parenthesis could be used, so
> it wouldn't apply to OVS_NO_RETURN, for instance. I don't know
> the side effects.

I see, thanks!

>
> Another thing is that the '-I' parameter is useful when an
> identifier causes syntactic confusion. I can navigate to symbols
> using OVS_UNUSED and I don't see any problems. Same for OVS_NO_RETURN.
>
> Funny thing is that I had issues with OVS_LOCKABLE but not anymore.
> I did upgrade my fedora in between, so not sure.
>
> Anyways, I still have issues navigating to dpdk_do_tx_copy() because
> of OVS_NO_THREAD_SAFETY_ANALYSIS. It works if the identifier is
> included in the list.
>
> > I'm also ok keeping it static, if so, should we add these above?
>
> I understand the goal of fixing one time and leave it automatically,
> but ctags man-page is clear about using the parameter when an
> identifier causes problems. I don't know what else it does, so I
> took the safe approach.
>
> What do you think?

Thanks for explaining this. In this case, I think keeping it static makes sense.
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 ovn] Fix seg fault while encoding DHCP domain search option.

[Ankur Sharma]

Hi Dhathri,

Please Add "Fixes" header to commit message.
Acked-by: Ankur Sharma 

Regards,
Ankur

From: dev  on behalf of 
svc.eng.git-pa...@nutanix.com 
Sent: Tuesday, June 30, 2020 10:04 AM
To: ovs-dev@openvswitch.org 
Cc: Dhathri Purohith 
Subject: [ovs-dev] [PATCH v1 ovn] Fix seg fault while encoding DHCP domain 
search option.

From: Dhathri Purohith 

Some versions of strtok_r make the original string NULL at the
end of parsing. Adding Null check before ovs_strlcpy() to
prevent segfault in such cases.

Fixes: d79bb92c4b49 ("Add support for DHCP domain search option (119)")

Signed-off-by: Dhathri Purohith 
---
 lib/actions.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/actions.c b/lib/actions.c
index ee7c825..c435188 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2382,7 +2382,9 @@ encode_put_dhcpv4_option(const struct ovnact_gen_option 
*o,
 memcpy(dns_encoded + encode_offset, label, len);
 encode_offset += len;
 }
-   ovs_strlcpy(suffix, domain, strlen(domain));
+if (domain != NULL) {
+ovs_strlcpy(suffix, domain, strlen(domain));
+}
 }
 /* Add the end marker (0 byte) to determine the end of the
  * domain. */
--
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=mrAPsNW3OUjyTaiBr9x2Wdad6HyRZlWB0aP_jabab2Q=BP9Lyas6JfOFnD5o4bicAfuqjSVXjXMSyUVHPki1Ijo=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Taller de ortografía y redacción

[Textos claros, concisos y entendibles]

El entorno laboral actual requiere de profesionistas capaces de expresarse por 
escrito de forma clara, coherente y con
impecable ortografía. Este curso en línea está enfocado en abordar las 
principales reglas ortográficas del idioma
español y en otorgar recomendaciones a los participantes para una redacción 
correcta de los textos profesionales o
de negocios más comunes tanto tradicionales como digitales.

Paquete: Taller de ortografía y redacción para profesionistas y negocios. 
(Curso en línea en vivo)


Sáb 11/07/20   | 10 am a 12:30 pm | Taller de ortografía básica para 
profesionistas y negocios.
Sáb 18/07/20   | 10 am a 12:30 pm | Taller de redacción para profesionistas y 
negocios.
Sáb 25/07/20   | 10 am a 12:00 pm | Redacción de documentos y textos ejecutivos.


Incluye: Conexión al curso para una computadora o móvil, Material del curso, 
Reconocimiento, Interacción en directo para resolución de dudas.

Proporcionaremos a los participantes guías para una correcta redacción de 
textos que sean claros, concisos y entendibles
para el ámbito profesional.

¿Te interesa este curso?
Solicita información respondiendo a este correo con la palabra TRedacción, 
junto con los siguientes datos: 

Nombre:
Teléfono:
Empresa:
Correo Alterno: 

¿Dudas sobre este Evento?

Centro de atención a clientes:
Llámanos al 5530167085 - 5530167085 6630



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

[ovs-dev] ATTN: Please Check Your Credit Report

[Tiffany Castillo]

Image of your report
For: 

Date: 




Due to Equifax's latest security breach, your current
TransUnion, Equifax, and Experian scores may have changed. 





Your Scores are available now at no charge. 


View Your Score



Unsubscribe 
To stop receiving emails from us please click above or write to:
A/43, Amarabati, Kolkata 700110

###T_URL###









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

Re: [ovs-dev] 10-25 packet drops every few (10-50) seconds TCP (iperf3)

[Shahaji Bhosle via dev]

Hi Flavio,
I still see intermittent drops with rcu_nocbs. So I wrote that do_nothing()
loop..to avoid all the other distractions to see if Linux is messing with
the OVS loop just to see what is going on. The interesting thing I see the
case *BOLD* below where I use an ISB() instruction my STD deviation is well
within Both the results are basically DO NOTHING FOR 100msec and see what
happens to time :)
Thanks, Shahaji

static inline uint64_t
*rte_get_tsc_cycles*(void)
{
uint64_t tsc;
#ifdef USE_ISB
asm volatile("*isb*; mrs %0, pmccntr_el0" : "=r"(tsc));
#else
asm volatile("mrs %0, pmccntr_el0" : "=r"(tsc));
#endif
return tsc;
}
#endif /*RTE_ARM_EAL_RDTSC_USE_PMU*/

==
usleep(100);
for (volatile int i=0; i wrote:

>
>
> Hi Shahaji,
>
> Did it help with the rcu_nocbs?
>
> fbl
>
> On Tue, Jun 30, 2020 at 12:56:27PM -0400, Shahaji Bhosle wrote:
> > Thanks Flavio,
> > Are there any special requirements for RCU on ARM vs x86.
> >
> > I am following what the above document is saying...Do you think I need to
> > do something more than the below?
> > Thanks again and appreciate the help. Shahaji
> >
> > 1. Isolate the CPU cores
> > *isolcpus=1,2,3,4,5,6,7 nohz_full=1-7 rcu_nocbs=1-7*
> > 2. Setting CONFIG_NO_HZ_FULL=y
> > root@bcm958802a8046c:~/vinay_rx/dynticks-testing# zcat /proc/config.gz
> > |grep HZ
> > CONFIG_NO_HZ_COMMON=y
> > # CONFIG_HZ_PERIODIC is not set
> > # CONFIG_NO_HZ_IDLE is not set
> > *CONFIG_NO_HZ_FULL*=y
> > # CONFIG_NO_HZ_FULL_ALL is not set
> > # CONFIG_NO_HZ is not set
> > # CONFIG_HZ_100 is not set
> > CONFIG_HZ_250=y
> > # CONFIG_HZ_300 is not set
> > # CONFIG_HZ_1000 is not set
> > CONFIG_HZ=250
> >
> >
> >
> > On Tue, Jun 30, 2020 at 12:50 PM Flavio Leitner 
> wrote:
> >
> > >
> > > Right, you might want to review Documentation/timers/no_hz.rst from
> > > the kernel sources and look for RCU implications section where
> > > it explains how to move RCU callbacks.
> > >
> > > fbl
> > >
> > > On Tue, Jun 30, 2020 at 12:08:05PM -0400, Shahaji Bhosle wrote:
> > > > Hi Flavio,
> > > > I wrote a small program which has do_nothing for loop and I measure
> the
> > > > timestamps across the do nothing loop. I am seeing 3% of the time
> around
> > > > the 1 second mark when the arch_timer fires I get the timestamps to
> be
> > > off
> > > > by 25% of the exprected value. I ran trace-cmd to see what is going
> on
> > > and
> > > > see the below. Looks like some issue with *gic_handle_irg*(), not
> seeing
> > > > tihs behaviour on x86 host, something special with ARM v8.
> > > > Thanks, Shahaji
> > > >
> > > >   %21.77  (14181) arm_stb_user_lorcu_dyntick #922
> > > >  |
> > > >  --- *rcu_dyntick*
> > > > |
> > > > |--%46.85-- gic_handle_irq  # 432
> > > > |
> > > > |--%23.32-- context_tracking_user_exit  # 215
> > > > |
> > > > |--%22.34-- context_tracking_user_enter  # 206
> > > > |
> > > > |--%2.60-- SyS_execve  # 24
> > > > |
> > > > |--%1.30-- do_page_fault  # 12
> > > > |
> > > > |--%0.65-- SyS_write  # 6
> > > > |
> > > > |--%0.65-- schedule  # 6
> > > > |
> > > > |--%0.65-- SyS_nanosleep  # 6
> > > > |
> > > > |--%0.65-- syscall_trace_enter  # 6
> > > > |
> > > > |--%0.65-- SyS_faccessat  # 6
> > > >
> > > >   %5.01  (14181) arm_stb_user_lorcu_utilization #212
> > > >  |
> > > >  --- *rcu_utilization*
> > > > |
> > > > |--%96.23-- gic_handle_irq  # 204
> > > > |
> > > > |--%1.89-- SyS_nanosleep  # 4
> > > > |
> > > > |--%0.94-- SyS_exit_group  # 2
> > > > |
> > > > |--%0.94-- do_notify_resume  # 2
> > > >
> > > >   %4.86  (14181) arm_stb_user_lo  user_exit #206
> > > >  |
> > > >  --- *user_exit*
> > > >   context_tracking_user_exit
> > > >
> > > >   %4.86  (14181) arm_stb_user_lo context_tracking_user_exit #206
> > > >  |
> > > >  --- context_tracking_user_exit
> > > >
> > > >   %4.86  (14181) arm_stb_user_locontext_tracking_user_enter #206
> > > >  |
> > > >  --- context_tracking_user_enter
> > > >
> > > >   %4.86  (14181) arm_stb_user_lo user_enter #206
> > > >  |
> > > >  --- *user_enter*
> > > >   context_tracking_user_enter
> > > >
> > > >   %2.95  (14181) arm_stb_user_lo gic_handle_irq #125
> > > >  |
> > > >  --- gic_handle_irq
> > > >
> > > >
> > > > On Tue, Jun 30, 2020 at 9:45 AM Flavio Leitner 
> wrote:
> > > >
> > > > > On Tue, Jun 02, 2020 at 12:56:51PM -0700, Vinay Gupta wrote:
> > > > > > Hi Flavio,
> > > > > >
> > > > > > Thanks for your reply.
> > > > > > I have captured the suggested 

Re: [ovs-dev] [PATCH ovn 0/4] Reduce number of flows in IN_IP_INPUT table for DNAT.

[Dumitru Ceara]

On 6/26/20 3:53 PM, Dumitru Ceara wrote:
> On 6/26/20 3:51 PM, Numan Siddique wrote:
>>
>>
>> On Fri, Jun 26, 2020 at 7:02 PM Dumitru Ceara > > wrote:
>>
>> On 6/26/20 3:05 PM, Numan Siddique wrote:
>> >
>> >
>> > On Wed, Jun 24, 2020 at 9:21 PM Dumitru Ceara > 
>> > >> wrote:
>> >
>> >     The first three patches refactor the ARP/NS responder code for
>> logical
>> >     routers in order to make it easier for patch #4 to configure
>> the flows
>> >     with different priorities depending on logical port type.
>> >
>> >     Suggested-by: Han Zhou mailto:hz...@ovn.org>
>> >>
>> >     Reported-by: Girish Moodalbail > 
>> >     >>
>> >     Reported-at:
>> >   
>>  https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050186.html
>> >     Signed-off-by: Dumitru Ceara > 
>> >     >>
>> >
>> >     Dumitru Ceara (4):
>> >           ovn-northd: Store ETH address of router inport in xxreg0.
>> >           ovn-northd: Refactor ARP/NS responder in router pipeline.
>> >           ovn-northd: Refactor NAT address parsing.
>> >           ovn-northd: Minimize number of ARP/NS responder flows
>> for DNAT.
>> >
>> >
>> > Hi Dumitru,
>> >
>> > Thanks for this patch series.
>> >
>> > I didn't review them thoroughly. I've few comments.
>> >
>>
>> Hi Numan,
>>
>> Thanks for looking at these patches.
>>
>> > 1. Looks like reg0 and xxreg0 are already used in the router
>> pipeline. I
>> > think it's better to use a different register (xxreg1 may be).
>>
>> Yes, xxreg0 is already used in the router pipeline, but only in later
>> stages. However, I'll use xxreg1 instead just to make sure there's no
>> confusion.
>>
>>
>> Thanks.
>>  
>>
>>
>> >    I'd suggest adding a new macro REG_IP_ROUTING (or something more
>> > meaningful) instead of using reg0/xxreg0 in the existing
>> >    code in the lr_in_ip_input/lr_in_arp_resolve stages.
>> >
>>
>> I'm not really sure I follow. I added a macro:
>>
>> #define REG_INPORT_ETH_ADDR "xxreg0[80..127]"
>>
>> And there's no bare reference to xxreg0 in my patches, I always used the
>> macro. Did I miss anything?
>>
>>
>> Sorry for not being very clear.
>> I mean the existing logical flows (not related to your patch).
>> One eg
>> - https://github.com/ovn-org/ovn/blob/master/northd/ovn-northd.c#L9749
>>
>> I know it's not exactly related to your patch set, but if you're fine
>> adding a new macro
>> for existing xxreg0 and using the macro instead that would be great :)
>>
>> Feel free to ignore this if it's too much work.
>>  
> 
> Ah I had misunderstood, sorry. No problem, I'll add another patch to the
> series to refactor this part too.
> 
> Thanks,
> Dumitru
> 
>>
>>
>> $ git diff origin/master northd/ovn-northd.c | grep xxreg
>> +#define REG_INPORT_ETH_ADDR "xxreg0[80..127]"
>> $
>>
>> > 2. Does patch 2 require updating the ovn-northd.8.xml ?
>> >
>>
>> It shouldn't have to because it's just a refactoring that doesn't change
>> the way flows are generated.
>>
>> > 3. For patches which modify the logical flows, can you please add
>> a few
>> > test cases in ovn-northd.at 
>> . That would also help
>> > me while
>> >    reviewing the code :)
>> >
>>
>> Sure, will do in v2.
>>
>>
>> Thanks
>> Numan
>>  

Hi Numan,

I sent a v2 of the series including your suggestions:

https://patchwork.ozlabs.org/project/openvswitch/list/?series=186776

Thanks,
Dumitru


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

[ovs-dev] [PATCH v2 ovn 3/5] ovn-northd: Refactor ARP/NS responder in router pipeline.

[Dumitru Ceara]

Add functions to build the ARP/NS responder flows for table
S_ROUTER_IN_IP_INPUT and use them in all places where responder
flows are created.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.8.xml |8 +
 northd/ovn-northd.c |  314 +--
 tests/ovn-northd.at |   72 +--
 3 files changed, 181 insertions(+), 213 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 78e2a71..84224ff 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1778,7 +1778,7 @@ arp.tha = arp.sha;
 arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 output;
 
@@ -1870,7 +1870,7 @@ arp.tha = arp.sha;
 arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 output;
 
@@ -1900,7 +1900,7 @@ nd_na {
 nd.tll = xreg0[0..47];
 ip6.src = A;
 nd.target = A;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 output;
 }
@@ -2570,7 +2570,7 @@ reg8[0..15] = 0;
 xxreg0 = G;
 xxreg1 = A;
 eth.src = E;
-outport = P;
+outport = inport;
 flags.loopback = 1;
 next;
 
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 9277040..fde9198 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7967,6 +7967,105 @@ lrouter_nat_is_stateless(const struct nbrec_nat *nat)
 return false;
 }
 
+/* Builds the logical flow that replies to ARP requests for an 'ip_address'
+ * owned by the router. The flow is inserted in table S_ROUTER_IN_IP_INPUT
+ * with the given priority.
+ */
+static void
+build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
+   const char *ip_address, const char *eth_addr,
+   struct ds *extra_match, uint16_t priority,
+   struct hmap *lflows, const struct ovsdb_idl_row *hint)
+{
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ds actions = DS_EMPTY_INITIALIZER;
+
+if (op) {
+ds_put_format(, "inport == %s && ", op->json_key);
+}
+
+ds_put_format(, "arp.op == 1 && arp.tpa == %s", ip_address);
+
+if (extra_match && ds_last(extra_match) != EOF) {
+ds_put_format(, " && %s", ds_cstr(extra_match));
+}
+ds_put_format(,
+  "eth.dst = eth.src; "
+  "eth.src = %s; "
+  "arp.op = 2; /* ARP reply */ "
+  "arp.tha = arp.sha; "
+  "arp.sha = %s; "
+  "arp.tpa = arp.spa; "
+  "arp.spa = %s; "
+  "outport = inport; "
+  "flags.loopback = 1; "
+  "output;",
+  eth_addr,
+  eth_addr,
+  ip_address);
+
+ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
+ds_cstr(), ds_cstr(), hint);
+
+ds_destroy();
+ds_destroy();
+}
+
+/* Builds the logical flow that replies to NS requests for an 'ip_address'
+ * owned by the router. The flow is inserted in table S_ROUTER_IN_IP_INPUT
+ * with the given priority. If 'sn_ip_address' is non-NULL, requests are
+ * restricted only to packets with IP destination 'ip_address' or
+ * 'sn_ip_address'.
+ */
+static void
+build_lrouter_nd_flow(struct ovn_datapath *od, struct ovn_port *op,
+  const char *action, const char *ip_address,
+  const char *sn_ip_address, const char *eth_addr,
+  struct ds *extra_match, uint16_t priority,
+  struct hmap *lflows,
+  const struct ovsdb_idl_row *hint)
+{
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ds actions = DS_EMPTY_INITIALIZER;
+
+if (op) {
+ds_put_format(, "inport == %s && ", op->json_key);
+}
+
+if (sn_ip_address) {
+ds_put_format(, "ip6.dst == {%s, %s} && ",
+  ip_address, sn_ip_address);
+}
+
+ds_put_format(, "nd_ns && nd.target == %s", ip_address);
+
+if (extra_match && ds_last(extra_match) != EOF) {
+ds_put_format(, " && %s", ds_cstr(extra_match));
+}
+
+ds_put_format(,
+  "%s { "
+"eth.src = %s; "
+"ip6.src = %s; "
+"nd.target = %s; "
+"nd.tll = %s; "
+"outport = inport; "
+"flags.loopback = 1; "
+"output; "
+  "};",
+  action,
+  eth_addr,
+  ip_address,
+  ip_address,
+  eth_addr);
+
+ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
+ds_cstr(), ds_cstr(), hint);
+
+ds_destroy();
+ds_destroy();
+}
+
 static void
 build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 struct hmap *lflows, 

[ovs-dev] [PATCH v2 ovn 2/5] ovn-northd: Store ETH address of router inport in xreg0.

[Dumitru Ceara]

This helps simplifying logical flows that need to use the port's
configured ETH address:
- ARP responders for owned IPs
- NS responders for owned IPs

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.8.xml |   22 ---
 northd/ovn-northd.c |  159 +--
 tests/ovn-northd.at |  140 +
 3 files changed, 236 insertions(+), 85 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index a7639f3..78e2a71 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1487,7 +1487,9 @@ output;
   For each enabled router port P with Ethernet address
   E, a priority-50 flow that matches inport ==
   P  (eth.mcast || eth.dst ==
-  E), with action next;.
+  E), stores the router port ethernet address
+  and advances to next table, with action
+  xreg0[0..47]=E; next;.
 
 
 
@@ -1507,7 +1509,7 @@ output;
   a priority-50 flow that matches inport == GW
eth.dst == E, where GW
   is the logical router gateway port, with action
-  next;.
+  xreg0[0..47]=E; next;.
 
 
 
@@ -1770,10 +1772,10 @@ next;
 
 
 eth.dst = eth.src;
-eth.src = E;
+eth.src = xreg0[0..47];
 arp.op = 2; /* ARP reply. */
 arp.tha = arp.sha;
-arp.sha = E;
+arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
 outport = P;
@@ -1822,10 +1824,10 @@ output;
 
 
 nd_na_router {
-eth.src = E;
+eth.src = xreg0[0..47];
 ip6.src = A;
 nd.target = A;
-nd.tll = E;
+nd.tll = xreg0[0..47];
 outport = inport;
 flags.loopback = 1;
 output;
@@ -1862,10 +1864,10 @@ nd_na_router {
 
 
 eth.dst = eth.src;
-eth.src = E;
+eth.src = xreg0[0..47];
 arp.op = 2; /* ARP reply. */
 arp.tha = arp.sha;
-arp.sha = E;
+arp.sha = xreg0[0..47];
 arp.tpa = arp.spa;
 arp.spa = A;
 outport = P;
@@ -1894,8 +1896,8 @@ output;
 
 eth.dst = eth.src;
 nd_na {
-eth.src = E;
-nd.tll = E;
+eth.src = xreg0[0..47];
+nd.tll = xreg0[0..47];
 ip6.src = A;
 nd.target = A;
 outport = P;
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 1a47f5f..9277040 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -223,6 +223,11 @@ enum ovn_stage {
 #define REGBIT_LOOKUP_NEIGHBOR_RESULT "reg9[2]"
 #define REGBIT_SKIP_LOOKUP_NEIGHBOR "reg9[3]"
 
+/* Register to store the eth address associated to a router port for packets
+ * received in S_ROUTER_IN_ADMISSION.
+ */
+#define REG_INPORT_ETH_ADDR "xreg0[0..47]"
+
 /* Register for ECMP bucket selection. */
 #define REG_ECMP_GROUP_ID   "reg8[0..15]"
 #define REG_ECMP_MEMBER_ID  "reg8[16..31]"
@@ -246,41 +251,43 @@ enum ovn_stage {
  * +--+-+
  *
  * Logical Router pipeline:
- * +-+--+---+-+
- * | R0  | REGBIT_ND_RA_OPTS_RESULT |   | |
- * | |IPv4-NEXT-HOP |   | |
- * +-+--+   | |
- * | R1  | IPv4-SRC-IP for ARP-REQ  |   | |
- * +-+--+   | |
- * | R2  |UNUSED| X | |
- * +-+--+ X | |
- * | R3  |UNUSED| R |IPv6 |
- * +-+--+ E |  NEXT-HOP   |
- * | R4  |UNUSED| G | |
- * +-+--+ 0 | |
- * | R5  |UNUSED|   | |
- * +-+--+   | |
- * | R6  |UNUSED|   | |
- * +-+--+   | |
- * | R7  |UNUSED|   | |
- * +-+--+---+-+
- * | R8  | ECMP_GROUP_ID|   | |
- * | | ECMP_MEMBER_ID   |   | |
- * +-+--+   | |
- * | R9  |UNUSED|   | |
- * +-+--+   | |
- * | R10 |UNUSED| X | |
- * +-+--+ X | |
- * | R11 |UNUSED| R | IPv6-SRC-IP |
- * +-+--+ E |   for NS|
- * | R12 |UNUSED| G | |
- * +-+--+ 1 | |
- * | R13 |UNUSED|   | |
- * +-+--+   | |
- * | R14 |UNUSED|   | |
- * +-+--+   | |
- * | R15 |UNUSED|   | |
- * +-+--+---+-+
+ * +-+--+---+-+---+-+
+ * | R0  | REGBIT_ND_RA_OPTS_RESULT |   

[ovs-dev] [PATCH v2 ovn 5/5] ovn-northd: Minimize number of ARP/NS responder flows for DNAT.

[Dumitru Ceara]

Most ARP/NS responder flows can be configured per datapath instead of per
router port.

The only exception is with distributed gateway router ports which need
special treatment. This patch changes the ARP/NS responder behavior and adds:
- Priority 92 flows to reply to ARP requests on distributed gateway router
  ports, on the chassis where the DNAT entry is bound.
- Priority 91 flows to drop ARP requests on distributed gateway router ports,
  on chassis where the DNAT entry is not bound.
- Priority 90 flows to reply to ARP requests on all other router ports. This
  last type of flows is programmed exactly once per logical router limiting
  the total number of required logical flows.

Suggested-by: Han Zhou 
Reported-by: Girish Moodalbail 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050186.html
Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.8.xml |   16 +++-
 northd/ovn-northd.c |  203 ---
 tests/ovn-northd.at |   65 +--
 tests/ovn.at|8 +-
 4 files changed, 190 insertions(+), 102 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 84224ff..11607c0 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1857,9 +1857,8 @@ nd_na_router {
   IPv4: For a configured DNAT IP address or a load balancer
   IPv4 VIP A, for each router port P with
   Ethernet address E, a priority-90 flow matches
-  inport == P  arp.op == 1 
-  arp.tpa == A (ARP request)
-  with the following actions:
+  arp.op == 1  arp.tpa == A
+  (ARP request) with the following actions:
 
 
 
@@ -1876,6 +1875,11 @@ output;
 
 
 
+  IPv4: For a configured load balancer IPv4 VIP, a similar flow is
+  added with the additional match inport == P.
+
+
+
   If the router port P is a distributed gateway router
   port, then the is_chassis_resident(P) is
   also added in the match condition for the load balancer IPv4
@@ -1922,9 +1926,11 @@ nd_na {
 
   
 If the corresponding NAT rule cannot be handled in a
-distributed manner, then this flow is only programmed on
+distributed manner, then a priority-92 flow is programmed on
 the gateway port instance on the
-redirect-chassis.  This behavior avoids
+redirect-chassis.  A priority-91 drop flow is
+programmed on the other chassis when ARP requests/NS packets
+are received on the gateway port. This behavior avoids
 generation of multiple ARP responses from different chassis,
 and allows upstream MAC learning to point to the
 redirect-chassis.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 51ef532..25fb90f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8039,7 +8039,7 @@ lrouter_nat_is_stateless(const struct nbrec_nat *nat)
 static void
 build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
const char *ip_address, const char *eth_addr,
-   struct ds *extra_match, uint16_t priority,
+   struct ds *extra_match, bool drop, uint16_t priority,
struct hmap *lflows, const struct ovsdb_idl_row *hint)
 {
 struct ds match = DS_EMPTY_INITIALIZER;
@@ -8054,20 +8054,24 @@ build_lrouter_arp_flow(struct ovn_datapath *od, struct 
ovn_port *op,
 if (extra_match && ds_last(extra_match) != EOF) {
 ds_put_format(, " && %s", ds_cstr(extra_match));
 }
-ds_put_format(,
-  "eth.dst = eth.src; "
-  "eth.src = %s; "
-  "arp.op = 2; /* ARP reply */ "
-  "arp.tha = arp.sha; "
-  "arp.sha = %s; "
-  "arp.tpa = arp.spa; "
-  "arp.spa = %s; "
-  "outport = inport; "
-  "flags.loopback = 1; "
-  "output;",
-  eth_addr,
-  eth_addr,
-  ip_address);
+if (drop) {
+ds_put_format(, "drop;");
+} else {
+ds_put_format(,
+  "eth.dst = eth.src; "
+  "eth.src = %s; "
+  "arp.op = 2; /* ARP reply */ "
+  "arp.tha = arp.sha; "
+  "arp.sha = %s; "
+  "arp.tpa = arp.spa; "
+  "arp.spa = %s; "
+  "outport = inport; "
+  "flags.loopback = 1; "
+  "output;",
+  eth_addr,
+  eth_addr,
+  ip_address);
+}
 
 ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
 ds_cstr(), ds_cstr(), hint);
@@ -8086,7 +8090,7 @@ static 

[ovs-dev] [PATCH v2 ovn 4/5] ovn-northd: Refactor NAT address parsing.

[Dumitru Ceara]

Store NAT entries pointers in ovn_datapath and pre-parse the external IP
addresses. This simplifies the code and makes it easier to reuse the parsed
external IP and solicited-node address without reparsing.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |  115 ++-
 1 file changed, 85 insertions(+), 30 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index fde9198..51ef532 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -608,6 +608,9 @@ struct ovn_datapath {
  * the "redirect-chassis". */
 struct ovn_port *l3redirect_port;
 
+/* NAT entries configured on the router. */
+struct ovn_nat *nat_entries;
+
 struct ovn_port **localnet_ports;
 size_t n_localnet_ports;
 
@@ -620,6 +623,65 @@ struct ovn_datapath {
 struct hmap nb_pgs;
 };
 
+/* Contains a NAT entry with the external addresses pre-parsed. */
+struct ovn_nat {
+const struct nbrec_nat *nb;
+struct lport_addresses ext_addrs;
+};
+
+/* Returns true if a 'nat_entry' is valid, i.e.:
+ * - parsing was successful.
+ * - the string yielded exactly one IPv4 address or exactly one IPv6 address.
+ */
+static bool nat_entry_is_valid(const struct ovn_nat *nat_entry)
+{
+const struct lport_addresses *ext_addrs = _entry->ext_addrs;
+
+return (ext_addrs->n_ipv4_addrs == 1 && ext_addrs->n_ipv6_addrs == 0) ||
+(ext_addrs->n_ipv4_addrs == 0 && ext_addrs->n_ipv6_addrs == 1);
+}
+
+static bool nat_entry_is_v6(const struct ovn_nat *nat_entry)
+{
+return nat_entry->ext_addrs.n_ipv6_addrs > 0;
+}
+
+static void init_nat_entries(struct ovn_datapath *od)
+{
+if (!od->nbr || od->nbr->n_nat == 0) {
+return;
+}
+
+od->nat_entries = xmalloc(od->nbr->n_nat * sizeof *od->nat_entries);
+
+for (size_t i = 0; i < od->nbr->n_nat; i++) {
+const struct nbrec_nat *nat = od->nbr->nat[i];
+struct ovn_nat *nat_entry = >nat_entries[i];
+
+nat_entry->nb = nat;
+if (!extract_ip_addresses(nat->external_ip,
+  _entry->ext_addrs) ||
+!nat_entry_is_valid(nat_entry)) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+
+VLOG_WARN_RL(,
+ "Bad ip address %s in nat configuration "
+ "for router %s", nat->external_ip, od->nbr->name);
+}
+}
+}
+
+static void destroy_nat_entries(struct ovn_datapath *od)
+{
+if (!od->nbr) {
+return;
+}
+
+for (size_t i = 0; i < od->nbr->n_nat; i++) {
+destroy_lport_addresses(>nat_entries[i].ext_addrs);
+}
+}
+
 /* A group of logical router datapaths which are connected - either
  * directly or indirectly.
  * Each logical router can belong to only one group. */
@@ -677,6 +739,8 @@ ovn_datapath_destroy(struct hmap *datapaths, struct 
ovn_datapath *od)
 ovn_destroy_tnlids(>port_tnlids);
 bitmap_free(od->ipam_info.allocated_ipv4s);
 free(od->router_ports);
+destroy_nat_entries(od);
+free(od->nat_entries);
 free(od->localnet_ports);
 ovn_ls_port_group_destroy(>nb_pgs);
 destroy_mcast_info_for_datapath(od);
@@ -1105,6 +1169,7 @@ join_datapaths(struct northd_context *ctx, struct hmap 
*datapaths,
 ovs_list_push_back(nb_only, >list);
 }
 init_mcast_info_for_datapath(od);
+init_nat_entries(od);
 ovs_list_push_back(lr_list, >lr_list);
 }
 }
@@ -8454,30 +8519,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 snat_ips[n_snat_ips++] = snat_ip;
 }
 
-for (int i = 0; i < op->od->nbr->n_nat; i++) {
-const struct nbrec_nat *nat;
-
-nat = op->od->nbr->nat[i];
+for (size_t i = 0; i < op->od->nbr->n_nat; i++) {
+struct ovn_nat *nat_entry = >od->nat_entries[i];
+const struct nbrec_nat *nat = nat_entry->nb;
 
-ovs_be32 ip;
-struct in6_addr ipv6;
-bool is_v6 = false;
-if (!ip_parse(nat->external_ip, ) || !ip) {
-if (!ipv6_parse(nat->external_ip, )) {
-static struct vlog_rate_limit rl =
-VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(, "bad ip address %s in nat configuration "
- "for router %s", nat->external_ip, op->key);
-continue;
-}
-is_v6 = true;
+/* Skip entries we failed to parse. */
+if (!nat_entry_is_valid(nat_entry)) {
+continue;
 }
 
 if (!strcmp(nat->type, "snat")) {
-if (is_v6) {
+if (nat_entry_is_v6(nat_entry)) {
+struct in6_addr *ipv6 =
+_entry->ext_addrs.ipv6_addrs[0].addr;
+
 snat_ips[n_snat_ips].family = AF_INET6;
-

[ovs-dev] [PATCH v2 ovn 0/5] Reduce number of flows in IN_IP_INPUT table for DNAT.

[Dumitru Ceara]

Patch 1 documents and refactors the usage of OVS registers in logical
flows.

Patches 2-4 refactor the ARP/NS responder code for logical routers in
order to make it easier for patch 5 to configure the flows with different
priorities depending on logical port type.

Suggested-by: Han Zhou 
Reported-by: Girish Moodalbail 
Reported-at: 
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050186.html
Signed-off-by: Dumitru Ceara 

Dumitru Ceara (5):
  ovn-northd: Document OVS register usage in logical flows.
  ovn-northd: Store ETH address of router inport in xreg0.
  ovn-northd: Refactor ARP/NS responder in router pipeline.
  ovn-northd: Refactor NAT address parsing.
  ovn-northd: Minimize number of ARP/NS responder flows for DNAT.


 northd/ovn-northd.8.xml |   46 ++-
 northd/ovn-northd.c |  733 +--
 tests/ovn-northd.at |  149 ++
 tests/ovn.at|8 -
 4 files changed, 634 insertions(+), 302 deletions(-)


---
v2:
- Addressed Numan's comments:
  - Inserted a new patch in the beginning of the series to document
OVS register usage in logical flows. Also refactored the code
to avoid using bare register names.
  - Added unit tests to ovn-northd.at in every patch that changed
logical flows.
  - Added/updated documentation in ovn-northd.8.xml in every patch that
changed logical flows.

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

[ovs-dev] [PATCH v2 ovn 1/5] ovn-northd: Document OVS register usage in logical flows.

[Dumitru Ceara]

Also, use macros instead of bare references to register names.

Signed-off-by: Dumitru Ceara 
---
 northd/ovn-northd.c |  166 ---
 1 file changed, 118 insertions(+), 48 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ae1b85a..1a47f5f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -227,8 +227,63 @@ enum ovn_stage {
 #define REG_ECMP_GROUP_ID   "reg8[0..15]"
 #define REG_ECMP_MEMBER_ID  "reg8[16..31]"
 
+/* Registers used for routing. */
+#define REG_NEXT_HOP_IPV4 "reg0"
+#define REG_NEXT_HOP_IPV6 "xxreg0"
+#define REG_SRC_IPV4 "reg1"
+#define REG_SRC_IPV6 "xxreg1"
+
 #define FLAGBIT_NOT_VXLAN "flags[1] == 0"
 
+/*
+ * OVS register usage:
+ *
+ * Logical Switch pipeline:
+ * +--+-+
+ * | R0   | REGBIT_{CONNTRACK/DHCP/DNS/HAIRPIN} |
+ * +--+-+
+ * | R1 - R15 |  UNUSED |
+ * +--+-+
+ *
+ * Logical Router pipeline:
+ * +-+--+---+-+
+ * | R0  | REGBIT_ND_RA_OPTS_RESULT |   | |
+ * | |IPv4-NEXT-HOP |   | |
+ * +-+--+   | |
+ * | R1  | IPv4-SRC-IP for ARP-REQ  |   | |
+ * +-+--+   | |
+ * | R2  |UNUSED| X | |
+ * +-+--+ X | |
+ * | R3  |UNUSED| R |IPv6 |
+ * +-+--+ E |  NEXT-HOP   |
+ * | R4  |UNUSED| G | |
+ * +-+--+ 0 | |
+ * | R5  |UNUSED|   | |
+ * +-+--+   | |
+ * | R6  |UNUSED|   | |
+ * +-+--+   | |
+ * | R7  |UNUSED|   | |
+ * +-+--+---+-+
+ * | R8  | ECMP_GROUP_ID|   | |
+ * | | ECMP_MEMBER_ID   |   | |
+ * +-+--+   | |
+ * | R9  |UNUSED|   | |
+ * +-+--+   | |
+ * | R10 |UNUSED| X | |
+ * +-+--+ X | |
+ * | R11 |UNUSED| R | IPv6-SRC-IP |
+ * +-+--+ E |   for NS|
+ * | R12 |UNUSED| G | |
+ * +-+--+ 1 | |
+ * | R13 |UNUSED|   | |
+ * +-+--+   | |
+ * | R14 |UNUSED|   | |
+ * +-+--+   | |
+ * | R15 |UNUSED|   | |
+ * +-+--+---+-+
+ *
+ */
+
 /* Returns an "enum ovn_stage" built from the arguments. */
 static enum ovn_stage
 ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline,
@@ -7127,15 +7182,15 @@ build_routing_policy_flow(struct hmap *lflows, struct 
ovn_datapath *od,
 ds_put_format(, "pkt.mark = %u; ", pkt_mark);
 }
 bool is_ipv4 = strchr(rule->nexthop, '.') ? true : false;
-ds_put_format(, "%sreg0 = %s; "
-  "%sreg1 = %s; "
+ds_put_format(, "%s = %s; "
+  "%s = %s; "
   "eth.src = %s; "
   "outport = %s; "
   "flags.loopback = 1; "
   "next;",
-  is_ipv4 ? "" : "xx",
+  is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
   rule->nexthop,
-  is_ipv4 ? "" : "xx",
+  is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6,
   lrp_addr_s,
   out_port->lrp_networks.ea_s,
   out_port->json_key);
@@ -7525,14 +7580,14 @@ build_ecmp_route_flow(struct hmap *lflows, struct 
ovn_datapath *od,
   REG_ECMP_MEMBER_ID" == %"PRIu16,
   eg->id, er->id);
 ds_clear();
-ds_put_format(, "%sreg0 = %s; "
-  "%sreg1 = %s; "
+ds_put_format(, "%s = %s; "
+  "%s = %s; "
   "eth.src = %s; "
   "outport = %s; "
   "next;",
-  is_ipv4 ? "" : "xx",
+  is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
   route->nexthop,
-  is_ipv4 ? "" : "xx",
+  is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6,
   lrp_addr_s,
   

[ovs-dev] Stop. Cleaning. Your. Gutters. Forever.

[Makayla Moore]

LeafFilter Gutter Protection | End Gutter Cleaning Foreverwww. leaffilter. com
Eliminate gutter cleaning forever with LeafFilter, the most advanced 
debris-blocking gutter protection.  Schedule a FREE LeafFilter estimate today. 


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

Re: [ovs-dev] [ovs-dev v2] datapath-windows, conntrack: Fix conntrack new state

[William Tu]

On Sun, Jun 28, 2020 at 7:19 PM Rui Cao  wrote:
>
> Thanks William.  Could you help backport the patch to branch-2.13?
done, thanks
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 10-25 packet drops every few (10-50) seconds TCP (iperf3)

[Flavio Leitner]

Hi Shahaji,

Did it help with the rcu_nocbs? 

fbl

On Tue, Jun 30, 2020 at 12:56:27PM -0400, Shahaji Bhosle wrote:
> Thanks Flavio,
> Are there any special requirements for RCU on ARM vs x86.
> 
> I am following what the above document is saying...Do you think I need to
> do something more than the below?
> Thanks again and appreciate the help. Shahaji
> 
> 1. Isolate the CPU cores
> *isolcpus=1,2,3,4,5,6,7 nohz_full=1-7 rcu_nocbs=1-7*
> 2. Setting CONFIG_NO_HZ_FULL=y
> root@bcm958802a8046c:~/vinay_rx/dynticks-testing# zcat /proc/config.gz
> |grep HZ
> CONFIG_NO_HZ_COMMON=y
> # CONFIG_HZ_PERIODIC is not set
> # CONFIG_NO_HZ_IDLE is not set
> *CONFIG_NO_HZ_FULL*=y
> # CONFIG_NO_HZ_FULL_ALL is not set
> # CONFIG_NO_HZ is not set
> # CONFIG_HZ_100 is not set
> CONFIG_HZ_250=y
> # CONFIG_HZ_300 is not set
> # CONFIG_HZ_1000 is not set
> CONFIG_HZ=250
> 
> 
> 
> On Tue, Jun 30, 2020 at 12:50 PM Flavio Leitner  wrote:
> 
> >
> > Right, you might want to review Documentation/timers/no_hz.rst from
> > the kernel sources and look for RCU implications section where
> > it explains how to move RCU callbacks.
> >
> > fbl
> >
> > On Tue, Jun 30, 2020 at 12:08:05PM -0400, Shahaji Bhosle wrote:
> > > Hi Flavio,
> > > I wrote a small program which has do_nothing for loop and I measure the
> > > timestamps across the do nothing loop. I am seeing 3% of the time around
> > > the 1 second mark when the arch_timer fires I get the timestamps to be
> > off
> > > by 25% of the exprected value. I ran trace-cmd to see what is going on
> > and
> > > see the below. Looks like some issue with *gic_handle_irg*(), not seeing
> > > tihs behaviour on x86 host, something special with ARM v8.
> > > Thanks, Shahaji
> > >
> > >   %21.77  (14181) arm_stb_user_lorcu_dyntick #922
> > >  |
> > >  --- *rcu_dyntick*
> > > |
> > > |--%46.85-- gic_handle_irq  # 432
> > > |
> > > |--%23.32-- context_tracking_user_exit  # 215
> > > |
> > > |--%22.34-- context_tracking_user_enter  # 206
> > > |
> > > |--%2.60-- SyS_execve  # 24
> > > |
> > > |--%1.30-- do_page_fault  # 12
> > > |
> > > |--%0.65-- SyS_write  # 6
> > > |
> > > |--%0.65-- schedule  # 6
> > > |
> > > |--%0.65-- SyS_nanosleep  # 6
> > > |
> > > |--%0.65-- syscall_trace_enter  # 6
> > > |
> > > |--%0.65-- SyS_faccessat  # 6
> > >
> > >   %5.01  (14181) arm_stb_user_lorcu_utilization #212
> > >  |
> > >  --- *rcu_utilization*
> > > |
> > > |--%96.23-- gic_handle_irq  # 204
> > > |
> > > |--%1.89-- SyS_nanosleep  # 4
> > > |
> > > |--%0.94-- SyS_exit_group  # 2
> > > |
> > > |--%0.94-- do_notify_resume  # 2
> > >
> > >   %4.86  (14181) arm_stb_user_lo  user_exit #206
> > >  |
> > >  --- *user_exit*
> > >   context_tracking_user_exit
> > >
> > >   %4.86  (14181) arm_stb_user_lo context_tracking_user_exit #206
> > >  |
> > >  --- context_tracking_user_exit
> > >
> > >   %4.86  (14181) arm_stb_user_locontext_tracking_user_enter #206
> > >  |
> > >  --- context_tracking_user_enter
> > >
> > >   %4.86  (14181) arm_stb_user_lo user_enter #206
> > >  |
> > >  --- *user_enter*
> > >   context_tracking_user_enter
> > >
> > >   %2.95  (14181) arm_stb_user_lo gic_handle_irq #125
> > >  |
> > >  --- gic_handle_irq
> > >
> > >
> > > On Tue, Jun 30, 2020 at 9:45 AM Flavio Leitner  wrote:
> > >
> > > > On Tue, Jun 02, 2020 at 12:56:51PM -0700, Vinay Gupta wrote:
> > > > > Hi Flavio,
> > > > >
> > > > > Thanks for your reply.
> > > > > I have captured the suggested information but do not see anything
> > that
> > > > > could cause the packet drops.
> > > > > Can you please take a look at the below data and see if you can find
> > > > > something unusual ?
> > > > > The PMDs are running on CPU 1,2,3,4 and CPU 1-7 are isolated cores.
> > > > >
> > > > >
> > > >
> > ---
> > > > > root@bcm958802a8046c:~# cstats ; sleep 10; cycles
> > > > > pmd thread numa_id 0 core_id 1:
> > > > >   idle cycles: 99140849 (7.93%)
> > > > >   processing cycles: 1151423715 (92.07%)
> > > > >   avg cycles per packet: 116.94 (1250564564/10693918)
> > > > >   avg processing cycles per packet: 107.67 (1151423715/10693918)
> > > > > pmd thread numa_id 0 core_id 2:
> > > > >   idle cycles: 118373662 (9.47%)
> > > > >   processing cycles: 1132193442 (90.53%)
> > > > >   avg cycles 

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

[Flavio Leitner]

On Wed, Jun 24, 2020 at 09:48:00AM -0400, Aaron Conole wrote:
> Currently, the channel handlers are polled globally.  On some
> systems, this causes a thundering herd issue where multiple
> handler threads become active, only to do no work and immediately
> sleep.
> 
> The approach here is to push the netlink socket channels to discreet
> handler threads to process, rather than polling on every thread.
> This will eliminate the need to wake multiple threads.
> 
> To check:
> 
>   ip netns add left
>   ip netns add right
>   ip link add center-left type veth peer name left0
>   ip link add center-right type veth peer name right0
>   ip link set left0 netns left
>   ip link set right0 netns right
>   ip link set center-left up
>   ip link set center-right up
>   ip netns exec left ip link set left0 up
>   ip netns exec left ip addr add 172.31.110.10/24 dev left0
>   ip netns exec right ip link set right0 up
>   ip netns exec right ip addr add 172.31.110.11/24 dev right0
> 
>   ovs-vsctl add-br br0
>   ovs-vsctl add-port br0 center-right
>   ovs-vsctl add-port br0 center-left
> 
>   # in one terminal
>   perf record -e sched:sched_wakeup,irq:softirq_entry -ag
> 
>   # in a separate terminal
>   ip netns exec left arping -I left0 -c 1 172.31.110.11
> 
>   # in the perf terminal after exiting
>   perf script
> 
> Look for the number of 'handler' threads which were made active.

I am surprised to find out that in 2020 and Linux doesn't provide
a reasonable solution for the thundering herd issue. I wonder how
many applications have this issue out there. It looks like the
devil in is the details.

Anyways, I think the previous model didn't work because the number
of attached ports is increasing and so is the number of cores. The
number of sockets escalates quickly and it had the packet re-ordering
issue.

Moving forward, we moved to one socket per port with all threads
polling the same socket in the hope that the kernel would only wake
one or very few threads. Looks like that doesn't work and the packet
re-ordering is still an issue.

Therefore this approach where there is one socket per port but only
thread polling would solve the thundering herd issue and the packet
re-ordering issue.

One concern is with load capacity, since now only one thread would
be responsible to process a port, but I think that's what we had
initially, correct? So, it's not an issue.

It looks like now it is allocating enough memory for all port_idx
per thread even though it will not use all of them. Perhaps that
can be fixed? For instance, instead of indexing the array with
port_idx, use handler->port_idx as an identifier? 

More inline

> Reported-by: David Ahern 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-December/365857.html
> Cc: Matteo Croce 
> Cc: Flavio Leitner 
> Signed-off-by: Aaron Conole 
> ---
> I haven't finished all my testing, but I wanted to send this
> for early feedback in case I went completely off course.
> 
>  lib/dpif-netlink.c | 190 ++---
>  1 file changed, 112 insertions(+), 78 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 1817e9f849..d59375cf00 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -179,9 +179,11 @@ struct dpif_windows_vport_sock {
>  
>  struct dpif_handler {
>  struct epoll_event *epoll_events;
> -int epoll_fd; /* epoll fd that includes channel socks. */
> -int n_events; /* Num events returned by epoll_wait(). */
> -int event_offset; /* Offset into 'epoll_events'. */
> +struct dpif_channel *channels; /* Array of channels for each port. */
> +size_t n_channels; /* Count of channels for each port. */
> +int epoll_fd;  /* epoll fd that includes channel socks. 
> */
> +int n_events;  /* Num events returned by epoll_wait(). */
> +int event_offset;  /* Offset into 'epoll_events'. */
>  
>  #ifdef _WIN32
>  /* Pool of sockets. */
> @@ -201,7 +203,6 @@ struct dpif_netlink {
>  struct fat_rwlock upcall_lock;
>  struct dpif_handler *handlers;
>  uint32_t n_handlers;   /* Num of upcall handlers. */
> -struct dpif_channel *channels; /* Array of channels for each port. */
>  int uc_array_size; /* Size of 'handler->channels' and */
> /* 'handler->epoll_events'. */
>  
> @@ -452,15 +453,22 @@ static bool
>  vport_get_pid(struct dpif_netlink *dpif, uint32_t port_idx,
>uint32_t *upcall_pid)
>  {
> -/* Since the nl_sock can only be assigned in either all
> - * or none "dpif" channels, the following check
> - * would suffice. */
> -if (!dpif->channels[port_idx].sock) {
> -return false;
> -}
> +size_t i;
>  ovs_assert(!WINDOWS || dpif->n_handlers <= 1);
>  
> -*upcall_pid = nl_sock_pid(dpif->channels[port_idx].sock);
> +/* The 

[ovs-dev] Dr Grace H Admas

[reward reward]

Congratulation!!!
Dear winner,

Please kindly view the mail attachment and contact the Claim Manager with the 
required details for payment Verification/Release. If this Notification Letter 
hits your Junk/Spam folder, simply move it from your Spam/Junk folder to your 
inbox for better viewing and easy accessibility.

Sincerely,

Dr Grace H Admas

COVID-19!/Chief Executive Promo Officer


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

Re: [ovs-dev] OVS needs to release new stable versions.

[Ilya Maximets]

On 6/30/20 12:57 AM, Ben Pfaff wrote:
> On Tue, Jun 30, 2020 at 12:43:48AM +0200, Ilya Maximets wrote:
>> On 6/29/20 10:47 PM, Ben Pfaff wrote:
>>> On Fri, Jun 26, 2020 at 12:18:37PM +0200, Ilya Maximets wrote:
 So, what is the proposed plan:

   1. We should add missed git tags to 2.11.3 and 2.11.4 releases.

  Ben could you, please, take care of this? (Alternatively, I could do 
 that,
  but I'm not sure what with the keys to sign tags and how this key
  management handled these days since the collapse of web of trust.)
>>>
>>> I think that it's probably best to take myself out off the critical path
>>> here.  I started out using signed tags because they were easy for me,
>>> since I already had a key in Debian's web of trust.  But I don't know of
>>> a reason why they need to be signed, or signed by a particular key.  I
>>> think it is perfectly reasonable if you push the tags, signed or
>>> unsigned.
>>
>> OK.  I'll do that.
> 
> Great.

I was mistaken, 2.11.4 has not been released yet, so we only need to tag 2.11.3.

I created the v2.11.3 tag, signed it and pushed to repository.  And now it
is available on the "Releases" tab on github.

> 
>>> I don't know what you mean by "the collapse of the web of trust".  Did I
>>> miss a memo?
>>
>> It was a reference to Certificate Spamming Attacks happened last year [1],
>> and some consequences like creating new implementations of key servers
>> (keys.openpgp.org) with some design changes in compare with SKS.
>>
>> [1] https://gist.github.com/rjhansen/67ab921ffb4084c865b3618d6955275f
> 
> Oh my.  I did see something about that at the time, I think.  Ugh.
> 
   3. Prepare patches for OVS stable releases for all branches starting from
  branch-2.5 and apply them.  Create, sign and push tags.

  Ben, I know you handled this part last time.  What is the preferred 
 way?
  Someone else could prepare patches and send for review, I guess.
>>>
>>> I think we should designate a newer "long term stable" release now that
>>> OVN has released separately.
>>
>> Yeah.  That's a good point.  Maybe 2.13 is a good candidate for a new LTS?
> 
> I hope so?  I'd really rather not be in the middle of that decision.

For me 2.13 looks good.  But, I guess, the easiest way to decide is to create
a patch and discuss it on a dev mailing list.  I'll send one.

> 
>>> That aside, I have never prepared separate patches for OVS stable
>>> releases.  When I apply bug fixes, I backport them as far as necessary.
>>> It would be a burden to figure this out retrospectively, but it's
>>> usually easy at the time of bug fix.
>>
>> By "Prepare patches for OVS stable releases" I meant patches like this:
>>* 2ff0b7715 2019-09-06 | Prepare for 2.10.5.
>>* 5f19eaaf2 2019-09-06 | Set release date for 2.10.4. (tag: v2.10.4)
> 
> Oh!  Got it.  Justin used to do most of that work, so I didn't think
> about it.  My part of the job was, typically, acking the patches and
> pushing the tags (I did the latter because I was the one with the GPG
> key).
> 
> I suspect that a lot of it could be automated with a script (which would
> probably be more reliable than doing it manually, which I *think* Justin
> probably did).
> 
>> But yes, I agree that looking through patches and backport them selectively
>> is too much work and will likely require even more people.  Backporting
>> patches as far as necessary is a good practice. I do that too.
> 
> Wonderful!  I was sure hoping that there was not a big backlog of
> patches that might need backporting, since it's much more difficult to
> do it that way in my experience.
> 
>> We could do selective backports by request in case something missed, but
>> we should not revisit all the patches while preparing stable releases on
>> older branches. 
> 
> Yes.
> 
   4. Update relevant docs outside of openvswitch main repository (web-site,
  wiki, etc.)


 P.S.: We should, probably, do some periodic checks on released
 branches and make stable releases a bit more frequently.  For example,
 we could make regular stable release every few months in case we have
 fixes on top of the previous release.  I could keep monitoring things
 and ping people, but this might be better to have some plan, I think.
>>>
>>> Yes, probably.  At one point Justin and I were definitely the people who
>>> coordinated this, but now if it is left to us then it probably won't get
>>> done, or not done often enough.
>>>
>>> I really need some new people to take the leadership role here.
>>>
>>
>> I could take care of this.  I mean, I could periodically look at numbers
>> of patches on stable branches and prepare minor releases if needed.
> 
> That would be fantastic.
> 

OK.  So, in this case point 3 of my proposed plan is on me. :)

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org

[ovs-dev] Google Anniversary

[maymie]

Your e-mail address winning details:
Winning No: GUK/6736006529/2020


Congratulations on your MEGA WIN,kindly fill the attached form to
activate your payment.

Once again,Congratulations.

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

[ovs-dev] I need Your Help about my Father`s $3.6 million usd

[Janet Aringo]

I beleive that this message will meet you in good health.

My Name is Janet Aringo, am the only child of Hon.Peter Oloo Aringo, the
former minister of education in Uganda, my father died on april 2nd 2020
through coronavirus.

This message is for you alone and i want your trust and understanding, am
23 years of age, my late father Hon. Peter Oloo Aringo have total $3.6
million usd deposited at NCBA BANK UGANDA, according to my father`s bank
statement of account, he was planning to invest the $3.6 million usd
outside Africa before his death through this deadly pandemic called
Coronavirus, all i need is your trust and understanding because if you
help me you will benefit alot in my father`s money and i want this to be
between you and i which means you need to keep it as secret.

My Father`s accountant contacted me few days ago and told me that
according to the agreement my father has with the NCBA Bank, the fund
suppose to transfer out of the NCBA bank latest by end of this month july
if not the Uganda government will claim it which i don`t want it to
happen.


My father`s accountant told me this morning that the $3.6 million usd will
enter dormant account as unclaimed fund by the end of june,so i need to
transfer it out of Africa urgently before the wicked government claim it
next month, i have been crying since 3 days now because i don`t have
anybody in Abroad because am the only child of my late father, i got your
email through google search and i will give you 40 % of the total $3.6
million usd which will be $1.440,000,00 usd(  One Million Four Hundred and
Forty Thousand United States Dollars, if you stand as the next of kin of
my late father.

Please reply me on this email if you are interested and keep this as
secret ok.

Email me here for more communication and direction
janetari...@outlook.com

Miss Janet Aringo.
Daughter to late Peter Oloo Aringo.


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

[ovs-dev] Google Anniversary

[maymie]

Your e-mail address winning details:
Winning No: GUK/6736006529/2020


Congratulations on your MEGA WIN,kindly fill the attached form to
activate your payment.

Once again,Congratulations.

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

[ovs-dev] [PATCH v1 ovn] Fix seg fault while encoding DHCP domain search option.

[svc.eng.git-pa...@nutanix.com]

From: Dhathri Purohith 

Some versions of strtok_r make the original string NULL at the
end of parsing. Adding Null check before ovs_strlcpy() to
prevent segfault in such cases.

Fixes: d79bb92c4b49 ("Add support for DHCP domain search option (119)")

Signed-off-by: Dhathri Purohith 
---
 lib/actions.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/actions.c b/lib/actions.c
index ee7c825..c435188 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -2382,7 +2382,9 @@ encode_put_dhcpv4_option(const struct ovnact_gen_option 
*o,
 memcpy(dns_encoded + encode_offset, label, len);
 encode_offset += len;
 }
-   ovs_strlcpy(suffix, domain, strlen(domain));
+if (domain != NULL) {
+ovs_strlcpy(suffix, domain, strlen(domain));
+}
 }
 /* Add the end marker (0 byte) to determine the end of the
  * domain. */
-- 
1.8.3.1

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

Re: [ovs-dev] 10-25 packet drops every few (10-50) seconds TCP (iperf3)

[Shahaji Bhosle via dev]

Thanks Flavio,
Are there any special requirements for RCU on ARM vs x86.

I am following what the above document is saying...Do you think I need to
do something more than the below?
Thanks again and appreciate the help. Shahaji

1. Isolate the CPU cores
*isolcpus=1,2,3,4,5,6,7 nohz_full=1-7 rcu_nocbs=1-7*
2. Setting CONFIG_NO_HZ_FULL=y
root@bcm958802a8046c:~/vinay_rx/dynticks-testing# zcat /proc/config.gz
|grep HZ
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
# CONFIG_NO_HZ_IDLE is not set
*CONFIG_NO_HZ_FULL*=y
# CONFIG_NO_HZ_FULL_ALL is not set
# CONFIG_NO_HZ is not set
# CONFIG_HZ_100 is not set
CONFIG_HZ_250=y
# CONFIG_HZ_300 is not set
# CONFIG_HZ_1000 is not set
CONFIG_HZ=250



On Tue, Jun 30, 2020 at 12:50 PM Flavio Leitner  wrote:

>
> Right, you might want to review Documentation/timers/no_hz.rst from
> the kernel sources and look for RCU implications section where
> it explains how to move RCU callbacks.
>
> fbl
>
> On Tue, Jun 30, 2020 at 12:08:05PM -0400, Shahaji Bhosle wrote:
> > Hi Flavio,
> > I wrote a small program which has do_nothing for loop and I measure the
> > timestamps across the do nothing loop. I am seeing 3% of the time around
> > the 1 second mark when the arch_timer fires I get the timestamps to be
> off
> > by 25% of the exprected value. I ran trace-cmd to see what is going on
> and
> > see the below. Looks like some issue with *gic_handle_irg*(), not seeing
> > tihs behaviour on x86 host, something special with ARM v8.
> > Thanks, Shahaji
> >
> >   %21.77  (14181) arm_stb_user_lorcu_dyntick #922
> >  |
> >  --- *rcu_dyntick*
> > |
> > |--%46.85-- gic_handle_irq  # 432
> > |
> > |--%23.32-- context_tracking_user_exit  # 215
> > |
> > |--%22.34-- context_tracking_user_enter  # 206
> > |
> > |--%2.60-- SyS_execve  # 24
> > |
> > |--%1.30-- do_page_fault  # 12
> > |
> > |--%0.65-- SyS_write  # 6
> > |
> > |--%0.65-- schedule  # 6
> > |
> > |--%0.65-- SyS_nanosleep  # 6
> > |
> > |--%0.65-- syscall_trace_enter  # 6
> > |
> > |--%0.65-- SyS_faccessat  # 6
> >
> >   %5.01  (14181) arm_stb_user_lorcu_utilization #212
> >  |
> >  --- *rcu_utilization*
> > |
> > |--%96.23-- gic_handle_irq  # 204
> > |
> > |--%1.89-- SyS_nanosleep  # 4
> > |
> > |--%0.94-- SyS_exit_group  # 2
> > |
> > |--%0.94-- do_notify_resume  # 2
> >
> >   %4.86  (14181) arm_stb_user_lo  user_exit #206
> >  |
> >  --- *user_exit*
> >   context_tracking_user_exit
> >
> >   %4.86  (14181) arm_stb_user_lo context_tracking_user_exit #206
> >  |
> >  --- context_tracking_user_exit
> >
> >   %4.86  (14181) arm_stb_user_locontext_tracking_user_enter #206
> >  |
> >  --- context_tracking_user_enter
> >
> >   %4.86  (14181) arm_stb_user_lo user_enter #206
> >  |
> >  --- *user_enter*
> >   context_tracking_user_enter
> >
> >   %2.95  (14181) arm_stb_user_lo gic_handle_irq #125
> >  |
> >  --- gic_handle_irq
> >
> >
> > On Tue, Jun 30, 2020 at 9:45 AM Flavio Leitner  wrote:
> >
> > > On Tue, Jun 02, 2020 at 12:56:51PM -0700, Vinay Gupta wrote:
> > > > Hi Flavio,
> > > >
> > > > Thanks for your reply.
> > > > I have captured the suggested information but do not see anything
> that
> > > > could cause the packet drops.
> > > > Can you please take a look at the below data and see if you can find
> > > > something unusual ?
> > > > The PMDs are running on CPU 1,2,3,4 and CPU 1-7 are isolated cores.
> > > >
> > > >
> > >
> ---
> > > > root@bcm958802a8046c:~# cstats ; sleep 10; cycles
> > > > pmd thread numa_id 0 core_id 1:
> > > >   idle cycles: 99140849 (7.93%)
> > > >   processing cycles: 1151423715 (92.07%)
> > > >   avg cycles per packet: 116.94 (1250564564/10693918)
> > > >   avg processing cycles per packet: 107.67 (1151423715/10693918)
> > > > pmd thread numa_id 0 core_id 2:
> > > >   idle cycles: 118373662 (9.47%)
> > > >   processing cycles: 1132193442 (90.53%)
> > > >   avg cycles per packet: 124.39 (1250567104/10053309)
> > > >   avg processing cycles per packet: 112.62 (1132193442/10053309)
> > > > pmd thread numa_id 0 core_id 3:
> > > >   idle cycles: 53805933 (4.30%)
> > > >   processing cycles: 1196762002 (95.70%)
> > > >   avg cycles per packet: 107.35 (1250567935/11649948)
> > > >   avg processing cycles per packet: 102.73 (1196762002/11649948)
> > > 

Re: [ovs-dev] 10-25 packet drops every few (10-50) seconds TCP (iperf3)

[Flavio Leitner]

Right, you might want to review Documentation/timers/no_hz.rst from
the kernel sources and look for RCU implications section where
it explains how to move RCU callbacks.

fbl

On Tue, Jun 30, 2020 at 12:08:05PM -0400, Shahaji Bhosle wrote:
> Hi Flavio,
> I wrote a small program which has do_nothing for loop and I measure the
> timestamps across the do nothing loop. I am seeing 3% of the time around
> the 1 second mark when the arch_timer fires I get the timestamps to be off
> by 25% of the exprected value. I ran trace-cmd to see what is going on and
> see the below. Looks like some issue with *gic_handle_irg*(), not seeing
> tihs behaviour on x86 host, something special with ARM v8.
> Thanks, Shahaji
> 
>   %21.77  (14181) arm_stb_user_lorcu_dyntick #922
>  |
>  --- *rcu_dyntick*
> |
> |--%46.85-- gic_handle_irq  # 432
> |
> |--%23.32-- context_tracking_user_exit  # 215
> |
> |--%22.34-- context_tracking_user_enter  # 206
> |
> |--%2.60-- SyS_execve  # 24
> |
> |--%1.30-- do_page_fault  # 12
> |
> |--%0.65-- SyS_write  # 6
> |
> |--%0.65-- schedule  # 6
> |
> |--%0.65-- SyS_nanosleep  # 6
> |
> |--%0.65-- syscall_trace_enter  # 6
> |
> |--%0.65-- SyS_faccessat  # 6
> 
>   %5.01  (14181) arm_stb_user_lorcu_utilization #212
>  |
>  --- *rcu_utilization*
> |
> |--%96.23-- gic_handle_irq  # 204
> |
> |--%1.89-- SyS_nanosleep  # 4
> |
> |--%0.94-- SyS_exit_group  # 2
> |
> |--%0.94-- do_notify_resume  # 2
> 
>   %4.86  (14181) arm_stb_user_lo  user_exit #206
>  |
>  --- *user_exit*
>   context_tracking_user_exit
> 
>   %4.86  (14181) arm_stb_user_lo context_tracking_user_exit #206
>  |
>  --- context_tracking_user_exit
> 
>   %4.86  (14181) arm_stb_user_locontext_tracking_user_enter #206
>  |
>  --- context_tracking_user_enter
> 
>   %4.86  (14181) arm_stb_user_lo user_enter #206
>  |
>  --- *user_enter*
>   context_tracking_user_enter
> 
>   %2.95  (14181) arm_stb_user_lo gic_handle_irq #125
>  |
>  --- gic_handle_irq
> 
> 
> On Tue, Jun 30, 2020 at 9:45 AM Flavio Leitner  wrote:
> 
> > On Tue, Jun 02, 2020 at 12:56:51PM -0700, Vinay Gupta wrote:
> > > Hi Flavio,
> > >
> > > Thanks for your reply.
> > > I have captured the suggested information but do not see anything that
> > > could cause the packet drops.
> > > Can you please take a look at the below data and see if you can find
> > > something unusual ?
> > > The PMDs are running on CPU 1,2,3,4 and CPU 1-7 are isolated cores.
> > >
> > >
> > ---
> > > root@bcm958802a8046c:~# cstats ; sleep 10; cycles
> > > pmd thread numa_id 0 core_id 1:
> > >   idle cycles: 99140849 (7.93%)
> > >   processing cycles: 1151423715 (92.07%)
> > >   avg cycles per packet: 116.94 (1250564564/10693918)
> > >   avg processing cycles per packet: 107.67 (1151423715/10693918)
> > > pmd thread numa_id 0 core_id 2:
> > >   idle cycles: 118373662 (9.47%)
> > >   processing cycles: 1132193442 (90.53%)
> > >   avg cycles per packet: 124.39 (1250567104/10053309)
> > >   avg processing cycles per packet: 112.62 (1132193442/10053309)
> > > pmd thread numa_id 0 core_id 3:
> > >   idle cycles: 53805933 (4.30%)
> > >   processing cycles: 1196762002 (95.70%)
> > >   avg cycles per packet: 107.35 (1250567935/11649948)
> > >   avg processing cycles per packet: 102.73 (1196762002/11649948)
> > > pmd thread numa_id 0 core_id 4:
> > >   idle cycles: 189102938 (15.12%)
> > >   processing cycles: 1061463293 (84.88%)
> > >   avg cycles per packet: 143.47 (1250566231/8716828)
> > >   avg processing cycles per packet: 121.77 (1061463293/8716828)
> > > pmd thread numa_id 0 core_id 5:
> > > pmd thread numa_id 0 core_id 6:
> > > pmd thread numa_id 0 core_id 7:
> >
> >
> > The core_id 3 is high loaded, and then it's more likely to show
> > the drop issue when some other event happens.
> >
> > I think you need to run perf as I recommended before and see if
> > there are context switches happening and why they are happening.
> >
> > If a context switch happens, it's either because the core is not
> > well isolated or some other thing is going on. It will help to
> > understand why the queue wasn't serviced for a certain amount of
> > time.
> >
> > The issue is that running perf might introduce some load, so you
> > will need adjust the traffic rate 

Re: [ovs-dev] [PATCH] lib/tc: only update the stats for non-empty counter

[Simon Horman]

On Mon, Jun 29, 2020 at 05:31:18PM +0800, we...@ucloud.cn wrote:
> From: wenxu 
> 
> A packet with first frag and execute act_ct action.
> The packet will stole by defrag. So the stats counter
> for "gact action goto chain" will always 0. The openvswitch
> update each action in order. So the flower stats finally
> alway be zero. The rule will be delete adter max-idle time
> even there are packet executing the action.
> 
> ovs-appctl dpctl/dump-flows
> recirc_id(0),in_port(1),eth_type(0x0800),ipv4(dst=11.0.0.7,frag=first), 
> packets:0, bytes:0, used:5.390s, actions:ct(zone=1,nat),recirc(0x4)
> 
> filter protocol ip pref 2 flower chain 0 handle 0x2
>   eth_type ipv4
>   dst_ip 1.1.1.1
>   ip_flags frag/firstfrag
>   skip_hw
>   not_in_hw
>  action order 1: ct zone 1 nat pipe
>   index 2 ref 1 bind 1 installed 11 sec used 1 sec
>  Action statistics:
>  Sent 15000 bytes 11 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>  cookie e04106c2ac41769b278edaa9b5309960
> 
>  action order 2: gact action goto chain 1
>   random type none pass val 0
>   index 2 ref 1 bind 1 installed 11 sec used 11 sec
>  Action statistics:
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>  cookie e04106c2ac41769b278edaa9b5309960
> 
> Signed-off-by: wenxu 

Thanks,

this patch looks good to me.

I've added it to Travis CI for it to check with a view to pushing this
change.

https://travis-ci.org/github/horms2/ovs/builds/703600018
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-offload-tc: Add drop action support.

[William Tu]

On Tue, Jun 30, 2020 at 9:13 AM Simon Horman  wrote:
>
> On Tue, Jun 30, 2020 at 09:06:40AM -0700, William Tu wrote:
> > On Tue, Jun 30, 2020 at 9:01 AM Simon Horman  
> > wrote:
> > >
> > > On Tue, Jun 30, 2020 at 08:30:45AM -0700, William Tu wrote:
> > > > Currently drop action is not offloaded when using userspace datapath
> > > > with tc offload.  The patch programs tc gact (generic action) chain
> > > > ID 0 to drop the packet by setting it to TC_ACT_SHOT.
> > > >
> > > > Example:
> > > > $ ovs-appctl dpctl/add-flow netdev@ovs-netdev \
> > > >   'recirc_id(0),in_port(2),eth(),eth_type(0x0806),\
> > > >   arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)' drop
> > > >
> > > > $ tc filter show dev ovs-p0 ingress
> > > > filter protocol arp pref 2 flower chain 0
> > > > filter protocol arp pref 2 flower chain 0 handle 0x1
> > > >   eth_type arp
> > > >   arp_tip 10.255.1.116
> > > >   arp_op reply
> > > >   arp_tha 00:50:56:e1:4b:ab
> > > >   skip_hw
> > > >   not_in_hw
> > > >   action order 1: gact action drop
> > > > ...
> > > >
> > > > Signed-off-by: William Tu 
> > >
> > > Hi William,
> > >
> > > this change looks good to me other than that I'm unsure
> > > why we need #ifndef __KERNEL__.
> > >
> > Because for kernel datapath, we don't define
> > OVS_ACTION_ATTR_DROP
> >
> > at datapath/linux/compat/include/linux/openvswitch.h
> > ...
> > #ifndef __KERNEL__
> > OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
> > OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
> > OVS_ACTION_ATTR_DROP,  /* u32 xlate_error. */
> > OVS_ACTION_ATTR_LB_OUTPUT, /* u32 bond-id. */
> > #endif
>
> I feel that I'm missing something terribly obvious, but it seems to me that
> lib/netdev-offload-tc.c is only ever compiled as user-space code (its not
> compiled into the kernel datapath) and thus __KERNEL__ would never be set.

I see, so probably checking __KERNEL__ here is not necessary. Will remove it.

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

Re: [ovs-dev] [PATCH] netdev-offload-tc: Add drop action support.

[Simon Horman]

On Tue, Jun 30, 2020 at 09:06:40AM -0700, William Tu wrote:
> On Tue, Jun 30, 2020 at 9:01 AM Simon Horman  
> wrote:
> >
> > On Tue, Jun 30, 2020 at 08:30:45AM -0700, William Tu wrote:
> > > Currently drop action is not offloaded when using userspace datapath
> > > with tc offload.  The patch programs tc gact (generic action) chain
> > > ID 0 to drop the packet by setting it to TC_ACT_SHOT.
> > >
> > > Example:
> > > $ ovs-appctl dpctl/add-flow netdev@ovs-netdev \
> > >   'recirc_id(0),in_port(2),eth(),eth_type(0x0806),\
> > >   arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)' drop
> > >
> > > $ tc filter show dev ovs-p0 ingress
> > > filter protocol arp pref 2 flower chain 0
> > > filter protocol arp pref 2 flower chain 0 handle 0x1
> > >   eth_type arp
> > >   arp_tip 10.255.1.116
> > >   arp_op reply
> > >   arp_tha 00:50:56:e1:4b:ab
> > >   skip_hw
> > >   not_in_hw
> > >   action order 1: gact action drop
> > > ...
> > >
> > > Signed-off-by: William Tu 
> >
> > Hi William,
> >
> > this change looks good to me other than that I'm unsure
> > why we need #ifndef __KERNEL__.
> >
> Because for kernel datapath, we don't define
> OVS_ACTION_ATTR_DROP
> 
> at datapath/linux/compat/include/linux/openvswitch.h
> ...
> #ifndef __KERNEL__
> OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
> OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
> OVS_ACTION_ATTR_DROP,  /* u32 xlate_error. */
> OVS_ACTION_ATTR_LB_OUTPUT, /* u32 bond-id. */
> #endif

I feel that I'm missing something terribly obvious, but it seems to me that
lib/netdev-offload-tc.c is only ever compiled as user-space code (its not
compiled into the kernel datapath) and thus __KERNEL__ would never be set.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 10-25 packet drops every few (10-50) seconds TCP (iperf3)

[Shahaji Bhosle via dev]

Hi Flavio,
I wrote a small program which has do_nothing for loop and I measure the
timestamps across the do nothing loop. I am seeing 3% of the time around
the 1 second mark when the arch_timer fires I get the timestamps to be off
by 25% of the exprected value. I ran trace-cmd to see what is going on and
see the below. Looks like some issue with *gic_handle_irg*(), not seeing
tihs behaviour on x86 host, something special with ARM v8.
Thanks, Shahaji

  %21.77  (14181) arm_stb_user_lorcu_dyntick #922
 |
 --- *rcu_dyntick*
|
|--%46.85-- gic_handle_irq  # 432
|
|--%23.32-- context_tracking_user_exit  # 215
|
|--%22.34-- context_tracking_user_enter  # 206
|
|--%2.60-- SyS_execve  # 24
|
|--%1.30-- do_page_fault  # 12
|
|--%0.65-- SyS_write  # 6
|
|--%0.65-- schedule  # 6
|
|--%0.65-- SyS_nanosleep  # 6
|
|--%0.65-- syscall_trace_enter  # 6
|
|--%0.65-- SyS_faccessat  # 6

  %5.01  (14181) arm_stb_user_lorcu_utilization #212
 |
 --- *rcu_utilization*
|
|--%96.23-- gic_handle_irq  # 204
|
|--%1.89-- SyS_nanosleep  # 4
|
|--%0.94-- SyS_exit_group  # 2
|
|--%0.94-- do_notify_resume  # 2

  %4.86  (14181) arm_stb_user_lo  user_exit #206
 |
 --- *user_exit*
  context_tracking_user_exit

  %4.86  (14181) arm_stb_user_lo context_tracking_user_exit #206
 |
 --- context_tracking_user_exit

  %4.86  (14181) arm_stb_user_locontext_tracking_user_enter #206
 |
 --- context_tracking_user_enter

  %4.86  (14181) arm_stb_user_lo user_enter #206
 |
 --- *user_enter*
  context_tracking_user_enter

  %2.95  (14181) arm_stb_user_lo gic_handle_irq #125
 |
 --- gic_handle_irq


On Tue, Jun 30, 2020 at 9:45 AM Flavio Leitner  wrote:

> On Tue, Jun 02, 2020 at 12:56:51PM -0700, Vinay Gupta wrote:
> > Hi Flavio,
> >
> > Thanks for your reply.
> > I have captured the suggested information but do not see anything that
> > could cause the packet drops.
> > Can you please take a look at the below data and see if you can find
> > something unusual ?
> > The PMDs are running on CPU 1,2,3,4 and CPU 1-7 are isolated cores.
> >
> >
> ---
> > root@bcm958802a8046c:~# cstats ; sleep 10; cycles
> > pmd thread numa_id 0 core_id 1:
> >   idle cycles: 99140849 (7.93%)
> >   processing cycles: 1151423715 (92.07%)
> >   avg cycles per packet: 116.94 (1250564564/10693918)
> >   avg processing cycles per packet: 107.67 (1151423715/10693918)
> > pmd thread numa_id 0 core_id 2:
> >   idle cycles: 118373662 (9.47%)
> >   processing cycles: 1132193442 (90.53%)
> >   avg cycles per packet: 124.39 (1250567104/10053309)
> >   avg processing cycles per packet: 112.62 (1132193442/10053309)
> > pmd thread numa_id 0 core_id 3:
> >   idle cycles: 53805933 (4.30%)
> >   processing cycles: 1196762002 (95.70%)
> >   avg cycles per packet: 107.35 (1250567935/11649948)
> >   avg processing cycles per packet: 102.73 (1196762002/11649948)
> > pmd thread numa_id 0 core_id 4:
> >   idle cycles: 189102938 (15.12%)
> >   processing cycles: 1061463293 (84.88%)
> >   avg cycles per packet: 143.47 (1250566231/8716828)
> >   avg processing cycles per packet: 121.77 (1061463293/8716828)
> > pmd thread numa_id 0 core_id 5:
> > pmd thread numa_id 0 core_id 6:
> > pmd thread numa_id 0 core_id 7:
>
>
> The core_id 3 is high loaded, and then it's more likely to show
> the drop issue when some other event happens.
>
> I think you need to run perf as I recommended before and see if
> there are context switches happening and why they are happening.
>
> If a context switch happens, it's either because the core is not
> well isolated or some other thing is going on. It will help to
> understand why the queue wasn't serviced for a certain amount of
> time.
>
> The issue is that running perf might introduce some load, so you
> will need adjust the traffic rate accordingly.
>
> HTH,
> fbl
>
>
>
> >
> >
> > *Runtime summary*  comm  parent   sched-in
> > run-timemin-run avg-run max-run  stddev  migrations
> >   (count)   (msec) (msec)
> >(msec)  (msec)   %
> >
> -
> > ksoftirqd/0[7]   2  10.079  

Re: [ovs-dev] [PATCH] netdev-offload-tc: Add drop action support.

[William Tu]

On Tue, Jun 30, 2020 at 9:01 AM Simon Horman  wrote:
>
> On Tue, Jun 30, 2020 at 08:30:45AM -0700, William Tu wrote:
> > Currently drop action is not offloaded when using userspace datapath
> > with tc offload.  The patch programs tc gact (generic action) chain
> > ID 0 to drop the packet by setting it to TC_ACT_SHOT.
> >
> > Example:
> > $ ovs-appctl dpctl/add-flow netdev@ovs-netdev \
> >   'recirc_id(0),in_port(2),eth(),eth_type(0x0806),\
> >   arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)' drop
> >
> > $ tc filter show dev ovs-p0 ingress
> > filter protocol arp pref 2 flower chain 0
> > filter protocol arp pref 2 flower chain 0 handle 0x1
> >   eth_type arp
> >   arp_tip 10.255.1.116
> >   arp_op reply
> >   arp_tha 00:50:56:e1:4b:ab
> >   skip_hw
> >   not_in_hw
> >   action order 1: gact action drop
> > ...
> >
> > Signed-off-by: William Tu 
>
> Hi William,
>
> this change looks good to me other than that I'm unsure
> why we need #ifndef __KERNEL__.
>
Because for kernel datapath, we don't define
OVS_ACTION_ATTR_DROP

at datapath/linux/compat/include/linux/openvswitch.h
...
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
OVS_ACTION_ATTR_DROP,  /* u32 xlate_error. */
OVS_ACTION_ATTR_LB_OUTPUT, /* u32 bond-id. */
#endif

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

Re: [ovs-dev] [PATCH] netdev-offload-tc: Add drop action support.

[Simon Horman]

On Tue, Jun 30, 2020 at 08:30:45AM -0700, William Tu wrote:
> Currently drop action is not offloaded when using userspace datapath
> with tc offload.  The patch programs tc gact (generic action) chain
> ID 0 to drop the packet by setting it to TC_ACT_SHOT.
> 
> Example:
> $ ovs-appctl dpctl/add-flow netdev@ovs-netdev \
>   'recirc_id(0),in_port(2),eth(),eth_type(0x0806),\
>   arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)' drop
> 
> $ tc filter show dev ovs-p0 ingress
> filter protocol arp pref 2 flower chain 0
> filter protocol arp pref 2 flower chain 0 handle 0x1
>   eth_type arp
>   arp_tip 10.255.1.116
>   arp_op reply
>   arp_tha 00:50:56:e1:4b:ab
>   skip_hw
>   not_in_hw
>   action order 1: gact action drop
> ...
> 
> Signed-off-by: William Tu 

Hi William,

this change looks good to me other than that I'm unsure
why we need #ifndef __KERNEL__.

> ---
>  lib/netdev-offload-tc.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 258d31f54b08..8c6bcc0aa2c7 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1788,6 +1788,12 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
> *match,
>  action->chain = nl_attr_get_u32(nla);
>  flower.action_count++;
>  recirc_act = true;
> +#ifndef __KERNEL__
> +} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_DROP) {
> +action->type = TC_ACT_GOTO;
> +action->chain = 0;  /* 0 is reserved and not used by recirc. */
> +flower.action_count++;
> +#endif
>  } else {
>  VLOG_DBG_RL(, "unsupported put action type: %d",
>  nl_attr_type(nla));
> -- 
> 2.7.4
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] WAIT FOR YOUR RESPONSE

[ibrahim dioufb]

Dear Friend

I have written to you severally without receiving any response from
you,please let me know if you have received the attached document with
the details that I sent to you.

Thank you
Best Regards
Mr.Kojo Mattah
Email:cojomat...@gmail.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev v2] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

[Alin Serdean]

From: Jinjun Gao
Sent: Tuesday, June 30, 2020 2:45 PM
To: d...@openvswitch.org; Alin 
Serdean
Cc: kumaran...@vmware.com; Jinjun 
Gao
Subject: [ovs-dev v2] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

Add helper and master if existing to a conntrack entry:
1, For CTA_HELP, only support FTP/TFTP;
2, For CTA_TUPLE_MASTER, only support FTP.

Signed-off-by: Jinjun Gao 
---
v2:
  1, Add CTA_TUPLE_MASTER support in CT event subscribe/report system.
  2, Release CT entry lock in fail path.
  3, Add more comments.
---
Thank you for addressing the comments, applied on master!

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

[ovs-dev] The most effective gutter protection - Clog Free Forever

[Chloe Edwards]

LeafFilter Gutter Protection | End Gutter Cleaning Foreverwww. leaffilter. com
Eliminate gutter cleaning forever with LeafFilter, the most advanced 
debris-blocking gutter protection.  Schedule a FREE LeafFilter estimate today. 


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

Re: [ovs-dev] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

[Jinjun Gao]

Thanks for review, Alin.

Please see the comments in inline [Jinjun].

--
Jinjun


From: Alin Serdean 
Date: Tuesday, June 30, 2020 at 5:34 PM
To: Jinjun Gao , "d...@openvswitch.org" 

Cc: Anand Kumar 
Subject: RE: [ovs-dev] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

Thanks for sending the patch.

Running the command:
make datapath_windows_analyze
resulted in the following:
ovsext\Conntrack.c(873): warning C28167: The function 'OvsCtExecute_' changes 
the IRQL and does not
restore the IRQL before it exits. It should be annotated to reflect the change 
or the IRQL should
be restored. IRQL was last set to 2 at line 955.
[datapath-windows\ovsext\ovsext.vcxproj]

There is a lock acquired but it is not released on the failure path.

You can fold in following incremental:

[Jinjun]: Will add it, thanks.

$ git diff datapath-windows/ovsext/Conntrack.c
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 414b3f4b2..717d04f49 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -974,6 +974,7 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
   OVS_CT_POOL_TAG);
 if (!entry->helper_name) {
 OVS_LOG_ERROR("Error while allocating memory");
+OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
 return NDIS_STATUS_RESOURCES;
 }

The rest looks good, but I have the following question:

 /* FTP ACTIVE - Server initiates the connection */
 if ((incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
-(incomingKey.src.port == entryKey.src.port) &&
Why is the line above removed?
 [Jinjun]: I used pyftpdlib as ftp server to test this patch. Pyftpdlib doesn’t 
use 20 as data port and it chooses a random port as data port in active mode. 
It causes the incomingKey cannot match any entry in related table. In this 
case, the data flow cannot find control flow and add it as tuple master. After 
deleting above line, it works! If we don’t consider to support random data port 
in active mode, we need not to remove above line. It should be better to 
support random data port case in active mode. How do you think about it? If you 
agree, I can add some comments here to clear it.

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

Re: [ovs-dev] [PATCH ovn v2] Fix selection fields for UDP and SCTP load balancers.

[Numan Siddique]

On Tue, Jun 30, 2020 at 8:19 PM Mark Michelson  wrote:

> Acked-by: Mark Michelson 
>
>
Thanks for the review.

I applied this patch to master and branch-20.06.

Numan


> On 6/30/20 1:21 AM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > The commit 5af304e7478a ("Support selection fields in load balancer.")
> > didn't handle the selection fields for UDP and SCTP protocol.
> > If CMS has set the selection fields - tp_src and tp_dst for UDP or SCTP
> > load balancers, ovn-northd was adding lflows as
> > ct_lb(backends=, hash_fields(tp_src,tp_dst))
> > instead of ct_lb(backends=, hash_fields(udp_src,udp_dst)) and
> > ct_lb(backends=, hash_fields(sctp_src,sctp_dst)) respectively.
> >
> > Because of this, select group flows were encoded with tcp_src and tcp_dst
> > hash fields.
> >
> > This patch fixes this issue.
> >
> > Test cases are now added for UDP and SCTP protocols.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1846189
> > Signed-off-by: Numan Siddique 
> > ---
> >
> > v1 -> v2
> > 
> >* Addressed the concerns raised by Mark.
> >  Removed the refactoring of the code and the load balancer
> >  protocol is not stored in the internal structure.
> >
> >   northd/ovn-northd.c |  14 +++-
> >   tests/ovn.at| 179 ++--
> >   tests/system-ovn.at |  23 ++
> >   3 files changed, 207 insertions(+), 9 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 2f973bff6e..9da4b5b860 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -3351,10 +3351,22 @@ ovn_lb_create(struct northd_context *ctx, struct
> hmap *lbs,
> >   n_vips++;
> >   }
> >
> > +char *proto = NULL;
> > +if (nbrec_lb->protocol && nbrec_lb->protocol[0]) {
> > +proto = nbrec_lb->protocol;
> > +}
> > +
> >   if (lb->nlb->n_selection_fields) {
> >   struct ds sel_fields = DS_EMPTY_INITIALIZER;
> >   for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) {
> > -ds_put_format(_fields, "%s,",
> lb->nlb->selection_fields[i]);
> > +char *field = lb->nlb->selection_fields[i];
> > +if (!strcmp(field, "tp_src") && proto) {
> > +ds_put_format(_fields, "%s_src,", proto);
> > +} else if (!strcmp(field, "tp_dst") && proto) {
> > +ds_put_format(_fields, "%s_dst,", proto);
> > +} else {
> > +ds_put_format(_fields, "%s,", field);
> > +}
> >   }
> >   ds_chomp(_fields, ',');
> >   lb->selection_fields = ds_steal_cstr(_fields);
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 6a9bdf1d56..1a1cfbf124 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -1029,6 +1029,18 @@ ct_lb(backends=fd0f::2,fd0f::3;
> hash_fields="eth_src,eth_dst,ip_src,ip_dst,tp_sr
> >   encodes as group:5
> >   uses group: id(5),
> name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,tp_src,tp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
> >   has prereqs ip
> > +ct_lb(backends=fd0f::2,fd0f::3;
> hash_fields="eth_src,eth_dst,ip_src,ip_dst,tcp_src,tcp_dst");
> > +encodes as group:6
> > +uses group: id(6),
> name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,tcp_src,tcp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
> > +has prereqs ip
> > +ct_lb(backends=fd0f::2,fd0f::3;
> hash_fields="eth_src,eth_dst,ip_src,ip_dst,udp_src,udp_dst");
> > +encodes as group:7
> > +uses group: id(7),
> name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,udp_src,udp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
> > +has prereqs ip
> > +ct_lb(backends=fd0f::2,fd0f::3;
> hash_fields="eth_src,eth_dst,ip_src,ip_dst,sctp_src,sctp_dst");
> > +encodes as group:8
> > +uses group: id(8),
> name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,sctp_src,sctp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
> > +has prereqs ip
> >
> >   # ct_next
> >   ct_next;
> > @@ -1571,13 +1583,13 @@ handle_svc_check(reg0);
> >   # select
> >   reg9[16..31] = select(1=50, 2=100, 3, );
> >   formats as reg9[16..31] = select(1=50, 2=100, 3=100);
> > -encodes as group:6
> > -uses group: id(6),
> 

Re: [ovs-dev] [PATCH] ctags: Include new annotations to ctags ignore list.

[Flavio Leitner]

On Sat, Jun 27, 2020 at 05:11:15PM -0700, William Tu wrote:
> On Wed, Jun 10, 2020 at 04:49:45PM -0300, Flavio Leitner wrote:
> > The annotation OVS_NO_THREAD_SAFETY_ANALYSIS and OVS_LOCKABLE are
> > not part of the list, so ctags can't find functions using them.
> > 
> > The annotation list comes from a regex and to include more items
> > make the regex more difficult to read and maintain. Convert to a
> > static list because it isn't supposed to change much and there
> > is no standard names.
> > 
> > Also add a comment to remind to keep the list up-to-date.
> > 
> > Signed-off-by: Flavio Leitner 
> 
> Hi Flavio,
> 
> Instead of a static list, how about adding
> sed -n -e 's/^#define \(OVS_[A-Z_]\+\)(\.\.\.)$/\1+/p' \
>-e 's/^#define \(OVS_[A-Z_]\+\)$/\1+/p' include/openvswitch/compiler.h
> 
> So with the 2nd sed command, it creates
> OVS_NO_RETURN+
> OVS_UNUSED+
> OVS_WARN_UNUSED_RESULT+
> OVS_LOCKABLE+
> OVS_GUARDED+
> OVS_NO_THREAD_SAFETY_ANALYSIS+
> OVS_PACKED_ENUM+
> 
> Which covers the OVS_NO_THREAD_SAFETY_ANALYSIS+ and others.

The '+' is required only when parenthesis could be used, so
it wouldn't apply to OVS_NO_RETURN, for instance. I don't know
the side effects.

Another thing is that the '-I' parameter is useful when an
identifier causes syntactic confusion. I can navigate to symbols
using OVS_UNUSED and I don't see any problems. Same for OVS_NO_RETURN.

Funny thing is that I had issues with OVS_LOCKABLE but not anymore.
I did upgrade my fedora in between, so not sure.

Anyways, I still have issues navigating to dpdk_do_tx_copy() because
of OVS_NO_THREAD_SAFETY_ANALYSIS. It works if the identifier is
included in the list.

> I'm also ok keeping it static, if so, should we add these above?

I understand the goal of fixing one time and leave it automatically,
but ctags man-page is clear about using the parameter when an
identifier causes problems. I don't know what else it does, so I
took the safe approach.

What do you think?

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

[ovs-dev] [PATCH] netdev-offload-tc: Add drop action support.

[William Tu]

Currently drop action is not offloaded when using userspace datapath
with tc offload.  The patch programs tc gact (generic action) chain
ID 0 to drop the packet by setting it to TC_ACT_SHOT.

Example:
$ ovs-appctl dpctl/add-flow netdev@ovs-netdev \
  'recirc_id(0),in_port(2),eth(),eth_type(0x0806),\
  arp(op=2,tha=00:50:56:e1:4b:ab,tip=10.255.1.116)' drop

$ tc filter show dev ovs-p0 ingress
filter protocol arp pref 2 flower chain 0
filter protocol arp pref 2 flower chain 0 handle 0x1
  eth_type arp
  arp_tip 10.255.1.116
  arp_op reply
  arp_tha 00:50:56:e1:4b:ab
  skip_hw
  not_in_hw
action order 1: gact action drop
...

Signed-off-by: William Tu 
---
 lib/netdev-offload-tc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 258d31f54b08..8c6bcc0aa2c7 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1788,6 +1788,12 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 action->chain = nl_attr_get_u32(nla);
 flower.action_count++;
 recirc_act = true;
+#ifndef __KERNEL__
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_DROP) {
+action->type = TC_ACT_GOTO;
+action->chain = 0;  /* 0 is reserved and not used by recirc. */
+flower.action_count++;
+#endif
 } else {
 VLOG_DBG_RL(, "unsupported put action type: %d",
 nl_attr_type(nla));
-- 
2.7.4

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

Re: [ovs-dev] OVS needs to release new stable versions.

[Ben Pfaff]

On Tue, Jun 30, 2020 at 09:08:22AM -0400, Mark Michelson wrote:
> On 6/29/20 6:57 PM, Ben Pfaff wrote:
> > On Tue, Jun 30, 2020 at 12:43:48AM +0200, Ilya Maximets wrote:
> > > On 6/29/20 10:47 PM, Ben Pfaff wrote:
> > > > On Fri, Jun 26, 2020 at 12:18:37PM +0200, Ilya Maximets wrote:
> > > > > So, what is the proposed plan:
> > > > > 
> > > > >1. We should add missed git tags to 2.11.3 and 2.11.4 releases.
> > > > > 
> > > > >   Ben could you, please, take care of this? (Alternatively, I 
> > > > > could do that,
> > > > >   but I'm not sure what with the keys to sign tags and how this 
> > > > > key
> > > > >   management handled these days since the collapse of web of 
> > > > > trust.)
> > > > 
> > > > I think that it's probably best to take myself out off the critical path
> > > > here.  I started out using signed tags because they were easy for me,
> > > > since I already had a key in Debian's web of trust.  But I don't know of
> > > > a reason why they need to be signed, or signed by a particular key.  I
> > > > think it is perfectly reasonable if you push the tags, signed or
> > > > unsigned.
> > > 
> > > OK.  I'll do that.
> > 
> > Great.
> > 
> > > > I don't know what you mean by "the collapse of the web of trust".  Did I
> > > > miss a memo?
> > > 
> > > It was a reference to Certificate Spamming Attacks happened last year [1],
> > > and some consequences like creating new implementations of key servers
> > > (keys.openpgp.org) with some design changes in compare with SKS.
> > > 
> > > [1] https://gist.github.com/rjhansen/67ab921ffb4084c865b3618d6955275f
> > 
> > Oh my.  I did see something about that at the time, I think.  Ugh.
> > 
> > > > >3. Prepare patches for OVS stable releases for all branches 
> > > > > starting from
> > > > >   branch-2.5 and apply them.  Create, sign and push tags.
> > > > > 
> > > > >   Ben, I know you handled this part last time.  What is the 
> > > > > preferred way?
> > > > >   Someone else could prepare patches and send for review, I guess.
> > > > 
> > > > I think we should designate a newer "long term stable" release now that
> > > > OVN has released separately.
> > > 
> > > Yeah.  That's a good point.  Maybe 2.13 is a good candidate for a new LTS?
> > 
> > I hope so?  I'd really rather not be in the middle of that decision.
> > 
> > > > That aside, I have never prepared separate patches for OVS stable
> > > > releases.  When I apply bug fixes, I backport them as far as necessary.
> > > > It would be a burden to figure this out retrospectively, but it's
> > > > usually easy at the time of bug fix.
> > > 
> > > By "Prepare patches for OVS stable releases" I meant patches like this:
> > > * 2ff0b7715 2019-09-06 | Prepare for 2.10.5.
> > > * 5f19eaaf2 2019-09-06 | Set release date for 2.10.4. (tag: v2.10.4)
> > 
> > Oh!  Got it.  Justin used to do most of that work, so I didn't think
> > about it.  My part of the job was, typically, acking the patches and
> > pushing the tags (I did the latter because I was the one with the GPG
> > key).
> > 
> > I suspect that a lot of it could be automated with a script (which would
> > probably be more reliable than doing it manually, which I *think* Justin
> > probably did).
> 
> I've released three versions of OVN so far, all making the commits manually.
> And so far, I've made a mistake 1 time out of the 3.
> 
> So yes, I'd say a script is a much better approach, and if Justin does have
> such a script, I'd love to have a copy of it :)
> 
> If that script is not shareable for some reason, then I'll just create my
> own, probably when it comes time to release OVN 20.06.1

Oh, I don't think there is a script--I think Justin always did it by
hand too.  But I think one should be created...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] Fix selection fields for UDP and SCTP load balancers.

[Mark Michelson]

Acked-by: Mark Michelson 

On 6/30/20 1:21 AM, num...@ovn.org wrote:

From: Numan Siddique 

The commit 5af304e7478a ("Support selection fields in load balancer.")
didn't handle the selection fields for UDP and SCTP protocol.
If CMS has set the selection fields - tp_src and tp_dst for UDP or SCTP
load balancers, ovn-northd was adding lflows as
ct_lb(backends=, hash_fields(tp_src,tp_dst))
instead of ct_lb(backends=, hash_fields(udp_src,udp_dst)) and
ct_lb(backends=, hash_fields(sctp_src,sctp_dst)) respectively.

Because of this, select group flows were encoded with tcp_src and tcp_dst
hash fields.

This patch fixes this issue.

Test cases are now added for UDP and SCTP protocols.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1846189
Signed-off-by: Numan Siddique 
---

v1 -> v2

   * Addressed the concerns raised by Mark.
 Removed the refactoring of the code and the load balancer
 protocol is not stored in the internal structure.

  northd/ovn-northd.c |  14 +++-
  tests/ovn.at| 179 ++--
  tests/system-ovn.at |  23 ++
  3 files changed, 207 insertions(+), 9 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 2f973bff6e..9da4b5b860 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3351,10 +3351,22 @@ ovn_lb_create(struct northd_context *ctx, struct hmap 
*lbs,
  n_vips++;
  }
  
+char *proto = NULL;

+if (nbrec_lb->protocol && nbrec_lb->protocol[0]) {
+proto = nbrec_lb->protocol;
+}
+
  if (lb->nlb->n_selection_fields) {
  struct ds sel_fields = DS_EMPTY_INITIALIZER;
  for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) {
-ds_put_format(_fields, "%s,", lb->nlb->selection_fields[i]);
+char *field = lb->nlb->selection_fields[i];
+if (!strcmp(field, "tp_src") && proto) {
+ds_put_format(_fields, "%s_src,", proto);
+} else if (!strcmp(field, "tp_dst") && proto) {
+ds_put_format(_fields, "%s_dst,", proto);
+} else {
+ds_put_format(_fields, "%s,", field);
+}
  }
  ds_chomp(_fields, ',');
  lb->selection_fields = ds_steal_cstr(_fields);
diff --git a/tests/ovn.at b/tests/ovn.at
index 6a9bdf1d56..1a1cfbf124 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1029,6 +1029,18 @@ ct_lb(backends=fd0f::2,fd0f::3; 
hash_fields="eth_src,eth_dst,ip_src,ip_dst,tp_sr
  encodes as group:5
  uses group: id(5), 
name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,tp_src,tp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
  has prereqs ip
+ct_lb(backends=fd0f::2,fd0f::3; 
hash_fields="eth_src,eth_dst,ip_src,ip_dst,tcp_src,tcp_dst");
+encodes as group:6
+uses group: id(6), 
name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,tcp_src,tcp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
+has prereqs ip
+ct_lb(backends=fd0f::2,fd0f::3; 
hash_fields="eth_src,eth_dst,ip_src,ip_dst,udp_src,udp_dst");
+encodes as group:7
+uses group: id(7), 
name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,udp_src,udp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
+has prereqs ip
+ct_lb(backends=fd0f::2,fd0f::3; 
hash_fields="eth_src,eth_dst,ip_src,ip_dst,sctp_src,sctp_dst");
+encodes as group:8
+uses group: id(8), 
name(type=select,selection_method=hash,fields(eth_src,eth_dst,ip_src,ip_dst,sctp_src,sctp_dst),bucket=bucket_id=0,weight:100,actions=ct(nat(dst=fd0f::2),commit,table=19,zone=NXM_NX_REG13[0..15]),bucket=bucket_id=1,weight:100,actions=ct(nat(dst=fd0f::3),commit,table=19,zone=NXM_NX_REG13[0..15]))
+has prereqs ip
  
  # ct_next

  ct_next;
@@ -1571,13 +1583,13 @@ handle_svc_check(reg0);
  # select
  reg9[16..31] = select(1=50, 2=100, 3, );
  formats as reg9[16..31] = select(1=50, 2=100, 3=100);
-encodes as group:6
-uses group: id(6), 
name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:50,actions=load:1->xreg4[16..31],resubmit(,19),bucket=bucket_id=1,weight:100,actions=load:2->xreg4[16..31],resubmit(,19),bucket=bucket_id=2,weight:100,actions=load:3->xreg4[16..31],resubmit(,19))
+encodes as group:9
+uses group: id(9), 

Re: [ovs-dev] 10-25 packet drops every few (10-50) seconds TCP (iperf3)

[Flavio Leitner]

On Tue, Jun 02, 2020 at 12:56:51PM -0700, Vinay Gupta wrote:
> Hi Flavio,
> 
> Thanks for your reply.
> I have captured the suggested information but do not see anything that
> could cause the packet drops.
> Can you please take a look at the below data and see if you can find
> something unusual ?
> The PMDs are running on CPU 1,2,3,4 and CPU 1-7 are isolated cores.
> 
> ---
> root@bcm958802a8046c:~# cstats ; sleep 10; cycles
> pmd thread numa_id 0 core_id 1:
>   idle cycles: 99140849 (7.93%)
>   processing cycles: 1151423715 (92.07%)
>   avg cycles per packet: 116.94 (1250564564/10693918)
>   avg processing cycles per packet: 107.67 (1151423715/10693918)
> pmd thread numa_id 0 core_id 2:
>   idle cycles: 118373662 (9.47%)
>   processing cycles: 1132193442 (90.53%)
>   avg cycles per packet: 124.39 (1250567104/10053309)
>   avg processing cycles per packet: 112.62 (1132193442/10053309)
> pmd thread numa_id 0 core_id 3:
>   idle cycles: 53805933 (4.30%)
>   processing cycles: 1196762002 (95.70%)
>   avg cycles per packet: 107.35 (1250567935/11649948)
>   avg processing cycles per packet: 102.73 (1196762002/11649948)
> pmd thread numa_id 0 core_id 4:
>   idle cycles: 189102938 (15.12%)
>   processing cycles: 1061463293 (84.88%)
>   avg cycles per packet: 143.47 (1250566231/8716828)
>   avg processing cycles per packet: 121.77 (1061463293/8716828)
> pmd thread numa_id 0 core_id 5:
> pmd thread numa_id 0 core_id 6:
> pmd thread numa_id 0 core_id 7:


The core_id 3 is high loaded, and then it's more likely to show
the drop issue when some other event happens.

I think you need to run perf as I recommended before and see if
there are context switches happening and why they are happening.

If a context switch happens, it's either because the core is not
well isolated or some other thing is going on. It will help to
understand why the queue wasn't serviced for a certain amount of
time.

The issue is that running perf might introduce some load, so you
will need adjust the traffic rate accordingly.

HTH,
fbl



> 
> 
> *Runtime summary*  comm  parent   sched-in
> run-timemin-run avg-run max-run  stddev  migrations
>   (count)   (msec) (msec)
>(msec)  (msec)   %
> -
> ksoftirqd/0[7]   2  10.079  0.079
> 0.079   0.0790.00   0
>   rcu_sched[8]   2 140.067  0.002
> 0.004   0.0099.96   0
>rcuos/4[38]   2  60.027  0.002
> 0.004   0.008   20.97   0
>rcuos/5[45]   2  40.018  0.004
> 0.004   0.0056.63   0
>kworker/0:1[71]   2 120.156  0.008
> 0.013   0.0196.72   0
>  mmcqd/0[1230]   2  30.054  0.001
> 0.018   0.031   47.29   0
> kworker/0:1H[1248]   2  10.006  0.006
> 0.006   0.0060.00   0
>kworker/u16:2[1547]   2 160.045  0.001
> 0.002   0.012   26.19   0
> ntpd[5282]   1  10.063  0.063
> 0.063   0.0630.00   0
> watchdog[6988]   1  20.089  0.012
> 0.044   0.076   72.26   0
> ovs-vswitchd[9239]   1  20.326  0.152
> 0.163   0.1736.45   0
>revalidator8[9309/9239]9239  21.260  0.607
> 0.630   0.6523.58   0
>perf[27150]   27140  10.000  0.000
> 0.000   0.0000.00   0
> 
> Terminated tasks:
>   sleep[27151]   27150  41.002  0.015
> 0.250   0.677   58.22   0
> 
> Idle stats:
> CPU  0 idle for999.814  msec  ( 99.84%)
> 
> 
> 
> *CPU  1 idle entire time windowCPU  2 idle entire time windowCPU  3
> idle entire time windowCPU  4 idle entire time window*
> CPU  5 idle for500.326  msec  ( 49.96%)
> CPU  6 idle entire time window
> CPU  7 idle entire time window
> 
> Total number of unique tasks: 14
> Total number of context switches: 115
>Total run time (msec):  3.198
> Total scheduling time (msec): 1001.425  (x 8)
> (END)
> 
> 
> 
> *02:16:22  UID  TGID   TID%usr %system  %guest   %wait
>  %CPU   CPU  Command *02:16:230  9239 -  100.000.00
>0.000.00  100.00 5  ovs-vswitchd
> 02:16:23 

[ovs-dev] recherche

[Catherine ORVAL]

Bonjour, Vous avez un bien immobilier  vendre et vous
vous demandez si c'est le bon moment? Sachez que c'est bien le
cas.  En effet, le march actuel de l'immobilier tant
encore plus tendu qu'avant le COVID, c'est le bon moment pour obtenir
le meilleur prix pour votre bien.

En tant que professionnels de l'immobilier nous avons beaucoup plus de
demandes que d'offres sur toute l'Ile de France et en particulier
 Paris, dans le Val de Marne et la Seine St Denis o
l'offre est trs largement insuffisante.

Peut-tre hsitez-vous  mettre votre bien en vente
car vous avez vous-mme, du mal  trouver un bien pour
vous reloger par la suite?

Sachez que nous pouvons prendre en charge l'ensemble de votre projet
immobilier en vous accompagnant dans la vente de votre bien actuel
ainsi que dans l'achat de votre nouveau bien dans les meilleures
conditions.

Nous sommes pour cela en rapport avec de futurs acheteurs, des
courtiers, des notaires et nous mettons en place tout un processus
permettant  tout le monde de s'y retrouver.

Nous vous aiderons  vendre votre bien au meilleur prix et
ngocierons avec l'acheteur les dlais ncessaires
vous permettant de retrouver votre nouvelle demeure dans les meilleures
conditions et ce, quelle que soit la rgion puisque nous avons
des partenaires dans toute la France. Dcrivez-nous votre projet
sur notre formulaire de contactICIet nous reviendrons vers
vous trs rapidement.

Bien Cordialement L'quipe ORVAL Immobilier







Ne plus recevoir de mailsICI.

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

Re: [ovs-dev] OVS needs to release new stable versions.

[Mark Michelson]

On 6/29/20 6:57 PM, Ben Pfaff wrote:

On Tue, Jun 30, 2020 at 12:43:48AM +0200, Ilya Maximets wrote:

On 6/29/20 10:47 PM, Ben Pfaff wrote:

On Fri, Jun 26, 2020 at 12:18:37PM +0200, Ilya Maximets wrote:

So, what is the proposed plan:

   1. We should add missed git tags to 2.11.3 and 2.11.4 releases.

  Ben could you, please, take care of this? (Alternatively, I could do that,
  but I'm not sure what with the keys to sign tags and how this key
  management handled these days since the collapse of web of trust.)


I think that it's probably best to take myself out off the critical path
here.  I started out using signed tags because they were easy for me,
since I already had a key in Debian's web of trust.  But I don't know of
a reason why they need to be signed, or signed by a particular key.  I
think it is perfectly reasonable if you push the tags, signed or
unsigned.


OK.  I'll do that.


Great.


I don't know what you mean by "the collapse of the web of trust".  Did I
miss a memo?


It was a reference to Certificate Spamming Attacks happened last year [1],
and some consequences like creating new implementations of key servers
(keys.openpgp.org) with some design changes in compare with SKS.

[1] https://gist.github.com/rjhansen/67ab921ffb4084c865b3618d6955275f


Oh my.  I did see something about that at the time, I think.  Ugh.


   3. Prepare patches for OVS stable releases for all branches starting from
  branch-2.5 and apply them.  Create, sign and push tags.

  Ben, I know you handled this part last time.  What is the preferred way?
  Someone else could prepare patches and send for review, I guess.


I think we should designate a newer "long term stable" release now that
OVN has released separately.


Yeah.  That's a good point.  Maybe 2.13 is a good candidate for a new LTS?


I hope so?  I'd really rather not be in the middle of that decision.


That aside, I have never prepared separate patches for OVS stable
releases.  When I apply bug fixes, I backport them as far as necessary.
It would be a burden to figure this out retrospectively, but it's
usually easy at the time of bug fix.


By "Prepare patches for OVS stable releases" I meant patches like this:
* 2ff0b7715 2019-09-06 | Prepare for 2.10.5.
* 5f19eaaf2 2019-09-06 | Set release date for 2.10.4. (tag: v2.10.4)


Oh!  Got it.  Justin used to do most of that work, so I didn't think
about it.  My part of the job was, typically, acking the patches and
pushing the tags (I did the latter because I was the one with the GPG
key).

I suspect that a lot of it could be automated with a script (which would
probably be more reliable than doing it manually, which I *think* Justin
probably did).


I've released three versions of OVN so far, all making the commits 
manually. And so far, I've made a mistake 1 time out of the 3.


So yes, I'd say a script is a much better approach, and if Justin does 
have such a script, I'd love to have a copy of it :)


If that script is not shareable for some reason, then I'll just create 
my own, probably when it comes time to release OVN 20.06.1





But yes, I agree that looking through patches and backport them selectively
is too much work and will likely require even more people.  Backporting
patches as far as necessary is a good practice. I do that too.


Wonderful!  I was sure hoping that there was not a big backlog of
patches that might need backporting, since it's much more difficult to
do it that way in my experience.


We could do selective backports by request in case something missed, but
we should not revisit all the patches while preparing stable releases on
older branches.


Yes.


   4. Update relevant docs outside of openvswitch main repository (web-site,
  wiki, etc.)


P.S.: We should, probably, do some periodic checks on released
branches and make stable releases a bit more frequently.  For example,
we could make regular stable release every few months in case we have
fixes on top of the previous release.  I could keep monitoring things
and ping people, but this might be better to have some plan, I think.


Yes, probably.  At one point Justin and I were definitely the people who
coordinated this, but now if it is left to us then it probably won't get
done, or not done often enough.

I really need some new people to take the leadership role here.



I could take care of this.  I mean, I could periodically look at numbers
of patches on stable branches and prepare minor releases if needed.


That would be fantastic.



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

Re: [ovs-dev] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

[Alin Serdean]

Thanks for sending the patch.

Running the command:
make datapath_windows_analyze
resulted in the following:
ovsext\Conntrack.c(873): warning C28167: The function 'OvsCtExecute_' changes 
the IRQL and does not
restore the IRQL before it exits. It should be annotated to reflect the change 
or the IRQL should
be restored. IRQL was last set to 2 at line 955.
[datapath-windows\ovsext\ovsext.vcxproj]

There is a lock acquired but it is not released on the failure path.

You can fold in following incremental:

$ git diff datapath-windows/ovsext/Conntrack.c
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 414b3f4b2..717d04f49 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -974,6 +974,7 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
   OVS_CT_POOL_TAG);
 if (!entry->helper_name) {
 OVS_LOG_ERROR("Error while allocating memory");
+OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
 return NDIS_STATUS_RESOURCES;
 }

The rest looks good, but I have the following question:

 /* FTP ACTIVE - Server initiates the connection */
 if ((incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
-(incomingKey.src.port == entryKey.src.port) &&
Why is the line above removed?


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

Re: [ovs-dev] [PATCH ovn v2] Split SB Port_Group per datapath.

[Dumitru Ceara]

On 6/30/20 2:08 PM, Mark Michelson wrote:
> On 6/30/20 4:42 AM, Numan Siddique wrote:
>> On Mon, Jun 29, 2020 at 8:55 PM Dumitru Ceara  wrote:
>>
>>> In order to avoid ovn-controller reinstalling all logical flows that
>>> refer a port_group when some ports are added/removed from the port group
>>> we now change the way ovn-northd populates the Southbound DB Port_Group
>>> table.
>>>
>>> Instead of copying NB.Port_Group.name to SB.Port_Group.name we now
>>> create one SB.Port_Group record for every datapath that has ports
>>> referenced by the NB.Port_Group.ports field. In order to maintain the
>>> SB.Port_Group.name uniqueness constraint, ovn-northd populates the field
>>> with the value: _.
>>>
>>> In specific scenarios we see significant improvements in time to
>>> install/remove all logical flows to/from OVS. One such scenario, in the
>>> BZ referenced below has:
>>>
>>> $ ovn-nbctl acl-list pg
>>>    from-lport  1001 (inport == @pg && ip) drop
>>>  to-lport  1001 (outport == @pg && ip) drop
>>>
>>> Then, incrementally, creates new logical ports on different logical
>>> switches, binds them to OVS interfaces and adds them to the port_group.
>>>
>>> Measuring the total time to perform the above steps 500 times (for 500
>>> new ports attached to 100 switches, 5 per switch) on a test setup
>>> we observe an improvement of 50% in time it takes to install all
>>> openflow rules when port_groups are split in the SB database.
>>>
>>> Suggested-by: Numan Siddique 
>>> Reported-by: Venkata Anil 
>>> Reported-at: https://bugzilla.redhat.com/1818128
>>> Signed-off-by: Dumitru Ceara 
>>>
>>> ---
>>> v2:
>>> - add tests in ovn-northd.at as suggested by Numan.
>>>
>>
>> Thanks for v2 and adding tests.
>>
>> Acked-by: Numan Siddique 
>>
>> Numan
> 
> I pushed this to master and branch-20.06
> 

Thanks, Numan and Mark, for the reviews.

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn v2] Split SB Port_Group per datapath.

[Mark Michelson]

On 6/30/20 4:42 AM, Numan Siddique wrote:

On Mon, Jun 29, 2020 at 8:55 PM Dumitru Ceara  wrote:


In order to avoid ovn-controller reinstalling all logical flows that
refer a port_group when some ports are added/removed from the port group
we now change the way ovn-northd populates the Southbound DB Port_Group
table.

Instead of copying NB.Port_Group.name to SB.Port_Group.name we now
create one SB.Port_Group record for every datapath that has ports
referenced by the NB.Port_Group.ports field. In order to maintain the
SB.Port_Group.name uniqueness constraint, ovn-northd populates the field
with the value: _.

In specific scenarios we see significant improvements in time to
install/remove all logical flows to/from OVS. One such scenario, in the
BZ referenced below has:

$ ovn-nbctl acl-list pg
   from-lport  1001 (inport == @pg && ip) drop
 to-lport  1001 (outport == @pg && ip) drop

Then, incrementally, creates new logical ports on different logical
switches, binds them to OVS interfaces and adds them to the port_group.

Measuring the total time to perform the above steps 500 times (for 500
new ports attached to 100 switches, 5 per switch) on a test setup
we observe an improvement of 50% in time it takes to install all
openflow rules when port_groups are split in the SB database.

Suggested-by: Numan Siddique 
Reported-by: Venkata Anil 
Reported-at: https://bugzilla.redhat.com/1818128
Signed-off-by: Dumitru Ceara 

---
v2:
- add tests in ovn-northd.at as suggested by Numan.



Thanks for v2 and adding tests.

Acked-by: Numan Siddique 

Numan


I pushed this to master and branch-20.06





---
  TODO.rst  |  8 ++
  controller/lflow.c|  4 ++-
  include/ovn/expr.h|  4 ++-
  lib/actions.c |  2 +-
  lib/expr.c| 48 ---
  lib/ovn-util.h|  7 +
  northd/ovn-northd.c   | 79
++-
  tests/ovn-northd.at   | 79
+++
  tests/test-ovn.c  | 10 +++
  utilities/ovn-trace.c |  3 +-
  10 files changed, 198 insertions(+), 46 deletions(-)

diff --git a/TODO.rst b/TODO.rst
index 6df6b84..4b2fc2d 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -140,3 +140,11 @@ OVN To-do List
  * ovn-controller: Stop copying the local OVS configuration into the
Chassis external_ids column (same for the "is-remote" configuration from
ovn-ic) a few releases after the 20.06 version (21.06 maybe ?).
+
+* ovn-controller: Remove backwards compatibility for Southbound DB
Port_Group
+  names in expr.c a few releases after the 20.09 version. Right now
+  ovn-controller maintains backwards compatibility when connecting to a
+  SB database that doesn't store Port_Group.name as
+  . This causes an
additional
+  hashtable lookup in parse_port_group() which can be avoided when we are
sure
+  that the Southbound DB uses the new format.
diff --git a/controller/lflow.c b/controller/lflow.c
index c850a0d..5454361 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -566,7 +566,9 @@ consider_logical_flow(const struct sbrec_logical_flow
*lflow,
  struct sset port_groups_ref = SSET_INITIALIZER(_groups_ref);
  expr = expr_parse_string(lflow->match, , l_ctx_in->addr_sets,
   l_ctx_in->port_groups,
- _sets_ref, _groups_ref, );
+ _sets_ref, _groups_ref,
+ lflow->logical_datapath->tunnel_key,
+ );
  const char *addr_set_name;
  SSET_FOR_EACH (addr_set_name, _sets_ref) {
  lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_ADDRSET,
addr_set_name,
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 21bf51c..9838251 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -391,12 +391,14 @@ struct expr *expr_parse(struct lexer *, const struct
shash *symtab,
  const struct shash *addr_sets,
  const struct shash *port_groups,
  struct sset *addr_sets_ref,
-struct sset *port_groups_ref);
+struct sset *port_groups_ref,
+int64_t dp_id);
  struct expr *expr_parse_string(const char *, const struct shash *symtab,
 const struct shash *addr_sets,
 const struct shash *port_groups,
 struct sset *addr_sets_ref,
 struct sset *port_groups_ref,
+   int64_t dp_id,
 char **errorp);

  struct expr *expr_clone(struct expr *);
diff --git a/lib/actions.c b/lib/actions.c
index ee7c825..fc6e191 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -242,7 +242,7 @@ add_prerequisite(struct action_context *ctx, const
char *prerequisite)
  char *error;

  expr = expr_parse_string(prerequisite, ctx->pp->symtab, 

[ovs-dev] [ovs-dev v2] datapath-windows: Add CTA_HELP and CTA_TUPLE_MASTER

[Jinjun Gao]

Add helper and master if existing to a conntrack entry:
1, For CTA_HELP, only support FTP/TFTP;
2, For CTA_TUPLE_MASTER, only support FTP.

Signed-off-by: Jinjun Gao 
---
v2:
  1, Add CTA_TUPLE_MASTER support in CT event subscribe/report system.
  2, Release CT entry lock in fail path.
  3, Add more comments.
---
 datapath-windows/ovsext/Conntrack-related.c |  5 ++-
 datapath-windows/ovsext/Conntrack.c | 50 +
 datapath-windows/ovsext/Conntrack.h |  1 +
 3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-related.c 
b/datapath-windows/ovsext/Conntrack-related.c
index 950be98..a5bba5c 100644
--- a/datapath-windows/ovsext/Conntrack-related.c
+++ b/datapath-windows/ovsext/Conntrack-related.c
@@ -47,8 +47,11 @@ OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY 
entryKey)
 }

 /* FTP ACTIVE - Server initiates the connection */
+/* Some ftp server, such as pyftpdlib, may use random (>1024) data port
+ * except 20. In this case, the incomingKey's src port is different with
+ * entryKey's src port.
+ */
 if ((incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
-(incomingKey.src.port == entryKey.src.port) &&
 (incomingKey.dst.addr.ipv4 == entryKey.dst.addr.ipv4) &&
 (incomingKey.dst.port == entryKey.dst.port) &&
 (incomingKey.dl_type == entryKey.dl_type) &&
diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index 55917c4..d065591 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -246,7 +246,6 @@ OvsPostCtEventEntry(POVS_CT_ENTRY entry, UINT8 type)
 {
 OVS_CT_EVENT_ENTRY ctEventEntry = {0};
 NdisMoveMemory(, entry, sizeof(OVS_CT_ENTRY));
-ctEventEntry.entry.parent = NULL;
 ctEventEntry.type = type;
 OvsPostCtEvent();
 }
@@ -480,6 +479,9 @@ OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
 RemoveEntryList(>link);
 OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
 NdisFreeSpinLock(&(entry->lock));
+if (entry->helper_name) {
+OvsFreeMemoryWithTag(entry->helper_name, OVS_CT_POOL_TAG);
+}
 OvsFreeMemoryWithTag(entry, OVS_CT_POOL_TAG);
 NdisInterlockedDecrement((PLONG));
 return;
@@ -883,6 +885,7 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
 BOOLEAN triggerUpdateEvent = FALSE;
 BOOLEAN entryCreated = FALSE;
 POVS_CT_ENTRY entry = NULL;
+POVS_CT_ENTRY parent = NULL;
 PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
 OvsConntrackKeyLookupCtx ctx = { 0 };
 LOCK_STATE_EX lockStateTable;
@@ -959,8 +962,6 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,

 if (OvsDetectFtpPacket(key)) {
 /* FTP parser will always be loaded */
-UNREFERENCED_PARAMETER(helper);
-
 status = OvsCtHandleFtp(curNbl, key, layers, currentTime, entry,
 (ntohs(key->ipKey.l4.tpDst) == IPPORT_FTP));
 if (status != NDIS_STATUS_SUCCESS) {
@@ -968,10 +969,25 @@ OvsCtExecute_(OvsForwardingContext *fwdCtx,
 }
 }

+parent = entry->parent;
+/* The entry should have the same helper name with parent's */
+if (!entry->helper_name &&
+(helper || (parent && parent->helper_name))) {
+
+helper = helper ? helper : parent->helper_name;
+entry->helper_name = OvsAllocateMemoryWithTag(strlen(helper) + 1,
+  OVS_CT_POOL_TAG);
+if (!entry->helper_name) {
+OVS_LOG_ERROR("Error while allocating memory");
+OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql);
+return NDIS_STATUS_RESOURCES;
+}
+memcpy(entry->helper_name, helper, strlen(helper) + 1);
+}
+
 /* Add original tuple information to flow Key */
 if (entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) {
-if (entry->parent != NULL) {
-POVS_CT_ENTRY parent = entry->parent;
+if (parent != NULL) {
 OVS_ACQUIRE_SPIN_LOCK(&(parent->lock), irql);
 OvsCtUpdateTuple(key, >key);
 OVS_RELEASE_SPIN_LOCK(&(parent->lock), irql);
@@ -1042,8 +1058,8 @@ OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
 if (helper == NULL) {
 return NDIS_STATUS_INVALID_PARAMETER;
 }
-if (strcmp("ftp", helper) != 0) {
-/* Only support FTP */
+if (strcmp("ftp", helper) != 0 && strcmp("tftp", helper) != 0) 
{
+/* Only support FTP/TFTP */
 return NDIS_STATUS_NOT_SUPPORTED;
 }
 break;
@@ -1683,6 +1699,26 @@ OvsCreateNlMsgFromCtEntry(POVS_CT_ENTRY entry,
 }
 }

+if (entry->helper_name) {
+UINT32 offset;
+offset = NlMsgStartNested(, CTA_HELP);
+if (!offset) {
+return NDIS_STATUS_FAILURE;
+   

Re: [ovs-dev] [PATCH v4 1/7] dpif-netdev: implement subtable lookup validation.

[Van Haaren, Harry]

> -Original Message-
> From: Van Haaren, Harry
> Sent: Tuesday, June 30, 2020 11:01 AM
> To: William Tu 
> Cc: ovs-dev ; Stokes, Ian ; 
> Ilya
> Maximets ; Federico Iezzi 
> Subject: RE: [PATCH v4 1/7] dpif-netdev: implement subtable lookup validation.
> 
> > -Original Message-
> > From: William Tu 
> > Sent: Saturday, June 27, 2020 7:27 PM
> > To: Van Haaren, Harry 
> > Cc: ovs-dev ; Stokes, Ian ;
> > Ilya Maximets ; Federico Iezzi 
> > Subject: Re: [PATCH v4 1/7] dpif-netdev: implement subtable lookup 
> > validation.
> >
> > On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
> >  wrote:



> > > - Add check to ensure autovalidator is 0th item in lookup array
> >
> > What's your concern here? Users can only change the prio, not
> > the .probe function. So autovalidator is always at 0th item.
> > If you worry about code being changed accidentally in the future,
> > how about using BUILD_ASSERT_DECL?
> 
> Yes - good improvement.

Indeed the goal is to ensure the autovalidator is the 0th lookup, and
that code is not changed accidentally. It seems like using a BUILD_ASSERT_DECL
would achieve that, however the compiler doesn't agree with me:

lib/dpif-netdev-lookup.c:67:45: error: expression in static assertion is not 
constant
 BUILD_ASSERT_DECL(subtable_lookups[0].probe == 
dpcls_subtable_autovalidator_probe);
   ~~^~

In this case, the subtable_lookups[] is static, but not const. It cannot be 
const as
the user must be able to change the .priority field of each lookup 
implementation.
Doing a build time assert is (correctly) flagging that we're asserting 
non-const data,
which could be changed during runtime.

The autovalidator component has a runtime check using ovs_assert() today, so a
check is already being done:
ovs_assert(lookup_funcs[0].probe(0, 0) == dpcls_subtable_autovalidator);

Summary:
- The existing check in autovalidator is enough to ensure its element 0 in the 
lookups array.
- Adding a BUILD_ASSERT_DECL isn't allowed by the compiler, as its not const 
data being validated.

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

[ovs-dev] 支払方法の情報を更新する

[Amazon]

 
サインインが検出されました



尊敬する客様、新しいデバイスからお客様のアカウントへのサインインが検出されました。

日時: Jun 30, 2020 19:02:37 Japan Standard Time 
デバイス: Microsoft Internet Explorer Windows (デスクトップ) 
付近: Japan 


これがお客様ご自身による操作だった場合、このメッセージは無視してください。 そうでない場合は、お知らせください。

 1. アカウントサービスからAmazonプライム会員情報を管理するにアクセスします。
2. Amazonプライムに登録したAmazon.co.jpのアカウントを使用してサインインします。 
3. 左側に表示されている「現在の支払方法」の下にある「支払方法を変更する」のリンクをクリックします。

4. 有効期限の更新または新しいクレジットカード情報を入力してください。


Amazonプライムを継続してご利用いただくために、会費のお支払いにご指定いただいたクレジットカードが使用できない場合は、アカウントに登録されている別 
のクレジットカードに会費を請求させて頂きます。会費の請求が出来ない場合は、お客様のAmazonプライム会員資格は失効し、特典をご利用できなくなります。

Amazon.co.jpカスタマーサービス

支払方法の情報を更新する 
注意：本通知メールは送信専用アドレスで送信しております。返信いただいても受信できませんのでご了承下さい。このメッセージはd...@openvswitch.org
 に送信されました。 



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

Re: [ovs-dev] [PATCH v4 7/7] docs/dpdk/bridge: add datapath performance section.

[Van Haaren, Harry]

> -Original Message-
> From: William Tu 
> Sent: Saturday, June 27, 2020 7:28 PM
> To: Van Haaren, Harry 
> Cc: ovs-dev ; Stokes, Ian ;
> Ilya Maximets ; Federico Iezzi 
> Subject: Re: [PATCH v4 7/7] docs/dpdk/bridge: add datapath performance
> section.
> 
> On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
>  wrote:
> >
> > This commit adds a section to the dpdk/bridge.rst netdev documentation,
> > detailing the added DPCLS functionality. The newly added commands are
> > documented, and sample output is provided.
> >
> > Running the DPCLS autovalidator with unit tests by default is possible
> > through re-compiling the autovalidator to have the highest priority at
> > startup time. This avoids making changes to all tests, and enables
> > debug and CI builds to validate every lookup implementation with all
> > unit tests.
> >
> > Signed-off-by: Harry van Haaren 
> >
> Acked-by: William Tu 

Thanks will include your Acks in v5.

> snip
> > +Running Unit Tests with Autovalidator
> > ++
> > +
> > +To run the OVS unit test suite with the DPCLS autovalidator as the default
> > +implementation, it is required to recompile OVS. During the recompilation,
> > +the default priority of the `autovalidator` implementation is set to the
> > +maximum priority, ensuring every test will be run with every lookup
> > +implementation ::
> > +
> > +$ ./configure --enable-autovalidator
> > +
> 
> Now I understand the use case.
> Do you see any error if you run ex: make check-system-userspace?

Will check this.


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

Re: [ovs-dev] [PATCH v4 5/7] dpif-lookup: add avx512 gather implementation.

[Van Haaren, Harry]

> -Original Message-
> From: William Tu 
> Sent: Saturday, June 27, 2020 7:27 PM
> To: Van Haaren, Harry 
> Cc: ovs-dev ; Stokes, Ian ;
> Ilya Maximets ; Federico Iezzi 
> Subject: Re: [PATCH v4 5/7] dpif-lookup: add avx512 gather implementation.
> 
> On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
>  wrote:
> >
> > This commit adds an AVX-512 dpcls lookup implementation.
> > It uses the AVX-512 SIMD ISA to perform multiple miniflow
> > operations in parallel.
> >
> > To run this implementation, the "avx512f" and "bmi2" ISAs are
> > required. These ISA checks are performed at runtime while
> > probing the subtable implementation. If a CPU does not provide
> > both "avx512f" and "bmi2", then this code does not execute.
> >
> > The avx512 code is built as a seperate static library, with added
> > CFLAGS to enable the required ISA features. By building only this
> > static library with avx512 enabled, it is ensured that the main OVS
> > core library is *not* using avx512, and that OVS continues to run
> > as before on CPUs that do not support avx512.
> >
> > The approach taken in this implementation is to use the
> > gather instruction to access the packet miniflow, allowing
> > any miniflow blocks to be loaded into an AVX-512 register.
> > This maximises the usefulness of the register, and hence this
> > implementation handles any subtable with up to miniflow 8 bits.
> >
> > Note that specialization of these avx512 lookup routines
> > still provides performance value, as the hashing of the
> > resulting data is performed in scalar code, and compile-time
> > loop unrolling occurs when specialized to miniflow bits.
> >
> > Signed-off-by: Harry van Haaren 
> >
> > ---
> >
> > v4:
> > - Remove TODO comment on prio-set command (was accidentally
> >   added to this commit in v3)
> > - Fixup v3 changlog to not include #warning comment (William Tu)
> > - Remove #define for debugging in lookup.h
> > - Fix builds on older gcc versions that don't support -mavx512f.
> >   Solution involves CC_CHECK and #ifdefs in code (OVS Robot, William Tu)
> >
> > v3:
> > - Improve function name for _any subtable lookup
> > - Use "" include not <> for immintrin.h
> > - Add checks for SSE42 instructions in core OVS for CRC32 based hashing
> >   If not available, disable AVX512 lookup implementation as it requires
> >   uses CRC32 for hashing, and the hashing algorithm must match core OVS.
> > - Rework ovs_asserts() into function selection time check
> > - Add #define for magic number 8, number of u64 blocks in AVX512 register
> > - Add #if CHECKER around AVX code, sparse doesn't like checking it
> > - Simplify avx512 enabled building, fixes builds with --enable-shared
> > ---
> >  configure.ac   |   2 +
> >  lib/automake.mk|  17 ++
> >  lib/dpif-netdev-lookup-avx512-gather.c | 265 +
> >  lib/dpif-netdev-lookup.c   |  17 ++
> >  lib/dpif-netdev-lookup.h   |   4 +
> >  5 files changed, 305 insertions(+)
> >  create mode 100644 lib/dpif-netdev-lookup-avx512-gather.c
> >
> > diff --git a/configure.ac b/configure.ac
> > index 81893e56e..1367c868b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -178,6 +178,8 @@ OVS_ENABLE_OPTION([-Wno-null-pointer-arithmetic])
> >  OVS_ENABLE_OPTION([-Warray-bounds-pointer-arithmetic])
> >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused], [HAVE_WNO_UNUSED])
> >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter],
> [HAVE_WNO_UNUSED_PARAMETER])
> > +OVS_CONDITIONAL_CC_OPTION([-mavx512f], [HAVE_AVX512F])
> > +OVS_CHECK_CC_OPTION([-mavx512f], [CFLAGS="$CFLAGS -DHAVE_AVX512F"])
> 
> Do you need both checks?
> I thought the first one OVS_CONDITIONAL_CC_OPTION([-mavx512f],
> [HAVE_AVX512F])
> is good enough since at lib/automake.mk, you add the -mavx512f to CFLAGS.

>From testing during development, both are required.
CONDITIONAL_CC_OPTION adds a build-system flag, indicating its present, but 
doesn't
seem to add a C #define for it, that can be used for conditional compilation?

The CHECK_CC_OPTION is used to manually add a #define via command-line -D 
parameter, it is used to add the avx512_gather probe function in the available 
lookup function struct.

There may be a more elegant way to achieve both in the same line, my AC-fu is 
somewhat outdated, suggestions welcome if you know of a better method :)



> > +#include "immintrin.h"
> > +
> > +/* Each AVX512 register (zmm register in assembly notation) can contain up
> to
> > + * 512 bits, which is equivelent to 8 uint64_t variables. This is the 
> > maximum
> 
> typo: equivalent

Will fix.


> > + * number of miniflow blocks that can be processed in a single pass of the
> > + * AVX512 code at a time.
> > + */
> > +#define NUM_U64_IN_ZMM_REG (8)
> > +#define BLOCKS_CACHE_SIZE (NETDEV_MAX_BURST *
> NUM_U64_IN_ZMM_REG)
> > +
> > +
> > +VLOG_DEFINE_THIS_MODULE(dpif_lookup_avx512_gather);
> > +
> > +static inline __m512i
> > 

Re: [ovs-dev] [PATCH v4 0/7] DPCLS Subtable ISA Optimization

[Van Haaren, Harry]

> -Original Message-
> From: William Tu 
> Sent: Saturday, June 27, 2020 7:27 PM
> To: Van Haaren, Harry 
> Cc: ovs-dev ; Stokes, Ian ; 
> Ilya
> Maximets ; Federico Iezzi 
> Subject: Re: [PATCH v4 0/7] DPCLS Subtable ISA Optimization
> 
> On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
>  wrote:
> >
> > v4 work done:
> > - Fixed shared object build link errors (--enable-shared flag)
> > - Enabled autovalidator to run with unit-tests (--enable-autovalidator flag)
> > - Address other feedback on v3
> > - Improve binutils AVX512 issue detection (in line with proposed DPDK 
> > method)
> >
> I tested v4 on travis and there are a couple of errors.
> Please take a look here:
> https://travis-ci.org/github/williamtu/ovs-travis/builds/702701139

Had a look - it seems to be a simple compile issue based on the avx512 gather()
instruction parameter checking, will include in v5, exact error here:
https://travis-ci.org/github/williamtu/ovs-travis/jobs/702701140#L1476

> other comments are in each patch.

Replies per thread too.

> > v5 Plans for work:
> > - Add NEWS section
> > - Integrate patches 5 and 6 into a single commit
> > - Address (any remaining?) feedback on v4
> 
> Thanks, look forward to v5.
> William

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

Re: [ovs-dev] [PATCH v4 4/7] dpdk: enable cpu feature detection.

[Van Haaren, Harry]

> -Original Message-
> From: William Tu 
> Sent: Saturday, June 27, 2020 7:27 PM
> To: Van Haaren, Harry 
> Cc: ovs-dev ; Stokes, Ian ;
> Ilya Maximets ; Federico Iezzi 
> Subject: Re: [PATCH v4 4/7] dpdk: enable cpu feature detection.
> 
> On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
>  wrote:
> >
> > This commit implements a method to retrieve the CPU ISA capabilities.
> > These ISA capabilities can be used in OVS to at runtime select a function
> > implementation to make the best use of the available ISA on the CPU.
> >
> > Signed-off-by: Harry van Haaren 
> >
> > ---
> >
> > v4:
> > - Improve commit title and message
> > ---
> >  lib/dpdk-stub.c | 13 +
> >  lib/dpdk.c  | 27 +++
> >  lib/dpdk.h  |  2 ++
> >  3 files changed, 42 insertions(+)
> >
> > diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
> > index c332c217c..9935f3d2b 100644
> > --- a/lib/dpdk-stub.c
> > +++ b/lib/dpdk-stub.c
> > @@ -79,6 +79,19 @@ print_dpdk_version(void)
> >  {
> >  }
> >
> > +int
> > +dpdk_get_cpu_has_isa(const char *arch OVS_UNUSED,
> > + const char *feature OVS_UNUSED)
> 
> should we just have bool as a return type?

Sure yes, will fix.

> > +{
> > +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> > +if (ovsthread_once_start()) {
> > +VLOG_ERR("DPDK not supported in this version of Open vSwitch, "
> > + "cannot use CPU flag based optimizations");
> > +ovsthread_once_done();
> 
> How about using VLOG_ERR_ONCE()?

Ah nice - indeed manually doing the ovsthread_once_start() etc felt a little 
clunky.
Updated in v5.

> > +}
> > +return 0;
> and just return false here.

Yep, will do.

> The rest looks good to me, thanks
> 
> William

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

Re: [ovs-dev] [PATCH v4 6/7] configure/m4: binutils avx512 configure time check.

[Van Haaren, Harry]

> -Original Message-
> From: William Tu 
> Sent: Saturday, June 27, 2020 7:27 PM
> To: Van Haaren, Harry 
> Cc: ovs-dev ; Stokes, Ian ;
> Ilya Maximets ; Federico Iezzi 
> Subject: Re: [PATCH v4 6/7] configure/m4: binutils avx512 configure time 
> check.
> 
> On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
>  wrote:
> >
> > This commit checks at configure time if the assembling in use
> > has a known bug in assembling AVX512 code. If this bug is present,
> > all AVX512 code is disabled.
> >
> > Checking the version string of the binutils or assembler is not
> > a good method to detect the issue, as backported fixes would not
> > be reflected.
> >
> > The method used here is also proposed for DPDK:
> > http://patches.dpdk.org/patch/71723/
> >
> > Signed-off-by: Harry van Haaren 
> >
> > ---
> >
> > v4:
> > - This patch is left seperate to ease reviews, as this is a logically
> >   seperate check. In v5 this code change will be merged with the commit
> >   that introduces the AVX512 code itself to ensure build consistency.
> > ---
> >  configure.ac |  1 +
> >  lib/dpif-netdev-lookup.c |  2 ++
> >  m4/openvswitch.m4| 24 
> >  3 files changed, 27 insertions(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 1367c868b..76d6de4e8 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -184,6 +184,7 @@ OVS_ENABLE_WERROR
> >  OVS_ENABLE_SPARSE
> >  OVS_CTAGS_IDENTIFIERS
> >  OVS_CHECK_DPCLS_AUTOVALIDATOR
> > +OVS_CHECK_BINUTILS_AVX512
> >
> >  AC_ARG_VAR(KARCH, [Kernel Architecture String])
> >  AC_SUBST(KARCH)
> > diff --git a/lib/dpif-netdev-lookup.c b/lib/dpif-netdev-lookup.c
> > index 2c740399b..2a77814f2 100644
> > --- a/lib/dpif-netdev-lookup.c
> > +++ b/lib/dpif-netdev-lookup.c
> > @@ -45,6 +45,7 @@ static struct dpcls_subtable_lookup_info_t
> subtable_lookups[] = {
> >
> >  #ifdef __x86_64__
> >  #if HAVE_AVX512F
> > +#if HAVE_LD_AVX512_GOOD
> >  #ifdef __SSE4_2__
> nit:
> maybe combine these three conditions into one #if?

Will investigate.

> >  /* Only available on x86_64 bit builds with SSE 4.2 used for OVS core. 
> > */
> >  { .prio = 0,
> > @@ -59,6 +60,7 @@ static struct dpcls_subtable_lookup_info_t
> subtable_lookups[] = {
> >  #endif
> >  #endif
> >  #endif
> > +#endif
> >  };
> 
> The rest looks good to me, thanks
> 
> William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 1/7] dpif-netdev: implement subtable lookup validation.

[Van Haaren, Harry]

> -Original Message-
> From: William Tu 
> Sent: Saturday, June 27, 2020 7:27 PM
> To: Van Haaren, Harry 
> Cc: ovs-dev ; Stokes, Ian ;
> Ilya Maximets ; Federico Iezzi 
> Subject: Re: [PATCH v4 1/7] dpif-netdev: implement subtable lookup validation.
> 
> On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren
>  wrote:
> >
> > This commit refactors the existing dpif subtable function pointer
> > infrastructure, and implements an autovalidator component.
> >
> > The refactoring of the existing dpcls subtable lookup function
> > handling, making it more generic, and cleaning up how to enable
> > more implementations in future.
> >
> > In order to ensure all implementations provide identical results,
> > the autovalidator is added. The autovalidator itself implements
> > the subtable lookup function prototype, but internally iterates
> > over all other available implementations. The end result is that
> > testing of each implementation becomes automatic, when the auto-
> > validator implementation is selected.
> >
> > Signed-off-by: Harry van Haaren 
> >
> > ---
> >
> > v4:
> > - Fix automake .c file order (William Tu)
> > - Fix include file ordering: netdev-lookup before netdev-perf (William Tu)
> > - Fix Typos (William Tu)
> > - Add --enable-autovalidator compile time flag to set the DPCLS 
> > Autovalidator
> >   to the highest priority by default. This is required to run the unit tests
> >   with all DPCLS implementations without changing every test-case.
> >   Backwards compatibility is kept with OVS 2.13.
> 
> Why adding this option at compile time?
> To run the unit test, we can simply use the set-prio command to set
> the autovalidator to the highest before starting the unit test, right?

As you noted later in the patchset 
(https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/372114.html)
the goal is to enable autovalidator *by default*. This is not the default
behavior we want in packaged versions of OVS, only for DPCLS test builds.

By making this compile-time instead of runtime, it ensures any test that
exists, or is developed in future can be tested with the autovalidator without
any extra manual effort to enable autovalidation.


> > - Add check to ensure autovalidator is 0th item in lookup array
> 
> What's your concern here? Users can only change the prio, not
> the .probe function. So autovalidator is always at 0th item.
> If you worry about code being changed accidentally in the future,
> how about using BUILD_ASSERT_DECL?

Yes - good improvement. 


> The rest looks good to me.
> Thanks!

Thanks for review.


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

Re: [ovs-dev] [PATCH V3 12/12] netdev-offload-dpdk: Fix Ethernet matching for type only

[Ilya Maximets]

On 6/30/20 6:43 AM, Eli Britstein wrote:
> 
> On 6/29/2020 9:10 PM, Ilya Maximets wrote:
 While calling from the userspace datapath, we always have a match on 
 dl_type.
 This means that it's not possible to hit the 'else' condition.
 I'm worried about the usecase described there.  It's hard to track changes
 in i40e_flow.c.  Can someone test this with XL710?
>>> Yes, that's true. I think we should have it for consistency with the rest 
>>> of the code, and also just in case.
>>>
>>> Regarding the XL710 comment - I think it should not have been merged into 
>>> OVS in the first place. If it is an issue with XL710, the relevant PMD 
>>> should handle it, and not OVS. I just kept it in case I miss something 
>>> here. Do you think it's OK to remove it?
>> Thinking about correctness, if dl_type match requested and we're
>> matching any L2 packets instead, this does not sound good.  In
>> this case we might perform incorrect set of actions for a packet
>> that should be handled differently.  Also, we might end up having
>> several flows with the same match and different actions in case the
>> only dufference was in dl_type, which is not a good thing too.
> This commit addresses exactly this issue, and applies specific ETH matches if 
> applicable, including dl_type.
>> So, from that point of view it's better to remove the 'else' branch
>> entirely.  Good reasoning should be described in a commit message.
> I don't see how that implies removing the else entirely. If there is from 
> some reason (though won't happen at least currently) no matches on any eth 
> (src/dst/type), it does make sense to match on generic "ETH". For 
> completeness I'd prefer to keep the else branch, but maybe drop only the 
> comment about XL710. What do you think?

AFAIU, rte_flow allows to not specify RTE_FLOW_ITEM_TYPE_ETH.
So, I'm not sure why we need to add a match on RTE_FLOW_ITEM_TYPE_ETH
if not requested by upper layers.

You could remove the current comment, but you will need to add another
one to justify adding ETH item while it's not requested.
Will HW drop all the packets if we don't have ETH pattern item?  That
might be a good justification (still HW/PMD specific, though).

>> For the 'if' condition:  You could do 'masks.dl_type' check first,
>> i.e. before checking eth addresses, this way we will save a few cpu
>> cycles in most cases.
> OK.

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

Re: [ovs-dev] [PATCH v1 0/6] Memory access optimization for flow scalability of userspace datapath.

[Yanqin Wei]

Hi, every contributor

These patches could significantly improve multi-flow throughput of userspace 
datapath.  If you feel it will take too much time to review all patches, I 
suggest you could look at the 2nd/3rd first, which have the major improvement 
in these patches.
[ovs-dev][PATCH v1 2/6] dpif-netdev: add tunnel_valid flag to skip ip/ipv6 
address comparison
[ovs-dev][PATCH v1 3/6] dpif-netdev: improve emc lookup performance by 
contiguous storage of hash value.

Any comments from anyone are appreciated.

Best Regards,
Wei Yanqin

> -Original Message-
> From: Yanqin Wei 
> Sent: Tuesday, June 2, 2020 3:10 PM
> To: d...@openvswitch.org
> Cc: nd ; i.maxim...@ovn.org; u9012...@gmail.com; Malvika
> Gupta ; Lijian Zhang ;
> Ruifeng Wang ; Lance Yang
> ; Yanqin Wei 
> Subject: [ovs-dev][PATCH v1 0/6] Memory access optimization for flow
> scalability of userspace datapath.
> 
> OVS userspace datapath is a program with heavy memory access. It needs to
> load/store a large number of memory, including packet header, metadata,
> EMC/SMC/DPCLS tables and so on. It causes a lot of cache line missing and
> refilling, which has a great impact on flow scalability. And in some cases, 
> EMC
> has a negative impact on the overall performance. It is difficult for user to
> dynamically manage the enabling of EMC.
> 
> This series of patches improve memory access of userspace datapath as
> follows:
> 1. Reduce the number of metadata cache line accessed by non-tunnel traffic.
> 2. Decrease unnecessary memory load/store for batch/flow.
> 3. Modify the layout of EMC data struct. Centralize the storage of hash value.
> 
> In the NIC2NIC traffic tests, the overall performance improvement is observed,
> especially in multi-flow cases.
> Flows   delta
> 1-1K flows  5-10%
> 10K flows   20%
> 100K flows  40%
> EMC disable 10%
> 
> Malvika Gupta (1):
>   [ovs-dev] dpif-netdev: Modify dfc_processing function to void function
> 
> Yanqin Wei (5):
>   netdev: avoid unnecessary packet batch refilling in netdev feature
> check
>   dpif-netdev: add tunnel_valid flag to skip ip/ipv6 address comparison
>   dpif-netdev: improve emc lookup performance by contiguous storage of
> hash value.
>   dpif-netdev: skip flow hash calculation in case of smc disabled
>   dpif-netdev: remove unnecessary key length calculation in fast path
> 
>  lib/dp-packet.h   |  12 +++--
>  lib/dpif-netdev.c | 115 --
>  lib/flow.c|   2 +-
>  lib/netdev.c  |  13 --
>  lib/packets.h |  46 ---
>  5 files changed, 120 insertions(+), 68 deletions(-)
> 
> --
> 2.17.1

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

Re: [ovs-dev] [PATCH ovn v2] Split SB Port_Group per datapath.

[Numan Siddique]

On Mon, Jun 29, 2020 at 8:55 PM Dumitru Ceara  wrote:

> In order to avoid ovn-controller reinstalling all logical flows that
> refer a port_group when some ports are added/removed from the port group
> we now change the way ovn-northd populates the Southbound DB Port_Group
> table.
>
> Instead of copying NB.Port_Group.name to SB.Port_Group.name we now
> create one SB.Port_Group record for every datapath that has ports
> referenced by the NB.Port_Group.ports field. In order to maintain the
> SB.Port_Group.name uniqueness constraint, ovn-northd populates the field
> with the value: _.
>
> In specific scenarios we see significant improvements in time to
> install/remove all logical flows to/from OVS. One such scenario, in the
> BZ referenced below has:
>
> $ ovn-nbctl acl-list pg
>   from-lport  1001 (inport == @pg && ip) drop
> to-lport  1001 (outport == @pg && ip) drop
>
> Then, incrementally, creates new logical ports on different logical
> switches, binds them to OVS interfaces and adds them to the port_group.
>
> Measuring the total time to perform the above steps 500 times (for 500
> new ports attached to 100 switches, 5 per switch) on a test setup
> we observe an improvement of 50% in time it takes to install all
> openflow rules when port_groups are split in the SB database.
>
> Suggested-by: Numan Siddique 
> Reported-by: Venkata Anil 
> Reported-at: https://bugzilla.redhat.com/1818128
> Signed-off-by: Dumitru Ceara 
>
> ---
> v2:
> - add tests in ovn-northd.at as suggested by Numan.
>

Thanks for v2 and adding tests.

Acked-by: Numan Siddique 

Numan


> ---
>  TODO.rst  |  8 ++
>  controller/lflow.c|  4 ++-
>  include/ovn/expr.h|  4 ++-
>  lib/actions.c |  2 +-
>  lib/expr.c| 48 ---
>  lib/ovn-util.h|  7 +
>  northd/ovn-northd.c   | 79
> ++-
>  tests/ovn-northd.at   | 79
> +++
>  tests/test-ovn.c  | 10 +++
>  utilities/ovn-trace.c |  3 +-
>  10 files changed, 198 insertions(+), 46 deletions(-)
>
> diff --git a/TODO.rst b/TODO.rst
> index 6df6b84..4b2fc2d 100644
> --- a/TODO.rst
> +++ b/TODO.rst
> @@ -140,3 +140,11 @@ OVN To-do List
>  * ovn-controller: Stop copying the local OVS configuration into the
>Chassis external_ids column (same for the "is-remote" configuration from
>ovn-ic) a few releases after the 20.06 version (21.06 maybe ?).
> +
> +* ovn-controller: Remove backwards compatibility for Southbound DB
> Port_Group
> +  names in expr.c a few releases after the 20.09 version. Right now
> +  ovn-controller maintains backwards compatibility when connecting to a
> +  SB database that doesn't store Port_Group.name as
> +  . This causes an
> additional
> +  hashtable lookup in parse_port_group() which can be avoided when we are
> sure
> +  that the Southbound DB uses the new format.
> diff --git a/controller/lflow.c b/controller/lflow.c
> index c850a0d..5454361 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -566,7 +566,9 @@ consider_logical_flow(const struct sbrec_logical_flow
> *lflow,
>  struct sset port_groups_ref = SSET_INITIALIZER(_groups_ref);
>  expr = expr_parse_string(lflow->match, , l_ctx_in->addr_sets,
>   l_ctx_in->port_groups,
> - _sets_ref, _groups_ref, );
> + _sets_ref, _groups_ref,
> + lflow->logical_datapath->tunnel_key,
> + );
>  const char *addr_set_name;
>  SSET_FOR_EACH (addr_set_name, _sets_ref) {
>  lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_ADDRSET,
> addr_set_name,
> diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> index 21bf51c..9838251 100644
> --- a/include/ovn/expr.h
> +++ b/include/ovn/expr.h
> @@ -391,12 +391,14 @@ struct expr *expr_parse(struct lexer *, const struct
> shash *symtab,
>  const struct shash *addr_sets,
>  const struct shash *port_groups,
>  struct sset *addr_sets_ref,
> -struct sset *port_groups_ref);
> +struct sset *port_groups_ref,
> +int64_t dp_id);
>  struct expr *expr_parse_string(const char *, const struct shash *symtab,
> const struct shash *addr_sets,
> const struct shash *port_groups,
> struct sset *addr_sets_ref,
> struct sset *port_groups_ref,
> +   int64_t dp_id,
> char **errorp);
>
>  struct expr *expr_clone(struct expr *);
> diff --git a/lib/actions.c b/lib/actions.c
> index ee7c825..fc6e191 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -242,7 +242,7 @@ add_prerequisite(struct action_context *ctx, const
> char 

Re: [ovs-dev] [PATCH ovn v2] chassis.c: Add comment to SB DB transaction only when needed.

[Numan Siddique]

On Mon, Jun 29, 2020 at 5:02 PM Dumitru Ceara  wrote:

> The chassis_run() function incorrectly adds a "ovn-controller:
> registering chassis" comment to every SB transaction. This should be done
> only when the chassis record is created or updated. If nothing changes in
> the chassis record we shouldn't add useless extra information to the
> transaction request.
>
> Reported-by: Daniel Alvarez 
> Reported-at: https://bugzilla.redhat.com/1850511
> Fixes: 5344f24ecb1a ("ovn-controller: Refactor chassis.c to abstract the
> string parsing")
> Signed-off-by: Dumitru Ceara 
>

Thanks Dumitru.

I applied this patch to master.  Looking into the bug report, I think this
can be a candidate for branch-20.06.
So I applied to branch-20.06 as well.

Numan


>
> ---
> v2:
> - changed chassis TXN comment as suggested by Numan.
> ---
>  controller/chassis.c | 64
> +---
>  1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/controller/chassis.c b/controller/chassis.c
> index d619361..aa3fed0 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -489,53 +489,64 @@ chassis_get_stale_record(const struct
> sbrec_chassis_table *chassis_table,
>   * Otherwise (i.e., first time we create the record) then we check if
> there's
>   * a stale record from a previous controller run that didn't end
> gracefully
>   * and reuse it. If not then we create a new record.
> + *
> + * Sets '*chassis_rec' to point to the local chassis record.
> + * Returns true if this record was already in the database, false if it
> was
> + * just inserted.
>   */
> -static const struct sbrec_chassis *
> +static bool
>  chassis_get_record(struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index *sbrec_chassis_by_name,
> const struct sbrec_chassis_table *chassis_table,
> const struct ovs_chassis_cfg *ovs_cfg,
> -   const char *chassis_id)
> +   const char *chassis_id,
> +   const struct sbrec_chassis **chassis_rec)
>  {
> -const struct sbrec_chassis *chassis_rec;
> -
>  if (chassis_info_id_inited(_state)) {
> -chassis_rec = chassis_lookup_by_name(sbrec_chassis_by_name,
> -
>  chassis_info_id(_state));
> -if (!chassis_rec) {
> +*chassis_rec = chassis_lookup_by_name(sbrec_chassis_by_name,
> +
> chassis_info_id(_state));
> +if (!(*chassis_rec)) {
>  VLOG_DBG("Could not find Chassis, will create it"
>   ": stored (%s) ovs (%s)",
>   chassis_info_id(_state), chassis_id);
>  if (ovnsb_idl_txn) {
>  /* Recreate the chassis record.  */
> -chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> +*chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> +return false;
>  }
>  }
>  } else {
> -chassis_rec =
> +*chassis_rec =
>  chassis_get_stale_record(chassis_table, ovs_cfg, chassis_id);
>
> -if (!chassis_rec && ovnsb_idl_txn) {
> -chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> +if (!(*chassis_rec) && ovnsb_idl_txn) {
> +*chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> +return false;
>  }
>  }
> -return chassis_rec;
> +return true;
>  }
>
> -/* Update a Chassis record based on the config in the ovs config. */
> -static void
> +/* Update a Chassis record based on the config in the ovs config.
> + * Returns true if 'chassis_rec' was updated, false otherwise.
> + */
> +static bool
>  chassis_update(const struct sbrec_chassis *chassis_rec,
> struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct ovs_chassis_cfg *ovs_cfg,
> const char *chassis_id,
> const struct sset *transport_zones)
>  {
> +bool updated = false;
> +
>  if (strcmp(chassis_id, chassis_rec->name)) {
>  sbrec_chassis_set_name(chassis_rec, chassis_id);
> +updated = true;
>  }
>
>  if (strcmp(ovs_cfg->hostname, chassis_rec->hostname)) {
>  sbrec_chassis_set_hostname(chassis_rec, ovs_cfg->hostname);
> +updated = true;
>  }
>
>  if (chassis_other_config_changed(ovs_cfg->bridge_mappings,
> @@ -561,6 +572,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
>   * systems, this behavior should be removed in the future. */
>  sbrec_chassis_set_external_ids(chassis_rec, _config);
>  smap_destroy(_config);
> +updated = true;
>  }
>
>  update_chassis_transport_zones(transport_zones, chassis_rec);
> @@ -571,7 +583,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
>  _cfg->encap_ip_set,
> ovs_cfg->encap_csum,
>  chassis_rec);
>  if (!tunnels_changed) {
> -return;
> +return updated;
> 

Re: [ovs-dev] [PATCH v3 3/4] netdev-offload: Add xdp flow api provider

[Toshiaki Makita]

On 2020/06/30 1:17, 0-day Robot wrote:

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

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


checkpatch:
WARNING: Comment with 'xxx' marker
#252 FILE: lib/netdev-afxdp.c:329:
 /* XXX: close output_map_fd somewhere? */

ERROR: Improper whitespace around control block
#734 FILE: lib/netdev-offload-xdp.c:258:
 FLOWMAP_FOR_EACH_INDEX(idx, mf->map) {


Adding a whitespace like

 FLOWMAP_FOR_EACH_INDEX (idx, mf->map) {

fixes the error, but as far as I can see, all existing usage of this macro
does not have this kind of whitespace.

Which is correct, need whitespace or not?

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