Looks Good.
On Tue, Mar 15, 2011 at 11:36 AM, Ben Pfaff <b...@nicira.com> wrote: > This helps to increase the level of abstraction of "struct ofconn", > in preparation for moving it from ofproto.c into a new file. > --- > ofproto/ofproto.c | 54 ++++++++++++++++++++++++++++++---------------------- > 1 files changed, 31 insertions(+), 23 deletions(-) > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index 23069eb..bbceae2 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -2520,20 +2520,26 @@ queue_tx(struct ofpbuf *msg, const struct ofconn > *ofconn, > } > > static void > +ofconn_send_reply(const struct ofconn *ofconn, struct ofpbuf *msg) > +{ > + queue_tx(msg, ofconn, ofconn->reply_counter); > +} > + > +static void > send_error_oh(const struct ofconn *ofconn, const struct ofp_header *oh, > int error) > { > struct ofpbuf *buf = ofputil_encode_error_msg(error, oh); > if (buf) { > COVERAGE_INC(ofproto_error); > - queue_tx(buf, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, buf); > } > } > > static int > handle_echo_request(struct ofconn *ofconn, const struct ofp_header *oh) > { > - queue_tx(make_echo_reply(oh), ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, make_echo_reply(oh)); > return 0; > } > > @@ -2568,7 +2574,7 @@ handle_features_request(struct ofconn *ofconn, const > struct ofp_header *oh) > hton_ofp_phy_port(ofpbuf_put(buf, &port->opp, sizeof port->opp)); > } > > - queue_tx(buf, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, buf); > return 0; > } > > @@ -2589,7 +2595,7 @@ handle_get_config_request(struct ofconn *ofconn, const > struct ofp_header *oh) > osc = make_openflow_xid(sizeof *osc, OFPT_GET_CONFIG_REPLY, oh->xid, > &buf); > osc->flags = htons(flags); > osc->miss_send_len = htons(ofconn->miss_send_len); > - queue_tx(buf, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, buf); > > return 0; > } > @@ -3275,7 +3281,7 @@ append_ofp_stats_reply(size_t nbytes, struct ofconn > *ofconn, > struct ofp_stats_reply *reply = msg->data; > reply->flags = htons(OFPSF_REPLY_MORE); > *msgp = make_ofp_stats_reply(reply->header.xid, reply->type, nbytes); > - queue_tx(msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, msg); > } > return ofpbuf_put_uninit(*msgp, nbytes); > } > @@ -3311,7 +3317,7 @@ append_nxstats_reply(size_t nbytes, struct ofconn > *ofconn, > struct nicira_stats_msg *reply = msg->data; > reply->flags = htons(OFPSF_REPLY_MORE); > *msgp = make_nxstats_reply(reply->header.xid, reply->subtype, nbytes); > - queue_tx(msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, msg); > } > ofpbuf_prealloc_tailroom(*msgp, nbytes); > } > @@ -3332,7 +3338,7 @@ handle_desc_stats_request(struct ofconn *ofconn, > ovs_strlcpy(ods->sw_desc, p->sw_desc, sizeof ods->sw_desc); > ovs_strlcpy(ods->serial_num, p->serial_desc, sizeof ods->serial_num); > ovs_strlcpy(ods->dp_desc, p->dp_desc, sizeof ods->dp_desc); > - queue_tx(msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, msg); > > return 0; > } > @@ -3358,7 +3364,7 @@ handle_table_stats_request(struct ofconn *ofconn, > put_32aligned_be64(&ots->lookup_count, htonll(0)); /* XXX */ > put_32aligned_be64(&ots->matched_count, htonll(0)); /* XXX */ > > - queue_tx(msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, msg); > return 0; > } > > @@ -3412,7 +3418,7 @@ handle_port_stats_request(struct ofconn *ofconn, const > struct ofp_header *oh) > } > } > > - queue_tx(msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, msg); > return 0; > } > > @@ -3507,7 +3513,7 @@ handle_flow_stats_request(struct ofconn *ofconn, const > struct ofp_header *oh) > put_ofp_flow_stats(ofconn, rule, fsr->out_port, &reply); > } > } > - queue_tx(reply, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, reply); > > return 0; > } > @@ -3584,7 +3590,7 @@ handle_nxst_flow(struct ofconn *ofconn, const struct > ofp_header *oh) > put_nx_flow_stats(ofconn, rule, nfsr->out_port, &reply); > } > } > - queue_tx(reply, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, reply); > > return 0; > } > @@ -3680,7 +3686,7 @@ handle_aggregate_stats_request(struct ofconn *ofconn, > reply = append_ofp_stats_reply(sizeof *reply, ofconn, &msg); > query_aggregate_stats(ofproto, &target, request->out_port, > request->table_id, reply); > - queue_tx(msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, msg); > return 0; > } > > @@ -3713,7 +3719,7 @@ handle_nxst_aggregate(struct ofconn *ofconn, const > struct ofp_header *oh) > reply = ofpbuf_put_uninit(buf, sizeof *reply); > query_aggregate_stats(ofproto, &target, request->out_port, > request->table_id, reply); > - queue_tx(buf, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, buf); > > return 0; > } > @@ -3801,7 +3807,7 @@ handle_queue_stats_request(struct ofconn *ofconn, const > struct ofp_header *oh) > ofpbuf_delete(cbdata.msg); > return ofp_mkerr(OFPET_QUEUE_OP_FAILED, OFPQOFC_BAD_PORT); > } > - queue_tx(cbdata.msg, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, cbdata.msg); > > return 0; > } > @@ -4209,7 +4215,7 @@ handle_role_request(struct ofconn *ofconn, const struct > ofp_header *oh) > > reply = make_nxmsg_xid(sizeof *reply, NXT_ROLE_REPLY, oh->xid, &buf); > reply->role = htonl(role); > - queue_tx(buf, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, buf); > > return 0; > } > @@ -4241,7 +4247,7 @@ handle_barrier_request(struct ofconn *ofconn, const > struct ofp_header *oh) > /* Currently, everything executes synchronously, so we can just > * immediately send the barrier reply. */ > ob = make_openflow_xid(sizeof *ob, OFPT_BARRIER_REPLY, oh->xid, &buf); > - queue_tx(buf, ofconn, ofconn->reply_counter); > + ofconn_send_reply(ofconn, buf); > return 0; > } > > @@ -4773,18 +4779,20 @@ rule_send_removed(struct ofproto *p, struct rule > *rule, uint8_t reason) > fr.byte_count = rule->byte_count; > > LIST_FOR_EACH (ofconn, node, &p->all_conns) { > + struct ofpbuf *msg; > + > if (!rconn_is_connected(ofconn->rconn) > || !ofconn_receives_async_msgs(ofconn)) { > continue; > } > > - /* Account flow expirations under ofconn->reply_counter, the counter > - * for replies to OpenFlow requests. That works because preventing > - * OpenFlow requests from being processed also prevents new flows > from > - * being added (and expiring). (It also prevents processing OpenFlow > - * requests that would not add new flows, so it is imperfect.) */ > - queue_tx(ofputil_encode_flow_removed(&fr, ofconn->flow_format), > - ofconn, ofconn->reply_counter); > + /* This accounts flow expirations as if they were replies to OpenFlow > + * requests. That works because preventing OpenFlow requests from > + * being processed also prevents new flows from being added (and > + * expiring). (It also prevents processing OpenFlow requests that > + * would not add new flows, so it is imperfect.) */ > + msg = ofputil_encode_flow_removed(&fr, ofconn->flow_format); > + ofconn_send_reply(ofconn, msg); > } > } > > -- > 1.7.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev