| Message ID | 1551607005-16321-1-git-send-email-xiangxia.m.yue@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config | expand |
On 03/03/2019 11:56, xiangxia.m.yue@gmail.com wrote: > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > If we try to set VFs mac address on a VF (not PF) net device, > the kernel will be crash. The commands are show as below: > > $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs > $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00 > > [exception RIP: mlx5_eswitch_set_vport_mac+41] > [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b > [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778 > [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63 > [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812 > [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab > [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f > [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412 > [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6 > [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860 > [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38 > [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b > [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c > > and > > [exception RIP: mlx5_eswitch_get_vport_config+12] > [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core] > [ffffa70607e57688] do_setlink at ffffffffbc67fa59 > [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778 > [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63 > [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812 > [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab > [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f > [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412 > [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6 > [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860 > [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38 > [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b > [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c > > Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager") > Cc: Eli Cohen <eli@mellanox.com> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > index 6cb9710..774edc9 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, > u64 node_guid; > int err = 0; > > + if (!ESW_ALLOWED(esw)) > + return -EPERM; > + this will introduce a bug with smart nic. from the commit in the fixes line, in smart nic the PF is not an esw manager so it will block changing vf mac with the pf. the fix should be checking if esw is null first. > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > return -EPERM; > if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) > @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, > { > struct mlx5_vport *evport; > > + if (!ESW_ALLOWED(esw)) > + return -EPERM; > + > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > return -EPERM; > if (!LEGAL_VPORT(esw, vport)) >
On Sun, Mar 3, 2019 at 8:42 PM Roi Dayan <roid@mellanox.com> wrote: > > > > On 03/03/2019 11:56, xiangxia.m.yue@gmail.com wrote: > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > > > If we try to set VFs mac address on a VF (not PF) net device, > > the kernel will be crash. The commands are show as below: > > > > $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs > > $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00 > > > > [exception RIP: mlx5_eswitch_set_vport_mac+41] > > [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b > > [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778 > > [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63 > > [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812 > > [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab > > [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f > > [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412 > > [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6 > > [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860 > > [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38 > > [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b > > [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c > > > > and > > > > [exception RIP: mlx5_eswitch_get_vport_config+12] > > [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core] > > [ffffa70607e57688] do_setlink at ffffffffbc67fa59 > > [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778 > > [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63 > > [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812 > > [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab > > [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f > > [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412 > > [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6 > > [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860 > > [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38 > > [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b > > [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c > > > > Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager") > > Cc: Eli Cohen <eli@mellanox.com> > > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > --- > > drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > index 6cb9710..774edc9 100644 > > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, > > u64 node_guid; > > int err = 0; > > > > + if (!ESW_ALLOWED(esw)) > > + return -EPERM; > > + > > this will introduce a bug with smart nic. > from the commit in the fixes line, in smart nic the PF > is not an esw manager so it will block changing vf mac > with the pf. the fix should be checking if esw is null first. Thanks for your reply, I don't get the smart nic card and can't test it. So to fix this bug, we only check the esw is null right ? diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c index 6cb9710..dc332ba 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, u64 node_guid; int err = 0; + if (!esw) + return -EPERM; + if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) return -EPERM; if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, { struct mlx5_vport *evport; + if (!esw) + return -EPERM; + if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) return -EPERM; if (!LEGAL_VPORT(esw, vport)) > > > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > > return -EPERM; > > if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) > > @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, > > { > > struct mlx5_vport *evport; > > > > + if (!ESW_ALLOWED(esw)) > > + return -EPERM; > > + > > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > > return -EPERM; > > if (!LEGAL_VPORT(esw, vport)) > >
On Sun, Mar 3, 2019 at 8:42 PM Roi Dayan <roid@mellanox.com> wrote: > > > > On 03/03/2019 11:56, xiangxia.m.yue@gmail.com wrote: > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > > > If we try to set VFs mac address on a VF (not PF) net device, > > the kernel will be crash. The commands are show as below: > > > > $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs > > $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00 > > > > [exception RIP: mlx5_eswitch_set_vport_mac+41] > > [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b > > [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778 > > [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63 > > [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812 > > [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab > > [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f > > [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412 > > [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6 > > [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860 > > [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38 > > [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b > > [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c > > > > and > > > > [exception RIP: mlx5_eswitch_get_vport_config+12] > > [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core] > > [ffffa70607e57688] do_setlink at ffffffffbc67fa59 > > [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778 > > [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63 > > [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812 > > [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab > > [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f > > [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412 > > [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6 > > [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860 > > [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38 > > [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b > > [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c > > > > Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager") > > Cc: Eli Cohen <eli@mellanox.com> > > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > > --- > > drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > index 6cb9710..774edc9 100644 > > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, > > u64 node_guid; > > int err = 0; > > > > + if (!ESW_ALLOWED(esw)) > > + return -EPERM; > > + > > this will introduce a bug with smart nic. > from the commit in the fixes line, in smart nic the PF > is not an esw manager so it will block changing vf mac > with the pf. the fix should be checking if esw is null first. and when i fix this bug, I review all the eswitch NDOs, i find except mlx5e_set_vf_mac mlx5e_get_vf_config other NDOs use the ESW_ALLOWED to check, that include mlx5e_set_vf_vlan mlx5e_set_vf_spoofchk mlx5e_set_vf_trust mlx5e_set_vf_rate (the bug will be fixed in patch 2) mlx5e_set_vf_link_state mlx5e_get_vf_stats > > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > > return -EPERM; > > if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) > > @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, > > { > > struct mlx5_vport *evport; > > > > + if (!ESW_ALLOWED(esw)) > > + return -EPERM; > > + > > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > > return -EPERM; > > if (!LEGAL_VPORT(esw, vport)) > >
On 04/03/2019 03:04, Tonghao Zhang wrote: > On Sun, Mar 3, 2019 at 8:42 PM Roi Dayan <roid@mellanox.com> wrote: >> >> >> >> On 03/03/2019 11:56, xiangxia.m.yue@gmail.com wrote: >>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com> >>> >>> If we try to set VFs mac address on a VF (not PF) net device, >>> the kernel will be crash. The commands are show as below: >>> >>> $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs >>> $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00 >>> >>> [exception RIP: mlx5_eswitch_set_vport_mac+41] >>> [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b >>> [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778 >>> [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63 >>> [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812 >>> [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab >>> [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f >>> [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412 >>> [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6 >>> [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860 >>> [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38 >>> [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b >>> [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c >>> >>> and >>> >>> [exception RIP: mlx5_eswitch_get_vport_config+12] >>> [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core] >>> [ffffa70607e57688] do_setlink at ffffffffbc67fa59 >>> [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778 >>> [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63 >>> [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812 >>> [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab >>> [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f >>> [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412 >>> [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6 >>> [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860 >>> [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38 >>> [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b >>> [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c >>> >>> Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager") >>> Cc: Eli Cohen <eli@mellanox.com> >>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> >>> --- >>> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c >>> index 6cb9710..774edc9 100644 >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c >>> @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, >>> u64 node_guid; >>> int err = 0; >>> >>> + if (!ESW_ALLOWED(esw)) >>> + return -EPERM; >>> + >> >> this will introduce a bug with smart nic. >> from the commit in the fixes line, in smart nic the PF >> is not an esw manager so it will block changing vf mac >> with the pf. the fix should be checking if esw is null first. > Thanks for your reply, I don't get the smart nic card and can't test > it. So to fix this bug, > we only check the esw is null right ? correct. in smart nic we have PF and ECPF. PF is vport manager but not esw manager and ECPF is the esw manager. We set vf mac through the pf so the condition here should only be vport group manager. > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > index 6cb9710..dc332ba 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, > u64 node_guid; > int err = 0; > > + if (!esw) > + return -EPERM; > + > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > return -EPERM; maybe just add the condition to the same if. if (!esw || !MLX5_CAP_GEN(esw->dev, vport_group_manager)) return -EPERM; > if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) > @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct > mlx5_eswitch *esw, > { > struct mlx5_vport *evport; > > + if (!esw) > + return -EPERM; > + > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > return -EPERM; > if (!LEGAL_VPORT(esw, vport)) > >> >>> if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) >>> return -EPERM; >>> if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) >>> @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, >>> { >>> struct mlx5_vport *evport; >>> >>> + if (!ESW_ALLOWED(esw)) >>> + return -EPERM; >>> + >>> if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) >>> return -EPERM; >>> if (!LEGAL_VPORT(esw, vport)) >>>
On 04/03/2019 03:26, Tonghao Zhang wrote: > On Sun, Mar 3, 2019 at 8:42 PM Roi Dayan <roid@mellanox.com> wrote: >> >> >> >> On 03/03/2019 11:56, xiangxia.m.yue@gmail.com wrote: >>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com> >>> >>> If we try to set VFs mac address on a VF (not PF) net device, >>> the kernel will be crash. The commands are show as below: >>> >>> $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs >>> $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00 >>> >>> [exception RIP: mlx5_eswitch_set_vport_mac+41] >>> [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b >>> [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778 >>> [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63 >>> [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812 >>> [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab >>> [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f >>> [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412 >>> [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6 >>> [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860 >>> [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38 >>> [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b >>> [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c >>> >>> and >>> >>> [exception RIP: mlx5_eswitch_get_vport_config+12] >>> [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core] >>> [ffffa70607e57688] do_setlink at ffffffffbc67fa59 >>> [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778 >>> [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63 >>> [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812 >>> [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab >>> [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f >>> [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412 >>> [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6 >>> [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860 >>> [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38 >>> [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b >>> [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c >>> >>> Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager") >>> Cc: Eli Cohen <eli@mellanox.com> >>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> >>> --- >>> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c >>> index 6cb9710..774edc9 100644 >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c >>> @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, >>> u64 node_guid; >>> int err = 0; >>> >>> + if (!ESW_ALLOWED(esw)) >>> + return -EPERM; >>> + >> >> this will introduce a bug with smart nic. >> from the commit in the fixes line, in smart nic the PF >> is not an esw manager so it will block changing vf mac >> with the pf. the fix should be checking if esw is null first. > and when i fix this bug, I review all the eswitch NDOs, i find except > mlx5e_set_vf_mac > mlx5e_get_vf_config > > other NDOs use the ESW_ALLOWED to check, that include > mlx5e_set_vf_vlan > mlx5e_set_vf_spoofchk > mlx5e_set_vf_trust > mlx5e_set_vf_rate (the bug will be fixed in patch 2) > mlx5e_set_vf_link_state > mlx5e_get_vf_stats > because some operations are limited to esw manager and some to vport group manager. the commit in your fixes list what is allowed. 1. set_vf_vlan - disallowed 2. set_vf_spoofchk - disallowed 3. set_vf_mac - allowed 4. get_vf_config - allowed 5. set_vf_trust - disallowed 6. set_vf_rate - disallowed 7. get_vf_stat - allowed 8. set_vf_link_state - disallowed from this maybe get_vf_stat is the only one checking for esw manager but maybe should be checked against vport manager. I'll have to check about that one to be sure. >>> if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) >>> return -EPERM; >>> if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) >>> @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, >>> { >>> struct mlx5_vport *evport; >>> >>> + if (!ESW_ALLOWED(esw)) >>> + return -EPERM; >>> + >>> if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) >>> return -EPERM; >>> if (!LEGAL_VPORT(esw, vport)) >>>
On Mon, Mar 4, 2019 at 3:06 PM Roi Dayan <roid@mellanox.com> wrote: > > > > On 04/03/2019 03:04, Tonghao Zhang wrote: > > On Sun, Mar 3, 2019 at 8:42 PM Roi Dayan <roid@mellanox.com> wrote: > >> > >> > >> > >> On 03/03/2019 11:56, xiangxia.m.yue@gmail.com wrote: > >>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com> > >>> > >>> If we try to set VFs mac address on a VF (not PF) net device, > >>> the kernel will be crash. The commands are show as below: > >>> > >>> $ echo 2 > /sys/class/net/$MLX_PF0/device/sriov_numvfs > >>> $ ip link set $MLX_VF0 vf 0 mac 00:11:22:33:44:00 > >>> > >>> [exception RIP: mlx5_eswitch_set_vport_mac+41] > >>> [ffffb8b7079e3688] do_setlink at ffffffff8f67f85b > >>> [ffffb8b7079e37a8] __rtnl_newlink at ffffffff8f683778 > >>> [ffffb8b7079e3b68] rtnl_newlink at ffffffff8f683a63 > >>> [ffffb8b7079e3b90] rtnetlink_rcv_msg at ffffffff8f67d812 > >>> [ffffb8b7079e3c10] netlink_rcv_skb at ffffffff8f6b88ab > >>> [ffffb8b7079e3c60] netlink_unicast at ffffffff8f6b808f > >>> [ffffb8b7079e3ca0] netlink_sendmsg at ffffffff8f6b8412 > >>> [ffffb8b7079e3d18] sock_sendmsg at ffffffff8f6452f6 > >>> [ffffb8b7079e3d30] ___sys_sendmsg at ffffffff8f645860 > >>> [ffffb8b7079e3eb0] __sys_sendmsg at ffffffff8f647a38 > >>> [ffffb8b7079e3f38] do_syscall_64 at ffffffff8f00401b > >>> [ffffb8b7079e3f50] entry_SYSCALL_64_after_hwframe at ffffffff8f80008c > >>> > >>> and > >>> > >>> [exception RIP: mlx5_eswitch_get_vport_config+12] > >>> [ffffa70607e57678] mlx5e_get_vf_config at ffffffffc03c7f8f [mlx5_core] > >>> [ffffa70607e57688] do_setlink at ffffffffbc67fa59 > >>> [ffffa70607e577a8] __rtnl_newlink at ffffffffbc683778 > >>> [ffffa70607e57b68] rtnl_newlink at ffffffffbc683a63 > >>> [ffffa70607e57b90] rtnetlink_rcv_msg at ffffffffbc67d812 > >>> [ffffa70607e57c10] netlink_rcv_skb at ffffffffbc6b88ab > >>> [ffffa70607e57c60] netlink_unicast at ffffffffbc6b808f > >>> [ffffa70607e57ca0] netlink_sendmsg at ffffffffbc6b8412 > >>> [ffffa70607e57d18] sock_sendmsg at ffffffffbc6452f6 > >>> [ffffa70607e57d30] ___sys_sendmsg at ffffffffbc645860 > >>> [ffffa70607e57eb0] __sys_sendmsg at ffffffffbc647a38 > >>> [ffffa70607e57f38] do_syscall_64 at ffffffffbc00401b > >>> [ffffa70607e57f50] entry_SYSCALL_64_after_hwframe at ffffffffbc80008c > >>> > >>> Fixes: a8d70a054a718 ("net/mlx5: E-Switch, Disallow vlan/spoofcheck setup if not being esw manager") > >>> Cc: Eli Cohen <eli@mellanox.com> > >>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com> > >>> --- > >>> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > >>> index 6cb9710..774edc9 100644 > >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > >>> @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, > >>> u64 node_guid; > >>> int err = 0; > >>> > >>> + if (!ESW_ALLOWED(esw)) > >>> + return -EPERM; > >>> + > >> > >> this will introduce a bug with smart nic. > >> from the commit in the fixes line, in smart nic the PF > >> is not an esw manager so it will block changing vf mac > >> with the pf. the fix should be checking if esw is null first. > > Thanks for your reply, I don't get the smart nic card and can't test > > it. So to fix this bug, > > we only check the esw is null right ? > > correct. in smart nic we have PF and ECPF. PF is vport manager but not > esw manager and ECPF is the esw manager. > We set vf mac through the pf so the condition here should only be > vport group manager. pretty good and v2 will be sent > > > > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > index 6cb9710..dc332ba 100644 > > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c > > @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, > > u64 node_guid; > > int err = 0; > > > > + if (!esw) > > + return -EPERM; > > + > > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > > return -EPERM; > > maybe just add the condition to the same if. > if (!esw || !MLX5_CAP_GEN(esw->dev, vport_group_manager)) > return -EPERM; > > > > if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) > > @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct > > mlx5_eswitch *esw, > > { > > struct mlx5_vport *evport; > > > > + if (!esw) > > + return -EPERM; > > + > > if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > > return -EPERM; > > if (!LEGAL_VPORT(esw, vport)) > > > >> > >>> if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > >>> return -EPERM; > >>> if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) > >>> @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, > >>> { > >>> struct mlx5_vport *evport; > >>> > >>> + if (!ESW_ALLOWED(esw)) > >>> + return -EPERM; > >>> + > >>> if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) > >>> return -EPERM; > >>> if (!LEGAL_VPORT(esw, vport)) > >>>
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c index 6cb9710..774edc9 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c @@ -1871,6 +1871,9 @@ int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw, u64 node_guid; int err = 0; + if (!ESW_ALLOWED(esw)) + return -EPERM; + if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) return -EPERM; if (!LEGAL_VPORT(esw, vport) || is_multicast_ether_addr(mac)) @@ -1945,6 +1948,9 @@ int mlx5_eswitch_get_vport_config(struct mlx5_eswitch *esw, { struct mlx5_vport *evport; + if (!ESW_ALLOWED(esw)) + return -EPERM; + if (!MLX5_CAP_GEN(esw->dev, vport_group_manager)) return -EPERM; if (!LEGAL_VPORT(esw, vport))