On Thu, 2009-09-24 at 14:53 -0700, Erik Nordmark wrote: > Sebastien Roy wrote: > > On Wed, 2009-09-16 at 12:30 -0700, Erik Nordmark wrote: > >> The webrev for usr/src is at > >> http://cr.opensolaris.org/~nordmark/refactor-review/ > > > > I reviewed the changes under usr/src/uts/common/inet/iptun. Please let > > me know if you think I should be looking at other changes and I'll take > > a look. Here are my comments: > > Thanks for the careful review. I'll go over the code with these comments > tomorrow. > > But there is also rewritten fanout code in ip_input.c and ip6_input.c > for tunnels, plus the ipclassifier.c. It would be good if you can look > at those.
Okay, will do. > > > * 786-800: It seems like there should be some utility function to set > > conn addresses. For example, the fact that IPv4 addresses are > > stored as IPv4-mapped IPv6 addresses in the conn_t seems like an > > implementation detail of ip that iptun shouldn't really need to care > > about. > > But the conn_t doesn't belong to IP; it belongs to all the ULPs plus all > of IP. Introducing an extra function which contains one line seems like > introducing an abstraction of low "hight" (forces the reader to find and > then understand the abstraction, yet the abstraction is trivial.) That's fine. > > > * 2043: In the case of IPTUN_FIXED_MTU, don't we only need update > > ixa_pmtu when we initially set iptun_mtu? > > No, and tried to make this clear by adding strings like "upper" before > "mtu" in some place. > fixedmtu merely effects which mtu the iptun device presents to its upper > layers. > ixa_pmtu is what it sees from IP. Normally they differ by the size of > the iptun headers, but in the case of fixedmtu they are completely > decoupled. Understood, thanks for clarifying. -Seb _______________________________________________ networking-discuss mailing list [email protected]
