[Python-Dev] PEP 3144 review. (original) (raw)

R. David Murray rdmurray at bitdance.com
Tue Sep 29 21:24:38 CEST 2009

On Tue, 29 Sep 2009 at 14:19, Glyph Lefkowitz wrote:

In addition to the somebody-must-have-mentioned-this-already feeling that I got, I hesitated to post this message because it doesn't actually seem that important to me. While I'm firmly convinced that Network.ip is a design mistake, it's not like the rest of Python, or for that matter any software, is completely perfect. In fact I think this mistake is significantly less bad than some of the others already present in Python. If Peter remains unconvinced, I do think that we should put it in the stdlib, move on, and get to fixing some of the other stuff we agree needs fixing rather than continuing to re-hash this. Primarily because, as far as I can tell, if hashing and equality are defined the way that everyone seems to be agreeing they be defined (ignoring the .ip attribute) then those of us who think .ip is a design error can use the library and safely ignore it completely.

I think Glyph is petty much on target here. I think I dislike the .ip attribute more than he does, but I'm willing to let go of it now that the equality issue has been addressed so that the class acts like a Network object except for the ip attribute.

I do want to show why I think the ip attribute is a bad idea, using a concrete example, rather than the hand waving we've done up to this point. This is a stripped down example of something that could be a real application. I started writing this to show what the difference between using the ipaddr classes versus the completely separate address and network classes would look like, but ended up with something where there would be very little difference in the final program.

Given this simple configuration file:

demo.ini

[gatewayrouter] inside = e1/0 192.168.1.1/24 outside = e1/1 172.16.200.1/23 dmz = e1/2 192.168.2.1/24

[dmzhosts] webserver = 172.16.200.2 192.168.2.30 22 80 442 mailserver = 172.16.200.3 182.168.2.31 22 25 587

Here is a (runable) program to generate the skeleton of a Cisco router configuration file from that configuration (I've left out a bunch of additinoal Cisco configuration I'd include if this were a real application):

import ipaddr import ConfigParser

config = ConfigParser.SafeConfigParser() config.read('demo.ini') output = open('gatewayrouter.config', 'w')

class InterfaceData(object):

 def __init__(self, interfacespec):
     self.interfacename, ipwithnetmask = interfacespec.split()
     self.network = ipaddr.IPv4Network(ipwithnetmask)
     self.ip = self.network.ip
     self.netmask = self.network.netmask

class DMZHost(object):

 def __init__(self, hostdata):
     data = hostdata.split()
     outsideip, insideip = data[0:2]
     self.ports = data[2:]
     self.outsideip = ipaddr.IPv4Address(outsideip)
     self.insideip = ipaddr.IPv4Address(insideip)

interfaces = {} for name in ('inside', 'outside', 'dmz'): interfacedata = InterfaceData(config.get('gatewayrouter', name)) interfaces[name] = interfacedata globals()[name] = interfacedata.network

dmzhosts = {} for host, hostdata in config.items('dmzhosts'): dmzhosts[host] = DMZHost(hostdata)

for idata in interfaces.values(): print >>output, 'interface {}'.format(idata.interfacename) print >>output, 'ip address {} {}'.format(idata.ip, idata.netmask)

print >>output, 'ip extended access-list outside_in' for host in dmzhosts.values(): for port in host.ports: print >>output, 'permit ip any host {} eq {}'.format( host.outsideip, port) for icmp in ('echo', 'echo-reply', 'host-unreachable', 'time-exceeded'): print >>output, 'permit icmp any host {} {}'.format( interfaces['inside'].ip, icmp) print >>output, 'permit icmp any {} {} {}'.format( dmz.network, dmz.hostmask, icmp)

print >>output, 'ip extended access-list nat' print >>output, 'permit ip {} {} any'.format(inside.network, inside.hostmask) print >>output, 'permit ip {} {} any'.format(dmz.network, dmz.hostmask)

print >>output, ('ip nat inside source access-list nat ' 'interface {} overload').format(interfaces['outside'].interfacename) for host in dmzhosts.values(): print >>output, 'ip nat inside source static {} {}'.format( host.insideip, host.outsideip)

There are several things to note about this code. The first is that I wanted my own wrapper classes to deal with the data, IPv4Network/Address by themselves weren't enough, nor would an IPWithNetmask class have been. I also notice that I immediately pulled out the 'ip' address from the IPv4Network object to a top level attribute in my wrapper, so for me a ParseIPWithMask that returned separate IPAddress and an IPNetwork classes would have been more natural. On the other hand, I also pulled out the 'netmask', which I would still have needed to do.

There's one place in this code where the inclusion of the 'ip' information in the IPNetwork class could have been used. In the line that allows ICMP traffic to the router's outside port, I could have written 'inside.ip' instead of interfaces['inside'].ip. I think that would have confused me when I came back to read the code later, though, since 'inside' is otherwise a network object.

But the real 'ah ha' moment in writing this program came when I wrote the lines for the 'nat' access list. I first wrote the line like this:

print >>output, 'permit ip {} {} any'.format(inside.ip, inside.hostmask)

because I wanted the base IP of the inside network to go into the access-list statement. The code would have run, but it would have produced incorrect output. It also felt unnatural to write '.network' there, since the object, in my mind, was a network, which is presumably why I wrote 'inside.ip' at first, before catching myself. But that I can certainly live with, just as I can live with '.ip' if I have to :)

--David

More information about the Python-Dev mailing list