[libvirt] problem with outbound limiting

[Stefan Berger]

On 08/10/2011 11:20 AM, Eric Blake wrote:
> On 08/10/2011 09:04 AM, Stefan Berger wrote:
>> On 08/10/2011 02:55 AM, Upendra Moturi wrote:
>>
>> Hi,
>>
>> does this patch solve your problem? I am setting the MTU to fixed 2kb.
>> Doing tests with scp seems to indicate that this improved the situation
>> -- at least for me.
>>
>> Stefan
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>>
>> ---
>> src/util/network.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> Index: libvirt-acl/src/util/network.c
>> ===================================================================
>> --- libvirt-acl.orig/src/util/network.c
>> +++ libvirt-acl/src/util/network.c
>> @@ -1156,8 +1156,8 @@ virBandwidthEnable(virBandwidthPtr bandw
>>
>> virCommandFree(cmd);
>> cmd = virCommandNew(TC);
>> - virCommandAddArgList(cmd,"class", "add", "dev", iface, "parent",
>> - "1:", "classid", "1:1", "htb", NULL);
>> + virCommandAddArgList(cmd,"class", "add", "dev", iface, "parent",
>> + "1:", "classid", "1:1", "htb", NULL);
>> virCommandAddArgList(cmd, "rate", average, NULL);
>
> This hunk was just indentation; you should push the whitespace changes 
> as a separate commit (and can push that now, under the trivial rule, 
> without needing further review).  [It doesn't help matters that 
> thunderbird botched the whitespace in my reply]
>
>> @@ -1202,7 +1202,7 @@ virBandwidthEnable(virBandwidthPtr bandw
>> virCommandAddArgList(cmd, "filter", "add", "dev", iface, "parent",
>> "ffff:", "protocol", "ip", "u32", "match", "ip",
>> "src", "0.0.0.0/0", "police", "rate", average,
>> - "burst", burst, "mtu", burst, "drop", "flowid",
>> + "burst", burst, "mtu", "2kb", "drop", "flowid",
>> ":1", NULL);
>
> Here's the meat of the proposed patch.  Is 2kb always valid, or will 
> we run into problems on NICs that insist on traditional limits near 
> the 1518 mark?  Should it be user-configurable?  I'm reluctant to ACK 
> this without a bit more understanding of why 2kb works, as well as 
> feedback on test results with the patch in place.
>
Yes, I am not an exper on tc. I have seen examples with the mtu being 
set to '1' for dropping icmp traffic for example. So if a VM produces 
smaller than 2kb packets, which on traditional networks it probably 
should, then the packets should go through. We could also leave out the 
mtu parameter above and it would go to 2kb by default. Check via

tc filter show dev <ifname> root

The 2kb parameter doesn't explain why 2MB doesn't work, though.
Even if this wasn't the final fix, it at least 'improves the situation' 
(for me).

     Stefan