Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64
From: Daniel Borkmann @ 2017-08-04 13:52 UTC (permalink / raw)
  To: Michael Holzheu; +Cc: davem, ast, netdev
In-Reply-To: <20170804154420.6faae20a@TP-holzheu>

On 08/04/2017 03:44 PM, Michael Holzheu wrote:
> Am Fri,  4 Aug 2017 14:20:54 +0200
> schrieb Daniel Borkmann <daniel@iogearbox.net>:
[...]
>
> What about "Cc: stable@vger.kernel.org"?

Handled by Dave, see also: Documentation/networking/netdev-FAQ.txt +117

Cheers,
Daniel

^ permalink raw reply

* Re: [PATCH] samples/bpf: Fix cross compiler error with bpf sample
From: Daniel Borkmann @ 2017-08-04 13:58 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Alexei Starovoitov,
	open list:BPF (Safe dynamic programs and tools)
In-Reply-To: <20170804054623.5229-1-joelaf@google.com>

On 08/04/2017 07:46 AM, Joel Fernandes wrote:
> When cross-compiling the bpf sample map_perf_test for aarch64, I find that
> __NR_getpgrp is undefined. This causes build errors. Fix it by allowing the
> deprecated syscall in the sample.
>
> Signed-off-by: Joel Fernandes <joelaf@google.com>
> ---
>   samples/bpf/map_perf_test_user.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/samples/bpf/map_perf_test_user.c b/samples/bpf/map_perf_test_user.c
> index 1a8894b5ac51..6e6fc7121640 100644
> --- a/samples/bpf/map_perf_test_user.c
> +++ b/samples/bpf/map_perf_test_user.c
> @@ -8,7 +8,9 @@
>   #include <sched.h>
>   #include <stdio.h>
>   #include <sys/types.h>
> +#define __ARCH_WANT_SYSCALL_DEPRECATED
>   #include <asm/unistd.h>
> +#undef __ARCH_WANT_SYSCALL_DEPRECATED

So the only arch that sets __ARCH_WANT_SYSCALL_DEPRECATED
is score:

   $ git grep -n __ARCH_WANT_SYSCALL_DEPRECATED
   arch/score/include/uapi/asm/unistd.h:7:#define __ARCH_WANT_SYSCALL_DEPRECATED
   include/uapi/asm-generic/unistd.h:837:#ifdef __ARCH_WANT_SYSCALL_DEPRECATED
   include/uapi/asm-generic/unistd.h:899:#endif /* __ARCH_WANT_SYSCALL_DEPRECATED */

But even if this would make aarch64 compile, the syscall
numbers don't match up:

   $ git grep -n __NR_getpgrp include/uapi/asm-generic/unistd.h
   include/uapi/asm-generic/unistd.h:841:#define __NR_getpgrp 1060
   include/uapi/asm-generic/unistd.h:843:__SYSCALL(__NR_getpgrp, sys_getpgrp)

The only thing that can be found on arm64 is:

   $ git grep -n __NR_getpgrp arch/arm64/
   arch/arm64/include/asm/unistd32.h:154:#define __NR_getpgrp 65
   arch/arm64/include/asm/unistd32.h:155:__SYSCALL(__NR_getpgrp, sys_getpgrp)

In arch/arm64/include/asm/unistd.h, it does include the
uapi/asm/unistd.h when compat is not set, but without the
__ARCH_WANT_SYSCALL_DEPRECATED. That doesn't look correct
unless I'm missing something, hmm, can't we just attach the
kprobes to a different syscall, one that is not deprecated,
so that we don't run into this in the first place?

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH] PCI: Update ACS quirk for more Intel 10G NICs
From: Alex Williamson @ 2017-08-04 14:28 UTC (permalink / raw)
  To: David Laight
  Cc: 'Bjorn Helgaas', Roland Dreier, Bjorn Helgaas,
	linux-pci@vger.kernel.org, netdev@vger.kernel.org, Emil Tantilov
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD004B362@AcuExch.aculab.com>

On Fri, 4 Aug 2017 13:28:32 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Bjorn Helgaas
> > Sent: 03 August 2017 22:49
> > On Thu, Jul 20, 2017 at 02:41:01PM -0700, Roland Dreier wrote:  
> > > From: Roland Dreier <roland@purestorage.com>
> > >
> > > Add one more variant of the 82599 plus the device IDs for X540 and X550
> > > variants.  Intel has confirmed that none of these devices does peer-to-peer
> > > between functions.  The X540 and X550 have added ACS capabilities in their
> > > PCI config space, but the ACS control register is hard-wired to 0 for both
> > > devices, so we still need the quirk for IOMMU grouping to allow assignment
> > > of individual SR-IOV functions.  
> ...
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -4335,12 +4335,33 @@ static const struct pci_dev_acs_enabled {
> > >  	{ PCI_VENDOR_ID_INTEL, 0x1507, pci_quirk_mf_endpoint_acs },
> > >  	{ PCI_VENDOR_ID_INTEL, 0x1514, pci_quirk_mf_endpoint_acs },
> > >  	{ PCI_VENDOR_ID_INTEL, 0x151C, pci_quirk_mf_endpoint_acs },
> > > +	{ PCI_VENDOR_ID_INTEL, 0x1528, pci_quirk_mf_endpoint_acs },
> > >  	{ PCI_VENDOR_ID_INTEL, 0x1529, pci_quirk_mf_endpoint_acs },
> > > +	{ PCI_VENDOR_ID_INTEL, 0x154A, pci_quirk_mf_endpoint_acs },  
> ...
> 
> That list is looking a bit long.
> Is it possible to run-time determine that the ACS control register is hard wired
> to zero, and apply the quirk to all such devices.
> Or even changing to a (device & mask) == value test??

In fact, hard-wired ACS doesn't need a quirk at all, please see the
other thread of the discussion.  Thanks,

Alex

^ permalink raw reply

* RE: [PATCH] net: phy: marvell: logical vs bitwise OR typo
From: David Laight @ 2017-08-04 14:54 UTC (permalink / raw)
  To: 'Dan Carpenter', Andrew Lunn
  Cc: Florian Fainelli, netdev@vger.kernel.org,
	kernel-janitors@vger.kernel.org
In-Reply-To: <20170804081720.2g6yuthplhc4rv6b@mwanda>

From: Dan Carpenter
> Sent: 04 August 2017 09:17
> This was supposed to be a bitwise OR but there is a || vs | typo.
> 
> Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 361fe9927ef2..15cbcdba618a 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -83,7 +83,7 @@
>  #define MII_88E1121_PHY_MSCR_REG	21
>  #define MII_88E1121_PHY_MSCR_RX_DELAY	BIT(5)
>  #define MII_88E1121_PHY_MSCR_TX_DELAY	BIT(4)
> -#define MII_88E1121_PHY_MSCR_DELAY_MASK	(~(BIT(5) || BIT(4)))
> +#define MII_88E1121_PHY_MSCR_DELAY_MASK	(~(BIT(5) | BIT(4)))

Wouldn't:
+#define MII_88E1121_PHY_MSCR_DELAY_MASK	(~(MII_88E1121_PHY_MSCR_RX_DELAY | MII_88E1121_PHY_MSCR_TX_DELAY))
be more correct?
If a little long?
Actually the ~ looks odd here....
Reads code....
Kill the define and explicitly mask off the two values just before
conditionally setting them.

	David

^ permalink raw reply

* [PATCH 0/6] In-kernel QMI handling
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel

This series starts by moving the common definitions of the QMUX protocol to the
uapi header, as they are shared with clients - both in kernel and userspace.

This series then introduces in-kernel helper functions for aiding the handling
of QMI encoded messages in the kernel. QMI encoding is a wire-format used in
exchanging messages between the majority of QRTR clients and services.

It then adds an abstractions to reduce the duplication of common code in
drivers that needs to query the name server and send and receive encoded
messages to a remote service.

Finally it introduces a sample implementation for showing QRTR and the QMI
helpers in action. The sample device instantiates in response to finding the
"test service" and implements the "test protocol".

Bjorn Andersson (6):
  net: qrtr: Invoke sk_error_report() after setting sk_err
  net: qrtr: Move constants to header file
  net: qrtr: Add control packet definition to uapi
  soc: qcom: Introduce QMI encoder/decoder
  soc: qcom: Introduce QMI helpers
  samples: Introduce Qualcomm QRTR sample client

 drivers/soc/qcom/Kconfig          |   8 +
 drivers/soc/qcom/Makefile         |   3 +
 drivers/soc/qcom/qmi_encdec.c     | 812 ++++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/qmi_interface.c  | 540 +++++++++++++++++++++++++
 include/linux/soc/qcom/qmi.h      | 249 ++++++++++++
 include/uapi/linux/qrtr.h         |  35 ++
 net/qrtr/qrtr.c                   |  16 +-
 samples/Kconfig                   |   8 +
 samples/Makefile                  |   2 +-
 samples/qrtr/Makefile             |   1 +
 samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++
 11 files changed, 2261 insertions(+), 16 deletions(-)
 create mode 100644 drivers/soc/qcom/qmi_encdec.c
 create mode 100644 drivers/soc/qcom/qmi_interface.c
 create mode 100644 include/linux/soc/qcom/qmi.h
 create mode 100644 samples/qrtr/Makefile
 create mode 100644 samples/qrtr/qrtr_sample_client.c

-- 
2.12.0

^ permalink raw reply

* [PATCH 1/6] net: qrtr: Invoke sk_error_report() after setting sk_err
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

Rather than manually waking up any context sleeping on the sock to
signal an error we should call sk_error_report(). This has the added
benefit that in-kernel consumers can override this notificatino with
its own callback.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 net/qrtr/qrtr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 5586609afa27..2058b27821a4 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -541,7 +541,7 @@ static void qrtr_reset_ports(void)
 
 		sock_hold(&ipc->sk);
 		ipc->sk.sk_err = ENETRESET;
-		wake_up_interruptible(sk_sleep(&ipc->sk));
+		ipc->sk.sk_error_report(&ipc->sk);
 		sock_put(&ipc->sk);
 	}
 	mutex_unlock(&qrtr_port_lock);
-- 
2.12.0

^ permalink raw reply related

* [PATCH 2/6] net: qrtr: Move constants to header file
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

The constants are used by both the name server and clients, so make
their value explicit and move them to the uapi header.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 include/uapi/linux/qrtr.h | 3 +++
 net/qrtr/qrtr.c           | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
index 9d76c566f66e..63e8803e4d90 100644
--- a/include/uapi/linux/qrtr.h
+++ b/include/uapi/linux/qrtr.h
@@ -4,6 +4,9 @@
 #include <linux/socket.h>
 #include <linux/types.h>
 
+#define QRTR_NODE_BCAST	0xffffffffu
+#define QRTR_PORT_CTRL	0xfffffffeu
+
 struct sockaddr_qrtr {
 	__kernel_sa_family_t sq_family;
 	__u32 sq_node;
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 2058b27821a4..0d7d3968414e 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -61,8 +61,6 @@ struct qrtr_hdr {
 } __packed;
 
 #define QRTR_HDR_SIZE sizeof(struct qrtr_hdr)
-#define QRTR_NODE_BCAST ((unsigned int)-1)
-#define QRTR_PORT_CTRL ((unsigned int)-2)
 
 struct qrtr_sock {
 	/* WARNING: sk must be the first member */
-- 
2.12.0

^ permalink raw reply related

* [PATCH 3/6] net: qrtr: Add control packet definition to uapi
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

The QMUX protocol specification defines structure of the special control
packet messages being sent between handlers of the control port.

Add these to the uapi header, as this structure and the associated types
are shared between the kernel and all userspace handlers of control
messages.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 include/uapi/linux/qrtr.h | 32 ++++++++++++++++++++++++++++++++
 net/qrtr/qrtr.c           | 12 ------------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
index 63e8803e4d90..179af64846e0 100644
--- a/include/uapi/linux/qrtr.h
+++ b/include/uapi/linux/qrtr.h
@@ -13,4 +13,36 @@ struct sockaddr_qrtr {
 	__u32 sq_port;
 };
 
+enum qrtr_pkt_type {
+	QRTR_TYPE_DATA		= 1,
+	QRTR_TYPE_HELLO		= 2,
+	QRTR_TYPE_BYE		= 3,
+	QRTR_TYPE_NEW_SERVER	= 4,
+	QRTR_TYPE_DEL_SERVER	= 5,
+	QRTR_TYPE_DEL_CLIENT	= 6,
+	QRTR_TYPE_RESUME_TX	= 7,
+	QRTR_TYPE_EXIT          = 8,
+	QRTR_TYPE_PING          = 9,
+	QRTR_TYPE_NEW_LOOKUP	= 10,
+	QRTR_TYPE_DEL_LOOKUP	= 11,
+};
+
+struct qrtr_ctrl_pkt {
+	__le32 cmd;
+
+	union {
+		struct {
+			__le32 service;
+			__le32 instance;
+			__le32 node;
+			__le32 port;
+		} server;
+
+		struct {
+			__le32 node;
+			__le32 port;
+		} client;
+	};
+} __packed;
+
 #endif /* _LINUX_QRTR_H */
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 0d7d3968414e..fac7cd6ea445 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -26,18 +26,6 @@
 #define QRTR_MIN_EPH_SOCKET 0x4000
 #define QRTR_MAX_EPH_SOCKET 0x7fff
 
-enum qrtr_pkt_type {
-	QRTR_TYPE_DATA		= 1,
-	QRTR_TYPE_HELLO		= 2,
-	QRTR_TYPE_BYE		= 3,
-	QRTR_TYPE_NEW_SERVER	= 4,
-	QRTR_TYPE_DEL_SERVER	= 5,
-	QRTR_TYPE_DEL_CLIENT	= 6,
-	QRTR_TYPE_RESUME_TX	= 7,
-	QRTR_TYPE_EXIT		= 8,
-	QRTR_TYPE_PING		= 9,
-};
-
 /**
  * struct qrtr_hdr - (I|R)PCrouter packet header
  * @version: protocol version
-- 
2.12.0

^ permalink raw reply related

* [PATCH 4/6] soc: qcom: Introduce QMI encoder/decoder
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

Add the helper library for encoding and decoding QMI encoded messages.
The implementation is taken from lib/qmi_encdec.c of the Qualcomm kernel
(msm-3.18).

Modifications has been made to the public API, source buffers has been
made const and the debug-logging part was omitted, for now.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/Kconfig      |   8 +
 drivers/soc/qcom/Makefile     |   2 +
 drivers/soc/qcom/qmi_encdec.c | 812 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/qmi.h  | 116 ++++++
 4 files changed, 938 insertions(+)
 create mode 100644 drivers/soc/qcom/qmi_encdec.c
 create mode 100644 include/linux/soc/qcom/qmi.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 9fca977ef18d..2541ae07ad2a 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -24,6 +24,14 @@ config QCOM_PM
 	  modes. It interface with various system drivers to put the cores in
 	  low power modes.
 
+config QCOM_QMI_HELPERS
+	bool
+	help
+	  Helper library for handling QMI encoded messages.  QMI encoded
+	  messages are used in communication between the majority of QRTR
+	  clients and this helpers provide the common functionality needed for
+	  doing this from a kernel driver.
+
 config QCOM_SMEM
 	tristate "Qualcomm Shared Memory Manager (SMEM)"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 414f0de274fa..27b60da7a062 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,6 +1,8 @@
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_PM)	+=	spm.o
+obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
+qmi_helpers-y	+= qmi_encdec.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= smd-rpm.o
 obj-$(CONFIG_QCOM_SMEM) +=	smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
new file mode 100644
index 000000000000..4eb3099c64e7
--- /dev/null
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -0,0 +1,812 @@
+/*
+ * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/string.h>
+#include <linux/soc/qcom/qmi.h>
+
+#define QMI_ENCDEC_ENCODE_TLV(type, length, p_dst) do { \
+	*p_dst++ = type; \
+	*p_dst++ = ((uint8_t)((length) & 0xFF)); \
+	*p_dst++ = ((uint8_t)(((length) >> 8) & 0xFF)); \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_TLV(p_type, p_length, p_src) do { \
+	*p_type = (uint8_t)*p_src++; \
+	*p_length = (uint8_t)*p_src++; \
+	*p_length |= ((uint8_t)*p_src) << 8; \
+} while (0)
+
+#define QMI_ENCDEC_ENCODE_N_BYTES(p_dst, p_src, size) \
+do { \
+	memcpy(p_dst, p_src, size); \
+	p_dst = (uint8_t *)p_dst + size; \
+	p_src = (uint8_t *)p_src + size; \
+} while (0)
+
+#define QMI_ENCDEC_DECODE_N_BYTES(p_dst, p_src, size) \
+do { \
+	memcpy(p_dst, p_src, size); \
+	p_dst = (uint8_t *)p_dst + size; \
+	p_src = (uint8_t *)p_src + size; \
+} while (0)
+
+#define UPDATE_ENCODE_VARIABLES(temp_si, buf_dst, \
+				encoded_bytes, tlv_len, encode_tlv, rc) \
+do { \
+	buf_dst = (uint8_t *)buf_dst + rc; \
+	encoded_bytes += rc; \
+	tlv_len += rc; \
+	temp_si = temp_si + 1; \
+	encode_tlv = 1; \
+} while (0)
+
+#define UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc) \
+do { \
+	buf_src = (uint8_t *)buf_src + rc; \
+	decoded_bytes += rc; \
+} while (0)
+
+#define TLV_LEN_SIZE sizeof(uint16_t)
+#define TLV_TYPE_SIZE sizeof(uint8_t)
+#define OPTIONAL_TLV_TYPE_START 0x10
+
+static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
+		      const void *in_c_struct, uint32_t out_buf_len,
+		      int enc_level);
+
+static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
+		      const void *in_buf, uint32_t in_buf_len, int dec_level);
+
+/**
+ * skip_to_next_elem() - Skip to next element in the structure to be encoded
+ * @ei_array: Struct info describing the element to be skipped.
+ * @level: Depth level of encoding/decoding to identify nested structures.
+ *
+ * Returns struct info of the next element that can be encoded.
+ *
+ * This function is used while encoding optional elements. If the flag
+ * corresponding to an optional element is not set, then encoding the
+ * optional element can be skipped. This function can be used to perform
+ * that operation.
+ */
+static struct qmi_elem_info *skip_to_next_elem(struct qmi_elem_info *ei_array,
+					   int level)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint8_t tlv_type;
+
+	if (level > 1) {
+		temp_ei = temp_ei + 1;
+	} else {
+		do {
+			tlv_type = temp_ei->tlv_type;
+			temp_ei = temp_ei + 1;
+		} while (tlv_type == temp_ei->tlv_type);
+	}
+
+	return temp_ei;
+}
+
+/**
+ * qmi_calc_min_msg_len() - Calculate the minimum length of a QMI message
+ * @ei_array: Struct info array describing the structure.
+ * @level: Level to identify the depth of the nested structures.
+ *
+ * Returns expected minimum length of the QMI message or 0 on error.
+ */
+static int qmi_calc_min_msg_len(struct qmi_elem_info *ei_array,
+				int level)
+{
+	int min_msg_len = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	if (!ei_array)
+		return min_msg_len;
+
+	while (temp_ei->data_type != QMI_EOTI) {
+		/* Optional elements do not count in minimum length */
+		if (temp_ei->data_type == QMI_OPT_FLAG) {
+			temp_ei = skip_to_next_elem(temp_ei, level);
+			continue;
+		}
+
+		if (temp_ei->data_type == QMI_DATA_LEN) {
+			min_msg_len += (temp_ei->elem_size == sizeof(uint8_t) ?
+					sizeof(uint8_t) : sizeof(uint16_t));
+			temp_ei++;
+			continue;
+		} else if (temp_ei->data_type == QMI_STRUCT) {
+			min_msg_len += qmi_calc_min_msg_len(temp_ei->ei_array,
+							    (level + 1));
+			temp_ei++;
+		} else if (temp_ei->data_type == QMI_STRING) {
+			if (level > 1)
+				min_msg_len += temp_ei->elem_len <= U8_MAX ?
+					sizeof(uint8_t) : sizeof(uint16_t);
+			min_msg_len += temp_ei->elem_len * temp_ei->elem_size;
+			temp_ei++;
+		} else {
+			min_msg_len += (temp_ei->elem_len * temp_ei->elem_size);
+			temp_ei++;
+		}
+
+		/*
+		 * Type & Length info. not prepended for elements in the
+		 * nested structure.
+		 */
+		if (level == 1)
+			min_msg_len += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+	}
+	return min_msg_len;
+}
+
+/**
+ * qmi_encode_basic_elem() - Encodes elements of basic/primary data type
+ * @buf_dst: Buffer to store the encoded information.
+ * @buf_src: Buffer containing the elements to be encoded.
+ * @elem_len: Number of elements, in the buf_src, to be encoded.
+ * @elem_size: Size of a single instance of the element to be encoded.
+ *
+ * Returns the number of bytes of encoded information.
+ *
+ * This function encodes the "elem_len" number of data elements, each of
+ * size "elem_size" bytes from the source buffer "buf_src" and stores the
+ * encoded information in the destination buffer "buf_dst". The elements are
+ * of primary data type which include uint8_t - uint64_t or similar. This
+ * function returns the number of bytes of encoded information.
+ */
+static int qmi_encode_basic_elem(void *buf_dst, const void *buf_src,
+				 uint32_t elem_len, uint32_t elem_size)
+{
+	uint32_t i, rc = 0;
+
+	for (i = 0; i < elem_len; i++) {
+		QMI_ENCDEC_ENCODE_N_BYTES(buf_dst, buf_src, elem_size);
+		rc += elem_size;
+	}
+
+	return rc;
+}
+
+/**
+ * qmi_encode_struct_elem() - Encodes elements of struct data type
+ * @ei_array: Struct info array descibing the struct element.
+ * @buf_dst: Buffer to store the encoded information.
+ * @buf_src: Buffer containing the elements to be encoded.
+ * @elem_len: Number of elements, in the buf_src, to be encoded.
+ * @out_buf_len: Available space in the encode buffer.
+ * @enc_level: Depth of the nested structure from the main structure.
+ *
+ * Returns the number of bytes of encoded information on success or negative
+ * errno on error.
+ *
+ * This function encodes the "elem_len" number of struct elements, each of
+ * size "ei_array->elem_size" bytes from the source buffer "buf_src" and
+ * stores the encoded information in the destination buffer "buf_dst". The
+ * elements are of struct data type which includes any C structure. This
+ * function returns the number of bytes of encoded information.
+ */
+static int qmi_encode_struct_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t elem_len, uint32_t out_buf_len,
+				  int enc_level)
+{
+	int i, rc, encoded_bytes = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	for (i = 0; i < elem_len; i++) {
+		rc = qmi_encode(temp_ei->ei_array, buf_dst, buf_src,
+				out_buf_len - encoded_bytes, enc_level);
+		if (rc < 0) {
+			pr_err("%s: STRUCT Encode failure\n", __func__);
+			return rc;
+		}
+		buf_dst = buf_dst + rc;
+		buf_src = buf_src + temp_ei->elem_size;
+		encoded_bytes += rc;
+	}
+
+	return encoded_bytes;
+}
+
+/**
+ * qmi_encode_string_elem() - Encodes elements of string data type
+ * @ei_array: Struct info array descibing the string element.
+ * @buf_dst: Buffer to store the encoded information.
+ * @buf_src: Buffer containing the elements to be encoded.
+ * @out_buf_len: Available space in the encode buffer.
+ * @enc_level: Depth of the string element from the main structure.
+ *
+ * Returns the number of bytes of encoded information on success or negative
+ * errno on error.
+ *
+ * This function encodes a string element of maximum length "ei_array->elem_len"
+ * bytes from the source buffer "buf_src" and stores the encoded information in
+ * the destination buffer "buf_dst". This function returns the number of bytes
+ * of encoded information.
+ */
+static int qmi_encode_string_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t out_buf_len, int enc_level)
+{
+	int rc;
+	int encoded_bytes = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint32_t string_len = 0;
+	uint32_t string_len_sz = 0;
+
+	string_len = strlen(buf_src);
+	string_len_sz = temp_ei->elem_len <= U8_MAX ?
+			sizeof(uint8_t) : sizeof(uint16_t);
+	if (string_len > temp_ei->elem_len) {
+		pr_err("%s: String to be encoded is longer - %d > %d\n",
+			__func__, string_len, temp_ei->elem_len);
+		return -EINVAL;
+	}
+
+	if (enc_level == 1) {
+		if (string_len + TLV_LEN_SIZE + TLV_TYPE_SIZE >
+		    out_buf_len) {
+			pr_err("%s: Output len %d > Out Buf len %d\n",
+				__func__, string_len, out_buf_len);
+			return -ETOOSMALL;
+		}
+	} else {
+		if (string_len + string_len_sz > out_buf_len) {
+			pr_err("%s: Output len %d > Out Buf len %d\n",
+				__func__, string_len, out_buf_len);
+			return -ETOOSMALL;
+		}
+		rc = qmi_encode_basic_elem(buf_dst, &string_len,
+					   1, string_len_sz);
+		encoded_bytes += rc;
+	}
+
+	rc = qmi_encode_basic_elem(buf_dst + encoded_bytes, buf_src,
+				   string_len, temp_ei->elem_size);
+	encoded_bytes += rc;
+	return encoded_bytes;
+}
+
+/**
+ * qmi_encode() - Core Encode Function
+ * @ei_array: Struct info array describing the structure to be encoded.
+ * @out_buf: Buffer to hold the encoded QMI message.
+ * @in_c_struct: Pointer to the C structure to be encoded.
+ * @out_buf_len: Available space in the encode buffer.
+ * @enc_level: Encode level to indicate the depth of the nested structure,
+ *             within the main structure, being encoded.
+ *
+ * Returns the number of bytes of encoded information on success or negative
+ * errno on error.
+ */
+static int qmi_encode(struct qmi_elem_info *ei_array, void *out_buf,
+		      const void *in_c_struct, uint32_t out_buf_len,
+		      int enc_level)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint8_t opt_flag_value = 0;
+	uint32_t data_len_value = 0, data_len_sz;
+	uint8_t *buf_dst = (uint8_t *)out_buf;
+	uint8_t *tlv_pointer;
+	uint32_t tlv_len;
+	uint8_t tlv_type;
+	uint32_t encoded_bytes = 0;
+	const void *buf_src;
+	int encode_tlv = 0;
+	int rc;
+
+	tlv_pointer = buf_dst;
+	tlv_len = 0;
+	if (enc_level == 1)
+		buf_dst = buf_dst + (TLV_LEN_SIZE + TLV_TYPE_SIZE);
+
+	while (temp_ei->data_type != QMI_EOTI) {
+		buf_src = in_c_struct + temp_ei->offset;
+		tlv_type = temp_ei->tlv_type;
+
+		if (temp_ei->is_array == NO_ARRAY) {
+			data_len_value = 1;
+		} else if (temp_ei->is_array == STATIC_ARRAY) {
+			data_len_value = temp_ei->elem_len;
+		} else if (data_len_value <= 0 ||
+			    temp_ei->elem_len < data_len_value) {
+			pr_err("%s: Invalid data length\n", __func__);
+			return -EINVAL;
+		}
+
+		switch (temp_ei->data_type) {
+		case QMI_OPT_FLAG:
+			rc = qmi_encode_basic_elem(&opt_flag_value, buf_src,
+						   1, sizeof(uint8_t));
+			if (opt_flag_value)
+				temp_ei = temp_ei + 1;
+			else
+				temp_ei = skip_to_next_elem(temp_ei, enc_level);
+			break;
+
+		case QMI_DATA_LEN:
+			memcpy(&data_len_value, buf_src, temp_ei->elem_size);
+			data_len_sz = temp_ei->elem_size == sizeof(uint8_t) ?
+					sizeof(uint8_t) : sizeof(uint16_t);
+			/* Check to avoid out of range buffer access */
+			if ((data_len_sz + encoded_bytes + TLV_LEN_SIZE +
+			    TLV_TYPE_SIZE) > out_buf_len) {
+				pr_err("%s: Too Small Buffer @DATA_LEN\n",
+					__func__);
+				return -ETOOSMALL;
+			}
+			rc = qmi_encode_basic_elem(buf_dst, &data_len_value,
+						   1, data_len_sz);
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			if (!data_len_value)
+				temp_ei = skip_to_next_elem(temp_ei, enc_level);
+			else
+				encode_tlv = 0;
+			break;
+
+		case QMI_UNSIGNED_1_BYTE:
+		case QMI_UNSIGNED_2_BYTE:
+		case QMI_UNSIGNED_4_BYTE:
+		case QMI_UNSIGNED_8_BYTE:
+		case QMI_SIGNED_2_BYTE_ENUM:
+		case QMI_SIGNED_4_BYTE_ENUM:
+			/* Check to avoid out of range buffer access */
+			if (((data_len_value * temp_ei->elem_size) +
+			    encoded_bytes + TLV_LEN_SIZE + TLV_TYPE_SIZE) >
+			    out_buf_len) {
+				pr_err("%s: Too Small Buffer @data_type:%d\n",
+					__func__, temp_ei->data_type);
+				return -ETOOSMALL;
+			}
+			rc = qmi_encode_basic_elem(buf_dst, buf_src,
+				data_len_value, temp_ei->elem_size);
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			break;
+
+		case QMI_STRUCT:
+			rc = qmi_encode_struct_elem(temp_ei, buf_dst, buf_src,
+				data_len_value, (out_buf_len - encoded_bytes),
+				(enc_level + 1));
+			if (rc < 0)
+				return rc;
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			break;
+
+		case QMI_STRING:
+			rc = qmi_encode_string_elem(temp_ei, buf_dst, buf_src,
+				out_buf_len - encoded_bytes, enc_level);
+			if (rc < 0)
+				return rc;
+			UPDATE_ENCODE_VARIABLES(temp_ei, buf_dst,
+				encoded_bytes, tlv_len, encode_tlv, rc);
+			break;
+		default:
+			pr_err("%s: Unrecognized data type\n", __func__);
+			return -EINVAL;
+
+		}
+
+		if (encode_tlv && enc_level == 1) {
+			QMI_ENCDEC_ENCODE_TLV(tlv_type, tlv_len, tlv_pointer);
+			encoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+			tlv_pointer = buf_dst;
+			tlv_len = 0;
+			buf_dst = buf_dst + TLV_LEN_SIZE + TLV_TYPE_SIZE;
+			encode_tlv = 0;
+		}
+	}
+	return encoded_bytes;
+}
+
+/**
+ * qmi_decode_basic_elem() - Decodes elements of basic/primary data type
+ * @buf_dst: Buffer to store the decoded element.
+ * @buf_src: Buffer containing the elements in QMI wire format.
+ * @elem_len: Number of elements to be decoded.
+ * @elem_size: Size of a single instance of the element to be decoded.
+ *
+ * Returns the total size of the decoded data elements, in bytes.
+ *
+ * This function decodes the "elem_len" number of elements in QMI wire format,
+ * each of size "elem_size" bytes from the source buffer "buf_src" and stores
+ * the decoded elements in the destination buffer "buf_dst". The elements are
+ * of primary data type which include uint8_t - uint64_t or similar. This
+ * function returns the number of bytes of decoded information.
+ */
+static int qmi_decode_basic_elem(void *buf_dst, const void *buf_src,
+				 uint32_t elem_len, uint32_t elem_size)
+{
+	uint32_t i, rc = 0;
+
+	for (i = 0; i < elem_len; i++) {
+		QMI_ENCDEC_DECODE_N_BYTES(buf_dst, buf_src, elem_size);
+		rc += elem_size;
+	}
+
+	return rc;
+}
+
+/**
+ * qmi_decode_struct_elem() - Decodes elements of struct data type
+ * @ei_array: Struct info array descibing the struct element.
+ * @buf_dst: Buffer to store the decoded element.
+ * @buf_src: Buffer containing the elements in QMI wire format.
+ * @elem_len: Number of elements to be decoded.
+ * @tlv_len: Total size of the encoded inforation corresponding to
+ *           this struct element.
+ * @dec_level: Depth of the nested structure from the main structure.
+ *
+ * Returns the total size of the decoded data elements on success, negative
+ * errno on error.
+ *
+ * This function decodes the "elem_len" number of elements in QMI wire format,
+ * each of size "(tlv_len/elem_len)" bytes from the source buffer "buf_src"
+ * and stores the decoded elements in the destination buffer "buf_dst". The
+ * elements are of struct data type which includes any C structure. This
+ * function returns the number of bytes of decoded information.
+ */
+static int qmi_decode_struct_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t elem_len, uint32_t tlv_len,
+				  int dec_level)
+{
+	int i, rc, decoded_bytes = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	for (i = 0; i < elem_len && decoded_bytes < tlv_len; i++) {
+		rc = qmi_decode(temp_ei->ei_array, buf_dst, buf_src,
+				tlv_len - decoded_bytes, dec_level);
+		if (rc < 0)
+			return rc;
+		buf_src = buf_src + rc;
+		buf_dst = buf_dst + temp_ei->elem_size;
+		decoded_bytes += rc;
+	}
+
+	if ((dec_level <= 2 && decoded_bytes != tlv_len) ||
+	    (dec_level > 2 && (i < elem_len || decoded_bytes > tlv_len))) {
+		pr_err("%s: Fault in decoding: dl(%d), db(%d), tl(%d), i(%d), el(%d)\n",
+			__func__, dec_level, decoded_bytes, tlv_len,
+			i, elem_len);
+		return -EFAULT;
+	}
+	return decoded_bytes;
+}
+
+/**
+ * qmi_decode_string_elem() - Decodes elements of string data type
+ * @ei_array: Struct info array descibing the string element.
+ * @buf_dst: Buffer to store the decoded element.
+ * @buf_src: Buffer containing the elements in QMI wire format.
+ * @tlv_len: Total size of the encoded inforation corresponding to
+ *           this string element.
+ * @dec_level: Depth of the string element from the main structure.
+ *
+
+ * Returns the total size of the decoded data elements on success, negative
+ * errno on error.
+
+ *
+ * This function decodes the string element of maximum length
+ * "ei_array->elem_len" from the source buffer "buf_src" and puts it into
+ * the destination buffer "buf_dst". This function returns number of bytes
+ * decoded from the input buffer.
+ */
+static int qmi_decode_string_elem(struct qmi_elem_info *ei_array,
+				  void *buf_dst, const void *buf_src,
+				  uint32_t tlv_len, int dec_level)
+{
+	int rc;
+	int decoded_bytes = 0;
+	uint32_t string_len = 0;
+	uint32_t string_len_sz = 0;
+	struct qmi_elem_info *temp_ei = ei_array;
+
+	if (dec_level == 1) {
+		string_len = tlv_len;
+	} else {
+		string_len_sz = temp_ei->elem_len <= U8_MAX ?
+				sizeof(uint8_t) : sizeof(uint16_t);
+		rc = qmi_decode_basic_elem(&string_len, buf_src,
+					   1, string_len_sz);
+		decoded_bytes += rc;
+	}
+
+	if (string_len > temp_ei->elem_len) {
+		pr_err("%s: String len %d > Max Len %d\n",
+			__func__, string_len, temp_ei->elem_len);
+		return -ETOOSMALL;
+	} else if (string_len > tlv_len) {
+		pr_err("%s: String len %d > Input Buffer Len %d\n",
+			__func__, string_len, tlv_len);
+		return -EFAULT;
+	}
+
+	rc = qmi_decode_basic_elem(buf_dst, buf_src + decoded_bytes,
+				   string_len, temp_ei->elem_size);
+	*((char *)buf_dst + string_len) = '\0';
+	decoded_bytes += rc;
+	return decoded_bytes;
+}
+
+/**
+ * find_ei() - Find element info corresponding to TLV Type
+ * @ei_array: Struct info array of the message being decoded.
+ * @type: TLV Type of the element being searched.
+ *
+ * Returns pointer to struct info, if found
+ *
+ * Every element that got encoded in the QMI message will have a type
+ * information associated with it. While decoding the QMI message,
+ * this function is used to find the struct info regarding the element
+ * that corresponds to the type being decoded.
+ */
+static struct qmi_elem_info *find_ei(struct qmi_elem_info *ei_array,
+				   uint32_t type)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	while (temp_ei->data_type != QMI_EOTI) {
+		if (temp_ei->tlv_type == (uint8_t)type)
+			return temp_ei;
+		temp_ei = temp_ei + 1;
+	}
+	return NULL;
+}
+
+/**
+ * qmi_decode() - Core Decode Function
+ * @ei_array: Struct info array describing the structure to be decoded.
+ * @out_c_struct: Buffer to hold the decoded C struct
+ * @in_buf: Buffer containing the QMI message to be decoded
+ * @in_buf_len: Length of the QMI message to be decoded
+ * @dec_level: Decode level to indicate the depth of the nested structure,
+ *             within the main structure, being decoded
+ *
+ * Returns the number of bytes of decoded information on success, negative
+ * errno on error.
+ */
+static int qmi_decode(struct qmi_elem_info *ei_array, void *out_c_struct,
+		      const void *in_buf, uint32_t in_buf_len,
+		      int dec_level)
+{
+	struct qmi_elem_info *temp_ei = ei_array;
+	uint8_t opt_flag_value = 1;
+	uint32_t data_len_value = 0, data_len_sz = 0;
+	uint8_t *buf_dst = out_c_struct;
+	const uint8_t *tlv_pointer;
+	uint32_t tlv_len = 0;
+	uint32_t tlv_type;
+	uint32_t decoded_bytes = 0;
+	const void *buf_src = in_buf;
+	int rc;
+
+	while (decoded_bytes < in_buf_len) {
+		if (dec_level >= 2 && temp_ei->data_type == QMI_EOTI)
+			return decoded_bytes;
+
+		if (dec_level == 1) {
+			tlv_pointer = buf_src;
+			QMI_ENCDEC_DECODE_TLV(&tlv_type,
+					      &tlv_len, tlv_pointer);
+			buf_src += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+			decoded_bytes += (TLV_TYPE_SIZE + TLV_LEN_SIZE);
+			temp_ei = find_ei(ei_array, tlv_type);
+			if (!temp_ei && (tlv_type < OPTIONAL_TLV_TYPE_START)) {
+				pr_err("%s: Inval element info\n", __func__);
+				return -EINVAL;
+			} else if (!temp_ei) {
+				UPDATE_DECODE_VARIABLES(buf_src,
+						decoded_bytes, tlv_len);
+				continue;
+			}
+		} else {
+			/*
+			 * No length information for elements in nested
+			 * structures. So use remaining decodable buffer space.
+			 */
+			tlv_len = in_buf_len - decoded_bytes;
+		}
+
+		buf_dst = out_c_struct + temp_ei->offset;
+		if (temp_ei->data_type == QMI_OPT_FLAG) {
+			memcpy(buf_dst, &opt_flag_value, sizeof(uint8_t));
+			temp_ei = temp_ei + 1;
+			buf_dst = out_c_struct + temp_ei->offset;
+		}
+
+		if (temp_ei->data_type == QMI_DATA_LEN) {
+			data_len_sz = temp_ei->elem_size == sizeof(uint8_t) ?
+					sizeof(uint8_t) : sizeof(uint16_t);
+			rc = qmi_decode_basic_elem(&data_len_value, buf_src,
+						   1, data_len_sz);
+			memcpy(buf_dst, &data_len_value, sizeof(uint32_t));
+			temp_ei = temp_ei + 1;
+			buf_dst = out_c_struct + temp_ei->offset;
+			tlv_len -= data_len_sz;
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+		}
+
+		if (temp_ei->is_array == NO_ARRAY) {
+			data_len_value = 1;
+		} else if (temp_ei->is_array == STATIC_ARRAY) {
+			data_len_value = temp_ei->elem_len;
+		} else if (data_len_value > temp_ei->elem_len) {
+			pr_err("%s: Data len %d > max spec %d\n",
+				__func__, data_len_value, temp_ei->elem_len);
+			return -ETOOSMALL;
+		}
+
+		switch (temp_ei->data_type) {
+		case QMI_UNSIGNED_1_BYTE:
+		case QMI_UNSIGNED_2_BYTE:
+		case QMI_UNSIGNED_4_BYTE:
+		case QMI_UNSIGNED_8_BYTE:
+		case QMI_SIGNED_2_BYTE_ENUM:
+		case QMI_SIGNED_4_BYTE_ENUM:
+			rc = qmi_decode_basic_elem(buf_dst, buf_src,
+				data_len_value, temp_ei->elem_size);
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+			break;
+
+		case QMI_STRUCT:
+			rc = qmi_decode_struct_elem(temp_ei, buf_dst, buf_src,
+				data_len_value, tlv_len, (dec_level + 1));
+			if (rc < 0)
+				return rc;
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+			break;
+
+		case QMI_STRING:
+			rc = qmi_decode_string_elem(temp_ei, buf_dst, buf_src,
+						     tlv_len, dec_level);
+			if (rc < 0)
+				return rc;
+			UPDATE_DECODE_VARIABLES(buf_src, decoded_bytes, rc);
+			break;
+
+		default:
+			pr_err("%s: Unrecognized data type\n", __func__);
+			return -EINVAL;
+		}
+		temp_ei = temp_ei + 1;
+	}
+	return decoded_bytes;
+}
+
+/**
+ * qmi_encode_message() - Encode C structure as QMI encoded message
+ * @type:	Type of QMI message
+ * @msg_id:	Message ID of the message
+ * @len:	Passed as max length of the message, updated to actual size
+ * @txn_id:	Transaction ID
+ * @ei:		QMI message descriptor
+ * @c_struct:	Reference to structure to encode
+ *
+ * Returns buffer with encoded message, or negative ERR_PTR() on error
+ */
+void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
+			 unsigned int txn_id, struct qmi_elem_info *ei,
+			 const void *c_struct)
+{
+	struct qmi_header *hdr;
+	void *msg;
+	int ret;
+
+	/* Check the possibility of a zero length QMI message */
+	if (!c_struct) {
+		ret = qmi_calc_min_msg_len(ei, 1);
+		if (ret) {
+			pr_err("%s: Calc. len %d != 0, but NULL c_struct\n",
+				__func__, ret);
+			return ERR_PTR(-EINVAL);
+		} else {
+			return NULL;
+		}
+	}
+
+	if (!ei)
+		return ERR_PTR(-EINVAL);
+
+	hdr = msg = kzalloc(sizeof(*hdr) + *len, GFP_KERNEL);
+	if (!msg)
+		return ERR_PTR(-ENOMEM);
+
+	ret = qmi_encode(ei, msg + sizeof(*hdr), c_struct, *len, 1);
+	if (ret < 0) {
+		kfree(msg);
+		return ERR_PTR(ret);
+	}
+
+	hdr->type = type;
+	hdr->txn_id = txn_id;
+	hdr->msg_id = msg_id;
+	hdr->msg_len = ret;
+
+	*len = sizeof(*hdr) + ret;
+
+	return msg;
+}
+EXPORT_SYMBOL(qmi_encode_message);
+
+/**
+ * qmi_decode_message() - Decode QMI encoded message to C structure
+ * @buf:	Buffer with encoded message
+ * @len:	Amount of data in @buf
+ * @ei:		QMI message descriptor
+ * @c_struct:	Reference to structure to decode into
+ *
+ * Returns the number of bytes of decoded information on success, negative
+ * errno on error.
+ */
+int qmi_decode_message(const void *buf, size_t len,
+		       struct qmi_elem_info *ei, void *c_struct)
+{
+	if (!ei)
+		return -EINVAL;
+
+	if (!c_struct || !buf || !len)
+		return -EINVAL;
+
+	return qmi_decode(ei, c_struct, buf + sizeof(struct qmi_header),
+			  len - sizeof(struct qmi_header), 1);
+}
+EXPORT_SYMBOL(qmi_decode_message);
+
+/* Common header in all QMI responses */
+struct qmi_elem_info qmi_response_type_v01_ei[] = {
+	{
+		.data_type	= QMI_SIGNED_2_BYTE_ENUM,
+		.elem_len	= 1,
+		.elem_size	= sizeof(uint16_t),
+		.is_array	= NO_ARRAY,
+		.tlv_type	= QMI_COMMON_TLV_TYPE,
+		.offset		= offsetof(struct qmi_response_type_v01,
+					   result),
+		.ei_array	= NULL,
+	},
+	{
+		.data_type      = QMI_SIGNED_2_BYTE_ENUM,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint16_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = QMI_COMMON_TLV_TYPE,
+		.offset         = offsetof(struct qmi_response_type_v01,
+					   error),
+		.ei_array       = NULL,
+	},
+	{
+		.data_type	= QMI_EOTI,
+		.elem_len	= 0,
+		.elem_size	= 0,
+		.is_array	= NO_ARRAY,
+		.tlv_type	= QMI_COMMON_TLV_TYPE,
+		.offset		= 0,
+		.ei_array	= NULL,
+	},
+};
+EXPORT_SYMBOL(qmi_response_type_v01_ei);
+
+MODULE_DESCRIPTION("QMI encoder/decoder helper");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
new file mode 100644
index 000000000000..c012a1e9e24b
--- /dev/null
+++ b/include/linux/soc/qcom/qmi.h
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2012-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2017, Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef __QMI_HELPERS_H__
+#define __QMI_HELPERS_H__
+
+#include <linux/qrtr.h>
+#include <linux/types.h>
+
+/**
+ * qmi_header - wireformat header of QMI messages
+ * @type:	type of message
+ * @txn_id:	transaction id
+ * @msg_id:	message id
+ * @msg_len:	length of message payload following header
+ */
+struct qmi_header {
+	uint8_t type;
+	uint16_t txn_id;
+	uint16_t msg_id;
+	uint16_t msg_len;
+} __packed;
+
+#define QMI_REQUEST	0
+#define QMI_RESPONSE	2
+#define QMI_INDICATION	4
+
+#define QMI_COMMON_TLV_TYPE 0
+
+enum qmi_elem_type {
+	QMI_EOTI,
+	QMI_OPT_FLAG,
+	QMI_DATA_LEN,
+	QMI_UNSIGNED_1_BYTE,
+	QMI_UNSIGNED_2_BYTE,
+	QMI_UNSIGNED_4_BYTE,
+	QMI_UNSIGNED_8_BYTE,
+	QMI_SIGNED_2_BYTE_ENUM,
+	QMI_SIGNED_4_BYTE_ENUM,
+	QMI_STRUCT,
+	QMI_STRING,
+};
+
+enum qmi_array_type {
+	NO_ARRAY,
+	STATIC_ARRAY,
+	VAR_LEN_ARRAY,
+};
+
+/**
+ * struct qmi_elem_info - describes how to encode a single QMI element
+ * @data_type:	Data type of this element.
+ * @elem_len:	Array length of this element, if an array.
+ * @elem_size:	Size of a single instance of this data type.
+ * @is_array:	Array type of this element.
+ * @tlv_type:	QMI message specific type to identify which element
+ *		is present in an incoming message.
+ * @offset:	Specifies the offset of the first instance of this
+ *		element in the data structure.
+ * @ei_array:	Null-terminated array of @qmi_elem_info to describe nested
+ *		structures.
+ */
+struct qmi_elem_info {
+	enum qmi_elem_type data_type;
+	uint32_t elem_len;
+	uint32_t elem_size;
+	enum qmi_array_type is_array;
+	uint8_t tlv_type;
+	uint32_t offset;
+	struct qmi_elem_info *ei_array;
+};
+
+#define QMI_RESULT_SUCCESS_V01			0
+#define QMI_RESULT_FAILURE_V01			1
+
+#define QMI_ERR_NONE_V01			0
+#define QMI_ERR_MALFORMED_MSG_V01		1
+#define QMI_ERR_NO_MEMORY_V01			2
+#define QMI_ERR_INTERNAL_V01			3
+#define QMI_ERR_CLIENT_IDS_EXHAUSTED_V01	5
+#define QMI_ERR_INVALID_ID_V01			41
+#define QMI_ERR_ENCODING_V01			58
+#define QMI_ERR_INCOMPATIBLE_STATE_V01		90
+#define QMI_ERR_NOT_SUPPORTED_V01		94
+
+/**
+ * qmi_response_type_v01 - common response header (decoded)
+ * @result:	result of the transaction
+ * @error:	error value, when @result is QMI_RESULT_FAILURE_V01
+ */
+struct qmi_response_type_v01 {
+	u32 result;
+	u32 error;
+};
+
+extern struct qmi_elem_info qmi_response_type_v01_ei[];
+
+void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
+			 unsigned int txn_id, struct qmi_elem_info *ei,
+			 const void *c_struct);
+
+int qmi_decode_message(const void *buf, size_t len,
+		       struct qmi_elem_info *ei, void *c_struct);
+
+
+#endif
-- 
2.12.0

^ permalink raw reply related

* [PATCH 5/6] soc: qcom: Introduce QMI helpers
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

Drivers that needs to communicate with a remote QMI service all has to
perform the operations of discovering the service, encoding and decoding
the messages and operate the socket. This introduces an abstraction for
these common operations, reducing most of the duplication in such cases.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/soc/qcom/Makefile        |   1 +
 drivers/soc/qcom/qmi_interface.c | 540 +++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/qmi.h     | 133 ++++++++++
 3 files changed, 674 insertions(+)
 create mode 100644 drivers/soc/qcom/qmi_interface.c

diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 27b60da7a062..812402ae9cfa 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
 obj-$(CONFIG_QCOM_PM)	+=	spm.o
 obj-$(CONFIG_QCOM_QMI_HELPERS)	+= qmi_helpers.o
 qmi_helpers-y	+= qmi_encdec.o
+qmi_helpers-y	+= qmi_interface.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= smd-rpm.o
 obj-$(CONFIG_QCOM_SMEM) +=	smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
new file mode 100644
index 000000000000..41853a9becfd
--- /dev/null
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -0,0 +1,540 @@
+/*
+ * Sample QRTR client driver
+ *
+ * Copyright (C) 2017 Linaro Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/qrtr.h>
+#include <linux/net.h>
+#include <linux/completion.h>
+#include <linux/idr.h>
+#include <linux/string.h>
+#include <net/sock.h>
+#include <linux/workqueue.h>
+#include <linux/soc/qcom/qmi.h>
+
+/**
+ * qrtr_client_new_server() - handler of NEW_SERVER control message
+ * @qrtr:	qrtr handle
+ * @node:	node of the new server
+ * @port:	port of the new server
+ *
+ * Calls the new_server callback to inform the client about a newly registered
+ * server matching the currently registered service lookup.
+ */
+static void qrtr_client_new_server(struct qrtr_handle *qrtr,
+				   unsigned int node, unsigned int port)
+{
+	struct qrtr_handle_ops *ops = &qrtr->ops;
+	struct qrtr_service *service;
+	int ret;
+
+	if (!ops->new_server)
+		return;
+
+	/* Ignore EOF marker */
+	if (!node && !port)
+		return;
+
+	service = kzalloc(sizeof(*service), GFP_KERNEL);
+	if (!service)
+		return;
+
+	service->node = node;
+	service->port = port;
+
+	ret = ops->new_server(qrtr, service);
+	if (ret < 0)
+		kfree(service);
+	else
+		list_add(&service->list_node, &qrtr->services);
+}
+
+/**
+ * qrtr_client_del_server() - handler of DEL_SERVER control message
+ * @qrtr:	qrtr handle
+ * @node:	node of the dying server, a value of -1 matches all nodes
+ * @port:	port of the dying server, a value of -1 matches all ports
+ *
+ * Calls the del_server callback for each previously seen server, allowing the
+ * client to react to the disappearing server.
+ */
+static void qrtr_client_del_server(struct qrtr_handle *qrtr,
+				   unsigned int node, unsigned int port)
+{
+	struct qrtr_handle_ops *ops = &qrtr->ops;
+	struct qrtr_service *service;
+	struct qrtr_service *tmp;
+
+	list_for_each_entry_safe(service, tmp, &qrtr->services, list_node) {
+		if (node != -1 && service->node != node)
+			continue;
+		if (port != -1 && service->port != port)
+			continue;
+
+		if (ops->del_server)
+			ops->del_server(qrtr, service);
+
+		list_del(&service->list_node);
+		kfree(service);
+	}
+}
+
+static void qrtr_client_ctrl_pkt(struct qrtr_handle *qrtr,
+				 const void *buf, size_t len)
+{
+	const struct qrtr_ctrl_pkt *pkt = buf;
+
+	if (len < sizeof(struct qrtr_ctrl_pkt)) {
+		pr_debug("ignoring short control packet\n");
+		return;
+	}
+
+	switch (le32_to_cpu(pkt->cmd)) {
+	case QRTR_TYPE_NEW_SERVER:
+		qrtr_client_new_server(qrtr,
+				       le32_to_cpu(pkt->server.node),
+				       le32_to_cpu(pkt->server.port));
+		break;
+	case QRTR_TYPE_DEL_SERVER:
+		qrtr_client_del_server(qrtr,
+				       le32_to_cpu(pkt->server.node),
+				       le32_to_cpu(pkt->server.port));
+		break;
+	}
+}
+
+static void qrtr_client_data_ready_work(struct work_struct *work)
+{
+	struct qrtr_handle *qrtr = container_of(work, struct qrtr_handle, work);
+	struct qrtr_handle_ops *ops = &qrtr->ops;
+	struct sockaddr_qrtr sq;
+	struct msghdr msg = { .msg_name = &sq, .msg_namelen = sizeof(sq) };
+	struct kvec iv;
+	ssize_t msglen;
+
+	for (;;) {
+		iv.iov_base = qrtr->recv_buf;
+		iv.iov_len = qrtr->recv_buf_size;
+
+		msglen = kernel_recvmsg(qrtr->sock, &msg, &iv, 1, iv.iov_len,
+					MSG_DONTWAIT);
+		if (msglen == -EAGAIN)
+			break;
+
+		if (msglen == -ENETRESET) {
+			if (ops->net_reset)
+				ops->net_reset(qrtr);
+			break;
+		}
+
+		if (msglen < 0) {
+			pr_err("qrtr_handle recvmsg failed: %zd\n", msglen);
+			break;
+		}
+
+		if (sq.sq_node == qrtr->sq.sq_node &&
+		    sq.sq_port == QRTR_PORT_CTRL) {
+			qrtr_client_ctrl_pkt(qrtr, qrtr->recv_buf, msglen);
+		} else {
+			if (!ops->msg_handler)
+				continue;
+
+			ops->msg_handler(qrtr, &sq, qrtr->recv_buf, msglen);
+		}
+	}
+}
+
+static void qrtr_client_schedule_worker(struct sock *sk)
+{
+	struct qrtr_handle *qrtr = sk->sk_user_data;
+
+	/*
+	 * This will be NULL if we receive data while being in
+	 * qrtr_client_release()
+	 */
+	if (!qrtr)
+		return;
+
+	queue_work(qrtr->wq, &qrtr->work);
+}
+
+/**
+ * qrtr_client_init() - initialize a qrtr_handle
+ * @qrtr:		reference to qrtr handle
+ * @recv_buf_size:	maximum size of received messages
+ * @ops:		qrtr_handle_ops struct with callbacks
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * This function initializes @qrtr to allow sending, receiving and handling
+ * QRTR control messages and data packets.
+ */
+int qrtr_client_init(struct qrtr_handle *qrtr, size_t recv_buf_size,
+		     struct qrtr_handle_ops *ops)
+{
+	struct sockaddr_qrtr sq;
+	struct socket *sock;
+	int ret;
+	int sl = sizeof(sq);
+
+	if (recv_buf_size < sizeof(struct qrtr_ctrl_pkt))
+		recv_buf_size = sizeof(struct qrtr_ctrl_pkt);
+
+	INIT_LIST_HEAD(&qrtr->services);
+	INIT_WORK(&qrtr->work, qrtr_client_data_ready_work);
+
+	qrtr->wq = alloc_workqueue("qrtr_handle", WQ_UNBOUND, 1);
+	if (!qrtr->wq)
+		return -ENOMEM;
+
+	qrtr->ops = *ops;
+	qrtr->recv_buf_size = recv_buf_size;
+	qrtr->recv_buf = kzalloc(recv_buf_size, GFP_KERNEL);
+	if (!qrtr->recv_buf)
+		return -ENOMEM;
+
+	ret = sock_create_kern(&init_net, AF_QIPCRTR, SOCK_DGRAM,
+			       PF_QIPCRTR, &sock);
+	if (ret < 0)
+		goto err_free_recv_buf;
+
+	ret = kernel_getsockname(sock, (struct sockaddr *)&sq, &sl);
+	if (ret < 0)
+		goto err_release_sock;
+
+	qrtr->sock = sock;
+	qrtr->sq = sq;
+
+	sock->sk->sk_user_data = qrtr;
+	sock->sk->sk_data_ready = qrtr_client_schedule_worker;
+	sock->sk->sk_error_report = qrtr_client_schedule_worker;
+
+	return 0;
+err_release_sock:
+	sock_release(sock);
+
+err_free_recv_buf:
+	kfree(qrtr->recv_buf);
+
+	return ret;
+}
+EXPORT_SYMBOL(qrtr_client_init);
+
+/**
+ * qrtr_client_release() - tear down a qrtr_handle
+ * @qrtr:	qrtr handle
+ *
+ * This will tear down the qrtr handle, stop handling of any incoming messages
+ * and release the underlying socket.
+ */
+void qrtr_client_release(struct qrtr_handle *qrtr)
+{
+	struct socket *sock = qrtr->sock;
+
+	sock->sk->sk_user_data = NULL;
+	cancel_work_sync(&qrtr->work);
+
+	kfree(qrtr->recv_buf);
+
+	qrtr_client_del_server(qrtr, -1, -1);
+
+	sock_release(sock);
+	qrtr->sock = NULL;
+
+	destroy_workqueue(qrtr->wq);
+}
+EXPORT_SYMBOL(qrtr_client_release);
+
+/**
+ * qrtr_client_new_lookup() - register a new lookup with the name service
+ * @qrtr:	qrtr handle
+ * @service:	service id of the request
+ * @instance:	instance id of the request
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * Registering a lookup query with the name server will cause the name server
+ * to send NEW_SERVER and DEL_SERVER control messages to this socket as
+ * matching services are registered.
+ */
+int qrtr_client_new_lookup(struct qrtr_handle *qrtr,
+			   unsigned int service, unsigned int instance)
+{
+	struct qrtr_ctrl_pkt pkt;
+	struct sockaddr_qrtr sq;
+	struct msghdr msg = {0};
+	struct kvec iv = { &pkt, sizeof(pkt) };
+	int ret;
+
+	memset(&pkt, 0, sizeof(pkt));
+	pkt.cmd = cpu_to_le32(QRTR_TYPE_NEW_LOOKUP);
+	pkt.server.service = cpu_to_le32(service);
+	pkt.server.instance = cpu_to_le32(instance);
+
+	sq.sq_family = qrtr->sq.sq_family;
+	sq.sq_node = qrtr->sq.sq_node;
+	sq.sq_port = QRTR_PORT_CTRL;
+
+	msg.msg_name = &sq;
+	msg.msg_namelen = sizeof(sq);
+
+	ret = kernel_sendmsg(qrtr->sock, &msg, &iv, 1, sizeof(pkt));
+	if (ret < 0)
+		pr_err("failed to send lookup registration: %d\n", ret);
+
+	return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL(qrtr_client_new_lookup);
+
+/**
+ * qmi_txn_init() - allocate transaction id within the given QMI handle
+ * @qmi:	QMI handle
+ * @txn:	transaction context
+ * @ei:		description of how to decode a matching response (optional)
+ * @c_struct:	pointer to the object to decode the response into (optional)
+ *
+ * Returns transaction id on success, negative errno on failure.
+ *
+ * This allocates a transaction id within the QMI handle. If @ei and @c_struct
+ * are specified any responses to this transaction will be decoded as described
+ * by @ei into @c_struct.
+ */
+int qmi_txn_init(struct qmi_handle *qmi, struct qmi_txn *txn,
+		 struct qmi_elem_info *ei, void *c_struct)
+{
+	int ret;
+
+	memset(txn, 0, sizeof(*txn));
+
+	init_completion(&txn->completion);
+	txn->qmi = qmi;
+	txn->ei = ei;
+	txn->dest = c_struct;
+
+	mutex_lock(&qmi->txn_lock);
+	ret = idr_alloc_cyclic(&qmi->txns, txn, 0, INT_MAX, GFP_KERNEL);
+	if (ret < 0)
+		pr_err("failed to allocate transaction id\n");
+
+	txn->id = ret;
+	mutex_unlock(&qmi->txn_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(qmi_txn_init);
+
+/**
+ * qmi_txn_wait() - wait for a response on a transaction
+ * @txn:	transaction handle
+ * @timeout:	timeout, in jiffies
+ *
+ * Returns the transaction response on success, negative errno on failure.
+ *
+ * If the transaction is decoded by the means of @ei and @c_struct the return
+ * value will be the returned value of qmi_decode_message(), otherwise it's up
+ * to the specified message handler to fill out the result.
+ */
+int qmi_txn_wait(struct qmi_txn *txn, unsigned long timeout)
+{
+	struct qmi_handle *qmi = txn->qmi;
+	int ret;
+
+	ret = wait_for_completion_interruptible_timeout(&txn->completion,
+							timeout);
+
+	mutex_lock(&qmi->txn_lock);
+	idr_remove(&qmi->txns, txn->id);
+	mutex_unlock(&qmi->txn_lock);
+
+	if (ret < 0)
+		return ret;
+	else if (ret == 0)
+		return -ETIMEDOUT;
+	else
+		return txn->result;
+}
+EXPORT_SYMBOL(qmi_txn_wait);
+
+/**
+ * qmi_txn_cancel() - cancel an ongoing transaction
+ * @txn:	transaction id
+ */
+void qmi_txn_cancel(struct qmi_txn *txn)
+{
+	struct qmi_handle *qmi = txn->qmi;
+
+	mutex_lock(&qmi->txn_lock);
+	idr_remove(&qmi->txns, txn->id);
+	mutex_unlock(&qmi->txn_lock);
+}
+EXPORT_SYMBOL(qmi_txn_cancel);
+
+static void qmi_client_handle_data(struct qrtr_handle *qrtr,
+				   struct sockaddr_qrtr *sq,
+				   const void *buf, size_t len)
+{
+	const struct qmi_header *hdr = buf;
+	struct qmi_handle *qmi = container_of(qrtr, struct qmi_handle, qrtr);
+	struct qmi_msg_handler *handler;
+	struct qmi_txn *txn = NULL;
+	void *dest;
+	int ret;
+
+	if (len < sizeof(*hdr)) {
+		pr_err("ignoring short QMI packet\n");
+		return;
+	}
+
+	mutex_lock(&qmi->txn_lock);
+
+	/* If this is a response, find the matching transaction handle */
+	if (hdr->type == QMI_RESPONSE)
+		txn = idr_find(&qmi->txns, hdr->txn_id);
+
+	if (txn && txn->dest && txn->ei) {
+		ret = qmi_decode_message(buf, len, txn->ei, txn->dest);
+		if (ret < 0)
+			pr_err("failed to decode incoming message\n");
+
+		txn->result = ret;
+		complete(&txn->completion);
+	} else if (qmi->handlers) {
+		for (handler = qmi->handlers; handler->fn; handler++) {
+			if (handler->type == hdr->type &&
+			    handler->msg_id == hdr->msg_id)
+				break;
+		}
+
+		if (!handler->fn)
+			goto out;
+
+		dest = kzalloc(handler->decoded_size, GFP_KERNEL);
+		if (!dest)
+			goto out;
+
+		ret = qmi_decode_message(buf, len, handler->ei, dest);
+		if (ret < 0)
+			pr_err("failed to decode incoming message\n");
+		else
+			handler->fn(qmi, sq, txn, dest);
+
+		kfree(dest);
+	}
+
+out:
+	mutex_unlock(&qmi->txn_lock);
+}
+
+/**
+ * qmi_client_init() - initialize a QMI client handle
+ * @qmi:	QMI handle to initialize
+ * @max_msg_len: maximum size of incoming message
+ * @handlers:	NULL-terminated list of QMI message handlers
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * This initializes the QMI client handle to allow sending and receiving QMI
+ * messages. As messages are received the appropriate handler will be invoked.
+ */
+int qmi_client_init(struct qmi_handle *qmi, size_t max_msg_len,
+		    struct qmi_msg_handler *handlers)
+{
+	struct qrtr_handle_ops ops = { 0 };
+	int ret;
+
+	mutex_init(&qmi->txn_lock);
+	idr_init(&qmi->txns);
+
+	ops.msg_handler = qmi_client_handle_data;
+
+	ret = qrtr_client_init(&qmi->qrtr, max_msg_len, &ops);
+	if (ret < 0)
+		return ret;
+
+	qmi->handlers = handlers;
+
+	return 0;
+}
+EXPORT_SYMBOL(qmi_client_init);
+
+/**
+ * qrtr_client_release() - release the QMI client handle
+ * @qmi:	QMI client handle
+ *
+ * This closes the underlying socket and stops any handling of QMI messages.
+ */
+void qmi_client_release(struct qmi_handle *qmi)
+{
+	qrtr_client_release(&qmi->qrtr);
+
+	idr_destroy(&qmi->txns);
+
+}
+EXPORT_SYMBOL(qmi_client_release);
+
+/**
+ * qmi_send_message() - send a QMI message
+ * @qmi:	QMI client handle
+ * @sq:		destination sockaddr
+ * @txn:	transaction object to use for the message
+ * @type:	type of message to send
+ * @msg_id:	message id
+ * @len:	max length of the QMI message
+ * @ei:		QMI message description
+ * @c_struct:	object to be encoded
+ *
+ * Returns 0 on success, negative errno on failure.
+ *
+ * This function encodes @c_struct using @ei into a message of type @type,
+ * with @msg_id and @txn into a buffer of maximum size @len, and sends this to
+ * @sq.
+ */
+ssize_t qmi_send_message(struct qmi_handle *qmi,
+			 struct sockaddr_qrtr *sq, struct qmi_txn *txn,
+			 int type, int msg_id, size_t len,
+			 struct qmi_elem_info *ei, const void *c_struct)
+{
+	struct qrtr_handle *qrtr = &qmi->qrtr;
+	struct msghdr msghdr = {0};
+	struct kvec iv;
+	void *msg;
+	int ret;
+
+	msg = qmi_encode_message(type,
+				 msg_id, &len,
+				 txn->id, ei,
+				 c_struct);
+	if (IS_ERR(msg))
+		return PTR_ERR(msg);
+
+	iv.iov_base = msg;
+	iv.iov_len = len;
+
+	if (sq) {
+		msghdr.msg_name = sq;
+		msghdr.msg_namelen = sizeof(*sq);
+	}
+
+	ret = kernel_sendmsg(qrtr->sock, &msghdr, &iv, 1, len);
+	if (ret < 0)
+		pr_err("failed to send QMI message\n");
+
+	kfree(msg);
+
+	return ret < 0 ? ret : 0;
+}
+EXPORT_SYMBOL(qmi_send_message);
diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
index c012a1e9e24b..add7570fa14b 100644
--- a/include/linux/soc/qcom/qmi.h
+++ b/include/linux/soc/qcom/qmi.h
@@ -105,6 +105,135 @@ struct qmi_response_type_v01 {
 
 extern struct qmi_elem_info qmi_response_type_v01_ei[];
 
+/**
+ * struct qrtr_service - context to track lookup-results
+ * @node:	node of the service
+ * @port:	port of the service
+ * @cookie:	handle for client's use
+ * @list_node:	list_head for house keeping
+ */
+struct qrtr_service {
+	unsigned int node;
+	unsigned int port;
+
+	void *cookie;
+	struct list_head list_node;
+};
+
+struct qrtr_handle;
+
+/**
+ * struct qrtr_handle_ops - callbacks from qrtr_handle
+ * @new_server:		invoked as a new_server message arrives
+ * @del_server:		invoked as a del_server message arrives
+ * @net_reset:		invoked as the name server is restarted
+ * @msg_handler:	invoked as a non-control message arrives
+ */
+struct qrtr_handle_ops {
+	int (*new_server)(struct qrtr_handle *, struct qrtr_service *);
+	void (*del_server)(struct qrtr_handle *, struct qrtr_service *);
+	void (*net_reset)(struct qrtr_handle *);
+	void (*msg_handler)(struct qrtr_handle *, struct sockaddr_qrtr *,
+			    const void *, size_t);
+};
+
+/**
+ * struct qrtr_handle - qrtr client context
+ * @sock:	socket handle
+ * @sq:		sockaddr of @sock
+ * @work:	work for handling incoming messages
+ * @wq:		workqueue to post @work on
+ * @recv_buf:	scratch buffer for handling incoming messages
+ * @recv_buf_size:	size of @recv_buf
+ * @services:	list of services advertised to the client
+ * @ops:	reference to callbacks
+ */
+struct qrtr_handle {
+	struct socket *sock;
+	struct sockaddr_qrtr sq;
+
+	struct work_struct work;
+	struct workqueue_struct *wq;
+
+	void *recv_buf;
+	size_t recv_buf_size;
+
+	struct list_head services;
+
+	struct qrtr_handle_ops ops;
+};
+
+/**
+ * struct qmi_txn - transaction context
+ * @qmi:	QMI handle this transaction is associated with
+ * @id:		transaction id
+ * @completion:	completion object as the transaction receives a response
+ * @result:	result code for the completed transaction
+ * @ei:		description of the QMI encoded response (optional)
+ * @dest:	destination buffer to decode message into (optional)
+ */
+struct qmi_txn {
+	struct qmi_handle *qmi;
+
+	int id;
+
+	struct completion completion;
+	int result;
+
+	struct qmi_elem_info *ei;
+	void *dest;
+};
+
+/**
+ * struct qmi_msg_handler - description of QMI message handler
+ * @type:	type of message
+ * @msg_id:	message id
+ * @ei:		description of the QMI encoded message
+ * @decoded_size:	size of the decoded object
+ * @fn:		function to invoke as the message is decoded
+ */
+struct qmi_msg_handler {
+	unsigned int type;
+	unsigned int msg_id;
+
+	struct qmi_elem_info *ei;
+
+	size_t decoded_size;
+	void (*fn)(struct qmi_handle *qmi, struct sockaddr_qrtr *sq,
+		   struct qmi_txn *txn, const void *decoded);
+};
+
+/**
+ * struct qmi_handle - QMI client handle
+ * @qrtr:	qrtr handle backing the QMI client
+ * @txns:	outstanding transactions
+ * @txn_lock:	lock for modifications of @txns
+ * @handlers:	list of handlers for incoming messages
+ */
+struct qmi_handle {
+	struct qrtr_handle qrtr;
+
+	struct idr txns;
+	struct mutex txn_lock;
+
+	struct qmi_msg_handler *handlers;
+};
+
+int qrtr_client_init(struct qrtr_handle *qrtr, size_t recv_buf_size,
+		     struct qrtr_handle_ops *ops);
+void qrtr_client_release(struct qrtr_handle *qrtr);
+int qrtr_client_new_lookup(struct qrtr_handle *qrtr,
+			   unsigned int service, unsigned int instance);
+
+int qmi_client_init(struct qmi_handle *qmi, size_t max_msg_len,
+		    struct qmi_msg_handler *handlers);
+void qmi_client_release(struct qmi_handle *qmi);
+
+ssize_t qmi_send_message(struct qmi_handle *qmi,
+			 struct sockaddr_qrtr *sq, struct qmi_txn *txn,
+			 int type, int msg_id, size_t len,
+			 struct qmi_elem_info *ei, const void *c_struct);
+
 void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
 			 unsigned int txn_id, struct qmi_elem_info *ei,
 			 const void *c_struct);
@@ -112,5 +241,9 @@ void *qmi_encode_message(int type, unsigned int msg_id, size_t *len,
 int qmi_decode_message(const void *buf, size_t len,
 		       struct qmi_elem_info *ei, void *c_struct);
 
+int qmi_txn_init(struct qmi_handle *qmi, struct qmi_txn *txn,
+		 struct qmi_elem_info *ei, void *c_struct);
+int qmi_txn_wait(struct qmi_txn *txn, unsigned long timeout);
+void qmi_txn_cancel(struct qmi_txn *txn);
 
 #endif
-- 
2.12.0

^ permalink raw reply related

* [PATCH 6/6] samples: Introduce Qualcomm QRTR sample client
From: Bjorn Andersson @ 2017-08-04 14:59 UTC (permalink / raw)
  To: David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

Introduce a sample driver that register for server notifications and
spawn clients for each available test service (service 15). The spawned
clients implements the interface for encoding "ping" and "data" requests
and decode the responses from the remote.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 samples/Kconfig                   |   8 +
 samples/Makefile                  |   2 +-
 samples/qrtr/Makefile             |   1 +
 samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 613 insertions(+), 1 deletion(-)
 create mode 100644 samples/qrtr/Makefile
 create mode 100644 samples/qrtr/qrtr_sample_client.c

diff --git a/samples/Kconfig b/samples/Kconfig
index 9cb63188d3ef..18796928ab58 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -62,6 +62,14 @@ config SAMPLE_KDB
 	  Build an example of how to dynamically add the hello
 	  command to the kdb shell.
 
+config SAMPLE_QRTR_CLIENT
+	tristate "Build qrtr client sample -- loadable modules only"
+	depends on m
+	select QCOM_QMI_HELPERS
+	help
+	  Build an qrtr client sample driver, which demonstrates how to
+	  communicate with a remote QRTR service, using QMI encoded messages.
+
 config SAMPLE_RPMSG_CLIENT
 	tristate "Build rpmsg client sample -- loadable modules only"
 	depends on RPMSG && m
diff --git a/samples/Makefile b/samples/Makefile
index db54e766ddb1..4bf64276860c 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -3,4 +3,4 @@
 obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ trace_events/ livepatch/ \
 			   hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ \
 			   configfs/ connector/ v4l/ trace_printk/ blackfin/ \
-			   vfio-mdev/ statx/
+			   vfio-mdev/ statx/ qrtr/
diff --git a/samples/qrtr/Makefile b/samples/qrtr/Makefile
new file mode 100644
index 000000000000..3f2c2cfdf2e7
--- /dev/null
+++ b/samples/qrtr/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SAMPLE_QRTR_CLIENT) += qrtr_sample_client.o
diff --git a/samples/qrtr/qrtr_sample_client.c b/samples/qrtr/qrtr_sample_client.c
new file mode 100644
index 000000000000..ccb359de4340
--- /dev/null
+++ b/samples/qrtr/qrtr_sample_client.c
@@ -0,0 +1,603 @@
+/*
+ * Sample QRTR client driver
+ *
+ * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 Linaro Ltd.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/qrtr.h>
+#include <linux/net.h>
+#include <linux/completion.h>
+#include <linux/idr.h>
+#include <linux/string.h>
+#include <net/sock.h>
+#include <linux/soc/qcom/qmi.h>
+
+#define PING_REQ1_TLV_TYPE		0x1
+#define PING_RESP1_TLV_TYPE		0x2
+#define PING_OPT1_TLV_TYPE		0x10
+#define PING_OPT2_TLV_TYPE		0x11
+
+#define DATA_REQ1_TLV_TYPE		0x1
+#define DATA_RESP1_TLV_TYPE		0x2
+#define DATA_OPT1_TLV_TYPE		0x10
+#define DATA_OPT2_TLV_TYPE		0x11
+
+#define TEST_MED_DATA_SIZE_V01		8192
+#define TEST_MAX_NAME_SIZE_V01		255
+
+#define TEST_PING_REQ_MSG_ID_V01	0x20
+#define TEST_DATA_REQ_MSG_ID_V01	0x21
+
+#define TEST_PING_REQ_MAX_MSG_LEN_V01	266
+#define TEST_DATA_REQ_MAX_MSG_LEN_V01	8456
+
+struct test_name_type_v01 {
+	uint32_t name_len;
+	char name[TEST_MAX_NAME_SIZE_V01];
+};
+
+static struct qmi_elem_info test_name_type_v01_ei[] = {
+	{
+		.data_type      = QMI_DATA_LEN,
+		.elem_len	= 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array	= NO_ARRAY,
+		.tlv_type	= QMI_COMMON_TLV_TYPE,
+		.offset		= offsetof(struct test_name_type_v01,
+					   name_len),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = TEST_MAX_NAME_SIZE_V01,
+		.elem_size      = sizeof(char),
+		.is_array       = VAR_LEN_ARRAY,
+		.tlv_type       = QMI_COMMON_TLV_TYPE,
+		.offset         = offsetof(struct test_name_type_v01,
+					   name),
+	},
+	{}
+};
+
+struct test_ping_req_msg_v01 {
+	char ping[4];
+
+	uint8_t client_name_valid;
+	struct test_name_type_v01 client_name;
+};
+
+struct qmi_elem_info test_ping_req_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = 4,
+		.elem_size      = sizeof(char),
+		.is_array       = STATIC_ARRAY,
+		.tlv_type       = PING_REQ1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_req_msg_v01,
+					   ping),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_req_msg_v01,
+					   client_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_req_msg_v01,
+					   client_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+struct test_ping_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+
+	uint8_t pong_valid;
+	char pong[4];
+
+	uint8_t service_name_valid;
+	struct test_name_type_v01 service_name;
+};
+
+struct qmi_elem_info test_ping_resp_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct qmi_response_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_RESP1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   resp),
+		.ei_array       = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   pong_valid),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = 4,
+		.elem_size      = sizeof(char),
+		.is_array       = STATIC_ARRAY,
+		.tlv_type       = PING_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   pong),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   service_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = PING_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_ping_resp_msg_v01,
+					   service_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+struct test_data_req_msg_v01 {
+	uint32_t data_len;
+	uint8_t data[TEST_MED_DATA_SIZE_V01];
+
+	uint8_t client_name_valid;
+	struct test_name_type_v01 client_name;
+};
+
+struct qmi_elem_info test_data_req_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_DATA_LEN,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint32_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_REQ1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   data_len),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = TEST_MED_DATA_SIZE_V01,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = VAR_LEN_ARRAY,
+		.tlv_type       = DATA_REQ1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   data),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   client_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_req_msg_v01,
+					   client_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+struct test_data_resp_msg_v01 {
+	struct qmi_response_type_v01 resp;
+
+	uint8_t data_valid;
+	uint32_t data_len;
+	uint8_t data[TEST_MED_DATA_SIZE_V01];
+
+	uint8_t service_name_valid;
+	struct test_name_type_v01 service_name;
+};
+
+struct qmi_elem_info test_data_resp_msg_v01_ei[] = {
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct qmi_response_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_RESP1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   resp),
+		.ei_array       = qmi_response_type_v01_ei,
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   data_valid),
+	},
+	{
+		.data_type      = QMI_DATA_LEN,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint32_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   data_len),
+	},
+	{
+		.data_type      = QMI_UNSIGNED_1_BYTE,
+		.elem_len       = TEST_MED_DATA_SIZE_V01,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = VAR_LEN_ARRAY,
+		.tlv_type       = DATA_OPT1_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   data),
+	},
+	{
+		.data_type      = QMI_OPT_FLAG,
+		.elem_len       = 1,
+		.elem_size      = sizeof(uint8_t),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   service_name_valid),
+	},
+	{
+		.data_type      = QMI_STRUCT,
+		.elem_len       = 1,
+		.elem_size      = sizeof(struct test_name_type_v01),
+		.is_array       = NO_ARRAY,
+		.tlv_type       = DATA_OPT2_TLV_TYPE,
+		.offset         = offsetof(struct test_data_resp_msg_v01,
+					   service_name),
+		.ei_array       = test_name_type_v01_ei,
+	},
+	{}
+};
+
+/*
+ * ping_pong_store() - ping_pong attribute store handler
+ * @dev:	sample device context
+ * @attr:	the ping_pong attribute
+ * @buf:	write buffer
+ * @count:	length of @buf
+ *
+ * Returns @count, or negative errno on failure.
+ *
+ * This function allows user space to send out a ping_pong QMI encoded message
+ * to the associated remote test service and will return with the result of the
+ * transaction. It serves as an example of how to provide a custom response
+ * handler.
+ */
+static ssize_t ping_pong_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct qmi_handle *qmi = dev_get_drvdata(dev);
+	struct test_ping_req_msg_v01 req = {0};
+	struct qmi_txn txn;
+	int ret;
+
+	memcpy(req.ping, "ping", sizeof(req.ping));
+
+	ret = qmi_txn_init(qmi, &txn, NULL, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = qmi_send_message(qmi, NULL, &txn,
+			       QMI_REQUEST,
+			       TEST_PING_REQ_MSG_ID_V01,
+			       TEST_PING_REQ_MAX_MSG_LEN_V01,
+			       test_ping_req_msg_v01_ei, &req);
+	if (ret < 0) {
+		qmi_txn_cancel(&txn);
+		return ret;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0)
+		count = ret;
+
+	return count;
+}
+static DEVICE_ATTR_WO(ping_pong);
+
+static void ping_pong_cb(struct qmi_handle *qmi, struct sockaddr_qrtr *sq,
+			 struct qmi_txn *txn, const void *data)
+{
+	const struct test_ping_resp_msg_v01 *resp = data;
+
+	if (!txn) {
+		pr_err("spurious ping response\n");
+		return;
+	}
+
+	if (resp->resp.result == QMI_RESULT_FAILURE_V01)
+		txn->result = -ENXIO;
+	else if (!resp->pong_valid || memcmp(resp->pong, "pong", 4))
+		txn->result = -EINVAL;
+
+	complete(&txn->completion);
+}
+
+/*
+ * data_store() - data attribute store handler
+ * @dev:	sample device context
+ * @attr:	the data attribute
+ * @buf:	buffer with message to encode
+ * @count:	length of @buf
+ *
+ * Returns @count, or negative errno on failure.
+ *
+ * This function allows user space to send out a data QMI encoded message to
+ * the associated remote test service and will return with the result of the
+ * transaction. It serves as an example of how to have the QMI helpers decode a
+ * transaction response into a provided object automatically.
+ */
+static ssize_t data_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct qmi_handle *qmi = dev_get_drvdata(dev);
+	struct test_data_resp_msg_v01 *resp;
+	struct test_data_req_msg_v01 *req;
+	struct qmi_txn txn;
+	int ret;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req)
+		return -ENOMEM;
+
+	resp = kzalloc(sizeof(*resp), GFP_KERNEL);
+	if (!resp) {
+		kfree(req);
+		return -ENOMEM;
+	}
+
+	req->data_len = min_t(size_t, sizeof(req->data), count);
+	memcpy(req->data, buf, req->data_len);
+
+	ret = qmi_txn_init(qmi, &txn, test_data_resp_msg_v01_ei, resp);
+	if (ret < 0) {
+		count = ret;
+		goto out;
+	}
+
+	ret = qmi_send_message(qmi, NULL, &txn,
+			       QMI_REQUEST,
+			       TEST_DATA_REQ_MSG_ID_V01,
+			       TEST_DATA_REQ_MAX_MSG_LEN_V01,
+			       test_data_req_msg_v01_ei, req);
+	if (ret < 0) {
+		qmi_txn_cancel(&txn);
+		count = ret;
+		goto out;
+	}
+
+	ret = qmi_txn_wait(&txn, 5 * HZ);
+	if (ret < 0) {
+		count = ret;
+	} else if (!resp->data_valid ||
+		   resp->data_len != req->data_len ||
+		   memcmp(resp->data, req->data, req->data_len)) {
+		pr_err("response data doesn't match expectation\n");
+		count = -EINVAL;
+	}
+
+out:
+	kfree(resp);
+	kfree(req);
+
+	return count;
+}
+static DEVICE_ATTR_WO(data);
+
+static struct attribute *qrtr_dev_attrs[] = {
+	&dev_attr_ping_pong.attr,
+	&dev_attr_data.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(qrtr_dev);
+
+static struct qmi_msg_handler qrtr_sample_handlers[] = {
+	{
+		.type = QMI_RESPONSE,
+		.msg_id = TEST_PING_REQ_MSG_ID_V01,
+		.ei = test_ping_resp_msg_v01_ei,
+		.decoded_size = sizeof(struct test_ping_req_msg_v01),
+		.fn = ping_pong_cb
+	},
+	{}
+};
+
+static int qrtr_sample_probe(struct platform_device *pdev)
+{
+	struct qrtr_handle *qrtr;
+	struct qmi_handle *qmi;
+	struct sockaddr_qrtr *sq;
+	int ret;
+
+	qmi = devm_kzalloc(&pdev->dev, sizeof(*qmi), GFP_KERNEL);
+	if (!qmi)
+		return -ENOMEM;
+
+	qrtr = &qmi->qrtr;
+
+	ret = qmi_client_init(qmi, TEST_DATA_REQ_MAX_MSG_LEN_V01,
+			      qrtr_sample_handlers);
+	if (ret < 0)
+		return ret;
+
+	sq = dev_get_platdata(&pdev->dev);
+	ret = kernel_connect(qrtr->sock, (struct sockaddr *)sq,
+			     sizeof(*sq), 0);
+	if (ret < 0) {
+		pr_err("failed to connect to remote service port\n");
+		qmi_client_release(qmi);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, qmi);
+
+	return 0;
+}
+
+static int qrtr_sample_remove(struct platform_device *pdev)
+{
+	struct qmi_handle *qmi = platform_get_drvdata(pdev);
+
+	qmi_client_release(qmi);
+
+	return 0;
+}
+
+static struct platform_driver qrtr_sample_driver = {
+	.probe = qrtr_sample_probe,
+	.remove = qrtr_sample_remove,
+	.driver = {
+		.name = "qrtr_sample_client",
+	},
+};
+
+static int qrtr_sample_new_server(struct qrtr_handle *qrtr,
+				  struct qrtr_service *service)
+{
+	struct platform_device *pdev;
+	struct sockaddr_qrtr sq = { AF_QIPCRTR, service->node, service->port };
+	char name[32];
+	int ret;
+
+	snprintf(name, sizeof(name), "qrtr_sample_client@%d:%d",
+		 service->node, service->port);
+
+	pdev = platform_device_alloc(name, PLATFORM_DEVID_NONE);
+	if (!pdev)
+		return -ENOMEM;
+
+	ret = platform_device_add_data(pdev, &sq, sizeof(sq));
+	if (ret)
+		goto err_put_device;
+
+	pdev->dev.groups = qrtr_dev_groups;
+	pdev->driver_override = (char *)qrtr_sample_driver.driver.name;
+	ret = platform_device_add(pdev);
+	if (ret)
+		goto err_put_device;
+
+	service->cookie = pdev;
+
+	return 0;
+
+err_put_device:
+	platform_device_put(pdev);
+
+	return ret;
+}
+
+static void qrtr_sample_del_server(struct qrtr_handle *qrtr,
+				   struct qrtr_service *service)
+{
+	struct platform_device *pdev = service->cookie;
+
+	platform_device_unregister(pdev);
+}
+
+static struct qrtr_handle lookup_client;
+
+static struct qrtr_handle_ops lookup_ops;
+
+static void qrtr_sample_net_reset_work(struct work_struct *work)
+{
+	int ret;
+
+	qrtr_client_release(&lookup_client);
+
+	ret = qrtr_client_init(&lookup_client, 0, &lookup_ops);
+	if (ret < 0)
+		return;
+
+	qrtr_client_new_lookup(&lookup_client, 15, 0);
+}
+static DECLARE_WORK(net_reset_work, qrtr_sample_net_reset_work);
+
+static void qrtr_sample_net_reset(struct qrtr_handle *qrtr)
+{
+	schedule_work(&net_reset_work);
+}
+
+static struct qrtr_handle_ops lookup_ops = {
+	.new_server = qrtr_sample_new_server,
+	.del_server = qrtr_sample_del_server,
+	.net_reset = qrtr_sample_net_reset,
+};
+
+static int qrtr_sample_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&qrtr_sample_driver);
+	if (ret)
+		return ret;
+
+	ret = qrtr_client_init(&lookup_client, 0, &lookup_ops);
+	if (ret < 0)
+		goto err_unregister_driver;
+
+	qrtr_client_new_lookup(&lookup_client, 15, 0);
+
+	return 0;
+
+err_unregister_driver:
+	platform_driver_unregister(&qrtr_sample_driver);
+
+	return ret;
+}
+
+static void qrtr_sample_exit(void)
+{
+	qrtr_client_release(&lookup_client);
+
+	platform_driver_unregister(&qrtr_sample_driver);
+}
+
+module_init(qrtr_sample_init);
+module_exit(qrtr_sample_exit);
+
+MODULE_DESCRIPTION("Sample QRTR client driver");
+MODULE_LICENSE("GPL v2");
-- 
2.12.0

^ permalink raw reply related

* [net-next PATCH v2] net: comment fixes against BPF devmap helper calls
From: John Fastabend @ 2017-08-04 15:24 UTC (permalink / raw)
  To: davem, daniel, ast; +Cc: netdev

Update BPF comments to accurately reflect XDP usage.

Fixes: 97f91a7cf04ff ("bpf: add bpf_redirect_map helper routine")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 include/uapi/linux/bpf.h |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1106a8c..1d06be1 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -345,14 +345,20 @@ enum bpf_attach_type {
  * int bpf_redirect(ifindex, flags)
  *     redirect to another netdev
  *     @ifindex: ifindex of the net device
- *     @flags: bit 0 - if set, redirect to ingress instead of egress
- *             other bits - reserved
- *     Return: TC_ACT_REDIRECT
- * int bpf_redirect_map(key, map, flags)
+ *     @flags:
+ *	  cls_bpf:
+ *          bit 0 - if set, redirect to ingress instead of egress
+ *          other bits - reserved
+ *	  xdp_bpf:
+ *	    all bits - reserved
+ *     Return: cls_bpf: TC_ACT_REDIRECT on success or TC_ACT_SHOT on error
+ *	       xdp_bfp: XDP_REDIRECT on success or XDP_ABORT on error
+ * int bpf_redirect_map(map, key, flags)
  *     redirect to endpoint in map
+ *     @map: pointer to dev map
  *     @key: index in map to lookup
- *     @map: fd of map to do lookup in
  *     @flags: --
+ *     Return: XDP_REDIRECT on success or XDP_ABORT on error
  *
  * u32 bpf_get_route_realm(skb)
  *     retrieve a dst's tclassid

^ permalink raw reply related

* Re: [PATCH 1/2] net: mvneta: remove bogus use of
From: Gregory CLEMENT @ 2017-08-04 15:26 UTC (permalink / raw)
  To: Rob Herring
  Cc: Thomas Petazzoni, netdev, devicetree@vger.kernel.org,
	Frank Rowand, Marcin Wojtas
In-Reply-To: <CAL_JsqKTwju4=J=X1kvk=77fE1VddPh2N-wCi3A0mCAu+Qg1mA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,
 
 On jeu., juil. 20 2017, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> On Thu, Jul 20, 2017 at 10:06 AM, Gregory CLEMENT
> <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> Hi Rob,
>>
>>  On jeu., juil. 20 2017, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>
>> (Adding Marcin in CC who wrote this part of code)
>>
>>> Nothing sets ever sets data, so it is always NULL. Remove it as this is
>>> the only user of data ptr in the whole kernel, and it is going to be
>>> removed from struct device_node.
>>
>> Actually the use of device_node.data ptr is not bogus and it is set in
>> mvneta_bm_probe:
>> http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/marvell/mvneta_bm.c#L433
>
> Indeed. Looks like some complicated kconfig logic, so I'd not been
> able to trigger a build failure nor did 0-day (so far).
>
>> Your patch will break the BM support on this driver. So if you need to
>> remove this data ptr, then you have to offer an alternative for it.
>
> How about something like this (WS damaged) patch:

I finally took time to test your patch. There was some missing part
which prevented it to be build, like including linux/of_platform.h, or
providing tub function when CONFIG_MVNETA_BM was not enable.

Also the fact that you still call mvneta_bm_port_init() even if bm_priv
was NULL was not really nice. So I proposed the following patch, that I
tested on a clearfog with and without CONFIG_MVNETA_BM enabled.

>From 03c4028bc1f52d3d214e8506d9f0f66660d3985d Mon Sep 17 00:00:00 2001
From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Date: Fri, 4 Aug 2017 17:18:38 +0200
Subject: [PATCH] net: mvneta: remove data pointer usage from device_node
 structure

In order to be able to remove the data pointer from the device_node
structure. We have to modify the way the BM resources are shared between
the mvneta port.

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 drivers/net/ethernet/marvell/mvneta.c    | 18 ++++++++++++------
 drivers/net/ethernet/marvell/mvneta_bm.c | 13 +++++++++++++
 drivers/net/ethernet/marvell/mvneta_bm.h |  8 ++++++--
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 63b6147753fe..fd84447582f7 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4295,12 +4295,16 @@ static int mvneta_probe(struct platform_device *pdev)
 
        /* Obtain access to BM resources if enabled and already initialized */
        bm_node = of_parse_phandle(dn, "buffer-manager", 0);
-       if (bm_node && bm_node->data) {
-               pp->bm_priv = bm_node->data;
-               err = mvneta_bm_port_init(pdev, pp);
-               if (err < 0) {
-                       dev_info(&pdev->dev, "use SW buffer management\n");
-                       pp->bm_priv = NULL;
+       if (bm_node) {
+               pp->bm_priv = mvneta_bm_get(bm_node);
+               if (pp->bm_priv) {
+                       err = mvneta_bm_port_init(pdev, pp);
+                       if (err < 0) {
+                               dev_info(&pdev->dev,
+                                        "use SW buffer management\n");
+                               mvneta_bm_put(pp->bm_priv);
+                               pp->bm_priv = NULL;
+                       }
                }
        }
        of_node_put(bm_node);
@@ -4369,6 +4373,7 @@ static int mvneta_probe(struct platform_device *pdev)
                mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
                mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
                                       1 << pp->id);
+               mvneta_bm_put(pp->bm_priv);
        }
 err_free_stats:
        free_percpu(pp->stats);
@@ -4410,6 +4415,7 @@ static int mvneta_remove(struct platform_device *pdev)
                mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
                mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
                                       1 << pp->id);
+               mvneta_bm_put(pp->bm_priv);
        }
 
        return 0;
diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c b/drivers/net/ethernet/marvell/mvneta_bm.c
index 466939f8f0cf..01e3152e76c8 100644
--- a/drivers/net/ethernet/marvell/mvneta_bm.c
+++ b/drivers/net/ethernet/marvell/mvneta_bm.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/skbuff.h>
 #include <net/hwbm.h>
@@ -392,6 +393,18 @@ static void mvneta_bm_put_sram(struct mvneta_bm *priv)
                      MVNETA_BM_BPPI_SIZE);
 }
 
+struct mvneta_bm *mvneta_bm_get(struct device_node *node)
+{
+       struct platform_device *pdev = of_find_device_by_node(node);
+
+       return pdev ? platform_get_drvdata(pdev) : NULL;
+}
+
+void mvneta_bm_put(struct mvneta_bm *priv)
+{
+       platform_device_put(priv->pdev);
+}
+
 static int mvneta_bm_probe(struct platform_device *pdev)
 {
        struct device_node *dn = pdev->dev.of_node;
diff --git a/drivers/net/ethernet/marvell/mvneta_bm.h b/drivers/net/ethernet/marvell/mvneta_bm.h
index a32de432800c..daa9a91d2708 100644
--- a/drivers/net/ethernet/marvell/mvneta_bm.h
+++ b/drivers/net/ethernet/marvell/mvneta_bm.h
@@ -134,6 +134,9 @@ void *mvneta_frag_alloc(unsigned int frag_size);
 void mvneta_frag_free(unsigned int frag_size, void *data);
 
 #if IS_ENABLED(CONFIG_MVNETA_BM)
+struct mvneta_bm *mvneta_bm_get(struct device_node *node);
+void mvneta_bm_put(struct mvneta_bm *priv);
+
 void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
                            struct mvneta_bm_pool *bm_pool, u8 port_map);
 void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct mvneta_bm_pool *bm_pool,
@@ -176,7 +179,8 @@ static inline void mvneta_bm_pool_put_bp(struct mvneta_bm *priv,
                                         dma_addr_t buf_phys_addr) {}
 
 static inline u32 mvneta_bm_pool_get_bp(struct mvneta_bm *priv,
-                                       struct mvneta_bm_pool *bm_pool)
-{ return 0; }
+                               struct mvneta_bm_pool *bm_pool) { return 0; }
+struct mvneta_bm *mvneta_bm_get(struct device_node *node) {return NULL; }
+void mvneta_bm_put(struct mvneta_bm *priv) {}
 #endif /* CONFIG_MVNETA_BM */
 #endif
-- 
2.13.2



>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c
> b/drivers/net/ethernet/marvell/mvneta.c
> index 0aab74c2a209..3a3021cb93a7 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4296,11 +4296,12 @@ static int mvneta_probe(struct platform_device *pdev)
>
>   /* Obtain access to BM resources if enabled and already initialized */
>   bm_node = of_parse_phandle(dn, "buffer-manager", 0);
> - if (bm_node && bm_node->data) {
> - pp->bm_priv = bm_node->data;
> + if (bm_node) {
> + pp->bm_priv = mvneta_bm_get();
>   err = mvneta_bm_port_init(pdev, pp);
>   if (err < 0) {
>   dev_info(&pdev->dev, "use SW buffer management\n");
> + mvneta_bm_put(pp->bm_priv);
>   pp->bm_priv = NULL;
>   }
>   }
> @@ -4370,6 +4371,7 @@ static int mvneta_probe(struct platform_device *pdev)
>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
>         1 << pp->id);
> + mvneta_bm_put(pp->bm_priv);
>   }
>  err_free_stats:
>   free_percpu(pp->stats);
> @@ -4411,6 +4413,7 @@ static int mvneta_remove(struct platform_device *pdev)
>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
>         1 << pp->id);
> + mvneta_bm_put(pp->bm_priv);
>   }
>
>   return 0;
> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c
> b/drivers/net/ethernet/marvell/mvneta_bm.c
> index 466939f8f0cf..276af8aff3c7 100644
> --- a/drivers/net/ethernet/marvell/mvneta_bm.c
> +++ b/drivers/net/ethernet/marvell/mvneta_bm.c
> @@ -392,6 +392,17 @@ static void mvneta_bm_put_sram(struct mvneta_bm *priv)
>        MVNETA_BM_BPPI_SIZE);
>  }
>
> +struct mvneta_bm *mvneta_bm_get(struct device_node *node)
> +{
> + struct platform_device *pdev = of_find_device_by_node(node);
> + return pdev ? platform_get_drvdata(pdev) : NULL;
> +}
> +
> +void mvneta_bm_put(struct mvneta_bm *priv)
> +{
> + platform_device_put(priv->pdev);
> +}
> +
>  static int mvneta_bm_probe(struct platform_device *pdev)
>  {
>   struct device_node *dn = pdev->dev.of_node;
> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.h
> b/drivers/net/ethernet/marvell/mvneta_bm.h
> index a32de432800c..50b27feab666 100644
> --- a/drivers/net/ethernet/marvell/mvneta_bm.h
> +++ b/drivers/net/ethernet/marvell/mvneta_bm.h
> @@ -134,6 +134,9 @@ void *mvneta_frag_alloc(unsigned int frag_size);
>  void mvneta_frag_free(unsigned int frag_size, void *data);
>
>  #if IS_ENABLED(CONFIG_MVNETA_BM)
> +struct mvneta_bm *mvneta_bm_get(struct device_node *node);
> +void mvneta_bm_put(struct mvneta_bm *priv);
> +
>  void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
>      struct mvneta_bm_pool *bm_pool, u8 port_map);
>  void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct
> mvneta_bm_pool *bm_pool,
> @@ -160,6 +163,12 @@ static inline u32 mvneta_bm_pool_get_bp(struct
> mvneta_bm *priv,
>       (bm_pool->id << MVNETA_BM_POOL_ACCESS_OFFS));
>  }
>  #else
> +static inline struct mvneta_bm *mvneta_bm_get(struct device_node *node)
> +{
> + return NULL;
> +}
> +static inline void mvneta_bm_put(struct mvneta_bm *priv) {}
> +
>  void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
>      struct mvneta_bm_pool *bm_pool, u8 port_map) {}
>  void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct
> mvneta_bm_pool *bm_pool,

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 related

* Re: [net-next PATCH v2] net: comment fixes against BPF devmap helper calls
From: Daniel Borkmann @ 2017-08-04 15:30 UTC (permalink / raw)
  To: John Fastabend, davem, ast; +Cc: netdev
In-Reply-To: <20170804152404.28045.15462.stgit@john-Precision-Tower-5810>

On 08/04/2017 05:24 PM, John Fastabend wrote:
> Update BPF comments to accurately reflect XDP usage.
>
> Fixes: 97f91a7cf04ff ("bpf: add bpf_redirect_map helper routine")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Thanks!

^ permalink raw reply

* Re: [PATCH net-next v2 00/13] Change DSA's FDB API and perform switchdev cleanup
From: Vivien Didelot @ 2017-08-04 15:29 UTC (permalink / raw)
  To: Jiri Pirko, Arkadi Sharshevsky
  Cc: netdev, davem, ivecera, f.fainelli, andrew, Woojung.Huh, stephen,
	mlxsw
In-Reply-To: <20170804052301.GA1906@nanopsycho.orion>

Hi Arkadi, Jiri,

Jiri Pirko <jiri@resnulli.us> writes:

>>It seems impossible currently to move the self to be the default, and
>>this introduces regression which you don't approve, so it seems few
>>options left:
>>
>>a) Leave two ways to add fdb, through the bridge (by using the master
>>   flag) which is introduced in this patchset, and by using the self
>>   which is the legacy way. In this way no regression will be introduced,
>>   yet, it feels confusing a bit. The benefit is that we (DSA/mlxsw)
>>   will be synced.
>>b) Leave only the self (which means removing patch no 4,5).
>
> I belive that option a) is the correct way to go. Introduction of self
> inclusion was a mistake from the very beginning. I think that we should
> just move one and correct this mistake.
>
> Vivien, any arguments against a)?

I do agree with a). Arkadi, when moving switchdev implementations inside
of DSA core, can I ask you to move the ones considered as the legacy way
into legacy.c and ideally comment it? Configuration from userspace is
still very confusing and this will remind us to get rid of it one day.


Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH 0/6] In-kernel QMI handling
From: Dan Williams @ 2017-08-04 15:36 UTC (permalink / raw)
  To: Bjorn Andersson, David S. Miller, Andy Gross, David Brown
  Cc: linux-arm-msm, linux-soc, netdev, linux-kernel
In-Reply-To: <20170804145938.25427-1-bjorn.andersson@linaro.org>

On Fri, 2017-08-04 at 07:59 -0700, Bjorn Andersson wrote:
> This series starts by moving the common definitions of the QMUX
> protocol to the
> uapi header, as they are shared with clients - both in kernel and
> userspace.
> 
> This series then introduces in-kernel helper functions for aiding the
> handling
> of QMI encoded messages in the kernel. QMI encoding is a wire-format
> used in
> exchanging messages between the majority of QRTR clients and
> services.

This raises a few red-flags for me.  So far, we've kept almost
everything QMI related in userspace and handled all QMI control-channel 
messages from libraries like libqmi or uqmi via the cdc-wdm driver and
the "rmnet" interface via the qmi_wwan driver.  The kernel drivers just
serve as the transport.

Can you describe what kinds of in-kernel drivers need to actually parse
QMI messages as part of their operation?

Dan

> It then adds an abstractions to reduce the duplication of common code
> in
> drivers that needs to query the name server and send and receive
> encoded
> messages to a remote service.
> 
> Finally it introduces a sample implementation for showing QRTR and
> the QMI
> helpers in action. The sample device instantiates in response to
> finding the
> "test service" and implements the "test protocol".
> 
> Bjorn Andersson (6):
>   net: qrtr: Invoke sk_error_report() after setting sk_err
>   net: qrtr: Move constants to header file
>   net: qrtr: Add control packet definition to uapi
>   soc: qcom: Introduce QMI encoder/decoder
>   soc: qcom: Introduce QMI helpers
>   samples: Introduce Qualcomm QRTR sample client
> 
>  drivers/soc/qcom/Kconfig          |   8 +
>  drivers/soc/qcom/Makefile         |   3 +
>  drivers/soc/qcom/qmi_encdec.c     | 812
> ++++++++++++++++++++++++++++++++++++++
>  drivers/soc/qcom/qmi_interface.c  | 540 +++++++++++++++++++++++++
>  include/linux/soc/qcom/qmi.h      | 249 ++++++++++++
>  include/uapi/linux/qrtr.h         |  35 ++
>  net/qrtr/qrtr.c                   |  16 +-
>  samples/Kconfig                   |   8 +
>  samples/Makefile                  |   2 +-
>  samples/qrtr/Makefile             |   1 +
>  samples/qrtr/qrtr_sample_client.c | 603 ++++++++++++++++++++++++++++
>  11 files changed, 2261 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/soc/qcom/qmi_encdec.c
>  create mode 100644 drivers/soc/qcom/qmi_interface.c
>  create mode 100644 include/linux/soc/qcom/qmi.h
>  create mode 100644 samples/qrtr/Makefile
>  create mode 100644 samples/qrtr/qrtr_sample_client.c
> 

^ permalink raw reply

* Re: [PATCH 1/2] net: mvneta: remove bogus use of
From: Marcin Wojtas @ 2017-08-04 15:49 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Rob Herring, Thomas Petazzoni, netdev, devicetree@vger.kernel.org,
	Frank Rowand
In-Reply-To: <87tw1n73r9.fsf@free-electrons.com>

Hi Gregory,

>From my side: +1 to your modification.

Thanks,
Marcin

2017-08-04 17:26 GMT+02:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi Rob,
>
>  On jeu., juil. 20 2017, Rob Herring <robh@kernel.org> wrote:
>
>> On Thu, Jul 20, 2017 at 10:06 AM, Gregory CLEMENT
>> <gregory.clement@free-electrons.com> wrote:
>>> Hi Rob,
>>>
>>>  On jeu., juil. 20 2017, Rob Herring <robh@kernel.org> wrote:
>>>
>>> (Adding Marcin in CC who wrote this part of code)
>>>
>>>> Nothing sets ever sets data, so it is always NULL. Remove it as this is
>>>> the only user of data ptr in the whole kernel, and it is going to be
>>>> removed from struct device_node.
>>>
>>> Actually the use of device_node.data ptr is not bogus and it is set in
>>> mvneta_bm_probe:
>>> http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/marvell/mvneta_bm.c#L433
>>
>> Indeed. Looks like some complicated kconfig logic, so I'd not been
>> able to trigger a build failure nor did 0-day (so far).
>>
>>> Your patch will break the BM support on this driver. So if you need to
>>> remove this data ptr, then you have to offer an alternative for it.
>>
>> How about something like this (WS damaged) patch:
>
> I finally took time to test your patch. There was some missing part
> which prevented it to be build, like including linux/of_platform.h, or
> providing tub function when CONFIG_MVNETA_BM was not enable.
>
> Also the fact that you still call mvneta_bm_port_init() even if bm_priv
> was NULL was not really nice. So I proposed the following patch, that I
> tested on a clearfog with and without CONFIG_MVNETA_BM enabled.
>
> From 03c4028bc1f52d3d214e8506d9f0f66660d3985d Mon Sep 17 00:00:00 2001
> From: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Date: Fri, 4 Aug 2017 17:18:38 +0200
> Subject: [PATCH] net: mvneta: remove data pointer usage from device_node
>  structure
>
> In order to be able to remove the data pointer from the device_node
> structure. We have to modify the way the BM resources are shared between
> the mvneta port.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mvneta.c    | 18 ++++++++++++------
>  drivers/net/ethernet/marvell/mvneta_bm.c | 13 +++++++++++++
>  drivers/net/ethernet/marvell/mvneta_bm.h |  8 ++++++--
>  3 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 63b6147753fe..fd84447582f7 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4295,12 +4295,16 @@ static int mvneta_probe(struct platform_device *pdev)
>
>         /* Obtain access to BM resources if enabled and already initialized */
>         bm_node = of_parse_phandle(dn, "buffer-manager", 0);
> -       if (bm_node && bm_node->data) {
> -               pp->bm_priv = bm_node->data;
> -               err = mvneta_bm_port_init(pdev, pp);
> -               if (err < 0) {
> -                       dev_info(&pdev->dev, "use SW buffer management\n");
> -                       pp->bm_priv = NULL;
> +       if (bm_node) {
> +               pp->bm_priv = mvneta_bm_get(bm_node);
> +               if (pp->bm_priv) {
> +                       err = mvneta_bm_port_init(pdev, pp);
> +                       if (err < 0) {
> +                               dev_info(&pdev->dev,
> +                                        "use SW buffer management\n");
> +                               mvneta_bm_put(pp->bm_priv);
> +                               pp->bm_priv = NULL;
> +                       }
>                 }
>         }
>         of_node_put(bm_node);
> @@ -4369,6 +4373,7 @@ static int mvneta_probe(struct platform_device *pdev)
>                 mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
>                 mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
>                                        1 << pp->id);
> +               mvneta_bm_put(pp->bm_priv);
>         }
>  err_free_stats:
>         free_percpu(pp->stats);
> @@ -4410,6 +4415,7 @@ static int mvneta_remove(struct platform_device *pdev)
>                 mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
>                 mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
>                                        1 << pp->id);
> +               mvneta_bm_put(pp->bm_priv);
>         }
>
>         return 0;
> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c b/drivers/net/ethernet/marvell/mvneta_bm.c
> index 466939f8f0cf..01e3152e76c8 100644
> --- a/drivers/net/ethernet/marvell/mvneta_bm.c
> +++ b/drivers/net/ethernet/marvell/mvneta_bm.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/skbuff.h>
>  #include <net/hwbm.h>
> @@ -392,6 +393,18 @@ static void mvneta_bm_put_sram(struct mvneta_bm *priv)
>                       MVNETA_BM_BPPI_SIZE);
>  }
>
> +struct mvneta_bm *mvneta_bm_get(struct device_node *node)
> +{
> +       struct platform_device *pdev = of_find_device_by_node(node);
> +
> +       return pdev ? platform_get_drvdata(pdev) : NULL;
> +}
> +
> +void mvneta_bm_put(struct mvneta_bm *priv)
> +{
> +       platform_device_put(priv->pdev);
> +}
> +
>  static int mvneta_bm_probe(struct platform_device *pdev)
>  {
>         struct device_node *dn = pdev->dev.of_node;
> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.h b/drivers/net/ethernet/marvell/mvneta_bm.h
> index a32de432800c..daa9a91d2708 100644
> --- a/drivers/net/ethernet/marvell/mvneta_bm.h
> +++ b/drivers/net/ethernet/marvell/mvneta_bm.h
> @@ -134,6 +134,9 @@ void *mvneta_frag_alloc(unsigned int frag_size);
>  void mvneta_frag_free(unsigned int frag_size, void *data);
>
>  #if IS_ENABLED(CONFIG_MVNETA_BM)
> +struct mvneta_bm *mvneta_bm_get(struct device_node *node);
> +void mvneta_bm_put(struct mvneta_bm *priv);
> +
>  void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
>                             struct mvneta_bm_pool *bm_pool, u8 port_map);
>  void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct mvneta_bm_pool *bm_pool,
> @@ -176,7 +179,8 @@ static inline void mvneta_bm_pool_put_bp(struct mvneta_bm *priv,
>                                          dma_addr_t buf_phys_addr) {}
>
>  static inline u32 mvneta_bm_pool_get_bp(struct mvneta_bm *priv,
> -                                       struct mvneta_bm_pool *bm_pool)
> -{ return 0; }
> +                               struct mvneta_bm_pool *bm_pool) { return 0; }
> +struct mvneta_bm *mvneta_bm_get(struct device_node *node) {return NULL; }
> +void mvneta_bm_put(struct mvneta_bm *priv) {}
>  #endif /* CONFIG_MVNETA_BM */
>  #endif
> --
> 2.13.2
>
>
>
>>
>> diff --git a/drivers/net/ethernet/marvell/mvneta.c
>> b/drivers/net/ethernet/marvell/mvneta.c
>> index 0aab74c2a209..3a3021cb93a7 100644
>> --- a/drivers/net/ethernet/marvell/mvneta.c
>> +++ b/drivers/net/ethernet/marvell/mvneta.c
>> @@ -4296,11 +4296,12 @@ static int mvneta_probe(struct platform_device *pdev)
>>
>>   /* Obtain access to BM resources if enabled and already initialized */
>>   bm_node = of_parse_phandle(dn, "buffer-manager", 0);
>> - if (bm_node && bm_node->data) {
>> - pp->bm_priv = bm_node->data;
>> + if (bm_node) {
>> + pp->bm_priv = mvneta_bm_get();
>>   err = mvneta_bm_port_init(pdev, pp);
>>   if (err < 0) {
>>   dev_info(&pdev->dev, "use SW buffer management\n");
>> + mvneta_bm_put(pp->bm_priv);
>>   pp->bm_priv = NULL;
>>   }
>>   }
>> @@ -4370,6 +4371,7 @@ static int mvneta_probe(struct platform_device *pdev)
>>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
>>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
>>         1 << pp->id);
>> + mvneta_bm_put(pp->bm_priv);
>>   }
>>  err_free_stats:
>>   free_percpu(pp->stats);
>> @@ -4411,6 +4413,7 @@ static int mvneta_remove(struct platform_device *pdev)
>>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_long, 1 << pp->id);
>>   mvneta_bm_pool_destroy(pp->bm_priv, pp->pool_short,
>>         1 << pp->id);
>> + mvneta_bm_put(pp->bm_priv);
>>   }
>>
>>   return 0;
>> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.c
>> b/drivers/net/ethernet/marvell/mvneta_bm.c
>> index 466939f8f0cf..276af8aff3c7 100644
>> --- a/drivers/net/ethernet/marvell/mvneta_bm.c
>> +++ b/drivers/net/ethernet/marvell/mvneta_bm.c
>> @@ -392,6 +392,17 @@ static void mvneta_bm_put_sram(struct mvneta_bm *priv)
>>        MVNETA_BM_BPPI_SIZE);
>>  }
>>
>> +struct mvneta_bm *mvneta_bm_get(struct device_node *node)
>> +{
>> + struct platform_device *pdev = of_find_device_by_node(node);
>> + return pdev ? platform_get_drvdata(pdev) : NULL;
>> +}
>> +
>> +void mvneta_bm_put(struct mvneta_bm *priv)
>> +{
>> + platform_device_put(priv->pdev);
>> +}
>> +
>>  static int mvneta_bm_probe(struct platform_device *pdev)
>>  {
>>   struct device_node *dn = pdev->dev.of_node;
>> diff --git a/drivers/net/ethernet/marvell/mvneta_bm.h
>> b/drivers/net/ethernet/marvell/mvneta_bm.h
>> index a32de432800c..50b27feab666 100644
>> --- a/drivers/net/ethernet/marvell/mvneta_bm.h
>> +++ b/drivers/net/ethernet/marvell/mvneta_bm.h
>> @@ -134,6 +134,9 @@ void *mvneta_frag_alloc(unsigned int frag_size);
>>  void mvneta_frag_free(unsigned int frag_size, void *data);
>>
>>  #if IS_ENABLED(CONFIG_MVNETA_BM)
>> +struct mvneta_bm *mvneta_bm_get(struct device_node *node);
>> +void mvneta_bm_put(struct mvneta_bm *priv);
>> +
>>  void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
>>      struct mvneta_bm_pool *bm_pool, u8 port_map);
>>  void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct
>> mvneta_bm_pool *bm_pool,
>> @@ -160,6 +163,12 @@ static inline u32 mvneta_bm_pool_get_bp(struct
>> mvneta_bm *priv,
>>       (bm_pool->id << MVNETA_BM_POOL_ACCESS_OFFS));
>>  }
>>  #else
>> +static inline struct mvneta_bm *mvneta_bm_get(struct device_node *node)
>> +{
>> + return NULL;
>> +}
>> +static inline void mvneta_bm_put(struct mvneta_bm *priv) {}
>> +
>>  void mvneta_bm_pool_destroy(struct mvneta_bm *priv,
>>      struct mvneta_bm_pool *bm_pool, u8 port_map) {}
>>  void mvneta_bm_bufs_free(struct mvneta_bm *priv, struct
>> mvneta_bm_pool *bm_pool,
>
> --
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: User per-cpu 64-bit statistics
From: Florian Fainelli @ 2017-08-04 15:51 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, davem, Andrew Lunn, Vivien Didelot, David S. Miller,
	open list
In-Reply-To: <1501824968.25002.35.camel@edumazet-glaptop3.roam.corp.google.com>

On 08/03/2017 10:36 PM, Eric Dumazet wrote:
> On Thu, 2017-08-03 at 21:33 -0700, Florian Fainelli wrote:
>> During testing with a background iperf pushing 1Gbit/sec worth of
>> traffic and having both ifconfig and ethtool collect statistics, we
>> could see quite frequent deadlocks. Convert the often accessed DSA slave
>> network devices statistics to per-cpu 64-bit statistics to remove these
>> deadlocks and provide fast efficient statistics updates.
>>
> 
> This seems to be a bug fix, it would be nice to get a proper tag like :
> 
> Fixes: f613ed665bb3 ("net: dsa: Add support for 64-bit statistics")

Right, should have been added, thanks!

> 
> Problem here is that if multiple cpus can call dsa_switch_rcv() at the
> same time, then u64_stats_update_begin() contract is not respected.

This is really where I struggled understanding what is wrong in the
non-per CPU version, my understanding is that we have:

- writers for xmit executes in process context
- writers for receive executes from NAPI (from the DSA's master network
device through it's own NAPI doing netif_receive_skb -> netdev_uses_dsa
-> netif_receive_skb)

readers should all execute in process context. The test scenario that
led to a deadlock involved running iperf in the background, having a
while loop with both ifconfig and ethtool reading stats, and somehow
when iperf exited, either reader would just be locked. So I guess this
leaves us with the two writers not being mutually excluded then, right?

> 
> include/linux/u64_stats_sync.h states :
> 
>  * Usage :
>  *
>  * Stats producer (writer) should use following template granted it already got
>  * an exclusive access to counters (a lock is already taken, or per cpu
>  * data is used [in a non preemptable context])
>  *
>  *   spin_lock_bh(...) or other synchronization to get exclusive access
>  *   ...
>  *   u64_stats_update_begin(&stats->syncp);
> 
> 
> 

-- 
Florian

^ permalink raw reply

* Re: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data ACKed/SACKed
From: Neal Cardwell @ 2017-08-04 15:54 UTC (permalink / raw)
  To: maowenan
  Cc: David Miller, netdev@vger.kernel.org, Yuchung Cheng,
	Nandita Dukkipati
In-Reply-To: <F95AC9340317A84688A5F0DF0246F3F20293523A@DGGEMI512-MBX.china.huawei.com>

On Fri, Aug 4, 2017 at 3:12 AM, maowenan <maowenan@huawei.com> wrote:
> > > --- a/net/ipv4/tcp_output.c
> > > +++ b/net/ipv4/tcp_output.c
> > > @@ -2380,21 +2380,12 @@ bool tcp_schedule_loss_probe(struct sock *sk)
> > >     u32 rtt = usecs_to_jiffies(tp->srtt_us >> 3);
> > >     u32 timeout, rto_delta_us;
> > >
> > > -   /* No consecutive loss probes. */
> > > -   if (WARN_ON(icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)) {
> > > -           tcp_rearm_rto(sk);
> > > -           return false;
> > > -   }
> [Mao Wenan] I'm sorry I can't get why you delete this and below "if" branch?

We deleted those two "if" branches in tcp_schedule_loss_probe()
because they were assuming that TLP probes would only be scheduled in
a context where an RTO had already been scheduled. With the old
implementation that was true: on every ACK (tcp_ack()) or send of new
data (tcp_event_new_data_sent()) we would first schedule an RTO (by
calling tcp_rearm_rto()) and then schedule a TLP (by calling
tcp_schedule_loss_probe()). So the checks were the right ones for the
old implementation.

With the new implementation, we do not first rearm the RTO on every
incoming ACK. That means when we get to tcp_schedule_loss_probe() we
may find either an RTO or TLP is pending.

Hope that helps clear that up.

cheers,
neal

^ permalink raw reply

* Re: [PATCH] iproute2/misc: do not mix CFLAGS with LDFLAGS
From: Stephen Hemminger @ 2017-08-04 16:09 UTC (permalink / raw)
  To: Marcus Meissner; +Cc: netdev
In-Reply-To: <20170804095402.30943-1-meissner@suse.de>

On Fri,  4 Aug 2017 11:54:02 +0200
Marcus Meissner <meissner@suse.de> wrote:

> during linking, do not use CFLAGS. This avoid clashes when doing PIE builds.
> ---
>  misc/Makefile | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/misc/Makefile b/misc/Makefile
> index 72807678..1d86c44d 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -23,17 +23,17 @@ all: $(TARGETS)
>  ss: $(SSOBJ)
>  	$(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@
>  
> -nstat: nstat.c
> -	$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o nstat nstat.c $(LIBNETLINK) -lm
> +nstat: nstat.o
> +	$(QUIET_CC)$(CC) $(LDFLAGS) -o nstat nstat.o $(LIBNETLINK) -lm
>  
> -ifstat: ifstat.c
> -	$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o ifstat ifstat.c $(LIBNETLINK) -lm
> +ifstat: ifstat.o
> +	$(QUIET_CC)$(CC) $(LDFLAGS) -o ifstat ifstat.o $(LIBNETLINK) -lm
>  
> -rtacct: rtacct.c
> -	$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o rtacct rtacct.c $(LIBNETLINK) -lm
> +rtacct: rtacct.o
> +	$(QUIET_CC)$(CC) $(LDFLAGS) -o rtacct rtacct.o $(LIBNETLINK) -lm
>  
> -arpd: arpd.c
> -	$(QUIET_CC)$(CC) $(CFLAGS) -I$(DBM_INCLUDE) $(LDFLAGS) -o arpd arpd.c $(LIBNETLINK) -ldb -lpthread
> +arpd: arpd.o
> +	$(QUIET_CC)$(CC) $(LDFLAGS) -o arpd arpd.o $(LIBNETLINK) -ldb -lpthread
>  
>  ssfilter.c: ssfilter.y
>  	$(QUIET_YACC)bison ssfilter.y -o ssfilter.c

Some CFLAGS do need to be passed to gcc when doing linking, think of -flto
I don't see this on gcc with Debian and hardening.

^ permalink raw reply

* [PATCH net-next V2] net ipv6: convert fib6_table rwlock to a percpu lock
From: Shaohua Li @ 2017-08-04 16:38 UTC (permalink / raw)
  To: netdev, davem
  Cc: Kernel-team, Shaohua Li, Wei Wang, Eric Dumazet,
	Stephen Hemminger

From: Shaohua Li <shli@fb.com>

In a syn flooding test, the fib6_table rwlock is a significant
bottleneck. While converting the rwlock to rcu sounds straighforward,
but is very challenging if it's possible. A percpu spinlock (lglock has
been removed from kernel, so I added a simple implementation here) is
quite trival for this problem since updating the routing table is a rare
event. In my test, the server receives around 1.5 Mpps in syn flooding
test without the patch in a dual sockets and 56-CPU system. With the
patch, the server receives around 3.8Mpps, and perf report doesn't show
the locking issue.

Of course the percpu lock isn't as good as rcu, so this isn't intended
to replace rcu, but this is much better than current readwrite lock.
Before we have a rcu implementation, this is a good temporary solution.
Plus, this is a trival change, there is nothing to prevent pursuing a
rcu implmentation.

Cc: Wei Wang <weiwan@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Shaohua Li <shli@fb.com>
---
 include/net/ip6_fib.h | 57 +++++++++++++++++++++++++++++++++++++-
 net/ipv6/addrconf.c   |  8 +++---
 net/ipv6/ip6_fib.c    | 76 ++++++++++++++++++++++++++++-----------------------
 net/ipv6/route.c      | 54 ++++++++++++++++++------------------
 4 files changed, 129 insertions(+), 66 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 1d790ea..124eb04 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -247,7 +247,7 @@ struct rt6_statistics {
 struct fib6_table {
 	struct hlist_node	tb6_hlist;
 	u32			tb6_id;
-	rwlock_t		tb6_lock;
+	spinlock_t __percpu	*percpu_tb6_lock;
 	struct fib6_node	tb6_root;
 	struct inet_peer_base	tb6_peers;
 	unsigned int		flags;
@@ -255,6 +255,61 @@ struct fib6_table {
 #define RT6_TABLE_HAS_DFLT_ROUTER	BIT(0)
 };
 
+static inline void fib6_table_read_lock_bh(struct fib6_table *table)
+{
+	preempt_disable();
+	spin_lock_bh(this_cpu_ptr(table->percpu_tb6_lock));
+}
+
+static inline void fib6_table_read_unlock_bh(struct fib6_table *table)
+{
+	spin_unlock_bh(this_cpu_ptr(table->percpu_tb6_lock));
+	preempt_enable();
+}
+
+static inline void fib6_table_read_lock(struct fib6_table *table)
+{
+	preempt_disable();
+	spin_lock(this_cpu_ptr(table->percpu_tb6_lock));
+}
+
+static inline void fib6_table_read_unlock(struct fib6_table *table)
+{
+	spin_unlock(this_cpu_ptr(table->percpu_tb6_lock));
+	preempt_enable();
+}
+
+static inline void fib6_table_write_lock_bh(struct fib6_table *table)
+{
+	int first = nr_cpu_ids;
+	int i;
+
+	for_each_possible_cpu(i) {
+		if (first == nr_cpu_ids) {
+			first = i;
+			spin_lock_bh(per_cpu_ptr(table->percpu_tb6_lock, i));
+		} else
+			spin_lock_nest_lock(
+				per_cpu_ptr(table->percpu_tb6_lock, i),
+				per_cpu_ptr(table->percpu_tb6_lock, first));
+	}
+}
+
+static inline void fib6_table_write_unlock_bh(struct fib6_table *table)
+{
+	int first = nr_cpu_ids;
+	int i;
+
+	for_each_possible_cpu(i) {
+		if (first == nr_cpu_ids) {
+			first = i;
+			continue;
+		}
+		spin_unlock(per_cpu_ptr(table->percpu_tb6_lock, i));
+	}
+	spin_unlock_bh(per_cpu_ptr(table->percpu_tb6_lock, first));
+}
+
 #define RT6_TABLE_UNSPEC	RT_TABLE_UNSPEC
 #define RT6_TABLE_MAIN		RT_TABLE_MAIN
 #define RT6_TABLE_DFLT		RT6_TABLE_MAIN
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 30ee23e..22e2ad2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2313,7 +2313,7 @@ static struct rt6_info *addrconf_get_prefix_route(const struct in6_addr *pfx,
 	if (!table)
 		return NULL;
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	fn = fib6_locate(&table->tb6_root, pfx, plen, NULL, 0);
 	if (!fn)
 		goto out;
@@ -2330,7 +2330,7 @@ static struct rt6_info *addrconf_get_prefix_route(const struct in6_addr *pfx,
 		break;
 	}
 out:
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 	return rt;
 }
 
@@ -5929,7 +5929,7 @@ void addrconf_disable_policy_idev(struct inet6_dev *idev, int val)
 			struct fib6_table *table = rt->rt6i_table;
 			int cpu;
 
-			read_lock(&table->tb6_lock);
+			fib6_table_read_lock(table);
 			addrconf_set_nopolicy(ifa->rt, val);
 			if (rt->rt6i_pcpu) {
 				for_each_possible_cpu(cpu) {
@@ -5939,7 +5939,7 @@ void addrconf_disable_policy_idev(struct inet6_dev *idev, int val)
 					addrconf_set_nopolicy(*rtp, val);
 				}
 			}
-			read_unlock(&table->tb6_lock);
+			fib6_table_read_unlock(table);
 		}
 		spin_unlock(&ifa->lock);
 	}
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 69ed004..a6ae228 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -187,8 +187,16 @@ static void fib6_link_table(struct net *net, struct fib6_table *tb)
 	 * Initialize table lock at a single place to give lockdep a key,
 	 * tables aren't visible prior to being linked to the list.
 	 */
-	rwlock_init(&tb->tb6_lock);
-
+	for_each_possible_cpu(h) {
+		/*
+		 * make sure the first lock and other locks have different
+		 * lockdep map, so we can treat the first lock as nested lock
+		 */
+		if (h == 0)
+			spin_lock_init(per_cpu_ptr(tb->percpu_tb6_lock, h));
+		else
+			spin_lock_init(per_cpu_ptr(tb->percpu_tb6_lock, h));
+	}
 	h = tb->tb6_id & (FIB6_TABLE_HASHSZ - 1);
 
 	/*
@@ -198,23 +206,34 @@ static void fib6_link_table(struct net *net, struct fib6_table *tb)
 	hlist_add_head_rcu(&tb->tb6_hlist, &net->ipv6.fib_table_hash[h]);
 }
 
-#ifdef CONFIG_IPV6_MULTIPLE_TABLES
-
-static struct fib6_table *fib6_alloc_table(struct net *net, u32 id)
+static struct fib6_table *fib6_alloc_table(struct net *net, u32 id, gfp_t gfp)
 {
 	struct fib6_table *table;
 
-	table = kzalloc(sizeof(*table), GFP_ATOMIC);
-	if (table) {
+	table = kzalloc(sizeof(*table), gfp);
+	if (!table)
+		return NULL;
+	table->percpu_tb6_lock = alloc_percpu_gfp(struct spinlock, gfp);
+	if (table->percpu_tb6_lock) {
 		table->tb6_id = id;
 		table->tb6_root.leaf = net->ipv6.ip6_null_entry;
 		table->tb6_root.fn_flags = RTN_ROOT | RTN_TL_ROOT | RTN_RTINFO;
 		inet_peer_base_init(&table->tb6_peers);
+	} else {
+		kfree(table);
+		return NULL;
 	}
 
 	return table;
 }
 
+static void fib6_free_table(struct fib6_table *table)
+{
+	free_percpu(table->percpu_tb6_lock);
+	kfree(table);
+}
+
+#ifdef CONFIG_IPV6_MULTIPLE_TABLES
 struct fib6_table *fib6_new_table(struct net *net, u32 id)
 {
 	struct fib6_table *tb;
@@ -225,7 +244,7 @@ struct fib6_table *fib6_new_table(struct net *net, u32 id)
 	if (tb)
 		return tb;
 
-	tb = fib6_alloc_table(net, id);
+	tb = fib6_alloc_table(net, id, GFP_ATOMIC);
 	if (tb)
 		fib6_link_table(net, tb);
 
@@ -462,9 +481,9 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
 		w->count = 0;
 		w->skip = 0;
 
-		read_lock_bh(&table->tb6_lock);
+		fib6_table_read_lock_bh(table);
 		res = fib6_walk(net, w);
-		read_unlock_bh(&table->tb6_lock);
+		fib6_table_read_unlock_bh(table);
 		if (res > 0) {
 			cb->args[4] = 1;
 			cb->args[5] = w->root->fn_sernum;
@@ -479,9 +498,9 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
 		} else
 			w->skip = 0;
 
-		read_lock_bh(&table->tb6_lock);
+		fib6_table_read_lock_bh(table);
 		res = fib6_walk_continue(w);
-		read_unlock_bh(&table->tb6_lock);
+		fib6_table_read_unlock_bh(table);
 		if (res <= 0) {
 			fib6_walker_unlink(net, w);
 			cb->args[4] = 0;
@@ -1811,10 +1830,10 @@ static void __fib6_clean_all(struct net *net,
 	for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
 		head = &net->ipv6.fib_table_hash[h];
 		hlist_for_each_entry_rcu(table, head, tb6_hlist) {
-			write_lock_bh(&table->tb6_lock);
+			fib6_table_write_lock_bh(table);
 			fib6_clean_tree(net, &table->tb6_root,
 					func, false, sernum, arg);
-			write_unlock_bh(&table->tb6_lock);
+			fib6_table_write_unlock_bh(table);
 		}
 	}
 	rcu_read_unlock();
@@ -1962,27 +1981,16 @@ static int __net_init fib6_net_init(struct net *net)
 	if (!net->ipv6.fib_table_hash)
 		goto out_rt6_stats;
 
-	net->ipv6.fib6_main_tbl = kzalloc(sizeof(*net->ipv6.fib6_main_tbl),
-					  GFP_KERNEL);
+	net->ipv6.fib6_main_tbl = fib6_alloc_table(net, RT6_TABLE_MAIN,
+		GFP_KERNEL);
 	if (!net->ipv6.fib6_main_tbl)
 		goto out_fib_table_hash;
 
-	net->ipv6.fib6_main_tbl->tb6_id = RT6_TABLE_MAIN;
-	net->ipv6.fib6_main_tbl->tb6_root.leaf = net->ipv6.ip6_null_entry;
-	net->ipv6.fib6_main_tbl->tb6_root.fn_flags =
-		RTN_ROOT | RTN_TL_ROOT | RTN_RTINFO;
-	inet_peer_base_init(&net->ipv6.fib6_main_tbl->tb6_peers);
-
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
-	net->ipv6.fib6_local_tbl = kzalloc(sizeof(*net->ipv6.fib6_local_tbl),
-					   GFP_KERNEL);
+	net->ipv6.fib6_local_tbl = fib6_alloc_table(net, RT6_TABLE_LOCAL,
+		GFP_KERNEL);
 	if (!net->ipv6.fib6_local_tbl)
 		goto out_fib6_main_tbl;
-	net->ipv6.fib6_local_tbl->tb6_id = RT6_TABLE_LOCAL;
-	net->ipv6.fib6_local_tbl->tb6_root.leaf = net->ipv6.ip6_null_entry;
-	net->ipv6.fib6_local_tbl->tb6_root.fn_flags =
-		RTN_ROOT | RTN_TL_ROOT | RTN_RTINFO;
-	inet_peer_base_init(&net->ipv6.fib6_local_tbl->tb6_peers);
 #endif
 	fib6_tables_init(net);
 
@@ -1990,7 +1998,7 @@ static int __net_init fib6_net_init(struct net *net)
 
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
 out_fib6_main_tbl:
-	kfree(net->ipv6.fib6_main_tbl);
+	fib6_free_table(net->ipv6.fib6_main_tbl);
 #endif
 out_fib_table_hash:
 	kfree(net->ipv6.fib_table_hash);
@@ -2008,10 +2016,10 @@ static void fib6_net_exit(struct net *net)
 
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
 	inetpeer_invalidate_tree(&net->ipv6.fib6_local_tbl->tb6_peers);
-	kfree(net->ipv6.fib6_local_tbl);
+	fib6_free_table(net->ipv6.fib6_local_tbl);
 #endif
 	inetpeer_invalidate_tree(&net->ipv6.fib6_main_tbl->tb6_peers);
-	kfree(net->ipv6.fib6_main_tbl);
+	fib6_free_table(net->ipv6.fib6_main_tbl);
 	kfree(net->ipv6.fib_table_hash);
 	kfree(net->ipv6.rt6_stats);
 	fib6_notifier_exit(net);
@@ -2175,9 +2183,9 @@ static void *ipv6_route_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 
 iter_table:
 	ipv6_route_check_sernum(iter);
-	read_lock(&iter->tbl->tb6_lock);
+	fib6_table_read_lock(iter->tbl);
 	r = fib6_walk_continue(&iter->w);
-	read_unlock(&iter->tbl->tb6_lock);
+	fib6_table_read_unlock(iter->tbl);
 	if (r > 0) {
 		if (v)
 			++*pos;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index aba07fc..9e2d141 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -877,7 +877,7 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 	struct fib6_node *fn;
 	struct rt6_info *rt;
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 restart:
 	rt = fn->leaf;
@@ -890,7 +890,7 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 			goto restart;
 	}
 	dst_use(&rt->dst, jiffies);
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 
 	trace_fib6_table_lookup(net, rt, table->tb6_id, fl6);
 
@@ -944,9 +944,9 @@ static int __ip6_ins_rt(struct rt6_info *rt, struct nl_info *info,
 	struct fib6_table *table;
 
 	table = rt->rt6i_table;
-	write_lock_bh(&table->tb6_lock);
+	fib6_table_write_lock_bh(table);
 	err = fib6_add(&table->tb6_root, rt, info, mxc, extack);
-	write_unlock_bh(&table->tb6_lock);
+	fib6_table_write_unlock_bh(table);
 
 	return err;
 }
@@ -1044,7 +1044,7 @@ static struct rt6_info *rt6_make_pcpu_route(struct rt6_info *rt)
 		return net->ipv6.ip6_null_entry;
 	}
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	if (rt->rt6i_pcpu) {
 		p = this_cpu_ptr(rt->rt6i_pcpu);
 		prev = cmpxchg(p, NULL, pcpu_rt);
@@ -1065,7 +1065,7 @@ static struct rt6_info *rt6_make_pcpu_route(struct rt6_info *rt)
 	}
 	dst_hold(&pcpu_rt->dst);
 	rt6_dst_from_metrics_check(pcpu_rt);
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 	return pcpu_rt;
 }
 
@@ -1081,7 +1081,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 	if (net->ipv6.devconf_all->forwarding == 0)
 		strict |= RT6_LOOKUP_F_REACHABLE;
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 
 	fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 	saved_fn = fn;
@@ -1108,7 +1108,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 
 	if (rt == net->ipv6.ip6_null_entry || (rt->rt6i_flags & RTF_CACHE)) {
 		dst_use(&rt->dst, jiffies);
-		read_unlock_bh(&table->tb6_lock);
+		fib6_table_read_unlock_bh(table);
 
 		rt6_dst_from_metrics_check(rt);
 
@@ -1125,7 +1125,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 		struct rt6_info *uncached_rt;
 
 		dst_use(&rt->dst, jiffies);
-		read_unlock_bh(&table->tb6_lock);
+		fib6_table_read_unlock_bh(table);
 
 		uncached_rt = ip6_rt_cache_alloc(rt, &fl6->daddr, NULL);
 		dst_release(&rt->dst);
@@ -1153,14 +1153,14 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 		pcpu_rt = rt6_get_pcpu_route(rt);
 
 		if (pcpu_rt) {
-			read_unlock_bh(&table->tb6_lock);
+			fib6_table_read_unlock_bh(table);
 		} else {
 			/* We have to do the read_unlock first
 			 * because rt6_make_pcpu_route() may trigger
 			 * ip6_dst_gc() which will take the write_lock.
 			 */
 			dst_hold(&rt->dst);
-			read_unlock_bh(&table->tb6_lock);
+			fib6_table_read_unlock_bh(table);
 			pcpu_rt = rt6_make_pcpu_route(rt);
 			dst_release(&rt->dst);
 		}
