Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround to ignore interfaces whose name got truncated by netstat #149

Closed
wants to merge 2 commits into from

Conversation

hellais
Copy link
Contributor

@hellais hellais commented May 8, 2016

When interface names are longer than 7 characters netstat will truncate
them to the first 7 chars which leads to looking up an invalid interface
name.

This workaround ignores interfaces that are too long.

This fixes #147

@guedou
Copy link
Member

guedou commented May 10, 2016

While a fix to #147 is really important, I am not a big fan of ignoring truncated interface names. It should be the ultimate solution.
Instead, a better fix will be to first try to fix truncated interface names using output from ifconfig -l, then fail if its not possible.

We are still working on a good way to add unit tests to functions such as read_routes(). PR #127 fixes read_routes6() on OS X and add unit tests to other platforms using the mock module.

@dark-lbp
Copy link
Contributor

May be try netiface for that problem?
netifaces

@guedou
Copy link
Member

guedou commented May 18, 2016

@dark-lbp there is no need to rely on an external module. I think that @hellais should be able to implement my proposal quite easily.

When interface names are longer than 7 characters netstat will truncate
them to the first 7 chars which leads to looking up an invalid interface
name.
We attempt to guess the name of the interface based on the output of
ifconfig -l. If there is only one candate we consider that otherwise we
ignore it.
@hellais
Copy link
Contributor Author

hellais commented May 18, 2016

Rebased and pushed with support for guessing the interface name.

@hellais hellais force-pushed the fix/os-error-not-configured branch from e702467 to 9144c8c Compare May 18, 2016 15:11
return it. If there are none or more, then we return None.
"""
f = os.popen('ifconfig -l')
ifaces = f.readlines()[0].strip().split(' ')
Copy link
Member

@p-l- p-l- May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inefficient. Maybe this would be better?

from __future__ import with_statement

[...]

    with os.popen('ifconfig -l') as fdesc:
        ifaces = fdesc.readline().strip().split()
    for iface in ifaces:
        if iface.startswith(netif):
            return iface

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I can change to use the with statement.

Regarding:

    for iface in ifaces:
        if iface.startswith(netif):
            return iface

I want to avoid returning an interface name when there are multiple matches as that means that I am actually not sure what interface is the correct one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Anyway, please avoid filter + lambda: matches = [iface for iface in ifaces if iface.startswith(netif)]

@guedou
Copy link
Member

guedou commented May 19, 2016

@Osso does this fix your bug ?

@hellais could you share the output of netsat and get_if_list(), I would like to add a mocked unit tests to our regressions.

@Osso
Copy link
Contributor

Osso commented May 19, 2016

Yes this fixes my bug
I get this warning but all is good:
WARNING: Could not guess partial interface name None

# This means the interface name is probably truncated by
# netstat -nr. We attempt to guess it's name and if not we
# ignore it.
netif = _guess_iface_name(netif)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be interesting to assign the returned value to fixed_netif in order to display a correct warning. See @Osso comment.

@guedou
Copy link
Member

guedou commented May 25, 2016

read_routes6()might be also broken.
Could you share the output of netstat -rn -f inet6 ?

Thanks.

@Osso
Copy link
Contributor

Osso commented May 25, 2016

I bet it is broken too but the default vagrant configuration is without ipv6.
As a result on my ipv6 netstat I do not have any truncated interface.

@guedou
Copy link
Member

guedou commented Jun 29, 2016

@hellais could you apply my comment ?

@guedou
Copy link
Member

guedou commented Aug 9, 2016

Due to no recent activity on this PR, I modified the original patch and created PR #260.

@guedou guedou closed this Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When interface names are too long read_routes() with exceptions.OSError: Device not configured
5 participants