Thanks for the review!
Garrett D'Amore wrote:
On the removal of mipagent, do you need to do anything to update SMF?
I'm not sure how legacy_run services are handled... do they have
database entries in SMF? Please make sure you test a real upgrade to
make sure it all "Just Works" :-) (Says the man who's been bitten by
SMF in the past....)
AFAIK, nothing is needed other than removing the legacy init.d script.
I'll triple check just in case.
I notice that ire_create,ire_create_mp have a _lot_ of arguments. This
has got to play murder on the register set. But at least it is one
less now. (And in a few other places too.) Someday it would be nice to
try to figure out how to simplify it even further....
Indeed. Feel free to file a bug. :-)
There are a lot of branches you removed. Several on hot code paths.
Yay! I also wonder, have you tested with route flap tests? Or with
just a general routing performance test? A lot of these changes are
going to help that performance.
I'd be glad to do so if someone can point me at a rig and/or an easy to
use test.
If I understand these changes, interface routes once existed, but are
now no longer supported.
What was removed is the Mobile IP per-ill routing table which was a
special routing table on Mobile IP foreign agents. For each mobile node
in the foreign agent's link, an IP tunnel was created to the home agent.
Each IP tunnel ill had one of these special routing tables that was
only used when forwarding packets from such an ill to the local link.
The Mobile IP reverse tunnel table was also removed. It was a forwarding
table in which routes were matched by source address. This could somehow
be massaged into general applicability, but it utterly broken as
implemented (6511070). It was also specific to reverse tunnels, and a
bit of engineering would need to be applied to make this a more general
thing.
I guess that's a good thing. But did they
maybe have other (potential) uses than just mobile IP?
As designed and implemented, I don't see an immediate use for either of
these tables other than for the removed Mobile IP implementation.
In ipsec_info.h:203, you've deleted a bit. Would it have been safer to
create a reserved bit? Is this message ever exposed to other modules?
(e.g. can a 3rd party streams module see it?) I'm thinking about binary
compatibility here.
ipsec_out_t is strictly an ip module implementation detail. No potential
for binary compatibility issues.
Likewise, it might be a good idea to reserve the IFF_MIPRUNNING value in
if.h, indicate that it used to mean something. There could be some old
3rd party code that looked at it. (Yeah, I know its unlikely.)
There is a comment where the constant was previously stating:
/* 0x0004000000 was IFF_MIPRUNNING */
To make it evident that the value is currently unused. I don't see a
reason why someone couldn't come along and re-use that value down the
road since mipagent was the only application known to have a use for it.
Ditto for RTA_SRCIFP, although I kind think this one was never visible
to 3rd party code.
The header file may have been included by 3rd party code, but this
particular routing socket address type couldn't have been of any use
except by mipagent and on systems running mipagent (which we're fairly
certain there are none out there). Do you really feel that reserving
this value will do anything?
I only looked at the *sdiffs*, I didn't go through and check to see if
you have located all the occurrences. (I think someone else already
commented on that.) But otherwise, as far as I can tell, the diffs look
good.
Thanks. Yes, Meem has found a bunch of places where comments and forward
declarations needed removal.
I'm looking forward the simplified code paths in IP.
Glad to be of help. :-)
It may be worthwhile at some point to look at other changes that can
simplify the code, and see if there might be some "larger" scale
simplifications can be made by removal of the need for these extra
checks. But that's a bigger task.... IP datapath refactoring anyone? :-)
Right, there is a greater need to simplify the ip module. This is only a
tiny fraction of what's needed.
Thanks again,
-Seb
_______________________________________________
networking-discuss mailing list
[email protected]
