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).
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).
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).
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 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
