Netdev List
 help / color / mirror / Atom feed
* [PATCH 10/11] sfp: convert to fwnode
From: Russell King @ 2017-12-01 10:25 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Convert sfp-bus to use fwnode rather than device_node internally, so
we can support more than just device tree firmware.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c |  3 ++-
 drivers/net/phy/sfp-bus.c | 14 +++++++-------
 include/linux/sfp.h       |  7 ++++---
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 5a1bd48eb6d7..bca142a4863c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -499,7 +499,8 @@ static int phylink_register_sfp(struct phylink *pl, struct device_node *np)
 	if (!sfp_np)
 		return 0;
 
-	pl->sfp_bus = sfp_register_upstream(sfp_np, pl->netdev, pl,
+	pl->sfp_bus = sfp_register_upstream(of_fwnode_handle(sfp_np),
+					    pl->netdev, pl,
 					    &sfp_phylink_ops);
 	if (!pl->sfp_bus)
 		return -ENOMEM;
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index a28859022ddb..1356dba0d9d3 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -15,7 +15,7 @@ struct sfp_bus {
 	/* private: */
 	struct kref kref;
 	struct list_head node;
-	struct device_node *device_node;
+	struct fwnode_handle *fwnode;
 
 	const struct sfp_socket_ops *socket_ops;
 	struct device *sfp_dev;
@@ -260,7 +260,7 @@ static const struct sfp_upstream_ops *sfp_get_upstream_ops(struct sfp_bus *bus)
 	return bus->registered ? bus->upstream_ops : NULL;
 }
 
-static struct sfp_bus *sfp_bus_get(struct device_node *np)
+static struct sfp_bus *sfp_bus_get(struct fwnode_handle *fwnode)
 {
 	struct sfp_bus *sfp, *new, *found = NULL;
 
@@ -269,7 +269,7 @@ static struct sfp_bus *sfp_bus_get(struct device_node *np)
 	mutex_lock(&sfp_mutex);
 
 	list_for_each_entry(sfp, &sfp_buses, node) {
-		if (sfp->device_node == np) {
+		if (sfp->fwnode == fwnode) {
 			kref_get(&sfp->kref);
 			found = sfp;
 			break;
@@ -278,7 +278,7 @@ static struct sfp_bus *sfp_bus_get(struct device_node *np)
 
 	if (!found && new) {
 		kref_init(&new->kref);
-		new->device_node = np;
+		new->fwnode = fwnode;
 		list_add(&new->node, &sfp_buses);
 		found = new;
 		new = NULL;
@@ -423,11 +423,11 @@ EXPORT_SYMBOL_GPL(sfp_upstream_stop);
  *
  * On error, returns %NULL.
  */
-struct sfp_bus *sfp_register_upstream(struct device_node *np,
+struct sfp_bus *sfp_register_upstream(struct fwnode_handle *fwnode,
 				      struct net_device *ndev, void *upstream,
 				      const struct sfp_upstream_ops *ops)
 {
-	struct sfp_bus *bus = sfp_bus_get(np);
+	struct sfp_bus *bus = sfp_bus_get(fwnode);
 	int ret = 0;
 
 	if (bus) {
@@ -537,7 +537,7 @@ EXPORT_SYMBOL_GPL(sfp_module_remove);
 struct sfp_bus *sfp_register_socket(struct device *dev, struct sfp *sfp,
 				    const struct sfp_socket_ops *ops)
 {
-	struct sfp_bus *bus = sfp_bus_get(dev->of_node);
+	struct sfp_bus *bus = sfp_bus_get(dev->fwnode);
 	int ret = 0;
 
 	if (bus) {
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index b6089fe72378..47ea32d3e816 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -356,7 +356,7 @@ enum {
 	SFP_PAGE			= 0x7f,
 };
 
-struct device_node;
+struct fwnode_handle;
 struct ethtool_eeprom;
 struct ethtool_modinfo;
 struct net_device;
@@ -397,7 +397,7 @@ int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
 			  u8 *data);
 void sfp_upstream_start(struct sfp_bus *bus);
 void sfp_upstream_stop(struct sfp_bus *bus);
-struct sfp_bus *sfp_register_upstream(struct device_node *np,
+struct sfp_bus *sfp_register_upstream(struct fwnode_handle *fwnode,
 				      struct net_device *ndev, void *upstream,
 				      const struct sfp_upstream_ops *ops);
 void sfp_unregister_upstream(struct sfp_bus *bus);
@@ -441,7 +441,8 @@ static inline void sfp_upstream_stop(struct sfp_bus *bus)
 {
 }
 
-static inline struct sfp_bus *sfp_register_upstream(struct device_node *np,
+static inline struct sfp_bus *sfp_register_upstream(
+	struct fwnode_handle *fwnode,
 	struct net_device *ndev, void *upstream,
 	const struct sfp_upstream_ops *ops)
 {
-- 
2.7.4


^ permalink raw reply related

* [PATCH 09/11] sfp: fix sparse warning
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

drivers/net/phy/sfp-bus.c:298:13: warning: context imbalance in 'sfp_bus_release' - wrong count at exit

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/sfp-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 2b8c256156a2..a28859022ddb 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -291,7 +291,7 @@ static struct sfp_bus *sfp_bus_get(struct device_node *np)
 	return found;
 }
 
-static void sfp_bus_release(struct kref *kref) __releases(sfp_mutex)
+static void sfp_bus_release(struct kref *kref)
 {
 	struct sfp_bus *bus = container_of(kref, struct sfp_bus, kref);
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 08/11] sfp: add documentation for kernel APIs
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Add kernel-doc documentation for sfp kernel APIs, and link it into the
networking kapi documentation under "Network device support".

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/networking/kapi.rst |  12 +++++
 drivers/net/phy/sfp-bus.c         | 104 ++++++++++++++++++++++++++++++++++++++
 include/linux/sfp.h               |  50 +++++++++++++-----
 3 files changed, 152 insertions(+), 14 deletions(-)

diff --git a/Documentation/networking/kapi.rst b/Documentation/networking/kapi.rst
index df4141b72f24..f03ae64be8bc 100644
--- a/Documentation/networking/kapi.rst
+++ b/Documentation/networking/kapi.rst
@@ -157,3 +157,15 @@ PHYLINK
    :internal:
 
 .. kernel-doc:: drivers/net/phy/phylink.c
+
+SFP support
+-----------
+
+.. kernel-doc:: drivers/net/phy/sfp-bus.c
+   :internal:
+
+.. kernel-doc:: include/linux/sfp.h
+   :internal:
+
+.. kernel-doc:: drivers/net/phy/sfp-bus.c
+   :export:
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 8a1b1f4c1b7c..2b8c256156a2 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -8,7 +8,11 @@
 
 #include "sfp.h"
 
+/**
+ * struct sfp_bus - internal representation of a sfp bus
+ */
 struct sfp_bus {
+	/* private: */
 	struct kref kref;
 	struct list_head node;
 	struct device_node *device_node;
@@ -26,6 +30,20 @@ struct sfp_bus {
 	bool started;
 };
 
+/**
+ * sfp_parse_port() - Parse the EEPROM base ID, setting the port type
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ * @id: a pointer to the module's &struct sfp_eeprom_id
+ * @support: optional pointer to an array of unsigned long for the
+ *   ethtool support mask
+ *
+ * Parse the EEPROM identification given in @id, and return one of
+ * %PORT_TP, %PORT_FIBRE or %PORT_OTHER. If @support is non-%NULL,
+ * also set the ethtool %ETHTOOL_LINK_MODE_xxx_BIT corresponding with
+ * the connector type.
+ *
+ * If the port type is not known, returns %PORT_OTHER.
+ */
 int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		   unsigned long *support)
 {
@@ -78,6 +96,24 @@ int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 }
 EXPORT_SYMBOL_GPL(sfp_parse_port);
 
+/**
+ * sfp_parse_interface() - Parse the phy_interface_t
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ * @id: a pointer to the module's &struct sfp_eeprom_id
+ *
+ * Derive the phy_interface_t mode for the information found in the
+ * module's identifying EEPROM. There is no standard or defined way
+ * to derive this information, so we use some heuristics.
+ *
+ * If the encoding is 64b66b, then the module must be >= 10G, so
+ * return %PHY_INTERFACE_MODE_10GKR.
+ *
+ * If it's 8b10b, then it's 1G or slower. If it's definitely a fibre
+ * module, return %PHY_INTERFACE_MODE_1000BASEX mode, otherwise return
+ * %PHY_INTERFACE_MODE_SGMII mode.
+ *
+ * If the encoding is not known, return %PHY_INTERFACE_MODE_NA.
+ */
 phy_interface_t sfp_parse_interface(struct sfp_bus *bus,
 				    const struct sfp_eeprom_id *id)
 {
@@ -117,6 +153,15 @@ phy_interface_t sfp_parse_interface(struct sfp_bus *bus,
 }
 EXPORT_SYMBOL_GPL(sfp_parse_interface);
 
+/**
+ * sfp_parse_support() - Parse the eeprom id for supported link modes
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ * @id: a pointer to the module's &struct sfp_eeprom_id
+ * @support: pointer to an array of unsigned long for the ethtool support mask
+ *
+ * Parse the EEPROM identification information and derive the supported
+ * ethtool link modes for the module.
+ */
 void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		       unsigned long *support)
 {
@@ -293,6 +338,16 @@ static void sfp_unregister_bus(struct sfp_bus *bus)
 	bus->registered = false;
 }
 
+/**
+ * sfp_get_module_info() - Get the ethtool_modinfo for a SFP module
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ * @modinfo: a &struct ethtool_modinfo
+ *
+ * Fill in the type and eeprom_len parameters in @modinfo for a module on
+ * the sfp bus specified by @bus.
+ *
+ * Returns 0 on success or a negative errno number.
+ */
 int sfp_get_module_info(struct sfp_bus *bus, struct ethtool_modinfo *modinfo)
 {
 	if (!bus->registered)
@@ -301,6 +356,17 @@ int sfp_get_module_info(struct sfp_bus *bus, struct ethtool_modinfo *modinfo)
 }
 EXPORT_SYMBOL_GPL(sfp_get_module_info);
 
+/**
+ * sfp_get_module_eeprom() - Read the SFP module EEPROM
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ * @ee: a &struct ethtool_eeprom
+ * @data: buffer to contain the EEPROM data (must be at least @ee->len bytes)
+ *
+ * Read the EEPROM as specified by the supplied @ee. See the documentation
+ * for &struct ethtool_eeprom for the region to be read.
+ *
+ * Returns 0 on success or a negative errno number.
+ */
 int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
 			  u8 *data)
 {
@@ -310,6 +376,15 @@ int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
 }
 EXPORT_SYMBOL_GPL(sfp_get_module_eeprom);
 
+/**
+ * sfp_upstream_start() - Inform the SFP that the network device is up
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ *
+ * Inform the SFP socket that the network device is now up, so that the
+ * module can be enabled by allowing TX_DISABLE to be deasserted. This
+ * should be called from the network device driver's &struct net_device_ops
+ * ndo_open() method.
+ */
 void sfp_upstream_start(struct sfp_bus *bus)
 {
 	if (bus->registered)
@@ -318,6 +393,15 @@ void sfp_upstream_start(struct sfp_bus *bus)
 }
 EXPORT_SYMBOL_GPL(sfp_upstream_start);
 
+/**
+ * sfp_upstream_stop() - Inform the SFP that the network device is down
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ *
+ * Inform the SFP socket that the network device is now up, so that the
+ * module can be disabled by asserting TX_DISABLE, disabling the laser
+ * in optical modules. This should be called from the network device
+ * driver's &struct net_device_ops ndo_stop() method.
+ */
 void sfp_upstream_stop(struct sfp_bus *bus)
 {
 	if (bus->registered)
@@ -326,6 +410,19 @@ void sfp_upstream_stop(struct sfp_bus *bus)
 }
 EXPORT_SYMBOL_GPL(sfp_upstream_stop);
 
+/**
+ * sfp_register_upstream() - Register the neighbouring device
+ * @np: device node for the SFP bus
+ * @ndev: network device associated with the interface
+ * @upstream: the upstream private data
+ * @ops: the upstream's &struct sfp_upstream_ops
+ *
+ * Register the upstream device (eg, PHY) with the SFP bus. MAC drivers
+ * should use phylink, which will call this function for them. Returns
+ * a pointer to the allocated &struct sfp_bus.
+ *
+ * On error, returns %NULL.
+ */
 struct sfp_bus *sfp_register_upstream(struct device_node *np,
 				      struct net_device *ndev, void *upstream,
 				      const struct sfp_upstream_ops *ops)
@@ -353,6 +450,13 @@ struct sfp_bus *sfp_register_upstream(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(sfp_register_upstream);
 
+/**
+ * sfp_unregister_upstream() - Unregister sfp bus
+ * @bus: a pointer to the &struct sfp_bus structure for the sfp module
+ *
+ * Unregister a previously registered upstream connection for the SFP
+ * module. @bus is returned from sfp_register_upstream().
+ */
 void sfp_unregister_upstream(struct sfp_bus *bus)
 {
 	rtnl_lock();
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index 4a906f560817..b6089fe72378 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -3,7 +3,7 @@
 
 #include <linux/phy.h>
 
-struct __packed sfp_eeprom_base {
+struct sfp_eeprom_base {
 	u8 phys_id;
 	u8 phys_ext_id;
 	u8 connector;
@@ -166,12 +166,12 @@ struct __packed sfp_eeprom_base {
 	union {
 		__be16 optical_wavelength;
 		u8 cable_spec;
-	};
+	} __packed;
 	u8 reserved62;
 	u8 cc_base;
-};
+} __packed;
 
-struct __packed sfp_eeprom_ext {
+struct sfp_eeprom_ext {
 	__be16 options;
 	u8 br_max;
 	u8 br_min;
@@ -181,12 +181,21 @@ struct __packed sfp_eeprom_ext {
 	u8 enhopts;
 	u8 sff8472_compliance;
 	u8 cc_ext;
-};
-
-struct __packed sfp_eeprom_id {
+} __packed;
+
+/**
+ * struct sfp_eeprom_id - raw SFP module identification information
+ * @base: base SFP module identification structure
+ * @ext: extended SFP module identification structure
+ *
+ * See the SFF-8472 specification and related documents for the definition
+ * of these structure members. This can be obtained from
+ * ftp://ftp.seagate.com/sff
+ */
+struct sfp_eeprom_id {
 	struct sfp_eeprom_base base;
 	struct sfp_eeprom_ext ext;
-};
+} __packed;
 
 /* SFP EEPROM registers */
 enum {
@@ -353,13 +362,26 @@ struct ethtool_modinfo;
 struct net_device;
 struct sfp_bus;
 
+/**
+ * struct sfp_upstream_ops - upstream operations structure
+ * @module_insert: called after a module has been detected to determine
+ *   whether the module is supported for the upstream device.
+ * @module_remove: called after the module has been removed.
+ * @link_down: called when the link is non-operational for whatever
+ *   reason.
+ * @link_up: called when the link is operational.
+ * @connect_phy: called when an I2C accessible PHY has been detected
+ *   on the module.
+ * @disconnect_phy: called when a module with an I2C accessible PHY has
+ *   been removed.
+ */
 struct sfp_upstream_ops {
-	int (*module_insert)(void *, const struct sfp_eeprom_id *id);
-	void (*module_remove)(void *);
-	void (*link_down)(void *);
-	void (*link_up)(void *);
-	int (*connect_phy)(void *, struct phy_device *);
-	void (*disconnect_phy)(void *);
+	int (*module_insert)(void *priv, const struct sfp_eeprom_id *id);
+	void (*module_remove)(void *priv);
+	void (*link_down)(void *priv);
+	void (*link_up)(void *priv);
+	int (*connect_phy)(void *priv, struct phy_device *);
+	void (*disconnect_phy)(void *priv);
 };
 
 #if IS_ENABLED(CONFIG_SFP)
-- 
2.7.4


^ permalink raw reply related

* [PATCH 06/11] phylink: restart 802.3z negotiation when starting net device
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Restart 802.3z negotiation when the net device is brought up to ensure
that the link partner has our current link modes.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 560486463930..40563c8d7352 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -751,6 +751,12 @@ void phylink_start(struct phylink *pl)
 	phylink_resolve_flow(pl, &pl->link_config);
 	phylink_mac_config(pl, &pl->link_config);
 
+	/* Restart autonegotiation if using 802.3z to ensure that the link
+	 * parameters are properly negotiated.  This is necessary for DSA
+	 * switches using 802.3z negotiation to ensure they see our modes.
+	 */
+	phylink_mac_an_restart(pl);
+
 	clear_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state);
 	phylink_run_resolve(pl);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 07/11] phylink: add documentation for kernel APIs
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Add kernel-doc documentation for phylink kernel APIs, and link it into
the networking kapi documentation under "Network device support".

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 Documentation/networking/kapi.rst |  12 +++
 drivers/net/phy/phylink.c         | 182 +++++++++++++++++++++++++++++++++++++
 include/linux/phylink.h           | 183 ++++++++++++++++++++++++++++----------
 3 files changed, 329 insertions(+), 48 deletions(-)

diff --git a/Documentation/networking/kapi.rst b/Documentation/networking/kapi.rst
index 580289f345da..df4141b72f24 100644
--- a/Documentation/networking/kapi.rst
+++ b/Documentation/networking/kapi.rst
@@ -145,3 +145,15 @@ PHY Support
 
 .. kernel-doc:: drivers/net/phy/mdio_bus.c
    :internal:
+
+PHYLINK
+-------
+
+  PHYLINK interfaces traditional network drivers with PHYLIB, fixed-links,
+  and SFF modules (eg, hot-pluggable SFP) that may contain PHYs.  PHYLINK
+  provides management of the link state and link modes.
+
+.. kernel-doc:: include/linux/phylink.h
+   :internal:
+
+.. kernel-doc:: drivers/net/phy/phylink.c
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 40563c8d7352..5a1bd48eb6d7 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -36,7 +36,11 @@ enum {
 	PHYLINK_DISABLE_LINK,
 };
 
+/**
+ * struct phylink - internal data type for phylink
+ */
 struct phylink {
+	/* private: */
 	struct net_device *netdev;
 	const struct phylink_mac_ops *ops;
 
@@ -87,6 +91,13 @@ static inline bool linkmode_empty(const unsigned long *src)
 	return bitmap_empty(src, __ETHTOOL_LINK_MODE_MASK_NBITS);
 }
 
+/**
+ * phylink_set_port_modes() - set the port type modes in the ethtool mask
+ * @mask: ethtool link mode mask
+ *
+ * Sets all the port type modes in the ethtool mask.  MAC drivers should
+ * use this in their 'validate' callback.
+ */
 void phylink_set_port_modes(unsigned long *mask)
 {
 	phylink_set(mask, TP);
@@ -496,6 +507,19 @@ static int phylink_register_sfp(struct phylink *pl, struct device_node *np)
 	return 0;
 }
 
+/**
+ * phylink_create() - create a phylink instance
+ * @ndev: a pointer to the &struct net_device
+ * @np: a pointer to a &struct device_node describing the network interface
+ * @iface: the desired link mode defined by &typedef phy_interface_t
+ * @ops: a pointer to a &struct phylink_mac_ops for the MAC.
+ *
+ * Create a new phylink instance, and parse the link parameters found in @np.
+ * This will parse in-band modes, fixed-link or SFP configuration.
+ *
+ * Returns a pointer to a &struct phylink, or an error-pointer value. Users
+ * must use IS_ERR() to check for errors from this function.
+ */
 struct phylink *phylink_create(struct net_device *ndev, struct device_node *np,
 			       phy_interface_t iface,
 			       const struct phylink_mac_ops *ops)
@@ -548,6 +572,13 @@ struct phylink *phylink_create(struct net_device *ndev, struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(phylink_create);
 
+/**
+ * phylink_destroy() - cleanup and destroy the phylink instance
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * Destroy a phylink instance. Any PHY that has been attached must have been
+ * cleaned up via phylink_disconnect_phy() prior to calling this function.
+ */
 void phylink_destroy(struct phylink *pl)
 {
 	if (pl->sfp_bus)
@@ -644,6 +675,21 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy)
 	return 0;
 }
 
+/**
+ * phylink_connect_phy() - connect a PHY to the phylink instance
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @phy: a pointer to a &struct phy_device.
+ *
+ * Connect @phy to the phylink instance specified by @pl by calling
+ * phy_attach_direct(). Configure the @phy according to the MAC driver's
+ * capabilities, start the PHYLIB state machine and enable any interrupts
+ * that the PHY supports.
+ *
+ * This updates the phylink's ethtool supported and advertising link mode
+ * masks.
+ *
+ * Returns 0 on success or a negative errno.
+ */
 int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 {
 	int ret;
@@ -665,6 +711,17 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 }
 EXPORT_SYMBOL_GPL(phylink_connect_phy);
 
+/**
+ * phylink_of_phy_connect() - connect the PHY specified in the DT mode.
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @dn: a pointer to a &struct device_node.
+ *
+ * Connect the phy specified in the device node @dn to the phylink instance
+ * specified by @pl. Actions specified in phylink_connect_phy() will be
+ * performed.
+ *
+ * Returns 0 on success or a negative errno.
+ */
 int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn)
 {
 	struct device_node *phy_node;
@@ -706,6 +763,13 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn)
 }
 EXPORT_SYMBOL_GPL(phylink_of_phy_connect);
 
+/**
+ * phylink_disconnect_phy() - disconnect any PHY attached to the phylink
+ *   instance.
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * Disconnect any current PHY from the phylink instance described by @pl.
+ */
 void phylink_disconnect_phy(struct phylink *pl)
 {
 	struct phy_device *phy;
@@ -727,6 +791,14 @@ void phylink_disconnect_phy(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_disconnect_phy);
 
+/**
+ * phylink_mac_change() - notify phylink of a change in MAC state
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @up: indicates whether the link is currently up.
+ *
+ * The MAC driver should call this driver when the state of its link
+ * changes (eg, link failure, new negotiation results, etc.)
+ */
 void phylink_mac_change(struct phylink *pl, bool up)
 {
 	if (!up)
@@ -736,6 +808,14 @@ void phylink_mac_change(struct phylink *pl, bool up)
 }
 EXPORT_SYMBOL_GPL(phylink_mac_change);
 
+/**
+ * phylink_start() - start a phylink instance
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * Start the phylink instance specified by @pl, configuring the MAC for the
+ * desired link mode(s) and negotiation style. This should be called from the
+ * network device driver's &struct net_device_ops ndo_open() method.
+ */
 void phylink_start(struct phylink *pl)
 {
 	WARN_ON(!lockdep_rtnl_is_held());
@@ -767,6 +847,15 @@ void phylink_start(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_start);
 
+/**
+ * phylink_stop() - stop a phylink instance
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * Stop the phylink instance specified by @pl. This should be called from the
+ * network device driver's &struct net_device_ops ndo_stop() method.  The
+ * network device's carrier state should not be changed prior to calling this
+ * function.
+ */
 void phylink_stop(struct phylink *pl)
 {
 	WARN_ON(!lockdep_rtnl_is_held());
@@ -782,6 +871,15 @@ void phylink_stop(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_stop);
 
+/**
+ * phylink_ethtool_get_wol() - get the wake on lan parameters for the PHY
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @wol: a pointer to &struct ethtool_wolinfo to hold the read parameters
+ *
+ * Read the wake on lan parameters from the PHY attached to the phylink
+ * instance specified by @pl. If no PHY is currently attached, report no
+ * support for wake on lan.
+ */
 void phylink_ethtool_get_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 {
 	WARN_ON(!lockdep_rtnl_is_held());
@@ -794,6 +892,17 @@ void phylink_ethtool_get_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_wol);
 
+/**
+ * phylink_ethtool_set_wol() - set wake on lan parameters
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @wol: a pointer to &struct ethtool_wolinfo for the desired parameters
+ *
+ * Set the wake on lan parameters for the PHY attached to the phylink
+ * instance specified by @pl. If no PHY is attached, returns %EOPNOTSUPP
+ * error.
+ *
+ * Returns zero on success or negative errno code.
+ */
 int phylink_ethtool_set_wol(struct phylink *pl, struct ethtool_wolinfo *wol)
 {
 	int ret = -EOPNOTSUPP;
@@ -829,6 +938,15 @@ static void phylink_get_ksettings(const struct phylink_link_state *state,
 				AUTONEG_DISABLE;
 }
 
+/**
+ * phylink_ethtool_ksettings_get() - get the current link settings
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @kset: a pointer to a &struct ethtool_link_ksettings to hold link settings
+ *
+ * Read the current link settings for the phylink instance specified by @pl.
+ * This will be the link settings read from the MAC, PHY or fixed link
+ * settings depending on the current negotiation mode.
+ */
 int phylink_ethtool_ksettings_get(struct phylink *pl,
 				  struct ethtool_link_ksettings *kset)
 {
@@ -875,6 +993,11 @@ int phylink_ethtool_ksettings_get(struct phylink *pl,
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_ksettings_get);
 
+/**
+ * phylink_ethtool_ksettings_set() - set the link settings
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @kset: a pointer to a &struct ethtool_link_ksettings for the desired modes
+ */
 int phylink_ethtool_ksettings_set(struct phylink *pl,
 				  const struct ethtool_link_ksettings *kset)
 {
@@ -968,6 +1091,17 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_ksettings_set);
 
+/**
+ * phylink_ethtool_nway_reset() - restart negotiation
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * Restart negotiation for the phylink instance specified by @pl. This will
+ * cause any attached phy to restart negotiation with the link partner, and
+ * if the MAC is in a BaseX mode, the MAC will also be requested to restart
+ * negotiation.
+ *
+ * Returns zero on success, or negative error code.
+ */
 int phylink_ethtool_nway_reset(struct phylink *pl)
 {
 	int ret = 0;
@@ -982,6 +1116,11 @@ int phylink_ethtool_nway_reset(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_nway_reset);
 
+/**
+ * phylink_ethtool_get_pauseparam() - get the current pause parameters
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @pause: a pointer to a &struct ethtool_pauseparam
+ */
 void phylink_ethtool_get_pauseparam(struct phylink *pl,
 				    struct ethtool_pauseparam *pause)
 {
@@ -993,6 +1132,11 @@ void phylink_ethtool_get_pauseparam(struct phylink *pl,
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_pauseparam);
 
+/**
+ * phylink_ethtool_set_pauseparam() - set the current pause parameters
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @pause: a pointer to a &struct ethtool_pauseparam
+ */
 int phylink_ethtool_set_pauseparam(struct phylink *pl,
 				   struct ethtool_pauseparam *pause)
 {
@@ -1070,6 +1214,16 @@ int phylink_ethtool_get_module_eeprom(struct phylink *pl,
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_module_eeprom);
 
+/**
+ * phylink_ethtool_get_eee_err() - read the energy efficient ethernet error
+ *   counter
+ * @pl: a pointer to a &struct phylink returned from phylink_create().
+ *
+ * Read the Energy Efficient Ethernet error counter from the PHY associated
+ * with the phylink instance specified by @pl.
+ *
+ * Returns positive error counter value, or negative error code.
+ */
 int phylink_get_eee_err(struct phylink *pl)
 {
 	int ret = 0;
@@ -1083,6 +1237,11 @@ int phylink_get_eee_err(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_get_eee_err);
 
+/**
+ * phylink_ethtool_get_eee() - read the energy efficient ethernet parameters
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @eee: a pointer to a &struct ethtool_eee for the read parameters
+ */
 int phylink_ethtool_get_eee(struct phylink *pl, struct ethtool_eee *eee)
 {
 	int ret = -EOPNOTSUPP;
@@ -1096,6 +1255,11 @@ int phylink_ethtool_get_eee(struct phylink *pl, struct ethtool_eee *eee)
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_eee);
 
+/**
+ * phylink_ethtool_set_eee() - set the energy efficient ethernet parameters
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @eee: a pointer to a &struct ethtool_eee for the desired parameters
+ */
 int phylink_ethtool_set_eee(struct phylink *pl, struct ethtool_eee *eee)
 {
 	int ret = -EOPNOTSUPP;
@@ -1267,6 +1431,24 @@ static int phylink_mii_write(struct phylink *pl, unsigned int phy_id,
 	return 0;
 }
 
+/**
+ * phylink_mii_ioctl() - generic mii ioctl interface
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ * @ifr: a pointer to a &struct ifreq for socket ioctls
+ * @cmd: ioctl cmd to execute
+ *
+ * Perform the specified MII ioctl on the PHY attached to the phylink instance
+ * specified by @pl. If no PHY is attached, emulate the presence of the PHY.
+ *
+ * Returns: zero on success or negative error code.
+ *
+ * %SIOCGMIIPHY:
+ *  read register from the current PHY.
+ * %SIOCGMIIREG:
+ *  read register from the specified PHY.
+ * %SIOCSMIIREG:
+ *  set a register on the specified PHY.
+ */
 int phylink_mii_ioctl(struct phylink *pl, struct ifreq *ifr, int cmd)
 {
 	struct mii_ioctl_data *mii = if_mii(ifr);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 4d0f42da9078..30e9d0070377 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -28,10 +28,23 @@ static inline bool phylink_autoneg_inband(unsigned int mode)
 	return mode == MLO_AN_INBAND;
 }
 
+/**
+ * struct phylink_link_state - link state structure
+ * @advertising: ethtool bitmask containing advertised link modes
+ * @lp_advertising: ethtool bitmask containing link partner advertised link
+ *   modes
+ * @interface: link &typedef phy_interface_t mode
+ * @speed: link speed, one of the SPEED_* constants.
+ * @duplex: link duplex mode, one of DUPLEX_* constants.
+ * @pause: link pause state, described by MLO_PAUSE_* constants.
+ * @link: true if the link is up.
+ * @an_enabled: true if autonegotiation is enabled/desired.
+ * @an_complete: true if autonegotiation has completed.
+ */
 struct phylink_link_state {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
-	phy_interface_t interface;	/* PHY_INTERFACE_xxx */
+	phy_interface_t interface;
 	int speed;
 	int duplex;
 	int pause;
@@ -40,61 +53,135 @@ struct phylink_link_state {
 	unsigned int an_complete:1;
 };
 
+/**
+ * struct phylink_mac_ops - MAC operations structure.
+ * @validate: Validate and update the link configuration.
+ * @mac_link_state: Read the current link state from the hardware.
+ * @mac_config: configure the MAC for the selected mode and state.
+ * @mac_an_restart: restart 802.3z BaseX autonegotiation.
+ * @mac_link_down: take the link down.
+ * @mac_link_up: allow the link to come up.
+ *
+ * The individual methods are described more fully below.
+ */
 struct phylink_mac_ops {
-	/**
-	 * validate: validate and update the link configuration
-	 * @ndev: net_device structure associated with MAC
-	 * @config: configuration to validate
-	 *
-	 * Update the %config->supported and %config->advertised masks
-	 * clearing bits that can not be supported.
-	 *
-	 * Note: the PHY may be able to transform from one connection
-	 * technology to another, so, eg, don't clear 1000BaseX just
-	 * because the MAC is unable to support it.  This is more about
-	 * clearing unsupported speeds and duplex settings.
-	 *
-	 * If the %config->interface mode is %PHY_INTERFACE_MODE_1000BASEX
-	 * or %PHY_INTERFACE_MODE_2500BASEX, select the appropriate mode
-	 * based on %config->advertised and/or %config->speed.
-	 */
 	void (*validate)(struct net_device *ndev, unsigned long *supported,
 			 struct phylink_link_state *state);
-
-	/* Read the current link state from the hardware */
-	int (*mac_link_state)(struct net_device *, struct phylink_link_state *);
-
-	/* Configure the MAC */
-	/**
-	 * mac_config: configure the MAC for the selected mode and state
-	 * @ndev: net_device structure for the MAC
-	 * @mode: one of MLO_AN_FIXED, MLO_AN_PHY, MLO_AN_INBAND
-	 * @state: state structure
-	 *
-	 * The action performed depends on the currently selected mode:
-	 *
-	 * %MLO_AN_FIXED, %MLO_AN_PHY:
-	 *   set the specified speed, duplex, pause mode, and phy interface
-	 *   mode in the provided @state.
-	 * %MLO_AN_INBAND:
-	 *   place the link in an inband negotiation mode (such as
-	 *   1000base-X or Cisco SGMII mode depending on the phy interface
-	 *   mode), advertising the parameters given in advertising in @state.
-	 */
+	int (*mac_link_state)(struct net_device *ndev,
+			      struct phylink_link_state *state);
 	void (*mac_config)(struct net_device *ndev, unsigned int mode,
 			   const struct phylink_link_state *state);
-
-	/**
-	 * mac_an_restart: restart 802.3z BaseX autonegotiation
-	 * @ndev: net_device structure for the MAC
-	 */
 	void (*mac_an_restart)(struct net_device *ndev);
-
-	void (*mac_link_down)(struct net_device *, unsigned int mode);
-	void (*mac_link_up)(struct net_device *, unsigned int mode,
-			    struct phy_device *);
+	void (*mac_link_down)(struct net_device *ndev, unsigned int mode);
+	void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
+			    struct phy_device *phy);
 };
 
+#if 0 /* For kernel-doc purposes only. */
+/**
+ * validate - Validate and update the link configuration
+ * @ndev: a pointer to a &struct net_device for the MAC.
+ * @supported: ethtool bitmask for supported link modes.
+ * @state: a pointer to a &struct phylink_link_state.
+ *
+ * Clear bits in the @supported and @state->advertising masks that
+ * are not supportable by the MAC.
+ *
+ * Note that the PHY may be able to transform from one connection
+ * technology to another, so, eg, don't clear 1000BaseX just
+ * because the MAC is unable to BaseX mode. This is more about
+ * clearing unsupported speeds and duplex settings.
+ *
+ * If the @state->interface mode is %PHY_INTERFACE_MODE_1000BASEX
+ * or %PHY_INTERFACE_MODE_2500BASEX, select the appropriate mode
+ * based on @state->advertising and/or @state->speed and update
+ * @state->interface accordingly.
+ */
+void validate(struct net_device *ndev, unsigned long *supported,
+	      struct phylink_link_state *state);
+
+/**
+ * mac_link_state() - Read the current link state from the hardware
+ * @ndev: a pointer to a &struct net_device for the MAC.
+ * @state: a pointer to a &struct phylink_link_state.
+ *
+ * Read the current link state from the MAC, reporting the current
+ * speed in @state->speed, duplex mode in @state->duplex, pause mode
+ * in @state->pause using the %MLO_PAUSE_RX and %MLO_PAUSE_TX bits,
+ * negotiation completion state in @state->an_complete, and link
+ * up state in @state->link.
+ */
+int mac_link_state(struct net_device *ndev,
+		   struct phylink_link_state *state);
+
+/**
+ * mac_config() - configure the MAC for the selected mode and state
+ * @ndev: a pointer to a &struct net_device for the MAC.
+ * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
+ * @state: a pointer to a &struct phylink_link_state.
+ *
+ * The action performed depends on the currently selected mode:
+ *
+ * %MLO_AN_FIXED, %MLO_AN_PHY:
+ *   Configure the specified @state->speed, @state->duplex and
+ *   @state->pause (%MLO_PAUSE_TX / %MLO_PAUSE_RX) mode.
+ *
+ * %MLO_AN_INBAND:
+ *   place the link in an inband negotiation mode (such as 802.3z
+ *   1000base-X or Cisco SGMII mode depending on the @state->interface
+ *   mode). In both cases, link state management (whether the link
+ *   is up or not) is performed by the MAC, and reported via the
+ *   mac_link_state() callback. Changes in link state must be made
+ *   by calling phylink_mac_change().
+ *
+ *   If in 802.3z mode, the link speed is fixed, dependent on the
+ *   @state->interface. Duplex is negotiated, and pause is advertised
+ *   according to @state->an_enabled, @state->pause and
+ *   @state->advertising flags. Beware of MACs which only support full
+ *   duplex at gigabit and higher speeds.
+ *
+ *   If in Cisco SGMII mode, the link speed and duplex mode are passed
+ *   in the serial bitstream 16-bit configuration word, and the MAC
+ *   should be configured to read these bits and acknowledge the
+ *   configuration word. Nothing is advertised by the MAC. The MAC is
+ *   responsible for reading the configuration word and configuring
+ *   itself accordingly.
+ */
+void mac_config(struct net_device *ndev, unsigned int mode,
+		const struct phylink_link_state *state);
+
+/**
+ * mac_an_restart() - restart 802.3z BaseX autonegotiation
+ * @ndev: a pointer to a &struct net_device for the MAC.
+ */
+void mac_an_restart(struct net_device *ndev);
+
+/**
+ * mac_link_down() - take the link down
+ * @ndev: a pointer to a &struct net_device for the MAC.
+ * @mode: link autonegotiation mode
+ *
+ * If @mode is not an in-band negotiation mode (as defined by
+ * phylink_autoneg_inband()), force the link down and disable any
+ * Energy Efficient Ethernet MAC configuration.
+ */
+void mac_link_down(struct net_device *ndev, unsigned int mode);
+
+/**
+ * mac_link_up() - allow the link to come up
+ * @ndev: a pointer to a &struct net_device for the MAC.
+ * @mode: link autonegotiation mode
+ * @phy: any attached phy
+ *
+ * If @mode is not an in-band negotiation mode (as defined by
+ * phylink_autoneg_inband()), allow the link to come up. If @phy
+ * is non-%NULL, configure Energy Efficient Ethernet by calling
+ * phy_init_eee() and perform appropriate MAC configuration for EEE.
+ */
+void mac_link_up(struct net_device *ndev, unsigned int mode,
+		 struct phy_device *phy);
+#endif
+
 struct phylink *phylink_create(struct net_device *, struct device_node *,
 	phy_interface_t iface, const struct phylink_mac_ops *ops);
 void phylink_destroy(struct phylink *);
-- 
2.7.4


^ permalink raw reply related

* [PATCH 05/11] phylink: remove phylink_init_eee()
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

phylink_init_eee() serves no purpose, remove it.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 13 -------------
 include/linux/phylink.h   |  1 -
 2 files changed, 14 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 06c6ad0603ac..560486463930 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1064,19 +1064,6 @@ int phylink_ethtool_get_module_eeprom(struct phylink *pl,
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_get_module_eeprom);
 
-int phylink_init_eee(struct phylink *pl, bool clk_stop_enable)
-{
-	int ret = -EPROTONOSUPPORT;
-
-	WARN_ON(!lockdep_rtnl_is_held());
-
-	if (pl->phydev)
-		ret = phy_init_eee(pl->phydev, clk_stop_enable);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(phylink_init_eee);
-
 int phylink_get_eee_err(struct phylink *pl)
 {
 	int ret = 0;
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index cab22ad3bd6f..4d0f42da9078 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -123,7 +123,6 @@ int phylink_ethtool_set_pauseparam(struct phylink *,
 int phylink_ethtool_get_module_info(struct phylink *, struct ethtool_modinfo *);
 int phylink_ethtool_get_module_eeprom(struct phylink *,
 				      struct ethtool_eeprom *, u8 *);
-int phylink_init_eee(struct phylink *, bool);
 int phylink_get_eee_err(struct phylink *);
 int phylink_ethtool_get_eee(struct phylink *, struct ethtool_eee *);
 int phylink_ethtool_set_eee(struct phylink *, struct ethtool_eee *);
-- 
2.7.4


^ permalink raw reply related

* [PATCH 03/11] phylink: get rid of separate Cisco SGMII and 802.3z modes
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Since the handling of SGMII and 802.3z is now the same, combine the
MLO_AN_xxx constants into a single MLO_AN_INBAND, and use the PHY
interface mode to distinguish between Cisco SGMII and 802.3z.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 39 ++++++++++++++-------------------------
 include/linux/phylink.h   | 19 +++++++------------
 2 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index d7006fdc8f71..34ecdb892f57 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -117,8 +117,7 @@ static const char *phylink_an_mode_str(unsigned int mode)
 	static const char *modestr[] = {
 		[MLO_AN_PHY] = "phy",
 		[MLO_AN_FIXED] = "fixed",
-		[MLO_AN_SGMII] = "SGMII",
-		[MLO_AN_8023Z] = "802.3z",
+		[MLO_AN_INBAND] = "inband",
 	};
 
 	return mode < ARRAY_SIZE(modestr) ? modestr[mode] : "unknown";
@@ -244,6 +243,7 @@ static int phylink_parse_mode(struct phylink *pl, struct device_node *np)
 		phylink_set(pl->supported, Asym_Pause);
 		phylink_set(pl->supported, Pause);
 		pl->link_config.an_enabled = true;
+		pl->link_an_mode = MLO_AN_INBAND;
 
 		switch (pl->link_config.interface) {
 		case PHY_INTERFACE_MODE_SGMII:
@@ -253,17 +253,14 @@ static int phylink_parse_mode(struct phylink *pl, struct device_node *np)
 			phylink_set(pl->supported, 100baseT_Full);
 			phylink_set(pl->supported, 1000baseT_Half);
 			phylink_set(pl->supported, 1000baseT_Full);
-			pl->link_an_mode = MLO_AN_SGMII;
 			break;
 
 		case PHY_INTERFACE_MODE_1000BASEX:
 			phylink_set(pl->supported, 1000baseX_Full);
-			pl->link_an_mode = MLO_AN_8023Z;
 			break;
 
 		case PHY_INTERFACE_MODE_2500BASEX:
 			phylink_set(pl->supported, 2500baseX_Full);
-			pl->link_an_mode = MLO_AN_8023Z;
 			break;
 
 		case PHY_INTERFACE_MODE_10GKR:
@@ -280,7 +277,6 @@ static int phylink_parse_mode(struct phylink *pl, struct device_node *np)
 			phylink_set(pl->supported, 10000baseLR_Full);
 			phylink_set(pl->supported, 10000baseLRM_Full);
 			phylink_set(pl->supported, 10000baseER_Full);
-			pl->link_an_mode = MLO_AN_SGMII;
 			break;
 
 		default:
@@ -422,8 +418,7 @@ static void phylink_resolve(struct work_struct *w)
 			phylink_mac_config(pl, &link_state);
 			break;
 
-		case MLO_AN_SGMII:
-		case MLO_AN_8023Z:
+		case MLO_AN_INBAND:
 			phylink_get_mac_state(pl, &link_state);
 			if (pl->phydev) {
 				bool changed = false;
@@ -654,7 +649,8 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 	int ret;
 
 	if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
-		    pl->link_an_mode == MLO_AN_8023Z))
+		    (pl->link_an_mode == MLO_AN_INBAND &&
+		     phy_interface_mode_is_8023z(pl->link_interface))))
 		return -EINVAL;
 
 	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
@@ -677,7 +673,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn)
 
 	/* Fixed links and 802.3z are handled without needing a PHY */
 	if (pl->link_an_mode == MLO_AN_FIXED ||
-	    pl->link_an_mode == MLO_AN_8023Z)
+	    (pl->link_an_mode == MLO_AN_INBAND &&
+	     phy_interface_mode_is_8023z(pl->link_interface)))
 		return 0;
 
 	phy_node = of_parse_phandle(dn, "phy-handle", 0);
@@ -851,8 +848,7 @@ int phylink_ethtool_ksettings_get(struct phylink *pl,
 		phylink_get_ksettings(&link_state, kset);
 		break;
 
-	case MLO_AN_SGMII:
-	case MLO_AN_8023Z:
+	case MLO_AN_INBAND:
 		/* If there is a phy attached, then use the reported
 		 * settings from the phy with no modification.
 		 */
@@ -1029,8 +1025,7 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl,
 			phylink_mac_config(pl, config);
 			break;
 
-		case MLO_AN_SGMII:
-		case MLO_AN_8023Z:
+		case MLO_AN_INBAND:
 			phylink_mac_config(pl, config);
 			phylink_mac_an_restart(pl);
 			break;
@@ -1247,9 +1242,7 @@ static int phylink_mii_read(struct phylink *pl, unsigned int phy_id,
 	case MLO_AN_PHY:
 		return -EOPNOTSUPP;
 
-	case MLO_AN_SGMII:
-		/* No phy, fall through to 8023z method */
-	case MLO_AN_8023Z:
+	case MLO_AN_INBAND:
 		if (phy_id == 0) {
 			val = phylink_get_mac_state(pl, &state);
 			if (val < 0)
@@ -1274,9 +1267,7 @@ static int phylink_mii_write(struct phylink *pl, unsigned int phy_id,
 	case MLO_AN_PHY:
 		return -EOPNOTSUPP;
 
-	case MLO_AN_SGMII:
-		/* No phy, fall through to 8023z method */
-	case MLO_AN_8023Z:
+	case MLO_AN_INBAND:
 		break;
 	}
 
@@ -1291,7 +1282,7 @@ int phylink_mii_ioctl(struct phylink *pl, struct ifreq *ifr, int cmd)
 	WARN_ON(!lockdep_rtnl_is_held());
 
 	if (pl->phydev) {
-		/* PHYs only exist for MLO_AN_PHY and MLO_AN_SGMII */
+		/* PHYs only exist for MLO_AN_PHY and SGMII */
 		switch (cmd) {
 		case SIOCGMIIPHY:
 			mii->phy_id = pl->phydev->mdio.addr;
@@ -1360,10 +1351,8 @@ static int phylink_sfp_module_insert(void *upstream,
 
 	switch (iface) {
 	case PHY_INTERFACE_MODE_SGMII:
-		mode = MLO_AN_SGMII;
-		break;
 	case PHY_INTERFACE_MODE_1000BASEX:
-		mode = MLO_AN_8023Z;
+		mode = MLO_AN_INBAND;
 		break;
 	default:
 		return -EINVAL;
@@ -1390,7 +1379,7 @@ static int phylink_sfp_module_insert(void *upstream,
 		   phylink_an_mode_str(mode), phy_modes(config.interface),
 		   __ETHTOOL_LINK_MODE_MASK_NBITS, support);
 
-	if (mode == MLO_AN_8023Z && pl->phydev)
+	if (phy_interface_mode_is_8023z(iface) && pl->phydev)
 		return -EINVAL;
 
 	changed = !bitmap_equal(pl->supported, support,
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index af67edd4ae38..cab22ad3bd6f 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -20,13 +20,12 @@ enum {
 
 	MLO_AN_PHY = 0,	/* Conventional PHY */
 	MLO_AN_FIXED,	/* Fixed-link mode */
-	MLO_AN_SGMII,	/* Cisco SGMII protocol */
-	MLO_AN_8023Z,	/* 1000base-X protocol */
+	MLO_AN_INBAND,	/* In-band protocol */
 };
 
 static inline bool phylink_autoneg_inband(unsigned int mode)
 {
-	return mode == MLO_AN_SGMII || mode == MLO_AN_8023Z;
+	return mode == MLO_AN_INBAND;
 }
 
 struct phylink_link_state {
@@ -69,7 +68,7 @@ struct phylink_mac_ops {
 	/**
 	 * mac_config: configure the MAC for the selected mode and state
 	 * @ndev: net_device structure for the MAC
-	 * @mode: one of MLO_AN_FIXED, MLO_AN_PHY, MLO_AN_8023Z, MLO_AN_SGMII
+	 * @mode: one of MLO_AN_FIXED, MLO_AN_PHY, MLO_AN_INBAND
 	 * @state: state structure
 	 *
 	 * The action performed depends on the currently selected mode:
@@ -77,14 +76,10 @@ struct phylink_mac_ops {
 	 * %MLO_AN_FIXED, %MLO_AN_PHY:
 	 *   set the specified speed, duplex, pause mode, and phy interface
 	 *   mode in the provided @state.
-	 * %MLO_AN_8023Z:
-	 *   place the link in 1000base-X mode, advertising the parameters
-	 *   given in advertising in @state.
-	 * %MLO_AN_SGMII:
-	 *   place the link in Cisco SGMII mode - there is no advertisment
-	 *   to make as the PHY communicates the speed and duplex to the
-	 *   MAC over the in-band control word.  Configuration of the pause
-	 *   mode is as per MLO_AN_PHY since this is not included.
+	 * %MLO_AN_INBAND:
+	 *   place the link in an inband negotiation mode (such as
+	 *   1000base-X or Cisco SGMII mode depending on the phy interface
+	 *   mode), advertising the parameters given in advertising in @state.
 	 */
 	void (*mac_config)(struct net_device *ndev, unsigned int mode,
 			   const struct phylink_link_state *state);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 04/11] phylink: add support for 2500baseX and 10GbaseKR
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Add support for handling the faster 2.5G and 10G link modes when used
with SFP modules.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 34ecdb892f57..06c6ad0603ac 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1352,6 +1352,8 @@ static int phylink_sfp_module_insert(void *upstream,
 	switch (iface) {
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_10GKR:
 		mode = MLO_AN_INBAND;
 		break;
 	default:
-- 
2.7.4


^ permalink raw reply related

* [PATCH 02/11] phylink: merge SGMII and 802.3z handling
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

The code handling SGMII and 802.3z is essentially the same, except that
we assume 802.3z has no PHY.  Re-organise the code such that these cases
are merged, and exclude 802.3z mode from having a PHY attached.  This
results in the same link handling behaviour as before.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a02d67689d6e..d7006fdc8f71 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -423,6 +423,7 @@ static void phylink_resolve(struct work_struct *w)
 			break;
 
 		case MLO_AN_SGMII:
+		case MLO_AN_8023Z:
 			phylink_get_mac_state(pl, &link_state);
 			if (pl->phydev) {
 				bool changed = false;
@@ -448,10 +449,6 @@ static void phylink_resolve(struct work_struct *w)
 				}
 			}
 			break;
-
-		case MLO_AN_8023Z:
-			phylink_get_mac_state(pl, &link_state);
-			break;
 		}
 	}
 
@@ -656,6 +653,10 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 {
 	int ret;
 
+	if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
+		    pl->link_an_mode == MLO_AN_8023Z))
+		return -EINVAL;
+
 	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
 	if (ret)
 		return ret;
@@ -674,8 +675,9 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn)
 	struct phy_device *phy_dev;
 	int ret;
 
-	/* Fixed links are handled without needing a PHY */
-	if (pl->link_an_mode == MLO_AN_FIXED)
+	/* Fixed links and 802.3z are handled without needing a PHY */
+	if (pl->link_an_mode == MLO_AN_FIXED ||
+	    pl->link_an_mode == MLO_AN_8023Z)
 		return 0;
 
 	phy_node = of_parse_phandle(dn, "phy-handle", 0);
@@ -850,13 +852,13 @@ int phylink_ethtool_ksettings_get(struct phylink *pl,
 		break;
 
 	case MLO_AN_SGMII:
+	case MLO_AN_8023Z:
 		/* If there is a phy attached, then use the reported
 		 * settings from the phy with no modification.
 		 */
 		if (pl->phydev)
 			break;
 
-	case MLO_AN_8023Z:
 		phylink_get_mac_state(pl, &link_state);
 
 		/* The MAC is reporting the link results from its own PCS
-- 
2.7.4


^ permalink raw reply related

* [PATCH 01/11] phy: add phy_interface_mode_is_8023z() helper
From: Russell King @ 2017-12-01 10:24 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <20171201102306.GA18792@n2100.armlinux.org.uk>

Add and use phy_interface_mode_is_8023z() helper to identify the
interface modes that use 802.3z negotiation.  Use it in phylink's
phylink_mac_an_restart().

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c |  3 +--
 include/linux/phy.h       | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 5dc9668dde34..a02d67689d6e 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -320,8 +320,7 @@ static void phylink_mac_config(struct phylink *pl,
 static void phylink_mac_an_restart(struct phylink *pl)
 {
 	if (pl->link_config.an_enabled &&
-	    (pl->link_config.interface == PHY_INTERFACE_MODE_1000BASEX ||
-	     pl->link_config.interface == PHY_INTERFACE_MODE_2500BASEX))
+	    phy_interface_mode_is_8023z(pl->link_config.interface))
 		pl->ops->mac_an_restart(pl->netdev);
 }
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index dc82a07cb4fd..71d777fe6c3d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -763,6 +763,20 @@ static inline bool phy_interface_mode_is_rgmii(phy_interface_t mode)
 };
 
 /**
+ * phy_interface_mode_is_8023z() - does the phy interface mode use 802.3z
+ *   negotiation
+ * @mode: one of &enum phy_interface_t
+ *
+ * Returns true if the phy interface mode uses the 16-bit negotiation
+ * word as defined in 802.3z. (See 802.3-2015 37.2.1 Config_Reg encoding)
+ */
+static inline bool phy_interface_mode_is_8023z(phy_interface_t mode)
+{
+	return mode == PHY_INTERFACE_MODE_1000BASEX ||
+	       mode == PHY_INTERFACE_MODE_2500BASEX;
+}
+
+/**
  * phy_interface_is_rgmii - Convenience function for testing if a PHY interface
  * is RGMII (all variants)
  * @phydev: the phy_device struct
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 00/11] SFP/phylink updates
From: Russell King - ARM Linux @ 2017-12-01 10:23 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev
In-Reply-To: <20171130135835.GC10595@n2100.armlinux.org.uk>

Hi,

This series, which follows on from the fixes posted earlier, improves
the phylink/sfp support.  Changes included here are:

- Merge 802.3z and SGMII modes into one "in-band" mode, using the
  PHY_INTERFACE_MODE_xxx definition to determine which should be used.
  This allows more flexibility as more interface modes become
  available.

- Allow 2500base-X and 10GBASE-KR to be requested from SFP.

- Remove unused and unnecessary phylink_init_eee()

- Restart 802.3z autonegotiation when starting the network device to
  ensure that the negotiated parameters are always correct.  It has
  been observed on mvneta that this is not always the case without
  this change.

- Add kerneldoc documentation for phylink and sfp upstream facing APIs
  and link it in to the networking documentation.

- Resolve a sparse warning in sfp-bus.c

- Convert phylink/sfp to use fwnode rather than DT so that other firmware
  systems can take advantage of this - I have received a request for it
  to be usable with ACPI.  The exception to this is our interactions with
  phylib, as phylib itself does not yet support fwnode.

 Documentation/networking/kapi.rst |  24 +++
 drivers/net/phy/phylink.c         | 341 +++++++++++++++++++++++++++++---------
 drivers/net/phy/sfp-bus.c         | 120 +++++++++++++-
 include/linux/phy.h               |  14 ++
 include/linux/phylink.h           | 196 +++++++++++++++-------
 include/linux/sfp.h               |  57 +++++--
 6 files changed, 591 insertions(+), 161 deletions(-)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

^ permalink raw reply

* Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name
From: Quentin Monnet @ 2017-12-01 10:22 UTC (permalink / raw)
  To: Roman Gushchin, netdev
  Cc: linux-kernel, kernel-team, ast, daniel, jakub.kicinski, kafai
In-Reply-To: <20171130134302.2840-2-guro@fb.com>

Thanks Roman!
One comment in-line.

2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> The bpf_prog_load() function will guess program type if it's not
> specified explicitly. This functionality will be used to implement
> loading of different programs without asking a user to specify
> the program type. In first order it will be used by bpftool.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5aa45f89da93..9f2410beaa18 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1721,6 +1721,41 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRACEPOINT);
>  BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP);
>  BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT);
>  
> +static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
> +{
> +	if (!prog->section_name)
> +		goto err;
> +
> +	if (strncmp(prog->section_name, "socket", 6) == 0)
> +		return BPF_PROG_TYPE_SOCKET_FILTER;
> +	if (strncmp(prog->section_name, "kprobe/", 7) == 0)
> +		return BPF_PROG_TYPE_KPROBE;
> +	if (strncmp(prog->section_name, "kretprobe/", 10) == 0)
> +		return BPF_PROG_TYPE_KPROBE;
> +	if (strncmp(prog->section_name, "tracepoint/", 11) == 0)
> +		return BPF_PROG_TYPE_TRACEPOINT;
> +	if (strncmp(prog->section_name, "xdp", 3) == 0)
> +		return BPF_PROG_TYPE_XDP;
> +	if (strncmp(prog->section_name, "perf_event", 10) == 0)
> +		return BPF_PROG_TYPE_PERF_EVENT;
> +	if (strncmp(prog->section_name, "cgroup/skb", 10) == 0)
> +		return BPF_PROG_TYPE_CGROUP_SKB;
> +	if (strncmp(prog->section_name, "cgroup/sock", 11) == 0)
> +		return BPF_PROG_TYPE_CGROUP_SOCK;
> +	if (strncmp(prog->section_name, "cgroup/dev", 10) == 0)
> +		return BPF_PROG_TYPE_CGROUP_DEVICE;
> +	if (strncmp(prog->section_name, "sockops", 7) == 0)
> +		return BPF_PROG_TYPE_SOCK_OPS;
> +	if (strncmp(prog->section_name, "sk_skb", 6) == 0)
> +		return BPF_PROG_TYPE_SK_SKB;

I do not really like these hard-coded lengths, maybe we could work out
something nicer with a bit of pre-processing work? Perhaps something like:

#define SOCKET_FILTER_SEC_PREFIX "socket"
#define KPROBE_SEC_PREFIX "kprobe/"
[…]

#define TRY_TYPE(string, __TYPE)				\
	do {							\
		if (!strncmp(string, __TYPE ## _SEC_PREFIX,	\
			     sizeof(__TYPE ## _SEC_PREFIX)))	\
			return BPF_PROG_TYPE_ ## __TYPE;	\
	} while(0);

static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog)
{
	if (!prog->section_name)
		goto err;

	TRY_TYPE(prog->section_name, SOCKET_FILTER);
	TRY_TYPE(prog->section_name, KPROBE);
	[…]

err:
	pr_warning("…",
	           prog->section_name);

	return BPF_PROG_TYPE_UNSPEC;
}

> +
> +err:
> +	pr_warning("failed to guess program type based on section name %s\n",
> +		   prog->section_name);
> +
> +	return BPF_PROG_TYPE_UNSPEC;
> +}
> +
>  int bpf_map__fd(struct bpf_map *map)
>  {
>  	return map ? map->fd : -EINVAL;

^ permalink raw reply

* [PATCH net] stmmac: reset last TSO segment size after device open
From: Lars Persson @ 2017-12-01 10:12 UTC (permalink / raw)
  To: peppe.cavallaro, alexandre.torgue, netdev; +Cc: Lars Persson

The mss variable tracks the last max segment size sent to the TSO
engine. We do not update the hardware as long as we receive skb:s with
the same value in gso_size.

During a network device down/up cycle (mapped to stmmac_release() and
stmmac_open() callbacks) we issue a reset to the hardware and it
forgets the setting for mss. However we did not zero out our mss
variable so the next transmission of a gso packet happens with an
undefined hardware setting.

This triggers a hang in the TSO engine and eventuelly the netdev
watchdog will bark.

Fixes: f748be531d70 ("stmmac: support new GMAC4")
Signed-off-by: Lars Persson <larper@axis.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f63c2ddced3c..d7250539d0bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2588,6 +2588,7 @@ static int stmmac_open(struct net_device *dev)
 
 	priv->dma_buf_sz = STMMAC_ALIGN(buf_sz);
 	priv->rx_copybreak = STMMAC_RX_COPYBREAK;
+	priv->mss = 0;
 
 	ret = alloc_dma_desc_resources(priv);
 	if (ret < 0) {
-- 
2.11.0

^ permalink raw reply related

* Re: Linux 4.14 - regression: broken tun/tap / bridge network with virtio - bisected
From: Andreas Hartmann @ 2017-12-01 10:11 UTC (permalink / raw)
  To: netdev; +Cc: john.fastabend, Michal Kubecek
In-Reply-To: <9615150a-eb78-2f9d-798f-6aa460932aec@01019freenet.de>

Hello!

I hopefully could get rid of both of my problems (hanging network w/
virtio) and endless hanging qemu-process on VM shutdown by upgrading
qemu from 2.6.2 to 2.10.1. I hope it will persist.

BTW: Base system is Leap 42.2.

Sorry for the noise,
thanks,
kind regards,
Andreas

^ permalink raw reply

* [PATCH 3/3] tap: free skb if flags error
From: wexu @ 2017-12-01 10:10 UTC (permalink / raw)
  To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
In-Reply-To: <1512123038-15773-1-git-send-email-wexu@redhat.com>

From: Wei Xu <wexu@redhat.com>

tap_recvmsg() supports accepting skb by msg_control after
commit 3b4ba04acca8 ("tap: support receiving skb from msg_control"),
the skb if presented should be freed within the function, otherwise
it would be leaked.

Signed-off-by: Wei Xu <wexu@redhat.com>
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>

---
 drivers/net/tap.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index e9489b8..0a886fda 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -829,8 +829,11 @@ static ssize_t tap_do_read(struct tap_queue *q,
 	DEFINE_WAIT(wait);
 	ssize_t ret = 0;
 
-	if (!iov_iter_count(to))
+	if (!iov_iter_count(to)) {
+		if (skb)
+			kfree_skb(skb);
 		return 0;
+	}
 
 	if (skb)
 		goto put;
@@ -1154,11 +1157,14 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
 		       size_t total_len, int flags)
 {
 	struct tap_queue *q = container_of(sock, struct tap_queue, sock);
+	struct sk_buff *skb = m->msg_control;
 	int ret;
-	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
+	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
+		if (skb)
+			kfree_skb(skb);
 		return -EINVAL;
-	ret = tap_do_read(q, &m->msg_iter, flags & MSG_DONTWAIT,
-			  m->msg_control);
+	}
+	ret = tap_do_read(q, &m->msg_iter, flags & MSG_DONTWAIT, skb);
 	if (ret > total_len) {
 		m->msg_flags |= MSG_TRUNC;
 		ret = flags & MSG_TRUNC ? ret : total_len;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 2/3] tun: free skb in early errors
From: wexu @ 2017-12-01 10:10 UTC (permalink / raw)
  To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
In-Reply-To: <1512123038-15773-1-git-send-email-wexu@redhat.com>

From: Wei Xu <wexu@redhat.com>

tun_recvmsg() supports accepting skb by msg_control after
commit ac77cfd4258f ("tun: support receiving skb through msg_control"),
the skb if presented should be freed no matter how far it can go
along, otherwise it would be leaked.

This patch fixes several missed cases.

Signed-off-by: Wei Xu <wexu@redhat.com>
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>

---
 drivers/net/tun.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9574900..4f4a842 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1952,8 +1952,11 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 
 	tun_debug(KERN_INFO, tun, "tun_do_read\n");
 
-	if (!iov_iter_count(to))
+	if (!iov_iter_count(to)) {
+		if (skb)
+			kfree_skb(skb);
 		return 0;
+	}
 
 	if (!skb) {
 		/* Read frames from ring */
@@ -2069,22 +2072,24 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
 {
 	struct tun_file *tfile = container_of(sock, struct tun_file, socket);
 	struct tun_struct *tun = tun_get(tfile);
+	struct sk_buff *skb = m->msg_control;
 	int ret;
 
-	if (!tun)
-		return -EBADFD;
+	if (!tun) {
+		ret = -EBADFD;
+		goto out_free_skb;
+	}
 
 	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) {
 		ret = -EINVAL;
-		goto out;
+		goto out_put_tun;
 	}
 	if (flags & MSG_ERRQUEUE) {
 		ret = sock_recv_errqueue(sock->sk, m, total_len,
 					 SOL_PACKET, TUN_TX_TIMESTAMP);
 		goto out;
 	}
-	ret = tun_do_read(tun, tfile, &m->msg_iter, flags & MSG_DONTWAIT,
-			  m->msg_control);
+	ret = tun_do_read(tun, tfile, &m->msg_iter, flags & MSG_DONTWAIT, skb);
 	if (ret > (ssize_t)total_len) {
 		m->msg_flags |= MSG_TRUNC;
 		ret = flags & MSG_TRUNC ? ret : total_len;
@@ -2092,6 +2097,13 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
 out:
 	tun_put(tun);
 	return ret;
+
+out_put_tun:
+	tun_put(tun);
+out_free_skb:
+	if (skb)
+		kfree_skb(skb);
+	return ret;
 }
 
 static int tun_peek_len(struct socket *sock)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 1/3] vhost: fix skb leak in handle_rx()
From: wexu @ 2017-12-01 10:10 UTC (permalink / raw)
  To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu
In-Reply-To: <1512123038-15773-1-git-send-email-wexu@redhat.com>

From: Wei Xu <wexu@redhat.com>

Matthew found a roughly 40% tcp throughput regression with commit
c67df11f(vhost_net: try batch dequing from skb array) as discussed
in the following thread:
https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html

Eventually we figured out that it was a skb leak in handle_rx()
when sending packets to the VM. This usually happens when a guest
can not drain out vq as fast as vhost fills in, afterwards it sets
off the traffic jam and leaks skb(s) which occurs as no headcount
to send on the vq from vhost side.

This can be avoided by making sure we have got enough headcount
before actually consuming a skb from the batched rx array while
transmitting, which is simply done by moving checking the zero
headcount a bit ahead.

Signed-off-by: Wei Xu <wexu@redhat.com>
Reported-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
 drivers/vhost/net.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8d626d7..c7bdeb6 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -778,16 +778,6 @@ static void handle_rx(struct vhost_net *net)
 		/* On error, stop handling until the next kick. */
 		if (unlikely(headcount < 0))
 			goto out;
-		if (nvq->rx_array)
-			msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
-		/* On overrun, truncate and discard */
-		if (unlikely(headcount > UIO_MAXIOV)) {
-			iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
-			err = sock->ops->recvmsg(sock, &msg,
-						 1, MSG_DONTWAIT | MSG_TRUNC);
-			pr_debug("Discarded rx packet: len %zd\n", sock_len);
-			continue;
-		}
 		/* OK, now we need to know about added descriptors. */
 		if (!headcount) {
 			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
@@ -800,6 +790,16 @@ static void handle_rx(struct vhost_net *net)
 			 * they refilled. */
 			goto out;
 		}
+		if (nvq->rx_array)
+			msg.msg_control = vhost_net_buf_consume(&nvq->rxq);
+		/* On overrun, truncate and discard */
+		if (unlikely(headcount > UIO_MAXIOV)) {
+			iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
+			err = sock->ops->recvmsg(sock, &msg,
+						 1, MSG_DONTWAIT | MSG_TRUNC);
+			pr_debug("Discarded rx packet: len %zd\n", sock_len);
+			continue;
+		}
 		/* We don't need to be notified again. */
 		iov_iter_init(&msg.msg_iter, READ, vq->iov, in, vhost_len);
 		fixup = msg.msg_iter;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net,stable v4 0/3]  vhost: fix a few skb leaks
From: wexu @ 2017-12-01 10:10 UTC (permalink / raw)
  To: virtualization, netdev, linux-kernel; +Cc: jasowang, mst, mjrosato, wexu

From: Wei Xu <wexu@redhat.com>

Matthew found a roughly 40% tcp throughput regression with commit
c67df11f(vhost_net: try batch dequing from skb array) as discussed
in the following thread:
https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html

v4:
- fix zero iov iterator count in tap/tap_do_read()(Jason)
- don't put tun in case of EBADFD(Jason)
- Replace msg->msg_control with new 'skb' when calling tun/tap_do_read()

v3:
- move freeing skb from vhost to tun/tap recvmsg() to not
  confuse the callers.

v2:
- add Matthew as the reporter, thanks matthew.
- moving zero headcount check ahead instead of defer consuming skb
  due to jason and mst's comment.
- add freeing skb in favor of recvmsg() fails.

Wei Xu (3):
  vhost: fix skb leak in handle_rx()
  tun: free skb in early errors
  tap: free skb if flags error

 drivers/net/tap.c   | 14 ++++++++++----
 drivers/net/tun.c   | 24 ++++++++++++++++++------
 drivers/vhost/net.c | 20 ++++++++++----------
 3 files changed, 38 insertions(+), 20 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH][V2] wlcore, wl1251: fix spelling: "Couldnt" -> "Couldn't" and remove error on -ENOMEM
From: Colin King @ 2017-12-01 10:02 UTC (permalink / raw)
  To: Kalle Valo, linux-wireless, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake in error message text.  Also remove the
error message on an kzalloc failure as this is redundant.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/ti/wl1251/init.c | 4 +---
 drivers/net/wireless/ti/wlcore/acx.c  | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ti/wl1251/init.c b/drivers/net/wireless/ti/wl1251/init.c
index 1d799bffaa9f..e7d77acdf18c 100644
--- a/drivers/net/wireless/ti/wl1251/init.c
+++ b/drivers/net/wireless/ti/wl1251/init.c
@@ -310,10 +310,8 @@ static int wl1251_hw_init_data_path_config(struct wl1251 *wl)
 	/* asking for the data path parameters */
 	wl->data_path = kzalloc(sizeof(struct acx_data_path_params_resp),
 				GFP_KERNEL);
-	if (!wl->data_path) {
-		wl1251_error("Couldnt allocate data path parameters");
+	if (!wl->data_path)
 		return -ENOMEM;
-	}
 
 	ret = wl1251_acx_data_path_params(wl, wl->data_path);
 	if (ret < 0) {
diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
index a4859993db3c..3ca9167d6146 100644
--- a/drivers/net/wireless/ti/wlcore/acx.c
+++ b/drivers/net/wireless/ti/wlcore/acx.c
@@ -146,7 +146,7 @@ int wl1271_acx_feature_cfg(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	ret = wl1271_cmd_configure(wl, ACX_FEATURE_CFG,
 				   feature, sizeof(*feature));
 	if (ret < 0) {
-		wl1271_error("Couldnt set HW encryption");
+		wl1271_error("Couldn't set HW encryption");
 		goto out;
 	}
 
-- 
2.14.1

^ permalink raw reply related

* pull-request: can-next 2017-12-01
From: Marc Kleine-Budde @ 2017-12-01  9:57 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, kernel@pengutronix.de, linux-can@vger.kernel.org


[-- Attachment #1.1: Type: text/plain, Size: 3290 bytes --]

Hello David,

this is a pull request of 10 patches for net-next/master.

The first two patches are by Arnd Bergmann, they convert the peak_usb
from using "struct timeval" to "ktime_t". The error handling in the
vxcan driver is clean up by Markus Elfring's patch. Bhumika Goyal
contributes a patch for the c_can_pci driver to make the pci data const.
The six patches by Pankaj Bansal for the flexcan driver add LS1021A
support by making the endianness of the driver configurable by the
device tree.

regards,
Marc

---

The following changes since commit 201c78e05c5adaffa163b022c9b3a4d30debe100:

  Merge branch 'macb-rx-packet-filtering' (2017-11-30 14:12:47 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-4.17-20171201

for you to fetch changes up to fa2edcfb5f2e99e9f47f7ecba9abc5f192aa388b:

  arm: dts: Add nodes for flexcan devices present on LS1021A-Rev2 SoC (2017-12-01 09:14:23 +0100)

----------------------------------------------------------------
linux-can-next-for-4.17-20171201

----------------------------------------------------------------
Arnd Bergmann (2):
      can: peak_usb: remove some 'struct timeval' users
      can: peak_usb: use ktime_t consistently

Bhumika Goyal (1):
      can: c_can_pci: make c_can_pci_data const

Markus Elfring (1):
      can: vxcan: Use common error handling code in vxcan_newlink()

Pankaj Bansal (6):
      can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers.
      can: flexcan: adding platform specific details for LS1021A
      Documentation: can: flexcan: Add big-endian property to device tree
      powerpc: dts: P1010: Add endianness property to flexcan node
      arm: dts: Remove p1010-flexcan compatible from imx series dts
      arm: dts: Add nodes for flexcan devices present on LS1021A-Rev2 SoC

 .../devicetree/bindings/net/can/fsl-flexcan.txt    |   6 +
 arch/arm/boot/dts/imx25.dtsi                       |   4 +-
 arch/arm/boot/dts/imx28.dtsi                       |   4 +-
 arch/arm/boot/dts/imx35.dtsi                       |   4 +-
 arch/arm/boot/dts/imx53.dtsi                       |   4 +-
 arch/arm/boot/dts/ls1021a-qds.dts                  |  16 ++
 arch/arm/boot/dts/ls1021a-twr.dts                  |  16 ++
 arch/arm/boot/dts/ls1021a.dtsi                     |  36 +++
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi        |   2 +
 drivers/net/can/c_can/c_can_pci.c                  |   4 +-
 drivers/net/can/flexcan.c                          | 241 ++++++++++++---------
 drivers/net/can/usb/peak_usb/pcan_usb.c            |   9 +-
 drivers/net/can/usb/peak_usb/pcan_usb_core.c       |  53 ++---
 drivers/net/can/usb/peak_usb/pcan_usb_core.h       |   5 +-
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c        |   9 +-
 drivers/net/can/vxcan.c                            |  16 +-
 16 files changed, 258 insertions(+), 171 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] wlcore, wl1251: fix spelling mistake: "Couldnt" -> "Couldn't"
From: Joe Perches @ 2017-12-01  9:42 UTC (permalink / raw)
  To: Colin King, Kalle Valo, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel
In-Reply-To: <20171201093433.9154-1-colin.king@canonical.com>

On Fri, 2017-12-01 at 09:34 +0000, Colin King wrote:
> Trivial fix to spelling mistake in error message text.
[]
> diff --git a/drivers/net/wireless/ti/wl1251/init.c b/drivers/net/wireless/ti/wl1251/init.c
[]
> @@ -311,7 +311,7 @@ static int wl1251_hw_init_data_path_config(struct wl1251 *wl)
>  	wl->data_path = kzalloc(sizeof(struct acx_data_path_params_resp),
>  				GFP_KERNEL);
>  	if (!wl->data_path) {
> -		wl1251_error("Couldnt allocate data path parameters");
> +		wl1251_error("Couldn't allocate data path parameters");

Might be better to remove this one.

^ permalink raw reply

* [PATCH] wlcore, wl1251: fix spelling mistake: "Couldnt" -> "Couldn't"
From: Colin King @ 2017-12-01  9:34 UTC (permalink / raw)
  To: Kalle Valo, linux-wireless, netdev; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake in error message text.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/ti/wl1251/init.c | 2 +-
 drivers/net/wireless/ti/wlcore/acx.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ti/wl1251/init.c b/drivers/net/wireless/ti/wl1251/init.c
index 1d799bffaa9f..642308bdf742 100644
--- a/drivers/net/wireless/ti/wl1251/init.c
+++ b/drivers/net/wireless/ti/wl1251/init.c
@@ -311,7 +311,7 @@ static int wl1251_hw_init_data_path_config(struct wl1251 *wl)
 	wl->data_path = kzalloc(sizeof(struct acx_data_path_params_resp),
 				GFP_KERNEL);
 	if (!wl->data_path) {
-		wl1251_error("Couldnt allocate data path parameters");
+		wl1251_error("Couldn't allocate data path parameters");
 		return -ENOMEM;
 	}
 
diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
index a4859993db3c..3ca9167d6146 100644
--- a/drivers/net/wireless/ti/wlcore/acx.c
+++ b/drivers/net/wireless/ti/wlcore/acx.c
@@ -146,7 +146,7 @@ int wl1271_acx_feature_cfg(struct wl1271 *wl, struct wl12xx_vif *wlvif)
 	ret = wl1271_cmd_configure(wl, ACX_FEATURE_CFG,
 				   feature, sizeof(*feature));
 	if (ret < 0) {
-		wl1271_error("Couldnt set HW encryption");
+		wl1271_error("Couldn't set HW encryption");
 		goto out;
 	}
 
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH 3/3] make sock_alloc_file() do sock_release() on failures
From: Eric Dumazet @ 2017-12-01  9:33 UTC (permalink / raw)
  To: Al Viro, netdev
In-Reply-To: <20171201002325.GL21978@ZenIV.linux.org.uk>

On Fri, 2017-12-01 at 00:23 +0000, Al Viro wrote:
> This changes calling conventions (and simplifies the hell out
> the callers).  New rules: once struct socket had been passed
> to sock_alloc_file(), it's been consumed either by struct file
> or by sock_release() done by sock_alloc_file().  Either way
> the caller should not do sock_release() after that point.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Nice cleanup !

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH 2/3] fix kcm_clone()
From: Eric Dumazet @ 2017-12-01  9:31 UTC (permalink / raw)
  To: Al Viro, netdev; +Cc: Tom Herbert
In-Reply-To: <20171201002244.GK21978@ZenIV.linux.org.uk>

Sorry for top posting.

Lets CC Tom on this patch ?

On Fri, 2017-12-01 at 00:22 +0000, Al Viro wrote:
> 1) it's fput() or sock_release(), not both
> 2) don't do fd_install() until the last failure exit.
> 3) not a bug per se, but... don't attach socket to struct file
>    until it's set up.
> 
> Take reserving descriptor into the caller, move fd_install() to the
> caller, sanitize failure exits and calling conventions.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  net/kcm/kcmsock.c | 71 +++++++++++++++++++++----------------------
> ------------
>  1 file changed, 27 insertions(+), 44 deletions(-)
> 
> diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
> index 0b750a22c4b9..c5fa634e63ca 100644
> --- a/net/kcm/kcmsock.c
> +++ b/net/kcm/kcmsock.c
> @@ -1625,60 +1625,35 @@ static struct proto kcm_proto = {
>  };
>  
>  /* Clone a kcm socket. */
> -static int kcm_clone(struct socket *osock, struct kcm_clone *info,
> -		     struct socket **newsockp)
> +static struct file *kcm_clone(struct socket *osock)
>  {
>  	struct socket *newsock;
>  	struct sock *newsk;
> -	struct file *newfile;
> -	int err, newfd;
> +	struct file *file;
>  
> -	err = -ENFILE;
>  	newsock = sock_alloc();
>  	if (!newsock)
> -		goto out;
> +		return ERR_PTR(-ENFILE);
>  
>  	newsock->type = osock->type;
>  	newsock->ops = osock->ops;
>  
>  	__module_get(newsock->ops->owner);
>  
> -	newfd = get_unused_fd_flags(0);
> -	if (unlikely(newfd < 0)) {
> -		err = newfd;
> -		goto out_fd_fail;
> -	}
> -
> -	newfile = sock_alloc_file(newsock, 0, osock->sk-
> >sk_prot_creator->name);
> -	if (IS_ERR(newfile)) {
> -		err = PTR_ERR(newfile);
> -		goto out_sock_alloc_fail;
> -	}
> -
>  	newsk = sk_alloc(sock_net(osock->sk), PF_KCM, GFP_KERNEL,
>  			 &kcm_proto, true);
>  	if (!newsk) {
> -		err = -ENOMEM;
> -		goto out_sk_alloc_fail;
> +		sock_release(newsock);
> +		return ERR_PTR(-ENOMEM);
>  	}
> -
>  	sock_init_data(newsock, newsk);
>  	init_kcm_sock(kcm_sk(newsk), kcm_sk(osock->sk)->mux);
>  
> -	fd_install(newfd, newfile);
> -	*newsockp = newsock;
> -	info->fd = newfd;
> -
> -	return 0;
> +	file = sock_alloc_file(newsock, 0, osock->sk-
> >sk_prot_creator->name);
> +	if (IS_ERR(file))
> +		sock_release(newsock);
>  
> -out_sk_alloc_fail:
> -	fput(newfile);
> -out_sock_alloc_fail:
> -	put_unused_fd(newfd);
> -out_fd_fail:
> -	sock_release(newsock);
> -out:
> -	return err;
> +	return file;
>  }
>  
>  static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned
> long arg)
> @@ -1708,17 +1683,25 @@ static int kcm_ioctl(struct socket *sock,
> unsigned int cmd, unsigned long arg)
>  	}
>  	case SIOCKCMCLONE: {
>  		struct kcm_clone info;
> -		struct socket *newsock = NULL;
> -
> -		err = kcm_clone(sock, &info, &newsock);
> -		if (!err) {
> -			if (copy_to_user((void __user *)arg, &info,
> -					 sizeof(info))) {
> -				err = -EFAULT;
> -				sys_close(info.fd);
> -			}
> -		}
> +		struct file *file;
> +
> +		info.fd = get_unused_fd_flags(0);
> +		if (unlikely(info.fd < 0))
> +			return info.fd;
>  
> +		file = kcm_clone(sock);
> +		if (IS_ERR(file)) {
> +			put_unused_fd(info.fd);
> +			return PTR_ERR(file);
> +		}
> +		if (copy_to_user((void __user *)arg, &info,
> +				 sizeof(info))) {
> +			put_unused_fd(info.fd);
> +			fput(file);
> +			return -EFAULT;
> +		}
> +		fd_install(info.fd, file);
> +		err = 0;
>  		break;
>  	}
>  	default:

^ permalink raw reply

* Re: [PATCH 1/3] socketpair(): allocate descriptors first
From: Eric Dumazet @ 2017-12-01  9:28 UTC (permalink / raw)
  To: Al Viro, netdev
In-Reply-To: <20171201002203.GJ21978@ZenIV.linux.org.uk>

On Fri, 2017-12-01 at 00:22 +0000, Al Viro wrote:
> simplifies failure exits considerably...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ 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