RELENG_7 crash

[Mikolaj Golub]

On Tue, 21 Apr 2009 18:53:54 +0100 (BST) Robert Watson wrote:

 RW> On Tue, 21 Apr 2009, Mike Tancsa wrote:

 >> At 11:31 AM 4/21/2009, Ruslan Ermilov wrote:
 >>> :
 >>> : Note that these changes simply close races around use of ifindex_table,
 >>> : and make no attempt to solve the probem of disappearing ifnets.  Further
 >>>       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 >>> : refinement of this work, including with respect to ifindex_table
 >>> : resizing, is still required.
 >>> :
 >>> : In a future change, the ifnet lock should be converted from a mutex to an
 >>> : rwlock in order to reduce contention.
 >>
 >>        Thanks for the info!  In the mean, time, apart from disabling
 >> snmpwalking, is there anything I can do to mitigate triggering this
 >> bug ? The box runs ospf/zebra for routing daemons and mpd53 for l2tp
 >> LNS termination.

 RW> There are several bugs here, one difficult to fix (lack of
 RW> refcounting), but also stuff like ifp being derived from an interface
 RW> number twice, but checked against NULL only the first time (line 85
 RW> checked for NULL, re-queried but no check line 88).  Fixing the top
 RW> bit of the function to only query the ifp once and check it for NULL
 RW> then would be a good idea.  More fundamentally, we do need to refcount
 RW> ifnets when used from the management path, which is not all that hard
 RW> a change, but preferably to try the easy way first given where we are
 RW> in the release cycle.

I was thinking a bit on this problem (the same issue was reported in
kern/132734) and eliminating double call of ifnet_byindex() was the first
thing I did. But I decided then that the proper fix would be to wrap all
critical code in sysctl_ifdata in IFNET_RLOCK/IFNET_RUNLOCK (the patch is
attached). It looks like I am wrong and my idea about how the kernel works is
oversimplified? :-) Unfortunately, I didn't manage to reproduce the panic in
my environment so I was not able to do some experiments and tests.

-- 
Mikolaj Golub

-------------- next part --------------
A non-text attachment was scrubbed...
Name: if_mib.c.patch
Type: text/x-diff
Size: 2280 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20090421/7195e76b/if_mib.c.bin