crossbow discuss - Oct 2008 - Crossbow Code Review Phase III ready

(networking-discuss Bcc''ed)

Good morning,

The Crossbow team would like to invite you to participate in the
project''s
third and last phase of code review.

The third phase of the review starts today, and will last for
two weeks. It covers the following
parts of the code:


        VLANs
        Link Aggregations
        Xen
        mac_datapath_setup 
        All drivers (excluding e1000g)


The webrev is available in
http://www.opensolaris.org/os/project/crossbow/Code-review3

Interested? please send the code review comments by October 24th, 2008

Thanks,

 The Crossbow team.

Good morning, Kais,

I have a quick question on some MAC header parsing code in mac_sched.c, 
which I happened to look at today. In mac_rs_srs_fanout (and 
mac_rx_srs_proto_fanout) we parse the MAC header and then bump up the 
b_rptr to IP header, in the entire processing we assume that the packets 
are Ethernet packets, but we have more than one type of MAC plugins in 
Solaris today, which get me confused here. It looks incorrect to assume 
packets to be Ethernet type in generic MAC code such as packet 
scheduling, or am I missing anything?

Thanks,
Zhijun

Kais Belgaied wrote:
> (networking-discuss Bcc''ed)
>
> Good morning,
>
> The Crossbow team would like to invite you to participate in the
project''s
> third and last phase of code review.
>
> The third phase of the review starts today, and will last for
> two weeks. It covers the following
> parts of the code:
>
>
>         VLANs
>         Link Aggregations
>         Xen
>         mac_datapath_setup 
>         All drivers (excluding e1000g)
>
>
> The webrev is available in
> http://www.opensolaris.org/os/project/crossbow/Code-review3
>
> Interested? please send the code review comments by October 24th, 2008
>
> Thanks,
>
>  The Crossbow team.
>
>
> _______________________________________________
> crossbow-discuss mailing list
> crossbow-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
>

On 10/09/08 08:53, Zhijun Fu wrote:
> Good morning, Kais,
>
> I have a quick question on some MAC header parsing code in 
> mac_sched.c, which I happened to look at today. In mac_rs_srs_fanout 
> (and mac_rx_srs_proto_fanout) we parse the MAC header and then bump up 
> the b_rptr to IP header, in the entire processing we assume that the 
> packets are Ethernet packets, but we have more than one type of MAC 
> plugins in Solaris today, which get me confused here. It looks 
> incorrect to assume packets to be Ethernet type in generic MAC code 
> such as packet scheduling, or am I missing anything?
Hi Zhijun,

the code is supposed to check the MAC plugin type before assuming 
whether the packet is Ethernet or not.
I''ll need to double check though.

    Kais.
>
> Thanks,
> Zhijun
>
>

I''ve reviewed the igb and ixgbe driver, One general comment for these 
two drivers:

If the MSI-X vectors limitation per device is 2, same as that of ONNV.
These two  driver can not work well with MSI-X. For igb, it will
downgrade to use MSI instead. ixgbe will share 1 MSI-X vector for 1 rx
and 1 tx ring, the other 1 MSI-X vector will be used for link change.

Thanks,
Samuel

Kais Belgaied wrote:
> (networking-discuss Bcc''ed)
> 
> Good morning,
> 
> The Crossbow team would like to invite you to participate in the
project''s
> third and last phase of code review.
> 
> The third phase of the review starts today, and will last for
> two weeks. It covers the following
> parts of the code:
> 
> 
>        VLANs
>        Link Aggregations
>        Xen
>        mac_datapath_setup        All drivers (excluding e1000g)
> 
> 
> The webrev is available in
> http://www.opensolaris.org/os/project/crossbow/Code-review3
> 
> Interested? please send the code review comments by October 24th, 2008
> 
> Thanks,
> 
> The Crossbow team.
> 
>

Thanks for the review Samuel,

we''ll look at the suggested change and get back to you.

    Kais.

On 10/27/08 07:40, Samuel Tu wrote:
> I''ve reviewed the igb and ixgbe driver, One general comment for
these
> two drivers:
>
> If the MSI-X vectors limitation per device is 2, same as that of ONNV.
> These two  driver can not work well with MSI-X. For igb, it will
> downgrade to use MSI instead. ixgbe will share 1 MSI-X vector for 1 rx
> and 1 tx ring, the other 1 MSI-X vector will be used for link change.
>
> Thanks,
> Samuel
>
> Kais Belgaied wrote:
>

Slightly belated, but there''s nothing earth-shattering in there:

On Thu, 2008-10-09 at 04:41 -0700, Kais Belgaied wrote:
>         Link Aggregations
General:

* It would have been nice to have had a diff against something newer
than build 94, as many of the changes were

aggr_grp.c:

* 199-231: I don''t see what any of this has to do with ports, other
than
that code that happens to manage ports calls it.  This is a group-wide
reference scheme, and the _port_ namespace of the functions and
structure members involved confuses the issue, I think.

* 1039,1253,1395: This seems like a layer violation, in that
mac_perim_enter*() is a MAC _client_ function, but aggr is entering the
perimeter of the group MAC, for which it is the driver.  It looks like
the perimeter mechanism here is just being used in place of an internal
aggr control lock, where a private locking mechanism would be more
correct.  Nothing at the MAC layer is being synchronized, but rather
it''s the internal aggr operations that are being synchronized.


aggr_port.c:

* General: Similar to my comment in aggr_grp.c, there are a bunch of
calls like ASSERT(MAC_PERIM_HELD(grp->lg_mh)).  Isn''t the management
of
the MAC perimeters the job of the mac module and the mac clients?  I
don''t think that it''s in the spirit of the new MAC client API
to have
drivers poking around above them to make sure that the mac module is
doing its job...

* 117: This comment claims that this needs to be fixed as part of
Crossbow.

* 132-149: Comments please, why do we need special the special reference
holding routines just for notifications?  Isn''t this simply a generic
aggr_port_refhold() and aggr_port_refrele() scheme that happens to only
be used by the notification code?

* 174-178: This comment states, "we should be doing
<something>", but
the code does <something else>.  Why, and should this be fixed?

* 206,208: Should this handle failure?

* 210: I question the necessity to use link names as a MAC client
identifier.  If the MAC client name is simply meant to be a unique
identifier used for debugging, then why not simply use "aggr"
"instance"?  The aggr module isn''t going to open the same
port more than
once, right?

* 315-321: I don''t understand what this verbose comment about mac
perimeters and deadlocks has to do with the function call below it.  Is
this a block comment that belongs at the top of the definition for
aggr_port_wait_timer_refheld()?


aggr_recv.c:

* 73-87: This XXX needs to be resolved.


-Seb

On Mon, 2008-10-27 at 17:27 -0400, Sebastien Roy wrote:
> Slightly belated, but there''s nothing earth-shattering in there:
> 
> On Thu, 2008-10-09 at 04:41 -0700, Kais Belgaied wrote:
> >         Link Aggregations
> 
> General:
> 
> * It would have been nice to have had a diff against something newer
> than build 94, as many of the changes were
(didn''t finish this sentence) ... included in the fine-grained
privilges
wad.

-Seb

Thanks Seb for the review,

we''re going through all code review comments and shall be sending the 
answers soon.

   Kais.

On 10/27/08 14:33, Sebastien Roy wrote:
> On Mon, 2008-10-27 at 17:27 -0400, Sebastien Roy wrote:
>   
>> Slightly belated, but there''s nothing earth-shattering in
there:
>>
>> On Thu, 2008-10-09 at 04:41 -0700, Kais Belgaied wrote:
>>     
>>>         Link Aggregations
>>>       
>> General:
>>
>> * It would have been nice to have had a diff against something newer
>> than build 94, as many of the changes were
>>     
>
> (didn''t finish this sentence) ... included in the fine-grained
privilges
> wad.
>
> -Seb
>
>
> _______________________________________________
> crossbow-discuss mailing list
> crossbow-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
>
>   
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/crossbow-discuss/attachments/20081027/646f22c2/attachment.html>

Hi Huq,

thanks for the review,

we''ll send the replies to the whole set in the next few days.

    Kais

On 10/28/08 00:02, zeeshanul huq - Sun Microsystems - Beijing China
wrote:
> Hi Kais,
>
> Here is my code review comments of bge driver:
>
> In general, I don''t think bge driver properly match the crossbow
design:
> Reason 1: Only BCM5700 ~ BCM5704 support multiple rx rings, other 
> chipsets only support single ring.
> Reason 2: Those rings are designed in hardware for different priority 
> level.
> Reason 3: It support one interrupt only.
>
> Comment 1. "bge_send.c" line 561:
> srp->pushed_bytes += MBLKL(mp);
> ->
> srp->pushed_bytes += msgdsize(mp);
>
> Comment 2. MAC_ADDRESS_REGS_MAX can be extended to 16 for BCM5702, 
> BCM5703, BCM5704. Therefore we may increase multiple rx ring number up to
> RECV_RULES_NUM_MAX/2 = 8, which currently is 4. (BCM5703 and BCM5704 
> are general chipsets integrated in Sun platforms, e.g. 
> v20/v40/v210/v240/T1000)
>
> Regards,
> Zeeshanul Huq

Here''s what I have so far on mac_datapath_setup.c

ND0001 (E) -- in general all references to "NIC or VNICs" should be  
replaced by "MAC clients".

ND0002/93-95 (E) -- should have an RFE instead of this comment. It''s  
refering to old variables which no longer exist anyway.

ND0003/91 (E) -- should say what happens when all rings are allocated,  
i.e. sharing a common TX ring.

ND00041/98 (T) -- should be a boolean_t

ND0004/106 (E) -- should remove the reference to dladm, since this  
might be specified by other components in the future.

ND0005/124 (T) -- s/mac_soft_ring_count/mac_rx_ring_count/g to be  
consistent with mac_tx_ring_count

ND0006/138 (E) -- get rid of the extra tabs here

ND0007/141 (E) -- where''s the "add" part?

ND0008/144 (T) -- this is called only from mac_tx_srs_del_ring(), why  
does it need to be a macro?

ND0010/186 (T) -- shouldn''t we also unregister the cpu callback here  
also?

ND0011/208 (T) -- we''re doing this casting everywhere in mac. Since  
MAC_PERIM_HELD() is a macro which should be invoked only from mac, we  
should make it take a mac_impl_t do the cast itself.

ND0016/204 (E) -- add a general comment describing what the SRS global  
list is used.

ND0017/236 (T) -- shouldn''t this be an ASSERT()? If not add a comment  
describing the case where this function can be called for an SRS which  
is not in the global list

ND0018/313 (T) -- mac_srs_client_poll_enable() should be declared  
static and its declaration removed from mac_impl.h

ND0019 (T) -- srs_type is a misnomer since it is used to store misc  
SRS state.

ND0020 (T)/339 -- mac_srs_client_poll_disable() modifies srs_type  
under the SRS lock, mac_srs_client_poll_enable() doesn''t.

ND0021 (T)/433-475 -- move mac_client_poll_{enable,disable}() to  
mac_client.c since it''s invoked by MAC clients.

ND0022 (E)/484 -- which "yet another thread" and which
"underlying
capability"? This comment confusing.

ND0023 (T)/489 -- what are the locking expectations here? is the SRS  
lock expected to be held? Something else? Should have corresponding  
ASSERT()s

ND0024 (E)/552 -- s/bw/bandwidth limit/

ND0025 (E)/553 -- and not the bandwidth limit?

ND0026 (E)/559 -- s/dladm-settable/data-link/

ND0027 (T)/577-578 -- this could be removed along with bw_enabled

ND0028 (T)/586 -- why the division since we''re only used this value in
the comparison at the following line?

ND0029 (E)/630 -- remove the ''*'' mid-line

ND0030 (T)/696 -- it would be nice to describe in a comment the impact  
of the mac_latency_optimize knob for the CPU allocation.

ND0031 (T)/708-710 -- we should have a P4 RFE for this

ND0032 (E)/716 -- It would be nice to add a short comment here on how  
the CPUs will ultimately get assigned to the soft rings and the  
various SRS when processing their REINIT state.

ND0033 (E)/710 -- ... but only when the CPUs are specified by the cpus  
property.

ND0012/758 (E) -- the whole file is already called  
mac_datapath_setup(), so we can remove this comment (and NO SHOUTING)

ND0013/773 (E) -- move to beginning of the file

ND0014/776, 807 (T) -- use "id" to refer to the processor id, not
"bind"

ND0015/776 (T) -- the return value of mac_srs_worker_bind() is igned  
by all callers. My comments are (1) is this really the right thing to  
do? I.e. what is the impact of this function failing and the failures  
ignored, and (2) if ignoring the return values is the right thing to  
do, this should be made a void function.

ND0034 (T) -- srs_worker_bind should be renamed srs_worker_cpuid

ND0035 (T)/773 -- shouldn''t this be a boolean_t? Also update the  
checks at 783, etc accordingly

ND0036 (T)/838 -- it seems some SRS locking is needed to walk the soft  
ring list

ND0037 (T)/883 -- again, this function walks the list of soft rings,  
either assert that the appropriate locks are held or take them here

