| Message ID | 20250427075147.2083997-1-roid@nvidia.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev] classifier: Introduce disable-ports-trie. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | fail | cirrus build: failed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 27/04/2025 10:51, Roi Dayan wrote: > From: Eli Britstein <elibr@nvidia.com> > > According to [1], transport port masks are optimized using a prefix tree, > to get less specific datapath flows. However, this might be not optimal > for some cases. HW offload that is based on templates and has a table per > template (all the fields that are matched) will create many such tables > and yield poor performance. Furthermore, such templates are limited resources. > > In such cases, it is preferred to have more specific datapath flows that > share the same match template rather then less flows distributed across > multiple templates. > > To achieve that, introduce other_config:disable-ports-trie (true/false) > configuration (default as false). > > [1] 69d6040e861a ("lib/classifier: Use a prefix tree to optimize ports wildcarding.") > > Acked-by: Roi Dayan <roid@nvidia.com> > Signed-off-by: Eli Britstein <elibr@nvidia.com> > --- Hi, Any feedkback about this optional config? Thanks, Roi > lib/classifier.c | 21 +++++++++++++-------- > lib/classifier.h | 3 ++- > lib/ovs-router.c | 4 ++-- > lib/tnl-ports.c | 7 ++++--- > ofproto/ofproto-dpif.c | 9 ++++++--- > ofproto/ofproto-provider.h | 2 ++ > ofproto/ofproto.c | 5 ++++- > ofproto/ofproto.h | 2 +- > tests/classifier.at | 25 +++++++++++++++++++++++++ > tests/test-classifier.c | 8 ++++---- > vswitchd/bridge.c | 3 ++- > vswitchd/vswitch.xml | 18 ++++++++++++++++++ > 12 files changed, 83 insertions(+), 24 deletions(-) > > diff --git a/lib/classifier.c b/lib/classifier.c > index 55f23b976f30..37684a5aa026 100644 > --- a/lib/classifier.c > +++ b/lib/classifier.c > @@ -116,7 +116,8 @@ static const struct cls_match *find_match_wc(const struct cls_subtable *, > const struct flow *, > struct trie_ctx *, > unsigned int n_tries, > - struct flow_wildcards *); > + struct flow_wildcards *, > + bool disable_ports_trie); > static struct cls_match *find_equal(const struct cls_subtable *, > const struct miniflow *, uint32_t hash); > > @@ -971,7 +972,8 @@ static const struct cls_rule * > classifier_lookup__(const struct classifier *cls, ovs_version_t version, > struct flow *flow, struct flow_wildcards *wc, > bool allow_conjunctive_matches, > - struct hmapx *conj_flows) > + struct hmapx *conj_flows, > + bool disable_ports_trie) > { > struct trie_ctx trie_ctx[CLS_MAX_TRIES]; > const struct cls_match *match; > @@ -1007,7 +1009,7 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, > /* Skip subtables with no match, or where the match is lower-priority > * than some certain match we've already found. */ > match = find_match_wc(subtable, version, flow, trie_ctx, cls->n_tries, > - wc); > + wc, disable_ports_trie); > if (!match || match->priority <= hard_pri) { > continue; > } > @@ -1132,7 +1134,7 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, > > flow->conj_id = id; > rule = classifier_lookup__(cls, version, flow, wc, false, > - NULL); > + NULL, disable_ports_trie); > flow->conj_id = saved_conj_id; > > if (rule) { > @@ -1207,9 +1209,11 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, > const struct cls_rule * > classifier_lookup(const struct classifier *cls, ovs_version_t version, > struct flow *flow, struct flow_wildcards *wc, > - struct hmapx *conj_flows) > + struct hmapx *conj_flows, > + bool disable_ports_trie) > { > - return classifier_lookup__(cls, version, flow, wc, true, conj_flows); > + return classifier_lookup__(cls, version, flow, wc, true, conj_flows, > + disable_ports_trie); > } > > /* Finds and returns a rule in 'cls' with exactly the same priority and > @@ -1727,7 +1731,8 @@ find_match(const struct cls_subtable *subtable, ovs_version_t version, > static const struct cls_match * > find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, > const struct flow *flow, struct trie_ctx *trie_ctx, > - unsigned int n_tries, struct flow_wildcards *wc) > + unsigned int n_tries, struct flow_wildcards *wc, > + bool disable_ports_trie) > { > if (OVS_UNLIKELY(!wc)) { > return find_match(subtable, version, flow, > @@ -1774,7 +1779,7 @@ find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, > subtable->index_maps[i], > &mask_offset, &basis); > rule = find_match(subtable, version, flow, hash); > - if (!rule && subtable->ports_mask_len) { > + if (!rule && subtable->ports_mask_len && !disable_ports_trie) { > /* The final stage had ports, but there was no match. Instead of > * unwildcarding all the ports bits, use the ports trie to figure out a > * smaller set of bits to unwildcard. */ > diff --git a/lib/classifier.h b/lib/classifier.h > index 7928601e0f59..38eb92f45267 100644 > --- a/lib/classifier.h > +++ b/lib/classifier.h > @@ -399,7 +399,8 @@ static inline void classifier_publish(struct classifier *); > const struct cls_rule *classifier_lookup(const struct classifier *, > ovs_version_t, struct flow *, > struct flow_wildcards *, > - struct hmapx *conj_flows); > + struct hmapx *conj_flows, > + bool disable_ports_trie); > bool classifier_rule_overlaps(const struct classifier *, > const struct cls_rule *, ovs_version_t); > const struct cls_rule *classifier_find_rule_exactly(const struct classifier *, > diff --git a/lib/ovs-router.c b/lib/ovs-router.c > index d955a3a543b8..81fe3382a287 100644 > --- a/lib/ovs-router.c > +++ b/lib/ovs-router.c > @@ -117,7 +117,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst, > struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark}; > > cr_src = classifier_lookup(&cls, OVS_VERSION_MAX, &flow_src, NULL, > - NULL); > + NULL, false); > if (cr_src) { > struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src); > if (!p_src->local) { > @@ -128,7 +128,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst, > } > } > > - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL); > + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL, false); > if (cr) { > struct ovs_router_entry *p = ovs_router_entry_cast(cr); > > diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c > index 56119b30010e..3b3545a11194 100644 > --- a/lib/tnl-ports.c > +++ b/lib/tnl-ports.c > @@ -112,7 +112,8 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr, > tnl_port_init_flow(&match.flow, mac, addr, nw_proto, tp_port); > > do { > - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL); > + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL, > + false); > p = tnl_port_cast(cr); > /* Try again if the rule was released before we get the reference. */ > } while (p && !ovs_refcount_try_ref_rcu(&p->ref_cnt)); > @@ -245,7 +246,7 @@ map_delete(struct eth_addr mac, struct in6_addr *addr, > > tnl_port_init_flow(&flow, mac, addr, nw_proto, tp_port); > > - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL); > + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL, false); > tnl_port_unref(cr); > } > > @@ -303,7 +304,7 @@ odp_port_t > tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc) > { > const struct cls_rule *cr = classifier_lookup(&cls, OVS_VERSION_MAX, flow, > - wc, NULL); > + wc, NULL, false); > > return (cr) ? tnl_port_cast(cr)->portno : ODPP_NONE; > } > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index ed9e44ce2b03..5b9b1e70ba92 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -4587,9 +4587,12 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, ovs_version_t version, > struct hmapx *conj_flows) > { > struct classifier *cls = &ofproto->up.tables[table_id].cls; > - return rule_dpif_cast(rule_from_cls_rule(classifier_lookup(cls, version, > - flow, wc, > - conj_flows))); > + bool disable_ports_trie = ofproto->up.disable_ports_trie; > + const struct cls_rule *cr; > + > + cr = classifier_lookup(cls, version, flow, wc, conj_flows, > + disable_ports_trie); > + return rule_dpif_cast(rule_from_cls_rule(cr)); > } > > void > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index 7df3f5246912..4527100c2d65 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -146,6 +146,8 @@ struct ofproto { > struct vl_mff_map vl_mff_map; > /* refcount to this ofproto, held by rule/group/xlate_caches */ > struct ovs_refcount refcount; > + > + bool disable_ports_trie; > }; > > void ofproto_init_tables(struct ofproto *, int n_tables); > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index ef615e59c354..8bd47c68c16b 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -481,7 +481,7 @@ ofproto_bump_tables_version(struct ofproto *ofproto) > > int > ofproto_create(const char *datapath_name, const char *datapath_type, > - struct ofproto **ofprotop) > + const struct smap *other_config, struct ofproto **ofprotop) > OVS_EXCLUDED(ofproto_mutex) > { > const struct ofproto_class *class; > @@ -589,6 +589,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type, > ofproto->slowpath_meter_id = UINT32_MAX; > ofproto->controller_meter_id = UINT32_MAX; > > + ofproto->disable_ports_trie = smap_get_bool(other_config, > + "disable-ports-trie", false); > + > /* Set the initial tables version. */ > ofproto_bump_tables_version(ofproto); > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > index 3f85509a1add..ea0be0a4a19c 100644 > --- a/ofproto/ofproto.h > +++ b/ofproto/ofproto.h > @@ -275,7 +275,7 @@ int ofproto_type_run(const char *datapath_type); > void ofproto_type_wait(const char *datapath_type); > > int ofproto_create(const char *datapath, const char *datapath_type, > - struct ofproto **ofprotop) > + const struct smap *other_config, struct ofproto **ofprotop) > OVS_EXCLUDED(ofproto_mutex); > void ofproto_destroy(struct ofproto *, bool del); > int ofproto_delete(const char *name, const char *type); > diff --git a/tests/classifier.at b/tests/classifier.at > index dfadf5e5a9ce..b83b76b3eacf 100644 > --- a/tests/classifier.at > +++ b/tests/classifier.at > @@ -282,6 +282,31 @@ Datapath actions: 3 > OVS_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([flow classifier - disable ports trie]) > +OVS_VSWITCHD_START([set Open_vSwitch . other_config:disable-ports-trie=true]) > +add_of_ports br0 1 2 > + > +AT_DATA([flows.txt], [dnl > + priority=1,ip,tcp,tp_dst=80 action=drop > + priority=0 actions=2 > +]) > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > + > +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=10.1.2.15,nw_proto=6,nw_tos=0,nw_ttl=128,tp_src=8,tp_dst=80'], [0], [stdout]) > +AT_CHECK([tail -2 stdout], [0], > + [Megaflow: recirc_id=0,eth,tcp,in_port=1,nw_frag=no,tp_dst=80 > +Datapath actions: drop > +]) > + > +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=10.1.2.15,nw_proto=6,nw_tos=0,nw_ttl=128,tp_src=8,tp_dst=79'], [0], [stdout]) > +AT_CHECK([tail -2 stdout], [0], > + [Megaflow: recirc_id=0,eth,tcp,in_port=1,nw_frag=no,tp_dst=79 > +Datapath actions: 2 > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([flow classifier - ipv6 ND dependency]) > OVS_VSWITCHD_START > add_of_ports br0 1 2 > diff --git a/tests/test-classifier.c b/tests/test-classifier.c > index 2c1604a01e2e..445b6206697f 100644 > --- a/tests/test-classifier.c > +++ b/tests/test-classifier.c > @@ -441,7 +441,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules, > /* This assertion is here to suppress a GCC 4.9 array-bounds warning */ > ovs_assert(cls->n_tries <= CLS_MAX_TRIES); > > - cr0 = classifier_lookup(cls, version, &flow, &wc, NULL); > + cr0 = classifier_lookup(cls, version, &flow, &wc, NULL, false); > cr1 = tcls_lookup(tcls, &flow); > assert((cr0 == NULL) == (cr1 == NULL)); > if (cr0 != NULL) { > @@ -454,7 +454,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules, > /* Make sure the rule should have been visible. */ > assert(cls_rule_visible_in_version(cr0, version)); > } > - cr2 = classifier_lookup(cls, version, &flow, NULL, NULL); > + cr2 = classifier_lookup(cls, version, &flow, NULL, NULL, false); > assert(cr2 == cr0); > } > } > @@ -1370,10 +1370,10 @@ lookup_classifier(void *aux_) > if (aux->use_wc) { > flow_wildcards_init_catchall(&wc); > cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x], > - &wc, NULL); > + &wc, NULL, false); > } else { > cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x], > - NULL, NULL); > + NULL, NULL, false); > } > if (cr) { > hits++; > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c > index 456b784c01d0..1a2236c77719 100644 > --- a/vswitchd/bridge.c > +++ b/vswitchd/bridge.c > @@ -933,7 +933,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) > if (!br->ofproto) { > int error; > > - error = ofproto_create(br->name, br->type, &br->ofproto); > + error = ofproto_create(br->name, br->type, &ovs_cfg->other_config, > + &br->ofproto); > if (error) { > VLOG_ERR("failed to create bridge %s: %s", br->name, > ovs_strerror(error)); > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index 76df9aab055a..737e4c07adf5 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -176,6 +176,24 @@ > </p> > </column> > > + <column name="other_config" key="disable-ports-trie" > + type='{"type": "boolean"}'> > + <p> > + Set this value to <code>true</code> to disable transport ports prefix > + tree optimization. > + </p> > + <p> > + The default value is <code>false</code>. Changing this value requires > + restarting the daemon. > + </p> > + <p> > + The default behavior tries to optimize transport ports wildcards, > + to get less datapath flows. The down side of it that those flows > + are distributed accross multiple match templates, which is > + problematic for HW offloads. > + </p> > + </column> > + > <column name="other_config" key="max-idle" > type='{"type": "integer", "minInteger": 500}'> > <p>
On 11 May 2025, at 15:47, Roi Dayan via dev wrote: > On 27/04/2025 10:51, Roi Dayan wrote: >> From: Eli Britstein <elibr@nvidia.com> >> >> According to [1], transport port masks are optimized using a prefix tree, >> to get less specific datapath flows. However, this might be not optimal >> for some cases. HW offload that is based on templates and has a table per >> template (all the fields that are matched) will create many such tables >> and yield poor performance. Furthermore, such templates are limited resources. >> >> In such cases, it is preferred to have more specific datapath flows that >> share the same match template rather then less flows distributed across >> multiple templates. >> >> To achieve that, introduce other_config:disable-ports-trie (true/false) >> configuration (default as false). >> >> [1] 69d6040e861a ("lib/classifier: Use a prefix tree to optimize ports wildcarding.") >> >> Acked-by: Roi Dayan <roid@nvidia.com> >> Signed-off-by: Eli Britstein <elibr@nvidia.com> >> --- > > Hi, > > Any feedkback about this optional config? Hi Roi, It’s on Ilya’s queue, see here; https://patchwork.ozlabs.org/project/openvswitch/list/ >> lib/classifier.c | 21 +++++++++++++-------- >> lib/classifier.h | 3 ++- >> lib/ovs-router.c | 4 ++-- >> lib/tnl-ports.c | 7 ++++--- >> ofproto/ofproto-dpif.c | 9 ++++++--- >> ofproto/ofproto-provider.h | 2 ++ >> ofproto/ofproto.c | 5 ++++- >> ofproto/ofproto.h | 2 +- >> tests/classifier.at | 25 +++++++++++++++++++++++++ >> tests/test-classifier.c | 8 ++++---- >> vswitchd/bridge.c | 3 ++- >> vswitchd/vswitch.xml | 18 ++++++++++++++++++ >> 12 files changed, 83 insertions(+), 24 deletions(-) >> >> diff --git a/lib/classifier.c b/lib/classifier.c >> index 55f23b976f30..37684a5aa026 100644 >> --- a/lib/classifier.c >> +++ b/lib/classifier.c >> @@ -116,7 +116,8 @@ static const struct cls_match *find_match_wc(const struct cls_subtable *, >> const struct flow *, >> struct trie_ctx *, >> unsigned int n_tries, >> - struct flow_wildcards *); >> + struct flow_wildcards *, >> + bool disable_ports_trie); >> static struct cls_match *find_equal(const struct cls_subtable *, >> const struct miniflow *, uint32_t hash); >> >> @@ -971,7 +972,8 @@ static const struct cls_rule * >> classifier_lookup__(const struct classifier *cls, ovs_version_t version, >> struct flow *flow, struct flow_wildcards *wc, >> bool allow_conjunctive_matches, >> - struct hmapx *conj_flows) >> + struct hmapx *conj_flows, >> + bool disable_ports_trie) >> { >> struct trie_ctx trie_ctx[CLS_MAX_TRIES]; >> const struct cls_match *match; >> @@ -1007,7 +1009,7 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, >> /* Skip subtables with no match, or where the match is lower-priority >> * than some certain match we've already found. */ >> match = find_match_wc(subtable, version, flow, trie_ctx, cls->n_tries, >> - wc); >> + wc, disable_ports_trie); >> if (!match || match->priority <= hard_pri) { >> continue; >> } >> @@ -1132,7 +1134,7 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, >> >> flow->conj_id = id; >> rule = classifier_lookup__(cls, version, flow, wc, false, >> - NULL); >> + NULL, disable_ports_trie); >> flow->conj_id = saved_conj_id; >> >> if (rule) { >> @@ -1207,9 +1209,11 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, >> const struct cls_rule * >> classifier_lookup(const struct classifier *cls, ovs_version_t version, >> struct flow *flow, struct flow_wildcards *wc, >> - struct hmapx *conj_flows) >> + struct hmapx *conj_flows, >> + bool disable_ports_trie) >> { >> - return classifier_lookup__(cls, version, flow, wc, true, conj_flows); >> + return classifier_lookup__(cls, version, flow, wc, true, conj_flows, >> + disable_ports_trie); >> } >> >> /* Finds and returns a rule in 'cls' with exactly the same priority and >> @@ -1727,7 +1731,8 @@ find_match(const struct cls_subtable *subtable, ovs_version_t version, >> static const struct cls_match * >> find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, >> const struct flow *flow, struct trie_ctx *trie_ctx, >> - unsigned int n_tries, struct flow_wildcards *wc) >> + unsigned int n_tries, struct flow_wildcards *wc, >> + bool disable_ports_trie) >> { >> if (OVS_UNLIKELY(!wc)) { >> return find_match(subtable, version, flow, >> @@ -1774,7 +1779,7 @@ find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, >> subtable->index_maps[i], >> &mask_offset, &basis); >> rule = find_match(subtable, version, flow, hash); >> - if (!rule && subtable->ports_mask_len) { >> + if (!rule && subtable->ports_mask_len && !disable_ports_trie) { >> /* The final stage had ports, but there was no match. Instead of >> * unwildcarding all the ports bits, use the ports trie to figure out a >> * smaller set of bits to unwildcard. */ >> diff --git a/lib/classifier.h b/lib/classifier.h >> index 7928601e0f59..38eb92f45267 100644 >> --- a/lib/classifier.h >> +++ b/lib/classifier.h >> @@ -399,7 +399,8 @@ static inline void classifier_publish(struct classifier *); >> const struct cls_rule *classifier_lookup(const struct classifier *, >> ovs_version_t, struct flow *, >> struct flow_wildcards *, >> - struct hmapx *conj_flows); >> + struct hmapx *conj_flows, >> + bool disable_ports_trie); >> bool classifier_rule_overlaps(const struct classifier *, >> const struct cls_rule *, ovs_version_t); >> const struct cls_rule *classifier_find_rule_exactly(const struct classifier *, >> diff --git a/lib/ovs-router.c b/lib/ovs-router.c >> index d955a3a543b8..81fe3382a287 100644 >> --- a/lib/ovs-router.c >> +++ b/lib/ovs-router.c >> @@ -117,7 +117,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst, >> struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark}; >> >> cr_src = classifier_lookup(&cls, OVS_VERSION_MAX, &flow_src, NULL, >> - NULL); >> + NULL, false); >> if (cr_src) { >> struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src); >> if (!p_src->local) { >> @@ -128,7 +128,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst, >> } >> } >> >> - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL); >> + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL, false); >> if (cr) { >> struct ovs_router_entry *p = ovs_router_entry_cast(cr); >> >> diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c >> index 56119b30010e..3b3545a11194 100644 >> --- a/lib/tnl-ports.c >> +++ b/lib/tnl-ports.c >> @@ -112,7 +112,8 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr, >> tnl_port_init_flow(&match.flow, mac, addr, nw_proto, tp_port); >> >> do { >> - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL); >> + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL, >> + false); >> p = tnl_port_cast(cr); >> /* Try again if the rule was released before we get the reference. */ >> } while (p && !ovs_refcount_try_ref_rcu(&p->ref_cnt)); >> @@ -245,7 +246,7 @@ map_delete(struct eth_addr mac, struct in6_addr *addr, >> >> tnl_port_init_flow(&flow, mac, addr, nw_proto, tp_port); >> >> - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL); >> + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL, false); >> tnl_port_unref(cr); >> } >> >> @@ -303,7 +304,7 @@ odp_port_t >> tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc) >> { >> const struct cls_rule *cr = classifier_lookup(&cls, OVS_VERSION_MAX, flow, >> - wc, NULL); >> + wc, NULL, false); >> >> return (cr) ? tnl_port_cast(cr)->portno : ODPP_NONE; >> } >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index ed9e44ce2b03..5b9b1e70ba92 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -4587,9 +4587,12 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, ovs_version_t version, >> struct hmapx *conj_flows) >> { >> struct classifier *cls = &ofproto->up.tables[table_id].cls; >> - return rule_dpif_cast(rule_from_cls_rule(classifier_lookup(cls, version, >> - flow, wc, >> - conj_flows))); >> + bool disable_ports_trie = ofproto->up.disable_ports_trie; >> + const struct cls_rule *cr; >> + >> + cr = classifier_lookup(cls, version, flow, wc, conj_flows, >> + disable_ports_trie); >> + return rule_dpif_cast(rule_from_cls_rule(cr)); >> } >> >> void >> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h >> index 7df3f5246912..4527100c2d65 100644 >> --- a/ofproto/ofproto-provider.h >> +++ b/ofproto/ofproto-provider.h >> @@ -146,6 +146,8 @@ struct ofproto { >> struct vl_mff_map vl_mff_map; >> /* refcount to this ofproto, held by rule/group/xlate_caches */ >> struct ovs_refcount refcount; >> + >> + bool disable_ports_trie; >> }; >> >> void ofproto_init_tables(struct ofproto *, int n_tables); >> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c >> index ef615e59c354..8bd47c68c16b 100644 >> --- a/ofproto/ofproto.c >> +++ b/ofproto/ofproto.c >> @@ -481,7 +481,7 @@ ofproto_bump_tables_version(struct ofproto *ofproto) >> >> int >> ofproto_create(const char *datapath_name, const char *datapath_type, >> - struct ofproto **ofprotop) >> + const struct smap *other_config, struct ofproto **ofprotop) >> OVS_EXCLUDED(ofproto_mutex) >> { >> const struct ofproto_class *class; >> @@ -589,6 +589,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type, >> ofproto->slowpath_meter_id = UINT32_MAX; >> ofproto->controller_meter_id = UINT32_MAX; >> >> + ofproto->disable_ports_trie = smap_get_bool(other_config, >> + "disable-ports-trie", false); >> + >> /* Set the initial tables version. */ >> ofproto_bump_tables_version(ofproto); >> >> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h >> index 3f85509a1add..ea0be0a4a19c 100644 >> --- a/ofproto/ofproto.h >> +++ b/ofproto/ofproto.h >> @@ -275,7 +275,7 @@ int ofproto_type_run(const char *datapath_type); >> void ofproto_type_wait(const char *datapath_type); >> >> int ofproto_create(const char *datapath, const char *datapath_type, >> - struct ofproto **ofprotop) >> + const struct smap *other_config, struct ofproto **ofprotop) >> OVS_EXCLUDED(ofproto_mutex); >> void ofproto_destroy(struct ofproto *, bool del); >> int ofproto_delete(const char *name, const char *type); >> diff --git a/tests/classifier.at b/tests/classifier.at >> index dfadf5e5a9ce..b83b76b3eacf 100644 >> --- a/tests/classifier.at >> +++ b/tests/classifier.at >> @@ -282,6 +282,31 @@ Datapath actions: 3 >> OVS_VSWITCHD_STOP >> AT_CLEANUP >> >> +AT_SETUP([flow classifier - disable ports trie]) >> +OVS_VSWITCHD_START([set Open_vSwitch . other_config:disable-ports-trie=true]) >> +add_of_ports br0 1 2 >> + >> +AT_DATA([flows.txt], [dnl >> + priority=1,ip,tcp,tp_dst=80 action=drop >> + priority=0 actions=2 >> +]) >> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) >> + >> +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=10.1.2.15,nw_proto=6,nw_tos=0,nw_ttl=128,tp_src=8,tp_dst=80'], [0], [stdout]) >> +AT_CHECK([tail -2 stdout], [0], >> + [Megaflow: recirc_id=0,eth,tcp,in_port=1,nw_frag=no,tp_dst=80 >> +Datapath actions: drop >> +]) >> + >> +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=10.1.2.15,nw_proto=6,nw_tos=0,nw_ttl=128,tp_src=8,tp_dst=79'], [0], [stdout]) >> +AT_CHECK([tail -2 stdout], [0], >> + [Megaflow: recirc_id=0,eth,tcp,in_port=1,nw_frag=no,tp_dst=79 >> +Datapath actions: 2 >> +]) >> + >> +OVS_VSWITCHD_STOP >> +AT_CLEANUP >> + >> AT_SETUP([flow classifier - ipv6 ND dependency]) >> OVS_VSWITCHD_START >> add_of_ports br0 1 2 >> diff --git a/tests/test-classifier.c b/tests/test-classifier.c >> index 2c1604a01e2e..445b6206697f 100644 >> --- a/tests/test-classifier.c >> +++ b/tests/test-classifier.c >> @@ -441,7 +441,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules, >> /* This assertion is here to suppress a GCC 4.9 array-bounds warning */ >> ovs_assert(cls->n_tries <= CLS_MAX_TRIES); >> >> - cr0 = classifier_lookup(cls, version, &flow, &wc, NULL); >> + cr0 = classifier_lookup(cls, version, &flow, &wc, NULL, false); >> cr1 = tcls_lookup(tcls, &flow); >> assert((cr0 == NULL) == (cr1 == NULL)); >> if (cr0 != NULL) { >> @@ -454,7 +454,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules, >> /* Make sure the rule should have been visible. */ >> assert(cls_rule_visible_in_version(cr0, version)); >> } >> - cr2 = classifier_lookup(cls, version, &flow, NULL, NULL); >> + cr2 = classifier_lookup(cls, version, &flow, NULL, NULL, false); >> assert(cr2 == cr0); >> } >> } >> @@ -1370,10 +1370,10 @@ lookup_classifier(void *aux_) >> if (aux->use_wc) { >> flow_wildcards_init_catchall(&wc); >> cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x], >> - &wc, NULL); >> + &wc, NULL, false); >> } else { >> cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x], >> - NULL, NULL); >> + NULL, NULL, false); >> } >> if (cr) { >> hits++; >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c >> index 456b784c01d0..1a2236c77719 100644 >> --- a/vswitchd/bridge.c >> +++ b/vswitchd/bridge.c >> @@ -933,7 +933,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) >> if (!br->ofproto) { >> int error; >> >> - error = ofproto_create(br->name, br->type, &br->ofproto); >> + error = ofproto_create(br->name, br->type, &ovs_cfg->other_config, >> + &br->ofproto); >> if (error) { >> VLOG_ERR("failed to create bridge %s: %s", br->name, >> ovs_strerror(error)); >> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml >> index 76df9aab055a..737e4c07adf5 100644 >> --- a/vswitchd/vswitch.xml >> +++ b/vswitchd/vswitch.xml >> @@ -176,6 +176,24 @@ >> </p> >> </column> >> >> + <column name="other_config" key="disable-ports-trie" >> + type='{"type": "boolean"}'> >> + <p> >> + Set this value to <code>true</code> to disable transport ports prefix >> + tree optimization. >> + </p> >> + <p> >> + The default value is <code>false</code>. Changing this value requires >> + restarting the daemon. >> + </p> >> + <p> >> + The default behavior tries to optimize transport ports wildcards, >> + to get less datapath flows. The down side of it that those flows >> + are distributed accross multiple match templates, which is >> + problematic for HW offloads. >> + </p> >> + </column> >> + >> <column name="other_config" key="max-idle" >> type='{"type": "integer", "minInteger": 500}'> >> <p> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 4/27/25 9:51 AM, Roi Dayan via dev wrote: > From: Eli Britstein <elibr@nvidia.com> > > According to [1], transport port masks are optimized using a prefix tree, > to get less specific datapath flows. However, this might be not optimal > for some cases. HW offload that is based on templates and has a table per > template (all the fields that are matched) will create many such tables > and yield poor performance. Furthermore, such templates are limited resources. > > In such cases, it is preferred to have more specific datapath flows that > share the same match template rather then less flows distributed across > multiple templates. > > To achieve that, introduce other_config:disable-ports-trie (true/false) > configuration (default as false). > > [1] 69d6040e861a ("lib/classifier: Use a prefix tree to optimize ports wildcarding.") > > Acked-by: Roi Dayan <roid@nvidia.com> > Signed-off-by: Eli Britstein <elibr@nvidia.com> Hi, Eli and Roi. Thanks for the patch! I think, it's reasonable to provide an option to turn off prefix matching for ports, since we allow to do that for other fields. A few comments for the implementation: 1. We configure prefixes per flow table, so this new option should be at the same level as a new column in the Flow_Table table. Ideally, we would just allow configuration through the 'prefixes' column, but it doesn't really fit in these as it is not a single field, and also we can't just turn off this by default and expect users to add new prefixes. So, yeah, adding a new column like 'use-port-prefixes' with a default 'true' value, is likely a good way forward. Note: "positive" names like "enable/use-..." are normally easier to understand than the "negative" ("disable-...") ones. 2. There shouldn't be a need to pass a boolean flag around in all the lookup calls. It should be enough to set ports_mask_len to zero while creating subtables. This will also allow changing the value in runtime. Note: there is a synchronization bug that Numan reported recently, but I'm working on a fix. 3. The hardware details that you're mentioning in the documentation like "multiple match templates" are not related to OVS and do not make a lot of sense for the reader, so should not be in the OVS docs. Related OVS construct is the "prefixes" column, so it should be mentioned instead. Best regards, Ilya Maximets.
diff --git a/lib/classifier.c b/lib/classifier.c index 55f23b976f30..37684a5aa026 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -116,7 +116,8 @@ static const struct cls_match *find_match_wc(const struct cls_subtable *, const struct flow *, struct trie_ctx *, unsigned int n_tries, - struct flow_wildcards *); + struct flow_wildcards *, + bool disable_ports_trie); static struct cls_match *find_equal(const struct cls_subtable *, const struct miniflow *, uint32_t hash); @@ -971,7 +972,8 @@ static const struct cls_rule * classifier_lookup__(const struct classifier *cls, ovs_version_t version, struct flow *flow, struct flow_wildcards *wc, bool allow_conjunctive_matches, - struct hmapx *conj_flows) + struct hmapx *conj_flows, + bool disable_ports_trie) { struct trie_ctx trie_ctx[CLS_MAX_TRIES]; const struct cls_match *match; @@ -1007,7 +1009,7 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, /* Skip subtables with no match, or where the match is lower-priority * than some certain match we've already found. */ match = find_match_wc(subtable, version, flow, trie_ctx, cls->n_tries, - wc); + wc, disable_ports_trie); if (!match || match->priority <= hard_pri) { continue; } @@ -1132,7 +1134,7 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, flow->conj_id = id; rule = classifier_lookup__(cls, version, flow, wc, false, - NULL); + NULL, disable_ports_trie); flow->conj_id = saved_conj_id; if (rule) { @@ -1207,9 +1209,11 @@ classifier_lookup__(const struct classifier *cls, ovs_version_t version, const struct cls_rule * classifier_lookup(const struct classifier *cls, ovs_version_t version, struct flow *flow, struct flow_wildcards *wc, - struct hmapx *conj_flows) + struct hmapx *conj_flows, + bool disable_ports_trie) { - return classifier_lookup__(cls, version, flow, wc, true, conj_flows); + return classifier_lookup__(cls, version, flow, wc, true, conj_flows, + disable_ports_trie); } /* Finds and returns a rule in 'cls' with exactly the same priority and @@ -1727,7 +1731,8 @@ find_match(const struct cls_subtable *subtable, ovs_version_t version, static const struct cls_match * find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, const struct flow *flow, struct trie_ctx *trie_ctx, - unsigned int n_tries, struct flow_wildcards *wc) + unsigned int n_tries, struct flow_wildcards *wc, + bool disable_ports_trie) { if (OVS_UNLIKELY(!wc)) { return find_match(subtable, version, flow, @@ -1774,7 +1779,7 @@ find_match_wc(const struct cls_subtable *subtable, ovs_version_t version, subtable->index_maps[i], &mask_offset, &basis); rule = find_match(subtable, version, flow, hash); - if (!rule && subtable->ports_mask_len) { + if (!rule && subtable->ports_mask_len && !disable_ports_trie) { /* The final stage had ports, but there was no match. Instead of * unwildcarding all the ports bits, use the ports trie to figure out a * smaller set of bits to unwildcard. */ diff --git a/lib/classifier.h b/lib/classifier.h index 7928601e0f59..38eb92f45267 100644 --- a/lib/classifier.h +++ b/lib/classifier.h @@ -399,7 +399,8 @@ static inline void classifier_publish(struct classifier *); const struct cls_rule *classifier_lookup(const struct classifier *, ovs_version_t, struct flow *, struct flow_wildcards *, - struct hmapx *conj_flows); + struct hmapx *conj_flows, + bool disable_ports_trie); bool classifier_rule_overlaps(const struct classifier *, const struct cls_rule *, ovs_version_t); const struct cls_rule *classifier_find_rule_exactly(const struct classifier *, diff --git a/lib/ovs-router.c b/lib/ovs-router.c index d955a3a543b8..81fe3382a287 100644 --- a/lib/ovs-router.c +++ b/lib/ovs-router.c @@ -117,7 +117,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst, struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark}; cr_src = classifier_lookup(&cls, OVS_VERSION_MAX, &flow_src, NULL, - NULL); + NULL, false); if (cr_src) { struct ovs_router_entry *p_src = ovs_router_entry_cast(cr_src); if (!p_src->local) { @@ -128,7 +128,7 @@ ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst, } } - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL); + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL, false); if (cr) { struct ovs_router_entry *p = ovs_router_entry_cast(cr); diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c index 56119b30010e..3b3545a11194 100644 --- a/lib/tnl-ports.c +++ b/lib/tnl-ports.c @@ -112,7 +112,8 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr, tnl_port_init_flow(&match.flow, mac, addr, nw_proto, tp_port); do { - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL); + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &match.flow, NULL, NULL, + false); p = tnl_port_cast(cr); /* Try again if the rule was released before we get the reference. */ } while (p && !ovs_refcount_try_ref_rcu(&p->ref_cnt)); @@ -245,7 +246,7 @@ map_delete(struct eth_addr mac, struct in6_addr *addr, tnl_port_init_flow(&flow, mac, addr, nw_proto, tp_port); - cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL); + cr = classifier_lookup(&cls, OVS_VERSION_MAX, &flow, NULL, NULL, false); tnl_port_unref(cr); } @@ -303,7 +304,7 @@ odp_port_t tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc) { const struct cls_rule *cr = classifier_lookup(&cls, OVS_VERSION_MAX, flow, - wc, NULL); + wc, NULL, false); return (cr) ? tnl_port_cast(cr)->portno : ODPP_NONE; } diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index ed9e44ce2b03..5b9b1e70ba92 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4587,9 +4587,12 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, ovs_version_t version, struct hmapx *conj_flows) { struct classifier *cls = &ofproto->up.tables[table_id].cls; - return rule_dpif_cast(rule_from_cls_rule(classifier_lookup(cls, version, - flow, wc, - conj_flows))); + bool disable_ports_trie = ofproto->up.disable_ports_trie; + const struct cls_rule *cr; + + cr = classifier_lookup(cls, version, flow, wc, conj_flows, + disable_ports_trie); + return rule_dpif_cast(rule_from_cls_rule(cr)); } void diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 7df3f5246912..4527100c2d65 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -146,6 +146,8 @@ struct ofproto { struct vl_mff_map vl_mff_map; /* refcount to this ofproto, held by rule/group/xlate_caches */ struct ovs_refcount refcount; + + bool disable_ports_trie; }; void ofproto_init_tables(struct ofproto *, int n_tables); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index ef615e59c354..8bd47c68c16b 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -481,7 +481,7 @@ ofproto_bump_tables_version(struct ofproto *ofproto) int ofproto_create(const char *datapath_name, const char *datapath_type, - struct ofproto **ofprotop) + const struct smap *other_config, struct ofproto **ofprotop) OVS_EXCLUDED(ofproto_mutex) { const struct ofproto_class *class; @@ -589,6 +589,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type, ofproto->slowpath_meter_id = UINT32_MAX; ofproto->controller_meter_id = UINT32_MAX; + ofproto->disable_ports_trie = smap_get_bool(other_config, + "disable-ports-trie", false); + /* Set the initial tables version. */ ofproto_bump_tables_version(ofproto); diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index 3f85509a1add..ea0be0a4a19c 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -275,7 +275,7 @@ int ofproto_type_run(const char *datapath_type); void ofproto_type_wait(const char *datapath_type); int ofproto_create(const char *datapath, const char *datapath_type, - struct ofproto **ofprotop) + const struct smap *other_config, struct ofproto **ofprotop) OVS_EXCLUDED(ofproto_mutex); void ofproto_destroy(struct ofproto *, bool del); int ofproto_delete(const char *name, const char *type); diff --git a/tests/classifier.at b/tests/classifier.at index dfadf5e5a9ce..b83b76b3eacf 100644 --- a/tests/classifier.at +++ b/tests/classifier.at @@ -282,6 +282,31 @@ Datapath actions: 3 OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([flow classifier - disable ports trie]) +OVS_VSWITCHD_START([set Open_vSwitch . other_config:disable-ports-trie=true]) +add_of_ports br0 1 2 + +AT_DATA([flows.txt], [dnl + priority=1,ip,tcp,tp_dst=80 action=drop + priority=0 actions=2 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=10.1.2.15,nw_proto=6,nw_tos=0,nw_ttl=128,tp_src=8,tp_dst=80'], [0], [stdout]) +AT_CHECK([tail -2 stdout], [0], + [Megaflow: recirc_id=0,eth,tcp,in_port=1,nw_frag=no,tp_dst=80 +Datapath actions: drop +]) + +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,dl_type=0x0800,nw_src=192.168.0.1,nw_dst=10.1.2.15,nw_proto=6,nw_tos=0,nw_ttl=128,tp_src=8,tp_dst=79'], [0], [stdout]) +AT_CHECK([tail -2 stdout], [0], + [Megaflow: recirc_id=0,eth,tcp,in_port=1,nw_frag=no,tp_dst=79 +Datapath actions: 2 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([flow classifier - ipv6 ND dependency]) OVS_VSWITCHD_START add_of_ports br0 1 2 diff --git a/tests/test-classifier.c b/tests/test-classifier.c index 2c1604a01e2e..445b6206697f 100644 --- a/tests/test-classifier.c +++ b/tests/test-classifier.c @@ -441,7 +441,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules, /* This assertion is here to suppress a GCC 4.9 array-bounds warning */ ovs_assert(cls->n_tries <= CLS_MAX_TRIES); - cr0 = classifier_lookup(cls, version, &flow, &wc, NULL); + cr0 = classifier_lookup(cls, version, &flow, &wc, NULL, false); cr1 = tcls_lookup(tcls, &flow); assert((cr0 == NULL) == (cr1 == NULL)); if (cr0 != NULL) { @@ -454,7 +454,7 @@ compare_classifiers(struct classifier *cls, size_t n_invisible_rules, /* Make sure the rule should have been visible. */ assert(cls_rule_visible_in_version(cr0, version)); } - cr2 = classifier_lookup(cls, version, &flow, NULL, NULL); + cr2 = classifier_lookup(cls, version, &flow, NULL, NULL, false); assert(cr2 == cr0); } } @@ -1370,10 +1370,10 @@ lookup_classifier(void *aux_) if (aux->use_wc) { flow_wildcards_init_catchall(&wc); cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x], - &wc, NULL); + &wc, NULL, false); } else { cr = classifier_lookup(aux->cls, version, &aux->lookup_flows[x], - NULL, NULL); + NULL, NULL, false); } if (cr) { hits++; diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 456b784c01d0..1a2236c77719 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -933,7 +933,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) if (!br->ofproto) { int error; - error = ofproto_create(br->name, br->type, &br->ofproto); + error = ofproto_create(br->name, br->type, &ovs_cfg->other_config, + &br->ofproto); if (error) { VLOG_ERR("failed to create bridge %s: %s", br->name, ovs_strerror(error)); diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 76df9aab055a..737e4c07adf5 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -176,6 +176,24 @@ </p> </column> + <column name="other_config" key="disable-ports-trie" + type='{"type": "boolean"}'> + <p> + Set this value to <code>true</code> to disable transport ports prefix + tree optimization. + </p> + <p> + The default value is <code>false</code>. Changing this value requires + restarting the daemon. + </p> + <p> + The default behavior tries to optimize transport ports wildcards, + to get less datapath flows. The down side of it that those flows + are distributed accross multiple match templates, which is + problematic for HW offloads. + </p> + </column> + <column name="other_config" key="max-idle" type='{"type": "integer", "minInteger": 500}'> <p>