On Thu, Aug 25, 2022 at 2:36 AM Eugenio Pérez <epere...@redhat.com> wrote:
>
> Since there may be many commands we need to issue to load the NIC
> state, let's split them in individual functions
>
> Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> --
> v2: Add vhost_vdpa_net_load_cmd helper
> ---
> net/vhost-vdpa.c | 54 ++++++++++++++++++++++++++++++++----------------
> 1 file changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 468e460ac2..c89e2262d9 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -365,35 +365,31 @@ static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState
> *s, size_t out_len,
> return vhost_svq_poll(svq);
> }
>
> -static int vhost_vdpa_net_load(NetClientState *nc)
> +static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
> + const struct virtio_net_ctrl_hdr
> *ctrl,
> + const void *data, size_t data_size)
> {
> - VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> - const struct vhost_vdpa *v = &s->vhost_vdpa;
> - const VirtIONet *n;
> - uint64_t features;
> + assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(*ctrl));
>
> - assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> + memcpy(s->cvq_cmd_out_buffer, ctrl, sizeof(*ctrl));
> + memcpy(s->cvq_cmd_out_buffer + sizeof(ctrl), data, data_size);
>
> - if (!v->shadow_vqs_enabled) {
> - return 0;
> - }
> + return vhost_vdpa_net_cvq_add(s, sizeof(ctrl) + data_size,
> + sizeof(virtio_net_ctrl_ack));
> +}
>
> - n = VIRTIO_NET(v->dev->vdev);
> - features = n->parent_obj.guest_features;
> +static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
> +{
> + uint64_t features = n->parent_obj.guest_features;
> if (features & BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> const struct virtio_net_ctrl_hdr ctrl = {
> .class = VIRTIO_NET_CTRL_MAC,
> .cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET,
> };
> - char *cursor = s->cvq_cmd_out_buffer;
> ssize_t dev_written;
>
> - memcpy(cursor, &ctrl, sizeof(ctrl));
> - cursor += sizeof(ctrl);
> - memcpy(cursor, n->mac, sizeof(n->mac));
> -
> - dev_written = vhost_vdpa_net_cvq_add(s, sizeof(ctrl) +
> sizeof(n->mac),
> - sizeof(virtio_net_ctrl_ack));
> + dev_written = vhost_vdpa_net_load_cmd(s, &ctrl, n->mac,
> + sizeof(n->mac));
Patch looks good, if there's another respin, I'd accept class/cmd as
parameter then there's no need for the caller to prepare
virtio_net_ctrl_hdr.
Thanks
> if (unlikely(dev_written < 0)) {
> return dev_written;
> }
> @@ -404,6 +400,28 @@ static int vhost_vdpa_net_load(NetClientState *nc)
> return 0;
> }
>
> +static int vhost_vdpa_net_load(NetClientState *nc)
> +{
> + VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> + struct vhost_vdpa *v = &s->vhost_vdpa;
> + const VirtIONet *n;
> + int r;
> +
> + assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> +
> + if (!v->shadow_vqs_enabled) {
> + return 0;
> + }
> +
> + n = VIRTIO_NET(v->dev->vdev);
> + r = vhost_vdpa_net_load_mac(s, n);
> + if (unlikely(r < 0)) {
> + return r;
> + }
> +
> + return 0;
> +}
> +
> static NetClientInfo net_vhost_vdpa_cvq_info = {
> .type = NET_CLIENT_DRIVER_VHOST_VDPA,
> .size = sizeof(VhostVDPAState),
> --
> 2.31.1
>
