On Wed, 2008-10-29 at 10:50 -0400, Anurag S. Maskey wrote:
> 
> Sebastien Roy wrote:
> > On Mon, 2008-10-27 at 10:17 -0400, Anurag S. Maskey wrote:
> >   
> >> I am requesting code review comments for CR 6745288 
> >> (http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6745288).  
> >> Bugster will be updated with status and evaluation comments after the 
> >> code review.
> >>
> >> The webrev is at:
> >>         http://cr.opensolaris.org/~anurag_m/onnv-bug-6745288/
> >>     
> >
> > I haven't reviewed this in depth, but I did have one question while
> > browsing the overall changes.  There are many operations that either
> > operate on the door fd (operations on persistent configuration) or
> > operate on the dld control fd (operations on temporary configuration).
> > Given that such cases exist where one or the other is never used, I
> > don't see the value in always opening both in dladm_open().  Why not
> > always open the required fd on-demand?
> >   
> A "truss -t open" with dladm shows that in a majority of cases both fd's 
> are opened (many times) regardless of whether the persistent or 
> temporary configuration is requested.  Thus, I went with opening both 
> fd's when the handle is opened. 

Okay, that's good information.  Given that you _always_ try and open the
door fd on demand in dladm_door_cal(), then I don't see why you bother
to try and open it in dladm_open().  That code can just be removed I
think.

> In the worst case, if only one of the fd's is required, one extra fd is 
> opened, which is still better than opening the same fd multiple times as 
> libdladm currently does.

Yes, that much is clear.

Thanks,
-Seb


_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to