Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: can: Fix compiling warning
From: Kees Cook @ 2019-08-12 17:19 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Oliver Hartkopp, Patrick Bellasi, linux-sparse, Mao Wenan, davem,
	netdev, linux-kernel, kernel-janitors, Ingo Molnar
In-Reply-To: <20190807105042.GK1974@kadam>

On Wed, Aug 07, 2019 at 01:50:42PM +0300, Dan Carpenter wrote:
> On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote:
> > I compiled the code (the original version), but I do not get that "Should it
> > be static?" warning:
> > 
> > user@box:~/net-next$ make C=1
> >   CALL    scripts/checksyscalls.sh
> >   CALL    scripts/atomic/check-atomics.sh
> >   DESCEND  objtool
> >   CHK     include/generated/compile.h
> >   CHECK   net/can/af_can.c
> > ./include/linux/sched.h:609:43: error: bad integer constant expression
> > ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield
> > `value'
> > ./include/linux/sched.h:610:43: error: bad integer constant expression
> > ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield
> > `bucket_id'
> >   CC [M]  net/can/af_can.o
> 
> The sched.h errors suppress Sparse warnings so it's broken/useless now.
> The code looks like this:
> 
> include/linux/sched.h
>    613  struct uclamp_se {
>    614          unsigned int value              : bits_per(SCHED_CAPACITY_SCALE);
>    615          unsigned int bucket_id          : bits_per(UCLAMP_BUCKETS);
>    616          unsigned int active             : 1;
>    617          unsigned int user_defined       : 1;
>    618  };
> 
> bits_per() is zero and Sparse doesn't like zero sized bitfields.

I just noticed these sparse warnings too -- what's happening here? Are
they _supposed_ to be 0-width fields? It doesn't look like it to me:

CONFIG_UCLAMP_BUCKETS_COUNT=5
...
#define UCLAMP_BUCKETS CONFIG_UCLAMP_BUCKETS_COUNT
...
        unsigned int bucket_id          : bits_per(UCLAMP_BUCKETS);

I would expect this to be 3 bits wide. ... Looks like gcc agrees:

