[ovs-dev] [PATCH 2/3] dpif: Fix memory leak

it when done. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- 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_,

[ovs-dev] [PATCH 3/3] bfd: Fix memory leak

: 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 <pkusunyif...@gmail.com> --- ovn/controller/bfd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ovn/controller/bfd.c b/ovn/controller

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-xlate: Fix bug that may leak ofproto_flow_mod

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 <pkusunyif...@gmail.com&

Re: [ovs-dev] [PATCH] odp-execute: Add helpful comment to odp_execute_actions().

Thanks for the comments that clarify the usage of this function. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Nov 29, 2017 at 2:14 PM, Ben Pfaff <b...@ovn.org> wrote: > It wasn't obvious how ownership transferred to odp_execute_actions() or > to its callback.

[ovs-dev] [PATCH] netdev: netdev_get_etheraddr is not functioning as advertised.

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. Signed-off-by: Yifeng Sun

[ovs-dev] [PATCHv2] netdev: netdev_get_etheraddr is not functioning as advertised.

v1 found by Ben, thanks Ben. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- 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,

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

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 <pkusunyif...@gmail.com> --- configure.ac | 1 + lib/autom

Re: [ovs-dev] [PATCH 01/12] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()

Looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Mon, Dec 11, 2017 at 1:50 PM, Greg Rose <gvrose8...@gmail.com> wrote: > From: Christophe JAILLET <christophe.jail...@wanadoo.fr> > > Upstream commit: > commit 5829e62ac17a40ab08c1b905565604a

Re: [ovs-dev] [PATCH v3 1/8] ovsdb: Improve documentation.

ood to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 13, 2017 at 3:10 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > Documentation/automake.mk| 5 +- > Documentation/conf.py|

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

v1 -> v2: refactored and improved code based on reviewer's comments. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- configure.ac| 1 + lib/automake.mk | 6 ++ lib/dns-resolve.c | 244 lib/dns-

Re: [ovs-dev] [PATCH v3 8/8] ovsdb-tool: Add new "db-name" and "schema-name" commands.

Tested this patch and it works, thanks. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 13, 2017 at 3:10 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > NEWS

Re: [ovs-dev] [PATCH] ovsdb-server: Drop 'txn' member from struct db.

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

Re: [ovs-dev] [PATCH 2/3] ovsdb-error: New function ovsdb_error_to_json_free().

Thanks for the change. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff <b...@ovn.org> wrote: > This simplifies little bits of code here and there. > > Signed-off-

Re: [ovs-dev] [PATCH 1/3] ovsdb-error: New function ovsdb_error_to_string_free().

close_db(db); return error; } Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 13, 2017 at 10:04 AM, Ben Pfaff <b...@ovn.org> wrote: > This allows slight code simplifications across the tree. > > Signed-off-by

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

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 <b...@ovn.org> wrote: > On Mon, Dec 04, 2017 at 06:03:40AM -0800, Yifeng Sun wrote: > > This patch is a simple implementation for the proposal discuss

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

ed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- 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

Re: [ovs-dev] [PATCH] ovsdb: Fix memory leak in corner case in ovsdb_txn_commit_().

Thanks for the fix. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Dec 19, 2017 at 2:20 PM, Ben Pfaff <b...@ovn.org> wrote: > Reported-by: Yifeng Sun <pkusunyif...@gmail.com> > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > ovsdb/transaction.c

Re: [ovs-dev] [PATCH] ovsdb-server: Drop 'txn' member from struct db.

n Tue, Dec 19, 2017 at 2:03 PM, Ben Pfaff <b...@ovn.org> 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, 20

Re: [ovs-dev] [PATCH] datapath: Enforce matching of kernel releases on loading openvswitch.ko

happened. On Wed, Dec 13, 2017 at 10:24 AM, Aaron Conole <acon...@redhat.com> wrote: > Yifeng Sun <pkusunyif...@gmail.com> writes: > > > Deployment and upgrade failure is quite often caused by that > openvswitch.ko was > > built upon kernel x.y.z-release-A

