Re: [ovs-dev] [PATCH ovn] ovn-controller: Reduce size of the SB monitor condition.
On Tue, Jun 6, 2023 at 7:57 AM Dumitru Ceara wrote:
>
> We don't need to explicitly add port bindings that were already bound
> locally. We implicitly get those because we monitor the datapaths
> they're attached to.
>
> When performing an ovn-heater 500-node density-heavy scale test [0], with
> conditional monitoring enabled, the unreasonably long poll intervals on
> the Southbound database (the ones that took more than one second) are
> measured as:
>
> -
>Count MinMax Median Mean 95 percentile
> -
> 56.0 1010.0 2664.0 1455.5 1544.9 2163.0
> 77.0 1015.0 3460.0 1896.0 1858.2 2907.8
> 69.0 1010.0 3118.0 1618.0 1688.1 2582.4
> -
>202.0 1010.0 3460.0 1610.0 1713.3 2711.5
>
> Compared to the baseline results (also with conditional monitoring
> enabled):
> -
>Count MinMax Median Mean 95 percentile
> -
>141.0 1003.018338.0 1721.0 2625.4 7626.0
>151.0 1019.080297.0 1808.0 3410.7 9089.0
>165.0 1007.050736.0 2354.0 3067.7 7309.8
> -
>457.0 1003.080297.0 2131.0 3044.6 7799.6
>
> We see significant improvement on the server side. This is explained
> by the fact that now the Southbound database server doesn't need to
> apply as many conditions as before when filtering individual monitor
> contents.
>
Thanks Dumitru for the great improvement! This is very helpful for the high
port-density environment.
Just to make sure I understand the test result correctly, in [0], it shows
22500 pods and 500 nodes, so is it 45 pods per node?
> Note: Sub-ports - OVN switch ports with parent_port set - have to be
> monitored unconditionally as we cannot efficiently determine their local
> datapaths without including all local OVS interfaces in the monitor.
> This, however, should not be a huge issue because the majority of ports
> are regular VIFs, not sub-ports.
I am not sure if we can make such a conclusion. For the current ovn-k8s or
environments similar to that, it shouldn't be a problem.
However, for environments that model pods/containers as sub-ports of the VM
VIFs, probably most of the majority of the ports would be sub-ports. This
is what sub-ports are designed for, right?
So, I think this would be a significant change of data monitored for those
environments. I'd suggest at least we should properly document the
implication in the documents (such as ovn-monitor-all, and also the
sub-port related parts). There may also be such users who prefer not
monitoring all sub-ports (for efficiency of ovn-controller) sacrificing SB
DB performance (probably they don't have very high port density so the
conditional monitoring impact is not that big). I am not aware of any such
users yet, but if they complain, we will have to provide a knob, if no
better ideas.
Other than this, the patch looks good to me.
Acked-by: Han Zhou
>
> [0]
https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2139194
> Reported-by: Ilya Maximets
> Signed-off-by: Dumitru Ceara
> ---
> controller/binding.c| 2 +-
> controller/binding.h| 2 +-
> controller/ovn-controller.c | 19 ---
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 9b0647b70e..ad03332915 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -761,7 +761,7 @@ local_binding_data_destroy(struct local_binding_data
*lbinding_data)
> }
>
> const struct sbrec_port_binding *
> -local_binding_get_primary_pb(struct shash *local_bindings,
> +local_binding_get_primary_pb(const struct shash *local_bindings,
> const char *pb_name)
> {
> struct local_binding *lbinding =
> diff --git a/controller/binding.h b/controller/binding.h
> index 0e57f02ee5..319cbd263c 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -150,7 +150,7 @@ void local_binding_data_init(struct
local_binding_data *);
> void local_binding_data_destroy(struct local_binding_data *);
>
> const struct sbrec_port_binding *local_binding_get_primary_pb(
> -struct shash *local_bindings, const char *pb_name);
> +const struct shash *local_bindings, const char *pb_name);
> ofp_port_t local_binding_get_lport_ofport(const struct shash
*local_bindings,
>
Re: [ovs-dev] [PATCH v15 0/4] Enhanced checksum support
On 6/19/23 08:06, Mike Pattrick wrote: > On Thu, Jun 15, 2023 at 3:30 PM Ilya Maximets wrote: >> >> On 6/14/23 21:03, Mike Pattrick wrote: >>> This patch set is a stripped down subset of the initial 17 patchset >>> introduced >>> by Flavio Leitner in 2021. >>> >>> The initial omnibus patchset was very complex and included a refactor, which >>> stymied review and would have made backporting more complex. It also didn't >>> resolve an ongoing issue with the DPDK netdev where we are currently >>> incorrectly setting vhost flags, resulting in connectivity inturruptions >>> when >>> upgrading OVS to the full TSO patchset without restarting the attached >>> virtual >>> machines. >>> >>> The current 4-patch set is stripped down to include the following: >>> >>> 1. Public facing documentation on the phylosophy of how OVS handles >>> checksums >>> 2. A method for the user to easily check which checksums are offloaded per >>> interface >>> 3 & 4. Most checksuming activity delayed until a packet is about to be: >>>- sent, or >>>- transformed in such a way that checksumming wouldn't be offloadable >>> regardless, such as encapsulation >>> >>> The main benefit of this set in its current state is an improved handling of >>> checksums when encapsulating packets. But the set lays a groundwork for >>> future >>> work including improvements to how dpdk negotiates virtio vhost flags and >>> more >>> efficent handling of checksums in userspace with tso. >>> >>> This 4-patch reduced set has gone through a lot of revisions so far, >>> including: >>> >>> v5 >>> - Refactor was mostly removed, except for valid->good >>> - Reset unsupported offload flags in send_prepare >>> - Moved send_prepare from process_upcall to netdev_upcall >>> >>> v6 >>> - Re-added tests that were incorrectly excluded from v5 >>> >>> v7 >>> - David Marchand found an issue while upgrading OVS and not rebooting >>> attached >>>vhost VMs where we can't change flags post negotiation or check if they >>> have >>>already been set. >>> - This was temporarily resolved by not setting the offending flags >>> - This issue will be addressed in a more robust fasion if this patchset is >>>applied. >>> >>> v8 >>> - v7 patch 3 failed intel ci, moved some code from patch 4 to patch 3 >>> >>> v9 >>> - Resolved a 10% performance hit in a DPDK-PVP workload that David Marchand >>>found >>> >>> v10 >>> - Large amount of formatting and grammar issues >>> - Ported change to AVX512 code >>> - ovs-appctl command removed >>> - new fields added to netdev status including the information removed from >>>ovs-appctl >>> >>> v11 >>> - AVX512 change introduced a checksum bug due to an incorrectly sized >>>datatype, caught by intel-ci and required a recent Xeon to reproduce. >>> >>> v12 >>> - Updated documentation >>> - Changed tso status field name >>> - Excluded the combination of rte_flow offload, userspace tso, and >>> RAW_ENCAP >>>from simultaniously ocuring >>> - Extended AVX512 support to IPv6 >>> >>> v13 >>> - Re-added erroniously missing mutex annocations from v12 >>> >>> v14 >>> - Rewrote a section of the documentation >>> - Removed exclusion of the combination of rte_flow offload, userspace tso, >>> and >>>RAW_ENCAP >>> - Added a TUNGETFEATURES check in netdev-linux.c for better support of >>> early >>>2.6 era kernels >>> >>> v15 >>> - Moved some new code outside of mutex protection >>> - Only check TUNGETFEATURES once >>> - Respect FLOW_TNL_F_CSUM flag >>> >>> Mike Pattrick (4): >>> Documentation: Document netdev offload. >>> dpif-netdev: Show netdev offloading flags. >>> userspace: Enable IP checksum offloading by default. >>> userspace: Enable L4 checksum offloading by default. >>> >>> Documentation/automake.mk | 1 + >>> Documentation/topics/index.rst| 1 + >>> .../topics/userspace-checksum-offloading.rst | 96 +++ >>> lib/conntrack.c | 30 +- >>> lib/dp-packet.c | 40 +++ >>> lib/dp-packet.h | 140 +- >>> lib/dpif-netdev-extract-avx512.c | 57 >>> lib/dpif-netdev.c | 2 + >>> lib/flow.c| 38 ++- >>> lib/ipf.c | 11 +- >>> lib/netdev-dpdk.c | 230 +++- >>> lib/netdev-dummy.c| 22 ++ >>> lib/netdev-linux.c| 258 +- >>> lib/netdev-native-tnl.c | 53 ++-- >>> lib/netdev.c | 81 +++--- >>> lib/odp-execute-avx512.c | 108 +--- >>> lib/odp-execute.c | 21 +- >>> lib/packets.c | 209 +++--- >>> lib/packets.h | 3 + >>>
[ovs-dev] [PATCH v4] Add editorconfig file.
EditorConfig is a file format and collection of text editor plugins for
maintaining consistent coding styles between different editors and IDEs.
Initialize the file following the coding rules in
Documentation/internals/contributing/coding-style.rst and add exceptions
declared in build-aux/initial-tab-allowed-files. Only enforce rules for
*.c and *.h files. Other files should use the default indenting rules
from text editors.
In order for this file to be taken into account (unless they use an
editor with built-in EditorConfig support), developers will have to
install a plugin.
Notes:
* All matching rules are considered. The last matching rule's properties
will override the previous ones.
* The max_line_length property is only supported by a limited number of
EditorConfig plugins. It will be ignored if unsupported.
Link: https://editorconfig.org/
Link: https://github.com/editorconfig/editorconfig-emacs
Link: https://github.com/editorconfig/editorconfig-vim
Link:
https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length
Signed-off-by: Robin Jarry
Cc: Mike Pattrick
Cc: Eelco Chaudron
Cc: Ilya Maximets
---
Notes:
v4:
* Listed files that use tabs more restrictively.
* I assumed that every header under include/linux uses tabs since this
is the coding style of the kernel.
* Also, any header named include/sparse/rte_*.h most likely comes from
DPDK which also uses tabs for indentation.
.editorconfig | 48
Makefile.am | 1 +
2 files changed, 49 insertions(+)
create mode 100644 .editorconfig
diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index ..685c7275005f
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,48 @@
+# See https://editorconfig.org/ for syntax reference.
+
+root = true
+
+[*]
+end_of_line = lf
+insert_final_newline = true
+trim_trailing_whitespace = true
+charset = utf-8
+
+[*.{c,h}]
+indent_style = space
+indent_size = 4
+max_line_length = 79
+
+[include/linux/**.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[include/sparse/rte_*.h]
+indent_style = tab
+tab_width = 8
+
+[include/windows/getopt.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[include/windows/netinet/{icmp6,ip6}.h]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/getopt_long.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/sflow*.{c,h}]
+indent_style = tab
+indent_size = tab
+tab_width = 8
+
+[lib/strsep.c]
+indent_style = tab
+indent_size = tab
+tab_width = 8
diff --git a/Makefile.am b/Makefile.am
index df9c33dfe631..db341504d37f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -82,6 +82,7 @@ EXTRA_DIST = \
.ci/osx-build.sh \
.ci/osx-prepare.sh \
.cirrus.yml \
+ .editorconfig \
.github/workflows/build-and-test.yml \
appveyor.yml \
boot.sh \
--
2.41.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] debian/tests: Run system testsuites as autopkgtest.
The test failure is only on one of the tests and not related to this patch, so looks like a fluke / flaky test. Other than that, I accidentally excluded calls to `update-alternatives` required to run the DPDK test as I mistakenly thought that was not relevant for the upstream package source, but they indeed are. (This is what happens when returning to code > 2 weeks later ;) ) New iteration on the way soon. -- Frode Nordahl On Mon, Jun 19, 2023 at 8:33 AM Frode Nordahl wrote: > > The autopkgtests [0][1] are relevant in an upstream context > because an Open vSwitch contributor may want to have a quick > way of running the upstream system testsuites on recent > Debian/Ubuntu releases in an automated and contained manner. > > During the Debian/Ubuntu/upstream package source sync work [2], a > relatively naive autopkgtest was added. It had been around since > Open vSwitch was initially packaged many years ago. > > Replace the autopkgtest with a test that runs all the upstream > system testsuites instead. > > To run the autopkgtest, take a look at [1] for prerequisites then: > > ./boot.sh && ./configure && make debian-source > autopkgtest \ > --test-name kernel \ > --env DEB_BUILD_OPTIONS="nocheck parallel=32 nodpdk" \ > ../openvswitch_3.1.90-1.dsc \ > -- qemu \ > --cpus 32 \ > --ram-size 8129 \ > autopkgtest-mantic-amd64.img > > 0: https://wiki.debian.org/ContinuousIntegration/autopkgtest > 1: https://packaging.ubuntu.com/html/auto-pkg-test.html > 2: https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/396219.html > > Signed-off-by: Frode Nordahl > --- > debian/automake.mk | 26 - > debian/rules | 8 ++ > debian/tests/afxdp | 1 + > debian/tests/afxdp-skip-tests.txt| 2 + > debian/tests/control | 38 +-- > debian/tests/dpdk| 46 +--- > debian/tests/kernel | 1 + > debian/tests/offloads| 1 + > debian/tests/offloads-skip-tests.txt | 2 + > debian/tests/openflow.py | 66 > debian/tests/run-tests.sh| 151 +++ > debian/tests/system-userspace| 1 + > debian/tests/testlist.py | 74 + > debian/tests/vanilla | 29 - > 14 files changed, 296 insertions(+), 150 deletions(-) > create mode 12 debian/tests/afxdp > create mode 100644 debian/tests/afxdp-skip-tests.txt > mode change 100755 => 12 debian/tests/dpdk > create mode 12 debian/tests/kernel > create mode 12 debian/tests/offloads > create mode 100644 debian/tests/offloads-skip-tests.txt > delete mode 100755 debian/tests/openflow.py > create mode 100755 debian/tests/run-tests.sh > create mode 12 debian/tests/system-userspace > create mode 100755 debian/tests/testlist.py > delete mode 100755 debian/tests/vanilla > > diff --git a/debian/automake.mk b/debian/automake.mk > index 7b2afafae..89341fccc 100644 > --- a/debian/automake.mk > +++ b/debian/automake.mk > @@ -61,10 +61,16 @@ EXTRA_DIST += \ > debian/rules \ > debian/source/format \ > debian/source/lintian-overrides \ > + debian/tests/afxdp \ > + debian/tests/afxdp-skip-tests.txt \ > debian/tests/control \ > debian/tests/dpdk \ > - debian/tests/openflow.py \ > - debian/tests/vanilla \ > + debian/tests/kernel \ > + debian/tests/offloads \ > + debian/tests/offloads-skip-tests.txt \ > + debian/tests/run-tests.sh \ > + debian/tests/system-userspace \ > + debian/tests/testlist.py \ > debian/watch > > check-debian-changelog-version: > @@ -113,7 +119,6 @@ CLEANFILES += debian/control > debian: debian/copyright debian/control > .PHONY: debian > > - > debian-deb: debian > @if test X"$(srcdir)" != X"$(top_builddir)"; then > \ > echo "Debian packages should be built from $(abs_srcdir)/"; > \ > @@ -130,3 +135,18 @@ else > $(AM_V_GEN) DEB_BUILD_OPTIONS="nocheck parallel=`nproc` nodpdk" \ > fakeroot debian/rules binary > endif > + > +debian-source: debian > + @if test X"$(srcdir)" != X"$(top_builddir)"; then > \ > + echo "Debian packages should be built from $(abs_srcdir)/"; > \ > + exit 1; > \ > + fi > + $(MAKE) distclean > + $(update_deb_copyright) > + $(update_deb_control) > + $(AM_V_GEN) fakeroot debian/rules clean > + tar -cvzf ../openvswitch_$(PACKAGE_VERSION).orig.tar.gz \ > + --exclude .git \ > + --transform 's,^\.,openvswitch-$(PACKAGE_VERSION),' \ > + . > + dpkg-source -b . > diff --git a/debian/rules b/debian/rules > index 28c249d07..f4c69651e
Re: [ovs-dev] [PATCH RESEND v11] netdev-dpdk: Add custom rx-steering configuration.
Kevin Traynor, Jun 15, 2023 at 12:53:
> > new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
> > +if (dev->requested_rx_steer_flags != 0) {
> > +new_n_rxq += 1;
>
> If rx-steering is set for the port and the flow has previously not been
> able to be offloaded, the dev->requested_n_rxq will always be different
> than the netdev->n_rxq. That means this device will do a reconfigure
> anytime there is a config change on any device.
>
> e.g. If rx sterring on device A and device A cannot offload flows (this
> is acceptable). Any config change to device B will result in reconfigure
> of device A, not based on flags but based on num of rxqs.
This is why I had added an intermediate dev->user_n_rxq field in v10.
I tried to get rid of it as you suggested but it causes this issue.
Also, I think there's another side effect that is actually worse, if the
rte flow offload fails, dev->requested_n_rxq is decremented by one. If
netdev_dpdk_reconfigure() is called without reinitializing
dev->requested_n_rxq (mtu change or some other event), the value will
eventually drop down to zero and then to negative values.
I'll reintroduce user_n_rxq as an intermediate value that cannot be
modified other by than the user so that these problems cannot occur.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 2/2] northd: introduce ls_datapath_group column in lb sb db table
Introduce ls_datapath_group column in the load_balancer table of the SB
db and deprecate datapath_group one. This patch make the table symmetric
with lr_datapath_group column in the load_balancer table of SB db.
Signed-off-by: Lorenzo Bianconi
---
controller/chassis.c| 8
controller/lflow.c | 10 ++
controller/local_data.c | 8
controller/ovn-controller.c | 7 +++
include/ovn/features.h | 1 +
northd/northd.c | 39 +++--
northd/northd.h | 1 +
ovn-sb.ovsschema| 6 +-
ovn-sb.xml | 6 ++
tests/ovn-controller.at | 4
tests/ovn-northd.at | 14 ++---
utilities/ovn-sbctl.c | 13 +
12 files changed, 103 insertions(+), 14 deletions(-)
diff --git a/controller/chassis.c b/controller/chassis.c
index ce88541ba..6454cf0e3 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -369,6 +369,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg
*ovs_cfg,
smap_replace(config, OVN_FEATURE_MAC_BINDING_TIMESTAMP, "true");
smap_replace(config, OVN_FEATURE_CT_LB_RELATED, "true");
smap_replace(config, OVN_FEATURE_FDB_TIMESTAMP, "true");
+smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true");
}
/*
@@ -502,6 +503,12 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
return true;
}
+if (!smap_get_bool(_rec->other_config,
+ OVN_FEATURE_LS_DPG_COLUMN,
+ false)) {
+return true;
+}
+
return false;
}
@@ -632,6 +639,7 @@ update_supported_sset(struct sset *supported)
sset_add(supported, OVN_FEATURE_MAC_BINDING_TIMESTAMP);
sset_add(supported, OVN_FEATURE_CT_LB_RELATED);
sset_add(supported, OVN_FEATURE_FDB_TIMESTAMP);
+sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN);
}
static void
diff --git a/controller/lflow.c b/controller/lflow.c
index 22faaf013..26b95911d 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1870,6 +1870,7 @@ add_lb_ct_snat_hairpin_vip_flow(const struct
ovn_controller_lb *lb,
local_datapaths, ,
, flow_table);
}
+/* datapath_group column is deprecated. */
if (lb->slb->datapath_group) {
for (size_t i = 0; i < lb->slb->datapath_group->n_datapaths; i++) {
add_lb_ct_snat_hairpin_for_dp(
@@ -1878,6 +1879,15 @@ add_lb_ct_snat_hairpin_vip_flow(const struct
ovn_controller_lb *lb,
local_datapaths, , , flow_table);
}
}
+if (lb->slb->ls_datapath_group) {
+for (size_t i = 0;
+ i < lb->slb->ls_datapath_group->n_datapaths; i++) {
+add_lb_ct_snat_hairpin_for_dp(
+lb, !!lb_vip->vip_port,
+lb->slb->ls_datapath_group->datapaths[i],
+local_datapaths, , , flow_table);
+}
+}
}
ofpbuf_uninit();
diff --git a/controller/local_data.c b/controller/local_data.c
index 01cfbdefe..3a7d3afeb 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -661,8 +661,16 @@ lb_is_local(const struct sbrec_load_balancer *sbrec_lb,
}
}
+/* datapath_group column is deprecated. */
struct sbrec_logical_dp_group *dp_group = sbrec_lb->datapath_group;
+for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
+if (get_local_datapath(local_datapaths,
+ dp_group->datapaths[i]->tunnel_key)) {
+return true;
+}
+}
+dp_group = sbrec_lb->ls_datapath_group;
for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
if (get_local_datapath(local_datapaths,
dp_group->datapaths[i]->tunnel_key)) {
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 80e836e44..196e8e7e7 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2699,12 +2699,19 @@ load_balancers_by_dp_init(const struct hmap
*local_datapaths,
load_balancers_by_dp_add_one(local_datapaths,
lb->datapaths[i], lb, lbs);
}
+/* datapath_group column is deprecated. */
for (size_t i = 0; lb->datapath_group
&& i < lb->datapath_group->n_datapaths; i++) {
load_balancers_by_dp_add_one(local_datapaths,
lb->datapath_group->datapaths[i],
lb, lbs);
}
+for (size_t i = 0; lb->ls_datapath_group
+ && i < lb->ls_datapath_group->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->ls_datapath_group->datapaths[i],
+
[ovs-dev] [PATCH ovn 1/2] northd: sync lb applied to logical routers in sb db lb table
Introduce lr_datapath_group column in the load_balancer table of the SB
db.
Sync load_balancers applied to logical_routers to Load_Balancer table in
the SouthBound database.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2193323
Signed-off-by: Lorenzo Bianconi
---
controller/local_data.c | 8 +++
controller/ovn-controller.c | 6 ++
lib/lb.h| 3 +-
northd/northd.c | 78 ++--
ovn-sb.ovsschema| 6 +-
ovn-sb.xml | 6 ++
tests/ovn-northd.at | 17 +-
tests/system-ovn.at | 117
8 files changed, 218 insertions(+), 23 deletions(-)
diff --git a/controller/local_data.c b/controller/local_data.c
index cf0b21bb1..01cfbdefe 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -670,5 +670,13 @@ lb_is_local(const struct sbrec_load_balancer *sbrec_lb,
}
}
+dp_group = sbrec_lb->lr_datapath_group;
+for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
+if (get_local_datapath(local_datapaths,
+ dp_group->datapaths[i]->tunnel_key)) {
+return true;
+}
+}
+
return false;
}
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index a47406979..80e836e44 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2705,6 +2705,12 @@ load_balancers_by_dp_init(const struct hmap
*local_datapaths,
lb->datapath_group->datapaths[i],
lb, lbs);
}
+for (size_t i = 0; lb->lr_datapath_group
+ && i < lb->lr_datapath_group->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->lr_datapath_group->datapaths[i],
+ lb, lbs);
+}
}
return lbs;
}
diff --git a/lib/lb.h b/lib/lb.h
index 23d8fc9e9..2456423cc 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -85,7 +85,8 @@ struct ovn_northd_lb {
size_t n_nb_lr;
unsigned long *nb_lr_map;
-struct ovn_dp_group *dpg;
+struct ovn_dp_group *ls_dpg;
+struct ovn_dp_group *lr_dpg;
};
struct ovn_lb_vip {
diff --git a/northd/northd.c b/northd/northd.c
index a45c8b53a..2203d6c4a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4497,10 +4497,11 @@ ovn_dp_group_get_or_create(struct ovsdb_idl_txn
*ovnsb_txn,
static void
sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
const struct sbrec_load_balancer_table *sbrec_load_balancer_table,
- struct ovn_datapaths *ls_datapaths, struct hmap *lbs)
+ struct ovn_datapaths *ls_datapaths,
+ struct ovn_datapaths *lr_datapaths, struct hmap *lbs)
{
-struct hmap dp_groups = HMAP_INITIALIZER(_groups);
-size_t bitmap_len = ods_size(ls_datapaths);
+struct hmap ls_dp_groups = HMAP_INITIALIZER(_dp_groups);
+struct hmap lr_dp_groups = HMAP_INITIALIZER(_dp_groups);
struct ovn_northd_lb *lb;
/* Delete any stale SB load balancer rows and create datapath
@@ -4525,7 +4526,8 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
* are not indexed in any way.
*/
lb = ovn_northd_lb_find(lbs, _uuid);
-if (!lb || !lb->n_nb_ls || !hmapx_add(_lbs, lb)) {
+if (!lb || (!lb->n_nb_ls && !lb->n_nb_lr) ||
+!hmapx_add(_lbs, lb)) {
sbrec_load_balancer_delete(sbrec_lb);
continue;
}
@@ -4533,11 +4535,20 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
lb->slb = sbrec_lb;
/* Find or create datapath group for this load balancer. */
-lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, _groups,
- lb->slb->datapath_group,
- lb->n_nb_ls, lb->nb_ls_map,
- bitmap_len, true,
- ls_datapaths, NULL);
+if (lb->n_nb_ls) {
+lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn, _dp_groups,
+lb->slb->datapath_group,
+lb->n_nb_ls, lb->nb_ls_map,
+ods_size(ls_datapaths),
+true, ls_datapaths, NULL);
+}
+if (lb->n_nb_lr) {
+lb->lr_dpg = ovn_dp_group_get_or_create(ovnsb_txn, _dp_groups,
+lb->slb->lr_datapath_group,
+lb->n_nb_lr, lb->nb_lr_map,
+ods_size(lr_datapaths),
+false, NULL, lr_datapaths);
+}
}
hmapx_destroy(_lbs);
@@ -4545,7
[ovs-dev] [PATCH ovn 0/2] sync lb applied to logical routers in sb db lb table
Changes since RFC v2: - introduce ls_datapath_group column and deprecate datapath_group one. Changes since RFC v1: - get rid of patch 1/2: northd: rename table datapath_group in ls_datapath_group in load_balancer sb db table Lorenzo Bianconi (2): northd: sync lb applied to logical routers in sb db lb table northd: introduce ls_datapath_group column in lb sb db table controller/chassis.c| 8 +++ controller/lflow.c | 10 +++ controller/local_data.c | 16 + controller/ovn-controller.c | 13 include/ovn/features.h | 1 + lib/lb.h| 3 +- northd/northd.c | 105 ++-- northd/northd.h | 1 + ovn-sb.ovsschema| 10 ++- ovn-sb.xml | 12 tests/ovn-controller.at | 4 ++ tests/ovn-northd.at | 31 +++--- tests/system-ovn.at | 117 utilities/ovn-sbctl.c | 13 14 files changed, 314 insertions(+), 30 deletions(-) -- 2.41.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] netdev-dpdk: Drop TSO in case of conflicting virtio features.
On Wed, Jun 14, 2023 at 9:45 AM David Marchand wrote: > > At some point in OVS history, some virtio features were announced as > supported (ECN and UFO virtio features). > > The userspace TSO code, which has been added later, does not support > those features and tries to disable them. > > This breaks OVS upgrades: if an existing VM already negotiated such > features, their lack on reconnection to an upgraded OVS triggers a > vhost socket disconnection by Qemu. > This results in an endless loop because Qemu then retries with the same > set of virtio features. > > This patch proposes to try and detect those vhost socket disconnection > and fallback restoring the old virtio features (and disabling TSO for this > vhost port). > > Signed-off-by: David Marchand > --- > Note: this series depends on Mike series and won't apply cleanly > without it. Acked-by: Mike Pattrick ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif: Add coverage counters for dpif_operate() failures.
Add additional error coverage counters for dpif operation failures.
This could help to quickly identify netlink problems when communicating
with the OVS kernel module.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2070630
Signed-off-by: Eelco Chaudron
---
lib/dpif.c | 39 ++-
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/lib/dpif.c b/lib/dpif.c
index 3305401fe..b1cbf39c4 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -55,18 +55,22 @@
VLOG_DEFINE_THIS_MODULE(dpif);
COVERAGE_DEFINE(dpif_destroy);
-COVERAGE_DEFINE(dpif_port_add);
-COVERAGE_DEFINE(dpif_port_del);
+COVERAGE_DEFINE(dpif_execute);
+COVERAGE_DEFINE(dpif_execute_error);
+COVERAGE_DEFINE(dpif_execute_with_help);
+COVERAGE_DEFINE(dpif_flow_del);
+COVERAGE_DEFINE(dpif_flow_del_error);
COVERAGE_DEFINE(dpif_flow_flush);
COVERAGE_DEFINE(dpif_flow_get);
+COVERAGE_DEFINE(dpif_flow_get_error);
COVERAGE_DEFINE(dpif_flow_put);
-COVERAGE_DEFINE(dpif_flow_del);
-COVERAGE_DEFINE(dpif_execute);
-COVERAGE_DEFINE(dpif_purge);
-COVERAGE_DEFINE(dpif_execute_with_help);
-COVERAGE_DEFINE(dpif_meter_set);
-COVERAGE_DEFINE(dpif_meter_get);
+COVERAGE_DEFINE(dpif_flow_put_error);
COVERAGE_DEFINE(dpif_meter_del);
+COVERAGE_DEFINE(dpif_meter_get);
+COVERAGE_DEFINE(dpif_meter_set);
+COVERAGE_DEFINE(dpif_port_add);
+COVERAGE_DEFINE(dpif_port_del);
+COVERAGE_DEFINE(dpif_purge);
static const struct dpif_class *base_dpif_classes[] = {
#if defined(__linux__) || defined(_WIN32)
@@ -1381,8 +1385,11 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops,
size_t n_ops,
COVERAGE_INC(dpif_flow_put);
log_flow_put_message(dpif, _module, put, error);
-if (error && put->stats) {
-memset(put->stats, 0, sizeof *put->stats);
+if (error) {
+COVERAGE_INC(dpif_flow_put_error);
+if (put->stats) {
+memset(put->stats, 0, sizeof *put->stats);
+}
}
break;
}
@@ -1392,10 +1399,10 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops,
size_t n_ops,
COVERAGE_INC(dpif_flow_get);
if (error) {
+COVERAGE_INC(dpif_flow_get_error);
memset(get->flow, 0, sizeof *get->flow);
}
log_flow_get_message(dpif, _module, get, error);
-
break;
}
@@ -1404,8 +1411,11 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops,
size_t n_ops,
COVERAGE_INC(dpif_flow_del);
log_flow_del_message(dpif, _module, del, error);
-if (error && del->stats) {
-memset(del->stats, 0, sizeof *del->stats);
+if (error) {
+COVERAGE_INC(dpif_flow_del_error);
+if (del->stats) {
+memset(del->stats, 0, sizeof *del->stats);
+}
}
break;
}
@@ -1414,6 +1424,9 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops,
size_t n_ops,
COVERAGE_INC(dpif_execute);
log_execute_message(dpif, _module, >execute,
false, error);
+if (error) {
+COVERAGE_INC(dpif_execute_error);
+}
break;
}
}
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 9/9] system-offloads-traffic.at: Add vxlan gbp offload test
From: Gavin Li Add a vxlan gbp offload test case: vxlan offloads with gbp extention - ping between two ports - offloads enabled ok Signed-off-by: Gavin Li Reviewed-by: Roi Dayan Reviewed-by: Simon Horman --- tests/system-offloads-traffic.at | 49 1 file changed, 49 insertions(+) diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index ae302a29499a..cabbcfbd168f 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -805,3 +805,52 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device ovs-p0/d /failed to offload flow/d "]) AT_CLEANUP + +AT_SETUP([offloads - ping over vxlan tunnel with gbp - offloads enabled]) +OVS_CHECK_TUNNEL_TSO() +OVS_CHECK_VXLAN() + +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) +ADD_BR([br-underlay]) + +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"]) + +ADD_NAMESPACES(at_ns0) + +dnl Set up underlay link from host into the namespace using veth pair. +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24") +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"]) +AT_CHECK([ip link set dev br-underlay up]) + +dnl Set up tunnel endpoints on OVS outside the namespace and with a native +dnl linux device inside the namespace. +ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24], [options:exts=gbp]) +AT_CHECK([ovs-ofctl add-flow br0 "in_port=br0 actions=load:0x200->NXM_NX_TUN_GBP_ID[], output:at_vxlan0]") +AT_CHECK([ovs-ofctl add-flow br0 "in_port=at_vxlan0, tun_gbp_id=512 actions=output:br0"]) +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) + +ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100], [10.1.1.1/24], + [id 0 dstport 4789 gbp]) +NS_CHECK_EXEC([at_ns0], [iptables -I OUTPUT -p ip -j MARK --set-mark 512 2>/dev/null], [0]) +NS_CHECK_EXEC([at_ns0], [iptables -I INPUT -m mark --mark 512 -j ACCEPT 2>/dev/null], [0], [ignore]) + +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], [0], [dnl +3 packets transmitted, 3 received, 0% packet loss, time 0ms +]) + +dnl Okay, now check the overlay with different packet sizes +NS_CHECK_EXEC([at_ns0], [ping -q -c 1000 -i 0.01 10.1.1.100 | FORMAT_PING], [0], [dnl +1000 packets transmitted, 1000 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | grep "tp_dst=4789,vxlan(gbp(id=512))" | wc -l], [0], [dnl +1 +]) +AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "eth_type(0x0800)" | grep "tp_dst=4789,vxlan(gbp(id=512,flags=0))" | wc -l], [0], [dnl +1 +]) + +dnl First, check the underlay +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + -- 2.38.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 8/9] netdev-tc-offloads: Probe for allowing vxlan gbp support
From: Gavin Li
Kernels that do not support vxlan gbp would treat the rule that has vxlan
gbp encap action or vxlan gbp id match differently, either reject it or
just skip the action/match and continue processing the knowing ones.
To solve the issue, probe and disallow inserting rules with vxlan gbp
action/match if kernel does not support it.
Signed-off-by: Gavin Li
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
lib/netdev-offload-tc.c | 64 +++--
1 file changed, 61 insertions(+), 3 deletions(-)
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index a32ed8021c4b..929a96d1bdbe 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -52,6 +52,7 @@ static struct hmap tc_to_ufid = HMAP_INITIALIZER(_to_ufid);
static bool multi_mask_per_prio = false;
static bool block_support = false;
static uint16_t ct_state_support;
+static bool vxlan_gbp_support = false;
struct netlink_field {
int offset;
@@ -668,7 +669,7 @@ static void parse_tc_flower_geneve_opts(struct tc_action
*action,
nl_msg_end_nested(buf, geneve_off);
}
-static void
+static int
parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
struct ofpbuf *buf)
{
@@ -676,7 +677,10 @@ parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
uint32_t gbp_raw;
if (!action->encap.gbp.id_present) {
-return;
+return 0;
+}
+if (!vxlan_gbp_support) {
+return -EOPNOTSUPP;
}
gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
@@ -684,6 +688,7 @@ parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
action->encap.gbp.id);
nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
nl_msg_end_nested(buf, gbp_off);
+return 0;
}
static void
@@ -846,6 +851,7 @@ parse_tc_flower_to_actions__(struct tc_flower *flower,
struct ofpbuf *buf,
size_t set_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_SET);
size_t tunnel_offset =
nl_msg_start_nested(buf, OVS_KEY_ATTR_TUNNEL);
+int ret;
if (action->encap.id_present) {
nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID, action->encap.id);
@@ -881,7 +887,10 @@ parse_tc_flower_to_actions__(struct tc_flower *flower,
struct ofpbuf *buf,
if (!action->encap.no_csum) {
nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_CSUM);
}
-parse_tc_flower_vxlan_tun_opts(action, buf);
+ret = parse_tc_flower_vxlan_tun_opts(action, buf);
+if (ret) {
+return ret;
+}
parse_tc_flower_geneve_opts(action, buf);
nl_msg_end_nested(buf, tunnel_offset);
nl_msg_end_nested(buf, set_offset);
@@ -1633,6 +1642,9 @@ parse_put_flow_set_action(struct tc_flower *flower,
struct tc_action *action,
}
break;
case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: {
+if (!vxlan_gbp_support) {
+return EOPNOTSUPP;
+}
if (odp_vxlan_tun_opts_from_attr(tun_attr,
>encap.gbp.id,
>encap.gbp.flags,
@@ -2788,6 +2800,51 @@ probe_tc_block_support(int ifindex)
}
}
+static void
+probe_vxlan_gbp_support(int ifindex)
+{
+struct tc_flower flower;
+struct tcf_id id;
+int block_id = 0;
+int prio = 1;
+int error;
+
+error = tc_add_del_qdisc(ifindex, true, block_id, TC_INGRESS);
+if (error) {
+return;
+}
+
+memset(, 0, sizeof flower);
+
+flower.tc_policy = TC_POLICY_SKIP_HW;
+flower.key.eth_type = htons(ETH_P_IP);
+flower.mask.eth_type = OVS_BE16_MAX;
+flower.tunnel = true;
+flower.mask.tunnel.id = OVS_BE64_MAX;
+flower.mask.tunnel.ipv4.ipv4_src = OVS_BE32_MAX;
+flower.mask.tunnel.ipv4.ipv4_dst = OVS_BE32_MAX;
+flower.mask.tunnel.tp_dst = OVS_BE16_MAX;
+flower.mask.tunnel.gbp.id = OVS_BE16_MAX;
+flower.key.tunnel.ipv4.ipv4_src = htonl(0x01010101);
+flower.key.tunnel.ipv4.ipv4_dst = htonl(0x01010102);
+flower.key.tunnel.tp_dst = htons(46354);
+flower.key.tunnel.gbp.id = htons(512);
+
+id = tc_make_tcf_id(ifindex, block_id, prio, TC_INGRESS);
+error = tc_replace_flower(, );
+if (error) {
+goto out;
+}
+
+tc_del_flower_filter();
+
+vxlan_gbp_support = true;
+VLOG_INFO("probe tc: vxlan gbp is supported.");
+
+out:
+tc_add_del_qdisc(ifindex, false, block_id, TC_INGRESS);
+}
+
static int
tc_get_policer_action_ids(struct hmap *map)
{
@@ -2915,6 +2972,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
probe_multi_mask_per_prio(ifindex);
probe_ct_state_support(ifindex);
+probe_vxlan_gbp_support(ifindex);
ovs_mutex_lock(_police_ids_mutex);
meter_police_ids = id_pool_create(METER_POLICE_IDS_BASE,
--
[ovs-dev] [PATCH v5 6/9] tc: Pass encap entirely to nl_msg_put_act_tunnel_key_set
From: Gavin Li
Most of the data members of struct tc_action{ } are defined as anonymous
struct in place. Instead of passing all members of an anonymous struct,
which is not flexible to new members being added, expose encap as named
struct and pass it entirely.
Signed-off-by: Gavin Li
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
lib/tc.c | 57 +++-
lib/tc.h | 38 +++--
2 files changed, 43 insertions(+), 52 deletions(-)
diff --git a/lib/tc.c b/lib/tc.c
index ae1ca57c9d2a..7434b0150f74 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2641,13 +2641,9 @@ nl_msg_put_act_tunnel_geneve_option(struct ofpbuf
*request,
}
static void
-nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, bool id_present,
- ovs_be64 id, ovs_be32 ipv4_src,
- ovs_be32 ipv4_dst, struct in6_addr *ipv6_src,
- struct in6_addr *ipv6_dst,
- ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
- struct tun_metadata *tun_metadata,
- uint8_t no_csum, uint32_t action_pc)
+nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
+ struct tc_action_encap *encap,
+ uint32_t action_pc)
{
size_t offset;
@@ -2659,30 +2655,33 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request,
bool id_present,
nl_msg_put_unspec(request, TCA_TUNNEL_KEY_PARMS, , sizeof tun);
-ovs_be32 id32 = be64_to_be32(id);
-if (id_present) {
+ovs_be32 id32 = be64_to_be32(encap->id);
+if (encap->id_present) {
nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_KEY_ID, id32);
}
-if (ipv4_dst) {
-nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC, ipv4_src);
-nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST, ipv4_dst);
-} else if (ipv6_addr_is_set(ipv6_dst)) {
+if (encap->ipv4.ipv4_dst) {
+nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_SRC,
+encap->ipv4.ipv4_src);
+nl_msg_put_be32(request, TCA_TUNNEL_KEY_ENC_IPV4_DST,
+encap->ipv4.ipv4_dst);
+} else if (ipv6_addr_is_set(>ipv6.ipv6_dst)) {
nl_msg_put_in6_addr(request, TCA_TUNNEL_KEY_ENC_IPV6_DST,
-ipv6_dst);
+>ipv6.ipv6_dst);
nl_msg_put_in6_addr(request, TCA_TUNNEL_KEY_ENC_IPV6_SRC,
-ipv6_src);
+>ipv6.ipv6_src);
}
-if (tos) {
-nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TOS, tos);
+if (encap->tos) {
+nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TOS, encap->tos);
}
-if (ttl) {
-nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TTL, ttl);
+if (encap->ttl) {
+nl_msg_put_u8(request, TCA_TUNNEL_KEY_ENC_TTL, encap->ttl);
}
-if (tp_dst) {
-nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT, tp_dst);
+if (encap->tp_dst) {
+nl_msg_put_be16(request, TCA_TUNNEL_KEY_ENC_DST_PORT,
+encap->tp_dst);
}
-nl_msg_put_act_tunnel_geneve_option(request, tun_metadata);
-nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, no_csum);
+nl_msg_put_act_tunnel_geneve_option(request, >data);
+nl_msg_put_u8(request, TCA_TUNNEL_KEY_NO_CSUM, encap->no_csum);
}
nl_msg_end_nested(request, offset);
}
@@ -3305,17 +3304,7 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct
tc_flower *flower)
}
act_offset = nl_msg_start_nested(request, act_index++);
-nl_msg_put_act_tunnel_key_set(request,
action->encap.id_present,
- action->encap.id,
- action->encap.ipv4.ipv4_src,
- action->encap.ipv4.ipv4_dst,
- >encap.ipv6.ipv6_src,
- >encap.ipv6.ipv6_dst,
- action->encap.tp_dst,
- action->encap.tos,
- action->encap.ttl,
- >encap.data,
- action->encap.no_csum,
+nl_msg_put_act_tunnel_key_set(request, >encap,
action_pc);
nl_msg_put_act_flags(request);
nl_msg_end_nested(request, act_offset);
diff --git a/lib/tc.h b/lib/tc.h
index 95fff37b9b61..1d648282a004 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -210,6 +210,25 @@ enum
[ovs-dev] [PATCH v5 7/9] tc: Add vxlan encap action with gbp option offload
From: Gavin Li
Add TC offload support for vxlan encap with gbp option
Signed-off-by: Gavin Li
Reviewed-by: Gavi Teitz
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
acinclude.m4 | 7
include/linux/tc_act/tc_tunnel_key.h | 17 +++-
lib/netdev-offload-tc.c | 31 ++-
lib/odp-util.c | 41 ++--
lib/odp-util.h | 3 ++
lib/tc.c | 58
lib/tc.h | 1 +
7 files changed, 143 insertions(+), 15 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index ac1eab790041..690a13c25967 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -191,6 +191,13 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
[Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
+ AC_COMPILE_IFELSE([
+AC_LANG_PROGRAM([#include ], [
+int x = TCA_TUNNEL_KEY_ENC_OPTS_VXLAN;
+])],
+[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN], [1],
+ [Define to 1 if TCA_TUNNEL_KEY_ENC_OPTS_VXLAN is available.])])
+
AC_COMPILE_IFELSE([
AC_LANG_PROGRAM([#include ], [
int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
diff --git a/include/linux/tc_act/tc_tunnel_key.h
b/include/linux/tc_act/tc_tunnel_key.h
index f13acf17dd75..17291b90bf3c 100644
--- a/include/linux/tc_act/tc_tunnel_key.h
+++ b/include/linux/tc_act/tc_tunnel_key.h
@@ -1,7 +1,7 @@
#ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
#define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
-#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
+#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN)
#include_next
#else
@@ -53,6 +53,10 @@ enum {
* TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
* attributes
*/
+ TCA_TUNNEL_KEY_ENC_OPTS_VXLAN, /* Nested
+* TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
+* attributes
+*/
__TCA_TUNNEL_KEY_ENC_OPTS_MAX,
};
@@ -70,6 +74,15 @@ enum {
#define TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX \
(__TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX - 1)
-#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
+enum {
+ TCA_TUNNEL_KEY_ENC_OPT_VXLAN_UNSPEC,
+ TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP, /* u32 */
+ __TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX,
+};
+
+#define TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX \
+ (__TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX - 1)
+
+#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_OPTS_VXLAN */
#endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 1c97681bc92b..a32ed8021c4b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -668,6 +668,24 @@ static void parse_tc_flower_geneve_opts(struct tc_action
*action,
nl_msg_end_nested(buf, geneve_off);
}
+static void
+parse_tc_flower_vxlan_tun_opts(struct tc_action *action,
+ struct ofpbuf *buf)
+{
+size_t gbp_off;
+uint32_t gbp_raw;
+
+if (!action->encap.gbp.id_present) {
+return;
+}
+
+gbp_off = nl_msg_start_nested(buf, OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
+gbp_raw = odp_encode_gbp_raw(action->encap.gbp.flags,
+ action->encap.gbp.id);
+nl_msg_put_u32(buf, OVS_VXLAN_EXT_GBP, gbp_raw);
+nl_msg_end_nested(buf, gbp_off);
+}
+
static void
flower_tun_opt_to_match(struct match *match, struct tc_flower *flower)
{
@@ -863,7 +881,7 @@ parse_tc_flower_to_actions__(struct tc_flower *flower,
struct ofpbuf *buf,
if (!action->encap.no_csum) {
nl_msg_put_flag(buf, OVS_TUNNEL_KEY_ATTR_CSUM);
}
-
+parse_tc_flower_vxlan_tun_opts(action, buf);
parse_tc_flower_geneve_opts(action, buf);
nl_msg_end_nested(buf, tunnel_offset);
nl_msg_end_nested(buf, set_offset);
@@ -1552,6 +1570,7 @@ parse_put_flow_set_action(struct tc_flower *flower,
struct tc_action *action,
action->type = TC_ACT_ENCAP;
action->encap.id_present = false;
+action->encap.gbp.id_present = false;
action->encap.no_csum = 1;
flower->action_count++;
NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
@@ -1613,6 +1632,16 @@ parse_put_flow_set_action(struct tc_flower *flower,
struct tc_action *action,
action->encap.data.present.len = nl_attr_get_size(tun_attr);
}
break;
+case OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS: {
+if (odp_vxlan_tun_opts_from_attr(tun_attr,
+ >encap.gbp.id,
+ >encap.gbp.flags,
+ >encap.gbp.id_present)) {
[ovs-dev] [PATCH v5 2/9] odp-util: Extract vxlan gbp option decoding to a function
From: Gavin Li
Extract vxlan gbp option decoding to odp_decode_gbp_raw to be used in
following commits.
Signed-off-by: Gavin Li
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
lib/odp-util.c | 9 +++--
lib/odp-util.h | 8
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 2ec889c417e5..f62dc86c5f9e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3162,8 +3162,7 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool
is_mask,
if (ext[OVS_VXLAN_EXT_GBP]) {
uint32_t gbp = nl_attr_get_u32(ext[OVS_VXLAN_EXT_GBP]);
-tun->gbp_id = htons(gbp & 0x);
-tun->gbp_flags = (gbp >> 16) & 0xFF;
+odp_decode_gbp_raw(gbp, >gbp_id, >gbp_flags);
}
break;
@@ -3753,12 +3752,10 @@ format_odp_tun_vxlan_opt(const struct nlattr *attr,
ovs_be16 id, id_mask;
uint8_t flags, flags_mask = 0;
-id = htons(key & 0x);
-flags = (key >> 16) & 0xFF;
+odp_decode_gbp_raw(key, , );
if (ma) {
uint32_t mask = nl_attr_get_u32(ma);
-id_mask = htons(mask & 0x);
-flags_mask = (mask >> 16) & 0xFF;
+odp_decode_gbp_raw(mask, _mask, _mask);
}
ds_put_cstr(ds, "gbp(");
diff --git a/lib/odp-util.h b/lib/odp-util.h
index a1d0d0fba5de..cf762bdc3547 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -374,6 +374,14 @@ void odp_put_push_eth_action(struct ofpbuf *odp_actions,
const struct eth_addr *eth_src,
const struct eth_addr *eth_dst);
+static inline void odp_decode_gbp_raw(uint32_t gbp_raw,
+ ovs_be16 *id,
+ uint8_t *flags)
+{
+*id = htons(gbp_raw & 0x);
+*flags = (gbp_raw >> 16) & 0xFF;
+}
+
struct attr_len_tbl {
int len;
const struct attr_len_tbl *next;
--
2.38.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 5/9] tc: Add vxlan gbp option flower match offload
From: Gavin Li
Add TC offload support for filtering vxlan tunnels with gbp option
Signed-off-by: Gavin Li
Reviewed-by: Gavi Teitz
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
include/linux/pkt_cls.h | 13 ++
lib/netdev-offload-tc.c | 17
lib/tc.c| 92 +++--
lib/tc.h| 7
4 files changed, 117 insertions(+), 12 deletions(-)
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index a8cd8db5bf88..fb4a7ecea4cc 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -273,6 +273,10 @@ enum {
* TCA_TUNNEL_KEY_ENC_OPTS_GENEVE
* attributes
*/
+ TCA_FLOWER_KEY_ENC_OPTS_VXLAN, /* Nested
+* TCA_TUNNEL_KEY_ENC_OPTS_VXLAN
+* attributes
+*/
__TCA_FLOWER_KEY_ENC_OPTS_MAX,
};
@@ -290,6 +294,15 @@ enum {
#define TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX \
(__TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX - 1)
+enum {
+ TCA_FLOWER_KEY_ENC_OPT_VXLAN_UNSPEC,
+ TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, /* u32 */
+ __TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX,
+};
+
+#define TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX \
+ (__TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX - 1)
+
enum {
TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 4f26dd8cca5f..1c97681bc92b 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1234,6 +1234,15 @@ parse_tc_flower_to_match(const struct netdev *netdev,
match_set_tun_tp_dst_masked(match, flower->key.tunnel.tp_dst,
flower->mask.tunnel.tp_dst);
}
+if (flower->mask.tunnel.gbp.id) {
+match_set_tun_gbp_id_masked(match, flower->key.tunnel.gbp.id,
+flower->mask.tunnel.gbp.id);
+}
+if (flower->mask.tunnel.gbp.flags) {
+match_set_tun_gbp_flags_masked(match,
+ flower->key.tunnel.gbp.flags,
+ flower->mask.tunnel.gbp.flags);
+}
if (!strcmp(netdev_get_type(netdev), "geneve")) {
flower_tun_opt_to_match(match, flower);
@@ -2193,6 +2202,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
*match,
flower.key.tunnel.ttl = tnl->ip_ttl;
flower.key.tunnel.tp_src = tnl->tp_src;
flower.key.tunnel.tp_dst = tnl->tp_dst;
+flower.key.tunnel.gbp.id = tnl->gbp_id;
+flower.key.tunnel.gbp.flags = tnl->gbp_flags;
+flower.key.tunnel.gbp.id_present = !!tnl_mask->gbp_id;
flower.mask.tunnel.ipv4.ipv4_src = tnl_mask->ip_src;
flower.mask.tunnel.ipv4.ipv4_dst = tnl_mask->ip_dst;
@@ -2207,6 +2219,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
*match,
* Degrading the flow down to exact match for now as a workaround. */
flower.mask.tunnel.tp_dst = OVS_BE16_MAX;
flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ?
tnl_mask->tun_id : 0;
+flower.mask.tunnel.gbp.id = tnl_mask->gbp_id;
+flower.mask.tunnel.gbp.flags = tnl_mask->gbp_flags;
+flower.mask.tunnel.gbp.id_present = !!tnl_mask->gbp_id;
memset(_mask->ip_src, 0, sizeof tnl_mask->ip_src);
memset(_mask->ip_dst, 0, sizeof tnl_mask->ip_dst);
@@ -2218,6 +2233,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match
*match,
memset(_mask->tp_dst, 0, sizeof tnl_mask->tp_dst);
memset(_mask->tun_id, 0, sizeof tnl_mask->tun_id);
+memset(_mask->gbp_id, 0, sizeof tnl_mask->gbp_id);
+memset(_mask->gbp_flags, 0, sizeof tnl_mask->gbp_flags);
tnl_mask->flags &= ~FLOW_TNL_F_KEY;
/* XXX: This is wrong! We're ignoring DF and CSUM flags configuration
diff --git a/lib/tc.c b/lib/tc.c
index 223fe6e5e5e9..ae1ca57c9d2a 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -39,6 +39,7 @@
#include "coverage.h"
#include "netlink-socket.h"
#include "netlink.h"
+#include "odp-util.h"
#include "openvswitch/ofpbuf.h"
#include "openvswitch/util.h"
#include "openvswitch/vlog.h"
@@ -699,6 +700,38 @@ nl_parse_geneve_key(const struct nlattr *in_nlattr,
return 0;
}
+static int
+nl_parse_vxlan_key(const struct nlattr *in_nlattr,
+ struct tc_flower_tunnel *tunnel)
+{
+const struct ofpbuf *msg;
+struct nlattr *nla;
+struct ofpbuf buf;
+uint32_t gbp_raw;
+size_t left;
+
+nl_attr_get_nested(in_nlattr, );
+msg =
+
+NL_ATTR_FOR_EACH (nla, left, ofpbuf_at(msg, 0, 0), msg->size) {
+uint16_t type = nl_attr_type(nla);
+
+switch (type) {
+case
[ovs-dev] [PATCH v5 4/9] netlink: Add new function to add NLA_F_NESTED to nested netlink messages
From: Gavin Li
Linux kernel netlink module added NLA_F_NESTED flag checking for nested
netlink messages in 5.2. A nested message without the flag set will be
treated as malformated one. The check is optional and is controlled by
message policy. To avoid this, add NLA_F_NESTED explicitly for all
nested netlink messages with a new function
nl_msg_start_nested_with_flag().
Signed-off-by: Gavin Li
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
lib/netlink.c | 9 +
lib/netlink.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/lib/netlink.c b/lib/netlink.c
index 6215282d6fbe..1e8d5a8ec57d 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -523,6 +523,15 @@ nl_msg_start_nested(struct ofpbuf *msg, uint16_t type)
return offset;
}
+/* Adds the header for nested Netlink attributes to 'msg', with the specified
+ * 'type', and returns the header's offset within 'msg'. It's similar to
+ * nl_msg_start_nested() and uses NLA_F_NESTED flag mandatorily. */
+size_t
+nl_msg_start_nested_with_flag(struct ofpbuf *msg, uint16_t type)
+{
+return nl_msg_start_nested(msg, type | NLA_F_NESTED);
+}
+
/* Finalizes a nested Netlink attribute in 'msg'. 'offset' should be the value
* returned by nl_msg_start_nested(). */
void
diff --git a/lib/netlink.h b/lib/netlink.h
index e9050c31bacd..008604aa60d1 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -81,6 +81,7 @@ void nl_msg_put_string__(struct ofpbuf *, uint16_t type,
const char *value,
void nl_msg_put_string(struct ofpbuf *, uint16_t type, const char *value);
size_t nl_msg_start_nested(struct ofpbuf *, uint16_t type);
+size_t nl_msg_start_nested_with_flag(struct ofpbuf *, uint16_t type);
void nl_msg_end_nested(struct ofpbuf *, size_t offset);
void nl_msg_cancel_nested(struct ofpbuf *, size_t offset);
bool nl_msg_end_non_empty_nested(struct ofpbuf *, size_t offset);
--
2.38.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 1/9] tc: Pass tunnel entirely to tunnel option parse and put functions
From: Gavin Li
Tc flower tunnel key options were encoded in nl_msg_put_flower_tunnel_opts
and decoded in nl_parse_flower_tunnel_opts. Only geneve was supported.
To avoid adding more arguments to the function to support more vxlan
options in the future, change the function arguments to pass tunnel
entirely to it instead of keep adding new arguments.
Signed-off-by: Gavin Li
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
lib/tc.c | 15 ---
lib/tc.h | 34 ++
2 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/lib/tc.c b/lib/tc.c
index 270dc95ce53b..223fe6e5e5e9 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -701,7 +701,7 @@ nl_parse_geneve_key(const struct nlattr *in_nlattr,
static int
nl_parse_flower_tunnel_opts(struct nlattr *options,
-struct tun_metadata *metadata)
+struct tc_flower_tunnel *tunnel)
{
const struct ofpbuf *msg;
struct nlattr *nla;
@@ -716,7 +716,7 @@ nl_parse_flower_tunnel_opts(struct nlattr *options,
uint16_t type = nl_attr_type(nla);
switch (type) {
case TCA_FLOWER_KEY_ENC_OPTS_GENEVE:
-err = nl_parse_geneve_key(nla, metadata);
+err = nl_parse_geneve_key(nla, >metadata);
if (err) {
return err;
}
@@ -828,13 +828,13 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct
tc_flower *flower)
if (attrs[TCA_FLOWER_KEY_ENC_OPTS] &&
attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK]) {
err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS],
- >key.tunnel.metadata);
+ >key.tunnel);
if (err) {
return err;
}
err = nl_parse_flower_tunnel_opts(attrs[TCA_FLOWER_KEY_ENC_OPTS_MASK],
- >mask.tunnel.metadata);
+ >mask.tunnel);
if (err) {
return err;
}
@@ -3446,8 +3446,9 @@ nl_msg_put_masked_value(struct ofpbuf *request, uint16_t
type,
static void
nl_msg_put_flower_tunnel_opts(struct ofpbuf *request, uint16_t type,
- struct tun_metadata *metadata)
+ struct tc_flower_tunnel *tunnel)
{
+struct tun_metadata *metadata = >metadata;
struct geneve_opt *opt;
size_t outer, inner;
int len, cnt = 0;
@@ -3536,9 +3537,9 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct
tc_flower *flower)
nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
}
nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS,
- >key.tunnel.metadata);
+ >key.tunnel);
nl_msg_put_flower_tunnel_opts(request, TCA_FLOWER_KEY_ENC_OPTS_MASK,
- >mask.tunnel.metadata);
+ >mask.tunnel);
}
#define FLOWER_PUT_MASKED_VALUE(member, type) \
diff --git a/lib/tc.h b/lib/tc.h
index cdd3b4f60ec8..b9d449677ed9 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -105,6 +105,23 @@ struct tc_cookie {
size_t len;
};
+struct tc_flower_tunnel {
+struct {
+ovs_be32 ipv4_src;
+ovs_be32 ipv4_dst;
+} ipv4;
+struct {
+struct in6_addr ipv6_src;
+struct in6_addr ipv6_dst;
+} ipv6;
+uint8_t tos;
+uint8_t ttl;
+ovs_be16 tp_src;
+ovs_be16 tp_dst;
+ovs_be64 id;
+struct tun_metadata metadata;
+};
+
struct tc_flower_key {
ovs_be16 eth_type;
uint8_t ip_proto;
@@ -161,22 +178,7 @@ struct tc_flower_key {
uint8_t rewrite_tclass;
} ipv6;
-struct {
-struct {
-ovs_be32 ipv4_src;
-ovs_be32 ipv4_dst;
-} ipv4;
-struct {
-struct in6_addr ipv6_src;
-struct in6_addr ipv6_dst;
-} ipv6;
-uint8_t tos;
-uint8_t ttl;
-ovs_be16 tp_src;
-ovs_be16 tp_dst;
-ovs_be64 id;
-struct tun_metadata metadata;
-} tunnel;
+struct tc_flower_tunnel tunnel;
};
enum tc_action_type {
--
2.38.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 3/9] odp-util: Extract vxlan gbp option encoding to a function
From: Gavin Li
Extract vxlan gbp option encoding to odp_encode_gbp_raw to be used in
following commits.
Signed-off-by: Gavin Li
Reviewed-by: Roi Dayan
Reviewed-by: Simon Horman
---
lib/odp-util.c | 5 +++--
lib/odp-util.h | 5 +
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index f62dc86c5f9e..d2414eb559ba 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3278,10 +3278,11 @@ tun_key_to_attr(struct ofpbuf *a, const struct flow_tnl
*tun_key,
if ((!tnl_type || !strcmp(tnl_type, "vxlan")) &&
(tun_key->gbp_flags || tun_key->gbp_id)) {
size_t vxlan_opts_ofs;
+uint32_t gbp_raw;
vxlan_opts_ofs = nl_msg_start_nested(a,
OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS);
-nl_msg_put_u32(a, OVS_VXLAN_EXT_GBP,
- (tun_key->gbp_flags << 16) | ntohs(tun_key->gbp_id));
+gbp_raw = odp_encode_gbp_raw(tun_key->gbp_flags, tun_key->gbp_id);
+nl_msg_put_u32(a, OVS_VXLAN_EXT_GBP, gbp_raw);
nl_msg_end_nested(a, vxlan_opts_ofs);
}
diff --git a/lib/odp-util.h b/lib/odp-util.h
index cf762bdc3547..163efe7a87b5 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -382,6 +382,11 @@ static inline void odp_decode_gbp_raw(uint32_t gbp_raw,
*flags = (gbp_raw >> 16) & 0xFF;
}
+static inline uint32_t odp_encode_gbp_raw(uint8_t flags, ovs_be16 id)
+{
+return (flags << 16) | ntohs(id);
+}
+
struct attr_len_tbl {
int len;
const struct attr_len_tbl *next;
--
2.38.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 0/9] Add vxlan gbp offload with TC
Hi, This series adds TC offload support for filtering vxlan tunnels with gbp option. First 4 patches do some refactoring and the later patches adds the feature. Thanks, Roi changelog v5: - Fix byte order issue in probe. v4: - probe TC kernel for vxlan gbp support. - add test. - style fix in patch 3. - log warn instead of err in patch 5. v3: - Add function nl_msg_start_nested_with_flag() to be used with TC fiedls that require the nested flag. currently only vxlan gbp tun opts. - Split put flower tunnel opts to sub functions for geneve and vxlan tun opts. v2: - Fix incorrect compat modification in patch "tc: Add vxlan gbp option flower match offload". Gavin Li (9): tc: Pass tunnel entirely to tunnel option parse and put functions odp-util: Extract vxlan gbp option decoding to a function odp-util: Extract vxlan gbp option encoding to a function netlink: Add new function to add NLA_F_NESTED to nested netlink messages tc: Add vxlan gbp option flower match offload tc: Pass encap entirely to nl_msg_put_act_tunnel_key_set tc: Add vxlan encap action with gbp option offload netdev-tc-offloads: Probe for allowing vxlan gbp support system-offloads-traffic.at: Add vxlan gbp offload test acinclude.m4 | 7 + include/linux/pkt_cls.h | 13 ++ include/linux/tc_act/tc_tunnel_key.h | 17 ++- lib/netdev-offload-tc.c | 106 - lib/netlink.c| 9 ++ lib/netlink.h| 1 + lib/odp-util.c | 53 --- lib/odp-util.h | 16 ++ lib/tc.c | 218 --- lib/tc.h | 80 +- tests/system-offloads-traffic.at | 49 ++ 11 files changed, 462 insertions(+), 107 deletions(-) -- 2.38.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 6/19/2023 6:04 PM, Eelco Chaudron wrote: On 19 Jun 2023, at 7:05, Chris Mi wrote: Add three sFlow offload test cases: 3: offloads - sflow with sampling=1 - offloads enabled ok 4: offloads - sflow with sampling=2 - offloads enabled ok 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok Signed-off-by: Chris Mi Reviewed-by: Roi Dayan Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco Sorry for missing your Acked-by for previous patches :( And thanks a lot for acking this patchset. That's so great :) Thanks, Chris ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 8/8] system-offloads-traffic.at: Add sFlow offload test cases
On 19 Jun 2023, at 7:05, Chris Mi wrote: > Add three sFlow offload test cases: > > 3: offloads - sflow with sampling=1 - offloads enabled ok > 4: offloads - sflow with sampling=2 - offloads enabled ok > 5: offloads - ping over vxlan tunnel with sflow - offloads enabled ok > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan > Acked-by: Eelco Chaudron Thanks for making all the suggested changes to this series. This is my final ack, which should conclude the series :) Acked-by: Eelco Chaudron //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 7/8] netdev-offload-tc: Add offload support for sFlow
On 19 Jun 2023, at 7:05, Chris Mi wrote: > Create a unique group ID to map the sFlow info when offloading sample > action to TC. When showing the offloaded datapath flows, translate the > group ID from TC sample action to sFlow info using the mapping. > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan Thanks for making all the suggested changes. Acked-by: Eelco Chaudron //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 5/8] netdev-offload: Add netdev offload recv and recv_wait APIs
On 19 Jun 2023, at 7:05, Chris Mi wrote: > Iterate each registered offload API. It's not a problem for today > since we only have one implementation. > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan > --- > You forgot to include my ACK on v27. So here it is again: Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 6/8] dpif-netlink: Add netdev offload recv in normal recv upcalls
On 19 Jun 2023, at 7:05, Chris Mi wrote: > In thread handler 0, add netdev offload recv in normal recv upcalls. > To avoid starvation, introduce a flag to alternate the order of > receiving normal upcalls and offload upcalls based on that flag. > > Add similar change for recv_wait. > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan You forgot to include my ACK on v27. So here it is again: Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 4/8] netdev-offload-tc: Add sample offload API for TC
On 19 Jun 2023, at 7:05, Chris Mi wrote: > Initialize psample socket. Add sample recv API to receive sampled > packets from psample socket. Add sample recv wait API to add psample > socket fd to poll list. > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan You forgot to include my ACK on v27. So here it is again: Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v28 3/8] netdev-offload-tc: Introduce group ID management API
On 19 Jun 2023, at 7:05, Chris Mi wrote: > When offloading sample action to TC, userspace creates a unique ID > to map sample action and tunnel info and passes this ID to kernel > instead of the sample info. Kernel will send this ID and sampled > packet to userspace. Using the ID, userspace can recover the sample > info and send sampled packet to the right sample monitoring host. > > Signed-off-by: Chris Mi > Reviewed-by: Roi Dayan Thanks for making all the suggested changes. Acked-by: Eelco Chaudron //Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v27 7/8] netdev-offload-tc: Add offload support for sFlow
On 19 Jun 2023, at 7:01, Chris Mi wrote:
>
>>> +if (err) {
>>> +VLOG_ERR_RL(_rl, "No OVS_ACTION_ATTR_USERSPACE attribute");
>>> +}
>>> +return err;
>>> +}
>>> +
>>> +static void
>>> +offload_sample_init(struct offload_sample *sample,
>>> +const struct nlattr *next_actions,
>>> +size_t next_actions_len,
>>> +struct tc_flower *flower,
>>> +const struct flow_tnl *tnl)
>>> +{
>>> +memset(sample, 0, sizeof *sample);
>>> +if (flower->tunnel) {
>>> +sample->tunnel = CONST_CAST(struct flow_tnl *, tnl);
>>> +}
>>> +sample->userspace_actions = CONST_CAST(struct nlattr *, next_actions);
>>> +sample->userspace_actions_len = next_actions_len;
>> Here we should initialize the userspace_actions, to make it work :)
>> This was previously in the clone action in patch 3:
>>
>> @@ -2148,8 +2148,11 @@ offload_sample_init(struct offload_sample *sample,
>> if (flower->tunnel) {
>> sample->tunnel = CONST_CAST(struct flow_tnl *, tnl);
>> }
>> -sample->userspace_actions = CONST_CAST(struct nlattr *, next_actions);
>> -sample->userspace_actions_len = next_actions_len;
>> +
>> +sample->userspace_actions = xmalloc(next_actions_len + NLA_HDRLEN);
>> +nullable_memcpy((char *) sample->userspace_actions + NLA_HDRLEN,
>> +next_actions, next_actions_len);
>> +sample->userspace_actions->nla_len = next_actions_len + NLA_HDRLEN;
>> sample->ifindex = flower->ifindex;
>>
>> One remaining question is, should we set the nla_type here to
>> OVS_ACTION_ATTR_SAMPLE or OVS_ACTION_ATTR_USERSPACE as this what they are,
>> or is it safe to not set this as the upcall handling will ignore the type?
> According to function dpif_get_actions(), nla_type is ignored:
>
> if (upcall->actions) {
> /* Actions were passed up from datapath. */
> *actions = nl_attr_get(upcall->actions);
> actions_len = nl_attr_get_size(upcall->actions);
> }
Thanks! I guess I could have done this myself :(
Cheers,
Eelco
Reviewing your v28 right now. And doing some final testing!
>>
>>> +sample->ifindex = flower->ifindex;
>>> +}
>>> +
>>> +static int
>>> +parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action,
>>> +const struct nlattr *next_actions, size_t
>>> next_actions_len,
>>> +const struct nlattr *sample_action,
>>> +const struct flow_tnl *tnl)
>>> +{
>>> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>>> +struct offload_sample sample;
>>> +const struct nlattr *nla;
>>> +unsigned int left;
>>> +uint32_t sgid;
>>> +int err;
>> As you change the function, you should set err to EINVAL here :)
>>
>>> +
>>> +offload_sample_init(, next_actions, next_actions_len, flower,
>>> tnl);
>>> +sample.action = CONST_CAST(struct nlattr *, sample_action);
>>> +
>>> +err = EINVAL;
>> Remove this here, as you no longer override it with offload_sample_init()
>>
>>> +NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) {
>>> +if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) {
>>> +err = parse_sample_actions_attribute(nla, );
>>> +if (err) {
>>> +break;
>>> +}
>>> +} else if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_PROBABILITY) {
>>> +tc_action->type = TC_ACT_SAMPLE;
>>> +tc_action->sample.rate = UINT32_MAX / nl_attr_get_u32(nla);
>>> +if (!tc_action->sample.rate) {
>>> +break;
>>> +}
>>> +} else {
>> Here you removed err = EINVAL; but it should be here if you want to error
>> out on unsupported attributes.
>>
>> I re-wrote the function a bit, to clean it up, and made it look a bit more
>> like the v18 version. I removed the conditional breaks in the if() branches,
>> as it made it looks like we check for errors in these branch, but in fact
>> the checks are there to verify the attributes where present.
>> Also made sure we do not trash the tc_action structure on failure.
>>
>> static int
>> parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action,
>> const struct nlattr *next_actions, size_t
>> next_actions_len,
>> const struct nlattr *sample_action,
>> const struct flow_tnl *tnl)
>> {
>> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
>> struct offload_sample sample;
>> const struct nlattr *nla;
>> unsigned int left;
>> uint32_t rate = 0;
>> int ret = EINVAL;
>> uint32_t sgid;
>>
>> offload_sample_init(, next_actions, next_actions_len, flower,
>> tnl);
>> sample.action = CONST_CAST(struct nlattr *, sample_action);
>>
>> NL_NESTED_FOR_EACH_UNSAFE (nla, left, sample_action) {
>> if (nl_attr_type(nla) == OVS_SAMPLE_ATTR_ACTIONS) {
>>
Re: [ovs-dev] [PATCH ovn v3 2/2] ci: Run the new check-system-dpdk tests as part of the ci.
Bleep bloop. Greetings Eelco Chaudron, 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 89 characters long (recommended limit is 79)
#218 FILE: .ci/linux-build.sh:151:
sudo bash -c "echo 2048 >
/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages"
WARNING: Line is 88 characters long (recommended limit is 79)
#314 FILE: .github/workflows/test.yml:116:
- { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, test_range:
"-100" }
WARNING: Line is 91 characters long (recommended limit is 79)
#315 FILE: .github/workflows/test.yml:117:
- { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, test_range:
"101-200" }
WARNING: Line is 88 characters long (recommended limit is 79)
#316 FILE: .github/workflows/test.yml:118:
- { compiler: gcc, testsuite: system-test-dpdk, dpdk: dpdk, test_range:
"201-" }
Lines checked: 363, Warnings: 4, Errors: 0
Please check this out. If you feel there has been an error, please email
acon...@redhat.com
Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v3 2/2] ci: Run the new check-system-dpdk tests as part of the ci.
This patch includes changes made earlier by David in the
ovs branch to cache the dpdk builds.
Co-authored-by: David Marchand
Signed-off-by: David Marchand
Signed-off-by: Eelco Chaudron
---
v2: Replaced 'sleep 1' with '' after consulting with Dumitru.
v3: No changes
Note that I ran the full GitHub ci 20x on the v1 patchset.
For v2, I did 10 runs of only the changed/fixed test. For v3,
I only did 1 run.
.ci/ci.sh | 12 ++-
.ci/dpdk-build.sh | 54 ++
.ci/dpdk-prepare.sh| 11 ++
.ci/linux-build.sh | 49 ++-
.github/workflows/test.yml | 80
Makefile.am|2 +
tests/system-ovn.at|2 +
7 files changed, 207 insertions(+), 3 deletions(-)
create mode 100755 .ci/dpdk-build.sh
create mode 100755 .ci/dpdk-prepare.sh
diff --git a/.ci/ci.sh b/.ci/ci.sh
index 90942bab6..10f11939c 100755
--- a/.ci/ci.sh
+++ b/.ci/ci.sh
@@ -16,6 +16,7 @@
OVN_PATH=${OVN_PATH:-$PWD}
OVS_PATH=${OVS_PATH:-$OVN_PATH/ovs}
+DPDK_PATH=${DPDK_PATH:-$OVN_PATH/dpdk-dir}
CONTAINER_CMD=${CONTAINER_CMD:-podman}
CONTAINER_WORKSPACE="/workspace"
CONTAINER_WORKDIR="/workspace/ovn-tmp"
@@ -80,6 +81,10 @@ function copy_sources_to_workdir() {
&& \
cp -a $CONTAINER_WORKSPACE/ovs/. $CONTAINER_WORKDIR/ovs \
&& \
+rm -rf $CONTAINER_WORKDIR/dpdk-dir \
+&& \
+cp -a $CONTAINER_WORKSPACE/dpdk-dir/. $CONTAINER_WORKDIR/dpdk-dir \
+&& \
git config --global --add safe.directory $CONTAINER_WORKDIR
"
}
@@ -95,7 +100,7 @@ function run_tests() {
cd $CONTAINER_WORKDIR \
&& \
ARCH=$ARCH CC=$CC LIBS=$LIBS OPTS=$OPTS TESTSUITE=$TESTSUITE \
-TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS \
+TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS DPDK=$DPDK \
./.ci/linux-build.sh
"
}
@@ -148,12 +153,17 @@ if [ "$ARCH" = "aarch64" ]; then
ASAN_OPTIONS="detect_leaks=0"
fi
+if [ -z "$DPDK" ]; then
+ mkdir -p "$DPDK_PATH"
+fi
+
CONTAINER_ID="$($CONTAINER_CMD run --privileged -d \
--pids-limit=-1 \
--env ASAN_OPTIONS=$ASAN_OPTIONS \
-v /lib/modules/$(uname -r):/lib/modules/$(uname -r):ro \
-v $OVN_PATH:$CONTAINER_WORKSPACE/ovn:Z \
-v $OVS_PATH:$CONTAINER_WORKSPACE/ovs:Z \
+-v $DPDK_PATH:$CONTAINER_WORKSPACE/dpdk-dir:Z \
$IMAGE_NAME)"
trap remove_container EXIT
diff --git a/.ci/dpdk-build.sh b/.ci/dpdk-build.sh
new file mode 100755
index 0..f44ac15b0
--- /dev/null
+++ b/.ci/dpdk-build.sh
@@ -0,0 +1,54 @@
+#!/bin/bash
+
+set -o errexit
+set -x
+
+function build_dpdk()
+{
+local VERSION_FILE="dpdk-dir/cached-version"
+local DPDK_VER=$1
+local DPDK_OPTS=""
+
+rm -rf dpdk-dir
+
+if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
+git clone --single-branch $DPDK_GIT dpdk-dir -b "${DPDK_VER##refs/*/}"
+pushd dpdk-dir
+git log -1 --oneline
+else
+wget https://fast.dpdk.org/rel/dpdk-$1.tar.xz
+tar xvf dpdk-$1.tar.xz > /dev/null
+DIR_NAME=$(tar -tf dpdk-$1.tar.xz | head -1 | cut -f1 -d"/")
+mv ${DIR_NAME} dpdk-dir
+pushd dpdk-dir
+fi
+
+# Switching to 'default' machine to make dpdk-dir cache usable on
+# different CPUs. We can't be sure that all CI machines are exactly same.
+DPDK_OPTS="$DPDK_OPTS -Dmachine=default"
+
+# Disable building DPDK unit tests. Not needed for OVS build or tests.
+DPDK_OPTS="$DPDK_OPTS -Dtests=false"
+
+# Disable DPDK developer mode, this results in less build checks and less
+# meson verbose outputs.
+DPDK_OPTS="$DPDK_OPTS -Ddeveloper_mode=disabled"
+
+# OVS compilation and the "ovn-system-dpdk" unit tests (run in the CI)
+# only depend on virtio/tap drivers.
+# We can disable all remaining drivers to save compilation time.
+DPDK_OPTS="$DPDK_OPTS -Denable_drivers=net/null,net/tap,net/virtio"
+
+# Install DPDK using prefix.
+DPDK_OPTS="$DPDK_OPTS --prefix=$(pwd)/build"
+
+meson $DPDK_OPTS build
+ninja -C build
+ninja -C build install
+
+echo "Installed DPDK in $(pwd)"
+popd
+echo "${DPDK_VER}" > ${VERSION_FILE}
+}
+
+build_dpdk $DPDK_VER
diff --git a/.ci/dpdk-prepare.sh b/.ci/dpdk-prepare.sh
new file mode 100755
index 0..f7e6215dd
--- /dev/null
+++ b/.ci/dpdk-prepare.sh
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -ev
+
+# Installing wheel separately because it may be needed to build some
+# of the packages during dependency backtracking and pip >= 22.0 will
+# abort backtracking on build failures:
+# https://github.com/pypa/pip/issues/10655
+pip3 install --disable-pip-version-check --user wheel
+pip3 install --disable-pip-version-check --user pyelftools
+pip3 install --user 'meson==0.53.2'
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 907a0dc6c..5a79a52da 100755
--- a/.ci/linux-build.sh
+++
[ovs-dev] [PATCH ovn v3 1/2] tests: add make check-system-dpdk to test suite.
Allow the ovn-system tests to run on the OVS-DPDK infrastructure,
i.e., DPDK ports and mbuf memory.
Co-authored-by: David Marchand
Signed-off-by: David Marchand
Signed-off-by: Eelco Chaudron
---
v2: No changes.
v3: Add 'dpdk-extra="--log-level=pmd.*:error --no-pci"' to test initialization.
tests/automake.mk | 23 +++-
tests/ofproto-macros.at|9 ++-
tests/system-dpdk-macros.at| 109
tests/system-dpdk-testsuite.at | 25 +
tests/system-ovn.at| 11
5 files changed, 167 insertions(+), 10 deletions(-)
create mode 100644 tests/system-dpdk-macros.at
create mode 100644 tests/system-dpdk-testsuite.at
diff --git a/tests/automake.mk b/tests/automake.mk
index fd5ee14af..eea0d00f4 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -2,11 +2,13 @@ EXTRA_DIST += \
$(COMMON_MACROS_AT) \
$(TESTSUITE_AT) \
$(SYSTEM_TESTSUITE_AT) \
+ $(SYSTEM_DPDK_TESTSUITE_AT) \
$(SYSTEM_KMOD_TESTSUITE_AT) \
$(SYSTEM_USERSPACE_TESTSUITE_AT) \
$(PERF_TESTSUITE_AT) \
$(MULTINODE_TESTSUITE_AT) \
$(TESTSUITE) \
+ $(SYSTEM_DPDK_TESTSUITE) \
$(SYSTEM_KMOD_TESTSUITE) \
$(SYSTEM_USERSPACE_TESTSUITE) \
$(PERF_TESTSUITE) \
@@ -44,20 +46,22 @@ TESTSUITE_AT = \
tests/ovn-ipsec.at \
tests/ovn-vif-plug.at
+SYSTEM_DPDK_TESTSUITE_AT = \
+ tests/system-dpdk-testsuite.at \
+ tests/system-dpdk-macros.at
+
SYSTEM_KMOD_TESTSUITE_AT = \
- tests/system-common-macros.at \
+ tests/system-kmod-macros.at \
tests/system-kmod-testsuite.at \
- tests/system-kmod-macros.at
+ tests/system-ovn-kmod.at
SYSTEM_USERSPACE_TESTSUITE_AT = \
tests/system-userspace-testsuite.at \
- tests/system-ovn.at \
tests/system-userspace-macros.at
SYSTEM_TESTSUITE_AT = \
tests/system-common-macros.at \
- tests/system-ovn.at \
- tests/system-ovn-kmod.at
+ tests/system-ovn.at
PERF_TESTSUITE_AT = \
tests/perf-testsuite.at \
@@ -73,6 +77,7 @@ check_SCRIPTS += tests/atlocal
TESTSUITE = $(srcdir)/tests/testsuite
TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
TESTSUITE_DIR = $(abs_top_builddir)/tests/testsuite.dir
+SYSTEM_DPDK_TESTSUITE = $(srcdir)/tests/system-dpdk-testsuite
SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite
SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite
PERF_TESTSUITE = $(srcdir)/tests/perf-testsuite
@@ -180,6 +185,10 @@ check-userspace-valgrind: all $(valgrind_wrappers)
$(check_DATA)
check-helgrind: all $(valgrind_wrappers) $(check_DATA)
-$(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true
VALGRIND='$(HELGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d
$(TESTSUITEFLAGS)
+check-system-dpdk: all
+ set $(SHELL) '$(SYSTEM_DPDK_TESTSUITE)' -C tests
AUTOTEST_PATH='$(AUTOTEST_PATH)'; \
+ $(SUDO) "$$@" $(TESTSUITEFLAGS) -j1 || (test X'$(RECHECK)' = Xyes &&
$(SUDO) "$$@" --recheck)
+
# Run kmod tests. Assume kernel modules has been installed or linked into the
kernel
check-kernel: all
set $(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests
AUTOTEST_PATH='$(AUTOTEST_PATH)'; \
@@ -232,6 +241,10 @@ $(TESTSUITE): package.m4 $(TESTSUITE_AT)
$(COMMON_MACROS_AT)
$(AM_V_at)mv $@.tmp $@
endif
+$(SYSTEM_DPDK_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT)
$(SYSTEM_DPDK_TESTSUITE_AT) $(COMMON_MACROS_AT)
+ $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
+ $(AM_V_at)mv $@.tmp $@
+
$(SYSTEM_KMOD_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT)
$(SYSTEM_KMOD_TESTSUITE_AT) $(COMMON_MACROS_AT)
$(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
$(AM_V_at)mv $@.tmp $@
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 2e0bbd20b..f4ebdafc7 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -218,11 +218,12 @@ m4_define([_OVS_VSWITCHD_START],
/ofproto|INFO|using datapath ID/d
/netdev_linux|INFO|.*device has unknown hardware address family/d
/ofproto|INFO|datapath ID changed to fedcba9876543210/d
-/dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d
/netlink_socket|INFO|netlink: could not enable listening to all nsid/d
/netdev: Flow API/d
/probe tc:/d
-/tc: Using policy/d']])
+/tc: Using policy/d
+/dpdk|INFO|/d
+/dpdk|WARN|/d']])
])
# OVS_VSWITCHD_START([vsctl-args], [vsctl-output], [=override],
@@ -247,7 +248,7 @@ m4_define([OVS_VSWITCHD_START],
AT_CHECK([add_of_br 0 $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2])
])
-# check_logs scans through all *.log files (except '*.log' and testsuite.log)
+# check_logs scans through all *.log files (except '*.log' and
'*testsuite.log')
# and reports all WARN, ERR, EMER log entries. User can add custom sed filters
# in $1.
m4_divert_push([PREPARE_TESTS])
@@ -255,7 +256,7 @@ check_logs () {
local logs
for log in
Re: [ovs-dev] Request adding long poll interval metrics to coverage metrics
On 16 Jun 2023, at 19:19, Aaron Conole wrote:
> Martin Kennelly writes:
>
>> Hey ovs community,
>>
>> I am a developer working on ovn-kubernetes and I want to programmatically
>> consume long poll information
>> i.e:
>> ovs|00211|timeval(handler25)|WARN|Unreasonably long 52388ms poll interval
>> (752ms user, 209ms system)
>>
>> This is currently exposed via journal logs but it's not practical to consume
>> it there programmatically and I was
>> hoping you could add it to coverage metrics.
>
> I think it could be useful. I do want to be careful about exposing
> these kinds of data in a way that could be misinterpreted. Already,
> that log in particular gets misinterpreted quite a bit, and RH gets
> customers claiming OVS is misbehaving when they've oversubscribed the
> system.
+1
> Mechanically, it would be pretty simple to do something like:
>
> ---
> diff --git a/lib/timeval.c b/lib/timeval.c
> index 193c7bab17..00e5f2a74d 100644
> --- a/lib/timeval.c
> +++ b/lib/timeval.c
> @@ -40,6 +40,7 @@
> #include "openvswitch/vlog.h"
>
> VLOG_DEFINE_THIS_MODULE(timeval);
> +COVERAGE_DEFINE(long_poll_interval);
>
> #if !defined(HAVE_CLOCK_GETTIME)
> typedef unsigned int clockid_t;
> @@ -645,6 +646,8 @@ log_poll_interval(long long int last_wakeup)
> struct rusage rusage;
>
> if (!getrusage_thread()) {
> +COVERAGE_INC(long_poll_interval);
> +
> VLOG_WARN("Unreasonably long %lldms poll interval"
>" (%lldms user, %lldms system)",
>interval,
> ---
>
> This would at least expose the coverage data via the coverage framework
> and it can be queried via ovs-appctl. Actually, the advantage here is
> that the coverage counter can track some details about X/sec over the
> last 5 seconds, minute, hour, in addition to the total, so we can see
> whether the condition is ongoing.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] debian/tests: Run system testsuites as autopkgtest.
The autopkgtests [0][1] are relevant in an upstream context because an Open vSwitch contributor may want to have a quick way of running the upstream system testsuites on recent Debian/Ubuntu releases in an automated and contained manner. During the Debian/Ubuntu/upstream package source sync work [2], a relatively naive autopkgtest was added. It had been around since Open vSwitch was initially packaged many years ago. Replace the autopkgtest with a test that runs all the upstream system testsuites instead. To run the autopkgtest, take a look at [1] for prerequisites then: ./boot.sh && ./configure && make debian-source autopkgtest \ --test-name kernel \ --env DEB_BUILD_OPTIONS="nocheck parallel=32 nodpdk" \ ../openvswitch_3.1.90-1.dsc \ -- qemu \ --cpus 32 \ --ram-size 8129 \ autopkgtest-mantic-amd64.img 0: https://wiki.debian.org/ContinuousIntegration/autopkgtest 1: https://packaging.ubuntu.com/html/auto-pkg-test.html 2: https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/396219.html Signed-off-by: Frode Nordahl --- debian/automake.mk | 26 - debian/rules | 8 ++ debian/tests/afxdp | 1 + debian/tests/afxdp-skip-tests.txt| 2 + debian/tests/control | 38 +-- debian/tests/dpdk| 46 +--- debian/tests/kernel | 1 + debian/tests/offloads| 1 + debian/tests/offloads-skip-tests.txt | 2 + debian/tests/openflow.py | 66 debian/tests/run-tests.sh| 151 +++ debian/tests/system-userspace| 1 + debian/tests/testlist.py | 74 + debian/tests/vanilla | 29 - 14 files changed, 296 insertions(+), 150 deletions(-) create mode 12 debian/tests/afxdp create mode 100644 debian/tests/afxdp-skip-tests.txt mode change 100755 => 12 debian/tests/dpdk create mode 12 debian/tests/kernel create mode 12 debian/tests/offloads create mode 100644 debian/tests/offloads-skip-tests.txt delete mode 100755 debian/tests/openflow.py create mode 100755 debian/tests/run-tests.sh create mode 12 debian/tests/system-userspace create mode 100755 debian/tests/testlist.py delete mode 100755 debian/tests/vanilla diff --git a/debian/automake.mk b/debian/automake.mk index 7b2afafae..89341fccc 100644 --- a/debian/automake.mk +++ b/debian/automake.mk @@ -61,10 +61,16 @@ EXTRA_DIST += \ debian/rules \ debian/source/format \ debian/source/lintian-overrides \ + debian/tests/afxdp \ + debian/tests/afxdp-skip-tests.txt \ debian/tests/control \ debian/tests/dpdk \ - debian/tests/openflow.py \ - debian/tests/vanilla \ + debian/tests/kernel \ + debian/tests/offloads \ + debian/tests/offloads-skip-tests.txt \ + debian/tests/run-tests.sh \ + debian/tests/system-userspace \ + debian/tests/testlist.py \ debian/watch check-debian-changelog-version: @@ -113,7 +119,6 @@ CLEANFILES += debian/control debian: debian/copyright debian/control .PHONY: debian - debian-deb: debian @if test X"$(srcdir)" != X"$(top_builddir)"; then \ echo "Debian packages should be built from $(abs_srcdir)/"; \ @@ -130,3 +135,18 @@ else $(AM_V_GEN) DEB_BUILD_OPTIONS="nocheck parallel=`nproc` nodpdk" \ fakeroot debian/rules binary endif + +debian-source: debian + @if test X"$(srcdir)" != X"$(top_builddir)"; then \ + echo "Debian packages should be built from $(abs_srcdir)/"; \ + exit 1; \ + fi + $(MAKE) distclean + $(update_deb_copyright) + $(update_deb_control) + $(AM_V_GEN) fakeroot debian/rules clean + tar -cvzf ../openvswitch_$(PACKAGE_VERSION).orig.tar.gz \ + --exclude .git \ + --transform 's,^\.,openvswitch-$(PACKAGE_VERSION),' \ + . + dpkg-source -b . diff --git a/debian/rules b/debian/rules index 28c249d07..f4c69651e 100755 --- a/debian/rules +++ b/debian/rules @@ -91,6 +91,14 @@ endif execute_before_dh_auto_clean: find . -name "*.pyc" -delete + if [ -f debian/control ]; then \ + cp debian/control debian/control.save; \ + fi + +execute_after_dh_auto_clean: + if [ -f debian/control.save ]; then \ + mv debian/control.save debian/control; \ + fi override_dh_auto_install: dh_auto_install --sourcedirectory=_debian diff --git a/debian/tests/afxdp b/debian/tests/afxdp new file mode 12 index 0..7080a3249 --- /dev/null +++ b/debian/tests/afxdp @@ -0,0 +1 @@ +run-tests.sh \ No newline at end of file diff --git
Re: [ovs-dev] [PATCH v15 0/4] Enhanced checksum support
On Thu, Jun 15, 2023 at 3:30 PM Ilya Maximets wrote: > > On 6/14/23 21:03, Mike Pattrick wrote: > > This patch set is a stripped down subset of the initial 17 patchset > > introduced > > by Flavio Leitner in 2021. > > > > The initial omnibus patchset was very complex and included a refactor, which > > stymied review and would have made backporting more complex. It also didn't > > resolve an ongoing issue with the DPDK netdev where we are currently > > incorrectly setting vhost flags, resulting in connectivity inturruptions > > when > > upgrading OVS to the full TSO patchset without restarting the attached > > virtual > > machines. > > > > The current 4-patch set is stripped down to include the following: > > > > 1. Public facing documentation on the phylosophy of how OVS handles > > checksums > > 2. A method for the user to easily check which checksums are offloaded per > > interface > > 3 & 4. Most checksuming activity delayed until a packet is about to be: > >- sent, or > >- transformed in such a way that checksumming wouldn't be offloadable > > regardless, such as encapsulation > > > > The main benefit of this set in its current state is an improved handling of > > checksums when encapsulating packets. But the set lays a groundwork for > > future > > work including improvements to how dpdk negotiates virtio vhost flags and > > more > > efficent handling of checksums in userspace with tso. > > > > This 4-patch reduced set has gone through a lot of revisions so far, > > including: > > > > v5 > > - Refactor was mostly removed, except for valid->good > > - Reset unsupported offload flags in send_prepare > > - Moved send_prepare from process_upcall to netdev_upcall > > > > v6 > > - Re-added tests that were incorrectly excluded from v5 > > > > v7 > > - David Marchand found an issue while upgrading OVS and not rebooting > > attached > >vhost VMs where we can't change flags post negotiation or check if they > > have > >already been set. > > - This was temporarily resolved by not setting the offending flags > > - This issue will be addressed in a more robust fasion if this patchset is > >applied. > > > > v8 > > - v7 patch 3 failed intel ci, moved some code from patch 4 to patch 3 > > > > v9 > > - Resolved a 10% performance hit in a DPDK-PVP workload that David Marchand > >found > > > > v10 > > - Large amount of formatting and grammar issues > > - Ported change to AVX512 code > > - ovs-appctl command removed > > - new fields added to netdev status including the information removed from > >ovs-appctl > > > > v11 > > - AVX512 change introduced a checksum bug due to an incorrectly sized > >datatype, caught by intel-ci and required a recent Xeon to reproduce. > > > > v12 > > - Updated documentation > > - Changed tso status field name > > - Excluded the combination of rte_flow offload, userspace tso, and > > RAW_ENCAP > >from simultaniously ocuring > > - Extended AVX512 support to IPv6 > > > > v13 > > - Re-added erroniously missing mutex annocations from v12 > > > > v14 > > - Rewrote a section of the documentation > > - Removed exclusion of the combination of rte_flow offload, userspace tso, > > and > >RAW_ENCAP > > - Added a TUNGETFEATURES check in netdev-linux.c for better support of > > early > >2.6 era kernels > > > > v15 > > - Moved some new code outside of mutex protection > > - Only check TUNGETFEATURES once > > - Respect FLOW_TNL_F_CSUM flag > > > > Mike Pattrick (4): > > Documentation: Document netdev offload. > > dpif-netdev: Show netdev offloading flags. > > userspace: Enable IP checksum offloading by default. > > userspace: Enable L4 checksum offloading by default. > > > > Documentation/automake.mk | 1 + > > Documentation/topics/index.rst| 1 + > > .../topics/userspace-checksum-offloading.rst | 96 +++ > > lib/conntrack.c | 30 +- > > lib/dp-packet.c | 40 +++ > > lib/dp-packet.h | 140 +- > > lib/dpif-netdev-extract-avx512.c | 57 > > lib/dpif-netdev.c | 2 + > > lib/flow.c| 38 ++- > > lib/ipf.c | 11 +- > > lib/netdev-dpdk.c | 230 +++- > > lib/netdev-dummy.c| 22 ++ > > lib/netdev-linux.c| 258 +- > > lib/netdev-native-tnl.c | 53 ++-- > > lib/netdev.c | 81 +++--- > > lib/odp-execute-avx512.c | 108 +--- > > lib/odp-execute.c | 21 +- > > lib/packets.c | 209 +++--- > > lib/packets.h | 3 + > > tests/dpif-netdev.at |
