net/mlx5e: Prevent tunnel reformat when tunnel mode not allowed
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
Quoted 151 lines · Dan Carpenter
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:808mlx5e_xfrm_add_state()
warn: missing error code 'err'
drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c770 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 }
Quoted 2 lines
regards,
dan carpenter
Hi Dan, thanks for the report!
Will send a fix to net soon.
Carolina