* Re: [RFC PATCH net-next] bridge: ability to disable forwarding on a port
From: roopa @ 2015-01-20 13:59 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: Scott Feldman, stephen@networkplumber.org, David S. Miller,
Jamal Hadi Salim, Jiří Pírko, Arad, Ronen,
Thomas Graf, john fastabend, vyasevic@redhat.com, Netdev,
Wilson Kok, Andy Gospodarek
In-Reply-To: <54BDFF20.9020909@intel.com>
On 1/19/15, 11:09 PM, Samudrala, Sridhar wrote:
>
> On 1/19/2015 10:20 PM, roopa wrote:
>> On 1/18/15, 11:37 PM, Scott Feldman wrote:
>>
>> <snip..>
>>>
>>> Not sure. I don't know the use case, but I think I might have heard
>>> that
>>> there could be a case
>>> where a switch port could be bridged with a vm's port running on the
>>> switch. (?)
>>>>
>>>> Ignoring the above usecase for a bit. And thinking through this
>>>> again, It
>>>> appears that this check should
>>>> be only on the ingress port, no ?
>>>>
>>>> If the ingress bridge port is an offloaded port, don't flood or
>>>> forward
>>>> because hardware has already done it.
>>>> And this is best done with the offload feature flag on the bridge
>>>> port.
>>> That's assuming hardware did the flood. Maybe your other option to
>>> mark the skb if already flooded by hw is best. That's enough info for
>>> the bridge driver to make a decision to flood or not to the other
>>> ports, and it's an implementation decision for the driver/device to do
>>> the flood offload, if desired, and mark the skb if it did.
>>
>> Still thinking we can just use the offload feature flag here. How
>> about avoid forwarding only if both src and dst ports have
>> forwarding offloaded/accelerated by a switch asic ?. That should
>> cover all cases.
>>>
>>> Btw, you're still saying flood or forward, but in my mind we're
>>> talking about flood only: flood of unknown unicast or flood of
>>> bcast/mcast pkts. Forwarding would be for known-unicast pkts, which
>>> should even involve the bridge driver since that forwarding is
>>> offloaded to the device.
>>
>> I was also taking into account pkts copied to the CPU due to an acl
>> rule such as log.
>> These unicast pkts can come to cpu even if it is already forwarded in
>> hw.
>
> Do you have a switch port netdev that corresponds to CPU port? Or are
> they seen on the bridge device?
No we dont have a netdev for the CPU port. These show up on the netdev
corresponding to the ingress front panel port.
^ permalink raw reply
* Re: Bug: mv643xxx fails with highmem
From: Russell King - ARM Linux @ 2015-01-20 13:41 UTC (permalink / raw)
To: Ezequiel Garcia, David Miller, Linus Torvalds
Cc: Nimrod Andy, Fabio Estevam, netdev, fugang.duan
In-Reply-To: <20141221165106.GN11285@n2100.arm.linux.org.uk>
Ping. What's happening on this?
I guess we don't care about regressions in the Linux kernel?
This bug breaks one of my machines, and I'm having to patch around it
by reverting the first two hunks of 69ad0dd7af22, which is only correct
for platforms where dma_unmap_page and dma_unmap_single result in the
same underlying code being used.
I'm not convinced that it's directly caused by 69ad0dd7af22 though.
On Sun, Dec 21, 2014 at 04:51:06PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 18, 2014 at 10:13:19AM -0300, Ezequiel Garcia wrote:
> > On 12/17/2014 09:03 PM, Russell King - ARM Linux wrote:
> > > However, exactly how it occurs, I don't know. My understanding from
> > > reading the various feature flags was that NETIF_F_HIGHDMA was required
> > > for highmem (see illegal_highdma()) so as this isn't set, we shouldn't
> > > be seeing highmem fragments - which is why I asked the question in my
> > > original email.
> > >
> > > If you want me to revert my fix above, and reproduce again, I can
> > > certainly try that - or put a WARN_ON_ONCE(PageHighMem(this_frag->page.p))
> > > in there, but I seem to remember that it wasn't particularly useful as
> > > the backtrace didn't show where the memory actually came from.
> > >
> >
> > No, that's OK. Thanks a lot for all the details. I'll try to come up with a
> > fix soon.
>
> Well, I decided to add the WARN_ON_ONCE() and re-test. This I provoked
> by touching etna_viv/src/etnaviv/etna_bo.c, and re-running make (etnaviv
> is on a shared NFS mount.)
>
> WARNING: CPU: 0 PID: 0 at /home/rmk/git/linux-cubox/drivers/net/ethernet/marvell/mv643xx_eth.c:884 mv643xx_eth_xmit+0x850/0x8dc()
> Modules linked in: bnep rfcomm bluetooth nfsd exportfs ext3 jbd ext2 etnaviv(C)
> snd_soc_spdif_tx orion_wdt snd_soc_kirkwood dove vmeta bmm_dmabuf hwmon snd_soc_kirkwood_spdif
> CPU: 0 PID: 0 Comm: swapper Tainted: G C 3.18.0+ #1056
> Backtrace:
> [<c0011f54>] (dump_backtrace) from [<c0012228>] (show_stack+0x18/0x1c)
> r6:00000374 r5:00000009 r4:00000000 r3:00000000
> [<c0012210>] (show_stack) from [<c04992d8>] (dump_stack+0x20/0x28)
> [<c04992b8>] (dump_stack) from [<c0050be4>] (warn_slowpath_common+0x6c/0x8c)
> [<c0050b78>] (warn_slowpath_common) from [<c0050c28>] (warn_slowpath_null+0x24/0x2c)
> r8:c064ea80 r7:e8a5d880 r6:d00d0d70 r5:e614877c r4:00000001
> [<c0050c04>] (warn_slowpath_null) from [<c02fee9c>] (mv643xx_eth_xmit+0x850/0x8dc)
> [<c02fe64c>] (mv643xx_eth_xmit) from [<c03b94fc>] (dev_hard_start_xmit+0x19c/0x328)
> r10:c0648054 r9:d0261f60 r8:e6148000 r7:d00ec1c0 r6:c0648040 r5:e623ec00
> r4:d013c580
> [<c03b9360>] (dev_hard_start_xmit) from [<c03d2728>] (sch_direct_xmit+0x148/0x24c)
> r10:e623ec00 r9:e63a4580 r8:e6148000 r7:e61f4e00 r6:c063e000 r5:00000000
> r4:00000000
> [<c03d25e0>] (sch_direct_xmit) from [<c03b985c>] (__dev_queue_xmit+0x1d4/0x590)
> r10:e623ec00 r9:00000000 r8:00000000 r7:e6148000 r6:c063e000 r5:e61f4e00
> r4:d0163e70
> [<c03b9688>] (__dev_queue_xmit) from [<c03b9c40>] (dev_queue_xmit+0x14/0x18)
> r10:c063e000 r9:00000000 r8:00000000 r7:d0163e70 r6:0000000e r5:d00caa00
> r4:d00caa94
> [<c03b9c2c>] (dev_queue_xmit) from [<c043c58c>] (ip6_finish_output2+0x1a0/0x524)[<c043c3ec>] (ip6_finish_output2) from [<c043e008>] (ip6_output+0xb4/0x174)
> r10:d0163e70 r9:c063e000 r8:c066f678 r7:00000000 r6:d0163e70 r5:00000000
> r4:000021c0
> [<c043df54>] (ip6_output) from [<c043c114>] (ip6_xmit+0x278/0x550)
> r7:00000000 r6:00000001 r5:00000000 r4:001463b6
> [<c043be9c>] (ip6_xmit) from [<c0466fbc>] (inet6_csk_xmit+0x74/0xa8)
> r10:d0163e70 r9:00000020 r8:d00d2080 r7:d00d25a0 r6:d0163e70 r5:00000000
> r4:d00d2080
> [<c0466f48>] (inet6_csk_xmit) from [<c03f87ac>] (tcp_transmit_skb+0x494/0x990)
> r7:e6094100 r6:c0658910 r5:00000020 r4:ffff5165
> [<c03f8318>] (tcp_transmit_skb) from [<c03f9970>] (tcp_write_xmit+0x138/0xc1c)
> r10:00002178 r9:00000000 r8:00002ca0 r7:00000006 r6:00000594 r5:d0163dc0
> r4:d00d2080
> [<c03f9838>] (tcp_write_xmit) from [<c03fa4d4>] (__tcp_push_pending_frames+0x38/0x98)
> r10:00000002 r9:00000078 r8:d03d7600 r7:d00d25a0 r6:d02841c0 r5:e448f778
> r4:d00d2080
> [<c03fa49c>] (__tcp_push_pending_frames) from [<c03f567c>] (tcp_rcv_established+0x15c/0x600)
> r4:d00d2080
> [<c03f5520>] (tcp_rcv_established) from [<c0461918>] (tcp_v6_do_rcv+0x2bc/0x46c) r10:00000002 r9:00000078 r8:d03d7600 r7:d00d25a0 r6:00000000 r5:d00d2080
> r4:d02841c0
> [<c046165c>] (tcp_v6_do_rcv) from [<c0462828>] (tcp_v6_rcv+0x7f8/0x810)
> r8:00000000 r7:d00d2080 r6:c063e000 r5:c066f678 r4:d02841c0
> [<c0462030>] (tcp_v6_rcv) from [<c043e750>] (ip6_input+0xec/0x424)
> r10:c066f678 r9:c052a75c r8:d02841c0 r7:c0649720 r6:00000006 r5:e6168400
> r4:00000006
> [<c043e664>] (ip6_input) from [<c043e100>] (ip6_rcv_finish+0x38/0xa4)
> r10:e6168400 r9:e6148000 r8:d02841c0 r7:00000001 r6:c066f678 r5:00000000
> r4:d02841c0 r3:c043e664
> [<c043e0c8>] (ip6_rcv_finish) from [<c043e494>] (ipv6_rcv+0x328/0x4f8)
> r4:e448f750 r3:00000000
> [<c043e16c>] (ipv6_rcv) from [<c03b43c0>] (__netif_receive_skb_core+0x2fc/0x5d0) r10:d02841c0 r9:c0649050 r8:c06480c4 r7:e6148000 r6:00000000 r5:0000dd86
> r4:c043e16c
> [<c03b40c4>] (__netif_receive_skb_core) from [<c03b6bac>] (__netif_receive_skb+0x2c/0x88)
> r10:00000001 r9:d02841c0 r8:e61484e0 r7:e8a5b310 r6:2cc7fffe r5:00000003
> r4:d02841c0
> [<c03b6b80>] (__netif_receive_skb) from [<c03b6d30>] (netif_receive_skb_internal+0x2c/0x68)
> r5:00000003 r4:d02841c0
> [<c03b6d04>] (netif_receive_skb_internal) from [<c03b7690>] (napi_gro_receive+0x7c/0xa8)
> r4:d02841c0
> [<c03b7614>] (napi_gro_receive) from [<c0300738>] (mv643xx_eth_poll+0x58c/0x6ac) r5:e6148000 r4:e614864c
> [<c03001ac>] (mv643xx_eth_poll) from [<c03b7370>] (net_rx_action+0xa4/0x1a8)
> r10:c0658910 r9:c0675940 r8:c0675940 r7:0000012c r6:00000040 r5:e61485c0
> r4:c03001ac
> [<c03b72cc>] (net_rx_action) from [<c00533c0>] (__do_softirq+0xf0/0x214)
> r10:00000003 r9:00000101 r8:c063e000 r7:00000003 r6:c0677480 r5:c067748c
> r4:00000000
> [<c00532d0>] (__do_softirq) from [<c005377c>] (irq_exit+0xac/0xfc)
> r10:e6ffcd40 r9:560f5815 r8:00000000 r7:0000001e r6:00000000 r5:00000000
> r4:c063e000
> [<c00536d0>] (irq_exit) from [<c0085330>] (__handle_domain_irq+0x7c/0xc0)
> r4:c065ef68 r3:00010001
> [<c00852b4>] (__handle_domain_irq) from [<c000fb0c>] (handle_IRQ+0x24/0x28)
> r8:00000001 r7:c063ff74 r6:ffffffff r5:600f0013 r4:c0077408 r3:c063ff40
> [<c000fae8>] (handle_IRQ) from [<c0008600>] (dove_legacy_handle_irq+0x34/0x5c)
> [<c00085cc>] (dove_legacy_handle_irq) from [<c0012ce0>] (__irq_svc+0x40/0x74)
> Exception stack(0xc063ff40 to 0xc063ff88)
> ff40: 00000000 0001114c c0658324 c001d940 c063e000 c06470c8 c06759fe c06759fe
> ff60: 00000001 560f5815 e6ffcd40 c063ff9c c063ff88 c063ff88 c000fc38 c0077408
> ff80: 600f0013 ffffffff
> [<c0077354>] (cpu_startup_entry) from [<c04952c4>] (rest_init+0x78/0x90)
> r7:ffffffff r3:c04b0554
> [<c049524c>] (rest_init) from [<c0609ca8>] (start_kernel+0x34c/0x3b0)
> r4:c06479b0 r3:00000001
> [<c060995c>] (start_kernel) from [<00008070>] (0x8070)
>
> Obviously, the useful bit is only down to just above
> __tcp_push_pending_frames(), since that's traces back to the TCP socket's
> send queue.
>
> If I had to guess where the highmem pages were coming from, I'd suggest
> that it was via the page cache and NFS - maybe something like GCC opens
> a new NFS file, writes to it. Pages are allocated to it, which happen
> to be allocated from highmem. NFS eventually sends these pages to the
> NFS server via TCP, which fragments the page and leaves pointers to the
> highmem page in the TCP skbuff. My NFS mounts are IPv6.
>
> For Freescale iMX6 FEC, I haven't yet been able to reproduce this there.
> The FEC has tx-checksum-ipv6 and rx-vlan-offload enabled, which the
> mv643xxx driver doesn't have. The FEC platform has twice the memory of
> the Dove platform (so has much more highmem) so I would've thought it
> would have been easier to reproduce there. Slightly different kernels
> too - Dove runs 3.18 plus additions, iMX6 is running Linus' tip from
> two days ago plus very similar additions (but same GPU and X server
> code.)
>
> Other stuff... Dove memory is 0 - 0x3fffffff. mv643xxx DMA mask
> uninitialised (coherent DMA mask set to 0xffffffff).
>
> iMX6 is 0x10000000 - 0x8fffffff, with the DMA mask defaulting to
> point at the coherent mask (due to being DT based) which is
> 0xffffffff.
>
> Here's the diff of the ethtool -k output:
>
> --- eth0.dove 2014-12-21 16:38:39.000000000 +0000
> +++ eth0.imx6 2014-12-21 16:38:50.792453703 +0000
> @@ -3,7 +3,7 @@
> tx-checksumming: on
> tx-checksum-ipv4: on
> tx-checksum-ip-generic: off [fixed]
> - tx-checksum-ipv6: off [fixed]
> + tx-checksum-ipv6: on
> tx-checksum-fcoe-crc: off [fixed]
> tx-checksum-sctp: off [fixed]
> scatter-gather: on
> @@ -17,7 +17,7 @@
> generic-segmentation-offload: on
> generic-receive-offload: on
> large-receive-offload: off [fixed]
> -rx-vlan-offload: off [fixed]
> +rx-vlan-offload: on
> tx-vlan-offload: off [fixed]
> ntuple-filters: off [fixed]
> receive-hashing: off [fixed]
> @@ -32,7 +32,6 @@
> tx-ipip-segmentation: off [fixed]
> tx-sit-segmentation: off [fixed]
> tx-udp_tnl-segmentation: off [fixed]
> -tx-mpls-segmentation: off [fixed]
> fcoe-mtu: off [fixed]
> tx-nocache-copy: off
> loopback: off [fixed]
>
> Hmm. I'm now wondering about this:
>
> static netdev_features_t harmonize_features(struct sk_buff *skb,
> netdev_features_t features)
> {
> ...
> if (skb->ip_summed != CHECKSUM_NONE &&
> !can_checksum_protocol(features, type)) {
> features &= ~NETIF_F_ALL_CSUM;
> } else if (illegal_highdma(skb->dev, skb)) {
> features &= ~NETIF_F_SG;
> }
>
> For Dove, can_checksum_protocol() would return false for IPv6, which
> would allow the first "if" statement to succeed, hence clearing
> NETIF_F_ALL_CSUM.
>
> This would prevent the second if() being evaluated - which seems to
> remove the check for any fragments in highmem. David - shouldn't these
> two checks be independent?
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Oliver Neukum @ 2015-01-20 13:48 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <1421756978-4093-12-git-send-email-olivier@sobrie.be>
On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote:
> When usb_queue_reset() is called it schedules a work in view of
> resetting the usb interface. When the reset work is running, it
> can be scheduled again (e.g. by the usb disconnect method of
> the driver).
>
> Consider that the reset work is queued again while the reset work
> is running and that this work leads to a forced unbinding of the
> usb interface (e.g. because a driver is bound to the interface
> and has no pre/post_reset methods - see usb_reset_device()).
> In such condition, usb_unbind_interface() gets called and this
> function calls usb_cancel_queued_reset() which does nothing
> because the flag "reset_running" is set to 1. The second reset
> work that has been scheduled is therefore not cancelled.
> Later, the usb_reset_device() tries to rebind the interface.
> If it fails, then the usb interface context which contain the
> reset work struct is freed and it most likely crash when the
> second reset work tries to be run.
>
> The following flow shows the problem:
> * usb_queue_reset_device()
> * __usb_queue_reset_device() <- If the reset work is queued after here, then
> reset_running = 1 it will never be cancelled.
> usb_reset_device()
> usb_forced_unbind_intf()
> usb_driver_release_interface()
> usb_unbind_interface()
> driver->disconnect()
> usb_queue_reset_device() <- second reset
That is the sledgehammer approach. Wouldn't it be better to guarantee
that usb_queue_reset_device() be a nop when reset_running==1 ?
> usb_cancel_queued_reset() <- does nothing because
> the flag reset_running
> is set
> usb_unbind_and_rebind_marked_interfaces()
> usb_rebind_intf()
> device_attach()
> driver->probe() <- fails (no more drivers hold a reference to
> the usb interface)
> reset_running = 0
> * hub_event()
> usb_disconnect()
> usb_disable_device()
> kobject_release()
> device_release()
> usb_release_interface()
> kfree(intf) <- usb interface context is released
> while we still have a pending reset
> work that should be run
>
> To avoid this problem, we use a delayed work so that if the reset
> work is currently run, we can avoid further call to
> __usb_queue_reset_device() work by using cancel_delayed_work().
> Unfortunately it increases the size of the usb_interface structure...
Regards
Oliver
--
Oliver Neukum <oneukum@suse.de>
^ permalink raw reply
* Re: Bug: mv643xxx fails with highmem
From: Ezequiel Garcia @ 2015-01-20 13:42 UTC (permalink / raw)
To: Russell King - ARM Linux, David Miller, Linus Torvalds
Cc: Nimrod Andy, Fabio Estevam, netdev, fugang.duan
In-Reply-To: <20150120134104.GA12253@n2100.arm.linux.org.uk>
On 01/20/2015 10:41 AM, Russell King - ARM Linux wrote:
> Ping. What's happening on this?
>
> I guess we don't care about regressions in the Linux kernel?
>
Sorry, a holiday trip got in the middle of this.
I'll try to submit a fix this week.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* PHYless ethernet switch MAC-MAC serdes connection
From: Vijay @ 2015-01-20 13:20 UTC (permalink / raw)
To: netdev
Hello All,
I have a custom board with Marvell 88E6046 ethernet switch which is
connected directly to freescale P1010 processor.
+------------+
| | +------------+
| | SGMII | |----- p0
| eth0 |-------------| p9 |
| | | |----- p1
| | +------------+
| | 88e6086 (Switch)
| |
| |
| | SGMII +-------------+
| eth1 |-------------| |
+--------------+ +-------------+
FS P1010 88E1512 ( PHY)
My dts looks like this,
mii_bus0: mdio@24000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,etsec2-mdio";
reg = <0x24000 0x1000 0xb0030 0x4>;
/* No PHY on external MDIO for 88e6086 Switch,
PHYless direct connection*/
/*------------*/
/* External PHY 88E1512 for eth1 */
phy1: ethernet-phy@1 {
interrupt-parent = <&mpic>;
interrupts = <3 1>;
reg = <0x1>;
};
};
mdio@25000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,etsec2-tbi";
reg = <0x25000 0x1000 0xb1030 0x4>;
tbi0: tbi-phy@11 { /* TBI
needed by gianfar for fixed-link eth0 */
reg = <0x11>;
device_type = "tbi-phy";
};
};
mdio@26000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,etsec2-tbi";
reg = <0x26000 0x1000 0xb1030 0x4>;
tbi1: tbi-phy@12 {
reg = <0x12>;
device_type = "tbi-phy";
};
};
enet1: ethernet@b1000 { /*switch
connected here*/
#address-cells = <1>;
#size-cells = <1>;
device_type = "network";
model = "eTSEC";
compatible = "fsl,etsec2";
reg = <0xb1000 0x1000>;
interrupts = <35 2 36 2 40 2>;
fsl,num_rx_queues = <0x1>;
fsl,num_tx_queues = <0x1>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupt-parent = <&mpic>;
fixed-link = <0 1 1000 0 0>;
tbi-handle = <&tbi0>;
phy-mode = "sgmii";
queue-group@0 {
#address-cells = <1>;
#size-cells = <1>;
reg = <0xb1000 0x1000>;
rx-bit-map = <0xff>;
tx-bit-map = <0xff>;
interrupts = <35 2 36 2 40 2>;
};
};
enet2: ethernet@b2000 {
#address-cells = <1>;
#size-cells = <1>;
device_type = "network";
model = "eTSEC";
compatible = "fsl,etsec2";
fsl,num_rx_queues = <0x1>;
fsl,num_tx_queues = <0x1>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupt-parent = <&mpic>;
phy-handle = <&phy1>;
tbi-handle = <&tbi1>;
phy-connection-type = "sgmii";
ptimer-handle = < &ptp_timer >;
queue-group@0 {
#address-cells = <1>;
#size-cells = <1>;
reg = <0xb2000 0x1000>;
rx-bit-map = <0xff>;
tx-bit-map = <0xff>;
interrupts = <31 2 32 2 33 2>;
};
};
dsa@0 {
compatible = "marvell,dsa";
#address-cells = <2>;
#size-cells = <0>;
dsa,ethernet = <&enet1>;
dsa,mii-bus = <&mii_bus0>;
switch@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <31 0>; /* Switch at SMI Add 0x1f */
port@0 {
reg = <0>;
label = "lan1";
};
port@1 {
reg = <1>;
label = "lan2";
};
port@9 {
reg = <9>;
label = "cpu";
};
};
};
I have disabled auto-negotiation in gianfar driver for fixed-link eth0
and associated TBI-PHY, so that SGMII link at 1Gb/ps can be forced with
switch port 9 GMII MAC.
Linux DSA driver is able to detect and configure switch. It discover
switch at external MDIO bus, DSA configure 'eth0' as master and 'lan1'
and 'lan2' as slave port.
After assigning IPs to eth0 (Switch) and eth1 ( with external PHY) like,
ifconfig eth0 up
ifconfig lan1 xx.xx.xx.x1 up
ifconfig lan2 xx.xx.xx.x2 up
ifconfig eth1 xx.xx.xx.x3 up
When I ping from any other host on network, only lan1 is able to
respond, because it was configured first. If any other port is pinged it
will be able to respond
only if 'lan1 is connected to network'. If ethernet cable from lan1 is
removed, rest of ports will not be able to respond, though they will be
receiving packets but
their TX counter will not increase.
It seems whichever port is configured first ( lan1 here ) will be able
to transmit packets. If eth1 ( connected to Phy not switch) is
configured first then only this
will be able to respond. All other ports then depends on eth1.
I tried to move 'eth1' to another mdio bus ( mdio@25000 ) by modifying
dts file above. But it seems 'PHY' and 'Switch' will only be detected if
they are on
mdio@24000.
What could be the reason of such strange behaviour ?
Why only one interface is controlling the bus its either eth0/lan0/lan1
or eth1. Rx counter of every interface increments but only one of these
have Tx counter incremented. If I disable switch ports ( lan0/lan1),
strangely eth1 also does not receive any packets ( though its Rx counter
increments but Tx is 0). Is it happening because dsa@0 also controlling
phy1 on same mdio bus ?
^ permalink raw reply
* [PATCH 0/3 net-next] rhashtable: Notify on resize to allow signaling interrupted dumps
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
Netlink dumps have a traditional weakness of being interruptible and
non-atomic. Once the message buffer has been filled, all protective
locks must be released and the buffer is handed over to the user to
copy the data. The dumping then continues from the previously stored
offsets in the data structures. Due to dropping of the locks, mutations
can occur.
This first series introduces a new notification mechanism which allows
users of rhashtable to bump a sequence number on resizes and report
an inconsistent dump to user space.
As discussed in the netdev thread ("[PATCH 7/9] rhashtable: Per bucket
locks & deferred expansion/shrinking").
Thomas Graf (3):
rhashtable: Provide notifier for deferred resizes
netlink: Mark dumps as inconsistent which have been interrupted by a
resize
netlink: Lock out table resizes while dumping Netlink sockets
include/linux/rhashtable.h | 10 +++++++++-
lib/rhashtable.c | 13 ++++++++++---
net/netlink/af_netlink.c | 10 ++++++++++
net/netlink/af_netlink.h | 1 +
net/netlink/diag.c | 4 ++++
5 files changed, 34 insertions(+), 4 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
In-Reply-To: <cover.1421759056.git.tgraf@suug.ch>
Lock out table resizes while dumping Netlink sockets to user space.
This keeps disruptions to a minimum for readers which don't handle
the NLM_F_DUMP_INTR flag.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/netlink/diag.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
index 50aa385..be4ea6e 100644
--- a/net/netlink/diag.c
+++ b/net/netlink/diag.c
@@ -114,6 +114,8 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
req = nlmsg_data(cb->nlh);
cb->seq = atomic_read(&nl_table_seq);
+ mutex_lock(&ht->mutex);
+
for (i = 0; i < htbl->size; i++) {
struct rhash_head *pos;
@@ -161,6 +163,7 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
num++;
}
done:
+ mutex_unlock(&ht->mutex);
cb->args[0] = num;
cb->args[1] = protocol;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/3] rhashtable: Provide notifier for deferred resizes
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
In-Reply-To: <cover.1421759056.git.tgraf@suug.ch>
Allowing users of rhashtable to bump a sequence counter and invalidate
Netlink dumps which have been interrupted by a deferred resize.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
include/linux/rhashtable.h | 10 +++++++++-
lib/rhashtable.c | 13 ++++++++++---
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index a2562ed..b711d16 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -1,7 +1,7 @@
/*
* Resizable, Scalable, Concurrent Hash Table
*
- * Copyright (c) 2014 Thomas Graf <tgraf@suug.ch>
+ * Copyright (c) 2014-2015 Thomas Graf <tgraf@suug.ch>
* Copyright (c) 2008-2014 Patrick McHardy <kaber@trash.net>
*
* Based on the following paper by Josh Triplett, Paul E. McKenney
@@ -64,6 +64,11 @@ typedef u32 (*rht_obj_hashfn_t)(const void *data, u32 seed);
struct rhashtable;
+enum rht_resize_op {
+ RHT_GROWING,
+ RHT_SHRINKING,
+};
+
/**
* struct rhashtable_params - Hash table construction parameters
* @nelem_hint: Hint on number of elements, should be 75% of desired size
@@ -79,6 +84,7 @@ struct rhashtable;
* @obj_hashfn: Function to hash object
* @grow_decision: If defined, may return true if table should expand
* @shrink_decision: If defined, may return true if table should shrink
+ * @resize_notify: Called when a table resize is scheduled.
*
* Note: when implementing the grow and shrink decision function, min/max
* shift must be enforced, otherwise, resizing watermarks they set may be
@@ -100,6 +106,8 @@ struct rhashtable_params {
size_t new_size);
bool (*shrink_decision)(const struct rhashtable *ht,
size_t new_size);
+ void (*resize_notify)(const struct rhashtable *ht,
+ enum rht_resize_op op);
};
/**
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 84a78e3..a4449c4 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -1,7 +1,7 @@
/*
* Resizable, Scalable, Concurrent Hash Table
*
- * Copyright (c) 2014 Thomas Graf <tgraf@suug.ch>
+ * Copyright (c) 2014-2015 Thomas Graf <tgraf@suug.ch>
* Copyright (c) 2008-2014 Patrick McHardy <kaber@trash.net>
*
* Based on the following paper:
@@ -489,10 +489,17 @@ static void rht_deferred_worker(struct work_struct *work)
mutex_lock(&ht->mutex);
tbl = rht_dereference(ht->tbl, ht);
- if (ht->p.grow_decision && ht->p.grow_decision(ht, tbl->size))
+ if (ht->p.grow_decision && ht->p.grow_decision(ht, tbl->size)) {
+ if (ht->p.resize_notify)
+ ht->p.resize_notify(ht, RHT_GROWING);
+
rhashtable_expand(ht);
- else if (ht->p.shrink_decision && ht->p.shrink_decision(ht, tbl->size))
+ } else if (ht->p.shrink_decision && ht->p.shrink_decision(ht, tbl->size)) {
+ if (ht->p.resize_notify)
+ ht->p.resize_notify(ht, RHT_SHRINKING);
+
rhashtable_shrink(ht);
+ }
mutex_unlock(&ht->mutex);
}
--
1.9.3
^ permalink raw reply related
* [PATCH 2/3] netlink: Mark dumps as inconsistent which have been interrupted by a resize
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
In-Reply-To: <cover.1421759056.git.tgraf@suug.ch>
A deferred resize of nl_table causes the offsets that Netlink diag keeps
to become inaccurate. Mark the dump as inconsistent and have user space
request a new dump.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/netlink/af_netlink.c | 10 ++++++++++
net/netlink/af_netlink.h | 1 +
net/netlink/diag.c | 1 +
3 files changed, 12 insertions(+)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7a94185..e214557 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -91,6 +91,9 @@ static inline int netlink_is_kernel(struct sock *sk)
struct netlink_table *nl_table;
EXPORT_SYMBOL_GPL(nl_table);
+atomic_t nl_table_seq;
+EXPORT_SYMBOL_GPL(nl_table_seq);
+
static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
static int netlink_dump(struct sock *sk);
@@ -3071,6 +3074,12 @@ static const struct net_proto_family netlink_family_ops = {
.owner = THIS_MODULE, /* for consistency 8) */
};
+static void nl_table_resize_notify(const struct rhashtable *ht,
+ enum rht_resize_op op)
+{
+ atomic_inc(&nl_table_seq);
+}
+
static int __net_init netlink_net_init(struct net *net)
{
#ifdef CONFIG_PROC_FS
@@ -3124,6 +3133,7 @@ static int __init netlink_proto_init(void)
.max_shift = 16, /* 64K */
.grow_decision = rht_grow_above_75,
.shrink_decision = rht_shrink_below_30,
+ .resize_notify = nl_table_resize_notify,
};
if (err != 0)
diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
index 7518375..51c23c0 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -74,5 +74,6 @@ struct netlink_table {
extern struct netlink_table *nl_table;
extern rwlock_t nl_table_lock;
+extern atomic_t nl_table_seq;
#endif
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
index 3ee63a3..50aa385 100644
--- a/net/netlink/diag.c
+++ b/net/netlink/diag.c
@@ -112,6 +112,7 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
int ret = 0, num = 0, i;
req = nlmsg_data(cb->nlh);
+ cb->seq = atomic_read(&nl_table_seq);
for (i = 0; i < htbl->size; i++) {
struct rhash_head *pos;
--
1.9.3
^ permalink raw reply related
* [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1421759776-376-1-git-send-email-_govind@gmx.com>
With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
is done only when work_done == budget. When in busy_poll is we return 0 in
napi_poll. We should return budget. Also do not return workdone > budget.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 1d1147c..ebcbe92 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -3175,7 +3175,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
}
#endif
if (!bnx2x_fp_lock_napi(fp))
- return work_done;
+ return budget;
for_each_cos_in_tx_queue(fp, cos)
if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
@@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
/* must not complete if we consumed full budget */
if (work_done >= budget) {
bnx2x_fp_unlock_napi(fp);
- break;
+ return budget;
}
}
--
2.2.2
^ permalink raw reply related
* [PATCH net 1/2] enic: fix rx napi poll return value
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1421759776-376-1-git-send-email-_govind@gmx.com>
With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
is done only when work_done == budget. When we are in busy_poll we return 0 in
napi_poll. We should return budget.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index b29e027..e356afa 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1335,7 +1335,7 @@ static int enic_poll_msix_rq(struct napi_struct *napi, int budget)
int err;
if (!enic_poll_lock_napi(&enic->rq[rq]))
- return work_done;
+ return budget;
/* Service RQ
*/
--
2.2.2
^ permalink raw reply related
* [PATCH net 0/2] fix napi return value
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
Since d75b1ade567ffab ("net: less interrupt masking in NAPI") we should return
budget for napi repoll.
In enic, we return 0 when busy_poll is happening. Fix this by returning budget.
Looks like bnx2x needs this fix as well. On bnx2x I performed compile
test only. I do not have hardware.
Govindarajulu Varadarajan (2):
enic: fix rx napi poll return value
bnx2x: fix napi poll return value for repoll
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
drivers/net/ethernet/cisco/enic/enic_main.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.2.2
^ permalink raw reply
* Re: [PATCH 04/11] hso: fix memory leak in hso_create_rfkill()
From: Oliver Neukum @ 2015-01-20 13:13 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <1421756978-4093-5-git-send-email-olivier@sobrie.be>
On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote:
> When the rfkill interface was created, a buffer containing the name
> of the rfkill node was allocated. This buffer was never freed when the
> device disappears.
>
> To fix the problem, we put the name given to rfkill_alloc() in
> the hso_net structure.
>
> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> ---
> drivers/net/usb/hso.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index 470ef9e..a49ac2e 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -153,6 +153,7 @@ struct hso_net {
> struct hso_device *parent;
> struct net_device *net;
> struct rfkill *rfkill;
> + char name[8];
>
> struct usb_endpoint_descriptor *in_endp;
> struct usb_endpoint_descriptor *out_endp;
> @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
> {
> struct hso_net *hso_net = dev2net(hso_dev);
> struct device *dev = &hso_net->net->dev;
> - char *rfkn;
>
> - rfkn = kzalloc(20, GFP_KERNEL);
> - if (!rfkn)
> - dev_err(dev, "%s - Out of memory\n", __func__);
> -
> - snprintf(rfkn, 20, "hso-%d",
> + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d",
> interface->altsetting->desc.bInterfaceNumber);
That number is not unique. Indeed it will be identical for all devices.
Regards
Oliver
--
Oliver Neukum <oneukum@suse.de>
^ permalink raw reply
* [PATCH net-next] act_connmark: Add missing dependency on NF_CONNTRACK_MARK
From: Thomas Graf @ 2015-01-20 12:44 UTC (permalink / raw)
To: davem; +Cc: netdev, Felix Fietkau, Jamal Hadi Salim
Depending on NETFILTER is not sufficient to ensure the presence of the
'mark' field in nf_conn, also needs to depend on NF_CONNTRACK_MARK.
Fixes: 22a5dc ("net: sched: Introduce connmark action")
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/sched/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 475e35e..7a57f66 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -713,6 +713,7 @@ config NET_ACT_BPF
config NET_ACT_CONNMARK
tristate "Netfilter Connection Mark Retriever"
depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
+ depends on NF_CONNTRACK_MARK
---help---
Say Y here to allow retrieving of conn mark
--
1.9.3
^ permalink raw reply related
* [PATCH 05/11] hso: fix small indentation error
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-5-git-send-email-olivier@sobrie.be>
Simply remove the useless extra tab.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index a49ac2e..2f2343d 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2206,8 +2206,8 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
for (i = 0; i < serial->num_rx_urbs; i++) {
if (serial->rx_urb[i]) {
- usb_kill_urb(serial->rx_urb[i]);
- serial->rx_urb_filled[i] = 0;
+ usb_kill_urb(serial->rx_urb[i]);
+ serial->rx_urb_filled[i] = 0;
}
}
serial->curr_rx_urb_idx = 0;
--
2.2.0
^ permalink raw reply related
* [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-11-git-send-email-olivier@sobrie.be>
When usb_queue_reset() is called it schedules a work in view of
resetting the usb interface. When the reset work is running, it
can be scheduled again (e.g. by the usb disconnect method of
the driver).
Consider that the reset work is queued again while the reset work
is running and that this work leads to a forced unbinding of the
usb interface (e.g. because a driver is bound to the interface
and has no pre/post_reset methods - see usb_reset_device()).
In such condition, usb_unbind_interface() gets called and this
function calls usb_cancel_queued_reset() which does nothing
because the flag "reset_running" is set to 1. The second reset
work that has been scheduled is therefore not cancelled.
Later, the usb_reset_device() tries to rebind the interface.
If it fails, then the usb interface context which contain the
reset work struct is freed and it most likely crash when the
second reset work tries to be run.
The following flow shows the problem:
* usb_queue_reset_device()
* __usb_queue_reset_device() <- If the reset work is queued after here, then
reset_running = 1 it will never be cancelled.
usb_reset_device()
usb_forced_unbind_intf()
usb_driver_release_interface()
usb_unbind_interface()
driver->disconnect()
usb_queue_reset_device() <- second reset
usb_cancel_queued_reset() <- does nothing because
the flag reset_running
is set
usb_unbind_and_rebind_marked_interfaces()
usb_rebind_intf()
device_attach()
driver->probe() <- fails (no more drivers hold a reference to
the usb interface)
reset_running = 0
* hub_event()
usb_disconnect()
usb_disable_device()
kobject_release()
device_release()
usb_release_interface()
kfree(intf) <- usb interface context is released
while we still have a pending reset
work that should be run
To avoid this problem, we use a delayed work so that if the reset
work is currently run, we can avoid further call to
__usb_queue_reset_device() work by using cancel_delayed_work().
Unfortunately it increases the size of the usb_interface structure...
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/usb/core/driver.c | 4 +++-
drivers/usb/core/hub.c | 4 ++--
drivers/usb/core/message.c | 4 ++--
include/linux/usb.h | 2 +-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 9bffd26..c93fbbbb 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -287,7 +287,9 @@ static int usb_unbind_device(struct device *dev)
static void usb_cancel_queued_reset(struct usb_interface *iface)
{
if (iface->reset_running == 0)
- cancel_work_sync(&iface->reset_ws);
+ cancel_delayed_work_sync(&iface->reset_ws);
+ else
+ cancel_delayed_work(&iface->reset_ws);
}
/* called from driver core with dev locked */
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b649fef..52fba97 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5604,13 +5604,13 @@ EXPORT_SYMBOL_GPL(usb_reset_device);
* NOTE: We don't do any reference count tracking because it is not
* needed. The lifecycle of the work_struct is tied to the
* usb_interface. Before destroying the interface we cancel the
- * work_struct, so the fact that work_struct is queued and or
+ * delayed_work, so the fact that delayed_work is queued and or
* running means the interface (and thus, the device) exist and
* are referenced.
*/
void usb_queue_reset_device(struct usb_interface *iface)
{
- schedule_work(&iface->reset_ws);
+ schedule_delayed_work(&iface->reset_ws, 0);
}
EXPORT_SYMBOL_GPL(usb_queue_reset_device);
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f7b7713..d9f3f68 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1650,7 +1650,7 @@ static void __usb_queue_reset_device(struct work_struct *ws)
{
int rc;
struct usb_interface *iface =
- container_of(ws, struct usb_interface, reset_ws);
+ container_of(ws, struct usb_interface, reset_ws.work);
struct usb_device *udev = interface_to_usbdev(iface);
rc = usb_lock_device_for_reset(udev, iface);
@@ -1847,7 +1847,7 @@ free_interfaces:
intf->dev.type = &usb_if_device_type;
intf->dev.groups = usb_interface_groups;
intf->dev.dma_mask = dev->dev.dma_mask;
- INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
+ INIT_DELAYED_WORK(&intf->reset_ws, __usb_queue_reset_device);
intf->minor = -1;
device_initialize(&intf->dev);
pm_runtime_no_callbacks(&intf->dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447a7e2..ffb3da1 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -187,7 +187,7 @@ struct usb_interface {
struct device dev; /* interface specific device info */
struct device *usb_dev;
atomic_t pm_usage_cnt; /* usage counter for autosuspend */
- struct work_struct reset_ws; /* for resets in atomic context */
+ struct delayed_work reset_ws; /* for resets in atomic context */
};
#define to_usb_interface(d) container_of(d, struct usb_interface, dev)
--
2.2.0
^ permalink raw reply related
* [PATCH 10/11] hso: add missing cancel_work_sync in disconnect()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-10-git-send-email-olivier@sobrie.be>
For hso serial devices, two cancel_work_sync were missing in the
disconnect method.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index c916ab5..c14fc80 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3107,6 +3107,8 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_lock(&serial->parent->mutex);
serial->parent->usb_gone = 1;
mutex_unlock(&serial->parent->mutex);
+ cancel_work_sync(&serial_table[i]->async_put_intf);
+ cancel_work_sync(&serial_table[i]->async_get_intf);
hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
set_serial_by_index(i, NULL);
--
2.2.0
^ permalink raw reply related
* [PATCH 09/11] hso: update serial_table in usb disconnect method
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-9-git-send-email-olivier@sobrie.be>
The serial_table is used to map the minor number of the usb serial device
to its associated context. The table is updated in the probe method and
in hso_serial_ref_free() which is called either from the tty cleanup
method or from the usb disconnect method.
This patch ensures that the serial_table is updated in the disconnect
method and no more from the cleanup method to avoid the following
potential race condition.
- hso_disconnect() is called for usb interface "x". Because the serial
port was open and because the cleanup method of the tty_port hasn't
been called yet, hso_serial_ref_free() is not run.
- hso_probe() is called and fails for a new hso serial usb interface
"y". The function hso_free_interface() is called and iterates
over the element of serial_table to find the device associated to
the usb interface context.
If the usb interface context of usb interface "y" has been created
at the same place as for usb interface "x", then the cleanup
functions are called for usb interfaces "x" and "y" and
hso_serial_ref_free() is called for both interfaces.
- release_tty() is called for serial port linked to usb interface "x"
and possibly crash because the tty_port structure contained in the
hso_device structure has been freed.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 5b157ad..c916ab5 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2597,7 +2597,6 @@ static void hso_free_serial_device(struct hso_device *hso_dev)
if (!serial)
return;
- set_serial_by_index(serial->minor, NULL);
hso_serial_common_free(serial);
@@ -3110,6 +3109,7 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_unlock(&serial->parent->mutex);
hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
+ set_serial_by_index(i, NULL);
}
}
--
2.2.0
^ permalink raw reply related
* [PATCH 08/11] hso: move tty_unregister outside hso_serial_common_free()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-8-git-send-email-olivier@sobrie.be>
The function hso_serial_common_free() is called either by the cleanup
method of the tty or by the usb disconnect method.
In the former case, the usb_disconnect() has been already called
and the sysfs group associated to the device has been removed.
By calling tty_unregister directly from the usb_disconnect() method,
we avoid a warning due to the removal of the sysfs group of the usb
device.
Example of warning:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 778 at fs/sysfs/group.c:225 sysfs_remove_group+0x50/0x94()
sysfs group c0645a88 not found for kobject 'ttyHS5'
Modules linked in:
CPU: 0 PID: 778 Comm: kworker/0:3 Tainted: G W 3.18.0+ #105
Workqueue: events release_one_tty
[<c000dfe4>] (unwind_backtrace) from [<c000c014>] (show_stack+0x14/0x1c)
[<c000c014>] (show_stack) from [<c0016bac>] (warn_slowpath_common+0x5c/0x7c)
[<c0016bac>] (warn_slowpath_common) from [<c0016c60>] (warn_slowpath_fmt+0x30/0x40)
[<c0016c60>] (warn_slowpath_fmt) from [<c00ddd14>] (sysfs_remove_group+0x50/0x94)
[<c00ddd14>] (sysfs_remove_group) from [<c0221e44>] (device_del+0x30/0x190)
[<c0221e44>] (device_del) from [<c0221fb0>] (device_unregister+0xc/0x18)
[<c0221fb0>] (device_unregister) from [<c0221fec>] (device_destroy+0x30/0x3c)
[<c0221fec>] (device_destroy) from [<c01fe1dc>] (tty_unregister_device+0x2c/0x5c)
[<c01fe1dc>] (tty_unregister_device) from [<c029a428>] (hso_serial_common_free+0x2c/0x88)
[<c029a428>] (hso_serial_common_free) from [<c029a4c0>] (hso_serial_ref_free+0x3c/0xb8)
[<c029a4c0>] (hso_serial_ref_free) from [<c01ff430>] (release_one_tty+0x30/0x84)
[<c01ff430>] (release_one_tty) from [<c00271d4>] (process_one_work+0x21c/0x3c8)
[<c00271d4>] (process_one_work) from [<c0027758>] (worker_thread+0x3d8/0x560)
[<c0027758>] (worker_thread) from [<c002be4c>] (kthread+0xc0/0xcc)
[<c002be4c>] (kthread) from [<c0009630>] (ret_from_fork+0x14/0x24)
---[ end trace cb88537fdc8fa208 ]---
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 55074da..5b157ad 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2234,14 +2234,17 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
return 0;
}
-static void hso_serial_common_free(struct hso_serial *serial)
+static void hso_serial_tty_unregister(struct hso_serial *serial)
{
- int i;
-
if (serial->parent->dev)
device_remove_file(serial->parent->dev, &dev_attr_hsotype);
tty_unregister_device(tty_drv, serial->minor);
+}
+
+static void hso_serial_common_free(struct hso_serial *serial)
+{
+ int i;
for (i = 0; i < serial->num_rx_urbs; i++) {
/* unlink and free RX URB */
@@ -2323,6 +2326,7 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
return 0;
exit:
+ hso_serial_tty_unregister(serial);
hso_serial_common_free(serial);
return -1;
}
@@ -2683,6 +2687,7 @@ static struct hso_device *hso_create_bulk_serial_device(
return hso_dev;
exit2:
+ hso_serial_tty_unregister(serial);
hso_serial_common_free(serial);
exit:
hso_free_tiomget(serial);
@@ -3103,6 +3108,7 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_lock(&serial->parent->mutex);
serial->parent->usb_gone = 1;
mutex_unlock(&serial->parent->mutex);
+ hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
}
}
--
2.2.0
^ permalink raw reply related
* [PATCH 07/11] hso: replace reset_device work by usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-7-git-send-email-olivier@sobrie.be>
There is no need for a dedicated reset work in the hso driver since
there is already a reset work foreseen in usb_interface that does
the same.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 484e423..55074da 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -274,7 +274,6 @@ struct hso_device {
u8 usb_gone;
struct work_struct async_get_intf;
struct work_struct async_put_intf;
- struct work_struct reset_device;
struct usb_device *usb;
struct usb_interface *interface;
@@ -340,7 +339,6 @@ static void async_put_intf(struct work_struct *data);
static int hso_put_activity(struct hso_device *hso_dev);
static int hso_get_activity(struct hso_device *hso_dev);
static void tiocmget_intr_callback(struct urb *urb);
-static void reset_device(struct work_struct *data);
/*****************************************************************************/
/* Helping functions */
/*****************************************************************************/
@@ -696,7 +694,7 @@ static void handle_usb_error(int status, const char *function,
case -ETIMEDOUT:
explanation = "protocol error";
if (hso_dev)
- schedule_work(&hso_dev->reset_device);
+ usb_queue_reset_device(hso_dev->interface);
break;
default:
explanation = "unknown status";
@@ -2347,7 +2345,6 @@ static struct hso_device *hso_create_device(struct usb_interface *intf,
INIT_WORK(&hso_dev->async_get_intf, async_get_intf);
INIT_WORK(&hso_dev->async_put_intf, async_put_intf);
- INIT_WORK(&hso_dev->reset_device, reset_device);
return hso_dev;
}
@@ -3086,26 +3083,6 @@ out:
return result;
}
-static void reset_device(struct work_struct *data)
-{
- struct hso_device *hso_dev =
- container_of(data, struct hso_device, reset_device);
- struct usb_device *usb = hso_dev->usb;
- int result;
-
- if (hso_dev->usb_gone) {
- D1("No reset during disconnect\n");
- } else {
- result = usb_lock_device_for_reset(usb, hso_dev->interface);
- if (result < 0)
- D1("unable to lock device for reset: %d\n", result);
- else {
- usb_reset_device(usb);
- usb_unlock_device(usb);
- }
- }
-}
-
static void hso_serial_ref_free(struct kref *ref)
{
struct hso_device *hso_dev = container_of(ref, struct hso_device, ref);
--
2.2.0
^ permalink raw reply related
* [PATCH 06/11] hso: rename hso_dev into serial in hso_free_interface()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Olivier Sobrie
In-Reply-To: <1421756978-4093-6-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
In other functions of the driver, variables of type "struct hso_serial"
are denoted by "serial" and variables of type "struct hso_device" are
denoted by "hso_dev". This patch makes the hso_free_interface()
consistent with these notations.
Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
---
drivers/net/usb/hso.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 2f2343d..484e423 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3115,17 +3115,17 @@ static void hso_serial_ref_free(struct kref *ref)
static void hso_free_interface(struct usb_interface *interface)
{
- struct hso_serial *hso_dev;
+ struct hso_serial *serial;
int i;
for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
if (serial_table[i] &&
(serial_table[i]->interface == interface)) {
- hso_dev = dev2ser(serial_table[i]);
- tty_port_tty_hangup(&hso_dev->port, false);
- mutex_lock(&hso_dev->parent->mutex);
- hso_dev->parent->usb_gone = 1;
- mutex_unlock(&hso_dev->parent->mutex);
+ serial = dev2ser(serial_table[i]);
+ tty_port_tty_hangup(&serial->port, false);
+ mutex_lock(&serial->parent->mutex);
+ serial->parent->usb_gone = 1;
+ mutex_unlock(&serial->parent->mutex);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
}
}
--
2.2.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 04/11] hso: fix memory leak in hso_create_rfkill()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-4-git-send-email-olivier@sobrie.be>
When the rfkill interface was created, a buffer containing the name
of the rfkill node was allocated. This buffer was never freed when the
device disappears.
To fix the problem, we put the name given to rfkill_alloc() in
the hso_net structure.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 470ef9e..a49ac2e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -153,6 +153,7 @@ struct hso_net {
struct hso_device *parent;
struct net_device *net;
struct rfkill *rfkill;
+ char name[8];
struct usb_endpoint_descriptor *in_endp;
struct usb_endpoint_descriptor *out_endp;
@@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
{
struct hso_net *hso_net = dev2net(hso_dev);
struct device *dev = &hso_net->net->dev;
- char *rfkn;
- rfkn = kzalloc(20, GFP_KERNEL);
- if (!rfkn)
- dev_err(dev, "%s - Out of memory\n", __func__);
-
- snprintf(rfkn, 20, "hso-%d",
+ snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d",
interface->altsetting->desc.bInterfaceNumber);
- hso_net->rfkill = rfkill_alloc(rfkn,
+ hso_net->rfkill = rfkill_alloc(hso_net->name,
&interface_to_usbdev(interface)->dev,
RFKILL_TYPE_WWAN,
&hso_rfkill_ops, hso_dev);
if (!hso_net->rfkill) {
dev_err(dev, "%s - Out of memory\n", __func__);
- kfree(rfkn);
return;
}
if (rfkill_register(hso_net->rfkill) < 0) {
rfkill_destroy(hso_net->rfkill);
- kfree(rfkn);
hso_net->rfkill = NULL;
dev_err(dev, "%s - Failed to register rfkill\n", __func__);
return;
--
2.2.0
^ permalink raw reply related
* [PATCH 03/11] hso: fix memory leak when device disconnects
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-3-git-send-email-olivier@sobrie.be>
In the disconnect path, tx_buffer should freed like tx_data to avoid
a memory leak when the device disconnects.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 3a6c630..470ef9e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2253,6 +2253,7 @@ static void hso_serial_common_free(struct hso_serial *serial)
/* unlink and free TX URB */
usb_free_urb(serial->tx_urb);
+ kfree(serial->tx_buffer);
kfree(serial->tx_data);
tty_port_destroy(&serial->port);
}
--
2.2.0
^ permalink raw reply related
* [PATCH 02/11] hso: fix crash when device disappears while serial port is open
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-2-git-send-email-olivier@sobrie.be>
When the device disappear, the function hso_disconnect() is called to
perform cleanup. In the cleanup function, hso_free_interface() calls
tty_port_tty_hangup() in view of scheduling a work to hang up the tty if
needed. If the port was not open then hso_serial_ref_free() is called
directly to cleanup everything. Otherwise, hso_serial_ref_free() is called
when the last fd associated to the port is closed.
For each open port, tty_release() will call the close method,
hso_serial_close(), which drops the last kref and call
hso_serial_ref_free() which unregisters, destroys the tty port
and finally frees the structure in which the tty_port structure
is included. Later, in tty_release(), more precisely when release_tty()
is called, the tty_port previously freed is accessed to cancel
the tty buf workqueue and it leads to a crash.
In view of avoiding this crash, we add a cleanup method that is called
at the end of the hangup process and we drop the last kref in this
function when all the ports have been closed, when tty_port is no
more needed and when it is safe to free the structure containing the
tty_port structure.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cb0bcc1..3a6c630 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1270,7 +1270,6 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
goto err_out;
D1("Opening %d", serial->minor);
- kref_get(&serial->parent->ref);
/* setup */
tty->driver_data = serial;
@@ -1289,7 +1288,8 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
if (result) {
hso_stop_serial_device(serial->parent);
serial->port.count--;
- kref_put(&serial->parent->ref, hso_serial_ref_free);
+ } else {
+ kref_get(&serial->parent->ref);
}
} else {
D1("Port was already open");
@@ -1339,8 +1339,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
usb_autopm_put_interface(serial->parent->interface);
mutex_unlock(&serial->parent->mutex);
-
- kref_put(&serial->parent->ref, hso_serial_ref_free);
}
/* close the requested serial port */
@@ -1391,6 +1389,16 @@ static int hso_serial_write_room(struct tty_struct *tty)
return room;
}
+static void hso_serial_cleanup(struct tty_struct *tty)
+{
+ struct hso_serial *serial = tty->driver_data;
+
+ if (!serial)
+ return;
+
+ kref_put(&serial->parent->ref, hso_serial_ref_free);
+}
+
/* setup the term */
static void hso_serial_set_termios(struct tty_struct *tty, struct ktermios *old)
{
@@ -3215,6 +3223,7 @@ static const struct tty_operations hso_serial_ops = {
.close = hso_serial_close,
.write = hso_serial_write,
.write_room = hso_serial_write_room,
+ .cleanup = hso_serial_cleanup,
.ioctl = hso_serial_ioctl,
.set_termios = hso_serial_set_termios,
.chars_in_buffer = hso_serial_chars_in_buffer,
--
2.2.0
^ permalink raw reply related
* [PATCH 01/11] hso: remove useless header file timer.h
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-1-git-send-email-olivier@sobrie.be>
No timer related function is used in this driver.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index babda7d..cb0bcc1 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -58,7 +58,6 @@
#include <linux/module.h>
#include <linux/ethtool.h>
#include <linux/usb.h>
-#include <linux/timer.h>
#include <linux/tty.h>
#include <linux/tty_driver.h>
#include <linux/tty_flip.h>
--
2.2.0
^ 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