On 2023/8/17 18:18, Eugenio Perez Martin wrote:
> On Fri, Jul 7, 2023 at 5:27 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>
>> ---
>> v3:
>> - return early if mismatch the condition suggested by Eugenio
>>
>> v2:
>> https://lore.kernel.org/all/2f2560f749186c0eb1055f9926f464587e419eeb.1688051252.git.yin31...@gmail.com/
>> - 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 | 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.
Hi Eugenio,
Thanks for the detailed explanation.
Since this patch series has already been merged into master, I will
submit a separate patch to correct this problem.
I will take care of this problem in the future.
Thanks!
>
> Apart from that,
>
> Acked-by: Eugenio Pérez <epere...@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
>>
>
