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....)
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....
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.
If I understand these changes, interface routes once existed, but are
now no longer supported. I guess that's a good thing. But did they
maybe have other (potential) uses than just mobile IP?
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.
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.)
Ditto for RTA_SRCIFP, although I kind think this one was never visible
to 3rd party code.
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.
I'm looking forward the simplified code paths in IP.
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? :-)
-- Garrett
Garrett D'Amore wrote:
Sebastien Roy wrote:
Mobile IPv4 is being removed from OpenSolaris as detailed in PSARC
2007/311
(http://www.opensolaris.org/os/community/arc/caselog/2007/311/), and
you can help that effort by participating in the code-review.
To give you an idea of the scope of this review, the work entails the
removal of two packages (SUNWmipr and SUNWmipu), the removal of 81
source files, and the modification of 27 remaining files. Within
those modified files, 2157 lines of code were removed, mostly within
the ip kernel module.
Please provide comments by Friday July 13th. Also please notify me
as soon as possible if you plan on participating in the review so
that I can account for appropriate review coverage.
The webrev is located here:
http://cr.opensolaris.org/~seb/rm_mobileip_webrev/
For those within SWAN, the workspace (including cscope databases in
usr/src and usr/src/uts) is located here:
/net/zhadum.east/export/ws/seb/rm_mobileip_cr/
Regression tests run have included the TCP, NFSv4, Connectathon NFS,
NFSv2, and IPv6 basic API tests. All pass. In addition, I've run
netperf and ttcp tests showing that the change does not affect
performance (performance improvements were in the noise). I'm still
waiting on a working CGTP test suite to run CGTP tests. I plan on
doing this prior to integration.
I'd bet, with a high degree of confidence, that if you tried doing an
IP forwarding test with small packets, you'd find that performance
improvements are _not_ in the noise, unless your noise filter is set
too low.
Another way to test would be to try doing performance runs with a 10g
card using something _other_ than TCP (UDP rx would be good). Look at
the improvements to the packets-per-second count rather than the
thruput numbers. :-)
If you're not CPU bound, you won't see the benefit.
-- Garrett
Thanks,
-Seb
_______________________________________________
networking-discuss mailing list
[email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
