qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Yuri Benditovich
Subject: Re: [PATCH v3 1/6] virtio-net: introduce RSS and hash report features
Date: Thu, 12 Mar 2020 11:08:20 +0200



On Thu, Mar 12, 2020 at 10:23 AM Michael S. Tsirkin <address@hidden> 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 <address@hidden> 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
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]