[ovs-dev] [ACL Meters 6/7] ofproto: Add support for specifying a meter in controller actions.

Justin Pettit jpettit at ovn.org
Tue Jul 31 00:55:12 UTC 2018


> On Jul 30, 2018, at 12:49 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Sun, Jul 29, 2018 at 11:46:37PM -0700, Justin Pettit wrote:
>> Signed-off-by: Justin Pettit <jpettit at ovn.org>
> 
> In ofproto_check_ofpacts(), the other checks, if they fail, return an
> error and prevent the flow from being added.  The new one doesn't; is
> the difference intentional?

Yes.  My thought is that those controller actions are likely important, so if the meter doesn't exist, it's still safer to set up the flow and log a warning.

> Similarly, is there anything that prevents
> a meter from being deleted while still in use and, if that happens, does
> anything particularly bad happen?

Nothing prevents it.  However, if it happens, then the metering just stops being enforced, and the other actions are unaffected.

> In ovs-ofctl.8.in, the wording seems a little vague because "associate"
> is such a weak word.  Maybe change
>    Associate packets sent to the controller with meter \fIid\fR.
> to
>    Use meter \fIid\fR to rate-limit the OpenFlow packet-in messages
>    that this action sends to the controller.
> or something similarly descriptive

Yes, that is better.

> Do only "drop" actions make sense for controller metering?

Probably.  I added some documentation to that effect to the description of the "meter" argument to the log() action in ovn-sb.xml.

> Please add some tests for the new action to the test "ovn -- action
> parsing" in ovn.at.
> 
> Please document the new action in ovn-sb.xml.

Which new action?  I've extended the log() action documentation to add "meter", which was missing before.  There was no existing log action parsing test.  I'll add that as a separate patch, since I think the existing log() parsing code could use a little cleanup in that area, too.

Thanks!

--Justin




More information about the dev mailing list