All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Carolina Jubran <cjubran@nvidia.com>
Cc: linux-rdma@vger.kernel.org
Subject: [bug report] net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed
Date: Fri, 17 Oct 2025 11:54:03 +0300	[thread overview]
Message-ID: <aPIEK4rLB586FdDt@stanley.mountain> (raw)

Hello Carolina Jubran,

Commit 22239eb258bc ("net/mlx5e: Prevent tunnel reformat when tunnel
mode not allowed") from Oct 5, 2025 (linux-next), leads to the
following Smatch static checker warning:

	drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c:808 mlx5e_xfrm_add_state()
	warn: missing error code 'err'

drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
    770 static int mlx5e_xfrm_add_state(struct net_device *dev,
    771                                 struct xfrm_state *x,
    772                                 struct netlink_ext_ack *extack)
    773 {
    774         struct mlx5e_ipsec_sa_entry *sa_entry = NULL;
    775         bool allow_tunnel_mode = false;
    776         struct mlx5e_ipsec *ipsec;
    777         struct mlx5e_priv *priv;
    778         gfp_t gfp;
    779         int err;
    780 
    781         priv = netdev_priv(dev);
    782         if (!priv->ipsec)
    783                 return -EOPNOTSUPP;
    784 
    785         ipsec = priv->ipsec;
    786         gfp = (x->xso.flags & XFRM_DEV_OFFLOAD_FLAG_ACQ) ? GFP_ATOMIC : GFP_KERNEL;
    787         sa_entry = kzalloc(sizeof(*sa_entry), gfp);
    788         if (!sa_entry)
    789                 return -ENOMEM;
    790 
    791         sa_entry->x = x;
    792         sa_entry->dev = dev;
    793         sa_entry->ipsec = ipsec;
    794         /* Check if this SA is originated from acquire flow temporary SA */
    795         if (x->xso.flags & XFRM_DEV_OFFLOAD_FLAG_ACQ)
    796                 goto out;
    797 
    798         err = mlx5e_xfrm_validate_state(priv->mdev, x, extack);
    799         if (err)
    800                 goto err_xfrm;
    801 
    802         if (!mlx5_eswitch_block_ipsec(priv->mdev)) {
    803                 err = -EBUSY;
    804                 goto err_xfrm;
    805         }
    806 
    807         if (mlx5_eswitch_block_mode(priv->mdev))
--> 808                 goto unblock_ipsec;

Should we set the error code on this path?  err = -EINVAL?  If not the
way to silence these warnings would be to set "ret = 0;" within five
lines of the goto.  (The compiler will obviously remove that, it's just
for the checker and human readers).  Another option would be to add a
comment.

    809 
    810         if (x->props.mode == XFRM_MODE_TUNNEL &&
    811             x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
    812                 allow_tunnel_mode = mlx5e_ipsec_fs_tunnel_allowed(sa_entry);
    813                 if (!allow_tunnel_mode) {
    814                         NL_SET_ERR_MSG_MOD(extack,
    815                                            "Packet offload tunnel mode is disabled due to encap settings");
    816                         err = -EINVAL;
    817                         goto unblock_mode;
    818                 }
    819         }
    820 
    821         /* check esn */
    822         if (x->props.flags & XFRM_STATE_ESN)
    823                 mlx5e_ipsec_update_esn_state(sa_entry);
    824         else
    825                 /* According to RFC4303, section "3.3.3. Sequence Number Generation",
    826                  * the first packet sent using a given SA will contain a sequence
    827                  * number of 1.
    828                  */
    829                 sa_entry->esn_state.esn = 1;
    830 
    831         mlx5e_ipsec_build_accel_xfrm_attrs(sa_entry, &sa_entry->attrs);
    832 
    833         err = mlx5_ipsec_create_work(sa_entry);
    834         if (err)
    835                 goto unblock_encap;
    836 
    837         err = mlx5e_ipsec_create_dwork(sa_entry);
    838         if (err)
    839                 goto release_work;
    840 
    841         /* create hw context */
    842         err = mlx5_ipsec_create_sa_ctx(sa_entry);
    843         if (err)
    844                 goto release_dwork;
    845 
    846         err = mlx5e_accel_ipsec_fs_add_rule(sa_entry);
    847         if (err)
    848                 goto err_hw_ctx;
    849 
    850         /* We use *_bh() variant because xfrm_timer_handler(), which runs
    851          * in softirq context, can reach our state delete logic and we need
    852          * xa_erase_bh() there.
    853          */
    854         err = xa_insert_bh(&ipsec->sadb, sa_entry->ipsec_obj_id, sa_entry,
    855                            GFP_KERNEL);
    856         if (err)
    857                 goto err_add_rule;
    858 
    859         mlx5e_ipsec_set_esn_ops(sa_entry);
    860 
    861         if (sa_entry->dwork)
    862                 queue_delayed_work(ipsec->wq, &sa_entry->dwork->dwork,
    863                                    MLX5_IPSEC_RESCHED);
    864 
    865         if (allow_tunnel_mode) {
    866                 xa_lock_bh(&ipsec->sadb);
    867                 __xa_set_mark(&ipsec->sadb, sa_entry->ipsec_obj_id,
    868                               MLX5E_IPSEC_TUNNEL_SA);
    869                 xa_unlock_bh(&ipsec->sadb);
    870         }
    871 
    872 out:
    873         x->xso.offload_handle = (unsigned long)sa_entry;
    874         if (allow_tunnel_mode)
    875                 mlx5_eswitch_unblock_encap(priv->mdev);
    876 
    877         mlx5_eswitch_unblock_mode(priv->mdev);
    878 
    879         return 0;
    880 
    881 err_add_rule:
    882         mlx5e_accel_ipsec_fs_del_rule(sa_entry);
    883 err_hw_ctx:
    884         mlx5_ipsec_free_sa_ctx(sa_entry);
    885 release_dwork:
    886         kfree(sa_entry->dwork);
    887 release_work:
    888         if (sa_entry->work)
    889                 kfree(sa_entry->work->data);
    890         kfree(sa_entry->work);
    891 unblock_encap:
    892         if (allow_tunnel_mode)
    893                 mlx5_eswitch_unblock_encap(priv->mdev);
    894 unblock_mode:
    895         mlx5_eswitch_unblock_mode(priv->mdev);
    896 unblock_ipsec:
    897         mlx5_eswitch_unblock_ipsec(priv->mdev);
    898 err_xfrm:
    899         kfree(sa_entry);
    900         NL_SET_ERR_MSG_WEAK_MOD(extack, "Device failed to offload this state");
    901         return err;
    902 }

regards,
dan carpenter

             reply	other threads:[~2025-10-17  8:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-17  8:54 Dan Carpenter [this message]
2025-10-19 12:25 ` [bug report] net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed Carolina Jubran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPIEK4rLB586FdDt@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=cjubran@nvidia.com \
    --cc=linux-rdma@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.