[ovs-discuss] Question about handle_odp_miss_msg() in ofproto.c

YIMIN CHEN ymchen.nbzj at gmail.com
Tue Jun 5 02:53:52 UTC 2012


Hi,

To add to my previous email, I did see the following comment for rule_execute():

 * If 'rule' is an exact-match rule and 'flow' actually equals the rule's flow,
 * the caller must already have accurately composed ODP actions for it given
 * 'packet' using rule_make_actions().  If 'rule' is a wildcard rule, or if
 * 'rule' is an exact-match rule but 'flow' is not the rule's flow, then this
 * function will compose a set of ODP actions based on 'rule''s OpenFlow
 * actions and apply them to 'packet'.
 *

>From the comment, seems like rule_execute() will construct odp actions for
1) wildcards
2) exact-match with no matching flow.

rule_make_action() will construct for
1) exact-match with matching flow

But in handle_odp_miss_msg(), rule_make_actions() is called this way,
seems for wildcard as well:

    if (rule->cr.wc.wildcards) {  <=== so called for wildcard as well???
        rule = rule_create_subrule(p, rule, &flow);
        rule_make_actions(p, rule, packet);
    } else {
        if (!rule->may_install) {
            /* The rule is not installable, that is, we need to process every
             * packet, so process the current packet and set its actions into
             * 'subrule'. */
            rule_make_actions(p, rule, packet);
        } else {
            /* XXX revalidate rule if it needs it */
        }
    }

That is why I am confused. Your clarification is appreciated!

Thanks!
Yimin


On Tue, Jun 5, 2012 at 10:44 AM, YIMIN CHEN <ymchen.nbzj at gmail.com> wrote:
> Hi,
>
> Sorry to bother you again!
>
> I am working on openvswitch 1.1.0pre2 package, and trying to
> understand the logic in handle_odp_miss_msg() in ofproto.c. I would
> really appreciate anyone familiar with this code giving me some
> feedback. I have copied the code snip below.
>
> My question is:
> 1) What is the main difference between rule_make_actions() and
> rule_execute()? In handle_odp_miss_msg(), we call both
> rule_make_actions() and rule_execute(), both calls xact_actions() that
> goes through list of actions to contruct odp actions. I can see
> rule_execute() not only calls xact_actions() to construct, but also
> execute odp actions. But I couldn't figure out the reason for doing
> construction twice. I must be missing some logic here. Could anyone
> please clarify for me? Same questions I inlined below as well.
>
> Thanks!
>
> static void
> handle_odp_miss_msg(struct ofproto *p, struct ofpbuf *packet)
> {
>    struct odp_msg *msg = packet->data;
>    struct rule *rule;
>    struct ofpbuf payload;
>    flow_t flow;
> ...
> ...
>    rule = lookup_valid_rule(p, &flow);
>    if (!rule) {
>        /* Don't send a packet-in if OFPPC_NO_PACKET_IN asserted. */
>        struct ofport *port = port_array_get(&p->ports, msg->port);
>        if (port) {
>            if (port->opp.config & OFPPC_NO_PACKET_IN) {
>                COVERAGE_INC(ofproto_no_packet_in);
>                /* XXX install 'drop' flow entry */
>                ofpbuf_delete(packet);
>                return;
>            }
>        } else {
>            VLOG_WARN_RL(&rl, "packet-in on unknown port %"PRIu16, msg->port);
>        }
>
>        COVERAGE_INC(ofproto_packet_in);
>        send_packet_in(p, packet);
>        return;
>    }
>
>    if (rule->cr.wc.wildcards) { <=== so if it is a wildcard rule we
> construct the odp actions
>        rule = rule_create_subrule(p, rule, &flow);
>        rule_make_actions(p, rule, packet);
>    } else {
>        if (!rule->may_install) {
>            /* The rule is not installable, that is, we need to process every
>             * packet, so process the current packet and set its actions into
>             * 'subrule'. */
>            rule_make_actions(p, rule, packet);
>        } else {
>            /* XXX revalidate rule if it needs it */
>        }
>    }
>
>    if (rule->super && rule->super->cr.priority == FAIL_OPEN_PRIORITY) {
>        /*
>         * Extra-special case for fail-open mode.
>         *
>         * We are in fail-open mode and the packet matched the fail-open rule,
>         * but we are connected to a controller too.  We should send the packet
>         * up to the controller in the hope that it will try to set up a flow
>         * and thereby allow us to exit fail-open.
>         *
>         * See the top-level comment in fail-open.c for more information.
>         */
>        send_packet_in(p, ofpbuf_clone_with_headroom(packet,
>                                                     DPIF_RECV_MSG_PADDING));
>    }
>
>    ofpbuf_pull(packet, sizeof *msg);
>    rule_execute(p, rule, packet, &flow); <== in rule_execute
>    rule_reinstall(p, rule);
>
> static void
> rule_execute(struct ofproto *ofproto, struct rule *rule,
>             struct ofpbuf *packet, const flow_t *flow)
> {
>    const union odp_action *actions;
>    struct odp_flow_stats stats;
>    size_t n_actions;
>    struct odp_actions a;
>
>    assert(ofpbuf_headroom(packet) >= sizeof(struct ofp_packet_in));
>
>    /* Grab or compose the ODP actions.
>     *
>     * The special case for an exact-match 'rule' where 'flow' is not the
>     * rule's flow is important to avoid, e.g., sending a packet out its input
>     * port simply because the ODP actions were composed for the wrong
>     * scenario. */
>    if (rule->cr.wc.wildcards || !flow_equal(flow, &rule->cr.flow)) {
> <=== if wildcard we call xlate_actions() again, why?
>        struct rule *super = rule->super ? rule->super : rule;
>        if (xlate_actions(super->actions, super->n_actions, flow, ofproto,
>                          packet, &a, NULL, 0, NULL)) {
>            ofpbuf_delete(packet);
>            return;
>        }
>        actions = a.actions;
>        n_actions = a.n_actions;
>    } else {
>        actions = rule->odp_actions;
>        n_actions = rule->n_odp_actions;
>    }



More information about the discuss mailing list