* Re: Problems in tc-matchall.8, tc-sample.8
From: Eric S. Raymond @ 2018-06-12 23:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20180612153350.75e77f01@xeon-e3>
Stephen Hemminger <stephen@networkplumber.org>:
> 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.
Then I should bugfix against this repository?
https://github.com/shemminger/iproute2
--
<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 23:12 UTC (permalink / raw)
To: Eric Dumazet, edumazet@google.com, netdev@vger.kernel.org
In-Reply-To: <212193c0-2fee-7f88-5473-9f5f4c548cb8@gmail.com>
Ok, patch v3 sent.
It was rightly pointed out to me that I shouldn't commit the mortal sin of top posting - but bear with me guys, I'll dig up my 25-year old .muttrc :-)
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
* [PATCH v3] tcp: verify the checksum of the first data segment in a new connection
From: Frank van der Linden @ 2018-06-12 23:09 UTC (permalink / raw)
To: edumazet, netdev; +Cc: fllinden
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 | 4 ++++
net/ipv6/tcp_ipv6.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index f70586b..ef8cd0f 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1689,6 +1689,10 @@ int tcp_v4_rcv(struct sk_buff *skb)
reqsk_put(req);
goto discard_it;
}
+ if (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;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6d664d8..5d4eb9d 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1475,6 +1475,10 @@ static int tcp_v6_rcv(struct sk_buff *skb)
reqsk_put(req);
goto discard_it;
}
+ if (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;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/1] ip: add rmnet initial support
From: Subash Abhinov Kasiviswanathan @ 2018-06-12 23:06 UTC (permalink / raw)
To: Daniele Palmas; +Cc: netdev, Stephen Hemminger
In-Reply-To: <1528812777-7512-1-git-send-email-dnlplm@gmail.com>
> +
> +static void print_explain(FILE *f)
> +{
> + fprintf(f,
> + "Usage: ... rmnet mux_id MUXID\n"
> + "\n"
> + "MUXID := 1-127\n"
> + );
> +}
Hi Daniele
This range can be from 1-254.
> +
> +static void explain(void)
> +{
> + print_explain(stderr);
> +}
> +
> +static int rmnet_parse_opt(struct link_util *lu, int argc, char
> **argv,
> + struct nlmsghdr *n)
> +{
> + __u16 mux_id;
> +
> + while (argc > 0) {
> + if (matches(*argv, "mux_id") == 0) {
> + NEXT_ARG();
> + if (get_u16(&mux_id, *argv, 0))
> + invarg("mux_id is invalid", *argv);
> + addattr_l(n, 1024, IFLA_RMNET_MUX_ID, &mux_id, 2);
You could use addattr16() instead since it is __u16.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ 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:47 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Alexander Duyck, intel-wired-lan, Netdev
In-Reply-To: <20180612105029.77b40381@xeon-e3>
On Tue, Jun 12, 2018 at 10:50 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue, 12 Jun 2018 11:18:25 -0400
> Alexander Duyck <alexander.h.duyck@intel.com> wrote:
>
>> This patch series is meant to allow support for the L2 forward offload, aka
>> MACVLAN offload without the need for using ndo_select_queue.
>>
>> The existing solution currently requires that we use ndo_select_queue in
>> the transmit path if we want to associate specific Tx queues with a given
>> MACVLAN interface. In order to get away from this we need to repurpose the
>> tc_to_txq array and XPS pointer for the MACVLAN interface and use those as
>> a means of accessing the queues on the lower device. As a result we cannot
>> offload a device that is configured as multiqueue, however it doesn't
>> really make sense to configure a macvlan interfaced as being multiqueue
>> anyway since it doesn't really have a qdisc of its own in the first place.
>>
>> I am submitting this as an RFC for the netdev mailing list, and officially
>> submitting it for testing to Jeff Kirsher's next-queue in order to validate
>> the ixgbe specific bits.
>>
>> The big changes in this set are:
>> Allow lower device to update tc_to_txq and XPS map of offloaded MACVLAN
>> Disable XPS for single queue devices
>> Replace accel_priv with sb_dev in ndo_select_queue
>> Add sb_dev parameter to fallback function for ndo_select_queue
>> Consolidated ndo_select_queue functions that appeared to be duplicates
>>
>> v2: Implement generic "select_queue" functions instead of "fallback" functions.
>> Tweak last two patches to account for changes in dev_pick_tx_xxx functions.
>>
>> ---
>>
>> Alexander Duyck (7):
>> net-sysfs: Drop support for XPS and traffic_class on single queue device
>> net: Add support for subordinate device traffic classes
>> ixgbe: Add code to populate and use macvlan tc to Tx queue map
>> net: Add support for subordinate traffic classes to netdev_pick_tx
>> net: Add generic ndo_select_queue functions
>> net: allow ndo_select_queue to pass netdev
>> net: allow fallback function to pass netdev
>>
>>
>> drivers/infiniband/hw/hfi1/vnic_main.c | 2
>> drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c | 4 -
>> drivers/net/bonding/bond_main.c | 3
>> drivers/net/ethernet/amazon/ena/ena_netdev.c | 5 -
>> drivers/net/ethernet/broadcom/bcmsysport.c | 6 -
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 6 +
>> drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h | 3
>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 5 -
>> drivers/net/ethernet/hisilicon/hns/hns_enet.c | 5 -
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 62 ++++++--
>> drivers/net/ethernet/lantiq_etop.c | 10 -
>> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 7 +
>> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3
>> drivers/net/ethernet/mellanox/mlx5/core/en.h | 3
>> drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 5 -
>> drivers/net/ethernet/renesas/ravb_main.c | 3
>> drivers/net/ethernet/sun/ldmvsw.c | 3
>> drivers/net/ethernet/sun/sunvnet.c | 3
>> drivers/net/ethernet/ti/netcp_core.c | 9 -
>> drivers/net/hyperv/netvsc_drv.c | 6 -
>> drivers/net/macvlan.c | 10 -
>> drivers/net/net_failover.c | 7 +
>> drivers/net/team/team.c | 3
>> drivers/net/tun.c | 3
>> drivers/net/wireless/marvell/mwifiex/main.c | 3
>> drivers/net/xen-netback/interface.c | 4 -
>> drivers/net/xen-netfront.c | 3
>> drivers/staging/netlogic/xlr_net.c | 9 -
>> drivers/staging/rtl8188eu/os_dep/os_intfs.c | 3
>> drivers/staging/rtl8723bs/os_dep/os_intfs.c | 7 -
>> include/linux/netdevice.h | 34 ++++-
>> net/core/dev.c | 156 ++++++++++++++++++---
>> net/core/net-sysfs.c | 36 ++++-
>> net/mac80211/iface.c | 4 -
>> net/packet/af_packet.c | 7 +
>> 35 files changed, 312 insertions(+), 130 deletions(-)
>>
>> --
>
> This makes sense. I thought you were hoping to get rid of select queue in future?
That would be nice, however there are still a bunch of corner cases
that are not handled that have been dumped into select queue. For
example in the case of ixgbe the issue is FCoE. There are a number of
other places that are using it as well as I seem to recall netvsc and
bonding both use it to store off the original Rx->Tx queue mapping
when passing through the interface.
For now I figure we can take this one hill at a time and I am just
making it so we don't have to use ndo_select_queue in order to make
vmdq work for macvlan offload.
- Alex
^ permalink raw reply
* Re: [PATCH v2] xen/netfront: raise max number of slots in xennet_get_responses()
From: David Miller @ 2018-06-12 22:43 UTC (permalink / raw)
To: jgross; +Cc: linux-kernel, xen-devel, netdev, boris.ostrovsky
In-Reply-To: <20180612065753.10569-1-jgross@suse.com>
From: Juergen Gross <jgross@suse.com>
Date: Tue, 12 Jun 2018 08:57:53 +0200
> The max number of slots used in xennet_get_responses() is set to
> MAX_SKB_FRAGS + (rx->status <= RX_COPY_THRESHOLD).
>
> In old kernel-xen MAX_SKB_FRAGS was 18, while nowadays it is 17. This
> difference is resulting in frequent messages "too many slots" and a
> reduced network throughput for some workloads (factor 10 below that of
> a kernel-xen based guest).
>
> Replacing MAX_SKB_FRAGS by XEN_NETIF_NR_SLOTS_MIN for calculation of
> the max number of slots to use solves that problem (tests showed no
> more messages "too many slots" and throughput was as high as with the
> kernel-xen based guest system).
>
> Replace MAX_SKB_FRAGS-2 by XEN_NETIF_NR_SLOTS_MIN-1 in
> netfront_tx_slot_available() for making it clearer what is really being
> tested without actually modifying the tested value.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Applied, thanks.
^ permalink raw reply
* 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
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