On Tue, Jun 11, 2019 at 04:27:47PM -0700, Florian Fainelli wrote:
> On 6/11/19 10:57 AM, Robert Hancock wrote:
> > We are using an embedded platform with a KSZ9897 switch. I am getting
> > the oops below in ksz_mib_read_work when testing with net-next branch.
> > After adding in some debug output, the problem is in this code:
> >
> > for (i = 0; i < dev->mib_port_cnt; i++) {
> > p = &dev->ports[i];
> > mib = &p->mib;
> > mutex_lock(&mib->cnt_mutex);
> >
> > /* Only read MIB counters when the port is told to do.
> > * If not, read only dropped counters when link is not up.
> > */
> > if (!p->read) {
> > const struct dsa_port *dp = dsa_to_port(dev->ds, i);
> >
> > if (!netif_carrier_ok(dp->slave))
> > mib->cnt_ptr = dev->reg_mib_cnt;
> > }
> >
> > The oops is happening on port index 3 (i.e. 4th port) which is not
> > connected on our platform and so has no entry in the device tree. For
> > that port, dp->slave is NULL and so netif_carrier_ok explodes.
> >
> > If I change the code to skip the port entirely in the loop if dp->slave
> > is NULL it seems to fix the crash, but I'm not that familiar with this
> > code. Can someone confirm whether that is the proper fix?
>
> Yes, the following should do it, if you confirm that is the case, I can
> send that later with your Tested-by.
>
> diff --git a/drivers/net/dsa/microchip/ksz_common.c
> b/drivers/net/dsa/microchip/ksz_common.c
> index 39dace8e3512..5470b28332cf 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -93,6 +93,9 @@ static void ksz_mib_read_work(struct work_struct *work)
> if (!p->read) {
> const struct dsa_port *dp = dsa_to_port(dev->ds, i);
>
> + if (dsa_is_unused_port(dp))
> + continue;
> +
> if (!netif_carrier_ok(dp->slave))
> mib->cnt_ptr = dev->reg_mib_cnt;
> }
>
Hi Florian
There is a mutex held within the loop. So a continue is not going to
work here.
Andrew