Re: [ovs-dev] [PATCH] datapath: Enforce matching of kernel releases on loading openvswitch.ko

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 <acon...@redhat.com> wrote: > Y

Re: [ovs-dev] [PATCH] datapath: Enforce matching of kernel releases on loading openvswitch.ko

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 <g...@ovn.org> wrote: > > > On 12 December 2017 at 07:59, Yifeng Sun <pkusunyif...@gmail.com> wrote: > >> Deployment and upgrade

[ovs-dev] [PATCH 1/2] execution: Fix bug that leaks ovsdb_row

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 <pkusunyif...@gmail.com> --- ovsdb/execution.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ovsdb/execution.c b

[ovs-dev] [PATCH 2/2] ofproto-dpif-xlate: Fix bug that may leak ofproto_flow_mod

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 <pkusunyif...@gmail.com> --- ofproto/ofproto-dpif-xlate.c | 2 ++ 1 file changed, 2 insertions(+) diff

[ovs-dev] [PATCH] netdev-dummy: Remove wrong netdev_close

) 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 <pkusunyif...@gmail.com> --- lib/

Re: [ovs-dev] [PATCHv3 1/3] ovsdb-idl: Fix memory leak

Thanks for all your advice. I will pay close attention next time. On Thu, Nov 2, 2017 at 2:14 PM, Ben Pfaff <b...@ovn.org> wrote: > On Tue, Oct 31, 2017 at 10:52:08AM -0700, Yifeng Sun wrote: > > Valgrind testcase 2339 (ovn -- ipam connectivity) reports the leak below: > &

[ovs-dev] [PATCH 1/2] valgrind: Add support to run kernel datapath testsuite under valgrind

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 <pkusunyif...@gmail.com> --- Documentat

[ovs-dev] [PATCH 2/2] bond: Fix bug that writes to freed memory

ge.c:2998) by 0x407244: main (ovs-vswitchd.c:119) Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- 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

Re: [ovs-dev] DNS support feature (was: Re: DNS support options)

org> 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 this

[ovs-dev] [PATCH 1/3] ovsdb-idl: Fix memory leak

Reported by `make check-valgrind`. This patch was tested by `make check` and `make check-valgrind`. 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 5617e08d633c..be29c92957c0

[ovs-dev] [PATCH 2/3] test-ovsdb: Fix memory leak

Reported by `make check-valgrind`. This patch was tested by `make check` and `make check-valgrind`. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- tests/test-ovsdb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 451172

[ovs-dev] [PATCHv2 2/2] test-ovsdb: Fix memory leak (Amend comments)

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

[ovs-dev] [PATCHv2 1/2] ovsdb-idl: Fix memory leak (Amend comments)

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

[ovs-dev] [PATCHv3 3/3] ovsdb-server.c: Fix memory leak

->role need to be freed explicitly. v1->v3: Amend valgrind report. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- 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

[ovs-dev] [PATCHv3 1/3] ovsdb-idl: Fix memory leak

) by 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 <pkusunyif...@gmail.com> --- lib/ovsdb-idl.c | 1 + 1 file changed, 1 insertion(+) diff

Re: [ovs-dev] [PATCH 2/2] bond: Fix bug that writes to freed memory

Thanks, Yifeng On Wed, Dec 20, 2017 at 9:39 AM, Ben Pfaff <b...@ovn.org> 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.

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

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 <b...@ovn.org> wrote: > On Tue, Dec 19, 2017 at 05:08:42AM -0800, Yifeng Sun wrote: > > This patch is a simple implementation for the pro

Re: [ovs-dev] [PATCH] ovn/utilities: add info and warn level log functions

Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Sun, Apr 29, 2018 at 2:02 PM, Ben Pfaff <b...@ovn.org> wrote: > From: Paul Greenberg <green...@outlook.com> > > There is no INFO or WARN level log function. It is either ERROR or > FATA

Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: fix snprintf call

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

Re: [ovs-dev] [PATCH 1/2] lldp: fix string warnings

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

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

l 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 attempting to fix t

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

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

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

://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 insertions(+), 3

