From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id E198B1E2B for ; Thu, 13 Apr 2017 04:39:18 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Apr 2017 19:39:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,192,1488873600"; d="scan'208";a="1134737869" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga001.fm.intel.com with ESMTP; 12 Apr 2017 19:39:16 -0700 Date: Thu, 13 Apr 2017 10:36:23 +0800 From: Yuanhan Liu To: "Tan, Jianfeng" Cc: dev@dpdk.org, Maxime Coquelin Message-ID: <20170413023623.GH7333@yliu-dev.sh.intel.com> References: <1491551860-15005-1-git-send-email-jianfeng.tan@intel.com> <20170413015812.GE7333@yliu-dev.sh.intel.com> <5e7b5876-0f98-9c42-965e-e91b318cd17f@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5e7b5876-0f98-9c42-965e-e91b318cd17f@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] net/virtio-user: fix cannot get initialized X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Apr 2017 02:39:19 -0000 On Thu, Apr 13, 2017 at 10:18:43AM +0800, Tan, Jianfeng wrote: > > > On 4/13/2017 9:58 AM, Yuanhan Liu wrote: > >On Fri, Apr 07, 2017 at 07:57:40AM +0000, Jianfeng Tan wrote: > >>After the introduction of vhost MTU, VIRTIO_NET_F_MTU is enabled > >>by default. Actually, that's my point. The vhost lib just claim he supports MTU feature and MTU is not enabled by the virtio-user device. Meaning, the MTU feature bit should not be set after the negoitation. You were seeing the bug because you found the bit is set, right? If so, that means the virtio-user feature negotiation is broken. --yliu > However, virtio-user vtpci does not support to get > >>MTU from device yet, i.e., vtpci_read_dev_config(MTU) fails. > >>Plus, struct virtio_net_config is defined as a uninitialized > >>variable, and could be different values in > >>virtio_negotiate_features() and virtio_init_device(). > >> > >>In some cases, it passes the check in virtio_negotiate_features() > >>but fails the check in virtio_init_device(). As a result, > >>virtio-user canno be initialized. > >> > >>To fix it, (1) accessing uninitialized variable is not a good > >>practice, so initialize it as zero; (2) explicitly disable MTU > >>feature in virtio-user. > >> > >>Fixes: 49d26d9e3f47 ("net/virtio: support MTU feature") > >>Cc: Maxime Coquelin > >>Cc: Yuanhan Liu > >> > >>Signed-off-by: Jianfeng Tan > >>--- > >> drivers/net/virtio/virtio_ethdev.c | 4 ++-- > >> drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ++++++ > >> 2 files changed, 8 insertions(+), 2 deletions(-) > >> > >>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > >>index 78cb3e8..4c43784 100644 > >>--- a/drivers/net/virtio/virtio_ethdev.c > >>+++ b/drivers/net/virtio/virtio_ethdev.c > >>@@ -1163,7 +1163,7 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features) > >> /* If supported, ensure MTU value is valid before acknowledging it. */ > >> if (host_features & req_features & (1ULL << VIRTIO_NET_F_MTU)) { > >>- struct virtio_net_config config; > >>+ struct virtio_net_config config = {0}; > >virtio-user does not support the MTU feature, this patch should not be > >reached. The virtio-user feature negotiation should be broken. > > > > --yliu > > > > Yes, it will not come to here anyway. But some static code analyzer might > report this as an error: there's chance to read uninitialized variable. > > Thanks, > Jianfeng