ND0038 (E)/912 -- "etc."?

ND0039 (E)/921 -- Move 921 after 922, 936 after 938

ND0040 (T)/954, 986, 1013 -- Cleanup comment or fix the code

ND0042 (E)/1023 -- s/Bandwidth/bandwidth

ND0043 (T)/1035 -- not needed

ND0044 (E)/1121 -- add general comments describing this function

ND0045 (T)/1129-1135 -- this block of code can be removed

ND0046 (T)/1156-1161 -- can''t this be done at the same time as the  
first loop at 1129-1135

ND0047 (E)/1207 -- refering to non-existing poll_pkt_cnt

ND0048 (T) -- the NEXT_BIND_CPU() macro should be converted into a  
function, and the srs_bind_cpu and srs_curr_cpu can then be moved to  
statics within that function. We can then avoid having the static  
declarations of srs_*_cpu global variables and the corresponding  
declarations in mac.h (which should have been in mac_impl.h in the  
first place anyway)

ND0049 (T)/1244 -- SRS_WORKER_PROCESS is no longer there, what are the  
valid values? Also if srs_process_flag is still useful, I''d suggest  
renaming it to srs_process_default_flags and moving it to the  
beginning of the file.

ND0050 (T)/1251 -- Move with other statics to beginning of file

ND0051 (E)/1254 -- Remove

ND0052 (E)/1255 -- The comment should more specifically say that the  
CPUs are associated with the specified client, not just all traffic  
for the whole NIC

ND0053 (T)/1288,1290 -- the strings_increased and strings_decreased  
are not useful. Why not simply do the comparison later?

ND0054 (T)/1279 -- shouldn''t the SRS fanout state be modified while  
holding the SRS lock?

ND0055 (T)/1318-1319 -- should be a ''else if''

ND0056 (E)/1357 -- add some comments about sharing the CPUs between RX  
and TX threads, and its impact

ND0057 (T)/1450-1466 -- duplicate of 1361-1377, refactor

ND0058 (E)/1471 -- cleanup

ND0059 (E)/1525 -- s/initialize/initializing/

ND0060 (T)/1554-1560 -- or more simply

	soft_ring_count = mac_compute_soft_ring_count(flent,
	    rx_srs_cnt > 1 ? rx_srs_cnt - 1, rx_srs_cnt);




On Oct 9, 2008, at 4:41 AM, Kais Belgaied wrote:
> (networking-discuss Bcc''ed)
>
> Good morning,
>
> The Crossbow team would like to invite you to participate in the  
> project''s
> third and last phase of code review.
>
> The third phase of the review starts today, and will last for
> two weeks. It covers the following
> parts of the code:
>
>
>        VLANs
>        Link Aggregations
>        Xen
>        mac_datapath_setup
>        All drivers (excluding e1000g)
>
>
> The webrev is available in
> http://www.opensolaris.org/os/project/crossbow/Code-review3
>
> Interested? please send the code review comments by October 24th, 2008
>
> Thanks,
>
> The Crossbow team.
>
>
> _______________________________________________
> crossbow-discuss mailing list
> crossbow-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
-- 
Nicolas Droux - Solaris Networking - Sun Microsystems, Inc.
nicolas.droux at sun.com - http://blogs.sun.com/droux

Here is the rest of my comments on mac_datapath_setup.c.

ND0061 (E)/1594-1595: remove (also applies to other functions in this  
file)

ND0062 (E)/1604-1608: this seems to be an xxx comment, clean it up

ND0063 (T)/1630, 1638-1639: what are the locking assumptions to update  
fe_rx_srs and fe_rx_srs_cnt here?

ND0064 (E)/1737: should add a comment there stating that the caller is  
responsible to update the classification type according to the state  
of the group after all SRS''s have been created.

ND0065 (T): FLOW_VNIC is a misnomer since it is also used for other  
non-primary MAC clients such as the LDOM vsw. Suggest a global search- 
and-replace with a more appropriate name, e.g. FLOW_NON_PRIMARY_MAC.

ND0066 (E)/1750: the comment of this function talks about computing  
the number of soft rings, but the function actually returns the fanout  
type.

ND0067 (E)/2064: typo "remianing''

ND0068 (E)/2080-2090: this comment should be updated since it still  
refers to hardware rings assigned to different protocols, and hardware  
ring polling which we don''t do.

ND0069 (E)/2138: "non primary"/"non-primary"

ND0070 (E)/2141: most of this is really talking about clients, not  
links. There are MAC clients which may not be directly associated with  
data-links, such as the LDOM vsw.

ND0071 (E)/2287-2297: fix todo comment

ND0072 (E)/2482: remove extra blank spaces

ND0073 (T)/2578-... & in general: use ''(val & flag) !=
0'' and ''(val &
flag) == 0'' consistently.

ND0074 (E)/2669: mac_rx_classify_flow_state_change() no longer exists,  
comment needs a refresh

ND0075 (E)/2821-2825: this comment can be removed

ND0076 (E)/2921: s/will be hang/will hang/


On Oct 30, 2008, at 4:07 PM, Nicolas Droux wrote:
> Here''s what I have so far on mac_datapath_setup.c
>
> ND0001 (E) -- in general all references to "NIC or VNICs" should
be
> replaced by "MAC clients".
>
> ND0002/93-95 (E) -- should have an RFE instead of this comment.
It''s
> refering to old variables which no longer exist anyway.
>
> ND0003/91 (E) -- should say what happens when all rings are allocated,
> i.e. sharing a common TX ring.
>
> ND00041/98 (T) -- should be a boolean_t
>
> ND0004/106 (E) -- should remove the reference to dladm, since this
> might be specified by other components in the future.
>
> ND0005/124 (T) -- s/mac_soft_ring_count/mac_rx_ring_count/g to be
> consistent with mac_tx_ring_count
>
> ND0006/138 (E) -- get rid of the extra tabs here
>
> ND0007/141 (E) -- where''s the "add" part?
>
> ND0008/144 (T) -- this is called only from mac_tx_srs_del_ring(), why
> does it need to be a macro?
>
> ND0010/186 (T) -- shouldn''t we also unregister the cpu callback
here
> also?
>
> ND0011/208 (T) -- we''re doing this casting everywhere in mac.
Since
> MAC_PERIM_HELD() is a macro which should be invoked only from mac, we
> should make it take a mac_impl_t do the cast itself.
>
> ND0016/204 (E) -- add a general comment describing what the SRS global
> list is used.
>
> ND0017/236 (T) -- shouldn''t this be an ASSERT()? If not add a
comment
> describing the case where this function can be called for an SRS which
> is not in the global list
>
> ND0018/313 (T) -- mac_srs_client_poll_enable() should be declared
> static and its declaration removed from mac_impl.h
>
> ND0019 (T) -- srs_type is a misnomer since it is used to store misc
> SRS state.
>
> ND0020 (T)/339 -- mac_srs_client_poll_disable() modifies srs_type
> under the SRS lock, mac_srs_client_poll_enable() doesn''t.
>
> ND0021 (T)/433-475 -- move mac_client_poll_{enable,disable}() to
> mac_client.c since it''s invoked by MAC clients.
>
> ND0022 (E)/484 -- which "yet another thread" and which
"underlying
> capability"? This comment confusing.
>
> ND0023 (T)/489 -- what are the locking expectations here? is the SRS
> lock expected to be held? Something else? Should have corresponding
> ASSERT()s
>
> ND0024 (E)/552 -- s/bw/bandwidth limit/
>
> ND0025 (E)/553 -- and not the bandwidth limit?
>
> ND0026 (E)/559 -- s/dladm-settable/data-link/
>
> ND0027 (T)/577-578 -- this could be removed along with bw_enabled
>
> ND0028 (T)/586 -- why the division since we''re only used this
value in
> the comparison at the following line?
>
> ND0029 (E)/630 -- remove the ''*'' mid-line
>
> ND0030 (T)/696 -- it would be nice to describe in a comment the impact
> of the mac_latency_optimize knob for the CPU allocation.
>
> ND0031 (T)/708-710 -- we should have a P4 RFE for this
>
> ND0032 (E)/716 -- It would be nice to add a short comment here on how
> the CPUs will ultimately get assigned to the soft rings and the
> various SRS when processing their REINIT state.
>
> ND0033 (E)/710 -- ... but only when the CPUs are specified by the cpus
> property.
>
> ND0012/758 (E) -- the whole file is already called
> mac_datapath_setup(), so we can remove this comment (and NO SHOUTING)
>
> ND0013/773 (E) -- move to beginning of the file
>
> ND0014/776, 807 (T) -- use "id" to refer to the processor id, not
> "bind"
>
> ND0015/776 (T) -- the return value of mac_srs_worker_bind() is igned
> by all callers. My comments are (1) is this really the right thing to
> do? I.e. what is the impact of this function failing and the failures
> ignored, and (2) if ignoring the return values is the right thing to
> do, this should be made a void function.
>
> ND0034 (T) -- srs_worker_bind should be renamed srs_worker_cpuid
>
> ND0035 (T)/773 -- shouldn''t this be a boolean_t? Also update the
> checks at 783, etc accordingly
>
> ND0036 (T)/838 -- it seems some SRS locking is needed to walk the soft
> ring list
>
> ND0037 (T)/883 -- again, this function walks the list of soft rings,
> either assert that the appropriate locks are held or take them here
>
> ND0038 (E)/912 -- "etc."?
>
> ND0039 (E)/921 -- Move 921 after 922, 936 after 938
>
> ND0040 (T)/954, 986, 1013 -- Cleanup comment or fix the code
>
> ND0042 (E)/1023 -- s/Bandwidth/bandwidth
>
> ND0043 (T)/1035 -- not needed
>
> ND0044 (E)/1121 -- add general comments describing this function
>
> ND0045 (T)/1129-1135 -- this block of code can be removed
>
> ND0046 (T)/1156-1161 -- can''t this be done at the same time as the
> first loop at 1129-1135
>
> ND0047 (E)/1207 -- refering to non-existing poll_pkt_cnt
>
> ND0048 (T) -- the NEXT_BIND_CPU() macro should be converted into a
> function, and the srs_bind_cpu and srs_curr_cpu can then be moved to
> statics within that function. We can then avoid having the static
> declarations of srs_*_cpu global variables and the corresponding
> declarations in mac.h (which should have been in mac_impl.h in the
> first place anyway)
>
> ND0049 (T)/1244 -- SRS_WORKER_PROCESS is no longer there, what are the
> valid values? Also if srs_process_flag is still useful, I''d
suggest
> renaming it to srs_process_default_flags and moving it to the
> beginning of the file.
>
> ND0050 (T)/1251 -- Move with other statics to beginning of file
>
> ND0051 (E)/1254 -- Remove
>
> ND0052 (E)/1255 -- The comment should more specifically say that the
> CPUs are associated with the specified client, not just all traffic
> for the whole NIC
>
> ND0053 (T)/1288,1290 -- the strings_increased and strings_decreased
> are not useful. Why not simply do the comparison later?
>
> ND0054 (T)/1279 -- shouldn''t the SRS fanout state be modified
while
> holding the SRS lock?
>
> ND0055 (T)/1318-1319 -- should be a ''else if''
>
> ND0056 (E)/1357 -- add some comments about sharing the CPUs between RX
> and TX threads, and its impact
>
> ND0057 (T)/1450-1466 -- duplicate of 1361-1377, refactor
>
> ND0058 (E)/1471 -- cleanup
>
> ND0059 (E)/1525 -- s/initialize/initializing/
>
> ND0060 (T)/1554-1560 -- or more simply
>
> 	soft_ring_count = mac_compute_soft_ring_count(flent,
> 	    rx_srs_cnt > 1 ? rx_srs_cnt - 1, rx_srs_cnt);
>
>
>
>
> On Oct 9, 2008, at 4:41 AM, Kais Belgaied wrote:
>
>> (networking-discuss Bcc''ed)
>>
>> Good morning,
>>
>> The Crossbow team would like to invite you to participate in the
>> project''s
>> third and last phase of code review.
>>
>> The third phase of the review starts today, and will last for
>> two weeks. It covers the following
>> parts of the code:
>>
>>
>>       VLANs
>>       Link Aggregations
>>       Xen
>>       mac_datapath_setup
>>       All drivers (excluding e1000g)
>>
>>
>> The webrev is available in
>> http://www.opensolaris.org/os/project/crossbow/Code-review3
>>
>> Interested? please send the code review comments by October 24th,  
>> 2008
>>
>> Thanks,
>>
>> The Crossbow team.
>>
>>
>> _______________________________________________
>> crossbow-discuss mailing list
>> crossbow-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
>
> -- 
> Nicolas Droux - Solaris Networking - Sun Microsystems, Inc.
> nicolas.droux at sun.com - http://blogs.sun.com/droux
>
> _______________________________________________
> crossbow-discuss mailing list
> crossbow-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
-- 
Nicolas Droux - Solaris Kernel Networking - Sun Microsystems, Inc.
nicolas.droux at sun.com - http://blogs.sun.com/droux

Thanks for the review Nicolas,

in the next couple of days we''ll be sending the complete list of
responses.

    Kais