* [PATCH V1 net 0/2] fixes of interrupt moderation bugs @ 2019-11-04 11:58 akiyano 2019-11-04 11:58 ` [PATCH V1 net 1/2] net: ena: fix issues in setting interrupt moderation params in ethtool akiyano 2019-11-04 11:58 ` [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval akiyano 0 siblings, 2 replies; 7+ messages in thread From: akiyano @ 2019-11-04 11:58 UTC (permalink / raw) To: davem, netdev Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, gtzalik, netanel, alisaidi, benh, ndagan, shayagr From: Arthur Kiyanovski <akiyano@amazon.com> This patchset includes a couple of fixes of bugs in the implemenation of interrupt moderation. Arthur Kiyanovski (2): net: ena: fix issues in setting interrupt moderation params in ethtool net: ena: fix too long default tx interrupt moderation interval drivers/net/ethernet/amazon/ena/ena_com.h | 2 +- drivers/net/ethernet/amazon/ena/ena_ethtool.c | 40 ++++++++----------- 2 files changed, 17 insertions(+), 25 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V1 net 1/2] net: ena: fix issues in setting interrupt moderation params in ethtool 2019-11-04 11:58 [PATCH V1 net 0/2] fixes of interrupt moderation bugs akiyano @ 2019-11-04 11:58 ` akiyano 2019-11-04 11:58 ` [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval akiyano 1 sibling, 0 replies; 7+ messages in thread From: akiyano @ 2019-11-04 11:58 UTC (permalink / raw) To: davem, netdev Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, gtzalik, netanel, alisaidi, benh, ndagan, shayagr From: Arthur Kiyanovski <akiyano@amazon.com> Issue 1: -------- Reproduction steps: 1. sudo ethtool -C eth0 rx-usecs 128 2. sudo ethtool -C eth0 adaptive-rx on 3. sudo ethtool -C eth0 adaptive-rx off 4. ethtool -c eth0 expected output: rx-usecs 128 actual output: rx-usecs 0 Reason for issue: In stage 3, ethtool userspace calls first the ena_get_coalesce() handler to get the current value of all properties, and then the ena_set_coalesce() handler. When ena_get_coalesce() is called the adaptive interrupt moderation is still on. There is an if in the code that returns the rx_coalesce_usecs only if the adaptive interrupt moderation is off. And since it is still on, rx_coalesce_usecs is not set, meaning it stays 0. Solution to issue: Remove this if static interrupt moderation intervals have nothing to do with dynamic ones. Issue 2: -------- Reproduction steps: 1. sudo ethtool -C eth0 rx-usecs 128 2. sudo ethtool -C eth0 adaptive-rx on 3. sudo ethtool -C eth0 rx-usecs 128 4. ethtool -c eth0 expected output: rx-usecs 128 actual output: rx-usecs 0 Reason for issue: In stage 3, when ena_set_coalesce() is called, the handler tests if rx adaptive interrupt moderation is on, and if it is, it returns before getting to the part in the function that sets the rx non-adaptive interrupt moderation interval. Solution to issue: Remove the return from the function when rx adaptive interrupt moderation is on. Additional small fixes in this commit: -------------------------------------- 1. Remove 2 unnecessary comments. 2. Remove 4 unnecesary "{}" in single row if statements. 3. Reorder ena_set_coalesce() to make sense. 4. Change the names of ena_update_tx/rx_rings_intr_moderation() functions to ena_update_tx/rx_rings_nonadaptive_intr_moderation() for clarity. Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> --- drivers/net/ethernet/amazon/ena/ena_ethtool.c | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c index 16553d92fad2..b042c70504cd 100644 --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c @@ -306,19 +306,16 @@ static int ena_get_coalesce(struct net_device *net_dev, struct ena_adapter *adapter = netdev_priv(net_dev); struct ena_com_dev *ena_dev = adapter->ena_dev; - if (!ena_com_interrupt_moderation_supported(ena_dev)) { - /* the devie doesn't support interrupt moderation */ + if (!ena_com_interrupt_moderation_supported(ena_dev)) return -EOPNOTSUPP; - } coalesce->tx_coalesce_usecs = ena_com_get_nonadaptive_moderation_interval_tx(ena_dev) * ena_dev->intr_delay_resolution; - if (!ena_com_get_adaptive_moderation_enabled(ena_dev)) - coalesce->rx_coalesce_usecs = - ena_com_get_nonadaptive_moderation_interval_rx(ena_dev) - * ena_dev->intr_delay_resolution; + coalesce->rx_coalesce_usecs = + ena_com_get_nonadaptive_moderation_interval_rx(ena_dev) + * ena_dev->intr_delay_resolution; coalesce->use_adaptive_rx_coalesce = ena_com_get_adaptive_moderation_enabled(ena_dev); @@ -326,7 +323,7 @@ static int ena_get_coalesce(struct net_device *net_dev, return 0; } -static void ena_update_tx_rings_intr_moderation(struct ena_adapter *adapter) +static void ena_update_tx_rings_nonadaptive_intr_moderation(struct ena_adapter *adapter) { unsigned int val; int i; @@ -337,7 +334,7 @@ static void ena_update_tx_rings_intr_moderation(struct ena_adapter *adapter) adapter->tx_ring[i].smoothed_interval = val; } -static void ena_update_rx_rings_intr_moderation(struct ena_adapter *adapter) +static void ena_update_rx_rings_nonadaptive_intr_moderation(struct ena_adapter *adapter) { unsigned int val; int i; @@ -355,35 +352,30 @@ static int ena_set_coalesce(struct net_device *net_dev, struct ena_com_dev *ena_dev = adapter->ena_dev; int rc; - if (!ena_com_interrupt_moderation_supported(ena_dev)) { - /* the devie doesn't support interrupt moderation */ + if (!ena_com_interrupt_moderation_supported(ena_dev)) return -EOPNOTSUPP; - } rc = ena_com_update_nonadaptive_moderation_interval_tx(ena_dev, coalesce->tx_coalesce_usecs); if (rc) return rc; - ena_update_tx_rings_intr_moderation(adapter); - - if (coalesce->use_adaptive_rx_coalesce) { - if (!ena_com_get_adaptive_moderation_enabled(ena_dev)) - ena_com_enable_adaptive_moderation(ena_dev); - return 0; - } + ena_update_tx_rings_nonadaptive_intr_moderation(adapter); rc = ena_com_update_nonadaptive_moderation_interval_rx(ena_dev, coalesce->rx_coalesce_usecs); if (rc) return rc; - ena_update_rx_rings_intr_moderation(adapter); + ena_update_rx_rings_nonadaptive_intr_moderation(adapter); - if (!coalesce->use_adaptive_rx_coalesce) { - if (ena_com_get_adaptive_moderation_enabled(ena_dev)) - ena_com_disable_adaptive_moderation(ena_dev); - } + if ((coalesce->use_adaptive_rx_coalesce) && + (!ena_com_get_adaptive_moderation_enabled(ena_dev))) + ena_com_enable_adaptive_moderation(ena_dev); + + if ((!coalesce->use_adaptive_rx_coalesce) && + (ena_com_get_adaptive_moderation_enabled(ena_dev))) + ena_com_disable_adaptive_moderation(ena_dev); return 0; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval 2019-11-04 11:58 [PATCH V1 net 0/2] fixes of interrupt moderation bugs akiyano 2019-11-04 11:58 ` [PATCH V1 net 1/2] net: ena: fix issues in setting interrupt moderation params in ethtool akiyano @ 2019-11-04 11:58 ` akiyano 2019-11-04 19:18 ` David Miller 1 sibling, 1 reply; 7+ messages in thread From: akiyano @ 2019-11-04 11:58 UTC (permalink / raw) To: davem, netdev Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, gtzalik, netanel, alisaidi, benh, ndagan, shayagr From: Arthur Kiyanovski <akiyano@amazon.com> Current default non-adaptive tx interrupt moderation interval is 196 us. This commit sets it to 0, which is much more sensible as a default value. It can be modified using ethtool -C. Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> --- drivers/net/ethernet/amazon/ena/ena_com.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h index 7c941eba0bc9..164c45c04867 100644 --- a/drivers/net/ethernet/amazon/ena/ena_com.h +++ b/drivers/net/ethernet/amazon/ena/ena_com.h @@ -72,7 +72,7 @@ /*****************************************************************************/ /* ENA adaptive interrupt moderation settings */ -#define ENA_INTR_INITIAL_TX_INTERVAL_USECS 196 +#define ENA_INTR_INITIAL_TX_INTERVAL_USECS 0 #define ENA_INTR_INITIAL_RX_INTERVAL_USECS 0 #define ENA_DEFAULT_INTR_DELAY_RESOLUTION 1 -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval 2019-11-04 11:58 ` [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval akiyano @ 2019-11-04 19:18 ` David Miller 2019-11-06 19:13 ` Kiyanovski, Arthur 0 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2019-11-04 19:18 UTC (permalink / raw) To: akiyano Cc: netdev, dwmw, zorik, matua, saeedb, msw, aliguori, nafea, gtzalik, netanel, alisaidi, benh, ndagan, shayagr From: <akiyano@amazon.com> Date: Mon, 4 Nov 2019 13:58:48 +0200 > From: Arthur Kiyanovski <akiyano@amazon.com> > > Current default non-adaptive tx interrupt moderation interval is 196 us. > This commit sets it to 0, which is much more sensible as a default value. > It can be modified using ethtool -C. > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> I do not agree that turning TX interrupt moderation off completely is a more sensible default value. Maybe a much smaller value, but turning off the coalescing delay completely is a bit much. ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval 2019-11-04 19:18 ` David Miller @ 2019-11-06 19:13 ` Kiyanovski, Arthur 2019-11-13 20:08 ` Machulsky, Zorik 0 siblings, 1 reply; 7+ messages in thread From: Kiyanovski, Arthur @ 2019-11-06 19:13 UTC (permalink / raw) To: David Miller Cc: netdev, Woodhouse, David, Machulsky, Zorik, Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt, Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel, Saidi, Ali, Herrenschmidt, Benjamin, Dagan, Noam, Agroskin, Shay, Jubran, Samih > -----Original Message----- > From: David Miller <davem@davemloft.net> > Sent: Monday, November 4, 2019 9:19 PM > To: Kiyanovski, Arthur <akiyano@amazon.com> > Cc: netdev@vger.kernel.org; Woodhouse, David <dwmw@amazon.co.uk>; > Machulsky, Zorik <zorik@amazon.com>; Matushevsky, Alexander > <matua@amazon.com>; Bshara, Saeed <saeedb@amazon.com>; Wilson, > Matt <msw@amazon.com>; Liguori, Anthony <aliguori@amazon.com>; > Bshara, Nafea <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; > Belgazal, Netanel <netanel@amazon.com>; Saidi, Ali > <alisaidi@amazon.com>; Herrenschmidt, Benjamin <benh@amazon.com>; > Dagan, Noam <ndagan@amazon.com>; Agroskin, Shay > <shayagr@amazon.com> > Subject: Re: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt > moderation interval > > From: <akiyano@amazon.com> > Date: Mon, 4 Nov 2019 13:58:48 +0200 > > > From: Arthur Kiyanovski <akiyano@amazon.com> > > > > Current default non-adaptive tx interrupt moderation interval is 196 us. > > This commit sets it to 0, which is much more sensible as a default value. > > It can be modified using ethtool -C. > > > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> > > I do not agree that turning TX interrupt moderation off completely is a more > sensible default value. > > Maybe a much smaller value, but turning off the coalescing delay completely > is a bit much. David, Up until now, the ENA device did not support interrupt moderation, so effectively the default tx interrupt moderation interval was 0. You are probably right that 0 is not an optimal value. However until we research and find such an optimal value, in order to avoid a degradation in default performance, we want the default value in the new driver to be (effectively) the same as in the old driver. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval 2019-11-06 19:13 ` Kiyanovski, Arthur @ 2019-11-13 20:08 ` Machulsky, Zorik [not found] ` <92294494768e4c41a0755218e51a0138@EX13D22EUA004.ant.amazon.com> 0 siblings, 1 reply; 7+ messages in thread From: Machulsky, Zorik @ 2019-11-13 20:08 UTC (permalink / raw) To: Kiyanovski, Arthur, David Miller Cc: netdev, Woodhouse, David, Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt, Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel, Saidi, Ali, Herrenschmidt, Benjamin, Dagan, Noam, Agroskin, Shay, Jubran, Samih > -----Original Message----- > From: David Miller <davem@davemloft.net> > Sent: Monday, November 4, 2019 9:19 PM > To: Kiyanovski, Arthur <akiyano@amazon.com> > Cc: netdev@vger.kernel.org; Woodhouse, David <dwmw@amazon.co.uk>; > Machulsky, Zorik <zorik@amazon.com>; Matushevsky, Alexander > <matua@amazon.com>; Bshara, Saeed <saeedb@amazon.com>; Wilson, > Matt <msw@amazon.com>; Liguori, Anthony <aliguori@amazon.com>; > Bshara, Nafea <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; > Belgazal, Netanel <netanel@amazon.com>; Saidi, Ali > <alisaidi@amazon.com>; Herrenschmidt, Benjamin <benh@amazon.com>; > Dagan, Noam <ndagan@amazon.com>; Agroskin, Shay > <shayagr@amazon.com> > Subject: Re: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt > moderation interval > > From: <akiyano@amazon.com> > Date: Mon, 4 Nov 2019 13:58:48 +0200 > > > From: Arthur Kiyanovski <akiyano@amazon.com> > > > > Current default non-adaptive tx interrupt moderation interval is 196 us. > > This commit sets it to 0, which is much more sensible as a default value. > > It can be modified using ethtool -C. > > > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> > > I do not agree that turning TX interrupt moderation off completely is a more > sensible default value. > > Maybe a much smaller value, but turning off the coalescing delay completely > is a bit much. David, Up until now, the ENA device did not support interrupt moderation, so effectively the default tx interrupt moderation interval was 0. You are probably right that 0 is not an optimal value. However until we research and find such an optimal value, in order to avoid a degradation in default performance, we want the default value in the new driver to be (effectively) the same as in the old driver. David, Just wanted to re-iterate what Arthur has mentioned. We clearly see BW and CPU utilization improvement with introduction of DIM on the Rx side and non-adaptive moderation on the Tx side in our driver. We'd like to deliver this to our customers ASAP. We are usually very cautious with introduction of the new features, therefore we'd like to keep interrupt moderation disabled by default for now. We'd advise our customers awaiting for it to enable it using ethtool. We are going to enable moderation by default after we accumulate more mileage with it and fine tune it further. That's the reason behind having Tx interval =0 for now (Rx moderation is disabled by default as well). Hope it makes sense. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <92294494768e4c41a0755218e51a0138@EX13D22EUA004.ant.amazon.com>]
* Re: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval [not found] ` <92294494768e4c41a0755218e51a0138@EX13D22EUA004.ant.amazon.com> @ 2019-11-18 22:50 ` Machulsky, Zorik 0 siblings, 0 replies; 7+ messages in thread From: Machulsky, Zorik @ 2019-11-18 22:50 UTC (permalink / raw) To: David Miller, Kiyanovski, Arthur Cc: netdev, Woodhouse, David, Machulsky, Zorik, Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt, Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel, Saidi, Ali, Herrenschmidt, Benjamin, Dagan, Noam, Agroskin, Shay > -----Original Message----- > From: David Miller <davem@davemloft.net> > Sent: Monday, November 4, 2019 9:19 PM > To: Kiyanovski, Arthur <akiyano@amazon.com> > Cc: netdev@vger.kernel.org; Woodhouse, David <dwmw@amazon.co.uk>; > Machulsky, Zorik <zorik@amazon.com>; Matushevsky, Alexander > <matua@amazon.com>; Bshara, Saeed <saeedb@amazon.com>; Wilson, > Matt <msw@amazon.com>; Liguori, Anthony <aliguori@amazon.com>; > Bshara, Nafea <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; > Belgazal, Netanel <netanel@amazon.com>; Saidi, Ali > <alisaidi@amazon.com>; Herrenschmidt, Benjamin <benh@amazon.com>; > Dagan, Noam <ndagan@amazon.com>; Agroskin, Shay > <shayagr@amazon.com> > Subject: Re: [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt > moderation interval > > From: <akiyano@amazon.com> > Date: Mon, 4 Nov 2019 13:58:48 +0200 > > > From: Arthur Kiyanovski <akiyano@amazon.com> > > > > Current default non-adaptive tx interrupt moderation interval is 196 us. > > This commit sets it to 0, which is much more sensible as a default value. > > It can be modified using ethtool -C. > > > > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> > > I do not agree that turning TX interrupt moderation off completely is a more > sensible default value. > > Maybe a much smaller value, but turning off the coalescing delay completely > is a bit much. David, Up until now, the ENA device did not support interrupt moderation, so effectively the default tx interrupt moderation interval was 0. You are probably right that 0 is not an optimal value. However until we research and find such an optimal value, in order to avoid a degradation in default performance, we want the default value in the new driver to be (effectively) the same as in the old driver. David, Just wanted to re-iterate what Arthur has mentioned. We clearly see BW and CPU utilization improvement with introduction of DIM on the Rx side and non-adaptive moderation on the Tx side in our driver. We'd like to deliver this to our customers ASAP. We are usually very cautious with introduction of the new features, therefore we'd like to keep interrupt moderation disabled by default for now. We'd advise our customers awaiting for it to enable it using ethtool. We are going to enable moderation by default after we accumulate more mileage with it and fine tune it further. That's the reason behind having Tx interval =0 for now (Rx moderation is disabled by default as well). Hope it makes sense. David, I'm bumping this up to bring it to your attention. Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-18 22:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-04 11:58 [PATCH V1 net 0/2] fixes of interrupt moderation bugs akiyano
2019-11-04 11:58 ` [PATCH V1 net 1/2] net: ena: fix issues in setting interrupt moderation params in ethtool akiyano
2019-11-04 11:58 ` [PATCH V1 net 2/2] net: ena: fix too long default tx interrupt moderation interval akiyano
2019-11-04 19:18 ` David Miller
2019-11-06 19:13 ` Kiyanovski, Arthur
2019-11-13 20:08 ` Machulsky, Zorik
[not found] ` <92294494768e4c41a0755218e51a0138@EX13D22EUA004.ant.amazon.com>
2019-11-18 22:50 ` Machulsky, Zorik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.