struct uclamp_se {
    unsigned int               value:11;             /*     0: 0  4 */
    unsigned int               bucket_id:3;          /*     0:11  4 */
...

So this is a sparse issue?

-- 
Kees Cook

^ permalink raw reply

* [PATCH net-next] net: devlink: remove redundant rtnl lock assert
From: Vlad Buslov @ 2019-08-12 17:02 UTC (permalink / raw)
  To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, Vlad Buslov, Jiri Pirko

It is enough for caller of devlink_compat_switch_id_get() to hold the net
device to guarantee that devlink port is not destroyed concurrently. Remove
rtnl lock assertion and modify comment to warn user that they must hold
either rtnl lock or reference to net device. This is necessary to
accommodate future implementation of rtnl-unlocked TC offloads driver
callbacks.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index e8f0b891f000..e98e7bd9740b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6938,11 +6938,10 @@ int devlink_compat_switch_id_get(struct net_device *dev,
 {
 	struct devlink_port *devlink_port;
 
-	/* RTNL mutex is held here which ensures that devlink_port
-	 * instance cannot disappear in the middle. No need to take
+	/* Caller must hold RTNL mutex or reference to dev, which ensures that
+	 * devlink_port instance cannot disappear in the middle. No need to take
 	 * any devlink lock as only permanent values are accessed.
 	 */
-	ASSERT_RTNL();
 	devlink_port = netdev_to_devlink_port(dev);
 	if (!devlink_port || !devlink_port->attrs.switch_port)
 		return -EOPNOTSUPP;
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v5 19/29] compat_ioctl: move hci_sock handlers into driver
From: Marcel Holtmann @ 2019-08-12 16:29 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alexander Viro, linux-fsdevel, linux-kernel, Johan Hedberg,
	David S. Miller, Deepa Dinamani, linux-bluetooth, netdev
In-Reply-To: <20190730195819.901457-7-arnd@arndb.de>

Hi Arnd,

> All these ioctl commands are compatible, so we can handle
> them with a trivial wrapper in hci_sock.c and remove
> the listing in fs/compat_ioctl.c.
> 
> A few of the commands pass integer arguments instead of
> pointers, so for correctness skip the compat_ptr() conversion
> here.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> fs/compat_ioctl.c        | 24 ------------------------
> net/bluetooth/hci_sock.c | 21 ++++++++++++++++++++-
> 2 files changed, 20 insertions(+), 25 deletions(-)

I think it is best if this series is applied as a whole. So whoever takes it

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v5 18/29] compat_ioctl: move rfcomm handlers into driver
From: Marcel Holtmann @ 2019-08-12 16:29 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Alexander Viro, linux-fsdevel, linux-kernel, Johan Hedberg,
	David S. Miller, Mauro Carvalho Chehab, linux-bluetooth, netdev
In-Reply-To: <20190730195819.901457-6-arnd@arndb.de>

Hi Arnd,

> All these ioctl commands are compatible, so we can handle
> them with a trivial wrapper in rfcomm/sock.c and remove
> the listing in fs/compat_ioctl.c.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> fs/compat_ioctl.c           |  6 ------
> net/bluetooth/rfcomm/sock.c | 14 ++++++++++++--
> 2 files changed, 12 insertions(+), 8 deletions(-)

I think it is best if this series is applied as a whole. So whoever takes it

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel


^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Roopa Prabhu @ 2019-08-12 16:23 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jiri Pirko, David Ahern, netdev, David Miller, Jakub Kicinski,
	Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
	Saeed Mahameed, mlxsw
In-Reply-To: <20190812084039.2fbd1f01@hermes.lan>

On Mon, Aug 12, 2019 at 8:40 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 12 Aug 2019 10:31:39 +0200
> Jiri Pirko <jiri@resnulli.us> wrote:
>
> > Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> > >On 8/11/19 7:34 PM, David Ahern wrote:
> > >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
> > >>> Could you please write me an example message of add/remove?
> > >>
> > >> altnames are for existing netdevs, yes? existing netdevs have an id and
> > >> a name - 2 existing references for identifying the existing netdev for
> > >> which an altname will be added. Even using the altname as the main
> > >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> > >> not take an the IFLA_ALT_IFNAME and return the full details of the
> > >> device if the altname is unique.
> > >>
> > >> So, what do the new RTM commands give you that you can not do with
> > >> RTM_*LINK?
> > >>
> > >
> > >
> > >To put this another way, the ALT_NAME is an attribute of an object - a
> > >LINK. It is *not* a separate object which requires its own set of
> > >commands for manipulating.
> >
> > Okay, again, could you provide example of a message to add/remove
> > altname using existing setlink message? Thanks!
>
> The existing IFALIAS takes an empty name to do deletion.

yes, if its a single alt name, keeping it similar to IFALIAS will help

^ permalink raw reply

* Re: [PATCH net-next] net: openvswitch: Set OvS recirc_id from tc chain index
From: Pravin Shelar @ 2019-08-12 16:18 UTC (permalink / raw)
  To: Paul Blakey
  Cc: Linux Kernel Network Developers, David S. Miller, Justin Pettit,
	Simon Horman, Marcelo Ricardo Leitner, Vlad Buslov, Jiri Pirko,
	Roi Dayan, Yossi Kuperman, Rony Efraim, Oz Shlomo
In-Reply-To: <b5342e56-4baa-97ab-8694-2f48d012afca@mellanox.com>

On Sun, Aug 11, 2019 at 3:46 AM Paul Blakey <paulb@mellanox.com> wrote:
>
>
> On 8/8/2019 11:53 PM, Pravin Shelar wrote:
> > On Wed, Aug 7, 2019 at 5:08 AM Paul Blakey <paulb@mellanox.com> wrote:
> >> Offloaded OvS datapath rules are translated one to one to tc rules,
> >> for example the following simplified OvS rule:
> >>
> >> recirc_id(0),in_port(dev1),eth_type(0x0800),ct_state(-trk) actions:ct(),recirc(2)
> >>
> >> Will be translated to the following tc rule:
> >>
> >> $ tc filter add dev dev1 ingress \
> >>              prio 1 chain 0 proto ip \
> >>                  flower tcp ct_state -trk \
> >>                  action ct pipe \
> >>                  action goto chain 2
> >>
> >> Received packets will first travel though tc, and if they aren't stolen
> >> by it, like in the above rule, they will continue to OvS datapath.
> >> Since we already did some actions (action ct in this case) which might
> >> modify the packets, and updated action stats, we would like to continue
> >> the proccessing with the correct recirc_id in OvS (here recirc_id(2))
> >> where we left off.
> >>
> >> To support this, introduce a new skb extension for tc, which
> >> will be used for translating tc chain to ovs recirc_id to
> >> handle these miss cases. Last tc chain index will be set
> >> by tc goto chain action and read by OvS datapath.
> >>
> >> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> >> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> >> Acked-by: Jiri Pirko <jiri@mellanox.com>
> >> ---
> >>   include/linux/skbuff.h    | 13 +++++++++++++
> >>   include/net/sch_generic.h |  5 ++++-
> >>   net/core/skbuff.c         |  6 ++++++
> >>   net/openvswitch/flow.c    |  9 +++++++++
> >>   net/sched/Kconfig         | 13 +++++++++++++
> >>   net/sched/act_api.c       |  1 +
> >>   net/sched/cls_api.c       | 12 ++++++++++++
> >>   7 files changed, 58 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> >> index 3aef8d8..fb2a792 100644
> >> --- a/include/linux/skbuff.h
> >> +++ b/include/linux/skbuff.h
> >> @@ -279,6 +279,16 @@ struct nf_bridge_info {
> >>   };
> >>   #endif
> >>
> >> +#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> >> +/* Chain in tc_skb_ext will be used to share the tc chain with
> >> + * ovs recirc_id. It will be set to the current chain by tc
> >> + * and read by ovs to recirc_id.
> >> + */
> >> +struct tc_skb_ext {
> >> +       __u32 chain;
> >> +};
> >> +#endif
> >> +
> >>   struct sk_buff_head {
> >>          /* These two members must be first. */
> >>          struct sk_buff  *next;
> >> @@ -4050,6 +4060,9 @@ enum skb_ext_id {
> >>   #ifdef CONFIG_XFRM
> >>          SKB_EXT_SEC_PATH,
> >>   #endif
> >> +#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> >> +       TC_SKB_EXT,
> >> +#endif
> >>          SKB_EXT_NUM, /* must be last */
> >>   };
> >>
> >> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> >> index 6b6b012..871feea 100644
> >> --- a/include/net/sch_generic.h
> >> +++ b/include/net/sch_generic.h
> >> @@ -275,7 +275,10 @@ struct tcf_result {
> >>                          unsigned long   class;
> >>                          u32             classid;
> >>                  };
> >> -               const struct tcf_proto *goto_tp;
> >> +               struct {
> >> +                       const struct tcf_proto *goto_tp;
> >> +                       u32 goto_index;
> >> +               };
> >>
> >>                  /* used in the skb_tc_reinsert function */
> >>                  struct {
> >> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> >> index ea8e8d3..2b40b5a 100644
> >> --- a/net/core/skbuff.c
> >> +++ b/net/core/skbuff.c
> >> @@ -4087,6 +4087,9 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
> >>   #ifdef CONFIG_XFRM
> >>          [SKB_EXT_SEC_PATH] = SKB_EXT_CHUNKSIZEOF(struct sec_path),
> >>   #endif
> >> +#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> >> +       [TC_SKB_EXT] = SKB_EXT_CHUNKSIZEOF(struct tc_skb_ext),
> >> +#endif
> >>   };
> >>
> >>   static __always_inline unsigned int skb_ext_total_length(void)
> >> @@ -4098,6 +4101,9 @@ static __always_inline unsigned int skb_ext_total_length(void)
> >>   #ifdef CONFIG_XFRM
> >>                  skb_ext_type_len[SKB_EXT_SEC_PATH] +
> >>   #endif
> >> +#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> >> +               skb_ext_type_len[TC_SKB_EXT] +
> >> +#endif
> >>                  0;
> >>   }
> >>
> >> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> >> index bc89e16..0287ead 100644
> >> --- a/net/openvswitch/flow.c
> >> +++ b/net/openvswitch/flow.c
> >> @@ -816,6 +816,9 @@ static int key_extract_mac_proto(struct sk_buff *skb)
> >>   int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
> >>                           struct sk_buff *skb, struct sw_flow_key *key)
> >>   {
> >> +#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> >> +       struct tc_skb_ext *tc_ext;
> >> +#endif
> >>          int res, err;
> >>
> >>          /* Extract metadata from packet. */
> >> @@ -848,7 +851,13 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
> >>          if (res < 0)
> >>                  return res;
> >>          key->mac_proto = res;
> >> +
> >> +#if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
> >> +       tc_ext = skb_ext_find(skb, TC_SKB_EXT);
> >> +       key->recirc_id = tc_ext ? tc_ext->chain : 0;
> >> +#else
> >>          key->recirc_id = 0;
> >> +#endif
> >>
> > Most of cases the config would be turned on, so the ifdef is not that
> > useful. Can you add static key to avoid searching the skb-ext in non
> > offload cases.
>
> Hi,
>
> What do you mean by a static key?
>
https://www.kernel.org/doc/Documentation/static-keys.txt

Static key can be enabled when a flow is added to the tc filter.

^ permalink raw reply

* Re: [PATCH v2 2/2] Documentation: sphinx: Don't parse socket() as identifier reference
From: Mauro Carvalho Chehab @ 2019-08-12 16:11 UTC (permalink / raw)
  To: Jonathan Neuschäfer
  Cc: linux-doc, Jonathan Corbet, Alexei Starovoitov, Daniel Borkmann,
	David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
	John Fastabend, linux-kernel, netdev, xdp-newbies, bpf
In-Reply-To: <20190812160708.32172-2-j.neuschaefer@gmx.net>

Em Mon, 12 Aug 2019 18:07:05 +0200
Jonathan Neuschäfer <j.neuschaefer@gmx.net> escreveu:

> With the introduction of Documentation/sphinx/automarkup.py, socket() is
> parsed as a reference to the in-kernel definition of socket. Sphinx then
> decides that struct socket is a good match, which is usually not
> intended, when the syscall is meant instead. This was observed in
> Documentation/networking/af_xdp.rst.
> 
> Prevent socket() from being misinterpreted by adding it to the Skipfuncs
> list in automarkup.py.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
> 
> v2:
> - block socket() in Documentation/sphinx/automarkup.py, as suggested by
>   Jonathan Corbet
> 
> v1:
> - https://lore.kernel.org/lkml/20190810121738.19587-1-j.neuschaefer@gmx.net/
> ---
>  Documentation/sphinx/automarkup.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
> index a8798369e8f7..5b6119ff69f4 100644
> --- a/Documentation/sphinx/automarkup.py
> +++ b/Documentation/sphinx/automarkup.py
> @@ -26,7 +26,8 @@ RE_function = re.compile(r'([\w_][\w\d_]+\(\))')
>  # just don't even try with these names.
>  #
>  Skipfuncs = [ 'open', 'close', 'read', 'write', 'fcntl', 'mmap',
> -              'select', 'poll', 'fork', 'execve', 'clone', 'ioctl']
> +              'select', 'poll', 'fork', 'execve', 'clone', 'ioctl',
> +              'socket' ]

