The webrev at
http://cr.opensolaris.org/~sowmini/nce_walker/
has been udpated based on comments from Jim/Meem. And I've also ripped
out the ndd support for the ndd commands to dump ire/nce tables.
Review input appreciated!
--Sowmini
On (01/02/08 14:05), James Carlson wrote:
>
> Great! A handful of comments on the code:
>
> - I agree with meem that the version-selecting option should be
> consistent among the dcmds, as much as is possible. If "-v" is
> defined, it should mean "verbose," not "version."
>
> - 1318: why 50? It looks like this could be allocated, if need be.
>
> - At line 1338, it looks like we arbitrarily read 50 bytes from the
> target. How do we know that hardware addresses are exactly 50
> bytes, or that any of the bytes following the address are mapped
> in memory? Shouldn't this be bounded by mp.b_wptr and/or
> ill->ill_nd_lla_len?
>
> - 1344: addrlen is unsigned, so "<" can't happen.
>
> - 1344-1345: could be tested at 1325 with "|| ill->ill_nd_lla_len == 0"
>
> - 1347: why is the cast needed?
>
> - 1573,1575: unused?
>
> > Sample output of this command is shown below. Having the mdb macro
> > allows us to drop the code for ip_ndp_cache_report,
> > and even for the ndd ipv*_ire_status commands ('::ire -v' provides the same
> > information), though the nce_walker webrev does not remove this code (yet).
>
> Yes; it'd be good to see that old ndd swill go. Please update CR
> 4616660 while you're at it.
>
