On Mon, 15 Feb 2010 08:04:51 -0800
Renee Danson Sommerfeld <renee.sommerfeld at sun.com> wrote:
> On Sun, Feb 14, 2010 at 04:21:39PM -0800, Michael Hunter wrote:
> > I need CR for the following two bugs.
> >
> > 14543 - nwam spams log about links in an aggregation
> > 14586 - aggregations are not brought up
> >
> > The webrev is at /net/kodiak.eng/builds/mph/nwam1_bug
>
> Looks good; just a couple questions about comments in nuc.c:
>
> * line 227: Why this change? Is it the case that we *do not* get events
> for static addresses?
No, the code is like:
Comment
send object set state
if necessary plumb IPv4 i/f
if necessary plumb IPv6 i/f
configure static addresses
I going to return the comment to static since it also says autoconf. I
assumed that the comment was saying we have to to this now instead of
after the plumbs 'cuz the plumbs cause all this stuff to happen which
we can't filter out. The static case seems more controlled to me (we
could do the set state right before the set or just before the first
one). But as I read it I think the comment cut a wider swath.
>
> * in ncu_create_link_action_event() (lines 1391-1403): the three
> previously existing checks for links that should be skipped each
> had a little note about what they were checking for; the one you
> added does not. I think the notes are somewhat unnecessary, as
> it's fairly obvious what we're skipping in each case (except maybe
> the last one, where we're making sure the NCU does not already
> exist), so I can see why you didn't add one; but the lack of
> consistency also bugs me. How about one note at the top (1391)
> along the lines of "We explicitly do not manage virtualbox
> interfaces, links belonging to other zones, and links that are
> part of an aggr, so skip those. Can also skip NCUs that already
> exist."
I added one in parallel and couldn't keep myself from fixing the grammar.
I'll leave a new webrev in place but am pushing...
Michael
>
> Thanks!
> renee
