netdev - Re: [PATCH, net-next, v5, 2/2] net/ethtool: Introduce link_ksettings API for virtual network devices

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]

Message-ID: <m2blpmwdwd.fsf@Criss-MacBook-Pro.local.i-did-not-set--mail-host-address--so-tickle-me>
Date:   Tue, 25 Feb 2020 15:33:22 -0600
From:   Cris Forno <cforno12@...ux.vnet.ibm.com>
To:     Michal Kubecek <mkubecek@...e.cz>, netdev@...r.kernel.org
Cc:     mst@...hat.com, jasowang@...hat.com, haiyangz@...rosoft.com,
        sthemmin@...rosoft.com, sashal@...nel.org, tlfalcon@...ux.ibm.com,
        davem@...emloft.net, willemdebruijn.kernel@...il.com,
        kuba@...nel.org
Subject: Re: [PATCH, net-next, v5, 2/2] net/ethtool: Introduce link_ksettings API for virtual network devices

These changes have been applied to v6. Thanks for the suggestions Michal!

Michal Kubecek <mkubecek@...e.cz> writes:

> On Tue, Feb 18, 2020 at 11:52:27AM -0600, Cris Forno wrote:
>> With get/set link settings functions in core/ethtool.c, ibmveth,
>> netvsc, and virtio now use the core's helper function.
>> 
>> Funtionality changes that pertain to ibmveth driver include:
>> 
>>   1. Changed the initial hardcoded link speed to 1GB.
>> 
>>   2. Added support for allowing a user to change the reported link
>>   speed via ethtool.
>> 
>> Changes to the netvsc driver include:
>> 
>>   1. When netvsc_get_link_ksettings is called, it will defer to the VF
>>   device if it exists to pull accelerated networking values, otherwise
>>   pull default or user-defined values.
>> 
>>   2. Similarly, if netvsc_set_link_ksettings called and a VF device
>>   exists, the real values of speed and duplex are changed.
>> 
>> Signed-off-by: Cris Forno <cforno12@...ux.vnet.ibm.com>
>> ---
> [...]
>> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
>> index 65e12cb..f733ec5 100644
>> --- a/drivers/net/hyperv/netvsc_drv.c
>> +++ b/drivers/net/hyperv/netvsc_drv.c
> [...]
>> @@ -1187,18 +1193,19 @@ static int netvsc_set_link_ksettings(struct net_device *dev,
>>  				     const struct ethtool_link_ksettings *cmd)
>>  {
>>  	struct net_device_context *ndc = netdev_priv(dev);
>> -	u32 speed;
>> +	struct net_device *vf_netdev = rtnl_dereference(ndc->vf_netdev);
>>  
>> -	speed = cmd->base.speed;
>> -	if (!ethtool_validate_speed(speed) ||
>> -	    !ethtool_validate_duplex(cmd->base.duplex) ||
>> -	    !netvsc_validate_ethtool_ss_cmd(cmd))
>> -		return -EINVAL;
>> +	if (vf_netdev) {
>> +		if (!vf_netdev->ethtool_ops->set_link_ksettings)
>> +			return -EOPNOTSUPP;
>>  
>> -	ndc->speed = speed;
>> -	ndc->duplex = cmd->base.duplex;
>> +		return vf_netdev->ethtool_ops->set_link_ksettings(vf_netdev,
>> +								  cmd);
>> +	}
>>  
>> -	return 0;
>> +	return ethtool_virtdev_set_link_ksettings(dev, cmd,
>> +						  &ndc->speed, &ndc->duplex,
>> +						  &netvsc_validate_ethtool_ss_cmd);
>>  }
>
> I may be missing something obvious but I cannot see how does
> netvsc_validate_ethtool_ss_cmd() differ from ethtool_virtdev_validate_cmd().
> If it does, it would be probably worth a comment at the function.
>
> Michal Kubecek