[ovs-dev] [RFC ovn] Add CoPP (Control Plane Protection).
dceara at redhat.com
Fri Oct 18 08:27:24 UTC 2019
On Fri, Oct 18, 2019 at 3:53 AM Han Zhou <zhouhan at gmail.com> wrote:
> On Thu, Oct 10, 2019 at 1:20 AM Dumitru Ceara <dceara at redhat.com> wrote:
> > Add new 'Copp' (Control plane protection) table to OVN Northbound DB:
> > - this stores mappings between control plane protocol names and meters
> > that should be used to rate limit controller-destined traffic for
> > those protocols.
> > Add new 'copp' columns to the following OVN Northbound DB tables:
> > - Logical_Switch
> > - Logical_Switch_Port
> > - Logical_Router
> > - Logical_Router_Port
> > This allows defining control plane policies with different
> > granularities. For example a user can decide to enforce a general
> > policy for the logical switch but at the same time configure a
> > different policy on some of the ports of the logical switch.
> > Control plane protocol policies applied to a logical port take
> > precedence over the ones defined at logical switch level. For
> > logical routers and logical router ports we take the same approach.
> > Add a new 'controller_meter' column to OVN Southbound Logical_Flow
> > table. This stores an optional string which should correspond to
> > the Meter that must be used for rate limiting controller actions
> > generated by packets hitting the flow.
> > Add CLI commands in 'ovn-nbctl' to allow the user to manage Control
> > Plane Protection Policies at different levels (logical switch,
> > logical router, logical port).
> > Add a new 'ctrl_meter_id' field to 'struct ovn_flow' to be used for
> > applying meters to flows that trigger controller actions.
> > Add a new 'ofctrl_add_flow_meter' function to create a new 'ovn_flow'
> > with an attached controller meter.
> > Change ofctrl_check_and_add_flow to allow specifying a meter ID for
> > packets that are punted to controller.
> > Change consider_logical_flow to parse controller_meter from the logical
> > flow and use it when building openflow entries.
> > Add a new 'ctrl_meter_id' field to 'struct ovnact_encode_params' to be
> > used when encoding controller actions from logical flow actions.
> > Change the ovn-northd implementation to set the new 'controller_meter'
> > field for flows that need to punt packets to ovn-controller. For some
> > protocols (ARP/ND/DNS) install two sets of flows:
> > - one flow with a lower priority for the whole datapath using the
> > per-datapath CoPP policy.
> > - one flow per port with a higher priority than the per-datapath one if
> > there is a per-port CoPP defined for the given port.
> > Post-RFC remaining items:
> > - add autotests for CoPP
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362732.html
> > CC: Han Zhou <hzhou8 at ebay.com>
> > CC: Numan Siddique <nusiddiq at redhat.com>
> > Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> Thanks Dumitru for the RFC. I didn't review in too much detail but just some quick feedback.
Thanks for having a look.
> Overall, it is great that this approach addresses rate limit in a generic way.
> It is easy to understand the meters applied to router/switch because each lflow belongs to a datapath.
> However, could you describe more about how the meters for a logical_switch_port or logical_router_port is defined? Is it applied when the specific port is the ingress port, or egress port, or both?
One of the things the RFC misses is thorough documentation. I will add
that in the next version.
The meters (regardless if they're defined globally for the
switch/router or on a port) are applied only for traffic that hits a
flow with action=controller(..). So it's ingress traffic in the
logical topology. There might be cases when the packets injected by
controller will hit flows that send them back to a controller
(sometimes different). One example that comes to mind is a GARP
originated for a logical router port that reaches another logical
router port which might or might not be on the same controller. In
this case too, because the packet hits a flow with
action=controller(..), the metering will be performed.
> Regarding the attacking problem we have discussed before, when there is attack from a single src through a single router port, even if we enforce a dedicated meter on the router port level, e.g. for ARP resolve, then all ARP resolving through that router port would still be impacted. Did you consider flow based meters to lower the impact in that situation?
Agreed, in case of an attack on a metered port, all incoming ARP
traffic on that port will be metered potentially affecting legit
packets. It's a first step.
What I had in mind for the next step is to allow finer grain rate
limiting for protocols where this is applicable (e.g., ARP) by
splitting the openflow in two parts:
- first hash the packets
- then rate limit per hash bucket
So a flow like:
table=X, match="arp", action="controller(<encode_put_arp>)"
Would be changed to (assuming 16 hash buckets):
table=X, match="arp", action="multipath(symmetric_l3, modulo_n, 16,
0, NXM_NX_REGX), resubmit(X+1)"
table=X+1, match="arp, regX=0",
table=X+1, match="arp, regX=1",
table=X+1, match="arp, regX=15",
But this is quite a big change and I'd like to at least have some
generic simple rate limiting in place first.
> In addition, I suggest to split such big change to smaller incremental ones when you submit the formal patch, if possible. For example, rate limit for each protocol can be a separate patch, which would make the review easier and some patches can be merged before the whole change is completely merged.
Right, I kept adding stuff and had the impression that it's not such a
big change but it definitely requires splitting now. Will do.
More information about the dev