Skip to contentSkip to conversation

bnx2x: PTP crash, VF VLAN fixes

PATCH In review Michal Schmidt 3 participants 17 messages Mar 3, 2017 – Mar 7, 2017

Subsystems (3)
Networking Network drivers Broadcom BNX2X 10 gigabit Ethernet driver
lore
Share & tools
lore

Michal Schmidt

author
#0 lore raw

cover bnx2x: PTP crash, VF VLAN fixes

Hello,
here are fixes for a crash with PTP, a crash in setting of VF multicast addresses, and non-working VLAN filters configuration from the VF side.

Michal Schmidt (7):
bnx2x: prevent crash when accessing PTP with interface down
bnx2x: lower verbosity of VF stats debug messages
bnx2x: fix possible overrun of VFPF multicast addresses array
bnx2x: fix detection of VLAN filtering feature for VF
bnx2x: do not rollback VF MAC/VLAN filters we did not configure
bnx2x: fix incorrect filter count in an error message
bnx2x: add missing configuration of VF VLAN filters

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 36 ++++++++++++++++----
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 15 ++++++---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h |  1 +
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c  | 40 ++++++++++++++++-------
 4 files changed, 70 insertions(+), 22 deletions(-)
Signature · 1 line
2.9.3
author
#1 lore raw

PATCH 1/7 bnx2x: prevent crash when accessing PTP with interface down

It is possible to crash the kernel by accessing a PTP device while its associated bnx2x interface is down. Before the interface is brought up, the timecounter is not initialized, so accessing it results in NULL dereference.

Fix it by checking if the interface is up.

Use -ENETDOWN as the error code when the interface is down.
-EFAULT in bnx2x_ptp_adjfreq() did not seem right.

Tested using phc_ctl get/set/adj/freq commands.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +19 -1
index d8d06fdfc4..d57290b9ea 100644
@@ -13738,7 +13738,7 @@ static int bnx2x_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
 	if (!netif_running(bp->dev)) {
 		DP(BNX2X_MSG_PTP,
 		   "PTP adjfreq called while the interface is down\n");
-		return -EFAULT;
+		return -ENETDOWN;
 	}
 
 	if (ppb < 0) {

@@ -13797,6 +13797,12 @@ static int bnx2x_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info);
 
+	if (!netif_running(bp->dev)) {
+		DP(BNX2X_MSG_PTP,
+		   "PTP adjtime called while the interface is down\n");
+		return -ENETDOWN;
+	}
+
 	DP(BNX2X_MSG_PTP, "PTP adjtime called, delta = %llx\n", delta);
 
 	timecounter_adjtime(&bp->timecounter, delta);

@@ -13809,6 +13815,12 @@ static int bnx2x_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 	struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info);
 	u64 ns;
 
+	if (!netif_running(bp->dev)) {
+		DP(BNX2X_MSG_PTP,
+		   "PTP gettime called while the interface is down\n");
+		return -ENETDOWN;
+	}
+
 	ns = timecounter_read(&bp->timecounter);
 
 	DP(BNX2X_MSG_PTP, "PTP gettime called, ns = %llu\n", ns);

@@ -13824,6 +13836,12 @@ static int bnx2x_ptp_settime(struct ptp_clock_info *ptp,
 	struct bnx2x *bp = container_of(ptp, struct bnx2x, ptp_clock_info);
 	u64 ns;
 
+	if (!netif_running(bp->dev)) {
+		DP(BNX2X_MSG_PTP,
+		   "PTP settime called while the interface is down\n");
+		return -ENETDOWN;
+	}
+
 	ns = timespec64_to_ns(ts);
 
 	DP(BNX2X_MSG_PTP, "PTP settime called, ns = %llu\n", ns);
Signature · 1 line
2.9.3
#2 lore raw

RE: [PATCH net 1/7] bnx2x: prevent crash when accessing PTP with interface down

Quoted 4 lines

It is possible to crash the kernel by accessing a PTP device while its associated bnx2x interface is down. Before the interface is brought up, the timecounter is not initialized, so accessing it results in NULL dereference.

Quoted 8 lines

Fix it by checking if the interface is up.

Use -ENETDOWN as the error code when the interface is down.
-EFAULT in bnx2x_ptp_adjfreq() did not seem right.

Tested using phc_ctl get/set/adj/freq commands.

Signed-off-by Michal Schmidt <[email protected]>

While I have no objections to the patch contents, does it even make sense to try adjusting frequencies on a DOWNed interface? Wouldn't it make more sense checking this in the calling context Instead?

author
#3 lore raw

Re: [PATCH net 1/7] bnx2x: prevent crash when accessing PTP with interface down

Dne 5.3.2017 v 10:43 Mintz, Yuval napsal(a):

Quoted 9 lines
Quoted 9 lines

It is possible to crash the kernel by accessing a PTP device while its associated bnx2x interface is down. Before the interface is brought up, the timecounter is not initialized, so accessing it results in NULL dereference.

Fix it by checking if the interface is up.

Use -ENETDOWN as the error code when the interface is down.
-EFAULT in bnx2x_ptp_adjfreq() did not seem right.

Quoted 8 lines
Quoted 3 lines

Tested using phc_ctl get/set/adj/freq commands.

Signed-off-by Michal Schmidt <[email protected]>

While I have no objections to the patch contents, does it even make sense to try adjusting frequencies on a DOWNed interface? Wouldn't it make more sense checking this in the calling context Instead?

The caller does not know. A PTP device is not necessarily associated with a net device.

Michal

author
#4 lore raw

PATCH 2/7 bnx2x: lower verbosity of VF stats debug messages

In reply to Michal Schmidt

When BNX2X_MSG_IOV is enabled, the driver produces too many VF statistics messages. Lower the verbosity of the VF stats messages similarly as in commit 76ca70fabbdaa3 ("bnx2x: [Debug] change verbosity of some prints").

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c +8 -6
index 6fad22adbb..9f0f851774 100644
@@ -1899,7 +1899,8 @@ void bnx2x_iov_adjust_stats_req(struct bnx2x *bp)
 			continue;
 		}
 
-		DP(BNX2X_MSG_IOV, "add addresses for vf %d\n", vf->abs_vfid);
+		DP_AND((BNX2X_MSG_IOV | BNX2X_MSG_STATS),
+		       "add addresses for vf %d\n", vf->abs_vfid);
 		for_each_vfq(vf, j) {
 			struct bnx2x_vf_queue *rxq = vfq_get(vf, j);
 

@@ -1920,11 +1921,12 @@ void bnx2x_iov_adjust_stats_req(struct bnx2x *bp)
 				cpu_to_le32(U64_HI(q_stats_addr));
 			cur_query_entry->address.lo =
 				cpu_to_le32(U64_LO(q_stats_addr));
-			DP(BNX2X_MSG_IOV,
-			   "added address %x %x for vf %d queue %d client %d\n",
-			   cur_query_entry->address.hi,
-			   cur_query_entry->address.lo, cur_query_entry->funcID,
-			   j, cur_query_entry->index);
+			DP_AND((BNX2X_MSG_IOV | BNX2X_MSG_STATS),
+			       "added address %x %x for vf %d queue %d client %d\n",
+			       cur_query_entry->address.hi,
+			       cur_query_entry->address.lo,
+			       cur_query_entry->funcID,
+			       j, cur_query_entry->index);
 			cur_query_entry++;
 			cur_data_offset += sizeof(struct per_queue_stats);
 			stats_count++;
Signature · 1 line
2.9.3
author
#5 lore raw

PATCH 3/7 bnx2x: fix possible overrun of VFPF multicast addresses array

In reply to Michal Schmidt

It is too late to check for the limit of the number of VF multicast addresses after they have already been copied to the req->multicast[] array, possibly overflowing it.

Do the check before copying.

Also fix the error path to not skip unlocking vf2pf_mutex.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c +11 -12
index bfae300cf2..c2d327d9df 100644
@@ -868,7 +868,7 @@ int bnx2x_vfpf_set_mcast(struct net_device *dev)
 	struct bnx2x *bp = netdev_priv(dev);
 	struct vfpf_set_q_filters_tlv *req = &bp->vf2pf_mbox->req.set_q_filters;
 	struct pfvf_general_resp_tlv *resp = &bp->vf2pf_mbox->resp.general_resp;
-	int rc, i = 0;
+	int rc = 0, i = 0;
 	struct netdev_hw_addr *ha;
 
 	if (bp->state != BNX2X_STATE_OPEN) {

@@ -883,6 +883,15 @@ int bnx2x_vfpf_set_mcast(struct net_device *dev)
 	/* Get Rx mode requested */
 	DP(NETIF_MSG_IFUP, "dev->flags = %x\n", dev->flags);
 
+	/* We support PFVF_MAX_MULTICAST_PER_VF mcast addresses tops */
+	if (netdev_mc_count(dev) > PFVF_MAX_MULTICAST_PER_VF) {
+		DP(NETIF_MSG_IFUP,
+		   "VF supports not more than %d multicast MAC addresses\n",
+		   PFVF_MAX_MULTICAST_PER_VF);
+		rc = -EINVAL;
+		goto out;
+	}
+
 	netdev_for_each_mc_addr(ha, dev) {
 		DP(NETIF_MSG_IFUP, "Adding mcast MAC: %pM\n",
 		   bnx2x_mc_addr(ha));

@@ -890,16 +899,6 @@ int bnx2x_vfpf_set_mcast(struct net_device *dev)
 		i++;
 	}
 
-	/* We support four PFVF_MAX_MULTICAST_PER_VF mcast
-	  * addresses tops
-	  */
-	if (i >= PFVF_MAX_MULTICAST_PER_VF) {
-		DP(NETIF_MSG_IFUP,
-		   "VF supports not more than %d multicast MAC addresses\n",
-		   PFVF_MAX_MULTICAST_PER_VF);
-		return -EINVAL;
-	}
-
 	req->n_multicast = i;
 	req->flags |= VFPF_SET_Q_FILTERS_MULTICAST_CHANGED;
 	req->vf_qid = 0;

@@ -924,7 +923,7 @@ int bnx2x_vfpf_set_mcast(struct net_device *dev)
 out:
 	bnx2x_vfpf_finalize(bp, &req->first_tlv);
 
-	return 0;
+	return rc;
 }
 
 /* request pf to add a vlan for the vf */
Signature · 1 line
2.9.3
#6 lore raw

RE: [PATCH net 3/7] bnx2x: fix possible overrun of VFPF multicast addresses array

Quoted 16 lines
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c +9 -0
@@ -883,6 +883,15 @@ int bnx2x_vfpf_set_mcast(struct net_device *dev)
 	/* Get Rx mode requested */
 	DP(NETIF_MSG_IFUP, "dev->flags = %x\n", dev->flags);
 
+	/* We support PFVF_MAX_MULTICAST_PER_VF mcast addresses tops
 */
+	if (netdev_mc_count(dev) > PFVF_MAX_MULTICAST_PER_VF) {
+		DP(NETIF_MSG_IFUP,
+		   "VF supports not more than %d multicast MAC
 addresses\n",
+		   PFVF_MAX_MULTICAST_PER_VF);
+		rc = -EINVAL;
+		goto out;
+	}
+
 	netdev_for_each_mc_addr(ha, dev) {

drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c int bnx2x_vfpf_set_mcast(struct net_device *dev) · line 897

Quoted 2 lines
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c +0 -0
 		DP(NETIF_MSG_IFUP, "Adding mcast MAC: %pM\n",
 		   bnx2x_mc_addr(ha));

You can push it even higher; It's a simply sanity and can be done prior to bnx2x_vfpf_prep(), in which case you'd be able to simply return instead of touching the goto label and passing through
the prep()/finalize() sequence.

BTW, just to mention that this is artificial limitation due to the HW channel. If we'd be really motivated we can have VFs that can configure as many approx. multicasts addresses as they want [similar to PFs], although that would require a new PF driver as well [that supports a revised implementation].

author
#7 lore raw

PATCH v2 3/7 bnx2x: fix possible overrun of VFPF multicast addresses array

It is too late to check for the limit of the number of VF multicast addresses after they have already been copied to the req->multicast[] array, possibly overflowing it.

Do the check before copying.

Checking early also avoids having to (and forgetting to) unlock vf2pf_mutex.

While we're looking at the error paths in the function, also return an error code from it when the PF responds with an error. Even though the caller ignores it.

v2: Move the check before bnx2x_vfpf_prep() as suggested by Yuval.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c +10 -12
index bfae300..2b2ae92 100644
@@ -864,46 +864,44 @@ int bnx2x_vfpf_config_rss(struct bnx2x *bp,
 }
 
 int bnx2x_vfpf_set_mcast(struct net_device *dev)
 {
 	struct bnx2x *bp = netdev_priv(dev);
 	struct vfpf_set_q_filters_tlv *req = &bp->vf2pf_mbox->req.set_q_filters;
 	struct pfvf_general_resp_tlv *resp = &bp->vf2pf_mbox->resp.general_resp;
-	int rc, i = 0;
+	int rc = 0, i = 0;
 	struct netdev_hw_addr *ha;
 
 	if (bp->state != BNX2X_STATE_OPEN) {
 		DP(NETIF_MSG_IFUP, "state is %x, returning\n", bp->state);
 		return -EINVAL;
 	}
 
+	/* We support PFVF_MAX_MULTICAST_PER_VF mcast addresses tops */
+	if (netdev_mc_count(dev) > PFVF_MAX_MULTICAST_PER_VF) {
+		DP(NETIF_MSG_IFUP,
+		   "VF supports not more than %d multicast MAC addresses\n",
+		   PFVF_MAX_MULTICAST_PER_VF);
+		return -EINVAL;
+	}
+
 	/* clear mailbox and prep first tlv */
 	bnx2x_vfpf_prep(bp, &req->first_tlv, CHANNEL_TLV_SET_Q_FILTERS,
 			sizeof(*req));
 
 	/* Get Rx mode requested */
 	DP(NETIF_MSG_IFUP, "dev->flags = %x\n", dev->flags);
 
 	netdev_for_each_mc_addr(ha, dev) {
 		DP(NETIF_MSG_IFUP, "Adding mcast MAC: %pM\n",
 		   bnx2x_mc_addr(ha));
 		memcpy(req->multicast[i], bnx2x_mc_addr(ha), ETH_ALEN);
 		i++;
 	}
 
-	/* We support four PFVF_MAX_MULTICAST_PER_VF mcast
-	  * addresses tops
-	  */
-	if (i >= PFVF_MAX_MULTICAST_PER_VF) {
-		DP(NETIF_MSG_IFUP,
-		   "VF supports not more than %d multicast MAC addresses\n",
-		   PFVF_MAX_MULTICAST_PER_VF);
-		return -EINVAL;
-	}
-
 	req->n_multicast = i;
 	req->flags |= VFPF_SET_Q_FILTERS_MULTICAST_CHANGED;
 	req->vf_qid = 0;
 
 	/* add list termination tlv */
 	bnx2x_add_tlv(bp, req, req->first_tlv.tl.length, CHANNEL_TLV_LIST_END,
 		      sizeof(struct channel_list_end_tlv));

@@ -920,15 +918,15 @@ int bnx2x_vfpf_set_mcast(struct net_device *dev)
 		BNX2X_ERR("Set Rx mode/multicast failed: %d\n",
 			  resp->hdr.status);
 		rc = -EINVAL;
 	}
 out:
 	bnx2x_vfpf_finalize(bp, &req->first_tlv);
 
-	return 0;
+	return rc;
 }
 
 /* request pf to add a vlan for the vf */
 int bnx2x_vfpf_update_vlan(struct bnx2x *bp, u16 vid, u8 vf_qid, bool add)
 {
 	struct vfpf_set_q_filters_tlv *req = &bp->vf2pf_mbox->req.set_q_filters;
 	struct pfvf_general_resp_tlv *resp = &bp->vf2pf_mbox->resp.general_resp;
Signature · 1 line
2.9.3
Acked-by on v2 3/7
#8 lore raw

RE: [PATCH net 3/7 v2] bnx2x: fix possible overrun of VFPF multicast addresses array

Quoted 8 lines

It is too late to check for the limit of the number of VF multicast addresses after they have already been copied to the req->multicast[] array, possibly overflowing it.

Do the check before copying.

Checking early also avoids having to (and forgetting to) unlock vf2pf_mutex.

Quoted 7 lines

While we're looking at the error paths in the function, also return an error code from it when the PF responds with an error. Even though the caller ignores it.

v2: Move the check before bnx2x_vfpf_prep() as suggested by Yuval.

Signed-off-by Michal Schmidt <[email protected]>
Acked-by Yuval Mintz <[email protected]
author
#9 lore raw

PATCH 4/7 bnx2x: fix detection of VLAN filtering feature for VF

In reply to Michal Schmidt

VFs are currently missing the VLAN filtering feature, because we were checking the PF's acquire response before actually performing the acquire.

Fix it by setting the feature flag later when we have the PF response.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c +11 -5
index d57290b9ea..ac76fc251d 100644
@@ -13292,17 +13292,15 @@ static int bnx2x_init_dev(struct bnx2x *bp, struct pci_dev *pdev,
 	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_HIGHDMA;
 
-	/* VF with OLD Hypervisor or old PF do not support filtering */
 	if (IS_PF(bp)) {
 		if (chip_is_e1x)
 			bp->accept_any_vlan = true;
 		else
 			dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
-#ifdef CONFIG_BNX2X_SRIOV
-	} else if (bp->acquire_resp.pfdev_info.pf_cap & PFVF_CAP_VLAN_FILTER) {
-		dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
-#endif
 	}
+	/* For VF we'll know whether to enable VLAN filtering after
+	 * getting a response to CHANNEL_TLV_ACQUIRE from PF.
+	 */
 
 	dev->features |= dev->hw_features | NETIF_F_HW_VLAN_CTAG_RX;
 	dev->features |= NETIF_F_HIGHDMA;

@@ -14009,6 +14007,14 @@ static int bnx2x_init_one(struct pci_dev *pdev,
 		rc = bnx2x_vfpf_acquire(bp, tx_count, rx_count);
 		if (rc)
 			goto init_one_freemem;
+
+#ifdef CONFIG_BNX2X_SRIOV
+		/* VF with OLD Hypervisor or old PF do not support filtering */
+		if (bp->acquire_resp.pfdev_info.pf_cap & PFVF_CAP_VLAN_FILTER) {
+			dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+			dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+		}
+#endif
 	}
 
 	/* Enable SRIOV if capability found in configuration space */
Signature · 1 line
2.9.3
author
#10 lore raw

PATCH 5/7 bnx2x: do not rollback VF MAC/VLAN filters we did not configure

In reply to Michal Schmidt

On failure to configure a VF MAC/VLAN filter we should not attempt to rollback filters that we failed to configure with -EEXIST.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 8 +++++++-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c +7 -1
index 9f0f851774..2068bb8f54 100644
@@ -434,7 +434,9 @@ static int bnx2x_vf_mac_vlan_config(struct bnx2x *bp,
 
 	/* Add/Remove the filter */
 	rc = bnx2x_config_vlan_mac(bp, &ramrod);
-	if (rc && rc != -EEXIST) {
+	if (rc == -EEXIST)
+		return 0;
+	if (rc) {
 		BNX2X_ERR("Failed to %s %s\n",
 			  filter->add ? "add" : "delete",
 			  (filter->type == BNX2X_VF_FILTER_VLAN_MAC) ?

@@ -444,6 +446,8 @@ static int bnx2x_vf_mac_vlan_config(struct bnx2x *bp,
 		return rc;
 	}
 
+	filter->applied = true;
+
 	return 0;
 }
 

@@ -471,6 +475,8 @@ int bnx2x_vf_mac_vlan_config_list(struct bnx2x *bp, struct bnx2x_virtf *vf,
 		BNX2X_ERR("Managed only %d/%d filters - rolling back\n",
 			  i, filters->count + 1);
 		while (--i >= 0) {
+			if (!filters->filters[i].applied)
+				continue;
 			filters->filters[i].add = !filters->filters[i].add;
 			bnx2x_vf_mac_vlan_config(bp, vf, qid,
 						 &filters->filters[i],
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h +1 -0
index 7a6d406f4c..888d0b6632 100644
@@ -114,6 +114,7 @@ struct bnx2x_vf_mac_vlan_filter {
 	(BNX2X_VF_FILTER_MAC | BNX2X_VF_FILTER_VLAN) /*shortcut*/
 
 	bool add;
+	bool applied;
 	u8 *mac;
 	u16 vid;
 };
Signature · 1 line
2.9.3
#11 lore raw

RE: [PATCH net 5/7] bnx2x: do not rollback VF MAC/VLAN filters we did not configure

Quoted 2 lines

On failure to configure a VF MAC/VLAN filter we should not attempt to rollback filters that we failed to configure with -EEXIST.

Is this theoretical or did you actually manage to hit it? If so, did it involve non-linux VFs?

Asking as linux VFs don't actually send multiple vlan/umac configurations Via same request, and with a single filter per-message you're not expected to ever do rollback.

author
#12 lore raw

Re: [PATCH net 5/7] bnx2x: do not rollback VF MAC/VLAN filters we did not configure

Dne 5.3.2017 v 11:13 Mintz, Yuval napsal(a):

Quoted 9 lines
Quoted 2 lines

On failure to configure a VF MAC/VLAN filter we should not attempt to rollback filters that we failed to configure with -EEXIST.

Is this theoretical or did you actually manage to hit it? If so, did it involve non-linux VFs?

Asking as linux VFs don't actually send multiple vlan/umac configurations Via same request, and with a single filter per-message you're not expected to ever do rollback.

This one is theoretical, found by reading the code, not actually hitting the rollback case.

Michal

author
#13 lore raw

PATCH 6/7 bnx2x: fix incorrect filter count in an error message

In reply to Michal Schmidt

filters->count is the number of filters we were supposed to configure. There is no reason to increase it by +1 when printing the count in an error message.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c +1 -1
index 2068bb8f54..bdfd53b46b 100644
@@ -473,7 +473,7 @@ int bnx2x_vf_mac_vlan_config_list(struct bnx2x *bp, struct bnx2x_virtf *vf,
 	/* Rollback if needed */
 	if (i != filters->count) {
 		BNX2X_ERR("Managed only %d/%d filters - rolling back\n",
-			  i, filters->count + 1);
+			  i, filters->count);
 		while (--i >= 0) {
 			if (!filters->filters[i].applied)
 				continue;
Signature · 1 line
2.9.3
author
#14 lore raw

PATCH 7/7 bnx2x: add missing configuration of VF VLAN filters

In reply to Michal Schmidt

Configuring VLANs from the VF side had no effect, because the PF ignored filters of type VFPF_VLAN_FILTER in the VF-PF message.

Add the missing filter type to configure.

Signed-off-by Michal Schmidt <[email protected]>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c +17 -0
index c2d327d9df..76a4668c50 100644
@@ -1777,6 +1777,23 @@ static int bnx2x_vf_mbx_qfilters(struct bnx2x *bp, struct bnx2x_virtf *vf)
 				goto op_err;
 		}
 
+		/* build vlan list */
+		fl = NULL;
+
+		rc = bnx2x_vf_mbx_macvlan_list(bp, vf, msg, &fl,
+					       VFPF_VLAN_FILTER);
+		if (rc)
+			goto op_err;
+
+		if (fl) {
+			/* set vlan list */
+			rc = bnx2x_vf_mac_vlan_config_list(bp, vf, fl,
+							   msg->vf_qid,
+							   false);
+			if (rc)
+				goto op_err;
+		}
+
 	}
 
 	if (msg->flags & VFPF_SET_Q_FILTERS_RX_MASK_CHANGED) {
Signature · 1 line
2.9.3
Acked-by on the v1 series
#15 lore raw
In reply to Michal Schmidt
Quoted 13 lines

Hello,
here are fixes for a crash with PTP, a crash in setting of VF multicast addresses, and non-working VLAN filters configuration from the VF side.

Michal Schmidt (7):
bnx2x: prevent crash when accessing PTP with interface down
bnx2x: lower verbosity of VF stats debug messages
bnx2x: fix possible overrun of VFPF multicast addresses array
bnx2x: fix detection of VLAN filtering feature for VF
bnx2x: do not rollback VF MAC/VLAN filters we did not configure
bnx2x: fix incorrect filter count in an error message
bnx2x: add missing configuration of VF VLAN filters

Quoted 6 lines
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  | 36
++++++++++++++++----  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
| 15 ++++++---  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h |  1 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c  | 40
++++++++++++++++-------
 4 files changed, 70 insertions(+), 22 deletions(-)

Had a couple of questions/comments, but no real objections. Thanks Michal!

Acked-by Yuval Mintz <[email protected]>
[For the entire series]
maintainer
#16 lore raw
In reply to Michal Schmidt

From: Michal Schmidt <[email protected]>
Date: Fri, 3 Mar 2017 17:08:27 +0100

Quoted 2 lines

here are fixes for a crash with PTP, a crash in setting of VF multicast addresses, and non-working VLAN filters configuration from the VF side.

Series applied, thanks.