Linux wireless drivers development
 help / color / mirror / Atom feed
* RE: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: Alastair D'Silva @ 2019-04-15 10:59 UTC (permalink / raw)
  To: 'David Laight', 'Petr Mladek'
  Cc: 'Jani Nikula', 'Joonas Lahtinen',
	'Rodrigo Vivi', 'David Airlie',
	'Daniel Vetter', 'Karsten Keil',
	'Jassi Brar', 'Tom Lendacky',
	'David S. Miller', 'Jose Abreu',
	'Kalle Valo', 'Stanislaw Gruszka',
	'Benson Leung', 'Enric Balletbo i Serra',
	'James E.J. Bottomley', 'Martin K. Petersen',
	'Greg Kroah-Hartman', 'Alexander Viro',
	'Sergey Senozhatsky', 'Steven Rostedt',
	'Andrew Morton', intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <6912ef2d83d34c9299d5a5ad120c276f@AcuMS.aculab.com>

> From: Alastair D'Silva
> > Sent: 15 April 2019 11:29
> ...
> > I do, and I believe the choice of the output length should be in the
> > hands of the caller.
> >
> > On further thought, it would make more sense to remove the hardcoded
> > list of sizes and just enforce a power of 2. The function shouldn't
> > dictate what the caller can and can't do beyond the technical limits of it's
> implementation.
> 
> Why powers of two?
> You may want the length to match sizeof (struct foo).
> You might even want the address increment to be larger that the number of
> lines dumped.

Good point, the base requirement is that it should be a multiple of groupsize.

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece




^ permalink raw reply

* RE: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: David Laight @ 2019-04-15 10:56 UTC (permalink / raw)
  To: 'Alastair D'Silva', 'Petr Mladek'
  Cc: 'Jani Nikula', 'Joonas Lahtinen',
	'Rodrigo Vivi', 'David Airlie',
	'Daniel Vetter', 'Karsten Keil',
	'Jassi Brar', 'Tom Lendacky',
	'David S. Miller', 'Jose Abreu',
	'Kalle Valo', 'Stanislaw Gruszka',
	'Benson Leung', 'Enric Balletbo i Serra',
	'James E.J. Bottomley', 'Martin K. Petersen',
	'Greg Kroah-Hartman', 'Alexander Viro',
	'Sergey Senozhatsky', 'Steven Rostedt',
	'Andrew Morton', intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, ath10k@lists.infradead.org,
	linux-wireless@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-fbdev@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-fsdevel@vger.kernel.org
In-Reply-To: <0dad01d4f376$113df2b0$33b9d810$@d-silva.org>

From: Alastair D'Silva
> Sent: 15 April 2019 11:29
...
> I do, and I believe the choice of the output length should be in the hands
> of the caller.
> 
> On further thought, it would make more sense to remove the hardcoded list of
> sizes and just enforce a power of 2. The function shouldn't dictate what the
> caller can and can't do beyond the technical limits of it's implementation.

Why powers of two?
You may want the length to match sizeof (struct foo).
You might even want the address increment to be larger
that the number of lines dumped.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-04-15 10:44 UTC (permalink / raw)
  To: 'David Laight', 'Petr Mladek'
  Cc: 'Alastair D'Silva', 'Jani Nikula',
	'Joonas Lahtinen', 'Rodrigo Vivi',
	'David Airlie', 'Daniel Vetter',
	'Karsten Keil', 'Jassi Brar',
	'Tom Lendacky', 'David S. Miller',
	'Jose Abreu', 'Kalle Valo',
	'Stanislaw Gruszka', 'Benson Leung',
	'Enric Balletbo i Serra', 'James E.J. Bottomley',
	'Martin K. Petersen', 'Greg Kroah-Hartman',
	'Alexander Viro', 'Sergey Senozhatsky',
	'Steven Rostedt', 'Andrew Morton', intel-gfx,
	dri-devel, linux-kernel, netdev, ath10k, linux-wireless,
	linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <10c791893f034c26b4fee1a4659133e5@AcuMS.aculab.com>

> -----Original Message-----
> From: David Laight <David.Laight@ACULAB.COM>
> Sent: Monday, 15 April 2019 8:21 PM
> To: 'Alastair D'Silva' <alastair@d-silva.org>; 'Petr Mladek'
> <pmladek@suse.com>
> Cc: 'Alastair D'Silva' <alastair@au1.ibm.com>; 'Jani Nikula'
> <jani.nikula@linux.intel.com>; 'Joonas Lahtinen'
> <joonas.lahtinen@linux.intel.com>; 'Rodrigo Vivi' <rodrigo.vivi@intel.com>;
> 'David Airlie' <airlied@linux.ie>; 'Daniel Vetter' <daniel@ffwll.ch>; 'Karsten
> Keil' <isdn@linux-pingi.de>; 'Jassi Brar' <jassisinghbrar@gmail.com>; 'Tom
> Lendacky' <thomas.lendacky@amd.com>; 'David S. Miller'
> <davem@davemloft.net>; 'Jose Abreu' <Jose.Abreu@synopsys.com>; 'Kalle
> Valo' <kvalo@codeaurora.org>; 'Stanislaw Gruszka' <sgruszka@redhat.com>;
> 'Benson Leung' <bleung@chromium.org>; 'Enric Balletbo i Serra'
> <enric.balletbo@collabora.com>; 'James E.J. Bottomley'
> <jejb@linux.ibm.com>; 'Martin K. Petersen' <martin.petersen@oracle.com>;
> 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>; 'Alexander Viro'
> <viro@zeniv.linux.org.uk>; 'Sergey Senozhatsky'
> <sergey.senozhatsky@gmail.com>; 'Steven Rostedt'
> <rostedt@goodmis.org>; 'Andrew Morton' <akpm@linux-foundation.org>;
> intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org;
> ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> Subject: RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in
> hex_dump_to_buffer with flags
> 
> From: Alastair D'Silva
> > Sent: 15 April 2019 11:07
> ...
> > In the above example the author only wants the hex output, while in
> > other situations, both hex & ASCII output is desirable. If you just
> > want ASCII output, the caller should just use a printk or one of it's
> wrappers.
> 
> Hexdump will 'sanitise' the ASCII.
> 

This is functionality that doesn't exist in the current hexdump implementation (you always get the hex version).

I think a better option would be to factor out the sanitisation and expose that as a separate function.

> Although I think you'd want a 'no hex' flag to suppress the hex.
> 
> Probably more useful flags are ones to suppress the address column.

This is already supported by the prefix_type parameter - are you proposing that we eliminate the parameter & combine it with flags?

> I've also used flags to enable (or disable) suppression of multiple lines of
> zeros of constant bytes.
> In that case you may want hexdump to return the flags for the next call when
> a large buffer is being dumped in fragments.
 
I'm afraid I don't quite follow here, hex_dump_to_buffer doesn't alter the flags, so the caller already knows it. 

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece





^ permalink raw reply

* Re: gsmtap design/extensions?
From: Johannes Berg @ 2019-04-15 10:41 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Marcel Holtmann, Vadim Yanitskiy, Harald Welte,
	OpenBSC Mailing List, Sean Tranchetti, radiotap, Dan Williams,
	netdev, open list:NFC SUBSYSTEM, Aleksander Morgado,
	Subash Abhinov Kasiviswanathan
In-Reply-To: <87d0ln1s0a.fsf@miraculix.mork.no>

On Mon, 2019-04-15 at 12:29 +0200, Bjørn Mork wrote:
> 
> I don't understand where you are going here? Neither QMI, AT command nor
> MBIM management are transported over netdevs. AT is usually transported
> using simple USB bulk streams, exported to userspace as tty devices by
> some USB serial driver.  Both QMI and MBIM management are transported as
> USB control messages, and exported to userspace as chardevs.  There are
> no netdevs involved.

OK. I've also been looking at a driver for Intel modems, and I think it
works that way.

> > and people do things like "socat" to set up
> > PTYs and pretend to have serial channels there, on top of the netdevs?
> 
> I assume you are referring to my MBIM DSS examples here.

I was more thinking of what I've been told the Intel modem does,
actually. But that in turn might very well be inspired by what you
documented there. I think the folks doing this were just trying to make
it work and nobody really understands the whole landscape.

> I don't know
> if anyone is actually using that, so you should probably ignore it...
> I'll happily admit it was a bad idea.  Should have just added the
> necessary code to map DSS channels to some sort of character device in
> the driver, like most users requested.

OK. So I guess a framework should consider that possibility, and let you
create new chardevs for a given (DSS) channel?

> But there really isn't anything in the MBIM spec saying how DSS should
> be used. DSS is a generic data stream.  Could easily be connected to a
> single TCP session for example, in which case you'd probably want to
> connect it to a TCP session in the other end too.  So I wouldn't want to
> force DSS into character devices on the host end.  

Fair point.

I suppose some of these may also be debug channels that give you modem
debug information, which is useful (if you can interpret it).

> This doesn't rule out
> a userspace controlled optional mapping though. We could probably still
> add that, replacing the VLAN mapping with a chardev for a specific DSS
> session if requested by userspace.  I guess this is something to
> consider for a generic cellular framework - supporting non-ip data
> streams between modem and host.

Right.

> Adding to my previous excuses: The DSS implementation in the cdc_mbim
> driver was added without having seen a single modem firmware with *any*
> type of DSS support.  It was purely based on spec{ification,culation}.
> The VLAN mapping, along with examples of how to use socat to further map
> the streams from VLANs into suitable application specific forms, seemed
> like a simple and flexible enough solution.

:-)

johannes


^ permalink raw reply

* [PATCH] iwlegacy: fix spelling mistake "acumulative" -> "accumulative"
From: Colin King @ 2019-04-15 10:37 UTC (permalink / raw)
  To: Stanislaw Gruszka, Kalle Valo, David S . Miller, linux-wireless,
	netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Fix spelling mistakes in rx stats text. I missed these from an earlier
round of fixing the same spelling mistake.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/intel/iwlegacy/3945-debug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlegacy/3945-debug.c b/drivers/net/wireless/intel/iwlegacy/3945-debug.c
index a2960032be81..4b912e707f38 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-debug.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-debug.c
@@ -185,7 +185,7 @@ il3945_ucode_rx_stats_read(struct file *file, char __user *user_buf,
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
 		      "%-32s     current"
-		      "acumulative       delta         max\n",
+		      "accumulative      delta         max\n",
 		      "Statistics_Rx - CCK:");
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
@@ -273,7 +273,7 @@ il3945_ucode_rx_stats_read(struct file *file, char __user *user_buf,
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
 		      "%-32s     current"
-		      "acumulative       delta         max\n",
+		      "accumulative      delta         max\n",
 		      "Statistics_Rx - GENERAL:");
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
@@ -346,7 +346,7 @@ il3945_ucode_tx_stats_read(struct file *file, char __user *user_buf,
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
 		      "%-32s     current"
-		      "acumulative       delta         max\n",
+		      "accumulative      delta         max\n",
 		      "Statistics_Tx:");
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
@@ -447,7 +447,7 @@ il3945_ucode_general_stats_read(struct file *file, char __user *user_buf,
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
 		      "%-32s     current"
-		      "acumulative       delta         max\n",
+		      "accumulative      delta         max\n",
 		      "Statistics_General:");
 	pos +=
 	    scnprintf(buf + pos, bufsz - pos,
-- 
2.20.1


^ permalink raw reply related

* RE: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
From: Alastair D'Silva @ 2019-04-15 10:33 UTC (permalink / raw)
  To: 'Petr Mladek'
  Cc: 'Alastair D'Silva', 'Jani Nikula',
	'Joonas Lahtinen', 'Rodrigo Vivi',
	'David Airlie', 'Daniel Vetter',
	'Karsten Keil', 'Jassi Brar',
	'Tom Lendacky', 'David S. Miller',
	'Jose Abreu', 'Kalle Valo',
	'Stanislaw Gruszka', 'Benson Leung',
	'Enric Balletbo i Serra', 'James E.J. Bottomley',
	'Martin K. Petersen', 'Greg Kroah-Hartman',
	'Alexander Viro', 'Sergey Senozhatsky',
	'Steven Rostedt', 'Andrew Morton', intel-gfx,
	dri-devel, linux-kernel, netdev, ath10k, linux-wireless,
	linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <20190415091812.s4e5zlwldbe62ego@pathway.suse.cz>

> > > On Wed 2019-04-10 13:17:18, Alastair D'Silva wrote:
> > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > >
> > > > Some buffers may only be partially filled with useful data, while
> > > > the rest is padded (typically with 0x00 or 0xff).
> > > >
> > > > This patch introduces flags which allow lines of padding bytes to
> > > > be suppressed, making the output easier to interpret:
> > > > HEXDUMP_SUPPRESS_0X00, HEXDUMP_SUPPRESS_0XFF
> > > >
> > > > The first and last lines are not suppressed by default, so the
> > > > function always outputs something. This behaviour can be further
> > > > controlled with the HEXDUMP_SUPPRESS_FIRST &
> > > HEXDUMP_SUPPRESS_LAST flags.
> > > >
> > > > An inline wrapper function is provided for backwards compatibility
> > > > with existing code, which maintains the original behaviour.
> > > >
> > >
> > > > diff --git a/lib/hexdump.c b/lib/hexdump.c index
> > > > b8a164814744..2f3bafb55a44 100644
> > > > --- a/lib/hexdump.c
> > > > +++ b/lib/hexdump.c
> > > > +void print_hex_dump_ext(const char *level, const char *prefix_str,
> > > > +			int prefix_type, int rowsize, int groupsize,
> > > > +			const void *buf, size_t len, u64 flags)
> > > >  {
> > > >  	const u8 *ptr = buf;
> > > > -	int i, linelen, remaining = len;
> > > > +	int i, remaining = len;
> > > >  	unsigned char linebuf[64 * 3 + 2 + 64 + 1];
> > > > +	bool first_line = true;
> > > >
> > > >  	if (rowsize != 16 && rowsize != 32 && rowsize != 64)
> > > >  		rowsize = 16;
> > > >
> > > >  	for (i = 0; i < len; i += rowsize) {
> > > > -		linelen = min(remaining, rowsize);
> > > > +		bool skip = false;
> > > > +		int linelen = min(remaining, rowsize);
> > > > +
> > > >  		remaining -= rowsize;
> > > >
> > > > +		if (flags & HEXDUMP_SUPPRESS_0X00)
> > > > +			skip = buf_is_all(ptr + i, linelen, 0x00);
> > > > +
> > > > +		if (!skip && (flags & HEXDUMP_SUPPRESS_0XFF))
> > > > +			skip = buf_is_all(ptr + i, linelen, 0xff);
> > > > +
> > > > +		if (first_line && !(flags & HEXDUMP_SUPPRESS_FIRST))
> > > > +			skip = false;
> > > > +
> > > > +		if (remaining <= 0 && !(flags &
HEXDUMP_SUPPRESS_LAST))
> > > > +			skip = false;
> > > > +
> > > > +		if (skip)
> > > > +			continue;
> > >
> > > IMHO, quietly skipping lines could cause a lot of confusion,
> > > espcially
> > when the address is not printed.
> > >
> > It's up to the caller to decide how they want it displayed.
> 
> I wonder who would want to quietly skip some data values.
> Are you using it yourself? Could you please provide an example?

Yes, but I don't have the content with me at the moment, so I can't share
it. I'm dumping persistent memory labels, which are 64kB long, but only the
first few hundred bytes are populated.
 
> I do not see why we would need to complicate the API and code by this.
> 
> The behavior proposed by Tvrtko Ursulin makes much more sense. I mean
> https://lkml.kernel.org/r/929244ed-cc7f-b0f3-b5ac-
> 50e798e83188@linux.intel.com

I agree that is better, I'll add that to V2.

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece




^ permalink raw reply

* Re: gsmtap design/extensions?
From: Bjørn Mork @ 2019-04-15 10:29 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Marcel Holtmann, Vadim Yanitskiy, Harald Welte,
	OpenBSC Mailing List, Sean Tranchetti, radiotap, Dan Williams,
	netdev, open list:NFC SUBSYSTEM, Aleksander Morgado,
	Subash Abhinov Kasiviswanathan
In-Reply-To: <92e8e142b6d441c1c995abc57d64ad7b7747a688.camel@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

>> As I said, as long as you do not get the QMI, AT command, MBIM etc.
>> control path session recorded as well, you have nothing to really
>> analyze.
>
> But you do - afaict there are no ways other than the netdevs to
> communicate with the device,

I don't understand where you are going here? Neither QMI, AT command nor
MBIM management are transported over netdevs. AT is usually transported
using simple USB bulk streams, exported to userspace as tty devices by
some USB serial driver.  Both QMI and MBIM management are transported as
USB control messages, and exported to userspace as chardevs.  There are
no netdevs involved.

> and people do things like "socat" to set up
> PTYs and pretend to have serial channels there, on top of the netdevs?

I assume you are referring to my MBIM DSS examples here.  I don't know
if anyone is actually using that, so you should probably ignore it...
I'll happily admit it was a bad idea.  Should have just added the
necessary code to map DSS channels to some sort of character device in
the driver, like most users requested.

But there really isn't anything in the MBIM spec saying how DSS should
be used. DSS is a generic data stream.  Could easily be connected to a
single TCP session for example, in which case you'd probably want to
connect it to a TCP session in the other end too.  So I wouldn't want to
force DSS into character devices on the host end.  This doesn't rule out
a userspace controlled optional mapping though. We could probably still
add that, replacing the VLAN mapping with a chardev for a specific DSS
session if requested by userspace.  I guess this is something to
consider for a generic cellular framework - supporting non-ip data
streams between modem and host.

Adding to my previous excuses: The DSS implementation in the cdc_mbim
driver was added without having seen a single modem firmware with *any*
type of DSS support.  It was purely based on spec{ification,culation}.
The VLAN mapping, along with examples of how to use socat to further map
the streams from VLANs into suitable application specific forms, seemed
like a simple and flexible enough solution.


Bjørn

^ permalink raw reply

* RE: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: Alastair D'Silva @ 2019-04-15 10:29 UTC (permalink / raw)
  To: 'Petr Mladek'
  Cc: 'Jani Nikula', 'Joonas Lahtinen',
	'Rodrigo Vivi', 'David Airlie',
	'Daniel Vetter', 'Karsten Keil',
	'Jassi Brar', 'Tom Lendacky',
	'David S. Miller', 'Jose Abreu',
	'Kalle Valo', 'Stanislaw Gruszka',
	'Benson Leung', 'Enric Balletbo i Serra',
	'James E.J. Bottomley', 'Martin K. Petersen',
	'Greg Kroah-Hartman', 'Alexander Viro',
	'Sergey Senozhatsky', 'Steven Rostedt',
	'Andrew Morton', intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <20190415090232.3ualhrt5ssrb2ixq@pathway.suse.cz>

<snip>
> > > On Wed 2019-04-10 13:17:17, Alastair D'Silva wrote:
> > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > >
> > > > With modern high resolution screens, we can display more data,
> > > > which makes life a bit easier when debugging.
> > >
> > > I have quite some doubts about this feature.
> > >
> > > We are talking about more than 256 characters per-line. I wonder if
> > > such a long line is really easier to read for a human.
> >
> > It's basically 2 separate panes of information side by side, the
> > hexdump and the ASCII version.
> >
> > I'm using this myself when dealing with the pmem labels, and it works
> > quite nicely.
> 
> I am sure that it works for you. But I do not believe that it would be
useful in
> general.

I do, and I believe the choice of the output length should be in the hands
of the caller.

On further thought, it would make more sense to remove the hardcoded list of
sizes and just enforce a power of 2. The function shouldn't dictate what the
caller can and can't do beyond the technical limits of it's implementation.

Other print/debug functions don't restrict the output size, and I can't see
a good justification why hexdump should either.

> > > I am not expert but there is a reason why the standard is 80
> > > characters
> > per-
> > > line. I guess that anything above 100 characters is questionable.
> > > https://en.wikipedia.org/wiki/Line_length
> > > somehow confirms that.
> > >
> > > Second, if we take 8 pixels per-character. Then we need
> > > 2048 to show the 256 characters. It is more than HD.
> > > IMHO, there is still huge number of people that even do not have HD
> > display,
> > > especially on a notebook.
> >
> > The intent is to make debugging easier when dealing with large chunks
> > of binary data. I don't expect end users to see this output.
> 
> How is it supposed to be used then? Only by your temporary patches?

Let me rephrase that, I don't expect end users to *use* this data.

Current usage of the hexdump functions are predominantly centred around
logging and debugging, and clearly targeted at someone intimately familiar
with the relevant subsystem. I expect future use would be similar.

Debugging may be as part of active development, or from a log supplied from
an end user. In either case, it should be up to the author (as a
representative for the consumers of the data) to decide how it should be
formatted.

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece






^ permalink raw reply

* RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: David Laight @ 2019-04-15 10:20 UTC (permalink / raw)
  To: 'Alastair D'Silva', 'Petr Mladek'
  Cc: 'Alastair D'Silva', 'Jani Nikula',
	'Joonas Lahtinen', 'Rodrigo Vivi',
	'David Airlie', 'Daniel Vetter',
	'Karsten Keil', 'Jassi Brar',
	'Tom Lendacky', 'David S. Miller',
	'Jose Abreu', 'Kalle Valo',
	'Stanislaw Gruszka', 'Benson Leung',
	'Enric Balletbo i Serra', 'James E.J. Bottomley',
	'Martin K. Petersen', 'Greg Kroah-Hartman',
	'Alexander Viro', 'Sergey Senozhatsky',
	'Steven Rostedt', 'Andrew Morton',
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	ath10k@lists.infradead.org, linux-wireless@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-fbdev@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-fsdevel@vger.kernel.org
In-Reply-To: <0da301d4f373$01d5bb80$05813280$@d-silva.org>

From: Alastair D'Silva
> Sent: 15 April 2019 11:07
...
> In the above example the author only wants the hex output, while in other
> situations, both hex & ASCII output is desirable. If you just want ASCII
> output, the caller should just use a printk or one of it's wrappers.

Hexdump will 'sanitise' the ASCII.

Although I think you'd want a 'no hex' flag to suppress the hex.

Probably more useful flags are ones to suppress the address column.
I've also used flags to enable (or disable) suppression of multiple
lines of zeros of constant bytes.
In that case you may want hexdump to return the flags for the
next call when a large buffer is being dumped in fragments.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* RE: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-04-15 10:07 UTC (permalink / raw)
  To: 'Petr Mladek'
  Cc: 'Alastair D'Silva', 'Jani Nikula',
	'Joonas Lahtinen', 'Rodrigo Vivi',
	'David Airlie', 'Daniel Vetter',
	'Karsten Keil', 'Jassi Brar',
	'Tom Lendacky', 'David S. Miller',
	'Jose Abreu', 'Kalle Valo',
	'Stanislaw Gruszka', 'Benson Leung',
	'Enric Balletbo i Serra', 'James E.J. Bottomley',
	'Martin K. Petersen', 'Greg Kroah-Hartman',
	'Alexander Viro', 'Sergey Senozhatsky',
	'Steven Rostedt', 'Andrew Morton', intel-gfx,
	dri-devel, linux-kernel, netdev, ath10k, linux-wireless,
	linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <20190415092424.qos7d54nbyr5hphu@pathway.suse.cz>

> -----Original Message-----
> From: Petr Mladek <pmladek@suse.com>
> Sent: Monday, 15 April 2019 7:24 PM
> To: Alastair D'Silva <alastair@d-silva.org>
> Cc: 'Alastair D'Silva' <alastair@au1.ibm.com>; 'Jani Nikula'
> <jani.nikula@linux.intel.com>; 'Joonas Lahtinen'
> <joonas.lahtinen@linux.intel.com>; 'Rodrigo Vivi'
<rodrigo.vivi@intel.com>;
> 'David Airlie' <airlied@linux.ie>; 'Daniel Vetter' <daniel@ffwll.ch>;
'Karsten
> Keil' <isdn@linux-pingi.de>; 'Jassi Brar' <jassisinghbrar@gmail.com>; 'Tom
> Lendacky' <thomas.lendacky@amd.com>; 'David S. Miller'
> <davem@davemloft.net>; 'Jose Abreu' <Jose.Abreu@synopsys.com>; 'Kalle
> Valo' <kvalo@codeaurora.org>; 'Stanislaw Gruszka' <sgruszka@redhat.com>;
> 'Benson Leung' <bleung@chromium.org>; 'Enric Balletbo i Serra'
> <enric.balletbo@collabora.com>; 'James E.J. Bottomley'
> <jejb@linux.ibm.com>; 'Martin K. Petersen' <martin.petersen@oracle.com>;
> 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org>; 'Alexander Viro'
> <viro@zeniv.linux.org.uk>; 'Sergey Senozhatsky'
> <sergey.senozhatsky@gmail.com>; 'Steven Rostedt'
> <rostedt@goodmis.org>; 'Andrew Morton' <akpm@linux-foundation.org>;
> intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org;
> ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in
> hex_dump_to_buffer with flags
> 
> On Sat 2019-04-13 09:31:27, Alastair D'Silva wrote:
> > > -----Original Message-----
> > > From: Petr Mladek <pmladek@suse.com>
> > > Sent: Saturday, 13 April 2019 12:12 AM
> > > To: Alastair D'Silva <alastair@au1.ibm.com>
> > > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>;
> > Joonas
> > > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi
> > > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel
> > > Vetter <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi
> > > Brar <jassisinghbrar@gmail.com>; Tom Lendacky
> > > <thomas.lendacky@amd.com>; David S. Miller
> <davem@davemloft.net>;
> > > Jose Abreu <Jose.Abreu@synopsys.com>; Kalle Valo
> > > <kvalo@codeaurora.org>; Stanislaw Gruszka <sgruszka@redhat.com>;
> > > Benson Leung <bleung@chromium.org>; Enric Balletbo i Serra
> > > <enric.balletbo@collabora.com>; James E.J. Bottomley
> > > <jejb@linux.ibm.com>; Martin K. Petersen
> > > <martin.petersen@oracle.com>; Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org>; Alexander Viro
> > > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky
> > > <sergey.senozhatsky@gmail.com>; Steven Rostedt
> > > <rostedt@goodmis.org>; Andrew Morton <akpm@linux-
> foundation.org>;
> > > intel- gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > > linux- kernel@vger.kernel.org; netdev@vger.kernel.org;
> > > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> > > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> > > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> > > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in
> > > hex_dump_to_buffer with flags
> > >
> > > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote:
> > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > >
> > > > In order to support additional features in hex_dump_to_buffer,
> > > > replace the ascii bool parameter with flags.
> > > >
> > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_engine_cs.c            |  2 +-
> > > >  drivers/isdn/hardware/mISDN/mISDNisar.c           |  6 ++++--
> > > >  drivers/mailbox/mailbox-test.c                    |  2 +-
> > > >  drivers/net/ethernet/amd/xgbe/xgbe-drv.c          |  2 +-
> > > >  drivers/net/ethernet/synopsys/dwc-xlgmac-common.c |  2 +-
> > > >  drivers/net/wireless/ath/ath10k/debug.c           |  3 ++-
> > > >  drivers/net/wireless/intel/iwlegacy/3945-mac.c    |  2 +-
> > > >  drivers/platform/chrome/wilco_ec/debugfs.c        |  3 ++-
> > > >  drivers/scsi/scsi_logging.c                       |  8 +++-----
> > > >  drivers/staging/fbtft/fbtft-core.c                |  2 +-
> > > >  fs/seq_file.c                                     |  3 ++-
> > > >  include/linux/printk.h                            |  2 +-
> > > >  lib/hexdump.c                                     | 15
++++++++-------
> > > >  lib/test_hexdump.c                                |  5 +++--
> > > >  14 files changed, 31 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > index 49fa43ff02ba..fb133e729f9a 100644
> > > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m,
> > > > const
> > > void *buf, size_t len)
> > > >  		WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len -
> > > pos,
> > > >  						rowsize,
sizeof(u32),
> > > >  						line, sizeof(line),
> > > > -						false) >=
sizeof(line));
> > > > +						0) >= sizeof(line));
> > >
> > > It might be more clear when we define:
> > >
> > > #define HEXDUMP_BINARY 0
> >
> > This feels unnecessary, and weird. Omitting the flag won't disable the
> > hex output (as expected), and if you don't want hex output why call
> > hexdump in the first place?
> 
> Why do we have HEXDUMP_ASCII then?
> Why is the above funtion not using HEXDUMP_ASCII?
> Who would call it with (HEXDUMP_ASCII | HEXDUMP_BINARY)?

The ASCII flag controls the optional ASCII output, and replaces the boolean
that existed prior. A user would enable it in the same situations where they
currently pass true for the ascii parameter.

In the above example the author only wants the hex output, while in other
situations, both hex & ASCII output is desirable. If you just want ASCII
output, the caller should just use a printk or one of it's wrappers.

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece


^ permalink raw reply

* [PATCH] mac80211: don't attempt to rename ERR_PTR() debugfs dirs
From: Johannes Berg @ 2019-04-15  9:39 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

We need to dereference the directory to get its parent to
be able to rename it, so it's clearly not safe to try to
do this with ERR_PTR() pointers. Skip in this case.

It seems that this is most likely what was causing the
report by syzbot, but I'm not entirely sure as it didn't
come with a reproducer this time.

Cc: stable@vger.kernel.org
Reported-by: syzbot+4ece1a28b8f4730547c9@syzkaller.appspotmail.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/debugfs_netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index cff0fb3578c9..deb3faf08337 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -841,7 +841,7 @@ void ieee80211_debugfs_rename_netdev(struct ieee80211_sub_if_data *sdata)
 
 	dir = sdata->vif.debugfs_dir;
 
-	if (!dir)
+	if (IS_ERR_OR_NULL(dir))
 		return;
 
 	sprintf(buf, "netdev:%s", sdata->name);
