Re: [ovs-dev] [PATCH 4/8] system-traffic.at: Add gre tunnel test that doesn't depend on upstream gre module

2018-08-10 Thread Yifeng Sun
-- ## > ## openvswitch 2.10.90 test suite. ## > ## --- ## > 18: datapath - ping over gre tunnel by simulated packets ok > > ## - ## > ## Test results. ## > ## --------- ## > > 1 test was successfu

[ovs-dev] [PATCH v2 1/4] ip6_gre: Fix a bug that clears address bits

2018-08-15 Thread Yifeng Sun
In compatable gre module, skb->cb is solely used as ovs_gso_cb. However, IPCB(skb) also points to skb->cb. IPCB(skb)->flags overlaps with ovs_gso_cb.tun_dst. As a result, this bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst and causes kernel to crash. Signed-off-by: Y

[ovs-dev] [PATCH v2 4/4] system-traffic: Add 5 new tunnel tests that don't depend on native linux modules

2018-08-15 Thread Yifeng Sun
Introduce 5 new tests that don't require native gre or erspan tunnels but sends simulated raw packets. These tests are supposed to only run for kernel version from 3.10.x to 4.15.x where compatible gre is being used by OVS kernel module. Signed-off-by: Yifeng Sun --- v1->v2: Add supp

[ovs-dev] [PATCH v2 2/4] test: Add two m4 functions to skip tests for certain kernel versions

2018-08-15 Thread Yifeng Sun
patches. Signed-off-by: Yifeng Sun --- v1->v2: Let tests skip in check-system-userspace when they always fail. tests/system-kmod-macros.at | 22 ++ tests/system-userspace-macros.at | 16 2 files changed, 38 insertions(+) diff --git a/tests/system-k

[ovs-dev] [PATCH v2 3/4] system-traffic: Skip 5 tunnel tests on certain kernel versions

2018-08-15 Thread Yifeng Sun
Skip gre, erspan and ip6erspan related tests on kernel version from 3.10.x to 4.15.x because compatable gre is used and these tests will always fail. Signed-off-by: Yifeng Sun --- tests/system-traffic.at | 5 + 1 file changed, 5 insertions(+) diff --git a/tests/system-traffic.at b/tests

Re: [ovs-dev] [PATCH 4/8] system-traffic.at: Add gre tunnel test that doesn't depend on upstream gre module

2018-08-14 Thread Yifeng Sun
1], [10.1.1.100/24] >> >> OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 2 fc00:100::100]) >> >> -ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap & >> +ip netns exec at_ns0 tcpdump -U proto gre -i p0 -w p0.pcap & >> sleep 1 >> >> dnl First, check the underlay. >> @@ -800,10

[ovs-dev] [PATCH] dns-resolve: Fix a bug that frees node inside HMAP_FOR_EACH

2018-08-16 Thread Yifeng Sun
HMAP_FOR_EACH_SAFE should be used when node is freed inside. Signed-off-by: Yifeng Sun --- lib/dns-resolve.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dns-resolve.c b/lib/dns-resolve.c index 299ab27ab5ca..3c6d70e8fbba 100644 --- a/lib/dns-resolve.c +++ b/lib

[ovs-dev] [PATCH] porting: Add fixes to support kernel 4.15.x

2018-08-16 Thread Yifeng Sun
will be done later. Signed-off-by: Yifeng Sun --- acinclude.m4 | 6 -- datapath/linux/compat/vxlan.c | 10 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index 6e7ea4c..9fffe9c 100644 --- a/acinclude.m4 +++ b/acinclude.m4

Re: [ovs-dev] [PATCH] porting: Add fixes to support kernel 4.15.x

2018-08-16 Thread Yifeng Sun
Thanks Greg, I added your diff to the patch and added you in signed-off-by. Yifeng On Thu, Aug 16, 2018 at 11:14 AM, Gregory Rose wrote: > On 8/16/2018 5:06 AM, Yifeng Sun wrote: > > This patch enables OVS kernel module to run on kernel 4.15.x. > Two conntrack-related

Re: [ovs-dev] [PATCH v2] porting: Add fixes to support kernel 4.15.x

2018-08-16 Thread Yifeng Sun
Yes, he should be considered a co-author. Should I use co-authored-by in this case? Thanks, Yifeng On Thu, Aug 16, 2018 at 2:36 PM, Ben Pfaff wrote: > On Thu, Aug 16, 2018 at 06:19:39AM -0700, Yifeng Sun wrote: > > This patch enables OVS kernel module to run on kernel 4.15.x. > >

[ovs-dev] [PATCH v2] porting: Add fixes to support kernel 4.15.x

2018-08-16 Thread Yifeng Sun
will be done later. Signed-off-by: Greg Rose Signed-off-by: Yifeng Sun --- v1->v2: Add travis and documentation from Greg. .travis.yml| 10 +- Documentation/faq/releases.rst | 2 +- acinclude.m4 | 6 -- datapath/linux/compat/vxlan.c | 10 ++

[ovs-dev] [PATCH v3] porting: Add fixes to support kernel 4.15.x

2018-08-16 Thread Yifeng Sun
will be done later. Signed-off-by: Greg Rose Signed-off-by: Yifeng Sun Co-authored-by: Greg Rose Co-authored-by: Yifeng Sun --- v1->v2: Add travis and documentation from Greg. v2->v3: Fix co-authored-by, thanks Ben. .travis.yml| 10 +- Documentation/faq/releases.rs

Re: [ovs-dev] [PATCH 1/8] ip6_gre: Fix a bug that clears address bits

2018-08-09 Thread Yifeng Sun
fix. >> >> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun >> wrote: >> >> In compatable gre module, skb->cb is used as ovs_gso_cb. >>> This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst. >>> >>> can you explain more about ovs_gs

Re: [ovs-dev] [PATCH 1/8] ip6_gre: Fix a bug that clears address bits

2018-08-09 Thread Yifeng Sun
u, Aug 9, 2018 at 3:59 PM, Gregory Rose wrote: > On 8/8/2018 11:32 AM, Yifeng Sun wrote: > >> In compatable gre module, skb->cb is used as ovs_gso_cb. >> This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst. >> >> Signed-off-by: Yifeng Sun >>

Re: [ovs-dev] [PATCH 1/8] ip6_gre: Fix a bug that clears address bits

2018-08-09 Thread Yifeng Sun
Yes, I agree. It should be the way to go. Thanks, Yifeng On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose wrote: > On 8/9/2018 11:03 AM, Yifeng Sun wrote: > > There is a difference regarding how skb_tunnel_info is stored in skb > between ovs and upstream kernel. In ovs's compatibl

Re: [ovs-dev] [PATCH 4/8] system-traffic.at: Add gre tunnel test that doesn't depend on upstream gre module

2018-08-09 Thread Yifeng Sun
. On Thu, Aug 9, 2018 at 11:55 AM, William Tu wrote: > > > On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun > wrote: > >> Introduce a new test that doesn't setup native gre tunnels but sends >> simulated raw packets. >> This test is supposed to only run for ke

Re: [ovs-dev] [PATCH 3/8] system-traffic.at: Skip gre-related failed tests on kernel version from 4.4 to 4.15

2018-08-09 Thread Yifeng Sun
Good question. I tried on RHEL7x but somehow these tests failed. For now, I have run these tests on 4.4, 4.15, 4.16 and they all passed. On Thu, Aug 9, 2018 at 11:59 AM, William Tu wrote: > > > On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun > wrote: > >> Skip gre, erspan

Re: [ovs-dev] [PATCH 6/8] system-traffic.at: Add erspan v2 tunnel test that doesn't depend on upstream gre module

2018-08-09 Thread Yifeng Sun
, br0 actually generate ERSPAN packets, which are captured on p0 through tcpdump. On Thu, Aug 9, 2018 at 12:09 PM, William Tu wrote: > > > On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun > wrote: > >> Introduce a new test that doesn't setup native erspan v2 tunnels but sends >

Re: [ovs-dev] [PATCH 6/8] system-traffic.at: Add erspan v2 tunnel test that doesn't depend on upstream gre module

2018-08-09 Thread Yifeng Sun
Yes, exactly. Thanks, Yifeng On Thu, Aug 9, 2018 at 1:13 PM, William Tu wrote: > > > On Thu, Aug 9, 2018 at 12:52 PM, Yifeng Sun > wrote: > >> The above packet hex is supposed to be generated by linux native tunnel. >> But since native tunnel conflicts with openv

Re: [ovs-dev] [PATCH] netdev-linux: Avoid division by 0 if kernel reports bad scheduler data.

2018-08-20 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Sat, Aug 18, 2018 at 10:17 AM, Ben Pfaff wrote: > If the kernel reported a value of 0 for the second value in > /proc/net/psched, it would cause a division-by-zero fault in > read_psched(). I don't know of a kernel that would ac

Re: [ovs-dev] [PATCH] socket-util: Rate limit logs for bind attempts.

2018-08-20 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Mon, Aug 20, 2018 at 4:10 PM, Ben Pfaff wrote: > This reduces the amount of logging when higher-level code retries binding > ports that are in use. > > Signed-off-by: Ben Pfaff > --- > lib/socket-util.c | 3 ++- &g

Re: [ovs-dev] [PATCH 1/2] porting: Support for kernel 4.16.x & 4.17.x

2018-08-24 Thread Yifeng Sun
Thanks Greg for your review and testing. Yifeng On Thu, Aug 23, 2018 at 10:17 PM, Gregory Rose wrote: > On 8/21/2018 7:42 AM, Yifeng Sun wrote: > > Add support for kernel version up to 4.17.x. On Travis, build passed > for all kernel versions. And no new test fails ar

[ovs-dev] [PATCH 2/2] tests: Extend tests of simulated packets to kernel 4.17.x

2018-08-21 Thread Yifeng Sun
GRE-related tests are skipped on ubuntu-18.04.1 because the vanilla `ip` will fail to set dev's mac address. This bug is described in this link: https://bugzilla.redhat.com/show_bug.cgi?id=1550097 This patch enables GRE tests to run even if the buggy `ip` is being used. Signed-off-by: Yifeng

[ovs-dev] [PATCH 1/2] porting: Support for kernel 4.16.x & 4.17.x

2018-08-21 Thread Yifeng Sun
-by: Yifeng Sun --- .travis.yml | 2 ++ Documentation/faq/releases.rst | 2 +- NEWS | 2 +- acinclude.m4 | 6 ++-- datapath/linux/Modules.mk| 1

Re: [ovs-dev] [PATCH 2/4] nx-match: Avoid double-free on some error paths.

2018-08-27 Thread Yifeng Sun
Looks good to me, thanks for the fix. Reviewed-by: Yifeng Sun On Fri, Aug 24, 2018 at 2:50 PM Ben Pfaff wrote: > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9966 > Reported-at > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9966Reported-

Re: [ovs-dev] [RFC patch v1] datapath: Fix builds on older kernels.

2018-08-27 Thread Yifeng Sun
an never tell > what's getting backported and > to which kernel. Keeps us busy! > > Thanks Darrell, looks like a good catch. I'll let Yifeng provide the > review. > > - Greg > > > > > CC: Yifeng Sun > > Fixes: bf61b8b1c1db ("datapath: Add support for kernel 4

Re: [ovs-dev] [PATCH 3/4] ofp-port: Fix leak on error path in parse_intel_port_custom_property().

2018-08-27 Thread Yifeng Sun
It seems (struct ofputil_port_stats).custom_stats.counters is still leaked by the below code path even after this fix. parse_intel_port_custom_property <- ofputil_pull_ofp14_port_stats <- ofputil_decode_port_stats <- ofputil_count_port_stats I created a diff, how do you like it? Thanks. diff

[ovs-dev] [PATCH] ip_tunnel: Fix bugs that could crash kernel

2018-07-20 Thread Yifeng Sun
tcpdump -U -i br-underlay -w underlay.pcap & sleep 1 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP Signed-off-by: Yifeng Sun --- datapath/linux/compat/ip_t

Re: [ovs-dev] [PATCH v2] ofp-actions: Split ofpacts_check__() into many functions.

2018-07-17 Thread Yifeng Sun
FPM13_MAX) ? OFPERR_OFPMMFC_INVALID_METER : 0; Other than that, this patch looks good to me. 'make check' passed all tests. Thanks. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Thu, Jul 12, 2018 at 4:33 PM, Ben Pfaff wrote: > On Fri, Jun 15, 2018 at 04:29:22PM -0700, Ben Pfaff wrote: > &g

Re: [ovs-dev] [PATCH v2] ofp-actions: Split ofpacts_check__() into many functions.

