* Re: [PATCH net-next 2/2] ila: Cache a route to translated address
From: Jiri Benc @ 2016-10-14 9:04 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Tom Herbert, davem, netdev, kernel-team
In-Reply-To: <58007996.2070500@cumulusnetworks.com>
On Thu, 13 Oct 2016 23:22:14 -0700, Roopa Prabhu wrote:
> This removes the last and only user of lwt orig_output. we can drop it
> subsequently. But since orig_input is still in use, probably better to keep it
> around for symmetry and for other uses in the future.
If it's no longer used, let's remove it. It can be always added later
again if needed. We don't keep things just because they maybe can be
used for something in the future.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH net-next 1/2] lwtunnel: Add destroy state operation
From: Jiri Benc @ 2016-10-14 8:59 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, roopa, kernel-team
In-Reply-To: <20161014005743.288956-2-tom@herbertland.com>
On Thu, 13 Oct 2016 17:57:42 -0700, Tom Herbert wrote:
> @@ -43,13 +44,11 @@ struct lwtunnel_encap_ops {
> int (*get_encap_size)(struct lwtunnel_state *lwtstate);
> int (*cmp_encap)(struct lwtunnel_state *a, struct lwtunnel_state *b);
> int (*xmit)(struct sk_buff *skb);
> + void (*destroy_state)(struct lwtunnel_state *lws);
> };
Could you add destroy_state next to build_state? Seems weird to have
those two scattered at the opposite ends of the structure. Looks good
otherwise.
Thanks,
Jiri
^ permalink raw reply
* RE: Kernel 4.6.7-rt13: Intel Ethernet driver igb causes huge latencies in cyclictest
From: Koehrer Mathias (ETAS/ESW5) @ 2016-10-14 8:58 UTC (permalink / raw)
To: Julia Cartwright
Cc: Williams, Mitch A, Kirsher, Jeffrey T,
linux-rt-users@vger.kernel.org, Sebastian Andrzej Siewior,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, Greg
In-Reply-To: <20161013161839.GV10625@jcartwri.amer.corp.natinst.com>
[-- Attachment #1: Type: text/plain, Size: 9462 bytes --]
Hi Julia,
> Have you tested on a vanilla (non-RT) kernel? I doubt there is anything RT specific
> about what you are seeing, but it might be nice to get confirmation. Also, bisection
> would probably be easier if you confirm on a vanilla kernel.
>
> I find it unlikely that it's a kernel config option that changed which regressed you, but
> instead was a code change to a driver. Which driver is now the question, and the
> surface area is still big (processor mapping attributes for this region, PCI root
> complex configuration, PCI brige configuration, igb driver itself, etc.).
>
> Big enough that I'd recommend a bisection. It looks like a bisection between 3.18
> and 4.8 would take you about 18 tries to narrow down, assuming all goes well.
>
I have now repeated my tests using the vanilla kernel.
There I got the very same issue.
Using kernel 4.0 is fine, however starting with kernel 4.1, the issue appears.
Here is my exact (reproducible) test description:
I applied the following patch to the kernel to get the igb trace.
This patch instruments the igb_rd32() function to measure the call
to readl() which is used to access registers of the igb NIC.
++++++++++++++++++ BEGIN PATCH ++++++++++++++++++++++++++++++++++++
Index: linux-4.8/drivers/net/ethernet/intel/igb/trace.h
===================================================================
--- /dev/null
+++ linux-4.8/drivers/net/ethernet/intel/igb/trace.h
@@ -0,0 +1,34 @@
+
+#if !defined(_TRACE_IGB_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_IGB_H_
+
+#include <linux/tracepoint.h>
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM igb
+
+
+#define _TRACE_H_
+
+
+TRACE_EVENT(igb,
+ TP_PROTO(u32 val),
+ TP_ARGS(val),
+ TP_STRUCT__entry(
+ __field(u32, val)
+ ),
+ TP_fast_assign(
+ __entry->val = val;
+ ),
+ TP_printk("val: %u",
+ __entry->val)
+);
+
+
+#endif
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH drivers/net/ethernet/intel/igb
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+
+#include <trace/define_trace.h>
Index: linux-4.8/drivers/net/ethernet/intel/igb/Makefile
===================================================================
--- linux-4.8.orig/drivers/net/ethernet/intel/igb/Makefile
+++ linux-4.8/drivers/net/ethernet/intel/igb/Makefile
@@ -28,6 +28,7 @@
#
# Makefile for the Intel(R) 82575 PCI-Express ethernet driver
#
+ccflags-y += -I.
obj-$(CONFIG_IGB) += igb.o
Index: linux-4.8/drivers/net/ethernet/intel/igb/igb_main.c
===================================================================
--- linux-4.8.orig/drivers/net/ethernet/intel/igb/igb_main.c
+++ linux-4.8/drivers/net/ethernet/intel/igb/igb_main.c
@@ -57,6 +57,9 @@
#include <linux/i2c.h>
#include "igb.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
#define MAJ 5
#define MIN 3
#define BUILD 0
@@ -753,7 +756,9 @@ u32 igb_rd32(struct e1000_hw *hw, u32 re
if (E1000_REMOVED(hw_addr))
return ~value;
+ trace_igb(801);
value = readl(&hw_addr[reg]);
+ trace_igb(802);
/* reads should not return all F's */
if (!(~value) && (!reg || !(~readl(hw_addr)))) {
++++++++++++++++++ END PATCH ++++++++++++++++++++++++++++++++++++
I build the kernel with this patch applied, rebooted the PC to run this kernel and used the
following script for my test.
++++++++++++++++++ BEGIN SCRIPT +++++++++++++++++++++++++++++++++
#!/bin/bash
for f in /sys/devices/system/cpu/cpu[0-9]*/cpufreq/scaling_governor ; do
if [ -w $f ]; then
echo "performance" > $f
fi
done
if true; then
rmmod igb
modprobe igb
ethtool -L eth2 combined 1
ifconfig eth2 up 192.168.100.111
fi
ifconfig
mount /sys/kernel/debug
( cd /sys/kernel/debug/tracing
echo 0 > tracing_on
echo 0 > events/enable
echo 1 > events/igb/enable
echo "print-parent" > trace_options
echo "latency-format" > trace_options
echo 1 > tracing_on
sleep 4
cat trace
)
++++++++++++++++++ END SCRIPT +++++++++++++++++++++++++++++++++
The results of this for kernel 4.0:
[...]
kworker/-1239 3...1 49699046us : igb: val: 801
kworker/-1239 3...1 49699047us : igb: val: 802
kworker/-1239 3...1 49699047us : igb: val: 801
kworker/-1239 3...1 49699048us+: igb: val: 802
kworker/-1239 3...1 49699099us : igb: val: 801
kworker/-1239 3...1 49699100us : igb: val: 802
kworker/-1239 3...1 49699100us : igb: val: 801
kworker/-1239 3...1 49699102us : igb: val: 802
kworker/-1239 3...1 49699102us : igb: val: 801
kworker/-1239 3...1 49699103us : igb: val: 802
kworker/-1239 3...1 49699103us : igb: val: 801
kworker/-1239 3...1 49699104us : igb: val: 802
kworker/-1239 3...1 49699104us : igb: val: 801
kworker/-1239 3...1 49699105us : igb: val: 802
kworker/-1239 3...1 49699105us : igb: val: 801
kworker/-1239 3...1 49699107us : igb: val: 802
kworker/-1239 3...1 49699107us : igb: val: 801
kworker/-1239 3...1 49699108us : igb: val: 802
kworker/-1239 3...1 49699108us : igb: val: 801
kworker/-1239 3...1 49699109us : igb: val: 802
kworker/-1239 3...1 49699109us : igb: val: 801
kworker/-1239 3...1 49699110us : igb: val: 802
kworker/-1239 3...1 49699110us : igb: val: 801
kworker/-1239 3...1 49699111us : igb: val: 802
kworker/-1239 3...1 49699111us : igb: val: 801
kworker/-1239 3...1 49699113us+: igb: val: 802
kworker/-1239 3...1 49699163us : igb: val: 801
kworker/-1239 3...1 49699164us : igb: val: 802
kworker/-1239 3...1 49699164us : igb: val: 801
kworker/-1239 3...1 49699166us : igb: val: 802
kworker/-1239 3...1 49699166us : igb: val: 801
kworker/-1239 3...1 49699167us : igb: val: 802
kworker/-1239 3...1 49699167us : igb: val: 801
kworker/-1239 3...1 49699168us : igb: val: 802
kworker/-1239 3...1 49699168us : igb: val: 801
kworker/-1239 3...1 49699169us : igb: val: 802
kworker/-1239 3...1 49699169us : igb: val: 801
kworker/-1239 3...1 49699170us : igb: val: 802
kworker/-1239 3...1 49699171us : igb: val: 801
kworker/-1239 3...1 49699173us : igb: val: 802
kworker/-1239 3...1 49699173us : igb: val: 801
kworker/-1239 3...1 49699174us : igb: val: 802
The results of this for kernel 4.1:
[...]
kworker/-1333 2...1 75697302us+: igb: val: 801
kworker/-1333 2...1 75697323us+: igb: val: 802
kworker/-1333 2...1 75697373us+: igb: val: 801
kworker/-1333 2...1 75697395us : igb: val: 802
kworker/-1333 2...1 75697395us+: igb: val: 801
kworker/-1333 2...1 75697415us : igb: val: 802
kworker/-1333 2...1 75697416us+: igb: val: 801
kworker/-1333 2...1 75697436us : igb: val: 802
kworker/-1333 2...1 75697436us+: igb: val: 801
kworker/-1333 2...1 75697457us : igb: val: 802
kworker/-1333 2...1 75697457us+: igb: val: 801
kworker/-1333 2...1 75697478us : igb: val: 802
kworker/-1333 2...1 75697478us+: igb: val: 801
kworker/-1333 2...1 75697499us : igb: val: 802
kworker/-1333 2...1 75697499us+: igb: val: 801
kworker/-1333 2...1 75697520us : igb: val: 802
kworker/-1333 2...1 75697520us+: igb: val: 801
kworker/-1333 2...1 75697541us : igb: val: 802
kworker/-1333 2...1 75697541us+: igb: val: 801
kworker/-1333 2...1 75697562us : igb: val: 802
kworker/-1333 2...1 75697562us+: igb: val: 801
kworker/-1333 2...1 75697583us : igb: val: 802
kworker/-1333 2...1 75697583us+: igb: val: 801
kworker/-1333 2...1 75697604us+: igb: val: 802
kworker/-1333 2...1 75697654us+: igb: val: 801
kworker/-1333 2...1 75697675us : igb: val: 802
kworker/-1333 2...1 75697675us+: igb: val: 801
kworker/-1333 2...1 75697696us : igb: val: 802
kworker/-1333 2...1 75697696us+: igb: val: 801
kworker/-1333 2...1 75697717us : igb: val: 802
kworker/-1333 2...1 75697717us+: igb: val: 801
kworker/-1333 2...1 75697738us : igb: val: 802
kworker/-1333 2...1 75697738us+: igb: val: 801
kworker/-1333 2...1 75697759us : igb: val: 802
kworker/-1333 2...1 75697759us+: igb: val: 801
kworker/-1333 2...1 75697780us : igb: val: 802
kworker/-1333 2...1 75697781us+: igb: val: 801
kworker/-1333 2...1 75697801us : igb: val: 802
kworker/-1333 2...1 75697801us+: igb: val: 801
kworker/-1333 2...1 75697822us : igb: val: 802
It can be cleary seen that with the kernel 4.1 the time for the execution of the "readl"
function is much slower! It tasks always about 21us!
In comparison to that the kernel 4.0 is fast. Here the time for "readl" is about
1-2 us.
All measurements have been done on the very same PC.
The PC is an HP EliteDesk 800 G1,
a dual port Intel i350-T2 server adapter has been plugged in.
CPU: Intel(R) Core(TM) i7-4790 CPU @ 3.60GHz
x86_64 bit mode.
Please find attached the kernel configuration for the 4.1 kernel.
I had a look at the generated .o file of igb_main.o.
However, in both cases - kernel 4.0 and kernel 4.1 - the code for the
igb_rd32 function looks the same.
Especially the call to the readl() function is the very same.
This means, that I think that some other stuff in kernel 4.1 has changed,
which has impact on the igb accesses.
Any idea what component could cause this kind of issue?
Thanks for any feedback!
Regards
Mathias
[-- Attachment #2: config.gz --]
[-- Type: application/x-gzip, Size: 23977 bytes --]
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell,
linux-next-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jouni Malinen
In-Reply-To: <CAKv+Gu896xme5sd5i8hs7tA=Xt=qQKCiAx7fQg1ZECn50NttbQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2016-10-14 at 09:47 +0100, Ard Biesheuvel wrote:
>
> Do you have a reference for the sg_set_buf() call on odata?
> crypto/ccm.c does not seem to have it (afaict),
It's indirect - crypto_ccm_encrypt() calls crypto_ccm_init_crypt()
which does it.
> and the same problem
> does not exist in the accelerated arm64 implementation. In the mean
> time, I will try and see if we can move aad[] off the stack in the
> WPA code.
I had that with per-CPU buffers, just sent the patch upthread.
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:53 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Ard Biesheuvel
In-Reply-To: <1476429916.4382.12.camel@sipsolutions.net>
For reference, this was my patch moving the mac80211 buffers to percpu.
diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index 7663c28ba353..c3709ddf71e9 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -29,6 +29,8 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *) aead_req_data;
+ printk(KERN_INFO "ccm size: %d\n", sizeof(aead_req_data));
+
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_table(sg, 3);
@@ -37,6 +39,9 @@ void ieee80211_aes_ccm_encrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
sg_set_buf(&sg[2], mic, mic_len);
aead_request_set_tfm(aead_req, tfm);
+
+ printk(KERN_INFO "aead: %pf\n", crypto_aead_alg(crypto_aead_reqtfm(aead_req))->encrypt);
+
aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
aead_request_set_ad(aead_req, sg[0].length);
@@ -67,6 +72,8 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
aead_request_set_crypt(aead_req, sg, sg, data_len + mic_len, b_0);
aead_request_set_ad(aead_req, sg[0].length);
+ printk(KERN_INFO "aead: %pf\n", crypto_aead_alg(crypto_aead_reqtfm(aead_req))->decrypt);
+
return crypto_aead_decrypt(aead_req);
}
diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index bdf0790d89cc..ebb8c2dc9928 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -20,7 +20,6 @@
#define CMAC_TLEN 8 /* CMAC TLen = 64 bits (8 octets) */
#define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
-#define AAD_LEN 20
static void gf_mulx(u8 *pad)
@@ -101,7 +100,7 @@ void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
memset(zero, 0, CMAC_TLEN);
addr[0] = aad;
- len[0] = AAD_LEN;
+ len[0] = CMAC_AAD_LEN;
addr[1] = data;
len[1] = data_len - CMAC_TLEN;
addr[2] = zero;
@@ -119,7 +118,7 @@ void ieee80211_aes_cmac_256(struct crypto_cipher *tfm, const u8 *aad,
memset(zero, 0, CMAC_TLEN_256);
addr[0] = aad;
- len[0] = AAD_LEN;
+ len[0] = CMAC_AAD_LEN;
addr[1] = data;
len[1] = data_len - CMAC_TLEN_256;
addr[2] = zero;
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index 3702041f44fd..6645f8963278 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -11,6 +11,8 @@
#include <linux/crypto.h>
+#define CMAC_AAD_LEN 20
+
struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
size_t key_len);
void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
diff --git a/net/mac80211/aes_gcm.c b/net/mac80211/aes_gcm.c
index 3afe361fd27c..13e64d383c46 100644
--- a/net/mac80211/aes_gcm.c
+++ b/net/mac80211/aes_gcm.c
@@ -25,6 +25,8 @@ void ieee80211_aes_gcm_encrypt(struct crypto_aead *tfm, u8 *j_0, u8 *aad,
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *)aead_req_data;
+ printk(KERN_DEBUG "gcm size: %d\n", sizeof(aead_req_data));
+
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_table(sg, 3);
diff --git a/net/mac80211/aes_gmac.c b/net/mac80211/aes_gmac.c
index 3ddd927aaf30..a2fc69ec5ca9 100644
--- a/net/mac80211/aes_gmac.c
+++ b/net/mac80211/aes_gmac.c
@@ -19,17 +19,18 @@
#define GMAC_MIC_LEN 16
#define GMAC_NONCE_LEN 12
-#define AAD_LEN 20
int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
- const u8 *data, size_t data_len, u8 *mic)
+ const u8 *data, size_t data_len, u8 *mic, u8 *zero)
{
struct scatterlist sg[4];
char aead_req_data[sizeof(struct aead_request) +
crypto_aead_reqsize(tfm)]
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *)aead_req_data;
- u8 zero[GMAC_MIC_LEN], iv[AES_BLOCK_SIZE];
+ u8 iv[AES_BLOCK_SIZE];
+
+ printk(KERN_DEBUG "gmac size: %d\n", sizeof(aead_req_data));
if (data_len < GMAC_MIC_LEN)
return -EINVAL;
@@ -38,7 +39,7 @@ int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
memset(zero, 0, GMAC_MIC_LEN);
sg_init_table(sg, 4);
- sg_set_buf(&sg[0], aad, AAD_LEN);
+ sg_set_buf(&sg[0], aad, GMAC_AAD_LEN);
sg_set_buf(&sg[1], data, data_len - GMAC_MIC_LEN);
sg_set_buf(&sg[2], zero, GMAC_MIC_LEN);
sg_set_buf(&sg[3], mic, GMAC_MIC_LEN);
@@ -49,7 +50,7 @@ int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
aead_request_set_tfm(aead_req, tfm);
aead_request_set_crypt(aead_req, sg, sg, 0, iv);
- aead_request_set_ad(aead_req, AAD_LEN + data_len);
+ aead_request_set_ad(aead_req, GMAC_AAD_LEN + data_len);
crypto_aead_encrypt(aead_req);
diff --git a/net/mac80211/aes_gmac.h b/net/mac80211/aes_gmac.h
index d328204d73a8..f06833c9095f 100644
--- a/net/mac80211/aes_gmac.h
+++ b/net/mac80211/aes_gmac.h
@@ -11,10 +11,13 @@
#include <linux/crypto.h>
+#define GMAC_MIC_LEN 16
+#define GMAC_AAD_LEN 20
+
struct crypto_aead *ieee80211_aes_gmac_key_setup(const u8 key[],
size_t key_len);
int ieee80211_aes_gmac(struct crypto_aead *tfm, const u8 *aad, u8 *nonce,
- const u8 *data, size_t data_len, u8 *mic);
+ const u8 *data, size_t data_len, u8 *mic, u8 *zero);
void ieee80211_aes_gmac_key_free(struct crypto_aead *tfm);
#endif /* AES_GMAC_H */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 103187ca9474..bc2a3282e0a1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1129,6 +1129,14 @@ enum mac80211_scan_state {
SCAN_ABORT,
};
+struct ieee80211_crypto_bufs {
+
+ u8 buf1[32];
+ u8 buf2[16];
+};
+
+extern struct ieee80211_crypto_bufs __percpu *ieee80211_crypto_bufs;
+
struct ieee80211_local {
/* embed the driver visible part.
* don't cast (use the static inlines below), but we keep
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 1075ac24c8c5..6175cde94c53 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -33,6 +33,8 @@
#include "led.h"
#include "debugfs.h"
+struct ieee80211_crypto_bufs __percpu *ieee80211_crypto_bufs;
+
void ieee80211_configure_filter(struct ieee80211_local *local)
{
u64 mc;
@@ -1234,6 +1236,10 @@ static int __init ieee80211_init(void)
BUILD_BUG_ON(offsetof(struct ieee80211_tx_info, driver_data) +
IEEE80211_TX_INFO_DRIVER_DATA_SIZE > sizeof(skb->cb));
+ ieee80211_crypto_bufs = alloc_percpu(struct ieee80211_crypto_bufs);
+ if (!ieee80211_crypto_bufs)
+ return -ENOMEM;
+
ret = rc80211_minstrel_init();
if (ret)
return ret;
@@ -1264,6 +1270,8 @@ static void __exit ieee80211_exit(void)
ieee80211_iface_exit();
+ free_percpu(ieee80211_crypto_bufs);
+
rcu_barrier();
}
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index b48c1e13e281..c02634c4210c 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -405,8 +405,13 @@ static int ccmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb,
u8 *pos;
u8 pn[6];
u64 pn64;
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 b_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *b_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
@@ -534,8 +539,14 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
}
if (!(status->flag & RX_FLAG_DECRYPTED)) {
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 b_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *b_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
+
/* hardware didn't decrypt/verify MIC */
ccmp_special_blocks(skb, pn, b_0, aad);
@@ -639,8 +650,13 @@ static int gcmp_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
u8 *pos;
u8 pn[6];
u64 pn64;
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 j_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *j_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
if (info->control.hw_key &&
!(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV) &&
@@ -764,8 +780,14 @@ ieee80211_crypto_gcmp_decrypt(struct ieee80211_rx_data *rx)
}
if (!(status->flag & RX_FLAG_DECRYPTED)) {
- u8 aad[2 * AES_BLOCK_SIZE];
- u8 j_0[AES_BLOCK_SIZE];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *j_0 = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < 2 * AES_BLOCK_SIZE);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < AES_BLOCK_SIZE);
+
/* hardware didn't decrypt/verify MIC */
gcmp_special_blocks(skb, pn, j_0, aad);
@@ -935,8 +957,12 @@ ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
struct ieee80211_tx_info *info;
struct ieee80211_key *key = tx->key;
struct ieee80211_mmie *mmie;
- u8 aad[20];
u64 pn64;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
return TX_DROP;
@@ -979,8 +1005,12 @@ ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx)
struct ieee80211_tx_info *info;
struct ieee80211_key *key = tx->key;
struct ieee80211_mmie_16 *mmie;
- u8 aad[20];
u64 pn64;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
return TX_DROP;
@@ -1022,8 +1052,13 @@ ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_key *key = rx->key;
struct ieee80211_mmie *mmie;
- u8 aad[20], mic[8], ipn[6];
+ u8 mic[8], ipn[6];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (!ieee80211_is_mgmt(hdr->frame_control))
return RX_CONTINUE;
@@ -1072,8 +1107,13 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_key *key = rx->key;
struct ieee80211_mmie_16 *mmie;
- u8 aad[20], mic[16], ipn[6];
+ u8 mic[16], ipn[6];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < CMAC_AAD_LEN);
if (!ieee80211_is_mgmt(hdr->frame_control))
return RX_CONTINUE;
@@ -1123,9 +1163,15 @@ ieee80211_crypto_aes_gmac_encrypt(struct ieee80211_tx_data *tx)
struct ieee80211_key *key = tx->key;
struct ieee80211_mmie_16 *mmie;
struct ieee80211_hdr *hdr;
- u8 aad[20];
u64 pn64;
u8 nonce[12];
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *zero = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < GMAC_AAD_LEN);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < GMAC_MIC_LEN);
if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
return TX_DROP;
@@ -1158,7 +1204,8 @@ ieee80211_crypto_aes_gmac_encrypt(struct ieee80211_tx_data *tx)
/* MIC = AES-GMAC(IGTK, AAD || Management Frame Body || MMIE, 128) */
if (ieee80211_aes_gmac(key->u.aes_gmac.tfm, aad, nonce,
- skb->data + 24, skb->len - 24, mmie->mic) < 0)
+ skb->data + 24, skb->len - 24, mmie->mic,
+ zero) < 0)
return TX_DROP;
return TX_CONTINUE;
@@ -1171,8 +1218,15 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_key *key = rx->key;
struct ieee80211_mmie_16 *mmie;
- u8 aad[20], mic[16], ipn[6], nonce[12];
+ u8 mic[16], ipn[6], nonce[12];
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ struct ieee80211_crypto_bufs *bufs =
+ this_cpu_ptr(ieee80211_crypto_bufs);
+ u8 *aad = bufs->buf1;
+ u8 *zero = bufs->buf2;
+
+ BUILD_BUG_ON(sizeof(bufs->buf1) < GMAC_AAD_LEN);
+ BUILD_BUG_ON(sizeof(bufs->buf2) < GMAC_MIC_LEN);
if (!ieee80211_is_mgmt(hdr->frame_control))
return RX_CONTINUE;
@@ -1204,7 +1258,7 @@ ieee80211_crypto_aes_gmac_decrypt(struct ieee80211_rx_data *rx)
if (ieee80211_aes_gmac(key->u.aes_gmac.tfm, aad, nonce,
skb->data + 24, skb->len - 24,
- mic) < 0 ||
+ mic, zero) < 0 ||
memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
key->u.aes_gmac.icverrors++;
return RX_DROP_UNUSABLE;
^ permalink raw reply related
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 8:47 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476434561.31114.7.camel@sipsolutions.net>
On 14 October 2016 at 09:42, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2016-10-14 at 09:41 +0100, Ard Biesheuvel wrote:
>
>> > I assume the stack buffer itself is not the problem here, but aad,
>> > which is allocated on the stack one frame up.
>> > Do we really need to revert the whole patch to fix that?
>>
>> Ah never mind, this is about 'odata'. Apologies, should have read
>> first
>
> Right, odata also goes into an sg list and further on.
>
> I think we should wait for Herbert to chime in before we do any further
> work though, perhaps he has any better ideas.
>
Do you have a reference for the sg_set_buf() call on odata?
crypto/ccm.c does not seem to have it (afaict), and the same problem
does not exist in the accelerated arm64 implementation. In the mean
time, I will try and see if we can move aad[] off the stack in the WPA
code.
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:45 UTC (permalink / raw)
To: Sergey Senozhatsky, Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Network Development,
Sergey Senozhatsky, Herbert Xu, David S. Miller,
Linux Wireless List, linux-kernel@vger.kernel.org
In-Reply-To: <20161014083957.GA421@swordfish>
On Fri, 2016-10-14 at 17:39 +0900, Sergey Senozhatsky wrote:
>
> given that we have a known issue shouldn't VMAP_STACK be
> disabled for now, or would you rather prefer to mark MAC80211
> as incompatible: "depends on CFG80211 && !VMAP_STACK"?
Yeah. It's a bit complicated by the fact that most people will probably
have hardware crypto in their wifi NICs, so that they won't actually
hit the software crypto path. As I said in my other email though, we
can't guarantee - even if the driver says it can do hardware crypto -
that it really will do it for all frames (some might not be able to do
for management frames for example), so we also can't really catch this
at runtime ...
Making mac80211 depend on !VMAP_STACK is probably technically best, but
I fear it'll break a lot of people's configurations who don't have a
problem right now (e.g. Linus's, who probably enabled this, but I know
where he uses wifi he uses an Intel NIC that will always do HW crypto).
Andy, what do you think?
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:42 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu_v+4D_7EvRkrSrStheuP1L-oPh=o7Ub+NU7-wgLYks4w@mail.gmail.com>
On Fri, 2016-10-14 at 09:41 +0100, Ard Biesheuvel wrote:
> > I assume the stack buffer itself is not the problem here, but aad,
> > which is allocated on the stack one frame up.
> > Do we really need to revert the whole patch to fix that?
>
> Ah never mind, this is about 'odata'. Apologies, should have read
> first
Right, odata also goes into an sg list and further on.
I think we should wait for Herbert to chime in before we do any further
work though, perhaps he has any better ideas.
johannes
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 8:41 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <CAKv+Gu8hTy8U7FcA58P1YWt2rHU09xP9v1UU0KcnD4uuaK8How@mail.gmail.com>
On 14 October 2016 at 09:39, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 14 October 2016 at 09:28, Johannes Berg <johannes@sipsolutions.net> wrote:
>>
>>> 1. revert that patch (doing so would need some major adjustments now,
>>> since it's pretty old and a number of new things were added in the
>>> meantime)
>>
>> This it will have to be, I guess.
>>
>>> 2. allocate a per-CPU buffer for all the things that we put on the
>>> stack and use in SG lists, those are:
>>> * CCM/GCM: AAD (32B), B_0/J_0 (16B)
>>> * GMAC: AAD (20B), zero (16B)
>>> * (not sure why CMAC isn't using this API, but it would be like GMAC)
>>
>> This doesn't work - I tried to move the mac80211 buffers, but because
>> we also put the struct aead_request on the stack, and crypto_ccm has
>> the "odata" in there, and we can't separate the odata from that struct,
>> we'd have to also put that into a per-CPU buffer, but it's very big -
>> 456 bytes for CCM, didn't measure the others but I'd expect them to be
>> larger, if different.
>>
>> I don't think we can allocate half a kb for each CPU just to be able to
>> possibly use the acceleration here. We can't even make that conditional
>> on not having hardware crypto in the wifi NIC because drivers are
>> always allowed to pass undecrypted frames, regardless of whether or not
>> HW crypto was attempted, so we don't know upfront if we'll have to
>> decrypt anything in software...
>>
>> Given that, I think we have had a bug in here basically since Ard's
>> patch, we never should've put these structs on the stack. Herbert, you
>> also touched this later and converted the API usage, did you see the
>> way the stack is used here and think it should be OK, or did you simply
>> not realize that?
>>
>> Ard, are you able to help out working on a revert of your patch? That
>> would require also reverting a number of other patches (various fixes,
>> API adjustments, etc. to the AEAD usage), but the more complicated part
>> is that in the meantime Jouni introduced GCMP and CCMP-256, both of
>> which we of course need to retain.
>>
>
> I am missing some context here, but could you explain what exactly is
> the problem here?
>
> Look at this code
>
> """
> struct scatterlist sg[3];
>
> char aead_req_data[sizeof(struct aead_request) +
> crypto_aead_reqsize(tfm)]
> __aligned(__alignof__(struct aead_request));
> struct aead_request *aead_req = (void *) aead_req_data;
>
> memset(aead_req, 0, sizeof(aead_req_data));
>
> sg_init_table(sg, 3);
> sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
> sg_set_buf(&sg[1], data, data_len);
> sg_set_buf(&sg[2], mic, mic_len);
>
> aead_request_set_tfm(aead_req, tfm);
> aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
> aead_request_set_ad(aead_req, sg[0].length);
> """
>
> I assume the stack buffer itself is not the problem here, but aad,
> which is allocated on the stack one frame up.
> Do we really need to revert the whole patch to fix that?
Ah never mind, this is about 'odata'. Apologies, should have read first
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Sergey Senozhatsky @ 2016-10-14 8:39 UTC (permalink / raw)
To: Andy Lutomirski, Johannes Berg
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrW8imkGTtR9-mFB=e=Gdr1QXVuhXN3GRs2B6wbmPaaGhA@mail.gmail.com>
On (10/13/16 14:49), Andy Lutomirski wrote:
[..]
> > > FAIL: 00004100002cba02 > ffffc900802cba02 || 1 -> (00004100002cba02
> > > >> 39) == 130
> >
> > Yeah, we already know that in this function the aad variable is on the
> > stack, it explicitly is.
> >
> > The question, though, is why precisely that fails in the crypto code.
> > Can you send the Oops report itself?
> >
>
> It's failing before that. With CONFIG_VMAP_STACK=y, the stack may not
> be physically contiguous and can't be used for DMA, so putting it in a
> scatterlist is bogus in general, and the crypto code mostly wants a
> scatterlist.
>
> There are a couple (faster!) APIs for crypto that don't use
> scatterlists, but I don't think AEAD works with them.
given that we have a known issue shouldn't VMAP_STACK be
disabled for now, or would you rather prefer to mark MAC80211
as incompatible: "depends on CFG80211 && !VMAP_STACK"?
-ss
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Ard Biesheuvel @ 2016-10-14 8:39 UTC (permalink / raw)
To: Johannes Berg
Cc: Andy Lutomirski, Stephen Rothwell, linux-next@vger.kernel.org,
Sergey Senozhatsky, Network Development, Sergey Senozhatsky,
Herbert Xu, David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Jouni Malinen
In-Reply-To: <1476433699.31114.6.camel@sipsolutions.net>
On 14 October 2016 at 09:28, Johannes Berg <johannes@sipsolutions.net> wrote:
>
>> 1. revert that patch (doing so would need some major adjustments now,
>> since it's pretty old and a number of new things were added in the
>> meantime)
>
> This it will have to be, I guess.
>
>> 2. allocate a per-CPU buffer for all the things that we put on the
>> stack and use in SG lists, those are:
>> * CCM/GCM: AAD (32B), B_0/J_0 (16B)
>> * GMAC: AAD (20B), zero (16B)
>> * (not sure why CMAC isn't using this API, but it would be like GMAC)
>
> This doesn't work - I tried to move the mac80211 buffers, but because
> we also put the struct aead_request on the stack, and crypto_ccm has
> the "odata" in there, and we can't separate the odata from that struct,
> we'd have to also put that into a per-CPU buffer, but it's very big -
> 456 bytes for CCM, didn't measure the others but I'd expect them to be
> larger, if different.
>
> I don't think we can allocate half a kb for each CPU just to be able to
> possibly use the acceleration here. We can't even make that conditional
> on not having hardware crypto in the wifi NIC because drivers are
> always allowed to pass undecrypted frames, regardless of whether or not
> HW crypto was attempted, so we don't know upfront if we'll have to
> decrypt anything in software...
>
> Given that, I think we have had a bug in here basically since Ard's
> patch, we never should've put these structs on the stack. Herbert, you
> also touched this later and converted the API usage, did you see the
> way the stack is used here and think it should be OK, or did you simply
> not realize that?
>
> Ard, are you able to help out working on a revert of your patch? That
> would require also reverting a number of other patches (various fixes,
> API adjustments, etc. to the AEAD usage), but the more complicated part
> is that in the meantime Jouni introduced GCMP and CCMP-256, both of
> which we of course need to retain.
>
I am missing some context here, but could you explain what exactly is
the problem here?
Look at this code
"""
struct scatterlist sg[3];
char aead_req_data[sizeof(struct aead_request) +
crypto_aead_reqsize(tfm)]
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *) aead_req_data;
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_table(sg, 3);
sg_set_buf(&sg[0], &aad[2], be16_to_cpup((__be16 *)aad));
sg_set_buf(&sg[1], data, data_len);
sg_set_buf(&sg[2], mic, mic_len);
aead_request_set_tfm(aead_req, tfm);
aead_request_set_crypt(aead_req, sg, sg, data_len, b_0);
aead_request_set_ad(aead_req, sg[0].length);
"""
I assume the stack buffer itself is not the problem here, but aad,
which is allocated on the stack one frame up.
Do we really need to revert the whole patch to fix that?
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 8:28 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Ard Biesheuvel, Jouni Malinen
In-Reply-To: <1476429916.4382.12.camel@sipsolutions.net>
> 1. revert that patch (doing so would need some major adjustments now,
> since it's pretty old and a number of new things were added in the
> meantime)
This it will have to be, I guess.
> 2. allocate a per-CPU buffer for all the things that we put on the
> stack and use in SG lists, those are:
> * CCM/GCM: AAD (32B), B_0/J_0 (16B)
> * GMAC: AAD (20B), zero (16B)
> * (not sure why CMAC isn't using this API, but it would be like GMAC)
This doesn't work - I tried to move the mac80211 buffers, but because
we also put the struct aead_request on the stack, and crypto_ccm has
the "odata" in there, and we can't separate the odata from that struct,
we'd have to also put that into a per-CPU buffer, but it's very big -
456 bytes for CCM, didn't measure the others but I'd expect them to be
larger, if different.
I don't think we can allocate half a kb for each CPU just to be able to
possibly use the acceleration here. We can't even make that conditional
on not having hardware crypto in the wifi NIC because drivers are
always allowed to pass undecrypted frames, regardless of whether or not
HW crypto was attempted, so we don't know upfront if we'll have to
decrypt anything in software...
Given that, I think we have had a bug in here basically since Ard's
patch, we never should've put these structs on the stack. Herbert, you
also touched this later and converted the API usage, did you see the
way the stack is used here and think it should be OK, or did you simply
not realize that?
Ard, are you able to help out working on a revert of your patch? That
would require also reverting a number of other patches (various fixes,
API adjustments, etc. to the AEAD usage), but the more complicated part
is that in the meantime Jouni introduced GCMP and CCMP-256, both of
which we of course need to retain.
johannes
^ permalink raw reply
* Re: [PATCH trivial] net: add bbr to config DEFAULT_TCP_CONG
From: Markus Trippelsdorf @ 2016-10-14 8:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Neal Cardwell, netdev
In-Reply-To: <1476431026.5650.38.camel@edumazet-glaptop3.roam.corp.google.com>
On 2016.10.14 at 09:43 +0200, Eric Dumazet wrote:
> On Fri, 2016-10-14 at 09:33 +0200, Markus Trippelsdorf wrote:
> > While playing with BBR I noticed that it was missing in the list of
> > possible config DEFAULT_TCP_CONG choices. Fixed thusly.
> >
> > Signed-off-by: Markus Trippelsdorf <markus@trippelsdorf.de>
> >
> > diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
> > index 300b06888fdf..b54b3ca939db 100644
> > --- a/net/ipv4/Kconfig
> > +++ b/net/ipv4/Kconfig
> > @@ -715,6 +715,7 @@ config DEFAULT_TCP_CONG
> > default "reno" if DEFAULT_RENO
> > default "dctcp" if DEFAULT_DCTCP
> > default "cdg" if DEFAULT_CDG
> > + default "bbr" if DEFAULT_BBR
> > default "cubic"
>
> Not sure if we want this at this moment.
>
> BBR needs FQ packet scheduler, and this is not exactly trivial to
> achieve.
For a start, it could be automatically selected:
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 300b06888fdf..845d8d3e9e27 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -642,6 +642,8 @@ config TCP_CONG_CDG
config TCP_CONG_BBR
tristate "BBR TCP"
+ select NET_SCHED
+ select NET_SCH_FQ
default n
---help---
--
Markus
^ permalink raw reply related
* [PATCH] brcmfmac: print name of connect status event
From: Rafał Miłecki @ 2016-10-14 7:45 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Franky Lin, Hante Meuleman,
Pieter-Paul Giesberts, Franky Lin, linux-wireless,
brcm80211-dev-list.pdl, netdev, linux-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This simplifies debugging. Format %s (%u) comes from similar debugging
message in brcmf_fweh_event_worker.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ++-
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 4 ++--
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h | 2 ++
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b777e1b..1e7c6f0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5506,7 +5506,8 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
u32 reason = e->reason;
struct station_info sinfo;
- brcmf_dbg(CONN, "event %d, reason %d\n", event, reason);
+ brcmf_dbg(CONN, "event %s (%u), reason %d\n",
+ brcmf_fweh_event_name(event), event, reason);
if (event == BRCMF_E_LINK && reason == BRCMF_E_REASON_LINK_BSSCFG_DIS &&
ndev != cfg_to_ndev(cfg)) {
brcmf_dbg(CONN, "AP mode link down\n");
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 79c081f..c79306b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -69,7 +69,7 @@ static struct brcmf_fweh_event_name fweh_event_names[] = {
*
* @code: code to lookup.
*/
-static const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
+const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
{
int i;
for (i = 0; i < ARRAY_SIZE(fweh_event_names); i++) {
@@ -79,7 +79,7 @@ static const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
return "unknown";
}
#else
-static const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
+const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code)
{
return "nodebug";
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
index 26ff5a9..5fba4b4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
@@ -287,6 +287,8 @@ struct brcmf_fweh_info {
void *data);
};
+const char *brcmf_fweh_event_name(enum brcmf_fweh_event_code code);
+
void brcmf_fweh_attach(struct brcmf_pub *drvr);
void brcmf_fweh_detach(struct brcmf_pub *drvr);
int brcmf_fweh_register(struct brcmf_pub *drvr, enum brcmf_fweh_event_code code,
--
2.9.3
^ permalink raw reply related
* Re: [PATCH trivial] net: add bbr to config DEFAULT_TCP_CONG
From: Eric Dumazet @ 2016-10-14 7:43 UTC (permalink / raw)
To: Markus Trippelsdorf; +Cc: Neal Cardwell, netdev
In-Reply-To: <20161014073333.GA10006@x4>
On Fri, 2016-10-14 at 09:33 +0200, Markus Trippelsdorf wrote:
> While playing with BBR I noticed that it was missing in the list of
> possible config DEFAULT_TCP_CONG choices. Fixed thusly.
>
> Signed-off-by: Markus Trippelsdorf <markus@trippelsdorf.de>
>
> diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
> index 300b06888fdf..b54b3ca939db 100644
> --- a/net/ipv4/Kconfig
> +++ b/net/ipv4/Kconfig
> @@ -715,6 +715,7 @@ config DEFAULT_TCP_CONG
> default "reno" if DEFAULT_RENO
> default "dctcp" if DEFAULT_DCTCP
> default "cdg" if DEFAULT_CDG
> + default "bbr" if DEFAULT_BBR
> default "cubic"
Not sure if we want this at this moment.
BBR needs FQ packet scheduler, and this is not exactly trivial to
achieve.
^ permalink raw reply
* [PATCH trivial] net: add bbr to config DEFAULT_TCP_CONG
From: Markus Trippelsdorf @ 2016-10-14 7:33 UTC (permalink / raw)
To: Neal Cardwell; +Cc: netdev
While playing with BBR I noticed that it was missing in the list of
possible config DEFAULT_TCP_CONG choices. Fixed thusly.
Signed-off-by: Markus Trippelsdorf <markus@trippelsdorf.de>
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 300b06888fdf..b54b3ca939db 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -715,6 +715,7 @@ config DEFAULT_TCP_CONG
default "reno" if DEFAULT_RENO
default "dctcp" if DEFAULT_DCTCP
default "cdg" if DEFAULT_CDG
+ default "bbr" if DEFAULT_BBR
default "cubic"
config TCP_MD5SIG
--
Markus
^ permalink raw reply related
* RE: [PATCH 4/6] fjes: Implement debug mode for fjes driver
From: Izumi, Taku @ 2016-10-14 7:30 UTC (permalink / raw)
To: 'David Miller'; +Cc: netdev@vger.kernel.org
In-Reply-To: <20161013.101930.1034896203915503108.davem@davemloft.net>
Dear David,
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, October 13, 2016 11:20 PM
> To: Izumi, Taku/泉 拓
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH 4/6] fjes: Implement debug mode for fjes driver
>
> From: Taku Izumi <izumi.taku@jp.fujitsu.com>
> Date: Tue, 11 Oct 2016 17:55:20 +0900
>
> > This patch implements debug mode for fjes driver.
> > You can get firmware activity information by enabling
> > debug mode. This is useful for debugging.
> >
> > To enable debug mode, write value of debugging mode to
> > debug_mode file in debugfs:
> >
> > # echo 1 > /sys/kernel/debug/fjes/fjes.0/debug_mode
> >
> > To disable debug mode, write 0 to debug_mode file in debugfs:
> >
> > # echo 0 > /sys/kernel/debug/fjes/fjes.0/debug_mode
> >
> > Firmware activity information can be retrieved via
> > /sys/kernel/debug/fjes/fjes.0/debug_data file.
> >
> > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
>
> There is no reason to use debugfs for this, we have facilities such
> as ETHTOOL_SET_DUMP et al. that you can use to implement this.
Thank you for reviewing.
I see. I'll rewrite this patch to use ethtool.
Sincerely,
Taku Izumi
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Johannes Berg @ 2016-10-14 7:25 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org, Ard Biesheuvel
In-Reply-To: <CALCETrW8imkGTtR9-mFB=e=Gdr1QXVuhXN3GRs2B6wbmPaaGhA@mail.gmail.com>
On Thu, 2016-10-13 at 14:49 -0700, Andy Lutomirski wrote:
>
> It's failing before that. With CONFIG_VMAP_STACK=y, the stack may
> not be physically contiguous and can't be used for DMA, so putting it
> in a scatterlist is bogus in general, and the crypto code mostly
> wants a scatterlist.
I see, so all this stuff is getting inlined, and we crash in
sg_set_buf() because it does sg_set_page() and that obviously needs to
do virt_to_page(), which is invalid on this address now.
With CONFIG_DEBUG_SG we'd have hit the BUG_ON there instead.
It does indeed look like AEAD doesn't have any non-SG API.
So ultimately, the bug already goes back to Ard's commit 7ec7c4a9a686
("mac80211: port CCMP to cryptoapi's CCM driver") since that already
potentially used stack space for DMA.
Since we don't have any space in the SKB or anywhere else at this point
(other than the stack that we can't use), I see two ways out of this:
1. revert that patch (doing so would need some major adjustments now,
since it's pretty old and a number of new things were added in the
meantime)
2. allocate a per-CPU buffer for all the things that we put on the
stack and use in SG lists, those are:
* CCM/GCM: AAD (32B), B_0/J_0 (16B)
* GMAC: AAD (20B), zero (16B)
* (not sure why CMAC isn't using this API, but it would be like
GMAC)
Thoughts?
johannes
^ permalink raw reply
* pull-request: wireless-drivers 2016-10-14
From: Kalle Valo @ 2016-10-14 7:18 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
first wireless-drivers pull request for 4.9 and this time we have
unusually many fixes even before -rc1 is released. Most important here
are the wlcore and rtlwifi commits which fix critical regressions,
otherwise smaller impact fixes and one new sdio id for ath6kl.
Please let me know if there are any problems.
Kalle
The following changes since commit 03a1eabc3f54469abd4f1784182851b2e29630cc:
Merge branch 'mlxsw-fixes' (2016-10-04 20:28:10 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2016-10-14
for you to fetch changes up to 1ea2643961b0d1b8d0e4a11af5aa69b0f92d0533:
ath6kl: add Dell OEM SDIO I/O for the Venue 8 Pro (2016-10-13 14:16:33 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 4.9
wlcore
* fix a double free regression causing hard to track crashes
rtl8xxxu
* fix driver reload issues, a memory leak and an endian bug
rtlwifi
* fix a major regression introduced in 4.9 with firmware loading on
certain hardware
ath10k
* fix regression about broken cal_data debugfs file (since 4.7)
ath9k
* revert temperature compensation for AR9003+ devices, it was causing
too much problems
ath6kl
* add Dell OEM SDIO I/O for the Venue 8 Pro
----------------------------------------------------------------
Adam Williamson (1):
ath6kl: add Dell OEM SDIO I/O for the Venue 8 Pro
Felix Fietkau (1):
Revert "ath9k_hw: implement temperature compensation support for AR9003+"
Jes Sorensen (4):
rtl8xxxu: Fix memory leak in handling rxdesc16 packets
rtl8xxxu: Fix big-endian problem reporting mactime
rtl8xxxu: Fix rtl8723bu driver reload issue
rtl8xxxu: Fix rtl8192eu driver reload issue
Larry Finger (1):
rtlwifi: Fix regression caused by commit d86e64768859
Marty Faltesek (1):
ath10k: cache calibration data when the core is stopped
Wei Yongjun (1):
wlcore: sdio: drop kfree for memory allocated with devm_kzalloc
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/debug.c | 75 ++++++++++----------
drivers/net/wireless/ath/ath6kl/sdio.c | 1 +
drivers/net/wireless/ath/ath9k/ar9003_calib.c | 25 +------
drivers/net/wireless/ath/ath9k/hw.h | 1 -
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 4 +-
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 8 ++-
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 4 ++
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++-
drivers/net/wireless/realtek/rtlwifi/core.c | 2 +-
.../net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 8 +--
.../net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 13 ++--
.../net/wireless/realtek/rtlwifi/rtl8192cu/sw.c | 12 ++--
.../net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 6 +-
.../net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 8 +--
.../net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 9 +--
.../net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 12 ++--
.../net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 6 +-
.../net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 18 ++---
drivers/net/wireless/realtek/rtlwifi/wifi.h | 2 -
drivers/net/wireless/ti/wlcore/sdio.c | 1 -
21 files changed, 110 insertions(+), 117 deletions(-)
^ permalink raw reply
* Re: [PATCH net 0/3] qed: Fix dependencies and warnings series
From: Arnd Bergmann @ 2016-10-14 6:51 UTC (permalink / raw)
To: Yuval Mintz; +Cc: davem, netdev, dan.carpenter
In-Reply-To: <1476388623-31032-1-git-send-email-Yuval.Mintz@caviumnetworks.com>
On Thursday, October 13, 2016 10:57:00 PM CEST Yuval Mintz wrote:
> Hi Dave,
>
> The first patch in this series follows Dan Carpenter's reports about
> Smatch warnings for recent qed additions and fixes those.
>
> The second patch is the most significant one [and the reason this is
> ntended for 'net'] - it's based on Arnd Bermann's suggestion for fixing
> compilation issues that were introduced with the roce addition as a result
> of certain combinations of qed, qede and qedr Kconfig options.
>
> The third follows the discussion with Arnd and clears a lot of the warnings
> that arise when compiling the drivers with "C=1".
>
> Please consider applying this series to 'net'.
>
I've put all three patches into my randconfig test tree for double-checking,
but looking at the patches I'm fairly sure they are good.
I'll let you know within a few hours if something breaks unexpectedly.
Thanks for picking up my suggestions!
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply
* Re: [PATCH v2] net: Require exact match for TCP socket lookups if dif is l3mdev
From: Eric Dumazet @ 2016-10-14 6:33 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <1476420459-20966-1-git-send-email-dsa@cumulusnetworks.com>
On Thu, 2016-10-13 at 21:47 -0700, David Ahern wrote:
> Currently, socket lookups for l3mdev (vrf) use cases can match a socket
> that is bound to a port but not a device (ie., a global socket). If the
> sysctl tcp_l3mdev_accept is not set this leads to ack packets going out
> based on the main table even though the packet came in from an L3 domain.
> The end result is that the connection does not establish creating
> confusion for users since the service is running and a socket shows in
> ss output. Fix by requiring an exact dif to sk_bound_dev_if match if the
> skb came through an interface enslaved to an l3mdev device and the
> tcp_l3mdev_accept is not set.
>
> skb's through an l3mdev interface are marked by setting a flag in the
> inet{6}_skb_parm. The IPv6 variant is already set. This patch adds the
> flag for IPv4. Marking the skb avoids a device lookup on the dif.
>
> The inet_skb_parm struct currently has a 1-byte hold following the
> flags, so the flags is expanded to u16 without increasing the size of
> the struct. This is needed to add another flag.
>
> Fixes: 193125dbd8eb ("net: Introduce VRF device driver")
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> v2
> - reordered the checks in inet_exact_dif_match per Eric's comment
> - changed the l3mdev determination from looking up the dif to using
> a flag set on the skb which is much faster
>
> drivers/net/vrf.c | 2 ++
> include/linux/ipv6.h | 10 ++++++++++
> include/net/ip.h | 13 ++++++++++++-
> net/ipv4/inet_hashtables.c | 7 ++++---
> net/ipv6/inet6_hashtables.c | 7 ++++---
> 5 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index 85c271c70d42..820de6a9ddde 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -956,6 +956,7 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
> if (skb->pkt_type == PACKET_LOOPBACK) {
> skb->dev = vrf_dev;
> skb->skb_iif = vrf_dev->ifindex;
> + IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
> skb->pkt_type = PACKET_HOST;
> goto out;
> }
> @@ -996,6 +997,7 @@ static struct sk_buff *vrf_ip_rcv(struct net_device *vrf_dev,
> {
> skb->dev = vrf_dev;
> skb->skb_iif = vrf_dev->ifindex;
> + IPCB(skb)->flags |= IPSKB_L3SLAVE;
>
> /* loopback traffic; do not push through packet taps again.
> * Reset pkt_type for upper layers to process skb
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index 7e9a789be5e0..dd3bb34aac8b 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -144,6 +144,16 @@ static inline int inet6_iif(const struct sk_buff *skb)
> return l3_slave ? skb->skb_iif : IP6CB(skb)->iif;
> }
>
> +static inline bool inet6_exact_dif_match(struct net *net, struct sk_buff *skb)
> +{
> +#ifdef CONFIG_NET_L3_MASTER_DEV
> + if (!net->ipv4.sysctl_tcp_l3mdev_accept &&
> + IP6CB(skb)->flags & IP6SKB_L3SLAVE)
There is a catch here.
TCP moves IP6CB() in a different location.
Reference :
971f10eca186 ("tcp: better TCP_SKB_CB layout to reduce cache line misses")
Problem is that the lookup can happen from IP early demux, before TCP
moved IP{6}CB around.
So you might need to let the caller pass IP6CB(skb)->flags (or
TCP_SKB_CB(skb)->header.h6.flags ) instead of skb since
inet6_exact_dif_match() does not know where to fetch the flags.
Same issue for IPv4.
^ permalink raw reply
* Re: [PATCH net-next 2/2] ila: Cache a route to translated address
From: Roopa Prabhu @ 2016-10-14 6:22 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, kernel-team
In-Reply-To: <20161014005743.288956-3-tom@herbertland.com>
On 10/13/16, 5:57 PM, Tom Herbert wrote:
> Add a dst_cache to ila_lwt structure. This holds a cached route for the
> translated address. In ila_output we now perform a route lookup after
> translation and if possible (destination in original route is full 128
> bits) we set the dst_cache. Subsequent calls to ila_output can then use
> the cache to avoid the route lookup.
>
> This eliminates the need to set the gateway on ILA routes as previously
> was being done. Now we can do something like:
>
> ./ip route add 3333::2000:0:0:2/128 encap ila 2222:0:0:2 \
> csum-mode neutral-map dev eth0 ## No via needed!
>
> Signed-off-by: Tom Herbert <tom@herbertland.com>
> ---
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
This is a nice way to cache and redirect to a new dst.
This removes the last and only user of lwt orig_output. we can drop it
subsequently. But since orig_input is still in use, probably better to keep it
around for symmetry and for other uses in the future.
thanks.
> net/ipv6/ila/ila_lwt.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/ila/ila_lwt.c b/net/ipv6/ila/ila_lwt.c
> index e50c27a..df14b00 100644
> --- a/net/ipv6/ila/ila_lwt.c
> +++ b/net/ipv6/ila/ila_lwt.c
> @@ -6,29 +6,80 @@
> #include <linux/socket.h>
> #include <linux/types.h>
> #include <net/checksum.h>
> +#include <net/dst_cache.h>
> #include <net/ip.h>
> #include <net/ip6_fib.h>
> +#include <net/ip6_route.h>
> #include <net/lwtunnel.h>
> #include <net/protocol.h>
> #include <uapi/linux/ila.h>
> #include "ila.h"
>
> +struct ila_lwt {
> + struct ila_params p;
> + struct dst_cache dst_cache;
> + u32 connected : 1;
> +};
> +
> +static inline struct ila_lwt *ila_lwt_lwtunnel(
> + struct lwtunnel_state *lwstate)
> +{
> + return (struct ila_lwt *)lwstate->data;
> +}
> +
> static inline struct ila_params *ila_params_lwtunnel(
> struct lwtunnel_state *lwstate)
> {
> - return (struct ila_params *)lwstate->data;
> + return &((struct ila_lwt *)lwstate->data)->p;
> }
>
> static int ila_output(struct net *net, struct sock *sk, struct sk_buff *skb)
> {
> - struct dst_entry *dst = skb_dst(skb);
> + struct dst_entry *orig_dst = skb_dst(skb);
> + struct ila_lwt *ilwt = ila_lwt_lwtunnel(orig_dst->lwtstate);
> + struct dst_entry *dst;
> + int err = -EINVAL;
>
> if (skb->protocol != htons(ETH_P_IPV6))
> goto drop;
>
> - ila_update_ipv6_locator(skb, ila_params_lwtunnel(dst->lwtstate), true);
> + ila_update_ipv6_locator(skb, ila_params_lwtunnel(orig_dst->lwtstate),
> + true);
>
> - return dst->lwtstate->orig_output(net, sk, skb);
> + dst = dst_cache_get(&ilwt->dst_cache);
> + if (unlikely(!dst)) {
> + struct ipv6hdr *ip6h = ipv6_hdr(skb);
> + struct flowi6 fl6;
> +
> + /* Lookup a route for the new destination. Take into
> + * account that the base route may already have a gateway.
> + */
> +
> + memset(&fl6, 0, sizeof(fl6));
> + fl6.flowi6_oif = orig_dst->dev->ifindex;
> + fl6.flowi6_iif = LOOPBACK_IFINDEX;
> + fl6.daddr = *rt6_nexthop((struct rt6_info *)orig_dst,
> + &ip6h->daddr);
> +
> + dst = ip6_route_output(net, NULL, &fl6);
> + if (dst->error) {
> + err = -EHOSTUNREACH;
> + dst_release(dst);
> + goto drop;
> + }
> +
> + dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), NULL, 0);
> + if (IS_ERR(dst)) {
> + err = PTR_ERR(dst);
> + goto drop;
> + }
> +
> + if (ilwt->connected)
> + dst_cache_set_ip6(&ilwt->dst_cache, dst, &fl6.saddr);
> + }
> +
> + skb_dst_set(skb, dst);
> + return dst_output(dev_net(skb_dst(skb)->dev), sk, skb);
>
> drop:
> kfree_skb(skb);
> @@ -60,6 +111,7 @@ static int ila_build_state(struct net_device *dev, struct nlattr *nla,
> unsigned int family, const void *cfg,
> struct lwtunnel_state **ts)
> {
> + struct ila_lwt *ilwt;
> struct ila_params *p;
> struct nlattr *tb[ILA_ATTR_MAX + 1];
> size_t encap_len = sizeof(*p);
> @@ -99,6 +151,13 @@ static int ila_build_state(struct net_device *dev, struct nlattr *nla,
> if (!newts)
> return -ENOMEM;
>
> + ilwt = ila_lwt_lwtunnel(newts);
> + ret = dst_cache_init(&ilwt->dst_cache, GFP_ATOMIC);
> + if (ret) {
> + kfree(newts);
> + return ret;
> + }
> +
> newts->len = encap_len;
> p = ila_params_lwtunnel(newts);
>
> @@ -120,11 +179,19 @@ static int ila_build_state(struct net_device *dev, struct nlattr *nla,
> newts->flags |= LWTUNNEL_STATE_OUTPUT_REDIRECT |
> LWTUNNEL_STATE_INPUT_REDIRECT;
>
> + if (cfg6->fc_dst_len == sizeof(struct in6_addr))
> + ilwt->connected = 1;
> +
> *ts = newts;
>
> return 0;
> }
>
> +static void ila_destroy_state(struct lwtunnel_state *lwt)
> +{
> + dst_cache_destroy(&ila_lwt_lwtunnel(lwt)->dst_cache);
> +}
> +
> static int ila_fill_encap_info(struct sk_buff *skb,
> struct lwtunnel_state *lwtstate)
> {
> @@ -159,6 +226,7 @@ static int ila_encap_cmp(struct lwtunnel_state *a, struct lwtunnel_state *b)
>
> static const struct lwtunnel_encap_ops ila_encap_ops = {
> .build_state = ila_build_state,
> + .destroy_state = ila_destroy_state,
> .output = ila_output,
> .input = ila_input,
> .fill_encap = ila_fill_encap_info,
^ permalink raw reply
* Re: [PATCH net 2/5] net/ncsi: Split out logic for ncsi_dev_state_suspend_select
From: Joel Stanley @ 2016-10-14 6:02 UTC (permalink / raw)
To: Gavin Shan; +Cc: netdev, davem
In-Reply-To: <1476413614-24586-3-git-send-email-gwshan@linux.vnet.ibm.com>
Hi Gavin,
On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> This splits out the code that handles ncsi_dev_state_suspend_select
> so that we can add more code to the handler in subsequent patch.
> Apart from adding a error tag to reuse the code in error path,
> no logical changes introduced.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
> net/ncsi/ncsi-manage.c | 38 +++++++++++++++++++++++++-------------
> 1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 1bc96dc..5758a26 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -540,21 +540,30 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
> nd->state = ncsi_dev_state_suspend_select;
> /* Fall through */
> case ncsi_dev_state_suspend_select:
> + ndp->pending_req_num = 1;
> +
> + nca.type = NCSI_PKT_CMD_SP;
> + nca.package = np->id;
> + nca.channel = NCSI_RESERVED_CHANNEL;
> + if (ndp->flags & NCSI_DEV_HWA)
> + nca.bytes[0] = 0;
> + else
> + nca.bytes[0] = 1;
> +
> + nd->state = ncsi_dev_state_suspend_dcnt;
> +
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + goto error;
> +
> + break;
> case ncsi_dev_state_suspend_dcnt:
> case ncsi_dev_state_suspend_dc:
> case ncsi_dev_state_suspend_deselect:
> ndp->pending_req_num = 1;
>
> nca.package = np->id;
> - if (nd->state == ncsi_dev_state_suspend_select) {
> - nca.type = NCSI_PKT_CMD_SP;
> - nca.channel = NCSI_RESERVED_CHANNEL;
> - if (ndp->flags & NCSI_DEV_HWA)
> - nca.bytes[0] = 0;
> - else
> - nca.bytes[0] = 1;
> - nd->state = ncsi_dev_state_suspend_dcnt;
> - } else if (nd->state == ncsi_dev_state_suspend_dcnt) {
> + if (nd->state == ncsi_dev_state_suspend_dcnt) {
> nca.type = NCSI_PKT_CMD_DCNT;
> nca.channel = nc->id;
> nd->state = ncsi_dev_state_suspend_dc;
This is a messy switch statement. How about break out out all of the
states as you've done with suspend_select, instead of grouping them
and then doing if ... else if .. else if. I realise there might be one
or two lines duplicated for each state, but I think that's okay at the
expense of readability.
Also, patch 1 could also be merged into this when making this cleanup.
What do you think?
> @@ -570,10 +579,8 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
> }
>
> ret = ncsi_xmit_cmd(&nca);
> - if (ret) {
> - nd->state = ncsi_dev_state_functional;
> - return;
> - }
> + if (ret)
> + goto error;
>
> break;
> case ncsi_dev_state_suspend_done:
> @@ -587,6 +594,11 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
> netdev_warn(nd->dev, "Wrong NCSI state 0x%x in suspend\n",
> nd->state);
> }
> +
> + return;
> +
> +error:
> + nd->state = ncsi_dev_state_functional;
> }
>
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH v2 1/3] net: smc91x: isolate u16 writes alignment workaround
From: Robert Jarzmik @ 2016-10-14 6:12 UTC (permalink / raw)
To: David Miller
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
nico-vtqb6HGKxmzR7s880joybQ, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
arnd-r2nGTMty4D4, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161013.095012.244532868281403236.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> writes:
> From: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> Date: Sun, 9 Oct 2016 22:33:45 +0200
>
>> Writes to u16 has a special handling on 3 PXA platforms, where the
>> hardware wiring forces these writes to be u32 aligned.
>>
>> This patch isolates this handling for PXA platforms as before, but
>> enables this "workaround" to be set up dynamically, which will be the
>> case in device-tree build types.
>>
>> This patch was tested on 2 PXA platforms : mainstone, which relies on
>> the workaround, and lubbock, which doesn't.
>>
>> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
>
> Please resubmit this patch series:
>
> 1) Respun against net-next, these don't currently apply cleanly there.
>
> 2) With a proper "[PATCH 0/3] ..." posting explaining at a high level
> what this patch series does, how it does it, and why it does it
> that way.
Sure, let me retest it after the rebase, and I'll post again.
Cheers.
--
Robert
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net 3/5] net/ncsi: Fix stale link state of inactive channels on failover
From: Joel Stanley @ 2016-10-14 6:02 UTC (permalink / raw)
To: Gavin Shan; +Cc: netdev, davem
In-Reply-To: <1476413614-24586-4-git-send-email-gwshan@linux.vnet.ibm.com>
On Fri, Oct 14, 2016 at 1:23 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> The issue was found on BCM5718 which has two NCSI channels in one
> package: C0 and C1. Both of them are connected to different LANs,
> means they are in link-up state and C0 is chosen as the active
> one until resetting BCM5718 happens as below.
>
> Resetting BCM5718 results in LSC (Link State Change) AEN packet
> received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
> received on C0 to report link-down, it fails over to C1 because C1
> is in link-up state as software can see. However, C1 is in link-down
> state in hardware. It means the link state is out of synchronization
> between hardware and software, resulting in inappropriate channel (C1)
> selected as active one.
>
> This resolves the issue by sending separate GLS (Get Link Status)
> commands to all channels in the package before trying to do failover.
> The last link state on all channels in the package is retrieved. With
> it, C0 is selected as active one as expected.
I follow this, and can see that happening in the
ncsi_dev_state_suspend_gls state. However, what is
> - nd->state = ncsi_dev_state_suspend_dcnt;
> + if (ndp->flags & NCSI_DEV_RESHUFFLE)
> + nd->state = ncsi_dev_state_suspend_gls;
> + else
> + nd->state = ncsi_dev_state_suspend_dcnt;
However, what is this doing? I'm not quite sure what
NCSI_DEV_RESHUFFLE is and why we enable it?
>
> ret = ncsi_xmit_cmd(&nca);
> if (ret)
> goto error;
>
> break;
> + case ncsi_dev_state_suspend_gls:
> + ndp->pending_req_num = np->channel_num;
> +
> + nca.type = NCSI_PKT_CMD_GLS;
> + nca.package = np->id;
> + nd->state = ncsi_dev_state_suspend_dcnt;
> +
> + NCSI_FOR_EACH_CHANNEL(np, nc) {
> + nca.channel = nc->id;
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + goto error;
> + }
> +
> + break;
> case ncsi_dev_state_suspend_dcnt:
> case ncsi_dev_state_suspend_dc:
> case ncsi_dev_state_suspend_deselect:
> --
> 2.1.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox