From: Ben Pfaff Date: Thu, 30 Apr 2009 00:19:08 +0000 (-0700) Subject: secchan: Fix TCP flags and IP TOS tracking for packets sent from userspace. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e791965dfd886f3adb76c3080373d5f0d61cb237;p=openvswitch secchan: Fix TCP flags and IP TOS tracking for packets sent from userspace. --- diff --git a/secchan/ofproto.c b/secchan/ofproto.c index 30e07fce..bed9e511 100644 --- a/secchan/ofproto.c +++ b/secchan/ofproto.c @@ -164,7 +164,7 @@ 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); + struct ofpbuf *packet, uint16_t in_port); static void rule_remove(struct ofproto *, struct rule *); static bool rule_make_actions(struct ofproto *, struct rule *, const struct ofpbuf *packet); @@ -964,7 +964,7 @@ ofproto_add_flow(struct ofproto *p, rule = rule_create(NULL, actions, n_actions, idle_timeout >= 0 ? idle_timeout : 5 /* XXX */, 0); cls_rule_from_flow(&rule->cr, flow, wildcards, priority); - rule_insert(p, rule, NULL); + rule_insert(p, rule, NULL, 0); } void @@ -1391,35 +1391,85 @@ rule_has_out_port(const struct rule *rule, uint16_t out_port) return false; } +/* Executes the actions indicated by 'rule' on 'packet', which is in flow + * 'flow' and is considered to have arrived on ODP port 'in_port'. + * + * The flow that 'packet' actually contains does not need to actually match + * 'rule'; the actions in 'rule' will be applied to it either way. Likewise, + * the packet and byte counters for 'rule' will be credited for the packet sent + * out whether or not the packet actually matches 'rule'. + * + * 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'. */ static void -rule_insert(struct ofproto *p, struct rule *rule, struct ofpbuf *packet) +rule_execute(struct ofproto *ofproto, struct rule *rule, + struct ofpbuf *packet, const flow_t *flow) +{ + const union odp_action *actions; + size_t n_actions; + struct odp_actions a; + + /* 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)) { + struct rule *super = rule->super ? rule->super : rule; + if (xlate_actions(super->actions, super->n_actions, flow, ofproto, + packet, &a, NULL, 0)) { + return; + } + actions = a.actions; + n_actions = a.n_actions; + } else { + actions = rule->odp_actions; + n_actions = rule->n_odp_actions; + } + + /* Execute the ODP actions. */ + if (!dpif_execute(&ofproto->dpif, flow->in_port, + actions, n_actions, packet)) { + struct odp_flow_stats stats; + flow_extract_stats(flow, packet, &stats); + update_stats(rule, &stats); + rule->used = time_msec(); + } +} + +static void +rule_insert(struct ofproto *p, struct rule *rule, struct ofpbuf *packet, + uint16_t in_port) { struct rule *displaced_rule; + /* Insert the rule in the classifier. */ 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. - */ - } + if (!rule->cr.wc.wildcards) { + rule_make_actions(p, rule, packet); + } + + /* Send the packet and credit it to the rule. */ + if (packet) { + flow_t flow; + flow_extract(packet, in_port, &flow); + rule_execute(p, rule, packet, &flow); + } - /* We might need to change the rules for arbitrary subrules. */ + /* Install the rule in the datapath only after sending the packet, to + * avoid packet reordering. */ + if (rule->cr.wc.wildcards) { p->need_revalidate = true; } else { - rule_make_actions(p, rule, packet); rule_install(p, rule, displaced_rule); } + + /* Free the rule that was displaced, if any. */ if (displaced_rule) { rule_destroy(p, displaced_rule); } @@ -2462,54 +2512,13 @@ update_stats(struct rule *rule, const struct odp_flow_stats *stats) } } -static int -send_buffered(struct ofproto *p, struct ofconn *ofconn, uint32_t buffer_id, - struct rule *rule, struct ofpbuf **packetp) -{ - struct odp_actions actions; - struct ofpbuf *packet; - uint16_t in_port; - flow_t flow; - int error; - - *packetp = NULL; - if (!ofconn->pktbuf) { - VLOG_WARN_RL(&rl, "attempt to send buffered packet via connection " - "without buffers"); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE); - } - - error = pktbuf_retrieve(ofconn->pktbuf, buffer_id, &packet, &in_port); - if (error) { - return error; - } - *packetp = packet; - - flow_extract(packet, in_port, &flow); - error = xlate_actions(rule->actions, rule->n_actions, &flow, p, packet, - &actions, NULL, NULL); - if (error) { - return error; - } - - error = dpif_execute(&p->dpif, in_port, - actions.actions, actions.n_actions, packet); - if (!error) { - struct odp_flow_stats stats; - - flow_extract_stats(&flow, packet, &stats); - update_stats(rule, &stats); - } - - return 0; -} - static int add_flow(struct ofproto *p, struct ofconn *ofconn, struct ofp_flow_mod *ofm, size_t n_actions) { struct ofpbuf *packet; struct rule *rule; + uint16_t in_port; int error; rule = rule_create(NULL, (const union ofp_action *) ofm->actions, @@ -2517,14 +2526,14 @@ add_flow(struct ofproto *p, struct ofconn *ofconn, ntohs(ofm->hard_timeout)); cls_rule_from_match(&rule->cr, &ofm->match, ntohs(ofm->priority)); + packet = NULL; + error = 0; if (ofm->buffer_id != htonl(UINT32_MAX)) { - error = send_buffered(p, ofconn, ntohl(ofm->buffer_id), rule, &packet); - } else { - packet = NULL; - error = 0; + error = pktbuf_retrieve(ofconn->pktbuf, ntohl(ofm->buffer_id), + &packet, &in_port); } - rule_insert(p, rule, packet); + rule_insert(p, rule, packet, in_port); ofpbuf_delete(packet); return error; } @@ -2893,12 +2902,7 @@ handle_odp_msg(struct ofproto *p, struct ofpbuf *packet) rule_install(p, rule, NULL); /* Execute rule on packet. */ - if (!dpif_execute(&p->dpif, msg->port, rule->odp_actions, - rule->n_odp_actions, &payload)) { - /* XXX use flow_extract_stats(). */ - rule->packet_count++; - rule->byte_count += payload.size; - } + rule_execute(p, rule, &payload, &flow); ofpbuf_delete(packet); } diff --git a/secchan/pktbuf.c b/secchan/pktbuf.c index 9fb0017f..1370f649 100644 --- a/secchan/pktbuf.c +++ b/secchan/pktbuf.c @@ -124,6 +124,12 @@ pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct ofpbuf **bufferp, struct packet *p; int error; + if (!pb) { + VLOG_WARN_RL(&rl, "attempt to send buffered packet via connection " + "without buffers"); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE); + } + p = &pb->packets[id & PKTBUF_MASK]; if (p->cookie == id >> PKTBUF_BITS) { struct ofpbuf *buffer = p->buffer;