Today users can enable prefix matches for tun_id, tun_src, tun_dst,
ip_src, ip_dst, ipv6_src and ipv6_dst.  However, they are limited to
only 3 of these enabled at the same time.  This means that if our flow
table is handling both IPv4 and IPv6 traffic, we can't optimize all
the addresses, we'll either have to split IPv4 and IPv6 rules into
separate tables or sacrifice one of the fields, as we can select only
3 out of 4 fields (ip_src, ip_dst, ipv6_src and ipv6_dst).

The maximum number of tries is a little arbitrary.  Increasing it will
slightly increase memory usage and may take a couple extra processing
cycles, but should not change classification results, so should be
reasonable.

Actually enabling more prefixes will consume more memory and reduce
efficiency of a single flow classification, but that's a trade user can
make knowing the traffic pattern and how their particular flow table
looks like.  While efficiency of a single flow classification may go
down, the overall performance of the system may be significantly
improved by having way less datapath flows with wider matches.

The number of tunnels in a typical setup is not that high, so I'm not
sure if it makes sense to increase the limit higher.  At the same time
combined IPv4 + IPv6 handling is pretty common.  For example, that's
the case with OVN.

Tests in ofproto-dpif.at cover IPv4 and IPv6 address classification
separately, and these fields can't overlap, so not adding any new tests.

Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
---

It is really unfortunate that IPv4 and IPv6 fields cannot be enabled
together at the times when importance and spread of IPv6 setups increases
fast.  So, I think, we need to backport this change down to 3.3 LTS.
It doesn't change the default behavior of the classifier and hardly
touches the code, so it should be safe to backport.  At least the LTS
users will be able to opt-in into the optimizations, if they are using
mixed v4 + v6 setups, which are common, e.g. with OVN.

Thoughts?

 NEWS                       | 6 ++++++
 lib/classifier.h           | 4 ++--
 tests/classifier.at        | 4 ++--
 vswitchd/vswitch.ovsschema | 6 +++---
 vswitchd/vswitch.xml       | 2 +-
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 200a8bb40..0ec0c71b1 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,11 @@
 Post-v3.4.0
 --------------------
+   - The limit on the number of fields for address prefix tracking in flow
+     tables increased from 3 to 4.  For example, it is now possible to
+     specify both IPv4 and IPv6 address fields at the same time:
+      $ ovs-vsctl set Bridge br0 flow_tables:0=@N -- \
+          --id=@N create Flow_Table \
+                    name=table0 prefixes=nw_dst,nw_src,ipv6_dst,ipv6_src
    - Userspace datapath:
      * The default zone limit, if set, is now inherited by any zone
        that does not have a specific value defined, rather than being
diff --git a/lib/classifier.h b/lib/classifier.h
index 709b0f156..7928601e0 100644
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -169,7 +169,7 @@
  * is a name of a field that should be used for prefix tracking.
  *
  * There is a maximum number of fields that can be enabled for any one
- * flow table.  Currently this limit is 3.
+ * flow table.  Currently this limit is 4.
  *
  *
  * Partitioning (Lookup Time and Wildcard Optimization)
@@ -327,7 +327,7 @@ struct cls_trie {
 
 enum {
     CLS_MAX_INDICES = 3,   /* Maximum number of lookup indices per subtable. */
-    CLS_MAX_TRIES = 3      /* Maximum number of prefix trees per classifier. */
+    CLS_MAX_TRIES = 4,     /* Maximum number of prefix trees per classifier. */
 };
 
 /* A flow classifier. */
diff --git a/tests/classifier.at b/tests/classifier.at
index 93a13f32b..05e82ea96 100644
--- a/tests/classifier.at
+++ b/tests/classifier.at
@@ -180,8 +180,8 @@ Datapath actions: drop
 ])
 
 AT_CHECK([ovs-vsctl set Flow_Table t0 prefixes=ipv6_label], [0])
-AT_CHECK([ovs-vsctl set Flow_Table t0 prefixes=nw_dst,nw_src,tun_dst,tun_src], 
[1], [],
-[ovs-vsctl: nw_dst,nw_src,tun_dst,tun_src: 4 value(s) specified but the 
maximum number is 3
+AT_CHECK([ovs-vsctl set Flow_Table t0 
prefixes=nw_dst,nw_src,tun_dst,tun_src,ipv6_src], [1], [],
+[ovs-vsctl: nw_dst,nw_src,tun_dst,tun_src,ipv6_src: 5 value(s) specified but 
the maximum number is 4
 ])
 AT_CHECK([ovs-vsctl set Flow_Table t0 prefixes=nw_dst,nw_dst], [1], [],
 [ovs-vsctl: nw_dst,nw_dst: set contains duplicate value
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 68689fe2a..c658291c7 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "8.7.0",
- "cksum": "3751637058 27869",
+ "version": "8.8.0",
+ "cksum": "2823623553 27869",
  "tables": {
    "Open_vSwitch": {
      "columns": {
@@ -385,7 +385,7 @@
        "groups": {
          "type": {"key": "string", "min": 0, "max": "unlimited"}},
        "prefixes": {
-         "type": {"key": "string", "min": 0, "max": 3}},
+         "type": {"key": "string", "min": 0, "max": 4}},
        "external_ids": {
          "type": {"key": "string", "value": "string",
                   "min": 0, "max": "unlimited"}}}},
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 36cb4e495..89f844bf8 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -4897,7 +4897,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch 
options:peer=p1 \
 
         <p>
           There is a maximum number of fields that can be enabled for any
-          one flow table.  Currently this limit is 3.
+          one flow table.  Currently this limit is 4.
         </p>
       </column>
     </group>
-- 
2.47.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to