Netdev List
 help / color / mirror / Atom feed
* Re: [patch] net/mlx5: remove a duplicate condition
From: Saeed Mahameed @ 2016-11-24 15:30 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Saeed Mahameed, Matan Barak, Leon Romanovsky, Linux Netdev List,
	linux-rdma, linux-kernel, kernel-janitors
In-Reply-To: <20161124110345.GC17225@mwanda>

On Thu, Nov 24, 2016 at 1:03 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> We verified that MLX5_FLOW_CONTEXT_ACTION_COUNT was set on the first
> line of the function so we don't need to check again here.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply

* Re: [PATCH v2] cpsw: ethtool: add support for getting/setting EEE registers
From: Andrew Lunn @ 2016-11-24 15:38 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: Florian Fainelli, netdev, linux-omap@vger.kernel.org,
	Grygorii Strashko, N, Mugunthan V, Rami Rosen
In-Reply-To: <CAGm1_ktuAbt0pr_NJu2GLDOavFV_bvMBmH5RvDxr5AyXvfwB3A@mail.gmail.com>

> As for enabling advertising and correct working of cpsw do you mean it
> would be better to disable EEE in any PHY on cpsw initialization as
> long as cpsw doesn't provide support for EEE?
> 
> We observe some strange behavior with our gigabit PHYs and a link
> partner in a EEE-capable unmanaged NetGear switch. Disabling
> advertising seems to help. Though we're still investigating the issue.

Hi Florian

Am i right in saying, a PHY should not advertise EEE until the MAC
driver calls phy_init_eee(), indicating the MAC supports EEE?

If so, it looks like we need to change a few of the PHY drivers, in
particular, the bcm-*.c.

    Andrew

^ permalink raw reply

* [PATCH net 1/1] driver: ipvlan: Fix one possible memleak in ipvlan_link_new
From: fgao @ 2016-11-24 15:39 UTC (permalink / raw)
  To: davem, maheshb, edumazet, netdev, gfree.wind; +Cc: Gao Feng

From: Gao Feng <fgao@ikuai8.com>

When ipvlan_link_new fails and creates one ipvlan port, it does not
destroy the ipvlan port created. It causes mem leak and the physical
device contains invalid ipvlan data.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
 drivers/net/ipvlan/ipvlan_main.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index f442eb3..0fef178 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -497,6 +497,7 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 	struct net_device *phy_dev;
 	int err;
 	u16 mode = IPVLAN_MODE_L3;
+	bool create = false;
 
 	if (!tb[IFLA_LINK])
 		return -EINVAL;
@@ -513,6 +514,7 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 		err = ipvlan_port_create(phy_dev);
 		if (err < 0)
 			return err;
+		create = true;
 	}
 
 	if (data && data[IFLA_IPVLAN_MODE])
@@ -536,22 +538,27 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 
 	err = register_netdevice(dev);
 	if (err < 0)
-		return err;
+		goto destroy_ipvlan_port;
 
 	err = netdev_upper_dev_link(phy_dev, dev);
 	if (err) {
-		unregister_netdevice(dev);
-		return err;
+		goto unregister_netdev;
 	}
 	err = ipvlan_set_port_mode(port, mode);
 	if (err) {
-		unregister_netdevice(dev);
-		return err;
+		goto unregister_netdev;
 	}
 
 	list_add_tail_rcu(&ipvlan->pnode, &port->ipvlans);
 	netif_stacked_transfer_operstate(phy_dev, dev);
 	return 0;
+
+unregister_netdev:
+	unregister_netdevice(dev);
+destroy_ipvlan_port:
+	if (create)
+		ipvlan_port_destroy(phy_dev);
+	return err;
 }
 
 static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
-- 
1.9.1

^ permalink raw reply related

