[PATCH net 0/7] bnx2x: PTP crash, VF VLAN fixes

[PATCH net 0/7] bnx2x: PTP crash, VF VLAN fixes

[Michal Schmidt]

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(-)

-- 
2.9.3

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

[Michal Schmidt]

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 <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index d8d06fdfc4..d57290b9ea 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -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);
-- 
2.9.3

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

[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 <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index 6fad22adbb..9f0f851774 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -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++;
-- 
2.9.3

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

[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 <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index bfae300cf2..c2d327d9df 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -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 */
-- 
2.9.3

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

[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 <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index d57290b9ea..ac76fc251d 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -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 */
-- 
2.9.3

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

[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 <mschmidt@redhat.com>
---
 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(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index 9f0f851774..2068bb8f54 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -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],
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
index 7a6d406f4c..888d0b6632 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
@@ -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;
 };
-- 
2.9.3

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

[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 <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index 2068bb8f54..bdfd53b46b 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -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;
-- 
2.9.3

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

[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 <mschmidt@redhat.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index c2d327d9df..76a4668c50 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -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) {
-- 
2.9.3

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

[Mintz, Yuval]

> 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 <mschmidt@redhat.com>

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?

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

[Mintz, Yuval]

> @@ -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));

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].

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

[Mintz, Yuval]

> 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.

RE: [PATCH net 0/7] bnx2x: PTP crash, VF VLAN fixes

[Mintz, Yuval]

> 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(-)

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

Acked-by: Yuval Mintz <Yuval.Mintz@cavium.com>
[For the entire series]

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

[Michal Schmidt]

Dne 5.3.2017 v 10:43 Mintz, Yuval napsal(a):
>> 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 <mschmidt@redhat.com>
>
> 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

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

[Michal Schmidt]

Dne 5.3.2017 v 11:13 Mintz, Yuval napsal(a):
>> 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

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

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

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 <mschmidt@redhat.com>
---
  drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c | 22 ++++++++++------------
  1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
index bfae300..2b2ae92 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c
@@ -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;
-- 
2.9.3

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

[Mintz, Yuval]

> 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 <mschmidt@redhat.com>

Acked-by: Yuval Mintz <Yuval.Mintz@cavium.com

Re: [PATCH net 0/7] bnx2x: PTP crash, VF VLAN fixes

[David Miller]

From: Michal Schmidt <mschmidt@redhat.com>
Date: Fri,  3 Mar 2017 17:08:27 +0100

> 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.