Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v6 21/23] crypto: port ChaCha20 to Zinc
From: Eric Biggers @ 2018-10-03  5:56 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: linux-kernel, netdev, linux-crypto, davem, gregkh, Samuel Neves,
	Andy Lutomirski, Jean-Philippe Aumasson
In-Reply-To: <20180925145622.29959-22-Jason@zx2c4.com>

On Tue, Sep 25, 2018 at 04:56:20PM +0200, Jason A. Donenfeld wrote:
> diff --git a/crypto/chacha20_zinc.c b/crypto/chacha20_zinc.c
> new file mode 100644
> index 000000000000..f7d70b3efc31
> --- /dev/null
> +++ b/crypto/chacha20_zinc.c
> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#include <asm/unaligned.h>
> +#include <crypto/algapi.h>
> +#include <crypto/internal/skcipher.h>
> +#include <zinc/chacha20.h>
> +#include <linux/module.h>
> +
> +static int crypto_chacha20_setkey(struct crypto_skcipher *tfm, const u8 *key,
> +				  unsigned int keysize)
> +{
> +	struct chacha20_ctx *ctx = crypto_skcipher_ctx(tfm);
> +
> +	if (keysize != CHACHA20_KEY_SIZE)
> +		return -EINVAL;
> +	chacha20_init(ctx, key, 0);
> +	return 0;
> +}
> +
> +static int crypto_chacha20_crypt(struct skcipher_request *req)
> +{
> +	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +	struct chacha20_ctx *ctx = crypto_skcipher_ctx(tfm);
> +	struct skcipher_walk walk;
> +	simd_context_t simd_context;
> +	int err, i;
> +
> +	err = skcipher_walk_virt(&walk, req, true);
> +	if (unlikely(err))
> +		return err;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->counter); ++i)
> +		ctx->counter[i] = get_unaligned_le32(walk.iv + i * sizeof(u32));
> +

Multiple threads may use the same tfm concurrently, so the tfm context must not
be used to store per-request information such as the IV.

- Eric

^ permalink raw reply

* Re: [PATCH v2 1/9] net: ip_rt_get_source() - use new style struct initializer instead of memset
From: David Miller @ 2018-10-02 23:13 UTC (permalink / raw)
  To: zenczykowski; +Cc: maze, netdev, dsahern
In-Reply-To: <20180930064454.187537-1-zenczykowski@gmail.com>


All 9 patches applied, thanks.

Please provide a proper "0/N" header posting in furture patch series
submissions.

Thank you.

^ permalink raw reply

* Re: pull-request: wireless-drivers 2018-10-01
From: David Miller @ 2018-10-02 23:17 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87a7nxlvxp.fsf@kamboji.qca.qualcomm.com>

From: Kalle Valo <kvalo@codeaurora.org>
Date: Mon, 01 Oct 2018 14:59:14 +0300

> here's a really small pull request to net tree for 4.19. More info below
> and please let me know if there are any problems.

Pulled, thank you.

^ permalink raw reply

* Re: [PATCH v2 1/9] net: ip_rt_get_source() - use new style struct initializer instead of memset
From: Maciej Żenczykowski @ 2018-10-02 23:25 UTC (permalink / raw)
  To: David Miller; +Cc: Linux NetDev, dsahern
In-Reply-To: <20181002.161330.1360820011604705440.davem@davemloft.net>

> Please provide a proper "0/N" header posting in furture patch series
> submissions.
>
> Thank you.

Seeing as all but 2 of these patches were entirely independent should
I have sent them as individual patches instead (with one patch pair)?

Or are series preferred even if I ended up reuploading 9 of 11 with no
changes after the NAK on the two in v1?

Thanks,
Maciej

^ permalink raw reply

* Re: [PATCH net-next v6 11/23] zinc: import Andy Polyakov's Poly1305 ARM and ARM64 implementations
From: Eric Biggers @ 2018-10-03  6:12 UTC (permalink / raw)
  To: Andy Polyakov
  Cc: Jason A. Donenfeld, linux-kernel, netdev, linux-crypto, davem,
	gregkh, Samuel Neves, Andy Lutomirski, Jean-Philippe Aumasson,
	Russell King, linux-arm-kernel
In-Reply-To: <20180925145622.29959-12-Jason@zx2c4.com>

On Tue, Sep 25, 2018 at 04:56:10PM +0200, Jason A. Donenfeld wrote:
> These NEON and non-NEON implementations come from Andy Polyakov's
> implementation, and are included here in raw form without modification,
> so that subsequent commits that fix these up for the kernel can see how
> it has changed. This awkward commit splitting has been requested for the
> ARM[64] implementations in particular.
> 
> While this is CRYPTOGAMS code, the originating code for this happens to
> be the same as OpenSSL's commit 5bb1cd2292b388263a0cc05392bb99141212aa53

Sorry to ruin the fun, but actually there are no Poly1305 implementations in
CRYPTOGAMS (https://github.com/dot-asm/cryptogams).  Nor are there any ChaCha20
implementations.

Andy P., can you please add your Poly1305 and ChaCha20 implementations to the
CRYPTOGAMS repository, so that they have a clear kernel-compatible license?

It would be great if you'd include a kernel-compatible license directly in the
versions in the OpenSSL tree too...

Thanks!

- Eric

^ permalink raw reply

* Re: [PATCH v2 1/9] net: ip_rt_get_source() - use new style struct initializer instead of memset
From: David Miller @ 2018-10-03  0:01 UTC (permalink / raw)
  To: zenczykowski; +Cc: netdev, dsahern
In-Reply-To: <CANP3RGfB=E+ES3K62GTsXVWM7S9EOo=+SAWh1zqcMq0hxs45EQ@mail.gmail.com>

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Tue, 2 Oct 2018 16:25:39 -0700

>> Please provide a proper "0/N" header posting in furture patch series
>> submissions.
>>
>> Thank you.
> 
> Seeing as all but 2 of these patches were entirely independent should
> I have sent them as individual patches instead (with one patch pair)?

They are logically related, so a series is best in this case.

^ permalink raw reply

* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Eric Biggers @ 2018-10-03  6:49 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Ard Biesheuvel, Herbert Xu, LKML, Netdev,
	Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman
In-Reply-To: <CAHmME9r6-DHLwePQGBGcnPUZuZgZ2GtSUX9arsWzLqhKxd=uDA@mail.gmail.com>

On Tue, Oct 02, 2018 at 02:22:47PM +0200, Jason A. Donenfeld wrote:
> On Tue, Oct 2, 2018 at 8:04 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > Also, I still think the name Zinc (zinc is not crypto/) is needlessly
> > divisive and condescending, and unsaying it (in v2 and up) doesn't
> > really work on the Internet (especially since you are still repeating
> > it in your conference talk.)
> > Jason, you seem to hate the existing crypto framework with passion,
> > and the name reflects that.
> 
> It's not divisive or condescending. I don't hate the existing
> framework with a passion -- this is patently false. The name even with
> its original acronym doesn't imply anything essentially negative. I
> see that you've repeatedly misinterpreted it this way -- which is why
> I removed that from v2 for the avoidance of doubt --  but that doesn't
> change the fact that its proper interpretation is not a condescending
> or divisive one.
> 
> Look, people love to bikeshed about names. I'm sure you'll be able to
> CC-in a large crew of people who have opinions in one way or another,
> and this thread could begin to have many voices on that front or on
> multiple fronts. There are real benefits of sticking with the name
> I've given to the library I've spent enormous amounts of time writing.
> And so I'm going to stick with Zinc, since that's why our library is
> called. Sorry. We can talk about this at Plumbers in Vancouver if you
> want, but I think you're not going to get very far with a mailing list
> naming bikeshed.
> 

It's not really about the name, though.  It's actually about the whole way of
thinking about the submission.  Is it a new special library with its own things
going on, or is it just some crypto helper functions?  It's really just the
latter, but you've been presenting it as the former, which is causing a lot of
unnecessary confusion and controversy.  And I think that largely because you've
started from that perspective, you've ended up with some oddities like your own
directories in lib/ and include/ separate from the other existing crypto helper
functions, and proposals to maintain it completely separately from the existing
crypto code including having a separate git tree and separate upstreaming path.

For example, we already have include/crypto/.  What's the difference between
include/crypto/ and include/zinc/?  How do users know whether to include e.g.
<crypto/chacha20.h> rather than <zinc/chacha20.h>?  And are users going to
remember and understand that "zinc" actually means "crypto" but not really?  It
would be much more straightforward and intuitive if we just had
<crypto/chacha20.h>, and your new helper functions were declared in there.

Dismissing these concerns as bikeshedding about the name is missing the point.

- Eric

^ permalink raw reply

* [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kai-Heng Feng @ 2018-10-03  7:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kai-Heng Feng, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, Kalle Valo, David S. Miller, linux-wireless,
	netdev

To avoid the firmware loading race between Bluetooth and WiFi on Intel
8260, load firmware exclusively when BT_INTEL is enabled.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 .../net/wireless/intel/iwlwifi/pcie/trans.c   | 37 ++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index cc8c53dc0ab6..c30d3989e2a8 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -71,6 +71,7 @@
 #include <linux/vmalloc.h>
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
+#include <linux/intel-wifi-bt.h>
 
 #include "iwl-drv.h"
 #include "iwl-trans.h"
@@ -1335,6 +1336,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
 	bool hw_rfkill;
 	int ret;
 
+#if IS_ENABLED(CONFIG_BT_INTEL)
+	void (*firmware_lock_func)(void);
+	void (*firmware_unlock_func)(void);
+#endif
 	/* This may fail if AMT took ownership of the device */
 	if (iwl_pcie_prepare_card_hw(trans)) {
 		IWL_WARN(trans, "Exit HW not ready\n");
@@ -1394,6 +1399,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
 	 * RF-Kill switch is toggled, we will find out after having loaded
 	 * the firmware and return the proper value to the caller.
 	 */
+
 	iwl_enable_fw_load_int(trans);
 
 	/* really make sure rfkill handshake bits are cleared */
@@ -1401,8 +1407,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
 	iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
 
 	/* Load the given image to the HW */
-	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000)
+	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
+#if IS_ENABLED(CONFIG_BT_INTEL)
+		firmware_lock_func = symbol_request(btintel_firmware_lock);
+		firmware_unlock_func = symbol_request(btintel_firmware_unlock);
+		if (!firmware_lock_func || !firmware_unlock_func) {
+			if (firmware_lock_func) {
+				symbol_put(btintel_firmware_lock);
+				firmware_lock_func = NULL;
+			}
+
+			if (firmware_unlock_func) {
+				symbol_put(btintel_firmware_unlock);
+				firmware_unlock_func = NULL;
+			}
+		}
+
+		if (firmware_lock_func)
+			firmware_lock_func();
+#endif
 		ret = iwl_pcie_load_given_ucode_8000(trans, fw);
+
+#if IS_ENABLED(CONFIG_BT_INTEL)
+		if (firmware_unlock_func) {
+			firmware_unlock_func();
+			symbol_put(btintel_firmware_lock);
+			firmware_lock_func = NULL;
+			symbol_put(btintel_firmware_unlock);
+			firmware_unlock_func = NULL;
+		}
+#endif
+	}
 	else
 		ret = iwl_pcie_load_given_ucode(trans, fw);
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Luciano Coelho @ 2018-10-03  7:24 UTC (permalink / raw)
  To: Kai-Heng Feng, linux-kernel
  Cc: Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
	Kalle Valo, David S. Miller, linux-wireless, netdev
In-Reply-To: <20181003071513.13004-3-kai.heng.feng@canonical.com>

On Wed, 2018-10-03 at 15:15 +0800, Kai-Heng Feng wrote:
> To avoid the firmware loading race between Bluetooth and WiFi on Intel
> 8260, load firmware exclusively when BT_INTEL is enabled.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---

Where is this coming from? Can you explain what "the firmware loading
race" is?


>  .../net/wireless/intel/iwlwifi/pcie/trans.c   | 37 ++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> index cc8c53dc0ab6..c30d3989e2a8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> @@ -71,6 +71,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/module.h>
> +#include <linux/intel-wifi-bt.h>

I don't see this upstream.  Is it something that was recently added?
Looks odd...

Regardless, this should also be protected on CONFIG_BT_INTEL.


>  #include "iwl-drv.h"
>  #include "iwl-trans.h"
> @@ -1335,6 +1336,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>  	bool hw_rfkill;
>  	int ret;
>  
> +#if IS_ENABLED(CONFIG_BT_INTEL)
> +	void (*firmware_lock_func)(void);
> +	void (*firmware_unlock_func)(void);
> +#endif
>  	/* This may fail if AMT took ownership of the device */
>  	if (iwl_pcie_prepare_card_hw(trans)) {
>  		IWL_WARN(trans, "Exit HW not ready\n");
> @@ -1394,6 +1399,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>  	 * RF-Kill switch is toggled, we will find out after having loaded
>  	 * the firmware and return the proper value to the caller.
>  	 */
> +

Stray empty line.

>  	iwl_enable_fw_load_int(trans);
>  
>  	/* really make sure rfkill handshake bits are cleared */
> @@ -1401,8 +1407,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>  	iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
>  
>  	/* Load the given image to the HW */
> -	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000)
> +	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
> +#if IS_ENABLED(CONFIG_BT_INTEL)
> +		firmware_lock_func = symbol_request(btintel_firmware_lock);
> +		firmware_unlock_func = symbol_request(btintel_firmware_unlock);
> +		if (!firmware_lock_func || !firmware_unlock_func) {
> +			if (firmware_lock_func) {
> +				symbol_put(btintel_firmware_lock);
> +				firmware_lock_func = NULL;
> +			}
> +
> +			if (firmware_unlock_func) {
> +				symbol_put(btintel_firmware_unlock);
> +				firmware_unlock_func = NULL;
> +			}
> +		}
> +
> +		if (firmware_lock_func)
> +			firmware_lock_func();
> +#endif
>  		ret = iwl_pcie_load_given_ucode_8000(trans, fw);
> +
> +#if IS_ENABLED(CONFIG_BT_INTEL)
> +		if (firmware_unlock_func) {
> +			firmware_unlock_func();
> +			symbol_put(btintel_firmware_lock);
> +			firmware_lock_func = NULL;
> +			symbol_put(btintel_firmware_unlock);
> +			firmware_unlock_func = NULL;
> +		}
> +#endif
> +	}
>  	else
>  		ret = iwl_pcie_load_given_ucode(trans, fw);
>  

I'm not sure I like adding this BT-specific stuff here, especially not
without a detailed explanation.

Did you also send the other patches in this series to linux-wireless? I
can't see them...

--
Cheers,
Luca.

^ permalink raw reply

* Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kai Heng Feng @ 2018-10-03  7:27 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: lkml, Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
	Kalle Valo, David S. Miller, linux-wireless, netdev
In-Reply-To: <2787bccf20b6f647de9f4cafae5fd223e771b167.camel@intel.com>



> On Oct 3, 2018, at 3:24 PM, Luciano Coelho <luciano.coelho@intel.com> wrote:
> 
> On Wed, 2018-10-03 at 15:15 +0800, Kai-Heng Feng wrote:
>> To avoid the firmware loading race between Bluetooth and WiFi on Intel
>> 8260, load firmware exclusively when BT_INTEL is enabled.
>> 
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
> 
> Where is this coming from? Can you explain what "the firmware loading
> race" is?

Looks like the patch is not correctly threaded. I’ll resend the series.

> 
> 
>> .../net/wireless/intel/iwlwifi/pcie/trans.c   | 37 ++++++++++++++++++-
>> 1 file changed, 36 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
>> index cc8c53dc0ab6..c30d3989e2a8 100644
>> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
>> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
>> @@ -71,6 +71,7 @@
>> #include <linux/vmalloc.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/module.h>
>> +#include <linux/intel-wifi-bt.h>
> 
> I don't see this upstream.  Is it something that was recently added?
> Looks odd...
> 
> Regardless, this should also be protected on CONFIG_BT_INTEL.

Thanks, I’ll update this one.

> 
> 
>> #include "iwl-drv.h"
>> #include "iwl-trans.h"
>> @@ -1335,6 +1336,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>> 	bool hw_rfkill;
>> 	int ret;
>> 
>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>> +	void (*firmware_lock_func)(void);
>> +	void (*firmware_unlock_func)(void);
>> +#endif
>> 	/* This may fail if AMT took ownership of the device */
>> 	if (iwl_pcie_prepare_card_hw(trans)) {
>> 		IWL_WARN(trans, "Exit HW not ready\n");
>> @@ -1394,6 +1399,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>> 	 * RF-Kill switch is toggled, we will find out after having loaded
>> 	 * the firmware and return the proper value to the caller.
>> 	 */
>> +
> 
> Stray empty line.
> 
>> 	iwl_enable_fw_load_int(trans);
>> 
>> 	/* really make sure rfkill handshake bits are cleared */
>> @@ -1401,8 +1407,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>> 	iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
>> 
>> 	/* Load the given image to the HW */
>> -	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000)
>> +	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>> +		firmware_lock_func = symbol_request(btintel_firmware_lock);
>> +		firmware_unlock_func = symbol_request(btintel_firmware_unlock);
>> +		if (!firmware_lock_func || !firmware_unlock_func) {
>> +			if (firmware_lock_func) {
>> +				symbol_put(btintel_firmware_lock);
>> +				firmware_lock_func = NULL;
>> +			}
>> +
>> +			if (firmware_unlock_func) {
>> +				symbol_put(btintel_firmware_unlock);
>> +				firmware_unlock_func = NULL;
>> +			}
>> +		}
>> +
>> +		if (firmware_lock_func)
>> +			firmware_lock_func();
>> +#endif
>> 		ret = iwl_pcie_load_given_ucode_8000(trans, fw);
>> +
>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>> +		if (firmware_unlock_func) {
>> +			firmware_unlock_func();
>> +			symbol_put(btintel_firmware_lock);
>> +			firmware_lock_func = NULL;
>> +			symbol_put(btintel_firmware_unlock);
>> +			firmware_unlock_func = NULL;
>> +		}
>> +#endif
>> +	}
>> 	else
>> 		ret = iwl_pcie_load_given_ucode(trans, fw);
>> 
> 
> I'm not sure I like adding this BT-specific stuff here, especially not
> without a detailed explanation.
> 
> Did you also send the other patches in this series to linux-wireless? I
> can't see them…

I’ll resend one soon.

Thanks!

Kai-Heng

> 
> --
> Cheers,
> Luca.

^ permalink raw reply

* Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Matt Chen @ 2018-10-03  7:29 UTC (permalink / raw)
  To: kai.heng.feng
  Cc: luciano.coelho, LKML, johannes.berg, Grumbach, Emmanuel,
	linuxwifi, Kalle Valo, David Miller, linux-wireless, netdev
In-Reply-To: <81B978A9-270E-4C0B-BE67-D05899B1BC3B@canonical.com>

I think Canonical were facing some wifi fw load error from some 8260
earlier module during the BT still loading the fw.
I believe we had later 8260 sku that fixed this issue.

Hi Kai-Heng,

Can you check with OEM for which SKU they are reporting the issue ?
Kai Heng Feng <kai.heng.feng@canonical.com> 於 2018年10月3日 週三 下午3:28寫道:
>
>
>
> > On Oct 3, 2018, at 3:24 PM, Luciano Coelho <luciano.coelho@intel.com> wrote:
> >
> > On Wed, 2018-10-03 at 15:15 +0800, Kai-Heng Feng wrote:
> >> To avoid the firmware loading race between Bluetooth and WiFi on Intel
> >> 8260, load firmware exclusively when BT_INTEL is enabled.
> >>
> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> >> ---
> >
> > Where is this coming from? Can you explain what "the firmware loading
> > race" is?
>
> Looks like the patch is not correctly threaded. I’ll resend the series.
>
> >
> >
> >> .../net/wireless/intel/iwlwifi/pcie/trans.c   | 37 ++++++++++++++++++-
> >> 1 file changed, 36 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> >> index cc8c53dc0ab6..c30d3989e2a8 100644
> >> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> >> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> >> @@ -71,6 +71,7 @@
> >> #include <linux/vmalloc.h>
> >> #include <linux/pm_runtime.h>
> >> #include <linux/module.h>
> >> +#include <linux/intel-wifi-bt.h>
> >
> > I don't see this upstream.  Is it something that was recently added?
> > Looks odd...
> >
> > Regardless, this should also be protected on CONFIG_BT_INTEL.
>
> Thanks, I’ll update this one.
>
> >
> >
> >> #include "iwl-drv.h"
> >> #include "iwl-trans.h"
> >> @@ -1335,6 +1336,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
> >>      bool hw_rfkill;
> >>      int ret;
> >>
> >> +#if IS_ENABLED(CONFIG_BT_INTEL)
> >> +    void (*firmware_lock_func)(void);
> >> +    void (*firmware_unlock_func)(void);
> >> +#endif
> >>      /* This may fail if AMT took ownership of the device */
> >>      if (iwl_pcie_prepare_card_hw(trans)) {
> >>              IWL_WARN(trans, "Exit HW not ready\n");
> >> @@ -1394,6 +1399,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
> >>       * RF-Kill switch is toggled, we will find out after having loaded
> >>       * the firmware and return the proper value to the caller.
> >>       */
> >> +
> >
> > Stray empty line.
> >
> >>      iwl_enable_fw_load_int(trans);
> >>
> >>      /* really make sure rfkill handshake bits are cleared */
> >> @@ -1401,8 +1407,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
> >>      iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
> >>
> >>      /* Load the given image to the HW */
> >> -    if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000)
> >> +    if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
> >> +#if IS_ENABLED(CONFIG_BT_INTEL)
> >> +            firmware_lock_func = symbol_request(btintel_firmware_lock);
> >> +            firmware_unlock_func = symbol_request(btintel_firmware_unlock);
> >> +            if (!firmware_lock_func || !firmware_unlock_func) {
> >> +                    if (firmware_lock_func) {
> >> +                            symbol_put(btintel_firmware_lock);
> >> +                            firmware_lock_func = NULL;
> >> +                    }
> >> +
> >> +                    if (firmware_unlock_func) {
> >> +                            symbol_put(btintel_firmware_unlock);
> >> +                            firmware_unlock_func = NULL;
> >> +                    }
> >> +            }
> >> +
> >> +            if (firmware_lock_func)
> >> +                    firmware_lock_func();
> >> +#endif
> >>              ret = iwl_pcie_load_given_ucode_8000(trans, fw);
> >> +
> >> +#if IS_ENABLED(CONFIG_BT_INTEL)
> >> +            if (firmware_unlock_func) {
> >> +                    firmware_unlock_func();
> >> +                    symbol_put(btintel_firmware_lock);
> >> +                    firmware_lock_func = NULL;
> >> +                    symbol_put(btintel_firmware_unlock);
> >> +                    firmware_unlock_func = NULL;
> >> +            }
> >> +#endif
> >> +    }
> >>      else
> >>              ret = iwl_pcie_load_given_ucode(trans, fw);
> >>
> >
> > I'm not sure I like adding this BT-specific stuff here, especially not
> > without a detailed explanation.
> >
> > Did you also send the other patches in this series to linux-wireless? I
> > can't see them…
>
> I’ll resend one soon.
>
> Thanks!
>
> Kai-Heng
>
> >
> > --
> > Cheers,
> > Luca.
>

^ permalink raw reply

* [RESEND] [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kai-Heng Feng @ 2018-10-03  7:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kai-Heng Feng, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, Kalle Valo, David S. Miller, linux-wireless,
	netdev
In-Reply-To: <20181003073556.28154-1-kai.heng.feng@canonical.com>

To avoid the firmware loading race between Bluetooth and WiFi on Intel
8260, load firmware exclusively when BT_INTEL is enabled.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 .../net/wireless/intel/iwlwifi/pcie/trans.c   | 39 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index cc8c53dc0ab6..bbd7e199e968 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -72,6 +72,10 @@
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 
+#if IS_ENABLED(CONFIG_BT_INTEL)
+#include <linux/intel-wifi-bt.h>
+#endif
+
 #include "iwl-drv.h"
 #include "iwl-trans.h"
 #include "iwl-csr.h"
@@ -1335,6 +1339,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
 	bool hw_rfkill;
 	int ret;
 
+#if IS_ENABLED(CONFIG_BT_INTEL)
+	void (*firmware_lock_func)(void);
+	void (*firmware_unlock_func)(void);
+#endif
 	/* This may fail if AMT took ownership of the device */
 	if (iwl_pcie_prepare_card_hw(trans)) {
 		IWL_WARN(trans, "Exit HW not ready\n");
@@ -1401,8 +1409,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
 	iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
 
 	/* Load the given image to the HW */
-	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000)
+	if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
+#if IS_ENABLED(CONFIG_BT_INTEL)
+		firmware_lock_func = symbol_request(btintel_firmware_lock);
+		firmware_unlock_func = symbol_request(btintel_firmware_unlock);
+		if (!firmware_lock_func || !firmware_unlock_func) {
+			if (firmware_lock_func) {
+				symbol_put(btintel_firmware_lock);
+				firmware_lock_func = NULL;
+			}
+
+			if (firmware_unlock_func) {
+				symbol_put(btintel_firmware_unlock);
+				firmware_unlock_func = NULL;
+			}
+		}
+
+		if (firmware_lock_func)
+			firmware_lock_func();
+#endif
 		ret = iwl_pcie_load_given_ucode_8000(trans, fw);
+
+#if IS_ENABLED(CONFIG_BT_INTEL)
+		if (firmware_unlock_func) {
+			firmware_unlock_func();
+			symbol_put(btintel_firmware_lock);
+			firmware_lock_func = NULL;
+			symbol_put(btintel_firmware_unlock);
+			firmware_unlock_func = NULL;
+		}
+#endif
+	}
 	else
 		ret = iwl_pcie_load_given_ucode(trans, fw);
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kai Heng Feng @ 2018-10-03  7:38 UTC (permalink / raw)
  To: Matt Chen
  Cc: Luciano Coelho, LKML, johannes.berg, Grumbach, Emmanuel,
	linuxwifi, Kalle Valo, David Miller, linux-wireless, netdev
In-Reply-To: <CALx5=V_0fyTcpTdYh=ZOLWL79cop1F=5GyMuK6S9QnP5qHKouQ@mail.gmail.com>



> On Oct 3, 2018, at 3:29 PM, Matt Chen <mattsled@gmail.com> wrote:
> 
> I think Canonical were facing some wifi fw load error from some 8260
> earlier module during the BT still loading the fw.
> I believe we had later 8260 sku that fixed this issue.

But there are already 8260 that is affected by this bug in the wild.

Search "Bluetooth: hci0: Failed to send firmware data (-38)” and there are lots of user are affected.

Kai-Heng

> 
> Hi Kai-Heng,
> 
> Can you check with OEM for which SKU they are reporting the issue ?
> Kai Heng Feng <kai.heng.feng@canonical.com> 於 2018年10月3日 週三 下午3:28寫道:
>> 
>> 
>> 
>>> On Oct 3, 2018, at 3:24 PM, Luciano Coelho <luciano.coelho@intel.com> wrote:
>>> 
>>> On Wed, 2018-10-03 at 15:15 +0800, Kai-Heng Feng wrote:
>>>> To avoid the firmware loading race between Bluetooth and WiFi on Intel
>>>> 8260, load firmware exclusively when BT_INTEL is enabled.
>>>> 
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>> ---
>>> 
>>> Where is this coming from? Can you explain what "the firmware loading
>>> race" is?
>> 
>> Looks like the patch is not correctly threaded. I’ll resend the series.
>> 
>>> 
>>> 
>>>> .../net/wireless/intel/iwlwifi/pcie/trans.c   | 37 ++++++++++++++++++-
>>>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
>>>> index cc8c53dc0ab6..c30d3989e2a8 100644
>>>> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
>>>> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
>>>> @@ -71,6 +71,7 @@
>>>> #include <linux/vmalloc.h>
>>>> #include <linux/pm_runtime.h>
>>>> #include <linux/module.h>
>>>> +#include <linux/intel-wifi-bt.h>
>>> 
>>> I don't see this upstream.  Is it something that was recently added?
>>> Looks odd...
>>> 
>>> Regardless, this should also be protected on CONFIG_BT_INTEL.
>> 
>> Thanks, I’ll update this one.
>> 
>>> 
>>> 
>>>> #include "iwl-drv.h"
>>>> #include "iwl-trans.h"
>>>> @@ -1335,6 +1336,10 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>>>>     bool hw_rfkill;
>>>>     int ret;
>>>> 
>>>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>>>> +    void (*firmware_lock_func)(void);
>>>> +    void (*firmware_unlock_func)(void);
>>>> +#endif
>>>>     /* This may fail if AMT took ownership of the device */
>>>>     if (iwl_pcie_prepare_card_hw(trans)) {
>>>>             IWL_WARN(trans, "Exit HW not ready\n");
>>>> @@ -1394,6 +1399,7 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>>>>      * RF-Kill switch is toggled, we will find out after having loaded
>>>>      * the firmware and return the proper value to the caller.
>>>>      */
>>>> +
>>> 
>>> Stray empty line.
>>> 
>>>>     iwl_enable_fw_load_int(trans);
>>>> 
>>>>     /* really make sure rfkill handshake bits are cleared */
>>>> @@ -1401,8 +1407,37 @@ static int iwl_trans_pcie_start_fw(struct iwl_trans *trans,
>>>>     iwl_write32(trans, CSR_UCODE_DRV_GP1_CLR, CSR_UCODE_SW_BIT_RFKILL);
>>>> 
>>>>     /* Load the given image to the HW */
>>>> -    if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000)
>>>> +    if (trans->cfg->device_family >= IWL_DEVICE_FAMILY_8000) {
>>>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>>>> +            firmware_lock_func = symbol_request(btintel_firmware_lock);
>>>> +            firmware_unlock_func = symbol_request(btintel_firmware_unlock);
>>>> +            if (!firmware_lock_func || !firmware_unlock_func) {
>>>> +                    if (firmware_lock_func) {
>>>> +                            symbol_put(btintel_firmware_lock);
>>>> +                            firmware_lock_func = NULL;
>>>> +                    }
>>>> +
>>>> +                    if (firmware_unlock_func) {
>>>> +                            symbol_put(btintel_firmware_unlock);
>>>> +                            firmware_unlock_func = NULL;
>>>> +                    }
>>>> +            }
>>>> +
>>>> +            if (firmware_lock_func)
>>>> +                    firmware_lock_func();
>>>> +#endif
>>>>             ret = iwl_pcie_load_given_ucode_8000(trans, fw);
>>>> +
>>>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>>>> +            if (firmware_unlock_func) {
>>>> +                    firmware_unlock_func();
>>>> +                    symbol_put(btintel_firmware_lock);
>>>> +                    firmware_lock_func = NULL;
>>>> +                    symbol_put(btintel_firmware_unlock);
>>>> +                    firmware_unlock_func = NULL;
>>>> +            }
>>>> +#endif
>>>> +    }
>>>>     else
>>>>             ret = iwl_pcie_load_given_ucode(trans, fw);
>>>> 
>>> 
>>> I'm not sure I like adding this BT-specific stuff here, especially not
>>> without a detailed explanation.
>>> 
>>> Did you also send the other patches in this series to linux-wireless? I
>>> can't see them…
>> 
>> I’ll resend one soon.
>> 
>> Thanks!
>> 
>> Kai-Heng
>> 
>>> 
>>> --
>>> Cheers,
>>> Luca.
>> 

^ permalink raw reply

* [PATCH net 0/3] net: qualcomm: rmnet: Updates 2018-10-02
From: Subash Abhinov Kasiviswanathan @ 2018-10-03  0:52 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan

This series is a set of small fixes for rmnet driver

Patch 1 is a fix for a scenario reported by syzkaller 
Patch  2 & 3 are fixes for incorrect allocation flags

Sean Tranchetti (1):
  net: qualcomm: rmnet: Skip processing loopback packets

Subash Abhinov Kasiviswanathan (2):
  net: qualcomm: rmnet: Fix incorrect allocation flag in transmit
  net: qualcomm: rmnet: Fix incorrect allocation flag in receive path

 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH net 1/3] net: qualcomm: rmnet: Skip processing loopback packets
From: Subash Abhinov Kasiviswanathan @ 2018-10-03  0:52 UTC (permalink / raw)
  To: davem, netdev; +Cc: Sean Tranchetti, Subash Abhinov Kasiviswanathan
In-Reply-To: <1538527923-29562-1-git-send-email-subashab@codeaurora.org>

From: Sean Tranchetti <stranche@codeaurora.org>

RMNET RX handler was processing invalid packets that were
originally sent on the real device and were looped back via
dev_loopback_xmit(). This was detected using syzkaller.

Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 7fd86d4..6908b26 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -189,6 +189,9 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
 	if (!skb)
 		goto done;
 
+	if (skb->pkt_type == PACKET_LOOPBACK)
+		return RX_HANDLER_PASS;
+
 	dev = skb->dev;
 	port = rmnet_get_port(dev);
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net 2/3] net: qualcomm: rmnet: Fix incorrect allocation flag in transmit
From: Subash Abhinov Kasiviswanathan @ 2018-10-03  0:52 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1538527923-29562-1-git-send-email-subashab@codeaurora.org>

The incoming skb needs to be reallocated in case the headroom
is not sufficient to add the MAP header. This allocation needs to
be atomic otherwise it results in the following splat

[32805.801456] BUG: sleeping function called from invalid context
[32805.841141] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[32805.904773] task: ffffffd7c5f62280 task.stack: ffffff80464a8000
[32805.910851] pc : ___might_sleep+0x180/0x188
[32805.915143] lr : ___might_sleep+0x180/0x188
[32806.131520] Call trace:
[32806.134041]  ___might_sleep+0x180/0x188
[32806.137980]  __might_sleep+0x50/0x84
[32806.141653]  __kmalloc_track_caller+0x80/0x3bc
[32806.146215]  __kmalloc_reserve+0x3c/0x88
[32806.150241]  pskb_expand_head+0x74/0x288
[32806.154269]  rmnet_egress_handler+0xb0/0x1d8
[32806.162239]  rmnet_vnd_start_xmit+0xc8/0x13c
[32806.166627]  dev_hard_start_xmit+0x148/0x280
[32806.181181]  sch_direct_xmit+0xa4/0x198
[32806.185125]  __qdisc_run+0x1f8/0x310
[32806.188803]  net_tx_action+0x23c/0x26c
[32806.192655]  __do_softirq+0x220/0x408
[32806.196420]  do_softirq+0x4c/0x70

Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 6908b26..1f98d65 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -147,7 +147,7 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 	}
 
 	if (skb_headroom(skb) < required_headroom) {
-		if (pskb_expand_head(skb, required_headroom, 0, GFP_KERNEL))
+		if (pskb_expand_head(skb, required_headroom, 0, GFP_ATOMIC))
 			return -ENOMEM;
 	}
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net 3/3] net: qualcomm: rmnet: Fix incorrect allocation flag in receive path
From: Subash Abhinov Kasiviswanathan @ 2018-10-03  0:52 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan, Sean Tranchetti
In-Reply-To: <1538527923-29562-1-git-send-email-subashab@codeaurora.org>

The incoming skb needs to be reallocated in case the headroom
is not sufficient to adjust the ethernet header. This allocation
needs to be atomic otherwise it results in this splat

 [<600601bb>] ___might_sleep+0x185/0x1a3
 [<603f6314>] ? _raw_spin_unlock_irqrestore+0x0/0x27
 [<60069bb0>] ? __wake_up_common_lock+0x95/0xd1
 [<600602b0>] __might_sleep+0xd7/0xe2
 [<60065598>] ? enqueue_task_fair+0x112/0x209
 [<600eea13>] __kmalloc_track_caller+0x5d/0x124
 [<600ee9b6>] ? __kmalloc_track_caller+0x0/0x124
 [<602696d5>] __kmalloc_reserve.isra.34+0x30/0x7e
 [<603f629b>] ? _raw_spin_lock_irqsave+0x0/0x3d
 [<6026b744>] pskb_expand_head+0xbf/0x310
 [<6025ca6a>] rmnet_rx_handler+0x7e/0x16b
 [<6025c9ec>] ? rmnet_rx_handler+0x0/0x16b
 [<6027ad0c>] __netif_receive_skb_core+0x301/0x96f
 [<60033c17>] ? set_signals+0x0/0x40
 [<6027bbcb>] __netif_receive_skb+0x24/0x8e

Fixes: 74692caf1b0b ("net: qualcomm: rmnet: Process packets over ethernet")
Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 1f98d65..11167ab 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -113,7 +113,7 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 	struct sk_buff *skbn;
 
 	if (skb->dev->type == ARPHRD_ETHER) {
-		if (pskb_expand_head(skb, ETH_HLEN, 0, GFP_KERNEL)) {
+		if (pskb_expand_head(skb, ETH_HLEN, 0, GFP_ATOMIC)) {
 			kfree_skb(skb);
 			return;
 		}
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCHv4 bpf-next 00/13] Add socket lookup support
From: Daniel Borkmann @ 2018-10-03  1:08 UTC (permalink / raw)
  To: Joe Stringer
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez
In-Reply-To: <20181002203541.26599-1-joe@wand.net.nz>

On 10/02/2018 10:35 PM, Joe Stringer wrote:
> This series proposes a new helper for the BPF API which allows BPF programs to
> perform lookups for sockets in a network namespace. This would allow programs
> to determine early on in processing whether the stack is expecting to receive
> the packet, and perform some action (eg drop, forward somewhere) based on this
> information.
> 
> The series is structured roughly into:
> * Misc refactor
> * Add the socket pointer type
> * Add reference tracking to ensure that socket references are freed
> * Extend the BPF API to add sk_lookup_xxx() / sk_release() functions
> * Add tests/documentation
> 
> The helper proposed in this series includes a parameter for a tuple which must
> be filled in by the caller to determine the socket to look up. The simplest
> case would be filling with the contents of the packet, ie mapping the packet's
> 5-tuple into the parameter. In common cases, it may alternatively be useful to
> reverse the direction of the tuple and perform a lookup, to find the socket
> that initiates this connection; and if the BPF program ever performs a form of
> IP address translation, it may further be useful to be able to look up
> arbitrary tuples that are not based upon the packet, but instead based on state
> held in BPF maps or hardcoded in the BPF program.
> 
> Currently, access into the socket's fields are limited to those which are
> otherwise already accessible, and are restricted to read-only access.
> 
> Changes since v3:
> * New patch: "bpf: Reuse canonical string formatter for ctx errs"
> * Add PTR_TO_SOCKET to is_ctx_reg().
> * Add a few new checks to prevent mixing of socket/non-socket pointers.
> * Swap order of checks in sock_filter_is_valid_access().
> * Prefix register spill macros with "bpf_".
> * Add acks from previous round
> * Rebase
> 
> Changes since v2:
> * New patch: "selftests/bpf: Generalize dummy program types".
>   This enables adding verifier tests for socket lookup with tail calls.
> * Define the semantics of the new helpers more clearly in uAPI header.
> * Fix release of caller_net when netns is not specified.
> * Use skb->sk to find caller net when skb->dev is unavailable.
> * Fix build with !CONFIG_NET.
> * Replace ptr_id defensive coding when releasing reference state with an
>   internal error (-EFAULT).
> * Remove flags argument to sk_release().
> * Add several new assembly tests suggested by Daniel.
> * Add a few new C tests.
> * Fix typo in verifier error message.
> 
> Changes since v1:
> * Limit netns_id field to 32 bits
> * Reuse reg_type_mismatch() in more places
> * Reduce the number of passes at convert_ctx_access()
> * Replace ptr_id defensive coding when releasing reference state with an
>   internal error (-EFAULT)
> * Rework 'struct bpf_sock_tuple' to allow passing a packet pointer
> * Allow direct packet access from helper
> * Fix compile error with CONFIG_IPV6 enabled
> * Improve commit messages
> 
> Changes since RFC:
> * Split up sk_lookup() into sk_lookup_tcp(), sk_lookup_udp().
> * Only take references on the socket when necessary.
>   * Make sk_release() only free the socket reference in this case.
> * Fix some runtime reference leaks:
>   * Disallow BPF_LD_[ABS|IND] instructions while holding a reference.
>   * Disallow bpf_tail_call() while holding a reference.
> * Prevent the same instruction being used for reference and other
>   pointer type.
> * Simplify locating copies of a reference during helper calls by caching
>   the pointer id from the caller.
> * Fix kbuild compilation warnings with particular configs.
> * Improve code comments describing the new verifier pieces.
> * Tested by Nitin
> 
> This tree is also available at:
> https://github.com/joestringer/linux/commits/submit/sk-lookup-v4
> 
> Joe Stringer (13):
>   bpf: Add iterator for spilled registers
>   bpf: Simplify ptr_min_max_vals adjustment
>   bpf: Reuse canonical string formatter for ctx errs
>   bpf: Generalize ptr_or_null regs check
>   bpf: Add PTR_TO_SOCKET verifier type
>   bpf: Macrofy stack state copy
>   bpf: Add reference tracking to verifier
>   bpf: Add helper to retrieve socket in BPF
>   selftests/bpf: Generalize dummy program types
>   selftests/bpf: Add tests for reference tracking
>   libbpf: Support loading individual progs
>   selftests/bpf: Add C tests for reference tracking
>   Documentation: Describe bpf reference tracking

Applied to bpf-next, thanks Joe!

^ permalink raw reply

* Re: [PATCH 14/16] netfilter: Replace spin_is_locked() with lockdep
From: Jozsef Kadlecsik @ 2018-10-03  8:29 UTC (permalink / raw)
  To: Lance Roy
  Cc: linux-kernel, Paul E. McKenney, Pablo Neira Ayuso,
	Florian Westphal, David S. Miller, netfilter-devel, coreteam,
	netdev
In-Reply-To: <20181003053902.6910-15-ldr709@gmail.com>

On Tue, 2 Oct 2018, Lance Roy wrote:

> lockdep_assert_held() is better suited to checking locking requirements,
> since it won't get confused when someone else holds the lock. This is
> also a step towards possibly removing spin_is_locked().
> 
> Signed-off-by: Lance Roy <ldr709@gmail.com>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: <netfilter-devel@vger.kernel.org>
> Cc: <coreteam@netfilter.org>
> Cc: <netdev@vger.kernel.org>
> ---
>  net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Best regards,
Jozsef

> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> index 8a33dac4e805..e287da68d5fa 100644
> --- a/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -15,7 +15,7 @@
>  
>  #define __ipset_dereference_protected(p, c)	rcu_dereference_protected(p, c)
>  #define ipset_dereference_protected(p, set) \
> -	__ipset_dereference_protected(p, spin_is_locked(&(set)->lock))
> +	__ipset_dereference_protected(p, lockdep_is_held(&(set)->lock))
>  
>  #define rcu_dereference_bh_nfnl(p)	rcu_dereference_bh_check(p, 1)
>  
> -- 
> 2.19.0
> 
> 

-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

^ permalink raw reply

* Re: [PATCH v2 net] inet: frags: rework rhashtable dismantle
From: Eric Dumazet @ 2018-10-03  1:52 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller
  Cc: netdev, Eric Dumazet, Thomas Graf, Herbert Xu
In-Reply-To: <20181002054917.37102-1-edumazet@google.com>



On 10/01/2018 10:49 PM, Eric Dumazet wrote:
> syszbot found an interesting use-after-free [1] happening
> while IPv4 fragment rhashtable was destroyed at netns dismantle.
>

David, please do not apply this patch.

I am working on another version, a bit more polished and allowing for more parallelism.

Thanks

^ permalink raw reply

* Re: [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kalle Valo @ 2018-10-03  8:58 UTC (permalink / raw)
  To: Luciano Coelho
  Cc: Kai-Heng Feng, linux-kernel, Johannes Berg, Emmanuel Grumbach,
	Intel Linux Wireless, David S. Miller, linux-wireless, netdev
In-Reply-To: <2787bccf20b6f647de9f4cafae5fd223e771b167.camel@intel.com>

Luciano Coelho <luciano.coelho@intel.com> writes:

>> +#if IS_ENABLED(CONFIG_BT_INTEL)
>> +		if (firmware_unlock_func) {
>> +			firmware_unlock_func();
>> +			symbol_put(btintel_firmware_lock);
>> +			firmware_lock_func = NULL;
>> +			symbol_put(btintel_firmware_unlock);
>> +			firmware_unlock_func = NULL;
>> +		}
>> +#endif
>> +	}
>>  	else
>>  		ret = iwl_pcie_load_given_ucode(trans, fw);
>>  
>
> I'm not sure I like adding this BT-specific stuff here, especially not
> without a detailed explanation.

This looks like an ugly hack and the commit log tells nothing. This
really needs strong justifications to even consider doing something like
this.

-- 
Kalle Valo

^ permalink raw reply

* Re: [RESEND] [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kalle Valo @ 2018-10-03  9:10 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: linux-kernel, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, David S. Miller, linux-wireless, netdev
In-Reply-To: <20181003073556.28154-3-kai.heng.feng@canonical.com>

Kai-Heng Feng <kai.heng.feng@canonical.com> writes:

> To avoid the firmware loading race between Bluetooth and WiFi on Intel
> 8260, load firmware exclusively when BT_INTEL is enabled.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Still the commit log tells nothing about the actual problem which makes
review impossible.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH 00/22] various dynamic_debug patches
From: Rafael J. Wysocki @ 2018-10-03  9:25 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Jason Baron, Andrew Morton, linux-kernel, Greg Kroah-Hartman,
	netdev, Petr Mladek, Steven Rostedt, linux-btrfs, linux-acpi, x86
In-Reply-To: <4563778.42gAD4asSf@aspire.rjw.lan>

On Thursday, September 20, 2018 10:05:00 AM CEST Rafael J. Wysocki wrote:
> On Thursday, September 20, 2018 12:04:22 AM CEST Rasmus Villemoes wrote:
> > This started as an experiment to see how hard it would be to change
> > the four pointers in struct _ddebug into relative offsets, a la
> > CONFIG_GENERIC_BUG_RELATIVE_POINTERS, thus saving 16 bytes per
> > pr_debug site (and thus exactly making up for the extra space used by
> > the introduction of jump labels in 9049fc74). I stumbled on a few
> > things that are probably worth fixing regardless of whether the latter
> > half of this series is deemed worthwhile.
> > 
> > Patch relationships: 1-2, 3-4, 5-6 and 15-16 can be applied
> > individually, though 2, 4 and 6 probably makes most sense in the
> > context of the final goal of the series.
> 
> OK, I can take the [15-16/22] separately.

And they have been applied now.

Thanks,
Rafael

^ permalink raw reply

* Re: [RESEND] [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kai Heng Feng @ 2018-10-03  9:27 UTC (permalink / raw)
  To: Kalle Valo
  Cc: LKML, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, David S. Miller,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87y3bfiees.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>



> On Oct 3, 2018, at 5:10 PM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> 
> Kai-Heng Feng <kai.heng.feng-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
> 
>> To avoid the firmware loading race between Bluetooth and WiFi on Intel
>> 8260, load firmware exclusively when BT_INTEL is enabled.
>> 
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> 
> Still the commit log tells nothing about the actual problem which makes
> review impossible.

Sorry for that. The first two patches [1] only sends to linux-bluetooth and LMKL.

I don’t know what really happened at hardware/firmware level, but making btusb and iwlwifi load firmware sequentially can workaround the issue.

Matt Chen may be able to explain this issue with more detail.

[1] https://lkml.org/lkml/2018/10/3/322

> 
> -- 
> Kalle Valo

^ permalink raw reply

* Re: [RESEND] [PATCH 3/3] iwlwifi: Load firmware exclusively for Intel WiFi
From: Kalle Valo @ 2018-10-03  9:40 UTC (permalink / raw)
  To: Kai Heng Feng
  Cc: LKML, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
	Intel Linux Wireless, David S. Miller, linux-wireless, netdev
In-Reply-To: <265BAF93-4BFF-4850-92F7-80AC39496473@canonical.com>

Kai Heng Feng <kai.heng.feng@canonical.com> writes:

>> On Oct 3, 2018, at 5:10 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> 
>> Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
>> 
>>> To avoid the firmware loading race between Bluetooth and WiFi on Intel
>>> 8260, load firmware exclusively when BT_INTEL is enabled.
>>> 
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> 
>> Still the commit log tells nothing about the actual problem which makes
>> review impossible.
>
> Sorry for that. The first two patches [1] only sends to linux-bluetooth and LMKL.

For a patchset like this you should CC linux-wireless for all patches,
otherwise people just get confused. And even more so as patch 3 seems to
depend on the other patches.

> I don’t know what really happened at hardware/firmware level, but
> making btusb and iwlwifi load firmware sequentially can workaround the
> issue.

We don't apply ugly workarounds without understanding the issue.

> Matt Chen may be able to explain this issue with more detail.

Then you need to work with Matt so that the issue is properly explained
in the commit log.

-- 
Kalle Valo

^ 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