Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/3] net: socionext: Stop PHY before resetting netsec
From: masahisa.kojima @ 2018-10-19  1:08 UTC (permalink / raw)
  To: netdev
  Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
	osaki.yoshitoyo, Masahisa Kojima
In-Reply-To: <20181019010843.3605-1-masahisa.kojima@linaro.org>

From: Masahisa Kojima <masahisa.kojima@linaro.org>

After resetting netsec IP, driver have to wait until
netsec mode turns to NRM mode.
But sometimes mode transition to NRM will not complete
if the PHY is in normal operation state.
To avoid this situation, stop PHY before resetting netsec.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
---
 drivers/net/ethernet/socionext/netsec.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 4289ccb26e4e..273cc5fc07e0 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1343,11 +1343,11 @@ static int netsec_netdev_stop(struct net_device *ndev)
 	netsec_uninit_pkt_dring(priv, NETSEC_RING_TX);
 	netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
 
-	ret = netsec_reset_hardware(priv, false);
-
 	phy_stop(ndev->phydev);
 	phy_disconnect(ndev->phydev);
 
+	ret = netsec_reset_hardware(priv, false);
+
 	pm_runtime_put_sync(priv->dev);
 
 	return ret;
@@ -1415,7 +1415,7 @@ static const struct net_device_ops netsec_netdev_ops = {
 };
 
 static int netsec_of_probe(struct platform_device *pdev,
-			   struct netsec_priv *priv)
+			   struct netsec_priv *priv, u32 *phy_addr)
 {
 	priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
 	if (!priv->phy_np) {
@@ -1423,6 +1423,8 @@ static int netsec_of_probe(struct platform_device *pdev,
 		return -EINVAL;
 	}
 
+	*phy_addr = of_mdio_parse_addr(&pdev->dev, priv->phy_np);
+
 	priv->clk = devm_clk_get(&pdev->dev, NULL); /* get by 'phy_ref_clk' */
 	if (IS_ERR(priv->clk)) {
 		dev_err(&pdev->dev, "phy_ref_clk not found\n");
@@ -1473,6 +1475,7 @@ static int netsec_register_mdio(struct netsec_priv *priv, u32 phy_addr)
 {
 	struct mii_bus *bus;
 	int ret;
+	u16 data;
 
 	bus = devm_mdiobus_alloc(priv->dev);
 	if (!bus)
@@ -1486,6 +1489,10 @@ static int netsec_register_mdio(struct netsec_priv *priv, u32 phy_addr)
 	bus->parent = priv->dev;
 	priv->mii_bus = bus;
 
+	/* set phy power down */
+	data = netsec_phy_read(bus, phy_addr, 0) | 0x800;
+	netsec_phy_write(bus, phy_addr, 0, data);
+
 	if (dev_of_node(priv->dev)) {
 		struct device_node *mdio_node, *parent = dev_of_node(priv->dev);
 
@@ -1623,7 +1630,7 @@ static int netsec_probe(struct platform_device *pdev)
 	}
 
 	if (dev_of_node(&pdev->dev))
-		ret = netsec_of_probe(pdev, priv);
+		ret = netsec_of_probe(pdev, priv, &phy_addr);
 	else
 		ret = netsec_acpi_probe(pdev, priv, &phy_addr);
 	if (ret)
-- 
2.14.2

^ permalink raw reply related

* [PATCH 2/3] net: socionext: Add dummy PHY register read in phy_write()
From: masahisa.kojima @ 2018-10-19  1:08 UTC (permalink / raw)
  To: netdev
  Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
	osaki.yoshitoyo, Masahisa Kojima
In-Reply-To: <20181019010843.3605-1-masahisa.kojima@linaro.org>

From: Masahisa Kojima <masahisa.kojima@linaro.org>

There is a compatibility issue between RTL8211E implemented
in Developerbox and netsec network controller IP(F_GMAC4).

RTL8211E expects MDC clock must be kept toggling for several
clock cycle with MDIO high before entering the IDLE state.
To meet this requirement, netsec driver needs to issue dummy
read(e.g. read PHYID1(offset 0x2) register) right after write.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
---
 drivers/net/ethernet/socionext/netsec.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index 273cc5fc07e0..e7faaf8be99e 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -431,9 +431,12 @@ static int netsec_mac_update_to_phy_state(struct netsec_priv *priv)
 	return 0;
 }
 
+static int netsec_phy_read(struct mii_bus *bus, int phy_addr, int reg_addr);
+
 static int netsec_phy_write(struct mii_bus *bus,
 			    int phy_addr, int reg, u16 val)
 {
+	int status;
 	struct netsec_priv *priv = bus->priv;
 
 	if (netsec_mac_write(priv, GMAC_REG_GDR, val))
@@ -446,8 +449,19 @@ static int netsec_phy_write(struct mii_bus *bus,
 			      GMAC_REG_SHIFT_CR_GAR)))
 		return -ETIMEDOUT;
 
-	return netsec_mac_wait_while_busy(priv, GMAC_REG_GAR,
-					  NETSEC_GMAC_GAR_REG_GB);
+	status = netsec_mac_wait_while_busy(priv, GMAC_REG_GAR,
+					    NETSEC_GMAC_GAR_REG_GB);
+
+	/* Developerbox implements RTL8211E PHY and there is
+	 * a compatibility problem with F_GMAC4.
+	 * RTL8211E expects MDC clock must be kept toggling for several
+	 * clock cycle with MDIO high before entering the IDLE state.
+	 * To meet this requirement, netsec driver needs to issue dummy
+	 * read(e.g. read PHYID1(offset 0x2) register) right after write.
+	 */
+	netsec_phy_read(bus, phy_addr, 2);
+
+	return status;
 }
 
 static int netsec_phy_read(struct mii_bus *bus, int phy_addr, int reg_addr)
-- 
2.14.2

^ permalink raw reply related

* [PATCH 3/3] net: socionext: Reset tx queue in ndo_stop
From: masahisa.kojima @ 2018-10-19  1:08 UTC (permalink / raw)
  To: netdev
  Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
	osaki.yoshitoyo, Masahisa Kojima
In-Reply-To: <20181019010843.3605-1-masahisa.kojima@linaro.org>

From: Masahisa Kojima <masahisa.kojima@linaro.org>

Without resetting tx queue in ndo_stop, packets and bytes count
are not reset when the interface is down.
Eventually, tx queue is exhausted and packets will not be
sent out.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
---
 drivers/net/ethernet/socionext/netsec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index e7faaf8be99e..4b32da76d577 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -954,6 +954,9 @@ static void netsec_uninit_pkt_dring(struct netsec_priv *priv, int id)
 	dring->head = 0;
 	dring->tail = 0;
 	dring->pkt_cnt = 0;
+
+	if (id == NETSEC_RING_TX)
+		netdev_reset_queue(priv->ndev);
 }
 
 static void netsec_free_dring(struct netsec_priv *priv, int id)
-- 
2.14.2

^ permalink raw reply related

* [PATCH v3 1/2] dt-bindings: phy: Update SERDES_MAX to be SERDES_MAX + 1
From: Gustavo A. R. Silva @ 2018-10-19  9:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: Rob Herring, Mark Rutland, devicetree, Kishon Vijay Abraham I,
	David S. Miller, Quentin Schulz, netdev, Gustavo A. R. Silva
In-Reply-To: <cover.1539940387.git.gustavo@embeddedor.com>

SERDES_MAX is a valid value to index ctrl->phys in
drivers/phy/mscc/phy-ocelot-serdes.c. But, currently,
there is an out-of-bounds bug in the mentioned driver
when reading from ctrl->phys, because the size of
array ctrl->phys is SERDES_MAX.

Partially fix this by updating SERDES_MAX to be SERDES6G_MAX + 1.

Notice that this is the first part of the solution to
the out-of-bounds bug mentioned above. Although this
change is not dependent on any other one.

Suggested-by: Quentin Schulz <quentin.schulz@bootlin.com>
Reviewed-by: Quentin Schulz <quentin.schulz@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
Changes in v3:
 - Post the patch to netdev.

Changes in v2:
 - Add Quentin's Reviewed-by to commit log.
 - Add Rob's Acked-by to commit log.

 include/dt-bindings/phy/phy-ocelot-serdes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dt-bindings/phy/phy-ocelot-serdes.h b/include/dt-bindings/phy/phy-ocelot-serdes.h
index bd28f21..fe70ada 100644
--- a/include/dt-bindings/phy/phy-ocelot-serdes.h
+++ b/include/dt-bindings/phy/phy-ocelot-serdes.h
@@ -7,6 +7,6 @@
 #define SERDES1G_MAX	SERDES1G(5)
 #define SERDES6G(x)	(SERDES1G_MAX + 1 + (x))
 #define SERDES6G_MAX	SERDES6G(2)
-#define SERDES_MAX	SERDES6G_MAX
+#define SERDES_MAX	(SERDES6G_MAX + 1)
 
 #endif
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 2/2] phy: ocelot-serdes: fix out-of-bounds read
From: Gustavo A. R. Silva @ 2018-10-19  9:21 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kishon Vijay Abraham I, David S. Miller, Quentin Schulz, netdev,
	Gustavo A. R. Silva
In-Reply-To: <cover.1539940387.git.gustavo@embeddedor.com>

Currently, there is an out-of-bounds read on array ctrl->phys,
once variable i reaches the maximum array size of SERDES_MAX
in the for loop.

Fix this by changing the condition in the for loop from
i <= SERDES_MAX to i < SERDES_MAX.

Addresses-Coverity-ID: 1473966 ("Out-of-bounds read")
Addresses-Coverity-ID: 1473959 ("Out-of-bounds read")
Fixes: 51f6b410fc22 ("phy: add driver for Microsemi Ocelot SerDes muxing")
Reviewed-by: Quentin Schulz <quentin.schulz@bootlin.com>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
Changes in v3:
 - Post the patch to netdev.

Changes in v2:
 - Rebase and add Quentin's Reviewed-by to commit log.

 drivers/phy/mscc/phy-ocelot-serdes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
index b2be546..cbb49d9 100644
--- a/drivers/phy/mscc/phy-ocelot-serdes.c
+++ b/drivers/phy/mscc/phy-ocelot-serdes.c
@@ -206,7 +206,7 @@ static struct phy *serdes_simple_xlate(struct device *dev,
 	port = args->args[0];
 	idx = args->args[1];
 
-	for (i = 0; i <= SERDES_MAX; i++) {
+	for (i = 0; i < SERDES_MAX; i++) {
 		struct serdes_macro *macro = phy_get_drvdata(ctrl->phys[i]);
 
 		if (idx != macro->idx)
@@ -260,7 +260,7 @@ static int serdes_probe(struct platform_device *pdev)
 	if (IS_ERR(ctrl->regs))
 		return PTR_ERR(ctrl->regs);
 
-	for (i = 0; i <= SERDES_MAX; i++) {
+	for (i = 0; i < SERDES_MAX; i++) {
 		ret = serdes_phy_create(ctrl, i, &ctrl->phys[i]);
 		if (ret)
 			return ret;
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v4 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
From: Alexei Starovoitov @ 2018-10-19  1:33 UTC (permalink / raw)
  To: Daniel Borkmann, Song Liu, netdev@vger.kernel.org
  Cc: ast@kernel.org, Kernel Team
In-Reply-To: <74f40850-9040-4131-b6a0-5727e9e5e19e@iogearbox.net>

On 10/18/18 5:14 PM, Daniel Borkmann wrote:
>> +	case bpf_ctx_range(struct __sk_buff, data_meta):
>> +	case bpf_ctx_range(struct __sk_buff, flow_keys):
>> +		return false;
> ... if it's disallowed anyway (disallowing it is the right thing to do,
> but no need to save/restore then..)?
>

that's a good point.
why shouldn't we allow cg_skb to access data_meta?
xdp can set it and cgroup_skb_ingress will consume it here.


^ permalink raw reply

* [PATCH] can: flexcan: Implement CAN Runtime PM
From: Joakim Zhang @ 2018-10-19  9:45 UTC (permalink / raw)
  To: wg@grandegger.com, mkl@pengutronix.de, davem@davemloft.net
  Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, dl-linux-imx, A.s. Dong,
	Joakim Zhang

From: Dong Aisheng <aisheng.dong@nxp.com>

Flexcan will be disabled during suspend if no wakeup function required
and enabled after resume accordingly. During this period, we could
explicitly disable clocks.

Implement Runtime PM which will:
1) Keep device in suspend state (clocks disabled) if it's not openned
2) Make Power Domain framework be able to shutdown the corresponding power
domain of this device.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 drivers/net/can/flexcan.c | 125 +++++++++++++++++++++++++-------------
 1 file changed, 82 insertions(+), 43 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8e972ef08637..3813f6708201 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
 #define DRV_NAME			"flexcan"
@@ -266,6 +267,7 @@ struct flexcan_priv {
 	u32 reg_imask1_default;
 	u32 reg_imask2_default;
 
+	struct device *dev;
 	struct clk *clk_ipg;
 	struct clk *clk_per;
 	const struct flexcan_devtype_data *devtype_data;
@@ -369,6 +371,27 @@ static inline void flexcan_error_irq_disable(const struct flexcan_priv *priv)
 	priv->write(reg_ctrl, &regs->ctrl);
 }
 
+static int flexcan_clks_enable(const struct flexcan_priv *priv)
+{
+	int err;
+
+	err = clk_prepare_enable(priv->clk_ipg);
+	if (err)
+		return err;
+
+	err = clk_prepare_enable(priv->clk_per);
+	if (err)
+		clk_disable_unprepare(priv->clk_ipg);
+
+	return err;
+}
+
+static void flexcan_clks_disable(const struct flexcan_priv *priv)
+{
+	clk_disable_unprepare(priv->clk_ipg);
+	clk_disable_unprepare(priv->clk_per);
+}
+
 static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
 {
 	if (!priv->reg_xceiver)
@@ -495,19 +518,11 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 	const struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
-	err = clk_prepare_enable(priv->clk_ipg);
-	if (err)
-		return err;
-
-	err = clk_prepare_enable(priv->clk_per);
-	if (err)
-		goto out_disable_ipg;
+	pm_runtime_get_sync(priv->dev);
 
 	err = __flexcan_get_berr_counter(dev, bec);
 
-	clk_disable_unprepare(priv->clk_per);
- out_disable_ipg:
-	clk_disable_unprepare(priv->clk_ipg);
+	pm_runtime_put(priv->dev);
 
 	return err;
 }
@@ -1096,17 +1111,13 @@ static int flexcan_open(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
-	err = clk_prepare_enable(priv->clk_ipg);
+	err = pm_runtime_get_sync(priv->dev);
 	if (err)
 		return err;
 
-	err = clk_prepare_enable(priv->clk_per);
-	if (err)
-		goto out_disable_ipg;
-
 	err = open_candev(dev);
 	if (err)
-		goto out_disable_per;
+		goto out_pm_runtime;
 
 	err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
 	if (err)
@@ -1128,10 +1139,8 @@ static int flexcan_open(struct net_device *dev)
 	free_irq(dev->irq, dev);
  out_close:
 	close_candev(dev);
- out_disable_per:
-	clk_disable_unprepare(priv->clk_per);
- out_disable_ipg:
-	clk_disable_unprepare(priv->clk_ipg);
+ out_pm_runtime:
+	pm_runtime_put(priv->dev);
 
 	return err;
 }
@@ -1145,11 +1154,11 @@ static int flexcan_close(struct net_device *dev)
 	flexcan_chip_stop(dev);
 
 	free_irq(dev->irq, dev);
-	clk_disable_unprepare(priv->clk_per);
-	clk_disable_unprepare(priv->clk_ipg);
 
 	close_candev(dev);
 
+	pm_runtime_put(priv->dev);
+
 	can_led_event(dev, CAN_LED_EVENT_STOP);
 
 	return 0;
@@ -1188,18 +1197,10 @@ static int register_flexcandev(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->regs;
 	u32 reg, err;
 
-	err = clk_prepare_enable(priv->clk_ipg);
-	if (err)
-		return err;
-
-	err = clk_prepare_enable(priv->clk_per);
-	if (err)
-		goto out_disable_ipg;
-
 	/* select "bus clock", chip must be disabled */
 	err = flexcan_chip_disable(priv);
 	if (err)
-		goto out_disable_per;
+		return err;
 	reg = priv->read(&regs->ctrl);
 	reg |= FLEXCAN_CTRL_CLK_SRC;
 	priv->write(reg, &regs->ctrl);
@@ -1228,13 +1229,8 @@ static int register_flexcandev(struct net_device *dev)
 
 	err = register_candev(dev);
 
-	/* disable core and turn off clocks */
  out_chip_disable:
 	flexcan_chip_disable(priv);
- out_disable_per:
-	clk_disable_unprepare(priv->clk_per);
- out_disable_ipg:
-	clk_disable_unprepare(priv->clk_ipg);
 
 	return err;
 }
@@ -1342,6 +1338,7 @@ static int flexcan_probe(struct platform_device *pdev)
 		priv->write = flexcan_write_le;
 	}
 
+	priv->dev = &pdev->dev;
 	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
@@ -1388,22 +1385,35 @@ static int flexcan_probe(struct platform_device *pdev)
 	if (err)
 		goto failed_offload;
 
+	pm_runtime_enable(&pdev->dev);
+	err = pm_runtime_get_sync(&pdev->dev);
+	if (err < 0) {
+		dev_err(&pdev->dev, "pm_runtime_get failed(%d)\n", err);
+		goto failed_rpm_disable;
+	}
+
 	err = register_flexcandev(dev);
 	if (err) {
 		dev_err(&pdev->dev, "registering netdev failed\n");
-		goto failed_register;
+		goto failed_rpm_put;
 	}
 
 	devm_can_led_init(dev);
 
+	pm_runtime_put(&pdev->dev);
+
 	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
 		 priv->regs, dev->irq);
 
 	return 0;
 
+ failed_rpm_put:
+	pm_runtime_put(&pdev->dev);
+ failed_rpm_disable:
+	pm_runtime_disable(&pdev->dev);
  failed_offload:
- failed_register:
 	free_candev(dev);
+
 	return err;
 }
 
@@ -1413,6 +1423,7 @@ static int flexcan_remove(struct platform_device *pdev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 
 	unregister_flexcandev(dev);
+	pm_runtime_disable(&pdev->dev);
 	can_rx_offload_del(&priv->offload);
 	free_candev(dev);
 
@@ -1423,38 +1434,66 @@ static int __maybe_unused flexcan_suspend(struct device *device)
 {
 	struct net_device *dev = dev_get_drvdata(device);
 	struct flexcan_priv *priv = netdev_priv(dev);
-	int err;
+	int err = 0;
 
 	if (netif_running(dev)) {
 		err = flexcan_chip_disable(priv);
 		if (err)
 			return err;
+		err = pm_runtime_force_suspend(device);
+
 		netif_stop_queue(dev);
 		netif_device_detach(dev);
 	}
 	priv->can.state = CAN_STATE_SLEEPING;
 
-	return 0;
+	return err;
 }
 
 static int __maybe_unused flexcan_resume(struct device *device)
 {
 	struct net_device *dev = dev_get_drvdata(device);
 	struct flexcan_priv *priv = netdev_priv(dev);
-	int err;
+	int err = 0;
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 	if (netif_running(dev)) {
 		netif_device_attach(dev);
 		netif_start_queue(dev);
-		err = flexcan_chip_enable(priv);
+
+		err = pm_runtime_force_resume(device);
 		if (err)
 			return err;
+		err = flexcan_chip_enable(priv);
 	}
+	return err;
+}
+
+static int __maybe_unused flexcan_runtime_suspend(struct device *device)
+{
+	struct net_device *dev = dev_get_drvdata(device);
+	struct flexcan_priv *priv = netdev_priv(dev);
+
+	flexcan_clks_disable(priv);
+
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(flexcan_pm_ops, flexcan_suspend, flexcan_resume);
+static int __maybe_unused flexcan_runtime_resume(struct device *device)
+{
+	struct net_device *dev = dev_get_drvdata(device);
+	struct flexcan_priv *priv = netdev_priv(dev);
+
+	flexcan_clks_enable(priv);
+
+	return 0;
+}
+
+static const struct dev_pm_ops flexcan_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(flexcan_suspend, flexcan_resume)
+	SET_RUNTIME_PM_OPS(flexcan_runtime_suspend, flexcan_runtime_resume,
+						NULL)
+};
 
 static struct platform_driver flexcan_driver = {
 	.driver = {
-- 
2.17.1

^ permalink raw reply related

* BUG: unable to handle kernel NULL pointer dereference in __do_softirq
From: syzbot @ 2018-10-19  9:49 UTC (permalink / raw)
  To: brouer, davem, edumazet, jasowang, linux-kernel, mst, netdev, sd,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    4cb967a3d9d9 selftests/bpf: fix file resource leak
git tree:       bpf-next
console output: https://syzkaller.appspot.com/x/log.txt?x=15a82ba1400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2c8e578694e13f29
dashboard link: https://syzkaller.appspot.com/bug?extid=6528c946c37ba2809d0a
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=127664f6400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6528c946c37ba2809d0a@syzkaller.appspotmail.com

device nr0\x01 entered promiscuous mode
device nr0\x01 entered promiscuous mode
device nr0\x01 entered promiscuous mode
device nr0\x01 entered promiscuous mode
device nr0\x01 entered promiscuous mode
BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
PGD 1d3a54067 P4D 1d3a54067 PUD 1ba78a067 PMD 0
Oops: 0010 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 10511 Comm: syz-executor4 Not tainted 4.19.0-rc7+ #125
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:          (null)
Code: Bad RIP value.
RSP: 0018:ffff8801daf07868 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff860a27bf
RDX: 0000000000000100 RSI: 0000000000000000 RDI: ffff8801ab2bd298
RBP: ffff8801daf07df8 R08: ffff8801c7b2e140 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
R13: dffffc0000000000 R14: ffff8801daf07dd0 R15: ffff8801ab2bd298
FS:  00007f851cacc700(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 00000001bc908000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  <IRQ>
  __do_softirq+0x30b/0xad8 kernel/softirq.c:292
  do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:1047
  </IRQ>
  do_softirq.part.13+0x126/0x160 kernel/softirq.c:336
  do_softirq kernel/softirq.c:328 [inline]
  __local_bh_enable_ip+0x21d/0x260 kernel/softirq.c:189
  local_bh_enable include/linux/bottom_half.h:32 [inline]
  tun_get_user+0x1c0b/0x4250 drivers/net/tun.c:1963
  tun_chr_write_iter+0xb9/0x160 drivers/net/tun.c:1993
  call_write_iter include/linux/fs.h:1808 [inline]
  new_sync_write fs/read_write.c:474 [inline]
  __vfs_write+0x6b8/0x9f0 fs/read_write.c:487
  vfs_write+0x1fc/0x560 fs/read_write.c:549
  ksys_write+0x101/0x260 fs/read_write.c:598
  __do_sys_write fs/read_write.c:610 [inline]
  __se_sys_write fs/read_write.c:607 [inline]
  __x64_sys_write+0x73/0xb0 fs/read_write.c:607
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457569
Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f851cacbc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457569
RDX: 000000000000017b RSI: 0000000020000000 RDI: 000000000000000e
RBP: 000000000072c040 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f851cacc6d4
R13: 00000000004c5560 R14: 00000000004d8e98 R15: 00000000ffffffff
Modules linked in:
CR2: 0000000000000000
---[ end trace 108e300304238a6e ]---
RIP: 0010:          (null)
Code: Bad RIP value.
RSP: 0018:ffff8801daf07868 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff860a27bf
RDX: 0000000000000100 RSI: 0000000000000000 RDI: ffff8801ab2bd298
RBP: ffff8801daf07df8 R08: ffff8801c7b2e140 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
R13: dffffc0000000000 R14: ffff8801daf07dd0 R15: ffff8801ab2bd298
FS:  00007f851cacc700(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 00000001bc908000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [PATCH v4 bpf-next 0/2] bpf: add cg_skb_is_valid_access
From: Alexei Starovoitov @ 2018-10-19  1:46 UTC (permalink / raw)
  To: Alexei Starovoitov, Song Liu
  Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	Kernel Team
In-Reply-To: <20181018233403.xnea2m7ntwxue44r@ast-mbp.dhcp.thefacebook.com>

On 10/18/18 4:34 PM, Alexei Starovoitov wrote:
> On Thu, Oct 18, 2018 at 09:06:47AM -0700, Song Liu wrote:
>> Changes v3 -> v4:
>> 1. Fixed crash issue reported by Alexei.
>>
>> Changes v2 -> v3:
>> 1. Added helper function bpf_compute_and_save_data_pointers() and
>>    bpf_restore_data_pointers().
>>
>> Changes v1 -> v2:
>> 1. Updated the list of read-only fields, and read-write fields.
>> 2. Added dummy sk to bpf_prog_test_run_skb().
>>
>> This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access
>> some __skb_buff data directly.
>
> Applied, Thanks

Daniel brought up good point re: data_meta. Let's address it one way
or the other and respin the series instead of adding follow up patches.
So I tossed this series out of bpf-next.

^ permalink raw reply

* Re: netconsole warning in 4.19.0-rc7
From: Meelis Roos @ 2018-10-19  9:51 UTC (permalink / raw)
  To: Dave Jones, Cong Wang, LKML, Linux Kernel Network Developers
In-Reply-To: <20181018145922.ytqxvdihp3iqounl@codemonkey.org.uk>

>   > > I took another look at that error path. Turns out this is all we need I
>   > > think..
>   >
>   > With this patch applied on top of 4.19-rc8, I stll get the warning:
>   >
>   > [   13.722919] WARNING: CPU: 0 PID: 0 at kernel/softirq.c:168 __local_bh_enable_ip+0x2e/0x44
> 
> It's going to be a couple days before I get chance to get back to this.
> Did the previous patch in this thread (without the _bh) fare any better?

Tried it with rcu_read_unlock() instead, still the same.

-- 
Meelis Roos <mroos@linux.ee>

^ permalink raw reply

* Re: [PATCH v2 net] r8169: fix NAPI handling under high load
From: Eric Dumazet @ 2018-10-19  1:51 UTC (permalink / raw)
  To: Francois Romieu
  Cc: Heiner Kallweit, David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org
In-Reply-To: <20181018225900.GA16653@electric-eye.fr.zoreil.com>



On 10/18/2018 03:59 PM, Francois Romieu wrote:
> Eric Dumazet <eric.dumazet@gmail.com> :
> [...]
>> One has to wonder why rtl8169_poll(), which might be called in a loop under DOS,
>> has to call rtl_ack_events() ?
> 
> So as to cover a wider temporal range before any event can trigger an
> extra irq. I was more worried about irq cost than about IO cost (and
> I still am).
> 
Normally the IRQ would not be enabled under DOS.

Only when a poll would receive less packets than the budget
the following would normally allow the device to send another IRQ

if (work_done < budget) {
    napi_complete_done(napi, work_done);
    rtl_irq_enable(tp, enable_mask);
    mmiowb();
}

^ permalink raw reply

* Re: [PATCH v4 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
From: Alexei Starovoitov @ 2018-10-19  2:03 UTC (permalink / raw)
  To: Daniel Borkmann, Song Liu, netdev@vger.kernel.org
  Cc: ast@kernel.org, Kernel Team
In-Reply-To: <13f972be-ab41-8519-24cc-4e891579dc08@fb.com>

On 10/18/18 6:33 PM, Alexei Starovoitov wrote:
> On 10/18/18 5:14 PM, Daniel Borkmann wrote:
>>> +    case bpf_ctx_range(struct __sk_buff, data_meta):
>>> +    case bpf_ctx_range(struct __sk_buff, flow_keys):
>>> +        return false;
>> ... if it's disallowed anyway (disallowing it is the right thing to do,
>> but no need to save/restore then..)?
>>
>
> that's a good point.
> why shouldn't we allow cg_skb to access data_meta?
> xdp can set it and cgroup_skb_ingress will consume it here.

I'll take it back.
When xdp doesn't set meta_data it will be zero and
bpf_compute_data_pointers() will point data_meta to skb->data.
On ingress that's eth header, but for tx it will point
to reserved space for future eth header.
So we cannot do that.
Let's keep it disabled and adjust
bpf_compute_and_save_data_pointers() to save only 'data' pointer.


^ permalink raw reply

* Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Jason Wang @ 2018-10-19  2:17 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Toshiaki Makita, netdev, virtualization, tglx, Michael S. Tsirkin,
	David S. Miller
In-Reply-To: <20181018084753.wefvsypdevbzoadg@linutronix.de>


On 2018/10/18 下午4:47, Sebastian Andrzej Siewior wrote:
> On 2018-10-17 14:48:02 [+0800], Jason Wang wrote:
>> On 2018/10/17 上午9:13, Toshiaki Makita wrote:
>>> I'm not sure what condition triggered this warning.
> If the seqlock is acquired once in softirq and then in process context
> again it is enough evidence for lockdep to trigger this warning.
>
>>> Toshiaki Makita
>>
>> Or maybe NAPI is enabled unexpectedly somewhere?
>>
>> Btw, the schedule_delayed_work() in virtnet_open() is also suspicious, if
>> the work is executed before virtnet_napi_enable(), there will be a deadloop
>> for napi_disable().
> something like this? It is also likely if it runs OOM on queue 2, it
> will run OOM again on queue 3.
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index fbcfb4d272336..87d6ec4765270 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1263,22 +1263,22 @@ static void refill_work(struct work_struct *work)
>   {
>   	struct virtnet_info *vi =
>   		container_of(work, struct virtnet_info, refill.work);
> -	bool still_empty;
> +	int still_empty = 0;
>   	int i;
>   
>   	for (i = 0; i < vi->curr_queue_pairs; i++) {
>   		struct receive_queue *rq = &vi->rq[i];
>   
>   		napi_disable(&rq->napi);
> -		still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
> +		if (!try_fill_recv(vi, rq, GFP_KERNEL))
> +		    still_empty++;
>   		virtnet_napi_enable(rq->vq, &rq->napi);
> -
> -		/* In theory, this can happen: if we don't get any buffers in
> -		 * we will *never* try to fill again.
> -		 */
> -		if (still_empty)
> -			schedule_delayed_work(&vi->refill, HZ/2);
>   	}
> +	/* In theory, this can happen: if we don't get any buffers in
> +	 * we will *never* try to fill again.
> +	 */
> +	if (still_empty)
> +		schedule_delayed_work(&vi->refill, HZ/2);
>   }


I think this part is not a must or an independent optimization?


Thanks


>   
>   static int virtnet_receive(struct receive_queue *rq, int budget,
> @@ -1407,12 +1407,13 @@ static int virtnet_open(struct net_device *dev)
>   {
>   	struct virtnet_info *vi = netdev_priv(dev);
>   	int i, err;
> +	int need_refill = 0;
>   
>   	for (i = 0; i < vi->max_queue_pairs; i++) {
>   		if (i < vi->curr_queue_pairs)
>   			/* Make sure we have some buffers: if oom use wq. */
>   			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> -				schedule_delayed_work(&vi->refill, 0);
> +				need_refill++;
>   
>   		err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i);
>   		if (err < 0)
> @@ -1428,6 +1429,8 @@ static int virtnet_open(struct net_device *dev)
>   		virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
>   		virtnet_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi);
>   	}
> +	if (need_refill)
> +		schedule_delayed_work(&vi->refill, 0);
>   
>   	return 0;
>   }
> @@ -2236,6 +2239,7 @@ static int virtnet_restore_up(struct virtio_device *vdev)
>   {
>   	struct virtnet_info *vi = vdev->priv;
>   	int err, i;
> +	int need_refill = 0;
>   
>   	err = init_vqs(vi);
>   	if (err)
> @@ -2246,13 +2250,15 @@ static int virtnet_restore_up(struct virtio_device *vdev)
>   	if (netif_running(vi->dev)) {
>   		for (i = 0; i < vi->curr_queue_pairs; i++)
>   			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> -				schedule_delayed_work(&vi->refill, 0);
> +				need_refill++;
>   
>   		for (i = 0; i < vi->max_queue_pairs; i++) {
>   			virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
>   			virtnet_napi_tx_enable(vi, vi->sq[i].vq,
>   					       &vi->sq[i].napi);
>   		}
> +		if (need_refill)
> +			schedule_delayed_work(&vi->refill, 0);
>   	}
>   
>   	netif_device_attach(vi->dev);
>
>> Thanks
> Sebastian

^ permalink raw reply

* Re: [PATCH] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: Jason Wang @ 2018-10-19  2:19 UTC (permalink / raw)
  To: David Miller, bigeasy
  Cc: stephen, netdev, virtualization, tglx, makita.toshiaki, mst
In-Reply-To: <20181018.162308.2295937118791060714.davem@davemloft.net>


On 2018/10/19 上午7:23, David Miller wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Date: Thu, 18 Oct 2018 10:43:13 +0200
>
>> on 32bit, lockdep notices that virtnet_open() and refill_work() invoke
>> try_fill_recv() from process context while virtnet_receive() invokes the
>> same function from BH context. The problem that the seqcounter within
>> u64_stats_update_begin() may deadlock if it is interrupted by BH and
>> then acquired again.
>>
>> Introduce u64_stats_update_begin_bh() which disables BH on 32bit
>> architectures. Since the BH might interrupt the worker, this new
>> function should not limited to SMP like the others which are expected
>> to be used in softirq.
>>
>> With this change we might lose increments but this is okay. The
>> important part that the two 32bit parts of the 64bit counter are not
>> corrupted.
>>
>> Fixes: 461f03dc99cf6 ("virtio_net: Add kick stats").
>> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Trying to get down to the bottom of this:
>
> 1) virtnet_receive() runs from softirq but only if NAPI is active and
>     enabled.  It is in this context that it invokes try_fill_recv().
>
> 2) refill_work() runs from process context, but disables NAPI (and
>     thus invocation of virtnet_receive()) before calling
>     try_fill_recv().
>
> 3) virtnet_open() invokes from process context as well, but before the
>     NAPI instances are enabled, it is same as case #2.
>
> 4) virtnet_restore_up() is the same situations as #3.
>
> Therefore I agree that this is a false positive, and simply lockdep
> cannot see the NAPI synchronization done by case #2.
>
> I think we shouldn't add unnecessary BH disabling here, and instead
> find some way to annotate this for lockdep's sake.
>
> Thank you.
>

+1

Thanks

^ permalink raw reply

* Re: [PATCH 0/3] Bugfix for the netsec driver
From: Florian Fainelli @ 2018-10-19  2:56 UTC (permalink / raw)
  To: masahisa.kojima, netdev
  Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
	osaki.yoshitoyo
In-Reply-To: <20181019010843.3605-1-masahisa.kojima@linaro.org>



On 10/18/2018 6:08 PM, masahisa.kojima@linaro.org wrote:
> From: Masahisa Kojima <masahisa.kojima@linaro.org>
> 
> This patch series include bugfix for the netsec ethernet
> controller driver, fix the problem in interface down/up.

Since all of these are bugfixes you would want to provide a Fixes: tag
for the offending commits, that way you can get automated backporting to
stable trees, also patches should be targeted at the "net" tree, which
is indicated with a subject start with [PATCH net], more comments in the
patches.

> 
> Masahisa Kojima (3):
>   net: socionext: stop PHY before resetting netsec
>   net: socionext: Add dummy PHY register read in phy_write()
>   net: socionext: reset tx queue in ndo_stop
> 
>  drivers/net/ethernet/socionext/netsec.c | 36 +++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 

-- 
Florian

^ permalink raw reply

* Re: [PATCH 1/3] net: socionext: Stop PHY before resetting netsec
From: Florian Fainelli @ 2018-10-19  2:59 UTC (permalink / raw)
  To: masahisa.kojima, netdev
  Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
	osaki.yoshitoyo
In-Reply-To: <20181019010843.3605-2-masahisa.kojima@linaro.org>



On 10/18/2018 6:08 PM, masahisa.kojima@linaro.org wrote:
> From: Masahisa Kojima <masahisa.kojima@linaro.org>
> 
> After resetting netsec IP, driver have to wait until
> netsec mode turns to NRM mode.
> But sometimes mode transition to NRM will not complete
> if the PHY is in normal operation state.
> To avoid this situation, stop PHY before resetting netsec.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
> ---
>  drivers/net/ethernet/socionext/netsec.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 4289ccb26e4e..273cc5fc07e0 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -1343,11 +1343,11 @@ static int netsec_netdev_stop(struct net_device *ndev)
>  	netsec_uninit_pkt_dring(priv, NETSEC_RING_TX);
>  	netsec_uninit_pkt_dring(priv, NETSEC_RING_RX);
>  
> -	ret = netsec_reset_hardware(priv, false);
> -
>  	phy_stop(ndev->phydev);
>  	phy_disconnect(ndev->phydev);
>  
> +	ret = netsec_reset_hardware(priv, false);
> +
>  	pm_runtime_put_sync(priv->dev);
>  
>  	return ret;
> @@ -1415,7 +1415,7 @@ static const struct net_device_ops netsec_netdev_ops = {
>  };
>  
>  static int netsec_of_probe(struct platform_device *pdev,
> -			   struct netsec_priv *priv)
> +			   struct netsec_priv *priv, u32 *phy_addr)
>  {
>  	priv->phy_np = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
>  	if (!priv->phy_np) {
> @@ -1423,6 +1423,8 @@ static int netsec_of_probe(struct platform_device *pdev,
>  		return -EINVAL;
>  	}
>  
> +	*phy_addr = of_mdio_parse_addr(&pdev->dev, priv->phy_np);
> +
>  	priv->clk = devm_clk_get(&pdev->dev, NULL); /* get by 'phy_ref_clk' */
>  	if (IS_ERR(priv->clk)) {
>  		dev_err(&pdev->dev, "phy_ref_clk not found\n");
> @@ -1473,6 +1475,7 @@ static int netsec_register_mdio(struct netsec_priv *priv, u32 phy_addr)
>  {
>  	struct mii_bus *bus;
>  	int ret;
> +	u16 data;
>  
>  	bus = devm_mdiobus_alloc(priv->dev);
>  	if (!bus)
> @@ -1486,6 +1489,10 @@ static int netsec_register_mdio(struct netsec_priv *priv, u32 phy_addr)
>  	bus->parent = priv->dev;
>  	priv->mii_bus = bus;
>  
> +	/* set phy power down */
> +	data = netsec_phy_read(bus, phy_addr, 0) | 0x800;
> +	netsec_phy_write(bus, phy_addr, 0, data);

This is not explained in your commit message, and this is using open
coded values instead of the symbolic names from include/uapi/linux/mii.h.

Note that depending on the type of PHY you are interfaced with if the
PHY is in power down, the only register it is allowed to accept writes
for is MII_BMCR (per 802.3 clause 22 spec), any other read or write to a
different register can be discarded by its MDIO snooping logic. Since
probing the MDIO bus involves reading MII_PHYSID1/ID2, what is this
trying to achieve?

> +
>  	if (dev_of_node(priv->dev)) {
>  		struct device_node *mdio_node, *parent = dev_of_node(priv->dev);
>  
> @@ -1623,7 +1630,7 @@ static int netsec_probe(struct platform_device *pdev)
>  	}
>  
>  	if (dev_of_node(&pdev->dev))
> -		ret = netsec_of_probe(pdev, priv);
> +		ret = netsec_of_probe(pdev, priv, &phy_addr);
>  	else
>  		ret = netsec_acpi_probe(pdev, priv, &phy_addr);
>  	if (ret)
> 

-- 
Florian

^ permalink raw reply

* Re: [PATCH 2/3] net: socionext: Add dummy PHY register read in phy_write()
From: Florian Fainelli @ 2018-10-19  3:01 UTC (permalink / raw)
  To: masahisa.kojima, netdev
  Cc: ilias.apalodimas, jaswinder.singh, ard.biesheuvel,
	osaki.yoshitoyo
In-Reply-To: <20181019010843.3605-3-masahisa.kojima@linaro.org>



On 10/18/2018 6:08 PM, masahisa.kojima@linaro.org wrote:
> From: Masahisa Kojima <masahisa.kojima@linaro.org>
> 
> There is a compatibility issue between RTL8211E implemented
> in Developerbox and netsec network controller IP(F_GMAC4).
> 
> RTL8211E expects MDC clock must be kept toggling for several
> clock cycle with MDIO high before entering the IDLE state.
> To meet this requirement, netsec driver needs to issue dummy
> read(e.g. read PHYID1(offset 0x2) register) right after write.
> 
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> Signed-off-by: Yoshitoyo Osaki <osaki.yoshitoyo@socionext.com>
> ---
>  drivers/net/ethernet/socionext/netsec.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index 273cc5fc07e0..e7faaf8be99e 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -431,9 +431,12 @@ static int netsec_mac_update_to_phy_state(struct netsec_priv *priv)
>  	return 0;
>  }
>  
> +static int netsec_phy_read(struct mii_bus *bus, int phy_addr, int reg_addr);
> +
>  static int netsec_phy_write(struct mii_bus *bus,
>  			    int phy_addr, int reg, u16 val)
>  {
> +	int status;
>  	struct netsec_priv *priv = bus->priv;
>  
>  	if (netsec_mac_write(priv, GMAC_REG_GDR, val))
> @@ -446,8 +449,19 @@ static int netsec_phy_write(struct mii_bus *bus,
>  			      GMAC_REG_SHIFT_CR_GAR)))
>  		return -ETIMEDOUT;
>  
> -	return netsec_mac_wait_while_busy(priv, GMAC_REG_GAR,
> -					  NETSEC_GMAC_GAR_REG_GB);
> +	status = netsec_mac_wait_while_busy(priv, GMAC_REG_GAR,
> +					    NETSEC_GMAC_GAR_REG_GB);
> +
> +	/* Developerbox implements RTL8211E PHY and there is
> +	 * a compatibility problem with F_GMAC4.
> +	 * RTL8211E expects MDC clock must be kept toggling for several
> +	 * clock cycle with MDIO high before entering the IDLE state.
> +	 * To meet this requirement, netsec driver needs to issue dummy
> +	 * read(e.g. read PHYID1(offset 0x2) register) right after write.
> +	 */
> +	netsec_phy_read(bus, phy_addr, 2);

MII_PHYSID1 instead of 0x2 would be preferable. It is not clear to me
from your commit message if this is a problem specific to your MDIO
controller implementation and the RTL8211E PHY or if this is a general
problem of the PHY irrespective of the MDIO controller it is interface
with. If the latter, then we should seek a solution at a different layer
such that other systems don't have that same problem.

Thank you!
-- 
Florian

^ permalink raw reply

* Re: [RFC PATCH net-next 2/4] net: 8021q: vlan_core: allow use list of vlans for real device
From: Bjørn Mork @ 2018-10-19 11:22 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: grygorii.strashko, davem, linux-omap, netdev, linux-kernel,
	alexander.h.duyck
In-Reply-To: <20181016182035.18234-3-ivan.khoronzhuk@linaro.org>

Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> writes:

> @@ -236,6 +239,13 @@ __vlan_find_dev_deep_rcu(struct net_device *real_dev,
>  	return NULL;
>  }
>  
> +static inline int
> +vlan_for_each(struct net_device *dev,
> +	      int (*action)(struct net_device *dev, int vid, void *arg),
> +	      void *arg)
> +{
> +}
> +


This stub should return 0, shouldn't it?


Bjørn

^ permalink raw reply

* [PATCH] tipc: use destination length for copy string
From: Guoqing Jiang @ 2018-10-19  4:08 UTC (permalink / raw)
  To: jon.maloy, ying.xue, davem; +Cc: netdev, tipc-discussion, Guoqing Jiang

Got below warning with gcc 8.2 compiler.

net/tipc/topsrv.c: In function ‘tipc_topsrv_start’:
net/tipc/topsrv.c:660:2: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
  strncpy(srv->name, name, strlen(name) + 1);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/tipc/topsrv.c:660:27: note: length computed here
  strncpy(srv->name, name, strlen(name) + 1);
                           ^~~~~~~~~~~~
So change it to correct length and use strscpy.

Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
 net/tipc/topsrv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
index 2627b5d812e9..b84c0059214f 100644
--- a/net/tipc/topsrv.c
+++ b/net/tipc/topsrv.c
@@ -657,7 +657,7 @@ int tipc_topsrv_start(struct net *net)
 	srv->max_rcvbuf_size = sizeof(struct tipc_subscr);
 	INIT_WORK(&srv->awork, tipc_topsrv_accept);
 
-	strncpy(srv->name, name, strlen(name) + 1);
+	strscpy(srv->name, name, sizeof(srv->name));
 	tn->topsrv = srv;
 	atomic_set(&tn->subscription_count, 0);
 
-- 
2.12.3

^ permalink raw reply related

* Re: [PATCH bpf-next 2/3] tools, perf: use smp_{rmb,mb} barriers instead of {rmb,mb}
From: Alexei Starovoitov @ 2018-10-19  3:53 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Peter Zijlstra, paulmck, will.deacon, acme, yhs, john.fastabend,
	netdev
In-Reply-To: <1e3eea86-f708-f48b-4db4-911a84b94e06@iogearbox.net>

On Thu, Oct 18, 2018 at 09:00:46PM +0200, Daniel Borkmann wrote:
> On 10/18/2018 05:33 PM, Alexei Starovoitov wrote:
> > On Thu, Oct 18, 2018 at 05:04:34PM +0200, Daniel Borkmann wrote:
> >>  #endif /* _TOOLS_LINUX_ASM_IA64_BARRIER_H */
> >> diff --git a/tools/arch/powerpc/include/asm/barrier.h b/tools/arch/powerpc/include/asm/barrier.h
> >> index a634da0..905a2c6 100644
> >> --- a/tools/arch/powerpc/include/asm/barrier.h
> >> +++ b/tools/arch/powerpc/include/asm/barrier.h
> >> @@ -27,4 +27,20 @@
> >>  #define rmb()  __asm__ __volatile__ ("sync" : : : "memory")
> >>  #define wmb()  __asm__ __volatile__ ("sync" : : : "memory")
> >>
> >> +#if defined(__powerpc64__)
> >> +#define smp_lwsync()	__asm__ __volatile__ ("lwsync" : : : "memory")
> >> +
> >> +#define smp_store_release(p, v)			\
> >> +do {						\
> >> +	smp_lwsync();				\
> >> +	WRITE_ONCE(*p, v);			\
> >> +} while (0)
> >> +
> >> +#define smp_load_acquire(p)			\
> >> +({						\
> >> +	typeof(*p) ___p1 = READ_ONCE(*p);	\
> >> +	smp_lwsync();				\
> >> +	___p1;					\
> > 
> > I don't like this proliferation of asm.
> > Why do we think that we can do better job than compiler?
> > can we please use gcc builtins instead?
> > https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
> > __atomic_load_n(ptr, __ATOMIC_ACQUIRE);
> > __atomic_store_n(ptr, val, __ATOMIC_RELEASE);
> > are done specifically for this use case if I'm not mistaken.
> > I think it pays to learn what compiler provides.
> 
> But are you sure the C11 memory model matches exact same model as kernel?
> Seems like last time Will looked into it [0] it wasn't the case ...

I'm only suggesting equivalence of __atomic_load_n(ptr, __ATOMIC_ACQUIRE)
with kernel's smp_load_acquire().
I've seen a bunch of user space ring buffer implementations implemented
with __atomic_load_n() primitives.
But let's ask experts who live in both worlds.

Paul,
what would you recommend?
Should we copy paste smp_store_release() from kernel to be used
in user space library/tools
or use __atomic_load_n() builtins instead?


> The above was pulled in and slightly adapted from kernel side of arch
> asm barriers. Hm, it would probably be safest if an arch decides to adapt
> C11 barriers first from kernel side and user space could then use the
> exact same matching builtin functions for scenarios like these as well.
> 
>   [0] https://lore.kernel.org/lkml/20170308174300.GL20400@arm.com/

^ permalink raw reply

* Re: [PATCH net-next] net: ethernet: ti: cpsw: don't flush mcast entries while switch promisc mode
From: Ivan Khoronzhuk @ 2018-10-19 12:04 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: davem, linux-omap, netdev, linux-kernel
In-Reply-To: <6c34a3ce-dbee-538e-bda7-8dd485315267@ti.com>

On Thu, Oct 18, 2018 at 07:03:06PM -0500, Grygorii Strashko wrote:
>
>
>On 10/18/18 1:00 PM, Ivan Khoronzhuk wrote:
>>No need now to flush mcast entries in switch mode while toggling to
>>promiscuous mode. It's not needed as vlan reg_mcast = ALL_PORTS
>>and mcast/vlan ports = ALL_PORTS, the same happening for vlan
>>unreg_mcast, it's set to ALL_PORT_MASK just after calling promisc
>>mode routine by calling set allmulti. I suppose main reason to flush
>>them is to use unreg_mcast to receive all to host port. Thus, now, all
>>mcast packets are received anyway and no reason to flush mcast entries
>>unsafely, as they were synced with __dev_mc_sync() previously and are
>>not restored. Another way is to _dev_mc_unsync() them, but no need.
>
>User have possibility to add additional mcast entries or edit existing 
>in switch-mode, which is now done using custom tool. So, Host in 
>promisc
>mode will not receive packets for mcast address X if port mask for this
>addr set to (ALL_PORTS - HOST_PORT). Am I missing smth?

I didn't take into account the custom tool changing entries directly,
but even in this case there is at least a couple of interesting
questions:

1) Before the patch applied only several days ago -
   5da1948969bc1991920987ce4361ea56046e5a98
   "ti: cpsw: fix lost of mcast packets while rx_mode update"
   It was impossible to do correctly anyway, as all mcast entries not
   in the mc list were flushed (after rx_mode cb), by:
   cpsw_ale_flush_multicast(cpsw->ale, ALE_ALL_PORTS, vid);
   and those in mc, rewritten by adding them back in corrected form.
   ... or this cb was not supposed to be called at all ...

2) What is the reason to add mcast switch entires
   (ALL_PORTS - HOST_PORT) if its function is added anyway by
   unreq_mcast & (ALL_PORTS - HOST_PORT) ?
   So, doesn't matter it's added or not - it will work :-|.

3) Even so, toggling promisc mode will clear all these changes anyway,
   even I will call _dev_mc_unsync() after flushing them.

4) If user can tune ALE table by hand, what stops him do it after moving
   to promisc mode, seems he knows what he's doing?

5) It could be possible only for not default vlan entries, but mcast
   vlan support is not supported yet. Who is gona restore those
   entries after promisc off?

This behaviour is arguable, and flushing mcast entries can bring more
issues then leaving. For me it doesn't matter, I can archive the same
by adding after flush one line, it's even shorter:
__dev_mc_unsync(priv->ndev, NULL);


>
>>
>>Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>---
>>
>>Based on net-next/master
>>Tasted on am572x EVM and BBB
>>
>>  drivers/net/ethernet/ti/cpsw.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>>diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>>index 226be2a56c1f..0e475020a674 100644
>>--- a/drivers/net/ethernet/ti/cpsw.c
>>+++ b/drivers/net/ethernet/ti/cpsw.c
>>@@ -638,9 +638,6 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
>>  			} while (time_after(timeout, jiffies));
>>  			cpsw_ale_control_set(ale, 0, ALE_AGEOUT, 1);
>>-			/* Clear all mcast from ALE */
>>-			cpsw_ale_flush_multicast(ale, ALE_ALL_PORTS, -1);
>>-
>>  			/* Flood All Unicast Packets to Host port */
>>  			cpsw_ale_control_set(ale, 0, ALE_P0_UNI_FLOOD, 1);
>>  			dev_dbg(&ndev->dev, "promiscuity enabled\n");
>>
>
>-- 
>regards,
>-grygorii

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* [PATCH net-next] octeontx2-af: Remove set but not used variables 'devnum, is_pf'
From: YueHaibing @ 2018-10-19 13:03 UTC (permalink / raw)
  To: Sunil Goutham, Linu Cherian, Geetha sowjanya, Jerin Jacob,
	David S. Miller
  Cc: YueHaibing, netdev, linux-kernel, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/marvell/octeontx2/af/rvu.c: In function 'rvu_detach_rsrcs':
drivers/net/ethernet/marvell/octeontx2/af/rvu.c:855:6: warning:
 variable 'devnum' set but not used [-Wunused-but-set-variable]
 
drivers/net/ethernet/marvell/octeontx2/af/rvu.c:853:7: warning:
 variable 'is_pf' set but not used [-Wunused-but-set-variable]

drivers/net/ethernet/marvell/octeontx2/af/rvu.c: In function 'rvu_mbox_handler_ATTACH_RESOURCES':
drivers/net/ethernet/marvell/octeontx2/af/rvu.c:1054:7: warning:
 variable 'is_pf' set but not used [-Wunused-but-set-variable]

drivers/net/ethernet/marvell/octeontx2/af/rvu.c:1053:6: warning:
 variable 'devnum' set but not used [-Wunused-but-set-variable]

It never used since introduction in commit
746ea74241fa ("octeontx2-af: Add RVU block LF provisioning support")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index c06cca9..60b3623 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -914,18 +914,9 @@ static int rvu_detach_rsrcs(struct rvu *rvu, struct rsrc_detach *detach,
 			    u16 pcifunc)
 {
 	struct rvu_hwinfo *hw = rvu->hw;
-	bool is_pf, detach_all = true;
+	bool detach_all = true;
 	struct rvu_block *block;
-	int devnum, blkid;
-
-	/* Check if this is for a RVU PF or VF */
-	if (pcifunc & RVU_PFVF_FUNC_MASK) {
-		is_pf = false;
-		devnum = rvu_get_hwvf(rvu, pcifunc);
-	} else {
-		is_pf = true;
-		devnum = rvu_get_pf(pcifunc);
-	}
+	int blkid;
 
 	spin_lock(&rvu->rsrc_lock);
 
@@ -1114,22 +1105,12 @@ static int rvu_mbox_handler_ATTACH_RESOURCES(struct rvu *rvu,
 					     struct msg_rsp *rsp)
 {
 	u16 pcifunc = attach->hdr.pcifunc;
-	int devnum, err;
-	bool is_pf;
+	int err;
 
 	/* If first request, detach all existing attached resources */
 	if (!attach->modify)
 		rvu_detach_rsrcs(rvu, NULL, pcifunc);
 
-	/* Check if this is for a RVU PF or VF */
-	if (pcifunc & RVU_PFVF_FUNC_MASK) {
-		is_pf = false;
-		devnum = rvu_get_hwvf(rvu, pcifunc);
-	} else {
-		is_pf = true;
-		devnum = rvu_get_pf(pcifunc);
-	}
-
 	spin_lock(&rvu->rsrc_lock);
 
 	/* Check if the request can be accommodated */

^ permalink raw reply related

* [PATCH v5 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
From: Song Liu @ 2018-10-19  5:01 UTC (permalink / raw)
  To: netdev; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20181019050107.3684711-1-songliubraving@fb.com>

BPF programs of BPF_PROG_TYPE_CGROUP_SKB need to access headers in the
skb. This patch enables direct access of skb for these programs.

Two helper functions bpf_compute_and_save_data_end() and
bpf_restore_data_end() are introduced. There are used in
__cgroup_bpf_run_filter_skb(), to compute proper data_end for the
BPF program, and restore original data afterwards.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 include/linux/filter.h | 21 +++++++++++++++++++++
 kernel/bpf/cgroup.c    |  6 ++++++
 net/core/filter.c      | 36 +++++++++++++++++++++++++++++++++++-
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 5771874bc01e..91b4c934f02e 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -548,6 +548,27 @@ static inline void bpf_compute_data_pointers(struct sk_buff *skb)
 	cb->data_end  = skb->data + skb_headlen(skb);
 }
 
+/* Similar to bpf_compute_data_pointers(), except that save orginal
+ * data in cb->data and cb->meta_data for restore.
+ */
+static inline void bpf_compute_and_save_data_end(
+	struct sk_buff *skb, void **saved_data_end)
+{
+	struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+	*saved_data_end = cb->data_end;
+	cb->data_end  = skb->data + skb_headlen(skb);
+}
+
+/* Restore data saved by bpf_compute_data_pointers(). */
+static inline void bpf_restore_data_end(
+	struct sk_buff *skb, void *saved_data_end)
+{
+	struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+	cb->data_end = saved_data_end;
+}
+
 static inline u8 *bpf_skb_cb(struct sk_buff *skb)
 {
 	/* eBPF programs may read/write skb->cb[] area to transfer meta
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 00f6ed2e4f9a..9425c2fb872f 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -553,6 +553,7 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
 {
 	unsigned int offset = skb->data - skb_network_header(skb);
 	struct sock *save_sk;
+	void *saved_data_end;
 	struct cgroup *cgrp;
 	int ret;
 
@@ -566,8 +567,13 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
 	save_sk = skb->sk;
 	skb->sk = sk;
 	__skb_push(skb, offset);
+
+	/* compute pointers for the bpf prog */
+	bpf_compute_and_save_data_end(skb, &saved_data_end);
+
 	ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[type], skb,
 				 bpf_prog_run_save_cb);
+	bpf_restore_data_end(skb, saved_data_end);
 	__skb_pull(skb, offset);
 	skb->sk = save_sk;
 	return ret == 1 ? 0 : -EPERM;
diff --git a/net/core/filter.c b/net/core/filter.c
index 1a3ac6c46873..e3ca30bd6840 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5346,6 +5346,40 @@ static bool sk_filter_is_valid_access(int off, int size,
 	return bpf_skb_is_valid_access(off, size, type, prog, info);
 }
 
+static bool cg_skb_is_valid_access(int off, int size,
+				   enum bpf_access_type type,
+				   const struct bpf_prog *prog,
+				   struct bpf_insn_access_aux *info)
+{
+	switch (off) {
+	case bpf_ctx_range(struct __sk_buff, tc_classid):
+	case bpf_ctx_range(struct __sk_buff, data_meta):
+	case bpf_ctx_range(struct __sk_buff, flow_keys):
+		return false;
+	}
+	if (type == BPF_WRITE) {
+		switch (off) {
+		case bpf_ctx_range(struct __sk_buff, mark):
+		case bpf_ctx_range(struct __sk_buff, priority):
+		case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+			break;
+		default:
+			return false;
+		}
+	}
+
+	switch (off) {
+	case bpf_ctx_range(struct __sk_buff, data):
+		info->reg_type = PTR_TO_PACKET;
+		break;
+	case bpf_ctx_range(struct __sk_buff, data_end):
+		info->reg_type = PTR_TO_PACKET_END;
+		break;
+	}
+
+	return bpf_skb_is_valid_access(off, size, type, prog, info);
+}
+
 static bool lwt_is_valid_access(int off, int size,
 				enum bpf_access_type type,
 				const struct bpf_prog *prog,
@@ -7038,7 +7072,7 @@ const struct bpf_prog_ops xdp_prog_ops = {
 
 const struct bpf_verifier_ops cg_skb_verifier_ops = {
 	.get_func_proto		= cg_skb_func_proto,
-	.is_valid_access	= sk_filter_is_valid_access,
+	.is_valid_access	= cg_skb_is_valid_access,
 	.convert_ctx_access	= bpf_convert_ctx_access,
 };
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 bpf-next 0/2] bpf: add cg_skb_is_valid_access
From: Song Liu @ 2018-10-19  5:01 UTC (permalink / raw)
  To: netdev; +Cc: ast, daniel, kernel-team, Song Liu

Changes v4 -> v5:
1. Replaced bpf_compute_and_save_data_pointers() with
   bpf_compute_and_save_data_end();
   Replaced bpf_restore_data_pointers() with bpf_restore_data_end().
2. Fixed indentation in test_verifier.c

Changes v3 -> v4:
1. Fixed crash issue reported by Alexei.

Changes v2 -> v3:
1. Added helper function bpf_compute_and_save_data_pointers() and
   bpf_restore_data_pointers().

Changes v1 -> v2:
1. Updated the list of read-only fields, and read-write fields.
2. Added dummy sk to bpf_prog_test_run_skb().

This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access
some __skb_buff data directly.

Song Liu (2):
  bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
  bpf: add tests for direct packet access from CGROUP_SKB

 include/linux/filter.h                      |  21 +++
 kernel/bpf/cgroup.c                         |   6 +
 net/bpf/test_run.c                          |   7 +
 net/core/filter.c                           |  36 ++++-
 tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
 5 files changed, 240 insertions(+), 1 deletion(-)

^ permalink raw reply

* [PATCH v5 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB
From: Song Liu @ 2018-10-19  5:01 UTC (permalink / raw)
  To: netdev; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20181019050107.3684711-1-songliubraving@fb.com>

Tests are added to make sure CGROUP_SKB cannot access:
  tc_classid, data_meta, flow_keys

and can read and write:
  mark, prority, and cb[0-4]

and can read other fields.

To make selftest with skb->sk work, a dummy sk is added in
bpf_prog_test_run_skb().

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 net/bpf/test_run.c                          |   7 +
 tools/testing/selftests/bpf/test_verifier.c | 171 ++++++++++++++++++++
 2 files changed, 178 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0c423b8cd75c..8dccac305268 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,8 @@
 #include <linux/etherdevice.h>
 #include <linux/filter.h>
 #include <linux/sched/signal.h>
+#include <net/sock.h>
+#include <net/tcp.h>
 
 static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
 		struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
@@ -115,6 +117,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	u32 retval, duration;
 	int hh_len = ETH_HLEN;
 	struct sk_buff *skb;
+	struct sock sk = {0};
 	void *data;
 	int ret;
 
@@ -137,11 +140,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		break;
 	}
 
+	sock_net_set(&sk, &init_net);
+	sock_init_data(NULL, &sk);
+
 	skb = build_skb(data, 0);
 	if (!skb) {
 		kfree(data);
 		return -ENOMEM;
 	}
+	skb->sk = &sk;
 
 	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
 	__skb_put(skb, size);
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index cf4cd32b6772..f1ae8d09770f 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4862,6 +4862,177 @@ static struct bpf_test tests[] = {
 		.result = REJECT,
 		.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
 	},
+	{
+		"direct packet read test#1 for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+				    offsetof(struct __sk_buff, data)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+				    offsetof(struct __sk_buff, data_end)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+				    offsetof(struct __sk_buff, len)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, pkt_type)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, mark)),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+				    offsetof(struct __sk_buff, mark)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+				    offsetof(struct __sk_buff, queue_mapping)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+				    offsetof(struct __sk_buff, protocol)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+				    offsetof(struct __sk_buff, vlan_present)),
+			BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+			BPF_JMP_REG(BPF_JGT, BPF_REG_0, BPF_REG_3, 1),
+			BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"direct packet read test#2 for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+				    offsetof(struct __sk_buff, vlan_tci)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, vlan_proto)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, priority)),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+				    offsetof(struct __sk_buff, priority)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+				    offsetof(struct __sk_buff,
+					     ingress_ifindex)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+				    offsetof(struct __sk_buff, tc_index)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+				    offsetof(struct __sk_buff, hash)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"direct packet read test#3 for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+				    offsetof(struct __sk_buff, cb[0])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, cb[1])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, cb[2])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+				    offsetof(struct __sk_buff, cb[3])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+				    offsetof(struct __sk_buff, cb[4])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+				    offsetof(struct __sk_buff, napi_id)),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_4,
+				    offsetof(struct __sk_buff, cb[0])),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_5,
+				    offsetof(struct __sk_buff, cb[1])),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+				    offsetof(struct __sk_buff, cb[2])),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_7,
+				    offsetof(struct __sk_buff, cb[3])),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_8,
+				    offsetof(struct __sk_buff, cb[4])),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"direct packet read test#4 for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+				    offsetof(struct __sk_buff, family)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+				    offsetof(struct __sk_buff, remote_ip4)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+				    offsetof(struct __sk_buff, local_ip4)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, remote_ip6[0])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, remote_ip6[1])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, remote_ip6[2])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+				    offsetof(struct __sk_buff, remote_ip6[3])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, local_ip6[0])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, local_ip6[1])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, local_ip6[2])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+				    offsetof(struct __sk_buff, local_ip6[3])),
+			BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+				    offsetof(struct __sk_buff, remote_port)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+				    offsetof(struct __sk_buff, local_port)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"invalid access of tc_classid for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+				    offsetof(struct __sk_buff, tc_classid)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = REJECT,
+		.errstr = "invalid bpf_context access",
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"invalid access of data_meta for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+				    offsetof(struct __sk_buff, data_meta)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = REJECT,
+		.errstr = "invalid bpf_context access",
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"invalid access of flow_keys for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+				    offsetof(struct __sk_buff, flow_keys)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = REJECT,
+		.errstr = "invalid bpf_context access",
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
+	{
+		"invalid write access to napi_id for CGROUP_SKB",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+				    offsetof(struct __sk_buff, napi_id)),
+			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_9,
+				    offsetof(struct __sk_buff, napi_id)),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.result = REJECT,
+		.errstr = "invalid bpf_context access",
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	},
 	{
 		"valid cgroup storage access",
 		.insns = {
-- 
2.17.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox