* Re: [Patch net] smc: convert to ->poll_mask
From: David Miller @ 2018-06-12 22:37 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, penguin-kernel, hch, ubraun
In-Reply-To: <20180611210714.3754-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 11 Jun 2018 14:07:14 -0700
> smc->clcsock is an internal TCP socket, after TCP socket
> converts to ->poll_mask, ->poll doesn't exist any more.
> So just convert smc socket to ->poll_mask too.
>
> Fixes: 2c7d3dacebd4 ("net/tcp: convert to ->poll_mask")
> Reported-by: syzbot+f5066e369b2d5fff630f@syzkaller.appspotmail.com
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ursula Braun <ubraun@linux.ibm.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thanks Cong.
^ permalink raw reply
* Re: [PATCH] net: stmmac: dwmac-meson8b: Fix an error handling path in 'meson8b_dwmac_probe()'
From: David Miller @ 2018-06-12 22:36 UTC (permalink / raw)
To: christophe.jaillet
Cc: peppe.cavallaro, alexandre.torgue, joabreu, carlo, khilman,
netdev, linux-arm-kernel, linux-amlogic, linux-kernel,
kernel-janitors
In-Reply-To: <20180611175227.27509-1-christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Mon, 11 Jun 2018 19:52:27 +0200
> If 'of_device_get_match_data()' fails, we need to release some resources as
> done in the other error handling path of this function.
>
> Fixes: efacb568c962 ("net: stmmac: dwmac-meson: extend phy mode setting")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied.
^ permalink raw reply
* Re: Backport bonding patches to fix active-passive
From: David Miller @ 2018-06-12 22:35 UTC (permalink / raw)
To: nate; +Cc: netdev
In-Reply-To: <CAG2YfWNKXy+VkrbaxfaKof_T8bOF5skESCdQmPzU9_DQ-7an_w@mail.gmail.com>
From: Nate Clark <nate@neworld.us>
Date: Mon, 11 Jun 2018 13:44:40 -0400
> Would it be possible to queue up the three commits for backporting to
> 4.9 stable:
> b5bf0f5b16b9c316c34df9f31d4be8729eb86845 bonding: correctly update
> link status during mii-commit
> 3f3c278c94dd994fe0d9f21679ae19b9c0a55292 bonding: fix active-backup transition
> ad729bc9acfb7c47112964b4877ef5404578ed13 bonding: require speed/duplex
> only for 802.3ad, alb and tlb
>
> All of those commits apply cleanly to 4.9.107.
I only deal with -stable backports to the most recent two releases.
If you want something to happen for earlier releases you'll need to
ask the -stable tree maintainers directly.
Thank you.
^ permalink raw reply
* Re: Problems in tc-matchall.8, tc-sample.8
From: Stephen Hemminger @ 2018-06-12 22:33 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: netdev
In-Reply-To: <20180612220003.GE4849@thyrsus.com>
On Tue, 12 Jun 2018 18:00:03 -0400
"Eric S. Raymond" <esr@thyrsus.com> wrote:
> Stephen Hemminger <stephen@networkplumber.org>:
> > Please resubmit as real patch with signed-off-by
>
> I would like to follow your intructions, but that description leaves me
> not quite certain what you want. A git format-patch thing? If so, what
> git url should I clone from?
iproute patches are handled the same as the Linux kernel.
Please submit patches to the netdev@vger.kernel.org with the same kind
of diff format (and signed-off-by) as the kernel.
Like the kernel, patches which are pure bug fixes go to the master
branch, and patches with new functionality are handled with the iproute2-next repository.
^ permalink raw reply
* Re: [Intel-wired-lan] [jkirsher/next-queue PATCH v2 0/7] Add support for L2 Fwd Offload w/o ndo_select_queue
From: Alexander Duyck @ 2018-06-12 22:33 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Alexander Duyck, intel-wired-lan, Jeff Kirsher, Netdev
In-Reply-To: <be1b5bed-d8b2-244e-167a-1f79bfb5f6e9@gmail.com>
On Tue, Jun 12, 2018 at 10:56 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 06/12/2018 08:18 AM, Alexander Duyck 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.
>
> Interesting, so at some point I had came up with the following for
> mapping queues between the DSA slave network devices and the DSA master
> network device (doing the actual transmission). The DSA master network
> device driver is just a normal network device driver.
>
> The set-up is as follows: 4 external Ethernet switch ports, each with 8
> egress queues and the DSA master (bcmsysport.c), aka CPU Ethernet
> controller has 32 output queues, so you can do a 1:1 mapping of those,
> that's actually what we want. A subsequent hardware generation only
> provides 16 output queues, so we can still do 2:1 mapping.
>
> The implementation is done like this:
>
> - DSA slave network devices are always created after the DSA master
> network device so we can leverage that
>
> - a specific notifier is running from the DSA core and tells the DSA
> master about the switch position in the tree (position 0 = directly
> attached), and the switch port number and a pointer to the slave network
> device
>
> - we establish the mapping between the queues within the bcmsysport
> driver as a simple array
>
> - when transmitting, DSA slave network devices set a specific queue/port
> number within the 16-bits that skb->queue_mapping permits
>
> - this gets re-used by bcmsysport.c to extract the correct queue number
> during ndo_select_queue such that the appropriate queue number gets used
> and congestion works end-to-end.
>
> The reason why we do that is because there is some out of band HW that
> monitors the queue depth of the switch port's egress queue and
> back-pressure the Ethernet controller directly when trying to transmit
> to a congested queue.
>
> I had initially considered establishing the mapping using tc and some
> custom "bind" argument of some kind, but ended-up doing things the way
> they are which are more automatic though they leave less configuration
> to an user. This has a number of caveats though:
>
> - this is made generic within the context of DSA in that nothing is
> switch driver or Ethernet MAC driver specific and the notifier
> represents the contract between these two seemingly independent subsystems
>
> - the queue indicated between DSA slave and master is unfortunately
> switch driver/controller specific (BRCM_TAG_SET_PORT_QUEUE,
> BRCM_TAG_GET_PORT, BRCM_TAG_GET_QUEUE)
>
> What I like about your patchset is the mapping establishment, but as you
> will read from my reply in patch 2, I think the (upper) 1:N (lower)
> mapping might not work for my specific use case.
>
> Anyhow, not intended to be blocking this, as it seems to be going in the
> right direction anyway.
I think I am still not getting why the 1:N would be an issue. At least
the way I have the code implemented here the lower queues all have a
qdisc associated with them, just not the upper device. Generally I am
using the macvlan as a bump in the wire to take care of filtering for
the bridging mode. If I have to hairpin packets and send them back up
on on of the the upper interfaces I want to do that in software rather
than hardware so I try to take care of it there instead of routing it
through the hardware.
>>
>> 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
>
> Interesting, turns out I had a possibly similar use case with DSA with
> the slave network devices need to select an outgoing queue number for
I was kind of assuming this could be applied to a number of possible
use cases. As it was I was wondering if maybe we should look at adding
this as an option for just a standard VLAN as we could perform the
same kind of filtering and just deliver the packet directly to the
VLAN interface instead of requiring the extra trip through the stack
after the tag has been stripped.
^ permalink raw reply
* Re: [PATCH net] tc-testing: ife: fix wrong teardown command in test b7b8
From: David Miller @ 2018-06-12 22:32 UTC (permalink / raw)
To: dcaratti; +Cc: lucasb, mrv, netdev
In-Reply-To: <37eb01ee5c46cb7c5e094390e65eb476aa09f07e.1528725486.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Mon, 11 Jun 2018 16:02:36 +0200
> fix failures in the 'teardown' stage of test b7b8, probably a leftover of
> commit 7c5995b33d6e ("tc-testing: fixed copy-pasting error in ife tests")
>
> Fixes: a56e6bcd34b55 ("tc-testing: updated ife test cases")
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied, thank youo.
^ permalink raw reply
* Re: [PATCH v2] tcp: verify the checksum of the first data segment in a new connection
From: van der Linden, Frank @ 2018-06-12 22:30 UTC (permalink / raw)
To: Eric Dumazet, edumazet@google.com, netdev@vger.kernel.org
In-Reply-To: <212193c0-2fee-7f88-5473-9f5f4c548cb8@gmail.com>
Sure, fair enough. I was assuming there might be a reason of why tcp_filter was always done after the data (not pseudo header) checksum. If there isn't (and obviously the the possible MD5 checks are done before it too), then that's definitely the right thing to do.
I'll resend. Though if you have the simpler change already lined up, I'll happily refrain from sending it myself.
Frank
On 6/12/18, 3:03 PM, "Eric Dumazet" <eric.dumazet@gmail.com> wrote:
On 06/12/2018 02:53 PM, van der Linden, Frank wrote:
> The convention seems to be to call tcp_checksum_complete after tcp_filter has a chance to deal with the packet. I wanted to preserve that.
>
> If that is not a concern, then I agree that this is a far better way to go.
>
> Frank
Given that we can drop the packet earlier from :
if (skb_checksum_init(skb, IPPROTO_TCP, inet_compute_pseudo))
goto csum_error;
I am quite sure we really do not care of tcp_filter() being
hit or not by packets with bad checksum.
Thanks
^ permalink raw reply
* Re: [PATCH] net: thunderx: prevent concurrent data re-writing by nicvf_set_rx_mode
From: David Miller @ 2018-06-12 22:25 UTC (permalink / raw)
To: dnelson
Cc: Vadim.Lomovtsev, rric, sgoutham, linux-arm-kernel, netdev,
linux-kernel, Vadim.Lomovtsev
In-Reply-To: <036618ae-887f-44b5-2b39-451b81191cc1@redhat.com>
From: Dean Nelson <dnelson@redhat.com>
Date: Mon, 11 Jun 2018 06:22:14 -0500
> On 06/10/2018 02:35 PM, David Miller wrote:
>> From: Vadim Lomovtsev <Vadim.Lomovtsev@caviumnetworks.com>
>> Date: Fri, 8 Jun 2018 02:27:59 -0700
>>
>>> + /* Save message data locally to prevent them from
>>> + * being overwritten by next ndo_set_rx_mode call().
>>> + */
>>> + spin_lock(&nic->rx_mode_wq_lock);
>>> + mode = vf_work->mode;
>>> + mc = vf_work->mc;
>>> + vf_work->mc = NULL;
>
> If I'm reading this code correctly, I believe nic->rx_mode_work.mc
> will
> have been set to NULL before the lock is dropped by
> nicvf_set_rx_mode_task() and acquired by nicvf_set_rx_mode().
>
>
>>> + spin_unlock(&nic->rx_mode_wq_lock);
>> At the moment you drop this lock, the memory behind 'mc' can be
>> freed up by:
>>
>>> + spin_lock(&nic->rx_mode_wq_lock);
>>> + kfree(nic->rx_mode_work.mc);
>
> So the kfree() will be called with a NULL pointer and quickly return.
>
>
>> And you'll crash when you dereference it above via
>> __nicvf_set_rx_mode_task().
>>
>
> I believe the call to kfree() in nicvf_set_rx_mode() is there to free
> up a mc_list that has been allocated by nicvf_set_rx_mode() during a
> previous callback to the function, one that has not yet been processed
> by nicvf_set_rx_mode_task().
>
> In this way only the last 'unprocessed' callback to
> nicvf_set_rx_mode()
> gets processed should there be multiple callbacks occurring between
> the
> times the nicvf_set_rx_mode_task() runs.
>
> In my testing with this patch, this is what I see happening.
You're right, my bad.
Patch applied.
^ permalink raw reply
* Re: [PATCH] net: phy: mdio-gpio: Cut surplus includes
From: David Miller @ 2018-06-12 22:24 UTC (permalink / raw)
To: linus.walleij; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <20180611111903.7221-1-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Mon, 11 Jun 2018 13:19:03 +0200
> The GPIO MDIO driver now needs only <linux/gpio/consumer.h>
> so cut the legacy <linux/gpio.h> and <linux/of_gpio.h>
> includes that are no longer used.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net 0/3] hv_netvsc: notification and namespace fixes
From: David Miller @ 2018-06-12 22:22 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev
In-Reply-To: <20180611194456.8268-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 11 Jun 2018 12:44:53 -0700
> This set of patches addresses two set of fixes. First it backs out
> the common callback model which was merged in net-next without
> completing all the review feedback or getting maintainer approval.
>
> Then it fixes the transparent VF management code to handle network
> namespaces.
Series applied.
^ permalink raw reply
* Re: [PATCH net 0/4] nfp: fix a warning, stats, naming and route leak
From: David Miller @ 2018-06-12 22:18 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20180612043338.5447-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 11 Jun 2018 21:33:34 -0700
> Various fixes for the NFP. Patch 1 fixes a harmless GCC 8 warning.
> Patch 2 ensures statistics are correct after users decrease the number
> of channels/rings. Patch 3 restores phy_port_name behaviour for flower,
> ndo_get_phy_port_name used to return -EOPNOTSUPP on one of the netdevs,
> and we need to keep it that way otherwise interface names may change.
> Patch 4 fixes refcnt leak in flower tunnel offload code.
Series applied.
^ permalink raw reply
* Re: [Intel-wired-lan] [jkirsher/next-queue PATCH v2 2/7] net: Add support for subordinate device traffic classes
From: Alexander Duyck @ 2018-06-12 22:18 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Alexander Duyck, intel-wired-lan, Jeff Kirsher, Netdev
In-Reply-To: <f4eaac32-204e-259d-b69b-c2c9885d55fa@gmail.com>
On Tue, Jun 12, 2018 at 10:49 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 06/12/2018 08:18 AM, Alexander Duyck wrote:
>> This patch is meant to provide the basic tools needed to allow us to create
>> subordinate device traffic classes. The general idea here is to allow
>> subdividing the queues of a device into queue groups accessible through an
>> upper device such as a macvlan.
>>
>> The idea here is to enforce the idea that an upper device has to be a
>> single queue device, ideally with IFF_NO_QUQUE set. With that being the
>> case we can pretty much guarantee that the tc_to_txq mappings and XPS maps
>> for the upper device are unused. As such we could reuse those in order to
>> support subdividing the lower device and distributing those queues between
>> the subordinate devices.
>
> This is not necessarily a valid paradigm to work with. For instance in
> DSA we have IFF_NO_QUEUE devices, but we still expose multiple egress
> queues because that is how an application can choose how it wants to get
> packets transmitted at the switch level. We have a 1:1 representation
> between a queue at the net_device level, and what an egress queue at the
> switch level is, so things like buffer reservation etc. can be configured.
I'm not saying that IFF_NO_QUEUE implies that a device is single
queue, but in this case we enforce that the upper device has to be a
single queue device so that the code in netdev_pick_tx will ignore the
XPS and tc_to_txq mappings for that netdev. I had mentioned
IFF_NO_QUEUE as a suggestion as that allows us to avoid head-of-line
blocking if the lower device starts to apply back-pressure.
> I think you should consider that an upper device might want to have a
> 1:1 mapping to the lower device's queues and make that permissible.
> Thoughts?
I had considered that. However the issue becomes that at that point it
makes the setup much more rigid. With this approach I can enable and
disable the offload without needing to stop the upper device to either
create or remove qdiscs. I would much rather keep the upper device
generic and leave it to the lower device to populate the rings and
such.
^ permalink raw reply
* Re: [PATCH v2] tcp: verify the checksum of the first data segment in a new connection
From: Eric Dumazet @ 2018-06-12 22:03 UTC (permalink / raw)
To: van der Linden, Frank, Eric Dumazet, edumazet@google.com,
netdev@vger.kernel.org
In-Reply-To: <EB56EB1B-8E64-4D2C-9604-5ACFD3857F0D@amazon.com>
On 06/12/2018 02:53 PM, van der Linden, Frank wrote:
> The convention seems to be to call tcp_checksum_complete after tcp_filter has a chance to deal with the packet. I wanted to preserve that.
>
> If that is not a concern, then I agree that this is a far better way to go.
>
> Frank
Given that we can drop the packet earlier from :
if (skb_checksum_init(skb, IPPROTO_TCP, inet_compute_pseudo))
goto csum_error;
I am quite sure we really do not care of tcp_filter() being
hit or not by packets with bad checksum.
Thanks
^ permalink raw reply
* Re: Problems in tc-matchall.8, tc-sample.8
From: Eric S. Raymond @ 2018-06-12 22:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180612141700.3a679643@xeon-e3>
Stephen Hemminger <stephen@networkplumber.org>:
> Please resubmit as real patch with signed-off-by
I would like to follow your intructions, but that description leaves me
not quite certain what you want. A git format-patch thing? If so, what
git url should I clone from?
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
My work is funded by the Internet Civil Engineering Institute: https://icei.org
Please visit their site and donate: the civilization you save might be your own.
^ permalink raw reply
* Re: [PATCH v2] tcp: verify the checksum of the first data segment in a new connection
From: van der Linden, Frank @ 2018-06-12 21:53 UTC (permalink / raw)
To: Eric Dumazet, edumazet@google.com, netdev@vger.kernel.org
In-Reply-To: <9541859a-1346-e13a-b97c-a2a63f3b19f4@gmail.com>
The convention seems to be to call tcp_checksum_complete after tcp_filter has a chance to deal with the packet. I wanted to preserve that.
If that is not a concern, then I agree that this is a far better way to go.
Frank
On 6/12/18, 2:50 PM, "Eric Dumazet" <eric.dumazet@gmail.com> wrote:
On 06/12/2018 02:41 PM, Frank van der Linden wrote:
> commit 079096f103fa ("tcp/dccp: install syn_recv requests into ehash
> table") introduced an optimization for the handling of child sockets
> created for a new TCP connection.
>
> But this optimization passes any data associated with the last ACK of the
> connection handshake up the stack without verifying its checksum, because it
> calls tcp_child_process(), which in turn calls tcp_rcv_state_process()
> directly. These lower-level processing functions do not do any checksum
> verification.
>
> Insert a tcp_checksum_complete call in the TCP_NEW_SYN_RECEIVE path to
> fix this.
>
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
This is way too complicated.
You should call tcp_checksum_complete() earlier and avoid all this mess.
IPV4 part shown here :
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index fed3f1c6616708997f621535efe9412e4afa0a50..7b5f32aa3835b0124b0a9bd342c371df7b46f471 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1730,6 +1730,10 @@ int tcp_v4_rcv(struct sk_buff *skb)
reqsk_put(req);
goto discard_it;
}
+ if (unlikely(tcp_checksum_complete(skb))) {
+ reqsk_put(req);
+ goto csum_error;
+ }
if (unlikely(sk->sk_state != TCP_LISTEN)) {
inet_csk_reqsk_queue_drop_and_put(sk, req);
goto lookup;
^ permalink raw reply
* Re: [PATCH v2] tcp: verify the checksum of the first data segment in a new connection
From: Eric Dumazet @ 2018-06-12 21:50 UTC (permalink / raw)
To: Frank van der Linden, edumazet, netdev
In-Reply-To: <5b203e1b.vy4yU6CwMEwLmNtj%fllinden@amazon.com>
On 06/12/2018 02:41 PM, Frank van der Linden wrote:
> commit 079096f103fa ("tcp/dccp: install syn_recv requests into ehash
> table") introduced an optimization for the handling of child sockets
> created for a new TCP connection.
>
> But this optimization passes any data associated with the last ACK of the
> connection handshake up the stack without verifying its checksum, because it
> calls tcp_child_process(), which in turn calls tcp_rcv_state_process()
> directly. These lower-level processing functions do not do any checksum
> verification.
>
> Insert a tcp_checksum_complete call in the TCP_NEW_SYN_RECEIVE path to
> fix this.
>
> Signed-off-by: Frank van der Linden <fllinden@amazon.com>
This is way too complicated.
You should call tcp_checksum_complete() earlier and avoid all this mess.
IPV4 part shown here :
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index fed3f1c6616708997f621535efe9412e4afa0a50..7b5f32aa3835b0124b0a9bd342c371df7b46f471 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1730,6 +1730,10 @@ int tcp_v4_rcv(struct sk_buff *skb)
reqsk_put(req);
goto discard_it;
}
+ if (unlikely(tcp_checksum_complete(skb))) {
+ reqsk_put(req);
+ goto csum_error;
+ }
if (unlikely(sk->sk_state != TCP_LISTEN)) {
inet_csk_reqsk_queue_drop_and_put(sk, req);
goto lookup;
^ permalink raw reply related
* Re: [PATCH] tcp: verify the checksum of the first data segment in a new connection
From: van der Linden, Frank @ 2018-06-12 21:44 UTC (permalink / raw)
To: Eric Dumazet, edumazet@google.com, netdev@vger.kernel.org
In-Reply-To: <4A996A47-0BA5-4880-BDAD-05037407F1B9@amazon.com>
Resubmitted. The various release/deref requirements in that path make a straight "goto csum_error" impossible without duplicating some lines, but this is 2nd best.
Frank
On 6/11/18, 4:43 PM, "van der Linden, Frank" <fllinden@amazon.com> wrote:
Yeah, true, it's missing INERRS in this case. I'll fix it up a bit.
Frank
On 6/11/18, 4:38 PM, "Eric Dumazet" <eric.dumazet@gmail.com> wrote:
On 06/11/2018 04:25 PM, van der Linden, Frank wrote:
> A few comments on this one:
>
> - obviously this is fairly serious, as it can let corrupted data all the way up to the application
Sure, although anyone relying on CRC checksum for ensuring TCP data integrity
has big troubles ;)
I would rather have a refined version of this patch doing a "goto csum_error"
so that we properly increment TCP_MIB_CSUMERRORS and TCP_MIB_INERRS
Thanks !
^ permalink raw reply
* [PATCH v2] tcp: verify the checksum of the first data segment in a new connection
From: Frank van der Linden @ 2018-06-12 21:41 UTC (permalink / raw)
To: edumazet, netdev
commit 079096f103fa ("tcp/dccp: install syn_recv requests into ehash
table") introduced an optimization for the handling of child sockets
created for a new TCP connection.
But this optimization passes any data associated with the last ACK of the
connection handshake up the stack without verifying its checksum, because it
calls tcp_child_process(), which in turn calls tcp_rcv_state_process()
directly. These lower-level processing functions do not do any checksum
verification.
Insert a tcp_checksum_complete call in the TCP_NEW_SYN_RECEIVE path to
fix this.
Signed-off-by: Frank van der Linden <fllinden@amazon.com>
---
net/ipv4/tcp_ipv4.c | 10 +++++++++-
net/ipv6/tcp_ipv6.c | 10 +++++++++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index f70586b..f361cf9 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1639,6 +1639,7 @@ int tcp_v4_rcv(struct sk_buff *skb)
const struct iphdr *iph;
const struct tcphdr *th;
bool refcounted;
+ bool csumerr = false;
struct sock *sk;
int ret;
@@ -1703,7 +1704,12 @@ int tcp_v4_rcv(struct sk_buff *skb)
th = (const struct tcphdr *)skb->data;
iph = ip_hdr(skb);
tcp_v4_fill_cb(skb, iph, th);
- nsk = tcp_check_req(sk, skb, req, false, &req_stolen);
+
+ csumerr = tcp_checksum_complete(skb);
+ if (!csumerr) {
+ nsk = tcp_check_req(sk, skb, req, false,
+ &req_stolen);
+ }
}
if (!nsk) {
reqsk_put(req);
@@ -1798,6 +1804,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
sk_drops_add(sk, skb);
if (refcounted)
sock_put(sk);
+ if (csumerr)
+ goto csum_error;
goto discard_it;
do_time_wait:
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6d664d8..17a20fa 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1425,6 +1425,7 @@ static int tcp_v6_rcv(struct sk_buff *skb)
const struct tcphdr *th;
const struct ipv6hdr *hdr;
bool refcounted;
+ bool csumerr = false;
struct sock *sk;
int ret;
struct net *net = dev_net(skb->dev);
@@ -1486,7 +1487,12 @@ static int tcp_v6_rcv(struct sk_buff *skb)
th = (const struct tcphdr *)skb->data;
hdr = ipv6_hdr(skb);
tcp_v6_fill_cb(skb, hdr, th);
- nsk = tcp_check_req(sk, skb, req, false, &req_stolen);
+
+ csumerr = tcp_checksum_complete(skb);
+ if (!csumerr) {
+ nsk = tcp_check_req(sk, skb, req, false,
+ &req_stolen);
+ }
}
if (!nsk) {
reqsk_put(req);
@@ -1577,6 +1583,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
sk_drops_add(sk, skb);
if (refcounted)
sock_put(sk);
+ if (csumerr)
+ goto csum_error;
goto discard_it;
do_time_wait:
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] Revert "net: do not allow changing SO_REUSEADDR/SO_REUSEPORT on bound sockets"
From: Maciej Żenczykowski @ 2018-06-12 21:40 UTC (permalink / raw)
To: David Miller; +Cc: bart.vanassche, Linux NetDev, Eric Dumazet
In-Reply-To: <20180612.111031.377845938725821301.davem@davemloft.net>
Any ideas about how to fix the core issue of tb->fast* being
effectively invalid?
ie. currently any reader of tb->fastreuse(port) which isn't simply
testing for it being >= 0 is basically a bug (-1 is the empty tb case,
so that AFAICT keeps on working).
For example sk_reuseport_match(tb, sk) can both fail to match when it
should, and can match when it shouldn't...
(at a quick glance, all the readers, and thus bugs are constrained to
the inet_csk_get_port() function)
Do we just delete that entire 'tb->fast*' optimization? It would
certainly make the code much simpler...
Do we put special case per-family/protocol code (ie. presumably
another indirect call) to fix up tb->fast in the
setsockopt(SOREUSEADDR/PORT) codepath?
Something else?
(btw. I'm not certain if both 0->1 and 1->0 transitions on a bound
socket are equally buggy, I think one is more dangerous then the
other)
^ permalink raw reply
* Re: Problems in tc-matchall.8, tc-sample.8
From: Stephen Hemminger @ 2018-06-12 21:17 UTC (permalink / raw)
To: esr; +Cc: netdev
In-Reply-To: <20180612191614.916303A4F8E@snark.thyrsus.com>
On Tue, 12 Jun 2018 15:16:14 -0400 (EDT)
esr@thyrsus.com wrote:
> This is automatically generated email about markup problems in a man
> page for which you appear to be responsible. If you are not the right
> person or list, please tell me so I can correct my database.
>
> See http://catb.org/~esr/doclifter/bugs.html for details on how and
> why these patches were generated. Feel free to email me with any
> questions. Note: These patches do not change the modification date of
> any manual page. You may wish to do that by hand.
>
> I apologize if this message seems spammy or impersonal. The volume of
> markup bugs I am tracking is over five hundred - there is no real
> alternative to generating bugmail from a database and template.
>
> --
> Eric S. Raymond
Please resubmit as real patch with signed-off-by
^ permalink raw reply
* Re: [RFC PATCH 00/12] Enable PM hibernation on guest VMs
From: Konrad Rzeszutek Wilk @ 2018-06-12 21:09 UTC (permalink / raw)
To: Anchal Agarwal
Cc: tglx, mingo, hpa, x86, boris.ostrovsky, roger.pau, netdev, jgross,
xen-devel, linux-kernel, kamatam, fllinden, vallish, guruanb,
eduval, rjw, pavel, len.brown, linux-pm, cyberax
In-Reply-To: <20180612205619.28156-1-anchalag@amazon.com>
On Tue, Jun 12, 2018 at 08:56:07PM +0000, Anchal Agarwal wrote:
> Hello,
> I am sending out a series of patches that implements guest
> PM hibernation. These guests are running on xen hypervisor.
> The patches had been tested against mainstream kernel and latest
> xen version-4.11. EC2 instance hibernation feature is provided to
> the AWS EC2 customers. PM hibernation uses swap space where
> hibernation image is stored and restored from. I would like
> the community to review and provide some feedback on the patch
> series and if they look good, merge them into 4.17 kernel.
4.17 got released on Jun 3rd. Kind of hard to do a time-machine.
^ permalink raw reply
* [RFC PATCH 09/12] x86/xen: save and restore steal clock
From: Anchal Agarwal @ 2018-06-12 20:56 UTC (permalink / raw)
To: tglx, mingo, hpa, x86
Cc: boris.ostrovsky, konrad.wilk, roger.pau, netdev, jgross,
xen-devel, linux-kernel, kamatam, anchalag, fllinden, vallish,
guruanb, eduval, rjw, pavel, len.brown, linux-pm, cyberax
In-Reply-To: <20180612205619.28156-1-anchalag@amazon.com>
From: Munehisa Kamata <kamatam@amazon.com>
Save steal clock values of all present CPUs in the system core ops
suspend callbacks. Also, restore a boot CPU's steal clock in the system
core resume callback. For non-boot CPUs, restore after they're brought
up, because runstate info for non-boot CPUs are not active until then.
Signed-off-by: Munehisa Kamata <kamatam@amazon.com>
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Reviewed-by: Munehisa Kamata <kamatam@amazon.com>
Reviewed-by: Eduardo Valentin <eduval@amazon.com>
---
arch/x86/xen/suspend.c | 13 ++++++++++++-
arch/x86/xen/time.c | 3 +++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index 784c448..dae0f74 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -91,12 +91,20 @@ void xen_arch_suspend(void)
static int xen_syscore_suspend(void)
{
struct xen_remove_from_physmap xrfp;
- int ret;
+ int cpu, ret;
/* Xen suspend does similar stuffs in its own logic */
if (xen_suspend_mode_is_xen_suspend())
return 0;
+ for_each_present_cpu(cpu) {
+ /*
+ * Nonboot CPUs are already offline, but the last copy of
+ * runstate info is still accessible.
+ */
+ xen_save_steal_clock(cpu);
+ }
+
xrfp.domid = DOMID_SELF;
xrfp.gpfn = __pa(HYPERVISOR_shared_info) >> PAGE_SHIFT;
@@ -118,6 +126,9 @@ static void xen_syscore_resume(void)
pvclock_resume();
+ /* Nonboot CPUs will be resumed when they're brought up */
+ xen_restore_steal_clock(smp_processor_id());
+
gnttab_resume();
}
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index e0f1bcf..85f8534 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -523,6 +523,9 @@ static void xen_hvm_setup_cpu_clockevents(void)
{
int cpu = smp_processor_id();
xen_setup_runstate_info(cpu);
+ if (cpu)
+ xen_restore_steal_clock(cpu);
+
/*
* xen_setup_timer(cpu) - snprintf is bad in atomic context. Hence
* doing it xen_hvm_cpu_notify (which gets called by smp_init during
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 12/12] PM / hibernate: update the resume offset on SNAPSHOT_SET_SWAP_AREA
From: Anchal Agarwal @ 2018-06-12 20:56 UTC (permalink / raw)
To: tglx, mingo, hpa, x86
Cc: jgross, len.brown, eduval, vallish, netdev, fllinden, kamatam,
rjw, linux-kernel, anchalag, cyberax, pavel, linux-pm, xen-devel,
boris.ostrovsky, guruanb, roger.pau
In-Reply-To: <20180612205619.28156-1-anchalag@amazon.com>
From: Aleksei Besogonov <cyberax@amazon.com>
The SNAPSHOT_SET_SWAP_AREA is supposed to be used to set the hibernation
offset on a running kernel to enable hibernating to a swap file.
However, it doesn't actually update the swsusp_resume_block variable. As
a result, the hibernation fails at the last step (after all the data is
written out) in the validation of the swap signature in
mark_swapfiles().
Before this patch, the command line processing was the only place where
swsusp_resume_block was set.
Signed-off-by: Aleksei Besogonov <cyberax@amazon.com>
Signed-off-by: Munehisa Kamata <kamatam@amazon.com>
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Reviewed-by: Munehisa Kamata <kamatam@amazon.com>
Reviewed-by: Eduardo Valentin <eduval@amazon.com>
---
kernel/power/user.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/power/user.c b/kernel/power/user.c
index abd2255..b522a42 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -379,8 +379,12 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
if (swdev) {
offset = swap_area.offset;
data->swap = swap_type_of(swdev, offset, NULL);
- if (data->swap < 0)
+ if (data->swap < 0) {
error = -ENODEV;
+ } else {
+ swsusp_resume_device = swdev;
+ swsusp_resume_block = offset;
+ }
} else {
data->swap = -1;
error = -EINVAL;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related
* [RFC PATCH 11/12] x86/xen: close event channels for PIRQs in system core suspend callback
From: Anchal Agarwal @ 2018-06-12 20:56 UTC (permalink / raw)
To: tglx, mingo, hpa, x86
Cc: boris.ostrovsky, konrad.wilk, roger.pau, netdev, jgross,
xen-devel, linux-kernel, kamatam, anchalag, fllinden, vallish,
guruanb, eduval, rjw, pavel, len.brown, linux-pm, cyberax
In-Reply-To: <20180612205619.28156-1-anchalag@amazon.com>
From: Munehisa Kamata <kamatam@amazon.com>
Close event channels allocated for devices which are backed by PIRQ and
still active when suspending the system core. Normally, the devices are
emulated legacy devices, e.g. PS/2 keyboard, floppy controller and etc.
Without this, in PM hibernation, information about the event channel
remains in hibernation image, but there is no guarantee that the same
event channel numbers are assigned to the devices when restoring the
system. This may cause conflict like the following and prevent some
devices from being restored correctly.
[ 102.330821] ------------[ cut here ]------------
[ 102.333264] WARNING: CPU: 0 PID: 2324 at
drivers/xen/events/events_base.c:878 bind_evtchn_to_irq+0x88/0xf0
...
[ 102.348057] Call Trace:
[ 102.348057] [<ffffffff813001df>] dump_stack+0x63/0x84
[ 102.348057] [<ffffffff81071811>] __warn+0xd1/0xf0
[ 102.348057] [<ffffffff810718fd>] warn_slowpath_null+0x1d/0x20
[ 102.348057] [<ffffffff8139a1f8>] bind_evtchn_to_irq+0x88/0xf0
[ 102.348057] [<ffffffffa00cd420>] ? blkif_copy_from_grant+0xb0/0xb0 [xen_blkfront]
[ 102.348057] [<ffffffff8139a307>] bind_evtchn_to_irqhandler+0x27/0x80
[ 102.348057] [<ffffffffa00cc785>] talk_to_blkback+0x425/0xcd0 [xen_blkfront]
[ 102.348057] [<ffffffff811e0c8a>] ? __kmalloc+0x1ea/0x200
[ 102.348057] [<ffffffffa00ce84d>] blkfront_restore+0x2d/0x60 [xen_blkfront]
[ 102.348057] [<ffffffff813a0078>] xenbus_dev_restore+0x58/0x100
[ 102.348057] [<ffffffff813a1ff0>] ? xenbus_frontend_delayed_resume+0x20/0x20
[ 102.348057] [<ffffffff813a200e>] xenbus_dev_cond_restore+0x1e/0x30
[ 102.348057] [<ffffffff813f797e>] dpm_run_callback+0x4e/0x130
[ 102.348057] [<ffffffff813f7f17>] device_resume+0xe7/0x210
[ 102.348057] [<ffffffff813f7810>] ? pm_dev_dbg+0x80/0x80
[ 102.348057] [<ffffffff813f9374>] dpm_resume+0x114/0x2f0
[ 102.348057] [<ffffffff810c00cf>] hibernation_snapshot+0x15f/0x380
[ 102.348057] [<ffffffff810c0ac3>] hibernate+0x183/0x290
[ 102.348057] [<ffffffff810be1af>] state_store+0xcf/0xe0
[ 102.348057] [<ffffffff813020bf>] kobj_attr_store+0xf/0x20
[ 102.348057] [<ffffffff8127c88a>] sysfs_kf_write+0x3a/0x50
[ 102.348057] [<ffffffff8127c3bb>] kernfs_fop_write+0x10b/0x190
[ 102.348057] [<ffffffff81200008>] __vfs_write+0x28/0x120
[ 102.348057] [<ffffffff81200c19>] ? rw_verify_area+0x49/0xb0
[ 102.348057] [<ffffffff81200e62>] vfs_write+0xb2/0x1b0
[ 102.348057] [<ffffffff81202196>] SyS_write+0x46/0xa0
[ 102.348057] [<ffffffff81520cf7>] entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 102.423005] ---[ end trace b8d6718e22e2b107 ]---
[ 102.425031] genirq: Flags mismatch irq 6. 00000000 (blkif) vs. 00000000 (floppy)
Note that we don't explicitly re-allocate event channels for such
devices in the resume callback. Re-allocation will occur when PM core
re-enable IRQs for the devices at later point.
Signed-off-by: Munehisa Kamata <kamatam@amazon.com>
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Reviewed-by: Munehisa Kamata <kamatam@amazon.com>
Reviewed-by: Eduardo Valentin <eduval@amazon.com>
---
arch/x86/xen/suspend.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index dae0f74..affa63d 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -105,6 +105,8 @@ static int xen_syscore_suspend(void)
xen_save_steal_clock(cpu);
}
+ xen_shutdown_pirqs();
+
xrfp.domid = DOMID_SELF;
xrfp.gpfn = __pa(HYPERVISOR_shared_info) >> PAGE_SHIFT;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 10/12] xen/events: add xen_shutdown_pirqs helper function
From: Anchal Agarwal @ 2018-06-12 20:56 UTC (permalink / raw)
To: tglx, mingo, hpa, x86
Cc: boris.ostrovsky, konrad.wilk, roger.pau, netdev, jgross,
xen-devel, linux-kernel, kamatam, anchalag, fllinden, vallish,
guruanb, eduval, rjw, pavel, len.brown, linux-pm, cyberax
In-Reply-To: <20180612205619.28156-1-anchalag@amazon.com>
From: Munehisa Kamata <kamatam@amazon.com>
Add a simple helper function to "shutdown" active PIRQs, which actually
closes event channels but keeps related IRQ structures intact. PM
suspend/hibernation code will rely on this.
Signed-off-by: Munehisa Kamata <kamatam@amazon.com>
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Reviewed-by: Munehisa Kamata <kamatam@amazon.com>
Reviewed-by: Eduardo Valentin <eduval@amazon.com>
---
drivers/xen/events/events_base.c | 12 ++++++++++++
include/xen/events.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 762378f..88137c8 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1581,6 +1581,18 @@ void xen_irq_resume(void)
restore_pirqs();
}
+void xen_shutdown_pirqs(void)
+{
+ struct irq_info *info;
+
+ list_for_each_entry(info, &xen_irq_list_head, list) {
+ if (info->type != IRQT_PIRQ || !VALID_EVTCHN(info->evtchn))
+ continue;
+
+ shutdown_pirq(irq_get_irq_data(info->irq));
+ }
+}
+
static struct irq_chip xen_dynamic_chip __read_mostly = {
.name = "xen-dyn",
diff --git a/include/xen/events.h b/include/xen/events.h
index c3e6bc6..e4d5ccb 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -70,6 +70,7 @@ static inline void notify_remote_via_evtchn(int port)
void notify_remote_via_irq(int irq);
void xen_irq_resume(void);
+void xen_shutdown_pirqs(void);
/* Clear an irq's pending state, in preparation for polling on it */
void xen_clear_irq_pending(int irq);
--
2.7.4
^ 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