| 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: <5508b43e-2d2a-d86f-890f-58ca8e87e456@mellanox.com>
Date: Wed, 15 Jan 2020 09:31:54 +0000
From: Roi Dayan <roid@...lanox.com>
To: Tonghao Zhang <xiangxia.m.yue@...il.com>,
Or Gerlitz <gerlitz.or@...il.com>
CC: Saeed Mahameed <saeedm@....mellanox.co.il>,
Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink
On 2020-01-09 1:05 PM, Tonghao Zhang wrote:
> On Thu, Jan 9, 2020 at 6:32 PM Or Gerlitz <gerlitz.or@...il.com> wrote:
>>
>> On Tue, Jan 7, 2020 at 11:52 AM <xiangxia.m.yue@...il.com> wrote:
>>
>>> We can install forwarding packets rule between uplink
>>> in eswitchdev 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
>>
>> yeah, IMOH there's probably a firmware bug under which installation
>> of this rule doesn't fail but as you said the traffic doesn't pass.
>>
>> We don't fail the rule b/c due to what we call "merged eswitch"
>> which is a building block of the uplink LAG and ECMP configurations
>> that we do support, the driver lets you install a flow from (say) VF0
>> on PF/uplink0 to uplink1 etc.
>>
>> Talking to Roi, he prefers to further investigate / open a case to the FW
>> and later see if/how to block this rule in the driver.
> make any process, please let me know, thanks
>
>>> 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>
>>> ---
>>> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c | 7 ++++++-
>>> drivers/net/ethernet/mellanox/mlx5/core/en_rep.h | 1 +
>>> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 16 +++++++++++++---
>>> 3 files changed, 20 insertions(+), 4 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..63fad66 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>>> @@ -1434,10 +1434,15 @@ 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 ||
>>> - netdev->netdev_ops == &mlx5e_netdev_ops_uplink_rep)
>>> + mlx5e_eswitch_uplink_rep(netdev))
>>> return true;
>>>
>>> return false;
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
>>> index 31f83c8..282c64b 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
>>> @@ -198,6 +198,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_uplink_rep(struct net_device *netdev);
>>> bool mlx5e_eswitch_rep(struct net_device *netdev);
>>>
>>> #else /* CONFIG_MLX5_ESWITCH */
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>>> index f91e057e..b2c18fa 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
>>> @@ -3214,6 +3214,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;
>>>
>>> @@ -3339,9 +3343,15 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
>>>
>>> if (!mlx5e_is_valid_eswitch_fwd_dev(priv, 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",
>>> - priv->netdev->name, out_dev->name);
>>> + "devices are both uplink "
>>> + "or not on same switch HW, "
>>> + "can't offload forwarding");
>>> + pr_err("devices %s %s are both uplink "
>>> + "or not on same switch HW, "
>>> + "can't offload forwarding\n",
>>> + priv->netdev->name,
>>> + out_dev->name);
>>> +
>>> return -EOPNOTSUPP;
>>> }
>>>
>>> --
>>> 1.8.3.1
>>>
Hi,
I forwarded the issue to fw for a response.
Though, we can progress with blocking uplink->uplink rules but this patch
currently breaks LAG mode.
In LAG mode we add a rule to both esw0 of uplink0 and esw1 of uplink1,
so for the check it looks like priv and out_dev are both uplinks,
but this is allowed as Or said. we forward to the other eswitch but
different vport. so the check is missing if the vport is uplink.
so normally priv is the representor priv but in case of LAG
mlx5e_add_fdb_flow()->is_peer_flow_needed()->mlx5e_tc_add_fdb_peer_flow()
we pass priv as the uplink of the other eswitch.
the vport is saved in esw_attr->in_rep.
so for this case we need to compare if in_rep (and not priv) is actually
vf representor or uplink representor.
the case i tested is create bond0 with the 2 uplinks.
add rule from representor to bond0.
e.g. tc filter add dev ens1f0_0 protocol arp parent ffff: prio 3 flower \
dst_mac e4:1d:2d:fd:8b:02 skip_sw action mirred egress redirect dev bond0
Thanks,
Roi
Powered by blists - more mailing lists