> On Apr 18, 2014, at 1:09 PM, Andy Zhou <[email protected]> wrote:
>
>> On Fri, Apr 18, 2014 at 12:59 PM, Jarno Rajahalme <[email protected]>
>> wrote:
>>
>>> On Apr 18, 2014, at 2:50 AM, Andy Zhou <[email protected]> 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 <[email protected]>¬
>>> ---
>>> 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
You could also mark the mask bits manually, and save the cost of the extra
classifier lookup.
>>
>>> + /* 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.
They still have the same mask, so partitioning has no benefit.
>>
>>> + 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.
Nonetheless you seem to have different indentations in switch and the cases?
>>
>>> 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
>>> [email protected]
>>> http://openvswitch.org/mailman/listinfo/dev
>>
>>
>> With the caveats above,
>>
>> Acked-by: Jarno Rajahalme <[email protected]>
> Thanks, I will wait a bit before pushing in case you have additional comments.
>>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
