Re: [Bridge] bridge: offload bridge port attributes to switch asic if feature flag set

[roopa <roopa@cumulusnetworks.com>]

On 2/5/15, 4:34 AM, Dan Carpenter wrote:
> Hello Roopa Prabhu,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 68e331c785b8: "bridge: offload bridge port attributes to
> switch asic if feature flag set" from Jan 29, 2015, leads to the
> following Smatch complaint:
>
> net/bridge/br_netlink.c:552 br_setlink()
> 	 error: we previously assumed 'p' could be null (see line 518)
>
> net/bridge/br_netlink.c
>     517	
>     518		if (p && protinfo) {
>                      ^
> Check for NULL.
>
>     519			if (protinfo->nla_type & NLA_F_NESTED) {
>     520				err = nla_parse_nested(tb, IFLA_BRPORT_MAX,
>     521						       protinfo, br_port_policy);
>     522				if (err)
>     523					return err;
>     524	
>     525				spin_lock_bh(&p->br->lock);
>     526				err = br_setport(p, tb);
>     527				spin_unlock_bh(&p->br->lock);
>     528			} else {
>     529				/* Binary compatibility with old RSTP */
>     530				if (nla_len(protinfo) < sizeof(u8))
>     531					return -EINVAL;
>     532	
>     533				spin_lock_bh(&p->br->lock);
>     534				err = br_set_port_state(p, nla_get_u8(protinfo));
>     535				spin_unlock_bh(&p->br->lock);
>     536			}
>     537			if (err)
>     538				goto out;
>     539		}
>     540	
>     541		if (afspec) {
>     542			err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
>     543					afspec, RTM_SETLINK);
>     544		}
>     545	
>     546		if (!(flags & BRIDGE_FLAGS_SELF)) {
>     547			/* set bridge attributes in hardware if supported
>     548			 */
>     549			ret_offload = netdev_switch_port_bridge_setlink(dev, nlh,
>     550									flags);
>     551			if (ret_offload && ret_offload != -EOPNOTSUPP)
>     552				br_warn(p->br, "error setting attrs on port %u(%s)\n",
>                                          ^^^^^
> New dererference.
>
>     553					(unsigned int)p->port_no, p->dev->name);
>     554		}
>
>
> Also:
> 	net/bridge/br_netlink.c:592 br_dellink()
> 	error: we previously assumed 'p' could be null (see line 575)
Thanks, yes, looks like i don't address the case where the message comes 
in on the bridge device.
I will post a patch a patch soon.