@@ -1503,7 +1503,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net,
 	 * routes.
 	 */
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	fn = fib6_lookup(&table->tb6_root, &fl6->daddr, &fl6->saddr);
 restart:
 	for (rt = fn->leaf; rt; rt = rt->dst.rt6_next) {
@@ -1536,7 +1536,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net,
 out:
 	dst_hold(&rt->dst);
 
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 
 	trace_fib6_table_lookup(net, rt, table->tb6_id, fl6);
 	return rt;
@@ -2140,9 +2140,9 @@ static int __ip6_del_rt(struct rt6_info *rt, struct nl_info *info)
 	}
 
 	table = rt->rt6i_table;
-	write_lock_bh(&table->tb6_lock);
+	fib6_table_write_lock_bh(table);
 	err = fib6_del(rt, info);
-	write_unlock_bh(&table->tb6_lock);
+	fib6_table_write_unlock_bh(table);
 
 out:
 	ip6_rt_put(rt);
@@ -2168,7 +2168,7 @@ static int __ip6_del_rt_siblings(struct rt6_info *rt, struct fib6_config *cfg)
 	if (rt == net->ipv6.ip6_null_entry)
 		goto out_put;
 	table = rt->rt6i_table;
-	write_lock_bh(&table->tb6_lock);
+	fib6_table_write_lock_bh(table);
 
 	if (rt->rt6i_nsiblings && cfg->fc_delete_all_nh) {
 		struct rt6_info *sibling, *next_sibling;
@@ -2198,7 +2198,7 @@ static int __ip6_del_rt_siblings(struct rt6_info *rt, struct fib6_config *cfg)
 
 	err = fib6_del(rt, info);
 out_unlock:
-	write_unlock_bh(&table->tb6_lock);
+	fib6_table_write_unlock_bh(table);
 out_put:
 	ip6_rt_put(rt);
 
@@ -2223,7 +2223,7 @@ static int ip6_route_del(struct fib6_config *cfg,
 		return err;
 	}
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 
 	fn = fib6_locate(&table->tb6_root,
 			 &cfg->fc_dst, cfg->fc_dst_len,
@@ -2246,7 +2246,7 @@ static int ip6_route_del(struct fib6_config *cfg,
 			if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol)
 				continue;
 			dst_hold(&rt->dst);
-			read_unlock_bh(&table->tb6_lock);
+			fib6_table_read_unlock_bh(table);
 
 			/* if gateway was specified only delete the one hop */
 			if (cfg->fc_flags & RTF_GATEWAY)
@@ -2255,7 +2255,7 @@ static int ip6_route_del(struct fib6_config *cfg,
 			return __ip6_del_rt_siblings(rt, cfg);
 		}
 	}
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 
 	return err;
 }
@@ -2434,7 +2434,7 @@ static struct rt6_info *rt6_get_route_info(struct net *net,
 	if (!table)
 		return NULL;
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	fn = fib6_locate(&table->tb6_root, prefix, prefixlen, NULL, 0);
 	if (!fn)
 		goto out;
@@ -2450,7 +2450,7 @@ static struct rt6_info *rt6_get_route_info(struct net *net,
 		break;
 	}
 out:
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 	return rt;
 }
 
@@ -2495,7 +2495,7 @@ struct rt6_info *rt6_get_dflt_router(const struct in6_addr *addr, struct net_dev
 	if (!table)
 		return NULL;
 
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	for (rt = table->tb6_root.leaf; rt; rt = rt->dst.rt6_next) {
 		if (dev == rt->dst.dev &&
 		    ((rt->rt6i_flags & (RTF_ADDRCONF | RTF_DEFAULT)) == (RTF_ADDRCONF | RTF_DEFAULT)) &&
@@ -2504,7 +2504,7 @@ struct rt6_info *rt6_get_dflt_router(const struct in6_addr *addr, struct net_dev
 	}
 	if (rt)
 		dst_hold(&rt->dst);
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 	return rt;
 }
 
@@ -2541,17 +2541,17 @@ static void __rt6_purge_dflt_routers(struct fib6_table *table)
 	struct rt6_info *rt;
 
 restart:
-	read_lock_bh(&table->tb6_lock);
+	fib6_table_read_lock_bh(table);
 	for (rt = table->tb6_root.leaf; rt; rt = rt->dst.rt6_next) {
 		if (rt->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF) &&
 		    (!rt->rt6i_idev || rt->rt6i_idev->cnf.accept_ra != 2)) {
 			dst_hold(&rt->dst);
-			read_unlock_bh(&table->tb6_lock);
+			fib6_table_read_unlock_bh(table);
 			ip6_del_rt(rt);
 			goto restart;
 		}
 	}
-	read_unlock_bh(&table->tb6_lock);
+	fib6_table_read_unlock_bh(table);
 
 	table->flags &= ~RT6_TABLE_HAS_DFLT_ROUTER;
 }
-- 
2.9.3

^ permalink raw reply related

* Re: [RFC] iproute: Add support for extended ack to rtnl_talk
From: Stephen Hemminger @ 2017-08-04 16:47 UTC (permalink / raw)
  To: Simon Horman
  Cc: David Ahern, Daniel Borkmann, Phil Sutter, David Miller, netdev
In-Reply-To: <20170804113147.GA25254@vergenet.net>

On Fri, 4 Aug 2017 13:31:48 +0200
Simon Horman <simon.horman@netronome.com> wrote:

> On Thu, Aug 03, 2017 at 02:26:58PM -0600, David Ahern wrote:
> > On 5/18/17 10:24 PM, David Ahern wrote:  
> > > On 5/18/17 3:02 AM, Daniel Borkmann wrote:  
> > >> So effectively this means libmnl has to be used for new stuff, noone
> > >> has time to do the work to convert the existing tooling over (which
> > >> by itself might be a challenge in testing everything to make sure
> > >> there are no regressions) given there's not much activity around
> > >> lib/libnetlink.c anyway, and existing users not using libmnl today
> > >> won't see/notice new improvements on netlink side when they do an
> > >> upgrade. So we'll be stuck with that dual library mess pretty much
> > >> for a very long time. :(  
> > > 
> > > lib/libnetlink.c with all of its duplicate functions weighs in at just
> > > 947 LOC -- a mere 12% of the code in lib/. From a total SLOC of iproute2
> > > it is a negligible part of the code base.
> > > 
> > > Given that, there is very little gain -- but a lot of risk in
> > > regressions -- in converting such a small, low level code base to libmnl
> > > just for the sake of using a library - something Phil noted in his
> > > cursory attempt at converting ip to libmnl. ie., The level effort
> > > required vs the benefit is just not worth it.
> > > 
> > > There are so many other parts of the ip code base that need work with a
> > > much higher return on the time investment.
> > >   
> > 
> > Stephen: It has been 3 months since the first extack patches were posted
> > and still nothing in iproute2, all of it hung up on your decision to
> > require libmnl. Do you plan to finish the libmnl support any time soon
> > and send out patches?  
> 
> FWIIW I would also like to see some way to get this enhancement accepted.

I will put in the libmnl version. If it doesn't work because no one sent
me test cases, then fine. send a patch for that.

^ permalink raw reply

* Re: XFRM pcpu cache issue
From: Florian Westphal @ 2017-08-04 16:55 UTC (permalink / raw)
  To: Ilan Tayari
  Cc: Florian Westphal, Steffen Klassert, netdev@vger.kernel.org,
	Yevgeny Kliteynik, Yossi Kuperman, Boris Pismenny, Yossef Efraim
In-Reply-To: <AM4PR0501MB1940130EB22A118F3524DA54DBB10@AM4PR0501MB1940.eurprd05.prod.outlook.com>

Ilan Tayari <ilant@mellanox.com> wrote:
> I debugged a little the regression I told you about the other day...
> 
> Steps and Symptoms:
> 1. Set up a host-to-host IPSec tunnel (or transport, doesn't matter)
> 2. Ping over IPSec, or do something to populate the pcpu cache
> 3. Join a MC group, then leave MC group
> 4. Try to ping again using same CPU as before -> traffic doesn't egress the machine at all
> 
> If trying from another CPU (with clean cache), it pings well.
> If clearing the pcpu cache, it works well again.

Yes, I think i see the problem, thanks for debugging this.

I dropped the stale_bundle() check vs. rfc, that was a stupid thing
to do because that is what would detect this....


Does this help?

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1818,7 +1818,8 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
 	    xdst->num_pols == num_pols &&
 	    !xfrm_pol_dead(xdst) &&
 	    memcmp(xdst->pols, pols,
-		   sizeof(struct xfrm_policy *) * num_pols) == 0) {
+		   sizeof(struct xfrm_policy *) * num_pols) == 0 &&
+	    xfrm_bundle_ok(xdst)) {
 		dst_hold(&xdst->u.dst);
 		return xdst;
 	}

^ permalink raw reply

* Re: [PATCH net 3/3] tcp: fix xmit timer to only be reset if data ACKed/SACKed
From: Neal Cardwell @ 2017-08-04 16:59 UTC (permalink / raw)
  To: maowenan
  Cc: David Miller, netdev@vger.kernel.org, Yuchung Cheng,
	Nandita Dukkipati
In-Reply-To: <F95AC9340317A84688A5F0DF0246F3F2029373BA@DGGEMI512-MBX.china.huawei.com>

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

On Fri, Aug 4, 2017 at 3:33 AM, maowenan <maowenan@huawei.com> wrote:
> [Mao Wenan]Follow previous mail, in lower version such as 3.10, I found
> there are many timer type, e.g:ICSK_TIME_EARLY_RETRANS, RTO,PTO
> are used. I'm not sure there exist some unknown problem if we don't check
> isck_pending here and below if branch? And could you please post one lower
> version patch such as 3.10?  Thanks a lot.
>
> #define ICSK_TIME_RETRANS       1       /* Retransmit timer */
> #define ICSK_TIME_DACK          2       /* Delayed ack timer */
> #define ICSK_TIME_PROBE0        3       /* Zero window probe timer */
> #define ICSK_TIME_EARLY_RETRANS 4       /* Early retransmit timer */
> #define ICSK_TIME_LOSS_PROBE    5       /* Tail loss probe timer */

I think you'll find that it you audit how these patches interact with
those other timer types you'll find that the behavior either matches
the existing code, or is improved. We ran these patches through our
packetdrill test suite and found that was the case for all our
existing tests. But please let us know if you find a specific scenario
where that is not the case.

I have attached patches for this fix rebased on to v3.10.107, the
latest stable release for 3.10. That's pretty far back in history, so
there were substantial conflict resolutions and adjustments required.
:-) Hope that helps.

thanks,
neal

[-- Attachment #2: 0001-tcp-introduce-tcp_rto_delta_us-helper-for-xmit-timer.patch --]
[-- Type: application/octet-stream, Size: 2261 bytes --]

From 6e5001de12d20fee1696fe50df51b05438278f4c Mon Sep 17 00:00:00 2001
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 27 Jul 2017 10:01:20 -0400
Subject: [PATCH 1/3] tcp: introduce tcp_rto_delta_us() helper for xmit timer
 fix

Pure refactor. This helper will be required in the xmit timer fix
later in the patch series. (Because the TLP logic will want to make
this calculation.)

[This version of the commit was compiled and briefly tested
based on top of v3.10.107.]

Change-Id: I1ccfba0b00465454bf5ce22e6fef5f7b5dd94d15
Fixes: 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)")
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Nandita Dukkipati <nanditad@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h    | 10 ++++++++++
 net/ipv4/tcp_input.c |  4 +---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 79cd118d5994..c4db9acefa9c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1592,4 +1592,14 @@ struct tcp_request_sock_ops {
 extern void tcp_v4_init(void);
 extern void tcp_init(void);
 
+/* At how many jiffies into the future should the RTO fire? */
+static inline s32 tcp_rto_delta(const struct sock *sk)
+{
+	const struct sk_buff *skb = tcp_write_queue_head(sk);
+	const u32 rto = inet_csk(sk)->icsk_rto;
+	const u32 rto_time_stamp = TCP_SKB_CB(skb)->when + rto;
+
+	return (s32)(rto_time_stamp - tcp_time_stamp);
+}
+
 #endif	/* _TCP_H */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0680058fe693..a36b7c55b76b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2972,9 +2972,7 @@ void tcp_rearm_rto(struct sock *sk)
 		/* Offset the time elapsed after installing regular RTO */
 		if (icsk->icsk_pending == ICSK_TIME_EARLY_RETRANS ||
 		    icsk->icsk_pending == ICSK_TIME_LOSS_PROBE) {
-			struct sk_buff *skb = tcp_write_queue_head(sk);
-			const u32 rto_time_stamp = TCP_SKB_CB(skb)->when + rto;
-			s32 delta = (s32)(rto_time_stamp - tcp_time_stamp);
+			s32 delta = tcp_rto_delta(sk);
 			/* delta may not be positive if the socket is locked
 			 * when the retrans timer fires and is rescheduled.
 			 */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog


[-- Attachment #3: 0002-tcp-enable-xmit-timer-fix-by-having-TLP-use-time-whe.patch --]
[-- Type: application/octet-stream, Size: 3106 bytes --]

From 50c4e5e3bd4a437a3fbc237e0c248f758aa2ab85 Mon Sep 17 00:00:00 2001
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 27 Jul 2017 10:09:30 -0400
Subject: [PATCH 2/3] tcp: enable xmit timer fix by having TLP use time when
 RTO should fire

Have tcp_schedule_loss_probe() base the TLP scheduling decision based
on when the RTO *should* fire. This is to enable the upcoming xmit
timer fix in this series, where tcp_schedule_loss_probe() cannot
assume that the last timer installed was an RTO timer (because we are
no longer doing the "rearm RTO, rearm RTO, rearm TLP" dance on every
ACK). So tcp_schedule_loss_probe() must independently figure out when
an RTO would want to fire.

In the new TLP implementation following in this series, we cannot
assume that icsk_timeout was set based on an RTO; after processing a
cumulative ACK the icsk_timeout we see can be from a previous TLP or
RTO. So we need to independently recalculate the RTO time (instead of
reading it out of icsk_timeout). Removing this dependency on the
nature of icsk_timeout makes things a little easier to reason about
anyway.

Note that the old and new code should be equivalent, since they are
both saying: "if the RTO is in the future, but at an earlier time than
the normal TLP time, then set the TLP timer to fire when the RTO would
have fired".

[This version of the commit was compiled and briefly tested
based on top of v3.10.107.]

Change-Id: I597ad6446edde15bf2cea8e56d603a2c52f8221b
Fixes: 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)")
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Nandita Dukkipati <nanditad@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_output.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8729a934124f..135440283f21 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1947,8 +1947,8 @@ bool tcp_schedule_loss_probe(struct sock *sk)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
-	u32 timeout, tlp_time_stamp, rto_time_stamp;
 	u32 rtt = tp->srtt >> 3;
+	u32 timeout, rto_delta;
 
 	if (WARN_ON(icsk->icsk_pending == ICSK_TIME_EARLY_RETRANS))
 		return false;
@@ -1987,14 +1987,10 @@ bool tcp_schedule_loss_probe(struct sock *sk)
 				(rtt + (rtt >> 1) + TCP_DELACK_MAX));
 	timeout = max_t(u32, timeout, msecs_to_jiffies(10));
 
-	/* If RTO is shorter, just schedule TLP in its place. */
-	tlp_time_stamp = tcp_time_stamp + timeout;
-	rto_time_stamp = (u32)inet_csk(sk)->icsk_timeout;
-	if ((s32)(tlp_time_stamp - rto_time_stamp) > 0) {
-		s32 delta = rto_time_stamp - tcp_time_stamp;
-		if (delta > 0)
-			timeout = delta;
-	}
+	/* If the RTO formula yields an earlier time, then use that time. */
+	rto_delta = tcp_rto_delta(sk);  /* How far in future is RTO? */
+	if (rto_delta > 0)
+		timeout = min_t(u32, timeout, rto_delta);
 
 	inet_csk_reset_xmit_timer(sk, ICSK_TIME_LOSS_PROBE, timeout,
 				  TCP_RTO_MAX);
-- 
2.14.0.rc1.383.gd1ce394fe2-goog


[-- Attachment #4: 0003-tcp-fix-xmit-timer-to-only-be-reset-if-data-ACKed-SA.patch --]
[-- Type: application/octet-stream, Size: 7689 bytes --]

From c920720b6bd7fa0cc855b16799b9ea11ef95db76 Mon Sep 17 00:00:00 2001
From: Neal Cardwell <ncardwell@google.com>
Date: Wed, 26 Jul 2017 21:43:27 -0400
Subject: [PATCH 3/3] tcp: fix xmit timer to only be reset if data ACKed/SACKed

Fix a TCP loss recovery performance bug raised recently on the netdev
list, in two threads:

(i)  July 26, 2017: netdev thread "TCP fast retransmit issues"
(ii) July 26, 2017: netdev thread:
     "[PATCH V2 net-next] TLP: Don't reschedule PTO when there's one
     outstanding TLP retransmission"

The basic problem is that incoming TCP packets that did not indicate
forward progress could cause the xmit timer (TLP or RTO) to be rearmed
and pushed back in time. In certain corner cases this could result in
the following problems noted in these threads:

 - Repeated ACKs coming in with bogus SACKs corrupted by middleboxes
   could cause TCP to repeatedly schedule TLPs forever. We kept
   sending TLPs after every ~200ms, which elicited bogus SACKs, which
   caused more TLPs, ad infinitum; we never fired an RTO to fill in
   the holes.

 - Incoming data segments could, in some cases, cause us to reschedule
   our RTO or TLP timer further out in time, for no good reason. This
   could cause repeated inbound data to result in stalls in outbound
   data, in the presence of packet loss.

This commit fixes these bugs by changing the TLP and RTO ACK
processing to:

 (a) Only reschedule the xmit timer once per ACK.

 (b) Only reschedule the xmit timer if tcp_clean_rtx_queue() deems the
     ACK indicates sufficient forward progress (a packet was
     cumulatively ACKed, or we got a SACK for a packet that was sent
     before the most recent retransmit of the write queue head).

This brings us back into closer compliance with the RFCs, since, as
the comment for tcp_rearm_rto() notes, we should only restart the RTO
timer after forward progress on the connection. Previously we were
restarting the xmit timer even in these cases where there was no
forward progress.

As a side benefit, this commit simplifies and speeds up the TCP timer
arming logic. We had been calling inet_csk_reset_xmit_timer() three
times on normal ACKs that cumulatively acknowledged some data:

1) Once near the top of tcp_ack() to switch from TLP timer to RTO:
        if (icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)
               tcp_rearm_rto(sk);

2) Once in tcp_clean_rtx_queue(), to update the RTO:
        if (flag & FLAG_ACKED) {
               tcp_rearm_rto(sk);

3) Once in tcp_ack() after tcp_fastretrans_alert() to switch from RTO
   to TLP:
        if (icsk->icsk_pending == ICSK_TIME_RETRANS)
               tcp_schedule_loss_probe(sk);

This commit, by only rescheduling the xmit timer once per ACK,
simplifies the code and reduces CPU overhead.

This commit was tested in an A/B test with Google web server
traffic. SNMP stats and request latency metrics were within noise
levels, substantiating that for normal web traffic patterns this is a
rare issue. This commit was also tested with packetdrill tests to
verify that it fixes the timer behavior in the corner cases discussed
in the netdev threads mentioned above.

This patch is a bug fix patch intended to be queued for -stable
relases.

[This version of the commit was compiled and briefly tested
based on top of v3.10.107.]

Change-Id: If0417380fd59290b65cf04a415373aa13dd1dad7
Fixes: 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)")
Reported-by: Klavs Klavsen <kl@vsen.dk>
Reported-by: Mao Wenan <maowenan@huawei.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Nandita Dukkipati <nanditad@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp_input.c  | 25 +++++++++++++++----------
 net/ipv4/tcp_output.c | 12 ------------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a36b7c55b76b..70f217cb3a1b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -111,6 +111,7 @@ int sysctl_tcp_early_retrans __read_mostly = 3;
 #define FLAG_ORIG_SACK_ACKED	0x200 /* Never retransmitted data are (s)acked	*/
 #define FLAG_SND_UNA_ADVANCED	0x400 /* Snd_una was changed (!= FLAG_DATA_ACKED) */
 #define FLAG_DSACKING_ACK	0x800 /* SACK blocks contained D-SACK info */
+#define FLAG_SET_XMIT_TIMER	0x1000 /* Set TLP or RTO timer */
 #define FLAG_SACK_RENEGING	0x2000 /* snd_una advanced to a sacked seq */
 #define FLAG_UPDATE_TS_RECENT	0x4000 /* tcp_replace_ts_recent() */
 
@@ -3002,6 +3003,13 @@ void tcp_resume_early_retransmit(struct sock *sk)
 	tcp_xmit_retransmit_queue(sk);
 }
 
+/* Try to schedule a loss probe; if that doesn't work, then schedule an RTO. */
+static void tcp_set_xmit_timer(struct sock *sk)
+{
+	if (!tcp_schedule_loss_probe(sk))
+		tcp_rearm_rto(sk);
+}
+
 /* If we get here, the whole TSO packet has not been acked. */
 static u32 tcp_tso_acked(struct sock *sk, struct sk_buff *skb)
 {
@@ -3132,7 +3140,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
 		}
 
 		tcp_ack_update_rtt(sk, flag, seq_rtt);
-		tcp_rearm_rto(sk);
+		flag |= FLAG_SET_XMIT_TIMER;  /* set TLP or RTO timer */
 
 		if (tcp_is_reno(tp)) {
 			tcp_remove_reno_sacks(sk, pkts_acked);
@@ -3392,10 +3400,6 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 	if (after(ack, tp->snd_nxt))
 		goto invalid_ack;
 
-	if (icsk->icsk_pending == ICSK_TIME_EARLY_RETRANS ||
-	    icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)
-		tcp_rearm_rto(sk);
-
 	if (after(ack, prior_snd_una))
 		flag |= FLAG_SND_UNA_ADVANCED;
 
@@ -3452,6 +3456,12 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 
 	pkts_acked = previous_packets_out - tp->packets_out;
 
+	if (tp->tlp_high_seq)
+		tcp_process_tlp_ack(sk, ack, flag);
+	/* If needed, reset TLP/RTO timer; RACK may later override this. */
+	if (flag & FLAG_SET_XMIT_TIMER)
+		tcp_set_xmit_timer(sk);
+
 	if (tcp_ack_is_dubious(sk, flag)) {
 		/* Advance CWND, if state allows this. */
 		if ((flag & FLAG_DATA_ACKED) && tcp_may_raise_cwnd(sk, flag))
@@ -3464,17 +3474,12 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 			tcp_cong_avoid(sk, ack, prior_in_flight);
 	}
 
-	if (tp->tlp_high_seq)
-		tcp_process_tlp_ack(sk, ack, flag);
-
 	if ((flag & FLAG_FORWARD_PROGRESS) || !(flag & FLAG_NOT_DUP)) {
 		struct dst_entry *dst = __sk_dst_get(sk);
 		if (dst)
 			dst_confirm(dst);
 	}
 
-	if (icsk->icsk_pending == ICSK_TIME_RETRANS)
-		tcp_schedule_loss_probe(sk);
 	if (tp->srtt != prior_rtt || tp->snd_cwnd != prior_cwnd)
 		tcp_update_pacing_rate(sk);
 	return 1;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 135440283f21..f5d670ccd403 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1945,28 +1945,16 @@ repair:
 
 bool tcp_schedule_loss_probe(struct sock *sk)
 {
-	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 	u32 rtt = tp->srtt >> 3;
 	u32 timeout, rto_delta;
 
-	if (WARN_ON(icsk->icsk_pending == ICSK_TIME_EARLY_RETRANS))
-		return false;
-	/* No consecutive loss probes. */
-	if (WARN_ON(icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)) {
-		tcp_rearm_rto(sk);
-		return false;
-	}
 	/* Don't do any loss probe on a Fast Open connection before 3WHS
 	 * finishes.
 	 */
 	if (sk->sk_state == TCP_SYN_RECV)
 		return false;
 
-	/* TLP is only scheduled when next timer event is RTO. */
-	if (icsk->icsk_pending != ICSK_TIME_RETRANS)
-		return false;
-
 	/* Schedule a loss probe in 2*RTT for SACK capable connections
 	 * in Open state, that are either limited by cwnd or application.
 	 */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog


^ permalink raw reply related

* Re: [PATCH net-next V2] net ipv6: convert fib6_table rwlock to a percpu lock
From: Eric Dumazet @ 2017-08-04 17:07 UTC (permalink / raw)
  To: Shaohua Li
  Cc: netdev, davem, Kernel-team, Shaohua Li, Wei Wang,
	Stephen Hemminger
In-Reply-To: <2dcefcf7e75a5c242a20d7904096be91addbfba7.1501864467.git.shli@fb.com>

On Fri, 2017-08-04 at 09:38 -0700, Shaohua Li wrote:
> From: Shaohua Li <shli@fb.com>
> 
> In a syn flooding test, the fib6_table rwlock is a significant
> bottleneck. While converting the rwlock to rcu sounds straighforward,
> but is very challenging if it's possible. A percpu spinlock (lglock has
> been removed from kernel, so I added a simple implementation here) is
> quite trival for this problem since updating the routing table is a rare
> event. In my test, the server receives around 1.5 Mpps in syn flooding
> test without the patch in a dual sockets and 56-CPU system. With the
> patch, the server receives around 3.8Mpps, and perf report doesn't show
> the locking issue.
> 
> Of course the percpu lock isn't as good as rcu, so this isn't intended
> to replace rcu, but this is much better than current readwrite lock.
> Before we have a rcu implementation, this is a good temporary solution.
> Plus, this is a trival change, there is nothing to prevent pursuing a
> rcu implmentation.
> 
> Cc: Wei Wang <weiwan@google.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---

Wei has almost done the RCU conversion.

This patch is probably coming too late.

^ 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