Netdev List
 help / color / mirror / Atom feed
* [RFC PATCH 06/11] ARM: dts: dra7: switch to use phy-gmii-sel
From: Grygorii Strashko @ 2018-10-08 23:49 UTC (permalink / raw)
  To: David S. Miller, netdev, Tony Lindgren, Rob Herring,
	Kishon Vijay Abraham I
  Cc: Sekhar Nori, linux-kernel, linux-omap, devicetree,
	Grygorii Strashko
In-Reply-To: <20181008234949.15416-1-grygorii.strashko@ti.com>

Switch to use phy-gmii-sel PHY instead of cpsw-phy-sel.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/dra7.dtsi | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index a0ddf49..07a0edf 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -187,6 +187,12 @@
 						};
 					};
 
+					phy_gmii_sel: phy-gmii-sel {
+						compatible = "ti,dra7xx-phy-gmii-sel";
+						syscon-scm = <&scm_conf>;
+						#phy-cells = <1>;
+					};
+
 					scm_conf_clocks: clocks {
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -1879,17 +1885,13 @@
 			cpsw_emac0: slave@48480200 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
+				phys = <&phy_gmii_sel 1>;
 			};
 
 			cpsw_emac1: slave@48480300 {
 				/* Filled in by U-Boot */
 				mac-address = [ 00 00 00 00 00 00 ];
-			};
-
-			phy_sel: cpsw-phy-sel@4a002554 {
-				compatible = "ti,dra7xx-cpsw-phy-sel";
-				reg= <0x4a002554 0x4>;
-				reg-names = "gmii-sel";
+				phys = <&phy_gmii_sel 2>;
 			};
 		};
 
-- 
2.10.5

^ permalink raw reply related

* [RFC PATCH 05/11] net: ethernet: ti: cpsw: add support for port interface mode selection phy
From: Grygorii Strashko @ 2018-10-08 23:49 UTC (permalink / raw)
  To: David S. Miller, netdev, Tony Lindgren, Rob Herring,
	Kishon Vijay Abraham I
  Cc: Sekhar Nori, linux-kernel, linux-omap, devicetree,
	Grygorii Strashko
In-Reply-To: <20181008234949.15416-1-grygorii.strashko@ti.com>

Add support for port interface mode selection phy (phy-gmii-sel):
- try to request interface mode selection phy from Port DT node and fail
silently if not defined and old CONFIG_TI_CPSW_PHY_SEL driver enabled.
- use new phy if requested successfully.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 832bce0..4607de2 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -26,6 +26,7 @@
 #include <linux/netdevice.h>
 #include <linux/net_tstamp.h>
 #include <linux/phy.h>
+#include <linux/phy/phy.h>
 #include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/pm_runtime.h>
@@ -387,6 +388,7 @@ struct cpsw_slave_data {
 	int		phy_if;
 	u8		mac_addr[ETH_ALEN];
 	u16		dual_emac_res_vlan;	/* Reserved VLAN for DualEMAC */
+	struct phy	*ifphy;
 };
 
 struct cpsw_platform_data {
@@ -1502,7 +1504,11 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 	phy_start(slave->phy);
 
 	/* Configure GMII_SEL register */
-	cpsw_phy_sel(cpsw->dev, slave->phy->interface, slave->slave_num);
+	if (!IS_ERR(slave->data->ifphy))
+		phy_set_netif_mode(slave->data->ifphy, slave->data->phy_if);
+	else
+		cpsw_phy_sel(cpsw->dev, slave->phy->interface,
+			     slave->slave_num);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
@@ -3135,6 +3141,16 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 		if (strcmp(slave_node->name, "slave"))
 			continue;
 
+		slave_data->ifphy = devm_of_phy_get(&pdev->dev, slave_node,
+						    NULL);
+		if (!IS_ENABLED(CONFIG_TI_CPSW_PHY_SEL) &&
+		    IS_ERR(slave_data->ifphy)) {
+			ret = PTR_ERR(slave_data->ifphy);
+			dev_err(&pdev->dev,
+				"%d: Error retrieving port phy: %d\n", i, ret);
+			return ret;
+		}
+
 		slave_data->phy_node = of_parse_phandle(slave_node,
 							"phy-handle", 0);
 		parp = of_get_property(slave_node, "phy_id", &lenp);
-- 
2.10.5

^ permalink raw reply related

* [RFC PATCH 04/11] dt-bindings: net: ti: cpsw: switch to use phy-gmii-sel phy
From: Grygorii Strashko @ 2018-10-08 23:49 UTC (permalink / raw)
  To: David S. Miller, netdev, Tony Lindgren, Rob Herring,
	Kishon Vijay Abraham I
  Cc: Sekhar Nori, linux-kernel, linux-omap, devicetree,
	Grygorii Strashko
In-Reply-To: <20181008234949.15416-1-grygorii.strashko@ti.com>

The cpsw-phy-sel driver was replaced with new PHY driver phy-gmii-sel, so
deprecate cpsw-phy-sel bindings and update CPSW binding to use phy-gmii-sel
PHY bindings.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index b3acebe..69d7ef8 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -22,7 +22,8 @@ Required properties:
 - cpsw-phy-sel		: Specifies the phandle to the CPSW phy mode selection
 			  device. See also cpsw-phy-sel.txt for it's binding.
 			  Note that in legacy cases cpsw-phy-sel may be
-			  a child device instead of a phandle.
+			  a child device instead of a phandle
+			  (DEPRECATED, use phy-gmii-sel PHY phandle).
 
 Optional properties:
 - ti,hwmods		: Must be "cpgmac0"
@@ -44,6 +45,7 @@ Optional properties:
 Slave Properties:
 Required properties:
 - phy-mode		: See ethernet.txt file in the same directory
+- phys			: phandle on phy-gmii-sel PHY (see phy/ti-phy-gmii-sel.txt)
 
 Optional properties:
 - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
@@ -85,12 +87,14 @@ Examples:
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 1 0>;
 		};
 		cpsw_emac1: slave@1 {
 			phy_id = <&davinci_mdio>, <1>;
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 2 0>;
 		};
 	};
 
@@ -114,11 +118,13 @@ Examples:
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 1 0>;
 		};
 		cpsw_emac1: slave@1 {
 			phy_id = <&davinci_mdio>, <1>;
 			phy-mode = "rgmii-txid";
 			/* Filled in by U-Boot */
 			mac-address = [ 00 00 00 00 00 00 ];
+			phys = <&phy_gmii_sel 2 0>;
 		};
 	};
-- 
2.10.5

^ permalink raw reply related

* [RFC PATCH 03/11] phy: ti: introduce phy-gmii-sel driver
From: Grygorii Strashko @ 2018-10-08 23:49 UTC (permalink / raw)
  To: David S. Miller, netdev, Tony Lindgren, Rob Herring,
	Kishon Vijay Abraham I
  Cc: Sekhar Nori, linux-kernel, linux-omap, devicetree,
	Grygorii Strashko
In-Reply-To: <20181008234949.15416-1-grygorii.strashko@ti.com>

TI am335x/am437x/dra7(am5)/dm814x CPSW3G Ethernet Subsystem supports two
10/100/1000 Ethernet ports with selectable G/MII, RMII, and RGMII
interfaces. The interface mode is selected by configuring the MII mode
selection register(s) (GMII_SEL) in the System Control Module chapter
(SCM). GMII_SEL register(s) and bit fields placement in SCM are different
between SoCs while fields meaning is the same.

Historically CPSW external Port's interface mode selection configuartion
was introduced using custom API and driver cpsw-phy-sel.c. This leads to
unnecessary driver, DT binding and custom API support effort.

This patch introduces CPSW Port's PHY Interface Mode selection Driver
(phy-gmii-sel) which implements standard Linux PHY interface and proposed
as a replacement for TI's specific driver cpsw-phy-sel.c and coresponding
custom API.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/phy/ti/Kconfig        |  10 ++
 drivers/phy/ti/Makefile       |   1 +
 drivers/phy/ti/phy-gmii-sel.c | 345 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 356 insertions(+)
 create mode 100644 drivers/phy/ti/phy-gmii-sel.c

diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
index 2050356..f137e01 100644
--- a/drivers/phy/ti/Kconfig
+++ b/drivers/phy/ti/Kconfig
@@ -76,3 +76,13 @@ config TWL4030_USB
 	  family chips (including the TWL5030 and TPS659x0 devices).
 	  This transceiver supports high and full speed devices plus,
 	  in host mode, low speed.
+
+config PHY_TI_GMII_SEL
+	tristate
+	default y if TI_CPSW=y
+	depends on TI_CPSW || COMPILE_TEST
+	select GENERIC_PHY
+	default m
+	help
+	  This driver supports configuring of the TI CPSW Port mode depending on
+	  the Ethernet PHY connected to the CPSW Port.
diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
index 9f36175..bea8f25 100644
--- a/drivers/phy/ti/Makefile
+++ b/drivers/phy/ti/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_OMAP_USB2)			+= phy-omap-usb2.o
 obj-$(CONFIG_TI_PIPE3)			+= phy-ti-pipe3.o
 obj-$(CONFIG_PHY_TUSB1210)		+= phy-tusb1210.o
 obj-$(CONFIG_TWL4030_USB)		+= phy-twl4030-usb.o
+obj-$(CONFIG_PHY_TI_GMII_SEL)		+= phy-gmii-sel.o
diff --git a/drivers/phy/ti/phy-gmii-sel.c b/drivers/phy/ti/phy-gmii-sel.c
new file mode 100644
index 0000000..61a3aa1
--- /dev/null
+++ b/drivers/phy/ti/phy-gmii-sel.c
@@ -0,0 +1,345 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Texas Instruments CPSW Port's PHY Interface Mode selection Driver
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * Based on cpsw-phy-sel.c driver created by Mugunthan V N <mugunthanvnm@ti.com>
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+
+/* AM33xx SoC specific definitions for the CONTROL port */
+#define AM33XX_GMII_SEL_MODE_MII	0
+#define AM33XX_GMII_SEL_MODE_RMII	1
+#define AM33XX_GMII_SEL_MODE_RGMII	2
+
+enum {
+	PHY_GMII_SEL_PORT_MODE,
+	PHY_GMII_SEL_RGMII_ID_MODE,
+	PHY_GMII_SEL_RMII_IO_CLK_EN,
+	PHY_GMII_SEL_LAST,
+};
+
+struct phy_gmii_sel_phy_priv {
+	struct phy_gmii_sel_priv *priv;
+	u32		id;
+	struct phy	*if_phy;
+	int		rmii_clock_external;
+	int		phy_if_mode;
+	struct regmap_field *fields[PHY_GMII_SEL_LAST];
+};
+
+struct phy_gmii_sel_soc_data {
+	u32 num_ports;
+	u32 features;
+	const struct reg_field (*regfields)[PHY_GMII_SEL_LAST];
+};
+
+struct phy_gmii_sel_priv {
+	struct device *dev;
+	const struct phy_gmii_sel_soc_data *soc_data;
+	struct regmap *regmap;
+	struct phy_provider *phy_provider;
+
+	struct phy_gmii_sel_phy_priv *if_phys;
+};
+
+static int phy_gmii_sel_mode(struct phy *phy, phy_interface_t intf_mode)
+{
+	struct phy_gmii_sel_phy_priv *if_phy = phy_get_drvdata(phy);
+	const struct phy_gmii_sel_soc_data *soc_data = if_phy->priv->soc_data;
+	struct device *dev = if_phy->priv->dev;
+	struct regmap_field *regfield;
+	int ret, rgmii_id = 0;
+	u32 mode = 0;
+
+	if_phy->phy_if_mode = intf_mode;
+
+	switch (if_phy->phy_if_mode) {
+	case PHY_INTERFACE_MODE_RMII:
+		mode = AM33XX_GMII_SEL_MODE_RMII;
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII:
+		mode = AM33XX_GMII_SEL_MODE_RGMII;
+		break;
+
+	case PHY_INTERFACE_MODE_RGMII_ID:
+	case PHY_INTERFACE_MODE_RGMII_RXID:
+	case PHY_INTERFACE_MODE_RGMII_TXID:
+		mode = AM33XX_GMII_SEL_MODE_RGMII;
+		rgmii_id = 1;
+		break;
+
+	default:
+		dev_warn(dev,
+			 "port%u: unsupported mode: \"%s\". Defaulting to MII.\n",
+			 if_phy->id, phy_modes(rgmii_id));
+		/* fall through */
+	case PHY_INTERFACE_MODE_MII:
+		mode = AM33XX_GMII_SEL_MODE_MII;
+		break;
+	};
+
+	dev_dbg(dev, "%s id:%u mode:%u rgmii_id:%d rmii_clk_ext:%d\n",
+		__func__, if_phy->id, mode, rgmii_id,
+		if_phy->rmii_clock_external);
+
+	regfield = if_phy->fields[PHY_GMII_SEL_PORT_MODE];
+	ret = regmap_field_write(regfield, mode);
+
+	if (soc_data->features & BIT(PHY_GMII_SEL_RGMII_ID_MODE) &&
+	    if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE]) {
+		regfield = if_phy->fields[PHY_GMII_SEL_RGMII_ID_MODE];
+		ret |= regmap_field_write(regfield, rgmii_id);
+	}
+
+	if (soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN) &&
+	    if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN]) {
+		regfield = if_phy->fields[PHY_GMII_SEL_RMII_IO_CLK_EN];
+		ret |= regmap_field_write(regfield,
+					  if_phy->rmii_clock_external);
+	}
+
+	if (ret) {
+		dev_err(dev, "port%u: set mode fail %d", if_phy->id, ret);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static const
+struct reg_field phy_gmii_sel_fields_am33xx[][PHY_GMII_SEL_LAST] = {
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x650, 0, 1),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x650, 4, 4),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD(0x650, 6, 6),
+	},
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x650, 2, 3),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD(0x650, 5, 5),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD(0x650, 7, 7),
+	},
+};
+
+static const
+struct phy_gmii_sel_soc_data phy_gmii_sel_soc_am33xx = {
+	.num_ports = 2,
+	.features = BIT(PHY_GMII_SEL_RGMII_ID_MODE) |
+		    BIT(PHY_GMII_SEL_RMII_IO_CLK_EN),
+	.regfields = phy_gmii_sel_fields_am33xx,
+};
+
+static const
+struct reg_field phy_gmii_sel_fields_dra7[][PHY_GMII_SEL_LAST] = {
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x554, 0, 1),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD((~0), 0, 0),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD((~0), 0, 0),
+	},
+	{
+		[PHY_GMII_SEL_PORT_MODE] = REG_FIELD(0x554, 4, 5),
+		[PHY_GMII_SEL_RGMII_ID_MODE] = REG_FIELD((~0), 0, 0),
+		[PHY_GMII_SEL_RMII_IO_CLK_EN] = REG_FIELD((~0), 0, 0),
+	},
+};
+
+static const
+struct phy_gmii_sel_soc_data phy_gmii_sel_soc_dra7 = {
+	.num_ports = 2,
+	.regfields = phy_gmii_sel_fields_dra7,
+};
+
+static const
+struct phy_gmii_sel_soc_data phy_gmii_sel_soc_dm814 = {
+	.num_ports = 2,
+	.features = BIT(PHY_GMII_SEL_RGMII_ID_MODE),
+	.regfields = phy_gmii_sel_fields_am33xx,
+};
+
+static const struct of_device_id phy_gmii_sel_id_table[] = {
+	{
+		.compatible	= "ti,am3352-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_am33xx,
+	},
+	{
+		.compatible	= "ti,dra7xx-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_dra7,
+	},
+	{
+		.compatible	= "ti,am43xx-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_am33xx,
+	},
+	{
+		.compatible	= "ti,dm814-phy-gmii-sel",
+		.data		= &phy_gmii_sel_soc_dm814,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(of, phy_gmii_sel_id_table);
+
+static const struct phy_ops phy_gmii_sel_ops = {
+	.set_netif_mode	= phy_gmii_sel_mode,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *phy_gmii_sel_of_xlate(struct device *dev,
+						struct of_phandle_args *args)
+{
+	struct phy_gmii_sel_priv *priv = dev_get_drvdata(dev);
+	int phy_id = args->args[0];
+
+	if (args->args_count < 1)
+		return ERR_PTR(-EINVAL);
+	if (priv->soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN) &&
+	    args->args_count < 2)
+		return ERR_PTR(-EINVAL);
+	if (!priv || !priv->if_phys)
+		return ERR_PTR(-ENODEV);
+	if (phy_id > priv->soc_data->num_ports)
+		return ERR_PTR(-EINVAL);
+	if (phy_id != priv->if_phys[phy_id - 1].id)
+		return ERR_PTR(-EINVAL);
+
+	phy_id--;
+	if (priv->soc_data->features & BIT(PHY_GMII_SEL_RMII_IO_CLK_EN))
+		priv->if_phys[phy_id].rmii_clock_external = args->args[1];
+	dev_dbg(dev, "%s id:%u ext:%d\n", __func__,
+		priv->if_phys[phy_id].id, args->args[1]);
+
+	return priv->if_phys[phy_id].if_phy;
+}
+
+static int phy_gmii_sel_init_ports(struct phy_gmii_sel_priv *priv)
+{
+	const struct phy_gmii_sel_soc_data *soc_data = priv->soc_data;
+	struct device *dev = priv->dev;
+	struct phy_gmii_sel_phy_priv *if_phys;
+	int i, num_ports, ret;
+
+	num_ports = priv->soc_data->num_ports;
+
+	if_phys = devm_kcalloc(priv->dev, num_ports,
+			       sizeof(*if_phys), GFP_KERNEL);
+	if (!if_phys)
+		return -ENOMEM;
+	dev_dbg(dev, "%s %d\n", __func__, num_ports);
+
+	for (i = 0; i < num_ports; i++) {
+		const struct reg_field *field;
+		struct regmap_field *regfield;
+
+		if_phys[i].id = i + 1;
+		if_phys[i].priv = priv;
+
+		field = &soc_data->regfields[i][PHY_GMII_SEL_PORT_MODE];
+		dev_dbg(dev, "%s field %x %d %d\n", __func__,
+			field->reg, field->msb, field->lsb);
+
+		regfield = devm_regmap_field_alloc(dev, priv->regmap, *field);
+		if (IS_ERR(regfield))
+			return PTR_ERR(regfield);
+		if_phys[i].fields[PHY_GMII_SEL_PORT_MODE] = regfield;
+
+		field = &soc_data->regfields[i][PHY_GMII_SEL_RGMII_ID_MODE];
+		if (field->reg != (~0)) {
+			regfield = devm_regmap_field_alloc(dev,
+							   priv->regmap,
+							   *field);
+			if (IS_ERR(regfield))
+				return PTR_ERR(regfield);
+			if_phys[i].fields[PHY_GMII_SEL_RGMII_ID_MODE] =
+				regfield;
+		}
+
+		field = &soc_data->regfields[i][PHY_GMII_SEL_RMII_IO_CLK_EN];
+		if (field->reg != (~0)) {
+			regfield = devm_regmap_field_alloc(dev,
+							   priv->regmap,
+							   *field);
+			if (IS_ERR(regfield))
+				return PTR_ERR(regfield);
+			if_phys[i].fields[PHY_GMII_SEL_RMII_IO_CLK_EN] =
+				regfield;
+		}
+
+		if_phys[i].if_phy = devm_phy_create(dev,
+						    priv->dev->of_node,
+						    &phy_gmii_sel_ops);
+		if (IS_ERR(if_phys[i].if_phy)) {
+			ret = PTR_ERR(if_phys[i].if_phy);
+			dev_err(dev, "Failed to create phy%d %d\n", i, ret);
+			return ret;
+		}
+		phy_set_drvdata(if_phys[i].if_phy, &if_phys[i]);
+	}
+
+	priv->if_phys = if_phys;
+	return 0;
+}
+
+static int phy_gmii_sel_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	const struct of_device_id *of_id;
+	struct phy_gmii_sel_priv *priv;
+	int ret;
+
+	of_id = of_match_node(phy_gmii_sel_id_table, pdev->dev.of_node);
+	if (!of_id)
+		return -EINVAL;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+	priv->soc_data = of_id->data;
+
+	priv->regmap = syscon_regmap_lookup_by_phandle(node, "syscon-scm");
+	if (IS_ERR(priv->regmap)) {
+		ret = PTR_ERR(priv->regmap);
+		dev_err(dev, "Failed to get syscon %d\n", ret);
+		return ret;
+	}
+
+	ret = phy_gmii_sel_init_ports(priv);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(&pdev->dev, priv);
+
+	priv->phy_provider =
+		devm_of_phy_provider_register(dev,
+					      phy_gmii_sel_of_xlate);
+	if (IS_ERR(priv->phy_provider)) {
+		ret = PTR_ERR(priv->phy_provider);
+		dev_err(dev, "Failed to create phy provider %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver phy_gmii_sel_driver = {
+	.probe		= phy_gmii_sel_probe,
+	.driver		= {
+		.name	= "phy-gmii-sel",
+		.of_match_table = phy_gmii_sel_id_table,
+	},
+};
+module_platform_driver(phy_gmii_sel_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Grygorii Strashko <grygorii.strashko@ti.com>");
+MODULE_DESCRIPTION("TI CPSW Port's PHY Interface Mode selection Driver");
-- 
2.10.5

^ permalink raw reply related

* [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings
From: Grygorii Strashko @ 2018-10-08 23:49 UTC (permalink / raw)
  To: David S. Miller, netdev, Tony Lindgren, Rob Herring,
	Kishon Vijay Abraham I
  Cc: Sekhar Nori, linux-kernel, linux-omap, devicetree,
	Grygorii Strashko
In-Reply-To: <20181008234949.15416-1-grygorii.strashko@ti.com>

Add CPSW Port's Interface Mode Selection PHY (phy-gmii-sel) DT Bindings

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/phy/ti-phy-gmii-sel.txt    | 68 ++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt

diff --git a/Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt b/Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt
new file mode 100644
index 0000000..fd81421
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti-phy-gmii-sel.txt
@@ -0,0 +1,68 @@
+CPSW Port's Interface Mode Selection PHY Tree Bindings
+-----------------------------------------------
+
+TI am335x/am437x/dra7(am5)/dm814x CPSW3G Ethernet Subsystem supports
+two 10/100/1000 Ethernet ports with selectable G/MII, RMII, and RGMII interfaces.
+The interface mode is selected by configuring the MII mode selection register(s)
+(GMII_SEL) in the System Control Module chapter (SCM). GMII_SEL register(s) and
+bit fields placement in SCM are different between SoCs while fields meaning
+is the same.
+                                               +--------------+
+        +-------------------------------+      |SCM           |
+        |                     CPSW      |      |  +---------+ |
+        |        +--------------------------------+gmii_sel | |
+        |        |                      |      |  +---------+ |
+        |   +----v---+     +--------+   |      +--------------+
+        |   |Port 1..<--+-->GMII/MII<------->
+        |   |        |  |  |        |   |
+        |   +--------+  |  +--------+   |
+        |               |               |
+        |               |  +--------+   |
+        |               |  | RMII   <------->
+        |               +-->        |   |
+        |               |  +--------+   |
+        |               |               |
+        |               |  +--------+   |
+        |               |  | RGMII  <------->
+        |               +-->        |   |
+        |                  +--------+   |
+        +-------------------------------+
+
+CPSW Port's Interface Mode Selection PHY describes MII interface mode between
+CPSW Port and Ethernet PHY which depends on Eth PHY and board configuration.
+
+CPSW Port's Interface Mode Selection PHY device should defined as child device
+of SCM node (scm_conf) and can be attached to each CPSW port node using standard
+PHY bindings (See phy/phy-bindings.txt).
+
+Required properties:
+- compatible		: Should be "ti,am3352-phy-gmii-sel" for am335x platform
+			  "ti,dra7xx-phy-gmii-sel" for dra7xx/am57xx platform
+			  "ti,am43xx-phy-gmii-sel" for am43xx platform
+			  "ti,dm814-phy-gmii-sel" for dm814x platform
+- #phy-cells		: must be 2.
+			  cell 1 - CPSW port number (starting from 1)
+			  cell 2 - RMII refclk mode
+- syscon-scm		: phandle on SCM node (mfd/syscon.txt)
+
+Examples:
+	phy_gmii_sel: phy-gmii-sel {
+		compatible = "ti,am3352-phy-gmii-sel";
+		syscon-scm = <&scm_conf>;
+		#phy-cells = <2>;
+	};
+
+	mac: ethernet@4a100000 {
+		compatible = "ti,am335x-cpsw","ti,cpsw";
+		...
+
+		cpsw_emac0: slave@4a100200 {
+			...
+			phys = <&phy_gmii_sel 1 1>;
+		};
+
+		cpsw_emac1: slave@4a100300 {
+			...
+			phys = <&phy_gmii_sel 2 1>;
+		};
+	};
-- 
2.10.5

^ permalink raw reply related

* [RFC PATCH 01/11] phy: core add phy_set_netif_mode() api
From: Grygorii Strashko @ 2018-10-08 23:49 UTC (permalink / raw)
  To: David S. Miller, netdev, Tony Lindgren, Rob Herring,
	Kishon Vijay Abraham I
  Cc: Sekhar Nori, linux-kernel, linux-omap, devicetree,
	Grygorii Strashko
In-Reply-To: <20181008234949.15416-1-grygorii.strashko@ti.com>

Add new API phy_set_netif_mode(struct phy *phy, phy_interface_t mode) and
new PHY operation callback .set_netif_mode() which intended to be implemnte
by PHY drivers which supports Network interrfaces mode selection. Both
accepts phy_interface_t vlaue as input parameter.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/phy/phy-core.c  | 15 +++++++++++++++
 include/linux/phy/phy.h | 12 ++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 35fd38c..d9aba1a 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -377,6 +377,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
 }
 EXPORT_SYMBOL_GPL(phy_set_mode);
 
+int phy_set_netif_mode(struct phy *phy, phy_interface_t mode)
+{
+	int ret;
+
+	if (!phy || !phy->ops->set_netif_mode)
+		return 0;
+
+	mutex_lock(&phy->mutex);
+	ret = phy->ops->set_netif_mode(phy, mode);
+	mutex_unlock(&phy->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(phy_set_netif_mode);
+
 int phy_reset(struct phy *phy)
 {
 	int ret;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 9713aeb..bc73d2b 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -17,6 +17,7 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/device.h>
+#include <linux/phy.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 
@@ -49,6 +50,7 @@ enum phy_mode {
  * @power_on: powering on the phy
  * @power_off: powering off the phy
  * @set_mode: set the mode of the phy
+ * @set_netif_mode: set the mode of the net interface phy
  * @reset: resetting the phy
  * @calibrate: calibrate the phy
  * @owner: the module owner containing the ops
@@ -59,6 +61,7 @@ struct phy_ops {
 	int	(*power_on)(struct phy *phy);
 	int	(*power_off)(struct phy *phy);
 	int	(*set_mode)(struct phy *phy, enum phy_mode mode);
+	int	(*set_netif_mode)(struct phy *phy, phy_interface_t mode);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
 	struct module *owner;
@@ -163,6 +166,7 @@ int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
 int phy_set_mode(struct phy *phy, enum phy_mode mode);
+int phy_set_netif_mode(struct phy *phy, phy_interface_t mode);
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
 	return phy->attrs.mode;
@@ -283,6 +287,14 @@ static inline int phy_set_mode(struct phy *phy, enum phy_mode mode)
 	return -ENOSYS;
 }
 
+static inline int phy_set_netif_mode(struct phy *phy,
+				     phy_interface_t mode)
+{
+	if (!phy)
+		return 0;
+	return -ENOTSUPP;
+}
+
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
 	return PHY_MODE_INVALID;
-- 
2.10.5

^ permalink raw reply related

* [PATCH net] rds: RDS (tcp) hangs on sendto() to unresponding address
From: Ka-Cheong Poon @ 2018-10-08 16:17 UTC (permalink / raw)
  To: netdev; +Cc: santosh.shilimkar, davem, rds-devel

In rds_send_mprds_hash(), if the calculated hash value is non-zero and
the MPRDS connections are not yet up, it will wait.  But it should not
wait if the send is non-blocking.  In this case, it should just use the
base c_path for sending the message.

Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
---
 net/rds/send.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/rds/send.c b/net/rds/send.c
index 57b3d5a..fe785ee 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1007,7 +1007,8 @@ static int rds_cmsg_send(struct rds_sock *rs, struct rds_message *rm,
 	return ret;
 }
 
-static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
+static int rds_send_mprds_hash(struct rds_sock *rs,
+			       struct rds_connection *conn, int nonblock)
 {
 	int hash;
 
@@ -1023,10 +1024,16 @@ static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
 		 * used.  But if we are interrupted, we have to use the zero
 		 * c_path in case the connection ends up being non-MP capable.
 		 */
-		if (conn->c_npaths == 0)
+		if (conn->c_npaths == 0) {
+			/* Cannot wait for the connection be made, so just use
+			 * the base c_path.
+			 */
+			if (nonblock)
+				return 0;
 			if (wait_event_interruptible(conn->c_hs_waitq,
 						     conn->c_npaths != 0))
 				hash = 0;
+		}
 		if (conn->c_npaths == 1)
 			hash = 0;
 	}
@@ -1256,7 +1263,7 @@ int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t payload_len)
 	}
 
 	if (conn->c_trans->t_mp_capable)
-		cpath = &conn->c_path[rds_send_mprds_hash(rs, conn)];
+		cpath = &conn->c_path[rds_send_mprds_hash(rs, conn, nonblock)];
 	else
 		cpath = &conn->c_path[0];
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH net-next v7 03/28] zinc: introduce minimal cryptography library
From: Eric Biggers @ 2018-10-08 23:22 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, netdev, davem, gregkh, Samuel Neves,
	Jean-Philippe Aumasson, Andy Lutomirski, Andrew Morton,
	Linus Torvalds, kernel-hardening, linux-crypto
In-Reply-To: <20181006025709.4019-4-Jason@zx2c4.com>

On Sat, Oct 06, 2018 at 04:56:44AM +0200, Jason A. Donenfeld wrote:
> Zinc stands for "Zinc Is Neat Crypto" or "Zinc as IN Crypto". It's also
> short, easy to type, and plays nicely with the recent trend of naming
> crypto libraries after elements. The guiding principle is "don't overdo
> it". It's less of a library and more of a directory tree for organizing
> well-curated direct implementations of cryptography primitives.
> 

As I've asked before: please Cc linux-crypto on the whole patch series,
including the cover letter.

- Eric

^ permalink raw reply

* Re: [PATCH bpf-next v3 07/15] bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP
From: Björn Töpel @ 2018-10-08 16:05 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Karlsson, Magnus, Duyck, Alexander H, Alexander Duyck,
	John Fastabend, Alexei Starovoitov, Jesper Dangaard Brouer,
	Willem de Bruijn, Daniel Borkmann, Michael S. Tsirkin, Netdev,
	Björn Töpel, michael.lundkvist, Brandeburg, Jesse,
	Singhai, Anjali, Zhang, Qi Z
In-Reply-To: <ac8d2c20-f0fc-725c-a0a9-bee0b1620af1@gmail.com>

Den mån 8 okt. 2018 kl 17:31 skrev Eric Dumazet <eric.dumazet@gmail.com>:
>
> On 05/02/2018 04:01 AM, Björn Töpel wrote:
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > The xskmap is yet another BPF map, very much inspired by
> > dev/cpu/sockmap, and is a holder of AF_XDP sockets. A user application
> > adds AF_XDP sockets into the map, and by using the bpf_redirect_map
> > helper, an XDP program can redirect XDP frames to an AF_XDP socket.
> >
> > Note that a socket that is bound to certain ifindex/queue index will
> > *only* accept XDP frames from that netdev/queue index. If an XDP
> > program tries to redirect from a netdev/queue index other than what
> > the socket is bound to, the frame will not be received on the socket.
> >
> > A socket can reside in multiple maps.
> >
> > v3: Fixed race and simplified code.
> > v2: Removed one indirection in map lookup.
> >
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> > ---
> >  include/linux/bpf.h       |  25 +++++
> >  include/linux/bpf_types.h |   3 +
> >  include/net/xdp_sock.h    |   7 ++
> >  include/uapi/linux/bpf.h  |   1 +
> >  kernel/bpf/Makefile       |   3 +
> >  kernel/bpf/verifier.c     |   8 +-
> >  kernel/bpf/xskmap.c       | 239 ++++++++++++++++++++++++++++++++++++++++++++++
> >  net/xdp/xsk.c             |   5 +
> >  8 files changed, 289 insertions(+), 2 deletions(-)
> >  create mode 100644 kernel/bpf/xskmap.c
> >
>
> This function is called under rcu_read_lock() , from map_update_elem()
>
> > +
> > +static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> > +                            u64 map_flags)
> > +{
> > +     struct xsk_map *m = container_of(map, struct xsk_map, map);
> > +     u32 i = *(u32 *)key, fd = *(u32 *)value;
> > +     struct xdp_sock *xs, *old_xs;
> > +     struct socket *sock;
> > +     int err;
> > +
> > +     if (unlikely(map_flags > BPF_EXIST))
> > +             return -EINVAL;
> > +     if (unlikely(i >= m->map.max_entries))
> > +             return -E2BIG;
> > +     if (unlikely(map_flags == BPF_NOEXIST))
> > +             return -EEXIST;
> > +
> > +     sock = sockfd_lookup(fd, &err);
> > +     if (!sock)
> > +             return err;
> > +
> > +     if (sock->sk->sk_family != PF_XDP) {
> > +             sockfd_put(sock);
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     xs = (struct xdp_sock *)sock->sk;
> > +
> > +     if (!xsk_is_setup_for_bpf_map(xs)) {
> > +             sockfd_put(sock);
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     sock_hold(sock->sk);
> > +
> > +     old_xs = xchg(&m->xsk_map[i], xs);
> > +     if (old_xs) {
> > +             /* Make sure we've flushed everything. */
>
> So it is illegal to call synchronize_net(), since it is a reschedule point.
>

Thanks for finding and pointing this out, Eric!

I'll have look and get back with a patch.


Björn


> > +             synchronize_net();
> > +             sock_put((struct sock *)old_xs);
> > +     }
> > +
> > +     sockfd_put(sock);
> > +     return 0;
> > +}
> >
>
>
>

^ permalink raw reply

* Re: [PATCH net 00/10] rxrpc: Fix packet reception code
From: David Howells @ 2018-10-08 23:00 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

Note that there's a conflict with net-next/master versus:

	d2944b1c66a502ada8aa67f508cd29ecbf035892
	rxrpc: Use rxrpc_free_skb() rather than rxrpc_lose_skb()

The issue is this change in net/rxrpc/input.c:

	-			rxrpc_lose_skb(skb, rxrpc_skb_rx_lost);
	+			rxrpc_free_skb(skb, rxrpc_skb_rx_lost);
				return;

The return statement returns 0 in one of the patches here and should do so in
the resulting merge.

David

^ permalink raw reply

* Re: [PATCHv2 bpf-next] bpf: fix building without CONFIG_INET
From: Daniel Borkmann @ 2018-10-08 22:54 UTC (permalink / raw)
  To: Song Liu, joe
  Cc: Arnd Bergmann, Alexei Starovoitov, David S . Miller,
	John Fastabend, Martin KaFai Lau, makita.toshiaki,
	Lawrence Brakmo, Andrey Ignatov, Jesper Dangaard Brouer,
	Jakub Kicinski, Mathieu Xhonneux, David Ahern, Networking,
	open list
In-Reply-To: <CAPhsuW5wNDEEQbHY_zCTcCQ_FKN4poBGq26H2Gksd=QB1SGZow@mail.gmail.com>

On 10/08/2018 10:52 PM, Song Liu wrote:
> On Mon, Oct 8, 2018 at 11:30 AM Joe Stringer <joe@wand.net.nz> wrote:
>>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> The newly added TCP and UDP handling fails to link when CONFIG_INET
>> is disabled:
>>
>> net/core/filter.o: In function `sk_lookup':
>> filter.c:(.text+0x7ff8): undefined reference to `tcp_hashinfo'
>> filter.c:(.text+0x7ffc): undefined reference to `tcp_hashinfo'
>> filter.c:(.text+0x8020): undefined reference to `__inet_lookup_established'
>> filter.c:(.text+0x8058): undefined reference to `__inet_lookup_listener'
>> filter.c:(.text+0x8068): undefined reference to `udp_table'
>> filter.c:(.text+0x8070): undefined reference to `udp_table'
>> filter.c:(.text+0x808c): undefined reference to `__udp4_lib_lookup'
>> net/core/filter.o: In function `bpf_sk_release':
>> filter.c:(.text+0x82e8): undefined reference to `sock_gen_put'
>>
>> Wrap the related sections of code in #ifdefs for the config option.
>>
>> Furthermore, sk_lookup() should always have been marked 'static', this
>> also avoids a warning about a missing prototype when building with
>> 'make W=1'.
>>
>> Fixes: 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> 
> Acked-by: Song Liu <songliubraving@fb.com>

Applied to bpf-next, thanks!

^ permalink raw reply

* [PATCH net 10/10] rxrpc: Fix the packet reception routine
From: David Howells @ 2018-10-08 22:48 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

The rxrpc_input_packet() function and its call tree was built around the
assumption that data_ready() handler called from UDP to inform a kernel
service that there is data to be had was non-reentrant.  This means that
certain locking could be dispensed with.

This, however, turns out not to be the case with a multi-queue network card
that can deliver packets to multiple cpus simultaneously.  Each of those
cpus can be in the rxrpc_input_packet() function at the same time.

Fix by adding or changing some structure members:

 (1) Add peer->rtt_input_lock to serialise access to the RTT buffer.

 (2) Make conn->service_id into a 32-bit variable so that it can be
     cmpxchg'd on all arches.

 (3) Add call->input_lock to serialise access to the Rx/Tx state.  Note
     that although the Rx and Tx states are (almost) entirely separate,
     there's no point completing the separation and having separate locks
     since it's a bi-phasal RPC protocol rather than a bi-direction
     streaming protocol.  Data transmission and data reception do not take
     place simultaneously on any particular call.

and making the following functional changes:

 (1) In rxrpc_input_data(), hold call->input_lock around the core to
     prevent simultaneous producing of packets into the Rx ring and
     updating of tracking state for a particular call.

 (2) In rxrpc_input_ping_response(), only read call->ping_serial once, and
     check it before checking RXRPC_CALL_PINGING as that's a cheaper test.
     The bit test and bit clear can then be combined.  No further locking
     is needed here.

 (3) In rxrpc_input_ack(), take call->input_lock after we've parsed much of
     the ACK packet.  The superseded ACK check is then done both before and
     after the lock is taken.

     The handing of ackinfo data is split, parsing before the lock is taken
     and processing with it held.  This is keyed on rxMTU being non-zero.

     Congestion management is also done within the locked section.

 (4) In rxrpc_input_ackall(), take call->input_lock around the Tx window
     rotation.  The ACKALL packet carries no information and is only really
     useful after all packets have been transmitted since it's imprecise.

 (5) In rxrpc_input_implicit_end_call(), we use rx->incoming_lock to
     prevent calls being simultaneously implicitly ended on two cpus and
     also to prevent any races with incoming call setup.

 (6) In rxrpc_input_packet(), use cmpxchg() to effect the service upgrade
     on a connection.  It is only permitted to happen once for a
     connection.

 (7) In rxrpc_new_incoming_call(), we have to recheck the routing inside
     rx->incoming_lock to see if someone else set up the call, connection
     or peer whilst we were getting there.  We can't trust the values from
     the earlier routing check unless we pin refs on them - which we want
     to avoid.

     Further, we need to allow for an incoming call to have its state
     changed on another CPU between us making it live and us adjusting it
     because the conn is now in the RXRPC_CONN_SERVICE state.

 (8) In rxrpc_peer_add_rtt(), take peer->rtt_input_lock around the access
     to the RTT buffer.  Don't need to lock around setting peer->rtt.

For reference, the inventory of state-accessing or state-altering functions
used by the packet input procedure is:

> rxrpc_input_packet()
  * PACKET CHECKING

  * ROUTING
    > rxrpc_post_packet_to_local()
    > rxrpc_find_connection_rcu() - uses RCU
      > rxrpc_lookup_peer_rcu() - uses RCU
      > rxrpc_find_service_conn_rcu() - uses RCU
      > idr_find() - uses RCU

  * CONNECTION-LEVEL PROCESSING
    - Service upgrade
      - Can only happen once per conn
      ! Changed to use cmpxchg
    > rxrpc_post_packet_to_conn()
    - Setting conn->hi_serial
      - Probably safe not using locks
      - Maybe use cmpxchg

  * CALL-LEVEL PROCESSING
    > Old-call checking
      > rxrpc_input_implicit_end_call()
        > rxrpc_call_completed()
	> rxrpc_queue_call()
	! Need to take rx->incoming_lock
	> __rxrpc_disconnect_call()
	> rxrpc_notify_socket()
    > rxrpc_new_incoming_call()
      - Uses rx->incoming_lock for the entire process
        - Might be able to drop this earlier in favour of the call lock
      > rxrpc_incoming_call()
      	! Conflicts with rxrpc_input_implicit_end_call()
    > rxrpc_send_ping()
      - Don't need locks to check rtt state
      > rxrpc_propose_ACK

  * PACKET DISTRIBUTION
    > rxrpc_input_call_packet()
      > rxrpc_input_data()
	* QUEUE DATA PACKET ON CALL
	> rxrpc_reduce_call_timer()
	  - Uses timer_reduce()
	! Needs call->input_lock()
	> rxrpc_receiving_reply()
	  ! Needs locking around ack state
	  > rxrpc_rotate_tx_window()
	  > rxrpc_end_tx_phase()
	> rxrpc_proto_abort()
	> rxrpc_input_dup_data()
	- Fills the Rx buffer
	- rxrpc_propose_ACK()
	- rxrpc_notify_socket()

      > rxrpc_input_ack()
	* APPLY ACK PACKET TO CALL AND DISCARD PACKET
	> rxrpc_input_ping_response()
	  - Probably doesn't need any extra locking
	  ! Need READ_ONCE() on call->ping_serial
	  > rxrpc_input_check_for_lost_ack()
	    - Takes call->lock to consult Tx buffer
	  > rxrpc_peer_add_rtt()
	    ! Needs to take a lock (peer->rtt_input_lock)
	    ! Could perhaps manage with cmpxchg() and xadd() instead
	> rxrpc_input_requested_ack
	  - Consults Tx buffer
	    ! Probably needs a lock
	  > rxrpc_peer_add_rtt()
	> rxrpc_propose_ack()
	> rxrpc_input_ackinfo()
	  - Changes call->tx_winsize
	    ! Use cmpxchg to handle change
	    ! Should perhaps track serial number
	  - Uses peer->lock to record MTU specification changes
	> rxrpc_proto_abort()
	! Need to take call->input_lock
	> rxrpc_rotate_tx_window()
	> rxrpc_end_tx_phase()
	> rxrpc_input_soft_acks()
	- Consults the Tx buffer
	> rxrpc_congestion_management()
	  - Modifies the Tx annotations
	  ! Needs call->input_lock()
	  > rxrpc_queue_call()

      > rxrpc_input_abort()
	* APPLY ABORT PACKET TO CALL AND DISCARD PACKET
	> rxrpc_set_call_completion()
	> rxrpc_notify_socket()

      > rxrpc_input_ackall()
	* APPLY ACKALL PACKET TO CALL AND DISCARD PACKET
	! Need to take call->input_lock
	> rxrpc_rotate_tx_window()
	> rxrpc_end_tx_phase()

    > rxrpc_reject_packet()

There are some functions used by the above that queue the packet, after
which the procedure is terminated:

 - rxrpc_post_packet_to_local()
   - local->event_queue is an sk_buff_head
   - local->processor is a work_struct
 - rxrpc_post_packet_to_conn()
   - conn->rx_queue is an sk_buff_head
   - conn->processor is a work_struct
 - rxrpc_reject_packet()
   - local->reject_queue is an sk_buff_head
   - local->processor is a work_struct

And some that offload processing to process context:

 - rxrpc_notify_socket()
   - Uses RCU lock
   - Uses call->notify_lock to call call->notify_rx
   - Uses call->recvmsg_lock to queue recvmsg side
 - rxrpc_queue_call()
   - call->processor is a work_struct
 - rxrpc_propose_ACK()
   - Uses call->lock to wrap __rxrpc_propose_ACK()

And a bunch that complete a call, all of which use call->state_lock to
protect the call state:

 - rxrpc_call_completed()
 - rxrpc_set_call_completion()
 - rxrpc_abort_call()
 - rxrpc_proto_abort()
   - Also uses rxrpc_queue_call()

Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |    7 ++-
 net/rxrpc/call_accept.c |   21 ++++++--
 net/rxrpc/call_object.c |    1 
 net/rxrpc/input.c       |  120 +++++++++++++++++++++++++++++++----------------
 net/rxrpc/peer_event.c  |    5 ++
 net/rxrpc/peer_object.c |    1 
 6 files changed, 105 insertions(+), 50 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 45307463b7dd..a6e6cae82c30 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -302,6 +302,7 @@ struct rxrpc_peer {
 
 	/* calculated RTT cache */
 #define RXRPC_RTT_CACHE_SIZE 32
+	spinlock_t		rtt_input_lock;	/* RTT lock for input routine */
 	ktime_t			rtt_last_req;	/* Time of last RTT request */
 	u64			rtt;		/* Current RTT estimate (in nS) */
 	u64			rtt_sum;	/* Sum of cache contents */
@@ -447,7 +448,7 @@ struct rxrpc_connection {
 	atomic_t		serial;		/* packet serial number counter */
 	unsigned int		hi_serial;	/* highest serial number received */
 	u32			security_nonce;	/* response re-use preventer */
-	u16			service_id;	/* Service ID, possibly upgraded */
+	u32			service_id;	/* Service ID, possibly upgraded */
 	u8			size_align;	/* data size alignment (for security) */
 	u8			security_size;	/* security header size */
 	u8			security_ix;	/* security type */
@@ -635,6 +636,8 @@ struct rxrpc_call {
 	bool			tx_phase;	/* T if transmission phase, F if receive phase */
 	u8			nr_jumbo_bad;	/* Number of jumbo dups/exceeds-windows */
 
+	spinlock_t		input_lock;	/* Lock for packet input to this call */
+
 	/* receive-phase ACK management */
 	u8			ackr_reason;	/* reason to ACK */
 	u16			ackr_skew;	/* skew on packet being ACK'd */
@@ -720,8 +723,6 @@ int rxrpc_service_prealloc(struct rxrpc_sock *, gfp_t);
 void rxrpc_discard_prealloc(struct rxrpc_sock *);
 struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *,
 					   struct rxrpc_sock *,
-					   struct rxrpc_peer *,
-					   struct rxrpc_connection *,
 					   struct sk_buff *);
 void rxrpc_accept_incoming_calls(struct rxrpc_local *);
 struct rxrpc_call *rxrpc_accept_call(struct rxrpc_sock *, unsigned long,
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index 1c4ebc0cb25b..652e314de38e 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -333,11 +333,11 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
  */
 struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 					   struct rxrpc_sock *rx,
-					   struct rxrpc_peer *peer,
-					   struct rxrpc_connection *conn,
 					   struct sk_buff *skb)
 {
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
+	struct rxrpc_connection *conn;
+	struct rxrpc_peer *peer;
 	struct rxrpc_call *call;
 
 	_enter("");
@@ -354,6 +354,13 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 		goto out;
 	}
 
+	/* The peer, connection and call may all have sprung into existence due
+	 * to a duplicate packet being handled on another CPU in parallel, so
+	 * we have to recheck the routing.  However, we're now holding
+	 * rx->incoming_lock, so the values should remain stable.
+	 */
+	conn = rxrpc_find_connection_rcu(local, skb, &peer);
+
 	call = rxrpc_alloc_incoming_call(rx, local, peer, conn, skb);
 	if (!call) {
 		skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
@@ -396,10 +403,12 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 
 	case RXRPC_CONN_SERVICE:
 		write_lock(&call->state_lock);
-		if (rx->discard_new_call)
-			call->state = RXRPC_CALL_SERVER_RECV_REQUEST;
-		else
-			call->state = RXRPC_CALL_SERVER_ACCEPTING;
+		if (call->state < RXRPC_CALL_COMPLETE) {
+			if (rx->discard_new_call)
+				call->state = RXRPC_CALL_SERVER_RECV_REQUEST;
+			else
+				call->state = RXRPC_CALL_SERVER_ACCEPTING;
+		}
 		write_unlock(&call->state_lock);
 		break;
 
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 0ca2c2dfd196..8f1a8f85b1f9 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -138,6 +138,7 @@ struct rxrpc_call *rxrpc_alloc_call(struct rxrpc_sock *rx, gfp_t gfp,
 	init_waitqueue_head(&call->waitq);
 	spin_lock_init(&call->lock);
 	spin_lock_init(&call->notify_lock);
+	spin_lock_init(&call->input_lock);
 	rwlock_init(&call->state_lock);
 	atomic_set(&call->usage, 1);
 	call->debug_id = debug_id;
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 04213a65c1ac..570b49d2da42 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -459,13 +459,15 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
 		}
 	}
 
+	spin_lock(&call->input_lock);
+
 	/* Received data implicitly ACKs all of the request packets we sent
 	 * when we're acting as a client.
 	 */
 	if ((state == RXRPC_CALL_CLIENT_SEND_REQUEST ||
 	     state == RXRPC_CALL_CLIENT_AWAIT_REPLY) &&
 	    !rxrpc_receiving_reply(call))
-		return;
+		goto unlock;
 
 	call->ackr_prev_seq = seq;
 
@@ -495,12 +497,16 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
 
 	if (flags & RXRPC_LAST_PACKET) {
 		if (test_bit(RXRPC_CALL_RX_LAST, &call->flags) &&
-		    seq != call->rx_top)
-			return rxrpc_proto_abort("LSN", call, seq);
+		    seq != call->rx_top) {
+			rxrpc_proto_abort("LSN", call, seq);
+			goto unlock;
+		}
 	} else {
 		if (test_bit(RXRPC_CALL_RX_LAST, &call->flags) &&
-		    after_eq(seq, call->rx_top))
-			return rxrpc_proto_abort("LSA", call, seq);
+		    after_eq(seq, call->rx_top)) {
+			rxrpc_proto_abort("LSA", call, seq);
+			goto unlock;
+		}
 	}
 
 	trace_rxrpc_rx_data(call->debug_id, seq, serial, flags, annotation);
@@ -567,8 +573,10 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
 skip:
 	offset += len;
 	if (flags & RXRPC_JUMBO_PACKET) {
-		if (skb_copy_bits(skb, offset, &flags, 1) < 0)
-			return rxrpc_proto_abort("XJF", call, seq);
+		if (skb_copy_bits(skb, offset, &flags, 1) < 0) {
+			rxrpc_proto_abort("XJF", call, seq);
+			goto unlock;
+		}
 		offset += sizeof(struct rxrpc_jumbo_header);
 		seq++;
 		serial++;
@@ -608,6 +616,9 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
 		trace_rxrpc_notify_socket(call->debug_id, serial);
 		rxrpc_notify_socket(call);
 	}
+
+unlock:
+	spin_unlock(&call->input_lock);
 	_leave(" [queued]");
 }
 
@@ -694,15 +705,14 @@ static void rxrpc_input_ping_response(struct rxrpc_call *call,
 
 	ping_time = call->ping_time;
 	smp_rmb();
-	ping_serial = call->ping_serial;
+	ping_serial = READ_ONCE(call->ping_serial);
 
 	if (orig_serial == call->acks_lost_ping)
 		rxrpc_input_check_for_lost_ack(call);
 
-	if (!test_bit(RXRPC_CALL_PINGING, &call->flags) ||
-	    before(orig_serial, ping_serial))
+	if (before(orig_serial, ping_serial) ||
+	    !test_and_clear_bit(RXRPC_CALL_PINGING, &call->flags))
 		return;
-	clear_bit(RXRPC_CALL_PINGING, &call->flags);
 	if (after(orig_serial, ping_serial))
 		return;
 
@@ -869,24 +879,31 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 	}
 
 	/* Discard any out-of-order or duplicate ACKs. */
-	if (before_eq(sp->hdr.serial, call->acks_latest)) {
-		_debug("discard ACK %d <= %d",
-		       sp->hdr.serial, call->acks_latest);
+	if (before_eq(sp->hdr.serial, call->acks_latest))
 		return;
-	}
+
+	buf.info.rxMTU = 0;
+	ioffset = offset + nr_acks + 3;
+	if (skb->len >= ioffset + sizeof(buf.info) &&
+	    skb_copy_bits(skb, ioffset, &buf.info, sizeof(buf.info)) < 0)
+		return rxrpc_proto_abort("XAI", call, 0);
+
+	spin_lock(&call->input_lock);
+
+	/* Discard any out-of-order or duplicate ACKs. */
+	if (before_eq(sp->hdr.serial, call->acks_latest))
+		goto out;
 	call->acks_latest_ts = skb->tstamp;
 	call->acks_latest = sp->hdr.serial;
 
 	/* Parse rwind and mtu sizes if provided. */
-	ioffset = offset + nr_acks + 3;
-	if (skb->len >= ioffset + sizeof(buf.info)) {
-		if (skb_copy_bits(skb, ioffset, &buf.info, sizeof(buf.info)) < 0)
-			return rxrpc_proto_abort("XAI", call, 0);
+	if (buf.info.rxMTU)
 		rxrpc_input_ackinfo(call, skb, &buf.info);
-	}
 
-	if (first_soft_ack == 0)
-		return rxrpc_proto_abort("AK0", call, 0);
+	if (first_soft_ack == 0) {
+		rxrpc_proto_abort("AK0", call, 0);
+		goto out;
+	}
 
 	/* Ignore ACKs unless we are or have just been transmitting. */
 	switch (READ_ONCE(call->state)) {
@@ -896,25 +913,31 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 	case RXRPC_CALL_SERVER_AWAIT_ACK:
 		break;
 	default:
-		return;
+		goto out;
 	}
 
 	if (before(hard_ack, call->tx_hard_ack) ||
-	    after(hard_ack, call->tx_top))
-		return rxrpc_proto_abort("AKW", call, 0);
-	if (nr_acks > call->tx_top - hard_ack)
-		return rxrpc_proto_abort("AKN", call, 0);
+	    after(hard_ack, call->tx_top)) {
+		rxrpc_proto_abort("AKW", call, 0);
+		goto out;
+	}
+	if (nr_acks > call->tx_top - hard_ack) {
+		rxrpc_proto_abort("AKN", call, 0);
+		goto out;
+	}
 
 	if (after(hard_ack, call->tx_hard_ack)) {
 		if (rxrpc_rotate_tx_window(call, hard_ack, &summary)) {
 			rxrpc_end_tx_phase(call, false, "ETA");
-			return;
+			goto out;
 		}
 	}
 
 	if (nr_acks > 0) {
-		if (skb_copy_bits(skb, offset, buf.acks, nr_acks) < 0)
-			return rxrpc_proto_abort("XSA", call, 0);
+		if (skb_copy_bits(skb, offset, buf.acks, nr_acks) < 0) {
+			rxrpc_proto_abort("XSA", call, 0);
+			goto out;
+		}
 		rxrpc_input_soft_acks(call, buf.acks, first_soft_ack, nr_acks,
 				      &summary);
 	}
@@ -927,7 +950,9 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 				  false, true,
 				  rxrpc_propose_ack_ping_for_lost_reply);
 
-	return rxrpc_congestion_management(call, skb, &summary, acked_serial);
+	rxrpc_congestion_management(call, skb, &summary, acked_serial);
+out:
+	spin_unlock(&call->input_lock);
 }
 
 /*
@@ -940,8 +965,12 @@ static void rxrpc_input_ackall(struct rxrpc_call *call, struct sk_buff *skb)
 
 	_proto("Rx ACKALL %%%u", sp->hdr.serial);
 
+	spin_lock(&call->input_lock);
+
 	if (rxrpc_rotate_tx_window(call, call->tx_top, &summary))
 		rxrpc_end_tx_phase(call, false, "ETL");
+
+	spin_unlock(&call->input_lock);
 }
 
 /*
@@ -1024,18 +1053,19 @@ static void rxrpc_input_call_packet(struct rxrpc_call *call,
 }
 
 /*
- * Handle a new call on a channel implicitly completing the preceding call on
- * that channel.
+ * Handle a new service call on a channel implicitly completing the preceding
+ * call on that channel.  This does not apply to client conns.
  *
  * TODO: If callNumber > call_id + 1, renegotiate security.
  */
-static void rxrpc_input_implicit_end_call(struct rxrpc_connection *conn,
+static void rxrpc_input_implicit_end_call(struct rxrpc_sock *rx,
+					  struct rxrpc_connection *conn,
 					  struct rxrpc_call *call)
 {
 	switch (READ_ONCE(call->state)) {
 	case RXRPC_CALL_SERVER_AWAIT_ACK:
 		rxrpc_call_completed(call);
-		break;
+		/* Fall through */
 	case RXRPC_CALL_COMPLETE:
 		break;
 	default:
@@ -1043,11 +1073,13 @@ static void rxrpc_input_implicit_end_call(struct rxrpc_connection *conn,
 			set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 			rxrpc_queue_call(call);
 		}
+		trace_rxrpc_improper_term(call);
 		break;
 	}
 
-	trace_rxrpc_improper_term(call);
+	spin_lock(&rx->incoming_lock);
 	__rxrpc_disconnect_call(conn, call);
+	spin_unlock(&rx->incoming_lock);
 	rxrpc_notify_socket(call);
 }
 
@@ -1244,10 +1276,16 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 			goto wrong_security;
 
 		if (sp->hdr.serviceId != conn->service_id) {
-			if (!test_bit(RXRPC_CONN_PROBING_FOR_UPGRADE, &conn->flags) ||
-			    conn->service_id != conn->params.service_id)
+			int old_id;
+
+			if (!test_bit(RXRPC_CONN_PROBING_FOR_UPGRADE, &conn->flags))
+				goto reupgrade;
+			old_id = cmpxchg(&conn->service_id, conn->params.service_id,
+					 sp->hdr.serviceId);
+
+			if (old_id != conn->params.service_id &&
+			    old_id != sp->hdr.serviceId)
 				goto reupgrade;
-			conn->service_id = sp->hdr.serviceId;
 		}
 
 		if (sp->hdr.callNumber == 0) {
@@ -1305,7 +1343,7 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 			if (rxrpc_to_client(sp))
 				goto reject_packet;
 			if (call)
-				rxrpc_input_implicit_end_call(conn, call);
+				rxrpc_input_implicit_end_call(rx, conn, call);
 			call = NULL;
 		}
 
@@ -1325,7 +1363,7 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 			goto bad_message;
 		if (sp->hdr.seq != 1)
 			goto discard;
-		call = rxrpc_new_incoming_call(local, rx, peer, conn, skb);
+		call = rxrpc_new_incoming_call(local, rx, skb);
 		if (!call)
 			goto reject_packet;
 		rxrpc_send_ping(call, skb, skew);
diff --git a/net/rxrpc/peer_event.c b/net/rxrpc/peer_event.c
index f3e6fc670da2..05b51bdbdd41 100644
--- a/net/rxrpc/peer_event.c
+++ b/net/rxrpc/peer_event.c
@@ -301,6 +301,8 @@ void rxrpc_peer_add_rtt(struct rxrpc_call *call, enum rxrpc_rtt_rx_trace why,
 	if (rtt < 0)
 		return;
 
+	spin_lock(&peer->rtt_input_lock);
+
 	/* Replace the oldest datum in the RTT buffer */
 	sum -= peer->rtt_cache[cursor];
 	sum += rtt;
@@ -312,6 +314,8 @@ void rxrpc_peer_add_rtt(struct rxrpc_call *call, enum rxrpc_rtt_rx_trace why,
 		peer->rtt_usage = usage;
 	}
 
+	spin_unlock(&peer->rtt_input_lock);
+
 	/* Now recalculate the average */
 	if (usage == RXRPC_RTT_CACHE_SIZE) {
 		avg = sum / RXRPC_RTT_CACHE_SIZE;
@@ -320,6 +324,7 @@ void rxrpc_peer_add_rtt(struct rxrpc_call *call, enum rxrpc_rtt_rx_trace why,
 		do_div(avg, usage);
 	}
 
+	/* Don't need to update this under lock */
 	peer->rtt = avg;
 	trace_rxrpc_rtt_rx(call, why, send_serial, resp_serial, rtt,
 			   usage, avg);
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 2d39eaf19620..5691b7d266ca 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -225,6 +225,7 @@ struct rxrpc_peer *rxrpc_alloc_peer(struct rxrpc_local *local, gfp_t gfp)
 		peer->service_conns = RB_ROOT;
 		seqlock_init(&peer->service_conn_lock);
 		spin_lock_init(&peer->lock);
+		spin_lock_init(&peer->rtt_input_lock);
 		peer->debug_id = atomic_inc_return(&rxrpc_debug_id);
 
 		if (RXRPC_TX_SMSS > 2190)

^ permalink raw reply related

* Re: [PATCH V2 net-next 2/5] net: Introduce a new MII time stamping interface.
From: Richard Cochran @ 2018-10-08 15:36 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Jacob Keller,
	Mark Rutland, Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20181008152800.u3mmtr4txt67o7jc@localhost>

On Mon, Oct 08, 2018 at 08:28:00AM -0700, Richard Cochran wrote:
> Let's stick to phylib for now.  We can cross the other bridge when we
> come to it.  Maybe the net_device->phylink will emerge for purposes
> other that time stamping.  Let's not guess about how it should look.

In fact, it is kernel policy to reject new framework that lacks users.
Even if we added new net_device->phylink hooks, I bet that davem would
reject them for that reason.  We can't just add to net_device for
vaporware.

The whole point of _this_ series is the first patch.  That new option
is important for complete PTP support, and it could be used by every
PTP hardware.  Unfortunately I only have one device in hand that
implements this, and that is the reason why you see patches 2-5.

Thanks,
Richard

^ permalink raw reply

* [PATCH net 09/10] rxrpc: Fix the rxrpc_tx_packet trace line
From: David Howells @ 2018-10-08 22:48 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

Fix the rxrpc_tx_packet trace line by storing the where parameter.

Fixes: 4764c0da69dc ("rxrpc: Trace packet transmission")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/trace/events/rxrpc.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 837393fa897b..573d5b901fb1 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -931,6 +931,7 @@ TRACE_EVENT(rxrpc_tx_packet,
 	    TP_fast_assign(
 		    __entry->call = call_id;
 		    memcpy(&__entry->whdr, whdr, sizeof(__entry->whdr));
+		    __entry->where = where;
 			   ),
 
 	    TP_printk("c=%08x %08x:%08x:%08x:%04x %08x %08x %02x %02x %s %s",

^ permalink raw reply related

* [PATCH net 08/10] rxrpc: Fix connection-level abort handling
From: David Howells @ 2018-10-08 22:48 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

Fix connection-level abort handling to cache the abort and error codes
properly so that a new incoming call can be properly aborted if it races
with the parent connection being aborted by another CPU.

The abort_code and error parameters can then be dropped from
rxrpc_abort_calls().

Fixes: f5c17aaeb2ae ("rxrpc: Calls should only have one terminal state")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |    4 ++--
 net/rxrpc/call_accept.c |    4 ++--
 net/rxrpc/conn_event.c  |   26 +++++++++++++++-----------
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index ab60c0313fd4..45307463b7dd 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -442,8 +442,7 @@ struct rxrpc_connection {
 	spinlock_t		state_lock;	/* state-change lock */
 	enum rxrpc_conn_cache_state cache_state;
 	enum rxrpc_conn_proto_state state;	/* current state of connection */
-	u32			local_abort;	/* local abort code */
-	u32			remote_abort;	/* remote abort code */
+	u32			abort_code;	/* Abort code of connection abort */
 	int			debug_id;	/* debug ID for printks */
 	atomic_t		serial;		/* packet serial number counter */
 	unsigned int		hi_serial;	/* highest serial number received */
@@ -453,6 +452,7 @@ struct rxrpc_connection {
 	u8			security_size;	/* security header size */
 	u8			security_ix;	/* security type */
 	u8			out_clientflag;	/* RXRPC_CLIENT_INITIATED if we are client */
+	short			error;		/* Local error code */
 };
 
 static inline bool rxrpc_to_server(const struct rxrpc_skb_priv *sp)
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index f55f67894465..1c4ebc0cb25b 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -405,11 +405,11 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 
 	case RXRPC_CONN_REMOTELY_ABORTED:
 		rxrpc_set_call_completion(call, RXRPC_CALL_REMOTELY_ABORTED,
-					  conn->remote_abort, -ECONNABORTED);
+					  conn->abort_code, conn->error);
 		break;
 	case RXRPC_CONN_LOCALLY_ABORTED:
 		rxrpc_abort_call("CON", call, sp->hdr.seq,
-				 conn->local_abort, -ECONNABORTED);
+				 conn->abort_code, conn->error);
 		break;
 	default:
 		BUG();
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 6df56ce68861..b6fca8ebb117 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -126,7 +126,7 @@ static void rxrpc_conn_retransmit_call(struct rxrpc_connection *conn,
 
 	switch (chan->last_type) {
 	case RXRPC_PACKET_TYPE_ABORT:
-		_proto("Tx ABORT %%%u { %d } [re]", serial, conn->local_abort);
+		_proto("Tx ABORT %%%u { %d } [re]", serial, conn->abort_code);
 		break;
 	case RXRPC_PACKET_TYPE_ACK:
 		trace_rxrpc_tx_ack(chan->call_debug_id, serial,
@@ -153,13 +153,12 @@ static void rxrpc_conn_retransmit_call(struct rxrpc_connection *conn,
  * pass a connection-level abort onto all calls on that connection
  */
 static void rxrpc_abort_calls(struct rxrpc_connection *conn,
-			      enum rxrpc_call_completion compl,
-			      u32 abort_code, int error)
+			      enum rxrpc_call_completion compl)
 {
 	struct rxrpc_call *call;
 	int i;
 
-	_enter("{%d},%x", conn->debug_id, abort_code);
+	_enter("{%d},%x", conn->debug_id, conn->abort_code);
 
 	spin_lock(&conn->channel_lock);
 
@@ -172,9 +171,11 @@ static void rxrpc_abort_calls(struct rxrpc_connection *conn,
 				trace_rxrpc_abort(call->debug_id,
 						  "CON", call->cid,
 						  call->call_id, 0,
-						  abort_code, error);
+						  conn->abort_code,
+						  conn->error);
 			if (rxrpc_set_call_completion(call, compl,
-						      abort_code, error))
+						      conn->abort_code,
+						      conn->error))
 				rxrpc_notify_socket(call);
 		}
 	}
@@ -207,10 +208,12 @@ static int rxrpc_abort_connection(struct rxrpc_connection *conn,
 		return 0;
 	}
 
+	conn->error = error;
+	conn->abort_code = abort_code;
 	conn->state = RXRPC_CONN_LOCALLY_ABORTED;
 	spin_unlock_bh(&conn->state_lock);
 
-	rxrpc_abort_calls(conn, RXRPC_CALL_LOCALLY_ABORTED, abort_code, error);
+	rxrpc_abort_calls(conn, RXRPC_CALL_LOCALLY_ABORTED);
 
 	msg.msg_name	= &conn->params.peer->srx.transport;
 	msg.msg_namelen	= conn->params.peer->srx.transport_len;
@@ -229,7 +232,7 @@ static int rxrpc_abort_connection(struct rxrpc_connection *conn,
 	whdr._rsvd	= 0;
 	whdr.serviceId	= htons(conn->service_id);
 
-	word		= htonl(conn->local_abort);
+	word		= htonl(conn->abort_code);
 
 	iov[0].iov_base	= &whdr;
 	iov[0].iov_len	= sizeof(whdr);
@@ -240,7 +243,7 @@ static int rxrpc_abort_connection(struct rxrpc_connection *conn,
 
 	serial = atomic_inc_return(&conn->serial);
 	whdr.serial = htonl(serial);
-	_proto("Tx CONN ABORT %%%u { %d }", serial, conn->local_abort);
+	_proto("Tx CONN ABORT %%%u { %d }", serial, conn->abort_code);
 
 	ret = kernel_sendmsg(conn->params.local->socket, &msg, iov, 2, len);
 	if (ret < 0) {
@@ -315,9 +318,10 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 		abort_code = ntohl(wtmp);
 		_proto("Rx ABORT %%%u { ac=%d }", sp->hdr.serial, abort_code);
 
+		conn->error = -ECONNABORTED;
+		conn->abort_code = abort_code;
 		conn->state = RXRPC_CONN_REMOTELY_ABORTED;
-		rxrpc_abort_calls(conn, RXRPC_CALL_REMOTELY_ABORTED,
-				  abort_code, -ECONNABORTED);
+		rxrpc_abort_calls(conn, RXRPC_CALL_REMOTELY_ABORTED);
 		return -ECONNABORTED;
 
 	case RXRPC_PACKET_TYPE_CHALLENGE:

^ permalink raw reply related

* [PATCH net 06/10] rxrpc: Carry call state out of locked section in rxrpc_rotate_tx_window()
From: David Howells @ 2018-10-08 22:48 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

Carry the call state out of the locked section in rxrpc_rotate_tx_window()
rather than sampling it afterwards.  This is only used to select tracepoint
data, but could have changed by the time we do the tracepoint.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/input.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 8834aa627371..af8ce64f4162 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -278,23 +278,26 @@ static bool rxrpc_rotate_tx_window(struct rxrpc_call *call, rxrpc_seq_t to,
 static bool rxrpc_end_tx_phase(struct rxrpc_call *call, bool reply_begun,
 			       const char *abort_why)
 {
+	unsigned int state;
 
 	ASSERT(test_bit(RXRPC_CALL_TX_LAST, &call->flags));
 
 	write_lock(&call->state_lock);
 
-	switch (call->state) {
+	state = call->state;
+	switch (state) {
 	case RXRPC_CALL_CLIENT_SEND_REQUEST:
 	case RXRPC_CALL_CLIENT_AWAIT_REPLY:
 		if (reply_begun)
-			call->state = RXRPC_CALL_CLIENT_RECV_REPLY;
+			call->state = state = RXRPC_CALL_CLIENT_RECV_REPLY;
 		else
-			call->state = RXRPC_CALL_CLIENT_AWAIT_REPLY;
+			call->state = state = RXRPC_CALL_CLIENT_AWAIT_REPLY;
 		break;
 
 	case RXRPC_CALL_SERVER_AWAIT_ACK:
 		__rxrpc_call_completed(call);
 		rxrpc_notify_socket(call);
+		state = call->state;
 		break;
 
 	default:
@@ -302,11 +305,10 @@ static bool rxrpc_end_tx_phase(struct rxrpc_call *call, bool reply_begun,
 	}
 
 	write_unlock(&call->state_lock);
-	if (call->state == RXRPC_CALL_CLIENT_AWAIT_REPLY) {
+	if (state == RXRPC_CALL_CLIENT_AWAIT_REPLY)
 		trace_rxrpc_transmit(call, rxrpc_transmit_await_reply);
-	} else {
+	else
 		trace_rxrpc_transmit(call, rxrpc_transmit_end);
-	}
 	_leave(" = ok");
 	return true;
 

^ permalink raw reply related

* [PATCH net 05/10] rxrpc: Don't check RXRPC_CALL_TX_LAST after calling rxrpc_rotate_tx_window()
From: David Howells @ 2018-10-08 22:47 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

We should only call the function to end a call's Tx phase if we rotated the
marked-last packet out of the transmission buffer.

Make rxrpc_rotate_tx_window() return an indication of whether it just
rotated the packet marked as the last out of the transmit buffer, carrying
the information out of the locked section in that function.

We can then check the return value instead of examining RXRPC_CALL_TX_LAST.

Fixes: 70790dbe3f66 ("rxrpc: Pass the last Tx packet marker in the annotation buffer")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/input.c |   35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 2dcef482acf2..8834aa627371 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -216,10 +216,11 @@ static void rxrpc_send_ping(struct rxrpc_call *call, struct sk_buff *skb,
 /*
  * Apply a hard ACK by advancing the Tx window.
  */
-static void rxrpc_rotate_tx_window(struct rxrpc_call *call, rxrpc_seq_t to,
+static bool rxrpc_rotate_tx_window(struct rxrpc_call *call, rxrpc_seq_t to,
 				   struct rxrpc_ack_summary *summary)
 {
 	struct sk_buff *skb, *list = NULL;
+	bool rot_last = false;
 	int ix;
 	u8 annotation;
 
@@ -243,15 +244,17 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, rxrpc_seq_t to,
 		skb->next = list;
 		list = skb;
 
-		if (annotation & RXRPC_TX_ANNO_LAST)
+		if (annotation & RXRPC_TX_ANNO_LAST) {
 			set_bit(RXRPC_CALL_TX_LAST, &call->flags);
+			rot_last = true;
+		}
 		if ((annotation & RXRPC_TX_ANNO_MASK) != RXRPC_TX_ANNO_ACK)
 			summary->nr_rot_new_acks++;
 	}
 
 	spin_unlock(&call->lock);
 
-	trace_rxrpc_transmit(call, (test_bit(RXRPC_CALL_TX_LAST, &call->flags) ?
+	trace_rxrpc_transmit(call, (rot_last ?
 				    rxrpc_transmit_rotate_last :
 				    rxrpc_transmit_rotate));
 	wake_up(&call->waitq);
@@ -262,6 +265,8 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, rxrpc_seq_t to,
 		skb->next = NULL;
 		rxrpc_free_skb(skb, rxrpc_skb_tx_freed);
 	}
+
+	return rot_last;
 }
 
 /*
@@ -332,11 +337,11 @@ static bool rxrpc_receiving_reply(struct rxrpc_call *call)
 		trace_rxrpc_timer(call, rxrpc_timer_init_for_reply, now);
 	}
 
-	if (!test_bit(RXRPC_CALL_TX_LAST, &call->flags))
-		rxrpc_rotate_tx_window(call, top, &summary);
 	if (!test_bit(RXRPC_CALL_TX_LAST, &call->flags)) {
-		rxrpc_proto_abort("TXL", call, top);
-		return false;
+		if (!rxrpc_rotate_tx_window(call, top, &summary)) {
+			rxrpc_proto_abort("TXL", call, top);
+			return false;
+		}
 	}
 	if (!rxrpc_end_tx_phase(call, true, "ETD"))
 		return false;
@@ -897,8 +902,12 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 	if (nr_acks > call->tx_top - hard_ack)
 		return rxrpc_proto_abort("AKN", call, 0);
 
-	if (after(hard_ack, call->tx_hard_ack))
-		rxrpc_rotate_tx_window(call, hard_ack, &summary);
+	if (after(hard_ack, call->tx_hard_ack)) {
+		if (rxrpc_rotate_tx_window(call, hard_ack, &summary)) {
+			rxrpc_end_tx_phase(call, false, "ETA");
+			return;
+		}
+	}
 
 	if (nr_acks > 0) {
 		if (skb_copy_bits(skb, offset, buf.acks, nr_acks) < 0)
@@ -907,11 +916,6 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 				      &summary);
 	}
 
-	if (test_bit(RXRPC_CALL_TX_LAST, &call->flags)) {
-		rxrpc_end_tx_phase(call, false, "ETA");
-		return;
-	}
-
 	if (call->rxtx_annotations[call->tx_top & RXRPC_RXTX_BUFF_MASK] &
 	    RXRPC_TX_ANNO_LAST &&
 	    summary.nr_acks == call->tx_top - hard_ack &&
@@ -933,8 +937,7 @@ static void rxrpc_input_ackall(struct rxrpc_call *call, struct sk_buff *skb)
 
 	_proto("Rx ACKALL %%%u", sp->hdr.serial);
 
-	rxrpc_rotate_tx_window(call, call->tx_top, &summary);
-	if (test_bit(RXRPC_CALL_TX_LAST, &call->flags))
+	if (rxrpc_rotate_tx_window(call, call->tx_top, &summary))
 		rxrpc_end_tx_phase(call, false, "ETL");
 }
 

^ permalink raw reply related

* [PATCH net 04/10] rxrpc: Don't need to take the RCU read lock in the packet receiver
From: David Howells @ 2018-10-08 22:47 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

We don't need to take the RCU read lock in the rxrpc packet receive
function because it's held further up the stack in the IP input routine
around the UDP receive routines.

Fix this by dropping the RCU read lock calls from rxrpc_input_packet().
This simplifies the code.

Fixes: 70790dbe3f66 ("rxrpc: Pass the last Tx packet marker in the annotation buffer")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/input.c |   41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 1866aeef2284..2dcef482acf2 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -1120,6 +1120,8 @@ int rxrpc_extract_header(struct rxrpc_skb_priv *sp, struct sk_buff *skb)
  * The socket is locked by the caller and this prevents the socket from being
  * shut down and the local endpoint from going away, thus sk_user_data will not
  * be cleared until this function returns.
+ *
+ * Called with the RCU read lock held from the IP layer via UDP.
  */
 int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 {
@@ -1215,8 +1217,6 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 	if (sp->hdr.serviceId == 0)
 		goto bad_message;
 
-	rcu_read_lock();
-
 	if (rxrpc_to_server(sp)) {
 		/* Weed out packets to services we're not offering.  Packets
 		 * that would begin a call are explicitly rejected and the rest
@@ -1228,7 +1228,7 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 			if (sp->hdr.type == RXRPC_PACKET_TYPE_DATA &&
 			    sp->hdr.seq == 1)
 				goto unsupported_service;
-			goto discard_unlock;
+			goto discard;
 		}
 	}
 
@@ -1248,7 +1248,7 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 			/* Connection-level packet */
 			_debug("CONN %p {%d}", conn, conn->debug_id);
 			rxrpc_post_packet_to_conn(conn, skb);
-			goto out_unlock;
+			goto out;
 		}
 
 		/* Note the serial number skew here */
@@ -1267,19 +1267,19 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 
 		/* Ignore really old calls */
 		if (sp->hdr.callNumber < chan->last_call)
-			goto discard_unlock;
+			goto discard;
 
 		if (sp->hdr.callNumber == chan->last_call) {
 			if (chan->call ||
 			    sp->hdr.type == RXRPC_PACKET_TYPE_ABORT)
-				goto discard_unlock;
+				goto discard;
 
 			/* For the previous service call, if completed
 			 * successfully, we discard all further packets.
 			 */
 			if (rxrpc_conn_is_service(conn) &&
 			    chan->last_type == RXRPC_PACKET_TYPE_ACK)
-				goto discard_unlock;
+				goto discard;
 
 			/* But otherwise we need to retransmit the final packet
 			 * from data cached in the connection record.
@@ -1290,16 +1290,14 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 						    sp->hdr.serial,
 						    sp->hdr.flags, 0);
 			rxrpc_post_packet_to_conn(conn, skb);
-			goto out_unlock;
+			goto out;
 		}
 
 		call = rcu_dereference(chan->call);
 
 		if (sp->hdr.callNumber > chan->call_id) {
-			if (rxrpc_to_client(sp)) {
-				rcu_read_unlock();
+			if (rxrpc_to_client(sp))
 				goto reject_packet;
-			}
 			if (call)
 				rxrpc_input_implicit_end_call(conn, call);
 			call = NULL;
@@ -1318,55 +1316,42 @@ int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 	if (!call || atomic_read(&call->usage) == 0) {
 		if (rxrpc_to_client(sp) ||
 		    sp->hdr.type != RXRPC_PACKET_TYPE_DATA)
-			goto bad_message_unlock;
+			goto bad_message;
 		if (sp->hdr.seq != 1)
-			goto discard_unlock;
+			goto discard;
 		call = rxrpc_new_incoming_call(local, rx, peer, conn, skb);
-		if (!call) {
-			rcu_read_unlock();
+		if (!call)
 			goto reject_packet;
-		}
 		rxrpc_send_ping(call, skb, skew);
 		mutex_unlock(&call->user_mutex);
 	}
 
 	rxrpc_input_call_packet(call, skb, skew);
-	goto discard_unlock;
+	goto discard;
 
-discard_unlock:
-	rcu_read_unlock();
 discard:
 	rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
 out:
 	trace_rxrpc_rx_done(0, 0);
 	return 0;
 
-out_unlock:
-	rcu_read_unlock();
-	goto out;
-
 wrong_security:
-	rcu_read_unlock();
 	trace_rxrpc_abort(0, "SEC", sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
 			  RXKADINCONSISTENCY, EBADMSG);
 	skb->priority = RXKADINCONSISTENCY;
 	goto post_abort;
 
 unsupported_service:
-	rcu_read_unlock();
 	trace_rxrpc_abort(0, "INV", sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
 			  RX_INVALID_OPERATION, EOPNOTSUPP);
 	skb->priority = RX_INVALID_OPERATION;
 	goto post_abort;
 
 reupgrade:
-	rcu_read_unlock();
 	trace_rxrpc_abort(0, "UPG", sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
 			  RX_PROTOCOL_ERROR, EBADMSG);
 	goto protocol_error;
 
-bad_message_unlock:
-	rcu_read_unlock();
 bad_message:
 	trace_rxrpc_abort(0, "BAD", sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
 			  RX_PROTOCOL_ERROR, EBADMSG);

^ permalink raw reply related

* [PATCH net 03/10] rxrpc: Use the UDP encap_rcv hook
From: David Howells @ 2018-10-08 22:47 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel
In-Reply-To: <153903883882.17944.17642727588248415623.stgit@warthog.procyon.org.uk>

Use the UDP encap_rcv hook to cut the bit out of the rxrpc packet reception
in which a packet is placed onto the UDP receive queue and then immediately
removed again by rxrpc.  Going via the queue in this manner seems like it
should be unnecessary.

This does, however, require the invention of a value to place in encap_type
as that's one of the conditions to switch packets out to the encap_rcv
hook.  Possibly the value doesn't actually matter for anything other than
sockopts on the UDP socket, which aren't accessible outside of rxrpc
anyway.

This seems to cut a bit of time out of the time elapsed between each
sk_buff being timestamped and turning up in rxrpc (the final number in the
following trace excerpts).  I measured this by making the rxrpc_rx_packet
trace point print the time elapsed between the skb being timestamped and
the current time (in ns), e.g.:

	... 424.278721: rxrpc_rx_packet: ...  ACK 25026

So doing a 512MiB DIO read from my test server, with an unmodified kernel:

	N       min     max     sum		mean    stddev
	27605   2626    7581    7.83992e+07     2840.04 181.029

and with the patch applied:

	N       min     max     sum		mean    stddev
	27547   1895    12165   6.77461e+07     2459.29 255.02

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/uapi/linux/udp.h |    1 +
 net/rxrpc/ar-internal.h  |    2 +-
 net/rxrpc/input.c        |   50 +++++++++++-----------------------------------
 net/rxrpc/local_object.c |   27 ++++++++++++++++++++-----
 4 files changed, 36 insertions(+), 44 deletions(-)

diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
index 09d00f8c442b..09502de447f5 100644
--- a/include/uapi/linux/udp.h
+++ b/include/uapi/linux/udp.h
@@ -40,5 +40,6 @@ struct udphdr {
 #define UDP_ENCAP_L2TPINUDP	3 /* rfc2661 */
 #define UDP_ENCAP_GTP0		4 /* GSM TS 09.60 */
 #define UDP_ENCAP_GTP1U		5 /* 3GPP TS 29.060 */
+#define UDP_ENCAP_RXRPC		6
 
 #endif /* _UAPI_LINUX_UDP_H */
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 63c43b3a2096..ab60c0313fd4 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -966,7 +966,7 @@ void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
 /*
  * input.c
  */
-void rxrpc_data_ready(struct sock *);
+int rxrpc_input_packet(struct sock *, struct sk_buff *);
 
 /*
  * insecure.c
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index c3114fa66c92..1866aeef2284 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -1121,7 +1121,7 @@ int rxrpc_extract_header(struct rxrpc_skb_priv *sp, struct sk_buff *skb)
  * shut down and the local endpoint from going away, thus sk_user_data will not
  * be cleared until this function returns.
  */
-void rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
+int rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 {
 	struct rxrpc_connection *conn;
 	struct rxrpc_channel *chan;
@@ -1135,6 +1135,13 @@ void rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 
 	_enter("%p", udp_sk);
 
+	if (skb->tstamp == 0)
+		skb->tstamp = ktime_get_real();
+
+	rxrpc_new_skb(skb, rxrpc_skb_rx_received);
+
+	skb_pull(skb, sizeof(struct udphdr));
+
 	/* The UDP protocol already released all skb resources;
 	 * we are free to add our own data there.
 	 */
@@ -1148,8 +1155,8 @@ void rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 		static int lose;
 		if ((lose++ & 7) == 7) {
 			trace_rxrpc_rx_lose(sp);
-			rxrpc_lose_skb(skb, rxrpc_skb_rx_lost);
-			return;
+			rxrpc_free_skb(skb, rxrpc_skb_rx_lost);
+			return 0;
 		}
 	}
 
@@ -1332,7 +1339,7 @@ void rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 	rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
 out:
 	trace_rxrpc_rx_done(0, 0);
-	return;
+	return 0;
 
 out_unlock:
 	rcu_read_unlock();
@@ -1371,38 +1378,5 @@ void rxrpc_input_packet(struct sock *udp_sk, struct sk_buff *skb)
 	trace_rxrpc_rx_done(skb->mark, skb->priority);
 	rxrpc_reject_packet(local, skb);
 	_leave(" [badmsg]");
-}
-
-void rxrpc_data_ready(struct sock *udp_sk)
-{
-	struct sk_buff *skb;
-	int ret;
-
-	for (;;) {
-		skb = skb_recv_udp(udp_sk, 0, 1, &ret);
-		if (!skb) {
-			if (ret == -EAGAIN)
-				return;
-
-			/* If there was a transmission failure, we get an error
-			 * here that we need to ignore.
-			 */
-			_debug("UDP socket error %d", ret);
-			continue;
-		}
-
-		rxrpc_new_skb(skb, rxrpc_skb_rx_received);
-
-		/* we'll probably need to checksum it (didn't call sock_recvmsg) */
-		if (skb_checksum_complete(skb)) {
-			rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
-			__UDP_INC_STATS(sock_net(udp_sk), UDP_MIB_INERRORS, 0);
-			_debug("csum failed");
-			continue;
-		}
-
-		__UDP_INC_STATS(sock_net(udp_sk), UDP_MIB_INDATAGRAMS, 0);
-
-		rxrpc_input_packet(udp_sk, skb);
-	}
+	return 0;
 }
diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index 30862f44c9f1..cad0691c2bb4 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -19,6 +19,7 @@
 #include <linux/ip.h>
 #include <linux/hashtable.h>
 #include <net/sock.h>
+#include <net/udp.h>
 #include <net/af_rxrpc.h>
 #include "ar-internal.h"
 
@@ -108,7 +109,7 @@ static struct rxrpc_local *rxrpc_alloc_local(struct rxrpc_net *rxnet,
  */
 static int rxrpc_open_socket(struct rxrpc_local *local, struct net *net)
 {
-	struct sock *sock;
+	struct sock *usk;
 	int ret, opt;
 
 	_enter("%p{%d,%d}",
@@ -123,10 +124,26 @@ static int rxrpc_open_socket(struct rxrpc_local *local, struct net *net)
 	}
 
 	/* set the socket up */
-	sock = local->socket->sk;
-	sock->sk_user_data	= local;
-	sock->sk_data_ready	= rxrpc_data_ready;
-	sock->sk_error_report	= rxrpc_error_report;
+	usk = local->socket->sk;
+	inet_sk(usk)->mc_loop = 0;
+
+	/* Enable CHECKSUM_UNNECESSARY to CHECKSUM_COMPLETE conversion */
+	inet_inc_convert_csum(usk);
+
+	rcu_assign_sk_user_data(usk, local);
+
+	udp_sk(usk)->encap_type = UDP_ENCAP_RXRPC;
+	udp_sk(usk)->encap_rcv = rxrpc_input_packet;
+	udp_sk(usk)->encap_destroy = NULL;
+	udp_sk(usk)->gro_receive = NULL;
+	udp_sk(usk)->gro_complete = NULL;
+
+	udp_encap_enable();
+#if IS_ENABLED(CONFIG_IPV6)
+	if (local->srx.transport.family == AF_INET6)
+		udpv6_encap_enable();
+#endif
+	usk->sk_error_report = rxrpc_error_report;
 
 	/* if a local address was supplied then bind it */
 	if (local->srx.transport_len > sizeof(sa_family_t)) {

^ permalink raw reply related

* [PATCH net 00/10] rxrpc: Fix packet reception code
From: David Howells @ 2018-10-08 22:47 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, pabeni, eric.dumazet, linux-afs, linux-kernel


Here are a set of patches that prepares for and fix problems in rxrpc's
package reception code.  There serious problems are:

 (A) There's a window between binding the socket and setting the data_ready
     hook in which packets can find their way into the UDP socket's receive
     queues.

 (B) The skb_recv_udp() will return an error (and clear the error state) if
     there was an error on the Tx side.  rxrpc doesn't handle this.

 (C) The rxrpc data_ready handler doesn't fully drain the UDP receive
     queue.

 (D) The rxrpc data_ready handler assumes it is called in a non-reentrant
 state.

The second patch fixes (A) - (C); the third patch renders (B) and (C)
non-issues by using the recap_rcv hook instead of data_ready - and the
final patch fixes (D).  That last is the most complex.

The preparatory patches are:

 (1) Fix some places that are doing things in the wrong net namespace.

 (2) Stop taking the rcu read lock as it's held by the IP input routine in
     the call chain.

 (3) Only end the Tx phase if *we* rotated the final packet out of the Tx
     buffer.

 (4) Don't assume that the call state won't change after dropping the
     call_state lock.

 (5) Only take receive window and MTU suze parameters from an ACK packet if
     it's the latest ACK packet.

 (6) Record connection-level abort information correctly.

 (7) Fix a trace line.

And then there are three main patches - note that these are mixed in with
the preparatory patches somewhat:

 (1) Fix the setup window (A), skb_recv_udp() error check (B) and packet
     drainage (C).

 (2) Switch to using the encap_rcv instead of data_ready to cut out the
     effects of the UDP read queues and get the packets delivered directly.

 (3) Add more locking into the various packet input paths to defend against
     re-entrance (D).

The patches are tagged here:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	rxrpc-fixes-20181008

and can also be found on the following branch:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-fixes

David
---
David Howells (10):
      rxrpc: Fix some missed refs to init_net
      rxrpc: Fix the data_ready handler
      rxrpc: Use the UDP encap_rcv hook
      rxrpc: Don't need to take the RCU read lock in the packet receiver
      rxrpc: Don't check RXRPC_CALL_TX_LAST after calling rxrpc_rotate_tx_window()
      rxrpc: Carry call state out of locked section in rxrpc_rotate_tx_window()
      rxrpc: Only take the rwind and mtu values from latest ACK
      rxrpc: Fix connection-level abort handling
      rxrpc: Fix the rxrpc_tx_packet trace line
      rxrpc: Fix the packet reception routine


 include/trace/events/rxrpc.h |    1 
 include/uapi/linux/udp.h     |    1 
 net/rxrpc/ar-internal.h      |   23 ++--
 net/rxrpc/call_accept.c      |   27 +++-
 net/rxrpc/call_object.c      |    5 -
 net/rxrpc/conn_client.c      |   10 +-
 net/rxrpc/conn_event.c       |   26 ++--
 net/rxrpc/input.c            |  253 ++++++++++++++++++++++--------------------
 net/rxrpc/local_object.c     |   30 ++++-
 net/rxrpc/peer_event.c       |    5 +
 net/rxrpc/peer_object.c      |   29 +++--
 11 files changed, 236 insertions(+), 174 deletions(-)

^ permalink raw reply

* Re: [PATCH bpf-next v3 07/15] bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP
From: Eric Dumazet @ 2018-10-08 15:31 UTC (permalink / raw)
  To: Björn Töpel, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, john.fastabend, ast, brouer,
	willemdebruijn.kernel, daniel, mst, netdev
  Cc: Björn Töpel, michael.lundkvist, jesse.brandeburg,
	anjali.singhai, qi.z.zhang
In-Reply-To: <20180502110136.3738-8-bjorn.topel@gmail.com>



On 05/02/2018 04:01 AM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> The xskmap is yet another BPF map, very much inspired by
> dev/cpu/sockmap, and is a holder of AF_XDP sockets. A user application
> adds AF_XDP sockets into the map, and by using the bpf_redirect_map
> helper, an XDP program can redirect XDP frames to an AF_XDP socket.
> 
> Note that a socket that is bound to certain ifindex/queue index will
> *only* accept XDP frames from that netdev/queue index. If an XDP
> program tries to redirect from a netdev/queue index other than what
> the socket is bound to, the frame will not be received on the socket.
> 
> A socket can reside in multiple maps.
> 
> v3: Fixed race and simplified code.
> v2: Removed one indirection in map lookup.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
>  include/linux/bpf.h       |  25 +++++
>  include/linux/bpf_types.h |   3 +
>  include/net/xdp_sock.h    |   7 ++
>  include/uapi/linux/bpf.h  |   1 +
>  kernel/bpf/Makefile       |   3 +
>  kernel/bpf/verifier.c     |   8 +-
>  kernel/bpf/xskmap.c       | 239 ++++++++++++++++++++++++++++++++++++++++++++++
>  net/xdp/xsk.c             |   5 +
>  8 files changed, 289 insertions(+), 2 deletions(-)
>  create mode 100644 kernel/bpf/xskmap.c
> 

This function is called under rcu_read_lock() , from map_update_elem()

> +
> +static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> +			       u64 map_flags)
> +{
> +	struct xsk_map *m = container_of(map, struct xsk_map, map);
> +	u32 i = *(u32 *)key, fd = *(u32 *)value;
> +	struct xdp_sock *xs, *old_xs;
> +	struct socket *sock;
> +	int err;
> +
> +	if (unlikely(map_flags > BPF_EXIST))
> +		return -EINVAL;
> +	if (unlikely(i >= m->map.max_entries))
> +		return -E2BIG;
> +	if (unlikely(map_flags == BPF_NOEXIST))
> +		return -EEXIST;
> +
> +	sock = sockfd_lookup(fd, &err);
> +	if (!sock)
> +		return err;
> +
> +	if (sock->sk->sk_family != PF_XDP) {
> +		sockfd_put(sock);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	xs = (struct xdp_sock *)sock->sk;
> +
> +	if (!xsk_is_setup_for_bpf_map(xs)) {
> +		sockfd_put(sock);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	sock_hold(sock->sk);
> +
> +	old_xs = xchg(&m->xsk_map[i], xs);
> +	if (old_xs) {
> +		/* Make sure we've flushed everything. */

So it is illegal to call synchronize_net(), since it is a reschedule point.

> +		synchronize_net();
> +		sock_put((struct sock *)old_xs);
> +	}
> +
> +	sockfd_put(sock);
> +	return 0;
> +}
> 

^ permalink raw reply

* Re: [PATCH V2 net-next 2/5] net: Introduce a new MII time stamping interface.
From: Richard Cochran @ 2018-10-08 15:28 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Jacob Keller,
	Mark Rutland, Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20181008150722.GC6216@lunn.ch>


On Mon, Oct 08, 2018 at 05:07:22PM +0200, Andrew Lunn wrote:
> So as you said, the phylib API has not changed much, which is common
> for mature code.

I meant that phy-LINK hasn't changed much.

> But i think long term, it will become less important.
> It will share the space with phylink. And any code which wants to be
> generically usable, should not depend on phydev.

Thanks for your view of the big picture.

> Architecturally, it
> seems wrong for you to hang what should be a generic time stamping
> framework on phydev. It is not future proof. net_device is future
> proof.

You still haven't said how net_device is going to work.

Today there are exactly zero phylink devices needing time stamping
support, but there are new phylib devices.  We don't have a
net_device->phylink connection, and it isn't needed yet.  Adding that
is way out of scope for this series.

Let's stick to phylib for now.  We can cross the other bridge when we
come to it.  Maybe the net_device->phylink will emerge for purposes
other that time stamping.  Let's not guess about how it should look.

We are only talking about kernel interfaces here, and so nothing is
set in stone.

Thanks,
Richard

^ permalink raw reply

* Re: selftests/bpf: test_kmod.sh hangs on all devices
From: Shuah Khan @ 2018-10-08 15:16 UTC (permalink / raw)
  To: Naresh Kamboju, Daniel Borkmann
  Cc: open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Anders Roxell,
	Rafael Tinoco, ast, netdev, songliubraving, Willem de Bruijn
In-Reply-To: <CA+G9fYuEFofW=L0cmmAAJGaO4ZvaPYUg=h6XgjAY=PiacXM3JA@mail.gmail.com>

Hi Naresh,

Please use shuah@kernel.org for faster responses. I updated MAINTAINERS
entry a while back removing shuahkh@osg.samsung.com address due to IT
infrastructure changes at Samsung.

On 10/08/2018 08:55 AM, Naresh Kamboju wrote:
> Daniel,
> 
> On Mon, 8 Oct 2018 at 18:58, Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 10/08/2018 03:13 PM, Naresh Kamboju wrote:
>>> BPF test case test_kmod.sh hangs on all devices running linux next.
>>>
>>> + cd /opt/kselftests/default-in-kernel/bpf
>>> + ./test_kmod.sh
>>> sysctl: cannot stat /proc/sys/net/core/bpf_jit_enable: No such file or directory
>>> sysctl: cannot stat /proc/sys/net/core/bpf_jit_harden: No such file or directory
>>> sysctl: cannot stat /proc/sys/net/core/bpf_jit_enable: No such file or directory
>>> sysctl: cannot stat /proc/sys/net/core/bpf_jit_harden: No such file or directory
>>> [ JIT enabled:0 hardened:0 ]
>>>
>>> https://lkft.validation.linaro.org/scheduler/job/429726
>>>
>>> Test hangs started from 4.19.0-rc4-next-20180918.
>>> Linux version 4.19.0-rc4-next-20180918 (oe-user@oe-host) (gcc version
>>> 7.1.1 20170707 (Linaro GCC 7.1-2017.08)) #1 SMP Tue Sep 18 05:26:00
>>> UTC 2018
>>>
>>> History can be compared from this page.
>>> https://qa-reports.linaro.org/lkft/linux-next-oe/tests/kselftest/bpf_test_kmod.sh
>>>
>>> OTOH,
>>> There is a kernel BUG,
>>
>> This is quite an old linux-next kernel, should be fixed by 100811936f89 ("bpf: test_bpf:
>> add init_net to dev for flow_dissector"). Please make sure you have that commit included
>> in your testing:
> 
> I will re-validate on latest code base and let you know.
> 
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=100811936f89fd455eda1984810c09003550555b
> 
> Thanks for the quick reply.
> 

Great. Looks like this has been sorted. Thanks Daniel.

thanks,
-- Shuah

^ permalink raw reply

* Re: [PATCH] netfilter: xt_quota: Don't use aligned attribute in sizeof
From: Pablo Neira Ayuso @ 2018-10-08 22:20 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jozsef Kadlecsik, Florian Westphal, netfilter-devel, coreteam,
	netdev, linux-kernel
In-Reply-To: <20181006233331.13150-1-natechancellor@gmail.com>

On Sat, Oct 06, 2018 at 04:33:31PM -0700, Nathan Chancellor wrote:
> Clang warns:
> 
> net/netfilter/xt_quota.c:47:44: warning: 'aligned' attribute ignored
> when parsing type [-Wignored-attributes]
>         BUILD_BUG_ON(sizeof(atomic64_t) != sizeof(__aligned_u64));
>                                                   ^~~~~~~~~~~~~
> 
> Use 'sizeof(__u64)' instead, as the alignment doesn't affect the size
> of the type.

Applied, thanks.

^ permalink raw reply

* Re: [PATCH V2 net-next 2/5] net: Introduce a new MII time stamping interface.
From: Andrew Lunn @ 2018-10-08 15:07 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, devicetree, David Miller, Florian Fainelli, Jacob Keller,
	Mark Rutland, Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20181008020439.m2cytnnv5ue7yc5l@localhost>

On Sun, Oct 07, 2018 at 07:04:39PM -0700, Richard Cochran wrote:
> On Sun, Oct 07, 2018 at 09:54:00PM +0200, Andrew Lunn wrote:
> > Sure, but things have moved on since then.
> 
> I was curious about this.  Based on your uses cases, I guess that you
> mean phylib?  But not much has changed AFAICT. (There is one new
> global function and two were removed, but that doesn't change the
> picture WRT time stamping.)
> 
> Phylink now has two or three new users, one of which is dsa.  Is that
> the big move?
> 
> The situation with MACs that handle their own PHYs without phylib is
> unchanged, AFAICT.
> 
> So what exactly do you mean?

Hi Richard

We are pushing phylink. I really do think anything using > 1Gbps links
should be using phylink, not phydev. And i think we have reached the
tipping point, that most new MACs will be > 1Gbps. 2.5G or maybe 5G
will be the new default. The MAC-PHY link is quiet messy when you get
above 1G. There are a number of options which you can use, and the MAC
and PHY need to negotiate a common set to use. phylink can do this,
phylib cannot easily do it. So i see phylib slowly becoming a legacy
API for MAC drivers.

We are also slowly seeing more SFPs, and Linux controlling them. SFP
are not new, they have been in top end switches for a long time. But
they are slowly becoming more popular in industrial settings, and such
embedded industrial systems tend to let Linux control them, not
firmware. And i think industry makes more use of PTP than other
fields, but i could be wrong. Since optical SFP modules are passive, a
bump-in-the-wire time stamper actually makes sense for these.

Also, fibre on the last mile is slowly becoming more of a thing, so
maybe we will start seeing CPE, consumer routers, with SFP ports?

As i said before, we are seeing more MACs which use firmware for
controlling the PHYs. I'm not sure why yet. Maybe it is coupled with
more MACs supporting > 1G, which is messy. Or the lack of good PHY
drivers for PHYs which support > 1G? Hopefully the drivers will
improve with time.

So as you said, the phylib API has not changed much, which is common
for mature code. But i think long term, it will become less important.
It will share the space with phylink. And any code which wants to be
generically usable, should not depend on phydev. Architecturally, it
seems wrong for you to hang what should be a generic time stamping
framework on phydev. It is not future proof. net_device is future
proof.

	  Andrew

^ permalink raw reply


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