[PATCH net] net/ibmvnic: Fix RTNL deadlock during device reset
Commit a5681e20b541 ("net/ibmnvic: Fix deadlock problem
in reset") made the change to hold the RTNL lock during
driver reset but still calls netdev_notify_peers, which
results in a deadlock. Instead, use call_netdevice_notifiers,
which is functionally the same except that it does not
take the RTNL lock again.
Fixes: a5681e20b541 ("net/ibmnvic: Fix deadlock problem in reset")
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index c0203a0..ed50b8d 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1859,7 +1859,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (adapter->reset_reason != VNIC_RESET_FAILOVER &&
adapter->reset_reason != VNIC_RESET_CHANGE_PARAM)
- netdev_notify_peers(netdev);
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, netdev);
netif_carrier_on(netdev);
--
1.8.3.1
[PATCH net-next 2/3] ibmvnic: Introduce driver limits for ring sizes
Introduce driver-defined maximums for queue ring sizes. Devices available for IBM vNIC today will likely not allow this amount, but this should give us some leeway for future devices that may support larger ring sizes. Signed-off-by: Thomas Falcon --- drivers/net/ethernet/ibm/ibmvnic.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index 2c1787109f1c..f9a12e5843c4 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -40,6 +40,7 @@ /* when changing this, update IBMVNIC_IO_ENTITLEMENT_DEFAULT */ #define IBMVNIC_BUFFS_PER_POOL 100 #define IBMVNIC_MAX_QUEUES 16 +#define IBMVNIC_MAX_QUEUE_SZ 4096 #define IBMVNIC_TSO_BUF_SZ 65536 #define IBMVNIC_TSO_BUFS 64 -- 2.12.3
[PATCH net-next 1/3] ibmvnic: Increase maximum queue size limit
Increase queue size limit to 16. Devices available for IBM vNIC today will not allow this amount, but this should give us some leeway for future devices that may support more RX or TX queues. Signed-off-by: Thomas Falcon --- drivers/net/ethernet/ibm/ibmvnic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h index f06eec145ca6..2c1787109f1c 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.h +++ b/drivers/net/ethernet/ibm/ibmvnic.h @@ -39,7 +39,7 @@ #define IBMVNIC_RX_WEIGHT 16 /* when changing this, update IBMVNIC_IO_ENTITLEMENT_DEFAULT */ #define IBMVNIC_BUFFS_PER_POOL 100 -#define IBMVNIC_MAX_QUEUES 10 +#define IBMVNIC_MAX_QUEUES 16 #define IBMVNIC_TSO_BUF_SZ 65536 #define IBMVNIC_TSO_BUFS 64 -- 2.12.3
[PATCH net-next 0/3] ibmvnic: Implement driver-defined queue limits
In this patch series, update the ibmvnic driver to use driver-defined queue limits instead of limits imposed by the Virtual I/O server management partition. For some deviced, initial max queue size and amount limits, despite their definition, can actually be exceeded if the client driver requests it. With this in mind, define a private ethtool flag that toggles the use of driver-defined limits. These limits are currently more than what supported hardware will likely allow, so the driver will attempt to get as close as possible to the user request but may not fully succeed. Thomas Falcon (3): ibmvnic: Increase maximum queue size limit ibmvnic: Introduce driver limits for ring sizes ibmvnic: Add ethtool private flag for driver-defined queue limits drivers/net/ethernet/ibm/ibmvnic.c | 129 +++-- drivers/net/ethernet/ibm/ibmvnic.h | 9 ++- 2 files changed, 102 insertions(+), 36 deletions(-) -- 2.12.3
[PATCH net-next 3/3] ibmvnic: Add ethtool private flag for driver-defined queue limits
When choosing channel amounts and ring sizes, the maximums in the
ibmvnic driver are defined by the virtual i/o server management
partition. Even though they are defined as maximums, the client
driver may in fact successfully request resources that exceed
these limits, which are mostly dependent on a user's hardware
With this in mind, provide an ethtool flag that when enabled will
allow the user to request resources limited by driver-defined
maximums instead of limits defined by the management partition.
The driver will try to honor the user's request but may not allowed
by the management partition. In this case, the driver requests
as close as it can get to the desired amount until it succeeds.
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 129 +++--
drivers/net/ethernet/ibm/ibmvnic.h | 6 ++
2 files changed, 100 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index a8369addfe68..ad898e8eaca1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2364,8 +2364,13 @@ static void ibmvnic_get_ringparam(struct net_device
*netdev,
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- ring->rx_max_pending = adapter->max_rx_add_entries_per_subcrq;
- ring->tx_max_pending = adapter->max_tx_entries_per_subcrq;
+ if (adapter->priv_flags & IBMVNIC_USE_SERVER_MAXES) {
+ ring->rx_max_pending = adapter->max_rx_add_entries_per_subcrq;
+ ring->tx_max_pending = adapter->max_tx_entries_per_subcrq;
+ } else {
+ ring->rx_max_pending = IBMVNIC_MAX_QUEUE_SZ;
+ ring->tx_max_pending = IBMVNIC_MAX_QUEUE_SZ;
+ }
ring->rx_mini_max_pending = 0;
ring->rx_jumbo_max_pending = 0;
ring->rx_pending = adapter->req_rx_add_entries_per_subcrq;
@@ -2378,21 +2383,23 @@ static int ibmvnic_set_ringparam(struct net_device
*netdev,
struct ethtool_ringparam *ring)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int ret;
- if (ring->rx_pending > adapter->max_rx_add_entries_per_subcrq ||
- ring->tx_pending > adapter->max_tx_entries_per_subcrq) {
- netdev_err(netdev, "Invalid request.\n");
- netdev_err(netdev, "Max tx buffers = %llu\n",
- adapter->max_rx_add_entries_per_subcrq);
- netdev_err(netdev, "Max rx buffers = %llu\n",
- adapter->max_tx_entries_per_subcrq);
- return -EINVAL;
- }
-
+ ret = 0;
adapter->desired.rx_entries = ring->rx_pending;
adapter->desired.tx_entries = ring->tx_pending;
- return wait_for_reset(adapter);
+ ret = wait_for_reset(adapter);
+
+ if (!ret &&
+ (adapter->req_rx_add_entries_per_subcrq != ring->rx_pending ||
+adapter->req_tx_entries_per_subcrq != ring->tx_pending))
+ netdev_info(netdev,
+ "Could not match full ringsize request. Requested:
RX %d, TX %d; Allowed: RX %llu, TX %llu\n",
+ ring->rx_pending, ring->tx_pending,
+ adapter->req_rx_add_entries_per_subcrq,
+ adapter->req_tx_entries_per_subcrq);
+ return ret;
}
static void ibmvnic_get_channels(struct net_device *netdev,
@@ -2400,8 +2407,14 @@ static void ibmvnic_get_channels(struct net_device
*netdev,
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- channels->max_rx = adapter->max_rx_queues;
- channels->max_tx = adapter->max_tx_queues;
+ if (adapter->priv_flags & IBMVNIC_USE_SERVER_MAXES) {
+ channels->max_rx = adapter->max_rx_queues;
+ channels->max_tx = adapter->max_tx_queues;
+ } else {
+ channels->max_rx = IBMVNIC_MAX_QUEUES;
+ channels->max_tx = IBMVNIC_MAX_QUEUES;
+ }
+
channels->max_other = 0;
channels->max_combined = 0;
channels->rx_count = adapter->req_rx_queues;
@@ -2414,11 +2427,23 @@ static int ibmvnic_set_channels(struct net_device
*netdev,
struct ethtool_channels *channels)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int ret;
+ ret = 0;
adapter->desired.rx_queues = channels->rx_count;
adapter->desired.tx_queues = channels->tx_count;
- return wait_for_reset(adapter);
+ ret = wait_for_reset(adapter);
+
+ if (!ret &&
+ (adapter->req_rx_queues != channels->rx_count ||
+adapter->req_tx_queues != channels->
[PATCH net] ibmvnic: Include missing return code checks in reset function
Check the return codes of these functions and halt reset
in case of failure. The driver will remain in a dormant state
until the next reset event, when device initialization will be
re-attempted.
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index ffe7acb..d834308 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1841,11 +1841,17 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->map_id = 1;
release_rx_pools(adapter);
release_tx_pools(adapter);
- init_rx_pools(netdev);
- init_tx_pools(netdev);
+ rc = init_rx_pools(netdev);
+ if (rc)
+ return rc;
+ rc = init_tx_pools(netdev);
+ if (rc)
+ return rc;
release_napi(adapter);
- init_napi(adapter);
+ rc = init_napi(adapter);
+ if (rc)
+ return rc;
} else {
rc = reset_tx_pools(adapter);
if (rc)
--
1.8.3.1
Re: [PATCH net-next 2/2] ibmvnic: Update firmware error reporting with cause string
On 08/06/2018 10:48 PM, Nathan Fontenot wrote:
On 08/06/2018 09:39 PM, Thomas Falcon wrote:
Print a string instead of the error code. Since there is a
possibility that the driver can recover, classify it as a
warning instead of an error.
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 34
++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 109e4a58efad..dafdd4ade705 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3825,15 +3825,41 @@ static void
handle_query_ip_offload_rsp(struct ibmvnic_adapter *adapter)
ibmvnic_send_crq(adapter, );
}
+static const char *ibmvnic_fw_err_cause(u16 cause)
+{
+ switch (cause) {
+ case ADAPTER_PROBLEM:
+ return "adapter problem";
+ case BUS_PROBLEM:
+ return "bus problem";
+ case FW_PROBLEM:
+ return "firmware problem";
+ case DD_PROBLEM:
+ return "device driver problem";
+ case EEH_RECOVERY:
+ return "EEH recovery";
+ case FW_UPDATED:
+ return "firmware updated";
+ case LOW_MEMORY:
+ return "low Memory";
+ default:
+ return "unknown";
+ }
+}
+
static void handle_error_indication(union ibmvnic_crq *crq,
struct ibmvnic_adapter *adapter)
{
struct device *dev = >vdev->dev;
+ u16 cause;
+
+ cause = be16_to_cpu(crq->error_indication.error_cause);
- dev_err(dev, "Firmware reports %serror, cause %d\n",
- crq->error_indication.flags
- & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
- be16_to_cpu(crq->error_indication.error_cause));
+ dev_warn_ratelimited(dev,
+ "Firmware reports %serror, cause: %s. Starting
recovery...\n",
^^^
You're going to want a space between after the %s here.
-Nathan
It does look odd at first glance, but there is a space after "FATAL" below.
Thanks,
Tom
+ crq->error_indication.flags
+ & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
+ ibmvnic_fw_err_cause(cause));
if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
ibmvnic_reset(adapter, VNIC_RESET_FATAL);
[PATCH net-next 0/2] ibmvnic: Update firmware error reporting
This patch set cleans out a lot of dead code from the ibmvnic driver and adds some more. The error ID field of the descriptor is not filled in by firmware, so do not print it and do not use it to query for more detailed information. Remove the unused code written for this. Finally, update the message to print a string explainng the error cause instead of just the error code. Thomas Falcon (2): ibmvnic: Remove code to request error information ibmvnic: Update firmware error reporting with cause string drivers/net/ethernet/ibm/ibmvnic.c | 168 ++--- drivers/net/ethernet/ibm/ibmvnic.h | 33 2 files changed, 26 insertions(+), 175 deletions(-) -- 2.12.3
[PATCH net-next 2/2] ibmvnic: Update firmware error reporting with cause string
Print a string instead of the error code. Since there is a
possibility that the driver can recover, classify it as a
warning instead of an error.
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 34 ++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 109e4a58efad..dafdd4ade705 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3825,15 +3825,41 @@ static void handle_query_ip_offload_rsp(struct
ibmvnic_adapter *adapter)
ibmvnic_send_crq(adapter, );
}
+static const char *ibmvnic_fw_err_cause(u16 cause)
+{
+ switch (cause) {
+ case ADAPTER_PROBLEM:
+ return "adapter problem";
+ case BUS_PROBLEM:
+ return "bus problem";
+ case FW_PROBLEM:
+ return "firmware problem";
+ case DD_PROBLEM:
+ return "device driver problem";
+ case EEH_RECOVERY:
+ return "EEH recovery";
+ case FW_UPDATED:
+ return "firmware updated";
+ case LOW_MEMORY:
+ return "low Memory";
+ default:
+ return "unknown";
+ }
+}
+
static void handle_error_indication(union ibmvnic_crq *crq,
struct ibmvnic_adapter *adapter)
{
struct device *dev = >vdev->dev;
+ u16 cause;
+
+ cause = be16_to_cpu(crq->error_indication.error_cause);
- dev_err(dev, "Firmware reports %serror, cause %d\n",
- crq->error_indication.flags
- & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
- be16_to_cpu(crq->error_indication.error_cause));
+ dev_warn_ratelimited(dev,
+"Firmware reports %serror, cause: %s. Starting
recovery...\n",
+crq->error_indication.flags
+ & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
+ibmvnic_fw_err_cause(cause));
if (crq->error_indication.flags & IBMVNIC_FATAL_ERROR)
ibmvnic_reset(adapter, VNIC_RESET_FATAL);
--
2.12.3
[PATCH net-next 1/2] ibmvnic: Remove code to request error information
When backing device firmware reports an error, it provides an
error ID, which is meant to be queried for more detailed error
information. Currently, however, an error ID is not provided by
the Virtual I/O server and there are not any plans to do so. For
now, it is always unfilled or zero, so request_error_information
will never be called. Remove it.
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 144 +
drivers/net/ethernet/ibm/ibmvnic.h | 33 -
2 files changed, 1 insertion(+), 176 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index ffe7acbeaa22..109e4a58efad 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -718,23 +718,6 @@ static int init_tx_pools(struct net_device *netdev)
return 0;
}
-static void release_error_buffers(struct ibmvnic_adapter *adapter)
-{
- struct device *dev = >vdev->dev;
- struct ibmvnic_error_buff *error_buff, *tmp;
- unsigned long flags;
-
- spin_lock_irqsave(>error_list_lock, flags);
- list_for_each_entry_safe(error_buff, tmp, >errors, list) {
- list_del(_buff->list);
- dma_unmap_single(dev, error_buff->dma, error_buff->len,
-DMA_FROM_DEVICE);
- kfree(error_buff->buff);
- kfree(error_buff);
- }
- spin_unlock_irqrestore(>error_list_lock, flags);
-}
-
static void ibmvnic_napi_enable(struct ibmvnic_adapter *adapter)
{
int i;
@@ -896,7 +879,6 @@ static void release_resources(struct ibmvnic_adapter
*adapter)
release_tx_pools(adapter);
release_rx_pools(adapter);
- release_error_buffers(adapter);
release_napi(adapter);
release_login_rsp_buffer(adapter);
}
@@ -3843,133 +3825,16 @@ static void handle_query_ip_offload_rsp(struct
ibmvnic_adapter *adapter)
ibmvnic_send_crq(adapter, );
}
-static void handle_error_info_rsp(union ibmvnic_crq *crq,
- struct ibmvnic_adapter *adapter)
-{
- struct device *dev = >vdev->dev;
- struct ibmvnic_error_buff *error_buff, *tmp;
- unsigned long flags;
- bool found = false;
- int i;
-
- if (!crq->request_error_rsp.rc.code) {
- dev_info(dev, "Request Error Rsp returned with rc=%x\n",
-crq->request_error_rsp.rc.code);
- return;
- }
-
- spin_lock_irqsave(>error_list_lock, flags);
- list_for_each_entry_safe(error_buff, tmp, >errors, list)
- if (error_buff->error_id == crq->request_error_rsp.error_id) {
- found = true;
- list_del(_buff->list);
- break;
- }
- spin_unlock_irqrestore(>error_list_lock, flags);
-
- if (!found) {
- dev_err(dev, "Couldn't find error id %x\n",
- be32_to_cpu(crq->request_error_rsp.error_id));
- return;
- }
-
- dev_err(dev, "Detailed info for error id %x:",
- be32_to_cpu(crq->request_error_rsp.error_id));
-
- for (i = 0; i < error_buff->len; i++) {
- pr_cont("%02x", (int)error_buff->buff[i]);
- if (i % 8 == 7)
- pr_cont(" ");
- }
- pr_cont("\n");
-
- dma_unmap_single(dev, error_buff->dma, error_buff->len,
-DMA_FROM_DEVICE);
- kfree(error_buff->buff);
- kfree(error_buff);
-}
-
-static void request_error_information(struct ibmvnic_adapter *adapter,
- union ibmvnic_crq *err_crq)
-{
- struct device *dev = >vdev->dev;
- struct net_device *netdev = adapter->netdev;
- struct ibmvnic_error_buff *error_buff;
- unsigned long timeout = msecs_to_jiffies(3);
- union ibmvnic_crq crq;
- unsigned long flags;
- int rc, detail_len;
-
- error_buff = kmalloc(sizeof(*error_buff), GFP_ATOMIC);
- if (!error_buff)
- return;
-
- detail_len = be32_to_cpu(err_crq->error_indication.detail_error_sz);
- error_buff->buff = kmalloc(detail_len, GFP_ATOMIC);
- if (!error_buff->buff) {
- kfree(error_buff);
- return;
- }
-
- error_buff->dma = dma_map_single(dev, error_buff->buff, detail_len,
-DMA_FROM_DEVICE);
- if (dma_mapping_error(dev, error_buff->dma)) {
- netdev_err(netdev, "Couldn't map error buffer\n");
- kfree(error_buff->buff);
- kfree(error_buff);
- return;
- }
-
- error_buff->len = detail_len;
- error_buff->error_
[PATCH net] ibmvnic: Revise RX/TX queue error messages
During a device failover, there may be latency between the loss
of the current backing device and a notification from firmware that
a failover has occurred. This latency can result in a large amount of
error printouts as firmware returns outgoing traffic with a generic
error code. These are not necessarily errors in this case as the
firmware is busy swapping in a new backing adapter and is not ready
to send packets yet. This patch reclassifies those error codes as
warnings with an explanation that a failover may be pending. All
other return codes will be considered errors.
Signed-off-by: Thomas Falcon
---
drivers/net/ethernet/ibm/ibmvnic.c | 39 ++
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index d0e196b..c5096368 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -329,7 +329,8 @@ static void replenish_rx_pool(struct ibmvnic_adapter
*adapter,
return;
failure:
- dev_info(dev, "replenish pools failure\n");
+ if (lpar_rc != H_PARAMETER && lpar_rc != H_CLOSED)
+ dev_err_ratelimited(dev, "rx: replenish packet buffer
failed\n");
pool->free_map[pool->next_free] = index;
pool->rx_buff[index].skb = NULL;
@@ -1617,7 +1618,8 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
_crq);
}
if (lpar_rc != H_SUCCESS) {
- dev_err(dev, "tx failed with code %ld\n", lpar_rc);
+ if (lpar_rc != H_CLOSED && lpar_rc != H_PARAMETER)
+ dev_err_ratelimited(dev, "tx: send failed\n");
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
@@ -3204,6 +3206,25 @@ static union ibmvnic_crq *ibmvnic_next_crq(struct
ibmvnic_adapter *adapter)
return crq;
}
+static void print_subcrq_error(struct device *dev, int rc, const char *func)
+{
+ switch (rc) {
+ case H_PARAMETER:
+ dev_warn_ratelimited(dev,
+"%s failed: Send request is malformed or
adapter failover pending. (rc=%d)\n",
+func, rc);
+ break;
+ case H_CLOSED:
+ dev_warn_ratelimited(dev,
+"%s failed: Backing queue closed. Adapter
is down or failover pending. (rc=%d)\n",
+func, rc);
+ break;
+ default:
+ dev_err_ratelimited(dev, "%s failed: (rc=%d)\n", func, rc);
+ break;
+ }
+}
+
static int send_subcrq(struct ibmvnic_adapter *adapter, u64 remote_handle,
union sub_crq *sub_crq)
{
@@ -3230,11 +3251,8 @@ static int send_subcrq(struct ibmvnic_adapter *adapter,
u64 remote_handle,
cpu_to_be64(u64_crq[2]),
cpu_to_be64(u64_crq[3]));
- if (rc) {
- if (rc == H_CLOSED)
- dev_warn(dev, "CRQ Queue closed\n");
- dev_err(dev, "Send error (rc=%d)\n", rc);
- }
+ if (rc)
+ print_subcrq_error(dev, rc, __func__);
return rc;
}
@@ -3252,11 +3270,8 @@ static int send_subcrq_indirect(struct ibmvnic_adapter
*adapter,
cpu_to_be64(remote_handle),
ioba, num_entries);
- if (rc) {
- if (rc == H_CLOSED)
- dev_warn(dev, "CRQ Queue closed\n");
- dev_err(dev, "Send (indirect) error (rc=%d)\n", rc);
- }
+ if (rc)
+ print_subcrq_error(dev, rc, __func__);
return rc;
}
--
1.8.3.1
[PATCH net] ibmvnic: Fix partial success login retries
In its current state, the driver will handle backing device
login in a loop for a certain number of retries while the
device returns a partial success, indicating that the driver
may need to try again using a smaller number of resources.
The variable it checks to continue retrying may change
over the course of operations, resulting in reallocation
of resources but exits without sending the login attempt.
Guard against this by introducing a boolean variable that
will retain the state indicating that the driver needs to
reattempt login with backing device firmware.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4bb4646..4382bff 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -796,9 +796,11 @@ static int ibmvnic_login(struct net_device *netdev)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
unsigned long timeout = msecs_to_jiffies(3);
int retry_count = 0;
+ bool retry;
int rc;
do {
+ retry = false;
if (retry_count > IBMVNIC_MAX_QUEUES) {
netdev_warn(netdev, "Login attempts exceeded\n");
return -1;
@@ -822,6 +824,9 @@ static int ibmvnic_login(struct net_device *netdev)
retry_count++;
release_sub_crqs(adapter, 1);
+ retry = true;
+ netdev_dbg(netdev,
+ "Received partial success, retrying...\n");
adapter->init_done_rc = 0;
reinit_completion(>init_done);
send_cap_queries(adapter);
@@ -849,7 +854,7 @@ static int ibmvnic_login(struct net_device *netdev)
netdev_warn(netdev, "Adapter login failed\n");
return -1;
}
- } while (adapter->init_done_rc == PARTIALSUCCESS);
+ } while (retry);
/* handle pending MAC address changes after successful login */
if (adapter->mac_change_pending) {
--
1.8.3.1
[PATCH net-next 6/8] ibmvnic: Create separate initialization routine for resets
Instead of having one initialization routine for all cases, create
a separate, simpler function for standard initialization, such as during
device probe. Use the original initialization function to handle
device reset scenarios. The goal of this patch is to avoid having
a single, cluttered init function to handle all possible
scenarios.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 48 --
1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index b1bbd5b..f26e1f8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -116,6 +116,7 @@ static void send_cap_queries(struct ibmvnic_adapter
*adapter);
static int init_sub_crqs(struct ibmvnic_adapter *);
static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter);
static int ibmvnic_init(struct ibmvnic_adapter *);
+static int ibmvnic_reset_init(struct ibmvnic_adapter *);
static void release_crq_queue(struct ibmvnic_adapter *);
static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p);
static int init_crq_queue(struct ibmvnic_adapter *adapter);
@@ -1807,7 +1808,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
return rc;
}
- rc = ibmvnic_init(adapter);
+ rc = ibmvnic_reset_init(adapter);
if (rc)
return IBMVNIC_INIT_FAILED;
@@ -4571,7 +4572,7 @@ static int init_crq_queue(struct ibmvnic_adapter *adapter)
return retrc;
}
-static int ibmvnic_init(struct ibmvnic_adapter *adapter)
+static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
{
struct device *dev = >vdev->dev;
unsigned long timeout = msecs_to_jiffies(3);
@@ -4630,6 +4631,49 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
return rc;
}
+static int ibmvnic_init(struct ibmvnic_adapter *adapter)
+{
+ struct device *dev = >vdev->dev;
+ unsigned long timeout = msecs_to_jiffies(3);
+ int rc;
+
+ adapter->from_passive_init = false;
+
+ init_completion(>init_done);
+ adapter->init_done_rc = 0;
+ ibmvnic_send_crq_init(adapter);
+ if (!wait_for_completion_timeout(>init_done, timeout)) {
+ dev_err(dev, "Initialization sequence timed out\n");
+ return -1;
+ }
+
+ if (adapter->init_done_rc) {
+ release_crq_queue(adapter);
+ return adapter->init_done_rc;
+ }
+
+ if (adapter->from_passive_init) {
+ adapter->state = VNIC_OPEN;
+ adapter->from_passive_init = false;
+ return -1;
+ }
+
+ rc = init_sub_crqs(adapter);
+ if (rc) {
+ dev_err(dev, "Initialization of sub crqs failed\n");
+ release_crq_queue(adapter);
+ return rc;
+ }
+
+ rc = init_sub_crq_irqs(adapter);
+ if (rc) {
+ dev_err(dev, "Failed to initialize sub crq irqs\n");
+ release_crq_queue(adapter);
+ }
+
+ return rc;
+}
+
static struct device_attribute dev_attr_failover;
static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
--
2.7.5
[PATCH net-next 4/8] ibmvnic: Return error code if init interrupted by transport event
If device init is interrupted by a failover, set the init return
code so that it can be checked and handled appropriately by the
init routine.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7083519..f1f744e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4249,7 +4249,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
dev_info(dev, "Partner initialized\n");
adapter->from_passive_init = true;
adapter->failover_pending = false;
- complete(>init_done);
+ if (!completion_done(>init_done)) {
+ complete(>init_done);
+ adapter->init_done_rc = -EIO;
+ }
ibmvnic_reset(adapter, VNIC_RESET_FAILOVER);
break;
case IBMVNIC_CRQ_INIT_COMPLETE:
--
2.7.5
[PATCH net-next 8/8] ibmvnic: Introduce hard reset recovery
Introduce a recovery hard reset to handle reset failure as a result of
change of device context following a transport event, such as a
backing device failover or partition migration. These operations reset
the device context to its initial state. If this occurs during a reset,
any initialization commands are likely to fail with an invalid state
error as backing device firmware requests reinitialization.
When this happens, make one more attempt by performing a hard reset,
which frees any resources currently allocated and performs device
initialization. If a transport event occurs during a device reset, a
flag is set which will trigger a new hard reset following the
completionof the current reset event.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 101 +++--
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index ee51deb..09f8e6b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1878,6 +1878,85 @@ static int do_reset(struct ibmvnic_adapter *adapter,
return 0;
}
+static int do_hard_reset(struct ibmvnic_adapter *adapter,
+struct ibmvnic_rwi *rwi, u32 reset_state)
+{
+ struct net_device *netdev = adapter->netdev;
+ int rc;
+
+ netdev_dbg(adapter->netdev, "Hard resetting driver (%d)\n",
+ rwi->reset_reason);
+
+ netif_carrier_off(netdev);
+ adapter->reset_reason = rwi->reset_reason;
+
+ ibmvnic_cleanup(netdev);
+ release_resources(adapter);
+ release_sub_crqs(adapter, 0);
+ release_crq_queue(adapter);
+
+ /* remove the closed state so when we call open it appears
+* we are coming from the probed state.
+*/
+ adapter->state = VNIC_PROBED;
+
+ rc = init_crq_queue(adapter);
+ if (rc) {
+ netdev_err(adapter->netdev,
+ "Couldn't initialize crq. rc=%d\n", rc);
+ return rc;
+ }
+
+ rc = ibmvnic_init(adapter);
+ if (rc)
+ return rc;
+
+ /* If the adapter was in PROBE state prior to the reset,
+* exit here.
+*/
+ if (reset_state == VNIC_PROBED)
+ return 0;
+
+ rc = ibmvnic_login(netdev);
+ if (rc) {
+ adapter->state = VNIC_PROBED;
+ return 0;
+ }
+ /* netif_set_real_num_xx_queues needs to take rtnl lock here
+* unless wait_for_reset is set, in which case the rtnl lock
+* has already been taken before initializing the reset
+*/
+ if (!adapter->wait_for_reset) {
+ rtnl_lock();
+ rc = init_resources(adapter);
+ rtnl_unlock();
+ } else {
+ rc = init_resources(adapter);
+ }
+ if (rc)
+ return rc;
+
+ ibmvnic_disable_irqs(adapter);
+ adapter->state = VNIC_CLOSED;
+
+ if (reset_state == VNIC_CLOSED)
+ return 0;
+
+ rc = __ibmvnic_open(netdev);
+ if (rc) {
+ if (list_empty(>rwi_list))
+ adapter->state = VNIC_CLOSED;
+ else
+ adapter->state = reset_state;
+
+ return 0;
+ }
+
+ netif_carrier_on(netdev);
+
+ return 0;
+}
+
static struct ibmvnic_rwi *get_next_rwi(struct ibmvnic_adapter *adapter)
{
struct ibmvnic_rwi *rwi;
@@ -1923,9 +2002,15 @@ static void __ibmvnic_reset(struct work_struct *work)
rwi = get_next_rwi(adapter);
while (rwi) {
- rc = do_reset(adapter, rwi, reset_state);
+ if (adapter->force_reset_recovery) {
+ adapter->force_reset_recovery = false;
+ rc = do_hard_reset(adapter, rwi, reset_state);
+ } else {
+ rc = do_reset(adapter, rwi, reset_state);
+ }
kfree(rwi);
- if (rc && rc != IBMVNIC_INIT_FAILED)
+ if (rc && rc != IBMVNIC_INIT_FAILED &&
+ !adapter->force_reset_recovery)
break;
rwi = get_next_rwi(adapter);
@@ -1951,9 +2036,9 @@ static void __ibmvnic_reset(struct work_struct *work)
static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
enum ibmvnic_reset_reason reason)
{
+ struct list_head *entry, *tmp_entry;
struct ibmvnic_rwi *rwi, *tmp;
struct net_device *netdev = adapter->netdev;
- struct list_head *entry;
int ret;
if (adapter->state == VNIC_REMOVING ||
@@ -1989,7 +2074,13 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
[PATCH net-next 7/8] ibmvnic: Set resetting state at earliest possible point
Set device resetting state at the earliest possible point: as soon as a reset is successfully scheduled. The reset state is toggled off when all resets have been processed to completion. Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index f26e1f8..ee51deb 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1919,7 +1919,6 @@ static void __ibmvnic_reset(struct work_struct *work) netdev = adapter->netdev; mutex_lock(>reset_lock); - adapter->resetting = true; reset_state = adapter->state; rwi = get_next_rwi(adapter); @@ -1994,7 +1993,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter, rwi->reset_reason = reason; list_add_tail(>list, >rwi_list); mutex_unlock(>rwi_lock); - + adapter->resetting = true; netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason); schedule_work(>ibmvnic_reset); -- 2.7.5
[PATCH net-next 5/8] ibmvnic: Handle error case when setting link state
If setting the link state is not successful, print a warning
with the resulting return code and return it to be handled
by the caller.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index f1f744e..b1bbd5b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -929,6 +929,10 @@ static int set_link_state(struct ibmvnic_adapter *adapter,
u8 link_state)
/* Partuial success, delay and re-send */
mdelay(1000);
resend = true;
+ } else if (adapter->init_done_rc) {
+ netdev_warn(netdev, "Unable to set link state, rc=%d\n",
+ adapter->init_done_rc);
+ return adapter->init_done_rc;
}
} while (resend);
--
2.7.5
[PATCH net-next 3/8] ibmvnic: Check CRQ command return codes
Check whether CRQ command is successful before awaiting a response
from the management partition. If the command was not successful, the
driver may hang waiting for a response that will never come.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 51 +++---
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index e6a081c..7083519 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -109,8 +109,8 @@ static union sub_crq *ibmvnic_next_scrq(struct
ibmvnic_adapter *,
struct ibmvnic_sub_crq_queue *);
static int ibmvnic_poll(struct napi_struct *napi, int data);
static void send_map_query(struct ibmvnic_adapter *adapter);
-static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
-static void send_request_unmap(struct ibmvnic_adapter *, u8);
+static int send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
+static int send_request_unmap(struct ibmvnic_adapter *, u8);
static int send_login(struct ibmvnic_adapter *adapter);
static void send_cap_queries(struct ibmvnic_adapter *adapter);
static int init_sub_crqs(struct ibmvnic_adapter *);
@@ -172,6 +172,7 @@ static int alloc_long_term_buff(struct ibmvnic_adapter
*adapter,
struct ibmvnic_long_term_buff *ltb, int size)
{
struct device *dev = >vdev->dev;
+ int rc;
ltb->size = size;
ltb->buff = dma_alloc_coherent(dev, ltb->size, >addr,
@@ -185,8 +186,12 @@ static int alloc_long_term_buff(struct ibmvnic_adapter
*adapter,
adapter->map_id++;
init_completion(>fw_done);
- send_request_map(adapter, ltb->addr,
-ltb->size, ltb->map_id);
+ rc = send_request_map(adapter, ltb->addr,
+ ltb->size, ltb->map_id);
+ if (rc) {
+ dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
+ return rc;
+ }
wait_for_completion(>fw_done);
if (adapter->fw_done_rc) {
@@ -215,10 +220,14 @@ static void free_long_term_buff(struct ibmvnic_adapter
*adapter,
static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
struct ibmvnic_long_term_buff *ltb)
{
+ int rc;
+
memset(ltb->buff, 0, ltb->size);
init_completion(>fw_done);
- send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+ rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+ if (rc)
+ return rc;
wait_for_completion(>fw_done);
if (adapter->fw_done_rc) {
@@ -952,6 +961,7 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
struct device *dev = >vdev->dev;
union ibmvnic_crq crq;
int len = 0;
+ int rc;
if (adapter->vpd->buff)
len = adapter->vpd->len;
@@ -959,7 +969,9 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
init_completion(>fw_done);
crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
crq.get_vpd_size.cmd = GET_VPD_SIZE;
- ibmvnic_send_crq(adapter, );
+ rc = ibmvnic_send_crq(adapter, );
+ if (rc)
+ return rc;
wait_for_completion(>fw_done);
if (!adapter->vpd->len)
@@ -992,7 +1004,12 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter
*adapter)
crq.get_vpd.cmd = GET_VPD;
crq.get_vpd.ioba = cpu_to_be32(adapter->vpd->dma_addr);
crq.get_vpd.len = cpu_to_be32((u32)adapter->vpd->len);
- ibmvnic_send_crq(adapter, );
+ rc = ibmvnic_send_crq(adapter, );
+ if (rc) {
+ kfree(adapter->vpd->buff);
+ adapter->vpd->buff = NULL;
+ return rc;
+ }
wait_for_completion(>fw_done);
return 0;
@@ -1691,6 +1708,7 @@ static int __ibmvnic_set_mac(struct net_device *netdev,
struct sockaddr *p)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
struct sockaddr *addr = p;
union ibmvnic_crq crq;
+ int rc;
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
@@ -1701,7 +1719,9 @@ static int __ibmvnic_set_mac(struct net_device *netdev,
struct sockaddr *p)
ether_addr_copy(_mac_addr.mac_addr[0], addr->sa_data);
init_completion(>fw_done);
- ibmvnic_send_crq(adapter, );
+ rc = ibmvnic_send_crq(adapter, );
+ if (rc)
+ return rc;
wait_for_completion(>fw_done);
/* netdev->dev_addr is changed in handle_change_mac_rsp function */
return adapter->fw_done_rc ? -EIO : 0;
@@ -2365,6 +2385,7 @@
[PATCH net-next 1/8] ibmvnic: Mark NAPI flag as disabled when released
Set adapter NAPI state as disabled if they are removed. This will allow them to be enabled again if reallocated in case of a hard reset. Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 4bb4646..eabc1e4 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -789,6 +789,7 @@ static void release_napi(struct ibmvnic_adapter *adapter) kfree(adapter->napi); adapter->napi = NULL; adapter->num_active_rx_napi = 0; + adapter->napi_enabled = false; } static int ibmvnic_login(struct net_device *netdev) -- 2.7.5
[PATCH net-next 2/8] ibmvnic: Introduce active CRQ state
Introduce an "active" state for a IBM vNIC Command-Response Queue. A CRQ
is considered active once it has initialized or linked with its partner by
sending an initialization request and getting a successful response back
from the management partition. Until this has happened, do not allow CRQ
commands to be sent other than the initialization request.
This change will avoid a protocol error in case of a device transport
event occurring during a initialization. When the driver receives a
transport event notification indicating that the backing hardware
has changed and needs reinitialization, any further commands other
than the initialization handshake with the VIOS management partition
will result in an invalid state error. Instead of sending a command
that will be returned with an error, print a warning and return an
error that will be handled by the caller.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 10 ++
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index eabc1e4..e6a081c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3147,6 +3147,12 @@ static int ibmvnic_send_crq(struct ibmvnic_adapter
*adapter,
(unsigned long int)cpu_to_be64(u64_crq[0]),
(unsigned long int)cpu_to_be64(u64_crq[1]));
+ if (!adapter->crq.active &&
+ crq->generic.first != IBMVNIC_CRQ_INIT_CMD) {
+ dev_warn(dev, "Invalid request detected while CRQ is inactive,
possible device state change during reset\n");
+ return -EINVAL;
+ }
+
/* Make sure the hypervisor sees the complete request */
mb();
@@ -4225,6 +4231,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
break;
case IBMVNIC_CRQ_INIT_COMPLETE:
dev_info(dev, "Partner initialization complete\n");
+ adapter->crq.active = true;
send_version_xchg(adapter);
break;
default:
@@ -4233,6 +4240,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
return;
case IBMVNIC_CRQ_XPORT_EVENT:
netif_carrier_off(netdev);
+ adapter->crq.active = false;
if (gen_crq->cmd == IBMVNIC_PARTITION_MIGRATED) {
dev_info(dev, "Migrated, re-enabling adapter\n");
ibmvnic_reset(adapter, VNIC_RESET_MOBILITY);
@@ -4420,6 +4428,7 @@ static int ibmvnic_reset_crq(struct ibmvnic_adapter
*adapter)
/* Clean out the queue */
memset(crq->msgs, 0, PAGE_SIZE);
crq->cur = 0;
+ crq->active = false;
/* And re-open it again */
rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
@@ -4454,6 +4463,7 @@ static void release_crq_queue(struct ibmvnic_adapter
*adapter)
DMA_BIDIRECTIONAL);
free_page((unsigned long)crq->msgs);
crq->msgs = NULL;
+ crq->active = false;
}
static int init_crq_queue(struct ibmvnic_adapter *adapter)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
b/drivers/net/ethernet/ibm/ibmvnic.h
index 22391e8..edfc312 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -865,6 +865,7 @@ struct ibmvnic_crq_queue {
int size, cur;
dma_addr_t msg_token;
spinlock_t lock;
+ bool active;
};
union sub_crq {
--
2.7.5
[PATCH net-next 0/8] ibmvnic: Failover hardening
Introduce additional transport event hardening to handle events during device reset. In the driver's current state, if a transport event is received during device reset, it can cause the device to become unresponsive as invalid operations are processed as the backing device context changes. After a transport event, the device expects a request to begin the initialization process. If the driver is still processing a previously queued device reset in this state, it is likely to fail as firmware will reject any commands other than the one to initialize the client driver's Command-Response Queue. Instead of failing and becoming dormant, the driver will make one more attempt to recover and continue operation. This is achieved by setting a state flag, which if true will direct the driver to clean up all allocated resources and perform a hard reset in an attempt to bring the driver back to an operational state. Thomas Falcon (8): ibmvnic: Mark NAPI flag as disabled when released ibmvnic: Introduce active CRQ state ibmvnic: Check CRQ command return codes ibmvnic: Return error code if init interrupted by transport event ibmvnic: Handle error case when setting link state ibmvnic: Create separate initialization routine for resets ibmvnic: Set resetting state at earliest possible point ibmvnic: Introduce hard reset recovery drivers/net/ethernet/ibm/ibmvnic.c | 223 + drivers/net/ethernet/ibm/ibmvnic.h | 2 + 2 files changed, 202 insertions(+), 23 deletions(-) -- 2.7.5
Re: [PATCH 0/3] ibmvnic: Fix bugs and memory leaks
On 05/16/2018 03:49 PM, Thomas Falcon wrote: > This is a small patch series fixing up some bugs and memory leaks > in the ibmvnic driver. The first fix frees up previously allocated > memory that should be freed in case of an error. The second fixes > a reset case that was failing due to TX/RX queue IRQ's being > erroneously disabled without being enabled again. The final patch > fixes incorrect reallocated of statistics buffers during a device > reset, resulting in loss of statistics information and a memory leak. > > Thomas Falcon (3): > ibmvnic: Free coherent DMA memory if FW map failed > ibmvnic: Fix non-fatal firmware error reset > ibmvnic: Fix statistics buffers memory leak Sorry, these are meant for the 'net' tree. Tom > > drivers/net/ethernet/ibm/ibmvnic.c | 28 +--- > 1 file changed, 17 insertions(+), 11 deletions(-) >
[PATCH 1/3] ibmvnic: Free coherent DMA memory if FW map failed
If the firmware map fails for whatever reason, remember to free
up the memory after.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 6e8d6a6..9e08917 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -192,6 +192,7 @@ static int alloc_long_term_buff(struct ibmvnic_adapter
*adapter,
if (adapter->fw_done_rc) {
dev_err(dev, "Couldn't map long term buffer,rc = %d\n",
adapter->fw_done_rc);
+ dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
return -1;
}
return 0;
--
1.8.3.1
[PATCH 2/3] ibmvnic: Fix non-fatal firmware error reset
It is not necessary to disable interrupt lines here during a reset to handle a non-fatal firmware error. Move that call within the code block that handles the other cases that do require interrupts to be disabled and re-enabled. Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 9e08917..1b9c22f 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -1822,9 +1822,8 @@ static int do_reset(struct ibmvnic_adapter *adapter, if (rc) return rc; } + ibmvnic_disable_irqs(adapter); } - - ibmvnic_disable_irqs(adapter); adapter->state = VNIC_CLOSED; if (reset_state == VNIC_CLOSED) -- 1.8.3.1
[PATCH 3/3] ibmvnic: Fix statistics buffers memory leak
Move initialization of statistics buffers from ibmvnic_init function into ibmvnic_probe. In the current state, ibmvnic_init will be called again during a device reset, resulting in the allocation of new buffers without freeing the old ones. Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 1b9c22f..4bb4646 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -4586,14 +4586,6 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) release_crq_queue(adapter); } - rc = init_stats_buffers(adapter); - if (rc) - return rc; - - rc = init_stats_token(adapter); - if (rc) - return rc; - return rc; } @@ -4662,13 +4654,21 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) goto ibmvnic_init_fail; } while (rc == EAGAIN); + rc = init_stats_buffers(adapter); + if (rc) + goto ibmvnic_init_fail; + + rc = init_stats_token(adapter); + if (rc) + goto ibmvnic_stats_fail; + netdev->mtu = adapter->req_mtu - ETH_HLEN; netdev->min_mtu = adapter->min_mtu - ETH_HLEN; netdev->max_mtu = adapter->max_mtu - ETH_HLEN; rc = device_create_file(>dev, _attr_failover); if (rc) - goto ibmvnic_init_fail; + goto ibmvnic_dev_file_err; netif_carrier_off(netdev); rc = register_netdev(netdev); @@ -4687,6 +4687,12 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id) ibmvnic_register_fail: device_remove_file(>dev, _attr_failover); +ibmvnic_dev_file_err: + release_stats_token(adapter); + +ibmvnic_stats_fail: + release_stats_buffers(adapter); + ibmvnic_init_fail: release_sub_crqs(adapter, 1); release_crq_queue(adapter); -- 1.8.3.1
[PATCH 0/3] ibmvnic: Fix bugs and memory leaks
This is a small patch series fixing up some bugs and memory leaks in the ibmvnic driver. The first fix frees up previously allocated memory that should be freed in case of an error. The second fixes a reset case that was failing due to TX/RX queue IRQ's being erroneously disabled without being enabled again. The final patch fixes incorrect reallocated of statistics buffers during a device reset, resulting in loss of statistics information and a memory leak. Thomas Falcon (3): ibmvnic: Free coherent DMA memory if FW map failed ibmvnic: Fix non-fatal firmware error reset ibmvnic: Fix statistics buffers memory leak drivers/net/ethernet/ibm/ibmvnic.c | 28 +--- 1 file changed, 17 insertions(+), 11 deletions(-) -- 1.8.3.1
[PATCH net] ibmvnic: Clean actual number of RX or TX pools
Avoid using value stored in the login response buffer when
cleaning TX and RX buffer pools since these could be inconsistent
depending on the device state. Instead use the field in the driver's
private data that tracks the number of active pools.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2df01ad..6e8d6a6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1128,7 +1128,7 @@ static void clean_rx_pools(struct ibmvnic_adapter
*adapter)
if (!adapter->rx_pool)
return;
- rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
+ rx_scrqs = adapter->num_active_rx_pools;
rx_entries = adapter->req_rx_add_entries_per_subcrq;
/* Free any remaining skbs in the rx buffer pools */
@@ -1177,7 +1177,7 @@ static void clean_tx_pools(struct ibmvnic_adapter
*adapter)
if (!adapter->tx_pool || !adapter->tso_pool)
return;
- tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
+ tx_scrqs = adapter->num_active_tx_pools;
/* Free any remaining skbs in the tx buffer pools */
for (i = 0; i < tx_scrqs; i++) {
--
1.8.3.1
[PATCH] ibmvnic: Clear pending interrupt after device reset
Due to a firmware bug, the hypervisor can send an interrupt to a
transmit or receive queue just prior to a partition migration, not
allowing the device enough time to handle it and send an EOI. When
the partition migrates, the interrupt is lost but an "EOI-pending"
flag for the interrupt line is still set in firmware. No further
interrupts will be sent until that flag is cleared, effectively
freezing that queue. To workaround this, the driver will disable the
hardware interrupt and send an H_EOI signal prior to re-enabling it.
This will flush the pending EOI and allow the driver to continue
operation.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index f84a920..ef7995fc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1034,16 +1034,14 @@ static int __ibmvnic_open(struct net_device *netdev)
netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
if (prev_state == VNIC_CLOSED)
enable_irq(adapter->rx_scrq[i]->irq);
- else
- enable_scrq_irq(adapter, adapter->rx_scrq[i]);
+ enable_scrq_irq(adapter, adapter->rx_scrq[i]);
}
for (i = 0; i < adapter->req_tx_queues; i++) {
netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
if (prev_state == VNIC_CLOSED)
enable_irq(adapter->tx_scrq[i]->irq);
- else
- enable_scrq_irq(adapter, adapter->tx_scrq[i]);
+ enable_scrq_irq(adapter, adapter->tx_scrq[i]);
}
rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP);
@@ -1184,6 +1182,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter
*adapter)
if (adapter->tx_scrq[i]->irq) {
netdev_dbg(netdev,
"Disabling tx_scrq[%d] irq\n", i);
+ disable_scrq_irq(adapter, adapter->tx_scrq[i]);
disable_irq(adapter->tx_scrq[i]->irq);
}
}
@@ -1193,6 +1192,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter
*adapter)
if (adapter->rx_scrq[i]->irq) {
netdev_dbg(netdev,
"Disabling rx_scrq[%d] irq\n", i);
+ disable_scrq_irq(adapter, adapter->rx_scrq[i]);
disable_irq(adapter->rx_scrq[i]->irq);
}
}
@@ -2601,12 +2601,19 @@ static int enable_scrq_irq(struct ibmvnic_adapter
*adapter,
{
struct device *dev = >vdev->dev;
unsigned long rc;
+ u64 val;
if (scrq->hw_irq > 0x1ULL) {
dev_err(dev, "bad hw_irq = %lx\n", scrq->hw_irq);
return 1;
}
+ val = (0xff00) | scrq->hw_irq;
+ rc = plpar_hcall_norets(H_EOI, val);
+ if (rc)
+ dev_err(dev, "H_EOI FAILED irq 0x%llx. rc=%ld\n",
+ val, rc);
+
rc = plpar_hcall_norets(H_VIOCTL, adapter->vdev->unit_address,
H_ENABLE_VIO_INTERRUPT, scrq->hw_irq, 0, 0);
if (rc)
--
1.8.3.1
Re: [PATCH] ibmvnic: Clear pending interrupt after device reset
On 04/15/2018 06:27 PM, Thomas Falcon wrote:
> Due to a firmware bug, the hypervisor can send an interrupt to a
> transmit or receive queue just prior to a partition migration, not
> allowing the device enough time to handle it and send an EOI. When
> the partition migrates, the interrupt is lost but an "EOI-pending"
> flag for the interrupt line is still set in firmware. No further
> interrupts will be sent until that flag is cleared, effectively
> freezing that queue. To workaround this, the driver will disable the
> hardware interrupt and send an H_EOI signal prior to re-enabling it.
> This will flush the pending EOI and allow the driver to continue
> operation.
Excuse me, I misspelled the linuxppc-dev email address.
Tom
> Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index f84a920..ef7995fc 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1034,16 +1034,14 @@ static int __ibmvnic_open(struct net_device *netdev)
> netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
> if (prev_state == VNIC_CLOSED)
> enable_irq(adapter->rx_scrq[i]->irq);
> - else
> - enable_scrq_irq(adapter, adapter->rx_scrq[i]);
> + enable_scrq_irq(adapter, adapter->rx_scrq[i]);
> }
>
> for (i = 0; i < adapter->req_tx_queues; i++) {
> netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
> if (prev_state == VNIC_CLOSED)
> enable_irq(adapter->tx_scrq[i]->irq);
> - else
> - enable_scrq_irq(adapter, adapter->tx_scrq[i]);
> + enable_scrq_irq(adapter, adapter->tx_scrq[i]);
> }
>
> rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP);
> @@ -1184,6 +1182,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter
> *adapter)
> if (adapter->tx_scrq[i]->irq) {
> netdev_dbg(netdev,
> "Disabling tx_scrq[%d] irq\n", i);
> + disable_scrq_irq(adapter, adapter->tx_scrq[i]);
> disable_irq(adapter->tx_scrq[i]->irq);
> }
> }
> @@ -1193,6 +1192,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter
> *adapter)
> if (adapter->rx_scrq[i]->irq) {
> netdev_dbg(netdev,
> "Disabling rx_scrq[%d] irq\n", i);
> + disable_scrq_irq(adapter, adapter->rx_scrq[i]);
> disable_irq(adapter->rx_scrq[i]->irq);
> }
> }
> @@ -2601,12 +2601,19 @@ static int enable_scrq_irq(struct ibmvnic_adapter
> *adapter,
> {
> struct device *dev = >vdev->dev;
> unsigned long rc;
> + u64 val;
>
> if (scrq->hw_irq > 0x1ULL) {
> dev_err(dev, "bad hw_irq = %lx\n", scrq->hw_irq);
> return 1;
> }
>
> + val = (0xff00) | scrq->hw_irq;
> + rc = plpar_hcall_norets(H_EOI, val);
> + if (rc)
> + dev_err(dev, "H_EOI FAILED irq 0x%llx. rc=%ld\n",
> + val, rc);
> +
> rc = plpar_hcall_norets(H_VIOCTL, adapter->vdev->unit_address,
> H_ENABLE_VIO_INTERRUPT, scrq->hw_irq, 0, 0);
> if (rc)
[PATCH] ibmvnic: Clear pending interrupt after device reset
Due to a firmware bug, the hypervisor can send an interrupt to a
transmit or receive queue just prior to a partition migration, not
allowing the device enough time to handle it and send an EOI. When
the partition migrates, the interrupt is lost but an "EOI-pending"
flag for the interrupt line is still set in firmware. No further
interrupts will be sent until that flag is cleared, effectively
freezing that queue. To workaround this, the driver will disable the
hardware interrupt and send an H_EOI signal prior to re-enabling it.
This will flush the pending EOI and allow the driver to continue
operation.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index f84a920..ef7995fc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1034,16 +1034,14 @@ static int __ibmvnic_open(struct net_device *netdev)
netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
if (prev_state == VNIC_CLOSED)
enable_irq(adapter->rx_scrq[i]->irq);
- else
- enable_scrq_irq(adapter, adapter->rx_scrq[i]);
+ enable_scrq_irq(adapter, adapter->rx_scrq[i]);
}
for (i = 0; i < adapter->req_tx_queues; i++) {
netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
if (prev_state == VNIC_CLOSED)
enable_irq(adapter->tx_scrq[i]->irq);
- else
- enable_scrq_irq(adapter, adapter->tx_scrq[i]);
+ enable_scrq_irq(adapter, adapter->tx_scrq[i]);
}
rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP);
@@ -1184,6 +1182,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter
*adapter)
if (adapter->tx_scrq[i]->irq) {
netdev_dbg(netdev,
"Disabling tx_scrq[%d] irq\n", i);
+ disable_scrq_irq(adapter, adapter->tx_scrq[i]);
disable_irq(adapter->tx_scrq[i]->irq);
}
}
@@ -1193,6 +1192,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter
*adapter)
if (adapter->rx_scrq[i]->irq) {
netdev_dbg(netdev,
"Disabling rx_scrq[%d] irq\n", i);
+ disable_scrq_irq(adapter, adapter->rx_scrq[i]);
disable_irq(adapter->rx_scrq[i]->irq);
}
}
@@ -2601,12 +2601,19 @@ static int enable_scrq_irq(struct ibmvnic_adapter
*adapter,
{
struct device *dev = >vdev->dev;
unsigned long rc;
+ u64 val;
if (scrq->hw_irq > 0x1ULL) {
dev_err(dev, "bad hw_irq = %lx\n", scrq->hw_irq);
return 1;
}
+ val = (0xff00) | scrq->hw_irq;
+ rc = plpar_hcall_norets(H_EOI, val);
+ if (rc)
+ dev_err(dev, "H_EOI FAILED irq 0x%llx. rc=%ld\n",
+ val, rc);
+
rc = plpar_hcall_norets(H_VIOCTL, adapter->vdev->unit_address,
H_ENABLE_VIO_INTERRUPT, scrq->hw_irq, 0, 0);
if (rc)
--
1.8.3.1
[PATCH net 3/5] ibmvnic: Fix reset scheduler error handling
In some cases, if the driver is waiting for a reset following
a device parameter change, failure to schedule a reset can result
in a hang since a completion signal is never sent.
If the device configuration is being altered by a tool such
as ethtool or ifconfig, it could cause the console to hang
if the reset request does not get scheduled. Add some additional
error handling code to exit the wait_for_completion if there is
one in progress.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 39 --
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 153a868..bbcd07a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1875,23 +1875,25 @@ static void __ibmvnic_reset(struct work_struct *work)
mutex_unlock(>reset_lock);
}
-static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
- enum ibmvnic_reset_reason reason)
+static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
+enum ibmvnic_reset_reason reason)
{
struct ibmvnic_rwi *rwi, *tmp;
struct net_device *netdev = adapter->netdev;
struct list_head *entry;
+ int ret;
if (adapter->state == VNIC_REMOVING ||
adapter->state == VNIC_REMOVED) {
+ ret = EBUSY;
netdev_dbg(netdev, "Adapter removing, skipping reset\n");
- return;
+ goto err;
}
if (adapter->state == VNIC_PROBING) {
netdev_warn(netdev, "Adapter reset during probe\n");
- adapter->init_done_rc = EAGAIN;
- return;
+ ret = adapter->init_done_rc = EAGAIN;
+ goto err;
}
mutex_lock(>rwi_lock);
@@ -1901,7 +1903,8 @@ static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
if (tmp->reset_reason == reason) {
netdev_dbg(netdev, "Skipping matching reset\n");
mutex_unlock(>rwi_lock);
- return;
+ ret = EBUSY;
+ goto err;
}
}
@@ -1909,7 +1912,8 @@ static void ibmvnic_reset(struct ibmvnic_adapter *adapter,
if (!rwi) {
mutex_unlock(>rwi_lock);
ibmvnic_close(netdev);
- return;
+ ret = ENOMEM;
+ goto err;
}
rwi->reset_reason = reason;
@@ -1918,6 +1922,12 @@ static void ibmvnic_reset(struct ibmvnic_adapter
*adapter,
netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
schedule_work(>ibmvnic_reset);
+
+ return 0;
+err:
+ if (adapter->wait_for_reset)
+ adapter->wait_for_reset = false;
+ return -ret;
}
static void ibmvnic_tx_timeout(struct net_device *dev)
@@ -2052,6 +2062,8 @@ static void ibmvnic_netpoll_controller(struct net_device
*dev)
static int wait_for_reset(struct ibmvnic_adapter *adapter)
{
+ int rc, ret;
+
adapter->fallback.mtu = adapter->req_mtu;
adapter->fallback.rx_queues = adapter->req_rx_queues;
adapter->fallback.tx_queues = adapter->req_tx_queues;
@@ -2059,11 +2071,15 @@ static int wait_for_reset(struct ibmvnic_adapter
*adapter)
adapter->fallback.tx_entries = adapter->req_tx_entries_per_subcrq;
init_completion(>reset_done);
- ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
adapter->wait_for_reset = true;
+ rc = ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
+ if (rc)
+ return rc;
wait_for_completion(>reset_done);
+ ret = 0;
if (adapter->reset_done_rc) {
+ ret = -EIO;
adapter->desired.mtu = adapter->fallback.mtu;
adapter->desired.rx_queues = adapter->fallback.rx_queues;
adapter->desired.tx_queues = adapter->fallback.tx_queues;
@@ -2071,12 +2087,15 @@ static int wait_for_reset(struct ibmvnic_adapter
*adapter)
adapter->desired.tx_entries = adapter->fallback.tx_entries;
init_completion(>reset_done);
- ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
+ adapter->wait_for_reset = true;
+ rc = ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
+ if (rc)
+ return ret;
wait_for_completion(>reset_done);
}
adapter->wait_for_reset = false;
- return adapter->reset_done_rc;
+ return ret;
}
static int ibmvnic_change_mtu(struct net_device *netdev, int new_mtu)
--
1.8.3.1
[PATCH net 5/5] ibmvnic: Do not reset CRQ for Mobility driver resets
From: Nathan Fontenot <nf...@linux.vnet.ibm.com>
When resetting the ibmvnic driver after a partition migration occurs
there is no requirement to do a reset of the main CRQ. The current
driver code does the required re-enable of the main CRQ, then does
a reset of the main CRQ later.
What we should be doing for a driver reset after a migration is to
re-enable the main CRQ, release all the sub-CRQs, and then allocate
new sub-CRQs after capability negotiation.
This patch updates the handling of mobility resets to do the proper
work and not reset the main CRQ. To do this the initialization/reset
of the main CRQ had to be moved out of the ibmvnic_init routine
and in to the ibmvnic_probe and do_reset routines.
Signed-off-by: Nathan Fontenot <nf...@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 55 ++
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 151542e..aad5658 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -118,6 +118,7 @@ static union sub_crq *ibmvnic_next_scrq(struct
ibmvnic_adapter *,
static int ibmvnic_init(struct ibmvnic_adapter *);
static void release_crq_queue(struct ibmvnic_adapter *);
static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p);
+static int init_crq_queue(struct ibmvnic_adapter *adapter);
struct ibmvnic_stat {
char name[ETH_GSTRING_LEN];
@@ -1224,7 +1225,6 @@ static int __ibmvnic_close(struct net_device *netdev)
rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
if (rc)
return rc;
- ibmvnic_cleanup(netdev);
adapter->state = VNIC_CLOSED;
return 0;
}
@@ -1244,6 +1244,7 @@ static int ibmvnic_close(struct net_device *netdev)
mutex_lock(>reset_lock);
rc = __ibmvnic_close(netdev);
+ ibmvnic_cleanup(netdev);
mutex_unlock(>reset_lock);
return rc;
@@ -1726,14 +1727,10 @@ static int do_reset(struct ibmvnic_adapter *adapter,
old_num_rx_queues = adapter->req_rx_queues;
old_num_tx_queues = adapter->req_tx_queues;
- if (rwi->reset_reason == VNIC_RESET_MOBILITY) {
- rc = ibmvnic_reenable_crq_queue(adapter);
- if (rc)
- return 0;
- ibmvnic_cleanup(netdev);
- } else if (rwi->reset_reason == VNIC_RESET_FAILOVER) {
- ibmvnic_cleanup(netdev);
- } else {
+ ibmvnic_cleanup(netdev);
+
+ if (adapter->reset_reason != VNIC_RESET_MOBILITY &&
+ adapter->reset_reason != VNIC_RESET_FAILOVER) {
rc = __ibmvnic_close(netdev);
if (rc)
return rc;
@@ -1752,6 +1749,23 @@ static int do_reset(struct ibmvnic_adapter *adapter,
*/
adapter->state = VNIC_PROBED;
+ if (adapter->wait_for_reset) {
+ rc = init_crq_queue(adapter);
+ } else if (adapter->reset_reason == VNIC_RESET_MOBILITY) {
+ rc = ibmvnic_reenable_crq_queue(adapter);
+ release_sub_crqs(adapter, 1);
+ } else {
+ rc = ibmvnic_reset_crq(adapter);
+ if (!rc)
+ rc = vio_enable_interrupts(adapter->vdev);
+ }
+
+ if (rc) {
+ netdev_err(adapter->netdev,
+ "Couldn't initialize crq. rc=%d\n", rc);
+ return rc;
+ }
+
rc = ibmvnic_init(adapter);
if (rc)
return IBMVNIC_INIT_FAILED;
@@ -4500,19 +4514,6 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
u64 old_num_rx_queues, old_num_tx_queues;
int rc;
- if (adapter->resetting && !adapter->wait_for_reset) {
- rc = ibmvnic_reset_crq(adapter);
- if (!rc)
- rc = vio_enable_interrupts(adapter->vdev);
- } else {
- rc = init_crq_queue(adapter);
- }
-
- if (rc) {
- dev_err(dev, "Couldn't initialize crq. rc=%d\n", rc);
- return rc;
- }
-
adapter->from_passive_init = false;
old_num_rx_queues = adapter->req_rx_queues;
@@ -4537,7 +4538,8 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
return -1;
}
- if (adapter->resetting && !adapter->wait_for_reset) {
+ if (adapter->resetting && !adapter->wait_for_reset &&
+ adapter->reset_reason != VNIC_RESET_MOBILITY) {
if (adapter->req_rx_queues != old_num_
[PATCH net 4/5] ibmvnic: Fix failover case for non-redundant configuration
There is a failover case for a non-redundant pseries VNIC
configuration that was not being handled properly. The current
implementation assumes that the driver will always have a redandant
device to communicate with following a failover notification. There
are cases, however, when a non-redundant configuration can receive
a failover request. If that happens, the driver should wait until
it receives a signal that the device is ready for operation.
The driver is agnostic of its backing hardware configuration,
so this fix necessarily affects all device failover management.
The driver needs to wait until it receives a signal that the device
is ready for resetting. A flag is introduced to track this intermediary
state where the driver is waiting for an active device.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 37 +
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index bbcd07a..151542e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -325,10 +325,11 @@ static void replenish_rx_pool(struct ibmvnic_adapter
*adapter,
adapter->replenish_add_buff_failure++;
atomic_add(buffers_added, >available);
- if (lpar_rc == H_CLOSED) {
+ if (lpar_rc == H_CLOSED || adapter->failover_pending) {
/* Disable buffer pool replenishment and report carrier off if
-* queue is closed. Firmware guarantees that a signal will
-* be sent to the driver, triggering a reset.
+* queue is closed or pending failover.
+* Firmware guarantees that a signal will be sent to the
+* driver, triggering a reset.
*/
deactivate_rx_pools(adapter);
netif_carrier_off(adapter->netdev);
@@ -1068,6 +1069,14 @@ static int ibmvnic_open(struct net_device *netdev)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
int rc;
+ /* If device failover is pending, just set device state and return.
+* Device operation will be handled by reset routine.
+*/
+ if (adapter->failover_pending) {
+ adapter->state = VNIC_OPEN;
+ return 0;
+ }
+
mutex_lock(>reset_lock);
if (adapter->state != VNIC_CLOSED) {
@@ -1225,6 +1234,14 @@ static int ibmvnic_close(struct net_device *netdev)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
int rc;
+ /* If device failover is pending, just set device state and return.
+* Device operation will be handled by reset routine.
+*/
+ if (adapter->failover_pending) {
+ adapter->state = VNIC_CLOSED;
+ return 0;
+ }
+
mutex_lock(>reset_lock);
rc = __ibmvnic_close(netdev);
mutex_unlock(>reset_lock);
@@ -1559,8 +1576,9 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
- if (lpar_rc == H_CLOSED) {
- /* Disable TX and report carrier off if queue is closed.
+ if (lpar_rc == H_CLOSED || adapter->failover_pending) {
+ /* Disable TX and report carrier off if queue is closed
+* or pending failover.
* Firmware guarantees that a signal will be sent to the
* driver, triggering a reset or some other action.
*/
@@ -1884,9 +1902,10 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
int ret;
if (adapter->state == VNIC_REMOVING ||
- adapter->state == VNIC_REMOVED) {
+ adapter->state == VNIC_REMOVED ||
+ adapter->failover_pending) {
ret = EBUSY;
- netdev_dbg(netdev, "Adapter removing, skipping reset\n");
+ netdev_dbg(netdev, "Adapter removing or pending failover,
skipping reset\n");
goto err;
}
@@ -4162,7 +4181,9 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
case IBMVNIC_CRQ_INIT:
dev_info(dev, "Partner initialized\n");
adapter->from_passive_init = true;
+ adapter->failover_pending = false;
complete(>init_done);
+ ibmvnic_reset(adapter, VNIC_RESET_FAILOVER);
break;
case IBMVNIC_CRQ_INIT_COMPLETE:
dev_info(dev, "Partner initialization complete\n");
@@ -4179,7 +4200,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
[PATCH net 1/5] ibmvnic: Fix DMA mapping mistakes
Fix some mistakes caught by the DMA debugger. The first change
fixes a unnecessary unmap that should have been removed in an
earlier update. The next hunk fixes another bad unmap by zeroing
the bit checked to determine that an unmap is needed. The final
change fixes some buffers that are unmapped with the wrong
direction specified.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index b492af6..58e0143 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -320,9 +320,6 @@ static void replenish_rx_pool(struct ibmvnic_adapter
*adapter,
dev_info(dev, "replenish pools failure\n");
pool->free_map[pool->next_free] = index;
pool->rx_buff[index].skb = NULL;
- if (!dma_mapping_error(dev, dma_addr))
- dma_unmap_single(dev, dma_addr, pool->buff_size,
-DMA_FROM_DEVICE);
dev_kfree_skb_any(skb);
adapter->replenish_add_buff_failure++;
@@ -2574,7 +2571,7 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
union sub_crq *next;
int index;
int i, j;
- u8 first;
+ u8 *first;
restart_loop:
while (pending_scrq(adapter, scrq)) {
@@ -2605,11 +2602,12 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
txbuff->data_dma[j] = 0;
}
/* if sub_crq was sent indirectly */
- first = txbuff->indir_arr[0].generic.first;
- if (first == IBMVNIC_CRQ_CMD) {
+ first = >indir_arr[0].generic.first;
+ if (*first == IBMVNIC_CRQ_CMD) {
dma_unmap_single(dev, txbuff->indir_dma,
sizeof(txbuff->indir_arr),
DMA_TO_DEVICE);
+ *first = 0;
}
if (txbuff->last_frag) {
@@ -3882,9 +3880,9 @@ static int handle_login_rsp(union ibmvnic_crq
*login_rsp_crq,
int i;
dma_unmap_single(dev, adapter->login_buf_token, adapter->login_buf_sz,
-DMA_BIDIRECTIONAL);
+DMA_TO_DEVICE);
dma_unmap_single(dev, adapter->login_rsp_buf_token,
-adapter->login_rsp_buf_sz, DMA_BIDIRECTIONAL);
+adapter->login_rsp_buf_sz, DMA_FROM_DEVICE);
/* If the number of queues requested can't be allocated by the
* server, the login response will return with code 1. We will need
--
1.8.3.1
[PATCH net 2/5] ibmvnic: Zero used TX descriptor counter on reset
The counter that tracks used TX descriptors pending completion needs to be zeroed as part of a device reset. This change fixes a bug causing transmit queues to be stopped unnecessarily and in some cases a transmit queue stall and timeout reset. If the counter is not reset, the remaining descriptors will not be "removed", effectively reducing queue capacity. If the queue is over half full, it will cause the queue to stall if stopped. Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 58e0143..153a868 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -2361,6 +2361,7 @@ static int reset_one_sub_crq_queue(struct ibmvnic_adapter *adapter, } memset(scrq->msgs, 0, 4 * PAGE_SIZE); + atomic_set(>used, 0); scrq->cur = 0; rc = h_reg_sub_crq(adapter->vdev->unit_address, scrq->msg_token, -- 1.8.3.1
[PATCH net 0/5] ibmvnic: Fix driver reset and DMA bugs
This patch series introduces some fixes to the driver reset routines and a patch that fixes mistakes caught by the kernel DMA debugger. The reset fixes include a fix to reset TX queue counters properly after a reset as well as updates to driver reset error-handling code. It also provides updates to the reset handling routine for redundant backing VF failover and partition migration cases. Nathan Fontenot (1): ibmvnic: Do not reset CRQ for Mobility driver resets Thomas Falcon (4): ibmvnic: Fix DMA mapping mistakes ibmvnic: Zero used TX descriptor counter on reset ibmvnic: Fix reset scheduler error handling ibmvnic: Fix failover case for non-redundant configuration drivers/net/ethernet/ibm/ibmvnic.c | 146 - drivers/net/ethernet/ibm/ibmvnic.h | 1 + 2 files changed, 98 insertions(+), 49 deletions(-) -- 1.8.3.1
Re: [PATCH net-next] ibmvnic: Potential NULL dereference in clean_one_tx_pool()
On 03/23/2018 06:36 AM, Dan Carpenter wrote:
> There is an && vs || typo here, which potentially leads to a NULL
> dereference.
Thanks for catching that!
>
> Fixes: e9e1e97884b7 ("ibmvnic: Update TX pool cleaning routine")
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5632c030811b..0389a7a52152 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1135,7 +1135,7 @@ static void clean_one_tx_pool(struct ibmvnic_adapter
> *adapter,
> u64 tx_entries;
> int i;
>
> - if (!tx_pool && !tx_pool->tx_buff)
> + if (!tx_pool || !tx_pool->tx_buff)
> return;
>
> tx_entries = tx_pool->num_buffers;
>
[PATCH net-next v5 1/8] ibmvnic: Generalize TX pool structure
Remove some unused fields in the structure and include values
describing the individual buffer size and number of buffers in
a TX pool. This allows us to use these fields for TX pool buffer
accounting as opposed to using hard coded values. Include a new
pool array for TSO transmissions.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
v5: Remove TSO resources in a later patch when they are unused.
---
drivers/net/ethernet/ibm/ibmvnic.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
b/drivers/net/ethernet/ibm/ibmvnic.h
index 099c89d49945..d287dc78db45 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -917,11 +917,11 @@ struct ibmvnic_tx_pool {
int *free_map;
int consumer_index;
int producer_index;
- wait_queue_head_t ibmvnic_tx_comp_q;
- struct task_struct *work_thread;
struct ibmvnic_long_term_buff long_term_buff;
struct ibmvnic_long_term_buff tso_ltb;
int tso_index;
+ int num_buffers;
+ int buf_size;
};
struct ibmvnic_rx_buff {
@@ -1044,6 +1044,7 @@ struct ibmvnic_adapter {
u64 promisc;
struct ibmvnic_tx_pool *tx_pool;
+ struct ibmvnic_tx_pool *tso_pool;
struct completion init_done;
int init_done_rc;
--
2.15.0
[PATCH net-next v5 6/8] ibmvnic: Improve TX buffer accounting
Improve TX pool buffer accounting to prevent the producer
index from overruning the consumer. First, set the next free
index to an invalid value if it is in use. If next buffer
to be consumed is in use, drop the packet.
Finally, if the transmit fails for some other reason, roll
back the consumer index and set the free map entry to its original
value. This should also be done if the DMA map fails.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 672e9221d4a5..af6f8193cb67 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1426,6 +1426,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
+ if (index == IBMVNIC_INVALID_MAP) {
+ dev_kfree_skb_any(skb);
+ tx_send_failed++;
+ tx_dropped++;
+ ret = NETDEV_TX_OK;
+ goto out;
+ }
+
+ tx_pool->free_map[tx_pool->consumer_index] = IBMVNIC_INVALID_MAP;
+
offset = index * tx_pool->buf_size;
dst = tx_pool->long_term_buff.buff + offset;
memset(dst, 0, tx_pool->buf_size);
@@ -1522,7 +1532,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_map_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
lpar_rc = send_subcrq_indirect(adapter, handle_array[queue_num],
(u64)tx_buff->indir_dma,
@@ -1534,13 +1544,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
if (lpar_rc != H_SUCCESS) {
dev_err(dev, "tx failed with code %ld\n", lpar_rc);
-
- if (tx_pool->consumer_index == 0)
- tx_pool->consumer_index =
- tx_pool->num_buffers - 1;
- else
- tx_pool->consumer_index--;
-
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
@@ -1556,7 +1559,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_send_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
if (atomic_add_return(num_entries, _scrq->used)
@@ -1569,7 +1572,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_bytes += skb->len;
txq->trans_start = jiffies;
ret = NETDEV_TX_OK;
+ goto out;
+tx_err_out:
+ /* roll back consumer index and map array*/
+ if (tx_pool->consumer_index == 0)
+ tx_pool->consumer_index =
+ tx_pool->num_buffers - 1;
+ else
+ tx_pool->consumer_index--;
+ tx_pool->free_map[tx_pool->consumer_index] = index;
out:
netdev->stats.tx_dropped += tx_dropped;
netdev->stats.tx_bytes += tx_bytes;
--
2.15.0
[PATCH net-next v5 3/8] ibmvnic: Update release TX pool routine
Introduce function that frees one TX pool. Use that to release
each pool in both the standard TX pool and TSO pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4dc304422ece..258d54e3a616 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -608,25 +608,30 @@ static void release_vpd_data(struct ibmvnic_adapter
*adapter)
adapter->vpd = NULL;
}
+static void release_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
+{
+ kfree(tx_pool->tx_buff);
+ kfree(tx_pool->free_map);
+ free_long_term_buff(adapter, _pool->long_term_buff);
+}
+
static void release_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int i;
if (!adapter->tx_pool)
return;
for (i = 0; i < adapter->num_active_tx_pools; i++) {
- netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
- tx_pool = >tx_pool[i];
- kfree(tx_pool->tx_buff);
- free_long_term_buff(adapter, _pool->long_term_buff);
- free_long_term_buff(adapter, _pool->tso_ltb);
- kfree(tx_pool->free_map);
+ release_one_tx_pool(adapter, >tx_pool[i]);
+ release_one_tx_pool(adapter, >tso_pool[i]);
}
kfree(adapter->tx_pool);
adapter->tx_pool = NULL;
+ kfree(adapter->tso_pool);
+ adapter->tso_pool = NULL;
adapter->num_active_tx_pools = 0;
}
--
2.15.0
[PATCH net-next v5 4/8] ibmvnic: Update TX pool initialization routine
Introduce function that initializes one TX pool. Use that to
create each pool entry in both the standard TX pool and TSO
pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 90 --
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 258d54e3a616..2bb5d562dde1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -635,13 +635,43 @@ static void release_tx_pools(struct ibmvnic_adapter
*adapter)
adapter->num_active_tx_pools = 0;
}
+static int init_one_tx_pool(struct net_device *netdev,
+ struct ibmvnic_tx_pool *tx_pool,
+ int num_entries, int buf_size)
+{
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int i;
+
+ tx_pool->tx_buff = kcalloc(num_entries,
+ sizeof(struct ibmvnic_tx_buff),
+ GFP_KERNEL);
+ if (!tx_pool->tx_buff)
+ return -1;
+
+ if (alloc_long_term_buff(adapter, _pool->long_term_buff,
+num_entries * buf_size))
+ return -1;
+
+ tx_pool->free_map = kcalloc(num_entries, sizeof(int), GFP_KERNEL);
+ if (!tx_pool->free_map)
+ return -1;
+
+ for (i = 0; i < num_entries; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+ tx_pool->num_buffers = num_entries;
+ tx_pool->buf_size = buf_size;
+
+ return 0;
+}
+
static int init_tx_pools(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- struct device *dev = >vdev->dev;
- struct ibmvnic_tx_pool *tx_pool;
int tx_subcrqs;
- int i, j;
+ int i, rc;
tx_subcrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
adapter->tx_pool = kcalloc(tx_subcrqs,
@@ -649,53 +679,29 @@ static int init_tx_pools(struct net_device *netdev)
if (!adapter->tx_pool)
return -1;
+ adapter->tso_pool = kcalloc(tx_subcrqs,
+ sizeof(struct ibmvnic_tx_pool), GFP_KERNEL);
+ if (!adapter->tso_pool)
+ return -1;
+
adapter->num_active_tx_pools = tx_subcrqs;
for (i = 0; i < tx_subcrqs; i++) {
- tx_pool = >tx_pool[i];
-
- netdev_dbg(adapter->netdev,
- "Initializing tx_pool[%d], %lld buffs\n",
- i, adapter->req_tx_entries_per_subcrq);
-
- tx_pool->tx_buff = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(struct ibmvnic_tx_buff),
- GFP_KERNEL);
- if (!tx_pool->tx_buff) {
- dev_err(dev, "tx pool buffer allocation failed\n");
- release_tx_pools(adapter);
- return -1;
- }
-
- if (alloc_long_term_buff(adapter, _pool->long_term_buff,
-adapter->req_tx_entries_per_subcrq *
-(adapter->req_mtu + VLAN_HLEN))) {
- release_tx_pools(adapter);
- return -1;
- }
-
- /* alloc TSO ltb */
- if (alloc_long_term_buff(adapter, _pool->tso_ltb,
-IBMVNIC_TSO_BUFS *
-IBMVNIC_TSO_BUF_SZ)) {
+ rc = init_one_tx_pool(netdev, >tx_pool[i],
+ adapter->req_tx_entries_per_subcrq,
+ adapter->req_mtu + VLAN_HLEN);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
- tx_pool->tso_index = 0;
-
- tx_pool->free_map = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(int), GFP_KERNEL);
- if (!tx_pool->free_map) {
+ init_one_tx_pool(netdev, >tso_pool[i],
+IBMVNIC_TSO_BUFS,
+IBMVNIC_TSO_BUF_SZ);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
}
return 0;
--
2.15.0
[PATCH net-next v5 8/8] ibmvnic: Remove unused TSO resources in TX pool structure
Finally, remove the TSO-specific fields in the TX pool
strcutures. These are no longer needed with the introduction
of separate buffer pools for TSO transmissions.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
b/drivers/net/ethernet/ibm/ibmvnic.h
index 566927e2974a..89efe700eafe 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -919,8 +919,6 @@ struct ibmvnic_tx_pool {
int consumer_index;
int producer_index;
struct ibmvnic_long_term_buff long_term_buff;
- struct ibmvnic_long_term_buff tso_ltb;
- int tso_index;
int num_buffers;
int buf_size;
};
--
2.15.0
[PATCH net-next v5 5/8] ibmvnic: Update TX and TX completion routines
Update TX and TX completion routines to account for TX pool
restructuring. TX routine first chooses the pool depending
on whether a packet is GSO or not, then uses it accordingly.
For the completion routine to know which pool it needs to use,
set the most significant bit of the correlator index to one
if the packet uses the TSO pool. On completion, unset the bit
and use the correlator index to release the buffer pool entry.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 55 +++---
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2bb5d562dde1..672e9221d4a5 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1414,8 +1414,11 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
ret = NETDEV_TX_OK;
goto out;
}
+ if (skb_is_gso(skb))
+ tx_pool = >tso_pool[queue_num];
+ else
+ tx_pool = >tx_pool[queue_num];
- tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
handle_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
@@ -1423,20 +1426,10 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
- if (skb_is_gso(skb)) {
- offset = tx_pool->tso_index * IBMVNIC_TSO_BUF_SZ;
- dst = tx_pool->tso_ltb.buff + offset;
- memset(dst, 0, IBMVNIC_TSO_BUF_SZ);
- data_dma_addr = tx_pool->tso_ltb.addr + offset;
- tx_pool->tso_index++;
- if (tx_pool->tso_index == IBMVNIC_TSO_BUFS)
- tx_pool->tso_index = 0;
- } else {
- offset = index * (adapter->req_mtu + VLAN_HLEN);
- dst = tx_pool->long_term_buff.buff + offset;
- memset(dst, 0, adapter->req_mtu + VLAN_HLEN);
- data_dma_addr = tx_pool->long_term_buff.addr + offset;
- }
+ offset = index * tx_pool->buf_size;
+ dst = tx_pool->long_term_buff.buff + offset;
+ memset(dst, 0, tx_pool->buf_size);
+ data_dma_addr = tx_pool->long_term_buff.addr + offset;
if (skb_shinfo(skb)->nr_frags) {
int cur, i;
@@ -1459,8 +1452,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
tx_pool->consumer_index =
- (tx_pool->consumer_index + 1) %
- adapter->req_tx_entries_per_subcrq;
+ (tx_pool->consumer_index + 1) % tx_pool->num_buffers;
tx_buff = _pool->tx_buff[index];
tx_buff->skb = skb;
@@ -1476,11 +1468,13 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_crq.v1.n_crq_elem = 1;
tx_crq.v1.n_sge = 1;
tx_crq.v1.flags1 = IBMVNIC_TX_COMP_NEEDED;
- tx_crq.v1.correlator = cpu_to_be32(index);
+
if (skb_is_gso(skb))
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->tso_ltb.map_id);
+ tx_crq.v1.correlator =
+ cpu_to_be32(index | IBMVNIC_TSO_POOL_MASK);
else
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
+ tx_crq.v1.correlator = cpu_to_be32(index);
+ tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
tx_crq.v1.sge_len = cpu_to_be32(skb->len);
tx_crq.v1.ioba = cpu_to_be64(data_dma_addr);
@@ -1543,7 +1537,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (tx_pool->consumer_index == 0)
tx_pool->consumer_index =
- adapter->req_tx_entries_per_subcrq - 1;
+ tx_pool->num_buffers - 1;
else
tx_pool->consumer_index--;
@@ -2547,6 +2541,7 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
struct ibmvnic_sub_crq_queue *scrq)
{
struct device *dev = >vdev->dev;
+ struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *txbuff;
union sub_crq *next;
int index;
@@ -2566,7 +2561,14 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
continue;
}
index = be32_to_cpu(next->tx_comp.correlators[i]);
- txbuff = >tx_pool[pool].tx_buff[index];
+ if (index & IBMVNIC_TSO_POOL_MASK) {
+
[PATCH net-next v5 2/8] ibmvnic: Update and clean up reset TX pool routine
Update TX pool reset routine to accommodate new TSO pool array. Introduce
a function that resets one TX pool, and use that function to initialize
each pool in both pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 45 +-
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 9c7d19c926f9..4dc304422ece 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -557,36 +557,41 @@ static int init_rx_pools(struct net_device *netdev)
return 0;
}
+static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
+struct ibmvnic_tx_pool *tx_pool)
+{
+ int rc, i;
+
+ rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ if (rc)
+ return rc;
+
+ memset(tx_pool->tx_buff, 0,
+ tx_pool->num_buffers *
+ sizeof(struct ibmvnic_tx_buff));
+
+ for (i = 0; i < tx_pool->num_buffers; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+
+ return 0;
+}
+
static int reset_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int tx_scrqs;
- int i, j, rc;
+ int i, rc;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
for (i = 0; i < tx_scrqs; i++) {
- netdev_dbg(adapter->netdev, "Re-setting tx_pool[%d]\n", i);
-
- tx_pool = >tx_pool[i];
-
- rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ rc = reset_one_tx_pool(adapter, >tso_pool[i]);
if (rc)
return rc;
-
- rc = reset_long_term_buff(adapter, _pool->tso_ltb);
+ rc = reset_one_tx_pool(adapter, >tx_pool[i]);
if (rc)
return rc;
-
- memset(tx_pool->tx_buff, 0,
- adapter->req_tx_entries_per_subcrq *
- sizeof(struct ibmvnic_tx_buff));
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
- tx_pool->tso_index = 0;
}
return 0;
--
2.15.0
[PATCH net-next v5 7/8] ibmvnic: Update TX pool cleaning routine
Update routine that cleans up any outstanding transmits that
have not received completions when the device needs to close.
Introduces a helper function that cleans one TX pool to make
code more readable.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 40 +++---
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index af6f8193cb67..5632c030811b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1128,34 +1128,42 @@ static void clean_rx_pools(struct ibmvnic_adapter
*adapter)
}
}
-static void clean_tx_pools(struct ibmvnic_adapter *adapter)
+static void clean_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
{
- struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *tx_buff;
u64 tx_entries;
+ int i;
+
+ if (!tx_pool && !tx_pool->tx_buff)
+ return;
+
+ tx_entries = tx_pool->num_buffers;
+
+ for (i = 0; i < tx_entries; i++) {
+ tx_buff = _pool->tx_buff[i];
+ if (tx_buff && tx_buff->skb) {
+ dev_kfree_skb_any(tx_buff->skb);
+ tx_buff->skb = NULL;
+ }
+ }
+}
+
+static void clean_tx_pools(struct ibmvnic_adapter *adapter)
+{
int tx_scrqs;
- int i, j;
+ int i;
- if (!adapter->tx_pool)
+ if (!adapter->tx_pool || !adapter->tso_pool)
return;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
- tx_entries = adapter->req_tx_entries_per_subcrq;
/* Free any remaining skbs in the tx buffer pools */
for (i = 0; i < tx_scrqs; i++) {
- tx_pool = >tx_pool[i];
- if (!tx_pool && !tx_pool->tx_buff)
- continue;
-
netdev_dbg(adapter->netdev, "Cleaning tx_pool[%d]\n", i);
- for (j = 0; j < tx_entries; j++) {
- tx_buff = _pool->tx_buff[j];
- if (tx_buff && tx_buff->skb) {
- dev_kfree_skb_any(tx_buff->skb);
- tx_buff->skb = NULL;
- }
- }
+ clean_one_tx_pool(adapter, >tx_pool[i]);
+ clean_one_tx_pool(adapter, >tso_pool[i]);
}
}
--
2.15.0
[PATCH net-next v5 0/8] ibmvnic: Update TX pool and TX routines
This patch restructures the TX pool data structure and provides a separate TX pool array for TSO transmissions. This is already used in some way due to our unique DMA situation, namely that we cannot use single DMA mappings for packet data. Previously, both buffer arrays used the same pool entry. This restructuring allows for some additional cleanup in the driver code, especially in some places in the device transmit routine. In addition, it allows us to more easily track the consumer and producer indexes of a particular pool. This has been further improved by better tracking of in-use buffers to prevent possible data corruption in case an invalid buffer entry is used. v5: Fix bisectability mistake in the first patch. Removed TSO-specific data in a later patch when it is no longer used. v4: Fix error in 7th patch that causes an oops by using the older fixed value for number of buffers instead of the respective field in the tx pool data structure v3: Forgot to update TX pool cleaning function to handle new data structures. Included 7th patch for that. v2: Fix typo in 3/6 commit subject line Thomas Falcon (8): ibmvnic: Generalize TX pool structure ibmvnic: Update and clean up reset TX pool routine ibmvnic: Update release TX pool routine ibmvnic: Update TX pool initialization routine ibmvnic: Update TX and TX completion routines ibmvnic: Improve TX buffer accounting ibmvnic: Update TX pool cleaning routine ibmvnic: Remove unused TSO resources in TX pool structure drivers/net/ethernet/ibm/ibmvnic.c | 275 + drivers/net/ethernet/ibm/ibmvnic.h | 8 +- 2 files changed, 160 insertions(+), 123 deletions(-) -- 2.15.0
Re: [PATCH net-next v3 1/7] ibmvnic: Generalize TX pool structure
On 03/16/2018 11:52 AM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Thu, 15 Mar 2018 11:02:37 -0500
>
>> Remove some unused fields in the structure and include values
>> describing the individual buffer size and number of buffers in
>> a TX pool. This allows us to use these fields for TX pool buffer
>> accounting as opposed to using hard coded values. Finally, split
>> TSO buffers out and provide an additional TX pool array for TSO.
>>
>> Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> ...
>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
>> b/drivers/net/ethernet/ibm/ibmvnic.h
>> index 099c89d..a2e21b3 100644
>> --- a/drivers/net/ethernet/ibm/ibmvnic.h
>> +++ b/drivers/net/ethernet/ibm/ibmvnic.h
>> @@ -917,11 +917,9 @@ struct ibmvnic_tx_pool {
>> int *free_map;
>> int consumer_index;
>> int producer_index;
>> -wait_queue_head_t ibmvnic_tx_comp_q;
>> -struct task_struct *work_thread;
>> struct ibmvnic_long_term_buff long_term_buff;
>> -struct ibmvnic_long_term_buff tso_ltb;
>> -int tso_index;
>> +int num_buffers;
>> +int buf_size;
>> };
>>
>> struct ibmvnic_rx_buff {
> Thomas, members like tso_ltb are used in the code at this point.
>
> You can't remove it here like this, because it makes your patch series
> non-bisectable. The tree should compile cleanly and work properly at
> each stage of your series.
>
> Thank you.
>
Thanks for your review, I'll send a reworked series soon.
[PATCH net-next v4 2/7] ibmvnic: Update and clean up reset TX pool routine
Update TX pool reset routine to accommodate new TSO pool array. Introduce
a function that resets one TX pool, and use that function to initialize
each pool in both pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 45 +-
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 9c7d19c926f9..4dc304422ece 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -557,36 +557,41 @@ static int init_rx_pools(struct net_device *netdev)
return 0;
}
+static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
+struct ibmvnic_tx_pool *tx_pool)
+{
+ int rc, i;
+
+ rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ if (rc)
+ return rc;
+
+ memset(tx_pool->tx_buff, 0,
+ tx_pool->num_buffers *
+ sizeof(struct ibmvnic_tx_buff));
+
+ for (i = 0; i < tx_pool->num_buffers; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+
+ return 0;
+}
+
static int reset_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int tx_scrqs;
- int i, j, rc;
+ int i, rc;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
for (i = 0; i < tx_scrqs; i++) {
- netdev_dbg(adapter->netdev, "Re-setting tx_pool[%d]\n", i);
-
- tx_pool = >tx_pool[i];
-
- rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ rc = reset_one_tx_pool(adapter, >tso_pool[i]);
if (rc)
return rc;
-
- rc = reset_long_term_buff(adapter, _pool->tso_ltb);
+ rc = reset_one_tx_pool(adapter, >tx_pool[i]);
if (rc)
return rc;
-
- memset(tx_pool->tx_buff, 0,
- adapter->req_tx_entries_per_subcrq *
- sizeof(struct ibmvnic_tx_buff));
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
- tx_pool->tso_index = 0;
}
return 0;
--
2.15.0
[PATCH net-next v4 6/7] ibmvnic: Improve TX buffer accounting
Improve TX pool buffer accounting to prevent the producer
index from overruning the consumer. First, set the next free
index to an invalid value if it is in use. If next buffer
to be consumed is in use, drop the packet.
Finally, if the transmit fails for some other reason, roll
back the consumer index and set the free map entry to its original
value. This should also be done if the DMA map fails.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 672e9221d4a5..af6f8193cb67 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1426,6 +1426,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
+ if (index == IBMVNIC_INVALID_MAP) {
+ dev_kfree_skb_any(skb);
+ tx_send_failed++;
+ tx_dropped++;
+ ret = NETDEV_TX_OK;
+ goto out;
+ }
+
+ tx_pool->free_map[tx_pool->consumer_index] = IBMVNIC_INVALID_MAP;
+
offset = index * tx_pool->buf_size;
dst = tx_pool->long_term_buff.buff + offset;
memset(dst, 0, tx_pool->buf_size);
@@ -1522,7 +1532,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_map_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
lpar_rc = send_subcrq_indirect(adapter, handle_array[queue_num],
(u64)tx_buff->indir_dma,
@@ -1534,13 +1544,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
if (lpar_rc != H_SUCCESS) {
dev_err(dev, "tx failed with code %ld\n", lpar_rc);
-
- if (tx_pool->consumer_index == 0)
- tx_pool->consumer_index =
- tx_pool->num_buffers - 1;
- else
- tx_pool->consumer_index--;
-
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
@@ -1556,7 +1559,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_send_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
if (atomic_add_return(num_entries, _scrq->used)
@@ -1569,7 +1572,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_bytes += skb->len;
txq->trans_start = jiffies;
ret = NETDEV_TX_OK;
+ goto out;
+tx_err_out:
+ /* roll back consumer index and map array*/
+ if (tx_pool->consumer_index == 0)
+ tx_pool->consumer_index =
+ tx_pool->num_buffers - 1;
+ else
+ tx_pool->consumer_index--;
+ tx_pool->free_map[tx_pool->consumer_index] = index;
out:
netdev->stats.tx_dropped += tx_dropped;
netdev->stats.tx_bytes += tx_bytes;
--
2.15.0
[PATCH net-next v4 4/7] ibmvnic: Update TX pool initialization routine
Introduce function that initializes one TX pool. Use that to
create each pool entry in both the standard TX pool and TSO
pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 90 --
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 258d54e3a616..2bb5d562dde1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -635,13 +635,43 @@ static void release_tx_pools(struct ibmvnic_adapter
*adapter)
adapter->num_active_tx_pools = 0;
}
+static int init_one_tx_pool(struct net_device *netdev,
+ struct ibmvnic_tx_pool *tx_pool,
+ int num_entries, int buf_size)
+{
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int i;
+
+ tx_pool->tx_buff = kcalloc(num_entries,
+ sizeof(struct ibmvnic_tx_buff),
+ GFP_KERNEL);
+ if (!tx_pool->tx_buff)
+ return -1;
+
+ if (alloc_long_term_buff(adapter, _pool->long_term_buff,
+num_entries * buf_size))
+ return -1;
+
+ tx_pool->free_map = kcalloc(num_entries, sizeof(int), GFP_KERNEL);
+ if (!tx_pool->free_map)
+ return -1;
+
+ for (i = 0; i < num_entries; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+ tx_pool->num_buffers = num_entries;
+ tx_pool->buf_size = buf_size;
+
+ return 0;
+}
+
static int init_tx_pools(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- struct device *dev = >vdev->dev;
- struct ibmvnic_tx_pool *tx_pool;
int tx_subcrqs;
- int i, j;
+ int i, rc;
tx_subcrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
adapter->tx_pool = kcalloc(tx_subcrqs,
@@ -649,53 +679,29 @@ static int init_tx_pools(struct net_device *netdev)
if (!adapter->tx_pool)
return -1;
+ adapter->tso_pool = kcalloc(tx_subcrqs,
+ sizeof(struct ibmvnic_tx_pool), GFP_KERNEL);
+ if (!adapter->tso_pool)
+ return -1;
+
adapter->num_active_tx_pools = tx_subcrqs;
for (i = 0; i < tx_subcrqs; i++) {
- tx_pool = >tx_pool[i];
-
- netdev_dbg(adapter->netdev,
- "Initializing tx_pool[%d], %lld buffs\n",
- i, adapter->req_tx_entries_per_subcrq);
-
- tx_pool->tx_buff = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(struct ibmvnic_tx_buff),
- GFP_KERNEL);
- if (!tx_pool->tx_buff) {
- dev_err(dev, "tx pool buffer allocation failed\n");
- release_tx_pools(adapter);
- return -1;
- }
-
- if (alloc_long_term_buff(adapter, _pool->long_term_buff,
-adapter->req_tx_entries_per_subcrq *
-(adapter->req_mtu + VLAN_HLEN))) {
- release_tx_pools(adapter);
- return -1;
- }
-
- /* alloc TSO ltb */
- if (alloc_long_term_buff(adapter, _pool->tso_ltb,
-IBMVNIC_TSO_BUFS *
-IBMVNIC_TSO_BUF_SZ)) {
+ rc = init_one_tx_pool(netdev, >tx_pool[i],
+ adapter->req_tx_entries_per_subcrq,
+ adapter->req_mtu + VLAN_HLEN);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
- tx_pool->tso_index = 0;
-
- tx_pool->free_map = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(int), GFP_KERNEL);
- if (!tx_pool->free_map) {
+ init_one_tx_pool(netdev, >tso_pool[i],
+IBMVNIC_TSO_BUFS,
+IBMVNIC_TSO_BUF_SZ);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
}
return 0;
--
2.15.0
[PATCH net-next v4 5/7] ibmvnic: Update TX and TX completion routines
Update TX and TX completion routines to account for TX pool
restructuring. TX routine first chooses the pool depending
on whether a packet is GSO or not, then uses it accordingly.
For the completion routine to know which pool it needs to use,
set the most significant bit of the correlator index to one
if the packet uses the TSO pool. On completion, unset the bit
and use the correlator index to release the buffer pool entry.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 55 +++---
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2bb5d562dde1..672e9221d4a5 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1414,8 +1414,11 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
ret = NETDEV_TX_OK;
goto out;
}
+ if (skb_is_gso(skb))
+ tx_pool = >tso_pool[queue_num];
+ else
+ tx_pool = >tx_pool[queue_num];
- tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
handle_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
@@ -1423,20 +1426,10 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
- if (skb_is_gso(skb)) {
- offset = tx_pool->tso_index * IBMVNIC_TSO_BUF_SZ;
- dst = tx_pool->tso_ltb.buff + offset;
- memset(dst, 0, IBMVNIC_TSO_BUF_SZ);
- data_dma_addr = tx_pool->tso_ltb.addr + offset;
- tx_pool->tso_index++;
- if (tx_pool->tso_index == IBMVNIC_TSO_BUFS)
- tx_pool->tso_index = 0;
- } else {
- offset = index * (adapter->req_mtu + VLAN_HLEN);
- dst = tx_pool->long_term_buff.buff + offset;
- memset(dst, 0, adapter->req_mtu + VLAN_HLEN);
- data_dma_addr = tx_pool->long_term_buff.addr + offset;
- }
+ offset = index * tx_pool->buf_size;
+ dst = tx_pool->long_term_buff.buff + offset;
+ memset(dst, 0, tx_pool->buf_size);
+ data_dma_addr = tx_pool->long_term_buff.addr + offset;
if (skb_shinfo(skb)->nr_frags) {
int cur, i;
@@ -1459,8 +1452,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
tx_pool->consumer_index =
- (tx_pool->consumer_index + 1) %
- adapter->req_tx_entries_per_subcrq;
+ (tx_pool->consumer_index + 1) % tx_pool->num_buffers;
tx_buff = _pool->tx_buff[index];
tx_buff->skb = skb;
@@ -1476,11 +1468,13 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_crq.v1.n_crq_elem = 1;
tx_crq.v1.n_sge = 1;
tx_crq.v1.flags1 = IBMVNIC_TX_COMP_NEEDED;
- tx_crq.v1.correlator = cpu_to_be32(index);
+
if (skb_is_gso(skb))
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->tso_ltb.map_id);
+ tx_crq.v1.correlator =
+ cpu_to_be32(index | IBMVNIC_TSO_POOL_MASK);
else
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
+ tx_crq.v1.correlator = cpu_to_be32(index);
+ tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
tx_crq.v1.sge_len = cpu_to_be32(skb->len);
tx_crq.v1.ioba = cpu_to_be64(data_dma_addr);
@@ -1543,7 +1537,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (tx_pool->consumer_index == 0)
tx_pool->consumer_index =
- adapter->req_tx_entries_per_subcrq - 1;
+ tx_pool->num_buffers - 1;
else
tx_pool->consumer_index--;
@@ -2547,6 +2541,7 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
struct ibmvnic_sub_crq_queue *scrq)
{
struct device *dev = >vdev->dev;
+ struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *txbuff;
union sub_crq *next;
int index;
@@ -2566,7 +2561,14 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
continue;
}
index = be32_to_cpu(next->tx_comp.correlators[i]);
- txbuff = >tx_pool[pool].tx_buff[index];
+ if (index & IBMVNIC_TSO_POOL_MASK) {
+
[PATCH net-next v4 7/7] ibmvnic: Update TX pool cleaning routine
Update routine that cleans up any outstanding transmits that
have not received completions when the device needs to close.
Introduces a helper function that cleans one TX pool to make
code more readable.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
v4: Update to use the number of buffers in the TX pool struct
instead of a fixed value saved in the adapter struct. Earlier
implementation resulted in a crash.
---
drivers/net/ethernet/ibm/ibmvnic.c | 40 +++---
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index af6f8193cb67..5632c030811b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1128,34 +1128,42 @@ static void clean_rx_pools(struct ibmvnic_adapter
*adapter)
}
}
-static void clean_tx_pools(struct ibmvnic_adapter *adapter)
+static void clean_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
{
- struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *tx_buff;
u64 tx_entries;
+ int i;
+
+ if (!tx_pool && !tx_pool->tx_buff)
+ return;
+
+ tx_entries = tx_pool->num_buffers;
+
+ for (i = 0; i < tx_entries; i++) {
+ tx_buff = _pool->tx_buff[i];
+ if (tx_buff && tx_buff->skb) {
+ dev_kfree_skb_any(tx_buff->skb);
+ tx_buff->skb = NULL;
+ }
+ }
+}
+
+static void clean_tx_pools(struct ibmvnic_adapter *adapter)
+{
int tx_scrqs;
- int i, j;
+ int i;
- if (!adapter->tx_pool)
+ if (!adapter->tx_pool || !adapter->tso_pool)
return;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
- tx_entries = adapter->req_tx_entries_per_subcrq;
/* Free any remaining skbs in the tx buffer pools */
for (i = 0; i < tx_scrqs; i++) {
- tx_pool = >tx_pool[i];
- if (!tx_pool && !tx_pool->tx_buff)
- continue;
-
netdev_dbg(adapter->netdev, "Cleaning tx_pool[%d]\n", i);
- for (j = 0; j < tx_entries; j++) {
- tx_buff = _pool->tx_buff[j];
- if (tx_buff && tx_buff->skb) {
- dev_kfree_skb_any(tx_buff->skb);
- tx_buff->skb = NULL;
- }
- }
+ clean_one_tx_pool(adapter, >tx_pool[i]);
+ clean_one_tx_pool(adapter, >tso_pool[i]);
}
}
--
2.15.0
[PATCH net-next v4 0/7] ibmvnic: Update TX pool and TX routines
This patch restructures the TX pool data structure and provides a separate TX pool array for TSO transmissions. This is already used in some way due to our unique DMA situation, namely that we cannot use single DMA mappings for packet data. Previously, both buffer arrays used the same pool entry. This restructuring allows for some additional cleanup in the driver code, especially in some places in the device transmit routine. In addition, it allows us to more easily track the consumer and producer indexes of a particular pool. This has been further improved by better tracking of in-use buffers to prevent possible data corruption in case an invalid buffer entry is used. v4: Fix error in 7th patch that causes an oops by using the older fixed value for number of buffers instead of the respective field in the tx pool data structure v3: Forgot to update TX pool cleaning function to handle new data structures. Included 7th patch for that. v2: Fix typo in 3/6 commit subject line Thomas Falcon (7): ibmvnic: Generalize TX pool structure ibmvnic: Update and clean up reset TX pool routine ibmvnic: Update release TX pool routine ibmvnic: Update TX pool initialization routine ibmvnic: Update TX and TX completion routines ibmvnic: Improve TX buffer accounting ibmvnic: Update TX pool cleaning routine drivers/net/ethernet/ibm/ibmvnic.c | 275 + drivers/net/ethernet/ibm/ibmvnic.h | 8 +- 2 files changed, 160 insertions(+), 123 deletions(-) -- 2.15.0
[PATCH net-next v4 3/7] ibmvnic: Update release TX pool routine
Introduce function that frees one TX pool. Use that to release
each pool in both the standard TX pool and TSO pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4dc304422ece..258d54e3a616 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -608,25 +608,30 @@ static void release_vpd_data(struct ibmvnic_adapter
*adapter)
adapter->vpd = NULL;
}
+static void release_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
+{
+ kfree(tx_pool->tx_buff);
+ kfree(tx_pool->free_map);
+ free_long_term_buff(adapter, _pool->long_term_buff);
+}
+
static void release_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int i;
if (!adapter->tx_pool)
return;
for (i = 0; i < adapter->num_active_tx_pools; i++) {
- netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
- tx_pool = >tx_pool[i];
- kfree(tx_pool->tx_buff);
- free_long_term_buff(adapter, _pool->long_term_buff);
- free_long_term_buff(adapter, _pool->tso_ltb);
- kfree(tx_pool->free_map);
+ release_one_tx_pool(adapter, >tx_pool[i]);
+ release_one_tx_pool(adapter, >tso_pool[i]);
}
kfree(adapter->tx_pool);
adapter->tx_pool = NULL;
+ kfree(adapter->tso_pool);
+ adapter->tso_pool = NULL;
adapter->num_active_tx_pools = 0;
}
--
2.15.0
[PATCH net-next v4 1/7] ibmvnic: Generalize TX pool structure
Remove some unused fields in the structure and include values
describing the individual buffer size and number of buffers in
a TX pool. This allows us to use these fields for TX pool buffer
accounting as opposed to using hard coded values. Finally, split
TSO buffers out and provide an additional TX pool array for TSO.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.h | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
b/drivers/net/ethernet/ibm/ibmvnic.h
index 099c89d49945..a2e21b39074f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -917,11 +917,9 @@ struct ibmvnic_tx_pool {
int *free_map;
int consumer_index;
int producer_index;
- wait_queue_head_t ibmvnic_tx_comp_q;
- struct task_struct *work_thread;
struct ibmvnic_long_term_buff long_term_buff;
- struct ibmvnic_long_term_buff tso_ltb;
- int tso_index;
+ int num_buffers;
+ int buf_size;
};
struct ibmvnic_rx_buff {
@@ -1044,6 +1042,7 @@ struct ibmvnic_adapter {
u64 promisc;
struct ibmvnic_tx_pool *tx_pool;
+ struct ibmvnic_tx_pool *tso_pool;
struct completion init_done;
int init_done_rc;
--
2.15.0
Re: [PATCH net-next v3 0/7] ibmvnic: Update TX pool and TX routines
On 03/15/2018 11:02 AM, Thomas Falcon wrote: > This patch restructures the TX pool data structure and provides a > separate TX pool array for TSO transmissions. This is already used > in some way due to our unique DMA situation, namely that we cannot > use single DMA mappings for packet data. Previously, both buffer > arrays used the same pool entry. This restructuring allows for > some additional cleanup in the driver code, especially in some > places in the device transmit routine. > > In addition, it allows us to more easily track the consumer > and producer indexes of a particular pool. This has been > further improved by better tracking of in-use buffers to > prevent possible data corruption in case an invalid buffer > entry is used. > > v3: Forgot to update TX pool cleaning function to handle new data > structures. Included 7th patch for that. > > v2: Fix typo in 3/6 commit subject line > > Thomas Falcon (7): > ibmvnic: Generalize TX pool structure > ibmvnic: Update and clean up reset TX pool routine > ibmvnic: Update release TX pool routine > ibmvnic: Update TX pool initialization routine > ibmvnic: Update TX and TX completion routines > ibmvnic: Improve TX buffer accounting > ibmvnic: Update TX pool cleaning routine > > drivers/net/ethernet/ibm/ibmvnic.c | 275 > + > drivers/net/ethernet/ibm/ibmvnic.h | 8 +- > 2 files changed, 160 insertions(+), 123 deletions(-) > Sorry again, I need to send another version because of a bug in the 7th patch.
[PATCH net-next v3 6/7] ibmvnic: Improve TX buffer accounting
Improve TX pool buffer accounting to prevent the producer
index from overruning the consumer. First, set the next free
index to an invalid value if it is in use. If next buffer
to be consumed is in use, drop the packet.
Finally, if the transmit fails for some other reason, roll
back the consumer index and set the free map entry to its original
value. This should also be done if the DMA map fails.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 672e922..af6f819 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1426,6 +1426,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
+ if (index == IBMVNIC_INVALID_MAP) {
+ dev_kfree_skb_any(skb);
+ tx_send_failed++;
+ tx_dropped++;
+ ret = NETDEV_TX_OK;
+ goto out;
+ }
+
+ tx_pool->free_map[tx_pool->consumer_index] = IBMVNIC_INVALID_MAP;
+
offset = index * tx_pool->buf_size;
dst = tx_pool->long_term_buff.buff + offset;
memset(dst, 0, tx_pool->buf_size);
@@ -1522,7 +1532,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_map_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
lpar_rc = send_subcrq_indirect(adapter, handle_array[queue_num],
(u64)tx_buff->indir_dma,
@@ -1534,13 +1544,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
if (lpar_rc != H_SUCCESS) {
dev_err(dev, "tx failed with code %ld\n", lpar_rc);
-
- if (tx_pool->consumer_index == 0)
- tx_pool->consumer_index =
- tx_pool->num_buffers - 1;
- else
- tx_pool->consumer_index--;
-
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
@@ -1556,7 +1559,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_send_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
if (atomic_add_return(num_entries, _scrq->used)
@@ -1569,7 +1572,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_bytes += skb->len;
txq->trans_start = jiffies;
ret = NETDEV_TX_OK;
+ goto out;
+tx_err_out:
+ /* roll back consumer index and map array*/
+ if (tx_pool->consumer_index == 0)
+ tx_pool->consumer_index =
+ tx_pool->num_buffers - 1;
+ else
+ tx_pool->consumer_index--;
+ tx_pool->free_map[tx_pool->consumer_index] = index;
out:
netdev->stats.tx_dropped += tx_dropped;
netdev->stats.tx_bytes += tx_bytes;
--
2.7.5
[PATCH net-next v3 3/7] ibmvnic: Update release TX pool routine
Introduce function that frees one TX pool. Use that to release
each pool in both the standard TX pool and TSO pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4dc3044..258d54e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -608,25 +608,30 @@ static void release_vpd_data(struct ibmvnic_adapter
*adapter)
adapter->vpd = NULL;
}
+static void release_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
+{
+ kfree(tx_pool->tx_buff);
+ kfree(tx_pool->free_map);
+ free_long_term_buff(adapter, _pool->long_term_buff);
+}
+
static void release_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int i;
if (!adapter->tx_pool)
return;
for (i = 0; i < adapter->num_active_tx_pools; i++) {
- netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
- tx_pool = >tx_pool[i];
- kfree(tx_pool->tx_buff);
- free_long_term_buff(adapter, _pool->long_term_buff);
- free_long_term_buff(adapter, _pool->tso_ltb);
- kfree(tx_pool->free_map);
+ release_one_tx_pool(adapter, >tx_pool[i]);
+ release_one_tx_pool(adapter, >tso_pool[i]);
}
kfree(adapter->tx_pool);
adapter->tx_pool = NULL;
+ kfree(adapter->tso_pool);
+ adapter->tso_pool = NULL;
adapter->num_active_tx_pools = 0;
}
--
2.7.5
[PATCH net-next v3 1/7] ibmvnic: Generalize TX pool structure
Remove some unused fields in the structure and include values
describing the individual buffer size and number of buffers in
a TX pool. This allows us to use these fields for TX pool buffer
accounting as opposed to using hard coded values. Finally, split
TSO buffers out and provide an additional TX pool array for TSO.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.h | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
b/drivers/net/ethernet/ibm/ibmvnic.h
index 099c89d..a2e21b3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -917,11 +917,9 @@ struct ibmvnic_tx_pool {
int *free_map;
int consumer_index;
int producer_index;
- wait_queue_head_t ibmvnic_tx_comp_q;
- struct task_struct *work_thread;
struct ibmvnic_long_term_buff long_term_buff;
- struct ibmvnic_long_term_buff tso_ltb;
- int tso_index;
+ int num_buffers;
+ int buf_size;
};
struct ibmvnic_rx_buff {
@@ -1044,6 +1042,7 @@ struct ibmvnic_adapter {
u64 promisc;
struct ibmvnic_tx_pool *tx_pool;
+ struct ibmvnic_tx_pool *tso_pool;
struct completion init_done;
int init_done_rc;
--
2.7.5
[PATCH net-next v3 5/7] ibmvnic: Update TX and TX completion routines
Update TX and TX completion routines to account for TX pool
restructuring. TX routine first chooses the pool depending
on whether a packet is GSO or not, then uses it accordingly.
For the completion routine to know which pool it needs to use,
set the most significant bit of the correlator index to one
if the packet uses the TSO pool. On completion, unset the bit
and use the correlator index to release the buffer pool entry.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 55 +++---
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2bb5d56..672e922 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1414,8 +1414,11 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
ret = NETDEV_TX_OK;
goto out;
}
+ if (skb_is_gso(skb))
+ tx_pool = >tso_pool[queue_num];
+ else
+ tx_pool = >tx_pool[queue_num];
- tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
handle_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
@@ -1423,20 +1426,10 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
- if (skb_is_gso(skb)) {
- offset = tx_pool->tso_index * IBMVNIC_TSO_BUF_SZ;
- dst = tx_pool->tso_ltb.buff + offset;
- memset(dst, 0, IBMVNIC_TSO_BUF_SZ);
- data_dma_addr = tx_pool->tso_ltb.addr + offset;
- tx_pool->tso_index++;
- if (tx_pool->tso_index == IBMVNIC_TSO_BUFS)
- tx_pool->tso_index = 0;
- } else {
- offset = index * (adapter->req_mtu + VLAN_HLEN);
- dst = tx_pool->long_term_buff.buff + offset;
- memset(dst, 0, adapter->req_mtu + VLAN_HLEN);
- data_dma_addr = tx_pool->long_term_buff.addr + offset;
- }
+ offset = index * tx_pool->buf_size;
+ dst = tx_pool->long_term_buff.buff + offset;
+ memset(dst, 0, tx_pool->buf_size);
+ data_dma_addr = tx_pool->long_term_buff.addr + offset;
if (skb_shinfo(skb)->nr_frags) {
int cur, i;
@@ -1459,8 +1452,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
tx_pool->consumer_index =
- (tx_pool->consumer_index + 1) %
- adapter->req_tx_entries_per_subcrq;
+ (tx_pool->consumer_index + 1) % tx_pool->num_buffers;
tx_buff = _pool->tx_buff[index];
tx_buff->skb = skb;
@@ -1476,11 +1468,13 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_crq.v1.n_crq_elem = 1;
tx_crq.v1.n_sge = 1;
tx_crq.v1.flags1 = IBMVNIC_TX_COMP_NEEDED;
- tx_crq.v1.correlator = cpu_to_be32(index);
+
if (skb_is_gso(skb))
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->tso_ltb.map_id);
+ tx_crq.v1.correlator =
+ cpu_to_be32(index | IBMVNIC_TSO_POOL_MASK);
else
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
+ tx_crq.v1.correlator = cpu_to_be32(index);
+ tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
tx_crq.v1.sge_len = cpu_to_be32(skb->len);
tx_crq.v1.ioba = cpu_to_be64(data_dma_addr);
@@ -1543,7 +1537,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (tx_pool->consumer_index == 0)
tx_pool->consumer_index =
- adapter->req_tx_entries_per_subcrq - 1;
+ tx_pool->num_buffers - 1;
else
tx_pool->consumer_index--;
@@ -2547,6 +2541,7 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
struct ibmvnic_sub_crq_queue *scrq)
{
struct device *dev = >vdev->dev;
+ struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *txbuff;
union sub_crq *next;
int index;
@@ -2566,7 +2561,14 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
continue;
}
index = be32_to_cpu(next->tx_comp.correlators[i]);
- txbuff = >tx_pool[pool].tx_buff[index];
+ if (index & IBMVNIC_TSO_POOL_MASK) {
+ tx_pool = >tso_pool[pool]
[PATCH net-next v3 7/7] ibmvnic: Update TX pool cleaning routine
Update routine that cleans up any outstanding transmits that
have not received completions when the device needs to close.
Introduces a helper function that cleans one TX pool to make
code more readable.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 40 +++---
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index af6f819..0e74546 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1128,34 +1128,42 @@ static void clean_rx_pools(struct ibmvnic_adapter
*adapter)
}
}
-static void clean_tx_pools(struct ibmvnic_adapter *adapter)
+static void clean_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
{
- struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *tx_buff;
u64 tx_entries;
+ int i;
+
+ if (!tx_pool && !tx_pool->tx_buff)
+ return;
+
+ tx_entries = adapter->req_tx_entries_per_subcrq;
+
+ for (i = 0; i < tx_entries; i++) {
+ tx_buff = _pool->tx_buff[i];
+ if (tx_buff && tx_buff->skb) {
+ dev_kfree_skb_any(tx_buff->skb);
+ tx_buff->skb = NULL;
+ }
+ }
+}
+
+static void clean_tx_pools(struct ibmvnic_adapter *adapter)
+{
int tx_scrqs;
- int i, j;
+ int i;
- if (!adapter->tx_pool)
+ if (!adapter->tx_pool || !adapter->tso_pool)
return;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
- tx_entries = adapter->req_tx_entries_per_subcrq;
/* Free any remaining skbs in the tx buffer pools */
for (i = 0; i < tx_scrqs; i++) {
- tx_pool = >tx_pool[i];
- if (!tx_pool && !tx_pool->tx_buff)
- continue;
-
netdev_dbg(adapter->netdev, "Cleaning tx_pool[%d]\n", i);
- for (j = 0; j < tx_entries; j++) {
- tx_buff = _pool->tx_buff[j];
- if (tx_buff && tx_buff->skb) {
- dev_kfree_skb_any(tx_buff->skb);
- tx_buff->skb = NULL;
- }
- }
+ clean_one_tx_pool(adapter, >tx_pool[i]);
+ clean_one_tx_pool(adapter, >tso_pool[i]);
}
}
--
2.7.5
[PATCH net-next v3 4/7] ibmvnic: Update TX pool initialization routine
Introduce function that initializes one TX pool. Use that to
create each pool entry in both the standard TX pool and TSO
pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 90 --
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 258d54e..2bb5d56 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -635,13 +635,43 @@ static void release_tx_pools(struct ibmvnic_adapter
*adapter)
adapter->num_active_tx_pools = 0;
}
+static int init_one_tx_pool(struct net_device *netdev,
+ struct ibmvnic_tx_pool *tx_pool,
+ int num_entries, int buf_size)
+{
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int i;
+
+ tx_pool->tx_buff = kcalloc(num_entries,
+ sizeof(struct ibmvnic_tx_buff),
+ GFP_KERNEL);
+ if (!tx_pool->tx_buff)
+ return -1;
+
+ if (alloc_long_term_buff(adapter, _pool->long_term_buff,
+num_entries * buf_size))
+ return -1;
+
+ tx_pool->free_map = kcalloc(num_entries, sizeof(int), GFP_KERNEL);
+ if (!tx_pool->free_map)
+ return -1;
+
+ for (i = 0; i < num_entries; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+ tx_pool->num_buffers = num_entries;
+ tx_pool->buf_size = buf_size;
+
+ return 0;
+}
+
static int init_tx_pools(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- struct device *dev = >vdev->dev;
- struct ibmvnic_tx_pool *tx_pool;
int tx_subcrqs;
- int i, j;
+ int i, rc;
tx_subcrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
adapter->tx_pool = kcalloc(tx_subcrqs,
@@ -649,53 +679,29 @@ static int init_tx_pools(struct net_device *netdev)
if (!adapter->tx_pool)
return -1;
+ adapter->tso_pool = kcalloc(tx_subcrqs,
+ sizeof(struct ibmvnic_tx_pool), GFP_KERNEL);
+ if (!adapter->tso_pool)
+ return -1;
+
adapter->num_active_tx_pools = tx_subcrqs;
for (i = 0; i < tx_subcrqs; i++) {
- tx_pool = >tx_pool[i];
-
- netdev_dbg(adapter->netdev,
- "Initializing tx_pool[%d], %lld buffs\n",
- i, adapter->req_tx_entries_per_subcrq);
-
- tx_pool->tx_buff = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(struct ibmvnic_tx_buff),
- GFP_KERNEL);
- if (!tx_pool->tx_buff) {
- dev_err(dev, "tx pool buffer allocation failed\n");
- release_tx_pools(adapter);
- return -1;
- }
-
- if (alloc_long_term_buff(adapter, _pool->long_term_buff,
-adapter->req_tx_entries_per_subcrq *
-(adapter->req_mtu + VLAN_HLEN))) {
- release_tx_pools(adapter);
- return -1;
- }
-
- /* alloc TSO ltb */
- if (alloc_long_term_buff(adapter, _pool->tso_ltb,
-IBMVNIC_TSO_BUFS *
-IBMVNIC_TSO_BUF_SZ)) {
+ rc = init_one_tx_pool(netdev, >tx_pool[i],
+ adapter->req_tx_entries_per_subcrq,
+ adapter->req_mtu + VLAN_HLEN);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
- tx_pool->tso_index = 0;
-
- tx_pool->free_map = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(int), GFP_KERNEL);
- if (!tx_pool->free_map) {
+ init_one_tx_pool(netdev, >tso_pool[i],
+IBMVNIC_TSO_BUFS,
+IBMVNIC_TSO_BUF_SZ);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
}
return 0;
--
2.7.5
[PATCH net-next v3 0/7] ibmvnic: Update TX pool and TX routines
This patch restructures the TX pool data structure and provides a separate TX pool array for TSO transmissions. This is already used in some way due to our unique DMA situation, namely that we cannot use single DMA mappings for packet data. Previously, both buffer arrays used the same pool entry. This restructuring allows for some additional cleanup in the driver code, especially in some places in the device transmit routine. In addition, it allows us to more easily track the consumer and producer indexes of a particular pool. This has been further improved by better tracking of in-use buffers to prevent possible data corruption in case an invalid buffer entry is used. v3: Forgot to update TX pool cleaning function to handle new data structures. Included 7th patch for that. v2: Fix typo in 3/6 commit subject line Thomas Falcon (7): ibmvnic: Generalize TX pool structure ibmvnic: Update and clean up reset TX pool routine ibmvnic: Update release TX pool routine ibmvnic: Update TX pool initialization routine ibmvnic: Update TX and TX completion routines ibmvnic: Improve TX buffer accounting ibmvnic: Update TX pool cleaning routine drivers/net/ethernet/ibm/ibmvnic.c | 275 + drivers/net/ethernet/ibm/ibmvnic.h | 8 +- 2 files changed, 160 insertions(+), 123 deletions(-) -- 2.7.5
[PATCH net-next v3 2/7] ibmvnic: Update and clean up reset TX pool routine
Update TX pool reset routine to accommodate new TSO pool array. Introduce
a function that resets one TX pool, and use that function to initialize
each pool in both pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 45 +-
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 9c7d19c..4dc3044 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -557,36 +557,41 @@ static int init_rx_pools(struct net_device *netdev)
return 0;
}
+static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
+struct ibmvnic_tx_pool *tx_pool)
+{
+ int rc, i;
+
+ rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ if (rc)
+ return rc;
+
+ memset(tx_pool->tx_buff, 0,
+ tx_pool->num_buffers *
+ sizeof(struct ibmvnic_tx_buff));
+
+ for (i = 0; i < tx_pool->num_buffers; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+
+ return 0;
+}
+
static int reset_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int tx_scrqs;
- int i, j, rc;
+ int i, rc;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
for (i = 0; i < tx_scrqs; i++) {
- netdev_dbg(adapter->netdev, "Re-setting tx_pool[%d]\n", i);
-
- tx_pool = >tx_pool[i];
-
- rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ rc = reset_one_tx_pool(adapter, >tso_pool[i]);
if (rc)
return rc;
-
- rc = reset_long_term_buff(adapter, _pool->tso_ltb);
+ rc = reset_one_tx_pool(adapter, >tx_pool[i]);
if (rc)
return rc;
-
- memset(tx_pool->tx_buff, 0,
- adapter->req_tx_entries_per_subcrq *
- sizeof(struct ibmvnic_tx_buff));
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
- tx_pool->tso_index = 0;
}
return 0;
--
2.7.5
Re: [PATCH net-next 0/6] ibmvnic: Update TX pool and TX routines
On 03/14/2018 01:03 PM, David Miller wrote: > From: Thomas Falcon <tlfal...@linux.vnet.ibm.com> > Date: Tue, 13 Mar 2018 19:34:17 -0500 > >> This patch restructures the TX pool data structure and provides a >> separate TX pool array for TSO transmissions. This is already used >> in some way due to our unique DMA situation, namely that we cannot >> use single DMA mappings for packet data. Previously, both buffer >> arrays used the same pool entry. This restructuring allows for >> some additional cleanup in the driver code, especially in some >> places in the device transmit routine. >> >> In addition, it allows us to more easily track the consumer >> and producer indexes of a particular pool. This has been >> further improved by better tracking of in-use buffers to >> prevent possible data corruption in case an invalid buffer >> entry is used. > ... >> ibmvnic: Update release RX pool routine > I think you need to fix up this Subject line to say TX instead > of RX. > > Thanks. > Drat, I missed that. I'll send another version shortly. Thanks.
[PATCH net-next 4/6] ibmvnic: Update TX pool initialization routine
Introduce function that initializes one TX pool. Use that to
create each pool entry in both the standard TX pool and TSO
pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 90 --
1 file changed, 48 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index e6e934e..cda5bf0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -635,13 +635,43 @@ static void release_tx_pools(struct ibmvnic_adapter
*adapter)
adapter->num_active_tx_pools = 0;
}
+static int init_one_tx_pool(struct net_device *netdev,
+ struct ibmvnic_tx_pool *tx_pool,
+ int num_entries, int buf_size)
+{
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int i;
+
+ tx_pool->tx_buff = kcalloc(num_entries,
+ sizeof(struct ibmvnic_tx_buff),
+ GFP_KERNEL);
+ if (!tx_pool->tx_buff)
+ return -1;
+
+ if (alloc_long_term_buff(adapter, _pool->long_term_buff,
+num_entries * buf_size))
+ return -1;
+
+ tx_pool->free_map = kcalloc(num_entries, sizeof(int), GFP_KERNEL);
+ if (!tx_pool->free_map)
+ return -1;
+
+ for (i = 0; i < num_entries; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+ tx_pool->num_buffers = num_entries;
+ tx_pool->buf_size = buf_size;
+
+ return 0;
+}
+
static int init_tx_pools(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- struct device *dev = >vdev->dev;
- struct ibmvnic_tx_pool *tx_pool;
int tx_subcrqs;
- int i, j;
+ int i, rc;
tx_subcrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
adapter->tx_pool = kcalloc(tx_subcrqs,
@@ -649,53 +679,29 @@ static int init_tx_pools(struct net_device *netdev)
if (!adapter->tx_pool)
return -1;
+ adapter->tso_pool = kcalloc(tx_subcrqs,
+ sizeof(struct ibmvnic_tx_pool), GFP_KERNEL);
+ if (!adapter->tso_pool)
+ return -1;
+
adapter->num_active_tx_pools = tx_subcrqs;
for (i = 0; i < tx_subcrqs; i++) {
- tx_pool = >tx_pool[i];
-
- netdev_dbg(adapter->netdev,
- "Initializing tx_pool[%d], %lld buffs\n",
- i, adapter->req_tx_entries_per_subcrq);
-
- tx_pool->tx_buff = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(struct ibmvnic_tx_buff),
- GFP_KERNEL);
- if (!tx_pool->tx_buff) {
- dev_err(dev, "tx pool buffer allocation failed\n");
- release_tx_pools(adapter);
- return -1;
- }
-
- if (alloc_long_term_buff(adapter, _pool->long_term_buff,
-adapter->req_tx_entries_per_subcrq *
-(adapter->req_mtu + VLAN_HLEN))) {
- release_tx_pools(adapter);
- return -1;
- }
-
- /* alloc TSO ltb */
- if (alloc_long_term_buff(adapter, _pool->tso_ltb,
-IBMVNIC_TSO_BUFS *
-IBMVNIC_TSO_BUF_SZ)) {
+ rc = init_one_tx_pool(netdev, >tx_pool[i],
+ adapter->req_tx_entries_per_subcrq,
+ adapter->req_mtu + VLAN_HLEN);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
- tx_pool->tso_index = 0;
-
- tx_pool->free_map = kcalloc(adapter->req_tx_entries_per_subcrq,
- sizeof(int), GFP_KERNEL);
- if (!tx_pool->free_map) {
+ init_one_tx_pool(netdev, >tso_pool[i],
+IBMVNIC_TSO_BUFS,
+IBMVNIC_TSO_BUF_SZ);
+ if (rc) {
release_tx_pools(adapter);
- return -1;
+ return rc;
}
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
}
return 0;
--
1.8.3.1
[PATCH net-next 1/6] ibmvnic: Generalize TX pool structure
Remove some unused fields in the structure and include values
describing the individual buffer size and number of buffers in
a TX pool. This allows us to use these fields for TX pool buffer
accounting as opposed to using hard coded values. Finally, split
TSO buffers out and provide an additional TX pool array for TSO.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.h | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h
b/drivers/net/ethernet/ibm/ibmvnic.h
index 099c89d..a2e21b3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -917,11 +917,9 @@ struct ibmvnic_tx_pool {
int *free_map;
int consumer_index;
int producer_index;
- wait_queue_head_t ibmvnic_tx_comp_q;
- struct task_struct *work_thread;
struct ibmvnic_long_term_buff long_term_buff;
- struct ibmvnic_long_term_buff tso_ltb;
- int tso_index;
+ int num_buffers;
+ int buf_size;
};
struct ibmvnic_rx_buff {
@@ -1044,6 +1042,7 @@ struct ibmvnic_adapter {
u64 promisc;
struct ibmvnic_tx_pool *tx_pool;
+ struct ibmvnic_tx_pool *tso_pool;
struct completion init_done;
int init_done_rc;
--
1.8.3.1
[PATCH net-next 5/6] ibmvnic: Update TX and TX completion routines
Update TX and TX completion routines to account for TX pool
restructuring. TX routine first chooses the pool depending
on whether a packet is GSO or not, then uses it accordingly.
For the completion routine to know which pool it needs to use,
set the most significant bit of the correlator index to one
if the packet uses the TSO pool. On completion, unset the bit
and use the correlator index to release the buffer pool entry.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 55 +++---
drivers/net/ethernet/ibm/ibmvnic.h | 1 +
2 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index cda5bf0..3b752e3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1414,8 +1414,11 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
ret = NETDEV_TX_OK;
goto out;
}
+ if (skb_is_gso(skb))
+ tx_pool = >tso_pool[queue_num];
+ else
+ tx_pool = >tx_pool[queue_num];
- tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
handle_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
@@ -1423,20 +1426,10 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
- if (skb_is_gso(skb)) {
- offset = tx_pool->tso_index * IBMVNIC_TSO_BUF_SZ;
- dst = tx_pool->tso_ltb.buff + offset;
- memset(dst, 0, IBMVNIC_TSO_BUF_SZ);
- data_dma_addr = tx_pool->tso_ltb.addr + offset;
- tx_pool->tso_index++;
- if (tx_pool->tso_index == IBMVNIC_TSO_BUFS)
- tx_pool->tso_index = 0;
- } else {
- offset = index * (adapter->req_mtu + VLAN_HLEN);
- dst = tx_pool->long_term_buff.buff + offset;
- memset(dst, 0, adapter->req_mtu + VLAN_HLEN);
- data_dma_addr = tx_pool->long_term_buff.addr + offset;
- }
+ offset = index * tx_pool->buf_size;
+ dst = tx_pool->long_term_buff.buff + offset;
+ memset(dst, 0, tx_pool->buf_size);
+ data_dma_addr = tx_pool->long_term_buff.addr + offset;
if (skb_shinfo(skb)->nr_frags) {
int cur, i;
@@ -1459,8 +1452,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
tx_pool->consumer_index =
- (tx_pool->consumer_index + 1) %
- adapter->req_tx_entries_per_subcrq;
+ (tx_pool->consumer_index + 1) % tx_pool->num_buffers;
tx_buff = _pool->tx_buff[index];
tx_buff->skb = skb;
@@ -1476,11 +1468,13 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_crq.v1.n_crq_elem = 1;
tx_crq.v1.n_sge = 1;
tx_crq.v1.flags1 = IBMVNIC_TX_COMP_NEEDED;
- tx_crq.v1.correlator = cpu_to_be32(index);
+
if (skb_is_gso(skb))
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->tso_ltb.map_id);
+ tx_crq.v1.correlator =
+ cpu_to_be32(index | IBMVNIC_TSO_POOL_MASK);
else
- tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
+ tx_crq.v1.correlator = cpu_to_be32(index);
+ tx_crq.v1.dma_reg = cpu_to_be16(tx_pool->long_term_buff.map_id);
tx_crq.v1.sge_len = cpu_to_be32(skb->len);
tx_crq.v1.ioba = cpu_to_be64(data_dma_addr);
@@ -1543,7 +1537,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (tx_pool->consumer_index == 0)
tx_pool->consumer_index =
- adapter->req_tx_entries_per_subcrq - 1;
+ tx_pool->num_buffers - 1;
else
tx_pool->consumer_index--;
@@ -2545,6 +2539,7 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
struct ibmvnic_sub_crq_queue *scrq)
{
struct device *dev = >vdev->dev;
+ struct ibmvnic_tx_pool *tx_pool;
struct ibmvnic_tx_buff *txbuff;
union sub_crq *next;
int index;
@@ -2564,7 +2559,14 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
continue;
}
index = be32_to_cpu(next->tx_comp.correlators[i]);
- txbuff = >tx_pool[pool].tx_buff[index];
+ if (index & IBMVNIC_TSO_POOL_MASK) {
+ tx_pool = >tso_pool[pool]
[PATCH net-next 6/6] ibmvnic: Improve TX buffer accounting
Improve TX pool buffer accounting to prevent the producer
index from overruning the consumer. First, set the next free
index to an invalid value if it is in use. If next buffer
to be consumed is in use, drop the packet.
Finally, if the transmit fails for some other reason, roll
back the consumer index and set the free map entry to its original
value. This should also be done if the DMA map fails.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 3b752e3..f599dad 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1426,6 +1426,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
index = tx_pool->free_map[tx_pool->consumer_index];
+ if (index == IBMVNIC_INVALID_MAP) {
+ dev_kfree_skb_any(skb);
+ tx_send_failed++;
+ tx_dropped++;
+ ret = NETDEV_TX_OK;
+ goto out;
+ }
+
+ tx_pool->free_map[tx_pool->consumer_index] = IBMVNIC_INVALID_MAP;
+
offset = index * tx_pool->buf_size;
dst = tx_pool->long_term_buff.buff + offset;
memset(dst, 0, tx_pool->buf_size);
@@ -1522,7 +1532,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_map_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
lpar_rc = send_subcrq_indirect(adapter, handle_array[queue_num],
(u64)tx_buff->indir_dma,
@@ -1534,13 +1544,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
}
if (lpar_rc != H_SUCCESS) {
dev_err(dev, "tx failed with code %ld\n", lpar_rc);
-
- if (tx_pool->consumer_index == 0)
- tx_pool->consumer_index =
- tx_pool->num_buffers - 1;
- else
- tx_pool->consumer_index--;
-
dev_kfree_skb_any(skb);
tx_buff->skb = NULL;
@@ -1556,7 +1559,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_send_failed++;
tx_dropped++;
ret = NETDEV_TX_OK;
- goto out;
+ goto tx_err_out;
}
if (atomic_add_return(num_entries, _scrq->used)
@@ -1569,7 +1572,16 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
tx_bytes += skb->len;
txq->trans_start = jiffies;
ret = NETDEV_TX_OK;
+ goto out;
+tx_err_out:
+ /* roll back consumer index and map array*/
+ if (tx_pool->consumer_index == 0)
+ tx_pool->consumer_index =
+ tx_pool->num_buffers - 1;
+ else
+ tx_pool->consumer_index--;
+ tx_pool->free_map[tx_pool->consumer_index] = index;
out:
netdev->stats.tx_dropped += tx_dropped;
netdev->stats.tx_bytes += tx_bytes;
--
1.8.3.1
[PATCH net-next 3/6] ibmvnic: Update release RX pool routine
Introduce function that frees one TX pool. Use that to release
each pool in both the standard TX pool and TSO pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 81a4cd9..e6e934e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -608,25 +608,30 @@ static void release_vpd_data(struct ibmvnic_adapter
*adapter)
adapter->vpd = NULL;
}
+static void release_one_tx_pool(struct ibmvnic_adapter *adapter,
+ struct ibmvnic_tx_pool *tx_pool)
+{
+ kfree(tx_pool->tx_buff);
+ kfree(tx_pool->free_map);
+ free_long_term_buff(adapter, _pool->long_term_buff);
+}
+
static void release_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int i;
if (!adapter->tx_pool)
return;
for (i = 0; i < adapter->num_active_tx_pools; i++) {
- netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
- tx_pool = >tx_pool[i];
- kfree(tx_pool->tx_buff);
- free_long_term_buff(adapter, _pool->long_term_buff);
- free_long_term_buff(adapter, _pool->tso_ltb);
- kfree(tx_pool->free_map);
+ release_one_tx_pool(adapter, >tx_pool[i]);
+ release_one_tx_pool(adapter, >tso_pool[i]);
}
kfree(adapter->tx_pool);
adapter->tx_pool = NULL;
+ kfree(adapter->tso_pool);
+ adapter->tso_pool = NULL;
adapter->num_active_tx_pools = 0;
}
--
1.8.3.1
[PATCH net-next 2/6] ibmvnic: Update and clean up reset TX pool routine
Update TX pool reset routine to accommodate new TSO pool array. Introduce
a function that resets one TX pool, and use that function to initialize
each pool in both pool arrays.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 45 +-
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 6ff43d7..81a4cd9 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -557,36 +557,41 @@ static int init_rx_pools(struct net_device *netdev)
return 0;
}
+static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
+struct ibmvnic_tx_pool *tx_pool)
+{
+ int rc, i;
+
+ rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ if (rc)
+ return rc;
+
+ memset(tx_pool->tx_buff, 0,
+ tx_pool->num_buffers *
+ sizeof(struct ibmvnic_tx_buff));
+
+ for (i = 0; i < tx_pool->num_buffers; i++)
+ tx_pool->free_map[i] = i;
+
+ tx_pool->consumer_index = 0;
+ tx_pool->producer_index = 0;
+
+ return 0;
+}
+
static int reset_tx_pools(struct ibmvnic_adapter *adapter)
{
- struct ibmvnic_tx_pool *tx_pool;
int tx_scrqs;
- int i, j, rc;
+ int i, rc;
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
for (i = 0; i < tx_scrqs; i++) {
- netdev_dbg(adapter->netdev, "Re-setting tx_pool[%d]\n", i);
-
- tx_pool = >tx_pool[i];
-
- rc = reset_long_term_buff(adapter, _pool->long_term_buff);
+ rc = reset_one_tx_pool(adapter, >tso_pool[i]);
if (rc)
return rc;
-
- rc = reset_long_term_buff(adapter, _pool->tso_ltb);
+ rc = reset_one_tx_pool(adapter, >tx_pool[i]);
if (rc)
return rc;
-
- memset(tx_pool->tx_buff, 0,
- adapter->req_tx_entries_per_subcrq *
- sizeof(struct ibmvnic_tx_buff));
-
- for (j = 0; j < adapter->req_tx_entries_per_subcrq; j++)
- tx_pool->free_map[j] = j;
-
- tx_pool->consumer_index = 0;
- tx_pool->producer_index = 0;
- tx_pool->tso_index = 0;
}
return 0;
--
1.8.3.1
[PATCH net-next 0/6] ibmvnic: Update TX pool and TX routines
This patch restructures the TX pool data structure and provides a separate TX pool array for TSO transmissions. This is already used in some way due to our unique DMA situation, namely that we cannot use single DMA mappings for packet data. Previously, both buffer arrays used the same pool entry. This restructuring allows for some additional cleanup in the driver code, especially in some places in the device transmit routine. In addition, it allows us to more easily track the consumer and producer indexes of a particular pool. This has been further improved by better tracking of in-use buffers to prevent possible data corruption in case an invalid buffer entry is used. Thomas Falcon (6): ibmvnic: Generalize TX pool structure ibmvnic: Update and clean up reset TX pool routine ibmvnic: Update release RX pool routine ibmvnic: Update TX pool initialization routine ibmvnic: Update TX and TX completion routines ibmvnic: Improve TX buffer accounting drivers/net/ethernet/ibm/ibmvnic.c | 235 + drivers/net/ethernet/ibm/ibmvnic.h | 8 +- 2 files changed, 136 insertions(+), 107 deletions(-) -- 1.8.3.1
[PATCH net-next] ibmvnic: Fix recent errata commit
Sorry, one of the patches I sent in an earlier series
has some dumb mistakes. One was that I had changed the
parameter for the errata workaround function but forgot
to make that change in the code that called it.
The second mistake was a forgotten return value at the end
of the function in case the workaround was not needed.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index e02d3b9..6ff43d7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1351,6 +1351,8 @@ static int ibmvnic_xmit_workarounds(struct sk_buff *skb,
*/
if (skb->len < netdev->min_mtu)
return skb_put_padto(skb, netdev->min_mtu);
+
+ return 0;
}
static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
@@ -1390,7 +1392,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
goto out;
}
- if (ibmvnic_xmit_workarounds(skb, adapter)) {
+ if (ibmvnic_xmit_workarounds(skb, netdev)) {
tx_dropped++;
tx_send_failed++;
ret = NETDEV_TX_OK;
--
1.8.3.1
Re: [PATCH net-next v2 0/4] ibmvnic: Fix VLAN and other device errata
On 03/12/2018 07:59 PM, David Miller wrote: > From: Thomas Falcon <tlfal...@linux.vnet.ibm.com> > Date: Mon, 12 Mar 2018 17:07:38 -0500 > >> On 03/12/2018 11:56 AM, David Miller wrote: >>> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com> >>> Date: Mon, 12 Mar 2018 11:51:01 -0500 >>> >>>> This patch series contains fixes for VLAN and other backing hardware >>>> errata. The VLAN fixes are mostly to account for the additional four >>>> bytes VLAN header in TX descriptors and buffers, when applicable. >>>> >>>> The other fixes for device errata are to pad small packets to avoid a >>>> possible connection error that can occur when some devices attempt to >>>> transmit small packets. The other fixes are GSO related. Some devices >>>> cannot handle a smaller MSS or a packet with a single segment, so >>>> disable GSO in those cases. >>>> >>>> v2: Fix style mistake (unneeded brackets) in patch 3/4 >>> Series applied, thanks Thomas. >>> >> Really sorry about this, but 3/4 is still wrong. There is actually a >> compiler warning caused by it, which I'm surprised wasn't caught by the test >> robot. Is there still time to send a v3? > If it's in my tree you must send a fixup patch. > Thanks, I will do that shortly.
Re: [PATCH net-next v2 0/4] ibmvnic: Fix VLAN and other device errata
On 03/12/2018 11:56 AM, David Miller wrote: > From: Thomas Falcon <tlfal...@linux.vnet.ibm.com> > Date: Mon, 12 Mar 2018 11:51:01 -0500 > >> This patch series contains fixes for VLAN and other backing hardware >> errata. The VLAN fixes are mostly to account for the additional four >> bytes VLAN header in TX descriptors and buffers, when applicable. >> >> The other fixes for device errata are to pad small packets to avoid a >> possible connection error that can occur when some devices attempt to >> transmit small packets. The other fixes are GSO related. Some devices >> cannot handle a smaller MSS or a packet with a single segment, so >> disable GSO in those cases. >> >> v2: Fix style mistake (unneeded brackets) in patch 3/4 > Series applied, thanks Thomas. > Really sorry about this, but 3/4 is still wrong. There is actually a compiler warning caused by it, which I'm surprised wasn't caught by the test robot. Is there still time to send a v3?
[PATCH net-next v2 2/4] ibmvnic: Account for VLAN header length in TX buffers
The extra four bytes of a VLAN packet was throwing off
TX buffer entry values used by the driver. Account for those
bytes when in buffer size and buffer entry calculations
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index ea9f351..14f0081 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -659,7 +659,7 @@ static int init_tx_pools(struct net_device *netdev)
if (alloc_long_term_buff(adapter, _pool->long_term_buff,
adapter->req_tx_entries_per_subcrq *
-adapter->req_mtu)) {
+(adapter->req_mtu + VLAN_HLEN))) {
release_tx_pools(adapter);
return -1;
}
@@ -1394,9 +1394,9 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (tx_pool->tso_index == IBMVNIC_TSO_BUFS)
tx_pool->tso_index = 0;
} else {
- offset = index * adapter->req_mtu;
+ offset = index * (adapter->req_mtu + VLAN_HLEN);
dst = tx_pool->long_term_buff.buff + offset;
- memset(dst, 0, adapter->req_mtu);
+ memset(dst, 0, adapter->req_mtu + VLAN_HLEN);
data_dma_addr = tx_pool->long_term_buff.addr + offset;
}
--
2.7.5
[PATCH net-next v2 1/4] ibmvnic: Account for VLAN tag in L2 Header descriptor
If a VLAN tag is present in the Ethernet header, account
for that when providing the L2 header to firmware.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7be4b06..ea9f351 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1221,7 +1221,10 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff
*skb,
int len = 0;
u8 *hdr;
- hdr_len[0] = sizeof(struct ethhdr);
+ if (skb_vlan_tagged(skb) && !skb_vlan_tag_present(skb))
+ hdr_len[0] = sizeof(struct vlan_ethhdr);
+ else
+ hdr_len[0] = sizeof(struct ethhdr);
if (skb->protocol == htons(ETH_P_IP)) {
hdr_len[1] = ip_hdr(skb)->ihl * 4;
--
2.7.5
[PATCH net-next v2 4/4] ibmvnic: Handle TSO backing device errata
TSO packets with one segment or with an MSS less than 224 can
cause errors on some backing devices, so disable GSO in those cases.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7ed87fb..e02d3b9 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2049,6 +2049,23 @@ static int ibmvnic_change_mtu(struct net_device *netdev,
int new_mtu)
return wait_for_reset(adapter);
}
+static netdev_features_t ibmvnic_features_check(struct sk_buff *skb,
+ struct net_device *dev,
+ netdev_features_t features)
+{
+ /* Some backing hardware adapters can not
+* handle packets with a MSS less than 224
+* or with only one segment.
+*/
+ if (skb_is_gso(skb)) {
+ if (skb_shinfo(skb)->gso_size < 224 ||
+ skb_shinfo(skb)->gso_segs == 1)
+ features &= ~NETIF_F_GSO_MASK;
+ }
+
+ return features;
+}
+
static const struct net_device_ops ibmvnic_netdev_ops = {
.ndo_open = ibmvnic_open,
.ndo_stop = ibmvnic_close,
@@ -2061,6 +2078,7 @@ static const struct net_device_ops ibmvnic_netdev_ops = {
.ndo_poll_controller= ibmvnic_netpoll_controller,
#endif
.ndo_change_mtu = ibmvnic_change_mtu,
+ .ndo_features_check = ibmvnic_features_check,
};
/* ethtool functions */
--
2.7.5
[PATCH net-next v2 3/4] ibmvnic: Pad small packets to minimum MTU size
Some backing devices cannot handle small packets well,
so pad any small packets to avoid that. It was recommended
that the VNIC driver should not send packets smaller than the
minimum MTU value provided by firmware, so pad small packets
to be at least that long.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
v2: Fix up unneeded brackets
drivers/net/ethernet/ibm/ibmvnic.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 14f0081..7ed87fb 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1340,6 +1340,19 @@ static void build_hdr_descs_arr(struct ibmvnic_tx_buff
*txbuff,
txbuff->indir_arr + 1);
}
+static int ibmvnic_xmit_workarounds(struct sk_buff *skb,
+ struct net_device *netdev)
+{
+ /* For some backing devices, mishandling of small packets
+* can result in a loss of connection or TX stall. Device
+* architects recommend that no packet should be smaller
+* than the minimum MTU value provided to the driver, so
+* pad any packets to that length
+*/
+ if (skb->len < netdev->min_mtu)
+ return skb_put_padto(skb, netdev->min_mtu);
+}
+
static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -1377,6 +1390,13 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
goto out;
}
+ if (ibmvnic_xmit_workarounds(skb, adapter)) {
+ tx_dropped++;
+ tx_send_failed++;
+ ret = NETDEV_TX_OK;
+ goto out;
+ }
+
tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
--
2.7.5
[PATCH net-next v2 0/4] ibmvnic: Fix VLAN and other device errata
This patch series contains fixes for VLAN and other backing hardware errata. The VLAN fixes are mostly to account for the additional four bytes VLAN header in TX descriptors and buffers, when applicable. The other fixes for device errata are to pad small packets to avoid a possible connection error that can occur when some devices attempt to transmit small packets. The other fixes are GSO related. Some devices cannot handle a smaller MSS or a packet with a single segment, so disable GSO in those cases. v2: Fix style mistake (unneeded brackets) in patch 3/4 Thomas Falcon (4): ibmvnic: Account for VLAN tag in L2 Header descriptor ibmvnic: Account for VLAN header length in TX buffers ibmvnic: Pad small packets to minimum MTU size ibmvnic: Handle TSO backing device errata drivers/net/ethernet/ibm/ibmvnic.c | 50 +++--- 1 file changed, 46 insertions(+), 4 deletions(-) -- 2.12.3
Re: [PATCH 3/4 net-next] ibmvnic: Pad small packets to minimum MTU size
On 03/11/2018 09:56 PM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Fri, 9 Mar 2018 13:23:56 -0600
>
>> +/* For some backing devices, mishandling of small packets
>> + * can result in a loss of connection or TX stall. Device
>> + * architects recommend that no packet should be smaller
>> + * than the minimum MTU value provided to the driver, so
>> + * pad any packets to that length
>> + */
>> +if (skb->len < netdev->min_mtu) {
>> +return skb_put_padto(skb, netdev->min_mtu);
>> +}
> Please do not use curly braces for a single statement
> basic block.
>
> Thank you.
>
Oops, sorry about that. I'll fix that and resend.
Thanks.
[PATCH 3/4 net-next] ibmvnic: Pad small packets to minimum MTU size
Some backing devices cannot handle small packets well,
so pad any small packets to avoid that. It was recommended
that the VNIC driver should not send packets smaller than the
minimum MTU value provided by firmware, so pad small packets
to be at least that long.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index ddb8afa11525..d47379b95477 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1345,6 +1345,20 @@ static void build_hdr_descs_arr(struct ibmvnic_tx_buff
*txbuff,
txbuff->indir_arr + 1);
}
+static int ibmvnic_xmit_workarounds(struct sk_buff *skb,
+ struct net_device *netdev)
+{
+ /* For some backing devices, mishandling of small packets
+* can result in a loss of connection or TX stall. Device
+* architects recommend that no packet should be smaller
+* than the minimum MTU value provided to the driver, so
+* pad any packets to that length
+*/
+ if (skb->len < netdev->min_mtu) {
+ return skb_put_padto(skb, netdev->min_mtu);
+ }
+}
+
static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -1382,6 +1396,13 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
goto out;
}
+ if (ibmvnic_xmit_workarounds(skb, adapter)) {
+ tx_dropped++;
+ tx_send_failed++;
+ ret = NETDEV_TX_OK;
+ goto out;
+ }
+
tx_pool = >tx_pool[queue_num];
tx_scrq = adapter->tx_scrq[queue_num];
txq = netdev_get_tx_queue(netdev, skb_get_queue_mapping(skb));
--
2.12.3
[PATCH 4/4 net-next] ibmvnic: Handle TSO backing device errata
TSO packets with one segment or with an MSS less than 224 can
cause errors on some backing devices, so disable GSO in those cases.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index d47379b95477..47377977591d 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2052,6 +2052,23 @@ static int ibmvnic_change_mtu(struct net_device *netdev,
int new_mtu)
return wait_for_reset(adapter);
}
+static netdev_features_t ibmvnic_features_check(struct sk_buff *skb,
+ struct net_device *dev,
+ netdev_features_t features)
+{
+ /* Some backing hardware adapters can not
+* handle packets with a MSS less than 224
+* or with only one segment.
+*/
+ if (skb_is_gso(skb)) {
+ if (skb_shinfo(skb)->gso_size < 224 ||
+ skb_shinfo(skb)->gso_segs == 1)
+ features &= ~NETIF_F_GSO_MASK;
+ }
+
+ return features;
+}
+
static const struct net_device_ops ibmvnic_netdev_ops = {
.ndo_open = ibmvnic_open,
.ndo_stop = ibmvnic_close,
@@ -2064,6 +2081,7 @@ static const struct net_device_ops ibmvnic_netdev_ops = {
.ndo_poll_controller= ibmvnic_netpoll_controller,
#endif
.ndo_change_mtu = ibmvnic_change_mtu,
+ .ndo_features_check = ibmvnic_features_check,
};
/* ethtool functions */
--
2.12.3
[PATCH 2/4 net-next] ibmvnic: Account for VLAN header length in TX buffers
The extra four bytes of a VLAN packet was throwing off
TX buffer entry values used by the driver. Account for those
bytes when in buffer size and buffer entry calculations
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index ecfeba7ea981..ddb8afa11525 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -659,7 +659,7 @@ static int init_tx_pools(struct net_device *netdev)
if (alloc_long_term_buff(adapter, _pool->long_term_buff,
adapter->req_tx_entries_per_subcrq *
-adapter->req_mtu)) {
+(adapter->req_mtu + VLAN_HLEN))) {
release_tx_pools(adapter);
return -1;
}
@@ -1399,9 +1399,9 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (tx_pool->tso_index == IBMVNIC_TSO_BUFS)
tx_pool->tso_index = 0;
} else {
- offset = index * adapter->req_mtu;
+ offset = index * (adapter->req_mtu + VLAN_HLEN);
dst = tx_pool->long_term_buff.buff + offset;
- memset(dst, 0, adapter->req_mtu);
+ memset(dst, 0, adapter->req_mtu + VLAN_HLEN);
data_dma_addr = tx_pool->long_term_buff.addr + offset;
}
--
2.12.3
[PATCH 0/4 net-next] ibmvnic: Fix VLAN and other device errata
This patch series contains fixes for VLAN and other backing hardware errata. The VLAN fixes are mostly to account for the additional four bytes VLAN header in TX descriptors and buffers, when applicable. The other fixes for device errata are to pad small packets to avoid a possible connection error that can occur when some devices attempt to transmit small packets. The other fixes are GSO related. Some devices cannot handle a smaller MSS or a packet with a single segment, so disable GSO in those cases. Thomas Falcon (4): ibmvnic: Account for VLAN tag in L2 Header descriptor ibmvnic: Account for VLAN header length in TX buffers ibmvnic: Pad small packets to minimum MTU size ibmvnic: Handle TSO backing device errata drivers/net/ethernet/ibm/ibmvnic.c | 50 +++--- 1 file changed, 46 insertions(+), 4 deletions(-) -- 2.12.3
[PATCH 1/4 net-next] ibmvnic: Account for VLAN tag in L2 Header descriptor
If a VLAN tag is present in the Ethernet header, account
for that when providing the L2 header to firmware.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 765407179fdd..ecfeba7ea981 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1226,7 +1226,10 @@ static int build_hdr_data(u8 hdr_field, struct sk_buff
*skb,
int len = 0;
u8 *hdr;
- hdr_len[0] = sizeof(struct ethhdr);
+ if (skb_vlan_tagged(skb) && !skb_vlan_tag_present(skb))
+ hdr_len[0] = sizeof(struct vlan_ethhdr);
+ else
+ hdr_len[0] = sizeof(struct ethhdr);
if (skb->protocol == htons(ETH_P_IP)) {
hdr_len[1] = ip_hdr(skb)->ihl * 4;
--
2.12.3
[RESEND PATCH 0/3 net-next] ibmvnic: Clean up net close and fix reset bug
This patch set cleans up and reorganizes the driver's net_device close function and leverages that to fix up a bug that can occur during some device resets. Some reset cases require the backing adapter to be disabled before continuing, but other cases, such as during a device failover or partition migration, do not require this step. Since the device will not be initialized at this stage and its command-processing queue is closed, do not send the request to disable the device as it could result in an error or timeout disrupting the reset. Thomas Falcon (3): ibmvnic: Clean up device close ibmvnic: Reorganize device close ibmvnic: Do not disable device during failover or partition migration drivers/net/ethernet/ibm/ibmvnic.c | 48 ++ 1 file changed, 23 insertions(+), 25 deletions(-) -- 1.8.3.1
[RESEND PATCH 1/3 net-next] ibmvnic: Clean up device close
Remove some dead code now that RX pools are being cleaned. This
was included to wait until any pending RX queue interrupts are
processed, but NAPI polling should be disabled by this point.
Another minor change is to use the net device parameter for any
print functions instead of accessing it from the adapter structure.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7654071..fca0533 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1162,7 +1162,7 @@ static int __ibmvnic_close(struct net_device *netdev)
if (adapter->tx_scrq) {
for (i = 0; i < adapter->req_tx_queues; i++)
if (adapter->tx_scrq[i]->irq) {
- netdev_dbg(adapter->netdev,
+ netdev_dbg(netdev,
"Disabling tx_scrq[%d] irq\n", i);
disable_irq(adapter->tx_scrq[i]->irq);
}
@@ -1174,18 +1174,8 @@ static int __ibmvnic_close(struct net_device *netdev)
if (adapter->rx_scrq) {
for (i = 0; i < adapter->req_rx_queues; i++) {
- int retries = 10;
-
- while (pending_scrq(adapter, adapter->rx_scrq[i])) {
- retries--;
- mdelay(100);
-
- if (retries == 0)
- break;
- }
-
if (adapter->rx_scrq[i]->irq) {
- netdev_dbg(adapter->netdev,
+ netdev_dbg(netdev,
"Disabling rx_scrq[%d] irq\n", i);
disable_irq(adapter->rx_scrq[i]->irq);
}
--
1.8.3.1
[RESEND PATCH 3/3 net-next] ibmvnic: Do not disable device during failover or partition migration
During a device failover or partition migration reset, it is not
necessary to disable the backing adapter since it should not be
running yet and its Command-Response Queue is closed. Sending
device commands during this time could result in an error or
timeout disrupting the reset process. In these cases, just halt
transmissions, clean up resources, and continue with reset.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index d93f286..7be4b06 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1653,12 +1653,15 @@ static int do_reset(struct ibmvnic_adapter *adapter,
rc = ibmvnic_reenable_crq_queue(adapter);
if (rc)
return 0;
+ ibmvnic_cleanup(netdev);
+ } else if (rwi->reset_reason == VNIC_RESET_FAILOVER) {
+ ibmvnic_cleanup(netdev);
+ } else {
+ rc = __ibmvnic_close(netdev);
+ if (rc)
+ return rc;
}
- rc = __ibmvnic_close(netdev);
- if (rc)
- return rc;
-
if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
adapter->wait_for_reset) {
release_resources(adapter);
--
1.8.3.1
[RESEND PATCH 2/3 net-next] ibmvnic: Reorganize device close
Introduce a function to halt network operations and clean up any
unused or outstanding socket buffers. Then, during device close,
disable backing adapter before halting all queues and performing
cleanup. This ensures all backing device operations will be
stopped before the driver cleans up shared resources.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index fca0533..d93f286 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1143,14 +1143,11 @@ static void clean_tx_pools(struct ibmvnic_adapter
*adapter)
}
}
-static int __ibmvnic_close(struct net_device *netdev)
+static void ibmvnic_cleanup(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- int rc = 0;
int i;
- adapter->state = VNIC_CLOSING;
-
/* ensure that transmissions are stopped if called by do_reset */
if (adapter->resetting)
netif_tx_disable(netdev);
@@ -1168,10 +1165,6 @@ static int __ibmvnic_close(struct net_device *netdev)
}
}
- rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
- if (rc)
- return rc;
-
if (adapter->rx_scrq) {
for (i = 0; i < adapter->req_rx_queues; i++) {
if (adapter->rx_scrq[i]->irq) {
@@ -1183,8 +1176,20 @@ static int __ibmvnic_close(struct net_device *netdev)
}
clean_rx_pools(adapter);
clean_tx_pools(adapter);
+}
+
+static int __ibmvnic_close(struct net_device *netdev)
+{
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int rc = 0;
+
+ adapter->state = VNIC_CLOSING;
+ rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+ if (rc)
+ return rc;
+ ibmvnic_cleanup(netdev);
adapter->state = VNIC_CLOSED;
- return rc;
+ return 0;
}
static int ibmvnic_close(struct net_device *netdev)
--
1.8.3.1
Re: [PATCH 0/3] ibmvnic: Clean up net close and fix reset bug
On 03/07/2018 05:41 PM, Thomas Falcon wrote: > This patch set cleans up and reorganizes the driver's net_device > close function and leverages that to fix up a bug that can occur > during some device resets. Some reset cases require the backing > adapter to be disabled before continuing, but other cases, such as > during a device failover or partition migration, do not require this > step. Since the device will not be initialized at this stage and > its command-processing queue is closed, do not send the request to > disable the device as it could result in an error or timeout > disrupting the reset. > > Thomas Falcon (3): > ibmvnic: Clean up device close > ibmvnic: Reorganize device close > ibmvnic: Do not disable device during failover or partition migration > > drivers/net/ethernet/ibm/ibmvnic.c | 48 > ++ > 1 file changed, 23 insertions(+), 25 deletions(-) > Crud, this series is meant for the net-next tree, but I forgot to include it in the patch tag.
[PATCH 3/3] ibmvnic: Do not disable device during failover or partition migration
During a device failover or partition migration reset, it is not
necessary to disable the backing adapter since it should not be
running yet and its Command-Response Queue is closed. Sending
device commands during this time could result in an error or
timeout disrupting the reset process. In these cases, just halt
transmissions, clean up resources, and continue with reset.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index d93f286..7be4b06 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1653,12 +1653,15 @@ static int do_reset(struct ibmvnic_adapter *adapter,
rc = ibmvnic_reenable_crq_queue(adapter);
if (rc)
return 0;
+ ibmvnic_cleanup(netdev);
+ } else if (rwi->reset_reason == VNIC_RESET_FAILOVER) {
+ ibmvnic_cleanup(netdev);
+ } else {
+ rc = __ibmvnic_close(netdev);
+ if (rc)
+ return rc;
}
- rc = __ibmvnic_close(netdev);
- if (rc)
- return rc;
-
if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
adapter->wait_for_reset) {
release_resources(adapter);
--
1.8.3.1
[PATCH 0/3] ibmvnic: Clean up net close and fix reset bug
This patch set cleans up and reorganizes the driver's net_device close function and leverages that to fix up a bug that can occur during some device resets. Some reset cases require the backing adapter to be disabled before continuing, but other cases, such as during a device failover or partition migration, do not require this step. Since the device will not be initialized at this stage and its command-processing queue is closed, do not send the request to disable the device as it could result in an error or timeout disrupting the reset. Thomas Falcon (3): ibmvnic: Clean up device close ibmvnic: Reorganize device close ibmvnic: Do not disable device during failover or partition migration drivers/net/ethernet/ibm/ibmvnic.c | 48 ++ 1 file changed, 23 insertions(+), 25 deletions(-) -- 1.8.3.1
[PATCH 2/3] ibmvnic: Reorganize device close
Introduce a function to halt network operations and clean up any
unused or outstanding socket buffers. Then, during device close,
disable backing adapter before halting all queues and performing
cleanup. This ensures all backing device operations will be
stopped before the driver cleans up shared resources.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index fca0533..d93f286 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1143,14 +1143,11 @@ static void clean_tx_pools(struct ibmvnic_adapter
*adapter)
}
}
-static int __ibmvnic_close(struct net_device *netdev)
+static void ibmvnic_cleanup(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- int rc = 0;
int i;
- adapter->state = VNIC_CLOSING;
-
/* ensure that transmissions are stopped if called by do_reset */
if (adapter->resetting)
netif_tx_disable(netdev);
@@ -1168,10 +1165,6 @@ static int __ibmvnic_close(struct net_device *netdev)
}
}
- rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
- if (rc)
- return rc;
-
if (adapter->rx_scrq) {
for (i = 0; i < adapter->req_rx_queues; i++) {
if (adapter->rx_scrq[i]->irq) {
@@ -1183,8 +1176,20 @@ static int __ibmvnic_close(struct net_device *netdev)
}
clean_rx_pools(adapter);
clean_tx_pools(adapter);
+}
+
+static int __ibmvnic_close(struct net_device *netdev)
+{
+ struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ int rc = 0;
+
+ adapter->state = VNIC_CLOSING;
+ rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
+ if (rc)
+ return rc;
+ ibmvnic_cleanup(netdev);
adapter->state = VNIC_CLOSED;
- return rc;
+ return 0;
}
static int ibmvnic_close(struct net_device *netdev)
--
1.8.3.1
[PATCH 1/3] ibmvnic: Clean up device close
Remove some dead code now that RX pools are being cleaned. This
was included to wait until any pending RX queue interrupts are
processed, but NAPI polling should be disabled by this point.
Another minor change is to use the net device parameter for any
print functions instead of accessing it from the adapter structure.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7654071..fca0533 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1162,7 +1162,7 @@ static int __ibmvnic_close(struct net_device *netdev)
if (adapter->tx_scrq) {
for (i = 0; i < adapter->req_tx_queues; i++)
if (adapter->tx_scrq[i]->irq) {
- netdev_dbg(adapter->netdev,
+ netdev_dbg(netdev,
"Disabling tx_scrq[%d] irq\n", i);
disable_irq(adapter->tx_scrq[i]->irq);
}
@@ -1174,18 +1174,8 @@ static int __ibmvnic_close(struct net_device *netdev)
if (adapter->rx_scrq) {
for (i = 0; i < adapter->req_rx_queues; i++) {
- int retries = 10;
-
- while (pending_scrq(adapter, adapter->rx_scrq[i])) {
- retries--;
- mdelay(100);
-
- if (retries == 0)
- break;
- }
-
if (adapter->rx_scrq[i]->irq) {
- netdev_dbg(adapter->netdev,
+ netdev_dbg(netdev,
"Disabling rx_scrq[%d] irq\n", i);
disable_irq(adapter->rx_scrq[i]->irq);
}
--
1.8.3.1
[PATCH 4/5 net-next] ibmvnic: Report queue stops and restarts as debug output
It's not necessary to report each time a queue is stopped and restarted
as an informational message. Change that to be a debug message so that
it can be observed if needed but not printed by default.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index b907c0a607e6..53b2c91fa786 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1528,7 +1528,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if (atomic_add_return(num_entries, _scrq->used)
>= adapter->req_tx_entries_per_subcrq) {
- netdev_info(netdev, "Stopping queue %d\n", queue_num);
+ netdev_dbg(netdev, "Stopping queue %d\n", queue_num);
netif_stop_subqueue(netdev, queue_num);
}
@@ -2541,8 +2541,8 @@ static int ibmvnic_complete_tx(struct ibmvnic_adapter
*adapter,
__netif_subqueue_stopped(adapter->netdev,
scrq->pool_index)) {
netif_wake_subqueue(adapter->netdev, scrq->pool_index);
- netdev_info(adapter->netdev, "Started queue %d\n",
- scrq->pool_index);
+ netdev_dbg(adapter->netdev, "Started queue %d\n",
+ scrq->pool_index);
}
}
--
2.15.0
[PATCH 5/5 net-next] ibmvnic: Do not attempt to login if RX or TX queues are not allocated
If a device reset fails for some reason, TX and RX queue resources
could be released. If a user attempts to open the device in this scenario,
it may result in a kernel panic as the driver tries to access this
memory. To fix this, include a check before device login that TX/RX
queues are still there before enabling the device. In addition, return a
value that can be checked in case of any errors to avoid waiting for a
completion that will never come.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 53b2c91fa786..765407179fdd 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -111,7 +111,7 @@ static int ibmvnic_poll(struct napi_struct *napi, int data);
static void send_map_query(struct ibmvnic_adapter *adapter);
static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
static void send_request_unmap(struct ibmvnic_adapter *, u8);
-static void send_login(struct ibmvnic_adapter *adapter);
+static int send_login(struct ibmvnic_adapter *adapter);
static void send_cap_queries(struct ibmvnic_adapter *adapter);
static int init_sub_crqs(struct ibmvnic_adapter *);
static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter);
@@ -809,8 +809,11 @@ static int ibmvnic_login(struct net_device *netdev)
}
reinit_completion(>init_done);
- send_login(adapter);
- if (!wait_for_completion_timeout(>init_done,
+ rc = send_login(adapter);
+ if (rc) {
+ dev_err(dev, "Unable to attempt device login\n");
+ return rc;
+ } else if (!wait_for_completion_timeout(>init_done,
timeout)) {
dev_err(dev, "Login timeout\n");
return -1;
@@ -3074,7 +3077,7 @@ static void vnic_add_client_data(struct ibmvnic_adapter
*adapter,
strncpy(>name, adapter->netdev->name, len);
}
-static void send_login(struct ibmvnic_adapter *adapter)
+static int send_login(struct ibmvnic_adapter *adapter)
{
struct ibmvnic_login_rsp_buffer *login_rsp_buffer;
struct ibmvnic_login_buffer *login_buffer;
@@ -3090,6 +3093,12 @@ static void send_login(struct ibmvnic_adapter *adapter)
struct vnic_login_client_data *vlcd;
int i;
+ if (!adapter->tx_scrq || !adapter->rx_scrq) {
+ netdev_err(adapter->netdev,
+ "RX or TX queues are not allocated, device login
failed\n");
+ return -1;
+ }
+
release_login_rsp_buffer(adapter);
client_data_len = vnic_client_data_len(adapter);
@@ -3187,7 +3196,7 @@ static void send_login(struct ibmvnic_adapter *adapter)
crq.login.len = cpu_to_be32(buffer_size);
ibmvnic_send_crq(adapter, );
- return;
+ return 0;
buf_rsp_map_failed:
kfree(login_rsp_buffer);
@@ -3196,7 +3205,7 @@ static void send_login(struct ibmvnic_adapter *adapter)
buf_map_failed:
kfree(login_buffer);
buf_alloc_failed:
- return;
+ return -1;
}
static void send_request_map(struct ibmvnic_adapter *adapter, dma_addr_t addr,
--
2.15.0
[PATCH 0/5 net-next] ibmvnic: Miscellaneous driver fixes and enhancements
There is not a general theme to this patch set other than that it fixes a few issues with the ibmvnic driver. I will just give a quick summary of what each patch does here. "ibmvnic: Fix TX descriptor tracking again" resolves a race condition introduced in an earlier fix to track outstanding transmit descriptors. This condition can throw off the tracking counter to the point that a transmit queue will halt forever. "ibmvnic: Allocate statistics buffers during probe" allocates queue statistics buffers on device probe to avoid a crash when accessing statistics of an unopened interface. "ibmvnic: Harden TX/RX pool cleaning" includes additional checks to avoid a bad access when cleaning RX and TX buffer pools during a device reset. "ibmvnic: Report queue stops and restarts as debug output" changes TX queue state notifications from informational to debug messages. This information is not necessarily useful to a user and under load can result in a lot of log output. "ibmvnic: Do not attempt to login if RX or TX queues are not allocated" checks that device queues have been allocated successfully before attempting device login. This resolves a panic that could occur if a user attempted to configure a device after a failed reset. Thanks for your attention. Thomas Falcon (5): ibmvnic: Fix TX descriptor tracking again ibmvnic: Allocate statistics buffers during probe ibmvnic: Harden TX/RX pool cleaning ibmvnic: Report queue stops and restarts as debug output ibmvnic: Do not attempt to login if RX or TX queues are not allocated drivers/net/ethernet/ibm/ibmvnic.c | 71 +++--- 1 file changed, 43 insertions(+), 28 deletions(-) -- 2.15.0
[PATCH 2/5 net-next] ibmvnic: Allocate statistics buffers during probe
Currently, buffers holding individual queue statistics are allocated when the device is opened. If an ibmvnic interface is hotplugged or initialized but never opened, an attempt to get statistics with ethtool will result in a kernel panic. Since the driver allocates a constant number, the maximum supported queues, of buffers, these can be allocated during device probe and freed when the device is hot-unplugged or the module is removed. Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 32ee202de13a..1a0f67b60003 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -845,8 +845,6 @@ static void release_resources(struct ibmvnic_adapter *adapter) release_tx_pools(adapter); release_rx_pools(adapter); - release_stats_token(adapter); - release_stats_buffers(adapter); release_error_buffers(adapter); release_napi(adapter); release_login_rsp_buffer(adapter); @@ -974,14 +972,6 @@ static int init_resources(struct ibmvnic_adapter *adapter) if (rc) return rc; - rc = init_stats_buffers(adapter); - if (rc) - return rc; - - rc = init_stats_token(adapter); - if (rc) - return rc; - adapter->vpd = kzalloc(sizeof(*adapter->vpd), GFP_KERNEL); if (!adapter->vpd) return -ENOMEM; @@ -4431,6 +4421,14 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter) release_crq_queue(adapter); } + rc = init_stats_buffers(adapter); + if (rc) + return rc; + + rc = init_stats_token(adapter); + if (rc) + return rc; + return rc; } @@ -4538,6 +4536,9 @@ static int ibmvnic_remove(struct vio_dev *dev) release_sub_crqs(adapter, 1); release_crq_queue(adapter); + release_stats_token(adapter); + release_stats_buffers(adapter); + adapter->state = VNIC_REMOVED; mutex_unlock(>reset_lock); -- 2.15.0
[PATCH 1/5 net-next] ibmvnic: Fix TX descriptor tracking again
Sorry, the previous change introduced a race condition between
transmit completion processing and tracking TX descriptors. If a
completion is received before the number of descriptors is logged,
the number of descriptors will be add but not removed. After enough
times, this could halt the transmit queue forever.
Log the number of descriptors used by a transmit before sending.
I stress tested the fix on two different systems running over the
weekend without any issues.
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5a86a916492c..32ee202de13a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1482,6 +1482,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
if ((*hdrs >> 7) & 1) {
build_hdr_descs_arr(tx_buff, _entries, *hdrs);
tx_crq.v1.n_crq_elem = num_entries;
+ tx_buff->num_entries = num_entries;
tx_buff->indir_arr[0] = tx_crq;
tx_buff->indir_dma = dma_map_single(dev, tx_buff->indir_arr,
sizeof(tx_buff->indir_arr),
@@ -1500,6 +1501,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
(u64)tx_buff->indir_dma,
(u64)num_entries);
} else {
+ tx_buff->num_entries = num_entries;
lpar_rc = send_subcrq(adapter, handle_array[queue_num],
_crq);
}
@@ -1536,7 +1538,6 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct
net_device *netdev)
netif_stop_subqueue(netdev, queue_num);
}
- tx_buff->num_entries = num_entries;
tx_packets++;
tx_bytes += skb->len;
txq->trans_start = jiffies;
--
2.15.0