-- 
2.17.2


^ permalink raw reply related

* Re: gsmtap design/extensions?
From: Johannes Berg @ 2019-04-15  9:28 UTC (permalink / raw)
  To: Guy Harris
  Cc: Harald Welte, openbsc, radiotap, linux-wireless,
	Subash Abhinov Kasiviswanathan, Dan Williams, Bjørn Mork,
	netdev, Sean Tranchetti, Aleksander Morgado
In-Reply-To: <1089142F-2966-4C41-921B-465FBA721E79@alum.mit.edu>

On Fri, 2019-04-12 at 12:48 -0700, Guy Harris wrote:

> I.e., there's a split there between "capture" and "getting the packets
> from a capture delivered to you over an IP network".

Right. That's what I was trying to get at, you said it much more
succinctly.

> Depending on how your system is set up:
> 
> 	$ ls -l /dev/bpf*
> 	crw-rw----  1 root  access_bpf   23,   0 Apr 10 22:57 /dev/bpf0
> 	crw-rw----  1 root  access_bpf   23,   1 Apr 10 22:56 /dev/bpf1
> 
> 		...
> 
> and it could just be rw-rw-rw-.  Perhaps other systems make it harder
> to grant capture privileges.

Yeah, generally it's not that *hard*. It still seems *wrong*, and e.g.
it would also mean if you do need to do it remotely you'd have to filter
out your own remote control (ssh) traffic lest you cause traffic
amplification, etc.

johannes


^ permalink raw reply

* Re: gsmtap design/extensions?
From: Johannes Berg @ 2019-04-15  9:26 UTC (permalink / raw)
  To: Harald Welte
  Cc: openbsc, radiotap, linux-wireless, Subash Abhinov Kasiviswanathan,
	Dan Williams, Bjørn Mork, netdev, Sean Tranchetti,
	Aleksander Morgado
In-Reply-To: <20190413073505.GD24451@nataraja>

Hi Harald,

> Agreed.  However, the protocol stacks on WiFi and cellular technologies
> are very different, to say the least.  There is no easy mapping of PHY
> related parameters to a given IP packet, and the related quality metrics
> of the radio channels don't work that way.

Sure. I'm (vaguely) aware of that.

> But yes, I agree, that whatever transport mechanism you wan to use between the modem
> and the user/Linux side should allow for additional, extensible metadata beyond
> the identifier of the PDP/PDN context.

Really the question it goes down to is whether there's possible overlap
here with GSMTAP or not.

If yes - we can define a GSMTAPv3, with TLVs or so, and add a "PDP/PDN
context" or "session" field or something like that, and maybe a tag that
says "IP packet is here", or some kind of "content type" field. I expect
there would be some such "sessions" that actually just transport the
local AT command chat.

If no, I guess we'll just define something else for this use case? Or,
perhaps, even get rid of it entirely and just rely on trace-cmd
recording or so, though then I guess we'd really want to teach libpcap
and/or wireshark to understand this in some way.

Though it almost sounds like in GSMTAP you're never actually
transporting IP data, but other types of packets that in turn transport
the (fragmented/compressed/encrypted) IP data. That would kinda mean
there's very little potential overlap.

> > This is the part I guess I don't understand. I haven't really tried, but
> > it seems you should be able to encapsulate arbitrary protocols into
> > 802.3 OUI-based ethertype or so?
> 
> But why?  I'm in an userspace program, and I want to send data to one or
> more other userspace programs.  Why would I not simply open a UDP socket
> to do so?  I would have to have CAP_NET_RAW and open a packet socket,
> and then generate ethernet frames from that?
> 
> I admit that the use case with wireshark is a bit odd, but there are
> other receivers out there.

Yeah, ok. I was thinking kinda the other way around - if all you need is
network transparency for wireshark it shouldn't matter, but that's
really discussed over in the other subthread with Guy Harris better.

Anyway it doesn't matter, and I'm beginning to understand that a (maybe
even the primary) use case is getting multiple parts of your stack to
talk to each other (over UDP).

> Yes, you're looking only at a single interface (the radio interface
> between one BSS and one STA).  You're not looking at five different
> interfaces at five different levels of network hierarchy/topology in the "wifi
> controller" and want to mix in a radio interface trace in the same
> timeline.

Indeed. Well, actually, we often do, but use different mechanics for
that (trace-cmd record comes to mind, it records all our driver/fw chat
as well as possibly adding in logging from wpa_supplicant etc.)

> > Basically, I was looking at it as if it was like WiFi in a sense - you
> > have an IP frame, you're going to transport it over some physical link,
> > so it gets PHY information in the sense of modulation etc.
> 
> As stated elsewhere, there's an M:N mapping between user (IP) payload
> and actual radio interface "MAC blocks", so I'm not aware of anyone
> mapping radio interface performance to user plane IP.

Right, OK.

> the "physical link info" is present in GSMTAP, but the granularity of
> GSMTAP frames is not user-IP frames, but "MAC blocks".  So your user IP
> frame might not be visible as it's still compressed, encrypted,
> fragmented, etc.

Sure. As Guy Harris pointed out, this is also true for wifi though - we
don't have compression, but certainly encryption & fragmentation,
wireshark was taught to handle that and recreate the original MSDU (mac
service data unit).

johannes


^ permalink raw reply

* Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Petr Mladek @ 2019-04-15  9:24 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: 'Alastair D'Silva', 'Jani Nikula',
	'Joonas Lahtinen', 'Rodrigo Vivi',
	'David Airlie', 'Daniel Vetter',
	'Karsten Keil', 'Jassi Brar',
	'Tom Lendacky', 'David S. Miller',
	'Jose Abreu', 'Kalle Valo',
	'Stanislaw Gruszka', 'Benson Leung',
	'Enric Balletbo i Serra', 'James E.J. Bottomley',
	'Martin K. Petersen', 'Greg Kroah-Hartman',
	'Alexander Viro', 'Sergey Senozhatsky',
	'Steven Rostedt', 'Andrew Morton', intel-gfx,
	dri-devel, linux-kernel, netdev, ath10k, linux-wireless,
	linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <093301d4f187$da0756a0$8e1603e0$@d-silva.org>

On Sat 2019-04-13 09:31:27, Alastair D'Silva wrote:
> > -----Original Message-----
> > From: Petr Mladek <pmladek@suse.com>
> > Sent: Saturday, 13 April 2019 12:12 AM
> > To: Alastair D'Silva <alastair@au1.ibm.com>
> > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>;
> Joonas
> > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi
> > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel Vetter
> > <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi Brar
> > <jassisinghbrar@gmail.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> > David S. Miller <davem@davemloft.net>; Jose Abreu
> > <Jose.Abreu@synopsys.com>; Kalle Valo <kvalo@codeaurora.org>;
> > Stanislaw Gruszka <sgruszka@redhat.com>; Benson Leung
> > <bleung@chromium.org>; Enric Balletbo i Serra
> > <enric.balletbo@collabora.com>; James E.J. Bottomley
> > <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro
> > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky
> > <sergey.senozhatsky@gmail.com>; Steven Rostedt <rostedt@goodmis.org>;
> > Andrew Morton <akpm@linux-foundation.org>; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org;
> > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> > Subject: Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in
> > hex_dump_to_buffer with flags
> > 
> > On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote:
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > In order to support additional features in hex_dump_to_buffer, replace
> > > the ascii bool parameter with flags.
> > >
> > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > ---
> > >  drivers/gpu/drm/i915/intel_engine_cs.c            |  2 +-
> > >  drivers/isdn/hardware/mISDN/mISDNisar.c           |  6 ++++--
> > >  drivers/mailbox/mailbox-test.c                    |  2 +-
> > >  drivers/net/ethernet/amd/xgbe/xgbe-drv.c          |  2 +-
> > >  drivers/net/ethernet/synopsys/dwc-xlgmac-common.c |  2 +-
> > >  drivers/net/wireless/ath/ath10k/debug.c           |  3 ++-
> > >  drivers/net/wireless/intel/iwlegacy/3945-mac.c    |  2 +-
> > >  drivers/platform/chrome/wilco_ec/debugfs.c        |  3 ++-
> > >  drivers/scsi/scsi_logging.c                       |  8 +++-----
> > >  drivers/staging/fbtft/fbtft-core.c                |  2 +-
> > >  fs/seq_file.c                                     |  3 ++-
> > >  include/linux/printk.h                            |  2 +-
> > >  lib/hexdump.c                                     | 15 ++++++++-------
> > >  lib/test_hexdump.c                                |  5 +++--
> > >  14 files changed, 31 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > index 49fa43ff02ba..fb133e729f9a 100644
> > > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > > @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const
> > void *buf, size_t len)
> > >  		WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len -
> > pos,
> > >  						rowsize, sizeof(u32),
> > >  						line, sizeof(line),
> > > -						false) >= sizeof(line));
> > > +						0) >= sizeof(line));
> > 
> > It might be more clear when we define:
> > 
> > #define HEXDUMP_BINARY 0
> 
> This feels unnecessary, and weird. Omitting the flag won't disable the hex
> output (as expected), and if you don't want hex output why call hexdump in
> the first place?

Why do we have HEXDUMP_ASCII then?
Why is the above funtion not using HEXDUMP_ASCII?
Who would call it with (HEXDUMP_ASCII | HEXDUMP_BINARY)?

Best Regards,
Petr

^ permalink raw reply

* Re: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
From: Petr Mladek @ 2019-04-15  9:18 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: 'Alastair D'Silva', 'Jani Nikula',
	'Joonas Lahtinen', 'Rodrigo Vivi',
	'David Airlie', 'Daniel Vetter',
	'Karsten Keil', 'Jassi Brar',
	'Tom Lendacky', 'David S. Miller',
	'Jose Abreu', 'Kalle Valo',
	'Stanislaw Gruszka', 'Benson Leung',
	'Enric Balletbo i Serra', 'James E.J. Bottomley',
	'Martin K. Petersen', 'Greg Kroah-Hartman',
	'Alexander Viro', 'Sergey Senozhatsky',
	'Steven Rostedt', 'Andrew Morton', intel-gfx,
	dri-devel, linux-kernel, netdev, ath10k, linux-wireless,
	linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <093101d4f187$612f0f20$238d2d60$@d-silva.org>

On Sat 2019-04-13 09:28:03, Alastair D'Silva wrote:
> > -----Original Message-----
> > From: Petr Mladek <pmladek@suse.com>
> > Sent: Saturday, 13 April 2019 12:04 AM
> > To: Alastair D'Silva <alastair@au1.ibm.com>
> > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>;
> Joonas
> > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi
> > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel Vetter
> > <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi Brar
> > <jassisinghbrar@gmail.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> > David S. Miller <davem@davemloft.net>; Jose Abreu
> > <Jose.Abreu@synopsys.com>; Kalle Valo <kvalo@codeaurora.org>;
> > Stanislaw Gruszka <sgruszka@redhat.com>; Benson Leung
> > <bleung@chromium.org>; Enric Balletbo i Serra
> > <enric.balletbo@collabora.com>; James E.J. Bottomley
> > <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro
> > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky
> > <sergey.senozhatsky@gmail.com>; Steven Rostedt <rostedt@goodmis.org>;
> > Andrew Morton <akpm@linux-foundation.org>; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org;
> > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> > Subject: Re: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of
> filler
> > bytes
> > 
> > On Wed 2019-04-10 13:17:18, Alastair D'Silva wrote:
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > Some buffers may only be partially filled with useful data, while the
> > > rest is padded (typically with 0x00 or 0xff).
> > >
> > > This patch introduces flags which allow lines of padding bytes to be
> > > suppressed, making the output easier to interpret:
> > > HEXDUMP_SUPPRESS_0X00, HEXDUMP_SUPPRESS_0XFF
> > >
> > > The first and last lines are not suppressed by default, so the
> > > function always outputs something. This behaviour can be further
> > > controlled with the HEXDUMP_SUPPRESS_FIRST &
> > HEXDUMP_SUPPRESS_LAST flags.
> > >
> > > An inline wrapper function is provided for backwards compatibility
> > > with existing code, which maintains the original behaviour.
> > >
> > 
> > > diff --git a/lib/hexdump.c b/lib/hexdump.c index
> > > b8a164814744..2f3bafb55a44 100644
> > > --- a/lib/hexdump.c
> > > +++ b/lib/hexdump.c
> > > +void print_hex_dump_ext(const char *level, const char *prefix_str,
> > > +			int prefix_type, int rowsize, int groupsize,
> > > +			const void *buf, size_t len, u64 flags)
> > >  {
> > >  	const u8 *ptr = buf;
> > > -	int i, linelen, remaining = len;
> > > +	int i, remaining = len;
> > >  	unsigned char linebuf[64 * 3 + 2 + 64 + 1];
> > > +	bool first_line = true;
> > >
> > >  	if (rowsize != 16 && rowsize != 32 && rowsize != 64)
> > >  		rowsize = 16;
> > >
> > >  	for (i = 0; i < len; i += rowsize) {
> > > -		linelen = min(remaining, rowsize);
> > > +		bool skip = false;
> > > +		int linelen = min(remaining, rowsize);
> > > +
> > >  		remaining -= rowsize;
> > >
> > > +		if (flags & HEXDUMP_SUPPRESS_0X00)
> > > +			skip = buf_is_all(ptr + i, linelen, 0x00);
> > > +
> > > +		if (!skip && (flags & HEXDUMP_SUPPRESS_0XFF))
> > > +			skip = buf_is_all(ptr + i, linelen, 0xff);
> > > +
> > > +		if (first_line && !(flags & HEXDUMP_SUPPRESS_FIRST))
> > > +			skip = false;
> > > +
> > > +		if (remaining <= 0 && !(flags & HEXDUMP_SUPPRESS_LAST))
> > > +			skip = false;
> > > +
> > > +		if (skip)
> > > +			continue;
> > 
> > IMHO, quietly skipping lines could cause a lot of confusion, espcially
> when the address is not printed.
> >
> It's up to the caller to decide how they want it displayed.

I wonder who would want to quietly skip some data values.
Are you using it yourself? Could you please provide an
example?

I do not see why we would need to complicate the API and code
by this.

The behavior proposed by Tvrtko Ursulin makes much more
sense. I mean
https://lkml.kernel.org/r/929244ed-cc7f-b0f3-b5ac-50e798e83188@linux.intel.com


> > I wonder how it would look like when we print something like:
> > 
> >     --- skipped XX lines full of 0x00 ---
> 
> This could be added as a later enhancement, with a new flag (eg.
> HEXDUMP_SUPPRESS_VERBOSE).

Who will add this later? Frankly, this looks like a half baked
feature that it good enough for you. If you want it upstream,
it must behave reasonably and it must be worth it.

Best Regards,
Petr

^ permalink raw reply

* Re: gsmtap design/extensions?
From: Johannes Berg @ 2019-04-15  9:11 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Vadim Yanitskiy, Harald Welte, OpenBSC Mailing List,
	Sean Tranchetti, radiotap, Dan Williams, netdev,
	open list:NFC SUBSYSTEM, Aleksander Morgado,
	Subash Abhinov Kasiviswanathan, Bjørn Mork
In-Reply-To: <D4D12CF9-9CE5-46FB-9738-F89DB6B6F9EA@holtmann.org>

On Fri, 2019-04-12 at 21:49 +0200, Marcel Holtmann wrote:
> 
> > Yes, that's true, though the control path is actually going through one
> > of the data pipes as well.
> 
> I think that viewpoint is too simplistic. And for sure we have no such
> system where the control path is done as IP packets for Ethernet
> packets.

That's true, but as far as I can tell in a lot of cases the pipe is
actually still a virtual netdev (session, encoded in whatever way) using
some sort of serial line format on top?

But I agree that typically this will not be sufficient anyway.

> > Right. This is something we'd typically use tracing for in wifi.
> 
> Same thing, but different way of doing it. Mind you that Bluetooth
> support is older than tracing.

Sure. Looking forward though, perhaps tracing *would* indeed be the
simplest thing to do.

> > There are a few reasons why I think that this model of having a single
> > underlying netdev controlled by the modem driver, with additional
> > netdevs layered on top in software will not work right in the future. I
> > think drivers should and will need to migrate to creating "real" netdevs
> > for the sessions instead of pure software ones.
> 
> I do not follow on why is that. Why would I care if wwan0 is self-
> sustained of just a VLAN device. Or for that matter any other kind of
> slave/child device.

From a driver perspective I think it makes a difference. For example, if
you just have a single netdev from the driver, it gets harder to do
multiple TX/RX queues properly.

> 3GPP and 3GPP2 are not Ethernet frame based. We only have raw IPv4 and
> IPv6 packets for the data path.

Sure. Though some drivers/devices fake them anyway.

> And for 3GPP you have context identifiers that at least within the
> context of the control path make logical sense of the data streams and
> what they are assigned to. This goes back to my original point. You
> need to capture the control path to see what APN context is set up and
> how. Mind you that you also have the fun between primary context and
> secondary context (primarily for quality of service in VoLTE cases).

Right.

> > It may well be that doing kernel-tracing instead of doing it via some
> > kind of monitor netdev is perfectly sufficient, but I have a feeling
> > that the relative simplicity of starting tcpdump/wireshark might be
> > preferable.
> 
> As I said, as long as you do not get the QMI, AT command, MBIM etc.
> control path session recorded as well, you have nothing to really
> analyze.

But you do - afaict there are no ways other than the netdevs to
communicate with the device, and people do things like "socat" to set up
PTYs and pretend to have serial channels there, on top of the netdevs?

johannes


^ permalink raw reply

* Re: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: Petr Mladek @ 2019-04-15  9:02 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: 'Jani Nikula', 'Joonas Lahtinen',
	'Rodrigo Vivi', 'David Airlie',
	'Daniel Vetter', 'Karsten Keil',
	'Jassi Brar', 'Tom Lendacky',
	'David S. Miller', 'Jose Abreu',
	'Kalle Valo', 'Stanislaw Gruszka',
	'Benson Leung', 'Enric Balletbo i Serra',
	'James E.J. Bottomley', 'Martin K. Petersen',
	'Greg Kroah-Hartman', 'Alexander Viro',
	'Sergey Senozhatsky', 'Steven Rostedt',
	'Andrew Morton', intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <092f01d4f186$8e9e7cd0$abdb7670$@d-silva.org>

On Sat 2019-04-13 09:22:05, Alastair D'Silva wrote:
> > -----Original Message-----
> > From: Petr Mladek <pmladek@suse.com>
> > Sent: Friday, 12 April 2019 11:48 PM
> > To: Alastair D'Silva <alastair@au1.ibm.com>
> > Cc: alastair@d-silva.org; Jani Nikula <jani.nikula@linux.intel.com>;
> Joonas
> > Lahtinen <joonas.lahtinen@linux.intel.com>; Rodrigo Vivi
> > <rodrigo.vivi@intel.com>; David Airlie <airlied@linux.ie>; Daniel Vetter
> > <daniel@ffwll.ch>; Karsten Keil <isdn@linux-pingi.de>; Jassi Brar
> > <jassisinghbrar@gmail.com>; Tom Lendacky <thomas.lendacky@amd.com>;
> > David S. Miller <davem@davemloft.net>; Jose Abreu
> > <Jose.Abreu@synopsys.com>; Kalle Valo <kvalo@codeaurora.org>;
> > Stanislaw Gruszka <sgruszka@redhat.com>; Benson Leung
> > <bleung@chromium.org>; Enric Balletbo i Serra
> > <enric.balletbo@collabora.com>; James E.J. Bottomley
> > <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Alexander Viro
> > <viro@zeniv.linux.org.uk>; Sergey Senozhatsky
> > <sergey.senozhatsky@gmail.com>; Steven Rostedt <rostedt@goodmis.org>;
> > Andrew Morton <akpm@linux-foundation.org>; intel-
> > gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org;
> > ath10k@lists.infradead.org; linux-wireless@vger.kernel.org; linux-
> > scsi@vger.kernel.org; linux-fbdev@vger.kernel.org;
> > devel@driverdev.osuosl.org; linux-fsdevel@vger.kernel.org
> > Subject: Re: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
> > 
> > On Wed 2019-04-10 13:17:17, Alastair D'Silva wrote:
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > With modern high resolution screens, we can display more data, which
> > > makes life a bit easier when debugging.
> > 
> > I have quite some doubts about this feature.
> > 
> > We are talking about more than 256 characters per-line. I wonder if such a
> > long line is really easier to read for a human.
> 
> It's basically 2 separate panes of information side by side, the hexdump and
> the ASCII version.
> 
> I'm using this myself when dealing with the pmem labels, and it works quite
> nicely.

I am sure that it works for you. But I do not believe that it
would be useful in general.


> > I am not expert but there is a reason why the standard is 80 characters
> per-
> > line. I guess that anything above 100 characters is questionable.
> > https://en.wikipedia.org/wiki/Line_length
> > somehow confirms that.
> > 
> > Second, if we take 8 pixels per-character. Then we need
> > 2048 to show the 256 characters. It is more than HD.
> > IMHO, there is still huge number of people that even do not have HD
> display,
> > especially on a notebook.
> 
> The intent is to make debugging easier when dealing with large chunks of
> binary data. I don't expect end users to see this output.

How is it supposed to be used then? Only by your temporary patches?

Best Regards,
Petr

^ permalink raw reply

* Re: [RFC PATCH v3 07/12] iwlwifi: Extended Key ID support (NATIVE)
From: Johannes Berg @ 2019-04-15  8:44 UTC (permalink / raw)
  To: Alexander Wetzel; +Cc: linux-wireless
In-Reply-To: <45ae97d6-3357-64ac-0a40-9ae3ea4a8ed2@wetzel-home.de>

On Sun, 2019-04-14 at 18:12 +0200, Alexander Wetzel wrote:
> > I'll take a look, but a trace-cmd recording would be more interesting
> > than the monitor interface, as it also tells us when what key was
> > installed etc.
> 
> I've just uploaded better captures also including traces to the same 
> location. Everything relevant for this mail is in the folder
> iwlwifi-debug here:
> https://www.awhome.eu/index.php/s/AJJXBLsZmzHdxpX

Great, thanks.

> It looks like we only have a problem when we get frames with the "new" 
> keyID and then again some with the "old" keyID. Of course we also could 
> have multiple problems, too... But in that case I would say let's first 
> look at this one.

This part kinda surprises me.

> The problematic frames are encrypted with the correct "old" key, 
> according to wireshark.
> 
> But on the STA they are scrambled and therefor probably have been 
> decrypted with the - in this case - wrong new key.
> 
> And as it happens there is also a really good looking first suspect why 
> this may have happened:
> According to the STA captures the broken frames came in one A-MPDU which 
> started with keyID 1 and then "appended" the older keyID 0 frames at the 
> end. (The OTA sniffer seems to be miss the A-MPDU details, see the STA 
> capture...)

This doesn't surprise me at all.

> Now it really looks like the mvm cards are trusting the standard and 
> decode all MPDUs within one A-MPDU with the same key while at the same 
> time allow mixing different keyIDs in a-MPDU.

Yes, I'm pretty sure the firmware will only be able to look at the whole
A-MPDU and thus must make a decision based on the first subframe
regarding the key.

> The so far mostly theoretical question how far mac80211 should support 
> the driver (e.g. key ID border signal) or if we want to let the 
> drivers/card handle that would become much more pressing...

Yeah, good point. I guess this really would have to be on the
transmitter though.

> Ideally the card/firmware would be able to detect that and start a new 
> A-MPDU. 

For similar reasons, I don't think it can do this even on TX.

> But for my understanding the driver could also set A-MPDU to 
> only one frame, forcing the firmware to flush all A-MPDUs under 
> construction and then set it back to the normal value. (Or that is how 
> I've planned to test that in that way with your tips in the past and 
> what's documented of the mvm/dvm firmware API.)

It's probably getting more complicated with newer versions of the API,
but yes, I guess it should be doable to suspend aggregation.

> Thanks for the reminder.
> I'll definitely will remember the option, I was not aware that there was 
> a more private way:-)

Just wanted to point it out. I'd agree the risk is minimal since you use
a separate test network anyway.

johannes


^ permalink raw reply

* linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree
From: Stephen Rothwell @ 2019-04-15  2:08 UTC (permalink / raw)
  To: Kalle Valo, Wireless
  Cc: Linux Next Mailing List, Linux Kernel Mailing List, Ihab Zhaika,
	Luca Coelho

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

Hi all,

Today's linux-next merge of the wireless-drivers-next tree got a
conflict in:

  drivers/net/wireless/intel/iwlwifi/pcie/drv.c

between commits:

  0d5bad14226a ("iwlwifi: rename structs to fit the new names")
  972d8e137779 ("iwlwifi: add new 0x2723/0x2080 card for 22000")

from the wireless-drivers tree and commit:

  ef8a913766cd ("iwlwifi: remove misconfigured pci ids from 22260 series")

from the wireless-drivers-next tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index 9f1af8da9dc1,0329b626ada6..000000000000
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@@ -953,19 -953,15 +953,15 @@@ static const struct pci_device_id iwl_h
  	{IWL_PCI_DEVICE(0xA0F0, 0x1652, killer1650i_2ax_cfg_qu_b0_hr_b0)},
  	{IWL_PCI_DEVICE(0xA0F0, 0x4070, iwl_ax101_cfg_qu_hr)},
  
 -	{IWL_PCI_DEVICE(0x2723, 0x0080, iwl22260_2ax_cfg)},
 -	{IWL_PCI_DEVICE(0x2723, 0x0084, iwl22260_2ax_cfg)},
 -	{IWL_PCI_DEVICE(0x2723, 0x0088, iwl22260_2ax_cfg)},
 -	{IWL_PCI_DEVICE(0x2723, 0x008C, iwl22260_2ax_cfg)},
 +	{IWL_PCI_DEVICE(0x2723, 0x0080, iwl_ax200_cfg_cc)},
 +	{IWL_PCI_DEVICE(0x2723, 0x0084, iwl_ax200_cfg_cc)},
 +	{IWL_PCI_DEVICE(0x2723, 0x0088, iwl_ax200_cfg_cc)},
 +	{IWL_PCI_DEVICE(0x2723, 0x008C, iwl_ax200_cfg_cc)},
  	{IWL_PCI_DEVICE(0x2723, 0x1653, killer1650w_2ax_cfg)},
  	{IWL_PCI_DEVICE(0x2723, 0x1654, killer1650x_2ax_cfg)},
 -	{IWL_PCI_DEVICE(0x2723, 0x2080, iwl22260_2ax_cfg)},
 -	{IWL_PCI_DEVICE(0x2723, 0x4080, iwl22260_2ax_cfg)},
 -	{IWL_PCI_DEVICE(0x2723, 0x4088, iwl22260_2ax_cfg)},
 +	{IWL_PCI_DEVICE(0x2723, 0x2080, iwl_ax200_cfg_cc)},
 +	{IWL_PCI_DEVICE(0x2723, 0x4080, iwl_ax200_cfg_cc)},
 +	{IWL_PCI_DEVICE(0x2723, 0x4088, iwl_ax200_cfg_cc)},
- 
- 	{IWL_PCI_DEVICE(0x1a56, 0x1653, killer1650w_2ax_cfg)},
- 	{IWL_PCI_DEVICE(0x1a56, 0x1654, killer1650x_2ax_cfg)},
- 
  	{IWL_PCI_DEVICE(0x2725, 0x0090, iwlax210_2ax_cfg_so_hr_a0)},
  	{IWL_PCI_DEVICE(0x7A70, 0x0090, iwlax210_2ax_cfg_so_hr_a0)},
  	{IWL_PCI_DEVICE(0x7A70, 0x0310, iwlax210_2ax_cfg_so_hr_a0)},

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

^ permalink raw reply

* linux-next: manual merge of the wireless-drivers-next tree with the wireless-drivers tree
From: Stephen Rothwell @ 2019-04-15  2:01 UTC (permalink / raw)
  To: Kalle Valo, Wireless
  Cc: Linux Next Mailing List, Linux Kernel Mailing List,
	Shahar S Matityahu, Luca Coelho

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

Hi all,

Today's linux-next merge of the wireless-drivers-next tree got a conflict
in:

  drivers/net/wireless/intel/iwlwifi/iwl-trans.h

between commit:

  07d35b4270ef ("iwlwifi: use sync nmi in case of init flow failure")

from the wireless-drivers tree and commit:

  4b1831e48974 ("iwlwifi: dbg_ini: support HW error trigger")

from the wireless-drivers-next tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/net/wireless/intel/iwlwifi/iwl-trans.h
index d8690acee40c,2235978adf70..000000000000
--- a/drivers/net/wireless/intel/iwlwifi/iwl-trans.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-trans.h
@@@ -830,6 -831,8 +830,7 @@@ struct iwl_trans 
  	u32 lmac_error_event_table[2];
  	u32 umac_error_event_table;
  	unsigned int error_event_table_tlv_status;
 -	wait_queue_head_t fw_halt_waitq;
+ 	bool hw_error;
  
  	/* pointer to trans specific struct */
  	/*Ensure that this pointer will always be aligned to sizeof pointer */

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

^ permalink raw reply

* rtlwifi-new v0.6 and dkms failures
From: David R. Bergstein @ 2019-04-14 21:39 UTC (permalink / raw)
  To: Larry Finger, Tony Chuang
  Cc: Pkshih, linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <8706d34c-1fac-84ba-759d-18d96750636b@gmail.com>

Larry,

Thanks for your prior assistance with the rtw88 module.  Since manually
compiling and installing works correctly, I have been working to
register the software in dkms, but am running into some difficulties. 
Below are some snippets from a terminal session to illustrate the issue:

udo dkms install rtlwifi-new/0.6

Creating symlink /var/lib/dkms/rtlwifi-new/0.6/source ->
                 /usr/src/rtlwifi-new-0.6

DKMS: add completed.

Kernel preparation unnecessary for this kernel.  Skipping...

Building module:
cleaning build area....(bad exit status: 2)
make -j12 KERNELRELEASE=5.0.0-11-lowlatency -C
/lib/modules/5.0.0-11-lowlatency/build
M=/var/lib/dkms/rtlwifi-new/0.6/build....
ERROR (dkms apport): binary package for rtlwifi-new: 0.6 not found
Error!  Build of rtl_pci.ko failed for: 5.0.0-11-lowlatency (x86_64)
Consult the make.log in the build directory
/var/lib/dkms/rtlwifi-new/0.6/build/ for more information.

Below is the make.log contents referenced above:

DKMS make.log for rtlwifi-new-0.6 for kernel 5.0.0-11-lowlatency (x86_64)
Sun 14 Apr 2019 05:25:37 PM EDT
make: Entering directory '/usr/src/linux-headers-5.0.0-11-lowlatency'
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/main.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/mac80211.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/util.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/debug.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/tx.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rx.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/mac.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/phy.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/efuse.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/fw.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/ps.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/sec.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/regd.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822b.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822b_table.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822c.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw8822c_table.o
  CC [M]  /var/lib/dkms/rtlwifi-new/0.6/build/pci.o
  LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtwpci.o
  LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw88.o
  Building modules, stage 2.
  MODPOST 2 modules
  CC      /var/lib/dkms/rtlwifi-new/0.6/build/rtw88.mod.o
  CC      /var/lib/dkms/rtlwifi-new/0.6/build/rtwpci.mod.o
  LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtw88.ko
  LD [M]  /var/lib/dkms/rtlwifi-new/0.6/build/rtwpci.ko
make: Leaving directory '/usr/src/linux-headers-5.0.0-11-lowlatency'

As you can see, the make log does not show any errors, which is baffling
to me.  Any insight you have on resolving this issue would be appreciated.

Sincerely,

David R. Bergstein



^ permalink raw reply

* WARNING: ODEBUG bug in rsi_probe
From: syzbot @ 2019-04-14 19:56 UTC (permalink / raw)
  To: amitkarwar, andreyknvl, davem, kvalo, linux-kernel, linux-usb,
	linux-wireless, netdev, siva8118, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    9a33b369 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan/tree/usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=163713bb200000
kernel config:  https://syzkaller.appspot.com/x/.config?x=23e37f59d94ddd15
dashboard link: https://syzkaller.appspot.com/bug?extid=1d1597a5aa3679c65b9f
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11ab7cd3200000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+1d1597a5aa3679c65b9f@syzkaller.appspotmail.com

rsi_91x: rsi_load_firmware: REGOUT read failed
rsi_91x: rsi_hal_device_init: Failed to load TA instructions
rsi_91x: rsi_probe: Failed in device init
------------[ cut here ]------------
ODEBUG: free active (active state 0) object type: timer_list hint:  
bl_cmd_timeout+0x0/0x50 drivers/net/wireless/rsi/rsi_91x_hal.c:577
WARNING: CPU: 0 PID: 5 at lib/debugobjects.c:325  
debug_print_object+0x162/0x250 lib/debugobjects.c:325
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xe8/0x16e lib/dump_stack.c:113
  panic+0x29d/0x5f2 kernel/panic.c:214
  __warn.cold+0x20/0x48 kernel/panic.c:571
  report_bug+0x262/0x2a0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:179 [inline]
  fixup_bug arch/x86/kernel/traps.c:174 [inline]
  do_error_trap+0x130/0x1f0 arch/x86/kernel/traps.c:272
  do_invalid_op+0x37/0x40 arch/x86/kernel/traps.c:291
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:debug_print_object+0x162/0x250 lib/debugobjects.c:325
Code: dd e0 a1 b3 8e 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bf 00 00 00 48  
8b 14 dd e0 a1 b3 8e 48 c7 c7 60 96 b3 8e e8 8e 93 d2 fd <0f> 0b 83 05 e9  
d6 59 10 01 48 83 c4 20 5b 5d 41 5c 41 5d c3 48 89
RSP: 0018:ffff8880a846f110 EFLAGS: 00010086
RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815b1e42 RDI: ffffed101508de14
RBP: 0000000000000001 R08: ffff8880a8444980 R09: ffffed1015a03edb
R10: ffffed1015a03eda R11: ffff8880ad01f6d7 R12: ffffffff917e7780
R13: ffffffff8161ec90 R14: ffffffff96d67928 R15: ffff88809927f060
  __debug_check_no_obj_freed lib/debugobjects.c:785 [inline]
  debug_check_no_obj_freed+0x2a3/0x42e lib/debugobjects.c:817
  slab_free_hook mm/slub.c:1426 [inline]
  slab_free_freelist_hook+0xfb/0x140 mm/slub.c:1456
  slab_free mm/slub.c:3003 [inline]
  kfree+0xce/0x290 mm/slub.c:3958
  rsi_probe+0xdf3/0x140d drivers/net/wireless/rsi/rsi_91x_sdio.c:1178
  usb_probe_interface+0x31d/0x820 drivers/usb/core/driver.c:361
  really_probe+0x2da/0xb10 drivers/base/dd.c:509
  driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
  __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
  bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
  __device_attach+0x223/0x3a0 drivers/base/dd.c:844
  bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
  device_add+0xad2/0x16e0 drivers/base/core.c:2106
  usb_set_configuration+0xdf7/0x1740 drivers/usb/core/message.c:2021
  generic_probe+0xa2/0xda drivers/usb/core/generic.c:210
  usb_probe_device+0xc0/0x150 drivers/usb/core/driver.c:266
  really_probe+0x2da/0xb10 drivers/base/dd.c:509
  driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
  __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
  bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
  __device_attach+0x223/0x3a0 drivers/base/dd.c:844
  bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
  device_add+0xad2/0x16e0 drivers/base/core.c:2106
  usb_new_device.cold+0x537/0xccf drivers/usb/core/hub.c:2534
  hub_port_connect drivers/usb/core/hub.c:5089 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
  port_event drivers/usb/core/hub.c:5350 [inline]
  hub_event+0x138e/0x3b00 drivers/usb/core/hub.c:5432
  process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
  worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
  kthread+0x313/0x420 kernel/kthread.c:253
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352

======================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH 0/1] mwifiex: add support for SD8987 chipset
From: Tamás Szűcs @ 2019-04-14 18:36 UTC (permalink / raw)
  To: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, David S . Miller,
	open list:MARVELL MWIFIEX WIRELESS DRIVER,
	open list:NETWORKING DRIVERS, open list
  Cc: Cathy Luo, Zhiyuan Yang, James Cao, Rakesh Parmar,
	Hemantkumar Suthar, Tamás Szűcs

Hi All,

Changes here have been tested with u-blox JODY-W2XX and Marvell
TB-AQB8987-QFN-EPA-2A. It would be great to have driver support upstream.
Kindly review.

Kind regards,
Tamas

Tamás Szűcs (1):
  mwifiex: add support for SD8987 chipset

 drivers/net/wireless/marvell/mwifiex/Kconfig |  4 +-
 drivers/net/wireless/marvell/mwifiex/sdio.c  |  5 ++
 drivers/net/wireless/marvell/mwifiex/sdio.h  | 69 ++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)

-- 
2.11.0


^ permalink raw reply

* [PATCH 1/1] mwifiex: add support for SD8987 chipset
From: Tamás Szűcs @ 2019-04-14 18:36 UTC (permalink / raw)
  To: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, David S . Miller,
	open list:MARVELL MWIFIEX WIRELESS DRIVER,
	open list:NETWORKING DRIVERS, open list
  Cc: Cathy Luo, Zhiyuan Yang, James Cao, Rakesh Parmar,
	Hemantkumar Suthar, Tamás Szűcs
In-Reply-To: <20190414183641.7539-1-tszucs@protonmail.ch>

This patch adds support for Marvell 88W8987 chipset with SDIO interface.
Register offsets and supported feature flags are updated. The corresponding
firmware image file shall be "mrvl/sd8987_uapsta.bin".

Signed-off-by: Tamás Szűcs <tszucs@protonmail.ch>
---
 drivers/net/wireless/marvell/mwifiex/Kconfig |  4 +-
 drivers/net/wireless/marvell/mwifiex/sdio.c  |  5 ++
 drivers/net/wireless/marvell/mwifiex/sdio.h  | 69 ++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/Kconfig b/drivers/net/wireless/marvell/mwifiex/Kconfig
index 524fd565cb2a..572d187a99f4 100644
--- a/drivers/net/wireless/marvell/mwifiex/Kconfig
+++ b/drivers/net/wireless/marvell/mwifiex/Kconfig
@@ -9,13 +9,13 @@ config MWIFIEX
 	  mwifiex.
 
 config MWIFIEX_SDIO
-	tristate "Marvell WiFi-Ex Driver for SD8786/SD8787/SD8797/SD8887/SD8897/SD8977/SD8997"
+	tristate "Marvell WiFi-Ex Driver for SD8786/SD8787/SD8797/SD8887/SD8897/SD8977/SD8987/SD8997"
 	depends on MWIFIEX && MMC
 	select FW_LOADER
 	select WANT_DEV_COREDUMP
 	---help---
 	  This adds support for wireless adapters based on Marvell
-	  8786/8787/8797/8887/8897/8997 chipsets with SDIO interface.
+	  8786/8787/8797/8887/8897/8977/8987/8997 chipsets with SDIO interface.
 
 	  If you choose to build it as a module, it will be called
 	  mwifiex_sdio.
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index a85648342d15..32bf2baeba9a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -491,6 +491,8 @@ static void mwifiex_sdio_coredump(struct device *dev)
 #define SDIO_DEVICE_ID_MARVELL_8801   (0x9139)
 /* Device ID for SD8977 */
 #define SDIO_DEVICE_ID_MARVELL_8977   (0x9145)
+/* Device ID for SD8987 */
+#define SDIO_DEVICE_ID_MARVELL_8987   (0x9149)
 /* Device ID for SD8997 */
 #define SDIO_DEVICE_ID_MARVELL_8997   (0x9141)
 
@@ -511,6 +513,8 @@ static const struct sdio_device_id mwifiex_ids[] = {
 		.driver_data = (unsigned long)&mwifiex_sdio_sd8801},
 	{SDIO_DEVICE(SDIO_VENDOR_ID_MARVELL, SDIO_DEVICE_ID_MARVELL_8977),
 		.driver_data = (unsigned long)&mwifiex_sdio_sd8977},
+	{SDIO_DEVICE(SDIO_VENDOR_ID_MARVELL, SDIO_DEVICE_ID_MARVELL_8987),
+		.driver_data = (unsigned long)&mwifiex_sdio_sd8987},
 	{SDIO_DEVICE(SDIO_VENDOR_ID_MARVELL, SDIO_DEVICE_ID_MARVELL_8997),
 		.driver_data = (unsigned long)&mwifiex_sdio_sd8997},
 	{},
@@ -2731,4 +2735,5 @@ MODULE_FIRMWARE(SD8797_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(SD8897_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(SD8887_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(SD8977_DEFAULT_FW_NAME);
+MODULE_FIRMWARE(SD8987_DEFAULT_FW_NAME);
 MODULE_FIRMWARE(SD8997_DEFAULT_FW_NAME);
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h b/drivers/net/wireless/marvell/mwifiex/sdio.h
index 912de2cde8d9..f672bdf52cc1 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -37,6 +37,7 @@
 #define SD8887_DEFAULT_FW_NAME "mrvl/sd8887_uapsta.bin"
 #define SD8801_DEFAULT_FW_NAME "mrvl/sd8801_uapsta.bin"
 #define SD8977_DEFAULT_FW_NAME "mrvl/sd8977_uapsta.bin"
+#define SD8987_DEFAULT_FW_NAME "mrvl/sd8987_uapsta.bin"
 #define SD8997_DEFAULT_FW_NAME "mrvl/sd8997_uapsta.bin"
 
 #define BLOCK_MODE	1
@@ -526,6 +527,58 @@ static const struct mwifiex_sdio_card_reg mwifiex_reg_sd8887 = {
 				 0x68, 0x69, 0x6a},
 };
 
+static const struct mwifiex_sdio_card_reg mwifiex_reg_sd8987 = {
+	.start_rd_port = 0,
+	.start_wr_port = 0,
+	.base_0_reg = 0xF8,
+	.base_1_reg = 0xF9,
+	.poll_reg = 0x5C,
+	.host_int_enable = UP_LD_HOST_INT_MASK | DN_LD_HOST_INT_MASK |
+			CMD_PORT_UPLD_INT_MASK | CMD_PORT_DNLD_INT_MASK,
+	.host_int_rsr_reg = 0x4,
+	.host_int_status_reg = 0x0C,
+	.host_int_mask_reg = 0x08,
+	.status_reg_0 = 0xE8,
+	.status_reg_1 = 0xE9,
+	.sdio_int_mask = 0xff,
+	.data_port_mask = 0xffffffff,
+	.io_port_0_reg = 0xE4,
+	.io_port_1_reg = 0xE5,
+	.io_port_2_reg = 0xE6,
+	.max_mp_regs = 196,
+	.rd_bitmap_l = 0x10,
+	.rd_bitmap_u = 0x11,
+	.rd_bitmap_1l = 0x12,
+	.rd_bitmap_1u = 0x13,
+	.wr_bitmap_l = 0x14,
+	.wr_bitmap_u = 0x15,
+	.wr_bitmap_1l = 0x16,
+	.wr_bitmap_1u = 0x17,
+	.rd_len_p0_l = 0x18,
+	.rd_len_p0_u = 0x19,
+	.card_misc_cfg_reg = 0xd8,
+	.card_cfg_2_1_reg = 0xd9,
+	.cmd_rd_len_0 = 0xc0,
+	.cmd_rd_len_1 = 0xc1,
+	.cmd_rd_len_2 = 0xc2,
+	.cmd_rd_len_3 = 0xc3,
+	.cmd_cfg_0 = 0xc4,
+	.cmd_cfg_1 = 0xc5,
+	.cmd_cfg_2 = 0xc6,
+	.cmd_cfg_3 = 0xc7,
+	.fw_dump_host_ready = 0xcc,
+	.fw_dump_ctrl = 0xf9,
+	.fw_dump_start = 0xf1,
+	.fw_dump_end = 0xf8,
+	.func1_dump_reg_start = 0x10,
+	.func1_dump_reg_end = 0x17,
+	.func1_scratch_reg = 0xE8,
+	.func1_spec_reg_num = 13,
+	.func1_spec_reg_table = {0x08, 0x58, 0x5C, 0x5D, 0x60,
+				 0x61, 0x62, 0x64, 0x65, 0x66,
+				 0x68, 0x69, 0x6a},
+};
+
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8786 = {
 	.firmware = SD8786_DEFAULT_FW_NAME,
 	.reg = &mwifiex_reg_sd87xx,
@@ -633,6 +686,22 @@ static const struct mwifiex_sdio_device mwifiex_sdio_sd8887 = {
 	.can_ext_scan = true,
 };
 
+static const struct mwifiex_sdio_device mwifiex_sdio_sd8987 = {
+	.firmware = SD8987_DEFAULT_FW_NAME,
+	.reg = &mwifiex_reg_sd8987,
+	.max_ports = 32,
+	.mp_agg_pkt_limit = 16,
+	.tx_buf_size = MWIFIEX_TX_DATA_BUF_SIZE_2K,
+	.mp_tx_agg_buf_size = MWIFIEX_MP_AGGR_BUF_SIZE_MAX,
+	.mp_rx_agg_buf_size = MWIFIEX_MP_AGGR_BUF_SIZE_MAX,
+	.supports_sdio_new_mode = true,
+	.has_control_mask = false,
+	.can_dump_fw = true,
+	.fw_dump_enh = true,
+	.can_auto_tdls = true,
+	.can_ext_scan = true,
+};
+
 static const struct mwifiex_sdio_device mwifiex_sdio_sd8801 = {
 	.firmware = SD8801_DEFAULT_FW_NAME,
 	.reg = &mwifiex_reg_sd87xx,
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 0/6] ath10k: SDIO and high latency patches from Silex
From: Erik Stromdahl @ 2019-04-14 16:53 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, ath10k
In-Reply-To: <87pnprflj4.fsf@kamboji.qca.qualcomm.com>



On 4/12/19 2:36 PM, Kalle Valo wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> 
>> This series adds a few more fixes for SDIO and high latency devices.
>>
>> I have had these on my private tree for quite some time now so they
>> should be considered fairly well tested.
>>
>> 4 out of 6 patches are from Alagu Sankar at Silex.
>> I have made some adjustments to some of them in order to make them
>> smaller and easier to review.
>>
>> Alagu Sankar (4):
>>    ath10k: use clean packet headers
>>    ath10k: high latency fixes for beacon buffer
>>    ath10k: sdio: read RX packets in bundles
>>    ath10k: sdio: add MSDU ID allocation in HTT TX path
>>
>> Erik Stromdahl (2):
>>    ath10k: sdio: add missing error check
>>    ath10k: sdio: replace skb_trim with explicit set of skb->len
> 
> Bad timing as also me and Wen have been cleaning up these patches and
> finalising the SDIO support so our work overlapped. I'll send our
> version of patches soon and we can then compare.
> 
Ok, I will rework these patches and apply them on top of yours and Wen's.
I'll send a v2 set later.

^ 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