Re: [PATCHv3] net: Define enum for the bits used in features.

["Michael S. Tsirkin" <mst@redhat.com>]

On Sat, Jun 04, 2011 at 01:34:38PM -0700, David Miller wrote:
> From: Mahesh Bandewar <maheshb@google.com>
> Date: Wed, 25 May 2011 15:42:16 -0700
> 
> > Little bit cleanup by defining enum for all bits used. Also use those enum
> > values to redefine flags.
> > 
> > Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> > ---
> > Changes since v2:
> >  (1) Removed the include which was part of the other patch (split mishap).
> >  (2) Changed the enums to add NETIF_F_ prefix.
> 
> I hate to be a pain after you've put so much work into these patches,
> but I simply don't like this approach.
> 
> I think the abstracted interfaces should come first.  You don't need to
> change any of the NETIF_F_* defines in order to do that.  You should only
> need to add the netdev_{set,clear,test}_*() macros.
> 
> If you want you can make the "bit" argument be the flag name after the
> NETIF_F_ prefix, so "netdev_test_active_feature(dev, SG)"
> 
> Then you convert every single access.
> 
> Then you make the flags type opaque, which should at that point be a
> 6 line change at best.
> 
> And then you can implement the flags however you want.


I've been thinking about this as well. It turns out most
things above can be done with the spatch (aka coccinelle) tool.
But I think the largest problem is what to do with multiple-feature
macros such as NETIF_F_GSO_SOFTWARE.

If we keep them an 'or' of bits, we more or less commit to
an implementation that can represent them all in a single
constant.

I played with variadic macros but could not come up with something
that does not generate a lot of code.

Ideas?

-- 
MST