* [PATCH net-next 1/6] net: bridge: Publish bridge accessor functions
2018-04-26 9:06 [PATCH net-next 0/6] mlxsw: SPAN: Support routes pointing at bridges Ido Schimmel
@ 2018-04-26 9:06 ` Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 2/6] mlxsw: spectrum: Extract mlxsw_sp_stp_spms_state() Ido Schimmel
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2018-04-26 9:06 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, stephen, jiri, nikolay, petrm, mlxsw, Ido Schimmel
From: Petr Machata <petrm@mellanox.com>
To allow querying FDB and vlan settings of a bridge, publish several
existing functions and add some new ones.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/linux/if_bridge.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_fdb.c | 25 +++++++++++++++++++++
net/bridge/br_private.h | 17 +++++++++------
net/bridge/br_vlan.c | 32 +++++++++++++++++++++++++++
4 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 02639ebea2f0..2020f61505b9 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -93,11 +93,66 @@ static inline bool br_multicast_router(const struct net_device *dev)
#if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_BRIDGE_VLAN_FILTERING)
bool br_vlan_enabled(const struct net_device *dev);
+
+struct net_bridge_vlan_group *
+br_vlan_group_rtnl(const struct net_device *br_dev);
+
+struct net_bridge_vlan_group *
+br_port_vlan_group_rtnl(const struct net_device *dev);
+
+u16 br_vlan_group_pvid(const struct net_bridge_vlan_group *vg);
+
+struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid);
+
+u16 br_vlan_flags(const struct net_bridge_vlan *v);
+
#else
static inline bool br_vlan_enabled(const struct net_device *dev)
{
return false;
}
+
+static inline struct net_bridge_vlan_group *
+br_vlan_group_rtnl(const struct net_device *br_dev)
+{
+ return NULL;
+}
+
+static inline struct net_bridge_vlan_group *
+br_port_vlan_group_rtnl(const struct net_device *dev)
+{
+ return NULL;
+}
+
+static inline u16 br_vlan_group_pvid(const struct net_bridge_vlan_group *vg)
+{
+ return 0;
+}
+
+static inline struct net_bridge_vlan *
+br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid)
+{
+ return NULL;
+}
+
+static inline u16 br_vlan_flags(const struct net_bridge_vlan *v)
+{
+ return 0;
+}
+#endif
+
+#if IS_ENABLED(CONFIG_BRIDGE)
+struct net_device *br_fdb_find_port_hold(const struct net_device *br_dev,
+ const unsigned char *addr,
+ __u16 vid);
+#else
+static inline struct net_device *
+br_fdb_find_port_hold(const struct net_device *br_dev,
+ const unsigned char *addr,
+ __u16 vid)
+{
+ return NULL;
+}
#endif
#endif
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index d9e69e4514be..cbdcf0e95224 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -121,6 +121,31 @@ static struct net_bridge_fdb_entry *br_fdb_find(struct net_bridge *br,
return fdb;
}
+struct net_device *br_fdb_find_port_hold(const struct net_device *br_dev,
+ const unsigned char *addr,
+ __u16 vid)
+{
+ struct net_bridge_fdb_entry *f;
+ struct net_device *dev = NULL;
+ struct net_bridge *br;
+
+ if (!netif_is_bridge_master(br_dev))
+ return NULL;
+
+ br = netdev_priv(br_dev);
+
+ spin_lock_bh(&br->hash_lock);
+ f = br_fdb_find(br, addr, vid);
+ if (f && f->dst) {
+ dev = f->dst->dev;
+ dev_hold(dev);
+ }
+ spin_unlock_bh(&br->hash_lock);
+
+ return dev;
+}
+EXPORT_SYMBOL_GPL(br_fdb_find_port_hold);
+
struct net_bridge_fdb_entry *br_fdb_find_rcu(struct net_bridge *br,
const unsigned char *addr,
__u16 vid)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index a7cb3ece5031..3c929d587171 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -594,11 +594,22 @@ static inline bool br_rx_handler_check_rcu(const struct net_device *dev)
return rcu_dereference(dev->rx_handler) == br_handle_frame;
}
+static inline bool br_rx_handler_check_rtnl(const struct net_device *dev)
+{
+ return rcu_dereference_rtnl(dev->rx_handler) == br_handle_frame;
+}
+
static inline struct net_bridge_port *br_port_get_check_rcu(const struct net_device *dev)
{
return br_rx_handler_check_rcu(dev) ? br_port_get_rcu(dev) : NULL;
}
+static inline struct net_bridge_port *
+br_port_get_check_rtnl(const struct net_device *dev)
+{
+ return br_rx_handler_check_rtnl(dev) ? br_port_get_rtnl_rcu(dev) : NULL;
+}
+
/* br_ioctl.c */
int br_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
int br_ioctl_deviceless_stub(struct net *net, unsigned int cmd,
@@ -955,12 +966,6 @@ static inline void nbp_vlan_flush(struct net_bridge_port *port)
{
}
-static inline struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg,
- u16 vid)
-{
- return NULL;
-}
-
static inline int nbp_vlan_init(struct net_bridge_port *port)
{
return 0;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 9896f4975353..1c118c190653 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -671,6 +671,7 @@ struct net_bridge_vlan *br_vlan_find(struct net_bridge_vlan_group *vg, u16 vid)
return br_vlan_lookup(&vg->vlan_hash, vid);
}
+EXPORT_SYMBOL_GPL(br_vlan_find);
/* Must be protected by RTNL. */
static void recalculate_group_addr(struct net_bridge *br)
@@ -1149,3 +1150,34 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,
stats->tx_packets += txpackets;
}
}
+
+struct net_bridge_vlan_group *
+br_vlan_group_rtnl(const struct net_device *br_dev)
+{
+ if (netif_is_bridge_master(br_dev))
+ return br_vlan_group(netdev_priv(br_dev));
+ else
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(br_vlan_group_rtnl);
+
+struct net_bridge_vlan_group *
+br_port_vlan_group_rtnl(const struct net_device *dev)
+{
+ struct net_bridge_port *p = br_port_get_check_rtnl(dev);
+
+ return p ? nbp_vlan_group(p) : NULL;
+}
+EXPORT_SYMBOL_GPL(br_port_vlan_group_rtnl);
+
+u16 br_vlan_group_pvid(const struct net_bridge_vlan_group *vg)
+{
+ return br_get_pvid(vg);
+}
+EXPORT_SYMBOL_GPL(br_vlan_group_pvid);
+
+u16 br_vlan_flags(const struct net_bridge_vlan *v)
+{
+ return v->flags;
+}
+EXPORT_SYMBOL_GPL(br_vlan_flags);
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 2/6] mlxsw: spectrum: Extract mlxsw_sp_stp_spms_state()
2018-04-26 9:06 [PATCH net-next 0/6] mlxsw: SPAN: Support routes pointing at bridges Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 1/6] net: bridge: Publish bridge accessor functions Ido Schimmel
@ 2018-04-26 9:06 ` Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 3/6] mlxsw: spectrum_switchdev: Publish two functions Ido Schimmel
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2018-04-26 9:06 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, stephen, jiri, nikolay, petrm, mlxsw, Ido Schimmel
From: Petr Machata <petrm@mellanox.com>
Instead of duplicating the decision regarding port forwarding state made
by mlxsw_sp_port_vid_stp_set(), extract the decision-making into a new
function and reuse.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 26 +++++++++++++-------------
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 1 +
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index ca38a30fbe91..7317fb8079d1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -441,29 +441,29 @@ static void mlxsw_sp_txhdr_construct(struct sk_buff *skb,
mlxsw_tx_hdr_type_set(txhdr, MLXSW_TXHDR_TYPE_CONTROL);
}
-int mlxsw_sp_port_vid_stp_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 vid,
- u8 state)
+enum mlxsw_reg_spms_state mlxsw_sp_stp_spms_state(u8 state)
{
- struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
- enum mlxsw_reg_spms_state spms_state;
- char *spms_pl;
- int err;
-
switch (state) {
case BR_STATE_FORWARDING:
- spms_state = MLXSW_REG_SPMS_STATE_FORWARDING;
- break;
+ return MLXSW_REG_SPMS_STATE_FORWARDING;
case BR_STATE_LEARNING:
- spms_state = MLXSW_REG_SPMS_STATE_LEARNING;
- break;
+ return MLXSW_REG_SPMS_STATE_LEARNING;
case BR_STATE_LISTENING: /* fall-through */
case BR_STATE_DISABLED: /* fall-through */
case BR_STATE_BLOCKING:
- spms_state = MLXSW_REG_SPMS_STATE_DISCARDING;
- break;
+ return MLXSW_REG_SPMS_STATE_DISCARDING;
default:
BUG();
}
+}
+
+int mlxsw_sp_port_vid_stp_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 vid,
+ u8 state)
+{
+ enum mlxsw_reg_spms_state spms_state = mlxsw_sp_stp_spms_state(state);
+ struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+ char *spms_pl;
+ int err;
spms_pl = kmalloc(MLXSW_REG_SPMS_LEN, GFP_KERNEL);
if (!spms_pl)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 804d4d2c8031..4a519d8edec8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -364,6 +364,7 @@ int __mlxsw_sp_port_headroom_set(struct mlxsw_sp_port *mlxsw_sp_port, int mtu,
int mlxsw_sp_port_ets_maxrate_set(struct mlxsw_sp_port *mlxsw_sp_port,
enum mlxsw_reg_qeec_hr hr, u8 index,
u8 next_index, u32 maxrate);
+enum mlxsw_reg_spms_state mlxsw_sp_stp_spms_state(u8 stp_state);
int mlxsw_sp_port_vid_stp_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 vid,
u8 state);
int mlxsw_sp_port_vp_mode_set(struct mlxsw_sp_port *mlxsw_sp_port, bool enable);
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 3/6] mlxsw: spectrum_switchdev: Publish two functions
2018-04-26 9:06 [PATCH net-next 0/6] mlxsw: SPAN: Support routes pointing at bridges Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 1/6] net: bridge: Publish bridge accessor functions Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 2/6] mlxsw: spectrum: Extract mlxsw_sp_stp_spms_state() Ido Schimmel
@ 2018-04-26 9:06 ` Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 4/6] mlxsw: spectrum: Register SPAN before switchdev Ido Schimmel
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2018-04-26 9:06 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, stephen, jiri, nikolay, petrm, mlxsw, Ido Schimmel
From: Petr Machata <petrm@mellanox.com>
Publish the existing function mlxsw_sp_bridge_port_find(), and add
another service accessor mlxsw_sp_bridge_port_stp_state(). Publish both
in a new file spectrum_switchdev.h.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 ++++-
.../ethernet/mellanox/mlxsw/spectrum_switchdev.h | 43 ++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.h
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c11c9a635866..db4aea0f8996 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -50,6 +50,7 @@
#include <net/switchdev.h>
#include "spectrum_router.h"
+#include "spectrum_switchdev.h"
#include "spectrum.h"
#include "core.h"
#include "reg.h"
@@ -239,7 +240,7 @@ __mlxsw_sp_bridge_port_find(const struct mlxsw_sp_bridge_device *bridge_device,
return NULL;
}
-static struct mlxsw_sp_bridge_port *
+struct mlxsw_sp_bridge_port *
mlxsw_sp_bridge_port_find(struct mlxsw_sp_bridge *bridge,
struct net_device *brport_dev)
{
@@ -2297,6 +2298,12 @@ static struct notifier_block mlxsw_sp_switchdev_notifier = {
.notifier_call = mlxsw_sp_switchdev_event,
};
+u8
+mlxsw_sp_bridge_port_stp_state(struct mlxsw_sp_bridge_port *bridge_port)
+{
+ return bridge_port->stp_state;
+}
+
static int mlxsw_sp_fdb_init(struct mlxsw_sp *mlxsw_sp)
{
struct mlxsw_sp_bridge *bridge = mlxsw_sp->bridge;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.h
new file mode 100644
index 000000000000..bc44d5effc28
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+ * drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.h
+ * Copyright (c) 2018 Mellanox Technologies. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/netdevice.h>
+
+struct mlxsw_sp_bridge;
+struct mlxsw_sp_bridge_port;
+
+struct mlxsw_sp_bridge_port *
+mlxsw_sp_bridge_port_find(struct mlxsw_sp_bridge *bridge,
+ struct net_device *brport_dev);
+
+u8 mlxsw_sp_bridge_port_stp_state(struct mlxsw_sp_bridge_port *bridge_port);
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 4/6] mlxsw: spectrum: Register SPAN before switchdev
2018-04-26 9:06 [PATCH net-next 0/6] mlxsw: SPAN: Support routes pointing at bridges Ido Schimmel
` (2 preceding siblings ...)
2018-04-26 9:06 ` [PATCH net-next 3/6] mlxsw: spectrum_switchdev: Publish two functions Ido Schimmel
@ 2018-04-26 9:06 ` Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 5/6] mlxsw: Respin SPAN on switchdev events Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror Ido Schimmel
5 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2018-04-26 9:06 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, stephen, jiri, nikolay, petrm, mlxsw, Ido Schimmel
From: Petr Machata <petrm@mellanox.com>
Since switchdev events can trigger SPAN respin, it is necessary that the
data structures are available. Register SPAN first, with a commentary on
what the dependencies are.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 7317fb8079d1..94132f6cec61 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3666,6 +3666,15 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
goto err_lag_init;
}
+ /* Initialize SPAN before router and switchdev, so that those components
+ * can call mlxsw_sp_span_respin().
+ */
+ err = mlxsw_sp_span_init(mlxsw_sp);
+ if (err) {
+ dev_err(mlxsw_sp->bus_info->dev, "Failed to init span system\n");
+ goto err_span_init;
+ }
+
err = mlxsw_sp_switchdev_init(mlxsw_sp);
if (err) {
dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize switchdev\n");
@@ -3684,15 +3693,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
goto err_afa_init;
}
- err = mlxsw_sp_span_init(mlxsw_sp);
- if (err) {
- dev_err(mlxsw_sp->bus_info->dev, "Failed to init span system\n");
- goto err_span_init;
- }
-
- /* Initialize router after SPAN is initialized, so that the FIB and
- * neighbor event handlers can issue SPAN respin.
- */
err = mlxsw_sp_router_init(mlxsw_sp);
if (err) {
dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize router\n");
@@ -3739,14 +3739,14 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
err_netdev_notifier:
mlxsw_sp_router_fini(mlxsw_sp);
err_router_init:
- mlxsw_sp_span_fini(mlxsw_sp);
-err_span_init:
mlxsw_sp_afa_fini(mlxsw_sp);
err_afa_init:
mlxsw_sp_counter_pool_fini(mlxsw_sp);
err_counter_pool_init:
mlxsw_sp_switchdev_fini(mlxsw_sp);
err_switchdev_init:
+ mlxsw_sp_span_fini(mlxsw_sp);
+err_span_init:
mlxsw_sp_lag_fini(mlxsw_sp);
err_lag_init:
mlxsw_sp_buffers_fini(mlxsw_sp);
@@ -3768,10 +3768,10 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
mlxsw_sp_acl_fini(mlxsw_sp);
unregister_netdevice_notifier(&mlxsw_sp->netdevice_nb);
mlxsw_sp_router_fini(mlxsw_sp);
- mlxsw_sp_span_fini(mlxsw_sp);
mlxsw_sp_afa_fini(mlxsw_sp);
mlxsw_sp_counter_pool_fini(mlxsw_sp);
mlxsw_sp_switchdev_fini(mlxsw_sp);
+ mlxsw_sp_span_fini(mlxsw_sp);
mlxsw_sp_lag_fini(mlxsw_sp);
mlxsw_sp_buffers_fini(mlxsw_sp);
mlxsw_sp_traps_fini(mlxsw_sp);
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 5/6] mlxsw: Respin SPAN on switchdev events
2018-04-26 9:06 [PATCH net-next 0/6] mlxsw: SPAN: Support routes pointing at bridges Ido Schimmel
` (3 preceding siblings ...)
2018-04-26 9:06 ` [PATCH net-next 4/6] mlxsw: spectrum: Register SPAN before switchdev Ido Schimmel
@ 2018-04-26 9:06 ` Ido Schimmel
2018-04-26 9:06 ` [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror Ido Schimmel
5 siblings, 0 replies; 10+ messages in thread
From: Ido Schimmel @ 2018-04-26 9:06 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, stephen, jiri, nikolay, petrm, mlxsw, Ido Schimmel
From: Petr Machata <petrm@mellanox.com>
Changes to switchdev artifact can make a SPAN entry offloadable or
unoffloadable. To that end:
- Listen to SWITCHDEV_FDB_*_TO_BRIDGE notifications in addition to
the *_TO_DEVICE ones, to catch whatever activity is sent to the
bridge (likely by mlxsw itself).
On each FDB notification, respin SPAN to reconcile it with the FDB
changes.
- Also respin on switchdev port attribute changes (which currently
covers changes to STP state of ports) and port object additions and
removals.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 63 ++++++++++++++++++++--
1 file changed, 59 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index db4aea0f8996..1af99fe5fd32 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -49,6 +49,7 @@
#include <linux/netlink.h>
#include <net/switchdev.h>
+#include "spectrum_span.h"
#include "spectrum_router.h"
#include "spectrum_switchdev.h"
#include "spectrum.h"
@@ -923,6 +924,9 @@ static int mlxsw_sp_port_attr_set(struct net_device *dev,
break;
}
+ if (switchdev_trans_ph_commit(trans))
+ mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
+
return err;
}
@@ -1647,18 +1651,57 @@ mlxsw_sp_port_mrouter_update_mdb(struct mlxsw_sp_port *mlxsw_sp_port,
}
}
+struct mlxsw_sp_span_respin_work {
+ struct work_struct work;
+ struct mlxsw_sp *mlxsw_sp;
+};
+
+static void mlxsw_sp_span_respin_work(struct work_struct *work)
+{
+ struct mlxsw_sp_span_respin_work *respin_work =
+ container_of(work, struct mlxsw_sp_span_respin_work, work);
+
+ rtnl_lock();
+ mlxsw_sp_span_respin(respin_work->mlxsw_sp);
+ rtnl_unlock();
+ kfree(respin_work);
+}
+
+static void mlxsw_sp_span_respin_schedule(struct mlxsw_sp *mlxsw_sp)
+{
+ struct mlxsw_sp_span_respin_work *respin_work;
+
+ respin_work = kzalloc(sizeof(*respin_work), GFP_ATOMIC);
+ if (!respin_work)
+ return;
+
+ INIT_WORK(&respin_work->work, mlxsw_sp_span_respin_work);
+ respin_work->mlxsw_sp = mlxsw_sp;
+
+ mlxsw_core_schedule_work(&respin_work->work);
+}
+
static int mlxsw_sp_port_obj_add(struct net_device *dev,
const struct switchdev_obj *obj,
struct switchdev_trans *trans)
{
struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
+ const struct switchdev_obj_port_vlan *vlan;
int err = 0;
switch (obj->id) {
case SWITCHDEV_OBJ_ID_PORT_VLAN:
- err = mlxsw_sp_port_vlans_add(mlxsw_sp_port,
- SWITCHDEV_OBJ_PORT_VLAN(obj),
- trans);
+ vlan = SWITCHDEV_OBJ_PORT_VLAN(obj);
+ err = mlxsw_sp_port_vlans_add(mlxsw_sp_port, vlan, trans);
+
+ if (switchdev_trans_ph_commit(trans)) {
+ /* The event is emitted before the changes are actually
+ * applied to the bridge. Therefore schedule the respin
+ * call for later, so that the respin logic sees the
+ * updated bridge state.
+ */
+ mlxsw_sp_span_respin_schedule(mlxsw_sp_port->mlxsw_sp);
+ }
break;
case SWITCHDEV_OBJ_ID_PORT_MDB:
err = mlxsw_sp_port_mdb_add(mlxsw_sp_port,
@@ -1809,6 +1852,8 @@ static int mlxsw_sp_port_obj_del(struct net_device *dev,
break;
}
+ mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
+
return err;
}
@@ -2236,8 +2281,16 @@ static void mlxsw_sp_switchdev_event_work(struct work_struct *work)
fdb_info = &switchdev_work->fdb_info;
mlxsw_sp_port_fdb_set(mlxsw_sp_port, fdb_info, false);
break;
+ case SWITCHDEV_FDB_ADD_TO_BRIDGE: /* fall through */
+ case SWITCHDEV_FDB_DEL_TO_BRIDGE:
+ /* These events are only used to potentially update an existing
+ * SPAN mirror.
+ */
+ break;
}
+ mlxsw_sp_span_respin(mlxsw_sp_port->mlxsw_sp);
+
out:
rtnl_unlock();
kfree(switchdev_work->fdb_info.addr);
@@ -2266,7 +2319,9 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
switch (event) {
case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */
- case SWITCHDEV_FDB_DEL_TO_DEVICE:
+ case SWITCHDEV_FDB_DEL_TO_DEVICE: /* fall through */
+ case SWITCHDEV_FDB_ADD_TO_BRIDGE: /* fall through */
+ case SWITCHDEV_FDB_DEL_TO_BRIDGE:
memcpy(&switchdev_work->fdb_info, ptr,
sizeof(switchdev_work->fdb_info));
switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN, GFP_ATOMIC);
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror
2018-04-26 9:06 [PATCH net-next 0/6] mlxsw: SPAN: Support routes pointing at bridges Ido Schimmel
` (4 preceding siblings ...)
2018-04-26 9:06 ` [PATCH net-next 5/6] mlxsw: Respin SPAN on switchdev events Ido Schimmel
@ 2018-04-26 9:06 ` Ido Schimmel
2018-04-26 10:50 ` Nikolay Aleksandrov
5 siblings, 1 reply; 10+ messages in thread
From: Ido Schimmel @ 2018-04-26 9:06 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, stephen, jiri, nikolay, petrm, mlxsw, Ido Schimmel
From: Petr Machata <petrm@mellanox.com>
When handling mirroring to a gretap or ip6gretap netdevice in mlxsw, the
underlay address (i.e. the remote address of the tunnel) may be routed
to a bridge.
In that case, look up the resolved neighbor Ethernet address in that
bridge's FDB. Then configure the offload to direct the mirrored traffic
to that port, possibly with tagging.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_span.c | 102 +++++++++++++++++++--
.../net/ethernet/mellanox/mlxsw/spectrum_span.h | 1 +
2 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index 65a77708ff61..92fb81839852 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -32,6 +32,7 @@
* POSSIBILITY OF SUCH DAMAGE.
*/
+#include <linux/if_bridge.h>
#include <linux/list.h>
#include <net/arp.h>
#include <net/gre.h>
@@ -39,8 +40,9 @@
#include <net/ip6_tunnel.h>
#include "spectrum.h"
-#include "spectrum_span.h"
#include "spectrum_ipip.h"
+#include "spectrum_span.h"
+#include "spectrum_switchdev.h"
int mlxsw_sp_span_init(struct mlxsw_sp *mlxsw_sp)
{
@@ -167,6 +169,79 @@ mlxsw_sp_span_entry_unoffloadable(struct mlxsw_sp_span_parms *sparmsp)
return 0;
}
+static struct net_device *
+mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev,
+ unsigned char *dmac,
+ u16 *p_vid)
+{
+ struct net_bridge_vlan_group *vg = br_vlan_group_rtnl(br_dev);
+ u16 pvid = br_vlan_group_pvid(vg);
+ struct net_device *edev = NULL;
+ struct net_bridge_vlan *v;
+
+ if (pvid)
+ edev = br_fdb_find_port_hold(br_dev, dmac, pvid);
+ if (!edev)
+ return NULL;
+
+ /* RTNL prevents edev from being removed. */
+ dev_put(edev);
+
+ vg = br_port_vlan_group_rtnl(edev);
+ v = br_vlan_find(vg, pvid);
+ if (!v)
+ return NULL;
+ if (!(br_vlan_flags(v) & BRIDGE_VLAN_INFO_UNTAGGED))
+ *p_vid = pvid;
+ return edev;
+}
+
+static struct net_device *
+mlxsw_sp_span_entry_bridge_8021d(const struct net_device *br_dev,
+ unsigned char *dmac)
+{
+ struct net_device *edev = br_fdb_find_port_hold(br_dev, dmac, 0);
+
+ /* RTNL prevents edev from being removed. */
+ dev_put(edev);
+
+ return edev;
+}
+
+static struct net_device *
+mlxsw_sp_span_entry_bridge(const struct net_device *br_dev,
+ unsigned char dmac[ETH_ALEN],
+ u16 *p_vid)
+{
+ struct mlxsw_sp_bridge_port *bridge_port;
+ enum mlxsw_reg_spms_state spms_state;
+ struct mlxsw_sp_port *port;
+ struct net_device *dev;
+ u8 stp_state;
+
+ if (br_vlan_enabled(br_dev))
+ dev = mlxsw_sp_span_entry_bridge_8021q(br_dev, dmac, p_vid);
+ else
+ dev = mlxsw_sp_span_entry_bridge_8021d(br_dev, dmac);
+ if (!dev)
+ return NULL;
+
+ port = mlxsw_sp_port_dev_lower_find(dev);
+ if (!port)
+ return NULL;
+
+ bridge_port = mlxsw_sp_bridge_port_find(port->mlxsw_sp->bridge, dev);
+ if (!bridge_port)
+ return NULL;
+
+ stp_state = mlxsw_sp_bridge_port_stp_state(bridge_port);
+ spms_state = mlxsw_sp_stp_spms_state(stp_state);
+ if (spms_state != MLXSW_REG_SPMS_STATE_FORWARDING)
+ return NULL;
+
+ return dev;
+}
+
static __maybe_unused int
mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
union mlxsw_sp_l3addr saddr,
@@ -177,13 +252,22 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
struct mlxsw_sp_span_parms *sparmsp)
{
unsigned char dmac[ETH_ALEN];
+ u16 vid = 0;
if (mlxsw_sp_l3addr_is_zero(gw))
gw = daddr;
- if (!l3edev || !mlxsw_sp_port_dev_check(l3edev) ||
- mlxsw_sp_span_dmac(tbl, &gw, l3edev, dmac))
- return mlxsw_sp_span_entry_unoffloadable(sparmsp);
+ if (!l3edev || mlxsw_sp_span_dmac(tbl, &gw, l3edev, dmac))
+ goto unoffloadable;
+
+ if (netif_is_bridge_master(l3edev)) {
+ l3edev = mlxsw_sp_span_entry_bridge(l3edev, dmac, &vid);
+ if (!l3edev)
+ goto unoffloadable;
+ }
+
+ if (!mlxsw_sp_port_dev_check(l3edev))
+ goto unoffloadable;
sparmsp->dest_port = netdev_priv(l3edev);
sparmsp->ttl = ttl;
@@ -191,7 +275,11 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
memcpy(sparmsp->smac, l3edev->dev_addr, ETH_ALEN);
sparmsp->saddr = saddr;
sparmsp->daddr = daddr;
+ sparmsp->vid = vid;
return 0;
+
+unoffloadable:
+ return mlxsw_sp_span_entry_unoffloadable(sparmsp);
}
#if IS_ENABLED(CONFIG_NET_IPGRE)
@@ -268,9 +356,10 @@ mlxsw_sp_span_entry_gretap4_configure(struct mlxsw_sp_span_entry *span_entry,
/* Create a new port analayzer entry for local_port. */
mlxsw_reg_mpat_pack(mpat_pl, pa_id, local_port, true,
MLXSW_REG_MPAT_SPAN_TYPE_REMOTE_ETH_L3);
+ mlxsw_reg_mpat_eth_rspan_pack(mpat_pl, sparms.vid);
mlxsw_reg_mpat_eth_rspan_l2_pack(mpat_pl,
MLXSW_REG_MPAT_ETH_RSPAN_VERSION_NO_HEADER,
- sparms.dmac, false);
+ sparms.dmac, !!sparms.vid);
mlxsw_reg_mpat_eth_rspan_l3_ipv4_pack(mpat_pl,
sparms.ttl, sparms.smac,
be32_to_cpu(sparms.saddr.addr4),
@@ -368,9 +457,10 @@ mlxsw_sp_span_entry_gretap6_configure(struct mlxsw_sp_span_entry *span_entry,
/* Create a new port analayzer entry for local_port. */
mlxsw_reg_mpat_pack(mpat_pl, pa_id, local_port, true,
MLXSW_REG_MPAT_SPAN_TYPE_REMOTE_ETH_L3);
+ mlxsw_reg_mpat_eth_rspan_pack(mpat_pl, sparms.vid);
mlxsw_reg_mpat_eth_rspan_l2_pack(mpat_pl,
MLXSW_REG_MPAT_ETH_RSPAN_VERSION_NO_HEADER,
- sparms.dmac, false);
+ sparms.dmac, !!sparms.vid);
mlxsw_reg_mpat_eth_rspan_l3_ipv6_pack(mpat_pl, sparms.ttl, sparms.smac,
sparms.saddr.addr6,
sparms.daddr.addr6);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h
index 4b87ec20e658..14a6de904db1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h
@@ -63,6 +63,7 @@ struct mlxsw_sp_span_parms {
unsigned char smac[ETH_ALEN];
union mlxsw_sp_l3addr daddr;
union mlxsw_sp_l3addr saddr;
+ u16 vid;
};
struct mlxsw_sp_span_entry_ops;
--
2.14.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror
2018-04-26 9:06 ` [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror Ido Schimmel
@ 2018-04-26 10:50 ` Nikolay Aleksandrov
2018-04-26 13:03 ` Petr Machata
2018-04-26 15:58 ` Stephen Hemminger
0 siblings, 2 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2018-04-26 10:50 UTC (permalink / raw)
To: Ido Schimmel, netdev, bridge; +Cc: davem, stephen, jiri, petrm, mlxsw
On 26/04/18 12:06, Ido Schimmel wrote:
> From: Petr Machata <petrm@mellanox.com>
>
> When handling mirroring to a gretap or ip6gretap netdevice in mlxsw, the
> underlay address (i.e. the remote address of the tunnel) may be routed
> to a bridge.
>
> In that case, look up the resolved neighbor Ethernet address in that
> bridge's FDB. Then configure the offload to direct the mirrored traffic
> to that port, possibly with tagging.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlxsw/spectrum_span.c | 102 +++++++++++++++++++--
> .../net/ethernet/mellanox/mlxsw/spectrum_span.h | 1 +
> 2 files changed, 97 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
> index 65a77708ff61..92fb81839852 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
> @@ -32,6 +32,7 @@
> * POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#include <linux/if_bridge.h>
> #include <linux/list.h>
> #include <net/arp.h>
> #include <net/gre.h>
> @@ -39,8 +40,9 @@
> #include <net/ip6_tunnel.h>
>
> #include "spectrum.h"
> -#include "spectrum_span.h"
> #include "spectrum_ipip.h"
> +#include "spectrum_span.h"
> +#include "spectrum_switchdev.h"
>
> int mlxsw_sp_span_init(struct mlxsw_sp *mlxsw_sp)
> {
> @@ -167,6 +169,79 @@ mlxsw_sp_span_entry_unoffloadable(struct mlxsw_sp_span_parms *sparmsp)
> return 0;
> }
>
> +static struct net_device *
> +mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev,
> + unsigned char *dmac,
> + u16 *p_vid)
> +{
> + struct net_bridge_vlan_group *vg = br_vlan_group_rtnl(br_dev);
> + u16 pvid = br_vlan_group_pvid(vg);
> + struct net_device *edev = NULL;
> + struct net_bridge_vlan *v;
> +
Hi,
These structures are not really exported anywhere outside of br_private.h
Instead of passing them around and risking someone else actually trying to
dereference an incomplete type, why don't you add just 2 new helpers -
br_vlan_pvid(netdevice) and br_vlan_info(netdevice, vid, *bridge_vlan_info)
br_vlan_info can return the exported and already available type "bridge_vlan_info"
(defined in uapi/linux/if_bridge.h) into the bridge_vlan_info arg
and br_vlan_pvid is obvious - return the current dev pvid if available.
Another bonus you'll avoid dealing with the bridge's vlan internals.
> + if (pvid)
> + edev = br_fdb_find_port_hold(br_dev, dmac, pvid);
> + if (!edev)
> + return NULL;
> +
> + /* RTNL prevents edev from being removed. */
> + dev_put(edev);
Minor point here - since this is always called in rtnl, the dev_hold/put dance isn't needed,
unless you plan to use the fdb_find_port_hold outside of rtnl of course. In case that is
not planned why don't you do br_fdb_find_port_rtnl() instead with RCU inside (thus not blocking
learning) and ASSERT_RTNL() to avoid some complexity ?
Thanks,
Nik
> +
> + vg = br_port_vlan_group_rtnl(edev);
> + v = br_vlan_find(vg, pvid);
> + if (!v)
> + return NULL;
> + if (!(br_vlan_flags(v) & BRIDGE_VLAN_INFO_UNTAGGED))
> + *p_vid = pvid;
> + return edev;
> +}
> +
> +static struct net_device *
> +mlxsw_sp_span_entry_bridge_8021d(const struct net_device *br_dev,
> + unsigned char *dmac)
> +{
> + struct net_device *edev = br_fdb_find_port_hold(br_dev, dmac, 0);
> +
> + /* RTNL prevents edev from being removed. */
> + dev_put(edev);
> +
> + return edev;
> +}
> +
> +static struct net_device *
> +mlxsw_sp_span_entry_bridge(const struct net_device *br_dev,
> + unsigned char dmac[ETH_ALEN],
> + u16 *p_vid)
> +{
> + struct mlxsw_sp_bridge_port *bridge_port;
> + enum mlxsw_reg_spms_state spms_state;
> + struct mlxsw_sp_port *port;
> + struct net_device *dev;
> + u8 stp_state;
> +
> + if (br_vlan_enabled(br_dev))
> + dev = mlxsw_sp_span_entry_bridge_8021q(br_dev, dmac, p_vid);
> + else
> + dev = mlxsw_sp_span_entry_bridge_8021d(br_dev, dmac);
> + if (!dev)
> + return NULL;
> +
> + port = mlxsw_sp_port_dev_lower_find(dev);
> + if (!port)
> + return NULL;
> +
> + bridge_port = mlxsw_sp_bridge_port_find(port->mlxsw_sp->bridge, dev);
> + if (!bridge_port)
> + return NULL;
> +
> + stp_state = mlxsw_sp_bridge_port_stp_state(bridge_port);
> + spms_state = mlxsw_sp_stp_spms_state(stp_state);
> + if (spms_state != MLXSW_REG_SPMS_STATE_FORWARDING)
> + return NULL;
> +
> + return dev;
> +}
> +
> static __maybe_unused int
> mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
> union mlxsw_sp_l3addr saddr,
> @@ -177,13 +252,22 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
> struct mlxsw_sp_span_parms *sparmsp)
> {
> unsigned char dmac[ETH_ALEN];
> + u16 vid = 0;
>
> if (mlxsw_sp_l3addr_is_zero(gw))
> gw = daddr;
>
> - if (!l3edev || !mlxsw_sp_port_dev_check(l3edev) ||
> - mlxsw_sp_span_dmac(tbl, &gw, l3edev, dmac))
> - return mlxsw_sp_span_entry_unoffloadable(sparmsp);
> + if (!l3edev || mlxsw_sp_span_dmac(tbl, &gw, l3edev, dmac))
> + goto unoffloadable;
> +
> + if (netif_is_bridge_master(l3edev)) {
> + l3edev = mlxsw_sp_span_entry_bridge(l3edev, dmac, &vid);
> + if (!l3edev)
> + goto unoffloadable;
> + }
> +
> + if (!mlxsw_sp_port_dev_check(l3edev))
> + goto unoffloadable;
>
> sparmsp->dest_port = netdev_priv(l3edev);
> sparmsp->ttl = ttl;
> @@ -191,7 +275,11 @@ mlxsw_sp_span_entry_tunnel_parms_common(struct net_device *l3edev,
> memcpy(sparmsp->smac, l3edev->dev_addr, ETH_ALEN);
> sparmsp->saddr = saddr;
> sparmsp->daddr = daddr;
> + sparmsp->vid = vid;
> return 0;
> +
> +unoffloadable:
> + return mlxsw_sp_span_entry_unoffloadable(sparmsp);
> }
>
> #if IS_ENABLED(CONFIG_NET_IPGRE)
> @@ -268,9 +356,10 @@ mlxsw_sp_span_entry_gretap4_configure(struct mlxsw_sp_span_entry *span_entry,
> /* Create a new port analayzer entry for local_port. */
> mlxsw_reg_mpat_pack(mpat_pl, pa_id, local_port, true,
> MLXSW_REG_MPAT_SPAN_TYPE_REMOTE_ETH_L3);
> + mlxsw_reg_mpat_eth_rspan_pack(mpat_pl, sparms.vid);
> mlxsw_reg_mpat_eth_rspan_l2_pack(mpat_pl,
> MLXSW_REG_MPAT_ETH_RSPAN_VERSION_NO_HEADER,
> - sparms.dmac, false);
> + sparms.dmac, !!sparms.vid);
> mlxsw_reg_mpat_eth_rspan_l3_ipv4_pack(mpat_pl,
> sparms.ttl, sparms.smac,
> be32_to_cpu(sparms.saddr.addr4),
> @@ -368,9 +457,10 @@ mlxsw_sp_span_entry_gretap6_configure(struct mlxsw_sp_span_entry *span_entry,
> /* Create a new port analayzer entry for local_port. */
> mlxsw_reg_mpat_pack(mpat_pl, pa_id, local_port, true,
> MLXSW_REG_MPAT_SPAN_TYPE_REMOTE_ETH_L3);
> + mlxsw_reg_mpat_eth_rspan_pack(mpat_pl, sparms.vid);
> mlxsw_reg_mpat_eth_rspan_l2_pack(mpat_pl,
> MLXSW_REG_MPAT_ETH_RSPAN_VERSION_NO_HEADER,
> - sparms.dmac, false);
> + sparms.dmac, !!sparms.vid);
> mlxsw_reg_mpat_eth_rspan_l3_ipv6_pack(mpat_pl, sparms.ttl, sparms.smac,
> sparms.saddr.addr6,
> sparms.daddr.addr6);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h
> index 4b87ec20e658..14a6de904db1 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.h
> @@ -63,6 +63,7 @@ struct mlxsw_sp_span_parms {
> unsigned char smac[ETH_ALEN];
> union mlxsw_sp_l3addr daddr;
> union mlxsw_sp_l3addr saddr;
> + u16 vid;
> };
>
> struct mlxsw_sp_span_entry_ops;
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror
2018-04-26 10:50 ` Nikolay Aleksandrov
@ 2018-04-26 13:03 ` Petr Machata
2018-04-26 15:58 ` Stephen Hemminger
1 sibling, 0 replies; 10+ messages in thread
From: Petr Machata @ 2018-04-26 13:03 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Ido Schimmel, netdev, bridge, davem, stephen, jiri, mlxsw
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> writes:
> On 26/04/18 12:06, Ido Schimmel wrote:
>> From: Petr Machata <petrm@mellanox.com>
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
>> index 65a77708ff61..92fb81839852 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
>> @@ -167,6 +169,79 @@ mlxsw_sp_span_entry_unoffloadable(struct mlxsw_sp_span_parms *sparmsp)
>> return 0;
>> }
>> +static struct net_device *
>> +mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev,
>> + unsigned char *dmac,
>> + u16 *p_vid)
>> +{
>> + struct net_bridge_vlan_group *vg = br_vlan_group_rtnl(br_dev);
>> + u16 pvid = br_vlan_group_pvid(vg);
>> + struct net_device *edev = NULL;
>> + struct net_bridge_vlan *v;
>> +
>
> Hi,
> These structures are not really exported anywhere outside of br_private.h
> Instead of passing them around and risking someone else actually trying to
> dereference an incomplete type, why don't you add just 2 new helpers -
> br_vlan_pvid(netdevice) and br_vlan_info(netdevice, vid, *bridge_vlan_info)
>
> br_vlan_info can return the exported and already available type "bridge_vlan_info"
> (defined in uapi/linux/if_bridge.h) into the bridge_vlan_info arg
> and br_vlan_pvid is obvious - return the current dev pvid if available.
>
> Another bonus you'll avoid dealing with the bridge's vlan internals.
All right, I'll do it like that.
>
>> + if (pvid)
>> + edev = br_fdb_find_port_hold(br_dev, dmac, pvid);
>> + if (!edev)
>> + return NULL;
>> +
>> + /* RTNL prevents edev from being removed. */
>> + dev_put(edev);
>
> Minor point here - since this is always called in rtnl, the dev_hold/put dance isn't needed,
> unless you plan to use the fdb_find_port_hold outside of rtnl of course. In case that is
> not planned why don't you do br_fdb_find_port_rtnl() instead with RCU inside (thus not blocking
> learning) and ASSERT_RTNL() to avoid some complexity ?
OK, sounds good.
I'll spin a v2.
Thanks,
Petr
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next 6/6] mlxsw: spectrum_span: Allow bridge for gretap mirror
2018-04-26 10:50 ` Nikolay Aleksandrov
2018-04-26 13:03 ` Petr Machata
@ 2018-04-26 15:58 ` Stephen Hemminger
1 sibling, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2018-04-26 15:58 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Ido Schimmel, netdev, bridge, davem, jiri, petrm, mlxsw
On Thu, 26 Apr 2018 13:50:12 +0300
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> On 26/04/18 12:06, Ido Schimmel wrote:
> > From: Petr Machata <petrm@mellanox.com>
> >
> > When handling mirroring to a gretap or ip6gretap netdevice in mlxsw, the
> > underlay address (i.e. the remote address of the tunnel) may be routed
> > to a bridge.
> >
> > In that case, look up the resolved neighbor Ethernet address in that
> > bridge's FDB. Then configure the offload to direct the mirrored traffic
> > to that port, possibly with tagging.
> >
> > Signed-off-by: Petr Machata <petrm@mellanox.com>
> > Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> > ---
> > .../net/ethernet/mellanox/mlxsw/spectrum_span.c | 102 +++++++++++++++++++--
> > .../net/ethernet/mellanox/mlxsw/spectrum_span.h | 1 +
> > 2 files changed, 97 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
> > index 65a77708ff61..92fb81839852 100644
> > --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
> > +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
> > @@ -32,6 +32,7 @@
> > * POSSIBILITY OF SUCH DAMAGE.
> > */
> >
> > +#include <linux/if_bridge.h>
> > #include <linux/list.h>
> > #include <net/arp.h>
> > #include <net/gre.h>
> > @@ -39,8 +40,9 @@
> > #include <net/ip6_tunnel.h>
> >
> > #include "spectrum.h"
> > -#include "spectrum_span.h"
> > #include "spectrum_ipip.h"
> > +#include "spectrum_span.h"
> > +#include "spectrum_switchdev.h"
> >
> > int mlxsw_sp_span_init(struct mlxsw_sp *mlxsw_sp)
> > {
> > @@ -167,6 +169,79 @@ mlxsw_sp_span_entry_unoffloadable(struct mlxsw_sp_span_parms *sparmsp)
> > return 0;
> > }
> >
> > +static struct net_device *
> > +mlxsw_sp_span_entry_bridge_8021q(const struct net_device *br_dev,
> > + unsigned char *dmac,
> > + u16 *p_vid)
> > +{
> > + struct net_bridge_vlan_group *vg = br_vlan_group_rtnl(br_dev);
> > + u16 pvid = br_vlan_group_pvid(vg);
> > + struct net_device *edev = NULL;
> > + struct net_bridge_vlan *v;
> > +
>
> Hi,
> These structures are not really exported anywhere outside of br_private.h
> Instead of passing them around and risking someone else actually trying to
> dereference an incomplete type, why don't you add just 2 new helpers -
> br_vlan_pvid(netdevice) and br_vlan_info(netdevice, vid, *bridge_vlan_info)
>
> br_vlan_info can return the exported and already available type "bridge_vlan_info"
> (defined in uapi/linux/if_bridge.h) into the bridge_vlan_info arg
> and br_vlan_pvid is obvious - return the current dev pvid if available.
>
> Another bonus you'll avoid dealing with the bridge's vlan internals.
I am particularly worried that any locking changes will impact the device
driver. Also lock inversion issues, and implied (or not RTNL).
Also what if a value is changed, there is no notification path back to the device.
Having an API that only takes net_device and vlan seems preferable.
^ permalink raw reply [flat|nested] 10+ messages in thread