Re: [PATCH v3 2/7] vdpa: Restore MAC address filtering state

[Eugenio Perez Martin]

On Fri, Jul 7, 2023 at 5:27 PM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> This patch refactors vhost_vdpa_net_load_mac() to
> restore the MAC address filtering state at device's startup.
>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
> ---
> v3:
>   - return early if mismatch the condition suggested by Eugenio
>
> v2: 
> https://lore.kernel.org/all/2f2560f749186c0eb1055f9926f464587e419eeb.1688051252.git.yin31149@gmail.com/
>   - use iovec suggested by Eugenio
>   - avoid sending CVQ command in default state
>
> v1: 
> https://lore.kernel.org/all/00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.git.yin31149@gmail.com/
>
>  net/vhost-vdpa.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 31ef6ad6ec..7189ccafaf 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -660,6 +660,58 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, 
> const VirtIONet *n)
>          }
>      }
>
> +    /*
> +     * According to VirtIO standard, "The device MUST have an
> +     * empty MAC filtering table on reset.".
> +     *
> +     * Therefore, there is no need to send this CVQ command if the
> +     * driver also sets an empty MAC filter table, which aligns with
> +     * the device's defaults.
> +     *
> +     * Note that the device's defaults can mismatch the driver's
> +     * configuration only at live migration.
> +     */
> +    if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX) ||
> +        n->mac_table.in_use == 0) {
> +        return 0;
> +    }
> +
> +    uint32_t uni_entries = n->mac_table.first_multi,

QEMU coding style prefers declarations at the beginning of the code
block. Previous uses of these variable names would need to be
refactored to met this rule.

Apart from that,

Acked-by: Eugenio Pérez <eperezma@redhat.com>

> +             uni_macs_size = uni_entries * ETH_ALEN,
> +             mul_entries = n->mac_table.in_use - uni_entries,
> +             mul_macs_size = mul_entries * ETH_ALEN;
> +    struct virtio_net_ctrl_mac uni = {
> +        .entries = cpu_to_le32(uni_entries),
> +    };
> +    struct virtio_net_ctrl_mac mul = {
> +        .entries = cpu_to_le32(mul_entries),
> +    };
> +    const struct iovec data[] = {
> +        {
> +            .iov_base = &uni,
> +            .iov_len = sizeof(uni),
> +        }, {
> +            .iov_base = n->mac_table.macs,
> +            .iov_len = uni_macs_size,
> +        }, {
> +            .iov_base = &mul,
> +            .iov_len = sizeof(mul),
> +        }, {
> +            .iov_base = &n->mac_table.macs[uni_macs_size],
> +            .iov_len = mul_macs_size,
> +        },
> +    };
> +    ssize_t dev_written = vhost_vdpa_net_load_cmd(s,
> +                                VIRTIO_NET_CTRL_MAC,
> +                                VIRTIO_NET_CTRL_MAC_TABLE_SET,
> +                                data, ARRAY_SIZE(data));
> +    if (unlikely(dev_written < 0)) {
> +        return dev_written;
> +    }
> +    if (*s->status != VIRTIO_NET_OK) {
> +        return -EIO;
> +    }
> +
>      return 0;
>  }
>
> --
> 2.25.1
>