Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3 0/6] enable creating [k,u]probe with perf_event_open
From: Daniel Borkmann @ 2017-12-01 14:54 UTC (permalink / raw)
  To: Song Liu, peterz, rostedt, mingo, davem, netdev, linux-kernel
  Cc: kernel-team, alexei.starovoitov
In-Reply-To: <20171130235023.1414663-1-songliubraving@fb.com>

On 12/01/2017 12:50 AM, Song Liu wrote:
> Changes PATCH v2 to PATCH v3:
>   Remove fixed type PERF_TYPE_KPROBE and PERF_TYPE_UPROBE, use dynamic
>   type instead.
>   Update userspace (samples/bpf, bcc) to look up type from sysfs.
>   Change License info in test_many_kprobe_user.c as Philippe Ombredanne
>   suggested.
> 
> Changes PATCH v1 to PATCH v2:
>   Split PERF_TYPE_PROBE into PERF_TYPE_KPROBE and PERF_TYPE_UPROBE.
>   Split perf_probe into perf_kprobe and perf_uprobe.
>   Remove struct probe_desc, use config1 and config2 instead.
> 
> Changes RFC v2 to PATCH v1:
>   Check type PERF_TYPE_PROBE in perf_event_set_filter().
>   Rebase on to tip perf/core.
> 
> Changes RFC v1 to RFC v2:
>   Fix build issue reported by kbuild test bot by adding ifdef of
>   CONFIG_KPROBE_EVENTS, and CONFIG_UPROBE_EVENTS.
> 
> RFC v1 cover letter:
> 
> This is to follow up the discussion over "new kprobe api" at Linux
> Plumbers 2017:
> 
> https://www.linuxplumbersconf.org/2017/ocw/proposals/4808
> 
> With current kernel, user space tools can only create/destroy [k,u]probes
> with a text-based API (kprobe_events and uprobe_events in tracefs). This
> approach relies on user space to clean up the [k,u]probe after using them.
> However, this is not easy for user space to clean up properly.
> 
> To solve this problem, we introduce a file descriptor based API.
> Specifically, we extended perf_event_open to create [k,u]probe, and attach
> this [k,u]probe to the file descriptor created by perf_event_open. These
> [k,u]probe are associated with this file descriptor, so they are not
> available in tracefs.
> 
> We reuse large portion of existing trace_kprobe and trace_uprobe code.
> Currently, the file descriptor API does not support arguments as the
> text-based API does. This should not be a problem, as user of the file
> decriptor based API read data through other methods (bpf, etc.).
> 
> I also include a patch to to bcc, and a patch to man-page perf_even_open.
> Please see the list below. A fork of bcc with this patch is also available
> on github:
> 
>   https://github.com/liu-song-6/bcc/tree/perf_event_open

How should this set be routed? The majority of changes are in
core tracing, so I guess taking that route as well would make
most sense here.

bpf_load.[ch] changes could potentially make surgery necessary
later on, would it make sense to pull only the two samples plus
tools/include/uapi commit then into bpf-next via pull-req after
they are considered fine and got applied from Peter/Steven?
Presumably git would handle this workflow fine, thus pull-req
could avoid the duplicate patch issue we had recently? I'm also
okay if the whole series goes via tracing and if we indeed get
into the situation, we just fix up any merge conflicts under
samples.

> Thanks,
> Song
> 
> man-pages patch:
>   perf_event_open.2: add type kprobe and uprobe
> 
> bcc patch:
>   bcc: Try use new API to create [k,u]probe with perf_event_open
> 
> kernel patches:
> 
> Song Liu (6):
>   perf: prepare perf_event.h for new types perf_kprobe and perf_uprobe
>   perf: copy new perf_event.h to tools/include/uapi
>   perf: implement pmu perf_kprobe
>   perf: implement pmu perf_uprobe
>   bpf: add option for bpf_load.c to use perf_kprobe
>   bpf: add new test test_many_kprobe
> 
>  include/linux/trace_events.h          |   4 +
>  include/uapi/linux/perf_event.h       |   6 ++
>  kernel/events/core.c                  |  81 ++++++++++++++-
>  kernel/trace/trace_event_perf.c       | 111 ++++++++++++++++++++
>  kernel/trace/trace_kprobe.c           |  91 +++++++++++++++--
>  kernel/trace/trace_probe.h            |  11 ++
>  kernel/trace/trace_uprobe.c           |  86 ++++++++++++++--
>  samples/bpf/Makefile                  |   3 +
>  samples/bpf/bpf_load.c                |  63 ++++++++++--
>  samples/bpf/bpf_load.h                |  14 +++
>  samples/bpf/test_many_kprobe_user.c   | 186 ++++++++++++++++++++++++++++++++++
>  tools/include/uapi/linux/perf_event.h |   6 ++
>  12 files changed, 633 insertions(+), 29 deletions(-)
>  create mode 100644 samples/bpf/test_many_kprobe_user.c
> 
> --
> 2.9.5
> 

^ permalink raw reply

* Re: [PATCH] vsock.7: document VSOCK socket address family
From: G. Branden Robinson @ 2017-12-01 14:57 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Jorgen S. Hansen,
	linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dexuan Cui
In-Reply-To: <20171201130901.GA12803-lxVrvc10SDRcolVlb+j0YCZi+YwRKgec@public.gmane.org>

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

At 2017-12-01T13:09:01+0000, Stefan Hajnoczi wrote:
> On Thu, Nov 30, 2017 at 01:21:26PM +0000, Jorgen S. Hansen wrote:
> > > On Nov 30, 2017, at 12:21 PM, Stefan Hajnoczi <stefanha-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
> Thanks for the quick review!
> 
> I forgot to ask you: Is SOCK_DGRAM reliable and in-order over VMCI?
> 
> > > +.PP
> > > +Valid socket types are
> > > +.B SOCK_STREAM
> > > +and
> > > +.B SOCK_DGRAM .
> > 
> > The space here results in a space between SOCK_DGRAM and the “.” in the formatted text. Is that intentional?
> 
> I haven't figured out the groff syntax to avoid the space :(.  Any
> ideas?

What you want is the .BR macro.

.BR SOCK_DGRAM .

The man macro package defines six "two-font" macros for switching
between roman, bold, and italic faces without intervening space.

See man(7) and groff_man(7).

-- 
Regards,
Branden

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

^ permalink raw reply

* Re: [PATCH net,stable v4 0/3] vhost: fix a few skb leaks
From: Michael S. Tsirkin @ 2017-12-01 14:58 UTC (permalink / raw)
  To: Matthew Rosato
  Cc: wexu, virtualization, netdev, linux-kernel, jasowang,
	David Miller
In-Reply-To: <9ef31370-4042-b701-e983-90b8f20a2a39@linux.vnet.ibm.com>

On Fri, Dec 01, 2017 at 09:54:02AM -0500, Matthew Rosato wrote:
> On 12/01/2017 09:47 AM, Michael S. Tsirkin wrote:
> > On Fri, Dec 01, 2017 at 05:10:35AM -0500, wexu@redhat.com wrote:
> >> From: Wei Xu <wexu@redhat.com>
> >>
> >> Matthew found a roughly 40% tcp throughput regression with commit
> >> c67df11f(vhost_net: try batch dequing from skb array) as discussed
> >> in the following thread:
> >> https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html
> > 
> > Series
> > 
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Tested this series on a z13 (s390x) on top of net-next using 4GB/4vcpu
> guests.  Verified that both the reported TCP throughput regression and
> memory leak are resolved.
> 
> net-next:  19.83 Gb/s
> net-next+: 35.02 Gb/s
> 
> Thanks all!
> 
> Matt

So we can also add
Tested-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>

Dave, pls take this through the net tree as most changes are
in tun/tap.

Thanks!

> > 
> > 
> >> v4:
> >> - fix zero iov iterator count in tap/tap_do_read()(Jason)
> >> - don't put tun in case of EBADFD(Jason)
> >> - Replace msg->msg_control with new 'skb' when calling tun/tap_do_read()
> >>
> >> v3:
> >> - move freeing skb from vhost to tun/tap recvmsg() to not
> >>   confuse the callers.
> >>
> >> v2:
> >> - add Matthew as the reporter, thanks matthew.
> >> - moving zero headcount check ahead instead of defer consuming skb
> >>   due to jason and mst's comment.
> >> - add freeing skb in favor of recvmsg() fails.
> >>
> >> Wei Xu (3):
> >>   vhost: fix skb leak in handle_rx()
> >>   tun: free skb in early errors
> >>   tap: free skb if flags error
> >>
> >>  drivers/net/tap.c   | 14 ++++++++++----
> >>  drivers/net/tun.c   | 24 ++++++++++++++++++------
> >>  drivers/vhost/net.c | 20 ++++++++++----------
> >>  3 files changed, 38 insertions(+), 20 deletions(-)
> >>
> >> -- 
> >> 1.8.3.1
> > 

^ permalink raw reply

* [PATCH net-next] cxgb4vf: Fix netdev_features flag
From: Ganesh Goudar @ 2017-12-01 14:59 UTC (permalink / raw)
  To: netdev, davem
  Cc: nirranjan, indranil, venkatesh, Ganesh Goudar, Arjun Vynipadath

GRO is not supported by Chelsio HW when rx_csum is disabled.
Update the netdev features flag when rx_csum is modified.

Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index b48361c..8e993ac 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -1114,6 +1114,10 @@ static int cxgb4vf_change_mtu(struct net_device *dev, int new_mtu)
 static netdev_features_t cxgb4vf_fix_features(struct net_device *dev,
 	netdev_features_t features)
 {
+	/* Disable GRO, if RX_CSUM is disabled */
+	if (!(features & NETIF_F_RXCSUM))
+		features &= ~NETIF_F_GRO;
+
 	/*
 	 * Since there is no support for separate rx/tx vlan accel
 	 * enable/disable make sure tx flag is always in same state as rx.
-- 
2.1.0

^ permalink raw reply related

* Re: Fixing CVE-2017-16939 in v4.4.y and possibly v3.18.y
From: Greg Kroah-Hartman @ 2017-12-01 15:09 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: netdev, stable
In-Reply-To: <20171130183740.GA20343@roeck-us.net>

On Thu, Nov 30, 2017 at 10:37:40AM -0800, Guenter Roeck wrote:
> Hi,
> 
> The fix for CVE-2017-16939 has been applied to v4.9.y, but not to v4.4.y
> and older kernels. However, I confirmed that running the published POC
> (see https://blogs.securiteam.com/index.php/archives/3535) does crash a 4.4
> kernel.
> 
> I confirmed that the following two patches fix the problem in v4.4.y.
> Please consider applying them to v4.4.y (and possibly v3.18.y).
> 
> fc9e50f5a5a4e ("netlink: add a start callback for starting a netlink dump")
> 1137b5e2529a8 ("ipsec: Fix aborted xfrm policy dump crash")
> 
> My apologies for the noise if this is already under consideration.

Ah, thanks for this, I had tried this a few times, and asked around, but
missed that just adding the first patch here would solve the issue.
Both are now queued up, thanks for bringing this up again.

greg k-h

^ permalink raw reply

* Re: [PATCH net-next] cxgb4vf: Fix netdev_features flag
From: Alexander Duyck @ 2017-12-01 15:16 UTC (permalink / raw)
  To: Ganesh Goudar
  Cc: Netdev, David Miller, nirranjan, indranil, venkatesh,
	Arjun Vynipadath
In-Reply-To: <1512140371-18062-1-git-send-email-ganeshgr@chelsio.com>

On Fri, Dec 1, 2017 at 6:59 AM, Ganesh Goudar <ganeshgr@chelsio.com> wrote:
> GRO is not supported by Chelsio HW when rx_csum is disabled.
> Update the netdev features flag when rx_csum is modified.
>
> Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> ---
>  drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> index b48361c..8e993ac 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> @@ -1114,6 +1114,10 @@ static int cxgb4vf_change_mtu(struct net_device *dev, int new_mtu)
>  static netdev_features_t cxgb4vf_fix_features(struct net_device *dev,
>         netdev_features_t features)
>  {
> +       /* Disable GRO, if RX_CSUM is disabled */
> +       if (!(features & NETIF_F_RXCSUM))
> +               features &= ~NETIF_F_GRO;
> +
>         /*
>          * Since there is no support for separate rx/tx vlan accel
>          * enable/disable make sure tx flag is always in same state as rx.

Wouldn't it make more sense to just put this in netdev_fix_features? I
would thing GRO cannot be supported on any device if RX_CSUM is
disabled since GRO depends on checksum validation from the hardware
before it can aggregate the frame.

- Alex

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: Allow compiling out legacy support
From: Vivien Didelot @ 2017-12-01 15:21 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: Florian Fainelli, Andrew Lunn, David S. Miller, open list
In-Reply-To: <20171130235857.12809-1-f.fainelli@gmail.com>

Hi Florian,

Florian Fainelli <f.fainelli@gmail.com> writes:

> +#if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
>  /* Legacy driver registration */
>  void register_switch_driver(struct dsa_switch_driver *type);
>  void unregister_switch_driver(struct dsa_switch_driver *type);
>  struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev);
>  
> +#else
> +static inline void register_switch_driver(struct dsa_switch_driver *type) { }
> +static inline void unregister_switch_driver(struct dsa_switch_driver *type) { }
> +static inline struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
> +{
> +	return NULL;
> +}
> +#endif

The .probe dsa_switch_ops is part of the legacy code, we may want to
wrap it in a CONFIG_NET_DSA_LEGACY check as well.

>  struct net_device *dsa_dev_to_net_device(struct device *dev);
>  
>  /* Keep inline for faster access in hot path */
> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 03c3bdf25468..b6ec8e9069e4 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -16,6 +16,14 @@ config NET_DSA
>  
>  if NET_DSA
>  
> +config NET_DSA_LEGACY

We need to have it enabled by default, otherwise we'll miss errors when
touching the code shared by both legacy and new bindings.

> +	bool "Support for older platform device and Device Tree registration"
> +	---help---
> +	  Say Y if you want to enable support for the older platform device and
> +	  deprectaed Device Tree binding registration.

          deprecated*

> +
> +	  This feature is scheduled for removal in 4.17.
> +
>  /* legacy.c */
> +#if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
>  int dsa_legacy_register(void);
>  void dsa_legacy_unregister(void);
>  int dsa_legacy_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> @@ -106,6 +107,28 @@ int dsa_legacy_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>  int dsa_legacy_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>  		       struct net_device *dev,
>  		       const unsigned char *addr, u16 vid);

the dsa_legacy_fdb_{add,del} routines are "legacy" in terms of FDB
handling, not in terms of DSA bindings, we must keep them.

> +#else
> +static inline int dsa_legacy_register(void)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline void dsa_legacy_unregister(void) { }
> +static inline int dsa_legacy_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> +				     struct net_device *dev,
> +				     const unsigned char *addr, u16 vid,
> +				     u16 flags)
> +{
> +	return 0;
> +}
> +
> +static inline int dsa_legacy_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
> +				     struct net_device *dev,
> +				     const unsigned char *addr, u16 vid)
> +{
> +	return 0;
> +}
> +#endif


Thanks,

        Vivien

^ permalink raw reply

* Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out
From: Lars P (Mailing List Account) @ 2017-12-01 15:34 UTC (permalink / raw)
  To: Bhadram Varka
  Cc: Joao.Pinto@synopsys.com, peppe.cavallaro@st.com, linux-netdev
In-Reply-To: <c2796154ffaf40f8932d1864974fc989@bgmail102.nvidia.com>

Hi Bhadram,

Does the Tegra by any chance have TSO enabled on multiple TX-DMA channels ?

I recently noticed a second TSO bug in the stmmac while making the
patch "stmmac: reset last TSO segment size after device open".

The last-used MSS setting in TSO is tracked as a device-global
variable and not per TX queue. Using TSO on tx queue 0 will record mss
to priv->mss and if we later use TSO on tx queue 1 with the same
gso_size then the driver will not use a context descriptor to set the
MSS size for this queue. This probably means that the TSO controller
in channel 1 goes nuts with an undefined mss setting.

BR,
 Lars Persson

On Mon, Nov 20, 2017 at 7:38 AM, Bhadram Varka <vbhadram@nvidia.com> wrote:
> Hi Joao/Peppe,
>
> Observed this issue more frequently with multi-channel case. Am I missing something in DT ?
> Please help here to understand the issue.
>
> Thanks,
> Bhadram
>
> -----Original Message-----
> From: Bhadram Varka
> Sent: Thursday, November 16, 2017 9:41 AM
> To: linux-netdev <netdev@vger.kernel.org>
> Subject: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out
>
> Hi,
>
> I am trying to enable multi-queue in Tegra186 EQOS (which has support for 4 channels). Observed below netdev watchdog warning. Its easily reproable with iperf test.
> In normal ping scenario this is not observed. I did not observe any issue if we disable TSO. Looks like issue in stmmac_tso_xmit() in multi-channel scenario.
>

^ permalink raw reply

* RE: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out
From: Bhadram Varka @ 2017-12-01 15:39 UTC (permalink / raw)
  To: Giuseppe CAVALLARO, Joao.Pinto@synopsys.com; +Cc: linux-netdev
In-Reply-To: <cd003889-64d6-f834-7166-68112aac56fc@st.com>

Hi Giuseppe,

I don't see any issue with if we execute "ping -s 1400" case. I believe in this case TSO not triggered.

Thanks,
Bhadram.

-----Original Message-----
From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com] 
Sent: Thursday, November 23, 2017 11:58 AM
To: Bhadram Varka <vbhadram@nvidia.com>; Joao.Pinto@synopsys.com
Cc: linux-netdev <netdev@vger.kernel.org>
Subject: Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

Hi Bhadram

you said that  In normal ping scenario this is not observed, I wonder if you could try for example, ping with -s 1400. In that case, if still fail I think the issue could be the FIFO tuning and I expect overflow on RX MMC counters.

Let me know
Regards,
Peppe

On 11/20/2017 3:22 PM, Bhadram Varka wrote:
> Hi Giuseppe,
>
> Thanks for responding.
>
> Actually I am using net-next tree for making the changes. Below patches already present in code base.
>
> a0daae1 net: stmmac: Disable flow ctrl for RX AVB queues and really 
> enable TX AVB queues
> 52a7623 net: stmmac: Use correct values in TQS/RQS fields
>
> Thanks,
> Bhadram.
>
> -----Original Message-----
> From: Giuseppe CAVALLARO [mailto:peppe.cavallaro@st.com]
> Sent: Monday, November 20, 2017 6:37 PM
> To: Bhadram Varka <vbhadram@nvidia.com>; Joao.Pinto@synopsys.com
> Cc: linux-netdev <netdev@vger.kernel.org>
> Subject: Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 
> timed out
>
> Hello Bhadram
>
> there are some new patches actually in net/net-next repo that you should have; for example:
>
>      [PATCH net-next v2 0/2] net: stmmac: Improvements for 
> multi-queuing and for AVB
>
> Let me know if these help you.
>
> Regards
> Peppe
>
> On 11/20/2017 7:38 AM, Bhadram Varka wrote:
>> Hi Joao/Peppe,
>>
>> Observed this issue more frequently with multi-channel case. Am I missing something in DT ?
>> Please help here to understand the issue.
>>
>> Thanks,
>> Bhadram
>>
>> -----Original Message-----
>> From: Bhadram Varka
>> Sent: Thursday, November 16, 2017 9:41 AM
>> To: linux-netdev <netdev@vger.kernel.org>
>> Subject: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 
>> timed out
>>
>> Hi,
>>
>> I am trying to enable multi-queue in Tegra186 EQOS (which has support for 4 channels). Observed below netdev watchdog warning. Its easily reproable with iperf test.
>> In normal ping scenario this is not observed. I did not observe any issue if we disable TSO. Looks like issue in stmmac_tso_xmit() in multi-channel scenario.
>>
>> [   88.801672] NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 0 timed out
>> [   88.808818] ------------[ cut here ]------------
>> [   88.813435] WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:320 dev_watchdog+0x2cc/0x2d8
>> [   88.821681] Modules linked in: dwmac_dwc_qos_eth stmmac_platform crc32_ce crct10dif_ce stmmac ip_tables x_tables ipv6
>> [   88.832290] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G S              4.14.0-rc7-01956-g9395db5-dirty #21
>> [   88.841663] Hardware name: NVIDIA Tegra186 P2771-0000 Development Board (DT)
>> [   88.848697] task: ffff8001ec8fd400 task.stack: ffff000009e38000
>> [   88.854606] PC is at dev_watchdog+0x2cc/0x2d8
>> [   88.858952] LR is at dev_watchdog+0x2cc/0x2d8
>> [   88.863300] pc : [<ffff00000894a76c>] lr : [<ffff00000894a76c>] pstate: 20000145
>> [   88.870678] sp : ffff00000802bd80
>> [   88.873983] x29: ffff00000802bd80 x28: 00000000000000a0
>> [   88.879287] x27: 00000000ffffffff x26: ffff8001eae2c3b0
>> [   88.884589] x25: 0000000000000005 x24: ffff8001ecb6be80
>> [   88.889891] x23: ffff8001eae2c39c x22: ffff8001eae2bfb0
>> [   88.895192] x21: ffff8001eae2c000 x20: ffff000008fe7000
>> [   88.900493] x19: 0000000000000001 x18: 0000000000000010
>> [   88.905795] x17: 0000000000000000 x16: 0000000000000000
>> [   88.911098] x15: ffffffffffffffff x14: 756f2064656d6974
>> [   88.916399] x13: 2031206575657571 x12: ffff000008fe9df0
>> [   88.921699] x11: ffff000008586180 x10: 642d6874652d6377
>> [   88.927000] x9 : 0000000000000016 x8 : 3a474f4448435441
>> [   88.932301] x7 : 572056454454454e x6 : 000000000000014f
>> [   88.937602] x5 : 0000000000000020 x4 : 0000000000000000
>> [   88.942902] x3 : 0000000000000000 x2 : ffff000008fec4c0
>> [   88.948203] x1 : ffff8001ec8fd400 x0 : 0000000000000041
>> [   88.953504] Call trace:
>> [   88.955944] Exception stack(0xffff00000802bc40 to 0xffff00000802bd80)
>> [   88.962371] bc40: 0000000000000041 ffff8001ec8fd400 ffff000008fec4c0 0000000000000000
>> [   88.970184] bc60: 0000000000000000 0000000000000020 000000000000014f 572056454454454e
>> [   88.977998] bc80: 3a474f4448435441 0000000000000016 642d6874652d6377 ffff000008586180
>> [   88.985811] bca0: ffff000008fe9df0 2031206575657571 756f2064656d6974 ffffffffffffffff
>> [   88.993624] bcc0: 0000000000000000 0000000000000000 0000000000000010 0000000000000001
>> [   89.001439] bce0: ffff000008fe7000 ffff8001eae2c000 ffff8001eae2bfb0 ffff8001eae2c39c
>> [   89.009252] bd00: ffff8001ecb6be80 0000000000000005 ffff8001eae2c3b0 00000000ffffffff
>> [   89.017065] bd20: 00000000000000a0 ffff00000802bd80 ffff00000894a76c ffff00000802bd80
>> [   89.024879] bd40: ffff00000894a76c 0000000020000145 ffff000000b67570 0000000000000001
>> [   89.032693] bd60: 0001000000000000 ffff8001ecb6b200 ffff00000802bd80 ffff00000894a76c
>> [   89.040508] [<ffff00000894a76c>] dev_watchdog+0x2cc/0x2d8
>> [   89.045900] [<ffff000008130d1c>] call_timer_fn.isra.5+0x24/0x80
>> [   89.051809] [<ffff000008130e1c>] expire_timers+0xa4/0xb0
>> [   89.057111] [<ffff000008130f68>] run_timer_softirq+0x140/0x170
>> [   89.062933] [<ffff00000808196c>] __do_softirq+0x12c/0x228
>> [   89.068323] [<ffff0000080ce180>] irq_exit+0xd0/0x108
>> [   89.073278] [<ffff00000811a180>] __handle_domain_irq+0x60/0xb8
>> [   89.079098] [<ffff000008081670>] gic_handle_irq+0x58/0xa8
>> [   89.084484] Exception stack(0xffff000009e3be20 to 0xffff000009e3bf60)
>> [   89.090910] be20: 0000000000000000 0000000000000000 0000000000000001 0000000000000000
>> [   89.098724] be40: 0000000000000000 ffff000009e3bf60 00008001ecffd000 0000000000000001
>> [   89.106537] be60: 0000000000000002 ffff000009e3bee0 0000000000000a00 0000000000000000
>> [   89.114351] be80: 0000000000000001 0000000000000000 001c3dfbd9959589 00001daf5b7a4860
>> [   89.122164] bea0: ffff00000825b000 0000000000000000 0000ffffc0311284 ffff000008fc5000
>> [   89.129978] bec0: ffff000008fe9000 ffff000008fe9000 ffff000008fd04a0 ffff000008fe9e90
>> [   89.137792] bee0: 0000000000000000 0000000000000000 ffff8001ec8fd400 0000000000000000
>> [   89.145605] bf00: 0000000000000000 ffff000009e3bf60 ffff00000808548c ffff000009e3bf60
>> [   89.153418] bf20: ffff000008085490 0000000000000145 0000000000000000 0000000000000000
>> [   89.161231] bf40: ffffffffffffffff ffff0000081409c4 ffff000009e3bf60 ffff000008085490
>> [   89.169044] [<ffff0000080830f0>] el1_irq+0xb0/0x124
>> [   89.173912] [<ffff000008085490>] arch_cpu_idle+0x10/0x18
>> [   89.179213] [<ffff000008105f10>] do_idle+0x120/0x1e0
>> [   89.184166] [<ffff00000810616c>] cpu_startup_entry+0x24/0x28
>> [   89.189814] [<ffff00000808f1c8>] secondary_start_kernel+0x110/0x120
>> [   89.196067] ---[ end trace 039d403d63546b77 ]---
>>    
>> Below are the DT changes -
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>> b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>> index 0b0552c..ffe1b80 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
>> @@ -27,21 +27,40 @@
>>                   #gpio-cells = <2>;
>>                   gpio-controller;
>>           };
>> +
>> +       mtl_tx_setup: tx-queues-config {
>> +               snps,tx-queues-to-use = <0x4>;
>> +               snps,tx-sched-sp;
>> +               queue0 {
>> +                       snps,weight = <0x10>;
>> +                       snps,dcb-algorithm;
>> +                       snps,priority = <0x0>;
>> +               };
>> +               queue1 {
>> +                       snps,avb-algorithm;
>> +                       snps,send_slope = <0x1000>;
>> +                       snps,idle_slope = <0x1000>;
>> +                       snps,high_credit = <0x3E800>;
>> +                       snps,low_credit = <0xFFC18000>;
>> +                       snps,priority = <0x1>;
>> +               };
>> +       };
>> +
>> +       mtl_rx_setup: rx-queues-config {
>> +               snps,rx-queues-to-use = <0x4>;
>> +               snps,rx-sched-sp;
>> +               queue0 {
>> +                       snps,dcb-algorithm;
>> +                       snps,map-to-dma-channel = <0x0>;
>> +                       snps,priority = <0x0>;
>> +               };
>> +       };
>>          ethernet@2490000 {
>>                   compatible = "nvidia,tegra186-eqos",
>>                                "snps,dwc-qos-ethernet-4.10";
>>                   reg = <0x0 0x02490000 0x0 0x10000>; @@ -57,6 +76,10 @@
>>                   snps,burst-map = <0x7>;
>>                   snps,txpbl = <32>;
>>                   snps,rxpbl = <8>;
>> +
>> +               snps,mtl-tx-config = <&mtl_tx_setup>;
>> +               snps,mtl-rx-config = <&mtl_rx_setup>;
>>
> ----------------------------------------------------------------------
> ------------- This email message is for the sole use of the intended 
> recipient(s) and may contain confidential information.  Any 
> unauthorized review, use, disclosure or distribution is prohibited.  
> If you are not the intended recipient, please contact the sender by 
> reply email and destroy all copies of the original message.
> ----------------------------------------------------------------------
> -------------
>

^ permalink raw reply

* RE: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out
From: Bhadram Varka @ 2017-12-01 15:49 UTC (permalink / raw)
  To: Lars P (Mailing List Account)
  Cc: Joao.Pinto@synopsys.com, peppe.cavallaro@st.com, linux-netdev
In-Reply-To: <CADnJP=uxr=tfpP26Z2b6g--TK1cX+f3Qf=W0Q9HmezfHXVXfhQ@mail.gmail.com>

Hi Lars,

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Lars P (Mailing List Account)
> Sent: Friday, December 01, 2017 9:05 PM
> To: Bhadram Varka <vbhadram@nvidia.com>
> Cc: Joao.Pinto@synopsys.com; peppe.cavallaro@st.com; linux-netdev
> <netdev@vger.kernel.org>
> Subject: Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1
> timed out
> 
> Hi Bhadram,
> 
> Does the Tegra by any chance have TSO enabled on multiple TX-DMA
> channels ?

Yes. TSO enabled for multiple TX DMA channels.

> I recently noticed a second TSO bug in the stmmac while making the patch
> "stmmac: reset last TSO segment size after device open".
> 
> The last-used MSS setting in TSO is tracked as a device-global variable and not
> per TX queue. Using TSO on tx queue 0 will record mss to priv->mss and if we
> later use TSO on tx queue 1 with the same gso_size then the driver will not
> use a context descriptor to set the MSS size for this queue. This probably
> means that the TSO controller in channel 1 goes nuts with an undefined mss
> setting.

I believe it would be better we can make the MSS on per queue basis instead of getting through global variable.

Thanks,
Bhadram.
> BR,
>  Lars Persson
> 
> On Mon, Nov 20, 2017 at 7:38 AM, Bhadram Varka <vbhadram@nvidia.com>
> wrote:
> > Hi Joao/Peppe,
> >
> > Observed this issue more frequently with multi-channel case. Am I missing
> something in DT ?
> > Please help here to understand the issue.
> >
> > Thanks,
> > Bhadram
> >
> > -----Original Message-----
> > From: Bhadram Varka
> > Sent: Thursday, November 16, 2017 9:41 AM
> > To: linux-netdev <netdev@vger.kernel.org>
> > Subject: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1
> timed
> > out
> >
> > Hi,
> >
> > I am trying to enable multi-queue in Tegra186 EQOS (which has support for
> 4 channels). Observed below netdev watchdog warning. Its easily reproable
> with iperf test.
> > In normal ping scenario this is not observed. I did not observe any issue if
> we disable TSO. Looks like issue in stmmac_tso_xmit() in multi-channel
> scenario.
> >

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply

* Re: [PATCH net] rxrpc: Fix the MAINTAINERS record
From: Joe Perches @ 2017-12-01 15:52 UTC (permalink / raw)
  To: David Howells, netdev; +Cc: linux-afs, linux-kernel
In-Reply-To: <151212663791.11269.11590772430402728422.stgit@warthog.procyon.org.uk>

On Fri, 2017-12-01 at 11:10 +0000, David Howells wrote:
> Fix the MAINTAINERS record so that it's more obvious who the maintainer for
> AF_RXRPC is.

Thanks Davids...

^ permalink raw reply

* Re: [PATCH 1/1] timecounter: Make cyclecounter struct part of timecounter struct
From: Richard Cochran @ 2017-12-01 15:56 UTC (permalink / raw)
  To: Sagar Arun Kamble
  Cc: linux-kernel, alsa-devel, linux-rdma, netdev, Stephen Boyd,
	Chris Wilson, John Stultz, intel-wired-lan, Thomas Gleixner,
	kvmarm, linux-arm-kernel
In-Reply-To: <1512114454-26958-1-git-send-email-sagar.a.kamble@intel.com>

On Fri, Dec 01, 2017 at 01:17:34PM +0530, Sagar Arun Kamble wrote:
> There is no real need for the users of timecounters to define cyclecounter
> and timecounter variables separately. Since timecounter will always be
> based on cyclecounter, have cyclecounter struct as member of timecounter
> struct.

Could you please put the PTP maintainer onto CC?

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH net-next] cxgb4vf: Fix netdev_features flag
From: Ganesh Goudar @ 2017-12-01 15:58 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Netdev, David Miller, nirranjan, indranil, venkatesh,
	Arjun Vynipadath
In-Reply-To: <CAKgT0UcG7PoYrqnhvpBycySoRJVCShDo+MiheMy9CUh4sR-hBQ@mail.gmail.com>

On Friday, December 12/01/17, 2017 at 07:16:38 -0800, Alexander Duyck wrote:
> On Fri, Dec 1, 2017 at 6:59 AM, Ganesh Goudar <ganeshgr@chelsio.com> wrote:
> > GRO is not supported by Chelsio HW when rx_csum is disabled.
> > Update the netdev features flag when rx_csum is modified.
> >
> > Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
> > Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> > ---
> >  drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> > index b48361c..8e993ac 100644
> > --- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> > +++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
> > @@ -1114,6 +1114,10 @@ static int cxgb4vf_change_mtu(struct net_device *dev, int new_mtu)
> >  static netdev_features_t cxgb4vf_fix_features(struct net_device *dev,
> >         netdev_features_t features)
> >  {
> > +       /* Disable GRO, if RX_CSUM is disabled */
> > +       if (!(features & NETIF_F_RXCSUM))
> > +               features &= ~NETIF_F_GRO;
> > +
> >         /*
> >          * Since there is no support for separate rx/tx vlan accel
> >          * enable/disable make sure tx flag is always in same state as rx.
> 
> Wouldn't it make more sense to just put this in netdev_fix_features? I
> would thing GRO cannot be supported on any device if RX_CSUM is
> disabled since GRO depends on checksum validation from the hardware
> before it can aggregate the frame.
> 
> - Alex
Makes sense, but let us see what others has to say.

Ganesh

^ permalink raw reply

* Re: Linux ECN Handling
From: Neal Cardwell @ 2017-12-01 16:35 UTC (permalink / raw)
  To: Steve Ibanez
  Cc: Eric Dumazet, Yuchung Cheng, Daniel Borkmann, Netdev,
	Florian Westphal, Mohammad Alizadeh, Lawrence Brakmo
In-Reply-To: <CACJspmKM+4r+GjvmO68vS9xbrZZN-Kv0e42G9sBngZ7Xj=RyyA@mail.gmail.com>

On Mon, Nov 27, 2017 at 1:49 PM, Steve Ibanez <sibanez@stanford.edu> wrote:
>
> Hi Neal,
>
> I tried out your new suggested patches and indeed it looks like it is
> working. The duration of the freezes looks like it has reduced from an
> RTO to 10ms (tcp probe reports SRTT measurements of about 200us just
> before the freeze). So the PTO value seems to be correctly set to
> max(2*SRTT, 10ms).

Great. Thank you for testing this!

Our team will look into testing these patches more and sending some
version of them upstream if things look good.

Also, BTW, in newer kernels, with bb4d991a28cc ("tcp: adjust tail loss
probe timeout") from July, the TLP timeout should be closer to 2*SRTT
+ 2 jiffies, so if your kernel has 1ms jiffies this should further
improve things.

Thanks,
neal

^ permalink raw reply

* Re: [PATCH iproute2] iproute2: Fix undeclared __kernel_long_t type build error in RHEL 6.8
From: Stephen Hemminger @ 2017-12-01 16:48 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: netdev, Leon Romanovsky, Riad Abo Raed, Guy Ergas
In-Reply-To: <20171201110451.25392-1-leon@kernel.org>

On Fri,  1 Dec 2017 13:04:51 +0200
Leon Romanovsky <leon@kernel.org> wrote:

> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Add asm/posix_types.h header file to the list of needed includes,
> because the headers files in RHEL 6.8 are too old and doesn't
> have declaration of __kernel_long_t.
> 
> In file included from ../include/uapi/linux/kernel.h:5,
>                  from ../include/uapi/linux/netfilter/x_tables.h:4,
>                  from ../include/xtables.h:20,
>                  from em_ipset.c:26:
> ../include/uapi/linux/sysinfo.h:9: error: expected specifier-qualifier-list before ‘__kernel_long_t’
> 
> Cc: Riad Abo Raed <riada@mellanox.com>
> Cc: Guy Ergas <guye@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>

I see the problem, but the solution of dragging in posix_types.h
would be too much of a long term maintenance issue.
All the headers in uapi are regularly generated from upstream
kernel headers; I don't want to start making exceptions.

Is it just the xtables stuff (which has always been problematic)?

^ permalink raw reply

* Re: [PATCH 2/3] fix kcm_clone()
From: Tom Herbert @ 2017-12-01 16:56 UTC (permalink / raw)
  To: Al Viro; +Cc: Linux Kernel Network Developers
In-Reply-To: <20171201002244.GK21978@ZenIV.linux.org.uk>

On Thu, Nov 30, 2017 at 4:22 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 1) it's fput() or sock_release(), not both
> 2) don't do fd_install() until the last failure exit.
> 3) not a bug per se, but... don't attach socket to struct file
>    until it's set up.
>
> Take reserving descriptor into the caller, move fd_install() to the
> caller, sanitize failure exits and calling conventions.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Tom Herbert <tom@herbertland.com>

Thanks for the fix! I'll try to test some today also.

> ---
>  net/kcm/kcmsock.c | 71 +++++++++++++++++++++----------------------------------
>  1 file changed, 27 insertions(+), 44 deletions(-)
>
> diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
> index 0b750a22c4b9..c5fa634e63ca 100644
> --- a/net/kcm/kcmsock.c
> +++ b/net/kcm/kcmsock.c
> @@ -1625,60 +1625,35 @@ static struct proto kcm_proto = {
>  };
>
>  /* Clone a kcm socket. */
> -static int kcm_clone(struct socket *osock, struct kcm_clone *info,
> -                    struct socket **newsockp)
> +static struct file *kcm_clone(struct socket *osock)
>  {
>         struct socket *newsock;
>         struct sock *newsk;
> -       struct file *newfile;
> -       int err, newfd;
> +       struct file *file;
>
> -       err = -ENFILE;
>         newsock = sock_alloc();
>         if (!newsock)
> -               goto out;
> +               return ERR_PTR(-ENFILE);
>
>         newsock->type = osock->type;
>         newsock->ops = osock->ops;
>
>         __module_get(newsock->ops->owner);
>
> -       newfd = get_unused_fd_flags(0);
> -       if (unlikely(newfd < 0)) {
> -               err = newfd;
> -               goto out_fd_fail;
> -       }
> -
> -       newfile = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
> -       if (IS_ERR(newfile)) {
> -               err = PTR_ERR(newfile);
> -               goto out_sock_alloc_fail;
> -       }
> -
>         newsk = sk_alloc(sock_net(osock->sk), PF_KCM, GFP_KERNEL,
>                          &kcm_proto, true);
>         if (!newsk) {
> -               err = -ENOMEM;
> -               goto out_sk_alloc_fail;
> +               sock_release(newsock);
> +               return ERR_PTR(-ENOMEM);
>         }
> -
>         sock_init_data(newsock, newsk);
>         init_kcm_sock(kcm_sk(newsk), kcm_sk(osock->sk)->mux);
>
> -       fd_install(newfd, newfile);
> -       *newsockp = newsock;
> -       info->fd = newfd;
> -
> -       return 0;
> +       file = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
> +       if (IS_ERR(file))
> +               sock_release(newsock);
>
> -out_sk_alloc_fail:
> -       fput(newfile);
> -out_sock_alloc_fail:
> -       put_unused_fd(newfd);
> -out_fd_fail:
> -       sock_release(newsock);
> -out:
> -       return err;
> +       return file;
>  }
>
>  static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> @@ -1708,17 +1683,25 @@ static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
>         }
>         case SIOCKCMCLONE: {
>                 struct kcm_clone info;
> -               struct socket *newsock = NULL;
> -
> -               err = kcm_clone(sock, &info, &newsock);
> -               if (!err) {
> -                       if (copy_to_user((void __user *)arg, &info,
> -                                        sizeof(info))) {
> -                               err = -EFAULT;
> -                               sys_close(info.fd);
> -                       }
> -               }
> +               struct file *file;
> +
> +               info.fd = get_unused_fd_flags(0);
> +               if (unlikely(info.fd < 0))
> +                       return info.fd;
>
> +               file = kcm_clone(sock);
> +               if (IS_ERR(file)) {
> +                       put_unused_fd(info.fd);
> +                       return PTR_ERR(file);
> +               }
> +               if (copy_to_user((void __user *)arg, &info,
> +                                sizeof(info))) {
> +                       put_unused_fd(info.fd);
> +                       fput(file);
> +                       return -EFAULT;
> +               }
> +               fd_install(info.fd, file);
> +               err = 0;
>                 break;
>         }
>         default:
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH net-next 0/5] bpftool: cgroup bpf operations
From: Roman Gushchin @ 2017-12-01 17:06 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, linux-kernel, kernel-team, ast, daniel, kafai
In-Reply-To: <20171130190454.26723962@cakuba.netronome.com>

On Thu, Nov 30, 2017 at 07:04:54PM -0800, Jakub Kicinski wrote:
> Hi Roman!
> 
> On Thu, 30 Nov 2017 13:42:57 +0000, Roman Gushchin wrote:
> > This patchset adds basic cgroup bpf operations to bpftool.
> > 
> > Right now there is no convenient way to perform these operations.
> > The /samples/bpf/load_sock_ops.c implements attach/detacg operations,
> > but only for BPF_CGROUP_SOCK_OPS programs. Bps (part of bcc) implements
> > bpf introspection, but lacks any cgroup-related specific.
> > 
> > I find having a tool to perform these basic operations in the kernel tree
> > very useful, as it can be used in the corresponding bpf documentation
> > without creating additional dependencies. And bpftool seems to be
> > a right tool to extend with such functionality.
> 
> Could you place your code in a new file and add a new "object level"?
> I.e. 
> bpftool cgroup list 
> bpftool cgroup attach ...
> bpftool cgroup help
> etc?  Note that you probably want the list to be first, so if someone
> types "bpftool cg" it runs list by default.
> 
> Does it make sense to support pinned files and specifying programs by
> id?  I used the "id"/"pinned" keywords so that users can choose to use
> either.  Perhaps you should at least prefix the file to with "file"?
> So:
> $ bpftool cgattach file ./mybpfprog.o /sys/fs/cgroup/user.slice/ ingress
> $ bpftool cgattach id 19 /sys/fs/cgroup/user.slice/ ingress
> $ bpftool cgattach pin /bpf/prog /sys/fs/cgroup/user.slice/ ingress
> Would this make sense?
> 
> Smaller nits on the coding style:
>  - please try to run checkpatch, perhaps you did, but some people
>    forget tools are in the kernel tree :)
>  - please keep includes in alphabetical order;
>  - please keep variable declarations in functions ordered longest to
>    shortest, if that's impossible because of dependency between
>    initializers - move the initializers to the code.
> 
> Please also don't forget to update/create new man page.

Ok, I'll try to address these comments in v2.

Thank you!

^ permalink raw reply

* Re: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect
From: Grygorii Strashko @ 2017-12-01 17:07 UTC (permalink / raw)
  To: Russell King - ARM Linux, Yan Markman
  Cc: Antoine Tenart, andrew@lunn.ch, f.fainelli@gmail.com,
	davem@davemloft.net, gregory.clement@free-electrons.com,
	thomas.petazzoni@free-electrons.com,
	miquel.raynal@free-electrons.com, Nadav Haklai, mw@semihalf.com,
	Stefan Chulski, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20171130132830.GA5529@n2100.armlinux.org.uk>

Hi Russell,

On 11/30/2017 07:28 AM, Russell King - ARM Linux wrote:
> On Thu, Nov 30, 2017 at 10:10:18AM +0000, Russell King - ARM Linux wrote:
>> On Thu, Nov 30, 2017 at 08:51:21AM +0000, Yan Markman wrote:
>>> The phylink_stop is called before phylink_disconnect_phy
>>> You could see in mvpp2.c:
>>>
>>> mvpp2_stop_dev() {
>>> 	phylink_stop(port->phylink);
>>> }
>>>
>>> mvpp2_stop()       {
>>> 	mvpp2_stop_dev(port);
>>> 	phylink_disconnect_phy(port->phylink);
>>> }
>>>
>>> .ndo_stop = mvpp2_stop,
>>
>> Sorry, I don't have this in mvpp2.c, so I have no visibility of what
>> you're working with.
>>
>> What you have above looks correct, and I see no reason why the p21
>> patch would not have resolved your issue.  The p21 patch ensures
>> that phylink_resolve() gets called and completes before phylink_stop()
>> returns.  In that case, phylink_resolve() will call the mac_link_down()
>> method if the link is not already down.  It will also print the "Link
>> is Down" message.
>>
>> Florian has already tested this patch after encountering a similar
>> issue, and has reported that it solves the problem for him.  I've also
>> tested it with mvneta, and the original mvpp2x driver on Macchiatobin.
>>
>> Maybe there's something different about mvpp2, but as I have no
>> visibility of that driver and the modifications therein, I can't
>> comment further other than stating that it works for three different
>> implementations.
>>
>> Maybe you could try and work out what's going on with the p21 patch
>> in your case?
> 
> I think I now realise what's probably going on.
> 
> If you call netif_carrier_off() before phylink_stop(), then phylink will
> believe that the link is already down, and so it won't bother calling
> mac_link_down() - it will believe that the link is already down.
> 
> I'll update the documentation for phylink_stop() to spell out this
> aspect.
> 

There are pretty high number of net drivers which do call
	netif_carrier_off(dev);
before
	phy_stop(dev->phydev);
in .ndo_stop() callback.

As per you comment this seems to be incorrect, so should such calls be removed?

-- 
regards,
-grygorii

^ permalink raw reply

* Re: [PATCH net-next 1/4] inet: Add a count to struct inet_listen_hashbucket
From: Eric Dumazet @ 2017-12-01 17:19 UTC (permalink / raw)
  To: Martin KaFai Lau, netdev; +Cc: David S . Miller, Eric Dumazet, Kernel Team
In-Reply-To: <20171130232327.561108-2-kafai@fb.com>

On Thu, 2017-11-30 at 15:23 -0800, Martin KaFai Lau wrote:
> This patch adds a count to the 'struct inet_listen_hashbucket'.
> It counts how many sk is hashed to a bucket.  It will be
> used to decide if the (to-be-added) portaddr listener's hashtable
> should be used during inet[6]_lookup_listener().
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> 

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH 01/11] phy: add phy_interface_mode_is_8023z() helper
From: Florian Fainelli @ 2017-12-01 17:20 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <E1eKiUS-0004yI-1T@rmk-PC.armlinux.org.uk>

On 12/01/2017 02:24 AM, Russell King wrote:
> Add and use phy_interface_mode_is_8023z() helper to identify the
> interface modes that use 802.3z negotiation.  Use it in phylink's
> phylink_mac_an_restart().
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH 02/11] phylink: merge SGMII and 802.3z handling
From: Florian Fainelli @ 2017-12-01 17:21 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <E1eKiUX-0004yP-DC@rmk-PC.armlinux.org.uk>

On 12/01/2017 02:24 AM, Russell King wrote:
> The code handling SGMII and 802.3z is essentially the same, except that
> we assume 802.3z has no PHY.  Re-organise the code such that these cases
> are merged, and exclude 802.3z mode from having a PHY attached.  This
> results in the same link handling behaviour as before.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 2/4] udp: Move udp[46]_portaddr_hash() to net/ip[v6].h
From: Eric Dumazet @ 2017-12-01 17:22 UTC (permalink / raw)
  To: Martin KaFai Lau, netdev; +Cc: David S . Miller, Eric Dumazet, Kernel Team
In-Reply-To: <20171130232327.561108-3-kafai@fb.com>

On Thu, 2017-11-30 at 15:23 -0800, Martin KaFai Lau wrote:
> This patch moves the udp[46]_portaddr_hash()
> to net/ip[v6].h.  The function name is renamed to
> ipv[46]_portaddr_hash().
> 
> It will be used by a later patch which adds a second listener
> hashtable hashed by the address and port.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH 03/11] phylink: get rid of separate Cisco SGMII and 802.3z modes
From: Florian Fainelli @ 2017-12-01 17:22 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <E1eKiUc-0004yW-TB@rmk-PC.armlinux.org.uk>

On 12/01/2017 02:24 AM, Russell King wrote:
> Since the handling of SGMII and 802.3z is now the same, combine the
> MLO_AN_xxx constants into a single MLO_AN_INBAND, and use the PHY
> interface mode to distinguish between Cisco SGMII and 802.3z.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH 04/11] phylink: add support for 2500baseX and 10GbaseKR
From: Florian Fainelli @ 2017-12-01 17:22 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <E1eKiUi-0004yd-9I@rmk-PC.armlinux.org.uk>

On 12/01/2017 02:24 AM, Russell King wrote:
> Add support for handling the faster 2.5G and 10G link modes when used
> with SFP modules.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH 05/11] phylink: remove phylink_init_eee()
From: Florian Fainelli @ 2017-12-01 17:23 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Jonathan Corbet; +Cc: linux-doc, netdev
In-Reply-To: <E1eKiUn-0004yw-KY@rmk-PC.armlinux.org.uk>

On 12/01/2017 02:24 AM, Russell King wrote:
> phylink_init_eee() serves no purpose, remove it.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ 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