Re: [ovs-dev] [PATCH v1] system-dpdk.at: Add ignore warning for context switches in CI.
Hi IIlya, > -Original Message- > From: Ilya Maximets > Sent: Saturday, March 12, 2022 3:26 AM > To: Amber, Kumar ; ovs-dev@openvswitch.org > Cc: i.maxim...@ovn.org; Phelan, Michael > Subject: Re: [ovs-dev] [PATCH v1] system-dpdk.at: Add ignore warning for > context switches in CI. > > On 3/2/22 10:42, Amber, Kumar wrote: > > Hi llya, > > s/LL/IL/ > > > > >> -Original Message- > >> From: Ilya Maximets > >> Sent: Tuesday, March 1, 2022 1:50 AM > >> To: Amber, Kumar ; ovs-dev@openvswitch.org > >> Cc: i.maxim...@ovn.org; Phelan, Michael > >> Subject: Re: [ovs-dev] [PATCH v1] system-dpdk.at: Add ignore warning > >> for context switches in CI. > >> > >> On 2/28/22 19:23, Amber, Kumar wrote: > >>> Hi IIya, > >>> > >>> Thanks for the comments > >>> > > > Hi. These warnings are more about long poll intervals and that > should not happen during tests. Which test is causing this problem? > > Long poll intervals usually indicate a problem with the test itself. > Usual suspect is poor pinning of PMD threads. > > Best regards, Ilya Maximets. > >>> > >>> Fuzzy testing for MFEX , I agree but the cores of the test were > >>> chosen such > >> that the cores are always available on any available Setup. > >>> Thus, this is a side-effect, but it guarantees that the test won't > >>> fail on most of the setups. Suppressing warnings is A good tradeoff. > >> > >> Why can't we use dummy numa for this test? This will avoid actual > >> pinning of the threads keeping them available for kernel scheduling. > >> We're not testing performance here after all. > >> > >> Best regards, Ilya Maximets. > > > > Based on the discussion and feedback, we have tried to remove the complexity > of the DPIF itself from testing MFEX. > > To Make it simpler to understand and introduce testing more complex traffic > type and improvements to the testing of mfex. > > > > Please have a look at the patch sets and feedback is always great to improve > testing. > > http://patchwork.ozlabs.org/project/openvswitch/list/?series=287947 > > Sure, I'll take a look. > But on a quick glance I see that these system tests are not removed by the > mentioned patch set. So, I'm not sure how it is related to the current > discussion. > > Best regards, Ilya Maiximets. Yes, I totally agree with the Performance aspect that we ae not testing it and that’s why we are also trying to find ways to improve testing. Until then it's important for us to enable this test in AVX512 CI and that why it’s a quick fix to enable the test. Thanks a lot for the Feedback will consider on the other patch and will produce something more concrete. Till then if this patch can be merged, the test will be running in CI once we have something better which is accepted to community will remove the test altogether. Regards Amber ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] system-dpdk: Fix mfex autovalidator tests.
On 3/3/22 11:40, Ferriter, Cian wrote:
>
>
>> -Original Message-
>> From: Amber, Kumar
>> Sent: Tuesday 1 March 2022 15:08
>> To: ovs-dev@openvswitch.org
>> Cc: Ferriter, Cian ; echau...@redhat.com; Stokes,
>> Ian ;
>> Van Haaren, Harry ; Amber, Kumar
>>
>> Subject: [PATCH v2] system-dpdk: Fix mfex autovalidator tests.
>>
>> AVX512 DPIF must be active in order for the MFEX AutoValidator to be
>> executed.
>> If the DPIF-AVX512 is not available, the unit test is skipped, as the
>> scalar DPIF does not use the MFEX function-pointer based optimizations.
>>
>> Fixes: 50be6715c0 ("test/sytem-dpdk: Add unit test for mfex autovalidator")
>> Suggested-by: Cian Ferriter
>> Signed-off-by: Kumar Amber
>
> Hi Amber,
>
> Thanks for addressing the changes.
>
> This patch LGTM. I have also tested that the unit tests are being run by
> introducing an error in the MFEX C code and running the unit tests. The
> errors are caught and the MFEX autovalidator tests fail.
>
> Acked-by: Cian Ferriter
Thanks! Applied and backported down to 2.16.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] ofp-prop: Silence the 'may be uninitialized' warning.
On 3/4/22 12:48, Eelco Chaudron wrote: > > > On 28 Feb 2022, at 2:56, Mike Pattrick wrote: > >> GCC 11.2.1-2.2 emits a false-positive warnings like: >> >> lib/ofp-packet.c: In function 'ofputil_decode_packet_in': >> lib/ofp-packet.c:155:25: warning: 'reason' may be used >> uninitialized [-Wmaybe-uninitialized] >> lib/ofp-packet.c: In function 'ofputil_decode_packet_in_private': >> lib/ofp-packet.c:886:27: warning: 'value' may be used >> uninitialized [-Wmaybe-uninitialized] >> >> Modifying callers of ofpprop_parse_* functions to always check >> the return value before using the value from these functions. >> >> Signed-off-by: Mike Pattrick > > Changes look good to me. > > Acked-by: Eelco Chaudron Thanks! Applied and backported down to 2.13. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tests: Ignore log about failing to set NETLINK_EXT_ACK.
On 2/25/22 16:38, Dumitru Ceara wrote:
> Since 4a6a4734622e ("netlink-socket: Log extack error messages in
> netlink transactions."), tests fail on older systems that don't support
> NETLINK_EXT_ACK. It's not really an issue, so we can just ignore the
> log.
>
> CC: Paolo Valerio
> CC: Eelco Chaudron
> Signed-off-by: Dumitru Ceara
> ---
Thanks! Applied to master and 2.17.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-cluster.at: Avoid test failures due to different hashing.
On 3/11/22 11:28, Eelco Chaudron wrote: > > > On 24 Feb 2022, at 20:36, Ilya Maximets wrote: > >> Depending on compiler flags and CPU architecture different hash >> function are used. That impacts the order of tables and columns >> in database representation making ovsdb report different columns >> in the warning about ephemeral-to-persistent conversion. >> >> Stripping out changing parts of the message to avoid the issue. >> >> Signed-off-by: Ilya Maximets >> --- > > Changes look good to me, tested by running make check-ovsdb-cluster a couple > of times. > > Acked-by: Eelco Chaudron Thanks! Applied and backported down to 2.13 for easier testing. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Simplify atomic function pointer stores
On 2/21/22 17:01, Cian Ferriter wrote: > The same pattern for atomic stores and initialization was used for the > DPIF and MFEX function pointers declared in struct dp_netdev_pmd_thread. > Simplify this pattern for all stores to 'miniflow_extract_opt' and > 'netdev_input_func'. > > Also replace the first store to 'miniflow_extract_opt' which was a > atomic_store_relaxed() with atomic_init(). > > Signed-off-by: Cian Ferriter > --- Applied. Thanks! Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 0/2] MFEX datapath independent Tests
On 2/25/22 12:46, Kumar Amber wrote: > The Patchset Introduces creating of varied lenght packets > for testing MFEX AVX512 implementations as well as Introduces > unit-tests which are independet from ovs-datapath. This also > allows us to test a lot more scenarios testing only MFEX. Hi. First of all, thanks for working on these tests, we really need to improve them. However, beside the fact that test-mfex.c fails to build, we already have this kind of a test. It's called test-flows.c. And we already have a script to generate various packets for that test called flowgen.py. We actually have 2 tests of that kind, the second one is used for fuzzing with the OSS-Fuzz project and located at ./tests/oss-fuzz/miniflow_target.c The main problem with current code, as I many times said in different threads, that we're not testing avx512 version of the miniflow_extract with the same packets we're testing the generic implementation, and we're not testing generic implementation with the traffic we're testing avx512 implementation with. Also we're not testing avx512 code with all the traffic generated in a large variety of common unit tests. This patch set creates a test independent not from the datapath, but from ovs-vswitchd. That might be OK on it's own, but it doesn't really improve the testing situation as we're still not testing datapath-specific avx512 implementations with the flows and packets we're testing generic code with in other existing tests. tests/test-mfex.c seems just a replacement for tests we have in system-dpdk.at, but former are not removed in this patch set, so I'm confused. I don't see big advantages of tests/test-mfex.c against existing tests. There are some, but not very important. Another point is that there is no place for randomness in unit tests. Please, analyze the code and come up with a pack of specific test cases that covers normal paths (positive and negative) and all important corner cases. If there are specific problems you faced before, add regression tests for these specific cases. Tests that may or may not fail are not good unit tests and probably are not unit tests at all. For the "datapath independent" testing, please, make the main code independent from the datapath, so it can be tested with existing test tools (test-flows.c, miniflow_target.c, other unit and system tests). There is no logical dependency between flow parsing and the datapath implementation, it should not be very hard to decouple them. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] system-dpdk.at: Add ignore warning for context switches in CI.
On 3/2/22 10:42, Amber, Kumar wrote: > Hi llya, s/LL/IL/ > >> -Original Message- >> From: Ilya Maximets >> Sent: Tuesday, March 1, 2022 1:50 AM >> To: Amber, Kumar ; ovs-dev@openvswitch.org >> Cc: i.maxim...@ovn.org; Phelan, Michael >> Subject: Re: [ovs-dev] [PATCH v1] system-dpdk.at: Add ignore warning for >> context switches in CI. >> >> On 2/28/22 19:23, Amber, Kumar wrote: >>> Hi IIya, >>> >>> Thanks for the comments >>> Hi. These warnings are more about long poll intervals and that should not happen during tests. Which test is causing this problem? Long poll intervals usually indicate a problem with the test itself. Usual suspect is poor pinning of PMD threads. Best regards, Ilya Maximets. >>> >>> Fuzzy testing for MFEX , I agree but the cores of the test were chosen such >> that the cores are always available on any available Setup. >>> Thus, this is a side-effect, but it guarantees that the test won't >>> fail on most of the setups. Suppressing warnings is A good tradeoff. >> >> Why can't we use dummy numa for this test? This will avoid actual pinning of >> the threads keeping them available for kernel scheduling. >> We're not testing performance here after all. >> >> Best regards, Ilya Maximets. > > Based on the discussion and feedback, we have tried to remove the complexity > of the DPIF itself from testing MFEX. > To Make it simpler to understand and introduce testing more complex traffic > type and improvements to the testing of mfex. > > Please have a look at the patch sets and feedback is always great to improve > testing. > http://patchwork.ozlabs.org/project/openvswitch/list/?series=287947 Sure, I'll take a look. But on a quick glance I see that these system tests are not removed by the mentioned patch set. So, I'm not sure how it is related to the current discussion. Best regards, Ilya Maiximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto-dpif-xlate: Fix NULL pointer dereference in xlate_normal()
Considering the following flows:
ovs-ofctl dump-flows br0
cookie=0x0, duration=2431.944s, table=0, n_packets=0, n_bytes=0, priority=0
actions=NORMAL
and assuming a packet originated from packet-out in this way:
ovs-ofctl packet-out br0
"in_port=controller,packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008,
action=ct(table=0)"
If in_port is OFPP_NONE or OFPP_CONTROLLER, this leads to a
NULL pointer (xport) dereference in xlate_normal().
Fix it by checking the xport pointer validity while deciding whether
it is a candidate for mac learning or not.
Signed-off-by: Paolo Valerio
---
ofproto/ofproto-dpif-xlate.c |2 +-
tests/ofproto-dpif.at| 29 +
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index cc9c1c628..d3b9648c2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3032,7 +3032,7 @@ xlate_normal(struct xlate_ctx *ctx)
bool is_grat_arp = is_gratuitous_arp(flow, wc);
if (ctx->xin->allow_side_effects
&& flow->packet_type == htonl(PT_ETH)
-&& in_port->pt_mode != NETDEV_PT_LEGACY_L3
+&& in_port && in_port->pt_mode != NETDEV_PT_LEGACY_L3
) {
update_learning_table(ctx, in_xbundle, flow->dl_src, vlan,
is_grat_arp);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 912f3a1da..6a2bce336 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5578,6 +5578,35 @@ OVS_WAIT_UNTIL([check_flows], [ovs dump-flows br0])
OVS_VSWITCHD_STOP
AT_CLEANUP
+# Checks for regression against a bug in which OVS crashed
+# with in_port=OFPP_NONE or in_port=OFPP_CONTROLLER and
+# recirculation is involved.
+AT_SETUP([ofproto-dpif - packet-out recirculation with OFPP_NONE and
OFPP_CONTROLLER])
+OVS_VSWITCHD_START
+add_of_ports br0 1 2
+
+AT_DATA([flows.txt], [dnl
+table=0 ip actions=mod_dl_dst:83:83:83:83:83:83,ct(table=1)
+table=1 ip actions=ct(commit),normal
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+packet=0010203040500800451c40110a0200350008
+AT_CHECK([ovs-ofctl packet-out br0 "in_port=none,packet=$packet
actions=table"])
+AT_CHECK([ovs-ofctl packet-out br0 "in_port=controller,packet=$packet
actions=table"])
+
+# Dumps out the flow table, extracts the number of packets that have gone
+# through the (single) flow in table 1, and returns success if it's exactly 2.
+check_flows () {
+n=$(ovs-ofctl dump-flows br0 table=1 | sed -n
's/.*n_packets=\([[0-9]]\{1,\}\).*/\1/p')
+echo "n_packets=$n"
+test "$n" = 2
+}
+OVS_WAIT_UNTIL([check_flows], [ovs dump-flows br0])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - debug_slow action])
OVS_VSWITCHD_START
add_of_ports br0 1 2 3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-build] |fail| pw1600225 [ovs-dev, branch-2.15] dpdk: Use DPDK 20.11.4 release
> Test-Label: intel-ovs-compilation > Test-Status: fail > http://patchwork.ozlabs.org/api/patches/1600225/ > > AVX-512_compilation: failed > DPLCS Test: fail > DPIF Test: fail > MFEX Test: fail > Errors in DPCLS test: >> Good hash: 121684652 len: 72 Test hash:0 len:0 >> 2022-03-02T16:05:44.579Z|00034|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> for avx512_ipv4_udp failed in pkt 15, disabling. >> 2022-03-02T16:05:44.579Z|00035|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> failure details: >> MFEX autovalidator pkt 15 >> Good hash: 121684652 len: 72 Test hash:0 len:0 >> 2022-03-02T16:05:44.579Z|00036|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> for avx512_ipv4_udp failed in pkt 19, disabling. >> 2022-03-02T16:05:44.579Z|00037|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> failure details: >> MFEX autovalidator pkt 19 >> Good hash: 121684652 len: 72 Test hash:0 len:0 >> 2022-03-02T16:05:44.579Z|00038|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> for avx512_ipv4_udp failed in pkt 21, disabling. >> 2022-03-02T16:05:44.579Z|00039|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> failure details: >> MFEX autovalidator pkt 21 >> Good hash: 121684652 len: 72 Test hash:0 len:0 >> 2022-03-02T16:05:44.579Z|00040|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> for avx512_ipv4_udp failed in pkt 25, disabling. >> 2022-03-02T16:05:44.579Z|00041|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> failure details: >> MFEX autovalidator pkt 25 >> Good hash: 121684652 len: 72 Test hash:0 len:0 >> 2022-03-02T16:05:44.579Z|00042|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> for avx512_ipv4_udp failed in pkt 27, disabling. >> 2022-03-02T16:05:44.579Z|00043|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> failure details: >> MFEX autovalidator pkt 27 >> Good hash: 121684652 len: 72 Test hash:0 len:0 >> 2022-03-02T16:05:44.579Z|00044|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> for avx512_ipv4_udp failed in pkt 31, disabling. >> 2022-03-02T16:05:44.579Z|00045|dpif_netdev_extract(pmd-c21/id:101)|ERR|Autovalidation >> failure details: >> MFEX autovalidator pkt 31 Hey, Michael, others. Above errors are not part of the current OVS code, nor the patch this report is for. But I see that code in the other patch that has no test report from the intel-ovs-compilation: https://patchwork.ozlabs.org/project/openvswitch/patch/20220225115654.521213-3-kumar.am...@intel.com/ Seems like report was sent to a wrong patch. The patch number at the top of the mail is correct though, so I'm not sure what is going on. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-21.12 0/2] Release patches for v21.12.1.
On 3/10/22 21:49, Mark Michelson wrote: > > Mark Michelson (2): > Set release date for 21.12.1. > Prepare for 21.12.2. > > NEWS | 6 +- > configure.ac | 2 +- > debian/changelog | 8 +++- > 3 files changed, 13 insertions(+), 3 deletions(-) > I guess, it's fine to release this yesterday. :) So, Acked-by: Ilya Maximets ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.03 2/2] Prepare for 22.03.1.
On 3/10/22 21:49, Mark Michelson wrote: > Signed-off-by: Mark Michelson > --- > NEWS | 3 +++ > configure.ac | 2 +- > debian/changelog | 6 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 0f8e068b1..15fa545d2 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,6 @@ > +OVN v22.03.1 - xx xxx > +-- > + > OVN v22.03.0 - 11 Mar 2022 > -- >- Refactor CoPP commands introducing a unique name index in CoPP NB table. > diff --git a/configure.ac b/configure.ac > index 283381b4e..70f86e1f5 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -13,7 +13,7 @@ > # limitations under the License. > > AC_PREREQ(2.63) > -AC_INIT(ovn, 22.03.0, b...@openvswitch.org) > +AC_INIT(ovn, 22.03.1, b...@openvswitch.org) > AC_CONFIG_MACRO_DIR([m4]) > AC_CONFIG_AUX_DIR([build-aux]) > AC_CONFIG_HEADERS([config.h]) > diff --git a/debian/changelog b/debian/changelog > index a26420b7a..a22ba6e91 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -1,3 +1,9 @@ > +OVN (22.03.1-1) unstable; urgency=low > + [ OVN team ] > + * New upstream version > + > + -- OVN team Thu, 10 Mar 2022 15:47:41 -0500 This date should be advanced too. Other than that: Acked-by: Ilya Maximets > + > ovn (22.03.0-1) unstable; urgency=low > > * New upstream version ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.03 1/2] Set release date for 22.03.0.
On 3/10/22 21:49, Mark Michelson wrote: > Signed-off-by: Mark Michelson > --- > NEWS | 2 +- > debian/changelog | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 9f3ce3cf3..0f8e068b1 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,4 +1,4 @@ > -OVN v22.03.0 - XX XXX > +OVN v22.03.0 - 11 Mar 2022 > -- >- Refactor CoPP commands introducing a unique name index in CoPP NB table. > Add following new CoPP commands to manage CoPP table: > diff --git a/debian/changelog b/debian/changelog > index fe0e7f488..a26420b7a 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ ovn (22.03.0-1) unstable; urgency=low > > * New upstream version > > - -- OVN team Thu, 24 Feb 2022 16:55:45 -0500 > + -- OVN team Thu, 10 Mar 2022 15:47:41 -0500 I think, we need to have "Fri, 11 Mar" in order to have the same date here and in the NEWS. Other than that: Acked-by: Ilya Maximets > > ovn (21.12.0-1) unstable; urgency=low > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 4/5] dpif-netdev: Fix PMD auto load balance with pmd-rxq-isolate.
On Fri, Mar 11, 2022 at 12:06 PM Kevin Traynor wrote:
>
> There are currently some checks for cross-numa polling cases to
> ensure that they won't effect the accuracy of the PMD ALB.
>
> If an rxq is pinned to a pmd core by the user it will not be
> reassigned by OVS, so even if it is non-local numa polled it
> will not impact PMD ALB accuracy.
>
> To establish this, a check was made on whether the pmd core was
> isolated or not. However, since other_config:pmd-rxq-isolate was
> introduced, rxqs may be pinned but the pmd core not isolated.
>
> It means that by setting pmd-rxq-isolate=false and doing non-local
> numa pinning, PMD ALB may not run where it should.
>
> If the core is isolated we can skip individual rxq checks but if not,
> we should check the individual rxqs for pinning before we disallow
> PMD ALB.
>
> Also, update function comments to make it's operation clearer.
>
> Fixes: 6193e03267c1 ("dpif-netdev: Allow pin rxq and non-isolate PMD.")
>
> Signed-off-by: Kevin Traynor
> ---
Acked-by: Mike Pattrick
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 5/5] alb.ut: Add tests for cross-numa polling.
PMD auto load balance currently only operates when the polling pmd core will not change numa after reassignment. Add a unit test for this. Signed-off-by: Kevin Traynor Acked-by: Mike Pattrick --- tests/alb.at | 46 ++ 1 file changed, 46 insertions(+) diff --git a/tests/alb.at b/tests/alb.at index 2bef06f39..0036bd1f2 100644 --- a/tests/alb.at +++ b/tests/alb.at @@ -97,4 +97,50 @@ OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ALB - cross-numa]) +OVS_VSWITCHD_START([add-port br0 p0 \ +-- set Interface p0 type=dummy-pmd options:n_rxq=4 \ +-- set Interface p0 options:numa_id=0 \ +-- set Open_vSwitch . other_config:pmd-cpu-mask=0x3 \ +-- set open_vswitch . other_config:pmd-rxq-assign=group \ +-- set open_vswitch . other_config:pmd-rxq-isolate=false \ +-- set open_vswitch . other_config:pmd-auto-lb="true" \ +-- set open_vswitch . other_config:pmd-auto-lb-load-threshold=0], + [], [], [--dummy-numa 1,2,1,2]) +OVS_WAIT_UNTIL([grep "PMD auto load balance is enabled" ovs-vswitchd.log]) +AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg]) + +# no pinned rxqs - cross-numa pmd could change +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance detected cross-numa polling"]) + +# all pinned rxqs - cross-numa pmd will not change +AT_CHECK([ovs-vsctl set Interface p0 other_config:pmd-rxq-affinity='0:0,1:0,2:1,3:1']) +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "Variance improvement 0%."]) + +# mix of pinned (non-isolated) and non-pinned rxqs - cross-numa pmd could change +AT_CHECK([ovs-vsctl remove Interface p0 other_config pmd-rxq-affinity]) +AT_CHECK([ovs-vsctl set Interface p0 other_config:pmd-rxq-affinity='0:0,1:0,2:1']) +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance detected cross-numa polling"]) + +# mix of pinned (isolated) and non-pinned rxqs - cross-numa pmd could change +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf]) +AT_CHECK([ovs-vsctl set Interface p0 options:n_rxq=6]) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-isolate=true]) +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance detected cross-numa polling"]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ALB - PMD/RxQ assignment type]) OVS_VSWITCHD_START([add-port br0 p0 \ -- 2.34.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 4/5] dpif-netdev: Fix PMD auto load balance with pmd-rxq-isolate.
There are currently some checks for cross-numa polling cases to
ensure that they won't effect the accuracy of the PMD ALB.
If an rxq is pinned to a pmd core by the user it will not be
reassigned by OVS, so even if it is non-local numa polled it
will not impact PMD ALB accuracy.
To establish this, a check was made on whether the pmd core was
isolated or not. However, since other_config:pmd-rxq-isolate was
introduced, rxqs may be pinned but the pmd core not isolated.
It means that by setting pmd-rxq-isolate=false and doing non-local
numa pinning, PMD ALB may not run where it should.
If the core is isolated we can skip individual rxq checks but if not,
we should check the individual rxqs for pinning before we disallow
PMD ALB.
Also, update function comments to make it's operation clearer.
Fixes: 6193e03267c1 ("dpif-netdev: Allow pin rxq and non-isolate PMD.")
Signed-off-by: Kevin Traynor
---
lib/dpif-netdev.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9ae488d6e..dce4a6d42 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5685,4 +5685,6 @@ sched_numa_list_put_in_place(struct sched_numa_list
*numa_list)
}
+/* Returns 'true' if OVS rxq scheduling algorithm assigned any unpinned rxq to
+ * a pmd core on a non-local numa node. */
static bool
sched_numa_list_cross_numa_polling(struct sched_numa_list *numa_list)
@@ -5690,16 +5692,19 @@ sched_numa_list_cross_numa_polling(struct
sched_numa_list *numa_list)
struct sched_numa *numa;
-/* For each numa */
HMAP_FOR_EACH (numa, node, _list->numas) {
-/* For each pmd */
for (int i = 0; i < numa->n_pmds; i++) {
struct sched_pmd *sched_pmd;
sched_pmd = >pmds[i];
-/* For each rxq. */
+if (sched_pmd->isolated) {
+/* All rxqs on this pmd core are pinned. */
+continue;
+}
for (unsigned k = 0; k < sched_pmd->n_rxq; k++) {
struct dp_netdev_rxq *rxq = sched_pmd->rxqs[k];
-
-if (!sched_pmd->isolated &&
+/* Check if the rxq is not pinned to a specific pmd core by the
+ * user AND the pmd core that OVS assigned is non-local to the
+ * rxq port. */
+if (rxq->core_id == OVS_CORE_UNSPEC &&
rxq->pmd->numa_id !=
netdev_get_numa_id(rxq->port->netdev)) {
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 2/5] dpif-netdev: Fix non-local numa selection.
In the event of no pmd cores available on the local numa for
an rxq to be assigned to, a pmd core from a non-local numa is
selected.
If there are more than one non-local numas with pmd cores they
are RR through and checked if they have non-isolated pmds.
When successfully finding a non-local numa with available pmds
for an rxq, that numa was not being stored. It meant if a similar
situation occurred for a subsequent rxq, the same numa would be
selected again.
Store the last numa used when successfully finding a non-local numa
with available pmds, so the numa RR state is kept for subsequent rxqs.
Fixes: f577c2d046b2 ("dpif-netdev: Rework rxq scheduling code.")
Signed-off-by: Kevin Traynor
Acked-by: Mike Pattrick
---
lib/dpif-netdev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8a3047439..9ae488d6e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6001,8 +6001,8 @@ sched_numa_list_schedule(struct sched_numa_list
*numa_list,
for (int j = 0; j < n_numa; j++) {
numa = sched_numa_list_next(numa_list, last_cross_numa);
-if (sched_numa_noniso_pmd_count(numa)) {
-break;
-}
last_cross_numa = numa;
+if (sched_numa_noniso_pmd_count(numa)) {
+break;
+}
numa = NULL;
}
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 3/5] pmd.at: Add tests for multi non-local numa pmds.
Ensure that if there are no local numa pmd cores
available that pmd cores from all other non-local
numas will be used.
Signed-off-by: Kevin Traynor
---
tests/pmd.at | 62 +++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/tests/pmd.at b/tests/pmd.at
index a2f9d34a2..a5b0a2523 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -10,4 +10,11 @@ parse_pmd_rxq_show () {
}
+# Given the output of `ovs-appctl dpif-netdev/pmd-rxq-show`,
+# prints the first rxq on each pmd in the form:
+# 'port:' port_name 'queue_id:' rxq_id
+parse_pmd_rxq_show_first_rxq () {
+ awk '/isolated/ {print $4, $5, $6, $7}' | sort
+}
+
# Given the output of `ovs-appctl dpif-netdev/pmd-rxq-show`,
# and with queues for each core on one line, prints the rxqs
@@ -200,5 +207,5 @@ OVS_VSWITCHD_STOP
AT_CLEANUP
-AT_SETUP([PMD - pmd-cpu-mask - NUMA])
+AT_SETUP([PMD - pmd-cpu-mask - dual NUMA])
OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy-pmd
options:n_rxq=8 options:numa_id=1 -- set Open_vSwitch .
other_config:pmd-cpu-mask=1],
[], [], [--dummy-numa 1,1,0,0])
@@ -360,4 +367,57 @@ OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([PMD - pmd-cpu-mask - multi NUMA])
+OVS_VSWITCHD_START([add-port br0 p0 \
+-- set Interface p0 type=dummy-pmd options:n_rxq=4 \
+-- set Interface p0 options:numa_id=0 \
+-- set Open_vSwitch . other_config:pmd-cpu-mask=0xf \
+-- set open_vswitch . other_config:pmd-rxq-assign=cycles],
+ [], [], [--dummy-numa 1,2,1,2])
+
+TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1))
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=group])
+
+OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx
queue assignment using group algorithm"])
+OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available
(non-isolated) pmd thread on numa node 0."])
+
+# check all pmds from both non-local numas are assigned an rxq
+AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | awk '/false$/ { printf("%s\t",
$0); next } 1' | parse_pmd_rxq_show_first_rxq], [0], [dnl
+port: p0 queue-id: 0
+port: p0 queue-id: 1
+port: p0 queue-id: 2
+port: p0 queue-id: 3
+])
+
+TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1))
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=cycles])
+
+OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx
queue assignment using cycles algorithm"])
+OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available
(non-isolated) pmd thread on numa node 0."])
+
+# check all pmds from both non-local numas are assigned an rxq
+AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | awk '/false$/ { printf("%s\t",
$0); next } 1' | parse_pmd_rxq_show_first_rxq], [0], [dnl
+port: p0 queue-id: 0
+port: p0 queue-id: 1
+port: p0 queue-id: 2
+port: p0 queue-id: 3
+])
+
+TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1))
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=roundrobin])
+
+OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx
queue assignment using roundrobin algorithm"])
+OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available
(non-isolated) pmd thread on numa node 0."])
+
+# check all pmds from both non-local numas are assigned an rxq
+AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | awk '/false$/ { printf("%s\t",
$0); next } 1' | parse_pmd_rxq_show_first_rxq], [0], [dnl
+port: p0 queue-id: 0
+port: p0 queue-id: 1
+port: p0 queue-id: 2
+port: p0 queue-id: 3
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([PMD - stats])
OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 ofport_request=7
type=dummy-pmd options:n_rxq=4],
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 1/5] dpif-netdev: Fix typo in function name.
Rename pmd_reblance_dry_run_needed() to
pmd_rebalance_dry_run_needed().
Fixes: a83a406096e9 ("dpif-netdev: Sync PMD ALB state with user commands.")
Signed-off-by: Kevin Traynor
Acked-by: Mike Pattrick
---
lib/dpif-netdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9f35713ef..8a3047439 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6112,5 +6112,5 @@ sched_numa_list_variance(struct sched_numa_list
*numa_list)
*/
static bool
-pmd_reblance_dry_run_needed(struct dp_netdev *dp)
+pmd_rebalance_dry_run_needed(struct dp_netdev *dp)
OVS_REQ_RDLOCK(dp->port_rwlock)
{
@@ -6683,5 +6683,5 @@ dpif_netdev_run(struct dpif *dpif)
!dp_netdev_is_reconf_required(dp) &&
!ports_require_restart(dp) &&
-pmd_reblance_dry_run_needed(dp) &&
+pmd_rebalance_dry_run_needed(dp) &&
pmd_rebalance_dry_run(dp)) {
VLOG_INFO("PMD auto load balance dry run. "
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 0/5] ALB/Rxq scheduling cross-numa polling fixes.
Some fixes found in the area of cross-numa polling during recent code inspection and testing. v4: - Reworked pmd.at UT (3/5) to account for numa hash map v3: - Add additional unit test (new patch 3/5) for rxq scheduling to ensure that all non-local numas are used. - Other patches unchanged v2: - Updated comments in 3/4 to make the fn operation clearer. - Added Acks on other patches GHA: https://github.com/kevintraynor/ovs/actions/runs/1970010319 Kevin Traynor (5): dpif-netdev: Fix typo in function name. dpif-netdev: Fix non-local numa selection. pmd.at: Add tests for multi non-local numa pmds. dpif-netdev: Fix PMD auto load balance with pmd-rxq-isolate. alb.ut: Add tests for cross-numa polling. lib/dpif-netdev.c | 25 +++ tests/alb.at | 46 +++ tests/pmd.at | 62 ++- 3 files changed, 122 insertions(+), 11 deletions(-) -- 2.34.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 18/18] python: add unit tests for filtering engine
Add unit test for OFFilter class. Acked-by: Eelco Chaudron Signed-off-by: Adrian Moreno --- python/automake.mk | 1 + python/ovs/tests/test_filter.py | 221 2 files changed, 222 insertions(+) create mode 100644 python/ovs/tests/test_filter.py diff --git a/python/automake.mk b/python/automake.mk index b2e7f6008..85ed8e715 100644 --- a/python/automake.mk +++ b/python/automake.mk @@ -55,6 +55,7 @@ ovs_pyfiles = \ ovs_pytests = \ python/ovs/tests/test_decoders.py \ + python/ovs/tests/test_filter.py \ python/ovs/tests/test_kv.py \ python/ovs/tests/test_list.py \ python/ovs/tests/test_odp.py \ diff --git a/python/ovs/tests/test_filter.py b/python/ovs/tests/test_filter.py new file mode 100644 index 0..823ad2b48 --- /dev/null +++ b/python/ovs/tests/test_filter.py @@ -0,0 +1,221 @@ +import pytest + +from ovs.flows.filter import OFFilter +from ovs.flows.ofp import OFPFlow +from ovs.flows.odp import ODPFlow + + +@pytest.mark.parametrize( +"expr,flow,expected,match", +[ +( +"nw_src=192.168.1.1 && tcp_dst=80", +OFPFlow( +"nw_src=192.168.1.1,tcp_dst=80 actions=drop" +), +True, +["nw_src", "tcp_dst"], +), +( +"nw_src=192.168.1.2 || tcp_dst=80", +OFPFlow( +"nw_src=192.168.1.1,tcp_dst=80 actions=drop" +), +True, +["nw_src", "tcp_dst"], +), +( +"nw_src=192.168.1.1 || tcp_dst=90", +OFPFlow( +"nw_src=192.168.1.1,tcp_dst=80 actions=drop" +), +True, +["nw_src", "tcp_dst"], +), +( +"nw_src=192.168.1.2 && tcp_dst=90", +OFPFlow( +"nw_src=192.168.1.1,tcp_dst=80 actions=drop" +), +False, +["nw_src", "tcp_dst"], +), +( +"nw_src=192.168.1.1", +OFPFlow( +"nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" +), +False, +["nw_src"], +), +( +"nw_src~=192.168.1.1", +OFPFlow( +"nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" +), +True, +["nw_src"], +), +( +"nw_src~=192.168.1.1/30", +OFPFlow( +"nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" +), +True, +["nw_src"], +), +( +"nw_src~=192.168.1.0/16", +OFPFlow( +"nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" +), +False, +["nw_src"], +), +( +"nw_src~=192.168.1.0/16", +OFPFlow( +"nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" +), +False, +["nw_src"], +), +( +"n_bytes=100", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" # noqa: E501 +), +True, +["n_bytes"], +), +( +"n_bytes>10", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" # noqa: E501 +), +True, +["n_bytes"], +), +( +"n_bytes>100", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" # noqa: E501 +), +False, +["n_bytes"], +), +( +"n_bytes<100", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" # noqa: E501 +), +False, +["n_bytes"], +), +( +"n_bytes<1000", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" # noqa: E501 +), +True, +["n_bytes"], +), +( +"n_bytes>0 && drop=true", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=drop" # noqa: E501 +), +True, +["n_bytes", "drop"], +), +( +"n_bytes>0 && drop=true", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=2" # noqa: E501 +), +False, +["n_bytes"], +), +( +"n_bytes>10 && !output.port=3", +OFPFlow( +"n_bytes=100 priority=100,nw_src=192.168.1.0/24,tcp_dst=80 actions=2" # noqa: E501 +), +True, +["n_bytes", "output"], +
[ovs-dev] [PATCH v3 12/18] tests: Wrap test-odp to also run python parsers
test-odp is used to parse datapath flow actions and matches within the odp tests. Wrap calls to this tool in a python script that also parses them using the python flow parsing library. Acked-by: Eelco Chaudron Signed-off-by: Adrian Moreno --- tests/automake.mk | 2 + tests/odp.at | 36 - tests/ovs-test-dpparse.py | 82 +++ 3 files changed, 102 insertions(+), 18 deletions(-) create mode 100755 tests/ovs-test-dpparse.py diff --git a/tests/automake.mk b/tests/automake.mk index 230085236..0279585cd 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -19,6 +19,7 @@ EXTRA_DIST += \ $(OVSDB_CLUSTER_TESTSUITE) \ tests/atlocal.in \ $(srcdir)/package.m4 \ + $(srcdir)/tests/ovs-test-dpparse.py \ $(srcdir)/tests/ovs-test-ofparse.py \ $(srcdir)/tests/testsuite \ $(srcdir)/tests/testsuite.patch @@ -525,6 +526,7 @@ CHECK_PYFILES = \ tests/flowgen.py \ tests/mfex_fuzzy.py \ tests/ovsdb-monitor-sort.py \ + tests/ovs-test-dpparse.py \ tests/ovs-test-ofparse.py \ tests/test-daemon.py \ tests/test-json.py \ diff --git a/tests/odp.at b/tests/odp.at index 4d08c59ca..00880565d 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -105,7 +105,7 @@ sed -i'back' 's/\(skb_mark(0)\),\(ct\)/\1,ct_state(0),ct_zone(0),\2/' odp-out.tx sed -i'back' 's/\(skb_mark([[^)]]*)\),\(recirc\)/\1,ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),\2/' odp-out.txt sed -i'back' 's/\(in_port(1)\),\(eth\)/\1,packet_type(ns=0,id=0),\2/' odp-out.txt -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` ]) AT_CLEANUP @@ -192,7 +192,7 @@ sed -n 's/,frag=no),.*/,frag=later)/p' odp-base.txt sed 's/^/skb_priority(0),tunnel(tun_id=0xfedcba9876543210,src=10.0.0.1,dst=10.0.0.2,ttl=128,erspan(ver=2,dir=1,hwid=0x7/0xf),flags(df|key)),skb_mark(0),recirc_id(0),dp_hash(0),/' odp-base.txt ) > odp.txt AT_CAPTURE_FILE([odp.txt]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` ]) AT_CLEANUP @@ -239,25 +239,25 @@ AT_DATA([odp-tcp6.txt], [dnl in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1/::255,dst=::2/::255,label=0/0xf0,proto=10/0xf0,tclass=0x70/0xf0,hlimit=128/0xf0,frag=no) in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1,dst=::2,label=0,proto=6,tclass=0,hlimit=128,frag=no),tcp(src=80/0xff00,dst=8080/0xff) ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` ])
[ovs-dev] [PATCH v3 16/18] python: add unit tests for openflow parsing
Add unit tests for OFPFlow class and ip-port range decoder
Signed-off-by: Adrian Moreno
---
python/automake.mk| 4 +-
python/ovs/tests/test_decoders.py | 130
python/ovs/tests/test_ofp.py | 534 ++
3 files changed, 667 insertions(+), 1 deletion(-)
create mode 100644 python/ovs/tests/test_decoders.py
create mode 100644 python/ovs/tests/test_ofp.py
diff --git a/python/automake.mk b/python/automake.mk
index 559ec3019..c9d9d02d1 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -54,8 +54,10 @@ ovs_pyfiles = \
python/ovs/winutils.py
ovs_pytests = \
+ python/ovs/tests/test_decoders.py \
python/ovs/tests/test_kv.py \
- python/ovs/tests/test_list.py
+ python/ovs/tests/test_list.py \
+ python/ovs/tests/test_ofp.py
# These python files are used at build time but not runtime,
# so they are not installed.
diff --git a/python/ovs/tests/test_decoders.py
b/python/ovs/tests/test_decoders.py
new file mode 100644
index 0..e387c5b2a
--- /dev/null
+++ b/python/ovs/tests/test_decoders.py
@@ -0,0 +1,130 @@
+from netaddr import IPAddress
+import pytest
+
+from ovs.flows.decoders import decode_ip_port_range
+
+
+@pytest.mark.parametrize(
+"input_string,expected",
+[
+(
+"192.168.0.0-192.168.0.200:1000-2000",
+{
+"addrs": {
+"start": IPAddress("192.168.0.0"),
+"end": IPAddress("192.168.0.200"),
+},
+"ports": {
+"start": 1000,
+"end": 2000,
+},
+},
+),
+(
+"192.168.0.0-192.168.0.200",
+{
+"addrs": {
+"start": IPAddress("192.168.0.0"),
+"end": IPAddress("192.168.0.200"),
+},
+},
+),
+(
+"192.168.0.0-192.168.0.200:2000",
+{
+"addrs": {
+"start": IPAddress("192.168.0.0"),
+"end": IPAddress("192.168.0.200"),
+},
+"ports": {
+"start": 2000,
+"end": 2000,
+},
+},
+),
+(
+"192.168.0.1:1000-2000",
+{
+"addrs": {
+"start": IPAddress("192.168.0.1"),
+"end": IPAddress("192.168.0.1"),
+},
+"ports": {
+"start": 1000,
+"end": 2000,
+},
+},
+),
+(
+
"[fe80::::0204:61ff:fe9d:f150]-[fe80::::0204:61ff:fe9d:f15f]:255",
# noqa: E501
+{
+"addrs": {
+"start": IPAddress(
+"fe80::::0204:61ff:fe9d:f150"
+),
+"end": IPAddress(
+"fe80::::0204:61ff:fe9d:f15f"
+),
+},
+"ports": {
+"start": 255,
+"end": 255,
+},
+},
+),
+(
+
"[fe80::204:61ff:254.157.241.86]-[fe80::204:61ff:254.157.241.100]:255-300", #
noqa: E501
+{
+"addrs": {
+"start": IPAddress("fe80::204:61ff:254.157.241.86"),
+"end": IPAddress("fe80::204:61ff:254.157.241.100"),
+},
+"ports": {
+"start": 255,
+"end": 300,
+},
+},
+),
+(
+"[fe80::f150]-[fe80::f15f]:255-300",
+{
+"addrs": {
+"start": IPAddress("fe80::f150"),
+"end": IPAddress("fe80::f15f"),
+},
+"ports": {
+"start": 255,
+"end": 300,
+},
+},
+),
+(
+
"fe80::::0204:61ff:fe9d:f150-fe80::::0204:61ff:fe9d:f15f",
# noqa: E501
+{
+"addrs": {
+"start": IPAddress(
+"fe80::::0204:61ff:fe9d:f150"
+),
+"end": IPAddress(
+"fe80::::0204:61ff:fe9d:f15f"
+),
+},
+},
+),
+(
+"fe80::::0204:61ff:fe9d:f156",
+{
+"addrs": {
+"start": IPAddress(
+"fe80::::0204:61ff:fe9d:f156"
+),
+"end": IPAddress(
+"fe80::::0204:61ff:fe9d:f156"
+),
+
[ovs-dev] [PATCH v3 17/18] python: add unit tests to datapath parsing
Signed-off-by: Adrian Moreno
---
python/automake.mk | 1 +
python/ovs/tests/test_odp.py | 527 +++
2 files changed, 528 insertions(+)
create mode 100644 python/ovs/tests/test_odp.py
diff --git a/python/automake.mk b/python/automake.mk
index c9d9d02d1..b2e7f6008 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -57,6 +57,7 @@ ovs_pytests = \
python/ovs/tests/test_decoders.py \
python/ovs/tests/test_kv.py \
python/ovs/tests/test_list.py \
+ python/ovs/tests/test_odp.py \
python/ovs/tests/test_ofp.py
# These python files are used at build time but not runtime,
diff --git a/python/ovs/tests/test_odp.py b/python/ovs/tests/test_odp.py
new file mode 100644
index 0..8ca8f38bf
--- /dev/null
+++ b/python/ovs/tests/test_odp.py
@@ -0,0 +1,527 @@
+import netaddr
+import pytest
+
+from ovs.flows.odp import ODPFlow
+from ovs.flows.kv import KeyValue
+from ovs.flows.decoders import (
+EthMask,
+IPMask,
+Mask32,
+Mask16,
+Mask8,
+Mask128,
+)
+
+
+@pytest.mark.parametrize(
+"input_string,expected",
+[
+(
+
"skb_priority(0x123),skb_mark(0x123),recirc_id(0x123),dp_hash(0x123),ct_zone(0x123),
actions:", # noqa: E501
+[
+KeyValue("skb_priority", Mask32("0x123")),
+KeyValue("skb_mark", Mask32("0x123")),
+KeyValue("recirc_id", 0x123),
+KeyValue("dp_hash", Mask32("0x123")),
+KeyValue("ct_zone", Mask16("0x123")),
+],
+),
+(
+
"tunnel(tun_id=0x7f10354,src=10.10.10.10,dst=20.20.20.20,ttl=64,flags(csum|key))
actions:", # noqa: E501
+[
+KeyValue(
+"tunnel",
+{
+"tun_id": 0x7F10354,
+"src": IPMask("10.10.10.10"),
+"dst": IPMask("20.20.20.20"),
+"ttl": 64,
+"flags": "csum|key",
+},
+)
+],
+),
+(
+
"tunnel(geneve({class=0,type=0,len=4,0xa/0xff}),vxlan(flags=0x80,vni=0x1c7),erspan(ver=2,dir=1,hwid=0x1)),
actions:", # noqa: E501
+[
+KeyValue(
+"tunnel",
+{
+"geneve": [
+{
+"class": Mask16("0"),
+"type": Mask8("0"),
+"len": Mask8("4"),
+"data": Mask128("0xa/0xff"),
+}
+],
+"vxlan": {"flags": 0x80, "vni": 0x1C7},
+"erspan": {"ver": 2, "dir": 1, "hwid": 0x1},
+},
+)
+],
+),
+(
+"in_port(2),eth(src=11:22:33:44:55:66,dst=66:55:44:33:22:11)
actions:", # noqa: E501
+[
+KeyValue("in_port", 2),
+KeyValue(
+"eth",
+{
+"src": EthMask("11:22:33:44:55:66"),
+"dst": EthMask("66:55:44:33:22:11"),
+},
+),
+],
+),
+(
+
"eth_type(0x800/0x006),ipv4(src=192.168.1.1/24,dst=192.168.0.0/16,proto=0x1,tos=0x2/0xf0)
actions:", # noqa: E501
+[
+KeyValue("eth_type", Mask16("0x800/0x006")),
+KeyValue(
+"ipv4",
+{
+"src": IPMask("192.168.1.1/24"),
+"dst": IPMask("192.168.0.0/16"),
+"proto": Mask8("0x1/0xFF"),
+"tos": Mask8("0x2/0xF0"),
+},
+),
+],
+),
+(
+
"encap(eth_type(0x800/0x006),ipv4(src=192.168.1.1/24,dst=192.168.0.0/16,proto=0x1,tos=0x2/0xf0))
actions:", # noqa: E501
+[
+KeyValue(
+"encap",
+{
+"eth_type": Mask16("0x800/0x006"),
+"ipv4": {
+"src": IPMask("192.168.1.1/24"),
+"dst": IPMask("192.168.0.0/16"),
+"proto": Mask8("0x1/0xff"),
+"tos": Mask8("0x2/0xf0"),
+},
+},
+),
+],
+),
+],
+)
+def test_odp_fields(input_string, expected):
+odp = ODPFlow(input_string)
+match = odp.match_kv
+for i in range(len(expected)):
+assert expected[i].key == match[i].key
+assert expected[i].value == match[i].value
+
+# Assert positions relative to action string are OK.
+mpos =
[ovs-dev] [PATCH v3 11/18] tests: wrap ovs-ofctl calls to test python parser
Some ovs-ofctl commands are used to parse or dump openflow flows, specially in ofp-actions.at Use a wrapper around ovs-ofctl, called ovs-test-ofparse.py that, apart from calling ovs-ofctl, also parses its output (or input, depending on the command) to make sure the python flow parsing library can also parse the flows. Acked-by: Eelco Chaudron Signed-off-by: Adrian Moreno --- tests/automake.mk | 3 ++ tests/ofp-actions.at | 46 tests/ovs-test-ofparse.py | 107 ++ 3 files changed, 133 insertions(+), 23 deletions(-) create mode 100755 tests/ovs-test-ofparse.py diff --git a/tests/automake.mk b/tests/automake.mk index 8a9151f81..230085236 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -19,9 +19,11 @@ EXTRA_DIST += \ $(OVSDB_CLUSTER_TESTSUITE) \ tests/atlocal.in \ $(srcdir)/package.m4 \ + $(srcdir)/tests/ovs-test-ofparse.py \ $(srcdir)/tests/testsuite \ $(srcdir)/tests/testsuite.patch + COMMON_MACROS_AT = \ tests/ovsdb-macros.at \ tests/ovs-macros.at \ @@ -523,6 +525,7 @@ CHECK_PYFILES = \ tests/flowgen.py \ tests/mfex_fuzzy.py \ tests/ovsdb-monitor-sort.py \ + tests/ovs-test-ofparse.py \ tests/test-daemon.py \ tests/test-json.py \ tests/test-jsonrpc.py \ diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 9d820eba6..6ee0d4773 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -327,7 +327,7 @@ AT_CAPTURE_FILE([input.txt]) AT_CAPTURE_FILE([expout]) AT_CAPTURE_FILE([experr]) AT_CHECK( - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < input.txt], + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < input.txt], [0], [expout], [experr]) AT_CLEANUP @@ -500,7 +500,7 @@ AT_CAPTURE_FILE([input.txt]) AT_CAPTURE_FILE([expout]) AT_CAPTURE_FILE([experr]) AT_CHECK( - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow11 < input.txt], + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow11 < input.txt], [0], [expout], [experr]) AT_CLEANUP @@ -735,7 +735,7 @@ AT_CAPTURE_FILE([input.txt]) AT_CAPTURE_FILE([expout]) AT_CAPTURE_FILE([experr]) AT_CHECK( - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow12 < input.txt], + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow12 < input.txt], [0], [expout], [experr]) AT_CLEANUP @@ -796,7 +796,7 @@ AT_CAPTURE_FILE([input.txt]) AT_CAPTURE_FILE([expout]) AT_CAPTURE_FILE([experr]) AT_CHECK( - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow13 < input.txt], + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow13 < input.txt], [0], [expout], [experr]) AT_CLEANUP @@ -825,16 +825,16 @@ AT_CAPTURE_FILE([input.txt]) AT_CAPTURE_FILE([expout]) AT_CAPTURE_FILE([experr]) AT_CHECK( - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow15 < input.txt], + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow15 < input.txt], [0], [expout], [experr]) AT_CLEANUP AT_SETUP([ofp-actions - inconsistent MPLS actions]) OVS_VSWITCHD_START dnl OK: Use fin_timeout action on TCP flow -AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)']) +AT_CHECK([ovs-test-ofparse.py -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)']) dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS -AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'], +AT_CHECK([ovs-test-ofparse.py -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'], [1], [], [dnl ovs-ofctl: none of the usable flow formats (OpenFlow10,NXM) is among the allowed flow formats (OpenFlow11) ]) @@ -848,8 +848,8 @@ dnl In OpenFlow 1.3, set_field always sets all the bits in the field, dnl but when we translate to NXAST_LOAD we need to only set the bits that dnl actually exist (e.g. mpls_label only has 20 bits) otherwise OVS rejects dnl the "load" action as invalid. Check that we do this correctly. -AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 mpls,actions=set_field:10-\>mpls_label]) -AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl +AT_CHECK([ovs-test-ofparse.py -O OpenFlow13 add-flow br0 mpls,actions=set_field:10-\>mpls_label]) +AT_CHECK([ovs-test-ofparse.py -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl NXST_FLOW reply: mpls actions=load:0xa->OXM_OF_MPLS_LABEL[[]] ]) @@ -861,12 +861,12 @@ AT_KEYWORDS([ofp-actions]) OVS_VSWITCHD_START dnl OpenFlow 1.0 has an "enqueue" action. For OpenFlow 1.1+, we translate dnl it to a series of actions that accomplish the same thing. -AT_CHECK([ovs-ofctl -O OpenFlow10 add-flow br0 'actions=enqueue(123,456)'])
[ovs-dev] [PATCH v3 08/18] python: add ovs datapath flow parsing
A ODPFlow is a Flow with the following sections:
ufid
info (e.g: bytes, packets, dp, etc)
match
actions
Only three datapath actions require special handling:
gre: because it has double parenthesis
geneve: because it supports many concatenated lists of options
nat: we reuse the decoder used for openflow actions
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/automake.mk | 1 +
python/ovs/flows/odp.py | 783
2 files changed, 784 insertions(+)
create mode 100644 python/ovs/flows/odp.py
diff --git a/python/automake.mk b/python/automake.mk
index 5b0c0d63f..aabbf7db2 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -31,6 +31,7 @@ ovs_pyfiles = \
python/ovs/flows/flow.py \
python/ovs/flows/kv.py \
python/ovs/flows/list.py \
+ python/ovs/flows/odp.py \
python/ovs/flows/ofp.py \
python/ovs/flows/ofp_act.py \
python/ovs/json.py \
diff --git a/python/ovs/flows/odp.py b/python/ovs/flows/odp.py
new file mode 100644
index 0..e894f4b96
--- /dev/null
+++ b/python/ovs/flows/odp.py
@@ -0,0 +1,783 @@
+""" Defines an Open vSwitch Datapath Flow.
+"""
+import re
+from functools import partial
+
+from ovs.flows.flow import Flow, Section
+
+from ovs.flows.kv import (
+KVParser,
+KVDecoders,
+nested_kv_decoder,
+decode_nested_kv,
+)
+from ovs.flows.decoders import (
+decode_default,
+decode_time,
+decode_int,
+decode_mask,
+Mask8,
+Mask16,
+Mask32,
+Mask64,
+Mask128,
+IPMask,
+EthMask,
+decode_free_output,
+decode_flag,
+decode_nat,
+)
+
+
+class ODPFlow(Flow):
+"""ODPFLow represents a Open vSwitch Datapath flow.
+
+Attributes:
+ufid: The UFID section with only one key-value, with keyword "ufid".
+info: The info section.
+match: The match section.
+actions: The actions section.
+id: The id object given at construction time.
+
+"""
+
+"""
+These class variables are used to cache the KVDecoders instances. This
+will speed up subsequent flow parsings.
+"""
+_info_decoders = None
+_match_decoders = None
+_action_decoders = None
+
+@staticmethod
+def info_decoders():
+"""Return the KVDecoders instance to parse the info section.
+
+Uses the cached version if available.
+"""
+if not ODPFlow._info_decoders:
+ODPFlow._info_decoders = ODPFlow._gen_info_decoders()
+return ODPFlow._info_decoders
+
+@staticmethod
+def match_decoders():
+"""Return the KVDecoders instance to parse the match section.
+
+Uses the cached version if available.
+"""
+if not ODPFlow._match_decoders:
+ODPFlow._match_decoders = ODPFlow._gen_match_decoders()
+return ODPFlow._match_decoders
+
+@staticmethod
+def action_decoders():
+"""Return the KVDecoders instance to parse the actions section.
+
+Uses the cached version if available.
+"""
+if not ODPFlow._action_decoders:
+ODPFlow._action_decoders = ODPFlow._gen_action_decoders()
+return ODPFlow._action_decoders
+
+def __init__(self, odp_string, id=None):
+"""Parse a odp flow string.
+
+The string is expected to have the following format:
+ [ufid], [match] [flow data] actions:[actions]
+
+Args:
+odp_string (str): A datapath flow string.
+
+Returns:
+A ODPFlow instance.
+"""
+sections = []
+
+# If UFID present, parse it and add it to it's own section.
+ufid_pos = odp_string.find("ufid:")
+if ufid_pos >= 0:
+ufid_string = odp_string[
+ufid_pos : (odp_string[ufid_pos:].find(",") + 1)
+]
+ufid_parser = KVParser(
+ufid_string, KVDecoders({"ufid": decode_default})
+)
+ufid_parser.parse()
+if len(ufid_parser.kv()) != 1:
+raise ValueError("malformed odp flow: %s" % odp_string)
+sections.append(
+Section("ufid", ufid_pos, ufid_string, ufid_parser.kv())
+)
+
+action_pos = odp_string.find("actions:")
+if action_pos < 0:
+raise ValueError("malformed odp flow: %s" % odp_string)
+
+# rest of the string is between ufid and actions
+rest = odp_string[
+(ufid_pos + len(ufid_string) if ufid_pos >= 0 else 0) : action_pos
+]
+
+action_pos += 8 # len("actions:")
+actions = odp_string[action_pos:]
+
+field_parts = rest.lstrip(" ").partition(" ")
+
+if len(field_parts) != 3:
+raise ValueError("malformed odp flow: %s" % odp_string)
+
+match = field_parts[0]
+info = field_parts[2]
+
+iparser = KVParser(info, ODPFlow.info_decoders())
+iparser.parse()
+isection =
[ovs-dev] [PATCH v3 14/18] python: introduce unit tests
Use pytest to run unit tests as part of the standard testsuite.
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
.github/workflows/build-and-test.yml| 3 +
Documentation/intro/install/general.rst | 4 ++
python/automake.mk | 9 ++-
python/ovs/tests/test_kv.py | 76 +
python/test_requirements.txt| 3 +
tests/atlocal.in| 19 +++
tests/automake.mk | 1 +
tests/pytest.at | 7 +++
tests/testsuite.at | 1 +
9 files changed, 121 insertions(+), 2 deletions(-)
create mode 100644 python/ovs/tests/test_kv.py
create mode 100644 python/test_requirements.txt
create mode 100644 tests/pytest.at
diff --git a/.github/workflows/build-and-test.yml
b/.github/workflows/build-and-test.yml
index eac3504e4..44df1c2d5 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -123,6 +123,9 @@ jobs:
with:
python-version: '3.9'
+- name: install python dependencies
+ run: pip install -r python/test_requirements.txt
+
- name: create ci signature file for the dpdk cache key
if: matrix.dpdk != '' || matrix.dpdk_shared != ''
# This will collect most of DPDK related lines, so hash will be different
diff --git a/Documentation/intro/install/general.rst
b/Documentation/intro/install/general.rst
index c4300cd53..711fb98a4 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -181,6 +181,10 @@ following to obtain better warnings:
come from the "hacking" flake8 plugin. If it's not installed, the warnings
just won't occur until it's run on a system with "hacking" installed.
+- the python packages listed in "python/test_requirements.txt" (compatible
+ with pip). If they are installed, the pytest-based Python unit tests will
+ be run.
+
You may find the ovs-dev script found in ``utilities/ovs-dev.py`` useful.
.. _general-install-reqs:
diff --git a/python/automake.mk b/python/automake.mk
index e5501c58e..43a27d3cb 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -53,6 +53,9 @@ ovs_pyfiles = \
python/ovs/vlog.py \
python/ovs/winutils.py
+ovs_pytests = \
+ python/ovs/tests/test_kv.py
+
# These python files are used at build time but not runtime,
# so they are not installed.
EXTRA_DIST += \
@@ -66,12 +69,14 @@ EXTRA_DIST += \
EXTRA_DIST += \
python/ovs/compat/sortedcontainers/LICENSE \
python/README.rst \
- python/setup.py
+ python/setup.py \
+ python/test_requirements.txt
# C extension support.
EXTRA_DIST += python/ovs/_json.c
-PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles)
+PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests)
+
EXTRA_DIST += $(PYFILES)
PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
diff --git a/python/ovs/tests/test_kv.py b/python/ovs/tests/test_kv.py
new file mode 100644
index 0..e81804d49
--- /dev/null
+++ b/python/ovs/tests/test_kv.py
@@ -0,0 +1,76 @@
+import pytest
+
+from ovs.flows.kv import KVParser, KeyValue
+
+
+@pytest.mark.parametrize(
+"input_data,expected",
+[
+(
+(
+"cookie=0x0, duration=147566.365s, table=0, n_packets=39,
n_bytes=2574, idle_age=65534, hard_age=65534", # noqa: E501
+None,
+),
+[
+KeyValue("cookie", 0),
+KeyValue("duration", "147566.365s"),
+KeyValue("table", 0),
+KeyValue("n_packets", 39),
+KeyValue("n_bytes", 2574),
+KeyValue("idle_age", 65534),
+KeyValue("hard_age", 65534),
+],
+),
+(
+(
+
"load:0x4->NXM_NX_REG13[],load:0x9->NXM_NX_REG11[],load:0x8->NXM_NX_REG12[],load:0x1->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],mod_dl_src:0a:58:a9:fe:00:02,resubmit(,8)",
# noqa: E501
+None,
+),
+[
+KeyValue("load", "0x4->NXM_NX_REG13[]"),
+KeyValue("load", "0x9->NXM_NX_REG11[]"),
+KeyValue("load", "0x8->NXM_NX_REG12[]"),
+KeyValue("load", "0x1->OXM_OF_METADATA[]"),
+KeyValue("load", "0x1->NXM_NX_REG14[]"),
+KeyValue("mod_dl_src", "0a:58:a9:fe:00:02"),
+KeyValue("resubmit", ",8"),
+],
+),
+(
+("l1(l2(l3(l4(", None),
+[KeyValue("l1", "l2(l3(l4()))")]
+),
+(
+("l1(l2(l3(l4(,foo:bar", None),
+[KeyValue("l1", "l2(l3(l4()))"), KeyValue("foo", "bar")],
+),
+(
+("enqueue:1:2,output=2", None),
+[KeyValue("enqueue", "1:2"), KeyValue("output", 2)],
+),
+(
+
[ovs-dev] [PATCH v3 15/18] python: add unit tests for ListParser
Add unit tests for ListParser class.
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/automake.mk| 3 +-
python/ovs/tests/test_list.py | 66 +++
2 files changed, 68 insertions(+), 1 deletion(-)
create mode 100644 python/ovs/tests/test_list.py
diff --git a/python/automake.mk b/python/automake.mk
index 43a27d3cb..559ec3019 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -54,7 +54,8 @@ ovs_pyfiles = \
python/ovs/winutils.py
ovs_pytests = \
- python/ovs/tests/test_kv.py
+ python/ovs/tests/test_kv.py \
+ python/ovs/tests/test_list.py
# These python files are used at build time but not runtime,
# so they are not installed.
diff --git a/python/ovs/tests/test_list.py b/python/ovs/tests/test_list.py
new file mode 100644
index 0..e5139869f
--- /dev/null
+++ b/python/ovs/tests/test_list.py
@@ -0,0 +1,66 @@
+import pytest
+
+from ovs.flows.list import ListParser, ListDecoders
+from ovs.flows.kv import KeyValue
+
+
+@pytest.mark.parametrize(
+"input_data,expected",
+[
+(
+("field1,field2,3,nested:value", None, [","]),
+[
+KeyValue("elem_0", "field1"),
+KeyValue("elem_1", "field2"),
+KeyValue("elem_2", 3),
+KeyValue("elem_3", "nested:value"),
+],
+),
+(
+(
+"field1,field2,3,nested:value",
+ListDecoders(
+[
+("key1", str),
+("key2", str),
+("key3", int),
+("key4", lambda x: x.split(":"), [","]),
+]
+),
+[","],
+),
+[
+KeyValue("key1", "field1"),
+KeyValue("key2", "field2"),
+KeyValue("key3", 3),
+KeyValue("key4", ["nested", "value"]),
+],
+),
+(
+("field1:field2:3", None, [":"]),
+[
+KeyValue("elem_0", "field1"),
+KeyValue("elem_1", "field2"),
+KeyValue("elem_2", 3),
+],
+),
+],
+)
+def test_kv_parser(input_data, expected):
+input_string = input_data[0]
+decoders = input_data[1]
+delims = input_data[2]
+tparser = ListParser(input_string, decoders, delims)
+tparser.parse()
+result = tparser.kv()
+assert len(expected) == len(result)
+for i in range(0, len(result)):
+assert result[i].key == expected[i].key
+assert result[i].value == expected[i].value
+kpos = result[i].meta.kpos
+kstr = result[i].meta.kstring
+vpos = result[i].meta.vpos
+vstr = result[i].meta.vstring
+assert input_string[kpos : kpos + len(kstr)] == kstr
+if vpos != -1:
+assert input_string[vpos : vpos + len(vstr)] == vstr
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 10/18] python: add a json encoder to flow fields
The json encoder can be used to convert Flows to json. Acked-by: Eelco Chaudron Signed-off-by: Adrian Moreno --- python/ovs/flows/decoders.py | 14 ++ 1 file changed, 14 insertions(+) diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py index 73d28e057..7378d4176 100644 --- a/python/ovs/flows/decoders.py +++ b/python/ovs/flows/decoders.py @@ -5,6 +5,7 @@ A decoder is generally a callable that accepts a string and returns the value object. """ +import json import netaddr import re @@ -522,3 +523,16 @@ def decode_nat(value): result[flag] = True return result + + +class FlowEncoder(json.JSONEncoder): +"""FlowEncoder is a json.JSONEncoder instance that can be used to +serialize flow fields.""" + +def default(self, obj): +if isinstance(obj, Decoder): +return obj.to_json() +elif isinstance(obj, netaddr.IPAddress): +return str(obj) + +return json.JSONEncoder.default(self, obj) -- 2.34.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 13/18] python: detect changes in flow formatting code
In order to minimize the risk of having the python flow parsing code and
the C flow formatting code divert, add a target that checks if the
formatting code has been changed since the last revision and warn the
developer if it has.
The script also makes it easy to update the dependency file so hopefully
it will not cause too much trouble for a developer that has modifed the
file without changing the flow string format.
Signed-off-by: Adrian Moreno
---
.gitignore | 1 +
python/automake.mk | 9 +++
python/build/flow-parse-deps.py | 106
python/ovs/flows/deps.py| 5 ++
4 files changed, 121 insertions(+)
create mode 100755 python/build/flow-parse-deps.py
create mode 100644 python/ovs/flows/deps.py
diff --git a/.gitignore b/.gitignore
index f1cdcf124..e6bca1cd2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -79,3 +79,4 @@ testsuite.tmp.orig
/Documentation/_build
/.venv
/cxx-check
+/flowparse-deps-check
diff --git a/python/automake.mk b/python/automake.mk
index 9bd7a9e2f..e5501c58e 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -28,6 +28,7 @@ ovs_pyfiles = \
python/ovs/fcntl_win.py \
python/ovs/flows/__init__.py \
python/ovs/flows/decoders.py \
+ python/ovs/flows/deps.py \
python/ovs/flows/filter.py \
python/ovs/flows/flow.py \
python/ovs/flows/kv.py \
@@ -57,6 +58,7 @@ ovs_pyfiles = \
EXTRA_DIST += \
python/build/__init__.py \
python/build/extract_ofp_fields.py \
+ python/build/flow-parse-deps.py \
python/build/nroff.py \
python/build/soutil.py
@@ -77,6 +79,7 @@ FLAKE8_PYFILES += \
$(filter-out python/ovs/compat/% python/ovs/dirs.py,$(PYFILES)) \
python/build/__init__.py \
python/build/extract_ofp_fields.py \
+ python/build/flow-parse-deps.py \
python/build/nroff.py \
python/build/soutil.py \
python/ovs/dirs.py.template \
@@ -137,3 +140,9 @@ $(srcdir)/python/ovs/flows/ofp_fields.py:
$(srcdir)/build-aux/gen_ofp_field_deco
EXTRA_DIST += python/ovs/flows/ofp_fields.py
CLEANFILES += python/ovs/flows/ofp_fields.py
+ALL_LOCAL += flowparse-deps-check
+DEPS = $(shell $(AM_V_GEN)$(run_python)
$(srcdir)/python/build/flow-parse-deps.py list)
+flowparse-deps-check: $(srcdir)/python/build/flow-parse-deps.py $(DEPS)
+ $(AM_V_GEN)$(run_python) $(srcdir)/python/build/flow-parse-deps.py check
+ touch $@
+CLEANFILES += flowparse-deps-check
diff --git a/python/build/flow-parse-deps.py b/python/build/flow-parse-deps.py
new file mode 100755
index 0..848ef6bac
--- /dev/null
+++ b/python/build/flow-parse-deps.py
@@ -0,0 +1,106 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Breaks lines read from stdin into groups using blank lines as
+# group separators, then sorts lines within the groups for
+# reproducibility.
+
+
+# ovs-test-ofparse is just a wrapper around ovs-ofctl
+# that also runs the python flow parsing utility to check that flows are
+# parseable
+
+import hashlib
+import sys
+import os
+
+DEPENDENCIES = ["lib/ofp-actions.c", "lib/odp-util.c"]
+DEPENDENCY_FILE = "python/ovs/flows/deps.py"
+SRC_DIR = os.path.join(os.path.dirname(__file__), "..", "..")
+
+
+def usage():
+print(
+"""
+Usage {cmd} [check | update | list]
+Tool to verify flow parsing python code is kept in sync with
+flow printing C code.
+
+Commands:
+ check: check the dependencies are met
+ update: update the dependencies based on current file content
+ list: list the dependency files
+""".format(
+cmd=sys.argv[0]
+)
+)
+
+
+def digest(filename):
+with open(os.path.join(SRC_DIR, filename), "rb") as f:
+return hashlib.md5(f.read()).hexdigest()
+
+
+def main():
+if len(sys.argv) != 2:
+usage()
+sys.exit(1)
+
+if sys.argv[1] == "list":
+print(" ".join(DEPENDENCIES))
+elif sys.argv[1] == "update":
+dep_str = list()
+for dep in DEPENDENCIES:
+dep_str.append(
+'"{dep}": "{digest}"'.format(dep=dep, digest=digest(dep))
+)
+
+depends = """# File automatically generated. Do not modify manually!
+dependencies = {{
+{dependencies_dict}
+}}""".format(
+dependencies_dict=",\n".join(dep_str)
+)
+with open(os.path.join(SRC_DIR, DEPENDENCY_FILE), "w") as
[ovs-dev] [PATCH v3 07/18] python: introduce OpenFlow Flow parsing
Introduce OFPFlow class and all its decoders.
Most of the decoders are generic (from decoders.py). Some have special
syntax and need a specific implementation.
Decoders for nat are moved to the common decoders.py because it's syntax
is shared with other types of flows (e.g: dpif flows).
Signed-off-by: Adrian Moreno
---
python/automake.mk | 2 +
python/ovs/flows/decoders.py | 108 +
python/ovs/flows/ofp.py | 428 +++
python/ovs/flows/ofp_act.py | 306 +
4 files changed, 844 insertions(+)
create mode 100644 python/ovs/flows/ofp.py
create mode 100644 python/ovs/flows/ofp_act.py
diff --git a/python/automake.mk b/python/automake.mk
index ac7dcdd44..5b0c0d63f 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -31,6 +31,8 @@ ovs_pyfiles = \
python/ovs/flows/flow.py \
python/ovs/flows/kv.py \
python/ovs/flows/list.py \
+ python/ovs/flows/ofp.py \
+ python/ovs/flows/ofp_act.py \
python/ovs/json.py \
python/ovs/jsonrpc.py \
python/ovs/ovsuuid.py \
diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py
index 883e61acf..73d28e057 100644
--- a/python/ovs/flows/decoders.py
+++ b/python/ovs/flows/decoders.py
@@ -6,6 +6,7 @@ object.
"""
import netaddr
+import re
class Decoder(object):
@@ -414,3 +415,110 @@ class IPMask(Decoder):
def to_json(self):
return str(self)
+
+
+def decode_free_output(value):
+"""The value of the output action can be found free, i.e: without the
+'output' keyword. This decoder decodes its value when found this way."""
+try:
+return "output", {"port": int(value)}
+except ValueError:
+return "output", {"port": value.strip('"')}
+
+
+ipv4 = r"(?:\d{1,3}.?){3}\d{1,3}"
+ipv4_capture = r"({ipv4})".format(ipv4=ipv4)
+ipv6 = r"[\w:\.]+"
+ipv6_capture = r"(?:\[*)?({ipv6})(?:\]*)?".format(ipv6=ipv6)
+port_range = r":(\d+)(?:-(\d+))?"
+ip_range_regexp = r"{ip_cap}(?:-{ip_cap})?(?:{port_range})?"
+ipv4_port_regex = re.compile(
+ip_range_regexp.format(ip_cap=ipv4_capture, port_range=port_range)
+)
+ipv6_port_regex = re.compile(
+ip_range_regexp.format(ip_cap=ipv6_capture, port_range=port_range)
+)
+
+
+def decode_ip_port_range(value):
+"""
+Decodes an IP and port range:
+{ip_start}-{ip-end}:{port_start}-{port_end}
+
+IPv6 addresses are surrounded by "[" and "]" if port ranges are also
+present
+
+Returns the following dictionary:
+{
+"addrs": {
+"start": {ip_start}
+"end": {ip_end}
+}
+"ports": {
+"start": {port_start},
+"end": {port_end}
+}
+(the "ports" key might be omitted)
+"""
+if value.count(":") > 1:
+match = ipv6_port_regex.match(value)
+else:
+match = ipv4_port_regex.match(value)
+
+ip_start = match.group(1)
+ip_end = match.group(2)
+port_start = match.group(3)
+port_end = match.group(4)
+
+result = {
+"addrs": {
+"start": netaddr.IPAddress(ip_start),
+"end": netaddr.IPAddress(ip_end or ip_start),
+}
+}
+if port_start:
+result["ports"] = {
+"start": int(port_start),
+"end": int(port_end or port_start),
+}
+
+return result
+
+
+def decode_nat(value):
+"""Decodes the 'nat' keyword of the ct action.
+
+The format is:
+nat
+Flag format.
+nat(type=addrs[:ports][,flag]...)
+Full format where the address-port range has the same format as
+the one described in decode_ip_port_range.
+
+Examples:
+nat(src=0.0.0.0)
+nat(src=0.0.0.0,persistent)
+nat(dst=192.168.1.0-192.168.1.253:4000-5000)
+nat(dst=192.168.1.0-192.168.1.253,hash)
+nat(dst=[fe80::f150]-[fe80::f15f]:255-300)
+"""
+if not value:
+return True # If flag format, the value is True.
+
+result = dict()
+type_parts = value.split("=")
+result["type"] = type_parts[0]
+
+if len(type_parts) > 1:
+value_parts = type_parts[1].split(",")
+if len(type_parts) != 2:
+raise ValueError("Malformed nat action: %s" % value)
+
+ip_port_range = decode_ip_port_range(value_parts[0])
+
+result = {"type": type_parts[0], **ip_port_range}
+
+for flag in value_parts[1:]:
+result[flag] = True
+
+return result
diff --git a/python/ovs/flows/ofp.py b/python/ovs/flows/ofp.py
new file mode 100644
index 0..98902c1e7
--- /dev/null
+++ b/python/ovs/flows/ofp.py
@@ -0,0 +1,428 @@
+"""Defines the parsers needed to parse ofproto flows.
+"""
+
+import functools
+
+from ovs.flows.kv import KVParser, KVDecoders, nested_kv_decoder
+from ovs.flows.ofp_fields import field_decoders
+from ovs.flows.flow import Flow, Section
+from ovs.flows.list import ListDecoders,
[ovs-dev] [PATCH v3 09/18] python: add flow filtering syntax
Based on pyparsing, create a very simple filtering syntax.
It supports basic logic statements (and, &, or, ||, not, !), numerical
operations (<, >), equality (=, !=), and masking (~=). The latter is only
supported in certain fields (IntMask, EthMask, IPMask).
Masking operation is semantically equivalent to "includes",
therefore:
ip_src ~= 192.168.1.1
means that ip_src field is either a host IP address equal to 192.168.1.1
or an IPMask that includes it (e.g: 192.168.1.1/24).
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/automake.mk | 1 +
python/ovs/flows/filter.py | 261 +
python/setup.py| 2 +-
3 files changed, 263 insertions(+), 1 deletion(-)
create mode 100644 python/ovs/flows/filter.py
diff --git a/python/automake.mk b/python/automake.mk
index aabbf7db2..9bd7a9e2f 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -28,6 +28,7 @@ ovs_pyfiles = \
python/ovs/fcntl_win.py \
python/ovs/flows/__init__.py \
python/ovs/flows/decoders.py \
+ python/ovs/flows/filter.py \
python/ovs/flows/flow.py \
python/ovs/flows/kv.py \
python/ovs/flows/list.py \
diff --git a/python/ovs/flows/filter.py b/python/ovs/flows/filter.py
new file mode 100644
index 0..e520e90dc
--- /dev/null
+++ b/python/ovs/flows/filter.py
@@ -0,0 +1,261 @@
+""" Defines a Flow Filtering syntax.
+"""
+import pyparsing as pp
+import netaddr
+from functools import reduce
+from operator import and_, or_
+
+from ovs.flows.decoders import (
+decode_default,
+decode_int,
+Decoder,
+IPMask,
+EthMask,
+)
+
+
+class EvaluationResult(object):
+"""An EvaluationResult is the result of an evaluation. It contains the
+boolean result and the list of key-values that were evaluated.
+
+Note that since boolean operations (and, not, or) are based only on
+__bool__ we use bitwise alternatives (&, ||, ~).
+"""
+
+def __init__(self, result, *kv):
+self.result = result
+self.kv = kv if kv else list()
+
+def __and__(self, other):
+"""Logical and operation."""
+return EvaluationResult(
+self.result and other.result, *self.kv, *other.kv
+)
+
+def __or__(self, other):
+"""Logical or operation."""
+return EvaluationResult(
+self.result or other.result, *self.kv, *other.kv
+)
+
+def __invert__(self):
+"""Logical not operation."""
+return EvaluationResult(not self.result, *self.kv)
+
+def __bool__(self):
+"""Boolean operation."""
+return self.result
+
+def __repr__(self):
+return "{} [{}]".format(self.result, self.kv)
+
+
+class ClauseExpression(object):
+""" A clause expression represents a specific expression in the filter.
+
+A clause has the following form:
+[field] [operator] [value]
+
+Valid operators are:
+= (equality)
+!= (inequality)
+< (arithmetic less-than)
+> (arithmetic more-than)
+~= (__contains__)
+
+When evaluated, the clause finds what relevant part of the flow to use for
+evaluation, tries to translate the clause value to the relevant type and
+performs the clause operation.
+
+Attributes:
+field (str): The flow field used in the clause.
+operator (str): The flow operator used in the clause.
+value (str): The value to perform the comparison against.
+"""
+operators = {}
+type_decoders = {
+int: decode_int,
+netaddr.IPAddress: IPMask,
+netaddr.EUI: EthMask,
+bool: bool,
+}
+
+def __init__(self, tokens):
+self.field = tokens[0]
+self.value = ""
+self.operator = ""
+
+if len(tokens) > 1:
+self.operator = tokens[1]
+self.value = tokens[2]
+
+def __repr__(self):
+return "{}(field: {}, operator: {}, value: {})".format(
+self.__class__.__name__, self.field, self.operator, self.value
+)
+
+def _find_data_in_kv(self, kv_list):
+"""Find a KeyValue for evaluation in a list of KeyValue.
+
+Args:
+kv_list (list[KeyValue]): list of KeyValue to look into.
+
+Returns:
+If found, tuple (kv, data) where kv is the KeyValue that matched
+and data is the data to be used for evaluation. None if not found.
+"""
+key_parts = self.field.split(".")
+field = key_parts[0]
+kvs = [kv for kv in kv_list if kv.key == field]
+if not kvs:
+return None
+
+for kv in kvs:
+if kv.key == self.field:
+# exact match
+return (kv, kv.value)
+if len(key_parts) > 1:
+data = kv.value
+for subkey in key_parts[1:]:
+try:
+data = data.get(subkey)
+except
[ovs-dev] [PATCH v3 06/18] python: add flow base class
It simplifies the implementation of different types of flows by creating
the concept of Section (e.g: match, action) and automatic accessors for
all the provided Sections
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/automake.mk | 1 +
python/ovs/flows/flow.py | 125 +++
2 files changed, 126 insertions(+)
create mode 100644 python/ovs/flows/flow.py
diff --git a/python/automake.mk b/python/automake.mk
index 16d8db98f..ac7dcdd44 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -28,6 +28,7 @@ ovs_pyfiles = \
python/ovs/fcntl_win.py \
python/ovs/flows/__init__.py \
python/ovs/flows/decoders.py \
+ python/ovs/flows/flow.py \
python/ovs/flows/kv.py \
python/ovs/flows/list.py \
python/ovs/json.py \
diff --git a/python/ovs/flows/flow.py b/python/ovs/flows/flow.py
new file mode 100644
index 0..2f053e77d
--- /dev/null
+++ b/python/ovs/flows/flow.py
@@ -0,0 +1,125 @@
+""" Defines the Flow class.
+"""
+
+
+class Section(object):
+"""A flow can be seen as composed of different sections, e.g:
+
+ [info] [match] actions=[actions]
+
+This class represents each of those sections.
+
+A section is basically a set of Key-Value pairs. Typically, they can be
+expressed as a dictionary, for instance the "match" part of a flow can be
+expressed as:
+{
+"nw_src": "192.168.1.1",
+"nw_dst": "192.168.1.2",
+}
+However, some of them must be expressed as a list which allows for
+duplicated keys. For instance, the "actions" section could be:
+[
+{
+"output": 32
+},
+{
+"output": 33
+}
+]
+
+The is_list flag is used to discriminate this.
+
+Attributes:
+name (str): Name of the section.
+pos (int): Position within the overall flow string.
+string (str): Section string.
+data (list[KeyValue]): Parsed data of the section.
+is_list (bool): Whether the key-values shall be expressed as a list
+(i.e: it allows repeated keys).
+"""
+
+def __init__(self, name, pos, string, data, is_list=False):
+self.name = name
+self.pos = pos
+self.string = string
+self.data = data
+self.is_list = is_list
+
+def __str__(self):
+return "{} (at {}): {}".format(self.name, self.pos, self.string)
+
+def __repr__(self):
+return "%s('%s')" % (self.__class__.__name__, self)
+
+def dict(self):
+return {self.name: self.format_data()}
+
+def format_data(self):
+"""Returns the section's key-values formatted in a dictionary or list
+depending on the value of is_list flag.
+"""
+if self.is_list:
+return [{item.key: item.value} for item in self.data]
+else:
+return {item.key: item.value for item in self.data}
+
+
+class Flow(object):
+"""The Flow class is a base class for other types of concrete flows
+(such as OFproto Flows or DPIF Flows).
+
+A flow is basically comprised of a number of sections.
+For each section named {section_name}, the flow object will have the
+following attributes:
+ - {section_name} will return the sections data in a formatted way.
+ - {section_name}_kv will return the sections data as a list of KeyValues.
+
+Args:
+sections (list[Section]): List of sections that comprise the flow
+orig (str): Original flow string.
+id (Any): Optional; identifier that clients can use to uniquely
+identify this flow.
+"""
+
+def __init__(self, sections, orig="", id=None):
+self._sections = sections
+self._orig = orig
+self._id = id
+for section in sections:
+setattr(
+self, section.name, self.section(section.name).format_data()
+)
+setattr(
+self,
+"{}_kv".format(section.name),
+self.section(section.name).data,
+)
+
+def section(self, name):
+"""Return the section by name."""
+return next(
+(sect for sect in self._sections if sect.name == name), None
+)
+
+@property
+def id(self):
+"""Return the Flow ID."""
+return self._id
+
+@property
+def sections(self):
+"""Return the all the sections in a list."""
+return self._sections
+
+@property
+def orig(self):
+"""Return the original flow string."""
+return self._orig
+
+def dict(self):
+"""Returns the Flow information in a dictionary."""
+flow_dict = {"orig": self.orig}
+for section in self.sections:
+flow_dict.update(section.dict())
+
+return flow_dict
--
2.34.1
___
dev mailing list
[ovs-dev] [PATCH v3 04/18] build-aux: split extract-ofp-fields
In order to be able to reuse the core extraction logic, split the command
in two parts. The core extraction logic is moved to python/build while
the command that writes the different files out of the extracted field
info is kept in build-aux.
Signed-off-by: Adrian Moreno
---
build-aux/extract-ofp-fields | 706 -
python/automake.mk | 7 +-
python/build/extract_ofp_fields.py | 421 +
3 files changed, 618 insertions(+), 516 deletions(-)
create mode 100644 python/build/extract_ofp_fields.py
diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 8766995d9..efec59c25 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -3,85 +3,23 @@
import getopt
import sys
import os.path
-import re
import xml.dom.minidom
import build.nroff
-line = ""
-
-# Maps from user-friendly version number to its protocol encoding.
-VERSION = {"1.0": 0x01,
- "1.1": 0x02,
- "1.2": 0x03,
- "1.3": 0x04,
- "1.4": 0x05,
- "1.5": 0x06}
-VERSION_REVERSE = dict((v,k) for k, v in VERSION.items())
-
-TYPES = {"u8": (1, False),
- "be16": (2, False),
- "be32": (4, False),
- "MAC": (6, False),
- "be64": (8, False),
- "be128":(16, False),
- "tunnelMD": (124, True)}
-
-FORMATTING = {"decimal":("MFS_DECIMAL", 1, 8),
- "hexadecimal":("MFS_HEXADECIMAL", 1, 127),
- "ct state": ("MFS_CT_STATE", 4, 4),
- "Ethernet": ("MFS_ETHERNET", 6, 6),
- "IPv4": ("MFS_IPV4", 4, 4),
- "IPv6": ("MFS_IPV6",16, 16),
- "OpenFlow 1.0 port": ("MFS_OFP_PORT", 2, 2),
- "OpenFlow 1.1+ port": ("MFS_OFP_PORT_OXM", 4, 4),
- "frag": ("MFS_FRAG", 1, 1),
- "tunnel flags": ("MFS_TNL_FLAGS",2, 2),
- "TCP flags": ("MFS_TCP_FLAGS",2, 2),
- "packet type":("MFS_PACKET_TYPE", 4, 4)}
-
-PREREQS = {"none": "MFP_NONE",
- "Ethernet": "MFP_ETHERNET",
- "ARP": "MFP_ARP",
- "VLAN VID": "MFP_VLAN_VID",
- "IPv4": "MFP_IPV4",
- "IPv6": "MFP_IPV6",
- "IPv4/IPv6": "MFP_IP_ANY",
- "NSH": "MFP_NSH",
- "CT": "MFP_CT_VALID",
- "MPLS": "MFP_MPLS",
- "TCP": "MFP_TCP",
- "UDP": "MFP_UDP",
- "SCTP": "MFP_SCTP",
- "ICMPv4": "MFP_ICMPV4",
- "ICMPv6": "MFP_ICMPV6",
- "ND": "MFP_ND",
- "ND solicit": "MFP_ND_SOLICIT",
- "ND advert": "MFP_ND_ADVERT"}
-
-# Maps a name prefix into an (experimenter ID, class) pair, so:
-#
-# - Standard OXM classes are written as (0, )
-#
-# - Experimenter OXM classes are written as (, 0x)
-#
-# If a name matches more than one prefix, the longest one is used.
-OXM_CLASSES = {"NXM_OF_":(0, 0x, 'extension'),
- "NXM_NX_":(0, 0x0001, 'extension'),
- "NXOXM_NSH_": (0x005ad650, 0x, 'extension'),
- "OXM_OF_":(0, 0x8000, 'standard'),
- "OXM_OF_PKT_REG": (0, 0x8001, 'standard'),
- "ONFOXM_ET_": (0x4f4e4600, 0x, 'standard'),
- "ERICOXM_OF_":(0, 0x1000, 'extension'),
-
- # This is the experimenter OXM class for Nicira, which is the
- # one that OVS would be using instead of NXM_OF_ and NXM_NX_
- # if OVS didn't have those grandfathered in. It is currently
- # used only to test support for experimenter OXM, since there
- # are barely any real uses of experimenter OXM in the wild.
- "NXOXM_ET_": (0x2320, 0x, 'extension')}
+from build.extract_ofp_fields import (
+extract_ofp_fields,
+PREREQS,
+OXM_CLASSES,
+VERSION,
+fatal,
+n_errors,
+)
+
+VERSION_REVERSE = dict((v, k) for k, v in VERSION.items())
+
def oxm_name_to_class(name):
-prefix = ''
+prefix = ""
class_ = None
for p, c in OXM_CLASSES.items():
if name.startswith(p) and len(p) > len(prefix):
@@ -92,267 +30,76 @@ def oxm_name_to_class(name):
def is_standard_oxm(name):
oxm_vendor, oxm_class, oxm_class_type = oxm_name_to_class(name)
-return oxm_class_type == 'standard'
-
-
-def decode_version_range(range):
-if range in VERSION:
-return (VERSION[range], VERSION[range])
-elif range.endswith('+'):
-return (VERSION[range[:-1]], max(VERSION.values()))
-else:
-a, b = re.match(r'^([^-]+)-([^-]+)$', range).groups()
-return (VERSION[a], VERSION[b])
-
-
-def get_line():
-global line
-global line_number
-line =
[ovs-dev] [PATCH v3 03/18] python: add list parser
Some openflow or dpif flows encode their arguments in lists, eg:
"some_action(arg1,arg2,arg3)". In order to decode this in a way that can
be then stored and queried, add ListParser and ListDecoders classes
that parse lists into KeyValue instances.
The ListParser / ListDecoders mechanism is quite similar to KVParser and
KVDecoders. Since the "key" of the different KeyValue objects is now
ommited, it has to be provided by ListDecoders.
For example, take the openflow action "resubmit" that can be written as:
resubmit([port],[table][,ct])
Can be decoded by creating a ListDecoders instance such as:
ListDecoders([
("port", decode_default),
("table", decode_int),
("ct", decode_flag),
])
Naturally, the order of the decoders must be kept.
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/automake.mk | 1 +
python/ovs/flows/list.py | 121 +++
2 files changed, 122 insertions(+)
create mode 100644 python/ovs/flows/list.py
diff --git a/python/automake.mk b/python/automake.mk
index 7ce842d66..73438d615 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -29,6 +29,7 @@ ovs_pyfiles = \
python/ovs/flows/__init__.py \
python/ovs/flows/decoders.py \
python/ovs/flows/kv.py \
+ python/ovs/flows/list.py \
python/ovs/json.py \
python/ovs/jsonrpc.py \
python/ovs/ovsuuid.py \
diff --git a/python/ovs/flows/list.py b/python/ovs/flows/list.py
new file mode 100644
index 0..57e7c5908
--- /dev/null
+++ b/python/ovs/flows/list.py
@@ -0,0 +1,121 @@
+import re
+
+from ovs.flows.kv import KeyValue, KeyMetadata, ParseError
+from ovs.flows.decoders import decode_default
+
+
+class ListDecoders(object):
+"""ListDecoders is used by ListParser to decode the elements in the list.
+
+A decoder is a function that accepts a value and returns its decoded
+object.
+
+ListDecoders is initialized with a list of tuples that contains the
+keyword and the decoding function associated with each position in the
+list. The order is, therefore, important.
+
+Args:
+decoders (list of tuples): Optional; a list of tuples.
+The first element in the tuple is the keyword associated with the
+value. The second element in the tuple is the decoder function.
+"""
+
+def __init__(self, decoders=None):
+self._decoders = decoders or list()
+
+def decode(self, index, value_str):
+"""Decode the index'th element of the list.
+
+Args:
+index (int): The position in the list of the element to decode.
+value_str (str): The value string to decode.
+"""
+if index < 0 or index >= len(self._decoders):
+return self._default_decoder(index, value_str)
+
+try:
+key = self._decoders[index][0]
+value = self._decoders[index][1](value_str)
+return key, value
+except Exception as e:
+raise ParseError(
+"Failed to decode value_str {}: {}".format(value_str, str(e))
+)
+
+@staticmethod
+def _default_decoder(index, value):
+key = "elem_{}".format(index)
+return key, decode_default(value)
+
+
+class ListParser(object):
+"""ListParser parses a list of values and stores them as key-value pairs.
+
+It uses a ListDecoders instance to decode each element in the list.
+
+Args:
+string (str): The string to parse.
+decoders (ListDecoders): Optional, the decoders to use.
+delims (list): Optional, list of delimiters of the list. Defaults to
+[','].
+"""
+def __init__(self, string, decoders=None, delims=[","]):
+self._string = string
+self._decoders = decoders or ListDecoders()
+self._keyval = list()
+self._regexp = r"({})".format("|".join(delims))
+
+def kv(self):
+return self._keyval
+
+def __iter__(self):
+return iter(self._keyval)
+
+def parse(self):
+"""Parse the list in string.
+
+Raises:
+ParseError if any parsing error occurs.
+"""
+kpos = 0
+index = 0
+while kpos < len(self._string) and self._string[kpos] != "\n":
+split_parts = re.split(self._regexp, self._string[kpos:], 1)
+value_str = split_parts[0]
+
+key, value = self._decoders.decode(index, value_str)
+
+meta = KeyMetadata(
+kpos=kpos,
+vpos=kpos,
+kstring=value_str,
+vstring=value_str,
+)
+self._keyval.append(KeyValue(key, value, meta))
+
+kpos += len(value_str) + 1
+index += 1
+
+
+def decode_nested_list(decoders, value, delims=[","]):
+"""Decodes a string value that contains a list of elements and returns
+them in a dictionary.
[ovs-dev] [PATCH v3 05/18] build-aux: generate ofp field decoders
Based on meta-field information extracted by extract_ofp_fields,
autogenerate the right decoder to be used.
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
build-aux/automake.mk| 6 ++-
build-aux/gen_ofp_field_decoders | 69
python/.gitignore| 1 +
python/automake.mk | 7
4 files changed, 81 insertions(+), 2 deletions(-)
create mode 100755 build-aux/gen_ofp_field_decoders
diff --git a/build-aux/automake.mk b/build-aux/automake.mk
index 6267ccd7c..b9a77a51c 100644
--- a/build-aux/automake.mk
+++ b/build-aux/automake.mk
@@ -5,6 +5,7 @@ EXTRA_DIST += \
build-aux/dist-docs \
build-aux/dpdkstrip.py \
build-aux/generate-dhparams-c \
+ build-aux/gen_ofp_field_decoders \
build-aux/initial-tab-allowed-files \
build-aux/sodepends.py \
build-aux/soexpand.py \
@@ -12,7 +13,8 @@ EXTRA_DIST += \
build-aux/xml2nroff
FLAKE8_PYFILES += \
-$(srcdir)/build-aux/xml2nroff \
build-aux/dpdkstrip.py \
+build-aux/gen_ofp_field_decoders \
build-aux/sodepends.py \
-build-aux/soexpand.py
+build-aux/soexpand.py \
+build-aux/xml2nroff
diff --git a/build-aux/gen_ofp_field_decoders b/build-aux/gen_ofp_field_decoders
new file mode 100755
index 0..3cc480042
--- /dev/null
+++ b/build-aux/gen_ofp_field_decoders
@@ -0,0 +1,69 @@
+#!/bin/env python
+
+import argparse
+
+import build.extract_ofp_fields as extract_fields
+
+
+def main():
+parser = argparse.ArgumentParser(
+description="Tool to generate python ofproto field decoders from"
+"meta-flow information"
+)
+parser.add_argument(
+"metaflow",
+metavar="FILE",
+type=str,
+help="Read meta-flow info from file",
+)
+
+args = parser.parse_args()
+
+fields = extract_fields.extract_ofp_fields(args.metaflow)
+
+field_decoders = {}
+for field in fields:
+decoder = get_decoder(field)
+field_decoders[field.get("name")] = decoder
+if field.get("extra_name"):
+field_decoders[field.get("extra_name")] = decoder
+
+code = """
+# This file is auto-generated. Do not edit!
+
+import functools
+from ovs.flows import decoders
+
+field_decoders = {{
+{decoders}
+}}
+""".format(
+decoders="\n".join(
+[
+"'{name}': {decoder},".format(name=name, decoder=decoder)
+for name, decoder in field_decoders.items()
+]
+)
+)
+print(code)
+
+
+def get_decoder(field):
+formatting = field.get("formatting")
+if formatting in ["decimal", "hexadecimal"]:
+if field.get("mask") == "MFM_NONE":
+return "decoders.decode_int"
+else:
+if field.get("n_bits") in [8, 16, 32, 64, 128, 992]:
+return "decoders.Mask{}".format(field.get("n_bits"))
+return "decoders.decode_mask({})".format(field.get("n_bits"))
+elif formatting in ["IPv4", "IPv6"]:
+return "decoders.IPMask"
+elif formatting == "Ethernet":
+return "decoders.EthMask"
+else:
+return "decoders.decode_default"
+
+
+if __name__ == "__main__":
+main()
diff --git a/python/.gitignore b/python/.gitignore
index 60ace6f05..c8ffd4574 100644
--- a/python/.gitignore
+++ b/python/.gitignore
@@ -1,2 +1,3 @@
dist/
*.egg-info
+ovs/flows/ofp_fields.py
diff --git a/python/automake.mk b/python/automake.mk
index d7d33928a..16d8db98f 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -125,3 +125,10 @@ $(srcdir)/python/ovs/dirs.py: python/ovs/dirs.py.template
mv $@.tmp $@
EXTRA_DIST += python/ovs/dirs.py.template
CLEANFILES += python/ovs/dirs.py
+
+$(srcdir)/python/ovs/flows/ofp_fields.py:
$(srcdir)/build-aux/gen_ofp_field_decoders include/openvswitch/meta-flow.h
+ $(AM_V_GEN)$(run_python) $< $(srcdir)/include/openvswitch/meta-flow.h >
$@.tmp
+ $(AM_V_at)mv $@.tmp $@
+EXTRA_DIST += python/ovs/flows/ofp_fields.py
+CLEANFILES += python/ovs/flows/ofp_fields.py
+
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 00/18] python: add flow parsing library
While troubleshooting or developing new features in OVS, a considerable
amount of time is spent analyzing flows (whether that's Openflow flows
or datapath flows). Currently, OVS has tools to dump flows with
different levels of verbosity as well as to filter flows prior to
dumping them, e.g: 'ovs-ofctl dump-flows', 'ovs-appctl
dpctl/dump-flows', etc.
The output of these commands is considered stable so it should be
possible to write more layered tools that enable advanced flow analysis.
However, the way flows are formatted into strings is not trivial to
parse.
This series introduces the a flow parsing library capable of parsing
both Openflow and DPIF flows.
The library is based on generic key-value and list parsers and a number
of common decoders. Based on that, an Openflow flow parser and a DPIF
flow parser are introduced by defining a way to decode each possible
field and action they might contain.
The library has the following features:
- Parsed key-value pairs keep some metadata that refer to the original
strings they were extracted from. That way the flows can be printed
and formatted in flexible ways.
- It includes a basic flow filtering mechanism. A filter can be defined
combining logical (||, &&, !), arithmetical (<, >, =) or mask (~=)
operations
- It supports IPAddress and Ethernet masking (based on netaddr)
- The decoder to use for each key (match or action) is set explicitly to
avoid expensive runtime type-guessing.
- The decoders to use for Openflow fields is automatically generated
based on meta-flow.h
- Additional dependencies:
- netaddr: For IP and Ethernet Address management.
- pyparsing: For filtering syntax.
- pytest: For unit tests.
One key goal challenge of including this library is avoiding diversion
between the C code that prints/parses the flows and the python parsing
code. To that effect, the series introduces the following mechanisms:
- Decoding information of openflow fields is automatically generated
based on meta-flow.h
- The calls to ovs-ofctl made from tests/ofp-actions.at are wrapped by a
python script that also runs the python parsers. If an exception is
raised by the python code (meaning it was not capable of parsing the
flow string), the test will fail
- The calls to the test-odp made from tests/odp.at are wrapped by a
python script that also runs the python parsers. If an exception is
raised by the python code (meaning it was not capable of parsing the
flow string), the test will fail.
- A python unit test testsuite ensures python code works and it's easy
to add more flow examples to it
- A dependency check is introduced. The python parsing code mainly
depends on lib/ofp-actions.c and lib/odp-util.c. This series stores
the md5sum of such files and adds a build target that ensures the
files have not been changed. That way, anyone who modifies those files
will get a warning the fist time they build the project. Dependency
digests are easily updated using a string so hopefully this warning
would not be too inconvenient.
Library usage
-
>>> from ovs.flows.ofp import OFPFlow
>>> flow = OFPFlow("cookie=0x2b32ab4d, table=41, n_packets=11, n_bytes=462,
>>> priority=33,ip,reg15=0x2/0x2,nw_src=10.128.0.2/24
>>> actions=move:NXM_OF_TCP_DST[]->NXM_NX_XXREG0[32..47],ct(table=16,zone=NXM_NX_REG13[0..15],nat)")
>>> flow.info
{'cookie': 724740941, 'table': 41, 'n_packets': 11, 'n_bytes': 462}
>>> flow.match
{'priority': 33, 'ip': True, 'reg15': Mask32('0x2/0x2'), 'nw_src':
IPMask('10.128.0.2/24')}
>>> flow.actions
[{'move': {'src': {'field': 'NXM_OF_TCP_DST'}, 'dst': {'field':
'NXM_NX_XXREG0', 'start': 32, 'end': 47}}}, {'ct': {'table': 16, 'zone':
{'field': 'NXM_NX_REG13', 'start': 0, 'end': 15}, 'nat': True}}]
>>> from ovs.flows.filter import OFFilter
>>> filt = OFFilter("nw_src ~= 10.128.0.10 and (table = 42 or n_packets > 0)")
>>> filt.evaluate(flow)
True
V2 -> V3:
- Simplified KV and list decoding code (Mark Michelson's suggestion)
- Fixed typos
- Added missing files to FLAKE8_PYFILES
- Go back to a simplified ipv4/6 regexp for ip-port range extraction.
Also added specific unit test for ip-port range decoding.
- Adapt ofp encap() action decoder to support new header types: mpls and mpls_mc
(the need for change was detected by patch 13)
V1 -> V2:
- list/kv parsers: changed the API to accept the string to parse in the
constructor.
- list/kv parsers: allow re.split() to return less than 3 elements
(enables support for python 3.6).
- decoders: add a more accurate IPv6 regexp and remove confusing max()
in IPMask and EthMask.
- odp/ofp flows: remove the *Factory class and implement caching of
decoders using class variables.
- odp/ofp flows: homogenize names of functions and made them static.
- moved pytest unit tests from a build target to a testsuite and added
their requirements to python/test_requirements.txt.
- Formatting fixes and missing dots (lots of them!).
RFC -> V1:
- filters: created a class to
[ovs-dev] [PATCH v3 01/18] python: add generic Key-Value parser
Most of ofproto and dpif flows are based on key-value pairs. These key-value pairs can be represented in several ways, eg: key:value, key=value, key(value). Add the following classes that allow parsing of key-value strings: * KeyValue: holds a key-value pair * KeyMetadata: holds some metadata associated with a KeyValue such as the original key and value strings and their position in the global string * KVParser: is able to parse a string and extract it's key-value pairs as KeyValue instances. Before creating the KeyValue instance it tries to decode the value via the KVDecoders * KVDecoders holds a number of decoders that KVParser can use to decode key-value pairs. It accepts a dictionary of keys and callables to allow users to specify what decoder (i.e: callable) to use for each key Also, flake8 seems to be incorrectly reporting an error (E203) in: "slice[index + offset : index + offset]" which is PEP8 compliant. So, ignore this error. Signed-off-by: Adrian Moreno --- Makefile.am | 3 +- python/automake.mk | 6 +- python/ovs/flows/__init__.py | 0 python/ovs/flows/decoders.py | 18 ++ python/ovs/flows/kv.py | 314 +++ python/setup.py | 2 +- 6 files changed, 340 insertions(+), 3 deletions(-) create mode 100644 python/ovs/flows/__init__.py create mode 100644 python/ovs/flows/decoders.py create mode 100644 python/ovs/flows/kv.py diff --git a/Makefile.am b/Makefile.am index cb8076433..4f51d225e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -391,6 +391,7 @@ ALL_LOCAL += flake8-check # E128 continuation line under-indented for visual indent # E129 visually indented line with same indent as next logical line # E131 continuation line unaligned for hanging indent +# E203 whitespace before ':' # E722 do not use bare except, specify exception instead # W503 line break before binary operator # W504 line break after binary operator @@ -403,7 +404,7 @@ ALL_LOCAL += flake8-check # H233 Python 3.x incompatible use of print operator # H238 old style class declaration, use new style (inherit from `object`) FLAKE8_SELECT = H231,H232,H233,H238 -FLAKE8_IGNORE = E121,E123,E125,E126,E127,E128,E129,E131,E722,W503,W504,F811,D,H,I +FLAKE8_IGNORE = E121,E123,E125,E126,E127,E128,E129,E131,E203,E722,W503,W504,F811,D,H,I flake8-check: $(FLAKE8_PYFILES) $(FLAKE8_WERROR)$(AM_V_GEN) \ src='$^' && \ diff --git a/python/automake.mk b/python/automake.mk index 767512f17..7ce842d66 100644 --- a/python/automake.mk +++ b/python/automake.mk @@ -16,7 +16,6 @@ ovs_pyfiles = \ python/ovs/compat/sortedcontainers/sorteddict.py \ python/ovs/compat/sortedcontainers/sortedset.py \ python/ovs/daemon.py \ - python/ovs/fcntl_win.py \ python/ovs/db/__init__.py \ python/ovs/db/custom_index.py \ python/ovs/db/data.py \ @@ -26,6 +25,10 @@ ovs_pyfiles = \ python/ovs/db/schema.py \ python/ovs/db/types.py \ python/ovs/fatal_signal.py \ + python/ovs/fcntl_win.py \ + python/ovs/flows/__init__.py \ + python/ovs/flows/decoders.py \ + python/ovs/flows/kv.py \ python/ovs/json.py \ python/ovs/jsonrpc.py \ python/ovs/ovsuuid.py \ @@ -42,6 +45,7 @@ ovs_pyfiles = \ python/ovs/version.py \ python/ovs/vlog.py \ python/ovs/winutils.py + # These python files are used at build time but not runtime, # so they are not installed. EXTRA_DIST += \ diff --git a/python/ovs/flows/__init__.py b/python/ovs/flows/__init__.py new file mode 100644 index 0..e69de29bb diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py new file mode 100644 index 0..0c2259c76 --- /dev/null +++ b/python/ovs/flows/decoders.py @@ -0,0 +1,18 @@ +"""Defines helpful decoders that can be used to decode information from the +flows. + +A decoder is generally a callable that accepts a string and returns the value +object. +""" + + +def decode_default(value): +"""Default decoder. + +It tries to convert into an integer value and, if it fails, just +returns the string. +""" +try: +return int(value, 0) +except ValueError: +return value diff --git a/python/ovs/flows/kv.py b/python/ovs/flows/kv.py new file mode 100644 index 0..9fd87c3cc --- /dev/null +++ b/python/ovs/flows/kv.py @@ -0,0 +1,314 @@ +"""Common helper classes for flow Key-Value parsing.""" + +import functools +import re + +from ovs.flows.decoders import decode_default + + +class ParseError(RuntimeError): +"""Exception raised when an error occurs during parsing.""" + +pass + + +class KeyMetadata(object): +"""Class for keeping key metadata. + +Attributes: +kpos (int): The position of the keyword in the parent string. +vpos (int): The position of the value in the parent string. +kstring (string): The keyword string as found in the flow
[ovs-dev] [PATCH v3 02/18] python: add mask, ip and eth decoders
Add more decoders that can be used by KVParser.
For IPv4 and IPv6 addresses, create a new class that wraps
netaddr.IPAddress.
For Ethernet addresses, create a new class that wraps netaddr.EUI.
For Integers, create a new class that performs basic bitwise mask
comparisons
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/ovs/flows/decoders.py | 398 +++
python/setup.py | 2 +-
2 files changed, 399 insertions(+), 1 deletion(-)
diff --git a/python/ovs/flows/decoders.py b/python/ovs/flows/decoders.py
index 0c2259c76..883e61acf 100644
--- a/python/ovs/flows/decoders.py
+++ b/python/ovs/flows/decoders.py
@@ -5,6 +5,15 @@ A decoder is generally a callable that accepts a string and
returns the value
object.
"""
+import netaddr
+
+
+class Decoder(object):
+"""Base class for all decoder classes."""
+
+def to_json(self):
+raise NotImplementedError()
+
def decode_default(value):
"""Default decoder.
@@ -16,3 +25,392 @@ def decode_default(value):
return int(value, 0)
except ValueError:
return value
+
+
+def decode_flag(value):
+"""Decode a flag. It's existence is just flagged by returning True."""
+return True
+
+
+def decode_int(value):
+"""Integer decoder.
+
+Both base10 and base16 integers are supported.
+
+Used for fields such as:
+n_bytes=34
+metadata=0x4
+"""
+return int(value, 0)
+
+
+def decode_time(value):
+"""Time decoder.
+
+Used for fields such as:
+duration=1234.123s
+"""
+if value == "never":
+return value
+
+time_str = value.rstrip("s")
+return float(time_str)
+
+
+class IntMask(Decoder):
+"""Base class for Integer Mask decoder classes.
+
+It supports decoding a value/mask pair. The class has to be derived,
+and the size attribute must be set.
+"""
+
+size = None # Size in bits.
+
+def __init__(self, string):
+if not self.size:
+raise NotImplementedError(
+"IntMask should be derived and size should be fixed"
+)
+
+parts = string.split("/")
+if len(parts) > 1:
+self._value = int(parts[0], 0)
+self._mask = int(parts[1], 0)
+if self._mask.bit_length() > self.size:
+raise ValueError(
+"Integer mask {} is bigger than size {}".format(
+self._mask, self.size
+)
+)
+else:
+self._value = int(parts[0], 0)
+self._mask = self.max_mask()
+
+if self._value.bit_length() > self.size:
+raise ValueError(
+"Integer value {} is bigger than size {}".format(
+self._value, self.size
+)
+)
+
+@property
+def value(self):
+return self._value
+
+@property
+def mask(self):
+return self._mask
+
+def max_mask(self):
+return 2 ** self.size - 1
+
+def fully(self):
+"""Returns True if it's fully masked."""
+return self._mask == self.max_mask()
+
+def __str__(self):
+if self.fully():
+return str(self._value)
+else:
+return "{}/{}".format(hex(self._value), hex(self._mask))
+
+def __repr__(self):
+return "%s('%s')" % (self.__class__.__name__, self)
+
+def __eq__(self, other):
+"""Equality operator.
+
+Both value and mask must be the same for the comparison to result True.
+This can be used to implement filters that expect a specific mask,
+e.g: ct.state = 0x1/0xff.
+
+Args:
+other (IntMask): Another IntMask to compare against.
+
+Returns:
+True if the other IntMask is the same as this one.
+"""
+if isinstance(other, IntMask):
+return self.value == other.value and self.mask == other.mask
+elif isinstance(other, int):
+return self.value == other and self.mask == self.max_mask()
+else:
+raise ValueError("Cannot compare against ", other)
+
+def __contains__(self, other):
+"""Contains operator.
+
+Args:
+other (int or IntMask): Another integer or fully-masked IntMask
+to compare against.
+
+Returns:
+True if the other integer or fully-masked IntMask is
+contained in this IntMask.
+
+Example:
+0x1 in IntMask("0xf1/0xff"): True
+0x1 in IntMask("0xf1/0x0f"): True
+0x1 in IntMask("0xf1/0xf0"): False
+"""
+if isinstance(other, IntMask):
+if other.fully():
+return other.value in self
+else:
+raise ValueError(
+"Comparing non fully-masked IntMasks is not supported"
+)
+else:
+return other & self._mask == self._value &
Re: [ovs-dev] [PATCH v3 3/5] pmd.at: Add tests for multi non-local numa pmds.
On 11/03/2022 13:23, Kevin Traynor wrote: Ensure that if there are no local numa pmd cores available that pmd cores from all other non-local numas will be used. NACK this patch. The numas are held in a hash map so order is not guaranteed but I mixed up what the pmd-rxq-show output was sorted on, so the test is potentially unreliable (even though it passed for me). As rest of series has acked fixes, if there are no more comments on them, I suggest move fwd with them and I'll send a better UT for this later. Signed-off-by: Kevin Traynor --- tests/pmd.at | 91 +++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/tests/pmd.at b/tests/pmd.at index a2f9d34a2..91af5bbd6 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -200,5 +200,5 @@ OVS_VSWITCHD_STOP AT_CLEANUP -AT_SETUP([PMD - pmd-cpu-mask - NUMA]) +AT_SETUP([PMD - pmd-cpu-mask - dual NUMA]) OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy-pmd options:n_rxq=8 options:numa_id=1 -- set Open_vSwitch . other_config:pmd-cpu-mask=1], [], [], [--dummy-numa 1,1,0,0]) @@ -360,4 +360,93 @@ OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([PMD - pmd-cpu-mask - multi NUMA]) +OVS_VSWITCHD_START([add-port br0 p0 \ +-- set Interface p0 type=dummy-pmd options:n_rxq=4 \ +-- set Interface p0 options:numa_id=0 \ +-- set Open_vSwitch . other_config:pmd-cpu-mask=0xf \ +-- set open_vswitch . other_config:pmd-rxq-assign=cycles], + [], [], [--dummy-numa 1,2,1,2]) + +TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1)) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=group]) + +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx queue assignment using group algorithm"]) +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available (non-isolated) pmd thread on numa node 0."]) + +# check pmds from both non-local numas are used +AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 0 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 1 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 2 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 3 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +]) + +TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1)) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=cycles]) + +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx queue assignment using cycles algorithm"]) +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available (non-isolated) pmd thread on numa node 0."]) + +# check pmds from both non-local numas are used +AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 0 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 1 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 2 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 3 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +]) + +TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1)) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=roundrobin]) + +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx queue assignment using roundrobin algorithm"]) +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available (non-isolated) pmd thread on numa node 0."]) + +# check pmds from both non-local numas are used +AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 0 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 1 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 2 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 3 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +]) +
[ovs-dev] [PATCH v3 5/5] alb.ut: Add tests for cross-numa polling.
PMD auto load balance currently only operates when the polling pmd core will not change numa after reassignment. Add a unit test for this. Signed-off-by: Kevin Traynor Acked-by: Mike Pattrick --- tests/alb.at | 46 ++ 1 file changed, 46 insertions(+) diff --git a/tests/alb.at b/tests/alb.at index 2bef06f39..0036bd1f2 100644 --- a/tests/alb.at +++ b/tests/alb.at @@ -97,4 +97,50 @@ OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ALB - cross-numa]) +OVS_VSWITCHD_START([add-port br0 p0 \ +-- set Interface p0 type=dummy-pmd options:n_rxq=4 \ +-- set Interface p0 options:numa_id=0 \ +-- set Open_vSwitch . other_config:pmd-cpu-mask=0x3 \ +-- set open_vswitch . other_config:pmd-rxq-assign=group \ +-- set open_vswitch . other_config:pmd-rxq-isolate=false \ +-- set open_vswitch . other_config:pmd-auto-lb="true" \ +-- set open_vswitch . other_config:pmd-auto-lb-load-threshold=0], + [], [], [--dummy-numa 1,2,1,2]) +OVS_WAIT_UNTIL([grep "PMD auto load balance is enabled" ovs-vswitchd.log]) +AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg]) + +# no pinned rxqs - cross-numa pmd could change +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance detected cross-numa polling"]) + +# all pinned rxqs - cross-numa pmd will not change +AT_CHECK([ovs-vsctl set Interface p0 other_config:pmd-rxq-affinity='0:0,1:0,2:1,3:1']) +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "Variance improvement 0%."]) + +# mix of pinned (non-isolated) and non-pinned rxqs - cross-numa pmd could change +AT_CHECK([ovs-vsctl remove Interface p0 other_config pmd-rxq-affinity]) +AT_CHECK([ovs-vsctl set Interface p0 other_config:pmd-rxq-affinity='0:0,1:0,2:1']) +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance detected cross-numa polling"]) + +# mix of pinned (isolated) and non-pinned rxqs - cross-numa pmd could change +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0xf]) +AT_CHECK([ovs-vsctl set Interface p0 options:n_rxq=6]) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-isolate=true]) +get_log_next_line_num +ovs-appctl time/warp 60 1 +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."]) +OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance detected cross-numa polling"]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ALB - PMD/RxQ assignment type]) OVS_VSWITCHD_START([add-port br0 p0 \ -- 2.34.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 4/5] dpif-netdev: Fix PMD auto load balance with pmd-rxq-isolate.
There are currently some checks for cross-numa polling cases to
ensure that they won't effect the accuracy of the PMD ALB.
If an rxq is pinned to a pmd core by the user it will not be
reassigned by OVS, so even if it is non-local numa polled it
will not impact PMD ALB accuracy.
To establish this, a check was made on whether the pmd core was
isolated or not. However, since other_config:pmd-rxq-isolate was
introduced, rxqs may be pinned but the pmd core not isolated.
It means that by setting pmd-rxq-isolate=false and doing non-local
numa pinning, PMD ALB may not run where it should.
If the core is isolated we can skip individual rxq checks but if not,
we should check the individual rxqs for pinning before we disallow
PMD ALB.
Also, update function comments to make it's operation clearer.
Fixes: 6193e03267c1 ("dpif-netdev: Allow pin rxq and non-isolate PMD.")
Signed-off-by: Kevin Traynor
---
lib/dpif-netdev.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9ae488d6e..dce4a6d42 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5685,4 +5685,6 @@ sched_numa_list_put_in_place(struct sched_numa_list
*numa_list)
}
+/* Returns 'true' if OVS rxq scheduling algorithm assigned any unpinned rxq to
+ * a pmd core on a non-local numa node. */
static bool
sched_numa_list_cross_numa_polling(struct sched_numa_list *numa_list)
@@ -5690,16 +5692,19 @@ sched_numa_list_cross_numa_polling(struct
sched_numa_list *numa_list)
struct sched_numa *numa;
-/* For each numa */
HMAP_FOR_EACH (numa, node, _list->numas) {
-/* For each pmd */
for (int i = 0; i < numa->n_pmds; i++) {
struct sched_pmd *sched_pmd;
sched_pmd = >pmds[i];
-/* For each rxq. */
+if (sched_pmd->isolated) {
+/* All rxqs on this pmd core are pinned. */
+continue;
+}
for (unsigned k = 0; k < sched_pmd->n_rxq; k++) {
struct dp_netdev_rxq *rxq = sched_pmd->rxqs[k];
-
-if (!sched_pmd->isolated &&
+/* Check if the rxq is not pinned to a specific pmd core by the
+ * user AND the pmd core that OVS assigned is non-local to the
+ * rxq port. */
+if (rxq->core_id == OVS_CORE_UNSPEC &&
rxq->pmd->numa_id !=
netdev_get_numa_id(rxq->port->netdev)) {
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 3/5] pmd.at: Add tests for multi non-local numa pmds.
Ensure that if there are no local numa pmd cores available that pmd cores from all other non-local numas will be used. Signed-off-by: Kevin Traynor --- tests/pmd.at | 91 +++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/tests/pmd.at b/tests/pmd.at index a2f9d34a2..91af5bbd6 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -200,5 +200,5 @@ OVS_VSWITCHD_STOP AT_CLEANUP -AT_SETUP([PMD - pmd-cpu-mask - NUMA]) +AT_SETUP([PMD - pmd-cpu-mask - dual NUMA]) OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy-pmd options:n_rxq=8 options:numa_id=1 -- set Open_vSwitch . other_config:pmd-cpu-mask=1], [], [], [--dummy-numa 1,1,0,0]) @@ -360,4 +360,93 @@ OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([PMD - pmd-cpu-mask - multi NUMA]) +OVS_VSWITCHD_START([add-port br0 p0 \ +-- set Interface p0 type=dummy-pmd options:n_rxq=4 \ +-- set Interface p0 options:numa_id=0 \ +-- set Open_vSwitch . other_config:pmd-cpu-mask=0xf \ +-- set open_vswitch . other_config:pmd-rxq-assign=cycles], + [], [], [--dummy-numa 1,2,1,2]) + +TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1)) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=group]) + +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx queue assignment using group algorithm"]) +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available (non-isolated) pmd thread on numa node 0."]) + +# check pmds from both non-local numas are used +AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 0 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 1 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 2 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 3 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +]) + +TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1)) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=cycles]) + +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx queue assignment using cycles algorithm"]) +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available (non-isolated) pmd thread on numa node 0."]) + +# check pmds from both non-local numas are used +AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 0 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 1 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 2 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 3 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +]) + +TMP=$(($(cat ovs-vswitchd.log | wc -l | tr -d [[:blank:]])+1)) +AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=roundrobin]) + +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Performing pmd to rx queue assignment using roundrobin algorithm"]) +OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "There's no available (non-isolated) pmd thread on numa node 0."]) + +# check pmds from both non-local numas are used +AT_CHECK([ovs-appctl dpif-netdev/pmd-rxq-show | sed SED_NUMA_CORE_PATTERN], [0], [dnl +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 0 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 1 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 2 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +pmd thread numa_id core_id : + isolated : false + port: p0queue-id: 3 (enabled) pmd usage: NOT AVAIL + overhead: NOT AVAIL +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([PMD - stats]) OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 ofport_request=7 type=dummy-pmd options:n_rxq=4], -- 2.34.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 2/5] dpif-netdev: Fix non-local numa selection.
In the event of no pmd cores available on the local numa for
an rxq to be assigned to, a pmd core from a non-local numa is
selected.
If there are more than one non-local numas with pmd cores they
are RR through and checked if they have non-isolated pmds.
When successfully finding a non-local numa with available pmds
for an rxq, that numa was not being stored. It meant if a similar
situation occurred for a subsequent rxq, the same numa would be
selected again.
Store the last numa used when successfully finding a non-local numa
with available pmds, so the numa RR state is kept for subsequent rxqs.
Fixes: f577c2d046b2 ("dpif-netdev: Rework rxq scheduling code.")
Signed-off-by: Kevin Traynor
Acked-by: Mike Pattrick
---
lib/dpif-netdev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8a3047439..9ae488d6e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6001,8 +6001,8 @@ sched_numa_list_schedule(struct sched_numa_list
*numa_list,
for (int j = 0; j < n_numa; j++) {
numa = sched_numa_list_next(numa_list, last_cross_numa);
-if (sched_numa_noniso_pmd_count(numa)) {
-break;
-}
last_cross_numa = numa;
+if (sched_numa_noniso_pmd_count(numa)) {
+break;
+}
numa = NULL;
}
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 1/5] dpif-netdev: Fix typo in function name.
Rename pmd_reblance_dry_run_needed() to
pmd_rebalance_dry_run_needed().
Fixes: a83a406096e9 ("dpif-netdev: Sync PMD ALB state with user commands.")
Signed-off-by: Kevin Traynor
Acked-by: Mike Pattrick
---
lib/dpif-netdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9f35713ef..8a3047439 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6112,5 +6112,5 @@ sched_numa_list_variance(struct sched_numa_list
*numa_list)
*/
static bool
-pmd_reblance_dry_run_needed(struct dp_netdev *dp)
+pmd_rebalance_dry_run_needed(struct dp_netdev *dp)
OVS_REQ_RDLOCK(dp->port_rwlock)
{
@@ -6683,5 +6683,5 @@ dpif_netdev_run(struct dpif *dpif)
!dp_netdev_is_reconf_required(dp) &&
!ports_require_restart(dp) &&
-pmd_reblance_dry_run_needed(dp) &&
+pmd_rebalance_dry_run_needed(dp) &&
pmd_rebalance_dry_run(dp)) {
VLOG_INFO("PMD auto load balance dry run. "
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 0/5] ALB/Rxq scheduling cross-numa polling fixes.
Some fixes found in the area of cross-numa polling during recent code inspection and testing. v3: - Add additional unit test (new patch 3/5) for rxq scheduling to ensure that all non-local numas are used. - Other patches unchanged v2: - Updated comments in 3/4 to make the fn operation clearer. - Added Acks on other patches GHA: https://github.com/kevintraynor/ovs/actions/runs/1968938674 Kevin Traynor (5): dpif-netdev: Fix typo in function name. dpif-netdev: Fix non-local numa selection. pmd.at: Add tests for multi non-local numa pmds. dpif-netdev: Fix PMD auto load balance with pmd-rxq-isolate. alb.ut: Add tests for cross-numa polling. lib/dpif-netdev.c | 25 +++-- tests/alb.at | 46 tests/pmd.at | 91 ++- 3 files changed, 151 insertions(+), 11 deletions(-) -- 2.34.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 11 Mar 2022, at 3:14, Chris Mi wrote:
> On 2022-03-04 6:50 PM, Eelco Chaudron wrote:
>> I’m re-adding some of my v18 comments, which I know are depending on Ilya’s
>> feedback, although I still think we should have provider classes, i.e., not
>> based on the datapath class.
>>
>> Some other questions could still be answered regardless of Ilya’s feedback
>> (will mark them with **). Please answer them inline, before sending another
>> revision so we can have some discussion around them if needed.
> OK.
>>
>> //Eelco
>>
>> On 15 Feb 2022, at 10:17, Chris Mi wrote:
>>
>>> Implement dpif-offload API for netlink datapath. And implement a
>>> dummy dpif-offload API for netdev datapath to make tests pass.
>>
>> ** Why do we need a dummy? If there is no hardware offload class we should
>> just pass, shouldn’t we?
> According to one of your below comment, I moved dp_offload_class_lookup from
> dpif to dpif-netlink.
> After that, dpif-offload for dummy is not needed.
>>
>>> Issue: 2181036
>>> Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
>>> Signed-off-by: Chris Mi
>>> Reviewed-by: Eli Britstein
>>> ---
>>> lib/automake.mk | 2 +
>>> lib/dpif-netdev.c | 8 +-
>>> lib/dpif-netlink.c | 4 +-
>>> lib/dpif-offload-netdev.c | 43 +++
>>> lib/dpif-offload-netlink.c | 221
>>> lib/dpif-offload-provider.h | 25 +++-
>>> lib/dpif-offload.c | 157 +
>>> lib/dpif-provider.h | 6 +-
>>> lib/dpif.c | 17 ++-
>>> 9 files changed, 476 insertions(+), 7 deletions(-)
>>> create mode 100644 lib/dpif-offload-netdev.c
>>> create mode 100644 lib/dpif-offload-netlink.c
>>>
>>> diff --git a/lib/automake.mk b/lib/automake.mk
>>> index 781fba47a..9ed61029a 100644
>>> --- a/lib/automake.mk
>>> +++ b/lib/automake.mk
>>> @@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
>>> lib/dpif-netdev-perf.c \
>>> lib/dpif-netdev-perf.h \
>>> lib/dpif-offload.c \
>>> + lib/dpif-offload-netdev.c \
>>> lib/dpif-offload-provider.h \
>>> lib/dpif-provider.h \
>>> lib/dpif.c \
>>> @@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
>>> lib/dpif-netlink.h \
>>> lib/dpif-netlink-rtnl.c \
>>> lib/dpif-netlink-rtnl.h \
>>> + lib/dpif-offload-netlink.c \
>>> lib/if-notifier.c \
>>> lib/netdev-linux.c \
>>> lib/netdev-linux.h \
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index e28e0b554..5ebd127b9 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
>>> ovs_refcount_ref(>ref_cnt);
>>>
>>> dpif = xmalloc(sizeof *dpif);
>>> -dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8,
>>> netflow_id);
>>> +dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
>>> + netflow_id);
>>> dpif->dp = dp;
>>> dpif->last_port_seq = seq_read(dp->port_seq);
>>>
>>> @@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
>>> if (error == 0 || error == EAFNOSUPPORT) {
>>> dpif_dummy_register__(type);
>>> }
>>> +error = dp_offload_unregister_provider(type);
>>> +if (error == 0 || error == EAFNOSUPPORT) {
>>> +dpif_offload_dummy_register(type);
>>> +}
>>> }
>>>
>>> void
>>> @@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
>>> }
>>>
>>> dpif_dummy_register__("dummy");
>>> +dpif_offload_dummy_register("dummy");
>>>
>>> unixctl_command_register("dpif-dummy/change-port-number",
>>>"dp port new-number",
>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>>> index 71e35ccdd..75f85c254 100644
>>> --- a/lib/dpif-netlink.c
>>> +++ b/lib/dpif-netlink.c
>>> @@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
>>> **dpifp)
>>> dpif->port_notifier = NULL;
>>> fat_rwlock_init(>upcall_lock);
>>>
>>> -dpif_init(>dpif, _netlink_class, dp->name,
>>> - dp->dp_ifindex, dp->dp_ifindex);
>>> +dpif_init(>dpif, _netlink_class,
>>> _offload_netlink_class,
>>> + dp->name, dp->dp_ifindex, dp->dp_ifindex);
>>>
>>> dpif->dp_ifindex = dp->dp_ifindex;
>>> dpif->user_features = dp->user_features;
>>> diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
>>> new file mode 100644
>>> index 0..35dac2cc3
>>> --- /dev/null
>>> +++ b/lib/dpif-offload-netdev.c
>>> @@ -0,0 +1,43 @@
>>> +/*
>>> + * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights
>>> reserved.
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at:
>>> + *
>>> + *
>>>
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix netdev native tunnel neigh discovery spa
On 14 Dec 2021, at 4:59, we...@ucloud.cn wrote:
> From: wenxu
>
> netdev native tunnel encap process, when tunnel neighbor cache miss,
> send arp/nd request. Before spa use tunnel source, change to:
> find the spa which have the same subset with the nexthop of tunnel dst
> on egress port, if false, use the tunnel src as spa.
>
> For example:
> tunnel src is a vip with 10.0.0.7/32, tunnel dst is 10.0.1.7
> the br-phy with address 192.168.0.7/24 and the default gateway is 192.168.0.1
> So the spa of arp request for 192.168.0.1 should be 192.168.0.7 but not
> 10.0.0.7
>
> Signed-off-by: wenxu
> ---
> lib/ovs-router.c | 2 +-
> lib/ovs-router.h | 2 ++
> ofproto/ofproto-dpif-xlate.c | 12 +---
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 09b81c6..6d3c731 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -164,7 +164,7 @@ static void rt_init_match(struct match *match, uint32_t
> mark,
> match->flow.pkt_mark = mark;
> }
>
> -static int
> +int
> get_src_addr(const struct in6_addr *ip6_dst,
Rather than directly expose this function as is, maybe we should have a more
generic name. For example:
ovs_router_get_netdev_source_address()
> const char output_bridge[], struct in6_addr *psrc)
> {
> diff --git a/lib/ovs-router.h b/lib/ovs-router.h
> index 34ea163..f7ce7cf 100644
> --- a/lib/ovs-router.h
> +++ b/lib/ovs-router.h
> @@ -26,6 +26,8 @@
> extern "C" {
> #endif
>
> +int get_src_addr(const struct in6_addr *ip6_dst,
> + const char output_bridge[], struct in6_addr *psrc);
> bool ovs_router_lookup(uint32_t mark, const struct in6_addr *ip_dst,
> char out_dev[],
> struct in6_addr *src, struct in6_addr *gw);
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 9d336bc..c556312 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3603,9 +3603,10 @@ native_tunnel_output(struct xlate_ctx *ctx, const
> struct xport *xport,
> struct netdev_tnl_build_header_params tnl_params;
> struct ovs_action_push_tnl tnl_push_data;
> struct xport *out_dev = NULL;
> -ovs_be32 s_ip = 0, d_ip = 0;
> +ovs_be32 s_ip = 0, d_ip = 0, neigh_s_ip = 0;
It’s not really a neighbor source IP, I would call it nh_s_ip (nh = next-hop).
> struct in6_addr s_ip6 = in6addr_any;
> struct in6_addr d_ip6 = in6addr_any;
> +struct in6_addr neigh_s_ip6 = in6addr_any;
> struct eth_addr smac;
> struct eth_addr dmac;
> int err;
> @@ -3646,8 +3647,13 @@ native_tunnel_output(struct xlate_ctx *ctx, const
> struct xport *xport,
> }
>
> d_ip = in6_addr_get_mapped_ipv4(_ip6);
> +err = get_src_addr(_ip6, out_dev->xbridge->name, _s_ip6);
> +if (err) {
> +neigh_s_ip6 = s_ip6;
> +}
> if (d_ip) {
> s_ip = in6_addr_get_mapped_ipv4(_ip6);
> +neigh_s_ip = in6_addr_get_mapped_ipv4(_s_ip6);
> }
>
> err = tnl_neigh_lookup(out_dev->xbridge->name, _ip6, );
> @@ -3657,9 +3663,9 @@ native_tunnel_output(struct xlate_ctx *ctx, const
> struct xport *xport,
> "sending %s request",
> buf_dip6, out_dev->xbridge->name, d_ip ? "ARP" : "ND");
Maybe we can move all the d_ip handling to the “error” path? This makes it more
compact. For example:
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index cc9c1c628..e416a15be 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3673,14 +3673,25 @@ native_tunnel_output(struct xlate_ctx *ctx, const
struct xport *xport,
err = tnl_neigh_lookup(out_dev->xbridge->name, _ip6, );
if (err) {
+struct in6_addr nh_s_ip6 = in6addr_any;
+
xlate_report(ctx, OFT_DETAIL,
"neighbor cache miss for %s on bridge %s, "
"sending %s request",
buf_dip6, out_dev->xbridge->name, d_ip ? "ARP" : "ND");
+
+err = get_src_addr(_ip6, out_dev->xbridge->name, _s_ip6);
+if (err) {
+nh_s_ip6 = s_ip6;
+}
+
if (d_ip) {
-tnl_send_arp_request(ctx, out_dev, smac, s_ip, d_ip);
+ovs_be32 nh_s_ip;
+
+nh_s_ip = in6_addr_get_mapped_ipv4(_s_ip6);
+tnl_send_arp_request(ctx, out_dev, smac, nh_s_ip, d_ip);
} else {
-tnl_send_nd_request(ctx, out_dev, smac, _ip6, _ip6);
+tnl_send_nd_request(ctx, out_dev, smac, _s_ip6, _ip6);
}
return err;
}
> if (d_ip) {
> -tnl_send_arp_request(ctx, out_dev, smac, s_ip, d_ip);
> +tnl_send_arp_request(ctx, out_dev, smac, neigh_s_ip, d_ip);
> } else {
> -tnl_send_nd_request(ctx, out_dev, smac, _ip6, _ip6);
> +tnl_send_nd_request(ctx, out_dev, smac, _s_ip6, _ip6);
> }
>
Re: [ovs-dev] [PATCH] ovsdb-cluster.at: Avoid test failures due to different hashing.
On 24 Feb 2022, at 20:36, Ilya Maximets wrote: > Depending on compiler flags and CPU architecture different hash > function are used. That impacts the order of tables and columns > in database representation making ovsdb report different columns > in the warning about ephemeral-to-persistent conversion. > > Stripping out changing parts of the message to avoid the issue. > > Signed-off-by: Ilya Maximets > --- Changes look good to me, tested by running make check-ovsdb-cluster a couple of times. Acked-by: Eelco Chaudron Cheers, Eelco ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
