netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
@ 2019-03-03  9:56 xiangxia.m.yue
  2019-03-03  9:56 ` [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate xiangxia.m.yue
  2019-03-03 12:42 ` [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config Roi Dayan
  0 siblings, 2 replies; 12+ messages in thread
From: xiangxia.m.yue @ 2019-03-03  9:56 UTC (permalink / raw)
  To: saeedm, gerlitz.or, davem; +Cc: netdev, Tonghao Zhang, Eli Cohen

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;
+
 	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))
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate
  2019-03-03  9:56 [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config xiangxia.m.yue
@ 2019-03-03  9:56 ` xiangxia.m.yue
  2019-03-03 14:12   ` Roi Dayan
  2019-03-03 12:42 ` [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config Roi Dayan
  1 sibling, 1 reply; 12+ messages in thread
From: xiangxia.m.yue @ 2019-03-03  9:56 UTC (permalink / raw)
  To: saeedm, gerlitz.or, davem; +Cc: netdev, Tonghao Zhang, Mohamad Haj Yahia

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

If we try to set VFs rate 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 rate 100

Fixes: c9497c98901c ("net/mlx5: Add support for setting VF min rate")
Cc: Mohamad Haj Yahia <mohamad@mellanox.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 774edc9..8c2f1e6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -2122,19 +2122,24 @@ static int normalize_vports_min_rate(struct mlx5_eswitch *esw, u32 divider)
 int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, int vport,
 				u32 max_rate, u32 min_rate)
 {
-	u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
-	bool min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
-					fw_max_bw_share >= MLX5_MIN_BW_SHARE;
-	bool max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
 	struct mlx5_vport *evport;
+	u32 fw_max_bw_share;
 	u32 previous_min_rate;
 	u32 divider;
+	bool min_rate_supported;
+	bool max_rate_supported;
 	int err = 0;
 
 	if (!ESW_ALLOWED(esw))
 		return -EPERM;
 	if (!LEGAL_VPORT(esw, vport))
 		return -EINVAL;
+
+	fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
+	min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
+				fw_max_bw_share >= MLX5_MIN_BW_SHARE;
+	max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
+
 	if ((min_rate && !min_rate_supported) || (max_rate && !max_rate_supported))
 		return -EOPNOTSUPP;
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
  2019-03-03  9:56 [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config xiangxia.m.yue
  2019-03-03  9:56 ` [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate xiangxia.m.yue
@ 2019-03-03 12:42 ` Roi Dayan
  2019-03-04  1:04   ` Tonghao Zhang
  2019-03-04  1:26   ` Tonghao Zhang
  1 sibling, 2 replies; 12+ messages in thread
From: Roi Dayan @ 2019-03-03 12:42 UTC (permalink / raw)
  To: xiangxia.m.yue, Saeed Mahameed, gerlitz.or, davem; +Cc: netdev, Eli Cohen



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))
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate
  2019-03-03  9:56 ` [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate xiangxia.m.yue
@ 2019-03-03 14:12   ` Roi Dayan
  2019-03-03 14:17     ` Roi Dayan
  2019-03-03 14:17     ` Roi Dayan
  0 siblings, 2 replies; 12+ messages in thread
From: Roi Dayan @ 2019-03-03 14:12 UTC (permalink / raw)
  To: xiangxia.m.yue, Saeed Mahameed, gerlitz.or, davem
  Cc: netdev, Mohamad Haj Yahia



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 rate 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 rate 100

actually this didn't reproduce with this command + your first fix.

setting rate calls netlink IFLA_VF_TX_RATE which is calling
nfo_get_vf_config() which is now returning error correctly.

we need the netlink call to be IFLA_VF_RATE. seems we get there
when we set min/max_tx_rate.
so i reproduce the error with this command
$ ip link set dev $VF vf 0 max_tx_rate 2 max_tx_rate 1

can you verify and fix the commit msg?

> 
> Fixes: c9497c98901c ("net/mlx5: Add support for setting VF min rate")
> Cc: Mohamad Haj Yahia <mohamad@mellanox.com>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index 774edc9..8c2f1e6 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -2122,19 +2122,24 @@ static int normalize_vports_min_rate(struct mlx5_eswitch *esw, u32 divider)
>  int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, int vport,
>  				u32 max_rate, u32 min_rate)
>  {
> -	u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
> -	bool min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
> -					fw_max_bw_share >= MLX5_MIN_BW_SHARE;
> -	bool max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
>  	struct mlx5_vport *evport;
> +	u32 fw_max_bw_share;
>  	u32 previous_min_rate;
>  	u32 divider;
> +	bool min_rate_supported;
> +	bool max_rate_supported;
>  	int err = 0;
>  
>  	if (!ESW_ALLOWED(esw))
>  		return -EPERM;
>  	if (!LEGAL_VPORT(esw, vport))
>  		return -EINVAL;
> +
> +	fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
> +	min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
> +				fw_max_bw_share >= MLX5_MIN_BW_SHARE;
> +	max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
> +
>  	if ((min_rate && !min_rate_supported) || (max_rate && !max_rate_supported))
>  		return -EOPNOTSUPP;
>  
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate
  2019-03-03 14:12   ` Roi Dayan
@ 2019-03-03 14:17     ` Roi Dayan
  2019-03-03 14:17     ` Roi Dayan
  1 sibling, 0 replies; 12+ messages in thread
From: Roi Dayan @ 2019-03-03 14:17 UTC (permalink / raw)
  To: xiangxia.m.yue, Saeed Mahameed, gerlitz.or, davem
  Cc: netdev, Mohamad Haj Yahia



On 03/03/2019 16:12, Roi Dayan 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 rate 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 rate 100
> 
> actually this didn't reproduce with this command + your first fix.
> 
> setting rate calls netlink IFLA_VF_TX_RATE which is calling
> nfo_get_vf_config() which is now returning error correctly.
> 
> we need the netlink call to be IFLA_VF_RATE. seems we get there
> when we set min/max_tx_rate.
> so i reproduce the error with this command
> $ ip link set dev $VF vf 0 max_tx_rate 2 max_tx_rate 1

i had a typo here. the cmd is
> $ ip link set dev $VF vf 0 max_tx_rate 2 min_tx_rate 1

> 
> can you verify and fix the commit msg?
> 
>>
>> Fixes: c9497c98901c ("net/mlx5: Add support for setting VF min rate")
>> Cc: Mohamad Haj Yahia <mohamad@mellanox.com>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>> index 774edc9..8c2f1e6 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>> @@ -2122,19 +2122,24 @@ static int normalize_vports_min_rate(struct mlx5_eswitch *esw, u32 divider)
>>  int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, int vport,
>>  				u32 max_rate, u32 min_rate)
>>  {
>> -	u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
>> -	bool min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
>> -					fw_max_bw_share >= MLX5_MIN_BW_SHARE;
>> -	bool max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
>>  	struct mlx5_vport *evport;
>> +	u32 fw_max_bw_share;
>>  	u32 previous_min_rate;
>>  	u32 divider;
>> +	bool min_rate_supported;
>> +	bool max_rate_supported;
>>  	int err = 0;
>>  
>>  	if (!ESW_ALLOWED(esw))
>>  		return -EPERM;
>>  	if (!LEGAL_VPORT(esw, vport))
>>  		return -EINVAL;
>> +
>> +	fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
>> +	min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
>> +				fw_max_bw_share >= MLX5_MIN_BW_SHARE;
>> +	max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
>> +
>>  	if ((min_rate && !min_rate_supported) || (max_rate && !max_rate_supported))
>>  		return -EOPNOTSUPP;
>>  
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate
  2019-03-03 14:12   ` Roi Dayan
  2019-03-03 14:17     ` Roi Dayan
@ 2019-03-03 14:17     ` Roi Dayan
  2019-03-04  3:06       ` Tonghao Zhang
  1 sibling, 1 reply; 12+ messages in thread
From: Roi Dayan @ 2019-03-03 14:17 UTC (permalink / raw)
  To: xiangxia.m.yue, Saeed Mahameed, gerlitz.or, davem
  Cc: netdev, Mohamad Haj Yahia



On 03/03/2019 16:12, Roi Dayan 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 rate 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 rate 100
> 
> actually this didn't reproduce with this command + your first fix.
> 
> setting rate calls netlink IFLA_VF_TX_RATE which is calling
> nfo_get_vf_config() which is now returning error correctly.
> 
> we need the netlink call to be IFLA_VF_RATE. seems we get there
> when we set min/max_tx_rate.
> so i reproduce the error with this command
> $ ip link set dev $VF vf 0 max_tx_rate 2 max_tx_rate 1

I had a typo here. the cmd is
$ ip link set dev $VF vf 0 max_tx_rate 2 min_tx_rate 1

> 
> can you verify and fix the commit msg?
> 
>>
>> Fixes: c9497c98901c ("net/mlx5: Add support for setting VF min rate")
>> Cc: Mohamad Haj Yahia <mohamad@mellanox.com>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>> index 774edc9..8c2f1e6 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>> @@ -2122,19 +2122,24 @@ static int normalize_vports_min_rate(struct mlx5_eswitch *esw, u32 divider)
>>  int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, int vport,
>>  				u32 max_rate, u32 min_rate)
>>  {
>> -	u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
>> -	bool min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
>> -					fw_max_bw_share >= MLX5_MIN_BW_SHARE;
>> -	bool max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
>>  	struct mlx5_vport *evport;
>> +	u32 fw_max_bw_share;
>>  	u32 previous_min_rate;
>>  	u32 divider;
>> +	bool min_rate_supported;
>> +	bool max_rate_supported;
>>  	int err = 0;
>>  
>>  	if (!ESW_ALLOWED(esw))
>>  		return -EPERM;
>>  	if (!LEGAL_VPORT(esw, vport))
>>  		return -EINVAL;
>> +
>> +	fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
>> +	min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
>> +				fw_max_bw_share >= MLX5_MIN_BW_SHARE;
>> +	max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
>> +
>>  	if ((min_rate && !min_rate_supported) || (max_rate && !max_rate_supported))
>>  		return -EOPNOTSUPP;
>>  
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
  2019-03-03 12:42 ` [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config Roi Dayan
@ 2019-03-04  1:04   ` Tonghao Zhang
  2019-03-04  7:06     ` Roi Dayan
  2019-03-04  1:26   ` Tonghao Zhang
  1 sibling, 1 reply; 12+ messages in thread
From: Tonghao Zhang @ 2019-03-04  1:04 UTC (permalink / raw)
  To: Roi Dayan; +Cc: Saeed Mahameed, gerlitz.or, davem, netdev, Eli Cohen

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))
> >

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
  2019-03-03 12:42 ` [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config Roi Dayan
  2019-03-04  1:04   ` Tonghao Zhang
@ 2019-03-04  1:26   ` Tonghao Zhang
  2019-03-04  7:11     ` Roi Dayan
  1 sibling, 1 reply; 12+ messages in thread
From: Tonghao Zhang @ 2019-03-04  1:26 UTC (permalink / raw)
  To: Roi Dayan; +Cc: Saeed Mahameed, gerlitz.or, davem, netdev, Eli Cohen

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))
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate
  2019-03-03 14:17     ` Roi Dayan
@ 2019-03-04  3:06       ` Tonghao Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Tonghao Zhang @ 2019-03-04  3:06 UTC (permalink / raw)
  To: Roi Dayan; +Cc: Saeed Mahameed, gerlitz.or, davem, netdev, Mohamad Haj Yahia

On Sun, Mar 3, 2019 at 10:17 PM Roi Dayan <roid@mellanox.com> wrote:
>
>
>
> On 03/03/2019 16:12, Roi Dayan 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 rate 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 rate 100
> >
> > actually this didn't reproduce with this command + your first fix.
> >
> > setting rate calls netlink IFLA_VF_TX_RATE which is calling
> > nfo_get_vf_config() which is now returning error correctly.
> >
> > we need the netlink call to be IFLA_VF_RATE. seems we get there
> > when we set min/max_tx_rate.
> > so i reproduce the error with this command
> > $ ip link set dev $VF vf 0 max_tx_rate 2 max_tx_rate 1
>
> I had a typo here. the cmd is
> $ ip link set dev $VF vf 0 max_tx_rate 2 min_tx_rate 1
>
> >
> > can you verify and fix the commit msg?
Yes, thanks.
> >>
> >> Fixes: c9497c98901c ("net/mlx5: Add support for setting VF min rate")
> >> Cc: Mohamad Haj Yahia <mohamad@mellanox.com>
> >> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >> ---
> >>  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 13 +++++++++----
> >>  1 file changed, 9 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> >> index 774edc9..8c2f1e6 100644
> >> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> >> @@ -2122,19 +2122,24 @@ static int normalize_vports_min_rate(struct mlx5_eswitch *esw, u32 divider)
> >>  int mlx5_eswitch_set_vport_rate(struct mlx5_eswitch *esw, int vport,
> >>                              u32 max_rate, u32 min_rate)
> >>  {
> >> -    u32 fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
> >> -    bool min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
> >> -                                    fw_max_bw_share >= MLX5_MIN_BW_SHARE;
> >> -    bool max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
> >>      struct mlx5_vport *evport;
> >> +    u32 fw_max_bw_share;
> >>      u32 previous_min_rate;
> >>      u32 divider;
> >> +    bool min_rate_supported;
> >> +    bool max_rate_supported;
> >>      int err = 0;
> >>
> >>      if (!ESW_ALLOWED(esw))
> >>              return -EPERM;
> >>      if (!LEGAL_VPORT(esw, vport))
> >>              return -EINVAL;
> >> +
> >> +    fw_max_bw_share = MLX5_CAP_QOS(esw->dev, max_tsar_bw_share);
> >> +    min_rate_supported = MLX5_CAP_QOS(esw->dev, esw_bw_share) &&
> >> +                            fw_max_bw_share >= MLX5_MIN_BW_SHARE;
> >> +    max_rate_supported = MLX5_CAP_QOS(esw->dev, esw_rate_limit);
> >> +
> >>      if ((min_rate && !min_rate_supported) || (max_rate && !max_rate_supported))
> >>              return -EOPNOTSUPP;
> >>
> >>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
  2019-03-04  1:04   ` Tonghao Zhang
@ 2019-03-04  7:06     ` Roi Dayan
  2019-03-04  8:26       ` Tonghao Zhang
  0 siblings, 1 reply; 12+ messages in thread
From: Roi Dayan @ 2019-03-04  7:06 UTC (permalink / raw)
  To: Tonghao Zhang; +Cc: Saeed Mahameed, gerlitz.or, davem, netdev, Eli Cohen



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))
>>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
  2019-03-04  1:26   ` Tonghao Zhang
@ 2019-03-04  7:11     ` Roi Dayan
  0 siblings, 0 replies; 12+ messages in thread
From: Roi Dayan @ 2019-03-04  7:11 UTC (permalink / raw)
  To: Tonghao Zhang; +Cc: Saeed Mahameed, gerlitz.or, davem, netdev, Eli Cohen



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))
>>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config
  2019-03-04  7:06     ` Roi Dayan
@ 2019-03-04  8:26       ` Tonghao Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Tonghao Zhang @ 2019-03-04  8:26 UTC (permalink / raw)
  To: Roi Dayan; +Cc: Saeed Mahameed, gerlitz.or, davem, netdev, Eli Cohen

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))
> >>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-03-04  8:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-03  9:56 [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config xiangxia.m.yue
2019-03-03  9:56 ` [PATCH net 2/2] net/mlx5: Avoid panic when setting VF rate xiangxia.m.yue
2019-03-03 14:12   ` Roi Dayan
2019-03-03 14:17     ` Roi Dayan
2019-03-03 14:17     ` Roi Dayan
2019-03-04  3:06       ` Tonghao Zhang
2019-03-03 12:42 ` [PATCH 1/2] net/mlx5: Avoid panic when setting vport mac, getting vport config Roi Dayan
2019-03-04  1:04   ` Tonghao Zhang
2019-03-04  7:06     ` Roi Dayan
2019-03-04  8:26       ` Tonghao Zhang
2019-03-04  1:26   ` Tonghao Zhang
2019-03-04  7:11     ` Roi Dayan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).