Both patches sound OK on my eyes. Yet, I would just fold them into
a single one.

In any case:

Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> 
>  #
>  # Find all occurrences of function() and try to replace them with
> --
> 2.20.1
> 



Thanks,
Mauro

^ permalink raw reply

* [PATCH v2 2/2] Documentation: sphinx: Don't parse socket() as identifier reference
From: Jonathan Neuschäfer @ 2019-08-12 16:07 UTC (permalink / raw)
  To: linux-doc
  Cc: Jonathan Neuschäfer, Jonathan Corbet, Alexei Starovoitov,
	Daniel Borkmann, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, Mauro Carvalho Chehab,
	linux-kernel, netdev, xdp-newbies, bpf
In-Reply-To: <20190812160708.32172-1-j.neuschaefer@gmx.net>

With the introduction of Documentation/sphinx/automarkup.py, socket() is
parsed as a reference to the in-kernel definition of socket. Sphinx then
decides that struct socket is a good match, which is usually not
intended, when the syscall is meant instead. This was observed in
Documentation/networking/af_xdp.rst.

Prevent socket() from being misinterpreted by adding it to the Skipfuncs
list in automarkup.py.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---

v2:
- block socket() in Documentation/sphinx/automarkup.py, as suggested by
  Jonathan Corbet

v1:
- https://lore.kernel.org/lkml/20190810121738.19587-1-j.neuschaefer@gmx.net/
---
 Documentation/sphinx/automarkup.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/sphinx/automarkup.py b/Documentation/sphinx/automarkup.py
index a8798369e8f7..5b6119ff69f4 100644
--- a/Documentation/sphinx/automarkup.py
+++ b/Documentation/sphinx/automarkup.py
@@ -26,7 +26,8 @@ RE_function = re.compile(r'([\w_][\w\d_]+\(\))')
 # just don't even try with these names.
 #
 Skipfuncs = [ 'open', 'close', 'read', 'write', 'fcntl', 'mmap',
-              'select', 'poll', 'fork', 'execve', 'clone', 'ioctl']
+              'select', 'poll', 'fork', 'execve', 'clone', 'ioctl',
+              'socket' ]

 #
 # Find all occurrences of function() and try to replace them with
--
2.20.1


^ permalink raw reply related

* Re: [PATCH bpf] s390/bpf: fix lcgr instruction encoding
From: Daniel Borkmann @ 2019-08-12 16:06 UTC (permalink / raw)
  To: Ilya Leoshkevich, bpf, netdev; +Cc: gor, heiko.carstens
In-Reply-To: <20190812150332.98109-1-iii@linux.ibm.com>

On 8/12/19 5:03 PM, Ilya Leoshkevich wrote:
> "masking, test in bounds 3" fails on s390, because
> BPF_ALU64_IMM(BPF_NEG, BPF_REG_2, 0) ignores the top 32 bits of
> BPF_REG_2. The reason is that JIT emits lcgfr instead of lcgr.
> The associated comment indicates that the code was intended to emit lcgr
> in the first place, it's just that the wrong opcode was used.
> 
> Fix by using the correct opcode.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Applied, thanks!

^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: David Ahern @ 2019-08-12 16:01 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
	Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
	Saeed Mahameed, mlxsw
In-Reply-To: <20190812083139.GA2428@nanopsycho>

On 8/12/19 2:31 AM, Jiri Pirko wrote:
> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
>> On 8/11/19 7:34 PM, David Ahern wrote:
>>> On 8/10/19 12:30 AM, Jiri Pirko wrote:
>>>> Could you please write me an example message of add/remove?
>>>
>>> altnames are for existing netdevs, yes? existing netdevs have an id and
>>> a name - 2 existing references for identifying the existing netdev for
>>> which an altname will be added. Even using the altname as the main
>>> 'handle' for a setlink change, I see no reason why the GETLINK api can
>>> not take an the IFLA_ALT_IFNAME and return the full details of the
>>> device if the altname is unique.
>>>
>>> So, what do the new RTM commands give you that you can not do with
>>> RTM_*LINK?
>>>
>>
>>
>> To put this another way, the ALT_NAME is an attribute of an object - a
>> LINK. It is *not* a separate object which requires its own set of
>> commands for manipulating.
> 
> Okay, again, could you provide example of a message to add/remove
> altname using existing setlink message? Thanks!
> 

Examples from your cover letter with updates

$ ip link set dummy0 altname someothername
$ ip link set dummy0 altname someotherveryveryveryverylongname

$ ip link set dummy0 del altname someothername
$ ip link set dummy0 del altname someotherveryveryveryverylongname

This syntactic sugar to what is really happening:

RTM_NEWLINK, dummy0, IFLA_ALT_IFNAME

if you are allowing many alt names, then yes, you need a flag to say
delete this specific one which is covered by Roopa's nested suggestion.

^ permalink raw reply

* Re: [PATCHv2 net 0/2] Add netdev_level_ratelimited to avoid netdev msg flush
From: Thomas Falcon @ 2019-08-12 15:56 UTC (permalink / raw)
  To: Hangbin Liu, David Miller; +Cc: netdev, joe
In-Reply-To: <20190812073733.GV18865@dhcp-12-139.nay.redhat.com>


On 8/12/19 2:37 AM, Hangbin Liu wrote:
> On Sun, Aug 11, 2019 at 09:08:20PM -0700, David Miller wrote:
>> From: Hangbin Liu <liuhangbin@gmail.com>
>> Date: Fri,  9 Aug 2019 08:29:39 +0800
>>
>>> ibmveth 30000003 env3: h_multicast_ctrl rc=4 when adding an entry to the filter table
>>> error when add more thann 256 memberships in one multicast group. I haven't
>>> found this issue on other driver. It looks like an ibm driver issue and need
>>> to be fixed separately.
>> You need to root cause and fix the reason this message appears so much.
>>
>> Once I let you rate limit the message you will have zero incentive to
>> fix the real problem and fix it.
> Sorry, I'm not familiar with ibmveth driver...
>
> Hi Thomas,
>
> Would you please help check why this issue happens


Hi, thanks for reporting this. I was able to recreate this on my own 
system. The virtual ethernet's multicast filter list size is limited, 
and the driver will check that there is available space before adding 
entries.  The problem is that the size is encoded as big endian, but the 
driver does not convert it for little endian systems after retrieving it 
from the device tree.  As a result the driver is requesting more than 
the hypervisor can allow and getting this error in reply. I will submit 
a patch to correct this soon.

Thanks again,

Tom

> Thanks
> Hangbbin
>

^ permalink raw reply

* Re: [PATCH v3 16/17] ixgbe: no need to check return value of debugfs_create functions
From: Jeff Kirsher @ 2019-08-12 15:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, netdev; +Cc: David S. Miller, intel-wired-lan
In-Reply-To: <20190810101732.26612-17-gregkh@linuxfoundation.org>

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

On Sat, 2019-08-10 at 12:17 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  .../net/ethernet/intel/ixgbe/ixgbe_debugfs.c  | 22 +++++----------
> ----
>  1 file changed, 5 insertions(+), 17 deletions(-)


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

^ permalink raw reply

* Re: [PATCH v3 15/17] i40e: no need to check return value of debugfs_create functions
From: Jeff Kirsher @ 2019-08-12 15:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman, netdev; +Cc: David S. Miller, intel-wired-lan
In-Reply-To: <20190810101732.26612-16-gregkh@linuxfoundation.org>

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

On Sat, 2019-08-10 at 12:17 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  .../net/ethernet/intel/i40e/i40e_debugfs.c    | 22 ++++-------------
> --
>  1 file changed, 4 insertions(+), 18 deletions(-)


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

^ permalink raw reply

* Re: [PATCH v3 14/17] fm10k: no need to check return value of debugfs_create functions
From: Jeff Kirsher @ 2019-08-12 15:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, netdev; +Cc: David S. Miller, intel-wired-lan
In-Reply-To: <20190810101732.26612-15-gregkh@linuxfoundation.org>

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

On Sat, 2019-08-10 at 12:17 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic
> should
> never do something different based on this.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

> ---
>  drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c | 2 --
>  1 file changed, 2 deletions(-)


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

^ permalink raw reply

* Re: [PATCH bpf] s390/bpf: fix lcgr instruction encoding
From: Vasily Gorbik @ 2019-08-12 15:46 UTC (permalink / raw)
  To: Ilya Leoshkevich; +Cc: bpf, netdev, heiko.carstens
In-Reply-To: <20190812150332.98109-1-iii@linux.ibm.com>

