From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tokarev <mjt@tls.msk.ru>
Subject: Re: MTU on a virtio-net device?
Date: Thu, 23 Oct 2008 17:19:54 +0400
Message-ID: <490079FA.8070005@msgid.tls.msk.ru>
References: <49003700.2020905@msgid.tls.msk.ru> <4900698C.90905@redhat.com> <49006E53.1090106@msgid.tls.msk.ru> <490073EA.5060009@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org
To: dor@redhat.com
Return-path: <kvm-owner@vger.kernel.org>
Received: from hobbit.corpit.ru ([81.13.33.150]:23910 "EHLO hobbit.corpit.ru"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750951AbYJWNT5 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 23 Oct 2008 09:19:57 -0400
In-Reply-To: <490073EA.5060009@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Dor Laor wrote:
> Michael Tokarev wrote:
>> Dor Laor wrote:
>>   
>>> Michael Tokarev wrote:
>>>     
>>>> Right now (2.6.27), there's no way to change MTU of a
>>>> virtio-net interface, since the mtu-changing method is
>>>> not provided.  Is there a simple way to add such a
>>>> beast?
>>>>       
>>> It should be a nice easy patch for mtu < 4k.
>>> You can just implement a 'change_mtu' handler like:
[]
>> Well, this isn't enough I think.  That is, new_mtu's upper cap should be
>> less than PAGE_SIZE due to various additional data structures.  But it
>> is enough to start playing.
>>   
> The virtio header is in a separate ring entry so no prob.

virtio header is one thing.  Ethernet frame is another.  And
so on.  From the last experiment (sending 2000bytes-payload
pings resulting in 2008 bytes total, and 528 bytes missing
with original mtu=1500), it seems like the necessary upper
cap is PAGE_SIZE-28.  Or something similar.

Also see receive_skb() routine:

receive_skb(struct net_device *dev, struct sk_buff *skb, unsigned len)
{
  if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
    /*drop*/
  }
  len -= sizeof(struct virtio_net_hdr);
  if (len <= MAX_PACKET_LEN) {
  ...

So it seems that virtio_net_hdr is in here, just like
ethernet header.

[]
>> So something else has to be changed for this to work, it seems.
> You're right, this was needs to be changed to:
> /* FIXME: MTU in config. */
> #define MAX_PACKET_LEN (ETH_HLEN+ETH_DATA_LEN)
> 
> You can change it to PAGE_SIZE or have the current mtu.

so s/MAX_PACKET_LEN/dev->mtu/g for the whole driver, it
seems.  Plus/minus sizeof(virtio_net_hdr) - checking this now.
This constant is used in 3 places:

receive_skb(): if (len <= MAX_PACKET_LEN) {
 (this one seems to be wrong, but again I don't know much
  internals of all this stuff)
 here, dev->mtu is what we want.

try_fill_recv(): skb = netdev_alloc_skb(vi->dev, MAX_PACKET_LEN);
 here, we don't have dev, but have vi->dev, should be ok too.
try_fill_recv(): skb_put(skb, MAX_PACKET_LEN);
 ditto

And by the way, what is "big_packets" here?

Ok, so I changed MAX_PACKET_LEN to be PAGE_SIZE (current MTU
seems to be more appropriate but PAGE_SIZE is enough for
testing anyway).  It seems to be working, and network
speed increased significantly with MTU=3500 compared with
former 1500 - it seems it's about 2 times faster (which is
quite expectable, since there's 2x less context switches,
transmissions and the like).

>>>> I'm asking because I'm not familiar with the internals,

Still... ;)

Thanks!

/mjt