Netdev List
 help / color / mirror / Atom feed
* [PATCH next-queue 2/8] ixgbe: prep ipsec constants for later use
From: Shannon Nelson @ 2018-08-13 18:43 UTC (permalink / raw)
  To: intel-wired-lan, jeffrey.t.kirsher; +Cc: steffen.klassert, netdev
In-Reply-To: <1534185825-12451-1-git-send-email-shannon.nelson@oracle.com>

Pull out a couple of values from a function so they can be used
later elsewhere.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 4340651..3afb1fe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -5,6 +5,9 @@
 #include <net/xfrm.h>
 #include <crypto/aead.h>
 
+#define IXGBE_IPSEC_KEY_BITS  160
+static const char aes_gcm_name[] = "rfc4106(gcm(aes))";
+
 /**
  * ixgbe_ipsec_set_tx_sa - set the Tx SA registers
  * @hw: hw specific details
@@ -407,7 +410,6 @@ static int ixgbe_ipsec_parse_proto_keys(struct xfrm_state *xs,
 	struct net_device *dev = xs->xso.dev;
 	unsigned char *key_data;
 	char *alg_name = NULL;
-	const char aes_gcm_name[] = "rfc4106(gcm(aes))";
 	int key_len;
 
 	if (!xs->aead) {
@@ -435,9 +437,9 @@ static int ixgbe_ipsec_parse_proto_keys(struct xfrm_state *xs,
 	 * we don't need to do any byteswapping.
 	 * 160 accounts for 16 byte key and 4 byte salt
 	 */
