[ovs-dev] [PATCH ovs V2 20/21] netdev-linux: always add ingress qdisc

[Paul Blakey]

On 05/01/2017 23:43, Joe Stringer wrote:
> On 25 December 2016 at 03:39, Paul Blakey <paulb at mellanox.com> wrote:
>> flow offloading by tc needs ingress qdisc on the device.
>> Deleting the ingress qdisc was done in order to flush
>> policing filters, so instead we just flush the filter and
>> leave the ingress added (and add it if there wasn't any).
>>
>> Signed-off-by: Paul Blakey <paulb at mellanox.com>
>> Reviewed-by: Roi Dayan <roid at mellanox.com>
> I'm concerned that this patch will break a bunch of things.
>
> I don't think we generally want an ingress qdisc unless there is
> specific policing configuration, or hardware offloads is enabled. This
> looks like it sets it up regardless.
>
> I suspect that if you changed policing configuration, it will not take
> effect with this patch.
Right, policing will not work with offloads for now.
> Hardware offloads is orthogonal to policing; I suggest that there
> should be a new netdev-provider API to enable hardware offloads, which
> will ensure that the qdiscs are set up appropriately.
>
> If policing is configured with a rate/burst, I would expect this to
> take effect. If this policing is not possible to be offloaded to
> hardware, then I'd expect either OVS complains in the logs that
> hardware offloads are configured with policing, and describe how it
> works - either A), policing is ignored and flows are offloaded, or B)
> policing works, but flows are not offloaded.
Makes sense, thanks.