netdev - Re: [net-next 05/11] net/mlx5e: Fail attempt to offload e-switch TC flows with egress upper devices

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 20 Dec 2018 00:22:03 +0200
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Linux Netdev List <netdev@...r.kernel.org>,
        Eli Britstein <elibr@...lanox.com>
Subject: Re: [net-next 05/11] net/mlx5e: Fail attempt to offload e-switch TC
 flows with egress upper devices

On Thu, Dec 20, 2018 at 12:16 AM Saeed Mahameed <saeedm@...lanox.com> wrote:
>
> From: Eli Britstein <elibr@...lanox.com>
>
> We use the switchdev parent HW id helper to identify if the mirred device
> shares the same ASIC/port with the ingress device. This can get us wrong in
> the presence of upper devices (e.g vlan, team, etc) set over the HW devices
> (VF or uplink representors), b/c the switchdev ID is retrieved recursively.
>
> To fail offload attempts in such cases, we condition the check on the egress
> device to have not only the same switchdev ID but also the relevant mlx5 netdev ops.

The patch was made before we pushed vf lag.. need to fix it up a bit:

net/mlx5e: Fail attempt to offload e-switch TC flows with egress vlan devices

We use the switchdev parent HW id helper to identify if the mirred
device shares the same ASIC/port with the ingress device. This can get
us wrong in the presence of vlan upper devices set over the HW devices
(VF or uplink representors), b/c the switchdev ID is retrieved
recursively. To fail offload attempts in such cases, we condition the
check on the egress device to have not only the same switchdev ID but
also the relevant mlx5 netdev ops.


>
> Fixes: 03a9d11e6eeb ('net/mlx5e: Add TC drop and mirred/redirect action parsing for SRIOV offloads')
> Signed-off-by: Eli Britstein <elibr@...lanox.com>
> Reviewed-by: Roi Dayan <roid@...lanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 9 +++++++++
>  drivers/net/ethernet/mellanox/mlx5/core/en_rep.h | 3 +++
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c  | 3 +++
>  3 files changed, 15 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> index 91c3eb85f32e..f414f19c1159 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> @@ -1316,6 +1316,15 @@ static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
>         .ndo_get_vf_stats        = mlx5e_get_vf_stats,
>  };
>
> +bool mlx5e_eswitch_rep(struct net_device *netdev)
> +{
> +       if (netdev->netdev_ops == &mlx5e_netdev_ops_vf_rep ||
> +           netdev->netdev_ops == &mlx5e_netdev_ops_uplink_rep)
> +               return true;
> +
> +       return false;
> +}
> +
>  static void mlx5e_build_rep_params(struct net_device *netdev)
>  {
>         struct mlx5e_priv *priv = netdev_priv(netdev);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
> index 5645d3cef1bb..edd722824697 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
> @@ -176,6 +176,9 @@ void mlx5e_rep_encap_entry_detach(struct mlx5e_priv *priv,
>                                   struct mlx5e_encap_entry *e);
>
>  void mlx5e_rep_queue_neigh_stats_work(struct mlx5e_priv *priv);
> +
> +bool mlx5e_eswitch_rep(struct net_device *netdev);
> +
>  #else /* CONFIG_MLX5_ESWITCH */
>  static inline bool mlx5e_is_uplink_rep(struct mlx5e_priv *priv) { return false; }
>  static inline int mlx5e_add_sqs_fwd_rules(struct mlx5e_priv *priv) { return 0; }
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index c1a9120412b8..a4c6287dfd65 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2578,6 +2578,9 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
>                                 struct net_device *uplink_dev = mlx5_eswitch_uplink_get_proto_dev(esw, REP_ETH);
>                                 struct net_device *uplink_upper = netdev_master_upper_dev_get(uplink_dev);
>
> +                               if (!mlx5e_eswitch_rep(out_dev))
> +                                       return -EOPNOTSUPP;
> +

no, out_dev can be the team/bond device and this way we break vf lag

>                                 if (uplink_upper &&
>                                     netif_is_lag_master(uplink_upper) &&
>                                     uplink_upper == out_dev)

should move it here, after we realize that and potentially made
out_dev be the uplink rep

Otherwise, Acked- by: Or Gerlitz <ogerlitz@...lanox.com>