* [bug report] net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed
@ 2025-10-17 8:54 Dan Carpenter
2025-10-19 12:25 ` Carolina Jubran
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2025-10-17 8:54 UTC (permalink / raw)
To: Carolina Jubran; +Cc: linux-rdma
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed
2025-10-17 8:54 [bug report] net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed Dan Carpenter
@ 2025-10-19 12:25 ` Carolina Jubran
0 siblings, 0 replies; 2+ messages in thread
From: Carolina Jubran @ 2025-10-19 12:25 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-rdma
On 17/10/2025 11:54, Dan Carpenter wrote:
> 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
Hi Dan, thanks for the report!
Will send a fix to net soon.
Carolina
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-10-19 12:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-17 8:54 [bug report] net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed Dan Carpenter
2025-10-19 12:25 ` Carolina Jubran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox