Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next 1/3] net: dsa: mediatek: add VLAN support for MT7530
From: sean.wang @ 2017-12-13  4:01 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, vivien.didelot
  Cc: netdev, linux-kernel, linux-mediatek, Sean Wang
In-Reply-To: <cover.1513136754.git.sean.wang@mediatek.com>

From: Sean Wang <sean.wang@mediatek.com>

MT7530 can treat each port as either VLAN-unaware port or VLAN-aware port
through the implementation of port matrix mode or port security mode on
the ingress port, respectively. On one hand, Each port has been acting as
the VLAN-unaware one whenever the device is created in the initial or
certain port joins or leaves into/from the bridge at the runtime. On the
other hand, the patch just filling the required callbacks for VLAN
operations is achieved via extending the port to be into port security
mode when the port is configured as VLAN-aware port. Which mode can make
the port be able to recognize VID from incoming packets and look up VLAN
table to validate and judge which port it should be going to. And the
range for VID from 1 to 4094 is valid for the hardware.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/net/dsa/mt7530.c | 291 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mt7530.h |  83 +++++++++++++-
 2 files changed, 367 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2820d69..252e8ba 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -805,6 +805,69 @@ mt7530_port_bridge_join(struct dsa_switch *ds, int port,
 }
 
 static void
+mt7530_port_set_vlan_unaware(struct dsa_switch *ds, int port)
+{
+	struct mt7530_priv *priv = ds->priv;
+	bool all_user_ports_removed = true;
+	int i;
+
+	/* When a port is removed from the bridge, the port would be set up
+	 * back to the default as is at initial boot which is a VLAN-unaware
+	 * port.
+	 */
+	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
+		   MT7530_PORT_MATRIX_MODE);
+	mt7530_rmw(priv, MT7530_PVC_P(port), VLAN_ATTR_MASK,
+		   VLAN_ATTR(MT7530_VLAN_TRANSPARENT));
+
+	priv->ports[port].vlan_filtering = false;
+
+	for (i = 0; i < MT7530_NUM_PORTS; i++) {
+		if (dsa_is_user_port(ds, i) &&
+		    priv->ports[i].vlan_filtering) {
+			all_user_ports_removed = false;
+			break;
+		}
+	}
+
+	/* CPU port also does the same thing until all user ports belonging to
+	 * the CPU port get out of VLAN filtering mode.
+	 */
+	if (all_user_ports_removed) {
+		mt7530_write(priv, MT7530_PCR_P(MT7530_CPU_PORT),
+			     PCR_MATRIX(dsa_user_ports(priv->ds)));
+		mt7530_write(priv, MT7530_PVC_P(MT7530_CPU_PORT),
+			     PORT_SPEC_TAG);
+	}
+}
+
+static void
+mt7530_port_set_vlan_aware(struct dsa_switch *ds, int port)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	/* The real fabric path would be decided on the membership in the
+	 * entry of VLAN table. PCR_MATRIX set up here with ALL_MEMBERS
+	 * means potential VLAN can be consisting of certain subset of all
+	 * ports.
+	 */
+	mt7530_rmw(priv, MT7530_PCR_P(port),
+		   PCR_MATRIX_MASK, PCR_MATRIX(MT7530_ALL_MEMBERS));
+
+	/* Trapped into security mode allows packet forwarding through VLAN
+	 * table lookup.
+	 */
+	mt7530_rmw(priv, MT7530_PCR_P(port), PCR_PORT_VLAN_MASK,
+		   MT7530_PORT_SECURITY_MODE);
+
+	/* Set the port as a user port which is to be able to recognize VID
+	 * from incoming packets before fetching entry within the VLAN table.
+	 */
+	mt7530_rmw(priv, MT7530_PVC_P(port), VLAN_ATTR_MASK,
+		   VLAN_ATTR(MT7530_VLAN_USER));
+}
+
+static void
 mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 			 struct net_device *bridge)
 {
@@ -817,8 +880,11 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 		/* Remove this port from the port matrix of the other ports
 		 * in the same bridge. If the port is disabled, port matrix
 		 * is kept and not being setup until the port becomes enabled.
+		 * And the other port's port matrix cannot be broken when the
+		 * other port is still a VLAN-aware port.
 		 */
-		if (dsa_is_user_port(ds, i) && i != port) {
+		if (!priv->ports[i].vlan_filtering &&
+		    dsa_is_user_port(ds, i) && i != port) {
 			if (dsa_to_port(ds, i)->bridge_dev != bridge)
 				continue;
 			if (priv->ports[i].enable)
@@ -836,6 +902,8 @@ mt7530_port_bridge_leave(struct dsa_switch *ds, int port,
 			   PCR_MATRIX(BIT(MT7530_CPU_PORT)));
 	priv->ports[port].pm = PCR_MATRIX(BIT(MT7530_CPU_PORT));
 
+	mt7530_port_set_vlan_unaware(ds, port);
+
 	mutex_unlock(&priv->reg_mutex);
 }
 
@@ -906,6 +974,223 @@ mt7530_port_fdb_dump(struct dsa_switch *ds, int port,
 	return 0;
 }
 
+static int
+mt7530_vlan_cmd(struct mt7530_priv *priv, enum mt7530_vlan_cmd cmd, u16 vid)
+{
+	struct mt7530_dummy_poll p;
+	u32 val;
+	int ret;
+
+	val = VTCR_BUSY | VTCR_FUNC(cmd) | vid;
+	mt7530_write(priv, MT7530_VTCR, val);
+
+	INIT_MT7530_DUMMY_POLL(&p, priv, MT7530_VTCR);
+	ret = readx_poll_timeout(_mt7530_read, &p, val,
+				 !(val & VTCR_BUSY), 20, 20000);
+	if (ret < 0) {
+		dev_err(priv->dev, "poll timeout\n");
+		return ret;
+	}
+
+	val = mt7530_read(priv, MT7530_VTCR);
+	if (val & VTCR_INVALID) {
+		dev_err(priv->dev, "read VTCR invalid\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+mt7530_port_vlan_filtering(struct dsa_switch *ds, int port,
+			   bool vlan_filtering)
+{
+	struct mt7530_priv *priv = ds->priv;
+
+	priv->ports[port].vlan_filtering = vlan_filtering;
+
+	if (vlan_filtering) {
+		/* The port is being kept as VLAN-unaware port when bridge is
+		 * set up with vlan_filtering not being set, Otherwise, the
+		 * port and the corresponding CPU port is required the setup
+		 * for becoming a VLAN-aware port.
+		 */
+		mt7530_port_set_vlan_aware(ds, port);
+		mt7530_port_set_vlan_aware(ds, MT7530_CPU_PORT);
+	}
+
+	return 0;
+}
+
+static int
+mt7530_port_vlan_prepare(struct dsa_switch *ds, int port,
+			 const struct switchdev_obj_port_vlan *vlan,
+			 struct switchdev_trans *trans)
+{
+	/* nothing needed */
+
+	return 0;
+}
+
+static void
+mt7530_hw_vlan_add(struct mt7530_priv *priv,
+		   struct mt7530_hw_vlan_entry *entry)
+{
+	u8 new_members;
+	u32 val;
+
+	new_members = entry->old_members | BIT(entry->port) |
+		      BIT(MT7530_CPU_PORT);
+
+	/* Validate the entry with independent learning, create egress tag per
+	 * VLAN and joining the port as one of the port members.
+	 */
+	val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) | VLAN_VALID;
+	mt7530_write(priv, MT7530_VAWD1, val);
+
+	/* Decide whether adding tag or not for those outgoing packets from the
+	 * port inside the VLAN.
+	 */
+	val = entry->untagged ? MT7530_VLAN_EGRESS_UNTAG :
+				MT7530_VLAN_EGRESS_TAG;
+	mt7530_rmw(priv, MT7530_VAWD2,
+		   ETAG_CTRL_P_MASK(entry->port),
+		   ETAG_CTRL_P(entry->port, val));
+
+	/* CPU port is always taken as a tagged port for serving more than one
+	 * VLANs across and also being applied with egress type stack mode for
+	 * that VLAN tags would be appended after hardware special tag used as
+	 * DSA tag.
+	 */
+	mt7530_rmw(priv, MT7530_VAWD2,
+		   ETAG_CTRL_P_MASK(MT7530_CPU_PORT),
+		   ETAG_CTRL_P(MT7530_CPU_PORT,
+			       MT7530_VLAN_EGRESS_STACK));
+}
+
+static void
+mt7530_hw_vlan_del(struct mt7530_priv *priv,
+		   struct mt7530_hw_vlan_entry *entry)
+{
+	u8 new_members;
+	u32 val;
+
+	new_members = entry->old_members & ~BIT(entry->port);
+
+	val = mt7530_read(priv, MT7530_VAWD1);
+	if (!(val & VLAN_VALID)) {
+		dev_err(priv->dev,
+			"Cannot be deleted due to invalid entry\n");
+		return;
+	}
+
+	/* If certain member apart from CPU port is still alive in the VLAN,
+	 * the entry would be kept valid. Otherwise, the entry is got to be
+	 * disabled.
+	 */
+	if (new_members && new_members != BIT(MT7530_CPU_PORT)) {
+		val = IVL_MAC | VTAG_EN | PORT_MEM(new_members) |
+		      VLAN_VALID;
+		mt7530_write(priv, MT7530_VAWD1, val);
+	} else {
+		mt7530_write(priv, MT7530_VAWD1, 0);
+		mt7530_write(priv, MT7530_VAWD2, 0);
+	}
+}
+
+static void
+mt7530_hw_vlan_update(struct mt7530_priv *priv, u16 vid,
+		      struct mt7530_hw_vlan_entry *entry,
+		      mt7530_vlan_op vlan_op)
+{
+	u32 val;
+
+	/* Fetch entry */
+	mt7530_vlan_cmd(priv, MT7530_VTCR_RD_VID, vid);
+
+	val = mt7530_read(priv, MT7530_VAWD1);
+
+	entry->old_members = (val >> PORT_MEM_SHFT) & PORT_MEM_MASK;
+
+	/* Manipulate entry */
+	vlan_op(priv, entry);
+
+	/* Flush result to hardware */
+	mt7530_vlan_cmd(priv, MT7530_VTCR_WR_VID, vid);
+}
+
+static void
+mt7530_port_vlan_add(struct dsa_switch *ds, int port,
+		     const struct switchdev_obj_port_vlan *vlan,
+		     struct switchdev_trans *trans)
+{
+	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
+	bool pvid = vlan->flags & BRIDGE_VLAN_INFO_PVID;
+	struct mt7530_hw_vlan_entry new_entry;
+	struct mt7530_priv *priv = ds->priv;
+	int ret;
+	u16 vid;
+
+	/* The port is kept as VLAN-unaware if bridge with vlan_filtering not
+	 * being set.
+	 */
+	if (!priv->ports[port].vlan_filtering)
+		return;
+
+	mutex_lock(&priv->reg_mutex);
+
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+		mt7530_hw_vlan_entry_init(&new_entry, port, untagged);
+		mt7530_hw_vlan_update(priv, vid, &new_entry,
+				      mt7530_hw_vlan_add);
+	}
+
+	if (pvid) {
+		mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK,
+			   G0_PORT_VID(vlan->vid_end));
+		priv->ports[port].pvid = vlan->vid_end;
+	}
+
+	mutex_unlock(&priv->reg_mutex);
+}
+
+static int
+mt7530_port_vlan_del(struct dsa_switch *ds, int port,
+		     const struct switchdev_obj_port_vlan *vlan)
+{
+	struct mt7530_hw_vlan_entry target_entry;
+	struct mt7530_priv *priv = ds->priv;
+	u16 vid, pvid;
+
+	/* The port is kept as VLAN-unaware if bridge with vlan_filtering not
+	 * being set.
+	 */
+	if (!priv->ports[port].vlan_filtering)
+		return 0;
+
+	mutex_lock(&priv->reg_mutex);
+
+	pvid = priv->ports[port].pvid;
+	for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
+		mt7530_hw_vlan_entry_init(&target_entry, port, 0);
+		mt7530_hw_vlan_update(priv, vid, &target_entry,
+				      mt7530_hw_vlan_del);
+
+		/* PVID is being restored to the default whenever the PVID port
+		 * is being removed from the VLAN.
+		 */
+		if (pvid == vid)
+			pvid = G0_PORT_VID_DEF;
+	}
+
+	mt7530_rmw(priv, MT7530_PPBV1_P(port), G0_PORT_VID_MASK, pvid);
+	priv->ports[port].pvid = pvid;
+
+	mutex_unlock(&priv->reg_mutex);
+
+	return 0;
+}
+
 static enum dsa_tag_protocol
 mtk_get_tag_protocol(struct dsa_switch *ds, int port)
 {
@@ -1035,6 +1320,10 @@ static const struct dsa_switch_ops mt7530_switch_ops = {
 	.port_fdb_add		= mt7530_port_fdb_add,
 	.port_fdb_del		= mt7530_port_fdb_del,
 	.port_fdb_dump		= mt7530_port_fdb_dump,
+	.port_vlan_filtering	= mt7530_port_vlan_filtering,
+	.port_vlan_prepare	= mt7530_port_vlan_prepare,
+	.port_vlan_add		= mt7530_port_vlan_add,
+	.port_vlan_del		= mt7530_port_vlan_del,
 };
 
 static int
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 74db982..d9b407a 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -17,6 +17,7 @@
 #define MT7530_NUM_PORTS		7
 #define MT7530_CPU_PORT			6
 #define MT7530_NUM_FDB_RECORDS		2048
+#define MT7530_ALL_MEMBERS		0xff
 
 #define	NUM_TRGMII_CTRL			5
 
@@ -88,21 +89,42 @@ enum mt7530_fdb_cmd {
 /* Register for vlan table control */
 #define MT7530_VTCR			0x90
 #define  VTCR_BUSY			BIT(31)
-#define  VTCR_FUNC			(((x) & 0xf) << 12)
-#define  VTCR_FUNC_RD_VID		0x1
-#define  VTCR_FUNC_WR_VID		0x2
-#define  VTCR_FUNC_INV_VID		0x3
-#define  VTCR_FUNC_VAL_VID		0x4
+#define  VTCR_INVALID			BIT(16)
+#define  VTCR_FUNC(x)			(((x) & 0xf) << 12)
 #define  VTCR_VID			((x) & 0xfff)
 
+enum mt7530_vlan_cmd {
+	/* Read/Write the specified VID entry from VAWD register based
+	 * on VID.
+	 */
+	MT7530_VTCR_RD_VID = 0,
+	MT7530_VTCR_WR_VID = 1,
+};
+
 /* Register for setup vlan and acl write data */
 #define MT7530_VAWD1			0x94
 #define  PORT_STAG			BIT(31)
+/* Independent VLAN Learning */
 #define  IVL_MAC			BIT(30)
+/* Per VLAN Egress Tag Control */
+#define  VTAG_EN			BIT(28)
+/* VLAN Member Control */
 #define  PORT_MEM(x)			(((x) & 0xff) << 16)
-#define  VALID				BIT(1)
+/* VLAN Entry Valid */
+#define  VLAN_VALID			BIT(0)
+#define  PORT_MEM_SHFT			16
+#define  PORT_MEM_MASK			0xff
 
 #define MT7530_VAWD2			0x98
+/* Egress Tag Control */
+#define  ETAG_CTRL_P(p, x)		(((x) & 0x3) << ((p) << 1))
+#define  ETAG_CTRL_P_MASK(p)		ETAG_CTRL_P(p, 3)
+
+enum mt7530_vlan_egress_attr {
+	MT7530_VLAN_EGRESS_UNTAG = 0,
+	MT7530_VLAN_EGRESS_TAG = 2,
+	MT7530_VLAN_EGRESS_STACK = 3,
+};
 
 /* Register for port STP state control */
 #define MT7530_SSP_P(x)			(0x2000 + ((x) * 0x100))
@@ -120,11 +142,23 @@ enum mt7530_stp_state {
 /* Register for port control */
 #define MT7530_PCR_P(x)			(0x2004 + ((x) * 0x100))
 #define  PORT_VLAN(x)			((x) & 0x3)
+
+enum mt7530_port_mode {
+	/* Port Matrix Mode: Frames are forwarded by the PCR_MATRIX members. */
+	MT7530_PORT_MATRIX_MODE = PORT_VLAN(0),
+
+	/* Security Mode: Discard any frame due to ingress membership
+	 * violation or VID missed on the VLAN table.
+	 */
+	MT7530_PORT_SECURITY_MODE = PORT_VLAN(3),
+};
+
 #define  PCR_MATRIX(x)			(((x) & 0xff) << 16)
 #define  PORT_PRI(x)			(((x) & 0x7) << 24)
 #define  EG_TAG(x)			(((x) & 0x3) << 28)
 #define  PCR_MATRIX_MASK		PCR_MATRIX(0xff)
 #define  PCR_MATRIX_CLR			PCR_MATRIX(0)
+#define  PCR_PORT_VLAN_MASK		PORT_VLAN(3)
 
 /* Register for port security control */
 #define MT7530_PSC_P(x)			(0x200c + ((x) * 0x100))
@@ -134,10 +168,20 @@ enum mt7530_stp_state {
 #define MT7530_PVC_P(x)			(0x2010 + ((x) * 0x100))
 #define  PORT_SPEC_TAG			BIT(5)
 #define  VLAN_ATTR(x)			(((x) & 0x3) << 6)
+#define  VLAN_ATTR_MASK			VLAN_ATTR(3)
+
+enum mt7530_vlan_port_attr {
+	MT7530_VLAN_USER = 0,
+	MT7530_VLAN_TRANSPARENT = 3,
+};
+
 #define  STAG_VPID			(((x) & 0xffff) << 16)
 
 /* Register for port port-and-protocol based vlan 1 control */
 #define MT7530_PPBV1_P(x)		(0x2014 + ((x) * 0x100))
+#define  G0_PORT_VID(x)			(((x) & 0xfff) << 0)
+#define  G0_PORT_VID_MASK		G0_PORT_VID(0xfff)
+#define  G0_PORT_VID_DEF		G0_PORT_VID(1)
 
 /* Register for port MAC control register */
 #define MT7530_PMCR_P(x)		(0x3000 + ((x) * 0x100))
@@ -345,9 +389,20 @@ struct mt7530_fdb {
 	bool noarp;
 };
 
+/* struct mt7530_port -	This is the main data structure for holding the state
+ *			of the port.
+ * @enable:	The status used for show port is enabled or not.
+ * @pm:		The matrix used to show all connections with the port.
+ * @pvid:	The VLAN specified is to be considered a PVID at ingress.  Any
+ *		untagged frames will be assigned to the related VLAN.
+ * @vlan_filtering: The flags indicating whether the port that can recognize
+ *		    VLAN-tagged frames.
+ */
 struct mt7530_port {
 	bool enable;
 	u32 pm;
+	u16 pvid;
+	bool vlan_filtering;
 };
 
 /* struct mt7530_priv -	This is the main data structure for holding the state
@@ -382,6 +437,22 @@ struct mt7530_priv {
 	struct mutex reg_mutex;
 };
 
+struct mt7530_hw_vlan_entry {
+	int port;
+	u8  old_members;
+	bool untagged;
+};
+
+static inline void mt7530_hw_vlan_entry_init(struct mt7530_hw_vlan_entry *e,
+					     int port, bool untagged)
+{
+	e->port = port;
+	e->untagged = untagged;
+}
+
+typedef void (*mt7530_vlan_op)(struct mt7530_priv *,
+			       struct mt7530_hw_vlan_entry *);
+
 struct mt7530_hw_stats {
 	const char	*string;
 	u16		reg;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 net-next 0/3] add VLAN support to DSA MT7530
From: sean.wang @ 2017-12-13  4:01 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, vivien.didelot
  Cc: netdev, linux-kernel, linux-mediatek, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

Changes since v1:
- fix up the typo
- prefer ordering declarations longest to shortest
- update that vlan_prepare callback should not change any state
- use lower case letter for function naming

The patchset extends DSA MT7530 to VLAN support through filling required
callbacks in patch 1 and merging the special tag with VLAN tag in patch 2
for allowing that the hardware can handle these packets with VID from the
CPU port.

Sean Wang (3):
  net: dsa: mediatek: add VLAN support for MT7530
  net: dsa: mediatek: combine MediaTek tag with VLAN tag
  net: dsa: mediatek: update MAINTAINERS entry with MediaTek switch
    driver

 MAINTAINERS              |   7 ++
 drivers/net/dsa/mt7530.c | 291 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/mt7530.h |  83 +++++++++++++-
 net/dsa/tag_mtk.c        |  38 +++++--
 4 files changed, 403 insertions(+), 16 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [BUG] renesas/ravb: two possible sleep-in-atomic bugs in ravb_set_link_ksettings and ravb_nway_reset
From: Jia-Ju Bai @ 2017-12-13  3:43 UTC (permalink / raw)
  To: sergei.shtylyov, davem, horms+renesas, kazuya.mizuguchi.ks,
	niklas.soderlund+renesas, geert+renesas, erosca, yuval.shaia
  Cc: netdev, linux-renesas-soc,
	linux-kernel@vger.kernel.org >> Linux Kernel Mailing List

Accoring to drivers/net/ethernet/renesas/ravb_main.c, the driver may 
sleep under a spinlock.
The function call paths are:
ravb_set_link_ksettings (acquire the spinlock)
   phy_ethtool_ksettings_set
     phy_start_aneg
       phy_start_aneg_priv
         mutex_lock --> may sleep

ravb_nway_reset (acquire the spinlock)
   phy_start_aneg
     phy_start_aneg_priv
       mutex_lock --> may sleep

I do not find a good way to fix them, so I only report.
These possible bugs are found by my static analysis tool (DSAC) and 
checked by my code review.


Thanks,
Jia-Ju Bai

^ permalink raw reply

* [BUG] renesas/sh_eth: two possible sleep-in-atomic bugs in sh_eth_set_link_ksettings and sh_eth_nway_reset
From: Jia-Ju Bai @ 2017-12-13  3:39 UTC (permalink / raw)
  To: sergei.shtylyov, davem, niklas.soderlund+renesas, geert+renesas,
	laurent.pinchart, horms+renesas
  Cc: netdev, linux-renesas-soc, Linux Kernel Mailing List

Accoring to drivers/net/ethernet/renesas/sh_eth.c, the driver may sleep 
under a spinlock.
The function call paths are:
sh_eth_set_link_ksettings (acquire the spinlock)
   phy_ethtool_ksettings_set
     phy_start_aneg
       phy_start_aneg_priv
         mutex_lock --> may sleep

sh_eth_nway_reset (acquire the spinlock)
   phy_start_aneg
     phy_start_aneg_priv
       mutex_lock --> may sleep

I do not find a good way to fix them, so I only report.
These possible bugs are found by my static analysis tool (DSAC) and 
checked by my code review.


Thanks,
Jia-Ju Bai

^ permalink raw reply

* [BUG] b44: two possible sleep-in-atomic bugs in b44_set_link_ksettings and b44_ioctl
From: Jia-Ju Bai @ 2017-12-13  3:29 UTC (permalink / raw)
  To: michael.chan, hauke, David Miller, f.fainelli, opendmb,
	sathya.perla
  Cc: netdev, linux-kernel

The driver may sleep under a spinlock.
The function call paths are:
b44_set_link_ksettings (acquire the spinlock)
   phy_ethtool_ksettings_set
     phy_start_aneg
       phy_start_aneg_priv
         mutex_lock --> may sleep

b44_ioctl (acquire the spinlock)
   phy_mii_ioctl
     mdiobus_read
       mutex_lock --> may sleep

I do not find a good way to fix them, so I only report.
These possible bugs are found by my static analysis tool (DSAC) and 
checked by my code review.


Thanks,
Jia-Ju Bai

^ permalink raw reply

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Jakub Kicinski @ 2017-12-13  2:44 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171213015125.GC21978@ZenIV.linux.org.uk>

On Wed, 13 Dec 2017 01:51:25 +0000, Al Viro wrote:
> On Tue, Dec 12, 2017 at 05:35:28PM -0800, Jakub Kicinski wrote:
> 
> > It used to be __always_inline, but apparently LLVM/clang doesn't
> > propagate constants :(  
> > 
> > 4e59532541c8 ("nfp: don't depend on compiler constant propagation")  
> 
> Doesn't propagate constants or doesn't have exact same set of
> rules for __builtin_constant_p()?  IOW, if you dropped that
> BUILD_BUG_ON(), what would be left after optimizations?

Hm.  You're right.  It just doesn't recognize the parameter as constant
in __builtin_constant_p().  I haven't compiled the actual code, but
here is a trivial test:

18:36 ~$ clang --version
clang version 4.0.1 (tags/RELEASE_401/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

18:36 ~$ cat /tmp/test.c
#include <stdio.h>

static inline int test_thing(unsigned int a)
{
	printf("const: %d\n", __builtin_constant_p(a));
	if (a > 10)
		return a - 1;
	return a;
}

int main()
{
	printf("const: %d\n", __builtin_constant_p(0));
	return test_thing(0);
}
18:36 ~$ clang -g /tmp/test.c -O2 -Wall -W -o /tmp/test
18:36 ~$ /tmp/test 
const: 1
const: 0
18:36 ~$ objdump -S /tmp/test
...
00000000004004e0 <main>:
	return a;
}

int main()
{
	printf("const: %d\n", __builtin_constant_p(0));
  4004e0:	50                   	push   %rax
  4004e1:	bf a0 05 40 00       	mov    $0x4005a0,%edi
  4004e6:	be 01 00 00 00       	mov    $0x1,%esi
  4004eb:	31 c0                	xor    %eax,%eax
  4004ed:	e8 fe fe ff ff       	callq  4003f0 <printf@plt>
	printf("const: %d\n", __builtin_constant_p(a));
  4004f2:	bf a0 05 40 00       	mov    $0x4005a0,%edi
  4004f7:	31 f6                	xor    %esi,%esi
  4004f9:	31 c0                	xor    %eax,%eax
  4004fb:	e8 f0 fe ff ff       	callq  4003f0 <printf@plt>
	return test_thing(0);
  400500:	31 c0                	xor    %eax,%eax
  400502:	59                   	pop    %rcx
  400503:	c3                   	retq   
  400504:	66 2e 0f 1f 84 00 00 	nopw   %cs:0x0(%rax,%rax,1)
  40050b:	00 00 00 
  40050e:	66 90                	xchg   %ax,%ax

...

^ permalink raw reply

* Re: [PATCH net] tcp: refresh tcp_mstamp from timers callbacks
From: Soheil Hassas Yeganeh @ 2017-12-13  2:42 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: Eric Dumazet, David Miller, netdev, Mike Maloney
In-Reply-To: <CADVnQykx=sQt7iKW9jAZRKGAHA3fQEA-6wAiTDmcvwPO6r1Ngg@mail.gmail.com>

On Tue, Dec 12, 2017 at 9:26 PM, Neal Cardwell <ncardwell@google.com> wrote:
> On Tue, Dec 12, 2017 at 9:22 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Only the retransmit timer currently refreshes tcp_mstamp
>>
>> We should do the same for delayed acks and keepalives.
>>
>> Even if RFC 7323 does not request it, this is consistent to what linux
>> did in the past, when TS values were based on jiffies.
>>
>> Fixes: 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Soheil Hassas Yeganeh <soheil@google.com>
>> Cc: Mike Maloney <maloney@google.com>
>> Cc: Neal Cardwell <ncardwell@google.com>
>> ---
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
>
> Thanks, Eric!
>
> neal

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

This is a very nice catch! Thank you Eric!

^ permalink raw reply

* Re: [PATCH net] tcp: refresh tcp_mstamp from timers callbacks
From: Neal Cardwell @ 2017-12-13  2:26 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Soheil Hassas Yeganeh, Mike Maloney
In-Reply-To: <1513131772.25033.60.camel@gmail.com>

On Tue, Dec 12, 2017 at 9:22 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Only the retransmit timer currently refreshes tcp_mstamp
>
> We should do the same for delayed acks and keepalives.
>
> Even if RFC 7323 does not request it, this is consistent to what linux
> did in the past, when TS values were based on jiffies.
>
> Fixes: 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Mike Maloney <maloney@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> ---

Acked-by: Neal Cardwell <ncardwell@google.com>

Thanks, Eric!

neal

^ permalink raw reply

* [PATCH v4 iproute2 net-next] gre6: add collect metadata support
From: William Tu @ 2017-12-13  2:22 UTC (permalink / raw)
  To: netdev; +Cc: Daniel Borkmann

The patch adds 'external' option to support collect metadata
gre6 tunnel.  The 'external' keyword is already used to set the
device into collect metadata mode such as vxlan, geneve, ipip,
etc.  This patch extends support for ipv6 gre and gretap.
Example of L3 and L2 gre device:
bash:~# ip link add dev ip6gre123 type ip6gre external
bash:~# ip link add dev ip6gretap123 type ip6gretap external

Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
change in v4:
  - rephrase and add excluded options when 'external' is used.
change in v3:
  - explain a little more on man page about 'external'
change in v2:
  - remove "noexternal" in man page
---
 ip/link_gre6.c        | 55 ++++++++++++++++++++++++++++++++-------------------
 man/man8/ip-link.8.in | 17 ++++++++++++++++
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 0a82eaecf2cd..2cb46ca116d0 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -105,6 +105,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 	__u16 encapflags = TUNNEL_ENCAP_FLAG_CSUM6;
 	__u16 encapsport = 0;
 	__u16 encapdport = 0;
+	__u8 metadata = 0;
 	int len;
 	__u32 fwmark = 0;
 	__u32 erspan_idx = 0;
@@ -178,6 +179,9 @@ get_failed:
 		if (greinfo[IFLA_GRE_ENCAP_SPORT])
 			encapsport = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_SPORT]);
 
+		if (greinfo[IFLA_GRE_COLLECT_METADATA])
+			metadata = 1;
+
 		if (greinfo[IFLA_GRE_ENCAP_DPORT])
 			encapdport = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_DPORT]);
 
@@ -355,6 +359,8 @@ get_failed:
 			encapflags |= TUNNEL_ENCAP_FLAG_REMCSUM;
 		} else if (strcmp(*argv, "noencap-remcsum") == 0) {
 			encapflags &= ~TUNNEL_ENCAP_FLAG_REMCSUM;
+		} else if (strcmp(*argv, "external") == 0) {
+			metadata = 1;
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			NEXT_ARG();
 			if (strcmp(*argv, "inherit") == 0) {
@@ -388,26 +394,30 @@ get_failed:
 		argc--; argv++;
 	}
 
-	addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
-	addattr32(n, 1024, IFLA_GRE_OKEY, okey);
-	addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
-	addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
-	addattr_l(n, 1024, IFLA_GRE_LOCAL, &laddr, sizeof(laddr));
-	addattr_l(n, 1024, IFLA_GRE_REMOTE, &raddr, sizeof(raddr));
-	if (link)
-		addattr32(n, 1024, IFLA_GRE_LINK, link);
-	addattr_l(n, 1024, IFLA_GRE_TTL, &hop_limit, 1);
-	addattr_l(n, 1024, IFLA_GRE_ENCAP_LIMIT, &encap_limit, 1);
-	addattr_l(n, 1024, IFLA_GRE_FLOWINFO, &flowinfo, 4);
-	addattr32(n, 1024, IFLA_GRE_FLAGS, flags);
-	addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
-	if (erspan_idx != 0)
-		addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx);
-
-	addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
-	addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
-	addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
-	addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
+	if (!metadata) {
+		addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
+		addattr32(n, 1024, IFLA_GRE_OKEY, okey);
+		addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
+		addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
+		addattr_l(n, 1024, IFLA_GRE_LOCAL, &laddr, sizeof(laddr));
+		addattr_l(n, 1024, IFLA_GRE_REMOTE, &raddr, sizeof(raddr));
+		if (link)
+			addattr32(n, 1024, IFLA_GRE_LINK, link);
+		addattr_l(n, 1024, IFLA_GRE_TTL, &hop_limit, 1);
+		addattr_l(n, 1024, IFLA_GRE_ENCAP_LIMIT, &encap_limit, 1);
+		addattr_l(n, 1024, IFLA_GRE_FLOWINFO, &flowinfo, 4);
+		addattr32(n, 1024, IFLA_GRE_FLAGS, flags);
+		addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
+		if (erspan_idx != 0)
+			addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx);
+
+		addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
+		addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
+		addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
+		addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
+	} else {
+		addattr_l(n, 1024, IFLA_GRE_COLLECT_METADATA, NULL, 0);
+	}
 
 	return 0;
 }
@@ -426,6 +436,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (!tb)
 		return;
 
+	if (tb[IFLA_GRE_COLLECT_METADATA]) {
+		print_bool(PRINT_ANY, "collect_metadata", "external", true);
+		return;
+	}
+
 	if (tb[IFLA_GRE_FLAGS])
 		flags = rta_getattr_u32(tb[IFLA_GRE_FLAGS]);
 
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 0db2582e19f7..7b14b18a8dbf 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -768,6 +768,8 @@ the following additional arguments are supported:
 .BI "dscp inherit"
 ] [
 .BI dev " PHYS_DEV "
+] [
+.RB external
 ]
 
 .in +8
@@ -846,6 +848,21 @@ or
 .IR 00 ".." ff
 when tunneling non-IP packets. The default value is 00.
 
+.sp
+.RB external
+- make this tunnel externally controlled (or not, which is the default).
+In the kernel, this is referred to as collect metadata mode.  This flag is
+mutually exclusive with the
+.BR remote ,
+.BR local ,
+.BR seq ,
+.BR key,
+.BR csum,
+.BR hoplimit,
+.BR encaplimit,
+.BR flowlabel " and " tclass
+options.
+
 .in -8
 
 .TP
-- 
2.7.4

^ permalink raw reply related

* [PATCH net] tcp: refresh tcp_mstamp from timers callbacks
From: Eric Dumazet @ 2017-12-13  2:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Soheil Hassas Yeganeh, Neal Cardwell, Mike Maloney

From: Eric Dumazet <edumazet@google.com>

Only the retransmit timer currently refreshes tcp_mstamp

We should do the same for delayed acks and keepalives.

Even if RFC 7323 does not request it, this is consistent to what linux
did in the past, when TS values were based on jiffies.

Fixes: 385e20706fac ("tcp: use tp->tcp_mstamp in output path")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Mike Maloney <maloney@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_timer.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 16df6dd44b988a128d97df3a7953437499a216e8..968fda1983762e6d7c078a28ccfcbd9066788daf 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -264,6 +264,7 @@ void tcp_delack_timer_handler(struct sock *sk)
 			icsk->icsk_ack.pingpong = 0;
 			icsk->icsk_ack.ato      = TCP_ATO_MIN;
 		}
+		tcp_mstamp_refresh(tcp_sk(sk));
 		tcp_send_ack(sk);
 		__NET_INC_STATS(sock_net(sk), LINUX_MIB_DELAYEDACKS);
 	}
@@ -632,6 +633,7 @@ static void tcp_keepalive_timer (struct timer_list *t)
 		goto out;
 	}
 
+	tcp_mstamp_refresh(tp);
 	if (sk->sk_state == TCP_FIN_WAIT2 && sock_flag(sk, SOCK_DEAD)) {
 		if (tp->linger2 >= 0) {
 			const int tmo = tcp_fin_time(sk) - TCP_TIMEWAIT_LEN;

^ permalink raw reply related

* RE: [PATCH net-next v5 0/4] net: fec: fix refclk enable for SMSC LAN8710/20
From: Andy Duan @ 2017-12-13  2:21 UTC (permalink / raw)
  To: Richard Leitner, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	andrew-g2DYL2Zd6BY@public.gmane.org,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org,
	sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org,
	baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org,
	david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org,
	lukma-ynQEQJNshbs@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	richard.leitner-WcANXNA0UjBBDgjK7y7TUQ@public.gmane.org
In-Reply-To: <20171211121700.10200-1-dev-M/VWbR8SM2SsTnJN9+BGXg@public.gmane.org>

From: Richard Leitner <dev-M/VWbR8SM2SsTnJN9+BGXg@public.gmane.org> Sent: Monday, December 11, 2017 8:17 PM
>This patch series fixes the use of the SMSC LAN8710/20 with a Freescale ETH
>when the refclk is generated by the FSL.
>
>This patchset depends on the "phylib: Add device reset GPIO support" patch
>submitted by Geert Uytterhoeven/Sergei Shtylyov, which was merged to net-
>next as commit bafbdd527d56 ("phylib: Add device reset GPIO support").
>
>Changes v5:
>	- fix reset delay calculation (max_t instead of min_t)
>
>Changes v4:
>	- simplify dts parsing
>	- simplify reset delay evaluation and execution
>	- fec: ensure to only reset once during fec_enet_open()
>	- remove dependency notes from commit message
>	- add reviews and acks
>
>Changes v3:
>	- use phylib to hard-reset the PHY
>	- implement reset delays in phylib
>	- add new phylib API & flag (PHY_RST_AFTER_CLK_EN) to determine if
>	  a PHY is affected
>
>Changes v2:
>	- simplify and fix fec_reset_phy function to support multiple calls
>	- include: linux: phy: harmonize phy_id{,_mask} type
>	- reset the phy instead of not turning the clock on and off
>	  (which would have caused a power consumption regression)
>
>Richard Leitner (4):
>  phylib: Add device reset delay support
>  phylib: add reset after clk enable support
>  net: phy: smsc: LAN8710/20: add PHY_RST_AFTER_CLK_EN flag
>  net: fec: add phy_reset_after_clk_enable() support
>
> Documentation/devicetree/bindings/net/phy.txt | 10 ++++++++++
> drivers/net/ethernet/freescale/fec_main.c     | 20 ++++++++++++++++++++
> drivers/net/phy/mdio_device.c                 | 13 +++++++++++--
> drivers/net/phy/phy_device.c                  | 24 ++++++++++++++++++++++++
> drivers/net/phy/smsc.c                        |  2 +-
> drivers/of/of_mdio.c                          |  4 ++++
> include/linux/mdio.h                          |  2 ++
> include/linux/phy.h                           |  2 ++
> 8 files changed, 74 insertions(+), 3 deletions(-)
>
>--
>2.11.0

The series look fine better. Thanks.

Acked-by: Fugang Duan <fugang.duan-3arQi8VN3Tc@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 09/33] nds32: Cache and TLB routines
From: Guo Ren @ 2017-12-13  2:16 UTC (permalink / raw)
  To: Greentime Hu
  Cc: greentime, linux-kernel, arnd, linux-arch, tglx, jason,
	marc.zyngier, robh+dt, netdev, deanbo422, devicetree, viro,
	dhowells, will.deacon, daniel.lezcano, linux-serial,
	geert.uytterhoeven, linus.walleij, mark.rutland, greg,
	Vincent Chen
In-Reply-To: <9d0e976c3e60bf6834dffb6ef798ad2a03e1c9ac.1512723245.git.green.hu@gmail.com>

On Fri, Dec 08, 2017 at 05:11:52PM +0800, Greentime Hu wrote:
> From: Greentime Hu <greentime@andestech.com>
 [...]
> diff --git a/arch/nds32/mm/cacheflush.c b/arch/nds32/mm/cacheflush.c
 [...]
> +#ifndef CONFIG_CPU_CACHE_ALIASING
> +void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,
> +		      pte_t * pte)
 [...]
> +	if (vma->vm_mm == current->active_mm) {
> +
> +		__nds32__mtsr_dsb(addr, NDS32_SR_TLB_VPN);
> +		__nds32__tlbop_rwr(*pte);
> +		__nds32__isb();
If there is an interruption between "mtsr_dsb" and "tlbop_rwr" and a
update_mmu_cache() is invoked again, then an error page mapping is
set up in your tlb-buffer when tlbop_rwr is excuted from interrupt.
Because it's another addr in NDS32_SR_TLB_VPN.

It seems that tlb-hardrefill can help build tlb-buffer mapping, why you
update it in this software way?

 Guo Ren

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH v2 next-queue 08/10] ixgbe: process the Tx ipsec offload
From: Alexander Duyck @ 2017-12-13  1:59 UTC (permalink / raw)
  To: Shannon Nelson
  Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Netdev,
	Sowmini Varadhan
In-Reply-To: <1513121823-27944-9-git-send-email-shannon.nelson@oracle.com>

On Tue, Dec 12, 2017 at 3:37 PM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> If the skb has a security association referenced in the skb, then
> set up the Tx descriptor with the ipsec offload bits.  While we're
> here, we fix an oddly named field in the context descriptor struct.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---

So I was thinking about it and I have a pretty good idea of the things
that might be messed up with the combination of ipsec offload and TSO
or Tx checksum offload.

First in the case of TSO we need to not do the IP checksum update that
we do for GSO_PARTIAL. We could probably just wrap that code in a
check that says to do that only if we are performing a GSO_PARTIAL
type offload, or we just don't do it when performing an IPSEC
offfload.

Second we are reporting the wrong IPv4 header length. I am assuming
that the esp header doesn't occupy skb_checksum_start. As a result we
are computing the IPv4 header and including the AH/ESP header in it
and then writing that to vlan_macip_lens. What we may want to do is
just update the code so that if the TX_FLAGS_IPSEC is set we report an
IP length of 20, otherwise we use the computed value. As a result we
cannot do GSO_PARTIAL offload and IPSEC at the same time but I doubt
that would be a common combination.

> v2: use ihl != 5
>     move the ixgbe_ipsec_tx() call to near the call to ixgbe_tso()
>     drop the ipsec packet if the tx offload setup fails
>     simplify the ixgbe_ipsec_tx() parameters by using 'first'
>     leave out the ixgbe_tso() changes since we don't support TSO
>        with ipsec yet.
>
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h       | 10 +++-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 79 ++++++++++++++++++++++++++
>  drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c   |  4 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c  | 24 ++++++--
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h  |  2 +-
>  5 files changed, 110 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index a094b23..3d2b7bf 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -171,10 +171,11 @@ enum ixgbe_tx_flags {
>         IXGBE_TX_FLAGS_CC       = 0x08,
>         IXGBE_TX_FLAGS_IPV4     = 0x10,
>         IXGBE_TX_FLAGS_CSUM     = 0x20,
> +       IXGBE_TX_FLAGS_IPSEC    = 0x40,
>
>         /* software defined flags */
> -       IXGBE_TX_FLAGS_SW_VLAN  = 0x40,
> -       IXGBE_TX_FLAGS_FCOE     = 0x80,
> +       IXGBE_TX_FLAGS_SW_VLAN  = 0x80,
> +       IXGBE_TX_FLAGS_FCOE     = 0x100,
>  };
>
>  /* VLAN info */
> @@ -1014,6 +1015,8 @@ void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter);
>  void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
>                     union ixgbe_adv_rx_desc *rx_desc,
>                     struct sk_buff *skb);
> +int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct ixgbe_tx_buffer *first,
> +                  struct ixgbe_ipsec_tx_data *itd);
>  #else
>  static inline void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter) { };
>  static inline void ixgbe_stop_ipsec_offload(struct ixgbe_adapter *adapter) { };
> @@ -1021,5 +1024,8 @@ static inline void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter) { };
>  static inline void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
>                                   union ixgbe_adv_rx_desc *rx_desc,
>                                   struct sk_buff *skb) { };
> +static inline int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
> +                                struct ixgbe_tx_buffer *first,
> +                                struct ixgbe_ipsec_tx_data *itd) { return 0; };
>  #endif /* CONFIG_XFRM_OFFLOAD */
>  #endif /* _IXGBE_H_ */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index 7e421b8..5ed8a4f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -695,12 +695,91 @@ static void ixgbe_ipsec_del_sa(struct xfrm_state *xs)
>         }
>  }
>
> +/**
> + * ixgbe_ipsec_offload_ok - can this packet use the xfrm hw offload
> + * @skb: current data packet
> + * @xs: pointer to transformer state struct
> + **/
> +static bool ixgbe_ipsec_offload_ok(struct sk_buff *skb, struct xfrm_state *xs)
> +{
> +       if (xs->props.family == AF_INET) {
> +               /* Offload with IPv4 options is not supported yet */
> +               if (ip_hdr(skb)->ihl != 5)
> +                       return false;
> +       } else {
> +               /* Offload with IPv6 extension headers is not support yet */
> +               if (ipv6_ext_hdr(ipv6_hdr(skb)->nexthdr))
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
>  static const struct xfrmdev_ops ixgbe_xfrmdev_ops = {
>         .xdo_dev_state_add = ixgbe_ipsec_add_sa,
>         .xdo_dev_state_delete = ixgbe_ipsec_del_sa,
> +       .xdo_dev_offload_ok = ixgbe_ipsec_offload_ok,
>  };
>
>  /**
> + * ixgbe_ipsec_tx - setup Tx flags for ipsec offload
> + * @tx_ring: outgoing context
> + * @first: current data packet
> + * @itd: ipsec Tx data for later use in building context descriptor
> + **/
> +int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
> +                  struct ixgbe_tx_buffer *first,
> +                  struct ixgbe_ipsec_tx_data *itd)
> +{
> +       struct ixgbe_adapter *adapter = netdev_priv(tx_ring->netdev);
> +       struct ixgbe_ipsec *ipsec = adapter->ipsec;
> +       struct xfrm_state *xs;
> +       struct tx_sa *tsa;
> +
> +       if (!first->skb->sp->len) {
> +               netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
> +                          __func__, first->skb->sp->len);
> +               return 0;
> +       }
> +
> +       xs = xfrm_input_state(first->skb);
> +       if (!xs) {
> +               netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
> +                          __func__, xs);
> +               return 0;
> +       }
> +
> +       itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
> +       if (itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
> +               netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
> +                          __func__, itd->sa_idx, xs->xso.offload_handle);
> +               return 0;
> +       }
> +
> +       tsa = &ipsec->tx_tbl[itd->sa_idx];
> +       if (!tsa->used) {
> +               netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
> +                          __func__, itd->sa_idx);
> +               return 0;
> +       }
> +
> +       first->tx_flags |= IXGBE_TX_FLAGS_IPSEC | IXGBE_TX_FLAGS_CC;
> +
> +       itd->flags = 0;
> +       if (xs->id.proto == IPPROTO_ESP) {
> +               itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP |
> +                             IXGBE_ADVTXD_TUCMD_L4T_TCP;
> +               if (first->protocol == htons(ETH_P_IP))
> +                       itd->flags |= IXGBE_ADVTXD_TUCMD_IPV4;
> +               itd->trailer_len = xs->props.trailer_len;
> +       }

