cxgbe's native netmap support broken since r307394

[Vincenzo Maffione]

Hi Navdeep,

  Indeed, we have reviewed the code, and we think it is ok to
implement nm_os_ifnet_lock() with IFNET_RLOCK(), instead of using
IFNET_WLOCK().
Since IFNET_RLOCK() results into sx_slock(), this should fix the issue.

On FreeBSD, this locking is needed to protect a flag read by nm_iszombie().
However, on Linux the same lock is also needed to protect the call to
the nm_hw_register() callback, so we prefer to have an "unified"
locking scheme, i.e. always calling nm_hw_register under the lock.

Does this make sense to you? Would it be easy for you to make a quick
test by replacing IFNET_WLOCK with IFNET_RLOCK?

Thanks,
  Vincenzo

2016-12-17 23:28 GMT+01:00 Navdeep Parhar <np at freebsd.org>:
> Luigi, Vincenzo,
>
> The last major update to netmap (r307394 and followups) broke cxgbe's
> native netmap support.  The problem is that netmap_hw_reg now holds an
> rw_lock around the driver's netmap_on/off routines.  It has always been
> safe for the driver to sleep during these operations but now it panics
> instead.
>
> Why is IFNET_WLOCK needed here?  It seems like a regression to disallow
> sleep on the control path.
>
> Regards,
> Navdeep
>
> begin_synchronized_op with the following non-sleepable locks held:
> exclusive rw ifnet_rw (ifnet_rw) r = 0 (0xffffffff8271d680) locked @
> /root/ws/head/sys/dev/netmap/netmap_freebsd.c:95
> stack backtrace:
> #0 0xffffffff810837a5 at witness_debugger+0xe5
> #1 0xffffffff81084d88 at witness_warn+0x3b8
> #2 0xffffffff83ef2bcc at begin_synchronized_op+0x6c
> #3 0xffffffff83f14beb at cxgbe_netmap_reg+0x5b
> #4 0xffffffff809846f1 at netmap_hw_reg+0x81
> #5 0xffffffff809806de at netmap_do_regif+0x19e
> #6 0xffffffff8098121d at netmap_ioctl+0x7ad
> #7 0xffffffff8098682f at freebsd_netmap_ioctl+0x5f



-- 
Vincenzo Maffione