Looks good to me, thanks.
Reviewed-by: Yifeng Sun
On Wed, Jun 13, 2018 at 12:43 PM, Aaron Conole wrote:
> lib/netdev-dpdk.c: In function :
> lib/netdev-dpdk.c:2865:49: warning: output may be truncated before the
> last format character [-Wformat-truncation=]
> snprintf
Thanks for the improvement.
Reviewed-by: Yifeng Sun
On Wed, Jun 13, 2018 at 12:43 PM, Aaron Conole wrote:
> lib/lldp/lldpd.c: In function :
> lib/lldp/lldpd.c:520:17: warning: output truncated before terminating nul
> copying as many bytes from a string as its length [-Wstringop-t
Hi Greg,
I am debugging a GRE related issue. Do you mind telling me which kernel
version will lead to (err == -EEXIST) here?
else if (err == -EEXIST)
pr_warn("Cannot take GRE protocol entry - The ERSPAN
feature may not be supported\n");
Thanks,
Yifeng
On Tue, Jun 5, 2018 at
Hi Greg,
I tried 4.4 and 4.16 both with ip_gre module loaded and didn't trigger this
error.
I will try more, thanks!
Yifeng
On Fri, Jun 22, 2018 at 5:17 PM, Gregory Rose wrote:
>
>
> On 6/22/2018 4:40 PM, Yifeng Sun wrote:
>
> Hi Greg,
>
> I am debugging a GRE
Thanks a lot for the info, I will try them out.
Best,
Yifeng
On Mon, Jun 25, 2018 at 9:00 AM, Gregory Rose wrote:
>
> On 6/22/2018 5:36 PM, Yifeng Sun wrote:
>
> Hi Greg,
>
> I tried 4.4 and 4.16 both with ip_gre module loaded and didn't trigger
> this error.
&
xes it by treating enum type as int type.
(6.7.2.2 Enumerationspecifiers)
Signed-off-by: Yifeng Sun
---
lib/ofp-meter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/ofp-meter.c b/lib/ofp-meter.c
index e63daabaf5f9..0a01cba60be2 100644
--- a/lib/ofp-meter.
ed 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
---
v1 -> v2: Refactored and improved code based
On Tue, Jun 26, 2018 at 10:30:30AM -0700, Yifeng Sun wrote:
> > Travis job fails because of the below error:
> >
> > lib/ofp-meter.c:340:48: error: format specifies type 'unsigned short'
> > but the argument has underlying type 'unsigned int' [-Werror,-
Travis job fails because of the below error and this patch solves this issue.
lib/ofp-meter.c:340:48: error: format specifies type 'unsigned short'
but the argument has underlying type 'unsigned int' [-Werror,-Wformat]
ds_put_format(s, "flags:0x%"PRIx16"
Hi Shashank,
libunbound also supports windows. I think this is one of reasons we
selected libunbound.
Best,
Yifeng
On Tue, Jun 26, 2018 at 7:27 PM, Shashank Ram wrote:
>
>
> On Tue, Jun 26, 2018 at 7:08 PM Yifeng Sun wrote:
>
>> This patch is a simple implementatio
ons:
https://travis-ci.org/yifsun/ovs-travis/builds/397573497
Signed-off-by: Yifeng Sun
---
acinclude.m4 | 6 --
datapath/linux/compat/include/net/dst_metadata.h | 6 ++
datapath/linux/compat/include/net/erspan.h | 2 +-
3 files changed, 11 insert
ons:
https://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/
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
el doesn't affect tunnel functionality.
>>
>> This fix passed the travis for different kernel versions:
>> https://travis-ci.org/yifsun/ovs-travis/builds/397915843
>>
>> Signed-off-by: Yifeng Sun
>>
>
> Yifeng,
>
> Thanks for the patch and attemptin
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:
error 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
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
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: Yife
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 |
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
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(-
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;
ret
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
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 expecti
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
ncy of kernel's 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 ch
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(+
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(+)
>
ful run.
>
> ## -- ##
> ## openvswitch 2.9.90 test suite. ##
> ## -- ##
> 111: nsh - forward ok
>
> ## - ##
> ## Test results. ##
> ## - ##
>
> 1 test was su
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
> > ve
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
&
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 the test. I
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 dele
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
Thanks. The code looks much cleaner.
Reviewed-by: Yifeng Sun
On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
> ---
> lib/netdev-dpdk.c | 16
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/lib/netdev-d
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
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:
>
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_
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
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
'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-stri
(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 whi
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
does 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
> > Repor
}
>
> 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 n
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
I tested and it fixed the loading issue on kernel 4.14. Thanks for the
patch.
Tested-by: Yifeng Sun
Reviewed-by: Yifeng Sun
On Thu, Jul 26, 2018 at 9:11 AM, Greg Rose wrote:
> When for some reason the built-in kernel ip6_gre module is loaded that
> would prevent the openvswitch
wrote:
> Hello everyone, please allow me to introduce Yifeng Sun (CCed), who
> recently joined VMware's Open vSwitch team. I've asked Yifeng to start
> out by working on DNS support for Open vSwitch. Yifeng, can you tell us
> about what you've discovered so far, based on th
PM Yifeng Sun wrote:
>
>> I feel that unbound stands out in the available open source DNS resolver.
>>
>> Below is the summary for unbound:
>> * The actual resolving work is done by a background process or thread. A
>> background process or thread seems unavoidabl
#x27;s a good thing. One way to go with this is to go with
>> the
>> > extreme of always performing DNS lookups. We can then recommend that
>> users
>> > of OVS that will be performing frequent DNS lookups also run a DNS
>> > forwarder that has built-in caching (as
brary for DNS resolution,
> it may also be worth looking into what sort of caching those libraries
> provide. I am not familiar with that information off the top of my head.
>
> Mark
>
> On Fri, Oct 27, 2017 at 11:14 AM Yifeng Sun
> wrote:
>
>> Mark, thanks a lot for the d
Reported by `make check-valgrind`.
This patch was tested by `make check` and `make check-valgrind`.
Signed-off-by: Yifeng Sun
---
lib/ovsdb-idl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 5617e08d633c..be29c92957c0 100644
--- a/lib/ovsdb-idl.c
Reported by `make check-valgrind`.
This patch was tested by `make check` and `make check-valgrind`.
Signed-off-by: Yifeng Sun
---
tests/test-ovsdb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 451172cdcc34..b5147fc055e3 100644
--- a/tests
When options are freed, options->role need to be freed explicitly.
Signed-off-by: Yifeng Sun
---
ovsdb/ovsdb-server.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 030d86ba467f..cd30dd48425b 100644
--
v1 -> v2:
When ovsdb_idl_table is freed, its indexes are not freed.
Valgrind report is as below:
45 (32 direct, 13 indirect) bytes in 1 blocks are definitely lost in loss
record 65 of 83
at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4A6D64: xmalloc
v1 -> v2:
range_end_atom is allocated in ovsdb_atom_from_string__() and no one is holding
a reference to it at the end of do_parse_atom_strings(). It should be freed, as
also pointed out by ovsdb_atom_destroy().
Valgrind report is as below:
16 bytes in 1 blocks are definitely lost in loss reco
0x406C98: main (ovn-controller.c:619)
The leak happens when vsdb_idl_table is freed but its indexes are not freed.
v1->v2: Amend comments.
v2->v3: Fix error in patch.
Signed-off-by: Yifeng Sun
---
lib/ovsdb-idl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ovsdb-idl.c b/lib
->role need to be freed explicitly.
v1->v3: Amend valgrind report.
Signed-off-by: Yifeng Sun
---
ovsdb/ovsdb-server.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 030d86ba467f..cd30dd48425b 100644
)
range_end_atom is allocated in ovsdb_atom_from_string__() and no one is holding
a reference to it at the end of do_parse_atom_strings(). It should be freed
here, as also pointed out by ovsdb_atom_destroy().
v1->v2: Amend comments.
v2->v3: Fix error in patch.
Signed-off-by: Yife
.c:1837)
by 0x40A985: bridge_add_ports__ (bridge.c:931)
by 0x40C7EA: bridge_add_ports (bridge.c:947)
by 0x40C7EA: bridge_reconfigure (bridge.c:663)
by 0x410485: bridge_run (bridge.c:2998)
by 0x406F64: main (ovs-vswitchd.c:119)
Signed-off-by: Yifeng Sun
---
lib/netdev-dummy.
Thanks for all your advice. I will pay close attention next time.
On Thu, Nov 2, 2017 at 2:14 PM, Ben Pfaff wrote:
> On Tue, Oct 31, 2017 at 10:52:08AM -0700, Yifeng Sun wrote:
> > Valgrind testcase 2339 (ovn -- ipam connectivity) reports the leak below:
> > 45 (32 direct, 13 ind
'.
Signed-off-by: Yifeng Sun
---
lib/dpif-netdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index db7831874fed..19c81583a892 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2925,6 +2925,9 @@ dpif_netdev_execute(struct dpi
batch and release
it when done.
Signed-off-by: Yifeng Sun
---
lib/dpif.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/dpif.c b/lib/dpif.c
index 3f0742d382ed..310dec14655e 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1279,6 +1279,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_pac
: bfd_run (bfd.c:257)
by 0x407F72: main (ovn-controller.c:718)
gateway_chassis wasn't released before the 'continue' line.
Signed-off-by: Yifeng Sun
---
ovn/controller/bfd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
ind
If there is an error after ovsdb_rbac_insert, 'row' is leaked.
So move the existing ovsdb_row_destroy to the function end.
Signed-off-by: Yifeng Sun
---
ovsdb/execution.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ovsdb/execution.c b/ovsdb/executi
When ofm is not referenced by xc_entry, we should release its
resources by calling ofproto_flow_mod_uninit because no one is
going to use it in this function.
Signed-off-by: Yifeng Sun
---
ofproto/ofproto-dpif-xlate.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ofproto/ofproto-dpif
20, 2017 at 04:26:39AM -0800, Yifeng Sun wrote:
> > When ofm is not referenced by xc_entry, we should release its
> > resources by calling ofproto_flow_mod_uninit because no one is
> > going to use it in this function.
> >
> > Signed-off-by: Yifeng Sun
> > ---
Thanks for the comments that clarify the usage of this function.
Reviewed-by: Yifeng Sun
On Wed, Nov 29, 2017 at 2:14 PM, Ben Pfaff wrote:
> It wasn't obvious how ownership transferred to odp_execute_actions() or
> to its callback.
>
> CC: Yifeng Sun
> Signed-off-by: Ben
netdev_get_etheraddr claims to clear 'mac' on error, but it fails to do so.
When looking further into both netdev_windows_get_etheraddr() and
netdev_linux_get_etheraddr(), 'mac' is also not cleared. This will lead to
usage of uninitialised ofputil_phy_port.hw_addr.
Signe
; v2: fixed a bug in v1 found by Ben, thanks Ben.
Signed-off-by: Yifeng Sun
---
lib/netdev.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/netdev.c b/lib/netdev.c
index c52680659e3f..2d69fe5dac68 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -897,7 +897,13 @@ netd
esses are returned, only the first one is used;
/etc/nsswitch.conf isn't respected as unbound library doesn't look at
it;
Depends on caller to retry later to use the resolved results.
Signed-off-by: Yifeng Sun
---
configure.ac | 1 +
lib/automake.mk | 6 ++
li
With this patch, kernel datapath testsuite can be run under valgrind by using
the "check-kernel-valgrind" target and the results can be found under directory
"tests/system-kmod-testsuite.dir/".
Signed-off-by: Yifeng Sun
---
Documentation/topics/testing.rst | 4 +++
ridge_run (bridge.c:2998)
by 0x407244: main (ovs-vswitchd.c:119)
Signed-off-by: Yifeng Sun
---
ofproto/bond.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 8ecd22c7d5d3..6f3d7b5b3817 100644
--- a/ofproto/bond.c
+++ b/ofproto
Looks good to me.
Reviewed-by: Yifeng Sun
On Mon, Dec 11, 2017 at 1:50 PM, Greg Rose wrote:
> From: Christophe JAILLET
>
> Upstream commit:
> commit 5829e62ac17a40ab08c1b905565604a4b5fa7af6
> Author: Christophe JAILLET
> Date: Mon Sep 11 21:56:20 2017 +0200
>
>
.
Signed-off-by: Yifeng Sun
---
datapath/datapath.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 178081993b42..0e3a1db63847 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -51,6 +51,8 @@
#include
#include
r);
close_db(db);
return error;
}
Tested-by: Yifeng Sun
Reviewed-by: Yifeng Sun
On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff wrote:
> This allows slight code simplifications across the tree.
>
> Signed-off-by: Ben Pfaff
> ---
> lib
Thanks for the change.
Tested-by: Yifeng Sun
Reviewed-by: Yifeng Sun
On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff wrote:
> This simplifies little bits of code here and there.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovsdb-error.c | 17 +
> lib/ovsdb
Looks good to me.
Reviewed-by: Yifeng Sun
On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff wrote:
> This will have its first user in an upcoming commit.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovsdb-parser.c | 14 +-
> lib/ovsdb-parser.h | 3 ++-
> 2 files c
Thanks a lot Ben for your review. I will apply your advice in v2.
Yifeng
On Wed, Dec 13, 2017 at 11:02 AM, Ben Pfaff wrote:
> On Mon, Dec 04, 2017 at 06:03:40AM -0800, Yifeng Sun wrote:
> > This patch is a simple implementation for the proposal discussed in
> > https://mail.
514, some error happened.
On Wed, Dec 13, 2017 at 10:24 AM, Aaron Conole wrote:
> Yifeng Sun writes:
>
> > Deployment and upgrade failure is quite often caused by that
> openvswitch.ko was
> > built upon kernel x.y.z-release-A while it is loaded into a running
> ke
a new kernel release, its kernel version may not
change, like the case that I mentioned in the previous email. In this case,
openvswitch.ko is up to date with the kernel version but may not work with
the new release.
On Wed, Dec 13, 2017 at 2:00 PM, Aaron Conole wrote:
> Yifeng Sun writes:
>
Hi Guru,
Thanks for clarifying it. In this case, this patch is not necessary.
Best,
Yifeng
On Wed, Dec 13, 2017 at 2:24 PM, Guru Shetty wrote:
>
>
> On 12 December 2017 at 07:59, Yifeng Sun wrote:
>
>> Deployment and upgrade failure is quite often caused by that
>
v1 -> v2: refactored and improved code based on reviewer's comments.
Signed-off-by: Yifeng Sun
---
configure.ac| 1 +
lib/automake.mk | 6 ++
lib/dns-resolve.c | 244
lib/dns-resolve.h | 26 ++
li
to me.
Reviewed-by: Yifeng Sun
On Wed, Dec 13, 2017 at 3:10 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
> ---
> Documentation/automake.mk| 5 +-
> Documentation/conf.py| 6 +
> Documentation/ref/index.rst | 3 +
> Docu
Tested this patch and it works, thanks.
Tested-by: Yifeng Sun
Reviewed-by: Yifeng Sun
On Wed, Dec 13, 2017 at 3:10 PM, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff
> ---
> NEWS | 1 +
> ovsdb/ovsdb-tool.1.in | 4
> ovsdb/ovsdb
It seems that txn is not freed when ovsdb_txn_commit returns an error.
Other than that, this patch looks good to me.
Thanks,
Yifeng
On Fri, Dec 15, 2017 at 11:01 AM, Ben Pfaff wrote:
> This member was only used in one particular code path, so this commit
> adds code to pass it around as a funct
it message.
Signed-off-by: Yifeng Sun
---
configure.ac| 1 +
lib/automake.mk | 6 ++
lib/dns-resolve.c | 244
lib/dns-resolve.h | 26 ++
lib/socket-util.c | 56 +--
m4/openvswitch.m4 | 10 +
}
On Tue, Dec 19, 2017 at 2:03 PM, Ben Pfaff wrote:
> Thank you for the review.
>
> ovsdb_txn_commit() is at least meant to free its argument whether it
> succeeds or not. Do you see a bug there?
>
> I applied this to master.
>
> On Tue, Dec 19, 2017 at 09:50:45AM -0
Thanks for the fix.
Reviewed-by: Yifeng Sun
On Tue, Dec 19, 2017 at 2:20 PM, Ben Pfaff wrote:
> Reported-by: Yifeng Sun
> Signed-off-by: Ben Pfaff
> ---
> ovsdb/transaction.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/ovsdb/transaction.c b/ovsdb/
Thanks,
Yifeng
On Wed, Dec 20, 2017 at 9:39 AM, Ben Pfaff wrote:
> On Mon, Dec 11, 2017 at 05:44:07AM -0800, Yifeng Sun wrote:
> > pr_op->pr_rule is pointing to memory in bond->hash. It shouldn't be
> written
> > if bond->hash is already freed.
> >
>
Hi Ben,
Thank you very much for the code review, I will fix and come up with v4
soon.
Best,
Yifeng
On Wed, Dec 20, 2017 at 9:32 AM, Ben Pfaff wrote:
> On Tue, Dec 19, 2017 at 05:08:42AM -0800, Yifeng Sun wrote:
> > This patch is a simple implementation for the proposal discussed in
Thanks for the change.
Reviewed-by: Yifeng Sun
On Wed, Dec 20, 2017 at 7:42 PM, Justin Pettit wrote:
> Signed-off-by: Justin Pettit
> ---
> ofproto/ofproto-dpif-upcall.h | 2 +-
> ofproto/ofproto-dpif-xlate.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
&
Thanks, looks good to me.
Reviewed-by: Yifeng Sun
On Wed, Dec 20, 2017 at 7:42 PM, Justin Pettit wrote:
> Most other OVS libraries' delete and uninitialization functions allow a
> null argument, but this one would cause a segfault.
>
> Signed-off-by: Justin Pettit
> --
Looks good, thanks for the change.
Reviewed-by: Yifeng Sun
On Thu, Dec 28, 2017 at 12:34 PM, Ben Pfaff wrote:
> flow_to_string() is relatively expensive. It is better to avoid it if the
> string is not actually going to be used.
>
> Signed-off-by: Ben Pfaff
> ---
> of
Looks good, thanks.
Reviewed-by: Yifeng Sun
On Thu, Dec 28, 2017 at 12:34 PM, Ben Pfaff wrote:
> When this function was introduced in 2012, it modified its 'flow' argument
> and logged the changes (at debug level). However, since 2013 it has no
> longer modified its '
Looks good to me, thanks.
Reviewed-by: Yifeng Sun
On Wed, Jan 10, 2018 at 3:47 PM, Ben Pfaff wrote:
> Found by Clang.
>
> CC: Michal Weglicki
> Fixes: 971f4b394c6e ("netdev: Custom statistics.")
> Signed-off-by: Ben Pfaff
> ---
> Somehow I didn't notic
I read through lib/seq.c to learn how this patch works.
Looks good to me, but I feel not very confident.
Reviewed-by: Yifeng Sun
On Sun, Dec 31, 2017 at 9:16 PM, Ben Pfaff wrote:
> The OVSDB log code has always had the ability to commit the log to disk and
> wait for the commit to
After further study, I understand the "seq" code and think it is
well-designed.
Thank you for this patch and the previous reply.
This patch looks good to me.
Reviewed-by: Yifeng Sun
On Wed, Jan 10, 2018 at 4:21 PM, Ben Pfaff wrote:
> Thanks for the review.
>
> The "s
)
by 0x4148B4: build_ports (ovn-northd.c:2109)
by 0x4148B4: ovnnb_db_run.isra.37 (ovn-northd.c:6202)
by 0x406FE0: main (ovn-northd.c:6854)
Signed-off-by: Yifeng Sun
---
ovn/northd/ovn-northd.c | 1 +
1 file changed, 1
)
Signed-off-by: Yifeng Sun
---
ovn/controller/pinctrl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 7542db3..d8bfde0 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -1371,6 +1371,7 @@ send_ipv6_ras(const struct
1 - 100 of 529 matches
Mail list logo