I still have some concerns about these bits. First I would say you
could move the TUCMD_IPV4 portion into the end of the tx_csum logic
just after the no_csum label. No point in carrying this separately if
we can just take care of this there. If it is safe here I would assume
it would be safe to perform generically.

Also you are still setting the TCP flag for these packets. I'm
wondering if the TCP flag is really needed. I get that your current
setup only runs on TCP flows but what about other types of flows?
Couldn't UDP be encapsulated via ESP?

> +       if (tsa->encrypt)
> +               itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN;
> +
> +       return 1;
> +}
> +
> +/**
>   * ixgbe_ipsec_rx - decode ipsec bits from Rx descriptor
>   * @rx_ring: receiving ring
>   * @rx_desc: receive data descriptor
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> index f1bfae0..d7875b3 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> @@ -1261,7 +1261,7 @@ void ixgbe_clear_interrupt_scheme(struct ixgbe_adapter *adapter)
>  }
>
>  void ixgbe_tx_ctxtdesc(struct ixgbe_ring *tx_ring, u32 vlan_macip_lens,
> -                      u32 fcoe_sof_eof, u32 type_tucmd, u32 mss_l4len_idx)
> +                      u32 fceof_saidx, u32 type_tucmd, u32 mss_l4len_idx)
>  {
>         struct ixgbe_adv_tx_context_desc *context_desc;
>         u16 i = tx_ring->next_to_use;
> @@ -1275,7 +1275,7 @@ void ixgbe_tx_ctxtdesc(struct ixgbe_ring *tx_ring, u32 vlan_macip_lens,
>         type_tucmd |= IXGBE_TXD_CMD_DEXT | IXGBE_ADVTXD_DTYP_CTXT;
>
>         context_desc->vlan_macip_lens   = cpu_to_le32(vlan_macip_lens);
> -       context_desc->seqnum_seed       = cpu_to_le32(fcoe_sof_eof);
> +       context_desc->fceof_saidx       = cpu_to_le32(fceof_saidx);
>         context_desc->type_tucmd_mlhl   = cpu_to_le32(type_tucmd);
>         context_desc->mss_l4len_idx     = cpu_to_le32(mss_l4len_idx);
>  }
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 0ee1e5e..84fbfb9 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -7756,10 +7756,12 @@ static inline bool ixgbe_ipv6_csum_is_sctp(struct sk_buff *skb)
>  }
>
>  static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
> -                         struct ixgbe_tx_buffer *first)
> +                         struct ixgbe_tx_buffer *first,
> +                         struct ixgbe_ipsec_tx_data *itd)
>  {
>         struct sk_buff *skb = first->skb;
>         u32 vlan_macip_lens = 0;
> +       u32 fceof_saidx = 0;
>         u32 type_tucmd = 0;
>
>         if (skb->ip_summed != CHECKSUM_PARTIAL) {
> @@ -7800,7 +7802,12 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
>         vlan_macip_lens |= skb_network_offset(skb) << IXGBE_ADVTXD_MACLEN_SHIFT;
>         vlan_macip_lens |= first->tx_flags & IXGBE_TX_FLAGS_VLAN_MASK;
>
> -       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, 0, type_tucmd, 0);
> +       if (first->tx_flags & IXGBE_TX_FLAGS_IPSEC) {
> +               fceof_saidx |= itd->sa_idx;
> +               type_tucmd |= itd->flags | itd->trailer_len;

These would all be 0 if the IPSEC flag isn't set, so I would say just
write them in all cases and avoid the conditional jump.

> +       }
> +
> +       ixgbe_tx_ctxtdesc(tx_ring, vlan_macip_lens, fceof_saidx, type_tucmd, 0);
>  }
>
>  #define IXGBE_SET_FLAG(_input, _flag, _result) \
> @@ -7843,11 +7850,16 @@ static void ixgbe_tx_olinfo_status(union ixgbe_adv_tx_desc *tx_desc,
>                                         IXGBE_TX_FLAGS_CSUM,
>                                         IXGBE_ADVTXD_POPTS_TXSM);
>
> -       /* enble IPv4 checksum for TSO */
> +       /* enable IPv4 checksum for TSO */
>         olinfo_status |= IXGBE_SET_FLAG(tx_flags,
>                                         IXGBE_TX_FLAGS_IPV4,
>                                         IXGBE_ADVTXD_POPTS_IXSM);
>
> +       /* enable IPsec */
> +       olinfo_status |= IXGBE_SET_FLAG(tx_flags,
> +                                       IXGBE_TX_FLAGS_IPSEC,
> +                                       IXGBE_ADVTXD_POPTS_IPSEC);
> +
>         /*
>          * Check Context must be set if Tx switch is enabled, which it
>          * always is for case where virtual functions are running
> @@ -8306,6 +8318,7 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>         u32 tx_flags = 0;
>         unsigned short f;
>         u16 count = TXD_USE_COUNT(skb_headlen(skb));
> +       struct ixgbe_ipsec_tx_data ipsec_tx = { 0 };
>         __be16 protocol = skb->protocol;
>         u8 hdr_len = 0;
>
> @@ -8410,11 +8423,14 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
>         }
>
>  #endif /* IXGBE_FCOE */
> +
> +       if (skb->sp && !ixgbe_ipsec_tx(tx_ring, first, &ipsec_tx))
> +               goto out_drop;
>         tso = ixgbe_tso(tx_ring, first, &hdr_len);
>         if (tso < 0)
>                 goto out_drop;
>         else if (!tso)
> -               ixgbe_tx_csum(tx_ring, first);
> +               ixgbe_tx_csum(tx_ring, first, &ipsec_tx);
>
>         /* add the ATR filter if ATR is on */
>         if (test_bit(__IXGBE_TX_FDIR_INIT_DONE, &tx_ring->state))
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index 3df0763..0ac725fa 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -2856,7 +2856,7 @@ union ixgbe_adv_rx_desc {
>  /* Context descriptors */
>  struct ixgbe_adv_tx_context_desc {
>         __le32 vlan_macip_lens;
> -       __le32 seqnum_seed;
> +       __le32 fceof_saidx;
>         __le32 type_tucmd_mlhl;
>         __le32 mss_l4len_idx;
>  };
> --
> 2.7.4
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply

* [PATCH AUTOSEL for 4.9 057/100] net: ipconfig: fix ic_close_devs() use-after-free
From: alexander.levin @ 2017-12-13  1:57 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
  Cc: Mark Rutland, Alexey Kuznetsov, David S . Miller,
	Hideaki YOSHIFUJI, James Morris, Patrick McHardy,
	netdev@vger.kernel.org, alexander.levin
In-Reply-To: <20171213015722.6722-1-alexander.levin@verizon.com>

From: Mark Rutland <mark.rutland@arm.com>

[ Upstream commit ffefb6f4d6ad699a2b5484241bc46745a53235d0 ]

Our chosen ic_dev may be anywhere in our list of ic_devs, and we may
free it before attempting to close others. When we compare d->dev and
ic_dev->dev, we're potentially dereferencing memory returned to the
allocator. This causes KASAN to scream for each subsequent ic_dev we
check.

As there's a 1-1 mapping between ic_devs and netdevs, we can instead
compare d and ic_dev directly, which implicitly handles the !ic_dev
case, and avoids the use-after-free. The ic_dev pointer may be stale,
but we will not dereference it.

Original splat:

[    6.487446] ==================================================================
[    6.494693] BUG: KASAN: use-after-free in ic_close_devs+0xc4/0x154 at addr ffff800367efa708
[    6.503013] Read of size 8 by task swapper/0/1
[    6.507452] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 4.11.0-rc3-00002-gda42158 #8
[    6.514993] Hardware name: AppliedMicro Mustang/Mustang, BIOS 3.05.05-beta_rc Jan 27 2016
[    6.523138] Call trace:
[    6.525590] [<ffff200008094778>] dump_backtrace+0x0/0x570
[    6.530976] [<ffff200008094d08>] show_stack+0x20/0x30
[    6.536017] [<ffff200008bee928>] dump_stack+0x120/0x188
[    6.541231] [<ffff20000856d5e4>] kasan_object_err+0x24/0xa0
[    6.546790] [<ffff20000856d924>] kasan_report_error+0x244/0x738
[    6.552695] [<ffff20000856dfec>] __asan_report_load8_noabort+0x54/0x80
[    6.559204] [<ffff20000aae86ac>] ic_close_devs+0xc4/0x154
[    6.564590] [<ffff20000aaedbac>] ip_auto_config+0x2ed4/0x2f1c
[    6.570321] [<ffff200008084b04>] do_one_initcall+0xcc/0x370
[    6.575882] [<ffff20000aa31de8>] kernel_init_freeable+0x5f8/0x6c4
[    6.581959] [<ffff20000a16df00>] kernel_init+0x18/0x190
[    6.587171] [<ffff200008084710>] ret_from_fork+0x10/0x40
[    6.592468] Object at ffff800367efa700, in cache kmalloc-128 size: 128
[    6.598969] Allocated:
[    6.601324] PID = 1
[    6.603427]  save_stack_trace_tsk+0x0/0x418
[    6.607603]  save_stack_trace+0x20/0x30
[    6.611430]  kasan_kmalloc+0xd8/0x188
[    6.615087]  ip_auto_config+0x8c4/0x2f1c
[    6.619002]  do_one_initcall+0xcc/0x370
[    6.622832]  kernel_init_freeable+0x5f8/0x6c4
[    6.627178]  kernel_init+0x18/0x190
[    6.630660]  ret_from_fork+0x10/0x40
[    6.634223] Freed:
[    6.636233] PID = 1
[    6.638334]  save_stack_trace_tsk+0x0/0x418
[    6.642510]  save_stack_trace+0x20/0x30
[    6.646337]  kasan_slab_free+0x88/0x178
[    6.650167]  kfree+0xb8/0x478
[    6.653131]  ic_close_devs+0x130/0x154
[    6.656875]  ip_auto_config+0x2ed4/0x2f1c
[    6.660875]  do_one_initcall+0xcc/0x370
[    6.664705]  kernel_init_freeable+0x5f8/0x6c4
[    6.669051]  kernel_init+0x18/0x190
[    6.672534]  ret_from_fork+0x10/0x40
[    6.676098] Memory state around the buggy address:
[    6.680880]  ffff800367efa600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    6.688078]  ffff800367efa680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[    6.695276] >ffff800367efa700: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[    6.702469]                       ^
[    6.705952]  ffff800367efa780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[    6.713149]  ffff800367efa800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[    6.720343] ==================================================================
[    6.727536] Disabling lock debugging due to kernel taint

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: David S. Miller <davem@davemloft.net>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: James Morris <jmorris@namei.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 net/ipv4/ipconfig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 071a785c65eb..b23464d9c538 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -306,7 +306,7 @@ static void __init ic_close_devs(void)
 	while ((d = next)) {
 		next = d->next;
 		dev = d->dev;
-		if ((!ic_dev || dev != ic_dev->dev) && !netdev_uses_dsa(dev)) {
+		if (d != ic_dev && !netdev_uses_dsa(dev)) {
 			pr_debug("IP-Config: Downing %s\n", dev->name);
 			dev_change_flags(dev, d->flags);
 		}
-- 
2.11.0

^ permalink raw reply related

* Re: [BUG] skge: a possible sleep-in-atomic bug in skge_remove
From: David Miller @ 2017-12-13  1:57 UTC (permalink / raw)
  To: stephen
  Cc: baijiaju1990, mlindner, shemminger, shemminger, netdev,
	linux-kernel
In-Reply-To: <20171212102240.4a09cf9a@xeon-e3>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 12 Dec 2017 10:22:40 -0800

> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Jia-Ju Bai <baijiaju1990@gmail.com>
>> Date: Tue, 12 Dec 2017 16:38:12 +0800
>> 
>> > According to drivers/net/ethernet/marvell/skge.c, the driver may sleep
>> > under a spinlock.
>> > The function call path is:
>> > skge_remove (acquire the spinlock)
>> >   free_irq --> may sleep
>> > 
>> > I do not find a good way to fix it, so I only report.
>> > This possible bug is found by my static analysis tool (DSAC) and
>> > checked by my code review.  
>> 
>> This was added by:
>> 
>> commit a9e9fd7182332d0cf5f3e601df3e71dd431b70d7
>> Author: Stephen Hemminger <shemminger@vyatta.com>
>> Date:   Tue Sep 27 13:41:37 2011 -0400
>> 
>>     skge: handle irq better on single port card
>> 
>> I think the free_irq() can be moved below the unlock.
>> 
>> Stephen, please take a look.
> 
> The IRQ was being free twice.
> How did you see it, I really doubt any multi-port SKGE cards
> still exist.

He sees it by reading the code, please take a look at this
and move the free_irq() out of the spin locked section since
it can sleep.

^ permalink raw reply

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Al Viro @ 2017-12-13  1:51 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171212173528.340cd002@cakuba.netronome.com>

On Tue, Dec 12, 2017 at 05:35:28PM -0800, Jakub Kicinski wrote:

> It used to be __always_inline, but apparently LLVM/clang doesn't
> propagate constants :(  
> 
> 4e59532541c8 ("nfp: don't depend on compiler constant propagation")

Doesn't propagate constants or doesn't have exact same set of
rules for __builtin_constant_p()?  IOW, if you dropped that
BUILD_BUG_ON(), what would be left after optimizations?

^ permalink raw reply

* Re: linux-next: build failure after merge of the bpf-next tree
From: Alexei Starovoitov @ 2017-12-13  1:47 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Daniel Borkmann, Alexei Starovoitov, Networking,
	Linux-Next Mailing List, Linux Kernel Mailing List, Yonghong Song
In-Reply-To: <20171213123245.44c86f3f@canb.auug.org.au>

On Wed, Dec 13, 2017 at 12:32:45PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the bpf-next tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
> 
> kernel/events/core.o: In function `perf_ioctl':
> core.c:(.text+0x98c4): undefined reference to `bpf_event_query_prog_array'
> 
> Caused by commit
> 
>   f371b304f12e ("bpf/tracing: allow user space to query prog array on the same tp")
> 
> CONFIG_BPF_SYSCALL is not set for this build.
> 
> I have used the bpf-next tree from next-20171212 for today.

Thanks for the report.
Yonghong, please take a look and send a followup fix based on bpf-next tree.

^ permalink raw reply

* Re: [PATCH v3 iproute2 net-next] gre6: add collect metadata support
From: William Tu @ 2017-12-13  1:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Linux Kernel Network Developers, Daniel Borkmann
In-Reply-To: <20171212174002.225f0a53@xeon-e3>

On Tue, Dec 12, 2017 at 5:40 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue, 12 Dec 2017 17:07:23 -0800
> William Tu <u9012063@gmail.com> wrote:
>
>> +.RB external
>> +- make this tunnel externally controlled (or not, which is the default).
>> +This is also known as collect_metadata mode for tunnels.
>
> Much better.
> The man page still needs some work.
>
> The phrase "is also known as" is a little confusing and putting a specific
> variable name in man page is misleading. Maybe better wording would
> be:
>
> In the kernel, this is referred to as collect metadata mode.
>
thanks! I will rephrase and submit another version.

^ permalink raw reply

* Re: [PATCH v3 iproute2 net-next] gre6: add collect metadata support
From: Stephen Hemminger @ 2017-12-13  1:40 UTC (permalink / raw)
  To: William Tu; +Cc: netdev, Daniel Borkmann
In-Reply-To: <1513127243-82244-1-git-send-email-u9012063@gmail.com>

On Tue, 12 Dec 2017 17:07:23 -0800
William Tu <u9012063@gmail.com> wrote:

> +.RB external
> +- make this tunnel externally controlled (or not, which is the default).
> +This is also known as collect_metadata mode for tunnels.

Much better.
The man page still needs some work.

The phrase "is also known as" is a little confusing and putting a specific
variable name in man page is misleading. Maybe better wording would
be:

In the kernel, this is referred to as collect metadata mode.

^ permalink raw reply

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Jakub Kicinski @ 2017-12-13  1:35 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171213013056.GB21978@ZenIV.linux.org.uk>

On Wed, 13 Dec 2017 01:30:56 +0000, Al Viro wrote:
> On Tue, Dec 12, 2017 at 05:04:37PM -0800, Jakub Kicinski wrote:
> > On Wed, 13 Dec 2017 00:36:59 +0000, Al Viro wrote:  
> > > On Tue, Dec 12, 2017 at 03:59:33PM -0800, Jakub Kicinski wrote:  
> > > > > +static __always_inline __##type type##_replace_bits(__##type old,	\
> > > > > +					base val, base mask)		\
> > > > > +{									\
> > > > > +	__##type m = to(mask);						\
> > > > > +        if (__builtin_constant_p(val) &&				\    
> > > > 
> > > > Is the lack of a __builtin_constant_p(mask) test intentional?  Sometimes
> > > > the bitfield is a packed array and people may have a helper to which
> > > > only the mask is passed as non-constant and the value is implied by the
> > > > helper, thus constant.    
> > > 
> > > If the mask in non-constant, we probably shouldn't be using that at all;
> > > could you show a real-world example where that would be the case?  
> > 
> > FIELD_* macros explicitly forbid this, since the code would be...
> > suboptimal with the runtime ffsl.  Real life examples are the hackish
> > macro NFP_ETH_SET_BIT_CONFIG() in
> > 
> > drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c  
> 
> Why not simply make nfp_eth_set_bit_config() static inline and be
> done with that?  It's not that large and there are only few callers,
> so...

It used to be __always_inline, but apparently LLVM/clang doesn't
propagate constants :(  

4e59532541c8 ("nfp: don't depend on compiler constant propagation")

> > I remember there was also some Renesas code..  maybe this:
> > 
> > https://patchwork.kernel.org/patch/9881279/
> > 
> > it looks like cpg_z_clk_recalc_rate() and cpg_z2_clk_recalc_rate() only
> > differ in mask.  
> 
> *shrug*
> 
> That thing would expand into "reg &= 15" in one case and "reg >>= 8; reg &= 15"
> in another.  Either of which is cheaper than a function call, and definitely
> cheaper than any kind of dynamic calculation of shift, no matter how implemented.

^ permalink raw reply

* linux-next: build failure after merge of the bpf-next tree
From: Stephen Rothwell @ 2017-12-13  1:32 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov, Networking
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Yonghong Song

Hi all,

After merging the bpf-next tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:

kernel/events/core.o: In function `perf_ioctl':
core.c:(.text+0x98c4): undefined reference to `bpf_event_query_prog_array'

Caused by commit

  f371b304f12e ("bpf/tracing: allow user space to query prog array on the same tp")

CONFIG_BPF_SYSCALL is not set for this build.

I have used the bpf-next tree from next-20171212 for today.

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Al Viro @ 2017-12-13  1:30 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171212170437.4b129e50@cakuba.netronome.com>

On Tue, Dec 12, 2017 at 05:04:37PM -0800, Jakub Kicinski wrote:
> On Wed, 13 Dec 2017 00:36:59 +0000, Al Viro wrote:
> > On Tue, Dec 12, 2017 at 03:59:33PM -0800, Jakub Kicinski wrote:
> > > > +static __always_inline __##type type##_replace_bits(__##type old,	\
> > > > +					base val, base mask)		\
> > > > +{									\
> > > > +	__##type m = to(mask);						\
> > > > +        if (__builtin_constant_p(val) &&				\  
> > > 
> > > Is the lack of a __builtin_constant_p(mask) test intentional?  Sometimes
> > > the bitfield is a packed array and people may have a helper to which
> > > only the mask is passed as non-constant and the value is implied by the
> > > helper, thus constant.  
> > 
> > If the mask in non-constant, we probably shouldn't be using that at all;
> > could you show a real-world example where that would be the case?
> 
> FIELD_* macros explicitly forbid this, since the code would be...
> suboptimal with the runtime ffsl.  Real life examples are the hackish
> macro NFP_ETH_SET_BIT_CONFIG() in
> 
> drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c

Why not simply make nfp_eth_set_bit_config() static inline and be
done with that?  It's not that large and there are only few callers,
so...

> I remember there was also some Renesas code..  maybe this:
> 
> https://patchwork.kernel.org/patch/9881279/
> 
> it looks like cpg_z_clk_recalc_rate() and cpg_z2_clk_recalc_rate() only
> differ in mask.

*shrug*

That thing would expand into "reg &= 15" in one case and "reg >>= 8; reg &= 15"
in another.  Either of which is cheaper than a function call, and definitely
cheaper than any kind of dynamic calculation of shift, no matter how implemented.

^ permalink raw reply

* Re: [PATCH net-next 1/3] net:tracepoint: replace tcp_set_state tracepoint with sock_set_state tracepoint
From: Song Liu @ 2017-12-13  1:19 UTC (permalink / raw)
  To: Yafang Shao
  Cc: David Miller, marcelo.leitner@gmail.com, edumazet@google.com,
	Cong Wang, mingo@redhat.com, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org, Steven Rostedt, Brendan Gregg,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1512919904-14166-2-git-send-email-laoar.shao@gmail.com>


> On Dec 10, 2017, at 7:31 AM, Yafang Shao <laoar.shao@gmail.com> wrote:
> 
> As sk_state is a common field for struct sock, so the state
> transition should not be a TCP specific feature.
> So I rename tcp_set_state tracepoint to sock_set_state tracepoint with
> some minor changes and move it into file trace/events/sock.h.
> 
> The minor changes against on the original tcp_set_state tracepoint:
> - Protocol name is printed to distinguish which protocol it is belonging
>  to
> - The macros defined in the file are undefed at the end of this file as
>  they are only used in this file
> 
> Two helpers are introduced to trace sk_state transition
> - void sk_state_store(struct sock *sk, int state);
> - void sk_set_state(struct sock *sk, int state);
> 
> As trace header should not be included in other header files,
> so they are defined in sock.c.
> 
> The protocol such as SCTP maybe compiled as a ko, hence export
> sk_set_state().
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> include/net/sock.h              | 15 ++-----
> include/trace/events/sock.h     | 95 +++++++++++++++++++++++++++++++++++++++++
> include/trace/events/tcp.h      | 76 ---------------------------------
> net/core/sock.c                 | 13 ++++++
> net/ipv4/inet_connection_sock.c |  4 +-
> net/ipv4/inet_hashtables.c      |  2 +-
> net/ipv4/tcp.c                  |  4 --
> 7 files changed, 114 insertions(+), 95 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 79e1a2c..b307b60 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2348,18 +2348,9 @@ static inline int sk_state_load(const struct sock *sk)
> 	return smp_load_acquire(&sk->sk_state);
> }
> 
> -/**
> - * sk_state_store - update sk->sk_state
> - * @sk: socket pointer
> - * @newstate: new state
> - *
> - * Paired with sk_state_load(). Should be used in contexts where
> - * state change might impact lockless readers.
> - */
> -static inline void sk_state_store(struct sock *sk, int newstate)
> -{
> -	smp_store_release(&sk->sk_state, newstate);
> -}
> +/* For sock_set_state tracepoint */
> +void sk_state_store(struct sock *sk, int newstate);
> +void sk_set_state(struct sock *sk, int state);
> 
> void sock_enable_timestamp(struct sock *sk, int flag);
> int sock_get_timestamp(struct sock *, struct timeval __user *);
> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
> index ec4dade..2728892 100644
> --- a/include/trace/events/sock.h
> +++ b/include/trace/events/sock.h
> @@ -6,7 +6,33 @@
> #define _TRACE_SOCK_H
> 
> #include <net/sock.h>
> +#include <net/ipv6.h>
> #include <linux/tracepoint.h>
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +
> +#define inet_protocol_name(protocol) {IPPROTO_##protocol, #protocol}
> +#define show_inet_protocol_name(val)	\
> +	__print_symbolic(val,	\
> +	inet_protocol_name(TCP),	\
> +	inet_protocol_name(DCCP),	\
> +	inet_protocol_name(SCTP))
> +
> +#define tcp_state_name(state)   { state, #state }
> +#define show_tcp_state_name(val)			\
> +		__print_symbolic(val,			\
> +		tcp_state_name(TCP_ESTABLISHED),	\
> +		tcp_state_name(TCP_SYN_SENT),	\
> +		tcp_state_name(TCP_SYN_RECV),	\
> +		tcp_state_name(TCP_FIN_WAIT1),	\
> +		tcp_state_name(TCP_FIN_WAIT2),	\
> +		tcp_state_name(TCP_TIME_WAIT),	\
> +		tcp_state_name(TCP_CLOSE),	\
> +		tcp_state_name(TCP_CLOSE_WAIT),	\
> +		tcp_state_name(TCP_LAST_ACK),	\
> +		tcp_state_name(TCP_LISTEN),	\
> +		tcp_state_name(TCP_CLOSING),	\
> +		tcp_state_name(TCP_NEW_SYN_RECV))

Please include Steven's fix to export names to user space: 

https://patchwork.kernel.org/patch/10052201/


> TRACE_EVENT(sock_rcvqueue_full,
> 
> @@ -63,6 +89,75 @@
> 		__entry->rmem_alloc)
> );
> 
> +TRACE_EVENT(sock_set_state,
> +
> +	TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
> +
> +	TP_ARGS(sk, oldstate, newstate),
> +
> +	TP_STRUCT__entry(
> +		__field(const void *, skaddr)
> +		__field(int, oldstate)
> +		__field(int, newstate)
> +		__field(__u16, sport)
> +		__field(__u16, dport)
> +		__field(__u8, protocol);
> +		__array(__u8, saddr, 4)
> +		__array(__u8, daddr, 4)
> +		__array(__u8, saddr_v6, 16)
> +		__array(__u8, daddr_v6, 16)
> +	),
> +
> +	TP_fast_assign(
> +		struct inet_sock *inet = inet_sk(sk);
> +		struct in6_addr *pin6;
> +		__be32 *p32;
> +
> +		__entry->skaddr = sk;
> +		__entry->oldstate = oldstate;
> +		__entry->newstate = newstate;
> +
> +		__entry->protocol = sk->sk_protocol;
> +		__entry->sport = ntohs(inet->inet_sport);
> +		__entry->dport = ntohs(inet->inet_dport);
> +
> +		p32 = (__be32 *) __entry->saddr;
> +		*p32 = inet->inet_saddr;
> +
> +		p32 = (__be32 *) __entry->daddr;
> +		*p32 =  inet->inet_daddr;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +		if (sk->sk_family == AF_INET6) {
> +			pin6 = (struct in6_addr *)__entry->saddr_v6;
> +			*pin6 = sk->sk_v6_rcv_saddr;
> +			pin6 = (struct in6_addr *)__entry->daddr_v6;
> +			*pin6 = sk->sk_v6_daddr;
> +		} else
> +#endif
> +		{
> +			pin6 = (struct in6_addr *)__entry->saddr_v6;
> +			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> +			pin6 = (struct in6_addr *)__entry->daddr_v6;
> +			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> +		}
> +	),
> +
> +	TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4"
> +				"saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> +		show_inet_protocol_name(__entry->protocol),
> +		__entry->sport, __entry->dport,
> +		__entry->saddr, __entry->daddr,
> +		__entry->saddr_v6, __entry->daddr_v6,
> +		show_tcp_state_name(__entry->oldstate),
> +		show_tcp_state_name(__entry->newstate))
> +);
> +
> +#undef show_tcp_state_name
> +#undef tcp_state_name
> +#undef show_inet_protocol_name
> +#undef inet_protocol_name
> +
> #endif /* _TRACE_SOCK_H */
> 
> /* This part must be outside protection */
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 07cccca..7399399 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -9,22 +9,6 @@
> #include <linux/tracepoint.h>
> #include <net/ipv6.h>
> 
> -#define tcp_state_name(state)	{ state, #state }
> -#define show_tcp_state_name(val)			\
> -	__print_symbolic(val,				\
> -		tcp_state_name(TCP_ESTABLISHED),	\
> -		tcp_state_name(TCP_SYN_SENT),		\
> -		tcp_state_name(TCP_SYN_RECV),		\
> -		tcp_state_name(TCP_FIN_WAIT1),		\
> -		tcp_state_name(TCP_FIN_WAIT2),		\
> -		tcp_state_name(TCP_TIME_WAIT),		\
> -		tcp_state_name(TCP_CLOSE),		\
> -		tcp_state_name(TCP_CLOSE_WAIT),		\
> -		tcp_state_name(TCP_LAST_ACK),		\
> -		tcp_state_name(TCP_LISTEN),		\
> -		tcp_state_name(TCP_CLOSING),		\
> -		tcp_state_name(TCP_NEW_SYN_RECV))
> -
> /*
>  * tcp event with arguments sk and skb
>  *
> @@ -177,66 +161,6 @@
> 	TP_ARGS(sk)
> );
> 
> -TRACE_EVENT(tcp_set_state,
> -
> -	TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
> -
> -	TP_ARGS(sk, oldstate, newstate),
> -
> -	TP_STRUCT__entry(
> -		__field(const void *, skaddr)
> -		__field(int, oldstate)
> -		__field(int, newstate)
> -		__field(__u16, sport)
> -		__field(__u16, dport)
> -		__array(__u8, saddr, 4)
> -		__array(__u8, daddr, 4)
> -		__array(__u8, saddr_v6, 16)
> -		__array(__u8, daddr_v6, 16)
> -	),
> -
> -	TP_fast_assign(
> -		struct inet_sock *inet = inet_sk(sk);
> -		struct in6_addr *pin6;
> -		__be32 *p32;
> -
> -		__entry->skaddr = sk;
> -		__entry->oldstate = oldstate;
> -		__entry->newstate = newstate;
> -
> -		__entry->sport = ntohs(inet->inet_sport);
> -		__entry->dport = ntohs(inet->inet_dport);
> -
> -		p32 = (__be32 *) __entry->saddr;
> -		*p32 = inet->inet_saddr;
> -
> -		p32 = (__be32 *) __entry->daddr;
> -		*p32 =  inet->inet_daddr;
> -
> -#if IS_ENABLED(CONFIG_IPV6)
> -		if (sk->sk_family == AF_INET6) {
> -			pin6 = (struct in6_addr *)__entry->saddr_v6;
> -			*pin6 = sk->sk_v6_rcv_saddr;
> -			pin6 = (struct in6_addr *)__entry->daddr_v6;
> -			*pin6 = sk->sk_v6_daddr;
> -		} else
> -#endif
> -		{
> -			pin6 = (struct in6_addr *)__entry->saddr_v6;
> -			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> -			pin6 = (struct in6_addr *)__entry->daddr_v6;
> -			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> -		}
> -	),
> -
> -	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> -		  __entry->sport, __entry->dport,
> -		  __entry->saddr, __entry->daddr,
> -		  __entry->saddr_v6, __entry->daddr_v6,
> -		  show_tcp_state_name(__entry->oldstate),
> -		  show_tcp_state_name(__entry->newstate))
> -);
> -
> TRACE_EVENT(tcp_retransmit_synack,
> 
> 	TP_PROTO(const struct sock *sk, const struct request_sock *req),
> diff --git a/net/core/sock.c b/net/core/sock.c
> index c0b5b2f..ee0c1bc 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2859,6 +2859,19 @@ int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
> }
> EXPORT_SYMBOL(sock_get_timestampns);
> 
> +void sk_state_store(struct sock *sk, int state)
> +{
> +	trace_sock_set_state(sk, sk->sk_state, state);
> +	smp_store_release(&sk->sk_state, state);
> +}
> +
> +void sk_set_state(struct sock *sk, int state)
> +{
> +	trace_sock_set_state(sk, sk->sk_state, state);
> +	sk->sk_state = state;
> +}
> +EXPORT_SYMBOL(sk_set_state);

These two functions are short. Can we keep this in sock.h?

Thanks,
Song

> +
> void sock_enable_timestamp(struct sock *sk, int flag)
> {
> 	if (!sock_flag(sk, flag)) {
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 4ca46dc..001f7b0 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -783,7 +783,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
> 	if (newsk) {
> 		struct inet_connection_sock *newicsk = inet_csk(newsk);
> 
> -		newsk->sk_state = TCP_SYN_RECV;
> +		sk_set_state(newsk, TCP_SYN_RECV);
> 		newicsk->icsk_bind_hash = NULL;
> 
> 		inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
> @@ -888,7 +888,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
> 			return 0;
> 	}
> 
> -	sk->sk_state = TCP_CLOSE;
> +	sk_set_state(sk, TCP_CLOSE);
> 	return err;
> }
> EXPORT_SYMBOL_GPL(inet_csk_listen_start);
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index f6f5810..5973693 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -544,7 +544,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
> 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
> 	} else {
> 		percpu_counter_inc(sk->sk_prot->orphan_count);
> -		sk->sk_state = TCP_CLOSE;
> +		sk_set_state(sk, TCP_CLOSE);
> 		sock_set_flag(sk, SOCK_DEAD);
> 		inet_csk_destroy_sock(sk);
> 	}
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1803116..000504f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -283,8 +283,6 @@
> #include <asm/ioctls.h>
> #include <net/busy_poll.h>
> 
> -#include <trace/events/tcp.h>
> -
> struct percpu_counter tcp_orphan_count;
> EXPORT_SYMBOL_GPL(tcp_orphan_count);
> 
> @@ -2040,8 +2038,6 @@ void tcp_set_state(struct sock *sk, int state)
> {
> 	int oldstate = sk->sk_state;
> 
> -	trace_tcp_set_state(sk, oldstate, state);
> -
> 	switch (state) {
> 	case TCP_ESTABLISHED:
> 		if (oldstate != TCP_ESTABLISHED)
> --
> 1.8.3.1
> 

^ permalink raw reply

* [PATCH v3 iproute2 net-next] gre6: add collect metadata support
From: William Tu @ 2017-12-13  1:07 UTC (permalink / raw)
  To: netdev; +Cc: Daniel Borkmann

The patch adds 'external' option to support collect metadata
gre6 tunnel.  The 'external' keyword is already used to set the
device into collect metadata mode such as vxlan, geneve, ipip,
etc.  This patch extends support for ipv6 gre and gretap.
Examples of L3 and L2 gre device:
bash:~# ip link add dev ip6gre123 type ip6gre external
bash:~# ip link add dev ip6gretap123 type ip6gretap external

Signed-off-by: William Tu <u9012063@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
change in v3:
  - explain a little more on man page about 'external'
change in v2:
  - remove "noexternal" in man page
---
 ip/link_gre6.c        | 55 ++++++++++++++++++++++++++++++++-------------------
 man/man8/ip-link.8.in |  7 +++++++
 2 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 0a82eaecf2cd..2cb46ca116d0 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -105,6 +105,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
 	__u16 encapflags = TUNNEL_ENCAP_FLAG_CSUM6;
 	__u16 encapsport = 0;
 	__u16 encapdport = 0;
+	__u8 metadata = 0;
 	int len;
 	__u32 fwmark = 0;
 	__u32 erspan_idx = 0;
@@ -178,6 +179,9 @@ get_failed:
 		if (greinfo[IFLA_GRE_ENCAP_SPORT])
 			encapsport = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_SPORT]);
 
+		if (greinfo[IFLA_GRE_COLLECT_METADATA])
+			metadata = 1;
+
 		if (greinfo[IFLA_GRE_ENCAP_DPORT])
 			encapdport = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_DPORT]);
 
@@ -355,6 +359,8 @@ get_failed:
 			encapflags |= TUNNEL_ENCAP_FLAG_REMCSUM;
 		} else if (strcmp(*argv, "noencap-remcsum") == 0) {
 			encapflags &= ~TUNNEL_ENCAP_FLAG_REMCSUM;
+		} else if (strcmp(*argv, "external") == 0) {
+			metadata = 1;
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			NEXT_ARG();
 			if (strcmp(*argv, "inherit") == 0) {
@@ -388,26 +394,30 @@ get_failed:
 		argc--; argv++;
 	}
 
-	addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
-	addattr32(n, 1024, IFLA_GRE_OKEY, okey);
-	addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
-	addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
-	addattr_l(n, 1024, IFLA_GRE_LOCAL, &laddr, sizeof(laddr));
-	addattr_l(n, 1024, IFLA_GRE_REMOTE, &raddr, sizeof(raddr));
-	if (link)
-		addattr32(n, 1024, IFLA_GRE_LINK, link);
-	addattr_l(n, 1024, IFLA_GRE_TTL, &hop_limit, 1);
-	addattr_l(n, 1024, IFLA_GRE_ENCAP_LIMIT, &encap_limit, 1);
-	addattr_l(n, 1024, IFLA_GRE_FLOWINFO, &flowinfo, 4);
-	addattr32(n, 1024, IFLA_GRE_FLAGS, flags);
-	addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
-	if (erspan_idx != 0)
-		addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx);
-
-	addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
-	addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
-	addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
-	addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
+	if (!metadata) {
+		addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
+		addattr32(n, 1024, IFLA_GRE_OKEY, okey);
+		addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
+		addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
+		addattr_l(n, 1024, IFLA_GRE_LOCAL, &laddr, sizeof(laddr));
+		addattr_l(n, 1024, IFLA_GRE_REMOTE, &raddr, sizeof(raddr));
+		if (link)
+			addattr32(n, 1024, IFLA_GRE_LINK, link);
+		addattr_l(n, 1024, IFLA_GRE_TTL, &hop_limit, 1);
+		addattr_l(n, 1024, IFLA_GRE_ENCAP_LIMIT, &encap_limit, 1);
+		addattr_l(n, 1024, IFLA_GRE_FLOWINFO, &flowinfo, 4);
+		addattr32(n, 1024, IFLA_GRE_FLAGS, flags);
+		addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
+		if (erspan_idx != 0)
+			addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx);
+
+		addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
+		addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
+		addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
+		addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
+	} else {
+		addattr_l(n, 1024, IFLA_GRE_COLLECT_METADATA, NULL, 0);
+	}
 
 	return 0;
 }
@@ -426,6 +436,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	if (!tb)
 		return;
 
+	if (tb[IFLA_GRE_COLLECT_METADATA]) {
+		print_bool(PRINT_ANY, "collect_metadata", "external", true);
+		return;
+	}
+
 	if (tb[IFLA_GRE_FLAGS])
 		flags = rta_getattr_u32(tb[IFLA_GRE_FLAGS]);
 
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 0db2582e19f7..4d954b31d438 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -768,6 +768,8 @@ the following additional arguments are supported:
 .BI "dscp inherit"
 ] [
 .BI dev " PHYS_DEV "
+] [
+.RB external
 ]
 
 .in +8
@@ -846,6 +848,11 @@ or
 .IR 00 ".." ff
 when tunneling non-IP packets. The default value is 00.
 
+.sp
+.RB external
+- make this tunnel externally controlled (or not, which is the default).
+This is also known as collect_metadata mode for tunnels.
+
 .in -8
 
 .TP
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Jakub Kicinski @ 2017-12-13  1:04 UTC (permalink / raw)
  To: Al Viro; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171213003659.GA21978@ZenIV.linux.org.uk>

On Wed, 13 Dec 2017 00:36:59 +0000, Al Viro wrote:
> On Tue, Dec 12, 2017 at 03:59:33PM -0800, Jakub Kicinski wrote:
> > > +static __always_inline __##type type##_replace_bits(__##type old,	\
> > > +					base val, base mask)		\
> > > +{									\
> > > +	__##type m = to(mask);						\
> > > +        if (__builtin_constant_p(val) &&				\  
> > 
> > Is the lack of a __builtin_constant_p(mask) test intentional?  Sometimes
> > the bitfield is a packed array and people may have a helper to which
> > only the mask is passed as non-constant and the value is implied by the
> > helper, thus constant.  
> 
> If the mask in non-constant, we probably shouldn't be using that at all;
> could you show a real-world example where that would be the case?

FIELD_* macros explicitly forbid this, since the code would be...
suboptimal with the runtime ffsl.  Real life examples are the hackish
macro NFP_ETH_SET_BIT_CONFIG() in

drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c

I remember there was also some Renesas code..  maybe this:

https://patchwork.kernel.org/patch/9881279/

it looks like cpg_z_clk_recalc_rate() and cpg_z2_clk_recalc_rate() only
differ in mask.

^ 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