Netdev List
 help / color / mirror / Atom feed
* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Jian-Hong Pan @ 2018-07-03 15:11 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Michal Kubeček, Konstantin Böhm, contact, Ken Yu,
	Michael Röder, Rob Herring, lora, Alexander Graf,
	Jan Jongboom, Janus Piwek, Jon Ortego, devicetree, Jiri Pirko,
	Marcel Holtmann, Mark Brown, Dollar Chen, Brian Ray,
	linux-arm-kernel, lora, Matthias Brugger, Ben Whitten, netdev,
	linux-kernel, linux-spi, Steve deRosier
In-Reply-To: <20180701110804.32415-1-afaerber@suse.de>

Hi Andreas,

First, thanks for your great jobs.  LoRaWAN module needs the
compatible devices drivers.
I will reply the message under the LoRaWAN specification.

2018-07-01 19:07 GMT+08:00 Andreas Färber <afaerber@suse.de>:
> Hello,
>
> LoRa is a long-range, low-power wireless technology by Semtech.
> Unlike other LPWAN technologies, users don't need to rely on infrastructure
> providers and SIM cards and expensive subscription plans, they can set up
> their own gateways. Modules, adapters and evaluation boards are available
> from a large number of vendors.
>
> Many vendors also make available Open Source software examples on GitHub.
> But when taking a closer look, many of them combine licenses in ways that are
> not redistributable. My reports have remained without response or solution.
>
> https://github.com/ernstdevreede/lmic_pi/issues/2
> https://github.com/Snootlab/lmic_chisterapi/issues/2
> https://github.com/Snootlab/lora_chisterapi/issues/2
>
> Another issue was that most such projects around the Raspberry Pi make use of
> spidev to communicate with the Semtech chipsets from userspace. The Linux spi
> maintainers have chosen to greet any such users of spidev with a friendly
> WARN_ON(), preferring in-kernel spi drivers and white-listing individual
> devices only.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spidev.c?h=v4.17#n722
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spidev.c?h=v4.17#n667
>
> Also I don't quite see the point in having userspace probe what SPI devices
> are connected to a generic spidev driver when we have an easy Device Tree
> hardware description on arm/arm64 that could give us that info.
>
> I raised the topic during Q&A of a FOSDEM 2017 talk (cut off at the end
> of the video) but unfortunately found no one to collaborate on this.
>
> https://archive.fosdem.org/2017/schedule/event/lorawan/
>
> Instead of porting from wiringPi to a differently licensed GPIO library
> and dealing with seemingly unmaintained LoRaWAN code dumps, I started a
> spi kernel driver for SX1276 back in 2016. But obviously a kernel driver
> isn't too helpful without a userspace API to send and receive packets.
>
> This patchset, updated from 2017 and extended, is implementing kernel drivers
> for various LoRa chipsets and modules. As API I'm proposing a PF_LORA socket
> implementation. Why? LoRa uses data packets with headers and checksums
> and differing MTUs and multiple protocols layered on top of it. Apart from
> simple headers for addressing used by RadioHead library and IMST's LoRa P2P
> protocol, the main use case (not implemented in this patchset) is expected
> to be LoRaWAN. And LoRaWAN has competing proprietary protocols, such as
> Link Labs' Symphony Link or GlobalSat M.O.S.T. or RadioShuttle, that might
> at some point want to adopt a standard API for their implementations, too.
>
> Ready-made LoRa hardware modules come in three flavors,
> a) with SPI access to the underlying Semtech chipsets, needing a software
>    implementation of e.g. LoRaWAN protocol stack (i.e., a soft MAC),
> b) with a custom, often UART based interface and a pre-certified LoRaWAN
>    protocol stack already integrated (i.e., hard/full MAC), and
> c) with a microcontroller that serves not only for the protocol stack but
>    also as application processor, not offering a ready-made interface.
>
> This patchset focuses on option a). An SX1276 based LoRaWAN stack appeared
> to be the project of Jian-Hong Pan and is not included here.

Thanks, LoRaWAN module needs the compatible device drivers.

> This patchset also includes drivers for b), from text based AT commands to
> a binary SLIP based HCI protocol.
> Hardware examples for c) are Murata CMWX1ZZABZ-078 and RAK813.
>
> This patchset is clearly not ready for merging, but is being submitted for
> discussion, as requested by Jiri, in particular of the design choices:
>
> 1) PF_LORA/AF_LORA and associated identifiers are proposed to represent
>    this technology. While for an SX1276 - case a) above - it might work to
>    layer LoRaWAN as a protocol option for PF_LORA and add LoRaWAN address
>    fields to the union in my sockaddr_lora, how would that work for devices
>    that only support LoRaWAN but not pure LoRa? Do we need both AF_LORA and
>    AF_LORAWAN, or just a separate ETH_P_LORAWAN or ARPHRD_LORAWAN?

The LoRaWAN module also register the LoRa device as a network device.
Will use the macros AF_LORAWAN, PF_LORAWAN, ARPHRD_LORAWAN and ETH_P_LORAWAN.

> 2) PF_LORA is used with SOCK_DGRAM here. The assumption is that RAW mode
>    would be DGRAM plus preamble plus optional checksum.

Is the preamble added by the hardware itself here or software here?

> 3) Only the transmit path is partially implemented already. The assumption
>    is that the devices should go into receive mode by default and only
>    interrupt that when asked to transmit.

If it goes with LoRaWAN spec., end devices should be in idle (or
called standby mode) by default.  Unless the device is asked to be in
transmitting or receiving timing slot.

> 4) Some hardware settings need to be supplied externally, such as the radio
>    frequency for some modules, but many others can be runtime-configured,
>    such as Spreading Factor, Bandwidth, Sync Word, or which antenna to use.
>    What settings should be implemented as socket option vs. netlink layer
>    vs. ioctl vs. sysfs? What are the criteria to apply?

- We can have a pre-defined table according to LoRaWAN Regional Parameters.
- Device driver declares the hardware's capability, for example
frequency, TX power.  And then registers as a LoRaWAN compatible
device.
- LoRaWAN module handle the requests from upper layer or MAC commands
from a gateway (netlink layer), than uses the pre-defined interface
functions to set the parameters.

LoRaWAN module will export the operation functions interface.

> 5) Many of the modules support multiple modes, such as LoRa, LoRaWAN and FSK.
>    Lacking a LoRaWAN implementation, I am currently switching them into LoRa
>    mode at probe time wherever possible. How do we deal with that properly?

- There are data rate tables defined in LoRaWAN Regional Parameters.
Those contain which data rate uses LoRa mode or FSK mode.
- LoRaWAN should handle the data rate changing requests and then calls
the corresponding operation functions.  Of course, the hardware's
capability registered before should also be under consideration at the
same time.

>   a) Is there any precedence from the Wifi world for dynamically selecting
>      between our own trusted Open Source implementation vs. hardware/firmware
>      accelerated and/or certified implementations?
>
>   b) Would a proof of concept for FSK (non-LoRa) modes be required for
>      merging any LoRa driver for chipsets that support both? Or is there any
>      facility or design guidelines that would allow us to focus on LoRa and
>      LoRaWAN and leave non-LoRa radio modes to later contributors?
>
> As evident by the many questions, this is my first deep dive into the Linux
> net subsystem. It's also my first experiments with the new serdev subsystem,
> so in particular the receive paths will need some review and optimizations.
>
> This patchset was developed and tested mainly as KMP, originally at
> https://github.com/afaerber/lora-modules. It was recently transformed into a
> linux-next based tree, still mostly tested on our openSUSE Tumbleweed kernel
> with a differing AF_LORA value below current AF_MAX limit.
>
> Some corresponding Device Tree Overlays have been collected here:
> https://github.com/afaerber/dt-overlays
>
> Only European models for 868 MHz and 433 MHz could be tested when available.

I can test 922-928 MHz in Taiwan.
But I need a workable LoRaWAN gateway first! XD

Cheers,
Jian-Hong Pan

> Thanks to all companies and people that have supported this project so far.
>
> Have a lot of fun!
>
> Cheers,
> Andreas
>
> Cc: Jian-Hong Pan <starnight@g.ncu.edu.tw>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Matthias Brugger <mbrugger@suse.com>
> Cc: Konstantin Böhm <konstantin.boehm@ancud.de>
> Cc: Jan Jongboom <jan.jongboom@arm.com>
> Cc: Janus Piwek <jpiwek@arroweurope.com>
> Cc: Michael Röder <michael.roeder@avnet.eu>
> Cc: Dollar Chen (陳義元) <dollar.chen@wtmec.com>
> Cc: Ken Yu (禹凯) <ken.yu@rakwireless.com>
> Cc: Jon Ortego <Jon.Ortego@imst.de>
> Cc: contact@snootlab.com
> Cc: Ben Whitten <ben.whitten@lairdtech.com>
> Cc: Brian Ray <brian.ray@link-labs.com>
> Cc: lora@globalsat.com.tw
> Cc: lora@radioshuttle.de
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Michal Kubeček <mkubecek@suse.cz>
> Cc: Rob Herring <robh@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Steve deRosier <derosier@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-spi@vger.kernel.org
>
> Andreas Färber (15):
>   net: Reserve protocol numbers for LoRa
>   net: lora: Define sockaddr_lora
>   net: lora: Add protocol numbers
>   net: Add lora subsystem
>   HACK: net: lora: Deal with .poll_mask in 4.18-rc2
>   net: lora: Prepare for device drivers
>   net: lora: Add Semtech SX1276
>   net: lora: sx1276: Add debugfs
>   net: lora: Prepare EUI helpers
>   net: lora: Add Microchip RN2483
>   net: lora: Add IMST WiMOD
>   net: lora: Add USI WM-SG-SM-42
>   net: lora: Prepare RAK RAK811
>   net: lora: Prepare Semtech SX1257
>   net: lora: Add Semtech SX1301
>
>  drivers/net/Makefile                |   1 +
>  drivers/net/lora/Kconfig            |  72 ++++
>  drivers/net/lora/Makefile           |  32 ++
>  drivers/net/lora/dev.c              | 125 ++++++
>  drivers/net/lora/rak811.c           | 219 +++++++++++
>  drivers/net/lora/rn2483.c           | 344 +++++++++++++++++
>  drivers/net/lora/rn2483.h           |  40 ++
>  drivers/net/lora/rn2483_cmd.c       | 130 +++++++
>  drivers/net/lora/sx1257.c           |  96 +++++
>  drivers/net/lora/sx1276.c           | 740 ++++++++++++++++++++++++++++++++++++
>  drivers/net/lora/sx1301.c           | 446 ++++++++++++++++++++++
>  drivers/net/lora/usi.c              | 411 ++++++++++++++++++++
>  drivers/net/lora/wimod.c            | 597 +++++++++++++++++++++++++++++
>  include/linux/lora/dev.h            |  44 +++
>  include/linux/lora/skb.h            |  29 ++
>  include/linux/socket.h              |   4 +-
>  include/uapi/linux/if_arp.h         |   1 +
>  include/uapi/linux/if_ether.h       |   1 +
>  include/uapi/linux/lora.h           |  24 ++
>  net/Kconfig                         |   1 +
>  net/Makefile                        |   1 +
>  net/lora/Kconfig                    |  15 +
>  net/lora/Makefile                   |   8 +
>  net/lora/af_lora.c                  | 152 ++++++++
>  net/lora/af_lora.h                  |  13 +
>  net/lora/dgram.c                    | 297 +++++++++++++++
>  security/selinux/hooks.c            |   4 +-
>  security/selinux/include/classmap.h |   4 +-
>  28 files changed, 3848 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/net/lora/Kconfig
>  create mode 100644 drivers/net/lora/Makefile
>  create mode 100644 drivers/net/lora/dev.c
>  create mode 100644 drivers/net/lora/rak811.c
>  create mode 100644 drivers/net/lora/rn2483.c
>  create mode 100644 drivers/net/lora/rn2483.h
>  create mode 100644 drivers/net/lora/rn2483_cmd.c
>  create mode 100644 drivers/net/lora/sx1257.c
>  create mode 100644 drivers/net/lora/sx1276.c
>  create mode 100644 drivers/net/lora/sx1301.c
>  create mode 100644 drivers/net/lora/usi.c
>  create mode 100644 drivers/net/lora/wimod.c
>  create mode 100644 include/linux/lora/dev.h
>  create mode 100644 include/linux/lora/skb.h
>  create mode 100644 include/uapi/linux/lora.h
>  create mode 100644 net/lora/Kconfig
>  create mode 100644 net/lora/Makefile
>  create mode 100644 net/lora/af_lora.c
>  create mode 100644 net/lora/af_lora.h
>  create mode 100644 net/lora/dgram.c
>
> --
> 2.16.4
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-03 15:15 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
	Neal Cardwell, Steve Ibanez, Eric Dumazet
In-Reply-To: <CAK6E8=f2n9PKwne5COvT0sFyFymfJDDeE9Vxk6PeTpOEwg7z8w@mail.gmail.com>

On 7/2/18, 4:50 PM, "Yuchung Cheng" <ycheng@google.com> wrote:

    On Mon, Jul 2, 2018 at 2:39 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
    >
    > DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
    > notifications to keep track if it needs to send an ACK for packets that
    > were received with a particular ECN state but whose ACK was delayed.
    >
    > Under some circumstances, for example when a delayed ACK is sent with a
    > data packet, DCTCP state was not being updated due to a lack of
    > notification that the previously delayed ACK was sent. As a result, it
    > would sometimes send a duplicate ACK when a new data packet arrived.
    >
    > This patch insures that DCTCP's state is correctly updated so it will
    > not send the duplicate ACK.
    Sorry to chime-in late here (lame excuse: IETF deadline)
    
    IIRC this issue would exist prior to 4.11 kernel. While it'd be good
    to fix that, it's not clear which patch introduced the regression
    between 4.11 and 4.16? I assume you tested Eric's most recent quickack
    fix.
    
    In terms of the fix itself, it seems odd the tcp_send_ack() call in
    DCTCP generates NON_DELAYED_ACK event to toggle DCTCP's
    delayed_ack_reserved bit. Shouldn't the fix to have DCTCP send the
    "prior" ACK w/o cancelling delayed-ACK and mis-recording that it's
    cancelled, because that prior-ACK really is a supplementary old ACK.
    
    But it's still unclear how this bug introduces the regression 4.11 - 4.16
   
Feedback is always appreciated! This issue is also present in 4.11 (that is where I discovered). I think the bug was introduces much earlier.

Yes, I tested with Eric's quickack fix, it did not fix either of the two issues that are fixed with this patch set.

As I mentioned earlier, the bug was introduced before 4.11. I am not sure I understand your comments. Yes, at some level it would make sense to change the delayed_ack_reserved bit directly, but we would still need to do it whenever we send the ACK, so I do not think it can be helped. Please clarify if I misunderstood your comment.
    
    >
    > Improved based on comments from Neal Cardwell <ncardwell@google.com>.
    >
    > Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
    > ---
    >  net/ipv4/tcp_output.c | 4 ++--
    >  1 file changed, 2 insertions(+), 2 deletions(-)
    >
    > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
    > index f8f6129160dd..acefb64e8280 100644
    > --- a/net/ipv4/tcp_output.c
    > +++ b/net/ipv4/tcp_output.c
    > @@ -172,6 +172,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
    >                         __sock_put(sk);
    >         }
    >         tcp_dec_quickack_mode(sk, pkts);
    > +       if (inet_csk_ack_scheduled(sk))
    > +               tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
    >         inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
    >  }
    >
    > @@ -3567,8 +3569,6 @@ void tcp_send_ack(struct sock *sk)
    >         if (sk->sk_state == TCP_CLOSE)
    >                 return;
    >
    > -       tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
    > -
    >         /* We are not putting this on the write queue, so
    >          * tcp_transmit_skb() will set the ownership to this
    >          * sock.
    > --
    > 2.17.1
    >
    

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] tcp: fix high tail latencies in DCTCP
From: Neal Cardwell @ 2018-07-03 15:25 UTC (permalink / raw)
  To: Lawrence Brakmo
  Cc: Netdev, Kernel Team, bmatheny, ast, Yuchung Cheng, Steve Ibanez,
	Eric Dumazet
In-Reply-To: <DAC7EDC0-D829-4107-AE04-4850118F571D@fb.com>

On Tue, Jul 3, 2018 at 11:10 AM Lawrence Brakmo <brakmo@fb.com> wrote:
>
> On 7/2/18, 5:52 PM, "netdev-owner@vger.kernel.org on behalf of Neal Cardwell" <netdev-owner@vger.kernel.org on behalf of ncardwell@google.com> wrote:
>
>     On Mon, Jul 2, 2018 at 5:39 PM Lawrence Brakmo <brakmo@fb.com> wrote:
>     >
>     > When have observed high tail latencies when using DCTCP for RPCs as
>     > compared to using Cubic. For example, in one setup there are 2 hosts
>     > sending to a 3rd one, with each sender having 3 flows (1 stream,
>     > 1 1MB back-to-back RPCs and 1 10KB back-to-back RPCs). The following
>     > table shows the 99% and 99.9% latencies for both Cubic and dctcp:
>     >
>     >            Cubic 99%  Cubic 99.9%   dctcp 99%    dctcp 99.9%
>     >  1MB RPCs    2.6ms       5.5ms         43ms          208ms
>     > 10KB RPCs    1.1ms       1.3ms         53ms          212ms
>     >
>     > Looking at tcpdump traces showed that there are two causes for the
>     > latency.
>     >
>     >   1) RTOs caused by the receiver sending a dup ACK and not ACKing
>     >      the last (and only) packet sent.
>     >   2) Delaying ACKs when the sender has a cwnd of 1, so everything
>     >      pauses for the duration of the delayed ACK.
>     >
>     > The first patch fixes the cause of the dup ACKs, not updating DCTCP
>     > state when an ACK that was initially delayed has been sent with a
>     > data packet.
>     >
>     > The second patch insures that an ACK is sent immediately when a
>     > CWR marked packet arrives.
>     >
>     > With the patches the latencies for DCTCP now look like:
>     >
>     >            dctcp 99%  dctcp 99.9%
>     >  1MB RPCs    5.8ms       6.9ms
>     > 10KB RPCs    146us       203us
>     >
>     > Note that while the 1MB RPCs tail latencies are higher than Cubic's,
>     > the 10KB latencies are much smaller than Cubic's. These patches fix
>     > issues on the receiver, but tcpdump traces indicate there is an
>     > opportunity to also fix an issue at the sender that adds about 3ms
>     > to the tail latencies.
>     >
>     > The following trace shows the issue that tiggers an RTO (fixed by these patches):
>     >
>     >    Host A sends the last packets of the request
>     >    Host B receives them, and the last packet is marked with congestion (CE)
>     >    Host B sends ACKs for packets not marked with congestion
>     >    Host B sends data packet with reply and ACK for packet marked with
>     >           congestion (TCP flag ECE)
>     >    Host A receives ACKs with no ECE flag
>     >    Host A receives data packet with ACK for the last packet of request
>     >           and which has TCP ECE bit set
>     >    Host A sends 1st data packet of the next request with TCP flag CWR
>     >    Host B receives the packet (as seen in tcpdump at B), no CE flag
>     >    Host B sends a dup ACK that also has the TCP ECE flag
>     >    Host A RTO timer fires!
>     >    Host A to send the next packet
>     >    Host A receives an ACK for everything it has sent (i.e. Host B
>     >           did receive 1st packet of request)
>     >    Host A send more packets…
>     >
>     > [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
>     > [PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet
>     >
>     >  net/ipv4/tcp_input.c  | 16 +++++++++++-----
>     >  net/ipv4/tcp_output.c |  4 ++--
>     >  2 files changed, 13 insertions(+), 7 deletions(-)
>
>     Thanks, Larry. Just for context, can you please let us know whether
>     your tests included zero, one, or both of Eric's recent commits
>     (listed below) that tuned the number of ACKs after ECN events? (Or
>     maybe the tests were literally using a net-next kernel?) Just wanted
>     to get a better handle on any possible interactions there.
>
>     Thanks!
>
>     neal
>
> Yes, my test kernel includes both patches listed below.

OK, great.

> BTW, I will send a new
> patch where I move the call to tcp_incr_quickack from tcp_ecn_check_ce to tcp_ecn_accept_cwr.

OK, sounds good to me.

thanks,
neal

^ permalink raw reply

* Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301
From: Mark Brown @ 2018-07-03 15:31 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Steve deRosier, Matthias Brugger, Jiri Pirko, Ben Whitten, netdev,
	Marcel Holtmann, Dollar Chen, linux-kernel, Michael Röder,
	Janus Piwek, linux-spi, LoRa_Community_Support, Jian-Hong Pan,
	Ken Yu, David S . Miller, linux-arm-kernel
In-Reply-To: <809fc546-6012-37f3-9485-cfd8eaf99488@suse.de>


[-- Attachment #1.1: Type: text/plain, Size: 2006 bytes --]

On Tue, Jul 03, 2018 at 05:09:38PM +0200, Andreas Färber wrote:
> Am 03.07.2018 um 16:50 schrieb Mark Brown:

> >> 2) This SPI device is in turn exposing the two SPI masters that you
> >> already found below, and I didn't see a sane way to split that code out
> >> into drivers/spi/, so it's in drivers/net/lora/ here - has there been
> >> any precedence either way?

> > A MFD?

> I know of mfd, but how would the the the net vs. spi pieces interact
> then? Some functions would need to be exported then or is there an
> easier way without needing to set a cross-module API in stone?

It's an in-kernel ABI it's not exactly set in stone but yeah, you'll
need some interface.  A lot of devices work by having the children know
that they're part of a MFD and fish things out of the parent device,
either the pdata or (in the common case where the MFD bit mostly just
instantiates subdevices and holds a regmap) with dev_get_regmap().

> > A register map would work just as well here, we already have plenty of
> > devices that abstract at this level (most obviously the I2C/SPI devices
> > that use it to offer both interfaces with a single core driver).

> The address and data registers together form a two-byte SPI message!

> It is transmitted by writing to the CS register.

> The received data is afterwards available in another register.

Right, but it seems from the code that the hardware understands that
it's formatting register I/O and not just shifting in and out a byte
stream which is what a SPI controller does.  I'd not be surprised to
learn that the register you're calling a chip select register is a
strobe that initiates the transfer (and that this may be some of the
difficulty you're having with handling it in the way the framework
expects), the pattern with writing 1 followed immediately by 0 is a bit
of a flag here.

I've seen such before hardware where I know it was intentionally
designed that way so it wouldn't be totally surprising.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: recvmsg bug not clear
From: Randy Dunlap @ 2018-07-03 16:18 UTC (permalink / raw)
  To: 積丹尼 Dan Jacobson, linux-kernel,
	netdev@vger.kernel.org, Eric Dumazet
In-Reply-To: <87601whg6r.fsf@jidanni.org>

[adding netdev]

On 07/03/18 01:33, 積丹尼 Dan Jacobson wrote:
> Most users won't even know what string from this to google on to find
> out what this is all about:
> 
> 
> Jul 03 13:57:20 jidanni7 kernel: ------------[ cut here ]------------
> Jul 03 13:57:20 jidanni7 kernel: recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0
> Jul 03 13:57:20 jidanni7 kernel: WARNING: CPU: 2 PID: 1501 at /build/linux-uwVqDp/linux-4.16.16/net/ipv4/tcp.c:1881 tcp_recvmsg+0x649/0xb90
> Jul 03 13:57:20 jidanni7 kernel: Modules linked in: nft_r...
> 
> 
> So please make clearer messages.

That's a message from net/ipv4/tcp.c


-- 
~Randy

^ permalink raw reply

* [PATCH net-next v3 2/2] tcp: ack immediately when a cwr packet arrives
From: Lawrence Brakmo @ 2018-07-03 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
	Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180703162615.314231-1-brakmo@fb.com>

We observed high 99 and 99.9% latencies when doing RPCs with DCTCP. The
problem is triggered when the last packet of a request arrives CE
marked. The reply will carry the ECE mark causing TCP to shrink its cwnd
to 1 (because there are no packets in flight). When the 1st packet of
the next request arrives, the ACK was sometimes delayed even though it
is CWR marked, adding up to 40ms to the RPC latency.

This patch insures that CWR makred data packets arriving will be acked
immediately.

Modified based on comments by Neal Cardwell <ncardwell@google.com>

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 net/ipv4/tcp_input.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2c5d70bc294e..23c2a43de8a4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -246,8 +246,15 @@ static void tcp_ecn_queue_cwr(struct tcp_sock *tp)
 
 static void tcp_ecn_accept_cwr(struct tcp_sock *tp, const struct sk_buff *skb)
 {
-	if (tcp_hdr(skb)->cwr)
+	if (tcp_hdr(skb)->cwr) {
 		tp->ecn_flags &= ~TCP_ECN_DEMAND_CWR;
+
+		/* If the sender is telling us it has entered CWR, then its
+		 * cwnd may be very low (even just 1 packet), so we should ACK
+		 * immediately.
+		 */
+		tcp_enter_quickack_mode((struct sock *)tp, 2);
+	}
 }
 
 static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp)
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next v3 0/2] tcp: fix high tail latencies in DCTCP
From: Lawrence Brakmo @ 2018-07-03 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
	Yuchung Cheng, Steve Ibanez, Eric Dumazet

When have observed high tail latencies when using DCTCP for RPCs as
compared to using Cubic. For example, in one setup there are 2 hosts
sending to a 3rd one, with each sender having 3 flows (1 stream,
1 1MB back-to-back RPCs and 1 10KB back-to-back RPCs). The following
table shows the 99% and 99.9% latencies for both Cubic and dctcp:

           Cubic 99%  Cubic 99.9%   dctcp 99%    dctcp 99.9%
1MB RPCs    2.6ms       5.5ms         43ms          208ms
10KB RPCs    1.1ms       1.3ms         53ms          212ms

Looking at tcpdump traces showed that there are two causes for the
latency.  

  1) RTOs caused by the receiver sending a dup ACK and not ACKing
     the last (and only) packet sent.
  2) Delaying ACKs when the sender has a cwnd of 1, so everything
     pauses for the duration of the delayed ACK.

The first patch fixes the cause of the dup ACKs, not updating DCTCP
state when an ACK that was initially delayed has been sent with a
data packet.

The second patch insures that an ACK is sent immediately when a
CWR marked packet arrives.

With the patches the latencies for DCTCP now look like:

           dctcp 99%  dctcp 99.9%
1MB RPCs    5.8ms       6.9ms
10KB RPCs    146us       203us

Note that while the 1MB RPCs tail latencies are higher than Cubic's,
the 10KB latencies are much smaller than Cubic's. These patches fix
issues on the receiver, but tcpdump traces indicate there is an
opportunity to also fix an issue at the sender that adds about 3ms
to the tail latencies.

The following trace shows the issue that tiggers an RTO (fixed by these patches):

   Host A sends the last packets of the request
   Host B receives them, and the last packet is marked with congestion (CE)
   Host B sends ACKs for packets not marked with congestion
   Host B sends data packet with reply and ACK for packet marked with
          congestion (TCP flag ECE)
   Host A receives ACKs with no ECE flag
   Host A receives data packet with ACK for the last packet of request
          and which has TCP ECE bit set
   Host A sends 1st data packet of the next request with TCP flag CWR
   Host B receives the packet (as seen in tcpdump at B), no CE flag
   Host B sends a dup ACK that also has the TCP ECE flag
   Host A RTO timer fires!
   Host A to send the next packet
   Host A receives an ACK for everything it has sent (i.e. Host B
          did receive 1st packet of request)
   Host A send more packets…

v2: Removed call to tcp_ca_event from tcp_send_ack since I added one in
    tcp_event_ack_sent. Based on Neal Cardwell <ncardwell@google.com>
    feedback.
    Modified tcp_ecn_check_ce (and renamed it tcp_ecn_check) instead of modifying
    tcp_ack_send_check to insure an ACK when cwr is received.
v3: Handling cwr in tcp_ecn_accept_cwr instead of in tcp_ecn_check.

[PATCH net-next v3 1/2] tcp: notify when a delayed ack is sent
[PATCH net-next v3 2/2] tcp: ack immediately when a cwr packet

 net/ipv4/tcp_input.c  | 9 ++++++++-
 net/ipv4/tcp_output.c | 4 ++--
 2 files changed, 10 insertions(+), 3 deletions(-)

^ permalink raw reply

* [PATCH net-next v3 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-03 16:26 UTC (permalink / raw)
  To: netdev
  Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
	Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180703162615.314231-1-brakmo@fb.com>

DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
notifications to keep track if it needs to send an ACK for packets that
were received with a particular ECN state but whose ACK was delayed.

Under some circumstances, for example when a delayed ACK is sent with a
data packet, DCTCP state was not being updated due to a lack of
notification that the previously delayed ACK was sent. As a result, it
would sometimes send a duplicate ACK when a new data packet arrived.

This patch insures that DCTCP's state is correctly updated so it will
not send the duplicate ACK.

Improved based on comments from Neal Cardwell <ncardwell@google.com>.

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 net/ipv4/tcp_output.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f8f6129160dd..acefb64e8280 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -172,6 +172,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
 			__sock_put(sk);
 	}
 	tcp_dec_quickack_mode(sk, pkts);
+	if (inet_csk_ack_scheduled(sk))
+		tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
 	inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
 }
 
@@ -3567,8 +3569,6 @@ void tcp_send_ack(struct sock *sk)
 	if (sk->sk_state == TCP_CLOSE)
 		return;
 
-	tcp_ca_event(sk, CA_EVENT_NON_DELAYED_ACK);
-
 	/* We are not putting this on the write queue, so
 	 * tcp_transmit_skb() will set the ownership to this
 	 * sock.
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v4 08/18] net: davinci_emac: potentially get the MAC address from MTD
From: Florian Fainelli @ 2018-07-03 16:39 UTC (permalink / raw)
  To: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, Russell King,
	Grygorii Strashko, David S . Miller, Srinivas Kandagatla,
	Lukas Wunner, Rob Herring, Dan Carpenter, Ivan Khoronzhuk,
	David Lechner, Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
  Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
	Bartosz Golaszewski
In-Reply-To: <20180629094039.7543-9-brgl@bgdev.pl>



On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> On da850-evm board we can read the MAC address from MTD. It's currently
> done in the relevant board file, but we want to get rid of all the MAC
> reading callbacks from the board file (SPI and NAND). Move the reading
> of the MAC address from SPI to the emac driver's probe function.

This should be made something generic to all drivers, not just something
the davinci_emac driver does, something like this actually:

https://lkml.org/lkml/2018/3/24/312

> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/net/ethernet/ti/davinci_emac.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> index a1a6445b5a7e..48e6a7755811 100644
> --- a/drivers/net/ethernet/ti/davinci_emac.c
> +++ b/drivers/net/ethernet/ti/davinci_emac.c
> @@ -67,7 +67,7 @@
>  #include <linux/of_irq.h>
>  #include <linux/of_net.h>
>  #include <linux/mfd/syscon.h>
> -
> +#include <linux/mtd/mtd.h>
>  #include <asm/irq.h>
>  #include <asm/page.h>
>  
> @@ -1783,7 +1783,10 @@ static int davinci_emac_probe(struct platform_device *pdev)
>  	struct cpdma_params dma_params;
>  	struct clk *emac_clk;
>  	unsigned long emac_bus_frequency;
> -
> +#ifdef CONFIG_MTD
> +	size_t mac_addr_len;
> +	struct mtd_info *mtd;
> +#endif /* CONFIG_MTD */
>  
>  	/* obtain emac clock from kernel */
>  	emac_clk = devm_clk_get(&pdev->dev, NULL);
> @@ -1815,6 +1818,19 @@ static int davinci_emac_probe(struct platform_device *pdev)
>  		goto err_free_netdev;
>  	}
>  
> +#ifdef CONFIG_MTD
> +	mtd = get_mtd_device_nm("MAC-Address");
> +	if (!IS_ERR(mtd)) {
> +		rc = mtd_read(mtd, 0, ETH_ALEN,
> +			      &mac_addr_len, priv->mac_addr);
> +		if (rc == 0)
> +			dev_info(&pdev->dev,
> +				 "Read MAC addr from SPI Flash: %pM\n",
> +				 priv->mac_addr);
> +		put_mtd_device(mtd);
> +	}
> +#endif /* CONFIG_MTD */
> +
>  	/* MAC addr and PHY mask , RMII enable info from platform_data */
>  	memcpy(priv->mac_addr, pdata->mac_addr, ETH_ALEN);
>  	priv->phy_id = pdata->phy_id;
> 

-- 
Florian

^ permalink raw reply

* Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301
From: Andreas Färber @ 2018-07-03 16:40 UTC (permalink / raw)
  To: Mark Brown
  Cc: netdev, linux-arm-kernel, linux-kernel, Jian-Hong Pan, Jiri Pirko,
	Marcel Holtmann, David S . Miller, Matthias Brugger, Janus Piwek,
	Michael Röder, Dollar Chen, Ken Yu, Ben Whitten,
	Steve deRosier, linux-spi, LoRa_Community_Support, Hasnain Virk
In-Reply-To: <20180703153137.GB13744@sirena.org.uk>

Am 03.07.2018 um 17:31 schrieb Mark Brown:
> On Tue, Jul 03, 2018 at 05:09:38PM +0200, Andreas Färber wrote:
>> Am 03.07.2018 um 16:50 schrieb Mark Brown:
>>> A register map would work just as well here, we already have plenty of
>>> devices that abstract at this level (most obviously the I2C/SPI devices
>>> that use it to offer both interfaces with a single core driver).
> 
>> The address and data registers together form a two-byte SPI message!
> 
>> It is transmitted by writing to the CS register.
> 
>> The received data is afterwards available in another register.
> 
> Right, but it seems from the code that the hardware understands that
> it's formatting register I/O and not just shifting in and out a byte
> stream which is what a SPI controller does.  I'd not be surprised to
> learn that the register you're calling a chip select register is a
> strobe that initiates the transfer (and that this may be some of the
> difficulty you're having with handling it in the way the framework
> expects), the pattern with writing 1 followed immediately by 0 is a bit
> of a flag here.

Yeah, the current implementation assumes exactly that. :)

> I've seen such before hardware where I know it was intentionally
> designed that way so it wouldn't be totally surprising.

If we don't implement a spi_controller here, then IIUC we can't have
multiple spi_device implementations for the devices on the receiving
end, as they rely on a spi_controller for their APIs.

Do you have an alternative solution for abstraction? A regmap would seem
to require putting everything into a monolithic SX1301 driver despite
those connected chipsets actually being regular, external SPI chips that
could also be attached to non-SX1301 SPI masters.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply

* Re: [PATCH net-next 01/10] r8169: add basic phylib support
From: Florian Fainelli @ 2018-07-03 16:42 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org
In-Reply-To: <238e0c62-7e95-df62-3837-5e8d1b7248f0@gmail.com>



On 07/02/2018 02:15 PM, Heiner Kallweit wrote:
> On 02.07.2018 23:02, Andrew Lunn wrote:
>>> +static int r8169_mdio_read_reg(struct mii_bus *mii_bus, int phyaddr, int phyreg)
>>> +{
>>> +	struct rtl8169_private *tp = mii_bus->priv;
>>> +
>>> +	return rtl_readphy(tp, phyreg);
>>
>> So there is no support for phyaddr?
>>
> Right, the chip can access only the one internal PHY, therefore it
> doesn't support phyaddr.

Then you might also want to set mii_bus->phy_mask accordingly such that
only the internal PHY address bit is cleared there?
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 02/10] r8169: use phy_resume/phy_suspend
From: Florian Fainelli @ 2018-07-03 16:44 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org
In-Reply-To: <8ef120d3-ac4a-7ffd-2104-76d6f1268105@gmail.com>



On 07/02/2018 02:24 PM, Heiner Kallweit wrote:
> On 02.07.2018 23:06, Andrew Lunn wrote:
>>>  static void r8168_pll_power_down(struct rtl8169_private *tp)
>>>  {
>>>  	if (r8168_check_dash(tp))
>>> @@ -4510,7 +4469,8 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
>>>  	if (rtl_wol_pll_power_down(tp))
>>>  		return;
>>>  
>>> -	r8168_phy_power_down(tp);
>>> +	/* cover the case that PHY isn't connected */
>>> +	phy_suspend(mdiobus_get_phy(tp->mii_bus, 0));
>>
>> This could do some more explanation. Why would it not be connected?
>>
> The PHY gets connected when the net_device is opened. If a network
> port isn't used then it will be runtime-suspended a few seconds after
> boot. In this case we call r8168_pll_power_down() with the PHY not
> being connected. 

Would not the !netif_running() check in rtl8169_net_suspend() take care
of that though?
-- 
Florian

^ permalink raw reply

* Re: [PATCH v4 08/18] net: davinci_emac: potentially get the MAC address from MTD
From: David Lechner @ 2018-07-03 16:47 UTC (permalink / raw)
  To: Florian Fainelli, Bartosz Golaszewski, Sekhar Nori, Kevin Hilman,
	Russell King, Grygorii Strashko, David S . Miller,
	Srinivas Kandagatla, Lukas Wunner, Rob Herring, Dan Carpenter,
	Ivan Khoronzhuk, Greg Kroah-Hartman, Andrew Lunn, Jonathan Corbet
  Cc: linux-arm-kernel, linux-kernel, linux-omap, netdev,
	Bartosz Golaszewski
In-Reply-To: <03b77e24-9ab9-fa01-2387-9de0408a9942@gmail.com>

On 07/03/2018 11:39 AM, Florian Fainelli wrote:
> 
> 
> On 06/29/2018 02:40 AM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> On da850-evm board we can read the MAC address from MTD. It's currently
>> done in the relevant board file, but we want to get rid of all the MAC
>> reading callbacks from the board file (SPI and NAND). Move the reading
>> of the MAC address from SPI to the emac driver's probe function.
> 
> This should be made something generic to all drivers, not just something
> the davinci_emac driver does, something like this actually:
> 
> https://lkml.org/lkml/2018/3/24/312
> 

I was thinking about suggesting adding a nvmem provider for MTD devices
as well. It would fix the kernel config dependency problems I was running
into since nvmem lookups can do -EPROBE_DEFER.

^ permalink raw reply

* Re: [PATCH net-next 07/10] r8169: migrate speed_down function to phylib
From: Florian Fainelli @ 2018-07-03 16:48 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org
In-Reply-To: <8d516fa8-30a5-df7e-737c-119d6e458d80@gmail.com>



On 07/02/2018 02:31 PM, Heiner Kallweit wrote:
> On 02.07.2018 23:20, Andrew Lunn wrote:
>>  On Mon, Jul 02, 2018 at 09:37:08PM +0200, Heiner Kallweit wrote:
>>> Change rtl_speed_down() to use phylib.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/net/ethernet/realtek/r8169.c | 33 +++++++++++++---------------
>>>  1 file changed, 15 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
>>> index 311321ee..807fbc75 100644
>>> --- a/drivers/net/ethernet/realtek/r8169.c
>>> +++ b/drivers/net/ethernet/realtek/r8169.c
>>> @@ -4240,6 +4240,10 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp)
>>>  		rtl_writephy(tp, 0x0b, 0x0000); //w 0x0b 15 0 0
>>>  	}
>>>  
>>> +	/* We may have called rtl_speed_down before */
>>> +	dev->phydev->advertising = dev->phydev->supported;
>>> +	genphy_config_aneg(dev->phydev);
>>> +
>>>  	genphy_soft_reset(dev->phydev);
>>>  
>>>  	rtl8169_set_speed(dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
>>> @@ -4323,28 +4327,21 @@ static void rtl_init_mdio_ops(struct rtl8169_private *tp)
>>>  	}
>>>  }
>>>  
>>> +#define BASET10		(ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full)
>>> +#define BASET100	(ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full)
>>> +#define BASET1000	(ADVERTISED_1000baseT_Half | ADVERTISED_1000baseT_Full)
>>> +
>>>  static void rtl_speed_down(struct rtl8169_private *tp)
>>>  {
>>> -	u32 adv;
>>> -	int lpa;
>>> +	struct phy_device *phydev = tp->dev->phydev;
>>> +	u32 adv = phydev->lp_advertising & phydev->supported;
>>>  
>>> -	rtl_writephy(tp, 0x1f, 0x0000);
>>> -	lpa = rtl_readphy(tp, MII_LPA);
>>> +	if (adv & BASET10)
>>> +		phydev->advertising &= ~(BASET100 | BASET1000);
>>> +	else if (adv & BASET100)
>>> +		phydev->advertising &= ~BASET1000;
>>>  
>>> -	if (lpa & (LPA_10HALF | LPA_10FULL))
>>> -		adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full;
>>> -	else if (lpa & (LPA_100HALF | LPA_100FULL))
>>> -		adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
>>> -		      ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full;
>>> -	else
>>> -		adv = ADVERTISED_10baseT_Half | ADVERTISED_10baseT_Full |
>>> -		      ADVERTISED_100baseT_Half | ADVERTISED_100baseT_Full |
>>> -		      (tp->mii.supports_gmii ?
>>> -		       ADVERTISED_1000baseT_Half |
>>> -		       ADVERTISED_1000baseT_Full : 0);
>>> -
>>> -	rtl8169_set_speed(tp->dev, AUTONEG_ENABLE, SPEED_1000, DUPLEX_FULL,
>>> -			  adv);
>>> +	genphy_config_aneg(phydev);
>>>  }
>>
>> It probably it is me being too tired, but i don't get what this is
>> doing? Changing the local advertisement based on what the remote is
>> advertising. Why?
>>
> It also took me some time to understand what this speed_down is doing.
> If we suspend and wait for a WoL packet, then we don't have to burn all
> the energy for a GBit connection. Therefore we switch to the lowest
> speed supported by chip and link partner. This is done by removing
> higher speeds from the advertised modes and restarting an autonego.

This is something that the tg3 driver also does, we should probably
consider doing this as part of a generic PHY library helpers since I was
told by several HW engineers that usually 10Mbits for WoL is much more
energy efficient.

One thing that bothers me a bit is that this should ideally be offered
as both blocking and non-blocking options, because we might want to make
sure that at the time we suspend, and we already had a link established,
we successfully re-negotiate the link with the partner. I agree that
there could be any sort of link disruption happening at any point though..
-- 
Florian

^ permalink raw reply

* Re: [GIT] Networking
From: Linus Torvalds @ 2018-07-03 17:03 UTC (permalink / raw)
  To: Ursula Braun
  Cc: David Miller, Andrew Morton, Network Development,
	Linux Kernel Mailing List
In-Reply-To: <5b5509f1-9713-f1bc-cb33-18ebdb1cb8c5@linux.ibm.com>

On Tue, Jul 3, 2018 at 6:52 AM Ursula Braun <ubraun@linux.ibm.com> wrote:
>
> Thanks Linus for your revert. I run a test, and found it is almost fine.
> Just these 2 lines are missing:

I should have realized that. I actually looked at it and decided your
other changes made that unnecessary, but that was a thinko on my part.

Fixed up. Thanks,

               Linus

^ permalink raw reply

* Re: [RFC PATCH 00/11] OVS eBPF datapath.
From: Alexei Starovoitov @ 2018-07-03 17:56 UTC (permalink / raw)
  To: William Tu
  Cc: <dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org>,
	Tom Herbert via iovisor-dev, Linux Kernel Network Developers
In-Reply-To: <CALDO+SYzDDpTmJttghfjUYKbo3AHDaT4L154Acwn5BGqkytkHQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Jun 28, 2018 at 07:19:35AM -0700, William Tu wrote:
> Hi Alexei,
> 
> Thanks a lot for the feedback!
> 
> On Wed, Jun 27, 2018 at 8:00 PM, Alexei Starovoitov
> <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Sat, Jun 23, 2018 at 05:16:32AM -0700, William Tu wrote:
> >>
> >> Discussion
> >> ==========
> >> We are still actively working on finishing the feature, currently
> >> the basic forwarding and tunnel feature work, but still under
> >> heavy debugging and development.  The purpose of this RFC is to
> >> get some early feedbacks and direction for finishing the complete
> >> features in existing kernel's OVS datapath (the net/openvswitch/*).
> >
> > Thank you for sharing the patches.
> >
> >> Three major issues we are worried:
> >>   a. Megaflow support in BPF.
> >>   b. Connection Tracking support in BPF.
> >
> > my opinion on the above two didn't change.
> > To recap:
> > A. Non scalable megaflow map is no go. I'd like to see packet classification
> > algorithm like hicuts or efficuts to be implemented instead, since it can be
> > shared by generic bpf, bpftiler, ovs and likely others.
> 
> We did try the decision tree approach using dpdk's acl lib. The lookup
> speed is 6 times faster than the magaflow using tuple space.
> However, the update/insertion requires rebuilding/re-balancing the decision
> tree so it's way too slow. I think hicuts or efficuts suffers the same issue.
> So decision tree algos are scalable only for lookup operation due to its
> optimization over tree depth, but not scalable under
> update/insert/delete operations.
> 
> On customer's system we see megaflow update/insert rate around 10 rules/sec,
> this makes decision tree unusable, unless we invent something to optimize the
> update/insert time or incremental update of these decision tree algo.

is this a typo? you probably meant 10K rule updates a second ?
Last time I've dealt with these algorithms we had 100K acl updates a second.
It was an important metric that we were optimizing for.
I'm pretty sure '*cuts' algos do many thousands per second non optimized.

> >>   c. Verifier limitation.
> >
> > Not sure what limitations you're concerned about.
> >
> 
> Mostly related to stack.  The flow key OVS uses (struct sw_flow_key)
> is 464 byte. We trim a lot, now around 300 byte, but still huge, considering
> the BPF's stack limit is 512 byte.

have you tried using per-cpu array of one element with large value
instead of stack?
In the latest verifier most of the operations that can be done with the stack
pointer can be done with pointer to map value too.


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#1358): https://lists.iovisor.org/g/iovisor-dev/message/1358
Mute This Topic: https://lists.iovisor.org/mt/22656941/1132507
Group Owner: iovisor-dev+owner-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org
Unsubscribe: https://lists.iovisor.org/g/iovisor-dev/unsub  [glki-iovisor-dev@m.gmane.org]
-=-=-=-=-=-=-=-=-=-=-=-

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-03 18:38 UTC (permalink / raw)
  To: Neal Cardwell, Yuchung Cheng
  Cc: Netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
	Steve Ibanez, Eric Dumazet, Daniel Borkmann
In-Reply-To: <CADVnQykD8i68w8zySXv1tnVnrYYiGC9oVRMk70xeC9mE9PtnfQ@mail.gmail.com>

On 7/3/18, 6:15 AM, "Neal Cardwell" <ncardwell@google.com> wrote:

    On Mon, Jul 2, 2018 at 7:49 PM Yuchung Cheng <ycheng@google.com> wrote:
    >
    > On Mon, Jul 2, 2018 at 2:39 PM, Lawrence Brakmo <brakmo@fb.com> wrote:
    > >
    > > DCTCP depends on the CA_EVENT_NON_DELAYED_ACK and CA_EVENT_DELAYED_ACK
    > > notifications to keep track if it needs to send an ACK for packets that
    > > were received with a particular ECN state but whose ACK was delayed.
    > >
    > > Under some circumstances, for example when a delayed ACK is sent with a
    > > data packet, DCTCP state was not being updated due to a lack of
    > > notification that the previously delayed ACK was sent. As a result, it
    > > would sometimes send a duplicate ACK when a new data packet arrived.
    > >
    > > This patch insures that DCTCP's state is correctly updated so it will
    > > not send the duplicate ACK.
    > Sorry to chime-in late here (lame excuse: IETF deadline)
    >
    > IIRC this issue would exist prior to 4.11 kernel. While it'd be good
    > to fix that, it's not clear which patch introduced the regression
    > between 4.11 and 4.16? I assume you tested Eric's most recent quickack
    > fix.
    
    I don't think Larry is saying there's a regression between 4.11 and
    4.16. His recent "tcp: force cwnd at least 2 in tcp_cwnd_reduction"
    patch here:
    
      https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_935233_&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=pq_Mqvzfy-C8ltkgyx1u_g&m=7ko5C_ln2b7gZvz2A_UrZzz0AlcnhrW7-9KRahj_PEA&s=HcpZvo1TulN4-Y7Jhba5KM1MIaPwnBC95T8pLZfJESI&e=
    
    said that 4.11 was bad (high tail latency and lots of RTOs) and 4.16
    was still bad but with different netstat counters (no RTOs but still
    high tail latency):
    
    """
    On 4.11, pcap traces indicate that in some instances the 1st packet of
    the RPC is received but no ACK is sent before the packet is
    retransmitted. On 4.11 netstat shows TCP timeouts, with some of them
    spurious.
    
    On 4.16, we don't see retransmits in netstat but the high tail latencies
    are still there.
    """
    
    I suspect the RTOs disappeared but latencies remained too high because
    between 4.11 and 4.16 we introduced:
      tcp: allow TLP in ECN CWR
      https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=b4f70c3d4ec32a2ff4c62e1e2da0da5f55fe12bd
    
    So the RTOs probably disappeared because this commit turned them into
    TLPs. But the latencies remained high because the fundamental bug
    remained throughout 4.11 and 4.16 and today: the DCTCP use of
    tcp_send_ack() with an old rcv_nxt caused delayed ACKs to be cancelled
    when they should not have been.
    
    > In terms of the fix itself, it seems odd the tcp_send_ack() call in
    > DCTCP generates NON_DELAYED_ACK event to toggle DCTCP's
    > delayed_ack_reserved bit. Shouldn't the fix to have DCTCP send the
    > "prior" ACK w/o cancelling delayed-ACK and mis-recording that it's
    > cancelled, because that prior-ACK really is a supplementary old ACK.
    
    This patch is fixing an issue that's orthogonal to the one you are
    talking about. Using the taxonomy from our team's internal discussion
    yesterday, the issue you mention where the DCTCP "prior" ACK is
    cancelling delayed ACKs is issue (4); the issue that this particular
    "tcp: notify when a delayed ack is sent" patch from Larry fixes is
    issue (3). It's a bit tricky because both issues appeared in Larry's
    trace summary and packetdrill script to reproduce the issue.
    
    neal

I was able to track the patch that introduced the problem:

    commit 3759824da87b30ce7a35b4873b62b0ba38905ef5
    Author: Yuchung Cheng <ycheng@google.com>
    Date:   Wed Jul 1 14:11:15 2015 -0700

        tcp: PRR uses CRB mode by default and SS mode conditionally

I tested a kernel which reverted the relevant change (see diff below) and the high tail latencies of more than 40ms disappeared. However, the 10KB high percentile latencies are 4ms vs. less than 200us with my patches. It looks like the patch above ended up reducing the cwnd to 1 in the scenarios that were triggering the high tail latencies. That is, it increased the likelihood of triggering actual bugs in the network stack code that my patch set fixes.


diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2c5d70bc294e..50fabb07d739 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2468,13 +2468,14 @@ void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int flag)
                u64 dividend = (u64)tp->snd_ssthresh * tp->prr_delivered +
                               tp->prior_cwnd - 1;
                sndcnt = div_u64(dividend, tp->prior_cwnd) - tp->prr_out;
-       } else if ((flag & FLAG_RETRANS_DATA_ACKED) &&
-                  !(flag & FLAG_LOST_RETRANS)) {
+       } else {
+//     } else if ((flag & FLAG_RETRANS_DATA_ACKED) &&
+//                !(flag & FLAG_LOST_RETRANS)) {
                sndcnt = min_t(int, delta,
                               max_t(int, tp->prr_delivered - tp->prr_out,
                                     newly_acked_sacked) + 1);
-       } else {
-               sndcnt = min(delta, newly_acked_sacked);
+//     } else {
+//             sndcnt = min(delta, newly_acked_sacked);
        }
        /* Force a fast retransmit upon entering fast recovery */
        sndcnt = max(sndcnt, (tp->prr_out ? 0 : 1));

    

^ permalink raw reply related

* Re: [Intel-wired-lan] [jkirsher/next-queue PATCH v2 0/7] Add support for L2 Fwd Offload w/o ndo_select_queue
From: Jeff Kirsher @ 2018-07-03 18:42 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: intel-wired-lan, netdev
In-Reply-To: <20180612151322.86792.97587.stgit@ahduyck-green-test.jf.intel.com>

On Tue, Jun 12, 2018 at 8:18 AM, Alexander Duyck
<alexander.h.duyck@intel.com> wrote:
> This patch series is meant to allow support for the L2 forward offload, aka
> MACVLAN offload without the need for using ndo_select_queue.
>
> The existing solution currently requires that we use ndo_select_queue in
> the transmit path if we want to associate specific Tx queues with a given
> MACVLAN interface. In order to get away from this we need to repurpose the
> tc_to_txq array and XPS pointer for the MACVLAN interface and use those as
> a means of accessing the queues on the lower device. As a result we cannot
> offload a device that is configured as multiqueue, however it doesn't
> really make sense to configure a macvlan interfaced as being multiqueue
> anyway since it doesn't really have a qdisc of its own in the first place.
>
> I am submitting this as an RFC for the netdev mailing list, and officially
> submitting it for testing to Jeff Kirsher's next-queue in order to validate
> the ixgbe specific bits.
>
> The big changes in this set are:
>   Allow lower device to update tc_to_txq and XPS map of offloaded MACVLAN
>   Disable XPS for single queue devices
>   Replace accel_priv with sb_dev in ndo_select_queue
>   Add sb_dev parameter to fallback function for ndo_select_queue
>   Consolidated ndo_select_queue functions that appeared to be duplicates
>
> v2: Implement generic "select_queue" functions instead of "fallback" functions.
>     Tweak last two patches to account for changes in dev_pick_tx_xxx functions.
>
> ---
>
> Alexander Duyck (7):
>       net-sysfs: Drop support for XPS and traffic_class on single queue device
>       net: Add support for subordinate device traffic classes
>       ixgbe: Add code to populate and use macvlan tc to Tx queue map
>       net: Add support for subordinate traffic classes to netdev_pick_tx
>       net: Add generic ndo_select_queue functions
>       net: allow ndo_select_queue to pass netdev
>       net: allow fallback function to pass netdev
>

Alex, there were recent changes to Dave's net-next which caused a
conflict with one or more of your patches.  So I have removed this
series from my tree for now, and will work with you on updating the
series to work with the latest net-next tree.

^ permalink raw reply

* Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
From: David Ahern @ 2018-07-03 19:23 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
In-Reply-To: <CADvbK_ciwsQuJ3Ep0_oFzmFSAPGMB9vjB=KXZ=feG5X1M=oZnw@mail.gmail.com>

On 7/3/18 5:36 AM, Xin Long wrote:
> On Mon, Jul 2, 2018 at 11:12 PM, David Ahern <dsahern@gmail.com> wrote:
>> On 7/2/18 12:30 AM, Xin Long wrote:
>>> +ping_ipv4()
>>> +{
>>> +     sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>>> +     bc_forwarding_disable
>>> +     ping_test $h1 198.51.100.255
>>> +
>>> +     iptables -A INPUT -i vrf-r1 -p icmp -j DROP
>>> +     bc_forwarding_restore
>>> +     bc_forwarding_enable
>>> +     ping_test $h1 198.51.100.255
>>> +
>>> +     bc_forwarding_restore
>>> +     iptables -D INPUT -i vrf-r1 -p icmp -j DROP
>>> +     sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>>> +}
>>
>> Both tests fail for me:
>> TEST: ping                                              [FAIL]
>> TEST: ping                                              [FAIL]
> I think 'ip vrf exec ...' is not working in your env, while
> the testing is using "ip vrf exec vrf-h1 ping ..."
> 
> You can test it by:
> # ip link add dev vrf-test type vrf table 1111
> # ip vrf exec vrf-test ls

well, that's embarrassing. yes, I updated ip and forgot to apply the bpf
workaround to define the syscall number (not defined in jessie).

> 
>>
>> Why the need for the iptables rule?
> This iptables rule is to block the echo_request packet going to
> route's local_in.
> When bc_forwarding is NOT doing forwarding well but the packet
> goes to the route's local_in, it will fail.
> 
> Without this rule, the 2nd ping will always succeed, we can't tell the
> echo_reply is from route or h2.
> 
> Or you have a better way to test this?

your commands are not a proper test. The test should succeed and fail
based on the routing lookup, not iptables rules.

> 
>>
>> And, PAUSE_ON_FAIL is not working to take a look at why tests are
>> failing. e.g.,
>>
>> PAUSE_ON_FAIL=yes ./router_broadcast.sh
>>
>> just continues on. Might be something with the infrastructure scripts.
> Yes, in ./router_broadcast.sh, it loads lib.sh where it loads forwarding.config
> where it has "PAUSE_ON_FAIL=no", which would override your
> "PAUSE_ON_FAIL=yes".
> 

ack. bit by that as well.

^ permalink raw reply

* Re: [PATCH net-next 01/10] r8169: add basic phylib support
From: Heiner Kallweit @ 2018-07-03 19:48 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org
In-Reply-To: <143f31fa-e8ea-7e79-46a7-b9a6d3ac68ef@gmail.com>

