diff mbox

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

Message ID 1306363336-13614-1-git-send-email-maheshb@google.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

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.

Changes since v1:
 Split the patch into two pieces.

 include/linux/netdevice.h |   99 +++++++++++++++++++++++++++++++--------------
 1 files changed, 69 insertions(+), 30 deletions(-)

Comments

=?ISO-8859-2?Q?Micha=B3_Miros=B3aw?= May 26, 2011, 10:40 a.m. UTC | #1
2011/5/26 Mahesh Bandewar <maheshb@google.com>:
> Little bit cleanup by defining enum for all bits used. Also use those enum
> values to redefine flags.
[...]

This BIT2FLAG macro just obfuscates the code (and gives you more
characters to read).

I'd also prefer this patch included the GSO/TSO bits and they are
logically part of this cleanup. But if you think it's easier to review
split, then that's fine by me.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 4, 2011, 8:34 p.m. UTC | #2
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin June 6, 2011, 3:58 a.m. UTC | #3
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?
David Miller June 6, 2011, 5:15 a.m. UTC | #4
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 6 Jun 2011 06:58:08 +0300

> 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.

Since the GSO accessors deal with mutliple bits, you can create
special GSO specific interfaces to manipulate them.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin June 6, 2011, 3:32 p.m. UTC | #5
On Sun, Jun 05, 2011 at 10:15:37PM -0700, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Mon, 6 Jun 2011 06:58:08 +0300
> 
> > 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.
> 
> Since the GSO accessors deal with mutliple bits, you can create
> special GSO specific interfaces to manipulate them.

Yes but it's not just GSO.
It's anything that includes more than 1 feature.
Examples:
NETIF_F_ALL_CSUM
NETIF_F_ALL_TX_OFFLOADS
NETIF_F_V6_CSUM
NETIF_F_SOFT_FEATURES

etc

Creating many accessors for each will need a lot
of code duplication ...
=?ISO-8859-2?Q?Micha=B3_Miros=B3aw?= June 6, 2011, 3:48 p.m. UTC | #6
2011/6/4 David Miller <davem@davemloft.net>:
> 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.

I suggested that it's better to first introduce the enum because it
can be used right away (e.g. in ethtool.c feature name table).
Whatever the new access scheme will be, it will also use that enum.

Best Regards,
Michał Mirosław
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 6, 2011, 7:20 p.m. UTC | #7
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 6 Jun 2011 18:32:53 +0300

> On Sun, Jun 05, 2011 at 10:15:37PM -0700, David Miller wrote:
>> Since the GSO accessors deal with mutliple bits, you can create
>> special GSO specific interfaces to manipulate them.
> 
> Yes but it's not just GSO.
> It's anything that includes more than 1 feature.
> Examples:
> NETIF_F_ALL_CSUM
> NETIF_F_ALL_TX_OFFLOADS
> NETIF_F_V6_CSUM
> NETIF_F_SOFT_FEATURES
> 
> etc
> 
> Creating many accessors for each will need a lot
> of code duplication ...

Yet this is something you must resolve in order to change the feature
bit implementation.

Whether this issue is difficult or not to address, it has to be done
either way.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin June 6, 2011, 8:35 p.m. UTC | #8
On Mon, Jun 06, 2011 at 12:20:59PM -0700, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Mon, 6 Jun 2011 18:32:53 +0300
> 
> > On Sun, Jun 05, 2011 at 10:15:37PM -0700, David Miller wrote:
> >> Since the GSO accessors deal with mutliple bits, you can create
> >> special GSO specific interfaces to manipulate them.
> > 
> > Yes but it's not just GSO.
> > It's anything that includes more than 1 feature.
> > Examples:
> > NETIF_F_ALL_CSUM
> > NETIF_F_ALL_TX_OFFLOADS
> > NETIF_F_V6_CSUM
> > NETIF_F_SOFT_FEATURES
> > 
> > etc
> > 
> > Creating many accessors for each will need a lot
> > of code duplication ...
> 
> Yet this is something you must resolve in order to change the feature
> bit implementation.
> 
> Whether this issue is difficult or not to address, it has to be done
> either way.

