* [PATCH V4 05/10] net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function call
From: Marek Vasut @ 2019-06-25 23:43 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190625234348.16246-1-marex@denx.de>
The indirect function call to dev->dev_ops->get_port_addr() is expensive
especially if called for every single register access, and only returns
the value of PORT_CTRL_ADDR() macro. Use PORT_CTRL_ADDR() macro directly
instead.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
---
V2: New patch
V3: - Rebase on next/master
- Test on KSZ9477EVB
V4: No change
---
drivers/net/dsa/microchip/ksz9477.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index e8b96566abd9..7d209fd9f26f 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -83,7 +83,7 @@ static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
u32 addr;
u8 data;
- addr = dev->dev_ops->get_port_addr(port, offset);
+ addr = PORT_CTRL_ADDR(port, offset);
ksz_read8(dev, addr, &data);
if (set)
--
2.20.1
^ permalink raw reply related
* [PATCH V4 02/10] net: dsa: microchip: Remove ksz_{get,set}()
From: Marek Vasut @ 2019-06-25 23:43 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190625234348.16246-1-marex@denx.de>
These functions and callbacks are never used, remove them.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
V2: No change
V3: - Rebase on next/master
- Test on KSZ9477EVB
V4: Add RB
---
drivers/net/dsa/microchip/ksz9477_spi.c | 2 --
drivers/net/dsa/microchip/ksz_common.h | 24 ------------------------
drivers/net/dsa/microchip/ksz_priv.h | 2 --
drivers/net/dsa/microchip/ksz_spi.h | 10 ----------
4 files changed, 38 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index e7118319c192..86d12d48a2a9 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -80,8 +80,6 @@ static const struct ksz_io_ops ksz9477_spi_ops = {
.write8 = ksz_spi_write8,
.write16 = ksz_spi_write16,
.write32 = ksz_spi_write32,
- .get = ksz_spi_get,
- .set = ksz_spi_set,
};
static int ksz9477_spi_probe(struct spi_device *spi)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 1781539c3a81..c15b49528bad 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -105,30 +105,6 @@ static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
return ret;
}
-static inline int ksz_get(struct ksz_device *dev, u32 reg, void *data,
- size_t len)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->get(dev, reg, data, len);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
-static inline int ksz_set(struct ksz_device *dev, u32 reg, void *data,
- size_t len)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->set(dev, reg, data, len);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
u8 *data)
{
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 5ef6153bd2cc..d3ddf98156bb 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -109,8 +109,6 @@ struct ksz_io_ops {
int (*write8)(struct ksz_device *dev, u32 reg, u8 value);
int (*write16)(struct ksz_device *dev, u32 reg, u16 value);
int (*write32)(struct ksz_device *dev, u32 reg, u32 value);
- int (*get)(struct ksz_device *dev, u32 reg, void *data, size_t len);
- int (*set)(struct ksz_device *dev, u32 reg, void *data, size_t len);
};
struct alu_struct {
diff --git a/drivers/net/dsa/microchip/ksz_spi.h b/drivers/net/dsa/microchip/ksz_spi.h
index 427811bd60b3..976bace31f37 100644
--- a/drivers/net/dsa/microchip/ksz_spi.h
+++ b/drivers/net/dsa/microchip/ksz_spi.h
@@ -56,14 +56,4 @@ static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
return ksz_spi_write(dev, reg, &value, 4);
}
-static int ksz_spi_get(struct ksz_device *dev, u32 reg, void *data, size_t len)
-{
- return ksz_spi_read(dev, reg, data, len);
-}
-
-static int ksz_spi_set(struct ksz_device *dev, u32 reg, void *data, size_t len)
-{
- return ksz_spi_write(dev, reg, data, len);
-}
-
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH V4 01/10] net: dsa: microchip: Remove ksz_{read,write}24()
From: Marek Vasut @ 2019-06-25 23:43 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
In-Reply-To: <20190625234348.16246-1-marex@denx.de>
These functions and callbacks are never used, remove them.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
V2: No change
V3: - Rebase on next/master
- Test on KSZ9477EVB
V4: Add RB
---
drivers/net/dsa/microchip/ksz9477_spi.c | 25 -------------------------
drivers/net/dsa/microchip/ksz_common.h | 22 ----------------------
drivers/net/dsa/microchip/ksz_priv.h | 2 --
3 files changed, 49 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz9477_spi.c b/drivers/net/dsa/microchip/ksz9477_spi.c
index 75178624d3f5..e7118319c192 100644
--- a/drivers/net/dsa/microchip/ksz9477_spi.c
+++ b/drivers/net/dsa/microchip/ksz9477_spi.c
@@ -73,37 +73,12 @@ static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
return ksz9477_spi_write_reg(spi, reg, dev->txbuf, len);
}
-static int ksz_spi_read24(struct ksz_device *dev, u32 reg, u32 *val)
-{
- int ret;
-
- *val = 0;
- ret = ksz_spi_read(dev, reg, (u8 *)val, 3);
- if (!ret) {
- *val = be32_to_cpu(*val);
- /* convert to 24bit */
- *val >>= 8;
- }
-
- return ret;
-}
-
-static int ksz_spi_write24(struct ksz_device *dev, u32 reg, u32 value)
-{
- /* make it to big endian 24bit from MSB */
- value <<= 8;
- value = cpu_to_be32(value);
- return ksz_spi_write(dev, reg, &value, 3);
-}
-
static const struct ksz_io_ops ksz9477_spi_ops = {
.read8 = ksz_spi_read8,
.read16 = ksz_spi_read16,
- .read24 = ksz_spi_read24,
.read32 = ksz_spi_read32,
.write8 = ksz_spi_write8,
.write16 = ksz_spi_write16,
- .write24 = ksz_spi_write24,
.write32 = ksz_spi_write32,
.get = ksz_spi_get,
.set = ksz_spi_set,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 21cd794e18f1..1781539c3a81 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -61,17 +61,6 @@ static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
return ret;
}
-static inline int ksz_read24(struct ksz_device *dev, u32 reg, u32 *val)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->read24(dev, reg, val);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
static inline int ksz_read32(struct ksz_device *dev, u32 reg, u32 *val)
{
int ret;
@@ -105,17 +94,6 @@ static inline int ksz_write16(struct ksz_device *dev, u32 reg, u16 value)
return ret;
}
-static inline int ksz_write24(struct ksz_device *dev, u32 reg, u32 value)
-{
- int ret;
-
- mutex_lock(&dev->reg_mutex);
- ret = dev->ops->write24(dev, reg, value);
- mutex_unlock(&dev->reg_mutex);
-
- return ret;
-}
-
static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
{
int ret;
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index c615d2a81dd5..5ef6153bd2cc 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -105,11 +105,9 @@ struct ksz_device {
struct ksz_io_ops {
int (*read8)(struct ksz_device *dev, u32 reg, u8 *value);
int (*read16)(struct ksz_device *dev, u32 reg, u16 *value);
- int (*read24)(struct ksz_device *dev, u32 reg, u32 *value);
int (*read32)(struct ksz_device *dev, u32 reg, u32 *value);
int (*write8)(struct ksz_device *dev, u32 reg, u8 value);
int (*write16)(struct ksz_device *dev, u32 reg, u16 value);
- int (*write24)(struct ksz_device *dev, u32 reg, u32 value);
int (*write32)(struct ksz_device *dev, u32 reg, u32 value);
int (*get)(struct ksz_device *dev, u32 reg, void *data, size_t len);
int (*set)(struct ksz_device *dev, u32 reg, void *data, size_t len);
--
2.20.1
^ permalink raw reply related
* [PATCH V4 00/10] net: dsa: microchip: Convert to regmap
From: Marek Vasut @ 2019-06-25 23:43 UTC (permalink / raw)
To: netdev; +Cc: Marek Vasut, Andrew Lunn, Florian Fainelli, Tristram Ha,
Woojung Huh
This patchset converts KSZ9477 switch driver to regmap.
This was tested with extra patches on KSZ8795. This was also tested
on KSZ9477 on Microchip KSZ9477EVB board, which I now have.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Woojung Huh <Woojung.Huh@microchip.com>
Marek Vasut (10):
net: dsa: microchip: Remove ksz_{read,write}24()
net: dsa: microchip: Remove ksz_{get,set}()
net: dsa: microchip: Inline ksz_spi.h
net: dsa: microchip: Move ksz_cfg and ksz_port_cfg to ksz9477.c
net: dsa: microchip: Use PORT_CTRL_ADDR() instead of indirect function
call
net: dsa: microchip: Factor out register access opcode generation
net: dsa: microchip: Initial SPI regmap support
net: dsa: microchip: Dispose of ksz_io_ops
net: dsa: microchip: Factor out regmap config generation into common
header
net: dsa: microchip: Replace ad-hoc bit manipulation with regmap
drivers/net/dsa/microchip/Kconfig | 1 +
drivers/net/dsa/microchip/ksz9477.c | 35 +++---
drivers/net/dsa/microchip/ksz9477_spi.c | 114 +++--------------
drivers/net/dsa/microchip/ksz_common.c | 6 +-
drivers/net/dsa/microchip/ksz_common.h | 157 +++++++-----------------
drivers/net/dsa/microchip/ksz_priv.h | 23 +---
drivers/net/dsa/microchip/ksz_spi.h | 69 -----------
7 files changed, 84 insertions(+), 321 deletions(-)
delete mode 100644 drivers/net/dsa/microchip/ksz_spi.h
--
2.20.1
^ permalink raw reply
* Re: [PATCH net-next 11/18] ionic: Add Rx filter and rx_mode nod support
From: Jakub Kicinski @ 2019-06-25 23:44 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-12-snelson@pensando.io>
On Thu, 20 Jun 2019 13:24:17 -0700, Shannon Nelson wrote:
> Add the Rx filtering and rx_mode NDO callbacks. Also add
> the deferred work thread handling needed to manage the filter
> requests otuside of the netif_addr_lock spinlock.
>
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> static int ionic_vlan_rx_kill_vid(struct net_device *netdev, __be16 proto,
> u16 vid)
> {
> - netdev_info(netdev, "%s: stubbed\n", __func__);
> + struct lif *lif = netdev_priv(netdev);
> + struct ionic_admin_ctx ctx = {
> + .work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
> + .cmd.rx_filter_del = {
> + .opcode = CMD_OPCODE_RX_FILTER_DEL,
> + .lif_index = cpu_to_le16(lif->index),
> + },
> + };
> + struct rx_filter *f;
> + int err;
> +
> + spin_lock_bh(&lif->rx_filters.lock);
> +
> + f = ionic_rx_filter_by_vlan(lif, vid);
> + if (!f) {
> + spin_unlock_bh(&lif->rx_filters.lock);
> + return -ENOENT;
> + }
> +
> + netdev_dbg(netdev, "rx_filter del VLAN %d (id %d)\n", vid,
> + le32_to_cpu(ctx.cmd.rx_filter_del.filter_id));
> +
> + ctx.cmd.rx_filter_del.filter_id = cpu_to_le32(f->filter_id);
> + ionic_rx_filter_free(lif, f);
> + spin_unlock_bh(&lif->rx_filters.lock);
> +
> + err = ionic_adminq_post_wait(lif, &ctx);
> + if (err)
> + return err;
>
> return 0;
nit: return directly?
> }
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> index 8129fa20695a..c3ecf1df9c2c 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> @@ -60,6 +60,29 @@ struct qcq {
> #define napi_to_qcq(napi) container_of(napi, struct qcq, napi)
> #define napi_to_cq(napi) (&napi_to_qcq(napi)->cq)
>
> +enum deferred_work_type {
> + DW_TYPE_RX_MODE,
> + DW_TYPE_RX_ADDR_ADD,
> + DW_TYPE_RX_ADDR_DEL,
> + DW_TYPE_LINK_STATUS,
> + DW_TYPE_LIF_RESET,
> +};
> +
> +struct deferred_work {
If you don't mind prefixing these structures with ionic_ that'd be
great. I'm worried deferred_work is too close to delayed_work..
> + struct list_head list;
> + enum deferred_work_type type;
> + union {
> + unsigned int rx_mode;
> + u8 addr[ETH_ALEN];
> + };
> +};
> +
> +struct deferred {
> + spinlock_t lock; /* lock for deferred work list */
> + struct list_head list;
> + struct work_struct work;
> +};
^ permalink raw reply
* [PATCH net-next 01/10] net: dsa: sja1105: Build PTP support in main DSA driver
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
As Arnd Bergmann pointed out in commit 78fe8a28fb96 ("net: dsa: sja1105:
fix ptp link error"), there is no point in having PTP support as a
separate loadable kernel module.
So remove the exported symbols and make sja1105.ko contain PTP support
or not based on CONFIG_NET_DSA_SJA1105_PTP.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/Makefile | 2 +-
drivers/net/dsa/sja1105/sja1105_ptp.c | 12 ------------
drivers/net/dsa/sja1105/sja1105_spi.c | 2 --
drivers/net/dsa/sja1105/sja1105_static_config.c | 3 ---
4 files changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/dsa/sja1105/Makefile b/drivers/net/dsa/sja1105/Makefile
index 9a22f68b39e9..4483113e6259 100644
--- a/drivers/net/dsa/sja1105/Makefile
+++ b/drivers/net/dsa/sja1105/Makefile
@@ -10,5 +10,5 @@ sja1105-objs := \
sja1105_dynamic_config.o \
ifdef CONFIG_NET_DSA_SJA1105_PTP
-obj-$(CONFIG_NET_DSA_SJA1105) += sja1105_ptp.o
+sja1105-objs += sja1105_ptp.o
endif
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 3041cf9d5856..c7ce1edd8471 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -77,7 +77,6 @@ int sja1105_get_ts_info(struct dsa_switch *ds, int port,
info->phc_index = ptp_clock_index(priv->clock);
return 0;
}
-EXPORT_SYMBOL_GPL(sja1105_get_ts_info);
int sja1105et_ptp_cmd(const void *ctx, const void *data)
{
@@ -95,7 +94,6 @@ int sja1105et_ptp_cmd(const void *ctx, const void *data)
return sja1105_spi_send_packed_buf(priv, SPI_WRITE, regs->ptp_control,
buf, SJA1105_SIZE_PTP_CMD);
}
-EXPORT_SYMBOL_GPL(sja1105et_ptp_cmd);
int sja1105pqrs_ptp_cmd(const void *ctx, const void *data)
{
@@ -113,7 +111,6 @@ int sja1105pqrs_ptp_cmd(const void *ctx, const void *data)
return sja1105_spi_send_packed_buf(priv, SPI_WRITE, regs->ptp_control,
buf, SJA1105_SIZE_PTP_CMD);
}
-EXPORT_SYMBOL_GPL(sja1105pqrs_ptp_cmd);
/* The switch returns partial timestamps (24 bits for SJA1105 E/T, which wrap
* around in 0.135 seconds, and 32 bits for P/Q/R/S, wrapping around in 34.35
@@ -146,7 +143,6 @@ u64 sja1105_tstamp_reconstruct(struct sja1105_private *priv, u64 now,
return ts_reconstructed;
}
-EXPORT_SYMBOL_GPL(sja1105_tstamp_reconstruct);
/* Reads the SPI interface for an egress timestamp generated by the switch
* for frames sent using management routes.
@@ -219,7 +215,6 @@ int sja1105_ptpegr_ts_poll(struct sja1105_private *priv, int port, u64 *ts)
return 0;
}
-EXPORT_SYMBOL_GPL(sja1105_ptpegr_ts_poll);
int sja1105_ptp_reset(struct sja1105_private *priv)
{
@@ -240,7 +235,6 @@ int sja1105_ptp_reset(struct sja1105_private *priv)
return rc;
}
-EXPORT_SYMBOL_GPL(sja1105_ptp_reset);
static int sja1105_ptp_gettime(struct ptp_clock_info *ptp,
struct timespec64 *ts)
@@ -387,7 +381,6 @@ int sja1105_ptp_clock_register(struct sja1105_private *priv)
return sja1105_ptp_reset(priv);
}
-EXPORT_SYMBOL_GPL(sja1105_ptp_clock_register);
void sja1105_ptp_clock_unregister(struct sja1105_private *priv)
{
@@ -397,8 +390,3 @@ void sja1105_ptp_clock_unregister(struct sja1105_private *priv)
ptp_clock_unregister(priv->clock);
priv->clock = NULL;
}
-EXPORT_SYMBOL_GPL(sja1105_ptp_clock_unregister);
-
-MODULE_AUTHOR("Vladimir Oltean <olteanv@gmail.com>");
-MODULE_DESCRIPTION("SJA1105 PHC Driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/dsa/sja1105/sja1105_spi.c b/drivers/net/dsa/sja1105/sja1105_spi.c
index f7e51debb930..84dc603138cf 100644
--- a/drivers/net/dsa/sja1105/sja1105_spi.c
+++ b/drivers/net/dsa/sja1105/sja1105_spi.c
@@ -100,7 +100,6 @@ int sja1105_spi_send_packed_buf(const struct sja1105_private *priv,
return 0;
}
-EXPORT_SYMBOL_GPL(sja1105_spi_send_packed_buf);
/* If @rw is:
* - SPI_WRITE: creates and sends an SPI write message at absolute
@@ -136,7 +135,6 @@ int sja1105_spi_send_int(const struct sja1105_private *priv,
return rc;
}
-EXPORT_SYMBOL_GPL(sja1105_spi_send_int);
/* Should be used if a @packed_buf larger than SJA1105_SIZE_SPI_MSG_MAXLEN
* must be sent/received. Splitting the buffer into chunks and assembling
diff --git a/drivers/net/dsa/sja1105/sja1105_static_config.c b/drivers/net/dsa/sja1105/sja1105_static_config.c
index 58f273eaf1ea..242f001c59fe 100644
--- a/drivers/net/dsa/sja1105/sja1105_static_config.c
+++ b/drivers/net/dsa/sja1105/sja1105_static_config.c
@@ -35,7 +35,6 @@ void sja1105_pack(void *buf, const u64 *val, int start, int end, size_t len)
}
dump_stack();
}
-EXPORT_SYMBOL_GPL(sja1105_pack);
void sja1105_unpack(const void *buf, u64 *val, int start, int end, size_t len)
{
@@ -53,7 +52,6 @@ void sja1105_unpack(const void *buf, u64 *val, int start, int end, size_t len)
start, end);
dump_stack();
}
-EXPORT_SYMBOL_GPL(sja1105_unpack);
void sja1105_packing(void *buf, u64 *val, int start, int end,
size_t len, enum packing_op op)
@@ -76,7 +74,6 @@ void sja1105_packing(void *buf, u64 *val, int start, int end,
}
dump_stack();
}
-EXPORT_SYMBOL_GPL(sja1105_packing);
/* Little-endian Ethernet CRC32 of data packed as big-endian u32 words */
u32 sja1105_crc32(const void *buf, size_t len)
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 07/10] net: dsa: sja1105: Add a high-level overview of the dynamic config interface
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
When trying to add support for LOCKEDS (static FDB entries) on SJA1105
P/Q/R/S, at first I didn't remember how the abstraction I created
worked, and actually thought it works by mistake.
To avoid other people staring at the code and not making much sense out
of it, add some comments at the top of the file.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
.../net/dsa/sja1105/sja1105_dynamic_config.c | 92 +++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
index 56c83b9d52e4..3acd48615981 100644
--- a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
+++ b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
@@ -3,6 +3,98 @@
*/
#include "sja1105.h"
+/* In the dynamic configuration interface, the switch exposes a register-like
+ * view of some of the static configuration tables.
+ * Many times the field organization of the dynamic tables is abbreviated (not
+ * all fields are dynamically reconfigurable) and different from the static
+ * ones, but the key reason for having it is that we can spare a switch reset
+ * for settings that can be changed dynamically.
+ *
+ * This file creates a per-switch-family abstraction called
+ * struct sja1105_dynamic_table_ops and two operations that work with it:
+ * - sja1105_dynamic_config_write
+ * - sja1105_dynamic_config_read
+ *
+ * Compared to the struct sja1105_table_ops from sja1105_static_config.c,
+ * the dynamic accessors work with a compound buffer:
+ *
+ * packed_buf
+ *
+ * |
+ * V
+ * +-----------------------------------------+------------------+
+ * | ENTRY BUFFER | COMMAND BUFFER |
+ * +-----------------------------------------+------------------+
+ *
+ * <----------------------- packed_size ------------------------>
+ *
+ * The ENTRY BUFFER may or may not have the same layout, or size, as its static
+ * configuration table entry counterpart. When it does, the same packing
+ * function is reused (bar exceptional cases - see
+ * sja1105pqrs_dyn_l2_lookup_entry_packing).
+ *
+ * The reason for the COMMAND BUFFER being at the end is to be able to send
+ * a dynamic write command through a single SPI burst. By the time the switch
+ * reacts to the command, the ENTRY BUFFER is already populated with the data
+ * sent by the core.
+ *
+ * The COMMAND BUFFER is always SJA1105_SIZE_DYN_CMD bytes (one 32-bit word) in
+ * size.
+ *
+ * Sometimes the ENTRY BUFFER does not really exist (when the number of fields
+ * that can be reconfigured is small), then the switch repurposes some of the
+ * unused 32 bits of the COMMAND BUFFER to hold ENTRY data.
+ *
+ * The key members of struct sja1105_dynamic_table_ops are:
+ * - .entry_packing: A function that deals with packing an ENTRY structure
+ * into an SPI buffer, or retrieving an ENTRY structure
+ * from one.
+ * The @packed_buf pointer it's given does always point to
+ * the ENTRY portion of the buffer.
+ * - .cmd_packing: A function that deals with packing/unpacking the COMMAND
+ * structure to/from the SPI buffer.
+ * It is given the same @packed_buf pointer as .entry_packing,
+ * so most of the time, the @packed_buf points *behind* the
+ * COMMAND offset inside the buffer.
+ * To access the COMMAND portion of the buffer, the function
+ * knows its correct offset.
+ * Giving both functions the same pointer is handy because in
+ * extreme cases (see sja1105pqrs_dyn_l2_lookup_entry_packing)
+ * the .entry_packing is able to jump to the COMMAND portion,
+ * or vice-versa (sja1105pqrs_l2_lookup_cmd_packing).
+ * - .access: A bitmap of:
+ * OP_READ: Set if the hardware manual marks the ENTRY portion of the
+ * dynamic configuration table buffer as R (readable) after
+ * an SPI read command (the switch will populate the buffer).
+ * OP_WRITE: Set if the manual marks the ENTRY portion of the dynamic
+ * table buffer as W (writable) after an SPI write command
+ * (the switch will read the fields provided in the buffer).
+ * OP_DEL: Set if the manual says the VALIDENT bit is supported in the
+ * COMMAND portion of this dynamic config buffer (i.e. the
+ * specified entry can be invalidated through a SPI write
+ * command).
+ * OP_SEARCH: Set if the manual says that the index of an entry can
+ * be retrieved in the COMMAND portion of the buffer based
+ * on its ENTRY portion, as a result of a SPI write command.
+ * Only the TCAM-based FDB table on SJA1105 P/Q/R/S supports
+ * this.
+ * - .max_entry_count: The number of entries, counting from zero, that can be
+ * reconfigured through the dynamic interface. If a static
+ * table can be reconfigured at all dynamically, this
+ * number always matches the maximum number of supported
+ * static entries.
+ * - .packed_size: The length in bytes of the compound ENTRY + COMMAND BUFFER.
+ * Note that sometimes the compound buffer may contain holes in
+ * it (see sja1105_vlan_lookup_cmd_packing). The @packed_buf is
+ * contiguous however, so @packed_size includes any unused
+ * bytes.
+ * - .addr: The base SPI address at which the buffer must be written to the
+ * switch's memory. When looking at the hardware manual, this must
+ * always match the lowest documented address for the ENTRY, and not
+ * that of the COMMAND, since the other 32-bit words will follow along
+ * at the correct addresses.
+ */
+
#define SJA1105_SIZE_DYN_CMD 4
#define SJA1105ET_SIZE_MAC_CONFIG_DYN_ENTRY \
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 06/10] net: dsa: sja1105: Back up static FDB entries in kernel memory
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
After commit 8456721dd4ec ("net: dsa: sja1105: Add support for
configuring address ageing time"), we started to reset the switch rather
often (each time the bridge core changes the ageing time on a switch
port).
The unfortunate reality is that SJA1105 doesn't have any {cold, warm,
whatever} reset mode in which it accepts a new configuration stream
without flushing the FDB. Instead, in its world, the FDB *is* an
optional part of the static configuration.
So we play its game, and do what we also do for VLANs: for each 'bridge
fdb' command, we add the FDB entry through the dynamic interface, and we
append the in-kernel static config memory with info that we're going to
use later, when the next reset command is going to be issued.
The result is that 'bridge fdb' commands are now persistent (dynamically
learned entries are lost, but that's ok).
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 111 ++++++++++++++++++++++---
1 file changed, 99 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 46a3c81825ec..80d8d2f5c472 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -816,6 +816,77 @@ static void sja1105_phylink_validate(struct dsa_switch *ds, int port,
__ETHTOOL_LINK_MODE_MASK_NBITS);
}
+static int
+sja1105_find_static_fdb_entry(struct sja1105_private *priv, int port,
+ const struct sja1105_l2_lookup_entry *requested)
+{
+ struct sja1105_l2_lookup_entry *l2_lookup;
+ struct sja1105_table *table;
+ int i;
+
+ table = &priv->static_config.tables[BLK_IDX_L2_LOOKUP];
+ l2_lookup = table->entries;
+
+ for (i = 0; i < table->entry_count; i++)
+ if (l2_lookup[i].macaddr == requested->macaddr &&
+ l2_lookup[i].vlanid == requested->vlanid &&
+ l2_lookup[i].destports & BIT(port))
+ return i;
+
+ return -1;
+}
+
+/* We want FDB entries added statically through the bridge command to persist
+ * across switch resets, which are a common thing during normal SJA1105
+ * operation. So we have to back them up in the static configuration tables
+ * and hence apply them on next static config upload... yay!
+ */
+static int
+sja1105_static_fdb_change(struct sja1105_private *priv, int port,
+ const struct sja1105_l2_lookup_entry *requested,
+ bool keep)
+{
+ struct sja1105_l2_lookup_entry *l2_lookup;
+ struct sja1105_table *table;
+ int rc, match;
+
+ table = &priv->static_config.tables[BLK_IDX_L2_LOOKUP];
+
+ match = sja1105_find_static_fdb_entry(priv, port, requested);
+ if (match < 0) {
+ /* Can't delete a missing entry. */
+ if (!keep)
+ return 0;
+
+ /* No match => new entry */
+ rc = sja1105_table_resize(table, table->entry_count + 1);
+ if (rc)
+ return rc;
+
+ match = table->entry_count - 1;
+ }
+
+ /* Assign pointer after the resize (it may be new memory) */
+ l2_lookup = table->entries;
+
+ /* We have a match.
+ * If the job was to add this FDB entry, it's already done (mostly
+ * anyway, since the port forwarding mask may have changed, case in
+ * which we update it).
+ * Otherwise we have to delete it.
+ */
+ if (keep) {
+ l2_lookup[match] = *requested;
+ return 0;
+ }
+
+ /* To remove, the strategy is to overwrite the element with
+ * the last one, and then reduce the array size by 1
+ */
+ l2_lookup[match] = l2_lookup[table->entry_count - 1];
+ return sja1105_table_resize(table, table->entry_count - 1);
+}
+
/* First-generation switches have a 4-way set associative TCAM that
* holds the FDB entries. An FDB index spans from 0 to 1023 and is comprised of
* a "bin" (grouping of 4 entries) and a "way" (an entry within a bin).
@@ -866,7 +937,7 @@ int sja1105et_fdb_add(struct dsa_switch *ds, int port,
struct sja1105_private *priv = ds->priv;
struct device *dev = ds->dev;
int last_unused = -1;
- int bin, way;
+ int bin, way, rc;
bin = sja1105et_fdb_hash(priv, addr, vid);
@@ -910,9 +981,13 @@ int sja1105et_fdb_add(struct dsa_switch *ds, int port,
}
l2_lookup.index = sja1105et_fdb_index(bin, way);
- return sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
- l2_lookup.index, &l2_lookup,
- true);
+ rc = sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
+ l2_lookup.index, &l2_lookup,
+ true);
+ if (rc < 0)
+ return rc;
+
+ return sja1105_static_fdb_change(priv, port, &l2_lookup, true);
}
int sja1105et_fdb_del(struct dsa_switch *ds, int port,
@@ -920,7 +995,7 @@ int sja1105et_fdb_del(struct dsa_switch *ds, int port,
{
struct sja1105_l2_lookup_entry l2_lookup = {0};
struct sja1105_private *priv = ds->priv;
- int index, bin, way;
+ int index, bin, way, rc;
bool keep;
bin = sja1105et_fdb_hash(priv, addr, vid);
@@ -942,8 +1017,12 @@ int sja1105et_fdb_del(struct dsa_switch *ds, int port,
else
keep = false;
- return sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
- index, &l2_lookup, keep);
+ rc = sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
+ index, &l2_lookup, keep);
+ if (rc < 0)
+ return rc;
+
+ return sja1105_static_fdb_change(priv, port, &l2_lookup, keep);
}
int sja1105pqrs_fdb_add(struct dsa_switch *ds, int port,
@@ -994,9 +1073,13 @@ int sja1105pqrs_fdb_add(struct dsa_switch *ds, int port,
l2_lookup.index = i;
skip_finding_an_index:
- return sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
- l2_lookup.index, &l2_lookup,
- true);
+ rc = sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
+ l2_lookup.index, &l2_lookup,
+ true);
+ if (rc < 0)
+ return rc;
+
+ return sja1105_static_fdb_change(priv, port, &l2_lookup, true);
}
int sja1105pqrs_fdb_del(struct dsa_switch *ds, int port,
@@ -1030,8 +1113,12 @@ int sja1105pqrs_fdb_del(struct dsa_switch *ds, int port,
else
keep = false;
- return sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
- l2_lookup.index, &l2_lookup, keep);
+ rc = sja1105_dynamic_config_write(priv, BLK_IDX_L2_LOOKUP,
+ l2_lookup.index, &l2_lookup, keep);
+ if (rc < 0)
+ return rc;
+
+ return sja1105_static_fdb_change(priv, port, &l2_lookup, keep);
}
static int sja1105_fdb_add(struct dsa_switch *ds, int port,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 09/10] net: dsa: sja1105: Use correct dsa_8021q VIDs for FDB commands
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
A FDB entry means that "frames that match this VID and DMAC must be
forwarded to this port".
In the case of dsa_8021q however, the VID is not a single one (and
neither two, as my previous patch assumed). The VID can be set either by
the CPU port (1 tx_vid), or by any of the other front-panel port (n-1
rx_vid's).
Fixes: 93647594d8f5 ("net: dsa: sja1105: Hide the dsa_8021q VLANs from the bridge fdb command")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 82 ++++++++++++++++++--------
1 file changed, 56 insertions(+), 26 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index ed0b721c794e..cadee7694935 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1126,44 +1126,60 @@ static int sja1105_fdb_add(struct dsa_switch *ds, int port,
const unsigned char *addr, u16 vid)
{
struct sja1105_private *priv = ds->priv;
- int rc;
+ u16 rx_vid, tx_vid;
+ int rc, i;
+
+ if (dsa_port_is_vlan_filtering(&ds->ports[port]))
+ return priv->info->fdb_add_cmd(ds, port, addr, vid);
/* Since we make use of VLANs even when the bridge core doesn't tell us
* to, translate these FDB entries into the correct dsa_8021q ones.
+ * The basic idea (also repeats for removal below) is:
+ * - Each of the other front-panel ports needs to be able to forward a
+ * pvid-tagged (aka tagged with their rx_vid) frame that matches this
+ * DMAC.
+ * - The CPU port (aka the tx_vid of this port) needs to be able to
+ * send a frame matching this DMAC to the specified port.
+ * For a better picture see net/dsa/tag_8021q.c.
*/
- if (!dsa_port_is_vlan_filtering(&ds->ports[port])) {
- unsigned int upstream = dsa_upstream_port(priv->ds, port);
- u16 tx_vid = dsa_8021q_tx_vid(ds, port);
- u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+ for (i = 0; i < SJA1105_NUM_PORTS; i++) {
+ if (i == port)
+ continue;
+ if (i == dsa_upstream_port(priv->ds, port))
+ continue;
- rc = priv->info->fdb_add_cmd(ds, port, addr, tx_vid);
+ rx_vid = dsa_8021q_rx_vid(ds, i);
+ rc = priv->info->fdb_add_cmd(ds, port, addr, rx_vid);
if (rc < 0)
return rc;
- return priv->info->fdb_add_cmd(ds, upstream, addr, rx_vid);
}
- return priv->info->fdb_add_cmd(ds, port, addr, vid);
+ tx_vid = dsa_8021q_tx_vid(ds, port);
+ return priv->info->fdb_add_cmd(ds, port, addr, tx_vid);
}
static int sja1105_fdb_del(struct dsa_switch *ds, int port,
const unsigned char *addr, u16 vid)
{
struct sja1105_private *priv = ds->priv;
- int rc;
+ u16 rx_vid, tx_vid;
+ int rc, i;
- /* Since we make use of VLANs even when the bridge core doesn't tell us
- * to, translate these FDB entries into the correct dsa_8021q ones.
- */
- if (!dsa_port_is_vlan_filtering(&ds->ports[port])) {
- unsigned int upstream = dsa_upstream_port(priv->ds, port);
- u16 tx_vid = dsa_8021q_tx_vid(ds, port);
- u16 rx_vid = dsa_8021q_rx_vid(ds, port);
+ if (dsa_port_is_vlan_filtering(&ds->ports[port]))
+ return priv->info->fdb_del_cmd(ds, port, addr, vid);
- rc = priv->info->fdb_del_cmd(ds, port, addr, tx_vid);
+ for (i = 0; i < SJA1105_NUM_PORTS; i++) {
+ if (i == port)
+ continue;
+ if (i == dsa_upstream_port(priv->ds, port))
+ continue;
+
+ rx_vid = dsa_8021q_rx_vid(ds, i);
+ rc = priv->info->fdb_del_cmd(ds, port, addr, rx_vid);
if (rc < 0)
return rc;
- return priv->info->fdb_del_cmd(ds, upstream, addr, rx_vid);
}
- return priv->info->fdb_del_cmd(ds, port, addr, vid);
+ tx_vid = dsa_8021q_tx_vid(ds, port);
+ return priv->info->fdb_del_cmd(ds, port, addr, tx_vid);
}
static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
@@ -1171,8 +1187,12 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
{
struct sja1105_private *priv = ds->priv;
struct device *dev = ds->dev;
+ u16 rx_vid, tx_vid;
int i;
+ rx_vid = dsa_8021q_rx_vid(ds, port);
+ tx_vid = dsa_8021q_tx_vid(ds, port);
+
for (i = 0; i < SJA1105_MAX_L2_LOOKUP_COUNT; i++) {
struct sja1105_l2_lookup_entry l2_lookup = {0};
u8 macaddr[ETH_ALEN];
@@ -1198,14 +1218,24 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
continue;
u64_to_ether_addr(l2_lookup.macaddr, macaddr);
- /* We need to hide the dsa_8021q VLAN from the user.
- * Convert the TX VID into the pvid that is active in
- * standalone and non-vlan_filtering modes, aka 1.
- * The RX VID is applied on the CPU port, which is not seen by
- * the bridge core anyway, so there's nothing to hide.
+ /* We need to hide the dsa_8021q VLANs from the user. This
+ * basically means hiding the duplicates and only showing
+ * the pvid that is supposed to be active in standalone and
+ * non-vlan_filtering modes (aka 1).
+ * - For statically added FDB entries (bridge fdb add), we
+ * can convert the TX VID (coming from the CPU port) into the
+ * pvid and ignore the RX VIDs of the other ports.
+ * - For dynamically learned FDB entries, a single entry with
+ * no duplicates is learned - that which has the real port's
+ * pvid, aka RX VID.
*/
- if (!dsa_port_is_vlan_filtering(&ds->ports[port]))
- l2_lookup.vlanid = 1;
+ if (!dsa_port_is_vlan_filtering(&ds->ports[port])) {
+ if (l2_lookup.vlanid == tx_vid ||
+ l2_lookup.vlanid == rx_vid)
+ l2_lookup.vlanid = 1;
+ else
+ continue;
+ }
cb(macaddr, l2_lookup.vlanid, l2_lookup.lockeds, data);
}
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 10/10] net: dsa: sja1105: Implement is_static for FDB entries on E/T
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
The first generation switches don't tell us through the dynamic config
interface whether the dumped FDB entries are static or not (the LOCKEDS
bit from P/Q/R/S).
However, now that we're keeping a mirror of all 'bridge fdb' commands in
the static config, this is an opportunity to compare a dumped FDB entry
to the driver's private database. After all, what makes an entry static
is that *we* added it.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index cadee7694935..caebf76eaa3e 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1218,6 +1218,21 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
continue;
u64_to_ether_addr(l2_lookup.macaddr, macaddr);
+ /* On SJA1105 E/T, the switch doesn't implement the LOCKEDS
+ * bit, so it doesn't tell us whether a FDB entry is static
+ * or not.
+ * But, of course, we can find out - we're the ones who added
+ * it in the first place.
+ */
+ if (priv->info->device_id == SJA1105E_DEVICE_ID ||
+ priv->info->device_id == SJA1105T_DEVICE_ID) {
+ int match;
+
+ match = sja1105_find_static_fdb_entry(priv, port,
+ &l2_lookup);
+ l2_lookup.lockeds = (match >= 0);
+ }
+
/* We need to hide the dsa_8021q VLANs from the user. This
* basically means hiding the duplicates and only showing
* the pvid that is supposed to be active in standalone and
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 08/10] net: dsa: sja1105: Populate is_static for FDB entries on P/Q/R/S
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
The reason why this wasn't tackled earlier is that I had hoped I
understood the user manual wrong. But unfortunately hacks are required
in order to retrieve the static/dynamic nature of FDB entries on SJA1105
P/Q/R/S, since this info is stored in the writeback buffer of the
dynamic config command.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
.../net/dsa/sja1105/sja1105_dynamic_config.c | 62 ++++++++++++++++++-
drivers/net/dsa/sja1105/sja1105_main.c | 3 +-
.../net/dsa/sja1105/sja1105_static_config.h | 2 +-
3 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
index 3acd48615981..6bfb1696a6f2 100644
--- a/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
+++ b/drivers/net/dsa/sja1105/sja1105_dynamic_config.c
@@ -149,13 +149,11 @@ sja1105pqrs_l2_lookup_cmd_packing(void *buf, struct sja1105_dyn_cmd *cmd,
{
u8 *p = buf + SJA1105PQRS_SIZE_L2_LOOKUP_ENTRY;
const int size = SJA1105_SIZE_DYN_CMD;
- u64 lockeds = 0;
u64 hostcmd;
sja1105_packing(p, &cmd->valid, 31, 31, size, op);
sja1105_packing(p, &cmd->rdwrset, 30, 30, size, op);
sja1105_packing(p, &cmd->errors, 29, 29, size, op);
- sja1105_packing(p, &lockeds, 28, 28, size, op);
sja1105_packing(p, &cmd->valident, 27, 27, size, op);
/* VALIDENT is supposed to indicate "keep or not", but in SJA1105 E/T,
@@ -205,6 +203,64 @@ sja1105pqrs_l2_lookup_cmd_packing(void *buf, struct sja1105_dyn_cmd *cmd,
SJA1105PQRS_SIZE_L2_LOOKUP_ENTRY, op);
}
+/* The switch is so retarded that it makes our command/entry abstraction
+ * crumble apart.
+ *
+ * On P/Q/R/S, the switch tries to say whether a FDB entry
+ * is statically programmed or dynamically learned via a flag called LOCKEDS.
+ * The hardware manual says about this fiels:
+ *
+ * On write will specify the format of ENTRY.
+ * On read the flag will be found cleared at times the VALID flag is found
+ * set. The flag will also be found cleared in response to a read having the
+ * MGMTROUTE flag set. In response to a read with the MGMTROUTE flag
+ * cleared, the flag be set if the most recent access operated on an entry
+ * that was either loaded by configuration or through dynamic reconfiguration
+ * (as opposed to automatically learned entries).
+ *
+ * The trouble with this flag is that it's part of the *command* to access the
+ * dynamic interface, and not part of the *entry* retrieved from it.
+ * Otherwise said, for a sja1105_dynamic_config_read, LOCKEDS is supposed to be
+ * an output from the switch into the command buffer, and for a
+ * sja1105_dynamic_config_write, the switch treats LOCKEDS as an input
+ * (hence we can write either static, or automatically learned entries, from
+ * the core).
+ * But the manual contradicts itself in the last phrase where it says that on
+ * read, LOCKEDS will be set to 1 for all FDB entries written through the
+ * dynamic interface (therefore, the value of LOCKEDS from the
+ * sja1105_dynamic_config_write is not really used for anything, it'll store a
+ * 1 anyway).
+ * This means you can't really write a FDB entry with LOCKEDS=0 (automatically
+ * learned) into the switch, which kind of makes sense.
+ * As for reading through the dynamic interface, it doesn't make too much sense
+ * to put LOCKEDS into the command, since the switch will inevitably have to
+ * ignore it (otherwise a command would be like "read the FDB entry 123, but
+ * only if it's dynamically learned" <- well how am I supposed to know?) and
+ * just use it as an output buffer for its findings. But guess what... that's
+ * what the entry buffer is for!
+ * Unfortunately, what really breaks this abstraction is the fact that it
+ * wasn't designed having the fact in mind that the switch can output
+ * entry-related data as writeback through the command buffer.
+ * However, whether a FDB entry is statically or dynamically learned *is* part
+ * of the entry and not the command data, no matter what the switch thinks.
+ * In order to do that, we'll need to wrap around the
+ * sja1105pqrs_l2_lookup_entry_packing from sja1105_static_config.c, and take
+ * a peek outside of the caller-supplied @buf (the entry buffer), to reach the
+ * command buffer.
+ */
+static size_t
+sja1105pqrs_dyn_l2_lookup_entry_packing(void *buf, void *entry_ptr,
+ enum packing_op op)
+{
+ struct sja1105_l2_lookup_entry *entry = entry_ptr;
+ u8 *cmd = buf + SJA1105PQRS_SIZE_L2_LOOKUP_ENTRY;
+ const int size = SJA1105_SIZE_DYN_CMD;
+
+ sja1105_packing(cmd, &entry->lockeds, 28, 28, size, op);
+
+ return sja1105pqrs_l2_lookup_entry_packing(buf, entry_ptr, op);
+}
+
static void
sja1105et_l2_lookup_cmd_packing(void *buf, struct sja1105_dyn_cmd *cmd,
enum packing_op op)
@@ -485,7 +541,7 @@ struct sja1105_dynamic_table_ops sja1105et_dyn_ops[BLK_IDX_MAX_DYN] = {
/* SJA1105P/Q/R/S: Second generation */
struct sja1105_dynamic_table_ops sja1105pqrs_dyn_ops[BLK_IDX_MAX_DYN] = {
[BLK_IDX_L2_LOOKUP] = {
- .entry_packing = sja1105pqrs_l2_lookup_entry_packing,
+ .entry_packing = sja1105pqrs_dyn_l2_lookup_entry_packing,
.cmd_packing = sja1105pqrs_l2_lookup_cmd_packing,
.access = (OP_READ | OP_WRITE | OP_DEL | OP_SEARCH),
.max_entry_count = SJA1105_MAX_L2_LOOKUP_COUNT,
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 80d8d2f5c472..ed0b721c794e 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1070,6 +1070,7 @@ int sja1105pqrs_fdb_add(struct dsa_switch *ds, int port,
dev_err(ds->dev, "FDB is full, cannot add entry.\n");
return -EINVAL;
}
+ l2_lookup.lockeds = true;
l2_lookup.index = i;
skip_finding_an_index:
@@ -1205,7 +1206,7 @@ static int sja1105_fdb_dump(struct dsa_switch *ds, int port,
*/
if (!dsa_port_is_vlan_filtering(&ds->ports[port]))
l2_lookup.vlanid = 1;
- cb(macaddr, l2_lookup.vlanid, false, data);
+ cb(macaddr, l2_lookup.vlanid, l2_lookup.lockeds, data);
}
return 0;
}
diff --git a/drivers/net/dsa/sja1105/sja1105_static_config.h b/drivers/net/dsa/sja1105/sja1105_static_config.h
index 2a3a1a92d7c3..684465fc0882 100644
--- a/drivers/net/dsa/sja1105/sja1105_static_config.h
+++ b/drivers/net/dsa/sja1105/sja1105_static_config.h
@@ -132,7 +132,7 @@ struct sja1105_l2_lookup_entry {
u64 mask_vlanid;
u64 mask_macaddr;
u64 iotag;
- bool lockeds;
+ u64 lockeds;
union {
/* LOCKEDS=1: Static FDB entries */
struct {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 05/10] net: dsa: sja1105: Make P/Q/R/S learn MAC addresses
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
At the end of the commit 1da73821343c ("net: dsa: sja1105: Add FDB
operations for P/Q/R/S series") message, I said that:
At the moment only FDB entries installed statically through 'bridge fdb'
are visible in the dump callback - the dynamically learned ones are
still under investigation.
It looks like the reason why they were not visible in 'bridge fdb' was
that they were never learned - always flooded.
SJA1105 P/Q/R/S manual says about the MAXADDRP[port] field:
Specify the maximum number of MAC address dynamically learned from
the respective port. It is used to limit the number of learned MAC
addresses per port.
It looks like not providing a value in the static config (aka providing
zeroes) is enough for it to not store the learned addresses in the FDB.
For now we divide the 1024 entry FDB "equally" amongst the 5 ports. This
may be revisited if the situation calls for that - for now I'm happy
that learning works.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 3 +++
drivers/net/dsa/sja1105/sja1105_static_config.c | 4 ++++
drivers/net/dsa/sja1105/sja1105_static_config.h | 1 +
3 files changed, 8 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index bc9f37cd3876..46a3c81825ec 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -203,6 +203,7 @@ static int sja1105_init_static_fdb(struct sja1105_private *priv)
static int sja1105_init_l2_lookup_params(struct sja1105_private *priv)
{
struct sja1105_table *table;
+ u64 max_fdb_entries = SJA1105_MAX_L2_LOOKUP_COUNT / SJA1105_NUM_PORTS;
struct sja1105_l2_lookup_params_entry default_l2_lookup_params = {
/* Learned FDB entries are forgotten after 300 seconds */
.maxage = SJA1105_AGEING_TIME_MS(300000),
@@ -210,6 +211,8 @@ static int sja1105_init_l2_lookup_params(struct sja1105_private *priv)
.dyn_tbsz = SJA1105ET_FDB_BIN_SIZE,
/* And the P/Q/R/S equivalent setting: */
.start_dynspc = 0,
+ .maxaddrp = {max_fdb_entries, max_fdb_entries, max_fdb_entries,
+ max_fdb_entries, max_fdb_entries, },
/* 2^8 + 2^5 + 2^3 + 2^2 + 2^1 + 1 in Koopman notation */
.poly = 0x97,
/* This selects between Independent VLAN Learning (IVL) and
diff --git a/drivers/net/dsa/sja1105/sja1105_static_config.c b/drivers/net/dsa/sja1105/sja1105_static_config.c
index a1e9656c881c..b31c737dc560 100644
--- a/drivers/net/dsa/sja1105/sja1105_static_config.c
+++ b/drivers/net/dsa/sja1105/sja1105_static_config.c
@@ -230,7 +230,11 @@ sja1105pqrs_l2_lookup_params_entry_packing(void *buf, void *entry_ptr,
{
const size_t size = SJA1105PQRS_SIZE_L2_LOOKUP_PARAMS_ENTRY;
struct sja1105_l2_lookup_params_entry *entry = entry_ptr;
+ int offset, i;
+ for (i = 0, offset = 58; i < 5; i++, offset += 11)
+ sja1105_packing(buf, &entry->maxaddrp[i],
+ offset + 10, offset + 0, size, op);
sja1105_packing(buf, &entry->maxage, 57, 43, size, op);
sja1105_packing(buf, &entry->start_dynspc, 42, 33, size, op);
sja1105_packing(buf, &entry->drpnolearn, 32, 28, size, op);
diff --git a/drivers/net/dsa/sja1105/sja1105_static_config.h b/drivers/net/dsa/sja1105/sja1105_static_config.h
index a9586d0b4b3b..2a3a1a92d7c3 100644
--- a/drivers/net/dsa/sja1105/sja1105_static_config.h
+++ b/drivers/net/dsa/sja1105/sja1105_static_config.h
@@ -151,6 +151,7 @@ struct sja1105_l2_lookup_entry {
};
struct sja1105_l2_lookup_params_entry {
+ u64 maxaddrp[5]; /* P/Q/R/S only */
u64 start_dynspc; /* P/Q/R/S only */
u64 drpnolearn; /* P/Q/R/S only */
u64 use_static; /* P/Q/R/S only */
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 04/10] net: dsa: sja1105: Actually implement the P/Q/R/S FDB bits
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
In commit 1da73821343c ("net: dsa: sja1105: Add FDB operations for
P/Q/R/S series"), these bits were set in the static config, but
apparently they did not do anything. The reason is that the packing
accessors for them were part of a patch I forgot to send.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_static_config.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_static_config.c b/drivers/net/dsa/sja1105/sja1105_static_config.c
index 242f001c59fe..a1e9656c881c 100644
--- a/drivers/net/dsa/sja1105/sja1105_static_config.c
+++ b/drivers/net/dsa/sja1105/sja1105_static_config.c
@@ -232,9 +232,14 @@ sja1105pqrs_l2_lookup_params_entry_packing(void *buf, void *entry_ptr,
struct sja1105_l2_lookup_params_entry *entry = entry_ptr;
sja1105_packing(buf, &entry->maxage, 57, 43, size, op);
+ sja1105_packing(buf, &entry->start_dynspc, 42, 33, size, op);
+ sja1105_packing(buf, &entry->drpnolearn, 32, 28, size, op);
sja1105_packing(buf, &entry->shared_learn, 27, 27, size, op);
sja1105_packing(buf, &entry->no_enf_hostprt, 26, 26, size, op);
sja1105_packing(buf, &entry->no_mgmt_learn, 25, 25, size, op);
+ sja1105_packing(buf, &entry->use_static, 24, 24, size, op);
+ sja1105_packing(buf, &entry->owr_dyn, 23, 23, size, op);
+ sja1105_packing(buf, &entry->learn_once, 22, 22, size, op);
return size;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 03/10] net: dsa: sja1105: Make vid 1 the default pvid
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
In SJA1105 there is no concept of 'default values' per se, everything
needs to be driver-supplied through the static configuration tables.
The issue is that the hardware manual says that 'at least the default
untagging VLAN' is mandatory to be provided through the static config.
But VLAN 0 isn't a very good initial pvid - its use is reserved for
priority-tagged frames, and the layers of the stack that care about
those already make sure that this VLAN is installed, as can be seen in
the message below:
8021q: adding VLAN 0 to HW filter on device swp2
So change the pvid provided through the static configuration to 1, which
matches the bridge core's defaults.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 9395e8f5f790..bc9f37cd3876 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -80,7 +80,7 @@ static int sja1105_init_mac_settings(struct sja1105_private *priv)
.maxage = 0xFF,
/* Internal VLAN (pvid) to apply to untagged ingress */
.vlanprio = 0,
- .vlanid = 0,
+ .vlanid = 1,
.ing_mirr = false,
.egr_mirr = false,
/* Don't drop traffic with other EtherType than ETH_P_IP */
@@ -264,20 +264,15 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
.vmemb_port = 0,
.vlan_bc = 0,
.tag_port = 0,
- .vlanid = 0,
+ .vlanid = 1,
};
int i;
table = &priv->static_config.tables[BLK_IDX_VLAN_LOOKUP];
- /* The static VLAN table will only contain the initial pvid of 0.
+ /* The static VLAN table will only contain the initial pvid of 1.
* All other VLANs are to be configured through dynamic entries,
* and kept in the static configuration table as backing memory.
- * The pvid of 0 is sufficient to pass traffic while the ports are
- * standalone and when vlan_filtering is disabled. When filtering
- * gets enabled, the switchdev core sets up the VLAN ID 1 and sets
- * it as the new pvid. Actually 'pvid 1' still comes up in 'bridge
- * vlan' even when vlan_filtering is off, but it has no effect.
*/
if (table->entry_count) {
kfree(table->entries);
@@ -291,7 +286,7 @@ static int sja1105_init_static_vlan(struct sja1105_private *priv)
table->entry_count = 1;
- /* VLAN ID 0: all DT-defined ports are members; no restrictions on
+ /* VLAN 1: all DT-defined ports are members; no restrictions on
* forwarding; always transmit priority-tagged frames as untagged.
*/
for (i = 0; i < SJA1105_NUM_PORTS; i++) {
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 02/10] net: dsa: sja1105: Cancel PTP delayed work on unregister
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
In-Reply-To: <20190625233942.1946-1-olteanv@gmail.com>
Currently when the driver unloads and PTP is enabled, the delayed work
that prevents the timecounter from expiring becomes a ticking time bomb.
The kernel will schedule the work thread within 60 seconds of driver
removal, but the work handler is no longer there, leading to this
strange and inconclusive stack trace:
[ 64.473112] Unable to handle kernel paging request at virtual address 79746970
[ 64.480340] pgd = 008c4af9
[ 64.483042] [79746970] *pgd=00000000
[ 64.486620] Internal error: Oops: 80000005 [#1] SMP ARM
[ 64.491820] Modules linked in:
[ 64.494871] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0-rc5-01634-ge3a2773ba9e5 #1246
[ 64.503007] Hardware name: Freescale LS1021A
[ 64.507259] PC is at 0x79746970
[ 64.510393] LR is at call_timer_fn+0x3c/0x18c
[ 64.514729] pc : [<79746970>] lr : [<c03bd734>] psr: 60010113
[ 64.520965] sp : c1901de0 ip : 00000000 fp : c1903080
[ 64.526163] r10: c1901e38 r9 : ffffe000 r8 : c19064ac
[ 64.531363] r7 : 79746972 r6 : e98dd260 r5 : 00000100 r4 : c1a9e4a0
[ 64.537859] r3 : c1900000 r2 : ffffa400 r1 : 79746972 r0 : e98dd260
[ 64.544359] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 64.551460] Control: 10c5387d Table: a8a2806a DAC: 00000051
[ 64.557176] Process swapper/0 (pid: 0, stack limit = 0x1ddb27f0)
[ 64.563147] Stack: (0xc1901de0 to 0xc1902000)
[ 64.567481] 1de0: eb6a4918 3d60d7c3 c1a9e554 e98dd260 eb6a34c0 c1a9e4a0 ffffa400 c19064ac
[ 64.575616] 1e00: ffffe000 c03bd95c c1901e34 c1901e34 eb6a34c0 c1901e30 c1903d00 c186f4c0
[ 64.583751] 1e20: c1906488 29e34000 c1903080 c03bdca4 00000000 eaa6f218 00000000 eb6a45c0
[ 64.591886] 1e40: eb6a45c0 20010193 00000003 c03c0a68 20010193 3f7231be c1903084 00000002
[ 64.600022] 1e60: 00000082 00000001 ffffe000 c1a9e0a4 00000100 c0302298 02b64722 0000000f
[ 64.608157] 1e80: c186b3c8 c1877540 c19064ac 0000000a c186b350 ffffa401 c1903d00 c1107348
[ 64.616292] 1ea0: 00200102 c0d87a14 ea823c00 ffffe000 00000012 00000000 00000000 ea810800
[ 64.624427] 1ec0: f0803000 c1876ba8 00000000 c034c784 c18774b8 c039fb50 c1906c90 c1978aac
[ 64.632562] 1ee0: f080200c f0802000 c1901f10 c0709ca8 c03091a0 60010013 ffffffff c1901f44
[ 64.640697] 1f00: 00000000 c1900000 c1876ba8 c0301a8c 00000000 000070a0 eb6ac1a0 c031da60
[ 64.648832] 1f20: ffffe000 c19064ac c19064f0 00000001 00000000 c1906488 c1876ba8 00000000
[ 64.656967] 1f40: ffffffff c1901f60 c030919c c03091a0 60010013 ffffffff 00000051 00000000
[ 64.665102] 1f60: ffffe000 c0376aa4 c1a9da37 ffffffff 00000037 3f7231be c1ab20c0 000000cc
[ 64.673238] 1f80: c1906488 c1906480 ffffffff 00000037 c1ab20c0 c1ab20c0 00000001 c0376e1c
[ 64.681373] 1fa0: c1ab2118 c1700ea8 ffffffff ffffffff 00000000 c1700754 c17dfa40 ebfffd80
[ 64.689509] 1fc0: 00000000 c17dfa40 3f7733be 00000000 00000000 c1700330 00000051 10c0387d
[ 64.697644] 1fe0: 00000000 8f000000 410fc075 10c5387d 00000000 00000000 00000000 00000000
[ 64.705788] [<c03bd734>] (call_timer_fn) from [<c03bd95c>] (expire_timers+0xd8/0x144)
[ 64.713579] [<c03bd95c>] (expire_timers) from [<c03bdca4>] (run_timer_softirq+0xe4/0x1dc)
[ 64.721716] [<c03bdca4>] (run_timer_softirq) from [<c0302298>] (__do_softirq+0x130/0x3c8)
[ 64.729854] [<c0302298>] (__do_softirq) from [<c034c784>] (irq_exit+0xbc/0xd8)
[ 64.737040] [<c034c784>] (irq_exit) from [<c039fb50>] (__handle_domain_irq+0x60/0xb4)
[ 64.744833] [<c039fb50>] (__handle_domain_irq) from [<c0709ca8>] (gic_handle_irq+0x58/0x9c)
[ 64.753143] [<c0709ca8>] (gic_handle_irq) from [<c0301a8c>] (__irq_svc+0x6c/0x90)
[ 64.760583] Exception stack(0xc1901f10 to 0xc1901f58)
[ 64.765605] 1f00: 00000000 000070a0 eb6ac1a0 c031da60
[ 64.773740] 1f20: ffffe000 c19064ac c19064f0 00000001 00000000 c1906488 c1876ba8 00000000
[ 64.781873] 1f40: ffffffff c1901f60 c030919c c03091a0 60010013 ffffffff
[ 64.788456] [<c0301a8c>] (__irq_svc) from [<c03091a0>] (arch_cpu_idle+0x38/0x3c)
[ 64.795816] [<c03091a0>] (arch_cpu_idle) from [<c0376aa4>] (do_idle+0x1bc/0x298)
[ 64.803175] [<c0376aa4>] (do_idle) from [<c0376e1c>] (cpu_startup_entry+0x18/0x1c)
[ 64.810707] [<c0376e1c>] (cpu_startup_entry) from [<c1700ea8>] (start_kernel+0x480/0x4ac)
[ 64.818839] Code: bad PC value
[ 64.821890] ---[ end trace e226ed97b1c584cd ]---
[ 64.826482] Kernel panic - not syncing: Fatal exception in interrupt
[ 64.832807] CPU1: stopping
[ 64.835501] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D 5.2.0-rc5-01634-ge3a2773ba9e5 #1246
[ 64.845013] Hardware name: Freescale LS1021A
[ 64.849266] [<c0312394>] (unwind_backtrace) from [<c030cc74>] (show_stack+0x10/0x14)
[ 64.856972] [<c030cc74>] (show_stack) from [<c0ff4138>] (dump_stack+0xb4/0xc8)
[ 64.864159] [<c0ff4138>] (dump_stack) from [<c0310854>] (handle_IPI+0x3bc/0x3dc)
[ 64.871519] [<c0310854>] (handle_IPI) from [<c0709ce8>] (gic_handle_irq+0x98/0x9c)
[ 64.879050] [<c0709ce8>] (gic_handle_irq) from [<c0301a8c>] (__irq_svc+0x6c/0x90)
[ 64.886489] Exception stack(0xea8cbf60 to 0xea8cbfa8)
[ 64.891514] bf60: 00000000 0000307c eb6c11a0 c031da60 ffffe000 c19064ac c19064f0 00000002
[ 64.899649] bf80: 00000000 c1906488 c1876ba8 00000000 00000000 ea8cbfb0 c030919c c03091a0
[ 64.907780] bfa0: 600d0013 ffffffff
[ 64.911250] [<c0301a8c>] (__irq_svc) from [<c03091a0>] (arch_cpu_idle+0x38/0x3c)
[ 64.918609] [<c03091a0>] (arch_cpu_idle) from [<c0376aa4>] (do_idle+0x1bc/0x298)
[ 64.925967] [<c0376aa4>] (do_idle) from [<c0376e1c>] (cpu_startup_entry+0x18/0x1c)
[ 64.933496] [<c0376e1c>] (cpu_startup_entry) from [<803025cc>] (0x803025cc)
[ 64.940422] Rebooting in 3 seconds..
In this case, what happened is that the DSA driver failed to probe at
boot time due to a PHY issue during phylink_connect_phy:
[ 2.245607] fsl-gianfar soc:ethernet@2d90000 eth2: error -19 setting up slave phy
[ 2.258051] sja1105 spi0.1: failed to create slave for port 0.0
Fixes: bb77f36ac21d ("net: dsa: sja1105: Add support for the PTP clock")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_ptp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index c7ce1edd8471..d19cfdf681af 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -387,6 +387,7 @@ void sja1105_ptp_clock_unregister(struct sja1105_private *priv)
if (IS_ERR_OR_NULL(priv->clock))
return;
+ cancel_delayed_work_sync(&priv->refresh_work);
ptp_clock_unregister(priv->clock);
priv->clock = NULL;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 00/10] FDB, VLAN and PTP fixes for SJA1105 DSA
From: Vladimir Oltean @ 2019-06-25 23:39 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, davem; +Cc: netdev, Vladimir Oltean
This patchset is an assortment of fixes for the net-next version of the
sja1105 DSA driver:
- Avoid a kernel panic when the driver fails to probe or unregisters
- Finish Arnd Bermann's idea of compiling PTP support as part of the
main DSA driver and not separately
- Better handling of initial port-based VLAN as well as VLANs for
dsa_8021q FDB entries
- Fix address learning for the SJA1105 P/Q/R/S family
- Make static FDB entries persistent across switch resets
- Fix reporting of statically-added FDB entries in 'bridge fdb show'
Vladimir Oltean (10):
net: dsa: sja1105: Build PTP support in main DSA driver
net: dsa: sja1105: Cancel PTP delayed work on unregister
net: dsa: sja1105: Make vid 1 the default pvid
net: dsa: sja1105: Actually implement the P/Q/R/S FDB bits
net: dsa: sja1105: Make P/Q/R/S learn MAC addresses
net: dsa: sja1105: Back up static FDB entries in kernel memory
net: dsa: sja1105: Add a high-level overview of the dynamic config
interface
net: dsa: sja1105: Populate is_static for FDB entries on P/Q/R/S
net: dsa: sja1105: Use correct dsa_8021q VIDs for FDB commands
net: dsa: sja1105: Implement is_static for FDB entries on E/T
drivers/net/dsa/sja1105/Makefile | 2 +-
.../net/dsa/sja1105/sja1105_dynamic_config.c | 154 +++++++++++-
drivers/net/dsa/sja1105/sja1105_main.c | 227 ++++++++++++++----
drivers/net/dsa/sja1105/sja1105_ptp.c | 13 +-
drivers/net/dsa/sja1105/sja1105_spi.c | 2 -
.../net/dsa/sja1105/sja1105_static_config.c | 12 +-
.../net/dsa/sja1105/sja1105_static_config.h | 3 +-
7 files changed, 343 insertions(+), 70 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH net-next 10/18] ionic: Add management of rx filters
From: Jakub Kicinski @ 2019-06-25 23:37 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-11-snelson@pensando.io>
On Thu, 20 Jun 2019 13:24:16 -0700, Shannon Nelson wrote:
> +int ionic_rx_filter_save(struct lif *lif, u32 flow_id, u16 rxq_index,
> + u32 hash, struct ionic_admin_ctx *ctx)
> +{
> + struct device *dev = lif->ionic->dev;
> + struct hlist_head *head;
> + struct rx_filter *f;
> + unsigned int key;
> +
> + f = devm_kzalloc(dev, sizeof(*f), GFP_KERNEL);
> + if (!f)
> + return -ENOMEM;
> +
> + f->flow_id = flow_id;
> + f->filter_id = le32_to_cpu(ctx->comp.rx_filter_add.filter_id);
> + f->rxq_index = rxq_index;
> + memcpy(&f->cmd, &ctx->cmd, sizeof(f->cmd));
> +
> + INIT_HLIST_NODE(&f->by_hash);
> + INIT_HLIST_NODE(&f->by_id);
> +
> + switch (le16_to_cpu(f->cmd.match)) {
> + case RX_FILTER_MATCH_VLAN:
> + key = le16_to_cpu(f->cmd.vlan.vlan) & RX_FILTER_HLISTS_MASK;
> + break;
> + case RX_FILTER_MATCH_MAC:
> + key = *(u32 *)f->cmd.mac.addr & RX_FILTER_HLISTS_MASK;
> + break;
> + case RX_FILTER_MATCH_MAC_VLAN:
> + key = le16_to_cpu(f->cmd.mac_vlan.vlan) & RX_FILTER_HLISTS_MASK;
> + break;
> + default:
I know you use devm_kzalloc() but can't this potentially keep arbitrary
amounts of memory held until the device is removed (and it's the entire
device not just a LIF)?
> + return -ENOTSUPP;
EOPNOTSUPP, please do not use ENOTSUPP in the drivers. It's a high
error code, unknown to libc. We should use EOPNOTSUPP or EINVAL.
> + }
^ permalink raw reply
* Re: [PATCH net-next 09/18] ionic: Add the basic NDO callbacks for netdev support
From: Jakub Kicinski @ 2019-06-25 23:27 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-10-snelson@pensando.io>
On Thu, 20 Jun 2019 13:24:15 -0700, Shannon Nelson wrote:
> +static int ionic_set_features(struct net_device *netdev,
> + netdev_features_t features)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + int err;
> +
> + netdev_dbg(netdev, "%s: lif->features=0x%08llx new_features=0x%08llx\n",
> + __func__, (u64)lif->netdev->features, (u64)features);
> +
> + err = ionic_set_nic_features(lif, features);
Presumably something gets added here in later patch?
> + return err;
> +}
> +
> +static int ionic_set_mac_address(struct net_device *netdev, void *sa)
> +{
> + netdev_info(netdev, "%s: stubbed\n", __func__);
> + return 0;
> +}
> +
> +static int ionic_change_mtu(struct net_device *netdev, int new_mtu)
> +{
> + struct lif *lif = netdev_priv(netdev);
> + struct ionic_admin_ctx ctx = {
> + .work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
> + .cmd.lif_setattr = {
> + .opcode = CMD_OPCODE_LIF_SETATTR,
> + .index = cpu_to_le16(lif->index),
> + .attr = IONIC_LIF_ATTR_MTU,
> + .mtu = cpu_to_le32(new_mtu),
> + },
> + };
> + int err;
> +
> + if (new_mtu < IONIC_MIN_MTU || new_mtu > IONIC_MAX_MTU) {
> + netdev_err(netdev, "Invalid MTU %d\n", new_mtu);
> + return -EINVAL;
> + }
We do the min/max checks in the core now (netdev->min_mtu,
netdev->max_mtu). You'll have to keep this if out of tree,
unfortunately.
> + err = ionic_adminq_post_wait(lif, &ctx);
> + if (err)
> + return err;
> +
> + netdev->mtu = new_mtu;
> + err = ionic_reset_queues(lif);
> +
> + return err;
> +}
^ permalink raw reply
* [PATCH bpf-next 2/2] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-06-25 23:26 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190625232601.3227055-1-andriin@fb.com>
Add test verifying perf buffer API functionality.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
.../selftests/bpf/prog_tests/perf_buffer.c | 86 +++++++++++++++++++
.../selftests/bpf/progs/test_perf_buffer.c | 31 +++++++
2 files changed, 117 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
new file mode 100644
index 000000000000..3ba3e26141ac
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <sched.h>
+#include <sys/socket.h>
+#include <test_progs.h>
+
+static void on_sample(void *ctx, void *data, __u32 size)
+{
+ cpu_set_t *cpu_seen = ctx;
+ int cpu = *(int *)data;
+
+ CPU_SET(cpu, cpu_seen);
+}
+
+void test_perf_buffer(void)
+{
+ int err, prog_fd, prog_pfd, nr_cpus, i, duration = 0;
+ const char *prog_name = "kprobe/sys_nanosleep";
+ const char *file = "./test_perf_buffer.o";
+ struct bpf_map *perf_buf_map;
+ cpu_set_t cpu_set, cpu_seen;
+ struct bpf_program *prog;
+ struct bpf_object *obj;
+ struct perf_buffer *pb;
+
+ nr_cpus = libbpf_num_possible_cpus();
+ if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
+ return;
+
+ /* load program */
+ err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+ if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+ return;
+
+ prog = bpf_object__find_program_by_title(obj, prog_name);
+ if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
+ goto out_close;
+
+ /* load map */
+ perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
+ if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
+ goto out_close;
+
+ /* attach kprobe */
+ prog_pfd = bpf_program__attach_kprobe(prog, false /* retprobe */,
+ "sys_nanosleep");
+ if (CHECK(prog_pfd < 0, "attach_kprobe", "err %d\n", prog_pfd))
+ goto out_close;
+
+ /* set up perf buffer */
+ pb = perf_buffer__new(perf_buf_map, 1, on_sample, NULL, &cpu_seen);
+ if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
+ goto out_detach;
+
+ /* trigger kprobe on every CPU */
+ CPU_ZERO(&cpu_seen);
+ for (i = 0; i < nr_cpus; i++) {
+ CPU_ZERO(&cpu_set);
+ CPU_SET(i, &cpu_set);
+
+ err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
+ &cpu_set);
+ if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
+ i, err))
+ goto out_detach;
+
+ usleep(1);
+ }
+
+ /* read perf buffer */
+ err = perf_buffer__poll(pb, 100);
+ if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
+ goto out_free_pb;
+
+ if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
+ "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
+ goto out_free_pb;
+
+out_free_pb:
+ perf_buffer__free(pb);
+out_detach:
+ libbpf_perf_event_disable_and_close(prog_pfd);
+out_close:
+ bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
new file mode 100644
index 000000000000..ba961f608fd5
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+ int type;
+ int key_size;
+ int value_size;
+ int max_entries;
+} perf_buf_map SEC(".maps") = {
+ .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 56,
+};
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+ int cpu = bpf_get_smp_processor_id();
+
+ bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
+ &cpu, sizeof(cpu));
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 1/2] libbpf: add perf buffer reading API
From: Andrii Nakryiko @ 2019-06-25 23:26 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190625232601.3227055-1-andriin@fb.com>
BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
to user space for additional processing. libbpf already has very low-level API
to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
use and requires a lot of code to set everything up. This patch adds
perf_buffer abstraction on top of it, abstracting setting up and polling
per-CPU logic into simple and convenient API, similar to what BCC provides.
perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
map entries. It accepts two user-provided callbacks: one for handling raw
samples and one for get notifications of lost samples due to buffer overflow.
perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
utilizing epoll instance.
perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
All APIs are not thread-safe. User should ensure proper locking/coordination if
used in multi-threaded set up.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 282 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 12 ++
tools/lib/bpf/libbpf.map | 5 +-
3 files changed, 298 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9a4199b51300..c74cc535902a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,7 +32,9 @@
#include <linux/limits.h>
#include <linux/perf_event.h>
#include <linux/ring_buffer.h>
+#include <sys/epoll.h>
#include <sys/ioctl.h>
+#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
@@ -4322,6 +4324,286 @@ bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
return ret;
}
+struct perf_cpu_buf {
+ int fd;
+ void *base; /* mmap()'ed memory */
+ void *buf; /* for reconstructing segmented data */
+ size_t buf_size;
+};
+
+struct perf_buffer {
+ perf_buffer_sample_fn sample_cb;
+ perf_buffer_lost_fn lost_cb;
+ void *ctx; /* passed into callbacks */
+
+ size_t page_size;
+ size_t mmap_size;
+ struct perf_cpu_buf **cpu_bufs;
+ struct epoll_event *events;
+ int cpu_cnt;
+ int epfd; /* perf event FD */
+ int mapfd; /* BPF_MAP_TYPE_PERF_EVENT_ARRAY BPF map FD */
+};
+
+static void perf_buffer__free_cpu_buf(struct perf_buffer *pb,
+ struct perf_cpu_buf *cpu_buf, int cpu)
+{
+ if (!cpu_buf)
+ return;
+ if (cpu_buf->base &&
+ munmap(cpu_buf->base, pb->mmap_size + pb->page_size))
+ pr_warning("failed to munmap cpu_buf #%d\n", cpu);
+ if (cpu_buf->fd >= 0) {
+ ioctl(cpu_buf->fd, PERF_EVENT_IOC_DISABLE, 0);
+ close(cpu_buf->fd);
+ }
+ free(cpu_buf->buf);
+ free(cpu_buf);
+}
+
+void perf_buffer__free(struct perf_buffer *pb)
+{
+ int i;
+
+ if (!pb)
+ return;
+ if (pb->cpu_bufs) {
+ for (i = 0; i < pb->cpu_cnt && pb->cpu_bufs[i]; i++) {
+ struct perf_cpu_buf *cpu_buf = pb->cpu_bufs[i];
+
+ bpf_map_delete_elem(pb->mapfd, &i);
+ perf_buffer__free_cpu_buf(pb, cpu_buf, i);
+ }
+ free(pb->cpu_bufs);
+ }
+ if (pb->epfd >= 0)
+ close(pb->epfd);
+ free(pb->events);
+ free(pb);
+}
+
+static struct perf_cpu_buf *perf_buffer__open_cpu_buf(struct perf_buffer *pb,
+ int cpu)
+{
+ struct perf_event_attr attr = {};
+ struct perf_cpu_buf *cpu_buf;
+ char msg[STRERR_BUFSIZE];
+ int err;
+
+ cpu_buf = calloc(1, sizeof(*cpu_buf));
+ if (!cpu_buf)
+ return ERR_PTR(-ENOMEM);
+
+ attr.config = PERF_COUNT_SW_BPF_OUTPUT;
+ attr.type = PERF_TYPE_SOFTWARE;
+ attr.sample_type = PERF_SAMPLE_RAW;
+ attr.sample_period = 1;
+ attr.wakeup_events = 1;
+ cpu_buf->fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, cpu,
+ -1, PERF_FLAG_FD_CLOEXEC);
+ if (cpu_buf->fd < 0) {
+ err = -errno;
+ pr_warning("failed to open perf buffer event on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ cpu_buf->base = mmap(NULL, pb->mmap_size + pb->page_size,
+ PROT_READ | PROT_WRITE, MAP_SHARED,
+ cpu_buf->fd, 0);
+ if (cpu_buf->base == MAP_FAILED) {
+ cpu_buf->base = NULL;
+ err = -errno;
+ pr_warning("failed to mmap perf buffer on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ if (ioctl(cpu_buf->fd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+ err = -errno;
+ pr_warning("failed to enable perf buffer event on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ return cpu_buf;
+
+error:
+ perf_buffer__free_cpu_buf(pb, cpu_buf, cpu);
+ return (struct perf_cpu_buf *)ERR_PTR(err);
+}
+
+struct perf_buffer *perf_buffer__new(struct bpf_map *map, size_t page_cnt,
+ perf_buffer_sample_fn sample_cb,
+ perf_buffer_lost_fn lost_cb, void *ctx)
+{
+ char msg[STRERR_BUFSIZE];
+ struct perf_buffer *pb;
+ int err, cpu;
+
+ if (bpf_map__def(map)->type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
+ pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
+ bpf_map__name(map));
+ return ERR_PTR(-EINVAL);
+ }
+ if (bpf_map__fd(map) < 0) {
+ pr_warning("map '%s' doesn't have associated FD\n",
+ bpf_map__name(map));
+ return ERR_PTR(-EINVAL);
+ }
+ if (page_cnt & (page_cnt - 1)) {
+ pr_warning("page count should be power of two, but is %zu\n",
+ page_cnt);
+ return ERR_PTR(-EINVAL);
+ }
+
+ pb = calloc(1, sizeof(*pb));
+ if (!pb)
+ return ERR_PTR(-ENOMEM);
+
+ pb->sample_cb = sample_cb;
+ pb->lost_cb = lost_cb;
+ pb->ctx = ctx;
+ pb->page_size = getpagesize();
+ pb->mmap_size = pb->page_size * page_cnt;
+ pb->mapfd = bpf_map__fd(map);
+
+ pb->epfd = epoll_create1(EPOLL_CLOEXEC);
+ if (pb->epfd < 0) {
+ err = -errno;
+ pr_warning("failed to create epoll instance: %s\n",
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ pb->cpu_cnt = libbpf_num_possible_cpus();
+ if (pb->cpu_cnt < 0) {
+ err = pb->cpu_cnt;
+ goto error;
+ }
+ pb->events = calloc(pb->cpu_cnt, sizeof(*pb->events));
+ if (!pb->events) {
+ err = -ENOMEM;
+ pr_warning("failed to allocate events: out of memory\n");
+ goto error;
+ }
+ pb->cpu_bufs = calloc(pb->cpu_cnt, sizeof(*pb->cpu_bufs));
+ if (!pb->cpu_bufs) {
+ err = -ENOMEM;
+ pr_warning("failed to allocate buffers: out of memory\n");
+ goto error;
+ }
+
+ for (cpu = 0; cpu < pb->cpu_cnt; cpu++) {
+ struct perf_cpu_buf *cpu_buf;
+
+ cpu_buf = perf_buffer__open_cpu_buf(pb, cpu);
+ if (IS_ERR(cpu_buf)) {
+ err = PTR_ERR(cpu_buf);
+ goto error;
+ }
+
+ pb->cpu_bufs[cpu] = cpu_buf;
+
+ err = bpf_map_update_elem(pb->mapfd, &cpu, &cpu_buf->fd, 0);
+ if (err) {
+ pr_warning("failed to set cpu #%d perf FD %d: %s\n",
+ cpu, cpu_buf->fd,
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ pb->events[cpu].events = EPOLLIN;
+ pb->events[cpu].data.ptr = cpu_buf;
+ if (epoll_ctl(pb->epfd, EPOLL_CTL_ADD, cpu_buf->fd,
+ &pb->events[cpu]) < 0) {
+ err = -errno;
+ pr_warning("failed to epoll_ctl cpu #%d perf FD %d: %s\n",
+ cpu, cpu_buf->fd,
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+ }
+
+ return pb;
+
+error:
+ if (pb)
+ perf_buffer__free(pb);
+ return ERR_PTR(err);
+}
+
+struct perf_sample_raw {
+ struct perf_event_header header;
+ uint32_t size;
+ char data[0];
+};
+
+struct perf_sample_lost {
+ struct perf_event_header header;
+ uint64_t id;
+ uint64_t lost;
+ uint64_t sample_id;
+};
+
+static enum bpf_perf_event_ret
+perf_buffer__process_record(struct perf_event_header *e, void *ctx)
+{
+ struct perf_buffer *pb = ctx;
+ void *data = e;
+
+ switch (e->type) {
+ case PERF_RECORD_SAMPLE: {
+ struct perf_sample_raw *s = data;
+
+ pb->sample_cb(pb->ctx, s->data, s->size);
+ break;
+ }
+ case PERF_RECORD_LOST: {
+ struct perf_sample_lost *s = data;
+
+ if (pb->lost_cb)
+ pb->lost_cb(pb->ctx, s->lost);
+ break;
+ }
+ default:
+ pr_warning("unknown perf sample type %d\n", e->type);
+ return LIBBPF_PERF_EVENT_ERROR;
+ }
+ return LIBBPF_PERF_EVENT_CONT;
+}
+
+static int perf_buffer__process_records(struct perf_buffer *pb,
+ struct perf_cpu_buf *cpu_buf)
+{
+ enum bpf_perf_event_ret ret;
+
+ ret = bpf_perf_event_read_simple(cpu_buf->base, pb->mmap_size,
+ pb->page_size, &cpu_buf->buf,
+ &cpu_buf->buf_size,
+ perf_buffer__process_record, pb);
+ if (ret != LIBBPF_PERF_EVENT_CONT)
+ return ret;
+ return 0;
+}
+
+int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms)
+{
+ int cnt, err;
+
+ cnt = epoll_wait(pb->epfd, pb->events, pb->cpu_cnt, timeout_ms);
+ for (int i = 0; i < cnt; i++) {
+ struct perf_cpu_buf *cpu_buf = pb->events[i].data.ptr;
+
+ err = perf_buffer__process_records(pb, cpu_buf);
+ if (err) {
+ pr_warning("error while processing records: %d\n", err);
+ return err;
+ }
+ }
+ return cnt < 0 ? -errno : cnt;
+}
+
struct bpf_prog_info_array_desc {
int array_offset; /* e.g. offset of jited_prog_insns */
int count_offset; /* e.g. offset of jited_prog_len */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bf7020a565c6..3bfde1a475ce 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -354,6 +354,18 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
+struct perf_buffer;
+typedef void (*perf_buffer_sample_fn)(void *ctx, void *data, __u32 size);
+typedef void (*perf_buffer_lost_fn)(void *ctx, __u64 cnt);
+
+LIBBPF_API struct perf_buffer *perf_buffer__new(struct bpf_map *map,
+ size_t page_cnt,
+ perf_buffer_sample_fn sample_cb,
+ perf_buffer_lost_fn lost_cb,
+ void *ctx);
+LIBBPF_API void perf_buffer__free(struct perf_buffer *pb);
+LIBBPF_API int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms);
+
enum bpf_perf_event_ret {
LIBBPF_PERF_EVENT_DONE = 0,
LIBBPF_PERF_EVENT_ERROR = -1,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2382fbda4cbb..10f48103110a 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
- btf__parse_elf;
bpf_object__load_xattr;
bpf_program__attach_kprobe;
bpf_program__attach_perf_event;
bpf_program__attach_raw_tracepoint;
bpf_program__attach_tracepoint;
bpf_program__attach_uprobe;
+ btf__parse_elf;
libbpf_num_possible_cpus;
libbpf_perf_event_disable_and_close;
+ perf_buffer__free;
+ perf_buffer__new;
+ perf_buffer__poll;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 0/2] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-06-25 23:25 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
This patchset adds a high-level API for setting up and polling perf buffers
associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
described in corresponding commit.
Andrii Nakryiko (2):
libbpf: add perf buffer reading API
selftests/bpf: test perf buffer API
tools/lib/bpf/libbpf.c | 282 ++++++++++++++++++
tools/lib/bpf/libbpf.h | 12 +
tools/lib/bpf/libbpf.map | 5 +-
.../selftests/bpf/prog_tests/perf_buffer.c | 86 ++++++
.../selftests/bpf/progs/test_perf_buffer.c | 31 ++
5 files changed, 415 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
--
2.17.1
^ permalink raw reply
* Re: [PATCH net-next 08/18] ionic: Add notifyq support
From: Jakub Kicinski @ 2019-06-25 23:21 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-9-snelson@pensando.io>
On Thu, 20 Jun 2019 13:24:14 -0700, Shannon Nelson wrote:
> + case EVENT_OPCODE_HEARTBEAT:
> + netdev_info(netdev, "Notifyq EVENT_OPCODE_HEARTBEAT eid=%lld\n",
> + eid);
> + break;
I wonder how often this gets sent and whether the info log level is
really necessary for a correctly working heartbeat?
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Vladimir Oltean @ 2019-06-25 23:13 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <CA+h21hq_w8-96ehKYxcziSq1TjOjoKduZ+pB3umBfjODaKWd+A@mail.gmail.com>
On Wed, 26 Jun 2019 at 02:10, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > > Hi Russell,
> > > > >
> > > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > > This should be removed - state->link is not for use in mac_config.
> > > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > > >
> > > > >
> > > > > What do you mean exactly that state->link is not for use, is that true in
> > > > > general?
> > > >
> > > > Yes. mac_config() should not touch it; it is not always in a defined
> > > > state. For example, if you set modes via ethtool (the
> > > > ethtool_ksettings_set API) then state->link will probably contain
> > > > zero irrespective of the true link state.
> > > >
> > >
> > > Experimentally, state->link is zero at the same time as state->speed
> > > is -1, so just ignoring !state->link made sense. This is not in-band
> > > AN. What is your suggestion? Should I proceed to try and configure the
> > > MAC for SPEED_UNKNOWN?
> >
> > What would you have done with a PHY when the link is down, what speed
> > would you have configured in the phylib adjust_link callback? phylib
> > also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
> >
>
> With phylib, I'd make the driver ignore the speed and do nothing.
> With phylink, I'd make the core not call mac_config.
> But what happened is I saw phylink call mac_config anyway, said
> 'weird' and proceeded to ignore it as I would have for phylib.
> I'm just not understanding your position - it seems like you're
> implying there's a bug in phylink and the function call with
> MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
I meant MLO_AN_PHY, sorry.
> place, which is what I wanted to confirm.
>
> > What we do in Marvell drivers is set to the lowest speed (10M) in such
> > cases, which is fine as the MAC supports 10M.
> >
> > It wouldn't be appropriate for phylink to force something on MAC
> > drivers, it's easier if the MAC just defaults SPEED_UNKNOWN to something
> > itself.
> >
> > >
> > > > It exists in this structure because it was convenient to just use one
> > > > structure to store all the link information in various parts of the
> > > > code, and when requesting the negotiated in-band MAC configuration.
> > > >
> > > > I've come to the conclusion that that decision was a mistake, based
> > > > on patches such as the above mistakenly thinking that everything in
> > > > the state structure is fair game. I've since updated the docs to
> > > > explicitly spell it out, but I'm also looking at the feasibility of
> > > > changing the mac_config() interface entirely - splitting it into two
> > > > (mac_config_fixed() and mac_config_inband()) and passing only the
> > > > appropriate parameters to each.
> > > >
> > > > However, having looked at that, I think such a change will make some
> > > > MAC drivers quite a bit more complicated - having all the config
> > > > steps in one method appears to make the configuration of MAC drivers
> > > > easier (eg, mvneta, mvpp2.)
> > > >
> > > > > In drivers/net/dsa/sja1105/sja1105_main.c, if I remove the "if
> > > > > (!state->link)" guard, I see PHYLINK calls with a SPEED_UNKNOWN argument for
> > > > > ports that are BR_STATE_DISABLED. Is that normal?
> > > >
> > > > This looks like another driver which has been converted to phylink
> > > > without my review; I certainly wasn't aware of it. It gets a few
> > > > things wrong, such as:
> > > >
> > > > 1) not checking state->interface in the validate callback - so it
> > > > is basically saying that it can support any PHY interface mode
> > > > that the kernel happens to support.
> > > >
> > >
> > > Partially true. It does check the DT bindings for supported MII modes
> > > in sja1105_init_mii_settings (for fundamental reasons... the switch
> > > expects an 'all-in-one' configuration buffer with the operating modes
> > > of all MACs - don't ask me to delay the uploading of this static
> > > config until all ports collected their interface_mode from phylink via
> > > the mac_config callback - it's a deadlock).
> >
> > Ok, so you need to reject interface modes that are not compatible
> > with the currently configured mode in the validate() callback, but
> > please keep PHY_INTERFACE_MODE_NA reporting back the capabilities.
> > (this is now documented.)
> >
>
> Ok.
>
> > > It is a gigabit switch with MII/RMII/RGMII MACs - I have never seen
> > > any PHY wired for these modes that can change system interface type.
> >
> > It is unlikely that MII/RMII/RGMII will switch modes, but in terms of
> > correct implementation, sticking to the way the function is expected
> > to behave means that I don't get surprises when changing phylink layer
> > in the future.
> >
> > >
> > > > 2) if phylink is configured to use in-band, then state->speed is
> > > > undefined; this driver will fail. (If folk don't want to support
> > > > that, we ought to have a way to tell phylink to reject anything
> > > > that attempts to set it to in-band mode!)
> > > >
> > >
> > > Ok.
> > >
> > > > 3) it doesn't implement phylink_mac_link_state DSA ops, so it doesn't
> > > > support SGMII or 802.3z phy interface modes (see 1).
> > > >
> > >
> > > No, it doesn't.
> > > Some odd switch in this device family supports SGMII on 1 of its
> > > ports, however I haven't put my hands on it.
> > > When I do I'll add checks for strange scenarios, like connecting it to
> > > an Aquantia PHY that can switch between SGMII and USXGMII (although
> > > why would anyone pair a 10G capable PHY to a 1G capable MAC...)
> >
> > It's unlikely that it would switch between SGMII and USXGMII
> > dynamically, as USXGMII supports speeds from 10G down to 10M.
> >
> > Where interface mode switching tends to be used is with modes such
> > as 10GBASE-R, which doesn't support anything except 10G. In order
> > for the PHY to operate at slower speeds, it has a few options:
> >
> > 1) perform rate adaption.
> > 2) dynamically switch interface type to an interface type that
> > supports the desired speed.
> > 3) just not support slower speeds.
> >
>
> So am I reading this correctly - it kind of makes sense for gigabit
> MAC drivers to not check for the MII interface changing protocol?
>
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
>
> Thanks,
> -Vladimir
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Vladimir Oltean @ 2019-06-25 23:10 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <20190625225759.zztqgnwtk4v7milp@shell.armlinux.org.uk>
On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > Hi Russell,
> > > >
> > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > This should be removed - state->link is not for use in mac_config.
> > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > >
> > > >
> > > > What do you mean exactly that state->link is not for use, is that true in
> > > > general?
> > >
> > > Yes. mac_config() should not touch it; it is not always in a defined
> > > state. For example, if you set modes via ethtool (the
> > > ethtool_ksettings_set API) then state->link will probably contain
> > > zero irrespective of the true link state.
> > >
> >
> > Experimentally, state->link is zero at the same time as state->speed
> > is -1, so just ignoring !state->link made sense. This is not in-band
> > AN. What is your suggestion? Should I proceed to try and configure the
> > MAC for SPEED_UNKNOWN?
>
> What would you have done with a PHY when the link is down, what speed
> would you have configured in the phylib adjust_link callback? phylib
> also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
>
With phylib, I'd make the driver ignore the speed and do nothing.
With phylink, I'd make the core not call mac_config.
But what happened is I saw phylink call mac_config anyway, said
'weird' and proceeded to ignore it as I would have for phylib.
I'm just not understanding your position - it seems like you're
implying there's a bug in phylink and the function call with
MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
place, which is what I wanted to confirm.
> What we do in Marvell drivers is set to the lowest speed (10M) in such
> cases, which is fine as the MAC supports 10M.
>
> It wouldn't be appropriate for phylink to force something on MAC
> drivers, it's easier if the MAC just defaults SPEED_UNKNOWN to something
> itself.
>
> >
> > > It exists in this structure because it was convenient to just use one
> > > structure to store all the link information in various parts of the
> > > code, and when requesting the negotiated in-band MAC configuration.
> > >
> > > I've come to the conclusion that that decision was a mistake, based
> > > on patches such as the above mistakenly thinking that everything in
> > > the state structure is fair game. I've since updated the docs to
> > > explicitly spell it out, but I'm also looking at the feasibility of
> > > changing the mac_config() interface entirely - splitting it into two
> > > (mac_config_fixed() and mac_config_inband()) and passing only the
> > > appropriate parameters to each.
> > >
> > > However, having looked at that, I think such a change will make some
> > > MAC drivers quite a bit more complicated - having all the config
> > > steps in one method appears to make the configuration of MAC drivers
> > > easier (eg, mvneta, mvpp2.)
> > >
> > > > In drivers/net/dsa/sja1105/sja1105_main.c, if I remove the "if
> > > > (!state->link)" guard, I see PHYLINK calls with a SPEED_UNKNOWN argument for
> > > > ports that are BR_STATE_DISABLED. Is that normal?
> > >
> > > This looks like another driver which has been converted to phylink
> > > without my review; I certainly wasn't aware of it. It gets a few
> > > things wrong, such as:
> > >
> > > 1) not checking state->interface in the validate callback - so it
> > > is basically saying that it can support any PHY interface mode
> > > that the kernel happens to support.
> > >
> >
> > Partially true. It does check the DT bindings for supported MII modes
> > in sja1105_init_mii_settings (for fundamental reasons... the switch
> > expects an 'all-in-one' configuration buffer with the operating modes
> > of all MACs - don't ask me to delay the uploading of this static
> > config until all ports collected their interface_mode from phylink via
> > the mac_config callback - it's a deadlock).
>
> Ok, so you need to reject interface modes that are not compatible
> with the currently configured mode in the validate() callback, but
> please keep PHY_INTERFACE_MODE_NA reporting back the capabilities.
> (this is now documented.)
>
Ok.
> > It is a gigabit switch with MII/RMII/RGMII MACs - I have never seen
> > any PHY wired for these modes that can change system interface type.
>
> It is unlikely that MII/RMII/RGMII will switch modes, but in terms of
> correct implementation, sticking to the way the function is expected
> to behave means that I don't get surprises when changing phylink layer
> in the future.
>
> >
> > > 2) if phylink is configured to use in-band, then state->speed is
> > > undefined; this driver will fail. (If folk don't want to support
> > > that, we ought to have a way to tell phylink to reject anything
> > > that attempts to set it to in-band mode!)
> > >
> >
> > Ok.
> >
> > > 3) it doesn't implement phylink_mac_link_state DSA ops, so it doesn't
> > > support SGMII or 802.3z phy interface modes (see 1).
> > >
> >
> > No, it doesn't.
> > Some odd switch in this device family supports SGMII on 1 of its
> > ports, however I haven't put my hands on it.
> > When I do I'll add checks for strange scenarios, like connecting it to
> > an Aquantia PHY that can switch between SGMII and USXGMII (although
> > why would anyone pair a 10G capable PHY to a 1G capable MAC...)
>
> It's unlikely that it would switch between SGMII and USXGMII
> dynamically, as USXGMII supports speeds from 10G down to 10M.
>
> Where interface mode switching tends to be used is with modes such
> as 10GBASE-R, which doesn't support anything except 10G. In order
> for the PHY to operate at slower speeds, it has a few options:
>
> 1) perform rate adaption.
> 2) dynamically switch interface type to an interface type that
> supports the desired speed.
> 3) just not support slower speeds.
>
So am I reading this correctly - it kind of makes sense for gigabit
MAC drivers to not check for the MII interface changing protocol?
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
Thanks,
-Vladimir
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Write in validate_chain
From: syzbot @ 2019-06-25 23:07 UTC (permalink / raw)
To: akpm, ast, cai, crecklin, daniel, john.fastabend, keescook,
linux-kernel, linux-mm, netdev, syzkaller-bugs
In-Reply-To: <000000000000e672c6058bd7ee45@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=14a4e9b5a00000
start commit: abf02e29 Merge tag 'pm-5.2-rc6' of git://git.kernel.org/pu..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=16a4e9b5a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=12a4e9b5a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=28ec3437a5394ee0
dashboard link: https://syzkaller.appspot.com/bug?extid=8893700724999566d6a9
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=167098b2a00000
Reported-by: syzbot+8893700724999566d6a9@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox