netdev - Re: [PATCH net-next 1/1] netdevsim: support ethtool ring and coalesce settings

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]

Message-ID: <20201113114522.pn5ap6m4a2aqoz2j@lion.mk-sys.cz>
Date:   Fri, 13 Nov 2020 12:45:22 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Antonio Cardace <acardace@...hat.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/1] netdevsim: support ethtool ring and
 coalesce settings

On Thu, Nov 12, 2020 at 04:12:29PM +0100, Antonio Cardace wrote:
> Add ethtool ring and coalesce settings support for testing.
> 
> Signed-off-by: Antonio Cardace <acardace@...hat.com>
> ---
>  drivers/net/netdevsim/ethtool.c   | 65 +++++++++++++++++++++++++++----
>  drivers/net/netdevsim/netdevsim.h |  9 ++++-
>  2 files changed, 65 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
> index f1884d90a876..25acd3bc1781 100644
> --- a/drivers/net/netdevsim/ethtool.c
> +++ b/drivers/net/netdevsim/ethtool.c
> @@ -7,15 +7,18 @@
>  
>  #include "netdevsim.h"
>  
> +#define UINT32_MAX 0xFFFFFFFFU

We already have U32_MAX in <linux/limits.h>

> +#define ETHTOOL_COALESCE_ALL_PARAMS UINT32_MAX

I would rather prefer this constant to include only bits corresponding
to parameters which actually exist, i.e. either GENMASK(21, 0) or
combination of existing ETHTOOL_COALESCE_* macros. It should probably
be defined in include/linux/ethtool.h then.

[...]
> +static void nsim_get_ringparam(struct net_device *dev, struct ethtool_ringparam *ring)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +
> +	memcpy(ring, &ns->ethtool.ring, sizeof(ns->ethtool.ring));
> +}
> +
> +static int nsim_set_ringparam(struct net_device *dev, struct ethtool_ringparam *ring)
> +{
> +	struct netdevsim *ns = netdev_priv(dev);
> +
> +	memcpy(&ns->ethtool.ring, ring, sizeof(ns->ethtool.ring));
>  	return 0;
>  }
[...]
>  
> +static void nsim_ethtool_coalesce_init(struct netdevsim *ns)
> +{
> +	ns->ethtool.ring.rx_max_pending = UINT32_MAX;
> +	ns->ethtool.ring.rx_jumbo_max_pending = UINT32_MAX;
> +	ns->ethtool.ring.rx_mini_max_pending = UINT32_MAX;
> +	ns->ethtool.ring.tx_max_pending = UINT32_MAX;
> +}

This way an ETHTOOL_MSG_RINGS_SET request would never fail. It would be
more useful for selftests if the max values were more realistic and
ideally also configurable via debugfs.

Michal