Re: Panic in netnatm

On Wed, 27 Jul 2005, Craig Rodrigues wrote:

> On Wed, Jul 27, 2005 at 10:35:34PM +0100, Robert Watson wrote:
>> Otherwise, this looks good to me!  (And I guess neither Harti or Bruce got
>> a chance to test these code paths?)
>
> Harti is on vacation until the end of August so I received no feedback 
> from him. I updated my patch based on your comments.  What do you think?

I think you still need an NATM_UNLOCK() in the 'goto failed' case, 
probably placed outside of the "if (npcb) {' block since it will need to 
be unlocked in both cases.  I.e., this additional block:

_at__at_ -256,6 +258,7 _at__at_
                         rt->rt_llinfo = NULL;
                         rt->rt_flags &= ~RTF_LLINFO;
                 }
+               NATM_UNLOCK();
  #endif
                 /* mark as invalid. We cannot RTM_DELETE the route from
                  * here, because the recursive call to rtrequest1 does

(will need to be hand-applied due to xterm copy+paste).

Robert N M Watson
>
>
> --- if_atm.c.orig	Tue Jul 26 10:28:42 2005
> +++ if_atm.c	Wed Jul 27 17:44:45 2005
> _at__at_ -221,6 +221,7 _at__at_
> 		 * let native ATM know we are using this VCI/VPI
> 		 * (i.e. reserve it)
> 		 */
> +		NATM_LOCK();
> 		sin = (struct sockaddr_in *) rt_key(rt);
> 		if (sin->sin_family != AF_INET)
> 			goto failed;
> _at__at_ -256,6 +257,7 _at__at_
> 			rt->rt_llinfo = NULL;
> 			rt->rt_flags &= ~RTF_LLINFO;
> 		}
> +		NATM_UNLOCK();
> #endif
> 		/* mark as invalid. We cannot RTM_DELETE the route from
> 		 * here, because the recursive call to rtrequest1 does
> _at__at_ -269,10 +271,12 _at__at_
> 		 * tell native ATM we are done with this VC
> 		 */
> 		if (rt->rt_flags & RTF_LLINFO) {
> +			NATM_LOCK();
> 			npcb_free((struct natmpcb *)rt->rt_llinfo,
> 			    NPCB_DESTROY);
> 			rt->rt_llinfo = NULL;
> 			rt->rt_flags &= ~RTF_LLINFO;
> +			NATM_UNLOCK();
> 		}
> #endif
> 		/*
>
>
> -- 
> Craig Rodrigues
> rodrigc_at_crodrigues.org
>