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-419883fa8...@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

Reply via email to