| Message ID | 1532010858-6183-1-git-send-email-tariqt@mellanox.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: rollback orig value on failure of dev_qdisc_change_tx_queue_len | expand |
On Thu, Jul 19, 2018 at 7:34 AM Tariq Toukan <tariqt@mellanox.com> wrote: > > Fix dev_change_tx_queue_len so it rolls back original value > upon a failure in dev_qdisc_change_tx_queue_len. > This is already done for notifirers' failures, share the code. > > The revert of changes in dev_qdisc_change_tx_queue_len > in such case is needed but missing (marked as TODO), yet it is > still better to not apply the new requested value. You misunderstand the TODO, that is for reverting tx queue len change for previous queues in the loop. I still don't have any nice solution for this yet. Yeah, your change itself looks good. Please update the changelog. Thanks.
On 19/07/2018 8:25 PM, Cong Wang wrote: > On Thu, Jul 19, 2018 at 7:34 AM Tariq Toukan <tariqt@mellanox.com> wrote: >> >> Fix dev_change_tx_queue_len so it rolls back original value >> upon a failure in dev_qdisc_change_tx_queue_len. >> This is already done for notifirers' failures, share the code. >> >> The revert of changes in dev_qdisc_change_tx_queue_len >> in such case is needed but missing (marked as TODO), yet it is >> still better to not apply the new requested value. > > You misunderstand the TODO, that is for reverting tx queue len > change for previous queues in the loop. I still don't have any > nice solution for this yet. > I understood this, but maybe didn't describe it clearly. I'll re-phrase. > Yeah, your change itself looks good. > Thanks. > Please update the changelog. > > Thanks. >
diff --git a/net/core/dev.c b/net/core/dev.c index a5aa1c7444e6..559a91271f82 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -7149,16 +7149,19 @@ int dev_change_tx_queue_len(struct net_device *dev, unsigned long new_len) dev->tx_queue_len = new_len; res = call_netdevice_notifiers(NETDEV_CHANGE_TX_QUEUE_LEN, dev); res = notifier_to_errno(res); - if (res) { - netdev_err(dev, - "refused to change device tx_queue_len\n"); - dev->tx_queue_len = orig_len; - return res; - } - return dev_qdisc_change_tx_queue_len(dev); + if (res) + goto err_rollback; + res = dev_qdisc_change_tx_queue_len(dev); + if (res) + goto err_rollback; } return 0; + +err_rollback: + netdev_err(dev, "refused to change device tx_queue_len\n"); + dev->tx_queue_len = orig_len; + return res; } /**
Fix dev_change_tx_queue_len so it rolls back original value upon a failure in dev_qdisc_change_tx_queue_len. This is already done for notifirers' failures, share the code. The revert of changes in dev_qdisc_change_tx_queue_len in such case is needed but missing (marked as TODO), yet it is still better to not apply the new requested value. Fixes: 48bfd55e7e41 ("net_sched: plug in qdisc ops change_tx_queue_len") Signed-off-by: Tariq Toukan <tariqt@mellanox.com> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com> Reported-by: Ran Rozenstein <ranro@mellanox.com> Cc: Cong Wang <xiyou.wangcong@gmail.com> --- net/core/dev.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)