Re: [PATCH v5 5/9] net: Define enum for net device features.
From: Ben Hutchings <hidden>
Date: 2011-11-17 00:00:28
On Thu, 2011-11-17 at 00:34 +0100, Michał Mirosław wrote:
On Wed, Nov 16, 2011 at 10:39:37PM +0000, Ben Hutchings wrote:quoted
On Wed, 2011-11-16 at 02:29 +0100, Michał Mirosław wrote:quoted
Define feature values by bit position instead of direct 2**i values and force the values to be of type netdev_features_t. Cleaned and extended from patch by Mahesh Bandewar [off-list ref]: + added netdev_features_t casts + included bits under NETIF_F_GSO_MASK + moved feature #defines out of struct net_device definition Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- include/linux/netdev_features.h | 129 +++++++++++++++++++++++++++----------- 1 files changed, 91 insertions(+), 38 deletions(-)diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h index af52381..04ac8f8 100644 --- a/include/linux/netdev_features.h +++ b/include/linux/netdev_features.h[...]quoted
+ /**/NETIF_F_GSO_SHIFT, /* keep the order of SKB_GSO_* bits */ + NETIF_F_TSO_BIT /* ... TCPv4 segmentation */ + = NETIF_F_GSO_SHIFT, + NETIF_F_UFO_BIT, /* ... UDPv4 fragmentation */ + NETIF_F_GSO_ROBUST_BIT, /* ... ->SKB_GSO_DODGY */ + NETIF_F_TSO_ECN_BIT, /* ... TCP ECN support */ + NETIF_F_TSO6_BIT, /* ... TCPv6 segmentation */ + NETIF_F_FSO_BIT, /* ... FCoE segmentation */ + NETIF_F_GSO_RESERVED1, /* ... free (fill GSO_MASK to 8 bits) */ + /**/NETIF_F_GSO_LAST, /* [can't be last bit, see GSO_MASK] */ + NETIF_F_GSO_RESERVED2 /* ... free (fill GSO_MASK to 8 bits) */ + = NETIF_F_GSO_LAST, -/* Segmentation offload features */ -#define NETIF_F_GSO_SHIFT 16 -#define NETIF_F_GSO_MASK 0x00ff0000 -#define NETIF_F_TSO (SKB_GSO_TCPV4 << NETIF_F_GSO_SHIFT) -#define NETIF_F_UFO (SKB_GSO_UDP << NETIF_F_GSO_SHIFT) -#define NETIF_F_GSO_ROBUST (SKB_GSO_DODGY << NETIF_F_GSO_SHIFT) -#define NETIF_F_TSO_ECN (SKB_GSO_TCP_ECN << NETIF_F_GSO_SHIFT) -#define NETIF_F_TSO6 (SKB_GSO_TCPV6 << NETIF_F_GSO_SHIFT) -#define NETIF_F_FSO (SKB_GSO_FCOE << NETIF_F_GSO_SHIFT)[...] You should either add BUILD_BUG_ON()s somewhere to ensure that the netdev feature and skb GSO flags remain in sync, or redefine the skb GSO flags using the netdev feature flags (which I thought was the reason for moving features to their own header).Main motiviation for the moving out of features to separate headers was to avoid circular inclusion of linux/netdevice.h from linux/skbuff.h. We get readability for free. BUILD_BUG_ON() is a good idea. It should go to skbuff.h and as separate patch as it's something new and independent.
It may be functionally independent, but this change is unsafe without the addition of such assertions. Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.