netdev - Re: [PATCH net-next 2/2] mlx5: convert to new udp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 Jul 2020 08:35:53 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "kuba@...nel.org" <kuba@...nel.org>
CC:     "mkubecek@...e.cz" <mkubecek@...e.cz>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH net-next 2/2] mlx5: convert to new udp_tunnel
 infrastructure

On Fri, 2020-07-24 at 19:51 -0700, Jakub Kicinski wrote:
> Allocate nic_info dynamically - n_entries is not constant.
> 
> Drop the ndo callbacks from the reprs, those should be local to
> the same netns as the main netdev, no need to get the same callbacks
> multiple times.
> 

Isn't this a problem ? so it seems this is the root cause of the
regression failure we saw with this patch.

in a switchdev mode the "main" netdev is unregistered and we register
another netdev with ndos: "mlx5e_netdev_ops_uplink_rep" as the new main
netdev (the uplink representor) where you removed the vxlan ndos, 
see below.. 

[...]
> 
>  static netdev_features_t mlx5e_tunnel_features_check(struct
> mlx5e_priv *priv,
>  						     struct sk_buff
> *skb,
>  						     netdev_features_t
> features)
> @@ -4620,8 +4543,8 @@ const struct net_device_ops mlx5e_netdev_ops =
> {
>  	.ndo_change_mtu          = mlx5e_change_nic_mtu,
>  	.ndo_do_ioctl            = mlx5e_ioctl,
>  	.ndo_set_tx_maxrate      = mlx5e_set_tx_maxrate,
> -	.ndo_udp_tunnel_add      = mlx5e_add_vxlan_port,
> -	.ndo_udp_tunnel_del      = mlx5e_del_vxlan_port,
> +	.ndo_udp_tunnel_add      = udp_tunnel_nic_add_port,
> +	.ndo_udp_tunnel_del      = udp_tunnel_nic_del_port,
>  	.ndo_features_check      = mlx5e_features_check,
>  	.ndo_tx_timeout          = mlx5e_tx_timeout,
>  	.ndo_bpf		 = mlx5e_xdp,
> @@ -4935,6 +4858,8 @@ static void mlx5e_build_nic_netdev(struct
> net_device *netdev)
>  	netdev->hw_features      |= NETIF_F_HW_VLAN_CTAG_FILTER;
>  	netdev->hw_features      |= NETIF_F_HW_VLAN_STAG_TX;
>  
> +	mlx5_vxlan_set_netdev_info(mdev->vxlan, netdev);
> +
>  	if (mlx5_vxlan_allowed(mdev->vxlan) ||
> mlx5_geneve_tx_allowed(mdev) ||
>  	    mlx5e_any_tunnel_proto_supported(mdev)) {
>  		netdev->hw_enc_features |= NETIF_F_HW_CSUM;
> @@ -5240,8 +5165,7 @@ static void mlx5e_nic_enable(struct mlx5e_priv
> *priv)
>  	rtnl_lock();
>  	if (netif_running(netdev))
>  		mlx5e_open(netdev);
> -	if (mlx5_vxlan_allowed(priv->mdev->vxlan))
> -		udp_tunnel_get_rx_info(netdev);
> +	udp_tunnel_nic_reset_ntf(priv->netdev);
>  	netif_device_attach(netdev);
>  	rtnl_unlock();
>  }
> @@ -5256,8 +5180,6 @@ static void mlx5e_nic_disable(struct mlx5e_priv
> *priv)
>  	rtnl_lock();
>  	if (netif_running(priv->netdev))
>  		mlx5e_close(priv->netdev);
> -	if (mlx5_vxlan_allowed(priv->mdev->vxlan))
> -		udp_tunnel_drop_rx_info(priv->netdev);
>  	netif_device_detach(priv->netdev);
>  	rtnl_unlock();
>  
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> index c300729fb498..000a7f264fda 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> @@ -633,8 +633,6 @@ static const struct net_device_ops
> mlx5e_netdev_ops_uplink_rep = {
>  	.ndo_has_offload_stats	 = mlx5e_rep_has_offload_stats,
>  	.ndo_get_offload_stats	 = mlx5e_rep_get_offload_stats,
>  	.ndo_change_mtu          = mlx5e_uplink_rep_change_mtu,
> -	.ndo_udp_tunnel_add      = mlx5e_add_vxlan_port,
> -	.ndo_udp_tunnel_del      = mlx5e_del_vxlan_port,

Here, this is uplink representor (i.e main netdev).
we need the udp_tunnel_ndos.

also we need to add:
mlx5_vxlan_set_netdev_info(mdev->vxlan, netdev);

in mlx5e_build_rep_netdev() under 
if (rep->vport == MLX5_VPORT_UPLINK) statement.

[...]