> On Jul 30, 2018, at 12:49 PM, Ben Pfaff <[email protected]> wrote: > > On Sun, Jul 29, 2018 at 11:46:37PM -0700, Justin Pettit wrote: >> Signed-off-by: Justin Pettit <[email protected]> > > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
