From patchwork Thu Apr 2 16:06:25 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 15955 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n32G6svZ032185 for ; Thu, 2 Apr 2009 16:06:54 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1763059AbZDBQGe (ORCPT ); Thu, 2 Apr 2009 12:06:34 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1758457AbZDBQGd (ORCPT ); Thu, 2 Apr 2009 12:06:33 -0400 Received: from g1t0026.austin.hp.com ([15.216.28.33]:29460 "EHLO g1t0026.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1762110AbZDBQGc (ORCPT ); Thu, 2 Apr 2009 12:06:32 -0400 Received: from g5t0030.atlanta.hp.com (g5t0030.atlanta.hp.com [16.228.8.142]) by g1t0026.austin.hp.com (Postfix) with ESMTP id A4F53C456; Thu, 2 Apr 2009 16:06:30 +0000 (UTC) Received: from ldl.fc.hp.com (ldl.fc.hp.com [15.11.146.30]) by g5t0030.atlanta.hp.com (Postfix) with ESMTP id 64FFE24078; Thu, 2 Apr 2009 16:06:30 +0000 (UTC) Received: from localhost (ldl.fc.hp.com [127.0.0.1]) by ldl.fc.hp.com (Postfix) with ESMTP id DCC4339C043; Thu, 2 Apr 2009 10:06:29 -0600 (MDT) X-Virus-Scanned: Debian amavisd-new at ldl.fc.hp.com Received: from ldl.fc.hp.com ([127.0.0.1]) by localhost (ldl.fc.hp.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4kz91h7qPx3T; Thu, 2 Apr 2009 10:06:27 -0600 (MDT) Received: from [192.168.1.60] (g3t0027.americas.hpqcorp.net [16.232.34.131]) by ldl.fc.hp.com (Postfix) with ESMTP id C064939C017; Thu, 2 Apr 2009 10:06:26 -0600 (MDT) Subject: Re: virtio_net: MAC address releated breakage if there is no MAC area in config From: Alex Williamson To: Christian Borntraeger Cc: Rusty Russell , netdev@vger.kernel.org, kvm@vger.kernel.org In-Reply-To: <200904021333.25119.borntraeger@de.ibm.com> References: <200904021333.25119.borntraeger@de.ibm.com> Organization: HP OSLO R&D Date: Thu, 02 Apr 2009 10:06:25 -0600 Message-Id: <1238688385.6124.255.camel@bling> Mime-Version: 1.0 X-Mailer: Evolution 2.24.3 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Thu, 2009-04-02 at 13:33 +0200, Christian Borntraeger wrote: > I read this as the mac config field is optional (similar to all the optional > fields we added in virtio_blk later). > > I see two options: > 1. Change our sample userspace to always allocate the config (like lguest and > qemu) > 2. Change the kernel code to not write into the config unless a specific feature > bit is set. (e.g. VIRTIO_NET_F_SETMAC) > > > Opinions? Hi Christian, Sorry for the breakage. My interpretation of the virtio-net config space was that the mac field was always present and the host had programmed a valid value when the F_MAC feature is available. However, from the history of the flag, it seems like you're interpretation is likely correct. Setting the config value from the randomly generated mac was largely opportunistic since there's no userspace that doesn't provide a mac by default. So perhaps we can drop that and gate the set_mac_address entry point as shown below. How does this look? Thanks, Alex virtio_net: Set the mac config only when VIRITO_NET_F_MAC VIRTIO_NET_F_MAC indicates the presence of the mac field in config space, not the validity of the value it contains. Allow the mac to be changed at runtime, but only push the change into config space with the VIRTIO_NET_F_MAC feature present. Signed-off-by: Alex Williamson Acked-by: Christian Borntraeger --- -- To unsubscribe from this list: send the line "unsubscribe kvm" 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 index a6f1e19..9c82a39 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -575,8 +575,9 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p) if (ret) return ret; - vdev->config->set(vdev, offsetof(struct virtio_net_config, mac), - dev->dev_addr, dev->addr_len); + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) + vdev->config->set(vdev, offsetof(struct virtio_net_config, mac), + dev->dev_addr, dev->addr_len); return 0; } @@ -876,11 +877,8 @@ static int virtnet_probe(struct virtio_device *vdev) vdev->config->get(vdev, offsetof(struct virtio_net_config, mac), dev->dev_addr, dev->addr_len); - } else { + } else random_ether_addr(dev->dev_addr); - vdev->config->set(vdev, offsetof(struct virtio_net_config, mac), - dev->dev_addr, dev->addr_len); - } /* Set up our device-specific information */ vi = netdev_priv(dev);