[net-next 0/3][pull request] 40GbE Intel Wired LAN Driver Updates 2020-10-07

[net-next 0/3][pull request] 40GbE Intel Wired LAN Driver Updates 2020-10-07

[Tony Nguyen]

This series contains updates to i40e and e1000 drivers.

Jaroslaw adds support for changing FEC on i40e if the firmware supports it.

Aleksandr fixes setting and reporting of VF MAC address under various
circumstances for i40e.

Jesse fixes a kbuild-bot warning regarding ternary operator on e1000. 

The following are changes since commit 9faebeb2d80065926dfbc09cb73b1bb7779a89cd:
  Merge branch 'ethtool-allow-dumping-policies-to-user-space'
and are available in the git repository at:
  https://github.com/anguy11/next-queue.git 40GbE

Aleksandr Loktionov (1):
  i40e: Fix MAC address setting for a VF via Host/VM

Jaroslaw Gawin (1):
  i40e: Allow changing FEC settings on X722 if supported by FW

Jesse Brandeburg (1):
  e1000: remove unused and incorrect code

 drivers/net/ethernet/intel/e1000/e1000_hw.c   | 10 +------
 drivers/net/ethernet/intel/i40e/i40e_adminq.c |  6 +++++
 .../net/ethernet/intel/i40e/i40e_adminq_cmd.h |  2 ++
 .../net/ethernet/intel/i40e/i40e_ethtool.c    | 22 +++++++++++++---
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 19 ++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_type.h   |  1 +
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 26 +++++++++++++++++--
 7 files changed, 72 insertions(+), 14 deletions(-)

-- 
2.26.2

[net-next 1/3] i40e: Allow changing FEC settings on X722 if supported by FW

[Tony Nguyen]

From: Jaroslaw Gawin <jaroslawx.gawin@intel.com>

Starting with API version 1.10 firmware for X722 devices has ability
to change FEC settings in PHY. Code added in this patch allows
changing FEC settings if the capability flag indicates the device
supports this feature.

Signed-off-by: Jaroslaw Gawin <jaroslawx.gawin@intel.com>
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c |  6 +++++
 .../net/ethernet/intel/i40e/i40e_adminq_cmd.h |  2 ++
 .../net/ethernet/intel/i40e/i40e_ethtool.c    | 22 ++++++++++++++++---
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 19 ++++++++++++++++
 drivers/net/ethernet/intel/i40e/i40e_type.h   |  1 +
 5 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index c897a2863e4f..593912b17609 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -541,6 +541,12 @@ static void i40e_set_hw_flags(struct i40e_hw *hw)
 		    (aq->api_maj_ver == 1 &&
 		     aq->api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_X722))
 			hw->flags |= I40E_HW_FLAG_AQ_PHY_ACCESS_CAPABLE;
+
+		if (aq->api_maj_ver > 1 ||
+		    (aq->api_maj_ver == 1 &&
+		     aq->api_min_ver >= I40E_MINOR_VER_FW_REQUEST_FEC_X722))
+			hw->flags |= I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE;
+
 		fallthrough;
 	default:
 		break;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
index c0c8efe42fce..1e960c3c7ef0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h
@@ -24,6 +24,8 @@
 #define I40E_MINOR_VER_GET_LINK_INFO_X722 0x0009
 /* API version 1.6 for X722 devices adds ability to stop FW LLDP agent */
 #define I40E_MINOR_VER_FW_LLDP_STOPPABLE_X722 0x0006
+/* API version 1.10 for X722 devices adds ability to request FEC encoding */
+#define I40E_MINOR_VER_FW_REQUEST_FEC_X722 0x000A
 
 struct i40e_aq_desc {
 	__le16 flags;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index dc1577156bb6..b29447c0f549 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -891,6 +891,7 @@ static void i40e_get_settings_link_up(struct i40e_hw *hw,
 		if (hw_link_info->requested_speeds & I40E_LINK_SPEED_10GB)
 			ethtool_link_ksettings_add_link_mode(ks, advertising,
 							     10000baseT_Full);
+		i40e_get_settings_link_up_fec(hw_link_info->req_fec_info, ks);
 		break;
 	case I40E_PHY_TYPE_SGMII:
 		ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
@@ -1484,11 +1485,18 @@ static int i40e_set_fec_param(struct net_device *netdev,
 	int err = 0;
 
 	if (hw->device_id != I40E_DEV_ID_25G_SFP28 &&
-	    hw->device_id != I40E_DEV_ID_25G_B) {
+	    hw->device_id != I40E_DEV_ID_25G_B &&
+	    hw->device_id != I40E_DEV_ID_KX_X722) {
 		err = -EPERM;
 		goto done;
 	}
 
+	if (hw->mac.type == I40E_MAC_X722 &&
+	    !(hw->flags & I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE)) {
+		netdev_err(netdev, "Setting FEC encoding not supported by firmware. Please update the NVM image.\n");
+		return -EINVAL;
+	}
+
 	switch (fecparam->fec) {
 	case ETHTOOL_FEC_AUTO:
 		fec_cfg = I40E_AQ_SET_FEC_AUTO;
@@ -4951,8 +4959,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 		}
 	}
 
-	if (((changed_flags & I40E_FLAG_RS_FEC) ||
-	     (changed_flags & I40E_FLAG_BASE_R_FEC)) &&
+	if (changed_flags & I40E_FLAG_RS_FEC &&
 	    pf->hw.device_id != I40E_DEV_ID_25G_SFP28 &&
 	    pf->hw.device_id != I40E_DEV_ID_25G_B) {
 		dev_warn(&pf->pdev->dev,
@@ -4960,6 +4967,15 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 		return -EOPNOTSUPP;
 	}
 
+	if (changed_flags & I40E_FLAG_BASE_R_FEC &&
+	    pf->hw.device_id != I40E_DEV_ID_25G_SFP28 &&
+	    pf->hw.device_id != I40E_DEV_ID_25G_B &&
+	    pf->hw.device_id != I40E_DEV_ID_KX_X722) {
+		dev_warn(&pf->pdev->dev,
+			 "Device does not support changing FEC configuration\n");
+		return -EOPNOTSUPP;
+	}
+
 	/* Process any additional changes needed as a result of flag changes.
 	 * The changed_flags value reflects the list of bits that were
 	 * changed in the code above.
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 929c64789119..4f8a2154b93f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6622,6 +6622,25 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 			else
 				req_fec = "CL74 FC-FEC/BASE-R";
 		}
+		netdev_info(vsi->netdev,
+			    "NIC Link is Up, %sbps Full Duplex, Requested FEC: %s, Negotiated FEC: %s, Autoneg: %s, Flow Control: %s\n",
+			    speed, req_fec, fec, an, fc);
+	} else if (pf->hw.device_id == I40E_DEV_ID_KX_X722) {
+		req_fec = "None";
+		fec = "None";
+		an = "False";
+
+		if (pf->hw.phy.link_info.an_info & I40E_AQ_AN_COMPLETED)
+			an = "True";
+
+		if (pf->hw.phy.link_info.fec_info &
+		    I40E_AQ_CONFIG_FEC_KR_ENA)
+			fec = "CL74 FC-FEC/BASE-R";
+
+		if (pf->hw.phy.link_info.req_fec_info &
+		    I40E_AQ_REQUEST_FEC_KR)
+			req_fec = "CL74 FC-FEC/BASE-R";
+
 		netdev_info(vsi->netdev,
 			    "NIC Link is Up, %sbps Full Duplex, Requested FEC: %s, Negotiated FEC: %s, Autoneg: %s, Flow Control: %s\n",
 			    speed, req_fec, fec, an, fc);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 97d29df65f9e..c0bdc666f557 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -595,6 +595,7 @@ struct i40e_hw {
 #define I40E_HW_FLAG_FW_LLDP_PERSISTENT     BIT_ULL(5)
 #define I40E_HW_FLAG_AQ_PHY_ACCESS_EXTENDED BIT_ULL(6)
 #define I40E_HW_FLAG_DROP_MODE              BIT_ULL(7)
+#define I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE BIT_ULL(8)
 	u64 flags;
 
 	/* Used in set switch config AQ command */
-- 
2.26.2

Re: [net-next 1/3] i40e: Allow changing FEC settings on X722 if supported by FW

[Willem de Bruijn]

On Wed, Oct 7, 2020 at 7:11 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
> From: Jaroslaw Gawin <jaroslawx.gawin@intel.com>
>
> Starting with API version 1.10 firmware for X722 devices has ability
> to change FEC settings in PHY. Code added in this patch allows
> changing FEC settings if the capability flag indicates the device
> supports this feature.
>
> Signed-off-by: Jaroslaw Gawin <jaroslawx.gawin@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

Acked-by: Willem de Bruijn <willemb@google.com>

(for netdrv)

> @@ -1484,11 +1485,18 @@ static int i40e_set_fec_param(struct net_device *netdev,
>         int err = 0;
>
>         if (hw->device_id != I40E_DEV_ID_25G_SFP28 &&
> -           hw->device_id != I40E_DEV_ID_25G_B) {
> +           hw->device_id != I40E_DEV_ID_25G_B &&
> +           hw->device_id != I40E_DEV_ID_KX_X722) {
>                 err = -EPERM;
>                 goto done;
>         }
>
> +       if (hw->mac.type == I40E_MAC_X722 &&
> +           !(hw->flags & I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE)) {
> +               netdev_err(netdev, "Setting FEC encoding not supported by firmware. Please update the NVM image.\n");
> +               return -EINVAL;
> +       }
> +

no need to respin for this, but this early return is inconsistent with
other error paths in the function. Label done is not needed at all,
could convert them all.

Re: [net-next 1/3] i40e: Allow changing FEC settings on X722 if supported by FW

[Jakub Kicinski]

On Wed,  7 Oct 2020 16:10:48 -0700 Tony Nguyen wrote:
> +	if (hw->mac.type == I40E_MAC_X722 &&
> +	    !(hw->flags & I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE)) {
> +		netdev_err(netdev, "Setting FEC encoding not supported by firmware. Please update the NVM image.\n");
> +		return -EINVAL;

EOPNOTSUPP, since no FEC settings are supported by the device 
at the time of request.

Re: [net-next 1/3] i40e: Allow changing FEC settings on X722 if supported by FW

[Nguyen, Anthony L]

On Fri, 2020-10-09 at 16:36 -0700, Jakub Kicinski wrote:
> On Wed,  7 Oct 2020 16:10:48 -0700 Tony Nguyen wrote:
> > +	if (hw->mac.type == I40E_MAC_X722 &&
> > +	    !(hw->flags & I40E_HW_FLAG_X722_FEC_REQUEST_CAPABLE)) {
> > +		netdev_err(netdev, "Setting FEC encoding not supported
> > by firmware. Please update the NVM image.\n");
> > +		return -EINVAL;
> 
> EOPNOTSUPP, since no FEC settings are supported by the device 
> at the time of request.

I'll get this changed.

Thanks,
Tony

[net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM

[Tony Nguyen]

From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Fix MAC setting flow for the PF driver.

Without this change the MAC address setting was interpreted
incorrectly in the following use cases:
1) Print incorrect VF MAC or zero MAC
ip link show dev $pf
2) Don't preserve MAC between driver reload
rmmod iavf; modprobe iavf
3) Update VF MAC when macvlan was set
ip link add link $vf address $mac $vf.1 type macvlan
4) Failed to update mac address when VF was trusted
ip link set dev $vf address $mac

This includes all other configurations including above commands.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 .../ethernet/intel/i40e/i40e_virtchnl_pf.c    | 26 +++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index c96e2f2d4cba..4919d22d7b6b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -2713,6 +2713,10 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 				spin_unlock_bh(&vsi->mac_filter_hash_lock);
 				goto error_param;
 			}
+			if (is_valid_ether_addr(al->list[i].addr) &&
+			    is_zero_ether_addr(vf->default_lan_addr.addr))
+				ether_addr_copy(vf->default_lan_addr.addr,
+						al->list[i].addr);
 		}
 	}
 	spin_unlock_bh(&vsi->mac_filter_hash_lock);
@@ -2740,6 +2744,7 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 {
 	struct virtchnl_ether_addr_list *al =
 	    (struct virtchnl_ether_addr_list *)msg;
+	bool was_unimac_deleted = false;
 	struct i40e_pf *pf = vf->pf;
 	struct i40e_vsi *vsi = NULL;
 	i40e_status ret = 0;
@@ -2759,6 +2764,8 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 			ret = I40E_ERR_INVALID_MAC_ADDR;
 			goto error_param;
 		}
+		if (ether_addr_equal(al->list[i].addr, vf->default_lan_addr.addr))
+			was_unimac_deleted = true;
 	}
 	vsi = pf->vsi[vf->lan_vsi_idx];
 
@@ -2779,10 +2786,25 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 		dev_err(&pf->pdev->dev, "Unable to program VF %d MAC filters, error %d\n",
 			vf->vf_id, ret);
 
+	if (vf->trusted && was_unimac_deleted) {
+		struct i40e_mac_filter *f;
+		struct hlist_node *h;
+		u8 *macaddr = NULL;
+		int bkt;
+
+		/* set last unicast mac address as default */
+		spin_lock_bh(&vsi->mac_filter_hash_lock);
+		hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
+			if (is_valid_ether_addr(f->macaddr))
+				macaddr = f->macaddr;
+		}
+		if (macaddr)
+			ether_addr_copy(vf->default_lan_addr.addr, macaddr);
+		spin_unlock_bh(&vsi->mac_filter_hash_lock);
+	}
 error_param:
 	/* send the response to the VF */
-	return i40e_vc_send_resp_to_vf(vf, VIRTCHNL_OP_DEL_ETH_ADDR,
-				       ret);
+	return i40e_vc_send_resp_to_vf(vf, VIRTCHNL_OP_DEL_ETH_ADDR, ret);
 }
 
 /**
-- 
2.26.2

Re: [net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM

[Willem de Bruijn]

On Wed, Oct 7, 2020 at 7:11 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>
> Fix MAC setting flow for the PF driver.
>
> Without this change the MAC address setting was interpreted
> incorrectly in the following use cases:
> 1) Print incorrect VF MAC or zero MAC
> ip link show dev $pf
> 2) Don't preserve MAC between driver reload
> rmmod iavf; modprobe iavf
> 3) Update VF MAC when macvlan was set
> ip link add link $vf address $mac $vf.1 type macvlan
> 4) Failed to update mac address when VF was trusted
> ip link set dev $vf address $mac
>
> This includes all other configurations including above commands.
>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

If this is a fix, should it target net and/or is there a commit for a Fixes tag?

> @@ -2740,6 +2744,7 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>  {
>         struct virtchnl_ether_addr_list *al =
>             (struct virtchnl_ether_addr_list *)msg;
> +       bool was_unimac_deleted = false;
>         struct i40e_pf *pf = vf->pf;
>         struct i40e_vsi *vsi = NULL;
>         i40e_status ret = 0;
> @@ -2759,6 +2764,8 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                         ret = I40E_ERR_INVALID_MAC_ADDR;
>                         goto error_param;
>                 }
> +               if (ether_addr_equal(al->list[i].addr, vf->default_lan_addr.addr))
> +                       was_unimac_deleted = true;
>         }
>         vsi = pf->vsi[vf->lan_vsi_idx];
>
> @@ -2779,10 +2786,25 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                 dev_err(&pf->pdev->dev, "Unable to program VF %d MAC filters, error %d\n",
>                         vf->vf_id, ret);
>
> +       if (vf->trusted && was_unimac_deleted) {
> +               struct i40e_mac_filter *f;
> +               struct hlist_node *h;
> +               u8 *macaddr = NULL;
> +               int bkt;
> +
> +               /* set last unicast mac address as default */
> +               spin_lock_bh(&vsi->mac_filter_hash_lock);
> +               hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
> +                       if (is_valid_ether_addr(f->macaddr))
> +                               macaddr = f->macaddr;