I think I found a truly elegant solution to this
problem which this margin is too narrow to contain ...
On Mon, Jun 6, 2011 at 12:20 PM, David Miller <davem@davemloft.net> wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Mon, 6 Jun 2011 18:32:53 +0300
>
>> On Sun, Jun 05, 2011 at 10:15:37PM -0700, David Miller wrote:
>>> Since the GSO accessors deal with mutliple bits, you can create
>>> special GSO specific interfaces to manipulate them.
>>
>> Yes but it's not just GSO.
>> It's anything that includes more than 1 feature.
>> Examples:
>> NETIF_F_ALL_CSUM
>> NETIF_F_ALL_TX_OFFLOADS
>> NETIF_F_V6_CSUM
>> NETIF_F_SOFT_FEATURES
>>
>> etc
>>
>> Creating many accessors for each will need a lot
>> of code duplication ...
>
> Yet this is something you must resolve in order to change the feature
> bit implementation.
>
> Whether this issue is difficult or not to address, it has to be done
> either way.
>

I agree that the cleanup is not really necessary to the feature
extension as such but this along with the other patch that I have
posted is the beginning of that work. It's definitely not complete and
also not as simple as it sounds / feels because of these constants
defined which are "or-ed" flag values (listed above). I think it will
be nice to get this done in as little code as possible, but I think
that should be the constraint.

In these two patches I have created separate header file
"netdev_features.h" where everything related to "features" should
reside including all these accessor macros / functions.

--mahesh..
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin June 9, 2011, 4:46 p.m. UTC | #10
On Mon, Jun 06, 2011 at 11:35:15PM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 06, 2011 at 12:20:59PM -0700, David Miller wrote:
> > From: "Michael S. Tsirkin" <mst@redhat.com>
> > Date: Mon, 6 Jun 2011 18:32:53 +0300
> > 
> > > On Sun, Jun 05, 2011 at 10:15:37PM -0700, David Miller wrote:
> > >> Since the GSO accessors deal with mutliple bits, you can create
> > >> special GSO specific interfaces to manipulate them.
> > > 
> > > Yes but it's not just GSO.
> > > It's anything that includes more than 1 feature.
> > > Examples:
> > > NETIF_F_ALL_CSUM
> > > NETIF_F_ALL_TX_OFFLOADS
> > > NETIF_F_V6_CSUM
> > > NETIF_F_SOFT_FEATURES
> > > 
> > > etc
> > > 
> > > Creating many accessors for each will need a lot
> > > of code duplication ...
> > 
> > Yet this is something you must resolve in order to change the feature
> > bit implementation.
> > 
> > Whether this issue is difficult or not to address, it has to be done
> > either way.
> 
> I think I found a truly elegant solution to this
> problem which this margin is too narrow to contain ...

OK, it looks like using variadic macros from C99 makes this
possible, even though use of ungarded comma in macros
below makes me cringe:

/* Set all bits in the first 64 arguments, ignore the rest */
#define NETIF_F_OR_64( \
	_000, _001 , _002 , _003 , _004 , _005 , _006 , _007, \
	_010, _011 , _012 , _013 , _014 , _015 , _016 , _017, \
	_020, _021 , _022 , _023 , _024 , _025 , _026 , _027, \
	_030, _031 , _032 , _033 , _034 , _035 , _036 , _037, \
	_040, _041 , _042 , _043 , _044 , _045 , _046 , _047, \
	_050, _051 , _052 , _053 , _054 , _055 , _056 , _057, \
	_060, _061 , _062 , _063 , _064 , _065 , _066 , _067, \
	_070, _071 , _072 , _073 , _074 , _075 , _076 , _077, \
	... ) \
	((_000) | (_001)  | (_002)  | (_003)  | (_004)  | (_005)  | (_006)  | (_007) | \
	 (_010) | (_011)  | (_012)  | (_013)  | (_014)  | (_015)  | (_016)  | (_017) | \
	 (_020) | (_021)  | (_022)  | (_023)  | (_024)  | (_025)  | (_026)  | (_027) | \
	 (_030) | (_031)  | (_032)  | (_033)  | (_034)  | (_035)  | (_036)  | (_037) | \
	 (_040) | (_041)  | (_042)  | (_043)  | (_044)  | (_045)  | (_046)  | (_047) | \
	 (_050) | (_051)  | (_052)  | (_053)  | (_054)  | (_055)  | (_056)  | (_057) | \
	 (_060) | (_061)  | (_062)  | (_063)  | (_064)  | (_065)  | (_066)  | (_067) | \
	 (_070) | (_071)  | (_072)  | (_073)  | (_074)  | (_075)  | (_076)  | (_077) )

