All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-13 13:56 ` Ivan Vecera
  0 siblings, 0 replies; 12+ messages in thread
From: Ivan Vecera @ 2024-03-13 13:56 UTC (permalink / raw)
  To: netdev
  Cc: aleksandr.loktionov, mschmidt, horms, Jesse Brandeburg,
	Tony Nguyen, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, moderated list:INTEL ETHERNET DRIVERS, open list

Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
administratively set MAC") fixed an issue where untrusted VF was
allowed to remove its own MAC address although this was assigned
administratively from PF. Unfortunately the introduced check
is wrong because it causes that MAC filters for other MAC addresses
including multi-cast ones are not removed.

<snip>
	if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
	    i40e_can_vf_change_mac(vf))
		was_unimac_deleted = true;
	else
		continue;

	if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
	...
</snip>

The else path with `continue` effectively skips any MAC filter
removal except one for primary MAC addr when VF is allowed to do so.
Fix the check condition so the `continue` is only done for primary
MAC address.

Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index b34c71770887..10267a300770 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 		/* Allow to delete VF primary MAC only if it was not set
 		 * administratively by PF or if VF is trusted.
 		 */
-		if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
-		    i40e_can_vf_change_mac(vf))
-			was_unimac_deleted = true;
-		else
-			continue;
+		if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
+			if (i40e_can_vf_change_mac(vf))
+				was_unimac_deleted = true;
+			else
+				continue;
+		}
 
 		if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
 			ret = -EINVAL;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-13 13:56 ` Ivan Vecera
  0 siblings, 0 replies; 12+ messages in thread
From: Ivan Vecera @ 2024-03-13 13:56 UTC (permalink / raw)
  To: netdev
  Cc: moderated list:INTEL ETHERNET DRIVERS, open list,
	aleksandr.loktionov, Eric Dumazet, Tony Nguyen, horms,
	Jakub Kicinski, Paolo Abeni, David S. Miller

Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
administratively set MAC") fixed an issue where untrusted VF was
allowed to remove its own MAC address although this was assigned
administratively from PF. Unfortunately the introduced check
is wrong because it causes that MAC filters for other MAC addresses
including multi-cast ones are not removed.

<snip>
	if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
	    i40e_can_vf_change_mac(vf))
		was_unimac_deleted = true;
	else
		continue;

	if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
	...
</snip>

The else path with `continue` effectively skips any MAC filter
removal except one for primary MAC addr when VF is allowed to do so.
Fix the check condition so the `continue` is only done for primary
MAC address.

Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
index b34c71770887..10267a300770 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
@@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
 		/* Allow to delete VF primary MAC only if it was not set
 		 * administratively by PF or if VF is trusted.
 		 */
-		if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
-		    i40e_can_vf_change_mac(vf))
-			was_unimac_deleted = true;
-		else
-			continue;
+		if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
+			if (i40e_can_vf_change_mac(vf))
+				was_unimac_deleted = true;
+			else
+				continue;
+		}
 
 		if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
 			ret = -EINVAL;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net] i40e: Fix VF MAC filter removal
  2024-03-13 13:56 ` [Intel-wired-lan] " Ivan Vecera
@ 2024-03-14 11:57   ` Michal Schmidt
  -1 siblings, 0 replies; 12+ messages in thread
From: Michal Schmidt @ 2024-03-14 11:57 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, aleksandr.loktionov, horms, Jesse Brandeburg,
	Tony Nguyen, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, moderated list:INTEL ETHERNET DRIVERS, open list

On Wed, Mar 13, 2024 at 2:56 PM Ivan Vecera <ivecera@redhat.com> wrote:
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
>
> <snip>
>         if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
>             i40e_can_vf_change_mac(vf))
>                 was_unimac_deleted = true;
>         else
>                 continue;
>
>         if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>         ...
> </snip>
>
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
>
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                 /* Allow to delete VF primary MAC only if it was not set
>                  * administratively by PF or if VF is trusted.
>                  */
> -               if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -                   i40e_can_vf_change_mac(vf))
> -                       was_unimac_deleted = true;
> -               else
> -                       continue;
> +               if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +                       if (i40e_can_vf_change_mac(vf))
> +                               was_unimac_deleted = true;
> +                       else
> +                               continue;
> +               }
>
>                 if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>                         ret = -EINVAL;
> --
> 2.43.0

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-14 11:57   ` Michal Schmidt
  0 siblings, 0 replies; 12+ messages in thread
From: Michal Schmidt @ 2024-03-14 11:57 UTC (permalink / raw)
  To: Ivan Vecera
  Cc: netdev, open list, aleksandr.loktionov, Eric Dumazet,
	Tony Nguyen, horms, moderated list:INTEL ETHERNET DRIVERS,
	Jakub Kicinski, Paolo Abeni, David S. Miller

On Wed, Mar 13, 2024 at 2:56 PM Ivan Vecera <ivecera@redhat.com> wrote:
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
>
> <snip>
>         if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
>             i40e_can_vf_change_mac(vf))
>                 was_unimac_deleted = true;
>         else
>                 continue;
>
>         if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>         ...
> </snip>
>
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
>
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                 /* Allow to delete VF primary MAC only if it was not set
>                  * administratively by PF or if VF is trusted.
>                  */
> -               if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -                   i40e_can_vf_change_mac(vf))
> -                       was_unimac_deleted = true;
> -               else
> -                       continue;
> +               if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +                       if (i40e_can_vf_change_mac(vf))
> +                               was_unimac_deleted = true;
> +                       else
> +                               continue;
> +               }
>
>                 if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>                         ret = -EINVAL;
> --
> 2.43.0

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net] i40e: Fix VF MAC filter removal
  2024-03-13 13:56 ` [Intel-wired-lan] " Ivan Vecera
@ 2024-03-14 15:54   ` Brett Creeley
  -1 siblings, 0 replies; 12+ messages in thread
From: Brett Creeley @ 2024-03-14 15:54 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: aleksandr.loktionov, mschmidt, horms, Jesse Brandeburg,
	Tony Nguyen, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, moderated list:INTEL ETHERNET DRIVERS, open list

On 3/13/2024 6:56 AM, Ivan Vecera wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
> 
> <snip>
>          if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
>              i40e_can_vf_change_mac(vf))
>                  was_unimac_deleted = true;
>          else
>                  continue;
> 
>          if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>          ...
> </snip>
> 
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
> 
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                  /* Allow to delete VF primary MAC only if it was not set
>                   * administratively by PF or if VF is trusted.
>                   */
> -               if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -                   i40e_can_vf_change_mac(vf))
> -                       was_unimac_deleted = true;
> -               else
> -                       continue;
> +               if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +                       if (i40e_can_vf_change_mac(vf))
> +                               was_unimac_deleted = true;
> +                       else
> +                               continue;
> +               }

Seems okay to me.

Reviewed-by: Brett Creeley <brett.creeley@amd.com>

> 
>                  if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>                          ret = -EINVAL;
> --
> 2.43.0
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-14 15:54   ` Brett Creeley
  0 siblings, 0 replies; 12+ messages in thread
From: Brett Creeley @ 2024-03-14 15:54 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: moderated list:INTEL ETHERNET DRIVERS, open list,
	aleksandr.loktionov, Eric Dumazet, Tony Nguyen, horms,
	Jakub Kicinski, Paolo Abeni, David S. Miller

On 3/13/2024 6:56 AM, Ivan Vecera wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
> 
> <snip>
>          if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
>              i40e_can_vf_change_mac(vf))
>                  was_unimac_deleted = true;
>          else
>                  continue;
> 
>          if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>          ...
> </snip>
> 
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
> 
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>                  /* Allow to delete VF primary MAC only if it was not set
>                   * administratively by PF or if VF is trusted.
>                   */
> -               if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -                   i40e_can_vf_change_mac(vf))
> -                       was_unimac_deleted = true;
> -               else
> -                       continue;
> +               if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +                       if (i40e_can_vf_change_mac(vf))
> +                               was_unimac_deleted = true;
> +                       else
> +                               continue;
> +               }

Seems okay to me.

Reviewed-by: Brett Creeley <brett.creeley@amd.com>

> 
>                  if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>                          ret = -EINVAL;
> --
> 2.43.0
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net] i40e: Fix VF MAC filter removal
  2024-03-13 13:56 ` [Intel-wired-lan] " Ivan Vecera
@ 2024-03-27  7:29   ` Ivan Vecera
  -1 siblings, 0 replies; 12+ messages in thread
From: Ivan Vecera @ 2024-03-27  7:29 UTC (permalink / raw)
  To: netdev, Tony Nguyen
  Cc: aleksandr.loktionov, mschmidt, horms, Jesse Brandeburg,
	Tony Nguyen, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, moderated list:INTEL ETHERNET DRIVERS, open list

On 13. 03. 24 14:56, Ivan Vecera wrote:
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
> 
> <snip>
> 	if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> 	    i40e_can_vf_change_mac(vf))
> 		was_unimac_deleted = true;
> 	else
> 		continue;
> 
> 	if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> 	...
> </snip>
> 
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
> 
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>   		/* Allow to delete VF primary MAC only if it was not set
>   		 * administratively by PF or if VF is trusted.
>   		 */
> -		if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -		    i40e_can_vf_change_mac(vf))
> -			was_unimac_deleted = true;
> -		else
> -			continue;
> +		if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +			if (i40e_can_vf_change_mac(vf))
> +				was_unimac_deleted = true;
> +			else
> +				continue;
> +		}
>   
>   		if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>   			ret = -EINVAL;

Hi Tony,
the fix is not part of your recent pull series for i40e... I have 
submitted it to 'net' instead of 'iwl-net' as it fixes recent commit 
that causes MAC filter resource leaks that should be fixed as soon as 
possible. But its status in patchwork is 'Awaiting upstream' so it has 
to be resubmitted by yourself... Or should this be picked directly by 
netdev maintainers?

Thanks,
Ivan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-27  7:29   ` Ivan Vecera
  0 siblings, 0 replies; 12+ messages in thread
From: Ivan Vecera @ 2024-03-27  7:29 UTC (permalink / raw)
  To: netdev, Tony Nguyen
  Cc: moderated list:INTEL ETHERNET DRIVERS, open list,
	aleksandr.loktionov, Eric Dumazet, Tony Nguyen, horms,
	Jakub Kicinski, Paolo Abeni, David S. Miller

On 13. 03. 24 14:56, Ivan Vecera wrote:
> Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> administratively set MAC") fixed an issue where untrusted VF was
> allowed to remove its own MAC address although this was assigned
> administratively from PF. Unfortunately the introduced check
> is wrong because it causes that MAC filters for other MAC addresses
> including multi-cast ones are not removed.
> 
> <snip>
> 	if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> 	    i40e_can_vf_change_mac(vf))
> 		was_unimac_deleted = true;
> 	else
> 		continue;
> 
> 	if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> 	...
> </snip>
> 
> The else path with `continue` effectively skips any MAC filter
> removal except one for primary MAC addr when VF is allowed to do so.
> Fix the check condition so the `continue` is only done for primary
> MAC address.
> 
> Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove administratively set MAC")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> index b34c71770887..10267a300770 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct i40e_vf *vf, u8 *msg)
>   		/* Allow to delete VF primary MAC only if it was not set
>   		 * administratively by PF or if VF is trusted.
>   		 */
> -		if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> -		    i40e_can_vf_change_mac(vf))
> -			was_unimac_deleted = true;
> -		else
> -			continue;
> +		if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> +			if (i40e_can_vf_change_mac(vf))
> +				was_unimac_deleted = true;
> +			else
> +				continue;
> +		}
>   
>   		if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
>   			ret = -EINVAL;

Hi Tony,
the fix is not part of your recent pull series for i40e... I have 
submitted it to 'net' instead of 'iwl-net' as it fixes recent commit 
that causes MAC filter resource leaks that should be fixed as soon as 
possible. But its status in patchwork is 'Awaiting upstream' so it has 
to be resubmitted by yourself... Or should this be picked directly by 
netdev maintainers?

Thanks,
Ivan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net] i40e: Fix VF MAC filter removal
  2024-03-27  7:29   ` [Intel-wired-lan] " Ivan Vecera
@ 2024-03-27 18:10     ` Tony Nguyen
  -1 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-27 18:10 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: aleksandr.loktionov, mschmidt, horms, Jesse Brandeburg,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	moderated list:INTEL ETHERNET DRIVERS, open list



On 3/27/2024 12:29 AM, Ivan Vecera wrote:

...

> Hi Tony,
> the fix is not part of your recent pull series for i40e... I have 
> submitted it to 'net' instead of 'iwl-net' as it fixes recent commit 
> that causes MAC filter resource leaks that should be fixed as soon as 
> possible. But its status in patchwork is 'Awaiting upstream' so it has 
> to be resubmitted by yourself... Or should this be picked directly by 
> netdev maintainers?

Hi Ivan,

The normal process is that it goes through the IWL trees. It's currently 
applied to the tree awaiting our validation's tested-by. I will send it 
on after that occurs.

Thanks,
Tony

> Thanks,
> Ivan
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-27 18:10     ` Tony Nguyen
  0 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-27 18:10 UTC (permalink / raw)
  To: Ivan Vecera, netdev
  Cc: open list, aleksandr.loktionov, Eric Dumazet,
	moderated list:INTEL ETHERNET DRIVERS, horms, Jakub Kicinski,
	Paolo Abeni, David S. Miller



On 3/27/2024 12:29 AM, Ivan Vecera wrote:

...

> Hi Tony,
> the fix is not part of your recent pull series for i40e... I have 
> submitted it to 'net' instead of 'iwl-net' as it fixes recent commit 
> that causes MAC filter resource leaks that should be fixed as soon as 
> possible. But its status in patchwork is 'Awaiting upstream' so it has 
> to be resubmitted by yourself... Or should this be picked directly by 
> netdev maintainers?

Hi Ivan,

The normal process is that it goes through the IWL trees. It's currently 
applied to the tree awaiting our validation's tested-by. I will send it 
on after that occurs.

Thanks,
Tony

> Thanks,
> Ivan
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
  2024-03-14 15:54   ` [Intel-wired-lan] " Brett Creeley
@ 2024-03-28 16:10     ` Romanowski, Rafal
  -1 siblings, 0 replies; 12+ messages in thread
