* Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301
From: Andreas Färber @ 2018-07-03 3:21 UTC (permalink / raw)
To: Ben Whitten
Cc: Mark Brown, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Jian-Hong Pan, Jiri Pirko,
Marcel Holtmann, David S . Miller, Matthias Brugger, Janus Piwek,
Michael Röder, Dollar Chen, Ken Yu, Steve deRosier,
linux-spi@vger.kernel.org, LoRa_Community_Support@semtech.com
In-Reply-To: <BY1PR02MB111481D94EB0D68AD5FCA689E7430@BY1PR02MB1114.namprd02.prod.outlook.com>
Hi Ben,
Am 02.07.2018 um 22:43 schrieb Ben Whitten:
>> 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?
>
> In my work in progress driver I just register one controller for the sx1301 with two chip selects and use the chip select information to choose the correct radio to send to, this is based on the DT reg information. No need to register two separate masters.
I had considered that and discarded it. The SX1301 has not just two CS
registers though but also two pairs of addr, data registers. That speaks
for two masters with a single chip-select each, unless I'm
misunderstanding the meaning of the registers.
>> Am 02.07.2018 um 18:12 schrieb Mark Brown:
>>> On Sun, Jul 01, 2018 at 01:08:04PM +0200, Andreas Färber wrote:
>>>
>>>> +static void sx1301_radio_spi_set_cs(struct spi_device *spi, bool enable)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + dev_dbg(&spi->dev, "setting SPI CS to %s\n", enable ? "1" : "0");
>>>> +
>>>> + if (enable)
>>>> + return;
>>>> +
>>>> + ret = sx1301_radio_set_cs(spi->controller, enable);
>>>> + if (ret)
>>>> + dev_warn(&spi->dev, "failed to write CS (%d)\n", ret);
>>>> +}
>>>
>>> So we never disable chip select?
>>
>> Not here, I instead did that in transfer_one below.
>>
>> Unfortunately there seems to be no documentation, only reference code:
>>
>> https://github.com/Lora-
>> net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L121
>> https://github.com/Lora-
>> net/lora_gateway/blob/master/libloragw/src/loragw_radio.c#L165
>>
>> It sets CS to 0 before writing to address and data registers, then
>> immediately sets CS to 1 and back to 0 before reading or ending the
>> write transaction. I've tried to force the same behavior in this driver.
>> My guess was that CS is high-active during the short 1-0 cycle, because
>> if it's low-active during the register writes then why the heck is it
>> set to 0 again in the end instead of keeping at 1... confusing.
>>
>> Maybe the Semtech folks CC'ed can comment how these registers work?
>>
>>>> + if (tx_buf) {
>>>> + ret = sx1301_write(ssx->parent, ssx->regs +
>> REG_RADIO_X_ADDR, tx_buf ? tx_buf[0] : 0);
>>>
>>> This looks confused. We're in an if (tx_buf) block but there's a use of
>>> the ternery operator that appears to be checking if we have a tx_buf?
>>
>> Yeah, as mentioned this RFC is not ready for merging - checkpatch.pl
>> will complain about lines too long, and TODOs are sprinkled all over or
>> not even mentioned. It's a Proof of Concept that a net_device could work
>> for a wide range of spi and serdev based drivers, and on top this device
>> has more than one channel, which may influence network-level design
>> discussions.
>>
>> That said, I'll happily drop the second check. Thanks for spotting!
>>
>>>> + if (ret) {
>>>> + dev_err(&spi->dev, "SPI radio address write
>> failed\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = sx1301_write(ssx->parent, ssx->regs +
>> REG_RADIO_X_DATA, (tx_buf && xfr->len >= 2) ? tx_buf[1] : 0);
>>>> + if (ret) {
>>>> + dev_err(&spi->dev, "SPI radio data write failed\n");
>>>> + return ret;
>>>> + }
>>>
>>> This looks awfully like you're coming in at the wrong abstraction layer
>>> and the hardware actually implements a register abstraction rather than
>>> a SPI one so you should be using regmap as the abstraction.
>>
>> I don't understand. Ben has suggested using regmap for the SPI _device_
>> that we're talking to, which may be a good idea. But this SX1301 device
>> in turn has two SPI _masters_ talking to an SX125x slave each. I don't
>> see how using regmap instead of my wrappers avoids this spi_controller?
>> The whole point of this spi_controller is to abstract and separate the
>> SX1255 vs. SX1257 vs. whatever-radio-attached into a separate driver,
>> instead of mixing it into the SX1301 driver - to me that looks cleaner
>> and more extensible. It also has the side-effect that we could configure
>> the two radios via DT (frequencies, clk output, etc.).
>
> You want an SPI controller in the SX1301 as the down stream radios are SPI and could be attached directly to a host SPI bus, makes sense to have one radio driver and talk through the SX1301.
> But you should use the regmap to access the SX1301 master controller registers.
> Example I use with one SPI master and some clock info:
> eg:
> sx1301: sx1301@0 {
Node names should not repeat the chipset, that goes into compatible.
lora-concentrator@0?
> compatible = "semtech,sx1301";
> reg = <0>;
> #address-cells = <1>;
> #size-cells = <0>;
I would still find it cleaner to have (a) sub-node(s) for the radios.
> spi-max-frequency = <8000000>;
Datasheet says 10 MHz, why 8 MHz?
> gpios-reset = <&pioA 26 GPIO_ACTIVE_HIGH>;
reset-gpios?
> clocks = <&radio1 0>, <&clkhs 0>;
> clock-names = "clk32m", "clkhs";
>
> radio0: sx1257@0 {
lora@0?
> compatible = "semtech,sx125x";
No wildcards in bindings please, use concrete "semtech,sx1257".
> reg = <0>;
> spi-max-frequency = <8000000>;
Datasheet says 10 ns - I reported to Semtech that it should probably say
10 MHz, too.
> tx;
Might we configure that on the sx1301 instead?
> clocks = <&tcxo 0>;
> clock-names = "tcxo";
> };
>
> radio1: sx1257@1 {
> compatible = "semtech,sx125x";
> reg = <1>;
> spi-max-frequency = <8000000>;
> #clock-cells = <0>;
> clocks = <&tcxo 0>;
> clock-names = "tcxo";
> clock-output-names = "clk32m";
> };
> };
[snip]
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] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jonathan Maxwell @ 2018-07-03 3:15 UTC (permalink / raw)
To: Neal Cardwell
Cc: David Miller, Eric Dumazet, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Netdev, LKML, Jon Maxwell
In-Reply-To: <CADVnQynNGbmBvheBdzzr5K8BVUXewzr61ZNsTz-mteE3eGUm3g@mail.gmail.com>
On Tue, Jul 3, 2018 at 1:00 PM, Neal Cardwell <ncardwell@google.com> wrote:
> On Mon, Jul 2, 2018 at 9:18 PM Jon Maxwell <jmaxwell37@gmail.com> wrote:
>>
>> Every time the TCP retransmission timer fires. It checks to see if there is a
>> timeout before scheduling the next retransmit timer. The retransmit interval
>> between each retransmission increases exponentially. The issue is that in order
>> for the timeout to occur the retransmit timer needs to fire again. If the user
>> timeout check happens after the 9th retransmit for example. It needs to wait for
>> the 10th retransmit timer to fire in order to evaluate whether a timeout has
>> occurred or not. If the interval is large enough then the timeout will be
>> inaccurate.
>>
>> For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
>>
>> 1st retransmit:
>>
>> 22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
>>
>> Last retransmit:
>>
>> 22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
>>
>> Timeout:
>>
>> send: Connection timed out
>> Sun Jul 1 22:25:34 EDT 2018
>>
>> We can see that last retransmit took ~7 seconds. Which pushed the total
>> timeout to ~15 seconds instead of the expected 10 seconds. This gets more
>> inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
>>
>> Fix this by recalculating the last retransmit interval so that it fires when
>> the timeout should occur. Only implement when icsk->icsk_user_timeout is set.
>>
>> Test results with the patch is the expected 10 second timeout:
>>
>> 1st retransmit:
>>
>> 01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
>>
>> Last retransmit:
>>
>> 01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
>>
>> Timeout:
>>
>> send: Connection timed out
>> Mon Jul 2 01:38:09 EDT 2018
>>
>> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
>> ---
>> net/ipv4/tcp_timer.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>> index 3b3611729928..94491a481722 100644
>> --- a/net/ipv4/tcp_timer.c
>> +++ b/net/ipv4/tcp_timer.c
>> @@ -407,6 +407,7 @@ void tcp_retransmit_timer(struct sock *sk)
>> struct tcp_sock *tp = tcp_sk(sk);
>> struct net *net = sock_net(sk);
>> struct inet_connection_sock *icsk = inet_csk(sk);
>> + __u32 time_remaining = 0;
>>
>> if (tp->fastopen_rsk) {
>> WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
>> @@ -535,6 +536,12 @@ void tcp_retransmit_timer(struct sock *sk)
>> /* Use normal (exponential) backoff */
>> icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
>> }
>> + if (icsk->icsk_user_timeout) {
>> + time_remaining = jiffies_to_msecs(icsk->icsk_user_timeout) -
>> + (tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp);
>> + if (time_remaining < icsk->icsk_rto)
>> + icsk->icsk_rto = time_remaining;
>> + }
>
> Thanks, a more precise user timeout sounds nice. A couple thoughts:
>
> (a) The icsk->icsk_rto is in jiffies, and the time_remaining is in
> msecs, so it looks like there is a units mismatch here in the
> comparisons and assignment.
>
> (b) It also seems like the time_remaining could be negative, because
> (a) the icsk_user_timeout is not involved in the baseline RTO
> calculation, so that perhaps the first RTO to fire might be beyond the
> icsk_user_timeout AFAIK, and (b) if the machine is very busy then the
> timer handler can be delayed beyond the targeted icsk_user_timeout.
> But time_remaining is a __u32, and icsk->icsk_rto is also a __u32, so
> it seems like a negative number in time_remaining would usually be
> treated as a very large unsigned positive number in this comparison:
>
> + if (time_remaining < icsk->icsk_rto)
>
> (c) If the user timeout is changed between RTO expirations to push the
> user timeout further in the future, then it seems like this commit
> will have side effects that left the icsk->icsk_rto in a weird state
> that does not do the expected exponential backoff correctly.
>
> (d) There are also wrapping issues to watch out for, since the
> tcp_time_stamp(tcp_sk(sk)) and tcp_sk(sk)->retrans_stamp are
> milliseconds, which will wrap every 49 days or so. Seems like the code
> is OK in that respect.
>
> (e) It also might be nice to put this logic in a helper, rather than
> growing the body of tcp_retransmit_timer().
>
> What about something like (pseudocode):
>
> --
>
> static __u32 tcp_clamp_rto_to_user_timeout(sk):
> rto = icsk->icsk_rto;
> if (!icsk->icsk_user_timeout)
> return rto;
> elapsed = tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp;
> user_timeout = jiffies_to_msecs(icsk->icsk_user_timeout);
> if (elapsed >= user_timeout)
> rto = 1; /* user timeout has passed; fire ASAP */
> else
> rto = min(rto, msecs_to_jiffies(user_timeout - elapsed));
> return rto;
>
> tcp_retransmit_timer():
> ...
> rto = tcp_clamp_rto_to_user_timeout(sk);
> inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, rto, TCP_RTO_MAX);
>
Thanks Neal, that looks like a good idea. Let me test that out in my reproducer.
Regards
Jon
> --
>
> neal
^ permalink raw reply
* Re: [RFC net-next 15/15] net: lora: Add Semtech SX1301
From: Andreas Färber @ 2018-07-03 3:01 UTC (permalink / raw)
To: Ben Whitten
Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Jian-Hong Pan, Jiri Pirko,
Marcel Holtmann, David S . Miller, Matthias Brugger, Janus Piwek,
Michael Röder, Dollar Chen, Ken Yu, Steve deRosier,
Mark Brown, linux-spi@vger.kernel.org, Hasnain Virk,
LoRa_Community_Support
In-Reply-To: <BY1PR02MB11148FFC047AE7509DD811E0E7430@BY1PR02MB1114.namprd02.prod.outlook.com>
Hi Ben,
Am 02.07.2018 um 13:51 schrieb Ben Whitten:
> Excellent work on doing this I have also been working on and off
> this personally for some time.
Thanks. Colliding work is always unfortunate, I can relate...
> Have a look at my repository [1] for sx1301 and sx1257 drivers,
> I use regmaps capability of switching pages which should simplify
> your driver considerably, I also have a full register map and bit field.
Please note that my lora-next branch already has bug fixes and cleanups
over this patch. The probe error handling was broken, and I implemented
wrappers for paged reads and writes as well as burst modes, plus the
firmware loading.
https://github.com/afaerber/linux/commits/lora-next
I took a quick look at your sx1257 and noticed you licensed it as GPLv2.
Is there any particular reason for that? Since I wrote my driver without
copying from GPLv2 code, I prefer the less restrictive GPLv2+.
So far a work day has passed with no maintainer objecting to or
commenting on the underlying PF_LORA network layer design. Meanwhile
there's already three of us with code and more people have inquired
about testing and contributing, so I'm thinking about setting up a
staging tree on kernel.org to collaborate on...
Would you be willing to contribute your regmap ideas to my driver as a
patch to squash? Needs a Signed-off-by of course, which your GitHub
commits are lacking, so I can't merge them on my own.
> I have also been trying to use the clk framework to capture the various
> routing that the cards have.
I thought about clk too, but won't that cause name conflicts when
probing multiple concentrators? Would be nice to use that for
configuring the SX1257 clock output instead of my current hack.
Another thought I haven't investigated yet is whether we could use
remoteproc for ARB and AGC. I would at least prefer to have the firmware
as a binary loaded via the usual request_firmware(), not as byte array.
But then again the AGC gets firmware loaded twice, so maybe too complex
for remoteproc.
BTW do you have any insights on what MCU is in there? Would be nice to
understand in form of source code what the firmware is doing, to avoid
the hard dependency on a specific firmware version (imagine user
updating kernel-firmware - containing versions X,Y,Z - and kernel and
booting two different kernel versions, the older one stops working).
https://www.thethingsnetwork.org/forum/t/secret-price-of-a-lora-gateway/5730/74
Regards,
Andreas
> I will dig into this series this evening.
>
> [1] https://github.com/BWhitten/linux-stable/tree/971aadc8fdfe842020d912449bdd71b33d576fe3/drivers/net/lora
[...]
>> diff --git a/drivers/net/lora/sx1301.c b/drivers/net/lora/sx1301.c
>> new file mode 100644
>> index 000000000000..5c936c1116d1
>> --- /dev/null
>> +++ b/drivers/net/lora/sx1301.c
>> @@ -0,0 +1,446 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
[snip]
--
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] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Neal Cardwell @ 2018-07-03 3:00 UTC (permalink / raw)
To: jmaxwell37
Cc: David Miller, Eric Dumazet, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Netdev, LKML, jmaxwell
In-Reply-To: <20180703011726.8301-1-jmaxwell37@gmail.com>
On Mon, Jul 2, 2018 at 9:18 PM Jon Maxwell <jmaxwell37@gmail.com> wrote:
>
> Every time the TCP retransmission timer fires. It checks to see if there is a
> timeout before scheduling the next retransmit timer. The retransmit interval
> between each retransmission increases exponentially. The issue is that in order
> for the timeout to occur the retransmit timer needs to fire again. If the user
> timeout check happens after the 9th retransmit for example. It needs to wait for
> the 10th retransmit timer to fire in order to evaluate whether a timeout has
> occurred or not. If the interval is large enough then the timeout will be
> inaccurate.
>
> For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
>
> 1st retransmit:
>
> 22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
>
> Last retransmit:
>
> 22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
>
> Timeout:
>
> send: Connection timed out
> Sun Jul 1 22:25:34 EDT 2018
>
> We can see that last retransmit took ~7 seconds. Which pushed the total
> timeout to ~15 seconds instead of the expected 10 seconds. This gets more
> inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
>
> Fix this by recalculating the last retransmit interval so that it fires when
> the timeout should occur. Only implement when icsk->icsk_user_timeout is set.
>
> Test results with the patch is the expected 10 second timeout:
>
> 1st retransmit:
>
> 01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
>
> Last retransmit:
>
> 01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
>
> Timeout:
>
> send: Connection timed out
> Mon Jul 2 01:38:09 EDT 2018
>
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
> ---
> net/ipv4/tcp_timer.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 3b3611729928..94491a481722 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -407,6 +407,7 @@ void tcp_retransmit_timer(struct sock *sk)
> struct tcp_sock *tp = tcp_sk(sk);
> struct net *net = sock_net(sk);
> struct inet_connection_sock *icsk = inet_csk(sk);
> + __u32 time_remaining = 0;
>
> if (tp->fastopen_rsk) {
> WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
> @@ -535,6 +536,12 @@ void tcp_retransmit_timer(struct sock *sk)
> /* Use normal (exponential) backoff */
> icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
> }
> + if (icsk->icsk_user_timeout) {
> + time_remaining = jiffies_to_msecs(icsk->icsk_user_timeout) -
> + (tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp);
> + if (time_remaining < icsk->icsk_rto)
> + icsk->icsk_rto = time_remaining;
> + }
Thanks, a more precise user timeout sounds nice. A couple thoughts:
(a) The icsk->icsk_rto is in jiffies, and the time_remaining is in
msecs, so it looks like there is a units mismatch here in the
comparisons and assignment.
(b) It also seems like the time_remaining could be negative, because
(a) the icsk_user_timeout is not involved in the baseline RTO
calculation, so that perhaps the first RTO to fire might be beyond the
icsk_user_timeout AFAIK, and (b) if the machine is very busy then the
timer handler can be delayed beyond the targeted icsk_user_timeout.
But time_remaining is a __u32, and icsk->icsk_rto is also a __u32, so
it seems like a negative number in time_remaining would usually be
treated as a very large unsigned positive number in this comparison:
+ if (time_remaining < icsk->icsk_rto)
(c) If the user timeout is changed between RTO expirations to push the
user timeout further in the future, then it seems like this commit
will have side effects that left the icsk->icsk_rto in a weird state
that does not do the expected exponential backoff correctly.
(d) There are also wrapping issues to watch out for, since the
tcp_time_stamp(tcp_sk(sk)) and tcp_sk(sk)->retrans_stamp are
milliseconds, which will wrap every 49 days or so. Seems like the code
is OK in that respect.
(e) It also might be nice to put this logic in a helper, rather than
growing the body of tcp_retransmit_timer().
What about something like (pseudocode):
--
static __u32 tcp_clamp_rto_to_user_timeout(sk):
rto = icsk->icsk_rto;
if (!icsk->icsk_user_timeout)
return rto;
elapsed = tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp;
user_timeout = jiffies_to_msecs(icsk->icsk_user_timeout);
if (elapsed >= user_timeout)
rto = 1; /* user timeout has passed; fire ASAP */
else
rto = min(rto, msecs_to_jiffies(user_timeout - elapsed));
return rto;
tcp_retransmit_timer():
...
rto = tcp_clamp_rto_to_user_timeout(sk);
inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, rto, TCP_RTO_MAX);
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] net: phy: DP83TC811: Fix SGMII enable/disable
From: David Miller @ 2018-07-03 2:38 UTC (permalink / raw)
To: dmurphy; +Cc: andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <20180629153546.24107-2-dmurphy@ti.com>
From: Dan Murphy <dmurphy@ti.com>
Date: Fri, 29 Jun 2018 10:35:46 -0500
> If SGMII was selected in the DT then the device should
> write the SGMII enable bit.
>
> If SGMII is not selected in the DT then the SGMII bit
> should be disabled.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v2 - Removed erroneous defconfig addtion in the patch.
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] net: phy: DP83TC811: Add INT_STAT3
From: David Miller @ 2018-07-03 2:38 UTC (permalink / raw)
To: dmurphy; +Cc: andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <20180629153546.24107-1-dmurphy@ti.com>
From: Dan Murphy <dmurphy@ti.com>
Date: Fri, 29 Jun 2018 10:35:45 -0500
> Add INT_STAT3 interrupt setting and clearing
> support.
>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v2 - Removed bug fix removal of writing INT_STAT1 twice when disabling interrupts
Applied.
^ permalink raw reply
* Re: [PATCH 0/5] m68k: IO Fixes and Cleanups
From: Greg Ungerer @ 2018-07-03 2:15 UTC (permalink / raw)
To: Geert Uytterhoeven, David S . Miller, Dmitry Torokhov,
Helge Deller
Cc: linux-m68k, netdev, linux-input, linux-kernel
In-Reply-To: <20180702133532.5412-1-geert@linux-m68k.org>
Hi Geert,
On 02/07/18 23:35, Geert Uytterhoeven wrote:
> Hi all,
>
> This patch series contains fixes and cleanups for I/O accessors on m68k
> platforms (with MMU).
>
> The first patch contains small fixes without any dependencies.
> Patches 2 and 3 make small adjustments to drivers that are dependencies
> for further cleanup.
> Patch 4 and 5 complete the cleanup.
>
> Given the dependencies, I think it's easiest if the respective
> maintainers would provide their Acked-by, so all patches can go in
> through the m68k tree.
>
> Thanks for your comments!
I like it alot. If we can just fix up the warnings caused by patch
number 1 I am more than happy to ack.
Regards
Greg
> Geert Uytterhoeven (5):
> m68k/io: Add missing ioremap define guards, fix typo
> net: mac8390: Use standard memcpy_{from,to}io()
> Input: hilkbd - Add casts to HP9000/300 I/O accessors
> m68k/io: Move mem*io define guards to <asm/kmap.h>
> m68k/io: Switch mmu variant to <asm-generic/io.h>
>
> arch/m68k/include/asm/io.h | 7 +++++
> arch/m68k/include/asm/io_mm.h | 40 +++--------------------------
> arch/m68k/include/asm/io_no.h | 12 ---------
> arch/m68k/include/asm/kmap.h | 7 ++++-
> drivers/input/keyboard/hilkbd.c | 4 +--
> drivers/net/ethernet/8390/mac8390.c | 20 +++++++--------
> 6 files changed, 28 insertions(+), 62 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 1/5] m68k/io: Add missing ioremap define guards, fix typo
From: Greg Ungerer @ 2018-07-03 2:13 UTC (permalink / raw)
To: Geert Uytterhoeven, David S . Miller, Dmitry Torokhov,
Helge Deller
Cc: linux-m68k, netdev, linux-input, linux-kernel
In-Reply-To: <20180702133532.5412-2-geert@linux-m68k.org>
Hi Geert,
On 02/07/18 23:35, Geert Uytterhoeven wrote:
> - Add missing define guard for ioremap_wt(),
> - Fix typo s/ioremap_fillcache/ioremap_fullcache/,
> - Add define guard for iounmap() for consistency with other
> architectures.
>
> Fixes: 9746882f547d2f00 ("m68k: group io mapping definitions and functions")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
If I build for the m5475evb defconfig then I get warnings like this:
CC init/main.o
In file included from ./include/asm-generic/io.h:19:0,
from ./arch/m68k/include/asm/io_no.h:147,
from ./arch/m68k/include/asm/io.h:3,
from ./include/linux/bio.h:28,
from ./include/linux/blkdev.h:21,
from init/main.c:80:
./include/asm-generic/iomap.h:79:0: warning: "ioremap_wt" redefined
#define ioremap_wt ioremap_nocache
^
In file included from ./arch/m68k/include/asm/io_no.h:145:0,
from ./arch/m68k/include/asm/io.h:3,
from ./include/linux/bio.h:28,
from ./include/linux/blkdev.h:21,
from init/main.c:80:
./arch/m68k/include/asm/kmap.h:37:0: note: this is the location of the previous definition
#define ioremap_wt ioremap_wt
^
Regards
Greg
> ---
> arch/m68k/include/asm/kmap.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/include/asm/kmap.h b/arch/m68k/include/asm/kmap.h
> index 84b8333db8ad1987..bf1026def698f21f 100644
> --- a/arch/m68k/include/asm/kmap.h
> +++ b/arch/m68k/include/asm/kmap.h
> @@ -16,6 +16,7 @@
> */
> extern void __iomem *__ioremap(unsigned long physaddr, unsigned long size,
> int cacheflag);
> +#define iounmap iounmap
> extern void iounmap(void __iomem *addr);
> extern void __iounmap(void *addr, unsigned long size);
>
> @@ -33,13 +34,14 @@ static inline void __iomem *ioremap_nocache(unsigned long physaddr,
> }
>
> #define ioremap_uc ioremap_nocache
> +#define ioremap_wt ioremap_wt
> static inline void __iomem *ioremap_wt(unsigned long physaddr,
> unsigned long size)
> {
> return __ioremap(physaddr, size, IOMAP_WRITETHROUGH);
> }
>
> -#define ioremap_fillcache ioremap_fullcache
> +#define ioremap_fullcache ioremap_fullcache
> static inline void __iomem *ioremap_fullcache(unsigned long physaddr,
> unsigned long size)
> {
>
^ permalink raw reply
* Re: [PATCH net-next v4 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Jason Wang @ 2018-07-03 2:12 UTC (permalink / raw)
To: xiangxia.m.yue
Cc: mst, makita.toshiaki, virtualization, netdev, Tonghao Zhang
In-Reply-To: <1530536228-17462-4-git-send-email-xiangxia.m.yue@gmail.com>
On 2018年07月02日 20:57, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Factor out generic busy polling logic and will be
> used for in tx path in the next patch. And with the patch,
> qemu can set differently the busyloop_timeout for rx queue.
>
> Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
> ---
> drivers/vhost/net.c | 94 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 55 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 62bb8e8..2790959 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -429,6 +429,52 @@ static int vhost_net_enable_vq(struct vhost_net *n,
> return vhost_poll_start(poll, sock->file);
> }
>
> +static int sk_has_rx_data(struct sock *sk)
> +{
> + struct socket *sock = sk->sk_socket;
> +
> + if (sock->ops->peek_len)
> + return sock->ops->peek_len(sock);
> +
> + return skb_queue_empty(&sk->sk_receive_queue);
> +}
> +
> +static void vhost_net_busy_poll(struct vhost_net *net,
> + struct vhost_virtqueue *rvq,
> + struct vhost_virtqueue *tvq,
> + bool rx)
> +{
> + unsigned long uninitialized_var(endtime);
> + unsigned long busyloop_timeout;
> + struct socket *sock;
> + struct vhost_virtqueue *vq = rx ? tvq : rvq;
> +
> + mutex_lock_nested(&vq->mutex, rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> +
> + vhost_disable_notify(&net->dev, vq);
> + sock = rvq->private_data;
> + busyloop_timeout = rx ? rvq->busyloop_timeout : tvq->busyloop_timeout;
> +
> + preempt_disable();
> + endtime = busy_clock() + busyloop_timeout;
> + while (vhost_can_busy_poll(tvq->dev, endtime) &&
> + !(sock && sk_has_rx_data(sock->sk)) &&
> + vhost_vq_avail_empty(tvq->dev, tvq))
> + cpu_relax();
> + preempt_enable();
> +
> + if ((rx && !vhost_vq_avail_empty(&net->dev, vq)) ||
> + (!rx && (sock && sk_has_rx_data(sock->sk)))) {
> + vhost_poll_queue(&vq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
One last question, do we need this for rx? This check will be always
true under light or medium load.
Thanks
> + vhost_disable_notify(&net->dev, vq);
> + vhost_poll_queue(&vq->poll);
> + }
> +
> + mutex_unlock(&vq->mutex);
> +}
> +
> +
> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> struct vhost_virtqueue *vq,
> struct iovec iov[], unsigned int iov_size,
> @@ -621,16 +667,6 @@ static int peek_head_len(struct vhost_net_virtqueue *rvq, struct sock *sk)
> return len;
> }
>
> -static int sk_has_rx_data(struct sock *sk)
> -{
> - struct socket *sock = sk->sk_socket;
> -
> - if (sock->ops->peek_len)
> - return sock->ops->peek_len(sock);
> -
> - return skb_queue_empty(&sk->sk_receive_queue);
> -}
> -
> static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
> {
> struct vhost_virtqueue *vq = &nvq->vq;
> @@ -645,39 +681,19 @@ static void vhost_rx_signal_used(struct vhost_net_virtqueue *nvq)
>
> static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)
> {
> - struct vhost_net_virtqueue *rvq = &net->vqs[VHOST_NET_VQ_RX];
> - struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> - struct vhost_virtqueue *vq = &nvq->vq;
> - unsigned long uninitialized_var(endtime);
> - int len = peek_head_len(rvq, sk);
> + struct vhost_net_virtqueue *rnvq = &net->vqs[VHOST_NET_VQ_RX];
> + struct vhost_net_virtqueue *tnvq = &net->vqs[VHOST_NET_VQ_TX];
>
> - if (!len && vq->busyloop_timeout) {
> - /* Flush batched heads first */
> - vhost_rx_signal_used(rvq);
> - /* Both tx vq and rx socket were polled here */
> - mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> - vhost_disable_notify(&net->dev, vq);
> + int len = peek_head_len(rnvq, sk);
>
> - preempt_disable();
> - endtime = busy_clock() + vq->busyloop_timeout;
> -
> - while (vhost_can_busy_poll(&net->dev, endtime) &&
> - !sk_has_rx_data(sk) &&
> - vhost_vq_avail_empty(&net->dev, vq))
> - cpu_relax();
> -
> - preempt_enable();
> -
> - if (!vhost_vq_avail_empty(&net->dev, vq))
> - vhost_poll_queue(&vq->poll);
> - else if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> - vhost_disable_notify(&net->dev, vq);
> - vhost_poll_queue(&vq->poll);
> - }
> + if (!len && rnvq->vq.busyloop_timeout) {
> + /* Flush batched heads first */
> + vhost_rx_signal_used(rnvq);
>
> - mutex_unlock(&vq->mutex);
> + /* Both tx vq and rx socket were polled here */
> + vhost_net_busy_poll(net, &rnvq->vq, &tnvq->vq, true);
>
> - len = peek_head_len(rvq, sk);
> + len = peek_head_len(rnvq, sk);
> }
>
> return len;
^ permalink raw reply
* Re: [PATCHv2 net-next 0/5] sctp: fully support for dscp and flowlabel per transport
From: Marcelo Ricardo Leitner @ 2018-07-03 1:54 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem, hideaki.yoshifuji
In-Reply-To: <cover.1530526661.git.lucien.xin@gmail.com>
On Mon, Jul 02, 2018 at 06:21:10PM +0800, Xin Long wrote:
> Now dscp and flowlabel are set from sock when sending the packets,
> but being multi-homing, sctp also supports for dscp and flowlabel
> per transport, which is described in section 8.1.12 in RFC6458.
>
> v1->v2:
> - define ip_queue_xmit as inline in net/ip.h, instead of exporting
> it in Patch 1/5 according to David's suggestion.
> - fix the param len check in sctp_s/getsockopt_peer_addr_params()
> in Patch 3/5 to guarantee that an old app built with old kernel
> headers could work on the newer kernel per Marcelo's point.
>
> Xin Long (5):
> ipv4: add __ip_queue_xmit() that supports tos param
> sctp: add support for dscp and flowlabel per transport
> sctp: add spp_ipv6_flowlabel and spp_dscp for sctp_paddrparams
> sctp: add support for setting flowlabel when adding a transport
> sctp: check for ipv6_pinfo legal sndflow with flowlabel in
> sctp_v6_get_dst
Series
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
^ permalink raw reply
* Re: [PATCH v2] atmel: using strlcpy() to avoid possible buffer overflows
From: YueHaibing @ 2018-07-03 1:48 UTC (permalink / raw)
To: Kalle Valo
Cc: simon-xn1N/tgparsycpQjotevgVpr/1R2p/CL,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <8736x1snu9.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On 2018/7/2 22:40, Kalle Valo wrote:
> YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> writes:
>
>> 'firmware' is a module param which may been longer than firmware_id,
>> so using strlcpy() to guard against overflows. Also priv is allocated
>> with zeroed memory,no need to set firmware_id[0] to '\0'.
>>
>> v1 -> v2: remove priv->firmware_id[0] = '\0';
>>
>> Signed-off-by: YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/net/wireless/atmel/atmel.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> The changelog should be after "---" line. I can fix it this time, but in
> the future please use the correct location.
Thank you,will notice it.
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
>
^ permalink raw reply
* Re: [PATCH net] sctp: fix the issue that pathmtu may be set lower than MINSEGMENT
From: Marcelo Ricardo Leitner @ 2018-07-03 1:43 UTC (permalink / raw)
To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem, syzkaller
In-Reply-To: <20180702114512.GA15612@hmswarspite.think-freely.org>
On Mon, Jul 02, 2018 at 07:45:12AM -0400, Neil Horman wrote:
> On Mon, Jul 02, 2018 at 02:51:16PM +0800, Xin Long wrote:
> > After commit b6c5734db070 ("sctp: fix the handling of ICMP Frag Needed
> > for too small MTUs"), sctp_transport_update_pmtu would refetch pathmtu
> > from the dst and set it to transport's pathmtu without any check.
> >
> > The new pathmtu may be lower than MINSEGMENT if the dst is obsolete and
> > updated by .get_dst() in sctp_transport_update_pmtu.
In this case, it could have a smaller MTU as well, and thus we should
validate it against MINSEGMENT instead.
> >
> > Syzbot reported a warning in sctp_mtu_payload caused by this.
> >
> > This fix uses the refetched pathmtu only when it's greater than the
> > frag_needed pmtu.
> >
> > Fixes: b6c5734db070 ("sctp: fix the handling of ICMP Frag Needed for too small MTUs")
> > Reported-by: syzbot+f0d9d7cba052f9344b03@syzkaller.appspotmail.com
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> > net/sctp/transport.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> > index 445b7ef..ddfb687 100644
> > --- a/net/sctp/transport.c
> > +++ b/net/sctp/transport.c
> > @@ -282,7 +282,10 @@ bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
> >
> > if (dst) {
> > /* Re-fetch, as under layers may have a higher minimum size */
> > - pmtu = SCTP_TRUNC4(dst_mtu(dst));
> > + u32 mtu = SCTP_TRUNC4(dst_mtu(dst));
> > +
> > + if (pmtu < mtu)
> > + pmtu = mtu;
> nit, but why not u32 mtu = min(pmtu, SCTP_TRUNC4(dst_mtu(dst))) here ?
sctp_dst_mtu() is wrapping all that for us :)
- pmtu = SCTP_TRUNC4(dst_mtu(dst));
+ pmtu = sctp_dst_mtu(dst);
>
> Neil
>
> > change = t->pathmtu != pmtu;
> > }
> > t->pathmtu = pmtu;
> > --
> > 2.1.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH bpf v2 0/4] Bug fixes to the SKB TX path of AF_XDP
From: Alexei Starovoitov @ 2018-07-03 1:39 UTC (permalink / raw)
To: Magnus Karlsson
Cc: bjorn.topel, ast, daniel, netdev, eric.dumazet, liu.song.a23,
qi.z.zhang, pavel
In-Reply-To: <1530258500-9126-1-git-send-email-magnus.karlsson@intel.com>
On Fri, Jun 29, 2018 at 09:48:16AM +0200, Magnus Karlsson wrote:
> This patch set fixes three bugs in the SKB TX path of AF_XDP.
> Details in the individual commits.
>
> The structure of the patch set is as follows:
>
> Patch 1: Fix for lost completion message
> Patch 2-3: Fix for possible multiple completions of single packet
> Patch 4: Fix potential race during error
>
> Changes from v1:
>
> * Added explanation of race in commit message of patch 4.
Applied, Thanks
^ permalink raw reply
* [PATCH net-next] tcp: Improve setsockopt() TCP_USER_TIMEOUT accuracy
From: Jon Maxwell @ 2018-07-03 1:17 UTC (permalink / raw)
To: davem; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel, jmaxwell
Every time the TCP retransmission timer fires. It checks to see if there is a
timeout before scheduling the next retransmit timer. The retransmit interval
between each retransmission increases exponentially. The issue is that in order
for the timeout to occur the retransmit timer needs to fire again. If the user
timeout check happens after the 9th retransmit for example. It needs to wait for
the 10th retransmit timer to fire in order to evaluate whether a timeout has
occurred or not. If the interval is large enough then the timeout will be
inaccurate.
For example with a TCP_USER_TIMEOUT of 10 seconds without patch:
1st retransmit:
22:25:18.973488 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
22:25:26.205499 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Sun Jul 1 22:25:34 EDT 2018
We can see that last retransmit took ~7 seconds. Which pushed the total
timeout to ~15 seconds instead of the expected 10 seconds. This gets more
inaccurate the larger the TCP_USER_TIMEOUT value. As the interval increases.
Fix this by recalculating the last retransmit interval so that it fires when
the timeout should occur. Only implement when icsk->icsk_user_timeout is set.
Test results with the patch is the expected 10 second timeout:
1st retransmit:
01:37:59.022555 IP host1.49310 > host2.search-agent: Flags [.]
Last retransmit:
01:38:06.486558 IP host1.49310 > host2.search-agent: Flags [.]
Timeout:
send: Connection timed out
Mon Jul 2 01:38:09 EDT 2018
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 3b3611729928..94491a481722 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -407,6 +407,7 @@ void tcp_retransmit_timer(struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
struct net *net = sock_net(sk);
struct inet_connection_sock *icsk = inet_csk(sk);
+ __u32 time_remaining = 0;
if (tp->fastopen_rsk) {
WARN_ON_ONCE(sk->sk_state != TCP_SYN_RECV &&
@@ -535,6 +536,12 @@ void tcp_retransmit_timer(struct sock *sk)
/* Use normal (exponential) backoff */
icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
}
+ if (icsk->icsk_user_timeout) {
+ time_remaining = jiffies_to_msecs(icsk->icsk_user_timeout) -
+ (tcp_time_stamp(tcp_sk(sk)) - tcp_sk(sk)->retrans_stamp);
+ if (time_remaining < icsk->icsk_rto)
+ icsk->icsk_rto = time_remaining;
+ }
inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
__sk_dst_reset(sk);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next v2 0/2] tcp: fix high tail latencies in DCTCP
From: Neal Cardwell @ 2018-07-03 0:51 UTC (permalink / raw)
To: Lawrence Brakmo
Cc: Netdev, Kernel Team, bmatheny, ast, Yuchung Cheng, Steve Ibanez,
Eric Dumazet
In-Reply-To: <20180702213908.1246455-1-brakmo@fb.com>
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
---
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=522040ea5fdd1c33bbf75e1d7c7c0422b96a94ef
commit 522040ea5fdd1c33bbf75e1d7c7c0422b96a94ef
Author: Eric Dumazet <edumazet@google.com>
Date: Mon May 21 15:08:57 2018 -0700
tcp: do not aggressively quick ack after ECN events
ECN signals currently forces TCP to enter quickack mode for
up to 16 (TCP_MAX_QUICKACKS) following incoming packets.
We believe this is not needed, and only sending one immediate ack
for the current packet should be enough.
This should reduce the extra load noticed in DCTCP environments,
after congestion events.
This is part 2 of our effort to reduce pure ACK packets.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=15ecbe94a45ef88491ca459b26efdd02f91edb6d
commit 15ecbe94a45ef88491ca459b26efdd02f91edb6d
Author: Eric Dumazet <edumazet@google.com>
Date: Wed Jun 27 08:47:21 2018 -0700
tcp: add one more quick ack after after ECN events
Larry Brakmo proposal ( https://patchwork.ozlabs.org/patch/935233/
tcp: force cwnd at least 2 in tcp_cwnd_reduction) made us rethink
about our recent patch removing ~16 quick acks after ECN events.
tcp_enter_quickack_mode(sk, 1) makes sure one immediate ack is sent,
but in the case the sender cwnd was lowered to 1, we do not want
to have a delayed ack for the next packet we will receive.
Fixes: 522040ea5fdd ("tcp: do not aggressively quick ack after ECN events")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Neal Cardwell <ncardwell@google.com>
Cc: Lawrence Brakmo <brakmo@fb.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* linux-next: build failure after merge of the net-next tree
From: Stephen Rothwell @ 2018-07-03 0:47 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Sabrina Dubroca
[-- Attachment #1: Type: text/plain, Size: 2971 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:
net/ipv4/fou.c: In function 'gue_gro_receive':
net/ipv4/fou.c:451:35: error: passing argument 2 of 'skb_gro_flush_final_remcsum' from incompatible pointer type [-Werror=incompatible-pointer-types]
skb_gro_flush_final_remcsum(skb, pp, flush, &grc);
^~
In file included from include/net/inet_sock.h:23:0,
from include/linux/udp.h:20,
from net/ipv4/fou.c:6:
include/linux/netdevice.h:2799:20: note: expected 'struct sk_buff **' but argument is of type 'struct sk_buff *'
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/vxlan.c: In function 'vxlan_gro_receive':
drivers/net/vxlan.c:627:35: error: passing argument 2 of 'skb_gro_flush_final_remcsum' from incompatible pointer type [-Werror=incompatible-pointer-types]
skb_gro_flush_final_remcsum(skb, pp, flush, &grc);
^~
In file included from include/net/inet_sock.h:23:0,
from include/linux/udp.h:20,
from drivers/net/vxlan.c:17:
include/linux/netdevice.h:2799:20: note: expected 'struct sk_buff **' but argument is of type 'struct sk_buff *'
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
Caused by commit
d4546c2509b1 ("net: Convert GRO SKB handling to list_head.")
from the net-next tree interacting with commit
603d4cf8fe09 ("net: fix use-after-free in GRO with ESP")
from Linus' tree.
I took a guess and aplied the following merge fix patch:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 3 Jul 2018 10:37:05 +1000
Subject: [PATCH] net: update for conversion of GRO SKB handling
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e1d4084c0651..64480a0f2c16 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2797,7 +2797,7 @@ static inline void skb_gro_flush_final(struct sk_buff *skb, struct sk_buff *pp,
NAPI_GRO_CB(skb)->flush |= flush;
}
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
- struct sk_buff **pp,
+ struct sk_buff *pp,
int flush,
struct gro_remcsum *grc)
{
@@ -2813,7 +2813,7 @@ static inline void skb_gro_flush_final(struct sk_buff *skb, struct sk_buff *pp,
NAPI_GRO_CB(skb)->flush |= flush;
}
static inline void skb_gro_flush_final_remcsum(struct sk_buff *skb,
- struct sk_buff **pp,
+ struct sk_buff *pp,
int flush,
struct gro_remcsum *grc)
{
--
2.17.1
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
* [PATCH ethtool] ethtool: ixgbe dump strings for security registers
From: Shannon Nelson @ 2018-07-03 0:09 UTC (permalink / raw)
To: linville; +Cc: netdev
Add the ixgbe's security configuration registers into
the register dump.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
ixgbe.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
NOTE: Obviously this should wait until Intel accepts and pushes the related
ixgbe patch and Dave accepts it into net-next, but I figured I may
as well send them out together.
diff --git a/ixgbe.c b/ixgbe.c
index c632137..6779402 100644
--- a/ixgbe.c
+++ b/ixgbe.c
@@ -1265,5 +1265,31 @@ ixgbe_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
regs_buff[1127]);
}
+ if (regs_buff_len > 1139 && mac_type != ixgbe_mac_82598EB) {
+ fprintf(stdout,
+ "0x08800: SECTXCTRL (Security Tx Control) 0x%08X\n",
+ regs_buff[1139]);
+
+ fprintf(stdout,
+ "0x08804: SECTXSTAT (Security Tx Status) 0x%08X\n",
+ regs_buff[1140]);
+
+ fprintf(stdout,
+ "0x08808: SECTXBUFFAF (Security Tx Buffer Almost Full) 0x%08X\n",
+ regs_buff[1141]);
+
+ fprintf(stdout,
+ "0x08800: SECTXMINIFG (Security Tx Buffer Minimum IFG) 0x%08X\n",
+ regs_buff[1142]);
+
+ fprintf(stdout,
+ "0x08800: SECRXCTRL (Security Rx Control) 0x%08X\n",
+ regs_buff[1143]);
+
+ fprintf(stdout,
+ "0x08800: SECRXSTAT (Security Rx Status) 0x%08X\n",
+ regs_buff[1144]);
+ }
+
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH next-queue] ixgbe: add ipsec security registers into ethtool register dump
From: Shannon Nelson @ 2018-07-03 0:09 UTC (permalink / raw)
To: intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev
Add the ixgbe's security configuration registers into
the register dump.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index bd1ba88..1d68884 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -511,7 +511,7 @@ static void ixgbe_set_msglevel(struct net_device *netdev, u32 data)
static int ixgbe_get_regs_len(struct net_device *netdev)
{
-#define IXGBE_REGS_LEN 1139
+#define IXGBE_REGS_LEN 1145
return IXGBE_REGS_LEN * sizeof(u32);
}
@@ -874,6 +874,14 @@ static void ixgbe_get_regs(struct net_device *netdev,
/* X540 specific DCB registers */
regs_buff[1137] = IXGBE_READ_REG(hw, IXGBE_RTTQCNCR);
regs_buff[1138] = IXGBE_READ_REG(hw, IXGBE_RTTQCNTG);
+
+ /* Security config registers */
+ regs_buff[1139] = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
+ regs_buff[1140] = IXGBE_READ_REG(hw, IXGBE_SECTXSTAT);
+ regs_buff[1141] = IXGBE_READ_REG(hw, IXGBE_SECTXBUFFAF);
+ regs_buff[1142] = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
+ regs_buff[1143] = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
+ regs_buff[1144] = IXGBE_READ_REG(hw, IXGBE_SECRXSTAT);
}
static int ixgbe_get_eeprom_len(struct net_device *netdev)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Yuchung Cheng @ 2018-07-02 23:48 UTC (permalink / raw)
To: Lawrence Brakmo
Cc: netdev, Kernel Team, Blake Matheny, Alexei Starovoitov,
Neal Cardwell, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-2-brakmo@fb.com>
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
>
> 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: [net] fq_codel: fix NULL pointer deref in fq_codel_reset
From: Keller, Jacob E @ 2018-07-02 23:27 UTC (permalink / raw)
To: Cong Wang
Cc: Kirsher, Jeffrey T, davem@davemloft.net, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, jogreene@redhat.com,
Eric Dumazet
In-Reply-To: <CAM_iQpXyR2Q-O1EfbGaCFfsQUAw=g_AUq8E6evzM9jVXQuWPXg@mail.gmail.com>
> -----Original Message-----
> From: Cong Wang [mailto:xiyou.wangcong@gmail.com]
> Sent: Monday, June 11, 2018 1:23 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net;
> netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com;
> jogreene@redhat.com; Eric Dumazet <edumazet@google.com>
> Subject: Re: [net] fq_codel: fix NULL pointer deref in fq_codel_reset
>
> On Mon, Jun 11, 2018 at 12:57 PM, Keller, Jacob E
> <jacob.e.keller@intel.com> wrote:
> >
> > I'm open to alternative suggestinos for fixing this, I think Eric suggested that
> maybe we should just remove the ->reset() call from qdisc_destroy..?
>
> You can't remove ->reset() for non-failure call path.
>
> For failure path, yeah, but it is much simpler to just make
> q->flows_cnt be 0 for this specific case.
Alright. I'll rework the patch to set flows_cnt to 0 when init fails.
Thanks,
Jake
^ permalink raw reply
* [PATCH v4,net-next] vlan: implement vlan id and protocol changes
From: Chas Williams @ 2018-07-02 22:35 UTC (permalink / raw)
To: davem; +Cc: netdev, Chas Williams
In-Reply-To: <20180610231912.1543-1-3chas3@gmail.com>
vlan_changelink silently ignores attempts to change the vlan id
or protocol id of an existing vlan interface. Implement by adding
the new vlan id and protocol to the interface's vlan group and then
removing the old vlan id and protocol from the vlan group. This
avoids the netlink churn of deleting and re-adding an interface
to change the vlan id.
Signed-off-by: Chas Williams <3chas3@gmail.com>
---
include/linux/netdevice.h | 1 +
net/8021q/vlan.c | 4 ++--
net/8021q/vlan.h | 2 ++
net/8021q/vlan_netlink.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++-
net/core/dev.c | 1 +
5 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8bf8d6149f79..bf3f557eed8c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2416,6 +2416,7 @@ enum netdev_cmd {
NETDEV_CVLAN_FILTER_DROP_INFO,
NETDEV_SVLAN_FILTER_PUSH_INFO,
NETDEV_SVLAN_FILTER_DROP_INFO,
+ NETDEV_CHANGEVLAN,
};
const char *netdev_cmd_to_name(enum netdev_cmd cmd);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 99141986efa0..b6d0b2e2ada0 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -51,8 +51,8 @@ const char vlan_version[] = DRV_VERSION;
/* End of global variables definitions. */
-static int vlan_group_prealloc_vid(struct vlan_group *vg,
- __be16 vlan_proto, u16 vlan_id)
+int vlan_group_prealloc_vid(struct vlan_group *vg,
+ __be16 vlan_proto, u16 vlan_id)
{
struct net_device **array;
unsigned int pidx, vidx;
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 44df1c3df02d..c734dd21d70d 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -116,6 +116,8 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack);
void unregister_vlan_dev(struct net_device *dev, struct list_head *head);
bool vlan_dev_inherit_address(struct net_device *dev,
struct net_device *real_dev);
+int vlan_group_prealloc_vid(struct vlan_group *vg,
+ __be16 vlan_proto, u16 vlan_id);
static inline u32 vlan_get_ingress_priority(struct net_device *dev,
u16 vlan_tci)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 9b60c1e399e2..0a4ae6b15d89 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -107,10 +107,56 @@ static int vlan_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[],
struct netlink_ext_ack *extack)
{
+ struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
struct ifla_vlan_flags *flags;
struct ifla_vlan_qos_mapping *m;
struct nlattr *attr;
int rem;
+ int err = 0;
+ __be16 vlan_proto = vlan->vlan_proto;
+ u16 vlan_id = vlan->vlan_id;
+
+ if (data[IFLA_VLAN_ID])
+ vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
+
+ if (data[IFLA_VLAN_PROTOCOL])
+ vlan_proto = nla_get_be16(data[IFLA_VLAN_PROTOCOL]);
+
+ if (vlan->vlan_id != vlan_id || vlan->vlan_proto != vlan_proto) {
+ struct net_device *real_dev = vlan->real_dev;
+ struct vlan_info *vlan_info;
+ struct vlan_group *grp;
+ __be16 old_vlan_proto = vlan->vlan_proto;
+ u16 old_vlan_id = vlan->vlan_id;
+
+ err = vlan_vid_add(real_dev, vlan_proto, vlan_id);
+ if (err)
+ goto out;
+ vlan_info = rtnl_dereference(real_dev->vlan_info);
+ grp = &vlan_info->grp;
+ err = vlan_group_prealloc_vid(grp, vlan_proto, vlan_id);
+ if (err < 0) {
+ vlan_vid_del(real_dev, vlan_proto, vlan_id);
+ return err;
+ }
+ vlan_group_set_device(grp, vlan_proto, vlan_id, dev);
+ vlan->vlan_proto = vlan_proto;
+ vlan->vlan_id = vlan_id;
+
+ err = call_netdevice_notifiers(NETDEV_CHANGEVLAN, dev);
+ err = notifier_to_errno(err);
+ if (err) {
+ /* rollback */
+ vlan_group_set_device(grp, vlan_proto, vlan_id, NULL);
+ vlan_vid_del(real_dev, vlan_proto, vlan_id);
+ vlan->vlan_proto = old_vlan_proto;
+ vlan->vlan_id = old_vlan_id;
+ } else {
+ vlan_group_set_device(grp, old_vlan_proto,
+ old_vlan_id, NULL);
+ vlan_vid_del(real_dev, old_vlan_proto, old_vlan_id);
+ }
+ }
if (data[IFLA_VLAN_FLAGS]) {
flags = nla_data(data[IFLA_VLAN_FLAGS]);
@@ -128,7 +174,8 @@ static int vlan_changelink(struct net_device *dev, struct nlattr *tb[],
vlan_dev_set_egress_priority(dev, m->from, m->to);
}
}
- return 0;
+out:
+ return err;
}
static int vlan_newlink(struct net *src_net, struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index 08d58e0debe5..63adc37e3548 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1587,6 +1587,7 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
+ N(CHANGEVLAN)
}
#undef N
return "UNKNOWN_NETDEV_EVENT";
--
2.14.4
^ permalink raw reply related
* [PATCH net] net/ipv6: Revert attempt to simplify route replace and append
From: dsahern @ 2018-07-02 22:03 UTC (permalink / raw)
To: netdev; +Cc: idosch, sharpd, Thomas.Winter, David Ahern
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.
include/net/ip6_route.h | 6 ++
net/ipv6/ip6_fib.c | 156 +++++++++++++++++--------------
net/ipv6/route.c | 3 +-
tools/testing/selftests/net/fib_tests.sh | 41 --------
4 files changed, 93 insertions(+), 113 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 59656fc580df..7b9c82de11cc 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -66,6 +66,12 @@ static inline bool rt6_need_strict(const struct in6_addr *daddr)
(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL | IPV6_ADDR_LOOPBACK);
}
+static inline bool rt6_qualify_for_ecmp(const struct fib6_info *f6i)
+{
+ return (f6i->fib6_flags & (RTF_GATEWAY|RTF_ADDRCONF|RTF_DYNAMIC)) ==
+ RTF_GATEWAY;
+}
+
void ip6_route_input(struct sk_buff *skb);
struct dst_entry *ip6_route_input_lookup(struct net *net,
struct net_device *dev,
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 1fb2f3118d60..d212738e9d10 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -935,20 +935,19 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
{
struct fib6_info *leaf = rcu_dereference_protected(fn->leaf,
lockdep_is_held(&rt->fib6_table->tb6_lock));
- enum fib_event_type event = FIB_EVENT_ENTRY_ADD;
- struct fib6_info *iter = NULL, *match = NULL;
+ struct fib6_info *iter = NULL;
struct fib6_info __rcu **ins;
+ struct fib6_info __rcu **fallback_ins = NULL;
int replace = (info->nlh &&
(info->nlh->nlmsg_flags & NLM_F_REPLACE));
- int append = (info->nlh &&
- (info->nlh->nlmsg_flags & NLM_F_APPEND));
int add = (!info->nlh ||
(info->nlh->nlmsg_flags & NLM_F_CREATE));
int found = 0;
+ bool rt_can_ecmp = rt6_qualify_for_ecmp(rt);
u16 nlflags = NLM_F_EXCL;
int err;
- if (append)
+ if (info->nlh && (info->nlh->nlmsg_flags & NLM_F_APPEND))
nlflags |= NLM_F_APPEND;
ins = &fn->leaf;
@@ -970,8 +969,13 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
nlflags &= ~NLM_F_EXCL;
if (replace) {
- found++;
- break;
+ if (rt_can_ecmp == rt6_qualify_for_ecmp(iter)) {
+ found++;
+ break;
+ }
+ if (rt_can_ecmp)
+ fallback_ins = fallback_ins ?: ins;
+ goto next_iter;
}
if (rt6_duplicate_nexthop(iter, rt)) {
@@ -986,51 +990,71 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
fib6_metric_set(iter, RTAX_MTU, rt->fib6_pmtu);
return -EEXIST;
}
-
- /* first route that matches */
- if (!match)
- match = iter;
+ /* If we have the same destination and the same metric,
+ * but not the same gateway, then the route we try to
+ * add is sibling to this route, increment our counter
+ * of siblings, and later we will add our route to the
+ * list.
+ * Only static routes (which don't have flag
+ * RTF_EXPIRES) are used for ECMPv6.
+ *
+ * To avoid long list, we only had siblings if the
+ * route have a gateway.
+ */
+ if (rt_can_ecmp &&
+ rt6_qualify_for_ecmp(iter))
+ rt->fib6_nsiblings++;
}
if (iter->fib6_metric > rt->fib6_metric)
break;
+next_iter:
ins = &iter->fib6_next;
}
+ if (fallback_ins && !found) {
+ /* No ECMP-able route found, replace first non-ECMP one */
+ ins = fallback_ins;
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+ found++;
+ }
+
/* Reset round-robin state, if necessary */
if (ins == &fn->leaf)
fn->rr_ptr = NULL;
/* Link this route to others same route. */
- if (append && match) {
+ if (rt->fib6_nsiblings) {
+ unsigned int fib6_nsiblings;
struct fib6_info *sibling, *temp_sibling;
- if (rt->fib6_flags & RTF_REJECT) {
- NL_SET_ERR_MSG(extack,
- "Can not append a REJECT route");
- return -EINVAL;
- } else if (match->fib6_flags & RTF_REJECT) {
- NL_SET_ERR_MSG(extack,
- "Can not append to a REJECT route");
- return -EINVAL;
+ /* Find the first route that have the same metric */
+ sibling = leaf;
+ while (sibling) {
+ if (sibling->fib6_metric == rt->fib6_metric &&
+ rt6_qualify_for_ecmp(sibling)) {
+ list_add_tail(&rt->fib6_siblings,
+ &sibling->fib6_siblings);
+ break;
+ }
+ sibling = rcu_dereference_protected(sibling->fib6_next,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
}
- event = FIB_EVENT_ENTRY_APPEND;
- rt->fib6_nsiblings = match->fib6_nsiblings;
- list_add_tail(&rt->fib6_siblings, &match->fib6_siblings);
- match->fib6_nsiblings++;
-
/* For each sibling in the list, increment the counter of
* siblings. BUG() if counters does not match, list of siblings
* is broken!
*/
+ fib6_nsiblings = 0;
list_for_each_entry_safe(sibling, temp_sibling,
- &match->fib6_siblings, fib6_siblings) {
+ &rt->fib6_siblings, fib6_siblings) {
sibling->fib6_nsiblings++;
- BUG_ON(sibling->fib6_nsiblings != match->fib6_nsiblings);
+ BUG_ON(sibling->fib6_nsiblings != rt->fib6_nsiblings);
+ fib6_nsiblings++;
}
-
- rt6_multipath_rebalance(match);
+ BUG_ON(fib6_nsiblings != rt->fib6_nsiblings);
+ rt6_multipath_rebalance(temp_sibling);
}
/*
@@ -1043,8 +1067,9 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
add:
nlflags |= NLM_F_CREATE;
- err = call_fib6_entry_notifiers(info->nl_net, event, rt,
- extack);
+ err = call_fib6_entry_notifiers(info->nl_net,
+ FIB_EVENT_ENTRY_ADD,
+ rt, extack);
if (err)
return err;
@@ -1062,7 +1087,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
}
} else {
- struct fib6_info *tmp;
+ int nsiblings;
if (!found) {
if (add)
@@ -1077,57 +1102,48 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
if (err)
return err;
- /* if route being replaced has siblings, set tmp to
- * last one, otherwise tmp is current route. this is
- * used to set fib6_next for new route
- */
- if (iter->fib6_nsiblings)
- tmp = list_last_entry(&iter->fib6_siblings,
- struct fib6_info,
- fib6_siblings);
- else
- tmp = iter;
-
- /* insert new route */
atomic_inc(&rt->fib6_ref);
rcu_assign_pointer(rt->fib6_node, fn);
- rt->fib6_next = tmp->fib6_next;
+ rt->fib6_next = iter->fib6_next;
rcu_assign_pointer(*ins, rt);
-
if (!info->skip_notify)
inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
if (!(fn->fn_flags & RTN_RTINFO)) {
info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
fn->fn_flags |= RTN_RTINFO;
}
+ nsiblings = iter->fib6_nsiblings;
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
- /* delete old route */
- rt = iter;
-
- if (rt->fib6_nsiblings) {
- struct fib6_info *tmp;
-
+ if (nsiblings) {
/* Replacing an ECMP route, remove all siblings */
- list_for_each_entry_safe(iter, tmp, &rt->fib6_siblings,
- fib6_siblings) {
- iter->fib6_node = NULL;
- fib6_purge_rt(iter, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == iter)
- fn->rr_ptr = NULL;
- fib6_info_release(iter);
-
- rt->fib6_nsiblings--;
- info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
+ ins = &rt->fib6_next;
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+ while (iter) {
+ if (iter->fib6_metric > rt->fib6_metric)
+ break;
+ if (rt6_qualify_for_ecmp(iter)) {
+ *ins = iter->fib6_next;
+ iter->fib6_node = NULL;
+ fib6_purge_rt(iter, fn, info->nl_net);
+ if (rcu_access_pointer(fn->rr_ptr) == iter)
+ fn->rr_ptr = NULL;
+ fib6_info_release(iter);
+ nsiblings--;
+ info->nl_net->ipv6.rt6_stats->fib_rt_entries--;
+ } else {
+ ins = &iter->fib6_next;
+ }
+ iter = rcu_dereference_protected(*ins,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
}
+ WARN_ON(nsiblings != 0);
}
-
- WARN_ON(rt->fib6_nsiblings != 0);
-
- rt->fib6_node = NULL;
- fib6_purge_rt(rt, fn, info->nl_net);
- if (rcu_access_pointer(fn->rr_ptr) == rt)
- fn->rr_ptr = NULL;
- fib6_info_release(rt);
}
return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 86a0e4333d42..63f99411f0de 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3842,7 +3842,7 @@ static struct fib6_info *rt6_multipath_first_sibling(const struct fib6_info *rt)
lockdep_is_held(&rt->fib6_table->tb6_lock));
while (iter) {
if (iter->fib6_metric == rt->fib6_metric &&
- iter->fib6_nsiblings)
+ rt6_qualify_for_ecmp(iter))
return iter;
iter = rcu_dereference_protected(iter->fib6_next,
lockdep_is_held(&rt->fib6_table->tb6_lock));
@@ -4439,7 +4439,6 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
*/
cfg->fc_nlinfo.nlh->nlmsg_flags &= ~(NLM_F_EXCL |
NLM_F_REPLACE);
- cfg->fc_nlinfo.nlh->nlmsg_flags |= NLM_F_APPEND;
nhn++;
}
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 78245d60d8bc..0f45633bd634 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -740,13 +740,6 @@ ipv6_rt_add()
run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
log_test $? 2 "Attempt to add duplicate route - reject route"
- # iproute2 prepend only sets NLM_F_CREATE
- # - adds a new route; does NOT convert existing route to ECMP
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro prepend 2001:db8:104::/64 via 2001:db8:103::2"
- check_route6 "2001:db8:104::/64 via 2001:db8:101::2 dev veth1 metric 1024 2001:db8:104::/64 via 2001:db8:103::2 dev veth3 metric 1024"
- log_test $? 0 "Add new route for existing prefix (w/o NLM_F_EXCL)"
-
# route append with same prefix adds a new route
# - iproute2 sets NLM_F_CREATE | NLM_F_APPEND
add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
@@ -754,27 +747,6 @@ ipv6_rt_add()
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
log_test $? 0 "Append nexthop to existing route - gw"
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
- check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop dev veth3 weight 1"
- log_test $? 0 "Append nexthop to existing route - dev only"
-
- # multipath route can not have a nexthop that is a reject route
- add_route6 "2001:db8:104::/64" "via 2001:db8:101::2"
- run_cmd "$IP -6 ro append unreachable 2001:db8:104::/64"
- log_test $? 2 "Append nexthop to existing route - reject route"
-
- # reject route can not be converted to multipath route
- run_cmd "$IP -6 ro flush 2001:db8:104::/64"
- run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 via 2001:db8:103::2"
- log_test $? 2 "Append nexthop to existing reject route - gw"
-
- run_cmd "$IP -6 ro flush 2001:db8:104::/64"
- run_cmd "$IP -6 ro add unreachable 2001:db8:104::/64"
- run_cmd "$IP -6 ro append 2001:db8:104::/64 dev veth3"
- log_test $? 2 "Append nexthop to existing reject route - dev only"
-
# insert mpath directly
add_route6 "2001:db8:104::/64" "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::2 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
@@ -819,13 +791,6 @@ ipv6_rt_replace_single()
check_route6 "2001:db8:104::/64 metric 1024 nexthop via 2001:db8:101::3 dev veth1 weight 1 nexthop via 2001:db8:103::2 dev veth3 weight 1"
log_test $? 0 "Single path with multipath"
- # single path with reject
- #
- add_initial_route6 "nexthop via 2001:db8:101::2"
- run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
- check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
- log_test $? 0 "Single path with reject route"
-
# single path with single path using MULTIPATH attribute
#
add_initial_route6 "via 2001:db8:101::2"
@@ -873,12 +838,6 @@ ipv6_rt_replace_mpath()
check_route6 "2001:db8:104::/64 via 2001:db8:101::3 dev veth1 metric 1024"
log_test $? 0 "Multipath with single path via multipath attribute"
- # multipath with reject
- add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
- run_cmd "$IP -6 ro replace unreachable 2001:db8:104::/64"
- check_route6 "unreachable 2001:db8:104::/64 dev lo metric 1024"
- log_test $? 0 "Multipath with reject route"
-
# route replace fails - invalid nexthop 1
add_initial_route6 "nexthop via 2001:db8:101::2 nexthop via 2001:db8:103::2"
run_cmd "$IP -6 ro replace 2001:db8:104::/64 nexthop via 2001:db8:111::3 nexthop via 2001:db8:103::3"
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 08/10] r8169: remove rtl8169_set_speed_xmii
From: Heiner Kallweit @ 2018-07-02 21:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Miller, Florian Fainelli, Realtek linux nic maintainers,
netdev@vger.kernel.org
In-Reply-To: <20180702212150.GG12564@lunn.ch>
On 02.07.2018 23:21, Andrew Lunn wrote:
>> - auto_nego |= ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM;
>
> This bit you probably want to keep. The PHY never says it support
> Pause. The MAC needs to enable pause if the MAC supports pause.
>
Actually I assumed that phylib would do this for me. But:
In phy_probe() first phydev->supported is copied to
phydev->advertising, and only after this both pause flags are added
to phydev->supported. Therefore I think they are not advertised.
Is this intentional? It sounds a little weird to me to add the
pause flags to the supported features per default, but not
advertise them.
Except e.g. we call by chance phy_set_max_speed(), which copies
phydev->supported to phydev->advertising after having adjusted
the supported speeds.
If this is not a bug, then where would be the right place to add
the pause flags to phydev->advertising?
> Andrew
>
Heiner
^ permalink raw reply
* [PATCH net-next v2 2/2] tcp: ack immediately when a cwr packet arrives
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-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 | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 76ca88f63b70..6fd1f2378f6c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -254,10 +254,16 @@ static void tcp_ecn_withdraw_cwr(struct tcp_sock *tp)
tp->ecn_flags &= ~TCP_ECN_DEMAND_CWR;
}
-static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
+static void __tcp_ecn_check(struct sock *sk, const struct sk_buff *skb)
{
struct tcp_sock *tp = tcp_sk(sk);
+ /* 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.
+ */
+ if (tcp_hdr(skb)->cwr)
+ tcp_enter_quickack_mode(sk, 2);
+
switch (TCP_SKB_CB(skb)->ip_dsfield & INET_ECN_MASK) {
case INET_ECN_NOT_ECT:
/* Funny extension: if ECT is not set on a segment,
@@ -286,10 +292,10 @@ static void __tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
}
}
-static void tcp_ecn_check_ce(struct sock *sk, const struct sk_buff *skb)
+static void tcp_ecn_check(struct sock *sk, const struct sk_buff *skb)
{
if (tcp_sk(sk)->ecn_flags & TCP_ECN_OK)
- __tcp_ecn_check_ce(sk, skb);
+ __tcp_ecn_check(sk, skb);
}
static void tcp_ecn_rcv_synack(struct tcp_sock *tp, const struct tcphdr *th)
@@ -715,7 +721,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
}
icsk->icsk_ack.lrcvtime = now;
- tcp_ecn_check_ce(sk, skb);
+ tcp_ecn_check(sk, skb);
if (skb->len >= 128)
tcp_grow_window(sk, skb);
@@ -4439,7 +4445,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
u32 seq, end_seq;
bool fragstolen;
- tcp_ecn_check_ce(sk, skb);
+ tcp_ecn_check(sk, skb);
if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v2 1/2] tcp: notify when a delayed ack is sent
From: Lawrence Brakmo @ 2018-07-02 21:39 UTC (permalink / raw)
To: netdev
Cc: Kernel Team, Blake Matheny, Alexei Starovoitov, Neal Cardwell,
Yuchung Cheng, Steve Ibanez, Eric Dumazet
In-Reply-To: <20180702213908.1246455-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
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