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
| ||
|
Date: Sun, 22 Dec 2019 16:19:56 -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, 1/2] ethtool: Factored out similar ethtool link settings for virtual devices to core On Thu, Dec 19, 2019 at 3:54 PM Cris Forno <cforno12@...ux.vnet.ibm.com> wrote: > > 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. > > Signed-off-by: Cris Forno <cforno12@...ux.vnet.ibm.com> > --- > include/linux/ethtool.h | 2 ++ > net/core/ethtool.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index 95991e43..1b0417b 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -394,6 +394,8 @@ struct ethtool_ops { > struct ethtool_coalesce *); > int (*set_per_queue_coalesce)(struct net_device *, u32, > struct ethtool_coalesce *); > + bool (*virtdev_validate_link_ksettings)(const struct > + ethtool_link_ksettings *); > int (*get_link_ksettings)(struct net_device *, > struct ethtool_link_ksettings *); > int (*set_link_ksettings)(struct net_device *, > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > index cd9bc67..4091a94 100644 > --- a/net/core/ethtool.c > +++ b/net/core/ethtool.c > @@ -579,6 +579,32 @@ static int load_link_ksettings_from_user(struct ethtool_link_ksettings *to, > return 0; > } > > +/* Check if the user is trying to change anything besides speed/duplex */ > +static bool > +ethtool_virtdev_validate_cmd(const struct ethtool_link_ksettings *cmd) If called from other modules like drivers/net/virtio_net.ko, these functions cannot be static, need a declaration in include/linux/ethtool.h and an EXPORT_SYMBOL_GPL. Also return type should be on the same line. > +{ > + 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); > +} > + > /* convert a kernel internal ethtool_link_ksettings to > * ethtool_link_usettings in user space. return 0 on success, errno on > * error. > @@ -660,6 +686,17 @@ static int ethtool_get_link_ksettings(struct net_device *dev, > return store_link_ksettings_for_user(useraddr, &link_ksettings); > } > > +static int > +ethtool_virtdev_get_link_ksettings(struct net_device *dev, > + struct ethtool_link_ksettings *cmd, > + u32 *speed, u8 *duplex) No need to pass by reference, really. Indeed, the virtio_net caller passes vi->speed and vi->duplex instead of &vi->speed and &vi->duplex. More fundamentally, these three assignments are simple enough that I don't think a helper actually simplifies anything here. > +{ > + cmd->base.speed = *speed; > + cmd->base.duplex = *duplex; > + cmd->base.port = PORT_OTHER; > + return 0; > +} > + > /* Update device ethtool_link_settings. */ > static int ethtool_set_link_ksettings(struct net_device *dev, > void __user *useraddr) > @@ -696,6 +733,27 @@ static int ethtool_set_link_ksettings(struct net_device *dev, > return dev->ethtool_ops->set_link_ksettings(dev, &link_ksettings); > } > > +static int > +ethtool_virtdev_set_link_ksettings(struct net_device *dev, > + const struct ethtool_link_ksettings *cmd, > + u32 *dev_speed, u8 *dev_duplex) > +{ > + u32 speed; > + u8 duplex; > + > + speed = cmd->base.speed; > + duplex = cmd->base.duplex; > + /* don't allow custom speed and duplex */ > + if (!ethtool_validate_speed(speed) || > + !ethtool_validate_duplex(duplex) || > + !dev->ethtool_ops->virtdev_validate_link_ksettings(cmd)) > + return -EINVAL; > + *dev_speed = speed; > + *dev_duplex = duplex; > + > + return 0; > +} > + > /* Query device for its ethtool_cmd settings. > * > * Backward compatibility note: for compatibility with legacy ethtool, this is > -- > 1.8.3.1 >
Powered by blists - more mailing lists