From: Romanowski, Rafal @ 2024-03-28 16:10 UTC (permalink / raw)
  To: Brett Creeley, ivecera, netdev
  Cc: moderated list:INTEL ETHERNET DRIVERS, open list, Loktionov,
	Aleksandr, Eric Dumazet, Nguyen, Anthony L, horms,
	Jakub Kicinski, Paolo Abeni, David S. Miller

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Brett Creeley
> Sent: Thursday, March 14, 2024 4:54 PM
> To: ivecera <ivecera@redhat.com>; netdev@vger.kernel.org
> Cc: moderated list:INTEL ETHERNET DRIVERS <intel-wired-
> lan@lists.osuosl.org>; open list <linux-kernel@vger.kernel.org>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; horms@kernel.org; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller
> <davem@davemloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
> 
> On 3/13/2024 6:56 AM, Ivan Vecera wrote:
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> > administratively set MAC") fixed an issue where untrusted VF was
> > allowed to remove its own MAC address although this was assigned
> > administratively from PF. Unfortunately the introduced check is wrong
> > because it causes that MAC filters for other MAC addresses including
> > multi-cast ones are not removed.
> >
> > <snip>
> >          if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> >              i40e_can_vf_change_mac(vf))
> >                  was_unimac_deleted = true;
> >          else
> >                  continue;
> >
> >          if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> >          ...
> > </snip>
> >
> > The else path with `continue` effectively skips any MAC filter removal
> > except one for primary MAC addr when VF is allowed to do so.
> > Fix the check condition so the `continue` is only done for primary MAC
> > address.
> >
> > Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> > administratively set MAC")
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > ---
> >   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > index b34c71770887..10267a300770 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct
> i40e_vf *vf, u8 *msg)
> >                  /* Allow to delete VF primary MAC only if it was not set
> >                   * administratively by PF or if VF is trusted.
> >                   */
> > -               if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> > -                   i40e_can_vf_change_mac(vf))
> > -                       was_unimac_deleted = true;
> > -               else
> > -                       continue;
> > +               if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> > +                       if (i40e_can_vf_change_mac(vf))
> > +                               was_unimac_deleted = true;
> > +                       else
> > +                               continue;
> > +               }
> 
> Seems okay to me.
> 
> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
> 
> >
> >                  if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> >                          ret = -EINVAL;
> > --
> > 2.43.0
> >
> >

Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
@ 2024-03-28 16:10     ` Romanowski, Rafal
  0 siblings, 0 replies; 12+ messages in thread
From: Romanowski, Rafal @ 2024-03-28 16:10 UTC (permalink / raw)
  To: Brett Creeley, ivecera, netdev
  Cc: open list, Loktionov, Aleksandr, Eric Dumazet, Nguyen, Anthony L,
	horms, Jakub Kicinski, moderated list:INTEL ETHERNET DRIVERS,
	Paolo Abeni, David S. Miller

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Brett Creeley
> Sent: Thursday, March 14, 2024 4:54 PM
> To: ivecera <ivecera@redhat.com>; netdev@vger.kernel.org
> Cc: moderated list:INTEL ETHERNET DRIVERS <intel-wired-
> lan@lists.osuosl.org>; open list <linux-kernel@vger.kernel.org>; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Eric Dumazet
> <edumazet@google.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; horms@kernel.org; Jakub Kicinski
> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller
> <davem@davemloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH net] i40e: Fix VF MAC filter removal
> 
> On 3/13/2024 6:56 AM, Ivan Vecera wrote:
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > Commit 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> > administratively set MAC") fixed an issue where untrusted VF was
> > allowed to remove its own MAC address although this was assigned
> > administratively from PF. Unfortunately the introduced check is wrong
> > because it causes that MAC filters for other MAC addresses including
> > multi-cast ones are not removed.
> >
> > <snip>
> >          if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> >              i40e_can_vf_change_mac(vf))
> >                  was_unimac_deleted = true;
> >          else
> >                  continue;
> >
> >          if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> >          ...
> > </snip>
> >
> > The else path with `continue` effectively skips any MAC filter removal
> > except one for primary MAC addr when VF is allowed to do so.
> > Fix the check condition so the `continue` is only done for primary MAC
> > address.
> >
> > Fixes: 73d9629e1c8c ("i40e: Do not allow untrusted VF to remove
> > administratively set MAC")
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > ---
> >   drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 11 ++++++-----
> >   1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > index b34c71770887..10267a300770 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c
> > @@ -3143,11 +3143,12 @@ static int i40e_vc_del_mac_addr_msg(struct
> i40e_vf *vf, u8 *msg)
> >                  /* Allow to delete VF primary MAC only if it was not set
> >                   * administratively by PF or if VF is trusted.
> >                   */
> > -               if (ether_addr_equal(addr, vf->default_lan_addr.addr) &&
> > -                   i40e_can_vf_change_mac(vf))
> > -                       was_unimac_deleted = true;
> > -               else
> > -                       continue;
> > +               if (ether_addr_equal(addr, vf->default_lan_addr.addr)) {
> > +                       if (i40e_can_vf_change_mac(vf))
> > +                               was_unimac_deleted = true;
> > +                       else
> > +                               continue;
> > +               }
> 
> Seems okay to me.
> 
> Reviewed-by: Brett Creeley <brett.creeley@amd.com>
> 
> >
> >                  if (i40e_del_mac_filter(vsi, al->list[i].addr)) {
> >                          ret = -EINVAL;
> > --
> > 2.43.0
> >
> >

Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-03-28 16:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-13 13:56 [PATCH net] i40e: Fix VF MAC filter removal Ivan Vecera
2024-03-13 13:56 ` [Intel-wired-lan] " Ivan Vecera
2024-03-14 11:57 ` Michal Schmidt
2024-03-14 11:57   ` [Intel-wired-lan] " Michal Schmidt
2024-03-14 15:54 ` Brett Creeley
2024-03-14 15:54   ` [Intel-wired-lan] " Brett Creeley
2024-03-28 16:10   ` Romanowski, Rafal
2024-03-28 16:10     ` Romanowski, Rafal
2024-03-27  7:29 ` Ivan Vecera
2024-03-27  7:29   ` [Intel-wired-lan] " Ivan Vecera
2024-03-27 18:10   ` Tony Nguyen
2024-03-27 18:10     ` [Intel-wired-lan] " Tony Nguyen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.