Netstat Speed

[DarkBlue]

Following code works .
My question is can I make it faster ?


def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb=0
for line in os.popen("netstat -a -n | grep '%s'" % checkip) :
s=line
if s <>'' :
activeb=1
return activeb


Thanks

 

[DarkBlue]

s=line
is actually indented even if it does not appear to be so

Db

 

[Sybren Stuvel]

DarkBlue enlightened us with:

Following code works .

No it doesn't - it's indented badly. I guess you mean something like:

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb=0
for line in os.popen("netstat -a -n | grep '%s'" % checkip) :
s=line
if s <>'' :
activeb=1
return activeb

My question is can I make it faster ?

As soon as you set activeb=1, it no longer changes to any other value.
That means that you could immediately return from that point. It's
also cleaner to actually close an open file:

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb = 0
netstat = os.popen("netstat -a -n | grep '%s'" % checkip)
for line in netstat:
s = line
if s:
activeb = 1
break
netstat.close()
return activeb

Another way would be to let grep do the work for you. Pass it the '-q'
option and check its exit code.

Sybren

 

[Tal Einat]

Following code works .
My question is can I make it faster ?

Faster how? Are you calling this so many times that you need it to be
extremely fast? Are you calling it on the same IP multiple times, on
many different IPs at once, ...?

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb=0
for line in os.popen("netstat -a -n | grep '%s'" % checkip) :
s=line
if s <>'' :
activeb=1
return activeb

Since you state you want this faster I assume you're calling this quite
a bit. To start with, perhaps you could save the output of netstat, go
over it once collecting IPs, and save these for future lookup. Surely
this will be faster than running a shell command every time.

- Tal

 

[DarkBlue]

DarkBlue enlightened us with:

No it doesn't - it's indented badly. I guess you mean something like:

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb=0
for line in os.popen("netstat -a -n | grep '%s'" % checkip) :
s=line
if s <>'' :
activeb=1
return activeb



As soon as you set activeb=1, it no longer changes to any other value.
That means that you could immediately return from that point. It's
also cleaner to actually close an open file:

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb = 0
netstat = os.popen("netstat -a -n | grep '%s'" % checkip)
for line in netstat:
s = line
if s:
activeb = 1
break
netstat.close()
return activeb

Another way would be to let grep do the work for you. Pass it the '-q'
option and check its exit code.

Sybren

Thank you for pointing that out.
Speed did not improve , but it definitely
is cleaner code now.

Have a nice day
Db

 

[Sybren Stuvel]

DarkBlue enlightened us with:

Thank you for pointing that out. Speed did not improve , but it
definitely is cleaner code now.

Good

If you want a speed improvement, follow the other suggestion by
getting the netstat output once, and then process it whenever
required. If you give us more insight into the workings and purpose of
your program, we might be able to help you some more.

Sybren

 

[Jorgen Grahn]

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb = 0
netstat = os.popen("netstat -a -n | grep '%s'" % checkip)
for line in netstat:
s = line
if s:
activeb = 1
break
netstat.close()
return activeb

Another way would be to let grep do the work for you. Pass it the '-q'
option and check its exit code.

I suspect it's faster to skip the grep altogether and do the string search
in Python. That also means you can drop the implicit shell invocation, and
just exec netstat, period.

No matter what, I think you'll find most of the wall time is spent starting
netstat and having it wade through kernel statistics.

/Jorgen

 

[Jorgen Grahn]

DarkBlue enlightened us with:

No it doesn't - it's indented badly. I guess you mean something like:

def activeip(checkip):
# if there is any line returned we set activeb to 1 indicating that
# this ip is active during a netstat run
activeb=0
for line in os.popen("netstat -a -n | grep '%s'" % checkip) :
s=line
if s <>'' :
activeb=1
return activeb

Also try to use != instead of <>. Being equal or not has nothing to do
with being larger or smaller.

Yeah, that took me a few seconds to mentally convert to !=. Personally, I'd
just write "if line:" in this case.

While we're at it, please write the documentation from a /user/ standpoint,
not as internal scribblings. And don't call the address 'checkip' -- that
makes it look like you pass a functor to the function. Maybe something like
this:

| def ip_is_active(addr):
| """Return success if 'addr' shows up in the output from 'netstat -an'.
| We assume that a suitable version of netstat exists.
| """
| activeb=0
| ...

This doc string is vague -- but that's because it's unclear what the
function does and what you expect it to do. I see half a dozen problems or
so, but that might be ok if its callers don't expect too much.

/Jorgen

 

[Sybren Stuvel]

Jorgen Grahn enlightened us with:

| def ip_is_active(addr):
| """Return success if 'addr' shows up in the output from 'netstat -an'.
| We assume that a suitable version of netstat exists.
| """

If I have an "if" in the docstring, I always write the "else" case as
well:

"""Returns True if 'addr' shows up in the output from 'netstat -an',
and False otherwise. We assume that a suitable version of netstat
exists.
"""

This makes it clear what happens in either case.

Sybren

 

[Jorgen Grahn]

Jorgen Grahn enlightened us with:

If I have an "if" in the docstring, I always write the "else" case as
well:

"""Returns True if 'addr' shows up in the output from 'netstat -an',
and False otherwise. We assume that a suitable version of netstat
exists.
"""

This makes it clear what happens in either case.

Tastes differ. I think it's clear enough without the "else". Note that I
changed the name of the function too, to make it read like a test (or rather
an assertion): "if not ip_is_active('127.0.0.1')".

/Jorgen