* Re: [PATCH net-next] ipv4: tcp: ip_send_unicast_reply() is not BH safe
From: David Miller @ 2016-05-06 20:03 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, andreslc, edumazet
In-Reply-To: <1462553178.13075.56.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 06 May 2016 09:46:18 -0700
> 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>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/7] bpf: introduce direct packet access
From: David Miller @ 2016-05-06 20:02 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Thu, 5 May 2016 19:49:08 -0700
> This set of patches introduce 'direct packet access' from
> cls_bpf and act_bpf programs (which are root only).
Series applied, thanks Alexei.
^ permalink raw reply
* Re: [patch net 0/3] mlxsw: Couple of fixes
From: David Miller @ 2016-05-06 20:00 UTC (permalink / raw)
To: jiri; +Cc: netdev, idosch, eladr, yotamg, ogerlitz
In-Reply-To: <1462526244-11898-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 6 May 2016 11:17:21 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Ido Schimmel (2):
> mlxsw: spectrum: Fix rollback order in LAG join failure
> mlxsw: spectrum: Add missing rollback in flood configuration
>
> Jiri Pirko (1):
> mlxsw: spectrum: Fix ordering in mlxsw_sp_fini
What tree is this for? Because on 'net' this makes the build fail.
drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function ‘mlxsw_sp_fini’:
drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2162:2: error: implicit declaration of function ‘mlxsw_sp_buffers_fini’ [-Werror=implicit-function-declaration]
^ permalink raw reply
* Re: [net-next 00/11][pull request] 40GbE Intel Wired LAN Driver Updates 2016-05-05
From: David Miller @ 2016-05-06 19:55 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene, john.ronciak
In-Reply-To: <1462518228-30527-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 6 May 2016 00:03:37 -0700
> This series contains updates to i40e and i40evf.
Looks good, pulled, thanks!
^ permalink raw reply
* Re: [PATCH net v3 2/2] udp_offload: Set encapsulation before inner completes.
From: Alexander Duyck @ 2016-05-06 19:52 UTC (permalink / raw)
To: David Miller; +Cc: Jarno Rajahalme, Netdev, Tom Herbert
In-Reply-To: <20160506.153412.1756777490005831525.davem@davemloft.net>
On Fri, May 6, 2016 at 12:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Jarno Rajahalme <jarno@ovn.org>
> Date: Tue, 3 May 2016 16:10:21 -0700
>
>> UDP tunnel segmentation code relies on the inner offsets being set for
>> an UDP tunnel GSO packet, but the inner *_complete() functions will
>> set the inner offsets only if 'encapsulation' is set before calling
>> them. Currently, udp_gro_complete() sets 'encapsulation' only after
>> the inner *_complete() functions are done. This causes the inner
>> offsets having invalid values after udp_gro_complete() returns, which
>> in turn will make it impossible to properly segment the packet in case
>> it needs to be forwarded, which would be visible to the user either as
>> invalid packets being sent or as packet loss.
>>
>> This patch fixes this by setting skb's 'encapsulation' in
>> udp_gro_complete() before calling into the inner complete functions,
>> and by making each possible UDP tunnel gro_complete() callback set the
>> inner_mac_header to the beginning of the tunnel payload.
>>
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
>> v3: Added setting inner_mac_header from all possible callbacks to cover
>> cases where there is no inner mac header.
>
> Alex and Tom, can you please review this new version since you guys had
> so much feedback for v2?
>
> THanks.
I had reviewed it a day or so ago. It did address the issues I saw
with the original patch, and from what I can tell it is fixing the
original issue reported.
Reviewed-by: Alexander Duyck <aduyck@mirantis.com>
^ permalink raw reply
* Re: [PATCH net-next v2] net: vrf: Create FIB tables on link create
From: David Miller @ 2016-05-06 19:52 UTC (permalink / raw)
To: dsa; +Cc: netdev
In-Reply-To: <1462423572-15722-1-git-send-email-dsa@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Wed, 4 May 2016 21:46:12 -0700
> Tables have to exist for VRFs to function. Ensure they exist
> when VRF device is created.
>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> v2
> - create table before rt6 allocation per comment from DaveM
Yep, this looks better, applied.
^ permalink raw reply
* Re: [PATCH net 1/1] qede: prevent chip hang when increasing channels
From: David Miller @ 2016-05-06 19:51 UTC (permalink / raw)
To: sudarsana.kalluru; +Cc: netdev, Yuval.Mintz
In-Reply-To: <1462422916-25584-1-git-send-email-sudarsana.kalluru@qlogic.com>
From: Sudarsana Reddy Kalluru <sudarsana.kalluru@qlogic.com>
Date: Thu, 5 May 2016 00:35:16 -0400
> qede requires qed to provide enough resources to accommodate 16 combined
> channels, but that upper-bound isn't actually being enforced by it.
> Instead, qed inform back to qede how many channels can be opened based on
> available resources - but that calculation doesn't really take into account
> the resources requested by qede; Instead it considers other FW/HW available
> resources.
>
> As a result, if a user would increase the number of channels to more than
> 16 [e.g., using ethtool] the chip would hang.
>
> This change increments the resources requested by qede to 64 combined
> channels instead of 16; This value is an upper bound on the possible
> available channels [due to other FW/HW resources].
>
> Signed-off-by: Sudarsana Reddy Kalluru <sudarsana.kalluru@qlogic.com>
> Signed-off-by: Yuval Mintz <Yuval.Mintz@qlogic.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: ipv6: tcp reset, icmp need to consider L3 domain
From: David Miller @ 2016-05-06 19:49 UTC (permalink / raw)
To: dsa; +Cc: netdev
In-Reply-To: <1462422368-10822-1-git-send-email-dsa@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Wed, 4 May 2016 21:26:08 -0700
> Responses for packets to unused ports are getting lost with L3 domains.
>
> IPv4 has ip_send_unicast_reply for sending TCP responses which accounts
> for L3 domains; update the IPv6 counterpart tcp_v6_send_response.
> For icmp the L3 master check needs to be moved up in icmp6_send
> to properly respond to UDP packets to a port with no listener.
>
> Fixes: ca254490c8df ("net: Add VRF support to IPv6 stack")
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
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 19:48 UTC (permalink / raw)
To: Mike Manning; +Cc: Netdev
In-Reply-To: <572CF258.4070309@brocade.com>
On Fri, May 6, 2016 at 12:36 PM, Mike Manning <mmanning@brocade.com> wrote:
> On 05/06/2016 06:02 PM, Alexander Duyck wrote:
>> 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.
>>
>
> Thanks for the detailed review. I had initially used the existing type
> in net_device, but the problem with this was that it got overwritten to
> NET_ADDR_SET in dev_set_mac_address(), which I was reluctant to modify.
> It would just be a case of setting the type earlier in that function
> (and caching the previous value in case there is an error).
>
> However, based on your later comment, it seems I should not bother with
> the approach I have here, namely that if the VLAN MAC is set to the same
> value as that of the lower device MAC, that is to be considered as
> resetting it and thus for MAC inheritance to resume. Instead, I will just
> make this a 1-shot transition, i.e. the VLAN MAC starts off as inherited,
> and if it is set to anything (even the value of the lower device MAC),
> inheritance is stopped. I agree this makes for a far simpler changeset.
>
> I don't think I can remove real_dev_addr, as that is still needed for
> the existing functionality in vlan_sync_address() to determine if the sync
> should be done, also as a way of caching it for handling in vlan_dev_open().
The thing is that logic isn't really needed anymore though if you are
going to be following the lower dev. If you follow the code what it
is doing is adding the address via dev_uc_add if the lower address
moves away from the VLAN address. With your changes you are updating
the VLAN MAC address to the lower value in the NET_ADDR_STOLEN case so
you don't need to add or remove an extra unicast address. If the user
sets the MAC address you can then use the vlandev->dev_addr as the
address you add/remove from the unicast list and you probably don't
need to bother with tracking the lower device state anyway.
> As a matter of interest, what is the advantage of not updating the VLAN
> MAC when it is down? I appreciate that one should not add/delete
> secondary unicast addresses in this case, but there is no such
> restriction for copying the MAC.
Basically you are just wasting cycles messing with it while it is
down. You don't need to bother with syncing up the addresses until
you bring the interface up. At that point you essentially need to do
the vlan_sync_address type work anyway because you have to push your
address to the lower dev, or you have to pull it up from the lower dev
in the case of the stolen address. You don't want to have MAC
addresses written to the device for an interface that is down.
^ permalink raw reply
* Re: [RESEND PATCH 2/3] fs: poll/select/recvmmsg: use timespec64 for timeout events
From: David Miller @ 2016-05-06 19:45 UTC (permalink / raw)
To: john.stultz
Cc: eliezer.tamir, arnd, y2038, netdev, linux-kernel, oleg, arjan,
deepa.kernel, linux-fsdevel, akpm, torvalds, tglx, viro
In-Reply-To: <CALAqxLVPGBfuSmN_b_obAL5-ChfqpyipZtE-20x9omk_QeTQMw@mail.gmail.com>
From: John Stultz <john.stultz@linaro.org>
Date: Wed, 4 May 2016 17:01:24 -0700
> On Wed, May 4, 2016 at 4:51 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> On Wed, 04 May 2016 23:08:11 +0200 Arnd Bergmann <arnd@arndb.de> wrote:
>>
>>> > But I'm less comfortable making the call on this one. It looks
>>> > relatively straight forward, but it would be good to have maintainer
>>> > acks before I add it to my tree.
>>>
>>> Agreed. Feel free to add my
>>>
>>> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>>>
>>> at least (whoever picks it up).
>>
>> In reply to [1/3] John said
>>
>> : Looks ok at the first glance. I've queued these up for testing,
>> : however I only got #1 and #3 of the set. Are you hoping these two
>> : patches will go through tip/timers/core or are you looking for acks so
>> : they can go via another tree?
>>
>> However none of the patches are in linux-next.
>>
>> John had qualms about [2/3], but it looks like a straightforward
>> substitution in areas which will get plenty of testing
>
> Yea. My main concern is just not stepping on any other maintainers toes.
The networking changes look fine to me:
Acked-by: David S. Miller <davem@davemloft.net>
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH net-next] cnic: call cp->stop_hw() in cnic_start_hw() on allocation failure
From: David Miller @ 2016-05-06 19:45 UTC (permalink / raw)
To: jmaxwell37; +Cc: netdev, linux-kernel, jmaxwell, ivecera
In-Reply-To: <1462406151-5224-1-git-send-email-jmaxwell37@gmail.com>
From: Jon Maxwell <jmaxwell37@gmail.com>
Date: Thu, 5 May 2016 09:55:51 +1000
> We recently had a system crash in the cnic module. Vmcore analysis confirmed
> that "ip link up" was executed which failed due to an allocation failure
> because of memory fragmentation. Futher analysis revealed that the cnic irq
> vector was still allocated after the "ip link up" that failed. When
> "ip link down" was executed it called free_msi_irqs() which crashed the system
> because the cnic irq was still inuse.
...
> The cnic_start_hw() routine is not handling the allocation failure correctly.
> Fix this by checking whether CNIC_DRV_STATE_HANDLES_IRQ flag is set indicating
> that the hardware has been started in cnic_start_hw(). If it has then call
> cp->stop_hw() which frees the cnic irq vector and cnic resources. Otherwise
> just maintain the previous behaviour and free cnic resources.
>
> I reproduced this by injecting an ENOMEM error into cnic_cm_alloc_mem()s return
> code.
>
> # ip link set dev enpX down
> # ip link set dev enpX up <--- hit's allocation failure
> # ip link set dev enpX down <--- crashes here
>
> With this patch I confirmed there was no crash in the reproducer.
>
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: OpenWRT wrong adjustment of fq_codel defaults (Was: fq_codel_drop vs a udp flood)
From: Dave Taht @ 2016-05-06 19:43 UTC (permalink / raw)
To: Roman Yeryomin
Cc: make-wifi-fast, Rafał Miłecki, ath10k,
netdev@vger.kernel.org, codel@lists.bufferbloat.net,
Jonathan Morton, OpenWrt Development List, Felix Fietkau
In-Reply-To: <CACiydb+A_vFNdp7tbw67Rnp+kHwt5GsLa+Su+pO8YmOZYp_KHg@mail.gmail.com>
On Fri, May 6, 2016 at 11:56 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote:
> On 6 May 2016 at 21:43, Roman Yeryomin <leroi.lists@gmail.com> wrote:
>> On 6 May 2016 at 15:47, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>>>
>>> I've created a OpenWRT ticket[1] on this issue, as it seems that someone[2]
>>> closed Felix'es OpenWRT email account (bad choice! emails bouncing).
>>> Sounds like OpenWRT and the LEDE https://www.lede-project.org/ project
>>> is in some kind of conflict.
>>>
>>> OpenWRT ticket [1] https://dev.openwrt.org/ticket/22349
>>>
>>> [2] http://thread.gmane.org/gmane.comp.embedded.openwrt.devel/40298/focus=40335
>>
>> OK, so, after porting the patch to 4.1 openwrt kernel and playing a
>> bit with fq_codel limits I was able to get 420Mbps UDP like this:
>> tc qdisc replace dev wlan0 parent :1 fq_codel flows 16 limit 256
>
> Forgot to mention, I've reduced drop_batch_size down to 32
0) Not clear to me if that's the right line, there are 4 wifi queues,
and the third one
is the BE queue. That is too low a limit, also, for normal use. And:
for the purpose of this particular UDP test, flows 16 is ok, but not
ideal.
1) What's the tcp number (with a simultaneous ping) with this latest patchset?
(I care about tcp performance a lot more than udp floods - surviving a
udp flood yes, performance, no)
before/after?
tc -s qdisc show dev wlan0 during/after results?
IF you are doing builds for the archer c7v2, I can join in on this... (?)
I did do a test of the ath10k "before", fq_codel *never engaged*, and
tcp induced latencies under load, e at 100mbit, cracked 600ms, while
staying flat (20ms) at 100mbit. (not the same patches you are testing)
on x86. I have got tcp 300Mbit out of an osx box, similar latency,
have yet to get anything more on anything I currently have
before/after patchsets.
I'll go add flooding to the tests, I just finished a series comparing
two different speed stations and life was good on that.
"before" - fq_codel never engages, we see seconds of latency under load.
root@apu2:~# tc -s qdisc show dev wlp4s0
qdisc mq 0: root
Sent 8570563893 bytes 6326983 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
qdisc fq_codel 0: parent :1 limit 10240p flows 1024 quantum 1514
target 5.0ms interval 100.0ms ecn
Sent 2262 bytes 17 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
maxpacket 0 drop_overlimit 0 new_flow_count 0 ecn_mark 0
new_flows_len 0 old_flows_len 0
qdisc fq_codel 0: parent :2 limit 10240p flows 1024 quantum 1514
target 5.0ms interval 100.0ms ecn
Sent 220486569 bytes 152058 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
maxpacket 18168 drop_overlimit 0 new_flow_count 1 ecn_mark 0
new_flows_len 0 old_flows_len 1
qdisc fq_codel 0: parent :3 limit 10240p flows 1024 quantum 1514
target 5.0ms interval 100.0ms ecn
Sent 8340546509 bytes 6163431 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
maxpacket 68130 drop_overlimit 0 new_flow_count 120050 ecn_mark 0
new_flows_len 1 old_flows_len 3
qdisc fq_codel 0: parent :4 limit 10240p flows 1024 quantum 1514
target 5.0ms interval 100.0ms ecn
Sent 9528553 bytes 11477 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
maxpacket 66 drop_overlimit 0 new_flow_count 1 ecn_mark 0
new_flows_len 1 old_flows_len 0
```
>> This is certainly better than 30Mbps but still more than two times
>> less than before (900).
The number that I still am not sure we got is that you were sending
900mbit udp and recieving 900mbit on the prior tests?
>> TCP also improved a little (550 to ~590).
The limit is probably a bit low, also. You might want to try target
20ms as well.
>>
>> Felix, others, do you want to see the ported patch, maybe I did something wrong?
>> Doesn't look like it will save ath10k from performance regression.
what was tcp "before"? (I'm sorry, such a long thread)
>>
>>>
>>> On Fri, 6 May 2016 11:42:43 +0200
>>> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>>>
>>>> Hi Felix,
>>>>
>>>> This is an important fix for OpenWRT, please read!
>>>>
>>>> OpenWRT changed the default fq_codel sch->limit from 10240 to 1024,
>>>> without also adjusting q->flows_cnt. Eric explains below that you must
>>>> also adjust the buckets (q->flows_cnt) for this not to break. (Just
>>>> adjust it to 128)
>>>>
>>>> Problematic OpenWRT commit in question:
>>>> http://git.openwrt.org/?p=openwrt.git;a=patch;h=12cd6578084e
>>>> 12cd6578084e ("kernel: revert fq_codel quantum override to prevent it from causing too much cpu load with higher speed (#21326)")
>>>>
>>>>
>>>> I also highly recommend you cherry-pick this very recent commit:
>>>> net-next: 9d18562a2278 ("fq_codel: add batch ability to fq_codel_drop()")
>>>> https://git.kernel.org/davem/net-next/c/9d18562a227
>>>>
>>>> This should fix very high CPU usage in-case fq_codel goes into drop mode.
>>>> The problem is that drop mode was considered rare, and implementation
>>>> wise it was chosen to be more expensive (to save cycles on normal mode).
>>>> Unfortunately is it easy to trigger with an UDP flood. Drop mode is
>>>> especially expensive for smaller devices, as it scans a 4K big array,
>>>> thus 64 cache misses for small devices!
>>>>
>>>> The fix is to allow drop-mode to bulk-drop more packets when entering
>>>> drop-mode (default 64 bulk drop). That way we don't suddenly
>>>> experience a significantly higher processing cost per packet, but
>>>> instead can amortize this.
>>>>
>>>> To Eric, should we recommend OpenWRT to adjust default (max) 64 bulk
>>>> drop, given we also recommend bucket size to be 128 ? (thus the amount
>>>> of memory to scan is less, but their CPU is also much smaller).
>>>>
>>>> --Jesper
>>>>
>>>>
>>>> On Thu, 05 May 2016 12:23:27 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>>
>>>> > On Thu, 2016-05-05 at 19:25 +0300, Roman Yeryomin wrote:
>>>> > > On 5 May 2016 at 19:12, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>> > > > On Thu, 2016-05-05 at 17:53 +0300, Roman Yeryomin wrote:
>>>> > > >
>>>> > > >>
>>>> > > >> qdisc fq_codel 0: dev eth0 root refcnt 2 limit 1024p flows 1024
>>>> > > >> quantum 1514 target 5.0ms interval 100.0ms ecn
>>>> > > >> Sent 12306 bytes 128 pkt (dropped 0, overlimits 0 requeues 0)
>>>> > > >> backlog 0b 0p requeues 0
>>>> > > >> maxpacket 0 drop_overlimit 0 new_flow_count 0 ecn_mark 0
>>>> > > >> new_flows_len 0 old_flows_len 0
>>>> > > >
>>>> > > >
>>>> > > > Limit of 1024 packets and 1024 flows is not wise I think.
>>>> > > >
>>>> > > > (If all buckets are in use, each bucket has a virtual queue of 1 packet,
>>>> > > > which is almost the same than having no queue at all)
>>>> > > >
>>>> > > > I suggest to have at least 8 packets per bucket, to let Codel have a
>>>> > > > chance to trigger.
>>>> > > >
>>>> > > > So you could either reduce number of buckets to 128 (if memory is
>>>> > > > tight), or increase limit to 8192.
>>>> > >
>>>> > > Will try, but what I've posted is default, I didn't change/configure that.
>>>> >
>>>> > fq_codel has a default of 10240 packets and 1024 buckets.
>>>> >
>>>> > http://lxr.free-electrons.com/source/net/sched/sch_fq_codel.c#L413
>>>> >
>>>> > If someone changed that in the linux variant you use, he probably should
>>>> > explain the rationale.
>>>
>>> --
>>> Best regards,
>>> Jesper Dangaard Brouer
>>> MSc.CS, Principal Kernel Engineer at Red Hat
>>> Author of http://www.iptv-analyzer.org
>>> LinkedIn: http://www.linkedin.com/in/brouer
--
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org
_______________________________________________
Codel mailing list
Codel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/codel
^ permalink raw reply
* [PATCH] e1000e: prevent division by zero if TIMINCA is zero
From: Denys Vlasenko @ 2016-05-06 19:41 UTC (permalink / raw)
To: Jeff Kirsher
Cc: Denys Vlasenko, Ruinskiy, Dima, intel-wired-lan, netdev, LKML
Users report that under VMWare, er32(TIMINCA) returns zero.
This causes division by zero at init time as follows:
==> incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
/* latch SYSTIMH on read of SYSTIML */
systim_next = (cycle_t)er32(SYSTIML);
systim_next |= (cycle_t)er32(SYSTIMH) << 32;
time_delta = systim_next - systim;
temp = time_delta;
====> rem = do_div(temp, incvalue);
This change makes kernel survive this, and users report that
NIC does work after this change.
Since on real hardware incvalue is never zero, this should not affect
real hardware use case.
Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: "Ruinskiy, Dima" <dima.ruinskiy@intel.com>
CC: intel-wired-lan@lists.osuosl.org
CC: netdev@vger.kernel.org
CC: LKML <linux-kernel@vger.kernel.org>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 269087c..0626935 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4315,7 +4315,8 @@ static cycle_t e1000e_cyclecounter_read(const struct cyclecounter *cc)
time_delta = systim_next - systim;
temp = time_delta;
- rem = do_div(temp, incvalue);
+ /* VMWare users have seen incvalue of zero, don't div / 0 */
+ rem = incvalue ? do_div(temp, incvalue) : (time_delta != 0);
systim = systim_next;
--
1.8.1.4
^ permalink raw reply related
* Re: [PATCH net] netfilter: nf_conntrack: Use net_mutex for helper unregistration.
From: Joe Stringer @ 2016-05-06 19:38 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netdev, netfilter-devel, Florian Westphal
In-Reply-To: <20160506110325.GA2420@salvia>
On 6 May 2016 at 04:03, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Joe,
>
> On Thu, May 05, 2016 at 03:50:37PM -0700, Joe Stringer wrote:
>> diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
>> index 3b40ec575cd5..6860b19be406 100644
>> --- a/net/netfilter/nf_conntrack_helper.c
>> +++ b/net/netfilter/nf_conntrack_helper.c
>> @@ -449,10 +449,10 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
>> */
>> synchronize_rcu();
>>
>> - rtnl_lock();
>> + mutex_lock(&net_mutex);
>> for_each_net(net)
>> __nf_conntrack_helper_unregister(me, net);
>> - rtnl_unlock();
>> + mutex_unlock(&net_mutex);
>
> This simple solution works because we have no .exit callbacks in any
> of our helpers. Otherwise, the helper code may be already gone by when
> the worker has a chance to run to release the netns.
I'm open to any alternative solutions, but if helper code isn't doing
this yet then perhaps this fix is sufficient?
> If so, probably I can append this as comment to this function so we
> don't forget. If we ever have .exit callbacks (I don't expect so), we
> would need to wait for worker completion.
Sounds reasonable to me.
I see there's a bunch of other unregister locations like
nf_nat_l3proto_clean(), nf_nat_l4proto_clean(), nf_unregister_hook()
which might need similar treatment?
^ permalink raw reply
* Re: [PATCH net-next 1/2] sfc: Support setting rss_cpus to 'cores', 'packages' or 'hyperthreads'
From: David Miller @ 2016-05-06 19:38 UTC (permalink / raw)
To: ecree; +Cc: linux-net-drivers, netdev
In-Reply-To: <572A2B00.7060301@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Wed, 4 May 2016 18:01:52 +0100
> These settings autoconfigure the number of RSS channels to match the number of
> CPUs present.
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
I can't believe I allowed this 'rss_cpus' thing into the tree to begin with.
It's completely wrong and is exactly the kind of thing we are trying
to actively avoid in network drivers.
If another network driver wants to provide the same facility they will
add a module parameter with a slightly different name, a different
set of valid choices, and different semantics.
Define a proper global, stable, tree-wide mechanism to configure these
kinds of things and use that instead.
Thanks.
^ permalink raw reply
* Re: [PATCH net v3] vlan: Propagate MAC address to VLANs
From: Mike Manning @ 2016-05-06 19:36 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Netdev
In-Reply-To: <CAKgT0UeOhYkajP7PJo15cvTJsiEo_Z0OD7Kf=z1iXVRWQhkqaQ@mail.gmail.com>
On 05/06/2016 06:02 PM, Alexander Duyck wrote:
> 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.
>
Thanks for the detailed review. I had initially used the existing type
in net_device, but the problem with this was that it got overwritten to
NET_ADDR_SET in dev_set_mac_address(), which I was reluctant to modify.
It would just be a case of setting the type earlier in that function
(and caching the previous value in case there is an error).
However, based on your later comment, it seems I should not bother with
the approach I have here, namely that if the VLAN MAC is set to the same
value as that of the lower device MAC, that is to be considered as
resetting it and thus for MAC inheritance to resume. Instead, I will just
make this a 1-shot transition, i.e. the VLAN MAC starts off as inherited,
and if it is set to anything (even the value of the lower device MAC),
inheritance is stopped. I agree this makes for a far simpler changeset.
I don't think I can remove real_dev_addr, as that is still needed for
the existing functionality in vlan_sync_address() to determine if the sync
should be done, also as a way of caching it for handling in vlan_dev_open().
As a matter of interest, what is the advantage of not updating the VLAN
MAC when it is down? I appreciate that one should not add/delete
secondary unicast addresses in this case, but there is no such
restriction for copying the MAC.
>> --- 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.
>
As per first comment, if I do this, it gets overridden by
^ permalink raw reply
* Re: [PATCH 2/3] net/mlx5e: make VXLAN support conditional
From: David Miller @ 2016-05-06 19:35 UTC (permalink / raw)
To: arnd-r2nGTMty4D4
Cc: saeedm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb,
matanb-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w,
saeedm-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, matt-VPRAkNaXOzVWk0Htik3J/w,
richardcochran-Re5JQEeQqe8AvxtiuMwx3w,
amirv-VPRAkNaXOzVWk0Htik3J/w, hagaya-VPRAkNaXOzVWk0Htik3J/w,
maorg-VPRAkNaXOzVWk0Htik3J/w, ogerlitz-VPRAkNaXOzVWk0Htik3J/w,
majd-VPRAkNaXOzVWk0Htik3J/w, achiad-VPRAkNaXOzVWk0Htik3J/w,
tariqt-VPRAkNaXOzVWk0Htik3J/w, galp-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <4306900.EI0LQtX7kx@wuerfel>
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Date: Thu, 05 May 2016 20:09:19 +0200
> For reference, I've tried it out on the MLX4 driver, and it does
> seem nicer that way, see below.
Is it possible to wind down this conversation and have someone submit
whatever final patch everyone agrees to?
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 v3 2/2] udp_offload: Set encapsulation before inner completes.
From: David Miller @ 2016-05-06 19:34 UTC (permalink / raw)
To: jarno; +Cc: netdev, alexander.duyck, tom
In-Reply-To: <1462317021-7236-2-git-send-email-jarno@ovn.org>
From: Jarno Rajahalme <jarno@ovn.org>
Date: Tue, 3 May 2016 16:10:21 -0700
> UDP tunnel segmentation code relies on the inner offsets being set for
> an UDP tunnel GSO packet, but the inner *_complete() functions will
> set the inner offsets only if 'encapsulation' is set before calling
> them. Currently, udp_gro_complete() sets 'encapsulation' only after
> the inner *_complete() functions are done. This causes the inner
> offsets having invalid values after udp_gro_complete() returns, which
> in turn will make it impossible to properly segment the packet in case
> it needs to be forwarded, which would be visible to the user either as
> invalid packets being sent or as packet loss.
>
> This patch fixes this by setting skb's 'encapsulation' in
> udp_gro_complete() before calling into the inner complete functions,
> and by making each possible UDP tunnel gro_complete() callback set the
> inner_mac_header to the beginning of the tunnel payload.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---
> v3: Added setting inner_mac_header from all possible callbacks to cover
> cases where there is no inner mac header.
Alex and Tom, can you please review this new version since you guys had
so much feedback for v2?
THanks.
^ permalink raw reply
* Re: OpenWRT wrong adjustment of fq_codel defaults (Was: fq_codel_drop vs a udp flood)
From: Roman Yeryomin @ 2016-05-06 18:56 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: make-wifi-fast, Rafał Miłecki, ath10k,
codel@lists.bufferbloat.net, netdev@vger.kernel.org,
Jonathan Morton, OpenWrt Development List, Felix Fietkau
In-Reply-To: <CACiydbL3qZrvZ4a_OUXiN0dp7qdqOktaQdAUdfOpKkZ1OfEZbA@mail.gmail.com>
On 6 May 2016 at 21:43, Roman Yeryomin <leroi.lists@gmail.com> wrote:
> On 6 May 2016 at 15:47, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>>
>> I've created a OpenWRT ticket[1] on this issue, as it seems that someone[2]
>> closed Felix'es OpenWRT email account (bad choice! emails bouncing).
>> Sounds like OpenWRT and the LEDE https://www.lede-project.org/ project
>> is in some kind of conflict.
>>
>> OpenWRT ticket [1] https://dev.openwrt.org/ticket/22349
>>
>> [2] http://thread.gmane.org/gmane.comp.embedded.openwrt.devel/40298/focus=40335
>
> OK, so, after porting the patch to 4.1 openwrt kernel and playing a
> bit with fq_codel limits I was able to get 420Mbps UDP like this:
> tc qdisc replace dev wlan0 parent :1 fq_codel flows 16 limit 256
Forgot to mention, I've reduced drop_batch_size down to 32
> This is certainly better than 30Mbps but still more than two times
> less than before (900).
> TCP also improved a little (550 to ~590).
>
> Felix, others, do you want to see the ported patch, maybe I did something wrong?
> Doesn't look like it will save ath10k from performance regression.
>
>>
>> On Fri, 6 May 2016 11:42:43 +0200
>> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>>
>>> Hi Felix,
>>>
>>> This is an important fix for OpenWRT, please read!
>>>
>>> OpenWRT changed the default fq_codel sch->limit from 10240 to 1024,
>>> without also adjusting q->flows_cnt. Eric explains below that you must
>>> also adjust the buckets (q->flows_cnt) for this not to break. (Just
>>> adjust it to 128)
>>>
>>> Problematic OpenWRT commit in question:
>>> http://git.openwrt.org/?p=openwrt.git;a=patch;h=12cd6578084e
>>> 12cd6578084e ("kernel: revert fq_codel quantum override to prevent it from causing too much cpu load with higher speed (#21326)")
>>>
>>>
>>> I also highly recommend you cherry-pick this very recent commit:
>>> net-next: 9d18562a2278 ("fq_codel: add batch ability to fq_codel_drop()")
>>> https://git.kernel.org/davem/net-next/c/9d18562a227
>>>
>>> This should fix very high CPU usage in-case fq_codel goes into drop mode.
>>> The problem is that drop mode was considered rare, and implementation
>>> wise it was chosen to be more expensive (to save cycles on normal mode).
>>> Unfortunately is it easy to trigger with an UDP flood. Drop mode is
>>> especially expensive for smaller devices, as it scans a 4K big array,
>>> thus 64 cache misses for small devices!
>>>
>>> The fix is to allow drop-mode to bulk-drop more packets when entering
>>> drop-mode (default 64 bulk drop). That way we don't suddenly
>>> experience a significantly higher processing cost per packet, but
>>> instead can amortize this.
>>>
>>> To Eric, should we recommend OpenWRT to adjust default (max) 64 bulk
>>> drop, given we also recommend bucket size to be 128 ? (thus the amount
>>> of memory to scan is less, but their CPU is also much smaller).
>>>
>>> --Jesper
>>>
>>>
>>> On Thu, 05 May 2016 12:23:27 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>
>>> > On Thu, 2016-05-05 at 19:25 +0300, Roman Yeryomin wrote:
>>> > > On 5 May 2016 at 19:12, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> > > > On Thu, 2016-05-05 at 17:53 +0300, Roman Yeryomin wrote:
>>> > > >
>>> > > >>
>>> > > >> qdisc fq_codel 0: dev eth0 root refcnt 2 limit 1024p flows 1024
>>> > > >> quantum 1514 target 5.0ms interval 100.0ms ecn
>>> > > >> Sent 12306 bytes 128 pkt (dropped 0, overlimits 0 requeues 0)
>>> > > >> backlog 0b 0p requeues 0
>>> > > >> maxpacket 0 drop_overlimit 0 new_flow_count 0 ecn_mark 0
>>> > > >> new_flows_len 0 old_flows_len 0
>>> > > >
>>> > > >
>>> > > > Limit of 1024 packets and 1024 flows is not wise I think.
>>> > > >
>>> > > > (If all buckets are in use, each bucket has a virtual queue of 1 packet,
>>> > > > which is almost the same than having no queue at all)
>>> > > >
>>> > > > I suggest to have at least 8 packets per bucket, to let Codel have a
>>> > > > chance to trigger.
>>> > > >
>>> > > > So you could either reduce number of buckets to 128 (if memory is
>>> > > > tight), or increase limit to 8192.
>>> > >
>>> > > Will try, but what I've posted is default, I didn't change/configure that.
>>> >
>>> > fq_codel has a default of 10240 packets and 1024 buckets.
>>> >
>>> > http://lxr.free-electrons.com/source/net/sched/sch_fq_codel.c#L413
>>> >
>>> > If someone changed that in the linux variant you use, he probably should
>>> > explain the rationale.
>>
>> --
>> Best regards,
>> Jesper Dangaard Brouer
>> MSc.CS, Principal Kernel Engineer at Red Hat
>> Author of http://www.iptv-analyzer.org
>> LinkedIn: http://www.linkedin.com/in/brouer
_______________________________________________
Codel mailing list
Codel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/codel
^ permalink raw reply
* Re: [PATCH] Add support for configuring Infiniband GUIDs
From: Stephen Hemminger @ 2016-05-06 18:53 UTC (permalink / raw)
To: Eli Cohen; +Cc: shemminger, netdev
In-Reply-To: <1462549405-16003-1-git-send-email-eli@mellanox.com>
On Fri, 6 May 2016 10:43:25 -0500
Eli Cohen <eli@mellanox.com> 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>
I am not that familiar with Infiniband, but the documentation seems
to use a non-colon form:
# ip link set dev ib0 vf 0 node_guid 0002c90300216e70
Seems like ip should follow the lead of ibstat and friends.
^ permalink raw reply
* Re: [PATCH iproute2 0/2] ip link gre: fix external mode handling
From: Stephen Hemminger @ 2016-05-06 18:50 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, Paolo Abeni, Pravin Shelar
In-Reply-To: <cover.1461766016.git.jbenc@redhat.com>
On Wed, 27 Apr 2016 16:11:12 +0200
Jiri Benc <jbenc@redhat.com> wrote:
> Fix two bugs with handling of the 'external' keyword for GRE.
>
> Jiri Benc (2):
> ip link gre: create interfaces in external mode correctly
> ip link gre: print only relevant info in external mode
>
> ip/link_gre.c | 43 +++++++++++++++++++++++++------------------
> 1 file changed, 25 insertions(+), 18 deletions(-)
>
Applied
^ permalink raw reply
* Re: OpenWRT wrong adjustment of fq_codel defaults (Was: fq_codel_drop vs a udp flood)
From: Roman Yeryomin @ 2016-05-06 18:43 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: make-wifi-fast, Rafał Miłecki, ath10k,
codel@lists.bufferbloat.net, netdev@vger.kernel.org,
Jonathan Morton, OpenWrt Development List, Felix Fietkau
In-Reply-To: <20160506144740.210901f5@redhat.com>
On 6 May 2016 at 15:47, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> I've created a OpenWRT ticket[1] on this issue, as it seems that someone[2]
> closed Felix'es OpenWRT email account (bad choice! emails bouncing).
> Sounds like OpenWRT and the LEDE https://www.lede-project.org/ project
> is in some kind of conflict.
>
> OpenWRT ticket [1] https://dev.openwrt.org/ticket/22349
>
> [2] http://thread.gmane.org/gmane.comp.embedded.openwrt.devel/40298/focus=40335
OK, so, after porting the patch to 4.1 openwrt kernel and playing a
bit with fq_codel limits I was able to get 420Mbps UDP like this:
tc qdisc replace dev wlan0 parent :1 fq_codel flows 16 limit 256
This is certainly better than 30Mbps but still more than two times
less than before (900).
TCP also improved a little (550 to ~590).
Felix, others, do you want to see the ported patch, maybe I did something wrong?
Doesn't look like it will save ath10k from performance regression.
>
> On Fri, 6 May 2016 11:42:43 +0200
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
>> Hi Felix,
>>
>> This is an important fix for OpenWRT, please read!
>>
>> OpenWRT changed the default fq_codel sch->limit from 10240 to 1024,
>> without also adjusting q->flows_cnt. Eric explains below that you must
>> also adjust the buckets (q->flows_cnt) for this not to break. (Just
>> adjust it to 128)
>>
>> Problematic OpenWRT commit in question:
>> http://git.openwrt.org/?p=openwrt.git;a=patch;h=12cd6578084e
>> 12cd6578084e ("kernel: revert fq_codel quantum override to prevent it from causing too much cpu load with higher speed (#21326)")
>>
>>
>> I also highly recommend you cherry-pick this very recent commit:
>> net-next: 9d18562a2278 ("fq_codel: add batch ability to fq_codel_drop()")
>> https://git.kernel.org/davem/net-next/c/9d18562a227
>>
>> This should fix very high CPU usage in-case fq_codel goes into drop mode.
>> The problem is that drop mode was considered rare, and implementation
>> wise it was chosen to be more expensive (to save cycles on normal mode).
>> Unfortunately is it easy to trigger with an UDP flood. Drop mode is
>> especially expensive for smaller devices, as it scans a 4K big array,
>> thus 64 cache misses for small devices!
>>
>> The fix is to allow drop-mode to bulk-drop more packets when entering
>> drop-mode (default 64 bulk drop). That way we don't suddenly
>> experience a significantly higher processing cost per packet, but
>> instead can amortize this.
>>
>> To Eric, should we recommend OpenWRT to adjust default (max) 64 bulk
>> drop, given we also recommend bucket size to be 128 ? (thus the amount
>> of memory to scan is less, but their CPU is also much smaller).
>>
>> --Jesper
>>
>>
>> On Thu, 05 May 2016 12:23:27 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> > On Thu, 2016-05-05 at 19:25 +0300, Roman Yeryomin wrote:
>> > > On 5 May 2016 at 19:12, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > > > On Thu, 2016-05-05 at 17:53 +0300, Roman Yeryomin wrote:
>> > > >
>> > > >>
>> > > >> qdisc fq_codel 0: dev eth0 root refcnt 2 limit 1024p flows 1024
>> > > >> quantum 1514 target 5.0ms interval 100.0ms ecn
>> > > >> Sent 12306 bytes 128 pkt (dropped 0, overlimits 0 requeues 0)
>> > > >> backlog 0b 0p requeues 0
>> > > >> maxpacket 0 drop_overlimit 0 new_flow_count 0 ecn_mark 0
>> > > >> new_flows_len 0 old_flows_len 0
>> > > >
>> > > >
>> > > > Limit of 1024 packets and 1024 flows is not wise I think.
>> > > >
>> > > > (If all buckets are in use, each bucket has a virtual queue of 1 packet,
>> > > > which is almost the same than having no queue at all)
>> > > >
>> > > > I suggest to have at least 8 packets per bucket, to let Codel have a
>> > > > chance to trigger.
>> > > >
>> > > > So you could either reduce number of buckets to 128 (if memory is
>> > > > tight), or increase limit to 8192.
>> > >
>> > > Will try, but what I've posted is default, I didn't change/configure that.
>> >
>> > fq_codel has a default of 10240 packets and 1024 buckets.
>> >
>> > http://lxr.free-electrons.com/source/net/sched/sch_fq_codel.c#L413
>> >
>> > If someone changed that in the linux variant you use, he probably should
>> > explain the rationale.
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> Author of http://www.iptv-analyzer.org
> LinkedIn: http://www.linkedin.com/in/brouer
_______________________________________________
Codel mailing list
Codel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/codel
^ permalink raw reply
* Re: [REGRESSION] asix: Lots of asix_rx_fixup() errors and slow transmissions
From: John Stultz @ 2016-05-06 18:38 UTC (permalink / raw)
To: Dean Jenkins
Cc: David B. Robins, lkml, Mark Craske, David S. Miller, YongQin Liu,
Guodong Xu, linux-usb-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Ivan Vecera
In-Reply-To: <57291539.6080405-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
On Tue, May 3, 2016 at 2:16 PM, Dean Jenkins <Dean_Jenkins-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org> wrote:
> A good test would be to run "ping -c 1 -s $packet_length $ip_address" inside
> a script which has a loop with an increasing payload length $packet_length
> with a small delay between ping calls. This will show whether particular
> packet sizes trigger the failures.
>
> Then try with "ping -f -c 200 -s $packet_length $ip_address" to load up the
> USB link.
I've tried both of these on my x86_64 system. I can send single pings
up to 65507 without triggering the issue (after which I get errors
sending on the host side as I think I cross a 64k boundary with
headers, not the asix errors).
Then when I try ping -f -c 200 -s 65507 $ip_address, I don't see any
failures. I did it for a count of 2000 as well without any issues.
I'll be adding more debug prints in soon.
thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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] netdev: enc28j60 kernel panic fix.
From: David Russell @ 2016-05-06 18:26 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
In-Reply-To: <20160505085110.GA29858@electric-eye.fr.zoreil.com>
I kind of thought my patch was at best incomplete. When you state
this change silences the bug but does not fix it, what are the
implications of systems running this patch? We have some production
systems using this patch. They reboot daily, but have been solid.
In addition, if we sent you a pi and the ethernet controller and a
small but reasonable sum of money for your labor, would you be able to
properly fix it? Short of that, do you have any recommendations on
quick overviews of the networking stack in the kernel and then
documentation on the various flags and such?
Thanks.
-David Russell
APRS World, LLC
http://www.aprsworld.com/
On Thu, May 5, 2016 at 3:51 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> David Russell <david@aprsworld.com> :
>> When connected directly to another system (not via a switch)
>> eventually a condition where a NULL pointer dereference occurs in
>> enc28j60_hw_tx() and this patch simply checks for that condition and
>> returns gracefully without causing a kernel panic. I believe, but
>> have not investigated this is caused by a packet collision and am not
>> sure if the kernel tracks collisions or counts them as errors, so that
>> should probably be added if this is what's happening. I'm also not
>> familiar with the linux kernel, so may have fixed this in a less than
>> ideal way.
>
> Is it possible for EIR.EIR_TXERIF and EIR.EIR_TXIF to be set for the
> same packet ?
>
> If so the driver is intrinsically racy:
> - EIR.EIR_TXIF completes transmission, clears tx_skb and enables queueing
> again (see netif_wake_queue in enc28j60_tx_clear)
>
> - insert start_xmit here: tx_skb is set and enc28j60_hw_tx is scheduled
> for late execution (user context work)
>
> - EIR.EIR_EIR.EIR_TXERIF issues same enc28j60_tx_clear and clears tx_skb
>
> - enc28j60_hw_tx is run but tx_skb is NULL
>
>> diff --git a/drivers/net/ethernet/microchip/enc28j60.c
>> b/drivers/net/ethernet/microchip/enc28j60.c
>> index 86ea17e..36ac65f 100644
>> --- a/drivers/net/ethernet/microchip/enc28j60.c
>> +++ b/drivers/net/ethernet/microchip/enc28j60.c
>> @@ -1233,6 +1233,9 @@ static void enc28j60_irq_work_handler(struct
>> work_struct *work)
>> */
>> static void enc28j60_hw_tx(struct enc28j60_net *priv)
>> {
>> + if (!priv->tx_skb)
>> + return;
>> +
>> if (netif_msg_tx_queued(priv))
>> printk(KERN_DEBUG DRV_NAME
>> ": Tx Packet Len:%d\n", priv->tx_skb->len);
>
> enc28j60_hw_tx isn't the culprit. It's the victim.
>
> This change silences the bug but it does not fix it at all.
>
> --
> Ueimor
^ 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
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