i40e: don't restart nway if autoneg not supported

PATCH New Alice Michael 2 participants 12 messages Oct 26, 2018 – Oct 31, 2018

Subsystems (2)

Network drivers Intel Ethernet

lore

Share & tools

lore

Alice Michael

author

2018-10-26 21:33 UTC #0 lore raw

PATCH 1/6 i40e: don't restart nway if autoneg not supported

From: Mitch Williams <[email protected]>

On link types that do not support autoneg, we cannot attempt to restart nway negotiation. This results in a dead link that requires a power cycle to remedy.

Fix this by saving off the autoneg state and checking this value before we try to restart nway.

Signed-off-by Mitch Williams <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +5 -5

index 9f8464f..71db006 100644
@@ -1335,6 +1335,7 @@ static int i40e_set_pauseparam(struct net_device *netdev,
 	i40e_status status;
 	u8 aq_failures;
 	int err = 0;
+	u32 is_an;
 
 	/* Changing the port's flow control is not supported if this isn't the
 	 * port's controlling PF

@@ -1347,15 +1348,14 @@ static int i40e_set_pauseparam(struct net_device *netdev,
 	if (vsi != pf->vsi[pf->lan_vsi])
 		return -EOPNOTSUPP;
 
-	if (pause->autoneg != ((hw_link_info->an_info & I40E_AQ_AN_COMPLETED) ?
-	    AUTONEG_ENABLE : AUTONEG_DISABLE)) {
+	is_an = hw_link_info->an_info & I40E_AQ_AN_COMPLETED;
+	if (pause->autoneg != is_an) {
 		netdev_info(netdev, "To change autoneg please use: ethtool -s <dev> autoneg <on|off>\n");
 		return -EOPNOTSUPP;
 	}
 
 	/* If we have link and don't have autoneg */
-	if (!test_bit(__I40E_DOWN, pf->state) &&
-	    !(hw_link_info->an_info & I40E_AQ_AN_COMPLETED)) {
+	if (!test_bit(__I40E_DOWN, pf->state) && !is_an) {
 		/* Send message that it might not necessarily work*/
 		netdev_info(netdev, "Autoneg did not complete so changing settings may not result in an actual change.\n");
 	}

@@ -1406,7 +1406,7 @@ static int i40e_set_pauseparam(struct net_device *netdev,
 		err = -EAGAIN;
 	}
 
-	if (!test_bit(__I40E_DOWN, pf->state)) {
+	if (!test_bit(__I40E_DOWN, pf->state) && is_an) {
 		/* Give it a little more time to try to come back */
 		msleep(75);
 		if (!test_bit(__I40E_DOWN, pf->state))

Signature · 1 line

2.9.5

Alice Michael

author

2018-10-26 21:33 UTC #1 lore raw

PATCH 2/6 i40e: always set ks->base.speed in i40e_get_settings_link_up

From: Jacob Keller <[email protected]>

In i40e_get_settings_link_up, set ks->base.speed to SPEED_UNKNOWN in the case where we don't know the link speed.

Signed-off-by Jacob Keller <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +1 -0

index 71db006..ec46548 100644
@@ -906,6 +906,7 @@ static void i40e_get_settings_link_up(struct i40e_hw *hw,
 		ks->base.speed = SPEED_100;
 		break;
 	default:
+		ks->base.speed = SPEED_UNKNOWN;
 		break;
 	}
 	ks->base.duplex = DUPLEX_FULL;

Signature · 1 line

2.9.5

Bowers

2018-10-31 16:26 UTC #2 lore raw

PATCH 2/6 i40e: always set ks->base.speed in i40e_get_settings_link_up

Quoted 11 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, October 26, 2018 2:33 PM
To: Michael, Alice <[email protected]>; intel-wired- lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH S98 2/6] i40e: always set ks-
Quoted 1 line
base.speed in i40e_get_settings_link_up
From: Jacob Keller <[email protected]>

Quoted 7 lines

In i40e_get_settings_link_up, set ks->base.speed to SPEED_UNKNOWN in the case where we don't know the link speed.
Signed-off-by Jacob Keller <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 1 +
 1 file changed, 1 insertion(+)

Tested-by Andrew Bowers <[email protected]>

Alice Michael

author

2018-10-26 21:33 UTC #3 lore raw

PATCH 3/6 virtchnl: white space and reorder

In reply to Alice Michael

White space change.

Move the check on the virtchnl_vsi_queue_config_info struct to be close to the struct like all the other similar checks. This keeps it clearer and easier to read.

Signed-off-by Alice Michael <[email protected]>

---
 include/linux/avf/virtchnl.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

include/linux/avf/virtchnl.h +3 -3

index b248805..3130dec 100644
@@ -171,7 +171,7 @@ struct virtchnl_msg {
 
 VIRTCHNL_CHECK_STRUCT_LEN(20, virtchnl_msg);
 
-/* Message descriptions and data structures.*/
+/* Message descriptions and data structures. */
 
 /* VIRTCHNL_OP_VERSION
  * VF posts its version number to the PF. PF responds with its version number

@@ -342,6 +342,8 @@ struct virtchnl_vsi_queue_config_info {
 	struct virtchnl_queue_pair_info qpair[1];
 };
 
+VIRTCHNL_CHECK_STRUCT_LEN(72, virtchnl_vsi_queue_config_info);
+
 /* VIRTCHNL_OP_REQUEST_QUEUES
  * VF sends this message to request the PF to allocate additional queues to
  * this VF.  Each VF gets a guaranteed number of queues on init but asking for

@@ -357,8 +359,6 @@ struct virtchnl_vf_res_request {
 	u16 num_queue_pairs;
 };
 
-VIRTCHNL_CHECK_STRUCT_LEN(72, virtchnl_vsi_queue_config_info);
-
 /* VIRTCHNL_OP_CONFIG_IRQ_MAP
  * VF uses this message to map vectors to queues.
  * The rxq_map and txq_map fields are bitmaps used to indicate which queues

Signature · 1 line

2.9.5

Bowers

2018-10-31 16:26 UTC #4 lore raw

PATCH 3/6 virtchnl: white space and reorder

Quoted 11 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, October 26, 2018 2:34 PM
To: Michael, Alice <[email protected]>; intel-wired- lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH S98 3/6] virtchnl: white space and reorder
White space change.

Quoted 8 lines

Move the check on the virtchnl_vsi_queue_config_info struct to be close to the struct like all the other similar checks.
This keeps it clearer and easier to read.
Signed-off-by Alice Michael <[email protected]>
---
 include/linux/avf/virtchnl.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Tested-by Andrew Bowers <[email protected]>

Alice Michael

author

2018-10-26 21:33 UTC #5 lore raw

PATCH 4/6 virtchnl: Fix off by one error

In reply to Alice Michael

When calculating the valid length for a VIRTCHNL_OP_ENABLE_CHANNELS message, we accidentally allowed messages with one extra virtchnl_channel_info structure on the end. This happened due to an off by one error, because we forgot that valid_len already accounted for one virtchnl_channel_info structure, so we need to subtract one from the num_tc value.

Signed-off-by Alice Michael <[email protected]>

---
 include/linux/avf/virtchnl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

include/linux/avf/virtchnl.h +2 -2

index 3130dec..7605b59 100644
@@ -819,8 +819,8 @@ virtchnl_vc_validate_vf_msg(struct virtchnl_version_info *ver, u32 v_opcode,
 		if (msglen >= valid_len) {
 			struct virtchnl_tc_info *vti =
 				(struct virtchnl_tc_info *)msg;
-			valid_len += vti->num_tc *
-				sizeof(struct virtchnl_channel_info);
+			valid_len += (vti->num_tc - 1) *
+				     sizeof(struct virtchnl_channel_info);
 			if (vti->num_tc == 0)
 				err_msg_format = true;
 		}

Signature · 1 line

2.9.5

Bowers

2018-10-31 16:27 UTC #6 lore raw

PATCH 4/6 virtchnl: Fix off by one error

Quoted 15 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, October 26, 2018 2:34 PM
To: Michael, Alice <[email protected]>; intel-wired-
lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH S98 4/6] virtchnl: Fix off by one error
When calculating the valid length for a VIRTCHNL_OP_ENABLE_CHANNELS message, we accidentally allowed messages with one extra virtchnl_channel_info structure on the end. This happened due to an off by one error, because we forgot that valid_len already accounted for one virtchnl_channel_info structure, so we need to subtract one from the num_tc value.

Quoted 4 lines

Signed-off-by Alice Michael <[email protected]>
---
 include/linux/avf/virtchnl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by Andrew Bowers <[email protected]>

Alice Michael

author

2018-10-26 21:33 UTC #7 lore raw

PATCH 5/6 i40e: Use correct shift for vlan priority

In reply to Alice Michael

From: Richard Rodriguez <[email protected]>

When using port VLAN, for VFs, and setting priority bits, the device was sending out incorrect priority bits, and also setting the CFI bit incorrectly.

To fix this, changed shift and mask bit definition for this function, to use the correct ones.

Signed-off-by Richard Rodriguez <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h +2 -2

index bf67d62..f962102 100644
@@ -13,9 +13,9 @@
 #define I40E_DEFAULT_NUM_MDD_EVENTS_ALLOWED	3
 #define I40E_DEFAULT_NUM_INVALID_MSGS_ALLOWED	10
 
-#define I40E_VLAN_PRIORITY_SHIFT	12
+#define I40E_VLAN_PRIORITY_SHIFT	13
 #define I40E_VLAN_MASK			0xFFF
-#define I40E_PRIORITY_MASK		0x7000
+#define I40E_PRIORITY_MASK		0xE000
 
 /* Various queue ctrls */
 enum i40e_queue_ctrl {

Signature · 1 line

2.9.5

Bowers

2018-10-31 16:27 UTC #8 lore raw

PATCH 5/6 i40e: Use correct shift for vlan priority

Quoted 15 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, October 26, 2018 2:34 PM
To: Michael, Alice <[email protected]>; intel-wired-
lan at lists.osuosl.org
Cc: Rodriguez, Richard <[email protected]>
Subject: [Intel-wired-lan] [next PATCH S98 5/6] i40e: Use correct shift for vlan priority
From: Richard Rodriguez <[email protected]>
When using port VLAN, for VFs, and setting priority bits, the device was sending out incorrect priority bits, and also setting the CFI bit incorrectly.

Quoted 7 lines

To fix this, changed shift and mask bit definition for this function, to use the correct ones.
Signed-off-by Richard Rodriguez <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by Andrew Bowers <[email protected]>

Alice Michael

author

2018-10-26 21:33 UTC #9 lore raw

PATCH 6/6 i40e: suppress bogus error message

In reply to Alice Michael

From: Mitch Williams <[email protected]>

The i40e driver complains about unprivileged VFs trying to configure promiscuous mode each time a VF reset occurs. This isn't the fault of the poor VF driver - the PF driver itself is making the request.

To fix this, skip the privilege check if the request is to disable all promiscuous activity. This gets rid of the bogus message, but doesn't affect privilege checks, since we really only care if the unprivileged VF is trying to enable promiscuous mode.

Testing Hints (required if no HSD): watch dmesg log while VF resets occur. You can trigger a VFR by unloading i40evf or shutting down the VM with an assigned VF.

Signed-off-by Mitch Williams <[email protected]>

---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +2 -1

index ac5698e..c41e8ad 100644
@@ -1112,7 +1112,8 @@ static i40e_status i40e_config_vf_promiscuous_mode(struct i40e_vf *vf,
 	if (!i40e_vc_isvalid_vsi_id(vf, vsi_id) || !vsi)
 		return I40E_ERR_PARAM;
 
-	if (!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps)) {
+	if (!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) &&
+	    (allmulti || alluni)) {
 		dev_err(&pf->pdev->dev,
 			"Unprivileged VF %d is attempting to configure promiscuous mode\n",
 			vf->vf_id);

Signature · 1 line

2.9.5

Bowers

2018-10-31 16:28 UTC #10 lore raw

PATCH 6/6 i40e: suppress bogus error message

Quoted 20 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, October 26, 2018 2:34 PM
To: Michael, Alice <[email protected]>; intel-wired-
lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH S98 6/6] i40e: suppress bogus error message
From: Mitch Williams <[email protected]>
The i40e driver complains about unprivileged VFs trying to configure promiscuous mode each time a VF reset occurs. This isn't the fault of the poor VF driver - the PF driver itself is making the request.
To fix this, skip the privilege check if the request is to disable all promiscuous activity. This gets rid of the bogus message, but doesn't affect privilege checks, since we really only care if the unprivileged VF is trying to enable promiscuous mode.

Quoted 8 lines

Testing Hints (required if no HSD): watch dmesg log while VF resets occur. You can trigger a VFR by unloading i40evf or shutting down the VM with an assigned VF.
Signed-off-by Mitch Williams <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Tested-by Andrew Bowers <[email protected]>

Bowers

2018-10-31 16:26 UTC #11 lore raw

PATCH 1/6 i40e: don't restart nway if autoneg not supported

In reply to Alice Michael

Quoted 15 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Alice Michael
Sent: Friday, October 26, 2018 2:33 PM
To: Michael, Alice <[email protected]>; intel-wired-
lan at lists.osuosl.org
Subject: [Intel-wired-lan] [next PATCH S98 1/6] i40e: don't restart nway if autoneg not supported
From: Mitch Williams <[email protected]>
On link types that do not support autoneg, we cannot attempt to restart nway negotiation. This results in a dead link that requires a power cycle to remedy.

Quoted 7 lines

Fix this by saving off the autoneg state and checking this value before we try to restart nway.
Signed-off-by Mitch Williams <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Tested-by Andrew Bowers <[email protected]>