On Mon, Aug 12, 2019 at 05:03:32PM +0200, Ilya Leoshkevich wrote:
> "masking, test in bounds 3" fails on s390, because
> BPF_ALU64_IMM(BPF_NEG, BPF_REG_2, 0) ignores the top 32 bits of
> BPF_REG_2. The reason is that JIT emits lcgfr instead of lcgr.
> The associated comment indicates that the code was intended to emit lcgr
> in the first place, it's just that the wrong opcode was used.
> 
> Fix by using the correct opcode.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  arch/s390/net/bpf_jit_comp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index e636728ab452..6299156f9738 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -863,7 +863,7 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
>  		break;
>  	case BPF_ALU64 | BPF_NEG: /* dst = -dst */
>  		/* lcgr %dst,%dst */
> -		EMIT4(0xb9130000, dst_reg, dst_reg);
> +		EMIT4(0xb9030000, dst_reg, dst_reg);
>  		break;
>  	/*
>  	 * BPF_FROM_BE/LE
> -- 
> 2.21.0
> 
Please add
Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend")
or whatever it should be. With that:
Acked-by: Vasily Gorbik <gor@linux.ibm.com>


^ permalink raw reply

* Re: [PATCH rdma-next 0/4] Add XRQ and SRQ support to DEVX interface
From: Doug Ledford @ 2019-08-12 15:43 UTC (permalink / raw)
  To: Leon Romanovsky, Jason Gunthorpe
  Cc: RDMA mailing list, Edward Srouji, Saeed Mahameed, Yishai Hadas,
	linux-netdev
In-Reply-To: <20190808101059.GC28049@mtr-leonro.mtl.com>

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

On Thu, 2019-08-08 at 10:11 +0000, Leon Romanovsky wrote:
> On Thu, Aug 08, 2019 at 11:43:54AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> > 
> > Hi,
> > 
> > This small series extends DEVX interface with SRQ and XRQ legacy
> > commands.
> 
> Sorry for typo in cover letter, there is no SRQ here.

Series looks fine to me.  Are you planning on the first two via mlx5-
next and the remainder via RDMA tree?

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

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

^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Michal Kubecek @ 2019-08-12 15:43 UTC (permalink / raw)
  To: netdev
  Cc: Roopa Prabhu, Jiri Pirko, David Miller, Jakub Kicinski,
	Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
	Saeed Mahameed, mlxsw
In-Reply-To: <CAJieiUjQNHdgHCsQqMHO3u3DtGRBgZfx0Wo3aUj0LyUm_YJ5Eg@mail.gmail.com>

On Mon, Aug 12, 2019 at 08:21:31AM -0700, Roopa Prabhu wrote:
> On Sun, Aug 11, 2019 at 3:10 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > Not all of them are hardware based, there are also links based on
> > filesystem label or UUID. But my point is rather that udev creates
> > multiple links so that any of them can be used in any place where
> > a block device is to be identified.
> >
> > As network devices can have only one name, udev drops kernel provided
> > name completely and replaces it with name following one naming scheme.
> > Thus we have to know which naming scheme is going to be used and make
> > sure it does not change. With multiple alternative names, we could also
> > have all udev provided names at once (and also the original one from
> > kernel).
> 
> ok, understand the use-case.
> But, Its hard for me to understand how udev is going to manage this
> list of names without structure to them.
> Plus how is udev going to distinguish its own names from user given name ?.
> 
> I thought this list was giving an opportunity to use the long name
> everywhere else.
> But if this is going to be managed by udev with a bunch of structured
> names, I don't understand how the rest of the system is going to use
> these names.
> 
> Maybe we should just call this a udev managed list of names.
> 
> (again, i think the best way to do this for udev is to provide the
> symlink like facility via devlink or any other infra).

I certainly didn't want to suggest for alternative names to be managed
by udev. What I meant was that supporting multiple alternative names
would allow udev to create its names based on e.g. device bus address,
BIOS/UEFI slot number, MAC address etc. But it would still be up to
admins if they want to create their own names.

Michal

^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Stephen Hemminger @ 2019-08-12 15:40 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: David Ahern, Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
	Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
	Saeed Mahameed, mlxsw
In-Reply-To: <20190812083139.GA2428@nanopsycho>

On Mon, 12 Aug 2019 10:31:39 +0200
Jiri Pirko <jiri@resnulli.us> wrote:

> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> >On 8/11/19 7:34 PM, David Ahern wrote:  
> >> On 8/10/19 12:30 AM, Jiri Pirko wrote:  
> >>> Could you please write me an example message of add/remove?  
> >> 
> >> altnames are for existing netdevs, yes? existing netdevs have an id and
> >> a name - 2 existing references for identifying the existing netdev for
> >> which an altname will be added. Even using the altname as the main
> >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> >> not take an the IFLA_ALT_IFNAME and return the full details of the
> >> device if the altname is unique.
> >> 
> >> So, what do the new RTM commands give you that you can not do with
> >> RTM_*LINK?
> >>   
> >
> >
> >To put this another way, the ALT_NAME is an attribute of an object - a
> >LINK. It is *not* a separate object which requires its own set of
> >commands for manipulating.  
> 
> Okay, again, could you provide example of a message to add/remove
> altname using existing setlink message? Thanks!

The existing IFALIAS takes an empty name to do deletion.

^ permalink raw reply

* Re: [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries
From: David Miller @ 2019-08-12 15:28 UTC (permalink / raw)
  To: jiri; +Cc: dsahern, netdev, dsahern
In-Reply-To: <20190812083635.GB2428@nanopsycho>

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 12 Aug 2019 10:36:35 +0200

> I understand it with real devices, but dummy testing device, who's
> purpose is just to test API. Why?

Because you'll break all of the wonderful testing infrastructure
people like David have created.

^ permalink raw reply

* Re: [PATCH v2 0/3] Fix three issues found by syzbot
From: David Miller @ 2019-08-12 15:25 UTC (permalink / raw)
  To: ying.xue
  Cc: netdev, jon.maloy, hdanton, tipc-discussion, syzkaller-bugs,
	jakub.kicinski
In-Reply-To: <1565595162-1383-1-git-send-email-ying.xue@windriver.com>

From: Ying Xue <ying.xue@windriver.com>
Date: Mon, 12 Aug 2019 15:32:39 +0800

> Ying Xue (3):
>   tipc: fix memory leak issue
>   tipc: fix memory leak issue

Please make the subject lines for these two patches unique.  Perhaps
mention what part of the tipc code has the memory leak you are fixing.

Thanks.

^ permalink raw reply

* Re: [RFC PATCH v7] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Jes Sorensen @ 2019-08-12 15:21 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Chris Chiu, davem, linux-wireless, netdev, linux-kernel, linux,
	Daniel Drake
In-Reply-To: <87wofibgk7.fsf@kamboji.qca.qualcomm.com>

On 8/12/19 10:32 AM, Kalle Valo wrote:
> Jes Sorensen <jes.sorensen@gmail.com> writes:
>> Looks good to me! Nice work! I am actually very curious if this will
>> improve performance 8192eu as well.
>>
>> Ideally I'd like to figure out how to make host controlled rates work,
>> but in all my experiments with that, I never really got it to work well.
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@gmail.com>
> 
> This is marked as RFC so I'm not sure what's the plan. Should I apply
> this?

I think it's at a point where it's worth applying - I kinda wish I had
had time to test it, but I won't be near my stash of USB dongles for a
little while.

Cheers,
Jes


^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Roopa Prabhu @ 2019-08-12 15:21 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netdev, Jiri Pirko, David Miller, Jakub Kicinski,
	Stephen Hemminger, David Ahern, dcbw, Andrew Lunn, parav,
	Saeed Mahameed, mlxsw
In-Reply-To: <20190811221027.GD30089@unicorn.suse.cz>

On Sun, Aug 11, 2019 at 3:10 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Sat, Aug 10, 2019 at 12:39:31PM -0700, Roopa Prabhu wrote:
> > On Sat, Aug 10, 2019 at 8:50 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> > >
> > > On Sat, Aug 10, 2019 at 06:46:57AM -0700, Roopa Prabhu wrote:
> > > > On Fri, Aug 9, 2019 at 8:46 AM Michal Kubecek <mkubecek@suse.cz> wrote:
> > > > >
> > > > > On Fri, Aug 09, 2019 at 08:40:25AM -0700, Roopa Prabhu wrote:
> > > > > > to that point, I am also not sure why we have a new API For multiple
> > > > > > names. I mean why support more than two names  (existing old name and
> > > > > > a new name to remove the length limitation) ?
> > > > >
> > > > > One use case is to allow "predictable names" from udev/systemd to work
> > > > > the way do for e.g. block devices, see
> > > > >
> > > > >   http://lkml.kernel.org/r/20190628162716.GF29149@unicorn.suse.cz
> > > > >
> > > >
> > > > thanks for the link. don't know the details about alternate block
> > > > device names. Does user-space generate multiple and assign them to a
> > > > kernel object as proposed in this series ?. is there a limit to number
> > > > of names ?. my understanding of 'predictable names' was still a single
> > > > name but predictable structure to the name.
> > >
> > > It is a single name but IMHO mostly because we can only have one name.
> > > For block devices, udev uses symlinks to create multiple aliases based
> > > on different naming schemes, e.g.
> > >
> > > mike@lion:~> find -L /dev/disk/ -samefile /dev/sda2 -exec ls -l {} +
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/ata-WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/scsi-SATA_WDC_WD30EFRX-68_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/scsi-0ATA_WDC_WD30EFRX-68A_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/scsi-1ATA_WDC_WD30EFRX-68AX9N0_WD-WMC1T3114933-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/scsi-350014ee6589cfea0-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-id/wwn-0x50014ee6589cfea0-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-partlabel/root2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-partuuid/71affb47-a93b-40fd-8986-d2e227e1b39d -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-path/pci-0000:00:11.0-ata-1-part2 -> ../../sda2
> > > lrwxrwxrwx 1 root root 10 srp  5 21:47 /dev/disk/by-path/pci-0000:00:11.0-scsi-0:0:0:0-part2 -> ../../sda2
> > >
> > > Few years ago, udev even dropped support for renaming block and
> > > character devices (NAME="...") so that it now keeps kernel name and only
> > > creates symlinks to it. Recent versions only allow NAME="..." for
> > > network devices.
> >
> > ok thanks for the details. This looks like names that are structured
> > on hardware info which could fall into devlinks scope and they point
> > to a single name.
> > We should think about keeping them under devlink (by-id, by-mac etc).
> > It already can recognize network interfaces by id.
>
> Not all of them are hardware based, there are also links based on
> filesystem label or UUID. But my point is rather that udev creates
> multiple links so that any of them can be used in any place where
> a block device is to be identified.
>
> As network devices can have only one name, udev drops kernel provided
> name completely and replaces it with name following one naming scheme.
> Thus we have to know which naming scheme is going to be used and make
> sure it does not change. With multiple alternative names, we could also
> have all udev provided names at once (and also the original one from
> kernel).

ok, understand the use-case.
But, Its hard for me to understand how udev is going to manage this
list of names without structure to them.
Plus how is udev going to distinguish its own names from user given name ?.

I thought this list was giving an opportunity to use the long name
everywhere else.
But if this is going to be managed by udev with a bunch of structured
names, I don't understand how the rest of the system is going to use
these names.

Maybe we should just call this a udev managed list of names.

(again, i think the best way to do this for udev is to provide the
symlink like facility via devlink or any other infra).

^ permalink raw reply

* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Roopa Prabhu @ 2019-08-12 15:13 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: David Ahern, netdev, David Miller, Jakub Kicinski,
	Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
	Saeed Mahameed, mlxsw
In-Reply-To: <20190812083139.GA2428@nanopsycho>

On Mon, Aug 12, 2019 at 1:31 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Mon, Aug 12, 2019 at 03:37:26AM CEST, dsahern@gmail.com wrote:
> >On 8/11/19 7:34 PM, David Ahern wrote:
> >> On 8/10/19 12:30 AM, Jiri Pirko wrote:
> >>> Could you please write me an example message of add/remove?
> >>
> >> altnames are for existing netdevs, yes? existing netdevs have an id and
> >> a name - 2 existing references for identifying the existing netdev for
> >> which an altname will be added. Even using the altname as the main
> >> 'handle' for a setlink change, I see no reason why the GETLINK api can
> >> not take an the IFLA_ALT_IFNAME and return the full details of the
> >> device if the altname is unique.
> >>
> >> So, what do the new RTM commands give you that you can not do with
> >> RTM_*LINK?
> >>
> >
> >
> >To put this another way, the ALT_NAME is an attribute of an object - a
> >LINK. It is *not* a separate object which requires its own set of
> >commands for manipulating.
>
> Okay, again, could you provide example of a message to add/remove
> altname using existing setlink message? Thanks!

Will the below work ?... just throwing an example for discussion:

make the name list a nested list
IFLA_ALT_NAMES
        IFLA_ALT_NAME_OP /* ADD or DEL used with setlink */
        IFLA_ALT_NAME
        IFLA_ALT_NAME_LIST

