* [PATCH 1/3 net] ibmvnic: Modify buffer size on failover
2018-01-15 21:09 [PATCH 0/3 net] ibmvnic: Reset behavior fixes John Allen
@ 2018-01-15 21:11 ` John Allen
2018-01-15 21:49 ` John Allen
2018-01-18 12:18 ` kbuild test robot
2018-01-15 21:12 ` [PATCH 2/3 net] ibmvnic: Revert to previous mtu when unsupported value requested John Allen
2018-01-15 21:13 ` [PATCH 3/3 net] ibmvnic: Allocate and request vpd in init_resources John Allen
2 siblings, 2 replies; 6+ messages in thread
From: John Allen @ 2018-01-15 21:11 UTC (permalink / raw)
To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, desnesn
Using newer backing devices can cause the required padding at the end of
rx buffers to change. Currently we assume that the size of buffers will
never change, but in the case that we failover from a backing device with
smaller padding requirement to a backing device with a larger padding
requirement, the vnic server will fail to post rx buffers due to
inadequate space in our rx pool. This patch fixes the issue by checking
whether or not the buffer size has changed on a reset and if it has,
reallocate the buffer.
Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index b676fa9..5b68a28 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -412,13 +412,25 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
int rx_scrqs;
int i, j, rc;
+ size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
+ be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
+
rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
for (i = 0; i < rx_scrqs; i++) {
rx_pool = &adapter->rx_pool[i];
netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
- rc = reset_long_term_buff(adapter, &rx_pool->long_term_buff);
+ if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
+ rx_pool->buff_size = be64_to_cpu(size_array[i]);
+ rc = alloc_long_term_buff(adapter,
+ &rx_pool->long_term_buff,
+ rx_pool->size *
+ rx_pool->buff_size);
+ } else {
+ rc = reset_long_term_buff(adapter,
+ &rx_pool->long_term_buff);
+ }
if (rc)
return rc;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3 net] ibmvnic: Modify buffer size on failover
2018-01-15 21:11 ` [PATCH 1/3 net] ibmvnic: Modify buffer size on failover John Allen
@ 2018-01-15 21:49 ` John Allen
2018-01-18 12:18 ` kbuild test robot
1 sibling, 0 replies; 6+ messages in thread
From: John Allen @ 2018-01-15 21:49 UTC (permalink / raw)
To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, desnesn
On 01/15/2018 03:11 PM, John Allen wrote:
> Using newer backing devices can cause the required padding at the end of
> rx buffers to change. Currently we assume that the size of buffers will
> never change, but in the case that we failover from a backing device with
> smaller padding requirement to a backing device with a larger padding
> requirement, the vnic server will fail to post rx buffers due to
> inadequate space in our rx pool. This patch fixes the issue by checking
> whether or not the buffer size has changed on a reset and if it has,
> reallocate the buffer.
>
> Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
> ---
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index b676fa9..5b68a28 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -412,13 +412,25 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
> int rx_scrqs;
> int i, j, rc;
>
> + size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
> + be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
> +
> rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
> for (i = 0; i < rx_scrqs; i++) {
> rx_pool = &adapter->rx_pool[i];
>
> netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
>
> - rc = reset_long_term_buff(adapter, &rx_pool->long_term_buff);
> + if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
> + rx_pool->buff_size = be64_to_cpu(size_array[i]);
> + rc = alloc_long_term_buff(adapter,
> + &rx_pool->long_term_buff,
> + rx_pool->size *
> + rx_pool->buff_size);
We should be freeing the long_term_buff here before allocating a new one.
I will send a new version with the changes. Please ignore this version.
-John
> + } else {
> + rc = reset_long_term_buff(adapter,
> + &rx_pool->long_term_buff);
> + }
> if (rc)
> return rc;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/3 net] ibmvnic: Modify buffer size on failover
2018-01-15 21:11 ` [PATCH 1/3 net] ibmvnic: Modify buffer size on failover John Allen
2018-01-15 21:49 ` John Allen
@ 2018-01-18 12:18 ` kbuild test robot
1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2018-01-18 12:18 UTC (permalink / raw)
To: John Allen; +Cc: kbuild-all, netdev, Thomas Falcon, Nathan Fontenot, desnesn
[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]
Hi John,
I love your patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/John-Allen/ibmvnic-Modify-buffer-size-on-failover/20180118-190528
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
drivers/net//ethernet/ibm/ibmvnic.c: In function 'reset_rx_pools':
>> drivers/net//ethernet/ibm/ibmvnic.c:414:2: error: 'size_array' undeclared (first use in this function); did you mean 'sem_array'?
size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
^~~~~~~~~~
sem_array
drivers/net//ethernet/ibm/ibmvnic.c:414:2: note: each undeclared identifier is reported only once for each function it appears in
vim +414 drivers/net//ethernet/ibm/ibmvnic.c
407
408 static int reset_rx_pools(struct ibmvnic_adapter *adapter)
409 {
410 struct ibmvnic_rx_pool *rx_pool;
411 int rx_scrqs;
412 int i, j, rc;
413
> 414 size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
415 be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
416
417 rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
418 for (i = 0; i < rx_scrqs; i++) {
419 rx_pool = &adapter->rx_pool[i];
420
421 netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
422
423 if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
424 rx_pool->buff_size = be64_to_cpu(size_array[i]);
425 rc = alloc_long_term_buff(adapter,
426 &rx_pool->long_term_buff,
427 rx_pool->size *
428 rx_pool->buff_size);
429 } else {
430 rc = reset_long_term_buff(adapter,
431 &rx_pool->long_term_buff);
432 }
433 if (rc)
434 return rc;
435
436 for (j = 0; j < rx_pool->size; j++)
437 rx_pool->free_map[j] = j;
438
439 memset(rx_pool->rx_buff, 0,
440 rx_pool->size * sizeof(struct ibmvnic_rx_buff));
441
442 atomic_set(&rx_pool->available, 0);
443 rx_pool->next_alloc = 0;
444 rx_pool->next_free = 0;
445 rx_pool->active = 1;
446 }
447
448 return 0;
449 }
450
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56135 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3 net] ibmvnic: Revert to previous mtu when unsupported value requested
2018-01-15 21:09 [PATCH 0/3 net] ibmvnic: Reset behavior fixes John Allen
2018-01-15 21:11 ` [PATCH 1/3 net] ibmvnic: Modify buffer size on failover John Allen
@ 2018-01-15 21:12 ` John Allen
2018-01-15 21:13 ` [PATCH 3/3 net] ibmvnic: Allocate and request vpd in init_resources John Allen
2 siblings, 0 replies; 6+ messages in thread
From: John Allen @ 2018-01-15 21:12 UTC (permalink / raw)
To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, desnesn
If we request an unsupported mtu value, the vnic server will suggest a
different value. Currently we take the suggested value without question
and login with that value. However, the behavior doesn't seem completely
sane as attempting to change the mtu to some specific value will change
the mtu to some completely different value most of the time. This patch
fixes the issue by logging in with the previously used mtu value and
printing an error message saying that the given mtu is unsupported.
Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index f8f1396..bb56460 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3608,7 +3608,17 @@ static void handle_request_cap_rsp(union ibmvnic_crq *crq,
*req_value,
(long int)be64_to_cpu(crq->request_capability_rsp.
number), name);
- *req_value = be64_to_cpu(crq->request_capability_rsp.number);
+
+ if (be16_to_cpu(crq->request_capability_rsp.capability) ==
+ REQ_MTU) {
+ pr_err("mtu of %llu is not supported. Reverting.\n",
+ *req_value);
+ *req_value = adapter->fallback.mtu;
+ } else {
+ *req_value =
+ be64_to_cpu(crq->request_capability_rsp.number);
+ }
+
ibmvnic_send_req_caps(adapter, 1);
return;
default:
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3 net] ibmvnic: Allocate and request vpd in init_resources
2018-01-15 21:09 [PATCH 0/3 net] ibmvnic: Reset behavior fixes John Allen
2018-01-15 21:11 ` [PATCH 1/3 net] ibmvnic: Modify buffer size on failover John Allen
2018-01-15 21:12 ` [PATCH 2/3 net] ibmvnic: Revert to previous mtu when unsupported value requested John Allen
@ 2018-01-15 21:13 ` John Allen
2 siblings, 0 replies; 6+ messages in thread
From: John Allen @ 2018-01-15 21:13 UTC (permalink / raw)
To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, desnesn
In reset events in which our memory allocations need to be reallocated,
VPD data is being freed, but never reallocated. This can cause issues if
we later attempt to access that memory or reset and attempt to free the
memory. This patch moves the allocation of the VPD data to init_resources
so that it will be symmetrically freed during release resources.
Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index bb56460..f0dbb76 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -867,7 +867,7 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
if (adapter->vpd->buff)
len = adapter->vpd->len;
- reinit_completion(&adapter->fw_done);
+ init_completion(&adapter->fw_done);
crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
crq.get_vpd_size.cmd = GET_VPD_SIZE;
ibmvnic_send_crq(adapter, &crq);
@@ -929,6 +929,13 @@ static int init_resources(struct ibmvnic_adapter *adapter)
if (!adapter->vpd)
return -ENOMEM;
+ /* Vital Product Data (VPD) */
+ rc = ibmvnic_get_vpd(adapter);
+ if (rc) {
+ netdev_err(netdev, "failed to initialize Vital Product Data (VPD)\n");
+ return rc;
+ }
+
adapter->map_id = 1;
adapter->napi = kcalloc(adapter->req_rx_queues,
sizeof(struct napi_struct), GFP_KERNEL);
@@ -1002,7 +1009,7 @@ static int __ibmvnic_open(struct net_device *netdev)
static int ibmvnic_open(struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
- int rc, vpd;
+ int rc;
mutex_lock(&adapter->reset_lock);
@@ -1030,11 +1037,6 @@ static int ibmvnic_open(struct net_device *netdev)
rc = __ibmvnic_open(netdev);
netif_carrier_on(netdev);
- /* Vital Product Data (VPD) */
- vpd = ibmvnic_get_vpd(adapter);
- if (vpd)
- netdev_err(netdev, "failed to initialize Vital Product Data (VPD)\n");
-
mutex_unlock(&adapter->reset_lock);
return rc;
^ permalink raw reply related [flat|nested] 6+ messages in thread