Re: [PATCH v3 1/6] virtio-net: introduce RSS and hash report features

[Yuri Benditovich <yuri.benditovich@daynix.com>]

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

On Thu, Mar 12, 2020 at 10:23 AM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Thu, Mar 12, 2020 at 09:42:20AM +0200, Yuri Benditovich wrote:
> >
> >
> > On Thu, Mar 12, 2020 at 9:21 AM Michael S. Tsirkin <mst@redhat.com>
> wrote:
> >
> >     On Thu, Mar 12, 2020 at 09:02:38AM +0200, Yuri Benditovich wrote:
> >     >     >     > +#define virtio_net_config virtio_net_config_with_rss
> >     >     >
> >     >     >     Do we have to? Let's just tweak code to do the right
> thing...
> >     >     >
> >     >     >
> >     >     > Are we going to update the virtio_net some time?
> >     >     > If yes, IMO makes sense to do less tweaking in the middle of
> the
> >     code.
> >     >     > Then, upon update of virtio_net.h - easily remove all these
> defines
> >     that
> >     >     were
> >     >     > added in virtio-net.c
> >     >
> >     >     We'll update it in a month or two. But I'd be reluctant to
> merge
> >     hacks
> >     >     since people tend to copy-paste code ...
> >     >
> >     >
> >     > I agree that merging hacks is very bad practice.
> >     > Which change is more looks like a hack: redefine the struct to its
> _real_
> >     > layout or change the type of the struct in 5 places?
> >
> >     Anything that would be unacceptable as a permanent solution is a
> hack.
> >     In this case how about
> >             virtio_net_config_rss {
> >                     struct virtio_net_config config;
> >                     /* RSS things */
> >             }
> >
> >
> > No problem.
> >
> > '#define virtio_net_config virtio_net_config_with_rss ' is OK?
> >
>
> I don't think it is, macros are supposed to be all upper case.
>

Michael, just tell me please what you want:
You prefer to change everywhere ' virtio_net_config' to
'virtio_net_config_rss' and two month later to change it back?
You prefer to change everywhere  ' virtio_net_config'  to
'VIRTIO_NET_CONFIG' and define it according to the needs?
Something other?





>
> >
> >
> >
> >     --
> >     MST
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3217 bytes --]