| Message ID | 00f72fe154a882fd6dc15bc39e3a1ac63f9dadce.1687402580.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 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 >> >
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;
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(-)