* Re: [PATCH 0/8] net: mvpp2: add TX interrupts support
From: David Miller @ 2017-07-26 20:35 UTC (permalink / raw)
To: thomas.petazzoni
Cc: netdev, linux, antoine.tenart, miquel.raynal, linux-arm-kernel,
jason, andrew, sebastian.hesselbarth, gregory.clement, nadavh,
hannah, yehuday, stefanc, mw
In-Reply-To: <20170725155509.10574-1-thomas.petazzoni@free-electrons.com>
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Tue, 25 Jul 2017 17:55:01 +0200
> Hello,
>
> So far, the mvpp2 driver was using an hrtimer to handle TX
> completion. This patch series adds support for using TX interrupts
> (for each CPU) on PPv2.2, the variant of the IP used on Marvell Armada
> 7K/8K.
>
> This series has been tested on Marvell Armada 7K (PPv2.2) and Armada
> 375 (PPv2.1).
>
> Dave:
>
> - This series depends on the previous series sent by Antoine Ténart
> "net: mvpp2: MAC/GoP configuration and optional PHYs". Functionally
> speaking there is no real dependency, but we touch in a few areas
> the same piece of code, so I based my patch series on top of
> Antoine's.
>
> - Please do not apply the last patch of this series "arm64: dts:
> marvell: add TX interrupts for PPv2.2", it will be taken by the ARM
> mvebu maintainers.
Please don't do things this way.
Patiently wait for Antione's series to make it into my tree, then
submit your's.
Also, if we're continually going to elide the DTS file patches, just
don't bother adding them to the series. That way you don't have to
give me special instructions, and I don't have the possibility of
making a mistake and applying it accidently.
Thanks.
^ permalink raw reply
* Re: [RFC] switchdev: clarify ndo_get_phys_port_name() formats
From: Jakub Kicinski @ 2017-07-26 20:11 UTC (permalink / raw)
To: Or Gerlitz
Cc: Jiri Pirko, Linux Netdev List, Or Gerlitz, Michael Chan,
Sathya Perla, Simon Horman, David Miller
In-Reply-To: <CAJ3xEMg6jn-x5+bwh--P2nz26T91R0Af64hnc30p-k+_BjcuPQ@mail.gmail.com>
On Wed, 26 Jul 2017 12:23:17 +0300, Or Gerlitz wrote:
> On Wed, Jul 26, 2017 at 11:13 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > On Wed, 26 Jul 2017 07:48:40 +0200, Jiri Pirko wrote:
>
> >> I think it would make sense if the driver would just fill-up a struct in
> >> the ndo call and core would generate the string.
>
> > I do like the idea of core generating the string. I have a temptation
> > to try to involve devlink in this somehow, since port id and split info
> > is already available there. Perhaps that would only overcomplicate
> > things.
>
> > The other question is: can we remove the option to generate an arbitrary
> > string completely? I think Or and mlx5 folks may object since it would
> > mean mlx5 VF repr names would change.
>
> What we have today is the representor driver instance setting the VF index as
> the of phys port name and we're telling users to have this sort of udev rule:
>
> SUBSYSTEM=="net", ACTION=="add",
> ATTR{phys_switch_id}=="<phys_switch_id>", \ ATTR{phys_port_name}!="",
> NAME="$PF_NIC$attr{phys_port_name}"
Example names generated by this rule would be pfnic_0, pfnic_1 for vf
representors 0 and 1?
> that has affiliation to the PF where this VF belongs. AFAIK this
> model/assumption is now under push to
> higher level (open stack), so lets see if/what we want to change here
> w.r.t to sriov offloading drivers.
>
> I would opt for an approach where the value returned by the kernel is
> the minimal possible and
> user-space has flexibility to further orchestrate that with udev
> rules. I wasn't fully clear on Jakub's suggestion
> which parts must come from the kernel. Do we have any length
> limitation for the phys port name?
Looks like the limit today is IFNAMSIZ. I'm in favor of leaving the
flexibility to the userspace, why I suggested adding the pf%d or
pf%dvf%d to the name is that I don't think we have any other way today
of telling whether representor is for a physical port, VF or PF.
If I understand mlx5 code, you're not reporting port ids for physical
ports so presence of the name already implies it's a VF but in case you
want to add port splitting support, for example, reporting the name on
physical ports will become more of a necessity.
If we adopt Jiri's suggestion of returning structured data it will be
very easy to give user space type and indexes separately, but we should
probably still return the string for backwards compatibility.
^ permalink raw reply
* Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface
From: David Miller @ 2017-07-26 20:07 UTC (permalink / raw)
To: andrew
Cc: vivien.didelot, privat, corbet, f.fainelli, kernel, linux-doc,
linux-kernel, netdev
In-Reply-To: <20170726175224.GT12049@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 26 Jul 2017 19:52:24 +0200
>> > So I really want to group the patches into only a few series in order
>> > to not spend months on the process.
>
> I strongly agree with Vivien here. Good patches get accepted in about
> 3 days. You should expect feedback within a day or two. That allows
> you to have fast cycle times for getting patches in.
+1
Small simple patches will get everything in 10 times fast than if
you clump everything together into larger, harder to review ones.
^ permalink raw reply
* Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate
From: Kurt Van Dijck @ 2017-07-26 20:04 UTC (permalink / raw)
To: Franklin S Cooper Jr
Cc: Oliver Hartkopp, linux-kernel, devicetree, netdev, linux-can, wg,
mkl, robh+dt, quentin.schulz, andrew
In-Reply-To: <055c00f5-0775-4e35-309c-0d1c8aa0afdf@ti.com>
Hi,
I know my response is late ...
> Hi Oliver
> On 07/20/2017 02:43 AM, Oliver Hartkopp wrote:
> > Hi Franklin,
> >
> > On 07/20/2017 01:36 AM, Franklin S Cooper Jr wrote:
> >
> >> +#ifdef CONFIG_OF
> >> +void of_transceiver_is_fixed(struct net_device *dev)
> >> +{
> >
> > (..)
> >
> >> +}
> >> +EXPORT_SYMBOL(of_transceiver_is_fixed);
> >> +#endif
> >
> > I'm not sure about the naming here.
> >
> > As this is a CAN transceiver related option it should be named accordingly:
I contest the the name too:
1) the can transceiver isn't fixed at all, it limited to the higher
bitrates.
2) of_can_transceiver_is_fixed suggests to test if a transceiver is
fixed, it does not suggest to load some properties from the device tree.
of_can_load_transceiver looks way more clear to me.
That's my opinion.
The important things, like the contents of the functions, look good.
Kind regards,
Kurt Van Dijck
^ permalink raw reply
* Re: [PATCH net] net: phy: Run state machine to completion
From: Florian Fainelli @ 2017-07-26 19:53 UTC (permalink / raw)
To: netdev
Cc: rmk+kernel, slash.tmp, zach.brown, nathan.sullivan, Andrew Lunn,
open list
In-Reply-To: <8413cb0c-a304-7698-5cbe-94a420b74cb7@gmail.com>
On 07/26/2017 12:34 PM, Florian Fainelli wrote:
> On 07/26/2017 12:24 PM, Florian Fainelli wrote:
>> Marc reported that he was not getting the PHY library adjust_link()
>> callback function to run when calling phy_stop() + phy_disconnect()
>> which does not indeed happen because we don't make sure we flush the
>> PHYLIB delayed work and let it run to completion. We also need to have
>> a synchronous call to phy_state_machine() in order to have the state
>> machine actually act on PHY_HALTED, set the PHY device's link down, turn
>> the network device's carrier off and finally call the adjust_link()
>> function.
>>
>> Reported-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> Fixes: a390d1f379cf ("phylib: convert state_queue work to delayed_work")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> David, this dates back from before the commit mentioned in Fixes but it would
>> be hard to backport to earlier kernels if we flagged the original design flaw
>> that used timers. Also, I am not clear on the timer API whether there was a way
>> to ensure timers would run to completion before they would be cancelled.
>>
>> Marc, please add your Signed-off-by tag since you contributed the second line.
>>
>> Thanks!
>
> David, please hold off before applying this, I found a corner case with
> power management where we may be accessing clock gated registers and
> cause bus errors, will report back here.
This was a downstream only problem because I was missing these two commits:
49d52e8108a21749dc2114b924c907db43358984 ("net: phy: handle state
correctly in phy_stop_machine")
d5c3d84657db57bd23ecd58b97f1c99dd42a7b80 ("net: phy: Avoid polling PHY
with PHY_IGNORE_INTERRUPTS")
This was tested with bcmgenet (PHY_IGNORE_INTERRUPT and PHY_POLL) and
with bcm_sf2 (net/dsa/slave.c), would appreciate some review/testing
from others as well.
>
>>
>> drivers/net/phy/phy.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index d0626bf5c540..30e7c43e0d87 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -743,12 +743,17 @@ void phy_trigger_machine(struct phy_device *phydev, bool sync)
>> */
>> void phy_stop_machine(struct phy_device *phydev)
>> {
>> + /* Run the state machine to completion */
>> + flush_delayed_work(&phydev->state_queue);
>> cancel_delayed_work_sync(&phydev->state_queue);
>>
>> mutex_lock(&phydev->lock);
>> if (phydev->state > PHY_UP && phydev->state != PHY_HALTED)
>> phydev->state = PHY_UP;
>> mutex_unlock(&phydev->lock);
>> +
>> + /* Now we can run the state machine synchronously */
>> + phy_state_machine(&phydev->state_queue.work);
>> }
>>
>> /**
>>
>
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 10/18] net: mvpp2: use the GoP interrupt for link status changes
From: Russell King - ARM Linux @ 2017-07-26 19:38 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, jason, gregory.clement,
sebastian.hesselbarth, thomas.petazzoni, nadavh, mw, stefanc,
netdev, linux-arm-kernel
In-Reply-To: <20170726162648.GK12049@lunn.ch>
On Wed, Jul 26, 2017 at 06:26:48PM +0200, Andrew Lunn wrote:
> And this is another bit of code you probably need to change in a while
> with phylink lands.
The way the MAC driver handles link up/down and configuration events
changes significantly when a MAC driver switches to phylink, since
a directly connected SFP cage needs to have the MAC reconfigured
between SGMII and 1000base-X modes. If you add SFP+ into that, also
10Gbase-KR as well.
Note also that the "link up" condition for SFP (and probably SFF) is
more complex than just "is the module reporting that it's receiving
a signal" - especially with 1000base-X, there's negotiation to be
performed, so you also need to know (if the module is connected
directly to the MAC) whether the Serdes is in sync and has finished
negotiation (and itself says it has link with the remote end.)
With the Marvell 88x3310 PHY, the MAC driver already needs to switch
between 10Gbase-KR and SGMII modes, as the 88x3310 automatically
makes that switch on its MAC facing interface without software
intervention.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: Roopa Prabhu @ 2017-07-26 19:38 UTC (permalink / raw)
To: David Ahern; +Cc: Hangbin Liu, Cong Wang, network dev
In-Reply-To: <2d75317c-160c-01c2-5a5b-bc6111c099c6@gmail.com>
On Wed, Jul 26, 2017 at 12:00 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/26/17 12:55 PM, Roopa Prabhu wrote:
>> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
>>> On 7/26/17 12:27 PM, Roopa Prabhu wrote:
>>>> agreed...so looks like the check in v3 should be
>>>>
>>>>
>>>> + if ( rt == net->ipv6.ip6_null_entry ||
>>>> + (rt->dst.error &&
>>>> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>>>> + rt != net->ipv6.ip6_prohibit_entry &&
>>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>>> +#endif
>>>> + )) {
>>>> err = rt->dst.error;
>>>> ip6_rt_put(rt);
>>>> goto errout;
>>>>
>>>
>>> I don't think so. If I add a prohibit route and use the fibmatch
>>> attribute, I want to see the route from the FIB that was matched.
>>
>>
>> yes, exactly. wouldn't 'rt != net->ipv6.ip6_prohibit_entry' above let
>> it fall through to the route fill code ?
>>
>> ah...but i guess you are saying that they will have rt6_info's of
>> their own and will not match. got it. ack.
>>
>
> This:
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96a819d..24de81c804c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3637,11 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff
> *in_skb, struct nlmsghdr *nlh,
> dst = ip6_route_lookup(net, &fl6, 0);
>
> rt = container_of(dst, struct rt6_info, dst);
> - if (rt->dst.error) {
> - err = rt->dst.error;
> - ip6_rt_put(rt);
> - goto errout;
> - }
>
> if (rt == net->ipv6.ip6_null_entry) {
> err = rt->dst.error;
>
> Puts back the original behavior. In that case, only rt == null_entry
> drops to the error path which is correct. All other rt values will drop
> to rt6_fill_node and return rt data.
yes, i thought so too and hence acked v1. But, following congs
comment, realized that it may mask some real errors for fibmatch ?
I just tested a case of unreachable route with just the above patch
you posted, and I do get the error correctly.
so, I guess you are saying all real errors for fibmatch will have "rt
== net->ipv6.ip6_null_entry" and we should be ok.
sounds good to me.
^ permalink raw reply
* Re: [PATCH net] net: phy: Run state machine to completion
From: Florian Fainelli @ 2017-07-26 19:34 UTC (permalink / raw)
To: netdev
Cc: rmk+kernel, slash.tmp, zach.brown, nathan.sullivan, Andrew Lunn,
open list
In-Reply-To: <20170726192446.32045-1-f.fainelli@gmail.com>
On 07/26/2017 12:24 PM, Florian Fainelli wrote:
> Marc reported that he was not getting the PHY library adjust_link()
> callback function to run when calling phy_stop() + phy_disconnect()
> which does not indeed happen because we don't make sure we flush the
> PHYLIB delayed work and let it run to completion. We also need to have
> a synchronous call to phy_state_machine() in order to have the state
> machine actually act on PHY_HALTED, set the PHY device's link down, turn
> the network device's carrier off and finally call the adjust_link()
> function.
>
> Reported-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> Fixes: a390d1f379cf ("phylib: convert state_queue work to delayed_work")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> David, this dates back from before the commit mentioned in Fixes but it would
> be hard to backport to earlier kernels if we flagged the original design flaw
> that used timers. Also, I am not clear on the timer API whether there was a way
> to ensure timers would run to completion before they would be cancelled.
>
> Marc, please add your Signed-off-by tag since you contributed the second line.
>
> Thanks!
David, please hold off before applying this, I found a corner case with
power management where we may be accessing clock gated registers and
cause bus errors, will report back here.
>
> drivers/net/phy/phy.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index d0626bf5c540..30e7c43e0d87 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -743,12 +743,17 @@ void phy_trigger_machine(struct phy_device *phydev, bool sync)
> */
> void phy_stop_machine(struct phy_device *phydev)
> {
> + /* Run the state machine to completion */
> + flush_delayed_work(&phydev->state_queue);
> cancel_delayed_work_sync(&phydev->state_queue);
>
> mutex_lock(&phydev->lock);
> if (phydev->state > PHY_UP && phydev->state != PHY_HALTED)
> phydev->state = PHY_UP;
> mutex_unlock(&phydev->lock);
> +
> + /* Now we can run the state machine synchronously */
> + phy_state_machine(&phydev->state_queue.work);
> }
>
> /**
>
--
Florian
^ permalink raw reply
* Re: ARM GLX Khadas VIM Pro - Ethernet detected as only 10Mbps and stalled after some traffic
From: Jerome Brunet @ 2017-07-26 19:32 UTC (permalink / raw)
To: crow, Andrew Lunn; +Cc: netdev, open list:ARM/Amlogic Meson...
In-Reply-To: <CAG_g8w4xD5qY8po09QdqC8w7bgjbmbe_nT4jCbt4Fa3ShWNzew@mail.gmail.com>
On Tue, 2017-07-25 at 18:56 +0200, crow wrote:
> Hi,
> Today I did test on ArchLinuxArm the Kernel v4.13-rc2. On downloading
> the linux git source the network will eventually get stalled. Here are
> the information
>
> Over SSH (network works).
>
> [root@alarm ~]# uname -a
> Linux alarm 4.13.0-rc2-1-ARCH #1 SMP Mon Jul 24 20:02:50 MDT 2017
> aarch64 GNU/Linux
> [root@alarm ~]# mii-tool -vvv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 1000baseT-HD flow-control, link ok
[Replying again on the last thread :) ]
This 1000BaseT Half Duplex looks suspucious if the PHY is supposed to be a
10/100Mbps
> registers for MII PHY 8:
> 1000 782d 0181 4400 01e1 c1e1 000f 2001
> ffff ffff ffff ffff ffff ffff ffff ffff
> 0040 0002 40e8 5400 1c1c 0000 0000 aaaa
> fff0 ffff 0000 000a 1407 004a 0000 105a
> product info: vendor 00:60:51, model 0 rev 0
> basic mode: autonegotiation enabled
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> [root@alarm ~]# ethtool -S eth0
> NIC statistics:
> mmc_tx_octetcount_gb: 0
> mmc_tx_framecount_gb: 0
> mmc_tx_broadcastframe_g: 0
> mmc_tx_multicastframe_g: 0
> mmc_tx_64_octets_gb: 0
> mmc_tx_65_to_127_octets_gb: 0
> mmc_tx_128_to_255_octets_gb: 0
> mmc_tx_256_to_511_octets_gb: 0
> mmc_tx_512_to_1023_octets_gb: 0
> mmc_tx_1024_to_max_octets_gb: 0
> mmc_tx_unicast_gb: 0
> mmc_tx_multicast_gb: 0
> mmc_tx_broadcast_gb: 0
> mmc_tx_underflow_error: 0
> mmc_tx_singlecol_g: 0
> mmc_tx_multicol_g: 0
> mmc_tx_deferred: 0
> mmc_tx_latecol: 0
> mmc_tx_exesscol: 0
> mmc_tx_carrier_error: 0
> mmc_tx_octetcount_g: 0
> mmc_tx_framecount_g: 0
> mmc_tx_excessdef: 0
> mmc_tx_pause_frame: 0
> mmc_tx_vlan_frame_g: 0
> mmc_rx_framecount_gb: 133
> mmc_rx_octetcount_gb: 16646
> mmc_rx_octetcount_g: 16646
> mmc_rx_broadcastframe_g: 9
> mmc_rx_multicastframe_g: 22
> mmc_rx_crc_error: 0
> mmc_rx_align_error: 0
> mmc_rx_run_error: 0
> mmc_rx_jabber_error: 0
> mmc_rx_undersize_g: 0
> mmc_rx_oversize_g: 0
> mmc_rx_64_octets_gb: 45
> mmc_rx_65_to_127_octets_gb: 64
> mmc_rx_128_to_255_octets_gb: 13
> mmc_rx_256_to_511_octets_gb: 7
> mmc_rx_512_to_1023_octets_gb: 4
> mmc_rx_1024_to_max_octets_gb: 0
> mmc_rx_unicast_g: 102
> mmc_rx_length_error: 0
> mmc_rx_autofrangetype: 0
> mmc_rx_pause_frames: 0
> mmc_rx_fifo_overflow: 0
> mmc_rx_vlan_frames_gb: 0
> mmc_rx_watchdog_error: 0
> mmc_rx_ipc_intr_mask: 1073692671
> mmc_rx_ipc_intr: 0
> mmc_rx_ipv4_gd: 117
> mmc_rx_ipv4_hderr: 0
> mmc_rx_ipv4_nopay: 0
> mmc_rx_ipv4_frag: 0
> mmc_rx_ipv4_udsbl: 0
> mmc_rx_ipv4_gd_octets: 12585
> mmc_rx_ipv4_hderr_octets: 0
> mmc_rx_ipv4_nopay_octets: 0
> mmc_rx_ipv4_frag_octets: 0
> mmc_rx_ipv4_udsbl_octets: 0
> mmc_rx_ipv6_gd_octets: 104
> mmc_rx_ipv6_hderr_octets: 0
> mmc_rx_ipv6_nopay_octets: 0
> mmc_rx_ipv6_gd: 1
> mmc_rx_ipv6_hderr: 0
> mmc_rx_ipv6_nopay: 0
> mmc_rx_udp_gd: 31
> mmc_rx_udp_err: 0
> mmc_rx_tcp_gd: 85
> mmc_rx_tcp_err: 0
> mmc_rx_icmp_gd: 2
> mmc_rx_icmp_err: 0
> mmc_rx_udp_gd_octets: 2963
> mmc_rx_udp_err_octets: 0
> mmc_rx_tcp_gd_octets: 7254
> mmc_rx_tcp_err_octets: 0
> mmc_rx_icmp_gd_octets: 92
> mmc_rx_icmp_err_octets: 0
> tx_underflow: 0
> tx_carrier: 0
> tx_losscarrier: 0
> vlan_tag: 0
> tx_deferred: 0
> tx_vlan: 0
> tx_jabber: 0
> tx_frame_flushed: 0
> tx_payload_error: 0
> tx_ip_header_error: 0
> rx_desc: 0
> sa_filter_fail: 0
> overflow_error: 0
> ipc_csum_error: 0
> rx_collision: 0
> rx_crc_errors: 0
> dribbling_bit: 0
> rx_length: 0
> rx_mii: 0
> rx_multicast: 0
> rx_gmac_overflow: 0
> rx_watchdog: 0
> da_rx_filter_fail: 0
> sa_rx_filter_fail: 0
> rx_missed_cntr: 0
> rx_overflow_cntr: 0
> rx_vlan: 0
> tx_undeflow_irq: 0
> tx_process_stopped_irq: 0
> tx_jabber_irq: 0
> rx_overflow_irq: 0
> rx_buf_unav_irq: 0
> rx_process_stopped_irq: 0
> rx_watchdog_irq: 0
> tx_early_irq: 0
> fatal_bus_error_irq: 0
> rx_early_irq: 0
> threshold: 1
> tx_pkt_n: 83
> rx_pkt_n: 133
> normal_irq_n: 130
> rx_normal_irq_n: 129
> napi_poll: 130
> tx_normal_irq_n: 1
> tx_clean: 192
> tx_set_ic_bit: 1
> irq_receive_pmt_irq_n: 0
> mmc_tx_irq_n: 0
> mmc_rx_irq_n: 0
> mmc_rx_csum_offload_irq_n: 0
> irq_tx_path_in_lpi_mode_n: 72
> irq_tx_path_exit_lpi_mode_n: 72
> irq_rx_path_in_lpi_mode_n: 0
> irq_rx_path_exit_lpi_mode_n: 0
> phy_eee_wakeup_error_n: 65535
> ip_hdr_err: 0
> ip_payload_err: 0
> ip_csum_bypassed: 0
> ipv4_pkt_rcvd: 0
> ipv6_pkt_rcvd: 0
> no_ptp_rx_msg_type_ext: 0
> ptp_rx_msg_type_sync: 0
> ptp_rx_msg_type_follow_up: 0
> ptp_rx_msg_type_delay_req: 0
> ptp_rx_msg_type_delay_resp: 0
> ptp_rx_msg_type_pdelay_req: 0
> ptp_rx_msg_type_pdelay_resp: 0
> ptp_rx_msg_type_pdelay_follow_up: 0
> ptp_rx_msg_type_announce: 0
> ptp_rx_msg_type_management: 0
> ptp_rx_msg_pkt_reserved_type: 0
> ptp_frame_type: 0
> ptp_ver: 0
> timestamp_dropped: 0
> av_pkt_rcvd: 0
> av_tagged_pkt_rcvd: 0
> vlan_tag_priority_val: 0
> l3_filter_match: 0
> l4_filter_match: 0
> l3_l4_filter_no_match: 0
> irq_pcs_ane_n: 0
> irq_pcs_link_n: 0
> irq_rgmii_n: 0
> mtl_tx_status_fifo_full: 0
> mtl_tx_fifo_not_empty: 0
> mmtl_fifo_ctrl: 0
> mtl_tx_fifo_read_ctrl_write: 0
> mtl_tx_fifo_read_ctrl_wait: 0
> mtl_tx_fifo_read_ctrl_read: 0
> mtl_tx_fifo_read_ctrl_idle: 0
> mac_tx_in_pause: 0
> mac_tx_frame_ctrl_xfer: 0
> mac_tx_frame_ctrl_idle: 0
> mac_tx_frame_ctrl_wait: 0
> mac_tx_frame_ctrl_pause: 0
> mac_gmii_tx_proto_engine: 0
> mtl_rx_fifo_fill_level_full: 0
> mtl_rx_fifo_fill_above_thresh: 0
> mtl_rx_fifo_fill_below_thresh: 0
> mtl_rx_fifo_fill_level_empty: 0
> mtl_rx_fifo_read_ctrl_flush: 0
> mtl_rx_fifo_read_ctrl_read_data: 0
> mtl_rx_fifo_read_ctrl_status: 0
> mtl_rx_fifo_read_ctrl_idle: 0
> mtl_rx_fifo_ctrl_active: 0
> mac_rx_frame_ctrl_fifo: 0
> mac_gmii_rx_proto_engine: 0
> tx_tso_frames: 0
> tx_tso_nfrags: 0
> [root@alarm ~]#
>
>
>
>
> [root@alarm opt]# git clone
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
> Cloning into 'linux-stable'...
> remote: Counting objects: 6071472, done.
> remote: Compressing objects: 100% (961861/961861), done.
> Receiving objects: 0% (22798/6071472), 9.12 MiB | 3.47 MiB/s
>
>
>
> Over serial console:
> journalctl -f
> alarm systemd-timesyncd[256]: Timed out waiting for reply from
> 144.76.197.108:123 (2.arch.pool.ntp.org).
>
> [root@alarm ~]# ping -c3 8.8.8.8
> PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
> From 10.8.8.6 icmp_seq=1 Destination Host Unreachable
> From 10.8.8.6 icmp_seq=2 Destination Host Unreachable
> From 10.8.8.6 icmp_seq=3 Destination Host Unreachable
>
> --- 8.8.8.8 ping statistics ---
> 3 packets transmitted, 0 received, +3 errors, 100% packet loss, time 2047ms
> pipe 3
> [root@alarm ~]#
> [root@alarm ~]# mii-tool -vvv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 1000baseT-HD flow-control, link ok
> registers for MII PHY 8:
> 1000 782d 0181 4400 01e1 c1e1 000d 2001
> ffff ffff ffff ffff ffff ffff ffff ffff
> 0040 0002 40e8 5400 1c1c 0000 0000 aaaa
> fff0 ffff 0000 000a 1407 0000 0000 105a
> product info: vendor 00:60:51, model 0 rev 0
> basic mode: autonegotiation enabled
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> [root@alarm ~]# ethtool -S eth0
> NIC statistics:
> mmc_tx_octetcount_gb: 0
> mmc_tx_framecount_gb: 0
> mmc_tx_broadcastframe_g: 0
> mmc_tx_multicastframe_g: 0
> mmc_tx_64_octets_gb: 0
> mmc_tx_65_to_127_octets_gb: 0
> mmc_tx_128_to_255_octets_gb: 0
> mmc_tx_256_to_511_octets_gb: 0
> mmc_tx_512_to_1023_octets_gb: 0
> mmc_tx_1024_to_max_octets_gb: 0
> mmc_tx_unicast_gb: 0
> mmc_tx_multicast_gb: 0
> mmc_tx_broadcast_gb: 0
> mmc_tx_underflow_error: 0
> mmc_tx_singlecol_g: 0
> mmc_tx_multicol_g: 0
> mmc_tx_deferred: 0
> mmc_tx_latecol: 0
> mmc_tx_exesscol: 0
> mmc_tx_carrier_error: 0
> mmc_tx_octetcount_g: 0
> mmc_tx_framecount_g: 0
> mmc_tx_excessdef: 0
> mmc_tx_pause_frame: 0
> mmc_tx_vlan_frame_g: 0
> mmc_rx_framecount_gb: 14959
> mmc_rx_octetcount_gb: 20761536
> mmc_rx_octetcount_g: 20761536
> mmc_rx_broadcastframe_g: 22
> mmc_rx_multicastframe_g: 64
> mmc_rx_crc_error: 0
> mmc_rx_align_error: 0
> mmc_rx_run_error: 0
> mmc_rx_jabber_error: 0
> mmc_rx_undersize_g: 0
> mmc_rx_oversize_g: 0
> mmc_rx_64_octets_gb: 495
> mmc_rx_65_to_127_octets_gb: 658
> mmc_rx_128_to_255_octets_gb: 73
> mmc_rx_256_to_511_octets_gb: 63
> mmc_rx_512_to_1023_octets_gb: 124
> mmc_rx_1024_to_max_octets_gb: 13546
> mmc_rx_unicast_g: 14873
> mmc_rx_length_error: 0
> mmc_rx_autofrangetype: 0
> mmc_rx_pause_frames: 0
> mmc_rx_fifo_overflow: 0
> mmc_rx_vlan_frames_gb: 0
> mmc_rx_watchdog_error: 0
> mmc_rx_ipc_intr_mask: 2147385342
> mmc_rx_ipc_intr: 0
> mmc_rx_ipv4_gd: 14725
> mmc_rx_ipv4_hderr: 0
> mmc_rx_ipv4_nopay: 0
> mmc_rx_ipv4_frag: 0
> mmc_rx_ipv4_udsbl: 0
> mmc_rx_ipv4_gd_octets: 20476749
> mmc_rx_ipv4_hderr_octets: 0
> mmc_rx_ipv4_nopay_octets: 0
> mmc_rx_ipv4_frag_octets: 0
> mmc_rx_ipv4_udsbl_octets: 0
> mmc_rx_ipv6_gd_octets: 312
> mmc_rx_ipv6_hderr_octets: 0
> mmc_rx_ipv6_nopay_octets: 0
> mmc_rx_ipv6_gd: 3
> mmc_rx_ipv6_hderr: 0
> mmc_rx_ipv6_nopay: 0
> mmc_rx_udp_gd: 51
> mmc_rx_udp_err: 0
> mmc_rx_tcp_gd: 14673
> mmc_rx_tcp_err: 0
> mmc_rx_icmp_gd: 4
> mmc_rx_icmp_err: 0
> mmc_rx_udp_gd_octets: 3924
> mmc_rx_udp_err_octets: 0
> mmc_rx_tcp_gd_octets: 20178297
> mmc_rx_tcp_err_octets: 0
> mmc_rx_icmp_gd_octets: 220
> mmc_rx_icmp_err_octets: 0
> tx_underflow: 0
> tx_carrier: 0
> tx_losscarrier: 0
> vlan_tag: 0
> tx_deferred: 0
> tx_vlan: 0
> tx_jabber: 0
> tx_frame_flushed: 0
> tx_payload_error: 0
> tx_ip_header_error: 0
> rx_desc: 0
> sa_filter_fail: 0
> overflow_error: 0
> ipc_csum_error: 0
> rx_collision: 0
> rx_crc_errors: 0
> dribbling_bit: 0
> rx_length: 0
> rx_mii: 0
> rx_multicast: 0
> rx_gmac_overflow: 0
> rx_watchdog: 0
> da_rx_filter_fail: 0
> sa_rx_filter_fail: 0
> rx_missed_cntr: 0
> rx_overflow_cntr: 0
> rx_vlan: 0
> tx_undeflow_irq: 0
> tx_process_stopped_irq: 0
> tx_jabber_irq: 0
> rx_overflow_irq: 0
> rx_buf_unav_irq: 0
> rx_process_stopped_irq: 0
> rx_watchdog_irq: 0
> tx_early_irq: 0
> fatal_bus_error_irq: 0
> rx_early_irq: 6
> threshold: 1
> tx_pkt_n: 3709
> rx_pkt_n: 12926
> normal_irq_n: 4594
> rx_normal_irq_n: 4537
> napi_poll: 4597
> tx_normal_irq_n: 57
> tx_clean: 5109
> tx_set_ic_bit: 59
> irq_receive_pmt_irq_n: 0
> mmc_tx_irq_n: 0
> mmc_rx_irq_n: 0
> mmc_rx_csum_offload_irq_n: 0
> irq_tx_path_in_lpi_mode_n: 2921
> irq_tx_path_exit_lpi_mode_n: 2920
> irq_rx_path_in_lpi_mode_n: 0
> irq_rx_path_exit_lpi_mode_n: 0
> phy_eee_wakeup_error_n: 65535
> ip_hdr_err: 0
> ip_payload_err: 0
> ip_csum_bypassed: 0
> ipv4_pkt_rcvd: 0
> ipv6_pkt_rcvd: 0
> no_ptp_rx_msg_type_ext: 0
> ptp_rx_msg_type_sync: 0
> ptp_rx_msg_type_follow_up: 0
> ptp_rx_msg_type_delay_req: 0
> ptp_rx_msg_type_delay_resp: 0
> ptp_rx_msg_type_pdelay_req: 0
> ptp_rx_msg_type_pdelay_resp: 0
> ptp_rx_msg_type_pdelay_follow_up: 0
> ptp_rx_msg_type_announce: 0
> ptp_rx_msg_type_management: 0
> ptp_rx_msg_pkt_reserved_type: 0
> ptp_frame_type: 0
> ptp_ver: 0
> timestamp_dropped: 0
> av_pkt_rcvd: 0
> av_tagged_pkt_rcvd: 0
> vlan_tag_priority_val: 0
> l3_filter_match: 0
> l4_filter_match: 0
> l3_l4_filter_no_match: 0
> irq_pcs_ane_n: 0
> irq_pcs_link_n: 0
> irq_rgmii_n: 0
> mtl_tx_status_fifo_full: 0
> mtl_tx_fifo_not_empty: 0
> mmtl_fifo_ctrl: 0
> mtl_tx_fifo_read_ctrl_write: 0
> mtl_tx_fifo_read_ctrl_wait: 0
> mtl_tx_fifo_read_ctrl_read: 0
> mtl_tx_fifo_read_ctrl_idle: 0
> mac_tx_in_pause: 0
> mac_tx_frame_ctrl_xfer: 0
> mac_tx_frame_ctrl_idle: 0
> mac_tx_frame_ctrl_wait: 0
> mac_tx_frame_ctrl_pause: 0
> mac_gmii_tx_proto_engine: 0
> mtl_rx_fifo_fill_level_full: 0
> mtl_rx_fifo_fill_above_thresh: 0
> mtl_rx_fifo_fill_below_thresh: 0
> mtl_rx_fifo_fill_level_empty: 0
> mtl_rx_fifo_read_ctrl_flush: 0
> mtl_rx_fifo_read_ctrl_read_data: 0
> mtl_rx_fifo_read_ctrl_status: 0
> mtl_rx_fifo_read_ctrl_idle: 0
> mtl_rx_fifo_ctrl_active: 0
> mac_rx_frame_ctrl_fifo: 0
> mac_gmii_rx_proto_engine: 0
> tx_tso_frames: 0
> tx_tso_nfrags: 0
> [root@alarm ~]#
> [root@alarm ~]# ifconfig eth0 down && ifconfig eth0 up
> Meson GXL Internal PHY 0.e40908ff:08: attached PHY driver [Meson GXL
> Internal PHY] (mii_bus:phy_addr=0.e40908ff:08, irq=-1)
> meson8b-dwmac c9410000.ethernet eth0: PTP not supported by HW
> meson8b-dwmac c9410000.ethernet eth0: Link is Up - 100Mbps/Full - flow
> control off
> [root@alarm ~]#
>
>
>
> whole dmesg [1]. there are some messages like: mdio-mux-mmioreg
> c883455c.eth-phy-mux: failed to register mdio-mux bus
> /soc/periphs@c8834000/eth-phy-mux
>
> [1] https://defuse.ca/b/s2NpyJlw
>
> Regards,
>
>
> On Tue, Jun 27, 2017 at 7:14 PM, crow <crow@linux.org.ba> wrote:
> > Hi,
> > There are other user reporting same issue while using mainline kernel
> > but using Ubuntu, so this is for sure not Distribution related. For me
> > see the [0]. I hope someone would get time after 4.12 release to try
> > fix this issue.
> >
> > Regards,
> >
> > [0] http://forum.khadas.com/t/ubuntu-server-rom-linux-mainline-v170624-pre-a
> > lpha-version-emmc-installation/803/12
> >
> > On Thu, Jun 15, 2017 at 4:40 PM, crow <crow@linux.org.ba> wrote:
> > > Hi,
> > >
> > > On Sun, Jun 11, 2017 at 7:03 PM, crow <crow@linux.org.ba> wrote:
> > > > Hi Andrew,
> > > >
> > > > On Sun, Jun 11, 2017 at 5:21 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > > > > > Thank your for the suggestion, and thanks Martin to explaining me
> > > > > > over
> > > > > > IRC what actually I should do.
> > > > > >
> > > > > > I recompiled mainline kernel 4.12-rc4 with the Amlogic driver:
> > > > > > replaced drivers/net/phy/meson-gxl.c with
> > > > > > https://github.com/khadas/linux/blob/ubuntu-4.9/drivers/amlogic/ethe
> > > > > > rnet/phy/amlogic.c
> > > > > >
> > > > > > But this did not solve the issue. As soon as i start git clone i
> > > > > > lose
> > > > > > network connection to device (no session timeout/disconnect this
> > > > > > time,
> > > > > > but I am unable to reconnect over SSH or to get OK ping replay
> > > > > > back).
> > > >
> > > > 1) First problem reported I can't reproduce anymore, every reboot/cold
> > > > boot with mainline kernel the Ethernet speed is detected as
> > > > "100Mbps/Full" , but as seen in first post there was this issue.
> > >
> > > Once I did setup u-boot to have network in u-boot and did just an ping
> > > to activate network. And after boot Ethernet was detected as 10Mbps.
> > > But again was not able to reproduce it. I double check that I have 5E
> > > cable.
> > >
> > > in u-boot Ethernet is detected as below
> > > kvim#ping x.x.x.x
> > > Speed: 100, full duplex
> > > Using dwmac.c9410000 device
> > > host x.x.x.x is alive
> > > kvim#
> > >
> > > then I let ArchLinuxArm to boot and found out I can't connect to
> > > device over SSH. Check over serial console and found:
> > >
> > > # dmesg | tail -n 10
> > > [ 8.334790] meson8b-dwmac c9410000.ethernet eth0: device MAC
> > > address 00:15:18:01:81:31
> > > [ 8.436668] Meson GXL Internal PHY 0.e40908ff:08: attached PHY
> > > driver [Meson GXL Internal PHY] (mii_bus:phy_addr=0.e40908ff:08,
> > > irq=-1)
> > > [ 8.535171] meson8b-dwmac c9410000.ethernet eth0: PTP not supported by
> > > HW
> > > [ 10.225264] brcmfmac: brcmf_c_preinit_dcmds: Firmware version =
> > > wl0: Mar 1 2015 07:29:38 version 7.45.18 (r538002) FWID 01-6a2c8ad4
> > > [ 10.635703] meson8b-dwmac c9410000.ethernet eth0: Link is Up -
> > > 10Mbps/Half - flow control off
> > > # uname -a
> > > Linux khadasvimpro 4.12.0-rc4-3-ARCH #1 SMP Thu Jun 8 00:17:20 CEST
> > > 2017 aarch64 GNU/Linux
> > > #
> > > # mii-tool -vvv eth0
> > > Using SIOCGMIIPHY=0x8947
> > > eth0: no autonegotiation,, link ok
> > > registers for MII PHY 8:
> > > 1000 782d 0181 4400 01e1 0001 0005 2001
> > > ffff ffff ffff ffff ffff ffff ffff ffff
> > > 0040 0002 40e8 5400 1c1c 0000 0000 aaaa
> > > fff0 ffff 0000 000a 1407 0040 0000 105a
> > > product info: vendor 00:60:51, model 0 rev 0
> > > basic mode: autonegotiation enabled
> > > basic status: autonegotiation complete, link ok
> > > capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> > > 10baseT-FD 10baseT-HD
> > > advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> > > 10baseT-FD 10baseT-HD
> > > #
> > > # ifconfig eth0 down && ifconfig eth0 up
> > > [ 1972.596690] Meson GXL Internal PHY 0.e40908ff:08: attached PHY
> > > driver [Meson GXL Internal PHY] (mii_bus:phy_addr=0.e40908ff:08,
> > > irq=-1)
> > > [ 1972.704156] meson8b-dwmac c9410000.ethernet eth0: PTP not supported by
> > > HW
> > > [ 1974.795698] meson8b-dwmac c9410000.ethernet eth0: Link is Up -
> > > 100Mbps/Full - flow control off
> > > #
> > > # mii-tool -vvv eth0
> > > Using SIOCGMIIPHY=0x8947
> > > eth0: negotiated 1000baseT-HD flow-control, link ok
> > > registers for MII PHY 8:
> > > 1000 782d 0181 4400 01e1 c1e1 000f 2001
> > > ffff ffff ffff ffff ffff ffff ffff ffff
> > > 0040 0002 40e8 5400 1c1c 0000 0000 aaaa
> > > fff0 ffff 0000 020a 1407 00ca 0000 105a
> > > product info: vendor 00:60:51, model 0 rev 0
> > > basic mode: autonegotiation enabled
> > > basic status: autonegotiation complete, link ok
> > > capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> > > 10baseT-FD 10baseT-HD
> > > advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> > > 10baseT-FD 10baseT-HD
> > > link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> > > 10baseT-FD 10baseT-HD
> > > #
> > >
> > > 2) see below
> > > > 2) see below
> > > >
> > > > > So this shows it is more than a PHY problem. The Ethernet MAC driver
> > > > > is probably also part of the problem.
> > > >
> > > > There are some stmmac fixes [1] in soon to be rc5, compiled current
> > > > master (without amlogic.c) with the fixes but for me the issue still
> > > > persist. I will compile once released rc5 with amlogic.c and report
> > > > back.
> > > >
> > > > > Are there any mainline kernels which work O.K?
> > > >
> > > > Khadas VIM support was added in 4.12-rc1. And I did test all four rc's
> > > > but without success.
> > >
> > > I did test many Kernel builds but all test have failed when
> > > downloading bigger files / doing git clone.
> > > As Martin Blumenstingl suggested I start with first commit where
> > > Khadas VIM support was added [0]. Then also Neil Armstrong suggested
> > > [1]. Then all 4.12-rc1 - rc5.
> > > Martin Blumenstingl have also found himself that: "I can reproduce the
> > > Ethernet problem (tried downloading a 1GiB test file from leaseweb,
> > > network got stuck after downloading ~70 MiB)". He suggested that I
> > > should "play with the settings on your switch (disable jumbo frames,
> > > etc.) to rule out the "exotic" stuff?". Well other device (x86_64)
> > > connected on this same Switch port does not have any problem
> > > downloading big files or doing git clone, as well as Khadas VIM with
> > > Amlogic kernel. Also jumbo frames are not enabled, switch does have
> > > only standard settings.
> > >
> > > I also get questioned which qdisc I use:
> > > And it seems I am already using fq_codel (ArchLinuxArm uses systemd):
> > > $ tc -s -p qdisc
> > > qdisc noqueue 0: dev lo root refcnt 2
> > > Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> > > backlog 0b 0p requeues 0
> > > qdisc mq 0: dev eth0 root
> > > Sent 7382956 bytes 60703 pkt (dropped 0, overlimits 0 requeues 18)
> > > backlog 0b 0p requeues 18
> > > qdisc fq_codel 0: dev eth0 parent :1 limit 10240p flows 1024 quantum
> > > 1514 target 5.0ms interval 100.0ms memory_limit 32Mb ecn
> > > Sent 7382956 bytes 60703 pkt (dropped 0, overlimits 0 requeues 18)
> > > backlog 0b 0p requeues 18
> > > maxpacket 54 drop_overlimit 0 new_flow_count 14 ecn_mark 0
> > > new_flows_len 0 old_flows_len 0
> > > $ pacman -Qi systemd
> > > Name : systemd
> > > Version : 232-8
> > > Description : system and service manager
> > > Architecture : aarch64
> > > ...
> > > $
> > >
> > >
> > > Regards,
> > >
> > > > > Andrew
> > > >
> > > > [1] https://github.com/torvalds/linux/commit/426849e6611f2092553f8d53372
> > > > ae310818a6292
> > >
> > > [0] https://github.com/torvalds/linux/commit/e15d2774b8c096f116bf7192b37e8
> > > 652da71369e
> > > [1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/khilman/linux
> > > -amlogic/+/v4.12/integ
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply
* Re: ARM GLX Khadas VIM Pro - Ethernet detected as only 10Mbps and stalled after some traffic
From: Jerome Brunet @ 2017-07-26 19:27 UTC (permalink / raw)
To: crow, Andrew Lunn; +Cc: netdev, linux-amlogic
In-Reply-To: <CAG_g8w7aaryfdGgRtbazGMGm5svebgNPNcKyWBfxaObMHs3x-Q@mail.gmail.com>
On Sun, 2017-06-11 at 08:31 +0200, crow wrote:
> Hi Andrew,
>
> On Sat, Jun 10, 2017 at 5:27 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > > Also what Martin Blumenstingl wrote is following which is also crucial
> > > for fixing the issue:
> > > Amlogic has given their ethernet PHY driver some updates [2], it now
> > > includes wake-on-lan, and they now have an internal_phy_read_status
> > > which uses reset_internal_phy if there's a link and some error counter
> > > exceeds some magic value.
> >
> > Hi Crow
> >
> > You could probably just drop the Amlogic driver into mainline and see
> > if it works better. If that solves your problem, we can look at
> > merging the changes.
> >
> > Andrew
>
> Thank your for the suggestion, and thanks Martin to explaining me over
> IRC what actually I should do.
>
> I recompiled mainline kernel 4.12-rc4 with the Amlogic driver:
> replaced drivers/net/phy/meson-gxl.c with
> https://github.com/khadas/linux/blob/ubuntu-4.9/drivers/amlogic/ethernet/phy/a
> mlogic.c
>
> But this did not solve the issue. As soon as i start git clone i lose
> network connection to device (no session timeout/disconnect this time,
> but I am unable to reconnect over SSH or to get OK ping replay back).
>
> Here are the tests:
> Linux khadasvimpro 4.12.0-rc4-4-ARCH #1 SMP Sun Jun 11 03:39:21 CEST
> 2017 aarch64 GNU/Linux
>
> # modinfo meson_gxl
> filename:
> /lib/modules/4.12.0-rc4-4-ARCH/kernel/drivers/net/phy/meson-gxl.ko.gz
> license: GPL
> author: Neil Armstrong <narmstrong@baylibre.com>
> author: Baoqi wang
> description: Amlogic Meson GXL Internal PHY driver
> alias: mdio:0000000110000001010001000000????
> depends:
> intree: Y
> vermagic: 4.12.0-rc4-4-ARCH SMP mod_unload aarch64
> #
> # mii-tool -vvv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 1000baseT-HD flow-control, link ok
Hum, 1000Mbps Half duplex looks duplex looks suspicious
The PHY is supposed to be a 10/100, right ? or did I miss something ?
> registers for MII PHY 8:
> 1000 782d 0181 4400 01e1 c1e1 000f 2001
> ffff ffff ffff ffff ffff ffff ffff ffff
> 0040 0002 40e8 5400 1c1c 0000 0000 aaaa
> fff0 ffff 0000 040a 1407 004a 0000 105a
> product info: vendor 00:60:51, model 0 rev 0
> basic mode: autonegotiation enabled
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> $
>
> over SSH startet following but it stall already at 0%:
>
> $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-
> stable.git
> Cloning into 'linux-stable'...
> remote: Counting objects: 5948690, done.
> remote: Compressing objects: 100% (124799/124799), done.
> Receiving objects: 0% (11668/5948690), 2.27 MiB | 4.52 MiB/s
>
> shows timeout while trying to sync with NTP server:
>
> # journalctl -f
> systemd-timesyncd[299]: Timed out waiting for reply from
> 83.68.137.76:123 (2.at.pool.ntp.org).
> systemd-timesyncd[299]: Timed out waiting for reply from
> 86.59.113.114:123 (2.at.pool.ntp.org).
>
> while still not working dump the register:
> # mii-tool -vvv eth0
> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 1000baseT-HD flow-control, link ok
> registers for MII PHY 8:
> 1000 782d 0181 4400 01e1 c1e1 000d 2001
> ffff ffff ffff ffff ffff ffff ffff ffff
> 0040 0002 40e8 5400 1c1c 0000 0000 aaaa
> fff0 ffff 0000 040a 1407 0000 0000 105a
> product info: vendor 00:60:51, model 0 rev 0
> basic mode: autonegotiation enabled
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> advertising: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> #
>
> # ifconfig eth0 down && ifconfig eth0 up
> # dmesg -T | tail -n 10
> [Sun Jun 11 07:40:34 2017] meson8b-dwmac c9410000.ethernet eth0:
> device MAC address 00:15:18:01:81:31
> [Sun Jun 11 07:40:34 2017] random: crng init done
> [Sun Jun 11 07:40:34 2017] Meson GXL Internal PHY 0.e40908ff:08:
> attached PHY driver [Meson GXL Internal PHY]
> (mii_bus:phy_addr=0.e40908ff:08, irq=-1)
> [Sun Jun 11 07:40:34 2017] meson8b-dwmac c9410000.ethernet eth0: PTP
> not supported by HW
> [Sun Jun 11 07:40:36 2017] brcmfmac: brcmf_c_preinit_dcmds: Firmware
> version = wl0: Mar 1 2015 07:29:38 version 7.45.18 (r538002) FWID
> 01-6a2c8ad4
> [Sun Jun 11 07:40:36 2017] meson8b-dwmac c9410000.ethernet eth0: Link
> is Up - 100Mbps/Full - flow control off
> [Sun Jun 11 07:41:23 2017] EXT4-fs (mmcblk1p1): mounted filesystem
> with ordered data mode. Opts: (null)
> [Sun Jun 11 07:54:28 2017] Meson GXL Internal PHY 0.e40908ff:08:
> attached PHY driver [Meson GXL Internal PHY]
> (mii_bus:phy_addr=0.e40908ff:08, irq=-1)
> [Sun Jun 11 07:54:28 2017] meson8b-dwmac c9410000.ethernet eth0: PTP
> not supported by HW
> [Sun Jun 11 07:54:30 2017] meson8b-dwmac c9410000.ethernet eth0: Link
> is Up - 100Mbps/Full - flow control off
> #
>
> then I took eth0 and wlan0 up (same ArchLinuxArm with same mainline
> kernel same place where the files are stored eMMC) and this was
> working so it should not be the ArchLinuxArm which makes problem, and
> did same git clone as with custom Amlogic kernel (3.1.4 and 4.9.26
> under Khadas Ubuntu image) and test was successfully:
>
> $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-
> stable.git
> Cloning into 'linux-stable'...
> remote: Counting objects: 5948690, done.
> remote: Compressing objects: 100% (124799/124799), done.
> remote: Total 5948690 (delta 427756), reused 549521 (delta 425675)
> Receiving objects: 100% (5948690/5948690), 1.21 GiB | 4.94 MiB/s, done.
> Resolving deltas: 100% (4961965/4961965), done.
> Checking out files: 100% (59844/59844), done.
> $
>
> Regards,
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply
* [PATCH net] net: phy: Run state machine to completion
From: Florian Fainelli @ 2017-07-26 19:24 UTC (permalink / raw)
To: netdev
Cc: rmk+kernel, slash.tmp, zach.brown, nathan.sullivan,
Florian Fainelli, Andrew Lunn, open list
Marc reported that he was not getting the PHY library adjust_link()
callback function to run when calling phy_stop() + phy_disconnect()
which does not indeed happen because we don't make sure we flush the
PHYLIB delayed work and let it run to completion. We also need to have
a synchronous call to phy_state_machine() in order to have the state
machine actually act on PHY_HALTED, set the PHY device's link down, turn
the network device's carrier off and finally call the adjust_link()
function.
Reported-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
Fixes: a390d1f379cf ("phylib: convert state_queue work to delayed_work")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
David, this dates back from before the commit mentioned in Fixes but it would
be hard to backport to earlier kernels if we flagged the original design flaw
that used timers. Also, I am not clear on the timer API whether there was a way
to ensure timers would run to completion before they would be cancelled.
Marc, please add your Signed-off-by tag since you contributed the second line.
Thanks!
drivers/net/phy/phy.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d0626bf5c540..30e7c43e0d87 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -743,12 +743,17 @@ void phy_trigger_machine(struct phy_device *phydev, bool sync)
*/
void phy_stop_machine(struct phy_device *phydev)
{
+ /* Run the state machine to completion */
+ flush_delayed_work(&phydev->state_queue);
cancel_delayed_work_sync(&phydev->state_queue);
mutex_lock(&phydev->lock);
if (phydev->state > PHY_UP && phydev->state != PHY_HALTED)
phydev->state = PHY_UP;
mutex_unlock(&phydev->lock);
+
+ /* Now we can run the state machine synchronously */
+ phy_state_machine(&phydev->state_queue.work);
}
/**
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next] net: phy: Remove stale comments referencing timer
From: Andrew Lunn @ 2017-07-26 19:19 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, slash.tmp
In-Reply-To: <20170726190538.27674-1-f.fainelli@gmail.com>
On Wed, Jul 26, 2017 at 12:05:38PM -0700, Florian Fainelli wrote:
> Since commit a390d1f379cf ("phylib: convert state_queue work to
> delayed_work"), the PHYLIB state machine was converted to use delayed
> workqueues, yet some functions were still referencing the PHY library
> timer in their comments, fix that and remove the now unused
> linux/timer.h include.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH v3 net-next 2/2] liquidio: cleanup: removed cryptic and misleading macro
From: Felix Manlunas @ 2017-07-26 19:11 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20170726190945.GA1152@felix-thinkpad.cavium.com>
From: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/octeon_console.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
index 501ad95..15ad1ab 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
@@ -42,8 +42,6 @@ module_param(console_bitmask, int, 0644);
MODULE_PARM_DESC(console_bitmask,
"Bitmask indicating which consoles have debug output redirected to syslog.");
-#define CAST_ULL(v) ((u64)(v))
-
#define BOOTLOADER_PCI_READ_BUFFER_DATA_ADDR 0x0006c008
#define BOOTLOADER_PCI_READ_BUFFER_LEN_ADDR 0x0006c004
#define BOOTLOADER_PCI_READ_BUFFER_OWNER_ADDR 0x0006c000
@@ -233,7 +231,7 @@ static int __cvmx_bootmem_check_version(struct octeon_device *oct,
(exact_match && major_version != exact_match)) {
dev_err(&oct->pci_dev->dev, "bootmem ver mismatch %d.%d addr:0x%llx\n",
major_version, minor_version,
- CAST_ULL(oct->bootmem_desc_addr));
+ (long long)oct->bootmem_desc_addr);
return -1;
} else {
return 0;
--
2.9.0
^ permalink raw reply related
* [PATCH v3 net-next 1/2] liquidio: standardization: use min_t instead of custom macro
From: Felix Manlunas @ 2017-07-26 19:10 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
In-Reply-To: <20170726190945.GA1152@felix-thinkpad.cavium.com>
From: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Rick Farrington <ricardo.farrington@cavium.com>
Signed-off-by: Derek Chickles <derek.chickles@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
drivers/net/ethernet/cavium/liquidio/octeon_console.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
index e08f760..501ad95 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
@@ -42,7 +42,6 @@ module_param(console_bitmask, int, 0644);
MODULE_PARM_DESC(console_bitmask,
"Bitmask indicating which consoles have debug output redirected to syslog.");
-#define MIN(a, b) min((a), (b))
#define CAST_ULL(v) ((u64)(v))
#define BOOTLOADER_PCI_READ_BUFFER_DATA_ADDR 0x0006c008
@@ -704,7 +703,7 @@ static int octeon_console_read(struct octeon_device *oct, u32 console_num,
if (bytes_to_read <= 0)
return bytes_to_read;
- bytes_to_read = MIN(bytes_to_read, (s32)buf_size);
+ bytes_to_read = min_t(s32, bytes_to_read, buf_size);
/* Check to see if what we want to read is not contiguous, and limit
* ourselves to the contiguous block
--
2.9.0
^ permalink raw reply related
* [PATCH v3 net-next 0/2] liquidio: standardization and cleanup
From: Felix Manlunas @ 2017-07-26 19:09 UTC (permalink / raw)
To: davem
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
ricardo.farrington
From: Rick Farrington <ricardo.farrington@cavium.com>
This patchset corrects some non-standard macro usage.
1. Replaced custom MIN macro with use of standard 'min_t'.
2. Removed cryptic and misleading macro 'CAST_ULL'.
change log:
V1 -> V2:
1. Add driver cleanup of macro 'CAST_ULL'.
V2 -> V3:
1. Remove extra parentheses from previous usage of macro 'CAST_ULL'.
Rick Farrington (2):
liquidio: standardization: use min_t instead of custom macro
liquidio: cleanup: removed cryptic and misleading macro
drivers/net/ethernet/cavium/liquidio/octeon_console.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
--
2.9.0
^ permalink raw reply
* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Casey Leedom @ 2017-07-26 19:05 UTC (permalink / raw)
To: Alexander Duyck
Cc: Netdev, Bjorn Helgaas, linux-arm-kernel@lists.infradead.org,
David.Laight@aculab.com, ashok.raj@intel.com, Alex Williamson,
l.stach@pengutronix.de, Suravee.Suthikulpanit@amd.com,
catalin.marinas@arm.com, linux-pci@vger.kernel.org,
will.deacon@arm.com, Sinan Kaya, robin.murphy@arm.com,
linux-kernel@vger.kernel.org, davem@davemloft.net,
Ganesh GR <g
In-Reply-To: <CAKgT0Udn2vh6NaqZyiF69nXVnz2sT=e0ZgiDjWznhGZz-Gk+qQ@mail.gmail.com>
| From: Alexander Duyck <alexander.duyck@gmail.com>
| Sent: Wednesday, July 26, 2017 11:44 AM
|
| On Jul 26, 2017 11:26 AM, "Casey Leedom" <leedom@chelsio.com> wrote:
| |
| | I think that the patch will need to be extended to modify
| | drivers/pci.c/iov.c:sriov_enable() to explicitly turn off
| | Relaxed Ordering Enable if the Root Complex is marked
| for no RO TLPs.
|
| I'm not sure that would be an issue. Wouldn't most VFs inherit the PF's settings?
Ah yes, you're right. This is covered in section 3.5.4 of the Single Root I/O
Virtualization and Sharing Specification, Revision 1.0 (September 11, 2007),
governing the PCIe Capability Device Control register. It states that the VF
version of that register shall follow the setting of the corresponding PF.
So we should enhance the cxgb4vf/sge.c:t4vf_sge_alloc_rxq() in the same
way we did for the cxgb4 driver, but that's not critical since the Relaxed
Ordering Enable supersedes the internal chip's desire to use the Relaxed
Ordering Attribute.
Ding, send me a note if you'd like me to work that up for you.
| Also I thought most of the VF configuration space is read only.
Yes, but not all of it. And when a VF is exported to a Virtual Machine,
then the Hypervisor captures and interprets all accesses to the VF's
PCIe Configuration Space from the VM.
Thanks again for reminding me of the subtle aspect of the SR_IOV
specification that I forgot.
Casey
^ permalink raw reply
* [PATCH net-next] net: phy: Remove stale comments referencing timer
From: Florian Fainelli @ 2017-07-26 19:05 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, slash.tmp, Florian Fainelli
Since commit a390d1f379cf ("phylib: convert state_queue work to
delayed_work"), the PHYLIB state machine was converted to use delayed
workqueues, yet some functions were still referencing the PHY library
timer in their comments, fix that and remove the now unused
linux/timer.h include.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d0626bf5c540..ac1dcf0289fa 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -30,7 +30,6 @@
#include <linux/ethtool.h>
#include <linux/phy.h>
#include <linux/phy_led_triggers.h>
-#include <linux/timer.h>
#include <linux/workqueue.h>
#include <linux/mdio.h>
#include <linux/io.h>
@@ -705,8 +704,8 @@ EXPORT_SYMBOL(phy_start_aneg);
*
* Description: The PHY infrastructure can run a state machine
* which tracks whether the PHY is starting up, negotiating,
- * etc. This function starts the timer which tracks the state
- * of the PHY. If you want to maintain your own state machine,
+ * etc. This function starts the delayed workqueue which tracks
+ * the state of the PHY. If you want to maintain your own state machine,
* do not call this function.
*/
void phy_start_machine(struct phy_device *phydev)
@@ -737,9 +736,9 @@ void phy_trigger_machine(struct phy_device *phydev, bool sync)
* phy_stop_machine - stop the PHY state machine tracking
* @phydev: target phy_device struct
*
- * Description: Stops the state machine timer, sets the state to UP
- * (unless it wasn't up yet). This function must be called BEFORE
- * phy_detach.
+ * Description: Stops the state machine delayed workqueue, sets the
+ * state to UP (unless it wasn't up yet). This function must be
+ * called BEFORE phy_detach.
*/
void phy_stop_machine(struct phy_device *phydev)
{
--
2.9.3
^ permalink raw reply related
* Re: TCP fast retransmit issues
From: Neal Cardwell @ 2017-07-26 19:02 UTC (permalink / raw)
To: Willy Tarreau
Cc: Eric Dumazet, Klavs Klavsen, Netdev, Yuchung Cheng,
Nandita Dukkipati
In-Reply-To: <CADVnQynR8dUuUBDfeFR2xpL-wKzd+7TgbMv-CXe3TT-T0_PXyQ@mail.gmail.com>
On Wed, Jul 26, 2017 at 2:38 PM, Neal Cardwell <ncardwell@google.com> wrote:
> Yeah, it looks like I can reproduce this issue with (1) bad sacks
> causing repeated TLPs, and (2) TLPs timers being pushed out to later
> times due to incoming data. Scripts are attached.
I'm testing a fix of only scheduling a TLP if (flag & FLAG_DATA_ACKED)
is true...
neal
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: David Ahern @ 2017-07-26 19:00 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Hangbin Liu, Cong Wang, network dev
In-Reply-To: <CAJieiUi+Me_wXDz0A1oLH2LgBw9DsMoxctV56SjEwGpusFGt4Q@mail.gmail.com>
On 7/26/17 12:55 PM, Roopa Prabhu wrote:
> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
>> On 7/26/17 12:27 PM, Roopa Prabhu wrote:
>>> agreed...so looks like the check in v3 should be
>>>
>>>
>>> + if ( rt == net->ipv6.ip6_null_entry ||
>>> + (rt->dst.error &&
>>> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>>> + rt != net->ipv6.ip6_prohibit_entry &&
>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>> +#endif
>>> + )) {
>>> err = rt->dst.error;
>>> ip6_rt_put(rt);
>>> goto errout;
>>>
>>
>> I don't think so. If I add a prohibit route and use the fibmatch
>> attribute, I want to see the route from the FIB that was matched.
>
>
> yes, exactly. wouldn't 'rt != net->ipv6.ip6_prohibit_entry' above let
> it fall through to the route fill code ?
>
> ah...but i guess you are saying that they will have rt6_info's of
> their own and will not match. got it. ack.
>
This:
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96a819d..24de81c804c2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3637,11 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff
*in_skb, struct nlmsghdr *nlh,
dst = ip6_route_lookup(net, &fl6, 0);
rt = container_of(dst, struct rt6_info, dst);
- if (rt->dst.error) {
- err = rt->dst.error;
- ip6_rt_put(rt);
- goto errout;
- }
if (rt == net->ipv6.ip6_null_entry) {
err = rt->dst.error;
Puts back the original behavior. In that case, only rt == null_entry
drops to the error path which is correct. All other rt values will drop
to rt6_fill_node and return rt data.
^ permalink raw reply related
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: Roopa Prabhu @ 2017-07-26 18:55 UTC (permalink / raw)
To: David Ahern; +Cc: Hangbin Liu, Cong Wang, network dev
In-Reply-To: <64377a01-38df-6d43-16a4-401d426fb9b2@gmail.com>
On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
> On 7/26/17 12:27 PM, Roopa Prabhu wrote:
>> agreed...so looks like the check in v3 should be
>>
>>
>> + if ( rt == net->ipv6.ip6_null_entry ||
>> + (rt->dst.error &&
>> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>> + rt != net->ipv6.ip6_prohibit_entry &&
>> + rt != net->ipv6.ip6_blk_hole_entry &&
>> +#endif
>> + )) {
>> err = rt->dst.error;
>> ip6_rt_put(rt);
>> goto errout;
>>
>
> I don't think so. If I add a prohibit route and use the fibmatch
> attribute, I want to see the route from the FIB that was matched.
yes, exactly. wouldn't 'rt != net->ipv6.ip6_prohibit_entry' above let
it fall through to the route fill code ?
ah...but i guess you are saying that they will have rt6_info's of
their own and will not match. got it. ack.
^ permalink raw reply
* Re: [PATCH] netpoll: Fix device name check in netpoll_setup()
From: Doug Anderson @ 2017-07-26 18:44 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: David S . Miller, netdev, linux-kernel@vger.kernel.org
In-Reply-To: <20170725183625.132345-1-mka@chromium.org>
Hi,
On Tue, Jul 25, 2017 at 11:36 AM, Matthias Kaehlcke <mka@chromium.org> wrote:
> Apparently netpoll_setup() assumes that netpoll.dev_name is a pointer
> when checking if the device name is set:
>
> if (np->dev_name) {
> ...
>
> However the field is a character array, therefore the condition always
> yields true. Check instead whether the first byte of the array has a
> non-zero value.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> net/core/netpoll.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 8357f164c660..912731bed7b7 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -666,7 +666,7 @@ int netpoll_setup(struct netpoll *np)
> int err;
>
> rtnl_lock();
> - if (np->dev_name) {
> + if (np->dev_name[0]) {
> struct net *net = current->nsproxy->net_ns;
> ndev = __dev_get_by_name(net, np->dev_name);
> }
It's really up to the maintainer of the code, but my first instinct
here would be to instead remove the "if" test unless we really expect
dev->dev_name to be blank in lots of cases. It will slightly slow
down the error case but should avoid an "if" test in the non-error
case. By definition it should be safe since currently the "if" test
should always evaluate to true.
-Doug
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: David Ahern @ 2017-07-26 18:49 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Hangbin Liu, Cong Wang, network dev
In-Reply-To: <CAJieiUgyHDksxVjoCSz9pxbDy-ZZR9uUWWZXdth0diSKAcvx4Q@mail.gmail.com>
On 7/26/17 12:27 PM, Roopa Prabhu wrote:
> agreed...so looks like the check in v3 should be
>
>
> + if ( rt == net->ipv6.ip6_null_entry ||
> + (rt->dst.error &&
> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> + rt != net->ipv6.ip6_prohibit_entry &&
> + rt != net->ipv6.ip6_blk_hole_entry &&
> +#endif
> + )) {
> err = rt->dst.error;
> ip6_rt_put(rt);
> goto errout;
>
I don't think so. If I add a prohibit route and use the fibmatch
attribute, I want to see the route from the FIB that was matched.
^ permalink raw reply
* Re: [PATCHv3 net] ipv6: no need to return rt->dst.error if it is prohibit entry
From: David Ahern @ 2017-07-26 18:48 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Cong Wang, Roopa Prabhu
In-Reply-To: <07c66e6f-5b78-3317-18c6-bd2f955d5f90@gmail.com>
On 7/26/17 11:09 AM, David Ahern wrote:
> I don't understand why you are focused on the built-in null and prohibit
> route entries.
I see. You are using fib rules for the prohibit entry; I am using an
explicit route entry.
If I run 'ip ro get fibmatch' for the latter I want to see that route
entry since it is a route in the FIB:
# ip -6 ro get fibmatch vrf red 5000::1
prohibit 5000::/120 dev lo table red metric 1024 error -13 pref medium
So there are multiple cases to verify.
^ permalink raw reply
* Re: TCP fast retransmit issues
From: Neal Cardwell @ 2017-07-26 18:38 UTC (permalink / raw)
To: Willy Tarreau
Cc: Eric Dumazet, Klavs Klavsen, Netdev, Yuchung Cheng,
Nandita Dukkipati
In-Reply-To: <CADVnQykEX-LzhsqyfHNmV6fF9-d_2R0v+mOX-PnyagUhC1qQzQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]
On Wed, Jul 26, 2017 at 1:06 PM, Neal Cardwell <ncardwell@google.com> wrote:
> On Wed, Jul 26, 2017 at 12:43 PM, Neal Cardwell <ncardwell@google.com> wrote:
>> (2) It looks like there is a bug in the sender code where it seems to
>> be repeatedly using a TLP timer firing 211ms after every ACK is
>> received to transmit another TLP probe (a new packet in this case).
>> Somehow these weird invalid SACKs seem to be triggering a code path
>> that makes us think we can send another TLP, when we probably should
>> be firing an RTO. That's my interpretation, anyway. I will try to
>> reproduce this with packetdrill.
>
> Hmm. It looks like this might be a general issue, where any time we
> get an ACK that doesn't ACK/SACK anything new (whether because it's
> incoming data in a bi-directional flow, or a middlebox breaking the
> SACKs), then we schedule a TLP timer further out in time. Probably we
> should only push the TLP timer out if something is cumulatively ACKed.
>
> But that's not a trivial thing to do, because by the time we are
> deciding whether to schedule another TLP, we have already canceled the
> previous TLP and reinstalled an RTO. Hmm.
Yeah, it looks like I can reproduce this issue with (1) bad sacks
causing repeated TLPs, and (2) TLPs timers being pushed out to later
times due to incoming data. Scripts are attached.
neal
[-- Attachment #2: tlp-bad-sacks.pkt --]
[-- Type: application/octet-stream, Size: 1665 bytes --]
// Test for TLP behavior when all SACKs that come back are invalid
// (e.g. because of a middlebox).
// (Oops... it seems invalid SACKs can cause us to send TLPs forever.)
// Set up production config.
`../common/defaults.sh`
// Establish a connection.
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
+.020 < . 1:1(0) ack 1 win 257
+0 accept(3, ..., ...) = 4
// Send 10 MSS.
+0 write(4, ..., 20000) = 20000
+0 > . 1:10001(10000) ack 1
// First round:
// An ACK arrives with a bogus SACK.
+.020 < . 1:1(0) ack 1 win 257 <sack 1000000001:1000001001,nop,nop>
// At 2*RTT, send a TLP loss probe that is a new packet.
+.040 > . 10001:11001(1000) ack 1
+0 %{
assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd
assert tcpi_unacked == 11, tcpi_unacked
}%
// Second round (same as the first):
// An ACK arrives with another bogus SACK.
+.020 < . 1:1(0) ack 1 win 257 <sack 1000000001:1000002001,nop,nop>
// At 2*RTT, send a TLP loss probe that is a new packet.
+.040 > . 11001:12001(1000) ack 1
+0 %{
assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd
assert tcpi_unacked == 12, tcpi_unacked
}%
// Third round (same as the first):
// An ACK arrives with another bogus SACK.
+.020 < . 1:1(0) ack 1 win 257 <sack 1000000001:1000003001,nop,nop>
// At 2*RTT, send a TLP loss probe that is a new packet.
+.040 > . 12001:13001(1000) ack 1
+0 %{
assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd
assert tcpi_unacked == 13, tcpi_unacked
}%
[-- Attachment #3: tlp-bidirectional.pkt --]
[-- Type: application/octet-stream, Size: 1272 bytes --]
// Test for TLP behavior with bi-directional traffic (incoming data).
// Make sure that incoming data does not push back the TLP timer.
// (Oops... currently it does seem that incoming data pushes back
// the TLP timer.)
// Set up production config.
`../common/defaults.sh`
// Establish a connection.
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
+.020 < . 1:1(0) ack 1 win 257
+0 accept(3, ..., ...) = 4
// Send 10 MSS.
+0 write(4, ..., 20000) = 20000
+0 > . 1:10001(10000) ack 1
// Incoming data arrives
+.010 < . 1:1001(1000) ack 1 win 257
+0 > . 10001:10001(0) ack 1001
+.010 < . 1001:2001(1000) ack 1 win 257
+0 > . 10001:10001(0) ack 2001
+.010 < . 2001:3001(1000) ack 1 win 257
+0 > . 10001:10001(0) ack 3001
+.010 < . 3001:4001(1000) ack 1 win 257
+0 > . 10001:10001(0) ack 4001
// At 2*RTT after the last transmit, send a TLP loss probe
// that is a new packet.
+.032 > . 10001:11001(1000) ack 4001
+0 %{
assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd
assert tcpi_unacked == 11, tcpi_unacked
}%
^ permalink raw reply
* Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings
From: Franklin S Cooper Jr @ 2017-07-26 18:29 UTC (permalink / raw)
To: Oliver Hartkopp, Andrew Lunn
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-can-u79uwXL29TY76Z2rM5mHXA, wg-5Yr1BZd7O62+XT7JhA+gdA,
mkl-bIcnvbaLZ9MEGnE8C9+IrQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
dev.kurt-yI9piX4KPfawT/RRk36CISFp6vIno51x,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8
In-Reply-To: <355b90b3-97ce-1057-6617-d5d709449c48-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
On 07/26/2017 12:05 PM, Oliver Hartkopp wrote:
> On 07/26/2017 06:41 PM, Andrew Lunn wrote:
>> On Mon, Jul 24, 2017 at 06:05:19PM -0500, Franklin S Cooper Jr wrote:
>
>>> +
>>> +Optional:
>>> + max-arbitration-speed: a positive non 0 value that determines the max
>>> + speed that CAN can run in non CAN-FD mode or during the
>>> + arbitration phase in CAN-FD mode.
>>
>> Hi Franklin
>>
>> Since this and the next property are optional, it is good to document
>> what happens when they are not in the DT blob. Also document what 0
>> means.
The driver ignores values less than 0 with the exception being
max-data-speed which supports a value of -1. Not sure what I'm
documenting when the binding specifically says to use a positive non
zero value. Its the same reason I don't document what happens if you
give it a negative value.
>>
>>> +
>>> + max-data-speed: a positive non 0 value that determines the max
>>> data rate
>>> + that can be used in CAN-FD mode. A value of -1 implies
>>> + CAN-FD is not supported by the transceiver.
>>
>> -1 is ugly. I think it would be better to have a missing
>> max-data-speed property indicate that CAN-FD is not supported.
>
Although this leads to your later point I don't think this is the right
approach. Its an optional property. Not including the property should
not assume it isn't supported.
> Thanks Andrew! I had the same feeling about '-1' :-)
Ok I'll go back to having 0 = not supported. Which will handle the
documentation comment above.
>
>> And
>> maybe put 'fd' into the property name.
>
> Good point. In fact the common naming to set bitrates for CAN(FD)
> controllers are 'bitrate' and 'data bitrate'.
>
> 'speed' is not really a good word for that.
I'm fine with switching to using bitrate instead of speed. Kurk was
originally the one that suggested to use the term arbitration and data
since thats how the spec refers to it. Which I do agree with. But your
right that in the drivers (struct can_priv) we just use bittiming and
data_bittiming (CAN-FD timings). I don't think adding "fd" into the
property name makes sense unless we are calling it something like
"max-canfd-bitrate" which I would agree is the easiest to understand.
So what is the preference if we end up sticking with two properties?
Option 1 or 2?
1)
max-bitrate
max-data-bitrate
2)
max-bitrate
max-canfd-bitrate
>
> Finally, @Franklin:
>
> A CAN transceiver is limited in bandwidth. But you only have one RX and
> one TX line between the CAN controller and the CAN transceiver. The
> transceiver does not know about CAN FD - it has just a physical(!) layer
> with a limited bandwidth. This is ONE limitation.
>
> So I tend to specify only ONE 'max-bitrate' property for the
> fixed-transceiver binding.
>
> The fact whether the CAN controller is CAN FD capable or not is provided
> by the netlink configuration interface for CAN controllers.
Part of the reasoning to have two properties is to indicate that you
don't support CAN FD while limiting the "arbitration" bit rate. With one
property you can not determine this and end up having to make some
assumptions that can quickly end up biting people.
>
> Regards,
> Oliver
>
--
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
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