Re: [RFT PATCH 4/9] net: introduce and use netdev_features_t for device features sets
flat view
From: Ben Hutchings <hidden>
Date: 2011-06-20 21:01:39
On Mon, 2011-06-20 at 21:14 +0200, Michał Mirosław wrote:
The type and bits definitions are moved to separate header so that linux/skbuff.h won't need to include linux/netdevice.h.
[...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c index c914729..ca869c0 100644 --- a/drivers/net/sfc/efx.c +++ b/drivers/net/sfc/efx.c@@ -1883,7 +1883,7 @@ static void efx_set_multicast_list(struct net_device *net_dev) /* Otherwise efx_start_port() will do this */ } -static int efx_set_features(struct net_device *net_dev, u32 data) +static int efx_set_features(struct net_device *net_dev, netdev_features_t data) { struct efx_nic *efx = netdev_priv(net_dev);
The type of efx_nic_type::offload_features, defined in drivers/net/sfc/net_driver.h, will also need to be changed. [...]
quoted hunk ↗ jump to hunk
--- /dev/null +++ b/include/linux/netdev_features.h
[...]
+ /* Segmentation offload features */
[...]
+ /* Features valid for ethtool to change */ + /* = all defined minus driver/device-class-related */
[...]
+ /* List of features with software fallbacks. */
[...]
quoted hunk ↗ jump to hunk
+ /* + * If one device supports one of these features, then enable them + * for all in netdev_increment_features. + */
[...]
quoted hunk ↗ jump to hunk
+ /* + * If one device doesn't support one of these features, then disable it + * for all in netdev_increment_features. + */
[...]
+ /* changeable features with no special hardware requirements */
[...] A cosmetic detail: these comments should no longer be indented. [...]
quoted hunk ↗ jump to hunk
diff --git a/net/core/dev.c b/net/core/dev.c index 3041b6ae..a5a0e76 100644 --- a/net/core/dev.c +++ b/net/core/dev.c
[...]
quoted hunk ↗ jump to hunk
@@ -1872,8 +1872,8 @@ struct sk_buff *skb_gso_segment(struct sk_buff *skb, u32 features) if (dev && dev->ethtool_ops && dev->ethtool_ops->get_drvinfo) dev->ethtool_ops->get_drvinfo(dev, &info); - WARN(1, "%s: caps=(0x%lx, 0x%lx) len=%d data_len=%d ip_summed=%d\n", - info.driver, dev ? dev->features : 0L, + WARN(1, "%s: caps=(0x%llx, 0x%lx) len=%d data_len=%d ip_summed=%d\n", + info.driver, dev ? (long long)dev->features : 0LL,skb->sk ? skb->sk->sk_route_caps : 0L, skb->len, skb->data_len, skb->ip_summed);
The types of sock::sk_route_caps and sock::sk_route_nocaps also need to be fixed (they are still int and not even unsigned!). [...]
quoted hunk ↗ jump to hunk
@@ -5250,11 +5252,10 @@ u32 netdev_fix_features(struct net_device *dev, u32 features) return features; } -EXPORT_SYMBOL(netdev_fix_features);
I don't think that change belongs in this patch! [...]
quoted hunk ↗ jump to hunk
@@ -5270,8 +5271,10 @@ int __netdev_update_features(struct net_device *dev) if (dev->features == features) return 0; + BUILD_BUG_ON(sizeof(features) != sizeof(u32)); /* XXX: need: %Fx */ + netdev_dbg(dev, "Features changed: 0x%08x -> 0x%08x\n", - dev->features, features); + (u32)dev->features, (u32)features);
[...] You could either define macros for this: #define NETDEV_FEATURES_FMT "0x%08x" #define NETDEV_FEATURES_FMT_ARG(features) features or follow the current trend of extending %p and passing a pointer to the value. Ben. -- Ben Hutchings, Senior Software 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.