From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment
Date: Fri, 29 Jan 2016 17:13:06 +0800 [thread overview]
Message-ID: <20160129091306.GW4257@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <20160129085723.GV4257@yliu-dev.sh.intel.com>
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.
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 */
next prev parent reply other threads:[~2016-01-29 9:12 UTC|newest]
Thread overview: 120+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-18 9:13 [dpdk-dev] [PATCH 0/3] virtio: Add a new layer to abstract pci access method Tetsuya Mukawa
2016-01-18 9:13 ` [dpdk-dev] [PATCH 1/3] virtio: Change the parameter order of io_write8/16/32() Tetsuya Mukawa
2016-01-21 11:07 ` [dpdk-dev] [RFC PATCH 0/5] virtio: Add a new layer to abstract pci access method Tetsuya Mukawa
2016-01-21 11:10 ` Tetsuya Mukawa
2016-01-21 11:07 ` [dpdk-dev] [RFC PATCH 1/5] virtio: Change the parameter order of io_write8/16/32() Tetsuya Mukawa
2016-01-21 11:07 ` [dpdk-dev] [RFC PATCH 2/5] virtio: move rte_eal_pci_unmap_device() to virtio_pci.c Tetsuya Mukawa
2016-01-21 11:07 ` [dpdk-dev] [RFC PATCH 3/5] virtio: Add a new layer to abstract pci access method Tetsuya Mukawa
2016-01-22 7:26 ` Xie, Huawei
2016-01-22 7:35 ` Tetsuya Mukawa
2016-01-21 11:07 ` [dpdk-dev] [RFC PATCH 4/5] EAL: Add new EAL "--shm" option Tetsuya Mukawa
2016-01-22 1:43 ` Tan, Jianfeng
2016-01-22 2:07 ` Tan, Jianfeng
2016-01-22 3:23 ` Tetsuya Mukawa
2016-01-21 11:07 ` [dpdk-dev] [RFC PATCH 5/5] virtio: Extend virtio-net PMD to support container environment Tetsuya Mukawa
2016-01-22 8:14 ` Xie, Huawei
2016-01-22 10:37 ` Tetsuya Mukawa
2016-01-25 10:15 ` Xie, Huawei
2016-01-26 2:58 ` Tetsuya Mukawa
2016-01-27 9:39 ` Xie, Huawei
2016-01-28 2:33 ` Tetsuya Mukawa
2016-01-25 10:17 ` Xie, Huawei
2016-01-26 2:58 ` Tetsuya Mukawa
2016-01-25 10:29 ` Xie, Huawei
2016-01-26 2:58 ` Tetsuya Mukawa
2016-01-27 10:03 ` Xie, Huawei
2016-01-28 2:44 ` Tetsuya Mukawa
2016-01-29 8:56 ` Xie, Huawei
2016-01-27 15:58 ` Xie, Huawei
2016-01-28 2:47 ` Tetsuya Mukawa
2016-01-28 9:48 ` Xie, Huawei
2016-01-28 9:53 ` Tetsuya Mukawa
2016-01-27 16:45 ` Xie, Huawei
2016-01-28 2:47 ` Tetsuya Mukawa
2016-01-28 6:15 ` Xie, Huawei
2016-01-28 6:29 ` Tetsuya Mukawa
2016-01-29 8:57 ` Yuanhan Liu
2016-01-29 9:13 ` Yuanhan Liu [this message]
2016-02-01 1:49 ` Tetsuya Mukawa
2016-02-10 3:40 ` [dpdk-dev] [PATCH v2 0/5] Virtio-net PMD: QEMU QTest extension for container Tetsuya Mukawa
2016-02-10 3:40 ` [dpdk-dev] [PATCH v2 1/5] virtio: Retrieve driver name from eth_dev Tetsuya Mukawa
2016-02-10 3:40 ` [dpdk-dev] [PATCH v2 2/5] EAL: Add new EAL "--qtest-virtio" option Tetsuya Mukawa
2016-02-15 7:52 ` Tan, Jianfeng
2016-02-16 1:32 ` Tetsuya Mukawa
2016-02-16 5:53 ` David Marchand
2016-02-16 11:36 ` Tan, Jianfeng
2016-02-17 3:36 ` Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 0/6] Virtio-net PMD: QEMU QTest extension for container Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 1/6] virtio: Retrieve driver name from eth_dev Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 2/6] vhost: Add a function to check virtio device type Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 3/6] EAL: Add new EAL "--range-virtaddr" option Tetsuya Mukawa
2016-03-04 2:20 ` Tan, Jianfeng
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 00/12] Virtio-net PMD: QEMU QTest extension for container Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 01/12] virtio: Retrieve driver name from eth_dev Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 02/12] vhost: Add a function to check virtio device type Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 03/12] EAL: Add a new "--range-virtaddr" option Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 04/12] EAL: Add a new "--align-memsize" option Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 05/12] virtio, qtest: Add QTest utility basic functions Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 06/12] virtio, qtest: Add pci device initialization function to qtest utils Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 07/12] virtio, qtest: Add functionality to share memory between QTest guest Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 08/12] virtio, qtest: Add functionality to handle interrupt Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 09/12] virtio, qtest: Add misc functions to handle pci information Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 10/12] virtio: Add QTest support to vtpci abstraction Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 11/12] virtio: Add QTest support for virtio-net PMD Tetsuya Mukawa
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 0/6] Virtio-net PMD: QEMU QTest extension for container Tetsuya Mukawa
2016-06-02 7:31 ` Yuanhan Liu
2016-06-02 9:30 ` Tetsuya Mukawa
2016-06-03 4:17 ` Yuanhan Liu
2016-06-03 13:51 ` Thomas Monjalon
2016-06-06 5:10 ` Tetsuya Mukawa
2016-06-06 7:21 ` Yuanhan Liu
2016-06-06 8:33 ` Tetsuya Mukawa
2016-06-06 8:49 ` Yuanhan Liu
2016-06-06 9:30 ` Tetsuya Mukawa
2016-06-06 9:58 ` Yuanhan Liu
2016-06-06 10:50 ` Tan, Jianfeng
2016-06-07 7:12 ` Tetsuya Mukawa
2016-06-07 7:33 ` Yuanhan Liu
2016-06-06 8:03 ` Tan, Jianfeng
2016-06-06 9:28 ` Tetsuya Mukawa
2016-06-06 10:35 ` Tan, Jianfeng
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 1/6] virtio, qtest: Add QTest utility basic functions Tetsuya Mukawa
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 2/6] virtio, qtest: Add pci device initialization function to qtest utils Tetsuya Mukawa
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 3/6] virtio, qtest: Add functionality to share memory between QTest guest Tetsuya Mukawa
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 4/6] virtio, qtest: Add misc functions to handle pci information Tetsuya Mukawa
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 5/6] virtio: Add QTest support to vtpci abstraction Tetsuya Mukawa
2016-06-02 3:29 ` [dpdk-dev] [PATCH v5 6/6] virtio: Add QTest support for virtio-net PMD Tetsuya Mukawa
2016-06-02 3:30 ` [dpdk-dev] [PATCH v1 0/2] Supplement patches for virtio-qtest to support LSC interrupt Tetsuya Mukawa
2016-06-02 3:30 ` [dpdk-dev] [PATCH v1 1/2] virtio: Handle interrupt things under vtpci abstraction Tetsuya Mukawa
2016-06-02 3:30 ` [dpdk-dev] [PATCH v1 2/2] virtio, qtest: Add functionality to handle interrupt Tetsuya Mukawa
2016-03-09 8:33 ` [dpdk-dev] [PATCH v4 12/12] docs: add release note for qtest virtio container support Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 4/6] EAL: Add a new "--align-memsize" option Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 5/6] virtio: Add support for qtest virtio-net PMD Tetsuya Mukawa
2016-03-04 2:18 ` Tan, Jianfeng
2016-03-04 5:05 ` Tetsuya Mukawa
2016-03-04 6:10 ` Tan, Jianfeng
2016-03-04 9:53 ` Tetsuya Mukawa
2016-02-22 8:17 ` [dpdk-dev] [PATCH v3 6/6] docs: add release note for qtest virtio container support Tetsuya Mukawa
2016-02-22 15:40 ` Mcnamara, John
2016-02-23 10:28 ` Mcnamara, John
2016-02-24 1:20 ` Tetsuya Mukawa
2016-02-10 3:40 ` [dpdk-dev] [PATCH v2 3/5] vhost: Add a function to check virtio device type Tetsuya Mukawa
2016-02-10 3:40 ` [dpdk-dev] [PATCH v2 4/5] virtio: Add support for qtest virtio-net PMD Tetsuya Mukawa
2016-02-10 3:40 ` [dpdk-dev] [PATCH v2 5/5] docs: add release note for qtest virtio container support Tetsuya Mukawa
2016-01-28 9:33 ` [dpdk-dev] [PATCH v2 0/3] virtio: Add a new layer to abstract pci access method Tetsuya Mukawa
2016-01-28 9:33 ` [dpdk-dev] [PATCH v2 1/3] virtio: Change the parameter order of io_write8/16/32() Tetsuya Mukawa
2016-01-28 9:33 ` [dpdk-dev] [PATCH v2 2/3] virtio: move rte_eal_pci_unmap_device() to virtio_pci.c Tetsuya Mukawa
2016-01-28 9:33 ` [dpdk-dev] [PATCH v2 3/3] virtio: Add a new layer to abstract pci access method Tetsuya Mukawa
2016-01-29 9:17 ` Yuanhan Liu
2016-02-01 1:50 ` Tetsuya Mukawa
2016-02-01 13:15 ` Yuanhan Liu
2016-02-02 2:19 ` Tetsuya Mukawa
2016-02-02 2:45 ` Yuanhan Liu
2016-02-02 3:55 ` Tetsuya Mukawa
2016-01-18 9:13 ` [dpdk-dev] [PATCH 2/3] virtio: move rte_eal_pci_unmap_device() to virtio_pci.c Tetsuya Mukawa
2016-01-18 9:13 ` [dpdk-dev] [PATCH 3/3] virtio: Add a new layer to abstract pci access method Tetsuya Mukawa
2016-01-18 13:46 ` Yuanhan Liu
2016-01-19 1:22 ` Tetsuya Mukawa
2016-01-19 2:41 ` Xie, Huawei
2016-01-18 13:13 ` [dpdk-dev] [PATCH 0/3] " Tan, Jianfeng
2016-01-19 1:22 ` Tetsuya Mukawa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160129091306.GW4257@yliu-dev.sh.intel.com \
--to=yuanhan.liu@linux.intel.com \
--cc=dev@dpdk.org \
--cc=mukawa@igel.co.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).