The 03/18/2021 15:11, Dan Carpenter wrote:
Hi Dan,
>
> See also:
>
> drivers/net/ethernet/mscc/ocelot_mrp.c:254 ocelot_mrp_del_ring_role() error: we previously assumed 'ocelot_port' could be null (see line 247)
>
> regards,
> dan carpenter
>
> On Thu, Mar 18, 2021 at 03:07:32PM +0300, Dan Carpenter wrote:
> > Hello Horatiu Vultur,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch 7c588c3e96e9: "net: ocelot: Extend MRP" from Mar 16, 2021,
> > leads to the following Smatch complaint:
> >
> > drivers/net/ethernet/mscc/ocelot_mrp.c:180 ocelot_mrp_del()
> > error: we previously assumed 'ocelot_port' could be null (see line 173)
> >
> > drivers/net/ethernet/mscc/ocelot_mrp.c
> > 153 int ocelot_mrp_del(struct ocelot *ocelot, int port,
> > 154 const struct switchdev_obj_mrp *mrp)
> > 155 {
> > 156 struct ocelot_port *ocelot_port = ocelot->ports[port];
> > 157 int i;
> > 158
> > 159 if (!ocelot_port)
> > 160 return -EOPNOTSUPP;
> > 161
> > 162 if (ocelot_port->mrp_ring_id != mrp->ring_id)
> > 163 return 0;
> > 164
> > 165 ocelot_mrp_del_vcap(ocelot, port);
> > 166 ocelot_mrp_del_vcap(ocelot, port + ocelot->num_phys_ports);
> > 167
> > 168 ocelot_port->mrp_ring_id = 0;
> > 169
> > 170 for (i = 0; i < ocelot->num_phys_ports; ++i) {
> >
> > This loop tries to verify that all the ports have ring_id == 0 otherwise
> > we return. It's slightly confusing why this matters.
It is trying to detect if there is any existing MRP ring active on the
node. A port is part of a MRP ring if mrp_ring_id != 0. If there is no
MRP ring then we need to remove the entry in the MAC table otherwise
keep it there.
> >
> > 171 ocelot_port = ocelot->ports[i];
> > 172
> > 173 if (!ocelot_port)
> > ^^^^^^^^^^^
> > Assume the last element of the array is NULL
> >
> > 174 continue;
> > 175
> > 176 if (ocelot_port->mrp_ring_id != 0)
> > 177 goto out;
> >
> > This would be more clear if instead of a "goto out;" it just did a
> > direct "return 0;"
I can change this.
> >
> > 178 }
> > 179
> > 180 ocelot_mrp_del_mac(ocelot, ocelot_port);
> >
> > We delete the last mac address of all the ring_ids are zero but if the
> > last port is zero it will crash.
Yep, it should be ocelot->ports[port] as I have done it in the first
version.
I will send a patch for this.
> >
> > 181 out:
> > 182 return 0;
> >
> > regards,
> > dan carpenter
--
/Horatiu
