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

[Thomas Karlsson <thomas.karlsson@paneda.se>]

Den 2020-11-23 kl. 23:30, skrev Jakub Kicinski:
> On Mon, 23 Nov 2020 14:22:31 +0000 Thomas Karlsson wrote:
>> Hello,
>>
>> There is a special queue handling in macvlan.c for broadcast and
>> multicast packages that was arbitrarily set to 1000 in commit
>> 07d92d5cc977a7fe1e683e1d4a6f723f7f2778cb . While this is probably
>> sufficient for most uses cases it is insufficient to support high
>> packet rates. I currently have a setup with 144 000 multicast packets
>> incoming per second (144 different live audio RTP streams) and suffer
>> very frequent packet loss. With unicast this is not an issue and I
>> can in addition to the 144kpps load the macvlan interface with
>> another 450mbit/s using iperf.
>>
>> In order to verify that the queue is the problem I edited the define
>> to 100000 and recompiled the kernel module. After replacing it with
>> rmmod/insmod I get 0 packet loss (measured over 2 days where I before
>> had losses every other second or so) and can also load an additional
>> 450 mbit/s multicast traffic using iperf without losses. So basically
>> no change in performance between unicast/multicast when it comes to
>> lost packets on my machine.
>>
>> I think It would be best if this queue length was configurable
>> somehow. Either an option when creating the macvlan (like how
>> bridge/passthrough/etc are set) or at least when loading the module
>> (for instance by using a config in /etc/modprobe.d). One size does
>> not fit all in this situation.
> 
> The former please. You can add a netlink attribute, should be
> reasonably straightforward. The other macvlan attrs are defined
> under "MACVLAN section" in if_link.h.
> 

I did some work towards a patch using the first option,
by adding a netlink attribute in if_link.h as suggested.
I agree that this was reasonably straightforward, until userspace.

In order to use/test my new parameter I need to update iproute2 package
as far as I understand. But then since I use the macvlan with docker
I also need to update the docker macvlan driver to send this new
option to the kernel module.

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.