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
next 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.