On Fri, Jul 7, 2023 at 5:27 PM Hawkins Jiawei <yin31...@gmail.com> wrote:
>
> According to VirtIO standard, "The driver MUST follow
> the VIRTIO_NET_CTRL_MAC_TABLE_SET command by a le32 number,
> followed by that number of non-multicast MAC addresses,
> followed by another le32 number, followed by that number
> of multicast addresses."
>
> Considering that these data is not stored in contiguous memory,
> this patch refactors vhost_vdpa_net_load_cmd() to accept
> scattered data, eliminating the need for an addtional data copy or
> packing the data into s->cvq_cmd_out_buffer outside of
> vhost_vdpa_net_load_cmd().
>
> Signed-off-by: Hawkins Jiawei <yin31...@gmail.com>
> ---
> v3:
> - rename argument name to `data_sg` and `data_num`
> - use iov_to_buf() suggested by Eugenio
>
> v2:
> https://lore.kernel.org/all/6d3dc0fc076564a03501e222ef1102a6a7a643af.1688051252.git.yin31...@gmail.com/
> - refactor vhost_vdpa_load_cmd() to accept iovec suggested by
> Eugenio
>
> net/vhost-vdpa.c | 33 +++++++++++++++++++++++++--------
> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 373609216f..31ef6ad6ec 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -620,29 +620,38 @@ static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState
> *s, size_t out_len,
> }
>
> static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s, uint8_t class,
> - uint8_t cmd, const void *data,
> - size_t data_size)
> + uint8_t cmd, const struct iovec
> *data_sg,
> + size_t data_num)
> {
> const struct virtio_net_ctrl_hdr ctrl = {
> .class = class,
> .cmd = cmd,
> };
> + size_t data_size = iov_size(data_sg, data_num);
>
> assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl));
>
> + /* pack the CVQ command header */
> memcpy(s->cvq_cmd_out_buffer, &ctrl, sizeof(ctrl));
> - memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size);
>
> - return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size,
> + /* pack the CVQ command command-specific-data */
> + iov_to_buf(data_sg, data_num, 0,
> + s->cvq_cmd_out_buffer + sizeof(ctrl), data_size);
> +
> + return vhost_vdpa_net_cvq_add(s, data_size + sizeof(ctrl),
Nit, any reason for changing the order of the addends? sizeof(ctrl) +
data_size ?
> sizeof(virtio_net_ctrl_ack));
> }
>
> static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
> {
> if (virtio_vdev_has_feature(&n->parent_obj, VIRTIO_NET_F_CTRL_MAC_ADDR))
> {
> + const struct iovec data = {
> + .iov_base = (void *)n->mac,
Assign to void should always be valid, no need for casting here.
> + .iov_len = sizeof(n->mac),
> + };
> ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_MAC,
>
> VIRTIO_NET_CTRL_MAC_ADDR_SET,
> - n->mac, sizeof(n->mac));
> + &data, 1);
> if (unlikely(dev_written < 0)) {
> return dev_written;
> }
> @@ -665,9 +674,13 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
> }
>
> mq.virtqueue_pairs = cpu_to_le16(n->curr_queue_pairs);
> + const struct iovec data = {
> + .iov_base = &mq,
> + .iov_len = sizeof(mq),
> + };
> dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_MQ,
> - VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> &mq,
> - sizeof(mq));
> + VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET,
> + &data, 1);
> if (unlikely(dev_written < 0)) {
> return dev_written;
> }
> @@ -706,9 +719,13 @@ static int vhost_vdpa_net_load_offloads(VhostVDPAState
> *s,
> }
>
> offloads = cpu_to_le64(n->curr_guest_offloads);
> + const struct iovec data = {
> + .iov_base = &offloads,
> + .iov_len = sizeof(offloads),
> + };
> dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
> VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET,
> - &offloads, sizeof(offloads));
> + &data, 1);
> if (unlikely(dev_written < 0)) {
> return dev_written;
> }
Apart from that:
Acked-by: Eugenio Pérez <epere...@redhat.com>
Thanks!
