Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] MIPS,bpf: Improvements for MIPS eBPF JIT
From: David Miller @ 2017-08-21 17:32 UTC (permalink / raw)
  To: daniel; +Cc: david.daney, ast, netdev, linux-kernel, linux-mips, ralf
In-Reply-To: <599A98B7.6070909@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 21 Aug 2017 10:24:23 +0200

> On 08/21/2017 05:06 AM, David Miller wrote:
>> From: David Daney <david.daney@cavium.com>
>> Date: Fri, 18 Aug 2017 16:40:30 -0700
>>
>>> I suggest that the whole thing go via the BPF/net-next path as there
>>> are dependencies on code that is not yet merged to Linus' tree.
>>
>> What kind of dependency?  On networking or MIPS changes?
> 
> On networking, David implemented the JLT, JLE, JSLT and JSLE
> ops for the JIT in the patch set. Back then the MIPS JIT wasn't
> in net-next tree, thus this is basically just a follow-up, so
> that we have all covered with JIT support for net-next.

Ok, that makes sense, thanks for explaining.

^ permalink raw reply

* RE: [PATCH v3 net 2/2] net: ixgbe: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
From: Tantilov, Emil S @ 2017-08-21 17:36 UTC (permalink / raw)
  To: Ding Tianhong, davem@davemloft.net, Kirsher, Jeffrey T,
	keescook@chromium.org, linux-kernel@vger.kernel.org,
	sparclinux@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
	alexander.duyck@gmail.com, netdev@vger.kernel.org,
	linuxarm@huawei.com
In-Reply-To: <1503037265-11144-3-git-send-email-dingtianhong@huawei.com>

>-----Original Message-----
>From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
>owner@vger.kernel.org] On Behalf Of Ding Tianhong
>Sent: Thursday, August 17, 2017 11:21 PM
>To: davem@davemloft.net; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
>keescook@chromium.org; linux-kernel@vger.kernel.org;
>sparclinux@vger.kernel.org; intel-wired-lan@lists.osuosl.org;
>alexander.duyck@gmail.com; netdev@vger.kernel.org; linuxarm@huawei.com
>Cc: Ding Tianhong <dingtianhong@huawei.com>
>Subject: [PATCH v3 net 2/2] net: ixgbe: Use new
>PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
>
>The ixgbe driver use the compile check to determine if it can
>send TLPs to Root Port with the Relaxed Ordering Attribute set,
>this is too inconvenient, now the new flag
>PCI_DEV_FLAGS_NO_RELAXED_ORDERING
>has been added to the kernel and we could check the bit4 in the PCIe
>Device Control register to determine whether we should use the Relaxed
>Ordering Attributes or not, so use this new way in the ixgbe driver.
>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c  | 22 ---------------------
>-
> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 19 -------------------
> 2 files changed, 41 deletions(-)

This change looks good to me for ixgbe.

Acked-by: Emil Tantilov <emil.s.tantilov@intel.com>

Thanks,
Emil 
 
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
>index 523f9d0..8a32eb7 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_82598.c
>@@ -175,31 +175,9 @@ static s32 ixgbe_init_phy_ops_82598(struct ixgbe_hw
>*hw)
>  **/
> static s32 ixgbe_start_hw_82598(struct ixgbe_hw *hw)
> {
>-#ifndef CONFIG_SPARC
>-	u32 regval;
>-	u32 i;
>-#endif
> 	s32 ret_val;
>
> 	ret_val = ixgbe_start_hw_generic(hw);
>-
>-#ifndef CONFIG_SPARC
>-	/* Disable relaxed ordering */
>-	for (i = 0; ((i < hw->mac.max_tx_queues) &&
>-	     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
>-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL(i));
>-		regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
>-		IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL(i), regval);
>-	}
>-
>-	for (i = 0; ((i < hw->mac.max_rx_queues) &&
>-	     (i < IXGBE_DCA_MAX_QUEUES_82598)); i++) {
>-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
>-		regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
>-			    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
>-		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
>-	}
>-#endif
> 	if (ret_val)
> 		return ret_val;
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
>index d4933d2..96c324f 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
>@@ -350,25 +350,6 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
> 	}
> 	IXGBE_WRITE_FLUSH(hw);
>
>-#ifndef CONFIG_SPARC
>-	/* Disable relaxed ordering */
>-	for (i = 0; i < hw->mac.max_tx_queues; i++) {
>-		u32 regval;
>-
>-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
>-		regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
>-		IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
>-	}
>-
>-	for (i = 0; i < hw->mac.max_rx_queues; i++) {
>-		u32 regval;
>-
>-		regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
>-		regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
>-			    IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
>-		IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
>-	}
>-#endif
> 	return 0;
> }
>
>--
>1.8.3.1
>


^ permalink raw reply

* Re: [PATCH net-next] dsa: remove unused net_device arg from handlers
From: David Miller @ 2017-08-21 17:39 UTC (permalink / raw)
  To: fw; +Cc: netdev, f.fainelli, vivien.didelot
In-Reply-To: <20170817144700.16218-1-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Thu, 17 Aug 2017 16:47:00 +0200

> compile tested only, but saw no warnings/errors with
> allmodconfig build.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

I'll apply this for now.

Andrew says it looks OK, and if Florian F. needs it for his tag changes
we can revert.

^ permalink raw reply

* Re: [PATCH v2 net-next] net: ipv6: put host and anycast routes on device with address
From: David Miller @ 2017-08-21 17:41 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, yoshfuji, hannes
In-Reply-To: <1502997440-32334-1-git-send-email-dsahern@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Thu, 17 Aug 2017 12:17:20 -0700

> One nagging difference between ipv4 and ipv6 is host routes for ipv6
> addresses are installed using the loopback device or VRF / L3 Master
> device. e.g.,
> 
>     2001:db8:1::/120 dev veth0 proto kernel metric 256 pref medium
>     local 2001:db8:1::1 dev lo table local proto kernel metric 0 pref medium
> 
> Using the loopback device is convenient -- necessary for local tx, but
> has some nasty side effects, most notably setting the 'lo' device down
> causes all host routes for all local IPv6 address to be removed from the
> FIB and completely breaks IPv6 networking across all interfaces.
> 
> This patch puts FIB entries for IPv6 routes against the device. This
> simplifies the routes in the FIB, for example by making dst->dev and
> rt6i_idev->dev the same (a future patch can look at removing the device
> reference taken for rt6i_idev for FIB entries).
> 
> When copies are made on FIB lookups, the cloned route has dst->dev
> set to loopback (or the L3 master device). This is needed for the
> local Tx of packets to local addresses.
> 
> With fib entries allocated against the real network device, the addrconf
> code that reinserts host routes on admin up of 'lo' is no longer needed.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

I've applied this.

I'm unlikely to revert this just for java, given the kinds of things
they are doing.

^ permalink raw reply

* Re: Something hitting my total number of connections to the server
From: Eric Dumazet @ 2017-08-21 17:44 UTC (permalink / raw)
  To: Akshat Kakkar; +Cc: David Laight, netdev
In-Reply-To: <CAA5aLPhtK8YbSDua-F16C0W9fLm2w4=ta4g+Tv+wsUEqYPCxUg@mail.gmail.com>

On Mon, 2017-08-21 at 22:58 +0530, Akshat Kakkar wrote:

> As mentioned in my initial description, the server is not sending
> SYN-ACK. Thats what the main symptom. For completeness, its not
> sending any RST also.
> However, if I disable TCP timestamp ... the server starts giving SYN-ACK.
> The strangest thing is, my client doesnt initiate a connection with
> tcp timestamp, so how come disabling tcp timestamp is making things
> work.

As I said, maybe the bug was already fixed months ago.

By running an old kernel, you want us to spend time on something that
might already have been fixed.

Only if you run a current kernel _and_ reproduce the problem, then we
might take a look.

I suspect your client is a single host ?

- Why is timewait not being used ?

- What sysctls have been changed on your server ?

^ permalink raw reply

* Re: Something hitting my total number of connections to the server
From: Eric Dumazet @ 2017-08-21 17:44 UTC (permalink / raw)
  To: Akshat Kakkar; +Cc: David Laight, netdev
In-Reply-To: <1503337453.2499.7.camel@edumazet-glaptop3.roam.corp.google.com>

On Mon, 2017-08-21 at 10:44 -0700, Eric Dumazet wrote:

> - Why is timewait not being used ?
> 

s/timewait/timestamps/

^ permalink raw reply

* Re: [PATCH net-next] net: sched: Add the invalid handle check in qdisc_class_find
From: David Miller @ 2017-08-21 17:47 UTC (permalink / raw)
  To: gfree.wind; +Cc: netdev, xiyou.wangcong, jhs
In-Reply-To: <1503041004-105572-1-git-send-email-gfree.wind@vip.163.com>

From: gfree.wind@vip.163.com
Date: Fri, 18 Aug 2017 15:23:24 +0800

> From: Gao Feng <gfree.wind@vip.163.com>
> 
> Add the invalid handle "0" check to avoid unnecessary search, because
> the qdisc uses the skb->priority as the handle value to look up, and
> it is "0" usually.
> 
> Signed-off-by: Gao Feng <gfree.wind@vip.163.com>

Jamal, Cong, please review.

If 'id' zero is never hashed into the tables, this change looks
legitimate.

^ permalink raw reply

* Re: [PATCH 4/6] dpaa_eth: add NETIF_F_RXHASH
From: David Miller @ 2017-08-21 17:57 UTC (permalink / raw)
  To: madalin.bucur; +Cc: netdev, linuxppc-dev, linux-kernel
In-Reply-To: <1503046588-24349-5-git-send-email-madalin.bucur@nxp.com>

From: Madalin Bucur <madalin.bucur@nxp.com>
Date: Fri, 18 Aug 2017 11:56:26 +0300

> +	if (net_dev->features & NETIF_F_RXHASH && priv->keygen_in_use &&
> +	    !fman_port_get_hash_result_offset(priv->mac_dev->port[RX],
> +					      &hash_offset))
> +		skb_set_hash(skb, be32_to_cpu(*(u32 *)(vaddr + hash_offset)),
> +			     // if L4 exists, it was used in the hash generation
> +			     be32_to_cpu(fd->status) & FM_FD_STAT_L4CV ?
> +				PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3);

We do not use "//" c++ style comments in the kernel.

And putting a comment right in the middle of a set of arguments being
passed to a function makes the code harder to read, so please do not
do this.

^ permalink raw reply

* Re: [PATCH net-next] net: check type when freeing metadata dst
From: David Miller @ 2017-08-21 17:57 UTC (permalink / raw)
  To: equinox; +Cc: netdev, jakub.kicinski, sridhar.samudrala, horms
In-Reply-To: <20170818123135.518080-1-equinox@diac24.net>

From: David Lamparter <equinox@diac24.net>
Date: Fri, 18 Aug 2017 14:31:35 +0200

