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 <[email protected]>
Signed-off-by: Emil Tantilov <[email protected]>
---
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/[email protected]/
---
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 <[email protected]>
Kind regards,
Paul
