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)  

Reply via email to