On Fri, Apr 18, 2014 at 12:59 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> On Apr 18, 2014, at 2:50 AM, Andy Zhou <az...@nicira.com> wrote:
>
>> Currently, all packet lookup starts from internal table for possible
>> matching of post recirculation rules. This is not necessary for
>> datapath that does not support recirculation.
>>
>> This patch adds the ability to steering rule lookup starting table
>> based on whether datapath supports recirculation.
>>
>> Signed-off-by: Andy Zhou <az...@nicira.com>¬
>> ---
>> ofproto/ofproto-dpif.c | 83
>> +++++++++++++++++++++++++-------------------------
>> 1 file changed, 41 insertions(+), 42 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 3648dd7..1e51ff2 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -3171,41 +3171,57 @@ rule_dpif_get_actions(const struct rule_dpif *rule)
>> return rule_get_actions(&rule->up);
>> }
>>
>> -static uint8_t
>> -rule_dpif_lookup__ (struct ofproto_dpif *ofproto, const struct flow *flow,
>> - struct flow_wildcards *wc, struct rule_dpif **rule)
>> +/* Lookup 'flow' in table 0 of 'ofproto''s classifier.
>> + * If 'wc' is non-null, sets the fields that were relevant as part of
>> + * the lookup. Returns the table_id where a match or miss occurred.
>> + *
>> + * The return value will be zero unless there was a miss and
>> + * OFPTC11_TABLE_MISS_CONTINUE is in effect for the sequence of tables
>> + * where misses occur. */
>> +uint8_t
>> +rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
>> + struct flow_wildcards *wc, struct rule_dpif **rule)
>> {
>> enum rule_dpif_lookup_verdict verdict;
>> enum ofputil_port_config config = 0;
>> - uint8_t table_id = TBL_INTERNAL;
>> + uint8_t table_id;
>> +
>> + if (ofproto_dpif_get_enable_recirc(ofproto)) {
>
> Would this test be faster as “if (flow->recirc_id)”?
Yes. However the intention is to for non-recirc flows to hit the
recirc_id=0, actions=resubmit(table 0) rule to indicate recirc_id is
significant.
the
>
>> + /* Set metadata to the value of recirc_id to speed up internal
>> + * rule lookup. */
>> + flow->metadata = htonll(flow->recirc_id);
>
> Since we currently have only one kind of a match pattern in the internal
> table, this will actually make the lookup slightly slower.
This helps if we have multiple bond ports, each one has its own
recirc_id. MPLS recirc may also benefit from this.
>
>> + table_id = TBL_INTERNAL;
>> + } else {
>> + table_id = 0;
>> + }
>>
>> verdict = rule_dpif_lookup_from_table(ofproto, flow, wc, true,
>> &table_id, rule);
>>
>> switch (verdict) {
>> - case RULE_DPIF_LOOKUP_VERDICT_MATCH:
>> - return table_id;
>> - case RULE_DPIF_LOOKUP_VERDICT_CONTROLLER: {
>> - struct ofport_dpif *port;
>> -
>> - port = get_ofp_port(ofproto, flow->in_port.ofp_port);
>> - if (!port) {
>> - VLOG_WARN_RL(&rl, "packet-in on unknown OpenFlow port %"PRIu16,
>> - flow->in_port.ofp_port);
>> + case RULE_DPIF_LOOKUP_VERDICT_MATCH:
>> + return table_id;
>> + case RULE_DPIF_LOOKUP_VERDICT_CONTROLLER: {
>> + struct ofport_dpif *port;
>> +
>> + port = get_ofp_port(ofproto, flow->in_port.ofp_port);
>> + if (!port) {
>> + VLOG_WARN_RL(&rl, "packet-in on unknown OpenFlow port
>> %"PRIu16,
>> + flow->in_port.ofp_port);
>> + }
>> + config = port ? port->up.pp.config : 0;
>> + break;
>> }
>> - config = port ? port->up.pp.config : 0;
>> - break;
>> - }
>> - case RULE_DPIF_LOOKUP_VERDICT_DROP:
>> - config = OFPUTIL_PC_NO_PACKET_IN;
>> - break;
>> - case RULE_DPIF_LOOKUP_VERDICT_DEFAULT:
>> - if (!connmgr_wants_packet_in_on_miss(ofproto->up.connmgr)) {
>> + case RULE_DPIF_LOOKUP_VERDICT_DROP:
>> config = OFPUTIL_PC_NO_PACKET_IN;
>> - }
>> - break;
>> - default:
>> - OVS_NOT_REACHED();
>> + break;
>> + case RULE_DPIF_LOOKUP_VERDICT_DEFAULT:
>> + if (!connmgr_wants_packet_in_on_miss(ofproto->up.connmgr)) {
>> + config = OFPUTIL_PC_NO_PACKET_IN;
>> + }
>> + break;
>> + default:
>> + OVS_NOT_REACHED();
>> }
>>
>
> The previous indentation was according to the CodingStyle. It seems to me
> nothing changed here?
I combined the underscored version function. The diffs are showing
because the code being moved around. Nothing is changed.
>
>> choose_miss_rule(config, ofproto->miss_rule,
>> @@ -3213,23 +3229,6 @@ rule_dpif_lookup__ (struct ofproto_dpif *ofproto,
>> const struct flow *flow,
>> return table_id;
>> }
>>
>> -/* Lookup 'flow' in table 0 of 'ofproto''s classifier.
>> - * If 'wc' is non-null, sets the fields that were relevant as part of
>> - * the lookup. Returns the table_id where a match or miss occurred.
>> - *
>> - * The return value will be zero unless there was a miss and
>> - * OFPTC11_TABLE_MISS_CONTINUE is in effect for the sequence of tables
>> - * where misses occur. */
>> -uint8_t
>> -rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
>> - struct flow_wildcards *wc, struct rule_dpif **rule)
>> -{
>> - /* Set metadata to the value of recirc_id to speed up internal
>> - * rule lookup. */
>> - flow->metadata = htonll(flow->recirc_id);
>> - return rule_dpif_lookup__(ofproto, flow, wc, rule);
>> -}
>> -
>> static struct rule_dpif *
>> rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id,
>> const struct flow *flow, struct flow_wildcards *wc)
>> --
>> 1.9.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
>
> With the caveats above,
>
> Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
Thanks, I will wait a bit before pushing in case you have additional comments.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