/* Verify that argument #65 is zero */
#define NETIF_F_BUG_ON_64( \
	_000, _001 , _002 , _003 , _004 , _005 , _006 , _007, \
	_010, _011 , _012 , _013 , _014 , _015 , _016 , _017, \
	_020, _021 , _022 , _023 , _024 , _025 , _026 , _027, \
	_030, _031 , _032 , _033 , _034 , _035 , _036 , _037, \
	_040, _041 , _042 , _043 , _044 , _045 , _046 , _047, \
	_050, _051 , _052 , _053 , _054 , _055 , _056 , _057, \
	_060, _061 , _062 , _063 , _064 , _065 , _066 , _067, \
	_070, _071 , _072 , _073 , _074 , _075 , _076 , _077, \
	_100, ... ) \
	BUG_ON((_100))

/* Set multiple bits in f. At most 64 bits can be
 * set in this way.
 * Nested calls are padded with 0 arguments
 * to ensure there are at least 64 of them */
#define NETIF_F_INIT(f, ...) do { \
	f |= NETIF_F_OR_64(__VA_ARGS__, \
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0,\
			0, 0, 0, 0, 0, 0, 0, 0 \
		       ); \
	NETIF_F_BUG_ON_64(__VA_ARGS__, \
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0,\
		      0, 0, 0, 0, 0, 0, 0, 0 \
		     ); \
} while (0)

And now:
#define NETIF_F_GSO_SOFTWARE	 NETIF_F_TSO , NETIF_F_TSO_ECN , \
				 NETIF_F_TSO6 , NETIF_F_UFO


which makes

	NETIF_F_INIT(z, NETIF_F_GSO_SOFTWARE);

work as expected, and set all necessary bits,
so all we need to do is replace
	z = NETIF_F_GSO_SOFTWARE;
with call to macro above.

At most 64 different bits can be passed in this way
but NETIF_F_BUG_ON_64 above checks that.
If we want more than 64 bits, we just update
these macro definitions.

It seems that behaviour above is guaranteed by the language spec,
specifically the argument prescan rule.
Any C99 experts want to comment on this?

I have my doubts about whether the above is way too clever
even if it works. What do others think?


