diff mbox

[next,S19,11/15] i40evf: handle many MAC filters correctly

Message ID 1445471236-35918-12-git-send-email-catherine.sullivan@intel.com
State Accepted
Delegated to: Jeff Kirsher
Headers show

Commit Message

Catherine Sullivan Oct. 21, 2015, 11:47 p.m. UTC 
From: Mitch Williams <mitch.a.williams@intel.com>

When a lot (many hundreds) of MAC or VLAN filters are added at one time,
we can overflow the Admin Queue buffer size with all the requests.
Unfortunately, the driver would then calculate the message size
incorrectly, causing it to be rejected by the PF. Furthermore, there was
no mechanism to trigger another request to allow for configuring the
rest of the filters that didn't fit into the first request.

To fix this, recalculate the correct buffer size when we detect the
overflow condition instead of just assuming the max buffer size. Also,
don't clear the request bit in adapter->aq_required when we have an
overflow, so that the rest of the filters can be processed later.

Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
Change-ID: Idd7cbbc5af31315e0dcb1b10e6a02ad9817ce65c

---
Testing-hints: 5645131, 5645133
Bring up a VF and add 2000 IPv6 addresses to it. Before this patch, it
will crash. After this patch, it will crash less horribly.
 .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 32 ++++++++++++++++------
 1 file changed, 24 insertions(+), 8 deletions(-)

Comments

Bowers, AndrewX Oct. 26, 2015, 8:44 p.m. UTC | #1 
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Catherine Sullivan
> Sent: Wednesday, October 21, 2015 4:47 PM
> To: intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S19 11/15] i40evf: handle many MAC
> filters correctly
> 
> From: Mitch Williams <mitch.a.williams@intel.com>
> 
> When a lot (many hundreds) of MAC or VLAN filters are added at one time,
> we can overflow the Admin Queue buffer size with all the requests.
> Unfortunately, the driver would then calculate the message size incorrectly,
> causing it to be rejected by the PF. Furthermore, there was no mechanism to
> trigger another request to allow for configuring the rest of the filters that
> didn't fit into the first request.
> 
> To fix this, recalculate the correct buffer size when we detect the overflow
> condition instead of just assuming the max buffer size. Also, don't clear the
> request bit in adapter->aq_required when we have an overflow, so that the
> rest of the filters can be processed later.
> 
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Change-ID: Idd7cbbc5af31315e0dcb1b10e6a02ad9817ce65c
> 
> ---
> Testing-hints: 5645131, 5645133
> Bring up a VF and add 2000 IPv6 addresses to it. Before this patch, it will
> crash. After this patch, it will crash less horribly.
>  .../net/ethernet/intel/i40evf/i40evf_virtchnl.c    | 32 ++++++++++++++++---
> ---
>  1 file changed, 24 insertions(+), 8 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Patch code changes correctly applied, system still crashes but does not lock up system. Still have to reboot, however.
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
index 091ef6a..46b0516 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_virtchnl.c
@@ -391,6 +391,7 @@  void i40evf_add_ether_addrs(struct i40evf_adapter *adapter)
 	struct i40e_virtchnl_ether_addr_list *veal;
 	int len, i = 0, count = 0;
 	struct i40evf_mac_filter *f;
+	bool more = false;
 
 	if (adapter->current_op != I40E_VIRTCHNL_OP_UNKNOWN) {
 		/* bail because we already have a command pending */
@@ -415,7 +416,9 @@  void i40evf_add_ether_addrs(struct i40evf_adapter *adapter)
 		count = (I40EVF_MAX_AQ_BUF_SIZE -
 			 sizeof(struct i40e_virtchnl_ether_addr_list)) /
 			sizeof(struct i40e_virtchnl_ether_addr);
-		len = I40EVF_MAX_AQ_BUF_SIZE;
+		len = sizeof(struct i40e_virtchnl_ether_addr_list) +
+		      (count * sizeof(struct i40e_virtchnl_ether_addr));
+		more = true;
 	}
 
 	veal = kzalloc(len, GFP_ATOMIC);
@@ -431,7 +434,8 @@  void i40evf_add_ether_addrs(struct i40evf_adapter *adapter)
 			f->add = false;
 		}
 	}
-	adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_MAC_FILTER;
+	if (!more)
+		adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_MAC_FILTER;
 	i40evf_send_pf_msg(adapter, I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS,
 			   (u8 *)veal, len);
 	kfree(veal);
@@ -450,6 +454,7 @@  void i40evf_del_ether_addrs(struct i40evf_adapter *adapter)
 	struct i40e_virtchnl_ether_addr_list *veal;
 	struct i40evf_mac_filter *f, *ftmp;
 	int len, i = 0, count = 0;
+	bool more = false;
 
 	if (adapter->current_op != I40E_VIRTCHNL_OP_UNKNOWN) {
 		/* bail because we already have a command pending */
@@ -474,7 +479,9 @@  void i40evf_del_ether_addrs(struct i40evf_adapter *adapter)
 		count = (I40EVF_MAX_AQ_BUF_SIZE -
 			 sizeof(struct i40e_virtchnl_ether_addr_list)) /
 			sizeof(struct i40e_virtchnl_ether_addr);
-		len = I40EVF_MAX_AQ_BUF_SIZE;
+		len = sizeof(struct i40e_virtchnl_ether_addr_list) +
+		      (count * sizeof(struct i40e_virtchnl_ether_addr));
+		more = true;
 	}
 	veal = kzalloc(len, GFP_ATOMIC);
 	if (!veal)
@@ -490,7 +497,8 @@  void i40evf_del_ether_addrs(struct i40evf_adapter *adapter)
 			kfree(f);
 		}
 	}
-	adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_MAC_FILTER;
+	if (!more)
+		adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_MAC_FILTER;
 	i40evf_send_pf_msg(adapter, I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS,
 			   (u8 *)veal, len);
 	kfree(veal);
@@ -509,6 +517,7 @@  void i40evf_add_vlans(struct i40evf_adapter *adapter)
 	struct i40e_virtchnl_vlan_filter_list *vvfl;
 	int len, i = 0, count = 0;
 	struct i40evf_vlan_filter *f;
+	bool more = false;
 
 	if (adapter->current_op != I40E_VIRTCHNL_OP_UNKNOWN) {
 		/* bail because we already have a command pending */
@@ -534,7 +543,9 @@  void i40evf_add_vlans(struct i40evf_adapter *adapter)
 		count = (I40EVF_MAX_AQ_BUF_SIZE -
 			 sizeof(struct i40e_virtchnl_vlan_filter_list)) /
 			sizeof(u16);
-		len = I40EVF_MAX_AQ_BUF_SIZE;
+		len = sizeof(struct i40e_virtchnl_vlan_filter_list) +
+		      (count * sizeof(u16));
+		more = true;
 	}
 	vvfl = kzalloc(len, GFP_ATOMIC);
 	if (!vvfl)
@@ -549,7 +560,8 @@  void i40evf_add_vlans(struct i40evf_adapter *adapter)
 			f->add = false;
 		}
 	}
-	adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
+	if (!more)
+		adapter->aq_required &= ~I40EVF_FLAG_AQ_ADD_VLAN_FILTER;
 	i40evf_send_pf_msg(adapter, I40E_VIRTCHNL_OP_ADD_VLAN, (u8 *)vvfl, len);
 	kfree(vvfl);
 }
@@ -567,6 +579,7 @@  void i40evf_del_vlans(struct i40evf_adapter *adapter)
 	struct i40e_virtchnl_vlan_filter_list *vvfl;
 	struct i40evf_vlan_filter *f, *ftmp;
 	int len, i = 0, count = 0;
+	bool more = false;
 
 	if (adapter->current_op != I40E_VIRTCHNL_OP_UNKNOWN) {
 		/* bail because we already have a command pending */
@@ -592,7 +605,9 @@  void i40evf_del_vlans(struct i40evf_adapter *adapter)
 		count = (I40EVF_MAX_AQ_BUF_SIZE -
 			 sizeof(struct i40e_virtchnl_vlan_filter_list)) /
 			sizeof(u16);
-		len = I40EVF_MAX_AQ_BUF_SIZE;
+		len = sizeof(struct i40e_virtchnl_vlan_filter_list) +
+		      (count * sizeof(u16));
+		more = true;
 	}
 	vvfl = kzalloc(len, GFP_ATOMIC);
 	if (!vvfl)
@@ -608,7 +623,8 @@  void i40evf_del_vlans(struct i40evf_adapter *adapter)
 			kfree(f);
 		}
 	}
-	adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_VLAN_FILTER;
+	if (!more)
+		adapter->aq_required &= ~I40EVF_FLAG_AQ_DEL_VLAN_FILTER;
 	i40evf_send_pf_msg(adapter, I40E_VIRTCHNL_OP_DEL_VLAN, (u8 *)vvfl, len);
 	kfree(vvfl);
 }