With RTM_NEWLINK  you can specify a list to set and unset
With RTM_SETLINK  you can specify an individual name with a add or del op

notifications will always be RTM_NEWLINK with the full list.

The nested attribute can be structured differently.

Only thing is i am worried about increasing the size of link dump and
notification msgs.

What is the limit on the number of names again ?

^ permalink raw reply

* Re: [BUG] fec mdio times out under system stress
From: Fabio Estevam @ 2019-08-12 15:10 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, netdev,
	Andrew Lunn, Florian Fainelli, Heiner Kallweit
In-Reply-To: <20190811133707.GC13294@shell.armlinux.org.uk>

Hi Russell,

On Sun, Aug 11, 2019 at 10:37 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> Hi Fabio,
>
> When I woke up this morning, I found that one of the Hummingboards
> had gone offline (as in, lost network link) during the night.
> Investigating, I find that the system had gone into OOM, and at
> that time, triggered an unrelated:
>
> [4111697.698776] fec 2188000.ethernet eth0: MDIO read timeout
> [4111697.712996] MII_DATA: 0x6006796d
> [4111697.729415] MII_SPEED: 0x0000001a
> [4111697.745232] IEVENT: 0x00000000
> [4111697.745242] IMASK: 0x0a8000aa
> [4111698.002233] Atheros 8035 ethernet 2188000.ethernet-1:00: PHY state change RUNNING -> HALTED
> [4111698.009882] fec 2188000.ethernet eth0: Link is Down
>
> This is on a dual-core iMX6.
>
> It looks like the read actually completed (since MII_DATA contains
> the register data) but we somehow lost the interrupt (or maybe
> received the interrupt after wait_for_completion_timeout() timed
> out.)
>
> From what I can see, the OOM events happened on CPU1, CPU1 was
> allocated the FEC interrupt, and the PHY polling that suffered the
> MDIO timeout was on CPU0.
>
> Given that IEVENT is zero, it seems that CPU1 had read serviced the
> interrupt, but it is not clear how far through processing that it
> was - it may be that fec_enet_interrupt() had been delayed by the
> OOM condition.
>
> This seems rather fragile - as the system slowing down due to OOM
> triggers the network to completely collapse by phylib taking the
> PHY offline, making the system inaccessible except through the
> console.
>
> In my case, even serial console wasn't operational (except for
> magic sysrq).  Not sure what agetty was playing at... so the only
> way I could recover any information from the system was to connect
> the HDMI and plug in a USB keyboard.
>
> Any thoughts on how FEC MDIO accesses could be made more robust?

Sorry for the delay. I am currently on vacation with limited e-mail access.

I think it is worth trying Andrew's suggestion to increase FEC_MII_TIMEOUT.

Thanks

^ permalink raw reply

* Re: [net-next 01/15] ice: Implement ethtool ops for channels
From: Nguyen, Anthony L @ 2019-08-12 15:07 UTC (permalink / raw)
  To: Kirsher, Jeffrey T, jakub.kicinski@netronome.com
  Cc: nhorman@redhat.com, davem@davemloft.net, netdev@vger.kernel.org,
	Bowers, AndrewX, sassmann@redhat.com, Tieman, Henry W
In-Reply-To: <20190809141518.55fe7f8a@cakuba.netronome.com>

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

On Fri, 2019-08-09 at 14:15 -0700, Jakub Kicinski wrote:
> On Fri,  9 Aug 2019 11:31:25 -0700, Jeff Kirsher wrote:
> > From: Henry Tieman <henry.w.tieman@intel.com>
> > 
> > Add code to query and set the number of queues on the primary
> > VSI for a PF. This is accessed from the 'ethtool -l' and 'ethtool
> > -L'
> > commands, respectively.
> > 
> > Signed-off-by: Henry Tieman <henry.w.tieman@intel.com>
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> If you're using the same IRQ vector for RX and TX queue the channel
> counts as combined. Looks like you are counting RX and TX separately
> here. That's incorrect.

Hi Jakub,

The ice driver can support asymmetric queues.  We report these
seperately, as opposed to combined, so that the user can specify a
different number of Rx and Tx queues.

Thanks,
Tony

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3277 bytes --]

^ 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