On 02/08/2018 11:54 AM, 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?
Hi Dan, thanks for your report.
This one is already fixed, see here:
https://lkml.org/lkml/2018/7/31/577
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.
And it is still a success path, once err is init to 0.
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 }
Thanks,
Tariq
--
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
