netdev - Re: Hardcoded multicast queue length in macvlan.c driver causes poor multicast receive performance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <956c4fca-2a54-97cb-5b4c-3a286743884b@paneda.se>
Date:   Wed, 25 Nov 2020 23:15:39 +0100
From:   Thomas Karlsson <thomas.karlsson@...eda.se>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        <thomas.karlsson@...eda.se>
Subject: Re: Hardcoded multicast queue length in macvlan.c driver causes poor
 multicast receive performance

On 2020-11-25 19:07, Jakub Kicinski wrote:
> On Wed, 25 Nov 2020 18:12:34 +0100 Thomas Karlsson wrote:
>>>> For this reason I would like to know if you would consider
>>>> merging a patch using the module_param(...) variant instead?
>>>>
>>>> I would argue that this still makes the situation better
>>>> and resolves the packet-loss issue, although not necessarily
>>>> in an optimal way. However, The upside of being able to specify the
>>>> parameter on a per macvlan interface level instead of globally is not
>>>> that big in this situation. Normally you don't use that much
>>>> multicast anyway so it's a parameter that only will be touched by
>>>> a very small user base that can understand and handle the implications
>>>> of such a global setting.  
>>>
>>> How about implementing .changelink in macvlan? That way you could
>>> modify the macvlan device independent of Docker? 
>>>
>>> Make sure you only accept changes to the bc queue len if that's the
>>> only one you act on.
>>>   
>>
>> Hmm, I see. You mean that docker can create the interface and then I can
>> modify it afterwards? That might be a workaround but I just submitted
>> a patch (like seconds before your message) with the module_param() option
>> and this was very clean I think. both in how little code that needed to be
>> changed and in how simple it is to set the option in the target environment.
>>
>> This is my first time ever attemting a contribution to the kernel so
>> I'm quite happy to keep it simple like that too :)
> 
> Module params are highly inflexible, we have a general policy not 
> to accept them in the netdev world.
> 

I see, although the current define seems even less flexible :)
Although, I might not have fully understood the .changelink you suggest.
Is it via the ip link set ... command? Or is there a way to set the parameters
in a more "raw" form that does not require a patch to iproute2 with parameter parsing,
error handing, man pages updates, etc. I feel that I'm getting in over my head here.

I appreciate your feedback!

> There should even be a check> in our patchwork which should fail here, but it appears that the patch 
> did not apply in the first place:
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/385b9b4c-25f5-b507-4e69-419883fa8043@paneda.se/
> 
> Make sure you're developing on top of this tree:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/
> 

Right, thanks! It's a bit of a learning curve. I had incorrectly done the work on top of torvalds/linux