netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink
@ 2020-01-07  9:52 xiangxia.m.yue
  2020-01-09 10:31 ` Or Gerlitz
  0 siblings, 1 reply; 4+ messages in thread
From: xiangxia.m.yue @ 2020-01-07  9:52 UTC (permalink / raw)
  To: gerlitz.or, saeedm, roid; +Cc: netdev, Tonghao Zhang

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

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
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@gmail.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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink
  2020-01-07  9:52 [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink xiangxia.m.yue
@ 2020-01-09 10:31 ` Or Gerlitz
  2020-01-09 11:05   ` Tonghao Zhang
  0 siblings, 1 reply; 4+ messages in thread
From: Or Gerlitz @ 2020-01-09 10:31 UTC (permalink / raw)
  To: Tonghao Zhang; +Cc: Saeed Mahameed, Roi Dayan, Linux Netdev List

On Tue, Jan 7, 2020 at 11:52 AM <xiangxia.m.yue@gmail.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.

> 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@gmail.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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink
  2020-01-09 10:31 ` Or Gerlitz
@ 2020-01-09 11:05   ` Tonghao Zhang
  2020-01-15  9:31     ` Roi Dayan
  0 siblings, 1 reply; 4+ messages in thread
From: Tonghao Zhang @ 2020-01-09 11:05 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: Saeed Mahameed, Roi Dayan, Linux Netdev List

On Thu, Jan 9, 2020 at 6:32 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Tue, Jan 7, 2020 at 11:52 AM <xiangxia.m.yue@gmail.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@gmail.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
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink
  2020-01-09 11:05   ` Tonghao Zhang
@ 2020-01-15  9:31     ` Roi Dayan
  0 siblings, 0 replies; 4+ messages in thread
From: Roi Dayan @ 2020-01-15  9:31 UTC (permalink / raw)
  To: Tonghao Zhang, Or Gerlitz; +Cc: Saeed Mahameed, Linux Netdev List



On 2020-01-09 1:05 PM, Tonghao Zhang wrote:
> On Thu, Jan 9, 2020 at 6:32 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>
>> On Tue, Jan 7, 2020 at 11:52 AM <xiangxia.m.yue@gmail.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@gmail.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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-15  9:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07  9:52 [PATCH net-next] net/mlx5e: Don't allow forwarding between uplink xiangxia.m.yue
2020-01-09 10:31 ` Or Gerlitz
2020-01-09 11:05   ` Tonghao Zhang
2020-01-15  9:31     ` Roi Dayan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).