qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/3] Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support
@ 2023-06-22  3:01 Hawkins Jiawei
  2023-06-22  3:01 ` [PATCH RFC 1/3] vdpa: Restore MAC address filtering state Hawkins Jiawei
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Hawkins Jiawei @ 2023-06-22  3:01 UTC (permalink / raw)
  To: jasowang, mst, eperezma; +Cc: qemu-devel, yin31149, 18801353760

This series enables shadowed CVQ to intercept rx commands related to
VIRTIO_NET_F_CTRL_RX feature through shadowed CVQ, update the virtio
NIC device model so qemu send it in a migration, and the restore of
that rx state in the destination.

Note that this patch should be based on [1], and conflicts [2], which have not
been merged. I will submit the v2 patch after they are merged.

[1]. https://lore.kernel.org/all/cover.1685704856.git.yin31149@gmail.com/
[2]. https://lore.kernel.org/all/cover.1686746406.git.yin31149@gmail.com/

Hawkins Jiawei (3):
  vdpa: Restore MAC address filtering state
  vdpa: Restore packet receive filtering state relative with _F_CTRL_RX
    feature
  vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ

 net/vhost-vdpa.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 1 deletion(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH RFC 1/3] vdpa: Restore MAC address filtering state
  2023-06-22  3:01 [PATCH RFC 0/3] Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support Hawkins Jiawei
@ 2023-06-22  3:01 ` Hawkins Jiawei
  2023-06-25 10:37   ` Eugenio Perez Martin
  2023-06-22  3:01 ` [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature Hawkins Jiawei
  2023-06-22  3:01 ` [PATCH RFC 3/3] vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ Hawkins Jiawei
  2 siblings, 1 reply; 9+ messages in thread
From: Hawkins Jiawei @ 2023-06-22  3:01 UTC (permalink / raw)
  To: jasowang, mst, eperezma; +Cc: qemu-devel, yin31149, 18801353760

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.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature
  2023-06-22  3:01 [PATCH RFC 0/3] Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support Hawkins Jiawei
  2023-06-22  3:01 ` [PATCH RFC 1/3] vdpa: Restore MAC address filtering state Hawkins Jiawei
@ 2023-06-22  3:01 ` Hawkins Jiawei
  2023-06-25 10:56   ` Eugenio Perez Martin
  2023-06-22  3:01 ` [PATCH RFC 3/3] vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ Hawkins Jiawei
  2 siblings, 1 reply; 9+ messages in thread
From: Hawkins Jiawei @ 2023-06-22  3:01 UTC (permalink / raw)
  To: jasowang, mst, eperezma; +Cc: qemu-devel, yin31149, 18801353760

This patch introduces vhost_vdpa_net_load_rx_mode()
and vhost_vdpa_net_load_rx() to restore the packet
receive filtering state in relation to
VIRTIO_NET_F_CTRL_RX feature at device's startup.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 net/vhost-vdpa.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 10264d3e96..355a6aef15 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -754,6 +754,76 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s,
     return *s->status != VIRTIO_NET_OK;
 }
 
+static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s,
+                                       uint8_t cmd,
+                                       uint8_t on)
+{
+    ssize_t dev_written;
+    dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX,
+                                          cmd, &on, sizeof(on));
+    if (unlikely(dev_written < 0)) {
+        return dev_written;
+    }
+    if (*s->status != VIRTIO_NET_OK) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
+                                  const VirtIONet *n)
+{
+    uint8_t on;
+    int r;
+
+    if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
+        /* Load the promiscous mode */
+        if (n->mac_table.uni_overflow) {
+            /*
+             * According to VirtIO standard, "Since there are no guarantees,
+             * it can use a hash filter or silently switch to
+             * allmulti or promiscuous mode if it is given too many addresses."
+             *
+             * QEMU ignores non-multicast(unicast) MAC addresses and
+             * marks `uni_overflow` for the device internal state
+             * if guest sets too many non-multicast(unicast) MAC addresses.
+             * Therefore, we should turn promiscous mode on in this case.
+             */
+            on = 1;
+        } else {
+            on = n->promisc;
+        }
+        r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on);
+        if (r < 0) {
+            return r;
+        }
+
+        /* Load the all-multicast mode */
+        if (n->mac_table.multi_overflow) {
+            /*
+             * According to VirtIO standard, "Since there are no guarantees,
+             * it can use a hash filter or silently switch to
+             * allmulti or promiscuous mode if it is given too many addresses."
+             *
+             * QEMU ignores multicast MAC addresses and
+             * marks `multi_overflow` for the device internal state
+             * if guest sets too many multicast MAC addresses.
+             * Therefore, we should turn all-multicast mode on in this case.
+             */
+            on = 1;
+        } else {
+            on = n->allmulti;
+        }
+        r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_ALLMULTI, on);
+        if (r < 0) {
+            return r;
+        }
+    }
+
+    return 0;
+}
+
 static int vhost_vdpa_net_load(NetClientState *nc)
 {
     VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
@@ -780,6 +850,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
     if (unlikely(r)) {
         return r;
     }
+    r = vhost_vdpa_net_load_rx(s, n);
+    if (unlikely(r)) {
+        return r;
+    }
 
     return 0;
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 3/3] vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ
  2023-06-22  3:01 [PATCH RFC 0/3] Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support Hawkins Jiawei
  2023-06-22  3:01 ` [PATCH RFC 1/3] vdpa: Restore MAC address filtering state Hawkins Jiawei
  2023-06-22  3:01 ` [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature Hawkins Jiawei
@ 2023-06-22  3:01 ` Hawkins Jiawei
  2023-06-25 10:51   ` Eugenio Perez Martin
  2 siblings, 1 reply; 9+ messages in thread
From: Hawkins Jiawei @ 2023-06-22  3:01 UTC (permalink / raw)
  To: jasowang, mst, eperezma; +Cc: qemu-devel, yin31149, 18801353760

Enable SVQ with VIRTIO_NET_F_CTRL_RX feature.

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 net/vhost-vdpa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 355a6aef15..ca800f97e2 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -99,6 +99,7 @@ static const uint64_t vdpa_svq_device_features =
     BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) |
     BIT_ULL(VIRTIO_NET_F_STATUS) |
     BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
+    BIT_ULL(VIRTIO_NET_F_CTRL_RX) |
     BIT_ULL(VIRTIO_NET_F_MQ) |
     BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
     BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) |
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 1/3] vdpa: Restore MAC address filtering state
  2023-06-22  3:01 ` [PATCH RFC 1/3] vdpa: Restore MAC address filtering state Hawkins Jiawei
@ 2023-06-25 10:37   ` Eugenio Perez Martin
  2023-06-26  8:40     ` Hawkins Jiawei
  0 siblings, 1 reply; 9+ messages in thread
From: Eugenio Perez Martin @ 2023-06-25 10:37 UTC (permalink / raw)
  To: Hawkins Jiawei; +Cc: jasowang, mst, qemu-devel, 18801353760

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.

> +    }
> +
> +    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
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 3/3] vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ
  2023-06-22  3:01 ` [PATCH RFC 3/3] vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ Hawkins Jiawei
@ 2023-06-25 10:51   ` Eugenio Perez Martin
  0 siblings, 0 replies; 9+ messages in thread
From: Eugenio Perez Martin @ 2023-06-25 10:51 UTC (permalink / raw)
  To: Hawkins Jiawei; +Cc: jasowang, mst, qemu-devel, 18801353760

On Thu, Jun 22, 2023 at 5:02 AM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> Enable SVQ with VIRTIO_NET_F_CTRL_RX feature.
>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>

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

> ---
>  net/vhost-vdpa.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 355a6aef15..ca800f97e2 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -99,6 +99,7 @@ static const uint64_t vdpa_svq_device_features =
>      BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) |
>      BIT_ULL(VIRTIO_NET_F_STATUS) |
>      BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |
> +    BIT_ULL(VIRTIO_NET_F_CTRL_RX) |
>      BIT_ULL(VIRTIO_NET_F_MQ) |
>      BIT_ULL(VIRTIO_F_ANY_LAYOUT) |
>      BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) |
> --
> 2.25.1
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature
  2023-06-22  3:01 ` [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature Hawkins Jiawei
@ 2023-06-25 10:56   ` Eugenio Perez Martin
  2023-06-26  8:52     ` Hawkins Jiawei
  0 siblings, 1 reply; 9+ messages in thread
From: Eugenio Perez Martin @ 2023-06-25 10:56 UTC (permalink / raw)
  To: Hawkins Jiawei; +Cc: jasowang, mst, qemu-devel, 18801353760

On Thu, Jun 22, 2023 at 5:02 AM Hawkins Jiawei <yin31149@gmail.com> wrote:
>
> This patch introduces vhost_vdpa_net_load_rx_mode()
> and vhost_vdpa_net_load_rx() to restore the packet
> receive filtering state in relation to
> VIRTIO_NET_F_CTRL_RX feature at device's startup.
>
> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
> ---
>  net/vhost-vdpa.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 10264d3e96..355a6aef15 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -754,6 +754,76 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s,
>      return *s->status != VIRTIO_NET_OK;
>  }
>
> +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s,
> +                                       uint8_t cmd,
> +                                       uint8_t on)
> +{
> +    ssize_t dev_written;
> +    dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX,
> +                                          cmd, &on, sizeof(on));
> +    if (unlikely(dev_written < 0)) {
> +        return dev_written;
> +    }
> +    if (*s->status != VIRTIO_NET_OK) {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
> +                                  const VirtIONet *n)
> +{
> +    uint8_t on;
> +    int r;
> +
> +    if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
> +        /* Load the promiscous mode */
> +        if (n->mac_table.uni_overflow) {
> +            /*
> +             * According to VirtIO standard, "Since there are no guarantees,
> +             * it can use a hash filter or silently switch to
> +             * allmulti or promiscuous mode if it is given too many addresses."
> +             *
> +             * QEMU ignores non-multicast(unicast) MAC addresses and
> +             * marks `uni_overflow` for the device internal state
> +             * if guest sets too many non-multicast(unicast) MAC addresses.
> +             * Therefore, we should turn promiscous mode on in this case.
> +             */
> +            on = 1;
> +        } else {
> +            on = n->promisc;
> +        }
> +        r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on);
> +        if (r < 0) {
> +            return r;
> +        }
> +
> +        /* Load the all-multicast mode */
> +        if (n->mac_table.multi_overflow) {
> +            /*
> +             * According to VirtIO standard, "Since there are no guarantees,
> +             * it can use a hash filter or silently switch to
> +             * allmulti or promiscuous mode if it is given too many addresses."
> +             *
> +             * QEMU ignores multicast MAC addresses and
> +             * marks `multi_overflow` for the device internal state
> +             * if guest sets too many multicast MAC addresses.
> +             * Therefore, we should turn all-multicast mode on in this case.
> +             */
> +            on = 1;
> +        } else {
> +            on = n->allmulti;
> +        }
> +        r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_ALLMULTI, on);
> +        if (r < 0) {
> +            return r;
> +        }

Can we skip the sending of the CVQ commands if the state matches the
default state?

Thanks!

> +    }
> +
> +    return 0;
> +}
> +
>  static int vhost_vdpa_net_load(NetClientState *nc)
>  {
>      VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> @@ -780,6 +850,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>      if (unlikely(r)) {
>          return r;
>      }
> +    r = vhost_vdpa_net_load_rx(s, n);
> +    if (unlikely(r)) {
> +        return r;
> +    }
>
>      return 0;
>  }
> --
> 2.25.1
>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 1/3] vdpa: Restore MAC address filtering state
  2023-06-25 10:37   ` Eugenio Perez Martin
@ 2023-06-26  8:40     ` Hawkins Jiawei
  0 siblings, 0 replies; 9+ messages in thread
From: Hawkins Jiawei @ 2023-06-26  8:40 UTC (permalink / raw)
  To: Eugenio Perez Martin; +Cc: jasowang, mst, qemu-devel, 18801353760

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
>>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature
  2023-06-25 10:56   ` Eugenio Perez Martin
@ 2023-06-26  8:52     ` Hawkins Jiawei
  0 siblings, 0 replies; 9+ messages in thread
From: Hawkins Jiawei @ 2023-06-26  8:52 UTC (permalink / raw)
  To: Eugenio Perez Martin; +Cc: jasowang, mst, qemu-devel, 18801353760

On 2023/6/25 18:56, Eugenio Perez Martin wrote:
> On Thu, Jun 22, 2023 at 5:02 AM Hawkins Jiawei <yin31149@gmail.com> wrote:
>>
>> This patch introduces vhost_vdpa_net_load_rx_mode()
>> and vhost_vdpa_net_load_rx() to restore the packet
>> receive filtering state in relation to
>> VIRTIO_NET_F_CTRL_RX feature at device's startup.
>>
>> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
>> ---
>>   net/vhost-vdpa.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 10264d3e96..355a6aef15 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -754,6 +754,76 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s,
>>       return *s->status != VIRTIO_NET_OK;
>>   }
>>
>> +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s,
>> +                                       uint8_t cmd,
>> +                                       uint8_t on)
>> +{
>> +    ssize_t dev_written;
>> +    dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX,
>> +                                          cmd, &on, sizeof(on));
>> +    if (unlikely(dev_written < 0)) {
>> +        return dev_written;
>> +    }
>> +    if (*s->status != VIRTIO_NET_OK) {
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int vhost_vdpa_net_load_rx(VhostVDPAState *s,
>> +                                  const VirtIONet *n)
>> +{
>> +    uint8_t on;
>> +    int r;
>> +
>> +    if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) {
>> +        /* Load the promiscous mode */
>> +        if (n->mac_table.uni_overflow) {
>> +            /*
>> +             * According to VirtIO standard, "Since there are no guarantees,
>> +             * it can use a hash filter or silently switch to
>> +             * allmulti or promiscuous mode if it is given too many addresses."
>> +             *
>> +             * QEMU ignores non-multicast(unicast) MAC addresses and
>> +             * marks `uni_overflow` for the device internal state
>> +             * if guest sets too many non-multicast(unicast) MAC addresses.
>> +             * Therefore, we should turn promiscous mode on in this case.
>> +             */
>> +            on = 1;
>> +        } else {
>> +            on = n->promisc;
>> +        }
>> +        r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on);
>> +        if (r < 0) {
>> +            return r;
>> +        }
>> +
>> +        /* Load the all-multicast mode */
>> +        if (n->mac_table.multi_overflow) {
>> +            /*
>> +             * According to VirtIO standard, "Since there are no guarantees,
>> +             * it can use a hash filter or silently switch to
>> +             * allmulti or promiscuous mode if it is given too many addresses."
>> +             *
>> +             * QEMU ignores multicast MAC addresses and
>> +             * marks `multi_overflow` for the device internal state
>> +             * if guest sets too many multicast MAC addresses.
>> +             * Therefore, we should turn all-multicast mode on in this case.
>> +             */
>> +            on = 1;
>> +        } else {
>> +            on = n->allmulti;
>> +        }
>> +        r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_ALLMULTI, on);
>> +        if (r < 0) {
>> +            return r;
>> +        }
>
> Can we skip the sending of the CVQ commands if the state matches the
> default state?

Thanks for your reminder, I forgot this part when coding. I will
refactor the patch according to your suggestion and take care of it in
the following patches for this part.

Thanks!


>
> Thanks!
>
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int vhost_vdpa_net_load(NetClientState *nc)
>>   {
>>       VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
>> @@ -780,6 +850,10 @@ static int vhost_vdpa_net_load(NetClientState *nc)
>>       if (unlikely(r)) {
>>           return r;
>>       }
>> +    r = vhost_vdpa_net_load_rx(s, n);
>> +    if (unlikely(r)) {
>> +        return r;
>> +    }
>>
>>       return 0;
>>   }
>> --
>> 2.25.1
>>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-06-26  8:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-22  3:01 [PATCH RFC 0/3] Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support Hawkins Jiawei
2023-06-22  3:01 ` [PATCH RFC 1/3] vdpa: Restore MAC address filtering state Hawkins Jiawei
2023-06-25 10:37   ` Eugenio Perez Martin
2023-06-26  8:40     ` Hawkins Jiawei
2023-06-22  3:01 ` [PATCH RFC 2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature Hawkins Jiawei
2023-06-25 10:56   ` Eugenio Perez Martin
2023-06-26  8:52     ` Hawkins Jiawei
2023-06-22  3:01 ` [PATCH RFC 3/3] vdpa: Allow VIRTIO_NET_F_CTRL_RX in SVQ Hawkins Jiawei
2023-06-25 10:51   ` Eugenio Perez Martin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).