* 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
* 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: [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
* 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 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
* 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
* [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
* [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 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
* 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
* Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")
From: Eric Dumazet @ 2017-09-18 19:52 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Eric Dumazet, David S. Miller, netdev, LKML
In-Reply-To: <20170918194630.GH4914@intel.com>
On Mon, Sep 18, 2017 at 12:46 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> And five months later I'm still waiting for this patch to land...
Sure, while I was on vacation, I received thousands of emails that I
simply have not read.
^ permalink raw reply
* Re: [PATCH 3/3] selftests: silence test output by default
From: Shuah Khan @ 2017-09-18 19:48 UTC (permalink / raw)
To: Josef Bacik
Cc: Shuah Khan, Josef Bacik, David S. Miller, linux-kernel,
linux-kselftest, netdev, Shuah Khan
In-Reply-To: <20170918182426.cs4wxy3igfx7zyvb@destiny>
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
Without the change:
ok 1..10 selftests: netdevice.sh [PASS]
selftests: rtnetlink.sh
========================================
RTNETLINK answers: Operation not supported
Cannot find device "test-dummy0"
FAIL: cannot add dummy interface
not ok 1..11 selftests: rtnetlink.sh [FAIL]
selftests: rtnetlink.sh
========================================
not ok 1..11 selftests: rtnetlink.sh [FAIL]
thanks,
-- Shuah
^ permalink raw reply
* Re: [regression v4.11] 617f01211baf ("8139too: use napi_complete_done()")
From: Ville Syrjälä @ 2017-09-18 19:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David S. Miller, netdev, linux-kernel
In-Reply-To: <20170410121102.GM30290@intel.com>
On Mon, Apr 10, 2017 at 03:11:02PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 07, 2017 at 11:38:49AM -0700, Eric Dumazet wrote:
> > On Fri, 2017-04-07 at 21:17 +0300, Ville Syrjälä wrote:
> > > Hi,
> > >
> > > My old P3 laptop started to die on me in the middle of larger compile
> > > jobs (using distcc) after v4.11-rc<something>. I bisected the problem
> > > to 617f01211baf ("8139too: use napi_complete_done()").
> > >
> > > Unfortunately I wasn't able to capture a full oops as the machine doesn't
> > > have serial and ramoops failed me. I did get one partial oops on vgacon
> > > which showed rtl8139_poll() being involved (EIP was around
> > > _raw_spin_unlock_irqrestore() supposedly), so seems to agree with my
> > > bisect result.
> > >
> > > So maybe some kind of nasty thing going between the hard irq and
> > > softirq? Perhaps UP related? I tried to stare at the locking around
> > > rtl8139_poll() for a while but it looked mostly sane to me.
> > >
> >
> > Thanks a lot for the detective work, I am so sorry for this !
> >
> > Could you try the following patch ?
> >
> > I do not really see what could be wrong, the code should run just fine
> > on UP.
> >
> > Thanks.
> >
> > 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);
> >
> >
>
> Yep, that patch does appear to make it stable again.
>
> Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
And five months later I'm still waiting for this patch to land...
--
Ville Syrjälä
Intel OTC
^ permalink raw reply
* RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Tristram.Ha @ 2017-09-18 19:38 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170907214834.GT11248@lunn.ch>
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.
The KSZ9477 driver has its own problem as the tail tag length is dynamic
and not fixed. Right now the function feature that affects this behavior is
not turned on and so the problem does not happen.
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? I thought this bit is important for the new switchdev model.
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.
^ permalink raw reply
* [PATCH net-next] net_sched: sch_htb: add per class overlimits counter
From: Eric Dumazet @ 2017-09-18 19:36 UTC (permalink / raw)
To: David Miller
Cc: netdev, Denys Fedoryshchenko, Jamal Hadi Salim, Cong Wang,
Jiri Pirko
From: Eric Dumazet <edumazet@google.com>
HTB qdisc overlimits counter is properly increased, but we have no per
class counter, meaning it is difficult to diagnose HTB problems.
This patch adds this counter, visible in "tc -s class show dev eth0",
with current iproute2.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Denys Fedoryshchenko <nuclearcat@nuclearcat.com>
---
We have used this at Google for few years already, time to ship it !
net/sched/sch_htb.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 7e148376ba528efabe5a53a09653f9161c264be7..c6d7ae81b41f4e277afb93a3003fefcd3f27de35 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -142,6 +142,7 @@ struct htb_class {
struct rb_node node[TC_HTB_NUMPRIO]; /* node for self or feed tree */
unsigned int drops ____cacheline_aligned_in_smp;
+ unsigned int overlimits;
};
struct htb_level {
@@ -533,6 +534,9 @@ htb_change_class_mode(struct htb_sched *q, struct htb_class *cl, s64 *diff)
if (new_mode == cl->cmode)
return;
+ if (new_mode == HTB_CANT_SEND)
+ cl->overlimits++;
+
if (cl->prio_activity) { /* not necessary: speed optimization */
if (cl->cmode != HTB_CANT_SEND)
htb_deactivate_prios(q, cl);
@@ -1143,6 +1147,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
struct htb_class *cl = (struct htb_class *)arg;
struct gnet_stats_queue qs = {
.drops = cl->drops,
+ .overlimits = cl->overlimits,
};
__u32 qlen = 0;
^ permalink raw reply related
* [PATCH net-next 3/3] bpf: Test deletion in BPF_MAP_TYPE_LPM_TRIE
From: Craig Gallek @ 2017-09-18 19:30 UTC (permalink / raw)
To: Daniel Mack, Alexei Starovoitov, Daniel Borkmann,
David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-1-kraigatgoog@gmail.com>
From: Craig Gallek <kraig@google.com>
Extend the 'random' operation tests to include a delete operation
(delete half of the nodes from both lpm implementions and ensure
that lookups are still equivalent).
Also, add a simple IPv4 test which verifies lookup behavior as nodes
are deleted from the tree.
Signed-off-by: Craig Gallek <kraig@google.com>
---
tools/testing/selftests/bpf/test_lpm_map.c | 187 ++++++++++++++++++++++++++++-
1 file changed, 183 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_lpm_map.c b/tools/testing/selftests/bpf/test_lpm_map.c
index a5ed7c4f1819..6fedb9fec36b 100644
--- a/tools/testing/selftests/bpf/test_lpm_map.c
+++ b/tools/testing/selftests/bpf/test_lpm_map.c
@@ -104,6 +104,34 @@ static struct tlpm_node *tlpm_match(struct tlpm_node *list,
return best;
}
+static struct tlpm_node *tlpm_delete(struct tlpm_node *list,
+ const uint8_t *key,
+ size_t n_bits)
+{
+ struct tlpm_node *best = tlpm_match(list, key, n_bits);
+ struct tlpm_node *node;
+
+ if (!best || best->n_bits != n_bits)
+ return list;
+
+ if (best == list) {
+ node = best->next;
+ free(best);
+ return node;
+ }
+
+ for (node = list; node; node = node->next) {
+ if (node->next == best) {
+ node->next = best->next;
+ free(best);
+ return list;
+ }
+ }
+ /* should never get here */
+ assert(0);
+ return list;
+}
+
static void test_lpm_basic(void)
{
struct tlpm_node *list = NULL, *t1, *t2;
@@ -126,6 +154,13 @@ static void test_lpm_basic(void)
assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff, 0xff }, 15));
assert(!tlpm_match(list, (uint8_t[]){ 0x7f, 0xff }, 16));
+ list = tlpm_delete(list, (uint8_t[]){ 0xff, 0xff }, 16);
+ assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff }, 8));
+ assert(t1 == tlpm_match(list, (uint8_t[]){ 0xff, 0xff }, 16));
+
+ list = tlpm_delete(list, (uint8_t[]){ 0xff }, 8);
+ assert(!tlpm_match(list, (uint8_t[]){ 0xff }, 8));
+
tlpm_clear(list);
}
@@ -170,7 +205,7 @@ static void test_lpm_order(void)
static void test_lpm_map(int keysize)
{
- size_t i, j, n_matches, n_nodes, n_lookups;
+ size_t i, j, n_matches, n_matches_after_delete, n_nodes, n_lookups;
struct tlpm_node *t, *list = NULL;
struct bpf_lpm_trie_key *key;
uint8_t *data, *value;
@@ -182,6 +217,7 @@ static void test_lpm_map(int keysize)
*/
n_matches = 0;
+ n_matches_after_delete = 0;
n_nodes = 1 << 8;
n_lookups = 1 << 16;
@@ -235,15 +271,54 @@ static void test_lpm_map(int keysize)
}
}
+ /* Remove the first half of the elements in the tlpm and the
+ * corresponding nodes from the bpf-lpm. Then run the same
+ * large number of random lookups in both and make sure they match.
+ * Note: we need to count the number of nodes actually inserted
+ * since there may have been duplicates.
+ */
+ for (i = 0, t = list; t; i++, t = t->next)
+ ;
+ for (j = 0; j < i / 2; ++j) {
+ key->prefixlen = list->n_bits;
+ memcpy(key->data, list->key, keysize);
+ r = bpf_map_delete_elem(map, key);
+ assert(!r);
+ list = tlpm_delete(list, list->key, list->n_bits);
+ assert(list);
+ }
+ for (i = 0; i < n_lookups; ++i) {
+ for (j = 0; j < keysize; ++j)
+ data[j] = rand() & 0xff;
+
+ t = tlpm_match(list, data, 8 * keysize);
+
+ key->prefixlen = 8 * keysize;
+ memcpy(key->data, data, keysize);
+ r = bpf_map_lookup_elem(map, key, value);
+ assert(!r || errno == ENOENT);
+ assert(!t == !!r);
+
+ if (t) {
+ ++n_matches_after_delete;
+ assert(t->n_bits == value[keysize]);
+ for (j = 0; j < t->n_bits; ++j)
+ assert((t->key[j / 8] & (1 << (7 - j % 8))) ==
+ (value[j / 8] & (1 << (7 - j % 8))));
+ }
+ }
+
close(map);
tlpm_clear(list);
/* With 255 random nodes in the map, we are pretty likely to match
* something on every lookup. For statistics, use this:
*
- * printf(" nodes: %zu\n"
- * "lookups: %zu\n"
- * "matches: %zu\n", n_nodes, n_lookups, n_matches);
+ * printf(" nodes: %zu\n"
+ * " lookups: %zu\n"
+ * " matches: %zu\n"
+ * "matches(delete): %zu\n",
+ * n_nodes, n_lookups, n_matches, n_matches_after_delete);
*/
}
@@ -343,6 +418,108 @@ static void test_lpm_ipaddr(void)
close(map_fd_ipv6);
}
+static void test_lpm_delete(void)
+{
+ struct bpf_lpm_trie_key *key;
+ size_t key_size;
+ int map_fd;
+ __u64 value;
+
+ key_size = sizeof(*key) + sizeof(__u32);
+ key = alloca(key_size);
+
+ map_fd = bpf_create_map(BPF_MAP_TYPE_LPM_TRIE,
+ key_size, sizeof(value),
+ 100, BPF_F_NO_PREALLOC);
+ assert(map_fd >= 0);
+
+ /* Add nodes:
+ * 192.168.0.0/16 (1)
+ * 192.168.0.0/24 (2)
+ * 192.168.128.0/24 (3)
+ * 192.168.1.0/24 (4)
+ *
+ * (1)
+ * / \
+ * (IM) (3)
+ * / \
+ * (2) (4)
+ */
+ value = 1;
+ key->prefixlen = 16;
+ inet_pton(AF_INET, "192.168.0.0", key->data);
+ assert(bpf_map_update_elem(map_fd, key, &value, 0) == 0);
+
+ value = 2;
+ key->prefixlen = 24;
+ inet_pton(AF_INET, "192.168.0.0", key->data);
+ assert(bpf_map_update_elem(map_fd, key, &value, 0) == 0);
+
+ value = 3;
+ key->prefixlen = 24;
+ inet_pton(AF_INET, "192.168.128.0", key->data);
+ assert(bpf_map_update_elem(map_fd, key, &value, 0) == 0);
+
+ value = 4;
+ key->prefixlen = 24;
+ inet_pton(AF_INET, "192.168.1.0", key->data);
+ assert(bpf_map_update_elem(map_fd, key, &value, 0) == 0);
+
+ /* remove non-existent node */
+ key->prefixlen = 32;
+ inet_pton(AF_INET, "10.0.0.1", key->data);
+ assert(bpf_map_lookup_elem(map_fd, key, &value) == -1 &&
+ errno == ENOENT);
+
+ /* assert initial lookup */
+ key->prefixlen = 32;
+ inet_pton(AF_INET, "192.168.0.1", key->data);
+ assert(bpf_map_lookup_elem(map_fd, key, &value) == 0);
+ assert(value == 2);
+
+ /* remove leaf node */
+ key->prefixlen = 24;
+ inet_pton(AF_INET, "192.168.0.0", key->data);
+ assert(bpf_map_delete_elem(map_fd, key) == 0);
+
+ key->prefixlen = 32;
+ inet_pton(AF_INET, "192.168.0.1", key->data);
+ assert(bpf_map_lookup_elem(map_fd, key, &value) == 0);
+ assert(value == 1);
+
+ /* remove leaf (and intermediary) node */
+ key->prefixlen = 24;
+ inet_pton(AF_INET, "192.168.1.0", key->data);
+ assert(bpf_map_delete_elem(map_fd, key) == 0);
+
+ key->prefixlen = 32;
+ inet_pton(AF_INET, "192.168.1.1", key->data);
+ assert(bpf_map_lookup_elem(map_fd, key, &value) == 0);
+ assert(value == 1);
+
+ /* remove root node */
+ key->prefixlen = 16;
+ inet_pton(AF_INET, "192.168.0.0", key->data);
+ assert(bpf_map_delete_elem(map_fd, key) == 0);
+
+ key->prefixlen = 32;
+ inet_pton(AF_INET, "192.168.128.1", key->data);
+ assert(bpf_map_lookup_elem(map_fd, key, &value) == 0);
+ assert(value == 3);
+
+ /* remove last node */
+ key->prefixlen = 24;
+ inet_pton(AF_INET, "192.168.128.0", key->data);
+ assert(bpf_map_delete_elem(map_fd, key) == 0);
+
+ key->prefixlen = 32;
+ inet_pton(AF_INET, "192.168.128.1", key->data);
+ assert(bpf_map_lookup_elem(map_fd, key, &value) == -1 &&
+ errno == ENOENT);
+
+ close(map_fd);
+}
+
int main(void)
{
struct rlimit limit = { RLIM_INFINITY, RLIM_INFINITY };
@@ -365,6 +542,8 @@ int main(void)
test_lpm_ipaddr();
+ test_lpm_delete();
+
printf("test_lpm: OK\n");
return 0;
}
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* [PATCH net-next 2/3] bpf: Add uniqueness invariant to trivial lpm test implementation
From: Craig Gallek @ 2017-09-18 19:30 UTC (permalink / raw)
To: Daniel Mack, Alexei Starovoitov, Daniel Borkmann,
David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-1-kraigatgoog@gmail.com>
From: Craig Gallek <kraig@google.com>
The 'trivial' lpm implementation in this test allows equivalent nodes
to be added (that is, nodes consisting of the same prefix and prefix
length). For lookup operations, this is fine because insertion happens
at the head of the (singly linked) list and the first, best match is
returned. In order to support deletion, the tlpm data structue must
first enforce uniqueness. This change modifies the insertion algorithm
to search for equivalent nodes and remove them. Note: the
BPF_MAP_TYPE_LPM_TRIE already has a uniqueness invariant that is
implemented as node replacement.
Signed-off-by: Craig Gallek <kraig@google.com>
---
tools/testing/selftests/bpf/test_lpm_map.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_lpm_map.c b/tools/testing/selftests/bpf/test_lpm_map.c
index e97565243d59..a5ed7c4f1819 100644
--- a/tools/testing/selftests/bpf/test_lpm_map.c
+++ b/tools/testing/selftests/bpf/test_lpm_map.c
@@ -31,6 +31,10 @@ struct tlpm_node {
uint8_t key[];
};
+static struct tlpm_node *tlpm_match(struct tlpm_node *list,
+ const uint8_t *key,
+ size_t n_bits);
+
static struct tlpm_node *tlpm_add(struct tlpm_node *list,
const uint8_t *key,
size_t n_bits)
@@ -38,9 +42,17 @@ static struct tlpm_node *tlpm_add(struct tlpm_node *list,
struct tlpm_node *node;
size_t n;
+ n = (n_bits + 7) / 8;
+
+ /* 'overwrite' an equivalent entry if one already exists */
+ node = tlpm_match(list, key, n_bits);
+ if (node && node->n_bits == n_bits) {
+ memcpy(node->key, key, n);
+ return list;
+ }
+
/* add new entry with @key/@n_bits to @list and return new head */
- n = (n_bits + 7) / 8;
node = malloc(sizeof(*node) + n);
assert(node);
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Craig Gallek @ 2017-09-18 19:30 UTC (permalink / raw)
To: Daniel Mack, Alexei Starovoitov, Daniel Borkmann,
David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-1-kraigatgoog@gmail.com>
From: Craig Gallek <kraig@google.com>
This is a simple non-recursive delete operation. It prunes paths
of empty nodes in the tree, but it does not try to further compress
the tree as nodes are removed.
Signed-off-by: Craig Gallek <kraig@google.com>
---
kernel/bpf/lpm_trie.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 77 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index 1b767844a76f..9d58a576b2ae 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -389,10 +389,84 @@ static int trie_update_elem(struct bpf_map *map,
return ret;
}
-static int trie_delete_elem(struct bpf_map *map, void *key)
+/* Called from syscall or from eBPF program */
+static int trie_delete_elem(struct bpf_map *map, void *_key)
{
- /* TODO */
- return -ENOSYS;
+ struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
+ struct bpf_lpm_trie_key *key = _key;
+ struct lpm_trie_node __rcu **trim;
+ struct lpm_trie_node *node;
+ unsigned long irq_flags;
+ unsigned int next_bit;
+ size_t matchlen = 0;
+ int ret = 0;
+
+ if (key->prefixlen > trie->max_prefixlen)
+ return -EINVAL;
+
+ raw_spin_lock_irqsave(&trie->lock, irq_flags);
+
+ /* Walk the tree looking for an exact key/length match and keeping
+ * track of where we could begin trimming the tree. The trim-point
+ * is the sub-tree along the walk consisting of only single-child
+ * intermediate nodes and ending at a leaf node that we want to
+ * remove.
+ */
+ trim = &trie->root;
+ node = rcu_dereference_protected(
+ trie->root, lockdep_is_held(&trie->lock));
+ while (node) {
+ matchlen = longest_prefix_match(trie, node, key);
+
+ if (node->prefixlen != matchlen ||
+ node->prefixlen == key->prefixlen)
+ break;
+
+ next_bit = extract_bit(key->data, node->prefixlen);
+ /* If we hit a node that has more than one child or is a valid
+ * prefix itself, do not remove it. Reset the root of the trim
+ * path to its descendant on our path.
+ */
+ if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
+ (node->child[0] && node->child[1]))
+ trim = &node->child[next_bit];
+ node = rcu_dereference_protected(
+ node->child[next_bit], lockdep_is_held(&trie->lock));
+ }
+
+ if (!node || node->prefixlen != key->prefixlen ||
+ (node->flags & LPM_TREE_NODE_FLAG_IM)) {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ trie->n_entries--;
+
+ /* If the node we are removing is not a leaf node, simply mark it
+ * as intermediate and we are done.
+ */
+ if (rcu_access_pointer(node->child[0]) ||
+ rcu_access_pointer(node->child[1])) {
+ node->flags |= LPM_TREE_NODE_FLAG_IM;
+ goto out;
+ }
+
+ /* trim should now point to the slot holding the start of a path from
+ * zero or more intermediate nodes to our leaf node for deletion.
+ */
+ while ((node = rcu_dereference_protected(
+ *trim, lockdep_is_held(&trie->lock)))) {
+ RCU_INIT_POINTER(*trim, NULL);
+ trim = rcu_access_pointer(node->child[0]) ?
+ &node->child[0] :
+ &node->child[1];
+ kfree_rcu(node, rcu);
+ }
+
+out:
+ raw_spin_unlock_irqrestore(&trie->lock, irq_flags);
+
+ return ret;
}
#define LPM_DATA_SIZE_MAX 256
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* [PATCH net-next 0/3] Implement delete for BPF LPM trie
From: Craig Gallek @ 2017-09-18 19:30 UTC (permalink / raw)
To: Daniel Mack, Alexei Starovoitov, Daniel Borkmann,
David S . Miller; +Cc: netdev
From: Craig Gallek <kraig@google.com>
This was previously left as a TODO. Add the implementation and
extend the test to cover it.
Craig Gallek (3):
bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
bpf: Add uniqueness invariant to trivial lpm test implementation
bpf: Test deletion in BPF_MAP_TYPE_LPM_TRIE
kernel/bpf/lpm_trie.c | 80 +++++++++++-
tools/testing/selftests/bpf/test_lpm_map.c | 201 ++++++++++++++++++++++++++++-
2 files changed, 273 insertions(+), 8 deletions(-)
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply
* Re: [PATCH v4 net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Sergei Shtylyov @ 2017-09-18 19:29 UTC (permalink / raw)
To: Nisar Sayed, davem; +Cc: UNGLinuxDriver, netdev
In-Reply-To: <20170918220225.3767-3-Nisar.Sayed@microchip.com>
Hello!
On 09/19/2017 01:02 AM, Nisar Sayed wrote:
> Allow EEPROM write for less than MAX_EEPROM_SIZE
>
> Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
> Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
> ---
> drivers/net/usb/lan78xx.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index fcf85ae37435..3292f56ffe02 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1290,11 +1290,10 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
> if (ret)
> return ret;
>
> - /* Allow entire eeprom update only */
> - if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
> - (ee->offset == 0) &&
> - (ee->len == 512) &&
> - (data[0] == EEPROM_INDICATOR))
> + /* Invalid EEPROM_INDICATOR at offset zero will result in fail to
s/fail/a failure/.
> + * load data from EEPROM
> + */
> + if (ee->magic == LAN78XX_EEPROM_MAGIC)
> ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
> else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
> (ee->offset == 0) &&
>
MBR, Sergei
^ permalink raw reply
* [PATCH net-next 7/9] s390/qeth: translate SETVLAN/DELVLAN errors
From: Julian Wiedmann @ 2017-09-18 19:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20170918191822.65142-1-jwi@linux.vnet.ibm.com>
Properly return any error encountered during VLAN processing to the
the caller.
Resulting change in behaviour: if SETVLAN fails while registering a
new VLAN ID, the stack no longer creates the corresponding vlan device.
Signed-off-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
Reviewed-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core_mpc.h | 1 +
drivers/s390/net/qeth_l2_main.c | 47 ++++++++++++++++++++++++++++++----------
2 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/drivers/s390/net/qeth_core_mpc.h b/drivers/s390/net/qeth_core_mpc.h
index 2f1f0da3d089..6a7654df6e78 100644
--- a/drivers/s390/net/qeth_core_mpc.h
+++ b/drivers/s390/net/qeth_core_mpc.h
@@ -166,6 +166,7 @@ enum qeth_ipa_return_codes {
IPA_RC_L2_INVALID_VLAN_ID = 0x2015,
IPA_RC_L2_DUP_VLAN_ID = 0x2016,
IPA_RC_L2_VLAN_ID_NOT_FOUND = 0x2017,
+ IPA_RC_L2_VLAN_ID_NOT_ALLOWED = 0x2050,
IPA_RC_VNICC_VNICBP = 0x20B0,
IPA_RC_SBP_OSA_NOT_CONFIGURED = 0x2B0C,
IPA_RC_SBP_OSA_OS_MISMATCH = 0x2B10,
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 009de7248cc7..7b61c2ef4c74 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -78,7 +78,7 @@ static struct net_device *qeth_l2_netdev_by_devno(unsigned char *read_dev_no)
return ndev;
}
-static int qeth_setdel_makerc(struct qeth_card *card, int retcode)
+static int qeth_setdelmac_makerc(struct qeth_card *card, int retcode)
{
int rc;
@@ -128,8 +128,8 @@ static int qeth_l2_send_setdelmac(struct qeth_card *card, __u8 *mac,
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setdelmac.mac_length = OSA_ADDR_LEN;
memcpy(&cmd->data.setdelmac.mac, mac, OSA_ADDR_LEN);
- return qeth_setdel_makerc(card, qeth_send_ipa_cmd(card, iob,
- NULL, NULL));
+ return qeth_setdelmac_makerc(card, qeth_send_ipa_cmd(card, iob,
+ NULL, NULL));
}
static int qeth_l2_send_setmac(struct qeth_card *card, __u8 *mac)
@@ -289,17 +289,40 @@ static void qeth_l2_fill_header(struct qeth_hdr *hdr, struct sk_buff *skb,
}
}
+static int qeth_setdelvlan_makerc(struct qeth_card *card, int retcode)
+{
+ if (retcode)
+ QETH_CARD_TEXT_(card, 2, "err%04x", retcode);
+
+ switch (retcode) {
+ case IPA_RC_SUCCESS:
+ return 0;
+ case IPA_RC_L2_INVALID_VLAN_ID:
+ return -EINVAL;
+ case IPA_RC_L2_DUP_VLAN_ID:
+ return -EEXIST;
+ case IPA_RC_L2_VLAN_ID_NOT_FOUND:
+ return -ENOENT;
+ case IPA_RC_L2_VLAN_ID_NOT_ALLOWED:
+ return -EPERM;
+ case -ENOMEM:
+ return -ENOMEM;
+ default:
+ return -EIO;
+ }
+}
+
static int qeth_l2_send_setdelvlan_cb(struct qeth_card *card,
- struct qeth_reply *reply, unsigned long data)
+ struct qeth_reply *reply,
+ unsigned long data)
{
- struct qeth_ipa_cmd *cmd;
+ struct qeth_ipa_cmd *cmd = (struct qeth_ipa_cmd *) data;
QETH_CARD_TEXT(card, 2, "L2sdvcb");
- cmd = (struct qeth_ipa_cmd *) data;
if (cmd->hdr.return_code) {
- QETH_DBF_MESSAGE(2, "Error in processing VLAN %i on %s: 0x%x. "
- "Continuing\n", cmd->data.setdelvlan.vlan_id,
- QETH_CARD_IFNAME(card), cmd->hdr.return_code);
+ QETH_DBF_MESSAGE(2, "Error in processing VLAN %i on %s: 0x%x.\n",
+ cmd->data.setdelvlan.vlan_id,
+ QETH_CARD_IFNAME(card), cmd->hdr.return_code);
QETH_CARD_TEXT_(card, 2, "L2VL%4x", cmd->hdr.command);
QETH_CARD_TEXT_(card, 2, "err%d", cmd->hdr.return_code);
}
@@ -307,7 +330,7 @@ static int qeth_l2_send_setdelvlan_cb(struct qeth_card *card,
}
static int qeth_l2_send_setdelvlan(struct qeth_card *card, __u16 i,
- enum qeth_ipa_cmds ipacmd)
+ enum qeth_ipa_cmds ipacmd)
{
struct qeth_ipa_cmd *cmd;
struct qeth_cmd_buffer *iob;
@@ -318,8 +341,8 @@ static int qeth_l2_send_setdelvlan(struct qeth_card *card, __u16 i,
return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setdelvlan.vlan_id = i;
- return qeth_send_ipa_cmd(card, iob,
- qeth_l2_send_setdelvlan_cb, NULL);
+ return qeth_setdelvlan_makerc(card, qeth_send_ipa_cmd(card, iob,
+ qeth_l2_send_setdelvlan_cb, NULL));
}
static void qeth_l2_process_vlans(struct qeth_card *card)
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 1/9] s390/qeth: add basic VNICC support
From: Julian Wiedmann @ 2017-09-18 19:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20170918191822.65142-1-jwi@linux.vnet.ibm.com>
From: Hans Wippel <hwippel@linux.vnet.ibm.com>
VNIC Characteristics (VNICC) are features of HiperSockets that define
how packets are handled by the underlying network hardware. For example,
if the VNICC flooding is configured on a qeth device, ethernet frames to
unknown destination MAC addresses are received.
Currently, there is support for seven VNICCs: flooding, multicast
flooding, receive broadcast, learning, takeover learning, takeover
setvmac, bridge invisible. Also, six IPA commands exist for configuring
VNICCs on a qeth device: query characteristics, query commands, enable
characteristic, disable characteristic, set timeout, get timeout.
This patch adds the basic code infrastructure for VNICC support to qeth.
It allows querying VNICC support from the underlying hardware. To this
end, it adds:
* basic message formats for IPA commands
* basic data structures
* basic error handling
* query characteristics IPA command support
The query characteristics IPA command allows requesting the currently
supported and currently enabled VNIC characteristics from the underlying
hardware.
Support for the other IPA commands and for the configuration of VNICCs
is added in follow-up patches together with the respective user
interface functions.
Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
Reviewed-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core.h | 7 ++
drivers/s390/net/qeth_core_mpc.c | 4 +-
drivers/s390/net/qeth_core_mpc.h | 29 +++++++++
drivers/s390/net/qeth_l2_main.c | 136 ++++++++++++++++++++++++++++++++++++++-
4 files changed, 172 insertions(+), 4 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 59e09854c4f7..b96438df8fd4 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -182,6 +182,12 @@ struct qeth_sbp_info {
__u32 reflect_promisc_primary:1;
};
+struct qeth_vnicc_info {
+ /* supported/currently configured VNICCs; updated in IPA exchanges */
+ u32 sup_chars;
+ u32 cur_chars;
+};
+
static inline int qeth_is_ipa_supported(struct qeth_ipa_info *ipa,
enum qeth_ipa_funcs func)
{
@@ -673,6 +679,7 @@ struct qeth_card_options {
struct qeth_routing_info route6;
struct qeth_ipa_info ipa6;
struct qeth_sbp_info sbp; /* SETBRIDGEPORT options */
+ struct qeth_vnicc_info vnicc; /* VNICC options */
int fake_broadcast;
int layer2;
int performance_stats;
diff --git a/drivers/s390/net/qeth_core_mpc.c b/drivers/s390/net/qeth_core_mpc.c
index 6dd7d05e5693..5f8a2b834d39 100644
--- a/drivers/s390/net/qeth_core_mpc.c
+++ b/drivers/s390/net/qeth_core_mpc.c
@@ -167,7 +167,7 @@ static struct ipa_rc_msg qeth_ipa_rc_msg[] = {
{IPA_RC_IP_TABLE_FULL, "Add Addr IP Table Full - ipv6"},
{IPA_RC_UNKNOWN_ERROR, "IPA command failed - reason unknown"},
{IPA_RC_UNSUPPORTED_COMMAND, "Command not supported"},
- {IPA_RC_TRACE_ALREADY_ACTIVE, "trace already active"},
+ {IPA_RC_VNICC_OOSEQ, "Command issued out of sequence"},
{IPA_RC_INVALID_FORMAT, "invalid format or length"},
{IPA_RC_DUP_IPV6_REMOTE, "ipv6 address already registered remote"},
{IPA_RC_SBP_IQD_NOT_CONFIGURED, "Not configured for bridgeport"},
@@ -193,6 +193,7 @@ static struct ipa_rc_msg qeth_ipa_rc_msg[] = {
{IPA_RC_L2_INVALID_VLAN_ID, "L2 invalid vlan id"},
{IPA_RC_L2_DUP_VLAN_ID, "L2 duplicate vlan id"},
{IPA_RC_L2_VLAN_ID_NOT_FOUND, "L2 vlan id not found"},
+ {IPA_RC_VNICC_VNICBP, "VNIC is BridgePort"},
{IPA_RC_SBP_OSA_NOT_CONFIGURED, "Not configured for bridgeport"},
{IPA_RC_SBP_OSA_OS_MISMATCH, "OS mismatch"},
{IPA_RC_SBP_OSA_ANO_DEV_PRIMARY, "Primary bridgeport exists already"},
@@ -253,6 +254,7 @@ static struct ipa_cmd_names qeth_ipa_cmd_names[] = {
{IPA_CMD_DELGMAC, "delgmac"},
{IPA_CMD_SETVLAN, "setvlan"},
{IPA_CMD_DELVLAN, "delvlan"},
+ {IPA_CMD_VNICC, "vnic_characteristics"},
{IPA_CMD_SETBRIDGEPORT_OSA, "set_bridge_port(osa)"},
{IPA_CMD_SETCCID, "setccid"},
{IPA_CMD_DELCCID, "delccid"},
diff --git a/drivers/s390/net/qeth_core_mpc.h b/drivers/s390/net/qeth_core_mpc.h
index 912e0107de8f..c13816b8c92f 100644
--- a/drivers/s390/net/qeth_core_mpc.h
+++ b/drivers/s390/net/qeth_core_mpc.h
@@ -90,6 +90,7 @@ enum qeth_ipa_cmds {
IPA_CMD_DELGMAC = 0x24,
IPA_CMD_SETVLAN = 0x25,
IPA_CMD_DELVLAN = 0x26,
+ IPA_CMD_VNICC = 0x2a,
IPA_CMD_SETBRIDGEPORT_OSA = 0x2b,
IPA_CMD_SETCCID = 0x41,
IPA_CMD_DELCCID = 0x42,
@@ -165,6 +166,7 @@ enum qeth_ipa_return_codes {
IPA_RC_L2_INVALID_VLAN_ID = 0x2015,
IPA_RC_L2_DUP_VLAN_ID = 0x2016,
IPA_RC_L2_VLAN_ID_NOT_FOUND = 0x2017,
+ IPA_RC_VNICC_VNICBP = 0x20B0,
IPA_RC_SBP_OSA_NOT_CONFIGURED = 0x2B0C,
IPA_RC_SBP_OSA_OS_MISMATCH = 0x2B10,
IPA_RC_SBP_OSA_ANO_DEV_PRIMARY = 0x2B14,
@@ -197,6 +199,9 @@ enum qeth_ipa_return_codes {
IPA_RC_ENOMEM = 0xfffe,
IPA_RC_FFFF = 0xffff
};
+/* for VNIC Characteristics */
+#define IPA_RC_VNICC_OOSEQ 0x0005
+
/* for SET_DIAGNOSTIC_ASSIST */
#define IPA_RC_INVALID_SUBCMD IPA_RC_IP_TABLE_FULL
#define IPA_RC_HARDWARE_AUTH_ERROR IPA_RC_UNKNOWN_ERROR
@@ -551,6 +556,29 @@ struct qeth_ipacmd_diagass {
__u8 cdata[64];
} __attribute__ ((packed));
+/* VNIC Characteristics IPA Command: *****************************************/
+/* IPA commands/sub commands for VNICC */
+#define IPA_VNICC_QUERY_CHARS 0x00000000L
+
+/* VNICC header */
+struct qeth_ipacmd_vnicc_hdr {
+ u32 sup;
+ u32 cur;
+};
+
+/* VNICC sub command header */
+struct qeth_vnicc_sub_hdr {
+ u16 data_length;
+ u16 reserved;
+ u32 sub_command;
+};
+
+/* complete VNICC IPA command message */
+struct qeth_ipacmd_vnicc {
+ struct qeth_ipacmd_vnicc_hdr hdr;
+ struct qeth_vnicc_sub_hdr sub_hdr;
+};
+
/* SETBRIDGEPORT IPA Command: *********************************************/
enum qeth_ipa_sbp_cmd {
IPA_SBP_QUERY_COMMANDS_SUPPORTED = 0x00000000L,
@@ -692,6 +720,7 @@ struct qeth_ipa_cmd {
struct qeth_ipacmd_diagass diagass;
struct qeth_ipacmd_setbridgeport sbp;
struct qeth_ipacmd_addr_change addrchange;
+ struct qeth_ipacmd_vnicc vnicc;
} data;
} __attribute__ ((packed));
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 760b023eae95..2fa273b40dd1 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -33,6 +33,7 @@ static void qeth_bridge_state_change(struct qeth_card *card,
struct qeth_ipa_cmd *cmd);
static void qeth_bridge_host_event(struct qeth_card *card,
struct qeth_ipa_cmd *cmd);
+static void qeth_l2_vnicc_init(struct qeth_card *card);
static int qeth_l2_verify_dev(struct net_device *dev)
{
@@ -1045,9 +1046,14 @@ static int qeth_l2_start_ipassists(struct qeth_card *card)
static void qeth_l2_trace_features(struct qeth_card *card)
{
- QETH_CARD_TEXT(card, 2, "l2featur");
+ /* Set BridgePort features */
+ QETH_CARD_TEXT(card, 2, "featuSBP");
QETH_CARD_HEX(card, 2, &card->options.sbp.supported_funcs,
sizeof(card->options.sbp.supported_funcs));
+ /* VNIC Characteristics features */
+ QETH_CARD_TEXT(card, 2, "feaVNICC");
+ QETH_CARD_HEX(card, 2, &card->options.vnicc.sup_chars,
+ sizeof(card->options.vnicc.sup_chars));
}
static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
@@ -1072,8 +1078,6 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
if (card->options.sbp.supported_funcs)
dev_info(&card->gdev->dev,
"The device represents a Bridge Capable Port\n");
- qeth_trace_features(card);
- qeth_l2_trace_features(card);
if (!card->dev && qeth_l2_setup_netdev(card)) {
rc = -ENODEV;
@@ -1090,6 +1094,12 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
} else
card->info.hwtrap = 0;
+ /* for the rx_bcast characteristic, init VNICC after setmac */
+ qeth_l2_vnicc_init(card);
+
+ qeth_trace_features(card);
+ qeth_l2_trace_features(card);
+
qeth_l2_setup_bridgeport_attrs(card);
card->state = CARD_STATE_HARDSETUP;
@@ -2039,6 +2049,126 @@ int qeth_bridgeport_an_set(struct qeth_card *card, int enable)
}
EXPORT_SYMBOL_GPL(qeth_bridgeport_an_set);
+/* VNIC Characteristics support */
+
+/* handle VNICC IPA command return codes; convert to error codes */
+static int qeth_l2_vnicc_makerc(struct qeth_card *card, int ipa_rc)
+{
+ int rc;
+
+ switch (ipa_rc) {
+ case IPA_RC_SUCCESS:
+ return ipa_rc;
+ case IPA_RC_L2_UNSUPPORTED_CMD:
+ case IPA_RC_NOTSUPP:
+ rc = -EOPNOTSUPP;
+ break;
+ case IPA_RC_VNICC_OOSEQ:
+ rc = -EALREADY;
+ break;
+ case IPA_RC_VNICC_VNICBP:
+ rc = -EBUSY;
+ break;
+ case IPA_RC_L2_ADDR_TABLE_FULL:
+ rc = -ENOSPC;
+ break;
+ case IPA_RC_L2_MAC_NOT_AUTH_BY_ADP:
+ rc = -EACCES;
+ break;
+ default:
+ rc = -EIO;
+ }
+
+ QETH_CARD_TEXT_(card, 2, "err%04x", ipa_rc);
+ return rc;
+}
+
+/* generic VNICC request call back control */
+struct _qeth_l2_vnicc_request_cbctl {
+ u32 sub_cmd;
+};
+
+/* generic VNICC request call back */
+static int qeth_l2_vnicc_request_cb(struct qeth_card *card,
+ struct qeth_reply *reply,
+ unsigned long data)
+{
+ struct qeth_ipa_cmd *cmd = (struct qeth_ipa_cmd *) data;
+ struct qeth_ipacmd_vnicc *rep = &cmd->data.vnicc;
+
+ QETH_CARD_TEXT(card, 2, "vniccrcb");
+ if (cmd->hdr.return_code)
+ return 0;
+ /* return results to caller */
+ card->options.vnicc.sup_chars = rep->hdr.sup;
+ card->options.vnicc.cur_chars = rep->hdr.cur;
+
+ return 0;
+}
+
+/* generic VNICC request */
+static int qeth_l2_vnicc_request(struct qeth_card *card,
+ struct _qeth_l2_vnicc_request_cbctl *cbctl)
+{
+ struct qeth_ipacmd_vnicc *req;
+ struct qeth_cmd_buffer *iob;
+ struct qeth_ipa_cmd *cmd;
+ int rc;
+
+ QETH_CARD_TEXT(card, 2, "vniccreq");
+
+ /* get new buffer for request */
+ iob = qeth_get_ipacmd_buffer(card, IPA_CMD_VNICC, 0);
+ if (!iob)
+ return -ENOMEM;
+
+ /* create header for request */
+ cmd = (struct qeth_ipa_cmd *)(iob->data + IPA_PDU_HEADER_SIZE);
+ req = &cmd->data.vnicc;
+
+ /* create sub command header for request */
+ req->sub_hdr.data_length = sizeof(req->sub_hdr);
+ req->sub_hdr.sub_command = cbctl->sub_cmd;
+
+ /* create sub command specific request fields */
+ switch (cbctl->sub_cmd) {
+ case IPA_VNICC_QUERY_CHARS:
+ break;
+ default:
+ qeth_release_buffer(iob->channel, iob);
+ return -EOPNOTSUPP;
+ }
+
+ /* send request */
+ rc = qeth_send_ipa_cmd(card, iob, qeth_l2_vnicc_request_cb,
+ (void *) cbctl);
+
+ return qeth_l2_vnicc_makerc(card, rc);
+}
+
+/* VNICC query VNIC characteristics request */
+static int qeth_l2_vnicc_query_chars(struct qeth_card *card)
+{
+ struct _qeth_l2_vnicc_request_cbctl cbctl;
+
+ /* prepare callback control */
+ cbctl.sub_cmd = IPA_VNICC_QUERY_CHARS;
+
+ QETH_CARD_TEXT(card, 2, "vniccqch");
+ return qeth_l2_vnicc_request(card, &cbctl);
+}
+
+/* (re-)initialize VNICC */
+static void qeth_l2_vnicc_init(struct qeth_card *card)
+{
+ QETH_CARD_TEXT(card, 2, "vniccini");
+ /* initial query and storage of VNIC characteristics */
+ if (qeth_l2_vnicc_query_chars(card)) {
+ card->options.vnicc.sup_chars = 0;
+ card->options.vnicc.cur_chars = 0;
+ }
+}
+
module_init(qeth_l2_init);
module_exit(qeth_l2_exit);
MODULE_AUTHOR("Frank Blaschka <frank.blaschka@de.ibm.com>");
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 8/9] s390/qeth: fold VLAN handling into l3_rebuild_skb()
From: Julian Wiedmann @ 2017-09-18 19:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20170918191822.65142-1-jwi@linux.vnet.ibm.com>
Move the overly complicated VLAN processing from the L3 RX handler into
its l3_rebuild_skb() helper. No change in functionality.
Signed-off-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
Reviewed-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_l3_main.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 733f94cbd4d4..270ac9515f6b 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1646,13 +1646,12 @@ static int qeth_l3_vlan_rx_kill_vid(struct net_device *dev,
return 0;
}
-static int qeth_l3_rebuild_skb(struct qeth_card *card, struct sk_buff *skb,
- struct qeth_hdr *hdr, unsigned short *vlan_id)
+static void qeth_l3_rebuild_skb(struct qeth_card *card, struct sk_buff *skb,
+ struct qeth_hdr *hdr)
{
__u16 prot;
struct iphdr *ip_hdr;
unsigned char tg_addr[MAX_ADDR_LEN];
- int is_vlan = 0;
if (!(hdr->hdr.l3.flags & QETH_HDR_PASSTHRU)) {
prot = (hdr->hdr.l3.flags & QETH_HDR_IPV6) ? ETH_P_IPV6 :
@@ -1706,11 +1705,14 @@ static int qeth_l3_rebuild_skb(struct qeth_card *card, struct sk_buff *skb,
skb->protocol = eth_type_trans(skb, card->dev);
- if (hdr->hdr.l3.ext_flags &
- (QETH_HDR_EXT_VLAN_FRAME | QETH_HDR_EXT_INCLUDE_VLAN_TAG)) {
- *vlan_id = (hdr->hdr.l3.ext_flags & QETH_HDR_EXT_VLAN_FRAME) ?
- hdr->hdr.l3.vlan_id : *((u16 *)&hdr->hdr.l3.dest_addr[12]);
- is_vlan = 1;
+ /* copy VLAN tag from hdr into skb */
+ if (!card->options.sniffer &&
+ (hdr->hdr.l3.ext_flags & (QETH_HDR_EXT_VLAN_FRAME |
+ QETH_HDR_EXT_INCLUDE_VLAN_TAG))) {
+ u16 tag = (hdr->hdr.l3.ext_flags & QETH_HDR_EXT_VLAN_FRAME) ?
+ hdr->hdr.l3.vlan_id :
+ *((u16 *)&hdr->hdr.l3.dest_addr[12]);
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), tag);
}
if (card->dev->features & NETIF_F_RXCSUM) {
@@ -1724,7 +1726,6 @@ static int qeth_l3_rebuild_skb(struct qeth_card *card, struct sk_buff *skb,
skb->ip_summed = CHECKSUM_NONE;
} else
skb->ip_summed = CHECKSUM_NONE;
- return is_vlan;
}
static int qeth_l3_process_inbound_buffer(struct qeth_card *card,
@@ -1733,8 +1734,6 @@ static int qeth_l3_process_inbound_buffer(struct qeth_card *card,
int work_done = 0;
struct sk_buff *skb;
struct qeth_hdr *hdr;
- __u16 vlan_tag = 0;
- int is_vlan;
unsigned int len;
__u16 magic;
@@ -1764,12 +1763,8 @@ static int qeth_l3_process_inbound_buffer(struct qeth_card *card,
card->dev->addr_len);
netif_receive_skb(skb);
} else {
- is_vlan = qeth_l3_rebuild_skb(card, skb, hdr,
- &vlan_tag);
+ qeth_l3_rebuild_skb(card, skb, hdr);
len = skb->len;
- if (is_vlan && !card->options.sniffer)
- __vlan_hwaccel_put_tag(skb,
- htons(ETH_P_8021Q), vlan_tag);
napi_gro_receive(&card->napi, skb);
}
break;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 3/9] s390/qeth: add VNICC get/set timeout support
From: Julian Wiedmann @ 2017-09-18 19:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20170918191822.65142-1-jwi@linux.vnet.ibm.com>
From: Hans Wippel <hwippel@linux.vnet.ibm.com>
HiperSockets allow configuring so called VNIC Characteristics (VNICC)
that influence how the underlying hardware handles packets. For VNICCs,
additional commands for getting and setting timeouts are available.
Currently, the learning VNICC uses these commands.
* Learning VNICC: If learning is enabled on a qeth device, the device
learns the source MAC addresses of outgoing packets and incoming
packets to those learned MAC addresses are received.
For learning, the timeout specifies the idle period in seconds, after
which the underlying hardware removes a learned MAC address again.
This patch adds support for the IPA commands that are required to get
and set the current timeout values for the learning VNIC characteristic.
Also, it introduces the sysfs interface that allows users to configure
the timeout.
Signed-off-by: Hans Wippel <hwippel@linux.vnet.ibm.com>
Reviewed-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core.h | 3 +
drivers/s390/net/qeth_core_mpc.h | 11 ++++
drivers/s390/net/qeth_l2.h | 2 +
drivers/s390/net/qeth_l2_main.c | 135 +++++++++++++++++++++++++++++++++++++--
drivers/s390/net/qeth_l2_sys.c | 44 +++++++++++++
5 files changed, 190 insertions(+), 5 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 2236c0c9744a..e3d3609cd9e7 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -188,6 +188,9 @@ struct qeth_vnicc_info {
u32 cur_chars;
/* supported commands: bitmasks which VNICCs support respective cmd */
u32 set_char_sup;
+ u32 getset_timeout_sup;
+ /* timeout value for the learning characteristic */
+ u32 learning_timeout;
/* characteristics wanted/configured by user */
u32 wanted_chars;
/* has user explicitly enabled rx_bcast while online? */
diff --git a/drivers/s390/net/qeth_core_mpc.h b/drivers/s390/net/qeth_core_mpc.h
index 7f67a81a2ae6..2f1f0da3d089 100644
--- a/drivers/s390/net/qeth_core_mpc.h
+++ b/drivers/s390/net/qeth_core_mpc.h
@@ -562,6 +562,8 @@ struct qeth_ipacmd_diagass {
#define IPA_VNICC_QUERY_CMDS 0x00000001L
#define IPA_VNICC_ENABLE 0x00000002L
#define IPA_VNICC_DISABLE 0x00000004L
+#define IPA_VNICC_SET_TIMEOUT 0x00000008L
+#define IPA_VNICC_GET_TIMEOUT 0x00000010L
/* VNICC flags */
#define QETH_VNICC_FLOODING 0x80000000
@@ -575,6 +577,8 @@ struct qeth_ipacmd_diagass {
/* VNICC default values */
#define QETH_VNICC_ALL 0xff000000
#define QETH_VNICC_DEFAULT QETH_VNICC_RX_BCAST
+/* default VNICC timeout in seconds */
+#define QETH_VNICC_DEFAULT_TIMEOUT 600
/* VNICC header */
struct qeth_ipacmd_vnicc_hdr {
@@ -600,6 +604,12 @@ struct qeth_vnicc_set_char {
u32 vnic_char;
};
+/* get/set timeout for VNIC characteristic */
+struct qeth_vnicc_getset_timeout {
+ u32 vnic_char;
+ u32 timeout;
+};
+
/* complete VNICC IPA command message */
struct qeth_ipacmd_vnicc {
struct qeth_ipacmd_vnicc_hdr hdr;
@@ -607,6 +617,7 @@ struct qeth_ipacmd_vnicc {
union {
struct qeth_vnicc_query_cmds query_cmds;
struct qeth_vnicc_set_char set_char;
+ struct qeth_vnicc_getset_timeout getset_timeout;
};
};
diff --git a/drivers/s390/net/qeth_l2.h b/drivers/s390/net/qeth_l2.h
index 0619018c76f9..241df6b98ab4 100644
--- a/drivers/s390/net/qeth_l2.h
+++ b/drivers/s390/net/qeth_l2.h
@@ -16,6 +16,8 @@ void qeth_l2_setup_bridgeport_attrs(struct qeth_card *card);
int qeth_l2_vnicc_set_state(struct qeth_card *card, u32 vnicc, bool state);
int qeth_l2_vnicc_get_state(struct qeth_card *card, u32 vnicc, bool *state);
+int qeth_l2_vnicc_set_timeout(struct qeth_card *card, u32 timeout);
+int qeth_l2_vnicc_get_timeout(struct qeth_card *card, u32 *timeout);
bool qeth_l2_vnicc_is_in_use(struct qeth_card *card);
struct qeth_mac {
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 36a7fd7255e3..25a0f381bcd5 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -35,6 +35,8 @@ static void qeth_bridge_host_event(struct qeth_card *card,
struct qeth_ipa_cmd *cmd);
static void qeth_l2_vnicc_set_defaults(struct qeth_card *card);
static void qeth_l2_vnicc_init(struct qeth_card *card);
+static bool qeth_l2_vnicc_recover_timeout(struct qeth_card *card, u32 vnicc,
+ u32 *timeout);
static int qeth_l2_verify_dev(struct net_device *dev)
{
@@ -2096,9 +2098,13 @@ struct _qeth_l2_vnicc_request_cbctl {
u32 sub_cmd;
struct {
u32 vnic_char;
+ u32 timeout;
} param;
struct {
- u32 *sup_cmds;
+ union{
+ u32 *sup_cmds;
+ u32 *timeout;
+ };
} result;
};
@@ -2122,6 +2128,9 @@ static int qeth_l2_vnicc_request_cb(struct qeth_card *card,
if (cbctl->sub_cmd == IPA_VNICC_QUERY_CMDS)
*cbctl->result.sup_cmds = rep->query_cmds.sup_cmds;
+ if (cbctl->sub_cmd == IPA_VNICC_GET_TIMEOUT)
+ *cbctl->result.timeout = rep->getset_timeout.timeout;
+
return 0;
}
@@ -2162,6 +2171,13 @@ static int qeth_l2_vnicc_request(struct qeth_card *card,
req->sub_hdr.data_length += sizeof(req->set_char);
req->set_char.vnic_char = cbctl->param.vnic_char;
break;
+ case IPA_VNICC_SET_TIMEOUT:
+ req->getset_timeout.timeout = cbctl->param.timeout;
+ /* fallthrough */
+ case IPA_VNICC_GET_TIMEOUT:
+ req->sub_hdr.data_length += sizeof(req->getset_timeout);
+ req->getset_timeout.vnic_char = cbctl->param.vnic_char;
+ break;
default:
qeth_release_buffer(iob->channel, iob);
return -EOPNOTSUPP;
@@ -2215,6 +2231,24 @@ static int qeth_l2_vnicc_set_char(struct qeth_card *card, u32 vnic_char,
return qeth_l2_vnicc_request(card, &cbctl);
}
+/* VNICC get/set timeout for characteristic request */
+static int qeth_l2_vnicc_getset_timeout(struct qeth_card *card, u32 vnicc,
+ u32 cmd, u32 *timeout)
+{
+ struct _qeth_l2_vnicc_request_cbctl cbctl;
+
+ /* prepare callback control */
+ cbctl.sub_cmd = cmd;
+ cbctl.param.vnic_char = vnicc;
+ if (cmd == IPA_VNICC_SET_TIMEOUT)
+ cbctl.param.timeout = *timeout;
+ if (cmd == IPA_VNICC_GET_TIMEOUT)
+ cbctl.result.timeout = timeout;
+
+ QETH_CARD_TEXT(card, 2, "vniccgst");
+ return qeth_l2_vnicc_request(card, &cbctl);
+}
+
/* set current VNICC flag state; called from sysfs store function */
int qeth_l2_vnicc_set_state(struct qeth_card *card, u32 vnicc, bool state)
{
@@ -2258,8 +2292,14 @@ int qeth_l2_vnicc_set_state(struct qeth_card *card, u32 vnicc, bool state)
if (rc)
card->options.vnicc.wanted_chars =
card->options.vnicc.cur_chars;
- else if (state && vnicc == QETH_VNICC_RX_BCAST)
- card->options.vnicc.rx_bcast_enabled = true;
+ else {
+ /* successful online VNICC change; handle special cases */
+ if (state && vnicc == QETH_VNICC_RX_BCAST)
+ card->options.vnicc.rx_bcast_enabled = true;
+ if (!state && vnicc == QETH_VNICC_LEARNING)
+ qeth_l2_vnicc_recover_timeout(card, vnicc,
+ &card->options.vnicc.learning_timeout);
+ }
return rc;
}
@@ -2287,6 +2327,70 @@ int qeth_l2_vnicc_get_state(struct qeth_card *card, u32 vnicc, bool *state)
return rc;
}
+/* set VNICC timeout; called from sysfs store function. Currently, only learning
+ * supports timeout
+ */
+int qeth_l2_vnicc_set_timeout(struct qeth_card *card, u32 timeout)
+{
+ int rc = 0;
+
+ QETH_CARD_TEXT(card, 2, "vniccsto");
+
+ /* do not change anything if BridgePort is enabled */
+ if (qeth_bridgeport_is_in_use(card))
+ return -EBUSY;
+
+ /* check if characteristic and set_timeout are supported */
+ if (!(card->options.vnicc.sup_chars & QETH_VNICC_LEARNING) ||
+ !(card->options.vnicc.getset_timeout_sup & QETH_VNICC_LEARNING))
+ return -EOPNOTSUPP;
+
+ /* do we need to do anything? */
+ if (card->options.vnicc.learning_timeout == timeout)
+ return rc;
+
+ /* if card is not ready, simply store the value internally and return */
+ if (!qeth_card_hw_is_reachable(card)) {
+ card->options.vnicc.learning_timeout = timeout;
+ return rc;
+ }
+
+ /* send timeout value to card; if successful, store value internally */
+ rc = qeth_l2_vnicc_getset_timeout(card, QETH_VNICC_LEARNING,
+ IPA_VNICC_SET_TIMEOUT, &timeout);
+ if (!rc)
+ card->options.vnicc.learning_timeout = timeout;
+
+ return rc;
+}
+
+/* get current VNICC timeout; called from sysfs show function. Currently, only
+ * learning supports timeout
+ */
+int qeth_l2_vnicc_get_timeout(struct qeth_card *card, u32 *timeout)
+{
+ int rc = 0;
+
+ QETH_CARD_TEXT(card, 2, "vniccgto");
+
+ /* do not get anything if BridgePort is enabled */
+ if (qeth_bridgeport_is_in_use(card))
+ return -EBUSY;
+
+ /* check if characteristic and get_timeout are supported */
+ if (!(card->options.vnicc.sup_chars & QETH_VNICC_LEARNING) ||
+ !(card->options.vnicc.getset_timeout_sup & QETH_VNICC_LEARNING))
+ return -EOPNOTSUPP;
+ /* if card is ready, get timeout. Otherwise, just return stored value */
+ *timeout = card->options.vnicc.learning_timeout;
+ if (qeth_card_hw_is_reachable(card))
+ rc = qeth_l2_vnicc_getset_timeout(card, QETH_VNICC_LEARNING,
+ IPA_VNICC_GET_TIMEOUT,
+ timeout);
+
+ return rc;
+}
+
/* check if VNICC is currently enabled */
bool qeth_l2_vnicc_is_in_use(struct qeth_card *card)
{
@@ -2304,6 +2408,19 @@ bool qeth_l2_vnicc_is_in_use(struct qeth_card *card)
return true;
}
+/* recover user timeout setting */
+static bool qeth_l2_vnicc_recover_timeout(struct qeth_card *card, u32 vnicc,
+ u32 *timeout)
+{
+ if (card->options.vnicc.sup_chars & vnicc &&
+ card->options.vnicc.getset_timeout_sup & vnicc &&
+ !qeth_l2_vnicc_getset_timeout(card, vnicc, IPA_VNICC_SET_TIMEOUT,
+ timeout))
+ return false;
+ *timeout = QETH_VNICC_DEFAULT_TIMEOUT;
+ return true;
+}
+
/* recover user characteristic setting */
static bool qeth_l2_vnicc_recover_char(struct qeth_card *card, u32 vnicc,
bool enable)
@@ -2322,6 +2439,7 @@ static bool qeth_l2_vnicc_recover_char(struct qeth_card *card, u32 vnicc,
/* (re-)initialize VNICC */
static void qeth_l2_vnicc_init(struct qeth_card *card)
{
+ u32 *timeout = &card->options.vnicc.learning_timeout;
unsigned int chars_len, i;
unsigned long chars_tmp;
u32 sup_cmds, vnicc;
@@ -2332,7 +2450,8 @@ static void qeth_l2_vnicc_init(struct qeth_card *card)
card->options.vnicc.rx_bcast_enabled = 0;
/* initial query and storage of VNIC characteristics */
if (qeth_l2_vnicc_query_chars(card)) {
- if (card->options.vnicc.wanted_chars != QETH_VNICC_DEFAULT)
+ if (card->options.vnicc.wanted_chars != QETH_VNICC_DEFAULT ||
+ *timeout != QETH_VNICC_DEFAULT_TIMEOUT)
dev_err(&card->gdev->dev, "Configuring the VNIC characteristics failed\n");
/* fail quietly if user didn't change the default config */
card->options.vnicc.sup_chars = 0;
@@ -2346,12 +2465,16 @@ static void qeth_l2_vnicc_init(struct qeth_card *card)
for_each_set_bit(i, &chars_tmp, chars_len) {
vnicc = BIT(i);
qeth_l2_vnicc_query_cmds(card, vnicc, &sup_cmds);
+ if (!(sup_cmds & IPA_VNICC_SET_TIMEOUT) ||
+ !(sup_cmds & IPA_VNICC_GET_TIMEOUT))
+ card->options.vnicc.getset_timeout_sup &= ~vnicc;
if (!(sup_cmds & IPA_VNICC_ENABLE) ||
!(sup_cmds & IPA_VNICC_DISABLE))
card->options.vnicc.set_char_sup &= ~vnicc;
}
/* enforce assumed default values and recover settings, if changed */
- error = false;
+ error = qeth_l2_vnicc_recover_timeout(card, QETH_VNICC_LEARNING,
+ timeout);
chars_tmp = card->options.vnicc.wanted_chars ^ QETH_VNICC_DEFAULT;
chars_tmp |= QETH_VNICC_BRIDGE_INVISIBLE;
chars_len = sizeof(card->options.vnicc.wanted_chars) * BITS_PER_BYTE;
@@ -2370,8 +2493,10 @@ static void qeth_l2_vnicc_set_defaults(struct qeth_card *card)
/* characteristics values */
card->options.vnicc.sup_chars = QETH_VNICC_ALL;
card->options.vnicc.cur_chars = QETH_VNICC_DEFAULT;
+ card->options.vnicc.learning_timeout = QETH_VNICC_DEFAULT_TIMEOUT;
/* supported commands */
card->options.vnicc.set_char_sup = QETH_VNICC_ALL;
+ card->options.vnicc.getset_timeout_sup = QETH_VNICC_LEARNING;
/* settings wanted by users */
card->options.vnicc.wanted_chars = QETH_VNICC_DEFAULT;
}
diff --git a/drivers/s390/net/qeth_l2_sys.c b/drivers/s390/net/qeth_l2_sys.c
index 01936f7dbe7e..4608daedb204 100644
--- a/drivers/s390/net/qeth_l2_sys.c
+++ b/drivers/s390/net/qeth_l2_sys.c
@@ -309,6 +309,47 @@ static u32 qeth_l2_vnicc_sysfs_attr_to_char(const char *attr_name)
return 0;
}
+/* get current timeout setting */
+static ssize_t qeth_vnicc_timeout_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct qeth_card *card = dev_get_drvdata(dev);
+ u32 timeout;
+ int rc;
+
+ if (!card)
+ return -EINVAL;
+
+ rc = qeth_l2_vnicc_get_timeout(card, &timeout);
+ if (rc == -EBUSY)
+ return sprintf(buf, "n/a (BridgePort)\n");
+ if (rc == -EOPNOTSUPP)
+ return sprintf(buf, "n/a\n");
+ return rc ? rc : sprintf(buf, "%d\n", timeout);
+}
+
+/* change timeout setting */
+static ssize_t qeth_vnicc_timeout_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct qeth_card *card = dev_get_drvdata(dev);
+ u32 timeout;
+ int rc;
+
+ if (!card)
+ return -EINVAL;
+
+ rc = kstrtou32(buf, 10, &timeout);
+ if (rc)
+ return rc;
+
+ mutex_lock(&card->conf_mutex);
+ rc = qeth_l2_vnicc_set_timeout(card, timeout);
+ mutex_unlock(&card->conf_mutex);
+ return rc ? rc : count;
+}
+
/* get current setting of characteristic */
static ssize_t qeth_vnicc_char_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -359,6 +400,8 @@ static DEVICE_ATTR(flooding, 0644, qeth_vnicc_char_show, qeth_vnicc_char_store);
static DEVICE_ATTR(mcast_flooding, 0644, qeth_vnicc_char_show,
qeth_vnicc_char_store);
static DEVICE_ATTR(learning, 0644, qeth_vnicc_char_show, qeth_vnicc_char_store);
+static DEVICE_ATTR(learning_timeout, 0644, qeth_vnicc_timeout_show,
+ qeth_vnicc_timeout_store);
static DEVICE_ATTR(takeover_setvmac, 0644, qeth_vnicc_char_show,
qeth_vnicc_char_store);
static DEVICE_ATTR(takeover_learning, 0644, qeth_vnicc_char_show,
@@ -371,6 +414,7 @@ static struct attribute *qeth_l2_vnicc_attrs[] = {
&dev_attr_flooding.attr,
&dev_attr_mcast_flooding.attr,
&dev_attr_learning.attr,
+ &dev_attr_learning_timeout.attr,
&dev_attr_takeover_setvmac.attr,
&dev_attr_takeover_learning.attr,
&dev_attr_bridge_invisible.attr,
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 5/9] s390/qeth: don't take queue lock in send_packet_fast()
From: Julian Wiedmann @ 2017-09-18 19:18 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20170918191822.65142-1-jwi@linux.vnet.ibm.com>
Locking the output queue prior to TX is needed on OSA devices,
to synchronize against a packing flush from the TX completion code
(via qeth_check_outbound_queue()).
But send_packet_fast() is only used for IQDs, which don't do packing.
So remove the locking, and apply some easy cleanups.
Signed-off-by: Julian Wiedmann <jwi@linux.vnet.ibm.com>
Reviewed-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core.h | 3 +--
drivers/s390/net/qeth_core_main.c | 22 +++++-----------------
drivers/s390/net/qeth_l2_main.c | 2 +-
drivers/s390/net/qeth_l3_main.c | 4 ++--
4 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index e3d3609cd9e7..b7e8531625aa 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -962,8 +962,7 @@ int qeth_get_priority_queue(struct qeth_card *, struct sk_buff *, int, int);
int qeth_get_elements_no(struct qeth_card *card, struct sk_buff *skb,
int extra_elems, int data_offset);
int qeth_get_elements_for_frags(struct sk_buff *);
-int qeth_do_send_packet_fast(struct qeth_card *card,
- struct qeth_qdio_out_q *queue, struct sk_buff *skb,
+int qeth_do_send_packet_fast(struct qeth_qdio_out_q *queue, struct sk_buff *skb,
struct qeth_hdr *hdr, unsigned int offset,
unsigned int hd_len);
int qeth_do_send_packet(struct qeth_card *card, struct qeth_qdio_out_q *queue,
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 77032801e30b..68e118f1202e 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4040,35 +4040,23 @@ static int qeth_fill_buffer(struct qeth_qdio_out_q *queue,
return flush_cnt;
}
-int qeth_do_send_packet_fast(struct qeth_card *card,
- struct qeth_qdio_out_q *queue, struct sk_buff *skb,
+int qeth_do_send_packet_fast(struct qeth_qdio_out_q *queue, struct sk_buff *skb,
struct qeth_hdr *hdr, unsigned int offset,
unsigned int hd_len)
{
- struct qeth_qdio_out_buffer *buffer;
- int index;
+ int index = queue->next_buf_to_fill;
+ struct qeth_qdio_out_buffer *buffer = queue->bufs[index];
- /* spin until we get the queue ... */
- while (atomic_cmpxchg(&queue->state, QETH_OUT_Q_UNLOCKED,
- QETH_OUT_Q_LOCKED) != QETH_OUT_Q_UNLOCKED);
- /* ... now we've got the queue */
- index = queue->next_buf_to_fill;
- buffer = queue->bufs[queue->next_buf_to_fill];
/*
* check if buffer is empty to make sure that we do not 'overtake'
* ourselves and try to fill a buffer that is already primed
*/
if (atomic_read(&buffer->state) != QETH_QDIO_BUF_EMPTY)
- goto out;
- queue->next_buf_to_fill = (queue->next_buf_to_fill + 1) %
- QDIO_MAX_BUFFERS_PER_Q;
- atomic_set(&queue->state, QETH_OUT_Q_UNLOCKED);
+ return -EBUSY;
+ queue->next_buf_to_fill = (index + 1) % QDIO_MAX_BUFFERS_PER_Q;
qeth_fill_buffer(queue, buffer, skb, hdr, offset, hd_len);
qeth_flush_buffers(queue, index, 1);
return 0;
-out:
- atomic_set(&queue->state, QETH_OUT_Q_UNLOCKED);
- return -EBUSY;
}
EXPORT_SYMBOL_GPL(qeth_do_send_packet_fast);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 25a0f381bcd5..009de7248cc7 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -698,7 +698,7 @@ static int qeth_l2_xmit_iqd(struct qeth_card *card, struct sk_buff *skb,
rc = -E2BIG;
goto out;
}
- rc = qeth_do_send_packet_fast(card, queue, skb, hdr, data_offset,
+ rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
sizeof(*hdr) + data_offset);
out:
if (rc)
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index ab661a431f7c..733f94cbd4d4 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2771,8 +2771,8 @@ static netdev_tx_t qeth_l3_hard_start_xmit(struct sk_buff *skb,
rc = qeth_do_send_packet(card, queue, new_skb, hdr, hd_len,
hd_len, elements);
} else
- rc = qeth_do_send_packet_fast(card, queue, new_skb, hdr,
- data_offset, hd_len);
+ rc = qeth_do_send_packet_fast(queue, new_skb, hdr, data_offset,
+ hd_len);
if (!rc) {
card->stats.tx_packets++;
--
2.13.5
^ 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