netdev - Re: [RFC PATCH] net: realtek: r8169: implement set_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 21 Oct 2017 21:37:52 +0200
From:   Tobias Jakobi <tjakobi@...h.uni-bielefeld.de>
To:     nic_swsd@...ltek.com
Cc:     netdev@...r.kernel.org, tremyfr@...il.com, davem@...emloft.net
Subject: Re: [RFC PATCH] net: realtek: r8169: implement set_link_ksettings()

Any comment on this one? I have been using this in my tree since submission. No
issues to far.

I guess I could also put a Fixes-tag?

- Tobias


Tobias Jakobi wrote:
> Commit 6fa1ba61520576cf1346c4ff09a056f2950cb3bf partially
> implemented the new ethtool API, by replacing get_settings()
> with get_link_ksettings(). This breaks ethtool, since the
> userspace tool (according to the new API specs) never tries
> the legacy set() call, when the new get() call succeeds.
> 
> All attempts to chance some setting from userspace result in:
>> Cannot set new settings: Operation not supported
> 
> Implement the missing set() call.
> 
> Signed-off-by: Tobias Jakobi <tjakobi@...h.uni-bielefeld.de>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 40 +++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index e03fcf914690..24e8f7133038 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -2025,21 +2025,6 @@ static int rtl8169_set_speed(struct net_device *dev,
>  	return ret;
>  }
>  
> -static int rtl8169_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> -{
> -	struct rtl8169_private *tp = netdev_priv(dev);
> -	int ret;
> -
> -	del_timer_sync(&tp->timer);
> -
> -	rtl_lock_work(tp);
> -	ret = rtl8169_set_speed(dev, cmd->autoneg, ethtool_cmd_speed(cmd),
> -				cmd->duplex, cmd->advertising);
> -	rtl_unlock_work(tp);
> -
> -	return ret;
> -}
> -
>  static netdev_features_t rtl8169_fix_features(struct net_device *dev,
>  	netdev_features_t features)
>  {
> @@ -2166,6 +2151,29 @@ static int rtl8169_get_link_ksettings(struct net_device *dev,
>  	return rc;
>  }
>  
> +static int rtl8169_set_link_ksettings(struct net_device *dev,
> +				      const struct ethtool_link_ksettings *cmd)
> +{
> +	struct rtl8169_private *tp = netdev_priv(dev);
> +	int rc;
> +	u32 advertising;
> +
> +	if (!ethtool_convert_link_mode_to_legacy_u32(&advertising,
> +	    cmd->link_modes.advertising))
> +		return -EINVAL;
> +
> +	del_timer_sync(&tp->timer);
> +
> +	rtl_lock_work(tp);
> +
> +	rc = rtl8169_set_speed(dev, cmd->base.autoneg, cmd->base.speed,
> +			       cmd->base.duplex, advertising);
> +
> +	rtl_unlock_work(tp);
> +
> +	return rc;
> +}
> +
>  static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
>  			     void *p)
>  {
> @@ -2367,7 +2375,6 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
>  	.get_drvinfo		= rtl8169_get_drvinfo,
>  	.get_regs_len		= rtl8169_get_regs_len,
>  	.get_link		= ethtool_op_get_link,
> -	.set_settings		= rtl8169_set_settings,
>  	.get_msglevel		= rtl8169_get_msglevel,
>  	.set_msglevel		= rtl8169_set_msglevel,
>  	.get_regs		= rtl8169_get_regs,
> @@ -2379,6 +2386,7 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
>  	.get_ts_info		= ethtool_op_get_ts_info,
>  	.nway_reset		= rtl8169_nway_reset,
>  	.get_link_ksettings	= rtl8169_get_link_ksettings,
> +	.set_link_ksettings	= rtl8169_set_link_ksettings,
>  };
>  
>  static void rtl8169_get_mac_version(struct rtl8169_private *tp,
>