On Monday, May 1, 2017 6:29:18 PM EDT Thomas Monjalon wrote: > 01/05/2017 17:33, Mark Asselstine: > > On Monday, May 1, 2017 11:24:13 AM EDT Mark Asselstine wrote: > > > On Friday, April 28, 2017 11:38:17 AM EDT Thomas Monjalon wrote: > > > > 25/11/2016 04:16, alloc: > > > > > If the module path has upper case chars, the dpdk-devbind.py script > > > > > will > > > > > crunch them to lower case. This will result in the script never > > > > > finding a module. > > > > > > > > I wonder why this "lower" was done. > > > > I'm afraid we are missing something. > > > > Nobody else is complaining about this issue. > > > > Please confirm it is a real issue. > > > > > > The commit (d6537e6a7432ea9cf39fc4ab2112d4bce0e9fe57) that brought in > > > the > > > lower() call does not document any specific reason for its inclusion. So > > > unfortunalely we can't rely on historic wisdom to rule out this change. > > > > > > We can however look at the source to determine that the lower() call is > > > bogus. > > > > > > --- usertools/dpdk-devbind.py --- > > > > > > # check using depmod > > > > > > try: > > > depmod_out = check_output(["modinfo", "-n", mod], > > > > > > stderr=subprocess.STDOUT).lower() > > > > > > if "error" not in depmod_out: > > Actually, looking at this I can see only one reason for the lower(), and > > that is to catch 'ERROR vs. Error vs. error vs. ...". > > Yes it is exactly what I was thinking. > > > So Alloc can you make an > > > > additionaly change and in the line above change it to: > > if "error" not in depmod_out.lower(): > Good suggestion. > > > That should address any concerns. Of course this still leaves a corner > > case of 'error' being in the path, but the place this would exist would > > be in the kernel version and extra-version and I doubt many folks put > > 'error' in there. > > > > Mark > > > > > path = depmod_out.strip() > > > > > > if exists(path): > > > return path > > > > > > except: # if modinfo can't find module, it fails, so continue > > > > > > pass > > > > > > --- > > > From this we know that depmod_out will have the lowercase version of the > > > path to the module. We also know that exists() is case sensitive and > > > therein lies the issue. Since the path to the module will include kernel > > > attributes the only reason folks may not be seeing this issue as that > > > the > > > attributes are only numbers, periods and lowercase alpha chars. Add a > > > singe > > > upper alpha char in the kernel extended name and users will have this > > > issue, as we have seen it. > > Which kernel module has an upper case character?
Not any kernel module, but the kernel version/extra-version can easily have non-lowercase alpha chars. > > > > Can Alloc improve the commit log to make this clear, sure. But the > > > change is good and should be merged. > > It seems Alloc is not his real name? > Please use your real name for SoB (Chunguang Yang?). Seems he is no longer with Wind River so I will take care of sending an updated patch with updated commit log. Mark
