On Thu, Jun 29, 2023 at 5:26 PM Hawkins Jiawei <yin31...@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 <yin31...@gmail.com>
> ---
> v2:
> - use iovec suggested by Eugenio
> - avoid sending CVQ command in default state
>
> v1:
> https://lore.kernel.org/all/00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.git.yin31...@gmail.com/
>
> net/vhost-vdpa.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 0bd1c7817c..cb45c84c88 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -665,6 +665,57 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s,
> const VirtIONet *n)
> }
> }
>
> + if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
> + if (n->mac_table.in_use != 0) {
This may be just style nitpicking, but I find it more clear to return
early if conditions are not met and then send the CVQ command.
Something like:
/*
* According to ...
*/
if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) ||
(n->mac_table.in_use == 0)) {
return 0
}
uni_entries = n->mac_table.first_multi,
...
---
Now I just realized vhost_vdpa_net_load_mac does not follow this for
checking VIRTIO_NET_F_CTRL_MAC_ADDR.
I'm ok if you leave it this way though.
Thanks!
> + /*
> + * According to virtio_net_reset(), device uses an empty MAC
> filter
> + * table as its default state.
> + *
> + * 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.
> + */
> + uint32_t uni_entries = n->mac_table.first_multi,
> + 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 -EINVAL;
> + }
> + }
> + }
> +
> return 0;
> }
>
> --
> 2.25.1
>
