[v2,5/6] usertools/setup: fix loading vfio module

Message ID 20201126141832.2277628-6-ferruh.yigit@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series update dpdk-setup.sh |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ferruh Yigit Nov. 26, 2020, 2:18 p.m. UTC
  script is checking the existing of the kernel module file, but in some
distros kernel modules are stored compressed, like as 'vfio-pci.ko.xz'.

Add wildcard to cover both compressed and not compressed cases.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 usertools/dpdk-setup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Conor Walsh Nov. 26, 2020, 3:05 p.m. UTC | #1
Hi Ferruh,

I tested this patch on Ubuntu 20.04 (5.4.0-53-generic) and
Centos 8.2 (4.18.0-193.28.1.el8_2.x86_64), load VFIO worked on both systems.
I'm not sure if this is a problem that effects either of these distros though.

Thanks,
Conor.

> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday 26 November 2020 14:19
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org;
> techboard@dpdk.org; Stephen Hemminger
> <stephen@networkplumber.org>; Richardson, Bruce
> <bruce.richardson@intel.com>; Walsh, Conor <conor.walsh@intel.com>
> Subject: [PATCH v2 5/6] usertools/setup: fix loading vfio module
> 
> script is checking the existing of the kernel module file, but in some
> distros kernel modules are stored compressed, like as 'vfio-pci.ko.xz'.
> 
> Add wildcard to cover both compressed and not compressed cases.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

Tested-by: Conor Walsh <conor.walsh@intel.com>
  
David Marchand Nov. 26, 2020, 6:31 p.m. UTC | #2
On Thu, Nov 26, 2020 at 3:20 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> script is checking the existing of the kernel module file, but in some
> distros kernel modules are stored compressed, like as 'vfio-pci.ko.xz'.

Since this script expects modprobe to be installed (coming with kmod
tools), it means modinfo is available.
Checking for module availability should be "modinfo vfio-pci".
  
Ferruh Yigit Nov. 27, 2020, 9:55 a.m. UTC | #3
On 11/26/2020 6:31 PM, David Marchand wrote:
> On Thu, Nov 26, 2020 at 3:20 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> script is checking the existing of the kernel module file, but in some
>> distros kernel modules are stored compressed, like as 'vfio-pci.ko.xz'.
> 
> Since this script expects modprobe to be installed (coming with kmod
> tools), it means modinfo is available.
> Checking for module availability should be "modinfo vfio-pci".
> 

+1, that is definitely better.

I can send a new version with it if we decide to keep the script.

Mainly we are tying to decide to remove the script or keep with the changes in 
this set.
There aren't still much comment on it, only comment by you in favor of removal, 
I haven't seen any comment to keep the script.

Added more folks for comment, also techboard comments are welcome.
  
Thomas Monjalon Nov. 27, 2020, 1:56 p.m. UTC | #4
27/11/2020 10:55, Ferruh Yigit:
> On 11/26/2020 6:31 PM, David Marchand wrote:
> > On Thu, Nov 26, 2020 at 3:20 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >> script is checking the existing of the kernel module file, but in some
> >> distros kernel modules are stored compressed, like as 'vfio-pci.ko.xz'.
> > 
> > Since this script expects modprobe to be installed (coming with kmod
> > tools), it means modinfo is available.
> > Checking for module availability should be "modinfo vfio-pci".
> > 
> 
> +1, that is definitely better.
> 
> I can send a new version with it if we decide to keep the script.

It is too late to improve this old script.
We should let it die and rest in peace.

> Mainly we are tying to decide to remove the script or keep with the changes in 
> this set.

I appreciate your effort Ferruh.
It shows that almost nothing is worth to keep in this script.

> There aren't still much comment on it, only comment by you in favor of removal, 
> I haven't seen any comment to keep the script.
> 
> Added more folks for comment, also techboard comments are welcome.

My opinion is that we should follow the deprecation notice
and remove the script in 20.11.
If someone wants to keep the same VFIO manipulations,
it is possible to copy/paste from old versions.
  
Ferruh Yigit Nov. 27, 2020, 3:29 p.m. UTC | #5
On 11/27/2020 1:56 PM, Thomas Monjalon wrote:
> 27/11/2020 10:55, Ferruh Yigit:
>> On 11/26/2020 6:31 PM, David Marchand wrote:
>>> On Thu, Nov 26, 2020 at 3:20 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>
>>>> script is checking the existing of the kernel module file, but in some
>>>> distros kernel modules are stored compressed, like as 'vfio-pci.ko.xz'.
>>>
>>> Since this script expects modprobe to be installed (coming with kmod
>>> tools), it means modinfo is available.
>>> Checking for module availability should be "modinfo vfio-pci".
>>>
>>
>> +1, that is definitely better.
>>
>> I can send a new version with it if we decide to keep the script.
> 
> It is too late to improve this old script.
> We should let it die and rest in peace.
> 
>> Mainly we are tying to decide to remove the script or keep with the changes in
>> this set.
> 
> I appreciate your effort Ferruh.
> It shows that almost nothing is worth to keep in this script.
> 
>> There aren't still much comment on it, only comment by you in favor of removal,
>> I haven't seen any comment to keep the script.
>>
>> Added more folks for comment, also techboard comments are welcome.
> 
> My opinion is that we should follow the deprecation notice
> and remove the script in 20.11.
> If someone wants to keep the same VFIO manipulations,
> it is possible to copy/paste from old versions.
> 
> 
OK, there is already a patch to remove it, updating this series as rejected.
  

Patch

diff --git a/usertools/dpdk-setup.sh b/usertools/dpdk-setup.sh
index 1e36661e572e..e8667e094a53 100755
--- a/usertools/dpdk-setup.sh
+++ b/usertools/dpdk-setup.sh
@@ -62,7 +62,7 @@  load_vfio_module()
 {
 	remove_vfio_module
 
-	VFIO_PATH="kernel/drivers/vfio/pci/vfio-pci.ko"
+	VFIO_PATH="kernel/drivers/vfio/pci/vfio-pci.ko*"
 
 	echo "Loading VFIO module"
 	/sbin/lsmod | grep -s vfio_pci > /dev/null