diff mbox series

[RFC,2/3] vdpa: Restore packet receive filtering state relative with _F_CTRL_RX feature

Message ID 86eeddcd6f6b04e5c1e44e901ddea3b1b8b6c183.1687402580.git.yin31149@gmail.com (mailing list archive)
State New, archived
Headers show
Series Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support | expand

Commit Message

Hawkins Jiawei June 22, 2023, 3:01 a.m. UTC
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(+)

Comments

Eugenio Perez Martin June 25, 2023, 10:56 a.m. UTC | #1
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
>
Hawkins Jiawei June 26, 2023, 8:52 a.m. UTC | #2
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
>>
>
diff mbox series

Patch

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