netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Cris Forno <cforno12@linux.vnet.ibm.com>
Cc: netdev@vger.kernel.org, mst@redhat.com, jasowang@redhat.com,
	haiyangz@microsoft.com, sthemmin@microsoft.com,
	sashal@kernel.org, tlfalcon@linux.ibm.com
Subject: Re: [PATCH, net-next, v3, 0/2] net/ethtool: Introduce link_ksettings API for virtual network devices
Date: Thu, 19 Dec 2019 13:11:56 -0800	[thread overview]
Message-ID: <20191219131156.21332555@hermes.lan> (raw)
In-Reply-To: <20191219194057.4208-1-cforno12@linux.vnet.ibm.com>

On Thu, 19 Dec 2019 13:40:55 -0600
Cris Forno <cforno12@linux.vnet.ibm.com> wrote:

> This series provides an API for drivers of virtual network devices that allows
> users to alter initial device speed and duplex settings to reflect the actual
> capabilities of underlying hardware. The changes made include two helper
> functions ethtool_virtdev_get/set_link_ksettings, which are used to retrieve or
> update alterable link settings, respectively. In addition, there is a new
> ethtool operation defined to validate those settings provided by the user. This
> operation can use either a generic validation function,
> ethtool_virtdev_validate_cmd, or one defined by the driver. These changes
> resolve code duplication for existing virtual network drivers that have already
> implemented this behavior.  In the case of the ibmveth driver, this API is used
> to provide this capability for the first time.
> 
> ---
> v3: Factored out duplicated code to core/ethtool to provide API to virtual
>     drivers
>     
> v2: Updated default driver speed/duplex settings to avoid breaking existing
>     setups
> ---
> 
> Cris Forno (2):
>   Three virtual devices (ibmveth, virtio_net, and netvsc) all have
>     similar code to set/get link settings and validate ethtool command.
>     To     eliminate duplication of code, it is factored out into
>     core/ethtool.c.
>   With get/set link settings functions in core/ethtool.c, ibmveth,
>     netvsc, and virtio now use the core's helper function.
> 
>  drivers/net/ethernet/ibm/ibmveth.c | 60 +++++++++++++++++++++-----------------
>  drivers/net/ethernet/ibm/ibmveth.h |  3 ++
>  drivers/net/hyperv/netvsc_drv.c    | 21 ++++---------
>  drivers/net/virtio_net.c           | 45 ++++------------------------
>  include/linux/ethtool.h            |  2 ++
>  net/core/ethtool.c                 | 56 +++++++++++++++++++++++++++++++++++
>  6 files changed, 104 insertions(+), 83 deletions(-)
> 
> --
> 1.8.3.1

I don't think this makes sense for netvsc. The speed and duplex have no
meaning, why do you want to allow overriding it? If this is to try and make
some dashboard look good; then you aren't seeing the real speed which is
what only the host knows. Plus it does take into account the accelerated
networking path.

  parent reply	other threads:[~2019-12-19 21:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-19 19:40 [PATCH, net-next, v3, 0/2] net/ethtool: Introduce link_ksettings API for virtual network devices Cris Forno
2019-12-19 19:40 ` [PATCH, net-next, v3, 1/2] Three virtual devices (ibmveth, virtio_net, and netvsc) all have similar code to set/get link settings and validate ethtool command. To eliminate duplication of code, it is factored out into core/ethtool.c Cris Forno
2019-12-19 22:36   ` Michal Kubecek
2019-12-20  1:26     ` Thomas Falcon
2019-12-20  7:04       ` Michal Kubecek
2020-01-07 17:40         ` Cristobal Forno
2019-12-19 19:40 ` [PATCH, net-next, v3, 2/2] With get/set link settings functions in core/ethtool.c, ibmveth, netvsc, and virtio now use the core's helper function Cris Forno
2019-12-19 21:11 ` Stephen Hemminger [this message]
2019-12-19 22:16   ` [PATCH, net-next, v3, 0/2] net/ethtool: Introduce link_ksettings API for virtual network devices David Miller
2019-12-20  3:29     ` Stephen Hemminger
2019-12-20 22:53       ` David Miller
2019-12-20  3:58     ` Stephen Hemminger
2019-12-20 22:30     ` Stephen Hemminger
2019-12-19 20:54 Cris Forno

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191219131156.21332555@hermes.lan \
    --to=stephen@networkplumber.org \
    --cc=cforno12@linux.vnet.ibm.com \
    --cc=haiyangz@microsoft.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=sthemmin@microsoft.com \
    --cc=tlfalcon@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).