| Message ID | 4b9550c14bc8c98c8f48e04dbf3d3ac41489d3fd.1688743107.git.yin31149@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Vhost-vdpa Shadow Virtqueue _F_CTRL_RX commands support | expand |
On Fri, Jul 7, 2023 at 5:27 PM 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> > --- > v3: > - return early if mismatch the condition suggested by Eugenio > > v2: https://lore.kernel.org/all/2f2560f749186c0eb1055f9926f464587e419eeb.1688051252.git.yin31149@gmail.com/ > - use iovec suggested by Eugenio > - avoid sending CVQ command in default state > > v1: https://lore.kernel.org/all/00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.git.yin31149@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. Apart from that, Acked-by: Eugenio Pérez <eperezma@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 >
On 2023/8/17 18:18, Eugenio Perez Martin wrote: > On Fri, Jul 7, 2023 at 5:27 PM 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> >> --- >> v3: >> - return early if mismatch the condition suggested by Eugenio >> >> v2: https://lore.kernel.org/all/2f2560f749186c0eb1055f9926f464587e419eeb.1688051252.git.yin31149@gmail.com/ >> - use iovec suggested by Eugenio >> - avoid sending CVQ command in default state >> >> v1: https://lore.kernel.org/all/00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.git.yin31149@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 <eperezma@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 >> >
On Thu, Aug 17, 2023 at 2:47 PM Hawkins Jiawei <yin31149@gmail.com> wrote: > > On 2023/8/17 18:18, Eugenio Perez Martin wrote: > > On Fri, Jul 7, 2023 at 5:27 PM 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> > >> --- > >> v3: > >> - return early if mismatch the condition suggested by Eugenio > >> > >> v2: https://lore.kernel.org/all/2f2560f749186c0eb1055f9926f464587e419eeb.1688051252.git.yin31149@gmail.com/ > >> - use iovec suggested by Eugenio > >> - avoid sending CVQ command in default state > >> > >> v1: https://lore.kernel.org/all/00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.git.yin31149@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. > If the maintainer is ok with this, I'm totally ok with leaving the code as it is right now. Thanks! > Thanks! > > > > > > Apart from that, > > > > Acked-by: Eugenio Pérez <eperezma@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 > >> > > >
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, + 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; }
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> --- v3: - return early if mismatch the condition suggested by Eugenio v2: https://lore.kernel.org/all/2f2560f749186c0eb1055f9926f464587e419eeb.1688051252.git.yin31149@gmail.com/ - use iovec suggested by Eugenio - avoid sending CVQ command in default state v1: https://lore.kernel.org/all/00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.git.yin31149@gmail.com/ net/vhost-vdpa.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)