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  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.

Powered by blists - more mailing lists