Messages in this thread | | | Date | Thu, 12 Mar 2020 10:19:43 +0100 | From | Michal Kubecek <> | Subject | Re: [PATCH net-next 03/15] ethtool: provide netdev features with FEATURES_GET request |
| |
On Wed, Mar 11, 2020 at 03:49:08PM -0700, Jakub Kicinski wrote: > On Wed, 11 Mar 2020 22:40:18 +0100 (CET) Michal Kubecek wrote: > > > > +#define ETHTOOL_DEV_FEATURE_WORDS ((NETDEV_FEATURE_COUNT + 31) / 32) > > nit: since this line is touched perhaps worth converting to > DIV_ROUND_UP()?
Yes.
> > + for (i = 0; i < ETHTOOL_DEV_FEATURE_WORDS; i++) > > + dest[i] = (u32)(src >> (32 * i)); > > nit: cast unnecessary
OK.
> > + all_features = ~(netdev_features_t)0 >> > > + (8 * sizeof(all_features) - NETDEV_FEATURE_COUNT); > > nit: GENMASK_ULL(NETDEV_FEATURE_COUNT % 32 - 1, 0) ?
Nice, I wasn't aware of this macro. But unless I want to fill data->all directly, it would be without the "% 32" part.
> > +static int features_fill_reply(struct sk_buff *skb, > > + const struct ethnl_req_info *req_base, > > + const struct ethnl_reply_data *reply_base) > > +{ > > + const struct features_reply_data *data = FEATURES_REPDATA(reply_base); > > + bool compact = req_base->flags & ETHTOOL_FLAG_COMPACT_BITSETS; > > + int ret; > > + > > + ret = ethnl_put_bitset32(skb, ETHTOOL_A_FEATURES_HW, data->hw, > > + data->all, NETDEV_FEATURE_COUNT, > > + netdev_features_strings, compact); > > + if (ret < 0) > > + return ret; > > + ret = ethnl_put_bitset32(skb, ETHTOOL_A_FEATURES_WANTED, data->wanted, > > + NULL, NETDEV_FEATURE_COUNT, > > + netdev_features_strings, compact); > > + if (ret < 0) > > + return ret; > > + ret = ethnl_put_bitset32(skb, ETHTOOL_A_FEATURES_ACTIVE, data->active, > > + NULL, NETDEV_FEATURE_COUNT, > > + netdev_features_strings, compact); > > + if (ret < 0) > > + return ret; > > + ret = ethnl_put_bitset32(skb, ETHTOOL_A_FEATURES_NOCHANGE, > > + data->nochange, NULL, NETDEV_FEATURE_COUNT, > > + netdev_features_strings, compact); > > + > > + return ret; > > nit: return directly?
Ah, right. I eliminated "if (ret < 0)" but did not check again.
Michal
| |