[dpdk-dev,4/4] virtio: check if any kernel driver is manipulating the device

Message ID 1450982292-129560-5-git-send-email-huawei.xie@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Huawei Xie Dec. 24, 2015, 6:38 p.m. UTC
  virtio PMD could use IO port to configure the virtio device without
using uio driver.

There are two issues with previous implementation:
1) virtio PMD will take over each virtio device blindly even if some
are not intended for DPDK.
2) driver conflict between virtio PMD and virtio-net kernel driver.

This patch checks if there is any kernel driver manipulating the virtio
device before virtio PMD uses IO port to configure the device.

Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")

Signed-off-by: Huawei Xie <huawei.xie@intel.com>
---
 drivers/net/virtio/virtio_ethdev.c | 7 +++++++
 1 file changed, 7 insertions(+)
  

Comments

Yuanhan Liu Dec. 28, 2015, 5:26 a.m. UTC | #1
On Fri, Dec 25, 2015 at 02:38:12AM +0800, Huawei Xie wrote:
> virtio PMD could use IO port to configure the virtio device without
> using uio driver.
> 
> There are two issues with previous implementation:
> 1) virtio PMD will take over each virtio device blindly even if some
> are not intended for DPDK.
> 2) driver conflict between virtio PMD and virtio-net kernel driver.
> 
> This patch checks if there is any kernel driver manipulating the virtio
> device before virtio PMD uses IO port to configure the device.
> 
> Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
> 
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 00015ef..504346a 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1138,6 +1138,13 @@ static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
>  	int found = 0;
>  	size_t linesz;
>  
> +	if (pci_dev->kdrv != RTE_KDRV_NONE) {
> +		PMD_INIT_LOG(ERR,
> +			"%s(): kernel driver is manipulating this device." \
> +			" Please unbind the kernel driver.", __func__);

PMD_INIT_LOG already prints the function, no need to reference __func__
again here.

	--yliu
> +		return -1;
> +	}
> +
>  	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
>  		 pci_dev->addr.domain,
>  		 pci_dev->addr.bus,
> -- 
> 1.8.1.4
  
Huawei Xie Dec. 28, 2015, 5:29 a.m. UTC | #2
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, December 28, 2015 1:27 PM
> To: Xie, Huawei
> Cc: dev@dpdk.org; Jayakumar, Muthurajan; Troitsky, Nikita;
> peterx@redhat.com; stephen@networkplumber.org;
> Changchun.ouyang@hotmail.com; thomas.monjalon@6wind.com
> Subject: Re: [PATCH 4/4] virtio: check if any kernel driver is
> manipulating the device
> 
> On Fri, Dec 25, 2015 at 02:38:12AM +0800, Huawei Xie wrote:
> > virtio PMD could use IO port to configure the virtio device without
> > using uio driver.
> >
> > There are two issues with previous implementation:
> > 1) virtio PMD will take over each virtio device blindly even if some
> > are not intended for DPDK.
> > 2) driver conflict between virtio PMD and virtio-net kernel driver.
> >
> > This patch checks if there is any kernel driver manipulating the
> virtio
> > device before virtio PMD uses IO port to configure the device.
> >
> > Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
> >
> > Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> > ---
> >  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> > index 00015ef..504346a 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -1138,6 +1138,13 @@ static int
> virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
> >  	int found = 0;
> >  	size_t linesz;
> >
> > +	if (pci_dev->kdrv != RTE_KDRV_NONE) {
> > +		PMD_INIT_LOG(ERR,
> > +			"%s(): kernel driver is manipulating this device." \
> > +			" Please unbind the kernel driver.", __func__);
> 
> PMD_INIT_LOG already prints the function, no need to reference
> __func__
> again here.

Good. We have to fix the similar issue in virtio_resource_init_xx as well.

> 
> 	--yliu
> > +		return -1;
> > +	}
> > +
> >  	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
> >  		 pci_dev->addr.domain,
> >  		 pci_dev->addr.bus,
> > --
> > 1.8.1.4
  
Huawei Xie Jan. 4, 2016, 9:02 a.m. UTC | #3
On 12/25/2015 6:33 PM, Xie, Huawei wrote:
> virtio PMD could use IO port to configure the virtio device without
> using uio driver.
>
> There are two issues with previous implementation:
> 1) virtio PMD will take over each virtio device blindly even if some
> are not intended for DPDK.
> 2) driver conflict between virtio PMD and virtio-net kernel driver.
>
> This patch checks if there is any kernel driver manipulating the virtio
> device before virtio PMD uses IO port to configure the device.
>
> Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
>
> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 00015ef..504346a 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1138,6 +1138,13 @@ static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
>  	int found = 0;
>  	size_t linesz;
>  
> +	if (pci_dev->kdrv != RTE_KDRV_NONE) {
> +		PMD_INIT_LOG(ERR,
Better change ERR to INFO and revise the message followed, since user
might not want to use this device for DPDK.
> +			"%s(): kernel driver is manipulating this device." \
> +			" Please unbind the kernel driver.", __func__);
> +		return -1;
> +	}
> +
>  	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
>  		 pci_dev->addr.domain,
>  		 pci_dev->addr.bus,
  
Stephen Hemminger Jan. 4, 2016, 5:29 p.m. UTC | #4
On Mon, 4 Jan 2016 09:02:53 +0000
"Xie, Huawei" <huawei.xie@intel.com> wrote:

> > +		PMD_INIT_LOG(ERR,  
> Better change ERR to INFO and revise the message followed, since user
> might not want to use this device for DPDK.
> > +			"%s(): kernel driver is manipulating this device." \
> > +			" Please unbind the kernel driver.", __func__);

The addition of __func__ here is redundant since this is already in PMD_INIT_LOG macro.
  
Panu Matilainen Jan. 5, 2016, 2:44 p.m. UTC | #5
On 01/04/2016 11:02 AM, Xie, Huawei wrote:
> On 12/25/2015 6:33 PM, Xie, Huawei wrote:
>> virtio PMD could use IO port to configure the virtio device without
>> using uio driver.
>>
>> There are two issues with previous implementation:
>> 1) virtio PMD will take over each virtio device blindly even if some
>> are not intended for DPDK.
>> 2) driver conflict between virtio PMD and virtio-net kernel driver.
>>
>> This patch checks if there is any kernel driver manipulating the virtio
>> device before virtio PMD uses IO port to configure the device.
>>
>> Fixes: da978dfdc43b ("virtio: use port IO to get PCI resource")
>>
>> Signed-off-by: Huawei Xie <huawei.xie@intel.com>
>> ---
>>   drivers/net/virtio/virtio_ethdev.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index 00015ef..504346a 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -1138,6 +1138,13 @@ static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
>>   	int found = 0;
>>   	size_t linesz;
>>
>> +	if (pci_dev->kdrv != RTE_KDRV_NONE) {
>> +		PMD_INIT_LOG(ERR,
> Better change ERR to INFO and revise the message followed, since user
> might not want to use this device for DPDK.

Indeed. The whole point of this exercise is to have a clear way of 
telling DPDK which virtio devices it should (and should not) use, so it 
should just act accordingly and shut up.

>> +			"%s(): kernel driver is manipulating this device." \
>> +			" Please unbind the kernel driver.", __func__);

I'd suggest just dropping the whole message, DPDK doesn't log such 
messages for any other devices either. That, or make it a generic 
debug-level log in pci_scan_one().

	- Panu -
  

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 00015ef..504346a 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1138,6 +1138,13 @@  static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
 	int found = 0;
 	size_t linesz;
 
+	if (pci_dev->kdrv != RTE_KDRV_NONE) {
+		PMD_INIT_LOG(ERR,
+			"%s(): kernel driver is manipulating this device." \
+			" Please unbind the kernel driver.", __func__);
+		return -1;
+	}
+
 	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
 		 pci_dev->addr.domain,
 		 pci_dev->addr.bus,