DPDK patches and discussions
 help / color / mirror / Atom feed
From: Mark Asselstine <mark.asselstine@windriver.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: alloc <chunguang.yang@windriver.com>, <dev@dpdk.org>,
	"Wang, Weiwei" <Weiwei.Wang@windriver.com>
Subject: Re: [dpdk-dev] [PATCH] tools/dpdkdevbind.py: remove call to lower case for mod path
Date: Mon, 1 May 2017 11:24:13 -0400	[thread overview]
Message-ID: <1959888.bZH0xSPdBI@yow-masselst-lx1> (raw)
In-Reply-To: <3349839.Ts7blMUiL7@xps>

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:
            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.

Can Alloc improve the commit log to make this clear, sure. But the change is 
good and should be merged.

Mark

  reply	other threads:[~2017-05-01 15:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-25  3:16 alloc
2017-04-28  9:38 ` Thomas Monjalon
2017-05-01 15:24   ` Mark Asselstine [this message]
2017-05-01 15:33     ` Mark Asselstine
2017-05-01 16:29       ` Thomas Monjalon
2017-05-01 17:30         ` Mark Asselstine
  -- strict thread matches above, loose matches on Subject: below --
2016-11-25  3:10 alloc

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=1959888.bZH0xSPdBI@yow-masselst-lx1 \
    --to=mark.asselstine@windriver.com \
    --cc=Weiwei.Wang@windriver.com \
    --cc=chunguang.yang@windriver.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).