-	if (key_len == 160) {
+	if (key_len == IXGBE_IPSEC_KEY_BITS) {
 		*mysalt = ((u32 *)key_data)[4];
-	} else if (key_len != 128) {
+	} else if (key_len != (IXGBE_IPSEC_KEY_BITS - (sizeof(*mysalt) * 8))) {
 		netdev_err(dev, "IPsec hw offload only supports keys up to 128 bits with a 32 bit salt\n");
 		return -EINVAL;
 	} else {
-- 
2.7.4

^ permalink raw reply related

* [PATCH next-queue 4/8] ixgbe: add VF IPsec offload enable flag
From: Shannon Nelson @ 2018-08-13 18:43 UTC (permalink / raw)
  To: intel-wired-lan, jeffrey.t.kirsher; +Cc: steffen.klassert, netdev
In-Reply-To: <1534185825-12451-1-git-send-email-shannon.nelson@oracle.com>

Add a private flag to expressly enable support for VF IPsec offload.
The VF will have to be "trusted" in order to use the hardware offload,
but because of the general concerns of managing VF access, we want to
be sure the user specifically is enabling the feature.

This is likely a candidate for becoming a netdev feature flag.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h         | 1 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 9 +++++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c   | 3 ++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 4fc906c..89e709c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -605,6 +605,7 @@ struct ixgbe_adapter {
 #define IXGBE_FLAG2_EEE_ENABLED			BIT(15)
 #define IXGBE_FLAG2_RX_LEGACY			BIT(16)
 #define IXGBE_FLAG2_IPSEC_ENABLED		BIT(17)
+#define IXGBE_FLAG2_VF_IPSEC_ENABLED		BIT(18)
 
 	/* Tx fast path data */
 	int num_tx_queues;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index e5a8461..732b1e6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -136,6 +136,8 @@ static const char ixgbe_gstrings_test[][ETH_GSTRING_LEN] = {
 static const char ixgbe_priv_flags_strings[][ETH_GSTRING_LEN] = {
 #define IXGBE_PRIV_FLAGS_LEGACY_RX	BIT(0)
 	"legacy-rx",
+#define IXGBE_PRIV_FLAGS_VF_IPSEC_EN	BIT(1)
+	"vf-ipsec",
 };
 
 #define IXGBE_PRIV_FLAGS_STR_LEN ARRAY_SIZE(ixgbe_priv_flags_strings)
@@ -3409,6 +3411,9 @@ static u32 ixgbe_get_priv_flags(struct net_device *netdev)
 	if (adapter->flags2 & IXGBE_FLAG2_RX_LEGACY)
 		priv_flags |= IXGBE_PRIV_FLAGS_LEGACY_RX;
 
+	if (adapter->flags2 & IXGBE_FLAG2_VF_IPSEC_ENABLED)
+		priv_flags |= IXGBE_PRIV_FLAGS_VF_IPSEC_EN;
+
 	return priv_flags;
 }
 
@@ -3421,6 +3426,10 @@ static int ixgbe_set_priv_flags(struct net_device *netdev, u32 priv_flags)
 	if (priv_flags & IXGBE_PRIV_FLAGS_LEGACY_RX)
 		flags2 |= IXGBE_FLAG2_RX_LEGACY;
 
+	flags2 &= ~IXGBE_FLAG2_VF_IPSEC_ENABLED;
+	if (priv_flags & IXGBE_PRIV_FLAGS_VF_IPSEC_EN)
+		flags2 |= IXGBE_FLAG2_VF_IPSEC_ENABLED;
+
 	if (flags2 != adapter->flags2) {
 		adapter->flags2 = flags2;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 80108e1..ecd01fa 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -880,7 +880,8 @@ int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
 	int err;
 
 	sam = (struct sa_mbx_msg *)(&msgbuf[1]);
-	if (!adapter->vfinfo[vf].trusted) {
+	if (!adapter->vfinfo[vf].trusted ||
+	    !(adapter->flags2 & IXGBE_FLAG2_VF_IPSEC_ENABLED)) {
 		e_warn(drv, "VF %d attempted to add an IPsec SA\n", vf);
 		err = -EACCES;
 		goto err_out;
-- 
2.7.4

^ permalink raw reply related

* [PATCH next-queue 0/8] ixgbe/ixgbevf: IPsec offload support for VFs
From: Shannon Nelson @ 2018-08-13 18:43 UTC (permalink / raw)
  To: intel-wired-lan, jeffrey.t.kirsher; +Cc: steffen.klassert, netdev

This set of patches implements IPsec hardware offload for VF devices in
Intel's 10Gbe x540 family of Ethernet devices.

The IPsec HW offload feature has been in the x540/Niantic family of
network devices since their release in 2009, but there was no Linux
kernel support for the offload until 2017.  After the XFRM code added
support for the offload last year, the hw offload was added to the ixgbe
PF driver.

Since the related x540 VF device uses same setup as the PF for implementing
the offload, adding the feature to the ixgbevf seemed like a good idea.
In this case, the PF owns the device registers, so the VF simply packages
up the request information into a VF<->PF message and the PF does the
device configuration.  The resulting IPsec throughput is roughly equivalent
to what we see in the PF - nearly line-rate, with the expected drop in CPU
cycles burned.  (I'm not great at performance statistics, I'll let better
folks do the actual measurements as they pertain to their own usage)

To make use of the capability, first two things are needed: the PF must
be told to enable the offload for VFs (it is off by default) and the VF
must be trusted.  A new ethtool priv-flag for ixgbe is added to control
VF offload support.  For example:

	ethtool --set-priv-flags eth0 vf-ipsec on
	ip link set eth0 vf 1 trust on

Once those are set up and the VF device is UP, the user can add SAs the
same as for PFs, whether the VF is in the host or has been assigned to
a VM.

Note that the x540 chip supports a total of 1024 Rx plus 1024 Tx Security
Associations (SAs), shared among the PF and VFs that might request them.
It is entirely possible for a single VF to soak up all the offload
capability, which would likely annoy some people.  It seems rather
arbitrary to try to set a limit for how many a VF could be allowed,
but this is mitigated somewhat by the need for "trust" and "vf-ipsec"
to be enabled.  I suppose we could come up with a way to make a limit
configurable, but there is no existing method for adding that kind
configuration so I'll leave that to a future discussion.

Currently this doesn't support Tx offload as the hardware encryption
engine doesn't seem to engage on the Tx packets.  This may be a lingering
driver bug, more investigation is needed.  Until then, requests for a Tx
offload are failed and the userland requester will need to add Tx SAs
without the offload attribute.

Given that we don't have Tx offload support, the benefit here is less
than it could be, but is definitely still noticeable.  For example, with
informal iperf testing over a 10Gbps link, with full offload in a PF on
one side and a VF in a VM on the other side on a CPU with AES instructions:

    Reference:
	No IPsec:                         9.4 Gbps
	IPsec offload btwn two PFs:       9.2 Gbps
    VF as the iperf receiver:
	IPsec offload on PF, none on VF:  6.8 Gbps
	IPsec offload on PF and VF:       9.2 Gbps   << biggest benefit
    VF as the iperf sender:
	IPsec offload on PF, none on VF:  4.8 Gbps
	IPsec offload on PF and VF:       4.8 Gbps

The iperf traffic is primarily uni-directional, and we can see the most
benefit when VF is the iperf server and is receiving the test traffic.
Watching output from sar also shows a nice decrease in CPU utilization.


Shannon Nelson (8):
  ixgbe: reload ipsec ip table after sa tables
  ixgbe: prep ipsec constants for later use
  ixgbe: add VF ipsec management
  ixgbe: add VF IPsec offload enable flag
  ixgbe: add VF IPsec offload request message handling
  ixgbevf: add defines for IPsec offload request
  ixgbevf: add VF ipsec offload code
  ixgbevf: enable VF ipsec offload operations

 drivers/net/ethernet/intel/ixgbe/ixgbe.h          |  20 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c  |   9 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c    | 275 ++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h    |  13 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h      |   5 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c    |  17 +-
 drivers/net/ethernet/intel/ixgbevf/Makefile       |   1 +
 drivers/net/ethernet/intel/ixgbevf/defines.h      |  10 +-
 drivers/net/ethernet/intel/ixgbevf/ethtool.c      |   2 +
 drivers/net/ethernet/intel/ixgbevf/ipsec.c        | 673 ++++++++++++++++++++++
 drivers/net/ethernet/intel/ixgbevf/ipsec.h        |  66 +++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf.h      |  33 ++
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |  74 ++-
 drivers/net/ethernet/intel/ixgbevf/mbx.h          |   5 +
 drivers/net/ethernet/intel/ixgbevf/vf.c           |   4 +
 15 files changed, 1163 insertions(+), 44 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/ixgbevf/ipsec.c
 create mode 100644 drivers/net/ethernet/intel/ixgbevf/ipsec.h

-- 
2.7.4

^ permalink raw reply

* [PATCH next-queue 1/8] ixgbe: reload ipsec ip table after sa tables
From: Shannon Nelson @ 2018-08-13 18:43 UTC (permalink / raw)
  To: intel-wired-lan, jeffrey.t.kirsher; +Cc: steffen.klassert, netdev
In-Reply-To: <1534185825-12451-1-git-send-email-shannon.nelson@oracle.com>

Restore the ipsec hardware IP table after reloading the SA tables.
This doesn't make much difference now, but will matter when we add
support for VF ipsec offloads.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index e515246..4340651 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -301,14 +301,6 @@ void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
 	ixgbe_ipsec_clear_hw_tables(adapter);
 	ixgbe_ipsec_start_engine(adapter);
 
-	/* reload the IP addrs */
-	for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
-		struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i];
-
-		if (ipsa->used)
-			ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
-	}
-
 	/* reload the Rx and Tx keys */
 	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
 		struct rx_sa *rsa = &ipsec->rx_tbl[i];
@@ -322,6 +314,14 @@ void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
 		if (tsa->used)
 			ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt);
 	}
+
+	/* reload the IP addrs */
+	for (i = 0; i < IXGBE_IPSEC_MAX_RX_IP_COUNT; i++) {
+		struct rx_ip_sa *ipsa = &ipsec->ip_tbl[i];
+
+		if (ipsa->used)
+			ixgbe_ipsec_set_rx_ip(hw, i, ipsa->ipaddr);
+	}
 }
 
 /**
-- 
2.7.4

^ permalink raw reply related

* [PATCH next-queue 3/8] ixgbe: add VF ipsec management
From: Shannon Nelson @ 2018-08-13 18:43 UTC (permalink / raw)
  To: intel-wired-lan, jeffrey.t.kirsher; +Cc: steffen.klassert, netdev
In-Reply-To: <1534185825-12451-1-git-send-email-shannon.nelson@oracle.com>

Add functions to translate VF IPsec offload add and delete requests
into something the existing code can work with.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 256 ++++++++++++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h |  13 ++
 2 files changed, 260 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 3afb1fe..80108e1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -8,6 +8,8 @@
 #define IXGBE_IPSEC_KEY_BITS  160
 static const char aes_gcm_name[] = "rfc4106(gcm(aes))";
 
+static void ixgbe_ipsec_del_sa(struct xfrm_state *xs);
+
 /**
  * ixgbe_ipsec_set_tx_sa - set the Tx SA registers
  * @hw: hw specific details
@@ -289,6 +291,13 @@ static void ixgbe_ipsec_start_engine(struct ixgbe_adapter *adapter)
 /**
  * ixgbe_ipsec_restore - restore the ipsec HW settings after a reset
  * @adapter: board private structure
+ *
+ * Reload the HW tables from the SW tables after they've been bashed
+ * by a chip reset.
+ *
+ * Any VF entries are removed from the SW and HW tables since either
+ * (a) the VF also gets reset on PF reset and will ask again for the
+ * offloads, or (b) the VF has been removed by a change in the num_vfs.
  **/
 void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
 {
@@ -306,16 +315,24 @@ void ixgbe_ipsec_restore(struct ixgbe_adapter *adapter)
 
 	/* reload the Rx and Tx keys */
 	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT; i++) {
-		struct rx_sa *rsa = &ipsec->rx_tbl[i];
-		struct tx_sa *tsa = &ipsec->tx_tbl[i];
-
-		if (rsa->used)
-			ixgbe_ipsec_set_rx_sa(hw, i, rsa->xs->id.spi,
-					      rsa->key, rsa->salt,
-					      rsa->mode, rsa->iptbl_ind);
+		struct rx_sa *r = &ipsec->rx_tbl[i];
+		struct tx_sa *t = &ipsec->tx_tbl[i];
+
+		if (r->used) {
+			if (r->mode & IXGBE_RXTXMOD_VF)
+				ixgbe_ipsec_del_sa(r->xs);
+			else
+				ixgbe_ipsec_set_rx_sa(hw, i, r->xs->id.spi,
+						      r->key, r->salt,
+						      r->mode, r->iptbl_ind);
+		}
 
-		if (tsa->used)
-			ixgbe_ipsec_set_tx_sa(hw, i, tsa->key, tsa->salt);
+		if (t->used) {
+			if (t->mode & IXGBE_RXTXMOD_VF)
+				ixgbe_ipsec_del_sa(t->xs);
+			else
+				ixgbe_ipsec_set_tx_sa(hw, i, t->key, t->salt);
+		}
 	}
 
 	/* reload the IP addrs */
@@ -381,6 +398,8 @@ static struct xfrm_state *ixgbe_ipsec_find_rx_state(struct ixgbe_ipsec *ipsec,
 	rcu_read_lock();
 	hash_for_each_possible_rcu(ipsec->rx_sa_list, rsa, hlist,
 				   (__force u32)spi) {
+		if (rsa->mode & IXGBE_RXTXMOD_VF)
+			continue;
 		if (spi == rsa->xs->id.spi &&
 		    ((ip4 && *daddr == rsa->xs->id.daddr.a4) ||
 		      (!ip4 && !memcmp(daddr, &rsa->xs->id.daddr.a6,
@@ -809,6 +828,225 @@ static const struct xfrmdev_ops ixgbe_xfrmdev_ops = {
 };
 
 /**
+ * ixgbe_ipsec_vf_clear - clear the tables of data for a VF
+ * @adapter: board private structure
+ * @vf: VF id to be removed
+ **/
+void ixgbe_ipsec_vf_clear(struct ixgbe_adapter *adapter, u32 vf)
+{
+	struct ixgbe_ipsec *ipsec = adapter->ipsec;
+	int i;
+
+	/* search rx sa table */
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT && ipsec->num_rx_sa; i++) {
+		if (!ipsec->rx_tbl[i].used)
+			continue;
+		if (ipsec->rx_tbl[i].mode & IXGBE_RXTXMOD_VF &&
+		    ipsec->rx_tbl[i].vf == vf)
+			ixgbe_ipsec_del_sa(ipsec->rx_tbl[i].xs);
+	}
+
+	/* search tx sa table */
+	for (i = 0; i < IXGBE_IPSEC_MAX_SA_COUNT && ipsec->num_tx_sa; i++) {
+		if (!ipsec->tx_tbl[i].used)
+			continue;
+		if (ipsec->tx_tbl[i].mode & IXGBE_RXTXMOD_VF &&
+		    ipsec->tx_tbl[i].vf == vf)
+			ixgbe_ipsec_del_sa(ipsec->tx_tbl[i].xs);
+	}
+}
+
+/**
+ * ixgbe_ipsec_vf_add_sa - translate VF request to SA add
+ * @adapter: board private structure
+ * @msgbuf: The message buffer
+ * @vf: the VF index
+ *
+ * Make up a new xs and algorithm info from the data sent by the VF.
+ * We only need to sketch in just enough to set up the HW offload.
+ * Put the resulting offload_handle into the return message to the VF.
+ *
+ * Returns 0 or error value
+ **/
+int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
+{
+	struct ixgbe_ipsec *ipsec = adapter->ipsec;
+	struct xfrm_algo_desc *algo;
+	struct sa_mbx_msg *sam;
+	struct xfrm_state *xs;
+	size_t aead_len;
+	u16 sa_idx;
+	u32 pfsa;
+	int err;
+
+	sam = (struct sa_mbx_msg *)(&msgbuf[1]);
+	if (!adapter->vfinfo[vf].trusted) {
+		e_warn(drv, "VF %d attempted to add an IPsec SA\n", vf);
+		err = -EACCES;
+		goto err_out;
+	}
+
+	/* Tx IPsec offload doesn't seem to work on this
+	 * device, so block these requests for now.
+	 */
+	if (!(sam->flags & XFRM_OFFLOAD_INBOUND)) {
+		err = -ENXIO;
+		goto err_out;
+	}
+
+	xs = kzalloc(sizeof(*xs), GFP_KERNEL);
+	if (unlikely(!xs)) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	xs->xso.flags = sam->flags;
+	xs->id.spi = sam->spi;
+	xs->id.proto = sam->proto;
+	xs->props.family = sam->family;
+	if (xs->props.family == AF_INET6)
+		memcpy(&xs->id.daddr.a6, sam->addr, sizeof(xs->id.daddr.a6));
+	else
+		memcpy(&xs->id.daddr.a4, sam->addr, sizeof(xs->id.daddr.a4));
+	xs->xso.dev = adapter->netdev;
+
+	algo = xfrm_aead_get_byname(aes_gcm_name, IXGBE_IPSEC_AUTH_BITS, 1);
+	if (unlikely(!algo)) {
+		err = -ENOENT;
+		goto err_xs;
+	}
+
+	aead_len = sizeof(*xs->aead) + IXGBE_IPSEC_KEY_BITS / 8;
+	xs->aead = kzalloc(aead_len, GFP_KERNEL);
+	if (unlikely(!xs->aead)) {
+		err = -ENOMEM;
+		goto err_xs;
+	}
+
+	xs->props.ealgo = algo->desc.sadb_alg_id;
+	xs->geniv = algo->uinfo.aead.geniv;
+	xs->aead->alg_icv_len = IXGBE_IPSEC_AUTH_BITS;
+	xs->aead->alg_key_len = IXGBE_IPSEC_KEY_BITS;
+	memcpy(xs->aead->alg_key, sam->key, sizeof(sam->key));
+	memcpy(xs->aead->alg_name, aes_gcm_name, sizeof(aes_gcm_name));
+
+	/* set up the HW offload */
+	err = ixgbe_ipsec_add_sa(xs);
+	if (err)
+		goto err_aead;
+
+	pfsa = xs->xso.offload_handle;
+	if (pfsa < IXGBE_IPSEC_BASE_TX_INDEX) {
+		sa_idx = pfsa - IXGBE_IPSEC_BASE_RX_INDEX;
+		ipsec->rx_tbl[sa_idx].vf = vf;
+		ipsec->rx_tbl[sa_idx].mode |= IXGBE_RXTXMOD_VF;
+	} else {
+		sa_idx = pfsa - IXGBE_IPSEC_BASE_TX_INDEX;
+		ipsec->tx_tbl[sa_idx].vf = vf;
+		ipsec->tx_tbl[sa_idx].mode |= IXGBE_RXTXMOD_VF;
+	}
+
+	msgbuf[1] = xs->xso.offload_handle;
+
+	return 0;
+
+err_aead:
+	memset(xs->aead, 0, sizeof(*xs->aead));
+	kfree(xs->aead);
+err_xs:
+	memset(xs, 0, sizeof(*xs));
+	kfree(xs);
+err_out:
+	msgbuf[1] = err;
+	return err;
+}
+
+/**
+ * ixgbe_ipsec_vf_del_sa - translate VF request to SA delete
+ * @adapter: board private structure
+ * @msgbuf: The message buffer
+ * @vf: the VF index
+ *
+ * Given the offload_handle sent by the VF, look for the related SA table
+ * entry and use its xs field to call for a delete of the SA.
+ *
+ * Note: We silently ignore requests to delete entries that are already
+ *       set to unused because when a VF is set to "DOWN", the PF first
+ *       gets a reset and clears all the VF's entries; then the VF's
+ *       XFRM stack sends individual deletes for each entry, which the
+ *       reset already removed.  In the future it might be good to try to
+ *       optimize this so not so many unnecessary delete messages are sent.
+ *
+ * Returns 0 or error value
+ **/
+int ixgbe_ipsec_vf_del_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
+{
+	struct ixgbe_ipsec *ipsec = adapter->ipsec;
+	struct xfrm_state *xs;
+	u32 pfsa = msgbuf[1];
+	u16 sa_idx;
+
+	if (!adapter->vfinfo[vf].trusted) {
+		e_err(drv, "vf %d attempted to delete an SA\n", vf);
+		return -EPERM;
+	}
+
+	if (pfsa < IXGBE_IPSEC_BASE_TX_INDEX) {
+		struct rx_sa *rsa;
+
+		sa_idx = pfsa - IXGBE_IPSEC_BASE_RX_INDEX;
+		if (sa_idx >= IXGBE_IPSEC_MAX_SA_COUNT) {
+			e_err(drv, "vf %d SA index %d out of range\n",
+			      vf, sa_idx);
+			return -EINVAL;
+		}
+
+		rsa = &ipsec->rx_tbl[sa_idx];
+
+		if (!rsa->used)
+			return 0;
+
+		if (!(rsa->mode & IXGBE_RXTXMOD_VF) ||
+		    rsa->vf != vf) {
+			e_err(drv, "vf %d bad Rx SA index %d\n", vf, sa_idx);
+			return -ENOENT;
+		}
+
+		xs = ipsec->rx_tbl[sa_idx].xs;
+	} else {
+		struct tx_sa *tsa;
+
+		sa_idx = pfsa - IXGBE_IPSEC_BASE_TX_INDEX;
+		if (sa_idx >= IXGBE_IPSEC_MAX_SA_COUNT) {
+			e_err(drv, "vf %d SA index %d out of range\n",
+			      vf, sa_idx);
+			return -EINVAL;
+		}
+
+		tsa = &ipsec->tx_tbl[sa_idx];
+
+		if (!tsa->used)
+			return 0;
+
+		if (!(tsa->mode & IXGBE_RXTXMOD_VF) ||
+		    tsa->vf != vf) {
+			e_err(drv, "vf %d bad Tx SA index %d\n", vf, sa_idx);
+			return -ENOENT;
+		}
+
+		xs = ipsec->tx_tbl[sa_idx].xs;
+	}
+
+	ixgbe_ipsec_del_sa(xs);
+
+	/* remove the xs that was made-up in the add request */
+	memset(xs, 0, sizeof(*xs));
+	kfree(xs);
+
+	return 0;
+}
+
+/**
  * ixgbe_ipsec_tx - setup Tx flags for ipsec offload
  * @tx_ring: outgoing context
  * @first: current data packet
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
index 9ef7faa..d2b64ff 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.h
@@ -26,6 +26,7 @@ enum ixgbe_ipsec_tbl_sel {
 #define IXGBE_RXMOD_PROTO_ESP		0x00000004
 #define IXGBE_RXMOD_DECRYPT		0x00000008
 #define IXGBE_RXMOD_IPV6		0x00000010
+#define IXGBE_RXTXMOD_VF		0x00000020
 
 struct rx_sa {
 	struct hlist_node hlist;
@@ -37,6 +38,7 @@ struct rx_sa {
 	u8  iptbl_ind;
 	bool used;
 	bool decrypt;
+	u32 vf;
 };
 
 struct rx_ip_sa {
@@ -49,8 +51,10 @@ struct tx_sa {
 	struct xfrm_state *xs;
 	u32 key[4];
 	u32 salt;
+	u32 mode;
 	bool encrypt;
 	bool used;
+	u32 vf;
 };
 
 struct ixgbe_ipsec_tx_data {
@@ -67,4 +71,13 @@ struct ixgbe_ipsec {
 	struct tx_sa *tx_tbl;
 	DECLARE_HASHTABLE(rx_sa_list, 10);
 };
+
+struct sa_mbx_msg {
+	__be32 spi;
+	u8 flags;
+	u8 proto;
+	u16 family;
+	__be32 addr[4];
+	u32 key[5];
+};
 #endif /* _IXGBE_IPSEC_H_ */
-- 
2.7.4

^ permalink raw reply related

* [PATCH] bnxt_en: avoid string overflow for record->system_name
From: Arnd Bergmann @ 2018-08-13 21:26 UTC (permalink / raw)
  To: Michael Chan, David S. Miller
  Cc: Arnd Bergmann, Vasundhara Volam, Scott Branden, Andy Gospodarek,
	netdev, linux-kernel

The utsname()->nodename string may be 64 bytes long, and it gets
copied without the trailing nul byte into the shorter record->system_name,
as gcc now warns:

In file included from include/linux/bitmap.h:9,
                 from include/linux/ethtool.h:16,
                 from drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:13:
In function 'strncpy',
    inlined from 'bnxt_fill_coredump_record' at drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c:2863:2:
include/linux/string.h:254:9: error: '__builtin_strncpy' output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]

Using strlcpy() at least avoids overflowing the destination buffer
and adds proper nul-termination. It may still truncate long names
though, which probably can't be solved here.

Fixes: 6c5657d085ae ("bnxt_en: Add support for ethtool get dump.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index b1602ea64372..e52d7af3ab3e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -2860,8 +2860,8 @@ bnxt_fill_coredump_record(struct bnxt *bp, struct bnxt_coredump_record *record,
 	record->low_version = 0;
 	record->high_version = 1;
 	record->asic_state = 0;
-	strncpy(record->system_name, utsname()->nodename,
-		strlen(utsname()->nodename));
+	strlcpy(record->system_name, utsname()->nodename,
+		sizeof(record->system_name));
 	record->year = cpu_to_le16(tm.tm_year);
 	record->month = cpu_to_le16(tm.tm_mon);
 	record->day = cpu_to_le16(tm.tm_mday);
-- 
2.18.0

^ permalink raw reply related

* Re: pull request: bluetooth-next 2018-08-13
From: David Miller @ 2018-08-13 18:39 UTC (permalink / raw)
  To: johan.hedberg; +Cc: linux-bluetooth, netdev
In-Reply-To: <20180813150019.GA2305@x1c.home>

From: Johan Hedberg <johan.hedberg@gmail.com>
Date: Mon, 13 Aug 2018 18:00:19 +0300

> There was one pretty bad bug that slipped into the MediaTek HCI driver
> in the last bluetooth-next pull request. Would it be possible to get
> this one-liner fix pulled to net-next before you make your first 4.19
> pull request for Linus? Thanks.

Sure, pulled, thanks Johan.

^ permalink raw reply

* Re: [PATCH iproute2-next 1/3] tc: support conversions to or from 64 bit nanosecond-based time
From: Dave Taht @ 2018-08-13 18:37 UTC (permalink / raw)
  To: David Ahern
  Cc: Yousuk Seung, Linux Kernel Network Developers, Stephen Hemminger,
	Michael McLennan, Priyaranjan Jha, Neal Cardwell
In-Reply-To: <ce7d2a8e-3bff-9250-d6cf-c654098c396f@gmail.com>

On Sun, Aug 12, 2018 at 3:09 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 8/6/18 11:09 AM, Yousuk Seung wrote:
> > diff --git a/tc/tc_core.h b/tc/tc_core.h
> > index 1dfa9a4f773b..a0fe0923d171 100644
> > --- a/tc/tc_core.h
> > +++ b/tc/tc_core.h
> > @@ -7,6 +7,10 @@
> >
> >  #define TIME_UNITS_PER_SEC   1000000
> >
> > +#define NSEC_PER_USEC 1000
> > +#define NSEC_PER_MSEC 1000000
> > +#define NSEC_PER_SEC 1000000000LL
> > +
>
> These are not specific to tc so a header in include is a better location
> (utils.h or a new one)
>
> >  enum link_layer {
> >       LINKLAYER_UNSPEC,
> >       LINKLAYER_ETHERNET,
> > diff --git a/tc/tc_util.c b/tc/tc_util.c
> > index d7578528a31b..c39c9046dcae 100644
> > --- a/tc/tc_util.c
> > +++ b/tc/tc_util.c
>
> Similarly for these time functions - not specific to tc so move to
> lib/utils.c
>
> > @@ -385,6 +385,61 @@ char *sprint_ticks(__u32 ticks, char *buf)
> >       return sprint_time(tc_core_tick2time(ticks), buf);
> >  }
> >
> > +/* 64 bit times are represented internally in nanoseconds */
> > +int get_time64(__s64 *time, const char *str)
>
> __u64 seems more appropriate than __s64

The reason why these are signed is to leave room in the API to
print/manage negative values. Wasting the 64th bit thusly would only
matter after extreme uptimes.

There was something of a long debate on this when these patches went
around the first time. We ended up with signed time in the netem code
also.


> > +{
> > +     double nsec;
> > +     char *p;
> > +
> > +     nsec = strtod(str, &p);
> > +     if (p == str)
> > +             return -1;
> > +
> > +     if (*p) {
> > +             if (strcasecmp(p, "s") == 0 ||
> > +                 strcasecmp(p, "sec") == 0 ||
> > +                 strcasecmp(p, "secs") == 0)
> > +                     nsec *= NSEC_PER_SEC;
> > +             else if (strcasecmp(p, "ms") == 0 ||
> > +                      strcasecmp(p, "msec") == 0 ||
> > +                      strcasecmp(p, "msecs") == 0)
> > +                     nsec *= NSEC_PER_MSEC;
> > +             else if (strcasecmp(p, "us") == 0 ||
> > +                      strcasecmp(p, "usec") == 0 ||
> > +                      strcasecmp(p, "usecs") == 0)
> > +                     nsec *= NSEC_PER_USEC;
> > +             else if (strcasecmp(p, "ns") == 0 ||
> > +                      strcasecmp(p, "nsec") == 0 ||
> > +                      strcasecmp(p, "nsecs") == 0)
>
> strncasecmp would be more efficient
>
> > +                     nsec *= 1;
> > +             else
> > +                     return -1;
> > +     }
> > +
> > +     *time = nsec;
> > +     return 0;
> > +}
> > +
> > +void print_time64(char *buf, int len, __s64 time)
> > +{
> > +     double nsec = time;
> > +
> > +     if (time >= NSEC_PER_SEC)
> > +             snprintf(buf, len, "%.3fs", nsec/NSEC_PER_SEC);
> > +     else if (time >= NSEC_PER_MSEC)
> > +             snprintf(buf, len, "%.3fms", nsec/NSEC_PER_MSEC);
> > +     else if (time >= NSEC_PER_USEC)
> > +             snprintf(buf, len, "%.3fus", nsec/NSEC_PER_USEC);
> > +     else
> > +             snprintf(buf, len, "%lldns", time);
> > +}
> > +
> > +char *sprint_time64(__s64 time, char *buf)
> > +{
> > +     print_time64(buf, SPRINT_BSIZE-1, time);
> > +     return buf;
> > +}
> > +
> >  int get_size(unsigned int *size, const char *str)
> >  {
> >       double sz;



-- 

Dave Täht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619

^ permalink raw reply

* [PATCH 2/2] net: lan743x: fix building without CONFIG_PTP_1588_CLOCK
From: Arnd Bergmann @ 2018-08-13 21:19 UTC (permalink / raw)
  To: David S. Miller
  Cc: Arnd Bergmann, Bryan Whitehead, Microchip Linux Driver Support,
	netdev, linux-kernel
In-Reply-To: <20180813211938.2375199-1-arnd@arndb.de>

Building without CONFIG_PTP_1588_CLOCK results in multiple failures,
this was obviously not well tested:

drivers/net/ethernet/microchip/lan743x_ptp.c: In function 'lan743x_ptp_isr':
drivers/net/ethernet/microchip/lan743x_ptp.c:781:28: error: 'struct lan743x_ptp' has no member named 'ptp_clock'; did you mean 'tx_ts_lock'?
   ptp_schedule_worker(ptp->ptp_clock, 0);
                            ^~~~~~~~~
                            tx_ts_lock
drivers/net/ethernet/microchip/lan743x_ptp.c: In function 'lan743x_ptp_open':
drivers/net/ethernet/microchip/lan743x_ptp.c:879:6: error: unused variable 'ret' [-Werror=unused-variable]
  int ret = -ENODEV;
      ^~~
At top level:
drivers/net/ethernet/microchip/lan743x_ptp.c:63:13: error: 'lan743x_ptp_tx_ts_enqueue_ts' defined but not used [-Werror=unused-function]
 static void lan743x_ptp_tx_ts_enqueue_ts(struct lan743x_adapter *adapter,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
drivers/net/ethernet/microchip/lan743x_ethtool.c: In function 'lan743x_ethtool_get_ts_info':
drivers/net/ethernet/microchip/lan743x_ethtool.c:558:19: error: 'struct lan743x_ptp' has no member named 'ptp_clock'; did you mean 'tx_ts_lock'?

Those #ifdef checks are hard to get right, replace them all with
IS_ENABLED() checks that leave the same code visible to the compiler
but let it optimize out the unused bits based on the configuration.

Fixes: 07624df1c9ef ("lan743x: lan743x: Add PTP support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/microchip/lan743x_ptp.c | 15 ++++++---------
 drivers/net/ethernet/microchip/lan743x_ptp.h |  2 --
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 42064fd2beb5..66e7568a0e7c 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -4,6 +4,7 @@
 #include <linux/netdevice.h>
 #include "lan743x_main.h"
 
+#include <linux/ptp_clock_kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/netdevice.h>
@@ -139,7 +140,6 @@ static void lan743x_ptp_tx_ts_complete(struct lan743x_adapter *adapter)
 	spin_unlock_bh(&ptp->tx_ts_lock);
 }
 
-#ifdef CONFIG_PTP_1588_CLOCK
 static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter)
 {
 	struct lan743x_ptp *ptp = &adapter->ptp;
@@ -761,7 +761,6 @@ static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
 		mutex_unlock(&ptp->command_lock);
 	}
 }
-#endif /* CONFIG_PTP_1588_CLOCK */
 
 void lan743x_ptp_isr(void *context)
 {
@@ -890,7 +889,9 @@ int lan743x_ptp_open(struct lan743x_adapter *adapter)
 			  PTP_INT_BIT_TX_SWTS_ERR_ | PTP_INT_BIT_TX_TS_);
 	ptp->flags |= PTP_FLAG_ISR_ENABLED;
 
-#ifdef CONFIG_PTP_1588_CLOCK
+	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK))
+		return 0;
+
 	snprintf(ptp->pin_config[0].name, 32, "lan743x_ptp_pin_0");
 	ptp->pin_config[0].index = 0;
 	ptp->pin_config[0].func = PTP_PF_PEROUT;
@@ -932,9 +933,6 @@ int lan743x_ptp_open(struct lan743x_adapter *adapter)
 done:
 	lan743x_ptp_close(adapter);
 	return ret;
-#else
-	return 0;
-#endif
 }
 
 void lan743x_ptp_close(struct lan743x_adapter *adapter)
@@ -942,15 +940,14 @@ void lan743x_ptp_close(struct lan743x_adapter *adapter)
 	struct lan743x_ptp *ptp = &adapter->ptp;
 	int index;
 
-#ifdef CONFIG_PTP_1588_CLOCK
-	if (ptp->flags & PTP_FLAG_PTP_CLOCK_REGISTERED) {
+	if (IS_ENABLED(CONFIG_PTP_1588_CLOCK) &&
+	    ptp->flags & PTP_FLAG_PTP_CLOCK_REGISTERED) {
 		ptp_clock_unregister(ptp->ptp_clock);
 		ptp->ptp_clock = NULL;
 		ptp->flags &= ~PTP_FLAG_PTP_CLOCK_REGISTERED;
 		netif_info(adapter, drv, adapter->netdev,
 			   "ptp clock unregister\n");
 	}
-#endif
 
 	if (ptp->flags & PTP_FLAG_ISR_ENABLED) {
 		lan743x_csr_write(adapter, PTP_INT_EN_CLR,
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.h b/drivers/net/ethernet/microchip/lan743x_ptp.h
index 20f2223024dc..5fc1b3cd5e33 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.h
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.h
@@ -49,11 +49,9 @@ struct lan743x_ptp {
 	/* command_lock: used to prevent concurrent ptp commands */
 	struct mutex	command_lock;
 
-#ifdef CONFIG_PTP_1588_CLOCK
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_clock_info;
 	struct ptp_pin_desc pin_config[1];
-#endif /* CONFIG_PTP_1588_CLOCK */
 
 #define LAN743X_PTP_NUMBER_OF_EVENT_CHANNELS (2)
 	unsigned long used_event_ch;
-- 
2.18.0

^ permalink raw reply related

* [PATCH 1/2] net: lan743x: select CRC16
From: Arnd Bergmann @ 2018-08-13 21:19 UTC (permalink / raw)
  To: David S. Miller
  Cc: Arnd Bergmann, Bryan Whitehead, Andrew Lunn, netdev, linux-kernel

lan743x now fails to build when CONFIG_CRC16 is disabled:

drivers/net/ethernet/microchip/lan743x_main.o: In function crc16'

Force it on like all other users do.

Fixes: 4d94282afd95 ("lan743x: Add power management support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/microchip/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/microchip/Kconfig b/drivers/net/ethernet/microchip/Kconfig
index 71dca8bd51ac..16bd3f44dbe8 100644
--- a/drivers/net/ethernet/microchip/Kconfig
+++ b/drivers/net/ethernet/microchip/Kconfig
@@ -46,6 +46,7 @@ config LAN743X
 	tristate "LAN743x support"
 	depends on PCI
 	select PHYLIB
+	select CRC16
 	---help---
 	  Support for the Microchip LAN743x PCI Express Gigabit Ethernet chip
 
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Vlad Buslov @ 2018-08-13 18:33 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: netdev, davem, xiyou.wangcong, jiri
In-Reply-To: <2d95cda0-22e2-0f4f-3dc3-e7a1df4b68ee@mojatatu.com>


On Mon 13 Aug 2018 at 17:23, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 2018-08-13 1:20 p.m., Vlad Buslov wrote:
>> Lockdep reports deadlock for following locking scenario in ife action:
>> 
>> Task one:
>> 1) Executes ife action update.
>> 2) Takes tcfa_lock.
>> 3) Waits on ife_mod_lock which is already taken by task two.
>> 
>> Task two:
>> 
>> 1) Executes any path that obtains ife_mod_lock without disabling bh (any
>> path that takes ife_mod_lock while holding tcfa_lock has bh disabled) like
>> loading a meta module, or creating new action.
>> 2) Takes ife_mod_lock.
>> 3) Task is preempted by rate estimator timer.
>> 4) Timer callback waits on tcfa_lock which is taken by task one.
>> 
>> In described case tasks deadlock because they take same two locks in
>> different order. To prevent potential deadlock reported by lockdep, always
>> disable bh when obtaining ife_mod_lock.
>> 
>
> Looks like your recent changes on net-next exposed this.

Its because I've recently expanded my private tests to create all kinds
of actions with estimator.

>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> cheers,
> jamal

^ permalink raw reply

* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: David Miller @ 2018-08-13 18:27 UTC (permalink / raw)
  To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <vbf4lfygotr.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>

From: Vlad Buslov <vladbu@mellanox.com>
Date: Mon, 13 Aug 2018 20:26:40 +0300

> Is it okay to submit a fix for issue I uncovered when testing actions
> with estimators, or I should resubmit to net when net-next is moved?

Yes, this is fine.

^ permalink raw reply

* Re: [PATCH v1 0/3] WireGuard: Secure Network Tunnel
From: James Bottomley @ 2018-08-13 18:04 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: linux-kernel, netdev, davem, linux-crypto
In-Reply-To: <CAHmME9pn+zm67mmDDrkMLpSAXNEugCT3z2L-QXO2eQw+31V=YQ@mail.gmail.com>

On Mon, 2018-08-13 at 10:55 -0700, Jason A. Donenfeld wrote:
> > but it's very hard for a flow classifier because you have to
> 
> The construction and identifier strings might not obviously help with
> the extremely narrow idea you've brought up, but it is very important
> for safely introducing additional versions. Namely, it prevents
> against cross-protocol key reuse attacks and type confusion bugs. So
> don't be too quick to dismiss the importance of these for
> accomplishing what we're after.

I'm not saying a hash check isn't important for safety; I'm saying that
if you only have a hash of a dynamic part plus the protocol identifier
to go on it makes far more work for the flow classifier.  You can see
this easily if you contemplate the idea that the hash might be the
algorithm being changed.

> > so lets pick one of the above and try it out.
> 
> We have, multiple times, and it's absolutely trivial to do and works
> well. The exact thing you're concerned about has already been
> researched and worked with on live systems quite a bit over the last
> 3 years, and it works in a pretty straight forward way. I'm not sure
> there's much more to add here: the thing you want is already there
> and has been tested extensively. At this point the "pick one and
> let's try it out!" is an old story, and the focus now is on making
> sure the code quality and netdev api usage is correct for merging

Great, thanks, I'll look forward to seeing it in v2 then.

James

^ permalink raw reply

* Re: [PATCH net-next] openvswitch: Derive IP protocol number for IPv6 later frags
From: William Tu @ 2018-08-13 17:48 UTC (permalink / raw)
  To: pravin shelar; +Cc: Yi-Hung Wei, Linux Kernel Network Developers
In-Reply-To: <CAOrHB_D7mOjqvghNjBrXF_JnZw6QzzOaZ60LsphC8OacTwYXnA@mail.gmail.com>

On Sun, Aug 12, 2018 at 6:09 PM Pravin Shelar <pshelar@ovn.org> wrote:
>
> On Fri, Aug 10, 2018 at 10:19 AM, Yi-Hung Wei <yihung.wei@gmail.com> wrote:
> > Currently, OVS only parses the IP protocol number for the first
> > IPv6 fragment, but sets the IP protocol number for the later fragments
> > to be NEXTHDF_FRAGMENT.  This patch tries to derive the IP protocol
> > number for the IPV6 later frags so that we can match that.
> >
> > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> > ---
> >  net/openvswitch/flow.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> > index 56b8e7167790..3d654c4f71be 100644
> > --- a/net/openvswitch/flow.c
> > +++ b/net/openvswitch/flow.c
> > @@ -297,7 +297,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
> >
> >         nh_len = payload_ofs - nh_ofs;
> >         skb_set_transport_header(skb, nh_ofs + nh_len);
> > -       key->ip.proto = nexthdr;
> > +       if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
> > +               unsigned int offset = 0;

How about we start the 2nd time parsing from
unsigned int offset = payload_ofs;

> > +
> > +               key->ip.proto = ipv6_find_hdr(skb, &offset, -1, NULL, NULL);

Then we only find the last header from previous parsed offset.

William

> > +       } else {
> > +               key->ip.proto = nexthdr;
> > +       }
> parsing ipv6 ipv6_skip_exthdr() is called to find fragment hdr and
> then this patch calls ipv6_find_hdr() to find next protocol. I think
> we could call ipv6_find_hdr() to get fragment type and next hdr, that
> would save parsing same packet twice in some cases.
>
> Other option would be calling ipv6_find_hdr() after setting OVS_FRAG_TYPE_LATER.

^ permalink raw reply

* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Vlad Buslov @ 2018-08-13 17:26 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: davem, jhs, xiyou.wangcong, jiri
In-Reply-To: <1534180811-10416-1-git-send-email-vladbu@mellanox.com>

Hi David,

Is it okay to submit a fix for issue I uncovered when testing actions
with estimators, or I should resubmit to net when net-next is moved?

Thanks,
Vlad

^ permalink raw reply

* Re: [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Jamal Hadi Salim @ 2018-08-13 17:23 UTC (permalink / raw)
  To: Vlad Buslov, netdev; +Cc: davem, xiyou.wangcong, jiri
In-Reply-To: <1534180811-10416-1-git-send-email-vladbu@mellanox.com>

On 2018-08-13 1:20 p.m., Vlad Buslov wrote:
> Lockdep reports deadlock for following locking scenario in ife action:
> 
> Task one:
> 1) Executes ife action update.
> 2) Takes tcfa_lock.
> 3) Waits on ife_mod_lock which is already taken by task two.
> 
> Task two:
> 
> 1) Executes any path that obtains ife_mod_lock without disabling bh (any
> path that takes ife_mod_lock while holding tcfa_lock has bh disabled) like
> loading a meta module, or creating new action.
> 2) Takes ife_mod_lock.
> 3) Task is preempted by rate estimator timer.
> 4) Timer callback waits on tcfa_lock which is taken by task one.
> 
> In described case tasks deadlock because they take same two locks in
> different order. To prevent potential deadlock reported by lockdep, always
> disable bh when obtaining ife_mod_lock.
> 

Looks like your recent changes on net-next exposed this.

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply

* [PATCH net-next] net: sched: act_ife: disable bh when taking ife_mod_lock
From: Vlad Buslov @ 2018-08-13 17:20 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, Vlad Buslov

Lockdep reports deadlock for following locking scenario in ife action:

Task one:
1) Executes ife action update.
2) Takes tcfa_lock.
3) Waits on ife_mod_lock which is already taken by task two.

Task two:

1) Executes any path that obtains ife_mod_lock without disabling bh (any
path that takes ife_mod_lock while holding tcfa_lock has bh disabled) like
loading a meta module, or creating new action.
2) Takes ife_mod_lock.
3) Task is preempted by rate estimator timer.
4) Timer callback waits on tcfa_lock which is taken by task one.

In described case tasks deadlock because they take same two locks in
different order. To prevent potential deadlock reported by lockdep, always
disable bh when obtaining ife_mod_lock.

Lockdep warning:

[  508.101192] =====================================================
[  508.107708] WARNING: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected
[  508.114728] 4.18.0-rc8+ #646 Not tainted
[  508.119050] -----------------------------------------------------
[  508.125559] tc/5460 [HC0[0]:SC0[2]:HE1:SE0] is trying to acquire:
[  508.132025] 000000005a938c68 (ife_mod_lock){++++}, at: find_ife_oplist+0x1e/0xc0 [act_ife]
[  508.140996]
               and this task is already holding:
