DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@6wind.com>
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH 2/2] virtio: change io privilege level as early as possible
Date: Mon,  9 Mar 2015 15:56:39 +0100
Message-ID: <1425912999-13118-3-git-send-email-david.marchand@6wind.com> (raw)
In-Reply-To: <1425912999-13118-1-git-send-email-david.marchand@6wind.com>

Playing with virtio link status triggers a segfault because of an incorrect io
privilege level.

To reproduce the problem, virtio device must be bound to igb_uio to have lsc
enabled.

$ lspci |grep Ethernet
00:03.0 Ethernet controller: Red Hat, Inc Virtio network device
$ modprobe uio
$ insmod ./x86_64-native-linuxapp-gcc/kmod/igb_uio.ko
$ echo 0000:00:03.0 > /sys/bus/pci/devices/0000\:00\:03.0/driver/unbind
$ echo 1af4 1000 > /sys/bus/pci/drivers/igb_uio/new_id
$ ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 3 -w 0000:00:03.0 -- -i --txqflags=0xf01
[snip]
EAL: PCI device 0000:00:03.0 on NUMA socket -1
EAL:   probe driver: 1af4:1000 rte_virtio_pmd
Interactive-mode selected
Configuring Port 0 (socket 0)
Port 0: DE:AD:DE:01:02:03
Checking link statuses...
Port 0 Link Up - speed 10000 Mbps - full-duplex
Done
testpmd>

Then, from qemu monitor:
(qemu) set_link virtio-net-pci.0 off

testpmd> Segmentation fault

A call to rte_eal_iopl_init() in a specific constructor has been added so that,
on Linux, iopl() is called asap:
- for statically linked virtio pmd, the constructor will be called at binary
  start,
- for shared virtio pmd (loaded using -d eal option), it will be called at
  dlopen() in rte_eal_init().

On linux, iopl() effects are inherited by children threads, so calling it in a
constructor will ensure that any thread / process created after rte_eal_init()
inherits the io privilege level needed by the pmd.

We keep call to rte_eal_iopl_init() in the pmd init macro so that the pmd will
not register if the io privilege level is insufficient.

Besides, I moved rte_virtio_pmd_init() and related structure near its calling
site, so that all init code can be looked at in a glance.
The only change here is a new comment.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_virtio/virtio_ethdev.c |   71 ++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index d239083..29332fa 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1228,34 +1228,6 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	return 0;
 }
 
-static struct eth_driver rte_virtio_pmd = {
-	{
-		.name = "rte_virtio_pmd",
-		.id_table = pci_id_virtio_map,
-	},
-	.eth_dev_init = eth_virtio_dev_init,
-	.dev_private_size = sizeof(struct virtio_hw),
-};
-
-/*
- * Driver initialization routine.
- * Invoked once at EAL init time.
- * Register itself as the [Poll Mode] Driver of PCI virtio devices.
- * Returns 0 on success.
- */
-static int
-rte_virtio_pmd_init(const char *name __rte_unused,
-		    const char *param __rte_unused)
-{
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return -1;
-	}
-
-	rte_eth_driver_register(&rte_virtio_pmd);
-	return 0;
-}
-
 /*
  * Only 1 queue is supported, no queue release related operation
  */
@@ -1484,6 +1456,49 @@ __rte_unused uint8_t is_rx)
 	return 0;
 }
 
+/*
+ * Early init function, this is needed so that iopl() on linux is done before
+ * rte_eal_init (since it creates other threads that must inherit from the one
+ * that calls rte_eal_init).
+ */
+static void __attribute__((constructor, used))
+rte_virtio_early_init(void)
+{
+	/* return value is checked at pmd init time, no need to check here, see
+	 * below. */
+	rte_eal_iopl_init();
+}
+
+static struct eth_driver rte_virtio_pmd = {
+	{
+		.name = "rte_virtio_pmd",
+		.id_table = pci_id_virtio_map,
+	},
+	.eth_dev_init = eth_virtio_dev_init,
+	.dev_private_size = sizeof(struct virtio_hw),
+};
+
+/*
+ * Driver initialization routine.
+ * Invoked once at EAL init time.
+ * Register itself as the [Poll Mode] Driver of PCI virtio devices.
+ * Returns 0 on success.
+ */
+static int
+rte_virtio_pmd_init(const char *name __rte_unused,
+		    const char *param __rte_unused)
+{
+	/* rte_eal_iopl_init() is already called by rte_virtio_early_init().
+	 * Call it again here, to be sure we did get the io level we wanted. */
+	if (rte_eal_iopl_init() != 0) {
+		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
+		return -1;
+	}
+
+	rte_eth_driver_register(&rte_virtio_pmd);
+	return 0;
+}
+
 static struct rte_driver rte_virtio_driver = {
 	.type = PMD_PDEV,
 	.init = rte_virtio_pmd_init,
-- 
1.7.10.4

  parent reply	other threads:[~2015-03-09 14:56 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-06  0:45 [dpdk-dev] [PATCH 0/2] virtio: bugfixes Stephen Hemminger
2015-03-06  0:45 ` [dpdk-dev] [PATCH 1/2] virtio: initialize iopl when device is initialized Stephen Hemminger
2015-03-06  3:41   ` Ouyang, Changchun
2015-03-06 16:20     ` Stephen Hemminger
2015-03-06 16:33       ` David Marchand
2015-03-06 16:55         ` Stephen Hemminger
2015-03-06 22:04           ` David Marchand
2015-03-06 23:43             ` Stephen Hemminger
2015-03-07  6:53               ` David Marchand
2015-03-09 11:05       ` David Marchand
2015-03-09 14:56         ` [dpdk-dev] [PATCH 0/2] fix virtio interrupt handling David Marchand
2015-03-09 14:56           ` [dpdk-dev] [PATCH 1/2] eal/linux: move plugin load to very start of eal init David Marchand
2015-03-09 15:21             ` Neil Horman
     [not found]               ` <CALwxeUs4hPbYDPBUfz9u2AoiCoj_wwTsAyj=_1xxzuT6LLW6nw@mail.gmail.com>
2015-03-10 10:55                 ` Neil Horman
2015-10-14  0:05                   ` Stephen Hemminger
2015-10-14  9:55                     ` David Marchand
2015-03-09 14:56           ` David Marchand [this message]
2015-03-10 13:14             ` [dpdk-dev] [PATCH 2/2] virtio: change io privilege level as early as possible Neil Horman
2015-09-29 19:25               ` Stephen Hemminger
2015-09-30  8:28                 ` David Marchand
2015-09-30 14:52                   ` Neil Horman
2015-09-30 15:37                     ` Thomas Monjalon
2015-09-30 17:26                       ` Stephen Hemminger
2015-10-01 11:25                       ` Neil Horman
2015-10-12 20:08                         ` Stephen Hemminger
2015-10-14  0:07                         ` Stephen Hemminger
2015-10-14  8:00                           ` David Marchand
2015-10-14  9:49                             ` David Marchand
2015-10-14  9:50                               ` [dpdk-dev] [PATCH] eal: move interrupt init after device init David Marchand
2015-10-14 11:32                                 ` David Marchand
2015-10-20 21:22                                   ` Thomas Monjalon
2015-07-29 17:26       ` [dpdk-dev] [PATCH 1/2] virtio: initialize iopl when device is initialized Thomas Monjalon
2015-03-06  0:45 ` [dpdk-dev] [PATCH 2/2] virtio: allow running w/o vlan filtering Stephen Hemminger
2015-03-06  3:39   ` Ouyang, Changchun
2015-03-06 16:24     ` Stephen Hemminger
2015-07-29 12:56       ` Thomas Monjalon
2015-07-30  1:23         ` Ouyang, Changchun
2015-08-04 12:51         ` Vincent JARDIN
2015-08-05  1:01           ` Ouyang, Changchun
2015-08-05  1:22             ` Stephen Hemminger
2015-08-05 10:49             ` Vincent JARDIN
2015-10-21 13:58               ` Thomas Monjalon
2017-02-15  8:38                 ` Thomas Monjalon

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=1425912999-13118-3-git-send-email-david.marchand@6wind.com \
    --to=david.marchand@6wind.com \
    --cc=dev@dpdk.org \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git