+netdev and Saeed
On Thu, Aug 02, 2018 at 11:54:49AM +0300, Dan Carpenter wrote:
> Hello Tariq Toukan,
>
> The patch 0ec13877ce95: "net/mlx5e: Gather all XDP pre-requisite
> checks in a single function" from Mar 12, 2018, leads to the
> following Smatch warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c:4284 mlx5e_xdp_set()
> error: uninitialized symbol 'err'.
>
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> 4214 static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
> 4215 {
> 4216 struct mlx5e_priv *priv = netdev_priv(netdev);
> 4217 struct bpf_prog *old_prog;
> 4218 bool reset, was_opened;
> 4219 int err;
> ^^^^^^^
> I always encourage people to remove unneeded initializers so that GCC
> can detect uninitialized variables, but it turns out that GCC misses a
> bunch of bugs?
>
> 4220 int i;
> 4221
> 4222 mutex_lock(&priv->state_lock);
> 4223
> 4224 if (prog) {
> 4225 err = mlx5e_xdp_allowed(priv, prog);
> 4226 if (err)
> 4227 goto unlock;
> 4228 }
> 4229
> 4230 was_opened = test_bit(MLX5E_STATE_OPENED, &priv->state);
> 4231 /* no need for full reset when exchanging programs */
> 4232 reset = (!priv->channels.params.xdp_prog || !prog);
> 4233
> 4234 if (was_opened && reset)
> 4235 mlx5e_close_locked(netdev);
> 4236 if (was_opened && !reset) {
> 4237 /* num_channels is invariant here, so we can take the
> 4238 * batched reference right upfront.
> 4239 */
> 4240 prog = bpf_prog_add(prog, priv->channels.num);
> 4241 if (IS_ERR(prog)) {
> 4242 err = PTR_ERR(prog);
> 4243 goto unlock;
> 4244 }
> 4245 }
> 4246
> 4247 /* exchange programs, extra prog reference we got from caller
> 4248 * as long as we don't fail from this point onwards.
> 4249 */
> 4250 old_prog = xchg(&priv->channels.params.xdp_prog, prog);
> 4251 if (old_prog)
> 4252 bpf_prog_put(old_prog);
> 4253
> 4254 if (reset) /* change RQ type according to priv->xdp_prog */
> 4255 mlx5e_set_rq_type(priv->mdev, &priv->channels.params);
> 4256
> 4257 if (was_opened && reset)
> 4258 mlx5e_open_locked(netdev);
> 4259
> 4260 if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
> 4261 goto unlock;
> ^^^^^^^^^^^
> In the original code this was a success path.
>
> 4262
> 4263 /* exchanging programs w/o reset, we update ref counts on behalf
> 4264 * of the channels RQs here.
> 4265 */
> 4266 for (i = 0; i < priv->channels.num; i++) {
> 4267 struct mlx5e_channel *c = priv->channels.c[i];
> 4268
> 4269 clear_bit(MLX5E_RQ_STATE_ENABLED, &c->rq.state);
> 4270 napi_synchronize(&c->napi);
> 4271 /* prevent mlx5e_poll_rx_cq from accessing rq->xdp_prog */
> 4272
> 4273 old_prog = xchg(&c->rq.xdp_prog, prog);
> 4274
> 4275 set_bit(MLX5E_RQ_STATE_ENABLED, &c->rq.state);
> 4276 /* napi_schedule in case we have missed anything */
> 4277 napi_schedule(&c->napi);
> 4278
> 4279 if (old_prog)
> 4280 bpf_prog_put(old_prog);
> 4281 }
> 4282
> 4283 unlock:
> 4284 mutex_unlock(&priv->state_lock);
> 4285 return err;
> ^^^^^^^^^^
> 4286 }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
