From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f176.google.com (mail-pf0-f176.google.com [209.85.192.176]) by dpdk.org (Postfix) with ESMTP id C4F3436E for ; Mon, 1 Feb 2016 02:49:29 +0100 (CET) Received: by mail-pf0-f176.google.com with SMTP id x125so74626805pfb.0 for ; Sun, 31 Jan 2016 17:49:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=igel-co-jp.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-type:content-transfer-encoding; bh=eUz5Ktm6mzhzBThtMA520sG/h7leMdy+oVzCuKVenNI=; b=UVyV9EaUnuZxwHjbcxcep80CE/4xxY4EYFygGZap0U73RRj6lmj/x5xODxCs+QX9rZ x+z6duka9kDgRXqIYqnYt59AidiGGHQd16UfPdk/Q7MeqIDhH9H6gsvuzKNqPVExW6TX m5Pa3DZXzhhLMyVFT1TxxfYDiHx+oIfQwLfj4Gr1aS2anohFRZ+2co6oYJFzY/qxUMwz bM3U5cCpb1uvj5/ArfxqznqcOvQoWTZlTWvaGEj9ST39hVZQvwRvLCa4sy5q+A3lQ9Hm yW8Vpup1ogid3w4l3/sztzi2ouG1LpkVAz56yZnsEDFrcIJ3HgGy7LgA1DTxlwVEjZ0W jS/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=eUz5Ktm6mzhzBThtMA520sG/h7leMdy+oVzCuKVenNI=; b=Spr1qZIYuYkKY44ljrvzs9X2QOSnmAekEowxcSReeoqArlQLbP1ZgwmUzadgwW+l10 u5Wm2pSFz836ogK0kBnEwIucoc99cjqyo8zRL4boDJZJ1PGrebXRuEZ4fACCS6I0qyWe eVPXT/qoF+KjADU/6QTlWXaWEd8TK89tD7N19aOonZEli7HfMl4l++N2hgW2WJSgbXiV vPmSUdfUsk3SaMoC0wOOYzxuls2Er0X5i7XBG2jfJcBe0UW3L3+TLYmSoJ+yaKggrski gfgIzkmzXFSqXcKfX6R2XTOaHWlGtnVT+pv6Ab6svNsgqzPRWkA+che1DyJh/RFxdKic ACxA== X-Gm-Message-State: AG10YOQJMAuY6X7MQNBv8ut2AY0IKJNSal1qZglLAVdJGwcMqWc2nneLr09e+49ND2hthA== X-Received: by 10.98.67.67 with SMTP id q64mr34170506pfa.133.1454291369226; Sun, 31 Jan 2016 17:49:29 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by smtp.googlemail.com with ESMTPSA id u69sm38578562pfa.61.2016.01.31.17.49.27 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 31 Jan 2016 17:49:28 -0800 (PST) To: Yuanhan Liu References: <1453108389-21006-2-git-send-email-mukawa@igel.co.jp> <1453374478-30996-6-git-send-email-mukawa@igel.co.jp> <20160129085723.GV4257@yliu-dev.sh.intel.com> <20160129091306.GW4257@yliu-dev.sh.intel.com> From: Tetsuya Mukawa X-Enigmail-Draft-Status: N1110 Message-ID: <56AEB9A6.7070803@igel.co.jp> Date: Mon, 1 Feb 2016 10:49:26 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160129091306.GW4257@yliu-dev.sh.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Feb 2016 01:49:30 -0000 On 2016/01/29 18:13, Yuanhan Liu wrote: > On Fri, Jan 29, 2016 at 04:57:23PM +0800, Yuanhan Liu wrote: >> On Thu, Jan 21, 2016 at 08:07:58PM +0900, Tetsuya Mukawa wrote: >>> +static int >>> +virt_read_pci_cfg(struct virtio_hw *hw, void *buf, size_t len, off_t offset) >>> +{ >>> + qtest_read_pci_cfg(hw, "virtio-net", buf, len, offset); >>> + return 0; >>> +} >>> + >>> +static void * >>> +virt_get_mapped_addr(struct virtio_hw *hw, uint8_t bar, >>> + uint32_t offset, uint32_t length) >>> +{ >>> + uint64_t base; >>> + uint64_t size; >>> + >>> + if (qtest_get_bar_size(hw, "virtio-net", bar, &size) < 0) { >>> + PMD_INIT_LOG(ERR, "invalid bar: %u", bar); >>> + return NULL; >>> + } >>> + >>> + if (offset + length < offset) { >>> + PMD_INIT_LOG(ERR, "offset(%u) + lenght(%u) overflows", >>> + offset, length); >>> + return NULL; >>> + } >>> + >>> + if (offset + length > size) { >>> + PMD_INIT_LOG(ERR, >>> + "invalid cap: overflows bar space: %u > %"PRIu64, >>> + offset + length, size); >>> + return NULL; >>> + } >>> + >>> + if (qtest_get_bar_addr(hw, "virtio-net", bar, &base) < 0) { >>> + PMD_INIT_LOG(ERR, "invalid bar: %u", bar); >>> + return NULL; >>> + } >> So, I understood the usage now, and the cfg_ops abstraction doesn't look >> good yet necessary to me. For EAL managed pci device, bar length and >> addr are stored at memory_resources[], and for your case, it's from the >> qtest. And judging that it's compile time decision, I'd like it to be: >> >> #ifdef /* RTE_LIBRTE_VIRTIO_HOST_MODE */ > Oops, sorry, I was wrong. Your code could be co-exist with the > traditional virtio pmd driver, thus we can't do that. > > But still, I think dynamic "if ... else ..." should be better: > there are just few places (maybe 4: bar_size, bar length, map > device, read config) need that. Thanks for comments. I will use "if ... else ...." instead of introducing a cfg_ops. Tetsuya > > On the other hand, if you really want to do that abstraction, > you should go it with more fine granularity, such as the following > methods I proposed, instead of the big one: get_cfg_addr(). In > that way, we could avoid duplicate code. > > --yliu > >> >> static uint32_t >> get_bar_size(...) >> { >> return qtest_get_bar_size(..); >> } >> >> static uint64-t >> get_bar_addr(...) >> { >> return qtest_get_bar_addr(..); >> } >> >> ... >> ... >> >> #else >> >> static uint32_t >> get_bar_size(...) >> { >> return dev->mem_resource[bar].addr; >> } >> >> ... >> >> } >> #endif >> >> >> And then you just need do related changes at virtio_read_caps() and >> get_cfg_addr(). That'd be much simpler, without introducing duplicate >> code and uncessary complex. >> >> What do you think of that? >> >> --yliu >> >>> + >>> + return (void *)(base + offset); >>> +} >>> + >>> +static const struct virtio_pci_cfg_ops virt_cfg_ops = { >>> + .map = virt_map_pci_cfg, >>> + .unmap = virt_unmap_pci_cfg, >>> + .get_mapped_addr = virt_get_mapped_addr, >>> + .read = virt_read_pci_cfg, >>> +}; >>> +#endif /* RTE_LIBRTE_VIRTIO_HOST_MODE */