Message ID | 1306363336-13614-1-git-send-email-maheshb@google.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
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
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
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?
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
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 ...
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
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
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
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
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 --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
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(-)