Re: [PATCH net-next v1 1/4] net-e1000e: fix ethtool set_featurestaking new features into account too late

[Michał Mirosław]

2011/11/22 David Decotigny <david.decotigny@xxxxxxxxxx>:
[...]
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index a5bd7a3..b63f316 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -5901,24 +5901,28 @@ static void e1000_eeprom_checks(struct e1000_adapter *adapter)
> Â}
>
> Âstatic int e1000_set_features(struct net_device *netdev,
> - Â Â Â netdev_features_t features)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â netdev_features_t features)
> Â{
> Â Â Â Âstruct e1000_adapter *adapter = netdev_priv(netdev);
> Â Â Â Ânetdev_features_t changed = features ^ netdev->features;
> + Â Â Â int retval = 1; Â/* telling netdev that we are updating
> + Â Â Â Â Â Â Â Â Â Â Â Â * netdev->features by ourselves */
> +
> + Â Â Â netdev->features = features;
>
> Â Â Â Âif (changed & (NETIF_F_TSO | NETIF_F_TSO6))
> Â Â Â Â Â Â Â Âadapter->flags |= FLAG_TSO_FORCE;
>
> Â Â Â Âif (!(changed & (NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX |
> Â Â Â Â Â Â Â Â Â Â Â Â NETIF_F_RXCSUM)))
> - Â Â Â Â Â Â Â return 0;
> + Â Â Â Â Â Â Â return retval;
>

Would be less code if you set netdev->features here...

> Â Â Â Âif (netif_running(netdev))
> Â Â Â Â Â Â Â Âe1000e_reinit_locked(adapter);
> Â Â Â Âelse
> Â Â Â Â Â Â Â Âe1000e_reset(adapter);
>
> - Â Â Â return 0;
> + Â Â Â return retval;

... and return 1 here, noting in a comment that e1000e_reinit_locked()
might have changed netdev->features.

> Â}

Best Regards,
MichaÅ MirosÅaw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/