From patchwork Fri Nov 28 00:22:33 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rusty Russell X-Patchwork-Id: 11272 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 6ABBEDDDE0 for ; Fri, 28 Nov 2008 11:22:47 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752904AbYK1AWm (ORCPT ); Thu, 27 Nov 2008 19:22:42 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752449AbYK1AWm (ORCPT ); Thu, 27 Nov 2008 19:22:42 -0500 Received: from ozlabs.org ([203.10.76.45]:47458 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751868AbYK1AWl (ORCPT ); Thu, 27 Nov 2008 19:22:41 -0500 Received: from vivaldi.localnet (bh02i525f01.au.ibm.com [202.81.18.30]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPSA id F0122DDD04; Fri, 28 Nov 2008 11:22:39 +1100 (EST) From: Rusty Russell To: Mark McLoughlin Subject: Re: [PATCH] virtio_net: large tx MTU support Date: Fri, 28 Nov 2008 10:52:33 +1030 User-Agent: KMail/1.10.1 (Linux/2.6.27-7-generic; KDE/4.1.2; i686; ; ) Cc: netdev , virtualization , Herbert Xu References: <1227707891.9579.44.camel@blaa> <200811272300.58151.rusty@rustcorp.com.au> <1227794225.24571.36.camel@blaa> In-Reply-To: <1227794225.24571.36.camel@blaa> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200811281052.33474.rusty@rustcorp.com.au> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Friday 28 November 2008 00:27:05 Mark McLoughlin wrote: > Hi Rusty, > > On Thu, 2008-11-27 at 23:00 +1030, Rusty Russell wrote: > > On Thursday 27 November 2008 00:28:11 Mark McLoughlin wrote: > > > We don't really have a max tx packet size limit, so allow configuring > > > the device with up to 64k tx MTU. > > > > Hi Mark, > > > > Just one comment: maybe we should be conservative and maybe limit to 1500 > > if the host doesn't offer any of the GSO or MRG_RXBUF features? > > That was actually what I was going to do until I thought about it a bit > more and discussed it with Herbert. > > The virtio_net MTU only affects the transmit path, so there shouldn't be > any issue with a host that doesn't support those features. Not quite what I meant. A minimal host can reasonably expect ethernet-fitting packets. If it supports GSO of course it must handle larger ones. Otherwise we should add YA feature bit or even a max-mtu field. So, I was thinking something like this over your patch (I also removed the used-once MAX and MIN definitions; I dislike gratuitous indirection): --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -480,26 +480,24 @@ static struct ethtool_ops virtnet_ethtoo .set_sg = ethtool_op_set_sg, }; -#define MIN_MTU 68 -#define MAX_MTU 65535 - static int virtnet_change_mtu(struct net_device *dev, int new_mtu) { struct virtnet_info *vi = netdev_priv(dev); int max_mtu; - /* Only allow a large MTU if we know we have a chance - * of also supporting that MTU on the receive side. */ - if (vi->mergeable_rx_bufs || vi->big_packets) - max_mtu = MAX_MTU; + /* A host which can handle GSO must handle large packets. */ + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GSO) + || virtio_has_feature(vi->vdev, VIRTIO_NET_F_HOST_TSO4) + || virtio_has_feature(vi->vdev, VIRTIO_NET_F_HOST_TSO6) + || virtio_has_feature(vi->vdev, VIRTIO_NET_F_HOST_UFO)) + max_mtu = 65535; else max_mtu = ETH_DATA_LEN; - if (new_mtu < MIN_MTU || new_mtu > max_mtu) + if (new_mtu < 68 || new_mtu > max_mtu) return -EINVAL; dev->mtu = new_mtu; - return 0; }