Netdev List
 help / color / mirror / Atom feed
* RE: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Chris Preimesberger @ 2018-09-27 16:08 UTC (permalink / raw)
  To: Andrew Lunn, Eran Ben Elisha
  Cc: Neil Horman, linville@tuxdriver.com, netdev@vger.kernel.org
In-Reply-To: <20180927153204.GE12979@lunn.ch>

Please correct me if I'm wrong, but...
It looks like Eran's proposed fix would remove all warning and
alarm indications from ethtool's output. It's worth mentioning
that for me, the following fields always reported correctly
as Off while no alarm condition was present
and On while alarm condition(s) were present
*per the QSFP's true/programmed threshold values*
*not per the incorrectly reported threshold values*

         Laser bias current high alarm   (Chan 1)  : Off
         Laser bias current low alarm    (Chan 1)  : Off
         Laser bias current high warning (Chan 1)  : Off
         Laser bias current low warning  (Chan 1)  : Off
         Laser bias current high alarm   (Chan 2)  : Off
         Laser bias current low alarm    (Chan 2)  : Off
         Laser bias current high warning (Chan 2)  : Off
         Laser bias current low warning  (Chan 2)  : Off
         Laser bias current high alarm   (Chan 3)  : Off
         Laser bias current low alarm    (Chan 3)  : Off
         Laser bias current high warning (Chan 3)  : Off
         Laser bias current low warning  (Chan 3)  : Off
         Laser bias current high alarm   (Chan 4)  : Off
         Laser bias current low alarm    (Chan 4)  : Off
         Laser bias current high warning (Chan 4)  : Off
         Laser bias current low warning  (Chan 4)  : Off
         Module temperature high alarm             : Off
         Module temperature low alarm              : Off
         Module temperature high warning           : Off
         Module temperature low warning            : Off
         Module voltage high alarm                 : Off
         Module voltage low alarm                  : Off
         Module voltage high warning               : Off
         Module voltage low warning                : Off
         Laser tx power high alarm   (Channel 1)   : Off
         Laser tx power low alarm    (Channel 1)   : Off
         Laser tx power high warning (Channel 1)   : Off
         Laser tx power low warning  (Channel 1)   : Off
         Laser tx power high alarm   (Channel 2)   : Off
         Laser tx power low alarm    (Channel 2)   : Off
         Laser tx power high warning (Channel 2)   : Off
         Laser tx power low warning  (Channel 2)   : Off
         Laser tx power high alarm   (Channel 3)   : Off
         Laser tx power low alarm    (Channel 3)   : Off
         Laser tx power high warning (Channel 3)   : Off
         Laser tx power low warning  (Channel 3)   : Off
         Laser tx power high alarm   (Channel 4)   : Off
         Laser tx power low alarm    (Channel 4)   : Off
         Laser tx power high warning (Channel 4)   : Off
         Laser tx power low warning  (Channel 4)   : Off
         Laser rx power high alarm   (Channel 1)   : Off
         Laser rx power low alarm    (Channel 1)   : Off
         Laser rx power high warning (Channel 1)   : Off
         Laser rx power low warning  (Channel 1)   : Off
         Laser rx power high alarm   (Channel 2)   : Off
         Laser rx power low alarm    (Channel 2)   : Off
         Laser rx power high warning (Channel 2)   : Off
         Laser rx power low warning  (Channel 2)   : Off
         Laser rx power high alarm   (Channel 3)   : Off
         Laser rx power low alarm    (Channel 3)   : Off
         Laser rx power high warning (Channel 3)   : Off
         Laser rx power low warning  (Channel 3)   : Off
         Laser rx power high alarm   (Channel 4)   : Off
         Laser rx power low alarm    (Channel 4)   : Off
         Laser rx power high warning (Channel 4)   : Off
         Laser rx power low warning  (Channel 4)   : Off


I would like to request that any fix keeps the above information
included in the ethtool -m output because it is working and valuable.

The only values that report incorrectly can be seen
by issuing the command:
ethtool -m interfaceXXX | grep  threshold

Ideally, any fix would display the thresholds correctly
instead of omit them.


Thank you and best regards,
Chris

^ permalink raw reply

* Re: [PATCH ethtool] ethtool: Fix uninitialized variable use at qsfp dump
From: Andrew Lunn @ 2018-09-27 16:08 UTC (permalink / raw)
  To: Eran Ben Elisha
  Cc: netdev, John W. Linville, Chris Preimesberger, Neil Horman,
	Vidya Sagar Ravipati
In-Reply-To: <1538056115-6677-1-git-send-email-eranbe@mellanox.com>

