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>
---
net/vhost-vdpa.c | 39 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index ecfa8852b5..10264d3e96 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -651,8 +651,45 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
if (unlikely(dev_written < 0)) {
return dev_written;
}
+ if (*s->status != VIRTIO_NET_OK) {
+ return -EINVAL;
+ }
+ }
+
+ if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
+ /* Load the MAC address filtering */
+ uint32_t uni_entries = n->mac_table.first_multi,
+ uni_macs_size = uni_entries * ETH_ALEN,
+ uni_size = sizeof(struct virtio_net_ctrl_mac) + uni_macs_size,
+ mul_entries = n->mac_table.in_use - uni_entries,
+ mul_macs_size = mul_entries * ETH_ALEN,
+ mul_size = sizeof(struct virtio_net_ctrl_mac) + mul_macs_size,
+ data_size = uni_size + mul_size;
+ void *data = g_malloc(data_size);
+ struct virtio_net_ctrl_mac *ctrl_mac;
+
+ /* Pack the non-multicast(unicast) MAC addresses */
+ ctrl_mac = data;
+ ctrl_mac->entries = cpu_to_le32(uni_entries);
+ memcpy(ctrl_mac->macs, n->mac_table.macs, uni_macs_size);
+
+ /* Pack the multicast MAC addresses */
+ ctrl_mac = data + uni_size;
+ ctrl_mac->entries = cpu_to_le32(mul_entries);
+ memcpy(ctrl_mac->macs, &n->mac_table.macs[uni_macs_size],
+ mul_macs_size);
+
+ ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_MAC,
+ VIRTIO_NET_CTRL_MAC_TABLE_SET,
+ data, data_size);
+ g_free(data);
- return *s->status != VIRTIO_NET_OK;
+ if (unlikely(dev_written < 0)) {
+ return dev_written;
+ }
+ if (*s->status != VIRTIO_NET_OK) {
+ return -EINVAL;
+ }
}
return 0;
--
2.25.1On Thu, Jun 22, 2023 at 5:02 AM 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>
> ---
> net/vhost-vdpa.c | 39 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index ecfa8852b5..10264d3e96 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -651,8 +651,45 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
> if (unlikely(dev_written < 0)) {
> return dev_written;
> }
> + if (*s->status != VIRTIO_NET_OK) {
> + return -EINVAL;
> + }
I think this part should go in its individual patch, explaining why it
is needed and with corresponding Fixes tag.
> + }
> +
> + if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
> + /* Load the MAC address filtering */
> + uint32_t uni_entries = n->mac_table.first_multi,
> + uni_macs_size = uni_entries * ETH_ALEN,
> + uni_size = sizeof(struct virtio_net_ctrl_mac) + uni_macs_size,
> + mul_entries = n->mac_table.in_use - uni_entries,
> + mul_macs_size = mul_entries * ETH_ALEN,
> + mul_size = sizeof(struct virtio_net_ctrl_mac) + mul_macs_size,
> + data_size = uni_size + mul_size;
> + void *data = g_malloc(data_size);
If we keep this part, please use g_autofree here [1].
But I think it is not worth copying all the data actually. Maybe it is
worth it to convert vhost_vdpa_net_load_cmd to const iovec?
Thanks!
[1] https://www.qemu.org/docs/master/devel/style#automatic-memory-deallocation
> + struct virtio_net_ctrl_mac *ctrl_mac;
> +
> + /* Pack the non-multicast(unicast) MAC addresses */
> + ctrl_mac = data;
> + ctrl_mac->entries = cpu_to_le32(uni_entries);
> + memcpy(ctrl_mac->macs, n->mac_table.macs, uni_macs_size);
> +
> + /* Pack the multicast MAC addresses */
> + ctrl_mac = data + uni_size;
> + ctrl_mac->entries = cpu_to_le32(mul_entries);
> + memcpy(ctrl_mac->macs, &n->mac_table.macs[uni_macs_size],
> + mul_macs_size);
> +
> + ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_MAC,
> + VIRTIO_NET_CTRL_MAC_TABLE_SET,
> + data, data_size);
> + g_free(data);
>
> - return *s->status != VIRTIO_NET_OK;
> + if (unlikely(dev_written < 0)) {
> + return dev_written;
> + }
> + if (*s->status != VIRTIO_NET_OK) {
> + return -EINVAL;
> + }
> }
>
> return 0;
> --
> 2.25.1
>
On 2023/6/25 18:37, Eugenio Perez Martin wrote:
> On Thu, Jun 22, 2023 at 5:02 AM 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>
>> ---
>> net/vhost-vdpa.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index ecfa8852b5..10264d3e96 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -651,8 +651,45 @@ static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
>> if (unlikely(dev_written < 0)) {
>> return dev_written;
>> }
>> + if (*s->status != VIRTIO_NET_OK) {
>> + return -EINVAL;
>> + }
>
> I think this part should go in its individual patch, explaining why it
> is needed and with corresponding Fixes tag.
Yes, you are right. And I have already submitted that patch "Return
-EINVAL if device's ack is VIRTIO_NET_ERR" at [1], as mentioned in the
cover letter for this series at [2].
[1]. https://lore.kernel.org/all/cover.1686746406.git.yin31149@gmail.com/#t
[2]. https://lore.kernel.org/all/cover.1687402580.git.yin31149@gmail.com/
>
>> + }
>> +
>> + if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
>> + /* Load the MAC address filtering */
>> + uint32_t uni_entries = n->mac_table.first_multi,
>> + uni_macs_size = uni_entries * ETH_ALEN,
>> + uni_size = sizeof(struct virtio_net_ctrl_mac) + uni_macs_size,
>> + mul_entries = n->mac_table.in_use - uni_entries,
>> + mul_macs_size = mul_entries * ETH_ALEN,
>> + mul_size = sizeof(struct virtio_net_ctrl_mac) + mul_macs_size,
>> + data_size = uni_size + mul_size;
>> + void *data = g_malloc(data_size);
>
> If we keep this part, please use g_autofree here [1].
>
> But I think it is not worth copying all the data actually. Maybe it is
> worth it to convert vhost_vdpa_net_load_cmd to const iovec?
Yes, you are right. I will try to add a helper function in the v2 patch
as you have suggested.
Thanks!
>
> Thanks!
>
> [1] https://www.qemu.org/docs/master/devel/style#automatic-memory-deallocation
>
>> + struct virtio_net_ctrl_mac *ctrl_mac;
>> +
>> + /* Pack the non-multicast(unicast) MAC addresses */
>> + ctrl_mac = data;
>> + ctrl_mac->entries = cpu_to_le32(uni_entries);
>> + memcpy(ctrl_mac->macs, n->mac_table.macs, uni_macs_size);
>> +
>> + /* Pack the multicast MAC addresses */
>> + ctrl_mac = data + uni_size;
>> + ctrl_mac->entries = cpu_to_le32(mul_entries);
>> + memcpy(ctrl_mac->macs, &n->mac_table.macs[uni_macs_size],
>> + mul_macs_size);
>> +
>> + ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_MAC,
>> + VIRTIO_NET_CTRL_MAC_TABLE_SET,
>> + data, data_size);
>> + g_free(data);
>>
>> - return *s->status != VIRTIO_NET_OK;
>> + if (unlikely(dev_written < 0)) {
>> + return dev_written;
>> + }
>> + if (*s->status != VIRTIO_NET_OK) {
>> + return -EINVAL;
>> + }
>> }
>>
>> return 0;
>> --
>> 2.25.1
>>
>
© 2016 - 2026 Red Hat, Inc.