* Re: [PATCH net-next v2 2/4] Documentation: net: phy: Add a paragraph about pause frames/flow control
From: Florian Fainelli @ 2016-11-28 17:33 UTC (permalink / raw)
To: Sebastian Frias, netdev
Cc: davem, andrew, martin.blumenstingl, mans, alexandre.torgue,
peppe.cavallaro, timur, jbrunet
In-Reply-To: <1b7425f7-9183-69d4-76e8-42eefffeb1c6@laposte.net>
On 11/28/2016 02:38 AM, Sebastian Frias wrote:
> On 27/11/16 19:44, Florian Fainelli wrote:
>> Describe that the Ethernet MAC controller is ultimately responsible for
>> dealing with proper pause frames/flow control advertisement and
>> enabling, and that it is therefore allowed to have it change
>> phydev->supported/advertising with SUPPORTED_Pause and
>> SUPPORTED_AsymPause.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> Documentation/networking/phy.txt | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/networking/phy.txt b/Documentation/networking/phy.txt
>> index 4b25c0f24201..9a42a9414cea 100644
>> --- a/Documentation/networking/phy.txt
>> +++ b/Documentation/networking/phy.txt
>> @@ -127,8 +127,9 @@ Letting the PHY Abstraction Layer do Everything
>> values pruned from them which don't make sense for your controller (a 10/100
>> controller may be connected to a gigabit capable PHY, so you would need to
>> mask off SUPPORTED_1000baseT*). See include/linux/ethtool.h for definitions
>> - for these bitfields. Note that you should not SET any bits, or the PHY may
>> - get put into an unsupported state.
>> + for these bitfields. Note that you should not SET any bits, except the
>> + SUPPORTED_Pause and SUPPORTED_AsymPause bits (see below), or the PHY may get
>> + put into an unsupported state.
>>
>> Lastly, once the controller is ready to handle network traffic, you call
>> phy_start(phydev). This tells the PAL that you are ready, and configures the
>> @@ -139,6 +140,19 @@ Letting the PHY Abstraction Layer do Everything
>> When you want to disconnect from the network (even if just briefly), you call
>> phy_stop(phydev).
>>
>> +Pause frames / flow control
>> +
>> + The PHY does not participate directly in flow control/pause frames except by
>> + making sure that the SUPPORTED_Pause and SUPPORTED_AsymPause bits are set in
>> + MII_ADVERTISE to indicate towards the link partner that the Ethernet MAC
>> + controller supports such a thing. Since flow control/pause frames generation
>> + involves the Ethernet MAC driver, it is recommended that this driver takes care
>> + of properly indicating advertisement and support for such features by setting
>> + the SUPPORTED_Pause and SUPPORTED_AsymPause bits accordingly. This can be done
>> + either before or after phy_connect()
>
> If the bits are set after phy_connect(), how does the PHY framework knows there's
> an update to the bits? Should some call be made?
You would most likely either call phy_start() to start the PHY state
machine (again) or have to re-negotiate the link with e.g:
genphy_restart_aneg().
--
Florian
^ permalink raw reply
* Re: [PATCH net] sit: Set skb->protocol properly in ipip6_tunnel_xmit()
From: Eric Dumazet @ 2016-11-28 17:34 UTC (permalink / raw)
To: David Miller; +Cc: Alexander Duyck, sfr, elicooper, netdev
In-Reply-To: <1480352023.18162.63.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 2016-11-28 at 08:53 -0800, Eric Dumazet wrote:
> On Mon, 2016-11-28 at 11:47 -0500, David Miller wrote:
> > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date: Sun, 27 Nov 2016 13:04:00 +1100
> >
> > > [Just for Dave's information]
> > >
> > > On Fri, 25 Nov 2016 13:50:17 +0800 Eli Cooper <elicooper@gmx.com> wrote:
> > >>
> > >> Similar to commit ae148b085876
> > >> ("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()"),
> > >> sit tunnels also need to update skb->protocol; otherwise, TSO/GSO packets
> > >> might not be properly segmented, which causes the packets being dropped.
> > >>
> > >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > >> Tested-by: Eli Cooper <elicooper@gmx.com>
> > >> Cc: stable@vger.kernel.org
> > >> Signed-off-by: Eli Cooper <elicooper@gmx.com>
> > >
> > > I tested this patch and it does *not* solve my problem.
> >
> > I'm torn on this patch, because it looked exactly like it would solve the
> > kind of problem Stephen is running into.
> >
> > Even though it doesn't fix his case, it seems correct to me.
> >
> > I was wondering if it was also important to set the skb->protocol
> > before the call to ip_tunnel_encap() but I couldn't find a dependency.
> >
> > In any event I'd like to see some other people review this change
> > before I apply it.
> >
> > My only other guess for Stephen's problem is somehow the SKB headers
> > aren't set up properly for what the GSO engine expects.
>
> Well, mlx4 just works, and uses GSO engine just fine.
>
> So my guess is this is a bug in Intel IGB driver.
About Eli patch : I do not believe it is needed.
Here is the path followed by SIT packet being GSO at the device layer :
We can see that ip_output() was called, and ip_output() already does :
skb->protocol = htons(ETH_P_IP);
[ 71.209437] sit: skb->protocol = dd86
[ 71.215387] skb_mac_gso_segment type 8
[ 71.219176] ------------[ cut here ]------------
[ 71.219180] WARNING: CPU: 25 PID: 12087 at net/core/dev.c:2642 skb_mac_gso_segment+0x166/0x170
[ 71.219200] CPU: 25 PID: 12087 Comm: netperf Not tainted 4.9.0-smp-DEV #362
[ 71.219203] ffffc90019d4b4d8 ffffffff813ca25b 0000000000000009 0000000000000000
[ 71.219204] ffffc90019d4b518 ffffffff810d0c13 00000a5219d4b4f0 0000000000000008
[ 71.219205] ffff881ff2524ae8 0000000000000001 0000000000000001 ffff881fe1107a9c
[ 71.219205] Call Trace:
[ 71.219209] [<ffffffff813ca25b>] dump_stack+0x4d/0x72
[ 71.219213] [<ffffffff810d0c13>] __warn+0xd3/0xf0
[ 71.219214] [<ffffffff810d0cfe>] warn_slowpath_null+0x1e/0x20
[ 71.219215] [<ffffffff8167b656>] skb_mac_gso_segment+0x166/0x170
[ 71.219216] [<ffffffff8167b719>] __skb_gso_segment+0xb9/0x140
[ 71.219218] [<ffffffff8167bb26>] validate_xmit_skb+0x136/0x2d0
[ 71.219219] [<ffffffff8167bd03>] validate_xmit_skb_list+0x43/0x70
[ 71.219221] [<ffffffff816a4817>] sch_direct_xmit+0x147/0x1a0
[ 71.219223] [<ffffffff8167c431>] __dev_queue_xmit+0x421/0x620
[ 71.219224] [<ffffffff8167c640>] dev_queue_xmit+0x10/0x20
[ 71.219226] [<ffffffff816db2d4>] ip_finish_output2+0x254/0x330
[ 71.219227] [<ffffffff816dcaa6>] ip_finish_output+0x136/0x1d0
[ 71.219228] [<ffffffff816dd4db>] ip_output+0xab/0xc0
[ 71.219230] [<ffffffff816dc970>] ? ip_fragment.constprop.58+0x80/0x80
[ 71.219231] [<ffffffff816dccb5>] ip_local_out+0x35/0x40
[ 71.219233] [<ffffffff81725aec>] iptunnel_xmit+0x14c/0x1c0
[ 71.219235] [<ffffffff8178f77a>] sit_tunnel_xmit+0x50a/0x860
[ 71.219236] [<ffffffff8167bde0>] dev_hard_start_xmit+0xb0/0x200
[ 71.219238] [<ffffffff8167c592>] __dev_queue_xmit+0x582/0x620
[ 71.219239] [<ffffffff8167c640>] dev_queue_xmit+0x10/0x20
[ 71.219241] [<ffffffff81684fb1>] neigh_direct_output+0x11/0x20
[ 71.219243] [<ffffffff81750e91>] ip6_finish_output2+0x181/0x460
[ 71.219245] [<ffffffff8178d00b>] ? ip6table_mangle_hook+0x3b/0x130
[ 71.219246] [<ffffffff81750ea4>] ? ip6_finish_output2+0x194/0x460
[ 71.219247] [<ffffffff8178d00b>] ? ip6table_mangle_hook+0x3b/0x130
[ 71.219249] [<ffffffff81752436>] ip6_finish_output+0xa6/0x110
[ 71.219250] [<ffffffff81752535>] ip6_output+0x95/0xe0
[ 71.219251] [<ffffffff81752390>] ? ip6_fragment+0x9e0/0x9e0
[ 71.219252] [<ffffffff8174edaf>] dst_output+0xf/0x20
[ 71.219254] [<ffffffff8174f347>] NF_HOOK.constprop.45+0x77/0x90
[ 71.219255] [<ffffffff8174eda0>] ? ac6_proc_exit+0x20/0x20
[ 71.219256] [<ffffffff817505b8>] ip6_xmit+0x268/0x4d0
[ 71.219257] [<ffffffff8174eda0>] ? ac6_proc_exit+0x20/0x20
[ 71.219258] [<ffffffff817505b8>] ? ip6_xmit+0x268/0x4d0
[ 71.219260] [<ffffffff817834b0>] ? inet6_csk_route_socket+0x120/0x1d0
[ 71.219262] [<ffffffff8122f771>] ? __kmalloc_node_track_caller+0x31/0x40
[ 71.219264] [<ffffffff8178363e>] inet6_csk_xmit+0x7e/0xc0
[ 71.219265] [<ffffffff8122f771>] ? __kmalloc_node_track_caller+0x31/0x40
[ 71.219267] [<ffffffff816f62e7>] tcp_transmit_skb+0x547/0x910
[ 71.219268] [<ffffffff816f6a6d>] tcp_write_xmit+0x3bd/0xf70
[ 71.219270] [<ffffffff813d0580>] ? __radix_tree_create+0x2d0/0x360
[ 71.219271] [<ffffffff816f7734>] tcp_push_one+0x34/0x40
[ 71.219272] [<ffffffff816e9141>] tcp_sendmsg+0x511/0xbc0
[ 71.219275] [<ffffffff81716375>] inet_sendmsg+0x65/0xa0
[ 71.219277] [<ffffffff81659188>] sock_sendmsg+0x38/0x50
[ 71.219278] [<ffffffff8165a8d6>] SYSC_sendto+0x106/0x170
[ 71.219281] [<ffffffff81139c06>] ? do_setitimer+0x1f6/0x250
[ 71.219282] [<ffffffff81139ca1>] ? alarm_setitimer+0x41/0x70
[ 71.219283] [<ffffffff8165b55e>] SyS_sendto+0xe/0x10
[ 71.219286] [<ffffffff817b5720>] entry_SYSCALL_64_fastpath+0x13/0x94
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] tcp: sender chronographs instrumentation
From: Eric Dumazet @ 2016-11-28 17:38 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: davem, soheil, francisyyan, netdev, ncardwell, edumazet
In-Reply-To: <1480316838-154141-1-git-send-email-ycheng@google.com>
On Sun, 2016-11-27 at 23:07 -0800, Yuchung Cheng wrote:
> This patch set provides instrumentation on TCP sender limitations.
> While developing the BBR congestion control, we noticed that TCP
> sending process is often limited by factors unrelated to congestion
> control: insufficient sender buffer and/or insufficient receive
> window/buffer to saturate the network bandwidth. Unfortunately these
> limits are not visible to the users and often the poor performance
> is attributed to the congestion control of choice.
For the whole series :
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply
* Re: [[PATCH net-next RFC] 2/4] net: dsa: mv88e6xxx: Monitor and Management tables
From: Vivien Didelot @ 2016-11-28 17:42 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Andrew Lunn
In-Reply-To: <1479944598-20372-3-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> +/* Offset 0x1a: Monitor Control */
Thanks. We could also optionally add another line Offset 0x1A: Monitor &
MGMT Control, to mention the change on newer chips.
> +
> +int mv88e6095_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> + u16 reg;
> +
> + reg = upstream_port << GLOBAL_MONITOR_CONTROL_INGRESS_SHIFT |
> + upstream_port << GLOBAL_MONITOR_CONTROL_EGRESS_SHIFT |
> + upstream_port << GLOBAL_MONITOR_CONTROL_ARP_SHIFT;
> +
> + return mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +}
Monitor Control is just one of these registers which control several
stuffs.
On 6185, it controls only the monitor destination port for ingress,
egress and ARP. On 6352, it controls the monitor destination port for
ingress and egress, sets the CPU destination port and the mirror port.
Hence using GLOBAL_MONITOR_CONTROL_ARP_SHIFT is bad since bits 7:4
differ in those two (CPU Dest vs. ARP Dest). Unless you tell me that
these are the same.
Please add a .set_monitor_port ops and provide a more explicit function
like mv88e6xxx_g1_set_monitor_port() which configures the ingress/egress
monitor destination port and eventually the CPU/ARP dest if the effect
is similar (otherwise a .set_cpu_port ops is needed).
> +
> +int mv88e6390_monitor_ctrl(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> + u16 reg;
> + int err;
> +
> + /* Trap destination */
> + reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> + GLOBAL_MONITOR_CONTROL_CPU_DEST |
> + upstream_port;
> + err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> + if (err)
> + return err;
> +
> + /* 01:c2:80:00:00:00:00-01:c2:80:00:00:00:07 are Management */
> + reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> + GLOBAL_MONITOR_CONTROL_0180C280000000XLO | 0xff;
> + err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> + if (err)
> + return err;
> +
> + /* 01:c2:80:00:00:00:08-01:c2:80:00:00:00:0f are Management */
> + reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> + GLOBAL_MONITOR_CONTROL_0180C280000000XHI | 0xff;
> + err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> + if (err)
> + return err;
> +
> + /* 01:c2:80:00:00:00:20-01:c2:80:00:00:00:27 are Management */
> + reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> + GLOBAL_MONITOR_CONTROL_0180C280000002XLO | 0xff;
> + err = mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> + if (err)
> + return err;
> +
> + /* 01:c2:80:00:00:00:28-01:c2:80:00:00:00:2f are Management */
> + reg = GLOBAL_MONITOR_CONTROL_UPDATE |
> + GLOBAL_MONITOR_CONTROL_0180C280000002XHI | 0xff;
> +
> + return mv88e6xxx_g1_write(chip, GLOBAL_MONITOR_CONTROL, reg);
> +}
> +
This function does more that the 6095 implementation.
First please provide a static helper to write the Monitor & MGMT Control
table. Something like:
mv88e6xxx_g1_monitor_mgmt_write(struct mv88e6xxx_chip *chip,
u8 pointer, u8 data)
Then use it to implement a mv88e6xxx_g1_set_trap_port() function.
Rsvd2CPU is a different thing. 6352 has 2 dedicated registers in Global
2 to configure the management addresses 01:c2:80:00:00:00:2x (offset
0x2) and 01:c2:80:00:00:00:0x (offset 0x3). 6185 only has register 0x3.
Then 6390 stores the Rsvd2CPU addresses in this MGMT table.
I'd expect something like new .set_rsvd2cpu{0,2}(chip, u16 x) ops and
the following implementation:
int mv88e6xxx_g2_set_rsvd2cpu2(struct mv88e6xxx_chip *chip, u16 x);
int mv88e6xxx_g2_set_rsvd2cpu0(struct mv88e6xxx_chip *chip, u16 x);
int global2.c, and:
int mv88e6xxx_g1_set_rsvd2cpu2(struct mv88e6xxx_chip *chip, u16 x);
int mv88e6xxx_g1_set_rsvd2cpu0(struct mv88e6xxx_chip *chip, u16 x);
in global1.c (which use mv88e6xxx_g1_monitor_mgmt_write()).
Later (not required now), a nice wrapper in chip.c could set a rsvd2cpu
bit for a given address, or fallback to loading it in the ATU as MGMT.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] Documentation: net: phy: Add blurb about RGMII
From: Florian Fainelli @ 2016-11-28 17:43 UTC (permalink / raw)
To: Sebastian Frias, netdev
Cc: davem, andrew, martin.blumenstingl, mans, alexandre.torgue,
peppe.cavallaro, timur, jbrunet, mason
In-Reply-To: <4c71a69c-2942-f177-ef25-04686f5e4149@laposte.net>
On 11/28/2016 02:34 AM, Sebastian Frias wrote:
> On 27/11/16 19:44, Florian Fainelli wrote:
>> RGMII is a recurring source of pain for people with Gigabit Ethernet
>> hardware since it may require PHY driver and MAC driver level
>> configuration hints. Document what are the expectations from PHYLIB and
>> what options exist.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> Documentation/networking/phy.txt | 76 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 76 insertions(+)
>>
>> diff --git a/Documentation/networking/phy.txt b/Documentation/networking/phy.txt
>> index 9a42a9414cea..7a0cb1212b9e 100644
>> --- a/Documentation/networking/phy.txt
>> +++ b/Documentation/networking/phy.txt
>> @@ -65,6 +65,82 @@ The MDIO bus
>> drivers/net/ethernet/freescale/fsl_pq_mdio.c and an associated DTS file
>> for one of the users. (e.g. "git grep fsl,.*-mdio arch/powerpc/boot/dts/")
>>
>> +(RG)MII/electrical interface considerations
>> +
>> + The Reduced Gigabit Medium Independent Interface (RGMII) is a 12 pins
>> + electrical signal interface using a synchronous 125Mhz clock signal and several
>> + data lines. Due to this design decision, a 1.5ns to 2ns delay must be added
>> + between the clock line (RXC or TXC) and the data lines to let the PHY (clock
>> + sink) have enough setup and hold times to sample the data lines correctly. The
>> + PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let
>> + the PHY driver and optionaly the MAC driver implement the required delay. The
>> + values of phy_interface_t must be understood from the perspective of the PHY
>> + device itself, leading to the following:
>> +
>> + * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any
>> + internal delay by itself, it assumes that either the Ethernet MAC (if capable
>> + or the PCB traces) insert the correct 1.5-2ns delay
>
> For what is worth, the Atheros at803x driver comes with RX delay enabled as per HW
> reset.
Always, or is this a behavior possibly defined via a stra-pin that can
be changed?
> I understand that enforcing this documentation as is would result in changing the
> driver to disable RX delay, but it could break existing DTs, so I don't know if that
> path will be pursued.
Which is why the documentation update proposed indicates preferred vs.
mandatory.
>
> Would explicit "versioning" the DT nodes be something worth exploring? So far it
> seems the versioning is implicit: driver probably check the presence of some property
> and conclude that it has to behave in a way or another.
I would not go that route, can of worms.
>
>> +
>> + * PHY_INTERFACE_MODE_RGMII_TXID: the PHY should be inserting an internal delay
>> + for the transmit data lines (TXD[3:0]) processed by the PHY device
>> +
>> + * PHY_INTERFACE_MODE_RGMII_RXID: the PHY should be inserting an internal delay
>> + for the receive data lines (RXD[3:0]) processed by the PHY device
>> +
>> + * PHY_INTERFACE_MODE_RGMII_ID: the PHY should be inserting internal delays for
>> + both transmit AND receive data lines from/to the PHY device
>> +
>> + Whenever it is possible, it is preferrable to utilize the PHY side RGMII delay
>> + for several reasons:
>> +
>> + * PHY devices may offer sub-nanosecond granularity in how they allow a
>> + receiver/transmitter side delay (e.g: 0.5, 1.0, 1.5ns) to be specified. Such
>> + precision may be required to account for differences in PCB trace lengths
>> +
>> + * PHY devices are typically qualified for a large range of applications
>> + (industrial, medical, automotive...), and they provide a constant and
>> + reliable delay across temperature/pressure/voltage ranges
>> +
>> + * PHY device drivers in PHYLIB being reusable by nature, being able to
>> + configure correctly a specified delay enables more designs with similar delay
>> + requirements to be operate correctly
>> +
>> + For cases where the PHY is not capable of providing this delay, but the
>> + Ethernet MAC driver is capable of doing it, the correct phy_interface_t value
>> + should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be
>> + configured correctly in order to provide the required transmit and/or receive
>> + side delay from the perspective of the PHY device.
>
> While this clarifies the current situation very well, wouldn't the proposed approach
> require that a property such as "txid-delay-ns" on the PHY's DT node to be duplicated
> for the MAC?
The property name can be identical and represent essentially the same
things, but as as described, if the delay is implemented by the PHY,
then the MAC should disable it, conversely if the MAC needs to implement
it, the PHY should not contain any delay property. If both are found,
because the DTS is miswritten, then, the drivers should ignore/error out.
Or are you thinking about the case you described earlier with delays
that are neither 0 or 2, but e.g: 1 and 3 and you still want to have the
end-result be somewhere around 2ns?
--
Florian
^ permalink raw reply
* Re: [PATCH net] sit: Set skb->protocol properly in ipip6_tunnel_xmit()
From: Alexander Duyck @ 2016-11-28 17:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, sfr, elicooper, Netdev
In-Reply-To: <1480352023.18162.63.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, Nov 28, 2016 at 8:53 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2016-11-28 at 11:47 -0500, David Miller wrote:
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date: Sun, 27 Nov 2016 13:04:00 +1100
>>
>> > [Just for Dave's information]
>> >
>> > On Fri, 25 Nov 2016 13:50:17 +0800 Eli Cooper <elicooper@gmx.com> wrote:
>> >>
>> >> Similar to commit ae148b085876
>> >> ("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()"),
>> >> sit tunnels also need to update skb->protocol; otherwise, TSO/GSO packets
>> >> might not be properly segmented, which causes the packets being dropped.
>> >>
>> >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> >> Tested-by: Eli Cooper <elicooper@gmx.com>
>> >> Cc: stable@vger.kernel.org
>> >> Signed-off-by: Eli Cooper <elicooper@gmx.com>
>> >
>> > I tested this patch and it does *not* solve my problem.
>>
>> I'm torn on this patch, because it looked exactly like it would solve the
>> kind of problem Stephen is running into.
>>
>> Even though it doesn't fix his case, it seems correct to me.
>>
>> I was wondering if it was also important to set the skb->protocol
>> before the call to ip_tunnel_encap() but I couldn't find a dependency.
>>
>> In any event I'd like to see some other people review this change
>> before I apply it.
>>
>> My only other guess for Stephen's problem is somehow the SKB headers
>> aren't set up properly for what the GSO engine expects.
>
> Well, mlx4 just works, and uses GSO engine just fine.
>
> So my guess is this is a bug in Intel IGB driver.
>
>
> Alexander, can you take a look ?
>
> Features for eth0:
> rx-checksumming: on
> tx-checksumming: on
> tx-checksum-ipv4: off [fixed]
> tx-checksum-ip-generic: on
> tx-checksum-ipv6: off [fixed]
> tx-checksum-fcoe-crc: off [fixed]
> tx-checksum-sctp: on
> scatter-gather: on
> tx-scatter-gather: on
> tx-scatter-gather-fraglist: off [fixed]
> tcp-segmentation-offload: on
> tx-tcp-segmentation: on
> tx-tcp-ecn-segmentation: off [fixed]
> tx-tcp-mangleid-segmentation: off
> tx-tcp6-segmentation: on
> udp-fragmentation-offload: off [fixed]
> generic-segmentation-offload: on
> generic-receive-offload: on
> large-receive-offload: off [fixed]
> rx-vlan-offload: on
> tx-vlan-offload: on
> ntuple-filters: off
> receive-hashing: on
> highdma: on [fixed]
> rx-vlan-filter: on [fixed]
> vlan-challenged: off [fixed]
> tx-lockless: off [fixed]
> netns-local: off [fixed]
> tx-gso-robust: off [fixed]
> tx-fcoe-segmentation: off [fixed]
> tx-gre-segmentation: on
> tx-gre-csum-segmentation: on
> tx-ipxip4-segmentation: on
> tx-ipxip6-segmentation: on
> tx-udp_tnl-segmentation: on
> tx-udp_tnl-csum-segmentation: on
> tx-gso-partial: on
> fcoe-mtu: off [fixed]
> tx-nocache-copy: off
> loopback: off [fixed]
> rx-fcs: off [fixed]
> rx-all: off
> tx-vlan-stag-hw-insert: off [fixed]
> rx-vlan-stag-hw-parse: off [fixed]
> rx-vlan-stag-filter: off [fixed]
> l2-fwd-offload: off [fixed]
> busy-poll: off [fixed]
> hw-tc-offload: off [fixed]
I agree. This sounds like a bug with the GSO partial on igb.
I'll try setting up a SIT tunnel on my systems here and see if I can
reproduce the issue.
- Alex
^ permalink raw reply
* Re: net/sctp: vmalloc allocation failure in sctp_setsockopt/xt_alloc_table_info
From: Neil Horman @ 2016-11-28 17:46 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: Andrey Konovalov, Vlad Yasevich, linux-sctp, netdev, LKML,
Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, netfilter-devel, coreteam, Dmitry Vyukov,
Kostya Serebryany, Eric Dumazet, syzkaller
In-Reply-To: <20161128151312.GA13172@localhost.localdomain>
On Mon, Nov 28, 2016 at 01:13:12PM -0200, Marcelo Ricardo Leitner wrote:
> On Mon, Nov 28, 2016 at 09:39:31AM -0500, Neil Horman wrote:
> > On Mon, Nov 28, 2016 at 03:33:40PM +0100, Andrey Konovalov wrote:
> > > On Mon, Nov 28, 2016 at 3:13 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > On Mon, Nov 28, 2016 at 02:00:19PM +0100, Andrey Konovalov wrote:
> > > >> Hi!
> > > >>
> > > >> I've got the following error report while running the syzkaller fuzzer.
> > > >>
> > > >> On commit d8e435f3ab6fea2ea324dce72b51dd7761747523 (Nov 26).
> > > >>
> > > >> A reproducer is attached.
> > > >>
> > > >> a.out: vmalloc: allocation failure, allocated 823562240 of 1427091456
> > > >> bytes, mode:0x24000c2(GFP_KERNEL|__GFP_HIGHMEM)
> > > >>
> > > > How much total ram do you have in this system? The call appears to be
> > > > attempting to allocate 1.3 Gb of data. Even using vmalloc to allow
> > > > discontiguous allocation, thats alot of memory, and if enough is in use already,
> > > > I could make the argument that this might be expected behavior.
> > >
> > > Hi Neail,
> > >
> > > I have 2 Gb.
> > >
> > That would be why. Allocating 65% of the available system memory will almost
> > certainly lead to OOM failures quickly.
> >
> > > Just tested with 4 Gb, everything seems to be working fine.
> > > So I guess this is not actually a bug and allocating 1.3 Gb is OK.
>
> Still we probably should avoid the warn triggered by an userspace
> application: (untested)
>
I'm not sure I agree with that. Generally speaking it seems like the right
thing to do, if you want to avoid filling logs with warnings, but this is the
sort of error that is going to be accompanied by severe service interruption.
I'd rather see a reason behind that in the logs, than just have it occur
silently.
Neil
> --8<--
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index fc4977456c30..b56a0e128fc3 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -958,7 +958,8 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
> if (sz <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
> info = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
> if (!info) {
> - info = vmalloc(sz);
> + info = __vmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_HIGHMEM,
> + PAGE_KERNEL);
> if (!info)
> return NULL;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: net/sctp: vmalloc allocation failure in sctp_setsockopt/xt_alloc_table_info
From: Florian Westphal @ 2016-11-28 17:47 UTC (permalink / raw)
To: Neil Horman
Cc: Marcelo Ricardo Leitner, Andrey Konovalov, Vlad Yasevich,
linux-sctp, netdev, LKML, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, netfilter-devel, coreteam,
Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
In-Reply-To: <20161128174647.GC29839@hmsreliant.think-freely.org>
Neil Horman <nhorman@tuxdriver.com> wrote:
> I'm not sure I agree with that. Generally speaking it seems like the right
> thing to do, if you want to avoid filling logs with warnings, but this is the
> sort of error that is going to be accompanied by severe service interruption.
> I'd rather see a reason behind that in the logs, than just have it occur
> silently.
Its not silent -- the setsockopt call will fail and userspace should
display an error.
So I agree with Marcelo, lets suppress the oom spew here.
^ permalink raw reply
* Re: Large performance regression with 6in4 tunnel (sit)
From: Lance Richardson @ 2016-11-28 17:54 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: Sven-Haegar Koch, Eli Cooper, netdev, Eric Dumazet
In-Reply-To: <20161127142340.3a5c197e@canb.auug.org.au>
> From: "Stephen Rothwell" <sfr@canb.auug.org.au>
> To: "Sven-Haegar Koch" <haegar@sdinet.de>
> Cc: "Eli Cooper" <elicooper@gmx.com>, netdev@vger.kernel.org, "Eric Dumazet" <eric.dumazet@gmail.com>
> Sent: Saturday, November 26, 2016 10:23:40 PM
> Subject: Re: Large performance regression with 6in4 tunnel (sit)
>
> Hi Sven-Haegar,
>
> On Fri, 25 Nov 2016 05:06:53 +0100 (CET) Sven-Haegar Koch <haegar@sdinet.de>
> wrote:
> >
> > Somehow this problem description really reminds me of a report on
> > netdev a bit ago, which the following patch fixed:
> >
> > commit 9ee6c5dc816aa8256257f2cd4008a9291ec7e985
> > Author: Lance Richardson <lrichard@redhat.com>
> > Date: Wed Nov 2 16:36:17 2016 -0400
> >
> > ipv4: allow local fragmentation in ip_finish_output_gso()
> >
> > Some configurations (e.g. geneve interface with default
> > MTU of 1500 over an ethernet interface with 1500 MTU) result
> > in the transmission of packets that exceed the configured MTU.
> > While this should be considered to be a "bad" configuration,
> > it is still allowed and should not result in the sending
> > of packets that exceed the configured MTU.
> >
> > Could this be related?
> >
> > I suppose it would be difficult to test this patch on this machine?
>
> The kernel I am running on is based on 4.7.8, so the above patch
> doesn't come close to applying. Most fo what it is reverting was
> introduced in commit 359ebda25aa0 ("net/ipv4: Introduce IPSKB_FRAG_SEGS
> bit to inet_skb_parm.flags") in v4.8-rc1.
>
> --
> Cheers,
> Stephen Rothwell
>
This should be equivalent for 4.7.x:
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 4bd4921..8a253e2 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -224,8 +224,7 @@ static int ip_finish_output_gso(struct net *net, struct sock *sk,
int ret = 0;
/* common case: locally created skb or seglen is <= mtu */
- if (((IPCB(skb)->flags & IPSKB_FORWARDED) == 0) ||
- skb_gso_network_seglen(skb) <= mtu)
+ if (skb_gso_network_seglen(skb) <= mtu)
return ip_finish_output2(net, sk, skb);
/* Slowpath - GSO segment length is exceeding the dst MTU.
^ permalink raw reply related
* Re: [PATCH net-next v4 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Florian Fainelli @ 2016-11-28 17:54 UTC (permalink / raw)
To: Jerome Brunet, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Carlo Caione, Kevin Hilman, Giuseppe Cavallaro, Alexandre TORGUE,
Martin Blumenstingl, Andre Roth, Andrew Lunn, Neil Armstrong,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Julia Lawall, Yegor Yefremov,
Andreas Färber
In-Reply-To: <1480348229-25672-1-git-send-email-jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On 11/28/2016 07:50 AM, Jerome Brunet wrote:
> This patchset fixes an issue with the OdroidC2 board (DWMAC + RTL8211F).
> The platform seems to enter LPI on the Rx path too often while performing
> relatively high TX transfer. This eventually break the link (both Tx and
> Rx), and require to bring the interface down and up again to get the Rx
> path working again.
>
> The root cause of this issue is not fully understood yet but disabling EEE
> advertisement on the PHY prevent this feature to be negotiated.
> With this change, the link is stable and reliable, with the expected
> throughput performance.
>
> The patchset adds options in the generic phy driver to disable EEE
> advertisement, through device tree. The way it is done is very similar
> to the handling of the max-speed property.
>
> Patch 4 is provided here for testing purpose only. Please don't merge
> patch 4, this change will go through the amlogic's tree.
Sorry, but I really don't like the route this is going, and I should
have made myself clearer before on that, I really think utilizing a PHY
fixup is more appropriate here than an extremely generic DT property.
The fixup code can be in the affected PHY driver, or it can be somewhere
else, your call. There is no shortage of option on how to implement it,
and this would be something easy to enable/disable for known good
configurations (ala PCI/USB fixups).
If we start supporting generic "enable", "disable" type of properties
with values that map directly to register definitions of the HW, we
leave too much room for these properties to be utilized to implement a
specific policy, and this is not acceptable.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH ethtool 2/2] Ethtool: Implements PHY Loopback
From: Florian Fainelli @ 2016-11-28 17:56 UTC (permalink / raw)
To: Allan W. Nielsen, netdev; +Cc: linville, andrew, raju.lakkaraju
In-Reply-To: <1480339512-5882-3-git-send-email-allan.nielsen@microsemi.com>
On 11/28/2016 05:25 AM, Allan W. Nielsen wrote:
> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>
> Add loopback in ethtool tunables to access PHY drivers.
>
> Ethtool Help: ethtool -h for PHY tunables
> ethtool --set-phy-tunable DEVNAME Set PHY tunable
> [ loopback off|near|far|extn ]
> ethtool --get-phy-tunable DEVNAME Get PHY tunable
> [ loopback ]
>
> Ethtool ex:
> ethtool --set-phy-tunable eth0 loopback near
> ethtool --set-phy-tunable eth0 loopback far
> ethtool --set-phy-tunable eth0 loopback extn
> ethtool --set-phy-tunable eth0 loopback off
>
> ethtool --get-phy-tunable eth0 loopback
>
> Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
> ---
> +Near-End Loopback:
> +Transmitted data (TXD) is looped back in the PCS block onto the receive data
> +signal (RXD). When Near-End loopback enable, no data is transmitted over
> +the network. no data receive from the network.
This is also known as the local loopback test mode, right?
> +
> +Far-End Loopback:
> +This loopback is a special test mode to allow testing the PHY from link
> +partner side. In this mode data that is received from the link partner pass
> +through the PHY's receiver, looped back on the MII and transmitted back to
> +the link partner.
And this is the remote loopback mode.
> +
> +External Loopback:
> +An RJ45 loopback cable can be used to route the transmit signals an the
> +output of the trnsformer back to the receiver inputs and this loopback will
> +work at either 10 or 100 or 1000 Mbps speed.
> +RJ45 Loopback cable created by conncting pin 1 to pin 3 and connecting pin
> +2 to pin 6.
OK, this name makes sense to me, but for the two other names, we need to
use a terminology that is clearer to the reader and/or people familiar
and targeted at using this feature (e.g: in a lab or during manufacturing).
--
Florian
^ permalink raw reply
* Re: net/sctp: vmalloc allocation failure in sctp_setsockopt/xt_alloc_table_info
From: Neil Horman @ 2016-11-28 17:56 UTC (permalink / raw)
To: Florian Westphal
Cc: Marcelo Ricardo Leitner, Andrey Konovalov, Vlad Yasevich,
linux-sctp, netdev, LKML, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, netfilter-devel, coreteam,
Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
In-Reply-To: <20161128174710.GE28510@breakpoint.cc>
On Mon, Nov 28, 2016 at 06:47:10PM +0100, Florian Westphal wrote:
> Neil Horman <nhorman@tuxdriver.com> wrote:
> > I'm not sure I agree with that. Generally speaking it seems like the right
> > thing to do, if you want to avoid filling logs with warnings, but this is the
> > sort of error that is going to be accompanied by severe service interruption.
> > I'd rather see a reason behind that in the logs, than just have it occur
> > silently.
>
> Its not silent -- the setsockopt call will fail and userspace should
> display an error.
>
Thats not true. If the OOM succedes in freeing enough memory to fulfill the
request the setsockopt may complete without error, you're just left with a
killed process...somewhere. Thats seems a bit dodgy to me
Not saying it has to be a full stack trace, but some log annotation that shows
the oom killer got invoked seems called for here
Neil
> So I agree with Marcelo, lets suppress the oom spew here.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] ethtool: (uapi) Add ETHTOOL_PHY_LOOPBACK to PHY tunables
From: Florian Fainelli @ 2016-11-28 17:59 UTC (permalink / raw)
To: Andrew Lunn, Allan W. Nielsen; +Cc: netdev, raju.lakkaraju
In-Reply-To: <20161128141410.GF4379@lunn.ch>
On 11/28/2016 06:14 AM, Andrew Lunn wrote:
> On Mon, Nov 28, 2016 at 02:24:30PM +0100, Allan W. Nielsen wrote:
>> From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
>>
>> 3 types of PHY loopback are supported.
>> i.e. Near-End Loopback, Far-End Loopback and External Loopback.
>
> Hi Allan
>
> Is this integrated with ethtool --test? You only want the PHY to go
> into loopback mode when running ethtool --test external_lb or maybe
> ethtool --test offline.
>
> What i think should happen is that this tunable sets the mode the PHY
> will go into when loopback is enabled. It does not however enable
> loopback. It is running ethtool --test which actually enables the
> loopback, probably by setting BMCR_LOOPBACK. Once the test is
> finished, the bit is cleared and the PHY goes back into normal
> operation.
Agreed with Andrew, there needs to be a tight coupling between the
existing ethtool test infrastructure, and how to perform PHY loopback
testing.
It makes sense for the PHY drivers to be able to implement specific
loopback/test mode, but we need to clarify how these get called from
ethtool --test for instance, and if the Ethernet MAC driver needs to do
something specific as well.
--
Florian
^ permalink raw reply
* Re: [PATCH net] sit: Set skb->protocol properly in ipip6_tunnel_xmit()
From: David Miller @ 2016-11-28 18:01 UTC (permalink / raw)
To: eric.dumazet; +Cc: alexander.duyck, sfr, elicooper, netdev
In-Reply-To: <1480354467.18162.66.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 28 Nov 2016 09:34:27 -0800
> On Mon, 2016-11-28 at 08:53 -0800, Eric Dumazet wrote:
>> On Mon, 2016-11-28 at 11:47 -0500, David Miller wrote:
>> > From: Stephen Rothwell <sfr@canb.auug.org.au>
>> > Date: Sun, 27 Nov 2016 13:04:00 +1100
>> >
>> > > [Just for Dave's information]
>> > >
>> > > On Fri, 25 Nov 2016 13:50:17 +0800 Eli Cooper <elicooper@gmx.com> wrote:
>> > >>
>> > >> Similar to commit ae148b085876
>> > >> ("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()"),
>> > >> sit tunnels also need to update skb->protocol; otherwise, TSO/GSO packets
>> > >> might not be properly segmented, which causes the packets being dropped.
>> > >>
>> > >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> > >> Tested-by: Eli Cooper <elicooper@gmx.com>
>> > >> Cc: stable@vger.kernel.org
>> > >> Signed-off-by: Eli Cooper <elicooper@gmx.com>
>> > >
>> > > I tested this patch and it does *not* solve my problem.
>> >
>> > I'm torn on this patch, because it looked exactly like it would solve the
>> > kind of problem Stephen is running into.
>> >
>> > Even though it doesn't fix his case, it seems correct to me.
>> >
>> > I was wondering if it was also important to set the skb->protocol
>> > before the call to ip_tunnel_encap() but I couldn't find a dependency.
>> >
>> > In any event I'd like to see some other people review this change
>> > before I apply it.
>> >
>> > My only other guess for Stephen's problem is somehow the SKB headers
>> > aren't set up properly for what the GSO engine expects.
>>
>> Well, mlx4 just works, and uses GSO engine just fine.
>>
>> So my guess is this is a bug in Intel IGB driver.
>
> About Eli patch : I do not believe it is needed.
>
> Here is the path followed by SIT packet being GSO at the device layer :
>
> We can see that ip_output() was called, and ip_output() already does :
>
> skb->protocol = htons(ETH_P_IP);
Hmmm, ip6_finish_output2() also does a proper assignment of skb->protocol,
therefore why was commit ae148b085876fa771d9ef2c05f85d4b4bf09ce0d
("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()")
necessary at all?
^ permalink raw reply
* Re: net/sctp: vmalloc allocation failure in sctp_setsockopt/xt_alloc_table_info
From: Florian Westphal @ 2016-11-28 18:09 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Westphal, Marcelo Ricardo Leitner, netdev, LKML,
netfilter-devel
In-Reply-To: <20161128175626.GD29839@hmsreliant.think-freely.org>
Neil Horman <nhorman@tuxdriver.com> wrote:
[ trimming CCs ]
> On Mon, Nov 28, 2016 at 06:47:10PM +0100, Florian Westphal wrote:
> > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > I'm not sure I agree with that. Generally speaking it seems like the right
> > > thing to do, if you want to avoid filling logs with warnings, but this is the
> > > sort of error that is going to be accompanied by severe service interruption.
> > > I'd rather see a reason behind that in the logs, than just have it occur
> > > silently.
> >
> > Its not silent -- the setsockopt call will fail and userspace should
> > display an error.
> >
> Thats not true. If the OOM succedes in freeing enough memory to fulfill the
> request the setsockopt may complete without error, you're just left with a
> killed process...somewhere. Thats seems a bit dodgy to me
We should prevent OOM killer from running in first place (GFP_NORETRY should work).
^ permalink raw reply
* Re: net/sctp: vmalloc allocation failure in sctp_setsockopt/xt_alloc_table_info
From: Marcelo Ricardo Leitner @ 2016-11-28 18:18 UTC (permalink / raw)
To: Florian Westphal; +Cc: Neil Horman, netdev, LKML, netfilter-devel
In-Reply-To: <20161128180925.GF28510@breakpoint.cc>
On Mon, Nov 28, 2016 at 07:09:25PM +0100, Florian Westphal wrote:
> Neil Horman <nhorman@tuxdriver.com> wrote:
>
> [ trimming CCs ]
>
> > On Mon, Nov 28, 2016 at 06:47:10PM +0100, Florian Westphal wrote:
> > > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > I'm not sure I agree with that. Generally speaking it seems like the right
> > > > thing to do, if you want to avoid filling logs with warnings, but this is the
> > > > sort of error that is going to be accompanied by severe service interruption.
> > > > I'd rather see a reason behind that in the logs, than just have it occur
> > > > silently.
> > >
> > > Its not silent -- the setsockopt call will fail and userspace should
> > > display an error.
> > >
> > Thats not true. If the OOM succedes in freeing enough memory to fulfill the
> > request the setsockopt may complete without error, you're just left with a
> > killed process...somewhere. Thats seems a bit dodgy to me
>
__GFP_NOWARN is about allocation failures only and it won't disable OOM
kill messages. oom_kill_process() has no idea on GFP_NOWARN when doing
the logging.
> We should prevent OOM killer from running in first place (GFP_NORETRY should work).
Oh. Really?
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: enable multiqueue by default
From: David Miller @ 2016-11-28 18:18 UTC (permalink / raw)
To: jasowang
Cc: nhorman, jeder, hannes, netdev, mst, linux-kernel, virtualization,
myllynen, maxime.coquelin
In-Reply-To: <1480048646-17536-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 25 Nov 2016 12:37:26 +0800
> We use single queue even if multiqueue is enabled and let admin to
> enable it through ethtool later. This is used to avoid possible
> regression (small packet TCP stream transmission). But looks like an
> overkill since:
>
> - single queue user can disable multiqueue when launching qemu
> - brings extra troubles for the management since it needs extra admin
> tool in guest to enable multiqueue
> - multiqueue performs much better than single queue in most of the
> cases
>
> So this patch enables multiqueue by default: if #queues is less than or
> equal to #vcpu, enable as much as queue pairs; if #queues is greater
> than #vcpu, enable #vcpu queue pairs.
>
> Cc: Hannes Frederic Sowa <hannes@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Neil Horman <nhorman@redhat.com>
> Cc: Jeremy Eder <jeder@redhat.com>
> Cc: Marko Myllynen <myllynen@redhat.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied, thanks Jason.
^ permalink raw reply
* Re: [PATCH net] sit: Set skb->protocol properly in ipip6_tunnel_xmit()
From: Eric Dumazet @ 2016-11-28 18:17 UTC (permalink / raw)
To: David Miller; +Cc: alexander.duyck, sfr, elicooper, netdev
In-Reply-To: <20161128.130142.1225482479870737422.davem@davemloft.net>
On Mon, 2016-11-28 at 13:01 -0500, David Miller wrote:
> Hmmm, ip6_finish_output2() also does a proper assignment of skb->protocol,
> therefore why was commit ae148b085876fa771d9ef2c05f85d4b4bf09ce0d
> ("ip6_tunnel: Update skb->protocol to ETH_P_IPV6 in ip6_tnl_xmit()")
> necessary at all?
I was referring to Stephen bug report.
It appears that Eli changelog was not very precise, because his bug was
because of XFRM being involved.
xfrm_output() -> xfrm_output_gso() -> skb_gso_segment()
So XFRM calls skb_gso_segment() before ip_output() or
ip6_finish_output2() had a chance to change skb->protocol
^ permalink raw reply
* [PATCH net 00/16] net: fix fixed-link phydev leaks
From: Johan Hovold @ 2016-11-28 18:24 UTC (permalink / raw)
To: David S. Miller
Cc: Vince Bridgers, Florian Fainelli, Fugang Duan, Pantelis Antoniou,
Vitaly Bordug, Claudiu Manoil, Li Yang, Thomas Petazzoni,
Felix Fietkau, John Crispin, Matthias Brugger, Sergei Shtylyov,
Lars Persson, Mugunthan V N, Grygorii Strashko, Rob Herring,
Frank Rowand, Andrew Lunn, Vivien Didelot <
This series fixes failures to deregister and free fixed-link phydevs
that have been registered using the of_phy_register_fixed_link()
interface.
All but two drivers currently fail to do this and this series fixes most
of them with the exception of a staging driver and the stmmac drivers
which will be fixed by follow-on patches.
Included are also a couple of fixes for related of-node leaks.
Note that all patches except the of_mdio one have been compile-tested
only.
Also note that the series is against net due to dependencies not yet in
net-next.
Johan
Johan Hovold (16):
net: dsa: slave: fix of-node leak and phy priority
of_mdio: add helper to deregister fixed-link PHYs
net: ethernet: altera: fix fixed-link phydev leaks
net: ethernet: aurora: nb8800: fix fixed-link phydev leaks
net: ethernet: bcmsysport: fix fixed-link phydev leaks
net: ethernet: bcmgenet: fix fixed-link phydev leaks
net: ethernet: fec: fix fixed-link phydev leaks
net: ethernet: fs_enet: fix fixed-link phydev leaks
net: ethernet: gianfar: fix fixed-link phydev leaks
net: ethernet: ucc_geth: fix fixed-link phydev leaks
net: ethernet: marvell: mvneta: fix fixed-link phydev leaks
net: ethernet: mediatek: fix fixed-link phydev leaks
net: ethernet: renesas: ravb: fix fixed-link phydev leaks
net: ethernet: dwc_eth_qos: fix fixed-link phydev leaks
net: ethernet: ti: davinci_emac: fix fixed-link phydev and of-node
leaks
net: dsa: slave: fix fixed-link phydev leaks
drivers/net/ethernet/altera/altera_tse_main.c | 9 ++++++++-
drivers/net/ethernet/aurora/nb8800.c | 9 +++++++--
drivers/net/ethernet/broadcom/bcmsysport.c | 17 +++++++++++-----
drivers/net/ethernet/broadcom/genet/bcmmii.c | 6 ++++++
drivers/net/ethernet/freescale/fec_main.c | 5 +++++
.../net/ethernet/freescale/fs_enet/fs_enet-main.c | 7 ++++++-
drivers/net/ethernet/freescale/gianfar.c | 8 ++++++++
drivers/net/ethernet/freescale/ucc_geth.c | 23 +++++++++++++++-------
drivers/net/ethernet/marvell/mvneta.c | 5 +++++
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 ++++
drivers/net/ethernet/renesas/ravb_main.c | 17 +++++++++++++---
drivers/net/ethernet/synopsys/dwc_eth_qos.c | 20 ++++++++++++-------
drivers/net/ethernet/ti/cpsw.c | 16 ++-------------
drivers/net/ethernet/ti/davinci_emac.c | 10 +++++++++-
drivers/of/of_mdio.c | 15 ++++++++++++++
include/linux/of_mdio.h | 4 ++++
net/dsa/dsa.c | 12 ++---------
net/dsa/slave.c | 19 +++++++++++++++---
18 files changed, 152 insertions(+), 54 deletions(-)
--
2.7.3
^ permalink raw reply
* [PATCH net 01/16] net: dsa: slave: fix of-node leak and phy priority
From: Johan Hovold @ 2016-11-28 18:24 UTC (permalink / raw)
To: David S. Miller
Cc: Vince Bridgers, Florian Fainelli, Fugang Duan, Pantelis Antoniou,
Vitaly Bordug, Claudiu Manoil, Li Yang, Thomas Petazzoni,
Felix Fietkau, John Crispin, Matthias Brugger, Sergei Shtylyov,
Lars Persson, Mugunthan V N, Grygorii Strashko, Rob Herring,
Frank Rowand, Andrew Lunn, Vivien Didelot <
In-Reply-To: <1480357509-28074-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Make sure to drop the reference taken by of_parse_phandle() before
returning from dsa_slave_phy_setup().
Note that this also modifies the PHY priority so that any fixed-link
node is only parsed when no phy-handle is given, which is in accordance
with the common scheme for this.
Fixes: 0d8bcdd383b8 ("net: dsa: allow for more complex PHY setups")
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
net/dsa/slave.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 6b1282c006b1..2a5c20a13fe4 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1125,7 +1125,7 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
p->phy_interface = mode;
phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);
- if (of_phy_is_fixed_link(port_dn)) {
+ if (!phy_dn && of_phy_is_fixed_link(port_dn)) {
/* In the case of a fixed PHY, the DT node associated
* to the fixed PHY is the Port DT node
*/
@@ -1135,7 +1135,7 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
return ret;
}
phy_is_fixed = true;
- phy_dn = port_dn;
+ phy_dn = of_node_get(port_dn);
}
if (ds->ops->get_phy_flags)
@@ -1154,6 +1154,7 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
ret = dsa_slave_phy_connect(p, slave_dev, phy_id);
if (ret) {
netdev_err(slave_dev, "failed to connect to phy%d: %d\n", phy_id, ret);
+ of_node_put(phy_dn);
return ret;
}
} else {
@@ -1162,6 +1163,8 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
phy_flags,
p->phy_interface);
}
+
+ of_node_put(phy_dn);
}
if (p->phy && phy_is_fixed)
--
2.7.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 net 02/16] of_mdio: add helper to deregister fixed-link PHYs
From: Johan Hovold @ 2016-11-28 18:24 UTC (permalink / raw)
To: David S. Miller
Cc: Vince Bridgers, Florian Fainelli, Fugang Duan, Pantelis Antoniou,
Vitaly Bordug, Claudiu Manoil, Li Yang, Thomas Petazzoni,
Felix Fietkau, John Crispin, Matthias Brugger, Sergei Shtylyov,
Lars Persson, Mugunthan V N, Grygorii Strashko, Rob Herring,
Frank Rowand, Andrew Lunn, Vivien Didelot <
In-Reply-To: <1480357509-28074-1-git-send-email-johan@kernel.org>
Add helper to deregister fixed-link PHYs registered using
of_phy_register_fixed_link().
Convert the two drivers that care to deregister their fixed-link PHYs to
use the new helper, but note that most drivers currently fail to do so.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/net/ethernet/ti/cpsw.c | 16 ++--------------
drivers/of/of_mdio.c | 15 +++++++++++++++
include/linux/of_mdio.h | 4 ++++
net/dsa/dsa.c | 12 ++----------
4 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 58947aae31c7..9f0646512624 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2459,20 +2459,8 @@ static void cpsw_remove_dt(struct platform_device *pdev)
if (strcmp(slave_node->name, "slave"))
continue;
- if (of_phy_is_fixed_link(slave_node)) {
- struct phy_device *phydev;
-
- phydev = of_phy_find_device(slave_node);
- if (phydev) {
- fixed_phy_unregister(phydev);
- /* Put references taken by
- * of_phy_find_device() and
- * of_phy_register_fixed_link().
- */
- phy_device_free(phydev);
- phy_device_free(phydev);
- }
- }
+ if (of_phy_is_fixed_link(slave_node))
+ of_phy_deregister_fixed_link(slave_node);
of_node_put(slave_data->phy_node);
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 5a3145a02547..262281bd68fa 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -490,3 +490,18 @@ int of_phy_register_fixed_link(struct device_node *np)
return -ENODEV;
}
EXPORT_SYMBOL(of_phy_register_fixed_link);
+
+void of_phy_deregister_fixed_link(struct device_node *np)
+{
+ struct phy_device *phydev;
+
+ phydev = of_phy_find_device(np);
+ if (!phydev)
+ return;
+
+ fixed_phy_unregister(phydev);
+
+ put_device(&phydev->mdio.dev); /* of_phy_find_device() */
+ phy_device_free(phydev); /* fixed_phy_register() */
+}
+EXPORT_SYMBOL(of_phy_deregister_fixed_link);
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 2ab233661ae5..a58cca8bcb29 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -29,6 +29,7 @@ struct phy_device *of_phy_attach(struct net_device *dev,
extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
extern int of_mdio_parse_addr(struct device *dev, const struct device_node *np);
extern int of_phy_register_fixed_link(struct device_node *np);
+extern void of_phy_deregister_fixed_link(struct device_node *np);
extern bool of_phy_is_fixed_link(struct device_node *np);
#else /* CONFIG_OF */
@@ -83,6 +84,9 @@ static inline int of_phy_register_fixed_link(struct device_node *np)
{
return -ENOSYS;
}
+static inline void of_phy_deregister_fixed_link(struct device_node *np)
+{
+}
static inline bool of_phy_is_fixed_link(struct device_node *np)
{
return false;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index cb0091b99592..7899919cd9f0 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -506,16 +506,8 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index,
void dsa_cpu_dsa_destroy(struct device_node *port_dn)
{
- struct phy_device *phydev;
-
- if (of_phy_is_fixed_link(port_dn)) {
- phydev = of_phy_find_device(port_dn);
- if (phydev) {
- fixed_phy_unregister(phydev);
- put_device(&phydev->mdio.dev);
- phy_device_free(phydev);
- }
- }
+ if (of_phy_is_fixed_link(port_dn))
+ of_phy_deregister_fixed_link(port_dn);
}
static void dsa_switch_destroy(struct dsa_switch *ds)
--
2.7.3
^ permalink raw reply related
* [PATCH net 03/16] net: ethernet: altera: fix fixed-link phydev leaks
From: Johan Hovold @ 2016-11-28 18:24 UTC (permalink / raw)
To: David S. Miller
Cc: Vince Bridgers, Florian Fainelli, Fugang Duan, Pantelis Antoniou,
Vitaly Bordug, Claudiu Manoil, Li Yang, Thomas Petazzoni,
Felix Fietkau, John Crispin, Matthias Brugger, Sergei Shtylyov,
Lars Persson, Mugunthan V N, Grygorii Strashko, Rob Herring,
Frank Rowand, Andrew Lunn, Vivien Didelot <
In-Reply-To: <1480357509-28074-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Make sure to deregister and free any fixed-link PHY registered using
of_phy_register_fixed_link() on probe errors and on driver unbind.
Fixes: 7cdbc6f74f8e ("altera tse: add support for fixed-links.")
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/net/ethernet/altera/altera_tse_main.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index bda31f308cc2..6532829b70d2 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -819,6 +819,8 @@ static int init_phy(struct net_device *dev)
if (!phydev) {
netdev_err(dev, "Could not find the PHY\n");
+ if (fixed_link)
+ of_phy_deregister_fixed_link(priv->device->of_node);
return -ENODEV;
}
@@ -1545,10 +1547,15 @@ static int altera_tse_probe(struct platform_device *pdev)
static int altera_tse_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
+ struct altera_tse_private *priv = netdev_priv(ndev);
- if (ndev->phydev)
+ if (ndev->phydev) {
phy_disconnect(ndev->phydev);
+ if (of_phy_is_fixed_link(priv->device->of_node))
+ of_phy_deregister_fixed_link(priv->device->of_node);
+ }
+
platform_set_drvdata(pdev, NULL);
altera_tse_mdio_destroy(ndev);
unregister_netdev(ndev);
--
2.7.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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 net 05/16] net: ethernet: bcmsysport: fix fixed-link phydev leaks
From: Johan Hovold @ 2016-11-28 18:24 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, devicetree-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
Frank Rowand, Felix Fietkau, Florian Fainelli, Claudiu Manoil,
Li Yang, Mugunthan V N, Grygorii Strashko,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Johan Hovold, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lars Persson,
Matthias Brugger, linux-omap-u79uwXL29TY76Z2rM5mHXA, John Crispin,
Thomas Petazzoni, Fugang Duan, Sergei Shtylyov, Vivien Didelot,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480357509-28074-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Make sure to deregister and free any fixed-link PHY registered using
of_phy_register_fixed_link() on probe errors and on driver unbind.
Fixes: 186534a3f832 ("net: systemport: use the new fixed PHY helpers")
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index c3354b9941d1..25d1eb4933d0 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1755,13 +1755,13 @@ static int bcm_sysport_probe(struct platform_device *pdev)
if (priv->irq0 <= 0 || priv->irq1 <= 0) {
dev_err(&pdev->dev, "invalid interrupts\n");
ret = -EINVAL;
- goto err;
+ goto err_free_netdev;
}
priv->base = devm_ioremap_resource(&pdev->dev, r);
if (IS_ERR(priv->base)) {
ret = PTR_ERR(priv->base);
- goto err;
+ goto err_free_netdev;
}
priv->netdev = dev;
@@ -1779,7 +1779,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
ret = of_phy_register_fixed_link(dn);
if (ret) {
dev_err(&pdev->dev, "failed to register fixed PHY\n");
- goto err;
+ goto err_free_netdev;
}
priv->phy_dn = dn;
@@ -1821,7 +1821,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
ret = register_netdev(dev);
if (ret) {
dev_err(&pdev->dev, "failed to register net_device\n");
- goto err;
+ goto err_deregister_fixed_link;
}
priv->rev = topctrl_readl(priv, REV_CNTL) & REV_MASK;
@@ -1832,7 +1832,11 @@ static int bcm_sysport_probe(struct platform_device *pdev)
priv->base, priv->irq0, priv->irq1, txq, rxq);
return 0;
-err:
+
+err_deregister_fixed_link:
+ if (of_phy_is_fixed_link(dn))
+ of_phy_deregister_fixed_link(dn);
+err_free_netdev:
free_netdev(dev);
return ret;
}
@@ -1840,11 +1844,14 @@ static int bcm_sysport_probe(struct platform_device *pdev)
static int bcm_sysport_remove(struct platform_device *pdev)
{
struct net_device *dev = dev_get_drvdata(&pdev->dev);
+ struct device_node *dn = pdev->dev.of_node;
/* Not much to do, ndo_close has been called
* and we use managed allocations
*/
unregister_netdev(dev);
+ if (of_phy_is_fixed_link(dn))
+ of_phy_deregister_fixed_link(dn);
free_netdev(dev);
dev_set_drvdata(&pdev->dev, NULL);
--
2.7.3
^ permalink raw reply related
* [PATCH net 06/16] net: ethernet: bcmgenet: fix fixed-link phydev leaks
From: Johan Hovold @ 2016-11-28 18:24 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, devicetree-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
Frank Rowand, Felix Fietkau, Florian Fainelli, Claudiu Manoil,
Li Yang, Mugunthan V N, Grygorii Strashko,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Johan Hovold, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lars Persson,
Matthias Brugger, linux-omap-u79uwXL29TY76Z2rM5mHXA, John Crispin,
Thomas Petazzoni, Fugang Duan, Sergei Shtylyov, Vivien Didelot,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480357509-28074-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Make sure to deregister and free any fixed-link PHY registered using
of_phy_register_fixed_link() on probe errors and on driver unbind.
Note that we're still leaking any fixed-link PHY registered in the
non-OF probe path.
Fixes: 9abf0c2b717a ("net: bcmgenet: use the new fixed PHY helpers")
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/net/ethernet/broadcom/genet/bcmmii.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 2e745bd51df4..e87607621e62 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -627,6 +627,7 @@ static int bcmgenet_mii_bus_init(struct bcmgenet_priv *priv)
int bcmgenet_mii_init(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
+ struct device_node *dn = priv->pdev->dev.of_node;
int ret;
ret = bcmgenet_mii_alloc(priv);
@@ -640,6 +641,8 @@ int bcmgenet_mii_init(struct net_device *dev)
return 0;
out:
+ if (of_phy_is_fixed_link(dn))
+ of_phy_deregister_fixed_link(dn);
of_node_put(priv->phy_dn);
mdiobus_unregister(priv->mii_bus);
mdiobus_free(priv->mii_bus);
@@ -649,7 +652,10 @@ int bcmgenet_mii_init(struct net_device *dev)
void bcmgenet_mii_exit(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
+ struct device_node *dn = priv->pdev->dev.of_node;
+ if (of_phy_is_fixed_link(dn))
+ of_phy_deregister_fixed_link(dn);
of_node_put(priv->phy_dn);
mdiobus_unregister(priv->mii_bus);
mdiobus_free(priv->mii_bus);
--
2.7.3
^ permalink raw reply related
* [PATCH net 07/16] net: ethernet: fec: fix fixed-link phydev leaks
From: Johan Hovold @ 2016-11-28 18:25 UTC (permalink / raw)
To: David S. Miller
Cc: Andrew Lunn, devicetree-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
Frank Rowand, Felix Fietkau, Florian Fainelli, Claudiu Manoil,
Li Yang, Mugunthan V N, Grygorii Strashko,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, Johan Hovold, Rob Herring,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lars Persson,
Matthias Brugger, linux-omap-u79uwXL29TY76Z2rM5mHXA, John Crispin,
Thomas Petazzoni, Fugang Duan, Sergei Shtylyov, Vivien Didelot,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480357509-28074-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Make sure to deregister and free any fixed-link PHY registered using
of_phy_register_fixed_link() on probe errors and on driver unbind.
Fixes: 407066f8f371 ("net: fec: Support phys probed from devicetree and
fixed-link")
Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/net/ethernet/freescale/fec_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5aa9d4ded214..74dcdf097348 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3475,6 +3475,8 @@ fec_probe(struct platform_device *pdev)
failed_clk_ipg:
fec_enet_clk_enable(ndev, false);
failed_clk:
+ if (of_phy_is_fixed_link(np))
+ of_phy_deregister_fixed_link(np);
failed_phy:
of_node_put(phy_node);
failed_ioremap:
@@ -3488,6 +3490,7 @@ fec_drv_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
+ struct device_node *np = pdev->dev.of_node;
cancel_work_sync(&fep->tx_timeout_work);
fec_ptp_stop(pdev);
@@ -3495,6 +3498,8 @@ fec_drv_remove(struct platform_device *pdev)
fec_enet_mii_remove(fep);
if (fep->reg_phy)
regulator_disable(fep->reg_phy);
+ if (of_phy_is_fixed_link(np))
+ of_phy_deregister_fixed_link(np);
of_node_put(fep->phy_node);
free_netdev(ndev);
--
2.7.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox