[ovs-dev] [RFC ovn 0/5] respin CoPP series

[Dumitru Ceara]

On 4/29/21 6:31 PM, Mark Gray wrote:
> On 28/04/2021 10:44, Dumitru Ceara wrote:
>> On 4/28/21 10:21 AM, Mark Gray wrote:
>>> On 24/04/2021 11:39, Lorenzo Bianconi wrote:
>>>> This series respin CoPP support introduced here [0] by Dumitru rebasing on top
>>>> of ovn master branch and adding some missing meters (e.g. bfd).
>>>> The main goal of this series is to continue the discussion about the proposed
>>>> approach and to align on CMS APIs.
>>>> For the moment no unit-tests or ddlog support have been added.
>>>
>>> System tests for this change will be important too.
>>
>> Agreed.
>>
>>>>
>>>> [0] https://patchwork.ozlabs.org/project/openvswitch/list/?series=140778&state=*
>>>
>>>
>>> Did a bug or an issue trigger this work? It would be good to understand
>>> the problem being solved.
>>
>> This was initially triggered by the discussion [1] related to the
>> initial patch for bug [2].
>>
>> [1]
>> https://patchwork.ozlabs.org/project/openvswitch/patch/1564392607-27395-1-git-send-email-dceara@redhat.com/
>> [2] https://bugzilla.redhat.com/show_bug.cgi?id=1729846
> 
> This should be added to the cover letter.
>>
>>>
>>> For example, one general question I have was why it was decided to rate
>>> limit the controller traffic rather than police it on the receive side?
>>
>> As far as I see there's a way to police ingress traffic on an OVS
>> interface although I never tested to see if it works for controller
>> ports.  However, the problem with that is that it won't discriminate
>> between different types of traffic, e.g., ARP vs DHCP, and just police
>> all traffic received on that interface.
> 
> Yeah and this wouldn't make much sense in software as many of the cpu
> cycles would be sent just doing the classification :/
>>
>> The way I see it, the goal of the series is two-fold:
>> a. to protect pinctrl from being flooded with packets in general.
>> b. to give some flexibility to sysadmins/CMS to prioritize some type of
>> traffic (e.g., ARP) over others (e.g., DHCP).
>>
>> The configuration is flexible enough to allow users to classify traffic
>> based on protocol and group it in multiple classes, each of them sharing
>> a single meter.  An example:
>> - ARP/ND packets get assigned to a policy with a meter that allows more
>> pps/higher bursts.
>> - DHCP and others get assigned to a "best effort" class with lower rates.
>>
>> But you're right, all this needs to be configured by the user and we
>> should probably improve the documentation part and/or add a tutorial
>> about how to use the feature in different scenarios.
>>
> It sounds like this problem would be better solved by using an egress
> scheduler so that you could configure the relative priorities of
> different traffic classes and a single rate limiter to pinctrl in order
> to protect it (in lieu of an ingress policer). OpenFlow does provide
> some level of support for that
> (https://opennetworking.org/wp-content/uploads/2014/10/openflow-switch-v1.5.0.pdf
> - Section 7.3.5.8) but it is "switch dependent". I don't think Open
> vSwitch supports this. I am also not sure if the CONTROLLER port is
> covered by this. Unfortunately, you couldn't leverage anything like HTB
> or HFSC on the host because this controller connection is via a unix
> socket.

I guess you mean "Q: How do I configure egress traffic shaping?", here:

https://github.com/openvswitch/ovs/blob/master/Documentation/faq/qos.rst#quality-of-service-qos

Although I have the impression that this is only supported for the
userspace datapath:

https://github.com/openvswitch/ovs/blob/master/vswitchd/vswitch.xml#L4644

> 
> I think tutorial documentation would help in order look at the use case
> and how we would configure this. Perhaps, we can come up with a data
> model for the north bound database that would also cover the potential
> inclusion of scheduling in Open vSwitch in the future?

It might be better indeed to come up with a different way of configuring
the feature in the north bound database in order to make the
implementation details opaque.  That is having ovn-northd automatically
generate the south bound meters if we go ahead with the implementation
suggested by this RFC.

> 
>>>
>>> Another question that I have is why apply the meters on a per-datapath
>>> and per-port basis? Or, why not just apply a meter to all traffic to the
>>> controller. I understand that this makes it more generic and flexible
>>> but it would be good to understand the motivation.
>>
>> The justification I had for this in the initial RFC was to be as generic
>> and flexible as possible.  While it might be a bit overkill to allow
>> per-port configuration, I still think per-datapath configuration is
>> useful.  Different logical switches might have different requirements,
>> e.g., a logical switch connecting OVN to the external network vs an
>> "internal" logical switch that only connects VMs/pods.
> 
> For goal a) above, a single rate-limiter per chassis would be sufficient
> but would not work for b).
>>
>> Regards,
>> Dumitru
>>
>