Hello,

On Wed, Oct 18, 2017 at 08:13:09PM +0200, Alexander Bluhm wrote:
> On Wed, Oct 18, 2017 at 10:48:30AM +0200, Alexandr Nedvedicky wrote:
> >     I think you also want to add if_ih_remove() to loop_clone_destroy().
> 
> Yes, thanks for catching that.
> 
> >     I feel it should be called after if_detach().
> 
> I disagree.  Other pseudo-interfaces call it before.  But more
> importantly, if_ih_remove() calls refcnt_finalize() which sleeps
> until all references are gone.  We should not detach the interface
> before doing that.
> 
> correct?

    yes, it makes sense. I was perhaps mistaken by bridge_clone_destroy(),
    which calls if_deactivate(ifp); first:

 224 
 225         /* Undo pseudo-driver changes. */
 226         if_deactivate(ifp);
 227 
 228         if_ih_remove(ifp, ether_input, NULL);
 229 
 230         KASSERT(SRPL_EMPTY_LOCKED(&ifp->if_inputs));
 231    
 232         if_detach(ifp);

    my concern was to call if_ih_remove() _after_ if_deactivate().
    but it looks like it does not matter. your way is consistent with
    mpw_clone_destroy(). Thank you for fixing me. your patch looks good
    to me.

OK sashan


Reply via email to