netdev - Re: [PATCH, net-next, v3, 2/2] net: Enable virtual network devices to use ethtool's set/get link settings functions

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] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 22 Dec 2019 16:26:32 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Cris Forno <cforno12@...ux.vnet.ibm.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>, haiyangz@...rosoft.com,
        sthemmin@...rosoft.com, sashal@...nel.org, tlfalcon@...ux.ibm.com
Subject: Re: [PATCH, net-next, v3, 2/2] net: Enable virtual network devices to
 use ethtool's set/get link settings functions

On Thu, Dec 19, 2019 at 3:54 PM Cris Forno <cforno12@...ux.vnet.ibm.com> wrote:
>
> With get/set link settings functions in core/ethtool.c, ibmveth,
> netvsc, and virtio now use the core's helper function.
>
> Signed-off-by: Cris Forno <cforno12@...ux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 60 +++++++++++++++++++++-----------------
>  drivers/net/ethernet/ibm/ibmveth.h |  3 ++

There appears to be more going on here than simply replacing the local
version of functions with equivalent shared helpers.

Please briefly document in the commit message anything that is not a
just a noop.

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 5a635f0..5cbcb16 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2166,48 +2166,15 @@ static void virtnet_get_channels(struct net_device *dev,
>         channels->other_count = 0;
>  }
>
> -/* Check if the user is trying to change anything besides speed/duplex */
> -static bool
> -virtnet_validate_ethtool_cmd(const struct ethtool_link_ksettings *cmd)
> -{
> -       struct ethtool_link_ksettings diff1 = *cmd;
> -       struct ethtool_link_ksettings diff2 = {};
> -
> -       /* cmd is always set so we need to clear it, validate the port type
> -        * and also without autonegotiation we can ignore advertising
> -        */
> -       diff1.base.speed = 0;
> -       diff2.base.port = PORT_OTHER;
> -       ethtool_link_ksettings_zero_link_mode(&diff1, advertising);
> -       diff1.base.duplex = 0;
> -       diff1.base.cmd = 0;
> -       diff1.base.link_mode_masks_nwords = 0;
> -
> -       return !memcmp(&diff1.base, &diff2.base, sizeof(diff1.base)) &&
> -               bitmap_empty(diff1.link_modes.supported,
> -                            __ETHTOOL_LINK_MODE_MASK_NBITS) &&
> -               bitmap_empty(diff1.link_modes.advertising,
> -                            __ETHTOOL_LINK_MODE_MASK_NBITS) &&
> -               bitmap_empty(diff1.link_modes.lp_advertising,
> -                            __ETHTOOL_LINK_MODE_MASK_NBITS);
>  }

Stray parenthesis: build failure.