I've updated the webrev ( http://cr.opensolaris.org/~gdamore/hme-mii )
and I've tested the private link properties. (At least I can change
their values with dladm and ndd. I don't have equipment capable of
verifying that the actual physical link timing parameters have been
changed. Hopefully nobody uses these inter-packet gap tunables anyway.)
- Garrett
Garrett D'Amore wrote:
Girish Moodalbail wrote:
On 10/12/09 22:16, Garrett D'Amore wrote:
All,
I've converted hme to the common MII framework (and hence to
Brussels). This fixes at least one bug (it didn't work at forced 10
Mbps!), and cleans up a *a lot* of code. hme shrinks by about 2200
lines.
The webrev is here:
http://cr.opensolaris.org/~gdamore/hme-mii
Hello Garrett,
I have looked at only Brussels interfaces and here are my comments:
hme.c:
L123-L128: Since all the 4 private properties are per-interface, the
comments above those lines are incorrect. They talk about making
these parameters per-interface (which is already per-interface) and
using ndd command.
Good catch. I"m just remove the comment.
L130-133: Do we really need these to be global variables? They are
used only for initiation and so can be represented as macros right.
They could be macros, but making them globals also allows them to be
adjusted via /etc/system. I'm not going to change them for now,
because there simply isn't any compelling need to do so.
L1917: rv should be reset to 0. otherwise when everything is fine we
will return (ENOTSUP) or else @L1950 return (ENOTSUP) and L1956
return (0)
Accept. I guess I didn't try changing these parameters. Need to
cover that with more testing.
L1932: There is a missing 'else' clause.
Accept, good catch.
L72, L90, L127, L269: NIT: no need for ','
Yeah, but C is ok with it either way. I like to leave the last , in
place, so if another member gets added to the end, you don't have to
remember to add one without creating a syntax error. As this is
mostly a stylistic preference, I'm going to leave it alone for now.
Thanks for reviewing this stuff.
- Garrett
~Girish
_______________________________________________
networking-discuss mailing list
[email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
