[ovs-dev] [PATCH branch-2.3] ofproto: Prevent deleting flows from hidden tables.

[Ben Pfaff]

Thanks for the additional review.  I had already applied to this to
branch-2.3 on the basis of testing by Vijaya (though now I realize
that I forgot to mention that in this thread, sorry about that).

On Wed, Mar 18, 2015 at 04:22:44PM -0700, Jarno Rajahalme wrote:
> LGTM,
> 
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> > On Mar 13, 2015, at 12:54 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> > Trying to delete a hidden flow should return an "EPERM" error, but the
> > code here allowed it instead.
> > 
> > Reported-by: Vijaya Mohan Guvva <vguvva at caviumnetworks.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > This bug is not present on master, so this patch is for branch-2.3 only.
> > 
> > AUTHORS           |    1 +
> > ofproto/ofproto.c |   24 ++++++++++++++++++------
> > 2 files changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/AUTHORS b/AUTHORS
> > index 8418058..8b95d80 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -278,6 +278,7 @@ Torbjorn Tornkvist      kruskakli at gmail.com
> > Valentin Bud            valentin at hackaserver.com
> > Vasiliy Tolstov         v.tolstov at selfip.ru
> > Vasu Dasari             vdasari at gmail.com
> > +Vijaya Mohan Guvva      vguvva at caviumnetworks.com
> > Vishal Swarankar        vishal.swarnkar at gmail.com
> > Vjekoslav Brajkovic     balkan at cs.washington.edu
> > Voravit T.              voravit at kth.se
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 2048fde..377351e 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> > + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> >  * Copyright (c) 2010 Jean Tourrilhes - HP-Labs.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> > @@ -4295,21 +4295,30 @@ delete_flow__(struct rule *rule, struct ofopgroup *group,
> >  * Returns 0 on success, otherwise an OpenFlow error code. */
> > static enum ofperr
> > delete_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
> > +               enum ofputil_flow_mod_flags flags,
> >                const struct ofp_header *request,
> >                const struct rule_collection *rules,
> >                enum ofp_flow_removed_reason reason)
> >     OVS_REQUIRES(ofproto_mutex)
> > {
> >     struct ofopgroup *group;
> > +    enum ofperr error;
> >     size_t i;
> > 
> > +    error = OFPERR_OFPBRC_EPERM;
> >     group = ofopgroup_create(ofproto, ofconn, request, UINT32_MAX);
> >     for (i = 0; i < rules->n; i++) {
> > -        delete_flow__(rules->rules[i], group, reason);
> > +        struct rule *rule = rules->rules[i];
> > +
> > +        if (rule_is_modifiable(rule, flags)) {
> > +            /* At least one rule is modifiable, don't report EPERM error. */
> > +            error = 0;
> > +            delete_flow__(rule, group, reason);
> > +        }
> >     }
> >     ofopgroup_submit(group);
> > 
> > -    return 0;
> > +    return error;
> > }
> > 
> > /* Implements OFPFC_DELETE. */
> > @@ -4330,7 +4339,8 @@ delete_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
> >     rule_criteria_destroy(&criteria);
> > 
> >     if (!error && rules.n > 0) {
> > -        error = delete_flows__(ofproto, ofconn, request, &rules, OFPRR_DELETE);
> > +        error = delete_flows__(ofproto, ofconn, fm->flags, request,
> > +                               &rules, OFPRR_DELETE);
> >     }
> >     rule_collection_destroy(&rules);
> > 
> > @@ -4355,7 +4365,8 @@ delete_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
> >     rule_criteria_destroy(&criteria);
> > 
> >     if (!error && rules.n > 0) {
> > -        error = delete_flows__(ofproto, ofconn, request, &rules, OFPRR_DELETE);
> > +        error = delete_flows__(ofproto, ofconn, fm->flags, request,
> > +                               &rules, OFPRR_DELETE);
> >     }
> >     rule_collection_destroy(&rules);
> > 
> > @@ -5145,7 +5156,8 @@ handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh,
> >         }
> >     }
> >     if (rules.n > 0) {
> > -        delete_flows__(ofproto, ofconn, oh, &rules, OFPRR_METER_DELETE);
> > +        delete_flows__(ofproto, ofconn, OFPUTIL_FF_NO_READONLY,
> > +                       oh, &rules, OFPRR_METER_DELETE);
> >     }
> > 
> >     /* Delete the meters. */
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>