Re: [PATCH net-next V2] net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)

[Ben Hutchings]

On Thu, 2016-07-07 at 08:58 +0300, Netanel Belgazal wrote:
[...]
> +int ena_get_sset_count(struct net_device *netdev, int sset)
> +{
> +ÂÂÂÂÂÂÂif (sset != ETH_SS_STATS)
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -EOPNOTSUPP;
> +
> +ÂÂÂÂÂÂÂreturn netdev->num_tx_queues *
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ(ENA_STATS_ARRAY_TX + ENA_STATS_ARRAY_RX) +
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂENA_STATS_ARRAY_GLOBAL + ENA_STATS_ARRAY_ENA_COM;

num_tx_queues is the number of software queues originally allocated,
which may be larger than the number in use (real_num_tx_queues).

And when actually generating the strings:

[...]
> +static void ena_queue_strings(struct ena_adapter *adapter, u8 **data)
> +{
> +ÂÂÂÂÂÂÂconst struct ena_stats *ena_stats;
> +ÂÂÂÂÂÂÂint i, j;
> +
> +ÂÂÂÂÂÂÂfor (i = 0; i < adapter->num_queues; i++) {

you use adapter->num_queues.

[...]
> +static int ena_nway_reset(struct net_device *netdev)
> +{
> +ÂÂÂÂÂÂÂreturn -ENODEV;
> +}

That doesn't make sense. ÂThe device is there but presumably doesn't
support autonegotiation. ÂSo don't implement the operation.

[...]
> +static int ena_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂu8 *hfunc)
> +{
> +ÂÂÂÂÂÂÂstruct ena_adapter *adapter = netdev_priv(netdev);
> +ÂÂÂÂÂÂÂenum ena_admin_hash_functions ena_func;
> +ÂÂÂÂÂÂÂu8 func;
> +ÂÂÂÂÂÂÂint rc;
> +
> +ÂÂÂÂÂÂÂrc = ena_com_indirect_table_get(adapter->ena_dev, indir);
> +ÂÂÂÂÂÂÂif (rc)
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn rc;
> +
> +ÂÂÂÂÂÂÂrc = ena_com_get_hash_function(adapter->ena_dev, &ena_func, key);
> +ÂÂÂÂÂÂÂif (rc)
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn rc;
> +
> +ÂÂÂÂÂÂÂswitch (ena_func) {
> +ÂÂÂÂÂÂÂcase ENA_ADMIN_TOEPLITZ:
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂfunc = ETH_RSS_HASH_TOP;
> +ÂÂÂÂÂÂÂcase ENA_ADMIN_CRC32:
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂfunc = ETH_RSS_HASH_XOR;

Missing break statements.

> +ÂÂÂÂÂÂÂdefault:
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂnetif_err(adapter, drv, netdev,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ"Command parameter is not supported\n");
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂreturn -EOPNOTSUPP;
> +ÂÂÂÂÂÂÂ}
[...]
> +static const struct ethtool_ops ena_ethtool_ops = {
> +ÂÂÂÂÂÂÂ.get_settingsÂÂÂÂÂÂÂÂÂÂÂ= ena_get_settings,
[...]

get_settings is now deprecated in favour of get_link_ksettings.

Ben.

Ben.

-- 

Ben Hutchings
For every action, there is an equal and opposite criticism. - Harrison

Attachment: signature.asc
Description: This is a digitally signed message part