nit: could break here

Re: [net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM

[Nguyen, Anthony L]

On Fri, 2020-10-09 at 13:46 -0400, Willem de Bruijn wrote:
> On Wed, Oct 7, 2020 at 7:11 PM Tony Nguyen <
> anthony.l.nguyen@intel.com> wrote:
> > 
> > From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > 
> > Fix MAC setting flow for the PF driver.
> > 
> > Without this change the MAC address setting was interpreted
> > incorrectly in the following use cases:
> > 1) Print incorrect VF MAC or zero MAC
> > ip link show dev $pf
> > 2) Don't preserve MAC between driver reload
> > rmmod iavf; modprobe iavf
> > 3) Update VF MAC when macvlan was set
> > ip link add link $vf address $mac $vf.1 type macvlan
> > 4) Failed to update mac address when VF was trusted
> > ip link set dev $vf address $mac
> > 
> > This includes all other configurations including above commands.
> > 
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com
> > >
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> 
> If this is a fix, should it target net and/or is there a commit for a
> Fixes tag?

Thanks for the review Willem. I will add a fixes tag and send it to
net.

> > @@ -2740,6 +2744,7 @@ static int i40e_vc_del_mac_addr_msg(struct
> > i40e_vf *vf, u8 *msg)
> >  {
> >         struct virtchnl_ether_addr_list *al =
> >             (struct virtchnl_ether_addr_list *)msg;
> > +       bool was_unimac_deleted = false;
> >         struct i40e_pf *pf = vf->pf;
> >         struct i40e_vsi *vsi = NULL;
> >         i40e_status ret = 0;
> > @@ -2759,6 +2764,8 @@ static int i40e_vc_del_mac_addr_msg(struct
> > i40e_vf *vf, u8 *msg)
> >                         ret = I40E_ERR_INVALID_MAC_ADDR;
> >                         goto error_param;
> >                 }
> > +               if (ether_addr_equal(al->list[i].addr, vf-
> > >default_lan_addr.addr))
> > +                       was_unimac_deleted = true;
> >         }
> >         vsi = pf->vsi[vf->lan_vsi_idx];
> > 
> > @@ -2779,10 +2786,25 @@ static int i40e_vc_del_mac_addr_msg(struct
> > i40e_vf *vf, u8 *msg)
> >                 dev_err(&pf->pdev->dev, "Unable to program VF %d
> > MAC filters, error %d\n",
> >                         vf->vf_id, ret);
> > 
> > +       if (vf->trusted && was_unimac_deleted) {
> > +               struct i40e_mac_filter *f;
> > +               struct hlist_node *h;
> > +               u8 *macaddr = NULL;
> > +               int bkt;
> > +
> > +               /* set last unicast mac address as default */
> > +               spin_lock_bh(&vsi->mac_filter_hash_lock);
> > +               hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f,
> > hlist) {
> > +                       if (is_valid_ether_addr(f->macaddr))
> > +                               macaddr = f->macaddr;
> 
> nit: could break here

Will add the break.

Thanks,
Tony

RE: [net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM

[Loktionov, Aleksandr]

Good day Willem

The issue patch fixes has been introduced from the very beginning.
So as fixes tag I can suggest the very first commit 5c3c48ac6bf56367c4e89f6453cd2d61e50375bd  "i40e: implement virtual device interface"


With the best regards
Alex
ND ITP Linux 40G base driver TL 

-----Original Message-----
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> 
Sent: Friday, October 9, 2020 7:47 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
Cc: David Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Network Development <netdev@vger.kernel.org>; nhorman@redhat.com; sassmann@redhat.com; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Andrew Bowers <andrewx.bowers@intel.com>
Subject: Re: [net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM

On Wed, Oct 7, 2020 at 7:11 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
> From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>
> Fix MAC setting flow for the PF driver.
>
> Without this change the MAC address setting was interpreted 
> incorrectly in the following use cases:
> 1) Print incorrect VF MAC or zero MAC
> ip link show dev $pf
> 2) Don't preserve MAC between driver reload rmmod iavf; modprobe iavf
> 3) Update VF MAC when macvlan was set
> ip link add link $vf address $mac $vf.1 type macvlan
> 4) Failed to update mac address when VF was trusted ip link set dev 
> $vf address $mac
>
> This includes all other configurations including above commands.
>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

If this is a fix, should it target net and/or is there a commit for a Fixes tag?

> @@ -2740,6 +2744,7 @@ static int i40e_vc_del_mac_addr_msg(struct 
> i40e_vf *vf, u8 *msg)  {
>         struct virtchnl_ether_addr_list *al =
>             (struct virtchnl_ether_addr_list *)msg;
> +       bool was_unimac_deleted = false;
>         struct i40e_pf *pf = vf->pf;
>         struct i40e_vsi *vsi = NULL;
>         i40e_status ret = 0;
> @@ -2759,6 +2764,8 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                         ret = I40E_ERR_INVALID_MAC_ADDR;
>                         goto error_param;
>                 }
> +               if (ether_addr_equal(al->list[i].addr, vf->default_lan_addr.addr))
> +                       was_unimac_deleted = true;
>         }
>         vsi = pf->vsi[vf->lan_vsi_idx];
>
> @@ -2779,10 +2786,25 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                 dev_err(&pf->pdev->dev, "Unable to program VF %d MAC filters, error %d\n",
>                         vf->vf_id, ret);
>
> +       if (vf->trusted && was_unimac_deleted) {
> +               struct i40e_mac_filter *f;
> +               struct hlist_node *h;
> +               u8 *macaddr = NULL;
> +               int bkt;
> +
> +               /* set last unicast mac address as default */
> +               spin_lock_bh(&vsi->mac_filter_hash_lock);
> +               hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
> +                       if (is_valid_ether_addr(f->macaddr))
> +                               macaddr = f->macaddr;

nit: could break here
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN.
Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
 

Re: [net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM

[Willem de Bruijn]

On Fri, Oct 16, 2020 at 6:46 AM Loktionov, Aleksandr
<aleksandr.loktionov@intel.com> wrote:
>
> Good day Willem
>
> The issue patch fixes has been introduced from the very beginning.
> So as fixes tag I can suggest the very first commit 5c3c48ac6bf56367c4e89f6453cd2d61e50375bd  "i40e: implement virtual device interface"

Fixes: 5c3c48ac6bf5 ("i40e: implement virtual device interface")

Sounds great. Thanks, Alex.

>
> With the best regards
> Alex
> ND ITP Linux 40G base driver TL
>
> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: Friday, October 9, 2020 7:47 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: David Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; Network Development <netdev@vger.kernel.org>; nhorman@redhat.com; sassmann@redhat.com; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Andrew Bowers <andrewx.bowers@intel.com>
> Subject: Re: [net-next 2/3] i40e: Fix MAC address setting for a VF via Host/VM
>
> On Wed, Oct 7, 2020 at 7:11 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
> >
> > From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> >
> > Fix MAC setting flow for the PF driver.
> >
> > Without this change the MAC address setting was interpreted
> > incorrectly in the following use cases:
> > 1) Print incorrect VF MAC or zero MAC
> > ip link show dev $pf
> > 2) Don't preserve MAC between driver reload rmmod iavf; modprobe iavf
> > 3) Update VF MAC when macvlan was set
> > ip link add link $vf address $mac $vf.1 type macvlan
> > 4) Failed to update mac address when VF was trusted ip link set dev
> > $vf address $mac
> >
> > This includes all other configurations including above commands.
> >
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
>
> If this is a fix, should it target net and/or is there a commit for a Fixes tag?
>
> > @@ -2740,6 +2744,7 @@ static int i40e_vc_del_mac_addr_msg(struct
> > i40e_vf *vf, u8 *msg)  {
> >         struct virtchnl_ether_addr_list *al =
> >             (struct virtchnl_ether_addr_list *)msg;
> > +       bool was_unimac_deleted = false;
> >         struct i40e_pf *pf = vf->pf;
> >         struct i40e_vsi *vsi = NULL;
> >         i40e_status ret = 0;
> > @@ -2759,6 +2764,8 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
> >                         ret = I40E_ERR_INVALID_MAC_ADDR;
> >                         goto error_param;
> >                 }
> > +               if (ether_addr_equal(al->list[i].addr, vf->default_lan_addr.addr))
> > +                       was_unimac_deleted = true;
> >         }
> >         vsi = pf->vsi[vf->lan_vsi_idx];
> >
> > @@ -2779,10 +2786,25 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
> >                 dev_err(&pf->pdev->dev, "Unable to program VF %d MAC filters, error %d\n",
> >                         vf->vf_id, ret);
> >
> > +       if (vf->trusted && was_unimac_deleted) {
> > +               struct i40e_mac_filter *f;
> > +               struct hlist_node *h;
> > +               u8 *macaddr = NULL;
> > +               int bkt;
> > +
> > +               /* set last unicast mac address as default */
> > +               spin_lock_bh(&vsi->mac_filter_hash_lock);
> > +               hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
> > +                       if (is_valid_ether_addr(f->macaddr))
> > +                               macaddr = f->macaddr;
>
> nit: could break here
> ---------------------------------------------------------------------
> Intel Technology Poland sp. z o.o.
> ul. Sowackiego 173 | 80-298 Gdask | Sd Rejonowy Gdask Pnoc | VII Wydzia Gospodarczy Krajowego Rejestru Sdowego - KRS 101882 | NIP 957-07-52-316 | Kapita zakadowy 200.000 PLN.
> Ta wiadomo wraz z zacznikami jest przeznaczona dla okrelonego adresata i moe zawiera informacje poufne. W razie przypadkowego otrzymania tej wiadomoci, prosimy o powiadomienie nadawcy oraz trwae jej usunicie; jakiekolwiek przegldanie lub rozpowszechnianie jest zabronione.
> This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
>

[net-next 3/3] e1000: remove unused and incorrect code

[Tony Nguyen]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

The e1000_clear_vfta function was triggering a warning in kbuild-bot
testing. It's actually a bug but has no functional impact.

drivers/net/ethernet/intel/e1000/e1000_hw.c:4415:58: warning: Same expression in both branches of ternary operator. [duplicateExpressionTernary]

Fix this warning by removing the offending code and simplifying
the routine to do exactly what it did before, no functional
change.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
 drivers/net/ethernet/intel/e1000/e1000_hw.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_hw.c b/drivers/net/ethernet/intel/e1000/e1000_hw.c
index fb5af23880c3..4c0c9433bd60 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_hw.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_hw.c
@@ -4401,17 +4401,9 @@ void e1000_write_vfta(struct e1000_hw *hw, u32 offset, u32 value)
 static void e1000_clear_vfta(struct e1000_hw *hw)
 {
 	u32 offset;
-	u32 vfta_value = 0;
-	u32 vfta_offset = 0;
-	u32 vfta_bit_in_reg = 0;
 
 	for (offset = 0; offset < E1000_VLAN_FILTER_TBL_SIZE; offset++) {
-		/* If the offset we want to clear is the same offset of the
-		 * manageability VLAN ID, then clear all bits except that of the
-		 * manageability unit
-		 */
-		vfta_value = (offset == vfta_offset) ? vfta_bit_in_reg : 0;
-		E1000_WRITE_REG_ARRAY(hw, VFTA, offset, vfta_value);
+		E1000_WRITE_REG_ARRAY(hw, VFTA, offset, 0);
 		E1000_WRITE_FLUSH();
 	}
 }
-- 
2.26.2

Re: [net-next 3/3] e1000: remove unused and incorrect code

[Willem de Bruijn]

On Wed, Oct 7, 2020 at 7:11 PM Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
>
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> The e1000_clear_vfta function was triggering a warning in kbuild-bot
> testing. It's actually a bug but has no functional impact.
>
> drivers/net/ethernet/intel/e1000/e1000_hw.c:4415:58: warning: Same expression in both branches of ternary operator. [duplicateExpressionTernary]
>
> Fix this warning by removing the offending code and simplifying
> the routine to do exactly what it did before, no functional
> change.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

Acked-by: Willem de Bruijn <willemb@google.com>

(for netdrv)