2018-07-18 Thread Yifeng Sun
Sorry I made a mistake in the previous review. Operator '||' has higher precedence than operator '?:'. So the patch is correct. Thanks, Yifeng On Tue, Jul 17, 2018 at 1:57 PM, Yifeng Sun wrote: > Hi Ben, > > I found a small issue: > > +{ > +uint32_t mid = a->meter_

Re: [ovs-dev] [PATCH 5/5] netdev: Clean up class initialization.

2018-07-16 Thread Yifeng Sun
Thanks. Looks good to me. Reviewed-by: Yifeng Sun On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff wrote: > The macros are hard to read. This makes it a little more readable. > > Signed-off-by: Ben Pfaff > --- > configure.ac | 1 + > lib/netde

Re: [ovs-dev] [PATCH 1/5] netdev-dpdk: Fix incorrect byte order conversion in log message.

2018-07-16 Thread Yifeng Sun
Thanks. Reviewed-by: Yifeng Sun On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff wrote: > uint8_t values shouldn't be passed to ntohs(). > > Found by soarse. > > Signed-off-by: Ben Pfaff > --- > lib/netdev-dpdk.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions

Re: [ovs-dev] [PATCH 2/5] netdev-dpdk: Fix sparse complaints.

2018-07-16 Thread Yifeng Sun
Thanks for the fix. I am wondering why there was no running issue when dl_type is compared with wrong byte order. Reviewed-by: Yifeng Sun On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff wrote: > Neither of these is a real problem. > > Signed-off-by: Ben Pfaff > --- > lib/ne

Re: [ovs-dev] [PATCH] dynamic-string: Fix a bug that leads to assertion fail

2018-07-24 Thread Yifeng Sun
s nothing. Thanks, Yifeng On Tue, Jul 24, 2018 at 2:47 PM, Ben Pfaff wrote: > On Tue, Jul 24, 2018 at 08:37:08AM -0700, Yifeng Sun wrote: > > 'needed' should be size of needed memory space beyond allocated. > > > > Signed-off-by: Yifeng Sun > > Reported-by: Yun Zhou &g

Re: [ovs-dev] [ovs-discuss] ovsdb-server core dump and ovsdb corruption using raft cluster

2018-07-24 Thread Yifeng Sun
(args, args_); available = ds->allocated - ds->length + 1; Thanks, Yifeng Sun On Wed, Jul 18, 2018 at 10:48 AM, Girish Moodalbail wrote: > Hello all, > > We are able to reproduce this issue on OVS 2.9.2 at will. The OVSDB NB > server or OVSDB SB server dumps core

Re: [ovs-dev] [PATCH] ofproto-dpif: Fix typo in registered command

2018-07-24 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Mon, Jul 23, 2018 at 7:45 AM, Alin Gabriel Serdean wrote: > Also split line at 79 characters. > > Found by inspection. > > Signed-off-by: Alin Gabriel Serdean > --- > ofproto/ofproto-dpif.c | 3 ++- > 1 file cha

[ovs-dev] [PATCH] dynamic-string: Fix a bug that leads to assertion fail

2018-07-24 Thread Yifeng Sun
'needed' should be size of needed memory space beyond allocated. Signed-off-by: Yifeng Sun Reported-by: Yun Zhou Reported-by: Girish Moodalbail --- lib/dynamic-string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c index

Re: [ovs-dev] [PATCH] dynamic-string: Fix a bug that leads to assertion fail

2018-07-24 Thread Yifeng Sun
In practice, it is often easier just to use 'asprintf', below. > > *Attention:* In versions of the GNU C Library prior to 2.1 the > return value is the number of characters stored, not including the > terminating null; unless there was not enough space in S to store >

Re: [ovs-dev] [ovs-discuss] ovsdb-server core dump and ovsdb corruption using raft cluster

2018-07-24 Thread Yifeng Sun
My apologize, the patch has some issue. I need to dig further. Yifeng On Tue, Jul 24, 2018 at 1:40 PM, Yifeng Sun wrote: > Hi Yun and Girish, > > I submitted a patch, do you mind testing and reviewing it? Thanks. > > [PATCH] dynamic-string: Fix a bug that leads to assertion fail

Re: [ovs-dev] [PATCH v2 2/2] Datapath: Remove ovs_vport_init unreachable code

2018-07-19 Thread Yifeng Sun
Thanks for the work. Signed-off-by: Yifeng Sun On Thu, Jul 19, 2018 at 11:34 AM, Alin Gabriel Serdean wrote: > The line "ovs_stt_cleanup_module();" was unreachable. > Found using static analysis tools. > > Signed-off-by: Alin Gabriel Serdean > Co-authored-by: Yifen

Re: [ovs-dev] [PATCH 4/4] ofp-actions: Re-fix error path for parsing OpenFlow actions.

2018-08-30 Thread Yifeng Sun
Looks good to me, thanks. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Fri, Aug 24, 2018 at 2:51 PM Ben Pfaff wrote: > A previous commit attempted to fix the error path when the actions nested > within clone provoked an error. However, this commit just introduced a new >

Re: [ovs-dev] [PATCH 1/4] tests: Add regression tests for all the bugs found by oss-fuzz so far.

2018-08-30 Thread Yifeng Sun
Looks good to me and the added tests run fine. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Fri, Aug 24, 2018 at 5:09 PM Ben Pfaff wrote: > This should actually be at the end of the series because the series > fixes some of the bugs that it tests for. Other than ordering, the &g

Re: [ovs-dev] [PATCH v2 1/2] ofp-port: Further cleanups and fixes for ofputil_decode_port_stats().

2018-09-10 Thread Yifeng Sun
Thanks for the fix, looks good to me. Reviewed-by: Yifeng Sun On Thu, Aug 30, 2018 at 1:59 PM Ben Pfaff wrote: > This fixes leaks on the error path in parse_intel_port_custom_property(). > > ofp_print_ofpst_port_reply() failed to free the custom_stats in decoded > port stats.

Re: [ovs-dev] [patch v2] datapath: Fix builds on older kernels.

2018-08-29 Thread Yifeng Sun
Thanks for the fix. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Tue, Aug 28, 2018 at 7:54 PM Darrell Ball wrote: > On older kernels, for example 3.19, the function rt6_get_cookie() is > not available and used with ipv6 config enabled; it was introduced in > 4.2.

[ovs-dev] [PATCH] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.

2018-07-05 Thread Yifeng Sun
When MAX_MTU is larger than hw supported max MTU, dpif_netlink_rtnl_create will fail. This leads to testing failure '11: datapath - ping over gre tunnel' in 'make check-kmod'. This patch fixes this issue by retrying a smaller MTU when MAX_MTU is too large. Signed-off-by: Yifeng Sun --- lib

Re: [ovs-dev] [PATCH 2/2] ovn-nbctl: Clarify error messages in qos-add command.

2018-07-09 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Sat, Jul 7, 2018 at 2:11 PM, Justin Pettit wrote: > Signed-off-by: Justin Pettit > --- > ovn/utilities/ovn-nbctl.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/ovn/utilities

Re: [ovs-dev] [ovs-dev, 1 of 2] ovn-nbctl: Correct qos-add documentation.

2018-07-09 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Sat, Jul 7, 2018 at 2:55 PM, 0-day Robot wrote: > Bleep bloop. Greetings Justin Pettit, I am a robot and I have tried out > your patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting.

Re: [ovs-dev] [PATCH] datapath: work around the single GRE receive limitation.

2018-07-10 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Tue, Jul 10, 2018 at 10:50 AM, William Tu wrote: > Commit 9f57c67c379d ("gre: Remove support for sharing GRE protocol hook") > allows only single GRE packet receiver. When upstream kernel's gre module > is loaded, the

[ovs-dev] [PATCH] tests: Add gre test that doesn't requiring Linux gre module

2018-07-10 Thread Yifeng Sun
gre module and emulating a virtual Linux gre port that sends out arp and icmp packets. Suggested-by: William Tu Signed-off-by: Yifeng Sun --- tests/system-common-macros.at | 13 + tests/system-traffic.at | 67 +++ 2 files changed, 80 insertions

Re: [ovs-dev] [PATCH 2/2] ovs-ofctl: New helper command "parse-packet".

2018-07-11 Thread Yifeng Sun
Thanks. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Mon, Jul 9, 2018 at 1:04 PM, Ben Pfaff wrote: > This was useful for testing the previous patch. > > Signed-off-by: Ben Pfaff > --- > utilities/ovs-ofctl.c | 19 +++ > 1 file changed, 19 insertions

Re: [ovs-dev] [PATCH] configure: Disable -Wnull-pointer-arithmetic Clang warning.

2018-07-11 Thread Yifeng Sun
Thanks. Reviewed-by: Yifeng Sun On Mon, Jul 9, 2018 at 2:37 PM, Ben Pfaff wrote: > OVS trips over this warning all over the place, so it's not worth leaving > on. > > Signed-off-by: Ben Pfaff > --- > configure.ac | 1 + > 1 file changed, 1 insertion(+) > >

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.

2018-07-06 Thread Yifeng Sun
below: dpif_netlink_rtnl|WARN|setting MTU of tunnel gre_sys failed (Invalid argument) Thanks, Yifeng On Fri, Jul 6, 2018 at 10:58 AM, Ben Pfaff wrote: > On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > > When MAX_MTU is larger than hw supported max MTU, > > dpif_netl

Re: [ovs-dev] [PATCH] ofp-group: Don't assert-fail decoding bad OF1.5 group mod type or command.

2018-07-06 Thread Yifeng Sun
Looks good to me. Thanks. Reviewed-by: Yifeng Sun On Thu, Jul 5, 2018 at 3:28 PM, Ben Pfaff wrote: > When decoding a group mod, the current code validates the group type and > command after the whole group mod has been decoded. The OF1.5 decoder, > however, tries to use the type an

Re: [ovs-dev] [PATCH] DNS: Fix build issue with dns_resolve_init().

2018-07-06 Thread Yifeng Sun
Good catch! Thanks. Reviewed-by: Yifeng Sun On Fri, Jul 6, 2018 at 2:51 PM, Justin Pettit wrote: > CC: Yifeng Sun > Fixes: 771680d96f ("DNS: Add basic support for asynchronous DNS resolving") > Signed-off-by: Justin Pettit > --- > lib/dns-resolve-stub.c |

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.

2018-07-06 Thread Yifeng Sun
Oh yes, I missed that when I tried to make code look compact. Thanks for pointing it out. Yifeng On Fri, Jul 6, 2018 at 1:17 PM, Ben Pfaff wrote: > On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > > When MAX_MTU is larger than hw supported max MTU, > > dpif_netl

[ovs-dev] [PATCHv2] dpif-netlink-rtnl: Retry smaller MTU when default MAX_MTU is too large.

2018-07-06 Thread Yifeng Sun
When MAX_MTU is larger than hw supported max MTU, dpif_netlink_rtnl_create will fail. This leads to testing failure '11: datapath - ping over gre tunnel' in 'make check-kmod'. This patch fixes this issue by retrying a smaller MTU when MAX_MTU is too large. Signed-off-by: Yifeng Sun --- v1 ->

Re: [ovs-dev] [PATCH v2] ovs-ofctl: New helper command "parse-packet".

2018-07-12 Thread Yifeng Sun
Thanks. It worked like a charm. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Tue, Jul 10, 2018 at 1:40 PM, Ben Pfaff wrote: > This was useful for testing commit 4fe080160685 ("flow: Fix buffer overread > for crafted IPv6 packets."). > > Signed-off-by: Ben Pfaff &

Re: [ovs-dev] [PATCH] tests: Add gre test that doesn't requiring Linux gre module

2018-07-12 Thread Yifeng Sun
Hi Darrell, Thanks. There are really lots of things to consider. I will come up with v2. Best, Yifeng On Thu, Jul 12, 2018 at 7:14 PM, Darrell Ball wrote: > > > On Wed, Jul 11, 2018 at 2:49 PM, Yifeng Sun > wrote: > >> Hi Darrell, >> >> Thanks for th

Re: [ovs-dev] [PATCH] tests: Add gre test that doesn't requiring Linux gre module

2018-07-11 Thread Yifeng Sun
Hi William, Thanks for the test. I will do. Best, Yifeng On Wed, Jul 11, 2018 at 4:19 PM, William Tu wrote: > Thanks for the patch. > > On Tue, Jul 10, 2018 at 12:24 PM, Yifeng Sun > wrote: > > Currently check-kmod's gre test is broken on certain kernel > > version

Re: [ovs-dev] [PATCH] tests: Add gre test that doesn't requiring Linux gre module

2018-07-11 Thread Yifeng Sun
--- ## > ## openvswitch 2.9.90 test suite. ## > ## -- ## > 111: nsh - forward ok > > ## ----- ## > ## Test results. ## > ## - ## > > 1 test was successful. > make: Leaving directory `/home/dbal

Re: [ovs-dev] [PATCH 2/2] Datapath: Fix ovs_vport_init unreachable code and goto labels

2018-07-09 Thread Yifeng Sun
I think the correct fix may be as follows, do you mind rechecking it? Thanks. diff --git a/datapath/vport.c b/datapath/vport.c index 02f6b56d3243..fcf0fea0a245 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -93,7 +93,6 @@ int ovs_vport_init(void) goto err_stt;

Re: [ovs-dev] [PATCH 1/2] Datapath: Cleanup compat ip6_tunnel.c

2018-07-09 Thread Yifeng Sun
Good catch, thanks. Reviewed-by: Yifeng Sun On Mon, Jul 9, 2018 at 6:09 AM, Alin Gabriel Serdean wrote: > Remove double assignment of `ip6_tnl *t`. > > Signed-off-by: Alin Gabriel Serdean > --- > datapath/linux/compat/ip6_tunnel.c | 2 -- > 1 file changed, 2 deletions

[ovs-dev] [PATCHv4] DNS: Add basic support for asynchronous DNS resolving

2018-01-22 Thread Yifeng Sun
as unbound library doesn't look at it; - For async-resolving, caller need to retry later; there is no callback. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- v1 -> v2: refactored and impr

Re: [ovs-dev] [PATCH] ovn-northd: Consistently use Datapath_Binding UUID for hashing flows.

2018-03-08 Thread Yifeng Sun
Thanks for the fix. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Feb 23, 2018 at 1:03 PM, Ben Pfaff <b...@ovn.org> wrote: > In one place, ovn-northd was hashing Logical_Switch or Logical_Router UUIDs > for ovn_lflow, and in another place it was hashing Data

Re: [ovs-dev] [PATCH] json: Avoid extra memory allocation and string copy parsing object members.

2018-03-26 Thread Yifeng Sun
Thanks for the fix, looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Mar 23, 2018 at 3:46 PM, Ben Pfaff <b...@ovn.org> wrote: > Until now, every time the JSON parser added an object member, it made an > extra copy of the member name and then freed

Re: [ovs-dev] [PATCH] seq: Avoid some "possible leak" warnings from valgrind.

2018-03-26 Thread Yifeng Sun
Thanks for the fix. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Mar 23, 2018 at 3:46 PM, Ben Pfaff <b...@ovn.org> wrote: > valgrind regards a block to be "possibly" leaked when no pointers exist to > the beginning of the block but some pointers do point

[ovs-dev] [PATCH 2/2] ofp-flow: minimatch is initialized twice.

2018-04-03 Thread Yifeng Sun
It is possible that 'fm->match' gets initialized twice in this function, which makes the first one leak because its pointer is overwritten by the second initialization. This patch fixes this issue. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- lib/ofp-flow.c | 1 - 1 file c

[ovs-dev] [PATCH 1/2] ovsdb-idl: properly destroy ovsdb_idl.server

2018-04-03 Thread Yifeng Sun
bridge_run (bridge.c:2949) by 0x406FB4: main (ovs-vswitchd.c:121) Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- lib/ovsdb-idl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index eb28988b8127..49680d6ea330

Re: [ovs-dev] [PATCH 1/4] match: Add 'tun_md' member to struct minimatch.

2018-03-22 Thread Yifeng Sun
Thanks, looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Mar 20, 2018 at 1:46 PM, Ben Pfaff <b...@ovn.org> wrote: > struct match has had a 'tun_md' member for a long time, but struct > minimatch has never had one. This doesn't matter for the pu

Re: [ovs-dev] [PATCH 3/4] flow, match, classifier: Add new functions for miniflow and minimatch.

2018-03-22 Thread Yifeng Sun
Thanks, looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Mar 20, 2018 at 1:46 PM, Ben Pfaff <b...@ovn.org> wrote: > The miniflow and minimatch APIs lack several of the features of the flow > and match APIs. This commit adds a few of the

Re: [ovs-dev] [PATCH 2/4] flow: Improve type-safety of MINIFLOW_GET_TYPE.

2018-03-22 Thread Yifeng Sun
Thanks, looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Mar 20, 2018 at 1:46 PM, Ben Pfaff <b...@ovn.org> wrote: > Until mow, this macro has blindly read the passed-in type's size, but > that's unnecessarily risky. This commit changes it to verify

Re: [ovs-dev] [PATCH 4/4] ofp-flow: Reduce memory consumption for ofputil_flow_mod, using minimatch.

2018-03-22 Thread Yifeng Sun
Thanks for these changes, looks good to me. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Mar 20, 2018 at 1:46 PM, Ben Pfaff <b...@ovn.org> wrote: > Until now, struct ofputil_flow_mod, which represents a

Re: [ovs-dev] [PATCHv4] DNS: Add basic support for asynchronous DNS resolving

2018-02-26 Thread Yifeng Sun
Hi Ben, Thanks a lot for your comment. I will check on the things you mentioned. Best, Yifeng On Mon, Feb 26, 2018 at 2:59 PM, Ben Pfaff <b...@ovn.org> wrote: > On Mon, Jan 22, 2018 at 09:20:36AM -0800, Yifeng Sun wrote: > > This patch is a simple implementation for the pro

[ovs-dev] [PATCHv2] gre: Fix GRE loading bug by separating ERSPAN from tunnel

2018-06-28 Thread Yifeng Sun
://travis-ci.org/yifsun/ovs-travis/builds/397915843 Signed-off-by: Yifeng Sun --- v1 -> v2: remove linux version checking and add HAVE_METADATA_IP_TUNNEL checking according to Greg's review. acinclude.m4 | 7 +-- datapath/linux/compat/include/

Re: [ovs-dev] [PATCH] gre: Fix GRE loading bug by separating ERSPAN from tunnel

2018-06-28 Thread Yifeng Sun
Thanks for the review. I sent another patch. Best, Yifeng On Thu, Jun 28, 2018 at 8:03 AM, Gregory Rose wrote: > On 6/27/2018 12:02 PM, Yifeng Sun wrote: > >> On kernel versions (for example, 3.10) where upstream provides GRE >> functionality but no ERSPAN, GRE can fail to i

[ovs-dev] [PATCH] ovsdb-client: Fix a bug that uses wrong index

2018-09-27 Thread Yifeng Sun
This patch fixes the incorrect index to argv. Signed-off-by: Yifeng Sun --- ovsdb/ovsdb-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index ab3aa7c93307..7c8a59d0e749 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb

Re: [ovs-dev] [PATCH] odp-util: Fix a use-afer-free bug

2018-10-05 Thread Yifeng Sun
This patch should also fix the bug reported at https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10802 On Fri, Oct 5, 2018 at 2:50 PM Yifeng Sun wrote: > After ofpbug_put, actions may have been reallocated and > key will point to invalid memory address. > > Report

[ovs-dev] [PATCH v3] extend-table: Fix a bug that iterates wrong table

2018-10-05 Thread Yifeng Sun
This seems to be a copy and paste bug that iterates and frees the wrong table. This commit fixes that. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10730 Co-authored-by: Ben Pfaff Signed-off-by: Yifeng Sun Signed-off-by: Ben Pfaff --- v1->v2: Fix email subject by add

[ovs-dev] [PATCH] odp-util: Fix a use-afer-free bug

2018-10-05 Thread Yifeng Sun
After ofpbug_put, actions may have been reallocated and key will point to invalid memory address. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10796 Signed-off-by: Yifeng Sun --- lib/odp-util.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git

Re: [ovs-dev] [PATCH] expr: Set a limit on the depth of nested parentheses

2018-10-08 Thread Yifeng Sun
Thanks for the review! I will come up with a new version. Yifeng On Mon, Oct 8, 2018 at 11:02 AM Ben Pfaff wrote: > On Thu, Oct 04, 2018 at 04:30:10PM -0700, Yifeng Sun wrote: > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714 > > Signed-off

Re: [ovs-dev] [PATCH v3] expr: Set a limit on the depth of nested parentheses

2018-10-11 Thread Yifeng Sun
Thanks! On Thu, Oct 11, 2018 at 1:02 PM Ben Pfaff wrote: > On Wed, Oct 10, 2018 at 03:15:52PM -0700, Yifeng Sun wrote: > > This patch checks the depth of nested parentheses to prevent > > stack overflow. Since is_chassis_resident doesn't allow > > nested parentheses, its

Re: [ovs-dev] [PATCH 1/2] odp-util: Fix a bug that causes stack overflow

2018-10-11 Thread Yifeng Sun
In scan_set, '\0' is not always put in the buf. Ok, I think this patch is not the correct way to fix this issue, I will come up with a new version. Thanks, Yifeng On Thu, Oct 11, 2018 at 1:18 PM Ben Pfaff wrote: > On Tue, Oct 09, 2018 at 03:39:17PM -0700, Yifeng Sun wrote: > > ofpbu

Re: [ovs-dev] [PATCH] expr: Disallow < <= >= > comparisons against empty value set.

2018-10-11 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Thu, Oct 11, 2018 at 12:44 PM Ben Pfaff wrote: > OVN expression syntax does not allow a literal empty value set, like {}. > Rather, any literal value set has to have at least one value. However, > value sets that originate fro

Re: [ovs-dev] [PATCH 2/2] odp-util: Handle returning when parse_odp_key_mask_attr handles ufid

2018-10-11 Thread Yifeng Sun
Thanks for the review. How about returning a -EINVAL in this case? On Thu, Oct 11, 2018 at 1:39 PM Ben Pfaff wrote: > On Tue, Oct 09, 2018 at 03:39:18PM -0700, Yifeng Sun wrote: > > When parse_odp_key_mask_attr runs into ufid, it returns length of ufid > > without appending da

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Avoid deadlock on multicast snooping recursion.

2018-10-12 Thread Yifeng Sun
Looks good to me, thanks. Reviewed-by: Yifeng Sun On Mon, Aug 20, 2018 at 8:26 PM Ben Pfaff wrote: > Until now, OVS did multicast snooping outputs holding the read-lock on > the mcast_snooping object. This could recurse via a patch port to try to > take the write-lock on the sa

Re: [ovs-dev] [PATCH 2/2] odp-util: Handle returning when parse_odp_key_mask_attr handles ufid

2018-10-11 Thread Yifeng Sun
u, Oct 11, 2018 at 01:53:54PM -0700, Yifeng Sun wrote: > > Thanks for the review. > > > > How about returning a -EINVAL in this case? > > > > On Thu, Oct 11, 2018 at 1:39 PM Ben Pfaff wrote: > > > > > On Tue, Oct 09, 2018 at 03:39:18PM -0700, Yifeng Su

[ovs-dev] [PATCH] ofp-port: Free memory on error in ofp_print_ofpst_port_reply

2018-10-18 Thread Yifeng Sun
Counters in ops->custom_stats may already be valid at this error point. This patch frees the leaked memory. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10322 Signed-off-by: Yifeng Sun --- lib/ofp-port.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ofp-port.

[ovs-dev] [PATCH] odp-util: Initialize nsh_hdr in odp_nsh_hdr_from_attr

2018-10-18 Thread Yifeng Sun
://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10863 Signed-off-by: Yifeng Sun --- lib/odp-util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/odp-util.c b/lib/odp-util.c index d156fa7265de..1a52f3bc5ad9 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -2559,6 +2559,8

[ovs-dev] [PATCH v2] odp-util: Move ufid handling to odp_flow_from_string

2018-10-18 Thread Yifeng Sun
to understand ufid. This patch moves ufid parsing from parse_odp_key_mask_attr out to odp_flow_from_string. Reported-by: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10850 Signed-off-by: Yifeng Sun --- v1->v2: Improved according to Ben's review. lib/odp-util.c | 18 +++---

[ovs-dev] [PATCH] datapath: Use correct reply values in datapath and vport ops

2018-10-17 Thread Yifeng Sun
This patch fixes the bug that all datapath and vport ops are returning wrong values (OVS_FLOW_CMD_NEW or OVS_DP_CMD_NEW) in their replies. This commit backports upstream net-next's commit 804fe108fc92e59 ("openvswitch: Use correct reply values in datapath and vport ops"). Signed-off-

Re: [ovs-dev] [PATCH v3] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build

2018-10-31 Thread Yifeng Sun
30, 2018 at 11:50 PM Yifeng Sun > wrote: > >> I feel another option to fix this issue is to add a new function like >> dns_resolve_timeout__, what do you think? >> > > I think that can fix the problem. I am not too familiar with libunbound. > Does it support sp

Re: [ovs-dev] [PATCH v3] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build

2018-10-31 Thread Yifeng Sun
Hi Ben, The dns resolving depends on libunbound's ub_resolve, which, from Numan's experience as well as my reading on its documentation, doesn't support timeout. I agree there is a bug and we should fix it. Thanks, Yifeng On Wed, Oct 31, 2018 at 1:59 PM Ben Pfaff wrote: > On Thu, Oct 25, 2018

Re: [ovs-dev] [PATCH 2/6] rconn: New function rconn_is_reliable().

2018-10-31 Thread Yifeng Sun
Yes, I am reviewing 3. Please ask someone else to review 4+. Thanks. Yifeng On Wed, Oct 31, 2018 at 1:47 PM Ben Pfaff wrote: > On Wed, Oct 31, 2018 at 01:40:59PM -0700, Ben Pfaff wrote: > > Thanks for the reviews, I applied patches 1 and 2. > > > > Do you plan to review further patches? I

Re: [ovs-dev] [PATCH 3/6] connmgr: Improve interface for setting controllers.

2018-10-31 Thread Yifeng Sun
Thanks for the improvement. Tested-by: Yifeng Sun Reviewed-by: Yifeng Sun On Mon, Oct 29, 2018 at 3:59 PM Ben Pfaff wrote: > Using an shash instead of an array simplifies the code for both the caller > and the callee. Putting the set of allowed OpenFlow ve

Re: [ovs-dev] [PATCH v3] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build

2018-10-31 Thread Yifeng Sun
ers the first question. What about if no DNS servers are > available; what does the resolver do in that case? > > On Wed, Oct 31, 2018 at 03:24:13PM -0700, Yifeng Sun wrote: > > Hi Ben, > > > > The dns resolving depends on libunbound's ub_resolve, which, from > &

[ovs-dev] [PATCH] odp-util: Validate close-brace in scan_geneve and fix return values of san_xxx functions

2018-11-01 Thread Yifeng Sun
This patch adds validation of close-braces in scan_geneve. In addition, SCAN_TYPE expects scan_XXX functions to return 0 on errors. This patch inspects all related scan_XXX functions and fixes their return values. Signed-off-by: Yifeng Sun --- lib/odp-util.c | 20 +++- 1 file

Re: [ovs-dev] [PATCH v3] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build

2018-10-30 Thread Yifeng Sun
I feel another option to fix this issue is to add a new function like dns_resolve_timeout__, what do you think? Thanks, Yifeng On Thu, Oct 25, 2018 at 2:58 AM wrote: > From: Numan Siddique > > When 'make check' is called by the mock rpm build (which disables > networking), > the test

Re: [ovs-dev] [PATCH 1/6] connmgr: Modernize coding style.

2018-10-30 Thread Yifeng Sun
Looks good to me, thanks! Reviewed-by: Yifeng Sun On Mon, Oct 29, 2018 at 3:58 PM Ben Pfaff wrote: > This moves declarations closer to first use and merges them with > initialization when possible, moves "for" loop variable declarations into > the "for&qu

Re: [ovs-dev] [PATCH 2/6] rconn: New function rconn_is_reliable().

2018-10-30 Thread Yifeng Sun
Looks good, thanks! Reviewed-by: Yifeng Sun On Mon, Oct 29, 2018 at 3:58 PM Ben Pfaff wrote: > This will have its first user in an upcoming commit. > > Signed-off-by: Ben Pfaff > --- > include/openvswitch/rconn.h | 1 + > lib/rconn.c | 7 +++ >

Re: [ovs-dev] [PATCH] NSH: Fix NSH-related length macros that cause stack overflow

2018-10-26 Thread Yifeng Sun
. On Fri, Oct 26, 2018 at 3:05 PM Ben Pfaff wrote: > On Fri, Oct 26, 2018 at 02:55:55PM -0700, Ben Pfaff wrote: > > On Thu, Oct 25, 2018 at 02:41:50PM -0700, Yifeng Sun wrote: > > > In the filed of ver_flags_ttl_len of struct nshhdr, there are only 6 > > > bits that ar

<    1   2   3   4   5   6   >