[ovs-dev] [PATCH] Use prefix trie lookup for IPv4 by default.
Jarno Rajahalme
jrajahalme at nicira.com
Fri May 16 19:59:16 UTC 2014
Pushed to master,
Jarno
On May 7, 2014, at 11:11 AM, Ethan Jackson <ethan at nicira.com> wrote:
> Acked-by: Ethan Jackson <ethan at nicira.com>
>
>
> On Fri, May 2, 2014 at 8:44 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>> Unless otherwise configured, the prefix trie lookup is enabled for
>> IPv4 destination and source address fields. A new keyword "none" is
>> accepted as the value of "prefixes" in the OVSDB Flow_Table column.
>>
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>> ---
>> ofproto/ofproto.c | 10 ++++++++++
>> ofproto/ofproto.h | 3 +++
>> tests/classifier.at | 31 +++++++++++++++++++++++--------
>> vswitchd/bridge.c | 16 +++++++++++++++-
>> vswitchd/vswitch.xml | 21 +++++++++++++++++----
>> 5 files changed, 68 insertions(+), 13 deletions(-)
>>
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index eb1f3be..660b06d 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -68,6 +68,11 @@ COVERAGE_DEFINE(ofproto_recv_openflow);
>> COVERAGE_DEFINE(ofproto_reinit_ports);
>> COVERAGE_DEFINE(ofproto_update_port);
>>
>> +/* Default fields to use for prefix tries in each flow table, unless something
>> + * else is configured. */
>> +const enum mf_field_id default_prefix_fields[2] =
>> + { MFF_IPV4_DST, MFF_IPV4_SRC };
>> +
>> enum ofproto_state {
>> S_OPENFLOW, /* Processing OpenFlow commands. */
>> S_EVICT, /* Evicting flows from over-limit tables. */
>> @@ -6772,6 +6777,11 @@ oftable_init(struct oftable *table)
>> classifier_init(&table->cls, flow_segment_u32s);
>> table->max_flows = UINT_MAX;
>> atomic_init(&table->config, (unsigned int)OFPROTO_TABLE_MISS_DEFAULT);
>> +
>> + fat_rwlock_wrlock(&table->cls.rwlock);
>> + classifier_set_prefix_fields(&table->cls, default_prefix_fields,
>> + ARRAY_SIZE(default_prefix_fields));
>> + fat_rwlock_unlock(&table->cls.rwlock);
>> }
>>
>> /* Destroys 'table', including its classifier and eviction groups.
>> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
>> index 9ba6354..de078b7 100644
>> --- a/ofproto/ofproto.h
>> +++ b/ofproto/ofproto.h
>> @@ -381,6 +381,9 @@ struct ofproto_table_settings {
>> enum mf_field_id prefix_fields[CLS_MAX_TRIES];
>> };
>>
>> +extern const enum mf_field_id default_prefix_fields[2];
>> +BUILD_ASSERT_DECL(ARRAY_SIZE(default_prefix_fields) <= CLS_MAX_TRIES);
>> +
>> int ofproto_get_n_tables(const struct ofproto *);
>> uint8_t ofproto_get_n_visible_tables(const struct ofproto *);
>> void ofproto_configure_table(struct ofproto *, int table_id,
>> diff --git a/tests/classifier.at b/tests/classifier.at
>> index 5338a11..7bd0493 100644
>> --- a/tests/classifier.at
>> +++ b/tests/classifier.at
>> @@ -66,14 +66,6 @@ AT_SETUP([flow classifier - prefix lookup])
>> OVS_VSWITCHD_START
>> ADD_OF_PORTS([br0], [1], [2], [3])
>> AT_CHECK([ovs-vsctl set Bridge br0 flow_tables:0=@N1 -- --id=@N1 create Flow_Table name=t0], [0], [ignore], [])
>> -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_dst], [1], [],
>> -[ovs-vsctl: nw_dst,nw_dst: set contains duplicate value
>> -])
>> -AT_CHECK([ovs-vsctl set Flow_Table t0 prefixes=nw_dst], [0])
>> AT_DATA([flows.txt], [dnl
>> table=0 in_port=1 priority=16,tcp,nw_dst=10.1.0.0/255.255.0.0,action=output(3)
>> table=0 in_port=1 priority=32,tcp,nw_dst=10.1.2.0/255.255.255.0,tp_src=79,action=output(2)
>> @@ -87,6 +79,23 @@ table=0 in_port=3 priority=16,tcp,nw_src=10.1.0.0/255.255.0.0,action=output(1)
>> table=0 in_port=3 priority=0,ip,action=drop
>> ])
>> AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>> +
>> +# nw_dst and nw_src should be on by default
>> +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=192.168.0.2,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,skb_priority=0,tcp,in_port=1,nw_dst=192.168.0.0/16,nw_frag=no
>> +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_dst], [1], [],
>> +[ovs-vsctl: nw_dst,nw_dst: set contains duplicate value
>> +])
>> +
>> +AT_CHECK([ovs-vsctl set Flow_Table t0 prefixes=nw_dst], [0])
>> 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=192.168.0.2,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,skb_priority=0,tcp,in_port=1,nw_dst=192.168.0.0/16,nw_frag=no
>> @@ -107,5 +116,11 @@ AT_CHECK([tail -2 stdout], [0],
>> [Megaflow: recirc_id=0,skb_priority=0,tcp,in_port=1,nw_dst=10.1.2.15,nw_frag=no,tp_src=0x0/0xffc0,tp_dst=0x40/0xfff0
>> Datapath actions: 3
>> ])
>> +AT_CHECK([ovs-vsctl set Flow_Table t0 prefixes=none], [0])
>> +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.3.16,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,skb_priority=0,tcp,in_port=1,nw_dst=10.1.3.16,nw_frag=no
>> +Datapath actions: 3
>> +])
>> OVS_VSWITCHD_STOP(["/'prefixes' with incompatible field: ipv6_label/d"])
>> AT_CLEANUP
>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>> index 12852b4..c760869 100644
>> --- a/vswitchd/bridge.c
>> +++ b/vswitchd/bridge.c
>> @@ -3077,6 +3077,7 @@ bridge_configure_tables(struct bridge *br)
>>
>> if (j < br->cfg->n_flow_tables && i == br->cfg->key_flow_tables[j]) {
>> struct ovsrec_flow_table *cfg = br->cfg->value_flow_tables[j++];
>> + bool no_prefixes;
>>
>> s.name = cfg->name;
>> if (cfg->n_flow_limit && *cfg->flow_limit < UINT_MAX) {
>> @@ -3105,10 +3106,17 @@ bridge_configure_tables(struct bridge *br)
>> }
>> }
>> /* Prefix lookup fields. */
>> + no_prefixes = false;
>> s.n_prefix_fields = 0;
>> for (k = 0; k < cfg->n_prefixes; k++) {
>> const char *name = cfg->prefixes[k];
>> - const struct mf_field *mf = mf_from_name(name);
>> + const struct mf_field *mf;
>> +
>> + if (strcmp(name, "none") == 0) {
>> + no_prefixes = true;
>> + continue;
>> + }
>> + mf = mf_from_name(name);
>> if (!mf) {
>> VLOG_WARN("bridge %s: 'prefixes' with unknown field: %s",
>> br->name, name);
>> @@ -3126,6 +3134,12 @@ bridge_configure_tables(struct bridge *br)
>> }
>> s.prefix_fields[s.n_prefix_fields++] = mf->id;
>> }
>> + if (s.n_prefix_fields == 0 && !no_prefixes) {
>> + /* Use default values. */
>> + s.n_prefix_fields = ARRAY_SIZE(default_prefix_fields);
>> + memcpy(s.prefix_fields, default_prefix_fields,
>> + sizeof default_prefix_fields);
>> + }
>> if (s.n_prefix_fields > 0) {
>> int k;
>> struct ds ds = DS_EMPTY_INITIALIZER;
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 7f2fd58..8ee2d51 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2608,12 +2608,25 @@
>> feature for <code>tun_id</code> would only make sense if the
>> tunnel IDs have prefix structure similar to IP addresses.)
>> </p>
>> +
>> + <p>
>> + By default, the <code>prefixes=ip_dst,ip_src</code> are used
>> + on each flow table. This instructs the flow classifier to
>> + track the IP destination and source addresses used by the
>> + rules in this specific flow table.
>> + </p>
>> +
>> <p>
>> - For example, <code>prefixes=ip_dst,ip_src</code> instructs the
>> - flow classifier to track the IP destination and source
>> - addresses used by the rules in this specific flow table. To
>> - set the prefix fields, the flow table record needs to exist:
>> + The keyword <code>none</code> is recognized as an explicit
>> + override of the default values, causing no prefix fields to be
>> + tracked.
>> </p>
>> +
>> + <p>
>> + To set the prefix fields, the flow table record needs to
>> + exist:
>> + </p>
>> +
>> <dl>
>> <dt><code>ovs-vsctl set Bridge br0 flow_tables:0=@N1 -- --id=@N1 create Flow_Table name=table0</code></dt>
>> <dd>
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list