Netdev List
 help / color / mirror / Atom feed
* [PATCH RFC net-next 1/1] net/tls: Combined memory allocation for decryption request
From: Vakul Garg @ 2018-08-06 16:38 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180806163810.30880-1-vakul.garg@nxp.com>

For preparing decryption request, several memory chunks are required
(aead_req, sgin, sgout, iv, aad). For submitting the decrypt request to
an accelerator, it is required that the buffers which are read by the
accelerator must be dma-able and not come from stack. The buffers for
aad and iv can be separately kmalloced each, but it is inefficient.
This patch does a combined allocation for preparing decryption request
and then segments into aead_req || sgin || sgout || iv || aad.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 include/net/tls.h |   4 -
 net/tls/tls_sw.c  | 257 +++++++++++++++++++++++++++++++-----------------------
 2 files changed, 148 insertions(+), 113 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index d8b3b6578c01..d5c683e8bb22 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -124,10 +124,6 @@ struct tls_sw_context_rx {
 	struct sk_buff *recv_pkt;
 	u8 control;
 	bool decrypted;
-
-	char rx_aad_ciphertext[TLS_AAD_SPACE_SIZE];
-	char rx_aad_plaintext[TLS_AAD_SPACE_SIZE];
-
 };
 
 struct tls_record_info {
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index cd5ed2d1dbe8..a478b06fc015 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -118,19 +118,13 @@ static int tls_do_decryption(struct sock *sk,
 			     struct scatterlist *sgout,
 			     char *iv_recv,
 			     size_t data_len,
-			     struct sk_buff *skb,
-			     gfp_t flags)
+			     struct aead_request *aead_req)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
-	struct aead_request *aead_req;
-
 	int ret;
 
-	aead_req = aead_request_alloc(ctx->aead_recv, flags);
-	if (!aead_req)
-		return -ENOMEM;
-
+	aead_request_set_tfm(aead_req, ctx->aead_recv);
 	aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
 	aead_request_set_crypt(aead_req, sgin, sgout,
 			       data_len + tls_ctx->rx.tag_size,
@@ -139,8 +133,6 @@ static int tls_do_decryption(struct sock *sk,
 				  crypto_req_done, &ctx->async_wait);
 
 	ret = crypto_wait_req(crypto_aead_decrypt(aead_req), &ctx->async_wait);
-
-	aead_request_free(aead_req);
 	return ret;
 }
 
@@ -727,8 +719,138 @@ static struct sk_buff *tls_wait_data(struct sock *sk, int flags,
 	return skb;
 }
 
+/* This function decrypts the input skb into either out_iov or in out_sg
+ * or in skb buffers itself. The input parameter 'zc' indicates if
+ * zero-copy mode needs to be tried or not. With zero-copy mode, either
+ * out_iov or out_sg must be non-NULL. In case both out_iov and out_sg are
+ * NULL, then the decryption happens inside skb buffers itself, i.e.
+ * zero-copy gets disabled and 'zc' is updated.
+ */
+
+static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
+			    struct iov_iter *out_iov,
+			    struct scatterlist *out_sg,
+			    int *chunk, bool *zc)
+{
+	struct tls_context *tls_ctx = tls_get_ctx(sk);
+	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
+	struct strp_msg *rxm = strp_msg(skb);
+	int n_sgin, n_sgout, nsg, mem_size, aead_size, err, pages = 0;
+	struct aead_request *aead_req;
+	struct sk_buff *unused;
+	u8 *aad, *iv, *mem = NULL;
+	struct scatterlist *sgin = NULL;
+	struct scatterlist *sgout = NULL;
+	const int data_len = rxm->full_len - tls_ctx->rx.overhead_size;
+
+	if (*zc && (out_iov || out_sg)) {
+		if (out_iov)
+			n_sgout = iov_iter_npages(out_iov, INT_MAX) + 1;
+		else if (out_sg)
+			n_sgout = sg_nents(out_sg);
+		else
+			goto no_zerocopy;
+
+		n_sgin = skb_nsg(skb, rxm->offset + tls_ctx->rx.prepend_size,
+				 rxm->full_len - tls_ctx->rx.prepend_size);
+	} else {
+no_zerocopy:
+		n_sgout = 0;
+		*zc = false;
+		n_sgin = skb_cow_data(skb, 0, &unused);
+	}
+
+	if (n_sgin < 1)
+		return -EBADMSG;
+
+	/* Increment to accommodate AAD */
+	n_sgin = n_sgin + 1;
+
+	nsg = n_sgin + n_sgout;
+
+	aead_size = sizeof(*aead_req) + crypto_aead_reqsize(ctx->aead_recv);
+	mem_size = aead_size + (nsg * sizeof(struct scatterlist));
+	mem_size = mem_size + TLS_AAD_SPACE_SIZE;
+	mem_size = mem_size + crypto_aead_ivsize(ctx->aead_recv);
+
+	/* Allocate a single block of memory which contains
+	 * aead_req || sgin[] || sgout[] || aad || iv.
+	 * This order achieves correct alignment for aead_req, sgin, sgout.
+	 */
+	mem = kmalloc(mem_size, sk->sk_allocation);
+	if (!mem)
+		return -ENOMEM;
+
+	/* Segment the allocated memory */
+	aead_req = (struct aead_request *)mem;
+	sgin = (struct scatterlist *)(mem + aead_size);
+	sgout = sgin + n_sgin;
+	aad = (u8 *)(sgout + n_sgout);
+	iv = aad + TLS_AAD_SPACE_SIZE;
+
+	/* Prepare IV */
+	err = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
+			    iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
+			    tls_ctx->rx.iv_size);
+	if (err < 0) {
+		kfree(mem);
+		return err;
+	}
+	memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
+
+	/* Prepare AAD */
+	tls_make_aad(aad, rxm->full_len - tls_ctx->rx.overhead_size,
+		     tls_ctx->rx.rec_seq, tls_ctx->rx.rec_seq_size,
+		     ctx->control);
+
+	/* Prepare sgin */
+	sg_init_table(sgin, n_sgin);
+	sg_set_buf(&sgin[0], aad, TLS_AAD_SPACE_SIZE);
+	err = skb_to_sgvec(skb, &sgin[1],
+			   rxm->offset + tls_ctx->rx.prepend_size,
+			   rxm->full_len - tls_ctx->rx.prepend_size);
+	if (err < 0) {
+		kfree(mem);
+		return err;
+	}
+
+	if (n_sgout) {
+		if (out_iov) {
+			sg_init_table(sgout, n_sgout);
+			sg_set_buf(&sgout[0], aad, TLS_AAD_SPACE_SIZE);
+
+			*chunk = 0;
+			err = zerocopy_from_iter(sk, out_iov, data_len, &pages,
+						 chunk, &sgout[1],
+						 (n_sgout - 1), false);
+			if (err < 0)
+				goto fallback_to_reg_recv;
+		} else if (out_sg) {
+			memcpy(sgout, out_sg, n_sgout * sizeof(*sgout));
+		} else {
+			goto fallback_to_reg_recv;
+		}
+	} else {
+fallback_to_reg_recv:
+		sgout = sgin;
+		pages = 0;
+		*chunk = 0;
+		*zc = false;
+	}
+
+	/* Prepare and submit AEAD request */
+	err = tls_do_decryption(sk, sgin, sgout, iv, data_len, aead_req);
+
+	/* Release the pages in case iov was mapped to pages */
+	for (; pages > 0; pages--)
+		put_page(sg_page(&sgout[pages]));
+
+	kfree(mem);
+	return err;
+}
+
 static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
-			      struct scatterlist *sgout, bool *zc)
+			      struct iov_iter *dest, int *chunk, bool *zc)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
@@ -741,7 +863,7 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 		return err;
 #endif
 	if (!ctx->decrypted) {
-		err = decrypt_skb(sk, skb, sgout);
+		err = decrypt_internal(sk, skb, dest, NULL, chunk, zc);
 		if (err < 0)
 			return err;
 	} else {
@@ -760,67 +882,10 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
 int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 		struct scatterlist *sgout)
 {
-	struct tls_context *tls_ctx = tls_get_ctx(sk);
-	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
-	char iv[TLS_CIPHER_AES_GCM_128_SALT_SIZE + MAX_IV_SIZE];
-	struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
-	struct scatterlist *sgin = &sgin_arr[0];
-	struct strp_msg *rxm = strp_msg(skb);
-	int ret, nsg;
-	struct sk_buff *unused;
-
-	ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
-			    iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
-			    tls_ctx->rx.iv_size);
-	if (ret < 0)
-		return ret;
-
-	memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
-	if (!sgout) {
-		nsg = skb_cow_data(skb, 0, &unused);
-	} else {
-		nsg = skb_nsg(skb,
-			      rxm->offset + tls_ctx->rx.prepend_size,
-			      rxm->full_len - tls_ctx->rx.prepend_size);
-		if (nsg <= 0)
-			return nsg;
-	}
-
-	// We need one extra for ctx->rx_aad_ciphertext
-	nsg++;
-
-	if (nsg > ARRAY_SIZE(sgin_arr))
-		sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
-
-	if (!sgout)
-		sgout = sgin;
-
-	sg_init_table(sgin, nsg);
-	sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
-
-	nsg = skb_to_sgvec(skb, &sgin[1],
-			   rxm->offset + tls_ctx->rx.prepend_size,
-			   rxm->full_len - tls_ctx->rx.prepend_size);
-	if (nsg < 0) {
-		ret = nsg;
-		goto out;
-	}
-
-	tls_make_aad(ctx->rx_aad_ciphertext,
-		     rxm->full_len - tls_ctx->rx.overhead_size,
-		     tls_ctx->rx.rec_seq,
-		     tls_ctx->rx.rec_seq_size,
-		     ctx->control);
-
-	ret = tls_do_decryption(sk, sgin, sgout, iv,
-				rxm->full_len - tls_ctx->rx.overhead_size,
-				skb, sk->sk_allocation);
-
-out:
-	if (sgin != &sgin_arr[0])
-		kfree(sgin);
+	bool zc = true;
+	int chunk;
 
-	return ret;
+	return decrypt_internal(sk, skb, NULL, sgout, &chunk, &zc);
 }
 
 static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
