>> By looking up the NV code, I don't think there is a function which can >> convert door_handle_t to a fd. But I don't see any problem if dls >> keeps the fd information if needed. > > But the specific fd passed in only valid for the process which called > the initial ioctl. There would need to be some way to map that to a > descriptor in a different process making the "I want access to this > door" request... > Ah. That is correct. Sorry that I don't know how to solve this problem.
>>> * 206: The description of the "dev" argument is misleading. I believe >>> this must only be non-zero for devices under softmac. Is that the >>> case? >>> >> No. It is non-zero for all physical links. Including of those GLDv3 >> devices. > > Okay, can this be added to the comments? > Sure. >> If you prefer, I can change this argument to "novanity" to indicate it >> means this physical link does not support vanity naming (e.g., DL_IB >> links). > > That's fine too. > Okay. I will make the change. >>> * 547: Checking for the same MAC_CAPAB_PERSTREAM capability on every >>> open doesn't seem optimal, since you'll be getting the same result >>> everytime. The open function to call shouldn't change from call to >>> call, so wouldn't it be better to obtain the open function pointer >>> once, perhaps during mac_register()? MAC_CAPAB_PERSTREAM isn't >>> applicable at all unless MAC_CAPAB_PUTNEXT_TX is also supported, >>> right? If so, it might make sense to simply return the open function >>> as part of MAC_CAPAB_PUTNEXT_TX and get rid of MAC_CAPAB_PERSTREAM. >>> >> the MACs of legacy devices (and only them) would have below capabilities: >> >> MAC_CAPAB_TX_LOOPBACK, MAC_CAPAB_PUTNEXT_TX, MAC_CAPAB_PERSTREAM >> >> Do you prefer me to merge all three together - to be something like >> MAC_CAPAB_LEGACY or such. > > I'd love to see these merged together. May I suggest MAC_CAPAB_SOFTMAC? > That way, it's clear in the mac code that by asking for this > capability, we're asking, "is this the softmac driver"? > Okay. I changed to MAC_CAPAB_LEGACY. As the softmac driver today manages all physical devices, including gldv3 drivers. > 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? >> >>> * 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). >> 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. >>> * 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. Anyway, I see this is a separate issue from UV, and I filed a RFE for it. Thanks - Cathy