* Re: [net-next PATCH v1 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Andrew Lunn @ 2016-11-24 15:48 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: linux-amlogic, devicetree, netdev, davem, khilman, mark.rutland,
	robh+dt, linux-arm-kernel, alexandre.torgue, peppe.cavallaro,
	carlo, jbrunet
In-Reply-To: <20161124143417.10178-2-martin.blumenstingl@googlemail.com>

> The configuration values are provided as preprocessor macros to make the
> devicetree files easier to read.

Hi Martin

If i'm reading the code/comments correctly, you can set the delay to
0, 2, 4 or 6ns? So calling this property amlogic,tx-delay-ns would be
even easier to read.

     Andrew

^ permalink raw reply

* Re: [PATCH net-next 1/2] openvswitch: Add a missing break statement.
From: Jiri Benc @ 2016-11-24 15:55 UTC (permalink / raw)
  To: Jarno Rajahalme; +Cc: netdev
In-Reply-To: <1479874174-75329-1-git-send-email-jarno@ovn.org>

On Tue, 22 Nov 2016 20:09:33 -0800, Jarno Rajahalme wrote:
> Add a break statement to prevent fall-through from
> OVS_KEY_ATTR_ETHERNET to OVS_KEY_ATTR_TUNNEL.  Without the break
> actions setting ethernet addresses fail to validate with log messages
> complaining about invalid tunnel attributes.
> 
> Fixes: 0a6410fbde ("openvswitch: netlink: support L3 packets")
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

Acked-by: Jiri Benc <jbenc@redhat.com>

It's a good practice to CC the one who messed up :-) Please do that
next time.

Thanks for noticing the bug and fixing it.

 Jiri

^ permalink raw reply

* Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Jerome Brunet @ 2016-11-24 15:56 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, devicetree, netdev, davem,
	khilman, mark.rutland, robh+dt
  Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, carlo
In-Reply-To: <20161124143417.10178-1-martin.blumenstingl@googlemail.com>

On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
> 
> This raises a question though:
> Which device is supposed to enable the TX delay when both MAC and PHY
> support it? And should we implement it for each PHY / MAC separately
> or should we think about a more generic solution (currently it's not
> possible to disable the TX delay generated by the RTL8211F PHY via
> devicetree when using phy-mode "rgmii")?

Actually you can skip the part which activate the Tx-delay on the phy
by setting "phy-mode = "rgmii-id" instead of "rgmii"

phy->interface will no longer be PHY_INTERFACE_MODE_RGMII
but PHY_INTERFACE_MODE_RGMII_ID.

> 
> iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
> TX clock delay disabled on the MAC (as it's enabled in the PHY
> driver).
> TX throughput was virtually zero before:
> $ iperf3 -c 192.168.1.100 -R          
> Connecting to host 192.168.1.100, port 5201
> Reverse mode, remote host 192.168.1.100 is sending
> [  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth
> [  4]   0.00-1.00   sec   108 MBytes   901
> Mbits/sec                  
> [  4]   1.00-2.00   sec  94.2 MBytes   791
> Mbits/sec                  
> [  4]   2.00-3.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   3.00-4.00   sec  96.2 MBytes   808
> Mbits/sec                  
> [  4]   4.00-5.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   5.00-6.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   6.00-7.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   7.00-8.00   sec  96.5 MBytes   809
> Mbits/sec                  
> [  4]   8.00-9.00   sec   105 MBytes   884
> Mbits/sec                  
> [  4]   9.00-10.00  sec   111 MBytes   934
> Mbits/sec                  
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.00  sec  1000 MBytes   839
> Mbits/sec    0             sender
> [  4]   0.00-10.00  sec   998 MBytes   837
> Mbits/sec                  receiver
> 
> iperf Done.
> $ iperf3 -c 192.168.1.100   
> Connecting to host 192.168.1.100, port 5201
> [  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
> [  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139
> KBytes       
> [  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7
> KBytes       
> [  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187
> KBytes       
> [  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143
> KBytes       
> [  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129
> KBytes       
> [  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195
> KBytes       
> [  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7
> KBytes       
> [  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6
> KBytes       
> [  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107
> KBytes       
> [  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139
> KBytes       
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.01  sec  1.02 GBytes   874
> Mbits/sec  1189             sender
> [  4]   0.00-10.01  sec  1.02 GBytes   873
> Mbits/sec                  receiver
> 
> iperf Done.

> 
> 
> Martin Blumenstingl (2):
>   net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
>   net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
> 
>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 11
> +++++++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16
> +++++++++++-----
>  include/dt-bindings/net/dwmac-meson8b.h               | 18
> ++++++++++++++++++
>  3 files changed, 40 insertions(+), 5 deletions(-)
>  create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
> 

^ permalink raw reply

* Re: [RFC PATCH net v2 0/3] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-24 16:01 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Florian Fainelli, Carlo Caione, Kevin Hilman, Giuseppe Cavallaro,
	Alexandre TORGUE, Andre Roth, Neil Armstrong,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAFBinCCexmS_z9FCX-ud5NgGhhP7xJ_cLxpC7TEc=mLAdafosg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, 2016-11-24 at 15:40 +0100, Martin Blumenstingl wrote:
> Hi Jerome,
> 
> On Mon, Nov 21, 2016 at 4:35 PM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> wrote:
> > 
> > This patchset fixes an issue with the OdroidC2 board (DWMAC +
> > RTL8211F).
> > Initially reported as a low Tx throughput issue at gigabit speed,
> > the
> > platform enters LPI too often. 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.
> I have just sent a series which allows configuring the TX delay on
> the
> MAC (dwmac-meson8b glue) side: [0]
> Disabling the TX delay generated by the MAC fixes TX throughput for
> me, even when leaving EEE enabled in the RTL8211F PHY driver!
> 
> Unfortunately the RTL8211F PHY is a black-box for the community
> because there is no public datasheeet available.
> *maybe* (pure speculation!) they're enabling the TX delay based on
> some internal magic only when EEE is enabled.

Hi already tried acting on the register setting the TX_delay. I also
tried on the PHY. I never been able to improve situation on the
Odroic2. Only disabling EEE improved the situation.

To make sure, i tried again with your patch but the result remains
unchanged. With Tx_delay disabled (either the mac or the phy), the
situation is even worse, it seems that nothing gets through

> 
> Jerome, could you please re-test the behavior on your Odroid-C2 when
> you have EEE still enabled but the TX-delay disabled?
> In my case throughput is fine, and "$ ethtool -S eth0 | grep lpi"
> gives:
>     irq_tx_path_in_lpi_mode_n: 0
>     irq_tx_path_exit_lpi_mode_n: 0
>     irq_rx_path_in_lpi_mode_n: 0
>     irq_rx_path_exit_lpi_mode_n: 0
> 

I still have lpi interrupts on my side. I don't get how a properly
configured tx_delay would disable EEE. I must be missing something
here.

> 
> Regards,
> Martin
> 
> 
> [0] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/
> 001674.html
--
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: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.
From: David Miller @ 2016-11-24 16:04 UTC (permalink / raw)
  To: pavel; +Cc: peppe.cavallaro, netdev, linux-kernel
In-Reply-To: <20161124085506.GA25007@amd>

From: Pavel Machek <pavel@ucw.cz>
Date: Thu, 24 Nov 2016 09:55:06 +0100

> Hi!
> 
>> I'm debugging strange delays during transmit in stmmac driver. They
>> seem to be present in 4.4 kernel (and older kernels, too). Workload is
>> burst of udp packets being sent, pause, burst of udp packets, ...
>> 
>> Test code is attached, I use these parameters for testing:
>> 
>> ./udp-test raw 10.0.0.6 1234 1000 100 30
>> 
>> The delays seem to be related to coalescing:
>> 
>> drivers/net/ethernet/stmicro/stmmac/common.h
>> #define STMMAC_COAL_TX_TIMER    40000
>> #define STMMAC_MAX_COAL_TX_TICK 100000
>> #define STMMAC_TX_MAX_FRAMES    256
>> 
>> If I lower the parameters, delays are gone, but I get netdev watchdog
>> backtrace followed by broken driver.
>> 
>> Any ideas what is going on there?
> 
> 4.9-rc6 still has the delays. With the
> 
> #define STMMAC_COAL_TX_TIMER 1000
> #define STMMAC_TX_MAX_FRAMES 2
> 
> settings, delays go away, and driver still works. (It fails fairly
> fast in 4.4). Good news. But the question still is: what is going on
> there?

256 packets looks way too large for being a trigger for aborting the
TX coalescing timer.

Looking more deeply into this, the driver is using non-highres timers
to implement the TX coalescing.  This simply cannot work.

1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing.

I seriously think that the TX coalescing support should be ripped out
or disabled entirely until it is implemented properly in this driver.

^ permalink raw reply

* Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Jerome Brunet @ 2016-11-24 16:08 UTC (permalink / raw)
  To: Martin Blumenstingl, linux-amlogic, devicetree, netdev, davem,
	khilman, mark.rutland, robh+dt
  Cc: linux-arm-kernel, alexandre.torgue, peppe.cavallaro, carlo
In-Reply-To: <20161124143417.10178-1-martin.blumenstingl@googlemail.com>

On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
> 
> This raises a question though:
> Which device is supposed to enable the TX delay when both MAC and PHY
> support it? And should we implement it for each PHY / MAC separately
> or should we think about a more generic solution (currently it's not
> possible to disable the TX delay generated by the RTL8211F PHY via
> devicetree when using phy-mode "rgmii")?
> 
> iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
> TX clock delay disabled on the MAC (as it's enabled in the PHY
> driver).
> TX throughput was virtually zero before:
> $ iperf3 -c 192.168.1.100 -R          
> Connecting to host 192.168.1.100, port 5201
> Reverse mode, remote host 192.168.1.100 is sending
> [  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth
> [  4]   0.00-1.00   sec   108 MBytes   901
> Mbits/sec                  
> [  4]   1.00-2.00   sec  94.2 MBytes   791
> Mbits/sec                  
> [  4]   2.00-3.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   3.00-4.00   sec  96.2 MBytes   808
> Mbits/sec                  
> [  4]   4.00-5.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   5.00-6.00   sec  96.5 MBytes   810
> Mbits/sec                  
> [  4]   6.00-7.00   sec  96.6 MBytes   810
> Mbits/sec                  
> [  4]   7.00-8.00   sec  96.5 MBytes   809
> Mbits/sec                  
> [  4]   8.00-9.00   sec   105 MBytes   884
> Mbits/sec                  
> [  4]   9.00-10.00  sec   111 MBytes   934
> Mbits/sec                  
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.00  sec  1000 MBytes   839
> Mbits/sec    0             sender
> [  4]   0.00-10.00  sec   998 MBytes   837
> Mbits/sec                  receiver
> 
> iperf Done.
> $ iperf3 -c 192.168.1.100   
> Connecting to host 192.168.1.100, port 5201
> [  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port
> 5201
> [ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
> [  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117    139
> KBytes       
> [  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7
> KBytes       
> [  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106    187
> KBytes       
> [  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92    143
> KBytes       
> [  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140    129
> KBytes       
> [  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115    195
> KBytes       
> [  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7
> KBytes       
> [  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6
> KBytes       
> [  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124    107
> KBytes       
> [  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90    139
> KBytes       
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bandwidth       Retr
> [  4]   0.00-10.01  sec  1.02 GBytes   874
> Mbits/sec  1189             sender
> [  4]   0.00-10.01  sec  1.02 GBytes   873
> Mbits/sec                  receiver
> 

Cool, one more board working ;)
I tried your patch on another board (MXQ_V2.1, with sheep printed on
the PCB). It 's not improving the situation for this one unfortunately.
Actually I already tried playing with the TX delay on the MAC and PHY
but I could get any good results with the boards I have.

It is strange that we can adjust the delay by 2ns steps, when delay
seens by the phy should be 2ns ...

> iperf Done.
> 
> 
> Martin Blumenstingl (2):
>   net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
>   net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable
> 
>  Documentation/devicetree/bindings/net/meson-dwmac.txt | 11
> +++++++++++
>  drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16
> +++++++++++-----
>  include/dt-bindings/net/dwmac-meson8b.h               | 18
> ++++++++++++++++++
>  3 files changed, 40 insertions(+), 5 deletions(-)
>  create mode 100644 include/dt-bindings/net/dwmac-meson8b.h
> 

^ permalink raw reply

* Re: wl1251 & mac address & calibration data
From: Sebastian Reichel @ 2016-11-24 16:08 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
	Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
	linux-kernel
In-Reply-To: <20161124152045.GK13735@pali>

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

Hi,

On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote:
> On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > > > > "ifconfig hw ether XX" normally sets the address. I guess that's
> > > > > > ioctl?
> > > > > 
> > > > > This sets temporary address and it is ioctl. IIRC same as what ethtool 
> > > > > uses. (ifconfig is already deprecated).
> > > > > 
> > > > > > And I guess we should use similar mechanism for permanent
> > > > > > address.
> > > > > 
> > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac 
> > > > > address. But here we do not want to change permanent mac address. We 
> > > > > want to tell kernel driver current permanent mac address which is
> > > > > stored
> > > > 
> > > > Well... I'd still use similar mechanism :-).
> > > 
> > > Thats problematic, because in time when wlan0 interface is registered
> > > into system and visible in ifconfig output it already needs to have
> > > permanent mac address assigned.
> > > 
> > > We should assign permanent mac address before wlan0 of wl1251 is
> > > registered into system.
> > 
> > You can just add the MAC address to the NVS data, which is also
> > required for the device initialization.
> 
> NVS data file has fixed size, there is IIRC no place for it.
> 
> But one of my suggestion was to use another request_firmware for MAC
> address. So this is similar to what you are proposing, as NVS data are
> loaded by request_firmware too...

Just append it to NVS data and modify the size check accordingly?

> > I wonder if those information could be put into DT. Iirc some
> > network devices get their MAC address from DT. Maybe we can add
> > all NVS info to DT? How much data is it?
> 
> Proprietary, signed and closed bootloader NOLO does not support DT. So
> for booting you need to append DTS file to kernel image.

Yeah, so NOLO without U-Boot will depend on userspace to fixup DT.

> U-Boot is optional and can be used as intermediate bootloader between
> NOLO and kernel. But still it has problems with reading from nand, so
> cannot read NVS data nor MAC address.

It may in the future?

> > Userspace application can add all those information to the DT
> > using a DT overlay. Also the u-boot could parse and add it at
> > some point in the future.
> 
> In case when wl1251 is statically linked into kernel image, it is loaded
> and initialized before even userspace applications starts.
>
> So no... adding NVS data or MAC address into DT or DT overlay is not a
> solution.

Actually with data loaded from DT you *can* load data quite early in
the boot process, while your suggestions always require userspace.
So you argument against yourself?

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH] net/iucv: use explicit clean up labels in iucv_init()
From: Sebastian Andrzej Siewior @ 2016-11-24 16:10 UTC (permalink / raw)
  To: Ursula Braun, tglx; +Cc: linux-kernel, rt, David S. Miller, linux-s390, netdev
In-Reply-To: <20161124141415.llqki5vo7nqdusqv@linutronix.de>

Ursula suggested to use explicit labels for clean up in the error path
instead of one `out_free' label which handles multiple exits.
Since the previous patch got already applied, here is a follow up patch.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 net/iucv/iucv.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c
index f0d6afc5d4a9..8f7ef167c45a 100644
--- a/net/iucv/iucv.c
+++ b/net/iucv/iucv.c
@@ -2038,16 +2038,16 @@ static int __init iucv_init(void)
 	rc = cpuhp_setup_state(CPUHP_NET_IUCV_PREPARE, "net/iucv:prepare",
 			       iucv_cpu_prepare, iucv_cpu_dead);
 	if (rc)
-		goto out_free;
+		goto out_dev;
 	rc = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "net/iucv:online",
 			       iucv_cpu_online, iucv_cpu_down_prep);
 	if (rc < 0)
-		goto out_free;
+		goto out_prep;
 	iucv_online = rc;
 
 	rc = register_reboot_notifier(&iucv_reboot_notifier);
 	if (rc)
-		goto out_free;
+		goto out_remove_hp;
 	ASCEBC(iucv_error_no_listener, 16);
 	ASCEBC(iucv_error_no_memory, 16);
 	ASCEBC(iucv_error_pathid, 16);
@@ -2061,11 +2061,11 @@ static int __init iucv_init(void)
 
 out_reboot:
 	unregister_reboot_notifier(&iucv_reboot_notifier);
-out_free:
-	if (iucv_online)
-		cpuhp_remove_state(iucv_online);
+out_remove_hp:
+	cpuhp_remove_state(iucv_online);
+out_prep:
 	cpuhp_remove_state(CPUHP_NET_IUCV_PREPARE);
-
+out_dev:
 	root_device_unregister(iucv_root);
 out_int:
 	unregister_external_irq(EXT_IRQ_IUCV, iucv_external_interrupt);
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH net-next 2/2] openvswitch: Fix skb->protocol for vlan frames.
From: Jiri Benc @ 2016-11-24 16:10 UTC (permalink / raw)
  To: Jarno Rajahalme; +Cc: netdev
In-Reply-To: <1479874174-75329-2-git-send-email-jarno@ovn.org>

On Tue, 22 Nov 2016 20:09:34 -0800, Jarno Rajahalme wrote:
> Do not set skb->protocol to be the ethertype of the L3 header, unless
> the packet only has the L3 header.  For a non-hardware offloaded VLAN
> frame skb->protocol needs to be one of the VLAN ethertypes.
> 
> Any VLAN offloading is undone on the OVS netlink interface.  Due to
> this all VLAN packets sent to openvswitch module from userspace are
> non-offloaded.

This is exactly why I wanted to always accelerate the vlan tag, the
same way it is done in other parts of the networking stack: to prevent
all those weird corner cases.

Looks to me this is the only real way forward.

This patch is wrong, it would leave skb->protocol as ETH_P_TEB for L2
frames received via ARPHRD_NONE interface.

 Jiri

^ permalink raw reply

* [PATCH v2 1/2] can: rcar_can: Add r8a7796 support
From: Chris Paterson @ 2016-11-24 16:11 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Wolfgang Grandegger, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Simon Horman, Ramesh Shanmugasundaram,
	linux-can-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Chris Paterson
In-Reply-To: <1480003917-3953-1-git-send-email-chris.paterson2-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

Signed-off-by: Chris Paterson <chris.paterson2-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/net/can/rcar_can.txt | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt
index 8d40ab2..06bb7cc 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
@@ -10,6 +10,7 @@ Required properties:
 	      "renesas,can-r8a7793" if CAN controller is a part of R8A7793 SoC.
 	      "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
 	      "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
+	      "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
 	      "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device.
 	      "renesas,rcar-gen2-can" for a generic R-Car Gen2 compatible device.
 	      "renesas,rcar-gen3-can" for a generic R-Car Gen3 compatible device.
@@ -24,11 +25,12 @@ Required properties:
 - pinctrl-0: pin control group to be used for this controller.
 - pinctrl-names: must be "default".
 
-Required properties for "renesas,can-r8a7795" compatible:
-In R8A7795 SoC, "clkp2" can be CANFD clock. This is a div6 clock and can be
-used by both CAN and CAN FD controller at the same time. It needs to be scaled
-to maximum frequency if any of these controllers use it. This is done using
-the below properties.
+Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
+compatible:
+In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
+and can be used by both CAN and CAN FD controller at the same time. It needs to
+be scaled to maximum frequency if any of these controllers use it. This is done
+using the below properties:
 
 - assigned-clocks: phandle of clkp2(CANFD) clock.
 - assigned-clock-rates: maximum frequency of this clock.
-- 
1.9.1

--
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 v2 2/2] can: rcar_canfd: Add r8a7796 support
From: Chris Paterson @ 2016-11-24 16:11 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Wolfgang Grandegger, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Simon Horman, Ramesh Shanmugasundaram,
	linux-can, netdev, devicetree, linux-renesas-soc, Chris Paterson
In-Reply-To: <1480003917-3953-1-git-send-email-chris.paterson2@renesas.com>

Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 22a6f10..788f273 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: Must contain one or more of the following:
   - "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
   - "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible controller.
+  - "renesas,r8a7796-canfd" for R8A7796 (R-Car M3) compatible controller.
 
   When compatible with the generic version, nodes must list the
   SoC-specific version corresponding to the platform first, followed by the
@@ -23,11 +24,12 @@ The name of the child nodes are "channel0" and "channel1" respectively. Each
 child node supports the "status" property only, which is used to
 enable/disable the respective channel.
 
-Required properties for "renesas,r8a7795-canfd" compatible:
-In R8A7795 SoC, canfd clock is a div6 clock and can be used by both CAN
-and CAN FD controller at the same time. It needs to be scaled to maximum
-frequency if any of these controllers use it. This is done using the
-below properties.
+Required properties for "renesas,r8a7795-canfd" and "renesas,r8a7796-canfd"
+compatible:
+In R8A7795 and R8A7796 SoCs, canfd clock is a div6 clock and can be used by both
+CAN and CAN FD controller at the same time. It needs to be scaled to maximum
+frequency if any of these controllers use it. This is done using the below
+properties:
 
 - assigned-clocks: phandle of canfd clock.
 - assigned-clock-rates: maximum frequency of this clock.
-- 
1.9.1

^ permalink raw reply related

* [PATCH v2 0/2] can: Add r8a7796 support to CAN/CAN FD
From: Chris Paterson @ 2016-11-24 16:11 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Wolfgang Grandegger, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Simon Horman, Ramesh Shanmugasundaram,
	linux-can, netdev, devicetree, linux-renesas-soc, Chris Paterson
In-Reply-To: <1479903243-1860-1-git-send-email-chris.paterson2@renesas.com>

This series adds support for r8a7796 to rcar_can and rcar_canfd.

Original series: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support.

Changes since v1:
- Split bindings documentation changes from device tree changes.
- Rebased on renesas-devel-20161123v2-v4.9-rc6.


Chris Paterson (2):
  can: rcar_can: Add r8a7796 support
  can: rcar_canfd: Add r8a7796 support

 Documentation/devicetree/bindings/net/can/rcar_can.txt   | 12 +++++++-----
 Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 12 +++++++-----
 2 files changed, 14 insertions(+), 10 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver
From: Jason Gunthorpe @ 2016-11-24 16:15 UTC (permalink / raw)
  To: Vishwanathapura, Niranjana
  Cc: ira.weiny, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Dennis Dalessandro
In-Reply-To: <20161124000825.GA73280-wPcXA7LoDC+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

On Wed, Nov 23, 2016 at 04:08:25PM -0800, Vishwanathapura, Niranjana wrote:

> In order to pass the hfi function pointers to the hfi_vnic ULP, I can,
> a) Have hfi_vnic ULP define an interface API for hfi1 driver to call to
> register its callback (as you pointed). Unfortunately there will be a module
> dependency here.
> Or,

That is probably backwards

> b) Add a new member ‘struct vnic_ops’ either to the ib_device structure or
> ib_port_immutable structure. As it is hfi1 specific, only hfi1 driver will
> set it. No module dependency here.

You can add a hfi1_get_vnic_ops(struct ib_device *) and implement it
in your module..

> And will move the hfi_vnic module under
> ‘drivers/infiniband/ulp/hfi_vnic’.

I would prefer drivers/net/ethernet

This is clearly not a ULP since it doesn't use verbs.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 net 1/2] r8152: fix the sw rx checksum is unavailable
From: David Miller @ 2016-11-24 16:19 UTC (permalink / raw)
  To: mlord-e+AXbWqSrlAAvxtiuMwx3w
  Cc: hayeswang-Rasf1IRRPZFBDgjK7y7TUQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <baf5246d-9d8a-4029-6823-350ed561fd33-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

From: Mark Lord <mlord-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>
Date: Thu, 24 Nov 2016 07:31:17 -0500

> Any way we look at it though, the chip/driver are simply unreliable,
> and relying upon hardware checksums (which fail due to the driver
> looking at garbage rather than the checksum bits) leads to data
> corruption.

If the cpu/DMA implementation is the problem, then turning off
checksums is not an appropriate fix at all.

In fact, we have no idea what the cause is yet.

That makes turning off random features no more than grasping at straws
and makes no sense at all upstream.

It may make sense for you to do such a change locally in _your_ tree
to fix your situation temporarily.  But upstream we shouldn't be doing
it.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: David Miller @ 2016-11-24 16:21 UTC (permalink / raw)
  To: hayeswang; +Cc: mlord, netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB201055ED8@RTITMBSV03.realtek.com.tw>

From: Hayes Wang <hayeswang@realtek.com>
Date: Thu, 24 Nov 2016 13:26:55 +0000

> I don't think the garbage results from our driver or device.

This is my impression with what has been presented so far as well.

^ permalink raw reply

* [PATCH] irda: fix overly long udelay()
From: Arnd Bergmann @ 2016-11-24 16:26 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Arnd Bergmann, netdev, linux-kernel

irda_get_mtt() returns a hardcoded '10000' in some cases,
and with gcc-7, we get a build error because this triggers a
compile-time check in udelay():

drivers/net/irda/w83977af_ir.o: In function `w83977af_hard_xmit':
w83977af_ir.c:(.text.w83977af_hard_xmit+0x14c): undefined reference to `__bad_udelay'

Older compilers did not run into this because they either did not
completely inline the irda_get_mtt() or did not consider the
10000 value a constant expression.

The code has been wrong since the start of git history.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/irda/w83977af_ir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 4e3d2e7c697c..e8c3a8c32534 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -518,7 +518,9 @@ static netdev_tx_t w83977af_hard_xmit(struct sk_buff *skb,
 		
 		mtt = irda_get_mtt(skb);
 		pr_debug("%s(%ld), mtt=%d\n", __func__ , jiffies, mtt);
-			if (mtt)
+			if (mtt > 1000)
+				mdelay(mtt/1000);
+			else if (mtt)
 				udelay(mtt);
 
 			/* Enable DMA interrupt */
-- 
2.9.0

^ permalink raw reply related

* [PATCH net]  mvpp2: use correct size for memset
From: Arnd Bergmann @ 2016-11-24 16:28 UTC (permalink / raw)
  To: David S. Miller
  Cc: Arnd Bergmann, Jisheng Zhang, Marcin Wojtas, Philippe Reynes,
	netdev, linux-kernel

gcc-7 detects a short memset in mvpp2, introduced in the original
merge of the driver:

drivers/net/ethernet/marvell/mvpp2.c: In function 'mvpp2_cls_init':
drivers/net/ethernet/marvell/mvpp2.c:3296:2: error: 'memset' used with length equal to number of elements without multiplication by element size [-Werror=memset-elt-size]

The result seems to be that we write uninitialized data into the
flow table registers, although we did not get any warning about
that uninitialized data usage.

Using sizeof() lets us initialize then entire array instead.

Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/marvell/mvpp2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index ee857868bea5..dabc5418efcc 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -3293,7 +3293,7 @@ static void mvpp2_cls_init(struct mvpp2 *priv)
 	mvpp2_write(priv, MVPP2_CLS_MODE_REG, MVPP2_CLS_MODE_ACTIVE_MASK);
 
 	/* Clear classifier flow table */
-	memset(&fe.data, 0, MVPP2_CLS_FLOWS_TBL_DATA_WORDS);
+	memset(&fe.data, 0, sizeof(fe.data));
 	for (index = 0; index < MVPP2_CLS_FLOWS_TBL_SIZE; index++) {
 		fe.index = index;
 		mvpp2_cls_flow_write(priv, &fe);
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH 1/2] net: qcom/emac: move phy init code to separate files
From: David Miller @ 2016-11-24 16:32 UTC (permalink / raw)
  To: timur; +Cc: alokc, netdev
In-Reply-To: <1479769102-27909-2-git-send-email-timur@codeaurora.org>

From: Timur Tabi <timur@codeaurora.org>
Date: Mon, 21 Nov 2016 16:58:21 -0600

> diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii-fsm9900.c b/drivers/net/ethernet/qualcomm/emac/emac-sgmii-fsm9900.c
> new file mode 100644
> index 0000000..46f01b3
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii-fsm9900.c
> @@ -0,0 +1,246 @@
 ...
> +	return 0;
> +}
> +

Please do not place empty blank lines at the end of files.

GIT even warns about this.

^ permalink raw reply

* Re: [PATCH 1/2] net: qcom/emac: move phy init code to separate files
From: Timur Tabi @ 2016-11-24 16:39 UTC (permalink / raw)
  To: David Miller; +Cc: alokc, netdev
In-Reply-To: <20161124.113220.1130207009395086230.davem@davemloft.net>

David Miller wrote:
> Please do not place empty blank lines at the end of files.
>
> GIT even warns about this.

I need to rework this patchset anyway, and it won't be ready in time for 
4.10.  I will definitely fix this problem.  Thanks.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply

* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-24 16:43 UTC (permalink / raw)
  To: David Miller, hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <20161124.112152.692025478489876693.davem@davemloft.net>

On 16-11-24 11:21 AM, David Miller wrote:
> From: Hayes Wang <hayeswang@realtek.com>
> Date: Thu, 24 Nov 2016 13:26:55 +0000
>
>> I don't think the garbage results from our driver or device.
> This is my impression with what has been presented so far as well.

It's not garbage.

The latest run with the debug code I posted here earlier just spat out this below.
Using coherent (guarded, non-cacheable) RX buffers, with mb() calls:

[   15.199157] r8152_check_rx_desc: rx_desc looks bad.
[   15.204270] r8152_rx_bottom: offset=0/3376 bad rx_desc
[   15.209584] r8152_dump_rx_desc: 3d435253 3034336d 202f3a30 47524154 2f3d5445 3034336d rx_len=21075

The bad data in this case is ASCII:

         "SRC=m3400:/ TARGET=/m340"

This data is what is seen in /run/mount/utab, a file that is read/written over NFS on each boot.

         "SRC=m3400:/ TARGET=/m3400 ROOT=/ ATTRS=nolock,addr=192.168.8.1\n"

But how does this ASCII data end up at offset zero of the rx buffer??
Not possible -- this isn't even stale data, because only an rx_desc could
be at that offset in that buffer.

So even if this were a platform memory coherency issue, one should still
never see ASCII data at the beginning of an rx buffer.  The driver NEVER
writes anything to the rx buffers.  Only the USB hardware ever does.

And only the r8152 dongle/driver exhibits this issue.
Other USB dongles do not.  They *might* still have such issues,
but because they use software checksums, the bad packets are caught/rejected.

The r8152 driver, without the debug/error-checking additions, would have tried
to interpret that ASCII data as an "rx_desc", and would have interpreted the
"checksum bits" therein as "valid checksum", and the packet would have passed
through the network stack, corrupting data.

This driver worked without noticeable issues in 3.12.xx.
It hasn't worked since.  Because it now trusts the hardware checksums,
without first checking to see if noise-on-the-line or something else
has corrupted the data before receipt in the rx buffer.

Based on the above capture, I suspect a bug in the chip itself, which perhaps
is only manifest on a very slow CPU.

Nobody here tests with slow CPUs, but they are very prevalent in embedded space.
And very few people use USB network dongles nowadays either, as nearly all "computers"
have built-in networking.  The market for USB network dongles is mostly embedded space.

Ergo.

Cheers

^ permalink raw reply

* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-11-24 16:49 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
	Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
	linux-kernel
In-Reply-To: <20161124160830.kdpbonsz3l26uuo5@earth>

[-- Attachment #1: Type: Text/Plain, Size: 4604 bytes --]

On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote:
> Hi,
> 
> On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote:
> > On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> > > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > > > > > "ifconfig hw ether XX" normally sets the address. I guess
> > > > > > > that's ioctl?
> > > > > > 
> > > > > > This sets temporary address and it is ioctl. IIRC same as
> > > > > > what ethtool uses. (ifconfig is already deprecated).
> > > > > > 
> > > > > > > And I guess we should use similar mechanism for permanent
> > > > > > > address.
> > > > > > 
> > > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing
> > > > > > temporary mac address. But here we do not want to change
> > > > > > permanent mac address. We want to tell kernel driver
> > > > > > current permanent mac address which is stored
> > > > > 
> > > > > Well... I'd still use similar mechanism :-).
> > > > 
> > > > Thats problematic, because in time when wlan0 interface is
> > > > registered into system and visible in ifconfig output it
> > > > already needs to have permanent mac address assigned.
> > > > 
> > > > We should assign permanent mac address before wlan0 of wl1251
> > > > is registered into system.
> > > 
> > > You can just add the MAC address to the NVS data, which is also
> > > required for the device initialization.
> > 
> > NVS data file has fixed size, there is IIRC no place for it.
> > 
> > But one of my suggestion was to use another request_firmware for
> > MAC address. So this is similar to what you are proposing, as NVS
> > data are loaded by request_firmware too...
> 
> Just append it to NVS data and modify the size check accordingly?

First that would mean to have request_firmware() function which will not 
use direct VFS access, but instead use userspace helper.

NVS data file with some default values (not suitable for usage) is 
already present in linux-firmware and available in /lib/firmware/...

Also I'm not sure if such thing is allowed by license:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.ti-connectivity

> > > I wonder if those information could be put into DT. Iirc some
> > > network devices get their MAC address from DT. Maybe we can add
> > > all NVS info to DT? How much data is it?
> > 
> > Proprietary, signed and closed bootloader NOLO does not support DT.
> > So for booting you need to append DTS file to kernel image.
> 
> Yeah, so NOLO without U-Boot will depend on userspace to fixup DT.
> 
> > U-Boot is optional and can be used as intermediate bootloader
> > between NOLO and kernel. But still it has problems with reading
> > from nand, so cannot read NVS data nor MAC address.
> 
> It may in the future?

I already tried that, but I failed. I was not able to access N900's nand 
from u-boot. No idea where was problem...

And if somebody fix onenand in u-boot, then needs to reimplement Nokia's 
proprietary parser of that partition where is stored NVS and mac address 
&& make this support in upstream u-boot.

So... I doubt it will be in any future.

+ no men power

> > > Userspace application can add all those information to the DT
> > > using a DT overlay. Also the u-boot could parse and add it at
> > > some point in the future.
> > 
> > In case when wl1251 is statically linked into kernel image, it is
> > loaded and initialized before even userspace applications starts.
> > 
> > So no... adding NVS data or MAC address into DT or DT overlay is
> > not a solution.
> 
> Actually with data loaded from DT you *can* load data quite early in
> the boot process, while your suggestions always require userspace.
> So you argument against yourself?

You cannot add DTS in uboot (no support). And if you modify DTS on 
running kernel from userspace, then it is too late when wl1251 is 
statically linked into kernel image.

wl1251 will not wait until some userspace modify DTS and add data...

But request_firmware() in fallback mode *can* wait for userspace so 
wl1251 can postpone its operation until mdev/udev/whatever starts 
listening for events and push needed firmware data to kernel.

So no, there is no argument against... request_firmware() in fallback 
mode with userspace helper is by design blocking and waiting for 
userspace. But waiting for some change in DST in kernel is just 
nonsense.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [net-next PATCH v1 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Martin Blumenstingl @ 2016-11-24 16:52 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, khilman-rdvid1DuHRBWk0Htik3J/w,
	mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
	carlo-KA+7E9HrN00dnm+yROfE0A, jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20161124154858.GB20455-g2DYL2Zd6BY@public.gmane.org>

Hi Andrew,

On Thu, Nov 24, 2016 at 4:48 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> The configuration values are provided as preprocessor macros to make the
>> devicetree files easier to read.
>
> Hi Martin
>
> If i'm reading the code/comments correctly, you can set the delay to
> 0, 2, 4 or 6ns? So calling this property amlogic,tx-delay-ns would be
> even easier to read.
indeed, this sounds like a very nice idea (as it moves the calculation
from the programmer's brain to dwmac-meson8b.c)!

I'll send an updated version once I received enough feedback (in case
something else is wrong with the patches)
--
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox