From: Ben Pfaff Date: Tue, 28 Apr 2009 00:07:16 +0000 (-0700) Subject: secchan: Factor common code into new function rule_insert(). X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aec649d86f9f3e484190bef435be492946a85291;p=openvswitch secchan: Factor common code into new function rule_insert(). This is primarily a code cleanup. It also fixes a corner case for statistics that formerly was properly handled in add_flow() but not in ofproto_add_flow(). --- diff --git a/secchan/ofproto.c b/secchan/ofproto.c index b21acbb1..28e19ea0 100644 --- a/secchan/ofproto.c +++ b/secchan/ofproto.c @@ -163,6 +163,8 @@ static struct rule *rule_create(struct rule *super, const union ofp_action *, static void rule_free(struct rule *); static void rule_destroy(struct ofproto *, struct rule *); static struct rule *rule_from_cls_rule(const struct cls_rule *); +static void rule_insert(struct ofproto *, struct rule *, + struct ofpbuf *packet); static bool rule_make_actions(struct ofproto *, struct rule *, const struct ofpbuf *packet); static void rule_install(struct ofproto *, struct rule *, @@ -956,24 +958,11 @@ ofproto_add_flow(struct ofproto *p, const union ofp_action *actions, size_t n_actions, int idle_timeout) { - struct rule *rule, *displaced_rule; - + struct rule *rule; rule = rule_create(NULL, actions, n_actions, idle_timeout >= 0 ? idle_timeout : 5 /* XXX */, 0); cls_rule_from_flow(&rule->cr, flow, wildcards, priority); - - displaced_rule = rule_from_cls_rule(classifier_insert(&p->cls, &rule->cr)); - if (displaced_rule) { - rule_destroy(p, displaced_rule); - } - - if (!wildcards) { - rule_make_actions(p, rule, NULL); - rule_install(p, rule, NULL); - } else { - /* We might need to change the rules for arbitrary subrules. */ - p->need_revalidate = true; - } + rule_insert(p, rule, NULL); } void @@ -1399,6 +1388,46 @@ rule_has_out_port(const struct rule *rule, uint16_t out_port) return false; } +static void +rule_insert(struct ofproto *p, struct rule *rule, struct ofpbuf *packet) +{ + struct rule *displaced_rule; + + displaced_rule = rule_from_cls_rule(classifier_insert(&p->cls, &rule->cr)); + if (rule->cr.wc.wildcards) { + if (displaced_rule) { + /* The displaced rule matches exactly the same packets as the new + * rule, and it has the same priority, so we could usually transfer + * all displaced_rule's subrules to the new rule, then update the + * subrules' flow in the datapath. We used to do this. But there + * is one case where this optimization doesn't work: when + * 'displaced_rule' is a rule selected by NXAST_RESUBMIT, because + * in such a case 'displaced_rule' will not be the super-rule of + * the subrule that got actions from 'displaced_rule'. We could + * add more data to track such situations, but we have no evidence + * that the optimization would be worthwhile performance-wise + * anyhow. So we just let rule_destroy() check each subrule and + * transfer or destroy them as necessary. + */ + } + + /* We might need to change the rules for arbitrary subrules. */ + p->need_revalidate = true; + } else { + struct odp_flow_stats stats; + + rule_make_actions(p, rule, packet); + rule_install(p, rule, &stats); + + if (displaced_rule && displaced_rule->super) { + update_stats(displaced_rule->super, &stats); + } + } + if (displaced_rule) { + rule_destroy(p, displaced_rule); + } +} + /* Returns true if the actions changed, false otherwise. */ static bool rule_make_actions(struct ofproto *p, struct rule *rule, @@ -2386,6 +2415,7 @@ send_buffered(struct ofproto *p, struct ofconn *ofconn, uint32_t buffer_id, flow_t flow; int error; + *packetp = NULL; if (!ofconn->pktbuf) { VLOG_WARN_RL(&rl, "attempt to send buffered packet via connection " "without buffers"); @@ -2421,9 +2451,9 @@ static int add_flow(struct ofproto *p, struct ofconn *ofconn, struct ofp_flow_mod *ofm, size_t n_actions) { - struct rule *rule, *displaced_rule; - struct ofpbuf *packet = NULL; - int error = 0; + struct ofpbuf *packet; + struct rule *rule; + int error; rule = rule_create(NULL, (const union ofp_action *) ofm->actions, n_actions, ntohs(ofm->idle_timeout), @@ -2432,41 +2462,12 @@ add_flow(struct ofproto *p, struct ofconn *ofconn, if (ofm->buffer_id != htonl(UINT32_MAX)) { error = send_buffered(p, ofconn, ntohl(ofm->buffer_id), rule, &packet); - } - - displaced_rule = rule_from_cls_rule(classifier_insert(&p->cls, &rule->cr)); - if (rule->cr.wc.wildcards) { - if (displaced_rule) { - /* The displaced rule matches exactly the same packets as the new - * rule, and it has the same priority, so we could usually transfer - * all displaced_rule's subrules to the new rule, then update the - * subrules' flow in the datapath. We used to do this. But there - * is one case where this optimization doesn't work: when - * 'displaced_rule' is a rule selected by NXAST_RESUBMIT, because - * in such a case 'displaced_rule' will not be the super-rule of - * the subrule that got actions from 'displaced_rule'. We could - * add more data to track such situations, but we have no evidence - * that the optimization would be worthwhile performance-wise - * anyhow. So we just let rule_destroy() check each subrule and - * transfer or destroy them as necessary. - */ - rule_destroy(p, displaced_rule); - } - - /* We might need to change the rules for arbitrary subrules. */ - p->need_revalidate = true; } else { - struct odp_flow_stats stats; - - rule_make_actions(p, rule, packet); - rule_install(p, rule, &stats); - if (displaced_rule) { - if (displaced_rule->super) { - update_stats(displaced_rule->super, &stats); - } - rule_destroy(p, displaced_rule); - } + packet = NULL; + error = 0; } + + rule_insert(p, rule, packet); ofpbuf_delete(packet); return error; }