Changes between V3 and V2:
* Fix typos and correct alignment in commit messages.
* use napi_complete_done() return value to determine when the napi
handler needs to unmask the interrupts rather than implementing
non standard solution.
* Remove new features from this patchset and leave bug fixes only.
* Give example in the commit message for kernel crashes.
* Use BIT(x) instead of use the value explicitly.
Netanel Belgazal (14):
net/ena: remove ntuple filter support from device feature list
net/ena: fix error handling when probe fails
net/ena: fix queues number calculation
net/ena: fix ethtool RSS flow configuration
net/ena: fix RSS default hash configuration
net/ena: fix NULL dereference when removing the driver after device
reset failed
net/ena: refactor ena_get_stats64 to be atomic context safe
net/ena: fix potential access to freed memory during device reset
net/ena: use napi_complete_done() return value
net/ena: use READ_ONCE to access completion descriptors
net/ena: reduce the severity of ena printouts
net/ena: change driver's default timeouts
net/ena: change condition for host attribute configuration
net/ena: update driver version to 1.1.2
drivers/net/ethernet/amazon/ena/ena_admin_defs.h | 20 ++-
drivers/net/ethernet/amazon/ena/ena_com.c | 41 ++---
drivers/net/ethernet/amazon/ena/ena_com.h | 1 +
drivers/net/ethernet/amazon/ena/ena_eth_com.c | 8 +-
drivers/net/ethernet/amazon/ena/ena_netdev.c | 186 ++++++++++++++++-------
drivers/net/ethernet/amazon/ena/ena_netdev.h | 9 +-
6 files changed, 182 insertions(+), 83 deletions(-)
--
2.7.4
Remove NETIF_F_NTUPLE from netdev->features.
The ENA device driver does not support ntuple filtering.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 -
1 file changed, 1 deletion(-)
When driver fails in probe, it will release all resources,
including adapter.
In case of probe failure, ena_remove should not try to
free the adapter resources.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
1 file changed, 1 insertion(+)
When driver fails in probe, it will release all resources,
including adapter.
In case of probe failure, ena_remove should not try to
free the adapter resources.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
1 file changed, 1 insertion(+)
Is this change really a "fix"? remove() should only be called if
probe() has been successful before, otherwise not. Did you experience
something different?
Regards,
Lino
Hi,
You are right. I'll remove this patch.
Regards,
Netanel
On 01/28/2017 01:33 AM, Lino Sanfilippo wrote:
Hi,
On 26.01.2017 23:18, Netanel Belgazal wrote:
quoted
When driver fails in probe, it will release all resources,
including adapter.
In case of probe failure, ena_remove should not try to
free the adapter resources.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
1 file changed, 1 insertion(+)
Is this change really a "fix"? remove() should only be called if
probe() has been successful before, otherwise not. Did you experience
something different?
Regards,
Lino
ENA default hash configures IPv4_frag hash twice instead of
configure non-IP packets.
The bug caused IPv4 fragmented packets to be calculated based on
L2 source and destination address instead of L3 source and destination.
IPv4 packets can reach to the wrong Rx queue.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -2509,6 +2509,8 @@ static void ena_fw_reset_device(struct work_struct *work)err:rtnl_unlock();+clear_bit(ENA_FLAG_DEVICE_RUNNING,&adapter->flags);+dev_err(&pdev->dev,"Reset attempt failed. Can not reset the device\n");}
@@ -3118,7 +3120,9 @@ static void ena_remove(struct pci_dev *pdev)cancel_work_sync(&adapter->resume_io_task);-ena_com_dev_reset(ena_dev);+/* Reset the device only if the device is running. */+if(test_bit(ENA_FLAG_DEVICE_RUNNING,&adapter->flags))+ena_com_dev_reset(ena_dev);ena_free_mgmnt_irq(adapter);
ndo_get_stat64() can be called from atomic context, but the current
implementation sends an admin command to retrieve the statistics from
the device. This admin command can sleep.
This patch re-factors the implementation of ena_get_stats64() to use
the {rx,tx}bytes/count from the driver's inner counters, and to obtain
the rx drop counter from the asynchronous keep alive (heart bit)
event.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_admin_defs.h | 8 ++++
drivers/net/ethernet/amazon/ena/ena_netdev.c | 57 +++++++++++++++++-------
drivers/net/ethernet/amazon/ena/ena_netdev.h | 1 +
3 files changed, 51 insertions(+), 15 deletions(-)
If the ena driver detects that the device is not behave as expected,
it tries to reset the device.
The reset flow calls ena_down, which will frees all the resources
the driver allocates and then it will reset the device.
This flow can cause memory corruption if the device is still writes
to the driver's memory space.
To overcome this potential race, move the reset before the device
resources are freed.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 56 +++++++++++++++++++++-------
1 file changed, 43 insertions(+), 13 deletions(-)
@@ -80,14 +80,18 @@ static void ena_tx_timeout(struct net_device *dev){structena_adapter*adapter=netdev_priv(dev);+/* Change the state of the device to trigger reset+*Checkthatwearenotinthemiddleoratriggeralready+*/++if(test_and_set_bit(ENA_FLAG_TRIGGER_RESET,&adapter->flags))+return;+u64_stats_update_begin(&adapter->syncp);adapter->dev_stats.tx_timeout++;u64_stats_update_end(&adapter->syncp);netif_err(adapter,tx_err,dev,"Transmit time out\n");--/* Change the state of the device to trigger reset */-set_bit(ENA_FLAG_TRIGGER_RESET,&adapter->flags);}staticvoidupdate_rx_ring_mtu(structena_adapter*adapter,intmtu)
@@ -1109,7 +1113,8 @@ static int ena_io_poll(struct napi_struct *napi, int budget)tx_budget=tx_ring->ring_size/ENA_TX_POLL_BUDGET_DIVIDER;-if(!test_bit(ENA_FLAG_DEV_UP,&tx_ring->adapter->flags)){+if(!test_bit(ENA_FLAG_DEV_UP,&tx_ring->adapter->flags)||+test_bit(ENA_FLAG_TRIGGER_RESET,&tx_ring->adapter->flags)){napi_complete_done(napi,0);return0;}
@@ -1698,12 +1703,22 @@ static void ena_down(struct ena_adapter *adapter)adapter->dev_stats.interface_down++;u64_stats_update_end(&adapter->syncp);-/* After this point the napi handler won't enable the tx queue */-ena_napi_disable_all(adapter);netif_carrier_off(adapter->netdev);netif_tx_disable(adapter->netdev);+/* After this point the napi handler won't enable the tx queue */+ena_napi_disable_all(adapter);+/* After destroy the queue there won't be any new interrupts */++if(test_bit(ENA_FLAG_TRIGGER_RESET,&adapter->flags)){+intrc;++rc=ena_com_dev_reset(adapter->ena_dev);+if(rc)+dev_err(&adapter->pdev->dev,"Device reset failed\n");+}+ena_destroy_all_io_queues(adapter);ena_disable_io_intr_sync(adapter);
@@ -2065,6 +2080,14 @@ static void ena_netpoll(struct net_device *netdev)structena_adapter*adapter=netdev_priv(netdev);inti;+/* Dont schedule NAPI if the driver is in the middle of reset+*ornetdevisdown.+*/++if(!test_bit(ENA_FLAG_DEV_UP,&adapter->flags)||+test_bit(ENA_FLAG_TRIGGER_RESET,&adapter->flags))+return;+for(i=0;i<adapter->num_queues;i++)napi_schedule(&adapter->ena_napi[i].napi);}
@@ -2451,6 +2474,14 @@ static void ena_fw_reset_device(struct work_struct *work)booldev_up,wd_state;intrc;+if(unlikely(!test_bit(ENA_FLAG_TRIGGER_RESET,&adapter->flags))){+dev_err(&pdev->dev,+"device reset schedule while reset bit is off\n");+return;+}++netif_carrier_off(netdev);+del_timer_sync(&adapter->timer_service);rtnl_lock();
@@ -2482,6 +2507,8 @@ static void ena_fw_reset_device(struct work_struct *work)ena_com_mmio_reg_read_request_destroy(ena_dev);+clear_bit(ENA_FLAG_TRIGGER_RESET,&adapter->flags);+/* Finish with the destroy part. Start the init part */rc=ena_device_init(ena_dev,adapter->pdev,&get_feat_ctx,&wd_state);
Completion descriptors are accessed from the driver and from the device.
To avoid reading the old value, use READ_ONCE macro.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_com.h | 1 +
drivers/net/ethernet/amazon/ena/ena_eth_com.c | 8 ++++----
2 files changed, 5 insertions(+), 4 deletions(-)
The timeouts were too agressive and sometimes cause false alarms.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 4 ++--
drivers/net/ethernet/amazon/ena/ena_netdev.h | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
@@ -100,7 +100,7 @@/* Number of queues to check for missing queues per timer service */#define ENA_MONITORED_TX_QUEUES 4/* Max timeout packets before device reset */-#define MAX_NUM_OF_TIMEOUTED_PACKETS 32+#define MAX_NUM_OF_TIMEOUTED_PACKETS 128#define ENA_TX_RING_IDX_NEXT(idx, ring_size) (((idx) + 1) & ((ring_size) - 1))
@@ -116,9 +116,9 @@#define ENA_IO_IRQ_IDX(q) (ENA_IO_IRQ_FIRST_IDX + (q))/* ENA device should send keep alive msg every 1 sec.-*Wewaitfor3secjusttobeonthesafeside.+*Wewaitfor6secjusttobeonthesafeside.*/-#define ENA_DEVICE_KALIVE_TIMEOUT (3 * HZ)+#define ENA_DEVICE_KALIVE_TIMEOUT (6 * HZ)#define ENA_MMIO_DISABLE_REG_READ BIT(0)
Move the host info config to be the first admin command that is executed.
This change require the driver to remove the 'feature check'
from host info configuration flow.
The check is removed since the supported features bitmask field
is retrieved only after calling ENA_ADMIN_DEVICE_ATTRIBUTES admin command.
If set host info is not supported an error will be returned by the device.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 8 +++-----
drivers/net/ethernet/amazon/ena/ena_netdev.c | 5 +++--
2 files changed, 6 insertions(+), 7 deletions(-)
@@ -2451,11 +2451,9 @@ int ena_com_set_host_attributes(struct ena_com_dev *ena_dev)intret;-if(!ena_com_check_supported_feature_id(ena_dev,-ENA_ADMIN_HOST_ATTR_CONFIG)){-pr_warn("Set host attribute isn't supported\n");-return-EPERM;-}+/* Host attribute config is called before ena_com_get_dev_attr_feat+*soena_comcan'tcheckifthefeatureissupported.+*/memset(&cmd,0x0,sizeof(cmd));admin_queue=&ena_dev->admin_queue;
@@ -1126,7 +1126,13 @@ int ena_com_execute_admin_command(struct ena_com_admin_queue *admin_queue,comp_ctx=ena_com_submit_admin_cmd(admin_queue,cmd,cmd_size,comp,comp_size);if(unlikely(IS_ERR(comp_ctx))){-pr_err("Failed to submit command [%ld]\n",PTR_ERR(comp_ctx));+if(comp_ctx==ERR_PTR(-ENODEV))+pr_debug("Failed to submit command [%ld]\n",+PTR_ERR(comp_ctx));+else+pr_err("Failed to submit command [%ld]\n",+PTR_ERR(comp_ctx));+returnPTR_ERR(comp_ctx);}
@@ -1895,7 +1901,7 @@ int ena_com_set_dev_mtu(struct ena_com_dev *ena_dev, int mtu)intret;if(!ena_com_check_supported_feature_id(ena_dev,ENA_ADMIN_MTU)){-pr_info("Feature %d isn't supported\n",ENA_ADMIN_MTU);+pr_debug("Feature %d isn't supported\n",ENA_ADMIN_MTU);return-EPERM;}
Do not unamsk interrupts if we are in busy poll mode.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 44 ++++++++++++++++++----------
1 file changed, 29 insertions(+), 15 deletions(-)
@@ -1122,26 +1122,40 @@ static int ena_io_poll(struct napi_struct *napi, int budget)tx_work_done=ena_clean_tx_irq(tx_ring,tx_budget);rx_work_done=ena_clean_rx_irq(rx_ring,napi,budget);-if((budget>rx_work_done)&&(tx_budget>tx_work_done)){-napi_complete_done(napi,rx_work_done);+/* If the device is about to reset or down, avoid unmask+*theinterruptandreturn0soNAPIwon'treschedule+*/+if(unlikely(!test_bit(ENA_FLAG_DEV_UP,&tx_ring->adapter->flags)||+test_bit(ENA_FLAG_TRIGGER_RESET,&tx_ring->adapter->flags))){+napi_complete_done(napi,0);+ret=0;+}elseif((budget>rx_work_done)&&(tx_budget>tx_work_done)){napi_comp_call=1;-/* Tx and Rx share the same interrupt vector */-if(ena_com_get_adaptive_moderation_enabled(rx_ring->ena_dev))-ena_adjust_intr_moderation(rx_ring,tx_ring);-/* Update intr register: rx intr delay, tx intr delay and-*interruptunmask+/* Update numa and unmask the interrupt only when schedule+*fromtheinterruptcontext(vsfromsk_busy_loop)*/-ena_com_update_intr_reg(&intr_reg,-rx_ring->smoothed_interval,-tx_ring->smoothed_interval,-true);+if(napi_complete_done(napi,rx_work_done)){+/* Tx and Rx share the same interrupt vector */+if(ena_com_get_adaptive_moderation_enabled(rx_ring->ena_dev))+ena_adjust_intr_moderation(rx_ring,tx_ring);++/* Update intr register: rx intr delay,+*txintrdelayandinterruptunmask+*/+ena_com_update_intr_reg(&intr_reg,+rx_ring->smoothed_interval,+tx_ring->smoothed_interval,+true);++/* It is a shared MSI-X.+*TxandRxCQhavepointertoit.+*Soweuseoneofthemtoreachtheintrreg+*/+ena_com_unmask_intr(rx_ring->ena_com_io_cq,&intr_reg);+}-/* It is a shared MSI-X. Tx and Rx CQ have pointer to it.-*Soweuseoneofthemtoreachtheintrreg-*/-ena_com_unmask_intr(rx_ring->ena_com_io_cq,&intr_reg);ena_update_ring_numa_node(tx_ring,rx_ring);
ena_flow_data_to_flow_hash and ena_flow_hash_to_flow_type
treat the ena_flow_hash_to_flow_type enum as power of two values.
Change the values of ena_admin_flow_hash_fields to be power of two values.
This bug effect the ethtool set/get rxnfc.
ethtool will report wrong values hash fields for get and will
configure wrong hash fields in set.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_admin_defs.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
The ENA driver tries to open a queue per vCPU.
To determine how many vCPUs the instance have it uses num_possible_cpus()
while it should have use num_online_cpus() instead.
Signed-off-by: Netanel Belgazal <redacted>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Changes between V3 and V2:
* Fix typos and correct alignment in commit messages.
* use napi_complete_done() return value to determine when the napi
handler needs to unmask the interrupts rather than implementing
non standard solution.
* Remove new features from this patchset and leave bug fixes only.
* Give example in the commit message for kernel crashes.
* Use BIT(x) instead of use the value explicitly.
This series does not apply cleanly to net-next, please respin.