Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1672281

Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test

From Andrew Lunn <andrew@lunn.ch>
Newsgroups linux.kernel
Subject Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test
Date 2017-06-22 05:40 +0200
Message-ID <tV18l-7pp-5@gated-at.bofh.it> (permalink)
References (4 earlier) <tUDfI-8jH-5@gated-at.bofh.it> <tUElr-C1-5@gated-at.bofh.it> <tUEOt-Lf-3@gated-at.bofh.it> <tUO1t-6FS-47@gated-at.bofh.it> <tUZpV-68j-47@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


> I don't think returning to user space is an option, because it mean
> ethtool phy self test would fail because if availability of some software
> resoure, which is not some application would expect. here is what I can
> think of:
> 
> driver/net/ethernet/hisilicon/hns/hns_ethtool.c
> int phy_loopback_selftest(struct phy_device *dev)
> {
> 	int err;
> 
> 	mutex_lock(dev->mutex);
> 
> 	if (dev->drv->loopback)
> 		err = dev->drv->loopback(dev, 1);
> 
> 	if (err)
> 		goto out;
> 	
> 	/* mac driver do the test*/
> 	.................
> 
> 
> 	if (dev->drv->loopback)
> 		err = dev->drv->loopback(dev, 0);
> 
> 	if (err)
> 		goto out;
> 
> out:
> 	mutex_unlock(dev->mutex);
> 	return err;
> }

I don't think you need a mutex here. dev_ioctl.c:

       case SIOCETHTOOL:
                dev_load(net, ifr.ifr_name);
                rtnl_lock();
                ret = dev_ethtool(net, &ifr);
                rtnl_unlock();
 
So all ethtool operations are protected by the rtnl. You cannot have
two tests running at once.

> One question I can think of is that, how do we ensure mac driver enable and
> disable phy loopback in pair, enable loopback after dev->mutex is taken,
> disable loopback  before dev->mutex is released?
> I hope I am not missing something obvious, any suggestion and idea?

You cannot ensure this. But it would be a pretty obvious bug.

    Andrew

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test l00371289 <linyunsheng@huawei.com> - 2017-06-20 05:10 +0200
  Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test Andrew Lunn <andrew@lunn.ch> - 2017-06-20 15:30 +0200
    Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test l00371289 <linyunsheng@huawei.com> - 2017-06-21 04:10 +0200
      Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test Andrew Lunn <andrew@lunn.ch> - 2017-06-21 05:20 +0200
        Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test l00371289 <linyunsheng@huawei.com> - 2017-06-21 05:50 +0200
          Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test Andrew Lunn <andrew@lunn.ch> - 2017-06-21 15:40 +0200
            Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test l00371289 <linyunsheng@huawei.com> - 2017-06-22 03:50 +0200
              Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test Andrew Lunn <andrew@lunn.ch> - 2017-06-22 05:40 +0200
                Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test l00371289 <linyunsheng@huawei.com> - 2017-06-22 06:00 +0200

csiph-web