| Message ID | 6d3dc0fc076564a03501e222ef1102a6a7a643af.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:25 PM Hawkins Jiawei <yin31149@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 <yin31149@gmail.com> > --- > v2: > - refactor vhost_vdpa_load_cmd() to accept iovec suggested by > Eugenio > > net/vhost-vdpa.c | 42 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 32 insertions(+), 10 deletions(-) > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c > index 6f6a5c6df6..0bd1c7817c 100644 > --- a/net/vhost-vdpa.c > +++ b/net/vhost-vdpa.c > @@ -620,29 +620,43 @@ 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, > + size_t data_len) > { > const struct virtio_net_ctrl_hdr ctrl = { > .class = class, > .cmd = cmd, > }; > + void *cursor = s->cvq_cmd_out_buffer; > > - assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); > + /* pack the CVQ command header */ > + assert(sizeof(ctrl) < vhost_vdpa_net_cvq_cmd_page_len() - > + (cursor - s->cvq_cmd_out_buffer)); > + memcpy(cursor, &ctrl, sizeof(ctrl)); > + cursor += sizeof(ctrl); > > - memcpy(s->cvq_cmd_out_buffer, &ctrl, sizeof(ctrl)); > - memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size); > + /* pack the CVQ command command-specific-data */ > + for (int i = 0; i < data_len; ++i) { > + assert(data[i].iov_len < vhost_vdpa_net_cvq_cmd_page_len() - > + (cursor - s->cvq_cmd_out_buffer)); > + memcpy(cursor, data[i].iov_base, data[i].iov_len); > + cursor += data[i].iov_len; > + } Can we replace all of the above by iov_to_buf? > > - return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size, > + return vhost_vdpa_net_cvq_add(s, cursor - s->cvq_cmd_out_buffer, > 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, > + .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 +679,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 +724,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; > } > -- > 2.25.1 >
On 2023/7/4 22:17, Eugenio Perez Martin wrote: > On Thu, Jun 29, 2023 at 5:25 PM Hawkins Jiawei <yin31149@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 <yin31149@gmail.com> >> --- >> v2: >> - refactor vhost_vdpa_load_cmd() to accept iovec suggested by >> Eugenio >> >> net/vhost-vdpa.c | 42 ++++++++++++++++++++++++++++++++---------- >> 1 file changed, 32 insertions(+), 10 deletions(-) >> >> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c >> index 6f6a5c6df6..0bd1c7817c 100644 >> --- a/net/vhost-vdpa.c >> +++ b/net/vhost-vdpa.c >> @@ -620,29 +620,43 @@ 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, >> + size_t data_len) >> { >> const struct virtio_net_ctrl_hdr ctrl = { >> .class = class, >> .cmd = cmd, >> }; >> + void *cursor = s->cvq_cmd_out_buffer; >> >> - assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); >> + /* pack the CVQ command header */ >> + assert(sizeof(ctrl) < vhost_vdpa_net_cvq_cmd_page_len() - >> + (cursor - s->cvq_cmd_out_buffer)); >> + memcpy(cursor, &ctrl, sizeof(ctrl)); >> + cursor += sizeof(ctrl); >> >> - memcpy(s->cvq_cmd_out_buffer, &ctrl, sizeof(ctrl)); >> - memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size); >> + /* pack the CVQ command command-specific-data */ >> + for (int i = 0; i < data_len; ++i) { >> + assert(data[i].iov_len < vhost_vdpa_net_cvq_cmd_page_len() - >> + (cursor - s->cvq_cmd_out_buffer)); >> + memcpy(cursor, data[i].iov_base, data[i].iov_len); >> + cursor += data[i].iov_len; >> + } > > Can we replace all of the above by iov_to_buf? Yes, you are right. We should use iov_to_buf() here. I will refactor this patch according to your suggestion. Thanks! > >> >> - return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size, >> + return vhost_vdpa_net_cvq_add(s, cursor - s->cvq_cmd_out_buffer, >> 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, >> + .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 +679,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 +724,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; >> } >> -- >> 2.25.1 >> >
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c index 6f6a5c6df6..0bd1c7817c 100644 --- a/net/vhost-vdpa.c +++ b/net/vhost-vdpa.c @@ -620,29 +620,43 @@ 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, + size_t data_len) { const struct virtio_net_ctrl_hdr ctrl = { .class = class, .cmd = cmd, }; + void *cursor = s->cvq_cmd_out_buffer; - assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl)); + /* pack the CVQ command header */ + assert(sizeof(ctrl) < vhost_vdpa_net_cvq_cmd_page_len() - + (cursor - s->cvq_cmd_out_buffer)); + memcpy(cursor, &ctrl, sizeof(ctrl)); + cursor += sizeof(ctrl); - memcpy(s->cvq_cmd_out_buffer, &ctrl, sizeof(ctrl)); - memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size); + /* pack the CVQ command command-specific-data */ + for (int i = 0; i < data_len; ++i) { + assert(data[i].iov_len < vhost_vdpa_net_cvq_cmd_page_len() - + (cursor - s->cvq_cmd_out_buffer)); + memcpy(cursor, data[i].iov_base, data[i].iov_len); + cursor += data[i].iov_len; + } - return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size, + return vhost_vdpa_net_cvq_add(s, cursor - s->cvq_cmd_out_buffer, 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, + .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 +679,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 +724,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; }
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 <yin31149@gmail.com> --- v2: - refactor vhost_vdpa_load_cmd() to accept iovec suggested by Eugenio net/vhost-vdpa.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-)