* [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
From: Harini Katakam @ 2016-11-28 9:49 UTC (permalink / raw)
To: nicolas.ferre, davem, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, boris.brezillon, alexandre.belloni,
harinikatakamlinux
Cc: netdev, linux-kernel, devicetree, harinik, michals,
Punnaiah Choudary Kalluri
This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c
Signed-off-by: Punnaiah Choudary Kalluri <punnaia@xilinx.com>
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
drivers/net/ethernet/cadence/Makefile | 2 +-
drivers/net/ethernet/cadence/macb.c | 169 +++-----------------
drivers/net/ethernet/cadence/macb.h | 2 +
drivers/net/ethernet/cadence/macb_mdio.c | 266 +++++++++++++++++++++++++++++++
4 files changed, 294 insertions(+), 145 deletions(-)
create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
diff --git a/drivers/net/ethernet/cadence/Makefile b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
# Makefile for the Atmel network device drivers.
#
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 80ccfc4..ae2a797 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
}
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
- struct macb *bp = bus->priv;
- int value;
-
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
-
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
-
- value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
- return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
- u16 value)
-{
- struct macb *bp = bus->priv;
-
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
-
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
-
- return 0;
-}
-
/**
* macb_set_tx_clk() - Set a clock to a new frequency
* @clk Pointer to the clock to change
@@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device *dev)
static int macb_mii_probe(struct net_device *dev)
{
struct macb *bp = netdev_priv(dev);
- struct macb_platform_data *pdata;
struct phy_device *phydev;
- int phy_irq;
int ret;
- phydev = phy_find_first(bp->mii_bus);
+ if (dev->phydev)
+ return 0;
+
+ phydev = of_phy_find_device(bp->phy_node);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
return -ENXIO;
}
-
- pdata = dev_get_platdata(&bp->pdev->dev);
- if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
- ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
- "phy int");
- if (!ret) {
- phy_irq = gpio_to_irq(pdata->phy_irq_pin);
- phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
- }
- }
-
+ if (bp->phy_irq)
+ phydev->irq = bp->phy_irq;
/* attach the mac to the phy */
ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
bp->phy_interface);
@@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
bp->speed = 0;
bp->duplex = -1;
- return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
- struct macb_platform_data *pdata;
- struct device_node *np;
- int err = -ENXIO, i;
-
- /* Enable management port */
- macb_writel(bp, NCR, MACB_BIT(MPE));
-
- bp->mii_bus = mdiobus_alloc();
- if (!bp->mii_bus) {
- err = -ENOMEM;
- goto err_out;
- }
-
- bp->mii_bus->name = "MACB_mii_bus";
- bp->mii_bus->read = &macb_mdio_read;
- bp->mii_bus->write = &macb_mdio_write;
- snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
- bp->pdev->name, bp->pdev->id);
- bp->mii_bus->priv = bp;
- bp->mii_bus->parent = &bp->pdev->dev;
- pdata = dev_get_platdata(&bp->pdev->dev);
-
- dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
-
- np = bp->pdev->dev.of_node;
- if (np) {
- /* try dt phy registration */
- err = of_mdiobus_register(bp->mii_bus, np);
-
- /* fallback to standard phy registration if no phy were
- * found during dt phy registration
- */
- if (!err && !phy_find_first(bp->mii_bus)) {
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- struct phy_device *phydev;
-
- phydev = mdiobus_scan(bp->mii_bus, i);
- if (IS_ERR(phydev) &&
- PTR_ERR(phydev) != -ENODEV) {
- err = PTR_ERR(phydev);
- break;
- }
- }
-
- if (err)
- goto err_out_unregister_bus;
- }
- } else {
- if (pdata)
- bp->mii_bus->phy_mask = pdata->phy_mask;
-
- err = mdiobus_register(bp->mii_bus);
- }
-
- if (err)
- goto err_out_free_mdiobus;
-
- err = macb_mii_probe(bp->dev);
- if (err)
- goto err_out_unregister_bus;
+ phy_attached_info(phydev);
return 0;
-
-err_out_unregister_bus:
- mdiobus_unregister(bp->mii_bus);
-err_out_free_mdiobus:
- mdiobus_free(bp->mii_bus);
-err_out:
- return err;
}
static void macb_update_stats(struct macb *bp)
@@ -2060,7 +1942,8 @@ static int macb_open(struct net_device *dev)
netif_carrier_off(dev);
/* if the phy is not yet register, retry later*/
- if (!dev->phydev)
+ err = macb_mii_probe(dev);
+ if (err)
return -EAGAIN;
/* RX buffers initialization */
@@ -3122,16 +3005,16 @@ static int macb_probe(struct platform_device *pdev)
unsigned int queue_mask, num_queues;
struct macb_platform_data *pdata;
bool native_io;
- struct phy_device *phydev;
struct net_device *dev;
struct resource *regs;
void __iomem *mem;
const char *mac;
struct macb *bp;
+ int phy_irq;
int err;
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- mem = devm_ioremap_resource(&pdev->dev, regs);
+ mem = devm_ioremap(&pdev->dev, regs->start, resource_size(regs));
if (IS_ERR(mem))
return PTR_ERR(mem);
@@ -3250,21 +3133,26 @@ static int macb_probe(struct platform_device *pdev)
if (err)
goto err_out_free_netdev;
- err = macb_mii_init(bp);
- if (err)
- goto err_out_free_netdev;
-
- phydev = dev->phydev;
-
- netif_carrier_off(dev);
-
err = register_netdev(dev);
if (err) {
dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
goto err_out_unregister_mdio;
}
- phy_attached_info(phydev);
+ bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
+ "phy-handle", 0);
+
+ pdata = dev_get_platdata(&bp->pdev->dev);
+ if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
+ err = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
+ "phy int");
+ if (!err) {
+ phy_irq = gpio_to_irq(pdata->phy_irq_pin);
+ bp->phy_irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
+ }
+ }
+
+ netif_carrier_off(dev);
netdev_info(dev, "Cadence %s rev 0x%08x at 0x%08lx irq %d (%pM)\n",
macb_is_gem(bp) ? "GEM" : "MACB", macb_readl(bp, MID),
@@ -3273,10 +3161,6 @@ static int macb_probe(struct platform_device *pdev)
return 0;
err_out_unregister_mdio:
- phy_disconnect(dev->phydev);
- mdiobus_unregister(bp->mii_bus);
- mdiobus_free(bp->mii_bus);
-
/* Shutdown the PHY if there is a GPIO reset */
if (bp->reset_gpio)
gpiod_set_value(bp->reset_gpio, 0);
@@ -3304,9 +3188,6 @@ static int macb_remove(struct platform_device *pdev)
bp = netdev_priv(dev);
if (dev->phydev)
phy_disconnect(dev->phydev);
- mdiobus_unregister(bp->mii_bus);
- dev->phydev = NULL;
- mdiobus_free(bp->mii_bus);
/* Shutdown the PHY if there is a GPIO reset */
if (bp->reset_gpio)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d67adad..15e5c0f 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -874,6 +874,8 @@ struct macb {
unsigned int jumbo_max_len;
u32 wol;
+ struct device_node *phy_node;
+ int phy_irq;
};
static inline bool macb_is_gem(struct macb *bp)
diff --git a/drivers/net/ethernet/cadence/macb_mdio.c b/drivers/net/ethernet/cadence/macb_mdio.c
new file mode 100644
index 0000000..1277ca3
--- /dev/null
+++ b/drivers/net/ethernet/cadence/macb_mdio.c
@@ -0,0 +1,266 @@
+/*
+ * Cadence Macb mdio controller driver
+ *
+ * Copyright (C) 2014 - 2016 Xilinx, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/io.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+#include <linux/ptp_clock_kernel.h>
+#include "macb.h"
+
+struct macb_mdio_data {
+ void __iomem *regs;
+
+ struct clk *pclk;
+ struct clk *hclk;
+};
+
+#define macb_mdio_reg_writel(bp, offset, value) \
+ writel_relaxed(value, bp->regs + offset)
+#define macb_mdio_writel(bp, reg, value) \
+ macb_mdio_reg_writel(bp, MACB_##reg, value)
+
+#define macb_mdio_reg_readl(bp, offset) readl_relaxed(bp->regs + offset)
+#define macb_mdio_readl(bp, reg) macb_mdio_reg_readl(bp, MACB_##reg)
+
+#define MACB_MDIO_TIMEOUT 1000
+
+static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+{
+ struct macb_mdio_data *bp = bus->priv;
+ unsigned int timeout = MACB_MDIO_TIMEOUT;
+ int value;
+
+ macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
+ MACB_BF(RW, MACB_MAN_READ) |
+ MACB_BF(PHYA, mii_id) |
+ MACB_BF(REGA, regnum) |
+ MACB_BF(CODE, MACB_MAN_CODE)));
+
+ /* wait for end of transfer */
+ while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout)
+ return -ETIMEDOUT;
+
+ value = MACB_BFEXT(DATA, macb_mdio_readl(bp, MAN));
+
+ return value;
+}
+
+static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
+ u16 value)
+{
+ struct macb_mdio_data *bp = bus->priv;
+ unsigned int timeout = MACB_MDIO_TIMEOUT;
+
+ macb_mdio_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF) |
+ MACB_BF(RW, MACB_MAN_WRITE) |
+ MACB_BF(PHYA, mii_id) |
+ MACB_BF(REGA, regnum) |
+ MACB_BF(CODE, MACB_MAN_CODE) |
+ MACB_BF(DATA, value)));
+
+ /* wait for end of transfer */
+ while (!MACB_BFEXT(IDLE, macb_mdio_readl(bp, NSR)) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+static u32 gem_mdc_clk_div(struct macb_mdio_data *bp)
+{
+ u32 config;
+ unsigned long pclk_hz = clk_get_rate(bp->pclk);
+
+ if (pclk_hz <= 20000000)
+ config = GEM_BF(CLK, GEM_CLK_DIV8);
+ else if (pclk_hz <= 40000000)
+ config = GEM_BF(CLK, GEM_CLK_DIV16);
+ else if (pclk_hz <= 80000000)
+ config = GEM_BF(CLK, GEM_CLK_DIV32);
+ else if (pclk_hz <= 120000000)
+ config = GEM_BF(CLK, GEM_CLK_DIV48);
+ else if (pclk_hz <= 160000000)
+ config = GEM_BF(CLK, GEM_CLK_DIV64);
+ else
+ config = GEM_BF(CLK, GEM_CLK_DIV96);
+
+ return config;
+}
+
+static int macb_mdio_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct mii_bus *bus;
+ struct macb_mdio_data *bp;
+ struct resource *res;
+ int ret;
+ u32 config, i;
+
+ bus = mdiobus_alloc_size(sizeof(*bp));
+ if (!bus)
+ return -ENOMEM;
+
+ bus->name = "macb_mii_bus";
+ bus->read = &macb_mdio_read;
+ bus->write = &macb_mdio_write;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
+ bus->parent = &pdev->dev;
+ bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
+ GFP_KERNEL);
+ if (!bus->irq) {
+ ret = -ENOMEM;
+ goto err_out_free_mdiobus;
+ }
+
+ bp = bus->priv;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ bp->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (IS_ERR(bp->regs)) {
+ ret = PTR_ERR(bp->regs);
+ goto err_out_free_mdiobus;
+ }
+
+ bp->pclk = devm_clk_get(&pdev->dev, "pclk");
+ if (IS_ERR(bp->pclk)) {
+ ret = PTR_ERR(bp->pclk);
+ dev_err(&pdev->dev, "failed to get macb_clk (%u)\n", ret);
+ goto err_out_free_mdiobus;
+ }
+
+ bp->hclk = devm_clk_get(&pdev->dev, "hclk");
+ if (IS_ERR(bp->hclk)) {
+ ret = PTR_ERR(bp->hclk);
+ dev_err(&pdev->dev, "failed to get hclk (%u)\n", ret);
+ goto err_out_free_mdiobus;
+ }
+
+ ret = clk_prepare_enable(bp->pclk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable pclk (%u)\n", ret);
+ goto err_out_free_mdiobus;
+ }
+
+ ret = clk_prepare_enable(bp->hclk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable hclk (%u)\n", ret);
+ goto err_disable_pclk;
+ }
+
+ platform_set_drvdata(pdev, bus);
+
+ /* Enable management port */
+ config = macb_mdio_readl(bp, NCR);
+ config |= MACB_BIT(MPE);
+ macb_mdio_writel(bp, NCR, config);
+ config = gem_mdc_clk_div(bp);
+ macb_mdio_writel(bp, NCFGR, config);
+
+ np = pdev->dev.of_node;
+ if (np) {
+ /* try dt phy registration */
+ ret = of_mdiobus_register(bus, np);
+
+ /* Fallback to standard phy registration if no phy were
+ * found during dt phy registration
+ */
+ if (!ret && !phy_find_first(bus)) {
+ for (i = 0; i < PHY_MAX_ADDR; i++) {
+ struct phy_device *phydev;
+
+ phydev = mdiobus_scan(bus, i);
+ if (IS_ERR(phydev) &&
+ PTR_ERR(phydev) != -ENODEV) {
+ ret = PTR_ERR(phydev);
+ break;
+ }
+ }
+
+ if (ret)
+ goto err_out_unregister_bus;
+ }
+ } else {
+ for (i = 0; i < PHY_MAX_ADDR; i++)
+ bus->irq[i] = PHY_POLL;
+
+ ret = of_mdiobus_register(bus, np);
+ }
+
+ if (ret)
+ goto err_out_free_mdio_irq;
+
+ return 0;
+
+err_out_unregister_bus:
+ mdiobus_unregister(bus);
+err_out_free_mdio_irq:
+ kfree(bus->irq);
+err_disable_pclk:
+ clk_disable_unprepare(bp->pclk);
+ clk_disable_unprepare(bp->hclk);
+err_out_free_mdiobus:
+ mdiobus_free(bus);
+ return ret;
+}
+
+static int macb_mdio_remove(struct platform_device *pdev)
+{
+ struct mii_bus *bus = platform_get_drvdata(pdev);
+ struct macb_mdio_data *bp = bus->priv;
+ u32 config;
+
+ /* Disable management port */
+ config = macb_mdio_readl(bp, NCR);
+ config &= ~MACB_BIT(MPE);
+ macb_mdio_writel(bp, NCR, config);
+ mdiobus_unregister(bus);
+ clk_disable_unprepare(bp->hclk);
+ clk_disable_unprepare(bp->pclk);
+ mdiobus_free(bus);
+
+ return 0;
+}
+
+static const struct of_device_id macb_mdio_dt_ids[] = {
+ { .compatible = "cdns,macb-mdio" },
+
+};
+MODULE_DEVICE_TABLE(of, macb_mdio_dt_ids);
+
+static struct platform_driver macb_mdio_driver = {
+ .probe = macb_mdio_probe,
+ .remove = macb_mdio_remove,
+ .driver = {
+ .name = "macb-mdio",
+ .of_match_table = macb_mdio_dt_ids,
+ },
+};
+
+module_platform_driver(macb_mdio_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Cadence MACB MDIO driver");
+MODULE_AUTHOR("Xilinx");
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 2/2] Documentation: devictree: Add macb mdio bindings
From: Harini Katakam @ 2016-11-28 9:49 UTC (permalink / raw)
To: nicolas.ferre, davem, robh+dt, pawel.moll, mark.rutland,
ijc+devicetree, galak, boris.brezillon, alexandre.belloni,
harinikatakamlinux
Cc: netdev, linux-kernel, devicetree, harinik, michals
Add documentations for macb mdio driver.
Signed-off-by: Harini Katakam <harinik@xilinx.com>
---
.../devicetree/bindings/net/macb-mdio.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/macb-mdio.txt
diff --git a/Documentation/devicetree/bindings/net/macb-mdio.txt b/Documentation/devicetree/bindings/net/macb-mdio.txt
new file mode 100644
index 0000000..014cedf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/macb-mdio.txt
@@ -0,0 +1,31 @@
+* Cadence MACB MDIO controller
+
+Required properties:
+- compatible: Should be "cdns,macb-mdio"
+- reg: Address and length of the register set of MAC to be used
+- clock-names: Tuple listing input clock names.
+ Required elements: 'pclk', 'hclk'
+ Optional elements: 'tx_clk'
+- clocks: Phandles to input clocks.
+
+Examples:
+
+ mdio {
+ compatible = "cdns,macb-mdio";
+ reg = <0x0 0xff0b0000 0x0 0x1000>;
+ clocks = <&clk125>, <&clk125>, <&clk125>;
+ clock-names = "pclk", "hclk", "tx_clk";
+ ethernet_phyC: ethernet-phy@C {
+ reg = <0xC>;
+ };
+ ethernet_phy7: ethernet-phy@7 {
+ reg = <0x7>;
+ };
+ ethernet_phy3: ethernet-phy@3 {
+ reg = <0x3>;
+ };
+ ethernet_phy8: ethernet-phy@8 {
+ reg = <0x8>;
+ };
+ };
+
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 3/9] liquidio CN23XX: VF config setup
From: kbuild test robot @ 2016-11-28 10:21 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: kbuild-all, davem, netdev, Raghu Vatsavayi, Raghu Vatsavayi,
Derek Chickles, Satanand Burla, Felix Manlunas
In-Reply-To: <1480308702-6261-4-git-send-email-rvatsavayi@caviumnetworks.com>
[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]
Hi Raghu,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Raghu-Vatsavayi/liquidio-VF-operations/20161128-133447
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `.liquidio_vf_probe':
>> lio_vf_main.c:(.text+0x1644b14): undefined reference to `.cn23xx_setup_octeon_vf_device'
drivers/built-in.o: In function `.brcmf_net_setcarrier':
(.text+0x1ff9bf4): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `.netif_carrier_on' defined in .text section in net/built-in.o
drivers/built-in.o: In function `.brcmf_add_if':
(.text+0x1ff9e28): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `.kmem_cache_alloc_trace' defined in .text section in mm/built-in.o
drivers/built-in.o: In function `.brcmf_detach':
(.text+0x1ffa918): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_attach':
(.text+0x1ffaaa4): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_attach':
(.text+0x1ffaadc): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `.kmalloc_order_trace' defined in .text section in mm/built-in.o
drivers/built-in.o: In function `.brcmf_nvram_handle_end':
firmware.c:(.text+0x1ffabcc): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_nvram_handle_idle':
firmware.c:(.text+0x1ffac04): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_nvram_handle_comment':
firmware.c:(.text+0x1ffad0c): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_nvram_handle_value':
firmware.c:(.text+0x1ffadc8): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_nvram_handle_key':
firmware.c:(.text+0x1ffaec0): relocation truncated to fit: R_PPC64_REL24 (stub) against symbol `._mcount' defined in .text section in arch/powerpc/kernel/entry_64.o
drivers/built-in.o: In function `.brcmf_fw_request_nvram_done':
firmware.c:(.text+0x1ffb074): additional relocation overflows omitted from the output
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51098 bytes --]
^ permalink raw reply
* [PATCH net-next V2 6/9] liquidio CN23XX: init VF softcommand queues
From: Raghu Vatsavayi @ 2016-11-28 8:50 UTC (permalink / raw)
To: davem
Cc: netdev, Raghu Vatsavayi, Raghu Vatsavayi, Derek Chickles,
Satanand Burla, Felix Manlunas
In-Reply-To: <1480323038-13702-1-git-send-email-rvatsavayi@caviumnetworks.com>
Adds support for initializing softcommand, dispatch and
instructions queues for VF.
Signed-off-by: Raghu Vatsavayi <raghu.vatsavayi@caviumnetworks.com>
Signed-off-by: Derek Chickles <derek.chickles@caviumnetworks.com>
Signed-off-by: Satanand Burla <satananda.burla@caviumnetworks.com>
Signed-off-by: Felix Manlunas <felix.manlunas@caviumnetworks.com>
---
drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 74 +++++++++++++++++++++-
.../net/ethernet/cavium/liquidio/octeon_device.c | 5 ++
.../net/ethernet/cavium/liquidio/request_manager.c | 7 ++
3 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 61c8b78..162e47b 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -140,18 +140,51 @@ static void octeon_pci_flr(struct octeon_device *oct)
*/
static void octeon_destroy_resources(struct octeon_device *oct)
{
+ int i;
+
switch (atomic_read(&oct->status)) {
+ case OCT_DEV_IN_RESET:
+ case OCT_DEV_DROQ_INIT_DONE:
+ mdelay(100);
+ for (i = 0; i < MAX_OCTEON_OUTPUT_QUEUES(oct); i++) {
+ if (!(oct->io_qmask.oq & BIT_ULL(i)))
+ continue;
+ octeon_delete_droq(oct, i);
+ }
+
+ /* fallthrough */
+ case OCT_DEV_RESP_LIST_INIT_DONE:
+ octeon_delete_response_list(oct);
+
+ /* fallthrough */
+ case OCT_DEV_INSTR_QUEUE_INIT_DONE:
+ for (i = 0; i < MAX_OCTEON_INSTR_QUEUES(oct); i++) {
+ if (!(oct->io_qmask.iq & BIT_ULL(i)))
+ continue;
+ octeon_delete_instr_queue(oct, i);
+ }
+
+ /* fallthrough */
+ case OCT_DEV_SC_BUFF_POOL_INIT_DONE:
+ octeon_free_sc_buffer_pool(oct);
+
+ /* fallthrough */
+ case OCT_DEV_DISPATCH_INIT_DONE:
+ octeon_delete_dispatch_list(oct);
+ cancel_delayed_work_sync(&oct->nic_poll_work.work);
+
+ /* fallthrough */
case OCT_DEV_PCI_MAP_DONE:
octeon_unmap_pci_barx(oct, 0);
octeon_unmap_pci_barx(oct, 1);
- /* fallthrough */
+ /* fallthrough */
case OCT_DEV_PCI_ENABLE_DONE:
pci_clear_master(oct->pci_dev);
/* Disable the device, releasing the PCI INT */
pci_disable_device(oct->pci_dev);
- /* fallthrough */
+ /* fallthrough */
case OCT_DEV_BEGIN_STATE:
/* Nothing to be done here either */
break;
@@ -236,6 +269,14 @@ static int octeon_device_init(struct octeon_device *oct)
atomic_set(&oct->status, OCT_DEV_PCI_MAP_DONE);
+ /* Initialize the dispatch mechanism used to push packets arriving on
+ * Octeon Output queues.
+ */
+ if (octeon_init_dispatch_list(oct))
+ return 1;
+
+ atomic_set(&oct->status, OCT_DEV_DISPATCH_INIT_DONE);
+
if (octeon_set_io_queues_off(oct)) {
dev_err(&oct->pci_dev->dev, "setting io queues off failed\n");
return 1;
@@ -246,6 +287,35 @@ static int octeon_device_init(struct octeon_device *oct)
return 1;
}
+ /* Initialize soft command buffer pool */
+ if (octeon_setup_sc_buffer_pool(oct)) {
+ dev_err(&oct->pci_dev->dev, "sc buffer pool allocation failed\n");
+ return 1;
+ }
+ atomic_set(&oct->status, OCT_DEV_SC_BUFF_POOL_INIT_DONE);
+
+ /* Setup the data structures that manage this Octeon's Input queues. */
+ if (octeon_setup_instr_queues(oct)) {
+ dev_err(&oct->pci_dev->dev, "instruction queue initialization failed\n");
+ return 1;
+ }
+ atomic_set(&oct->status, OCT_DEV_INSTR_QUEUE_INIT_DONE);
+
+ /* Initialize lists to manage the requests of different types that
+ * arrive from user & kernel applications for this octeon device.
+ */
+ if (octeon_setup_response_list(oct)) {
+ dev_err(&oct->pci_dev->dev, "Response list allocation failed\n");
+ return 1;
+ }
+ atomic_set(&oct->status, OCT_DEV_RESP_LIST_INIT_DONE);
+
+ if (octeon_setup_output_queues(oct)) {
+ dev_err(&oct->pci_dev->dev, "Output queue initialization failed\n");
+ return 1;
+ }
+ atomic_set(&oct->status, OCT_DEV_DROQ_INIT_DONE);
+
return 0;
}
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index fe84e90..fcc5f10 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -797,6 +797,8 @@ int octeon_setup_instr_queues(struct octeon_device *oct)
CFG_GET_NUM_DEF_TX_DESCS(CHIP_CONF(oct, cn6xxx));
else if (OCTEON_CN23XX_PF(oct))
num_descs = CFG_GET_NUM_DEF_TX_DESCS(CHIP_CONF(oct, cn23xx_pf));
+ else if (OCTEON_CN23XX_VF(oct))
+ num_descs = CFG_GET_NUM_DEF_TX_DESCS(CHIP_CONF(oct, cn23xx_vf));
oct->num_iqs = 0;
@@ -842,6 +844,9 @@ int octeon_setup_output_queues(struct octeon_device *oct)
} else if (OCTEON_CN23XX_PF(oct)) {
num_descs = CFG_GET_NUM_DEF_RX_DESCS(CHIP_CONF(oct, cn23xx_pf));
desc_size = CFG_GET_DEF_RX_BUF_SIZE(CHIP_CONF(oct, cn23xx_pf));
+ } else if (OCTEON_CN23XX_VF(oct)) {
+ num_descs = CFG_GET_NUM_DEF_RX_DESCS(CHIP_CONF(oct, cn23xx_vf));
+ desc_size = CFG_GET_DEF_RX_BUF_SIZE(CHIP_CONF(oct, cn23xx_vf));
}
oct->num_oqs = 0;
oct->droq[0] = vmalloc_node(sizeof(*oct->droq[0]), numa_node);
diff --git a/drivers/net/ethernet/cavium/liquidio/request_manager.c b/drivers/net/ethernet/cavium/liquidio/request_manager.c
index 0e10e2a..ea2b7e4 100644
--- a/drivers/net/ethernet/cavium/liquidio/request_manager.c
+++ b/drivers/net/ethernet/cavium/liquidio/request_manager.c
@@ -28,6 +28,7 @@
#include "octeon_network.h"
#include "cn66xx_device.h"
#include "cn23xx_pf_device.h"
+#include "cn23xx_vf_device.h"
struct iq_post_status {
int status;
@@ -68,6 +69,9 @@ int octeon_init_instr_queue(struct octeon_device *oct,
conf = &(CFG_GET_IQ_CFG(CHIP_CONF(oct, cn6xxx)));
else if (OCTEON_CN23XX_PF(oct))
conf = &(CFG_GET_IQ_CFG(CHIP_CONF(oct, cn23xx_pf)));
+ else if (OCTEON_CN23XX_VF(oct))
+ conf = &(CFG_GET_IQ_CFG(CHIP_CONF(oct, cn23xx_vf)));
+
if (!conf) {
dev_err(&oct->pci_dev->dev, "Unsupported Chip %x\n",
oct->chip_id);
@@ -183,6 +187,9 @@ int octeon_delete_instr_queue(struct octeon_device *oct, u32 iq_no)
else if (OCTEON_CN23XX_PF(oct))
desc_size =
CFG_GET_IQ_INSTR_TYPE(CHIP_CONF(oct, cn23xx_pf));
+ else if (OCTEON_CN23XX_VF(oct))
+ desc_size =
+ CFG_GET_IQ_INSTR_TYPE(CHIP_CONF(oct, cn23xx_vf));
vfree(iq->request_list);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next v3 1/4] net: phy: add an option to disable EEE advertisement
From: Yegor Yefremov @ 2016-11-28 10:28 UTC (permalink / raw)
To: Jerome Brunet
Cc: devicetree, Florian Fainelli, Alexandre TORGUE, Andrew Lunn,
Martin Blumenstingl, netdev, Neil Armstrong, kernel list,
Andre Roth, Kevin Hilman, Carlo Caione, Giuseppe Cavallaro,
linux-amlogic, linux-arm-kernel
In-Reply-To: <1480326409-25419-2-git-send-email-jbrunet@baylibre.com>
On Mon, Nov 28, 2016 at 10:46 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> This patch adds an option to disable EEE advertisement in the generic PHY
> by providing a mask of prohibited modes corresponding to the value found in
> the MDIO_AN_EEE_ADV register.
>
> On some platforms, PHY Low power idle seems to be causing issues, even
> breaking the link some cases. The patch provides a convenient way for these
> platforms to disable EEE advertisement and work around the issue.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Tested using Baltos ir2110 device (cpsw + at8035 PHY).
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> drivers/net/phy/phy.c | 3 ++
> drivers/net/phy/phy_device.c | 80 +++++++++++++++++++++++++++++++++++++++-----
> include/linux/phy.h | 3 ++
> 3 files changed, 77 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 73adbaa9ac86..a3981cc6448a 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1396,6 +1396,9 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
> {
> int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
>
> + /* Mask prohibited EEE modes */
> + val &= ~phydev->eee_broken_modes;
> +
> phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, val);
>
> return 0;
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ba86c191a13e..83e52f1b80f2 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1121,6 +1121,43 @@ static int genphy_config_advert(struct phy_device *phydev)
> }
>
> /**
> + * genphy_config_eee_advert - disable unwanted eee mode advertisement
> + * @phydev: target phy_device struct
> + *
> + * Description: Writes MDIO_AN_EEE_ADV after disabling unsupported energy
> + * efficent ethernet modes. Returns 0 if the PHY's advertisement hasn't
> + * changed, and 1 if it has changed.
> + */
> +static int genphy_config_eee_advert(struct phy_device *phydev)
> +{
> + u32 broken = phydev->eee_broken_modes;
> + u32 old_adv, adv;
> +
> + /* Nothing to disable */
> + if (!broken)
> + return 0;
> +
> + /* If the following call fails, we assume that EEE is not
> + * supported by the phy. If we read 0, EEE is not advertised
> + * In both case, we don't need to continue
> + */
> + adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN);
> + if (adv <= 0)
> + return 0;
> +
> + old_adv = adv;
> + adv &= ~broken;
> +
> + /* Advertising remains unchanged with the broken mask */
> + if (old_adv == adv)
> + return 0;
> +
> + phy_write_mmd_indirect(phydev, MDIO_AN_EEE_ADV, MDIO_MMD_AN, adv);
> +
> + return 1;
> +}
> +
> +/**
> * genphy_setup_forced - configures/forces speed/duplex from @phydev
> * @phydev: target phy_device struct
> *
> @@ -1178,15 +1215,20 @@ EXPORT_SYMBOL(genphy_restart_aneg);
> */
> int genphy_config_aneg(struct phy_device *phydev)
> {
> - int result;
> + int err, changed;
> +
> + changed = genphy_config_eee_advert(phydev);
>
> if (AUTONEG_ENABLE != phydev->autoneg)
> return genphy_setup_forced(phydev);
>
> - result = genphy_config_advert(phydev);
> - if (result < 0) /* error */
> - return result;
> - if (result == 0) {
> + err = genphy_config_advert(phydev);
> + if (err < 0) /* error */
> + return err;
> +
> + changed |= err;
> +
> + if (changed == 0) {
> /* Advertisement hasn't changed, but maybe aneg was never on to
> * begin with? Or maybe phy was isolated?
> */
> @@ -1196,16 +1238,16 @@ int genphy_config_aneg(struct phy_device *phydev)
> return ctl;
>
> if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
> - result = 1; /* do restart aneg */
> + changed = 1; /* do restart aneg */
> }
>
> /* Only restart aneg if we are advertising something different
> * than we were before.
> */
> - if (result > 0)
> - result = genphy_restart_aneg(phydev);
> + if (changed > 0)
> + return genphy_restart_aneg(phydev);
>
> - return result;
> + return 0;
> }
> EXPORT_SYMBOL(genphy_config_aneg);
>
> @@ -1563,6 +1605,21 @@ static void of_set_phy_supported(struct phy_device *phydev)
> __set_phy_supported(phydev, max_speed);
> }
>
> +static void of_set_phy_eee_broken(struct phy_device *phydev)
> +{
> + struct device_node *node = phydev->mdio.dev.of_node;
> + u32 broken;
> +
> + if (!IS_ENABLED(CONFIG_OF_MDIO))
> + return;
> +
> + if (!node)
> + return;
> +
> + if (!of_property_read_u32(node, "eee-broken-modes", &broken))
> + phydev->eee_broken_modes = broken;
> +}
> +
> /**
> * phy_probe - probe and init a PHY device
> * @dev: device to probe and init
> @@ -1600,6 +1657,11 @@ static int phy_probe(struct device *dev)
> of_set_phy_supported(phydev);
> phydev->advertising = phydev->supported;
>
> + /* Get the EEE modes we want to prohibit. We will ask
> + * the PHY stop advertising these mode later on
> + */
> + of_set_phy_eee_broken(phydev);
> +
> /* Set the state to READY by default */
> phydev->state = PHY_READY;
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index edde28ce163a..b53177fd38af 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -417,6 +417,9 @@ struct phy_device {
> u32 advertising;
> u32 lp_advertising;
>
> + /* Energy efficient ethernet modes which should be prohibited */
> + u32 eee_broken_modes;
> +
> int autoneg;
>
> int link_timeout;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net-next v3 2/4] dt-bindings: net: add EEE capability constants
From: Yegor Yefremov @ 2016-11-28 10:28 UTC (permalink / raw)
To: Jerome Brunet
Cc: devicetree, Florian Fainelli, Alexandre TORGUE, Andrew Lunn,
Martin Blumenstingl, netdev, Neil Armstrong, kernel list,
Andre Roth, Kevin Hilman, Carlo Caione, Giuseppe Cavallaro,
linux-amlogic, linux-arm-kernel
In-Reply-To: <1480326409-25419-3-git-send-email-jbrunet@baylibre.com>
On Mon, Nov 28, 2016 at 10:46 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Tested using Baltos ir2110 device (cpsw + at8035 PHY).
Tested-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
> include/dt-bindings/net/mdio.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 include/dt-bindings/net/mdio.h
>
> diff --git a/include/dt-bindings/net/mdio.h b/include/dt-bindings/net/mdio.h
> new file mode 100644
> index 000000000000..99c6d903d439
> --- /dev/null
> +++ b/include/dt-bindings/net/mdio.h
> @@ -0,0 +1,19 @@
> +/*
> + * This header provides generic constants for ethernet MDIO bindings
> + */
> +
> +#ifndef _DT_BINDINGS_NET_MDIO_H
> +#define _DT_BINDINGS_NET_MDIO_H
> +
> +/*
> + * EEE capability Advertisement
> + */
> +
> +#define MDIO_EEE_100TX 0x0002 /* 100TX EEE cap */
> +#define MDIO_EEE_1000T 0x0004 /* 1000T EEE cap */
> +#define MDIO_EEE_10GT 0x0008 /* 10GT EEE cap */
> +#define MDIO_EEE_1000KX 0x0010 /* 1000KX EEE cap */
> +#define MDIO_EEE_10GKX4 0x0020 /* 10G KX4 EEE cap */
> +#define MDIO_EEE_10GKR 0x0040 /* 10G KR EEE cap */
> +
> +#endif
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net-next 3/9] liquidio CN23XX: VF config setup
From: kbuild test robot @ 2016-11-28 10:31 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: kbuild-all, davem, netdev, Raghu Vatsavayi, Raghu Vatsavayi,
Derek Chickles, Satanand Burla, Felix Manlunas
In-Reply-To: <1480308702-6261-4-git-send-email-rvatsavayi@caviumnetworks.com>
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
Hi Raghu,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Raghu-Vatsavayi/liquidio-VF-operations/20161128-133447
config: s390-allyesconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `liquidio_vf_probe':
>> drivers/net/ethernet/cavium/liquidio/.tmp_gl_lio_vf_main.o:(.text+0x1486d90): undefined reference to `cn23xx_setup_octeon_vf_device'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43469 bytes --]
^ permalink raw reply
* Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Sebastian Frias @ 2016-11-28 10:34 UTC (permalink / raw)
To: Florian Fainelli, Martin Blumenstingl, Jerome Brunet
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
carlo-KA+7E9HrN00dnm+yROfE0A, Mans Rullgard, Andrew Lunn, mason
In-Reply-To: <1b7f113e-34f9-69f4-a45f-9fd687d87990-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 25/11/16 18:44, Florian Fainelli wrote:
> On 11/25/2016 03:13 AM, Sebastian Frias wrote:
>> On 24/11/16 19:55, Florian Fainelli wrote:
<snip>
>>> Correct, the meaning of PHY_INTERFACE_MODE should be from the
>>> perspective of the PHY device:
>>>
>>> - PHY_INTERFACE_MODE_RGMII_TXID means that the PHY is responsible for
>>> adding a delay when the MAC transmits (TX MAC -> PHY (delay) -> wire)
>>> - PHY_INTERFACE_MODE_RGMII_RXID means that the PHY is responsible for
>>> adding a delay when the MAC receives (RX MAC <- (delay) PHY) <- wire)
>>>
>>
>> Thanks for the explanation.
>> Actually I had thought that the delay was to account for board routing
>> (wires) between the MAC and the PHY.
>> From your explanation it appears that the delay is to account for board
>> routing (wires) between the PHY and the RJ45 socket.
>
> The placement of the (delay) was not meant to be exact, but it was
> wrongly place anyway, so it should be between the MAC and PHY, always.
> This is why you see people either fixing the need for a delay by
> appropriately programming the PHY, or the MAC, or by just inserting a
> fixed delay on the PCB between the PHY and the MAC and programming no
> delays (or using the default values and hoping this works).
Thanks.
Your patch "[PATCH net-next 3/4] Documentation: net: phy: Add blurb about
RGMII" on the documentation makes it clear.
>>>
>>> This also seems reasonable to do, provided that the PHY is also properly
>>> configured not to add delays in both directions, and therefore assumes
>>> that the MAC does it.
>>>
>>> We have a fairly large problem with how RGMII delays are done in PHYLIB
>>> and Ethernet MAC drivers (or just in general), where we can't really
>>> intersect properly what a PHY is supporting (in terms of internal
>>> delays), and what the MAC supports either. One possible approach could
>>> be to update PHY drivers a list of PHY_INTERFACE_MODE_* that they
>>> support (ideally, even with normalized nanosecond delay values),
>>
>> Just to make sure I understood this, the DT would say something like:
>>
>> phy-connection-type = "rgmii-txid";
>> txid-delay-ns = <3>;
>>
>> For a 3ns TX delay, would that be good?
>
> That's one possibility, although, see below, some PHYs support
> sub-nanosecond values, but in general, that seems like a good
> representation. If the "txid-delay-ns" property is omitted, a standard
> 2ns delay is assumed.
Sounds good.
I did not see the "txid-delay-ns" property documented in your patches, if
it is not too late, maybe it could be "txid-delay-ps" using picoseconds as
unit, right?
>>> and
>>> then intersect that with the requested phy_interface_t during
>>> phy_{attach,connect} time, and feed this back to the MAC with a special
>>> error code/callback, so we could gracefully try to choose another
>>> PHY_INTERFACE_MODE_* value that the MAC supports....
>>>
>>> A larger problem is that a number of drivers have been deployed, and
>>> Device Trees, possibly with the meaning of "phy-mode" and
>>> "phy-connection-type" being from the MAC perspective, and not the PHY
>>> perspective *sigh*, good luck auditing those.
>>>
>>> So from there, here is possibly what we could do
>>>
>>> - submit a series of patches that update the PHYLIB documentation (there
>>> are other things missing here) and make it clear from which entity (PHY
>>> or MAC) does the delay apply to, document the "intersection" problem here
>>
>> I think documenting is necessary, thanks in advance!
>>
>> However, I'm wondering if there's a way to make this work in all cases.
>> Indeed, if we consider for example that TX delay is required, we have 4
>> cases:
>>
>> PHY | MAC | Who applies?
>> TXID supported | TXID supported | PHY
>> TXID supported | TXID not supported | PHY
>> TXID not supported | TXID supported | MAC
>> TXID not supported | TXID not supported | cannot be done
>>
>> That is basically what my patch:
>>
>> https://marc.info/?l=linux-netdev&m=147869658031783&w=2
>>
>> attempted to achieve. That would allow more combinations of MAC<->PHY to
>> work, right?
>
> Yes, indeed.
Just one thing, from your patch "[PATCH net-next 3/4] Documentation: net:
phy: Add blurb about RGMII" I have the impression that the 3rd option from
the table above, would be a little bit more complex to implement.
I will comment on the patch.
>> Nevertheless, I think we also need to keep in mind that most of this
>> discussion assumes the case where both, MAC and PHY have equal capabilities.
>> Could it happen that the PHY supports only 2ns delay, and the MAC only
>> 1ns delay?
>
> I doubt this exists at the MAC level what we should have is either a 2ns
> delay, in either RX or TX path, or nothing, because that's the value
> that results in shifting the data lines and the RX/TX lines by 90
> degrees at 125Mhz (1/125^6 = 8 ns, one quarter shift is 90 degrees =
> 2ns). The PHY may have a similar set of pre-programmed, fixed 2ns
> delays, but it is not uncommon to see 0.X ns resolution available:
>
> drivers/net/phy/mscc.c
> drivers/net/phy/dp83867.c w/ arch/arm/boot/dts/dra72-evm-revc.dts
>
> In these cases, if you end-up using a non 2ns delay, you are fixing a
> PCB problem more than an interoperability problem between your MAC and PHY.
I see, thanks.
>> Could it happen that the delay is bigger than what is supported by
>> either the PHY or MAC alone? maybe if combined it could work, for example
>> a 3ns delay required and the PHY supporting 2ns and the MAC 1ns, combined
>> it could work?
>
> I suppose such a thing would work yes, but it would be difficult to
> report correctly to the core PHYLIB how this can work considering the
> vast array of options available to introduce delays in that case:
> MAC-level, PHY-level, pinctrl/pad level and possibly at the PCB itself.
>
> Once we can't rely on the fixed 2ns delay to work, we are going to have
> people do various experiments until they can either measure what the
> right delay value is for the specific PCB, or they just found the value
> that happens to work. I don't think we can do much at that point from a
> core PHYLIB perspective other than tell the network driver that the PHY
> supports delay in either RX, TX or both directions, and have the MAC
> decide what to apply that makes sense here, considering that this is
> already kind of an exceptional situation to be in.
Fair enough.
And thanks again for documenting this.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] Documentation: net: phy: Add blurb about RGMII
From: Sebastian Frias @ 2016-11-28 10:34 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, andrew, martin.blumenstingl, mans, alexandre.torgue,
peppe.cavallaro, timur, jbrunet, mason
In-Reply-To: <20161127184449.12351-4-f.fainelli@gmail.com>
On 27/11/16 19:44, Florian Fainelli wrote:
> RGMII is a recurring source of pain for people with Gigabit Ethernet
> hardware since it may require PHY driver and MAC driver level
> configuration hints. Document what are the expectations from PHYLIB and
> what options exist.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/networking/phy.txt | 76 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/Documentation/networking/phy.txt b/Documentation/networking/phy.txt
> index 9a42a9414cea..7a0cb1212b9e 100644
> --- a/Documentation/networking/phy.txt
> +++ b/Documentation/networking/phy.txt
> @@ -65,6 +65,82 @@ The MDIO bus
> drivers/net/ethernet/freescale/fsl_pq_mdio.c and an associated DTS file
> for one of the users. (e.g. "git grep fsl,.*-mdio arch/powerpc/boot/dts/")
>
> +(RG)MII/electrical interface considerations
> +
> + The Reduced Gigabit Medium Independent Interface (RGMII) is a 12 pins
> + electrical signal interface using a synchronous 125Mhz clock signal and several
> + data lines. Due to this design decision, a 1.5ns to 2ns delay must be added
> + between the clock line (RXC or TXC) and the data lines to let the PHY (clock
> + sink) have enough setup and hold times to sample the data lines correctly. The
> + PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let
> + the PHY driver and optionaly the MAC driver implement the required delay. The
> + values of phy_interface_t must be understood from the perspective of the PHY
> + device itself, leading to the following:
> +
> + * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any
> + internal delay by itself, it assumes that either the Ethernet MAC (if capable
> + or the PCB traces) insert the correct 1.5-2ns delay
For what is worth, the Atheros at803x driver comes with RX delay enabled as per HW
reset.
I understand that enforcing this documentation as is would result in changing the
driver to disable RX delay, but it could break existing DTs, so I don't know if that
path will be pursued.
Would explicit "versioning" the DT nodes be something worth exploring? So far it
seems the versioning is implicit: driver probably check the presence of some property
and conclude that it has to behave in a way or another.
> +
> + * PHY_INTERFACE_MODE_RGMII_TXID: the PHY should be inserting an internal delay
> + for the transmit data lines (TXD[3:0]) processed by the PHY device
> +
> + * PHY_INTERFACE_MODE_RGMII_RXID: the PHY should be inserting an internal delay
> + for the receive data lines (RXD[3:0]) processed by the PHY device
> +
> + * PHY_INTERFACE_MODE_RGMII_ID: the PHY should be inserting internal delays for
> + both transmit AND receive data lines from/to the PHY device
> +
> + Whenever it is possible, it is preferrable to utilize the PHY side RGMII delay
> + for several reasons:
> +
> + * PHY devices may offer sub-nanosecond granularity in how they allow a
> + receiver/transmitter side delay (e.g: 0.5, 1.0, 1.5ns) to be specified. Such
> + precision may be required to account for differences in PCB trace lengths
> +
> + * PHY devices are typically qualified for a large range of applications
> + (industrial, medical, automotive...), and they provide a constant and
> + reliable delay across temperature/pressure/voltage ranges
> +
> + * PHY device drivers in PHYLIB being reusable by nature, being able to
> + configure correctly a specified delay enables more designs with similar delay
> + requirements to be operate correctly
> +
> + For cases where the PHY is not capable of providing this delay, but the
> + Ethernet MAC driver is capable of doing it, the correct phy_interface_t value
> + should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be
> + configured correctly in order to provide the required transmit and/or receive
> + side delay from the perspective of the PHY device.
While this clarifies the current situation very well, wouldn't the proposed approach
require that a property such as "txid-delay-ns" on the PHY's DT node to be duplicated
for the MAC?
>Conversely, if the Ethernet
> + MAC driver looks at the phy_interface_t value, for any other mode but
> + PHY_INTERFACE_MODE_RGMII, it should make sure that the MAC-level delays are
> + disabled.
> +
> + In case neither the Ethernet MAC, nor the PHY are capable of providing the
> + required delays, as defined per the RGMII standard, several options may be
> + available:
> +
> + * Some SoCs may offer a pin pad/mux/controller capable of configuring a given
> + set of pins' drive strength, delays and voltage, and it may be a suitable
> + option to insert the expected 2ns RGMII delay
> +
> + * Modifying the PCB design to include a fixed delay (e.g: using a specifically
> + designed serpentine), which may not require software configuration at all
> +
> +Common problems with RGMII delay mismatch
> +
> + When there is a RGMII delay mismatch between the Ethernet MAC and the PHY, this
> + will most likely result in the clock and data line sampling to capture unstable
> + signals, typical symptoms include:
> +
> + * Transmission/reception partially works, and there is frequent or occasional
> + packet loss observed
> +
> + * Ethernet MAC may report some, or all packets ingressing with a FCS/CRC error,
> + or just discard them all
> +
> + * Switching to lower speeds such as 10/100Mbits/sec makes the problem go away
> + (since there is enough setup/hold time in that case)
> +
> +
> Connecting to a PHY
>
> Sometime during startup, the network driver needs to establish a connection
>
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] Documentation: net: phy: Add a paragraph about pause frames/flow control
From: Sebastian Frias @ 2016-11-28 10:38 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, andrew, martin.blumenstingl, mans, alexandre.torgue,
peppe.cavallaro, timur, jbrunet
In-Reply-To: <20161127184449.12351-3-f.fainelli@gmail.com>
On 27/11/16 19:44, Florian Fainelli wrote:
> Describe that the Ethernet MAC controller is ultimately responsible for
> dealing with proper pause frames/flow control advertisement and
> enabling, and that it is therefore allowed to have it change
> phydev->supported/advertising with SUPPORTED_Pause and
> SUPPORTED_AsymPause.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/networking/phy.txt | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/networking/phy.txt b/Documentation/networking/phy.txt
> index 4b25c0f24201..9a42a9414cea 100644
> --- a/Documentation/networking/phy.txt
> +++ b/Documentation/networking/phy.txt
> @@ -127,8 +127,9 @@ Letting the PHY Abstraction Layer do Everything
> values pruned from them which don't make sense for your controller (a 10/100
> controller may be connected to a gigabit capable PHY, so you would need to
> mask off SUPPORTED_1000baseT*). See include/linux/ethtool.h for definitions
> - for these bitfields. Note that you should not SET any bits, or the PHY may
> - get put into an unsupported state.
> + for these bitfields. Note that you should not SET any bits, except the
> + SUPPORTED_Pause and SUPPORTED_AsymPause bits (see below), or the PHY may get
> + put into an unsupported state.
>
> Lastly, once the controller is ready to handle network traffic, you call
> phy_start(phydev). This tells the PAL that you are ready, and configures the
> @@ -139,6 +140,19 @@ Letting the PHY Abstraction Layer do Everything
> When you want to disconnect from the network (even if just briefly), you call
> phy_stop(phydev).
>
> +Pause frames / flow control
> +
> + The PHY does not participate directly in flow control/pause frames except by
> + making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in
> + MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC
> + controller supports such a thing. Since flow control/pause frames generation
> + involves the Ethernet MAC driver, it is recommended that this driver takes care
> + of properly indicating advertisement and support for such features by setting
> + the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done
> + either before or after phy_connect()
If the bits are set after phy_connect(), how does the PHY framework knows there's
an update to the bits? Should some call be made?
and/or as a result of implementing the
> + ethtool::set_pauseparam feature.
> +
> +
> Keeping Close Tabs on the PAL
>
> It is possible that the PAL's built-in state machine needs a little help to
>
^ permalink raw reply
* RE: PAYMENT
From: oficina @ 2016-11-28 7:23 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 295 bytes --]
Dear sir
Greetings,
Please sorry for our late response due to holiday
.
Please kindly note that customer has transfered all due payments to your
USD bank account today
Kindly see Bank TT copy in attachment below and check with your bankers.
Best Regards ,
Weng wei
Assistant Account Manager
[-- Attachment #2: pdf-icon-copy-min_png.zip --]
[-- Type: application/zip, Size: 1202914 bytes --]
^ permalink raw reply
* Re: [PATCH net] net, sched: respect rcu grace period on cls destruction
From: Paul E. McKenney @ 2016-11-28 10:47 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Cong Wang, David Miller, John Fastabend, Roi Dayan, ast,
Hannes Frederic Sowa, Jiri Pirko, Linux Kernel Network Developers
In-Reply-To: <583BF43C.4020103@iogearbox.net>
On Mon, Nov 28, 2016 at 10:09:16AM +0100, Daniel Borkmann wrote:
> On 11/28/2016 07:57 AM, Cong Wang wrote:
> >On Sat, Nov 26, 2016 at 4:18 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>Roi reported a crash in flower where tp->root was NULL in ->classify()
> >>callbacks. Reason is that in ->destroy() tp->root is set to NULL via
> >>RCU_INIT_POINTER(). It's problematic for some of the classifiers, because
> >>this doesn't respect RCU grace period for them, and as a result, still
> >>outstanding readers from tc_classify() will try to blindly dereference
> >>a NULL tp->root.
> >>
> >>The tp->root object is strictly private to the classifier implementation
> >>and holds internal data the core such as tc_ctl_tfilter() doesn't know
> >>about. Within some classifiers, such as cls_bpf, cls_basic, etc, tp->root
> >>is only checked for NULL in ->get() callback, but nowhere else. This is
> >>misleading and seemed to be copied from old classifier code that was not
> >>cleaned up properly. For example, d3fa76ee6b4a ("[NET_SCHED]: cls_basic:
> >>fix NULL pointer dereference") moved tp->root initialization into ->init()
> >>routine, where before it was part of ->change(), so ->get() had to deal
> >>with tp->root being NULL back then, so that was indeed a valid case, after
> >>d3fa76ee6b4a, not really anymore. We used to set tp->root to NULL long
> >>ago in ->destroy(), see 47a1a1d4be29 ("pkt_sched: remove unnecessary xchg()
> >>in packet classifiers"); but the NULLifying was reintroduced with the
> >>RCUification, but it's not correct for every classifier implementation.
> >>
> >>In the cases that are fixed here with one exception of cls_cgroup, tp->root
> >>object is allocated and initialized inside ->init() callback, which is always
> >>performed at a point in time after we allocate a new tp, which means tp and
> >>thus tp->root was not globally visible in the tp chain yet (see tc_ctl_tfilter()).
> >>Also, on destruction tp->root is strictly kfree_rcu()'ed in ->destroy()
> >>handler, same for the tp which is kfree_rcu()'ed right when we return
> >>from ->destroy() in tcf_destroy(). This means, the head object's lifetime
> >>for such classifiers is always tied to the tp lifetime. The RCU callback
> >>invocation for the two kfree_rcu() could be out of order, but that's fine
> >>since both are independent.
> >>
> >>Dropping the RCU_INIT_POINTER(tp->root, NULL) for these classifiers here
> >>means that 1) we don't need a useless NULL check in fast-path and, 2) that
> >>outstanding readers of that tp in tc_classify() can still execute under
> >>respect with RCU grace period as it is actually expected.
> >>
> >>Things that haven't been touched here: cls_fw and cls_route. They each
> >>handle tp->root being NULL in ->classify() path for historic reasons, so
> >>their ->destroy() implementation can stay as is. If someone actually
> >>cares, they could get cleaned up at some point to avoid the test in fast
> >>path. cls_u32 doesn't set tp->root to NULL. For cls_rsvp, I just added a
> >>!head should anyone actually be using/testing it, so it at least aligns with
> >>cls_fw and cls_route. For cls_flower we additionally need to defer rhashtable
> >>destruction (to a sleepable context) after RCU grace period as concurrent
> >>readers might still access it. (Note that in this case we need to hold module
> >>reference to keep work callback address intact, since we only wait on module
> >>unload for all call_rcu()s to finish.)
> >>
> >>This fixes one race to bring RCU grace period guarantees back. Next step
> >>as worked on by Cong however is to fix 1e052be69d04 ("net_sched: destroy
> >>proto tp when all filters are gone") to get the order of unlinking the tp
> >>in tc_ctl_tfilter() for the RTM_DELTFILTER case right by moving
> >>RCU_INIT_POINTER() before tcf_destroy() and let the notification for
> >>removal be done through the prior ->delete() callback. Both are independant
> >>issues. Once we have that right, we can then clean tp->root up for a number
> >>of classifiers by not making them RCU pointers, which requires a new callback
> >>(->uninit) that is triggered from tp's RCU callback, where we just kfree()
> >>tp->root from there.
> >
> >Looks good to my eyes,
> >
> >Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> Thanks for the review (also to John)!
>
> >The ugly part is the work struct, I am not an RCU expert so don't know if we
> >have any API to execute an RCU callback in process context. Paul?
>
> Same way we do this in BPF with prog destruction, by the way. I'm not aware
> of any callback API for RCU that lets you do this, but maybe Paul knows.
RCU callbacks are always executed in softirq context, so yes, you do need
to use something like a work struct. (Or a wakeup to a kthread or
whatever.)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH net] net, sched: respect rcu grace period on cls destruction
From: Daniel Borkmann @ 2016-11-28 10:51 UTC (permalink / raw)
To: paulmck
Cc: Cong Wang, David Miller, John Fastabend, Roi Dayan, ast,
Hannes Frederic Sowa, Jiri Pirko, Linux Kernel Network Developers
In-Reply-To: <20161128104736.GX31360@linux.vnet.ibm.com>
On 11/28/2016 11:47 AM, Paul E. McKenney wrote:
> On Mon, Nov 28, 2016 at 10:09:16AM +0100, Daniel Borkmann wrote:
>> On 11/28/2016 07:57 AM, Cong Wang wrote:
[...]
>>> The ugly part is the work struct, I am not an RCU expert so don't know if we
>>> have any API to execute an RCU callback in process context. Paul?
>>
>> Same way we do this in BPF with prog destruction, by the way. I'm not aware
>> of any callback API for RCU that lets you do this, but maybe Paul knows.
>
> RCU callbacks are always executed in softirq context, so yes, you do need
> to use something like a work struct. (Or a wakeup to a kthread or
> whatever.)
Ok, thanks for the confirmation!
Daniel
^ permalink raw reply
* Re: [PATCH net-next 0/5] net: add protocol level recvmmsg support
From: Paolo Abeni @ 2016-11-28 10:52 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, David S. Miller, Eric Dumazet, Hannes Frederic Sowa,
Sabrina Dubroca
In-Reply-To: <20161125183711.675fa4a7@redhat.com>
Hi Jesper,
On Fri, 2016-11-25 at 18:37 +0100, Jesper Dangaard Brouer wrote:
> > The measured performance delta is as follow:
> >
> > before after
> > (Kpps) (Kpps)
> >
> > udp flood[1] 570 1800(+215%)
> > max tput[2] 1850 3500(+89%)
> > single queue[3] 1850 1630(-11%)
> >
> > [1] line rate flood using multiple 64 bytes packets and multiple flows
>
> Is [1] sending multiple flow in the a single UDP-sink?
Yes, in the test scenario [1] there are multiple UDP flows using 16
different rx queues on the receiver host, and a single user space
reader.
> > [2] like [1], but using the minimum number of flows to saturate the user space
> > sink, that is 1 flow for the old kernel and 3 for the patched one.
> > the tput increases since the contention on the rx lock is low.
> > [3] like [1] but using a single flow with both old and new kernel. All the
> > packets land on the same rx queue and there is a single ksoftirqd instance
> > running
>
> It is important to know, if ksoftirqd and the UDP-sink runs on the same CPU?
No pinning is enforced. The scheduler moves the user space process on a
different cpu in respect to the ksoftriqd kernel thread.
> > The regression in the single queue scenario is actually due to the improved
> > performance of the recvmmsg() syscall: the user space process is now
> > significantly faster than the ksoftirqd process so that the latter needs often
> > to wake up the user space process.
>
> When measuring these things, make sure that we/you measure both the packets
> actually received in the userspace UDP-sink, and also measure packets
> RX processed by ksoftirq (and I often also look at what HW got delivered).
> Some times, when userspace is too slow, the kernel can/will drop packets.
>
> It is actually quite easily verified with cmdline:
>
> nstat > /dev/null && sleep 1 && nstat
>
> For HW measurements I use the tool ethtool_stats.pl:
> https://github.com/netoptimizer/network-testing/blob/master/bin/ethtool_stats.pl
We collected the UDP stats for all the three scenarios; we have lot of
drop in test[1] and little, by design, in test[2]. In test [3], with the
patched kernel, the drops are 0: ksoftirqd is way slower than the user
space sink.
> > Since ksoftirqd is the bottle-neck is such scenario, overall this causes a
> > tput reduction. In a real use case, where the udp sink is performing some
> > actual processing of the received data, such regression is unlikely to really
> > have an effect.
>
> My experience is that the performance of RX UDP is affected by:
> * if socket is connected or not (yes, RX side also)
> * state of /proc/sys/net/ipv4/ip_early_demux
>
> You don't need to run with all the combinations, but it would be nice
> if you specify what config your have based your measurements on (and
> keep them stable in your runs).
>
> I've actually implemented the "--connect" option to my udp_sink
> program[1] today, but I've not pushed it yet, if you are interested.
The reported numbers are all gathered with unconnected sockets and early
demux enabled.
We also used connected socket for test[3], with relative little
difference (the tput increased for both unpatched and patched kernel,
and the difference was roughly the same).
Paolo
^ permalink raw reply
* Re: ip manpage comments
From: Phil Sutter @ 2016-11-28 10:53 UTC (permalink / raw)
To: Jon LaBadie; +Cc: netdev
In-Reply-To: <20161127024932.GA9954@cyber.jgcomp.com>
Hi,
On Sat, Nov 26, 2016 at 09:49:32PM -0500, Jon LaBadie wrote:
> Though not new to *nix, I am new to using the ip(8) command.
> Thus some of my historical assumptions about ip may be wrong.
>
> It seems that an inclusive manpage for the ip command was
> broken up into a shorter ip(8) manpage and 15 or more
> ip-<subcommand>(8) manpages. I'm basing this assumption
> on long, inclusive manpages on https://linux.die.net/man/8/ip
> and CentOS 6 while CentOS 7 and Fedora 24 each have the
> sub-divided style.
>
> I won't debate the wisdom of this subdivision, only comment
> on how it was done.
>
> The ip(8) manpage make no mention of additional subordinate
> documents. The listing of the additional documents in the
> See Also section is insufficient. This section is typically
> used to mention related commands and other sources of reference
> materials such as info docs, wikis, blogs, or mailing lists.
In what aspect do you think it's insufficient? Only because you don't
think it's the appropriate place to list them or are there any other
reasons? Where do you think would be a better place for them?
Looking at the description of 'SEE ALSO' section in man-pages.7, it is
expected to find "A comma-separated list of related man pages" there.
> When one does investigate one of the subordinate manpages,
> they do not state that they document subcommands of the
> ip command. In fact, on the ip-address(8) manpage it says
>
> The `ip address command' ... (quotes added)
>
> My first thought was "typo", this is the manpage about the
> "ip-address" command. Of course there is no ip-address command.
> But "ip address" is not a command either, it is the "ip" command
> with an argument.
Well, yes and no. While it is true that the command itself on most
systems is 'ip' and 'address' is just the first parameter, by the
internal design 'ip' is merely a dispatcher to the actual subcommand
given. An interesting fact is that main() in ip.c checks argv[0] and in
case it's longer than two characters assumes the remaining part is the
subcommand (i.e. first parameter). So by symlinking 'ip' to 'ipaddress'
I can use the latter as an alternative to calling 'ip address'. You
might argue that the man page then should be named ipaddress.8, but
there is a reason for it not to be like this: Given that most
distributions don't make use of the symlink possibility, the dashed name
allows for opening ip-address.8 using 'man ip address' which is in my
opinion far more intuitive than having to use 'man ipaddress'. (Of
course nothing prevents one from having an ipaddress.8 redirecting to
ip-address.8.)
> There are several commands that have broken their manpage into
> several manpages. Two which come to mind are zsh(1) and perl(1).
> The authors of those pages clearly state on the primary manpage
> that this is an overview page and give clear pointers to the
> additional manpages as well as additional documentation. I would
> recommend reorganizing the ip(8) manpage in a similar fashion.
Well, go ahead if you like. Patches are appreciated, and if you put me
in Cc, I'll promise to help by reviewing them.
> Thank you for consideration of my opinion and for the development
> of an awesome tool.
Please keep in mind that you are talking to a community here, not tech
support of a product you bought. :)
Cheers, Phil
^ permalink raw reply
* [PATCH net V2] net/sched: pedit: make sure that offset is valid
From: Amir Vadai @ 2016-11-28 10:56 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Cong Wang, Jamal Hadi Salim, Or Gerlitz, Hadar Har-Zion,
Jiri Pirko, Amir Vadai
Add a validation function to make sure offset is valid:
1. Not below skb head (could happen when offset is negative).
2. Validate both 'offset' and 'at'.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
Hi Dave,
Please pull to -stable branches.
Changes from V0:
- Add a validation to the 'at' value (this is used as an offset too)
- Instead of validating the output of skb_header_pointer(), make sure that the
offset is good before calling it.
Thanks,
Amir
net/sched/act_pedit.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index b54d56d4959b..cf9b2fe8eac6 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -108,6 +108,17 @@ static void tcf_pedit_cleanup(struct tc_action *a, int bind)
kfree(keys);
}
+static bool offset_valid(struct sk_buff *skb, int offset)
+{
+ if (offset > 0 && offset > skb->len)
+ return false;
+
+ if (offset < 0 && -offset > skb_headroom(skb))
+ return false;
+
+ return true;
+}
+
static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
struct tcf_result *res)
{
@@ -134,6 +145,11 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
if (tkey->offmask) {
char *d, _d;
+ if (!offset_valid(skb, off + tkey->at)) {
+ pr_info("tc filter pedit 'at' offset %d out of bounds\n",
+ off + tkey->at);
+ goto bad;
+ }
d = skb_header_pointer(skb, off + tkey->at, 1,
&_d);
if (!d)
@@ -146,10 +162,10 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
" offset must be on 32 bit boundaries\n");
goto bad;
}
- if (offset > 0 && offset > skb->len) {
- pr_info("tc filter pedit"
- " offset %d can't exceed pkt length %d\n",
- offset, skb->len);
+
+ if (!offset_valid(skb, off + offset)) {
+ pr_info("tc filter pedit offset %d out of bounds\n",
+ offset);
goto bad;
}
--
2.10.2
^ permalink raw reply related
* [PATCH] bpf: cgroup: fix documentation of __cgroup_bpf_update()
From: Daniel Mack @ 2016-11-28 11:04 UTC (permalink / raw)
To: ast; +Cc: davem, netdev, roszenrami, cgroups, Daniel Mack
There's a 'not' missing in one paragraph. Add it.
Signed-off-by: Daniel Mack <daniel@zonque.org>
Reported-by: Rami Rosen <roszenrami@gmail.com>
Fixes: 3007098494be ("cgroup: add support for eBPF programs")
---
kernel/bpf/cgroup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index a0ab43f..b708e6e 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -70,9 +70,9 @@ void cgroup_bpf_inherit(struct cgroup *cgrp, struct cgroup *parent)
* releases the one that is currently attached, if any. @prog is then made
* the effective program of type @type in that cgroup.
*
- * If @prog is %NULL, the currently attached program of type @type is released,
- * and the effective program of the parent cgroup (if any) is inherited to
- * @cgrp.
+ * If @prog is not %NULL, the currently attached program of type @type is
+ * released, and the effective program of the parent cgroup (if any) is
+ * inherited to @cgrp.
*
* Then, the descendants of @cgrp are walked and the effective program for
* each of them is set to the effective program of @cgrp unless the
--
2.7.4
^ permalink raw reply related
* Re: SNAT --random & fully is not actually random for ips
From: Pablo Neira Ayuso @ 2016-11-28 11:06 UTC (permalink / raw)
To: Denys Fedoryshchenko; +Cc: Linux Kernel Network Developers, netfilter-devel
In-Reply-To: <97a6a1c557f0f1e6d55d8d09b326f8b1@nuclearcat.com>
On Mon, Nov 28, 2016 at 12:45:59PM +0200, Denys Fedoryshchenko wrote:
> Hello,
>
> I noticed that if i specify -j SNAT with options --random --random-fully
> still it keeps persistence for source IP.
So you specify both?
> Actually truly random src ip required in some scenarios like links balanced
> by IPs, but seems since 2012 at least it is not possible.
>
> But actually if i do something like:
> --- nf_nat_core.c.new 2016-11-28 09:55:54.000000000 +0000
> +++ nf_nat_core.c 2016-11-21 09:11:59.000000000 +0000
> @@ -282,13 +282,9 @@
> * client coming from the same IP (some Internet Banking sites
> * like this), even across reboots.
> */
> - if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
> - j = prandom_u32();
> - } else {
> - j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
> + j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
> range->flags & NF_NAT_RANGE_PERSISTENT ?
> 0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
> - }
>
> full_range = false;
> for (i = 0; i <= max; i++) {
>
> It works as intended. But i guess to not break compatibility it is better
> should be introduced as new option?
> Or maybe there is no really need for such option?
Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
^ permalink raw reply
* SNAT --random & fully is not actually random for ips
From: Denys Fedoryshchenko @ 2016-11-28 10:45 UTC (permalink / raw)
To: Linux Kernel Network Developers, Pablo Neira Ayuso
Hello,
I noticed that if i specify -j SNAT with options --random --random-fully
still it keeps persistence for source IP.
Actually truly random src ip required in some scenarios like links
balanced by IPs, but seems since 2012 at least it is not possible.
But actually if i do something like:
--- nf_nat_core.c.new 2016-11-28 09:55:54.000000000 +0000
+++ nf_nat_core.c 2016-11-21 09:11:59.000000000 +0000
@@ -282,13 +282,9 @@
* client coming from the same IP (some Internet Banking sites
* like this), even across reboots.
*/
- if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
- j = prandom_u32();
- } else {
- j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) /
sizeof(u32),
+ j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
range->flags & NF_NAT_RANGE_PERSISTENT ?
0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
- }
full_range = false;
for (i = 0; i <= max; i++) {
It works as intended. But i guess to not break compatibility it is
better should be introduced as new option?
Or maybe there is no really need for such option?
^ permalink raw reply
* Re: SNAT --random & fully is not actually random for ips
From: Denys Fedoryshchenko @ 2016-11-28 11:12 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Linux Kernel Network Developers, netfilter-devel
In-Reply-To: <20161128110651.GA1024@salvia>
On 2016-11-28 13:06, Pablo Neira Ayuso wrote:
> On Mon, Nov 28, 2016 at 12:45:59PM +0200, Denys Fedoryshchenko wrote:
>> Hello,
>>
>> I noticed that if i specify -j SNAT with options --random
>> --random-fully
>> still it keeps persistence for source IP.
>
> So you specify both?
>
>> Actually truly random src ip required in some scenarios like links
>> balanced
>> by IPs, but seems since 2012 at least it is not possible.
>>
>> But actually if i do something like:
>> --- nf_nat_core.c.new 2016-11-28 09:55:54.000000000 +0000
>> +++ nf_nat_core.c 2016-11-21 09:11:59.000000000 +0000
>> @@ -282,13 +282,9 @@
>> * client coming from the same IP (some Internet Banking sites
>> * like this), even across reboots.
>> */
>> - if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
>> - j = prandom_u32();
>> - } else {
>> - j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) /
>> sizeof(u32),
>> + j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) /
>> sizeof(u32),
>> range->flags & NF_NAT_RANGE_PERSISTENT ?
>> 0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
>> - }
>>
>> full_range = false;
>> for (i = 0; i <= max; i++) {
>>
>> It works as intended. But i guess to not break compatibility it is
>> better
>> should be introduced as new option?
>> Or maybe there is no really need for such option?
>
> Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
Ops, sorry i just did mistake with files, actually it is in reverse (
did this patch, and it worked properly with it, with random source ip).
--- nf_nat_core.c 2016-11-21 09:11:59.000000000 +0000
+++ nf_nat_core.c.new 2016-11-28 09:55:54.000000000 +0000
@@ -282,9 +282,13 @@
* client coming from the same IP (some Internet Banking sites
* like this), even across reboots.
*/
- j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
+ if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
+ j = prandom_u32();
+ } else {
+ j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) /
sizeof(u32),
range->flags & NF_NAT_RANGE_PERSISTENT ?
0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
+ }
full_range = false;
for (i = 0; i <= max; i++) {
This is current situation, RANDOM_FULLY actually does prandom_u32 for
source port only, but not for IP.
IP kept as persistent and kind of predictable, because hash function
based on source ip.
Sure i did tried to specify any combination of flags, but looking to
"find_best_ips_proto" function, it wont have any effect.
^ permalink raw reply
* Re: Crash due to mutex genl_lock called from RCU context
From: Herbert Xu @ 2016-11-28 11:22 UTC (permalink / raw)
To: Cong Wang
Cc: Eric Dumazet, Subash Abhinov Kasiviswanathan, Thomas Graf,
Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXB_LRqE83cWwU0ZDckLWPrMcZg-yD4+NKy+rBVz3YAyw@mail.gmail.com>
On Sun, Nov 27, 2016 at 10:53:21PM -0800, Cong Wang wrote:
>
> I just took a deeper look, some user calls rhashtable_destroy() in ->done(),
> so even removing that genl lock is not enough, perhaps we should just
> move it to a work struct like what Daniel does for the tcf_proto, but that is
> ugly... I don't know if RCU provides any API to execute the callback in process
> context.
I looked into doing it without a worker struct, but basically it
means that we'd have to add more crap to the common code path for
what is essentially a very rare case.
So I think we should go with the worker struct because all we lose
is a bit of memory.
---8<---
netlink: Call cb->done from a worker thread
The cb->done interface expects to be called in process context.
This was broken by the netlink RCU conversion. This patch fixes
it by adding a worker struct to make the cb->done call where
necessary.
Fixes: 21e4902aea80 ("netlink: Lockless lookup with RCU grace...")
Reported-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 62bea45..602e5eb 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -322,14 +322,11 @@ static void netlink_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
sk_mem_charge(sk, skb->truesize);
}
-static void netlink_sock_destruct(struct sock *sk)
+static void __netlink_sock_destruct(struct sock *sk)
{
struct netlink_sock *nlk = nlk_sk(sk);
if (nlk->cb_running) {
- if (nlk->cb.done)
- nlk->cb.done(&nlk->cb);
-
module_put(nlk->cb.module);
kfree_skb(nlk->cb.skb);
}
@@ -346,6 +343,28 @@ static void netlink_sock_destruct(struct sock *sk)
WARN_ON(nlk_sk(sk)->groups);
}
+static void netlink_sock_destruct_work(struct work_struct *work)
+{
+ struct netlink_sock *nlk = container_of(work, struct netlink_sock,
+ work);
+
+ nlk->cb.done(&nlk->cb);
+ __netlink_sock_destruct(&nlk->sk);
+}
+
+static void netlink_sock_destruct(struct sock *sk)
+{
+ struct netlink_sock *nlk = nlk_sk(sk);
+
+ if (nlk->cb_running && nlk->cb.done) {
+ INIT_WORK(&nlk->work, netlink_sock_destruct_work);
+ schedule_work(&nlk->work);
+ return;
+ }
+
+ __netlink_sock_destruct(sk);
+}
+
/* This lock without WQ_FLAG_EXCLUSIVE is good on UP and it is _very_ bad on
* SMP. Look, when several writers sleep and reader wakes them up, all but one
* immediately hit write lock and grab all the cpus. Exclusive sleep solves
diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
index 3cfd6cc..4fdb383 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -3,6 +3,7 @@
#include <linux/rhashtable.h>
#include <linux/atomic.h>
+#include <linux/workqueue.h>
#include <net/sock.h>
#define NLGRPSZ(x) (ALIGN(x, sizeof(unsigned long) * 8) / 8)
@@ -33,6 +34,7 @@ struct netlink_sock {
struct rhash_head node;
struct rcu_head rcu;
+ struct work_struct work;
};
static inline struct netlink_sock *nlk_sk(struct sock *sk)
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH 1/1] NET: usb: cdc_ncm: adding MBIM RESET_FUNCTION request and modifying ncm bind common code
From: Daniele Palmas @ 2016-11-28 11:23 UTC (permalink / raw)
To: Bjørn Mork; +Cc: Oliver Neukum, linux-usb, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87fume7xg5.fsf-3F4PFWf5pNjpjLOzFPqGjWGXanvQGlWp@public.gmane.org>
2016-11-26 22:17 GMT+01:00 Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>:
> Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> writes:
>
>> Finally, I found my modems (or at least a number of them) again today.
>> But I'm sorry to say, that the troublesome Huawei E3372h-153 is still
>> giving us a hard time. It does not work with your patch. The symptom is
>> the same as earlier: The modem returns MBIM frames with 32bit headers.
>>
>> So for now, I have to NAK this patch.
>>
>> I am sure we can find a good solution that makes all of these modems
>> work, but I cannot support a patch that breaks previously working
>> configurations. Sorry. I'll do a few experiments and see if there is a
>> simple fix for this. Otherwise we'll probably have to do the quirk
>> game.
>
>
> This is a proof-of-concept only, but it appears to be working. Please
> test with your device(s) too. It's still mostly your code, as you can
> see.
Sorry, this does not work :-(
The problem is always in the altsetting toggle: if I comment that
part, your patch is working fine.
I'm now wondering if the safest thing is to add a very simple quirk in
cdc_mbim that makes this toggle not to be applied with the buggy
modems and then unconditionally add the RESET_FUNCTION request in
cdc_mbim_bind after cdc_ncm_bind_common has been executed.
Daniele
>
> If this turns out to work, then I believe we should refactor
> cdc_ncm_init() and cdc_ncm_bind_common() to make the whole
> initialisation sequence a bit cleaner. And maybe also include
> cdc_mbim_bind(). Ideally, the MBIM specific RESET should happen there
> instead of "polluting" the NCM driver with MBIM specific code.
>
> But anyway: The sequence that seems to work for both the E3372h-153
> and the EM7455 is
>
> USB_CDC_GET_NTB_PARAMETERS
> USB_CDC_RESET_FUNCTION
> usb_set_interface(dev->udev, 'data interface no', 0);
> remaining parts of cdc_ncm_init(), excluding USB_CDC_GET_NTB_PARAMETERS
> usb_set_interface(dev->udev, 'data interface no', 'data alt setting');
>
> without any additional delay between the two usb_set_interface() calls.
> So the major difference from your patch is that I moved the two control
> requests out of cdc_ncm_init() to allow running them _before_ setting
> the data interface to altsetting 0.
>
> But maybe I was just lucky. This was barely proof tested. Needs a lot
> more testing and cleanups as suggested. I'd appreciate it if you
> continued that, as I don't really have any time for it...
>
> FWIW, I also ran a quick test with a D-Link DWM-156A7 (Mediatek MBIM
> firmware) and a Huawei E367 (Qualcomm device with early Huawei MBIM
> firmware, distinctly different from the E3372h-153 and most other
> MBIM devices I've seen)
>
>
>
> Bjørn
>
> ---
> drivers/net/usb/cdc_ncm.c | 48 ++++++++++++++++++++++++++++----------------
> include/uapi/linux/usb/cdc.h | 1 +
> 2 files changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 877c9516e781..be019cbf1719 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -488,16 +488,6 @@ static int cdc_ncm_init(struct usbnet *dev)
> u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
> int err;
>
> - err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
> - USB_TYPE_CLASS | USB_DIR_IN
> - |USB_RECIP_INTERFACE,
> - 0, iface_no, &ctx->ncm_parm,
> - sizeof(ctx->ncm_parm));
> - if (err < 0) {
> - dev_err(&dev->intf->dev, "failed GET_NTB_PARAMETERS\n");
> - return err; /* GET_NTB_PARAMETERS is required */
> - }
> -
> /* set CRC Mode */
> if (cdc_ncm_flags(dev) & USB_CDC_NCM_NCAP_CRC_MODE) {
> dev_dbg(&dev->intf->dev, "Setting CRC mode off\n");
> @@ -837,12 +827,43 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
> }
> }
>
> + iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
> + temp = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
> + USB_TYPE_CLASS | USB_DIR_IN
> + | USB_RECIP_INTERFACE,
> + 0, iface_no, &ctx->ncm_parm,
> + sizeof(ctx->ncm_parm));
> + if (temp < 0) {
> + dev_err(&dev->intf->dev, "failed GET_NTB_PARAMETERS\n");
> + goto error; /* GET_NTB_PARAMETERS is required */
> + }
> +
> + /* Some modems (e.g. Telit LE922A6) need to reset the MBIM function
> + * or they will fail to work properly.
> + * For details on RESET_FUNCTION request see document
> + * "USB Communication Class Subclass Specification for MBIM"
> + * RESET_FUNCTION should be harmless for all the other MBIM modems
> + */
> + if (cdc_ncm_comm_intf_is_mbim(ctx->control->cur_altsetting)) {
> + temp = usbnet_write_cmd(dev, USB_CDC_RESET_FUNCTION,
> + USB_TYPE_CLASS | USB_DIR_OUT
> + | USB_RECIP_INTERFACE,
> + 0, iface_no, NULL, 0);
> + if (temp < 0)
> + dev_err(&dev->intf->dev, "failed RESET_FUNCTION\n");
> + }
> +
> iface_no = ctx->data->cur_altsetting->desc.bInterfaceNumber;
>
> /* Reset data interface. Some devices will not reset properly
> * unless they are configured first. Toggle the altsetting to
> * force a reset
> + * This is applied only to ncm devices, since it has been verified
> + * to cause issues with some MBIM modems (e.g. Telit LE922A6).
> + * MBIM devices reset is achieved using MBIM request RESET_FUNCTION
> + * in cdc_ncm_init
> */
> +
> usb_set_interface(dev->udev, iface_no, data_altsetting);
> temp = usb_set_interface(dev->udev, iface_no, 0);
> if (temp) {
> @@ -854,13 +875,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
> if (cdc_ncm_init(dev))
> goto error2;
>
> - /* Some firmwares need a pause here or they will silently fail
> - * to set up the interface properly. This value was decided
> - * empirically on a Sierra Wireless MC7455 running 02.08.02.00
> - * firmware.
> - */
> - usleep_range(10000, 20000);
> -
> /* configure data interface */
> temp = usb_set_interface(dev->udev, iface_no, data_altsetting);
> if (temp) {
> diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
> index e2bc417b243b..30258fb229e6 100644
> --- a/include/uapi/linux/usb/cdc.h
> +++ b/include/uapi/linux/usb/cdc.h
> @@ -231,6 +231,7 @@ struct usb_cdc_mbim_extended_desc {
>
> #define USB_CDC_SEND_ENCAPSULATED_COMMAND 0x00
> #define USB_CDC_GET_ENCAPSULATED_RESPONSE 0x01
> +#define USB_CDC_RESET_FUNCTION 0x05
> #define USB_CDC_REQ_SET_LINE_CODING 0x20
> #define USB_CDC_REQ_GET_LINE_CODING 0x21
> #define USB_CDC_REQ_SET_CONTROL_LINE_STATE 0x22
> --
> 2.10.2
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: SNAT --random & fully is not actually random for ips
From: Pablo Neira Ayuso @ 2016-11-28 11:29 UTC (permalink / raw)
To: Denys Fedoryshchenko; +Cc: Linux Kernel Network Developers, netfilter-devel
In-Reply-To: <fb3151804cc3b2c57e69d1059b3417ee@nuclearcat.com>
On Mon, Nov 28, 2016 at 01:12:07PM +0200, Denys Fedoryshchenko wrote:
> On 2016-11-28 13:06, Pablo Neira Ayuso wrote:
> >Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
>
> Ops, sorry i just did mistake with files, actually it is in reverse ( did
> this patch, and it worked properly with it, with random source ip).
Oh, I see 8)
> --- nf_nat_core.c 2016-11-21 09:11:59.000000000 +0000
> +++ nf_nat_core.c.new 2016-11-28 09:55:54.000000000 +0000
> @@ -282,9 +282,13 @@
> * client coming from the same IP (some Internet Banking sites
> * like this), even across reboots.
> */
> - j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
> + if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
> + j = prandom_u32();
> + } else {
> + j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) / sizeof(u32),
> range->flags & NF_NAT_RANGE_PERSISTENT ?
> 0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
> + }
>
> full_range = false;
> for (i = 0; i <= max; i++) {
>
> This is current situation, RANDOM_FULLY actually does prandom_u32 for source
> port only, but not for IP.
> IP kept as persistent and kind of predictable, because hash function based
> on source ip.
>
> Sure i did tried to specify any combination of flags, but looking to
> "find_best_ips_proto" function, it wont have any effect.
IIRC the original intention on random-fully was to cover only ports.
Did you interpret from git history otherwise? Otherwise, safe
procedure is to add a new flag.
^ permalink raw reply
* Re: [PATCH net-next 1/2] sfc: separate out SFC4000 ("Falcon") support into new sfc-falcon driver
From: Edward Cree @ 2016-11-28 11:29 UTC (permalink / raw)
To: kbuild test robot; +Cc: kbuild-all, linux-net-drivers, davem, bkenward, netdev
In-Reply-To: <201611260855.aKMo9XWI%fengguang.wu@intel.com>
On 26/11/16 00:58, kbuild test robot wrote:
> Hi Edward,
>
> [auto build test ERROR on net-next/master]
>
> url: https://github.com/0day-ci/linux/commits/Edward-Cree/sfc-split-out-Falcon-driver/20161126-033439
> config: i386-randconfig-h1-11260702 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: the linux-review/Edward-Cree/sfc-split-out-Falcon-driver/20161126-033439 HEAD 738d215da7cf33cb4f2916dfba4fdb1558829e5a builds fine.
> It only hurts bisectibility.
>
> All error/warnings (new ones prefixed by >>):
>
> drivers/net/ethernet/sfc/falcon/built-in.o: In function `tenxpress_set_id_led':
>>> (.text+0x28db4): multiple definition of `tenxpress_set_id_led'
> drivers/net/ethernet/sfc/built-in.o:(.text+0x3bac7): first defined here
Right, I have to do at least some of the rip-stuff-out in the first patch, else
building both drivers built-in breaks. (I only tested building them as modules,
that was silly of me.)
Wondering if the right thing to do is just squash both patches together: it
could hardly make the series _less_ reviewable, and it might help git recognise
more of the copies as file renames.
Opinions?
-Ed
^ permalink raw reply
* Re: SNAT --random & fully is not actually random for ips
From: Denys Fedoryshchenko @ 2016-11-28 11:35 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Linux Kernel Network Developers, netfilter-devel
In-Reply-To: <20161128112955.GA1691@salvia>
On 2016-11-28 13:29, Pablo Neira Ayuso wrote:
> On Mon, Nov 28, 2016 at 01:12:07PM +0200, Denys Fedoryshchenko wrote:
>> On 2016-11-28 13:06, Pablo Neira Ayuso wrote:
>> >Why does your patch reverts NF_NAT_RANGE_PROTO_RANDOM_FULLY?
>>
>> Ops, sorry i just did mistake with files, actually it is in reverse (
>> did
>> this patch, and it worked properly with it, with random source ip).
>
> Oh, I see 8)
>
>> --- nf_nat_core.c 2016-11-21 09:11:59.000000000 +0000
>> +++ nf_nat_core.c.new 2016-11-28 09:55:54.000000000 +0000
>> @@ -282,9 +282,13 @@
>> * client coming from the same IP (some Internet Banking sites
>> * like this), even across reboots.
>> */
>> - j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) /
>> sizeof(u32),
>> + if (range->flags & NF_NAT_RANGE_PROTO_RANDOM_FULLY) {
>> + j = prandom_u32();
>> + } else {
>> + j = jhash2((u32 *)&tuple->src.u3, sizeof(tuple->src.u3) /
>> sizeof(u32),
>> range->flags & NF_NAT_RANGE_PERSISTENT ?
>> 0 : (__force u32)tuple->dst.u3.all[max] ^ zone->id);
>> + }
>>
>> full_range = false;
>> for (i = 0; i <= max; i++) {
>>
>> This is current situation, RANDOM_FULLY actually does prandom_u32 for
>> source
>> port only, but not for IP.
>> IP kept as persistent and kind of predictable, because hash function
>> based
>> on source ip.
>>
>> Sure i did tried to specify any combination of flags, but looking to
>> "find_best_ips_proto" function, it wont have any effect.
>
> IIRC the original intention on random-fully was to cover only ports.
> Did you interpret from git history otherwise? Otherwise, safe
> procedure is to add a new flag.
No, seems i didnt read man page well, sorry.
I will check it, maybe will try to add new option and submit a patch,
still studying impact on "balancing" with this change, seems it works
great.
But not really sure such thing needed for someone else, actually some
might have privacy concerns as well, and can use such option for
privacy.
^ 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