On Wed, Jan 7, 2026 at 8:32 PM Simon Horman <[email protected]> wrote: > > On Mon, Dec 29, 2025 at 03:16:13PM +0800, Cindy Lu wrote: > > Factor out MAC address update logic and reuse it from handle_ctrl_mac(). > > > > This ensures that old MAC entries are removed from the MPFS table > > before adding a new one and that the forwarding rules are updated > > accordingly. If updating the flow table fails, the original MAC and > > rules are restored as much as possible to keep the software and > > hardware state consistent. > > > > Signed-off-by: Cindy Lu <[email protected]> > > --- > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 95 +++++++++++++++++-------------- > > 1 file changed, 53 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > index 6e42bae7c9a1..c87e6395b060 100644 > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > > @@ -2125,62 +2125,48 @@ static void teardown_steering(struct mlx5_vdpa_net > > *ndev) > > mlx5_destroy_flow_table(ndev->rxft); > > } > > > > -static virtio_net_ctrl_ack handle_ctrl_mac(struct mlx5_vdpa_dev *mvdev, u8 > > cmd) > > +static int mlx5_vdpa_change_new_mac(struct mlx5_vdpa_net *ndev, > > + struct mlx5_core_dev *pfmdev, > > + const u8 *new_mac) > > { > > - struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > > - struct mlx5_control_vq *cvq = &mvdev->cvq; > > - virtio_net_ctrl_ack status = VIRTIO_NET_ERR; > > - struct mlx5_core_dev *pfmdev; > > - size_t read; > > - u8 mac[ETH_ALEN], mac_back[ETH_ALEN]; > > - > > - pfmdev = pci_get_drvdata(pci_physfn(mvdev->mdev->pdev)); > > - switch (cmd) { > > - case VIRTIO_NET_CTRL_MAC_ADDR_SET: > > - read = vringh_iov_pull_iotlb(&cvq->vring, &cvq->riov, (void > > *)mac, ETH_ALEN); > > - if (read != ETH_ALEN) > > - break; > > - > > - if (!memcmp(ndev->config.mac, mac, 6)) { > > - status = VIRTIO_NET_OK; > > - break; > > - } > > + struct mlx5_vdpa_dev *mvdev = &ndev->mvdev; > > + u8 old_mac[ETH_ALEN]; > > > > - if (is_zero_ether_addr(mac)) > > - break; > > + if (is_zero_ether_addr(new_mac)) > > + return -EINVAL; > > > > - if (!is_zero_ether_addr(ndev->config.mac)) { > > - if (mlx5_mpfs_del_mac(pfmdev, ndev->config.mac)) { > > - mlx5_vdpa_warn(mvdev, "failed to delete old > > MAC %pM from MPFS table\n", > > - ndev->config.mac); > > - break; > > - } > > + if (!is_zero_ether_addr(ndev->config.mac)) { > > + if (mlx5_mpfs_del_mac(pfmdev, ndev->config.mac)) { > > + mlx5_vdpa_warn(mvdev, "failed to delete old MAC %pM > > from MPFS table\n", > > + ndev->config.mac); > > + return -EIO; > > } > > + } > > > > - if (mlx5_mpfs_add_mac(pfmdev, mac)) { > > - mlx5_vdpa_warn(mvdev, "failed to insert new MAC %pM > > into MPFS table\n", > > - mac); > > - break; > > - } > > + if (mlx5_mpfs_add_mac(pfmdev, (u8 *)new_mac)) { > > + mlx5_vdpa_warn(mvdev, "failed to insert new MAC %pM into MPFS > > table\n", > > + new_mac); > > + return -EIO; > > + } > > > > /* backup the original mac address so that if failed to add > > the forward rules > > * we could restore it > > */ > > - memcpy(mac_back, ndev->config.mac, ETH_ALEN); > > + memcpy(old_mac, ndev->config.mac, ETH_ALEN); > > > > - memcpy(ndev->config.mac, mac, ETH_ALEN); > > + memcpy(ndev->config.mac, new_mac, ETH_ALEN); > > ... > > Hi Cindy, > > I realise that this makes the diffstat significantly more verbose. > And hides material changes. So perhaps there is a nicer way to do this. > > But with the current arrangement of this patch, I think that > the indentation from just above, until the end of this function > needs to be updated. > > I.e. the following incremental patch on top of this one. > > This was flagged by Smatch. > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c > b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index c87e6395b060..c796f502b604 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -2149,48 +2149,48 @@ static int mlx5_vdpa_change_new_mac(struct > mlx5_vdpa_net *ndev, > return -EIO; > } > > - /* backup the original mac address so that if failed to add > the forward rules > - * we could restore it > - */ > - memcpy(old_mac, ndev->config.mac, ETH_ALEN); > + /* backup the original mac address so that if failed to add the > forward rules > + * we could restore it > + */ > + memcpy(old_mac, ndev->config.mac, ETH_ALEN); > > - memcpy(ndev->config.mac, new_mac, ETH_ALEN); > + memcpy(ndev->config.mac, new_mac, ETH_ALEN); > > - /* Need recreate the flow table entry, so that the packet > could forward back > - */ > - mac_vlan_del(ndev, old_mac, 0, false); > + /* Need recreate the flow table entry, so that the packet could > forward back > + */ > + mac_vlan_del(ndev, old_mac, 0, false); > > - if (mac_vlan_add(ndev, ndev->config.mac, 0, false)) { > - mlx5_vdpa_warn(mvdev, "failed to insert forward > rules, try to restore\n"); > + if (mac_vlan_add(ndev, ndev->config.mac, 0, false)) { > + mlx5_vdpa_warn(mvdev, "failed to insert forward rules, try to > restore\n"); > > - /* Although it hardly run here, we still need double > check */ > - if (is_zero_ether_addr(old_mac)) { > - mlx5_vdpa_warn(mvdev, "restore mac failed: > Original MAC is zero\n"); > - return -EIO; > - } > + /* Although it hardly run here, we still need double check */ > + if (is_zero_ether_addr(old_mac)) { > + mlx5_vdpa_warn(mvdev, "restore mac failed: Original > MAC is zero\n"); > + return -EIO; > + } > > - /* Try to restore original mac address to MFPS table, > and try to restore > - * the forward rule entry. > - */ > - if (mlx5_mpfs_del_mac(pfmdev, ndev->config.mac)) { > - mlx5_vdpa_warn(mvdev, "restore mac failed: > delete MAC %pM from MPFS table failed\n", > - ndev->config.mac); > - } > + /* Try to restore original mac address to MFPS table, and try > to restore > + * the forward rule entry. > + */ > + if (mlx5_mpfs_del_mac(pfmdev, ndev->config.mac)) { > + mlx5_vdpa_warn(mvdev, "restore mac failed: delete MAC > %pM from MPFS table failed\n", > + ndev->config.mac); > + } > > - if (mlx5_mpfs_add_mac(pfmdev, old_mac)) { > - mlx5_vdpa_warn(mvdev, "restore mac failed: > insert old MAC %pM into MPFS table failed\n", > - old_mac); > - } > + if (mlx5_mpfs_add_mac(pfmdev, old_mac)) { > + mlx5_vdpa_warn(mvdev, "restore mac failed: insert old > MAC %pM into MPFS table failed\n", > + old_mac); > + } > > - memcpy(ndev->config.mac, old_mac, ETH_ALEN); > + memcpy(ndev->config.mac, old_mac, ETH_ALEN); > > - if (mac_vlan_add(ndev, ndev->config.mac, 0, false)) > - mlx5_vdpa_warn(mvdev, "restore forward rules > failed: insert forward rules failed\n"); > + if (mac_vlan_add(ndev, ndev->config.mac, 0, false)) > + mlx5_vdpa_warn(mvdev, "restore forward rules failed: > insert forward rules failed\n"); > > - return -EIO; > - } > + return -EIO; > + } > > - return 0; > + return 0; > } > > static virtio_net_ctrl_ack handle_ctrl_mac(struct mlx5_vdpa_dev *mvdev, u8 > cmd) > Thanks Simon, will change this