Re: [ovs-dev] [PATCH 3/3] datapath: Do not fail to load on gre protocol conflict

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. > &

Re: [ovs-dev] [PATCH 3/3] datapath: Do not fail to load on gre protocol conflict

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

Re: [ovs-dev] [PATCH 3/3] datapath: Do not fail to load on gre protocol conflict

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 relat

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

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 reviewe

[ovs-dev] [PATCH] [PATCHv2] ofp-meter: Fix ds_put_format that treats enum type as short integer

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" ", flags); S

Re: [ovs-dev] [PATCH] ofp-meter: Fix ds_put_format that treats enum type as short integer

: > 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,-Wformat] > >

[ovs-dev] [PATCH] ofp-meter: Fix ds_put_format that treats enum type as short integer

num 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.c +++ b/lib/ofp-mete

Re: [ovs-dev] DNS support feature (was: Re: DNS support options)

d, Oct 25, 2017 at 4:16 PM Yifeng Sun <pkusunyif...@gmail.com> 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 threa

Re: [ovs-dev] DNS support feature (was: Re: DNS support options)

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 well as negative caching.

Re: [ovs-dev] DNS support feature (was: Re: DNS support options)

g a third party library 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 <pkusunyif...@gmail.com>

Re: [ovs-dev] [PATCH] netdev-dummy: Lock mutex when retrieving custom stats.

Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Jan 10, 2018 at 3:47 PM, Ben Pfaff <b...@ovn.org> wrote: > Found by Clang. > > CC: Michal Weglicki <michalx.wegli...@intel.com> > Fixes: 971f4b394c6e ("netdev: Custom statisti

Re: [ovs-dev] [PATCH 01/15] log: Add async commit support.

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 <pkusunyif...@gmail.com> On Sun, Dec 31, 2017 at 9:16 PM, Ben Pfaff <b...@ovn.org> wrote: > The OVSDB log code has always had the ability to commit

Re: [ovs-dev] [PATCH 12/15] ovsdb-client: Add --timeout option.

Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Sun, Dec 31, 2017 at 9:16 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > NEWS| 1 + > ovsdb/ovsdb-client.1.in | 6 ++ &

Re: [ovs-dev] [PATCH 14/15] json: Make it safe to pass null pointers to json_equal().

Looks good to me, thanks for the patch. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Sun, Dec 31, 2017 at 9:16 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > lib/json.c | 6 +++--- > 1 file changed, 3 insertions(+),

Re: [ovs-dev] [PATCH 2/3] ofp-util: New data structure for mapping between table names and numbers.

It looks like that the not-null-validation of 'map' can be removed in static function ofputil_name_map_destroy but should be added for ofputil_port_map_destroy and ofputil_table_map_destroy. Other than that, this patch looks good to me. Thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.

Re: [ovs-dev] [PATCH 4/4] ovs-ofctl: Add "compose-packet" command for testing flow_compose().

I can't find the patch that enables flow_compose to accept 4 parameters. Can you please check that? thanks. Yifeng On Thu, Jan 18, 2018 at 2:45 PM, Ben Pfaff wrote: > I don't feel obligated to add a bunch of automatic tests for > flow_compose(), but this is handy for manual

Re: [ovs-dev] [PATCH 1/4] ofproto-dpif-trace: Generalize syntax for ofproto/trace.

Thanks for the patch, looks good to me. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Thu, Jan 18, 2018 at 2:45 PM, Ben Pfaff <b...@ovn.org> wrote: > ofproto/trace takes a bunch of options that have weird placement and &

Re: [ovs-dev] [PATCH 2/4] flow: Simplify flow_compose_l4().

Thanks for the refactor that makes code much cleaner. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Thu, Jan 18, 2018 at 2:45 PM, Ben Pfaff <b...@ovn.org> wrote: > Each of the cases in flow_compose_l4() separately tracked the number of > bytes of L4 data added to the pa

Re: [ovs-dev] [PATCH 4/4] ovs-ofctl: Add "compose-packet" command for testing flow_compose().

Sorry, I found it. Somehow I missed that patch. On Wed, Jan 24, 2018 at 11:07 AM, Yifeng Sun <pkusunyif...@gmail.com> wrote: > I can't find the patch that enables flow_compose to accept 4 parameters. > Can you please check that? thanks. > > Yifeng > > On Thu, Jan 18, 20

Re: [ovs-dev] [PATCH 3/4] flow: Add some L7 payload data to most L4 protocols that accept it.

Hi Ben, I found an issue in the lines below. It looks like that the 'else if' part is redundant. +if (dp_packet_size(packet) < packet_size) { +packet_expand(packet, , packet_size); +} else if (dp_packet_size(packet) < packet_size){ +

[ovs-dev] [PATCH 3/4] ovsdb-client: Fix memory leaks

) by 0x40845A: do_restore (ovsdb-client.c:1592) by 0x405BCD: main (ovsdb-client.c:170) Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- ovsdb/ovsdb-client.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ovsdb/ovsdb-clie

[ovs-dev] [PATCH 4/4] vlog: Fix memory leak

: ovs_cmdl_run_command__ (command-line.c:115) by 0x406C09: main (ovstest.c:133) Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- lib/vlog.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/vlog.c b/lib/vlog.c index 6e87665..a5c0c5e 100644 --- a/lib/vlog.c +++ b/lib/

[ovs-dev] [PATCH 1/4] pinctrl: Fix memory leak

) Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- 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 +

