Artem,
Looks good overall!
here's my feedback (if a file's not listed, I have no specific
comments on it).
--Sowmini
---------------------------------------------------------------------------
usr/src/cmd/svc/milestone/net-physical
- Don't we still need lines 209-218 for wifi drivers?
usr/src/uts/common/sys/mac_impl.h
- Use DLD_LINKPROP_NAME_MAX for mp_name?
usr/src/uts/common/io/mac/mac.c
- printf at line 3000 can be removed.
- question: what happens if err at line 2947 is EBUSY (e.g., for
mtu, where driver cannot apply the property just yet)? don't
we want to do_update in this case, so that the driver can pick
up the setting next time?
- mac_prop_update() should return an error if kmem_zalloc() fails at
line 3184 (and trickle that error back up from mac_set_prop()
and mac_set_prop_mac_only())
- line 3038, check for null return from kmem_zalloc()
usr/src/uts/common/io/dld/dld_drv.c
- drv_ioc_prop_common(): if I do a GETPROP and incorrectly set
DLD_PROP_FLAG_MAC_ONLY then the code would miss taking the locks.
We probably don't need to test for this explicitly since we'd only
get here from libdladm, but might be good to add an
ASSERT(!get || !mac_only);
after line 534.
usr/src/lib/libdladm/common/libdladm.h
- add comments about DLADM_OPT_PROP_MAC_ONLY/DLADM_OPT_PROP_MAC_NAME.
In terms of their interaction with mac_prop_load(),
the distinction between the 2 is a bit confusing since both of them
have the effect of not invoking the driver's setprop routine.
Are 2 flags needed? It is especially confusing that
setting MAC_NAME will result in mac_set_prop_mac_only() which is
not obvious from the constant's name. Would it be cleaner if
dlmgmtd mapped the MAC_NAME to MAC_ONLY with some comments explaining
why this was being done?
usr/src/uts/common/io/bge/bge_main2.c
- printfs at line 3085, 3091 can be removed (make this a DTRACE_PROBE
instead?)
- merge note: the uint64_t mtu will be a uint32_t after the nddcompat
fixes.
_______________________________________________
networking-discuss mailing list
[email protected]