> Commit 3fcece12bc1b ("net: store port/representator id in metadata_dst")
> added a new type field to metadata_dst, but metadata_dst_free() wasn't
> updated to check it before freeing the METADATA_IP_TUNNEL specific dst
> cache entry.
> 
> This is not currently causing problems since it's far enough back in the
> struct to be zeroed for the only other type currently in existance
> (METADATA_HW_PORT_MUX), but nevertheless it's not correct.
> 
> Fixes: 3fcece12bc1b ("net: store port/representator id in metadata_dst")
> Signed-off-by: David Lamparter <equinox@diac24.net>

Applied.

^ permalink raw reply

* Re: [PATCHv3 net-next] gre: introduce native tunnel support for ERSPAN
From: David Miller @ 2017-08-21 18:02 UTC (permalink / raw)
  To: u9012063; +Cc: netdev, mvohra, kuznet, yoshfuji
In-Reply-To: <1503062680-43972-1-git-send-email-u9012063@gmail.com>

From: William Tu <u9012063@gmail.com>
Date: Fri, 18 Aug 2017 06:24:40 -0700

> +static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> +		      int gre_hdr_len)
> +{
> +	struct net *net = dev_net(skb->dev);
> +	struct ip_tunnel_net *itn;
> +	struct ip_tunnel *tunnel;
> +	struct metadata_dst *tun_dst = NULL;
> +	const struct iphdr *iph;
> +	struct erspanhdr *ershdr;
> +	__be32 index;
> +	__be32 session_id;
> +	int len;

Please order local variables from longest to shortest line, ie. reverse
christmas tree format.

> +
> +	itn = net_generic(net, erspan_net_id);
> +	iph = ip_hdr(skb);
> +	len =  iph->ihl * 4 + gre_hdr_len + sizeof(*ershdr);
> +
> +	if (unlikely(!pskb_may_pull(skb, len)))
> +		return -ENOMEM;

I think the len passed here is wrong, it should be
"gre_hdr_len + sizeof(*ershdr)".
> +static void erspan_build_header(struct sk_buff *skb,
> +				__be32 id, u32 index, bool truncate)
> +{
> +	struct erspanhdr *ershdr;
> +	struct iphdr *iphdr = ip_hdr(skb);
> +	struct ethhdr *eth = eth_hdr(skb);
> +	struct qtag_prefix {
> +		__be16 eth_type;
> +		__be16 tci;
> +	} *qp;
> +	u16 vlan_tci = 0;
> +	enum erspan_encap_type enc_type = ERSPAN_ENCAP_NOVLAN;

Reverse christmas tree for the local variables, please.

> +static int erspan_validate(struct nlattr *tb[], struct nlattr *data[],
> +			   struct netlink_ext_ack *extack)
> +{
> +	int ret;
> +	__be16 flags = 0;

Likewise.

^ permalink raw reply

* Re: [PATCH net-next 2/2] liquidio: make VF driver notify NIC firmware of MTU change
From: David Miller @ 2017-08-21 18:04 UTC (permalink / raw)
  To: felix.manlunas
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	veerasenareddy.burru
In-Reply-To: <20170818183520.GA4522@felix-thinkpad.cavium.com>

From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Fri, 18 Aug 2017 11:35:20 -0700

> Signed-off-by: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
> Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
> ---
>  drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> index 0402b18..e947783 100644
> --- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> +++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
> @@ -1545,13 +1545,27 @@ static struct net_device_stats *liquidio_get_stats(struct net_device *netdev)
>  static int liquidio_change_mtu(struct net_device *netdev, int new_mtu)
>  {
>  	struct lio *lio = GET_LIO(netdev);
> +	struct octeon_device *oct = lio->oct_dev;
> +	struct octnic_ctrl_pkt nctrl;
> +	int ret = 0;

Please order local variable declarations from longest to shortest line.

^ permalink raw reply

* Re: [PATCH RESEND 0/2] enable hires timer to timeout datagram socket
From: Vallish Vaidyeshwara @ 2017-08-21 18:22 UTC (permalink / raw)
  To: Richard Cochran
  Cc: davem, shuah, netdev, linux-kernel, eduval, anchalag, tglx
In-Reply-To: <20170820014744.GA43685@amazon.com>

On Sun, Aug 20, 2017 at 01:47:45AM +0000, Vallish Vaidyeshwara wrote:
> On Sat, Aug 19, 2017 at 08:21:45AM +0200, Richard Cochran wrote:
> > On Fri, Aug 18, 2017 at 10:27:56PM +0000, Vallish Vaidyeshwara wrote:
> > > We have a on-demand application that uses long timeouts and needs to react to
> > > events within milliseconds.
> >
> 
> Hello Richard,
> 
> > Huh?  The test program you posted does not react to any event.
> >
> 
> Application has logic for complex events and test program is kept simple to
> highlight the change in behavior seen with system calls.
>

Hello Richard,

AWS Lambda is affected by this change in behavior in
system call. Following links has more information:
https://en.wikipedia.org/wiki/AWS_Lambda
https://aws.amazon.com/lambda/

Thanks.
-Vallish

^ permalink raw reply

* Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning
From: Edward Cree via iovisor-dev @ 2017-08-21 18:36 UTC (permalink / raw)
  To: Alexei Starovoitov, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	Alexei Starovoitov, Daniel Borkmann
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, iovisor-dev,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5d4e12aa-6861-a176-a8cf-a766bbca0a7a-b10kYP2dOMg@public.gmane.org>

On 19/08/17 00:37, Alexei Starovoitov wrote:
> that '14: safe' above is not correct.
>
> Disabling liveness as:
> @@ -3282,7 +3288,7 @@ static bool regsafe(struct bpf_reg_state *rold,
>                     struct bpf_reg_state *rcur,
>                     bool varlen_map_access, struct idpair *idmap)
>  {
> -       if (!(rold->live & REG_LIVE_READ))
> +       if (0 && !(rold->live & REG_LIVE_READ))
>
> makes the test work properly and proper verifier output is:
> from 9 to 11: R0=map_value(id=0,off=0,ks=8,vs=48,imm=0) R1=inv(id=0,smax_value=10) R2=inv11 R10=fp0
> 11: (64) (u32) r1 <<= (u32) 2
> 12: (0f) r0 += r1
> 13: (05) goto pc+0
> 14: (7a) *(u64 *)(r0 +0) = 4
>
> R0=map_value(id=0,off=0,ks=8,vs=48,umax_value=17179869180,var_off=(0x0; 0x3fffffffc)) R1=inv(id=0,umax_value=17179869180,var_off=(0x0; 0x3fffffffc)) R2=inv11 R10=fp0
> R0 unbounded memory access, make sure to bounds check any array access into a map
>
> I don't yet understand the underlying reason. R0 should have been
> marked as LIVE_READ by ST_MEM...
> Please help debug it further.
>
Having added a bunch of debugging, I found out that indeed R0 _had_ been
 marked as LIVE_READ.  The problem was that env->varlen_map_value_access
 wasn't set, because the access was at a constant offset (imm=0), but then
 when we compare register states we just say "oh yeah, it's a map_value,
 we don't need to look at the var_off".
This probably results from my unifying PTR_TO_MAP_VALUE with
 PTR_TO_MAP_VALUE_ADJ; before that the old and new R0 would have different
 reg->type so wouldn't match.
I'm tempted to just rip out env->varlen_map_value_access and always check
 the whole thing, because honestly I don't know what it was meant to do
 originally or how it can ever do any useful pruning.  While drastic, it
 does cause your test case to pass.

I'm not quite sure why your test passed when you disabled liveness, though;
 that I can't explain.

-Ed

^ permalink raw reply

* Re: [RFC PATCH] dt-binding: net: sfp binding documentation
From: Rob Herring @ 2017-08-21 19:10 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, David S . Miller,
	Russell King, netdev,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <d73b635069a2054d110632f1f4196cd4bc281e7f.1503224886.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>

On Sun, Aug 20, 2017 at 5:28 AM, Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> Add device-tree binding documentation SFP transceivers. Support for SFP
> transceivers has been recently introduced (drivers/net/phy/sfp.c).
>
> Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
> ---
>
> The SFP driver is on net-next.
>
> Not sure about the rate-select-gpio property name. The SFP+ standard
> (not supported yet) uses two signals, RS0 and RS1. RS0 is compatible
> with the SFP rate select signal, while RS1 controls the Tx rate.
> ---
>  Documentation/devicetree/bindings/net/sff-sfp.txt | 24 +++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/sff-sfp.txt
>
> diff --git a/Documentation/devicetree/bindings/net/sff-sfp.txt b/Documentation/devicetree/bindings/net/sff-sfp.txt
> new file mode 100644
> index 000000000000..f0c27bc3925e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/sff-sfp.txt
> @@ -0,0 +1,24 @@
> +Small Form Factor (SFF) Committee Small Form-factor Pluggable (SFP)
> +Transceiver
> +
> +Required properties:
> +
> +- compatible : must be "sff,sfp"

Need to document "sff" vendor prefix.

Kind of a short name, but I guess it is sufficient. Are there
revisions of the standard (not SFP+) or more than one form factor (I
don't recall any)?

> +
> +Optional Properties:
> +
> +- i2c-bus : phandle of an I2C bus controller for the SFP two wire serial
> +  interface

Why not a child of the i2c bus it is on? IOW, what should this be a child of?

> +
> +- moddef0-gpio : phandle of the MOD-DEF0 (AKA Mod_ABS) module presence input
> +  gpio signal

mod-def0-gpios?

> +
> +- los-gpio : phandle of the Receiver Loss of Signal Indication input gpio
> +  signal
> +
> +- tx-fault-gpio : phandle of the Module Transmitter Fault input gpio signal
> +
> +- tx-disable-gpio : phandle of the Transmitter Disable output gpio signal
> +
> +- rate-select-gpio : phandle of the Rx Signaling Rate Select (AKA RS0) output
> +  gpio

-gpios is the preferred form for all of these.

> --
> 2.14.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
--
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: [RFC PATCH] dt-binding: net: sfp binding documentation
From: Rob Herring @ 2017-08-21 19:12 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Russell King - ARM Linux, Mark Rutland, Andrew Lunn,
	Florian Fainelli, David S . Miller, netdev,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170821150653.jmoogmxklkfbrzxg-MwjkAAnuF3khR1HGirfZ1z4kX+cae0hd@public.gmane.org>

On Mon, Aug 21, 2017 at 10:06 AM, Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> Hi Russell,
>
> On Mon, Aug 21, 2017 at 01:53:17PM +0100, Russell King - ARM Linux wrote:
>> On Sun, Aug 20, 2017 at 01:28:06PM +0300, Baruch Siach wrote:
>> > Add device-tree binding documentation SFP transceivers. Support for SFP
>> > transceivers has been recently introduced (drivers/net/phy/sfp.c).
>> >
>> > Signed-off-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
>> > ---
>> >
>> > The SFP driver is on net-next.
>> >
>> > Not sure about the rate-select-gpio property name. The SFP+ standard
>> > (not supported yet) uses two signals, RS0 and RS1. RS0 is compatible
>> > with the SFP rate select signal, while RS1 controls the Tx rate.
>>
>> SFP+ is usable with this, but the platforms I have do not wire the
>> rate select pins on the SFP+ sockets to GPIOs, but hard-wire them.
>
> So maybe naming this signal 'rate-select0-gpio' would make it more future
> (SPF+) proof? Or 'rate-select-rx-gpio'?

Just extend it by making it an array of 2 gpios.

Rob
--
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

* XDP redirect measurements, gotchas and tracepoints
From: Jesper Dangaard Brouer @ 2017-08-21 19:25 UTC (permalink / raw)
  To: xdp-newbies@vger.kernel.org
  Cc: brouer, John Fastabend, Daniel Borkmann, Andy Gospodarek,
	netdev@vger.kernel.org, Paweł Staszewski


I'be been playing with the latest XDP_REDIRECT feature, that was
accepted in net-next (for ixgbe), see merge commit[1].
 [1] https://git.kernel.org/davem/net-next/c/6093ec2dc31

At a first glance the performance looks awesome, and it is(!) when
your system is tuned for this workload. When perfectly tuned I can
show 13,096,427 pps forwarding, which is very close to 10Gbit/s
wirespeed at 64bytes (14.88Mpps).  Using only a single CPU (E5-1650 v4
@3.60GHz) core.

First gotcha(1): be aware of what you measure.  The reported numbers from
xdp_redirect_map is how many packets the XDP program received.  It
have no info whether the packet was actually transmitted out.  This
info is avail via TX counters[2] or an xdp tracepoint.

[2] ethtool_stats:
    https://github.com/netoptimizer/network-testing/blob/master/bin/ethtool_stats.pl

Second gotcha(2): you cannot TX out a device, unless it also have a
xdp bpf program attached. (This is an implicit dependency, as the
driver code need to setup XDP resources before it can ndo_xdp_xmit).

Third gotcha(3): You got this far, loaded xdp on both interfaces, and
notice now that (with default setup) you can RX with 14Mpps but only
TX with 6.9Mpps (and might have 5% idle cycles).  I debugged this via
perf tracepoint event xdp:xdp_redirect, and found this was due to
overrunning the xdp TX ring-queue size.

 Thus, for this workload, we need to adjust either the TX ring-queue
size (ethtool -G) or the DMA completion interval (ethtool -C rx-usecs).
See tuning and measurements below signature.

Fourth gotcha(4): Monitoring XDP redirect performance via the
tracepoint xdp:xdp_redirect, is too slow, and affect the measurements
themselves.  I'm working on optimizing these tracepoints, and will
share results tomorrow.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


No-tuning (default auto-tuning rx-usecs 1):
 Notice tx_packets is too low compared to RX

Show adapter(s) (ixgbe1 ixgbe2) statistics (ONLY that changed!)
Ethtool(ixgbe1  ) stat:     14720134 (     14,720,134) <= fdir_miss /sec
Ethtool(ixgbe1  ) stat:    874951205 (    874,951,205) <= rx_bytes /sec
Ethtool(ixgbe1  ) stat:    952434290 (    952,434,290) <= rx_bytes_nic /sec
Ethtool(ixgbe1  ) stat:       271737 (        271,737) <= rx_missed_errors /sec
Ethtool(ixgbe1  ) stat:        27631 (         27,631) <= rx_no_dma_resources /sec
Ethtool(ixgbe1  ) stat:     14582520 (     14,582,520) <= rx_packets /sec
Ethtool(ixgbe1  ) stat:     14610072 (     14,610,072) <= rx_pkts_nic /sec
Ethtool(ixgbe1  ) stat:    874947566 (    874,947,566) <= rx_queue_2_bytes /sec
Ethtool(ixgbe1  ) stat:     14582459 (     14,582,459) <= rx_queue_2_packets /sec
Ethtool(ixgbe2  ) stat:    417934735 (    417,934,735) <= tx_bytes /sec
Ethtool(ixgbe2  ) stat:    445801114 (    445,801,114) <= tx_bytes_nic /sec
Ethtool(ixgbe2  ) stat:      6965579 (      6,965,579) <= tx_packets /sec
Ethtool(ixgbe2  ) stat:      6965771 (      6,965,771) <= tx_pkts_nic /sec


Tuned with rx-usecs 25:
 ethtool -C ixgbe1 rx-usecs 25 ;\
 ethtool -C ixgbe2 rx-usecs 25

Show adapter(s) (ixgbe1 ixgbe2) statistics (ONLY that changed!)
Ethtool(ixgbe1  ) stat:     14123764 (     14,123,764) <= fdir_miss /sec
Ethtool(ixgbe1  ) stat:    786101618 (    786,101,618) <= rx_bytes /sec
Ethtool(ixgbe1  ) stat:    952807289 (    952,807,289) <= rx_bytes_nic /sec
Ethtool(ixgbe1  ) stat:      1047989 (      1,047,989) <= rx_missed_errors /sec
Ethtool(ixgbe1  ) stat:       737938 (        737,938) <= rx_no_dma_resources /sec
Ethtool(ixgbe1  ) stat:     13101694 (     13,101,694) <= rx_packets /sec
Ethtool(ixgbe1  ) stat:     13839620 (     13,839,620) <= rx_pkts_nic /sec
Ethtool(ixgbe1  ) stat:    786101618 (    786,101,618) <= rx_queue_2_bytes /sec
Ethtool(ixgbe1  ) stat:     13101694 (     13,101,694) <= rx_queue_2_packets /sec
Ethtool(ixgbe2  ) stat:    785785590 (    785,785,590) <= tx_bytes /sec
Ethtool(ixgbe2  ) stat:    838179358 (    838,179,358) <= tx_bytes_nic /sec
Ethtool(ixgbe2  ) stat:     13096427 (     13,096,427) <= tx_packets /sec
Ethtool(ixgbe2  ) stat:     13096519 (     13,096,519) <= tx_pkts_nic /

Tuned with adjusting ring-queue sizes:
 ethtool -G ixgbe1 rx 1024 tx 1024 ;\
 ethtool -G ixgbe2 rx 1024 tx 1024

Show adapter(s) (ixgbe1 ixgbe2) statistics (ONLY that changed!)
Ethtool(ixgbe1  ) stat:     14169252 (     14,169,252) <= fdir_miss /sec
Ethtool(ixgbe1  ) stat:    783666937 (    783,666,937) <= rx_bytes /sec
Ethtool(ixgbe1  ) stat:    957332815 (    957,332,815) <= rx_bytes_nic /sec
Ethtool(ixgbe1  ) stat:      1053052 (      1,053,052) <= rx_missed_errors /sec
Ethtool(ixgbe1  ) stat:       844113 (        844,113) <= rx_no_dma_resources /sec
Ethtool(ixgbe1  ) stat:     13061116 (     13,061,116) <= rx_packets /sec
Ethtool(ixgbe1  ) stat:     13905221 (     13,905,221) <= rx_pkts_nic /sec
Ethtool(ixgbe1  ) stat:    783666937 (    783,666,937) <= rx_queue_2_bytes /sec
Ethtool(ixgbe1  ) stat:     13061116 (     13,061,116) <= rx_queue_2_packets /sec
Ethtool(ixgbe2  ) stat:    783312119 (    783,312,119) <= tx_bytes /sec
Ethtool(ixgbe2  ) stat:    835526092 (    835,526,092) <= tx_bytes_nic /sec
Ethtool(ixgbe2  ) stat:     13055202 (     13,055,202) <= tx_packets /sec
Ethtool(ixgbe2  ) stat:     13055093 (     13,055,093) <= tx_pkts_nic /sec

^ permalink raw reply

* [PATCH V2 net-next 0/2] liquidio: VF driver will notify NIC firmware of MTU change
From: Felix Manlunas @ 2017-08-21 19:35 UTC (permalink / raw)
  To: davem
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	veerasenareddy.burru

From: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>

Make VF driver notify NIC firmware of MTU change.  Firmware needs this
information for MTU propagation and enforcement.

The first patch in this series moves a macro definition to a proper place
to prevent a build error in the second patch which has the code that sends
the notification.

Change Log:
  V1 -> V2
    * Add "From:" line to patch #1 and #2 to give credit to the author.
    * In patch #2, order local variable declarations from longest to
      shortest line.

Veerasenareddy Burru (2):
  liquidio: move macro definition to a proper place
  liquidio: make VF driver notify NIC firmware of MTU change

 .../ethernet/cavium/liquidio/cn23xx_pf_device.h    |  2 --
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 27 ++++++++++++++++++----
 .../net/ethernet/cavium/liquidio/liquidio_common.h |  2 ++
 3 files changed, 24 insertions(+), 7 deletions(-)

-- 
2.9.0

^ permalink raw reply

* [PATCH V2 net-next 1/2] liquidio: move macro definition to a proper place
From: Felix Manlunas @ 2017-08-21 19:35 UTC (permalink / raw)
  To: davem
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	veerasenareddy.burru
In-Reply-To: <20170821193527.GA6102@felix-thinkpad.cavium.com>

From: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>

The macro LIO_CMD_WAIT_TM is not specific to the PF driver; it can be used
by the VF driver too, so move its definition from a PF-specific header file
to one that's common to PF and VF.

Signed-off-by: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.h | 2 --
 drivers/net/ethernet/cavium/liquidio/liquidio_common.h  | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.h b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.h
index dee6046..2aba524 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.h
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.h
@@ -24,8 +24,6 @@
 
 #include "cn23xx_pf_regs.h"
 
-#define LIO_CMD_WAIT_TM 100
-
 /* Register address and configuration for a CN23XX devices.
  * If device specific changes need to be made then add a struct to include
  * device specific fields as shown in the commented section
diff --git a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
index 906e30a..d0076c1 100644
--- a/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
+++ b/drivers/net/ethernet/cavium/liquidio/liquidio_common.h
@@ -237,6 +237,8 @@ static inline void add_sg_size(struct octeon_sg_entry *sg_entry,
 #define   OCTNET_CMD_VLAN_FILTER_ENABLE 0x1
 #define   OCTNET_CMD_VLAN_FILTER_DISABLE 0x0
 
+#define   LIO_CMD_WAIT_TM 100
+
 /* RX(packets coming from wire) Checksum verification flags */
 /* TCP/UDP csum */
 #define   CNNIC_L4SUM_VERIFIED             0x1
-- 
2.9.0

^ permalink raw reply related

* [PATCH V2 net-next 2/2] liquidio: make VF driver notify NIC firmware of MTU change
From: Felix Manlunas @ 2017-08-21 19:35 UTC (permalink / raw)
  To: davem
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	veerasenareddy.burru
In-Reply-To: <20170821193527.GA6102@felix-thinkpad.cavium.com>

From: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>

Signed-off-by: Veerasenareddy Burru <veerasenareddy.burru@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/lio_vf_main.c | 27 ++++++++++++++++++----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
index 0402b18..2e993ce 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_vf_main.c
@@ -1544,14 +1544,31 @@ static struct net_device_stats *liquidio_get_stats(struct net_device *netdev)
  */
 static int liquidio_change_mtu(struct net_device *netdev, int new_mtu)
 {
-	struct lio *lio = GET_LIO(netdev);
+	struct octnic_ctrl_pkt nctrl;
+	struct octeon_device *oct;
+	struct lio *lio;
+	int ret = 0;
 
-	lio->mtu = new_mtu;
+	lio = GET_LIO(netdev);
+	oct = lio->oct_dev;
+
+	memset(&nctrl, 0, sizeof(struct octnic_ctrl_pkt));
 
-	netif_info(lio, probe, lio->netdev, "MTU Changed from %d to %d\n",
-		   netdev->mtu, new_mtu);
+	nctrl.ncmd.u64 = 0;
+	nctrl.ncmd.s.cmd = OCTNET_CMD_CHANGE_MTU;
+	nctrl.ncmd.s.param1 = new_mtu;
+	nctrl.iq_no = lio->linfo.txpciq[0].s.q_no;
+	nctrl.wait_time = LIO_CMD_WAIT_TM;
+	nctrl.netpndev = (u64)netdev;
+	nctrl.cb_fn = liquidio_link_ctrl_cmd_completion;
+
+	ret = octnet_send_nic_ctrl_pkt(lio->oct_dev, &nctrl);
+	if (ret < 0) {
+		dev_err(&oct->pci_dev->dev, "Failed to set MTU\n");
+		return -EIO;
+	}
 
-	netdev->mtu = new_mtu;
+	lio->mtu = new_mtu;
 
 	return 0;
 }
-- 
2.9.0

^ permalink raw reply related

* [PATCH net] net: dsa: skb_put_padto() already frees nskb
From: Florian Fainelli @ 2017-08-21 19:41 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didtelot, woojung.huh, Florian Fainelli

skb_put_padto() already frees the passed sk_buff reference upon error,
so calling kfree_skb() on it again is not necessary.

Detected by CoverityScan, CID#1416687 ("USE_AFTER_FREE")

Fixes: e71cb9e00922 ("net: dsa: ksz: fix skb freeing")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/tag_ksz.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index de66ca8e6201..107172c82107 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -60,10 +60,8 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 					 skb_transport_header(skb) - skb->head);
 		skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
 
-		if (skb_put_padto(nskb, nskb->len + padlen)) {
-			kfree_skb(nskb);
+		if (skb_put_padto(nskb, nskb->len + padlen))
 			return NULL;
-		}
 
 		kfree_skb(skb);
 	}
-- 
2.9.3

^ permalink raw reply related

* [PATCH net] net: dsa: skb_put_padto() already frees nskb
From: Florian Fainelli @ 2017-08-21 19:41 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didelot, woojung.huh, Florian Fainelli

skb_put_padto() already frees the passed sk_buff reference upon error,
so calling kfree_skb() on it again is not necessary.

Detected by CoverityScan, CID#1416687 ("USE_AFTER_FREE")

Fixes: e71cb9e00922 ("net: dsa: ksz: fix skb freeing")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/tag_ksz.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index de66ca8e6201..107172c82107 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -60,10 +60,8 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
 					 skb_transport_header(skb) - skb->head);
 		skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));
 
-		if (skb_put_padto(nskb, nskb->len + padlen)) {
-			kfree_skb(nskb);
+		if (skb_put_padto(nskb, nskb->len + padlen))
 			return NULL;
-		}
 
 		kfree_skb(skb);
 	}
