[ovs-dev] Loan Application Breakdown
Good Day, Kindly Find Attached Files And Send Your Documents Back To Us. Apply With Us On Our 5% Interest Rate, We Offer for all categories. Personal, Home, Debt Consolidation And Business Loans. Even thou you are blacklisted or under debt review. Legal Registration No. : 2014/238085/07 Regards, Mrs. Paula Rigt Office Line: +27 679 616 466 Emails: expressloan2...@outlook.com And expressl...@webmail.co.za ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH RFC ovn] Add VXLAN support for non-VTEP datapath bindings
Hello,
this patch is not ready, sending it to collect initial feedback on the
path taken. Let me know.
Because of limited space in VXLAN VNI to pass over all three of -
datapath id, ingress port, egress port - the implementation ignores
ingress; and splits the remaining 24 bits of VNI into two chunks, 12
bits each - one for datapath and one for egress port.
Limitations: because ingress port is not passed, ACLs that rely on it
won't work with VXLAN; reduced number of networks and ports per
network (max 4096 for both).
Renamed MLF_RCV_FROM_VXLAN_BIT into MLF_RCV_FROM_VTEP_BIT to reflect
the new use case.
TODO:
* limit maximum number of networks / ports per network for vxlan
datapaths.
* forbid acls matching against ingress port for vxlan datapaths.
* update test suite.
* update documentation.
Signed-off-by: Ihar Hrachyshka
---
controller/physical.c| 81 ++--
include/ovn/logical-fields.h | 10 ++---
2 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/controller/physical.c b/controller/physical.c
index 144aeb7bd..28f639480 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -180,7 +180,8 @@ static void
put_encapsulation(enum mf_field_id mff_ovn_geneve,
const struct chassis_tunnel *tun,
const struct sbrec_datapath_binding *datapath,
- uint16_t outport, struct ofpbuf *ofpacts)
+ uint16_t outport, bool is_vtep,
+ struct ofpbuf *ofpacts)
{
if (tun->type == GENEVE) {
put_load(datapath->tunnel_key, MFF_TUN_ID, 0, 24, ofpacts);
@@ -191,7 +192,10 @@ put_encapsulation(enum mf_field_id mff_ovn_geneve,
MFF_TUN_ID, 0, 64, ofpacts);
put_move(MFF_LOG_INPORT, 0, MFF_TUN_ID, 40, 15, ofpacts);
} else if (tun->type == VXLAN) {
-put_load(datapath->tunnel_key, MFF_TUN_ID, 0, 24, ofpacts);
+uint64_t vni = (is_vtep?
+datapath->tunnel_key :
+datapath->tunnel_key | ((uint64_t) outport << 12));
+put_load(vni, MFF_TUN_ID, 0, 24, ofpacts);
} else {
OVS_NOT_REACHED();
}
@@ -323,8 +327,9 @@ put_remote_port_redirect_overlay(const struct
if (!rem_tun) {
return;
}
-put_encapsulation(mff_ovn_geneve, tun, binding->datapath,
- port_key, ofpacts_p);
+put_encapsulation(mff_ovn_geneve, tun, binding->datapath, port_key,
+ !strcmp(binding->type, "vtep"),
+ ofpacts_p);
/* Output to tunnel. */
ofpact_put_OUTPUT(ofpacts_p)->port = rem_tun->ofport;
} else {
@@ -360,8 +365,9 @@ put_remote_port_redirect_overlay(const struct
return;
}
-put_encapsulation(mff_ovn_geneve, tun, binding->datapath,
- port_key, ofpacts_p);
+put_encapsulation(mff_ovn_geneve, tun, binding->datapath, port_key,
+ !strcmp(binding->type, "vtep"),
+ ofpacts_p);
/* Output to tunnels with active/backup */
struct ofpact_bundle *bundle = ofpact_put_BUNDLE(ofpacts_p);
@@ -1364,7 +1370,7 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
if (!prev || tun->type != prev->type) {
put_encapsulation(mff_ovn_geneve, tun, mc->datapath,
- mc->tunnel_key, &remote_ofpacts);
+ mc->tunnel_key, true, &remote_ofpacts);
prev = tun;
}
ofpact_put_OUTPUT(&remote_ofpacts)->port = tun->ofport;
@@ -1609,11 +1615,12 @@ physical_run(struct physical_ctx *p_ctx,
* Process packets that arrive from a remote hypervisor (by matching
* on tunnel in_port). */
-/* Add flows for Geneve and STT encapsulations. These
- * encapsulations have metadata about the ingress and egress logical
- * ports. We set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and
- * MFF_LOG_OUTPORT from the tunnel key data, then resubmit to table
- * 33 to handle packets to the local hypervisor. */
+/* Add flows for Geneve, STT and non-VTEP VXLAN encapsulations. Geneve and
+ * STT encapsulations have metadata about the ingress and egress logical
+ * ports. Non-VTEP VXLAN encapsulations have metadata about the egress
+ * logical port only. We set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and
+ * MFF_LOG_OUTPORT from the tunnel key data where possible, then resubmit
+ * to table 33 to handle packets to the local hypervisor. */
HMAP_FOR_EACH (tun, hmap_node, &tunnels) {
struct match match = MATCH_CATCHALL_INITIALIZER;
match_set_in_port(&match, tun->ofport);
@@ -1642,11 +1649,7 @@ physical_run(struct physical_ctx *p_ctx,
&ofpacts, hc_uuid);
}
-/* Add flows for VXLAN encapsulations. Due to the limited amount of
- * metadata, we only suppor
Re: [ovs-dev] [PATCH v2 3/3] travis: Enable OvS Travis CI for arm
Hi Ilya, The patch is updated for reducing Arm CI running time. Do you think if the time increment is acceptable? https://patchwork.ozlabs.org/patch/1258100/ Best Regards, Wei Yanqin > -Original Message- > From: Yanqin Wei > Sent: Thursday, March 12, 2020 5:39 PM > To: Ilya Maximets ; ovs-dev@openvswitch.org; > b...@ovn.org; Lance Yang > Cc: dwil...@us.ibm.com; Gavin Hu ; Ruifeng Wang > ; Jieqiang Wang ; > Malvika Gupta ; nd > Subject: RE: [ovs-dev] [PATCH v2 3/3] travis: Enable OvS Travis CI for arm > > Thanks for the feedback. Replied in line. > > > -Original Message- > > From: Ilya Maximets > > Sent: Thursday, March 12, 2020 12:20 AM > > To: Yanqin Wei ; Ilya Maximets > > ; ovs-dev@openvswitch.org; b...@ovn.org; Lance > Yang > > > > Cc: dwil...@us.ibm.com; Gavin Hu ; Ruifeng Wang > > ; Jieqiang Wang ; > Malvika > > Gupta ; nd > > Subject: Re: [ovs-dev] [PATCH v2 3/3] travis: Enable OvS Travis CI for > > arm > > > > On 3/11/20 5:57 AM, Yanqin Wei wrote: > > > Hi Ilya, > > > > > > This patch has been in the review pipeline for some time. It runs > > > stable on > > our internal repo more than two months. > > > Could you give us some suggestion about the next action I can take > > > to speed > > up the merge of this patch? > > > > Hi. Sorry for things taking so long. > > I have this patch in my backlog for this or next week. > > > > The main concern right know is possible significant increase of the > > checking time. Are you sure that we need all the listed jobs? > > Do you expect some arm64 specific issues on the linking stage? > > I mean, maybe we could reduce number of different combinations of > "shared" > > flags. I had no chance to run this, so I don't know how much time > > these jobs really takes and what is the total time difference. > > [Yanqin] This is latest build report for x86 and arm. Sparse is disabled > here > because it has some compiling issue for both x86 and arm today. > X86 and Arm https://travis- > ci.com/github/MarcoDWei/ovs/builds/152933388?utm_medium=notification > &utm_source=email > Ran for 58 min 3 sec/ Total time 4 hrs 12 min 24 sec > X86 only https://travis- > ci.com/github/MarcoDWei/ovs/builds/152942934?utm_medium=notification > &utm_source=email > Ran for 38 min 40 sec /Total time 2 hrs 55 min 4 sec > > The total time increases around 1hr 17min for SIX new arm jobs. Running time > increases around 20 mins. Kernel datapath jobs look most time consuming > jobs. > OPTS="--disable-ssl"4 min 31 sec > KERNEL_LIST="5.0 4.20 4.19 4.18 4.17 4.16" 22 min 39 sec > KERNEL_LIST="4.15 4.14 4.9 4.4 3.16"17 min 34 sec > DPDK=1 OPTS="--enable-shared"11 min 17 sec > DPDK_SHARED=1 10 min 35 sec > DPDK_SHARED=1 OPTS="--enable-shared" 11 min 21 sec > > I agree with you to remove some shared tag combination because they have > low risk of CPU specific issues in linking stage. > Moreover we could chose a part of kernel version for ARM jobs, which could > significantly reduce total time and running time. The running time is > expected > to increase around 10 minutes. > OPTS="--disable-ssl" > KERNEL_LIST="5.0 4.20 4.16 4.9 3.16" > DPDK=1 OPTS="--enable-shared" > DPDK_SHARED=1 > > > > > Best regards, Ilya Maximets. > > > > > > > > > > Best Regards, > > > Wei Yanqin > > > > > >> -Original Message- > > >> From: Lance Yang > > >> Sent: Tuesday, January 21, 2020 9:06 AM > > >> To: Ilya Maximets ; ovs-dev@openvswitch.org > > >> Cc: b...@ovn.org; Yanqin Wei ; > > dwil...@us.ibm.com; > > >> Gavin Hu ; Ruifeng Wang > ; > > >> Jieqiang Wang ; Malvika Gupta > > >> ; nd > > >> Subject: RE: [ovs-dev] [PATCH v2 3/3] travis: Enable OvS Travis CI > > >> for arm > > >> > > >> > > >>> -Original Message- > > >>> From: Ilya Maximets > > >>> Sent: Saturday, December 7, 2019 12:39 AM > > >>> To: Lance Yang (Arm Technology China) ; ovs- > > >>> d...@openvswitch.org > > >>> Cc: i.maxim...@ovn.org; b...@ovn.org; Yanqin Wei (Arm Technology > > >>> China) ; dwil...@us.ibm.com; Gavin Hu (Arm > > >>> Technology > > >>> China) ; Ruifeng Wang (Arm Technology China) > > >>> ; Jieqiang Wang (Arm Technology China) > > >>> ; Malvika Gupta ; > > nd > > >>> > > >>> Subject: Re: [ovs-dev] [PATCH v2 3/3] travis: Enable OvS Travis CI > > >>> for arm > > >>> > > >>> On 06.12.2019 04:26, Lance Yang wrote: > > Enable part of travis jobs with gcc compiler for arm64 > > architecture > > > > 1. Add arm jobs into the matrix in .travis.yml configuration file 2. > > To enable OVS-DPDK jobs, set the build target according to > > different CPU architectures 3. Temporarily disable sparse checker > > because of static code checking failure on arm64 > > > > Successful travis build jobs report: > > https://travis-ci.org/yzyuestc/ovs/builds/621037339 > > > > Reviewed-by: Yanqin Wei > > Reviewed-by: Jieqiang
[ovs-dev] Ranking within the Megaflow Cache
Hello all, I was looking into the ranking within the Megaflow Cache, especially this patch https://patchwork.ozlabs.org/patch/648896/ which is implemented for the DPDK userspace datapath in the lib/dpif-netdev.c file. As this increases the performance by improving the linear search function, I was wondering if a similar ranking is implemented within the kernel datapath as well. I wasn't able to find the relevant source code for it, if possible can someone point me towards it (if the ranking is implemented at all in this case)? Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] 答复: 答复: [PATCH v7] Use TPACKET_V3 to accelerate veth for userspace datapath
William, this is just a simple experiment, I'm still trying other ideas to get higher performance improvement, final patch is for Linux kernel net tree, not for ovs. -邮件原件- 发件人: William Tu [mailto:u9012...@gmail.com] 发送时间: 2020年3月19日 22:53 收件人: Yi Yang (杨燚)-云服务集团 抄送: i.maxim...@ovn.org; yang_y...@163.com; ovs-dev@openvswitch.org 主题: Re: [ovs-dev] 答复: [PATCH v7] Use TPACKET_V3 to accelerate veth for userspace datapath On Wed, Mar 18, 2020 at 8:12 PM Yi Yang (杨燚)-云服务集团 wrote: > > Hi, folks > > As I said, TPACKET_V3 does have kernel implementation issue, I tried to fix > it in Linux kernel 5.5.9, here is my test data with tpacket_v3 and tso > enabled. On my low end server, my goal is to reach 16Gbps at least, I still > have another idea to improve it. > Can you share your kernel fix? Or have you sent patch somewhere? William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ovs-vswitchd is dropping packets injected by controller during upcall
On Fri, Mar 13, 2020 at 04:14:33PM +0530, Numan Siddique wrote: > Hi Ben, > > Its regarding the issue I mentioned in yesterday's meeting. > > What's the issue: > --- > - ovs-vswitchd is dropping packet during upcall - if the packet was > generated by ovn-controller (or any controller) with in_port set to > OFPP_CONTROLLER. > - This normally happens when there are ct actions in the flow pipeline. Thanks very much for the detailed description and especially for the test case, which worked nicely in the sandbox for me. I posted a fix: https://patchwork.ozlabs.org/patch/1258552/ I'd appreciate testing and review. Thanks again, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto-dpif-xlate: Fix recirculation when in_port is OFPP_CONTROLLER.
Recirculation usually requires finding the pre-recirculation input port.
Packets sent by the controller, with in_port of OFPP_CONTROLLER or
OFPP_NONE, do not have a real input port data structure, only a port
number. The code in xlate_lookup_ofproto_() mishandled this case,
failing to return the ofproto data structure. This commit fixes the
problem and adds a test to guard against regression.
Reported-by: Numan Siddique
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2020-March/368642.html
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif-xlate.c | 25 +
tests/ofproto-dpif.at| 30 ++
2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index adf57a5e8929..28dcc67ddac3 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1520,15 +1520,32 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer,
return NULL;
}
-/* If recirculation was initiated due to bond (in_port = OFPP_NONE)
- * then frozen state is static and xport_uuid is not defined, so xport
- * cannot be restored from frozen state. */
-if (recirc_id_node->state.metadata.in_port != OFPP_NONE) {
+ofp_port_t in_port = recirc_id_node->state.metadata.in_port;
+if (in_port != OFPP_NONE && in_port != OFPP_CONTROLLER) {
struct uuid xport_uuid = recirc_id_node->state.xport_uuid;
xport = xport_lookup_by_uuid(xcfg, &xport_uuid);
if (xport && xport->xbridge && xport->xbridge->ofproto) {
goto out;
}
+} else {
+/* OFPP_NONE and OFPP_CONTROLLER are not real ports. They indicate
+ * that the packet originated from the controller via an OpenFlow
+ * "packet-out". The right thing to do is to find just the
+ * ofproto. There is no xport, which is OK.
+ *
+ * OFPP_NONE can also indicate that a bond caused recirculation. */
+struct uuid uuid = recirc_id_node->state.ofproto_uuid;
+const struct xbridge *bridge = xbridge_lookup_by_uuid(xcfg, &uuid);
+if (bridge && bridge->ofproto) {
+if (errorp) {
+*errorp = NULL;
+}
+*xportp = NULL;
+if (ofp_in_port) {
+*ofp_in_port = in_port;
+}
+return bridge->ofproto;
+}
}
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ff1cc93707b8..d444cf0844a9 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5171,6 +5171,36 @@ AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath
actions: 2
OVS_VSWITCHD_STOP
AT_CLEANUP
+# Checks for regression against a bug in which OVS dropped packets
+# with in_port=CONTROLLER when they were recirculated (because
+# CONTROLLER isn't a real port and could not be looked up).
+AT_SETUP([ofproto-dpif - packet-out recirculation])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2
+
+AT_DATA([flows.txt], [dnl
+table=0 ip actions=mod_dl_dst:83:83:83:83:83:83,ct(table=1)
+table=1 ip actions=ct(commit),output:2
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+packet=0010203040500800451c40110a0200350008
+AT_CHECK([ovs-ofctl packet-out br0 "in_port=controller packet=$packet
actions=table"])
+
+# Dumps out the flow table, extracts the number of packets that have gone
+# through the (single) flow in table 1, and returns success if it's exactly 1.
+#
+# If this remains 0, then the recirculation isn't working properly since the
+# packet never goes through flow in table 1.
+check_flows () {
+n=$(ovs-ofctl dump-flows br0 table=1 | sed -n
's/.*n_packets=\([[0-9]]\{1,\}\).*/\1/p')
+echo "n_packets=$n"
+test "$n" = 1
+}
+OVS_WAIT_UNTIL([check_flows], [ovs dump-flows br0])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
AT_SETUP([ofproto-dpif - debug_slow action])
OVS_VSWITCHD_START
--
2.24.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Fix NULL pointer dereference.
On Thu, Mar 19, 2020 at 08:33:24PM +0100, Dumitru Ceara wrote: > On 3/18/20 12:12 AM, William Tu wrote: > > Coverity CID 279957 reports NULL pointer derefence when > > 'conn' is NULL and calling ct_print_conn_info. > > > > Cc: Usman Ansari > > Signed-off-by: William Tu > > Acked-by: Dumitru Ceara > > Thanks, > Dumitru Thanks Applied to master and 2.13 Willam ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: Fix typo in manpage fragment.
Bleep bloop. Greetings Ben Pfaff, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 110 characters long (recommended limit is 79) #22 FILE: ofproto/ofproto-unixctl.man:12: .IQ "\fBofproto/trace\-packet\-out\fR [\fIoptions\fR] \fIbridge\fR \fIbr_flow\fR [\fIpacket\fR] \fIactions\fR" Lines checked: 28, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] Documentation: Add note about iproute2 requirements for check-kmod
On Wed, Mar 11, 2020 at 10:49:17AM -0700, Greg Rose wrote: > On many systems the check-kmod and check-kernel test suites have > many failures due to the lack of feature support in the older > iproute2 utility packages shipped with those systems. Add a > note indicating that it might be necessary to update the iproute2 > utility package in order to fix those errors. > > Signed-off-by: Greg Rose > --- Applied to master, thanks. William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv8 2/2] tests: Add tests using tap device.
Similar to using veth across namespaces, this patch creates tap devices, assigns to namespaces, and allows traffic to go through different test cases. Signed-off-by: William Tu --- tests/automake.mk | 1 + tests/system-tap.at | 34 ++ tests/system-tso-testsuite.at | 1 + 3 files changed, 36 insertions(+) create mode 100644 tests/system-tap.at diff --git a/tests/automake.mk b/tests/automake.mk index 66859d5377c3..cbba5b170427 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -158,6 +158,7 @@ SYSTEM_USERSPACE_TESTSUITE_AT = \ SYSTEM_TSO_TESTSUITE_AT = \ tests/system-tso-testsuite.at \ + tests/system-tap.at \ tests/system-tso-macros.at SYSTEM_AFXDP_TESTSUITE_AT = \ diff --git a/tests/system-tap.at b/tests/system-tap.at new file mode 100644 index ..871a3bda4fcc --- /dev/null +++ b/tests/system-tap.at @@ -0,0 +1,34 @@ +AT_SETUP([traffic between namespaces using tap]) +AT_KEYWORDS([http_tap]) +OVS_TRAFFIC_VSWITCHD_START() +AT_SKIP_IF([test $HAVE_TUNCTL = no]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +AT_CHECK([ip tuntap add tap0 mode tap]) +on_exit 'ip tuntap del tap0 mode tap' +AT_CHECK([ip tuntap add tap1 mode tap]) +on_exit 'ip tuntap del tap1 mode tap' + +AT_CHECK([ovs-vsctl add-port br0 tap0 -- set int tap0 type=tap]) +AT_CHECK([ovs-vsctl add-port br0 tap1 -- set int tap1 type=tap]) +AT_CHECK([ip link set tap0 netns at_ns0]) +AT_CHECK([ip link set tap1 netns at_ns1]) + +AT_CHECK([ip netns exec at_ns0 ip link set dev tap0 up]) +AT_CHECK([ip netns exec at_ns1 ip link set dev tap1 up]) +AT_CHECK([ip netns exec at_ns0 ip addr add 10.1.1.1/24 dev tap0]) +AT_CHECK([ip netns exec at_ns1 ip addr add 10.1.1.2/24 dev tap1]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +OVS_START_L7([at_ns1], [http]) +NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log]) + +OVS_TRAFFIC_VSWITCHD_STOP(["/.*ethtool command ETHTOOL_G.*/d"]) + +AT_CLEANUP diff --git a/tests/system-tso-testsuite.at b/tests/system-tso-testsuite.at index 99d748006a86..594d1a6fde85 100644 --- a/tests/system-tso-testsuite.at +++ b/tests/system-tso-testsuite.at @@ -23,4 +23,5 @@ m4_include([tests/system-common-macros.at]) m4_include([tests/system-userspace-macros.at]) m4_include([tests/system-tso-macros.at]) +m4_include([tests/system-tap.at]) m4_include([tests/system-traffic.at]) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv8 1/2] userspace: Enable TSO support for non-DPDK.
This patch enables TSO support for non-DPDK use cases, and
also add check-system-tso testsuite. Before TSO, we have to
disable checksum offload, allowing the kernel to calculate the
TCP/UDP packet checsum. With TSO, we can skip the checksum
validation by enabling checksum offload, and with large packet
size, we see better performance.
Consider container to container use cases:
iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1)
And I got around 6Gbps, similar to TSO with DPDK-enabled.
Signed-off-by: William Tu
---
v8:
- make some namings more clear
v7: more refactoring on functions
- rss and flow mark related functions.
- dp_packet_clone_with_headroom
- fix definitions of DP_PACKET_OL_FLOW_MARK_MASK
- travis: https://travis-ci.org/github/williamtu/ovs-travis/builds/663658338
v6: fix indentation
v5: feedback from Flavio
- move some code around, break the long line
- travis is now working
https://travis-ci.org/github/williamtu/ovs-travis/builds/661607017
v4:
- Avoid duplications of DPDK and non-DPDK code by
refactoring the definition of DP_PACKET_OL flags
and relevant functions
- I got weird error in travis (I think this is unrelated)
https://travis-ci.org/github/williamtu/ovs-travis/builds/661313463
sindex.c:378:26: error: unknown type name ‘sqlite3_str’
static int query_appendf(sqlite3_str *query, const char *fmt, ...)
- test compile ok on dpdk and non-dpdk, make check-system-tso still
has a couple fails
v3:
- fix comments and some coding style
- add valgrind check
- travis: https://travis-ci.org/williamtu/ovs-travis/builds/660394007
v2:
- add make check-system-tso test
- combine logging for dpdk and non-dpdk
- I'm surprised that most of the test cases passed.
This is due to few tests using tcp/udp, so it does not trigger
TSO. I saw only geneve/vxlan fails randomly, maybe we can
check it later.
---
lib/dp-packet.c | 6 +-
lib/dp-packet.h | 566 +++---
lib/userspace-tso.c | 5 -
tests/.gitignore | 3 +
tests/automake.mk | 21 ++
tests/system-tso-macros.at| 31 +++
tests/system-tso-testsuite.at | 26 ++
7 files changed, 333 insertions(+), 325 deletions(-)
create mode 100644 tests/system-tso-macros.at
create mode 100644 tests/system-tso-testsuite.at
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index cd2623500e3d..e154ac13e4f2 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -192,10 +192,8 @@ dp_packet_clone_with_headroom(const struct dp_packet
*buffer, size_t headroom)
sizeof(struct dp_packet) -
offsetof(struct dp_packet, l2_pad_size));
-#ifdef DPDK_NETDEV
-new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
-new_buffer->mbuf.ol_flags &= ~DPDK_MBUF_NON_OFFLOADING_FLAGS;
-#endif
+*dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
+*dp_packet_ol_flags_ptr(new_buffer) &= ~DP_PACKET_OL_NONE_MASK;
if (dp_packet_rss_valid(buffer)) {
dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 9f8991faad52..ed277719c15e 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -46,20 +46,57 @@ enum OVS_PACKED_ENUM dp_packet_source {
DPBUF_AFXDP, /* Buffer data from XDP frame. */
};
+#ifdef DPDK_NETDEV
+/* DPDK mbuf ol_flags that are not really an offload flags. These are mostly
+ * related to mbuf memory layout and OVS should not touch/clear them. */
+#define DPDK_MBUF_NON_OFFLOADING_MASK (EXT_ATTACHED_MBUF | \
+ IND_ATTACHED_MBUF)
+#endif
+
#define DP_PACKET_CONTEXT_SIZE 64
+#ifdef DPDK_NETDEV
+#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = DPDK_DEF
+#else
+#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = GENERIC_DEF
+#endif
-#ifndef DPDK_NETDEV
/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
enum dp_packet_offload_mask {
-DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */
-DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */
+/* Reset offload. */
+DEF_OL_FLAG(DP_PACKET_OL_NONE_MASK, DPDK_MBUF_NON_OFFLOADING_MASK, 0x0),
+/* Is the 'rss_hash' valid? */
+DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH, PKT_RX_RSS_HASH, 0x1),
+/* Is the 'flow_mark' valid? (DPDK does not support) */
+DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK, PKT_RX_FDIR_ID, 0x2),
+/* Bad L4 checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0x4),
+/* Bad IP checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_BAD, PKT_RX_IP_CKSUM_BAD, 0x8),
+/* Valid L4 checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_GOOD, PKT_RX_L4_CKSUM_GOOD, 0x10),
+/* Valid IP checksum in the packet. */
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_GOOD, PKT_RX_IP_CKSUM_GOOD, 0x20),
+/*
Re: [ovs-dev] [PATCHv7 1/2] userspace: Enable TSO support for non-DPDK.
On Thu, Mar 19, 2020 at 2:14 PM Flavio Leitner wrote:
>
>
> Hi William,
>
> Nice that the amount of specific DPDK and non-DPDK reduced a lot!
> I haven't tried to build or test yet.
>
> It may be nitpicking because you didn't introduce some of the
> names in this patch so I understand it is not the goal of your
> patch. However, I think it can take the opportunity to make it
> clear and uniform. See below.
>
Hi Flavio,
No problem, thanks for your review.
> On Tue, Mar 17, 2020 at 01:02:08PM -0700, William Tu wrote:
> [...]
> > --- a/lib/dp-packet.h
> > +++ b/lib/dp-packet.h
> > @@ -46,21 +46,58 @@ enum OVS_PACKED_ENUM dp_packet_source {
> > DPBUF_AFXDP, /* Buffer data from XDP frame. */
> > };
> >
> > -#define DP_PACKET_CONTEXT_SIZE 64
> > -
> > -#ifndef DPDK_NETDEV
> > -/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
> > -enum dp_packet_offload_mask {
> > -DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */
> > -DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */
> > -};
> > -#else
> > +#ifdef DPDK_NETDEV
> > /* DPDK mbuf ol_flags that are not really an offload flags. These are
> > mostly
> > * related to mbuf memory layout and OVS should not touch/clear them. */
> > #define DPDK_MBUF_NON_OFFLOADING_FLAGS (EXT_ATTACHED_MBUF | \
> > IND_ATTACHED_MBUF)
>
> That should end with _MASK (multiple bits) as others.
OK.
>
> > #endif
> >
> > +#define DP_PACKET_CONTEXT_SIZE 64
> > +#ifdef DPDK_NETDEV
> > +#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = DPDK_DEF
> > +#else
> > +#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = GENERIC_DEF
> > +#endif
> > +
> > +/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
> > +enum dp_packet_offload_mask {
> > +/* Reset offload. */
> > +DEF_OL_FLAG(DP_PACKET_OL_NONE, DPDK_MBUF_NON_OFFLOADING_FLAGS, 0x0),
>
> Same issue here: I think it could be DP_PACKET_OL_NONE_MASK, otherwise it
> is easy to check as a bit (var & DP_PACKET_OL_NONE) which will return true
> even though the bit mask would not be same, for example:
> ((var & DP_PACKET_OL_MASK) == DP_PACKET_OL_MASK)
Yes.
>
> > +/* Is the 'rss_hash' valid? */
> > +DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH_MASK, PKT_RX_RSS_HASH, 0x1),
>
> The RSS above is a bit and not a MASK, so the name is misleading.
OK
>
> > +/* Is the 'flow_mark' valid? (DPDK does not support) */
> > +DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK_MASK, PKT_RX_FDIR_ID, 0x2),
>
> Here as well, a misleading name.
> What do you think?
OK I will remove the _MASK here also.
Thanks
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] Documentation: Add extra repo info for RHEL 8
On 3/19/2020 4:03 PM, 0-day Robot wrote: Bleep bloop. Greetings Gregory Rose, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 82 characters long (recommended limit is 79) #22 FILE: Documentation/intro/install/fedora.rst:73: $ subscription-manager repos --enable=codeready-builder-for-rhel-8-x86_64-rpms Lines checked: 29, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot I suppose I could put a break in the command line. I'll do that and send V2. - Greg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Ok sounds good. Will do that. Thanks a ton! On Thu, Mar 19, 2020 at 3:53 PM Ben Pfaff wrote: > I backported both to 2.12. > > They need a manual backport to 2.11, will you take care of it? > > On Thu, Mar 19, 2020 at 03:39:58PM -0700, aginwala wrote: > > Oh I see it seems the previous patch needs to be backported too > > > https://lists.linuxfoundation.org/pipermail/ovs-dev/2019-September/362925.html > > . > > Please see if you can get that too on 2.11 and 2.12 > > > > > > > > > > On Thu, Mar 19, 2020 at 3:35 PM Ben Pfaff wrote: > > > > > It doesn't apply cleanly. > > > > > > On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > > > > Hi Ben: > > > > > > > > Thanks for backporting previous patches. Please see if you can back > port > > > > this one too to 2.11 and 2.12. > > > > > > > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > > > > > > > > > Hi Ben: > > > > > Can you also backport this patch to 2.12 and 2.11 too? > > > > > > > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > > > > > > > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > > > > >> > memory leak is reported by valgrind while executing functional > test > > > > >> > "ovsdb-tool convert-to-standalone" > > > > >> > > > > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks > are > > > > >> definitely lost in loss record 20 of 20 > > > > >> > ==13842==at 0x4C2DB8F: malloc (in > > > > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > > > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > > > > >> > ==13842==by 0x43E386: json_create (json.c:1451) > > > > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > > > > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > > > > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > > > > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > > > > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > > > > >> > ==13842==by 0x40D108: parse_body (log.c:411) > > > > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > > > > >> > ==13842==by 0x406A0B: do_convert_to_standalone > > > (ovsdb-tool.c:1571) > > > > >> > ==13842==by 0x406A0B: do_cluster_standalone > (ovsdb-tool.c:1606) > > > > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ > > > (command-line.c:223) > > > > >> > ==13842==by 0x438720: ovs_cmdl_run_command > (command-line.c:254) > > > > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > > > > >> > > > > > >> > The problem was in do_convert_to_standalone() function which > while > > > > >> reading log file > > > > >> > allocate json object which was not deallocated at the end. > > > > >> > > > > > >> > Signed-off-by: Damijan Skvarc > > > > >> > > > > >> Applied to master, thanks. > > > > >> ___ > > > > >> dev mailing list > > > > >> d...@openvswitch.org > > > > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > >> > > > > > > > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] Documentation: Add extra repo info for RHEL 8
Bleep bloop. Greetings Gregory Rose, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 82 characters long (recommended limit is 79) #22 FILE: Documentation/intro/install/fedora.rst:73: $ subscription-manager repos --enable=codeready-builder-for-rhel-8-x86_64-rpms Lines checked: 29, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto: Fix typo in manpage fragment.
There was a missing ] and an extra space. Signed-off-by: Ben Pfaff --- ofproto/ofproto-unixctl.man | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofproto/ofproto-unixctl.man b/ofproto/ofproto-unixctl.man index 925752343e87..095afd57cc55 100644 --- a/ofproto/ofproto-unixctl.man +++ b/ofproto/ofproto-unixctl.man @@ -9,7 +9,7 @@ that may be used on \fBofproto/trace\fR. .IP "\fBofproto/trace\fR [\fIoptions\fR] [\fIdpname\fR] \fIodp_flow\fR [\fIpacket\fR] .IQ "\fBofproto/trace\fR [\fIoptions\fR] \fIbridge\fR \fIbr_flow\fR [\fIpacket\fR]] .IQ "\fBofproto/trace\-packet\-out\fR [\fIoptions\fR] [\fIdpname\fR] \fIodp_flow\fR [\fIpacket\fR] \fIactions\fR" -.IQ "\fBofproto/trace\-packet\-out\fR [\fIoptions\fR \fIbridge\fR \fIbr_flow\fR [\fIpacket\fR] \fIactions\fR" +.IQ "\fBofproto/trace\-packet\-out\fR [\fIoptions\fR] \fIbridge\fR \fIbr_flow\fR [\fIpacket\fR] \fIactions\fR" Traces the path of an imaginary packet through \fIswitch\fR and reports the path that it took. The initial treatment of the packet varies based on the command: -- 2.24.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
I backported both to 2.12. They need a manual backport to 2.11, will you take care of it? On Thu, Mar 19, 2020 at 03:39:58PM -0700, aginwala wrote: > Oh I see it seems the previous patch needs to be backported too > https://lists.linuxfoundation.org/pipermail/ovs-dev/2019-September/362925.html > . > Please see if you can get that too on 2.11 and 2.12 > > > > > On Thu, Mar 19, 2020 at 3:35 PM Ben Pfaff wrote: > > > It doesn't apply cleanly. > > > > On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > > > Hi Ben: > > > > > > Thanks for backporting previous patches. Please see if you can back port > > > this one too to 2.11 and 2.12. > > > > > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > > > > > > > Hi Ben: > > > > Can you also backport this patch to 2.12 and 2.11 too? > > > > > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > > > > > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > > > >> > memory leak is reported by valgrind while executing functional test > > > >> > "ovsdb-tool convert-to-standalone" > > > >> > > > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are > > > >> definitely lost in loss record 20 of 20 > > > >> > ==13842==at 0x4C2DB8F: malloc (in > > > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > > > >> > ==13842==by 0x43E386: json_create (json.c:1451) > > > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > > > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > > > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > > > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > > > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > > > >> > ==13842==by 0x40D108: parse_body (log.c:411) > > > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > > > >> > ==13842==by 0x406A0B: do_convert_to_standalone > > (ovsdb-tool.c:1571) > > > >> > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) > > > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ > > (command-line.c:223) > > > >> > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > > > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > > > >> > > > > >> > The problem was in do_convert_to_standalone() function which while > > > >> reading log file > > > >> > allocate json object which was not deallocated at the end. > > > >> > > > > >> > Signed-off-by: Damijan Skvarc > > > >> > > > >> Applied to master, thanks. > > > >> ___ > > > >> dev mailing list > > > >> d...@openvswitch.org > > > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > >> > > > > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Ok sure, will do in a bit! On Thu, Mar 19, 2020 at 3:40 PM Ben Pfaff wrote: > There is a merge conflict. Post a backported version? > > On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > > Hi Ben: > > > > Thanks for backporting previous patches. Please see if you can back port > > this one too to 2.11 and 2.12. > > > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > > > > > Hi Ben: > > > Can you also backport this patch to 2.12 and 2.11 too? > > > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > > > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > > >> > memory leak is reported by valgrind while executing functional test > > >> > "ovsdb-tool convert-to-standalone" > > >> > > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are > > >> definitely lost in loss record 20 of 20 > > >> > ==13842==at 0x4C2DB8F: malloc (in > > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > > >> > ==13842==by 0x43E386: json_create (json.c:1451) > > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > > >> > ==13842==by 0x40D108: parse_body (log.c:411) > > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > > >> > ==13842==by 0x406A0B: do_convert_to_standalone > (ovsdb-tool.c:1571) > > >> > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) > > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ > (command-line.c:223) > > >> > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > > >> > > > >> > The problem was in do_convert_to_standalone() function which while > > >> reading log file > > >> > allocate json object which was not deallocated at the end. > > >> > > > >> > Signed-off-by: Damijan Skvarc > > >> > > >> Applied to master, thanks. > > >> ___ > > >> dev mailing list > > >> d...@openvswitch.org > > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >> > > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Oh I see it seems the previous patch needs to be backported too https://lists.linuxfoundation.org/pipermail/ovs-dev/2019-September/362925.html . Please see if you can get that too on 2.11 and 2.12 On Thu, Mar 19, 2020 at 3:35 PM Ben Pfaff wrote: > It doesn't apply cleanly. > > On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > > Hi Ben: > > > > Thanks for backporting previous patches. Please see if you can back port > > this one too to 2.11 and 2.12. > > > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > > > > > Hi Ben: > > > Can you also backport this patch to 2.12 and 2.11 too? > > > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > > > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > > >> > memory leak is reported by valgrind while executing functional test > > >> > "ovsdb-tool convert-to-standalone" > > >> > > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are > > >> definitely lost in loss record 20 of 20 > > >> > ==13842==at 0x4C2DB8F: malloc (in > > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > > >> > ==13842==by 0x43E386: json_create (json.c:1451) > > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > > >> > ==13842==by 0x40D108: parse_body (log.c:411) > > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > > >> > ==13842==by 0x406A0B: do_convert_to_standalone > (ovsdb-tool.c:1571) > > >> > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) > > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ > (command-line.c:223) > > >> > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > > >> > > > >> > The problem was in do_convert_to_standalone() function which while > > >> reading log file > > >> > allocate json object which was not deallocated at the end. > > >> > > > >> > Signed-off-by: Damijan Skvarc > > >> > > >> Applied to master, thanks. > > >> ___ > > >> dev mailing list > > >> d...@openvswitch.org > > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >> > > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
There is a merge conflict. Post a backported version? On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > Hi Ben: > > Thanks for backporting previous patches. Please see if you can back port > this one too to 2.11 and 2.12. > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > > > Hi Ben: > > Can you also backport this patch to 2.12 and 2.11 too? > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > >> > memory leak is reported by valgrind while executing functional test > >> > "ovsdb-tool convert-to-standalone" > >> > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are > >> definitely lost in loss record 20 of 20 > >> > ==13842==at 0x4C2DB8F: malloc (in > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > >> > ==13842==by 0x43E386: json_create (json.c:1451) > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > >> > ==13842==by 0x40D108: parse_body (log.c:411) > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > >> > ==13842==by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571) > >> > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223) > >> > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > >> > > >> > The problem was in do_convert_to_standalone() function which while > >> reading log file > >> > allocate json object which was not deallocated at the end. > >> > > >> > Signed-off-by: Damijan Skvarc > >> > >> Applied to master, thanks. > >> ___ > >> dev mailing list > >> d...@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
It doesn't apply cleanly. On Thu, Mar 19, 2020 at 03:07:39PM -0700, aginwala wrote: > Hi Ben: > > Thanks for backporting previous patches. Please see if you can back port > this one too to 2.11 and 2.12. > > On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > > > Hi Ben: > > Can you also backport this patch to 2.12 and 2.11 too? > > > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > > > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > >> > memory leak is reported by valgrind while executing functional test > >> > "ovsdb-tool convert-to-standalone" > >> > > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are > >> definitely lost in loss record 20 of 20 > >> > ==13842==at 0x4C2DB8F: malloc (in > >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) > >> > ==13842==by 0x43E386: json_create (json.c:1451) > >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) > >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) > >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > >> > ==13842==by 0x40D108: parse_body (log.c:411) > >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > >> > ==13842==by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571) > >> > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) > >> > ==13842==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223) > >> > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > >> > > >> > The problem was in do_convert_to_standalone() function which while > >> reading log file > >> > allocate json object which was not deallocated at the end. > >> > > >> > Signed-off-by: Damijan Skvarc > >> > >> Applied to master, thanks. > >> ___ > >> dev mailing list > >> d...@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] Documentation: Add note about iproute2 requirements for check-kmod
On 3/11/2020 10:52 AM, William Tu wrote: On Wed, Mar 11, 2020 at 10:49 AM Greg Rose wrote: On many systems the check-kmod and check-kernel test suites have many failures due to the lack of feature support in the older iproute2 utility packages shipped with those systems. Add a note indicating that it might be necessary to update the iproute2 utility package in order to fix those errors. Signed-off-by: Greg Rose --- LGTM Acked-by: William Tu Back to the top. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
Bleep bloop. Greetings Usman Ansari, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author Usman Ansari needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Usman Ansari WARNING: Line is 85 characters long (recommended limit is 79) #28 FILE: include/openvswitch/hmap.h:139: (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \ WARNING: Line is 85 characters long (recommended limit is 79) #34 FILE: include/openvswitch/hmap.h:144: (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \ WARNING: Line is 85 characters long (recommended limit is 79) #43 FILE: include/openvswitch/hmap.h:173: (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \ WARNING: Line is 85 characters long (recommended limit is 79) #52 FILE: include/openvswitch/hmap.h:182: ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false) \ WARNING: Line is 85 characters long (recommended limit is 79) #61 FILE: include/openvswitch/hmap.h:193: (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \ WARNING: Line is 84 characters long (recommended limit is 79) #70 FILE: include/openvswitch/hmap.h:214: (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false);) Lines checked: 76, Warnings: 7, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
On Thu, Mar 19, 2020 at 02:47:17PM -0700, ua1...@gmail.com wrote: > From: Usman Ansari > > Coverity reports a false positive below: > Incorrect expression, Assign_where_compare_meant: use of "=" > where "==" may have been intended. > Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. > "make check" passes for this change > Coverity reports over 500 errors resolved > > Suggested-by: Ben Pfaff > Signed-off-by: Usman Ansari Thanks. I folded the too-long lines, fixed the typo in your Signed-off-by line, and applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Hi Ben: Thanks for backporting previous patches. Please see if you can back port this one too to 2.11 and 2.12. On Thu, Mar 19, 2020 at 1:53 PM aginwala wrote: > Hi Ben: > Can you also backport this patch to 2.12 and 2.11 too? > > On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > >> On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: >> > memory leak is reported by valgrind while executing functional test >> > "ovsdb-tool convert-to-standalone" >> > >> > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are >> definitely lost in loss record 20 of 20 >> > ==13842==at 0x4C2DB8F: malloc (in >> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) >> > ==13842==by 0x45EE2E: xmalloc (util.c:138) >> > ==13842==by 0x43E386: json_create (json.c:1451) >> > ==13842==by 0x43BDD2: json_object_create (json.c:254) >> > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) >> > ==13842==by 0x43E167: json_parser_input (json.c:1371) >> > ==13842==by 0x43D6EA: json_lex_input (json.c:991) >> > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) >> > ==13842==by 0x40D108: parse_body (log.c:411) >> > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) >> > ==13842==by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571) >> > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) >> > ==13842==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223) >> > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) >> > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) >> > >> > The problem was in do_convert_to_standalone() function which while >> reading log file >> > allocate json object which was not deallocated at the end. >> > >> > Signed-off-by: Damijan Skvarc >> >> Applied to master, thanks. >> ___ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/2] compat: Fix nf_ip_hook parameters for RHEL 8
A RHEL release version check was only checking for RHEL releases greater than 7.0 so that ended up including a compat fixup that is not needed for 8.0. Fix up the version check. Signed-off-by: Greg Rose --- datapath/linux/compat/stt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c index 7b46d1a..8a5853f 100644 --- a/datapath/linux/compat/stt.c +++ b/datapath/linux/compat/stt.c @@ -1559,7 +1559,7 @@ static void clean_percpu(struct work_struct *work) #endif #ifdef HAVE_NF_HOOK_STATE -#if RHEL_RELEASE_CODE > RHEL_RELEASE_VERSION(7,0) +#if RHEL_RELEASE_CODE > RHEL_RELEASE_VERSION(7,0) && RHEL_RELEASE_CODE < RHEL_RELEASE_VERSION(8,0) /* RHEL nfhook hacks. */ #ifndef __GENKSYMS__ #define LAST_PARAM const struct net_device *in, const struct net_device *out, \ -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/2] Documentation: Add extra repo info for RHEL 8
The extra development repo for RHEL 8 has changed. Document it. Signed-off-by: Greg Rose --- Documentation/intro/install/fedora.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/intro/install/fedora.rst b/Documentation/intro/install/fedora.rst index 6fe1fb5..de00c4c 100644 --- a/Documentation/intro/install/fedora.rst +++ b/Documentation/intro/install/fedora.rst @@ -69,6 +69,9 @@ repositories to help yum-builddep, e.g.:: $ subscription-manager repos --enable=rhel-7-server-extras-rpms $ subscription-manager repos --enable=rhel-7-server-optional-rpms +or for RHEL 8:: +$ subscription-manager repos --enable=codeready-builder-for-rhel-8-x86_64-rpms + DNF:: $ dnf builddep /tmp/ovs.spec -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
From: Usman Ansari
Coverity reports a false positive below:
Incorrect expression, Assign_where_compare_meant: use of "="
where "==" may have been intended.
Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'.
"make check" passes for this change
Coverity reports over 500 errors resolved
Suggested-by: Ben Pfaff
Signed-off-by: Usman Ansari
---
include/openvswitch/hmap.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/openvswitch/hmap.h b/include/openvswitch/hmap.h
index 8aea9c2..74a0929 100644
--- a/include/openvswitch/hmap.h
+++ b/include/openvswitch/hmap.h
@@ -136,12 +136,12 @@ struct hmap_node *hmap_random_node(const struct hmap *);
*/
#define HMAP_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, HMAP) \
for (INIT_CONTAINER(NODE, hmap_first_with_hash(HMAP, HASH), MEMBER); \
- (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
+ (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL),
false); \
ASSIGN_CONTAINER(NODE, hmap_next_with_hash(&(NODE)->MEMBER), \
MEMBER))
#define HMAP_FOR_EACH_IN_BUCKET(NODE, MEMBER, HASH, HMAP) \
for (INIT_CONTAINER(NODE, hmap_first_in_bucket(HMAP, HASH), MEMBER); \
- (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
+ (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL),
false); \
ASSIGN_CONTAINER(NODE, hmap_next_in_bucket(&(NODE)->MEMBER), MEMBER))
static inline struct hmap_node *hmap_first_with_hash(const struct hmap *,
@@ -170,7 +170,7 @@ bool hmap_contains(const struct hmap *, const struct
hmap_node *);
HMAP_FOR_EACH_INIT(NODE, MEMBER, HMAP, (void) 0)
#define HMAP_FOR_EACH_INIT(NODE, MEMBER, HMAP, ...) \
for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER), __VA_ARGS__; \
- (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
+ (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL),
false); \
ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER))
/* Safe when NODE may be freed (not needed when NODE may be removed from the
@@ -179,7 +179,7 @@ bool hmap_contains(const struct hmap *, const struct
hmap_node *);
HMAP_FOR_EACH_SAFE_INIT(NODE, NEXT, MEMBER, HMAP, (void) 0)
#define HMAP_FOR_EACH_SAFE_INIT(NODE, NEXT, MEMBER, HMAP, ...) \
for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER), __VA_ARGS__; \
- ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL) \
+ ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL),
false) \
? INIT_CONTAINER(NEXT, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER), 1 \
: 0); \
(NODE) = (NEXT))
@@ -190,7 +190,7 @@ bool hmap_contains(const struct hmap *, const struct
hmap_node *);
#define HMAP_FOR_EACH_CONTINUE_INIT(NODE, MEMBER, HMAP, ...)\
for (ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER), \
__VA_ARGS__; \
- (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
+ (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL),
false); \
ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER))
static inline struct hmap_node *
@@ -211,7 +211,7 @@ hmap_pop_helper__(struct hmap *hmap, size_t *bucket) {
#define HMAP_FOR_EACH_POP(NODE, MEMBER, HMAP) \
for (size_t bucket__ = 0; \
INIT_CONTAINER(NODE, hmap_pop_helper__(HMAP, &bucket__), MEMBER), \
- (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL);)
+ (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL),
false);)
static inline struct hmap_node *hmap_first(const struct hmap *);
static inline struct hmap_node *hmap_next(const struct hmap *,
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while running "db-is-standalone" command
OK, done. On Thu, Mar 19, 2020 at 01:53:40PM -0700, aginwala wrote: > Hi Ben: > Can you also backport this patch to 2.12 and 2.11 too? > > On Mon, Oct 7, 2019 at 11:21 AM Ben Pfaff wrote: > > > On Mon, Oct 07, 2019 at 10:30:07AM +0200, Damijan Skvarc wrote: > > > problem is reported by valgrind while running functional tests: > > > > > > ==21043== 160 (88 direct, 72 indirect) bytes in 1 blocks are definitely > > lost in loss record 7 of 8 > > > ==21043==at 0x4C2DB8F: malloc (in > > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > > ==21043==by 0x45EE2E: xmalloc (util.c:138) > > > ==21043==by 0x40CB81: ovsdb_log_open (log.c:270) > > > ==21043==by 0x406B4F: do_db_has_magic.isra.9 (ovsdb-tool.c:563) > > > ==21043==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223) > > > ==21043==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > > > ==21043==by 0x405A4C: main (ovsdb-tool.c:79) > > > > > > problem was in do_db_has_magic() which opens log file which is never > > closed. > > > > > > Signed-off-by: Damijan Skvarc > > > > Applied to master, thanks. > > ___ > > dev mailing list > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC] github-ci: Enable github actions.
On Tue, Mar 17, 2020 at 01:27:13PM -0700, William Tu wrote: > So far we only use travis to do run 'make check' per commit. > This enables per-commit check for 'make check-system-userspace' test. > We can think about what others to add using github actions. > > Example run: > https://github.com/williamtu/ovs-travis/runs/514822181?check_suite_focus=true I like the idea, but not sure if make check-system-userspace works well in travis environment. I mean, first it should pass or it will just create noise there. The second thing is to be stable, or we will see random unrelated failures in new commits. fbl > > Signed-off-by: William Tu > --- > .github/workflows/ovs.yml | 25 + > Makefile.am | 1 + > 2 files changed, 26 insertions(+) > create mode 100644 .github/workflows/ovs.yml > > diff --git a/.github/workflows/ovs.yml b/.github/workflows/ovs.yml > new file mode 100644 > index ..1e056aceed2c > --- /dev/null > +++ b/.github/workflows/ovs.yml > @@ -0,0 +1,25 @@ > +name: OVS CI > + > +on: > + push: > +branches: [ master ] > + pull_request: > +branches: [ master ] > + > +jobs: > + build: > +runs-on: ubuntu-latest > +steps: > +- uses: actions/checkout@v2 > +- name: configure > + run: ./boot.sh; ./configure > +- name: make > + run: make -j2 > +- name: check-system-userspace > + run: sudo make check-system-userspace TESTSUITEFLAGS='1-30' RECHECK=yes > +- name: Upload artifact > + uses: actions/upload-artifact@v1.0.0 > + if: failure() > + with: > +name: system-userspace > +path: tests/system-userspace-testsuite.dir/ > diff --git a/Makefile.am b/Makefile.am > index b279303d186c..80448d0c31c1 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -92,6 +92,7 @@ EXTRA_DIST = \ > $(MAN_ROOTS) \ > Vagrantfile \ > Vagrantfile-FreeBSD \ > + .github/workflows/ovs.yml \ > .mailmap > bin_PROGRAMS = > sbin_PROGRAMS = > -- > 2.7.4 > -- fbl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv7 1/2] userspace: Enable TSO support for non-DPDK.
Hi William,
Nice that the amount of specific DPDK and non-DPDK reduced a lot!
I haven't tried to build or test yet.
It may be nitpicking because you didn't introduce some of the
names in this patch so I understand it is not the goal of your
patch. However, I think it can take the opportunity to make it
clear and uniform. See below.
On Tue, Mar 17, 2020 at 01:02:08PM -0700, William Tu wrote:
[...]
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -46,21 +46,58 @@ enum OVS_PACKED_ENUM dp_packet_source {
> DPBUF_AFXDP, /* Buffer data from XDP frame. */
> };
>
> -#define DP_PACKET_CONTEXT_SIZE 64
> -
> -#ifndef DPDK_NETDEV
> -/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
> -enum dp_packet_offload_mask {
> -DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */
> -DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */
> -};
> -#else
> +#ifdef DPDK_NETDEV
> /* DPDK mbuf ol_flags that are not really an offload flags. These are mostly
> * related to mbuf memory layout and OVS should not touch/clear them. */
> #define DPDK_MBUF_NON_OFFLOADING_FLAGS (EXT_ATTACHED_MBUF | \
> IND_ATTACHED_MBUF)
That should end with _MASK (multiple bits) as others.
> #endif
>
> +#define DP_PACKET_CONTEXT_SIZE 64
> +#ifdef DPDK_NETDEV
> +#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = DPDK_DEF
> +#else
> +#define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = GENERIC_DEF
> +#endif
> +
> +/* Bit masks for the 'ol_flags' member of the 'dp_packet' structure. */
> +enum dp_packet_offload_mask {
> +/* Reset offload. */
> +DEF_OL_FLAG(DP_PACKET_OL_NONE, DPDK_MBUF_NON_OFFLOADING_FLAGS, 0x0),
Same issue here: I think it could be DP_PACKET_OL_NONE_MASK, otherwise it
is easy to check as a bit (var & DP_PACKET_OL_NONE) which will return true
even though the bit mask would not be same, for example:
((var & DP_PACKET_OL_MASK) == DP_PACKET_OL_MASK)
> +/* Is the 'rss_hash' valid? */
> +DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH_MASK, PKT_RX_RSS_HASH, 0x1),
The RSS above is a bit and not a MASK, so the name is misleading.
> +/* Is the 'flow_mark' valid? (DPDK does not support) */
> +DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK_MASK, PKT_RX_FDIR_ID, 0x2),
Here as well, a misleading name.
What do you think?
Thanks,
fbl
> +/* Bad L4 checksum in the packet. */
> +DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0x4),
> +/* Bad IP checksum in the packet. */
> +DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_BAD, PKT_RX_IP_CKSUM_BAD, 0x8),
> +/* Valid L4 checksum in the packet. */
> +DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_GOOD, PKT_RX_L4_CKSUM_GOOD, 0x10),
> +/* Valid IP checksum in the packet. */
> +DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_GOOD, PKT_RX_IP_CKSUM_GOOD, 0x20),
> +/* TCP Segmentation Offload. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_SEG, PKT_TX_TCP_SEG, 0x40),
> +/* Offloaded packet is IPv4. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_IPV4, PKT_TX_IPV4, 0x80),
> +/* Offloaded packet is IPv6. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_IPV6, PKT_TX_IPV6, 0x100),
> +/* Offload TCP checksum. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_CKSUM, PKT_TX_TCP_CKSUM, 0x200),
> +/* Offload UDP checksum. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_CKSUM, PKT_TX_UDP_CKSUM, 0x400),
> +/* Offload SCTP checksum. */
> +DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, PKT_TX_SCTP_CKSUM, 0x800),
> +};
> +
> +#define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \
> + DP_PACKET_OL_TX_UDP_CKSUM | \
> + DP_PACKET_OL_TX_SCTP_CKSUM)
> +#define DP_PACKET_OL_RX_IP_CKSUM_MASK (DP_PACKET_OL_RX_IP_CKSUM_GOOD | \
> + DP_PACKET_OL_RX_IP_CKSUM_BAD)
> +#define DP_PACKET_OL_RX_L4_CKSUM_MASK (DP_PACKET_OL_RX_L4_CKSUM_GOOD | \
> + DP_PACKET_OL_RX_L4_CKSUM_BAD)
> +
> /* Buffer for holding packet data. A dp_packet is automatically reallocated
> * as necessary if it grows too large for the available memory.
> * By default the packet type is set to Ethernet (PT_ETH).
> @@ -451,6 +488,45 @@ dp_packet_get_nd_payload(const struct dp_packet *b)
> }
>
> #ifdef DPDK_NETDEV
> +static inline uint64_t *
> +dp_packet_ol_flags_ptr(const struct dp_packet *b)
> +{
> +return CONST_CAST(uint64_t *, &b->mbuf.ol_flags);
> +}
> +
> +static inline uint32_t *
> +dp_packet_rss_ptr(const struct dp_packet *b)
> +{
> +return CONST_CAST(uint32_t *, &b->mbuf.hash.rss);
> +}
> +
> +static inline uint32_t *
> +dp_packet_flow_mark_ptr(const struct dp_packet *b)
> +{
> +return CONST_CAST(uint32_t *, &b->mbuf.hash.fdir.hi);
> +}
> +
> +#else
> +static inline uint32_t *
> +dp_packet_ol_flags_ptr(const struct dp_packet *b)
> +{
> +return CONST_CAST(uint32_t *, &b->ol_flags);
> +}
> +
> +static inline uint32_t *
> +dp_packet_rss_ptr(const struct dp_packet *b
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
On Thu, Mar 19, 2020 at 02:30:14PM -0400, Tim Rozet wrote: > In addition I can see in my setup that conntrack and ovs-dpctl all the > states are established: > sctp,orig=(src=169.254.33.1,dst=169.254.33.2,sport=38982,dport=31769),reply=(src=10.244.0.5,dst=169.254.33.1,sport=62324,dport=38982),zone=9,protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) > sctp,orig=(src=169.254.33.1,dst=169.254.33.2,sport=38982,dport=31769),reply=(src=169.254.33.2,dst=169.254.33.1,sport=31769,dport=38982),protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) > sctp,orig=(src=169.254.33.1,dst=10.244.0.5,sport=38982,dport=62324),reply=(src=10.244.0.5,dst=100.64.0.1,sport=62324,dport=38982),zone=8,protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) > sctp,orig=(src=100.64.0.1,dst=10.244.0.5,sport=38982,dport=62324),reply=(src=10.244.0.5,dst=100.64.0.1,sport=62324,dport=38982),zone=15,protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) > > At this point the connection is open and only heartbeats and HB Acks are > being sent. However, if I poll ovs-dpctl dump-flows, the only flow I see > with sctp get hit every few seconds with 1 packet is: > recirc_id(0x1c),in_port(3),ct_state(+new-est-rel-rpl-inv+trk),ct_label(0/0x1),eth(),eth_type(0x0800),ipv4(dst=169.254.33.2,proto=132,frag=no),sctp(dst=31769), > packets:1, bytes:98, used:3.885s, actions:hash(l4(0)),recirc(0xfd) > > Notice the match contains "+new" but there is no new session here. I'm > using openvswitch-2.12.0-1.fc31.x86_64. Not saying that that's the reason, but to have in mind, heartbeats can create new conntrack entries. That's how it (conntrack) supports SCTP's multihoming. Marcelo ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while running "db-is-standalone" command
Hi Ben: Can you also backport this patch to 2.12 and 2.11 too? On Mon, Oct 7, 2019 at 11:21 AM Ben Pfaff wrote: > On Mon, Oct 07, 2019 at 10:30:07AM +0200, Damijan Skvarc wrote: > > problem is reported by valgrind while running functional tests: > > > > ==21043== 160 (88 direct, 72 indirect) bytes in 1 blocks are definitely > lost in loss record 7 of 8 > > ==21043==at 0x4C2DB8F: malloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > ==21043==by 0x45EE2E: xmalloc (util.c:138) > > ==21043==by 0x40CB81: ovsdb_log_open (log.c:270) > > ==21043==by 0x406B4F: do_db_has_magic.isra.9 (ovsdb-tool.c:563) > > ==21043==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223) > > ==21043==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > > ==21043==by 0x405A4C: main (ovsdb-tool.c:79) > > > > problem was in do_db_has_magic() which opens log file which is never > closed. > > > > Signed-off-by: Damijan Skvarc > > Applied to master, thanks. > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovsdb-tool: fix memory leak while converting cluster into standalone database
Hi Ben: Can you also backport this patch to 2.12 and 2.11 too? On Mon, Oct 7, 2019 at 11:22 AM Ben Pfaff wrote: > On Mon, Oct 07, 2019 at 10:10:34AM +0200, Damijan Skvarc wrote: > > memory leak is reported by valgrind while executing functional test > > "ovsdb-tool convert-to-standalone" > > > > ==13842== 2,850 (280 direct, 2,570 indirect) bytes in 7 blocks are > definitely lost in loss record 20 of 20 > > ==13842==at 0x4C2DB8F: malloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > > ==13842==by 0x45EE2E: xmalloc (util.c:138) > > ==13842==by 0x43E386: json_create (json.c:1451) > > ==13842==by 0x43BDD2: json_object_create (json.c:254) > > ==13842==by 0x43DEE3: json_parser_push_object (json.c:1273) > > ==13842==by 0x43E167: json_parser_input (json.c:1371) > > ==13842==by 0x43D6EA: json_lex_input (json.c:991) > > ==13842==by 0x43DAC1: json_parser_feed (json.c:1149) > > ==13842==by 0x40D108: parse_body (log.c:411) > > ==13842==by 0x40D386: ovsdb_log_read (log.c:476) > > ==13842==by 0x406A0B: do_convert_to_standalone (ovsdb-tool.c:1571) > > ==13842==by 0x406A0B: do_cluster_standalone (ovsdb-tool.c:1606) > > ==13842==by 0x438670: ovs_cmdl_run_command__ (command-line.c:223) > > ==13842==by 0x438720: ovs_cmdl_run_command (command-line.c:254) > > ==13842==by 0x405A4C: main (ovsdb-tool.c:79) > > > > The problem was in do_convert_to_standalone() function which while > reading log file > > allocate json object which was not deallocated at the end. > > > > Signed-off-by: Damijan Skvarc > > Applied to master, thanks. > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
Tim Rozet writes:
> I filed https://bugzilla.redhat.com/show_bug.cgi?id=1815217 to track this
> issue.
Thanks!
> Tim Rozet
> Red Hat CTO Networking Team
>
> On Thu, Mar 19, 2020 at 3:11 PM Ben Pfaff wrote:
>
> On Thu, Mar 19, 2020 at 10:27:52AM -0400, Mark Michelson wrote:
> > I've recently been working on adding support for SCTP load balancers in
> > OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my
> > patch[2].
>
> Do we have any idea whether OVS conntrack works for SCTP in general?
>
> Aaron, you're the only person I can quickly find who has committed
> anything related to sctp and conntrack, with commit 93346d889271
> ("conntrack: add display support for sctp"). Did you test conntrack
> with sctp or did you have any reports of success or failure with it?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
Ben Pfaff writes:
> On Thu, Mar 19, 2020 at 10:27:52AM -0400, Mark Michelson wrote:
>> I've recently been working on adding support for SCTP load balancers in
>> OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my
>> patch[2].
>
> Do we have any idea whether OVS conntrack works for SCTP in general?
>
> Aaron, you're the only person I can quickly find who has committed
> anything related to sctp and conntrack, with commit 93346d889271
> ("conntrack: add display support for sctp"). Did you test conntrack
> with sctp or did you have any reports of success or failure with it?
I did test, but only using an action=NORMAL bridge. I don't know if the
general case for conntracking SCTP works.
A quick test on my system does display similar issues, and it's likely
something in kernel. I see that a +inv+trk rule is matching. I'll dig
a bit deeper, but there's probably a missing state interpretation.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Fix NULL pointer dereference.
On 3/18/20 12:12 AM, William Tu wrote:
> Coverity CID 279957 reports NULL pointer derefence when
> 'conn' is NULL and calling ct_print_conn_info.
>
> Cc: Usman Ansari
> Signed-off-by: William Tu
Acked-by: Dumitru Ceara
Thanks,
Dumitru
> ---
> lib/conntrack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index ff5a89457c0a..001a37ff6aff 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
> if (!conn) {
> pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
> char *log_msg = xasprintf("Missing master conn %p",
> rev_conn);
> -ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> +ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);
> free(log_msg);
> return;
> }
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] conntrack: Reset ct_state when entering a new zone.
On 3/19/20 9:12 AM, Dumitru Ceara wrote:
> On 3/18/20 2:57 PM, Ilya Maximets wrote:
>> On 3/5/20 12:28 PM, Dumitru Ceara wrote:
>>> When a new conntrack zone is entered, the ct_state field is zeroed in
>>> order to avoid using state information from different zones.
>>>
>>> One such scenario is when a packet is double NATed. Assuming two zones
>>> and 3 flows performing the following actions in order on the packet:
>>> 1. ct(zone=5,nat), recirc
>>> 2. ct(zone=1), recirc
>>> 3. ct(zone=1,nat)
>>>
>>> If at step #1 the packet matches an existing NAT entry, it will get
>>> translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
>>> At step #2 the new tuple might match an existing connection and
>>> pkt->md.ct_zone is set to 1.
>>> If at step #3 the packet matches an existing NAT entry in zone 1,
>>> handle_nat() will be called to perform the translation but it will
>>> return early because the packet's zone matches the conntrack zone and
>>> the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
>>> translations in zone 5.
>>>
>>> In order to reliably detect when a packet enters a new conntrack zone
>>> we also need to make sure that the pkt->md.ct_zone is properly
>>> initialized if pkt->md.ct_state is non-zero. This already happens for
>>> most cases. The only exception is when matched conntrack connection is
>>> of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
>>> cover this path we now call write_ct_md() in that case too. Remove
>>> setting the CS_TRACKED flag as in this case as it will be done by the
>>> new call to write_ct_md().
>>>
>>> Also, the error path above seems hard to hit as it would've caused a
>>> crash due to dereferencing a NULL pointer when calling:
>>> 'ct_print_conn_info(conn, log_msg, VLL_INFO, true, true)'. This patch
>>> changes the call to log the 'rev_conn' info instead.
>>>
>>> CC: Darrell Ball
>>> Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
>>> Signed-off-by: Dumitru Ceara
>>>
>>> ---
>>> V2:
>>> - Address Ilya's comments:
>>> - revert changes to pkt_metadata_init().
>>> - update ct_state in process_one() only if ct_state is already
>>> non-zero.
>>> - Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
>>> is non-zero.
>>> - Fix NULL pointer dereference in process_one() if conn_type is
>>> CT_CONN_TYPE_UN_NAT and master conn is not found.
>>> ---
>>
>> 'Fixes' tag seems a bit strange to me. I think it should be:
>> Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
>>
>> Regarding the issue. I've spent some time exploring the conntrack code
>> and also researching the original patch that introduced this code.
>> The issue was raised during the review of the original patch 286de2729955
>> by Daniele: https://patchwork.ozlabs.org/patch/743108/
>> And Darrell said that he actually had the similar code that clears ct_state
>> during zone transition at the beginning of process_one(). But, he decided
>> that most of such issues are likely configuration bugs that should by raised
>> to user with warnings.
>> However, such warnings was never introduced and since this causes a real
>> issue in OVN we should actually have this clearing of conntrack state on
>> zone transitioning.
>>
>> Acked-by: Ilya Maximets
>>
>> Darrell, Ben, I'd like to have some comments on this from you since I'm
>> not an expert in this area. Otherwise, I'm going to apply this patch on
>> next week.
>>
>> Best regards, Ilya Maximets.
>>
>
>
> Hi Ilya,
>
> Thanks for the review. I'll send a v3 with updated 'Fixes' tag and your
> ack before next week unless there are more concerns from other reviewers.
>
> Thanks,
> Dumitru
>
V3 available at: https://patchwork.ozlabs.org/patch/1258393/
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] conntrack: Reset ct_state when entering a new zone.
When a new conntrack zone is entered, the ct_state field is zeroed in
order to avoid using state information from different zones.
One such scenario is when a packet is double NATed. Assuming two zones
and 3 flows performing the following actions in order on the packet:
1. ct(zone=5,nat), recirc
2. ct(zone=1), recirc
3. ct(zone=1,nat)
If at step #1 the packet matches an existing NAT entry, it will get
translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
At step #2 the new tuple might match an existing connection and
pkt->md.ct_zone is set to 1.
If at step #3 the packet matches an existing NAT entry in zone 1,
handle_nat() will be called to perform the translation but it will
return early because the packet's zone matches the conntrack zone and
the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
translations in zone 5.
In order to reliably detect when a packet enters a new conntrack zone
we also need to make sure that the pkt->md.ct_zone is properly
initialized if pkt->md.ct_state is non-zero. This already happens for
most cases. The only exception is when matched conntrack connection is
of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
cover this path we now call write_ct_md() in that case too. Remove
setting the CS_TRACKED flag as in this case as it will be done by the
new call to write_ct_md().
CC: Darrell Ball
Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
Acked-by: Ilya Maximets
Signed-off-by: Dumitru Ceara
---
V3:
- Add Ilya's ack and fix "Fixes" tag.
- Remove NULL pointer dereference fix as there's already a patch for it:
https://patchwork.ozlabs.org/patch/1257010/
V2:
- Address Ilya's comments:
- revert changes to pkt_metadata_init().
- update ct_state in process_one() only if ct_state is already
non-zero.
- Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
is non-zero.
- Fix NULL pointer dereference in process_one() if conn_type is
CT_CONN_TYPE_UN_NAT and master conn is not found.
---
lib/conntrack.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index ff5a894..0c41664 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1277,6 +1277,11 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
const struct nat_action_info_t *nat_action_info,
ovs_be16 tp_src, ovs_be16 tp_dst, const char *helper)
{
+/* Reset ct_state whenever entering a new zone. */
+if (pkt->md.ct_state && pkt->md.ct_zone != zone) {
+pkt->md.ct_state = 0;
+}
+
bool create_new_conn = false;
conn_key_lookup(ct, &ctx->key, ctx->hash, now, &ctx->conn, &ctx->reply);
struct conn *conn = ctx->conn;
@@ -1300,7 +1305,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
conn_key_lookup(ct, &ctx->key, hash, now, &conn, &ctx->reply);
if (!conn) {
-pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
+pkt->md.ct_state |= CS_INVALID;
+write_ct_md(pkt, zone, NULL, NULL, NULL);
char *log_msg = xasprintf("Missing master conn %p", rev_conn);
ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
free(log_msg);
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] bugtool: Fix for Python3
On Thu, Mar 19, 2020 at 12:05 PM Timothy Redaelli wrote: > > Currently ovs-bugtool tool doesn't start on Python 3. > This commit fixes ovs-bugtool to make it works on Python 3. > > Replaced StringIO.StringIO with io.BytesIO since the script is > processing binary data. > > Reported-at: https://bugzilla.redhat.com/1809241 > Reported-by: Flavio Leitner > Signed-off-by: Timothy Redaelli > --- > Changes since v1: > * Converted StringIO to BytesIO > * Fix some other string/bytes conversion > --- Thanks for sending out v2. Hit an error below: ~/ovs# python3 Python 3.5.2 (default, Oct 8 2019, 13:06:37) ~/ovs# ./utilities/bugtool/ovs-bugtool -y -s --output=tar.gz --outfile=/tmp/t.tgz Traceback (most recent call last): File "./utilities/bugtool/ovs-bugtool", line 1405, in sys.exit(main()) File "./utilities/bugtool/ovs-bugtool", line 717, in main collect_data() File "./utilities/bugtool/ovs-bugtool", line 388, in collect_data v['output'] = BytesIOmtime(s) File "./utilities/bugtool/ovs-bugtool", line 1395, in __init__ BytesIO.__init__(self, buf) TypeError: a bytes-like object is required, not 'str' I think sometimes 's' is bytes type, sometimes 's' is a str type... William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
I filed https://bugzilla.redhat.com/show_bug.cgi?id=1815217 to track this
issue.
Tim Rozet
Red Hat CTO Networking Team
On Thu, Mar 19, 2020 at 3:11 PM Ben Pfaff wrote:
> On Thu, Mar 19, 2020 at 10:27:52AM -0400, Mark Michelson wrote:
> > I've recently been working on adding support for SCTP load balancers in
> > OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my
> > patch[2].
>
> Do we have any idea whether OVS conntrack works for SCTP in general?
>
> Aaron, you're the only person I can quickly find who has committed
> anything related to sctp and conntrack, with commit 93346d889271
> ("conntrack: add display support for sctp"). Did you test conntrack
> with sctp or did you have any reports of success or failure with it?
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
On Thu, Mar 19, 2020 at 10:27:52AM -0400, Mark Michelson wrote:
> I've recently been working on adding support for SCTP load balancers in
> OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my
> patch[2].
Do we have any idea whether OVS conntrack works for SCTP in general?
Aaron, you're the only person I can quickly find who has committed
anything related to sctp and conntrack, with commit 93346d889271
("conntrack: add display support for sctp"). Did you test conntrack
with sctp or did you have any reports of success or failure with it?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Fix NULL pointer dereference.
On Thu, Mar 19, 2020 at 9:31 AM Dumitru Ceara wrote:
>
> On 3/19/20 4:02 PM, William Tu wrote:
> > On Wed, Mar 18, 2020 at 8:23 AM Ben Pfaff wrote:
> >>
> >> On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
> >>> On 3/18/20 12:12 AM, William Tu wrote:
> Coverity CID 279957 reports NULL pointer derefence when
> 'conn' is NULL and calling ct_print_conn_info.
>
> Cc: Usman Ansari
> Signed-off-by: William Tu
> ---
> lib/conntrack.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index ff5a89457c0a..001a37ff6aff 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet
> *pkt,
> if (!conn) {
> pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
> char *log_msg = xasprintf("Missing master conn %p",
> rev_conn);
> -ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> +ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true,
> true);
> free(log_msg);
> return;
> }
>
> >>>
> >>> Hi.
> >>>
> >>> This issue is addressed as part of the following patch:
> >>> https://patchwork.ozlabs.org/patch/1249513/
> >>> I'm not sure if we need to split it and fix this issue separately.
> >>> Thoughts?
> >>
> >> It seems like a separate issue to me, just located in nearby code.
> >
> > so split and fix separately?
> > William
>
> Hi William,
>
> I'll send a v3 of https://patchwork.ozlabs.org/patch/1249513/ and remove
> the conflict in my patch. Better to keep fixes separate indeed.
>
> Thanks,
> Dumitru
>
OK thanks!
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] bugtool: Fix for Python3
Currently ovs-bugtool tool doesn't start on Python 3.
This commit fixes ovs-bugtool to make it works on Python 3.
Replaced StringIO.StringIO with io.BytesIO since the script is
processing binary data.
Reported-at: https://bugzilla.redhat.com/1809241
Reported-by: Flavio Leitner
Signed-off-by: Timothy Redaelli
---
Changes since v1:
* Converted StringIO to BytesIO
* Fix some other string/bytes conversion
---
utilities/bugtool/ovs-bugtool.in | 45
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in
index e55bfc2ed..c26c2be7a 100755
--- a/utilities/bugtool/ovs-bugtool.in
+++ b/utilities/bugtool/ovs-bugtool.in
@@ -33,8 +33,7 @@
# or func_output().
#
-import StringIO
-import commands
+from io import BytesIO
import fcntl
import getopt
import hashlib
@@ -48,7 +47,7 @@ import warnings
import zipfile
from select import select
from signal import SIGTERM
-from subprocess import PIPE, Popen
+from subprocess import PIPE, Popen, check_output
from xml.dom.minidom import getDOMImplementation, parse
@@ -348,7 +347,7 @@ def collect_data():
cap = v['cap']
if 'cmd_args' in v:
if 'output' not in v.keys():
-v['output'] = StringIOmtime()
+v['output'] = BytesIOmtime()
if v['repeat_count'] > 0:
if cap not in process_lists:
process_lists[cap] = []
@@ -373,20 +372,20 @@ def collect_data():
if 'filename' in v and v['filename'].startswith('/proc/'):
# proc files must be read into memory
try:
-f = open(v['filename'], 'r')
+f = open(v['filename'], 'rb')
s = f.read()
f.close()
if check_space(cap, v['filename'], len(s)):
-v['output'] = StringIOmtime(s)
+v['output'] = BytesIOmtime(s)
except:
pass
elif 'func' in v:
try:
s = v['func'](cap)
except Exception as e:
-s = str(e)
+s = str(e).encode()
if check_space(cap, k, len(s)):
-v['output'] = StringIOmtime(s)
+v['output'] = BytesIOmtime(s)
def main(argv=None):
@@ -704,7 +703,7 @@ exclude those logs from the archive.
# permit the user to filter out data
# We cannot use iteritems, since we modify 'data' as we pass through
-for (k, v) in sorted(data.items()):
+for (k, v) in data.items():
cap = v['cap']
if 'filename' in v:
key = k[0]
@@ -721,7 +720,7 @@ exclude those logs from the archive.
# include inventory
data['inventory.xml'] = {'cap': None,
-'output': StringIOmtime(make_inventory(data, subdir))}
+'output': BytesIOmtime(make_inventory(data, subdir))}
# create archive
if output_fd == -1:
@@ -782,7 +781,7 @@ def dump_scsi_hosts(cap):
def module_info(cap):
-output = StringIO.StringIO()
+output = BytesIO()
modules = open(PROC_MODULES, 'r')
procs = []
@@ -806,7 +805,7 @@ def multipathd_topology(cap):
def dp_list():
-output = StringIO.StringIO()
+output = BytesIO()
procs = [ProcOutput([OVS_DPCTL, 'dump-dps'],
caps[CAP_NETWORK_STATUS][MAX_TIME], output)]
@@ -828,7 +827,7 @@ def collect_ovsdb():
if os.path.isfile(OPENVSWITCH_COMPACT_DB):
os.unlink(OPENVSWITCH_COMPACT_DB)
-output = StringIO.StringIO()
+output = BytesIO()
max_time = 5
procs = [ProcOutput(['ovsdb-tool', 'compact',
OPENVSWITCH_CONF_DB, OPENVSWITCH_COMPACT_DB],
@@ -871,7 +870,7 @@ def fd_usage(cap):
def dump_rdac_groups(cap):
-output = StringIO.StringIO()
+output = BytesIO()
procs = [ProcOutput([MPPUTIL, '-a'], caps[cap][MAX_TIME], output)]
run_procs([procs])
@@ -896,7 +895,7 @@ def load_plugins(just_capabilities=False, filter=None):
for node in nodelist:
if node.nodeType == node.TEXT_NODE:
rc += node.data
-return rc.encode()
+return rc
def getBoolAttr(el, attr, default=False):
ret = default
@@ -1037,7 +1036,7 @@ def make_tar(subdir, suffix, output_fd, output_file):
s = os.stat(v['filename'])
ti.mtime = s.st_mtime
ti.size = s.st_size
-tf.addfile(ti, open(v['filename']))
+tf.addfile(ti, open(v['filename'], 'rb'))
except:
pass
finally:
@@ -1095,12 +1094,12 @@ def make_inventory(inventory, subdir):
s.setAttribute('date', time.strftime('%c'))
s.setAttribute('hostname', platform.node())
s.setAttribute('uname', ' '.jo
Re: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
On Thu, Mar 19, 2020 at 11:50:55AM -0700, Usman Ansari wrote: > Coverity reports a false positive below: > > Incorrect expression, Assign_where_compare_meant: use of "=" > > where "==" may have been intended. > > Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. > > "make check" passes for this change > > Coverity reports 80 errors resolved > > > Suggested-by: Ben Pfaff > > Signed-off-by: Usman Ansari The patch is missing. The tags all go in one paragraph. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
Attachment is blocked, let me copy-paste. Sorry about so many emails. On Thu, Mar 19, 2020 at 11:51 AM Usman Ansari wrote: > Coverity reports a false positive below: > > Incorrect expression, Assign_where_compare_meant: use of "=" > > where "==" may have been intended. > > Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. > > "make check" passes for this change > > Coverity reports 80 errors resolved > > > Suggested-by: Ben Pfaff > > Signed-off-by: Usman Ansari > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
Coverity reports a false positive below: Incorrect expression, Assign_where_compare_meant: use of "=" where "==" may have been intended. Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. "make check" passes for this change Coverity reports 80 errors resolved Suggested-by: Ben Pfaff Signed-off-by: Usman Ansari ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
Resending, I hope it works this time! On Wed, Mar 18, 2020 at 5:36 PM Ben Pfaff wrote: > On Wed, Mar 18, 2020 at 05:32:03PM -0700, Usman Ansari wrote: > > Suggested-by: Ben Pfaff > > > > Coverity reports a false positive below: > > Incorrect expression, Assign_where_compare_meant: use of "=" > > where "==" may have been intended. > > Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. > > "make check" passes for this change > > Coverity reports 80 errors resolved > > > > Signed-off-by: Usman Ansari > > Thanks. > > This doesn't apply. "git am" says: > > Applying: hmap.h: Fix Coverity false positive > error: corrupt patch at line 14 > Patch failed at 0001 hmap.h: Fix Coverity false positive > hint: Use 'git am --show-current-patch' to see the failed patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > > Also, all tags go at the bottom of the commit message, that is, > Suggested-by is in the wrong place. The committer documentation has > some more rules for tags. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
In addition I can see in my setup that conntrack and ovs-dpctl all the states are established: sctp,orig=(src=169.254.33.1,dst=169.254.33.2,sport=38982,dport=31769),reply=(src=10.244.0.5,dst=169.254.33.1,sport=62324,dport=38982),zone=9,protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) sctp,orig=(src=169.254.33.1,dst=169.254.33.2,sport=38982,dport=31769),reply=(src=169.254.33.2,dst=169.254.33.1,sport=31769,dport=38982),protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) sctp,orig=(src=169.254.33.1,dst=10.244.0.5,sport=38982,dport=62324),reply=(src=10.244.0.5,dst=100.64.0.1,sport=62324,dport=38982),zone=8,protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) sctp,orig=(src=100.64.0.1,dst=10.244.0.5,sport=38982,dport=62324),reply=(src=10.244.0.5,dst=100.64.0.1,sport=62324,dport=38982),zone=15,protoinfo=(state=ESTABLISHED,vtag_orig=3615038536,vtag_reply=554870550) At this point the connection is open and only heartbeats and HB Acks are being sent. However, if I poll ovs-dpctl dump-flows, the only flow I see with sctp get hit every few seconds with 1 packet is: recirc_id(0x1c),in_port(3),ct_state(+new-est-rel-rpl-inv+trk),ct_label(0/0x1),eth(),eth_type(0x0800),ipv4(dst=169.254.33.2,proto=132,frag=no),sctp(dst=31769), packets:1, bytes:98, used:3.885s, actions:hash(l4(0)),recirc(0xfd) Notice the match contains "+new" but there is no new session here. I'm using openvswitch-2.12.0-1.fc31.x86_64. Thanks, Tim Rozet Red Hat CTO Networking Team On Thu, Mar 19, 2020 at 10:39 AM Mark Michelson wrote: > On 3/19/20 10:27 AM, Mark Michelson wrote: > > Hi, > > > > I've recently been working on adding support for SCTP load balancers in > > OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my > > patch[2]. > > > > In short, during a typical SCTP association, it appears that conntrack > > never reaches the "+est" state. > > > > OVN installs the following two OF flows for load balancers: > > > > (1) cookie=0x3bfb3d5b, duration=1724.090s, table=14, n_packets=0, > > n_bytes=0, idle_age=1724, > > > priority=120,ct_state=+est+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 > > > > actions=set_field:0x8/0x8->reg10,ct(table=15,zone=NXM_NX_REG11[0..15],nat) > > (2) cookie=0xd84870e, duration=1724.090s, table=14, n_packets=6, > > n_bytes=636, idle_age=257, > > > priority=120,ct_state=+new+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 > > > actions=set_field:0x8/0x8->reg10,group:2 > > > > The idea is that when dealing with a new connection, we should load > > balance to a group (flow 2). Once the connection is established, we > > should dnat to assure that the traffic goes to the destination stored in > > conntrack (flow 1). > > > > With SCTP, we were seeing an issue where the INIT sent by the client > > would hit flow 2 as expected. The INIT_ACK from the server would then > > reach the client. The client would send a COOKIE_ECHO, and this would > > hit neither flow 1 nor flow 2. Conntrack apparently reached a state > > where it was neither "new" nor "est". > > > > Looking at the netfilter code for SCTP, this makes some amount of sense. > > After the INIT and INIT_ACK are exchanged, there are the COOKIE_ECHOED > > and COOKIE_WAIT states that can be entered prior to being ESTABLISHED. > > > > So Tim added a new flow for sctp that would perform the same actions as > > flow 1 above, but would match for "-new-est+trk". In other words, SCTP > > traffic that is tracked but is not new and also not established. This > > had the intended effect of matching on the COOKIE_ECHO from the client. > > > > However, Tim then noticed that even after the COOKIE_ECHO and > > COOKIE_ACK, additional DATA traffic from the client over the SCTP > > association never matched flow 1. In other words, the conntrack state > > apparently never actually reached "+est" according to OVS. > > > > My worry is if there might be a bug somewhere in OVS that makes it so > > that SCTP associations never reach an established conntrack state. Or > > perhaps this is by design? If so, then what should OVN do for SCTP > > associations when we normally want to match the +est state? > > > > Thanks, > > Mark Michelson > > [1] https://patchwork.ozlabs.org/patch/1257045/ > [2] https://bugzilla.redhat.com/show_bug.cgi?id=1718372 > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] util: Update OVS_TYPEOF macro for Windows.
On Thu, Mar 19, 2020 at 10:03:15AM -0700, Archana Holla via dev wrote: > OVS_TYPEOF macro doesn’t return the type of object for non __GNUC__ platforms. > Updating it for _WIN32 platforms when used from C++ code. > > Signed-off-by: Archana Holla > --- > include/openvswitch/util.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h > index 9189e64..af7d1b0 100644 > --- a/include/openvswitch/util.h > +++ b/include/openvswitch/util.h > @@ -86,8 +86,12 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const > char *, const char *); > #ifdef __GNUC__ > #define OVS_TYPEOF(OBJECT) typeof(OBJECT) > #else > +#if defined (__cplusplus) && defined(_WIN32) > +#define OVS_TYPEOF(OBJECT) decltype(OBJECT) > +#else > #define OVS_TYPEOF(OBJECT) void * > #endif > +#endif Thanks for submitting a patch. Does this solve a problem? decltype is a C++ feature. How is it specific to Windows? Please use #elif instead of introducing a nested #if...#endif. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] util: Update OVS_TYPEOF macro for Windows.
OVS_TYPEOF macro doesn’t return the type of object for non __GNUC__ platforms. Updating it for _WIN32 platforms when used from C++ code. Signed-off-by: Archana Holla --- include/openvswitch/util.h | 4 1 file changed, 4 insertions(+) diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h index 9189e64..af7d1b0 100644 --- a/include/openvswitch/util.h +++ b/include/openvswitch/util.h @@ -86,8 +86,12 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const char *, const char *); #ifdef __GNUC__ #define OVS_TYPEOF(OBJECT) typeof(OBJECT) #else +#if defined (__cplusplus) && defined(_WIN32) +#define OVS_TYPEOF(OBJECT) decltype(OBJECT) +#else #define OVS_TYPEOF(OBJECT) void * #endif +#endif /* Given OBJECT of type pointer-to-structure, expands to the offset of MEMBER * within an instance of the structure. -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Fix NULL pointer dereference.
On 3/19/20 4:02 PM, William Tu wrote:
> On Wed, Mar 18, 2020 at 8:23 AM Ben Pfaff wrote:
>>
>> On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
>>> On 3/18/20 12:12 AM, William Tu wrote:
Coverity CID 279957 reports NULL pointer derefence when
'conn' is NULL and calling ct_print_conn_info.
Cc: Usman Ansari
Signed-off-by: William Tu
---
lib/conntrack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index ff5a89457c0a..001a37ff6aff 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet
*pkt,
if (!conn) {
pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
char *log_msg = xasprintf("Missing master conn %p",
rev_conn);
-ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
+ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true,
true);
free(log_msg);
return;
}
>>>
>>> Hi.
>>>
>>> This issue is addressed as part of the following patch:
>>> https://patchwork.ozlabs.org/patch/1249513/
>>> I'm not sure if we need to split it and fix this issue separately.
>>> Thoughts?
>>
>> It seems like a separate issue to me, just located in nearby code.
>
> so split and fix separately?
> William
Hi William,
I'll send a v3 of https://patchwork.ozlabs.org/patch/1249513/ and remove
the conflict in my patch. Better to keep fixes separate indeed.
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-northd: Don't add arp responder flows for lports with 'unknown' address.
On Thu, Mar 19, 2020 at 5:27 AM wrote:
>
> From: Numan Siddique
>
> If a logical port has 'unknown' address, it means it can send and receive
> packet with any IP and MAC and generally port security is not set for
> such logical ports. If an lport has addresses set to - ["MAC1 IP1",
unknown],
> right now we add arp responder flows for IP1 and respond MAC1 in the arp
> response. But it's possible that the VIF of the logical port can use the
IP1
> with a different MAC. This patch supports this usecase. When another
logical port
> sends ARP request for IP1, the VIF of the logical port will anyway
respond.
>
> Reported-by: Maciej Józefczyk
> Signed-off-by: Numan Siddique
> ---
> northd/ovn-northd.8.xml | 5 +++--
> northd/ovn-northd.c | 13 -
> tests/ovn.at| 16
> 3 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 9b44720d1..7d03cbc83 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -699,8 +699,9 @@ output;
>
>
>These flows are omitted for logical ports (other than router
ports or
> - localport ports) that are down and for logical
ports of
> - type virtual.
> + localport ports) that are down, for logical ports
of
> + type virtual and for logical ports with 'unknown'
> + address set.
>
>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 4f94680b5..f648d2ea7 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1152,7 +1152,7 @@ struct ovn_port {
>
> bool derived; /* Indicates whether this is an additional port
> * derived from nbsp or nbrp. */
> -
> +bool has_unknown; /* If the addresses have 'unknown' defined. */
> /* The port's peer:
> *
> * - A switch port S of type "router" has a router port R as a
peer,
> @@ -2059,8 +2059,11 @@ join_logical_ports(struct northd_context *ctx,
> op->lsp_addrs
> = xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
> for (size_t j = 0; j < nbsp->n_addresses; j++) {
> -if (!strcmp(nbsp->addresses[j], "unknown")
> -|| !strcmp(nbsp->addresses[j], "router")) {
> +if (!strcmp(nbsp->addresses[j], "unknown")) {
> +op->has_unknown = true;
> +continue;
> +}
> +if (!strcmp(nbsp->addresses[j], "router")) {
> continue;
> }
> if (is_dynamic_lsp_address(nbsp->addresses[j])) {
> @@ -6127,7 +6130,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
> } else {
> /*
> * Add ARP/ND reply flows if either the
> - * - port is up or
> + * - port is up and it doesn't have 'unknown' address
defined or
> * - port type is router or
> * - port type is localport
> */
> @@ -6136,7 +6139,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
> continue;
> }
>
> -if (lsp_is_external(op->nbsp)) {
> +if (lsp_is_external(op->nbsp) || op->has_unknown) {
> continue;
> }
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 8cdbad743..1b6073ff0 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1758,11 +1758,13 @@ for is in 1 2 3; do
> sip=`ip_to_hex 192 168 0 $is$js`
> tip=`ip_to_hex 192 168 0 $id$jd`
> tip_unknown=`ip_to_hex 11 11 11 11`
> +reply_ha=;
> if test $d != $s; then
> -reply_ha=f0$d
> -else
> -reply_ha=
> +if test $jd != 1; then
> +reply_ha=f0$d
> +fi
> fi
> +
> test_arp $s f0$s $sip $tip $reply_ha
#9
> test_arp $s f0$s $sip $tip_unknown
#10
>
> @@ -2199,7 +2201,13 @@ for s in 1 2 3; do
> sip=192.168.0.$s
> tip=192.168.0.$d
> tip_unknown=11.11.11.11
> -if test $d != $s; then reply_ha=f0:00:00:00:00:0$d; else
reply_ha=; fi
> +reply_ha=;
> +if test $d != $s; then
> +if test $d != 1; then
> +reply_ha=f0:00:00:00:00:0$d;
> +fi
> +fi
> +
> test_arp $s f0:00:00:00:00:0$s $sip $tip $reply_ha
#9
> test_arp $s f0:00:00:00:00:0$s $sip $tip_unknown
#10
>
> --
> 2.24.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Acked-by: Han Zhou
___
dev mailing list
d...@ope
Re: [ovs-dev] [PATCH] python: Handle refTable values with setkey()
On Sat, Mar 14, 2020 at 5:41 AM Terry Wilson wrote:
>
> For columns like QoS.queues where we have a map containing refTable
> values, assigning w/ __setattr__ e.g. qos.queues={1: $queue_row}
> works, but using using qos.setkey('queues', 1, $queue_row) results
> in an Exception. The opdat argument can essentially just be the
> JSON representation of the map column instead of trying to build
> it.
>
> Signed-off-by: Terry Wilson
Hi Terry,
The below test case is failing for me. Can you please check
## openvswitch 2.13.90 test suite. ##
## --- ##
2097: idl handling of missing tables and columns - C FAILED (ovsdb-idl.at:956)
Thanks
Numan
> ---
> python/ovs/db/idl.py| 3 +--
> tests/idltest.ovsschema | 15 +++
> tests/ovsdb-idl.at | 12
> tests/test-ovsdb.py | 23 ++-
> 4 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index 020291d48..5850ac7ab 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -1567,10 +1567,9 @@ class Transaction(object):
> for col, val in row._mutations['_inserts'].items():
> column = row._table.columns[col]
> if column.type.is_map():
> -opdat = ["map"]
> datum = data.Datum.from_python(column.type, val,
> _row_to_uuid)
> -opdat.append(datum.as_list())
> +opdat = self._substitute_uuids(datum.to_json())
> else:
> opdat = ["set"]
> inner_opdat = []
> diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema
> index bee79fc50..e02b975bc 100644
> --- a/tests/idltest.ovsschema
> +++ b/tests/idltest.ovsschema
> @@ -171,6 +171,21 @@
>},
>"isRoot" : false
> },
> +"simple5": {
> + "columns" : {
> +"name": {"type": "string"},
> +"irefmap": {
> + "type": {
> +"key": {"type": "integer"},
> +"value": {"type": "uuid",
> + "refTable": "simple3"},
> +"min": 0,
> +"max": "unlimited"
> + }
> +}
> + },
> + "isRoot": true
> +},
> "singleton" : {
>"columns" : {
> "name" : {
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index cc38d69c1..fc5844357 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -1288,6 +1288,18 @@ OVSDB_CHECK_IDL_PY([partial-map idl],
> 009: done
> ]])
>
> +OVSDB_CHECK_IDL_PY([partial-map update set refmap idl],
> +[['["idltest", {"op":"insert", "table":"simple3",
> "row":{"name":"myString1"}},
> + {"op":"insert", "table":"simple5",
> "row":{"name":"myString2"}}]']],
> +['partialmapmutateirefmap'],
> +[[000: name=myString1 uset=[]
> +000: name=myString2 irefmap=[]
> +001: commit, status=success
> +002: name=myString1 uset=[]
> +002: name=myString2 irefmap=[(1 <0>)]
> +003: done
> +]])
> +
> m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN],
>[AT_SETUP([$1 - C])
> AT_KEYWORDS([ovsdb server idl partial update set column positive $5])
> diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
> index 1b94b79a0..a19680274 100644
> --- a/tests/test-ovsdb.py
> +++ b/tests/test-ovsdb.py
> @@ -28,6 +28,7 @@ import ovs.util
> import ovs.vlog
> from ovs.db import data
> from ovs.db import error
> +from ovs.db.idl import _row_to_uuid as row_to_uuid
> from ovs.fatal_signal import signal_alarm
>
> vlog = ovs.vlog.Vlog("test-ovsdb")
> @@ -159,7 +160,8 @@ def get_simple_printable_row_string(row, columns):
> is ovs.db.data.Atom):
> value = getattr(row, column)
> if isinstance(value, dict):
> -value = sorted(value.items())
> +value = sorted((row_to_uuid(k), row_to_uuid(v))
> + for k, v in value.items())
> s += "%s=%s " % (column, value)
> s = s.strip()
> s = re.sub('""|,|u?\'', "", s)
> @@ -212,6 +214,14 @@ def print_idl(idl, step):
> print(s)
> n += 1
>
> +if "simple5" in idl.tables:
> +simple5 = idl.tables["simple5"].rows
> +for row in simple5.values():
> +s = "%03d: " % step
> +s += get_simple_printable_row_string(row, ["name", "irefmap"])
> +print(s)
> +n += 1
> +
> if "link1" in idl.tables:
> l1 = idl.tables["link1"].rows
> for row in l1.values():
> @@ -303,6 +313,11 @@ def idltest_find_simple3(idl, i):
> return next(idl.index_equal("simple3", "simple3_by_name", i), None)
>
>
> +def idltest_find(idl, table, col, match):
> +return next((r for r in idl.tables[table].rows.values() if
> +
Re: [ovs-dev] [PATCH] conntrack: Fix NULL pointer dereference.
On Wed, Mar 18, 2020 at 8:23 AM Ben Pfaff wrote:
>
> On Wed, Mar 18, 2020 at 01:49:48PM +0100, Ilya Maximets wrote:
> > On 3/18/20 12:12 AM, William Tu wrote:
> > > Coverity CID 279957 reports NULL pointer derefence when
> > > 'conn' is NULL and calling ct_print_conn_info.
> > >
> > > Cc: Usman Ansari
> > > Signed-off-by: William Tu
> > > ---
> > > lib/conntrack.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > > index ff5a89457c0a..001a37ff6aff 100644
> > > --- a/lib/conntrack.c
> > > +++ b/lib/conntrack.c
> > > @@ -1302,7 +1302,7 @@ process_one(struct conntrack *ct, struct dp_packet
> > > *pkt,
> > > if (!conn) {
> > > pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
> > > char *log_msg = xasprintf("Missing master conn %p",
> > > rev_conn);
> > > -ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
> > > +ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true,
> > > true);
> > > free(log_msg);
> > > return;
> > > }
> > >
> >
> > Hi.
> >
> > This issue is addressed as part of the following patch:
> > https://patchwork.ozlabs.org/patch/1249513/
> > I'm not sure if we need to split it and fix this issue separately.
> > Thoughts?
>
> It seems like a separate issue to me, just located in nearby code.
so split and fix separately?
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH 0/5] XDP offload using flow API provider
On Tue, Mar 17, 2020 at 2:26 AM 牧田俊明 wrote: > > Any more feedback? > I'll work on implementing missing parts in RFC and prepare v2. > If anyone has feedback on the concept at this point, it would be helpful. > Hi Toshiaki, Thanks, I've read through the v1. If you have v2, please send it. Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] 答复: [PATCH v7] Use TPACKET_V3 to accelerate veth for userspace datapath
On Wed, Mar 18, 2020 at 8:12 PM Yi Yang (杨燚)-云服务集团 wrote: > > Hi, folks > > As I said, TPACKET_V3 does have kernel implementation issue, I tried to fix > it in Linux kernel 5.5.9, here is my test data with tpacket_v3 and tso > enabled. On my low end server, my goal is to reach 16Gbps at least, I still > have another idea to improve it. > Can you share your kernel fix? Or have you sent patch somewhere? William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
On 3/19/20 10:27 AM, Mark Michelson wrote: Hi, I've recently been working on adding support for SCTP load balancers in OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my patch[2]. In short, during a typical SCTP association, it appears that conntrack never reaches the "+est" state. OVN installs the following two OF flows for load balancers: (1) cookie=0x3bfb3d5b, duration=1724.090s, table=14, n_packets=0, n_bytes=0, idle_age=1724, priority=120,ct_state=+est+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 actions=set_field:0x8/0x8->reg10,ct(table=15,zone=NXM_NX_REG11[0..15],nat) (2) cookie=0xd84870e, duration=1724.090s, table=14, n_packets=6, n_bytes=636, idle_age=257, priority=120,ct_state=+new+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 actions=set_field:0x8/0x8->reg10,group:2 The idea is that when dealing with a new connection, we should load balance to a group (flow 2). Once the connection is established, we should dnat to assure that the traffic goes to the destination stored in conntrack (flow 1). With SCTP, we were seeing an issue where the INIT sent by the client would hit flow 2 as expected. The INIT_ACK from the server would then reach the client. The client would send a COOKIE_ECHO, and this would hit neither flow 1 nor flow 2. Conntrack apparently reached a state where it was neither "new" nor "est". Looking at the netfilter code for SCTP, this makes some amount of sense. After the INIT and INIT_ACK are exchanged, there are the COOKIE_ECHOED and COOKIE_WAIT states that can be entered prior to being ESTABLISHED. So Tim added a new flow for sctp that would perform the same actions as flow 1 above, but would match for "-new-est+trk". In other words, SCTP traffic that is tracked but is not new and also not established. This had the intended effect of matching on the COOKIE_ECHO from the client. However, Tim then noticed that even after the COOKIE_ECHO and COOKIE_ACK, additional DATA traffic from the client over the SCTP association never matched flow 1. In other words, the conntrack state apparently never actually reached "+est" according to OVS. My worry is if there might be a bug somewhere in OVS that makes it so that SCTP associations never reach an established conntrack state. Or perhaps this is by design? If so, then what should OVN do for SCTP associations when we normally want to match the +est state? Thanks, Mark Michelson [1] https://patchwork.ozlabs.org/patch/1257045/ [2] https://bugzilla.redhat.com/show_bug.cgi?id=1718372 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Conntrack with SCTP: +est is never reached.
On 3/19/20 10:27 AM, Mark Michelson wrote: Hi, I've recently been working on adding support for SCTP load balancers in OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my patch[2]. In short, during a typical SCTP association, it appears that conntrack never reaches the "+est" state. OVN installs the following two OF flows for load balancers: (1) cookie=0x3bfb3d5b, duration=1724.090s, table=14, n_packets=0, n_bytes=0, idle_age=1724, priority=120,ct_state=+est+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 actions=set_field:0x8/0x8->reg10,ct(table=15,zone=NXM_NX_REG11[0..15],nat) (2) cookie=0xd84870e, duration=1724.090s, table=14, n_packets=6, n_bytes=636, idle_age=257, priority=120,ct_state=+new+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 actions=set_field:0x8/0x8->reg10,group:2 The idea is that when dealing with a new connection, we should load balance to a group (flow 2). Once the connection is established, we should dnat to assure that the traffic goes to the destination stored in conntrack (flow 1). With SCTP, we were seeing an issue where the INIT sent by the client would hit flow 2 as expected. The INIT_ACK from the server would then reach the client. The client would send a COOKIE_ECHO, and this would hit neither flow 1 nor flow 2. Conntrack apparently reached a state where it was neither "new" nor "est". Looking at the netfilter code for SCTP, this makes some amount of sense. After the INIT and INIT_ACK are exchanged, there are the COOKIE_ECHOED and COOKIE_WAIT states that can be entered prior to being ESTABLISHED. So Tim added a new flow for sctp that would perform the same actions as flow 1 above, but would match for "-new-est+trk". In other words, SCTP traffic that is tracked but is not new and also not established. This had the intended effect of matching on the COOKIE_ECHO from the client. However, Tim then noticed that even after the COOKIE_ECHO and COOKIE_ACK, additional DATA traffic from the client over the SCTP association never matched flow 1. In other words, the conntrack state apparently never actually reached "+est" according to OVS. My worry is if there might be a bug somewhere in OVS that makes it so that SCTP associations never reach an established conntrack state. Or perhaps this is by design? If so, then what should OVN do for SCTP associations when we normally want to match the +est state? Thanks, Mark Michelson Forgot my footnote: [1] https://patchwork.ozlabs.org/patch/1257045/ ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
On Wed, Mar 18, 2020 at 5:37 PM Ben Pfaff wrote: > > On Wed, Mar 18, 2020 at 05:32:03PM -0700, Usman Ansari wrote: > > Suggested-by: Ben Pfaff > > > > Coverity reports a false positive below: > > Incorrect expression, Assign_where_compare_meant: use of "=" > > where "==" may have been intended. > > Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. > > "make check" passes for this change > > Coverity reports 80 errors resolved > > > > Signed-off-by: Usman Ansari > > Thanks. > > This doesn't apply. "git am" says: > > Applying: hmap.h: Fix Coverity false positive > error: corrupt patch at line 14 > Patch failed at 0001 hmap.h: Fix Coverity false positive > hint: Use 'git am --show-current-patch' to see the failed patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > > Also, all tags go at the bottom of the commit message, that is, > Suggested-by is in the wrong place. The committer documentation has > some more rules for tags. Hi Usman, You can also setup travis CI tests for your patch, see Documentation/topics/testing.rst And provide a link to the CI test result. Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] bugtool: Fix for Python3
On Thu, Mar 19, 2020 at 4:03 AM Timothy Redaelli wrote: > > On Wed, 18 Mar 2020 14:25:43 -0700 > William Tu wrote: > > > On Wed, Mar 18, 2020 at 8:23 AM Timothy Redaelli > > wrote: > > > > > > Currently ovs-bugtool tool doesn't start on Python3. > > > This commit fixes ovs-bugtool to make it works on Python.3 > > > > > > Reported-at: https://bugzilla.redhat.com/1809241 > > > Reported-by: Flavio Leitner > > > Signed-off-by: Timothy Redaelli > > > --- > > > > Thanks for the patch. I tried to fix it before but haven't get it done yet. > > I run your patch and got this error, can you take a look? > > > > ovs# ./utilities/bugtool/ovs-bugtool -y -s --output=tar.gz > > --outfile=/var/log/bugtool-report.tgz > > Traceback (most recent call last): > > File "./utilities/bugtool/ovs-bugtool", line 1405, in > > sys.exit(main()) > > File "./utilities/bugtool/ovs-bugtool", line 717, in main > > collect_data() > > File "./utilities/bugtool/ovs-bugtool", line 366, in collect_data > > run_procs(process_lists.values()) > > File "./utilities/bugtool/ovs-bugtool", line 1344, in run_procs > > p.read_line() > > File "./utilities/bugtool/ovs-bugtool", line 1313, in read_line > > self.inst.write(line.decode()) > > UnicodeDecodeError: 'utf-8' codec can't decode byte 0xcb in position > > 67: invalid continuation byte > > > > Thanks > > William > > > > Thank you, > I'll send a v2 as soon as possibile > thanks! my previous patch here FYI, if it helps https://mail.openvswitch.org/pipermail/ovs-dev/2020-February/368111.html ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Conntrack with SCTP: +est is never reached.
Hi, I've recently been working on adding support for SCTP load balancers in OVN[1]. In a recent test run by Tim Rozet, he ran into an issue with my patch[2]. In short, during a typical SCTP association, it appears that conntrack never reaches the "+est" state. OVN installs the following two OF flows for load balancers: (1) cookie=0x3bfb3d5b, duration=1724.090s, table=14, n_packets=0, n_bytes=0, idle_age=1724, priority=120,ct_state=+est+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 actions=set_field:0x8/0x8->reg10,ct(table=15,zone=NXM_NX_REG11[0..15],nat) (2) cookie=0xd84870e, duration=1724.090s, table=14, n_packets=6, n_bytes=636, idle_age=257, priority=120,ct_state=+new+trk,sctp,metadata=0x3,nw_dst=169.254.33.2,tp_dst=31769 actions=set_field:0x8/0x8->reg10,group:2 The idea is that when dealing with a new connection, we should load balance to a group (flow 2). Once the connection is established, we should dnat to assure that the traffic goes to the destination stored in conntrack (flow 1). With SCTP, we were seeing an issue where the INIT sent by the client would hit flow 2 as expected. The INIT_ACK from the server would then reach the client. The client would send a COOKIE_ECHO, and this would hit neither flow 1 nor flow 2. Conntrack apparently reached a state where it was neither "new" nor "est". Looking at the netfilter code for SCTP, this makes some amount of sense. After the INIT and INIT_ACK are exchanged, there are the COOKIE_ECHOED and COOKIE_WAIT states that can be entered prior to being ESTABLISHED. So Tim added a new flow for sctp that would perform the same actions as flow 1 above, but would match for "-new-est+trk". In other words, SCTP traffic that is tracked but is not new and also not established. This had the intended effect of matching on the COOKIE_ECHO from the client. However, Tim then noticed that even after the COOKIE_ECHO and COOKIE_ACK, additional DATA traffic from the client over the SCTP association never matched flow 1. In other words, the conntrack state apparently never actually reached "+est" according to OVS. My worry is if there might be a bug somewhere in OVS that makes it so that SCTP associations never reach an established conntrack state. Or perhaps this is by design? If so, then what should OVN do for SCTP associations when we normally want to match the +est state? Thanks, Mark Michelson ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC] github-ci: Enable github actions.
On Thu, Mar 19, 2020 at 6:09 AM Aaron Conole wrote: > > William Tu writes: > > > So far we only use travis to do run 'make check' per commit. > > This enables per-commit check for 'make check-system-userspace' test. > > We can think about what others to add using github actions. > > > > Example run: > > https://github.com/williamtu/ovs-travis/runs/514822181?check_suite_focus=true > > > > Signed-off-by: William Tu > > --- > > This is a cool feature - thanks for proposing it. > > I've only done a little bit of looking at it - I think I may need to > explicitly enable it for the bot repositories (but I still don't quite > understand the settings in 'Action permissions'). I'll spend more time > digging into it. > > Are there any thoughts to using the scripts in .travis and maybe > consolidating it to something more generic? > Thanks. This is independent of travis CI so I don't think we need to change .travis. As soon as you push this .github/workflows/ovs.yml to your own github master branch, the CI runner will kick off. Currently travis takes about 1 hour to finish our tests, so I'm thinking about moving or adding tests here at github actions. William > > .github/workflows/ovs.yml | 25 + > > Makefile.am | 1 + > > 2 files changed, 26 insertions(+) > > create mode 100644 .github/workflows/ovs.yml > > > > diff --git a/.github/workflows/ovs.yml b/.github/workflows/ovs.yml > > new file mode 100644 > > index ..1e056aceed2c > > --- /dev/null > > +++ b/.github/workflows/ovs.yml > > @@ -0,0 +1,25 @@ > > +name: OVS CI > > + > > +on: > > + push: > > +branches: [ master ] > > + pull_request: > > +branches: [ master ] > > + > > +jobs: > > + build: > > +runs-on: ubuntu-latest > > +steps: > > +- uses: actions/checkout@v2 > > +- name: configure > > + run: ./boot.sh; ./configure > > +- name: make > > + run: make -j2 > > +- name: check-system-userspace > > + run: sudo make check-system-userspace TESTSUITEFLAGS='1-30' > > RECHECK=yes > > +- name: Upload artifact > > + uses: actions/upload-artifact@v1.0.0 > > + if: failure() > > + with: > > +name: system-userspace > > +path: tests/system-userspace-testsuite.dir/ > > diff --git a/Makefile.am b/Makefile.am > > index b279303d186c..80448d0c31c1 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -92,6 +92,7 @@ EXTRA_DIST = \ > > $(MAN_ROOTS) \ > > Vagrantfile \ > > Vagrantfile-FreeBSD \ > > + .github/workflows/ovs.yml \ > > .mailmap > > bin_PROGRAMS = > > sbin_PROGRAMS = > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.
On Thu, Mar 19, 2020 at 12:04:40PM +0100, Simon Horman wrote: > On Thu, Mar 19, 2020 at 11:28:00AM +0200, Roi Dayan wrote: > > > > > > On 2020-03-11 7:39 AM, we...@ucloud.cn wrote: > > > From: wenxu > > > > > > The tc modify flow put always delete the original flow first and > > > then add the new flow. If the modfiy flow put operation failed, > > > the flow put operation will change from modify to create if success > > > to delete the original flow in tc (which will be always failed with > > > ENOENT, the flow is already be deleted before add the new flow in tc). > > > Finally, the modify flow put will failed to add in kernel datapath. > > > > > > Signed-off-by: wenxu > > ... > > > Acked-by: Roi Dayan > > Thanks, > > this looks good to me. > > I am exercising the patch applied on top of master and branch-2.13 > using Travis CI. And I plan to push the patch to those branches if > that succeeds. > > The patch does not apply cleanly on branch-2.12. Please consider > posting a backport to that and earlier branches if the change > is appropriate there. Thanks again, pushed to master and branch-2.13. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC] github-ci: Enable github actions.
William Tu writes: > So far we only use travis to do run 'make check' per commit. > This enables per-commit check for 'make check-system-userspace' test. > We can think about what others to add using github actions. > > Example run: > https://github.com/williamtu/ovs-travis/runs/514822181?check_suite_focus=true > > Signed-off-by: William Tu > --- This is a cool feature - thanks for proposing it. I've only done a little bit of looking at it - I think I may need to explicitly enable it for the bot repositories (but I still don't quite understand the settings in 'Action permissions'). I'll spend more time digging into it. Are there any thoughts to using the scripts in .travis and maybe consolidating it to something more generic? > .github/workflows/ovs.yml | 25 + > Makefile.am | 1 + > 2 files changed, 26 insertions(+) > create mode 100644 .github/workflows/ovs.yml > > diff --git a/.github/workflows/ovs.yml b/.github/workflows/ovs.yml > new file mode 100644 > index ..1e056aceed2c > --- /dev/null > +++ b/.github/workflows/ovs.yml > @@ -0,0 +1,25 @@ > +name: OVS CI > + > +on: > + push: > +branches: [ master ] > + pull_request: > +branches: [ master ] > + > +jobs: > + build: > +runs-on: ubuntu-latest > +steps: > +- uses: actions/checkout@v2 > +- name: configure > + run: ./boot.sh; ./configure > +- name: make > + run: make -j2 > +- name: check-system-userspace > + run: sudo make check-system-userspace TESTSUITEFLAGS='1-30' RECHECK=yes > +- name: Upload artifact > + uses: actions/upload-artifact@v1.0.0 > + if: failure() > + with: > +name: system-userspace > +path: tests/system-userspace-testsuite.dir/ > diff --git a/Makefile.am b/Makefile.am > index b279303d186c..80448d0c31c1 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -92,6 +92,7 @@ EXTRA_DIST = \ > $(MAN_ROOTS) \ > Vagrantfile \ > Vagrantfile-FreeBSD \ > + .github/workflows/ovs.yml \ > .mailmap > bin_PROGRAMS = > sbin_PROGRAMS = ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 ovn] Documentation: Change 'Open vSwitch' for 'OVN and logo
On Wed, Mar 18, 2020 at 10:29 PM Ben Pfaff wrote: > > On Wed, Mar 18, 2020 at 05:31:23PM +0100, Daniel Alvarez Sanchez wrote: > > On Tue, Mar 17, 2020 at 6:57 PM Ben Pfaff wrote: > > > > > On Tue, Mar 17, 2020 at 06:22:07PM +0100, Daniel Alvarez wrote: > > > > The current version of the documentation is still using the > > > > Open vSwitch logo and includes some references to OVS that > > > > should be changed by OVN. > > > > > > > > Signed-off-by: Daniel Alvarez Acked-by: Numan Siddique Thanks Numan > > > > > > Here's the original SVG for the logo. > > > > > > > Thanks Ben, I picked the OVN log and resized to 200x200 as per the Sphinx > > doc [0]. > > I'm a bit puzzled as the OVS logo [1] is 502x334 but somehow it works well > > in the OVS website. > > > > Do you have any suggestions? Thanks a lot! > > I'm very pleased to see that you and others are working on this! I > don't have suggestions right now. > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-northd: Don't add arp responder flows for lports with 'unknown' address.
From: Numan Siddique
If a logical port has 'unknown' address, it means it can send and receive
packet with any IP and MAC and generally port security is not set for
such logical ports. If an lport has addresses set to - ["MAC1 IP1", unknown],
right now we add arp responder flows for IP1 and respond MAC1 in the arp
response. But it's possible that the VIF of the logical port can use the IP1
with a different MAC. This patch supports this usecase. When another logical
port
sends ARP request for IP1, the VIF of the logical port will anyway respond.
Reported-by: Maciej Józefczyk
Signed-off-by: Numan Siddique
---
northd/ovn-northd.8.xml | 5 +++--
northd/ovn-northd.c | 13 -
tests/ovn.at| 16
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 9b44720d1..7d03cbc83 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -699,8 +699,9 @@ output;
These flows are omitted for logical ports (other than router ports or
- localport ports) that are down and for logical ports of
- type virtual.
+ localport ports) that are down, for logical ports of
+ type virtual and for logical ports with 'unknown'
+ address set.
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 4f94680b5..f648d2ea7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1152,7 +1152,7 @@ struct ovn_port {
bool derived; /* Indicates whether this is an additional port
* derived from nbsp or nbrp. */
-
+bool has_unknown; /* If the addresses have 'unknown' defined. */
/* The port's peer:
*
* - A switch port S of type "router" has a router port R as a peer,
@@ -2059,8 +2059,11 @@ join_logical_ports(struct northd_context *ctx,
op->lsp_addrs
= xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
for (size_t j = 0; j < nbsp->n_addresses; j++) {
-if (!strcmp(nbsp->addresses[j], "unknown")
-|| !strcmp(nbsp->addresses[j], "router")) {
+if (!strcmp(nbsp->addresses[j], "unknown")) {
+op->has_unknown = true;
+continue;
+}
+if (!strcmp(nbsp->addresses[j], "router")) {
continue;
}
if (is_dynamic_lsp_address(nbsp->addresses[j])) {
@@ -6127,7 +6130,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap
*ports,
} else {
/*
* Add ARP/ND reply flows if either the
- * - port is up or
+ * - port is up and it doesn't have 'unknown' address defined or
* - port type is router or
* - port type is localport
*/
@@ -6136,7 +6139,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap
*ports,
continue;
}
-if (lsp_is_external(op->nbsp)) {
+if (lsp_is_external(op->nbsp) || op->has_unknown) {
continue;
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 8cdbad743..1b6073ff0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1758,11 +1758,13 @@ for is in 1 2 3; do
sip=`ip_to_hex 192 168 0 $is$js`
tip=`ip_to_hex 192 168 0 $id$jd`
tip_unknown=`ip_to_hex 11 11 11 11`
+reply_ha=;
if test $d != $s; then
-reply_ha=f0$d
-else
-reply_ha=
+if test $jd != 1; then
+reply_ha=f0$d
+fi
fi
+
test_arp $s f0$s $sip $tip $reply_ha #9
test_arp $s f0$s $sip $tip_unknown #10
@@ -2199,7 +2201,13 @@ for s in 1 2 3; do
sip=192.168.0.$s
tip=192.168.0.$d
tip_unknown=11.11.11.11
-if test $d != $s; then reply_ha=f0:00:00:00:00:0$d; else reply_ha=; fi
+reply_ha=;
+if test $d != $s; then
+if test $d != 1; then
+reply_ha=f0:00:00:00:00:0$d;
+fi
+fi
+
test_arp $s f0:00:00:00:00:0$s $sip $tip $reply_ha #9
test_arp $s f0:00:00:00:00:0$s $sip $tip_unknown #10
--
2.24.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] travis: Fix kernel download retry.
On Thu, Mar 19, 2020 at 12:44 PM Ilya Maximets wrote:
>
> On 3/19/20 8:32 AM, David Marchand wrote:
> > wget stops retrying to download a file when hitting fatal http errors
> > like 503.
> > But if a previous try had resulted in a partially downloaded ${file}, the
> > next wget call tries to download to ${file}.1.
> >
> > Example:
> > +wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> > --2020-03-18 20:51:42--
> > https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> > Resolving cdn.kernel.org (cdn.kernel.org)... 151.101.1.176, 151.101.65.176,
> > 151.101.129.176, ...
> > Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443...
> > connected.
> > HTTP request sent, awaiting response... 200 OK
> > Length: 103076276 (98M) [application/x-xz]
> > Saving to: ‘linux-4.16.18.tar.xz’
> >
> > linux-4.16.18.tar.x 0%[] 13.07K --.-KB/sin 0s
> >
> > 2020-03-18 20:54:44 (133 MB/s) - Read error at byte 13383/103076276
> > (Connection reset by peer). Retrying.
> >
> > --2020-03-18 20:54:45-- (try: 2)
> > https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> > Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443...
> > connected.
> > HTTP request sent, awaiting response... 503 first byte timeout
> > 2020-03-18 20:55:46 ERROR 503: first byte timeout.
> >
> > +wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> > --2020-03-18 20:55:46--
> > https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> > Resolving cdn.kernel.org (cdn.kernel.org)... 151.101.1.176, 151.101.65.176,
> > 151.101.129.176, ...
> > Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443...
> > connected.
> > HTTP request sent, awaiting response... 200 OK
> > Length: 103076276 (98M) [application/x-xz]
> > Saving to: ‘linux-4.16.18.tar.xz.1’
> >
> > linux-4.16.18.tar.x 100%[===>] 98.30M 186MB/sin 0.5s
> >
> > 2020-03-18 20:55:56 (186 MB/s) - ‘linux-4.16.18.tar.xz.1’ saved
> > [103076276/103076276]
> >
> > Fixes: 048674b45f4b ("travis: Retry kernel download on 503 first byte
> > timeout.")
> >
> > Signed-off-by: David Marchand
> > ---
> > .travis/linux-build.sh | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> > index 359f7773ba..74e1028573 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -35,7 +35,9 @@ function install_kernel()
> >
> > url="${base_url}/linux-${version}.tar.xz"
> > # Download kernel sources. Try direct link on CDN failure.
> > -wget ${url} || wget ${url} || wget ${url/cdn/www}
> > +wget ${url} ||
> > +(rm -f linux-${version}.tar.xz && wget ${url}) ||
> > +(rm -f linux-${version}.tar.xz && wget ${url/cdn/www})
>
> How about using 'wget -c' instead?
This should work.
Restarting from zero seems safer to me.
Your choice.
--
David Marchand
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] travis: Fix kernel download retry.
On 3/19/20 8:32 AM, David Marchand wrote:
> wget stops retrying to download a file when hitting fatal http errors
> like 503.
> But if a previous try had resulted in a partially downloaded ${file}, the
> next wget call tries to download to ${file}.1.
>
> Example:
> +wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> --2020-03-18 20:51:42--
> https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> Resolving cdn.kernel.org (cdn.kernel.org)... 151.101.1.176, 151.101.65.176,
> 151.101.129.176, ...
> Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 103076276 (98M) [application/x-xz]
> Saving to: ‘linux-4.16.18.tar.xz’
>
> linux-4.16.18.tar.x 0%[] 13.07K --.-KB/sin 0s
>
> 2020-03-18 20:54:44 (133 MB/s) - Read error at byte 13383/103076276
> (Connection reset by peer). Retrying.
>
> --2020-03-18 20:54:45-- (try: 2)
> https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443... connected.
> HTTP request sent, awaiting response... 503 first byte timeout
> 2020-03-18 20:55:46 ERROR 503: first byte timeout.
>
> +wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> --2020-03-18 20:55:46--
> https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
> Resolving cdn.kernel.org (cdn.kernel.org)... 151.101.1.176, 151.101.65.176,
> 151.101.129.176, ...
> Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 103076276 (98M) [application/x-xz]
> Saving to: ‘linux-4.16.18.tar.xz.1’
>
> linux-4.16.18.tar.x 100%[===>] 98.30M 186MB/sin 0.5s
>
> 2020-03-18 20:55:56 (186 MB/s) - ‘linux-4.16.18.tar.xz.1’ saved
> [103076276/103076276]
>
> Fixes: 048674b45f4b ("travis: Retry kernel download on 503 first byte
> timeout.")
>
> Signed-off-by: David Marchand
> ---
> .travis/linux-build.sh | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 359f7773ba..74e1028573 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -35,7 +35,9 @@ function install_kernel()
>
> url="${base_url}/linux-${version}.tar.xz"
> # Download kernel sources. Try direct link on CDN failure.
> -wget ${url} || wget ${url} || wget ${url/cdn/www}
> +wget ${url} ||
> +(rm -f linux-${version}.tar.xz && wget ${url}) ||
> +(rm -f linux-${version}.tar.xz && wget ${url/cdn/www})
How about using 'wget -c' instead?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] db-ctl-base: add uuid specified method for create cmd
Bleep bloop. Greetings Tao YunXiang, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 198 characters long (recommended limit is 79) #102 FILE: lib/db-ctl-base.xml:293: [--id=@name] [--row_uuid=uuid] create table column[:key]=value... WARNING: Line is 90 characters long (recommended limit is 79) #111 FILE: lib/db-ctl-base.xml:307: If uuid is specified, then the UUID for the new row may be WARNING: Line is 92 characters long (recommended limit is 79) #112 FILE: lib/db-ctl-base.xml:308: specified as expected. Such references may precede or follow the create WARNING: Line is 85 characters long (recommended limit is 79) #172 FILE: lib/ovsdb-idl.c:4169: xasprintf(UUID_FMT, UUID_ARGS(&row->uuid; Lines checked: 256, Warnings: 4, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.
On Thu, Mar 19, 2020 at 11:28:00AM +0200, Roi Dayan wrote: > > > On 2020-03-11 7:39 AM, we...@ucloud.cn wrote: > > From: wenxu > > > > The tc modify flow put always delete the original flow first and > > then add the new flow. If the modfiy flow put operation failed, > > the flow put operation will change from modify to create if success > > to delete the original flow in tc (which will be always failed with > > ENOENT, the flow is already be deleted before add the new flow in tc). > > Finally, the modify flow put will failed to add in kernel datapath. > > > > Signed-off-by: wenxu ... > Acked-by: Roi Dayan Thanks, this looks good to me. I am exercising the patch applied on top of master and branch-2.13 using Travis CI. And I plan to push the patch to those branches if that succeeds. The patch does not apply cleanly on branch-2.12. Please consider posting a backport to that and earlier branches if the change is appropriate there. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] bugtool: Fix for Python3
On Wed, 18 Mar 2020 14:25:43 -0700 William Tu wrote: > On Wed, Mar 18, 2020 at 8:23 AM Timothy Redaelli wrote: > > > > Currently ovs-bugtool tool doesn't start on Python3. > > This commit fixes ovs-bugtool to make it works on Python.3 > > > > Reported-at: https://bugzilla.redhat.com/1809241 > > Reported-by: Flavio Leitner > > Signed-off-by: Timothy Redaelli > > --- > > Thanks for the patch. I tried to fix it before but haven't get it done yet. > I run your patch and got this error, can you take a look? > > ovs# ./utilities/bugtool/ovs-bugtool -y -s --output=tar.gz > --outfile=/var/log/bugtool-report.tgz > Traceback (most recent call last): > File "./utilities/bugtool/ovs-bugtool", line 1405, in > sys.exit(main()) > File "./utilities/bugtool/ovs-bugtool", line 717, in main > collect_data() > File "./utilities/bugtool/ovs-bugtool", line 366, in collect_data > run_procs(process_lists.values()) > File "./utilities/bugtool/ovs-bugtool", line 1344, in run_procs > p.read_line() > File "./utilities/bugtool/ovs-bugtool", line 1313, in read_line > self.inst.write(line.decode()) > UnicodeDecodeError: 'utf-8' codec can't decode byte 0xcb in position > 67: invalid continuation byte > > Thanks > William > Thank you, I'll send a v2 as soon as possibile ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [ovs-dev,v3]travis: Enable OvS Travis CI for arm.
Enable part of travis jobs with gcc compiler for arm64 architecture
1. Add arm jobs into the matrix in .travis.yml configuration file
2. To enable OVS-DPDK jobs, set the build target according to
different CPU architectures
3. Temporarily disable sparse checker because of static code checking
failure on arm64
Considering the balance of the CI coverage and running time, some kernel
and DPDK jobs are removed from Arm CI. The running time increases around by
7 minutes to 47 minutes in all.
Successful travis build jobs report:
https://travis-ci.org/github/yzyuestc/ovs/builds/663833478
Reviewed-by: Yanqin Wei
Reviewed-by: Ruifeng Wang
Reviewed-by: JingZhao Ni
Reviewed-by: Gavin Hu
Signed-off-by: Lance Yang
---
v3:
- Remove some kernel jobs: 4.18, 4.17, 4.16, 4.15, 4.14, and 4.3.
- Remove one OvS-DPDK shared library job.
---
.travis.yml| 15 +++
.travis/linux-build.sh | 16 ++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index ef9f867..1149758 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -51,6 +51,21 @@ matrix:
- os: osx
compiler: clang
env: OPTS="--disable-ssl"
+- arch: arm64
+ compiler: gcc
+ env: OPTS="--disable-ssl"
+- arch: arm64
+ compiler: gcc
+ env: KERNEL_LIST="5.5 4.19"
+- arch: arm64
+ compiler: gcc
+ env: KERNEL_LIST="4.9 3.16"
+- arch: arm64
+ compiler: gcc
+ env: DPDK=1 OPTS="--enable-shared"
+- arch: arm64
+ compiler: gcc
+ env: DPDK_SHARED=1
script: ./.travis/${TRAVIS_OS_NAME}-build.sh $OPTS
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 359f777..3140ec9 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -6,7 +6,7 @@ set -x
CFLAGS_FOR_OVS="-g -O2"
SPARSE_FLAGS=""
EXTRA_OPTS="--enable-Werror"
-TARGET="x86_64-native-linuxapp-gcc"
+TARGET=""
function install_kernel()
{
@@ -87,6 +87,16 @@ function install_dpdk()
local DPDK_VER=$1
local VERSION_FILE="dpdk-dir/travis-dpdk-cache-version"
+if [ -z "$TRAVIS_ARCH" ] ||
+ [ "$TRAVIS_ARCH" == "amd64" ]; then
+TARGET="x86_64-native-linuxapp-gcc"
+elif [ "$TRAVIS_ARCH" == "aarch64" ]; then
+TARGET="arm64-armv8a-linuxapp-gcc"
+else
+echo "Target is unknown"
+exit 1
+fi
+
if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
# Avoid using cache for git tree build.
rm -rf dpdk-dir
@@ -178,7 +188,9 @@ elif [ "$M32" ]; then
# difference on 'configure' and 'make' stages.
export CC="$CC -m32"
else
-OPTS="--enable-sparse"
+if [ "$TRAVIS_ARCH" != "aarch64" ]; then
+OPTS="--enable-sparse"
+fi
if [ "$AFXDP" ]; then
# netdev-afxdp uses memset for 64M for umem initialization.
SPARSE_FLAGS="${SPARSE_FLAGS} -Wno-memcpy-max-count"
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5] db-ctl-base: add uuid specified method for create cmd
Hi Ben, Thanks for your review. I have summited v6. The new change has fixed the bug, and add a unit test in ovs-vsctl.at. Please review it again. Thanks, Yun -- taoyunxi...@cmss.chinamobile.com >I guess it's a bug. > >Have you posted the code you tested? Please post a v6 that includes the >test that fails. > >On Wed, Mar 18, 2020 at 11:03:45PM +0800, Timo_Liu wrote: >> > >Hi Ben: > > > > > When using both --id and --row_uuid, I have found the specified uuid only >worked for table whose "is_root" flag is true. For non root table, such as >mirror table, when we execute table create with --id and --row_uuid，the json >transaction includes two ops: insert mirror table (with uuid specified by >--row_uuid)， and update Bridge table column, But for the update operation, the >row->uuid is not the specified uuid any more. > > I don't know why the "uuid-name" key for "insert" operation can tranmit >its uuid value to row->new_datum for "update" operatoin. Can you give me some >advice ? > >Thanks > >Timo > > > > > > > > >Re: Re: [ovs-dev] [PATCH v5] db-ctl-base: add uuid specified method for create >cmdOn Mon, Mar 16, 2020 at 09:31:48AM +0800, Timo_Liu wrote: >> One point that we should confirm with you: if --id && >> --row-uuid both specified, we should use UUID from --row-uuid to >> create table, am I right ? > >Yes. > >> Also, we should add test in which ovsdb unit test file ?(ovs-vsctl.at or >> ovsdb-execution.at) > >I think that ovs-vsctl.at is a good place for testing ovs-vsctl >features. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v6] db-ctl-base: add uuid specified method for create cmd
Commit a529e3cd1f (ovsdb-server: Allow OVSDB clients to specify the
UUID for inserted rows) solves ovsdb-client specifing the UUID for
insert operation. OVSDB now can support directly using uuid to identify
a row. But for xxxctl tool,specifying uuid when creating a row is not
yet supported . This patch tried to specify uuid when creating a row
by the ctl tools. A new parameter --row_uuid is added to setup row's UUID.
e.g. ovn-nbctl --row_uuid=3da0398b-a5a8-4bc9-808d-fa662865138f create
logical_switch name='abc'
Author: Liu Chang
Co-authored-by: Tao YunXiang
Co-authored-by: Rong Yin
Signed-off-by: Tao YunXiang
Signed-off-by: Liu Chang
Signed-off-by: Rong Yin
CC: Ben Pfaff
---
v5: db-ctl-base: add uuid specified method for create cmd
---
NEWS| 1 +
lib/db-ctl-base.c | 20 +++-
lib/db-ctl-base.xml | 7 ++-
lib/ovsdb-idl.c | 24
lib/ovsdb-idl.h | 1 +
tests/ovs-vsctl.at | 50 ++
6 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/NEWS b/NEWS
index 32ca2e0c6..27fc3ae9d 100644
--- a/NEWS
+++ b/NEWS
@@ -54,6 +54,7 @@ v2.13.0 - 14 Feb 2020
replication server. This value is configurable with the unixctl
command - ovsdb-server/set-active-ovsdb-server-probe-interval.
* ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
+ * db-ctl-base: New row_uuid parameter specified for create cmd
- 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
type filter supports new filters: "dpdk" and "partially-offloaded".
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab2af9eda..776037d95 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1718,10 +1718,12 @@ static void
cmd_create(struct ctl_context *ctx)
{
const char *id = shash_find_data(&ctx->options, "--id");
+const char *row_uuid = shash_find_data(&ctx->options, "--row_uuid");
const char *table_name = ctx->argv[1];
const struct ovsdb_idl_table_class *table;
const struct ovsdb_idl_row *row;
const struct uuid *uuid = NULL;
+struct uuid uuid_from_cmd;
int i;
ctx->error = get_table(table_name, &table);
@@ -1741,7 +1743,23 @@ cmd_create(struct ctl_context *ctx)
* warnings about that by pretending that there is a reference. */
symbol->strong_ref = true;
}
+if (row_uuid) {
+if (!uuid_from_string(&uuid_from_cmd, row_uuid)) {
+ctl_error(ctx, "row_uuid '%s' is not a valid UUID", row_uuid);
+return ;
+}
+ovsdb_idl_txn_set_uuid_specified(ctx->txn);
+symbol->uuid = uuid_from_cmd;
+}
uuid = &symbol->uuid;
+
+} else if (row_uuid) {
+if (!uuid_from_string(&uuid_from_cmd, row_uuid)) {
+ctl_error(ctx, "row_uuid '%s' is not a valid UUID", row_uuid);
+return ;
+}
+uuid = &uuid_from_cmd;
+ovsdb_idl_txn_set_uuid_specified(ctx->txn);
}
row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
@@ -2465,7 +2483,7 @@ static const struct ctl_command_syntax db_ctl_commands[]
= {
{"clear", 3, INT_MAX, "TABLE RECORD COLUMN...", pre_cmd_clear, cmd_clear,
NULL, "--if-exists", RW},
{"create", 2, INT_MAX, "TABLE COLUMN[:KEY]=VALUE...", pre_create,
- cmd_create, post_create, "--id=", RW},
+ cmd_create, post_create, "--id=,--row_uuid=", RW},
{"destroy", 1, INT_MAX, "TABLE [RECORD]...", pre_cmd_destroy, cmd_destroy,
NULL, "--if-exists,--all", RW},
{"wait-until", 2, INT_MAX, "TABLE RECORD [COLUMN[:KEY]=VALUE]...",
diff --git a/lib/db-ctl-base.xml b/lib/db-ctl-base.xml
index 10124c3ad..780054b4a 100644
--- a/lib/db-ctl-base.xml
+++ b/lib/db-ctl-base.xml
@@ -290,7 +290,7 @@
-[--id=@name] create table
column[:key]=value...
+[--id=@name]
[--row_uuid=uuid] create table
column[:key]=value...
Creates a new record in table and sets the initial values of
@@ -303,6 +303,11 @@
invocation in contexts where a UUID is expected. Such references may
precede or follow the create command.
+
+If uuid is specified, then the UUID for the
new row may be
+specified as expected. Such references may precede or follow the
create
+command.
+
Caution (ovs-vsctl as example)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 190143f36..505ed00d9 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -281,6 +281,7 @@ struct ovsdb_idl_txn {
char *error;
bool dry_run;
struct ds comment;
+bool uuid_specified;
/* Increments. */
const char *inc_table;
@@ -3550,6 +3551,7 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
txn->status = TXN_UNCOMMITTED;
txn->error = NULL;
txn->dry_run
[ovs-dev] [PATCH v3]travis: Enable OvS Travis CI for arm.
Enable part of travis jobs with gcc compiler for arm64 architecture
1. Add arm jobs into the matrix in .travis.yml configuration file
2. To enable OVS-DPDK jobs, set the build target according to
different CPU architectures
3. Temporarily disable sparse checker because of static code checking
failure on arm64
Considering the balance of the CI coverage and running time, some kernel
and DPDK jobs are removed from Arm CI. The running time increases around
by 7 minutes to 47 minutes in all.
Successful travis build jobs report:
https://travis-ci.org/github/yzyuestc/ovs/builds/663833478
Reviewed-by: Yanqin Wei
Reviewed-by: Ruifeng Wang
Reviewed-by: JingZhao Ni
Reviewed-by: Gavin Hu
Signed-off-by: Lance Yang
---
v3:
- Remove some kernel jobs: 4.18, 4.17, 4.16, 4.15, 4.14, and 4.3.
- Remove one OvS-DPDK shared library job.
---
.travis.yml| 15 +++
.travis/linux-build.sh | 16 ++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index ef9f867..1149758 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -51,6 +51,21 @@ matrix:
- os: osx
compiler: clang
env: OPTS="--disable-ssl"
+- arch: arm64
+ compiler: gcc
+ env: OPTS="--disable-ssl"
+- arch: arm64
+ compiler: gcc
+ env: KERNEL_LIST="5.5 4.19"
+- arch: arm64
+ compiler: gcc
+ env: KERNEL_LIST="4.9 3.16"
+- arch: arm64
+ compiler: gcc
+ env: DPDK=1 OPTS="--enable-shared"
+- arch: arm64
+ compiler: gcc
+ env: DPDK_SHARED=1
script: ./.travis/${TRAVIS_OS_NAME}-build.sh $OPTS
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 359f777..3140ec9 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -6,7 +6,7 @@ set -x
CFLAGS_FOR_OVS="-g -O2"
SPARSE_FLAGS=""
EXTRA_OPTS="--enable-Werror"
-TARGET="x86_64-native-linuxapp-gcc"
+TARGET=""
function install_kernel()
{
@@ -87,6 +87,16 @@ function install_dpdk()
local DPDK_VER=$1
local VERSION_FILE="dpdk-dir/travis-dpdk-cache-version"
+if [ -z "$TRAVIS_ARCH" ] ||
+ [ "$TRAVIS_ARCH" == "amd64" ]; then
+TARGET="x86_64-native-linuxapp-gcc"
+elif [ "$TRAVIS_ARCH" == "aarch64" ]; then
+TARGET="arm64-armv8a-linuxapp-gcc"
+else
+echo "Target is unknown"
+exit 1
+fi
+
if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
# Avoid using cache for git tree build.
rm -rf dpdk-dir
@@ -178,7 +188,9 @@ elif [ "$M32" ]; then
# difference on 'configure' and 'make' stages.
export CC="$CC -m32"
else
-OPTS="--enable-sparse"
+if [ "$TRAVIS_ARCH" != "aarch64" ]; then
+OPTS="--enable-sparse"
+fi
if [ "$AFXDP" ]; then
# netdev-afxdp uses memset for 64M for umem initialization.
SPARSE_FLAGS="${SPARSE_FLAGS} -Wno-memcpy-max-count"
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.
On 2020-03-11 7:39 AM, we...@ucloud.cn wrote:
> From: wenxu
>
> The tc modify flow put always delete the original flow first and
> then add the new flow. If the modfiy flow put operation failed,
> the flow put operation will change from modify to create if success
> to delete the original flow in tc (which will be always failed with
> ENOENT, the flow is already be deleted before add the new flow in tc).
> Finally, the modify flow put will failed to add in kernel datapath.
>
> Signed-off-by: wenxu
> ---
> lib/dpif-netlink.c | 7 ++-
> lib/netdev-offload-tc.c | 2 +-
> lib/netdev-offload.h| 3 +++
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 5b5c96d..b8d08a6 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -2091,6 +2091,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct
> dpif_flow_put *put)
> info.tunnel_csum_on = csum_on;
> info.recirc_id_shared_with_tc = (dpif->user_features
> & OVS_DP_F_TC_RECIRC_SHARING);
> +info.tc_modify_flow_deleted = false;
> err = netdev_flow_put(dev, &match,
>CONST_CAST(struct nlattr *, put->actions),
>put->actions_len,
> @@ -2141,7 +2142,11 @@ parse_flow_put(struct dpif_netlink *dpif, struct
> dpif_flow_put *put)
> out:
> if (err && err != EEXIST && (put->flags & DPIF_FP_MODIFY)) {
> /* Modified rule can't be offloaded, try and delete from HW */
> -int del_err = netdev_flow_del(dev, put->ufid, put->stats);
> +int del_err = 0;
> +
> +if (!info.tc_modify_flow_deleted) {
> +del_err = netdev_flow_del(dev, put->ufid, put->stats);
> +}
>
> if (!del_err) {
> /* Delete from hw success, so old flow was offloaded.
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 550e440..5e7b873 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -1727,7 +1727,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
> *match,
> if (get_ufid_tc_mapping(ufid, &id) == 0) {
> VLOG_DBG_RL(&rl, "updating old handle: %d prio: %d",
> id.handle, id.prio);
> -del_filter_and_ufid_mapping(&id, ufid);
> +info->tc_modify_flow_deleted = !del_filter_and_ufid_mapping(&id,
> ufid);
> }
>
> prio = get_prio_for_tc_flower(&flower);
> diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
> index cd6dfdf..b4b882a 100644
> --- a/lib/netdev-offload.h
> +++ b/lib/netdev-offload.h
> @@ -74,6 +74,9 @@ struct offload_info {
> * it will be in the pkt meta data.
> */
> uint32_t flow_mark;
> +
> +bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success
> + * to delete the original flow. */
> };
>
> int netdev_flow_flush(struct netdev *);
>
Acked-by: Roi Dayan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] conntrack: Reset ct_state when entering a new zone.
On 3/18/20 2:57 PM, Ilya Maximets wrote:
> On 3/5/20 12:28 PM, Dumitru Ceara wrote:
>> When a new conntrack zone is entered, the ct_state field is zeroed in
>> order to avoid using state information from different zones.
>>
>> One such scenario is when a packet is double NATed. Assuming two zones
>> and 3 flows performing the following actions in order on the packet:
>> 1. ct(zone=5,nat), recirc
>> 2. ct(zone=1), recirc
>> 3. ct(zone=1,nat)
>>
>> If at step #1 the packet matches an existing NAT entry, it will get
>> translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
>> At step #2 the new tuple might match an existing connection and
>> pkt->md.ct_zone is set to 1.
>> If at step #3 the packet matches an existing NAT entry in zone 1,
>> handle_nat() will be called to perform the translation but it will
>> return early because the packet's zone matches the conntrack zone and
>> the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
>> translations in zone 5.
>>
>> In order to reliably detect when a packet enters a new conntrack zone
>> we also need to make sure that the pkt->md.ct_zone is properly
>> initialized if pkt->md.ct_state is non-zero. This already happens for
>> most cases. The only exception is when matched conntrack connection is
>> of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
>> cover this path we now call write_ct_md() in that case too. Remove
>> setting the CS_TRACKED flag as in this case as it will be done by the
>> new call to write_ct_md().
>>
>> Also, the error path above seems hard to hit as it would've caused a
>> crash due to dereferencing a NULL pointer when calling:
>> 'ct_print_conn_info(conn, log_msg, VLL_INFO, true, true)'. This patch
>> changes the call to log the 'rev_conn' info instead.
>>
>> CC: Darrell Ball
>> Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
>> Signed-off-by: Dumitru Ceara
>>
>> ---
>> V2:
>> - Address Ilya's comments:
>> - revert changes to pkt_metadata_init().
>> - update ct_state in process_one() only if ct_state is already
>> non-zero.
>> - Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
>> is non-zero.
>> - Fix NULL pointer dereference in process_one() if conn_type is
>> CT_CONN_TYPE_UN_NAT and master conn is not found.
>> ---
>
> 'Fixes' tag seems a bit strange to me. I think it should be:
> Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
>
> Regarding the issue. I've spent some time exploring the conntrack code
> and also researching the original patch that introduced this code.
> The issue was raised during the review of the original patch 286de2729955
> by Daniele: https://patchwork.ozlabs.org/patch/743108/
> And Darrell said that he actually had the similar code that clears ct_state
> during zone transition at the beginning of process_one(). But, he decided
> that most of such issues are likely configuration bugs that should by raised
> to user with warnings.
> However, such warnings was never introduced and since this causes a real
> issue in OVN we should actually have this clearing of conntrack state on
> zone transitioning.
>
> Acked-by: Ilya Maximets
>
> Darrell, Ben, I'd like to have some comments on this from you since I'm
> not an expert in this area. Otherwise, I'm going to apply this patch on
> next week.
>
> Best regards, Ilya Maximets.
>
Hi Ilya,
Thanks for the review. I'll send a v3 with updated 'Fixes' tag and your
ack before next week unless there are more concerns from other reviewers.
Thanks,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] travis: Fix kernel download retry.
wget stops retrying to download a file when hitting fatal http errors
like 503.
But if a previous try had resulted in a partially downloaded ${file}, the
next wget call tries to download to ${file}.1.
Example:
+wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
--2020-03-18 20:51:42--
https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
Resolving cdn.kernel.org (cdn.kernel.org)... 151.101.1.176, 151.101.65.176,
151.101.129.176, ...
Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 103076276 (98M) [application/x-xz]
Saving to: ‘linux-4.16.18.tar.xz’
linux-4.16.18.tar.x 0%[] 13.07K --.-KB/sin 0s
2020-03-18 20:54:44 (133 MB/s) - Read error at byte 13383/103076276 (Connection
reset by peer). Retrying.
--2020-03-18 20:54:45-- (try: 2)
https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443... connected.
HTTP request sent, awaiting response... 503 first byte timeout
2020-03-18 20:55:46 ERROR 503: first byte timeout.
+wget https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
--2020-03-18 20:55:46--
https://cdn.kernel.org/pub/linux/kernel/v4.x/linux-4.16.18.tar.xz
Resolving cdn.kernel.org (cdn.kernel.org)... 151.101.1.176, 151.101.65.176,
151.101.129.176, ...
Connecting to cdn.kernel.org (cdn.kernel.org)|151.101.1.176|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 103076276 (98M) [application/x-xz]
Saving to: ‘linux-4.16.18.tar.xz.1’
linux-4.16.18.tar.x 100%[===>] 98.30M 186MB/sin 0.5s
2020-03-18 20:55:56 (186 MB/s) - ‘linux-4.16.18.tar.xz.1’ saved
[103076276/103076276]
Fixes: 048674b45f4b ("travis: Retry kernel download on 503 first byte timeout.")
Signed-off-by: David Marchand
---
.travis/linux-build.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 359f7773ba..74e1028573 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -35,7 +35,9 @@ function install_kernel()
url="${base_url}/linux-${version}.tar.xz"
# Download kernel sources. Try direct link on CDN failure.
-wget ${url} || wget ${url} || wget ${url/cdn/www}
+wget ${url} ||
+(rm -f linux-${version}.tar.xz && wget ${url}) ||
+(rm -f linux-${version}.tar.xz && wget ${url/cdn/www})
tar xvf linux-${version}.tar.xz > /dev/null
pushd linux-${version}
--
2.23.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
