diff mbox series

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

Message ID 20201007231050.1438704-3-anthony.l.nguyen@intel.com (mailing list archive)
State Not Applicable
Headers show
Series 40GbE Intel Wired LAN Driver Updates 2020-10-07 | expand

Commit Message

Tony Nguyen Oct. 7, 2020, 11:10 p.m. UTC 
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(-)

Comments

Willem de Bruijn Oct. 9, 2020, 5:46 p.m. UTC | #1 
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
Tony Nguyen Oct. 12, 2020, 4:27 p.m. UTC | #2 
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
Aleksandr Loktionov Oct. 16, 2020, 10:46 a.m. UTC | #3 
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.
Willem de Bruijn Oct. 16, 2020, 4:23 p.m. UTC | #4 
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.
>
diff mbox series

Patch

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);
 }
 
 /**