Cathy Zhou wrote: >> On a related note, since this capability is a private interface >> between mac and softmac, you could define the capability's value in >> mac.h, but separate it from the other capabilities in the capability >> number space. In other words, create some high-order bit in the enum >> that's reserved for private capabilities and put all of the private >> capabilities in that number space with a glaring comment stating >> "these are private to the softmac module" so that driver developers >> know to ignore them. >> > I understand of the need for a comment. But I am not sure about the > high-order bit part. Who would consume this bit? To determine what?
It would not be explicitly consumed by anything. It's just a suggestion to visibly separate the (to be) public capabilities from those which are private to the softmac driver in the header file. For example, this would be similar to how the DL_* MAC-Types are separated in sys/dlpi.h. > >>> >>>> * 973: I don't understand the "set" argument. Some intro. comment is >>>> needed to explain it, or alternatively, it should be removed in favor >>>> of some way for the mac module to determine the semantics based on >>>> the >>>> state of the mac_impl_t. Snaking up the call chain, I cannot >>>> determine any logic to when B_TRUE is passed in or B_FALSE is passed >>>> in. >>>> >>> We cannot remove it. >>> >>> Because whether "set" is true of false relies on whether the caller >>> is a per-stream type DLS clients. >> >> But doesn't the mac module know that all DLS clients are per-stream >> once it gets an answer to the MAC_CAPAB_PERSTREAM capability request? >> > No, the mac module won't know. Because there might be some DLS clients > or MAC clients that cannot use the per-stream scheme (for example, a > VLAN stream on a VLAN-incapable device, or an aggregation). Okay. > >>> Note that the per-stream type DLS client would have an single lower >>> stream associated to it, and the promiscuous mode would be directly >>> through that lower stream. In that case, "set" is set to false, and >>> mac_promisc_set() is not responsible to set the promiscuous mode of >>> the underlying device (through the shared lower stream), neither it >>> causes the update of the MAC's tx callback pointer (mi_txinfo and >>> mi_txloopinfo). The only thing mac_promisc_set() does is to see >>> whether it needs to call i_mac_notify(mip, MAC_NOTE_DEVPROMISC). >> >> Are you considering splitting off the promiscuous functionality based >> on Eric's comments as well? >> > The discussion is still going on. But I wouldn't think that is a simple > change, so I don't think I will reflect that change in my first > integration. Okay. > >>>> * 427: Is it really necessary to restrict the use of softmac in this >>>> way? If some 3rd party wants to provide a DL_FDDI MAC-Type plugin, >>>> for example, could softmac work with legacy DLPI FDDI drivers if this >>>> check didn't exist? >>>> >>> How to determine the m_type_ident of the mac_register_t? If that can >>> be derived from the media type, we can remove this check. >> >> Great question. There is no interface to do this today, but we could >> create one. We could have an additional hash table for MAC-Type >> plugins hashed on media type, and provide a simply mac module function >> to lookup based on media type and return the "ident" of the plugin >> providing support for that media type. >> > Why cannot make i_mactype_getplugin() to take media type as the argument > instead, and make this as a public API. The reason I didn't do that is because there may be more than one plugin for a given "native" MAC-Type (maybe a 3rd party wishes to implement another Ethernet plugin for DL_ETHER that does something interesting given plugin data for example.) The drivers select which plugin they wish to use using the "ident". Of course this points out that softmac wouldn't be able to disambiguate between MAC-Type plugins either... > > Anyway, I see this is a separate issue from UV, and I filed a RFE for it. That's fine, let's discuss this in the context of implementing that RFE. I'm fine with the code as-is for the UV putback. -Seb
