On 11/28/2017 02:02 PM, Jiri Pirko wrote:
> Mon, Nov 27, 2017 at 09:32:59PM CET, step...@networkplumber.org wrote:
>> On Mon, 27 Nov 2017 19:00:14 +0100
>> Michal Privoznik <mpriv...@redhat.com> wrote:
>>
>>> parse_action_control helper does advancing of the arg inside. So don't
>>> do it outside.
>>>
>>> Fixes: e67aba559581 ("tc: actions: add helpers to parse and print control
>>> actions")
>>> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
>>
>> The helpers are not helping here.
>> Adding another layer of indirection on moving argc/argv then causing caller
>> to have to keep track is bad design.
>>
>> Also since pars_action_control_slash is only used by police, why was it
>> moved into tc_util in the first place? I would prefer just to rip out that
>> bit and put it back in policer.
>
> I tried to make all the x-specific parsing to go away and make all done
> in core. That should have been done from the very beginning, we would
> lot of mess.
>
Okay, would it be a better solution if __parse_action_control() wouldn't
call NEXT_ARG_FWD() at the end? Then this patch wouldn't be needed and
every place that calls parse_action_control() would not need to special
case it.
Just a bit of background:
Libvirt has a capability of setting QoS on bridges/TAPs it manages. And
as part of that it issues the following command to set traffic limiting:
tc filter add dev virbr0 parent ffff: protocol all u32 match u32 0 0 \
police rate 50kbps burst 50kb mtu 64kb drop flowid :1
More info can be found in this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1514963
Fortunately, no a lot of distros ship 4.13+ so libvirt ain't broken yet.
But soon.
Michal
