* 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
* 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
* [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: [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 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: [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
* 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] 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: [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 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 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: [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: [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: [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
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