On Thu, Sep 27, 2018 at 04:48:35PM +0300, Eran Ben Elisha wrote:
> Struct sff_diags can be used uninitialized at sff8636_show_dom, this
> caused the tool to show unreported fields (supports_alarms) by the lower
> level driver.
> 
> Fixes: a5e73bb05ee4 ("ethtool:QSFP Plus/QSFP28 Diagnostics Information Support")
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> ---
>  qsfp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qsfp.c b/qsfp.c
> index 32e195d12dc0..d196aa1753de 100644
> --- a/qsfp.c
> +++ b/qsfp.c
> @@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd)
>  
>  static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
>  {
> -	struct sff_diags sd;
> +	struct sff_diags sd = {0};
>  	char *rx_power_string = NULL;
>  	char power_string[MAX_DESC_SIZE];
>  	int i;

Hi Erin

I wonder if similar bugs happen in sfpdiag.c?

void sff8472_show_all(const __u8 *id)
{
        struct sff_diags sd;
        char *rx_power_string = NULL;
        int i;

sd is also on the stack and not initialised. Could you expand your
patch to also clear this one as well, just to be safe.

Thanks
	Andrew

^ permalink raw reply

* Re: KMSAN: uninit-value in __dev_mc_add
From: Eric Dumazet @ 2018-09-27 22:22 UTC (permalink / raw)
  To: vdronov
  Cc: syzbot+001516d86dbe88862cec, David Miller, netdev, LKML,
	syzkaller-bugs
In-Reply-To: <20180927213026.16863-1-vdronov@redhat.com>

On Thu, Sep 27, 2018 at 2:30 PM Vladis Dronov <vdronov@redhat.com> wrote:
>
> Hello,
>
> This report is actually for the same bug which was reported in:
>
> https://syzkaller.appspot.com/bug?id=088efeac32fdde781038a777a63e436c0d4d7036
>
> The note there that the bug was fixed by "Commits: net: fix uninit-value in
> __hw_addr_add_ex()" is wrong. A C-reproducer from the 2nd syzkaller report
> can trigger the bug from this one.
>
> I've researched this and a result is a proposed patch, the problem is the tun
> device code allowing to set an arbitrary link type.
>
> https://lkml.org/lkml/2018/9/26/416
> https://lore.kernel.org/lkml/20180926093018.6646-1-vdronov@redhat.com/T/#u
> https://marc.info/?l=linux-netdev&m=153795423320016&w=2
>

I dunno, your patch looks quite not the right fix.

If TUN is able to change dev->type,  how comes it does not set the
appropriate dev->addr_len at the same time ?

Really the bug seems to be deeper, and without setting proper
dev->addr_len, we'll need more 'fixes' like yours.

Thanks.

^ permalink raw reply

* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Andrew Lunn @ 2018-09-27 15:32 UTC (permalink / raw)
  To: Eran Ben Elisha
  Cc: Neil Horman, Chris Preimesberger, linville@tuxdriver.com,
	netdev@vger.kernel.org
In-Reply-To: <06e89e21-41b2-0711-4ff7-0af5fd7ef53e@mellanox.com>

> Driver return 256 bytes (reading it correctly, I verified it, no overruns),
> however the extra bytes are presented due to this bug (expecting to parse
> 640 bytes).
> 
> Do you see another bug here? Am I missing something?

Hi Erin

Please could you try ethtool -m raw on so you get a binary dump.  The
file which Chris provided had more bytes in it than 256.

Thanks
	Andrew

^ permalink raw reply

* Re: [PATCH iproute2] vxlan: show correct ttl inherit info
From: Phil Sutter @ 2018-09-27 15:22 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, David Ahern
In-Reply-To: <20180927140750.GD24677@leo.usersys.redhat.com>

Hi Hangbin,

On Thu, Sep 27, 2018 at 10:07:51PM +0800, Hangbin Liu wrote:
> On Thu, Sep 27, 2018 at 11:27:45AM +0200, Phil Sutter wrote:
> > On Thu, Sep 27, 2018 at 03:28:36PM +0800, Hangbin Liu wrote:
> > > We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
> > > Otherwise show the ttl number, or auto when it is 0.
> > > 
> > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > > ---
> > >  ip/iplink_vxlan.c | 16 ++++++++++------
> > >  1 file changed, 10 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> > > index 831f39a..7fc0e2b 100644
> > > --- a/ip/iplink_vxlan.c
> > > +++ b/ip/iplink_vxlan.c
> > > @@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> > >  			NEXT_ARG();
> > >  			check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
> > >  			if (strcmp(*argv, "inherit") == 0) {
> > > -				addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
> > > +				addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
> > 
> > So for VXLAN, the attribute is just added but with a zero value. Looking
> > at respective kernel code, this seems fine. Now I wonder why for Geneve,
> > you set the value to 1 and when displaying explicitly check whether the
> > attribute is there *and* non-zero. OK, looks like Geneve driver always
> > exports IFLA_GENEVE_TTL_INHERIT. Oddly, I can't find where VXLAN driver
> > in kernel does export the IFLA_VXLAN_TTL_INHERIT attribute. Am I missing
> > something?
> 
> Hi Phil,
> 
> The vxlan ttl inherit exportation is just fixed by
> net commit 8fd78069874 ("vxlan: fill ttl inherit info") yesterday.

Ah, thanks!

> > Do you know why handling of the attributes in both drivers differ?
> 
> That's because I set IFLA_VXLAN_TTL_INHERIT type to NLA_FLAG. But when fix
> the geneve issue, I forgot this and set IFLA_GENEVE_TTL_INHERIT type to
> NLA_U8... :(
> 
> I noticed this when post the iproute2 patch. But I thought this should have
> no much influence, so I just leave it as it is.
> 
> If you think we'd better have a same behaver. We can drop the geneve iproute2
> patch and post a fix to net kernel tree.

I guess it doesn't make much difference, but NLA_FLAG would be more
appropriate. If you want to spend the extra effort to fix it, I guess
now's the time - later it can't be changed anymore due to compatibility
issues (I guess).

Thanks, Phil

^ permalink raw reply

* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Eran Ben Elisha @ 2018-09-27 15:20 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Neil Horman, Chris Preimesberger, linville@tuxdriver.com,
	netdev@vger.kernel.org
In-Reply-To: <20180927145241.GB12979@lunn.ch>



On 9/27/2018 5:52 PM, Andrew Lunn wrote:
>> Both drivers read up to 256 bytes. 0-127 (from page 0). and 128-256 (from
>> page 0). Driver is not capable of reading over 256 bytes currently.
> 
> Hi Erin
> 
> There should not be any need to read more than 256 bytes. For older
> SFP devices, two addresses on the i2c bus are used, each with 256
> bytes. For QSFP, one address is used, and you swap page by writing to
> offset 127.
> 
>> looking on qsfp.c parser in ethtool.c (user space), I see an uninitialized
>> bug issue that have caused bug #1 + #2.
>> Applied it locally solved the issue (Not showing alarm data, which should be
>> expected as driver do not fill it).
> 
> There appears to be a second bug somewhere. dumping the module info
> using HEX returned 256 bytes. But the binary dump had more bytes.
> Since you have the hardware, could you look into this?

See fix I posted few minutes ago.
title: [PATCH ethtool] ethtool: Fix uninitialized variable use at qsfp dump

This is HEX dump, similar for both with/without the fix:
Offset          Values
------          ------
0x0000:         11 07 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0010:         00 00 00 00 00 00 2a 2a 00 00 7f 0b 00 00 00 00
0x0020:         00 00 38 b6 3e 50 2b e9 40 0d 47 0d 47 ac 48 58
0x0030:         49 0f 3a 09 36 77 39 c9 3a 6a 00 00 00 00 00 00
0x0040:         00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0050:         00 00 00 00 00 00 00 aa aa 00 00 00 00 01 00 00
0x0060:         00 00 ff 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0070:         00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0080:         11 cc 07 80 00 00 00 00 00 00 00 05 ff 00 0a 00
0x0090:         00 00 00 44 4d 65 6c 6c 61 6e 6f 78 20 20 20 20
0x00a0:         20 20 20 20 00 00 02 c9 4d 4d 41 31 4c 31 30 2d
0x00b0:         43 52 20 20 20 20 20 20 41 31 65 bf 00 ce 00 60
0x00c0:         03 07 ff de 4d 54 31 36 33 39 44 4d 30 30 30 32
0x00d0:         36 20 20 20 31 36 30 39 32 36 20 20 0c 10 68 40
0x00e0:         00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00f0:         00 00 00 00 00 00 00 00 00 00 00 00 14 31 00 00

This is parsed output before the fix:
         Identifier                                : 0x11 (QSFP28)
         Extended identifier                       : 0xcc
         Extended identifier description           : 3.5W max. Power 
consumption
         Extended identifier description           : CDR present in TX, 
CDR present in RX
         Extended identifier description           : High Power Class (> 
3.5 W) not enabled
         Connector                                 : 0x07 (LC)
         Transceiver codes                         : 0x80 0x00 0x00 0x00 
0x00 0x00 0x00 0x00
         Transceiver type                          : 100G Ethernet: 100G 
Base-LR4
         Encoding                                  : 0x05 (64B/66B)
         BR, Nominal                               : 25500Mbps
         Rate identifier                           : 0x00
         Length (SMF,km)                           : 10km
         Length (OM3 50um)                         : 0m
         Length (OM2 50um)                         : 0m
         Length (OM1 62.5um)                       : 0m
         Length (Copper or Active cable)           : 0m
         Transmitter technology                    : 0x40 (1310 nm DFB)
         Laser wavelength                          : 1302.350nm
         Laser wavelength tolerance                : 1.030nm
         Vendor name                               : Mellanox
         Vendor OUI                                : 00:02:c9
         Vendor PN                                 : MMA1L10-CR
         Vendor rev                                : A1
         Vendor SN                                 : MT1639DM00026
         Date code                                 : 160926
         Revision Compliance                       : SFF-8636 Rev 
2.5/2.6/2.7
         Module temperature                        : 42.16 degrees C / 
107.90 degrees F
         Module voltage                            : 3.2523 V
         Alarm/warning flags implemented           : Yes
         Laser tx bias current (Channel 1)         : 36.454 mA
         Laser tx bias current (Channel 2)         : 36.696 mA
         Laser tx bias current (Channel 3)         : 37.006 mA
         Laser tx bias current (Channel 4)         : 37.404 mA
         Transmit avg optical power (Channel 1)    : 1.4812 mW / 1.71 dBm
         Transmit avg optical power (Channel 2)    : 1.3942 mW / 1.44 dBm
         Transmit avg optical power (Channel 3)    : 1.4793 mW / 1.70 dBm
         Transmit avg optical power (Channel 4)    : 1.4949 mW / 1.75 dBm
         Rcvr signal avg optical power(Channel 1)  : 1.4489 mW / 1.61 dBm
         Rcvr signal avg optical power(Channel 2)  : 1.5911 mW / 2.02 dBm
         Rcvr signal avg optical power(Channel 3)  : 1.1196 mW / 0.49 dBm
         Rcvr signal avg optical power(Channel 4)  : 1.6397 mW / 2.15 dBm
         Laser bias current high alarm   (Chan 1)  : Off
         Laser bias current low alarm    (Chan 1)  : Off
         Laser bias current high warning (Chan 1)  : Off
         Laser bias current low warning  (Chan 1)  : Off
         Laser bias current high alarm   (Chan 2)  : Off
         Laser bias current low alarm    (Chan 2)  : Off
         Laser bias current high warning (Chan 2)  : Off
         Laser bias current low warning  (Chan 2)  : Off
         Laser bias current high alarm   (Chan 3)  : Off
         Laser bias current low alarm    (Chan 3)  : Off
         Laser bias current high warning (Chan 3)  : Off
         Laser bias current low warning  (Chan 3)  : Off
         Laser bias current high alarm   (Chan 4)  : Off
         Laser bias current low alarm    (Chan 4)  : Off
         Laser bias current high warning (Chan 4)  : Off
         Laser bias current low warning  (Chan 4)  : Off
         Module temperature high alarm             : Off
         Module temperature low alarm              : Off
         Module temperature high warning           : Off
         Module temperature low warning            : Off
         Module voltage high alarm                 : Off
         Module voltage low alarm                  : Off
         Module voltage high warning               : Off
         Module voltage low warning                : Off
         Laser tx power high alarm   (Channel 1)   : Off
         Laser tx power low alarm    (Channel 1)   : Off
         Laser tx power high warning (Channel 1)   : Off
         Laser tx power low warning  (Channel 1)   : Off
         Laser tx power high alarm   (Channel 2)   : Off
         Laser tx power low alarm    (Channel 2)   : Off
         Laser tx power high warning (Channel 2)   : Off
         Laser tx power low warning  (Channel 2)   : Off
         Laser tx power high alarm   (Channel 3)   : Off
         Laser tx power low alarm    (Channel 3)   : Off
         Laser tx power high warning (Channel 3)   : Off
         Laser tx power low warning  (Channel 3)   : Off
         Laser tx power high alarm   (Channel 4)   : Off
         Laser tx power low alarm    (Channel 4)   : Off
         Laser tx power high warning (Channel 4)   : Off
         Laser tx power low warning  (Channel 4)   : Off
         Laser rx power high alarm   (Channel 1)   : Off
         Laser rx power low alarm    (Channel 1)   : Off
         Laser rx power high warning (Channel 1)   : Off
         Laser rx power low warning  (Channel 1)   : Off
         Laser rx power high alarm   (Channel 2)   : Off
         Laser rx power low alarm    (Channel 2)   : Off
         Laser rx power high warning (Channel 2)   : Off
         Laser rx power low warning  (Channel 2)   : Off
         Laser rx power high alarm   (Channel 3)   : Off
         Laser rx power low alarm    (Channel 3)   : Off
         Laser rx power high warning (Channel 3)   : Off
         Laser rx power low warning  (Channel 3)   : Off
         Laser rx power high alarm   (Channel 4)   : Off
         Laser rx power low alarm    (Channel 4)   : Off
         Laser rx power high warning (Channel 4)   : Off
         Laser rx power low warning  (Channel 4)   : Off
         Laser bias current high alarm threshold   : 0.000 mA
         Laser bias current low alarm threshold    : 0.000 mA
         Laser bias current high warning threshold : 0.000 mA
         Laser bias current low warning threshold  : 0.000 mA
         Laser output power high alarm threshold   : 0.0000 mW / -inf dBm
         Laser output power low alarm threshold    : 0.0000 mW / -inf dBm
         Laser output power high warning threshold : 0.0000 mW / -inf dBm
         Laser output power low warning threshold  : 0.0000 mW / -inf dBm
         Module temperature high alarm threshold   : 0.00 degrees C / 
32.00 degrees F
         Module temperature low alarm threshold    : 0.00 degrees C / 
32.00 degrees F
         Module temperature high warning threshold : 0.00 degrees C / 
32.00 degrees F
         Module temperature low warning threshold  : 0.00 degrees C / 
32.00 degrees F
         Module voltage high alarm threshold       : 0.0000 V
         Module voltage low alarm threshold        : 0.0000 V
         Module voltage high warning threshold     : 0.0000 V
         Module voltage low warning threshold      : 0.0000 V
         Laser rx power high alarm threshold       : 0.0000 mW / -inf dBm
         Laser rx power low alarm threshold        : 0.0000 mW / -inf dBm
         Laser rx power high warning threshold     : 0.0000 mW / -inf dBm
         Laser rx power low warning threshold      : 0.0000 mW / -inf dBm




This is parsed output after the fix:
         Identifier                                : 0x11 (QSFP28)
         Extended identifier                       : 0xcc
         Extended identifier description           : 3.5W max. Power 
consumption
         Extended identifier description           : CDR present in TX, 
CDR present in RX
         Extended identifier description           : High Power Class (> 
3.5 W) not enabled
         Connector                                 : 0x07 (LC)
         Transceiver codes                         : 0x80 0x00 0x00 0x00 
0x00 0x00 0x00 0x00
         Transceiver type                          : 100G Ethernet: 100G 
Base-LR4
         Encoding                                  : 0x05 (64B/66B)
         BR, Nominal                               : 25500Mbps
         Rate identifier                           : 0x00
         Length (SMF,km)                           : 10km
         Length (OM3 50um)                         : 0m
         Length (OM2 50um)                         : 0m
         Length (OM1 62.5um)                       : 0m
         Length (Copper or Active cable)           : 0m
         Transmitter technology                    : 0x40 (1310 nm DFB)
         Laser wavelength                          : 1302.350nm
         Laser wavelength tolerance                : 1.030nm
         Vendor name                               : Mellanox
         Vendor OUI                                : 00:02:c9
         Vendor PN                                 : MMA1L10-CR
         Vendor rev                                : A1
         Vendor SN                                 : MT1639DM00026
         Date code                                 : 160926
         Revision Compliance                       : SFF-8636 Rev 
2.5/2.6/2.7
         Module temperature                        : 42.16 degrees C / 
107.90 degrees F
         Module voltage                            : 3.2523 V
         Alarm/warning flags implemented           : No
         Laser tx bias current (Channel 1)         : 36.462 mA
         Laser tx bias current (Channel 2)         : 36.668 mA
         Laser tx bias current (Channel 3)         : 37.000 mA
         Laser tx bias current (Channel 4)         : 37.416 mA
         Transmit avg optical power (Channel 1)    : 1.4812 mW / 1.71 dBm
         Transmit avg optical power (Channel 2)    : 1.3940 mW / 1.44 dBm
         Transmit avg optical power (Channel 3)    : 1.4829 mW / 1.71 dBm
         Transmit avg optical power (Channel 4)    : 1.4866 mW / 1.72 dBm
         Rcvr signal avg optical power(Channel 1)  : 1.4518 mW / 1.62 dBm
         Rcvr signal avg optical power(Channel 2)  : 1.5938 mW / 2.02 dBm
         Rcvr signal avg optical power(Channel 3)  : 1.1211 mW / 0.50 dBm
         Rcvr signal avg optical power(Channel 4)  : 1.6378 mW / 2.14 dBm




Major diff:
* Alarm/warning flags implemented           : No
* All alarm data is not presented.

Driver return 256 bytes (reading it correctly, I verified it, no 
overruns), however the extra bytes are presented due to this bug 
(expecting to parse 640 bytes).

Do you see another bug here? Am I missing something?

Eran


> 
> Thanks
> 	Andrew
> 

^ permalink raw reply

* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-09-27 21:35 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-kernel, netdev, linux-crypto, davem, gregkh
In-Reply-To: <20180927182944.GA22921@gmail.com>

Hi Eric,

On Thu, Sep 27, 2018 at 8:29 PM Eric Biggers <ebiggers@kernel.org> wrote:
> Why is Herbert Xu's existing crypto tree being circumvented, especially for
> future patches (the initial merge isn't quite as important as that's a one-time
> event)?  I like being able to check out cryptodev to test upcoming crypto
> patches.  And currently, changes to APIs, algorithms, tests, and implementations
> all go through cryptodev, which is convenient for crypto developers.
>
> Apparently, you're proposing that someone adding a new algorithm will now have
> to submit the API portion to one maintainer (Herbert Xu) and the implementation
> portion to another maintainer (you), and they'll go through separate git trees.
> That's inconvenient for developers, and it seems that in practice you and
> Herbert will be stepping on each other's toes a lot.
>
> Can you please reach some kind of sane agreement with Herbert so that the
> development process isn't fractured into two?  Perhaps you could review patches,
> but Herbert could still apply them?

I think you're overthinking it a bit. Zinc will have a few software
implementations of primitives that are useful in cases where it's nice to call
the primitive directly. Think: various usages of sha2, siphash, the wireguard
suite (what this patchset includes), other things in lib/, etc. In so much as
this winds up duplicating things within the crypto API, I'll work with Herbert
to build one on top of the other -- as I've done in the two commits in this
series. But beyond that, think of the two initiatives as orthogonal. I'm
working on curating a few primitives that are maximally useful throughout
the kernel for various uses, and doing so in a way that I think brings
about a certain quality. Meanwhile the crypto API is amassing a huge
collection of primitives for some things, and that will continue to exist,
and Herbert will continue to maintain that. I expect for the crossover
to be fairly isolated and manageable, without too much foreseeable tree-
conflicts and such. Therefore, Samuel Neves and I plan to maintain the
codebase we've spent quite some time writing, and maintain our own tree for
it, which we'll be submitting through Greg. In other words, this is not
a matter of "circumvention" or "stepping on toes", but rather separate
efforts. I'm quite certain to the extent they overlap we'll be able to work
out fairly easily.

Either way, I'll take your suggestion and reach out to Herbert, since at
least a discussion between the two of us sounds like it could be productive.

> I'm also wondering about the criteria for making additions and changes to
> "Zinc".  You mentioned before that one of the "advantages" of Zinc is that it
> doesn't include "cipher modes from 90s cryptographers" -- what does that mean
> exactly?  You've also indicated before that you don't want people modifying the
> Poly1305 implementations as they are too error-prone.  Useful contributions
> could be blocked or discouraged in the future. Can you please elaborate on
> your criteria for contributions to Zinc?
>
> Also, will you allow algorithms that aren't up to modern security standards but
> are needed for compatibility reasons, e.g. MD5, SHA-1, and DES?  There are
> existing standards, APIs, and data formats that use these "legacy" algorithms;
> so implementations of them are often still needed, whether we like it or not.
>
> And does it matter who designed the algorithms, e.g. do algorithms from Daniel
> Bernstein get effectively a free pass, while algorithms from certain countries,
> governments, or organizations are not allowed?  E.g. wireless driver developers
> may need the SM4 block cipher (which is now supported by the crypto API) as it's
> specified in a Chinese wireless standard.  Will you allow SM4 in Zinc?  Or will
> people have to submit some algorithms to Herbert and some to you due to
> disagreements about what algorithms should be included?

Similarly here, I think you're over-politicizing everything. Stable address
generation for IPv6 uses SHA1 -- see net/ipv6/addrconf.c:3203 -- do you think
that this should use, say, the SM3 chinese hash function instead? No, of
course not, for a variety of interesting reasons. Rather, it should use some
simple hash function that's fast in software that we have available in Zinc.
On the other hand, it seems like parts of the kernel that have pretty high-
levels of cipher agility -- such as dmcrypt, ipsec, wifi apparently, and
so on -- will continue to use dynamic-dispatch system like the crypto API,
since that's what it was made to do and is effective at doing. And so, your
example of SM4 seems to fit perfectly into what the crypto API is well-suited
for, and it would fit naturally in there.

In other words, the "political criteria" for what we add to lib/zinc/ will
mostly be the same as for the rest of lib/: are there things using it that
benefit from it being there in a direct and obvious way, and does the
implementation meet certain quality standards.

> to change them yourself, e.g. when you added the part that converts the
> accumulator from base 26 to base 32.  I worry there may be double standards
> here

We do actually appreciate your concern here. However, there's a lot more that
went into that short patch than meets the eye:

  - It matches exactly what Andy Polyakov's code is doing for the exact
    same reason, so this isn't something that's actually "new". (There
    are paths inside his implementation that branch from the vector code
    to the scalar code.)
  - It has been discussed at length with Andy, including what kinds of
    proofs we'll need if we want to chop it down further (to remove that
    final reduction), and why we both don't want to do that yet, and so
    we go with the full carrying for the avoidance of risk.
  - We've proved its correctness with Z3, actually using an even looser
    constraint on digit size than what Andy mentioned to us, thus resulting
    in a stronger proof result. So we're certain this isn't rubbish.
  - There's been some considerable computing power sunk into fuzzing it.

There's no doubt about it, we've done our due-diligence here. This is in
fact the kind of efforts we require of submissions. You could fault us for
not detailing this in "the commit message" -- except as this is still a
patch series, we're putting improvements into the 00/XX change log, instead
of adding fixes and additions on top of the series. Of course in the ordinary
course of kernel development, this would exist instead as a standalone commit.
If you have a better idea of how this kind of thing can be communicated, and
where precisely, in the pre-merge process, I'd be interested in hearing
suggestions.

Thanks,
Jason

^ permalink raw reply

* Re: KMSAN: uninit-value in __dev_mc_add
From: Vladis Dronov @ 2018-09-27 21:30 UTC (permalink / raw)
  To: syzbot+001516d86dbe88862cec, David S . Miller, Eric Dumazet,
	netdev, linux-kernel, syzkaller-bugs
In-Reply-To: <0000000000005e2e530576c6f9ce@google.com>

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

Hello,

This report is actually for the same bug which was reported in:

https://syzkaller.appspot.com/bug?id=088efeac32fdde781038a777a63e436c0d4d7036

The note there that the bug was fixed by "Commits: net: fix uninit-value in
__hw_addr_add_ex()" is wrong. A C-reproducer from the 2nd syzkaller report
can trigger the bug from this one.

I've researched this and a result is a proposed patch, the problem is the tun
device code allowing to set an arbitrary link type.

https://lkml.org/lkml/2018/9/26/416
https://lore.kernel.org/lkml/20180926093018.6646-1-vdronov@redhat.com/T/#u
https://marc.info/?l=linux-netdev&m=153795423320016&w=2

A simplified reproducer is attached.

Best regards,
Vladis Dronov

[-- Attachment #2: kmsan-hw_addr_add_ex.c --]
[-- Type: text/plain, Size: 2185 bytes --]

#define _GNU_SOURCE
#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <linux/futex.h>
#include <pthread.h>
#include <sched.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/wait.h>
#include <unistd.h>

#include <errno.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char **argv)
{
  int ret, sockfd, tunfd;

  syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);

  // socket(AF_PACKET, SOCK_DGRAM|SOCK_NONBLOCK, 0)
  sockfd = syscall(__NR_socket, 0x11, 0x100000802, 0);
  if (sockfd < 0) {
    perror("socket()");
    ret = 1;
    goto exit_end;
  }

  memcpy((void*)0x20000240, "/dev/net/tun", 13);
  tunfd = open((char *)0x20000240, 0);
  if (tunfd < 0) {
    perror("open()");
    ret = 2;
    goto exit_sock_close;
  }

  memcpy((void*)0x200000c0, "\x69\x67\x62\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 16);
  *(uint16_t*)0x200000d0 = 0x4012;
  ret = syscall(__NR_ioctl, tunfd, 0x400454ca, 0x200000c0); // TUNSETIFF _IOW('T', 202, int)
  if (ret < 0) {
    perror("ioctl(TUNSETIFF)");
    ret = 3;
    goto exit_tun_close;
  }

  // TUNSETLINK _IOW('T', 205, int) / 0x30a = 778 = ARPHRD_IPGRE
  if (argc < 2)
    ret = syscall(__NR_ioctl, tunfd, 0x400454cd, 0x30a);
  else
    ret = syscall(__NR_ioctl, tunfd, 0x400454cd, atoi(argv[1]));
  if (ret < 0) {
    perror("ioctl(TUNSETLINK)");
    ret = 4;
    goto exit_tun_close;
  }

  memcpy((void*)0x20000040, "\x69\x67\x62\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 16);
  *(uint16_t*)0x20000050 = 0xa201;
  ret = syscall(__NR_ioctl, sockfd, 0x8914, 0x20000040); // SIOCSIFFLAGS 0x8914
  if (ret < 0) {
    perror("ioctl(SIOCSIFFLAGS)");
    ret = 5;
    goto exit_tun_close;
  }

  printf("done:\n");
  system("/usr/sbin/ip -details link show igb0");

exit_tun_close:
  close(tunfd);
exit_sock_close:
  close(sockfd);
exit_end:
  munmap((void *)0x20000000, 0x1000000);
  return 0;
}

^ permalink raw reply

* For editing of your photos 25
From: Jessica @ 2018-09-27 12:35 UTC (permalink / raw)
  To: netdev

Do you have needs for your photos cutting out and retouching?

We do editing for e-commerce photos, portrait photos and wedding photos.

You may choose to send us one or tow photos, we will provide testing to
check quality.

Thanks,
Jessica

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: change bridge maintainers
From: Nikolay Aleksandrov @ 2018-09-27 15:05 UTC (permalink / raw)
  To: Stephen Hemminger, davem; +Cc: netdev, Stephen Hemminger
In-Reply-To: <20180927084701.3468-1-sthemmin@microsoft.com>

On 27/09/18 11:47, Stephen Hemminger wrote:
> I haven't been doing reviews only but not active development on bridge
> code for several years. Roopa and Nikolay have been doing most of
> the new features and have agreed to take over as new co-maintainers.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  MAINTAINERS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7233a9ed0f5b..123ae2e65d94 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5470,7 +5470,8 @@ S:	Odd Fixes
>  F:	drivers/net/ethernet/agere/
>  
>  ETHERNET BRIDGE
> -M:	Stephen Hemminger <stephen@networkplumber.org>
> +M:	Roopa Prabhu <roopa@cumulusnetworks.com>
> +M:	Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>  L:	bridge@lists.linux-foundation.org (moderated for non-subscribers)
>  L:	netdev@vger.kernel.org
>  W:	http://www.linuxfoundation.org/en/Net:Bridge
> 

Thank you,
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply

* Re: [RFC PATCH iproute2-next V2] System specification exception API
From: Eran Ben Elisha @ 2018-09-27 15:04 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, Jakub Kicinski, Jiri Pirko, Stephen Hemminger,
	Andrew Lunn, Tobin C. Harding, Ariel Almog, Tal Alon
In-Reply-To: <20180927143432.GD2209@nanopsycho.orion>



On 9/27/2018 5:34 PM, Jiri Pirko wrote:
> Thu, Sep 27, 2018 at 04:02:48PM CEST, eranbe@mellanox.com wrote:
>>
>>
>> On 9/27/2018 3:47 PM, Jiri Pirko wrote:
>>> Wed, Sep 26, 2018 at 01:52:58PM CEST, eranbe@mellanox.com wrote:
>>>> The exception spec is targeted for Real Time Alerting, in order to know when
>>>> something bad had happened to a PCI device
>>>> - Provide alert debug information
>>>> - Self healing
>>>> - If problem needs vendor support, provide a way to gather all needed debugging
>>>>    information.
>>>>
>>>> The exception mechanism contains condition checkers which sense for malfunction. Upon a condition hit,
>>>> actions such as logs and correction can be taken.
>>>>
>>>> The condition checkers are divided into the following groups
>>>> - Hardware - a checker which is triggered by the device due to
>>>>    malfunction.
>>>> - Software - a checker which is triggered by the software due to
>>>>    malfunction.
>>>
>>> What do you mean by a "software malfunction", a "FW malfunction"?
>>> Also, I don't see this 2 groups in the man.
>>
>> Software malfunction can be a Transmit error (caused by bad send request).
> 
> Sorry, but I still don't undestand what "software malfuntion" are you
> talking about. Could you be more specific please?

* Driver is building a bad send Work request (bug in driver, bug in 
packet generator, etc). When it sends it, it gets back an error 
completion from the HW. This error might cause the HW Queue to be in 
error state and cannot be used again until it is being "recovered".

Condition: Error completion
Action: Queue recover
The entire scenario is due to SW malfunction.

* Driver is trying to configure HW QoS register bug failed by the FW.

Condition: command execution error
Action: Dump of command + Dump of SW internal related DB + Dump of FW 
related DB

* Another existing example is the ndo_tx_timeout routine. (This is being 
done in the networking stuck layer, and can be configured today from a 
sysfs). If a vendor driver has other specific checking routine like this 
one in its driver (which he needs to configure from userspace), then it 
can handled via devlink-exception and be tagged as a software condition.

> 
> 
>> FW/HW malfunction can be any catastrophic error report (the ones that should
>> be exposed to driver).
>> The comment here was to highlight that we can support different kinds of
>> condition groups.
>> If for a specific condition, we will need to highlight it is SW/HW, we can
>> concatenate it to its name.
>>
>> Eran
>>

>>>>

^ permalink raw reply

* Re: [PATCH v3 bpf-next 00/10] bpf: per-cpu cgroup local storage
From: Daniel Borkmann @ 2018-09-27 21:13 UTC (permalink / raw)
  To: Roman Gushchin, netdev; +Cc: Song Liu, linux-kernel, kernel-team
In-Reply-To: <20180926113326.29069-1-guro@fb.com>

On 09/26/2018 01:33 PM, Roman Gushchin wrote:
> This patchset implements per-cpu cgroup local storage and provides
> an example how per-cpu and shared cgroup local storage can be used
> for efficient accounting of network traffic.
> 
> v3->v2:
>   1) incorporated Song's feedback
>   2) rebased on top of current bpf-next
> 
> v2->v1:
>   1) added a selftest implementing network counters
>   2) added a missing free() in cgroup local storage selftest
> 
> Roman Gushchin (10):
>   bpf: extend cgroup bpf core to allow multiple cgroup storage types
>   bpf: rework cgroup storage pointer passing
>   bpf: introduce per-cpu cgroup local storage
>   bpf: don't allow create maps of per-cpu cgroup local storages
>   bpf: sync include/uapi/linux/bpf.h to tools/include/uapi/linux/bpf.h
>   bpftool: add support for PERCPU_CGROUP_STORAGE maps
>   selftests/bpf: add verifier per-cpu cgroup storage tests
>   selftests/bpf: extend the storage test to test per-cpu cgroup storage
>   samples/bpf: extend test_cgrp2_attach2 test to use per-cpu cgroup
>     storage
>   selftests/bpf: cgroup local storage-based network counters
> 
>  include/linux/bpf-cgroup.h                    |  55 ++++--
>  include/linux/bpf.h                           |  12 +-
>  include/linux/bpf_types.h                     |   1 +
>  include/uapi/linux/bpf.h                      |   1 +
>  kernel/bpf/cgroup.c                           |  74 +++++---
>  kernel/bpf/helpers.c                          |  25 ++-
>  kernel/bpf/local_storage.c                    | 167 +++++++++++++++---
>  kernel/bpf/map_in_map.c                       |   3 +-
>  kernel/bpf/syscall.c                          |  20 ++-
>  kernel/bpf/verifier.c                         |  23 ++-
>  net/bpf/test_run.c                            |  20 ++-
>  samples/bpf/test_cgrp2_attach2.c              |  19 +-
>  tools/bpf/bpftool/map.c                       |   4 +-
>  tools/include/uapi/linux/bpf.h                |   1 +
>  tools/testing/selftests/bpf/Makefile          |   6 +-
>  tools/testing/selftests/bpf/netcnt_common.h   |  23 +++
>  tools/testing/selftests/bpf/netcnt_prog.c     |  71 ++++++++
>  .../selftests/bpf/test_cgroup_storage.c       |  60 ++++++-
>  tools/testing/selftests/bpf/test_netcnt.c     | 153 ++++++++++++++++
>  tools/testing/selftests/bpf/test_verifier.c   | 139 ++++++++++++++-
>  20 files changed, 778 insertions(+), 99 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/netcnt_common.h
>  create mode 100644 tools/testing/selftests/bpf/netcnt_prog.c
>  create mode 100644 tools/testing/selftests/bpf/test_netcnt.c
> 

Applied to bpf-next, thanks Roman!

^ permalink raw reply

* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Andrew Lunn @ 2018-09-27 14:52 UTC (permalink / raw)
  To: Eran Ben Elisha
  Cc: Neil Horman, Chris Preimesberger, linville@tuxdriver.com,
	netdev@vger.kernel.org
In-Reply-To: <091551a9-7a85-4112-bbe2-ce7c7d3cf964@mellanox.com>

> Both drivers read up to 256 bytes. 0-127 (from page 0). and 128-256 (from
> page 0). Driver is not capable of reading over 256 bytes currently.

Hi Erin

There should not be any need to read more than 256 bytes. For older
SFP devices, two addresses on the i2c bus are used, each with 256
bytes. For QSFP, one address is used, and you swap page by writing to
offset 127.

> looking on qsfp.c parser in ethtool.c (user space), I see an uninitialized
> bug issue that have caused bug #1 + #2.
> Applied it locally solved the issue (Not showing alarm data, which should be
> expected as driver do not fill it).

There appears to be a second bug somewhere. dumping the module info
using HEX returned 256 bytes. But the binary dump had more bytes.
Since you have the hardware, could you look into this?

Thanks
	Andrew

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: change bridge maintainers
From: Roopa Prabhu @ 2018-09-27 14:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev, Stephen Hemminger
In-Reply-To: <20180927084701.3468-1-sthemmin@microsoft.com>

On Thu, Sep 27, 2018 at 1:47 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> I haven't been doing reviews only but not active development on bridge
> code for several years. Roopa and Nikolay have been doing most of
> the new features and have agreed to take over as new co-maintainers.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

Thanks

^ permalink raw reply

* Re: [RFC PATCH iproute2-next V2] System specification exception API
From: Jiri Pirko @ 2018-09-27 14:34 UTC (permalink / raw)
  To: Eran Ben Elisha
  Cc: netdev, Jakub Kicinski, Jiri Pirko, Stephen Hemminger,
	Andrew Lunn, Tobin C. Harding, Ariel Almog, Tal Alon
In-Reply-To: <8a064f7f-298e-9cbe-d58a-fdf3d79eef24@mellanox.com>

Thu, Sep 27, 2018 at 04:02:48PM CEST, eranbe@mellanox.com wrote:
>
>
>On 9/27/2018 3:47 PM, Jiri Pirko wrote:
>> Wed, Sep 26, 2018 at 01:52:58PM CEST, eranbe@mellanox.com wrote:
>> > The exception spec is targeted for Real Time Alerting, in order to know when
>> > something bad had happened to a PCI device
>> > - Provide alert debug information
>> > - Self healing
>> > - If problem needs vendor support, provide a way to gather all needed debugging
>> >   information.
>> > 
>> > The exception mechanism contains condition checkers which sense for malfunction. Upon a condition hit,
>> > actions such as logs and correction can be taken.
>> > 
>> > The condition checkers are divided into the following groups
>> > - Hardware - a checker which is triggered by the device due to
>> >   malfunction.
>> > - Software - a checker which is triggered by the software due to
>> >   malfunction.
>> 
>> What do you mean by a "software malfunction", a "FW malfunction"?
>> Also, I don't see this 2 groups in the man.
>
>Software malfunction can be a Transmit error (caused by bad send request).

Sorry, but I still don't undestand what "software malfuntion" are you
talking about. Could you be more specific please?


>FW/HW malfunction can be any catastrophic error report (the ones that should
>be exposed to driver).
>The comment here was to highlight that we can support different kinds of
>condition groups.
>If for a specific condition, we will need to highlight it is SW/HW, we can
>concatenate it to its name.
>
>Eran
>
>> 
>> 
>> > Both groups of condition checkers can be triggered due to error event or due to a periodic check.
>> > 
>> > Actions are the way to handle those events. Action can be in one of the
>> > following groups:
>> > - Dump -  SW trace, SW dump, HW trace, HW dump
>> > - Reset - Surgical correction (e.g. modify Q, flush Q, reset of device, etc)
>> > Actions can be performed by SW or HW.
>> > 
>> > User is allowed to enable or disable condition checkers and its action mapping.
>> > 
>> > This RFC man page patch describes the suggested API of devlink-exception in order
>> > to control conditions and actions.
>> > 
>> > V2:
>> > * Renaming terms:
>> > 	health -> exception
>> > 	sensor -> condition
>> > * Remove reinit command and merge with action command.
>> > * Consmetics in grammer.
>> > 
>> > Eran Ben Elisha (1):
>> >   man: Add devlink exception man page
>> > 
>> > man/man8/devlink-exception.8 | 158 +++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 158 insertions(+)
>> > create mode 100644 man/man8/devlink-exception.8
>> > 
>> > -- 
>> > 1.8.3.1
>> > 

^ permalink raw reply

* [PATCH net-next] net: sched: make function qdisc_free_cb() static
From: Wei Yongjun @ 2018-09-27 14:47 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Vlad Buslov
  Cc: Wei Yongjun, netdev, kernel-janitors

Fixes the following sparse warning:

net/sched/sch_generic.c:944:6: warning:
 symbol 'qdisc_free_cb' was not declared. Should it be static?

Fixes: 3a7d0d07a386 ("net: sched: extend Qdisc with rcu")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 net/sched/sch_generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 531fac1..3023929 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -941,7 +941,7 @@ void qdisc_free(struct Qdisc *qdisc)
 	kfree((char *) qdisc - qdisc->padded);
 }
 
-void qdisc_free_cb(struct rcu_head *head)
+static void qdisc_free_cb(struct rcu_head *head)
 {
 	struct Qdisc *q = container_of(head, struct Qdisc, rcu);

^ permalink raw reply related

* Re: [RFC PATCH iproute2-next V2] man: Add devlink exception man page
From: Jiri Pirko @ 2018-09-27 14:32 UTC (permalink / raw)
  To: Eran Ben Elisha
  Cc: netdev, Jakub Kicinski, Jiri Pirko, Stephen Hemminger,
	Andrew Lunn, Tobin C. Harding, Ariel Almog, Tal Alon
In-Reply-To: <1537962779-8910-2-git-send-email-eranbe@mellanox.com>

Wed, Sep 26, 2018 at 01:52:59PM CEST, eranbe@mellanox.com wrote:
>Add devlink-exception man page. Devlink-exception tool will control device
>exception attributes, conditions, actions and logging.
>
>Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>
>-------------------------------------------------------
>Copy paste man output to here for easier review process of the RFC.
>
>DEVLINK-EXCEPTION(8)                                                                                            Linux                                                                                           DEVLINK-EXCEPTION(8)
>
>NAME
>       devlink-exception - devlink exception configuration
>
>SYNOPSIS
>       devlink [ OPTIONS ] exception  { COMMAND | help }
>
>       OPTIONS := { -V[ersion] | -n[no-nice-names] }
>
>       devlink exception show [ DEV ] [ condition NAME ] [ action NAME ]
>
>       devlink exception condition set DEV name NAME [ action NAME { active | inactive } ]
>
>       devlink exception action set DEV name NAME period PERIOD count COUNT fail { ignore | down }
>
>       devlink exception help
>
>DESCRIPTION
>       devlink-exception tool allows user to configure the way driver treats unexpected status. The tool allows configuration of the conditions that can trigger exception activity. Set for each condition the follow up opera‐
>       tions, such as, reset and dump of info. In addition, set the exception activity termination action.
>
>   devlink exception show - Display devlink exception conditions and actions attributes
>       DEV    Specifies the devlink device to show.
>
>       condition NAME
>              Specifies the devlink condition to show.
>
>       action NAME
>              Specifies the devlink action to show.
>
>   devlink exception condition set - sets devlink exception condition attributes
>       DEV    Specifies the devlink device to set.
>
>       name NAME
>              Name of the condition to set.
>
>       action NAME { active | inactive }
>                  Specify which actions to activate and which to deactivate once a condition was triggered. Actions can be dump, reset, etc.
>
>   devlink exception action set - sets devlink action attributes.
>       Once this command is launched, period and count measurement will be reset.
>
>       DEV    Specifies the devlink device to set.
>
>       name NAME
>              Specifies the devlink action to set.
>
>       period PERIOD
>              The period on which we limit the amount of performed actions, measured in seconds.
>
>       count COUNT
>              The maximum number of actions performed in a limited time frame.
>
>       fail   { ignore | down }
>                  Specify the behavior once count limit was reached.
>
>                  ignore - Skip triggering this action.
>
>                  down - Driver will remain in nonoperational state.
>
>EXAMPLES
>       devlink exception show
>           Shows the exception state of all devlink devices on the system.
>
>       devlink exception show pci/0000:01:00.0
>           Shows the exception state of specified devlink device.
>
>       devlink exception condition set pci/0000:01:00.0 name TX_COMP_ERROR action reset off action dump on
>           Sets TX_COMP_ERROR condition parameters for a specific device.
>
>       devlink exception action set pci/0000:01:00.0 name reset period 3600 count 5 fail ignore
>           Sets exception attributes for reset action. Period timer and counter are being reset.

Looks good to me. But still, I need the code so I can play with it, to
see the outputs etc.

Thanks!


>
>SEE ALSO
>       devlink(8), devlink-port(8), devlink-sb(8), devlink-monitor(8), devlink-dev(8),
>
>AUTHOR
>       Eran ben Elisha <eranbe@mellanox.com>
>
>iproute2                                                                                                     15 Aug 2018                                                                                        DEVLINK-EXCEPTION(8)
>
>---
> man/man8/devlink-exception.8 | 158 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 158 insertions(+)
> create mode 100644 man/man8/devlink-exception.8
>
>diff --git a/man/man8/devlink-exception.8 b/man/man8/devlink-exception.8
>new file mode 100644
>index 000000000000..03f24b32cc98
>--- /dev/null
>+++ b/man/man8/devlink-exception.8
>@@ -0,0 +1,158 @@
>+.TH DEVLINK\-EXCEPTION 8 "15 Aug 2018" "iproute2" "Linux"
>+.SH NAME
>+devlink-exception \- devlink exception configuration
>+.SH SYNOPSIS
>+.sp
>+.ad l
>+.in +8
>+.ti -8
>+.B devlink
>+.RI "[ " OPTIONS " ]"
>+.BR exception
>+.RI  " { " COMMAND " | "
>+.BR help " }"
>+.sp
>+
>+.ti -8
>+.IR OPTIONS " := { "
>+\fB\-V\fR[\fIersion\fR] |
>+\fB\-n\fR[\fIno-nice-names\fR] }
>+
>+.ti -8
>+.B devlink exception show
>+.RI "[ " DEV " ]"
>+.RI "[ "
>+.B condition
>+.IR NAME
>+.RI "]"
>+.RI "[ "
>+.B action
>+.IR NAME
>+.RI "]"
>+
>+.ti -8
>+.B devlink exception condition set
>+.IR DEV
>+.B name
>+.IR NAME
>+.RI "[ "
>+.BR action
>+.IR NAME
>+.R "{" active "|" inactive "}" ]
>+
>+.ti -8
>+.B devlink exception action set
>+.IR DEV
>+.B name
>+.IR NAME
>+.BR period
>+.IR PERIOD
>+.BR count
>+.IR COUNT
>+.BR fail " { "
>+.IR ignore
>+.BR "| "
>+.IR down
>+.R "} "
>+
>+.ti -8
>+.B devlink exception help
>+
>+.SH "DESCRIPTION"
>+.B devlink-exception
>+tool allows user to configure the way driver treats unexpected status. The tool allows configuration of the conditions that can trigger exception activity. Set for each condition the follow up operations, such as, reset and dump of info. In addition, set the exception activity termination action.
>+
>+.SS devlink exception show - Display devlink exception conditions and actions attributes
>+.TP
>+.BI "DEV"
>+Specifies the devlink device to show.
>+
>+.PP
>+.TP
>+.BI condition " NAME"
>+Specifies the devlink condition to show.
>+
>+.TP
>+.BI action " NAME"
>+Specifies the devlink action to show.
>+
>+.SS devlink exception condition set - sets devlink exception condition attributes
>+
>+.TP
>+.B "DEV"
>+Specifies the devlink device to set.
>+
>+.TP
>+.BI name " NAME"
>+Name of the condition to set.
>+
>+.TP
>+.BR action
>+.IR NAME
>+.R "{" active "|" inactive "} "
>+.in +4
>+Specify which actions to activate and which to deactivate once a condition was triggered. Actions can be dump, reset, etc.
>+
>+.SS devlink exception action set - sets devlink action attributes.
>+Once this command is launched, period and count measurement will be reset.
>+
>+.TP
>+.B "DEV"
>+Specifies the devlink device to set.
>+
>+.TP
>+.BI name " NAME"
>+Specifies the devlink action to set.
>+
>+.TP
>+.BI period " PERIOD"
>+The period on which we limit the amount of performed actions, measured in seconds.
>+
>+.TP
>+.BI count " COUNT"
>+The maximum number of actions performed in a limited time frame.
>+
>+.TP
>+.BR fail
>+.R "{" ignore "|" down "}"
>+.in +4
>+Specify the behavior once count limit was reached.
>+
>+.I ignore
>+- Skip triggering this action.
>+
>+.I down
>+- Driver will remain in nonoperational state.
>+
>+.SH "EXAMPLES"
>+.PP
>+devlink exception show
>+.RS 4
>+Shows the exception state of all devlink devices on the system.
>+.RE
>+.PP
>+devlink exception show pci/0000:01:00.0
>+.RS 4
>+Shows the exception state of specified devlink device.
>+.RE
>+.PP
>+devlink exception condition set pci/0000:01:00.0 name TX_COMP_ERROR action reset off action dump on
>+.RS 4
>+Sets TX_COMP_ERROR condition parameters for a specific device.
>+.RE
>+.PP
>+devlink exception action set pci/0000:01:00.0 name reset period 3600 count 5 fail ignore
>+.RS 4
>+Sets exception attributes for reset action. Period timer and counter are being reset.
>+.RE
>+
>+.SH SEE ALSO
>+.BR devlink (8),
>+.BR devlink-port (8),
>+.BR devlink-sb (8),
>+.BR devlink-monitor (8),
>+.BR devlink-dev (8),
>+.br
>+
>+.SH AUTHOR
>+Eran ben Elisha <eranbe@mellanox.com>
>-- 
>1.8.3.1
>

