On Mon, Nov 16, 2020 at 05:17:02PM +0100, Michal Kubecek wrote: > On Sat, Nov 14, 2020 at 12:16:54AM +0100, Antonio Cardace wrote: > > Factor out some useful functions so that they can be reused > > by other ethtool-netdevsim scripts. > > > > Signed-off-by: Antonio Cardace <acard...@redhat.com> > > Reviewed-by: Michal Kubecek <mkube...@suse.cz> > > Just one comment: > > [...] > > +function get_netdev_name { > > + local -n old=$1 > > + > > + new=$(ls /sys/class/net) > > + > > + for netdev in $new; do > > + for check in $old; do > > + [ $netdev == $check ] && break > > + done > > + > > + if [ $netdev != $check ]; then > > + echo $netdev > > + break > > + fi > > + done > > +} > [...] > > +function make_netdev { > > + # Make a netdevsim > > + old_netdevs=$(ls /sys/class/net) > > + > > + if ! $(lsmod | grep -q netdevsim); then > > + modprobe netdevsim > > + fi > > + > > + echo $NSIM_ID > /sys/bus/netdevsim/new_device > > + echo `get_netdev_name old_netdevs` > > +} > > This would be rather unpredictable if someone ran another selftest (or > anything else that would create a network device) in parallel. IMHO it > would be safer (and easier) to get the name of the new device from > > /sys/bus/netdevsim/devices/netdevsim${NSIM_ID}/net/ > > But as this is not new code and you are just moving existing code, it > can be done in a separate patch.
Yes it does make sense, I can send a patch for this once this is merged. Thanks for the review. Antonio