Eric Cheng wrote: > Hi Cathy, > > I managed to review the nemo parts of clearview in the last few days > (still ongoing). I think the code looks good overall; but I am slightly > uncomfortable with the way per-stream support is implemented. The > changes seems to spread across many places in nemo (dld, dls, mac).
I agree. It is a painful part of the fast-path. > In particular, there are a few things that bothers me: > > -the splitting of dlpi processing into two halves > I understand why this is done but I wonder if it's really necessary > to allow every dlpi primitive to bypass dls. I'll touch on this more > later. > > -the adding of the 'set' boolean to various functions to indicate to > dls/mac to skip some processing if per-stream is enabled. The 'set' > argument has no use to anything other than softmac so adding this to > mac interfaces doesn't seem right to me (those aren't public yet > but it's best to keep them simple). > What I could do is to do what VNIC does for mac_active_set() and mac_active_shareable_set(), that is to add another function (e.g., mac_promisc_perstream_set()) which share the same function with the current mac_promisc_set(). > To address the above issues and perhaps simplify per-stream support > further, I would like to propose a slight modification to the current > design: > > 1. softmac always opens a shared lower stream (smac_lower), even if > per-stream is enabled. this lower stream is used for all non data > dlpi messages. dld_proto.c should not need any changes except for > the data path and DL_BIND_REQ. this stream does not need to be put > in DL_PROMISC_SAP mode unless necessitated by certain functionality > (vlans..etc). > > 2. the data path stream does not need to be opened until dls_bind() > is called. dls_bind() would call a function that does: > a. perstream_capab.mpc_open() > b. send DL_BIND_REQ to lower data path stream. > b could possibly be placed inside a. > on return from this function, dld_str_t (or dls_impl_t) should have > whatever info necessary to access the lower data path stream. > > 3. the softmac's mi_unicst/mi_promisc/mi_multcst entry points will > generate DLPI messages to the shared lower stream. within dls, all > data structures remain as before (dls_impl_t still get added to > dl_impl_hash..etc). > That is basically how shared-lower-stream scheme is implemented. It sounds like all the data messages would come from shared lower stream? How that can be fast-path? (Note that for fast-path, the mac/dld/dls layer is almost skipped in the data-path). > 4. when promisc mode is turned on, packets will get delivered to both > the data path stream (bound to a particular sap) and the shared lower > stream. to prevent getting duplicate data, i_dls_link_rx_promisc() > could be made to pass packets only to dls_impl_t's bound to > DLS_PROMISC_SAP and drop everything else when there exists a separate > lower data path stream. > I don't think I get your idea. Why the promisc mode on the per-stream stream is also turned on? How about those multicst/broadcst packets? Do they come up from both streams too? My current implementation basically follows the principle that shared stream and per-stream stream are separated and serve for different purpose. So they are independent from each other. Your proposal sounds more complicated than that. But maybe I mis-understood your idea. Thanks - Cathy > I cannot claim to understand all the implementation issues of softmac > yet so there might be reasons why my proposal is not feasible. Please > let me know your opinion. > > eric > >
