Cathy,

This is good.  Comments in-line.

Cathy Zhou wrote:
> A reminder ... I've uploaded this doc to the OpenSolaris website:
> 
> http://opensolaris.org/os/project/clearview/docs/softmac_fast_path
...
> Data Fast-Path for Softmacs
> ===========================
> 
> Release binding: minor

If Nevada were to ship today, would we have to wait until the next minor 
release to deliver this fastpath?  I think not, as this change is 
completely backward compatible, and only affects performance.  As such, 
I'd suggest "Patch" binding.  In reality, this will deliver in the same 
minor release as UV itself, but that's somewhat irrelevant.

> Commitment level: project private
> 
> Summary
> =======
> 
>     Due to its incompatibility with an upcoming project which is making
>     major changes in the same area of the kernel, the data fast-path for
>     softmacs was removed from the final delivery of the Clearview UV
>     project (see section 2.1 of [2]), which results in performance
>     regression on legacy network devices.
> 
>     This case proposes an alternate softmac fast-path architecture that
>     has less conflict with other projects.

The summary could be more concise, and leave the rationale of removing 
the fastpath as a detail of PSARC/2008/002.  For example:

"This case introduces a GLDv3 fast-path architecture for softmac 
devices.  The softmac architecture defined by PSARC/2006/499 initially 
contained a fast-path which was not delivered with Clearview UV as 
discussed in section 2.1 of PSARC/2008/002.  PSARC/2008/002 states that 
a future case would define a new fast-path, and that is this case."

> Details
> =======
> 
>     - Data fast-path and slow-path
> 
>       Currently, the softmac module discovers each legacy network device
>       and registers (soft)MACs to the GLDv3 framework on behalf of these
>       legacy devices. Whenever a softMAC is used, the softmac module opens
>       a DLIOCRAW stream over the legacy device, and set the stream to the
>       DL_PROMISC_SAP promiscuous mode.

I'd suggest, "and enables DL_PROMISC_SAP promiscuous mode on the stream."

>       This DLIOCRAW stream is then shared
>       by all data packets sent and received over this softMAC. We call this
>       DLIOCRAW stream "shared lower stream". 
> 
>       Although the above is consistent with the existing GLDv3 model and
>       requires less changes on the GLDv3 framework, it introduces

s/on/to/

>       unacceptable performance regression over legacy devices, because of
>       the extra data demultiplexing and filtering processing in the GLDv3
>       DLS/MAC layer.
> 
>       Therefore, this case proposes a softmac data fast-path architecture
>       which is less intrusive to the current Solaris network architecture
>       and will address the performance issue of the above approach:

The structure of the next few paragraphs has a confusing flow.

Before diving into details, it would help to state the scope of the 
fast-path, and how it works at a very high level.  Without that, the 
rationale for the details that follow isn't clear.  For example, that 
it's a fast-path for ip module data that works by having the ip and 
legacy device modules exchange data messages directly, bypassing the 
usual GLDv3 data-path.  You hint at this in passing a few paragraphs 
down, but I think it needs to be more explicitly stated.

Then you can state that because of this bypass, enabling the use of 
GLDv3 features requires first disabling the fast-path, and that is done 
automatically by the framework.

>       By default, the softmac fast-path mode will be used to assure the
>       performance; If softmac fast-path is disabled to support certain
>       features (for example, when a VNIC is created over a legacy device,
>       softmac fast-path must be disabled to assure the correctness of the
>       VNIC implementation), the system will fallback to the existing softmac
>       data-path model (called slow-path).
> 
>       The details of the new softmac data fast-path model is stated as below
>       (note that the softmac fast-path will only be used for the IP/ARP
>       streams over softMACs, which is the case performance matters most):
> 
>       1. When a stream (including a VLAN stream) is opened on a softMAC,
>          the softmac module will takes over the DLPI processing on this
>          stream;
> 
>       2. The softmac module first identifies an IP/ARP stream by seeing
>          whether there is a SIOCSLIFNAME ioctl sent from upstream, if
>          there is one, this stream is either an IP or an ARP stream
>          and will use fast-path potentially;
> 
>       3. For IP/ARP streams over a softMAC, softmac data fast-path will be
>          used by default, unless fast-path is disabled by any MAC client
>          explicitly;

The separation of 2 and 3 is confusing.  It would be easier to simply 
state that the fast-path is enabled for IP and ARP streams setup by the 
ip module unless a MAC client has explicitly disabled the fast path. 
Then say in the same bullet item that the framework detects IP and ARP 
streams by intercepting SIOCSLIFNAME ioctls.

> 
>       4. When the softmac fast-path is used, an dedicated lower-stream will
>          be setup for each IP/ARP stream (1-1 mapping). From now on, all

s/From now on, all the/From that point on, all/

>          the control and data messages will be exchanged between the
>          IP/ARP upper-stream and the legacy device through this dedicated
>          lower-stream. As a result, the DLS/MAC layer processing in GLDv3
>          will be skipped, and this greatly improves the performance;
> 
>       5. When the softmac data fast-path is disabled by a MAC client (e.g.,
>          by a VNIC), all the IP/ARP upper streams will try to switch from
>          the fast-path to the slow-path. The dedicated lower-stream will be
>          destroyed, and all the control and data-messages will go through
>          the existing GLDv3 code path and (in the end) the shared
>          lower-stream;
> 
>       6. On the other hand, when the last MAC client cancels its fast-path
>          disable request, all the IP/ARP streams will try to switch back to
>          the fast-path mode;
> 
>       Step 5 and 6 both rely on the data-path mode switching process
>       described below:
> 
>          1) To switch the softmac data-path mode (between fast-path and
>             slow-path), softmac will first send a DL_NOTE_REPLUMB
>             DL_NOTIFY_IND message upstream over each IP/ARP streams that
>             needs data-path mode switching;

I think it's confusing to dive into the details of how softmac disables 
or re-enables the fastpath without first explaining what triggers 
softmac to do these things.

> 
>          2) When IP receives this DL_NOTE_REPLUMB message, it will bring
>             down all the IP interfaces on the corresponding ill (IP Lower
>             level structure), and bring up those interfaces over again;
>             this will in turn cause the ARP to "replumb" the interface.
> 
>             During the replumb process, both IP and ARP will send
>             downstream the necessary DL_DISABMULTI_REQ and DL_UNBIND_REQ
>             messages and cleanup the old state of the underlying softMAC,
>             following with the necessary DL_BIND_REQ and DL_ENABMULTI_REQ
>             messages to setup the new state. Between the cleanup and re-setup
>             process, IP/ARP will also sent down a DL_NOTE_REPLUMB_DONE

s/sent/send/

>             DL_NOTIFY_CONF messages to the softMAC to indicate the
>             *switching point*;
> 
>          3) When softmac receives the DL_NOTE_REPLUMB_DONE message, it
>             either creates or destroys the dedicated lower-stream (depending
>             on which data-path mode the softMAC switches to), and change
>             the softmac data-path mode. From then on, softmac will process
>             all the succeeding control messages (including the DL_BIND_REQ and
>             DL_ENABMULTI_REQ messages) and data messages based on new
>             data-path mode.
> 
> Interfaces
> ==========
> 
>     - mac_fastpath_disable()/mac_fastpath_enable() (Project Private)
> 
>       Two MAC client interfaces will be added:
> 
>       int mac_fastpath_disable(mac_handle_t);
> 
>       This function will be called when a mac client requires to disable
>       the softmac data fast-path over a specific mac. It will return errno
>       in case that the fast-path is failed to be disabled. If the given mac
>       is a mac over a GLDv3 device, 0 (success) will be directly returned.
> 
>       void mac_fastpath_enable(mac_handle_t);
> 
>       The mac_fastpath_enable() function will be called when a mac client
>       cancels its softmac fast-path disabling requests over a specific mac.
>       When the last fast-path disabling request is canceled, the underlying
>       softmac will try to reenable the data fast-path. It is not considered
>       as a failure if fast-path fails to be reenabled, as that will only
>       have performance impact.
> 
>     - dld_str_open()/dld_str_close()/dld_str_private() (Project Private)
> 
>       Three new dls interfaces will be added:
> 
>       int dld_str_open(queue_t *, dev_t *, void *opaque);
> 
>       This function will be called by softmac to setup the dld stream
>       related structures, which will be used when softmac fast-path is
>       not used. An opaque pointer will be passed into this function
>       to keep softmac specific state associated with the given stream.
> 
>       int dld_str_close(queue_t *);
> 
>       This function will be called by softmac to finish up the dld stream
>       operation and destroy dld related structures.
> 
>       void *dld_str_private(queue_t *);
> 
>       This function will be called to query the opaque pointer associated
>       with the given stream.
> 
>     - the mac_capab_legacy_t structure (Project Private)
> 
>       The mac_capab_legacy_t structure was introduced by PSARC/2008/002,
>       it is the data pointer of the MAC_CAPAB_LEGACY MAC capability.
> 
>       This case proposes to add the following fields to the data structure:
> 
>       /*
>        * Info and callbacks of legacy devices.
>        */
>       typedef struct mac_capab_legacy_s {
>               ...
>               int             (*ml_fastpath_disable)(void *);
>               void            (*ml_fastpath_enable)(void *);
>               boolean_t       (*ml_active_set)(void *);
>               void            (*ml_active_clear)(void *);
>       } mac_capab_legacy_t;
> 
>       The ml_fastpath_enable() and ml_fastpath_disable() callbacks will be
>       called as the result of first mac_fastpath_disable() and last
>       mac_fastpath_disable() request on a particular softMAC.

Do you mean first mac_fastpath_enable() and last mac_fastpath_disable()?

The rest looks fine.

-Seb

Reply via email to