[ovs-dev] [nxm 24/42] ofproto: Simplify send_flow_removed().

[Justin Pettit]

The commit looks fine.  The comment within this function has an unnecessary "it" in the first sentence.  The comment is a bit confusing because it doesn't actually describe any functionality within the function, but I guess I don't have a better suggestion where to put it.

--Justin


On Oct 28, 2010, at 10:27 AM, Ben Pfaff wrote:

> I have no evidence that the optimization in this function is valuable.
> An upcoming commit will introduce a new form of flow expiration message
> that is sent to controllers that ask for it, while the standard OpenFlow
> 1.0 message is sent to other controllers.  Since retaining this
> optimization with that logic would complicate the function, this commit
> drops it.
> ---
> ofproto/ofproto.c |   22 ++++++++--------------
> 1 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 8422786..b231cc9 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -4545,8 +4545,6 @@ send_flow_removed(struct ofproto *p, struct rule *rule,
>                   long long int now, uint8_t reason)
> {
>     struct ofconn *ofconn;
> -    struct ofconn *prev;
> -    struct ofpbuf *buf = NULL;
> 
>     if (!rule->send_flow_removed) {
>         return;
> @@ -4558,20 +4556,16 @@ send_flow_removed(struct ofproto *p, struct rule *rule,
>      * being added (and expiring).  (It also prevents processing OpenFlow
>      * requests that would not add new flows, so it is imperfect.) */
> 
> -    prev = NULL;
>     LIST_FOR_EACH (ofconn, node, &p->all_conns) {
> -        if (rconn_is_connected(ofconn->rconn)
> -            && ofconn_receives_async_msgs(ofconn)) {
> -            if (prev) {
> -                queue_tx(ofpbuf_clone(buf), prev, prev->reply_counter);
> -            } else {
> -                buf = compose_flow_removed(p, rule, now, reason);
> -            }
> -            prev = ofconn;
> +        struct ofpbuf *msg;
> +
> +        if (!rconn_is_connected(ofconn->rconn)
> +            || !ofconn_receives_async_msgs(ofconn)) {
> +            continue;
>         }
> -    }
> -    if (prev) {
> -        queue_tx(buf, prev, prev->reply_counter);
> +
> +        msg = compose_flow_removed(p, rule, now, reason);
> +        queue_tx(msg, ofconn, ofconn->reply_counter);
>     }
> }
> 
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org