@@ -899,43 +964,17 @@ int tls_sw_recvmsg(struct sock *sk,
 		}
 
 		if (!ctx->decrypted) {
-			int page_count;
-			int to_copy;
-
-			page_count = iov_iter_npages(&msg->msg_iter,
-						     MAX_SKB_FRAGS);
-			to_copy = rxm->full_len - tls_ctx->rx.overhead_size;
-			if (!is_kvec && to_copy <= len && page_count < MAX_SKB_FRAGS &&
-			    likely(!(flags & MSG_PEEK)))  {
-				struct scatterlist sgin[MAX_SKB_FRAGS + 1];
-				int pages = 0;
+			int to_copy = rxm->full_len - tls_ctx->rx.overhead_size;
 
+			if (!is_kvec && to_copy <= len &&
+			    likely(!(flags & MSG_PEEK)))
 				zc = true;
-				sg_init_table(sgin, MAX_SKB_FRAGS + 1);
-				sg_set_buf(&sgin[0], ctx->rx_aad_plaintext,
-					   TLS_AAD_SPACE_SIZE);
-
-				err = zerocopy_from_iter(sk, &msg->msg_iter,
-							 to_copy, &pages,
-							 &chunk, &sgin[1],
-							 MAX_SKB_FRAGS,	false);
-				if (err < 0)
-					goto fallback_to_reg_recv;
-
-				err = decrypt_skb_update(sk, skb, sgin, &zc);
-				for (; pages > 0; pages--)
-					put_page(sg_page(&sgin[pages]));
-				if (err < 0) {
-					tls_err_abort(sk, EBADMSG);
-					goto recv_end;
-				}
-			} else {
-fallback_to_reg_recv:
-				err = decrypt_skb_update(sk, skb, NULL, &zc);
-				if (err < 0) {
-					tls_err_abort(sk, EBADMSG);
-					goto recv_end;
-				}
+
+			err = decrypt_skb_update(sk, skb, &msg->msg_iter,
+						 &chunk, &zc);
+			if (err < 0) {
+				tls_err_abort(sk, EBADMSG);
+				goto recv_end;
 			}
 			ctx->decrypted = true;
 		}
@@ -986,7 +1025,7 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	int err = 0;
 	long timeo;
 	int chunk;
-	bool zc;
+	bool zc = false;
 
 	lock_sock(sk);
 
@@ -1003,7 +1042,7 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	}
 
 	if (!ctx->decrypted) {
-		err = decrypt_skb_update(sk, skb, NULL, &zc);
+		err = decrypt_skb_update(sk, skb, NULL, &chunk, &zc);
 
 		if (err < 0) {
 			tls_err_abort(sk, EBADMSG);
-- 
2.13.6

^ permalink raw reply related

* [PATCH RFC net-next 0/1] net/tls: Combined memory allocation for decryption request
From: Vakul Garg @ 2018-08-06 16:38 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg

This patch does a combined memory allocation from heap for scatterlists,
aead_request, aad and iv for the tls record decryption path. In present
code, aead_request is allocated from heap, scatterlists on a conditional
basis are allocated on heap or on stack. This is inefficient as it may
requires multiple kmalloc/kfree.

The initialization vector passed in cryption request is allocated on
stack. This is a problem since the stack memory is not dma-able from
crypto accelerators.

Doing one combined memory allocation for each decryption request fixes
both the above issues. It also paves a way to be able to submit multiple
async decryption requests while the previous one is pending i.e. being 
processed or queued.

This patch has been built over Doron Roberts-Kedes's patch:

"net/tls: Calculate nsg for zerocopy path without skb_cow_data"


Vakul Garg (1):
  net/tls: Combined memory allocation for decryption request

 include/net/tls.h |   4 -
 net/tls/tls_sw.c  | 256 +++++++++++++++++++++++++++++++-----------------------
 2 files changed, 147 insertions(+), 113 deletions(-)

-- 
2.13.6

^ permalink raw reply

* Re: [PATCH] Use Kconfig flag to remove support of deprecated BE2/BE3 adapters
From: Ivan Vecera @ 2018-08-06 13:03 UTC (permalink / raw)
  To: Petr Oros, netdev
  Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
	David S. Miller, linux-kernel
In-Reply-To: <20180806121230.23690-1-poros@redhat.com>

On 6.8.2018 14:12, Petr Oros wrote:
>   Add flags to remove support of deprecated BE2/BE3 adapters.
>   BE2 disable will reduce .ko size by 2kb and BE3 by 3kb.
>   Disable both will reduce .ko size by 9kb.
> 
>   With dissabled support is also removed coresponding PCI IDs
>   and codepath with [BE2|BE3|BEx]_chip checks.
> 
>   New help style in Kconfig
> 
> Signed-off-by: Petr Oros <poros@redhat.com>

Note that this should be placed to -next...

Reviewed-by: Ivan Vecera <ivecera@redhat.com>

> ---
>  drivers/net/ethernet/emulex/benet/Kconfig   | 20 ++++++++++++++++++--
>  drivers/net/ethernet/emulex/benet/be.h      |  8 ++++++++
>  drivers/net/ethernet/emulex/benet/be_main.c |  6 +++++-
>  3 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/emulex/benet/Kconfig b/drivers/net/ethernet/emulex/benet/Kconfig
> index b4853ec9de8d..68a9c40adc2a 100644
> --- a/drivers/net/ethernet/emulex/benet/Kconfig
> +++ b/drivers/net/ethernet/emulex/benet/Kconfig
> @@ -1,7 +1,7 @@
>  config BE2NET
>  	tristate "ServerEngines' 10Gbps NIC - BladeEngine"
>  	depends on PCI
> -	---help---
> +	help
>  	  This driver implements the NIC functionality for ServerEngines'
>  	  10Gbps network adapter - BladeEngine.
>  
> @@ -10,6 +10,22 @@ config BE2NET_HWMON
>  	depends on BE2NET && HWMON
>  	depends on !(BE2NET=y && HWMON=m)
>  	default y
> -	---help---
> +	help
>  	  Say Y here if you want to expose thermal sensor data on
>  	  be2net network adapter.
> +
> +config BE2NET_BE2
> +	bool "Support for deprecated BE2 chipsets"
> +	depends on BE2NET
> +	default y
> +	help
> +	  Say Y here if you want to use deprecated Emulex devices based
> +	  on BE2 chipsets.
> +
> +config BE2NET_BE3
> +	bool "Support for deprecated BE3 chipsets"
> +	depends on BE2NET
> +	default y
> +	help
> +	  Say Y here if you want to use deprecated Emulex devices based
> +	  on BE3 chipsets.
> diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
> index 382891f81e09..3038578ec7a7 100644
> --- a/drivers/net/ethernet/emulex/benet/be.h
> +++ b/drivers/net/ethernet/emulex/benet/be.h
> @@ -779,11 +779,19 @@ static inline u16 be_max_any_irqs(struct be_adapter *adapter)
>  #define skyhawk_chip(adapter)	(adapter->pdev->device == OC_DEVICE_ID5 || \
>  				 adapter->pdev->device == OC_DEVICE_ID6)
>  
> +#ifdef CONFIG_BE2NET_BE3
>  #define BE3_chip(adapter)	(adapter->pdev->device == BE_DEVICE_ID2 || \
>  				 adapter->pdev->device == OC_DEVICE_ID2)
> +#else
> +#define BE3_chip(adapter)	(0)
> +#endif /* CONFIG_BE2NET_BE3 */
>  
> +#ifdef CONFIG_BE2NET_BE2
>  #define BE2_chip(adapter)	(adapter->pdev->device == BE_DEVICE_ID1 || \
>  				 adapter->pdev->device == OC_DEVICE_ID1)
> +#else
> +#define BE2_chip(adapter)	(0)
> +#endif /* CONFIG_BE2NET_BE2 */
>  
>  #define BEx_chip(adapter)	(BE3_chip(adapter) || BE2_chip(adapter))
>  
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 8f755009ff38..d5b3f0139832 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -47,10 +47,14 @@ MODULE_PARM_DESC(rx_frag_size, "Size of a fragment that holds rcvd data.");
>  static struct workqueue_struct *be_err_recovery_workq;
>  
>  static const struct pci_device_id be_dev_ids[] = {
> +#ifdef CONFIG_BE2NET_BE2
>  	{ PCI_DEVICE(BE_VENDOR_ID, BE_DEVICE_ID1) },
> -	{ PCI_DEVICE(BE_VENDOR_ID, BE_DEVICE_ID2) },
>  	{ PCI_DEVICE(BE_VENDOR_ID, OC_DEVICE_ID1) },
> +#endif /* CONFIG_BE2NET_BE2 */
> +#ifdef CONFIG_BE2NET_BE3
> +	{ PCI_DEVICE(BE_VENDOR_ID, BE_DEVICE_ID2) },
>  	{ PCI_DEVICE(BE_VENDOR_ID, OC_DEVICE_ID2) },
> +#endif /* CONFIG_BE2NET_BE3 */
>  	{ PCI_DEVICE(EMULEX_VENDOR_ID, OC_DEVICE_ID3)},
>  	{ PCI_DEVICE(EMULEX_VENDOR_ID, OC_DEVICE_ID4)},
>  	{ PCI_DEVICE(EMULEX_VENDOR_ID, OC_DEVICE_ID5)},
> 

^ permalink raw reply

* Re: [PATCH 07/10] dt-bindings: phy: add DT binding for Microsemi Ocelot SerDes muxing
From: Quentin Schulz @ 2018-08-06 12:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
	mark.rutland, davem, kishon, f.fainelli, linux-mips, devicetree,
	linux-kernel, netdev, allan.nielsen, thomas.petazzoni
In-Reply-To: <20180801143147.GA16322@lunn.ch>

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

Hi Andrew,

On Wed, Aug 01, 2018 at 04:31:47PM +0200, Andrew Lunn wrote:
> > > Maybe this should be serdes-mux? The SERDES itself should have some
> > > registers somewhere. If you ever decide to make use of phylink,
> > > e.g. to support SFP, you are going to need to know if the SERDES is
> > > up. So you might need to add the actual SERDES device, in addition to
> > > the mux for the SERDES.
> > > 
> > 
> > I'm not sure to follow.
> > 
> > To be honest, I might have mislead you. The whole configuration of the
> > serdes is in the hsio register address space. For now, muxing is the
> > only reason there is a driver for the serdes but there are other things
> > that can be configured (though not used yet): de/serializer, input/output
> > buffers, PLL, ... configuration registers for the SerDes.
> 
> When you are using the SERDES for networking, you need to know if the
> SERDES has achieved sync. For example, when the SERDES connects to an
> optical SFP module, the SERDES bit stream continues unmodified over
> the optical link to the SERDES in the peer. The optical module can
> tell you if it is receiving optical power, but it cannot tell you if
> the optical signal makes any sense. The SERDES however knows how to
> decode the bitstream, sync to it, etc. So you need some registers in
> the SERDES to get this status information. Typically, you can also get
> access to the SGMII/1000Base-X code word, so you can do
> auto-negotiation, or know if you need to send each bit 10 or 100 times
> in order to do 100Mbps or 10Mbps. If you are connecting to a PHY which
> can do > 1Gbps, you need to change the SERDES between SGMII,
> 1000Base-X, 2500Base-X, etc. Before you can say the link is up, you
> want the PHY to tell you it has link to its peer PHY, and you want to
> know the SERDES is ready. Typically the SERDES is last, since you
> don't know what to configure the SERDES to until the PHY is finished
> negotiating the link to its peer.
> 
> If you look at any of the Marvell SERDES interfaces, found in PHYs or
> switches, there are dozens of registers for controlling the SERDES.
> 
> Now, it could be we don't have a clear definition of what a SERDES
> is. The Marvell documents has a lot in its definition of SERDES, where
> as what you could be purely a 'dumb' parallel to serial convert, and
> all the rest of the logic is in the Ethernet MAC and the PCIe device?
> 
> Now, back to my original point. Where are the registers for 'the rest
> of this logic'? If they are in the MAC address space, we don't have a
> problem. If they are somewhere else, maybe you will need to add
> another device. What is this device called? That is why i'm trying to
> differentiate between the 'SERDES-MUX' and the 'SERDES'.
> 

If I've correctly read the datasheet of the switch, the sync status bit
is in the MAC address space. Same for 1000BASE-X/SGMII, autonegotiation.

The point I was trying to make was that this driver isn't only for
"muxing". There are also a handful of registers for
(electronically-related ?) features of SerDes (e.g. "control of phase
regulator logic", "deserializer phase control", a few
thresholds/hysteresis/frequencies, etc...

I understand that "serdes" isn't also fully matching the work done by
this driver as some features are handled within the MAC controller
address space.

Let me know if something bothers you/does not make sense,
Thanks,
Quentin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v6 10/18] x86/power/64: Remove VLA usage
From: Rafael J. Wysocki @ 2018-08-06 10:28 UTC (permalink / raw)
  To: Kees Cook
  Cc: Rafael J. Wysocki, Herbert Xu, Arnd Bergmann, Eric Biggers,
	Gustavo A. R. Silva, Alasdair Kergon, Rabin Vincent, Tim Chen,
	Pavel Machek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	the arch/x86 maintainers, Philipp Reisner, Lars Ellenberg,
	Jens Axboe, Giovanni Cabiddu, Mike Snitzer, Paul Mackerras,
	Greg Kroah-Hartman <gregkh@
In-Reply-To: <CAGXu5j+NPCc2hq5x7JUwgKYvY6zKN28YdtHbHVapnpgC3GwCsg@mail.gmail.com>

On Wednesday, July 25, 2018 8:01:47 PM CEST Kees Cook wrote:
> On Wed, Jul 25, 2018 at 4:32 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Tue, Jul 24, 2018 at 6:49 PM, Kees Cook <keescook@chromium.org> wrote:
> >> In the quest to remove all stack VLA usage from the kernel[1], this
> >> removes the discouraged use of AHASH_REQUEST_ON_STACK by switching to
> >> shash directly and allocating the descriptor in heap memory (which should
> >> be fine: the tfm has already been allocated there too).
> >>
> >> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
> >>
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >> Acked-by: Pavel Machek <pavel@ucw.cz>
> >
> > I think I can queue this up if there are no objections from others.
> >
> > Do you want me to do that?
> 
> Sure thing. It looks like the other stand-alone patches like this one
> are getting taken into the non-crypto trees, so that's fine.
> 

Applied now, thanks!

^ permalink raw reply

* [PATCH] Use Kconfig flag to remove support of deprecated BE2/BE3 adapters
From: Petr Oros @ 2018-08-06 12:12 UTC (permalink / raw)
  To: netdev
  Cc: ivecera, Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna,
	Somnath Kotur, David S. Miller, linux-kernel

  Add flags to remove support of deprecated BE2/BE3 adapters.
  BE2 disable will reduce .ko size by 2kb and BE3 by 3kb.
  Disable both will reduce .ko size by 9kb.

  With dissabled support is also removed coresponding PCI IDs
  and codepath with [BE2|BE3|BEx]_chip checks.

  New help style in Kconfig

Signed-off-by: Petr Oros <poros@redhat.com>
---
 drivers/net/ethernet/emulex/benet/Kconfig   | 20 ++++++++++++++++++--
 drivers/net/ethernet/emulex/benet/be.h      |  8 ++++++++
 drivers/net/ethernet/emulex/benet/be_main.c |  6 +++++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/Kconfig b/drivers/net/ethernet/emulex/benet/Kconfig
index b4853ec9de8d..68a9c40adc2a 100644
--- a/drivers/net/ethernet/emulex/benet/Kconfig
+++ b/drivers/net/ethernet/emulex/benet/Kconfig
@@ -1,7 +1,7 @@
 config BE2NET
 	tristate "ServerEngines' 10Gbps NIC - BladeEngine"
 	depends on PCI
-	---help---
+	help
 	  This driver implements the NIC functionality for ServerEngines'
 	  10Gbps network adapter - BladeEngine.
 
@@ -10,6 +10,22 @@ config BE2NET_HWMON
 	depends on BE2NET && HWMON
 	depends on !(BE2NET=y && HWMON=m)
 	default y
-	---help---
+	help
 	  Say Y here if you want to expose thermal sensor data on
 	  be2net network adapter.
+
+config BE2NET_BE2
+	bool "Support for deprecated BE2 chipsets"
+	depends on BE2NET
+	default y
+	help
+	  Say Y here if you want to use deprecated Emulex devices based
+	  on BE2 chipsets.
+
+config BE2NET_BE3
+	bool "Support for deprecated BE3 chipsets"
+	depends on BE2NET
+	default y
+	help
+	  Say Y here if you want to use deprecated Emulex devices based
+	  on BE3 chipsets.
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 382891f81e09..3038578ec7a7 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -779,11 +779,19 @@ static inline u16 be_max_any_irqs(struct be_adapter *adapter)
 #define skyhawk_chip(adapter)	(adapter->pdev->device == OC_DEVICE_ID5 || \
 				 adapter->pdev->device == OC_DEVICE_ID6)
 
+#ifdef CONFIG_BE2NET_BE3
 #define BE3_chip(adapter)	(adapter->pdev->device == BE_DEVICE_ID2 || \
 				 adapter->pdev->device == OC_DEVICE_ID2)
+#else
+#define BE3_chip(adapter)	(0)
+#endif /* CONFIG_BE2NET_BE3 */
 
+#ifdef CONFIG_BE2NET_BE2
 #define BE2_chip(adapter)	(adapter->pdev->device == BE_DEVICE_ID1 || \
 				 adapter->pdev->device == OC_DEVICE_ID1)
+#else
+#define BE2_chip(adapter)	(0)
+#endif /* CONFIG_BE2NET_BE2 */
 
 #define BEx_chip(adapter)	(BE3_chip(adapter) || BE2_chip(adapter))
 
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 8f755009ff38..d5b3f0139832 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -47,10 +47,14 @@ MODULE_PARM_DESC(rx_frag_size, "Size of a fragment that holds rcvd data.");
 static struct workqueue_struct *be_err_recovery_workq;
 
 static const struct pci_device_id be_dev_ids[] = {
+#ifdef CONFIG_BE2NET_BE2
 	{ PCI_DEVICE(BE_VENDOR_ID, BE_DEVICE_ID1) },
-	{ PCI_DEVICE(BE_VENDOR_ID, BE_DEVICE_ID2) },
 	{ PCI_DEVICE(BE_VENDOR_ID, OC_DEVICE_ID1) },
+#endif /* CONFIG_BE2NET_BE2 */
+#ifdef CONFIG_BE2NET_BE3
+	{ PCI_DEVICE(BE_VENDOR_ID, BE_DEVICE_ID2) },
 	{ PCI_DEVICE(BE_VENDOR_ID, OC_DEVICE_ID2) },
+#endif /* CONFIG_BE2NET_BE3 */
 	{ PCI_DEVICE(EMULEX_VENDOR_ID, OC_DEVICE_ID3)},
 	{ PCI_DEVICE(EMULEX_VENDOR_ID, OC_DEVICE_ID4)},
 	{ PCI_DEVICE(EMULEX_VENDOR_ID, OC_DEVICE_ID5)},
-- 
2.16.4

^ permalink raw reply related

* Re: [PATCH] can: sja1000: plx_pci: add support for ASEM CAN raw device
From: Marc Kleine-Budde @ 2018-08-06 12:05 UTC (permalink / raw)
  To: Flavio Suligoi, Wolfgang Grandegger
  Cc: David S . Miller, linux-can, netdev, linux-kernel
In-Reply-To: <1533556850-4297-1-git-send-email-f.suligoi@asem.it>


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

On 08/06/2018 02:00 PM, Flavio Suligoi wrote:
> This patch adds support for ASEM opto-isolated dual channels
> CAN raw device (http://www.asem.it)
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  drivers/net/can/sja1000/Kconfig   |  1 +
>  drivers/net/can/sja1000/plx_pci.c | 68 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/sja1000/Kconfig b/drivers/net/can/sja1000/Kconfig
> index 1e65cb6..f6dc899 100644
> --- a/drivers/net/can/sja1000/Kconfig
> +++ b/drivers/net/can/sja1000/Kconfig
> @@ -88,6 +88,7 @@ config CAN_PLX_PCI
>  	   - TEWS TECHNOLOGIES TPMC810 card (http://www.tews.com/)
>  	   - IXXAT Automation PC-I 04/PCI card (http://www.ixxat.com/)
>  	   - Connect Tech Inc. CANpro/104-Plus Opto (CRG001) card (http://www.connecttech.com)
> +	   - ASEM CAN raw - 2 isolated CAN channels (www.asem.it)
>  
>  config CAN_TSCAN1
>  	tristate "TS-CAN1 PC104 boards"
> diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
> index f8ff25c..864fe8d 100644
> --- a/drivers/net/can/sja1000/plx_pci.c
> +++ b/drivers/net/can/sja1000/plx_pci.c
> @@ -46,7 +46,8 @@ MODULE_SUPPORTED_DEVICE("Adlink PCI-7841/cPCI-7841, "
>  			"esd CAN-PCIe/2000, "
>  			"Connect Tech Inc. CANpro/104-Plus Opto (CRG001), "
>  			"IXXAT PC-I 04/PCI, "
> -			"ELCUS CAN-200-PCI")
> +			"ELCUS CAN-200-PCI, "
> +			"ASEM DUAL CAN-RAW")
>  MODULE_LICENSE("GPL v2");
>  
>  #define PLX_PCI_MAX_CHAN 2
> @@ -70,7 +71,9 @@ struct plx_pci_card {
>  					 */
>  
>  #define PLX_LINT1_EN	0x1		/* Local interrupt 1 enable */
> +#define PLX_LINT1_POL	(1 << 1)	/* Local interrupt 1 polarity */
>  #define PLX_LINT2_EN	(1 << 3)	/* Local interrupt 2 enable */
> +#define PLX_LINT2_POL	(1 << 4)	/* Local interrupt 1 polarity */
                                                                  ^^^ 2?
>  #define PLX_PCI_INT_EN	(1 << 6)	/* PCI Interrupt Enable */
>  #define PLX_PCI_RESET	(1 << 30)	/* PCI Adapter Software Reset */
>  
> @@ -92,6 +95,9 @@ struct plx_pci_card {
>   */
>  #define PLX_PCI_OCR	(OCR_TX0_PUSHPULL | OCR_TX1_PUSHPULL)
>  
> +/* OCR setting for ASEM Dual CAN raw */
> +#define ASEM_PCI_OCR	0xfe
> +
>  /*
>   * In the CDR register, you should set CBP to 1.
>   * You will probably also want to set the clock divider value to 7
> @@ -145,10 +151,20 @@ struct plx_pci_card {
>  #define MOXA_PCI_VENDOR_ID		0x1393
>  #define MOXA_PCI_DEVICE_ID		0x0100
>  
> +#define ASEM_DUAL_CAN_VENDOR_ID		0x10b5
> +#define ASEM_DUAL_CAN_DEVICE_ID		0x9030
> +#define ASEM_DUAL_CAN_SUB_VENDOR_ID	0x3000
> +#define ASEM_DUAL_CAN_SUB_DEVICE_ID	0x1001
> +#define ASEM_DUAL_CAN_SUB_DEVICE_ID_BIS	0x1002
> +#define ASEM_DUAL_CAN_RESET_REGISTER	0x54
> +#define ASEM_DUAL_CAN_RESET_MASK_CAN1	0x20
> +#define ASEM_DUAL_CAN_RESET_MASK_CAN2	0x04
> +
>  static void plx_pci_reset_common(struct pci_dev *pdev);
>  static void plx9056_pci_reset_common(struct pci_dev *pdev);
>  static void plx_pci_reset_marathon_pci(struct pci_dev *pdev);
>  static void plx_pci_reset_marathon_pcie(struct pci_dev *pdev);
> +static void plx_pci_reset_asem_dual_can_raw(struct pci_dev *pdev);
>  
>  struct plx_pci_channel_map {
>  	u32 bar;
> @@ -269,6 +285,14 @@ static struct plx_pci_card_info plx_pci_card_info_moxa = {
>  	 /* based on PLX9052 */
>  };
>  
> +static struct plx_pci_card_info plx_pci_card_info_asem_dual_can = {
> +	"ASEM Dual CAN raw PCI", 2,
> +	PLX_PCI_CAN_CLOCK, ASEM_PCI_OCR, PLX_PCI_CDR,
> +	{0, 0x00, 0x00}, { {2, 0x00, 0x00}, {4, 0x00, 0x00} },
> +	&plx_pci_reset_asem_dual_can_raw
> +	/* based on PLX9030 */
> +};
> +
>  static const struct pci_device_id plx_pci_tbl[] = {
>  	{
>  		/* Adlink PCI-7841/cPCI-7841 */
> @@ -375,6 +399,20 @@ static const struct pci_device_id plx_pci_tbl[] = {
>  		0, 0,
>  		(kernel_ulong_t)&plx_pci_card_info_moxa
>  	},
> +	{
> +		/* ASEM Dual CAN raw */
> +		ASEM_DUAL_CAN_VENDOR_ID, ASEM_DUAL_CAN_DEVICE_ID,
> +		ASEM_DUAL_CAN_SUB_VENDOR_ID, ASEM_DUAL_CAN_SUB_DEVICE_ID,
> +		0, 0,
> +		(kernel_ulong_t)&plx_pci_card_info_asem_dual_can
> +	},
> +	{
> +		/* ASEM Dual CAN raw -new model */
> +		ASEM_DUAL_CAN_VENDOR_ID, ASEM_DUAL_CAN_DEVICE_ID,
> +		ASEM_DUAL_CAN_SUB_VENDOR_ID, ASEM_DUAL_CAN_SUB_DEVICE_ID_BIS,
> +		0, 0,
> +		(kernel_ulong_t)&plx_pci_card_info_asem_dual_can
> +	},
>  	{ 0,}
>  };
>  MODULE_DEVICE_TABLE(pci, plx_pci_tbl);
> @@ -524,6 +562,34 @@ static void plx_pci_reset_marathon_pcie(struct pci_dev *pdev)
>  	}
>  }
>  
> +/* Special reset function for ASEM Dual CAN raw card */
> +static void plx_pci_reset_asem_dual_can_raw(struct pci_dev *pdev)
> +{
> +	void __iomem *bar0_addr;
> +	static const int reset_bar;
> +	u8 tmpval;
> +
> +	plx_pci_reset_common(pdev);
> +
> +	bar0_addr = pci_iomap(pdev, reset_bar, 0);
> +	if (!bar0_addr) {
> +		dev_err(&pdev->dev, "Failed to remap reset "
> +		"space %d (BAR%d)\n", 0, reset_bar);

please don't break strings

just return here, so you can remove the indention for the else...

> +	} else {
> +		/* reset the two SJA1000 chips */
> +		tmpval = ioread8(bar0_addr + ASEM_DUAL_CAN_RESET_REGISTER);
> +		tmpval &= ~(ASEM_DUAL_CAN_RESET_MASK_CAN1 |
> +			ASEM_DUAL_CAN_RESET_MASK_CAN2);
> +		iowrite8(tmpval, bar0_addr + ASEM_DUAL_CAN_RESET_REGISTER);
> +		usleep_range(300, 400);
> +		tmpval |= ASEM_DUAL_CAN_RESET_MASK_CAN1 |
> +			ASEM_DUAL_CAN_RESET_MASK_CAN2;
> +		iowrite8(tmpval, bar0_addr + ASEM_DUAL_CAN_RESET_REGISTER);
> +		usleep_range(300, 400);
> +		pci_iounmap(pdev, bar0_addr);
> +	}
> +}
> +
>  static void plx_pci_del_card(struct pci_dev *pdev)
>  {
>  	struct plx_pci_card *card = pci_get_drvdata(pdev);
> 

Marc

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


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

^ permalink raw reply

* [PATCH net-next] tcp: fix the calculation of sysctl_max_tw_buckets in tcp_sk_init()
From: Yafang Shao @ 2018-08-06 11:47 UTC (permalink / raw)
  To: edumazet, davem, yanhaishuang; +Cc: netdev, linux-kernel, Yafang Shao

tcp_hashinfo.ehash_mask is always an odd number, which is set in function
alloc_large_system_hash(). See bellow,
	if (_hash_mask)
		*_hash_mask = (1 << log2qty) - 1; <<< always odd number

Hence the local variable 'cnt' is a even number, as a result of that it is
no difference to do the incrementation here.

Maybe the compiler could also optimize it, but this code is a little ugly.

Fix: fee83d09 ("ipv4: Namespaceify tcp_max_syn_backlog knob")
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/ipv4/tcp_ipv4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9e041fa..a9b7c4b 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2543,7 +2543,7 @@ static int __net_init tcp_sk_init(struct net *net)
 	net->ipv4.sysctl_tcp_tw_reuse = 2;
 
 	cnt = tcp_hashinfo.ehash_mask + 1;
-	net->ipv4.tcp_death_row.sysctl_max_tw_buckets = (cnt + 1) / 2;
+	net->ipv4.tcp_death_row.sysctl_max_tw_buckets = cnt / 2;
 	net->ipv4.tcp_death_row.hashinfo = &tcp_hashinfo;
 
 	net->ipv4.sysctl_max_syn_backlog = max(128, cnt / 256);
-- 
1.8.3.1

^ permalink raw reply related

* RE: [PATCH v2 0/2] net/sctp: Avoid allocating high order memory with kmalloc()
From: David Laight @ 2018-08-06  9:34 UTC (permalink / raw)
  To: 'Michael Tuexen', Marcelo Ricardo Leitner
  Cc: Konstantin Khorenko, oleg.babin@gmail.com, netdev@vger.kernel.org,
	linux-sctp@vger.kernel.org, David S . Miller, Vlad Yasevich,
	Neil Horman, Xin Long, Andrey Ryabinin
In-Reply-To: <2D7531CF-9E0A-4B33-83E5-724B7C88F363@lurchi.franken.de>

From: Michael Tuexen
> Sent: 03 August 2018 21:57
...
> >> Given how useless SCTP streams are, does anything actually use
> >> more than about 4?
> >
> > Maybe Michael can help us with that. I'm also curious now.
> In the context of SIGTRAN I have seen 17 streams...

Ok, I've seen 17 there as well, 5 is probably more common.

> In the context of WebRTC I have seen more streams. In general,
> the streams concept seems to be useful. QUIC has lots of streams.
> 
> So I'm wondering why they are considered useless.
> David, can you elaborate on this?

I don't think a lot of people know what they actually are.

Streams just allow some receive data to forwarded to applications when receive
message(s) on stream(s) are lost and have to be retransmitted.

I suspect some people think that the separate streams have separate flow control,
not just separate data sequences.

M2PA separates control message (stream 0) from user data (stream 1).
I think the spec even suggests this is so control messages get through when
user data is flow controlled off - not true (it would be true for ISO
transport's 'expedited data).

M3UA will use 16 streams (one for each (ITU) SLS), but uses stream 0 for control.
If a data message is lost then data for the other sls can be passed to the
userpart/mtp3 - this might save bursty processing when the SACK-requested
retransmission arrives. But I doubt you'd want to run M3UA on anything lossy
enough for more than 4 data streams to make sense.

Even M3UA separating control onto stream 0 data onto 1-n doesn't seem useful to me.

If QUIC is using 'lots of streams' is it just using the stream-id as a qualifier
for the data? Rather than requiring the 'not head of line blocking' feature
of sctp streams?

Thought....
Could we let the application set large stream-ids, but actually mask them
down to (say) 32 for the protocol code?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH wpan 2/2] net: mac802154: tx: expand tailroom if necessary
From: Stefan Schmidt @ 2018-08-06  9:26 UTC (permalink / raw)
  To: Alexander Aring, stefan; +Cc: linux-wpan, netdev, kernel
In-Reply-To: <20180702203203.21316-2-aring@mojatatu.com>

Hello.

On 07/02/2018 10:32 PM, Alexander Aring wrote:
> This patch is necessary if case of AF_PACKET or other socket interface
> which I am aware of it and didn't allocated the necessary room.
> 
> Reported-by: David Palma <david.palma@ntnu.no>
> Reported-by: Rabi Narayan Sahoo <rabinarayans0828@gmail.com>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
>  net/mac802154/tx.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
> index 7e253455f9dd..bcd1a5e6ebf4 100644
> --- a/net/mac802154/tx.c
> +++ b/net/mac802154/tx.c
> @@ -63,8 +63,21 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb)
>  	int ret;
>  
>  	if (!(local->hw.flags & IEEE802154_HW_TX_OMIT_CKSUM)) {
> -		u16 crc = crc_ccitt(0, skb->data, skb->len);
> +		struct sk_buff *nskb;
> +		u16 crc;
> +
> +		if (unlikely(skb_tailroom(skb) < IEEE802154_FCS_LEN)) {
> +			nskb = skb_copy_expand(skb, 0, IEEE802154_FCS_LEN,
> +					       GFP_ATOMIC);
> +			if (likely(nskb)) {
> +				consume_skb(skb);
> +				skb = nskb;
> +			} else {
> +				goto err_tx;
> +			}
> +		}
>  
> +		crc = crc_ccitt(0, skb->data, skb->len);
>  		put_unaligned_le16(crc, skb_put(skb, 2));
>  	}
>  
> 

This patch has been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

I know you submitted this for wpan instead of wpan-next, but with rc8
being out I will not submit another pull request for 4.18. Instead I
added cc stable to the patch to make sure it gets picked into the stable
tree once 4.18 is out.

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCHv2 wpan] net: 6lowpan: fix reserved space for single frames
From: Stefan Schmidt @ 2018-08-06  9:24 UTC (permalink / raw)
  To: Alexander Aring, stefan; +Cc: linux-wpan, netdev, david.palma, rabinarayans0828
In-Reply-To: <20180714165210.20517-1-aring@mojatatu.com>

Hello.

On 07/14/2018 06:52 PM, Alexander Aring wrote:
> This patch fixes patch add handling to take care tail and headroom for
> single 6lowpan frames. We need to be sure we have a skb with the right
> head and tailroom for single frames. This patch do it by using
> skb_copy_expand() if head and tailroom is not enough allocated by upper
> layer.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=195059
> Reported-by: David Palma <david.palma@ntnu.no>
> Reported-by: Rabi Narayan Sahoo <rabinarayans0828@gmail.com>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>
> ---
> changes since v2:
>  skb to nskb, pointed out by stefan
> 
>  net/ieee802154/6lowpan/tx.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan/tx.c b/net/ieee802154/6lowpan/tx.c
> index e6ff5128e61a..ca53efa17be1 100644
> --- a/net/ieee802154/6lowpan/tx.c
> +++ b/net/ieee802154/6lowpan/tx.c
> @@ -265,9 +265,24 @@ netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *ldev)
>  	/* We must take a copy of the skb before we modify/replace the ipv6
>  	 * header as the header could be used elsewhere
>  	 */
> -	skb = skb_unshare(skb, GFP_ATOMIC);
> -	if (!skb)
> -		return NET_XMIT_DROP;
> +	if (unlikely(skb_headroom(skb) < ldev->needed_headroom ||
> +		     skb_tailroom(skb) < ldev->needed_tailroom)) {
> +		struct sk_buff *nskb;
> +
> +		nskb = skb_copy_expand(skb, ldev->needed_headroom,
> +				       ldev->needed_tailroom, GFP_ATOMIC);
> +		if (likely(nskb)) {
> +			consume_skb(skb);
> +			skb = nskb;
> +		} else {
> +			kfree_skb(skb);
> +			return NET_XMIT_DROP;
> +		}
> +	} else {
> +		skb = skb_unshare(skb, GFP_ATOMIC);
> +		if (!skb)
> +			return NET_XMIT_DROP;
> +	}
>  
>  	ret = lowpan_header(skb, ldev, &dgram_size, &dgram_offset);
>  	if (ret < 0) {
> 

This patch has been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

I know you submitted this for wpan instead of wpan-next, but with rc8
being out I will not submit another pull request for 4.18. Instead I
added cc stable to the patch to make sure it gets picked into the stable
tree once 4.18 is out.

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCH] mellanox: fix the dport endianness in call of __inet6_lookup_established()
From: Boris Pismenny @ 2018-08-06  9:23 UTC (permalink / raw)
  To: David Miller, viro; +Cc: netdev
In-Reply-To: <20180805.172728.867034375923011963.davem@davemloft.net>



On 8/6/2018 3:27 AM, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Sat, 4 Aug 2018 21:41:27 +0100
> 
>> __inet6_lookup_established() expect th->dport passed in host-endian,
>> not net-endian.  The reason is microoptimization in __inet6_lookup(),
>> but if you use the lower-level helpers, you have to play by their
>> rules...
>>      
>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 
> Mellanox folks, please review.
> 

Looks good to me. Thanks Al.

^ permalink raw reply

* [PATCH] Bluetooth: remove redundant variables 'adv_set' and 'cp'
From: YueHaibing @ 2018-08-06 11:08 UTC (permalink / raw)
  To: marcel, johan.hedberg
  Cc: linux-kernel, netdev, davem, linux-bluetooth, YueHaibing

Variables 'adv_set' and 'cp'  are being assigned but are never used hence
they are redundant and can be removed.

Cleans up clang warnings:
net/bluetooth/hci_event.c:1135:29: warning: variable 'adv_set' set but not used [-Wunused-but-set-variable]
net/bluetooth/mgmt.c:3359:39: warning: variable 'cp' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/bluetooth/hci_event.c | 3 ---
 net/bluetooth/mgmt.c      | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 754714c..8078587 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1132,7 +1132,6 @@ static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev,
 					 struct sk_buff *skb)
 {
 	struct hci_cp_le_set_ext_adv_enable *cp;
-	struct hci_cp_ext_adv_set *adv_set;
 	__u8 status = *((__u8 *) skb->data);
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
@@ -1144,8 +1143,6 @@ static void hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev,
 	if (!cp)
 		return;
 
-	adv_set = (void *) cp->data;
-
 	hci_dev_lock(hdev);
 
 	if (cp->enable) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 231602f7..3bdc8f3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3356,7 +3356,6 @@ int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip)
 static void set_default_phy_complete(struct hci_dev *hdev, u8 status,
 				     u16 opcode, struct sk_buff *skb)
 {
-	struct mgmt_cp_set_phy_confguration *cp;
 	struct mgmt_pending_cmd *cmd;
 
 	BT_DBG("status 0x%02x", status);
@@ -3367,8 +3366,6 @@ static void set_default_phy_complete(struct hci_dev *hdev, u8 status,
 	if (!cmd)
 		goto unlock;
 
-	cp = cmd->param;
-
 	if (status) {
 		mgmt_cmd_status(cmd->sk, hdev->id,
 				MGMT_OP_SET_PHY_CONFIGURATION,
-- 
2.7.0

^ permalink raw reply related

* Re: [stmmac][bug?] endianness of Flexible RX Parser code
From: Jose Abreu @ 2018-08-06  8:56 UTC (permalink / raw)
  To: Al Viro, Jose Abreu; +Cc: David S. Miller, netdev
In-Reply-To: <20180804011952.GD30522@ZenIV.linux.org.uk>

Hi Al,

On 04-08-2018 02:19, Al Viro wrote:
> 	The values passed in struct tc_u32_sel ->mask and ->val are
> 32bit net-endian.  Your tc_fill_entry() does this:
>
>         data = sel->keys[0].val;
>         mask = sel->keys[0].mask;
>
> ...
>                 entry->frag_ptr = frag;
>                 entry->val.match_en = (mask << (rem * 8)) &
>                         GENMASK(31, rem * 8);
>                 entry->val.match_data = (data << (rem * 8)) &
>                         GENMASK(31, rem * 8);
>                 entry->val.frame_offset = real_off;
>                 entry->prio = prio;
>
>                 frag->val.match_en = (mask >> (rem * 8)) &
>                         GENMASK(rem * 8 - 1, 0);
>                 frag->val.match_data = (data >> (rem * 8)) &
>                         GENMASK(rem * 8 - 1, 0);
>                 frag->val.frame_offset = real_off + 1;
>                 frag->prio = prio;
>                 frag->is_frag = true;
>
> and that looks very odd.  rem here is offset modulo 4.  Suppose offset is
> equal to 5, val contains {V0, V1, V2, V3} and mask - {M0, M1, M2, M3}.
> Then on little-endian host we get
> entry->val.match_en:	{0, M0, M1, M2}
> entry->val.match_data:	{0, V0, V1, V2}
> entry->val.frame_offset = 1;
> frag->val.match_en:	{M3, 0, 0, 0}
> frag->val.match_data:	{V3, 0, 0, 0}
> frag->val.frame_offset = 2;
> and on big-endian
> entry->val.match_en:	{M1, M2, M3, 0}
> entry->val.match_data:	{V1, V2, V3, 0}
> entry->val.frame_offset = 1;
> frag->val.match_en:	{0, 0, 0, M0}
> frag->val.match_data:	{0, 0, 0, V0}
> frag->val.frame_offset = 2;
>
> Little-endian variant looks like we mask octets 5, 6, 7 and 8 with
> M0..M3 resp. and want V0..V3 in those.  On big-endian, though, we
> look at the octets 11, 4, 5 and 6 instead.
>
> I don't know the hardware (and it might be pulling any kind of weird
> endianness-dependent stunts), but that really smells like a bug.

There is a feature in HW that supports Byte-Invariant Big-Endian
data transfer. It's not activated by default though ...

> It looks like that code is trying to do something like
>
>         data = ntohl(sel->keys[0].val);
>         mask = ntohl(sel->keys[0].mask);
> 	shift = rem * 8;
>
> 	entry->val.match_en = htonl(mask >> shift);
> 	entry->val.match_data = htonl(data >> shift);
> 	entry->val.frame_offset = real_off;
> 	...
> 	frag->val.match_en = htonl(mask << (32 - shift));
> 	frag->val.match_data = htonl(data << (32 - shift));
> 	entry->val.frame_offset = real_off + 1;
>
> Comments?

Looks good. Can you send a formal patch and a simple command that
I can use to validate this situation?

I used at the time at least two commands: one for validating
simple match by using 1 entry (i.e. plain match) and another to
validate 2 entries (i.e. two matches) and did not encounter any
problem ...

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* [PATCH net-next-internal] net: sched: cls_flower: set correct offload data in fl_reoffload
From: Vlad Buslov @ 2018-08-06  8:27 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, Vlad Buslov

fl_reoffload implementation sets following members of struct
tc_cls_flower_offload incorrectly:
 - masked key instead of mask
 - key instead of masked key

Fix fl_reoffload to provide correct data to offload callback.

Fixes: 31533cba4327 ("net: sched: cls_flower: implement offload tcf_proto_op")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index e8bd08ba998a..cbf165c33656 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1174,8 +1174,8 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
 				TC_CLSFLOWER_REPLACE : TC_CLSFLOWER_DESTROY;
 			cls_flower.cookie = (unsigned long)f;
 			cls_flower.dissector = &mask->dissector;
-			cls_flower.mask = &f->mkey;
-			cls_flower.key = &f->key;
+			cls_flower.mask = &mask->key;
+			cls_flower.key = &f->mkey;
 			cls_flower.exts = &f->exts;
 			cls_flower.classid = f->res.classid;
 
-- 
2.7.5

^ permalink raw reply related

* Re: [PATCH v8 bpf-next 00/10] veth: Driver XDP
From: Björn Töpel @ 2018-08-06  8:22 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Toshiaki Makita
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Jakub Kicinski,
	John Fastabend, Tariq Toukan, Björn Töpel,
	intel-wired-lan
In-Reply-To: <20180803114538.382664c8@redhat.com>

On 2018-08-03 11:45, Jesper Dangaard Brouer wrote:
> On Fri,  3 Aug 2018 16:58:08 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
>> This patch set introduces driver XDP for veth.
>> Basically this is used in conjunction with redirect action of another XDP
>> program.
>>
>>    NIC -----------> veth===veth
>>   (XDP) (redirect)        (XDP)
>>
> 
> I'm was playing with V7 on my testlab yesterday and I noticed one
> fundamental issue.  You are not updating the "ifconfig" stats counters,
> when in XDP mode.  This makes receive or send via XDP invisible to
> sysadm/management tools.  This for-sure is going to cause confusion...
> 
> I took a closer look at other driver. The ixgbe driver is doing the
> right thing.  Driver i40e have a bug, where RX/TX stats are swapped
> getting (strange!).  

Indeed! Thanks for finding/reporting this! I'll have look!


Björn

> The mlx5 driver is not updating the regular RX/TX
> counters, but A LOT of other ethtool stats counters (which are the ones
> I usually monitor when testing).
> 
> So, given other drivers also didn't get this right, we need to have a
> discussion outside your/this patchset.  Thus, I don't want to
> stop/stall this patchset, but this is something we need to fixup in a
> followup patchset to other drivers as well.
> 
> Thus, I'm acking the patchset, but I request that we do a joint effort
> of fixing this as followup patches.
> 
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> 

^ permalink raw reply

* [net-next:master 457/471] ERROR: "__aeabi_ldivmod" [drivers/ptp/ptp_qoriq.ko] undefined!
From: kbuild test robot @ 2018-08-06  8:07 UTC (permalink / raw)
  To: Yangbo Lu; +Cc: kbuild-all, netdev

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head:   981467033a37d916649647fa3afe1fe99bba1817
commit: 91305f2812624c0cf7ccbb44133b66d3b24676e4 [457/471] ptp_qoriq: support automatic configuration for ptp timer
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 91305f2812624c0cf7ccbb44133b66d3b24676e4
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "__aeabi_ldivmod" [drivers/ptp/ptp_qoriq.ko] undefined!
>> ERROR: "__aeabi_uldivmod" [drivers/ptp/ptp_qoriq.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66173 bytes --]

^ permalink raw reply

* Re: [PATCH v3 net-next 5/9] net: stmmac: Add MDIO related functions for XGMAC2
From: Jose Abreu @ 2018-08-06  7:59 UTC (permalink / raw)
  To: Florian Fainelli, Jose Abreu, netdev
  Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
	Andrew Lunn
In-Reply-To: <01fc5766-d3ad-3056-6c6c-f9a6d603f87e@gmail.com>

On 03-08-2018 20:06, Florian Fainelli wrote:
> On 08/03/2018 08:50 AM, Jose Abreu wrote:
>> Add the MDIO related funcionalities for the new IP block XGMAC2.
>>
>> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> ---
>> +satic int stmmac_xgmac2_c22_format(struct stmmac_priv *priv, int phyaddr,
>> +				    int phyreg, u32 *hw_addr)
>> +{
>> +	unsigned int mii_data = priv->hw->mii.data;
>> +	u32 tmp;
>> +
>> +	/* HW does not support C22 addr >= 4 */
>> +	if (phyaddr >= 4)
>> +		return -ENODEV;
> It would be nice if this could be moved at probe time so you don't have
> to wait until you connect to the PHY, read its PHY OUI and find out it
> has a MDIO address >= 4. Not a blocker, but something that could be
> improved further on.
>
> In premise you could even scan the MDIO bus' device tree node, and find
> that out ahead of time.

Oh, but I use phylib ... I only provide the read/write callbacks
so I think we should avoid duplicating the code that's already in
the phylib ... No?

>
>> +	/* Wait until any existing MII operation is complete */
>> +	if (readl_poll_timeout(priv->ioaddr + mii_data, tmp,
>> +			       !(tmp & MII_XGMAC_BUSY), 100, 10000))
>> +		return -EBUSY;
>> +
>> +	/* Set port as Clause 22 */
>> +	tmp = readl(priv->ioaddr + XGMAC_MDIO_C22P);
>> +	tmp |= BIT(phyaddr);
> Since the registers are being Read/Modify/Write here, don't you need to
> clear the previous address bits as well?
>
> You probably did not encounter any problems in your testing if you had
> only one PHY on the MDIO bus, but this is not something that is
> necessarily true, e.g: if you have an Ethernet switch, several MDIO bus
> addresses are going to be responding.
>
> Your MDIO bus implementation must be able to support one transaction
> with one PHY address and the next transaction with another PHY address ,
> etc...
>
> That is something that should be easy to fix and be resubmitted as part
> of v4.

I'm not following you here... I only set/unset the bit for the
corresponding phyaddr that phylib wants to read/write. Why would
I clear the remaining addresses?

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* Re: [PATCH v3 net-next 3/9] net: stmmac: Add DMA related callbacks for XGMAC2
From: Jose Abreu @ 2018-08-06  7:56 UTC (permalink / raw)
  To: Florian Fainelli, Jose Abreu, netdev
  Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <dc666664-d93e-0408-cfd7-86b271191d3e@gmail.com>

On 03-08-2018 19:58, Florian Fainelli wrote:
> On 08/03/2018 08:50 AM, Jose Abreu wrote:
>> Add the DMA related callbacks for the new IP block XGMAC2.
>>
>> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>> ---
>> +	value &= ~XGMAC_RD_OSR_LMT;
>> +	value |= (axi->axi_rd_osr_lmt << XGMAC_RD_OSR_LMT_SHIFT) &
>> +		XGMAC_RD_OSR_LMT;
>> +
>> +	for (i = 0; i < AXI_BLEN; i++) {
>> +		if (axi->axi_blen[i])
>> +			value &= ~XGMAC_UNDEF;
> Should not you be you clearing all XGMAC_BLEN* values since you do a
> logical or here? I am assuming this is not something that would likely
> change from one open/close but still?

Yeah, this won't change between open/close but I will add the
mask anyway.

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* Re: [PATCH v3 net-next 1/9] net: stmmac: Add XGMAC 2.10 HWIF entry
From: Jose Abreu @ 2018-08-06  7:55 UTC (permalink / raw)
  To: Florian Fainelli, Jose Abreu, netdev
  Cc: David S. Miller, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <4f309bf8-c669-ae9d-750c-8946715f0fe1@gmail.com>

Hi Florian,

On 03-08-2018 19:54, Florian Fainelli wrote:
> On 08/03/2018 08:50 AM, Jose Abreu wrote:
>> @@ -87,6 +88,7 @@ static const struct stmmac_hwif_entry {
>>  	{
>>  		.gmac = false,
>>  		.gmac4 = false,
>> +		.xgmac = false,
> In a future clean-up you would like want to remove this and replace this
> an enumeration which is less error prone than having to define a boolean
> for each of these previous generations only to say "this is not an xgmac".

Its a good idea! I really don't like the pattern of "if
(priv->plat->has_something)" in the code. I will think about
adding this but for now let's merge xgmac2 and I will try to
refactor the code later because I will need to change many files
and the patchset would be bigger.

Thanks and Best Regards,
Jose Miguel Abreu

^ permalink raw reply

* Re: [PATCH] net: ieee802154: 6lowpan: remove redundant pointers 'fq' and 'net'
From: Stefan Schmidt @ 2018-08-06  9:10 UTC (permalink / raw)
  To: Colin King, Alexander Aring, David S . Miller, linux-wpan, netdev
  Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180731154507.5247-1-colin.king@canonical.com>

Hello.

On 07/31/2018 05:45 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Pointers fq and net are being assigned but are never used hence they
> are redundant and can be removed.
> 
> Cleans up clang warnings:
> warning: variable 'fq' set but not used [-Wunused-but-set-variable]
> warning: variable 'net' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  net/ieee802154/6lowpan/reassembly.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
> index ec7a5da56129..e7857a8ac86d 100644
> --- a/net/ieee802154/6lowpan/reassembly.c
> +++ b/net/ieee802154/6lowpan/reassembly.c
> @@ -40,9 +40,6 @@ static int lowpan_frag_reasm(struct lowpan_frag_queue *fq,
>  static void lowpan_frag_init(struct inet_frag_queue *q, const void *a)
>  {
>  	const struct frag_lowpan_compare_key *key = a;
> -	struct lowpan_frag_queue *fq;
> -
> -	fq = container_of(q, struct lowpan_frag_queue, q);
>  
>  	BUILD_BUG_ON(sizeof(*key) > sizeof(q->key));
>  	memcpy(&q->key, key, sizeof(*key));
> @@ -52,10 +49,8 @@ static void lowpan_frag_expire(struct timer_list *t)
>  {
>  	struct inet_frag_queue *frag = from_timer(frag, t, timer);
>  	struct frag_queue *fq;
> -	struct net *net;
>  
>  	fq = container_of(frag, struct frag_queue, q);
> -	net = container_of(fq->q.net, struct net, ieee802154_lowpan.frags);
>  
>  	spin_lock(&fq->q.lock);
>  
> 

This patch has been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

regards
Stefan Schmidt

^ permalink raw reply

* Re: [PATCH] ieee802154: add rx LQI from userspace
From: Stefan Schmidt @ 2018-08-06  9:07 UTC (permalink / raw)
  To: Clément Péron, Romuald Cari, linux-wpan
  Cc: Alexander Aring, Stefan Schmidt, David S . Miller, netdev,
	linux-kernel, Clément Peron
In-Reply-To: <20180607140802.22666-1-peron.clem@gmail.com>

Hello.

On 06/07/2018 04:08 PM, Clément Péron wrote:
> From: Romuald CARI <romuald.cari@devialet.com>
> 
> The Link Quality Indication data exposed by drivers could not be accessed from
> userspace. Since this data is per-datagram received, it makes sense to make it
> available to userspace application through the ancillary data mechanism in
> recvmsg rather than through ioctls. This can be activated using the socket
> option WPAN_WANTLQI under SOL_IEEE802154 protocol.
> 
> This LQI data is available in the ancillary data buffer under the SOL_IEEE802154
> level as the type WPAN_LQI. The value is an unsigned byte indicating the link
> quality with values ranging 0-255.
> 
> Signed-off-by: Romuald Cari <romuald.cari@devialet.com>
> Signed-off-by: Clément Peron <clement.peron@devialet.com>
> ---
>  include/net/af_ieee802154.h |  1 +
>  net/ieee802154/socket.c     | 17 +++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/net/af_ieee802154.h b/include/net/af_ieee802154.h
> index a5563d27a3eb..8003a9f6eb43 100644
> --- a/include/net/af_ieee802154.h
> +++ b/include/net/af_ieee802154.h
> @@ -56,6 +56,7 @@ struct sockaddr_ieee802154 {
>  #define WPAN_WANTACK		0
>  #define WPAN_SECURITY		1
>  #define WPAN_SECURITY_LEVEL	2
> +#define WPAN_WANTLQI		3
>  
>  #define WPAN_SECURITY_DEFAULT	0
>  #define WPAN_SECURITY_OFF	1
> diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
> index a60658c85a9a..bc6b912603f1 100644
> --- a/net/ieee802154/socket.c
> +++ b/net/ieee802154/socket.c
> @@ -25,6 +25,7 @@
>  #include <linux/termios.h>	/* For TIOCOUTQ/INQ */
>  #include <linux/list.h>
>  #include <linux/slab.h>
> +#include <linux/socket.h>
>  #include <net/datalink.h>
>  #include <net/psnap.h>
>  #include <net/sock.h>
> @@ -452,6 +453,7 @@ struct dgram_sock {
>  	unsigned int bound:1;
>  	unsigned int connected:1;
>  	unsigned int want_ack:1;
> +	unsigned int want_lqi:1;
>  	unsigned int secen:1;
>  	unsigned int secen_override:1;
>  	unsigned int seclevel:3;
> @@ -486,6 +488,7 @@ static int dgram_init(struct sock *sk)
>  	struct dgram_sock *ro = dgram_sk(sk);
>  
>  	ro->want_ack = 1;
> +	ro->want_lqi = 0;
>  	return 0;
>  }
>  
> @@ -713,6 +716,7 @@ static int dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  	size_t copied = 0;
>  	int err = -EOPNOTSUPP;
>  	struct sk_buff *skb;
> +	struct dgram_sock *ro = dgram_sk(sk);
>  	DECLARE_SOCKADDR(struct sockaddr_ieee802154 *, saddr, msg->msg_name);
>  
>  	skb = skb_recv_datagram(sk, flags, noblock, &err);
> @@ -744,6 +748,13 @@ static int dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  		*addr_len = sizeof(*saddr);
>  	}
>  
> +	if (ro->want_lqi) {
> +		err = put_cmsg(msg, SOL_IEEE802154, WPAN_WANTLQI,
> +			       sizeof(uint8_t), &(mac_cb(skb)->lqi));
> +		if (err)
> +			goto done;
> +	}
> +
>  	if (flags & MSG_TRUNC)
>  		copied = skb->len;
>  done:
> @@ -847,6 +858,9 @@ static int dgram_getsockopt(struct sock *sk, int level, int optname,
>  	case WPAN_WANTACK:
>  		val = ro->want_ack;
>  		break;
> +	case WPAN_WANTLQI:
> +		val = ro->want_lqi;
> +		break;
>  	case WPAN_SECURITY:
>  		if (!ro->secen_override)
>  			val = WPAN_SECURITY_DEFAULT;
> @@ -892,6 +906,9 @@ static int dgram_setsockopt(struct sock *sk, int level, int optname,
>  	case WPAN_WANTACK:
>  		ro->want_ack = !!val;
>  		break;
> +	case WPAN_WANTLQI:
> +		ro->want_lqi = !!val;
> +		break;
>  	case WPAN_SECURITY:
>  		if (!ns_capable(net->user_ns, CAP_NET_ADMIN) &&
>  		    !ns_capable(net->user_ns, CAP_NET_RAW)) {
> 

This patch has been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

regards
Stefan Schmidt

^ permalink raw reply

* [PATCH net-next 13/14] net: core: add new/replace rate estimator lock parameter
From: Vlad Buslov @ 2018-08-06  6:54 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
	edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533538465-23199-1-git-send-email-vladbu@mellanox.com>

Extend rate estimator 'new' and 'replace' APIs with additional spinlock
parameter to be used by rtnl-unlocked actions to protect rate_est pointer
from concurrent modification.

Extract code that requires synchronization from gen_new_estimator into
__replace_estimator function in order to be used by both locked and
unlocked paths.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 include/net/gen_stats.h    |  2 ++
 net/core/gen_estimator.c   | 58 +++++++++++++++++++++++++++++++++++-----------
 net/netfilter/xt_RATEEST.c |  2 +-
 net/sched/act_api.c        |  2 +-
 net/sched/act_police.c     |  2 +-
 net/sched/sch_api.c        |  2 ++
 net/sched/sch_cbq.c        |  4 ++--
 net/sched/sch_drr.c        |  4 ++--
 net/sched/sch_hfsc.c       |  4 ++--
 net/sched/sch_htb.c        |  4 ++--
 net/sched/sch_qfq.c        |  4 ++--
 11 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index 0304ba2ae353..d1ef63d16abc 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -59,12 +59,14 @@ int gnet_stats_finish_copy(struct gnet_dump *d);
 int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
 		      struct gnet_stats_basic_cpu __percpu *cpu_bstats,
 		      struct net_rate_estimator __rcu **rate_est,
+		      spinlock_t *rate_est_lock,
 		      spinlock_t *stats_lock,
 		      seqcount_t *running, struct nlattr *opt);
 void gen_kill_estimator(struct net_rate_estimator __rcu **ptr);
 int gen_replace_estimator(struct gnet_stats_basic_packed *bstats,
 			  struct gnet_stats_basic_cpu __percpu *cpu_bstats,
 			  struct net_rate_estimator __rcu **ptr,
+			  spinlock_t *rate_est_lock,
 			  spinlock_t *stats_lock,
 			  seqcount_t *running, struct nlattr *opt);
 bool gen_estimator_active(struct net_rate_estimator __rcu **ptr);
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 98fd12721221..012e37011147 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -107,11 +107,43 @@ static void est_timer(struct timer_list *t)
 	mod_timer(&est->timer, est->next_jiffies);
 }
 
+static void __replace_estimator(struct net_rate_estimator __rcu **rate_est,
+				struct net_rate_estimator *new)
+{
+	struct net_rate_estimator *old = rcu_dereference_protected(*rate_est,
+								   1);
+
+	if (old) {
+		del_timer_sync(&old->timer);
+		new->avbps = old->avbps;
+		new->avpps = old->avpps;
+	}
+
+	new->next_jiffies = jiffies + ((HZ / 4) << new->intvl_log);
+	timer_setup(&new->timer, est_timer, 0);
+	mod_timer(&new->timer, new->next_jiffies);
+
+	rcu_assign_pointer(*rate_est, new);
+
+	if (old)
+		kfree_rcu(old, rcu);
+}
+
+static void replace_estimator(struct net_rate_estimator __rcu **rate_est,
+			      struct net_rate_estimator *new,
+			      spinlock_t *rate_est_lock)
+{
+	spin_lock_bh(rate_est_lock);
+	__replace_estimator(rate_est, new);
+	spin_unlock_bh(rate_est_lock);
+}
+
 /**
  * gen_new_estimator - create a new rate estimator
  * @bstats: basic statistics
  * @cpu_bstats: bstats per cpu
  * @rate_est: rate estimator statistics
+ * @rate_est_lock: rate_est lock (might be NULL)
  * @stats_lock: statistics lock
  * @running: qdisc running seqcount
  * @opt: rate estimator configuration TLV
@@ -128,12 +160,13 @@ static void est_timer(struct timer_list *t)
 int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
 		      struct gnet_stats_basic_cpu __percpu *cpu_bstats,
 		      struct net_rate_estimator __rcu **rate_est,
+		      spinlock_t *rate_est_lock,
 		      spinlock_t *stats_lock,
 		      seqcount_t *running,
 		      struct nlattr *opt)
 {
 	struct gnet_estimator *parm = nla_data(opt);
-	struct net_rate_estimator *old, *est;
+	struct net_rate_estimator *est;
 	struct gnet_stats_basic_packed b;
 	int intvl_log;
 
@@ -167,20 +200,15 @@ int gen_new_estimator(struct gnet_stats_basic_packed *bstats,
 		local_bh_enable();
 	est->last_bytes = b.bytes;
 	est->last_packets = b.packets;
-	old = rcu_dereference_protected(*rate_est, 1);
-	if (old) {
-		del_timer_sync(&old->timer);
-		est->avbps = old->avbps;
-		est->avpps = old->avpps;
-	}
 
-	est->next_jiffies = jiffies + ((HZ/4) << intvl_log);
-	timer_setup(&est->timer, est_timer, 0);
-	mod_timer(&est->timer, est->next_jiffies);
+	if (rate_est_lock)
+		replace_estimator(rate_est, est, rate_est_lock);
+	else
+		/* If no spinlock argument provided,
+		 * then assume that caller is already synchronized.
+		 */
+		__replace_estimator(rate_est, est);
 
-	rcu_assign_pointer(*rate_est, est);
-	if (old)
-		kfree_rcu(old, rcu);
 	return 0;
 }
 EXPORT_SYMBOL(gen_new_estimator);
@@ -209,6 +237,7 @@ EXPORT_SYMBOL(gen_kill_estimator);
  * @bstats: basic statistics
  * @cpu_bstats: bstats per cpu
  * @rate_est: rate estimator statistics
+ * @rate_est_lock: rate_est lock (might be NULL)
  * @stats_lock: statistics lock
  * @running: qdisc running seqcount (might be NULL)
  * @opt: rate estimator configuration TLV
@@ -221,10 +250,11 @@ EXPORT_SYMBOL(gen_kill_estimator);
 int gen_replace_estimator(struct gnet_stats_basic_packed *bstats,
 			  struct gnet_stats_basic_cpu __percpu *cpu_bstats,
 			  struct net_rate_estimator __rcu **rate_est,
+			  spinlock_t *rate_est_lock,
 			  spinlock_t *stats_lock,
 			  seqcount_t *running, struct nlattr *opt)
 {
-	return gen_new_estimator(bstats, cpu_bstats, rate_est,
+	return gen_new_estimator(bstats, cpu_bstats, rate_est, rate_est_lock,
 				 stats_lock, running, opt);
 }
 EXPORT_SYMBOL(gen_replace_estimator);
diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
index dec843cadf46..8e79bd50bc0b 100644
--- a/net/netfilter/xt_RATEEST.c
+++ b/net/netfilter/xt_RATEEST.c
@@ -154,7 +154,7 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
 	cfg.est.interval	= info->interval;
 	cfg.est.ewma_log	= info->ewma_log;
 
-	ret = gen_new_estimator(&est->bstats, NULL, &est->rate_est,
+	ret = gen_new_estimator(&est->bstats, NULL, &est->rate_est, NULL,
 				&est->lock, NULL, &cfg.opt);
 	if (ret < 0)
 		goto err2;
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 229d63c99be2..28c2d41f0cd2 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -401,7 +401,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
 	p->tcfa_tm.firstuse = 0;
 	if (est) {
 		err = gen_new_estimator(&p->tcfa_bstats, p->cpu_bstats,
-					&p->tcfa_rate_est,
+					&p->tcfa_rate_est, NULL,
 					&p->tcfa_lock, NULL, est);
 		if (err)
 			goto err3;
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 1f3192ea8df7..3eb5fe60c62c 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -138,7 +138,7 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
 
 	if (est) {
 		err = gen_replace_estimator(&police->tcf_bstats, NULL,
-					    &police->tcf_rate_est,
+					    &police->tcf_rate_est, NULL,
 					    &police->tcf_lock,
 					    NULL, est);
 		if (err)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 98541c6399db..57cdc105ce30 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1185,6 +1185,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 					sch->cpu_bstats,
 					&sch->rate_est,
 					NULL,
+					NULL,
 					running,
 					tca[TCA_RATE]);
 		if (err) {
@@ -1260,6 +1261,7 @@ static int qdisc_change(struct Qdisc *sch, struct nlattr **tca,
 				      sch->cpu_bstats,
 				      &sch->rate_est,
 				      NULL,
+				      NULL,
 				      qdisc_root_sleeping_running(sch),
 				      tca[TCA_RATE]);
 	}
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index f42025d53cfe..2a7ff53b4e7a 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1503,7 +1503,7 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 
 		if (tca[TCA_RATE]) {
 			err = gen_replace_estimator(&cl->bstats, NULL,
-						    &cl->rate_est,
+						    &cl->rate_est, NULL,
 						    NULL,
 						    qdisc_root_sleeping_running(sch),
 						    tca[TCA_RATE]);
@@ -1605,7 +1605,7 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 
 	if (tca[TCA_RATE]) {
 		err = gen_new_estimator(&cl->bstats, NULL, &cl->rate_est,
-					NULL,
+					NULL, NULL,
 					qdisc_root_sleeping_running(sch),
 					tca[TCA_RATE]);
 		if (err) {
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index e0b0cf8a9939..0896e239063b 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -95,7 +95,7 @@ static int drr_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	if (cl != NULL) {
 		if (tca[TCA_RATE]) {
 			err = gen_replace_estimator(&cl->bstats, NULL,
-						    &cl->rate_est,
+						    &cl->rate_est, NULL,
 						    NULL,
 						    qdisc_root_sleeping_running(sch),
 						    tca[TCA_RATE]);
@@ -129,7 +129,7 @@ static int drr_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 
 	if (tca[TCA_RATE]) {
 		err = gen_replace_estimator(&cl->bstats, NULL, &cl->rate_est,
-					    NULL,
+					    NULL, NULL,
 					    qdisc_root_sleeping_running(sch),
 					    tca[TCA_RATE]);
 		if (err) {
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 3278a76f6861..be4ead89b50e 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -972,7 +972,7 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 
 		if (tca[TCA_RATE]) {
 			err = gen_replace_estimator(&cl->bstats, NULL,
-						    &cl->rate_est,
+						    &cl->rate_est, NULL,
 						    NULL,
 						    qdisc_root_sleeping_running(sch),
 						    tca[TCA_RATE]);
@@ -1042,7 +1042,7 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 
 	if (tca[TCA_RATE]) {
 		err = gen_new_estimator(&cl->bstats, NULL, &cl->rate_est,
-					NULL,
+					NULL, NULL,
 					qdisc_root_sleeping_running(sch),
 					tca[TCA_RATE]);
 		if (err) {
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 43c4bfe625a9..f782a1d31264 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1395,7 +1395,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 		if (htb_rate_est || tca[TCA_RATE]) {
 			err = gen_new_estimator(&cl->bstats, NULL,
 						&cl->rate_est,
-						NULL,
+						NULL, NULL,
 						qdisc_root_sleeping_running(sch),
 						tca[TCA_RATE] ? : &est.nla);
 			if (err) {
@@ -1460,7 +1460,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 	} else {
 		if (tca[TCA_RATE]) {
 			err = gen_replace_estimator(&cl->bstats, NULL,
-						    &cl->rate_est,
+						    &cl->rate_est, NULL,
 						    NULL,
 						    qdisc_root_sleeping_running(sch),
 						    tca[TCA_RATE]);
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index bb1a9c11fc54..8026c1eebd79 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -461,7 +461,7 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	if (cl != NULL) { /* modify existing class */
 		if (tca[TCA_RATE]) {
 			err = gen_replace_estimator(&cl->bstats, NULL,
-						    &cl->rate_est,
+						    &cl->rate_est, NULL,
 						    NULL,
 						    qdisc_root_sleeping_running(sch),
 						    tca[TCA_RATE]);
@@ -488,7 +488,7 @@ static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	if (tca[TCA_RATE]) {
 		err = gen_new_estimator(&cl->bstats, NULL,
 					&cl->rate_est,
-					NULL,
+					NULL, NULL,
 					qdisc_root_sleeping_running(sch),
 					tca[TCA_RATE]);
 		if (err)
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 12/14] net: sched: act_mirred: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-06  6:54 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
	edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533538465-23199-1-git-send-email-vladbu@mellanox.com>

Re-introduce mirred list spinlock, that was removed some time ago, in order
to protect it from concurrent modifications, instead of relying on rtnl
lock.

Use tcf spinlock to protect mirred action private data from concurrent
modification in init and dump. Rearrange access to mirred data in order to
be performed only while holding the lock.

Rearrange net dev access to always hold reference while working with it,
instead of relying on rntl lock. Change get dev function to increment net
device reference before returning it to caller, instead of assuming that
caller is protected with rtnl lock.

Provide rcu version of mirred dev and tunnel info access functions. (to be
used by unlocked drivers)

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 include/net/tc_act/tc_mirred.h     |  5 +++
 include/net/tc_act/tc_tunnel_key.h | 33 ++++++++++++---
 net/sched/act_mirred.c             | 82 +++++++++++++++++++++++++-------------
 net/sched/cls_api.c                |  1 +
 4 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index a2e9cbca5c9e..cb30be55e444 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -37,4 +37,9 @@ static inline struct net_device *tcf_mirred_dev(const struct tc_action *a)
 	return rtnl_dereference(to_mirred(a)->tcfm_dev);
 }
 
+static inline struct net_device *tcf_mirred_dev_rcu(const struct tc_action *a)
+{
+	return rcu_dereference(to_mirred(a)->tcfm_dev);
+}
+
 #endif /* __NET_TC_MIR_H */
diff --git a/include/net/tc_act/tc_tunnel_key.h b/include/net/tc_act/tc_tunnel_key.h
index 46b8c7f1c8d5..e6e475d788c6 100644
--- a/include/net/tc_act/tc_tunnel_key.h
+++ b/include/net/tc_act/tc_tunnel_key.h
@@ -30,26 +30,47 @@ struct tcf_tunnel_key {
 
 static inline bool is_tcf_tunnel_set(const struct tc_action *a)
 {
+	bool ret = false;
 #ifdef CONFIG_NET_CLS_ACT
 	struct tcf_tunnel_key *t = to_tunnel_key(a);
-	struct tcf_tunnel_key_params *params = rtnl_dereference(t->params);
+	struct tcf_tunnel_key_params *params;
 
+	rcu_read_lock();
+	params = rcu_dereference(t->params);
 	if (a->ops && a->ops->type == TCA_ACT_TUNNEL_KEY)
-		return params->tcft_action == TCA_TUNNEL_KEY_ACT_SET;
+		ret = params->tcft_action == TCA_TUNNEL_KEY_ACT_SET;
+	rcu_read_unlock();
 #endif
-	return false;
+	return ret;
 }
 
 static inline bool is_tcf_tunnel_release(const struct tc_action *a)
 {
+	bool ret = false;
 #ifdef CONFIG_NET_CLS_ACT
 	struct tcf_tunnel_key *t = to_tunnel_key(a);
-	struct tcf_tunnel_key_params *params = rtnl_dereference(t->params);
+	struct tcf_tunnel_key_params *params;
 
+	rcu_read_lock();
+	params = rcu_dereference(t->params);
 	if (a->ops && a->ops->type == TCA_ACT_TUNNEL_KEY)
-		return params->tcft_action == TCA_TUNNEL_KEY_ACT_RELEASE;
+		ret = params->tcft_action == TCA_TUNNEL_KEY_ACT_RELEASE;
+	rcu_read_unlock();
+#endif
+	return ret;
+}
+
+static inline
+struct ip_tunnel_info *tcf_tunnel_info_rcu(const struct tc_action *a)
+{
+#ifdef CONFIG_NET_CLS_ACT
+	struct tcf_tunnel_key *t = to_tunnel_key(a);
+	struct tcf_tunnel_key_params *params = rcu_dereference(t->params);
+
+	return &params->tcft_enc_metadata->u.tun_info;
+#else
+	return NULL;
 #endif
-	return false;
 }
 
 static inline struct ip_tunnel_info *tcf_tunnel_info(const struct tc_action *a)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index b26d060da08e..9f622114f5a5 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -30,6 +30,7 @@
 #include <net/tc_act/tc_mirred.h>
 
 static LIST_HEAD(mirred_list);
+static DEFINE_SPINLOCK(mirred_list_lock);
 
 static bool tcf_mirred_is_act_redirect(int action)
 {
@@ -62,13 +63,23 @@ static bool tcf_mirred_can_reinsert(int action)
 	return false;
 }
 
+static struct net_device *tcf_mirred_dev_dereference(struct tcf_mirred *m)
+{
+	return rcu_dereference_protected(m->tcfm_dev,
+					 lockdep_is_held(&m->tcf_lock));
+}
+
 static void tcf_mirred_release(struct tc_action *a)
 {
 	struct tcf_mirred *m = to_mirred(a);
 	struct net_device *dev;
 
+	spin_lock(&mirred_list_lock);
 	list_del(&m->tcfm_list);
-	dev = rtnl_dereference(m->tcfm_dev);
+	spin_unlock(&mirred_list_lock);
+
+	/* last reference to action, no need to lock */
+	dev = rcu_dereference_protected(m->tcfm_dev, 1);
 	if (dev)
 		dev_put(dev);
 }
@@ -128,22 +139,9 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 		NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
 		return -EINVAL;
 	}
-	if (parm->ifindex) {
-		dev = __dev_get_by_index(net, parm->ifindex);
-		if (dev == NULL) {
-			if (exists)
-				tcf_idr_release(*a, bind);
-			else
-				tcf_idr_cleanup(tn, parm->index);
-			return -ENODEV;
-		}
-		mac_header_xmit = dev_is_mac_header_xmit(dev);
-	} else {
-		dev = NULL;
-	}
 
 	if (!exists) {
-		if (!dev) {
+		if (!parm->ifindex) {
 			tcf_idr_cleanup(tn, parm->index);
 			NL_SET_ERR_MSG_MOD(extack, "Specified device does not exist");
 			return -EINVAL;
@@ -161,19 +159,31 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 	}
 	m = to_mirred(*a);
 
-	ASSERT_RTNL();
+	spin_lock(&m->tcf_lock);
 	m->tcf_action = parm->action;
 	m->tcfm_eaction = parm->eaction;
-	if (dev != NULL) {
-		if (ret != ACT_P_CREATED)
-			dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
-		dev_hold(dev);
-		rcu_assign_pointer(m->tcfm_dev, dev);
+
+	if (parm->ifindex) {
+		dev = dev_get_by_index(net, parm->ifindex);
+		if (!dev) {
+			spin_unlock(&m->tcf_lock);
+			tcf_idr_release(*a, bind);
+			return -ENODEV;
+		}
+		mac_header_xmit = dev_is_mac_header_xmit(dev);
+		rcu_swap_protected(m->tcfm_dev, dev,
+				   lockdep_is_held(&m->tcf_lock));
+		if (dev)
+			dev_put(dev);
 		m->tcfm_mac_header_xmit = mac_header_xmit;
 	}
+	spin_unlock(&m->tcf_lock);
 
 	if (ret == ACT_P_CREATED) {
+		spin_lock(&mirred_list_lock);
 		list_add(&m->tcfm_list, &mirred_list);
+		spin_unlock(&mirred_list_lock);
+
 		tcf_idr_insert(tn, *a);
 	}
 
@@ -287,26 +297,33 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_mirred *m = to_mirred(a);
-	struct net_device *dev = rtnl_dereference(m->tcfm_dev);
 	struct tc_mirred opt = {
 		.index   = m->tcf_index,
-		.action  = m->tcf_action,
 		.refcnt  = refcount_read(&m->tcf_refcnt) - ref,
 		.bindcnt = atomic_read(&m->tcf_bindcnt) - bind,
-		.eaction = m->tcfm_eaction,
-		.ifindex = dev ? dev->ifindex : 0,
 	};
+	struct net_device *dev;
 	struct tcf_t t;
 
+	spin_lock(&m->tcf_lock);
+	opt.action = m->tcf_action;
+	opt.eaction = m->tcfm_eaction;
+	dev = tcf_mirred_dev_dereference(m);
+	if (dev)
+		opt.ifindex = dev->ifindex;
+
 	if (nla_put(skb, TCA_MIRRED_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
 	tcf_tm_dump(&t, &m->tcf_tm);
 	if (nla_put_64bit(skb, TCA_MIRRED_TM, sizeof(t), &t, TCA_MIRRED_PAD))
 		goto nla_put_failure;
+	spin_unlock(&m->tcf_lock);
+
 	return skb->len;
 
 nla_put_failure:
+	spin_unlock(&m->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
@@ -337,15 +354,19 @@ static int mirred_device_event(struct notifier_block *unused,
 
 	ASSERT_RTNL();
 	if (event == NETDEV_UNREGISTER) {
+		spin_lock(&mirred_list_lock);
 		list_for_each_entry(m, &mirred_list, tcfm_list) {
-			if (rcu_access_pointer(m->tcfm_dev) == dev) {
+			spin_lock(&m->tcf_lock);
+			if (tcf_mirred_dev_dereference(m) == dev) {
 				dev_put(dev);
 				/* Note : no rcu grace period necessary, as
 				 * net_device are already rcu protected.
 				 */
 				RCU_INIT_POINTER(m->tcfm_dev, NULL);
 			}
+			spin_unlock(&m->tcf_lock);
 		}
+		spin_unlock(&mirred_list_lock);
 	}
 
 	return NOTIFY_DONE;
@@ -358,8 +379,15 @@ static struct notifier_block mirred_device_notifier = {
 static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
 {
 	struct tcf_mirred *m = to_mirred(a);
+	struct net_device *dev;
+
+	rcu_read_lock();
+	dev = rcu_dereference(m->tcfm_dev);
+	if (dev)
+		dev_hold(dev);
+	rcu_read_unlock();
 
-	return rtnl_dereference(m->tcfm_dev);
+	return dev;
 }
 
 static int tcf_mirred_delete(struct net *net, u32 index)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e8b0bbd0883f..0cce0eadc28b 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2167,6 +2167,7 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
 		if (!dev)
 			continue;
 		ret = tc_setup_cb_egdev_call(dev, type, type_data, err_stop);
+		dev_put(dev);
 		if (ret < 0)
 			return ret;
 		ok_count += ret;
-- 
2.7.5

^ permalink raw reply related

* [PATCH net-next 14/14] net: sched: act_police: remove dependency on rtnl lock
From: Vlad Buslov @ 2018-08-06  6:54 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, xiyou.wangcong, jiri, pablo, kadlec, fw, ast, daniel,
	edumazet, keescook, marcelo.leitner, Vlad Buslov
In-Reply-To: <1533538465-23199-1-git-send-email-vladbu@mellanox.com>

Use tcf spinlock to protect police action private data from concurrent
modification during dump. (init already uses tcf spinlock when changing
police action state)

Pass tcf spinlock as estimator lock argument to gen_replace_estimator()
during action init.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 net/sched/act_police.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 3eb5fe60c62c..4777da7caadf 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -138,7 +138,8 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
 
 	if (est) {
 		err = gen_replace_estimator(&police->tcf_bstats, NULL,
-					    &police->tcf_rate_est, NULL,
+					    &police->tcf_rate_est,
+					    &police->tcf_lock,
 					    &police->tcf_lock,
 					    NULL, est);
 		if (err)
@@ -274,14 +275,15 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
 	struct tcf_police *police = to_police(a);
 	struct tc_police opt = {
 		.index = police->tcf_index,
-		.action = police->tcf_action,
-		.mtu = police->tcfp_mtu,
-		.burst = PSCHED_NS2TICKS(police->tcfp_burst),
 		.refcnt = refcount_read(&police->tcf_refcnt) - ref,
 		.bindcnt = atomic_read(&police->tcf_bindcnt) - bind,
 	};
 	struct tcf_t t;
 
+	spin_lock_bh(&police->tcf_lock);
+	opt.action = police->tcf_action;
+	opt.mtu = police->tcfp_mtu;
+	opt.burst = PSCHED_NS2TICKS(police->tcfp_burst);
 	if (police->rate_present)
 		psched_ratecfg_getrate(&opt.rate, &police->rate);
 	if (police->peak_present)
@@ -301,10 +303,12 @@ static int tcf_act_police_dump(struct sk_buff *skb, struct tc_action *a,
 	t.expires = jiffies_to_clock_t(police->tcf_tm.expires);
 	if (nla_put_64bit(skb, TCA_POLICE_TM, sizeof(t), &t, TCA_POLICE_PAD))
 		goto nla_put_failure;
+	spin_unlock_bh(&police->tcf_lock);
 
 	return skb->len;
 
 nla_put_failure:
+	spin_unlock_bh(&police->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
-- 
2.7.5

^ permalink raw reply related


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