* Re: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Andrew Lunn @ 2017-09-18 19:57 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41124D2E@CHN-SV-EXMX02.mchp-main.com>
On Mon, Sep 18, 2017 at 07:38:03PM +0000, Tristram.Ha@microchip.com wrote:
> Sorry for the late response.
>
> > > diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c index 010ca0a..d5faf14
> > 100644
> > > --- a/net/dsa/tag_ksz.c
> > > +++ b/net/dsa/tag_ksz.c
> > > static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device
> > *dev) {
> > > struct dsa_slave_priv *p = netdev_priv(dev);
> > > + struct dsa_switch *ds = p->dp->ds;
> > > + struct ksz_device *swdev = ds->priv;
> > > struct sk_buff *nskb;
> > > + int len;
> > > int padlen;
> > > - u8 *tag;
> > >
> > > padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
> > >
> > > - if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> > > + len = swdev->dev_ops->get_tx_len(swdev);
> >
> > This is ugly. We have a clean separation between a switch driver and a
> > tag driver. Here you are mixing them together. Don't. Look at the
> > mailing lists for what Florian and I suggested to Pavel. It will also
> > solve your include file issue above.
>
> It seems to me all tag_*.c are hard-coded. They do not have access to
> the switch device and just use the bit definitions defined in the top to
> do the job.
>
> This creates a problem for all KSZ switch devices as they have different
> tail tag formats and lengths. There will be potentially 4 additional
> DSA_TAG_PROT_KSZ* just to handle them. Extra files will be added
> with about the same code.
Hi Tristram
Think about factoring out the common code into a shared functions.
> But the most important thing is how do we support the offload_fwd_mark
> bit in skb when the only place that has access to skb does not have access to
> the switch hardware?
Well, so far, nothing has set offload_fwd_mark. This is about to
change, since i need it for IGMP snooping on the brX interface, for
Marvell at least. Bit i just need to set it to true for all frames.
What use cases do you have in mind where you need information from the
switch driver to decide on its value?
> A little out-of-topic is the MAC driver may have problem receiving the frame if
> the tail tag length is too big. Most of the MAC drivers have big enough receive
> buffer or require 64-bytes multiple so there are extra room to accommodate
> the big tail tag at the end of the frame. Still some MAC drivers use exactly 1500
> MTU and some additional length and may run into this problem. Currently the
> Atmel Cadence MAC driver has this potential problem when certain conditions
> are met.
This is a know problem. I just fixed the FEC driver which also
discarded DSA tagged frames when they were bigger than the MTU.
So far, it has not been too much of an issue, because most boards with
a switch, use an Ethernet interface from the same manufacture. Marvell
Switches with a Marvell Ethernet interface. Broadcom switch with a
Broadcom Interface. Atheros switch with an Atheros interface. And
naturally, these combinations just work. But Freescale FEC and Marvell
had not been combined before, and it was broken.
So i would not be surprised if the Cadence MAC driver had an issue.
Andrew
^ permalink raw reply
* [PATCH 1/2] mac80211: Add rcu read side critical sections
From: Ville Syrjala @ 2017-09-18 19:59 UTC (permalink / raw)
To: linux-wireless
Cc: Johannes Berg, David S. Miller, netdev, Ville Syrjälä
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
I got the following lockdep warning about the rcu_dereference()s in
ieee80211_tx_h_select_key(). After tracing all callers of
ieee80211_tx_h_select_key() I discovered that ieee80211_get_buffered_bc()
and ieee80211_build_data_template() had the rcu_read_lock/unlock() but
three other places did not. So I just blindly added them and made the
read side critical section extend as far as the lifetime of 'tx' which
is where we seem to be stuffing the rcu protected pointers. No real clue
whether this is correct or not.
[ 854.573700] ../net/mac80211/tx.c:594 suspicious rcu_dereference_check() usage!
[ 854.573704]
other info that might help us debug this:
[ 854.573707]
rcu_scheduler_active = 2, debug_locks = 1
[ 854.573712] 6 locks held by kworker/u2:0/2877:
[ 854.573715] #0: ("%s"wiphy_name(local->hw.wiphy)){++++.+}, at: [<c1067f37>] process_one_work+0x127/0x580
[ 854.573742] #1: ((&sdata->work)){+.+.+.}, at: [<c1067f37>] process_one_work+0x127/0x580
[ 854.573758] #2: (&wdev->mtx){+.+.+.}, at: [<f83271c3>] ieee80211_sta_work+0x23/0x1c70 [mac80211]
[ 854.573902] #3: (&local->sta_mtx){+.+.+.}, at: [<f82c9b10>] __sta_info_flush+0x60/0x160 [mac80211]
[ 854.573947] #4: (&(&txq->axq_lock)->rlock){+.-...}, at: [<f825729c>] ath_tx_node_cleanup+0x5c/0x180 [ath9k]
[ 854.573973] #5: (&(&fq->lock)->rlock){+.-...}, at: [<f82fb064>] ieee80211_tx_dequeue+0x24/0xa80 [mac80211]
[ 854.574023]
stack backtrace:
[ 854.574028] CPU: 0 PID: 2877 Comm: kworker/u2:0 Not tainted 4.13.0-mgm-ovl+ #52
[ 854.574032] Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26 05/10/2004
[ 854.574070] Workqueue: phy0 ieee80211_iface_work [mac80211]
[ 854.574076] Call Trace:
[ 854.574086] dump_stack+0x16/0x19
[ 854.574092] lockdep_rcu_suspicious+0xcb/0xf0
[ 854.574131] ieee80211_tx_h_select_key+0x1b5/0x500 [mac80211]
[ 854.574171] ieee80211_tx_dequeue+0x283/0xa80 [mac80211]
[ 854.574181] ath_tid_dequeue+0x84/0xf0 [ath9k]
[ 854.574189] ath_tx_node_cleanup+0xb8/0x180 [ath9k]
[ 854.574199] ath9k_sta_state+0x48/0xf0 [ath9k]
[ 854.574207] ? ath9k_del_ps_key.isra.19+0x60/0x60 [ath9k]
[ 854.574240] drv_sta_state+0xaf/0x8c0 [mac80211]
[ 854.574275] __sta_info_destroy_part2+0x10b/0x140 [mac80211]
[ 854.574309] __sta_info_flush+0xd5/0x160 [mac80211]
[ 854.574349] ieee80211_set_disassoc+0xd3/0x570 [mac80211]
[ 854.574390] ieee80211_sta_connection_lost+0x30/0x60 [mac80211]
[ 854.574431] ieee80211_sta_work+0x1ff/0x1c70 [mac80211]
[ 854.574436] ? mark_held_locks+0x62/0x90
[ 854.574443] ? _raw_spin_unlock_irqrestore+0x55/0x70
[ 854.574447] ? trace_hardirqs_on_caller+0x11c/0x1a0
[ 854.574452] ? trace_hardirqs_on+0xb/0x10
[ 854.574459] ? dev_mc_net_exit+0xe/0x20
[ 854.574467] ? skb_dequeue+0x48/0x70
[ 854.574504] ieee80211_iface_work+0x2d8/0x320 [mac80211]
[ 854.574509] process_one_work+0x1d1/0x580
[ 854.574513] ? process_one_work+0x127/0x580
[ 854.574519] worker_thread+0x31/0x380
[ 854.574525] kthread+0xd9/0x110
[ 854.574529] ? process_one_work+0x580/0x580
[ 854.574534] ? kthread_create_on_node+0x30/0x30
[ 854.574540] ret_from_fork+0x19/0x24
[ 854.574548] =============================
[ 854.574551] WARNING: suspicious RCU usage
[ 854.574555] 4.13.0-mgm-ovl+ #52 Not tainted
[ 854.574558] -----------------------------
[ 854.574561] ../net/mac80211/tx.c:608 suspicious rcu_dereference_check() usage!
[ 854.574564]
other info that might help us debug this:
[ 854.574568]
rcu_scheduler_active = 2, debug_locks = 1
[ 854.574572] 6 locks held by kworker/u2:0/2877:
[ 854.574574] #0: ("%s"wiphy_name(local->hw.wiphy)){++++.+}, at: [<c1067f37>] process_one_work+0x127/0x580
[ 854.574590] #1: ((&sdata->work)){+.+.+.}, at: [<c1067f37>] process_one_work+0x127/0x580
[ 854.574606] #2: (&wdev->mtx){+.+.+.}, at: [<f83271c3>] ieee80211_sta_work+0x23/0x1c70 [mac80211]
[ 854.574657] #3: (&local->sta_mtx){+.+.+.}, at: [<f82c9b10>] __sta_info_flush+0x60/0x160 [mac80211]
[ 854.574702] #4: (&(&txq->axq_lock)->rlock){+.-...}, at: [<f825729c>] ath_tx_node_cleanup+0x5c/0x180 [ath9k]
[ 854.574721] #5: (&(&fq->lock)->rlock){+.-...}, at: [<f82fb064>] ieee80211_tx_dequeue+0x24/0xa80 [mac80211]
[ 854.574771]
stack backtrace:
[ 854.574775] CPU: 0 PID: 2877 Comm: kworker/u2:0 Not tainted 4.13.0-mgm-ovl+ #52
[ 854.574779] Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26 05/10/2004
[ 854.574814] Workqueue: phy0 ieee80211_iface_work [mac80211]
[ 854.574821] Call Trace:
[ 854.574825] dump_stack+0x16/0x19
[ 854.574830] lockdep_rcu_suspicious+0xcb/0xf0
[ 854.574869] ieee80211_tx_h_select_key+0x44e/0x500 [mac80211]
[ 854.574908] ieee80211_tx_dequeue+0x283/0xa80 [mac80211]
[ 854.574919] ath_tid_dequeue+0x84/0xf0 [ath9k]
[ 854.574927] ath_tx_node_cleanup+0xb8/0x180 [ath9k]
[ 854.574936] ath9k_sta_state+0x48/0xf0 [ath9k]
[ 854.574945] ? ath9k_del_ps_key.isra.19+0x60/0x60 [ath9k]
[ 854.574978] drv_sta_state+0xaf/0x8c0 [mac80211]
[ 854.575012] __sta_info_destroy_part2+0x10b/0x140 [mac80211]
[ 854.575046] __sta_info_flush+0xd5/0x160 [mac80211]
[ 854.575087] ieee80211_set_disassoc+0xd3/0x570 [mac80211]
[ 854.575127] ieee80211_sta_connection_lost+0x30/0x60 [mac80211]
[ 854.575168] ieee80211_sta_work+0x1ff/0x1c70 [mac80211]
[ 854.575173] ? mark_held_locks+0x62/0x90
[ 854.575178] ? _raw_spin_unlock_irqrestore+0x55/0x70
[ 854.575182] ? trace_hardirqs_on_caller+0x11c/0x1a0
[ 854.575187] ? trace_hardirqs_on+0xb/0x10
[ 854.575192] ? dev_mc_net_exit+0xe/0x20
[ 854.575197] ? skb_dequeue+0x48/0x70
[ 854.575233] ieee80211_iface_work+0x2d8/0x320 [mac80211]
[ 854.575238] process_one_work+0x1d1/0x580
[ 854.575243] ? process_one_work+0x127/0x580
[ 854.575248] worker_thread+0x31/0x380
[ 854.575253] kthread+0xd9/0x110
[ 854.575257] ? process_one_work+0x580/0x580
[ 854.575262] ? kthread_create_on_node+0x30/0x30
[ 854.575267] ret_from_fork+0x19/0x24
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
net/mac80211/tx.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94826680cf2b..073022ee2462 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1770,15 +1770,21 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
struct ieee80211_tx_data tx;
struct sk_buff *skb2;
- if (ieee80211_tx_prepare(sdata, &tx, NULL, skb) == TX_DROP)
+ rcu_read_lock();
+
+ if (ieee80211_tx_prepare(sdata, &tx, NULL, skb) == TX_DROP) {
+ rcu_read_unlock();
return false;
+ }
info->band = band;
info->control.vif = vif;
info->hw_queue = vif->hw_queue[skb_get_queue_mapping(skb)];
- if (invoke_tx_handlers(&tx))
+ if (invoke_tx_handlers(&tx)) {
+ rcu_read_unlock();
return false;
+ }
if (sta) {
if (tx.sta)
@@ -1792,9 +1798,12 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
if (WARN_ON(skb2 != skb || !skb_queue_empty(&tx.skbs))) {
ieee80211_free_txskb(hw, skb2);
ieee80211_purge_tx_queue(hw, &tx.skbs);
+ rcu_read_unlock();
return false;
}
+ rcu_read_unlock();
+
return true;
}
EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
@@ -1818,14 +1827,18 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata,
return true;
}
+ rcu_read_lock();
+
/* initialises tx */
led_len = skb->len;
res_prepare = ieee80211_tx_prepare(sdata, &tx, sta, skb);
if (unlikely(res_prepare == TX_DROP)) {
ieee80211_free_txskb(&local->hw, skb);
+ rcu_read_unlock();
return true;
} else if (unlikely(res_prepare == TX_QUEUED)) {
+ rcu_read_unlock();
return true;
}
@@ -1835,16 +1848,22 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata,
info->hw_queue =
sdata->vif.hw_queue[skb_get_queue_mapping(skb)];
- if (invoke_tx_handlers_early(&tx))
+ if (invoke_tx_handlers_early(&tx)) {
+ rcu_read_unlock();
return false;
+ }
- if (ieee80211_queue_skb(local, sdata, tx.sta, tx.skb))
+ if (ieee80211_queue_skb(local, sdata, tx.sta, tx.skb)) {
+ rcu_read_unlock();
return true;
+ }
if (!invoke_tx_handlers_late(&tx))
result = __ieee80211_tx(local, &tx.skbs, led_len,
tx.sta, txpending);
+ rcu_read_unlock();
+
return result;
}
@@ -3411,6 +3430,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
ieee80211_tx_result r;
struct ieee80211_vif *vif;
+ rcu_read_lock();
+
spin_lock_bh(&fq->lock);
if (test_bit(IEEE80211_TXQ_STOP, &txqi->flags))
@@ -3513,6 +3534,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
out:
spin_unlock_bh(&fq->lock);
+ rcu_read_unlock();
+
return skb;
}
EXPORT_SYMBOL(ieee80211_tx_dequeue);
--
2.13.5
^ permalink raw reply related
* [PATCH 2/2] ath9k: Avoid a potential deadlock
From: Ville Syrjala @ 2017-09-18 19:59 UTC (permalink / raw)
To: linux-wireless
Cc: QCA ath9k Development, Kalle Valo, netdev,
Ville Syrjälä
In-Reply-To: <20170918195919.15860-1-ville.syrjala@linux.intel.com>
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Lockdep warns us that sc_pm_lock and cc_lock can cause a deadlock when
cc_lock is acquired by itself with interrupts enabled. Disable irqs
whenever taking cc_lock to avoid this.
[ 19.094524] kworker/u2:0/5 just changed the state of lock:
[ 19.094578] (&(&sc->sc_pm_lock)->rlock){-.-...}, at: [<f836c00e>] ath_isr+0x15e/0x200 [ath9k]
[ 19.094674] but this lock took another, HARDIRQ-unsafe lock in the past:
[ 19.094731] (&(&common->cc_lock)->rlock){+.-...}
[ 19.094741]
and interrupts could create inverse lock ordering between them.
[ 19.094866]
other info that might help us debug this:
[ 19.094926] Possible interrupt unsafe locking scenario:
[ 19.094985] CPU0 CPU1
[ 19.095036] ---- ----
[ 19.095086] lock(&(&common->cc_lock)->rlock);
[ 19.095197] local_irq_disable();
[ 19.095305] lock(&(&sc->sc_pm_lock)->rlock);
[ 19.095423] lock(&(&common->cc_lock)->rlock);
[ 19.095539] <Interrupt>
[ 19.095636] lock(&(&sc->sc_pm_lock)->rlock);
[ 19.095745]
*** DEADLOCK ***
[ 19.095965] 3 locks held by kworker/u2:0/5:
[ 19.096067] #0: ("%s"wiphy_name(local->hw.wiphy)){.+.+.+}, at: [<c1067f37>] process_one_work+0x127/0x580
[ 19.096260] #1: ((&local->dynamic_ps_enable_work)){+.+...}, at: [<c1067f37>] process_one_work+0x127/0x580
[ 19.096447] #2: (&sc->mutex){+.+...}, at: [<f836b8b0>] ath9k_config+0x30/0x1d0 [ath9k]
[ 19.096639]
the shortest dependencies between 2nd lock and 1st lock:
[ 19.096813] -> (&(&common->cc_lock)->rlock){+.-...} ops: 38 {
[ 19.096816] HARDIRQ-ON-W at:
[ 19.096816] __lock_acquire+0x57e/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock_bh+0x3f/0x50
[ 19.096816] ath_chanctx_set_channel+0xb6/0x2c0 [ath9k]
[ 19.096816] ath9k_config+0xa8/0x1d0 [ath9k]
[ 19.096816] ieee80211_hw_config+0xa8/0x5f0 [mac80211]
[ 19.096816] ieee80211_do_open+0x67a/0x920 [mac80211]
[ 19.096816] ieee80211_open+0x41/0x50 [mac80211]
[ 19.096816] __dev_open+0xab/0x140
[ 19.096816] __dev_change_flags+0x89/0x150
[ 19.096816] dev_change_flags+0x28/0x60
[ 19.096816] do_setlink+0x290/0x890
[ 19.096816] rtnl_newlink+0x7cf/0x8e0
[ 19.096816] rtnetlink_rcv_msg+0xbf/0x1f0
[ 19.096816] netlink_rcv_skb+0xb9/0xe0
[ 19.096816] rtnetlink_rcv+0x1e/0x30
[ 19.096816] netlink_unicast+0x13a/0x2c0
[ 19.096816] netlink_sendmsg+0x290/0x380
[ 19.096816] ___sys_sendmsg+0x1e2/0x280
[ 19.096816] __sys_sendmsg+0x3f/0x80
[ 19.096816] SyS_socketcall+0x58c/0x6b0
[ 19.096816] do_fast_syscall_32+0x96/0x1d0
[ 19.096816] entry_SYSENTER_32+0x4c/0x7b
[ 19.096816] IN-SOFTIRQ-W at:
[ 19.096816] __lock_acquire+0x55a/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock+0x3c/0x50
[ 19.096816] ath_ps_full_sleep+0x24/0x70 [ath9k]
[ 19.096816] call_timer_fn+0xa4/0x300
[ 19.096816] run_timer_softirq+0x1b1/0x560
[ 19.096816] __do_softirq+0xb0/0x430
[ 19.096816] do_softirq_own_stack+0x33/0x40
[ 19.096816] irq_exit+0xad/0xc0
[ 19.096816] smp_apic_timer_interrupt+0x31/0x40
[ 19.096816] apic_timer_interrupt+0x37/0x3c
[ 19.096816] wp_page_copy+0xb8/0x580
[ 19.096816] do_wp_page+0x64/0x420
[ 19.096816] handle_mm_fault+0x430/0x990
[ 19.096816] __do_page_fault+0x18b/0x430
[ 19.096816] do_page_fault+0xb/0x10
[ 19.096816] common_exception+0x62/0x6a
[ 19.096816] INITIAL USE at:
[ 19.096816] __lock_acquire+0x204/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock_bh+0x3f/0x50
[ 19.096816] ath_chanctx_set_channel+0xb6/0x2c0 [ath9k]
[ 19.096816] ath9k_config+0xa8/0x1d0 [ath9k]
[ 19.096816] ieee80211_hw_config+0xa8/0x5f0 [mac80211]
[ 19.096816] ieee80211_do_open+0x67a/0x920 [mac80211]
[ 19.096816] ieee80211_open+0x41/0x50 [mac80211]
[ 19.096816] __dev_open+0xab/0x140
[ 19.096816] __dev_change_flags+0x89/0x150
[ 19.096816] dev_change_flags+0x28/0x60
[ 19.096816] do_setlink+0x290/0x890
[ 19.096816] rtnl_newlink+0x7cf/0x8e0
[ 19.096816] rtnetlink_rcv_msg+0xbf/0x1f0
[ 19.096816] netlink_rcv_skb+0xb9/0xe0
[ 19.096816] rtnetlink_rcv+0x1e/0x30
[ 19.096816] netlink_unicast+0x13a/0x2c0
[ 19.096816] netlink_sendmsg+0x290/0x380
[ 19.096816] ___sys_sendmsg+0x1e2/0x280
[ 19.096816] __sys_sendmsg+0x3f/0x80
[ 19.096816] SyS_socketcall+0x58c/0x6b0
[ 19.096816] do_fast_syscall_32+0x96/0x1d0
[ 19.096816] entry_SYSENTER_32+0x4c/0x7b
[ 19.096816] }
[ 19.096816] ... key at: [<f837b694>] __key.61991+0x0/0xffffc96c [ath9k]
[ 19.096816] ... acquired at:
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock+0x3c/0x50
[ 19.096816] ath9k_ps_wakeup+0x85/0xe0 [ath9k]
[ 19.096816] ath9k_bss_info_changed+0x2a/0x1b0 [ath9k]
[ 19.096816] ieee80211_bss_info_change_notify+0xf3/0x360 [mac80211]
[ 19.096816] ieee80211_recalc_txpower+0x33/0x40 [mac80211]
[ 19.096816] ieee80211_set_tx_power+0x45/0x1d0 [mac80211]
[ 19.096816] cfg80211_wext_siwtxpower+0xd3/0x350 [cfg80211]
[ 19.096816] ioctl_standard_call+0x4e/0x400
[ 19.096816] wext_handle_ioctl+0xf4/0x190
[ 19.096816] dev_ioctl+0xb7/0x630
[ 19.096816] sock_ioctl+0x13e/0x2d0
[ 19.096816] do_vfs_ioctl+0x84/0x750
[ 19.096816] SyS_ioctl+0x34/0x60
[ 19.096816] do_fast_syscall_32+0x96/0x1d0
[ 19.096816] entry_SYSENTER_32+0x4c/0x7b
[ 19.096816] -> (&(&sc->sc_pm_lock)->rlock){-.-...} ops: 597 {
[ 19.096816] IN-HARDIRQ-W at:
[ 19.096816] __lock_acquire+0x6ae/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock_irqsave+0x45/0x60
[ 19.096816] ath_isr+0x15e/0x200 [ath9k]
[ 19.096816] __handle_irq_event_percpu+0x44/0x340
[ 19.096816] handle_irq_event_percpu+0x1d/0x50
[ 19.096816] handle_irq_event+0x32/0x60
[ 19.096816] handle_level_irq+0x81/0x100
[ 19.096816] handle_irq+0x9c/0xd0
[ 19.096816] do_IRQ+0x5c/0x120
[ 19.096816] common_interrupt+0x36/0x3c
[ 19.096816] _raw_spin_unlock_irqrestore+0x57/0x70
[ 19.096816] ath9k_config+0x16a/0x1d0 [ath9k]
[ 19.096816] ieee80211_hw_config+0xa8/0x5f0 [mac80211]
[ 19.096816] ieee80211_dynamic_ps_enable_work+0x1c3/0x680 [mac80211]
[ 19.096816] process_one_work+0x1d1/0x580
[ 19.096816] worker_thread+0x31/0x380
[ 19.096816] kthread+0xd9/0x110
[ 19.096816] ret_from_fork+0x19/0x24
[ 19.096816] IN-SOFTIRQ-W at:
[ 19.096816] __lock_acquire+0x55a/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock_irqsave+0x45/0x60
[ 19.096816] ath9k_ps_wakeup+0x24/0xe0 [ath9k]
[ 19.096816] ath9k_tasklet+0x42/0x260 [ath9k]
[ 19.096816] tasklet_action+0x196/0x1e0
[ 19.096816] __do_softirq+0xb0/0x430
[ 19.096816] do_softirq_own_stack+0x33/0x40
[ 19.096816] irq_exit+0xad/0xc0
[ 19.096816] do_IRQ+0x65/0x120
[ 19.096816] common_interrupt+0x36/0x3c
[ 19.096816] get_page_from_freelist+0x20a/0x970
[ 19.096816] __alloc_pages_nodemask+0xca/0xed0
[ 19.096816] __get_free_pages+0x14/0x30
[ 19.096816] pgd_alloc+0x1d/0x160
[ 19.096816] mm_init.isra.47+0x13a/0x1b0
[ 19.096816] copy_process.part.54+0xb55/0x1700
[ 19.096816] _do_fork+0xd4/0x6a0
[ 19.096816] SyS_clone+0x27/0x30
[ 19.096816] do_fast_syscall_32+0x96/0x1d0
[ 19.096816] entry_SYSENTER_32+0x4c/0x7b
[ 19.096816] INITIAL USE at:
[ 19.096816] __lock_acquire+0x204/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock_irqsave+0x45/0x60
[ 19.096816] ath9k_ps_wakeup+0x24/0xe0 [ath9k]
[ 19.096816] ath9k_start+0x29/0x1f0 [ath9k]
[ 19.096816] drv_start+0x71/0x270 [mac80211]
[ 19.096816] ieee80211_do_open+0x31f/0x920 [mac80211]
[ 19.096816] ieee80211_open+0x41/0x50 [mac80211]
[ 19.096816] __dev_open+0xab/0x140
[ 19.096816] __dev_change_flags+0x89/0x150
[ 19.096816] dev_change_flags+0x28/0x60
[ 19.096816] do_setlink+0x290/0x890
[ 19.096816] rtnl_newlink+0x7cf/0x8e0
[ 19.096816] rtnetlink_rcv_msg+0xbf/0x1f0
[ 19.096816] netlink_rcv_skb+0xb9/0xe0
[ 19.096816] rtnetlink_rcv+0x1e/0x30
[ 19.096816] netlink_unicast+0x13a/0x2c0
[ 19.096816] netlink_sendmsg+0x290/0x380
[ 19.096816] ___sys_sendmsg+0x1e2/0x280
[ 19.096816] __sys_sendmsg+0x3f/0x80
[ 19.096816] SyS_socketcall+0x58c/0x6b0
[ 19.096816] do_fast_syscall_32+0x96/0x1d0
[ 19.096816] entry_SYSENTER_32+0x4c/0x7b
[ 19.096816] }
[ 19.096816] ... key at: [<f837b67c>] __key.61994+0x0/0xffffc984 [ath9k]
[ 19.096816] ... acquired at:
[ 19.096816] check_usage_forwards+0x118/0x120
[ 19.096816] mark_lock+0x2e4/0x590
[ 19.096816] __lock_acquire+0x6ae/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] _raw_spin_lock_irqsave+0x45/0x60
[ 19.096816] ath_isr+0x15e/0x200 [ath9k]
[ 19.096816] __handle_irq_event_percpu+0x44/0x340
[ 19.096816] handle_irq_event_percpu+0x1d/0x50
[ 19.096816] handle_irq_event+0x32/0x60
[ 19.096816] handle_level_irq+0x81/0x100
[ 19.096816] handle_irq+0x9c/0xd0
[ 19.096816] do_IRQ+0x5c/0x120
[ 19.096816] common_interrupt+0x36/0x3c
[ 19.096816] _raw_spin_unlock_irqrestore+0x57/0x70
[ 19.096816] ath9k_config+0x16a/0x1d0 [ath9k]
[ 19.096816] ieee80211_hw_config+0xa8/0x5f0 [mac80211]
[ 19.096816] ieee80211_dynamic_ps_enable_work+0x1c3/0x680 [mac80211]
[ 19.096816] process_one_work+0x1d1/0x580
[ 19.096816] worker_thread+0x31/0x380
[ 19.096816] kthread+0xd9/0x110
[ 19.096816] ret_from_fork+0x19/0x24
[ 19.096816]
stack backtrace:
[ 19.096816] CPU: 0 PID: 5 Comm: kworker/u2:0 Not tainted 4.13.0-mgm-ovl+ #51
[ 19.096816] Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26 05/10/2004
[ 19.096816] Workqueue: phy0 ieee80211_dynamic_ps_enable_work [mac80211]
[ 19.096816] Call Trace:
[ 19.096816] <IRQ>
[ 19.096816] dump_stack+0x16/0x19
[ 19.096816] print_irq_inversion_bug.part.37+0x16c/0x179
[ 19.096816] check_usage_forwards+0x118/0x120
[ 19.096816] ? ret_from_fork+0x19/0x24
[ 19.096816] ? print_shortest_lock_dependencies+0x1a0/0x1a0
[ 19.096816] mark_lock+0x2e4/0x590
[ 19.096816] ? print_shortest_lock_dependencies+0x1a0/0x1a0
[ 19.096816] __lock_acquire+0x6ae/0x1260
[ 19.096816] lock_acquire+0xb1/0x1c0
[ 19.096816] ? ath_isr+0x15e/0x200 [ath9k]
[ 19.096816] _raw_spin_lock_irqsave+0x45/0x60
[ 19.096816] ? ath_isr+0x15e/0x200 [ath9k]
[ 19.096816] ath_isr+0x15e/0x200 [ath9k]
[ 19.096816] __handle_irq_event_percpu+0x44/0x340
[ 19.096816] handle_irq_event_percpu+0x1d/0x50
[ 19.096816] handle_irq_event+0x32/0x60
[ 19.096816] ? handle_nested_irq+0x100/0x100
[ 19.096816] handle_level_irq+0x81/0x100
[ 19.096816] handle_irq+0x9c/0xd0
[ 19.096816] </IRQ>
[ 19.096816] do_IRQ+0x5c/0x120
[ 19.096816] common_interrupt+0x36/0x3c
[ 19.096816] EIP: _raw_spin_unlock_irqrestore+0x57/0x70
[ 19.096816] EFLAGS: 00000286 CPU: 0
[ 19.096816] EAX: f60a3600 EBX: 00000286 ECX: 00000006 EDX: 00000001
[ 19.096816] ESI: f46c9e68 EDI: f46c8620 EBP: f60b5e8c ESP: f60b5e84
[ 19.096816] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
[ 19.096816] ath9k_config+0x16a/0x1d0 [ath9k]
[ 19.096816] ieee80211_hw_config+0xa8/0x5f0 [mac80211]
[ 19.096816] ? ieee80211_hw_config+0x1db/0x5f0 [mac80211]
[ 19.096816] ieee80211_dynamic_ps_enable_work+0x1c3/0x680 [mac80211]
[ 19.096816] ? process_one_work+0x127/0x580
[ 19.096816] ? process_one_work+0x127/0x580
[ 19.096816] process_one_work+0x1d1/0x580
[ 19.096816] ? process_one_work+0x127/0x580
[ 19.096816] worker_thread+0x31/0x380
[ 19.096816] kthread+0xd9/0x110
[ 19.096816] ? process_one_work+0x580/0x580
[ 19.096816] ? kthread_create_on_node+0x30/0x30
[ 19.096816] ret_from_fork+0x19/0x24
Cc: QCA ath9k Development <ath9k-devel@qca.qualcomm.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/net/wireless/ath/ath9k/channel.c | 5 +++--
drivers/net/wireless/ath/ath9k/link.c | 4 ++--
drivers/net/wireless/ath/ath9k/main.c | 16 +++++++++-------
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
index f0439f2d566b..fad020aa222e 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -29,6 +29,7 @@ static int ath_set_channel(struct ath_softc *sc)
struct cfg80211_chan_def *chandef = &sc->cur_chan->chandef;
struct ieee80211_channel *chan = chandef->chan;
int pos = chan->hw_value;
+ unsigned long flags;
int old_pos = -1;
int r;
@@ -42,9 +43,9 @@ static int ath_set_channel(struct ath_softc *sc)
chan->center_freq, chandef->width);
/* update survey stats for the old channel before switching */
- spin_lock_bh(&common->cc_lock);
+ spin_lock_irqsave(&common->cc_lock, flags);
ath_update_survey_stats(sc);
- spin_unlock_bh(&common->cc_lock);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
ath9k_cmn_get_channel(hw, ah, chandef);
diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c
index 27c50562dc47..3f4f01c829f0 100644
--- a/drivers/net/wireless/ath/ath9k/link.c
+++ b/drivers/net/wireless/ath/ath9k/link.c
@@ -367,10 +367,10 @@ void ath_ani_calibrate(unsigned long data)
/* Call ANI routine if necessary */
if (aniflag) {
- spin_lock(&common->cc_lock);
+ spin_lock_irqsave(&common->cc_lock, flags);
ath9k_hw_ani_monitor(ah, ah->curchan);
ath_update_survey_stats(sc);
- spin_unlock(&common->cc_lock);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
}
/* Perform calibration if necessary */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 8b4ac7f0a09b..ebb7e0f63279 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -97,11 +97,12 @@ void ath_ps_full_sleep(unsigned long data)
{
struct ath_softc *sc = (struct ath_softc *) data;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ unsigned long flags;
bool reset;
- spin_lock(&common->cc_lock);
+ spin_lock_irqsave(&common->cc_lock, flags);
ath_hw_cycle_counters_update(common);
- spin_unlock(&common->cc_lock);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
ath9k_hw_setrxabort(sc->sc_ah, 1);
ath9k_hw_stopdmarecv(sc->sc_ah, &reset);
@@ -394,10 +395,10 @@ void ath9k_tasklet(unsigned long data)
if ((ah->config.hw_hang_checks & HW_BB_WATCHDOG) &&
(status & ATH9K_INT_BB_WATCHDOG)) {
- spin_lock(&common->cc_lock);
+ spin_lock_irqsave(&common->cc_lock, flags);
ath_hw_cycle_counters_update(common);
ar9003_hw_bb_watchdog_dbg_info(ah);
- spin_unlock(&common->cc_lock);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
if (ar9003_hw_bb_watchdog_check(ah)) {
type = RESET_TYPE_BB_WATCHDOG;
@@ -1955,12 +1956,13 @@ static int ath9k_get_survey(struct ieee80211_hw *hw, int idx,
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
struct ieee80211_supported_band *sband;
struct ieee80211_channel *chan;
+ unsigned long flags;
int pos;
if (IS_ENABLED(CONFIG_ATH9K_TX99))
return -EOPNOTSUPP;
- spin_lock_bh(&common->cc_lock);
+ spin_lock_irqsave(&common->cc_lock, flags);
if (idx == 0)
ath_update_survey_stats(sc);
@@ -1974,7 +1976,7 @@ static int ath9k_get_survey(struct ieee80211_hw *hw, int idx,
sband = hw->wiphy->bands[NL80211_BAND_5GHZ];
if (!sband || idx >= sband->n_channels) {
- spin_unlock_bh(&common->cc_lock);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
return -ENOENT;
}
@@ -1982,7 +1984,7 @@ static int ath9k_get_survey(struct ieee80211_hw *hw, int idx,
pos = chan->hw_value;
memcpy(survey, &sc->survey[pos], sizeof(*survey));
survey->channel = chan;
- spin_unlock_bh(&common->cc_lock);
+ spin_unlock_irqrestore(&common->cc_lock, flags);
return 0;
}
--
2.13.5
^ permalink raw reply related
* [PATCH net] 8139too: revisit napi_complete_done() usage
From: Eric Dumazet @ 2017-09-18 20:03 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Eric Dumazet, David S. Miller, netdev
In-Reply-To: <20170918194630.GH4914@intel.com>
From: Eric Dumazet <edumazet@google.com>
It seems we have to be more careful in napi_complete_done()
use. This patch is not a revert, as it seems we can
avoid bug that Ville reported by moving the napi_complete_done()
test in the spinlock section.
Many thanks to Ville for detective work and all tests.
Fixes: 617f01211baf ("8139too: use napi_complete_done()")
Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/net/ethernet/realtek/8139too.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 89631753e79962d91456d93b71929af768917da1..cd2dbec331dd796f5296cd378561b3443f231673 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -2135,11 +2135,12 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
if (likely(RTL_R16(IntrStatus) & RxAckBits))
work_done += rtl8139_rx(dev, tp, budget);
- if (work_done < budget && napi_complete_done(napi, work_done)) {
+ if (work_done < budget) {
unsigned long flags;
spin_lock_irqsave(&tp->lock, flags);
- RTL_W16_F(IntrMask, rtl8139_intr_mask);
+ if (napi_complete_done(napi, work_done))
+ RTL_W16_F(IntrMask, rtl8139_intr_mask);
spin_unlock_irqrestore(&tp->lock, flags);
}
spin_unlock(&tp->rx_lock);
^ permalink raw reply related
* Re: [PATCH 1/2] mac80211: Add rcu read side critical sections
From: Johannes Berg @ 2017-09-18 20:11 UTC (permalink / raw)
To: Ville Syrjala, linux-wireless; +Cc: David S. Miller, netdev
In-Reply-To: <20170918195919.15860-1-ville.syrjala@linux.intel.com>
> I got the following lockdep warning about the rcu_dereference()s in
> ieee80211_tx_h_select_key(). After tracing all callers of
> ieee80211_tx_h_select_key() I discovered that
> ieee80211_get_buffered_bc()
> and ieee80211_build_data_template() had the rcu_read_lock/unlock()
> but
> three other places did not. So I just blindly added them and made the
> read side critical section extend as far as the lifetime of 'tx'
> which
> is where we seem to be stuffing the rcu protected pointers. No real
> clue whether this is correct or not.
Heh.
I think we should do it in ieee80211_tx_dequeue(), if not even in the
driver (and document that it's required)
johannes
> @@ -3411,6 +3430,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct
> ieee80211_hw *hw,
> ieee80211_tx_result r;
> struct ieee80211_vif *vif;
>
> + rcu_read_lock();
> +
> spin_lock_bh(&fq->lock);
>
> if (test_bit(IEEE80211_TXQ_STOP, &txqi->flags))
> @@ -3513,6 +3534,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct
> ieee80211_hw *hw,
> out:
> spin_unlock_bh(&fq->lock);
>
> + rcu_read_unlock();
>
i.e. this in itself should be sufficient, though you should probably
reorder and acquire the spinlock first since that might spin, and you
want to keep the RCU section minimal (it's trivial here, after all)
johannes
^ permalink raw reply
* Re: [PATCH 3/3] selftests: silence test output by default
From: Josef Bacik @ 2017-09-18 20:19 UTC (permalink / raw)
To: Shuah Khan
Cc: Josef Bacik, Shuah Khan, Josef Bacik, David S. Miller,
linux-kernel, linux-kselftest, netdev
In-Reply-To: <1f756821-2737-8982-90d6-b4933e97a3ef@osg.samsung.com>
On Mon, Sep 18, 2017 at 01:48:31PM -0600, Shuah Khan wrote:
> On 09/18/2017 12:24 PM, Josef Bacik wrote:
> > On Mon, Sep 18, 2017 at 12:13:40PM -0600, Shuah Khan wrote:
> >> On 09/18/2017 11:52 AM, Josef Bacik wrote:
> >>> On Mon, Sep 18, 2017 at 11:46:18AM -0600, Shuah Khan wrote:
> >>>> On 09/18/2017 11:37 AM, josef@toxicpanda.com wrote:
> >>>>> From: Josef Bacik <jbacik@fb.com>
> >>>>>
> >>>>> Some of the networking tests are very noisy and make it impossible to
> >>>>> see if we actually passed the tests as they run. Default to suppressing
> >>>>> the output from any tests run in order to make it easier to track what
> >>>>> failed.
> >>>>>
> >>>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
> >>>>> --
> >>>>
> >>>> This change suppresses pass/fail wrapper output for all tests, not just the
> >>>> networking tests.
> >>>>
> >>>> Could you please send me before and after results for what you are trying
> >>>> to fix.
> >>>>
> >>>
> >>> Yeah I wanted to suppress extraneous output from everybody, I just happened to
> >>> notice it because I was testing net. The default thing already spits out what
> >>> it's running and pass/fail, there's no need to include all of the random output
> >>> unless the user wants to go and run the test manually. As it is now it's
> >>> _impossible_ to tell what ran and what passed/failed because of all the random
> >>> output.
> >>
> >> Unfortunately kselftests have lots of users that want different things. A recent
> >> request is to use TAP13 format for output for external parsers to be able to parse.
> >> That is what this change to add TAP13 header does.
> >>
> >> The output you are seeing is the TAP 13 format to indicate the test has passed.
> >>
> >> The right fix would be to suppress the Pass/Fail from the individual shell script
> >> and have the shell script exit with error code. kselftest lib.mk will handle the
> >> error code and print out pass/fail like it is doing now.
> >>
> >> Using the common logic will help avoid duplicate code in tests/test scripts and
> >> also makes the pass/fail messages consistent.
> >>
> >> In the following output the individual test output can be eliminated since lib.mk
> >> run_tests does that for you. In addition, you will also get a count of tests at
> >> the end of the run of all tests in a test directory.
> >>
> >> TAP version 13
> >> selftests: run_netsocktests
> >> ========================================
> >> --------------------
> >> running socket test
> >> --------------------
> >> [PASS]
> >> ok 1..1 selftests: run_netsocktests [PASS]
> >> selftests: run_afpackettests
> >> ========================================
> >> must be run as root
> >> ok 1..2 selftests: run_afpackettests [PASS]
> >> selftests: test_bpf.sh
> >> ========================================
> >> test_bpf: [FAIL]
> >> not ok 1..3 selftests: test_bpf.sh [FAIL]
> >> selftests: netdevice.sh
> >> ========================================
> >> SKIP: Need root privileges
> >> ok 1..4 selftests: netdevice.sh [PASS]
> >>
> >> If you eliminate that you will just see the common lib.mk results.
> >>
> >> TAP version 13
> >> selftests: run_netsocktests
> >> ========================================
> >> ok 1..1 selftests: run_netsocktests [PASS]
> >> selftests: run_afpackettests
> >> ========================================
> >> must be run as root
> >> ok 1..2 selftests: run_afpackettests [PASS]
> >> ========================================
> >> selftests: test_bpf.sh
> >> ========================================
> >> not ok 1..3 selftests: test_bpf.sh [FAIL]
> >> selftests: netdevice.sh
> >> ========================================
> >> SKIP: Need root privileges
> >> ok 1..4 selftests: netdevice.sh [PASS]
> >>
> >>
> >> If you would like to fix the duplicate output, please send me patches
> >> to remove pass/fail output strings from tests instead. It is on my
> >> todo to do that this release.
> >>
> >
> > I'm confused, this is exactly what my patch does, it strips all of the
> > extraneous output and leaves only the TAP13 output. Here is the output without
> > my suppression patch
> >
> > https://da.gd/pup0
> >
> > and here is the output with my suppression patch
> >
> > https://da.gd/3olKj
> >
> > Unless I'm missing something subtle it appears to be exactly the output you
> > want, without the random crap from the other tests. The only thing I'm
> > redirecting is the output of the _test_ itself, $$BASENAME_TEST from what I can
> > tell is the actual test we're running, not the wrapper, so everything is as it
> > should be. Thanks,
> >
> > Josef
> >
>
> Yes you are right. Yes I see that you are redirecting all
> output from the test with (./$$BASENAME_TEST > /dev/null 2>&1)
> My bad.
>
> cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
>
> However, it also suppresses important error messages from individual tests
> like in the case of rtnetlink.sh from your before and after output.
>
> User knows the test failed, but doesn't know why. One way to solve
> the problem is creating a temp file with the output for reference.
> Something like:
>
> (./$$BASENAME_TEST > /tmp/kselftest.out 2>&1
>
>
So I didn't do a global file because we'd have to append to it to make sure we
didn't lose any history. If this doesn't bother you I can do that instead, and
then just remove the file whenever we start running things. Let me know if
that's what you would prefer. Thanks,
Josef
^ permalink raw reply
* RACK not getting disabled
From: hiren panchasara @ 2017-09-18 20:14 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
Hi all, I am trying to disable rack to see 3dupacks in action during
loss-detection but based on the pcap, I see that it's still trigger
loss-recovery on the first SACK (as if RACK is still enabled/active).
Here is what I did to disable rack:
net.ipv4.tcp_recovery = 0
I've also disabled metrics:
net.ipv4.tcp_no_metrics_save = 1
And also flushed existing entries with 'ip tcp_metrics flush' just to be
on a safer side.
Not really relevant here but I've also switched to reno.
I am on: 4.10.0-33-generic
pcap: https://transfer.sh/mfoiN/reno_no_rack.pcap
What am I missing? I can provide any additional info.
Cheers,
Hiren
[-- Attachment #2: Type: application/pgp-signature, Size: 603 bytes --]
^ permalink raw reply
* RE: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Tristram.Ha @ 2017-09-18 20:27 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907223625.GW11248@lunn.ch>
> > +/**
> > + * Some counters do not need to be read too often because they are less
> likely
> > + * to increase much.
> > + */
>
> What does comment mean? Are you caching statistics, and updating
> different values at different rates?
>
There are 34 counters. In normal case using generic bus I/O or PCI to read them
is very quick, but the switch is mostly accessed using SPI, or even I2C. As the SPI
access is very slow and cannot run in interrupt context I keep worrying reading
the MIB counters in a loop for 5 or more ports will prevent other critical hardware
access from executing soon enough. These accesses can be getting 1588 PTP
timestamps and opening/closing ports. (RSTP Conformance Test sends test traffic
to port supposed to be closed/opened after receiving specific RSTP BPDU.)
> > +static const struct {
> > + int interval;
> > + char string[ETH_GSTRING_LEN];
> > +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> > + { 1, "rx_hi" },
> > + { 4, "rx_undersize" },
> > + { 4, "rx_fragments" },
> > + { 4, "rx_oversize" },
> > + { 4, "rx_jabbers" },
> > + { 4, "rx_symbol_err" },
> > + { 4, "rx_crc_err" },
> > + { 4, "rx_align_err" },
> > + { 4, "rx_mac_ctrl" },
> > + { 1, "rx_pause" },
> > + { 1, "rx_bcast" },
> > + { 1, "rx_mcast" },
> > + { 1, "rx_ucast" },
> > + { 2, "rx_64_or_less" },
> > + { 2, "rx_65_127" },
> > + { 2, "rx_128_255" },
> > + { 2, "rx_256_511" },
> > + { 2, "rx_512_1023" },
> > + { 2, "rx_1024_1522" },
> > + { 2, "rx_1523_2000" },
> > + { 2, "rx_2001" },
> > + { 1, "tx_hi" },
> > + { 4, "tx_late_col" },
> > + { 1, "tx_pause" },
> > + { 1, "tx_bcast" },
> > + { 1, "tx_mcast" },
> > + { 1, "tx_ucast" },
> > + { 4, "tx_deferred" },
> > + { 4, "tx_total_col" },
> > + { 4, "tx_exc_col" },
> > + { 4, "tx_single_col" },
> > + { 4, "tx_mult_col" },
> > + { 1, "rx_total" },
> > + { 1, "tx_total" },
> > + { 2, "rx_discards" },
> > + { 2, "tx_discards" },
> > +};
> > +{
> > + u32 data;
> > + u16 ctrl_addr;
> > + u8 check;
> > + int timeout;
> > +
> > + ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
> > +
> > + ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
> > + mutex_lock(&dev->stats_mutex);
> > + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> > +
> > + for (timeout = 1; timeout > 0; timeout--) {
>
> A rather short timeount!
>
> > + ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> > +
> > + if (check & MIB_COUNTER_VALID) {
> > + ksz_read32(dev, REG_IND_DATA_LO, &data);
> > + if (check & MIB_COUNTER_OVERFLOW)
> > + *cnt += MIB_COUNTER_VALUE + 1;
> > + *cnt += data & MIB_COUNTER_VALUE;
> > + break;
> > + }
>
> Should there be a dev_err() here about timing out?
>
The timeout value should be at least 2. Hardware datasheet says the
valid bit should be checked, but in my experience it is always there.
Maybe a very slow SPI access speed can make that happen, but it is
pointless to run in slow speed if the system can run in the fastest speed
possible.
> > +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> > +{
> > + struct ksz_port *port;
> > + u8 ctrl;
> > + u8 restart;
> > + u8 link;
> > + u8 speed;
> > + u8 force;
> > + u8 p = phy;
> > + u16 data = 0;
> > + int processed = true;
> > +
> > + port = &dev->ports[p];
> > + switch (reg) {
> > + case PHY_REG_CTRL:
> > + ksz_pread8(dev, p, P_LOCAL_CTRL, &ctrl);
> > + ksz_pread8(dev, p, P_NEG_RESTART_CTRL, &restart);
> > + ksz_pread8(dev, p, P_SPEED_STATUS, &speed);
> > + ksz_pread8(dev, p, P_FORCE_CTRL, &force);
> > + if (restart & PORT_PHY_LOOPBACK)
> > + data |= PHY_LOOPBACK;
> > + if (force & PORT_FORCE_100_MBIT)
> > + data |= PHY_SPEED_100MBIT;
> > + if (!(force & PORT_AUTO_NEG_DISABLE))
> > + data |= PHY_AUTO_NEG_ENABLE;
> > + if (restart & PORT_POWER_DOWN)
> > + data |= PHY_POWER_DOWN;
>
> Same questions i asked Pavel
>
> Is there a way to access the PHY registers over SPI? The datasheet
> suggests there is, but it does not say how, as far as i can tell.
>
> Ideally, we want to use just a normal PHY driver.
>
The switch does not have actual PHY registers when used in SPI mode,
so the PHY register access has to be simulated.
A side-note is the PHY device id returned will pair the PHY with one of
the Micrel PHYs defined in the kernel. I just found out the new phylib
code removed the Asymmetric/Symmetric Pause support in the PHY
driver and let the MAC driver indicate the support. This is reasonable
as the MAC is responsible for sending and processing PAUSE frames.
However, the PHY in the switch port is not connected to the MAC.
Is there a way to indicate this support in the DSA model?
Ideally the switch should have control over which Pause mode should
be enabled. Generally Asymmetric Pause is used in Gigabit switch, and
flow control is better disabled if some other traffic shaping is used.
> > +static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> > + uint64_t *buf)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + struct ksz_port_mib *mib;
> > + struct ksz_port_mib_info *info;
> > + int i;
> > +
> > + mib = &dev->ports[port].mib;
> > +
> > + spin_lock(&dev->mib_read_lock);
> > + for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> > + info = &mib->info[i];
> > + buf[i] = info->counter;
> > + info->read_cnt += info->read_max;
> > + }
> > + spin_unlock(&dev->mib_read_lock);
> > +
> > + mib->ctrl = 1;
> > + schedule_work(&dev->mib_read);
>
> Humm. I want to take a closer look at this caching code...
The switch needs to read MIB counters periodically as the counters
will overflow when there are lots of traffic. The current
implementation is to read all ports every second but not all at the
same time as the operation may block other critical hardware
register access.
The idea is if the users check the MIB counters once they will check
them again very soon to note which counter has been increased.
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-18 20:41 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=djbT-35JuHq+hhOKOG+BEO3RCwgsVPoKcDhNBph5OezA@mail.gmail.com>
OK,
with:
net.ipv4.tcp_recovery = 0
net.ipv4.tcp_fack = 0
and your patch I got the following warning within 10 min uptime:
===
Sep 18 22:18:34 defiant kernel: ------------[ cut here ]------------
Sep 18 22:18:34 defiant kernel: WARNING: CPU: 0 PID: 702 at net/ipv4/
tcp_input.c:2392 tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 18 22:18:34 defiant kernel: Modules linked in: netconsole ctr ccm cls_bpf
sch_htb act_mirred cls_u32 sch_ingress sit tunnel4 ip_tunnel 8021q mrp
nf_conntrack_ipv6 nf_defrag_ipv6 nft_ct nft_set_bitmap nft_set_hash
nft_set_rbtree nf_tables_inet nf_tables_ipv6 nft_masq_ipv4
nf_nat_masquerade_ipv4 nft_masq nft_nat nft_counter nft_meta
nft_chain_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat
nf_conntrack libcrc32c crc32c_generic nf_tables_ipv4 nf_tables tun nct6775
nfnetlink hwmon_vid nls_iso8859_1 nls_cp437 vfat fat ext4 snd_hda_codec_hdmi
mbcache jbd2 snd_hda_codec_realtek snd_hda_codec_generic f2fs arc4 fscrypto
intel_rapl iTCO_wdt ath9k iTCO_vendor_support intel_powerclamp ath9k_common
ath9k_hw coretemp kvm_intel ath mac80211 kvm irqbypass intel_cstate cfg80211
pcspkr snd_hda_intel snd_hda_codec r8169
Sep 18 22:18:34 defiant kernel: joydev evdev mii snd_hda_core mousedev
mei_txe input_leds i2c_i801 mac_hid i915 lpc_ich mei shpchp snd_hwdep
snd_intel_sst_acpi snd_intel_sst_core snd_soc_rt5670
snd_soc_sst_atom_hifi2_platform battery snd_soc_sst_match snd_soc_rl6231
drm_kms_helper hci_uart ov5693(C) ov2722(C) lm3554(C) btbcm btqca v4l2_common
snd_soc_core btintel snd_compress videodev snd_pcm_dmaengine snd_pcm video
bluetooth snd_timer drm media tpm_tis snd i2c_hid soundcore tpm_tis_core
rfkill_gpio ac97_bus soc_button_array ecdh_generic rfkill crc16 tpm 8250_dw
intel_gtt syscopyarea sysfillrect acpi_pad sysimgblt intel_int0002_vgpio
fb_sys_fops pinctrl_cherryview i2c_algo_bit button sch_fq_codel tcp_bbr ifb
ip_tables x_tables btrfs xor raid6_pq algif_skcipher af_alg hid_logitech_hidpp
hid_logitech_dj usbhid hid uas
Sep 18 22:18:34 defiant kernel: usb_storage dm_crypt dm_mod dax raid10 md_mod
sd_mod crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc
ahci aesni_intel xhci_pci libahci aes_x86_64 crypto_simd glue_helper xhci_hcd
cryptd libata usbcore scsi_mod usb_common serio sdhci_acpi sdhci led_class
mmc_core
Sep 18 22:18:34 defiant kernel: CPU: 0 PID: 702 Comm: irq/123-enp3s0 Tainted:
G C 4.13.0-pf4 #1
Sep 18 22:18:34 defiant kernel: Hardware name: To Be Filled By O.E.M. To Be
Filled By O.E.M./J3710-ITX, BIOS P1.30 03/30/2016
Sep 18 22:18:34 defiant kernel: task: ffff88923a738000 task.stack:
ffff958001500000
Sep 18 22:18:34 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 18 22:18:34 defiant kernel: RSP: 0018:ffff88927fc03a48 EFLAGS: 00010202
Sep 18 22:18:34 defiant kernel: RAX: 0000000000000001 RBX: ffff889241130800
RCX: ffff88927fc03b0c
Sep 18 22:18:34 defiant kernel: RDX: 000000007fffffff RSI: 0000000000000001
RDI: ffff889241130800
Sep 18 22:18:34 defiant kernel: RBP: ffff88927fc03a50 R08: 0000000000000000
R09: 000000007b3e89be
Sep 18 22:18:34 defiant kernel: R10: 000000007b3ea2f6 R11: 000000007b3e89be
R12: 0000000000005320
Sep 18 22:18:34 defiant kernel: R13: ffff88927fc03b10 R14: 0000000000000001
R15: ffff88927fc03b0c
Sep 18 22:18:34 defiant kernel: FS: 0000000000000000(0000)
GS:ffff88927fc00000(0000) knlGS:0000000000000000
Sep 18 22:18:34 defiant kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
Sep 18 22:18:34 defiant kernel: CR2: 00007fe9c4106898 CR3: 0000000114a09000
CR4: 00000000001006f0
Sep 18 22:18:34 defiant kernel: Call Trace:
Sep 18 22:18:34 defiant kernel: <IRQ>
Sep 18 22:18:34 defiant kernel: tcp_try_undo_loss+0xb3/0xf0
Sep 18 22:18:34 defiant kernel: tcp_fastretrans_alert+0x746/0x990
Sep 18 22:18:34 defiant kernel: tcp_ack+0x741/0x1110
Sep 18 22:18:34 defiant kernel: tcp_rcv_established+0x325/0x770
Sep 18 22:18:34 defiant kernel: ? sk_filter_trim_cap+0xd4/0x1a0
Sep 18 22:18:34 defiant kernel: tcp_v4_do_rcv+0x90/0x1e0
Sep 18 22:18:34 defiant kernel: tcp_v4_rcv+0x950/0xa10
Sep 18 22:18:34 defiant kernel: ? nf_ct_deliver_cached_events+0xb8/0x110
[nf_conntrack]
Sep 18 22:18:34 defiant kernel: ip_local_deliver_finish+0x68/0x210
Sep 18 22:18:34 defiant kernel: ip_local_deliver+0xfa/0x110
Sep 18 22:18:34 defiant kernel: ? ip_rcv_finish+0x410/0x410
Sep 18 22:18:34 defiant kernel: ip_rcv_finish+0x120/0x410
Sep 18 22:18:34 defiant kernel: ip_rcv+0x28e/0x3b0
Sep 18 22:18:34 defiant kernel: ? inet_del_offload+0x40/0x40
Sep 18 22:18:34 defiant kernel: __netif_receive_skb_core+0x39b/0xb00
Sep 18 22:18:34 defiant kernel: ? skb_release_all+0x24/0x30
Sep 18 22:18:34 defiant kernel: ? consume_skb+0x38/0xa0
Sep 18 22:18:34 defiant kernel: __netif_receive_skb+0x18/0x60
Sep 18 22:18:34 defiant kernel: netif_receive_skb_internal+0x98/0x480
Sep 18 22:18:34 defiant kernel: netif_receive_skb+0x1c/0x80
Sep 18 22:18:34 defiant kernel: ifb_ri_tasklet+0x109/0x26a [ifb]
Sep 18 22:18:34 defiant kernel: tasklet_action+0x63/0x120
Sep 18 22:18:34 defiant kernel: __do_softirq+0xdf/0x2e5
Sep 18 22:18:34 defiant kernel: ? irq_finalize_oneshot.part.39+0xe0/0xe0
Sep 18 22:18:34 defiant kernel: do_softirq_own_stack+0x1c/0x30
Sep 18 22:18:34 defiant kernel: </IRQ>
Sep 18 22:18:34 defiant kernel: do_softirq.part.17+0x4e/0x60
Sep 18 22:18:34 defiant kernel: __local_bh_enable_ip+0x77/0x80
Sep 18 22:18:34 defiant kernel: irq_forced_thread_fn+0x5c/0x70
Sep 18 22:18:34 defiant kernel: irq_thread+0x131/0x1a0
Sep 18 22:18:34 defiant kernel: ? wake_threads_waitq+0x30/0x30
Sep 18 22:18:34 defiant kernel: kthread+0x126/0x140
Sep 18 22:18:34 defiant kernel: ? irq_thread_check_affinity+0x90/0x90
Sep 18 22:18:34 defiant kernel: ? kthread_create_on_node+0x70/0x70
Sep 18 22:18:34 defiant kernel: ret_from_fork+0x25/0x30
Sep 18 22:18:34 defiant kernel: Code: 5d c3 80 60 35 fb 48 8b 00 48 39 c2 74
85 48 3b 83 50 01 00 00 75 eb e9 77 ff ff ff 89 83 48 06 00 00 80 a3 1e 06 00
00 fb eb b3 <0f> ff 5b 5d c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f
Sep 18 22:18:34 defiant kernel: ---[ end trace 1aea180efeedb473 ]---
===
Should I continue with net.ipv4.tcp_recovery = 1, or this is enough?
On pondělí 18. září 2017 20:01:42 CEST Yuchung Cheng wrote:
> On Mon, Sep 18, 2017 at 10:59 AM, Oleksandr Natalenko
>
> <oleksandr@natalenko.name> wrote:
> > OK. Should I keep FACK disabled?
>
> Yes since it is disabled in the upstream by default. Although you can
> experiment FACK enabled additionally.
>
> Do we know the crash you first experienced is tied to this issue?
>
> > On pondělí 18. září 2017 19:51:21 CEST Yuchung Cheng wrote:
> >> Can you try this patch to verify my theory with tcp_recovery=0 and 1?
> >> thanks
> >>
> >> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> >> index 5af2f04f8859..9253d9ee7d0e 100644
> >> --- a/net/ipv4/tcp_input.c
> >> +++ b/net/ipv4/tcp_input.c
> >> @@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
> >> *sk, bool unmark_loss)
> >>
> >> }
> >> tp->snd_cwnd_stamp = tcp_time_stamp;
> >> tp->undo_marker = 0;
> >>
> >> + WARN_ON(tp->retrans_out);
> >>
> >> }
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-18 20:46 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=djbT-35JuHq+hhOKOG+BEO3RCwgsVPoKcDhNBph5OezA@mail.gmail.com>
Actually, same warning was just triggered with RACK enabled. But main warning
was not triggered in this case.
===
Sep 18 22:44:32 defiant kernel: ------------[ cut here ]------------
Sep 18 22:44:32 defiant kernel: WARNING: CPU: 1 PID: 702 at net/ipv4/
tcp_input.c:2392 tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 18 22:44:32 defiant kernel: Modules linked in: netconsole ctr ccm cls_bpf
sch_htb act_mirred cls_u32 sch_ingress sit tunnel4 ip_tunnel 8021q mrp
nf_conntrack_ipv6 nf_defrag_ipv6 nft_ct nft_set_bitmap nft_set_hash
nft_set_rbtree nf_tables_inet nf_tables_ipv6 nft_masq_ipv4
nf_nat_masquerade_ipv4 nft_masq nft_nat nft_counter nft_meta
nft_chain_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat
nf_conntrack libcrc32c crc32c_generic nf_tables_ipv4 nf_tables tun nct6775
nfnetlink hwmon_vid nls_iso8859_1 nls_cp437 vfat fat ext4 snd_hda_codec_hdmi
mbcache jbd2 snd_hda_codec_realtek snd_hda_codec_generic f2fs arc4 fscrypto
intel_rapl iTCO_wdt ath9k iTCO_vendor_support intel_powerclamp ath9k_common
ath9k_hw coretemp kvm_intel ath mac80211 kvm irqbypass intel_cstate cfg80211
pcspkr snd_hda_intel snd_hda_codec r8169
Sep 18 22:44:32 defiant kernel: joydev evdev mii snd_hda_core mousedev
mei_txe input_leds i2c_i801 mac_hid i915 lpc_ich mei shpchp snd_hwdep
snd_intel_sst_acpi snd_intel_sst_core snd_soc_rt5670
snd_soc_sst_atom_hifi2_platform battery snd_soc_sst_match snd_soc_rl6231
drm_kms_helper hci_uart ov5693(C) ov2722(C) lm3554(C) btbcm btqca v4l2_common
snd_soc_core btintel snd_compress videodev snd_pcm_dmaengine snd_pcm video
bluetooth snd_timer drm media tpm_tis snd i2c_hid soundcore tpm_tis_core
rfkill_gpio ac97_bus soc_button_array ecdh_generic rfkill crc16 tpm 8250_dw
intel_gtt syscopyarea sysfillrect acpi_pad sysimgblt intel_int0002_vgpio
fb_sys_fops pinctrl_cherryview i2c_algo_bit button sch_fq_codel tcp_bbr ifb
ip_tables x_tables btrfs xor raid6_pq algif_skcipher af_alg hid_logitech_hidpp
hid_logitech_dj usbhid hid uas
Sep 18 22:44:32 defiant kernel: usb_storage dm_crypt dm_mod dax raid10 md_mod
sd_mod crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc
ahci aesni_intel xhci_pci libahci aes_x86_64 crypto_simd glue_helper xhci_hcd
cryptd libata usbcore scsi_mod usb_common serio sdhci_acpi sdhci led_class
mmc_core
Sep 18 22:44:32 defiant kernel: CPU: 1 PID: 702 Comm: irq/123-enp3s0 Tainted:
G WC 4.13.0-pf4 #1
Sep 18 22:44:32 defiant kernel: Hardware name: To Be Filled By O.E.M. To Be
Filled By O.E.M./J3710-ITX, BIOS P1.30 03/30/2016
Sep 18 22:44:32 defiant kernel: task: ffff88923a738000 task.stack:
ffff958001500000
Sep 18 22:44:32 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 18 22:44:32 defiant kernel: RSP: 0018:ffff88927fc83a48 EFLAGS: 00010202
Sep 18 22:44:32 defiant kernel: RAX: 0000000000000001 RBX: ffff8892412d9800
RCX: ffff88927fc83b0c
Sep 18 22:44:32 defiant kernel: RDX: 000000007fffffff RSI: 0000000000000001
RDI: ffff8892412d9800
Sep 18 22:44:32 defiant kernel: RBP: ffff88927fc83a50 R08: 0000000000000000
R09: 0000000018dfb063
Sep 18 22:44:32 defiant kernel: R10: 0000000018dfd223 R11: 0000000018dfb063
R12: 0000000000005320
Sep 18 22:44:32 defiant kernel: R13: ffff88927fc83b10 R14: 0000000000000001
R15: ffff88927fc83b0c
Sep 18 22:44:32 defiant kernel: FS: 0000000000000000(0000)
GS:ffff88927fc80000(0000) knlGS:0000000000000000
Sep 18 22:44:32 defiant kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
0000000080050033
Sep 18 22:44:32 defiant kernel: CR2: 00007f1cd1a43620 CR3: 0000000114a09000
CR4: 00000000001006e0
Sep 18 22:44:32 defiant kernel: Call Trace:
Sep 18 22:44:32 defiant kernel: <IRQ>
Sep 18 22:44:32 defiant kernel: tcp_try_undo_loss+0xb3/0xf0
Sep 18 22:44:32 defiant kernel: tcp_fastretrans_alert+0x746/0x990
Sep 18 22:44:32 defiant kernel: tcp_ack+0x741/0x1110
Sep 18 22:44:32 defiant kernel: tcp_rcv_established+0x325/0x770
Sep 18 22:44:32 defiant kernel: ? sk_filter_trim_cap+0xd4/0x1a0
Sep 18 22:44:32 defiant kernel: tcp_v4_do_rcv+0x90/0x1e0
Sep 18 22:44:32 defiant kernel: tcp_v4_rcv+0x950/0xa10
Sep 18 22:44:32 defiant kernel: ? nf_ct_deliver_cached_events+0xb8/0x110
[nf_conntrack]
Sep 18 22:44:32 defiant kernel: ip_local_deliver_finish+0x68/0x210
Sep 18 22:44:32 defiant kernel: ip_local_deliver+0xfa/0x110
Sep 18 22:44:32 defiant kernel: ? ip_rcv_finish+0x410/0x410
Sep 18 22:44:32 defiant kernel: ip_rcv_finish+0x120/0x410
Sep 18 22:44:32 defiant kernel: ip_rcv+0x28e/0x3b0
Sep 18 22:44:32 defiant kernel: ? inet_del_offload+0x40/0x40
Sep 18 22:44:32 defiant kernel: __netif_receive_skb_core+0x39b/0xb00
Sep 18 22:44:32 defiant kernel: ? netif_receive_skb_internal+0xa0/0x480
Sep 18 22:44:32 defiant kernel: ? dev_gro_receive+0x2eb/0x4a0
Sep 18 22:44:32 defiant kernel: __netif_receive_skb+0x18/0x60
Sep 18 22:44:32 defiant kernel: netif_receive_skb_internal+0x98/0x480
Sep 18 22:44:32 defiant kernel: netif_receive_skb+0x1c/0x80
Sep 18 22:44:32 defiant kernel: ifb_ri_tasklet+0x109/0x26a [ifb]
Sep 18 22:44:32 defiant kernel: tasklet_action+0x63/0x120
Sep 18 22:44:32 defiant kernel: __do_softirq+0xdf/0x2e5
Sep 18 22:44:32 defiant kernel: ? irq_finalize_oneshot.part.39+0xe0/0xe0
Sep 18 22:44:32 defiant kernel: do_softirq_own_stack+0x1c/0x30
Sep 18 22:44:32 defiant kernel: </IRQ>
Sep 18 22:44:32 defiant kernel: do_softirq.part.17+0x4e/0x60
Sep 18 22:44:32 defiant kernel: __local_bh_enable_ip+0x77/0x80
Sep 18 22:44:32 defiant kernel: irq_forced_thread_fn+0x5c/0x70
Sep 18 22:44:32 defiant kernel: irq_thread+0x131/0x1a0
Sep 18 22:44:32 defiant kernel: ? wake_threads_waitq+0x30/0x30
Sep 18 22:44:32 defiant kernel: kthread+0x126/0x140
Sep 18 22:44:32 defiant kernel: ? irq_thread_check_affinity+0x90/0x90
Sep 18 22:44:32 defiant kernel: ? kthread_create_on_node+0x70/0x70
Sep 18 22:44:32 defiant kernel: ret_from_fork+0x25/0x30
Sep 18 22:44:32 defiant kernel: Code: 5d c3 80 60 35 fb 48 8b 00 48 39 c2 74
85 48 3b 83 50 01 00 00 75 eb e9 77 ff ff ff 89 83 48 06 00 00 80 a3 1e 06 00
00 fb eb b3 <0f> ff 5b 5d c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f
Sep 18 22:44:32 defiant kernel: ---[ end trace 1aea180efeedb474 ]---
===
On pondělí 18. září 2017 20:01:42 CEST Yuchung Cheng wrote:
> On Mon, Sep 18, 2017 at 10:59 AM, Oleksandr Natalenko
>
> <oleksandr@natalenko.name> wrote:
> > OK. Should I keep FACK disabled?
>
> Yes since it is disabled in the upstream by default. Although you can
> experiment FACK enabled additionally.
>
> Do we know the crash you first experienced is tied to this issue?
>
> > On pondělí 18. září 2017 19:51:21 CEST Yuchung Cheng wrote:
> >> Can you try this patch to verify my theory with tcp_recovery=0 and 1?
> >> thanks
> >>
> >> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> >> index 5af2f04f8859..9253d9ee7d0e 100644
> >> --- a/net/ipv4/tcp_input.c
> >> +++ b/net/ipv4/tcp_input.c
> >> @@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
> >> *sk, bool unmark_loss)
> >>
> >> }
> >> tp->snd_cwnd_stamp = tcp_time_stamp;
> >> tp->undo_marker = 0;
> >>
> >> + WARN_ON(tp->retrans_out);
> >>
> >> }
^ permalink raw reply
* [PATCH] tcp: avoid bogus warning in tcp_clean_rtx_queue
From: Arnd Bergmann @ 2017-09-18 20:48 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: Arnd Bergmann, Eric Dumazet, Neal Cardwell, Yuchung Cheng,
Soheil Hassas Yeganeh, Florian Westphal, netdev, linux-kernel
gcc-4.9 warns that it cannot trace the state of the 'last_ackt'
variable since the change to the TCP timestamping code, when
CONFIG_PROFILE_ANNOTATED_BRANCHES is set:
net/ipv4/tcp_input.c: In function 'tcp_clean_rtx_queue':
include/net/tcp.h:757:23: error: 'last_ackt' may be used uninitialized in this function [-Werror=maybe-uninitialized]
Other gcc versions, both older and newer do now show this
warning. Removing the 'likely' annotation makes it go away,
and has no effect on the object code without
CONFIG_PROFILE_ANNOTATED_BRANCHES, as tested with gcc-4.9
and gcc-7.1.1, so this seems to be a safe workaround.
Fixes: 9a568de4818d ("tcp: switch TCP TS option (RFC 7323) to 1ms clock")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/ipv4/tcp_input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c5d7656beeee..c52bc8e35d4d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3173,7 +3173,7 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
if (skb && (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED))
flag |= FLAG_SACK_RENEGING;
- if (likely(first_ackt) && !(flag & FLAG_RETRANS_DATA_ACKED)) {
+ if (first_ackt && !(flag & FLAG_RETRANS_DATA_ACKED)) {
seq_rtt_us = tcp_stamp_us_delta(tp->tcp_mstamp, first_ackt);
ca_rtt_us = tcp_stamp_us_delta(tp->tcp_mstamp, last_ackt);
}
--
2.9.0
^ permalink raw reply related
* RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Tristram.Ha @ 2017-09-18 20:55 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170918195708.GE15606@lunn.ch>
> > > This is ugly. We have a clean separation between a switch driver and a
> > > tag driver. Here you are mixing them together. Don't. Look at the
> > > mailing lists for what Florian and I suggested to Pavel. It will also
> > > solve your include file issue above.
> >
> > It seems to me all tag_*.c are hard-coded. They do not have access to
> > the switch device and just use the bit definitions defined in the top to
> > do the job.
> >
> > This creates a problem for all KSZ switch devices as they have different
> > tail tag formats and lengths. There will be potentially 4 additional
> > DSA_TAG_PROT_KSZ* just to handle them. Extra files will be added
> > with about the same code.
>
> Hi Tristram
>
> Think about factoring out the common code into a shared functions.
>
I am a little unsure what you have in mind. Can you elaborate?
Like creating an additional ksz_xmit function and passing a function pointer
to it to do the job?
> > But the most important thing is how do we support the offload_fwd_mark
> > bit in skb when the only place that has access to skb does not have access
> to
> > the switch hardware?
>
> Well, so far, nothing has set offload_fwd_mark. This is about to
> change, since i need it for IGMP snooping on the brX interface, for
> Marvell at least. Bit i just need to set it to true for all frames.
>
> What use cases do you have in mind where you need information from the
> switch driver to decide on its value?
>
In the old DSA implementation all the ports are partitioned into its own device
and the bridge joining them will do all the forwarding. This is useful for quick
testing with some protocols like RSTP but it is probably useless for real
operation.
The new switchdev model tries to use the switch hardware as much as
possible. This offload_fwd_mark bit means the frame is forwarded by the
hardware switch, so the software bridge does not need to do it again. Without
this bit there will be duplicated multicast frames coming out the ports if internal
forwarding is enabled.
When the DSA device is first created all ports are independent devices. When
a bridge is created and those devices are attached the ports share the same
membership so all frames will be forwarded internally.
When RSTP is used the port can be put in blocked state and so the forwarding
will stop for that port. Currently the switch driver will check that membership
to decide whether to set that bit.
> > A little out-of-topic is the MAC driver may have problem receiving the
> frame if
> > the tail tag length is too big. Most of the MAC drivers have big enough
> receive
> > buffer or require 64-bytes multiple so there are extra room to
> accommodate
> > the big tail tag at the end of the frame. Still some MAC drivers use exactly
> 1500
> > MTU and some additional length and may run into this problem. Currently
> the
> > Atmel Cadence MAC driver has this potential problem when certain
> conditions
> > are met.
>
> This is a know problem. I just fixed the FEC driver which also
> discarded DSA tagged frames when they were bigger than the MTU.
>
> So far, it has not been too much of an issue, because most boards with
> a switch, use an Ethernet interface from the same manufacture. Marvell
> Switches with a Marvell Ethernet interface. Broadcom switch with a
> Broadcom Interface. Atheros switch with an Atheros interface. And
> naturally, these combinations just work. But Freescale FEC and Marvell
> had not been combined before, and it was broken.
>
> So i would not be surprised if the Cadence MAC driver had an issue.
The KSZ switches never have a built-in MAC controller, so it is always a support
issue for us.
^ permalink raw reply
* [PATCH net-next 0/4] test_rhashtable: add test case for rhl table
From: Florian Westphal @ 2017-09-18 21:07 UTC (permalink / raw)
To: netdev
Add a test case for the rhlist interface.
While at it, cleanup current rhashtable test a bit and add a check
for max_size support.
test_rhashtable.c | 293 +++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 270 insertions(+), 23 deletions(-)
^ permalink raw reply
* [PATCH net-next 1/4] test_rhashtable: don't allocate huge static array
From: Florian Westphal @ 2017-09-18 21:07 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170918210711.10202-1-fw@strlen.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
lib/test_rhashtable.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index 0ffca990a833..c40d6e636f33 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -72,8 +72,6 @@ struct thread_data {
struct test_obj *objs;
};
-static struct test_obj array[MAX_ENTRIES];
-
static struct rhashtable_params test_rht_params = {
.head_offset = offsetof(struct test_obj, node),
.key_offset = offsetof(struct test_obj, value),
@@ -85,7 +83,7 @@ static struct rhashtable_params test_rht_params = {
static struct semaphore prestart_sem;
static struct semaphore startup_sem = __SEMAPHORE_INITIALIZER(startup_sem, 0);
-static int insert_retry(struct rhashtable *ht, struct rhash_head *obj,
+static int insert_retry(struct rhashtable *ht, struct test_obj *obj,
const struct rhashtable_params params)
{
int err, retries = -1, enomem_retries = 0;
@@ -93,7 +91,7 @@ static int insert_retry(struct rhashtable *ht, struct rhash_head *obj,
do {
retries++;
cond_resched();
- err = rhashtable_insert_fast(ht, obj, params);
+ err = rhashtable_insert_fast(ht, &obj->node, params);
if (err == -ENOMEM && enomem_retry) {
enomem_retries++;
err = -EBUSY;
@@ -107,7 +105,7 @@ static int insert_retry(struct rhashtable *ht, struct rhash_head *obj,
return err ? : retries;
}
-static int __init test_rht_lookup(struct rhashtable *ht)
+static int __init test_rht_lookup(struct rhashtable *ht, struct test_obj *array)
{
unsigned int i;
@@ -186,7 +184,7 @@ static void test_bucket_stats(struct rhashtable *ht)
pr_warn("Test failed: Total count mismatch ^^^");
}
-static s64 __init test_rhashtable(struct rhashtable *ht)
+static s64 __init test_rhashtable(struct rhashtable *ht, struct test_obj *array)
{
struct test_obj *obj;
int err;
@@ -203,7 +201,7 @@ static s64 __init test_rhashtable(struct rhashtable *ht)
struct test_obj *obj = &array[i];
obj->value.id = i * 2;
- err = insert_retry(ht, &obj->node, test_rht_params);
+ err = insert_retry(ht, obj, test_rht_params);
if (err > 0)
insert_retries += err;
else if (err)
@@ -216,7 +214,7 @@ static s64 __init test_rhashtable(struct rhashtable *ht)
test_bucket_stats(ht);
rcu_read_lock();
- test_rht_lookup(ht);
+ test_rht_lookup(ht, array);
rcu_read_unlock();
test_bucket_stats(ht);
@@ -286,7 +284,7 @@ static int threadfunc(void *data)
for (i = 0; i < entries; i++) {
tdata->objs[i].value.id = i;
tdata->objs[i].value.tid = tdata->id;
- err = insert_retry(&ht, &tdata->objs[i].node, test_rht_params);
+ err = insert_retry(&ht, &tdata->objs[i], test_rht_params);
if (err > 0) {
insert_retries += err;
} else if (err) {
@@ -349,6 +347,10 @@ static int __init test_rht_init(void)
test_rht_params.max_size = max_size ? : roundup_pow_of_two(entries);
test_rht_params.nelem_hint = size;
+ objs = vzalloc((test_rht_params.max_size + 1) * sizeof(struct test_obj));
+ if (!objs)
+ return -ENOMEM;
+
pr_info("Running rhashtable test nelem=%d, max_size=%d, shrinking=%d\n",
size, max_size, shrinking);
@@ -356,7 +358,8 @@ static int __init test_rht_init(void)
s64 time;
pr_info("Test %02d:\n", i);
- memset(&array, 0, sizeof(array));
+ memset(objs, 0, test_rht_params.max_size * sizeof(struct test_obj));
+
err = rhashtable_init(&ht, &test_rht_params);
if (err < 0) {
pr_warn("Test failed: Unable to initialize hashtable: %d\n",
@@ -364,9 +367,10 @@ static int __init test_rht_init(void)
continue;
}
- time = test_rhashtable(&ht);
+ time = test_rhashtable(&ht, objs);
rhashtable_destroy(&ht);
if (time < 0) {
+ vfree(objs);
pr_warn("Test failed: return code %lld\n", time);
return -EINVAL;
}
@@ -374,6 +378,7 @@ static int __init test_rht_init(void)
total_time += time;
}
+ vfree(objs);
do_div(total_time, runs);
pr_info("Average test time: %llu\n", total_time);
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 2/4] test_rhashtable: don't use global entries variable
From: Florian Westphal @ 2017-09-18 21:07 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170918210711.10202-1-fw@strlen.de>
pass the entries to test as an argument instead.
Followup patch will add an rhlist test case; rhlist delete opererations
are slow so we need to use a smaller number to test it.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
lib/test_rhashtable.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index c40d6e636f33..69f5b3849980 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -28,9 +28,9 @@
#define MAX_ENTRIES 1000000
#define TEST_INSERT_FAIL INT_MAX
-static int entries = 50000;
-module_param(entries, int, 0);
-MODULE_PARM_DESC(entries, "Number of entries to add (default: 50000)");
+static int parm_entries = 50000;
+module_param(parm_entries, int, 0);
+MODULE_PARM_DESC(parm_entries, "Number of entries to add (default: 50000)");
static int runs = 4;
module_param(runs, int, 0);
@@ -67,6 +67,7 @@ struct test_obj {
};
struct thread_data {
+ unsigned int entries;
int id;
struct task_struct *task;
struct test_obj *objs;
@@ -105,11 +106,12 @@ static int insert_retry(struct rhashtable *ht, struct test_obj *obj,
return err ? : retries;
}
-static int __init test_rht_lookup(struct rhashtable *ht, struct test_obj *array)
+static int __init test_rht_lookup(struct rhashtable *ht, struct test_obj *array,
+ unsigned int entries)
{
unsigned int i;
- for (i = 0; i < entries * 2; i++) {
+ for (i = 0; i < entries; i++) {
struct test_obj *obj;
bool expected = !(i % 2);
struct test_obj_val key = {
@@ -142,7 +144,7 @@ static int __init test_rht_lookup(struct rhashtable *ht, struct test_obj *array)
return 0;
}
-static void test_bucket_stats(struct rhashtable *ht)
+static void test_bucket_stats(struct rhashtable *ht, unsigned int entries)
{
unsigned int err, total = 0, chain_len = 0;
struct rhashtable_iter hti;
@@ -184,7 +186,8 @@ static void test_bucket_stats(struct rhashtable *ht)
pr_warn("Test failed: Total count mismatch ^^^");
}
-static s64 __init test_rhashtable(struct rhashtable *ht, struct test_obj *array)
+static s64 __init test_rhashtable(struct rhashtable *ht, struct test_obj *array,
+ unsigned int entries)
{
struct test_obj *obj;
int err;
@@ -212,12 +215,12 @@ static s64 __init test_rhashtable(struct rhashtable *ht, struct test_obj *array)
pr_info(" %u insertions retried due to memory pressure\n",
insert_retries);
- test_bucket_stats(ht);
+ test_bucket_stats(ht, entries);
rcu_read_lock();
- test_rht_lookup(ht, array);
+ test_rht_lookup(ht, array, entries);
rcu_read_unlock();
- test_bucket_stats(ht);
+ test_bucket_stats(ht, entries);
pr_info(" Deleting %d keys\n", entries);
for (i = 0; i < entries; i++) {
@@ -245,6 +248,7 @@ static struct rhashtable ht;
static int thread_lookup_test(struct thread_data *tdata)
{
+ unsigned int entries = tdata->entries;
int i, err = 0;
for (i = 0; i < entries; i++) {
@@ -281,7 +285,7 @@ static int threadfunc(void *data)
if (down_interruptible(&startup_sem))
pr_err(" thread[%d]: down_interruptible failed\n", tdata->id);
- for (i = 0; i < entries; i++) {
+ for (i = 0; i < tdata->entries; i++) {
tdata->objs[i].value.id = i;
tdata->objs[i].value.tid = tdata->id;
err = insert_retry(&ht, &tdata->objs[i], test_rht_params);
@@ -305,7 +309,7 @@ static int threadfunc(void *data)
}
for (step = 10; step > 0; step--) {
- for (i = 0; i < entries; i += step) {
+ for (i = 0; i < tdata->entries; i += step) {
if (tdata->objs[i].value.id == TEST_INSERT_FAIL)
continue;
err = rhashtable_remove_fast(&ht, &tdata->objs[i].node,
@@ -336,12 +340,16 @@ static int threadfunc(void *data)
static int __init test_rht_init(void)
{
+ unsigned int entries;
int i, err, started_threads = 0, failed_threads = 0;
u64 total_time = 0;
struct thread_data *tdata;
struct test_obj *objs;
- entries = min(entries, MAX_ENTRIES);
+ if (parm_entries < 0)
+ parm_entries = 1;
+
+ entries = min(parm_entries, MAX_ENTRIES);
test_rht_params.automatic_shrinking = shrinking;
test_rht_params.max_size = max_size ? : roundup_pow_of_two(entries);
@@ -367,7 +375,7 @@ static int __init test_rht_init(void)
continue;
}
- time = test_rhashtable(&ht, objs);
+ time = test_rhashtable(&ht, objs, entries);
rhashtable_destroy(&ht);
if (time < 0) {
vfree(objs);
@@ -409,6 +417,7 @@ static int __init test_rht_init(void)
}
for (i = 0; i < tcount; i++) {
tdata[i].id = i;
+ tdata[i].entries = entries;
tdata[i].objs = objs + i * entries;
tdata[i].task = kthread_run(threadfunc, &tdata[i],
"rhashtable_thrad[%d]", i);
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 3/4] test_rhashtable: add a check for max_size
From: Florian Westphal @ 2017-09-18 21:07 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170918210711.10202-1-fw@strlen.de>
add a test that tries to insert more than max_size elements.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
lib/test_rhashtable.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index 69f5b3849980..1eee90e6e394 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -246,6 +246,43 @@ static s64 __init test_rhashtable(struct rhashtable *ht, struct test_obj *array,
static struct rhashtable ht;
+static int __init test_rhashtable_max(struct test_obj *array,
+ unsigned int entries)
+{
+ unsigned int i, insert_retries = 0;
+ int err;
+
+ test_rht_params.max_size = roundup_pow_of_two(entries / 8);
+ err = rhashtable_init(&ht, &test_rht_params);
+ if (err)
+ return err;
+
+ for (i = 0; i < ht.max_elems; i++) {
+ struct test_obj *obj = &array[i];
+
+ obj->value.id = i * 2;
+ err = insert_retry(&ht, obj, test_rht_params);
+ if (err > 0)
+ insert_retries += err;
+ else if (err)
+ return err;
+ }
+
+ err = insert_retry(&ht, &array[ht.max_elems], test_rht_params);
+ if (err == -E2BIG) {
+ err = 0;
+ } else {
+ pr_info("insert element %u should have failed with %d, got %d\n",
+ ht.max_elems, -E2BIG, err);
+ if (err == 0)
+ err = -1;
+ }
+
+ rhashtable_destroy(&ht);
+
+ return err;
+}
+
static int thread_lookup_test(struct thread_data *tdata)
{
unsigned int entries = tdata->entries;
@@ -386,7 +423,11 @@ static int __init test_rht_init(void)
total_time += time;
}
+ pr_info("test if its possible to exceed max_size %d: %s\n",
+ test_rht_params.max_size, test_rhashtable_max(objs, entries) == 0 ?
+ "no, ok" : "YES, failed");
vfree(objs);
+
do_div(total_time, runs);
pr_info("Average test time: %llu\n", total_time);
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 4/4] test_rhashtable: add test case for rhl_table interface
From: Florian Westphal @ 2017-09-18 21:07 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170918210711.10202-1-fw@strlen.de>
also test rhltable. rhltable remove operations are slow as
deletions require a list walk, thus test with 1/16th of the given
entry count number to get a run duration similar to rhashtable one.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
lib/test_rhashtable.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 194 insertions(+), 2 deletions(-)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index 1eee90e6e394..b40b2880403e 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -23,6 +23,7 @@
#include <linux/semaphore.h>
#include <linux/slab.h>
#include <linux/sched.h>
+#include <linux/random.h>
#include <linux/vmalloc.h>
#define MAX_ENTRIES 1000000
@@ -66,6 +67,11 @@ struct test_obj {
struct rhash_head node;
};
+struct test_obj_rhl {
+ struct test_obj_val value;
+ struct rhlist_head list_node;
+};
+
struct thread_data {
unsigned int entries;
int id;
@@ -246,6 +252,186 @@ static s64 __init test_rhashtable(struct rhashtable *ht, struct test_obj *array,
static struct rhashtable ht;
+static int __init test_rhltable(unsigned int entries)
+{
+ struct test_obj_rhl *rhl_test_objects;
+ unsigned long *obj_in_table;
+ struct rhltable rhlt;
+ unsigned int i, j, k;
+ int ret, err;
+
+ if (entries == 0)
+ entries = 1;
+
+ rhl_test_objects = vzalloc(sizeof(*rhl_test_objects) * entries);
+ if (!rhl_test_objects)
+ return -ENOMEM;
+
+ ret = -ENOMEM;
+ obj_in_table = vzalloc(BITS_TO_LONGS(entries) * sizeof(unsigned long));
+ if (!obj_in_table)
+ goto out_free;
+
+ /* nulls_base not supported in rhlist interface */
+ test_rht_params.nulls_base = 0;
+ err = rhltable_init(&rhlt, &test_rht_params);
+ if (WARN_ON(err))
+ goto out_free;
+
+ k = prandom_u32();
+ ret = 0;
+ for (i = 0; i < entries; i++) {
+ rhl_test_objects[i].value.id = k;
+ err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node,
+ test_rht_params);
+ if (WARN(err, "error %d on element %d\n", err, i))
+ break;
+ if (err == 0)
+ set_bit(i, obj_in_table);
+ }
+
+ if (err)
+ ret = err;
+
+ pr_info("test %d add/delete pairs into rhlist\n", entries);
+ for (i = 0; i < entries; i++) {
+ struct rhlist_head *h, *pos;
+ struct test_obj_rhl *obj;
+ struct test_obj_val key = {
+ .id = k,
+ };
+ bool found;
+
+ rcu_read_lock();
+ h = rhltable_lookup(&rhlt, &key, test_rht_params);
+ if (WARN(!h, "key not found during iteration %d of %d", i, entries)) {
+ rcu_read_unlock();
+ break;
+ }
+
+ if (i) {
+ j = i - 1;
+ rhl_for_each_entry_rcu(obj, pos, h, list_node) {
+ if (WARN(pos == &rhl_test_objects[j].list_node, "old element found, should be gone"))
+ break;
+ }
+ }
+
+ cond_resched_rcu();
+
+ found = false;
+
+ rhl_for_each_entry_rcu(obj, pos, h, list_node) {
+ if (pos == &rhl_test_objects[i].list_node) {
+ found = true;
+ break;
+ }
+ }
+
+ rcu_read_unlock();
+
+ if (WARN(!found, "element %d not found", i))
+ break;
+
+ err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+ WARN(err, "rhltable_remove: err %d for iteration %d\n", err, i);
+ if (err == 0)
+ clear_bit(i, obj_in_table);
+ }
+
+ if (ret == 0 && err)
+ ret = err;
+
+ for (i = 0; i < entries; i++) {
+ WARN(test_bit(i, obj_in_table), "elem %d allegedly still present", i);
+
+ err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node,
+ test_rht_params);
+ if (WARN(err, "error %d on element %d\n", err, i))
+ break;
+ if (err == 0)
+ set_bit(i, obj_in_table);
+ }
+
+ pr_info("test %d random rhlist add/delete operations\n", entries);
+ for (j = 0; j < entries; j++) {
+ u32 i = prandom_u32_max(entries);
+ u32 prand = prandom_u32();
+
+ cond_resched();
+
+ if (prand == 0)
+ prand = prandom_u32();
+
+ if (prand & 1) {
+ prand >>= 1;
+ continue;
+ }
+
+ err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+ if (test_bit(i, obj_in_table)) {
+ clear_bit(i, obj_in_table);
+ if (WARN(err, "cannot remove element at slot %d", i))
+ continue;
+ } else {
+ if (WARN(err != -ENOENT, "removed non-existant element %d, error %d not %d",
+ i, err, -ENOENT))
+ continue;
+ }
+
+ if (prand & 1) {
+ prand >>= 1;
+ continue;
+ }
+
+ err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+ if (err == 0) {
+ if (WARN(test_and_set_bit(i, obj_in_table), "succeeded to insert same object %d", i))
+ continue;
+ } else {
+ if (WARN(!test_bit(i, obj_in_table), "failed to insert object %d", i))
+ continue;
+ }
+
+ if (prand & 1) {
+ prand >>= 1;
+ continue;
+ }
+
+ i = prandom_u32_max(entries);
+ if (test_bit(i, obj_in_table)) {
+ err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+ WARN(err, "cannot remove element at slot %d", i);
+ if (err == 0)
+ clear_bit(i, obj_in_table);
+ } else {
+ err = rhltable_insert(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+ WARN(err, "failed to insert object %d", i);
+ if (err == 0)
+ set_bit(i, obj_in_table);
+ }
+ }
+
+ for (i = 0; i < entries; i++) {
+ cond_resched();
+ err = rhltable_remove(&rhlt, &rhl_test_objects[i].list_node, test_rht_params);
+ if (test_bit(i, obj_in_table)) {
+ if (WARN(err, "cannot remove element at slot %d", i))
+ continue;
+ } else {
+ if (WARN(err != -ENOENT, "removed non-existant element, error %d not %d",
+ err, -ENOENT))
+ continue;
+ }
+ }
+
+ rhltable_destroy(&rhlt);
+out_free:
+ vfree(rhl_test_objects);
+ vfree(obj_in_table);
+ return ret;
+}
+
static int __init test_rhashtable_max(struct test_obj *array,
unsigned int entries)
{
@@ -480,11 +666,17 @@ static int __init test_rht_init(void)
failed_threads++;
}
}
- pr_info("Started %d threads, %d failed\n",
- started_threads, failed_threads);
rhashtable_destroy(&ht);
vfree(tdata);
vfree(objs);
+
+ /*
+ * rhltable_remove is very expensive, default values can cause test
+ * to run for 2 minutes or more, use a smaller number instead.
+ */
+ err = test_rhltable(entries / 16);
+ pr_info("Started %d threads, %d failed, rhltable test returns %d\n",
+ started_threads, failed_threads, err);
return 0;
}
--
2.13.5
^ permalink raw reply related
* Re: RACK not getting disabled
From: Eric Dumazet @ 2017-09-18 21:18 UTC (permalink / raw)
To: hiren panchasara; +Cc: netdev
In-Reply-To: <20170918201428.GB28186@strugglingcoder.info>
On Mon, 2017-09-18 at 13:14 -0700, hiren panchasara wrote:
> Hi all, I am trying to disable rack to see 3dupacks in action during
> loss-detection but based on the pcap, I see that it's still trigger
> loss-recovery on the first SACK (as if RACK is still enabled/active).
>
> Here is what I did to disable rack:
> net.ipv4.tcp_recovery = 0
>
> I've also disabled metrics:
> net.ipv4.tcp_no_metrics_save = 1
> And also flushed existing entries with 'ip tcp_metrics flush' just to be
> on a safer side.
>
> Not really relevant here but I've also switched to reno.
>
> I am on: 4.10.0-33-generic
> pcap: https://transfer.sh/mfoiN/reno_no_rack.pcap
>
> What am I missing? I can provide any additional info.
>
> Cheers,
> Hiren
A single SACK can contains enough information to trigger a retransmit.
If you absolutely want to see the old 3 dupack in action, you also want
to disable SACK.
^ permalink raw reply
* Re: RACK not getting disabled
From: hiren panchasara @ 2017-09-18 21:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1505769494.29839.34.camel@edumazet-glaptop3.roam.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
On 09/18/17 at 02:18P, Eric Dumazet wrote:
> On Mon, 2017-09-18 at 13:14 -0700, hiren panchasara wrote:
> > Hi all, I am trying to disable rack to see 3dupacks in action during
> > loss-detection but based on the pcap, I see that it's still trigger
> > loss-recovery on the first SACK (as if RACK is still enabled/active).
> >
> > Here is what I did to disable rack:
> > net.ipv4.tcp_recovery = 0
> >
> > I've also disabled metrics:
> > net.ipv4.tcp_no_metrics_save = 1
> > And also flushed existing entries with 'ip tcp_metrics flush' just to be
> > on a safer side.
> >
> > Not really relevant here but I've also switched to reno.
> >
> > I am on: 4.10.0-33-generic
> > pcap: https://transfer.sh/mfoiN/reno_no_rack.pcap
> >
> > What am I missing? I can provide any additional info.
> >
> > Cheers,
> > Hiren
>
>
> A single SACK can contains enough information to trigger a retransmit.
Bah, right. FACK!
>
> If you absolutely want to see the old 3 dupack in action, you also want
> to disable SACK.
I believe net.ipv4.tcp_fack = 0 would achieve that without disabling
sack.
Thanks for your help!
Cheers,
Hiren
[-- Attachment #2: Type: application/pgp-signature, Size: 603 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] selftests: silence test output by default
From: Shuah Khan @ 2017-09-18 21:36 UTC (permalink / raw)
To: Josef Bacik
Cc: Shuah Khan, Josef Bacik, David S. Miller, linux-kernel,
linux-kselftest, netdev, Shuah Khan
In-Reply-To: <20170918201905.qtax6kn6iscrebwa@destiny>
On 09/18/2017 02:19 PM, Josef Bacik wrote:
> On Mon, Sep 18, 2017 at 01:48:31PM -0600, Shuah Khan wrote:
>> On 09/18/2017 12:24 PM, Josef Bacik wrote:
>>> On Mon, Sep 18, 2017 at 12:13:40PM -0600, Shuah Khan wrote:
>>>> On 09/18/2017 11:52 AM, Josef Bacik wrote:
>>>>> On Mon, Sep 18, 2017 at 11:46:18AM -0600, Shuah Khan wrote:
>>>>>> On 09/18/2017 11:37 AM, josef@toxicpanda.com wrote:
>>>>>>> From: Josef Bacik <jbacik@fb.com>
>>>>>>>
>>>>>>> Some of the networking tests are very noisy and make it impossible to
>>>>>>> see if we actually passed the tests as they run. Default to suppressing
>>>>>>> the output from any tests run in order to make it easier to track what
>>>>>>> failed.
>>>>>>>
>>>>>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>>>>>> --
>>>>>>
>>>>>> This change suppresses pass/fail wrapper output for all tests, not just the
>>>>>> networking tests.
>>>>>>
>>>>>> Could you please send me before and after results for what you are trying
>>>>>> to fix.
>>>>>>
>>>>>
>>>>> Yeah I wanted to suppress extraneous output from everybody, I just happened to
>>>>> notice it because I was testing net. The default thing already spits out what
>>>>> it's running and pass/fail, there's no need to include all of the random output
>>>>> unless the user wants to go and run the test manually. As it is now it's
>>>>> _impossible_ to tell what ran and what passed/failed because of all the random
>>>>> output.
>>>>
>>>> Unfortunately kselftests have lots of users that want different things. A recent
>>>> request is to use TAP13 format for output for external parsers to be able to parse.
>>>> That is what this change to add TAP13 header does.
>>>>
>>>> The output you are seeing is the TAP 13 format to indicate the test has passed.
>>>>
>>>> The right fix would be to suppress the Pass/Fail from the individual shell script
>>>> and have the shell script exit with error code. kselftest lib.mk will handle the
>>>> error code and print out pass/fail like it is doing now.
>>>>
>>>> Using the common logic will help avoid duplicate code in tests/test scripts and
>>>> also makes the pass/fail messages consistent.
>>>>
>>>> In the following output the individual test output can be eliminated since lib.mk
>>>> run_tests does that for you. In addition, you will also get a count of tests at
>>>> the end of the run of all tests in a test directory.
>>>>
>>>> TAP version 13
>>>> selftests: run_netsocktests
>>>> ========================================
>>>> --------------------
>>>> running socket test
>>>> --------------------
>>>> [PASS]
>>>> ok 1..1 selftests: run_netsocktests [PASS]
>>>> selftests: run_afpackettests
>>>> ========================================
>>>> must be run as root
>>>> ok 1..2 selftests: run_afpackettests [PASS]
>>>> selftests: test_bpf.sh
>>>> ========================================
>>>> test_bpf: [FAIL]
>>>> not ok 1..3 selftests: test_bpf.sh [FAIL]
>>>> selftests: netdevice.sh
>>>> ========================================
>>>> SKIP: Need root privileges
>>>> ok 1..4 selftests: netdevice.sh [PASS]
>>>>
>>>> If you eliminate that you will just see the common lib.mk results.
>>>>
>>>> TAP version 13
>>>> selftests: run_netsocktests
>>>> ========================================
>>>> ok 1..1 selftests: run_netsocktests [PASS]
>>>> selftests: run_afpackettests
>>>> ========================================
>>>> must be run as root
>>>> ok 1..2 selftests: run_afpackettests [PASS]
>>>> ========================================
>>>> selftests: test_bpf.sh
>>>> ========================================
>>>> not ok 1..3 selftests: test_bpf.sh [FAIL]
>>>> selftests: netdevice.sh
>>>> ========================================
>>>> SKIP: Need root privileges
>>>> ok 1..4 selftests: netdevice.sh [PASS]
>>>>
>>>>
>>>> If you would like to fix the duplicate output, please send me patches
>>>> to remove pass/fail output strings from tests instead. It is on my
>>>> todo to do that this release.
>>>>
>>>
>>> I'm confused, this is exactly what my patch does, it strips all of the
>>> extraneous output and leaves only the TAP13 output. Here is the output without
>>> my suppression patch
>>>
>>> https://da.gd/pup0
>>>
>>> and here is the output with my suppression patch
>>>
>>> https://da.gd/3olKj
>>>
>>> Unless I'm missing something subtle it appears to be exactly the output you
>>> want, without the random crap from the other tests. The only thing I'm
>>> redirecting is the output of the _test_ itself, $$BASENAME_TEST from what I can
>>> tell is the actual test we're running, not the wrapper, so everything is as it
>>> should be. Thanks,
>>>
>>> Josef
>>>
>>
>> Yes you are right. Yes I see that you are redirecting all
>> output from the test with (./$$BASENAME_TEST > /dev/null 2>&1)
>> My bad.
>>
>> cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
>>
>> However, it also suppresses important error messages from individual tests
>> like in the case of rtnetlink.sh from your before and after output.
>>
>> User knows the test failed, but doesn't know why. One way to solve
>> the problem is creating a temp file with the output for reference.
>> Something like:
>>
>> (./$$BASENAME_TEST > /tmp/kselftest.out 2>&1
>>
>>
>
> So I didn't do a global file because we'd have to append to it to make sure we
> didn't lose any history. If this doesn't bother you I can do that instead, and
> then just remove the file whenever we start running things. Let me know if
> that's what you would prefer. Thanks,
>
> Josef
>
Right. Let's not worry about global file. Let's just use /tmp/$$BASENAME_TEST
for now. Something like the below diff:
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 223234cd98e9..317393e430c9 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -24,7 +24,8 @@ define RUN_TESTS
echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
else \
- cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
+ echo "Please check /tmp/$$BASENAME_TEST for full test output"; \
+ cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /tmp/$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
fi; \
done;
endef
thanks,
-- Shuah
^ permalink raw reply related
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Yuchung Cheng @ 2017-09-18 21:40 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <2149381.XGPd7soC9e@natalenko.name>
On Mon, Sep 18, 2017 at 1:46 PM, Oleksandr Natalenko
<oleksandr@natalenko.name> wrote:
> Actually, same warning was just triggered with RACK enabled. But main warning
> was not triggered in this case.
Thanks.
I assume this kernel does not have the patch that Neal proposed in his
first reply?
The main warning needs to be triggered by another peculiar SACK that
kicks the sender into recovery again (after undo). Please let it run
longer if possible to see if we can get both. But the new data does
indicate the we can (validly) be in CA_Open with retrans_out > 0.
>
> ===
> Sep 18 22:44:32 defiant kernel: ------------[ cut here ]------------
> Sep 18 22:44:32 defiant kernel: WARNING: CPU: 1 PID: 702 at net/ipv4/
> tcp_input.c:2392 tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 18 22:44:32 defiant kernel: Modules linked in: netconsole ctr ccm cls_bpf
> sch_htb act_mirred cls_u32 sch_ingress sit tunnel4 ip_tunnel 8021q mrp
> nf_conntrack_ipv6 nf_defrag_ipv6 nft_ct nft_set_bitmap nft_set_hash
> nft_set_rbtree nf_tables_inet nf_tables_ipv6 nft_masq_ipv4
> nf_nat_masquerade_ipv4 nft_masq nft_nat nft_counter nft_meta
> nft_chain_nat_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat
> nf_conntrack libcrc32c crc32c_generic nf_tables_ipv4 nf_tables tun nct6775
> nfnetlink hwmon_vid nls_iso8859_1 nls_cp437 vfat fat ext4 snd_hda_codec_hdmi
> mbcache jbd2 snd_hda_codec_realtek snd_hda_codec_generic f2fs arc4 fscrypto
> intel_rapl iTCO_wdt ath9k iTCO_vendor_support intel_powerclamp ath9k_common
> ath9k_hw coretemp kvm_intel ath mac80211 kvm irqbypass intel_cstate cfg80211
> pcspkr snd_hda_intel snd_hda_codec r8169
> Sep 18 22:44:32 defiant kernel: joydev evdev mii snd_hda_core mousedev
> mei_txe input_leds i2c_i801 mac_hid i915 lpc_ich mei shpchp snd_hwdep
> snd_intel_sst_acpi snd_intel_sst_core snd_soc_rt5670
> snd_soc_sst_atom_hifi2_platform battery snd_soc_sst_match snd_soc_rl6231
> drm_kms_helper hci_uart ov5693(C) ov2722(C) lm3554(C) btbcm btqca v4l2_common
> snd_soc_core btintel snd_compress videodev snd_pcm_dmaengine snd_pcm video
> bluetooth snd_timer drm media tpm_tis snd i2c_hid soundcore tpm_tis_core
> rfkill_gpio ac97_bus soc_button_array ecdh_generic rfkill crc16 tpm 8250_dw
> intel_gtt syscopyarea sysfillrect acpi_pad sysimgblt intel_int0002_vgpio
> fb_sys_fops pinctrl_cherryview i2c_algo_bit button sch_fq_codel tcp_bbr ifb
> ip_tables x_tables btrfs xor raid6_pq algif_skcipher af_alg hid_logitech_hidpp
> hid_logitech_dj usbhid hid uas
> Sep 18 22:44:32 defiant kernel: usb_storage dm_crypt dm_mod dax raid10 md_mod
> sd_mod crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel pcbc
> ahci aesni_intel xhci_pci libahci aes_x86_64 crypto_simd glue_helper xhci_hcd
> cryptd libata usbcore scsi_mod usb_common serio sdhci_acpi sdhci led_class
> mmc_core
> Sep 18 22:44:32 defiant kernel: CPU: 1 PID: 702 Comm: irq/123-enp3s0 Tainted:
> G WC 4.13.0-pf4 #1
> Sep 18 22:44:32 defiant kernel: Hardware name: To Be Filled By O.E.M. To Be
> Filled By O.E.M./J3710-ITX, BIOS P1.30 03/30/2016
> Sep 18 22:44:32 defiant kernel: task: ffff88923a738000 task.stack:
> ffff958001500000
> Sep 18 22:44:32 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 18 22:44:32 defiant kernel: RSP: 0018:ffff88927fc83a48 EFLAGS: 00010202
> Sep 18 22:44:32 defiant kernel: RAX: 0000000000000001 RBX: ffff8892412d9800
> RCX: ffff88927fc83b0c
> Sep 18 22:44:32 defiant kernel: RDX: 000000007fffffff RSI: 0000000000000001
> RDI: ffff8892412d9800
> Sep 18 22:44:32 defiant kernel: RBP: ffff88927fc83a50 R08: 0000000000000000
> R09: 0000000018dfb063
> Sep 18 22:44:32 defiant kernel: R10: 0000000018dfd223 R11: 0000000018dfb063
> R12: 0000000000005320
> Sep 18 22:44:32 defiant kernel: R13: ffff88927fc83b10 R14: 0000000000000001
> R15: ffff88927fc83b0c
> Sep 18 22:44:32 defiant kernel: FS: 0000000000000000(0000)
> GS:ffff88927fc80000(0000) knlGS:0000000000000000
> Sep 18 22:44:32 defiant kernel: CS: 0010 DS: 0000 ES: 0000 CR0:
> 0000000080050033
> Sep 18 22:44:32 defiant kernel: CR2: 00007f1cd1a43620 CR3: 0000000114a09000
> CR4: 00000000001006e0
> Sep 18 22:44:32 defiant kernel: Call Trace:
> Sep 18 22:44:32 defiant kernel: <IRQ>
> Sep 18 22:44:32 defiant kernel: tcp_try_undo_loss+0xb3/0xf0
> Sep 18 22:44:32 defiant kernel: tcp_fastretrans_alert+0x746/0x990
> Sep 18 22:44:32 defiant kernel: tcp_ack+0x741/0x1110
> Sep 18 22:44:32 defiant kernel: tcp_rcv_established+0x325/0x770
> Sep 18 22:44:32 defiant kernel: ? sk_filter_trim_cap+0xd4/0x1a0
> Sep 18 22:44:32 defiant kernel: tcp_v4_do_rcv+0x90/0x1e0
> Sep 18 22:44:32 defiant kernel: tcp_v4_rcv+0x950/0xa10
> Sep 18 22:44:32 defiant kernel: ? nf_ct_deliver_cached_events+0xb8/0x110
> [nf_conntrack]
> Sep 18 22:44:32 defiant kernel: ip_local_deliver_finish+0x68/0x210
> Sep 18 22:44:32 defiant kernel: ip_local_deliver+0xfa/0x110
> Sep 18 22:44:32 defiant kernel: ? ip_rcv_finish+0x410/0x410
> Sep 18 22:44:32 defiant kernel: ip_rcv_finish+0x120/0x410
> Sep 18 22:44:32 defiant kernel: ip_rcv+0x28e/0x3b0
> Sep 18 22:44:32 defiant kernel: ? inet_del_offload+0x40/0x40
> Sep 18 22:44:32 defiant kernel: __netif_receive_skb_core+0x39b/0xb00
> Sep 18 22:44:32 defiant kernel: ? netif_receive_skb_internal+0xa0/0x480
> Sep 18 22:44:32 defiant kernel: ? dev_gro_receive+0x2eb/0x4a0
> Sep 18 22:44:32 defiant kernel: __netif_receive_skb+0x18/0x60
> Sep 18 22:44:32 defiant kernel: netif_receive_skb_internal+0x98/0x480
> Sep 18 22:44:32 defiant kernel: netif_receive_skb+0x1c/0x80
> Sep 18 22:44:32 defiant kernel: ifb_ri_tasklet+0x109/0x26a [ifb]
> Sep 18 22:44:32 defiant kernel: tasklet_action+0x63/0x120
> Sep 18 22:44:32 defiant kernel: __do_softirq+0xdf/0x2e5
> Sep 18 22:44:32 defiant kernel: ? irq_finalize_oneshot.part.39+0xe0/0xe0
> Sep 18 22:44:32 defiant kernel: do_softirq_own_stack+0x1c/0x30
> Sep 18 22:44:32 defiant kernel: </IRQ>
> Sep 18 22:44:32 defiant kernel: do_softirq.part.17+0x4e/0x60
> Sep 18 22:44:32 defiant kernel: __local_bh_enable_ip+0x77/0x80
> Sep 18 22:44:32 defiant kernel: irq_forced_thread_fn+0x5c/0x70
> Sep 18 22:44:32 defiant kernel: irq_thread+0x131/0x1a0
> Sep 18 22:44:32 defiant kernel: ? wake_threads_waitq+0x30/0x30
> Sep 18 22:44:32 defiant kernel: kthread+0x126/0x140
> Sep 18 22:44:32 defiant kernel: ? irq_thread_check_affinity+0x90/0x90
> Sep 18 22:44:32 defiant kernel: ? kthread_create_on_node+0x70/0x70
> Sep 18 22:44:32 defiant kernel: ret_from_fork+0x25/0x30
> Sep 18 22:44:32 defiant kernel: Code: 5d c3 80 60 35 fb 48 8b 00 48 39 c2 74
> 85 48 3b 83 50 01 00 00 75 eb e9 77 ff ff ff 89 83 48 06 00 00 80 a3 1e 06 00
> 00 fb eb b3 <0f> ff 5b 5d c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f
> Sep 18 22:44:32 defiant kernel: ---[ end trace 1aea180efeedb474 ]---
> ===
>
> On pondělí 18. září 2017 20:01:42 CEST Yuchung Cheng wrote:
>> On Mon, Sep 18, 2017 at 10:59 AM, Oleksandr Natalenko
>>
>> <oleksandr@natalenko.name> wrote:
>> > OK. Should I keep FACK disabled?
>>
>> Yes since it is disabled in the upstream by default. Although you can
>> experiment FACK enabled additionally.
>>
>> Do we know the crash you first experienced is tied to this issue?
>>
>> > On pondělí 18. září 2017 19:51:21 CEST Yuchung Cheng wrote:
>> >> Can you try this patch to verify my theory with tcp_recovery=0 and 1?
>> >> thanks
>> >>
>> >> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> >> index 5af2f04f8859..9253d9ee7d0e 100644
>> >> --- a/net/ipv4/tcp_input.c
>> >> +++ b/net/ipv4/tcp_input.c
>> >> @@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
>> >> *sk, bool unmark_loss)
>> >>
>> >> }
>> >> tp->snd_cwnd_stamp = tcp_time_stamp;
>> >> tp->undo_marker = 0;
>> >>
>> >> + WARN_ON(tp->retrans_out);
>> >>
>> >> }
>
>
^ permalink raw reply
* [PATCH net-next 00/12] net: dsa: b53/bcm_sf2 cleanups
From: Florian Fainelli @ 2017-09-18 21:41 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
Hi all,
This patch series is a first pass set of clean-ups to reduce the number of LOCs
between b53 and bcm_sf2 and sharing as many functions as possible.
There is a number of additional cleanups queued up locally that require more
thorough testing.
Thanks!
Florian Fainelli (12):
net: dsa: b53: Remove is_cpu_port()
net: dsa: b53: Make b53_enable_cpu_port() take a port argument
net: dsa: b53: Defer port enabling to calling port_enable
net: dsa: bcm_sf2: Defer port enabling to calling port_enable
net: dsa: b53: Use a macro to define I/O operations
net: dsa: b53: Move Broadcom header setup to b53
net: dsa: b53: Define EEE register page
net: dsa: b53: Move EEE functions to b53
net: dsa: b53: Wire-up EEE
net: dsa: b53: Export b53_imp_vlan_setup()
net: dsa: bcm_sf2: Use SF2_NUM_EGRESS_QUEUES for CFP
net: dsa: bcm_sf2: Utilize b53_{enable,disable}_port
drivers/net/dsa/b53/b53_common.c | 150 ++++++++++++++++++++++++++++++++----
drivers/net/dsa/b53/b53_priv.h | 145 ++++++++---------------------------
drivers/net/dsa/b53/b53_regs.h | 48 ++++++++++++
drivers/net/dsa/bcm_sf2.c | 161 +++------------------------------------
drivers/net/dsa/bcm_sf2.h | 2 -
drivers/net/dsa/bcm_sf2_cfp.c | 6 +-
drivers/net/dsa/bcm_sf2_regs.h | 11 ---
7 files changed, 227 insertions(+), 296 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH net-next 01/12] net: dsa: b53: Remove is_cpu_port()
From: Florian Fainelli @ 2017-09-18 21:41 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170918214128.27896-1-f.fainelli@gmail.com>
This is not used anywhere, so remove it.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_priv.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 01bd8cbe9a3f..7528b22aeb03 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -186,11 +186,6 @@ static inline int is58xx(struct b53_device *dev)
#define B53_CPU_PORT_25 5
#define B53_CPU_PORT 8
-static inline int is_cpu_port(struct b53_device *dev, int port)
-{
- return dev->cpu_port;
-}
-
struct b53_device *b53_switch_alloc(struct device *base,
const struct b53_io_ops *ops,
void *priv);
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 02/12] net: dsa: b53: Make b53_enable_cpu_port() take a port argument
From: Florian Fainelli @ 2017-09-18 21:41 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170918214128.27896-1-f.fainelli@gmail.com>
In preparation for future changes allowing the configuring of multiple
CPU ports, make b53_enable_cpu_port() take a port argument.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 274f3679f33d..d8bc54cfcfbe 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -538,19 +538,18 @@ static void b53_disable_port(struct dsa_switch *ds, int port,
b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), reg);
}
-static void b53_enable_cpu_port(struct b53_device *dev)
+static void b53_enable_cpu_port(struct b53_device *dev, int port)
{
- unsigned int cpu_port = dev->cpu_port;
u8 port_ctrl;
/* BCM5325 CPU port is at 8 */
- if ((is5325(dev) || is5365(dev)) && cpu_port == B53_CPU_PORT_25)
- cpu_port = B53_CPU_PORT;
+ if ((is5325(dev) || is5365(dev)) && port == B53_CPU_PORT_25)
+ port = B53_CPU_PORT;
port_ctrl = PORT_CTRL_RX_BCST_EN |
PORT_CTRL_RX_MCST_EN |
PORT_CTRL_RX_UCST_EN;
- b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(cpu_port), port_ctrl);
+ b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), port_ctrl);
}
static void b53_enable_mib(struct b53_device *dev)
@@ -820,7 +819,7 @@ static int b53_setup(struct dsa_switch *ds)
if (BIT(port) & ds->enabled_port_mask)
b53_enable_port(ds, port, NULL);
else if (dsa_is_cpu_port(ds, port))
- b53_enable_cpu_port(dev);
+ b53_enable_cpu_port(dev, port);
else
b53_disable_port(ds, port, NULL);
}
--
2.9.3
^ permalink raw reply related
* [PATCH net-next 03/12] net: dsa: b53: Defer port enabling to calling port_enable
From: Florian Fainelli @ 2017-09-18 21:41 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170918214128.27896-1-f.fainelli@gmail.com>
There is no need to configure the enabled ports once in b53_setup() and then a
second time around when dsa_switch_ops::port_enable is called, just do it when
port_enable is called which is better in terms of power consumption and
correctness.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index d8bc54cfcfbe..3297af6aab8a 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -815,12 +815,13 @@ static int b53_setup(struct dsa_switch *ds)
if (ret)
dev_err(ds->dev, "failed to apply configuration\n");
+ /* Configure IMP/CPU port, disable unused ports. Enabled
+ * ports will be configured with .port_enable
+ */
for (port = 0; port < dev->num_ports; port++) {
- if (BIT(port) & ds->enabled_port_mask)
- b53_enable_port(ds, port, NULL);
- else if (dsa_is_cpu_port(ds, port))
+ if (dsa_is_cpu_port(ds, port))
b53_enable_cpu_port(dev, port);
- else
+ else if (!(BIT(port) & ds->enabled_port_mask))
b53_disable_port(ds, port, NULL);
}
--
2.9.3
^ permalink raw reply related
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