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
>> networking-discuss at opensolaris.org
>
> _______________________________________________
> networking-discuss mailing list
> networking-discuss at opensolaris.org


Reply via email to