| 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
| ||
|
Message-ID: <73d77bc7-6a1b-44de-a45f-967bbda68894@mellanox.com>
Date: Tue, 21 Jan 2020 09:39:38 +0000
From: Roi Dayan <roid@...lanox.com>
To: "xiangxia.m.yue@...il.com" <xiangxia.m.yue@...il.com>,
"gerlitz.or@...il.com" <gerlitz.or@...il.com>,
"saeedm@....mellanox.co.il" <saeedm@....mellanox.co.il>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2] net/mlx5e: Don't allow forwarding between
uplink
On 2020-01-19 4:25 AM, xiangxia.m.yue@...il.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@...il.com>
>
> We can install forwarding packets rule between uplink
> in switchdev mode, as show below. But the hardware does
> not do that as expected (mlnx_perf -i $PF1, we can't get
> the counter of the PF1). By the way, if we add the uplink
> PF0, PF1 to Open vSwitch and enable hw-offload, the rules
> can be offloaded but not work fine too. This patch add a
> check and if so return -EOPNOTSUPP.
>
> $ tc filter add dev $PF0 protocol all parent ffff: prio 1 handle 1 \
> flower skip_sw action mirred egress redirect dev $PF1
>
> $ tc -d -s filter show dev $PF0 ingress
> skip_sw
> in_hw in_hw_count 1
> action order 1: mirred (Egress Redirect to device enp130s0f1) stolen
> ...
> Sent 408954 bytes 4173 pkt (dropped 0, overlimits 0 requeues 0)
> Sent hardware 408954 bytes 4173 pkt
> ...
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@...il.com>
> ---
> v2: don't break LAG case
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 5 +++++
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.h | 1 +
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 23 ++++++++++++++++++++---
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> index f175cb2..ac2a035 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
> @@ -1434,6 +1434,11 @@ static struct devlink_port *mlx5e_get_devlink_port(struct net_device *dev)
> .ndo_set_features = mlx5e_set_features,
> };
>
> +bool mlx5e_eswitch_uplink_rep(struct net_device *netdev)
> +{
> + return netdev->netdev_ops == &mlx5e_netdev_ops_uplink_rep;
> +}
> +
> bool mlx5e_eswitch_rep(struct net_device *netdev)
> {
> if (netdev->netdev_ops == &mlx5e_netdev_ops_rep ||
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
> index 31f83c8..5211819 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
> @@ -199,6 +199,7 @@ void mlx5e_rep_encap_entry_detach(struct mlx5e_priv *priv,
> void mlx5e_rep_queue_neigh_stats_work(struct mlx5e_priv *priv);
>
> bool mlx5e_eswitch_rep(struct net_device *netdev);
> +bool mlx5e_eswitch_uplink_rep(struct net_device *netdev);
>
> #else /* CONFIG_MLX5_ESWITCH */
> static inline bool mlx5e_is_uplink_rep(struct mlx5e_priv *priv) { return false; }
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index db614bd..28f4522 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -3242,6 +3242,10 @@ static int add_vlan_pop_action(struct mlx5e_priv *priv,
> bool mlx5e_is_valid_eswitch_fwd_dev(struct mlx5e_priv *priv,
> struct net_device *out_dev)
> {
> + if (mlx5e_eswitch_uplink_rep(priv->netdev) &&
> + mlx5e_eswitch_uplink_rep(out_dev))
> + return false;
> +
> if (is_merged_eswitch_dev(priv, out_dev))
> return true;
>
> @@ -3361,6 +3365,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
> struct net_device *uplink_dev = mlx5_eswitch_uplink_get_proto_dev(esw, REP_ETH);
> struct net_device *uplink_upper;
> + struct mlx5e_rep_priv *rep_priv;
>
> if (is_duplicated_output_device(priv->netdev,
> out_dev,
> @@ -3396,10 +3401,22 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> return err;
> }
>
> - if (!mlx5e_is_valid_eswitch_fwd_dev(priv, out_dev)) {
> + /* Input vport was stored esw_attr->in_rep.
> + * In LAG case, *priv* is the private data of
> + * uplink which may be not the input vport.
> + * Use the input vport to check forwarding
> + * validity.
> + */
> + rep_priv = mlx5e_rep_to_rep_priv(attr->in_rep);
> + if (!mlx5e_is_valid_eswitch_fwd_dev(netdev_priv(rep_priv->netdev),
> + out_dev)) {
> NL_SET_ERR_MSG_MOD(extack,
> - "devices are not on same switch HW, can't offload forwarding");
> - pr_err("devices %s %s not on same switch HW, can't offload forwarding\n",
> + "devices are both uplink "
> + "are not on same switch HW, "
> + "can't offload forwarding");
> + pr_err("devices %s %s are both uplink "
> + "not on same switch HW, "
> + "can't offload forwarding\n",
> priv->netdev->name, out_dev->name);
can you fix the netlink msg to be the same as printk
the netlink msg iss "are both uplink are not on"
the printk msg is "are both uplink not on"
> return -EOPNOTSUPP;
> }
>
I noticed you still modified mlx5e_is_valid_eswitch_fwd_dev() which
is called from parse tc actions and also from resolving route for vxlan rules.
I tested the patch for normal, lag and ecmp modes.
ecmp vxlan encap rule breaks now as not supported.
the break is in get_route_and_out_devs() at this part
else if (mlx5e_eswitch_rep(dev) &&
mlx5e_is_valid_eswitch_fwd_dev(priv, dev))
since ecmp is like lag we fail on the same scenario here that
we test uplink priv but not input vport.
to activate ecmp we change both ports to switchdev mode
and add a multipath route rule.
you can see my test here
https://github.com/roidayan/ovs-tests/blob/master/test-ecmp-add-vxlan-rule.sh
Powered by blists - more mailing lists