[  508.147548] 00000000d46f6c56 (&(&p->tcfa_lock)->rlock){+.-.}, at: tcf_ife_init+0x6ae/0xf40 [act_ife]
[  508.157371] which would create a new lock dependency:
[  508.162828]  (&(&p->tcfa_lock)->rlock){+.-.} -> (ife_mod_lock){++++}
[  508.169572]
               but this new dependency connects a SOFTIRQ-irq-safe lock:
[  508.178197]  (&(&p->tcfa_lock)->rlock){+.-.}
[  508.178201]
               ... which became SOFTIRQ-irq-safe at:
[  508.189771]   _raw_spin_lock+0x2c/0x40
[  508.193906]   est_fetch_counters+0x41/0xb0
[  508.198391]   est_timer+0x83/0x3c0
[  508.202180]   call_timer_fn+0x16a/0x5d0
[  508.206400]   run_timer_softirq+0x399/0x920
[  508.210967]   __do_softirq+0x157/0x97d
[  508.215102]   irq_exit+0x152/0x1c0
[  508.218888]   smp_apic_timer_interrupt+0xc0/0x4e0
[  508.223976]   apic_timer_interrupt+0xf/0x20
[  508.228540]   cpuidle_enter_state+0xf8/0x5d0
[  508.233198]   do_idle+0x28a/0x350
[  508.236881]   cpu_startup_entry+0xc7/0xe0
[  508.241296]   start_secondary+0x2e8/0x3f0
[  508.245678]   secondary_startup_64+0xa5/0xb0
[  508.250347]
               to a SOFTIRQ-irq-unsafe lock:  (ife_mod_lock){++++}
[  508.256531]
               ... which became SOFTIRQ-irq-unsafe at:
[  508.267279] ...
[  508.267283]   _raw_write_lock+0x2c/0x40
[  508.273653]   register_ife_op+0x118/0x2c0 [act_ife]
[  508.278926]   do_one_initcall+0xf7/0x4d9
[  508.283214]   do_init_module+0x18b/0x44e
[  508.287521]   load_module+0x4167/0x5730
[  508.291739]   __do_sys_finit_module+0x16d/0x1a0
[  508.296654]   do_syscall_64+0x7a/0x3f0
[  508.300788]   entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  508.306302]
               other info that might help us debug this:

[  508.315286]  Possible interrupt unsafe locking scenario:

[  508.322771]        CPU0                    CPU1
[  508.327681]        ----                    ----
[  508.332604]   lock(ife_mod_lock);
[  508.336300]                                local_irq_disable();
[  508.342608]                                lock(&(&p->tcfa_lock)->rlock);
[  508.349793]                                lock(ife_mod_lock);
[  508.355990]   <Interrupt>
[  508.358974]     lock(&(&p->tcfa_lock)->rlock);
[  508.363803]
                *** DEADLOCK ***

[  508.370715] 2 locks held by tc/5460:
[  508.374680]  #0: 00000000e27e4fa4 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x583/0x7b0
[  508.383366]  #1: 00000000d46f6c56 (&(&p->tcfa_lock)->rlock){+.-.}, at: tcf_ife_init+0x6ae/0xf40 [act_ife]
[  508.393648]
               the dependencies between SOFTIRQ-irq-safe lock and the holding lock:
[  508.403505] -> (&(&p->tcfa_lock)->rlock){+.-.} ops: 1001553 {
[  508.409646]    HARDIRQ-ON-W at:
[  508.413136]                     _raw_spin_lock_bh+0x34/0x40
[  508.419059]                     gnet_stats_start_copy_compat+0xa2/0x230
[  508.426021]                     gnet_stats_start_copy+0x16/0x20
[  508.432333]                     tcf_action_copy_stats+0x95/0x1d0
[  508.438735]                     tcf_action_dump_1+0xb0/0x4e0
[  508.444795]                     tcf_action_dump+0xca/0x200
[  508.450673]                     tcf_exts_dump+0xd9/0x320
[  508.456392]                     fl_dump+0x1b7/0x4a0 [cls_flower]
[  508.462798]                     tcf_fill_node+0x380/0x530
[  508.468601]                     tfilter_notify+0xdf/0x1c0
[  508.474404]                     tc_new_tfilter+0x84a/0xc90
[  508.480270]                     rtnetlink_rcv_msg+0x5bd/0x7b0
[  508.486419]                     netlink_rcv_skb+0x184/0x220
[  508.492394]                     netlink_unicast+0x31b/0x460
[  508.507411]                     netlink_sendmsg+0x3fb/0x840
[  508.513390]                     sock_sendmsg+0x7b/0xd0
[  508.518907]                     ___sys_sendmsg+0x4c6/0x610
[  508.524797]                     __sys_sendmsg+0xd7/0x150
[  508.530510]                     do_syscall_64+0x7a/0x3f0
[  508.536201]                     entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  508.543301]    IN-SOFTIRQ-W at:
[  508.546834]                     _raw_spin_lock+0x2c/0x40
[  508.552522]                     est_fetch_counters+0x41/0xb0
[  508.558571]                     est_timer+0x83/0x3c0
[  508.563912]                     call_timer_fn+0x16a/0x5d0
[  508.569699]                     run_timer_softirq+0x399/0x920
[  508.575840]                     __do_softirq+0x157/0x97d
[  508.581538]                     irq_exit+0x152/0x1c0
[  508.586882]                     smp_apic_timer_interrupt+0xc0/0x4e0
[  508.593533]                     apic_timer_interrupt+0xf/0x20
[  508.599686]                     cpuidle_enter_state+0xf8/0x5d0
[  508.605895]                     do_idle+0x28a/0x350
[  508.611147]                     cpu_startup_entry+0xc7/0xe0
[  508.617097]                     start_secondary+0x2e8/0x3f0
[  508.623029]                     secondary_startup_64+0xa5/0xb0
[  508.629245]    INITIAL USE at:
[  508.632686]                    _raw_spin_lock_bh+0x34/0x40
[  508.638557]                    gnet_stats_start_copy_compat+0xa2/0x230
[  508.645491]                    gnet_stats_start_copy+0x16/0x20
[  508.651719]                    tcf_action_copy_stats+0x95/0x1d0
[  508.657992]                    tcf_action_dump_1+0xb0/0x4e0
[  508.663937]                    tcf_action_dump+0xca/0x200
[  508.669716]                    tcf_exts_dump+0xd9/0x320
[  508.675337]                    fl_dump+0x1b7/0x4a0 [cls_flower]
[  508.681650]                    tcf_fill_node+0x380/0x530
[  508.687366]                    tfilter_notify+0xdf/0x1c0
[  508.693031]                    tc_new_tfilter+0x84a/0xc90
[  508.698820]                    rtnetlink_rcv_msg+0x5bd/0x7b0
[  508.704869]                    netlink_rcv_skb+0x184/0x220
[  508.710758]                    netlink_unicast+0x31b/0x460
[  508.716627]                    netlink_sendmsg+0x3fb/0x840
[  508.722510]                    sock_sendmsg+0x7b/0xd0
[  508.727931]                    ___sys_sendmsg+0x4c6/0x610
[  508.733729]                    __sys_sendmsg+0xd7/0x150
[  508.739346]                    do_syscall_64	+0x7a/0x3f0
[  508.744943]                    entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  508.751930]  }
[  508.753964]  ... key      at: [<ffffffff916b3e20>] __key.61145+0x0/0x40
[  508.760946]  ... acquired at:
[  508.764294]    _raw_read_lock+0x2f/0x40
[  508.768513]    find_ife_oplist+0x1e/0xc0 [act_ife]
[  508.773692]    tcf_ife_init+0x82f/0xf40 [act_ife]
[  508.778785]    tcf_action_init_1+0x510/0x750
[  508.783468]    tcf_action_init+0x1e8/0x340
[  508.787938]    tcf_action_add+0xc5/0x240
[  508.792241]    tc_ctl_action+0x203/0x2a0
[  508.796550]    rtnetlink_rcv_msg+0x5bd/0x7b0
[  508.801200]    netlink_rcv_skb+0x184/0x220
[  508.805674]    netlink_unicast+0x31b/0x460
[  508.810129]    netlink_sendmsg+0x3fb/0x840
[  508.814611]    sock_sendmsg+0x7b/0xd0
[  508.818665]    ___sys_sendmsg+0x4c6/0x610
[  508.823029]    __sys_sendmsg+0xd7/0x150
[  508.827246]    do_syscall_64+0x7a/0x3f0
[  508.831483]    entry_SYSCALL_64_after_hwframe+0x49/0xbe

               the dependencies between the lock to be acquired
[  508.838945]  and SOFTIRQ-irq-unsafe lock:
[  508.851177] -> (ife_mod_lock){++++} ops: 95 {
[  508.855920]    HARDIRQ-ON-W at:
[  508.859478]                     _raw_write_lock+0x2c/0x40
[  508.865264]                     register_ife_op+0x118/0x2c0 [act_ife]
[  508.872071]                     do_one_initcall+0xf7/0x4d9
[  508.877947]                     do_init_module+0x18b/0x44e
[  508.883819]                     load_module+0x4167/0x5730
[  508.889595]                     __do_sys_finit_module+0x16d/0x1a0
[  508.896043]                     do_syscall_64+0x7a/0x3f0
[  508.901734]                     entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  508.908827]    HARDIRQ-ON-R at:
[  508.912359]                     _raw_read_lock+0x2f/0x40
[  508.918043]                     find_ife_oplist+0x1e/0xc0 [act_ife]
[  508.924692]                     tcf_ife_init+0x82f/0xf40 [act_ife]
[  508.931252]                     tcf_action_init_1+0x510/0x750
[  508.937393]                     tcf_action_init+0x1e8/0x340
[  508.943366]                     tcf_action_add+0xc5/0x240
[  508.949130]                     tc_ctl_action+0x203/0x2a0
[  508.954922]                     rtnetlink_rcv_msg+0x5bd/0x7b0
[  508.961024]                     netlink_rcv_skb+0x184/0x220
[  508.966970]                     netlink_unicast+0x31b/0x460
[  508.972915]                     netlink_sendmsg+0x3fb/0x840
[  508.978859]                     sock_sendmsg+0x7b/0xd0
[  508.984400]                     ___sys_sendmsg+0x4c6/0x610
[  508.990264]                     __sys_sendmsg+0xd7/0x150
[  508.995952]                     do_syscall_64+0x7a/0x3f0
[  509.001643]                     entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  509.008722]    SOFTIRQ-ON-W at:\
[  509.012242]                     _raw_write_lock+0x2c/0x40
[  509.018013]                     register_ife_op+0x118/0x2c0 [act_ife]
[  509.024841]                     do_one_initcall+0xf7/0x4d9
[  509.030720]                     do_init_module+0x18b/0x44e
[  509.036604]                     load_module+0x4167/0x5730
[  509.042397]                     __do_sys_finit_module+0x16d/0x1a0
[  509.048865]                     do_syscall_64+0x7a/0x3f0
[  509.054551]                     entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  509.061636]    SOFTIRQ-ON-R at:
[  509.065145]                     _raw_read_lock+0x2f/0x40
[  509.070854]                     find_ife_oplist+0x1e/0xc0 [act_ife]
[  509.077515]                     tcf_ife_init+0x82f/0xf40 [act_ife]
[  509.084051]                     tcf_action_init_1+0x510/0x750
[  509.090172]                     tcf_action_init+0x1e8/0x340
[  509.096124]                     tcf_action_add+0xc5/0x240
[  509.101891]                     tc_ctl_action+0x203/0x2a0
[  509.107671]                     rtnetlink_rcv_msg+0x5bd/0x7b0
[  509.113811]                     netlink_rcv_skb+0x184/0x220
[  509.119768]                     netlink_unicast+0x31b/0x460
[  509.125716]                     netlink_sendmsg+0x3fb/0x840
[  509.131668]                     sock_sendmsg+0x7b/0xd0
[  509.137167]                     ___sys_sendmsg+0x4c6/0x610
[  509.143010]                     __sys_sendmsg+0xd7/0x150
[  509.148718]                     do_syscall_64+0x7a/0x3f0
[  509.154443]                     entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  509.161533]    INITIAL USE at:
[  509.164956]                    _raw_read_lock+0x2f/0x40
[  509.170574]                    find_ife_oplist+0x1e/0xc0 [act_ife]
[  509.177134]                    tcf_ife_init+0x82f/0xf40 [act_ife]
[  509.183619]                    tcf_action_init_1+0x510/0x750
[  509.189674]                    tcf_action_init+0x1e8/0x340
[  509.195534]                    tcf_action_add+0xc5/0x240
[  509.201229]                    tc_ctl_action+0x203/0x2a0
[  509.206920]                    rtnetlink_rcv_msg+0x5bd/0x7b0
[  509.212936]                    netlink_rcv_skb+0x184/0x220
[  509.218818]                    netlink_unicast+0x31b/0x460
[  509.224699]                    netlink_sendmsg+0x3fb/0x840
[  509.230581]                    sock_sendmsg+0x7b/0xd0
[  509.235984]                    ___sys_sendmsg+0x4c6/0x610
[  509.241791]                    __sys_sendmsg+0xd7/0x150
[  509.247425]                    do_syscall_64+0x7a/0x3f0
[  509.253007]                    entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  509.259975]  }
[  509.261998]  ... key      at: [<ffffffffc1554258>] ife_mod_lock+0x18/0xffffffffffff8dc0 [act_ife]
[  509.271569]  ... acquired at:
[  509.274912]    _raw_read_lock+0x2f/0x40
[  509.279134]    find_ife_oplist+0x1e/0xc0 [act_ife]
[  509.284324]    tcf_ife_init+0x82f/0xf40 [act_ife]
[  509.289425]    tcf_action_init_1+0x510/0x750
[  509.294068]    tcf_action_init+0x1e8/0x340
[  509.298553]    tcf_action_add+0xc5/0x240
[  509.302854]    tc_ctl_action+0x203/0x2a0
[  509.307153]    rtnetlink_rcv_msg+0x5bd/0x7b0
[  509.311805]    netlink_rcv_skb+0x184/0x220
[  509.316282]    netlink_unicast+0x31b/0x460
[  509.320769]    netlink_sendmsg+0x3fb/0x840
[  509.325248]    sock_sendmsg+0x7b/0xd0
[  509.329290]    ___sys_sendmsg+0x4c6/0x610
[  509.333687]    __sys_sendmsg+0xd7/0x150
[  509.337902]    do_syscall_64+0x7a/0x3f0
[  509.342116]    entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  509.349601]
               stack backtrace:
[  509.354663] CPU: 6 PID: 5460 Comm: tc Not tainted 4.18.0-rc8+ #646
[  509.361216] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017

Fixes: ef6980b6becb ("introduce IFE action")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 net/sched/act_ife.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 5d200495e467..fdb928ca81bb 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -167,16 +167,16 @@ static struct tcf_meta_ops *find_ife_oplist(u16 metaid)
 {
 	struct tcf_meta_ops *o;
 
-	read_lock(&ife_mod_lock);
+	read_lock_bh(&ife_mod_lock);
 	list_for_each_entry(o, &ifeoplist, list) {
 		if (o->metaid == metaid) {
 			if (!try_module_get(o->owner))
 				o = NULL;
-			read_unlock(&ife_mod_lock);
+			read_unlock_bh(&ife_mod_lock);
 			return o;
 		}
 	}
-	read_unlock(&ife_mod_lock);
+	read_unlock_bh(&ife_mod_lock);
 
 	return NULL;
 }
@@ -190,12 +190,12 @@ int register_ife_op(struct tcf_meta_ops *mops)
 	    !mops->get || !mops->alloc)
 		return -EINVAL;
 
-	write_lock(&ife_mod_lock);
+	write_lock_bh(&ife_mod_lock);
 
 	list_for_each_entry(m, &ifeoplist, list) {
 		if (m->metaid == mops->metaid ||
 		    (strcmp(mops->name, m->name) == 0)) {
-			write_unlock(&ife_mod_lock);
+			write_unlock_bh(&ife_mod_lock);
 			return -EEXIST;
 		}
 	}
@@ -204,7 +204,7 @@ int register_ife_op(struct tcf_meta_ops *mops)
 		mops->release = ife_release_meta_gen;
 
 	list_add_tail(&mops->list, &ifeoplist);
-	write_unlock(&ife_mod_lock);
+	write_unlock_bh(&ife_mod_lock);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(unregister_ife_op);
@@ -214,7 +214,7 @@ int unregister_ife_op(struct tcf_meta_ops *mops)
 	struct tcf_meta_ops *m;
 	int err = -ENOENT;
 
-	write_lock(&ife_mod_lock);
+	write_lock_bh(&ife_mod_lock);
 	list_for_each_entry(m, &ifeoplist, list) {
 		if (m->metaid == mops->metaid) {
 			list_del(&mops->list);
@@ -222,7 +222,7 @@ int unregister_ife_op(struct tcf_meta_ops *mops)
 			break;
 		}
 	}
-	write_unlock(&ife_mod_lock);
+	write_unlock_bh(&ife_mod_lock);
 
 	return err;
 }
@@ -343,13 +343,13 @@ static int use_all_metadata(struct tcf_ife_info *ife)
 	int rc = 0;
 	int installed = 0;
 
-	read_lock(&ife_mod_lock);
+	read_lock_bh(&ife_mod_lock);
 	list_for_each_entry(o, &ifeoplist, list) {
 		rc = add_metainfo(ife, o->metaid, NULL, 0, true);
 		if (rc == 0)
 			installed += 1;
 	}
-	read_unlock(&ife_mod_lock);
+	read_unlock_bh(&ife_mod_lock);
 
 	if (installed)
 		return 0;
-- 
2.7.5

^ permalink raw reply related

* Re: [PATCH 0/3] net, IB/ipoib: Use dev_port to disambiguate
From: Jason Gunthorpe @ 2018-08-13 17:17 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, netdev
In-Reply-To: <20180813114224.7065-1-ar@cs.msu.ru>

On Mon, Aug 13, 2018 at 02:42:21PM +0300, Arseny Maslennikov wrote:
> Pre-3.15 userspace had trouble distinguishing different ports of a NIC
> on a single PCI bus/device/function. To solve this, a sysfs field `dev_port'
> was introduced quite a while ago (commit v3.14-rc3-739-g3f85944fe207), and
> some relevant device drivers were fixed to use it, but not in case of IPoIB.
> 
> The convention for some reason never got documented in the kernel, but
> was immediately adopted by userspace (notably udev[1][2], biosdevname[3])
> 
> 3/3 documents the sysfs field — that's why I'm CC-ing netdev.
> 
> This series was tested on current LTS and 4.18.
> 
> [1] https://lists.freedesktop.org/archives/systemd-devel/2014-June/020788.html
> [2] https://lists.freedesktop.org/archives/systemd-devel/2014-July/020804.html
> [3] https://github.com/CloudAutomationNTools/biosdevname/blob/c795d51dd93a5309652f0d635f12a3ecfabfaa72/src/eths.c#L38
> 
> Arseny Maslennikov (3):
>   IB/ipoib: Use dev_port to expose network interface port numbers
>   IB/ipoib: Stop using dev_id to expose port numbers
>   Documentation/ABI: document /sys/class/net/*/dev_port
> 
>  Documentation/ABI/testing/sysfs-class-net | 10 ++++++++++
>  drivers/infiniband/ulp/ipoib/ipoib_main.c |  2 +-
>  2 files changed, 11 insertions(+), 1 deletion(-)

This series doesn't apply to rdma for-next, and it is the merge window
now.

Can you resubmit this aginst 4.19-rc1 in two weeks? Thanks

Jason

^ permalink raw reply

* net-next is CLOSED...
From: David Miller @ 2018-08-13 17:08 UTC (permalink / raw)
  To: netdev


Please only submit bug fixes at this time, thank you.

^ permalink raw reply

* Re: pull-request: bpf-next 2018-08-13
From: David Miller @ 2018-08-13 17:08 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev
In-Reply-To: <20180812234922.2704-1-daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 13 Aug 2018 01:49:22 +0200

> The following pull-request contains BPF updates for your *net-next* tree.
> 
> The main changes are:
 ...
> Please consider pulling these changes from:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git

Pulled, thanks Daniel.

^ permalink raw reply

* Re: [PATCH bpf-next 0/4] Convert filter.txt to RST
From: Alexei Starovoitov @ 2018-08-13 19:37 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: Jonathan Corbet, Daniel Borkmann, Alexei Starovoitov,
	David S. Miller, Kees Cook, Andy Lutomirski, Will Drewry,
	linux-doc, Network Development, LKML
In-Reply-To: <20180811115058.GN32374@eros>

On Sat, Aug 11, 2018 at 09:50:58PM +1000, Tobin C. Harding wrote:
> On Fri, Aug 10, 2018 at 10:51:28AM -0700, Alexei Starovoitov wrote:
> > On Fri, Aug 10, 2018 at 5:57 AM Jonathan Corbet <corbet@lwn.net> wrote:
> > >
> > > The objective actually is to have SPDX tags in all files in the kernel.
> > > That includes documentation, even though people, as always, care less
> > > about the docs than they do the code.
> > 
> > right, but let's do that as a separate patch set.
> > In the current set I'd focus on reviewing the actual doc changes.
> > In particular completely removing
> > Documentation/networking/filter.txt
> > feels wrong, since lots of websites point directly there.
> > Can we have at least few words there pointing to new location?
> 
> Something like ...
> 
> 
> ------------ filter.txt
> 
> BPF documentation can now be found in the following places:
> 
> - Introduction to BPF (Linux Socket Filter) - Documentation/userspace-api/socket-filter.rst
> - Classic BPF (cBPF) - Documentation/userspace-api/cBPF.rst
> - Internal BPF (eBPF) - Documentation/userspace-api/eBPF.rst

Internal ?
that was the name we used for may be a month many years ago.
Please use 'extended BPF' in new filter.txt and all other places.

since merge window is open the patches would need to wait until
bpf-next opens up in few weeks.

Thanks

^ permalink raw reply

* Re: [PATCH net-next] lan743x: lan743x: Remove duplicated include from lan743x_ptp.c
From: David Miller @ 2018-08-13 16:46 UTC (permalink / raw)
  To: yuehaibing; +Cc: bryan.whitehead, UNGLinuxDriver, netdev, kernel-janitors
In-Reply-To: <1534142361-92174-1-git-send-email-yuehaibing@huawei.com>

From: Yue Haibing <yuehaibing@huawei.com>
Date: Mon, 13 Aug 2018 06:39:21 +0000

> Remove duplicated include.
> 
> Signed-off-by: Yue Haibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 iproute2-next] sch_cake: Make gso-splitting configurable
From: Toke Høiland-Jørgensen @ 2018-08-13 16:44 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, Dave Taht
In-Reply-To: <f58a05f6-a544-c9b3-7ef7-ae83f37a682e@gmail.com>

David Ahern <dsahern@gmail.com> writes:

> On 8/13/18 5:36 AM, Toke Høiland-Jørgensen wrote:
>> This patch makes sch_cake's gso/gro splitting configurable
>> from userspace.
>> 
>> To disable breaking apart superpackets in sch_cake:
>> 
>> tc qdisc replace dev whatever root cake no-split-gso
>> 
>> to enable:
>> 
>> tc qdisc replace dev whatever root cake split-gso
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> Signed-off-by: Dave Taht <dave.taht@gmail.com>
>> ---
>
> applied to iproute2-next. Thanks

Great, thanks :)

> I think you also need to display it if the attribute is returned.

We already print 'split-gso' when it is set; this was previously decided
by the kernel, this change just makes it user-configurable...

-Toke

^ permalink raw reply

* Re: [PATCH][net-next][v2] packet: switch kvzalloc to allocate memory
From: David Miller @ 2018-08-13 16:37 UTC (permalink / raw)
  To: lirongqing; +Cc: netdev
In-Reply-To: <1534128166-11972-1-git-send-email-lirongqing@baidu.com>

From: Li RongQing <lirongqing@baidu.com>
Date: Mon, 13 Aug 2018 10:42:46 +0800

> The patches includes following change:
> 
> *Use modern kvzalloc()/kvfree() instead of custom allocations.
> 
> *Remove order argument for alloc_pg_vec, it can get from req.
> 
> *Remove order argument for free_pg_vec, free_pg_vec now uses
> kvfree which does not need order argument.
> 
> *Remove pg_vec_order from struct packet_ring_buffer, no longer
> need to save/restore 'order'
> 
> *Remove variable 'order' for packet_set_ring, it is now unused
> 
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] net: socket: Fix potential spectre v1 gadget in sock_is_registered
From: Jeremy Cline @ 2018-08-13 19:03 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: David S . Miller, netdev, linux-kernel, stable
In-Reply-To: <20180813171642.wlxmnzgsg2rkwe4o@treble>

On 08/13/2018 06:16 PM, Josh Poimboeuf wrote:
> On Sun, Jul 29, 2018 at 11:59:36AM -0400, Jeremy Cline wrote:
>> On 07/29/2018 09:59 AM, Josh Poimboeuf wrote:
>>> On Fri, Jul 27, 2018 at 10:43:02PM +0000, Jeremy Cline wrote:
>>>> 'family' can be a user-controlled value, so sanitize it after the bounds
>>>> check to avoid speculative out-of-bounds access.
>>>>
>>>> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Jeremy Cline <jcline@redhat.com>
>>>> ---
>>>>  net/socket.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/socket.c b/net/socket.c
>>>> index f15d5cbb3ba4..608e29ae6baf 100644
>>>> --- a/net/socket.c
>>>> +++ b/net/socket.c
>>>> @@ -2672,7 +2672,8 @@ EXPORT_SYMBOL(sock_unregister);
>>>>  
>>>>  bool sock_is_registered(int family)
>>>>  {
>>>> -	return family < NPROTO && rcu_access_pointer(net_families[family]);
>>>> +	return family < NPROTO &&
>>>> +		rcu_access_pointer(net_families[array_index_nospec(family, NPROTO)]);
>>>>  }
>>>>  
>>>>  static int __init sock_init(void)
>>>
>>> This is another one where I think it would be better to do the nospec
>>> clamp higher up the call chain.  The untrusted 'family' value comes from
>>> __sock_diag_cmd():
>>>
>>> __sock_diag_cmd
>>>   sock_load_diag_module
>>>     sock_is_registered
>>>
>>> That function has a bounds check, and also uses the value in some other
>>> array accesses:
>>>
>>> 	if (req->sdiag_family >= AF_MAX)
>>> 		return -EINVAL;
>>>
>>> 	if (sock_diag_handlers[req->sdiag_family] == NULL)
>>> 		sock_load_diag_module(req->sdiag_family, 0);
>>>
>>> 	mutex_lock(&sock_diag_table_mutex);
>>> 	hndl = sock_diag_handlers[req->sdiag_family];
>>> 	...
>>>
>>> So I think clamping 'req->sdiag_family' right after the bounds check
>>> would be the way to go.
>>>
>>
>> Indeed, the clamp there would cover this clamp. I had a scheme that I
>> quickly fix all the gadgets in functions with local comparisons, but
>> clearly that's going to result in call chains with multiple clamps.
>>
>> I can fix this in a follow-up with a clamp here, or respin this patch
>> set, whatever is easier for David.
> 
> Hi Jeremy,
> 
> Just checking up on this... since this patch was merged, will you be
> doing a followup patch?
> 

Yes, apologies, I've been traveling. I'll have a patch tomorrow.

^ 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