diff --git a/sys/dev/ena/ena.c b/sys/dev/ena/ena.c index 63b4598a935..f4abe61f08a 100644 --- a/sys/dev/ena/ena.c +++ b/sys/dev/ena/ena.c @@ -2101,6 +2101,13 @@ ena_up(struct ena_adapter *adapter) ena_log(adapter->pdev, INFO, "device is going UP\n"); + /* + * ena_timer_service can use functions, which write to the admin queue. + * Those calls are not protected by ENA_LOCK, and because of that, the + * timer should be stopped when bringing the device up or down. + */ + ENA_TIMER_DRAIN(adapter); + /* setup interrupts for IO queues */ rc = ena_setup_io_intr(adapter); if (unlikely(rc != 0)) { @@ -2143,19 +2150,12 @@ ena_up(struct ena_adapter *adapter) if_setdrvflagbits(adapter->ifp, IFF_DRV_RUNNING, IFF_DRV_OACTIVE); - /* Activate timer service only if the device is running. - * If this flag is not set, it means that the driver is being - * reset and timer service will be activated afterwards. - */ - if (ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter)) { - callout_reset_sbt(&adapter->timer_service, SBT_1S, - SBT_1S, ena_timer_service, (void *)adapter, 0); - } - ENA_FLAG_SET_ATOMIC(ENA_FLAG_DEV_UP, adapter); ena_unmask_all_io_irqs(adapter); + ENA_TIMER_RESET(adapter); + return (0); err_up_complete: @@ -2165,6 +2165,8 @@ err_up_complete: err_create_queues_with_backoff: ena_free_io_irq(adapter); error: + ENA_TIMER_RESET(adapter); + return (rc); } @@ -2469,9 +2471,10 @@ ena_down(struct ena_adapter *adapter) if (!ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter)) return; - ena_log(adapter->pdev, INFO, "device is going DOWN\n"); + /* Drain timer service to avoid admin queue race condition. */ + ENA_TIMER_DRAIN(adapter); - callout_drain(&adapter->timer_service); + ena_log(adapter->pdev, INFO, "device is going DOWN\n"); ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_DEV_UP, adapter); if_setdrvflagbits(adapter->ifp, IFF_DRV_OACTIVE, @@ -2495,6 +2498,8 @@ ena_down(struct ena_adapter *adapter) ena_free_all_rx_resources(adapter); counter_u64_add(adapter->dev_stats.interface_down, 1); + + ENA_TIMER_RESET(adapter); } static uint32_t @@ -3249,8 +3254,10 @@ ena_timer_service(void *data) adapter->eni_metrics_sample_interval)) { /* * There is no race with other admin queue calls, as: - * - Timer service runs after interface is up, so all + * - Timer service runs after attach function ends, so all * configuration calls to the admin queue are finished. + * - Timer service is temporarily stopped when bringing + * the interface up or down. * - After interface is up, the driver doesn't use (at least * for now) other functions writing to the admin queue. * @@ -3279,7 +3286,7 @@ ena_timer_service(void *data) /* * Schedule another timeout one second from now. */ - callout_schedule_sbt(&adapter->timer_service, SBT_1S, SBT_1S, 0); + ENA_TIMER_RESET(adapter); } void @@ -3294,7 +3301,7 @@ ena_destroy_device(struct ena_adapter *adapter, bool graceful) if_link_state_change(ifp, LINK_STATE_DOWN); - callout_drain(&adapter->timer_service); + ENA_TIMER_DRAIN(adapter); dev_up = ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter); if (dev_up) @@ -3425,17 +3432,15 @@ ena_restore_device(struct ena_adapter *adapter) /* Indicate that device is running again and ready to work */ ENA_FLAG_SET_ATOMIC(ENA_FLAG_DEVICE_RUNNING, adapter); - if (ENA_FLAG_ISSET(ENA_FLAG_DEV_UP_BEFORE_RESET, adapter)) { - /* - * As the AENQ handlers weren't executed during reset because - * the flag ENA_FLAG_DEVICE_RUNNING was turned off, the - * timestamp must be updated again That will prevent next reset - * caused by missing keep alive. - */ - adapter->keep_alive_timestamp = getsbinuptime(); - callout_reset_sbt(&adapter->timer_service, SBT_1S, SBT_1S, - ena_timer_service, (void *)adapter, 0); - } + /* + * As the AENQ handlers weren't executed during reset because + * the flag ENA_FLAG_DEVICE_RUNNING was turned off, the + * timestamp must be updated again That will prevent next reset + * caused by missing keep alive. + */ + adapter->keep_alive_timestamp = getsbinuptime(); + ENA_TIMER_RESET(adapter); + ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_DEV_UP_BEFORE_RESET, adapter); ena_log(dev, INFO, @@ -3457,6 +3462,8 @@ err: ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_ONGOING_RESET, adapter); ena_log(dev, ERR, "Reset attempt failed. Can not reset the device\n"); + ENA_TIMER_RESET(adapter); + return (rc); } @@ -3504,7 +3511,7 @@ ena_attach(device_t pdev) * Set up the timer service - driver is responsible for avoiding * concurrency, as the callout won't be using any locking inside. */ - callout_init(&adapter->timer_service, true); + ENA_TIMER_INIT(adapter); adapter->keep_alive_timeout = DEFAULT_KEEP_ALIVE_TO; adapter->missing_tx_timeout = DEFAULT_TX_CMP_TO; adapter->missing_tx_max_queues = DEFAULT_TX_MONITORED_QUEUES; @@ -3689,6 +3696,9 @@ ena_attach(device_t pdev) if_setdrvflagbits(adapter->ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING); ENA_FLAG_SET_ATOMIC(ENA_FLAG_DEVICE_RUNNING, adapter); + /* Run the timer service */ + ENA_TIMER_RESET(adapter); + return (0); #ifdef DEV_NETMAP @@ -3742,7 +3752,7 @@ ena_detach(device_t pdev) /* Stop timer service */ ENA_LOCK_LOCK(); - callout_drain(&adapter->timer_service); + ENA_TIMER_DRAIN(adapter); ENA_LOCK_UNLOCK(); /* Release reset task */ diff --git a/sys/dev/ena/ena.h b/sys/dev/ena/ena.h index 260c2648289..bd87f39a329 100644 --- a/sys/dev/ena/ena.h +++ b/sys/dev/ena/ena.h @@ -499,6 +499,14 @@ struct ena_adapter { #define ENA_LOCK_UNLOCK() sx_unlock(&ena_global_lock) #define ENA_LOCK_ASSERT() sx_assert(&ena_global_lock, SA_XLOCKED) +#define ENA_TIMER_INIT(_adapter) \ + callout_init(&(_adapter)->timer_service, true) +#define ENA_TIMER_DRAIN(_adapter) \ + callout_drain(&(_adapter)->timer_service) +#define ENA_TIMER_RESET(_adapter) \ + callout_reset_sbt(&(_adapter)->timer_service, SBT_1S, SBT_1S, \ + ena_timer_service, (void*)(_adapter), 0) + #define clamp_t(type, _x, min, max) min_t(type, max_t(type, _x, min), max) #define clamp_val(val, lo, hi) clamp_t(__typeof(val), val, lo, hi)