[ovs-dev] [PATCH 2/4] ovn-northd: Fix memory leak

) 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 <pkusunyif...@gmail.com> --- ovn/northd/ovn-no

Re: [ovs-dev] [PATCH 01/15] log: Add async commit support.

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 <pkusunyif...@gmail.com> On Wed, Jan 10, 2018 at 4:21 PM, Ben Pfaff <b...@ovn.or

Re: [ovs-dev] [PATCH v2 2/2] ovs-ofctl: Add "compose-packet" command for testing flow_compose().

cpdump - -r-" to Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Jan 26, 2018 at 3:03 PM, Ben Pfaff <b...@ovn.org> wrote: > I don't feel obligated to add a bunch of automatic tests for > flow_compose(), but this is h

Re: [ovs-dev] [PATCH v2 1/2] flow: Add some L7 payload data to most L4 protocols that accept it.

Thanks for the patch. Tested and looks good to me. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Jan 26, 2018 at 3:03 PM, Ben Pfaff <b...@ovn.org> wrote: > This makes traffic generated by flow_compose() look slig

[ovs-dev] [PATCH] packet-type-aware.at: Fix check failure

The test (ptap - recirculate after packet_type change) failed because function format_odp_key_attr__ outputs src, dst and proto in the case of OVS_KEY_ATTR_IPV4. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- tests/packet-type-aware.at | 2 +- 1 file changed, 1 insertion(+), 1 de

[ovs-dev] [PATCH] classifier: refactor classifier_remove and introduce classifier_remove_assert

The return type of classifier_remove is changed to bool. Besides, classifier_remove_assert is introduced to assert that the classifier must contain the rule. This patch is based on Ben's advice. Signed-off-by: Yifeng Sun <pkusunyif...@gmail.com> --- lib/classifier.c

Re: [ovs-dev] [PATCH v2 1/3] ofp-actions: Make formatting and parsing functions take a struct argument.

Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff <b...@ovn.org> wrote: > An upcoming commit will add another parameter for parsing and formatting > actions. It is much easier to ad

Re: [ovs-dev] [PATCH] classifier: Refactor interface for classifier_remove().

Thanks for the patch, which is better than the one I submitted. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Jan 30, 2018 at 1:00 PM, Ben Pfaff <b...@ovn.org> wrote: > Until now, classifier_remove()

Re: [ovs-dev] [PATCH] classifier: Fix typo in comment.

Thanks for the fix. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Jan 30, 2018 at 12:34 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > lib/classifier.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) >

Re: [ovs-dev] [PATCH v2 2/3] ofp-util: New data structure for mapping between table names and numbers.

Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff <b...@ovn.org> wrote: > This shares the infrastructure for mapping port names and numbers. It will > be used in an upcoming commit. > > Signed-off-by:

