[PATCH iwl-net v2] idpf: set mac type when adding and removing MAC filters

[PATCH iwl-net v2] idpf: set mac type when adding and removing MAC filters

[Emil Tantilov]

On control planes that allow changing the MAC address of the interface,
the driver must provide a MAC type to avoid errors such as:

idpf 0000:0a:00.0: Transaction failed (op 535)
idpf 0000:0a:00.0: Received invalid MAC filter payload (op 535) (len 0)
idpf 0000:0a:00.0: Transaction failed (op 536)

These errors occur during driver load or when changing the MAC via:
ip link set <iface> address <mac>

Add logic to set the MAC type when sending ADD/DEL (opcodes 535/536) to
the control plane. Since only one primary MAC is supported per vport, the
driver only needs to send an ADD opcode when setting it. Remove the old
address by calling __idpf_del_mac_filter(), which skips the message and
just clears the entry from the internal list.

Fixes: ce1b75d0635c ("idpf: add ptypes and MAC filter support")
Reported-by: Jian Liu <jianliu@redhat.com>
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
---
Changelog:
v2:
- Make sure to clear the primary MAC from the internal list, following
  successful change.
- Update the description to include the error on 536 opcode and
  mention the removal of the old address.

v1:
https://lore.kernel.org/intel-wired-lan/20250806192130.3197-1-emil.s.tantilov@intel.com/
---
 drivers/net/ethernet/intel/idpf/idpf_lib.c      |  9 ++++++---
 drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 11 +++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 2c2a3e85d693..26edd2cda70b 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -2345,6 +2345,7 @@ static int idpf_set_mac(struct net_device *netdev, void *p)
 	struct idpf_vport_config *vport_config;
 	struct sockaddr *addr = p;
 	struct idpf_vport *vport;
+	u8 old_addr[ETH_ALEN];
 	int err = 0;
 
 	idpf_vport_ctrl_lock(netdev);
@@ -2367,17 +2368,19 @@ static int idpf_set_mac(struct net_device *netdev, void *p)
 	if (ether_addr_equal(netdev->dev_addr, addr->sa_data))
 		goto unlock_mutex;
 
+	ether_addr_copy(old_addr, vport->default_mac_addr);
+	ether_addr_copy(vport->default_mac_addr, addr->sa_data);
 	vport_config = vport->adapter->vport_config[vport->idx];
 	err = idpf_add_mac_filter(vport, np, addr->sa_data, false);
 	if (err) {
 		__idpf_del_mac_filter(vport_config, addr->sa_data);
+		ether_addr_copy(vport->default_mac_addr, netdev->dev_addr);
 		goto unlock_mutex;
 	}
 
-	if (is_valid_ether_addr(vport->default_mac_addr))
-		idpf_del_mac_filter(vport, np, vport->default_mac_addr, false);
+	if (is_valid_ether_addr(old_addr))
+		__idpf_del_mac_filter(vport_config, old_addr);
 
-	ether_addr_copy(vport->default_mac_addr, addr->sa_data);
 	eth_hw_addr_set(netdev, addr->sa_data);
 
 unlock_mutex:
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index a028c69f7fdc..e60438633cc4 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3765,6 +3765,15 @@ u32 idpf_get_vport_id(struct idpf_vport *vport)
 	return le32_to_cpu(vport_msg->vport_id);
 }
 
+static void idpf_set_mac_type(struct idpf_vport *vport,
+			      struct virtchnl2_mac_addr *mac_addr)
+{
+	if (ether_addr_equal(vport->default_mac_addr, mac_addr->addr))
+		mac_addr->type = VIRTCHNL2_MAC_ADDR_PRIMARY;
+	else
+		mac_addr->type = VIRTCHNL2_MAC_ADDR_EXTRA;
+}
+
 /**
  * idpf_mac_filter_async_handler - Async callback for mac filters
  * @adapter: private data struct
@@ -3894,6 +3903,7 @@ int idpf_add_del_mac_filters(struct idpf_vport *vport,
 			    list) {
 		if (add && f->add) {
 			ether_addr_copy(mac_addr[i].addr, f->macaddr);
+			idpf_set_mac_type(vport, &mac_addr[i]);
 			i++;
 			f->add = false;
 			if (i == total_filters)
@@ -3901,6 +3911,7 @@ int idpf_add_del_mac_filters(struct idpf_vport *vport,
 		}
 		if (!add && f->remove) {
 			ether_addr_copy(mac_addr[i].addr, f->macaddr);
+			idpf_set_mac_type(vport, &mac_addr[i]);
 			i++;
 			f->remove = false;
 			if (i == total_filters)
-- 
2.37.3

Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: set mac type when adding and removing MAC filters

[Paul Menzel]

Dear Emil,


Thank you for the patch.

Am 13.08.25 um 04:42 schrieb Emil Tantilov:
> On control planes that allow changing the MAC address of the interface,
> the driver must provide a MAC type to avoid errors such as:
> 
> idpf 0000:0a:00.0: Transaction failed (op 535)
> idpf 0000:0a:00.0: Received invalid MAC filter payload (op 535) (len 0)
> idpf 0000:0a:00.0: Transaction failed (op 536)
> 
> These errors occur during driver load or when changing the MAC via:
> ip link set <iface> address <mac>
> 
> Add logic to set the MAC type when sending ADD/DEL (opcodes 535/536) to
> the control plane. Since only one primary MAC is supported per vport, the
> driver only needs to send an ADD opcode when setting it. Remove the old
> address by calling __idpf_del_mac_filter(), which skips the message and
> just clears the entry from the internal list.

Could this be split into two patches?

1.  Set the type
2.  Improve logic

> Fixes: ce1b75d0635c ("idpf: add ptypes and MAC filter support")
> Reported-by: Jian Liu <jianliu@redhat.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> ---
> Changelog:
> v2:
> - Make sure to clear the primary MAC from the internal list, following
>    successful change.
> - Update the description to include the error on 536 opcode and
>    mention the removal of the old address.
> 
> v1:
> https://lore.kernel.org/intel-wired-lan/20250806192130.3197-1-emil.s.tantilov@intel.com/
> ---
>   drivers/net/ethernet/intel/idpf/idpf_lib.c      |  9 ++++++---
>   drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 11 +++++++++++
>   2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index 2c2a3e85d693..26edd2cda70b 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -2345,6 +2345,7 @@ static int idpf_set_mac(struct net_device *netdev, void *p)
>   	struct idpf_vport_config *vport_config;
>   	struct sockaddr *addr = p;
>   	struct idpf_vport *vport;
> +	u8 old_addr[ETH_ALEN];

old_mac_addr?

>   	int err = 0;
>   
>   	idpf_vport_ctrl_lock(netdev);
> @@ -2367,17 +2368,19 @@ static int idpf_set_mac(struct net_device *netdev, void *p)
>   	if (ether_addr_equal(netdev->dev_addr, addr->sa_data))
>   		goto unlock_mutex;
>   
> +	ether_addr_copy(old_addr, vport->default_mac_addr);
> +	ether_addr_copy(vport->default_mac_addr, addr->sa_data);
>   	vport_config = vport->adapter->vport_config[vport->idx];
>   	err = idpf_add_mac_filter(vport, np, addr->sa_data, false);
>   	if (err) {
>   		__idpf_del_mac_filter(vport_config, addr->sa_data);
> +		ether_addr_copy(vport->default_mac_addr, netdev->dev_addr);
>   		goto unlock_mutex;
>   	}
>   
> -	if (is_valid_ether_addr(vport->default_mac_addr))
> -		idpf_del_mac_filter(vport, np, vport->default_mac_addr, false);
> +	if (is_valid_ether_addr(old_addr))
> +		__idpf_del_mac_filter(vport_config, old_addr);
>   
> -	ether_addr_copy(vport->default_mac_addr, addr->sa_data);
>   	eth_hw_addr_set(netdev, addr->sa_data);
>   
>   unlock_mutex:
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index a028c69f7fdc..e60438633cc4 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -3765,6 +3765,15 @@ u32 idpf_get_vport_id(struct idpf_vport *vport)
>   	return le32_to_cpu(vport_msg->vport_id);
>   }
>   
> +static void idpf_set_mac_type(struct idpf_vport *vport,
> +			      struct virtchnl2_mac_addr *mac_addr)
> +{
> +	if (ether_addr_equal(vport->default_mac_addr, mac_addr->addr))
> +		mac_addr->type = VIRTCHNL2_MAC_ADDR_PRIMARY;
> +	else
> +		mac_addr->type = VIRTCHNL2_MAC_ADDR_EXTRA;

I’d use the ternary operator. That way, it’s clear the same variable is 
assigned a value in each branch.

> +}
> +
>   /**
>    * idpf_mac_filter_async_handler - Async callback for mac filters
>    * @adapter: private data struct
> @@ -3894,6 +3903,7 @@ int idpf_add_del_mac_filters(struct idpf_vport *vport,
>   			    list) {
>   		if (add && f->add) {
>   			ether_addr_copy(mac_addr[i].addr, f->macaddr);
> +			idpf_set_mac_type(vport, &mac_addr[i]);
>   			i++;
>   			f->add = false;
>   			if (i == total_filters)
> @@ -3901,6 +3911,7 @@ int idpf_add_del_mac_filters(struct idpf_vport *vport,
>   		}
>   		if (!add && f->remove) {
>   			ether_addr_copy(mac_addr[i].addr, f->macaddr);
> +			idpf_set_mac_type(vport, &mac_addr[i]);
>   			i++;
>   			f->remove = false;
>   			if (i == total_filters)

The overall diff looks good. Feel free to add:

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

RE: [PATCH iwl-net v2] idpf: set mac type when adding and removing MAC filters

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Tantilov, Emil S <emil.s.tantilov@intel.com>
> Sent: Wednesday, August 13, 2025 4:42 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; jianliu@redhat.com; Schmidt, Michal
> <mschmidt@redhat.com>; decot@google.com; willemb@google.com; Hay,
> Joshua A <joshua.a.hay@intel.com>
> Subject: [PATCH iwl-net v2] idpf: set mac type when adding and
> removing MAC filters
> 
> On control planes that allow changing the MAC address of the
> interface, the driver must provide a MAC type to avoid errors such as:
> 
> idpf 0000:0a:00.0: Transaction failed (op 535) idpf 0000:0a:00.0:
> Received invalid MAC filter payload (op 535) (len 0) idpf
> 0000:0a:00.0: Transaction failed (op 536)
> 
> These errors occur during driver load or when changing the MAC via:
> ip link set <iface> address <mac>
> 
> Add logic to set the MAC type when sending ADD/DEL (opcodes 535/536)
> to the control plane. Since only one primary MAC is supported per
> vport, the driver only needs to send an ADD opcode when setting it.
> Remove the old address by calling __idpf_del_mac_filter(), which skips
> the message and just clears the entry from the internal list.
> 
> Fixes: ce1b75d0635c ("idpf: add ptypes and MAC filter support")
> Reported-by: Jian Liu <jianliu@redhat.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> ---
> Changelog:
> v2:
> - Make sure to clear the primary MAC from the internal list, following
>   successful change.
> - Update the description to include the error on 536 opcode and
>   mention the removal of the old address.
> 
> v1:
> https://lore.kernel.org/intel-wired-lan/20250806192130.3197-1-
> emil.s.tantilov@intel.com/
> ---
>  drivers/net/ethernet/intel/idpf/idpf_lib.c      |  9 ++++++---
>  drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 11 +++++++++++
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> index 2c2a3e85d693..26edd2cda70b 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -2345,6 +2345,7 @@ static int idpf_set_mac(struct net_device
> *netdev, void *p)
>  	struct idpf_vport_config *vport_config;
>  	struct sockaddr *addr = p;
>  	struct idpf_vport *vport;
> +	u8 old_addr[ETH_ALEN];
>  	int err = 0;
> 
>  	idpf_vport_ctrl_lock(netdev);
> @@ -2367,17 +2368,19 @@ static int idpf_set_mac(struct net_device
> *netdev, void *p)
>  	if (ether_addr_equal(netdev->dev_addr, addr->sa_data))
>  		goto unlock_mutex;
> 
> +	ether_addr_copy(old_addr, vport->default_mac_addr);
> +	ether_addr_copy(vport->default_mac_addr, addr->sa_data);
>  	vport_config = vport->adapter->vport_config[vport->idx];
>  	err = idpf_add_mac_filter(vport, np, addr->sa_data, false);
>  	if (err) {
>  		__idpf_del_mac_filter(vport_config, addr->sa_data);
> +		ether_addr_copy(vport->default_mac_addr, netdev-
> >dev_addr);
>  		goto unlock_mutex;
>  	}
> 
> -	if (is_valid_ether_addr(vport->default_mac_addr))
> -		idpf_del_mac_filter(vport, np, vport->default_mac_addr,
> false);
> +	if (is_valid_ether_addr(old_addr))
> +		__idpf_del_mac_filter(vport_config, old_addr);
> 
> -	ether_addr_copy(vport->default_mac_addr, addr->sa_data);
>  	eth_hw_addr_set(netdev, addr->sa_data);
> 
>  unlock_mutex:
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index a028c69f7fdc..e60438633cc4 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -3765,6 +3765,15 @@ u32 idpf_get_vport_id(struct idpf_vport *vport)
>  	return le32_to_cpu(vport_msg->vport_id);  }
> 
> +static void idpf_set_mac_type(struct idpf_vport *vport,
> +			      struct virtchnl2_mac_addr *mac_addr) {
> +	if (ether_addr_equal(vport->default_mac_addr, mac_addr->addr))
> +		mac_addr->type = VIRTCHNL2_MAC_ADDR_PRIMARY;
> +	else
> +		mac_addr->type = VIRTCHNL2_MAC_ADDR_EXTRA; }
> +
>  /**
>   * idpf_mac_filter_async_handler - Async callback for mac filters
>   * @adapter: private data struct
> @@ -3894,6 +3903,7 @@ int idpf_add_del_mac_filters(struct idpf_vport
> *vport,
>  			    list) {
>  		if (add && f->add) {
>  			ether_addr_copy(mac_addr[i].addr, f->macaddr);
> +			idpf_set_mac_type(vport, &mac_addr[i]);
>  			i++;
>  			f->add = false;
>  			if (i == total_filters)
> @@ -3901,6 +3911,7 @@ int idpf_add_del_mac_filters(struct idpf_vport
> *vport,
>  		}
>  		if (!add && f->remove) {
>  			ether_addr_copy(mac_addr[i].addr, f->macaddr);
> +			idpf_set_mac_type(vport, &mac_addr[i]);
>  			i++;
>  			f->remove = false;
>  			if (i == total_filters)
> --
> 2.37.3

Re: [Intel-wired-lan] [PATCH iwl-net v2] idpf: set mac type when adding and removing MAC filters

[Tantilov, Emil S]

On 8/12/2025 9:55 PM, Paul Menzel wrote:
> Dear Emil,
> 
> 
> Thank you for the patch.
> 
> Am 13.08.25 um 04:42 schrieb Emil Tantilov:
>> On control planes that allow changing the MAC address of the interface,
>> the driver must provide a MAC type to avoid errors such as:
>>
>> idpf 0000:0a:00.0: Transaction failed (op 535)
>> idpf 0000:0a:00.0: Received invalid MAC filter payload (op 535) (len 0)
>> idpf 0000:0a:00.0: Transaction failed (op 536)
>>
>> These errors occur during driver load or when changing the MAC via:
>> ip link set <iface> address <mac>
>>
>> Add logic to set the MAC type when sending ADD/DEL (opcodes 535/536) to
>> the control plane. Since only one primary MAC is supported per vport, the
>> driver only needs to send an ADD opcode when setting it. Remove the old
>> address by calling __idpf_del_mac_filter(), which skips the message and
>> just clears the entry from the internal list.
> 
> Could this be split into two patches?
> 
> 1.  Set the type
> 2.  Improve logic

Both changes fix the errors. In the second change DEL/536 opcode
following ADD/535 will also result in "Transaction failed (op 536)" as
it attempt to remove an address which was already cleared by the
preceding ADD/535 opcode. I will update the description to make it clear
the change is more than improvement.
> 
>> Fixes: ce1b75d0635c ("idpf: add ptypes and MAC filter support")
>> Reported-by: Jian Liu <jianliu@redhat.com>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>> ---
>> Changelog:
>> v2:
>> - Make sure to clear the primary MAC from the internal list, following
>>    successful change.
>> - Update the description to include the error on 536 opcode and
>>    mention the removal of the old address.
>>
>> v1:
>> https://lore.kernel.org/intel-wired-lan/20250806192130.3197-1- 
>> emil.s.tantilov@intel.com/
>> ---
>>   drivers/net/ethernet/intel/idpf/idpf_lib.c      |  9 ++++++---
>>   drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 11 +++++++++++
>>   2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ 
>> ethernet/intel/idpf/idpf_lib.c
>> index 2c2a3e85d693..26edd2cda70b 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
>> @@ -2345,6 +2345,7 @@ static int idpf_set_mac(struct net_device 
>> *netdev, void *p)
>>       struct idpf_vport_config *vport_config;
>>       struct sockaddr *addr = p;
>>       struct idpf_vport *vport;
>> +    u8 old_addr[ETH_ALEN];
> 
> old_mac_addr?

Sure.

> 
>>       int err = 0;
>>       idpf_vport_ctrl_lock(netdev);
>> @@ -2367,17 +2368,19 @@ static int idpf_set_mac(struct net_device 
>> *netdev, void *p)
>>       if (ether_addr_equal(netdev->dev_addr, addr->sa_data))
>>           goto unlock_mutex;
>> +    ether_addr_copy(old_addr, vport->default_mac_addr);
>> +    ether_addr_copy(vport->default_mac_addr, addr->sa_data);
>>       vport_config = vport->adapter->vport_config[vport->idx];
>>       err = idpf_add_mac_filter(vport, np, addr->sa_data, false);
>>       if (err) {
>>           __idpf_del_mac_filter(vport_config, addr->sa_data);
>> +        ether_addr_copy(vport->default_mac_addr, netdev->dev_addr);
>>           goto unlock_mutex;
>>       }
>> -    if (is_valid_ether_addr(vport->default_mac_addr))
>> -        idpf_del_mac_filter(vport, np, vport->default_mac_addr, false);
>> +    if (is_valid_ether_addr(old_addr))
>> +        __idpf_del_mac_filter(vport_config, old_addr);
>> -    ether_addr_copy(vport->default_mac_addr, addr->sa_data);
>>       eth_hw_addr_set(netdev, addr->sa_data);
>>   unlock_mutex:
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/ 
>> drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
>> index a028c69f7fdc..e60438633cc4 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
>> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
>> @@ -3765,6 +3765,15 @@ u32 idpf_get_vport_id(struct idpf_vport *vport)
>>       return le32_to_cpu(vport_msg->vport_id);
>>   }
>> +static void idpf_set_mac_type(struct idpf_vport *vport,
>> +                  struct virtchnl2_mac_addr *mac_addr)
>> +{
>> +    if (ether_addr_equal(vport->default_mac_addr, mac_addr->addr))
>> +        mac_addr->type = VIRTCHNL2_MAC_ADDR_PRIMARY;
>> +    else
>> +        mac_addr->type = VIRTCHNL2_MAC_ADDR_EXTRA;
> 
> I’d use the ternary operator. That way, it’s clear the same variable is 
> assigned a value in each branch.

The assignment is fairly isolated in just this function, but if this is
the preferred style I will change it in v3 along with the old_addr
rename.

> 
>> +}
>> +
>>   /**
>>    * idpf_mac_filter_async_handler - Async callback for mac filters
>>    * @adapter: private data struct
>> @@ -3894,6 +3903,7 @@ int idpf_add_del_mac_filters(struct idpf_vport 
>> *vport,
>>                   list) {
>>           if (add && f->add) {
>>               ether_addr_copy(mac_addr[i].addr, f->macaddr);
>> +            idpf_set_mac_type(vport, &mac_addr[i]);
>>               i++;
>>               f->add = false;
>>               if (i == total_filters)
>> @@ -3901,6 +3911,7 @@ int idpf_add_del_mac_filters(struct idpf_vport 
>> *vport,
>>           }
>>           if (!add && f->remove) {
>>               ether_addr_copy(mac_addr[i].addr, f->macaddr);
>> +            idpf_set_mac_type(vport, &mac_addr[i]);
>>               i++;
>>               f->remove = false;
>>               if (i == total_filters)
> 
> The overall diff looks good. Feel free to add:
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>

Thank you,
Emil

> 
> 
> Kind regards,
> 
> Paul