| Message ID | d9d7641ef25d7a4477f8fc4df8cba026380dab76.1688051252.git.yin31149@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support | expand |
On Thu, Jun 29, 2023 at 5:26 PM 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> > --- > v2: > - avoid sending CVQ command in default state suggested by Eugenio > > v1: https://lore.kernel.org/all/86eeddcd6f6b04e5c1e44e901ddea3b1b8b6c183.1687402580.git.yin31149@gmail.com/ > > net/vhost-vdpa.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 104 insertions(+) > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > index cb45c84c88..9d5d88756c 100644 > --- a/net/vhost-vdpa.c > +++ b/net/vhost-vdpa.c > @@ -792,6 +792,106 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, > return 0; > } > > +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, > + uint8_t cmd, > + uint8_t on) > +{ > + ssize_t dev_written; > + const struct iovec data = { > + .iov_base = &on, > + .iov_len = sizeof(on), > + }; > + dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX, > + cmd, &data, 1); > + 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)) { Also suggesting early returns here. > + /* 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; > + } I think we can remove the "on" variable and just do: /* * According to ... */ if (n->mac_table.uni_overflow || n->promisc) { r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on); if (r < 0) { return r; } --- And the equivalent for multicast. Would that make sense? Thanks! > + if (on != 1) { > + /* > + * According to virtio_net_reset(), device turns promiscuous mode on > + * by default. > + * > + * Therefore, there is no need to send this CVQ command if the > + * driver also sets promiscuous mode on, which aligns with > + * the device's defaults. > + * > + * Note that the device's defaults can mismatch the driver's > + * configuration only at live migration. > + */ > + 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; > + } > + if (on != 0) { > + /* > + * According to virtio_net_reset(), device turns all-multicast mode > + * off by default. > + * > + * Therefore, there is no need to send this CVQ command if the > + * driver also sets all-multicast mode off, which aligns with > + * the device's defaults. > + * > + * Note that the device's defaults can mismatch the driver's > + * configuration only at live migration. > + */ > + 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); > @@ -818,6 +918,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 >
On 2023/7/4 23:39, Eugenio Perez Martin wrote: > On Thu, Jun 29, 2023 at 5:26 PM 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> >> --- >> v2: >> - avoid sending CVQ command in default state suggested by Eugenio >> >> v1: https://lore.kernel.org/all/86eeddcd6f6b04e5c1e44e901ddea3b1b8b6c183.1687402580.git.yin31149@gmail.com/ >> >> net/vhost-vdpa.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 104 insertions(+) >> >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c >> index cb45c84c88..9d5d88756c 100644 >> --- a/net/vhost-vdpa.c >> +++ b/net/vhost-vdpa.c >> @@ -792,6 +792,106 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, >> return 0; >> } >> >> +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, >> + uint8_t cmd, >> + uint8_t on) >> +{ >> + ssize_t dev_written; >> + const struct iovec data = { >> + .iov_base = &on, >> + .iov_len = sizeof(on), >> + }; >> + dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX, >> + cmd, &data, 1); >> + 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)) { > > Also suggesting early returns here. So, for CVQ commands related to VIRTIO_NET_F_CTRL_EXTRA_RX, would it be more appropriate to create a new function, maybe vhost_vdpa_net_load_rx_extra, to handle them instead of sending those CVQ commands within this function, if we choose to return early? > >> + /* 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; >> + } > > I think we can remove the "on" variable and just do: > > /* > * According to ... > */ > if (n->mac_table.uni_overflow || n->promisc) { > r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on); > if (r < 0) { > return r; > } > --- > > And the equivalent for multicast. > > Would that make sense? Yes, I will refactor these according to your suggestion. Thanks! > > Thanks! > >> + if (on != 1) { >> + /* >> + * According to virtio_net_reset(), device turns promiscuous mode on >> + * by default. >> + * >> + * Therefore, there is no need to send this CVQ command if the >> + * driver also sets promiscuous mode on, which aligns with >> + * the device's defaults. >> + * >> + * Note that the device's defaults can mismatch the driver's >> + * configuration only at live migration. >> + */ >> + 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; >> + } >> + if (on != 0) { >> + /* >> + * According to virtio_net_reset(), device turns all-multicast mode >> + * off by default. >> + * >> + * Therefore, there is no need to send this CVQ command if the >> + * driver also sets all-multicast mode off, which aligns with >> + * the device's defaults. >> + * >> + * Note that the device's defaults can mismatch the driver's >> + * configuration only at live migration. >> + */ >> + 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); >> @@ -818,6 +918,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 >> >
On Wed, Jul 5, 2023 at 4:09 AM Hawkins Jiawei <yin31149@gmail.com> wrote: > > On 2023/7/4 23:39, Eugenio Perez Martin wrote: > > On Thu, Jun 29, 2023 at 5:26 PM 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> > >> --- > >> v2: > >> - avoid sending CVQ command in default state suggested by Eugenio > >> > >> v1: https://lore.kernel.org/all/86eeddcd6f6b04e5c1e44e901ddea3b1b8b6c183.1687402580.git.yin31149@gmail.com/ > >> > >> net/vhost-vdpa.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 104 insertions(+) > >> > >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > >> index cb45c84c88..9d5d88756c 100644 > >> --- a/net/vhost-vdpa.c > >> +++ b/net/vhost-vdpa.c > >> @@ -792,6 +792,106 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, > >> return 0; > >> } > >> > >> +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, > >> + uint8_t cmd, > >> + uint8_t on) > >> +{ > >> + ssize_t dev_written; > >> + const struct iovec data = { > >> + .iov_base = &on, > >> + .iov_len = sizeof(on), > >> + }; > >> + dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX, > >> + cmd, &data, 1); > >> + 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)) { > > > > Also suggesting early returns here. > > So, for CVQ commands related to VIRTIO_NET_F_CTRL_EXTRA_RX, would it be > more appropriate to create a new function, maybe > vhost_vdpa_net_load_rx_extra, to handle them instead of sending those > CVQ commands within this function, if we choose to return early? > My understanding is that VIRTIO_NET_F_CTRL_RX_EXTRA depends on VIRTIO_NET_F_CTRL_RX, so we can do: if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) { return; } // Process CTRL_RX commands if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) { return; } // process CTRL_RX_EXTRA commands > > > >> + /* 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; > >> + } > > > > I think we can remove the "on" variable and just do: > > > > /* > > * According to ... > > */ > > if (n->mac_table.uni_overflow || n->promisc) { > > r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on); > > if (r < 0) { > > return r; > > } > > --- > > > > And the equivalent for multicast. > > > > Would that make sense? > > Yes, I will refactor these according to your suggestion. > > Thanks! > > > > > > Thanks! > > > >> + if (on != 1) { > >> + /* > >> + * According to virtio_net_reset(), device turns promiscuous mode on > >> + * by default. > >> + * > >> + * Therefore, there is no need to send this CVQ command if the > >> + * driver also sets promiscuous mode on, which aligns with > >> + * the device's defaults. > >> + * > >> + * Note that the device's defaults can mismatch the driver's > >> + * configuration only at live migration. > >> + */ > >> + 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; > >> + } > >> + if (on != 0) { > >> + /* > >> + * According to virtio_net_reset(), device turns all-multicast mode > >> + * off by default. > >> + * > >> + * Therefore, there is no need to send this CVQ command if the > >> + * driver also sets all-multicast mode off, which aligns with > >> + * the device's defaults. > >> + * > >> + * Note that the device's defaults can mismatch the driver's > >> + * configuration only at live migration. > >> + */ > >> + 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); > >> @@ -818,6 +918,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 > >> > > >
On 2023/7/5 14:40, Eugenio Perez Martin wrote: > On Wed, Jul 5, 2023 at 4:09 AM Hawkins Jiawei <yin31149@gmail.com> wrote: >> >> On 2023/7/4 23:39, Eugenio Perez Martin wrote: >>> On Thu, Jun 29, 2023 at 5:26 PM 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> >>>> --- >>>> v2: >>>> - avoid sending CVQ command in default state suggested by Eugenio >>>> >>>> v1: https://lore.kernel.org/all/86eeddcd6f6b04e5c1e44e901ddea3b1b8b6c183.1687402580.git.yin31149@gmail.com/ >>>> >>>> net/vhost-vdpa.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 104 insertions(+) >>>> >>>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c >>>> index cb45c84c88..9d5d88756c 100644 >>>> --- a/net/vhost-vdpa.c >>>> +++ b/net/vhost-vdpa.c >>>> @@ -792,6 +792,106 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, >>>> return 0; >>>> } >>>> >>>> +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, >>>> + uint8_t cmd, >>>> + uint8_t on) >>>> +{ >>>> + ssize_t dev_written; >>>> + const struct iovec data = { >>>> + .iov_base = &on, >>>> + .iov_len = sizeof(on), >>>> + }; >>>> + dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX, >>>> + cmd, &data, 1); >>>> + 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)) { >>> >>> Also suggesting early returns here. >> >> So, for CVQ commands related to VIRTIO_NET_F_CTRL_EXTRA_RX, would it be >> more appropriate to create a new function, maybe >> vhost_vdpa_net_load_rx_extra, to handle them instead of sending those >> CVQ commands within this function, if we choose to return early? >> > > My understanding is that VIRTIO_NET_F_CTRL_RX_EXTRA depends on > VIRTIO_NET_F_CTRL_RX, so we can do: > > if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) { > return; > } > > // Process CTRL_RX commands > > if (!virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_RX)) { > return; > } > > // process CTRL_RX_EXTRA commands Thanks for your explanation, it makes sense. I will send the v3 patch based on your suggestion. Thanks! > >>> >>>> + /* 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; >>>> + } >>> >>> I think we can remove the "on" variable and just do: >>> >>> /* >>> * According to ... >>> */ >>> if (n->mac_table.uni_overflow || n->promisc) { >>> r = vhost_vdpa_net_load_rx_mode(s, VIRTIO_NET_CTRL_RX_PROMISC, on); >>> if (r < 0) { >>> return r; >>> } >>> --- >>> >>> And the equivalent for multicast. >>> >>> Would that make sense? >> >> Yes, I will refactor these according to your suggestion. >> >> Thanks! >> >> >>> >>> Thanks! >>> >>>> + if (on != 1) { >>>> + /* >>>> + * According to virtio_net_reset(), device turns promiscuous mode on >>>> + * by default. >>>> + * >>>> + * Therefore, there is no need to send this CVQ command if the >>>> + * driver also sets promiscuous mode on, which aligns with >>>> + * the device's defaults. >>>> + * >>>> + * Note that the device's defaults can mismatch the driver's >>>> + * configuration only at live migration. >>>> + */ >>>> + 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; >>>> + } >>>> + if (on != 0) { >>>> + /* >>>> + * According to virtio_net_reset(), device turns all-multicast mode >>>> + * off by default. >>>> + * >>>> + * Therefore, there is no need to send this CVQ command if the >>>> + * driver also sets all-multicast mode off, which aligns with >>>> + * the device's defaults. >>>> + * >>>> + * Note that the device's defaults can mismatch the driver's >>>> + * configuration only at live migration. >>>> + */ >>>> + 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); >>>> @@ -818,6 +918,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 --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index cb45c84c88..9d5d88756c 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -792,6 +792,106 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState *s, return 0; } +static int vhost_vdpa_net_load_rx_mode(VhostVDPAState *s, + uint8_t cmd, + uint8_t on) +{ + ssize_t dev_written; + const struct iovec data = { + .iov_base = &on, + .iov_len = sizeof(on), + }; + dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_RX, + cmd, &data, 1); + 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; + } + if (on != 1) { + /* + * According to virtio_net_reset(), device turns promiscuous mode on + * by default. + * + * Therefore, there is no need to send this CVQ command if the + * driver also sets promiscuous mode on, which aligns with + * the device's defaults. + * + * Note that the device's defaults can mismatch the driver's + * configuration only at live migration. + */ + 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; + } + if (on != 0) { + /* + * According to virtio_net_reset(), device turns all-multicast mode + * off by default. + * + * Therefore, there is no need to send this CVQ command if the + * driver also sets all-multicast mode off, which aligns with + * the device's defaults. + * + * Note that the device's defaults can mismatch the driver's + * configuration only at live migration. + */ + 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); @@ -818,6 +918,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; }
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> --- v2: - avoid sending CVQ command in default state suggested by Eugenio v1: https://lore.kernel.org/all/86eeddcd6f6b04e5c1e44e901ddea3b1b8b6c183.1687402580.git.yin31149@gmail.com/ net/vhost-vdpa.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+)