| 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: <CAMDZJNXrZzkQaEBWBO4j3=0-LA1ExGaKa0+0Whg0R8NugbkLTw@mail.gmail.com>
Date: Wed, 22 Jan 2020 19:24:51 +0800
From: Tonghao Zhang <xiangxia.m.yue@...il.com>
To: Roi Dayan <roid@...lanox.com>
Cc: "gerlitz.or@...il.com" <gerlitz.or@...il.com>,
"saeedm@....mellanox.co.il" <saeedm@....mellanox.co.il>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2] net/mlx5e: Don't allow forwarding between uplink
On Tue, Jan 21, 2020 at 5:39 PM Roi Dayan <roid@...lanox.com> wrote:
>
>
>
> 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
I sent v3 again, and thanks for your review.
I'll be taking my holidays soon. And I can't reply to you immediately.
Powered by blists - more mailing lists