* Re: [PATCH] net: ethernet: mvneta: add support for 2.5G DRSGMII mode
From: Andrew Lunn @ 2017-01-24 13:19 UTC (permalink / raw)
To: Jan Lübbe
Cc: netdev, devicetree, davem, Rob Herring, Mark Rutland,
Thomas Petazzoni, Florian Fainelli
In-Reply-To: <1485246858.5508.47.camel@pengutronix.de>
> It works without any other changes. ;)
>
> Initially I looked at adding a SPEED_2500, but I wasn't sure which code
> would need to handle the new value. Would the path from the ethtool
> ioctl to the driver be enough (mvneta_ethtool_set_link_ksettings() as
> you said)?
You have this connected to an FPGA? Do you have a traditional PHY in
the FPGA? Or are you implementing something more like an Ethernet
switch?
I don't know this driver too well, but what often happens is the PHY
performs autoneg and the phylib then calls the adjust_link function to
set the MAC to what has been negotiated. So if you have a PHY attached
which can do 2.5Ghz, you might be seeing calls to adjust_link with
SPEED_2500.
What does ethtool <devname> show?
Thanks
Andrew
^ permalink raw reply
* [PATCH net 0/3] alx: fix fallout from multi queue conversion
From: Tobias Regnery @ 2017-01-24 13:34 UTC (permalink / raw)
To: netdev, jcliburn, chris.snook; +Cc: davem, Tobias Regnery
Here are 3 fixes for the multi queue conversion in v4.10.
The first patch fixes a wrong condition in an if statement.
Patches 2 and 3 fixes regressions in the corner case when requesting msi-x
interrupts fails and we fall back to msi or legacy interrupts.
Tobias Regnery (3):
alx: fix wrong condition to free descriptor memory
alx: fix fallback to msi or legacy interrupts
alx: work around hardware bug in interrupt fallback path
drivers/net/ethernet/atheros/alx/main.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH net 2/3] alx: fix fallback to msi or legacy interrupts
From: Tobias Regnery @ 2017-01-24 13:34 UTC (permalink / raw)
To: netdev, jcliburn, chris.snook; +Cc: davem, Tobias Regnery
In-Reply-To: <cover.1485263833.git.tobias.regnery@gmail.com>
If requesting msi-x interrupts fails we should fall back to msi or
legacy interrupts. However alx_realloc_ressources don't call
alx_init_intr, so we fail to set the right number of tx queues.
This results in watchdog timeouts and a nonfunctional adapter.
Fixes: d768319cd427 ("alx: enable multiple tx queues")
Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
---
drivers/net/ethernet/atheros/alx/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 765306bd78c2..75cbd46e429d 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -984,6 +984,7 @@ static int alx_realloc_resources(struct alx_priv *alx)
alx_free_rings(alx);
alx_free_napis(alx);
alx_disable_advanced_intr(alx);
+ alx_init_intr(alx, false);
err = alx_alloc_napis(alx);
if (err)
--
2.9.3
^ permalink raw reply related
* [PATCH net 1/3] alx: fix wrong condition to free descriptor memory
From: Tobias Regnery @ 2017-01-24 13:34 UTC (permalink / raw)
To: netdev, jcliburn, chris.snook; +Cc: davem, Tobias Regnery
In-Reply-To: <cover.1485263833.git.tobias.regnery@gmail.com>
The condition to free the descriptor memory is wrong, we want to free the
memory if it is set and not if it is unset. Invert the test to fix this
issue.
Fixes: b0999223f224b ("alx: add ability to allocate and free alx_napi structures")
Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
---
drivers/net/ethernet/atheros/alx/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index c8f525574d68..765306bd78c2 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -703,7 +703,7 @@ static void alx_free_rings(struct alx_priv *alx)
if (alx->qnapi[0] && alx->qnapi[0]->rxq)
kfree(alx->qnapi[0]->rxq->bufs);
- if (!alx->descmem.virt)
+ if (alx->descmem.virt)
dma_free_coherent(&alx->hw.pdev->dev,
alx->descmem.size,
alx->descmem.virt,
--
2.9.3
^ permalink raw reply related
* [PATCH net 3/3] alx: work around hardware bug in interrupt fallback path
From: Tobias Regnery @ 2017-01-24 13:34 UTC (permalink / raw)
To: netdev, jcliburn, chris.snook; +Cc: davem, Tobias Regnery
In-Reply-To: <cover.1485263833.git.tobias.regnery@gmail.com>
If requesting msi-x interrupts fails in alx_request_irq we fall back to
a single tx queue and msi or legacy interrupts.
Currently the adapter stops working in this case and we get tx watchdog
timeouts. For reasons unknown the adapter gets confused when we load the
dma adresses to the chip in alx_init_ring_ptrs twice: the first time with
multiple queues and the second time in the fallback case with a single
queue.
To fix this move the the call to alx_reinit_rings (which calls
alx_init_ring_ptrs) after alx_request_irq. At this time it is clear how
much tx queues we have and which dma addresses we use.
Fixes: d768319cd427 ("alx: enable multiple tx queues")
Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>
---
drivers/net/ethernet/atheros/alx/main.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 75cbd46e429d..7dcc907a449d 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -685,8 +685,6 @@ static int alx_alloc_rings(struct alx_priv *alx)
return -ENOMEM;
}
- alx_reinit_rings(alx);
-
return 0;
}
@@ -1242,6 +1240,12 @@ static int __alx_open(struct alx_priv *alx, bool resume)
if (err)
goto out_free_rings;
+ /* must be called after alx_request_irq because the chip stops working
+ * if we copy the dma addresses in alx_init_ring_ptrs twice when
+ * requesting msi-x interrupts failed
+ */
+ alx_reinit_rings(alx);
+
netif_set_real_num_tx_queues(alx->dev, alx->num_txq);
netif_set_real_num_rx_queues(alx->dev, alx->num_rxq);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 4/5] net: dsa: mv88e6xxx: Support multiple MDIO busses
From: Andrew Lunn @ 2017-01-24 13:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-1-git-send-email-andrew@lunn.ch>
The mv88e6390 has multiple MDIO busses. Generalize the parsing of the
device tree to support multiple mdio nodes. The external mdio bus has
a compatible strings to indicate it is external.
Keep a linked list of busses, placing the external mdio bus at the
tail of the list. When within the driver an mdio bus is needed,
e.g. for EEE or SERDES, use the head of the list which should be the
internal bus.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
.../devicetree/bindings/net/dsa/marvell.txt | 41 +++++++-
drivers/net/dsa/mv88e6xxx/chip.c | 110 +++++++++++++++------
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 10 +-
3 files changed, 126 insertions(+), 35 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index b3dd6b40e0de..86a05563c8ae 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -26,8 +26,12 @@ Optional properties:
- interrupt-controller : Indicates the switch is itself an interrupt
controller. This is used for the PHY interrupts.
#interrupt-cells = <2> : Controller uses two cells, number and flag
-- mdio : container of PHY and devices on the switches MDIO
- bus
+- mdio : Container of PHY and devices on the switches MDIO
+ bus.
+- mdio? : Container of PHYs and devices on the external MDIO
+ bus. The node must contains a compatible string of
+ "marvell,mv88e6xxx-mdio-external"
+
Example:
mdio {
@@ -53,3 +57,36 @@ Example:
};
};
};
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ switch0: switch@0 {
+ compatible = "marvell,mv88e6390";
+ reg = <0>;
+ reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+ };
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ switch1phy0: switch1phy0@0 {
+ reg = <0>;
+ interrupt-parent = <&switch0>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
+ mdio1 {
+ compatible = "marvell,mv88e6xxx-mdio-external";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ switch1phy9: switch1phy0@9 {
+ reg = <9>;
+ };
+ };
+ };
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b0fd1432f4f3..5668e778ed1d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -236,16 +236,29 @@ static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip,
return mv88e6xxx_write(chip, addr, reg, val);
}
+static struct mii_bus *mv88e6xxx_default_mdio_bus(struct mv88e6xxx_chip *chip)
+{
+ struct mv88e6xxx_mdio_bus *mdio_bus;
+
+ mdio_bus = list_first_entry(&chip->mdios, struct mv88e6xxx_mdio_bus,
+ list);
+ if (!mdio_bus)
+ return NULL;
+
+ return mdio_bus->bus;
+}
+
static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
int reg, u16 *val)
{
int addr = phy; /* PHY devices addresses start at 0x0 */
- struct mii_bus *bus = chip->mdio_bus;
+ struct mii_bus *bus;
- if (!chip->info->ops->phy_read)
+ bus = mv88e6xxx_default_mdio_bus(chip);
+ if (!bus)
return -EOPNOTSUPP;
- if (!bus)
+ if (!chip->info->ops->phy_read)
return -EOPNOTSUPP;
return chip->info->ops->phy_read(chip, bus, addr, reg, val);
@@ -255,12 +268,13 @@ static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
int reg, u16 val)
{
int addr = phy; /* PHY devices addresses start at 0x0 */
- struct mii_bus *bus = chip->mdio_bus;
+ struct mii_bus *bus;
- if (!chip->info->ops->phy_write)
+ bus = mv88e6xxx_default_mdio_bus(chip);
+ if (!bus)
return -EOPNOTSUPP;
- if (!bus)
+ if (!chip->info->ops->phy_write)
return -EOPNOTSUPP;
return chip->info->ops->phy_write(chip, bus, addr, reg, val);
@@ -2845,7 +2859,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
int i;
chip->ds = ds;
- ds->slave_mii_bus = chip->mdio_bus;
+ ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
mutex_lock(&chip->reg_lock);
@@ -2940,22 +2954,23 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
}
static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
- struct device_node *np)
+ struct device_node *np,
+ bool external)
{
static int index;
struct mv88e6xxx_mdio_bus *mdio_bus;
struct mii_bus *bus;
int err;
- if (np)
- chip->mdio_np = of_get_child_by_name(np, "mdio");
-
bus = devm_mdiobus_alloc_size(chip->dev, sizeof(*mdio_bus));
if (!bus)
return -ENOMEM;
mdio_bus = bus->priv;
+ mdio_bus->bus = bus;
mdio_bus->chip = chip;
+ INIT_LIST_HEAD(&mdio_bus->list);
+ mdio_bus->external = external;
if (np) {
bus->name = np->full_name;
@@ -2969,34 +2984,72 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
bus->write = mv88e6xxx_mdio_write;
bus->parent = chip->dev;
- if (chip->mdio_np)
- err = of_mdiobus_register(bus, chip->mdio_np);
+ if (np)
+ err = of_mdiobus_register(bus, np);
else
err = mdiobus_register(bus);
if (err) {
dev_err(chip->dev, "Cannot register MDIO bus (%d)\n", err);
- goto out;
+ return err;
}
- chip->mdio_bus = bus;
+
+ if (external)
+ list_add_tail(&mdio_bus->list, &chip->mdios);
+ else
+ list_add(&mdio_bus->list, &chip->mdios);
return 0;
+}
-out:
- if (chip->mdio_np)
- of_node_put(chip->mdio_np);
+static const struct of_device_id mv88e6xxx_mdio_external_match[] = {
+ { .compatible = "marvell,mv88e6xxx-mdio-external",
+ .data = (void *)true },
+ { },
+};
- return err;
+static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
+ struct device_node *np)
+{
+ const struct of_device_id *match;
+ struct device_node *child;
+ int err;
+
+ /* Always register one mdio bus for the internal/default mdio
+ * bus. This maybe represented in the device tree, but is
+ * optional.
+ */
+ child = of_get_child_by_name(np, "mdio");
+ err = mv88e6xxx_mdio_register(chip, child, false);
+ if (err)
+ return err;
+
+ /* Walk the device tree, and see if there are any other nodes
+ * which say they are compatible with the external mdio
+ * bus.
+ */
+ for_each_available_child_of_node(np, child) {
+ match = of_match_node(mv88e6xxx_mdio_external_match, child);
+ if (match) {
+ err = mv88e6xxx_mdio_register(chip, child, true);
+ if (err)
+ return err;
+ }
+ }
+
+ return 0;
}
-static void mv88e6xxx_mdio_unregister(struct mv88e6xxx_chip *chip)
+static void mv88e6xxx_mdios_unregister(struct mv88e6xxx_chip *chip)
{
- struct mii_bus *bus = chip->mdio_bus;
+ struct mv88e6xxx_mdio_bus *mdio_bus;
+ struct mii_bus *bus;
- mdiobus_unregister(bus);
+ list_for_each_entry(mdio_bus, &chip->mdios, list) {
+ bus = mdio_bus->bus;
- if (chip->mdio_np)
- of_node_put(chip->mdio_np);
+ mdiobus_unregister(bus);
+ }
}
static int mv88e6xxx_get_eeprom_len(struct dsa_switch *ds)
@@ -4123,6 +4176,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
chip->dev = dev;
mutex_init(&chip->reg_lock);
+ INIT_LIST_HEAD(&chip->mdios);
return chip;
}
@@ -4197,7 +4251,7 @@ static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
mv88e6xxx_phy_init(chip);
- err = mv88e6xxx_mdio_register(chip, NULL);
+ err = mv88e6xxx_mdios_register(chip, NULL);
if (err)
goto free;
@@ -4398,7 +4452,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
}
}
- err = mv88e6xxx_mdio_register(chip, np);
+ err = mv88e6xxx_mdios_register(chip, np);
if (err)
goto out_g2_irq;
@@ -4409,7 +4463,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
return 0;
out_mdio:
- mv88e6xxx_mdio_unregister(chip);
+ mv88e6xxx_mdios_unregister(chip);
out_g2_irq:
if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT) && chip->irq > 0)
mv88e6xxx_g2_irq_free(chip);
@@ -4430,7 +4484,7 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
mv88e6xxx_phy_destroy(chip);
mv88e6xxx_unregister_switch(chip);
- mv88e6xxx_mdio_unregister(chip);
+ mv88e6xxx_mdios_unregister(chip);
if (chip->irq > 0) {
if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_G2_INT))
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 6f7ddb594809..7d24add45e74 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -730,11 +730,8 @@ struct mv88e6xxx_chip {
/* set to size of eeprom if supported by the switch */
int eeprom_len;
- /* Device node for the MDIO bus */
- struct device_node *mdio_np;
-
- /* And the MDIO bus itself */
- struct mii_bus *mdio_bus;
+ /* List of mdio busses */
+ struct list_head mdios;
/* There can be two interrupt controllers, which are chained
* off a GPIO as interrupt source
@@ -751,7 +748,10 @@ struct mv88e6xxx_bus_ops {
};
struct mv88e6xxx_mdio_bus {
+ struct mii_bus *bus;
struct mv88e6xxx_chip *chip;
+ struct list_head list;
+ bool external;
};
struct mv88e6xxx_ops {
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/5] net: dsa: mv88e6xxx: Add mdio private structure
From: Andrew Lunn @ 2017-01-24 13:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-1-git-send-email-andrew@lunn.ch>
Have the MDIO bus driver code allocate a private structure and make
the chip a member of it. This will allow us to add further members in
the future.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 13 +++++++++----
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 4 ++++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index dedccf201452..b0fd1432f4f3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2902,7 +2902,8 @@ static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
{
- struct mv88e6xxx_chip *chip = bus->priv;
+ struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+ struct mv88e6xxx_chip *chip = mdio_bus->chip;
u16 val;
int err;
@@ -2921,7 +2922,8 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
{
- struct mv88e6xxx_chip *chip = bus->priv;
+ struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+ struct mv88e6xxx_chip *chip = mdio_bus->chip;
int err;
if (phy >= mv88e6xxx_num_ports(chip))
@@ -2941,17 +2943,20 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
struct device_node *np)
{
static int index;
+ struct mv88e6xxx_mdio_bus *mdio_bus;
struct mii_bus *bus;
int err;
if (np)
chip->mdio_np = of_get_child_by_name(np, "mdio");
- bus = devm_mdiobus_alloc(chip->dev);
+ bus = devm_mdiobus_alloc_size(chip->dev, sizeof(*mdio_bus));
if (!bus)
return -ENOMEM;
- bus->priv = (void *)chip;
+ mdio_bus = bus->priv;
+ mdio_bus->chip = chip;
+
if (np) {
bus->name = np->full_name;
snprintf(bus->id, MII_BUS_ID_SIZE, "%s", np->full_name);
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 7d75dd546bf7..6f7ddb594809 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -750,6 +750,10 @@ struct mv88e6xxx_bus_ops {
int (*write)(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val);
};
+struct mv88e6xxx_mdio_bus {
+ struct mv88e6xxx_chip *chip;
+};
+
struct mv88e6xxx_ops {
int (*get_eeprom)(struct mv88e6xxx_chip *chip,
struct ethtool_eeprom *eeprom, u8 *data);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/5] net: dsa: mv88e6xxx: Pass mii_bus to all PHY operations
From: Andrew Lunn @ 2017-01-24 13:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-1-git-send-email-andrew@lunn.ch>
In preparation for supporting multiple MDIO busses, pass the mii_bus
structure to all PHY operations. It will in future then be clear on
which MDIO bus the operation should be performed.
For reads/write from phylib, the mii_bus is readily available. However
some internal code also access the PHY, e.g. for EEE and SERDES. Make
this code use the one and only currently available MDIO bus.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 42 +++++++++++++++++++++++++----------
drivers/net/dsa/mv88e6xxx/global2.c | 10 +++++----
drivers/net/dsa/mv88e6xxx/global2.h | 12 ++++++----
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 10 +++++----
4 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 374d887e8256..dedccf201452 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -222,14 +222,16 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val)
return 0;
}
-static int mv88e6165_phy_read(struct mv88e6xxx_chip *chip, int addr,
- int reg, u16 *val)
+static int mv88e6165_phy_read(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 *val)
{
return mv88e6xxx_read(chip, addr, reg, val);
}
-static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip, int addr,
- int reg, u16 val)
+static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 val)
{
return mv88e6xxx_write(chip, addr, reg, val);
}
@@ -238,22 +240,30 @@ static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
int reg, u16 *val)
{
int addr = phy; /* PHY devices addresses start at 0x0 */
+ struct mii_bus *bus = chip->mdio_bus;
if (!chip->info->ops->phy_read)
return -EOPNOTSUPP;
- return chip->info->ops->phy_read(chip, addr, reg, val);
+ if (!bus)
+ return -EOPNOTSUPP;
+
+ return chip->info->ops->phy_read(chip, bus, addr, reg, val);
}
static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
int reg, u16 val)
{
int addr = phy; /* PHY devices addresses start at 0x0 */
+ struct mii_bus *bus = chip->mdio_bus;
if (!chip->info->ops->phy_write)
return -EOPNOTSUPP;
- return chip->info->ops->phy_write(chip, addr, reg, val);
+ if (!bus)
+ return -EOPNOTSUPP;
+
+ return chip->info->ops->phy_write(chip, bus, addr, reg, val);
}
static int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page)
@@ -623,8 +633,9 @@ static void mv88e6xxx_ppu_state_destroy(struct mv88e6xxx_chip *chip)
del_timer_sync(&chip->ppu_timer);
}
-static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip, int addr,
- int reg, u16 *val)
+static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 *val)
{
int err;
@@ -637,8 +648,9 @@ static int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip, int addr,
return err;
}
-static int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip, int addr,
- int reg, u16 val)
+static int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 val)
{
int err;
@@ -2897,8 +2909,11 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
if (phy >= mv88e6xxx_num_ports(chip))
return 0xffff;
+ if (!chip->info->ops->phy_read)
+ return -EOPNOTSUPP;
+
mutex_lock(&chip->reg_lock);
- err = mv88e6xxx_phy_read(chip, phy, reg, &val);
+ err = chip->info->ops->phy_read(chip, bus, phy, reg, &val);
mutex_unlock(&chip->reg_lock);
return err ? err : val;
@@ -2912,8 +2927,11 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
if (phy >= mv88e6xxx_num_ports(chip))
return 0xffff;
+ if (!chip->info->ops->phy_write)
+ return -EOPNOTSUPP;
+
mutex_lock(&chip->reg_lock);
- err = mv88e6xxx_phy_write(chip, phy, reg, val);
+ err = chip->info->ops->phy_write(chip, bus, phy, reg, val);
mutex_unlock(&chip->reg_lock);
return err;
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index ead2e265c9ef..1f9a12a1fad9 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -501,8 +501,9 @@ static int mv88e6xxx_g2_smi_phy_cmd(struct mv88e6xxx_chip *chip, u16 cmd)
return mv88e6xxx_g2_smi_phy_wait(chip);
}
-int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg,
- u16 *val)
+int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 *val)
{
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
int err;
@@ -518,8 +519,9 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg,
return mv88e6xxx_g2_read(chip, GLOBAL2_SMI_PHY_DATA, val);
}
-int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr, int reg,
- u16 val)
+int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 val)
{
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg;
int err;
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 2918f22388f7..00e635279ba1 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -23,10 +23,12 @@ static inline int mv88e6xxx_g2_require(struct mv88e6xxx_chip *chip)
return 0;
}
-int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, int addr, int reg,
- u16 *val);
-int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, int addr, int reg,
- u16 val);
+int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 *val);
+int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 val);
int mv88e6xxx_g2_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr);
int mv88e6xxx_g2_get_eeprom8(struct mv88e6xxx_chip *chip,
@@ -57,12 +59,14 @@ static inline int mv88e6xxx_g2_require(struct mv88e6xxx_chip *chip)
}
static inline int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
int addr, int reg, u16 *val)
{
return -EOPNOTSUPP;
}
static inline int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
int addr, int reg, u16 val)
{
return -EOPNOTSUPP;
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index ce8b43b14e96..7d75dd546bf7 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -758,10 +758,12 @@ struct mv88e6xxx_ops {
int (*set_switch_mac)(struct mv88e6xxx_chip *chip, u8 *addr);
- int (*phy_read)(struct mv88e6xxx_chip *chip, int addr, int reg,
- u16 *val);
- int (*phy_write)(struct mv88e6xxx_chip *chip, int addr, int reg,
- u16 val);
+ int (*phy_read)(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 *val);
+ int (*phy_write)(struct mv88e6xxx_chip *chip,
+ struct mii_bus *bus,
+ int addr, int reg, u16 val);
/* PHY Polling Unit (PPU) operations */
int (*ppu_enable)(struct mv88e6xxx_chip *chip);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/5] net: dsa: mv88e6xxx: Abstract mv88e6165 PHY operations
From: Andrew Lunn @ 2017-01-24 13:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-1-git-send-email-andrew@lunn.ch>
The mv88e6165 family has the internal PHYs mapped directly onto the
SMI register space as the switch. So the registers can be read
directly. Put a wrapper around this, in preparation for changing the
signature in order to support the external MDIO bus of the 6390.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/chip.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c7e08e13bb54..374d887e8256 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -222,6 +222,18 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int addr, int reg, u16 val)
return 0;
}
+static int mv88e6165_phy_read(struct mv88e6xxx_chip *chip, int addr,
+ int reg, u16 *val)
+{
+ return mv88e6xxx_read(chip, addr, reg, val);
+}
+
+static int mv88e6165_phy_write(struct mv88e6xxx_chip *chip, int addr,
+ int reg, u16 val)
+{
+ return mv88e6xxx_write(chip, addr, reg, val);
+}
+
static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
int reg, u16 *val)
{
@@ -3085,8 +3097,8 @@ static const struct mv88e6xxx_ops mv88e6097_ops = {
static const struct mv88e6xxx_ops mv88e6123_ops = {
/* MV88E6XXX_FAMILY_6165 */
.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
- .phy_read = mv88e6xxx_read,
- .phy_write = mv88e6xxx_write,
+ .phy_read = mv88e6165_phy_read,
+ .phy_write = mv88e6165_phy_write,
.port_set_link = mv88e6xxx_port_set_link,
.port_set_duplex = mv88e6xxx_port_set_duplex,
.port_set_speed = mv88e6185_port_set_speed,
@@ -3132,8 +3144,8 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
static const struct mv88e6xxx_ops mv88e6161_ops = {
/* MV88E6XXX_FAMILY_6165 */
.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
- .phy_read = mv88e6xxx_read,
- .phy_write = mv88e6xxx_write,
+ .phy_read = mv88e6165_phy_read,
+ .phy_write = mv88e6165_phy_write,
.port_set_link = mv88e6xxx_port_set_link,
.port_set_duplex = mv88e6xxx_port_set_duplex,
.port_set_speed = mv88e6185_port_set_speed,
@@ -3157,8 +3169,8 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
static const struct mv88e6xxx_ops mv88e6165_ops = {
/* MV88E6XXX_FAMILY_6165 */
.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
- .phy_read = mv88e6xxx_read,
- .phy_write = mv88e6xxx_write,
+ .phy_read = mv88e6165_phy_read,
+ .phy_write = mv88e6165_phy_write,
.port_set_link = mv88e6xxx_port_set_link,
.port_set_duplex = mv88e6xxx_port_set_duplex,
.port_set_speed = mv88e6185_port_set_speed,
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 0/5] External MDIO support for mv88e6xxx
From: Andrew Lunn @ 2017-01-24 13:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn
The mv88e6390 family of switches has two MDIO busses, one internal to
the switch and a second one for external usage. Older generations of switches
have a single MDIO bus, which is used both internally and externally.
Refactor the existing MDIO driver code to allow for multiple MDIO
busses, and implement the second MDIO bus on mv88e6390.
This is a rewrite of a patch previously submitted as part of "Batch
3". It has been broken up into 5 smaller patches. A compatible string
is now used in the device tree to indicate the external MDIO bus.
Andrew Lunn (5):
net: dsa: mv88e6xxx: Abstract mv88e6165 PHY operations
net: dsa: mv88e6xxx: Pass mii_bus to all PHY operations
net: dsa: mv88e6xxx: Add mdio private structure
net: dsa: mv88e6xxx: Support multiple MDIO busses
net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus
.../devicetree/bindings/net/dsa/marvell.txt | 41 ++++-
drivers/net/dsa/mv88e6xxx/chip.c | 169 ++++++++++++++++-----
drivers/net/dsa/mv88e6xxx/global2.c | 18 ++-
drivers/net/dsa/mv88e6xxx/global2.h | 12 +-
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 25 +--
5 files changed, 206 insertions(+), 59 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 5/5] net: dsa: mv88e6xxx: Implement the 6390 external MDIO bus
From: Andrew Lunn @ 2017-01-24 13:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vivien Didelot, Florian Fainelli, Andrew Lunn
In-Reply-To: <1485266031-4980-1-git-send-email-andrew@lunn.ch>
With all the infrastructure in place, implement access to the external
MDIO bus on the 6390 family.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/global2.c | 8 ++++++++
drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index 1f9a12a1fad9..353e26bea3c3 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -506,8 +506,12 @@ int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
int addr, int reg, u16 *val)
{
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_READ_DATA | (addr << 5) | reg;
+ struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
int err;
+ if (mdio_bus->external)
+ cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL;
+
err = mv88e6xxx_g2_smi_phy_wait(chip);
if (err)
return err;
@@ -524,8 +528,12 @@ int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
int addr, int reg, u16 val)
{
u16 cmd = GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA | (addr << 5) | reg;
+ struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
int err;
+ if (mdio_bus->external)
+ cmd |= GLOBAL2_SMI_PHY_CMD_EXTERNAL;
+
err = mv88e6xxx_g2_smi_phy_wait(chip);
if (err)
return err;
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 7d24add45e74..572d585dc1e2 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -387,6 +387,7 @@
#define GLOBAL2_PTP_AVB_DATA 0x17
#define GLOBAL2_SMI_PHY_CMD 0x18
#define GLOBAL2_SMI_PHY_CMD_BUSY BIT(15)
+#define GLOBAL2_SMI_PHY_CMD_EXTERNAL BIT(13)
#define GLOBAL2_SMI_PHY_CMD_MODE_22 BIT(12)
#define GLOBAL2_SMI_PHY_CMD_OP_22_WRITE_DATA ((0x1 << 10) | \
GLOBAL2_SMI_PHY_CMD_MODE_22 | \
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] net: reset ct before calling ndo_start_xmit
From: Eric Dumazet @ 2017-01-24 14:04 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, Hannes Frederic Sowa, Florian Westphal
In-Reply-To: <e33476125bdaf4619c221aa82c034831bbdce89b.1485250657.git.pabeni@redhat.com>
On Tue, 2017-01-24 at 10:40 +0100, Paolo Abeni wrote:
> Currently we use the NETIF_F_LLTX feature bit to identify such devices,
> since all the [legacy] phys drivers setting such bit are not prone
> the hangup issue. The plan is adding a specific 'this is a
> virtual device' priv flag and use it instead, in a later net-next
> patch.
This is too ugly in my opinion.
LLTX is LLTX, and has absolutely nothing to do with connection tracking.
We have ndo_features_check, and this can be trivially backported to
stable versions.
No need for yet another flag really.
^ permalink raw reply
* Re: TCP stops sending packets over loopback on 4.10-rc3?
From: Eric Dumazet @ 2017-01-24 14:07 UTC (permalink / raw)
To: Josef Bacik; +Cc: Linux Netdev List, Lawrence Brakmo, Kernel Team
In-Reply-To: <1485256853.4014.0@smtp.office365.com>
On Tue, 2017-01-24 at 06:20 -0500, Josef Bacik wrote:
> Hello,
>
> I've been trying to test some NBD changes I had made recently and I
> started having packet timeouts. I traced this down to tcp just
> stopping sending packets after a lot of writing. All NBD does is call
> kernel_sendmsg() with a request struct and some pages when it does
> writes. I did a bunch of tracing and I've narrowed it down to running
> out of sk_wmem_queued space. In tcp_sendmsg() here
>
> new_segment:
> /* Allocate new segment. If the interface is SG,
> * allocate skb fitting to single page.
> */
> if (!sk_stream_memory_free(sk))
> goto wait_for_sndbuf;
>
> we hit this pretty regularly, and eventually just get stuck in
> sk_stream_wait_memory until the timeout ends and we error out
> everything. Now sk_stream_memory_free checks the sk_wmem_queued and
> calls into the sk_prot->stream_memory_free(), so I broke this out like
> the following
>
>
> if (sk->sk_wmem_queued >= sk->sk_sndbuf) {
> trace_printk("sk_wmem_queued %d, sk_sndbuf %d\n",
> sk->sk_wmem_queued, sk->sk_sndbuf);
> goto wait_for_sndbuf;
> }
> if (sk->sk_prot->stream_memory_free &&
> !sk->sk_prot->stream_memory_free(sk)) {
> trace_printk("sk_stream_memory_free\n");
> goto wait_for_sndbuf;
> }
>
> And I got this in my tracing
>
> kworker/u16:5-112 [001] .... 1375.637564: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [001] .... 1375.639657: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [003] .... 1375.641128: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [003] .... 1375.643441: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [001] .... 1375.807614: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [001] .... 1377.538744: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [001] .... 1377.543418: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/2:4H-1535 [002] .... 1377.544685: tcp_sendmsg:
> sk_wmem_queued 4204872, sk_sndbuf 4194304
> kworker/u16:5-112 [000] .... 1379.378352: tcp_sendmsg:
> sk_wmem_queued 4205796, sk_sndbuf 4194304
> kworker/u16:5-112 [003] .... 1380.985721: tcp_sendmsg:
> sk_wmem_queued 4212416, sk_sndbuf 4194304
>
> This is as far as I've gotten and I'll keep digging into it, but I was
> wondering if this looks familiar to anybody? Also one thing I've
> noticed is sk_stream_wait_memory() will wait on sk_sleep(sk), but
> basically nothing wakes this up. For example it seems the main way we
> reduce sk_wmem_queued is through sk_wmem_free_skb(), which doesn't
> appear to wake anything up in any of its callers, so anybody who does
> end up sleeping will basically never wake up. That seems like it
> should be more broken than it is, so I'm curious to know how things are
> actually woken up in this case. Thanks,
git grep -n SOCK_QUEUE_SHRUNK
-> tcp_check_space()
^ permalink raw reply
* [PATCH] bpf: don't kfree an uninitialized im_node
From: Colin King @ 2017-01-24 14:16 UTC (permalink / raw)
To: Alexei Starovoitov, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There are some error exit paths to the label 'out' that end up
kfree'ing an uninitialized im_node. Fix this by inititializing
im_node to NULL to avoid kfree'ing a garbage address.
Issue found by CoverityScan, CID#1398022 ("Uninitialized pointer read")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
kernel/bpf/lpm_trie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index ba19241d..144e976 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -262,7 +262,7 @@ static int trie_update_elem(struct bpf_map *map,
void *_key, void *value, u64 flags)
{
struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
- struct lpm_trie_node *node, *im_node, *new_node = NULL;
+ struct lpm_trie_node *node, *im_node = NULL, *new_node = NULL;
struct lpm_trie_node __rcu **slot;
struct bpf_lpm_trie_key *key = _key;
unsigned long irq_flags;
--
2.10.2
^ permalink raw reply related
* Re: net: use-after-free in tw_timer_handler
From: Eric Dumazet @ 2017-01-24 14:28 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Eric Dumazet, syzkaller
In-Reply-To: <CACT4Y+ba65AD9qpjjJqM=_Pn333iWTm86BNmy2A5_EqHTY_kWA@mail.gmail.com>
On Mon, 2017-01-23 at 11:23 +0100, Dmitry Vyukov wrote:
> On Mon, Jan 23, 2017 at 11:19 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > Hello,
> >
> > While running syzkaller fuzzer I started seeing use-after-frees in
> > tw_timer_handler. It happens with very low frequency, so far I've seen
> > 22 of them. But all reports look consistent, so I would assume that it
> > is real, just requires a very tricky race to happen. I've stared
> > seeing it around Jan 17, however I did not update kernels for some
> > time before that so potentially the issues was introduced somewhat
> > earlier. Or maybe fuzzer just figured how to trigger it, and the bug
> > is actually old. I am seeing it on all of torvalds/linux-next/mmotm,
> > some commits if it matters: 7a308bb3016f57e5be11a677d15b821536419d36,
> > 5cf7a0f3442b2312326c39f571d637669a478235,
> > c497f8d17246720afe680ea1a8fa6e48e75af852.
> > Majority of reports points to net_drop_ns as the offending free, but
> > it may be red herring. Since the access happens in timer, it can
> > happen long after free and the memory could have been reused. I've
> > also seen few where the access in tw_timer_handler is reported as
> > out-of-bounds on task_struct and on struct filename.
>
>
>
> I've briefly skimmed through the code. Assuming that it requires a
> very tricky race to be triggered, the most suspicious looks
> inet_twsk_deschedule_put vs __inet_twsk_schedule:
>
> void inet_twsk_deschedule_put(struct inet_timewait_sock *tw)
> {
> if (del_timer_sync(&tw->tw_timer))
> inet_twsk_kill(tw);
> inet_twsk_put(tw);
> }
>
> void __inet_twsk_schedule(struct inet_timewait_sock *tw, int timeo, bool rearm)
> {
> tw->tw_kill = timeo <= 4*HZ;
> if (!rearm) {
> BUG_ON(mod_timer(&tw->tw_timer, jiffies + timeo));
> atomic_inc(&tw->tw_dr->tw_count);
> } else {
> mod_timer_pending(&tw->tw_timer, jiffies + timeo);
> }
> }
>
> Can't it somehow end up rearming already deleted timer? Or maybe the
> first mod_timer happens after del_timer_sync?
This code was changed a long time ago :
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ed2e923945892a8372ab70d2f61d364b0b6d9054
So I suspect a recent patch broke the logic.
You might start a bisection :
I would check if 4.7 and 4.8 trigger the issue you noticed.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] uapi: fix signess in ethtool_validate_speed()
From: Michael S. Tsirkin @ 2017-01-24 14:51 UTC (permalink / raw)
To: Volodymyr Bendiuga
Cc: David Miller, nikolay, netdev, Volodymyr Bendiuga,
Jonas Johansson
In-Reply-To: <CABHmqqAaram+jGGfAM4qGRSuucxZag-pRoc2aBqgQsnWVzNRUQ@mail.gmail.com>
On Tue, Jan 24, 2017 at 03:45:25PM +0100, Volodymyr Bendiuga wrote:
> On Fri, Jan 20, 2017 at 5:36 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Jan 20, 2017 at 02:20:53PM +0100, Volodymyr Bendiuga wrote:
> > From: Jonas Johansson <jonas.johansson@westermo.se>
> >
> > There is a comparison of speed variable which
> > is unsigned, and SPEED_UNKNOWN which is signed.
>
> So?
>
> >
> > Signed-off-by: Jonas Johansson <jonas.johansson@westermo.se>
> > Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@gmail.com>
> > ---
> > include/uapi/linux/ethtool.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > index f0db778..1ca4a77 100644
> > --- a/include/uapi/linux/ethtool.h
> > +++ b/include/uapi/linux/ethtool.h
> > @@ -1500,7 +1500,7 @@ enum ethtool_link_mode_bit_indices {
> >
> > #define SPEED_UNKNOWN -1
> >
> > -static inline int ethtool_validate_speed(__u32 speed)
> > +static inline int ethtool_validate_speed(__s32 speed)
> > {
> > return speed <= INT_MAX || speed == SPEED_UNKNOWN;
> > }
>
> Then comparison to INT_MAX does not make sense.
>
> Why doesn't it make sense?
>
A signed integer is always <= INT_MAX.
--
MST
^ permalink raw reply
* Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants
From: Michal Hocko @ 2017-01-24 15:00 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Kees Cook, Andrew Morton, David Rientjes, Mel Gorman,
Johannes Weiner, Al Viro, Linux-MM, LKML, Martin Schwidefsky,
Heiko Carstens, Herbert Xu, Anton Vorontsov, Colin Cross,
Tony Luck, Rafael J. Wysocki, Ben Skeggs, Kent Overstreet,
Santosh Raspatur, Hariprasad S, Tariq Toukan
In-Reply-To: <7c109e9e-e28b-3ddb-42b6-902f46bf0572@suse.cz>
On Fri 20-01-17 14:41:37, Vlastimil Babka wrote:
> On 01/12/2017 06:37 PM, Michal Hocko wrote:
> > On Thu 12-01-17 09:26:09, Kees Cook wrote:
> >> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > [...]
> >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> >>> index 4f74511015b8..e6bbb33d2956 100644
> >>> --- a/arch/s390/kvm/kvm-s390.c
> >>> +++ b/arch/s390/kvm/kvm-s390.c
> >>> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
> >>> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> >>> return -EINVAL;
> >>>
> >>> - keys = kmalloc_array(args->count, sizeof(uint8_t),
> >>> - GFP_KERNEL | __GFP_NOWARN);
> >>> - if (!keys)
> >>> - keys = vmalloc(sizeof(uint8_t) * args->count);
> >>> + keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> >>
> >> Before doing this conversion, can we add a kvmalloc_array() API? This
> >> conversion could allow for the reintroduction of integer overflow
> >> flaws. (This particular situation isn't at risk since ->count is
> >> checked, but I'd prefer we not create a risky set of examples for
> >> using kvmalloc.)
> >
> > Well, I am not opposed to kvmalloc_array but I would argue that this
> > conversion cannot introduce new overflow issues. The code would have
> > to be broken already because even though kmalloc_array checks for the
> > overflow but vmalloc fallback doesn't...
>
> Yeah I agree, but if some of the places were really wrong, after the
> conversion we won't see them anymore.
>
> > If there is a general interest for this API I can add it.
>
> I think it would be better, yes.
OK, fair enough. I will fold the following into the original patch. I
was little bit reluctant to create kvcalloc so I've made the original
callers more talkative and added | __GFP_ZERO. To be honest I do not
really like how kcalloc...
---
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index e6bbb33d2956..aa558dce6bb4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,7 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
- keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
+ keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
@@ -1168,7 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
- keys = kvmalloc(sizeof(uint8_t) * args->count, GFP_KERNEL);
+ keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 82354fd0a87e..6583d4601480 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -115,7 +115,7 @@ static int mlx4_buddy_init(struct mlx4_buddy *buddy, int max_order)
for (i = 0; i <= buddy->max_order; ++i) {
s = BITS_TO_LONGS(1 << (buddy->max_order - i));
- buddy->bits[i] = kvzalloc(s * sizeof(long), GFP_KERNEL);
+ buddy->bits[i] = kvmalloc_array(s, sizeof(long), GFP_KERNEL | __GFP_ZERO);
if (!buddy->bits[i])
goto err_out_free;
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 55fd570c3e1e..22c6e81d0c16 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -498,6 +498,14 @@ static inline void *kvzalloc(size_t size, gfp_t flags)
return kvmalloc(size, flags | __GFP_ZERO);
}
+static inline void *kvmalloc_array(size_t n, size_t size, gfp_t flags)
+{
+ if (size != 0 && n > SIZE_MAX / size)
+ return NULL;
+
+ return kvmalloc(n * size, flags);
+}
+
extern void kvfree(const void *addr);
static inline atomic_t *compound_mapcount_ptr(struct page *page)
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 4ca30a951bbc..58ec07946fe6 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -320,7 +320,7 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
goto free_htab;
err = -ENOMEM;
- htab->buckets = kvmalloc(htab->n_buckets * sizeof(struct bucket), GFP_USER);
+ htab->buckets = kvmalloc_array(htab->n_buckets, sizeof(struct bucket), GFP_USER);
if (!htab->buckets)
goto free_htab;
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 45c17b5562b5..8f9caf095172 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -957,7 +957,7 @@ EXPORT_SYMBOL(iov_iter_get_pages);
static struct page **get_pages_array(size_t n)
{
- return kvmalloc(n * sizeof(struct page *), GFP_KERNEL);
+ return kvmalloc_array(n, sizeof(struct page *), GFP_KERNEL);
}
static ssize_t pipe_get_pages_alloc(struct iov_iter *i,
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index a46a9fd8b540..0c4848bd86c4 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -687,7 +687,7 @@ int inet_ehash_locks_alloc(struct inet_hashinfo *hashinfo)
/* no more locks than number of hash buckets */
nblocks = min(nblocks, hashinfo->ehash_mask + 1);
- hashinfo->ehash_locks = kvmalloc(nblocks * locksz, GFP_KERNEL);
+ hashinfo->ehash_locks = kvmalloc_array(nblocks, locksz, GFP_KERNEL);
if (!hashinfo->ehash_locks)
return -ENOMEM;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index cdc55d5ee4ad..eca16612b1ae 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -712,10 +712,7 @@ EXPORT_SYMBOL(xt_check_entry_offsets);
*/
unsigned int *xt_alloc_entry_offsets(unsigned int size)
{
- if (size < (SIZE_MAX / sizeof(unsigned int)))
- return kvzalloc(size * sizeof(unsigned int), GFP_KERNEL);
-
- return NULL;
+ return kvmalloc_array(size * sizeof(unsigned int), GFP_KERNEL | __GFP_ZERO);
}
EXPORT_SYMBOL(xt_alloc_entry_offsets);
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index 30d6a39fd2c8..47cbfae44898 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -431,7 +431,7 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt)
if (mask != q->tab_mask) {
struct sk_buff **ntab;
- ntab = kvzalloc((mask + 1) * sizeof(struct sk_buff *), GFP_KERNEL);
+ ntab = kvmalloc_array((mask + 1), sizeof(struct sk_buff *), GFP_KERNEL | __GFP_ZERO);
if (!ntab)
return -ENOMEM;
--
Michal Hocko
SUSE Labs
^ permalink raw reply related
* Re: [PATCH net] net: reset ct before calling ndo_start_xmit
From: Paolo Abeni @ 2017-01-24 15:04 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, Hannes Frederic Sowa, Florian Westphal
In-Reply-To: <1485266667.16328.290.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2017-01-24 at 06:04 -0800, Eric Dumazet wrote:
> On Tue, 2017-01-24 at 10:40 +0100, Paolo Abeni wrote:
>
> > Currently we use the NETIF_F_LLTX feature bit to identify such
> > devices,
> > since all the [legacy] phys drivers setting such bit are not prone
> > the hangup issue. The plan is adding a specific 'this is a
> > virtual device' priv flag and use it instead, in a later net-next
> > patch.
>
> This is too ugly in my opinion.
>
> LLTX is LLTX, and has absolutely nothing to do with connection
> tracking.
>
> We have ndo_features_check, and this can be trivially backported to
> stable versions.
>
> No need for yet another flag really.
Thank you for the feedback.
Double checking to see if I understood the above correctly: do you
suggest to call nf_reset() from the affected drivers's
ndo_features_check(), eventually adding such ndo if needed ?
I think calling nf_reset() in the common code should be better: the
conntrack entry is hot in the cache and we may want to clear it early
for as many devices as possible.
Thank you,
Paolo
^ permalink raw reply
* Re: net: use-after-free in tw_timer_handler
From: Dmitry Vyukov @ 2017-01-24 15:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, LKML, Eric Dumazet, syzkaller
In-Reply-To: <1485268138.16328.297.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Jan 24, 2017 at 3:28 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2017-01-23 at 11:23 +0100, Dmitry Vyukov wrote:
>> On Mon, Jan 23, 2017 at 11:19 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> > Hello,
>> >
>> > While running syzkaller fuzzer I started seeing use-after-frees in
>> > tw_timer_handler. It happens with very low frequency, so far I've seen
>> > 22 of them. But all reports look consistent, so I would assume that it
>> > is real, just requires a very tricky race to happen. I've stared
>> > seeing it around Jan 17, however I did not update kernels for some
>> > time before that so potentially the issues was introduced somewhat
>> > earlier. Or maybe fuzzer just figured how to trigger it, and the bug
>> > is actually old. I am seeing it on all of torvalds/linux-next/mmotm,
>> > some commits if it matters: 7a308bb3016f57e5be11a677d15b821536419d36,
>> > 5cf7a0f3442b2312326c39f571d637669a478235,
>> > c497f8d17246720afe680ea1a8fa6e48e75af852.
>> > Majority of reports points to net_drop_ns as the offending free, but
>> > it may be red herring. Since the access happens in timer, it can
>> > happen long after free and the memory could have been reused. I've
>> > also seen few where the access in tw_timer_handler is reported as
>> > out-of-bounds on task_struct and on struct filename.
>>
>>
>>
>> I've briefly skimmed through the code. Assuming that it requires a
>> very tricky race to be triggered, the most suspicious looks
>> inet_twsk_deschedule_put vs __inet_twsk_schedule:
>>
>> void inet_twsk_deschedule_put(struct inet_timewait_sock *tw)
>> {
>> if (del_timer_sync(&tw->tw_timer))
>> inet_twsk_kill(tw);
>> inet_twsk_put(tw);
>> }
>>
>> void __inet_twsk_schedule(struct inet_timewait_sock *tw, int timeo, bool rearm)
>> {
>> tw->tw_kill = timeo <= 4*HZ;
>> if (!rearm) {
>> BUG_ON(mod_timer(&tw->tw_timer, jiffies + timeo));
>> atomic_inc(&tw->tw_dr->tw_count);
>> } else {
>> mod_timer_pending(&tw->tw_timer, jiffies + timeo);
>> }
>> }
>>
>> Can't it somehow end up rearming already deleted timer? Or maybe the
>> first mod_timer happens after del_timer_sync?
>
> This code was changed a long time ago :
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=ed2e923945892a8372ab70d2f61d364b0b6d9054
>
> So I suspect a recent patch broke the logic.
>
> You might start a bisection :
>
> I would check if 4.7 and 4.8 trigger the issue you noticed.
It happens with too low rate for bisecting (few times per day). I
could add some additional checks into code, but I don't know what
checks could be useful.
^ permalink raw reply
* Re: linux-next: manual merge of the kselftest tree with the net-next tree
From: Shuah Khan @ 2017-01-24 15:17 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, Networking, Bamvor Jian Zhang,
bamvor.zhangjian@huawei.com
Cc: linux-next, linux-kernel, David Herrmann, Daniel Mack
In-Reply-To: <20170124154524.2a993739@canb.auug.org.au>
On 01/23/2017 09:45 PM, Stephen Rothwell wrote:
> Hi Shuah,
>
> Today's linux-next merge of the kselftest tree got a conflict in:
>
> tools/testing/selftests/bpf/Makefile
>
> between commit:
>
> 4d3381f5a322 ("bpf: Add tests for the lpm trie map")
>
> from the net-next tree and commit:
>
> 88baa78d1f31 ("selftests: remove duplicated all and clean target")
>
> from the kselftest tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
Yes. Please.
>
> P.S. Shuah, that kselftest commit has a different email address in its
> Author and Signed-off-by.
>
Yes you are right. I didn't notice the discrepancy.
Bamvor! Your gitconfig is outdated perhaps!
thanks,
-- Shuah
^ permalink raw reply
* [PATCH v2 00/18] gtp: fixes and support multiple VRF's per GTP socket
From: Andreas Schultz @ 2017-01-24 15:28 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
The current linking of GTP network devices and GTP enabled sockets means that
we can not have multiple VRF's per GTP socket. This series seperates the
sockets from network device, makes sockets attached to GTP network device
optional and adds a API function to enable GTP encapsulation on socket
without having to create a new GTP device.
It is still possible to use the old API. The network device attached socket is
then used when no socket is specified on PDP context creation.
During that work some smaller problems where found and fixes for them are
included.
v2 changes:
* the socket that is hold by the pdp context has to be release in a rcu
callback. Otherwise a stray GTP rx could end uo with an invalid socket.
* accessing the skb->sk field in gtp_rx is invalid, that field has no
been populated at that point
* add dst_cache to speed up the routing
Regards
Andreas
^ permalink raw reply
* [PATCH v2 04/18] gtp: return error ptr in find pdp helpers
From: Andreas Schultz @ 2017-01-24 15:28 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124152848.6120-1-aschultz@tpip.net>
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 60946b7..e95c856 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -114,7 +114,7 @@ static struct pdp_ctx *gtp0_pdp_find(struct gtp_dev *gtp, u64 tid)
pdp->u.v0.tid == tid)
return pdp;
}
- return NULL;
+ return ERR_PTR(-ENOENT);
}
/* Resolve a PDP context structure based on the 32bit TEI. */
@@ -130,7 +130,7 @@ static struct pdp_ctx *gtp1_pdp_find(struct gtp_dev *gtp, u32 tid)
pdp->u.v1.i_tei == tid)
return pdp;
}
- return NULL;
+ return ERR_PTR(-ENOENT);
}
/* Resolve a PDP context based on IPv4 address of MS. */
@@ -147,7 +147,7 @@ static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr)
return pdp;
}
- return NULL;
+ return ERR_PTR(-ENOENT);
}
static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx,
@@ -199,7 +199,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
rcu_read_lock();
pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
- if (!pctx) {
+ if (IS_ERR(pctx)) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
ret = -1;
goto out_rcu;
@@ -256,7 +256,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
rcu_read_lock();
pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
- if (!pctx) {
+ if (IS_ERR(pctx)) {
netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
ret = -1;
goto out_rcu;
@@ -476,10 +476,10 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev,
*/
iph = ip_hdr(skb);
pctx = ipv4_pdp_find(gtp, iph->daddr);
- if (!pctx) {
+ if (IS_ERR(pctx)) {
netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n",
&iph->daddr);
- return -ENOENT;
+ return PTR_ERR(pctx);
}
netdev_dbg(dev, "found PDP context %p\n", pctx);
@@ -1085,8 +1085,8 @@ static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}
- if (pctx == NULL)
- return -ENOENT;
+ if (IS_ERR(pctx))
+ return PTR_ERR(pctx);
if (pctx->gtp_version == GTP_V0)
netdev_dbg(dev, "GTPv0-U: deleting tunnel id = %llx (pdp %p)\n",
@@ -1194,8 +1194,8 @@ static int gtp_genl_get_pdp(struct sk_buff *skb, struct genl_info *info)
pctx = ipv4_pdp_find(gtp, ip);
}
- if (pctx == NULL) {
- err = -ENOENT;
+ if (IS_ERR(pctx)) {
+ err = PTR_ERR(pctx);
goto err_unlock;
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 02/18] gtp: clear DF bit on GTP packet tx
From: Andreas Schultz @ 2017-01-24 15:28 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124152848.6120-1-aschultz@tpip.net>
3GPP TS 29.281 and 3GPP TS 29.060 imply that GTP-U packets should be
sent with the DF bit cleared. For example 3GPP TS 29.060, Release 8,
Section 13.2.2:
> Backbone router: Any router in the backbone may fragment the GTP
> packet if needed, according to IPv4.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 7580ccc..1df54d6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -612,7 +612,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
pktinfo.fl4.saddr, pktinfo.fl4.daddr,
pktinfo.iph->tos,
ip4_dst_hoplimit(&pktinfo.rt->dst),
- htons(IP_DF),
+ 0,
pktinfo.gtph_port, pktinfo.gtph_port,
true, false);
break;
--
2.10.2
^ permalink raw reply related
* [PATCH v2 03/18] gtp: make GTP sockets in gtp_newlink optional
From: Andreas Schultz @ 2017-01-24 15:28 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124152848.6120-1-aschultz@tpip.net>
A fixed binding of gtp enabled socket to netdevice does not make
sense (GTP TEID are unique per socket and not per L3 IP device).
To untangle netdevice and gtp sockets without breaking the UAPI,
make per netdevice sockets optional.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 158 ++++++++++++++++++++++++++++--------------------------
1 file changed, 83 insertions(+), 75 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 1df54d6..60946b7 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -276,30 +276,6 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
return ret;
}
-static void gtp_encap_disable(struct gtp_dev *gtp)
-{
- if (gtp->sock0 && gtp->sock0->sk) {
- udp_sk(gtp->sock0->sk)->encap_type = 0;
- rcu_assign_sk_user_data(gtp->sock0->sk, NULL);
- }
- if (gtp->sock1u && gtp->sock1u->sk) {
- udp_sk(gtp->sock1u->sk)->encap_type = 0;
- rcu_assign_sk_user_data(gtp->sock1u->sk, NULL);
- }
-
- gtp->sock0 = NULL;
- gtp->sock1u = NULL;
-}
-
-static void gtp_encap_destroy(struct sock *sk)
-{
- struct gtp_dev *gtp;
-
- gtp = rcu_dereference_sk_user_data(sk);
- if (gtp)
- gtp_encap_disable(gtp);
-}
-
/* UDP encapsulation receive handler. See net/ipv4/udp.c.
* Return codes: 0: success, <0: error, >0: pass up to userspace UDP socket.
*/
@@ -363,6 +339,17 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
return 0;
}
+static void gtp_encap_destroy(struct sock *sk)
+{
+ struct gtp_dev *gtp;
+
+ gtp = rcu_dereference_sk_user_data(sk);
+ if (gtp) {
+ udp_sk(sk)->encap_type = 0;
+ rcu_assign_sk_user_data(sk, NULL);
+ }
+}
+
static int gtp_dev_init(struct net_device *dev)
{
struct gtp_dev *gtp = netdev_priv(dev);
@@ -378,9 +365,6 @@ static int gtp_dev_init(struct net_device *dev)
static void gtp_dev_uninit(struct net_device *dev)
{
- struct gtp_dev *gtp = netdev_priv(dev);
-
- gtp_encap_disable(gtp);
free_percpu(dev->tstats);
}
@@ -658,35 +642,32 @@ static void gtp_link_setup(struct net_device *dev)
static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
static void gtp_hashtable_free(struct gtp_dev *gtp);
static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
- int fd_gtp0, int fd_gtp1, struct net *src_net);
+ struct net *src_net, struct nlattr *data[]);
+static void gtp_encap_disable(struct gtp_dev *gtp);
static int gtp_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[])
{
- int hashsize, err, fd0, fd1;
+ int hashsize, err;
struct gtp_dev *gtp;
struct gtp_net *gn;
- if (!data[IFLA_GTP_FD0] || !data[IFLA_GTP_FD1])
- return -EINVAL;
-
gtp = netdev_priv(dev);
- fd0 = nla_get_u32(data[IFLA_GTP_FD0]);
- fd1 = nla_get_u32(data[IFLA_GTP_FD1]);
-
- err = gtp_encap_enable(dev, gtp, fd0, fd1, src_net);
- if (err < 0)
- goto out_err;
-
if (!data[IFLA_GTP_PDP_HASHSIZE])
hashsize = 1024;
else
hashsize = nla_get_u32(data[IFLA_GTP_PDP_HASHSIZE]);
+ if (data[IFLA_GTP_FD0] || data[IFLA_GTP_FD1]) {
+ err = gtp_encap_enable(dev, gtp, src_net, data);
+ if (err < 0)
+ goto out_err;
+ }
+
err = gtp_hashtable_new(gtp, hashsize);
if (err < 0)
- goto out_encap;
+ goto out_socket;
err = register_netdevice(dev);
if (err < 0) {
@@ -703,7 +684,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
out_hashtable:
gtp_hashtable_free(gtp);
-out_encap:
+out_socket:
gtp_encap_disable(gtp);
out_err:
return err;
@@ -713,8 +694,11 @@ static void gtp_dellink(struct net_device *dev, struct list_head *head)
{
struct gtp_dev *gtp = netdev_priv(dev);
- gtp_encap_disable(gtp);
gtp_hashtable_free(gtp);
+ if (gtp->sock0)
+ sockfd_put(gtp->sock0);
+ if (gtp->sock1u)
+ sockfd_put(gtp->sock1u);
list_del_rcu(>p->list);
unregister_netdevice_queue(dev, head);
}
@@ -820,38 +804,63 @@ static void gtp_hashtable_free(struct gtp_dev *gtp)
kfree(gtp->tid_hash);
}
-static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
- int fd_gtp0, int fd_gtp1, struct net *src_net)
+static struct socket *gtp_encap_enable_socket(int fd, int type,
+ struct gtp_dev *gtp)
{
struct udp_tunnel_sock_cfg tuncfg = {NULL};
- struct socket *sock0, *sock1u;
+ struct socket *sock;
int err;
- netdev_dbg(dev, "enable gtp on %d, %d\n", fd_gtp0, fd_gtp1);
+ pr_debug("enable gtp on %d, %d\n", fd, type);
- sock0 = sockfd_lookup(fd_gtp0, &err);
- if (sock0 == NULL) {
- netdev_dbg(dev, "socket fd=%d not found (gtp0)\n", fd_gtp0);
- return -ENOENT;
+ sock = sockfd_lookup(fd, &err);
+ if (!sock) {
+ pr_debug("gtp socket fd=%d not found\n", fd);
+ return NULL;
}
- if (sock0->sk->sk_protocol != IPPROTO_UDP) {
- netdev_dbg(dev, "socket fd=%d not UDP\n", fd_gtp0);
+ if (sock->sk->sk_protocol != IPPROTO_UDP) {
+ pr_debug("socket fd=%d not UDP\n", fd);
err = -EINVAL;
- goto err1;
+ goto out_sock;
}
- sock1u = sockfd_lookup(fd_gtp1, &err);
- if (sock1u == NULL) {
- netdev_dbg(dev, "socket fd=%d not found (gtp1u)\n", fd_gtp1);
- err = -ENOENT;
- goto err1;
+ tuncfg.sk_user_data = gtp;
+ tuncfg.encap_type = type;
+ tuncfg.encap_rcv = gtp_encap_recv;
+ tuncfg.encap_destroy = gtp_encap_destroy;
+
+ setup_udp_tunnel_sock(sock_net(sock->sk), sock, &tuncfg);
+ return sock;
+
+out_sock:
+ sockfd_put(sock);
+ return ERR_PTR(err);
+}
+
+static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
+ struct net *src_net, struct nlattr *data[])
+{
+ struct socket *sock0 = NULL;
+ struct socket *sock1u = NULL;
+
+ if (data[IFLA_GTP_FD0]) {
+ u32 fd0 = nla_get_u32(data[IFLA_GTP_FD0]);
+
+ sock0 = gtp_encap_enable_socket(fd0, UDP_ENCAP_GTP0, gtp);
+ if (IS_ERR(sock0))
+ return PTR_ERR(sock0);
}
- if (sock1u->sk->sk_protocol != IPPROTO_UDP) {
- netdev_dbg(dev, "socket fd=%d not UDP\n", fd_gtp1);
- err = -EINVAL;
- goto err2;
+ if (data[IFLA_GTP_FD1]) {
+ u32 fd1 = nla_get_u32(data[IFLA_GTP_FD1]);
+
+ sock1u = gtp_encap_enable_socket(fd1, UDP_ENCAP_GTP1U, gtp);
+ if (IS_ERR(sock1u)) {
+ if (sock0)
+ sockfd_put(sock0);
+ return PTR_ERR(sock1u);
+ }
}
netdev_dbg(dev, "enable gtp on %p, %p\n", sock0, sock1u);
@@ -860,22 +869,21 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
gtp->sock1u = sock1u;
gtp->net = src_net;
- tuncfg.sk_user_data = gtp;
- tuncfg.encap_rcv = gtp_encap_recv;
- tuncfg.encap_destroy = gtp_encap_destroy;
+ return 0;
+}
- tuncfg.encap_type = UDP_ENCAP_GTP0;
- setup_udp_tunnel_sock(sock_net(gtp->sock0->sk), gtp->sock0, &tuncfg);
+static void gtp_encap_disable_sock(struct socket *sock)
+{
+ if (!sock || !sock->sk)
+ return;
- tuncfg.encap_type = UDP_ENCAP_GTP1U;
- setup_udp_tunnel_sock(sock_net(gtp->sock1u->sk), gtp->sock1u, &tuncfg);
+ gtp_encap_destroy(sock->sk);
+}
- err = 0;
-err2:
- sockfd_put(sock1u);
-err1:
- sockfd_put(sock0);
- return err;
+static void gtp_encap_disable(struct gtp_dev *gtp)
+{
+ gtp_encap_disable_sock(gtp->sock0);
+ gtp_encap_disable_sock(gtp->sock1u);
}
static struct net_device *gtp_find_dev(struct net *net, int ifindex)
--
2.10.2
^ permalink raw reply related
* [PATCH v2 01/18] gtp: add genl family modules alias
From: Andreas Schultz @ 2017-01-24 15:28 UTC (permalink / raw)
To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170124152848.6120-1-aschultz@tpip.net>
Auto-load the module when userspace asks for the gtp netlink
family.
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
drivers/net/gtp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8b6810b..7580ccc 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1376,3 +1376,4 @@ MODULE_LICENSE("GPL");
MODULE_AUTHOR("Harald Welte <hwelte@sysmocom.de>");
MODULE_DESCRIPTION("Interface driver for GTP encapsulated traffic");
MODULE_ALIAS_RTNL_LINK("gtp");
+MODULE_ALIAS_GENL_FAMILY("gtp");
--
2.10.2
^ permalink raw reply related
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