* Re: [RFC] nasty corner case in unix_dgram_sendmsg()
From: Rainer Weikusat @ 2019-02-26 15:31 UTC (permalink / raw)
To: Al Viro; +Cc: Jason Baron, netdev
In-Reply-To: <20190226063804.GI2217@ZenIV.linux.org.uk>
Al Viro <viro@zeniv.linux.org.uk> writes:
> On Tue, Feb 26, 2019 at 06:28:17AM +0000, Al Viro wrote:
[...]
>> * if after relocking we see that unix_peer(sk) now
>> is equal to other, we arrange for wakeup forwarding from other's
>> peer_wait *and* if that has (likely) succeeded we fail with -EAGAIN.
>> Huh?
This returns 1 if sending isn't possible at the moment, ie, if the
process which tries to send has to wait.
^ permalink raw reply
* Re: [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Stephen Hemminger @ 2019-02-26 15:50 UTC (permalink / raw)
To: Leslie Monis
Cc: davem, netdev, Mohit P . Tahiliani, Dave Taht, Jamal Hadi Salim
In-Reply-To: <20190226082046.GA2266@Inspiron-3521>
On Tue, 26 Feb 2019 13:50:46 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:
> On Mon, Feb 25, 2019 at 04:38:11PM -0800, Stephen Hemminger wrote:
> > On Tue, 26 Feb 2019 00:39:54 +0530
> > Leslie Monis <lesliemonis@gmail.com> wrote:
> >
> > > The current implementation of the PIE queuing discipline is according to the
> > > IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> > > [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> > > However, a lot of necessary modifications and enhancements have been proposed
> > > in RFC 8033, which have not yet been incorporated in the source code of Linux.
> > > This patch series helps in achieving the same.
> > >
> > > Performance tests carried out using Flent [https://flent.org/]
> > >
> > > Changes from v2 to v3:
> > > - Used div_u64() instead of direct division after explicit type casting as
> > > recommended by David
> > >
> > > Changes from v1 to v2:
> > > - Excluded the patch setting PIE dynamically active/inactive as the test
> > > results were unsatisfactory
> > > - Fixed a scaling issue when adding more auto-tuning cases which caused
> > > local variables to underflow
> > > - Changed the long if/else chain to a loop as suggested by Stephen
> > > - Changed the position of the accu_prob variable in the pie_vars
> > > structure as recommended by Stephen
> > >
> > > Mohit P. Tahiliani (7):
> > > net: sched: pie: change value of QUEUE_THRESHOLD
> > > net: sched: pie: change default value of pie_params->target
> > > net: sched: pie: change default value of pie_params->tupdate
> > > net: sched: pie: change initial value of pie_vars->burst_time
> > > net: sched: pie: add more cases to auto-tune alpha and beta
> > > net: sched: pie: add derandomization mechanism
> > > net: sched: pie: update references
> > >
> > > include/uapi/linux/pkt_sched.h | 2 +-
> > > net/sched/sch_pie.c | 107 ++++++++++++++++++++-------------
> > > 2 files changed, 66 insertions(+), 43 deletions(-)
> >
> > Are you concerned at all that changes to default values might change
> > expected behavior of existing users?
>
> Hi Stephen,
>
> As Dave mentioned, the changes which we have made do not really change the
> behaviour of the aqm drastically. Our performance tests show that these changes
> improve performance without any side-effects. So existing users (if there are
> any) should not be negatively affected in any way.
Thanks for answering. Looks good
^ permalink raw reply
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Eric Dumazet @ 2019-02-26 15:52 UTC (permalink / raw)
To: Sheng Lan, Stephen Hemminger
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <05fa74b9-6e3b-7bd3-fa1c-c02e37a521f8@huawei.com>
On 02/26/2019 05:02 AM, Sheng Lan wrote:
>
>
>
>> On Mon, 25 Feb 2019 22:49:39 +0800
>> Sheng Lan <lansheng@huawei.com> wrote:
>>
>>> From: Sheng Lan <lansheng@huawei.com>
>>> Subject: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
>>>
>>> It can be reproduced by following steps:
>>> 1. virtio_net NIC is configured with gso/tso on
>>> 2. configure nginx as http server with an index file bigger than 1M bytes
>>> 3. use tc netem to produce duplicate packets and delay:
>>> tc qdisc add dev eth0 root netem delay 100ms 10ms 30% duplicate 90%
>>> 4. continually curl the nginx http server to get index file on client
>>> 5. BUG_ON is seen quickly
>>>
>>> [10258690.371129] kernel BUG at net/core/skbuff.c:4028!
>>> [10258690.371748] invalid opcode: 0000 [#1] SMP PTI
>>> [10258690.372094] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G W 5.0.0-rc6 #2
>>> [10258690.372094] RSP: 0018:ffffa05797b43da0 EFLAGS: 00010202
>>> [10258690.372094] RBP: 00000000000005ea R08: 0000000000000000 R09: 00000000000005ea
>>> [10258690.372094] R10: ffffa0579334d800 R11: 00000000000002c0 R12: 0000000000000002
>>> [10258690.372094] R13: 0000000000000000 R14: ffffa05793122900 R15: ffffa0578f7cb028
>>> [10258690.372094] FS: 0000000000000000(0000) GS:ffffa05797b40000(0000) knlGS:0000000000000000
>>> [10258690.372094] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [10258690.372094] CR2: 00007f1a6dc00868 CR3: 000000001000e000 CR4: 00000000000006e0
>>> [10258690.372094] Call Trace:
>>> [10258690.372094] <IRQ>
>>> [10258690.372094] skb_to_sgvec+0x11/0x40
>>> [10258690.372094] start_xmit+0x38c/0x520 [virtio_net]
>>> [10258690.372094] dev_hard_start_xmit+0x9b/0x200
>>> [10258690.372094] sch_direct_xmit+0xff/0x260
>>> [10258690.372094] __qdisc_run+0x15e/0x4e0
>>> [10258690.372094] net_tx_action+0x137/0x210
>>> [10258690.372094] __do_softirq+0xd6/0x2a9
>>> [10258690.372094] irq_exit+0xde/0xf0
>>> [10258690.372094] smp_apic_timer_interrupt+0x74/0x140
>>> [10258690.372094] apic_timer_interrupt+0xf/0x20
>>> [10258690.372094] </IRQ>
>>>
>>> In __skb_to_sgvec, the skb->len is not equal to the sum of the skb's
>>> linear data size and nonlinear data size, thus BUG_ON triggered. The
>>> bad skb's nonlinear data size is less than skb->data_len, because the
>>> skb is cloned and a part of related cloned skb's nonlinear data is
>>> split off.
>>>
>>> Duplicate packet is cloned by skb_clone in netem_enqueue and may be delayed
>>> some time in qdisc. Due to the delay time, the original skb will be pushed
>>> again later in __tcp_push_pending_frames when tcp receives new packets.
>>> In tcp_write_xmit, when the tcp_mss_split_point returns a smaller limit,
>>> the original skb will be fragmented and the skb's nonlinear data will be
>>> split off. The length of the skb cloned by netem will not be updated.
>>> When we use virtio_net NIC, the duplicated cloned skb will be filled into
>>> a scatter-gather list in __skb_to_sgvec and trigger the BUG_ON.
>>>
>>> Here I replace the skb_clone with skb_copy in netem_enqueue to ensure
>>> the duplicated skb's nonlinear data is independent.
>>>
>>> Signed-off-by: Sheng Lan <lansheng@huawei.com>
>>> Reported-by: Qin Ji <jiqin.ji@huawei.com>
>>>
>>> Fixes: 0afb51e7 ("netem: reinsert for duplication")
>>
>> This sounds like a bug in the other layers (either TCP or Virtio net)
>> not handling a cloned skb properly.
>>
>
> I have traced the route of skb by printk, let me take an example to describe the problem to make it clearly:
> Mss value equals to 1448. Limit value is the split size when tcp do tso_fragment, is depending on the size of the sending congestion window and mss value.
>
> TCP layer transmit the index file to client, the original skb1 size is large:
> ...
> tcp_write_xmit (skb1->data_len == 62264, limit == 2*mss == 2896)
> tso_fragment (it needs to be fragmented by limit value)
> skb_split (after split, skb1->data_len == 2896, skb_shinfo(skb1)->frags[0] == 2896, skb_shinfo(skb1)->nr_frags == 1)
> ...
> netem_enqueue (netem construct a duplicate packet of skb1 by skb_clone)
> skb2 = skb_clone(skb1) (skb1->data_len == skb2->data_len == 2896, skb1 and skb2 share the nonlinear data frags[0] == 2896)
> waiting 30ms (skb1 and skb2 will be delayed in qdisc queue due to the netem delay configuration)
>
>
> TCP layer receives new packets and trys to retransmit the skb1:
> tcp_rcv_established
> __tcp_push_pending_frames
> tcp_write_xmit (skb1->data_len == 2896, cwnd size decreased or packets in flight increased, cause the limit decreased to 1*mss == 1448)
tcp_write_xmit() only deals with packet in the write queue,
they never were sent. They can not be any clone of them by definition, since
skbs in the TCP write queue are private to TCP stack,
Once a packet is sent, the master skb is moved to the rtx rb-tree,
while the clone is going through lower stacks.
When/if a retransmit is due, we always make sure there is no clone on it,
look at the various calls to skb_unclone()
> tso_fragment (limit value is less than skb1->data_len, skb1 will be fragmented again)
> skb_split (the second time split, skb1 is cloned now and share nonlinear data with skb2.
> skb1->data_len == 1448, frags[0] == 1448, a part of shared nonlinear data has been split off)
>
> Now we can see:
> skb1->data_len == 1448
> skb2->data_len == 2896 (2896 is wrong value)
> frags[0] == 1448
>
>
> The route of skb2:
> netem_enqueue (netem construct a duplicate packet, skb2 = skb_clone(skb1), put skb2 into queue)
> waiting 30ms (delayed packet)
> ...
> netem_dequeue (skb2->data_len == 2896, frags[0] == 1448)
> sch_direct_xmit
> dev_hard_start_xmit
> xmit_one
> start_xmit [virtio_net driver]
> skb_to_sgvec (BUG_ON here )
>
>
> The key is that skb be split by skb_split in tso_fragment again after netem clone it and share nonlinear data with another skb.
> Processing of TCP seems OK, which push and fragment delayed packets in write queue. And virtio_net is the trigger of the BUG_ON.
> So I replaced skb_clone with skb_copy in netem_enqueue, and the method worked. Currently, I have no better idea to fix it,
> would you give me some inspiring advice ? If I am wrong, please correct me.
>
> Thanks
>
^ permalink raw reply
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Stephen Hemminger @ 2019-02-26 15:59 UTC (permalink / raw)
To: Sheng Lan
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <05fa74b9-6e3b-7bd3-fa1c-c02e37a521f8@huawei.com>
On Tue, 26 Feb 2019 21:02:10 +0800
Sheng Lan <lansheng@huawei.com> wrote:
> > On Mon, 25 Feb 2019 22:49:39 +0800
> > Sheng Lan <lansheng@huawei.com> wrote:
> >
> >> From: Sheng Lan <lansheng@huawei.com>
> >> Subject: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
> >>
> >> It can be reproduced by following steps:
> >> 1. virtio_net NIC is configured with gso/tso on
> >> 2. configure nginx as http server with an index file bigger than 1M bytes
> >> 3. use tc netem to produce duplicate packets and delay:
> >> tc qdisc add dev eth0 root netem delay 100ms 10ms 30% duplicate 90%
> >> 4. continually curl the nginx http server to get index file on client
> >> 5. BUG_ON is seen quickly
> >>
> >> [10258690.371129] kernel BUG at net/core/skbuff.c:4028!
> >> [10258690.371748] invalid opcode: 0000 [#1] SMP PTI
> >> [10258690.372094] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G W 5.0.0-rc6 #2
> >> [10258690.372094] RSP: 0018:ffffa05797b43da0 EFLAGS: 00010202
> >> [10258690.372094] RBP: 00000000000005ea R08: 0000000000000000 R09: 00000000000005ea
> >> [10258690.372094] R10: ffffa0579334d800 R11: 00000000000002c0 R12: 0000000000000002
> >> [10258690.372094] R13: 0000000000000000 R14: ffffa05793122900 R15: ffffa0578f7cb028
> >> [10258690.372094] FS: 0000000000000000(0000) GS:ffffa05797b40000(0000) knlGS:0000000000000000
> >> [10258690.372094] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> [10258690.372094] CR2: 00007f1a6dc00868 CR3: 000000001000e000 CR4: 00000000000006e0
> >> [10258690.372094] Call Trace:
> >> [10258690.372094] <IRQ>
> >> [10258690.372094] skb_to_sgvec+0x11/0x40
> >> [10258690.372094] start_xmit+0x38c/0x520 [virtio_net]
> >> [10258690.372094] dev_hard_start_xmit+0x9b/0x200
> >> [10258690.372094] sch_direct_xmit+0xff/0x260
> >> [10258690.372094] __qdisc_run+0x15e/0x4e0
> >> [10258690.372094] net_tx_action+0x137/0x210
> >> [10258690.372094] __do_softirq+0xd6/0x2a9
> >> [10258690.372094] irq_exit+0xde/0xf0
> >> [10258690.372094] smp_apic_timer_interrupt+0x74/0x140
> >> [10258690.372094] apic_timer_interrupt+0xf/0x20
> >> [10258690.372094] </IRQ>
> >>
> >> In __skb_to_sgvec, the skb->len is not equal to the sum of the skb's
> >> linear data size and nonlinear data size, thus BUG_ON triggered. The
> >> bad skb's nonlinear data size is less than skb->data_len, because the
> >> skb is cloned and a part of related cloned skb's nonlinear data is
> >> split off.
> >>
> >> Duplicate packet is cloned by skb_clone in netem_enqueue and may be delayed
> >> some time in qdisc. Due to the delay time, the original skb will be pushed
> >> again later in __tcp_push_pending_frames when tcp receives new packets.
> >> In tcp_write_xmit, when the tcp_mss_split_point returns a smaller limit,
> >> the original skb will be fragmented and the skb's nonlinear data will be
> >> split off. The length of the skb cloned by netem will not be updated.
> >> When we use virtio_net NIC, the duplicated cloned skb will be filled into
> >> a scatter-gather list in __skb_to_sgvec and trigger the BUG_ON.
> >>
> >> Here I replace the skb_clone with skb_copy in netem_enqueue to ensure
> >> the duplicated skb's nonlinear data is independent.
> >>
> >> Signed-off-by: Sheng Lan <lansheng@huawei.com>
> >> Reported-by: Qin Ji <jiqin.ji@huawei.com>
> >>
> >> Fixes: 0afb51e7 ("netem: reinsert for duplication")
> >
> > This sounds like a bug in the other layers (either TCP or Virtio net)
> > not handling a cloned skb properly.
> >
>
> I have traced the route of skb by printk, let me take an example to describe the problem to make it clearly:
> Mss value equals to 1448. Limit value is the split size when tcp do tso_fragment, is depending on the size of the sending congestion window and mss value.
>
> TCP layer transmit the index file to client, the original skb1 size is large:
> ...
> tcp_write_xmit (skb1->data_len == 62264, limit == 2*mss == 2896)
> tso_fragment (it needs to be fragmented by limit value)
> skb_split (after split, skb1->data_len == 2896, skb_shinfo(skb1)->frags[0] == 2896, skb_shinfo(skb1)->nr_frags == 1)
> ...
> netem_enqueue (netem construct a duplicate packet of skb1 by skb_clone)
> skb2 = skb_clone(skb1) (skb1->data_len == skb2->data_len == 2896, skb1 and skb2 share the nonlinear data frags[0] == 2896)
> waiting 30ms (skb1 and skb2 will be delayed in qdisc queue due to the netem delay configuration)
>
>
> TCP layer receives new packets and trys to retransmit the skb1:
> tcp_rcv_established
> __tcp_push_pending_frames
> tcp_write_xmit (skb1->data_len == 2896, cwnd size decreased or packets in flight increased, cause the limit decreased to 1*mss == 1448)
> tso_fragment (limit value is less than skb1->data_len, skb1 will be fragmented again)
Maybe the fix is to stop TSO fragment from overwriting by doing something like:
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 730bc44dbad9..5fe91d0224f6 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1856,7 +1856,7 @@ static int tso_fragment(struct sock *sk, enum tcp_queue tcp_queue,
u8 flags;
/* All of a TSO frame must be composed of paged data. */
- if (skb->len != skb->data_len)
+ if (skb->len != skb->data_len || skb_cloned(skb))
return tcp_fragment(sk, tcp_queue, skb, len, mss_now, gfp);
buff = sk_stream_alloc_skb(sk, 0, gfp, true);
^ permalink raw reply related
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Eric Dumazet @ 2019-02-26 16:08 UTC (permalink / raw)
To: Stephen Hemminger, Sheng Lan
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <20190226075906.103fa072@shemminger-XPS-13-9360>
On 02/26/2019 07:59 AM, Stephen Hemminger wrote:
>
>
> Maybe the fix is to stop TSO fragment from overwriting by doing something like:
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 730bc44dbad9..5fe91d0224f6 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1856,7 +1856,7 @@ static int tso_fragment(struct sock *sk, enum tcp_queue tcp_queue,
> u8 flags;
>
> /* All of a TSO frame must be composed of paged data. */
> - if (skb->len != skb->data_len)
> + if (skb->len != skb->data_len || skb_cloned(skb))
> return tcp_fragment(sk, tcp_queue, skb, len, mss_now, gfp);
>
> buff = sk_stream_alloc_skb(sk, 0, gfp, true);
>
tso_fragment() is only called with packets that were not yet transmit.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: don't release block->lock when dumping chains
From: Vlad Buslov @ 2019-02-26 16:09 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <CAM_iQpXQXMxWDTcFvEEXra_1uKujhbeb630_kPfBqO1R8uwYtA@mail.gmail.com>
On Tue 26 Feb 2019 at 00:15, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Feb 25, 2019 at 7:45 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Function tc_dump_chain() obtains and releases block->lock on each iteration
>> of its inner loop that dumps all chains on block. Outputting chain template
>> info is fast operation so locking/unlocking mutex multiple times is an
>> overhead when lock is highly contested. Modify tc_dump_chain() to only
>> obtain block->lock once and dump all chains without releasing it.
>>
>> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
>> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> Thanks for the followup!
>
> Isn't it similar for __tcf_get_next_proto() in tcf_chain_dump()?
> And for tc_dump_tfilter()?
Not really. These two dump all tp filters and not just a template, which
is O(n) on number of filters and can be slow because it calls hw offload
API for each of them. Our typical use-case involves periodic filter dump
(to update stats) while multiple concurrent user-space threads are
updating filters, so it is important for them to be able to execute in
parallel.
^ permalink raw reply
* Re: linux-next: Tree for Feb 26 (net/sched/sch_pie.c)
From: Randy Dunlap @ 2019-02-26 16:21 UTC (permalink / raw)
To: Stephen Rothwell, Linux Next Mailing List
Cc: Linux Kernel Mailing List, netdev@vger.kernel.org,
Vijay Subramanian, Mythili Prabhu
In-Reply-To: <20190226190859.624e21f0@canb.auug.org.au>
On 2/26/19 12:08 AM, Stephen Rothwell wrote:
> Hi all,
>
> Changes since 20190225:
>
on i386:
ld: net/sched/sch_pie.o: in function `pie_timer':
sch_pie.c:(.text+0x604): undefined reference to `__udivdi3'
--
~Randy
^ permalink raw reply
* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Ard Biesheuvel @ 2019-02-26 16:21 UTC (permalink / raw)
To: Marc Zyngier
Cc: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu, Devicetree List, <netdev@vger.kernel.org>,
<linux-wireless@vger.kernel.org>, Linux Kernel Mailing List,
linux-rockchip, David S. Miller, linux-arm-kernel
In-Reply-To: <5310b73b-4821-6dff-b9c0-34c59fb7fd72@arm.com>
On Mon, 25 Feb 2019 at 15:53, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> Hi Ard,
>
> On 25/02/2019 12:45, Ard Biesheuvel wrote:
> > On Sun, 24 Feb 2019 at 15:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>
> >> For quite some time, I wondered why the PCI mwifiex device built in my
> >> Chromebook was unable to use the good old legacy interrupts. But as MSIs
> >> were working fine, I never really bothered investigating. I finally had a
> >> look, and the result isn't very pretty.
> >>
> >> On this machine (rk3399-based kevin), the wake-up interrupt is described as
> >> such:
> >>
> >> &pci_rootport {
> >> mvl_wifi: wifi@0,0 {
> >> compatible = "pci1b4b,2b42";
> >> reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
> >> 0x83010000 0x0 0x00100000 0x0 0x00100000>;
> >> interrupt-parent = <&gpio0>;
> >> interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> >> pinctrl-names = "default";
> >> pinctrl-0 = <&wlan_host_wake_l>;
> >> wakeup-source;
> >> };
> >> };
> >>
> >> Note how the interrupt is part of the properties directly attached to the
> >> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
> >> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
> >> altogether (Yay for the broken design!). This is in total violation of the
> >> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
> >> specifiers describe the PCI device interrupts, and must obey the
> >> INT-{A,B,C,D} mapping. Oops!
> >>
> >
> > The mapping of legacy PCIe INTx interrupts onto wired system
> > interrupts is a property of the PCIe host controller, not of a
> > particular PCIe device. So I would argue that the code is broken here
> > as well: it should never attempt to interpret 'interrupt' properties
> > at the PCI device level as having any bearing on how legacy interrupts
> > are routed.
>
> OpenFirmware says that this node contains the interrupt number of the
> device (4.1.1. Open Firmware-defined Properties for Child Nodes). The
> trick is that this property is generated *from* the device, and not set
> in stone.
>
> DT, on the other hand, takes whatever is described there and uses it as
> the gospel to configure the OS, no matter how the PCI device is actually
> configured. If the two don't match (like in this case), things break.
> This is the "DT describes the HW" mantra, for (sometimes) better or
> (more generally) worse.
>
> What the DT code does is to interpret the whole interrupt specifier,
> *including the interrupt-parent*. And that feels wrong. It should always
> be in the context of the host controller. But on the other side, the DT
> code is not in the business of validating the DT either...
>
> It outlines one thing: If you have to interpret per-device PCI
> properties from DT, you're in for serious trouble. I should get some
> better HW.
>
Yeah, it obviously makes no sense at all for the interrupt parent of a
PCI device to deviate from the host bridge's interrupt parent, and
it's quite unfortunate that we can't simply ban it now that the cat is
out of the bag already.
Arguably, the wake up widget is not part of the PCI device, but I have
no opinion as to whether it is better modeling it as a sub device as
you are proposing or as an entirely separate device referenced via a
phandle.
^ permalink raw reply
* Re: [PATCH net-next] net: remove unused struct inet_frag_queue.fragments field
From: Stefan Schmidt @ 2019-02-26 16:22 UTC (permalink / raw)
To: Peter Oskolkov, David Miller, netdev; +Cc: Peter Oskolkov, linux-wpan
In-Reply-To: <20190226014346.77719-1-posk@google.com>
Hello Peter.
On 26.02.19 02:43, Peter Oskolkov wrote:
> Now that all users of struct inet_frag_queue have been converted
> to use 'rb_fragments', remove the unused 'fragments' field.
>
> Build with `make allyesconfig` succeeded. ip_defrag selftest passed.
>
> Signed-off-by: Peter Oskolkov <posk@google.com>
> ---
> include/net/inet_frag.h | 4 +--
> net/ieee802154/6lowpan/reassembly.c | 1 -
> net/ipv4/inet_fragment.c | 44 ++++++++-----------------
> net/ipv4/ip_fragment.c | 2 --
> net/ipv6/netfilter/nf_conntrack_reasm.c | 1 -
> net/ipv6/reassembly.c | 1 -
> 6 files changed, 14 insertions(+), 39 deletions(-)
>
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index b02bf737d019..378904ee9129 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -56,7 +56,6 @@ struct frag_v6_compare_key {
> * @timer: queue expiration timer
> * @lock: spinlock protecting this frag
> * @refcnt: reference count of the queue
> - * @fragments: received fragments head
> * @rb_fragments: received fragments rb-tree root
> * @fragments_tail: received fragments tail
> * @last_run_head: the head of the last "run". see ip_fragment.c
> @@ -77,8 +76,7 @@ struct inet_frag_queue {
> struct timer_list timer;
> spinlock_t lock;
> refcount_t refcnt;
> - struct sk_buff *fragments; /* used in 6lopwpan IPv6. */
> - struct rb_root rb_fragments; /* Used in IPv4/IPv6. */
> + struct rb_root rb_fragments;
> struct sk_buff *fragments_tail;
> struct sk_buff *last_run_head;
> ktime_t stamp;
> diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
> index bd61633d2c32..4196bcd4105a 100644
> --- a/net/ieee802154/6lowpan/reassembly.c
> +++ b/net/ieee802154/6lowpan/reassembly.c
> @@ -179,7 +179,6 @@ static int lowpan_frag_reasm(struct lowpan_frag_queue *fq, struct sk_buff *skb,
>
> skb->dev = ldev;
> skb->tstamp = fq->q.stamp;
> - fq->q.fragments = NULL;
> fq->q.rb_fragments = RB_ROOT;
> fq->q.fragments_tail = NULL;
> fq->q.last_run_head = NULL;
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index 9f69411251d0..737808e27f8b 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -203,7 +203,6 @@ EXPORT_SYMBOL(inet_frag_rbtree_purge);
>
> void inet_frag_destroy(struct inet_frag_queue *q)
> {
> - struct sk_buff *fp;
> struct netns_frags *nf;
> unsigned int sum, sum_truesize = 0;
> struct inet_frags *f;
> @@ -212,20 +211,9 @@ void inet_frag_destroy(struct inet_frag_queue *q)
> WARN_ON(del_timer(&q->timer) != 0);
>
> /* Release all fragment data. */
> - fp = q->fragments;
> nf = q->net;
> f = nf->f;
> - if (fp) {
> - do {
> - struct sk_buff *xp = fp->next;
> -
> - sum_truesize += fp->truesize;
> - kfree_skb(fp);
> - fp = xp;
> - } while (fp);
> - } else {
> - sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
> - }
> + sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
> sum = sum_truesize + f->qsize;
>
> call_rcu(&q->rcu, inet_frag_destroy_rcu);
> @@ -489,26 +477,20 @@ EXPORT_SYMBOL(inet_frag_reasm_finish);
>
> struct sk_buff *inet_frag_pull_head(struct inet_frag_queue *q)
> {
> - struct sk_buff *head;
> + struct sk_buff *head, *skb;
>
> - if (q->fragments) {
> - head = q->fragments;
> - q->fragments = head->next;
> - } else {
> - struct sk_buff *skb;
> + head = skb_rb_first(&q->rb_fragments);
> + if (!head)
> + return NULL;
> + skb = FRAG_CB(head)->next_frag;
> + if (skb)
> + rb_replace_node(&head->rbnode, &skb->rbnode,
> + &q->rb_fragments);
> + else
> + rb_erase(&head->rbnode, &q->rb_fragments);
> + memset(&head->rbnode, 0, sizeof(head->rbnode));
> + barrier();
>
> - head = skb_rb_first(&q->rb_fragments);
> - if (!head)
> - return NULL;
> - skb = FRAG_CB(head)->next_frag;
> - if (skb)
> - rb_replace_node(&head->rbnode, &skb->rbnode,
> - &q->rb_fragments);
> - else
> - rb_erase(&head->rbnode, &q->rb_fragments);
> - memset(&head->rbnode, 0, sizeof(head->rbnode));
> - barrier();
> - }
> if (head == q->fragments_tail)
> q->fragments_tail = NULL;
>
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index 486ecb0aeb87..cf2b0a6a3337 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -261,7 +261,6 @@ static int ip_frag_reinit(struct ipq *qp)
> qp->q.flags = 0;
> qp->q.len = 0;
> qp->q.meat = 0;
> - qp->q.fragments = NULL;
> qp->q.rb_fragments = RB_ROOT;
> qp->q.fragments_tail = NULL;
> qp->q.last_run_head = NULL;
> @@ -451,7 +450,6 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
> ip_send_check(iph);
>
> __IP_INC_STATS(net, IPSTATS_MIB_REASMOKS);
> - qp->q.fragments = NULL;
> qp->q.rb_fragments = RB_ROOT;
> qp->q.fragments_tail = NULL;
> qp->q.last_run_head = NULL;
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index cb1b4772dac0..3de0e9b0a482 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -365,7 +365,6 @@ static int nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff *skb,
> skb_network_header_len(skb),
> skb->csum);
>
> - fq->q.fragments = NULL;
> fq->q.rb_fragments = RB_ROOT;
> fq->q.fragments_tail = NULL;
> fq->q.last_run_head = NULL;
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 24264d0a4b85..1a832f5e190b 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -304,7 +304,6 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
> rcu_read_lock();
> __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
> rcu_read_unlock();
> - fq->q.fragments = NULL;
> fq->q.rb_fragments = RB_ROOT;
> fq->q.fragments_tail = NULL;
> fq->q.last_run_head = NULL;
>
For the ieee802154 part:
Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
regards
Stefan Schmidt
^ permalink raw reply
* Re: [PATCH] bpf: decrease usercnt if bpf_map_new_fd() fails in bpf_map_get_fd_by_id()
From: Daniel Borkmann @ 2019-02-26 16:24 UTC (permalink / raw)
To: zerons, ast; +Cc: netdev, linux-kernel
In-Reply-To: <288b6db2-b14a-4f85-b89c-d873f0f6fcd8@gmail.com>
On 02/26/2019 03:58 PM, zerons wrote:
> On 2/26/19 22:44, Daniel Borkmann wrote:
>> On 02/26/2019 03:15 PM, zerons wrote:
>>> [ Upstream commit c91951f15978f1a0c6b65f063d30f7ea7bc6fb42 ]
>>
>> Thanks for the fix! What do you mean by "upstream commit" above in this context?
>
> This patch is based on that commit, I thought I should mention this.
> Sorry for the confusion.
Ah, no. Usually that line is placed into stable commits that are
cherry-picked to point to the original commit sha in the mainline
tree. In any case, I didn't find c91951f15978, hence my question.
Anyway, for normal patch submissions in future, please don't add
it into the log, we'll remove it this time.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] net: remove unused struct inet_frag_queue.fragments field
From: David Miller @ 2019-02-26 16:27 UTC (permalink / raw)
To: posk; +Cc: netdev, posk, linux-wpan
In-Reply-To: <20190226014346.77719-1-posk@google.com>
From: Peter Oskolkov <posk@google.com>
Date: Mon, 25 Feb 2019 17:43:46 -0800
> Now that all users of struct inet_frag_queue have been converted
> to use 'rb_fragments', remove the unused 'fragments' field.
>
> Build with `make allyesconfig` succeeded. ip_defrag selftest passed.
>
> Signed-off-by: Peter Oskolkov <posk@google.com>
Applied, thanks Peter.
^ permalink raw reply
* Re: AF_XDP design flaws
From: John Fastabend @ 2019-02-26 16:41 UTC (permalink / raw)
To: Maxim Mikityanskiy, netdev@vger.kernel.org, Björn Töpel,
Magnus Karlsson, David S. Miller
Cc: Tariq Toukan, Saeed Mahameed, Eran Ben Elisha
In-Reply-To: <AM6PR05MB5879DF6B2BD7DC426869875ED17B0@AM6PR05MB5879.eurprd05.prod.outlook.com>
On 2/26/19 6:49 AM, Maxim Mikityanskiy wrote:
> Hi everyone,
>
> I would like to discuss some design flaws of AF_XDP socket (XSK) implementation
> in kernel. At the moment I don't see a way to work around them without changing
> the API, so I would like to make sure that I'm not missing anything and to
> suggest and discuss some possible improvements that can be made.
>
> The issues I describe below are caused by the fact that the driver depends on
> the application doing some things, and if the application is
> slow/buggy/malicious, the driver is forced to busy poll because of the lack of a
> notification mechanism from the application side. I will refer to the i40e
> driver implementation a lot, as it is the first implementation of AF_XDP, but
> the issues are general and affect any driver. I already considered trying to fix
> it on driver level, but it doesn't seem possible, so it looks like the behavior
> and implementation of AF_XDP in the kernel has to be changed.
>
> RX side busy polling
> ====================
>
> On the RX side, the driver expects the application to put some descriptors in
> the Fill Ring. There is no way for the application to notify the driver that
> there are more Fill Ring descriptors to take, so the driver is forced to busy
> poll the Fill Ring if it gets empty. E.g., the i40e driver does it in NAPI poll:
>
> int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
> {
> ...
> failure = failure ||
> !i40e_alloc_rx_buffers_fast_zc(rx_ring,
> cleaned_count);
> ...
> return failure ? budget : (int)total_rx_packets;
> }
>
> Basically, it means that if there are no descriptors in the Fill Ring, NAPI will
> never stop, draining CPU.
>
> Possible cases when it happens
> ------------------------------
>
> 1. The application is slow, it received some frames in the RX Ring, and it is
> still handling the data, so it has no free frames to put to the Fill Ring.
>
> 2. The application is malicious, it opens an XSK and puts no frames to the Fill
> Ring. It can be used as a local DoS attack.
>
> 3. The application is buggy and stops filling the Fill Ring for whatever reason
> (deadlock, waiting for another blocking operation, other bugs).
>
> Although loading an XDP program requires root access, the DoS attack can be
> targeted to setups that already use XDP, i.e. an XDP program is already loaded.
> Even under root, userspace applications should not be able to disrupt system
> stability by just calling normal APIs without an intention to destroy the
> system, and here it happens in case 1.
I believe this is by design. If packets per second is your top priority
(at the expense of power, cpu, etc.) this is the behavior you might
want. To resolve your points if you don't trust the application it
should be isolated to a queue pair and cores so the impact is known and
managed.
That said having a flag to back-off seems like a good idea. But should
be doable as a feature without breaking current API.
>
> Possible way to solve the issue
> -------------------------------
>
> When the driver can't take new Fill Ring frames, it shouldn't busy poll.
> Instead, it signals the failure to the application (e.g., with POLLERR), and
> after that it's up to the application to restart polling (e.g., by calling
> sendto()) after refilling the Fill Ring. The issue with this approach is that it
> changes the API, so we either have to deal with it or to introduce some API
> version field.
See above. I like the idea here though.
>
> TX side getting stuck
> =====================
>
> On the TX side, there is the Completion Ring that the application has to clean.
> If it doesn't, the i40e driver stops taking descriptors from the TX Ring. If the
> application finally completes something, the driver can go on transmitting.
> However, it would require busy polling the Completion Ring (just like with the
> Fill Ring on the RX side). i40e doesn't do it, instead, it relies on the
> application to kick the TX by calling sendto(). The issue is that poll() doesn't
> return POLLOUT in this case, because the TX Ring is full, so the application
> will never call sendto(), and the ring is stuck forever (or at least until
> something triggers NAPI).
>
> Possible way to solve the issue
> -------------------------------
>
> When the driver can't reserve a descriptor in the Completion Ring, it should
> signal the failure to the application (e.g., with POLLERR). The application
> shouldn't call sendto() every time it sees that the number of not completed
> frames is greater than zero (like xdpsock sample does). Instead, the application
> should kick the TX only when it wants to flush the ring, and, in addition, after
> resolving the cause for POLLERR, i.e. after handling Completion Ring entries.
> The API will also have to change with this approach.
>
+1 seems to me this can be done without breaking existing API though.
> Triggering NAPI on a different CPU core
> =======================================
>
> .ndo_xsk_async_xmit runs on a random CPU core, so, to preserve CPU affinity,
> i40e triggers an interrupt to schedule NAPI, instead of calling napi_schedule
> directly. Scheduling NAPI on the correct CPU is what would every driver do, I
> guess, but currently it has to be implemented differently in every driver, and
> it relies on hardware features (the ability to trigger an IRQ).
Ideally the application would be pinned to a core and the traffic
steered to that core using ethtool/tc. Yes it requires a bit of mgmt on
the platform but I think this is needed for best pps numbers.
>
> I suggest introducing a kernel API that would allow triggering NAPI on a given
> CPU. A brief look shows that something like smp_call_function_single_async can
> be used. Advantages:
Assuming you want to avoid pinning cores/traffic for some reason. Could
this be done with some combination of cpumap + af_xdp? I haven't thought
too much about it though. Maybe Bjorn has some ideas.
>
> 1. It lifts the hardware requirement to be able to raise an interrupt on demand.
>
> 2. It would allow to move common code to the kernel (.ndo_xsk_async_xmit).
>
> 3. It is also useful in the situation where CPU affinity changes while being in
> NAPI poll. Currently, i40e and mlx5e try to stop NAPI polling by returning
> a value less than budget if CPU affinity changes. However, there are cases
> (e.g., NAPIF_STATE_MISSED) when NAPI will be rescheduled on a wrong CPU. It's a
> race between the interrupt, which will move NAPI to the correct CPU, and
> __napi_schedule from a wrong CPU. Having an API to schedule NAPI on a given CPU
> will benefit both mlx5e and i40e, because when this situation happens, it kills
> the performance.
How would we know what core to trigger NAPI on?
>
> I would be happy to hear your thoughts about these issues.
At least the first two observations seem incrementally solvable to me
and would be nice improvements. I assume the last case can be resolved
by pinning cores/traffic but for the general case perhaps it is useful.
>
> Thanks,
> Max
>
^ permalink raw reply
* Re: [PATCH net-next v4 0/6] devlink: make ethtool compat reliable
From: David Miller @ 2019-02-26 16:50 UTC (permalink / raw)
To: jakub.kicinski; +Cc: jiri, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 25 Feb 2019 19:34:01 -0800
> This is a follow up to the series which added device flash
> updates via devlink. I went with the approach of adding a
> new NDO in the end. It seems to end up looking cleaner.
>
> First patch removes the option to build devlink as a module.
> Users can still decide to not build it, but the module option
> ends up not being worth the maintenance cost.
>
> Next two patches add a NDO which can be used to ask the driver
> to return a devlink instance associated with a given netdev,
> instead of iterating over devlink ports. Drivers which implement
> this NDO must take into account the potential impact on the
> visibility of the devlink instance.
>
> With the new NDO in place we can remove NFP ethtool flash update
> code.
>
> Fifth patch makes sure we hold a reference to dev while
> callbacks are active.
>
> Last but not least the NULL-check of devlink->ops is moved
> to instance allocation time.
>
> Last but not least missing checks for devlink->ops are added.
> There is currently no driver registering devlink without ops,
> so can just fix this in -next.
>
> v2 (Michal): add netdev_to_devlink() in patch 3.
> v3 (Florian):
> - add missing checks for devlink->ops;
> - move locking/holding into devlink_compat_ functions.
> v4 (Jiri):
> - hold devlink_mutex around callbacks (patch 2);
> - require non-NULL ops (patch 6).
Series applied, thanks Jakub.
^ permalink raw reply
* [PATCH net 1/3] ipv4: Return error for RTA_VIA attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190226170004.4535-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
IPv4 currently does not support nexthops outside of the AF_INET family.
Specifically, it does not handle RTA_VIA attribute. If it is passed
in a route add request, the actual route added only uses the device
which is clearly not what the user intended:
$ ip ro add 172.16.1.0/24 via inet6 2001:db8:1::1 dev eth0
$ ip ro ls
...
172.16.1.0/24 dev eth0
Catch this and fail the route add:
$ ip ro add 172.16.1.0/24 via inet6 2001:db8:1::1 dev eth0
Error: IPv4 does not support RTA_VIA attribute.
Fixes: 03c0566542f4c ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv4/fib_frontend.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index fe4f6a624238..ed14ec245584 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -710,6 +710,10 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
case RTA_GATEWAY:
cfg->fc_gw = nla_get_be32(attr);
break;
+ case RTA_VIA:
+ NL_SET_ERR_MSG(extack, "IPv4 does not support RTA_VIA attribute");
+ err = -EINVAL;
+ goto errout;
case RTA_PRIORITY:
cfg->fc_priority = nla_get_u32(attr);
break;
--
2.11.0
^ permalink raw reply related
* [PATCH net 2/3] ipv6: Return error for RTA_VIA attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190226170004.4535-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
IPv6 currently does not support nexthops outside of the AF_INET6 family.
Specifically, it does not handle RTA_VIA attribute. If it is passed
in a route add request, the actual route added only uses the device
which is clearly not what the user intended:
$ ip -6 ro add 2001:db8:2::/64 via inet 172.16.1.1 dev eth0
$ ip ro ls
...
2001:db8:2::/64 dev eth0 metric 1024 pref medium
Catch this and fail the route add:
$ ip -6 ro add 2001:db8:2::/64 via inet 172.16.1.1 dev eth0
Error: IPv6 does not support RTA_VIA attribute.
Fixes: 03c0566542f4c ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/route.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ce15dc4ccbfa..b7a620023a52 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4182,6 +4182,10 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg->fc_gateway = nla_get_in6_addr(tb[RTA_GATEWAY]);
cfg->fc_flags |= RTF_GATEWAY;
}
+ if (tb[RTA_VIA]) {
+ NL_SET_ERR_MSG(extack, "IPv6 does not support RTA_VIA attribute");
+ goto errout;
+ }
if (tb[RTA_DST]) {
int plen = (rtm->rtm_dst_len + 7) >> 3;
--
2.11.0
^ permalink raw reply related
* [PATCH net 3/3] mpls: Return error for RTA_GATEWAY attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190226170004.4535-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
MPLS does not support nexthops with an MPLS address family.
Specifically, it does not handle RTA_GATEWAY attribute. Make it
clear by returning an error.
Fixes: 03c0566542f4c ("mpls: Netlink commands to add, remove, and dump routes")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/mpls/af_mpls.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 7d55d4c04088..fa763e2e50ec 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1838,6 +1838,9 @@ static int rtm_to_route_config(struct sk_buff *skb,
goto errout;
break;
}
+ case RTA_GATEWAY:
+ NL_SET_ERR_MSG(extack, "MPLS does not support RTA_GATEWAY attribute");
+ goto errout;
case RTA_VIA:
{
if (nla_get_via(nla, &cfg->rc_via_alen,
--
2.11.0
^ permalink raw reply related
* [PATCH net 0/3] net: Fail route add with unsupported nexthop attribute
From: David Ahern @ 2019-02-26 17:00 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
From: David Ahern <dsahern@gmail.com>
RTA_VIA was added for MPLS as a way of specifying a gateway from a
different address family. IPv4 and IPv6 do not currently support RTA_VIA
so using it leads to routes that are not what the user intended. Catch
and fail - returning a proper error message.
MPLS on the other hand does not support RTA_GATEWAY since it does not
make sense to have a nexthop from the MPLS address family. Similarly,
catch and fail - returning a proper error message.
David Ahern (3):
ipv4: Return error for RTA_VIA attribute
ipv6: Return error for RTA_VIA attribute
mpls: Return error for RTA_GATEWAY attribute
net/ipv4/fib_frontend.c | 4 ++++
net/ipv6/route.c | 4 ++++
net/mpls/af_mpls.c | 3 +++
3 files changed, 11 insertions(+)
--
2.11.0
^ permalink raw reply
* Re: [PATCH net-next 0/2] vxlan: create and changelink extack support
From: David Miller @ 2019-02-26 17:01 UTC (permalink / raw)
To: roopa; +Cc: netdev, dsa, sd, johannes
In-Reply-To: <1551160982-32685-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Mon, 25 Feb 2019 22:03:00 -0800
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This series adds extack support to changelink paths.
> In the process re-factors flag sets to a separate helper.
> Also adds some changelink testcases to rtnetlink.sh
>
> (This series was initially part of another series that
> tried to support changelink for more attributes.
> But after some feedback from sabrina, i have dropped the
> 'support changelink for more attributes' part because some
> of them cannot be supported today or may require additional
> use-case handling code. These can be done separately
> as and when we see the need for it.)
Series applied, thanks Roopa.
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: spectrum: remove set but not used variable 'autoneg_status'
From: David Miller @ 2019-02-26 17:07 UTC (permalink / raw)
To: yuehaibing; +Cc: jiri, idosch, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190225020328.42510-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Mon, 25 Feb 2019 02:03:28 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function 'mlxsw_sp_port_get_link_ksettings':
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3062:5: warning:
> variable 'autoneg_status' set but not used [-Wunused-but-set-variable]
>
> It's not used since commit 475b33cb66c9 ("mlxsw: spectrum: Remove unsupported
> eth_proto_lp_advertise field in PTYS")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: David Miller @ 2019-02-26 17:09 UTC (permalink / raw)
To: Nicolas.Ferre
Cc: f.fainelli, pthombar, netdev, andrew, hkallweit1, linux-kernel,
rafalc, piotrs, jank, Claudiu.Beznea
In-Reply-To: <a0f7be10-ff9b-c11c-fde9-30876940d719@microchip.com>
From: <Nicolas.Ferre@microchip.com>
Date: Tue, 26 Feb 2019 09:23:04 +0000
> This patch series seem pretty intrusive, so I would like that you wait
> for my explicit ACK before applying even next versions of it.
Ok.
^ permalink raw reply
* Re: [PATCH] net: sched: pie: fix mistake in reference link
From: David Miller @ 2019-02-26 17:12 UTC (permalink / raw)
To: lesliemonis; +Cc: netdev
In-Reply-To: <20190226102331.532-1-lesliemonis@gmail.com>
From: Leslie Monis <lesliemonis@gmail.com>
Date: Tue, 26 Feb 2019 15:53:31 +0530
> Fix the incorrect reference link to RFC 8033
>
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH RFC] net: Validate size of non-TSO packets in validate_xmit_skb().
From: David Miller @ 2019-02-26 17:13 UTC (permalink / raw)
To: michael.chan; +Cc: maheshb, edumazet, dja, netdev
In-Reply-To: <1551178601-16564-1-git-send-email-michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Tue, 26 Feb 2019 05:56:41 -0500
> There have been reports of oversize UDP packets being sent to the
> driver to be transmitted, causing error conditions. The issue is
> likely caused by the dst of the SKB switching between 'lo' with
> 64K MTU and the hardware device with a smaller MTU. Patches are
> being proposed by Mahesh Bandewar <maheshb@google.com> to fix the
> issue.
>
> Separately, we should add a length check in validate_xmit_skb()
> to drop these oversize packets before they reach the driver.
> This patch only validates non-TSO packets. Complete validation
> of segmented TSO packet size will probably be too slow.
>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Anything which changes the dst of an SKB really is responsible for
fixing up whatever became "incompatible" in the new path.
So like Eric I want to see this out of the fast path.
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()
From: Steve Wise @ 2019-02-26 17:13 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092615.GM23561@mtr-leonro.mtl.com>
On 2/23/2019 3:26 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
>> This function sends the constructed netlink message and then
>> receives the response, displaying any error text.
>>
>> Change 'rdma dev set' to use it.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>> rdma/dev.c | 2 +-
>> rdma/rdma.h | 1 +
>> rdma/utils.c | 21 +++++++++++++++++++++
>> 3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/rdma/dev.c b/rdma/dev.c
>> index 60ff4b31e320..d2949c378f08 100644
>> --- a/rdma/dev.c
>> +++ b/rdma/dev.c
>> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
>> mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
>> mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
>>
>> - return rd_send_msg(rd);
>> + return rd_sendrecv_msg(rd, seq);
>> }
>>
>> static int dev_one_set(struct rd *rd)
>> diff --git a/rdma/rdma.h b/rdma/rdma.h
>> index 547bb5749a39..20be2f12c4f8 100644
>> --- a/rdma/rdma.h
>> +++ b/rdma/rdma.h
>> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
>> */
>> int rd_send_msg(struct rd *rd);
>> int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
>> void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
>> int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
>> int rd_attr_cb(const struct nlattr *attr, void *data);
>> diff --git a/rdma/utils.c b/rdma/utils.c
>> index 069d44fece10..a6f2826c9605 100644
>> --- a/rdma/utils.c
>> +++ b/rdma/utils.c
>> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
>> return ret;
>> }
>>
>> +static int null_cb(const struct nlmsghdr *nlh, void *data)
>> +{
>> + return MNL_CB_OK;
>> +}
>> +
>> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
>> +{
>> + int ret;
>> +
>> + ret = rd_send_msg(rd);
>> + if (ret) {
>> + perror(NULL);
> This is more or less already done in rd_send_msg() and that function
> prints something in case of execution error. So the missing piece
> is to update rd_recv_msg(), so all places will "magically" print errors
> and not only dev_set_name().
Yea ok.
^ permalink raw reply
* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Marc Zyngier @ 2019-02-26 17:14 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Amitkumar Karwar, Enric Balletbo i Serra, Ganapathi Bhat,
Heiko Stuebner, Kalle Valo, Nishant Sarmukadam, Rob Herring,
Xinming Hu, Devicetree List, <netdev@vger.kernel.org>,
<linux-wireless@vger.kernel.org>, Linux Kernel Mailing List,
linux-rockchip, David S. Miller, linux-arm-kernel
In-Reply-To: <CAKv+Gu_=bQyVZqgUnJm9moCcaCcpDrag6NWGEpaR65=pMiTFJQ@mail.gmail.com>
On 26/02/2019 16:21, Ard Biesheuvel wrote:
> On Mon, 25 Feb 2019 at 15:53, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>> Hi Ard,
>>
>> On 25/02/2019 12:45, Ard Biesheuvel wrote:
>>> On Sun, 24 Feb 2019 at 15:08, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>
>>>> For quite some time, I wondered why the PCI mwifiex device built in my
>>>> Chromebook was unable to use the good old legacy interrupts. But as MSIs
>>>> were working fine, I never really bothered investigating. I finally had a
>>>> look, and the result isn't very pretty.
>>>>
>>>> On this machine (rk3399-based kevin), the wake-up interrupt is described as
>>>> such:
>>>>
>>>> &pci_rootport {
>>>> mvl_wifi: wifi@0,0 {
>>>> compatible = "pci1b4b,2b42";
>>>> reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
>>>> 0x83010000 0x0 0x00100000 0x0 0x00100000>;
>>>> interrupt-parent = <&gpio0>;
>>>> interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&wlan_host_wake_l>;
>>>> wakeup-source;
>>>> };
>>>> };
>>>>
>>>> Note how the interrupt is part of the properties directly attached to the
>>>> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
>>>> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
>>>> altogether (Yay for the broken design!). This is in total violation of the
>>>> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
>>>> specifiers describe the PCI device interrupts, and must obey the
>>>> INT-{A,B,C,D} mapping. Oops!
>>>>
>>>
>>> The mapping of legacy PCIe INTx interrupts onto wired system
>>> interrupts is a property of the PCIe host controller, not of a
>>> particular PCIe device. So I would argue that the code is broken here
>>> as well: it should never attempt to interpret 'interrupt' properties
>>> at the PCI device level as having any bearing on how legacy interrupts
>>> are routed.
>>
>> OpenFirmware says that this node contains the interrupt number of the
>> device (4.1.1. Open Firmware-defined Properties for Child Nodes). The
>> trick is that this property is generated *from* the device, and not set
>> in stone.
>>
>> DT, on the other hand, takes whatever is described there and uses it as
>> the gospel to configure the OS, no matter how the PCI device is actually
>> configured. If the two don't match (like in this case), things break.
>> This is the "DT describes the HW" mantra, for (sometimes) better or
>> (more generally) worse.
>>
>> What the DT code does is to interpret the whole interrupt specifier,
>> *including the interrupt-parent*. And that feels wrong. It should always
>> be in the context of the host controller. But on the other side, the DT
>> code is not in the business of validating the DT either...
>>
>> It outlines one thing: If you have to interpret per-device PCI
>> properties from DT, you're in for serious trouble. I should get some
>> better HW.
>>
>
> Yeah, it obviously makes no sense at all for the interrupt parent of a
> PCI device to deviate from the host bridge's interrupt parent, and
> it's quite unfortunate that we can't simply ban it now that the cat is
> out of the bag already.
>
> Arguably, the wake up widget is not part of the PCI device, but I have
> no opinion as to whether it is better modeling it as a sub device as
> you are proposing or as an entirely separate device referenced via a
> phandle.
It is not that clear. The widget seems to be an integral part of the
device, as it is the same basic IP that is used for SDIO and USB.
It looks like the good old pre-PCI-2.2 days, where you had to have a
separate cable between your network card and the base-board for the
wake-up interrupt to be delivered. Starting with PCI-2.2, the bus can
carry the signal just fine. With PCIe, it should just be an interrupt
TLP sent to the RC, but that's obviously not within the capabilities of
the HW.
Anyway, it'd be good if the Marvell people could chime in and let us
know how they'd prefer to handle this.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 2/4] Sync up rdma_netlink.h
From: Steve Wise @ 2019-02-26 17:15 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: dsahern, stephen, netdev, linux-rdma
In-Reply-To: <20190223092820.GN23561@mtr-leonro.mtl.com>
On 2/23/2019 3:28 AM, Leon Romanovsky wrote:
> On Thu, Feb 21, 2019 at 08:19:07AM -0800, Steve Wise wrote:
>> Pull in the latest rdma_netlink.h to get the RDMA_NLDEV_CMD_NEWLINK /
>> RDMA_NLDEV_CMD_DELLINK API.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>> ---
>> rdma/include/uapi/rdma/rdma_netlink.h | 74 +++++++++++++++++++++++++++--------
>> 1 file changed, 58 insertions(+), 16 deletions(-)
>>
> If my series go first, this patch won't be needed.
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
Some changes are still needed because your iproute2 series that was
recently merged didn't include the kernel NEWLINK/DELLINK changes. But
I've rebased this and all the IWPM changes are already there as part of
your series.
Thanks,
Steve.
^ 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