Cathy Zhou wrote: >> Seb/Cathy, >> >> I've been working on a wad of additional improvements to the UV sources. >> While many of the changes are just comment/message fix-ups and other nits, >> there are some substantive changes too: >> >> * All of the dl_* routines (that were in strplumb.c, nfs_dlinet.c >> and softmac_ctl.c) have been unified into a single set of >> routines under sundlpi.h, with headers in <sys/dlpi.h>. The new >> routines also fix a number of bugs in the old code, such >> incorrect assumptions that an M_PROTO/M_PCPROTO message will >> always have at least 4 bytes and that the SAP is always at the >> end of the DLSAP address. >> >> * A variety of ioctl-related simplifications and bugfixes have >> been made to dld_drv.c. >> >> * A new softmac_process_dlpi() utility routine in softmac_ctl.c. >> >> * A number of unnecessary dependencies on Ethernet have been >> removed from the softmac code. >> >> I'm still in the process of testing these changes, but I'd like to kick >> off the review now so that I can hopefully putback the wad before the >> end of the week. >> >> BTW, I'll be filing a Nevada bug for the dl_* duplication, which we can >> just toss into the UV Nevada putback. >> >> webrev: http://cr.opensolaris.org/~meem/uv-tweaks >> cscope: /net/atlantic.east/export/ws/meem/uv-tweaks/usr/src >> > Thanks for all these changes. See my comments below:
sundlpi.c - line222 why ETIME is different compared to other errno and needs special case? - line438 !MBLKIN(mp, addroff, addrlen + ABS(dliap->dl_sap_length) can be changed to !MBLKIN(mp, addroff, dliap->dl_addr_length) - You forgot to freemsg() in some successful case in dl_info(). nfs_dlinet.c - Line1551, 1557, 1563 explicitly check if (rc != 0)? dld_drv.c - Line 668 should be "diap->dia_npush > MAXAPUSH", not >=. - Line 683 you assign mod_hash_key_t to a mod_hash_val_t? and mod_hash_find() and mod_hash_insert() should use key not val as the second argument. softmac_main.c - Line 611 What if dl_notify() returns 0? Note that softmac->smac_notifications should only be set when dl_notify() returns 0. softmac_ctl.c - Line 214 should be "!MBLKIN(mp, addroff, dlp->dl_addr_length)"? - Line 222 As we discussed, this is not needed, right? - Line 276-279, 286-290 are not needed as the length check is done in softmac_process_dlpi(). Thanks - Cathy
