Page MenuHomeFreeBSD

Check for non-empty sysctl_ctx before attempting free
Needs ReviewPublic

Authored by stallamr_netapp.com on Jun 13 2021, 8:14 PM.

Details

Summary

sysctl_ctx gets created only in non-recovery mode and is filled when device attach is completed. However, when device_attach fails, IFLIB internally calls into IFDI_QUEUES_FREE and attempts to parse sysctl_ctx list with no entries in it. As a result, it panics the node. So, check for non-empty list before attempting the free. Also, sysctl_ctx is a VSI resource rather a queue resource. So, move the sysctl_ctx free to DETACH() instead.

Test Plan

Added a dummy failpoint to simulate iflib_device_register() failure before IFDI_ATTACH_POST

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 39878
Build 36767: arc lint + arc unit

Event Timeline

I think checking for TAILQ_EMPTY(&vsi->sysctl_ctx) is probably a hack. sysctl_ctx_free() handles an empty queue perfectly well. The problem seems to be that the list is not initialized (with sysctl_ctx_init()) in some paths where sysctl_ctx_free() may be called.

I would suggest instead calling sysctl_ctx_init(&vsi->sysctl_ctx) much earlier during ixl attach, in ixl_if_attach_pre().

sys/dev/ixl/if_ixl.c
867

The formatting is wrong: continuing lines should be indendent by four spaces.

Actually using separate sysctl_ctx was necessary in non-iflib version of the driver to allow releasing and re-allocating queues while handling EMP reset. In this version Rx queues memory is managed by the iflib with IFDI interface so using separate syssctl context may be not required. Let me double check that.

Thanks Krzysztof , will hold-onto this review and wait on you for further direction.