| Message ID | 20181029225751.5936-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/6] connmgr: Modernize coding style. | expand |
Looks good to me, thanks! Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Mon, Oct 29, 2018 at 3:58 PM Ben Pfaff <blp@ovn.org> wrote: > This moves declarations closer to first use and merges them with > initialization when possible, moves "for" loop variable declarations into > the "for" statements where possible, and otherwise makes this code look > like it was written a little more recently than it was. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > ofproto/connmgr.c | 280 > ++++++++++++++++++++++-------------------------------- > 1 file changed, 112 insertions(+), 168 deletions(-) > > diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c > index db9f6bad9a74..c7532cf01217 100644 > --- a/ofproto/connmgr.c > +++ b/ofproto/connmgr.c > @@ -242,9 +242,7 @@ struct connmgr * > connmgr_create(struct ofproto *ofproto, > const char *name, const char *local_port_name) > { > - struct connmgr *mgr; > - > - mgr = xmalloc(sizeof *mgr); > + struct connmgr *mgr = xmalloc(sizeof *mgr); > mgr->ofproto = ofproto; > mgr->name = xstrdup(name); > mgr->local_port_name = xstrdup(local_port_name); > @@ -304,26 +302,24 @@ void > connmgr_destroy(struct connmgr *mgr) > OVS_REQUIRES(ofproto_mutex) > { > - struct ofservice *ofservice, *next_ofservice; > - struct ofconn *ofconn, *next_ofconn; > - size_t i; > - > if (!mgr) { > return; > } > > + struct ofconn *ofconn, *next_ofconn; > LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) { > ofconn_destroy(ofconn); > } > > hmap_destroy(&mgr->controllers); > > + struct ofservice *ofservice, *next_ofservice; > HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, node, &mgr->services) { > ofservice_destroy(mgr, ofservice); > } > hmap_destroy(&mgr->services); > > - for (i = 0; i < mgr->n_snoops; i++) { > + for (size_t i = 0; i < mgr->n_snoops; i++) { > pvconn_close(mgr->snoops[i]); > } > free(mgr->snoops); > @@ -350,10 +346,6 @@ connmgr_run(struct connmgr *mgr, > const struct ofpbuf *ofp_msg)) > OVS_EXCLUDED(ofproto_mutex) > { > - struct ofconn *ofconn, *next_ofconn; > - struct ofservice *ofservice; > - size_t i; > - > if (mgr->in_band) { > if (!in_band_run(mgr->in_band)) { > in_band_destroy(mgr->in_band); > @@ -361,6 +353,7 @@ connmgr_run(struct connmgr *mgr, > } > } > > + struct ofconn *ofconn, *next_ofconn; > LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) { > ofconn_run(ofconn, handle_openflow); > } > @@ -372,19 +365,16 @@ connmgr_run(struct connmgr *mgr, > fail_open_run(mgr->fail_open); > } > > + struct ofservice *ofservice; > HMAP_FOR_EACH (ofservice, node, &mgr->services) { > struct vconn *vconn; > - int retval; > - > - retval = pvconn_accept(ofservice->pvconn, &vconn); > + int retval = pvconn_accept(ofservice->pvconn, &vconn); > if (!retval) { > - struct rconn *rconn; > - char *name; > - > /* Passing default value for creation of the rconn */ > - rconn = rconn_create(ofservice->probe_interval, 0, > ofservice->dscp, > - vconn_get_allowed_versions(vconn)); > - name = ofconn_make_name(mgr, vconn_get_name(vconn)); > + struct rconn *rconn = rconn_create( > + ofservice->probe_interval, 0, ofservice->dscp, > + vconn_get_allowed_versions(vconn)); > + char *name = ofconn_make_name(mgr, vconn_get_name(vconn)); > rconn_connect_unreliably(rconn, vconn, name); > free(name); > > @@ -400,11 +390,9 @@ connmgr_run(struct connmgr *mgr, > } > } > > - for (i = 0; i < mgr->n_snoops; i++) { > + for (size_t i = 0; i < mgr->n_snoops; i++) { > struct vconn *vconn; > - int retval; > - > - retval = pvconn_accept(mgr->snoops[i], &vconn); > + int retval = pvconn_accept(mgr->snoops[i], &vconn); > if (!retval) { > add_snooper(mgr, vconn); > } else if (retval != EAGAIN) { > @@ -417,24 +405,27 @@ connmgr_run(struct connmgr *mgr, > void > connmgr_wait(struct connmgr *mgr) > { > - struct ofservice *ofservice; > struct ofconn *ofconn; > - size_t i; > - > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > ofconn_wait(ofconn); > } > + > ofmonitor_wait(mgr); > + > if (mgr->in_band) { > in_band_wait(mgr->in_band); > } > + > if (mgr->fail_open) { > fail_open_wait(mgr->fail_open); > } > + > + struct ofservice *ofservice; > HMAP_FOR_EACH (ofservice, node, &mgr->services) { > pvconn_wait(ofservice->pvconn); > } > - for (i = 0; i < mgr->n_snoops; i++) { > + > + for (size_t i = 0; i < mgr->n_snoops; i++) { > pvconn_wait(mgr->snoops[i]); > } > } > @@ -444,17 +435,15 @@ connmgr_wait(struct connmgr *mgr) > void > connmgr_get_memory_usage(const struct connmgr *mgr, struct simap *usage) > { > - const struct ofconn *ofconn; > unsigned int packets = 0; > unsigned int ofconns = 0; > > + const struct ofconn *ofconn; > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > - int i; > - > ofconns++; > > packets += rconn_count_txqlen(ofconn->rconn); > - for (i = 0; i < N_SCHEDULERS; i++) { > + for (int i = 0; i < N_SCHEDULERS; i++) { > struct pinsched_stats stats; > > pinsched_get_stats(ofconn->schedulers[i], &stats); > @@ -597,10 +586,6 @@ connmgr_set_controllers(struct connmgr *mgr, > OVS_EXCLUDED(ofproto_mutex) > { > bool had_controllers = connmgr_has_controllers(mgr); > - struct shash new_controllers; > - struct ofconn *ofconn, *next_ofconn; > - struct ofservice *ofservice, *next_ofservice; > - size_t i; > > /* Required to add and remove ofconns. This could probably be > narrowed to > * cover a smaller amount of code, if that yielded some benefit. */ > @@ -608,13 +593,13 @@ connmgr_set_controllers(struct connmgr *mgr, > > /* Create newly configured controllers and services. > * Create a name to ofproto_controller mapping in 'new_controllers'. > */ > - shash_init(&new_controllers); > - for (i = 0; i < n_controllers; i++) { > + struct shash new_controllers = SHASH_INITIALIZER(&new_controllers); > + for (size_t i = 0; i < n_controllers; i++) { > const struct ofproto_controller *c = &controllers[i]; > > if (!vconn_verify_name(c->target)) { > bool add = false; > - ofconn = find_controller_by_target(mgr, c->target); > + struct ofconn *ofconn = find_controller_by_target(mgr, > c->target); > if (!ofconn) { > VLOG_INFO("%s: added primary controller \"%s\"", > mgr->name, c->target); > @@ -631,7 +616,7 @@ connmgr_set_controllers(struct connmgr *mgr, > } > } else if (!pvconn_verify_name(c->target)) { > bool add = false; > - ofservice = ofservice_lookup(mgr, c->target); > + struct ofservice *ofservice = ofservice_lookup(mgr, > c->target); > if (!ofservice) { > VLOG_INFO("%s: added service controller \"%s\"", > mgr->name, c->target); > @@ -656,11 +641,11 @@ connmgr_set_controllers(struct connmgr *mgr, > > /* Delete controllers that are no longer configured. > * Update configuration of all now-existing controllers. */ > + struct ofconn *ofconn, *next_ofconn; > HMAP_FOR_EACH_SAFE (ofconn, next_ofconn, hmap_node, > &mgr->controllers) { > const char *target = ofconn_get_target(ofconn); > - struct ofproto_controller *c; > - > - c = shash_find_data(&new_controllers, target); > + struct ofproto_controller *c = shash_find_data(&new_controllers, > + target); > if (!c) { > VLOG_INFO("%s: removed primary controller \"%s\"", > mgr->name, target); > @@ -672,11 +657,11 @@ connmgr_set_controllers(struct connmgr *mgr, > > /* Delete services that are no longer configured. > * Update configuration of all now-existing services. */ > + struct ofservice *ofservice, *next_ofservice; > HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, node, &mgr->services) { > const char *target = pvconn_get_name(ofservice->pvconn); > - struct ofproto_controller *c; > - > - c = shash_find_data(&new_controllers, target); > + struct ofproto_controller *c = shash_find_data(&new_controllers, > + target); > if (!c) { > VLOG_INFO("%s: removed service controller \"%s\"", > mgr->name, target); > @@ -723,9 +708,7 @@ connmgr_set_snoops(struct connmgr *mgr, const struct > sset *snoops) > void > connmgr_get_snoops(const struct connmgr *mgr, struct sset *snoops) > { > - size_t i; > - > - for (i = 0; i < mgr->n_snoops; i++) { > + for (size_t i = 0; i < mgr->n_snoops; i++) { > sset_add(snoops, pvconn_get_name(mgr->snoops[i])); > } > } > @@ -745,10 +728,9 @@ add_controller(struct connmgr *mgr, const char > *target, uint8_t dscp, > OVS_REQUIRES(ofproto_mutex) > { > char *name = ofconn_make_name(mgr, target); > - struct ofconn *ofconn; > - > - ofconn = ofconn_create(mgr, rconn_create(5, 8, dscp, > allowed_versions), > - OFCONN_PRIMARY, true); > + struct ofconn *ofconn = ofconn_create(mgr, rconn_create(5, 8, dscp, > + > allowed_versions), > + OFCONN_PRIMARY, true); > rconn_connect(ofconn->rconn, target, name); > hmap_insert(&mgr->controllers, &ofconn->hmap_node, > hash_string(target, 0)); > > @@ -772,17 +754,13 @@ find_controller_by_target(struct connmgr *mgr, const > char *target) > static void > update_in_band_remotes(struct connmgr *mgr) > { > - struct sockaddr_in *addrs; > - size_t max_addrs, n_addrs; > - struct ofconn *ofconn; > - size_t i; > - > /* Allocate enough memory for as many remotes as we could possibly > have. */ > - max_addrs = mgr->n_extra_remotes + hmap_count(&mgr->controllers); > - addrs = xmalloc(max_addrs * sizeof *addrs); > - n_addrs = 0; > + size_t max_addrs = mgr->n_extra_remotes + > hmap_count(&mgr->controllers); > + struct sockaddr_in *addrs = xmalloc(max_addrs * sizeof *addrs); > + size_t n_addrs = 0; > > /* Add all the remotes. */ > + struct ofconn *ofconn; > HMAP_FOR_EACH (ofconn, hmap_node, &mgr->controllers) { > const char *target = rconn_get_target(ofconn->rconn); > union { > @@ -796,7 +774,7 @@ update_in_band_remotes(struct connmgr *mgr) > addrs[n_addrs++] = sa.in; > } > } > - for (i = 0; i < mgr->n_extra_remotes; i++) { > + for (size_t i = 0; i < mgr->n_extra_remotes; i++) { > addrs[n_addrs++] = mgr->extra_in_band_remotes[i]; > } > > @@ -839,25 +817,26 @@ static int > set_pvconns(struct pvconn ***pvconnsp, size_t *n_pvconnsp, > const struct sset *sset) > { > - struct pvconn **pvconns = *pvconnsp; > - size_t n_pvconns = *n_pvconnsp; > - const char *name; > - int retval = 0; > - size_t i; > - > - for (i = 0; i < n_pvconns; i++) { > - pvconn_close(pvconns[i]); > + /* Free the old pvconns. */ > + struct pvconn **old_pvconns = *pvconnsp; > + size_t old_n_pvconns = *n_pvconnsp; > + for (size_t i = 0; i < old_n_pvconns; i++) { > + pvconn_close(old_pvconns[i]); > } > - free(pvconns); > + free(old_pvconns); > + > + /* Populate the new pvconns. */ > + struct pvconn **new_pvconns = xmalloc(sset_count(sset) > + * sizeof *new_pvconns); > + size_t new_n_pvconns = 0; > > - pvconns = xmalloc(sset_count(sset) * sizeof *pvconns); > - n_pvconns = 0; > + int retval = 0; > + const char *name; > SSET_FOR_EACH (name, sset) { > struct pvconn *pvconn; > - int error; > - error = pvconn_open(name, 0, 0, &pvconn); > + int error = pvconn_open(name, 0, 0, &pvconn); > if (!error) { > - pvconns[n_pvconns++] = pvconn; > + new_pvconns[new_n_pvconns++] = pvconn; > } else { > VLOG_ERR("failed to listen on %s: %s", name, > ovs_strerror(error)); > if (!retval) { > @@ -866,8 +845,8 @@ set_pvconns(struct pvconn ***pvconnsp, size_t > *n_pvconnsp, > } > } > > - *pvconnsp = pvconns; > - *n_pvconnsp = n_pvconns; > + *pvconnsp = new_pvconns; > + *n_pvconnsp = new_n_pvconns; > > return retval; > } > @@ -897,10 +876,9 @@ snoop_preference(const struct ofconn *ofconn) > static void > add_snooper(struct connmgr *mgr, struct vconn *vconn) > { > - struct ofconn *ofconn, *best; > - > /* Pick a controller for monitoring. */ > - best = NULL; > + struct ofconn *best = NULL; > + struct ofconn *ofconn; > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > if (ofconn->type == OFCONN_PRIMARY > && (!best || snoop_preference(ofconn) > > snoop_preference(best))) { > @@ -969,13 +947,12 @@ void > ofconn_send_role_status(struct ofconn *ofconn, uint32_t role, uint8_t > reason) > { > struct ofputil_role_status status; > - struct ofpbuf *buf; > - > status.reason = reason; > status.role = role; > ofconn_get_master_election_id(ofconn, &status.generation_id); > > - buf = ofputil_encode_role_status(&status, > ofconn_get_protocol(ofconn)); > + struct ofpbuf *buf > + = ofputil_encode_role_status(&status, > ofconn_get_protocol(ofconn)); > if (buf) { > ofconn_send(ofconn, buf, NULL); > } > @@ -992,7 +969,8 @@ ofconn_set_role(struct ofconn *ofconn, enum > ofp12_controller_role role) > LIST_FOR_EACH (other, node, &ofconn->connmgr->all_conns) { > if (other->role == OFPCR12_ROLE_MASTER) { > other->role = OFPCR12_ROLE_SLAVE; > - ofconn_send_role_status(other, OFPCR12_ROLE_SLAVE, > OFPCRR_MASTER_REQUEST); > + ofconn_send_role_status(other, OFPCR12_ROLE_SLAVE, > + OFPCRR_MASTER_REQUEST); > } > } > } > @@ -1159,19 +1137,15 @@ ofconn_send_error(const struct ofconn *ofconn, > const struct ofp_header *request, enum ofperr error) > { > static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(10, 10); > - struct ofpbuf *reply; > - > - reply = ofperr_encode_reply(error, request); > + struct ofpbuf *reply = ofperr_encode_reply(error, request); > if (!VLOG_DROP_INFO(&err_rl)) { > - const char *type_name; > - size_t request_len; > - enum ofpraw raw; > + size_t request_len = ntohs(request->length); > > - request_len = ntohs(request->length); > - type_name = (!ofpraw_decode_partial(&raw, request, > - MIN(64, request_len)) > - ? ofpraw_get_name(raw) > - : "invalid"); > + enum ofpraw raw; > + const char *type_name = (!ofpraw_decode_partial(&raw, request, > + MIN(64, > request_len)) > + ? ofpraw_get_name(raw) > + : "invalid"); > > VLOG_INFO("%s: sending %s error reply to %s message", > rconn_get_name(ofconn->rconn), ofperr_to_string(error), > @@ -1186,8 +1160,6 @@ void > ofconn_report_flow_mod(struct ofconn *ofconn, > enum ofp_flow_mod_command command) > { > - long long int now; > - > switch (command) { > case OFPFC_ADD: > ofconn->n_add++; > @@ -1204,7 +1176,7 @@ ofconn_report_flow_mod(struct ofconn *ofconn, > break; > } > > - now = time_msec(); > + long long int now = time_msec(); > if (ofconn->next_op_report == LLONG_MAX) { > ofconn->first_op = now; > ofconn->next_op_report = MAX(now + 10 * 1000, ofconn->op_backoff); > @@ -1260,9 +1232,9 @@ bundle_remove_all(struct ofconn *ofconn) > static void > bundle_remove_expired(struct ofconn *ofconn, long long int now) > { > - struct ofp_bundle *b, *next; > long long int limit = now - bundle_idle_timeout; > > + struct ofp_bundle *b, *next; > HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) { > if (b->used <= limit) { > ofconn_send_error(ofconn, b->msg, OFPERR_OFPBFC_TIMEOUT); > @@ -1284,9 +1256,7 @@ ofconn_create(struct connmgr *mgr, struct rconn > *rconn, enum ofconn_type type, > bool enable_async_msgs) > OVS_REQUIRES(ofproto_mutex) > { > - struct ofconn *ofconn; > - > - ofconn = xzalloc(sizeof *ofconn); > + struct ofconn *ofconn = xzalloc(sizeof *ofconn); > ofconn->connmgr = mgr; > ovs_list_push_back(&mgr->all_conns, &ofconn->node); > ofconn->rconn = rconn; > @@ -1310,9 +1280,6 @@ static void > ofconn_flush(struct ofconn *ofconn) > OVS_REQUIRES(ofproto_mutex) > { > - struct ofmonitor *monitor, *next_monitor; > - int i; > - > ofconn_log_flow_mods(ofconn); > > ofconn->role = OFPCR12_ROLE_EQUAL; > @@ -1321,7 +1288,7 @@ ofconn_flush(struct ofconn *ofconn) > > rconn_packet_counter_destroy(ofconn->packet_in_counter); > ofconn->packet_in_counter = rconn_packet_counter_create(); > - for (i = 0; i < N_SCHEDULERS; i++) { > + for (int i = 0; i < N_SCHEDULERS; i++) { > if (ofconn->schedulers[i]) { > int rate, burst; > > @@ -1346,6 +1313,7 @@ ofconn_flush(struct ofconn *ofconn) > ofconn->next_op_report = LLONG_MAX; > ofconn->op_backoff = LLONG_MIN; > > + struct ofmonitor *monitor, *next_monitor; > HMAP_FOR_EACH_SAFE (monitor, next_monitor, ofconn_node, > &ofconn->monitors) { > ofmonitor_destroy(monitor); > @@ -1387,14 +1355,12 @@ ofconn_destroy(struct ofconn *ofconn) > static void > ofconn_reconfigure(struct ofconn *ofconn, const struct ofproto_controller > *c) > { > - int probe_interval; > - > ofconn->band = c->band; > ofconn->enable_async_msgs = c->enable_async_msgs; > > rconn_set_max_backoff(ofconn->rconn, c->max_backoff); > > - probe_interval = c->probe_interval ? MAX(c->probe_interval, 5) : 0; > + int probe_interval = c->probe_interval ? MAX(c->probe_interval, 5) : > 0; > rconn_set_probe_interval(ofconn->rconn, probe_interval); > > ofconn_set_rate_limit(ofconn, c->rate_limit, c->burst_limit); > @@ -1421,9 +1387,8 @@ ofconn_run(struct ofconn *ofconn, > const struct ofpbuf *ofp_msg)) > { > struct connmgr *mgr = ofconn->connmgr; > - size_t i; > > - for (i = 0; i < N_SCHEDULERS; i++) { > + for (size_t i = 0; i < N_SCHEDULERS; i++) { > struct ovs_list txq; > > pinsched_run(ofconn->schedulers[i], &txq); > @@ -1433,7 +1398,7 @@ ofconn_run(struct ofconn *ofconn, > rconn_run(ofconn->rconn); > > /* Limit the number of iterations to avoid starving other tasks. */ > - for (i = 0; i < 50 && ofconn_may_recv(ofconn); i++) { > + for (int i = 0; i < 50 && ofconn_may_recv(ofconn); i++) { > struct ofpbuf *of_msg = rconn_recv(ofconn->rconn); > if (!of_msg) { > break; > @@ -1470,9 +1435,7 @@ ofconn_run(struct ofconn *ofconn, > static void > ofconn_wait(struct ofconn *ofconn) > { > - int i; > - > - for (i = 0; i < N_SCHEDULERS; i++) { > + for (int i = 0; i < N_SCHEDULERS; i++) { > pinsched_wait(ofconn->schedulers[i]); > } > rconn_run_wait(ofconn->rconn); > @@ -1585,9 +1548,7 @@ ofconn_make_name(const struct connmgr *mgr, const > char *target) > static void > ofconn_set_rate_limit(struct ofconn *ofconn, int rate, int burst) > { > - int i; > - > - for (i = 0; i < N_SCHEDULERS; i++) { > + for (int i = 0; i < N_SCHEDULERS; i++) { > struct pinsched **s = &ofconn->schedulers[i]; > > if (rate > 0) { > @@ -1719,14 +1680,13 @@ connmgr_send_flow_removed(struct connmgr *mgr, > > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > if (ofconn_receives_async_msg(ofconn, OAM_FLOW_REMOVED, > fr->reason)) { > - struct ofpbuf *msg; > - > /* Account flow expirations as 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_get_protocol(ofconn)); > + struct ofpbuf *msg = ofputil_encode_flow_removed( > + fr, ofconn_get_protocol(ofconn)); > ofconn_send_reply(ofconn, msg); > } > } > @@ -1744,12 +1704,12 @@ connmgr_send_table_status(struct connmgr *mgr, > const struct ofputil_table_desc *td, > uint8_t reason) > { > - struct ofputil_table_status ts; > - struct ofconn *ofconn; > - > - ts.reason = reason; > - ts.desc = *td; > + struct ofputil_table_status ts = { > + .reason = reason, > + .desc = *td > + }; > > + struct ofconn *ofconn; > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > if (ofconn_receives_async_msg(ofconn, OAM_TABLE_STATUS, reason)) { > struct ofpbuf *msg; > @@ -1841,10 +1801,9 @@ connmgr_set_fail_mode(struct connmgr *mgr, enum > ofproto_fail_mode fail_mode) > int > connmgr_get_max_probe_interval(const struct connmgr *mgr) > { > - const struct ofconn *ofconn; > - int max_probe_interval; > + int max_probe_interval = 0; > > - max_probe_interval = 0; > + const struct ofconn *ofconn; > HMAP_FOR_EACH (ofconn, hmap_node, &mgr->controllers) { > int probe_interval = rconn_get_probe_interval(ofconn->rconn); > max_probe_interval = MAX(max_probe_interval, probe_interval); > @@ -1857,14 +1816,13 @@ connmgr_get_max_probe_interval(const struct > connmgr *mgr) > int > connmgr_failure_duration(const struct connmgr *mgr) > { > - const struct ofconn *ofconn; > - int min_failure_duration; > - > if (!connmgr_has_controllers(mgr)) { > return 0; > } > > - min_failure_duration = INT_MAX; > + int min_failure_duration = INT_MAX; > + > + const struct ofconn *ofconn; > HMAP_FOR_EACH (ofconn, hmap_node, &mgr->controllers) { > int failure_duration = rconn_failure_duration(ofconn->rconn); > min_failure_duration = MIN(min_failure_duration, > failure_duration); > @@ -1942,13 +1900,11 @@ static bool > any_extras_changed(const struct connmgr *mgr, > const struct sockaddr_in *extras, size_t n) > { > - size_t i; > - > if (n != mgr->n_extra_remotes) { > return true; > } > > - for (i = 0; i < n; i++) { > + for (size_t i = 0; i < n; i++) { > const struct sockaddr_in *old = &mgr->extra_in_band_remotes[i]; > const struct sockaddr_in *new = &extras[i]; > > @@ -2029,16 +1985,13 @@ static int > ofservice_create(struct connmgr *mgr, const char *target, > uint32_t allowed_versions, uint8_t dscp) > { > - struct ofservice *ofservice; > struct pvconn *pvconn; > - int error; > - > - error = pvconn_open(target, allowed_versions, dscp, &pvconn); > + int error = pvconn_open(target, allowed_versions, dscp, &pvconn); > if (error) { > return error; > } > > - ofservice = xzalloc(sizeof *ofservice); > + struct ofservice *ofservice = xzalloc(sizeof *ofservice); > hmap_insert(&mgr->services, &ofservice->node, hash_string(target, 0)); > ofservice->pvconn = pvconn; > ofservice->allowed_versions = allowed_versions; > @@ -2111,11 +2064,9 @@ ofmonitor_create(const struct > ofputil_flow_monitor_request *request, > struct ofconn *ofconn, struct ofmonitor **monitorp) > OVS_REQUIRES(ofproto_mutex) > { > - struct ofmonitor *m; > - > *monitorp = NULL; > > - m = ofmonitor_lookup(ofconn, request->id); > + struct ofmonitor *m = ofmonitor_lookup(ofconn, request->id); > if (m) { > return OFPERR_OFPMOFC_MONITOR_EXISTS; > } > @@ -2167,13 +2118,11 @@ ofmonitor_report(struct connmgr *mgr, struct rule > *rule, > const struct rule_actions *old_actions) > OVS_REQUIRES(ofproto_mutex) > { > - enum nx_flow_monitor_flags update; > - struct ofconn *ofconn; > - > if (rule_is_hidden(rule)) { > return; > } > > + enum nx_flow_monitor_flags update; > switch (event) { > case NXFME_ADDED: > update = NXFMF_ADD; > @@ -2194,10 +2143,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule > *rule, > OVS_NOT_REACHED(); > } > > + struct ofconn *ofconn; > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > - enum nx_flow_monitor_flags flags = 0; > - struct ofmonitor *m; > - > if (ofconn->monitor_paused) { > /* Only send NXFME_DELETED notifications for flows that were > added > * before we paused. */ > @@ -2207,6 +2154,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule > *rule, > } > } > > + enum nx_flow_monitor_flags flags = 0; > + struct ofmonitor *m; > HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) { > if (m->flags & update > && (m->table_id == 0xff || m->table_id == rule->table_id) > @@ -2243,7 +2192,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule > *rule, > ovs_mutex_unlock(&rule->mutex); > > if (flags & NXFMF_ACTIONS) { > - const struct rule_actions *actions = > rule_get_actions(rule); > + const struct rule_actions *actions > + = rule_get_actions(rule); > fu.ofpacts = actions->ofpacts; > fu.ofpacts_len = actions->ofpacts_len; > } else { > @@ -2282,12 +2232,10 @@ ofmonitor_flush(struct connmgr *mgr) > > if (!ofconn->monitor_paused > && rconn_packet_counter_n_bytes(counter) > 128 * 1024) { > - struct ofpbuf *pause; > - > COVERAGE_INC(ofmonitor_pause); > ofconn->monitor_paused = monitor_seqno++; > - pause = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_PAUSED, > - OFP10_VERSION, htonl(0), 0); > + struct ofpbuf *pause = ofpraw_alloc_xid( > + OFPRAW_NXT_FLOW_MONITOR_PAUSED, OFP10_VERSION, htonl(0), > 0); > ofconn_send(ofconn, pause, counter); > } > } > @@ -2298,20 +2246,18 @@ ofmonitor_resume(struct ofconn *ofconn) > OVS_REQUIRES(ofproto_mutex) > { > struct rule_collection rules; > - struct ofpbuf *resumed; > - struct ofmonitor *m; > - struct ovs_list msgs; > - > rule_collection_init(&rules); > + > + struct ofmonitor *m; > HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) { > ofmonitor_collect_resume_rules(m, ofconn->monitor_paused, &rules); > } > > - ovs_list_init(&msgs); > + struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs); > ofmonitor_compose_refresh_updates(&rules, &msgs); > > - resumed = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_RESUMED, > OFP10_VERSION, > - htonl(0), 0); > + struct ofpbuf *resumed = > ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_RESUMED, > + OFP10_VERSION, htonl(0), 0); > ovs_list_push_back(&msgs, &resumed->list_node); > ofconn_send_replies(ofconn, &msgs); > > @@ -2329,9 +2275,8 @@ ofmonitor_may_resume(const struct ofconn *ofconn) > static void > ofmonitor_run(struct connmgr *mgr) > { > - struct ofconn *ofconn; > - > ovs_mutex_lock(&ofproto_mutex); > + struct ofconn *ofconn; > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > if (ofmonitor_may_resume(ofconn)) { > COVERAGE_INC(ofmonitor_resume); > @@ -2344,9 +2289,8 @@ ofmonitor_run(struct connmgr *mgr) > static void > ofmonitor_wait(struct connmgr *mgr) > { > - struct ofconn *ofconn; > - > ovs_mutex_lock(&ofproto_mutex); > + struct ofconn *ofconn; > LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { > if (ofmonitor_may_resume(ofconn)) { > poll_immediate_wake(); > -- > 2.16.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index db9f6bad9a74..c7532cf01217 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -242,9 +242,7 @@ struct connmgr * connmgr_create(struct ofproto *ofproto, const char *name, const char *local_port_name) { - struct connmgr *mgr; - - mgr = xmalloc(sizeof *mgr); + struct connmgr *mgr = xmalloc(sizeof *mgr); mgr->ofproto = ofproto; mgr->name = xstrdup(name); mgr->local_port_name = xstrdup(local_port_name); @@ -304,26 +302,24 @@ void connmgr_destroy(struct connmgr *mgr) OVS_REQUIRES(ofproto_mutex) { - struct ofservice *ofservice, *next_ofservice; - struct ofconn *ofconn, *next_ofconn; - size_t i; - if (!mgr) { return; } + struct ofconn *ofconn, *next_ofconn; LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) { ofconn_destroy(ofconn); } hmap_destroy(&mgr->controllers); + struct ofservice *ofservice, *next_ofservice; HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, node, &mgr->services) { ofservice_destroy(mgr, ofservice); } hmap_destroy(&mgr->services); - for (i = 0; i < mgr->n_snoops; i++) { + for (size_t i = 0; i < mgr->n_snoops; i++) { pvconn_close(mgr->snoops[i]); } free(mgr->snoops); @@ -350,10 +346,6 @@ connmgr_run(struct connmgr *mgr, const struct ofpbuf *ofp_msg)) OVS_EXCLUDED(ofproto_mutex) { - struct ofconn *ofconn, *next_ofconn; - struct ofservice *ofservice; - size_t i; - if (mgr->in_band) { if (!in_band_run(mgr->in_band)) { in_band_destroy(mgr->in_band); @@ -361,6 +353,7 @@ connmgr_run(struct connmgr *mgr, } } + struct ofconn *ofconn, *next_ofconn; LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) { ofconn_run(ofconn, handle_openflow); } @@ -372,19 +365,16 @@ connmgr_run(struct connmgr *mgr, fail_open_run(mgr->fail_open); } + struct ofservice *ofservice; HMAP_FOR_EACH (ofservice, node, &mgr->services) { struct vconn *vconn; - int retval; - - retval = pvconn_accept(ofservice->pvconn, &vconn); + int retval = pvconn_accept(ofservice->pvconn, &vconn); if (!retval) { - struct rconn *rconn; - char *name; - /* Passing default value for creation of the rconn */ - rconn = rconn_create(ofservice->probe_interval, 0, ofservice->dscp, - vconn_get_allowed_versions(vconn)); - name = ofconn_make_name(mgr, vconn_get_name(vconn)); + struct rconn *rconn = rconn_create( + ofservice->probe_interval, 0, ofservice->dscp, + vconn_get_allowed_versions(vconn)); + char *name = ofconn_make_name(mgr, vconn_get_name(vconn)); rconn_connect_unreliably(rconn, vconn, name); free(name); @@ -400,11 +390,9 @@ connmgr_run(struct connmgr *mgr, } } - for (i = 0; i < mgr->n_snoops; i++) { + for (size_t i = 0; i < mgr->n_snoops; i++) { struct vconn *vconn; - int retval; - - retval = pvconn_accept(mgr->snoops[i], &vconn); + int retval = pvconn_accept(mgr->snoops[i], &vconn); if (!retval) { add_snooper(mgr, vconn); } else if (retval != EAGAIN) { @@ -417,24 +405,27 @@ connmgr_run(struct connmgr *mgr, void connmgr_wait(struct connmgr *mgr) { - struct ofservice *ofservice; struct ofconn *ofconn; - size_t i; - LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { ofconn_wait(ofconn); } + ofmonitor_wait(mgr); + if (mgr->in_band) { in_band_wait(mgr->in_band); } + if (mgr->fail_open) { fail_open_wait(mgr->fail_open); } + + struct ofservice *ofservice; HMAP_FOR_EACH (ofservice, node, &mgr->services) { pvconn_wait(ofservice->pvconn); } - for (i = 0; i < mgr->n_snoops; i++) { + + for (size_t i = 0; i < mgr->n_snoops; i++) { pvconn_wait(mgr->snoops[i]); } } @@ -444,17 +435,15 @@ connmgr_wait(struct connmgr *mgr) void connmgr_get_memory_usage(const struct connmgr *mgr, struct simap *usage) { - const struct ofconn *ofconn; unsigned int packets = 0; unsigned int ofconns = 0; + const struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { - int i; - ofconns++; packets += rconn_count_txqlen(ofconn->rconn); - for (i = 0; i < N_SCHEDULERS; i++) { + for (int i = 0; i < N_SCHEDULERS; i++) { struct pinsched_stats stats; pinsched_get_stats(ofconn->schedulers[i], &stats); @@ -597,10 +586,6 @@ connmgr_set_controllers(struct connmgr *mgr, OVS_EXCLUDED(ofproto_mutex) { bool had_controllers = connmgr_has_controllers(mgr); - struct shash new_controllers; - struct ofconn *ofconn, *next_ofconn; - struct ofservice *ofservice, *next_ofservice; - size_t i; /* Required to add and remove ofconns. This could probably be narrowed to * cover a smaller amount of code, if that yielded some benefit. */ @@ -608,13 +593,13 @@ connmgr_set_controllers(struct connmgr *mgr, /* Create newly configured controllers and services. * Create a name to ofproto_controller mapping in 'new_controllers'. */ - shash_init(&new_controllers); - for (i = 0; i < n_controllers; i++) { + struct shash new_controllers = SHASH_INITIALIZER(&new_controllers); + for (size_t i = 0; i < n_controllers; i++) { const struct ofproto_controller *c = &controllers[i]; if (!vconn_verify_name(c->target)) { bool add = false; - ofconn = find_controller_by_target(mgr, c->target); + struct ofconn *ofconn = find_controller_by_target(mgr, c->target); if (!ofconn) { VLOG_INFO("%s: added primary controller \"%s\"", mgr->name, c->target); @@ -631,7 +616,7 @@ connmgr_set_controllers(struct connmgr *mgr, } } else if (!pvconn_verify_name(c->target)) { bool add = false; - ofservice = ofservice_lookup(mgr, c->target); + struct ofservice *ofservice = ofservice_lookup(mgr, c->target); if (!ofservice) { VLOG_INFO("%s: added service controller \"%s\"", mgr->name, c->target); @@ -656,11 +641,11 @@ connmgr_set_controllers(struct connmgr *mgr, /* Delete controllers that are no longer configured. * Update configuration of all now-existing controllers. */ + struct ofconn *ofconn, *next_ofconn; HMAP_FOR_EACH_SAFE (ofconn, next_ofconn, hmap_node, &mgr->controllers) { const char *target = ofconn_get_target(ofconn); - struct ofproto_controller *c; - - c = shash_find_data(&new_controllers, target); + struct ofproto_controller *c = shash_find_data(&new_controllers, + target); if (!c) { VLOG_INFO("%s: removed primary controller \"%s\"", mgr->name, target); @@ -672,11 +657,11 @@ connmgr_set_controllers(struct connmgr *mgr, /* Delete services that are no longer configured. * Update configuration of all now-existing services. */ + struct ofservice *ofservice, *next_ofservice; HMAP_FOR_EACH_SAFE (ofservice, next_ofservice, node, &mgr->services) { const char *target = pvconn_get_name(ofservice->pvconn); - struct ofproto_controller *c; - - c = shash_find_data(&new_controllers, target); + struct ofproto_controller *c = shash_find_data(&new_controllers, + target); if (!c) { VLOG_INFO("%s: removed service controller \"%s\"", mgr->name, target); @@ -723,9 +708,7 @@ connmgr_set_snoops(struct connmgr *mgr, const struct sset *snoops) void connmgr_get_snoops(const struct connmgr *mgr, struct sset *snoops) { - size_t i; - - for (i = 0; i < mgr->n_snoops; i++) { + for (size_t i = 0; i < mgr->n_snoops; i++) { sset_add(snoops, pvconn_get_name(mgr->snoops[i])); } } @@ -745,10 +728,9 @@ add_controller(struct connmgr *mgr, const char *target, uint8_t dscp, OVS_REQUIRES(ofproto_mutex) { char *name = ofconn_make_name(mgr, target); - struct ofconn *ofconn; - - ofconn = ofconn_create(mgr, rconn_create(5, 8, dscp, allowed_versions), - OFCONN_PRIMARY, true); + struct ofconn *ofconn = ofconn_create(mgr, rconn_create(5, 8, dscp, + allowed_versions), + OFCONN_PRIMARY, true); rconn_connect(ofconn->rconn, target, name); hmap_insert(&mgr->controllers, &ofconn->hmap_node, hash_string(target, 0)); @@ -772,17 +754,13 @@ find_controller_by_target(struct connmgr *mgr, const char *target) static void update_in_band_remotes(struct connmgr *mgr) { - struct sockaddr_in *addrs; - size_t max_addrs, n_addrs; - struct ofconn *ofconn; - size_t i; - /* Allocate enough memory for as many remotes as we could possibly have. */ - max_addrs = mgr->n_extra_remotes + hmap_count(&mgr->controllers); - addrs = xmalloc(max_addrs * sizeof *addrs); - n_addrs = 0; + size_t max_addrs = mgr->n_extra_remotes + hmap_count(&mgr->controllers); + struct sockaddr_in *addrs = xmalloc(max_addrs * sizeof *addrs); + size_t n_addrs = 0; /* Add all the remotes. */ + struct ofconn *ofconn; HMAP_FOR_EACH (ofconn, hmap_node, &mgr->controllers) { const char *target = rconn_get_target(ofconn->rconn); union { @@ -796,7 +774,7 @@ update_in_band_remotes(struct connmgr *mgr) addrs[n_addrs++] = sa.in; } } - for (i = 0; i < mgr->n_extra_remotes; i++) { + for (size_t i = 0; i < mgr->n_extra_remotes; i++) { addrs[n_addrs++] = mgr->extra_in_band_remotes[i]; } @@ -839,25 +817,26 @@ static int set_pvconns(struct pvconn ***pvconnsp, size_t *n_pvconnsp, const struct sset *sset) { - struct pvconn **pvconns = *pvconnsp; - size_t n_pvconns = *n_pvconnsp; - const char *name; - int retval = 0; - size_t i; - - for (i = 0; i < n_pvconns; i++) { - pvconn_close(pvconns[i]); + /* Free the old pvconns. */ + struct pvconn **old_pvconns = *pvconnsp; + size_t old_n_pvconns = *n_pvconnsp; + for (size_t i = 0; i < old_n_pvconns; i++) { + pvconn_close(old_pvconns[i]); } - free(pvconns); + free(old_pvconns); + + /* Populate the new pvconns. */ + struct pvconn **new_pvconns = xmalloc(sset_count(sset) + * sizeof *new_pvconns); + size_t new_n_pvconns = 0; - pvconns = xmalloc(sset_count(sset) * sizeof *pvconns); - n_pvconns = 0; + int retval = 0; + const char *name; SSET_FOR_EACH (name, sset) { struct pvconn *pvconn; - int error; - error = pvconn_open(name, 0, 0, &pvconn); + int error = pvconn_open(name, 0, 0, &pvconn); if (!error) { - pvconns[n_pvconns++] = pvconn; + new_pvconns[new_n_pvconns++] = pvconn; } else { VLOG_ERR("failed to listen on %s: %s", name, ovs_strerror(error)); if (!retval) { @@ -866,8 +845,8 @@ set_pvconns(struct pvconn ***pvconnsp, size_t *n_pvconnsp, } } - *pvconnsp = pvconns; - *n_pvconnsp = n_pvconns; + *pvconnsp = new_pvconns; + *n_pvconnsp = new_n_pvconns; return retval; } @@ -897,10 +876,9 @@ snoop_preference(const struct ofconn *ofconn) static void add_snooper(struct connmgr *mgr, struct vconn *vconn) { - struct ofconn *ofconn, *best; - /* Pick a controller for monitoring. */ - best = NULL; + struct ofconn *best = NULL; + struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { if (ofconn->type == OFCONN_PRIMARY && (!best || snoop_preference(ofconn) > snoop_preference(best))) { @@ -969,13 +947,12 @@ void ofconn_send_role_status(struct ofconn *ofconn, uint32_t role, uint8_t reason) { struct ofputil_role_status status; - struct ofpbuf *buf; - status.reason = reason; status.role = role; ofconn_get_master_election_id(ofconn, &status.generation_id); - buf = ofputil_encode_role_status(&status, ofconn_get_protocol(ofconn)); + struct ofpbuf *buf + = ofputil_encode_role_status(&status, ofconn_get_protocol(ofconn)); if (buf) { ofconn_send(ofconn, buf, NULL); } @@ -992,7 +969,8 @@ ofconn_set_role(struct ofconn *ofconn, enum ofp12_controller_role role) LIST_FOR_EACH (other, node, &ofconn->connmgr->all_conns) { if (other->role == OFPCR12_ROLE_MASTER) { other->role = OFPCR12_ROLE_SLAVE; - ofconn_send_role_status(other, OFPCR12_ROLE_SLAVE, OFPCRR_MASTER_REQUEST); + ofconn_send_role_status(other, OFPCR12_ROLE_SLAVE, + OFPCRR_MASTER_REQUEST); } } } @@ -1159,19 +1137,15 @@ ofconn_send_error(const struct ofconn *ofconn, const struct ofp_header *request, enum ofperr error) { static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(10, 10); - struct ofpbuf *reply; - - reply = ofperr_encode_reply(error, request); + struct ofpbuf *reply = ofperr_encode_reply(error, request); if (!VLOG_DROP_INFO(&err_rl)) { - const char *type_name; - size_t request_len; - enum ofpraw raw; + size_t request_len = ntohs(request->length); - request_len = ntohs(request->length); - type_name = (!ofpraw_decode_partial(&raw, request, - MIN(64, request_len)) - ? ofpraw_get_name(raw) - : "invalid"); + enum ofpraw raw; + const char *type_name = (!ofpraw_decode_partial(&raw, request, + MIN(64, request_len)) + ? ofpraw_get_name(raw) + : "invalid"); VLOG_INFO("%s: sending %s error reply to %s message", rconn_get_name(ofconn->rconn), ofperr_to_string(error), @@ -1186,8 +1160,6 @@ void ofconn_report_flow_mod(struct ofconn *ofconn, enum ofp_flow_mod_command command) { - long long int now; - switch (command) { case OFPFC_ADD: ofconn->n_add++; @@ -1204,7 +1176,7 @@ ofconn_report_flow_mod(struct ofconn *ofconn, break; } - now = time_msec(); + long long int now = time_msec(); if (ofconn->next_op_report == LLONG_MAX) { ofconn->first_op = now; ofconn->next_op_report = MAX(now + 10 * 1000, ofconn->op_backoff); @@ -1260,9 +1232,9 @@ bundle_remove_all(struct ofconn *ofconn) static void bundle_remove_expired(struct ofconn *ofconn, long long int now) { - struct ofp_bundle *b, *next; long long int limit = now - bundle_idle_timeout; + struct ofp_bundle *b, *next; HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) { if (b->used <= limit) { ofconn_send_error(ofconn, b->msg, OFPERR_OFPBFC_TIMEOUT); @@ -1284,9 +1256,7 @@ ofconn_create(struct connmgr *mgr, struct rconn *rconn, enum ofconn_type type, bool enable_async_msgs) OVS_REQUIRES(ofproto_mutex) { - struct ofconn *ofconn; - - ofconn = xzalloc(sizeof *ofconn); + struct ofconn *ofconn = xzalloc(sizeof *ofconn); ofconn->connmgr = mgr; ovs_list_push_back(&mgr->all_conns, &ofconn->node); ofconn->rconn = rconn; @@ -1310,9 +1280,6 @@ static void ofconn_flush(struct ofconn *ofconn) OVS_REQUIRES(ofproto_mutex) { - struct ofmonitor *monitor, *next_monitor; - int i; - ofconn_log_flow_mods(ofconn); ofconn->role = OFPCR12_ROLE_EQUAL; @@ -1321,7 +1288,7 @@ ofconn_flush(struct ofconn *ofconn) rconn_packet_counter_destroy(ofconn->packet_in_counter); ofconn->packet_in_counter = rconn_packet_counter_create(); - for (i = 0; i < N_SCHEDULERS; i++) { + for (int i = 0; i < N_SCHEDULERS; i++) { if (ofconn->schedulers[i]) { int rate, burst; @@ -1346,6 +1313,7 @@ ofconn_flush(struct ofconn *ofconn) ofconn->next_op_report = LLONG_MAX; ofconn->op_backoff = LLONG_MIN; + struct ofmonitor *monitor, *next_monitor; HMAP_FOR_EACH_SAFE (monitor, next_monitor, ofconn_node, &ofconn->monitors) { ofmonitor_destroy(monitor); @@ -1387,14 +1355,12 @@ ofconn_destroy(struct ofconn *ofconn) static void ofconn_reconfigure(struct ofconn *ofconn, const struct ofproto_controller *c) { - int probe_interval; - ofconn->band = c->band; ofconn->enable_async_msgs = c->enable_async_msgs; rconn_set_max_backoff(ofconn->rconn, c->max_backoff); - probe_interval = c->probe_interval ? MAX(c->probe_interval, 5) : 0; + int probe_interval = c->probe_interval ? MAX(c->probe_interval, 5) : 0; rconn_set_probe_interval(ofconn->rconn, probe_interval); ofconn_set_rate_limit(ofconn, c->rate_limit, c->burst_limit); @@ -1421,9 +1387,8 @@ ofconn_run(struct ofconn *ofconn, const struct ofpbuf *ofp_msg)) { struct connmgr *mgr = ofconn->connmgr; - size_t i; - for (i = 0; i < N_SCHEDULERS; i++) { + for (size_t i = 0; i < N_SCHEDULERS; i++) { struct ovs_list txq; pinsched_run(ofconn->schedulers[i], &txq); @@ -1433,7 +1398,7 @@ ofconn_run(struct ofconn *ofconn, rconn_run(ofconn->rconn); /* Limit the number of iterations to avoid starving other tasks. */ - for (i = 0; i < 50 && ofconn_may_recv(ofconn); i++) { + for (int i = 0; i < 50 && ofconn_may_recv(ofconn); i++) { struct ofpbuf *of_msg = rconn_recv(ofconn->rconn); if (!of_msg) { break; @@ -1470,9 +1435,7 @@ ofconn_run(struct ofconn *ofconn, static void ofconn_wait(struct ofconn *ofconn) { - int i; - - for (i = 0; i < N_SCHEDULERS; i++) { + for (int i = 0; i < N_SCHEDULERS; i++) { pinsched_wait(ofconn->schedulers[i]); } rconn_run_wait(ofconn->rconn); @@ -1585,9 +1548,7 @@ ofconn_make_name(const struct connmgr *mgr, const char *target) static void ofconn_set_rate_limit(struct ofconn *ofconn, int rate, int burst) { - int i; - - for (i = 0; i < N_SCHEDULERS; i++) { + for (int i = 0; i < N_SCHEDULERS; i++) { struct pinsched **s = &ofconn->schedulers[i]; if (rate > 0) { @@ -1719,14 +1680,13 @@ connmgr_send_flow_removed(struct connmgr *mgr, LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { if (ofconn_receives_async_msg(ofconn, OAM_FLOW_REMOVED, fr->reason)) { - struct ofpbuf *msg; - /* Account flow expirations as 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_get_protocol(ofconn)); + struct ofpbuf *msg = ofputil_encode_flow_removed( + fr, ofconn_get_protocol(ofconn)); ofconn_send_reply(ofconn, msg); } } @@ -1744,12 +1704,12 @@ connmgr_send_table_status(struct connmgr *mgr, const struct ofputil_table_desc *td, uint8_t reason) { - struct ofputil_table_status ts; - struct ofconn *ofconn; - - ts.reason = reason; - ts.desc = *td; + struct ofputil_table_status ts = { + .reason = reason, + .desc = *td + }; + struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { if (ofconn_receives_async_msg(ofconn, OAM_TABLE_STATUS, reason)) { struct ofpbuf *msg; @@ -1841,10 +1801,9 @@ connmgr_set_fail_mode(struct connmgr *mgr, enum ofproto_fail_mode fail_mode) int connmgr_get_max_probe_interval(const struct connmgr *mgr) { - const struct ofconn *ofconn; - int max_probe_interval; + int max_probe_interval = 0; - max_probe_interval = 0; + const struct ofconn *ofconn; HMAP_FOR_EACH (ofconn, hmap_node, &mgr->controllers) { int probe_interval = rconn_get_probe_interval(ofconn->rconn); max_probe_interval = MAX(max_probe_interval, probe_interval); @@ -1857,14 +1816,13 @@ connmgr_get_max_probe_interval(const struct connmgr *mgr) int connmgr_failure_duration(const struct connmgr *mgr) { - const struct ofconn *ofconn; - int min_failure_duration; - if (!connmgr_has_controllers(mgr)) { return 0; } - min_failure_duration = INT_MAX; + int min_failure_duration = INT_MAX; + + const struct ofconn *ofconn; HMAP_FOR_EACH (ofconn, hmap_node, &mgr->controllers) { int failure_duration = rconn_failure_duration(ofconn->rconn); min_failure_duration = MIN(min_failure_duration, failure_duration); @@ -1942,13 +1900,11 @@ static bool any_extras_changed(const struct connmgr *mgr, const struct sockaddr_in *extras, size_t n) { - size_t i; - if (n != mgr->n_extra_remotes) { return true; } - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) { const struct sockaddr_in *old = &mgr->extra_in_band_remotes[i]; const struct sockaddr_in *new = &extras[i]; @@ -2029,16 +1985,13 @@ static int ofservice_create(struct connmgr *mgr, const char *target, uint32_t allowed_versions, uint8_t dscp) { - struct ofservice *ofservice; struct pvconn *pvconn; - int error; - - error = pvconn_open(target, allowed_versions, dscp, &pvconn); + int error = pvconn_open(target, allowed_versions, dscp, &pvconn); if (error) { return error; } - ofservice = xzalloc(sizeof *ofservice); + struct ofservice *ofservice = xzalloc(sizeof *ofservice); hmap_insert(&mgr->services, &ofservice->node, hash_string(target, 0)); ofservice->pvconn = pvconn; ofservice->allowed_versions = allowed_versions; @@ -2111,11 +2064,9 @@ ofmonitor_create(const struct ofputil_flow_monitor_request *request, struct ofconn *ofconn, struct ofmonitor **monitorp) OVS_REQUIRES(ofproto_mutex) { - struct ofmonitor *m; - *monitorp = NULL; - m = ofmonitor_lookup(ofconn, request->id); + struct ofmonitor *m = ofmonitor_lookup(ofconn, request->id); if (m) { return OFPERR_OFPMOFC_MONITOR_EXISTS; } @@ -2167,13 +2118,11 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, const struct rule_actions *old_actions) OVS_REQUIRES(ofproto_mutex) { - enum nx_flow_monitor_flags update; - struct ofconn *ofconn; - if (rule_is_hidden(rule)) { return; } + enum nx_flow_monitor_flags update; switch (event) { case NXFME_ADDED: update = NXFMF_ADD; @@ -2194,10 +2143,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, OVS_NOT_REACHED(); } + struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { - enum nx_flow_monitor_flags flags = 0; - struct ofmonitor *m; - if (ofconn->monitor_paused) { /* Only send NXFME_DELETED notifications for flows that were added * before we paused. */ @@ -2207,6 +2154,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, } } + enum nx_flow_monitor_flags flags = 0; + struct ofmonitor *m; HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) { if (m->flags & update && (m->table_id == 0xff || m->table_id == rule->table_id) @@ -2243,7 +2192,8 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule, ovs_mutex_unlock(&rule->mutex); if (flags & NXFMF_ACTIONS) { - const struct rule_actions *actions = rule_get_actions(rule); + const struct rule_actions *actions + = rule_get_actions(rule); fu.ofpacts = actions->ofpacts; fu.ofpacts_len = actions->ofpacts_len; } else { @@ -2282,12 +2232,10 @@ ofmonitor_flush(struct connmgr *mgr) if (!ofconn->monitor_paused && rconn_packet_counter_n_bytes(counter) > 128 * 1024) { - struct ofpbuf *pause; - COVERAGE_INC(ofmonitor_pause); ofconn->monitor_paused = monitor_seqno++; - pause = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_PAUSED, - OFP10_VERSION, htonl(0), 0); + struct ofpbuf *pause = ofpraw_alloc_xid( + OFPRAW_NXT_FLOW_MONITOR_PAUSED, OFP10_VERSION, htonl(0), 0); ofconn_send(ofconn, pause, counter); } } @@ -2298,20 +2246,18 @@ ofmonitor_resume(struct ofconn *ofconn) OVS_REQUIRES(ofproto_mutex) { struct rule_collection rules; - struct ofpbuf *resumed; - struct ofmonitor *m; - struct ovs_list msgs; - rule_collection_init(&rules); + + struct ofmonitor *m; HMAP_FOR_EACH (m, ofconn_node, &ofconn->monitors) { ofmonitor_collect_resume_rules(m, ofconn->monitor_paused, &rules); } - ovs_list_init(&msgs); + struct ovs_list msgs = OVS_LIST_INITIALIZER(&msgs); ofmonitor_compose_refresh_updates(&rules, &msgs); - resumed = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_RESUMED, OFP10_VERSION, - htonl(0), 0); + struct ofpbuf *resumed = ofpraw_alloc_xid(OFPRAW_NXT_FLOW_MONITOR_RESUMED, + OFP10_VERSION, htonl(0), 0); ovs_list_push_back(&msgs, &resumed->list_node); ofconn_send_replies(ofconn, &msgs); @@ -2329,9 +2275,8 @@ ofmonitor_may_resume(const struct ofconn *ofconn) static void ofmonitor_run(struct connmgr *mgr) { - struct ofconn *ofconn; - ovs_mutex_lock(&ofproto_mutex); + struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { if (ofmonitor_may_resume(ofconn)) { COVERAGE_INC(ofmonitor_resume); @@ -2344,9 +2289,8 @@ ofmonitor_run(struct connmgr *mgr) static void ofmonitor_wait(struct connmgr *mgr) { - struct ofconn *ofconn; - ovs_mutex_lock(&ofproto_mutex); + struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { if (ofmonitor_may_resume(ofconn)) { poll_immediate_wake();
This moves declarations closer to first use and merges them with initialization when possible, moves "for" loop variable declarations into the "for" statements where possible, and otherwise makes this code look like it was written a little more recently than it was. Signed-off-by: Ben Pfaff <blp@ovn.org> --- ofproto/connmgr.c | 280 ++++++++++++++++++++++-------------------------------- 1 file changed, 112 insertions(+), 168 deletions(-)