netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3 net] ibmvnic: Reset behavior fixes
@ 2018-01-15 21:09 John Allen
  2018-01-15 21:11 ` [PATCH 1/3 net] ibmvnic: Modify buffer size on failover John Allen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: John Allen @ 2018-01-15 21:09 UTC (permalink / raw)
  To: netdev; +Cc: Thomas Falcon, Nathan Fontenot, desnesn

This patchset fixes a number of issues related to ibmvnic reset uncovered
from testing new Power9 machines with Everglades adapters and the new
functionality to change mtu and other parameters in the driver.

This set should be applied after Thomas Falcon's patch,
"[PATCH net] ibmvnic: Fix pending MAC address changes" which is currently
submitted awaiting acceptance to the net tree.

John Allen (3):
  ibmvnic: Modify buffer size on failover
  ibmvnic: Revert to previous mtu when unsupported value requested
  ibmvnic: Allocate and request vpd in init_resources

 drivers/net/ethernet/ibm/ibmvnic.c | 42 ++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 9 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [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

* [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

* 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

end of thread, other threads:[~2018-01-18 12:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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: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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).