Re: [ovs-dev] [PATCH net-next v4 08/10] net: openvswitch: fix possible memleak on destroy flow-table
On Sun, Oct 20, 2019 at 10:02 PM Tonghao Zhang wrote:
>
> On Sat, Oct 19, 2019 at 2:12 AM Pravin Shelar wrote:
> >
> > On Thu, Oct 17, 2019 at 8:16 PM Tonghao Zhang
> > wrote:
> > >
> > > On Fri, Oct 18, 2019 at 6:38 AM Pravin Shelar wrote:
> > > >
> > > > On Wed, Oct 16, 2019 at 5:50 AM wrote:
> > > > >
> > > > > From: Tonghao Zhang
> > > > >
> > > > > When we destroy the flow tables which may contain the flow_mask,
> > > > > so release the flow mask struct.
> > > > >
> > > > > Signed-off-by: Tonghao Zhang
> > > > > Tested-by: Greg Rose
> > > > > ---
> > > > > net/openvswitch/flow_table.c | 14 +-
> > > > > 1 file changed, 13 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/openvswitch/flow_table.c
> > > > > b/net/openvswitch/flow_table.c
> > > > > index 5df5182..d5d768e 100644
> > > > > --- a/net/openvswitch/flow_table.c
> > > > > +++ b/net/openvswitch/flow_table.c
> > > > > @@ -295,6 +295,18 @@ static void table_instance_destroy(struct
> > > > > table_instance *ti,
> > > > > }
> > > > > }
> > > > >
> > > > > +static void tbl_mask_array_destroy(struct flow_table *tbl)
> > > > > +{
> > > > > + struct mask_array *ma = ovsl_dereference(tbl->mask_array);
> > > > > + int i;
> > > > > +
> > > > > + /* Free the flow-mask and kfree_rcu the NULL is allowed. */
> > > > > + for (i = 0; i < ma->max; i++)
> > > > > + kfree_rcu(rcu_dereference_raw(ma->masks[i]), rcu);
> > > > > +
> > > > > + kfree_rcu(rcu_dereference_raw(tbl->mask_array), rcu);
> > > > > +}
> > > > > +
> > > > > /* No need for locking this function is called from RCU callback or
> > > > > * error path.
> > > > > */
> > > > > @@ -304,7 +316,7 @@ void ovs_flow_tbl_destroy(struct flow_table
> > > > > *table)
> > > > > struct table_instance *ufid_ti =
> > > > > rcu_dereference_raw(table->ufid_ti);
> > > > >
> > > > > free_percpu(table->mask_cache);
> > > > > - kfree_rcu(rcu_dereference_raw(table->mask_array), rcu);
> > > > > + tbl_mask_array_destroy(table);
> > > > > table_instance_destroy(ti, ufid_ti, false);
> > > > > }
> > > >
> > > > This should not be required. mask is linked to a flow and gets
> > > > released when flow is removed.
> > > > Does the memory leak occur when OVS module is abruptly unloaded and
> > > > userspace does not cleanup flow table?
> > > When we destroy the ovs datapath or net namespace is destroyed , the
> > > mask memory will be happened. The call tree:
> > > ovs_exit_net/ovs_dp_cmd_del
> > > -->__dp_destroy
> > > -->destroy_dp_rcu
> > > -->ovs_flow_tbl_destroy
> > > -->table_instance_destroy (which don't release the mask memory because
> > > don't call the ovs_flow_tbl_remove /flow_mask_remove directly or
> > > indirectly).
> > >
> > Thats what I suggested earlier, we need to call function similar to
> > ovs_flow_tbl_remove(), we could refactor code to use the code.
> > This is better since by introducing tbl_mask_array_destroy() is
> > creating a dangling pointer to mask in sw-flow object. OVS is anyway
> > iterating entire flow table to release sw-flow in
> > table_instance_destroy(), it is natural to release mask at that point
> > after releasing corresponding sw-flow.
> I got it, thanks. I rewrite the codes, can you help me to review it.
> If fine, I will sent it next version.
> >
> >
Sure, I can review it, Can you send the patch inlined in mail?
Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2] tests: Fix check-valgrind and check-lcov.
On Tue, Oct 22, 2019 at 7:08 AM Han Zhou wrote: > After split from OVS, make check-valgrind and check-lcov are not > working any more, because the $ovs_srcdir are missing for these tests. > Instead of add ovs_srcdir=$(ovs_srcdir) for each target, this patch > export ovs_srcdir once for all. > > Signed-off-by: Han Zhou > Acked-by: Numan Siddique Numan > --- > tests/automake.mk | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/automake.mk b/tests/automake.mk > index 013e592..47e6a5d 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -54,8 +54,10 @@ DISTCLEANFILES += tests/atconfig tests/atlocal > > AUTOTEST_PATH = > $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller > > +export ovs_srcdir > + > check-local: > - set $(SHELL) '$(TESTSUITE)' -C tests > AUTOTEST_PATH=$(AUTOTEST_PATH) ovs_srcdir=$(ovs_srcdir); \ > + set $(SHELL) '$(TESTSUITE)' -C tests > AUTOTEST_PATH=$(AUTOTEST_PATH); \ > "$$@" $(TESTSUITEFLAGS) || (test X'$(RECHECK)' = Xyes && "$$@" > --recheck) > > # Python Coverage support. > -- > 2.1.0 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-controller.c: Fix memory leak of local_datapath->ports.
On Tue, Oct 22, 2019 at 5:19 AM Han Zhou wrote:
> Fixes: 89f5048f960c ("ovn-controller: Minimize SB DB port_binding
> lookups.")
> Signed-off-by: Han Zhou
>
Acked-by: Numan Siddique
Numan
> ---
> controller/ovn-controller.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index b46a1d1..9ab98be 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -972,6 +972,7 @@ en_runtime_data_cleanup(struct engine_node *node)
> HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> &data->local_datapaths) {
> free(cur_node->peer_ports);
> +free(cur_node->ports);
> hmap_remove(&data->local_datapaths, &cur_node->hmap_node);
> free(cur_node);
> }
> @@ -1002,6 +1003,7 @@ en_runtime_data_run(struct engine_node *node)
> struct local_datapath *cur_node, *next_node;
> HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
> local_datapaths) {
> free(cur_node->peer_ports);
> +free(cur_node->ports);
> hmap_remove(local_datapaths, &cur_node->hmap_node);
> free(cur_node);
> }
> --
> 2.1.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rhel: Remove the cond 'build_python3'
On Mon, Oct 21, 2019 at 10:51 PM Ben Pfaff wrote: > On Mon, Oct 21, 2019 at 03:12:42PM +0530, num...@ovn.org wrote: > > From: Numan Siddique > > > > A previous patch removed python2 support from ovs. So we can remove > > this condition and make python3 mandatory for builds. Without this > > patch, make rpm-fedora on centos 7 fails unless we pass > > RPMBUILD_OPT="--with build_python3". > > > > Signed-off-by: Numan Siddique > > Thanks. I read this and applied this to master. I did not actually > test it. > Thanks for applying the patch. Numan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] lflow.c: Fix memory leak of lflow_ref_list_node->ref_name.
On Tue, Oct 22, 2019 at 6:57 AM Han Zhou wrote:
> The ref_name is copied in lflow_resource_add(), but forgot to free in
> lflow_resource_destroy_lflow(). It can be fixed by freeing it in
> lflow_resource_destroy_lflow(). However, this field is never really
> used, so just delete it from lflow_ref_list_node, together with the
> "type" field.
>
> Fixes: d2aa2c7cafead ("ovn-controller: Maintain resource references for
> logical flows.")
> Signed-off-by: Han Zhou
>
Acked-by: Numan Siddique
> ---
> controller/lflow.c | 2 --
> controller/lflow.h | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index e3ed20c..24e3a52 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -230,8 +230,6 @@ lflow_resource_add(struct lflow_resource_ref *lfrr,
> enum ref_type type,
> }
>
> struct lflow_ref_list_node *lrln = xzalloc(sizeof *lrln);
> -lrln->type = type;
> -lrln->ref_name = xstrdup(ref_name);
> lrln->lflow_uuid = *lflow_uuid;
> ovs_list_push_back(&rlfn->ref_lflow_head, &lrln->ref_list);
> ovs_list_push_back(&lfrn->lflow_ref_head, &lrln->lflow_list);
> diff --git a/controller/lflow.h b/controller/lflow.h
> index 0572668..abdc55e 100644
> --- a/controller/lflow.h
> +++ b/controller/lflow.h
> @@ -80,8 +80,6 @@ enum ref_type {
> struct lflow_ref_list_node {
> struct ovs_list lflow_list; /* list for same lflow */
> struct ovs_list ref_list; /* list for same ref */
> -enum ref_type type;
> -char *ref_name;
> struct uuid lflow_uuid;
> };
>
> --
> 2.1.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] lacp: report desync in ovs threads enabling slave
Bleep bloop. Greetings Gowrishankar Muthukrishnan, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 84 characters long (recommended limit is 79) #25 FILE: ofproto/bond.c:821: VLOG_DBG_RL(&rl, "bond %s: slave %s: main thread not yet enabled slave", Lines checked: 33, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] lacp: report desync in ovs threads enabling slave
It is helpful in reporting main thread that is yet to enable bond slave,
but link state was brought up by lacp thread and capture this desync
between ovs threads for debugging.
Fixes: a8448cb170 ("lacp: Avoid packet drop on LACP bond after link up")
Signed-off-by: Gowrishankar Muthukrishnan
---
ofproto/bond.c | 4
1 file changed, 4 insertions(+)
diff --git a/ofproto/bond.c b/ofproto/bond.c
index c5d5f2c03..3b148a244 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -817,6 +817,10 @@ bond_check_admissibility(struct bond *bond, const void
*slave_,
* When may_enable is TRUE, it means LACP is UP and waiting for the
* main thread to run LACP state machine and enable the slave. */
verdict = (slave->enabled || slave->may_enable) ? BV_ACCEPT : BV_DROP;
+if (!slave->enabled && slave->may_enable) {
+VLOG_DBG_RL(&rl, "bond %s: slave %s: main thread not yet enabled
slave",
+ bond->name, bond->active_slave->name);
+}
goto out;
case LACP_CONFIGURED:
if (!bond->lacp_fallback_ab) {
--
2.17.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] tests: Fix check-valgrind and check-lcov.
On Mon, Oct 21, 2019 at 4:29 PM Ben Pfaff wrote: > > On Mon, Oct 21, 2019 at 04:17:30PM -0700, Han Zhou wrote: > > After split from OVS, make check-valgrind and check-lcov are not > > working any more, because the $ovs_srcdir are missing for these tests. > > > > Signed-off-by: Han Zhou > > Seems reasonable. > > Another possibility could be to add "export ovs_srcdir" to automake.mk > or Makefile.am somewhere. Thank Ben. That's a better idea. I just sent v2: https://patchwork.ozlabs.org/patch/1180961/ ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2] tests: Fix check-valgrind and check-lcov.
After split from OVS, make check-valgrind and check-lcov are not working any more, because the $ovs_srcdir are missing for these tests. Instead of add ovs_srcdir=$(ovs_srcdir) for each target, this patch export ovs_srcdir once for all. Signed-off-by: Han Zhou --- tests/automake.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/automake.mk b/tests/automake.mk index 013e592..47e6a5d 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -54,8 +54,10 @@ DISTCLEANFILES += tests/atconfig tests/atlocal AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller +export ovs_srcdir + check-local: - set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) ovs_srcdir=$(ovs_srcdir); \ + set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH); \ "$$@" $(TESTSUITEFLAGS) || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) # Python Coverage support. -- 2.1.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] lflow.c: Fix memory leak of lflow_ref_list_node->ref_name.
The ref_name is copied in lflow_resource_add(), but forgot to free in
lflow_resource_destroy_lflow(). It can be fixed by freeing it in
lflow_resource_destroy_lflow(). However, this field is never really
used, so just delete it from lflow_ref_list_node, together with the
"type" field.
Fixes: d2aa2c7cafead ("ovn-controller: Maintain resource references for logical
flows.")
Signed-off-by: Han Zhou
---
controller/lflow.c | 2 --
controller/lflow.h | 2 --
2 files changed, 4 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index e3ed20c..24e3a52 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -230,8 +230,6 @@ lflow_resource_add(struct lflow_resource_ref *lfrr, enum
ref_type type,
}
struct lflow_ref_list_node *lrln = xzalloc(sizeof *lrln);
-lrln->type = type;
-lrln->ref_name = xstrdup(ref_name);
lrln->lflow_uuid = *lflow_uuid;
ovs_list_push_back(&rlfn->ref_lflow_head, &lrln->ref_list);
ovs_list_push_back(&lfrn->lflow_ref_head, &lrln->lflow_list);
diff --git a/controller/lflow.h b/controller/lflow.h
index 0572668..abdc55e 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -80,8 +80,6 @@ enum ref_type {
struct lflow_ref_list_node {
struct ovs_list lflow_list; /* list for same lflow */
struct ovs_list ref_list; /* list for same ref */
-enum ref_type type;
-char *ref_name;
struct uuid lflow_uuid;
};
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 00/10] optimize openvswitch flow looking up
On Tue, Oct 22, 2019 at 1:14 AM William Tu wrote: > > On Wed, Oct 16, 2019 at 5:50 AM wrote: > > > > From: Tonghao Zhang > > > > This series patch optimize openvswitch for performance or simplify > > codes. > > > > Patch 1, 2, 4: Port Pravin B Shelar patches to > > linux upstream with little changes. > > btw, should we keep Pravin as the author of the above three patches? Agree， but how i can to that, these patches should be sent by Pravin ? > Regards, > William > > > > > Patch 5, 6, 7: Optimize the flow looking up and > > simplify the flow hash. > > > > Patch 8, 9: are bugfix. > > > > The performance test is on Intel Xeon E5-2630 v4. > > The test topology is show as below: > > > > +---+ > > | +---+ | > > | | eth0 ovs-switcheth1 | | Host0 > > | +---+ | > > +---+ > > ^ | > > | | > > | | > > | | > > | v > > +-++ ++-+ > > | netperf | Host1 | netserver| Host2 > > +--+ +--+ > > > > We use netperf send the 64B packets, and insert 255+ flow-mask: > > $ ovs-dpctl add-flow ovs-switch > > "in_port(1),eth(dst=00:01:00:00:00:00/ff:ff:ff:ff:ff:01),eth_type(0x0800),ipv4(frag=no)" > > 2 > > ... > > $ ovs-dpctl add-flow ovs-switch > > "in_port(1),eth(dst=00:ff:00:00:00:00/ff:ff:ff:ff:ff:ff),eth_type(0x0800),ipv4(frag=no)" > > 2 > > $ > > $ netperf -t UDP_STREAM -H 2.2.2.200 -l 40 -- -m 18 > > > > * Without series patch, throughput 8.28Mbps > > * With series patch, throughput 46.05Mbps > > > > v3->v4: > > access ma->count with READ_ONCE/WRITE_ONCE API. More information, > > see patch 5 comments. > > > > v2->v3: > > update ma point when realloc mask_array in patch 5. > > > > v1->v2: > > use kfree_rcu instead of call_rcu > > > > Tonghao Zhang (10): > > net: openvswitch: add flow-mask cache for performance > > net: openvswitch: convert mask list in mask array > > net: openvswitch: shrink the mask array if necessary > > net: openvswitch: optimize flow mask cache hash collision > > net: openvswitch: optimize flow-mask looking up > > net: openvswitch: simplify the flow_hash > > net: openvswitch: add likely in flow_lookup > > net: openvswitch: fix possible memleak on destroy flow-table > > net: openvswitch: don't unlock mutex when changing the user_features > > fails > > net: openvswitch: simplify the ovs_dp_cmd_new > > > > net/openvswitch/datapath.c | 65 + > > net/openvswitch/flow.h | 1 - > > net/openvswitch/flow_table.c | 316 > > +-- > > net/openvswitch/flow_table.h | 19 ++- > > 4 files changed, 329 insertions(+), 72 deletions(-) > > > > -- > > 1.8.3.1 > > > > ___ > > dev mailing list > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] ovn-controller.c: Fix memory leak of local_datapath->ports.
Fixes: 89f5048f960c ("ovn-controller: Minimize SB DB port_binding lookups.")
Signed-off-by: Han Zhou
---
controller/ovn-controller.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index b46a1d1..9ab98be 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -972,6 +972,7 @@ en_runtime_data_cleanup(struct engine_node *node)
HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
&data->local_datapaths) {
free(cur_node->peer_ports);
+free(cur_node->ports);
hmap_remove(&data->local_datapaths, &cur_node->hmap_node);
free(cur_node);
}
@@ -1002,6 +1003,7 @@ en_runtime_data_run(struct engine_node *node)
struct local_datapath *cur_node, *next_node;
HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, local_datapaths) {
free(cur_node->peer_ports);
+free(cur_node->ports);
hmap_remove(local_datapaths, &cur_node->hmap_node);
free(cur_node);
}
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] faq: Give specific versions that introduced various features.
Some users would find it useful to know the particular OVS version that introduced a feature to the OVS tree kernel module or to the OVS userspace (DPDK) datapath implementation. This patch updates the FAQ to include that information. This information is primarily gleaned from the top-level NEWS file. For most of these, I did not verify them by looking carefully through the history, so some of them may be inaccurate. Requested-by: Jianjun Shen Signed-off-by: Ben Pfaff --- Documentation/faq/releases.rst | 55 ++ 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index ec6f90916064..87f84a85709c 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -103,35 +103,40 @@ Q: Are all features available with all datapaths? Hyper-V Also known as the Windows datapath. -The following table lists the datapath supported features from an Open -vSwitch user's perspective. +The following table lists the datapath supported features from an +Open vSwitch user's perspective. The "Linux upstream" column +lists the Linux kernel version that introduced a given feature +into its kernel module. The "Linux OVS tree" and "Userspace" +columns list the Open vSwitch release versions that introduced a +given feature into the included kernel module or the userspace +(DPDK) datapath, respectively. == == == = === FeatureLinux upstream Linux OVS tree Userspace Hyper-V == == == = === -Connection tracking 4.3YES YES YES -Conntrack Fragment Reass. 4.3YES YES YES -Conntrack Timeout Policies 5.2YES NO NO -Conntrack Zone Limit4.18 YES NO YES -NAT 4.6YES YES YES -Tunnel - LISP NO YES NO NO -Tunnel - STTNO YES NO YES -Tunnel - GRE3.11 YES YES YES -Tunnel - VXLAN 3.12 YES YES YES -Tunnel - Geneve 3.18 YES YES YES -Tunnel - GRE-IPv6 4.18 YES YES NO -Tunnel - VXLAN-IPv6 4.3YES YES NO -Tunnel - Geneve-IPv64.4YES YES NO -Tunnel - ERSPAN 4.18 YES YES NO -Tunnel - ERSPAN-IPv64.18 YES YES NO -QoS - Policing YESYES YES NO -QoS - Shaping YESYES NO NO -sFlow YESYES YES NO -IPFIX 3.10 YES YES YES -Set action YESYES YESPARTIAL -NIC Bonding YESYES YES YES -Multiple VTEPs YESYES YES YES -Meters 4.15 YES YES NO +Connection tracking 4.32.5 2.6 YES +Conntrack Fragment Reass. 4.32.6 2.8 YES +Conntrack Timeout Policies 5.22.12 NO NO +Conntrack Zone Limit4.18 2.10 NO YES +NAT 4.62.6 2.8 YES +Tunnel - LISP NO 2.11 NO NO +Tunnel - STTNO 2.4 NO YES +Tunnel - GRE3.11 1.0 2.4 YES +Tunnel - VXLAN 3.12 1.10 2.4 YES +Tunnel - Geneve 3.18 2.4 2.4 YES +Tunnel - GRE-IPv6 4.18 2.6 2.6 NO +Tunnel - VXLAN-IPv6 4.32.6 2.6 NO +Tunnel - Geneve-IPv64.42.6 2.6 NO +Tunnel - ERSPAN 4.18 2.10 2.10 NO +Tunnel - ERSPAN-IPv64.18 2.10 2.10 NO +QoS - Policing YES1.1 2.6 NO +QoS - Shaping YES1.1 NO NO +sFlow YES1.0 2.7 NO +IPFIX 3.10 1.11 2.7 YES +Set action
Re: [ovs-dev] [PATCH ovn] tests: Fix check-valgrind and check-lcov.
On Mon, Oct 21, 2019 at 04:17:30PM -0700, Han Zhou wrote: > After split from OVS, make check-valgrind and check-lcov are not > working any more, because the $ovs_srcdir are missing for these tests. > > Signed-off-by: Han Zhou Seems reasonable. Another possibility could be to add "export ovs_srcdir" to automake.mk or Makefile.am somewhere. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] tests: Fix check-valgrind and check-lcov.
After split from OVS, make check-valgrind and check-lcov are not working any more, because the $ovs_srcdir are missing for these tests. Signed-off-by: Han Zhou --- tests/automake.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/automake.mk b/tests/automake.mk index 013e592..c0f05c9 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -83,7 +83,7 @@ LCOV_OPTS = -b $(abs_top_builddir) -d $(abs_top_builddir) -q -c --rc lcov_branch GENHTML_OPTS = -q --branch-coverage --num-spaces 4 check-lcov: all $(check_DATA) clean-lcov find . -name '*.gcda' | xargs -n1 rm -f - -set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH); \ + -set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) ovs_srcdir=$(ovs_srcdir); \ "$$@" $(TESTSUITEFLAGS) || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) $(MKDIR_P) tests/lcov lcov $(LCOV_OPTS) -o tests/lcov/coverage.info @@ -127,7 +127,7 @@ HELGRIND = valgrind --log-file=helgrind.%p --tool=helgrind \ --suppressions=$(abs_top_srcdir)/tests/openssl.supp --num-callers=20 EXTRA_DIST += tests/glibc.supp tests/openssl.supp check-valgrind: all $(valgrind_wrappers) $(check_DATA) - $(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true VALGRIND='$(VALGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) + $(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true VALGRIND='$(VALGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' ovs_srcdir=$(ovs_srcdir) -d $(TESTSUITEFLAGS) @echo @echo '--' @echo 'Valgrind output can be found in tests/testsuite.dir/*/valgrind.*' -- 2.1.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] travis: Test build with afxdp.
On 21.10.2019 21:28, Aaron Conole wrote: Ilya Maximets writes: We can't easily update the kernel on TravisCI to run system tests with AF_XDP, but we could run build tests with libbpf and headers from newer kernels. Signed-off-by: Ilya Maximets --- https://travis-ci.org/ovsrobot/ovs/jobs/600899950 Nice. Looks like the afxdp related objects are getting compiled. Acked-by: Aaron Conole Thanks, Aaron and Ben! Applied to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net] net: openvswitch: free vport unless register_netdevice() succeeds
On 10/21/2019 3:01 AM, Stefano Brivio wrote: From: Hillf Danton syzbot found the following crash on: HEAD commit:1e78030e Merge tag 'mmc-v5.3-rc1' of git://git.kernel.org/.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=148d3d1a60 kernel config: https://syzkaller.appspot.com/x/.config?x=30cef20daf3e9977 dashboard link: https://syzkaller.appspot.com/bug?extid=13210896153522fe1ee5 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=136aa8c460 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=109ba79260 = BUG: memory leak unreferenced object 0x8881207e4100 (size 128): comm "syz-executor032", pid 7014, jiffies 4294944027 (age 13.830s) hex dump (first 32 bytes): 00 70 16 18 81 88 ff ff 80 af 8c 22 81 88 ff ff .p." 00 b6 23 17 81 88 ff ff 00 00 00 00 00 00 00 00 ..#. backtrace: [<0eb78212>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline] [<0eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline] [<0eb78212>] slab_alloc mm/slab.c:3319 [inline] [<0eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548 [<006ea6c6>] kmalloc include/linux/slab.h:552 [inline] [<006ea6c6>] kzalloc include/linux/slab.h:748 [inline] [<006ea6c6>] ovs_vport_alloc+0x37/0xf0 net/openvswitch/vport.c:130 [] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164 [<56ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199 [<5434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194 [ ] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614 [ ] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629 [ ] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654 [<6694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477 [<88381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665 [ ] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline] [ ] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328 [<67e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917 [ ] sock_sendmsg_nosec net/socket.c:637 [inline] [ ] sock_sendmsg+0x54/0x70 net/socket.c:657 [<4cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311 [ ] __sys_sendmsg+0x80/0xf0 net/socket.c:2356 [ ] __do_sys_sendmsg net/socket.c:2365 [inline] [ ] __se_sys_sendmsg net/socket.c:2363 [inline] [ ] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363 BUG: memory leak unreferenced object 0x88811723b600 (size 64): comm "syz-executor032", pid 7014, jiffies 4294944027 (age 13.830s) hex dump (first 32 bytes): 01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 05 35 82 c1 .5.. backtrace: [<352f46d8>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline] [<352f46d8>] slab_post_alloc_hook mm/slab.h:522 [inline] [<352f46d8>] slab_alloc mm/slab.c:3319 [inline] [<352f46d8>] __do_kmalloc mm/slab.c:3653 [inline] [<352f46d8>] __kmalloc+0x169/0x300 mm/slab.c:3664 [<8e48f3d1>] kmalloc include/linux/slab.h:557 [inline] [<8e48f3d1>] ovs_vport_set_upcall_portids+0x54/0xd0 net/openvswitch/vport.c:343 [<541e4f4a>] ovs_vport_alloc+0x7f/0xf0 net/openvswitch/vport.c:139 [ ] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164 [<56ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199 [<5434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194 [ ] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614 [ ] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629 [ ] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654 [<6694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477 [<88381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665 [ ] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline] [ ] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328 [<67e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917 [ ] sock_sendmsg_nosec net/socket.c:637 [inline] [ ] sock_sendmsg+0x54/0x70 net/socket.c:6
Re: [ovs-dev] [RFC ovn PATCH 0/5] Separate pinctrl to its own process
Hi Mark, Thanks for the patch. We had a brief discussion during last OVN meeting. Let me put my points inlined. On Fri, Oct 18, 2019 at 1:43 PM Mark Michelson wrote: > > This proposes a set of patches to move pinctrl operations out of the > ovn-controller process and into its own. > > The main reasons for doing this are the following: > 1) Separating pinctrl makes it so that receiving a packet-in can't wake > up ovn-controller. To avoid waking up ovn-controller, it doesn't have to be in a separate process. A thread with its own OVSDB IDL to SB DB can achieve the same, as what this old patch did: https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332887.html However, the problem of a separate SB connection introduced the concern for scalability. There were discussions and thoughts for a separate thread without introducing new SB connection, but once the two threads share same SB connection, there has to be some synchronization between the threads that ends up waking up or blocking each other whenever there is a pinctrl processing that requires read/write SB data. The current multi-thread implementation from Numan is a trade off that avoids new SB connection but syncing with the main thread when SB data is needed. It is perfect for pinctrl handling that doesn't require SB data, and then wakes up ovn-controller for updating SB data. Today (2.12) there were improvements on both ovn-controller and OVSDB server, that alleviated the scale problems on both side. - For ovn-controller, with incremental processing, when there is no input change, it doesn't trigger flow recomputing, even when pinctrl wakes up the main thread. The major concern may be when main thread does need a recompute, it could block pinctrl processing for messages that requires SB data accessing, such as ARP handling. - For SB DB - Active-active cluster alleviates the burden of a single server and spread to 3 or 5. However, RAFT is not designed for scale. Write always happen on the leader node, and the cost of cluster sync between leader and follower becomes higher when number of nodes increases. - The fast-resync feature (requiring active-active clustered mode) avoids the slowness of data resync to all clients after DB restart/failover. However, it doesn't help if ovsdb-server is overloaded for regular updates and notifications during normal operations, given that it is single threaded. Also, there are corner cases that fast-resync doesn't help, e.g. when DB restart/failover happened just after a compress, when all the transaction history is lost. I'd suggest to reconsider these scalability concerns, the pros and cons for a dedicated SB connection for pinctrl, before moving forward to this approach. > 2) Separating pinctrl allows for manipulating the southbound database > directly while handling packets in, thus minimizing the need for storing > local copies of data This is true, but similar as point 1), it doesn't necessarily need a separate process. The point is whether pinctrl (thread or process) should use a dedicated SB connection. > 3) This lays the groundwork for an easier eventual conversion of > ovn-controller to DDlog, since the DDlog code would need to only handle > flow creation, not packet in handling. > Agree with this point. This is probably the most important benefit of separating pinctrl as a process. Although it is still possible to have pinctrl as a thread sharing SB connection while converting the flow processing part with DDlog, a separate process does make the conversion cleaner. In addition, a separate process introduces some operational costs, although not a big concern. The tooling like ovn-ctl and packaging also needs to be updated. Thanks, Han ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] travis: Test build with afxdp.
Ilya Maximets writes:
> We can't easily update the kernel on TravisCI to run system tests
> with AF_XDP, but we could run build tests with libbpf and headers
> from newer kernels.
>
> Signed-off-by: Ilya Maximets
> ---
https://travis-ci.org/ovsrobot/ovs/jobs/600899950
Nice. Looks like the afxdp related objects are getting compiled.
Acked-by: Aaron Conole
> .travis.yml| 1 +
> .travis/linux-build.sh | 34 --
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index eabbad92d..5676d9748 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -39,6 +39,7 @@ env:
>- TESTSUITE=1 LIBS=-ljemalloc
>- KERNEL_LIST="5.0 4.20 4.19 4.18 4.17 4.16"
>- KERNEL_LIST="4.15 4.14 4.9 4.4 3.19 3.16"
> + - AFXDP=1 KERNEL=5.3
>- M32=1 OPTS="--disable-ssl"
>- DPDK=1 OPTS="--enable-shared"
>- DPDK_SHARED=1
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 758c8235c..ff4b8fa39 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -38,7 +38,7 @@ function install_kernel()
> wget ${url} || wget ${url} || wget ${url/cdn/www}
>
> tar xvf linux-${version}.tar.xz > /dev/null
> -cd linux-${version}
> +pushd linux-${version}
> make allmodconfig
>
> # Cannot use CONFIG_KCOV: -fsanitize-coverage=trace-pc is not supported
> by compiler
> @@ -60,9 +60,26 @@ function install_kernel()
> make net/bridge/
> fi
>
> -EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
> -echo "Installed kernel source in $(pwd)"
> -cd ..
> +if [ "$AFXDP" ]; then
> +sudo make headers_install INSTALL_HDR_PATH=/usr
> +pushd tools/lib/bpf/
> +# Bulding with gcc because there are some issues in make files
> +# that breaks building libbpf with clang on Travis.
> +CC=gcc sudo make install
> +CC=gcc sudo make install_headers
> +sudo ldconfig
> +popd
> +# The Linux kernel defines __always_inline in stddef.h (283d7573),
> and
> +# sys/cdefs.h tries to re-define it. Older libc-dev package in
> xenial
> +# doesn't have a fix for this issue. Applying it manually.
> +sudo sed -i '/^# define __always_inline .*/i # undef
> __always_inline' \
> +/usr/include/x86_64-linux-gnu/sys/cdefs.h || true
> +EXTRA_OPTS="${EXTRA_OPTS} --enable-afxdp"
> +else
> +EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
> +echo "Installed kernel source in $(pwd)"
> +fi
> +popd
> }
>
> function install_dpdk()
> @@ -127,8 +144,9 @@ function build_ovs()
> configure_ovs $OPTS
> make selinux-policy
>
> -# Only build datapath if we are testing kernel w/o running testsuite
> -if [ "${KERNEL}" ]; then
> +# Only build datapath if we are testing kernel w/o running testsuite and
> +# AF_XDP support.
> +if [ "${KERNEL}" ] && ! [ "$AFXDP" ]; then
> pushd datapath
> make -j4
> popd
> @@ -161,6 +179,10 @@ elif [ "$M32" ]; then
> export CC="$CC -m32"
> else
> OPTS="--enable-sparse"
> +if [ "$AFXDP" ]; then
> +# netdev-afxdp uses memset for 64M for umem initialization.
> +SPARSE_FLAGS="${SPARSE_FLAGS} -Wno-memcpy-max-count"
> +fi
> CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${SPARSE_FLAGS}"
> fi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Deterrent: Security update needed
Security update information. This link is valid for ONE USE ONLY and EXPIRES IN 24 HOURS Dear Member, Please for your safety, click security update below to upgrade your GoDaddy account to our new system. Security update If you don't respond to our instructions in less than 24 hours, your GoDaddy account will be terminated permanently. Thanks, The GoDaddy account team. Please do not reply to this email. Emails sent to this address will not be answered.. Copyright © 1999-2019 GoDaddy Operating Company, LLC. 14455 N. Hayden Rd, Ste. 219, Scottsdale, AZ 85260 USA. All rights reserved. Disclaimer The information contained in this communication from the sender is confidential. It is intended solely for use by the recipient and others authorized to receive it. If you are not the recipient, you are hereby notified that any disclosure, copying, distribution or taking action in relation of the contents of this information is strictly prohibited and may be unlawful. This email has been scanned for viruses and malware, and may have been automatically archived by Mimecast Ltd, an innovator in Software as a Service (SaaS) for business. Providing a safer and more useful place for your human generated data. Specializing in; Security, archiving and compliance. To find out more visit the Mimecast website. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] travis: Test build with afxdp.
On Mon, Oct 21, 2019 at 08:11:14PM +0200, Ilya Maximets wrote: > We can't easily update the kernel on TravisCI to run system tests > with AF_XDP, but we could run build tests with libbpf and headers > from newer kernels. > > Signed-off-by: Ilya Maximets I read this and it seemed reasonable, but I didn't apply it or test it, so take this ack with a grain of salt. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC ovn PATCH 0/5] Separate pinctrl to its own process
I realized that after my latest rebase, there are three tests that are
failing with this changeset:
IGMP snoop/querier/relay
ARP lookup before learning
vtep 3HVs, 1 VIFs/HV, 1 GW, 1 LS
They don't fail in master, so I know they're the fault of the branch.
With that in mind, I will fix these failures and post a v2 of this RFC.
Don't let that deter you from having a look at v1 though, since you
still can get a feel for what the change is proposing.
On 10/18/19 4:42 PM, Mark Michelson wrote:
This proposes a set of patches to move pinctrl operations out of the
ovn-controller process and into its own.
The main reasons for doing this are the following:
1) Separating pinctrl makes it so that receiving a packet-in can't wake
up ovn-controller.
2) Separating pinctrl allows for manipulating the southbound database
directly while handling packets in, thus minimizing the need for storing
local copies of data
3) This lays the groundwork for an easier eventual conversion of
ovn-controller to DDlog, since the DDlog code would need to only handle
flow creation, not packet in handling.
This is an RFC. With this set of changes, item (2) above is not well
addressed here. While the multithreading is removed from pinctrl, the
structural components have not been altered. Were this idea to be
approved, point (2) would be addressed when creating the final patch.
Please share your thoughts.
Mark Michelson (5):
Separate pinctrl to its own process.
Resolve duplicate functions in ovn-controller and ovn-pinctrl.
Remove multithreading from pinctrl.
Move ovn-pinctrl to its own directory.
Flesh out manpage with more details about ovn-pinctrl
Makefile.am | 1 +
controller/automake.mk| 3 +-
controller/binding.c | 22 +-
controller/binding.h | 3 +-
controller/controller-utils.c | 220 +++
controller/ovn-controller.c | 233 +---
controller/ovn-controller.h | 20 +
pinctrl/automake.mk | 25 ++
pinctrl/ovn-pinctrl.8.xml | 110 ++
pinctrl/ovn-pinctrl.c | 413 +
{controller => pinctrl}/pinctrl.c | 748 ++
{controller => pinctrl}/pinctrl.h | 0
tests/automake.mk | 2 +-
tests/ofproto-macros.at | 3 +
tests/ovn.at | 13 +-
tutorial/ovs-sandbox | 5 +
utilities/ovn-ctl | 40 ++
17 files changed, 1064 insertions(+), 797 deletions(-)
create mode 100644 controller/controller-utils.c
create mode 100644 pinctrl/automake.mk
create mode 100644 pinctrl/ovn-pinctrl.8.xml
create mode 100644 pinctrl/ovn-pinctrl.c
rename {controller => pinctrl}/pinctrl.c (84%)
rename {controller => pinctrl}/pinctrl.h (100%)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Add DNSSL support to OVN
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: Failed to merge in the changes. Patch failed at 0001 Add DNSSL support to OVN The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/OVN/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] travis: Test build with afxdp.
We can't easily update the kernel on TravisCI to run system tests
with AF_XDP, but we could run build tests with libbpf and headers
from newer kernels.
Signed-off-by: Ilya Maximets
---
.travis.yml| 1 +
.travis/linux-build.sh | 34 --
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index eabbad92d..5676d9748 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -39,6 +39,7 @@ env:
- TESTSUITE=1 LIBS=-ljemalloc
- KERNEL_LIST="5.0 4.20 4.19 4.18 4.17 4.16"
- KERNEL_LIST="4.15 4.14 4.9 4.4 3.19 3.16"
+ - AFXDP=1 KERNEL=5.3
- M32=1 OPTS="--disable-ssl"
- DPDK=1 OPTS="--enable-shared"
- DPDK_SHARED=1
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 758c8235c..ff4b8fa39 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -38,7 +38,7 @@ function install_kernel()
wget ${url} || wget ${url} || wget ${url/cdn/www}
tar xvf linux-${version}.tar.xz > /dev/null
-cd linux-${version}
+pushd linux-${version}
make allmodconfig
# Cannot use CONFIG_KCOV: -fsanitize-coverage=trace-pc is not supported by
compiler
@@ -60,9 +60,26 @@ function install_kernel()
make net/bridge/
fi
-EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
-echo "Installed kernel source in $(pwd)"
-cd ..
+if [ "$AFXDP" ]; then
+sudo make headers_install INSTALL_HDR_PATH=/usr
+pushd tools/lib/bpf/
+# Bulding with gcc because there are some issues in make files
+# that breaks building libbpf with clang on Travis.
+CC=gcc sudo make install
+CC=gcc sudo make install_headers
+sudo ldconfig
+popd
+# The Linux kernel defines __always_inline in stddef.h (283d7573), and
+# sys/cdefs.h tries to re-define it. Older libc-dev package in xenial
+# doesn't have a fix for this issue. Applying it manually.
+sudo sed -i '/^# define __always_inline .*/i # undef __always_inline' \
+/usr/include/x86_64-linux-gnu/sys/cdefs.h || true
+EXTRA_OPTS="${EXTRA_OPTS} --enable-afxdp"
+else
+EXTRA_OPTS="${EXTRA_OPTS} --with-linux=$(pwd)"
+echo "Installed kernel source in $(pwd)"
+fi
+popd
}
function install_dpdk()
@@ -127,8 +144,9 @@ function build_ovs()
configure_ovs $OPTS
make selinux-policy
-# Only build datapath if we are testing kernel w/o running testsuite
-if [ "${KERNEL}" ]; then
+# Only build datapath if we are testing kernel w/o running testsuite and
+# AF_XDP support.
+if [ "${KERNEL}" ] && ! [ "$AFXDP" ]; then
pushd datapath
make -j4
popd
@@ -161,6 +179,10 @@ elif [ "$M32" ]; then
export CC="$CC -m32"
else
OPTS="--enable-sparse"
+if [ "$AFXDP" ]; then
+# netdev-afxdp uses memset for 64M for umem initialization.
+SPARSE_FLAGS="${SPARSE_FLAGS} -Wno-memcpy-max-count"
+fi
CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${SPARSE_FLAGS}"
fi
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 05/10] net: openvswitch: optimize flow-mask looking up
> > Hi Tonghao,
> >
> > Does this improve performance? After all, the original code simply
> > check whether the mask is NULL, then goto next mask.
> I tested the performance, but I disable the mask cache, and use the
> dpdk-pktgen to generate packets:
> The test ovs flow:
> ovs-dpctl add-dp system@ovs-system
> ovs-dpctl add-if system@ovs-system eth6
> ovs-dpctl add-if system@ovs-system eth7
>
> for m in $(seq 1 100 | xargs printf '%.2x\n'); do
>ovs-dpctl add-flow ovs-system
> "in_port(1),eth(dst=00:$m:00:00:00:00/ff:ff:ff:ff:ff:$m),eth_type(0x0800),ipv4(frag=no)"
> 2
> done
>
> ovs-dpctl add-flow ovs-system
> "in_port(1),eth(dst=98:03:9b:6e:4a:f5/ff:ff:ff:ff:ff:ff),eth_type(0x0800),ipv4(frag=no)"
> 2
> ovs-dpctl add-flow ovs-system
> "in_port(2),eth(dst=98:03:9b:6e:4a:f4/ff:ff:ff:ff:ff:ff),eth_type(0x0800),ipv4(frag=no)"
> 1
>
> for m in $(seq 101 160 | xargs printf '%.2x\n'); do
> ovs-dpctl add-flow ovs-system
> "in_port(1),eth(dst=00:$m:00:00:00:00/ff:ff:ff:ff:ff:$m),eth_type(0x0800),ipv4(frag=no)"
> 2
> done
>
> for m in $(seq 1 100 | xargs printf '%.2x\n'); do
> ovs-dpctl del-flow ovs-system
> "in_port(1),eth(dst=00:$m:00:00:00:00/ff:ff:ff:ff:ff:$m),eth_type(0x0800),ipv4(frag=no)"
> done
>
> Without this patch: 982481pps (64B)
> With this patch: 1112495 pps (64B), about 13% improve
>
Hi Tonghao,
Thanks for doing the measurement.
Based on the result (skipping 100 NULL mask lookup with 13% improvement),
and with additional overhead of mask cache being invalidate while
refilling these 100
gap, I'd argue that this patch is not necessary. But let's wait for
others comments.
Regards,
William
> > However, with your patch, isn't this invalidated the mask cache entry which
> > point to the "M" you swap to the front? See my commands inline.
> >
> > >
> > > Signed-off-by: Tonghao Zhang
> > > Tested-by: Greg Rose
> > > ---
> > > static struct table_instance *table_instance_expand(struct
> > > table_instance *ti,
> > > @@ -704,21 +704,33 @@ static struct table_instance
> > > *table_instance_expand(struct table_instance *ti,
> > > return table_instance_rehash(ti, ti->n_buckets * 2, ufid);
> > > }
> > >
> > > -static void tbl_mask_array_delete_mask(struct mask_array *ma,
> > > - struct sw_flow_mask *mask)
> > > +static void tbl_mask_array_del_mask(struct flow_table *tbl,
> > > + struct sw_flow_mask *mask)
> > > {
> > > - int i;
> > > + struct mask_array *ma = ovsl_dereference(tbl->mask_array);
> > > + int i, ma_count = READ_ONCE(ma->count);
> > >
> > > /* Remove the deleted mask pointers from the array */
> > > - for (i = 0; i < ma->max; i++) {
> > > - if (mask == ovsl_dereference(ma->masks[i])) {
> > > - RCU_INIT_POINTER(ma->masks[i], NULL);
> > > - ma->count--;
> > > - kfree_rcu(mask, rcu);
> > > - return;
> > > - }
> > > + for (i = 0; i < ma_count; i++) {
> > > + if (mask == ovsl_dereference(ma->masks[i]))
> > > + goto found;
> > > }
> > > +
> > > BUG();
> > > + return;
> > > +
> > > +found:
> > > + WRITE_ONCE(ma->count, ma_count -1);
> > > +
> > > + rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]);
> > > + RCU_INIT_POINTER(ma->masks[ma_count -1], NULL);
> >
> > So when you swap the ma->masks[ma_count -1], the mask cache entry
> > who's 'mask_index == ma_count' become all invalid?
> Yes, a little tricky.
> > Regards,
> > William
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rhel: Remove the cond 'build_python3'
On Mon, Oct 21, 2019 at 03:12:42PM +0530, num...@ovn.org wrote: > From: Numan Siddique > > A previous patch removed python2 support from ovs. So we can remove > this condition and make python3 mandatory for builds. Without this > patch, make rpm-fedora on centos 7 fails unless we pass > RPMBUILD_OPT="--with build_python3". > > Signed-off-by: Numan Siddique Thanks. I read this and applied this to master. I did not actually test it. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] Add DNSSL support to OVN
Introduce the possibility to specify a DNSSL option to Router
Advertisement packets. DNS Search list can be specified using
'dnssl' tag in the ipv6_ra_configs column of logical router
port table
Signed-off-by: Lorenzo Bianconi
---
controller/pinctrl.c | 62 +++-
northd/ovn-northd.c | 4 +++
ovn-nb.xml | 4 +++
tests/ovn.at | 30 ++---
4 files changed, 89 insertions(+), 11 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 5bc4b35e7..b522e3359 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2194,6 +2194,7 @@ struct ipv6_ra_config {
struct lport_addresses prefixes;
struct in6_addr rdnss;
bool has_rdnss;
+struct ds dnssl;
};
struct ipv6_ra_state {
@@ -2214,6 +2215,17 @@ struct nd_rdnss_opt {
const ovs_be128 dns[0];
};
+/* DNSSL option RFC 6106 */
+#define ND_OPT_DNSSL31
+#define ND_DNSSL_OPT_LEN8
+struct ovs_nd_dnssl {
+u_int8_t type; /* ND_OPT_DNSSL */
+u_int8_t len; /* >= 2 */
+ovs_be16 reserved;
+ovs_be32 lifetime;
+char dnssl[0];
+};
+
static void
init_ipv6_ras(void)
{
@@ -2225,6 +2237,7 @@ ipv6_ra_config_delete(struct ipv6_ra_config *config)
{
if (config) {
destroy_lport_addresses(&config->prefixes);
+ds_destroy(&config->dnssl);
free(config);
}
}
@@ -2263,6 +2276,7 @@ ipv6_ra_update_config(const struct sbrec_port_binding *pb)
nd_ra_min_interval_default(config->max_interval));
config->mtu = smap_get_int(&pb->options, "ipv6_ra_mtu", ND_MTU_DEFAULT);
config->la_flags = IPV6_ND_RA_OPT_PREFIX_ON_LINK;
+ds_init(&config->dnssl);
const char *address_mode = smap_get(&pb->options, "ipv6_ra_address_mode");
if (!address_mode) {
@@ -2308,6 +2322,11 @@ ipv6_ra_update_config(const struct sbrec_port_binding
*pb)
}
config->has_rdnss = !!rdnss;
+const char *dnssl = smap_get(&pb->options, "ipv6_ra_dnssl");
+if (dnssl) {
+ds_put_buffer(&config->dnssl, dnssl, strlen(dnssl));
+}
+
return config;
fail:
@@ -2366,6 +2385,44 @@ packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num,
prev_l4_size + size));
}
+static void
+packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime,
+char *dnssl_list)
+{
+char *t0, *r0 = dnssl_list, dnssl[255] = {};
+size_t prev_l4_size = dp_packet_l4_size(b);
+struct ip6_hdr *nh = dp_packet_l3(b);
+size_t size = 8;
+int i = 0;
+
+while ((t0 = strtok_r(r0, ",", &r0))) {
+char *t1, *r1 = t0;
+
+size += strlen(t0) + 2;
+while ((t1 = strtok_r(r1, ".", &r1))) {
+dnssl[i++] = strlen(t1);
+memcpy(&dnssl[i], t1, strlen(t1));
+i += strlen(t1);
+}
+dnssl[i++] = 0;
+}
+size = ROUND_UP(size, 8);
+nh->ip6_plen = htons(prev_l4_size + size);
+
+struct ovs_nd_dnssl *nd_dnssl = dp_packet_put_uninit(b, size);
+nd_dnssl->type = ND_OPT_DNSSL;
+nd_dnssl->len = size / 8;
+nd_dnssl->reserved = 0;
+nd_dnssl->lifetime = lifetime;
+memcpy(&nd_dnssl->dnssl[0], dnssl, size);
+
+struct ovs_ra_msg *ra = dp_packet_l4(b);
+ra->icmph.icmp6_cksum = 0;
+uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra,
+ prev_l4_size + size));
+}
+
/* Called with in the pinctrl_handler thread context. */
static long long int
ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
@@ -2374,7 +2431,7 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state
*ra)
return ra->next_announce;
}
-uint64_t packet_stub[128 / 8];
+uint64_t packet_stub[512 / 8];
struct dp_packet packet;
dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
compose_nd_ra(&packet, ra->config->eth_src, ra->config->eth_dst,
@@ -2393,6 +2450,9 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state
*ra)
if (ra->config->has_rdnss) {
packet_put_ra_rdnss_opt(&packet, 1, 0x, &ra->config->rdnss);
}
+if (ra->config->dnssl.length) {
+packet_put_ra_dnssl_opt(&packet, 0x, ra->config->dnssl.string);
+}
uint64_t ofpacts_stub[4096 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d1de36e08..dbb279052 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6489,6 +6489,10 @@ copy_ra_to_sb(struct ovn_port *op, const char
*address_mode)
if (rdnss) {
smap_add(&options, "ipv6_ra_rdnss", rdnss);
}
+const char *dnssl = smap_get(&op->nbrp->ipv6_ra_configs, "dnssl");
+if (dnssl) {
+smap_add(&options, "ipv6_ra_dnssl", dnssl);
+}
smap_add(&options, "ipv6_ra_src_eth", op->lrp_networ
Re: [ovs-dev] [PATCH net-next v4 00/10] optimize openvswitch flow looking up
On Wed, Oct 16, 2019 at 5:50 AM wrote: > > From: Tonghao Zhang > > This series patch optimize openvswitch for performance or simplify > codes. > > Patch 1, 2, 4: Port Pravin B Shelar patches to > linux upstream with little changes. btw, should we keep Pravin as the author of the above three patches? Regards, William > > Patch 5, 6, 7: Optimize the flow looking up and > simplify the flow hash. > > Patch 8, 9: are bugfix. > > The performance test is on Intel Xeon E5-2630 v4. > The test topology is show as below: > > +---+ > | +---+ | > | | eth0 ovs-switcheth1 | | Host0 > | +---+ | > +---+ > ^ | > | | > | | > | | > | v > +-++ ++-+ > | netperf | Host1 | netserver| Host2 > +--+ +--+ > > We use netperf send the 64B packets, and insert 255+ flow-mask: > $ ovs-dpctl add-flow ovs-switch > "in_port(1),eth(dst=00:01:00:00:00:00/ff:ff:ff:ff:ff:01),eth_type(0x0800),ipv4(frag=no)" > 2 > ... > $ ovs-dpctl add-flow ovs-switch > "in_port(1),eth(dst=00:ff:00:00:00:00/ff:ff:ff:ff:ff:ff),eth_type(0x0800),ipv4(frag=no)" > 2 > $ > $ netperf -t UDP_STREAM -H 2.2.2.200 -l 40 -- -m 18 > > * Without series patch, throughput 8.28Mbps > * With series patch, throughput 46.05Mbps > > v3->v4: > access ma->count with READ_ONCE/WRITE_ONCE API. More information, > see patch 5 comments. > > v2->v3: > update ma point when realloc mask_array in patch 5. > > v1->v2: > use kfree_rcu instead of call_rcu > > Tonghao Zhang (10): > net: openvswitch: add flow-mask cache for performance > net: openvswitch: convert mask list in mask array > net: openvswitch: shrink the mask array if necessary > net: openvswitch: optimize flow mask cache hash collision > net: openvswitch: optimize flow-mask looking up > net: openvswitch: simplify the flow_hash > net: openvswitch: add likely in flow_lookup > net: openvswitch: fix possible memleak on destroy flow-table > net: openvswitch: don't unlock mutex when changing the user_features > fails > net: openvswitch: simplify the ovs_dp_cmd_new > > net/openvswitch/datapath.c | 65 + > net/openvswitch/flow.h | 1 - > net/openvswitch/flow_table.c | 316 > +-- > net/openvswitch/flow_table.h | 19 ++- > 4 files changed, 329 insertions(+), 72 deletions(-) > > -- > 1.8.3.1 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] ovsdb-server: Allow replication from older schema version servers.
From: Numan Siddique
Presently, replication is not allowed if there is a schema version mismatch
between
the schema returned by the active ovsdb-server and the local db schema. This is
causing failures in OVN DB HA deployments during uprades.
In the case of OpenStack tripleo deployment with OVN, OVN DB ovsdb-servers are
deployed on a multi node controller cluster in active/standby mode. During
minor updates or major upgrades, the cluster is updated one at a time. If
a node A is running active OVN DB ovsdb-servers and when it is updated, another
node B becomes active. After the update when OVN DB ovsdb-servers in A are
started,
these ovsdb-servers fail to replicate from the active if there is a schema
version mismatch.
This patch addresses this issue by allowing replication even if there is a
schema version mismatch only if all the active db schema tables and its colums
are
present in the local db schema.
This should not result in any data loss.
Signed-off-by: Numan Siddique
---
v1 -> v2
* Addressed review comments from Ben. The schema version numbers are
not checked.
ovsdb/replication.c| 156 +++--
tests/ovsdb-replication.at | 23 ++
tests/ovsdb-server.at | 109 ++
3 files changed, 263 insertions(+), 25 deletions(-)
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 752b3c89c..6191cb934 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -43,7 +43,7 @@ static struct uuid server_uuid;
static struct jsonrpc_session *session;
static unsigned int session_seqno = UINT_MAX;
-static struct jsonrpc_msg *create_monitor_request(struct ovsdb *db);
+static struct jsonrpc_msg *create_monitor_request(struct ovsdb_schema *);
static void add_monitored_table(struct ovsdb_table_schema *table,
struct json *monitor_requests);
@@ -100,16 +100,27 @@ enum ovsdb_replication_state {
static enum ovsdb_replication_state state;
+struct replication_db {
+struct ovsdb *db;
+bool schema_version_higher;
+ /* Points to the schema received from the active server if
+ * the local db schema version is higher. NULL otherwise. */
+struct ovsdb_schema *active_db_schema;
+};
+
+static bool check_replication_possible(struct ovsdb_schema *local_db_schema,
+ struct ovsdb_schema *active_db_schema);
+
/* All DBs known to ovsdb-server. The actual replication dbs are stored
* in 'replication dbs', which is a subset of all dbs and remote dbs whose
* schema matches. */
static struct shash local_dbs = SHASH_INITIALIZER(&local_dbs);
static struct shash *replication_dbs;
-static struct shash *replication_db_clone(struct shash *dbs);
+static struct shash *replication_dbs_create(void);
static void replication_dbs_destroy(void);
/* Find 'struct ovsdb' by name within 'replication_dbs' */
-static struct ovsdb* find_db(const char *db_name);
+static struct replication_db *find_db(const char *db_name);
void
@@ -152,8 +163,8 @@ send_schema_requests(const struct json *result)
if (name->type == JSON_STRING) {
/* Send one schema request for each remote DB. */
const char *db_name = json_string(name);
-struct ovsdb *db = find_db(db_name);
-if (db) {
+struct replication_db *rdb = find_db(db_name);
+if (rdb) {
struct jsonrpc_msg *request =
jsonrpc_create_request(
"get_schema",
@@ -161,7 +172,7 @@ send_schema_requests(const struct json *result)
json_string_create(db_name)),
NULL);
-request_ids_add(request->id, db);
+request_ids_add(request->id, rdb->db);
jsonrpc_session_send(session, request);
}
}
@@ -206,11 +217,11 @@ replication_run(void)
&& msg->params->array.n == 2
&& msg->params->array.elems[0]->type == JSON_STRING) {
char *db_name = msg->params->array.elems[0]->string;
-struct ovsdb *db = find_db(db_name);
-if (db) {
+struct replication_db *rdb = find_db(db_name);
+if (rdb) {
struct ovsdb_error *error;
error = process_notification(msg->params->array.elems[1],
- db);
+ rdb->db);
if (error) {
ovsdb_error_assert(error);
state = RPL_S_ERR;
@@ -218,6 +229,7 @@ replication_run(void)
}
}
} else if (msg->type == JSONRPC_REPLY) {
+struct replication_db *rdb;
struct ovsdb *db;
if (!request_ids_lookup_and_free(msg->id, &db)) {
VLOG_WARN("received unexpected rep
Re: [ovs-dev] [PATCH v9 2/2] netdev-dpdk:Detailed packet drop statistics
On 18/10/2019 13:59, Ilya Maximets wrote: > On 16.10.2019 19:48, Kevin Traynor wrote: >> On 16/10/2019 13:16, Ilya Maximets wrote: >>> Hi Kevin, >>> >>> Thanks for review. Some comments inline. >>> >>> On 16.10.2019 12:15, Kevin Traynor wrote: Hi Sriram, Thanks for working on making more fine grained stats for debugging. As mentioned yesterday, this patch needs rebase so I just reviewed docs and netdev-dpdk.c which applied. Comments below. On 21/09/2019 03:40, Sriram Vatala wrote: > OVS may be unable to transmit packets for multiple reasons and > today there is a single counter to track packets dropped due to > any of those reasons. The most common reason is that a VM is > unable to read packets fast enough causing the vhostuser port > transmit queue on the OVS side to become full. This manifests > as a problem with VNFs not receiving all packets. Having a > separate drop counter to track packets dropped because the > transmit queue is full will clearly indicate that the problem > is on the VM side and not in OVS. Similarly maintaining separate It reads like a bit of a contradiction. On one hand the "The *most common* reason is that a VM is unable to read packets fast enough". While having a stat "*will clearly indicate* that the problem is on the VM side". > counters for all possible drops helps in indicating sensible > cause for packet drops. > > This patch adds custom software stats counters to track packets > dropped at port level and these counters are displayed along with > other stats in "ovs-vsctl get interface statistics" > command. The detailed stats will be available for both dpdk and > vhostuser ports. > I think the commit msg could be a bit clearer, suggest something like below - feel free to modify (or reject): OVS may be unable to transmit packets for multiple reasons on the DPDK datapath and today there is a single counter to track packets dropped due to any of those reasons. This patch adds custom software stats for the different reasons packets may be dropped during tx on the DPDK datapath in OVS. - MTU drops : drops that occur due to a too large packet size - Qos drops: drops that occur due to egress QOS - Tx failures: drops as returned by the DPDK PMD send function Note that the reason for tx failures is not specificied in OVS. In practice for vhost ports it is most common that tx failures are because there are not enough available descriptors, which is usually caused by misconfiguration of the guest queues and/or because the guest is not consuming packets fast enough from the queues. These counters are displayed along with other stats in "ovs-vsctl get interface statistics" command and are available for dpdk and vhostuser/vhostuserclient ports. Signed-off-by: Sriram Vatala --- v9:... v8:... Note that signed-off, --- and version info should be like this ^^^. otherwise the review version comments will be in the git commit log when it is applied. > -- > Changes since v8: > Addressed comments given by Ilya. > > Signed-off-by: Sriram Vatala > --- >Documentation/topics/dpdk/vhost-user.rst | 13 ++- >lib/netdev-dpdk.c | 81 +++ >utilities/bugtool/automake.mk | 3 +- >utilities/bugtool/ovs-bugtool-get-port-stats | 25 ++ >.../plugins/network-status/openvswitch.xml| 1 + >5 files changed, 105 insertions(+), 18 deletions(-) >create mode 100755 utilities/bugtool/ovs-bugtool-get-port-stats > > diff --git a/Documentation/topics/dpdk/vhost-user.rst > b/Documentation/topics/dpdk/vhost-user.rst > index 724aa62f6..89388a2bf 100644 > --- a/Documentation/topics/dpdk/vhost-user.rst > +++ b/Documentation/topics/dpdk/vhost-user.rst > @@ -551,7 +551,18 @@ processing packets at the required rate. >The amount of Tx retries on a vhost-user or vhost-user-client > interface can be >shown with:: > > - $ ovs-vsctl get Interface dpdkvhostclient0 statistics:tx_retries > + $ ovs-vsctl get Interface dpdkvhostclient0 > statistics:netdev_dpdk_tx_retries I think the "netdev_dpdk_" prefixes should be removed for a few reasons, - These are custom stats that will only be displayed for the dpdk ports so they don't need any extra prefix to say they are for dpdk ports - The 'tx_retries' stat is part of OVS 2.12, I don't like to change its name to a different one in OVS 2.13 unless there is a very good reason - The existing stats don't have this type of prefixes (granted most of them are general stats): >>> >>> The main reason for the
[ovs-dev] [PATCH] lacp: warn transmit failure of lacp pdu
It might be difficult to trace whether LACP PDU tx (as in
response) was successful when the pdu was not transmitted by
egress slave for various reasons (including resource contention
within NIC) and only way to trace its fate is by looking at
ofproto->stats.tx_[packets/bytes] and slave port stats.
Adding a warning when there is tx failure could help user
debug at the root of this problem.
Signed-off-by: Gowrishankar Muthukrishnan
---
ofproto/ofproto-dpif.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 496a16c8a..4af771585 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3405,7 +3405,11 @@ send_pdu_cb(void *port_, const void *pdu, size_t
pdu_size)
pdu_size);
memcpy(packet_pdu, pdu, pdu_size);
-ofproto_dpif_send_packet(port, false, &packet);
+error = ofproto_dpif_send_packet(port, false, &packet);
+if (error) {
+VLOG_WARN_RL(&rl, "port %s: cannot transmit LACP PDU (%s).",
+ port->bundle->name, ovs_strerror(error));
+}
dp_packet_uninit(&packet);
} else {
static struct vlog_rate_limit rll = VLOG_RATE_LIMIT_INIT(1, 10);
--
2.17.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv4] netdev-linux: Detect numa node id.
On 17.10.2019 22:08, William Tu wrote:
The patch detects the numa node id from the name of the netdev,
by reading the '/sys/class/net//device/numa_node'.
If not available, ex: virtual device, or any error happens,
return numa id 0. Currently only the afxdp netdev type uses it,
other linux netdev types are disabled due to no use case.
Signed-off-by: William Tu
---
v4:
Feedbacks from Eelco
- Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/599308893
v3:
Feedbacks from Ilya and Eelco
- update doc, afxdp.rst
- fix coding style
- fix limit of numa node max, by using ovs_numa_numa_id_is_valid
- move the function to netdev-linux
- cache the result of numa_id
- add security check for netdev name
- use fscanf instead of read and convert to int
- revise some error message content
v2:
address feedback from Eelco
fix memory leak of xaspintf
log using INFO instead of WARN
---
Documentation/intro/install/afxdp.rst | 1 -
lib/netdev-afxdp.c| 11 ---
lib/netdev-linux-private.h| 2 ++
lib/netdev-linux.c| 58 ++-
4 files changed, 59 insertions(+), 13 deletions(-)
diff --git a/Documentation/intro/install/afxdp.rst
b/Documentation/intro/install/afxdp.rst
index 820e9d993d8f..6c00c4ad1356 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -309,7 +309,6 @@ Below is a script using namespaces and veth peer::
Limitations/Known Issues
-#. Device's numa ID is always 0, need a way to find numa id from a netdev.
#. No QoS support because AF_XDP netdev by-pass the Linux TC layer. A possible
work-around is to use OpenFlow meter action.
#. Most of the tests are done using i40e single port. Multiple ports and
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index 8eb270c150e8..cfd93fab9f45 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -543,17 +543,6 @@ out:
return err;
}
-int
-netdev_afxdp_get_numa_id(const struct netdev *netdev)
-{
-/* FIXME: Get netdev's PCIe device ID, then find
- * its NUMA node id.
- */
-VLOG_INFO("FIXME: Device %s always use numa id 0.",
- netdev_get_name(netdev));
-return 0;
-}
-
static void
xsk_remove_xdp_program(uint32_t ifindex, int xdpmode)
{
diff --git a/lib/netdev-linux-private.h b/lib/netdev-linux-private.h
index a350be151147..c8f2be47b10b 100644
--- a/lib/netdev-linux-private.h
+++ b/lib/netdev-linux-private.h
@@ -96,6 +96,8 @@ struct netdev_linux {
/* LAG information. */
bool is_lag_master; /* True if the netdev is a LAG master. */
+int numa_id;/* NUMA node id. */
+
#ifdef HAVE_AF_XDP
/* AF_XDP information. */
struct xsk_socket_info **xsks;
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index f4819237370a..add148d83eb7 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -236,6 +236,7 @@ enum {
VALID_VPORT_STAT_ERROR = 1 << 5,
VALID_DRVINFO = 1 << 6,
VALID_FEATURES = 1 << 7,
+VALID_NUMA_ID = 1 << 8,
};
struct linux_lag_slave {
@@ -1391,6 +1392,61 @@ netdev_linux_tap_batch_send(struct netdev *netdev_,
return 0;
}
+static int OVS_UNUSED
+netdev_linux_get_numa_id(const struct netdev *netdev_)
+{
+struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+char *numa_node_path;
+const char *name;
+int node_id;
+FILE *stream;
+
'netdev' fields should be protected by netdev->mutex. So, this
function should take it before accessing 'numa_id' or 'cache_valid'.
+if (netdev->cache_valid & VALID_NUMA_ID) {
+return netdev->numa_id;
+}
+
+netdev->numa_id = 0;
+netdev->cache_valid |= VALID_NUMA_ID;
+
+name = netdev_get_name(netdev_);
+if (strpbrk(name, "/\\")) {
+VLOG_ERR_RL(&rl, "\"%s\" is not a valid name for a port. "
+"A valid name must not include '/' or '\\'."
+"Using numa_id 0", name);
+return 0;
+}
+
+numa_node_path = xasprintf("/sys/class/net/%s/device/numa_node", name);
+
+stream = fopen(numa_node_path, "r");
+if (!stream) {
+/* Virtual device does not have this info. */
+VLOG_INFO_RL(&rl, "%s: Can't open '%s': %s, using numa_id 0",
+ name, numa_node_path, ovs_strerror(errno));
+free(numa_node_path);
+return 0;
+}
+
+if (fscanf(stream, "%d", &node_id) != 1) {
+goto error;
+};
Redundant ';'.
+
+if (!ovs_numa_numa_id_is_valid(node_id)) {
+goto error;
+}
Maybe it will look better if above 2 'if's will be combined like this:
if (fscanf(stream, "%d", &node_id) != 1
|| !ovs_numa_numa_id_is_valid(node_id) {
goto error;
}
What do you think?
+
+netdev->numa_id = node_id;
+fclose(stream);
+free(numa_node_path);
+re
Re: [ovs-dev] [PATCH net] net: openvswitch: free vport unless register_netdevice() succeeds
On Mon, 21 Oct 2019 19:08:01 +0800 Hillf Danton wrote: > Hey Stefano > > On Mon, 21 Oct 2019 18:02:48 +0800 Stefano Brivio wrote: > > > > --- > > This patch was sent to d...@openvswitch.org and appeared on netdev > > only as Pravin replied to it, giving his Acked-by. I contacted the > > Correct. I am unable to find the patches I sent to lkml on > https://lore.kernel.org/lkml/. > > > original author one month ago requesting to resend this to netdev, > > but didn't get an answer, so I'm now resending the original patch. > > > Nor patches to . > Say sorry to you for missing in action. > I sent a mail to sometime ago asking for > how to cure the pain, without a message echoed since. > That is my poor defend. Ouch, and also this reply didn't reach netdev. Looking at headers: Received: from r3-20.sinamail.sina.com.cn (r3-20.sinamail.sina.com.cn [202.108.3.20]) by mx1.redhat.com (Postfix) with SMTP id 9868983F45 for ; Mon, 21 Oct 2019 11:08:14 + (UTC) Received: from unknown (HELO localhost.localdomain)([222.131.66.83]) by sina.com with ESMTP id 5DAD919A82B3; Mon, 21 Oct 2019 19:08:12 +0800 (CST) it's not in DNSRBL or anything, and looks similar enough to e.g.: https://patchwork.kernel.org/patch/10663003/ that was received without issues by majord...@vger.kernel.org. The only relevant difference seems to be the missing HELO from r3-20.sinamail.sina.com.cn, I have no idea why that would be omitted. Headers added by MTA delivering this to me: X-Greylist: Sender passed SPF test, ACL 264 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 21 Oct 2019 11:08:19 + (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 21 Oct 2019 11:08:19 + (UTC) for IP:'202.108.3.20' DOMAIN:'r3-20.sinamail.sina.com.cn' HELO:'r3-20.sinamail.sina.com.cn' FROM:'hdan...@sina.com' RCPT:'' X-RedHat-Spam-Score: 0.001 (FREEMAIL_FROM,SPF_HELO_NONE,SPF_PASS) 202.108.3.20 r3-20.sinamail.sina.com.cn 202.108.3.20 r3-20.sinamail.sina.com.cn it even passes SPF. But maybe the missing HELO is a problem for SPF on vger.kernel.org? Checked against http://vger.kernel.org/majordomo-taboos.txt, also no matches. Sorry, I don't have any suggestions other than contacting postmas...@vger.kernel.org again -- perhaps from a different address. > > net/openvswitch/vport-internal_dev.c | 11 --- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > And feel free to pick up the diff if they make sense, as you see, > they were sent usually without the Signed-off-by tag. I think this submission should be fine, I added you as From: and kept your original Signed-off-by -- thanks for fixing this by the way! -- Stefano ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net] net: openvswitch: free vport unless register_netdevice() succeeds
Hey Stefano On Mon, 21 Oct 2019 18:02:48 +0800 Stefano Brivio wrote: > > --- > This patch was sent to d...@openvswitch.org and appeared on netdev > only as Pravin replied to it, giving his Acked-by. I contacted the Correct. I am unable to find the patches I sent to lkml on https://lore.kernel.org/lkml/. > original author one month ago requesting to resend this to netdev, > but didn't get an answer, so I'm now resending the original patch. > Nor patches to . Say sorry to you for missing in action. I sent a mail to sometime ago asking for how to cure the pain, without a message echoed since. That is my poor defend. > net/openvswitch/vport-internal_dev.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) And feel free to pick up the diff if they make sense, as you see, they were sent usually without the Signed-off-by tag. Thanks Hillf ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH net] net: openvswitch: free vport unless register_netdevice() succeeds
From: Hillf Danton syzbot found the following crash on: HEAD commit:1e78030e Merge tag 'mmc-v5.3-rc1' of git://git.kernel.org/.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=148d3d1a60 kernel config: https://syzkaller.appspot.com/x/.config?x=30cef20daf3e9977 dashboard link: https://syzkaller.appspot.com/bug?extid=13210896153522fe1ee5 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=136aa8c460 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=109ba79260 = BUG: memory leak unreferenced object 0x8881207e4100 (size 128): comm "syz-executor032", pid 7014, jiffies 4294944027 (age 13.830s) hex dump (first 32 bytes): 00 70 16 18 81 88 ff ff 80 af 8c 22 81 88 ff ff .p." 00 b6 23 17 81 88 ff ff 00 00 00 00 00 00 00 00 ..#. backtrace: [<0eb78212>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline] [<0eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline] [<0eb78212>] slab_alloc mm/slab.c:3319 [inline] [<0eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548 [<006ea6c6>] kmalloc include/linux/slab.h:552 [inline] [<006ea6c6>] kzalloc include/linux/slab.h:748 [inline] [<006ea6c6>] ovs_vport_alloc+0x37/0xf0 net/openvswitch/vport.c:130 [] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164 [<56ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199 [<5434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194 [ ] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614 [ ] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629 [ ] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654 [<6694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477 [<88381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665 [ ] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline] [ ] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328 [<67e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917 [ ] sock_sendmsg_nosec net/socket.c:637 [inline] [ ] sock_sendmsg+0x54/0x70 net/socket.c:657 [<4cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311 [ ] __sys_sendmsg+0x80/0xf0 net/socket.c:2356 [ ] __do_sys_sendmsg net/socket.c:2365 [inline] [ ] __se_sys_sendmsg net/socket.c:2363 [inline] [ ] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363 BUG: memory leak unreferenced object 0x88811723b600 (size 64): comm "syz-executor032", pid 7014, jiffies 4294944027 (age 13.830s) hex dump (first 32 bytes): 01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 05 35 82 c1 .5.. backtrace: [<352f46d8>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline] [<352f46d8>] slab_post_alloc_hook mm/slab.h:522 [inline] [<352f46d8>] slab_alloc mm/slab.c:3319 [inline] [<352f46d8>] __do_kmalloc mm/slab.c:3653 [inline] [<352f46d8>] __kmalloc+0x169/0x300 mm/slab.c:3664 [<8e48f3d1>] kmalloc include/linux/slab.h:557 [inline] [<8e48f3d1>] ovs_vport_set_upcall_portids+0x54/0xd0 net/openvswitch/vport.c:343 [<541e4f4a>] ovs_vport_alloc+0x7f/0xf0 net/openvswitch/vport.c:139 [ ] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164 [<56ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199 [<5434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194 [ ] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614 [ ] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629 [ ] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654 [<6694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477 [<88381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665 [ ] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline] [ ] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328 [<67e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917 [ ] sock_sendmsg_nosec net/socket.c:637 [inline] [ ] sock_sendmsg+0x54/0x70 net/socket.c:657 [<4cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311 [ ] __sys
[ovs-dev] [PATCH] rhel: Remove the cond 'build_python3'
From: Numan Siddique
A previous patch removed python2 support from ovs. So we can remove
this condition and make python3 mandatory for builds. Without this
patch, make rpm-fedora on centos 7 fails unless we pass
RPMBUILD_OPT="--with build_python3".
Signed-off-by: Numan Siddique
---
rhel/openvswitch-fedora.spec.in | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 80010a41b..3a87c6d0c 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -28,10 +28,7 @@
%bcond_without libcapng
# To enable DPDK support, specify '--with dpdk' when building
%bcond_with dpdk
-# Enable Python 3 by specifying '--with build_python3'.
-# This is enabled by default for versions of the distribution that
-# have Python 3 by default (Fedora > 22).
-%bcond_with build_python3
+
# If there is a need to automatically enable the package after installation,
# specify the "--with autoenable"
%bcond_with autoenable
@@ -61,9 +58,7 @@ Source:
http://openvswitch.org/releases/%{name}-%{version}.tar.gz
BuildRequires: gcc gcc-c++
BuildRequires: autoconf automake libtool
BuildRequires: systemd-units openssl openssl-devel
-%if 0%{?fedora} > 22 || %{with build_python3}
BuildRequires: python3-devel
-%endif
BuildRequires: desktop-file-utils
BuildRequires: groff graphviz
BuildRequires: checkpolicy, selinux-policy-devel
@@ -109,7 +104,6 @@ Requires: selinux-policy-targeted
%description selinux-policy
Tailored Open vSwitch SELinux policy
-%if 0%{?fedora} > 22 || %{with build_python3}
%package -n python3-openvswitch
Summary: Open vSwitch python3 bindings
License: ASL 2.0
@@ -120,7 +114,6 @@ Requires: python3-six
%description -n python3-openvswitch
Python bindings for the Open vSwitch database
-%endif
%package test
Summary: Open vSwitch testing utilities
@@ -244,11 +237,9 @@ install -p -m 0755
rhel/etc_sysconfig_network-scripts_ifdown-ovs \
install -p -m 0755 rhel/etc_sysconfig_network-scripts_ifup-ovs \
$RPM_BUILD_ROOT/%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
-%if 0%{?fedora} > 22 || %{with build_python3}
install -d -m 0755 $RPM_BUILD_ROOT%{python3_sitelib}
cp -a $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/* \
$RPM_BUILD_ROOT%{python3_sitelib}
-%endif
rm -rf $RPM_BUILD_ROOT/%{_datadir}/openvswitch/python/
@@ -388,10 +379,8 @@ fi
%defattr(-,root,root)
%{_datadir}/selinux/packages/%{name}/openvswitch-custom.pp
-%if 0%{?fedora} > 22 || %{with build_python3}
%files -n python3-openvswitch
%{python3_sitelib}/ovs
-%endif
%files test
%{_bindir}/ovs-test
--
2.21.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn.at: Fix vtep autotest.
On Fri, Oct 18, 2019 at 8:01 PM Numan Siddique wrote:
>
>
>
> On Fri, Oct 18, 2019 at 10:57 PM Ben Pfaff wrote:
>>
>> On Thu, Oct 17, 2019 at 03:51:42PM +0200, Dumitru Ceara wrote:
>> > With the removal of the OVS subtree the vtep autotest stopped working
>> > because the vtep.ovsschema couldn't be found anymore. The failure was
>> > however hidden by the "|| return 1" when failing to create the vtep DB.
>> >
>> > Fix the path to the vtep schema and make potential failures visible.
>> >
>> > CC: Numan Siddique
>> > Fixes: 84bf7d8435b8 ("Remove ovs subtree")
>> > Signed-off-by: Dumitru Ceara
>>
>> I wonder how the "|| return 1" got there in the first place.
>>
>> Acked-by: Ben Pfaff
>
>
> Thanks. I applied this patch to master.
>
> Numan
Thanks Ben and Numan!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] lldp: Fix for OVS crashes when a LLDP-enabled port is deleted
Issue:
When LLDP is enabled on a port, a structure to hold LLDP related state
is created and that structure has a reference to the port. The ofproto
monitor thread accesses the LLDP structure to periodically send packets
over the associated port. When the port is deleted, the LLDP structure
is not cleaned up and it continues to refer to the deleted port.
When the monitor thread attempts to access the deleted port OVS crashes.
Crash can happen with bridge delete and bond delete also.
Fix:
Remove all references to the LLDP structure and free it when
the port is deleted.
Signed-off-by: Surya Rudra
---
ofproto/ofproto-dpif.c | 10 +-
ofproto/ofproto.c | 3 +++
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 496a16c..ab5f487 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2374,24 +2374,24 @@ set_lldp(struct ofport *ofport_,
const struct smap *cfg)
{
struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
+struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
int error = 0;
if (cfg) {
if (!ofport->lldp) {
-struct ofproto_dpif *ofproto;
-
-ofproto = ofproto_dpif_cast(ofport->up.ofproto);
ofproto->backer->need_revalidate = REV_RECONFIGURE;
ofport->lldp = lldp_create(ofport->up.netdev, ofport_->mtu, cfg);
}
if (!lldp_configure(ofport->lldp, cfg)) {
+lldp_unref(ofport->lldp);
+ofport->lldp = NULL;
error = EINVAL;
}
-}
-if (error) {
+} else if (ofport->lldp) {
lldp_unref(ofport->lldp);
ofport->lldp = NULL;
+ofproto->backer->need_revalidate = REV_RECONFIGURE;
}
ofproto_dpif_monitor_port_update(ofport,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index b4249b0..3aaa45a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2556,6 +2556,9 @@ ofproto_port_unregister(struct ofproto *ofproto,
ofp_port_t ofp_port)
{
struct ofport *port = ofproto_get_port(ofproto, ofp_port);
if (port) {
+if (port->ofproto->ofproto_class->set_lldp) {
+port->ofproto->ofproto_class->set_lldp(port, NULL);
+}
if (port->ofproto->ofproto_class->set_stp_port) {
port->ofproto->ofproto_class->set_stp_port(port, NULL);
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tc: Limit the max action number to 16
On 2019-10-18 1:00 PM, Simon Horman wrote:
> On Wed, Oct 16, 2019 at 11:53:52AM +, Roi Dayan wrote:
>>
>>
>> On 2019-10-16 2:40 PM, Simon Horman wrote:
>>> On Wed, Oct 16, 2019 at 11:37:14AM +0300, Roi Dayan wrote:
From: Chris Mi
Currently, ovs supports to offload max TCA_ACT_MAX_PRIO(32) actions.
But net sched api has a limit of 4K message size which is not enough
for 32 actions when echo flag is set.
After a lot of testing, we find that 16 actions is a reasonable number.
So in this commit, we introduced a new define to limit the max actions.
Fixes: 0c70132cd288("tc: Make the actions order consistent")
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
>>>
>>> Hi Chris,
>>>
>>> I'm unclear on what problem is this patch solving.
>>
>> Hi Simon,
>>
>> I can help with the answer here.
>> When ovs send netlink msg to tc to add a filter we also add
>> the echo flag to get a reply data. this reply can be big as
>> it contains everything from the request and if it pass the 4K
>> size then the kernel will just not fill/send it but the return
>> status of the tc command is still 0 for success.
>>
>> In ovs we use that reply to get back the handle id on success but
>> for this case will fail.
>> To make sure this ack reply always exists for successful tc calls
>> we limit the number of actions here to avoid reaching the 4K size limit.
>
> Thanks,
>
> It sounds like it would be good to develop a mechanism where
> the information required can be retrieved in a less verbose manner,
> thus allowing a higher limit.
>
> But I do agree that this resolves a problem and I have applied it to
> master. Let me know if you think it is also appropriate for older branches.
>
> Kind regards,
> Simon
>
thanks Simon.
this patch can be applied also to branches 2.10, 2.11, 2.12.
>>
>> Thanks,
>> Roi
>>
>>>
---
lib/netdev-offload-tc.c | 4 ++--
lib/tc.c| 6 +++---
lib/tc.h| 4 +++-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 6814390eab2f..f6d1abb2e695 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1360,8 +1360,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct
match *match,
}
NL_ATTR_FOR_EACH(nla, left, actions, actions_len) {
-if (flower.action_count >= TCA_ACT_MAX_PRIO) {
-VLOG_DBG_RL(&rl, "Can only support %d actions",
flower.action_count);
+if (flower.action_count >= TCA_ACT_MAX_NUM) {
+VLOG_DBG_RL(&rl, "Can only support %d actions",
TCA_ACT_MAX_NUM);
return EOPNOTSUPP;
}
action = &flower.actions[flower.action_count];
diff --git a/lib/tc.c b/lib/tc.c
index 316a0ee5dc7c..d5487097d8ec 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1458,7 +1458,7 @@ static int
nl_parse_flower_actions(struct nlattr **attrs, struct tc_flower *flower)
{
const struct nlattr *actions = attrs[TCA_FLOWER_ACT];
-static struct nl_policy actions_orders_policy[TCA_ACT_MAX_PRIO + 1] =
{};
+static struct nl_policy actions_orders_policy[TCA_ACT_MAX_NUM + 1] =
{};
struct nlattr *actions_orders[ARRAY_SIZE(actions_orders_policy)];
const int max_size = ARRAY_SIZE(actions_orders_policy);
@@ -1477,8 +1477,8 @@ nl_parse_flower_actions(struct nlattr **attrs,
struct tc_flower *flower)
if (actions_orders[i]) {
int err;
-if (flower->action_count >= TCA_ACT_MAX_PRIO) {
-VLOG_DBG_RL(&error_rl, "Can only support %d actions",
flower->action_count);
+if (flower->action_count >= TCA_ACT_MAX_NUM) {
+VLOG_DBG_RL(&error_rl, "Can only support %d actions",
TCA_ACT_MAX_NUM);
return EOPNOTSUPP;
}
err = nl_parse_single_action(actions_orders[i], flower);
diff --git a/lib/tc.h b/lib/tc.h
index f4213579a2fd..f4073c6c47e6 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -208,6 +208,8 @@ enum tc_offloaded_state {
TC_OFFLOADED_STATE_NOT_IN_HW,
};
+#define TCA_ACT_MAX_NUM 16
+
struct tc_flower {
uint32_t handle;
uint32_t prio;
@@ -216,7 +218,7 @@ struct tc_flower {
struct tc_flower_key mask;
int action_count;
-struct tc_action actions[TCA_ACT_MAX_PRIO];
+struct tc_action actions[TCA_ACT_MAX_NUM];
struct ovs_flow_stats stats;
uint64_t lastused;
--
2.8.4
___
dev mailing list
d...@openvswitch.org
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.
