Re: [bug report] net/mlx5e: Gather all XDP pre-requisite checks in a single function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+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


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux