* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Arnd Bergmann @ 2019-04-13 8:04 UTC (permalink / raw)
To: Kirtika Ruchandani; +Cc: Ben Greear, linux-wireless, Tina Ruchandani
In-Reply-To: <CAK8P3a2DVgneKY9XMQe_LFuECNR7qvLtJV-g-skC4EUDA40+-g@mail.gmail.com>
On Sat, Apr 13, 2019 at 10:00 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Apr 13, 2019 at 12:07 AM Kirtika Ruchandani <kirtika@google.com> wrote:
> >
> > On Fri, Apr 12, 2019 at 2:49 PM Ben Greear <greearb@candelatech.com> wrote:
> > >
> > > On 4/12/19 2:43 PM, Kirtika Ruchandani wrote:
> > > > On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
> > > >>
> > > >> From: Ben Greear <greearb@candelatech.com>
> > > >>
> > > >> This lets us more precisely calculate the absolute timestamp
> > > >> of last-rix (ie, now - idle).
> > > >
> > > > Can you use 64-bit timestamps? struct timeval suffers from the
> > > > overflow after 2038 problem.
> > >
> > > What is the preferred API to do this? Whatever it is, it would need
> > > to compile on old crufty systems as well.
> >
> > I am not sure what the guidance for userspace is. The kernel uses
> > 'struct timespec64' I think.
> > Arnd (cc-ed) who has mostly led the 2038 problem in the kernel might
> > have more input on the
> > "old crufty systems" part.
>
> I'm not sure what you are trying to do, and there are different
> answers depending on the usecase.
>
> For getting the time in the kernel, see Documentation/core-api/timekeeping.rst
> do_gettimeofday() is going away for many reasons, so don't use that.
> It sounds like you want "ktime_to_ms(ktime_get())" here.
>
> In userspace interfaces, you should pass 64-bit nanoseconds as returned
> by ktime_get_ns().
>
> If you want to pretty-print the current wall-clock, use the %pt format string
> on a 'struct rtc_time'.
Ah, I see now this was just userspace code. In that case, using gettimeofday()
works fine, it will end up using a 64-bit version of 'timeval', and converting
that to 64-bit milliseconds is safe.
Using clock_gettime() is generally preferred over gettimeofday() since it
avoids the conversion from nanoseconds to microseconds (which you then
convert to milliseconds).
Arnd
^ permalink raw reply
* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Arnd Bergmann @ 2019-04-13 8:00 UTC (permalink / raw)
To: Kirtika Ruchandani; +Cc: Ben Greear, linux-wireless, Tina Ruchandani
In-Reply-To: <CAOXD1icnusnOZ0b0n6eTD6PRoEcf_tHVgivmCEPM1KsWu1uEng@mail.gmail.com>
On Sat, Apr 13, 2019 at 12:07 AM Kirtika Ruchandani <kirtika@google.com> wrote:
>
> On Fri, Apr 12, 2019 at 2:49 PM Ben Greear <greearb@candelatech.com> wrote:
> >
> > On 4/12/19 2:43 PM, Kirtika Ruchandani wrote:
> > > On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
> > >>
> > >> From: Ben Greear <greearb@candelatech.com>
> > >>
> > >> This lets us more precisely calculate the absolute timestamp
> > >> of last-rix (ie, now - idle).
> > >
> > > Can you use 64-bit timestamps? struct timeval suffers from the
> > > overflow after 2038 problem.
> >
> > What is the preferred API to do this? Whatever it is, it would need
> > to compile on old crufty systems as well.
>
> I am not sure what the guidance for userspace is. The kernel uses
> 'struct timespec64' I think.
> Arnd (cc-ed) who has mostly led the 2038 problem in the kernel might
> have more input on the
> "old crufty systems" part.
I'm not sure what you are trying to do, and there are different
answers depending on the usecase.
For getting the time in the kernel, see Documentation/core-api/timekeeping.rst
do_gettimeofday() is going away for many reasons, so don't use that.
It sounds like you want "ktime_to_ms(ktime_get())" here.
In userspace interfaces, you should pass 64-bit nanoseconds as returned
by ktime_get_ns().
If you want to pretty-print the current wall-clock, use the %pt format string
on a 'struct rtc_time'.
Arnd
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-13 7:55 UTC (permalink / raw)
To: Harald Welte
Cc: Johannes Berg, openbsc, radiotap, linux-wireless,
Subash Abhinov Kasiviswanathan, Dan Williams, Bjørn Mork,
netdev, Sean Tranchetti, Aleksander Morgado
In-Reply-To: <20190413073505.GD24451@nataraja>
On Apr 13, 2019, at 12:35 AM, Harald Welte <laforge@gnumonks.org> wrote:
> 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.
The granularity of Ethernet frames is not user IP frames, but Ethernet datagrams, so you user IP frame might not be visible as it's fragmented (the fragments might still be IP datagrams, but they would have to be reassembled - which Wireshark, for example, does, for those people still doing NFS-over-UDP :-)).
"Encrypted" may not apply there, but it *does* apply for 802.11 frames on a protected network (which Wireshark can decrypt, if you have 1) the network password for WEP or WPA-Personal and 2) the EAPOL handshake for WPA-Personal).
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-13 7:49 UTC (permalink / raw)
To: Harald Welte
Cc: Johannes Berg, Vadim Yanitskiy, OpenBSC Mailing List,
Sean Tranchetti, radiotap, Dan Williams, netdev, linux-wireless,
Aleksander Morgado, Subash Abhinov Kasiviswanathan,
Bjørn Mork
In-Reply-To: <20190413071227.GC24451@nataraja>
On Apr 13, 2019, at 12:12 AM, Harald Welte <laforge@gnumonks.org> wrote:
> On Fri, Apr 12, 2019 at 07:15:56PM +0200, Johannes Berg wrote:
>
>> Agree. Sorry about that. No disrespect was intended, but I'm still not
>> sure I understand the need for UDP encapsulation *as part of the
>> protocol*. I guess saying "GSMTAP can optionally be encapsulated in UDP
>> with the well-known port xyz" would be something else, and it'd make
>> more sense to me than saying it has to be.
>
> Sure, like with most protocols you can wrap them in anything you want.
>
> Let me put it like this:
> You don't have to run RTP inside UDP, you could equally put the RTP
> frames in to SCTP or DCTP. It's just not what the original users of
> the protocol/spec had envisioned, but it can for sure be done, and has
> no side-effect other than not being interoperable with existing
> implementations.
Or you can just have LINKTYPE_RTP/DLT_RTP and supply them inside nothing.
However, unlike RTP, there is no reason *not* to do that for GSMTAP - it's not as if the IP or UDP headers in a packet from a host supplying GSMTAP-encapsulated packets provide any information necessary or even useful for dissecting the encapsulated packets.
Whether it's useful, or possible, to have any interfaces on a *host* with cellular modem connectivity supply the cellular-network traffic as packets with GSMTAP headers - which appears to be what Johannes is thinking of - is another matter (but even if the answer is no, there is, as per my other message, a use for a LINKTYPE_GSMTAP/DLT_GSMTAP header type). That might not be possible, as cellular modems, as you note, tend to hide a lot of lower-layer details from the host.
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Harald Welte @ 2019-04-13 7:35 UTC (permalink / raw)
To: Johannes Berg
Cc: openbsc, radiotap, linux-wireless, Subash Abhinov Kasiviswanathan,
Dan Williams, Bjørn Mork, netdev, Sean Tranchetti,
Aleksander Morgado
In-Reply-To: <1462659018bc40830efbe2348791b8df45b54cff.camel@sipsolutions.net>
Hi Johannes,
I'll only adress some of the points that haven't been clarified in other
mails in this thread alreasdy:
On Fri, Apr 12, 2019 at 02:24:25PM +0200, Johannes Berg wrote:
> Kernel drivers need to transport multiple data streams, like VoLTE (IP)
> data, IP data, and control data. Currently, they do this in various ways
> (cdc_mbim uses VLAN tags, rmnet/qmi_wwan uses Qualcomm QMI headers),
> when you capture data you have to understand what you're dealing with
> and maybe extend wireshark to even understand these headers.
>
> I'm looking for a general vendor-agnostic encapsulation that would solve
> this part of the problem.
understood.
> At the same time, I come from WiFi, where - while we don't have these
> sessions as such but can have different virtual interfaces as well, e.g.
> for connections to different APs - it's often very useful to also
> understand the PHY parameters that were used for TX/seen on RX. Now, I
> realize this isn't something that modems will do right now - they just
> don't seem to tell you any lower-level information on RX of a packet,
> but it seemed like it might be worthwhile to not *exclude* this
> possibility either.
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.
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.
> > GSMTAP was designed as a format to encapsulate protocols normally not spoken over IP
> > (such as classic GSM radio protocols, e.g. the Layer 2 LAPDm over GSM Um)
> > inside an IP transport.
>
> Sure, but wifi is also not spoken over IP, yet we don't encapsulate our
> 802.11 frames into IP to show them in wireshark :-)
This is because your 802.11 implementations are in hardware, while the
open source GSM/UMTS/LTE implementations using GSMTAP are implemented in
userspace - or at the very least the higher-level parts of the protocol
stacks, or the tracing frameworks that are used to generate the GSMTAP
data from some hardware.
> 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.
Also, for debugging cellular network elements, it's very useful if you
can look at the RAN or core network interfaces (Abis, Gb, Gp, A, Iu, S1,
...) in the same protocol trace in which you also get traces from the
radio interface in GSMTAP. Then you can see what happens at each and
every interface / network element in one time-line.
And as the Osmcoom programs also allow generating log output wrapped in
GSMTAP, you get not only protocol traces of all the interfaces, but even
textual log information (with machine-readable log level + sub-system)
information all in one capture/timeline.
> But I guess if your primary use case is actually to transport them over
> the network, this makes more sense.
The use cases differ, but having UDP encapsulation enables a lot of
flexibility and has been proven very useful so far.
> Our primary use case with wifi is much more local - capture what's going
> on - but do the capturing on the local system and write it to a file
> there.
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.
> > We also have a virtual physical layer between OsmocomBB and OsmoBTS called
> > virt_phy where the GSMTAP over multicast IP is used to simulate/virtualize
> > the entire Layer1 / PHY / radio interface between phone and base station.
> >
> > Once again, all related network elements are implemented in userspace,
> > and having an
>
> ...?
sorry. '... UDP based transport is ahat enables theis use case'
> 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.
> That doesn't seem to be the case for GSM, I guess? Does the IP frame get
> encapsulated in some kind of 3GPP message first, and then transported
> over the physical link, and the physical link info isn't even there in
> GSMTAP?
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.
> > > Or would it make more sense to define a new ARPHDR_WWANTAP like
> > > ARPHDR_IEEE80211_RADIOTAP and just use that instead of encapsulating in
> > > IP/UDP, and then have a completely new (extensible) protocol inside of
> > > that?
> >
> > No userspace source would ever be able to generate such data and stream
> > it real-time into wireshark, would it? Sure, I can write pcap file with
> > such ARPHDR_* values, but I could never do this in real-time. For many
> > but not all use cases, that's really what it is: A vehicle to stream
> > real-time non-IP protocol traces into wireshark so it can visualize
> > the protocol traces.
>
> I think you can pipe a stream into wireshark?
>
> To me it feels like the wrong thing to actually make wireshark listen on
> "lo" or "eth0" or something to get data from the cellular that's
> (locally) generated by another application,
I admit it's a bit odd. But has been very useful, particularly in more
distributed setups with creating a shared timeline of various GSMTAP
sources that may or may not run on the same machine.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-13 7:36 UTC (permalink / raw)
To: Harald Welte
Cc: Johannes Berg, Vadim Yanitskiy, OpenBSC Mailing List,
Sean Tranchetti, radiotap, Dan Williams, netdev, linux-wireless,
Aleksander Morgado, Subash Abhinov Kasiviswanathan,
Bjørn Mork
In-Reply-To: <20190413064152.GB24451@nataraja>
On Apr 12, 2019, at 11:41 PM, Harald Welte <laforge@gnumonks.org> wrote:
> But that of course doesn't prevent new users from using different
> transport mechanisms of getting GSMTAP from e.g. the kernel into
> userspace.
Nor does it prevent libpcap (once I generalize the remote-capture mechanism) from supporting gsmtap://{host}[:port] (port optional) URLs that cause it to set up a UDP ({PF_INET or PF_INET6}, SOCK_DGRAM, IPPROTO_UDP) socket on which to receive UDP packets from the specified host (at the specified port or, if no port is specified, the default GSMTAP socket) and supplying packets that begin with a GSMTAP header, with a new linktype of LINKTYPE_GSMTAP/DLT_GSMTAP.
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Harald Welte @ 2019-04-13 7:12 UTC (permalink / raw)
To: Johannes Berg
Cc: Vadim Yanitskiy, OpenBSC Mailing List, Sean Tranchetti, radiotap,
Dan Williams, netdev, linux-wireless, Aleksander Morgado,
Subash Abhinov Kasiviswanathan, Bjørn Mork
In-Reply-To: <f0a64bb751b8c601f58130a7f3a3158d70f18f63.camel@sipsolutions.net>
Hi Johannes,
On Fri, Apr 12, 2019 at 07:15:56PM +0200, Johannes Berg wrote:
> Agree. Sorry about that. No disrespect was intended, but I'm still not
> sure I understand the need for UDP encapsulation *as part of the
> protocol*. I guess saying "GSMTAP can optionally be encapsulated in UDP
> with the well-known port xyz" would be something else, and it'd make
> more sense to me than saying it has to be.
Sure, like with most protocols you can wrap them in anything you want.
Let me put it like this:
You don't have to run RTP inside UDP, you could equally put the RTP
frames in to SCTP or DCTP. It's just not what the original users of
the protocol/spec had envisioned, but it can for sure be done, and has
no side-effect other than not being interoperable with existing
implementations.
> Right, see my other mail(s) from today as well. Basically the IP frame
> that we're actually sending, and then attaching to that the "session" or
> "mux id" that it's being sent on. Sorry, I probably also don't know the
> right term - those show up in the drivers now.
So it's basically the information whch PDP/PDN context your user IP packet
belongs to. A single integer "tag". that resembles the identifier that
was used by the control plane when activating that context (which is
basically a tunnel).
> > Sure, that works. But the real question is, to me: Are there common
> > GSMTAP payload types that both the existing GSMTAP users carry, as well
> > as what you would want to carry? If yes, then it makes sense to think
> > about a common encapsulation like GSMTAP. If the payload types differ,
> > then it seems rather like there are two distinct use cases that
> > wouldn't benefit from standardizing on one format.
>
> Agree, and I don't really know.
From what I can tell after the burst of e-mails in this thread so far,
I don't think there's much commonality here. A modem will "never" give
you access to the actual cellular protocol layers that we care about
in the Osmocom/srsLTE/YateBTS/OpenBTS/airprobe/gr-gsm/... world, for
which GSMTAP was designed (see more below).
I'm not saying it cannot be done. If you want to use GSMTAP, I'm happy
to help. But at least so far, I don't see why it would make any sense.
> Maybe I should start differently. Do you have an example GSMTAP capture
> file that I could look at in wireshark? Yes, I see you've pointed out
> how I can get all the software running, but if you have a file already
> that's almost certainly faster :-)
There are a couple of files attached at
https://osmocom.org/projects/baseband/wiki/WiresharkIntegration
> And then the question I'd want answer is this: If there's an IP frame
> that I send to the modem from the application using a regular UDP or TCP
> socket, what would the corresponding GSMTAP capture look like? Surely it
> includes the IP frame in some way?!
GSMTAP was generated initially for GSM. GSM is a circuit-switched digital
wireless telephony system that has no inherent/native way of transporting
IP payload. As such, the GSMTAP Um captures linked above will not contain
IP user data but will contain the signaling plane data of the Um air interface
protocol stack, consisting of the LAPDm potocol on Layer2 and the TS 04.08
RR (Radio Resource), MM (Mobility Management) and CC (Call Control) data.
Of course one can also use GSMTAP for GPRS, which is packet oriented. In
this case, you have the following layering stack inside a GSMTAP Um frame:
TCP
IP
SNDCP (TS 04.65)
LLC (TS 04.64)
RLC/MAC
PHY
There is segmentation/reassembly potentially at least at the LLC and at
the RLC/MAC layer. There can be encryption at the LLC layer so the IP
payload is already invisible at that point. You cab also have both
header and payload compression at the SNDCP layer.
In the end, you can have start/middle/end segments of LLC frames inside
a single RLC/MAC block, belonging to either one or multiple LLC frames,
and then the LLC frames contain [segments of] IP packets.
GSMTAP can be used on various other interfaces of the cellular network,
which are *not* the radio interface between modem/phone and base
station (such as e.g. between the Phone and the SIM card), so they're not
of interest to your use case and I'll not cover them here.
GSMTAP can also be used to encapsulate later cellular technologies such
as UMTS aka WCDMA aka 3G, or LTE aka 4G. In all those cases you always
have a different (sometimes deeper and sometimes not so deep) protocol
stack between the user IP payload and the actual radio interface (PHY).
You can think of it a bit if you are not thinking about a User IP packet
but you assume for the sake of an argument that you want to see "HTML".
Then underneath HTML you either have, depending on the technoolgy, HTTP
1.0, 1.1 or 2.0. And under that you can have any number of additional
protocol stacking whether it's TCP based, with or without SSL or TLS,
with IPv3 or IPv6, with or without IPsec, inside a VLAN or not, ... -
and most importantly, next to all of that you have important "control
information" like let's say DHCP or neighbor discovery.
So, in other words, the user IP plan is *very* far away from the PHY on
the radio interface, and the really interesting things are those that
are happening beneath or next to the user IP.
However, commercially available cellular modems will go to the utmost
extent to make sure you never have access to any of this, and they
invent various different ways to make the user (whether that's ofono,
the ModemManager, ...) as far away from that as possible. The
interfaces between the cellular protocol stacks and the user (whether
QMI, MBIM, AT commands, ...) are so abstract that virtually any useful
resemblance to what happens on the actual radio interface is lost.
You can get a glimpse to some of what's happening with Qualcomm DIAG
protocol, but even that doesn't really [always] expose full protocol
traces to you, particularly not on the user plane.
> If the answer to that question is yes, then I think there is some
> overlap, because you can always imagine the modem receiving an IP frame
> and telling you more about how it was encapsulated over the air, no?
Not really. The modem implements the entire protocol stacks for the
various cellular technologies and in the end provides you as the user
something like a tunnel endpoint.
> Mind, most if not all modems probably don't actually do that today, but
> I wonder how much of that is because of lack of infrastructure to do it,
> vs. it just not being necessary - since I've been told that the modems
> do in fact often output tracing that contains information about this.
> Just not directly combined with the IP frame.
The most widespread technology to obtain tracing/debugging information
is the Qualcomm DIAG protocol, which is a collection of dozens of
sub-systems each of whcih has up to hundreds of individual
flags/settings of categories that can be enabled and/or disabled.
We once did a bit of development in that area, see osmo-qcdiag to enable
this (http://git.osmocom.org/osmo-qcdiag/) as well as *extremely
minimal* wireshark dissectors at http://git.osmocom.org/wireshark/log/?h=laforge/qcdiag
However, this is a proprietary mechanism available only to Qualcomm stack
based devices, will only work on those devices where one has figured out
how to enable an access it. And the information it provides is
completely asynchronous to the user IP plane.
There is a similar project for (now very old) Infineon XGold based
phones, see https://github.com/2b-as/xgoldmon. It also generates
GSMTAP. But AFAIR only for the control plane data and not the user
plane data.
To be honest, I don't even think there's a lot of context that one could
theoretically provide "attached" to the user IP packet, as the
interesting bits are happening at lower protocol layers, and due to
segmentation/reassembly/compression/... etc. there is no 1:1 mapping
between the user IP packet and what's happening on the radio interface.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Harald Welte @ 2019-04-13 6:41 UTC (permalink / raw)
To: Guy Harris
Cc: Johannes Berg, Vadim Yanitskiy, OpenBSC Mailing List,
Sean Tranchetti, radiotap, Dan Williams, netdev, linux-wireless,
Aleksander Morgado, Subash Abhinov Kasiviswanathan,
Bjørn Mork
In-Reply-To: <1D6FE33B-DC15-47D8-A1AD-E95608300FDC@alum.mit.edu>
Dear Guy and others,
On Fri, Apr 12, 2019 at 03:47:26PM -0700, Guy Harris wrote:
> On Apr 12, 2019, at 12:54 PM, Guy Harris <guy@alum.mit.edu> wrote:
>
> > I see nothing about a struct gsmtap_hdr:
> > http://osmocom.org/projects/baseband/wiki/GSMTAP
> > that...
> > 2) would prevent it from being at the beginning of a LINKTYPE_GSMTAP/DLT_GSMTAP packet in a pcap or pcapng file.
> With a specification based on
> http://cgit.osmocom.org/libosmocore/plain/include/osmocom/core/gsmtap.h
I completely agree that there is no technical reason why one couldn't put GSMTAP
headers in current (v2) or future formats inside an ETHERTYPE / DLT and have
them natively appearing in a pcap file of the given DLT. There would be no objections
from my side to do so.
One could then simply call the same dissector in wireshark from that DLT or from
the existing UDP port number based dispatch via the IANA-registered GSMTAP port
number.
However, among the existing users of GSMTAP in the last decade or so,
there would be no advantage, as the related sources of GSMTAP frames all
exist in userspace and are feeding data, particularly from potentially
multiple sources, which can very well run on different hosts.
But that of course doesn't prevent new users from using different
transport mechanisms of getting GSMTAP from e.g. the kernel into
userspace.
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ 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-12 23:31 UTC (permalink / raw)
To: 'Petr Mladek', '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: <20190412141211.lvnz6wb7mwkykmxf@pathway.suse.cz>
> -----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?
> > drm_printf(m, "[%04zx] %s\n", pos, line);
> >
> > prev = buf + pos;
> > diff --git a/include/linux/printk.h b/include/linux/printk.h index
> > c014e5573665..82975853c400 100644
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -493,7 +493,7 @@ enum {
> >
> > extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> > int groupsize, char *linebuf, size_t
linebuflen,
> > - bool ascii);
> > + u64 flags);
>
> I wonder how fancy hex_dump could be. IMHO, u32 should be enough.
> The last famous words ;-)
>
> Best Regards,
> Petr
>
>
> ---
> This email has been checked for viruses by AVG.
> https://www.avg.com
^ permalink raw reply
* RE: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
From: Alastair D'Silva @ 2019-04-12 23:28 UTC (permalink / raw)
To: 'Petr Mladek', '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: <20190412140353.mgvksn3yk6n65hbk@pathway.suse.cz>
> -----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 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).
>
> Then we might even remove the SUPPRESS_FIRST, SUPPRESS_LAST and the
> ambiguous QUIET flags.
>
> > +
> > + first_line = false;
>
> This should be above the if (skip).
>
> > +
> > hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> > - linebuf, sizeof(linebuf), ascii);
> > + linebuf, sizeof(linebuf),
> > + flags & HEXDUMP_ASCII);
> >
> > switch (prefix_type) {
> > case DUMP_PREFIX_ADDRESS:
> > @@ -272,7 +316,7 @@ void print_hex_dump(const char *level, const char
> *prefix_str, int prefix_type,
> > }
> > }
> > }
> > -EXPORT_SYMBOL(print_hex_dump);
> > +EXPORT_SYMBOL(print_hex_dump_ext);
>
> We should still export even the original function that is still used
everywhere.
It's replaced with an inline wrapper function, there's no need to export 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: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: Alastair D'Silva @ 2019-04-12 23:22 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: <20190412134802.kprel2c2iqijd4ai@pathway.suse.cz>
> -----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 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.
--
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: Guy Harris @ 2019-04-12 22:47 UTC (permalink / raw)
To: Johannes Berg
Cc: Harald Welte, Vadim Yanitskiy, OpenBSC Mailing List,
Sean Tranchetti, radiotap, Dan Williams, netdev, linux-wireless,
Aleksander Morgado, Subash Abhinov Kasiviswanathan,
Bjørn Mork
In-Reply-To: <196E881D-AB4A-4569-9C1F-5BCB41FE832F@alum.mit.edu>
On Apr 12, 2019, at 12:54 PM, Guy Harris <guy@alum.mit.edu> wrote:
> I see nothing about a struct gsmtap_hdr:
>
> http://osmocom.org/projects/baseband/wiki/GSMTAP
>
> that...
>
> 2) would prevent it from being at the beginning of a LINKTYPE_GSMTAP/DLT_GSMTAP packet in a pcap or pcapng file.
With a specification based on
http://cgit.osmocom.org/libosmocore/plain/include/osmocom/core/gsmtap.h
^ permalink raw reply
* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Kirtika Ruchandani @ 2019-04-12 22:07 UTC (permalink / raw)
To: Ben Greear; +Cc: linux-wireless, Arnd Bergmann, Tina Ruchandani
In-Reply-To: <ab7f61a1-0af6-a754-d9a0-388ebd0c8f15@candelatech.com>
On Fri, Apr 12, 2019 at 2:49 PM Ben Greear <greearb@candelatech.com> wrote:
>
> On 4/12/19 2:43 PM, Kirtika Ruchandani wrote:
> > On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
> >>
> >> From: Ben Greear <greearb@candelatech.com>
> >>
> >> This lets us more precisely calculate the absolute timestamp
> >> of last-rix (ie, now - idle).
> >
> > Can you use 64-bit timestamps? struct timeval suffers from the
> > overflow after 2038 problem.
>
> What is the preferred API to do this? Whatever it is, it would need
> to compile on old crufty systems as well.
I am not sure what the guidance for userspace is. The kernel uses
'struct timespec64' I think.
Arnd (cc-ed) who has mostly led the 2038 problem in the kernel might
have more input on the
"old crufty systems" part.
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
>
^ permalink raw reply
* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Ben Greear @ 2019-04-12 21:49 UTC (permalink / raw)
To: Kirtika Ruchandani; +Cc: linux-wireless
In-Reply-To: <CAOXD1ifE_rxU0fV6tXK2WpdOU2x9-b0J4cGsuqf3whi7YapBzA@mail.gmail.com>
On 4/12/19 2:43 PM, Kirtika Ruchandani wrote:
> On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
>>
>> From: Ben Greear <greearb@candelatech.com>
>>
>> This lets us more precisely calculate the absolute timestamp
>> of last-rix (ie, now - idle).
>
> Can you use 64-bit timestamps? struct timeval suffers from the
> overflow after 2038 problem.
What is the preferred API to do this? Whatever it is, it would need
to compile on old crufty systems as well.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Kirtika Ruchandani @ 2019-04-12 21:43 UTC (permalink / raw)
To: greearb; +Cc: linux-wireless
In-Reply-To: <1555105210-22996-2-git-send-email-greearb@candelatech.com>
On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
>
> From: Ben Greear <greearb@candelatech.com>
>
> This lets us more precisely calculate the absolute timestamp
> of last-rix (ie, now - idle).
Can you use 64-bit timestamps? struct timeval suffers from the
overflow after 2038 problem.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> station.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/station.c b/station.c
> index 3b0c5f1..60804f2 100644
> --- a/station.c
> +++ b/station.c
> @@ -314,6 +314,12 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
> [NL80211_STA_INFO_ACK_SIGNAL_AVG] = { .type = NLA_U8 },
> };
> char *chain;
> + struct timeval now;
> + unsigned long long now_ms;
> +
> + gettimeofday(&now, NULL);
> + now_ms = now.tv_sec * 1000;
> + now_ms += (now.tv_usec / 1000);
>
> nla_parse(tb, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
> genlmsg_attrlen(gnlh, 0), NULL);
> @@ -561,7 +567,7 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
> printf("\n\tauthenticated at:\t%llu ms",
> (unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_AUTH_AT_MS]));
>
> - printf("\n");
> + printf("\n\tcurrent time:\t%llu ms\n", now_ms);
> return NL_SKIP;
> }
>
> --
> 2.7.5
>
^ permalink raw reply
* [PATCH 2/2] iw: Print current time in station info dump
From: greearb @ 2019-04-12 21:40 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
In-Reply-To: <1555105210-22996-1-git-send-email-greearb@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
This lets us more precisely calculate the absolute timestamp
of last-rix (ie, now - idle).
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
station.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/station.c b/station.c
index 3b0c5f1..60804f2 100644
--- a/station.c
+++ b/station.c
@@ -314,6 +314,12 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
[NL80211_STA_INFO_ACK_SIGNAL_AVG] = { .type = NLA_U8 },
};
char *chain;
+ struct timeval now;
+ unsigned long long now_ms;
+
+ gettimeofday(&now, NULL);
+ now_ms = now.tv_sec * 1000;
+ now_ms += (now.tv_usec / 1000);
nla_parse(tb, NL80211_ATTR_MAX, genlmsg_attrdata(gnlh, 0),
genlmsg_attrlen(gnlh, 0), NULL);
@@ -561,7 +567,7 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
printf("\n\tauthenticated at:\t%llu ms",
(unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_AUTH_AT_MS]));
- printf("\n");
+ printf("\n\tcurrent time:\t%llu ms\n", now_ms);
return NL_SKIP;
}
--
2.7.5
^ permalink raw reply related
* [PATCH 1/2] iw: Support authenticated-at station statistic.
From: greearb @ 2019-04-12 21:40 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
This can be helpful for calculating roaming time and other
higher precision stats.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
nl80211.h | 2 ++
station.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/nl80211.h b/nl80211.h
index e1ea2cb..ebaae12 100644
--- a/nl80211.h
+++ b/nl80211.h
@@ -3139,6 +3139,7 @@ enum nl80211_sta_bss_param {
* @NL80211_STA_INFO_TX_DURATION: aggregate PPDU duration for all frames
* sent to the station (u64, usec)
* @NL80211_STA_INFO_AIRTIME_WEIGHT: current airtime weight for station (u16)
+ * @NL80211_STA_INFO_AUTH_AT_MS: Timestamp of last assoc -> auth transition, in ms
* @__NL80211_STA_INFO_AFTER_LAST: internal
* @NL80211_STA_INFO_MAX: highest possible station info attribute
*/
@@ -3184,6 +3185,7 @@ enum nl80211_sta_info {
NL80211_STA_INFO_CONNECTED_TO_GATE,
NL80211_STA_INFO_TX_DURATION,
NL80211_STA_INFO_AIRTIME_WEIGHT,
+ NL80211_STA_INFO_AUTH_AT_MS,
/* keep last */
__NL80211_STA_INFO_AFTER_LAST,
diff --git a/station.c b/station.c
index 25cbbc3..3b0c5f1 100644
--- a/station.c
+++ b/station.c
@@ -557,6 +557,9 @@ static int print_sta_handler(struct nl_msg *msg, void *arg)
if (sinfo[NL80211_STA_INFO_CONNECTED_TIME])
printf("\n\tconnected time:\t%u seconds",
nla_get_u32(sinfo[NL80211_STA_INFO_CONNECTED_TIME]));
+ if (sinfo[NL80211_STA_INFO_AUTH_AT_MS])
+ printf("\n\tauthenticated at:\t%llu ms",
+ (unsigned long long)nla_get_u64(sinfo[NL80211_STA_INFO_AUTH_AT_MS]));
printf("\n");
return NL_SKIP;
--
2.7.5
^ permalink raw reply related
* [PATCH 2/2] mac80211: add auth-at-ms support.
From: greearb @ 2019-04-12 21:37 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
In-Reply-To: <20190412213710.17292-1-greearb@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Report when sta becomes authenticated.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
net/mac80211/sta_info.c | 3 +++
net/mac80211/sta_info.h | 2 ++
2 files changed, 5 insertions(+)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 044120c45950..6dab7abe1603 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1980,6 +1980,7 @@ int sta_info_move_state(struct sta_info *sta,
break;
case IEEE80211_STA_AUTHORIZED:
if (sta->sta_state == IEEE80211_STA_ASSOC) {
+ sta->auth_at_ms = ktime_to_ms(ktime_get_real());
ieee80211_vif_inc_num_mcast(sta->sdata);
set_bit(WLAN_STA_AUTHORIZED, &sta->_flags);
ieee80211_check_fast_xmit(sta);
@@ -2197,6 +2198,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
BIT_ULL(NL80211_STA_INFO_STA_FLAGS) |
BIT_ULL(NL80211_STA_INFO_BSS_PARAM) |
BIT_ULL(NL80211_STA_INFO_CONNECTED_TIME) |
+ BIT_ULL(NL80211_STA_INFO_AUTH_AT_MS) |
BIT_ULL(NL80211_STA_INFO_RX_DROP_MISC);
if (sdata->vif.type == NL80211_IFTYPE_STATION) {
@@ -2205,6 +2207,7 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
}
sinfo->connected_time = ktime_get_seconds() - sta->last_connected;
+ sinfo->auth_at_ms = sta->auth_at_ms;
sinfo->inactive_time =
jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index c5d557032be5..97777978b8f5 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -455,6 +455,7 @@ struct ieee80211_sta_rx_stats {
* the station when it leaves powersave or polls for frames
* @driver_buffered_tids: bitmap of TIDs the driver has data buffered on
* @txq_buffered_tids: bitmap of TIDs that mac80211 has txq data buffered on
+ * @auth_at_ms: time (in ms) of last assoc -> auth transition
* @last_connected: time (in seconds) when a station got connected
* @last_seq_ctrl: last received seq/frag number from this STA (per TID
* plus one for non-QoS frames)
@@ -531,6 +532,7 @@ struct sta_info {
unsigned long driver_buffered_tids;
unsigned long txq_buffered_tids;
+ unsigned long auth_at_ms;
long last_connected;
/* Updated from RX path only, no locking requirements */
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] wireless: Support auth-at-ms timer in sta-info
From: greearb @ 2019-04-12 21:37 UTC (permalink / raw)
To: linux-wireless; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
Report time stamp of when sta became authenticated.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
include/net/cfg80211.h | 2 ++
include/uapi/linux/nl80211.h | 2 ++
net/wireless/nl80211.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f49eb1464b7a..430501c09c01 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1268,6 +1268,7 @@ struct cfg80211_tid_stats {
* indicate the relevant values in this struct for them
* @connected_time: time(in secs) since a station is last connected
* @inactive_time: time since last station activity (tx/rx) in milliseconds
+ * @auth_at_ms: time in ms of the last assoc -> auth transition.
* @rx_bytes: bytes (size of MPDUs) received from this station
* @tx_bytes: bytes (size of MPDUs) transmitted to this station
* @llid: mesh local link id
@@ -1324,6 +1325,7 @@ struct station_info {
u64 filled;
u32 connected_time;
u32 inactive_time;
+ u64 auth_at_ms;
u64 rx_bytes;
u64 tx_bytes;
u16 llid;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 0f03cfcda965..8d0b7a88877c 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3079,6 +3079,7 @@ enum nl80211_sta_bss_param {
* @NL80211_STA_INFO_TX_DURATION: aggregate PPDU duration for all frames
* sent to the station (u64, usec)
* @NL80211_STA_INFO_AIRTIME_WEIGHT: current airtime weight for station (u16)
+ * @NL80211_STA_INFO_AUTH_AT_MS: Timestamp of last assoc -> auth transition, in ms
* @__NL80211_STA_INFO_AFTER_LAST: internal
* @NL80211_STA_INFO_MAX: highest possible station info attribute
*/
@@ -3124,6 +3125,7 @@ enum nl80211_sta_info {
NL80211_STA_INFO_CONNECTED_TO_GATE,
NL80211_STA_INFO_TX_DURATION,
NL80211_STA_INFO_AIRTIME_WEIGHT,
+ NL80211_STA_INFO_AUTH_AT_MS,
/* keep last */
__NL80211_STA_INFO_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bcb432815c64..74be48c1ae02 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4763,6 +4763,7 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
PUT_SINFO(CONNECTED_TIME, connected_time, u32);
PUT_SINFO(INACTIVE_TIME, inactive_time, u32);
+ PUT_SINFO_U64(AUTH_AT_MS, auth_at_ms);
if (sinfo->filled & (BIT_ULL(NL80211_STA_INFO_RX_BYTES) |
BIT_ULL(NL80211_STA_INFO_RX_BYTES64)) &&
--
2.20.1
^ permalink raw reply related
* Re: NL80211_SCAN_FLAG_RANDOM_ADDR ?
From: Arend Van Spriel @ 2019-04-12 21:21 UTC (permalink / raw)
To: Denis Kenzior, Sergey Matyukevich; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <241d2caa-6f41-6363-5164-d2ac83463beb@gmail.com>
On 4/12/2019 5:00 PM, Denis Kenzior wrote:
> Or put another way, besides hardware limitations, are there reasons why
> you would not want to randomize probe request address when connected?
I was hoping to find your answer using 'git blame'. You might already
have tried that. The wdev->current_bss check was added by the commit below:
commit ad2b26abc157460ca6fac1a53a2bfeade283adfa
Author: Johannes Berg <johannes.berg@intel.com>
Date: Thu Jun 12 21:39:05 2014 +0200
cfg80211: allow drivers to support random MAC addresses for scan
Add the necessary feature flags and a scan flag to support using
random MAC addresses for scan while unassociated.
The configuration for this supports an arbitrary MAC address
value and mask, so that any kind of configuration (e.g. fixed
OUI or full 46-bit random) can be requested. Full 46-bit random
is the default when no other configuration is passed.
Also add a small helper function to use the addr/mask correctly.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Unfortunately it does not answer your question. My memory is lacking
quite a bit but I think at the time it was not considered useful to have
random mac address for scan while being associated. Your permanent mac
address would be flying around anyway.
Regards,
Arend
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-12 19:48 UTC (permalink / raw)
To: Johannes Berg
Cc: Harald Welte, openbsc, radiotap, linux-wireless,
Subash Abhinov Kasiviswanathan, Dan Williams, Bjørn Mork,
netdev, Sean Tranchetti, Aleksander Morgado
In-Reply-To: <1462659018bc40830efbe2348791b8df45b54cff.camel@sipsolutions.net>
On Apr 12, 2019, at 5:24 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2019-04-11 at 01:32 +0200, Harald Welte wrote:
>
>> GSMTAP was designed as a format to encapsulate protocols normally not spoken over IP
>> (such as classic GSM radio protocols, e.g. the Layer 2 LAPDm over GSM Um)
>> inside an IP transport.
>
> Sure, but wifi is also not spoken over IP, yet we don't encapsulate our
> 802.11 frames into IP to show them in wireshark :-)
That's just because the rpcap protocol hasn't been revved yet to handle the new create/set options/activate mechanism in libpcap to allow monitor-mode capturing, so you only get to see fake Ethernet frames. :-)
I.e., there's a split there between "capture" and "getting the packets from a capture delivered to you over an IP network".
Perhaps there should be a GSMTAP link-layer header type, so you can have a GSMTAP pcap file or GSMTAP interface in a pcapng file, combined with a more general "remote capture" mechanism in libpcap so that you could capture on gsmtap://host:port and capture from a host using the GSMTAP-over-UDP encapsulation - or capture using rpcap.
>> No userspace source would ever be able to generate such data and stream
>> it real-time into wireshark, would it? Sure, I can write pcap file with
>> such ARPHDR_* values, but I could never do this in real-time. For many
>> but not all use cases, that's really what it is: A vehicle to stream
>> real-time non-IP protocol traces into wireshark so it can visualize
>> the protocol traces.
>
> I think you can pipe a stream into wireshark?
1) You could pipe into libpcap or otherwise have a way for a libpcap module to connect to a user space source and get packets from it.
2) You could pipe a pcap file into tcpdump/Wireshark/etc..
3) You could have an extcap program:
https://www.wireshark.org/docs/wsdg_html_chunked/ChCaptureExtcap.html
provide packets to Wireshark.
> To me it feels like the wrong thing to actually make wireshark listen on
> "lo" or "eth0" or something to get data from the cellular that's
> (locally) generated by another application, but I guess that's only
> about how you think about it - and if it's not generated locally then
> that's an easy transport. I'm not sure it makes *sense* because then you
> need permissions to capture on the wired network etc.
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.
> where you don't *really* need that for this stream
If there's a need for that, whatever provides the packets could impose that (by finding out the UID/GID of the other process if this is going over a UNIX-domain socket, at least on some UN*Xes, or by requiring a login if this is going over a network).
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-12 19:54 UTC (permalink / raw)
To: Johannes Berg
Cc: Harald Welte, Vadim Yanitskiy, OpenBSC Mailing List,
Sean Tranchetti, radiotap, Dan Williams, netdev, linux-wireless,
Aleksander Morgado, Subash Abhinov Kasiviswanathan,
Bjørn Mork
In-Reply-To: <f0a64bb751b8c601f58130a7f3a3158d70f18f63.camel@sipsolutions.net>
On Apr 12, 2019, at 10:15 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> Agree. Sorry about that. No disrespect was intended, but I'm still not
> sure I understand the need for UDP encapsulation *as part of the
> protocol*. I guess saying "GSMTAP can optionally be encapsulated in UDP
> with the well-known port xyz" would be something else, and it'd make
> more sense to me than saying it has to be.
I see nothing about a struct gsmtap_hdr:
http://osmocom.org/projects/baseband/wiki/GSMTAP
that
1) requires that it plus the payload be encapsulated in a UDP datagram
or
2) would prevent it from being at the beginning of a LINKTYPE_GSMTAP/DLT_GSMTAP packet in a pcap or pcapng file.
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Marcel Holtmann @ 2019-04-12 19:49 UTC (permalink / raw)
To: Johannes Berg
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: <1d64c578cd5b254d301cf1cac82f32a062916888.camel@sipsolutions.net>
Hi Johannes,
>> before you go all out and define this, it would suggest to understand
>> what meta-data for the connection contexts you actually need as well.
>> The data path itself is just a pipe and has not all the information
>> attached with it. That goes via the control path and that is normally
>> in user space and carries the real important information to make
>> useful analysis of how the data path / context is setup.
>
> 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.
>> From what I am seeing right now is that unless you have a method to
>> also feed the control path into your GSMTAPv3, then this is rather
>> useless.
>
> So the control path *itself* would be there, I guess, but ...
>
>> The majority of the debugging is really done for the control path. For
>> oFono that is OFONO_DEBUG=1 environment variable and while it works it
>> is not the most elegant solution. I would love to feed that into a
>> generic debugging / tap that you can read out later.
>
> there's definitely room for more information than _just_ the control
> path "chat", also application state etc. would be useful, and logging
> etc.
>
> Typically on wifi we feed all of this together into kernel tracing (to
> record with trace-cmd) rather than trying to encapsulate it some other
> way.
>
>> As a side note, for Bluetooth we created a path where the bluetoothd
>> can feed back its control debugging data back into the Bluetooth
>> monitor in the kernel to allow combined userspace, mgmt and HCI
>> tracing. Some really nasty issues could only be triaged by having all
>> the meta data with a common timestamp.
>
> 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.
> I don't really know what the right model for WWAN would be, I guess.
>
>
> Right now - and I really should've said this before - really the only
> problem I was thinking of was how we can mux multiple "chat" sessions
> with a device into a single data stream.
>
> Currently, this is all vendor-specific. If you have a Qualcomm modem,
> you'd be able to see all the open sessions on the underlying netdev, and
> the QMI header tells you what session a given 'packet' belongs to, and
> if you follow along maybe you can figure out if this is a control or IP
> 'packet' (could be an AT command).
>
> cdc_mbim uses VLAN tags instead to achieve this, and decapsulates the
> VLAN tags to send them down to the hardware in a different way.
>
> 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. 3GPP and 3GPP2 are not Ethernet frame based. We only have raw IPv4 and IPv6 packets for the data path.
> But if you do this, you lose the ability to listen to all the session
> streams at the same time, you can only do it for each netdev. Adding
> this ability back seems worthwhile, but then we probably shouldn't do it
> in a vendor-specific way, but rather in a generic way.
>
> So basically right now that's all I'm trying to solve. In WiFi we don't
> have the problem of "sessions" because we just use the addresses in the
> frames to disambiguate - on such 'monitor' netdevs we see the frames
> including full 802.11 headers.
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).
> Now, here's maybe where I'm getting off the right path - in wifi we
> mostly couple that with PHY information as well, and so we have PHY
> information + full 802.11 headers + data for capturing what's going on.
> I figured that theoretically at least that would be possible/useful for
> the modem as well (obviously control packets have no PHY data), but
> there doesn't seem to be any hardware that would actually expose data in
> this way. Everyone I've spoken to says these things are only available
> as modem trace data. What I haven't figured out though is if that's by
> some other design trade-off, or just because no such infrastructure
> is/was available.
>
> 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.
Regards
Marcel
^ permalink raw reply
* Re: pull-request: iwlwifi-fixes 2019-04-03
From: Kalle Valo @ 2019-04-12 18:35 UTC (permalink / raw)
To: Luca Coelho; +Cc: linux-wireless, linuxwifi, david.e.box, joe.konno
In-Reply-To: <6f02ef035f948c9d01d66f52f84403caf2384001.camel@coelho.fi>
Luca Coelho <luca@coelho.fi> writes:
> Hi Kalle,
>
> This is the second batch of fixes for v5.1. More details about the
> contents in the tag description.
>
> I have sent this out before and kbuildbot reported success.
>
> Please let me know if there are any issues.
>
> Cheers,
> Luca.
>
>
> The following changes since commit 4837696f6b5451242b2095764287b6336e844683:
>
> Merge tag 'iwlwifi-for-kalle-2019-03-22' of git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-fixes (2019-03-30 08:33:40 +0200)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-fixes.git tags/iwlwifi-for-kalle-2019-04-03
>
> for you to fetch changes up to debec2f23910cb17f2c0f6d5e30a8da00bb5f515:
>
> iwlwifi: add support for quz firmwares (2019-04-03 11:13:06 +0300)
>
> ----------------------------------------------------------------
> Second batch of iwlwifi fixes intended for v5.1
>
> * fix for a potential deadlock in the TX path;
> * a fix for offloaded rate-control;
> * support new PCI HW IDs which use a new FW;
>
> ----------------------------------------------------------------
Pulled, thanks Luca.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 5.1] mt76x02: avoid status_list.lock and sta->rate_ctrl_lock dependency
From: Kalle Valo @ 2019-04-12 18:33 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: linux-wireless, Felix Fietkau, Lorenzo Bianconi
In-Reply-To: <20190405114256.6416-1-sgruszka@redhat.com>
Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Move ieee80211_tx_status_ext() outside of status_list lock section
> in order to avoid locking dependency and possible deadlock reposed by
> LOCKDEP in below warning.
>
> Also do mt76_tx_status_lock() just before it's needed.
>
> [ 440.224832] WARNING: possible circular locking dependency detected
> [ 440.224833] 5.1.0-rc2+ #22 Not tainted
> [ 440.224834] ------------------------------------------------------
> [ 440.224835] kworker/u16:28/2362 is trying to acquire lock:
> [ 440.224836] 0000000089b8cacf (&(&q->lock)->rlock#2){+.-.}, at: mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.224842]
> but task is already holding lock:
> [ 440.224842] 000000002cfedc59 (&(&sta->lock)->rlock){+.-.}, at: ieee80211_stop_tx_ba_cb+0x32/0x1f0 [mac80211]
> [ 440.224863]
> which lock already depends on the new lock.
>
> [ 440.224863]
> the existing dependency chain (in reverse order) is:
> [ 440.224864]
> -> #3 (&(&sta->lock)->rlock){+.-.}:
> [ 440.224869] _raw_spin_lock_bh+0x34/0x40
> [ 440.224880] ieee80211_start_tx_ba_session+0xe4/0x3d0 [mac80211]
> [ 440.224894] minstrel_ht_get_rate+0x45c/0x510 [mac80211]
> [ 440.224906] rate_control_get_rate+0xc1/0x140 [mac80211]
> [ 440.224918] ieee80211_tx_h_rate_ctrl+0x195/0x3c0 [mac80211]
> [ 440.224930] ieee80211_xmit_fast+0x26d/0xa50 [mac80211]
> [ 440.224942] __ieee80211_subif_start_xmit+0xfc/0x310 [mac80211]
> [ 440.224954] ieee80211_subif_start_xmit+0x38/0x390 [mac80211]
> [ 440.224956] dev_hard_start_xmit+0xb8/0x300
> [ 440.224957] __dev_queue_xmit+0x7d4/0xbb0
> [ 440.224968] ip6_finish_output2+0x246/0x860 [ipv6]
> [ 440.224978] mld_sendpack+0x1bd/0x360 [ipv6]
> [ 440.224987] mld_ifc_timer_expire+0x1a4/0x2f0 [ipv6]
> [ 440.224989] call_timer_fn+0x89/0x2a0
> [ 440.224990] run_timer_softirq+0x1bd/0x4d0
> [ 440.224992] __do_softirq+0xdb/0x47c
> [ 440.224994] irq_exit+0xfa/0x100
> [ 440.224996] smp_apic_timer_interrupt+0x9a/0x220
> [ 440.224997] apic_timer_interrupt+0xf/0x20
> [ 440.224999] cpuidle_enter_state+0xc1/0x470
> [ 440.225000] do_idle+0x21a/0x260
> [ 440.225001] cpu_startup_entry+0x19/0x20
> [ 440.225004] start_secondary+0x135/0x170
> [ 440.225006] secondary_startup_64+0xa4/0xb0
> [ 440.225007]
> -> #2 (&(&sta->rate_ctrl_lock)->rlock){+.-.}:
> [ 440.225009] _raw_spin_lock_bh+0x34/0x40
> [ 440.225022] rate_control_tx_status+0x4f/0xb0 [mac80211]
> [ 440.225031] ieee80211_tx_status_ext+0x142/0x1a0 [mac80211]
> [ 440.225035] mt76x02_send_tx_status+0x2e4/0x340 [mt76x02_lib]
> [ 440.225037] mt76x02_tx_status_data+0x31/0x40 [mt76x02_lib]
> [ 440.225040] mt76u_tx_status_data+0x51/0xa0 [mt76_usb]
> [ 440.225042] process_one_work+0x237/0x5d0
> [ 440.225043] worker_thread+0x3c/0x390
> [ 440.225045] kthread+0x11d/0x140
> [ 440.225046] ret_from_fork+0x3a/0x50
> [ 440.225047]
> -> #1 (&(&list->lock)->rlock#8){+.-.}:
> [ 440.225049] _raw_spin_lock_bh+0x34/0x40
> [ 440.225052] mt76_tx_status_skb_add+0x51/0x100 [mt76]
> [ 440.225054] mt76x02u_tx_prepare_skb+0xbd/0x116 [mt76x02_usb]
> [ 440.225056] mt76u_tx_queue_skb+0x5f/0x180 [mt76_usb]
> [ 440.225058] mt76_tx+0x93/0x190 [mt76]
> [ 440.225070] ieee80211_tx_frags+0x148/0x210 [mac80211]
> [ 440.225081] __ieee80211_tx+0x75/0x1b0 [mac80211]
> [ 440.225092] ieee80211_tx+0xde/0x110 [mac80211]
> [ 440.225105] __ieee80211_tx_skb_tid_band+0x72/0x90 [mac80211]
> [ 440.225122] ieee80211_send_auth+0x1f3/0x360 [mac80211]
> [ 440.225141] ieee80211_auth.cold.40+0x6c/0x100 [mac80211]
> [ 440.225156] ieee80211_mgd_auth.cold.50+0x132/0x15f [mac80211]
> [ 440.225171] cfg80211_mlme_auth+0x149/0x360 [cfg80211]
> [ 440.225181] nl80211_authenticate+0x273/0x2e0 [cfg80211]
> [ 440.225183] genl_family_rcv_msg+0x196/0x3a0
> [ 440.225184] genl_rcv_msg+0x47/0x8e
> [ 440.225185] netlink_rcv_skb+0x3a/0xf0
> [ 440.225187] genl_rcv+0x24/0x40
> [ 440.225188] netlink_unicast+0x16d/0x210
> [ 440.225189] netlink_sendmsg+0x204/0x3b0
> [ 440.225191] sock_sendmsg+0x36/0x40
> [ 440.225193] ___sys_sendmsg+0x259/0x2b0
> [ 440.225194] __sys_sendmsg+0x47/0x80
> [ 440.225196] do_syscall_64+0x60/0x1f0
> [ 440.225197] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 440.225198]
> -> #0 (&(&q->lock)->rlock#2){+.-.}:
> [ 440.225200] lock_acquire+0xb9/0x1a0
> [ 440.225202] _raw_spin_lock_bh+0x34/0x40
> [ 440.225204] mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225215] ieee80211_agg_start_txq+0xe8/0x2b0 [mac80211]
> [ 440.225225] ieee80211_stop_tx_ba_cb+0xb8/0x1f0 [mac80211]
> [ 440.225235] ieee80211_ba_session_work+0x1c1/0x2f0 [mac80211]
> [ 440.225236] process_one_work+0x237/0x5d0
> [ 440.225237] worker_thread+0x3c/0x390
> [ 440.225239] kthread+0x11d/0x140
> [ 440.225240] ret_from_fork+0x3a/0x50
> [ 440.225240]
> other info that might help us debug this:
>
> [ 440.225241] Chain exists of:
> &(&q->lock)->rlock#2 --> &(&sta->rate_ctrl_lock)->rlock --> &(&sta->lock)->rlock
>
> [ 440.225243] Possible unsafe locking scenario:
>
> [ 440.225244] CPU0 CPU1
> [ 440.225244] ---- ----
> [ 440.225245] lock(&(&sta->lock)->rlock);
> [ 440.225245] lock(&(&sta->rate_ctrl_lock)->rlock);
> [ 440.225246] lock(&(&sta->lock)->rlock);
> [ 440.225247] lock(&(&q->lock)->rlock#2);
> [ 440.225248]
> *** DEADLOCK ***
>
> [ 440.225249] 5 locks held by kworker/u16:28/2362:
> [ 440.225250] #0: 0000000048fcd291 ((wq_completion)phy0){+.+.}, at: process_one_work+0x1b5/0x5d0
> [ 440.225252] #1: 00000000f1c6828f ((work_completion)(&sta->ampdu_mlme.work)){+.+.}, at: process_one_work+0x1b5/0x5d0
> [ 440.225254] #2: 00000000433d2b2c (&sta->ampdu_mlme.mtx){+.+.}, at: ieee80211_ba_session_work+0x5c/0x2f0 [mac80211]
> [ 440.225265] #3: 000000002cfedc59 (&(&sta->lock)->rlock){+.-.}, at: ieee80211_stop_tx_ba_cb+0x32/0x1f0 [mac80211]
> [ 440.225276] #4: 000000009d7b9a44 (rcu_read_lock){....}, at: ieee80211_agg_start_txq+0x33/0x2b0 [mac80211]
> [ 440.225286]
> stack backtrace:
> [ 440.225288] CPU: 2 PID: 2362 Comm: kworker/u16:28 Not tainted 5.1.0-rc2+ #22
> [ 440.225289] Hardware name: LENOVO 20KGS23S0P/20KGS23S0P, BIOS N23ET55W (1.30 ) 08/31/2018
> [ 440.225300] Workqueue: phy0 ieee80211_ba_session_work [mac80211]
> [ 440.225301] Call Trace:
> [ 440.225304] dump_stack+0x85/0xc0
> [ 440.225306] print_circular_bug.isra.38.cold.58+0x15c/0x195
> [ 440.225307] check_prev_add.constprop.48+0x5f0/0xc00
> [ 440.225309] ? check_prev_add.constprop.48+0x39d/0xc00
> [ 440.225311] ? __lock_acquire+0x41d/0x1100
> [ 440.225312] __lock_acquire+0xd98/0x1100
> [ 440.225313] ? __lock_acquire+0x41d/0x1100
> [ 440.225315] lock_acquire+0xb9/0x1a0
> [ 440.225317] ? mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225319] _raw_spin_lock_bh+0x34/0x40
> [ 440.225321] ? mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225323] mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225334] ieee80211_agg_start_txq+0xe8/0x2b0 [mac80211]
> [ 440.225344] ieee80211_stop_tx_ba_cb+0xb8/0x1f0 [mac80211]
> [ 440.225354] ieee80211_ba_session_work+0x1c1/0x2f0 [mac80211]
> [ 440.225356] process_one_work+0x237/0x5d0
> [ 440.225358] worker_thread+0x3c/0x390
> [ 440.225359] ? wq_calc_node_cpumask+0x70/0x70
> [ 440.225360] kthread+0x11d/0x140
> [ 440.225362] ? kthread_create_on_node+0x40/0x40
> [ 440.225363] ret_from_fork+0x3a/0x50
>
> Cc: stable@vger.kernel.org
> Fixes: 88046b2c9f6d ("mt76: add support for reporting tx status with skb")
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Acked-by: Felix Fietkau <nbd@nbd.name>
Patch applied to wireless-drivers.git, thanks.
bafdf85dfa59 mt76x02: avoid status_list.lock and sta->rate_ctrl_lock dependency
--
https://patchwork.kernel.org/patch/10887243/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox