Netdev List
 help / color / mirror / Atom feed
* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: Andrew Lunn @ 2016-05-06 15:27 UTC (permalink / raw)
  To: Dean Jenkins
  Cc: Guodong Xu, John Stultz, lkml, Mark Craske, David S. Miller,
	YongQin Liu, linux-usb, netdev, Ivan Vecera, David B. Robins
In-Reply-To: <572CB188.7050203@mentor.com>

> In other words, the full-speed hub is restricting the USB to
> Ethernet Adaptor to a 12Mbps (half-duplex) bandwidth to support
> Ethernet 100Mbps (full-duplex) traffic. That is not going to work
> very well because Ethernet frames (perhaps partial Ethernet frames)
> need to be discarded within the USB link.

If that really is true, the design is broken. I would expect the
adaptor to reliably transfer whole frames over USB, and drop whole
frames from its receive queue when the USB is congested. TCP is also
going to see the USB bottleneck as just like any bottleneck in the
network and back off. So TCP streams should not cause major congestion
on the USB link. Going over a 12Mbps USB link should be no different
to hitting an old Ethernet hub which can only do 10/Half.

> Therefore please retest with a working high-speed USB hub or remove
> the full-speed USB hub from the test environment and directly
> connect the USB to Ethernet Adaptor to the root hub of the USB port.
> Then repeat the tests to see whether anything improved.
> 
> In other words, you need to eliminate the dmesg messages saying "not
> running at top speed; connect to a high speed hub".

I would also suggest testing with the Ethernet at 10/half. You should
be able to use Ethtool to set that up. Your USB and Ethernet bandwidth
become more equal. If you still see errors, it suggests a protocol
implementation error somewhere.

	 Andrew

^ permalink raw reply

* Re: [PATCH iproute2] geneve: fix IPv6 remote address reporting
From: Phil Sutter @ 2016-05-06 15:28 UTC (permalink / raw)
  To: Edward Cree; +Cc: Stephen Hemminger, netdev, John W. Linville
In-Reply-To: <572CB4C3.3040708@solarflare.com>

On Fri, May 06, 2016 at 04:14:11PM +0100, Edward Cree wrote:
> On 06/05/16 15:43, Phil Sutter wrote:
> > On Fri, May 06, 2016 at 03:28:25PM +0100, Edward Cree wrote:
> >> Since we can only configure unicast, we probably want to be able to
> >> display unicast, rather than multicast.
> > Furthermore, the kernel even rejects multicast peer addresses.
> Yes, but a future kernel might not, and iproute2 is meant to be forward-
> compatible.

Sorry, but I fail to see how this might break forward compatibility.
Quite the contrary, suppose geneve in future supported multicast peers,
current iproute2 would fail to recognize it's existence. What am I
missing here?

> > Why do you then propose a dubious fix to a dubious check instead of
> > getting rid of it in the first place?
> Because John Linville clearly had some reason for putting a check there,
> and he probably knows better than me.  Chesterton's fence.

A valid point, indeed. In my opinion the same applies to your patch as
well, as instead of removing the fence you're moving it to the other
lane. :)

Cheers, Phil

^ permalink raw reply

* [PATCH] Add support for configuring Infiniband GUIDs
From: Eli Cohen @ 2016-05-06 15:43 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Eli Cohen

Add two NLA's that allow configuration of Infiniband node or port GUIDs
by referencing the IPoIB net device set over then physical function. The
format to be used is as follows:

ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70
ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78

Issue: 702759
Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e
Signed-off-by: Eli Cohen <eli@mellanox.com>
---
 ip/iplink.c           | 40 ++++++++++++++++++++++++++++++++++++++++
 man/man8/ip-link.8.in | 12 +++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/ip/iplink.c b/ip/iplink.c
index d2e586b6d133..3f885defdfeb 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -237,6 +237,30 @@ struct iplink_req {
 	char			buf[1024];
 };
 
+static int extract_guid(__u64 *guid, char *arg)
+{
+	__u64 ret;
+	int g[8];
+	int err;
+
+	err = sscanf(arg, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x",
+		     g, g + 1, g + 2, g + 3, g + 4, g + 5, g + 6, g + 7);
+	if (err != 8)
+		return -1;
+
+	ret = ((__u64)(g[0]) << 56) |
+	      ((__u64)(g[1]) << 48) |
+	      ((__u64)(g[2]) << 40) |
+	      ((__u64)(g[3]) << 32) |
+	      ((__u64)(g[4]) << 24) |
+	      ((__u64)(g[5]) << 16) |
+	      ((__u64)(g[6]) << 8) |
+	      ((__u64)(g[7]));
+	*guid = ret;
+
+	return 0;
+}
+
 static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 			   struct iplink_req *req, int dev_index)
 {
@@ -383,6 +407,22 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 				invarg("Invalid \"state\" value\n", *argv);
 			ivl.vf = vf;
 			addattr_l(&req->n, sizeof(*req), IFLA_VF_LINK_STATE, &ivl, sizeof(ivl));
+		} else if (matches(*argv, "node_guid") == 0) {
+			struct ifla_vf_guid ivg;
+
+			NEXT_ARG();
+			ivg.vf = vf;
+			if (extract_guid(&ivg.guid, *argv))
+				return -1;
+			addattr_l(&req->n, sizeof(*req), IFLA_VF_IB_NODE_GUID, &ivg, sizeof(ivg));
+		} else if (matches(*argv, "port_guid") == 0) {
+			struct ifla_vf_guid ivg;
+
+			NEXT_ARG();
+			ivg.vf = vf;
+			if (extract_guid(&ivg.guid, *argv))
+				return -1;
+			addattr_l(&req->n, sizeof(*req), IFLA_VF_IB_PORT_GUID, &ivg, sizeof(ivg));
 		} else {
 			/* rewind arg */
 			PREV_ARG();
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 805511423ef2..e143a5ec8a9a 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -143,7 +143,11 @@ ip-link \- network device configuration
 .br
 .RB "[ " state " { " auto " | " enable " | " disable " } ]"
 .br
-.RB "[ " trust " { " on " | " off " } ] ]"
+.RB "[ " trust " { " on " | " off " } ]"
+.br
+.RB "[ " node_guid " eui64 ]"
+.br
+.RB "[ " port_guid " eui64 ] ]"
 .br
 .in -9
 .RB "[ " master
@@ -1033,6 +1037,12 @@ sent by the VF.
 .BI trust " on|off"
 - trust the specified VF user. This enables that VF user can set a specific feature
 which may impact security and/or performance. (e.g. VF multicast promiscuous mode)
+.sp
+.BI node_guid " eui64"
+- configure node GUID for the VF.
+.sp
+.BI port_guid " eui64"
+- configure port GUID for the VF.
 .in -8
 
 .TP
-- 
2.8.1

^ permalink raw reply related

* [PATCH net-next] fq_codel: add memory limitation per queue
From: Eric Dumazet @ 2016-05-06 15:55 UTC (permalink / raw)
  To: David Miller; +Cc: Jesper Dangaard Brouer, Dave Täht, netdev, moeller0
In-Reply-To: <1462541156.13075.34.camel@edumazet-glaptop3.roam.corp.google.com>

From: Eric Dumazet <edumazet@google.com>

On small embedded routers, one wants to control maximal amount of
memory used by fq_codel, instead of controlling number of packets or
bytes, since GRO/TSO make these not practical.

Assuming skb->truesize is accurate, we have to keep track of
skb->truesize sum for skbs in queue.

This patch adds a new TCA_FQ_CODEL_MEMORY_LIMIT attribute.

I chose a default value of 32 MBytes, which looks reasonable even
for heavy duty usages. (Prior fq_codel users should not be hurt
when they upgrade their kernels)

Two fields are added to tc_fq_codel_qd_stats to report :
 - Current memory usage
 - Number of drops caused by memory limits

# tc qd replace dev eth1 root est 1sec 4sec fq_codel memory_limit 4M
..
# tc -s -d qd sh dev eth1
qdisc fq_codel 8008: root refcnt 257 limit 10240p flows 1024
 quantum 1514 target 5.0ms interval 100.0ms memory_limit 4Mb ecn 
 Sent 2083566791363 bytes 1376214889 pkt (dropped 4994406, overlimits 0
requeues 21705223) 
 rate 9841Mbit 812549pps backlog 3906120b 376p requeues 21705223 
  maxpacket 68130 drop_overlimit 4994406 new_flow_count 28855414
  ecn_mark 0 memory_used 4190048 drop_overmemory 4994406
  new_flows_len 1 old_flows_len 177


Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Dave Täht <dave.taht@gmail.com>
Cc: Sebastian Möller <moeller0@gmx.de>
---
 include/uapi/linux/pkt_sched.h |    3 +++
 net/sched/sch_fq_codel.c       |   27 ++++++++++++++++++++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index a11afecd4482..2382eed50278 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -719,6 +719,7 @@ enum {
 	TCA_FQ_CODEL_QUANTUM,
 	TCA_FQ_CODEL_CE_THRESHOLD,
 	TCA_FQ_CODEL_DROP_BATCH_SIZE,
+	TCA_FQ_CODEL_MEMORY_LIMIT,
 	__TCA_FQ_CODEL_MAX
 };
 
@@ -743,6 +744,8 @@ struct tc_fq_codel_qd_stats {
 	__u32	new_flows_len;	/* count of flows in new list */
 	__u32	old_flows_len;	/* count of flows in old list */
 	__u32	ce_mark;	/* packets above ce_threshold */
+	__u32	memory_usage;	/* in bytes */
+	__u32	drop_overmemory;
 };
 
 struct tc_fq_codel_cl_stats {
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index e7b42b0d5145..bb8bd9314629 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -60,8 +60,11 @@ struct fq_codel_sched_data {
 	u32		perturbation;	/* hash perturbation */
 	u32		quantum;	/* psched_mtu(qdisc_dev(sch)); */
 	u32		drop_batch_size;
+	u32		memory_limit;
 	struct codel_params cparams;
 	struct codel_stats cstats;
+	u32		memory_usage;
+	u32		drop_overmemory;
 	u32		drop_overlimit;
 	u32		new_flow_count;
 
@@ -143,6 +146,7 @@ static unsigned int fq_codel_drop(struct Qdisc *sch, unsigned int max_packets)
 	unsigned int maxbacklog = 0, idx = 0, i, len;
 	struct fq_codel_flow *flow;
 	unsigned int threshold;
+	unsigned int mem = 0;
 
 	/* Queue is full! Find the fat flow and drop packet(s) from it.
 	 * This might sound expensive, but with 1024 flows, we scan
@@ -167,11 +171,13 @@ static unsigned int fq_codel_drop(struct Qdisc *sch, unsigned int max_packets)
 	do {
 		skb = dequeue_head(flow);
 		len += qdisc_pkt_len(skb);
+		mem += skb->truesize;
 		kfree_skb(skb);
 	} while (++i < max_packets && len < threshold);
 
 	flow->dropped += i;
 	q->backlogs[idx] -= len;
+	q->memory_usage -= mem;
 	sch->qstats.drops += i;
 	sch->qstats.backlog -= len;
 	sch->q.qlen -= i;
@@ -193,6 +199,7 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	unsigned int idx, prev_backlog, prev_qlen;
 	struct fq_codel_flow *flow;
 	int uninitialized_var(ret);
+	bool memory_limited;
 
 	idx = fq_codel_classify(skb, sch, &ret);
 	if (idx == 0) {
@@ -215,7 +222,9 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 		flow->deficit = q->quantum;
 		flow->dropped = 0;
 	}
-	if (++sch->q.qlen <= sch->limit)
+	q->memory_usage += skb->truesize;
+	memory_limited = q->memory_usage > q->memory_limit;
+	if (++sch->q.qlen <= sch->limit && !memory_limited)
 		return NET_XMIT_SUCCESS;
 
 	prev_backlog = sch->qstats.backlog;
@@ -229,7 +238,8 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 	ret = fq_codel_drop(sch, q->drop_batch_size);
 
 	q->drop_overlimit += prev_qlen - sch->q.qlen;
-
+	if (memory_limited)
+		q->drop_overmemory += prev_qlen - sch->q.qlen;
 	/* As we dropped packet(s), better let upper stack know this */
 	qdisc_tree_reduce_backlog(sch, prev_qlen - sch->q.qlen,
 				  prev_backlog - sch->qstats.backlog);
@@ -308,6 +318,7 @@ begin:
 			list_del_init(&flow->flowchain);
 		goto begin;
 	}
+	q->memory_usage -= skb->truesize;
 	qdisc_bstats_update(sch, skb);
 	flow->deficit -= qdisc_pkt_len(skb);
 	/* We cant call qdisc_tree_reduce_backlog() if our qlen is 0,
@@ -355,6 +366,7 @@ static const struct nla_policy fq_codel_policy[TCA_FQ_CODEL_MAX + 1] = {
 	[TCA_FQ_CODEL_QUANTUM]	= { .type = NLA_U32 },
 	[TCA_FQ_CODEL_CE_THRESHOLD] = { .type = NLA_U32 },
 	[TCA_FQ_CODEL_DROP_BATCH_SIZE] = { .type = NLA_U32 },
+	[TCA_FQ_CODEL_MEMORY_LIMIT] = { .type = NLA_U32 },
 };
 
 static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt)
@@ -409,7 +421,11 @@ static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt)
 	if (tb[TCA_FQ_CODEL_DROP_BATCH_SIZE])
 		q->drop_batch_size = min(1U, nla_get_u32(tb[TCA_FQ_CODEL_DROP_BATCH_SIZE]));
 
-	while (sch->q.qlen > sch->limit) {
+	if (tb[TCA_FQ_CODEL_MEMORY_LIMIT])
+		q->memory_limit = min(1U << 31, nla_get_u32(tb[TCA_FQ_CODEL_MEMORY_LIMIT]));
+
+	while (sch->q.qlen > sch->limit ||
+	       q->memory_usage > q->memory_limit) {
 		struct sk_buff *skb = fq_codel_dequeue(sch);
 
 		q->cstats.drop_len += qdisc_pkt_len(skb);
@@ -454,6 +470,7 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt)
 
 	sch->limit = 10*1024;
 	q->flows_cnt = 1024;
+	q->memory_limit = 32 << 20; /* 32 MBytes */
 	q->drop_batch_size = 64;
 	q->quantum = psched_mtu(qdisc_dev(sch));
 	q->perturbation = prandom_u32();
@@ -515,6 +532,8 @@ static int fq_codel_dump(struct Qdisc *sch, struct sk_buff *skb)
 			q->quantum) ||
 	    nla_put_u32(skb, TCA_FQ_CODEL_DROP_BATCH_SIZE,
 			q->drop_batch_size) ||
+	    nla_put_u32(skb, TCA_FQ_CODEL_MEMORY_LIMIT,
+			q->memory_limit) ||
 	    nla_put_u32(skb, TCA_FQ_CODEL_FLOWS,
 			q->flows_cnt))
 		goto nla_put_failure;
@@ -543,6 +562,8 @@ static int fq_codel_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 	st.qdisc_stats.ecn_mark = q->cstats.ecn_mark;
 	st.qdisc_stats.new_flow_count = q->new_flow_count;
 	st.qdisc_stats.ce_mark = q->cstats.ce_mark;
+	st.qdisc_stats.memory_usage  = q->memory_usage;
+	st.qdisc_stats.drop_overmemory = q->drop_overmemory;
 
 	list_for_each(pos, &q->new_flows)
 		st.qdisc_stats.new_flows_len++;

^ permalink raw reply related

* [PATCH] Documentation/networking: more accurate LCO explanation
From: Shmulik Ladkani @ 2016-05-06 15:57 UTC (permalink / raw)
  To: David S . Miller, netdev; +Cc: Edward Cree, shmulik.ladkani

In few places the term "ones-complement sum" was used but the actual
meaning is "the complement of the ones-complement sum".

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---

 I assume readers interpret the term "ones-complement sum" as the sum
 using one's complement arithmentic, without the final bitwise
 complement of sum's result.
 Hence I added "the complement of" where applicable.

 Documentation/networking/checksum-offloads.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
index de2a327766..9567200e1f 100644
--- a/Documentation/networking/checksum-offloads.txt
+++ b/Documentation/networking/checksum-offloads.txt
@@ -69,17 +69,17 @@ LCO: Local Checksum Offload
 LCO is a technique for efficiently computing the outer checksum of an
  encapsulated datagram when the inner checksum is due to be offloaded.
 The ones-complement sum of a correctly checksummed TCP or UDP packet is
- equal to the sum of the pseudo header, because everything else gets
- 'cancelled out' by the checksum field.  This is because the sum was
+ equal to the complement of the sum of the pseudo header, because everything
+ else gets 'cancelled out' by the checksum field.  This is because the sum was
  complemented before being written to the checksum field.
 More generally, this holds in any case where the 'IP-style' ones complement
  checksum is used, and thus any checksum that TX Checksum Offload supports.
 That is, if we have set up TX Checksum Offload with a start/offset pair, we
  know that _after the device has filled in that checksum_, the ones
  complement sum from csum_start to the end of the packet will be equal to
- _whatever value we put in the checksum field beforehand_.  This allows us
- to compute the outer checksum without looking at the payload: we simply
- stop summing when we get to csum_start, then add the 16-bit word at
+ the complement of _whatever value we put in the checksum field beforehand_.
+ This allows us to compute the outer checksum without looking at the payload:
+ we simply stop summing when we get to csum_start, then add the 16-bit word at
  (csum_start + csum_offset).
 Then, when the true inner checksum is filled in (either by hardware or by
  skb_checksum_help()), the outer checksum will become correct by virtue of
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
From: Larry Finger @ 2016-05-06 16:12 UTC (permalink / raw)
  To: Wang YanQing, chaoming_li, kvalo, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20160505171904.GA16619@udknight>

On 05/05/2016 12:19 PM, Wang YanQing wrote:
> We can't use kfree_skb in irq disable context, because spin_lock_irqsave
> make sure we are always in irq disable context, use dev_kfree_skb_irq
> instead of kfree_skb is better than dev_kfree_skb_any.
>
> This patch fix below kernel warning:
> [ 7612.095528] ------------[ cut here ]------------
> [ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150 __local_bh_enable_ip+0x58/0x80()
> [ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal btcoexist rtl_pci rtlwifi rtl8723_common
> [ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G        W       4.4.0+ #4
> [ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW (1.19 ) 08/27/2015
> [ 7612.095574]  00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0 c104cc59 c19d4454
> [ 7612.095584]  00000003 0000116c c19d4784 00000096 c10508a8 c10508a8 00000200 c1b42400
> [ 7612.095594]  f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc c10508a8 f21f08b8
> [ 7612.095604] Call Trace:
> [ 7612.095614]  [<c12ce7c5>] dump_stack+0x41/0x5c
> [ 7612.095620]  [<c104cc59>] warn_slowpath_common+0x89/0xc0
> [ 7612.095628]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
> [ 7612.095634]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
> [ 7612.095640]  [<c104ccad>] warn_slowpath_null+0x1d/0x20
> [ 7612.095646]  [<c10508a8>] __local_bh_enable_ip+0x58/0x80
> [ 7612.095653]  [<c16b7d34>] destroy_conntrack+0x64/0xa0
> [ 7612.095660]  [<c16b300f>] nf_conntrack_destroy+0xf/0x20
> [ 7612.095665]  [<c1677565>] skb_release_head_state+0x55/0xa0
> [ 7612.095670]  [<c16775bb>] skb_release_all+0xb/0x20
> [ 7612.095674]  [<c167760b>] __kfree_skb+0xb/0x60
> [ 7612.095679]  [<c16776f0>] kfree_skb+0x30/0x70
> [ 7612.095686]  [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
> [ 7612.095692]  [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
> [ 7612.095698]  [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
> [ 7612.095705]  [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
> [ 7612.095712]  [<c17e3f16>] drv_start+0x36/0xc0
> [ 7612.095717]  [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
> [ 7612.095725]  [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
> [ 7612.095730]  [<c17f60bd>] ieee80211_open+0x4d/0x50
> [ 7612.095736]  [<c16891b3>] __dev_open+0xa3/0x130
> [ 7612.095742]  [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
> [ 7612.095748]  [<c1689499>] __dev_change_flags+0x89/0x140
> [ 7612.095753]  [<c127c70d>] ? selinux_capable+0xd/0x10
> [ 7612.095759]  [<c1689589>] dev_change_flags+0x29/0x60
> [ 7612.095765]  [<c1700b93>] devinet_ioctl+0x553/0x670
> [ 7612.095772]  [<c12db758>] ? _copy_to_user+0x28/0x40
> [ 7612.095777]  [<c17018b5>] inet_ioctl+0x85/0xb0
> [ 7612.095783]  [<c166e647>] sock_ioctl+0x67/0x260
> [ 7612.095788]  [<c166e5e0>] ? sock_fasync+0x80/0x80
> [ 7612.095795]  [<c115c99b>] do_vfs_ioctl+0x6b/0x550
> [ 7612.095800]  [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
> [ 7612.095807]  [<c10a8914>] ? timekeeping_suspend+0x294/0x320
> [ 7612.095813]  [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
> [ 7612.095820]  [<c1276e24>] ? security_file_ioctl+0x34/0x50
> [ 7612.095827]  [<c115cef0>] SyS_ioctl+0x70/0x80
> [ 7612.095832]  [<c1001804>] do_fast_syscall_32+0x84/0x120
> [ 7612.095839]  [<c183ff91>] sysenter_past_esp+0x36/0x55
> [ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---
>
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> ---
>   drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index 1ac41b8..99a3a03 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
>   							 true,
>   							 HW_DESC_TXBUFF_ADDR),
>   						 skb->len, PCI_DMA_TODEVICE);
> -				kfree_skb(skb);
> +				dev_kfree_skb_irq(skb);
>   				ring->idx = (ring->idx + 1) % ring->entries;
>   			}
>   			ring->idx = 0;

After testing, this patch is OK other than needing a Cc to stable. Please fix 
that and resubmit V2.

Larry

^ permalink raw reply

* Re: [PATCH] Documentation/networking: more accurate LCO explanation
From: Edward Cree @ 2016-05-06 16:12 UTC (permalink / raw)
  To: Shmulik Ladkani, David S . Miller, netdev
In-Reply-To: <1462550263-9742-1-git-send-email-shmulik.ladkani@gmail.com>

On 06/05/16 16:57, Shmulik Ladkani wrote:
> In few places the term "ones-complement sum" was used but the actual
> meaning is "the complement of the ones-complement sum".
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Acked-by: Edward Cree <ecree@solarflare.com>

^ permalink raw reply

* RE: [Intel-wired-lan] NULL dereference on v4.1.x while enabling VF
From: Skidmore, Donald C @ 2016-05-06 16:26 UTC (permalink / raw)
  To: William Dauchy; +Cc: NETDEV, intel-wired-lan@lists.osuosl.org, Alex Duyck
In-Reply-To: <CAJ75kXY_PMK0iTyM5bbhQoAS9je5AqpW67jyZpJEUoNw0BPcqw@mail.gmail.com>

Hey William,

My validation hasn't be able to recreate the dereference on v4.1.x, v4.5.x or net_next.  Where exactly did you place the two line script in your rc scripts.  Our validation was able to run it as soon as ~14 second after the first boot message logged in dmesg.  Is this anywhere close to where you were executing it?  Likewise he has attempted running at both run level 3 and 5 in case that changed any of the times of how soon the rc scripts are executed.  So the more detail you can give of setup the more it might help us recreate what you are seeing.

Thanks,
-Don Skidmore <donald.c.skidmore>

 

> -----Original Message-----
> From: William Dauchy [mailto:wdauchy@gmail.com]
> Sent: Tuesday, May 03, 2016 5:33 AM
> To: Skidmore, Donald C <donald.c.skidmore@intel.com>
> Cc: NETDEV <netdev@vger.kernel.org>; intel-wired-lan@lists.osuosl.org;
> Alex Duyck <aduyck@mirantis.com>
> Subject: Re: [Intel-wired-lan] NULL dereference on v4.1.x while enabling VF
> 
> Hello Don,
> 
> Thank you for your reply.
> 
> On Mon, May 2, 2016 at 11:33 PM, Skidmore, Donald C
> <donald.c.skidmore@intel.com> wrote:
> > Thanks for reporting the dereference.  Could you provide a little more
> detail on how you created this issue?  Are you just running the two
> commands (ip, sriov_numvfs) in some rc script and if you put a few second
> sleep in front of it you don't see the failure?
> 
> Your understanding is correct; a rc script is run with ip and echo in numvfs
> commands. I tried to reduce it to the minimum. If I put a sleep
> 20 in front of it, it does not crash. I also forgot to add I did not had the issue
> in 3.14.x with the same script.
> 
> Best,
> --
> William

^ permalink raw reply

* Re: [PATCH] Documentation/networking: more accurate LCO explanation
From: Alexander Duyck @ 2016-05-06 16:29 UTC (permalink / raw)
  To: Shmulik Ladkani; +Cc: David S . Miller, Netdev, Edward Cree
In-Reply-To: <1462550263-9742-1-git-send-email-shmulik.ladkani@gmail.com>

On Fri, May 6, 2016 at 8:57 AM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> In few places the term "ones-complement sum" was used but the actual
> meaning is "the complement of the ones-complement sum".

Looks like there might still be a few minor corrections needed.
Comments inline below.

>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> ---
>
>  I assume readers interpret the term "ones-complement sum" as the sum
>  using one's complement arithmentic, without the final bitwise
>  complement of sum's result.
>  Hence I added "the complement of" where applicable.
>
>  Documentation/networking/checksum-offloads.txt | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
> index de2a327766..9567200e1f 100644
> --- a/Documentation/networking/checksum-offloads.txt
> +++ b/Documentation/networking/checksum-offloads.txt
> @@ -69,17 +69,17 @@ LCO: Local Checksum Offload
>  LCO is a technique for efficiently computing the outer checksum of an
>   encapsulated datagram when the inner checksum is due to be offloaded.
>  The ones-complement sum of a correctly checksummed TCP or UDP packet is
> - equal to the sum of the pseudo header, because everything else gets
> - 'cancelled out' by the checksum field.  This is because the sum was
> + equal to the complement of the sum of the pseudo header, because everything
> + else gets 'cancelled out' by the checksum field.  This is because the sum was
>   complemented before being written to the checksum field.
>  More generally, this holds in any case where the 'IP-style' ones complement
>   checksum is used, and thus any checksum that TX Checksum Offload supports.
>  That is, if we have set up TX Checksum Offload with a start/offset pair, we
>   know that _after the device has filled in that checksum_, the ones
>   complement sum from csum_start to the end of the packet will be equal to
> - _whatever value we put in the checksum field beforehand_.  This allows us
> - to compute the outer checksum without looking at the payload: we simply
> - stop summing when we get to csum_start, then add the 16-bit word at
> + the complement of _whatever value we put in the checksum field beforehand_.

I don't really see the point of using an underscore before and after
that statement.  If it was only one or two words it might work for
emphasis but the statement is large enough that starting it with an
underscore just makes it harder to read.

> + This allows us to compute the outer checksum without looking at the payload:
> + we simply stop summing when we get to csum_start, then add the 16-bit word at
>   (csum_start + csum_offset).

You don't add the 16-bit word you add the compliment of the 16 bit word.

^ permalink raw reply

* [PATCH net-next] ipv4: tcp: ip_send_unicast_reply() is not BH safe
From: Eric Dumazet @ 2016-05-06 16:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Andres Lagar-Cavilla, edumazet

From: Eric Dumazet <edumazet@google.com>

I forgot that ip_send_unicast_reply() is not BH safe (yet).

Disabling preemption before calling it was not a good move.

Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Andres Lagar-Cavilla  <andreslc@google.com>
---
 net/ipv4/tcp_ipv4.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a7ab9472d64560d86ea24ac1b6e1a7800f89989d..8219d0d8dc8370d0d3e6fc4cd17b4925617968ab 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -692,7 +692,7 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 		     offsetof(struct inet_timewait_sock, tw_bound_dev_if));
 
 	arg.tos = ip_hdr(skb)->tos;
-	preempt_disable();
+	local_bh_disable();
 	ip_send_unicast_reply(*this_cpu_ptr(net->ipv4.tcp_sk),
 			      skb, &TCP_SKB_CB(skb)->header.h4.opt,
 			      ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
@@ -700,7 +700,7 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb)
 
 	__TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
 	__TCP_INC_STATS(net, TCP_MIB_OUTRSTS);
-	preempt_enable();
+	local_bh_enable();
 
 #ifdef CONFIG_TCP_MD5SIG
 out:
@@ -776,14 +776,14 @@ static void tcp_v4_send_ack(struct net *net,
 	if (oif)
 		arg.bound_dev_if = oif;
 	arg.tos = tos;
-	preempt_disable();
+	local_bh_disable();
 	ip_send_unicast_reply(*this_cpu_ptr(net->ipv4.tcp_sk),
 			      skb, &TCP_SKB_CB(skb)->header.h4.opt,
 			      ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
 			      &arg, arg.iov[0].iov_len);
 
 	__TCP_INC_STATS(net, TCP_MIB_OUTSEGS);
-	preempt_enable();
+	local_bh_enable();
 }
 
 static void tcp_v4_timewait_ack(struct sock *sk, struct sk_buff *skb)

^ permalink raw reply related

* [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
From: Wang YanQing @ 2016-05-06 16:33 UTC (permalink / raw)
  To: Larry.Finger; +Cc: chaoming_li, kvalo, linux-wireless, netdev, linux-kernel

We can't use kfree_skb in irq disable context, because spin_lock_irqsave
make sure we are always in irq disable context, use dev_kfree_skb_irq
instead of kfree_skb is better than dev_kfree_skb_any.

This patch fix below kernel warning:
[ 7612.095528] ------------[ cut here ]------------
[ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150 __local_bh_enable_ip+0x58/0x80()
[ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal btcoexist rtl_pci rtlwifi rtl8723_common
[ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G        W       4.4.0+ #4
[ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW (1.19 ) 08/27/2015
[ 7612.095574]  00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0 c104cc59 c19d4454
[ 7612.095584]  00000003 0000116c c19d4784 00000096 c10508a8 c10508a8 00000200 c1b42400
[ 7612.095594]  f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc c10508a8 f21f08b8
[ 7612.095604] Call Trace:
[ 7612.095614]  [<c12ce7c5>] dump_stack+0x41/0x5c
[ 7612.095620]  [<c104cc59>] warn_slowpath_common+0x89/0xc0
[ 7612.095628]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
[ 7612.095634]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
[ 7612.095640]  [<c104ccad>] warn_slowpath_null+0x1d/0x20
[ 7612.095646]  [<c10508a8>] __local_bh_enable_ip+0x58/0x80
[ 7612.095653]  [<c16b7d34>] destroy_conntrack+0x64/0xa0
[ 7612.095660]  [<c16b300f>] nf_conntrack_destroy+0xf/0x20
[ 7612.095665]  [<c1677565>] skb_release_head_state+0x55/0xa0
[ 7612.095670]  [<c16775bb>] skb_release_all+0xb/0x20
[ 7612.095674]  [<c167760b>] __kfree_skb+0xb/0x60
[ 7612.095679]  [<c16776f0>] kfree_skb+0x30/0x70
[ 7612.095686]  [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
[ 7612.095692]  [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
[ 7612.095698]  [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
[ 7612.095705]  [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
[ 7612.095712]  [<c17e3f16>] drv_start+0x36/0xc0
[ 7612.095717]  [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
[ 7612.095725]  [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
[ 7612.095730]  [<c17f60bd>] ieee80211_open+0x4d/0x50
[ 7612.095736]  [<c16891b3>] __dev_open+0xa3/0x130
[ 7612.095742]  [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
[ 7612.095748]  [<c1689499>] __dev_change_flags+0x89/0x140
[ 7612.095753]  [<c127c70d>] ? selinux_capable+0xd/0x10
[ 7612.095759]  [<c1689589>] dev_change_flags+0x29/0x60
[ 7612.095765]  [<c1700b93>] devinet_ioctl+0x553/0x670
[ 7612.095772]  [<c12db758>] ? _copy_to_user+0x28/0x40
[ 7612.095777]  [<c17018b5>] inet_ioctl+0x85/0xb0
[ 7612.095783]  [<c166e647>] sock_ioctl+0x67/0x260
[ 7612.095788]  [<c166e5e0>] ? sock_fasync+0x80/0x80
[ 7612.095795]  [<c115c99b>] do_vfs_ioctl+0x6b/0x550
[ 7612.095800]  [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
[ 7612.095807]  [<c10a8914>] ? timekeeping_suspend+0x294/0x320
[ 7612.095813]  [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
[ 7612.095820]  [<c1276e24>] ? security_file_ioctl+0x34/0x50
[ 7612.095827]  [<c115cef0>] SyS_ioctl+0x70/0x80
[ 7612.095832]  [<c1001804>] do_fast_syscall_32+0x84/0x120
[ 7612.095839]  [<c183ff91>] sysenter_past_esp+0x36/0x55
[ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---

Signed-off-by: Wang YanQing <udknight@gmail.com>
Cc: Stable <stable@vger.kernel.org>
---
 Changes:
 v1-v2:
 1: add a Cc to stable.

 drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index 1ac41b8..99a3a03 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
 							 true,
 							 HW_DESC_TXBUFF_ADDR),
 						 skb->len, PCI_DMA_TODEVICE);
-				kfree_skb(skb);
+				dev_kfree_skb_irq(skb);
 				ring->idx = (ring->idx + 1) % ring->entries;
 			}
 			ring->idx = 0;
-- 
1.8.5.6.2.g3d8a54e.dirty

^ permalink raw reply related

* Re: [PATCH] Documentation/networking: more accurate LCO explanation
From: David Miller @ 2016-05-06 16:50 UTC (permalink / raw)
  To: alexander.duyck; +Cc: shmulik.ladkani, netdev, ecree
In-Reply-To: <CAKgT0UcpbKVpip0pSaCOY7xVQ=3wwUxFing7Sht8ptH8VRB=FA@mail.gmail.com>

From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Fri, 6 May 2016 09:29:56 -0700

> I don't really see the point of using an underscore before and after
> that statement.  If it was only one or two words it might work for
> emphasis but the statement is large enough that starting it with an
> underscore just makes it harder to read.

Agreed.

^ permalink raw reply

* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: Dean Jenkins @ 2016-05-06 16:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Guodong Xu, John Stultz, lkml, Mark Craske, David S. Miller,
	YongQin Liu, linux-usb, netdev, Ivan Vecera, David B. Robins,
	Dean Jenkins
In-Reply-To: <20160506152732.GA23647@lunn.ch>

On 06/05/16 16:27, Andrew Lunn wrote:
>> In other words, the full-speed hub is restricting the USB to
>> Ethernet Adaptor to a 12Mbps (half-duplex) bandwidth to support
>> Ethernet 100Mbps (full-duplex) traffic. That is not going to work
>> very well because Ethernet frames (perhaps partial Ethernet frames)
>> need to be discarded within the USB link.
> If that really is true, the design is broken. I would expect the
> adaptor to reliably transfer whole frames over USB, and drop whole
> frames from its receive queue when the USB is congested. TCP is also
> going to see the USB bottleneck as just like any bottleneck in the
> network and back off. So TCP streams should not cause major congestion
> on the USB link.
The host's USB host controller polls the USB to Ethernet adaptor for 
more data. The USB to Ethernet adaptor cannot predict when the next poll 
request comes. The AX88772B can span Ethernet frames across multiple 
poll requests. This means it is possible get a partial Ethernet frame 
received in the USB host controller on one poll and it is assumed that 
the next poll (sometime in the near future) will get the remaining part 
of the Ethernet frame.

However, the USB to Ethernet adaptor does not contain an infinitely 
sized RX Ethernet buffer for the incoming Ethernet frames. I believe the 
USB to Ethernet adaptor is just a pipe and does not directly implement 
flow control for Ethernet frames so the RX buffer is going to overflow 
causing loss of whole Ethernet frames. I suspect the IP stack in the 
host computer implements flow control for Ethernet frames.

Because the AX88772B can span Ethernet frames across multiple poll 
requests there is a risk that the designers of the device could of 
implemented a solution to discard the remaining part of the Ethernet 
frame before the next poll arrives due to the RX buffer overflowing. I 
don't know the algorithm used in the AX88772B but there will be loss of 
data due to the mismatch in bandwidths. I agree that dropping whole 
Ethernet frames would be preferable to dropping partial Ethernet frames 
which would corrupt the data stream.

My suspicion is that the URB buffers are containing discontinues in the 
data stream because of lost data due to insufficient bandwidth on the 
USB link.

> Going over a 12Mbps USB link should be no different
> to hitting an old Ethernet hub which can only do 10/Half.
Not exactly, because USB is a transport link which is agnostic to the 
type of data that is flowing. It is up to the layers above USB to manage 
the data content.

In other words, the USB speed needs to be higher than the Ethernet speed 
to avoid mismatches in bandwidth.
>> Therefore please retest with a working high-speed USB hub or remove
>> the full-speed USB hub from the test environment and directly
>> connect the USB to Ethernet Adaptor to the root hub of the USB port.
>> Then repeat the tests to see whether anything improved.
>>
>> In other words, you need to eliminate the dmesg messages saying "not
>> running at top speed; connect to a high speed hub".
> I would also suggest testing with the Ethernet at 10/half. You should
> be able to use Ethtool to set that up. Your USB and Ethernet bandwidth
> become more equal. If you still see errors, it suggests a protocol
> implementation error somewhere.
I agree with the suggestion but I hope USB high speed (480Mbps) 
operation was the intended environment rather than the useless USB full 
speed (12Mbps) operation.

Let's hope that not using the USB hub improves things.

Regards,
Dean
>
> 	 Andrew

-- 
Dean Jenkins
Embedded Software Engineer
Linux Transportation Solutions
Mentor Embedded Software Division
Mentor Graphics (UK) Ltd.

^ permalink raw reply

* Re: [PATCHv2 net] bridge: fix igmp / mld query parsing
From: David Miller @ 2016-05-06 16:56 UTC (permalink / raw)
  To: linus.luessing; +Cc: netdev, bridge, linux-kernel, sw
In-Reply-To: <1462375502-8381-1-git-send-email-linus.luessing@c0d3.blue>

From: Linus Lüssing <linus.luessing@c0d3.blue>
Date: Wed,  4 May 2016 17:25:02 +0200

> With the newly introduced helper functions the skb pulling is hidden
> in the checksumming function - and undone before returning to the
> caller.
> 
> The IGMP and MLD query parsing functions in the bridge still
> assumed that the skb is pointing to the beginning of the IGMP/MLD
> message while it is now kept at the beginning of the IPv4/6 header.
> 
> If there is a querier somewhere else, then this either causes
> the multicast snooping to stay disabled even though it could be
> enabled. Or, if we have the querier enabled too, then this can
> create unnecessary IGMP / MLD query messages on the link.
> 
> Fixing this by taking the offset between IP and IGMP/MLD header into
> account, too.
> 
> Fixes: 9afd85c9e455 ("net: Export IGMP/MLD message validation code")
> Reported-by: Simon Wunderlich <sw@simonwunderlich.de>
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net v3] vlan: Propagate MAC address to VLANs
From: Alexander Duyck @ 2016-05-06 17:02 UTC (permalink / raw)
  To: Mike Manning; +Cc: Netdev
In-Reply-To: <572C9B9E.1000304@brocade.com>

On Fri, May 6, 2016 at 6:26 AM, Mike Manning <mmanning@brocade.com> wrote:
> The MAC address of the physical interface is only copied to the VLAN
> when it is first created, resulting in an inconsistency after MAC
> address changes of only newly created VLANs having an up-to-date MAC.
>
> The VLANs should continue inheriting the MAC address of the physical
> interface, unless explicitly changed to be different from this.
> This allows IPv6 EUI64 addresses for the VLAN to reflect any changes
> to the MAC of the physical interface and thus for DAD to behave as
> expected.
>
> Signed-off-by: Mike Manning <mmanning@brocade.com>
> ---
>  include/linux/if_vlan.h |    2 ++
>  net/8021q/vlan.c        |   17 +++++++++++------
>  net/8021q/vlan_dev.c    |   13 ++++++++++---
>  3 files changed, 23 insertions(+), 9 deletions(-)
>
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -138,6 +138,7 @@ struct netpoll;
>   *     @flags: device flags
>   *     @real_dev: underlying netdevice
>   *     @real_dev_addr: address of underlying netdevice
> + *     @addr_assign_type: address assignment type
>   *     @dent: proc dir entry
>   *     @vlan_pcpu_stats: ptr to percpu rx stats
>   */
> @@ -153,6 +154,7 @@ struct vlan_dev_priv {
>
>         struct net_device                       *real_dev;
>         unsigned char                           real_dev_addr[ETH_ALEN];
> +       unsigned char                           addr_assign_type;
>
>         struct proc_dir_entry                   *dent;
>         struct vlan_pcpu_stats __percpu         *vlan_pcpu_stats;

Please don't start adding new members to structures when it already
exists in the net_device.  If anything you should be able to drop
read_dev_addr if you do this correctly because you shouldn't need to
clone the lower dev address to watch for changes.  All you will need
to do is watch NET_ADDR_STOLEN.

> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -291,6 +291,15 @@ static void vlan_sync_address(struct net
>         if (ether_addr_equal(vlan->real_dev_addr, dev->dev_addr))
>                 return;
>
> +       /* vlan continues to inherit address of parent interface */
> +       if (vlan->addr_assign_type == NET_ADDR_STOLEN) {
> +               ether_addr_copy(vlandev->dev_addr, dev->dev_addr);
> +               goto out;
> +       }
> +
> +       if (!(vlandev->flags & IFF_UP))
> +               goto out;
> +
>         /* vlan address was different from the old address and is equal to
>          * the new address */
>         if (!ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
> @@ -303,6 +312,7 @@ static void vlan_sync_address(struct net
>             !ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
>                 dev_uc_add(dev, vlandev->dev_addr);
>
> +out:
>         ether_addr_copy(vlan->real_dev_addr, dev->dev_addr);
>  }
>
> @@ -389,13 +399,8 @@ static int vlan_device_event(struct noti
>
>         case NETDEV_CHANGEADDR:
>                 /* Adjust unicast filters on underlying device */
> -               vlan_group_for_each_dev(grp, i, vlandev) {
> -                       flgs = vlandev->flags;
> -                       if (!(flgs & IFF_UP))
> -                               continue;
> -
> +               vlan_group_for_each_dev(grp, i, vlandev)
>                         vlan_sync_address(dev, vlandev);
> -               }
>                 break;
>
>         case NETDEV_CHANGEMTU:

So all of this is far more complicated than it needs to be.  If
NET_ADDR_STOLEN is set you have to follow the lower device MAC
address, otherwise you maintain your own address and have to hold a
reference to it on the lower device.

You should also be able to maintain the current logic of not updating
a down interface on an address change.  You don't need to update a
stolen MAC address until the open routine is called for the interface.

> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -315,17 +315,21 @@ static int vlan_dev_stop(struct net_devi
>
>  static int vlan_dev_set_mac_address(struct net_device *dev, void *p)
>  {
> -       struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
> +       struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> +       struct net_device *real_dev = vlan->real_dev;
>         struct sockaddr *addr = p;
> +       bool is_real_addr;
>         int err;
>
>         if (!is_valid_ether_addr(addr->sa_data))
>                 return -EADDRNOTAVAIL;
>
> +       is_real_addr = ether_addr_equal(addr->sa_data, real_dev->dev_addr);
> +
>         if (!(dev->flags & IFF_UP))
>                 goto out;
>
> -       if (!ether_addr_equal(addr->sa_data, real_dev->dev_addr)) {
> +       if (!is_real_addr) {
>                 err = dev_uc_add(real_dev, addr->sa_data);
>                 if (err < 0)
>                         return err;
> @@ -336,6 +340,7 @@ static int vlan_dev_set_mac_address(stru
>
>  out:
>         ether_addr_copy(dev->dev_addr, addr->sa_data);
> +       vlan->addr_assign_type = is_real_addr ? NET_ADDR_STOLEN : NET_ADDR_SET;
>         return 0;
>  }

Yeah so you probably don't need most of this.  Just take a look at
dev_set_mac_address.  It will already update this to NET_ADDR_SET
which is really what you want if the user specified a MAC address.  At
that point you should stop following the lower dev because the user
may intend to have this MAC address be static while they change the
lower dev address.

> @@ -558,8 +563,10 @@ static int vlan_dev_init(struct net_devi
>         /* ipv6 shared card related stuff */
>         dev->dev_id = real_dev->dev_id;
>
> -       if (is_zero_ether_addr(dev->dev_addr))
> +       if (is_zero_ether_addr(dev->dev_addr)) {
>                 eth_hw_addr_inherit(dev, real_dev);
> +               vlan_dev_priv(dev)->addr_assign_type = NET_ADDR_STOLEN;
> +       }
>         if (is_zero_ether_addr(dev->broadcast))
>                 memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
>

This should be just dev->addr_assign_type = NET_ADDR_STOLEN.  No need
to put this in a private structure member.

^ permalink raw reply

* Re: [PATCH v9 net-next 1/2] hv_sock: introduce Hyper-V Sockets
From: David Miller @ 2016-05-06 17:03 UTC (permalink / raw)
  To: decui
  Cc: olaf, gregkh, jasowang, linux-kernel, joe, netdev, apw, devel,
	haiyangz
In-Reply-To: <1462381017-568-1-git-send-email-decui@microsoft.com>

From: Dexuan Cui <decui@microsoft.com>
Date: Wed,  4 May 2016 09:56:57 -0700

> +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV (5 * PAGE_SIZE)
> +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_SEND (5 * PAGE_SIZE)
> +
> +#define HVSOCK_RCV_BUF_SZ	VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV
 ...
> +struct hvsock_sock {
 ...
> +	/* The 'hdr' and 'buf' in the below 'send' and 'recv' definitions must
> +	 * be consecutive: see hvsock_send_data() and hvsock_recv_data().
> +	 */
> +	struct {
> +		struct vmpipe_proto_header hdr;
> +		u8 buf[HVSOCK_SND_BUF_SZ];
> +	} send;
> +
> +	struct {
> +		struct vmpipe_proto_header hdr;
> +		u8 buf[HVSOCK_RCV_BUF_SZ];
> +
> +		unsigned int data_len;
> +		unsigned int data_offset;
> +	} recv;

I don't think allocating 5 pages of unswappable memory for every Hyper-V socket
created is reasonable.

^ permalink raw reply

* Re: [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
From: Larry Finger @ 2016-05-06 17:04 UTC (permalink / raw)
  To: Wang YanQing, chaoming_li, kvalo, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20160506163351.GA4589@udknight>

On 05/06/2016 11:33 AM, Wang YanQing wrote:
> We can't use kfree_skb in irq disable context, because spin_lock_irqsave
> make sure we are always in irq disable context, use dev_kfree_skb_irq
> instead of kfree_skb is better than dev_kfree_skb_any.
>
> This patch fix below kernel warning:
> [ 7612.095528] ------------[ cut here ]------------
> [ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150 __local_bh_enable_ip+0x58/0x80()
> [ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal btcoexist rtl_pci rtlwifi rtl8723_common
> [ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G        W       4.4.0+ #4
> [ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW (1.19 ) 08/27/2015
> [ 7612.095574]  00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0 c104cc59 c19d4454
> [ 7612.095584]  00000003 0000116c c19d4784 00000096 c10508a8 c10508a8 00000200 c1b42400
> [ 7612.095594]  f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc c10508a8 f21f08b8
> [ 7612.095604] Call Trace:
> [ 7612.095614]  [<c12ce7c5>] dump_stack+0x41/0x5c
> [ 7612.095620]  [<c104cc59>] warn_slowpath_common+0x89/0xc0
> [ 7612.095628]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
> [ 7612.095634]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
> [ 7612.095640]  [<c104ccad>] warn_slowpath_null+0x1d/0x20
> [ 7612.095646]  [<c10508a8>] __local_bh_enable_ip+0x58/0x80
> [ 7612.095653]  [<c16b7d34>] destroy_conntrack+0x64/0xa0
> [ 7612.095660]  [<c16b300f>] nf_conntrack_destroy+0xf/0x20
> [ 7612.095665]  [<c1677565>] skb_release_head_state+0x55/0xa0
> [ 7612.095670]  [<c16775bb>] skb_release_all+0xb/0x20
> [ 7612.095674]  [<c167760b>] __kfree_skb+0xb/0x60
> [ 7612.095679]  [<c16776f0>] kfree_skb+0x30/0x70
> [ 7612.095686]  [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
> [ 7612.095692]  [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
> [ 7612.095698]  [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
> [ 7612.095705]  [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
> [ 7612.095712]  [<c17e3f16>] drv_start+0x36/0xc0
> [ 7612.095717]  [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
> [ 7612.095725]  [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
> [ 7612.095730]  [<c17f60bd>] ieee80211_open+0x4d/0x50
> [ 7612.095736]  [<c16891b3>] __dev_open+0xa3/0x130
> [ 7612.095742]  [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
> [ 7612.095748]  [<c1689499>] __dev_change_flags+0x89/0x140
> [ 7612.095753]  [<c127c70d>] ? selinux_capable+0xd/0x10
> [ 7612.095759]  [<c1689589>] dev_change_flags+0x29/0x60
> [ 7612.095765]  [<c1700b93>] devinet_ioctl+0x553/0x670
> [ 7612.095772]  [<c12db758>] ? _copy_to_user+0x28/0x40
> [ 7612.095777]  [<c17018b5>] inet_ioctl+0x85/0xb0
> [ 7612.095783]  [<c166e647>] sock_ioctl+0x67/0x260
> [ 7612.095788]  [<c166e5e0>] ? sock_fasync+0x80/0x80
> [ 7612.095795]  [<c115c99b>] do_vfs_ioctl+0x6b/0x550
> [ 7612.095800]  [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
> [ 7612.095807]  [<c10a8914>] ? timekeeping_suspend+0x294/0x320
> [ 7612.095813]  [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
> [ 7612.095820]  [<c1276e24>] ? security_file_ioctl+0x34/0x50
> [ 7612.095827]  [<c115cef0>] SyS_ioctl+0x70/0x80
> [ 7612.095832]  [<c1001804>] do_fast_syscall_32+0x84/0x120
> [ 7612.095839]  [<c183ff91>] sysenter_past_esp+0x36/0x55
> [ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---
>
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> Cc: Stable <stable@vger.kernel.org>
> ---
>   Changes:
>   v1-v2:
>   1: add a Cc to stable.
>
>   drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index 1ac41b8..99a3a03 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
>   							 true,
>   							 HW_DESC_TXBUFF_ADDR),
>   						 skb->len, PCI_DMA_TODEVICE);
> -				kfree_skb(skb);
> +				dev_kfree_skb_irq(skb);
>   				ring->idx = (ring->idx + 1) % ring->entries;
>   			}
>   			ring->idx = 0;
>
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

^ permalink raw reply

* Re: [PATCH] Add support for configuring Infiniband GUIDs
From: Sergei Shtylyov @ 2016-05-06 17:05 UTC (permalink / raw)
  To: Eli Cohen, shemminger; +Cc: netdev
In-Reply-To: <1462549405-16003-1-git-send-email-eli@mellanox.com>

Hello.

On 05/06/2016 06:43 PM, Eli Cohen wrote:

> Add two NLA's that allow configuration of Infiniband node or port GUIDs
> by referencing the IPoIB net device set over then physical function. The
> format to be used is as follows:
>
> ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70
> ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78
>
> Issue: 702759
> Change-Id: I5ffb54d6de7bfa8650bf5818f484279914991d6e
> Signed-off-by: Eli Cohen <eli@mellanox.com>
> ---
>   ip/iplink.c           | 40 ++++++++++++++++++++++++++++++++++++++++
>   man/man8/ip-link.8.in | 12 +++++++++++-
>   2 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/ip/iplink.c b/ip/iplink.c
> index d2e586b6d133..3f885defdfeb 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -237,6 +237,30 @@ struct iplink_req {
>   	char			buf[1024];
>   };
>
> +static int extract_guid(__u64 *guid, char *arg)
> +{
> +	__u64 ret;
> +	int g[8];
> +	int err;
> +
> +	err = sscanf(arg, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x",
> +		     g, g + 1, g + 2, g + 3, g + 4, g + 5, g + 6, g + 7);
> +	if (err != 8)

    Strange name for a variable, if sscanf() returns # of fields read... In 
fact, you don't even need this variable.

> +		return -1;
> +
> +	ret = ((__u64)(g[0]) << 56) |
> +	      ((__u64)(g[1]) << 48) |
> +	      ((__u64)(g[2]) << 40) |
> +	      ((__u64)(g[3]) << 32) |
> +	      ((__u64)(g[4]) << 24) |
> +	      ((__u64)(g[5]) << 16) |
> +	      ((__u64)(g[6]) << 8) |
> +	      ((__u64)(g[7]));
> +	*guid = ret;
> +
> +	return 0;
> +}
> +
>   static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
>   			   struct iplink_req *req, int dev_index)
>   {
[...]

MBR, Sergei

^ permalink raw reply

* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: John Stultz @ 2016-05-06 17:06 UTC (permalink / raw)
  To: Dean Jenkins
  Cc: Guodong Xu, lkml, Mark Craske, David S. Miller, YongQin Liu,
	linux-usb, netdev, Ivan Vecera, David B. Robins
In-Reply-To: <572B0023.9060505@mentor.com>

On Thu, May 5, 2016 at 1:11 AM, Dean Jenkins <Dean_Jenkins@mentor.com> wrote:
> On 05/05/16 00:45, John Stultz wrote:
>>
>> Just as a sample point, I have managed to reproduce exactly this issue
>> on an x86_64 system by simply scp'ing a large file.
>
> Please tell us the x86_64 kernel version number that you used and which
> Linux Distribution it was ? This allows other people a chance to reproduce
> your observations.

Sorry for being a little slow here, had some other issues I had to chase.

On my x86_64 system, its Ubuntu 14.04.4, with a 4.6.0-rc2 kernel.


>> [  417.819276] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
>> synchronisation was lost, remaining 988
>
> It is interesting that the reported "remaining" value is 988. Is 988 always
> shown ? I mean that do you see any other "remaining" values for the "Data
> Header synchronisation was lost" error message ?

Yep. Its always the same 988 remaining, on either architecture.


>> [  417.823415] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0xef830347, offset 4
>
> The gap in the timestamps shows 417.823415 - 417.819276 = 0.004139 = 4ms
> which is a large gap in terms of USB 2.0 high speed communications. This gap
> is expected to be in the 100us range for consecutive URBs. So 4ms is
> strange.
>
> The expectation is that the "Data Header synchronisation was lost" error
> message resets the 32-bit header word synchronisation to the start of the
> next URB buffer. The "Bad Header Length, offset 4" is the expected outcome
> for the next URB because it is unlikely the 32-bit header word is at the
> start of URB buffer due to Ethernet frames spanning across URBs.
>>
>> [  417.827502] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
>> 0x31e2b348, offset 4
>
> Timestamps show the gap to be 4ms which is strange for USB 2.0 high speed,
> are you sure high speed mode is being used ?
>>

Yep, on my x86_64 system, it seems to be.

[    3.101115] usb 1-5: new high-speed USB device number 2 using ehci-pci
[    3.232309] usb 1-5: New USB device found, idVendor=0b95, idProduct=772b
[    3.232327] usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[    3.232339] usb 1-5: Product: AX88772B
[    3.232350] usb 1-5: Manufacturer: ASIX Elec. Corp.
[    3.232360] usb 1-5: SerialNumber: 188298
[    4.032206] asix 1-5:1.0 eth1: register 'asix' at
usb-0000:00:04.1-5, ASIX AX88772B USB 2.0 Ethernet, 00:50:b6:18:82:98


> Please can you supply the output of ifconfig for the USB to Ethernet
> adaptor, your example above shows eth1 as the device.
>
> Please show the output of ifconfig eth1 before and after the issue is seen.
> This will show us whether the kernel logs any network errors and how many
> bytes have been transferred.

Before:
$ ifconfig eth1
eth1      Link encap:Ethernet  HWaddr 00:50:b6:18:82:98
          inet addr:192.168.0.12  Bcast:192.168.0.255  Mask:255.255.255.0
          inet6 addr: 2601:1c2:1002:83f0:250:b6ff:fe18:8298/64 Scope:Global
          inet6 addr: fe80::250:b6ff:fe18:8298/64 Scope:Link
          inet6 addr: 2601:1c2:1002:83f0:b0f0:71a0:6c7e:346b/64 Scope:Global
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:372 errors:0 dropped:0 overruns:0 frame:0
          TX packets:385 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:38523 (38.5 KB)  TX bytes:48801 (48.8 KB)


After:
$ ifconfig eth1
eth1      Link encap:Ethernet  HWaddr 00:50:b6:18:82:98
          inet addr:192.168.0.12  Bcast:192.168.0.255  Mask:255.255.255.0
          inet6 addr: 2601:1c2:1002:83f0:250:b6ff:fe18:8298/64 Scope:Global
          inet6 addr: fe80::250:b6ff:fe18:8298/64 Scope:Link
          inet6 addr: 2601:1c2:1002:83f0:b0f0:71a0:6c7e:346b/64 Scope:Global
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:151005 errors:169 dropped:0 overruns:0 frame:0
          TX packets:61351 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000
          RX bytes:225874384 (225.8 MB)  TX bytes:4431098 (4.4 MB)




> After the issue is seen, please can you show us the output of "dmesg | grep
> asix" so that we can see status messages from the ASIX driver that the USB
> to Ethernet adaptor is using. In particular we need to check that USB high
> speed operation (480Mbps) is being used and not full speed operation
> (12Mbps).


[    2.766525] usbcore: registered new interface driver asix
[    4.031443] asix 1-5:1.0 eth1: register 'asix' at
usb-0000:00:04.1-5, ASIX AX88772B USB 2.0 Ethernet, 00:50:b6:18:82:98
[   31.578983] asix 1-5:1.0 eth1: link down
[   33.244743] asix 1-5:1.0 eth1: link up, 100Mbps, full-duplex, lpa 0xCDE1
[  171.959244] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  171.959530] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x1651c2bf, offset 4
[  171.959768] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xfcf61092, offset 4
[  171.960001] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x3269b285, offset 4
[  171.960348] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xdf3bd279, offset 4
[  171.976453] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  171.976663] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x8ad939e0, offset 4
[  171.976835] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xeb99048f, offset 4
[  171.977002] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x5d1ac177, offset 4
[  172.312002] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  172.312319] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x3722a6f0, offset 4
[  172.312503] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x45371a3e, offset 4
[  172.312680] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xc396c451, offset 4
[  173.066398] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  173.066686] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xa445167f, offset 4
[  173.066921] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xa16d6b5b, offset 4
[  173.293636] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  173.293904] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xb1510dad, offset 4
[  173.294167] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x22709710, offset 4
[  173.304459] asix 1-5:1.0 eth1: asix_rx_fixup() Data Header
synchronisation was lost, remaining 988
[  173.304679] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x1472a821, offset 4
[  173.304861] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0x9e2b0c30, offset 4
[  173.305085] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xcfe77e12, offset 4
[  173.305308] asix 1-5:1.0 eth1: asix_rx_fixup() Bad Header Length
0xfc5b890d, offset 4
...


thanks
-john

^ permalink raw reply

* Re: [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
From: Alexander Duyck @ 2016-05-06 17:13 UTC (permalink / raw)
  To: Wang YanQing, Larry.Finger, chaoming_li, kvalo, linux-wireless,
	Netdev, linux-kernel@vger.kernel.org
In-Reply-To: <20160506163351.GA4589@udknight>

On Fri, May 6, 2016 at 9:33 AM, Wang YanQing <udknight@gmail.com> wrote:
> We can't use kfree_skb in irq disable context, because spin_lock_irqsave
> make sure we are always in irq disable context, use dev_kfree_skb_irq
> instead of kfree_skb is better than dev_kfree_skb_any.
>
> This patch fix below kernel warning:
> [ 7612.095528] ------------[ cut here ]------------
> [ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150 __local_bh_enable_ip+0x58/0x80()
> [ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal btcoexist rtl_pci rtlwifi rtl8723_common
> [ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G        W       4.4.0+ #4
> [ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW (1.19 ) 08/27/2015
> [ 7612.095574]  00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0 c104cc59 c19d4454
> [ 7612.095584]  00000003 0000116c c19d4784 00000096 c10508a8 c10508a8 00000200 c1b42400
> [ 7612.095594]  f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc c10508a8 f21f08b8
> [ 7612.095604] Call Trace:
> [ 7612.095614]  [<c12ce7c5>] dump_stack+0x41/0x5c
> [ 7612.095620]  [<c104cc59>] warn_slowpath_common+0x89/0xc0
> [ 7612.095628]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
> [ 7612.095634]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
> [ 7612.095640]  [<c104ccad>] warn_slowpath_null+0x1d/0x20
> [ 7612.095646]  [<c10508a8>] __local_bh_enable_ip+0x58/0x80
> [ 7612.095653]  [<c16b7d34>] destroy_conntrack+0x64/0xa0
> [ 7612.095660]  [<c16b300f>] nf_conntrack_destroy+0xf/0x20
> [ 7612.095665]  [<c1677565>] skb_release_head_state+0x55/0xa0
> [ 7612.095670]  [<c16775bb>] skb_release_all+0xb/0x20
> [ 7612.095674]  [<c167760b>] __kfree_skb+0xb/0x60
> [ 7612.095679]  [<c16776f0>] kfree_skb+0x30/0x70
> [ 7612.095686]  [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
> [ 7612.095692]  [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
> [ 7612.095698]  [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
> [ 7612.095705]  [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
> [ 7612.095712]  [<c17e3f16>] drv_start+0x36/0xc0
> [ 7612.095717]  [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
> [ 7612.095725]  [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
> [ 7612.095730]  [<c17f60bd>] ieee80211_open+0x4d/0x50
> [ 7612.095736]  [<c16891b3>] __dev_open+0xa3/0x130
> [ 7612.095742]  [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
> [ 7612.095748]  [<c1689499>] __dev_change_flags+0x89/0x140
> [ 7612.095753]  [<c127c70d>] ? selinux_capable+0xd/0x10
> [ 7612.095759]  [<c1689589>] dev_change_flags+0x29/0x60
> [ 7612.095765]  [<c1700b93>] devinet_ioctl+0x553/0x670
> [ 7612.095772]  [<c12db758>] ? _copy_to_user+0x28/0x40
> [ 7612.095777]  [<c17018b5>] inet_ioctl+0x85/0xb0
> [ 7612.095783]  [<c166e647>] sock_ioctl+0x67/0x260
> [ 7612.095788]  [<c166e5e0>] ? sock_fasync+0x80/0x80
> [ 7612.095795]  [<c115c99b>] do_vfs_ioctl+0x6b/0x550
> [ 7612.095800]  [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
> [ 7612.095807]  [<c10a8914>] ? timekeeping_suspend+0x294/0x320
> [ 7612.095813]  [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
> [ 7612.095820]  [<c1276e24>] ? security_file_ioctl+0x34/0x50
> [ 7612.095827]  [<c115cef0>] SyS_ioctl+0x70/0x80
> [ 7612.095832]  [<c1001804>] do_fast_syscall_32+0x84/0x120
> [ 7612.095839]  [<c183ff91>] sysenter_past_esp+0x36/0x55
> [ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---
>
> Signed-off-by: Wang YanQing <udknight@gmail.com>
> Cc: Stable <stable@vger.kernel.org>
> ---
>  Changes:
>  v1-v2:
>  1: add a Cc to stable.
>
>  drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
> index 1ac41b8..99a3a03 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
> @@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
>                                                          true,
>                                                          HW_DESC_TXBUFF_ADDR),
>                                                  skb->len, PCI_DMA_TODEVICE);
> -                               kfree_skb(skb);
> +                               dev_kfree_skb_irq(skb);
>                                 ring->idx = (ring->idx + 1) % ring->entries;
>                         }
>                         ring->idx = 0;

Is this always called in IRQ context?  You might be better off using
dev_kfree_skb_any instead if this is something that can be called from
net_device_ops since that way you avoid having to call into the Tx
softirq cleanup routine to free the buffers later unless you really
need it.

- Alex

^ permalink raw reply

* [PATCH v2] Documentation/networking: more accurate LCO explanation
From: Shmulik Ladkani @ 2016-05-06 17:27 UTC (permalink / raw)
  To: David S . Miller, netdev, Alexander Duyck; +Cc: Edward Cree, shmulik.ladkani

In few places the term "ones-complement sum" was used but the actual
meaning is "the complement of the ones-complement sum".

Also, avoid enclosing long statements with underscore, to ease
readability.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Acked-by: Edward Cree <ecree@solarflare.com>

---
 v2:
  - Fixed one more occurence where "complement of" was missing
  - Got rid of unreadable underscore wrapped statements

 Took the liberty having the underscore removal as part of this patch.
 Let me know if you feel this needs a patch split.

 Documentation/networking/checksum-offloads.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
index de2a327766..56e3686124 100644
--- a/Documentation/networking/checksum-offloads.txt
+++ b/Documentation/networking/checksum-offloads.txt
@@ -69,18 +69,18 @@ LCO: Local Checksum Offload
 LCO is a technique for efficiently computing the outer checksum of an
  encapsulated datagram when the inner checksum is due to be offloaded.
 The ones-complement sum of a correctly checksummed TCP or UDP packet is
- equal to the sum of the pseudo header, because everything else gets
- 'cancelled out' by the checksum field.  This is because the sum was
+ equal to the complement of the sum of the pseudo header, because everything
+ else gets 'cancelled out' by the checksum field.  This is because the sum was
  complemented before being written to the checksum field.
 More generally, this holds in any case where the 'IP-style' ones complement
  checksum is used, and thus any checksum that TX Checksum Offload supports.
 That is, if we have set up TX Checksum Offload with a start/offset pair, we
- know that _after the device has filled in that checksum_, the ones
+ know that after the device has filled in that checksum, the ones
  complement sum from csum_start to the end of the packet will be equal to
- _whatever value we put in the checksum field beforehand_.  This allows us
- to compute the outer checksum without looking at the payload: we simply
- stop summing when we get to csum_start, then add the 16-bit word at
- (csum_start + csum_offset).
+ the complement of whatever value we put in the checksum field beforehand.
+ This allows us to compute the outer checksum without looking at the payload:
+ we simply stop summing when we get to csum_start, then add the complement of
+ the 16-bit word at (csum_start + csum_offset).
 Then, when the true inner checksum is filled in (either by hardware or by
  skb_checksum_help()), the outer checksum will become correct by virtue of
  the arithmetic.
-- 
2.7.4

^ permalink raw reply related

* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: John Stultz @ 2016-05-06 17:40 UTC (permalink / raw)
  To: Dean Jenkins
  Cc: Guodong Xu, lkml, Mark Craske, David S. Miller, YongQin Liu,
	linux-usb, netdev, Ivan Vecera, David B. Robins
In-Reply-To: <572CB188.7050203@mentor.com>

On Fri, May 6, 2016 at 8:00 AM, Dean Jenkins <Dean_Jenkins@mentor.com> wrote:
> My conclusion is that your USB to Ethernet Adaptor is not running at high
> speed (480Mbps) mode which is causing a partial loss (corruption) of
> Ethernet frames across the USB link. A USB Protocol Analyser or software
> tool usbmon could be used to confirm this scenario.
>
> Therefore please retest with a working high-speed USB hub or remove the
> full-speed USB hub from the test environment and directly connect the USB to
> Ethernet Adaptor to the root hub of the USB port. Then repeat the tests to
> see whether anything improved.
>
> In other words, you need to eliminate the dmesg messages saying "not running
> at top speed; connect to a high speed hub".

The aarch64 system has a quirk that at the moment limits it to the
slower full-speed mode, which also exacerbates the issue (basically
taking a fairly slow 1.1.Mb/s network connection without your patch,
to an almost unusable 30Kb/s with it).

But that isn't the case on the x86_64 system, which is seeing a very
similar problem (though the performance effect isn't nearly as bad, as
the error rate in time seems relatively similar on both, and I think
my scp transmissions are cpu bound on this atom board :).

thanks
-john

^ permalink raw reply

* [PATCH v1 1/1] ISDN: eicon: replace custom hex_asc_lo() / hex_pack_byte()
From: Andy Shevchenko @ 2016-05-06 17:55 UTC (permalink / raw)
  To: Armin Schindler, netdev; +Cc: Andy Shevchenko

Instead of custom approach re-use generic helpers to convert byte to hex
format.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/isdn/hardware/eicon/message.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/isdn/hardware/eicon/message.c b/drivers/isdn/hardware/eicon/message.c
index d7c2866..1a1d997 100644
--- a/drivers/isdn/hardware/eicon/message.c
+++ b/drivers/isdn/hardware/eicon/message.c
@@ -1147,8 +1147,6 @@ static byte test_c_ind_mask_bit(PLCI *plci, word b)
 
 static void dump_c_ind_mask(PLCI *plci)
 {
-	static char hex_digit_table[0x10] =
-		{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
 	word i, j, k;
 	dword d;
 	char *p;
@@ -1165,7 +1163,7 @@ static void dump_c_ind_mask(PLCI *plci)
 				d = plci->c_ind_mask_table[i + j];
 				for (k = 0; k < 8; k++)
 				{
-					*(--p) = hex_digit_table[d & 0xf];
+					*(--p) = hex_asc_lo(d);
 					d >>= 4;
 				}
 			}
@@ -10507,7 +10505,6 @@ static void mixer_set_bchannel_id(PLCI *plci, byte *chi)
 
 static void mixer_calculate_coefs(DIVA_CAPI_ADAPTER *a)
 {
-	static char hex_digit_table[0x10] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
 	word n, i, j;
 	char *p;
 	char hex_line[2 * MIXER_MAX_DUMP_CHANNELS + MIXER_MAX_DUMP_CHANNELS / 8 + 4];
@@ -10690,13 +10687,13 @@ static void mixer_calculate_coefs(DIVA_CAPI_ADAPTER *a)
 	n = li_total_channels;
 	if (n > MIXER_MAX_DUMP_CHANNELS)
 		n = MIXER_MAX_DUMP_CHANNELS;
+
 	p = hex_line;
 	for (j = 0; j < n; j++)
 	{
 		if ((j & 0x7) == 0)
 			*(p++) = ' ';
-		*(p++) = hex_digit_table[li_config_table[j].curchnl >> 4];
-		*(p++) = hex_digit_table[li_config_table[j].curchnl & 0xf];
+		p = hex_byte_pack(p, li_config_table[j].curchnl);
 	}
 	*p = '\0';
 	dbug(1, dprintf("[%06lx] CURRENT %s",
@@ -10706,8 +10703,7 @@ static void mixer_calculate_coefs(DIVA_CAPI_ADAPTER *a)
 	{
 		if ((j & 0x7) == 0)
 			*(p++) = ' ';
-		*(p++) = hex_digit_table[li_config_table[j].channel >> 4];
-		*(p++) = hex_digit_table[li_config_table[j].channel & 0xf];
+		p = hex_byte_pack(p, li_config_table[j].channel);
 	}
 	*p = '\0';
 	dbug(1, dprintf("[%06lx] CHANNEL %s",
@@ -10717,8 +10713,7 @@ static void mixer_calculate_coefs(DIVA_CAPI_ADAPTER *a)
 	{
 		if ((j & 0x7) == 0)
 			*(p++) = ' ';
-		*(p++) = hex_digit_table[li_config_table[j].chflags >> 4];
-		*(p++) = hex_digit_table[li_config_table[j].chflags & 0xf];
+		p = hex_byte_pack(p, li_config_table[j].chflags);
 	}
 	*p = '\0';
 	dbug(1, dprintf("[%06lx] CHFLAG  %s",
@@ -10730,8 +10725,7 @@ static void mixer_calculate_coefs(DIVA_CAPI_ADAPTER *a)
 		{
 			if ((j & 0x7) == 0)
 				*(p++) = ' ';
-			*(p++) = hex_digit_table[li_config_table[i].flag_table[j] >> 4];
-			*(p++) = hex_digit_table[li_config_table[i].flag_table[j] & 0xf];
+			p = hex_byte_pack(p, li_config_table[i].flag_table[j]);
 		}
 		*p = '\0';
 		dbug(1, dprintf("[%06lx] FLAG[%02x]%s",
@@ -10744,8 +10738,7 @@ static void mixer_calculate_coefs(DIVA_CAPI_ADAPTER *a)
 		{
 			if ((j & 0x7) == 0)
 				*(p++) = ' ';
-			*(p++) = hex_digit_table[li_config_table[i].coef_table[j] >> 4];
-			*(p++) = hex_digit_table[li_config_table[i].coef_table[j] & 0xf];
+			p = hex_byte_pack(p, li_config_table[i].coef_table[j]);
 		}
 		*p = '\0';
 		dbug(1, dprintf("[%06lx] COEF[%02x]%s",
-- 
2.8.1

^ permalink raw reply related

* Re: [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
From: Larry Finger @ 2016-05-06 18:01 UTC (permalink / raw)
  To: Alexander Duyck, Wang YanQing, chaoming_li, kvalo, linux-wireless,
	Netdev, linux-kernel@vger.kernel.org
In-Reply-To: <CAKgT0Uff5dc3_iEjK6GScqgP5_1kcg3YQAv7AH3bzuVDguXiPw@mail.gmail.com>

On 05/06/2016 12:13 PM, Alexander Duyck wrote:
> On Fri, May 6, 2016 at 9:33 AM, Wang YanQing <udknight@gmail.com> wrote:
>> We can't use kfree_skb in irq disable context, because spin_lock_irqsave
>> make sure we are always in irq disable context, use dev_kfree_skb_irq
>> instead of kfree_skb is better than dev_kfree_skb_any.
>>
>> This patch fix below kernel warning:
>> [ 7612.095528] ------------[ cut here ]------------
>> [ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150 __local_bh_enable_ip+0x58/0x80()
>> [ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal btcoexist rtl_pci rtlwifi rtl8723_common
>> [ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G        W       4.4.0+ #4
>> [ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW (1.19 ) 08/27/2015
>> [ 7612.095574]  00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0 c104cc59 c19d4454
>> [ 7612.095584]  00000003 0000116c c19d4784 00000096 c10508a8 c10508a8 00000200 c1b42400
>> [ 7612.095594]  f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc c10508a8 f21f08b8
>> [ 7612.095604] Call Trace:
>> [ 7612.095614]  [<c12ce7c5>] dump_stack+0x41/0x5c
>> [ 7612.095620]  [<c104cc59>] warn_slowpath_common+0x89/0xc0
>> [ 7612.095628]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
>> [ 7612.095634]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
>> [ 7612.095640]  [<c104ccad>] warn_slowpath_null+0x1d/0x20
>> [ 7612.095646]  [<c10508a8>] __local_bh_enable_ip+0x58/0x80
>> [ 7612.095653]  [<c16b7d34>] destroy_conntrack+0x64/0xa0
>> [ 7612.095660]  [<c16b300f>] nf_conntrack_destroy+0xf/0x20
>> [ 7612.095665]  [<c1677565>] skb_release_head_state+0x55/0xa0
>> [ 7612.095670]  [<c16775bb>] skb_release_all+0xb/0x20
>> [ 7612.095674]  [<c167760b>] __kfree_skb+0xb/0x60
>> [ 7612.095679]  [<c16776f0>] kfree_skb+0x30/0x70
>> [ 7612.095686]  [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
>> [ 7612.095692]  [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
>> [ 7612.095698]  [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
>> [ 7612.095705]  [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
>> [ 7612.095712]  [<c17e3f16>] drv_start+0x36/0xc0
>> [ 7612.095717]  [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
>> [ 7612.095725]  [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
>> [ 7612.095730]  [<c17f60bd>] ieee80211_open+0x4d/0x50
>> [ 7612.095736]  [<c16891b3>] __dev_open+0xa3/0x130
>> [ 7612.095742]  [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
>> [ 7612.095748]  [<c1689499>] __dev_change_flags+0x89/0x140
>> [ 7612.095753]  [<c127c70d>] ? selinux_capable+0xd/0x10
>> [ 7612.095759]  [<c1689589>] dev_change_flags+0x29/0x60
>> [ 7612.095765]  [<c1700b93>] devinet_ioctl+0x553/0x670
>> [ 7612.095772]  [<c12db758>] ? _copy_to_user+0x28/0x40
>> [ 7612.095777]  [<c17018b5>] inet_ioctl+0x85/0xb0
>> [ 7612.095783]  [<c166e647>] sock_ioctl+0x67/0x260
>> [ 7612.095788]  [<c166e5e0>] ? sock_fasync+0x80/0x80
>> [ 7612.095795]  [<c115c99b>] do_vfs_ioctl+0x6b/0x550
>> [ 7612.095800]  [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
>> [ 7612.095807]  [<c10a8914>] ? timekeeping_suspend+0x294/0x320
>> [ 7612.095813]  [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
>> [ 7612.095820]  [<c1276e24>] ? security_file_ioctl+0x34/0x50
>> [ 7612.095827]  [<c115cef0>] SyS_ioctl+0x70/0x80
>> [ 7612.095832]  [<c1001804>] do_fast_syscall_32+0x84/0x120
>> [ 7612.095839]  [<c183ff91>] sysenter_past_esp+0x36/0x55
>> [ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---
>>
>> Signed-off-by: Wang YanQing <udknight@gmail.com>
>> Cc: Stable <stable@vger.kernel.org>
>> ---
>>   Changes:
>>   v1-v2:
>>   1: add a Cc to stable.
>>
>>   drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
>> index 1ac41b8..99a3a03 100644
>> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
>> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
>> @@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
>>                                                           true,
>>                                                           HW_DESC_TXBUFF_ADDR),
>>                                                   skb->len, PCI_DMA_TODEVICE);
>> -                               kfree_skb(skb);
>> +                               dev_kfree_skb_irq(skb);
>>                                  ring->idx = (ring->idx + 1) % ring->entries;
>>                          }
>>                          ring->idx = 0;
>
> Is this always called in IRQ context?  You might be better off using
> dev_kfree_skb_any instead if this is something that can be called from
> net_device_ops since that way you avoid having to call into the Tx
> softirq cleanup routine to free the buffers later unless you really
> need it.
>
> - Alex
>

Alex,

Six lines below the change is a spin_unlock_irqrestore(), which is always 
called. I believe that the patch is correct.

Larry

^ permalink raw reply

* Re: [PATCH v2] rtlwifi: pci: use dev_kfree_skb_irq instead of kfree_skb in rtl_pci_reset_trx_ring
From: Alexander Duyck @ 2016-05-06 18:05 UTC (permalink / raw)
  To: Larry Finger
  Cc: Wang YanQing, chaoming_li-kXabqFNEczNtrwSWzY7KCg,
	kvalo-sgV2jX0FEOL9JmXXK+q4OQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Netdev,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <572CDBF0.9000607-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

On Fri, May 6, 2016 at 11:01 AM, Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> wrote:
> On 05/06/2016 12:13 PM, Alexander Duyck wrote:
>>
>> On Fri, May 6, 2016 at 9:33 AM, Wang YanQing <udknight-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> We can't use kfree_skb in irq disable context, because spin_lock_irqsave
>>> make sure we are always in irq disable context, use dev_kfree_skb_irq
>>> instead of kfree_skb is better than dev_kfree_skb_any.
>>>
>>> This patch fix below kernel warning:
>>> [ 7612.095528] ------------[ cut here ]------------
>>> [ 7612.095546] WARNING: CPU: 3 PID: 4460 at kernel/softirq.c:150
>>> __local_bh_enable_ip+0x58/0x80()
>>> [ 7612.095550] Modules linked in: rtl8723be x86_pkg_temp_thermal
>>> btcoexist rtl_pci rtlwifi rtl8723_common
>>> [ 7612.095567] CPU: 3 PID: 4460 Comm: ifconfig Tainted: G        W
>>> 4.4.0+ #4
>>> [ 7612.095570] Hardware name: LENOVO 20DFA04FCD/20DFA04FCD, BIOS J5ET48WW
>>> (1.19 ) 08/27/2015
>>> [ 7612.095574]  00000000 00000000 da37fc70 c12ce7c5 00000000 da37fca0
>>> c104cc59 c19d4454
>>> [ 7612.095584]  00000003 0000116c c19d4784 00000096 c10508a8 c10508a8
>>> 00000200 c1b42400
>>> [ 7612.095594]  f29be780 da37fcb0 c104ccad 00000009 00000000 da37fcbc
>>> c10508a8 f21f08b8
>>> [ 7612.095604] Call Trace:
>>> [ 7612.095614]  [<c12ce7c5>] dump_stack+0x41/0x5c
>>> [ 7612.095620]  [<c104cc59>] warn_slowpath_common+0x89/0xc0
>>> [ 7612.095628]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
>>> [ 7612.095634]  [<c10508a8>] ? __local_bh_enable_ip+0x58/0x80
>>> [ 7612.095640]  [<c104ccad>] warn_slowpath_null+0x1d/0x20
>>> [ 7612.095646]  [<c10508a8>] __local_bh_enable_ip+0x58/0x80
>>> [ 7612.095653]  [<c16b7d34>] destroy_conntrack+0x64/0xa0
>>> [ 7612.095660]  [<c16b300f>] nf_conntrack_destroy+0xf/0x20
>>> [ 7612.095665]  [<c1677565>] skb_release_head_state+0x55/0xa0
>>> [ 7612.095670]  [<c16775bb>] skb_release_all+0xb/0x20
>>> [ 7612.095674]  [<c167760b>] __kfree_skb+0xb/0x60
>>> [ 7612.095679]  [<c16776f0>] kfree_skb+0x30/0x70
>>> [ 7612.095686]  [<f81b869d>] ? rtl_pci_reset_trx_ring+0x22d/0x370
>>> [rtl_pci]
>>> [ 7612.095692]  [<f81b869d>] rtl_pci_reset_trx_ring+0x22d/0x370 [rtl_pci]
>>> [ 7612.095698]  [<f81b87f9>] rtl_pci_start+0x19/0x190 [rtl_pci]
>>> [ 7612.095705]  [<f81970e6>] rtl_op_start+0x56/0x90 [rtlwifi]
>>> [ 7612.095712]  [<c17e3f16>] drv_start+0x36/0xc0
>>> [ 7612.095717]  [<c17f5ab3>] ieee80211_do_open+0x2d3/0x890
>>> [ 7612.095725]  [<c16820fe>] ? call_netdevice_notifiers_info+0x2e/0x60
>>> [ 7612.095730]  [<c17f60bd>] ieee80211_open+0x4d/0x50
>>> [ 7612.095736]  [<c16891b3>] __dev_open+0xa3/0x130
>>> [ 7612.095742]  [<c183fa53>] ? _raw_spin_unlock_bh+0x13/0x20
>>> [ 7612.095748]  [<c1689499>] __dev_change_flags+0x89/0x140
>>> [ 7612.095753]  [<c127c70d>] ? selinux_capable+0xd/0x10
>>> [ 7612.095759]  [<c1689589>] dev_change_flags+0x29/0x60
>>> [ 7612.095765]  [<c1700b93>] devinet_ioctl+0x553/0x670
>>> [ 7612.095772]  [<c12db758>] ? _copy_to_user+0x28/0x40
>>> [ 7612.095777]  [<c17018b5>] inet_ioctl+0x85/0xb0
>>> [ 7612.095783]  [<c166e647>] sock_ioctl+0x67/0x260
>>> [ 7612.095788]  [<c166e5e0>] ? sock_fasync+0x80/0x80
>>> [ 7612.095795]  [<c115c99b>] do_vfs_ioctl+0x6b/0x550
>>> [ 7612.095800]  [<c127c812>] ? selinux_file_ioctl+0x102/0x1e0
>>> [ 7612.095807]  [<c10a8914>] ? timekeeping_suspend+0x294/0x320
>>> [ 7612.095813]  [<c10a256a>] ? __hrtimer_run_queues+0x14a/0x210
>>> [ 7612.095820]  [<c1276e24>] ? security_file_ioctl+0x34/0x50
>>> [ 7612.095827]  [<c115cef0>] SyS_ioctl+0x70/0x80
>>> [ 7612.095832]  [<c1001804>] do_fast_syscall_32+0x84/0x120
>>> [ 7612.095839]  [<c183ff91>] sysenter_past_esp+0x36/0x55
>>> [ 7612.095844] ---[ end trace 97e9c637a20e8348 ]---
>>>
>>> Signed-off-by: Wang YanQing <udknight-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>>> ---
>>>   Changes:
>>>   v1-v2:
>>>   1: add a Cc to stable.
>>>
>>>   drivers/net/wireless/realtek/rtlwifi/pci.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c
>>> b/drivers/net/wireless/realtek/rtlwifi/pci.c
>>> index 1ac41b8..99a3a03 100644
>>> --- a/drivers/net/wireless/realtek/rtlwifi/pci.c
>>> +++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
>>> @@ -1572,7 +1572,7 @@ int rtl_pci_reset_trx_ring(struct ieee80211_hw *hw)
>>>                                                           true,
>>>
>>> HW_DESC_TXBUFF_ADDR),
>>>                                                   skb->len,
>>> PCI_DMA_TODEVICE);
>>> -                               kfree_skb(skb);
>>> +                               dev_kfree_skb_irq(skb);
>>>                                  ring->idx = (ring->idx + 1) %
>>> ring->entries;
>>>                          }
>>>                          ring->idx = 0;
>>
>>
>> Is this always called in IRQ context?  You might be better off using
>> dev_kfree_skb_any instead if this is something that can be called from
>> net_device_ops since that way you avoid having to call into the Tx
>> softirq cleanup routine to free the buffers later unless you really
>> need it.
>>
>> - Alex
>>
>
> Alex,
>
> Six lines below the change is a spin_unlock_irqrestore(), which is always
> called. I believe that the patch is correct.

Okay.  That works then.

Thanks.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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


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