> -- 
> MST
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael S. Tsirkin June 9, 2011, 8:12 p.m. UTC | #11
On Thu, Jun 09, 2011 at 07:46:45PM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 06, 2011 at 11:35:15PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jun 06, 2011 at 12:20:59PM -0700, David Miller wrote:
> > > From: "Michael S. Tsirkin" <mst@redhat.com>
> > > Date: Mon, 6 Jun 2011 18:32:53 +0300
> > > 
> > > > On Sun, Jun 05, 2011 at 10:15:37PM -0700, David Miller wrote:
> > > >> Since the GSO accessors deal with mutliple bits, you can create
> > > >> special GSO specific interfaces to manipulate them.
> > > > 
> > > > Yes but it's not just GSO.
> > > > It's anything that includes more than 1 feature.
> > > > Examples:
> > > > NETIF_F_ALL_CSUM
> > > > NETIF_F_ALL_TX_OFFLOADS
> > > > NETIF_F_V6_CSUM
> > > > NETIF_F_SOFT_FEATURES
> > > > 
> > > > etc
> > > > 
> > > > Creating many accessors for each will need a lot
> > > > of code duplication ...
> > > 
> > > Yet this is something you must resolve in order to change the feature
> > > bit implementation.
> > > 
> > > Whether this issue is difficult or not to address, it has to be done
> > > either way.
> > 
> > I think I found a truly elegant solution to this
> > problem which this margin is too narrow to contain ...
> 
> OK, it looks like using variadic macros from C99 makes this
> possible, even though use of ungarded comma in macros
> below makes me cringe:
> 
> /* Set all bits in the first 64 arguments, ignore the rest */
> #define NETIF_F_OR_64( \
> 	_000, _001 , _002 , _003 , _004 , _005 , _006 , _007, \
> 	_010, _011 , _012 , _013 , _014 , _015 , _016 , _017, \
> 	_020, _021 , _022 , _023 , _024 , _025 , _026 , _027, \
> 	_030, _031 , _032 , _033 , _034 , _035 , _036 , _037, \
> 	_040, _041 , _042 , _043 , _044 , _045 , _046 , _047, \
> 	_050, _051 , _052 , _053 , _054 , _055 , _056 , _057, \
> 	_060, _061 , _062 , _063 , _064 , _065 , _066 , _067, \
> 	_070, _071 , _072 , _073 , _074 , _075 , _076 , _077, \
> 	... ) \
> 	((_000) | (_001)  | (_002)  | (_003)  | (_004)  | (_005)  | (_006)  | (_007) | \
> 	 (_010) | (_011)  | (_012)  | (_013)  | (_014)  | (_015)  | (_016)  | (_017) | \
> 	 (_020) | (_021)  | (_022)  | (_023)  | (_024)  | (_025)  | (_026)  | (_027) | \
> 	 (_030) | (_031)  | (_032)  | (_033)  | (_034)  | (_035)  | (_036)  | (_037) | \
> 	 (_040) | (_041)  | (_042)  | (_043)  | (_044)  | (_045)  | (_046)  | (_047) | \
> 	 (_050) | (_051)  | (_052)  | (_053)  | (_054)  | (_055)  | (_056)  | (_057) | \
> 	 (_060) | (_061)  | (_062)  | (_063)  | (_064)  | (_065)  | (_066)  | (_067) | \
> 	 (_070) | (_071)  | (_072)  | (_073)  | (_074)  | (_075)  | (_076)  | (_077) )
> 
> /* Verify that argument #65 is zero */
> #define NETIF_F_BUG_ON_64( \
> 	_000, _001 , _002 , _003 , _004 , _005 , _006 , _007, \
> 	_010, _011 , _012 , _013 , _014 , _015 , _016 , _017, \
> 	_020, _021 , _022 , _023 , _024 , _025 , _026 , _027, \
> 	_030, _031 , _032 , _033 , _034 , _035 , _036 , _037, \
> 	_040, _041 , _042 , _043 , _044 , _045 , _046 , _047, \
> 	_050, _051 , _052 , _053 , _054 , _055 , _056 , _057, \
> 	_060, _061 , _062 , _063 , _064 , _065 , _066 , _067, \
> 	_070, _071 , _072 , _073 , _074 , _075 , _076 , _077, \
> 	_100, ... ) \
> 	BUG_ON((_100))
> 
> /* Set multiple bits in f. At most 64 bits can be
>  * set in this way.
>  * Nested calls are padded with 0 arguments
>  * to ensure there are at least 64 of them */
> #define NETIF_F_INIT(f, ...) do { \
> 	f |= NETIF_F_OR_64(__VA_ARGS__, \
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0,\
> 			0, 0, 0, 0, 0, 0, 0, 0 \
> 		       ); \
> 	NETIF_F_BUG_ON_64(__VA_ARGS__, \
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0,\
> 		      0, 0, 0, 0, 0, 0, 0, 0 \
> 		     ); \
> } while (0)


One thing I realized is there's no reason for
NETIF_F_INIT to get the ellipsis ... as we don't want users
to pass arbitrary lists of features, just the
predefined sets. So this can be a simpler:

 #define NETIF_F_INIT(f, bits) do { \
 	f |= NETIF_F_OR_64(bits, \
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0,\
 			0, 0, 0, 0, 0, 0, 0, 0 \
 		       ); 

> And now:
> #define NETIF_F_GSO_SOFTWARE	 NETIF_F_TSO , NETIF_F_TSO_ECN , \
> 				 NETIF_F_TSO6 , NETIF_F_UFO
> 
> 
> which makes
> 
> 	NETIF_F_INIT(z, NETIF_F_GSO_SOFTWARE);
> 
> work as expected, and set all necessary bits,
> so all we need to do is replace
> 	z = NETIF_F_GSO_SOFTWARE;
> with call to macro above.
> 
> At most 64 different bits can be passed in this way
> but NETIF_F_BUG_ON_64 above checks that.
> If we want more than 64 bits, we just update
> these macro definitions.
> 
> It seems that behaviour above is guaranteed by the language spec,
> specifically the argument prescan rule.
> Any C99 experts want to comment on this?

OK, the confirmation was located in C99 standard,
chapter 6.10.3.1 Argument substitution.

> I have my doubts about whether the above is way too clever
> even if it works. What do others think?
> 
> 
> > -- 
> > MST
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ca333e7..9bb5872 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -981,6 +981,49 @@  struct net_device_ops {
 };
 
 /*
+ * Net device feature bits; if you change something,
+ * also update netdev_features_strings[] in ethtool.c
+ */
+enum netdev_features {
+	NETIF_F_SG_BIT,			/* Scatter/gather IO. */
+	NETIF_F_IP_CSUM_BIT,		/* Can checksum TCP/UDP over IPv4. */
+	NETIF_F_NO_CSUM_BIT,		/* Does not require checksum. F.e. loopack. */
+	NETIF_F_HW_CSUM_BIT,		/* Can checksum all the packets. */
+	NETIF_F_IPV6_CSUM_BIT,		/* Can checksum TCP/UDP over IPV6 */
+	NETIF_F_HIGHDMA_BIT,		/* Can DMA to high memory. */
+	NETIF_F_FRAGLIST_BIT,		/* Scatter/gather IO. */
+	NETIF_F_HW_VLAN_TX_BIT,		/* Transmit VLAN hw acceleration */
+	NETIF_F_HW_VLAN_RX_BIT,		/* Receive VLAN hw acceleration */
+	NETIF_F_HW_VLAN_FILTER_BIT,	/* Receive filtering on VLAN */
+	NETIF_F_VLAN_CHALLENGED_BIT,	/* Device cannot handle VLAN packets */
+	NETIF_F_GSO_BIT,		/* Enable software GSO. */
+	NETIF_F_LLTX_BIT,		/* LockLess TX - deprecated. Please */
+					/* do not use LLTX in new drivers */
+	NETIF_F_NETNS_LOCAL_BIT,	/* Does not change network namespaces */
+	NETIF_F_GRO_BIT,		/* Generic receive offload */
+	NETIF_F_LRO_BIT,		/* large receive offload */
+	RESERVED16_BIT,			/* the GSO_MASK reserved bit 16 */
+	RESERVED17_BIT,			/* the GSO_MASK reserved bit 17 */
+	RESERVED18_BIT,			/* the GSO_MASK reserved bit 18 */
+	RESERVED19_BIT,			/* the GSO_MASK reserved bit 19 */
+	RESERVED20_BIT,			/* the GSO_MASK reserved bit 20 */
+	RESERVED21_BIT,			/* the GSO_MASK reserved bit 21 */
+	RESERVED22_BIT,			/* the GSO_MASK reserved bit 22 */
+	RESERVED23_BIT,			/* the GSO_MASK reserved bit 23 */
+	NETIF_F_FCOE_CRC_BIT,		/* FCoE CRC32 */
+	NETIF_F_SCTP_CSUM_BIT,		/* SCTP checksum offload */
+	NETIF_F_FCOE_MTU_BIT,		/* Supports max FCoE MTU, 2158 bytes*/
+	NETIF_F_NTUPLE_BIT,		/* N-tuple filters supported */
+	NETIF_F_RXHASH_BIT,		/* Receive hashing offload */
+	NETIF_F_RXCSUM_BIT,		/* Receive checksumming offload */
+	NETIF_F_NOCACHE_COPY_BIT,	/* Use no-cache copyfromuser */
+	NETIF_F_LOOPBACK_BIT,		/* Enable loopback */
+
+	/* Add you bit above this */
+	ND_FEATURE_NUM_BITS		/* (LAST VALUE) Total bits in use */
+};
+
+/*
  *	The DEVICE structure.
  *	Actually, this whole structure is a big mistake.  It mixes I/O
  *	data with strictly "high-level" data, and it has to know about
@@ -1035,36 +1078,32 @@  struct net_device {
 	/* mask of features inheritable by VLAN devices */
 	u32			vlan_features;
 
