[ovs-dev] [PATCH v3 10/41] ofproto: Handle monitor and delete commands in flow monitor requests

[Ben Pfaff]

On Mon, Jun 16, 2014 at 11:29:30AM +0900, Simon Horman wrote:
> Handle modify and delete commands in OpenFlow1.4 flow monitor requests.
> These commands are not yet allowed by the decoder which
> will be updated by a subsequent patch.
> 
> Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> ---
> v2
> * No change
> ---
>  ofproto/ofproto.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 3153e71..211f45e 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -4880,12 +4880,23 @@ handle_flow_monitor_request(struct ofconn *ofconn, const struct ofp_header *oh)
>              goto error;
>          }
>  
> -        error = ofmonitor_create(&request, ofconn, &m);
> -        if (error) {
> -            goto error;
> +        if (request.command == OFPFMC14_DELETE ||
> +            request.command == OFPFMC14_MODIFY) {
> +            error = flow_monitor_delete(ofconn, request.id);
> +            if (error) {
> +                goto error;
> +            }
>          }
>  
> -        list_insert(&monitor_list, &m->list_node);
> +        if (request.command == OFPFMC14_ADD ||
> +            request.command == OFPFMC14_MODIFY) {
> +            error = ofmonitor_create(&request, ofconn, &m);
> +            if (error) {
> +                goto error;
> +            }
> +
> +            list_insert(&monitor_list, &m->list_node);
> +        }
>      }
>  
>      rule_collection_init(&rules);

I think that this is correct, but this way it is written makes it look
like a "modify" could fail in the middle with the monitor deleted and
not re-created.  That can't actually happen because ofmonitor_create()
only fails if there's a duplicate id.  I'd rather have to code written
to be more obviously correct.  (One way to do that would be to have an
ofmonitor_modify() function, I guess, but there might be other ways
too.)