On 03.07.2018 18:42, Florian Fainelli wrote:
> 
> 
> On 07/02/2018 02:15 PM, Heiner Kallweit wrote:
>> On 02.07.2018 23:02, Andrew Lunn wrote:
>>>> +static int r8169_mdio_read_reg(struct mii_bus *mii_bus, int phyaddr, int phyreg)
>>>> +{
>>>> +	struct rtl8169_private *tp = mii_bus->priv;
>>>> +
>>>> +	return rtl_readphy(tp, phyreg);
>>>
>>> So there is no support for phyaddr?
>>>
>> Right, the chip can access only the one internal PHY, therefore it
>> doesn't support phyaddr.
> 
> Then you might also want to set mii_bus->phy_mask accordingly such that
> only the internal PHY address bit is cleared there?
> 
That's something I'm doing already, see following line in r8169_mdio_register():
new_bus->phy_mask = ~1;
But thanks for the hint anyway.

^ permalink raw reply

* Re: [PATCH net-next 02/10] r8169: use phy_resume/phy_suspend
From: Heiner Kallweit @ 2018-07-03 19:54 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: David Miller, Realtek linux nic maintainers,
	netdev@vger.kernel.org
In-Reply-To: <e8a9e121-9b5d-2e45-2547-148a782e6751@gmail.com>

On 03.07.2018 18:44, Florian Fainelli wrote:
> 
> 
> On 07/02/2018 02:24 PM, Heiner Kallweit wrote:
>> On 02.07.2018 23:06, Andrew Lunn wrote:
>>>>  static void r8168_pll_power_down(struct rtl8169_private *tp)
>>>>  {
>>>>  	if (r8168_check_dash(tp))
>>>> @@ -4510,7 +4469,8 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
>>>>  	if (rtl_wol_pll_power_down(tp))
>>>>  		return;
>>>>  
>>>> -	r8168_phy_power_down(tp);
>>>> +	/* cover the case that PHY isn't connected */
>>>> +	phy_suspend(mdiobus_get_phy(tp->mii_bus, 0));
>>>
>>> This could do some more explanation. Why would it not be connected?
>>>
>> The PHY gets connected when the net_device is opened. If a network
>> port isn't used then it will be runtime-suspended a few seconds after
>> boot. In this case we call r8168_pll_power_down() with the PHY not
>> being connected. 
> 
> Would not the !netif_running() check in rtl8169_net_suspend() take care
> of that though?
> 
We don't come that far ..
If the interface is down then tp->TxDescArray is NULL. Means we call
rtl_pll_power_down() in rtl8169_runtime_suspend() before reaching
rtl8169_net_suspend().

^ permalink raw reply

* Re: [PATCH net] net/ipv6: Revert attempt to simplify route replace and append
From: David Ahern @ 2018-07-03 20:02 UTC (permalink / raw)
  To: Ido Schimmel, dsahern; +Cc: netdev, sharpd, Thomas.Winter, petrm
In-Reply-To: <20180703134302.GB15120@splinter.mtl.com>

On 7/3/18 7:43 AM, Ido Schimmel wrote:
> On Mon, Jul 02, 2018 at 03:03:12PM -0700, dsahern@kernel.org wrote:
>> From: David Ahern <dsahern@gmail.com>
>>
>> NetworkManager likes to manage linklocal prefix routes and does so with
>> the NLM_F_APPEND flag, breaking attempts to simplify the IPv6 route
>> code and by extension enable multipath routes with device only nexthops.
>>
>> Revert f34436a43092 and its followup
>> 6eba08c3626b ("ipv6: Only emit append events for appended routes").
>> Update the test cases to reflect the old behavior.
>>
>> Fixes: f34436a43092 ("net/ipv6: Simplify route replace and appending into multipath route")
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>> Ido: I left 5a15a1b07c51. FIB_EVENT_ENTRY_APPEND is not generated for
>>      IPv6, so no harm in leaving it.
> 
> OK, but I had a follow-up series that did further changes in mlxsw
> (merge commit eab9a2d5f323) to support the new behavior. You want to
> revert it and squash to v2 or I'll send another revert?

I'll squash. The ability to add dev only nexthops is clearly needed.
Reverting in 1 patch will help on the next attempt.

It is unfortunate that mlxsw has to replicate the node lookup code.

> 
> Also, Petr added a multipath test that relies on IPv6 device only
> nexthops. See commit 54818c4c4b937.
> 

ok. I'll remove the ipv6 mpath tests as well.

^ permalink raw reply

* Re: [PATCH bpf] bpf: hash_map: decrement counter on error
From: Alexei Starovoitov @ 2018-07-03 20:28 UTC (permalink / raw)
  To: Mauricio Vasquez; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev
In-Reply-To: <5fba5067-1ba0-6ab1-21fd-ad6945cb886d@polito.it>

On Sun, Jul 01, 2018 at 11:33:58AM -0500, Mauricio Vasquez wrote:
> 
> On 06/30/2018 06:20 PM, Daniel Borkmann wrote:
> > On 06/29/2018 02:48 PM, Mauricio Vasquez B wrote:
> > > Decrement the number of elements in the map in case the allocation
> > > of a new node fails.
> > > 
> > > Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> > Thanks for the fix, Mauricio!
> > 
> > Could you reply with a Fixes: tag in order to track the commit originally
> > introducing this bug?
> > 
> > Thanks,
> > Daniel
> > 
> 
> Sure Daniel,
> 
> Fixes: 6c9059817432 ("bpf: pre-allocate hash map elements")

Good catch. Thanks for the fix.
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* [PATCH net] net: phy: fix flag masking in __set_phy_supported
From: Heiner Kallweit @ 2018-07-03 20:34 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org

Currently also the pause flags are removed from phydev->supported because
they're not included in PHY_DEFAULT_FEATURES. I don't think this is
intended, especially when considering that this function can be called
via phy_set_max_speed() anywhere in a driver. Change the masking to mask
out only the values we're going to change. In addition remove the
misleading comment, job of this small function is just to adjust the
supported and advertised speeds.

Fixes: f3a6bd393c2c ("phylib: Add phy_set_max_speed helper")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index bd0f339f..b9f5f40a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1724,11 +1724,8 @@ EXPORT_SYMBOL(genphy_loopback);
 
 static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 {
-	/* The default values for phydev->supported are provided by the PHY
-	 * driver "features" member, we want to reset to sane defaults first
-	 * before supporting higher speeds.
-	 */
-	phydev->supported &= PHY_DEFAULT_FEATURES;
+	phydev->supported &= ~(PHY_1000BT_FEATURES | PHY_100BT_FEATURES |
+			       PHY_10BT_FEATURES);
 
 	switch (max_speed) {
 	default:
-- 
2.18.0

^ permalink raw reply related

* [PATCH] isdn: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2018-07-03 21:17 UTC (permalink / raw)
  To: Paul Bolle, Karsten Keil
  Cc: gigaset307x-common, netdev, linux-kernel, Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.

Warning level 2 was used: -Wimplicit-fallthrough=2

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/isdn/gigaset/bas-gigaset.c          | 3 +++
 drivers/isdn/hardware/mISDN/avmfritz.c      | 1 +
 drivers/isdn/hardware/mISDN/hfcpci.c        | 1 +
 drivers/isdn/hardware/mISDN/mISDNinfineon.c | 1 +
 drivers/isdn/hardware/mISDN/mISDNisar.c     | 4 ++++
 drivers/isdn/hisax/avm_pci.c                | 1 +
 drivers/isdn/hisax/callc.c                  | 1 +
 drivers/isdn/hisax/config.c                 | 1 +
 drivers/isdn/hisax/gazel.c                  | 4 ++++
 drivers/isdn/hisax/isar.c                   | 2 ++
 drivers/isdn/hisax/l3_1tr6.c                | 1 +
 drivers/isdn/hisax/l3dss1.c                 | 1 +
 drivers/isdn/hysdn/hysdn_boot.c             | 2 ++
 drivers/isdn/i4l/isdn_v110.c                | 9 +++++++++
 drivers/isdn/mISDN/stack.c                  | 1 +
 15 files changed, 33 insertions(+)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 40c1411..ecdeb89 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -739,6 +739,7 @@ static void read_int_callback(struct urb *urb)
 
 	case HD_OPEN_B2CHANNEL_ACK:
 		++channel;
+		/* fall through */
 	case HD_OPEN_B1CHANNEL_ACK:
 		bcs = cs->bcs + channel;
 		update_basstate(ucs, BS_B1OPEN << channel, 0);
@@ -752,6 +753,7 @@ static void read_int_callback(struct urb *urb)
 
 	case HD_CLOSE_B2CHANNEL_ACK:
 		++channel;
+		/* fall through */
 	case HD_CLOSE_B1CHANNEL_ACK:
 		bcs = cs->bcs + channel;
 		update_basstate(ucs, 0, BS_B1OPEN << channel);
@@ -765,6 +767,7 @@ static void read_int_callback(struct urb *urb)
 
 	case HD_B2_FLOW_CONTROL:
 		++channel;
+		/* fall through */
 	case HD_B1_FLOW_CONTROL:
 		bcs = cs->bcs + channel;
 		atomic_add((l - BAS_NORMFRAME) * BAS_CORRFRAMES,
diff --git a/drivers/isdn/hardware/mISDN/avmfritz.c b/drivers/isdn/hardware/mISDN/avmfritz.c
index ae2b266..8eb28a8 100644
--- a/drivers/isdn/hardware/mISDN/avmfritz.c
+++ b/drivers/isdn/hardware/mISDN/avmfritz.c
@@ -361,6 +361,7 @@ modehdlc(struct bchannel *bch, int protocol)
 	switch (protocol) {
 	case -1: /* used for init */
 		bch->state = -1;
+		/* fall through */
 	case ISDN_P_NONE:
 		if (bch->state == ISDN_P_NONE)
 			break;
diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c
index 34c9387..72a271b 100644
--- a/drivers/isdn/hardware/mISDN/hfcpci.c
+++ b/drivers/isdn/hardware/mISDN/hfcpci.c
@@ -1296,6 +1296,7 @@ mode_hfcpci(struct bchannel *bch, int bc, int protocol)
 	case (-1): /* used for init */
 		bch->state = -1;
 		bch->nr = bc;
+		/* fall through */
 	case (ISDN_P_NONE):
 		if (bch->state == ISDN_P_NONE)
 			return 0;
diff --git a/drivers/isdn/hardware/mISDN/mISDNinfineon.c b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
index 1fc2906..3e01012 100644
--- a/drivers/isdn/hardware/mISDN/mISDNinfineon.c
+++ b/drivers/isdn/hardware/mISDN/mISDNinfineon.c
@@ -887,6 +887,7 @@ release_card(struct inf_hw *card) {
 				release_card(card->sc[i]);
 			card->sc[i] = NULL;
 		}
+		/* fall through */
 	default:
 		pci_disable_device(card->pdev);
 		pci_set_drvdata(card->pdev, NULL);
diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c
index b791688..386731e 100644
--- a/drivers/isdn/hardware/mISDN/mISDNisar.c
+++ b/drivers/isdn/hardware/mISDN/mISDNisar.c
@@ -972,6 +972,7 @@ isar_pump_statev_fax(struct isar_ch *ch, u8 devt) {
 				break;
 			case PCTRL_CMD_FTM:
 				p1 = 2;
+				/* fall through */
 			case PCTRL_CMD_FTH:
 				send_mbox(ch->is, dps | ISAR_HIS_PUMPCTRL,
 					  PCTRL_CMD_SILON, 1, &p1);
@@ -1177,6 +1178,7 @@ setup_pump(struct isar_ch *ch) {
 			send_mbox(ch->is, dps | ISAR_HIS_PUMPCFG,
 				  PMOD_DTMF, 1, param);
 		}
+		/* fall through */
 	case ISDN_P_B_MODEM_ASYNC:
 		ctrl = PMOD_DATAMODEM;
 		if (test_bit(FLG_ORIGIN, &ch->bch.Flags)) {
@@ -1268,6 +1270,7 @@ setup_iom2(struct isar_ch *ch) {
 	case ISDN_P_B_MODEM_ASYNC:
 	case ISDN_P_B_T30_FAX:
 		cmsb |= IOM_CTRL_RCV;
+		/* fall through */
 	case ISDN_P_B_L2DTMF:
 		if (test_bit(FLG_DTMFSEND, &ch->bch.Flags))
 			cmsb |= IOM_CTRL_RCV;
@@ -1560,6 +1563,7 @@ isar_l2l1(struct mISDNchannel *ch, struct sk_buff *skb)
 				ich->is->name, hh->id);
 			ret = -EINVAL;
 		}
+		/* fall through */
 	default:
 		pr_info("%s: %s unknown prim(%x,%x)\n",
 			ich->is->name, __func__, hh->prim, hh->id);
diff --git a/drivers/isdn/hisax/avm_pci.c b/drivers/isdn/hisax/avm_pci.c
index a18b605..b161456 100644
--- a/drivers/isdn/hisax/avm_pci.c
+++ b/drivers/isdn/hisax/avm_pci.c
@@ -207,6 +207,7 @@ modehdlc(struct BCState *bcs, int mode, int bc)
 		bcs->mode = 1;
 		bcs->channel = bc;
 		bc = 0;
+		/* fall through */
 	case (L1_MODE_NULL):
 		if (bcs->mode == L1_MODE_NULL)
 			return;
diff --git a/drivers/isdn/hisax/callc.c b/drivers/isdn/hisax/callc.c
index ddec47a..5f43783 100644
--- a/drivers/isdn/hisax/callc.c
+++ b/drivers/isdn/hisax/callc.c
@@ -1369,6 +1369,7 @@ leased_l1l2(struct PStack *st, int pr, void *arg)
 	case (PH_ACTIVATE | INDICATION):
 	case (PH_ACTIVATE | CONFIRM):
 		event = EV_LEASED;
+		/* fall through */
 	case (PH_DEACTIVATE | INDICATION):
 	case (PH_DEACTIVATE | CONFIRM):
 		if (test_bit(FLG_TWO_DCHAN, &chanp->cs->HW_Flags))
diff --git a/drivers/isdn/hisax/config.c b/drivers/isdn/hisax/config.c
index 7108bdb8..fcc9c46 100644
--- a/drivers/isdn/hisax/config.c
+++ b/drivers/isdn/hisax/config.c
@@ -1843,6 +1843,7 @@ static void hisax_b_l2l1(struct PStack *st, int pr, void *arg)
 	case PH_DEACTIVATE | REQUEST:
 		test_and_clear_bit(BC_FLG_BUSY, &bcs->Flag);
 		skb_queue_purge(&bcs->squeue);
+		/* fall through */
 	default:
 		B_L2L1(b_if, pr, arg);
 		break;
diff --git a/drivers/isdn/hisax/gazel.c b/drivers/isdn/hisax/gazel.c
index 35c6df6..a6d8af0 100644
--- a/drivers/isdn/hisax/gazel.c
+++ b/drivers/isdn/hisax/gazel.c
@@ -108,6 +108,7 @@ ReadISAC(struct IsdnCardState *cs, u_char offset)
 	switch (cs->subtyp) {
 	case R647:
 		off2 = ((off2 << 8 & 0xf000) | (off2 & 0xf));
+		/* fall through */
 	case R685:
 		return (readreg(cs->hw.gazel.isac, off2));
 	case R753:
@@ -125,6 +126,7 @@ WriteISAC(struct IsdnCardState *cs, u_char offset, u_char value)
 	switch (cs->subtyp) {
 	case R647:
 		off2 = ((off2 << 8 & 0xf000) | (off2 & 0xf));
+		/* fall through */
 	case R685:
 		writereg(cs->hw.gazel.isac, off2, value);
 		break;
@@ -203,6 +205,7 @@ ReadHSCX(struct IsdnCardState *cs, int hscx, u_char offset)
 	switch (cs->subtyp) {
 	case R647:
 		off2 = ((off2 << 8 & 0xf000) | (off2 & 0xf));
+		/* fall through */
 	case R685:
 		return (readreg(cs->hw.gazel.hscx[hscx], off2));
 	case R753:
@@ -220,6 +223,7 @@ WriteHSCX(struct IsdnCardState *cs, int hscx, u_char offset, u_char value)
 	switch (cs->subtyp) {
 	case R647:
 		off2 = ((off2 << 8 & 0xf000) | (off2 & 0xf));
+		/* fall through */
 	case R685:
 		writereg(cs->hw.gazel.hscx[hscx], off2, value);
 		break;
diff --git a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c
index d01ff11..82c1879 100644
--- a/drivers/isdn/hisax/isar.c
+++ b/drivers/isdn/hisax/isar.c
@@ -1089,6 +1089,7 @@ isar_pump_statev_fax(struct BCState *bcs, u_char devt) {
 				break;
 			case PCTRL_CMD_FTM:
 				p1 = 2;
+				/* fall through */
 			case PCTRL_CMD_FTH:
 				sendmsg(cs, dps | ISAR_HIS_PUMPCTRL,
 					PCTRL_CMD_SILON, 1, &p1);
@@ -1097,6 +1098,7 @@ isar_pump_statev_fax(struct BCState *bcs, u_char devt) {
 			case PCTRL_CMD_FRM:
 				if (frm_extra_delay)
 					mdelay(frm_extra_delay);
+				/* fall through */
 			case PCTRL_CMD_FRH:
 				p1 = bcs->hw.isar.mod = bcs->hw.isar.newmod;
 				bcs->hw.isar.newmod = 0;
diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
index da0a1c6..98f60d1 100644
--- a/drivers/isdn/hisax/l3_1tr6.c
+++ b/drivers/isdn/hisax/l3_1tr6.c
@@ -88,6 +88,7 @@ l3_1tr6_setup_req(struct l3_process *pc, u_char pr, void *arg)
 			break;
 		case 'C':
 			channel = 0x08;
+			/* fall through */
 		case 'P':
 			channel |= 0x80;
 			teln++;
diff --git a/drivers/isdn/hisax/l3dss1.c b/drivers/isdn/hisax/l3dss1.c
index 18a3484..368d152 100644
--- a/drivers/isdn/hisax/l3dss1.c
+++ b/drivers/isdn/hisax/l3dss1.c
@@ -1282,6 +1282,7 @@ l3dss1_setup_req(struct l3_process *pc, u_char pr,
 			switch (0x5f & *teln) {
 			case 'C':
 				channel = 0x08;
+				/* fall through */
 			case 'P':
 				channel |= 0x80;
 				teln++;
diff --git a/drivers/isdn/hysdn/hysdn_boot.c b/drivers/isdn/hysdn/hysdn_boot.c
index 4a04253..ba177c3 100644
--- a/drivers/isdn/hysdn/hysdn_boot.c
+++ b/drivers/isdn/hysdn/hysdn_boot.c
@@ -99,6 +99,7 @@ pof_handle_data(hysdn_card *card, int datlen)
 
 	case TAG_CBOOTDTA:
 		DecryptBuf(boot, datlen);	/* we need to encrypt the buffer */
+		/* fall through */
 	case TAG_BOOTDTA:
 		if (card->debug_flags & LOG_POF_RECORD)
 			hysdn_addlog(card, "POF got %s len=%d offs=0x%lx",
@@ -137,6 +138,7 @@ pof_handle_data(hysdn_card *card, int datlen)
 
 	case TAG_CABSDATA:
 		DecryptBuf(boot, datlen);	/* we need to encrypt the buffer */
+		/* fall through */
 	case TAG_ABSDATA:
 		if (card->debug_flags & LOG_POF_RECORD)
 			hysdn_addlog(card, "POF got %s len=%d offs=0x%lx",
diff --git a/drivers/isdn/i4l/isdn_v110.c b/drivers/isdn/i4l/isdn_v110.c
index 8b74ce4..2a5f666 100644
--- a/drivers/isdn/i4l/isdn_v110.c
+++ b/drivers/isdn/i4l/isdn_v110.c
@@ -354,6 +354,7 @@ EncodeMatrix(unsigned char *buf, int len, unsigned char *m, int mlen)
 				printk(KERN_WARNING "isdn_v110 (EncodeMatrix): buffer full!\n");
 				return line;
 			}
+			/* else: fall through */
 		case 128:
 			m[line] = 128;	/* leftmost -> set byte to 1000000 */
 			mbit = 64;	/* current bit in the matrix line */
@@ -386,20 +387,28 @@ EncodeMatrix(unsigned char *buf, int len, unsigned char *m, int mlen)
 		switch (++line % 10) {
 		case 1:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 2:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 3:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 4:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 5:
 			m[line++] = 0xbf;
+			/* fall through */
 		case 6:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 7:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 8:
 			m[line++] = 0xfe;
+			/* fall through */
 		case 9:
 			m[line++] = 0xfe;
 		}
diff --git a/drivers/isdn/mISDN/stack.c b/drivers/isdn/mISDN/stack.c
index 422dced..d97c6dd 100644
--- a/drivers/isdn/mISDN/stack.c
+++ b/drivers/isdn/mISDN/stack.c
@@ -539,6 +539,7 @@ create_l2entity(struct mISDNdevice *dev, struct mISDNchannel *ch,
 		rq.protocol = ISDN_P_NT_S0;
 		if (dev->Dprotocols & (1 << ISDN_P_NT_E1))
 			rq.protocol = ISDN_P_NT_E1;
+		/* fall through */
 	case ISDN_P_LAPD_TE:
 		ch->recv = mISDN_queue_message;
 		ch->peer = &dev->D.st->own;
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox