Re: [PATCH v2 3/8] virtio-net: utilize VirtIOConfigSizeParams & virtio_get_config_size

[Daniil Tatianin]

On 9/2/22 8:54 PM, Raphael Norwitz wrote:
> On Fri, Aug 26, 2022 at 05:32:43PM +0300, Daniil Tatianin wrote:
> > Use the new common helper. As an added bonus this also makes use of
> > config size sanity checking via the 'max_size' field.
> >
> > Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> > ---
> >   hw/net/virtio-net.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index dd0d056fde..dfc8dd8562 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -106,6 +106,11 @@ static const VirtIOFeature feature_sizes[] = {
> >       {}
> >   };
> >   
> > +static const VirtIOConfigSizeParams cfg_size_params = {
>
> Can we have a zero length virtio-net config size? The both the v1.0 and
> v1.1 section 5.1.4 say “The mac address field always exists (though is
> only valid if VIRTIO_NET_F_MAC is set)” so we should probably set
> min_size to offset_of status?

It currently hardcodes VIRTIO_NET_F_MAC as always on, but I guess it
doesn't hurt to be more explicit about it. Will add that in the next
version.
(resending because forgot to reply-all last time)
> Otherwise LGTM.
>
> > +    .max_size = sizeof(struct virtio_net_config),
> > +    .feature_sizes = feature_sizes
> > +};
> > +
> >   static VirtIONetQueue *virtio_net_get_subqueue(NetClientState *nc)
> >   {
> >       VirtIONet *n = qemu_get_nic_opaque(nc);
> > @@ -3246,8 +3251,7 @@ static void virtio_net_set_config_size(VirtIONet *n, 
> > uint64_t host_features)
> >   {
> >       virtio_add_feature(&host_features, VIRTIO_NET_F_MAC);
> >   
> > -    n->config_size = virtio_feature_get_config_size(feature_sizes,
> > -                                                    host_features);
> > +    n->config_size = virtio_get_config_size(&cfg_size_params, host_features);
> >   }
> >   
> >   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
> > --
> > 2.25.1