-	/* Net device feature bits; if you change something,
-	 * also update netdev_features_strings[] in ethtool.c */
-
-#define NETIF_F_SG		1	/* Scatter/gather IO. */
-#define NETIF_F_IP_CSUM		2	/* Can checksum TCP/UDP over IPv4. */
-#define NETIF_F_NO_CSUM		4	/* Does not require checksum. F.e. loopack. */
-#define NETIF_F_HW_CSUM		8	/* Can checksum all the packets. */
-#define NETIF_F_IPV6_CSUM	16	/* Can checksum TCP/UDP over IPV6 */
-#define NETIF_F_HIGHDMA		32	/* Can DMA to high memory. */
-#define NETIF_F_FRAGLIST	64	/* Scatter/gather IO. */
-#define NETIF_F_HW_VLAN_TX	128	/* Transmit VLAN hw acceleration */
-#define NETIF_F_HW_VLAN_RX	256	/* Receive VLAN hw acceleration */
-#define NETIF_F_HW_VLAN_FILTER	512	/* Receive filtering on VLAN */
-#define NETIF_F_VLAN_CHALLENGED	1024	/* Device cannot handle VLAN packets */
-#define NETIF_F_GSO		2048	/* Enable software GSO. */
-#define NETIF_F_LLTX		4096	/* LockLess TX - deprecated. Please */
-					/* do not use LLTX in new drivers */
-#define NETIF_F_NETNS_LOCAL	8192	/* Does not change network namespaces */
-#define NETIF_F_GRO		16384	/* Generic receive offload */
-#define NETIF_F_LRO		32768	/* large receive offload */
-
-/* the GSO_MASK reserves bits 16 through 23 */
-#define NETIF_F_FCOE_CRC	(1 << 24) /* FCoE CRC32 */
-#define NETIF_F_SCTP_CSUM	(1 << 25) /* SCTP checksum offload */
-#define NETIF_F_FCOE_MTU	(1 << 26) /* Supports max FCoE MTU, 2158 bytes*/
-#define NETIF_F_NTUPLE		(1 << 27) /* N-tuple filters supported */
-#define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
-#define NETIF_F_RXCSUM		(1 << 29) /* Receive checksumming offload */
-#define NETIF_F_NOCACHE_COPY	(1 << 30) /* Use no-cache copyfromuser */
-#define NETIF_F_LOOPBACK	(1 << 31) /* Enable loopback */
+#define BIT2FLAG(bit)		(1 << (bit))
+
+#define NETIF_F_SG		BIT2FLAG(NETIF_F_SG_BIT)
+#define NETIF_F_IP_CSUM		BIT2FLAG(NETIF_F_IP_CSUM_BIT)
+#define NETIF_F_NO_CSUM		BIT2FLAG(NETIF_F_NO_CSUM_BIT)
+#define NETIF_F_HW_CSUM		BIT2FLAG(NETIF_F_HW_CSUM_BIT)
+#define NETIF_F_IPV6_CSUM	BIT2FLAG(NETIF_F_IPV6_CSUM_BIT)
+#define NETIF_F_HIGHDMA		BIT2FLAG(NETIF_F_HIGHDMA_BIT)
+#define NETIF_F_FRAGLIST	BIT2FLAG(NETIF_F_FRAGLIST_BIT)
+#define NETIF_F_HW_VLAN_TX	BIT2FLAG(NETIF_F_HW_VLAN_TX_BIT)
+#define NETIF_F_HW_VLAN_RX	BIT2FLAG(NETIF_F_HW_VLAN_RX_BIT)
+#define NETIF_F_HW_VLAN_FILTER	BIT2FLAG(NETIF_F_HW_VLAN_FILTER_BIT)
+#define NETIF_F_VLAN_CHALLENGED	BIT2FLAG(NETIF_F_VLAN_CHALLENGED_BIT)
+#define NETIF_F_GSO		BIT2FLAG(NETIF_F_GSO_BIT)
+#define NETIF_F_LLTX		BIT2FLAG(NETIF_F_LLTX_BIT)
+#define NETIF_F_NETNS_LOCAL	BIT2FLAG(NETIF_F_NETNS_LOCAL_BIT)
+#define NETIF_F_GRO		BIT2FLAG(NETIF_F_GRO_BIT)
+#define NETIF_F_LRO		BIT2FLAG(NETIF_F_LRO_BIT)
+#define NETIF_F_FCOE_CRC	BIT2FLAG(NETIF_F_FCOE_CRC_BIT)
+#define NETIF_F_SCTP_CSUM	BIT2FLAG(NETIF_F_SCTP_CSUM_BIT)
+#define NETIF_F_FCOE_MTU	BIT2FLAG(NETIF_F_FCOE_MTU_BIT)
+#define NETIF_F_NTUPLE		BIT2FLAG(NETIF_F_NTUPLE_BIT)
+#define NETIF_F_RXHASH		BIT2FLAG(NETIF_F_RXHASH_BIT)
+#define NETIF_F_RXCSUM		BIT2FLAG(NETIF_F_RXCSUM_BIT)
+#define NETIF_F_NOCACHE_COPY	BIT2FLAG(NETIF_F_NOCACHE_COPY_BIT)
+#define NETIF_F_LOOPBACK	BIT2FLAG(NETIF_F_LOOPBACK_BIT)
 
 	/* Segmentation offload features */
 #define NETIF_F_GSO_SHIFT	16