On Fri, Nov 13, 2020 at 12:45:22PM +0100, Michal Kubecek wrote:
> 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 <acard...@redhat.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
>
Thanks Michal and Jakub, I will send a v2 patch that addresses the
comments you made.
Antonio
