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 <[email protected]> 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;
> }
_______________________________________________
discuss mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/discuss
