On Thu, 3 Oct 2019 15:32:33 +0000, Jubran, Samih wrote: > > > +int ena_update_queue_count(struct ena_adapter *adapter, u32 > > > new_channel_count) { > > > + struct ena_com_dev *ena_dev = adapter->ena_dev; > > > + bool dev_was_up; > > > + > > > + dev_was_up = test_bit(ENA_FLAG_DEV_UP, &adapter->flags); > > > + ena_close(adapter->netdev); > > > + adapter->num_io_queues = new_channel_count; > > > + /* We need to destroy the rss table so that the indirection > > > + * table will be reinitialized by ena_up() > > > + */
Ugh, this comment is broken in the same way the comment Colin fixed in commit 4208966f65f5 ("net: ena: clean up indentation issue") was. Please double check your submission for this and use checkpatch. > > > + ena_com_rss_destroy(ena_dev); > > > + ena_init_io_rings(adapter); > > > + return dev_was_up ? ena_open(adapter->netdev) : 0; > > > > You should try to prepare the resources for the new configuration before > > you attempt the change. Otherwise if allocation of new rings fails the open > > will leave the device in a broken state. > > Our ena_up() applies logarithmic backoff mechanism on the rings size if > the allocations fail, and therefore the device will stay functional. Sure, a heuristic like that will help, but doesn't give hard guarantees, which is what engineers like :) > > This is not always enforced upstream, but you can see mlx5 or nfp for > > examples of drivers which do this right.. > > > > > } > > > > > > static void ena_tx_csum(struct ena_com_tx_ctx *ena_tx_ctx, struct > > > sk_buff *skb)