-- 
2.9.3

^ permalink raw reply related

* [PATCH] vhost: fix end of range for access_ok
From: Michael S. Tsirkin @ 2017-08-21 19:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: Koichiro Den, Jason Wang, kvm, virtualization, netdev,
	David Miller

During access_ok checks, addr increases as we iterate over the data
structure, thus addr + len - 1 will point beyond the end of region we
are translating.  Harmless since we then verify that the region covers
addr, but let's not waste cpu cycles.

Reported-by: Koichiro Den <den@klaipeden.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Lightly tested, would appreciate an ack from reporter.

 drivers/vhost/vhost.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e4613a3..ecd70e4 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1176,7 +1176,7 @@ static int iotlb_access_ok(struct vhost_virtqueue *vq,
 {
 	const struct vhost_umem_node *node;
 	struct vhost_umem *umem = vq->iotlb;
-	u64 s = 0, size, orig_addr = addr;
+	u64 s = 0, size, orig_addr = addr, last = addr + len - 1;
 
 	if (vhost_vq_meta_fetch(vq, addr, len, type))
 		return true;
@@ -1184,7 +1184,7 @@ static int iotlb_access_ok(struct vhost_virtqueue *vq,
 	while (len > s) {
 		node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
 							   addr,
-							   addr + len - 1);
+							   last);
 		if (node == NULL || node->start > addr) {
 			vhost_iotlb_miss(vq, addr, access);
 			return false;
-- 
MST

^ permalink raw reply related

* Re: [PATCH 1/2] vhost: remove the possible fruitless search on iotlb prefetch
From: Michael S. Tsirkin @ 2017-08-21 19:45 UTC (permalink / raw)
  To: Koichiro Den; +Cc: jasowang, kvm, virtualization, netdev
In-Reply-To: <20170819064114.27219-1-den@klaipeden.com>

On Sat, Aug 19, 2017 at 03:41:14PM +0900, Koichiro Den wrote:
> Signed-off-by: Koichiro Den <den@klaipeden.com>
> ---
>  drivers/vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e4613a3c362d..93e909afc1c3 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1184,7 +1184,7 @@ static int iotlb_access_ok(struct vhost_virtqueue *vq,
>  	while (len > s) {
>  		node = vhost_umem_interval_tree_iter_first(&umem->umem_tree,
>  							   addr,
> -							   addr + len - 1);
> +							   addr + len - s - 1);
>  		if (node == NULL || node->start > addr) {
>  			vhost_iotlb_miss(vq, addr, access);
>  			return false;

This works but it probably makes sense to just refactor the code to make end of
range a variable. I posted a patch like this, pls take a look.

> -- 
> 2.9.4
> 

^ permalink raw reply

* Re: [PATCH net] net: dsa: skb_put_padto() already frees nskb
From: Andrew Lunn @ 2017-08-21 19:47 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot, woojung.huh
In-Reply-To: <20170821194143.27885-1-f.fainelli@gmail.com>

On Mon, Aug 21, 2017 at 12:41:43PM -0700, Florian Fainelli wrote:
> skb_put_padto() already frees the passed sk_buff reference upon error,
> so calling kfree_skb() on it again is not necessary.
> 
> Detected by CoverityScan, CID#1416687 ("USE_AFTER_FREE")
> 
> Fixes: e71cb9e00922 ("net: dsa: ksz: fix skb freeing")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: Re: [PATCH 2/4] dt-bindings: add binding for RTL8211E Ethernet PHY
From: Florian Fainelli @ 2017-08-21 19:54 UTC (permalink / raw)
  To: icenowy-h8G6r0blFSE
  Cc: Andrew Lunn, Rob Herring, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <19ad1316a74e344d3e1c783458eb4d59-h8G6r0blFSE@public.gmane.org>

On 08/21/2017 07:53 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
> 在 2017-05-05 02:29,Florian Fainelli 写道:
>> On 05/04/2017 11:26 AM, Icenowy Zheng wrote:
>>>
>>>
>>> 于 2017年5月5日 GMT+08:00 上午2:21:29, Florian Fainelli
>>> <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>>>> On 05/04/2017 11:10 AM, icenowy-h8G6r0blFSE@public.gmane.org wrote:
>>>>> 在 2017-04-22 08:22,Florian Fainelli 写道:
>>>>>> On 04/21/2017 04:24 PM, Icenowy Zheng wrote:
>>>>>>> From: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>>>
>>>>>>> Some RTL8211E Ethernet PHY have an issue that needs a workaround
>>>>>>> indicated with device tree.
>>>>>>>
>>>>>>> Add the binding for a property that indicates this workaround.
>>>>>>>
>>>>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/net/realtek,rtl8211e.txt   | 22
>>>>>>> ++++++++++++++++++++++
>>>>>>>  1 file changed, 22 insertions(+)
>>>>>>>  create mode 100644
>>>>>>> Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>> b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..c1913301bfe8
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl8211e.txt
>>>>>>> @@ -0,0 +1,22 @@
>>>>>>> +Realtek RTL8211E Ethernet PHY
>>>>>>> +
>>>>>>> +One batch of RTL8211E is slight broken, that needs some special
>>>> (and
>>>>>>> +full of magic numbers) tweaking in order to make GbE to operate
>>>>>>> properly.
>>>>>>> +The only well-known board that used the broken batch is Pine64+.
>>>>>>> +Configure it through an Ethernet OF device node.
>>>>>>> +
>>>>>>> +Optional properties:
>>>>>>> +
>>>>>>> +- realtek,disable-rx-delay:
>>>>>>> +  If set, RX delay will be completely disabled (according to
>>>>>>> Realtek). This
>>>>>>> +  will affect the performance on non-broken boards.
>>>>>>> +  default: do not disable RX delay.
>>>>>>
>>>>>> Please don't introduce custom properties to do that, instead correct
>>>>>> specify the "phy-mode" such that it is e.g: "rgmii-txid" which
>>>> indicates
>>>>>> that there should be no RX internal delay, but a TX internal delay
>>>> added
>>>>>> by the PHY.
>>>>>
>>>>> Checked the document, the meaning of "rgmii-txid" is not correct
>>>> here.
>>>>>
>>>>> This doesn't effect the MAC, and the MAC should still add TX delay.
>>>>>
>>>>> The definition of "rgmii-txid" in
>>>>> Documentation/devicetree/binding/net/ethernet.txt is "RGMII with
>>>>> internal TX delay provided by the PHY, the MAC should not add an TX
>>>> delay
>>>>> in this case". However, this do not indicate that the MAC doesn't add
>>>> TX
>>>>> delay; in fact that just totally disabled the PHY to provide the RX
>>>> delay.
>>>>> MAC still should to add delay on both TX/RX, which is the semantic of
>>>>> standard "rgmii".
>>>>>
>>>>> So I cannot used "rgmii-txid" here, but should continue to use this
>>>>> custom property.
> 
> Sorry for replying an old email, but it's because the driver of the MAC I
> used is merged (dwmac-sun8i).
> 
> The driver of the MAC currently only supports "mii", "rmii", and "rgmii",
> and according to the SoC's user manual, the MAC cannot has its delays
> disabled.
> 
> How should it handle this "rgmii-txid" here? Just treat it as "rgmii"?

Considering there are no configurable delays on the MAC side, all you
can do is treat all RGMII variants the same by configuring the MAC for
RGMII mode (with no additional capabilities and as opposed to MII, RMII
which are other clocking/data pins modes) and let the PHY configure the
delay accordingly based on "phy-mode"/phy_interface_t. You can use
phy_interface_is_rgmii() as a helper function to cover all 4 variants.
-- 
Florian

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ 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