Dave, Sorry for the late reply. below are my responses.
---------------------------------------------------------- General (these stray into design, sorry): - Various components (e.g. aggr and vlan) base decisions on both the apparent and native media types of the mac. How appropriate and extensible is this? For example, both aggr and vlan disallow the creation of their respective entities if the native media type of the underlying mac is not DL_ETHER. This may be an appropriate decision if the underlying type is DL_WIFI, but what if there is some other underlying type? | DEFER | as far as I am aware, aggr (802.3ad) is specific to ethernet. so I | think it appropriate to limit the media support to DL_ETHER. | to get aggr to work on non-ethernet hardware, the driver would have | to advertise a fake DL_ETHER native type to the plugin framework. | | vlans should, supposedly, work on wireless networks (802.11q) but I | am not aware of any hardware that supports it at the moment. If in | the future such hardware becomes available, vlan support can negotiated | using a vlan capability and the media type checks in dls can be removed. | usr/src/uts/common/io/mac/plugins/mac_wifi.c: - IEEE80211_HDRSIZE: why "+ 1"? | ACCEPT | the + 1 and roundup() are not necessary. | we'll rename this to WIFI_HDRSIZE and move it mac_wifi.h | since it's only used by the wifi plugin. | - mac_wifi_header_info() makes no attempt to deal with chained mblks. Is there a clear requirement on drivers that they must present the headers in a single block? | EXPLAIN | yes. this is a requirement. | unfortunately this isn't documented yet since gldv3 interfaces | aren't yet public. - mac_wifi_header_uncook(), line 404: mhi_hdrsize >= sizeof (struct ether_header) ? | ACCEPT | I agree it should be '>='. | usr/src/uts/common/sys/dld.h: - how will we increase the size of DLD_SECOBJ_VAL_MAX when that becomes necessary? | EXPLAIN | the ioctls are private so we're free to change them anytime. | for this case, DLD_SECOBJ_VAL_MAX will just be bumped up. | this is not a problem since user libraries are shipped together | with kernel bits. | usr/src/uts/common/io/dld/dld_drv.c: - drv_secobj_init(): is the kmem_cache actually worthwhile? The constructor does nothing but bzero, the kmem_cache_alloc() caller sets all of the fields and the kmem_cache_free() caller doesn't 're-init' the bufer. | EXPLAIN | it's worthwhile because we get a free walker in (k)mdb. | - line 707: prefer "sizeof (*sgp)", given line 710. - line 727: spurious blGeneral (these stray into design, sorry): - Various components (e.g. aggr and vlan) base decisions on both the apparent and native media types of the mac. How appropriate and extensible is this? For example, both aggr and vlan disallow the creation of their respective entities if the native media type of the underlying mac is not DL_ETHER. This may be an appropriate decision if the underlying type is DL_WIFI, but what if there is some other underlying type? | DEFER | as far as I am aware, aggr (802.3ad) is specific to ethernet. so I | think it appropriate to limit the media support to DL_ETHER. | to get aggr to work on non-ethernet hardware, the driver would have | to advertise a fake DL_ETHER native type to the plugin framework. | | vlans should, supposedly, work on wireless networks (802.11q) but I | am not aware of any hardware that supports it at the moment. If in | the future such hardware becomes available, vlan support can negotiated | using a vlan capability and the media type checks in dls can be removed. | usr/src/uts/common/io/mac/plugins/mac_wifi.c: - IEEE80211_HDRSIZE: why "+ 1"? | ACCEPT | the + 1 and roundup() are not necessary. | we'll rename this to WIFI_HDRSIZE and move it mac_wifi.h | since it's only used by the wifi plugin. | - mac_wifi_header_info() makes no attempt to deal with chained mblks. Is there a clear requirement on drivers that they must present the headers in a single block? | EXPLAIN | yes. this is a requirement. | unfortunately this isn't documented yet since gldv3 interfaces | aren't yet public. - mac_wifi_header_uncook(), line 404: mhi_hdrsize >= sizeof (struct ether_header) ? | ACCEPT | I agree it should be '>='. | usr/src/uts/common/sys/dld.h: - how will we increase the size of DLD_SECOBJ_VAL_MAX when that becomes necessary? | EXPLAIN | the ioctls are private so we're free to change them anytime. | for this case, DLD_SECOBJ_VAL_MAX will just be bumped up. | this is not a problem since user libraries are shipped together | with kernel bits. | usr/src/uts/common/io/dld/dld_drv.c: - drv_secobj_init(): is the kmem_cache actually worthwhile? The constructor does nothing but bzero, the kmem_cache_alloc() caller sets all of the fields and the kmem_cache_free() caller doesn't 're-init' the bufer. | EXPLAIN | it's worthwhile because we get a free walker in (k)mdb. | - line 707: prefer "sizeof (*sgp)", given line 710. - line 727: spurious blank. | ACCEPT | This message posted from opensolaris.org _______________________________________________ networking-discuss mailing list [email protected]