Re: [ovs-dev] [PATCH] util: Use lookup table to optimize hexit_value().

Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Feb 2, 2018 at 3:16 PM, Ben Pfaff <b...@ovn.org> wrote: > Daniel Alvarez Sanchez reported a significant overall speedup in ovn-northd > due to a similar patch. > > Reported-by: Danie

Re: [ovs-dev] [PATCH v2 2/3] ofp-util: New data structure for mapping between table names and numbers.

Sure, I just reviewed the 3rd patch. On Wed, Jan 31, 2018 at 11:36 AM, Ben Pfaff <b...@ovn.org> wrote: > Thanks for the reviews of patches 1 and 2. Do you plan to review patch > 3 as well? I rebased and sent out v3 just now. > > On Tue, Jan 30, 2018 at 01:29:56PM -0800

Re: [ovs-dev] [PATCH v3 3/3] Support accepting and displaying table names in OVS tools.

Thanks for the nice feature. It looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Jan 31, 2018 at 11:36 AM, Ben Pfaff <b...@ovn.org> wrote: > OpenFlow has little-known support for naming tables. Open vSwitch has > supported table names for ages, but

Re: [ovs-dev] [PATCH] tests: Make OVS_WAIT_UNTIL and OVS_WAIT_WHILE failures easier to debug.

Thanks for the change. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Thu, Feb 1, 2018 at 10:08 AM, Ben Pfaff <b...@ovn.org> wrote: > Until now, when OVS_WAIT_UNTIL or OVS_WAIT_WHILE ran, little information > was av

Re: [ovs-dev] [PATCH] util: Document and rely on ovs_assert() always evaluating its argument.

Thanks, looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Jan 31, 2018 at 11:23 AM, Ben Pfaff <b...@ovn.org> wrote: > The ovs_assert() macro always evaluates its argument, even when NDEBUG is > defined so that failure is ignored. This behavior

Re: [ovs-dev] [PATCH 2/4] ovs-vsctl: Use default socket name in tests.

Thanks. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Feb 6, 2018 at 10:15 AM, Ben Pfaff <b...@ovn.org> wrote: > Thanks for pointing that out. The following incremental patch fixes the > problem. I will fo

Re: [ovs-dev] [PATCH 2/4] ovs-vsctl: Use default socket name in tests.

It seems test "ovs-xapi-sync" is broken after this patch. I haven't figured out myself. Can you please take a look? Thanks. On Fri, Feb 2, 2018 at 1:51 PM, Ben Pfaff wrote: > By using the default socket name "db.sock", instead of "socket", we can > avoid passing --db=unix:socket

Re: [ovs-dev] [PATCH 4/4] ovs-vsctl: Add commands "add-bond-iface" and "del-bond-iface".

Looks good to me, thanks. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Feb 2, 2018 at 1:51 PM, Ben Pfaff <b...@ovn.org> wrote: > It was not too hard to build these commands using the database commands, > but

Re: [ovs-dev] [PATCH 3/4] NEWS: Consolidate ovs-vswitchd sections and fix indentation.

Looks good to me. Thanks. Tested-by: Yifeng Sun <pkusunyif...@gmail.com> Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Feb 2, 2018 at 1:51 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > NEWS | 11 +-

Re: [ovs-dev] [PATCH 1/2] ofp-util: Remove prototypes for unimplemented functions.

Thanks for the clean up. Looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Fri, Feb 9, 2018 at 2:57 PM, Ben Pfaff <b...@ovn.org> wrote: > Signed-off-by: Ben Pfaff <b...@ovn.org> > --- > include/openvswitch/ofp-util.h | 12 >

Re: [ovs-dev] [PATCH] ofp-flow: Fix return value for ofputil_decode_flow_stats_reply().

Looks good to me. Thanks for the correction. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Tue, Feb 13, 2018 at 10:49 AM, Ben Pfaff <b...@ovn.org> wrote: > This function returned errno values for some errors and OFPERR_* values > for others. The callers all expect

Re: [ovs-dev] [PATCH] ofp-meter: Fix use-after-free for decoding meter mods.

ofputil_pull_bands may change bands->data. Thanks for the fix. Reviewed-by: Yifeng Sun<pkusunyif...@gmail.com> On Wed, Feb 14, 2018 at 2:36 PM, Ben Pfaff <b...@ovn.org> wrote: > Found by libfuzzer-ngram. > > Reported-by: Bhargava Shastry <bshas...@sect.tu-berlin.de&

Re: [ovs-dev] [PATCH 1/3] tunnel: Avoid flow_to_string() call when rate-limited.

Looks good, thanks for the change. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Thu, Dec 28, 2017 at 12:34 PM, Ben Pfaff <b...@ovn.org> wrote: > flow_to_string() is relatively expensive. It is better to avoid it if the > string is not actually going to be used. >

Re: [ovs-dev] [PATCH 2/3] tunnel: Log sanely in tnl_port_receive().

Looks good, thanks. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Thu, Dec 28, 2017 at 12:34 PM, Ben Pfaff <b...@ovn.org> wrote: > When this function was introduced in 2012, it modified its 'flow' argument > and logged the changes (at debug level). However, since 2013

Re: [ovs-dev] [PATCH] ofproto-dpif: Fix a couple minor issues in comments.

Thanks for the change. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 20, 2017 at 7:42 PM, Justin Pettit <jpet...@ovn.org> wrote: > Signed-off-by: Justin Pettit <jpet...@ovn.org> > --- > ofproto/ofproto-dpif-upcall.h | 2 +- > ofproto/ofproto-d

Re: [ovs-dev] [PATCH] xcache: Handle null argument for xlate_cache_uninit().

Thanks, looks good to me. Reviewed-by: Yifeng Sun <pkusunyif...@gmail.com> On Wed, Dec 20, 2017 at 7:42 PM, Justin Pettit <jpet...@ovn.org> wrote: > Most other OVS libraries' delete and uninitialization functions allow a > null argument, but this one would cause a segfau

Re: [ovs-dev] [PATCH] compat: Allow IPv6 GRE/ERSPAN Tx when ip6_gre is loaded

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

[ovs-dev] [PATCH 2/8] system-common-macros.at: Skip tests for certain kernel versions

Some tests depend on upstream gre modules to setup testing environments. However, some kernel versions require compatable gre modules being used. This patch helps to skip tests that fail due to this reason. The new m4 functions will be used by later patches. Signed-off-by: Yifeng Sun --- tests

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

Skip gre, erspan and ip6erspan related tests on kernel version from 4.4.x to 4.15.x because 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/system-traffic.at index

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

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 --- datapath/linux/compat/ip6_gre.c | 1 - 1 file changed, 1 deletion(-) diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/com

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

Introduce a new test that doesn't setup native ip6erspan v2 tunnels but sends simulated raw packets. This test is supposed to only run for kernel version from 4.4.x to 4.15.x. Signed-off-by: Yifeng Sun --- tests/system-traffic.at | 47 +++ 1 file

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

Introduce a new test that doesn't setup native ip6erspan v1 tunnels but sends simulated raw packets. This test is supposed to only run for kernel version from 4.4.x to 4.15.x. Signed-off-by: Yifeng Sun --- tests/system-traffic.at | 51 + 1 file

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

Introduce a new test that doesn't setup native gre tunnels but sends simulated raw packets. This test is supposed to only run for kernel version from 4.4.x to 4.15.x. Signed-off-by: Yifeng Sun --- tests/system-traffic.at | 47 +++ 1 file changed, 47

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

Introduce a new test that doesn't setup native erspan v2 tunnels but sends simulated raw packets. This test is supposed to only run for kernel version from 4.4.x to 4.15.x. Signed-off-by: Yifeng Sun --- tests/system-traffic.at | 44 1 file changed

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

Introduce a new test that doesn't setup native erspan v1 tunnels but sends simulated raw packets. This test is supposed to only run for kernel version from 4.4.x to 4.15.x. Signed-off-by: Yifeng Sun --- tests/system-traffic.at | 48 1 file

  1   2   3   4   5   6   >