Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 0/4] -Wformat fixes
From: David Miller @ 2019-02-23 21:45 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev
In-Reply-To: <20190222040929.16657-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 21 Feb 2019 20:09:25 -0800

> This is a collection of some -Wformat fixes found during build, nothing
> critical, but nice to have for people turning on more warnings with
> their builds.

Applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH net-next] kcm: Remove unnecessary SLAB_PANIC for kmem_cache_create() in kcm_init
From: David Miller @ 2019-02-23 21:46 UTC (permalink / raw)
  To: yuehaibing; +Cc: ktkhai, gregkh, tom, asmadeus, linux-kernel, netdev
In-Reply-To: <20190222061530.9820-1-yuehaibing@huawei.com>

From: Yue Haibing <yuehaibing@huawei.com>
Date: Fri, 22 Feb 2019 14:15:30 +0800

> From: YueHaibing <yuehaibing@huawei.com>
> 
> There has check NULL on kmem_cache_create on failure in kcm_init,
> no need use SLAB_PANIC to panic the system.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

I'll apply this, thank you.

^ permalink raw reply

* Re: [PATCH net v3] net: set static variable an initial value in atl2_probe()
From: David Miller @ 2019-02-23 21:47 UTC (permalink / raw)
  To: maowenan; +Cc: kernel-janitors, chris.snook, jcliburn, dan.carpenter, netdev
In-Reply-To: <20190222065723.193392-1-maowenan@huawei.com>

From: Mao Wenan <maowenan@huawei.com>
Date: Fri, 22 Feb 2019 14:57:23 +0800

> cards_found is a static variable, but when it enters atl2_probe(),
> cards_found is set to zero, the value is not consistent with last probe,
> so next behavior is not our expect.
> 
> Signed-off-by: Mao Wenan <maowenan@huawei.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Heiner Kallweit @ 2019-02-23 21:48 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <20190218182136.GE14879@lunn.ch>

On 18.02.2019 19:21, Andrew Lunn wrote:
>>> Hi Heiner
>>>
>>> Watch out for boot vs reboot, and when rebooting if port 8 had link or
>>> not before you reboot.
>>>
>> Will do. Is there some known issue or bug?
> 
> Hi Heiner
> 
> No, but it is a variable which can make a difference. The fix i made
> for the Freescale GPIO controller was not an issue for cold boot, but
> reboot with link up did cause interrupt problems, etc.
> 
Hi Andrew,

it took me quite some time to debug this issue ..

At first a bisect pointed to one of my commits:
5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")

Further digging lead me to some suspicious dsa code:
In dsa_port_fixed_link_register_of() there's a call to genphy_read_status().
At the time of the call phydev->advertising is empty, therefore the fixed phy
settings are overwritten with defaults (10/half) what breaks the system.

Worth to be mentioned is that for the PHY these two flags are set:
- is_pseudo_fixed_link (that's ok)
- autoneg -> I'm not sure this is correct.

It seems that you once added the code in question:
39b0c705195e ("net: dsa: Allow configuration of CPU & DSA port speeds/duplex")

I did what I like to do most and removed some code.
W/o the calls to genphy_config_init() and genphy_read_status() it works again.
Do these calls have some purpose here with a fixed link?

My commit exposed the issue because before it genphy_read_status() read the
advertisement from chip registers instead of using phydev->advertising.

Very close to this function is dsa_port_setup_phy_of() which uses genphy_resume()
and genphy_read_status() and also looks somewhat suspicious. This code makes
quite some assumptions:
- PHY is a C22 PHY
- PHY is compatible with the generic PHY driver

>        Andrew
> 
Heiner

^ permalink raw reply

* Re: [PATCH net-next 00/10] mlxsw: Add support for new port types and speeds for Spectrum-2
From: David Miller @ 2019-02-23 21:55 UTC (permalink / raw)
  To: idosch; +Cc: netdev, jiri, shalomt, mlxsw
In-Reply-To: <20190222135613.27692-1-idosch@mellanox.com>

From: Ido Schimmel <idosch@mellanox.com>
Date: Fri, 22 Feb 2019 13:56:35 +0000

> This patchset adds support for new port types and speeds for Spectrum-2.
 ...

Series applied, thank you.

^ permalink raw reply

* Re: [pull request][net-next 0/9] Mellanox, mlx5 updates 2019-02-21
From: David Miller @ 2019-02-23 21:56 UTC (permalink / raw)
  To: saeedm; +Cc: netdev
In-Reply-To: <20190222214426.27833-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Fri, 22 Feb 2019 13:44:17 -0800

> This series adds misc updates to mlx5 driver.
> For more information please see tag log below.
> 
> Please pull and let me know if there is any problem.

Pulled, thanks Saeed.

^ permalink raw reply

* Re: [PATCH net-next 0/5] net: phy: aquantia: improve and extend driver
From: David Miller @ 2019-02-23 22:12 UTC (permalink / raw)
  To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <4ae20155-367d-4b11-63f9-28580b7cf1cb@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 22 Feb 2019 23:45:53 +0100

> This series improves and extends the Aquantia PHY driver.

Series applied, thanks Heiner.

^ permalink raw reply

* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Joe Perches @ 2019-02-23 22:13 UTC (permalink / raw)
  To: David Miller, laoar.shao
  Cc: daniel, edumazet, xiyou.wangcong, netdev, shaoyafang
In-Reply-To: <20190223.132117.343025457687449183.davem@davemloft.net>

On Sat, 2019-02-23 at 13:21 -0800, David Miller wrote:
> From: Yafang Shao <laoar.shao@gmail.com>
> Date: Sun, 17 Feb 2019 22:26:32 +0800
> 
> > SOCK_DEBUG() is a old facility for debugging.
> > If the user want to use it for debugging, the user must modify the
> > application first, that doesn't seem like a good way.
> > Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind
> > of debugging purpose.
> > So we'd better disable it by default.
> > The reason why I don't remove it comepletely is that someone may still
> > would like to use it for debugging.
> > 
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Suggested-by: Joe Perches <joe@perches.com>
> 
> Sorry, I'm not applying this.
> 
> You are forcing everyone who wants to use this to add a curstom local
> source code change into their build.

That may not actually be a bad thing.
It could become a CONFIG bit too.

I'm not sure how often it's actually used.

Outside of the likely to be removed tcp uses, it's
currently used only in appletalk/ddp, x25, and the 1 use
in dccp.

All of the existing uses could likely be removed without
much notice by anyone.



^ permalink raw reply

* Re: [PATCH bpf] bpf: properly check TCP_CONGESTION optlen
From: Alexei Starovoitov @ 2019-02-23 23:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Eric Dumazet, Alexei Starovoitov, Daniel Borkmann, netdev,
	Martin KaFai Lau, Song Liu, Yonghong Song, bpf, Lawrence Brakmo
In-Reply-To: <73905026-571a-867d-43eb-3d9b36f6c39c@gmail.com>

On Sat, Feb 23, 2019 at 12:48:53PM -0800, Eric Dumazet wrote:
> 
> 
> On 02/23/2019 12:38 PM, Alexei Starovoitov wrote:
> > On Sat, Feb 23, 2019 at 11:07:09AM -0800, Eric Dumazet wrote:
> >> If caller of bpf_setsockopt() is silly passing a negative optlen
> >> bad things happen.
> >>
> >> Fixes: 91b5b21c7c16 ("bpf: Add support for changing congestion control")
> >> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >> Cc: Lawrence Brakmo <brakmo@fb.com>
> >> ---
> >>  net/core/filter.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/core/filter.c b/net/core/filter.c
> >> index f7d0004fc16096eb42ece3a6acf645540ee2326b..6a5d89464168f2f35f43986c1dbc0446c9390a3c 100644
> >> --- a/net/core/filter.c
> >> +++ b/net/core/filter.c
> >> @@ -4194,8 +4194,9 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> >>  			char name[TCP_CA_NAME_MAX];
> >>  			bool reinit = bpf_sock->op > BPF_SOCK_OPS_NEEDS_ECN;
> >>  
> >> -			strncpy(name, optval, min_t(long, optlen,
> >> -						    TCP_CA_NAME_MAX-1));
> >> +			if (optlen < 0)
> >> +				return -EINVAL;
> >> +			strncpy(name, optval, min(optlen, TCP_CA_NAME_MAX - 1));
> > 
> > Unnecessary.
> > The verifier guarantees that optlen > 0 because
> > static const struct bpf_func_proto bpf_setsockopt_proto = {
> >         .func           = bpf_setsockopt,
> > ...
> >         .arg5_type      = ARG_CONST_SIZE,
> > };
> > 
> 
> Even on 32bit kernel ?
> 
> The suspect thing to me is the min_t(long, ....)
> 
> optlen is an integer, why do we need to promote to a long ?

I think the code is actually fine as-is.
I bet it was copy pasted from do_tcp_setsockopt
where similar min_t(long) is used to match strncpy_from_user() declaration.
Here min_t(long) or min_t(int) or min() doesn't matter.
I would keep it as-is to avoid noisy patches.


^ permalink raw reply

* Re: [PATCH v2 bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: Alexei Starovoitov @ 2019-02-23 23:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Ahern, brakmo, netdev, Martin Lau, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <49fbc4c9-6e8d-5115-0860-e418427a2ce5@gmail.com>

On Sat, Feb 23, 2019 at 12:43:51PM -0800, Eric Dumazet wrote:
> 
> 
> On 02/23/2019 12:40 PM, Alexei Starovoitov wrote:
> > On Sat, Feb 23, 2019 at 10:39:53AM -0800, Eric Dumazet wrote:
> >>
> >>
> >> On 02/22/2019 07:03 PM, David Ahern wrote:
> >>> On 2/22/19 8:06 PM, brakmo wrote:
> >>>> Network Resource Manager is a framework for limiting the bandwidth used
> >>>> by v2 cgroups. It consists of 4 BPF helpers and a sample BPF program to
> >>>> limit egress bandwdith as well as a sample user program and script to
> >>>> simplify NRM testing.
> >>>
> >>> 'resource manager' is a really generic name. Since you are referring to
> >>> bandwidth, how about renaming to Network Bandwidth Manager?
> >>>
> >>
> >> Or just use the normal word for a policer ...
> >>
> >> Really this is beyond me that TCP experts can still push policers out there,
> >> they are really a huge pain.
> > 
> > hmm. please see our NRM presentation at LPC.
> > It is a networking _resource_ management for cgroups.
> > Bandwidth enforcement is a particular example.
> > It's not a policer either.
> > 
> 
> Well, this definitely looks a policer to me, sorry if we disagree, this is fine.

this particular example certainly does look like it. we both agree.
It's overall direction of this work that is aiming to do
network resource management. For example bpf prog may choose
to react on SLA violations in one cgroup by throttling flows
in the other cgroup. Aggregated per-cgroup bandwidth doesn't
need to cross a threshold for bpf prog to take action.
It could do 'work conserving' 'policer'.
I think this set of patches represent a revolutionary approach and existing
networking nomenclature doesn't have precise words to describe it :)
'NRM' describes our goals the best.
Other folks may choose to use it differently, of course.
Note that NRM abbreviation doesn't leak anywhere in uapi.
It's only used in examples. So not sure what we're arguing about.


^ permalink raw reply

* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: kbuild test robot @ 2019-02-23 23:19 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: kbuild-all, David Miller, netdev, Jesper Dangaard Brouer,
	Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <155075021407.13610.6656977312753058829.stgit@alrua-x1>

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

Hi Toke,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/xdp-Always-use-a-devmap-for-XDP_REDIRECT-to-a-device/20190224-054907
config: i386-randconfig-a1-201907 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   net/core/filter.c: In function '__bpf_tx_xdp_map':
>> net/core/filter.c:3355:29: warning: passing argument 2 of '__dev_map_insert_ctx' from incompatible pointer type
      __dev_map_insert_ctx(map, dst);
                                ^
   In file included from include/linux/bpf-cgroup.h:5:0,
                    from include/linux/cgroup-defs.h:22,
                    from include/linux/cgroup.h:28,
                    from include/net/netprio_cgroup.h:17,
                    from include/linux/netdevice.h:46,
                    from include/net/sock.h:51,
                    from include/linux/sock_diag.h:8,
                    from net/core/filter.c:29:
   include/linux/bpf.h:715:20: note: expected 'struct net_device *' but argument is of type 'struct bpf_dtab_netdev *'
    static inline void __dev_map_insert_ctx(struct bpf_map *map,
                       ^

vim +/__dev_map_insert_ctx +3355 net/core/filter.c

  3339	
  3340	static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
  3341				    struct bpf_map *map,
  3342				    struct xdp_buff *xdp,
  3343				    u32 index)
  3344	{
  3345		int err;
  3346	
  3347		switch (map->map_type) {
  3348		case BPF_MAP_TYPE_DEVMAP:
  3349		case BPF_MAP_TYPE_DEVMAP_IDX: {
  3350			struct bpf_dtab_netdev *dst = fwd;
  3351	
  3352			err = dev_map_enqueue(dst, xdp, dev_rx);
  3353			if (unlikely(err))
  3354				return err;
> 3355			__dev_map_insert_ctx(map, dst);
  3356			break;
  3357		}
  3358		case BPF_MAP_TYPE_CPUMAP: {
  3359			struct bpf_cpu_map_entry *rcpu = fwd;
  3360	
  3361			err = cpu_map_enqueue(rcpu, xdp, dev_rx);
  3362			if (unlikely(err))
  3363				return err;
  3364			__cpu_map_insert_ctx(map, index);
  3365			break;
  3366		}
  3367		case BPF_MAP_TYPE_XSKMAP: {
  3368			struct xdp_sock *xs = fwd;
  3369	
  3370			err = __xsk_map_redirect(map, xdp, xs);
  3371			return err;
  3372		}
  3373		default:
  3374			break;
  3375		}
  3376		return 0;
  3377	}
  3378	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27631 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: kbuild test robot @ 2019-02-23 23:28 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: kbuild-all, David Miller, netdev, Jesper Dangaard Brouer,
	Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <155075021407.13610.6656977312753058829.stgit@alrua-x1>

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

Hi Toke,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/xdp-Always-use-a-devmap-for-XDP_REDIRECT-to-a-device/20190224-054907
config: i386-randconfig-l1-02240344 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net//core/filter.c: In function '__bpf_tx_xdp_map':
>> net//core/filter.c:3355:29: error: passing argument 2 of '__dev_map_insert_ctx' from incompatible pointer type [-Werror=incompatible-pointer-types]
      __dev_map_insert_ctx(map, dst);
                                ^
   In file included from include/linux/bpf-cgroup.h:5:0,
                    from include/linux/cgroup-defs.h:22,
                    from include/linux/cgroup.h:28,
                    from include/net/netprio_cgroup.h:17,
                    from include/linux/netdevice.h:46,
                    from include/net/sock.h:51,
                    from include/linux/sock_diag.h:8,
                    from net//core/filter.c:29:
   include/linux/bpf.h:715:20: note: expected 'struct net_device *' but argument is of type 'struct bpf_dtab_netdev *'
    static inline void __dev_map_insert_ctx(struct bpf_map *map,
                       ^
   cc1: some warnings being treated as errors

vim +/__dev_map_insert_ctx +3355 net//core/filter.c

  3339	
  3340	static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
  3341				    struct bpf_map *map,
  3342				    struct xdp_buff *xdp,
  3343				    u32 index)
  3344	{
  3345		int err;
  3346	
  3347		switch (map->map_type) {
  3348		case BPF_MAP_TYPE_DEVMAP:
  3349		case BPF_MAP_TYPE_DEVMAP_IDX: {
  3350			struct bpf_dtab_netdev *dst = fwd;
  3351	
  3352			err = dev_map_enqueue(dst, xdp, dev_rx);
  3353			if (unlikely(err))
  3354				return err;
> 3355			__dev_map_insert_ctx(map, dst);
  3356			break;
  3357		}
  3358		case BPF_MAP_TYPE_CPUMAP: {
  3359			struct bpf_cpu_map_entry *rcpu = fwd;
  3360	
  3361			err = cpu_map_enqueue(rcpu, xdp, dev_rx);
  3362			if (unlikely(err))
  3363				return err;
  3364			__cpu_map_insert_ctx(map, index);
  3365			break;
  3366		}
  3367		case BPF_MAP_TYPE_XSKMAP: {
  3368			struct xdp_sock *xs = fwd;
  3369	
  3370			err = __xsk_map_redirect(map, xdp, xs);
  3371			return err;
  3372		}
  3373		default:
  3374			break;
  3375		}
  3376		return 0;
  3377	}
  3378	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29026 bytes --]

^ permalink raw reply

* Re: [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit
From: kbuild test robot @ 2019-02-23 23:28 UTC (permalink / raw)
  To: wenxu; +Cc: kbuild-all, netdev, davem
In-Reply-To: <1550750930-27138-1-git-send-email-wenxu@ucloud.cn>

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

Hi wenxu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on v5.0-rc4 next-20190222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/wenxu-ucloud-cn/ip_tunnel-Add-ip-tunnel-tunnel_info-dst_cache-in-ip_tunnel_xmit/20190224-065214
config: x86_64-randconfig-x016-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from net/ipv4/ip_tunnel.c:48:
   net/ipv4/ip_tunnel.c: In function 'ip_tunnel_xmit':
>> include/net/ip_tunnels.h:207:15: warning: 'tun_info' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (info->key.tun_flags & TUNNEL_NOCACHE)
         ~~~~~~~~~^~~~~~~~~~
   net/ipv4/ip_tunnel.c:640:25: note: 'tun_info' was declared here
     struct ip_tunnel_info *tun_info;
                            ^~~~~~~~
   net/ipv4/ip_tunnel.c:734:5: warning: 'rt' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (!rt) {
        ^
--
   In file included from net//ipv4/ip_tunnel.c:48:
   net//ipv4/ip_tunnel.c: In function 'ip_tunnel_xmit':
>> include/net/ip_tunnels.h:207:15: warning: 'tun_info' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (info->key.tun_flags & TUNNEL_NOCACHE)
         ~~~~~~~~~^~~~~~~~~~
   net//ipv4/ip_tunnel.c:640:25: note: 'tun_info' was declared here
     struct ip_tunnel_info *tun_info;
                            ^~~~~~~~
   net//ipv4/ip_tunnel.c:734:5: warning: 'rt' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (!rt) {
        ^

vim +/tun_info +207 include/net/ip_tunnels.h

1d8fff90 Thomas Graf     2015-07-21  198  
db3c6139 Daniel Borkmann 2016-03-04  199  static inline bool
db3c6139 Daniel Borkmann 2016-03-04  200  ip_tunnel_dst_cache_usable(const struct sk_buff *skb,
db3c6139 Daniel Borkmann 2016-03-04  201  			   const struct ip_tunnel_info *info)
db3c6139 Daniel Borkmann 2016-03-04  202  {
db3c6139 Daniel Borkmann 2016-03-04  203  	if (skb->mark)
db3c6139 Daniel Borkmann 2016-03-04  204  		return false;
db3c6139 Daniel Borkmann 2016-03-04  205  	if (!info)
db3c6139 Daniel Borkmann 2016-03-04  206  		return true;
db3c6139 Daniel Borkmann 2016-03-04 @207  	if (info->key.tun_flags & TUNNEL_NOCACHE)
db3c6139 Daniel Borkmann 2016-03-04  208  		return false;
db3c6139 Daniel Borkmann 2016-03-04  209  
db3c6139 Daniel Borkmann 2016-03-04  210  	return true;
db3c6139 Daniel Borkmann 2016-03-04  211  }
db3c6139 Daniel Borkmann 2016-03-04  212  

:::::: The code at line 207 was first introduced by commit
:::::: db3c6139e6ead91b42e7c2ad044ed8beaee884e6 bpf, vxlan, geneve, gre: fix usage of dst_cache on xmit

:::::: TO: Daniel Borkmann <daniel@iogearbox.net>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28325 bytes --]

^ permalink raw reply

* r8169 Kernel bug in 4.20.11
From: Ron Murray @ 2019-02-23 23:33 UTC (permalink / raw)
  To: netdev


[-- Attachment #1.1.1: Type: text/plain, Size: 309 bytes --]

Hi.

   I've encountered this bug in the last couple of kernel versions now
(since, say 4.20.8 +/- 0.0.2).

   I don't see a mention in this mailing list/ Apologies if you've
already seen it.

-- 
Ron Murray <rjmx@rjmx.net>
PGP Fingerprint: 4D99 70E3 2317 334B 141E  7B63 12F7 E865 B5E2 E761


[-- Attachment #1.1.2: r8196-crash.txt --]
[-- Type: text/plain, Size: 4966 bytes --]

Feb 21 15:54:04 khufu kernel: ------------[ cut here ]------------
Feb 21 15:54:04 khufu kernel: NETDEV WATCHDOG: enp5s0 (r8169): transmit queue 0 timed out
Feb 21 15:54:04 khufu kernel: WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:461 dev_watchdog+0x211/0x220
Feb 21 15:54:04 khufu kernel: Modules linked in: fuse rfcomm hidp pci_stub vboxpci(O) vboxnetadp(O) vboxnetflt(O) vboxdrv(O) cpufreq_userspace cpufreq_powersave cpufreq_conservative nf_tables cmac bnep nfnetlink_log nfnetlink binfmt_misc nls_iso8859_1 nls_cp437 vfat fat uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common videodev btusb btrtl btbcm btintel media snd_usb_audio bluetooth snd_usbmidi_lib snd_hwdep snd_rawmidi snd_seq_device joydev ecdh_generic nvidia_drm(PO) nvidia_modeset(PO) nvidia(PO) arc4 snd_hda_codec_realtek drm_kms_helper snd_hda_codec_hdmi syscopyarea iwlmvm snd_hda_codec_generic sysfillrect sysimgblt fb_sys_fops mac80211 drm agpgart snd_hda_intel snd_hda_codec snd_hda_core pktcdvd iwlwifi snd_pcm_oss ipmi_devintf ipmi_msghandler cfg80211 snd_mixer_oss snd_pcm snd_timer evdev mxm_wmi rfkill snd kvm_amd soundcore k10temp fam15h_power pcspkr wmi button acpi_cpufreq kvm irqbypass efi_pstore efivars nct6775 hwmon_vid loop nfsd auth_rpcgss lockd grace sunrpc efivarfs
Feb 21 15:54:04 khufu kernel: ip_tables x_tables ipv6 autofs4 overlay btrfs xor raid6_pq algif_skcipher af_alg dm_crypt dm_mod dax hid_logitech_hidpp hid_logitech_dj usbhid crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel pata_acpi aes_x86_64 crypto_simd cryptd glue_helper r8169 realtek libphy pata_atiixp i2c_piix4
Feb 21 15:54:04 khufu kernel: CPU: 5 PID: 0 Comm: swapper/5 Tainted: P           O      4.20.11.khufu #0
Feb 21 15:54:04 khufu kernel: Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./970A-G/3.1, BIOS P1.20 01/12/2016
Feb 21 15:54:04 khufu kernel: RIP: 0010:dev_watchdog+0x211/0x220
Feb 21 15:54:04 khufu kernel: Code: 49 63 4c 24 e0 eb 8d 4c 89 ef c6 05 58 5d a1 00 01 e8 73 32 fd ff 89 d9 48 89 c2 4c 89 ee 48 c7 c7 b8 15 dc bc e8 40 be 9d ff <0f> 0b eb be 66 66 2e 0f 1f 84 00 00 00 00 00 48 c7 47 08 00 00 00
Feb 21 15:54:04 khufu kernel: RSP: 0018:ffffa3154fb43ea0 EFLAGS: 00010286
Feb 21 15:54:04 khufu kernel: RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000006
Feb 21 15:54:04 khufu kernel: RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffffa3154fb552c0
Feb 21 15:54:04 khufu kernel: RBP: ffffa3154d45e41c R08: ffffffffbc4bc310 R09: 00000000000003d8
Feb 21 15:54:04 khufu kernel: R10: 0000000000000007 R11: 0000000000000000 R12: ffffa3154d45e440
Feb 21 15:54:04 khufu kernel: R13: ffffa3154d45e000 R14: 0000000000000001 R15: ffffa31548f2a880
Feb 21 15:54:04 khufu kernel: FS:  0000000000000000(0000) GS:ffffa3154fb40000(0000) knlGS:0000000000000000
Feb 21 15:54:04 khufu kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Feb 21 15:54:04 khufu kernel: CR2: 0000557d6fc1e880 CR3: 00000003c8944000 CR4: 00000000000406e0
Feb 21 15:54:04 khufu kernel: Call Trace:
Feb 21 15:54:04 khufu kernel: <IRQ>
Feb 21 15:54:04 khufu kernel: ? qdisc_put_unlocked+0x30/0x30
Feb 21 15:54:04 khufu kernel: ? qdisc_put_unlocked+0x30/0x30
Feb 21 15:54:04 khufu kernel: call_timer_fn+0x17/0x80
Feb 21 15:54:04 khufu kernel: run_timer_softirq+0x14a/0x190
Feb 21 15:54:04 khufu kernel: ? tick_sched_timer+0x71/0x170
Feb 21 15:54:04 khufu kernel: ? timerqueue_add+0x52/0x80
Feb 21 15:54:04 khufu kernel: ? __hrtimer_run_queues+0x126/0x190
Feb 21 15:54:04 khufu kernel: ? ktime_get+0x31/0x90
Feb 21 15:54:04 khufu kernel: __do_softirq+0xbf/0x235
Feb 21 15:54:04 khufu kernel: irq_exit+0x59/0x80
Feb 21 15:54:04 khufu kernel: smp_apic_timer_interrupt+0x61/0xa0
Feb 21 15:54:04 khufu kernel: apic_timer_interrupt+0xf/0x20
Feb 21 15:54:04 khufu kernel: </IRQ>
Feb 21 15:54:04 khufu kernel: RIP: 0010:cpuidle_enter_state+0x135/0x1e0
Feb 21 15:54:04 khufu kernel: Code: e8 a0 f3 a7 ff 31 ff 48 89 c3 e8 c6 fd a7 ff 45 84 ff 74 12 9c 58 f6 c4 02 0f 85 9c 00 00 00 31 ff e8 ff 34 ac ff fb 45 85 ed <0f> 88 80 00 00 00 48 89 d9 48 b8 ff ff ff ff f3 01 00 00 48 29 e9
Feb 21 15:54:04 khufu kernel: RSP: 0018:ffffb20981927eb0 EFLAGS: 00000202 ORIG_RAX: ffffffffffffff13
Feb 21 15:54:04 khufu kernel: RAX: ffffa3154fb5e200 RBX: 0000362b0e1ab0d5 RCX: 000000000000001f
Feb 21 15:54:04 khufu kernel: RDX: 0000362b0e1ab0d5 RSI: 000000001fffc19c RDI: 0000000000000000
Feb 21 15:54:04 khufu kernel: RBP: 0000362b0e0b8b3a R08: 0000000000000002 R09: 000000000001dac0
Feb 21 15:54:04 khufu kernel: R10: 0000d8c2c0ebe281 R11: 0000000000002000 R12: ffffa3154d0cd600
Feb 21 15:54:04 khufu kernel: R13: 0000000000000002 R14: 0000000000000002 R15: 0000000000000000
Feb 21 15:54:04 khufu kernel: do_idle+0x149/0x1c0
Feb 21 15:54:04 khufu kernel: cpu_startup_entry+0x1b/0x20
Feb 21 15:54:04 khufu kernel: start_secondary+0x18a/0x1c0
Feb 21 15:54:04 khufu kernel: secondary_startup_64+0xa4/0xb0
Feb 21 15:54:04 khufu kernel: ---[ end trace faeefde2824c9f3f ]---

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

^ permalink raw reply

* Re: No traffic with Marvell switch and latest linux-next
From: Andrew Lunn @ 2019-02-23 23:42 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Russell King - ARM Linux admin, Florian Fainelli,
	netdev@vger.kernel.org
In-Reply-To: <188fcef7-81fe-cffc-af71-1f37725b8611@gmail.com>

> it took me quite some time to debug this issue ..
> 
> At first a bisect pointed to one of my commits:
> 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> 
> Further digging lead me to some suspicious dsa code:
> In dsa_port_fixed_link_register_of() there's a call to genphy_read_status().
> At the time of the call phydev->advertising is empty, therefore the fixed phy
> settings are overwritten with defaults (10/half) what breaks the system.
> 
> Worth to be mentioned is that for the PHY these two flags are set:
> - is_pseudo_fixed_link (that's ok)
> - autoneg -> I'm not sure this is correct.
> 
> It seems that you once added the code in question:
> 39b0c705195e ("net: dsa: Allow configuration of CPU & DSA port speeds/duplex")

Hi Heiner

For Ethernet switches, you can have device trees like this:

                       switch0: switch@0 {
                                compatible = "marvell,mv88e6085";
                                pinctrl-0 = <&pinctrl_gpio_switch0>;
                                pinctrl-names = "default";
                                reg = <0>;
                                dsa,member = <0 0>;
                                interrupt-parent = <&gpio0>;
                                interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
                                interrupt-controller;
                                #interrupt-cells = <2>;
                                eeprom-length = <512>;

                                ports {
                                        #address-cells = <1>;
                                        #size-cells = <0>;

                                        port@0 {
                                                reg = <0>;
                                                label = "lan0";
                                                phy-handle = <&switch0phy0>;
                                        };

...
                                        switch0port5: port@5 {
                                                reg = <5>;
                                                label = "dsa";
                                                phy-mode = "rgmii-txid";
                                                link = <&switch1port6
                                                        &switch2port9>;
                                                fixed-link {
                                                        speed = <1000>;
                                                        full-duplex;
                                                };
                                        };

This is a DSA port. It is used to connect two Ethernet switches
together. In this case, the MACs are connected back to back. There are
no PHYs involved. These ports don't have a netdev associated with
them, since they are just pipes between switches, not user interfaces.

If i remember correctly, the code you are looking at was to deal with
the "rgmii-txid". Without the TXID, i could not get frames to pass
between the ports.

There is a second use case as well. The Vybrid FEC ethernet controller
is a Fast Ethernet port. The switch ports are 1G. DSA drivers set the
port connecting to the SoC to its highest speed. Again, there is no
netdev associated to the switch port, and generally the MAC ports are
connected back to back. This 100 vs 1000 does not work for the
Vybrid. So we add a fixed PHY to the CPU port.

                                        port@6 {
                                                reg = <6>;
                                                label = "cpu";
                                                ethernet = <&fec1>;

                                                fixed-link {
                                                        speed = <100>;
                                                        full-duplex;
                                                };
                                        };

And there is a third use case:

                                       port@4 {
                                                reg = <4>;
                                                label = "optical4";

                                                fixed-link {
                                                        speed = <1000>;
                                                        full-duplex;
                                                        link-gpios = <&gpio6 3
                                                              GPIO_ACTIVE_HIGH>;
                                                };
                                        };

We have a GPIO which tells us if the link it up, because there is not
a PHY here, but an optical module. The GPIO tells us about loss of
signal. Unfortunately we cannot use the SFP driver here, because of a
hardware issue.

In all cases end up in the same code. We want to tell the MAC to
configure RGMII delays, or to configure the port speed, or to have the
correct initial link state. The adjust_link() call can do this, if
passed a phydev. And we have a phydev for the fixed-link PHY. However,
since it has never been attached to a netdev, phy_start() called, etc,
the state information is maybe not correct. So we call config_init()
and read_status(), so phydev should reflect the state of the
fixed-link PHY. And a fixed-link PHY is always c22, and it can be
driven by genphy. Take a look at drivers/net/phy/swphy.c which is the
core of the simulation, and fixed_phy.c

> I did what I like to do most and removed some code.
> W/o the calls to genphy_config_init() and genphy_read_status() it works again.
> Do these calls have some purpose here with a fixed link?

Maybe with all the core changes, these calls are no longer needed? We
just need to ensure the state in phydev reflects the state of the
fixed link, i.e. in this case 100 Full.

Looking forward, at some point we are going to have to make fixed-link
support higher speeds. That probably means we need a swphy-c45 which
emulates the standard registers for 2.5G, 5G and 10G. At that point
genphy will not work...

      Andrew

^ permalink raw reply

* [PATCH net] tcp: repaired skbs must init their tso_segs
From: Eric Dumazet @ 2019-02-23 23:51 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Eric Dumazet, Soheil Hassas Yeganeh,
	Neal Cardwell, Yuchung Cheng, syzbot, Andrey Vagin

syzbot reported a WARN_ON(!tcp_skb_pcount(skb))
in tcp_send_loss_probe() [1]

This was caused by TCP_REPAIR sent skbs that inadvertenly
were missing a call to tcp_init_tso_segs()

[1]
WARNING: CPU: 1 PID: 0 at net/ipv4/tcp_output.c:2534 tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.0.0-rc7+ #77
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 <IRQ>
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x172/0x1f0 lib/dump_stack.c:113
 panic+0x2cb/0x65c kernel/panic.c:214
 __warn.cold+0x20/0x45 kernel/panic.c:571
 report_bug+0x263/0x2b0 lib/bug.c:186
 fixup_bug arch/x86/kernel/traps.c:178 [inline]
 fixup_bug arch/x86/kernel/traps.c:173 [inline]
 do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
 do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
Code: 88 fc ff ff 4c 89 ef e8 ed 75 c8 fb e9 c8 fc ff ff e8 43 76 c8 fb e9 63 fd ff ff e8 d9 75 c8 fb e9 94 f9 ff ff e8 bf 03 91 fb <0f> 0b e9 7d fa ff ff e8 b3 03 91 fb 0f b6 1d 37 43 7a 03 31 ff 89
RSP: 0018:ffff8880ae907c60 EFLAGS: 00010206
RAX: ffff8880a989c340 RBX: 0000000000000000 RCX: ffffffff85dedbdb
RDX: 0000000000000100 RSI: ffffffff85dee0b1 RDI: 0000000000000005
RBP: ffff8880ae907c90 R08: ffff8880a989c340 R09: ffffed10147d1ae1
R10: ffffed10147d1ae0 R11: ffff8880a3e8d703 R12: ffff888091b90040
R13: ffff8880a3e8d540 R14: 0000000000008000 R15: ffff888091b90860
 tcp_write_timer_handler+0x5c0/0x8a0 net/ipv4/tcp_timer.c:583
 tcp_write_timer+0x10e/0x1d0 net/ipv4/tcp_timer.c:607
 call_timer_fn+0x190/0x720 kernel/time/timer.c:1325
 expire_timers kernel/time/timer.c:1362 [inline]
 __run_timers kernel/time/timer.c:1681 [inline]
 __run_timers kernel/time/timer.c:1649 [inline]
 run_timer_softirq+0x652/0x1700 kernel/time/timer.c:1694
 __do_softirq+0x266/0x95a kernel/softirq.c:292
 invoke_softirq kernel/softirq.c:373 [inline]
 irq_exit+0x180/0x1d0 kernel/softirq.c:413
 exiting_irq arch/x86/include/asm/apic.h:536 [inline]
 smp_apic_timer_interrupt+0x14a/0x570 arch/x86/kernel/apic/apic.c:1062
 apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
 </IRQ>
RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
Code: ff ff ff 48 89 c7 48 89 45 d8 e8 59 0c a1 fa 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 48 0c a1 fa eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
RSP: 0018:ffff8880a98afd78 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: 1ffffffff1125061 RBX: ffff8880a989c340 RCX: 0000000000000000
RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffff8880a989cbbc
RBP: ffff8880a98afda8 R08: ffff8880a989c340 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
R13: ffffffff889282f8 R14: 0000000000000001 R15: 0000000000000000
 arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
 default_idle_call+0x36/0x90 kernel/sched/idle.c:93
 cpuidle_idle_call kernel/sched/idle.c:153 [inline]
 do_idle+0x386/0x570 kernel/sched/idle.c:262
 cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
 start_secondary+0x404/0x5c0 arch/x86/kernel/smpboot.c:271
 secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
Kernel Offset: disabled
Rebooting in 86400 seconds..

Fixes: 79861919b889 ("tcp: fix TCP_REPAIR xmit queue setup")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Andrey Vagin <avagin@openvz.org>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_output.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 730bc44dbad9363814705b28c2f91a2253d91207..ccc78f3a4b60d3012430488bdfbcfc5122ff8627 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2347,6 +2347,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
 			/* "skb_mstamp_ns" is used as a start point for the retransmit timer */
 			skb->skb_mstamp_ns = tp->tcp_wstamp_ns = tp->tcp_clock_cache;
 			list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
+			tcp_init_tso_segs(skb, mss_now);
 			goto repair; /* Skip network transmission */
 		}
 
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* Re: No traffic with Marvell switch and latest linux-next
From: Florian Fainelli @ 2019-02-23 23:55 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn
  Cc: Russell King - ARM Linux admin, netdev@vger.kernel.org
In-Reply-To: <188fcef7-81fe-cffc-af71-1f37725b8611@gmail.com>

Le 2/23/19 à 1:48 PM, Heiner Kallweit a écrit :
> On 18.02.2019 19:21, Andrew Lunn wrote:
>>>> Hi Heiner
>>>>
>>>> Watch out for boot vs reboot, and when rebooting if port 8 had link or
>>>> not before you reboot.
>>>>
>>> Will do. Is there some known issue or bug?
>>
>> Hi Heiner
>>
>> No, but it is a variable which can make a difference. The fix i made
>> for the Freescale GPIO controller was not an issue for cold boot, but
>> reboot with link up did cause interrupt problems, etc.
>>
> Hi Andrew,
> 
> it took me quite some time to debug this issue ..
> 
> At first a bisect pointed to one of my commits:
> 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")

This is the same issue that Michal reported here:

https://www.spinics.net/lists/netdev/msg552955.html

> 
> Further digging lead me to some suspicious dsa code:
> In dsa_port_fixed_link_register_of() there's a call to genphy_read_status().
> At the time of the call phydev->advertising is empty, therefore the fixed phy
> settings are overwritten with defaults (10/half) what breaks the system.
> 
> Worth to be mentioned is that for the PHY these two flags are set:
> - is_pseudo_fixed_link (that's ok)
> - autoneg -> I'm not sure this is correct.
> 
> It seems that you once added the code in question:
> 39b0c705195e ("net: dsa: Allow configuration of CPU & DSA port speeds/duplex")
> 
> I did what I like to do most and removed some code.
> W/o the calls to genphy_config_init() and genphy_read_status() it works again.
> Do these calls have some purpose here with a fixed link?
> 
> My commit exposed the issue because before it genphy_read_status() read the
> advertisement from chip registers instead of using phydev->advertising.
> 
> Very close to this function is dsa_port_setup_phy_of() which uses genphy_resume()
> and genphy_read_status() and also looks somewhat suspicious. This code makes
> quite some assumptions:
> - PHY is a C22 PHY
> - PHY is compatible with the generic PHY driver
> 
>>        Andrew
>>
> Heiner
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Linus Walleij @ 2019-02-23 23:57 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, netdev, Heiner Kallweit
In-Reply-To: <20190223151723.GC10693@lunn.ch>

On Sat, Feb 23, 2019 at 4:17 PM Andrew Lunn <andrew@lunn.ch> wrote:
> On Sat, Feb 23, 2019 at 03:36:39AM +0100, Linus Walleij wrote:

> > This fixes a regression introduced by
> > commit 0d2e778e38e0ddffab4bb2b0e9ed2ad5165c4bf7
> > "net: phy: replace PHY_HAS_INTERRUPT with a check for
> > config_intr and ack_interrupt".
> >
> > This assumes that a PHY cannot trigger interrupt unless
> > it has .config_intr() or .ack_interrupt() implemented.
> > A later patch makes the code assume both need to be
> > implemented for interrupts to be present.
> >
> > But this PHY (which is inside a DSA) will happily
> > fire interrupts without either callback.
>
> Hi Linus
>
> Can you disable these interrupts?
>
> If you have dummy implementations, what is clearing the interrupt?

They are handled by the irqchip mask/unmask inside
the RTL8366RB, see:
drivers/net/dsa/rtl8366rb.c

So as soon as the phy core request the threaded IRQ
the irqchip will deal with this business on its own.

How exactly the RTL8366RB IRQ machine looks inside
I doubt even Realtek knows themselves, but from
my experiements, they seem all edge triggered,
and the irq will be raised every time an edge occurse
(such as inserting or removing the cable). The "ACK"
happens in hardware when we read the status register
in the nested interrupt handler in rtl8366rb_irq() so no
further registers need to be accessed.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH net] tcp: repaired skbs must init their tso_segs
From: Soheil Hassas Yeganeh @ 2019-02-24  0:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Eric Dumazet, Neal Cardwell,
	Yuchung Cheng, syzbot, Andrey Vagin
In-Reply-To: <20190223235151.168283-1-edumazet@google.com>

On Sat, Feb 23, 2019 at 6:51 PM Eric Dumazet <edumazet@google.com> wrote:
>
> syzbot reported a WARN_ON(!tcp_skb_pcount(skb))
> in tcp_send_loss_probe() [1]
>
> This was caused by TCP_REPAIR sent skbs that inadvertenly
> were missing a call to tcp_init_tso_segs()
>
> [1]
> WARNING: CPU: 1 PID: 0 at net/ipv4/tcp_output.c:2534 tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.0.0-rc7+ #77
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  <IRQ>
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
>  panic+0x2cb/0x65c kernel/panic.c:214
>  __warn.cold+0x20/0x45 kernel/panic.c:571
>  report_bug+0x263/0x2b0 lib/bug.c:186
>  fixup_bug arch/x86/kernel/traps.c:178 [inline]
>  fixup_bug arch/x86/kernel/traps.c:173 [inline]
>  do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
>  do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
> RIP: 0010:tcp_send_loss_probe+0x771/0x8a0 net/ipv4/tcp_output.c:2534
> Code: 88 fc ff ff 4c 89 ef e8 ed 75 c8 fb e9 c8 fc ff ff e8 43 76 c8 fb e9 63 fd ff ff e8 d9 75 c8 fb e9 94 f9 ff ff e8 bf 03 91 fb <0f> 0b e9 7d fa ff ff e8 b3 03 91 fb 0f b6 1d 37 43 7a 03 31 ff 89
> RSP: 0018:ffff8880ae907c60 EFLAGS: 00010206
> RAX: ffff8880a989c340 RBX: 0000000000000000 RCX: ffffffff85dedbdb
> RDX: 0000000000000100 RSI: ffffffff85dee0b1 RDI: 0000000000000005
> RBP: ffff8880ae907c90 R08: ffff8880a989c340 R09: ffffed10147d1ae1
> R10: ffffed10147d1ae0 R11: ffff8880a3e8d703 R12: ffff888091b90040
> R13: ffff8880a3e8d540 R14: 0000000000008000 R15: ffff888091b90860
>  tcp_write_timer_handler+0x5c0/0x8a0 net/ipv4/tcp_timer.c:583
>  tcp_write_timer+0x10e/0x1d0 net/ipv4/tcp_timer.c:607
>  call_timer_fn+0x190/0x720 kernel/time/timer.c:1325
>  expire_timers kernel/time/timer.c:1362 [inline]
>  __run_timers kernel/time/timer.c:1681 [inline]
>  __run_timers kernel/time/timer.c:1649 [inline]
>  run_timer_softirq+0x652/0x1700 kernel/time/timer.c:1694
>  __do_softirq+0x266/0x95a kernel/softirq.c:292
>  invoke_softirq kernel/softirq.c:373 [inline]
>  irq_exit+0x180/0x1d0 kernel/softirq.c:413
>  exiting_irq arch/x86/include/asm/apic.h:536 [inline]
>  smp_apic_timer_interrupt+0x14a/0x570 arch/x86/kernel/apic/apic.c:1062
>  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:807
>  </IRQ>
> RIP: 0010:native_safe_halt+0x2/0x10 arch/x86/include/asm/irqflags.h:58
> Code: ff ff ff 48 89 c7 48 89 45 d8 e8 59 0c a1 fa 48 8b 45 d8 e9 ce fe ff ff 48 89 df e8 48 0c a1 fa eb 82 90 90 90 90 90 90 fb f4 <c3> 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 f4 c3 90 90 90 90 90 90
> RSP: 0018:ffff8880a98afd78 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
> RAX: 1ffffffff1125061 RBX: ffff8880a989c340 RCX: 0000000000000000
> RDX: dffffc0000000000 RSI: 0000000000000001 RDI: ffff8880a989cbbc
> RBP: ffff8880a98afda8 R08: ffff8880a989c340 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
> R13: ffffffff889282f8 R14: 0000000000000001 R15: 0000000000000000
>  arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:555
>  default_idle_call+0x36/0x90 kernel/sched/idle.c:93
>  cpuidle_idle_call kernel/sched/idle.c:153 [inline]
>  do_idle+0x386/0x570 kernel/sched/idle.c:262
>  cpu_startup_entry+0x1b/0x20 kernel/sched/idle.c:353
>  start_secondary+0x404/0x5c0 arch/x86/kernel/smpboot.c:271
>  secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
> Fixes: 79861919b889 ("tcp: fix TCP_REPAIR xmit queue setup")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Andrey Vagin <avagin@openvz.org>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Very nice catch and fix! This was a really cryptic failure. Thank you Eric!

> ---
>  net/ipv4/tcp_output.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 730bc44dbad9363814705b28c2f91a2253d91207..ccc78f3a4b60d3012430488bdfbcfc5122ff8627 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2347,6 +2347,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
>                         /* "skb_mstamp_ns" is used as a start point for the retransmit timer */
>                         skb->skb_mstamp_ns = tp->tcp_wstamp_ns = tp->tcp_clock_cache;
>                         list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
> +                       tcp_init_tso_segs(skb, mss_now);
>                         goto repair; /* Skip network transmission */
>                 }
>
> --
> 2.21.0.rc0.258.g878e2cd30e-goog
>

^ permalink raw reply

* Re: [PATCH] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Andrew Lunn @ 2019-02-24  0:07 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Florian Fainelli, netdev, Heiner Kallweit
In-Reply-To: <CACRpkdaisrc1h-9Pa70bswV0K8PPj=+YGNRBZXq5cqvRNirATw@mail.gmail.com>

> They are handled by the irqchip mask/unmask inside
> the RTL8366RB, see:
> drivers/net/dsa/rtl8366rb.c
> 
> So as soon as the phy core request the threaded IRQ
> the irqchip will deal with this business on its own.
> 
> How exactly the RTL8366RB IRQ machine looks inside
> I doubt even Realtek knows themselves, but from
> my experiements, they seem all edge triggered,
> and the irq will be raised every time an edge occurse
> (such as inserting or removing the cable). The "ACK"
> happens in hardware when we read the status register
> in the nested interrupt handler in rtl8366rb_irq() so no
> further registers need to be accessed.

Hi Linus

Thanks for the explanation. So dummy functions are fine in this case.

However, in general, i don't think dummy functions will work for a PHY
driver, and may lead to interrupt storms. So it might be better to
have them in the driver, not the core, with comments about why they
are safe.

     Andrew

^ permalink raw reply

* [PATCH v2] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Linus Walleij @ 2019-02-24  0:11 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli; +Cc: netdev, Linus Walleij, Heiner Kallweit

This fixes a regression introduced by
commit 0d2e778e38e0ddffab4bb2b0e9ed2ad5165c4bf7
"net: phy: replace PHY_HAS_INTERRUPT with a check for
config_intr and ack_interrupt".

This assumes that a PHY cannot trigger interrupt unless
it has .config_intr() or .ack_interrupt() implemented.
A later patch makes the code assume both need to be
implemented for interrupts to be present.

But this PHY (which is inside a DSA) will happily
fire interrupts without either callback.

Implement dummy callbacks for .config_intr() and
.ack_interrupt() in the phy header to fix this.

Tested on the RTL8366RB on D-Link DIR-685.

Fixes: 0d2e778e38e0 ("net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt")
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Define generic inline helpers for
  genphy_no_ack_interrupt and genphy_no_config_intr
- Add a comment block explaining how interrupts are
  unmasked and ACKed in the RTL8366RB internal irqchip
  so it is clear what is going on.
---
 drivers/net/phy/realtek.c | 7 +++++++
 include/linux/phy.h       | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index c6010fb1aa0f..cb4a23041a94 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -282,6 +282,13 @@ static struct phy_driver realtek_drvs[] = {
 		.name		= "RTL8366RB Gigabit Ethernet",
 		.features	= PHY_GBIT_FEATURES,
 		.config_init	= &rtl8366rb_config_init,
+		/* These interrupts are handled by the irq controller
+		 * embedded inside the RTL8366RB, they get unmasked when the
+		 * irq is requested and ACKed by reading the status register,
+		 * which is done by the irqchip code.
+		 */
+		.ack_interrupt	= genphy_no_ack_interrupt,
+		.config_intr	= genphy_no_config_intr,
 		.suspend	= genphy_suspend,
 		.resume		= genphy_resume,
 	},
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 127fcc9c3778..333b56d8f746 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -992,6 +992,14 @@ static inline int genphy_no_soft_reset(struct phy_device *phydev)
 {
 	return 0;
 }
+static inline int genphy_no_ack_interrupt(struct phy_device *phydev)
+{
+	return 0;
+}
+static inline int genphy_no_config_intr(struct phy_device *phydev)
+{
+	return 0;
+}
 int genphy_read_mmd_unsupported(struct phy_device *phdev, int devad,
 				u16 regnum);
 int genphy_write_mmd_unsupported(struct phy_device *phdev, int devnum,
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Linus Walleij @ 2019-02-24  0:14 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, netdev, Heiner Kallweit
In-Reply-To: <20190224000727.GB26626@lunn.ch>

On Sun, Feb 24, 2019 at 1:07 AM Andrew Lunn <andrew@lunn.ch> wrote:

> > They are handled by the irqchip mask/unmask inside
> > the RTL8366RB, see:
> > drivers/net/dsa/rtl8366rb.c
> >
> > So as soon as the phy core request the threaded IRQ
> > the irqchip will deal with this business on its own.
> >
> > How exactly the RTL8366RB IRQ machine looks inside
> > I doubt even Realtek knows themselves, but from
> > my experiements, they seem all edge triggered,
> > and the irq will be raised every time an edge occurse
> > (such as inserting or removing the cable). The "ACK"
> > happens in hardware when we read the status register
> > in the nested interrupt handler in rtl8366rb_irq() so no
> > further registers need to be accessed.
>
> Hi Linus
>
> Thanks for the explanation. So dummy functions are fine in this case.
>
> However, in general, i don't think dummy functions will work for a PHY
> driver, and may lead to interrupt storms. So it might be better to
> have them in the driver, not the core, with comments about why they
> are safe.

OK shall I just send a v3 moving them back to the driver so we avoid
confusion on which version should be applied?

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH net-next v3] ip_tunnel: Add ip tunnel tun_info type dst_cache in ip_tunnel_xmit
From: wenxu @ 2019-02-24  0:24 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: wenxu <wenxu@ucloud.cn>

ip l add dev tun type gretap key 1000

Non-tunnel-dst ip tunnel device can send packet through lwtunnel
This patch provide the tun_inf dst cache support for this mode.

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 net/ipv4/ip_tunnel.c | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 893f013..2973067 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -654,14 +654,17 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		    const struct iphdr *tnl_params, u8 protocol)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
+	struct ip_tunnel_info *tun_info = NULL;
 	const struct iphdr *inner_iph;
-	struct flowi4 fl4;
-	u8     tos, ttl;
-	__be16 df;
-	struct rtable *rt;		/* Route to the other host */
 	unsigned int max_headroom;	/* The extra header space needed */
-	__be32 dst;
+	struct rtable *rt = NULL;		/* Route to the other host */
+	bool use_cache = false;
+	struct flowi4 fl4;
+	bool md = false;
 	bool connected;
+	u8 tos, ttl;
+	__be32 dst;
+	__be16 df;
 
 	inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
 	connected = (tunnel->parms.iph.daddr != 0);
@@ -671,7 +674,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	dst = tnl_params->daddr;
 	if (dst == 0) {
 		/* NBMA tunnel */
-		struct ip_tunnel_info *tun_info;
 
 		if (!skb_dst(skb)) {
 			dev->stats.tx_fifo_errors++;
@@ -681,8 +683,11 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		tun_info = skb_tunnel_info(skb);
 		if (tun_info && (tun_info->mode & IP_TUNNEL_INFO_TX) &&
 		    ip_tunnel_info_af(tun_info) == AF_INET &&
-		    tun_info->key.u.ipv4.dst)
+		    tun_info->key.u.ipv4.dst) {
 			dst = tun_info->key.u.ipv4.dst;
+			md = true;
+			connected = true;
+		}
 		else if (skb->protocol == htons(ETH_P_IP)) {
 			rt = skb_rtable(skb);
 			dst = rt_nexthop(rt, inner_iph->daddr);
@@ -721,7 +726,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		else
 			goto tx_error;
 
-		connected = false;
+		if (!md)
+			connected = false;
 	}
 
 	tos = tnl_params->tos;
@@ -743,8 +749,15 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	if (ip_tunnel_encap(skb, tunnel, &protocol, &fl4) < 0)
 		goto tx_error;
 
-	rt = connected ? dst_cache_get_ip4(&tunnel->dst_cache, &fl4.saddr) :
-			 NULL;
+	if (connected && md) {
+		use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
+		if (use_cache)
+			rt = dst_cache_get_ip4(&tun_info->dst_cache,
+					       &fl4.saddr);
+	} else {
+		rt = connected ? dst_cache_get_ip4(&tunnel->dst_cache,
+						&fl4.saddr) : NULL;
+	}
 
 	if (!rt) {
 		rt = ip_route_output_key(tunnel->net, &fl4);
@@ -753,7 +766,10 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 			dev->stats.tx_carrier_errors++;
 			goto tx_error;
 		}
-		if (connected)
+		if (use_cache)
+			dst_cache_set_ip4(&tun_info->dst_cache, &rt->dst,
+					  fl4.saddr);
+		else if (!md && connected)
 			dst_cache_set_ip4(&tunnel->dst_cache, &rt->dst,
 					  fl4.saddr);
 	}
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v2] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Andrew Lunn @ 2019-02-24  1:00 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Florian Fainelli, netdev, Heiner Kallweit
In-Reply-To: <20190224001115.17497-1-linus.walleij@linaro.org>

On Sun, Feb 24, 2019 at 01:11:15AM +0100, Linus Walleij wrote:
> This fixes a regression introduced by
> commit 0d2e778e38e0ddffab4bb2b0e9ed2ad5165c4bf7
> "net: phy: replace PHY_HAS_INTERRUPT with a check for
> config_intr and ack_interrupt".
> 
> This assumes that a PHY cannot trigger interrupt unless
> it has .config_intr() or .ack_interrupt() implemented.
> A later patch makes the code assume both need to be
> implemented for interrupts to be present.
> 
> But this PHY (which is inside a DSA) will happily
> fire interrupts without either callback.
> 
> Implement dummy callbacks for .config_intr() and
> .ack_interrupt() in the phy header to fix this.
> 
> Tested on the RTL8366RB on D-Link DIR-685.
> 
> Fixes: 0d2e778e38e0 ("net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt")
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

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

    Andrew

^ permalink raw reply

* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Cong Wang @ 2019-02-24  1:11 UTC (permalink / raw)
  To: David Miller
  Cc: Yafang Shao, Daniel Borkmann, Eric Dumazet, Joe Perches,
	Linux Kernel Network Developers, shaoyafang
In-Reply-To: <20190223.132117.343025457687449183.davem@davemloft.net>

On Sat, Feb 23, 2019 at 1:21 PM David Miller <davem@davemloft.net> wrote:
>
> You are forcing everyone who wants to use this to add a curstom local
> source code change into their build.

What's wrong with this? People carry custom changes in anyway,
do we really need to care about all the downstream changes?

If yes, can I just add some debugging prink's that no one is going
to use except me? My argument would be you can't force me
to add a custom local source code change.

Can we just play this fairly instead of just for any special people?

Thanks!

^ 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