^ permalink raw reply

* Re: [PATCH v3] PCI: Reprogram bridge prefetch registers on resume
From: Bjorn Helgaas @ 2018-09-27 20:52 UTC (permalink / raw)
  To: Daniel Drake
  Cc: andy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
	keith.busch-ral2JQCrhuEAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, rchang-eYqpPyKDWXRBDgjK7y7TUQ,
	linux-6IF/jdPJHihWk0Htik3J/w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	hkallweit1-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20180918213244.GE13616-1RhO1Y9PlrlHTL0Zs8A6p5iNqAH0jzoTYJqu5kTmcBRl57MIdRCFDg@public.gmane.org>

[+cc LKML]

On Tue, Sep 18, 2018 at 04:32:44PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 13, 2018 at 11:37:45AM +0800, Daniel Drake wrote:
> > On 38+ Intel-based Asus products, the nvidia GPU becomes unusable
> > after S3 suspend/resume. The affected products include multiple
> > generations of nvidia GPUs and Intel SoCs. After resume, nouveau logs
> > many errors such as:
> > 
> >     fifo: fault 00 [READ] at 0000005555555000 engine 00 [GR] client 04
> >           [HUB/FE] reason 4a [] on channel -1 [007fa91000 unknown]
> >     DRM: failed to idle channel 0 [DRM]
> > 
> > Similarly, the nvidia proprietary driver also fails after resume
> > (black screen, 100% CPU usage in Xorg process). We shipped a sample
> > to Nvidia for diagnosis, and their response indicated that it's a
> > problem with the parent PCI bridge (on the Intel SoC), not the GPU.
> > 
> > Runtime suspend/resume works fine, only S3 suspend is affected.
> > 
> > We found a workaround: on resume, rewrite the Intel PCI bridge
> > 'Prefetchable Base Upper 32 Bits' register (PCI_PREF_BASE_UPPER32). In
> > the cases that I checked, this register has value 0 and we just have to
> > rewrite that value.
> > 
> > Linux already saves and restores PCI config space during suspend/resume,
> > but this register was being skipped because upon resume, it already
> > has value 0 (the correct, pre-suspend value).
> > 
> > Intel appear to have previously acknowledged this behaviour and the
> > requirement to rewrite this register.
> > https://bugzilla.kernel.org/show_bug.cgi?id=116851#c23
> > 
> > Based on that, rewrite the prefetch register values even when that
> > appears unnecessary.
> > 
> > We have confirmed this solution on all the affected models we have
> > in-hands (X542UQ, UX533FD, X530UN, V272UN).
> > 
> > Additionally, this solves an issue where r8169 MSI-X interrupts were
> > broken after S3 suspend/resume on Asus X441UAR. This issue was recently
> > worked around in commit 7bb05b85bc2d ("r8169: don't use MSI-X on
> > RTL8106e"). It also fixes the same issue on RTL6186evl/8111evl on an
> > Aimfor-tech laptop that we had not yet patched. I suspect it will also
> > fix the issue that was worked around in commit 7c53a722459c ("r8169:
> > don't use MSI-X on RTL8168g").
> > 
> > Thomas Martitz reports that this change also solves an issue where
> > the AMD Radeon Polaris 10 GPU on the HP Zbook 14u G5 is unresponsive
> > after S3 suspend/resume.
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=201069
> > Signed-off-by: Daniel Drake <drake@endlessm.com>
> 
> Applied with Rafael's and Peter's reviewed-by to pci/enumeration for v4.20.
> Thanks for the the huge investigative effort!

Since this looks low-risk and fixes several painful issues, I think
this merits a stable tag and being included in v4.19 (instead of
waiting for v4.20).  

I moved it to for-linus for v4.19.  Let me know if you object.

> > ---
> >  drivers/pci/pci.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 29ff9619b5fa..5d58220b6997 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -1289,12 +1289,12 @@ int pci_save_state(struct pci_dev *dev)
> >  EXPORT_SYMBOL(pci_save_state);
> >  
> >  static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
> > -				     u32 saved_val, int retry)
> > +				     u32 saved_val, int retry, bool force)
> >  {
> >  	u32 val;
> >  
> >  	pci_read_config_dword(pdev, offset, &val);
> > -	if (val == saved_val)
> > +	if (!force && val == saved_val)
> >  		return;
> >  
> >  	for (;;) {
> > @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset,
> >  }
> >  
> >  static void pci_restore_config_space_range(struct pci_dev *pdev,
> > -					   int start, int end, int retry)
> > +					   int start, int end, int retry,
> > +					   bool force)
> >  {
> >  	int index;
> >  
> >  	for (index = end; index >= start; index--)
> >  		pci_restore_config_dword(pdev, 4 * index,
> >  					 pdev->saved_config_space[index],
> > -					 retry);
> > +					 retry, force);
> >  }
> >  
> >  static void pci_restore_config_space(struct pci_dev *pdev)
> >  {
> >  	if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL) {
> > -		pci_restore_config_space_range(pdev, 10, 15, 0);
> > +		pci_restore_config_space_range(pdev, 10, 15, 0, false);
> >  		/* Restore BARs before the command register. */
> > -		pci_restore_config_space_range(pdev, 4, 9, 10);
> > -		pci_restore_config_space_range(pdev, 0, 3, 0);
> > +		pci_restore_config_space_range(pdev, 4, 9, 10, false);
> > +		pci_restore_config_space_range(pdev, 0, 3, 0, false);
> > +	} else if (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> > +		pci_restore_config_space_range(pdev, 12, 15, 0, false);
> > +		/* Force rewriting of prefetch registers to avoid
> > +		 * S3 resume issues on Intel PCI bridges that occur when
> > +		 * these registers are not explicitly written.
> > +		 */
> > +		pci_restore_config_space_range(pdev, 9, 11, 0, true);
> > +		pci_restore_config_space_range(pdev, 0, 8, 0, false);
> >  	} else {
> > -		pci_restore_config_space_range(pdev, 0, 15, 0);
> > +		pci_restore_config_space_range(pdev, 0, 15, 0, false);
> >  	}
> >  }
> >  
> > -- 
> > 2.17.1
> > 
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply

* Re: [PATCH] netfilter: check if the socket netns is correct.
From: Guenter Roeck @ 2018-09-27 20:46 UTC (permalink / raw)
  To: Flavio Leitner
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
	netfilter-devel, coreteam, netdev, linux-kernel

Hi Flavio,

On Wed, Jun 27, 2018 at 10:34:25AM -0300, Flavio Leitner wrote:
> Netfilter assumes that if the socket is present in the skb, then
> it can be used because that reference is cleaned up while the skb
> is crossing netns.
> 
> We want to change that to preserve the socket reference in a future
> patch, so this is a preparation updating netfilter to check if the
> socket netns matches before use it.
> 
> Signed-off-by: Flavio Leitner <fbl@redhat.com>
> Acked-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
...
> --- a/net/netfilter/xt_socket.c
> +++ b/net/netfilter/xt_socket.c
> @@ -56,8 +56,12 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
>  	struct sk_buff *pskb = (struct sk_buff *)skb;
>  	struct sock *sk = skb->sk;
>  
> +	if (!net_eq(xt_net(par), sock_net(sk)))
> +		sk = NULL;
> +

I am having trouble with this code. With CONFIG_NET_NS enabled, it crashes
for me in read_pnet() because sk is NULL.

>  	if (!sk)
>  		sk = nf_sk_lookup_slow_v4(xt_net(par), skb, xt_in(par));

The old code seems to suggest that sk == NULL was possible.

I see the problem with the Chrome OS kernel rebased to v4.19-rc5, so I
can not guarantee that this really an upstream problem. The change seems
odd, though. Are you sure that it is not (or, rather, no longer) necessary
to check if sk == NULL before dereferencing it in sock_net() ?

> +
>  	if (sk) {
>  		bool wildcard;
>  		bool transparent = true;
> @@ -113,8 +117,12 @@ socket_mt6_v1_v2_v3(const struct sk_buff *skb, struct xt_action_param *par)
>  	struct sk_buff *pskb = (struct sk_buff *)skb;
>  	struct sock *sk = skb->sk;
>  
> +	if (!net_eq(xt_net(par), sock_net(sk)))
> +		sk = NULL;
> +
Same here.

>  	if (!sk)
>  		sk = nf_sk_lookup_slow_v6(xt_net(par), skb, xt_in(par));
> +
>  	if (sk) {
>  		bool wildcard;
>  		bool transparent = true;

Thanks,
Guenter

^ permalink raw reply

* My Greetings
From: Mrs Marianne Jeanne @ 2018-09-27 14:21 UTC (permalink / raw)


Beloved,
I am writing this mail to you with heavy tears in my eyes and great
sorrow in my heart.  As I informed you earlier, I am (Mrs.)Marianne 
Jeanne, 
suffering from long time Cancer of the breast. From all indications 
my condition is really deteriorating and it's quite obvious 
that I won't live more than 2 months according to my doctors.

I have some funds I inherited from my late loving husband Mr. Martin
Smith, the sum of ($8,5000.000) which he deposited in a Bank  . I need
a very honest and God fearing person that can use these funds for
Charity work, helping the Less Privileges, and 20% of this money will
be for your time and expenses, while 80% goes to charities.

Please let me know if I can TRUST YOU ON THIS to carry out this favour
for me.  I look forward to your prompt reply for more details.

Yours sincerely
Marianne Jeanne

^ permalink raw reply

* Re: [PATCH v2 07/22] soc/fsl/bman_portals: defer probe after bman's probe
From: Li Yang @ 2018-09-27 20:40 UTC (permalink / raw)
  To: Laurentiu Tudor
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Netdev, lkml,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
	bharat.bhushan
In-Reply-To: <20180926132247.10971-8-laurentiu.tudor@nxp.com>

On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> A crash in bman portal probing could not be triggered (as is the case
> with qman portals) but it does make calls [1] into the bman driver so
> lets make sure the bman portal probing happens after bman's.
>
> [1]  bman_p_irqsource_add() (in bman) called by:
>        init_pcfg() called by:
>          bman_portal_probe()
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

As this is part of a bug fix for v4.19, applied on soc/fsl for fix.

> ---
>  drivers/soc/fsl/qbman/bman_portal.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman_portal.c b/drivers/soc/fsl/qbman/bman_portal.c
> index 2f71f7df3465..f9edd28894fd 100644
> --- a/drivers/soc/fsl/qbman/bman_portal.c
> +++ b/drivers/soc/fsl/qbman/bman_portal.c
> @@ -91,7 +91,15 @@ static int bman_portal_probe(struct platform_device *pdev)
>         struct device_node *node = dev->of_node;
>         struct bm_portal_config *pcfg;
>         struct resource *addr_phys[2];
> -       int irq, cpu;
> +       int irq, cpu, err;
> +
> +       err = bman_is_probed();
> +       if (!err)
> +               return -EPROBE_DEFER;
> +       if (err < 0) {
> +               dev_err(&pdev->dev, "failing probe due to bman probe error\n");
> +               return -ENODEV;
> +       }
>
>         pcfg = devm_kmalloc(dev, sizeof(*pcfg), GFP_KERNEL);
>         if (!pcfg)
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v2 06/22] soc/fsl/qman_portals: defer probe after qman's probe
From: Li Yang @ 2018-09-27 20:37 UTC (permalink / raw)
  To: Laurentiu Tudor
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Netdev, lkml,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
	bharat.bhushan
In-Reply-To: <20180926132247.10971-7-laurentiu.tudor@nxp.com>

On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> Defer probe of qman portals after qman probing. This fixes the crash
> below, seen on NXP LS1043A SoCs:
>
> Unable to handle kernel NULL pointer dereference at virtual address
> 0000000000000004
> Mem abort info:
>   ESR = 0x96000004
>   Exception class = DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
> Data abort info:
>   ISV = 0, ISS = 0x00000004
>   CM = 0, WnR = 0
> [0000000000000004] user address but active_mm is swapper
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 4.18.0-rc1-next-20180622-00200-g986f5c179185 #9
> Hardware name: LS1043A RDB Board (DT)
> pstate: 80000005 (Nzcv daif -PAN -UAO)
> pc : qman_set_sdest+0x74/0xa0
> lr : qman_portal_probe+0x22c/0x470
> sp : ffff00000803bbc0
> x29: ffff00000803bbc0 x28: 0000000000000000
> x27: ffff0000090c1b88 x26: ffff00000927cb68
> x25: ffff00000927c000 x24: ffff00000927cb60
> x23: 0000000000000000 x22: 0000000000000000
> x21: ffff0000090e9000 x20: ffff800073b5c810
> x19: ffff800027401298 x18: ffffffffffffffff
> x17: 0000000000000001 x16: 0000000000000000
> x15: ffff0000090e96c8 x14: ffff80002740138a
> x13: ffff0000090f2000 x12: 0000000000000030
> x11: ffff000008f25000 x10: 0000000000000000
> x9 : ffff80007bdfd2c0 x8 : 0000000000004000
> x7 : ffff80007393cc18 x6 : 0040000000000001
> x5 : 0000000000000000 x4 : ffffffffffffffff
> x3 : 0000000000000004 x2 : ffff00000927c900
> x1 : 0000000000000000 x0 : 0000000000000004
> Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____))
> Call trace:
>  qman_set_sdest+0x74/0xa0
>  platform_drv_probe+0x50/0xa8
>  driver_probe_device+0x214/0x2f8
>  __driver_attach+0xd8/0xe0
>  bus_for_each_dev+0x68/0xc8
>  driver_attach+0x20/0x28
>  bus_add_driver+0x108/0x228
>  driver_register+0x60/0x110
>  __platform_driver_register+0x40/0x48
>  qman_portal_driver_init+0x20/0x84
>  do_one_initcall+0x58/0x168
>  kernel_init_freeable+0x184/0x22c
>  kernel_init+0x10/0x108
>  ret_from_fork+0x10/0x18
> Code: f9400443 11001000 927e4800 8b000063 (b9400063)
> ---[ end trace 4f6d50489ecfb930 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

As this is part of a bug fix for v4.19, applied on soc/fsl for fix.

> ---
>  drivers/soc/fsl/qbman/qman_portal.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/qman_portal.c b/drivers/soc/fsl/qbman/qman_portal.c
> index 6d9da3b1b5ad..eef93cab84f1 100644
> --- a/drivers/soc/fsl/qbman/qman_portal.c
> +++ b/drivers/soc/fsl/qbman/qman_portal.c
> @@ -229,6 +229,14 @@ static int qman_portal_probe(struct platform_device *pdev)
>         int irq, cpu, err;
>         u32 val;
>
> +       err = qman_is_probed();
> +       if (!err)
> +               return -EPROBE_DEFER;
> +       if (err < 0) {
> +               dev_err(&pdev->dev, "failing probe due to qman probe error\n");
> +               return -ENODEV;
> +       }
> +
>         pcfg = devm_kmalloc(dev, sizeof(*pcfg), GFP_KERNEL);
>         if (!pcfg)
>                 return -ENOMEM;
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v2 05/22] soc/fsl/qbman: add APIs to retrieve the probing status
From: Li Yang @ 2018-09-27 20:37 UTC (permalink / raw)
  To: Laurentiu Tudor
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Netdev, lkml,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Roy Pledge, madalin.bucur, David Miller, Shawn Guo, robin.murphy,
	bharat.bhushan
In-Reply-To: <20180926132247.10971-6-laurentiu.tudor@nxp.com>

On Wed, Sep 26, 2018 at 8:26 AM <laurentiu.tudor@nxp.com> wrote:
>
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>
> Add a couple of new APIs to check the probing status of qman and bman:
>  'int bman_is_probed()' and 'int qman_is_probed()'.
> They return the following values.
>  *  1 if qman/bman were probed correctly
>  *  0 if qman/bman were not yet probed
>  * -1 if probing of qman/bman failed
> Drivers that use qman/bman driver services are required to use these
> APIs before calling any functions exported by qman or bman drivers
> or otherwise they will crash the kernel.
> The APIs will be used in the following couple of qbman portal patches
> and later in the series in the dpaa1 ethernet driver.
>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

As this is part of a bug fix for v4.19, applied on soc/fsl for fix.

> ---
>  drivers/soc/fsl/qbman/bman_ccsr.c | 11 +++++++++++
>  drivers/soc/fsl/qbman/qman_ccsr.c | 11 +++++++++++
>  include/soc/fsl/bman.h            |  8 ++++++++
>  include/soc/fsl/qman.h            |  8 ++++++++
>  4 files changed, 38 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c b/drivers/soc/fsl/qbman/bman_ccsr.c
> index d180da003e4a..b209c79511bb 100644
> --- a/drivers/soc/fsl/qbman/bman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/bman_ccsr.c
> @@ -121,6 +121,7 @@ static void bm_set_memory(u64 ba, u32 size)
>   */
>  static dma_addr_t fbpr_a;
>  static size_t fbpr_sz;
> +static int __bman_probed;
>
>  static int bman_fbpr(struct reserved_mem *rmem)
>  {
> @@ -167,6 +168,12 @@ static irqreturn_t bman_isr(int irq, void *ptr)
>         return IRQ_HANDLED;
>  }
>
> +int bman_is_probed(void)
> +{
> +       return __bman_probed;
> +}
> +EXPORT_SYMBOL_GPL(bman_is_probed);
> +
>  static int fsl_bman_probe(struct platform_device *pdev)
>  {
>         int ret, err_irq;
> @@ -177,6 +184,8 @@ static int fsl_bman_probe(struct platform_device *pdev)
>         u16 id, bm_pool_cnt;
>         u8 major, minor;
>
> +       __bman_probed = -1;
> +
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         if (!res) {
>                 dev_err(dev, "Can't get %pOF property 'IORESOURCE_MEM'\n",
> @@ -266,6 +275,8 @@ static int fsl_bman_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       __bman_probed = 1;
> +
>         return 0;
>  };
>
> diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c b/drivers/soc/fsl/qbman/qman_ccsr.c
> index 0cfe79f85a66..383a49dcce68 100644
> --- a/drivers/soc/fsl/qbman/qman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/qman_ccsr.c
> @@ -274,6 +274,7 @@ static const struct qman_error_info_mdata error_mdata[] = {
>  static u32 __iomem *qm_ccsr_start;
>  /* A SDQCR mask comprising all the available/visible pool channels */
>  static u32 qm_pools_sdqcr;
> +static int __qman_probed;
>
>  static inline u32 qm_ccsr_in(u32 offset)
>  {
> @@ -689,6 +690,12 @@ static int qman_resource_init(struct device *dev)
>         return 0;
>  }
>
> +int qman_is_probed(void)
> +{
> +       return __qman_probed;
> +}
> +EXPORT_SYMBOL_GPL(qman_is_probed);
> +
>  static int fsl_qman_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> @@ -699,6 +706,8 @@ static int fsl_qman_probe(struct platform_device *pdev)
>         u16 id;
>         u8 major, minor;
>
> +       __qman_probed = -1;
> +
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         if (!res) {
>                 dev_err(dev, "Can't get %pOF property 'IORESOURCE_MEM'\n",
> @@ -845,6 +854,8 @@ static int fsl_qman_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> +       __qman_probed = 1;
> +
>         return 0;
>  }
>
> diff --git a/include/soc/fsl/bman.h b/include/soc/fsl/bman.h
> index eaaf56df4086..5b99cb2ea5ef 100644
> --- a/include/soc/fsl/bman.h
> +++ b/include/soc/fsl/bman.h
> @@ -126,4 +126,12 @@ int bman_release(struct bman_pool *pool, const struct bm_buffer *bufs, u8 num);
>   */
>  int bman_acquire(struct bman_pool *pool, struct bm_buffer *bufs, u8 num);
>
> +/**
> + * bman_is_probed - Check if bman is probed
> + *
> + * Returns 1 if the bman driver successfully probed, -1 if the bman driver
> + * failed to probe or 0 if the bman driver did not probed yet.
> + */
> +int bman_is_probed(void);
> +
>  #endif /* __FSL_BMAN_H */
> diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
> index d4dfefdee6c1..597783b8a3a0 100644
> --- a/include/soc/fsl/qman.h
> +++ b/include/soc/fsl/qman.h
> @@ -1186,4 +1186,12 @@ int qman_alloc_cgrid_range(u32 *result, u32 count);
>   */
>  int qman_release_cgrid(u32 id);
>
> +/**
> + * qman_is_probed - Check if qman is probed
> + *
> + * Returns 1 if the qman driver successfully probed, -1 if the qman driver
> + * failed to probe or 0 if the qman driver did not probed yet.
> + */
> +int qman_is_probed(void);
> +
>  #endif /* __FSL_QMAN_H */
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: test_bpf: add dummy dev->net for flow_dissector
From: Willem de Bruijn @ 2018-09-27 14:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: songliubraving, Network Development, Daniel Borkmann, Kernel Team,
	Willem de Bruijn, Petar Penkov
In-Reply-To: <906045d5-206d-09a4-9de9-97789adbe701@gmail.com>

On Thu, Sep 27, 2018 at 9:00 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/27/2018 12:15 AM, Song Liu wrote:
> > Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
> > However, this is not true for test_bpf. As a result, test_bpf.ko crashes
> > the system with the following stack trace:
> >
>
> > +     dev_net_set(&dev, &net);
>
> This is yet another fake stuff that will break some day, so many other things would
> need to be setup properly in a real "struct net"
>
> What about using the current net_ns ?
>
> dev_net_set(&dev, current->nsproxy->net_ns);
>
> This might be &init_net in the context of test_bpf(), so maybe simply use &init_net
> to clearly state that test_bpf is not net ns fully ready.

Agreed.

Thanks for fixing this.

^ permalink raw reply

* Re: [PATCH iproute2] vxlan: show correct ttl inherit info
From: Hangbin Liu @ 2018-09-27 14:07 UTC (permalink / raw)
  To: Phil Sutter, netdev, Stephen Hemminger, David Ahern
In-Reply-To: <20180927092745.GV14666@orbyte.nwl.cc>

On Thu, Sep 27, 2018 at 11:27:45AM +0200, Phil Sutter wrote:
> Hi Hangbin,
> 
> On Thu, Sep 27, 2018 at 03:28:36PM +0800, Hangbin Liu wrote:
> > We should only show ttl inherit when IFLA_VXLAN_TTL_INHERIT supplied.
> > Otherwise show the ttl number, or auto when it is 0.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> >  ip/iplink_vxlan.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
> > index 831f39a..7fc0e2b 100644
> > --- a/ip/iplink_vxlan.c
> > +++ b/ip/iplink_vxlan.c
> > @@ -145,7 +145,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
> >  			NEXT_ARG();
> >  			check_duparg(&attrs, IFLA_VXLAN_TTL, "ttl", *argv);
> >  			if (strcmp(*argv, "inherit") == 0) {
> > -				addattr_l(n, 1024, IFLA_VXLAN_TTL_INHERIT, NULL, 0);
> > +				addattr(n, 1024, IFLA_VXLAN_TTL_INHERIT);
> 
> So for VXLAN, the attribute is just added but with a zero value. Looking
> at respective kernel code, this seems fine. Now I wonder why for Geneve,
> you set the value to 1 and when displaying explicitly check whether the
> attribute is there *and* non-zero. OK, looks like Geneve driver always
> exports IFLA_GENEVE_TTL_INHERIT. Oddly, I can't find where VXLAN driver
> in kernel does export the IFLA_VXLAN_TTL_INHERIT attribute. Am I missing
> something?

Hi Phil,

The vxlan ttl inherit exportation is just fixed by
net commit 8fd78069874 ("vxlan: fill ttl inherit info") yesterday.

> Do you know why handling of the attributes in both drivers differ?

That's because I set IFLA_VXLAN_TTL_INHERIT type to NLA_FLAG. But when fix
the geneve issue, I forgot this and set IFLA_GENEVE_TTL_INHERIT type to
NLA_U8... :(

I noticed this when post the iproute2 patch. But I thought this should have
no much influence, so I just leave it as it is.

If you think we'd better have a same behaver. We can drop the geneve iproute2
patch and post a fix to net kernel tree.

Thanks
Hangbin

^ 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