* Re: [PATCH net-next 2/2] virtio-net: get rid of unnecessary container of rq stats
From: Toshiaki Makita @ 2018-08-01 1:46 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <d11c65d8-20be-1a85-c687-c6f182264e41@redhat.com>
On 2018/08/01 10:39, Jason Wang wrote:
> On 2018年07月31日 18:02, Toshiaki Makita wrote:
>> On 2018/07/31 18:43, Jason Wang wrote:
>>> We don't maintain tx counters in rx stats any more. There's no need
>>> for an extra container of rq stats.
>>>
>>> Cc: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>> drivers/net/virtio_net.c | 80
>>> ++++++++++++++++++++++--------------------------
>>> 1 file changed, 36 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index 72d3f68..14f661c 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -87,7 +87,8 @@ struct virtnet_sq_stats {
>>> u64 kicks;
>>> };
>>> -struct virtnet_rq_stat_items {
>>> +struct virtnet_rq_stats {
>>> + struct u64_stats_sync syncp;
>>> u64 packets;
>>> u64 bytes;
>>> u64 drops;
>>> @@ -98,17 +99,8 @@ struct virtnet_rq_stat_items {
>>> u64 kicks;
>>> };
>>> -struct virtnet_rq_stats {
>>> - struct u64_stats_sync syncp;
>>> - struct virtnet_rq_stat_items items;
>>> -};
>> I'm not thinking removing sq stat is needed but even if it is I want to
>> keep virtnet_rq_stats to avoid allocating unnecessary u64_stats_syncp on
>> stack in virtnet_receive. I would just remove virtnet_rx_stats if
>> necessary.
>
> It's a nop on 64bit machines. And an unsigned on 32bit. So it's overhead
> could be ignored I think.
It's not a big problem so that's OK. I just felt like you reverted
unnecessarily too much. Anyway it is already applied and I'm not
thinking of changing this any more.
--
Toshiaki Makita
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next 1/2] virtio-net: correctly update XDP_TX counters
From: Toshiaki Makita @ 2018-08-01 1:42 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <ffbfa3ba-3019-5324-4ecb-eb7a2699a4e8@redhat.com>
On 2018/08/01 10:31, Jason Wang wrote:
> On 2018年07月31日 17:57, Toshiaki Makita wrote:
>> On 2018/07/31 18:43, Jason Wang wrote:
>>> Commit 5b8f3c8d30a6 ("virtio_net: Add XDP related stats") tries to
>>> count TX XDP stats in virtnet_receive(). This will cause several
>>> issues:
>>>
>>> - virtnet_xdp_sq() was called without checking whether or not XDP is
>>> set. This may cause out of bound access when there's no enough txq
>>> for XDP.
>>> - Stats were updated even if there's no XDP/XDP_TX.>
>>> Fixing this by reusing virtnet_xdp_xmit() for XDP_TX which can counts
>>> TX XDP counter itself and remove the unnecessary tx stats embedded in
>>> rx stats.
>> Thanks for fixing this.
>> I wanted to avoid calling u64_stats_update_begin() (i.e. smp_wmb() in 32
>> bit systems) for every packet. So I'd like to keep sq stats in
>> virtnet_rx_stats.
>>
>
> We can optimize this by adding batching on top. (virtnet_xdp_xmit()
> accepts an array of xdp frames). If you like, please send a patch for this.
Yes, that sounds like a better optimization. will think about it...
Thanks,
Toshiaki Makita
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net-next 2/2] virtio-net: get rid of unnecessary container of rq stats
From: Jason Wang @ 2018-08-01 1:39 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <da012c13-e438-f45e-5c41-2f159e791f00@lab.ntt.co.jp>
On 2018年07月31日 18:02, Toshiaki Makita wrote:
> On 2018/07/31 18:43, Jason Wang wrote:
>> We don't maintain tx counters in rx stats any more. There's no need
>> for an extra container of rq stats.
>>
>> Cc: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/net/virtio_net.c | 80 ++++++++++++++++++++++--------------------------
>> 1 file changed, 36 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 72d3f68..14f661c 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -87,7 +87,8 @@ struct virtnet_sq_stats {
>> u64 kicks;
>> };
>>
>> -struct virtnet_rq_stat_items {
>> +struct virtnet_rq_stats {
>> + struct u64_stats_sync syncp;
>> u64 packets;
>> u64 bytes;
>> u64 drops;
>> @@ -98,17 +99,8 @@ struct virtnet_rq_stat_items {
>> u64 kicks;
>> };
>>
>> -struct virtnet_rq_stats {
>> - struct u64_stats_sync syncp;
>> - struct virtnet_rq_stat_items items;
>> -};
> I'm not thinking removing sq stat is needed but even if it is I want to
> keep virtnet_rq_stats to avoid allocating unnecessary u64_stats_syncp on
> stack in virtnet_receive. I would just remove virtnet_rx_stats if necessary.
It's a nop on 64bit machines. And an unsigned on 32bit. So it's overhead
could be ignored I think.
Thanks
>> -
>> -struct virtnet_rx_stats {
>> - struct virtnet_rq_stat_items rx;
>> -};
>> -
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the bpf tree
From: Stephen Rothwell @ 2018-08-01 1:35 UTC (permalink / raw)
To: David Miller, Networking, Daniel Borkmann, Alexei Starovoitov
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Yonghong Song,
Okash Khawaja
[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
tools/bpf/bpftool/map.c
between commit:
573b3aa69406 ("tools/bpftool: fix a percpu_array map dump problem")
from the bpf tree and commit:
2d3feca8c44f ("bpf: btf: print map dump and lookup with btf info")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc tools/bpf/bpftool/map.c
index f74a8bcbda87,9c8191845585..000000000000
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@@ -34,9 -34,7 +34,8 @@@
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
+#include <linux/kernel.h>
+ #include <linux/err.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
@@@ -162,11 -262,20 +264,21 @@@ static void print_entry_json(struct bpf
print_hex_data_json(key, info->key_size);
jsonw_name(json_wtr, "value");
print_hex_data_json(value, info->value_size);
+ if (btf) {
+ struct btf_dumper d = {
+ .btf = btf,
+ .jw = json_wtr,
+ .is_plain_text = false,
+ };
+
+ jsonw_name(json_wtr, "formatted");
+ do_dump_btf(&d, info, key, value);
+ }
} else {
- unsigned int i, n;
+ unsigned int i, n, step;
n = get_possible_cpus();
+ step = round_up(info->value_size, 8);
jsonw_name(json_wtr, "key");
print_hex_data_json(key, info->key_size);
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/2] virtio-net: correctly update XDP_TX counters
From: Jason Wang @ 2018-08-01 1:31 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <2cdb8081-90a8-3a87-b6a6-4395825594a1@lab.ntt.co.jp>
On 2018年07月31日 17:57, Toshiaki Makita wrote:
> On 2018/07/31 18:43, Jason Wang wrote:
>> Commit 5b8f3c8d30a6 ("virtio_net: Add XDP related stats") tries to
>> count TX XDP stats in virtnet_receive(). This will cause several
>> issues:
>>
>> - virtnet_xdp_sq() was called without checking whether or not XDP is
>> set. This may cause out of bound access when there's no enough txq
>> for XDP.
>> - Stats were updated even if there's no XDP/XDP_TX.>
>> Fixing this by reusing virtnet_xdp_xmit() for XDP_TX which can counts
>> TX XDP counter itself and remove the unnecessary tx stats embedded in
>> rx stats.
> Thanks for fixing this.
> I wanted to avoid calling u64_stats_update_begin() (i.e. smp_wmb() in 32
> bit systems) for every packet. So I'd like to keep sq stats in
> virtnet_rx_stats.
>
We can optimize this by adding batching on top. (virtnet_xdp_xmit()
accepts an array of xdp frames). If you like, please send a patch for this.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v1 3/3] net: WireGuard secure network tunnel
From: Shawn Landden @ 2018-08-01 1:21 UTC (permalink / raw)
To: linux-kernel, netdev, davem; +Cc: Jason A . Donenfeld, Greg KH
In-Reply-To: <20180731191102.2434-4-Jason@zx2c4.com>
Does ratelimiter_selftest still always fail on slow CPUs?
^ permalink raw reply
* Re: [PATCH] lib/bpf/libbpf: Simplify the code by using PTR_ERR_OR_ZERO
From: zhong jiang @ 2018-08-01 1:18 UTC (permalink / raw)
To: ast, daniel, davem; +Cc: netdev, linux-kernel
In-Reply-To: <1533054656-36134-1-git-send-email-zhongjiang@huawei.com>
On 2018/8/1 0:30, zhong jiang wrote:
> Use PTR_ERR_OR_ZERO is better than the open code.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> tools/lib/bpf/libbpf.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 40211b5..9b61468 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2271,9 +2271,7 @@ struct bpf_map *
>
> long libbpf_get_error(const void *ptr)
> {
> - if (IS_ERR(ptr))
> - return PTR_ERR(ptr);
> - return 0;
> + return PTR_ERR_OR_ZERO(ptr);
> }
>
> int bpf_prog_load(const char *file, enum bpf_prog_type type,
I am so sorry for that. I will repost it after check it correct carefully.
Thanks
zhong jiang
^ permalink raw reply
* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Pravin Shelar @ 2018-07-31 23:12 UTC (permalink / raw)
To: Matteo Croce
Cc: Justin Pettit, Greg Rose, Ben Pfaff, netdev, ovs dev,
Stefano Brivio, Jiri Benc, Aaron
In-Reply-To: <CAGnkfhyxQSz=8OsgTsjR3NfZ2FPwv+FjPZNPEY5VHZRsEiQ68w@mail.gmail.com>
On Tue, Jul 31, 2018 at 12:43 PM, Matteo Croce <mcroce@redhat.com> wrote:
> On Mon, Jul 16, 2018 at 4:54 PM Matteo Croce <mcroce@redhat.com> wrote:
>>
>> On Tue, Jul 10, 2018 at 6:31 PM Pravin Shelar <pshelar@ovn.org> wrote:
>> >
>> > On Wed, Jul 4, 2018 at 7:23 AM, Matteo Croce <mcroce@redhat.com> wrote:
>> > > From: Stefano Brivio <sbrivio@redhat.com>
>> > >
>> > > Open vSwitch sends to userspace all received packets that have
>> > > no associated flow (thus doing an "upcall"). Then the userspace
>> > > program creates a new flow and determines the actions to apply
>> > > based on its configuration.
>> > >
>> > > When a single port generates a high rate of upcalls, it can
>> > > prevent other ports from dispatching their own upcalls. vswitchd
>> > > overcomes this problem by creating many netlink sockets for each
>> > > port, but it quickly exceeds any reasonable maximum number of
>> > > open files when dealing with huge amounts of ports.
>> > >
>> > > This patch queues all the upcalls into a list, ordering them in
>> > > a per-port round-robin fashion, and schedules a deferred work to
>> > > queue them to userspace.
>> > >
>> > > The algorithm to queue upcalls in a round-robin fashion,
>> > > provided by Stefano, is based on these two rules:
>> > > - upcalls for a given port must be inserted after all the other
>> > > occurrences of upcalls for the same port already in the queue,
>> > > in order to avoid out-of-order upcalls for a given port
>> > > - insertion happens once the highest upcall count for any given
>> > > port (excluding the one currently at hand) is greater than the
>> > > count for the port we're queuing to -- if this condition is
>> > > never true, upcall is queued at the tail. This results in a
>> > > per-port round-robin order.
>> > >
>> > > In order to implement a fair round-robin behaviour, a variable
>> > > queueing delay is introduced. This will be zero if the upcalls
>> > > rate is below a given threshold, and grows linearly with the
>> > > queue utilisation (i.e. upcalls rate) otherwise.
>> > >
>> > > This ensures fairness among ports under load and with few
>> > > netlink sockets.
>> > >
>> > Thanks for the patch.
>> > This patch is adding following overhead for upcall handling:
>> > 1. kmalloc.
>> > 2. global spin-lock.
>> > 3. context switch to single worker thread.
>> > I think this could become bottle neck on most of multi core systems.
>> > You have mentioned issue with existing fairness mechanism, Can you
>> > elaborate on those, I think we could improve that before implementing
>> > heavy weight fairness in upcall handling.
>>
>> Hi Pravin,
>>
>> vswitchd allocates N * P netlink sockets, where N is the number of
>> online CPU cores, and P the number of ports.
>> With some setups, this number can grow quite fast, also exceeding the
>> system maximum file descriptor limit.
>> I've seen a 48 core server failing with -EMFILE when trying to create
>> more than 65535 netlink sockets needed for handling 1800+ ports.
>>
>> I made a previous attempt to reduce the sockets to one per CPU, but
>> this was discussed and rejected on ovs-dev because it would remove
>> fairness among ports[1].
Rather than reducing number of thread down to 1, we could find better
number of FDs per port.
How about this simple solution:
1. Allocate (N * P) FDs as long as it is under FD limit.
2. If FD limit (-EMFILE) is hit reduce N value by half and repeat step 1.
Thanks,
Pravin.
>> I think that the current approach of opening a huge number of sockets
>> doesn't really work, (it doesn't scale for sure), it still needs some
>> queueing logic (either in kernel or user space) if we really want to
>> be sure that low traffic ports gets their upcalls quota when other
>> ports are doing way more traffic.
>>
>> If you are concerned about the kmalloc or spinlock, we can solve them
>> with kmem_cache or two copies of the list and rcu, I'll happy to
>> discuss the implementation details, as long as we all agree that the
>> current implementation doesn't scale well and has an issue.
>>
>> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-February/344279.html
>>
>> --
>> Matteo Croce
>> per aspera ad upstream
>
> Hi all,
>
> any idea on how to solve the file descriptor limit hit by the netlink sockets?
> I see this issue happen very often, and raising the FD limit to 400k
> seems not the right way to solve it.
> Any other suggestion on how to improve the patch, or solve the problem
> in a different way?
>
> Regards,
>
>
>
> --
> Matteo Croce
> per aspera ad upstream
^ permalink raw reply
* Re: KASAN: use-after-free Read in rtnetlink_put_metrics
From: Cong Wang @ 2018-07-31 23:03 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: Eric Dumazet, syzbot+41f9c04b50ef70c66947, christian.brauner,
David Miller, David Ahern, Florian Westphal, Jiri Benc,
Kirill Tkhai, LKML, lucien xin, Linux Kernel Network Developers,
syzkaller-bugs
In-Reply-To: <20180731134014.GA32114@bistromath.localdomain>
On Tue, Jul 31, 2018 at 6:41 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> 2018-07-31, 05:41:56 -0700, Eric Dumazet wrote:
> >
> >
> > On 07/31/2018 05:31 AM, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit: 61f4b23769f0 netlink: Don't shift with UB on nlk->ngroups
> > > git tree: net
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=14a9de58400000
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=ffb4428fdc82f93b
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=41f9c04b50ef70c66947
> > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> [...]
>
> > Probably also caused by :
> >
> >
> > commit df18b50448fab1dff093731dfd0e25e77e1afcd1
> > Author: Sabrina Dubroca <sd@queasysnail.net>
> > Date: Mon Jul 30 16:23:10 2018 +0200
> >
> > net/ipv6: fix metrics leak
>
> Yeah, I'm looking into both those reports :/
Looks like this commit is completely unnecessary,
fib6_drop_pcpu_from() calls fib6_info_release()
which calls fib6_info_destroy_rcu(), so this metrics
will be released twice...
^ permalink raw reply
* Re: [PATCH v4 bpf-next 08/14] bpf: introduce the bpf_get_local_storage() helper function
From: Roman Gushchin @ 2018-08-01 0:28 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, linux-kernel, kernel-team, Alexei Starovoitov
In-Reply-To: <ad6c58e3-08a8-aa9c-2d69-528007fe126e@iogearbox.net>
On Wed, Aug 01, 2018 at 12:50:16AM +0200, Daniel Borkmann wrote:
> On 07/27/2018 11:52 PM, Roman Gushchin wrote:
> [...]
> > @@ -2533,6 +2541,16 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
> > }
> >
> > regs = cur_regs(env);
> > +
> > + /* check that flags argument in get_local_storage(map, flags) is 0,
> > + * this is required because get_local_storage() can't return an error.
> > + */
> > + if (func_id == BPF_FUNC_get_local_storage &&
> > + !tnum_equals_const(regs[BPF_REG_2].var_off, 0)) {
> > + verbose(env, "get_local_storage() doesn't support non-zero flags\n");
> > + return -EINVAL;
> > + }
>
> Hmm, this check is actually not correct. You will still be able to pass non-zero
> values in there. arg2_type from the helper is ARG_ANYTHING, so the register type
> could for example be one of the pointer types and it will still pass the verifier.
> The correct way to check would be to use register_is_null().
>
> > +
> > /* reset caller saved regs */
> > for (i = 0; i < CALLER_SAVED_REGS; i++) {
> > mark_reg_not_init(env, regs, caller_saved[i]);
Oh, perfect catch!
The diff is below. Please, let me know if you prefer me to resend
the whole patch/patchset.
Thank you!
Roman
--
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0f69f827ff9f..4547e07ab3db 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2546,7 +2546,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
* this is required because get_local_storage() can't return an error.
*/
if (func_id == BPF_FUNC_get_local_storage &&
- !tnum_equals_const(regs[BPF_REG_2].var_off, 0)) {
+ !register_is_null(®s[BPF_REG_2])) {
verbose(env, "get_local_storage() doesn't support non-zero flags\n");
return -EINVAL;
}
^ permalink raw reply related
* [PATCH net-next v2 7/7] selftests: mlxsw: Add test for ip_forward_update_priority
From: Petr Machata @ 2018-07-31 22:39 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
Verify that with that sysctl turned off, DSCP prioritization and rewrite
works the same way as in qos_dscp_bridge test. However when the sysctl
is charged, there should be a reprioritization after routing stage,
which will be observed by a different DSCP rewrite on egress.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
.../selftests/drivers/net/mlxsw/qos_dscp_router.sh | 233 +++++++++++++++++++++
1 file changed, 233 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
new file mode 100755
index 000000000000..281d90766e12
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
@@ -0,0 +1,233 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Test for DSCP prioritization in the router.
+#
+# With ip_forward_update_priority disabled, the packets are expected to keep
+# their DSCP (which in this test uses only values 0..7) intact as they are
+# forwarded by the switch. That is verified at $h2. ICMP responses are formed
+# with the same DSCP as the requests, and likewise pass through the switch
+# intact, which is verified at $h1.
+#
+# With ip_forward_update_priority enabled, router reprioritizes the packets
+# according to the table in reprioritize(). Thus, say, DSCP 7 maps to priority
+# 4, which on egress maps back to DSCP 4. The response packet then gets
+# reprioritized to 6, getting DSCP 6 on egress.
+#
+# +----------------------+ +----------------------+
+# | H1 | | H2 |
+# | + $h1 | | $h2 + |
+# | | 192.0.2.1/28 | | 192.0.2.18/28 | |
+# +----|-----------------+ +----------------|-----+
+# | |
+# +----|----------------------------------------------------------------|-----+
+# | SW | | |
+# | + $swp1 $swp2 + |
+# | 192.0.2.2/28 192.0.2.17/28 |
+# | APP=0,5,0 .. 7,5,7 APP=0,5,0 .. 7,5,7 |
+# +---------------------------------------------------------------------------+
+
+ALL_TESTS="
+ ping_ipv4
+ test_update
+ test_no_update
+"
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+NUM_NETIFS=4
+source $lib_dir/lib.sh
+
+reprioritize()
+{
+ local in=$1; shift
+
+ # This is based on rt_tos2priority in include/net/route.h. Assuming 1:1
+ # mapping between priorities and TOS, it yields a new priority for a
+ # packet with ingress priority of $in.
+ local -a reprio=(0 0 2 2 6 6 4 4)
+
+ echo ${reprio[$in]}
+}
+
+h1_create()
+{
+ local dscp;
+
+ simple_if_init $h1 192.0.2.1/28
+ tc qdisc add dev $h1 clsact
+ dscp_capture_install $h1 0
+ ip route add vrf v$h1 192.0.2.16/28 via 192.0.2.2
+}
+
+h1_destroy()
+{
+ ip route del vrf v$h1 192.0.2.16/28 via 192.0.2.2
+ dscp_capture_uninstall $h1 0
+ tc qdisc del dev $h1 clsact
+ simple_if_fini $h1 192.0.2.1/28
+}
+
+h2_create()
+{
+ simple_if_init $h2 192.0.2.18/28
+ tc qdisc add dev $h2 clsact
+ dscp_capture_install $h2 0
+ ip route add vrf v$h2 192.0.2.0/28 via 192.0.2.17
+}
+
+h2_destroy()
+{
+ ip route del vrf v$h2 192.0.2.0/28 via 192.0.2.17
+ dscp_capture_uninstall $h2 0
+ tc qdisc del dev $h2 clsact
+ simple_if_fini $h2 192.0.2.18/28
+}
+
+dscp_map()
+{
+ local base=$1; shift
+
+ for prio in {0..7}; do
+ echo app=$prio,5,$((base + prio))
+ done
+}
+
+switch_create()
+{
+ simple_if_init $swp1 192.0.2.2/28
+ __simple_if_init $swp2 v$swp1 192.0.2.17/28
+
+ lldptool -T -i $swp1 -V APP $(dscp_map 0) >/dev/null
+ lldptool -T -i $swp2 -V APP $(dscp_map 0) >/dev/null
+ lldpad_app_wait_set $swp1
+ lldpad_app_wait_set $swp2
+}
+
+switch_destroy()
+{
+ lldptool -T -i $swp2 -V APP -d $(dscp_map 0) >/dev/null
+ lldptool -T -i $swp1 -V APP -d $(dscp_map 0) >/dev/null
+ lldpad_app_wait_del
+
+ __simple_if_fini $swp2 192.0.2.17/28
+ simple_if_fini $swp1 192.0.2.2/28
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ swp2=${NETIFS[p3]}
+ h2=${NETIFS[p4]}
+
+ vrf_prepare
+
+ sysctl_set net.ipv4.ip_forward_update_priority 1
+ h1_create
+ h2_create
+ switch_create
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ switch_destroy
+ h2_destroy
+ h1_destroy
+ sysctl_restore net.ipv4.ip_forward_update_priority
+
+ vrf_cleanup
+}
+
+ping_ipv4()
+{
+ ping_test $h1 192.0.2.18
+}
+
+dscp_ping_test()
+{
+ local vrf_name=$1; shift
+ local sip=$1; shift
+ local dip=$1; shift
+ local prio=$1; shift
+ local reprio=$1; shift
+ local dev1=$1; shift
+ local dev2=$1; shift
+
+ local prio2=$($reprio $prio) # ICMP Request egress prio
+ local prio3=$($reprio $prio2) # ICMP Response egress prio
+
+ local dscp=$((prio << 2)) # ICMP Request ingress DSCP
+ local dscp2=$((prio2 << 2)) # ICMP Request egress DSCP
+ local dscp3=$((prio3 << 2)) # ICMP Response egress DSCP
+
+ RET=0
+
+ eval "local -A dev1_t0s=($(dscp_fetch_stats $dev1 0))"
+ eval "local -A dev2_t0s=($(dscp_fetch_stats $dev2 0))"
+
+ ip vrf exec $vrf_name \
+ ${PING} -Q $dscp ${sip:+-I $sip} $dip \
+ -c 10 -i 0.1 -w 2 &> /dev/null
+
+ eval "local -A dev1_t1s=($(dscp_fetch_stats $dev1 0))"
+ eval "local -A dev2_t1s=($(dscp_fetch_stats $dev2 0))"
+
+ for i in {0..7}; do
+ local dscpi=$((i << 2))
+ local expect2=0
+ local expect3=0
+
+ if ((i == prio2)); then
+ expect2=10
+ fi
+ if ((i == prio3)); then
+ expect3=10
+ fi
+
+ local delta=$((dev2_t1s[$i] - dev2_t0s[$i]))
+ ((expect2 == delta))
+ check_err $? "DSCP $dscpi@$dev2: Expected to capture $expect2 packets, got $delta."
+
+ delta=$((dev1_t1s[$i] - dev1_t0s[$i]))
+ ((expect3 == delta))
+ check_err $? "DSCP $dscpi@$dev1: Expected to capture $expect3 packets, got $delta."
+ done
+
+ log_test "DSCP rewrite: $dscp-(prio $prio2)-$dscp2-(prio $prio3)-$dscp3"
+}
+
+__test_update()
+{
+ local update=$1; shift
+ local reprio=$1; shift
+
+ sysctl_restore net.ipv4.ip_forward_update_priority
+ sysctl_set net.ipv4.ip_forward_update_priority $update
+
+ for prio in {0..7}; do
+ dscp_ping_test v$h1 192.0.2.1 192.0.2.18 $prio $reprio $h1 $h2
+ done
+}
+
+test_update()
+{
+ __test_update 1 reprioritize
+}
+
+test_no_update()
+{
+ __test_update 0 echo
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 6/7] selftests: forwarding: Move DSCP capture to lib.sh
From: Petr Machata @ 2018-07-31 22:39 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
dscp_capture_install() and dscp_capture_uninstall() are going to be
useful for a test added by a following patch, move them therefore to
lib.sh together with related helpers.
While doing so, change the rule preference from mere DSCP value to
DSCP+100 in order to support adding captures of packets with DSCP of 0.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
.../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh | 42 ----------------------
tools/testing/selftests/net/forwarding/lib.sh | 42 ++++++++++++++++++++++
2 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
index 9e875ee8dc1c..1ca631d5aaba 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
@@ -34,36 +34,6 @@ lib_dir=$(dirname $0)/../../../net/forwarding
NUM_NETIFS=4
source $lib_dir/lib.sh
-__dscp_capture_add_del()
-{
- local add_del=$1; shift
- local dev=$1; shift
- local base=$1; shift
- local dscp;
-
- for prio in {0..7}; do
- dscp=$((base + prio))
- __icmp_capture_add_del $add_del $dscp "" $dev \
- "ip_tos $((dscp << 2))"
- done
-}
-
-dscp_capture_install()
-{
- local dev=$1; shift
- local base=$1; shift
-
- __dscp_capture_add_del add $dev $base
-}
-
-dscp_capture_uninstall()
-{
- local dev=$1; shift
- local base=$1; shift
-
- __dscp_capture_add_del del $dev $base
-}
-
h1_create()
{
local dscp;
@@ -155,18 +125,6 @@ cleanup()
vrf_cleanup
}
-dscp_fetch_stats()
-{
- local dev=$1; shift
- local base=$1; shift
-
- for prio in {0..7}; do
- local dscp=$((base + prio))
- local t=$(tc_rule_stats_get $dev $dscp)
- echo "[$dscp]=$t "
- done
-}
-
ping_ipv4()
{
ping_test $h1 192.0.2.2
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 90af5cd23417..ca53b539aa2d 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -653,6 +653,48 @@ vlan_capture_uninstall()
__vlan_capture_add_del del 100 "$@"
}
+__dscp_capture_add_del()
+{
+ local add_del=$1; shift
+ local dev=$1; shift
+ local base=$1; shift
+ local dscp;
+
+ for prio in {0..7}; do
+ dscp=$((base + prio))
+ __icmp_capture_add_del $add_del $((dscp + 100)) "" $dev \
+ "skip_hw ip_tos $((dscp << 2))"
+ done
+}
+
+dscp_capture_install()
+{
+ local dev=$1; shift
+ local base=$1; shift
+
+ __dscp_capture_add_del add $dev $base
+}
+
+dscp_capture_uninstall()
+{
+ local dev=$1; shift
+ local base=$1; shift
+
+ __dscp_capture_add_del del $dev $base
+}
+
+dscp_fetch_stats()
+{
+ local dev=$1; shift
+ local base=$1; shift
+
+ for prio in {0..7}; do
+ local dscp=$((base + prio))
+ local t=$(tc_rule_stats_get $dev $((dscp + 100)))
+ echo "[$dscp]=$t "
+ done
+}
+
matchall_sink_create()
{
local dev=$1; shift
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 5/7] selftests: forwarding: Move lldpad waiting to lib.sh
From: Petr Machata @ 2018-07-31 22:38 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
The function lldpad_wait() will be useful for a test added by a
following patch. Likewise would the "sleep 5" with its extensive
comment.
Therefore move lldpad_wait() to lib.sh in order to allow reuse. Rename
it to lldpad_app_wait_set() to recognize that what this is intended to
wait on are the pending APP sets.
For the sleeping, add a function lldpad_app_wait_del(). That will serve
to hold the related explanatory comment (which edit for clarity), and as
a token in the caller to identify the sites where this sort of waiting
takes place. That will serve when/if a better way to handle this
business is found.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
.../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh | 23 +++-------------------
tools/testing/selftests/net/forwarding/lib.sh | 21 ++++++++++++++++++++
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
index cc527660a022..9e875ee8dc1c 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
@@ -103,16 +103,6 @@ dscp_map()
done
}
-lldpad_wait()
-{
- local dev=$1; shift
-
- while lldptool -t -i $dev -V APP -c app | grep -q pending; do
- echo "$dev: waiting for lldpad to push pending APP updates"
- sleep 5
- done
-}
-
switch_create()
{
ip link add name br1 type bridge vlan_filtering 1
@@ -124,22 +114,15 @@ switch_create()
lldptool -T -i $swp1 -V APP $(dscp_map 10) >/dev/null
lldptool -T -i $swp2 -V APP $(dscp_map 20) >/dev/null
- lldpad_wait $swp1
- lldpad_wait $swp2
+ lldpad_app_wait_set $swp1
+ lldpad_app_wait_set $swp2
}
switch_destroy()
{
lldptool -T -i $swp2 -V APP -d $(dscp_map 20) >/dev/null
lldptool -T -i $swp1 -V APP -d $(dscp_map 10) >/dev/null
-
- # Give lldpad a chance to push down the changes. If the device is downed
- # too soon, the updates will be left pending, but will have been struck
- # off the lldpad's DB already, and we won't be able to tell. Then on
- # next test iteration this would cause weirdness as newly-added APP
- # rules conflict with the old ones, sometimes getting stuck in an
- # "unknown" state.
- sleep 5
+ lldpad_app_wait_del
ip link set dev $swp2 nomaster
ip link set dev $swp1 nomaster
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 843a6715924f..90af5cd23417 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -247,6 +247,27 @@ setup_wait()
sleep $WAIT_TIME
}
+lldpad_app_wait_set()
+{
+ local dev=$1; shift
+
+ while lldptool -t -i $dev -V APP -c app | grep -q pending; do
+ echo "$dev: waiting for lldpad to push pending APP updates"
+ sleep 5
+ done
+}
+
+lldpad_app_wait_del()
+{
+ # Give lldpad a chance to push down the changes. If the device is downed
+ # too soon, the updates will be left pending. However, they will have
+ # been struck off the lldpad's DB already, so we won't be able to tell
+ # they are pending. Then on next test iteration this would cause
+ # weirdness as newly-added APP rules conflict with the old ones,
+ # sometimes getting stuck in an "unknown" state.
+ sleep 5
+}
+
pre_cleanup()
{
if [ "${PAUSE_ON_CLEANUP}" = "yes" ]; then
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 4/7] mlxsw: spectrum_router: Handle sysctl_ip_fwd_update_priority
From: Petr Machata @ 2018-07-31 22:38 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
This sysctl setting controls whether packet priority should be updated
after forwarding. Configure RGCR.usp accordingly so that the device is
in sync with the kernel handling.
Note that RGCR doesn't allow changing arbitrary parameters
mid-operation, however "usp" is exempt and can be reconfigured.
Also react to NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE notifications
that signify change in this configuration.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 5ee927626567..eec7166fad62 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2436,6 +2436,18 @@ static void mlxsw_sp_router_mp_hash_event_work(struct work_struct *work)
kfree(net_work);
}
+static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp);
+
+static void mlxsw_sp_router_update_priority_work(struct work_struct *work)
+{
+ struct mlxsw_sp_netevent_work *net_work =
+ container_of(work, struct mlxsw_sp_netevent_work, work);
+ struct mlxsw_sp *mlxsw_sp = net_work->mlxsw_sp;
+
+ __mlxsw_sp_router_init(mlxsw_sp);
+ kfree(net_work);
+}
+
static int mlxsw_sp_router_schedule_work(struct net *net,
struct notifier_block *nb,
void (*cb)(struct work_struct *))
@@ -2522,6 +2534,9 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
return mlxsw_sp_router_schedule_work(ptr, nb,
mlxsw_sp_router_mp_hash_event_work);
+ case NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE:
+ return mlxsw_sp_router_schedule_work(ptr, nb,
+ mlxsw_sp_router_update_priority_work);
}
return NOTIFY_DONE;
@@ -7390,6 +7405,7 @@ static int mlxsw_sp_dscp_init(struct mlxsw_sp *mlxsw_sp)
static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
{
+ bool usp = init_net.ipv4.sysctl_ip_fwd_update_priority;
char rgcr_pl[MLXSW_REG_RGCR_LEN];
u64 max_rifs;
int err;
@@ -7400,7 +7416,7 @@ static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
mlxsw_reg_rgcr_pack(rgcr_pl, true, true);
mlxsw_reg_rgcr_max_router_interfaces_set(rgcr_pl, max_rifs);
- mlxsw_reg_rgcr_usp_set(rgcr_pl, true);
+ mlxsw_reg_rgcr_usp_set(rgcr_pl, usp);
err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(rgcr), rgcr_pl);
if (err)
return err;
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 3/7] mlxsw: spectrum: Extract work-scheduling into a new function
From: Petr Machata @ 2018-07-31 22:37 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
The boilerplate to schedule NETEVENT_IPV4_MPATH_HASH_UPDATE and
NETEVENT_IPV6_MPATH_HASH_UPDATE handling is almost equivalent to that of
NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE that's coming in the next
patch. The only difference is which actual worker function should be
called. Extract this boilerplate into a named function in order to allow
reuse.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 38 +++++++++++++---------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8d67f0123699..5ee927626567 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2436,17 +2436,36 @@ static void mlxsw_sp_router_mp_hash_event_work(struct work_struct *work)
kfree(net_work);
}
+static int mlxsw_sp_router_schedule_work(struct net *net,
+ struct notifier_block *nb,
+ void (*cb)(struct work_struct *))
+{
+ struct mlxsw_sp_netevent_work *net_work;
+ struct mlxsw_sp_router *router;
+
+ if (!net_eq(net, &init_net))
+ return NOTIFY_DONE;
+
+ net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
+ if (!net_work)
+ return NOTIFY_BAD;
+
+ router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
+ INIT_WORK(&net_work->work, cb);
+ net_work->mlxsw_sp = router->mlxsw_sp;
+ mlxsw_core_schedule_work(&net_work->work);
+ return NOTIFY_DONE;
+}
+
static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
unsigned long event, void *ptr)
{
struct mlxsw_sp_netevent_work *net_work;
struct mlxsw_sp_port *mlxsw_sp_port;
- struct mlxsw_sp_router *router;
struct mlxsw_sp *mlxsw_sp;
unsigned long interval;
struct neigh_parms *p;
struct neighbour *n;
- struct net *net;
switch (event) {
case NETEVENT_DELAY_PROBE_TIME_UPDATE:
@@ -2500,20 +2519,9 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
break;
case NETEVENT_IPV4_MPATH_HASH_UPDATE:
case NETEVENT_IPV6_MPATH_HASH_UPDATE:
- net = ptr;
+ return mlxsw_sp_router_schedule_work(ptr, nb,
+ mlxsw_sp_router_mp_hash_event_work);
- if (!net_eq(net, &init_net))
- return NOTIFY_DONE;
-
- net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
- if (!net_work)
- return NOTIFY_BAD;
-
- router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
- INIT_WORK(&net_work->work, mlxsw_sp_router_mp_hash_event_work);
- net_work->mlxsw_sp = router->mlxsw_sp;
- mlxsw_core_schedule_work(&net_work->work);
- break;
}
return NOTIFY_DONE;
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 2/7] net: ipv4: Notify about changes to ip_forward_update_priority
From: Petr Machata @ 2018-07-31 22:36 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
Drivers may make offloading decision based on whether
ip_forward_update_priority is enabled or not. Therefore distribute
netevent notifications to give them a chance to react to a change.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
include/net/netevent.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/include/net/netevent.h b/include/net/netevent.h
index d9918261701c..4107016c3bb4 100644
--- a/include/net/netevent.h
+++ b/include/net/netevent.h
@@ -28,6 +28,7 @@ enum netevent_notif_type {
NETEVENT_DELAY_PROBE_TIME_UPDATE, /* arg is struct neigh_parms ptr */
NETEVENT_IPV4_MPATH_HASH_UPDATE, /* arg is struct net ptr */
NETEVENT_IPV6_MPATH_HASH_UPDATE, /* arg is struct net ptr */
+ NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE, /* arg is struct net ptr */
};
int register_netevent_notifier(struct notifier_block *nb);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index e21dda015513..b92f422f2fa8 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -201,6 +201,23 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
return ret;
}
+static int ipv4_fwd_update_priority(struct ctl_table *table, int write,
+ void __user *buffer,
+ size_t *lenp, loff_t *ppos)
+{
+ struct net *net;
+ int ret;
+
+ net = container_of(table->data, struct net,
+ ipv4.sysctl_ip_fwd_update_priority);
+ ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+ if (write && ret == 0)
+ call_netevent_notifiers(NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE,
+ net);
+
+ return ret;
+}
+
static int proc_tcp_congestion_control(struct ctl_table *ctl, int write,
void __user *buffer, size_t *lenp, loff_t *ppos)
{
@@ -668,7 +685,7 @@ static struct ctl_table ipv4_net_table[] = {
.data = &init_net.ipv4.sysctl_ip_fwd_update_priority,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = proc_dointvec_minmax,
+ .proc_handler = ipv4_fwd_update_priority,
.extra1 = &zero,
.extra2 = &one,
},
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 1/7] net: ipv4: Control SKB reprioritization after forwarding
From: Petr Machata @ 2018-07-31 22:36 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
In-Reply-To: <cover.1533076217.git.petrm@mellanox.com>
After IPv4 packets are forwarded, the priority of the corresponding SKB
is updated according to the TOS field of IPv4 header. This overrides any
prioritization done earlier by e.g. an skbedit action or ingress-qos-map
defined at a vlan device.
Such overriding may not always be desirable. Even if the packet ends up
being routed, which implies this is an L3 network node, an administrator
may wish to preserve whatever prioritization was done earlier on in the
pipeline.
Therefore introduce a sysctl that controls this behavior. Keep the
default value at 1 to maintain backward-compatible behavior.
Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
Documentation/networking/ip-sysctl.txt | 9 +++++++++
include/net/netns/ipv4.h | 1 +
net/ipv4/af_inet.c | 1 +
net/ipv4/ip_forward.c | 3 ++-
net/ipv4/sysctl_net_ipv4.c | 9 +++++++++
5 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 77c37fb0b6a6..e74515ecaa9c 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -81,6 +81,15 @@ fib_multipath_hash_policy - INTEGER
0 - Layer 3
1 - Layer 4
+ip_forward_update_priority - INTEGER
+ Whether to update SKB priority from "TOS" field in IPv4 header after it
+ is forwarded. The new SKB priority is mapped from TOS field value
+ according to an rt_tos2priority table (see e.g. man tc-prio).
+ Default: 1 (Update priority.)
+ Possible values:
+ 0 - Do not update priority.
+ 1 - Update priority.
+
route/max_size - INTEGER
Maximum number of routes allowed in the kernel. Increase
this when using large numbers of interfaces and/or routes.
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 661348f23ea5..e47503b4e4d1 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -98,6 +98,7 @@ struct netns_ipv4 {
int sysctl_ip_default_ttl;
int sysctl_ip_no_pmtu_disc;
int sysctl_ip_fwd_use_pmtu;
+ int sysctl_ip_fwd_update_priority;
int sysctl_ip_nonlocal_bind;
/* Shall we try to damage output packets if routing dev changes? */
int sysctl_ip_dynaddr;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index f2a0a3bab6b5..8eb6c74819b8 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1802,6 +1802,7 @@ static __net_init int inet_init_net(struct net *net)
* We set them here, in case sysctl is not compiled.
*/
net->ipv4.sysctl_ip_default_ttl = IPDEFTTL;
+ net->ipv4.sysctl_ip_fwd_update_priority = 1;
net->ipv4.sysctl_ip_dynaddr = 0;
net->ipv4.sysctl_ip_early_demux = 1;
net->ipv4.sysctl_udp_early_demux = 1;
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index b54b948b0596..32662e9e5d21 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -143,7 +143,8 @@ int ip_forward(struct sk_buff *skb)
!skb_sec_path(skb))
ip_rt_send_redirect(skb);
- skb->priority = rt_tos2priority(iph->tos);
+ if (net->ipv4.sysctl_ip_fwd_update_priority)
+ skb->priority = rt_tos2priority(iph->tos);
return NF_HOOK(NFPROTO_IPV4, NF_INET_FORWARD,
net, NULL, skb, skb->dev, rt->dst.dev,
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 5fa335fd3852..e21dda015513 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -664,6 +664,15 @@ static struct ctl_table ipv4_net_table[] = {
.proc_handler = proc_dointvec,
},
{
+ .procname = "ip_forward_update_priority",
+ .data = &init_net.ipv4.sysctl_ip_fwd_update_priority,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
+ {
.procname = "ip_nonlocal_bind",
.data = &init_net.ipv4.sysctl_ip_nonlocal_bind,
.maxlen = sizeof(int),
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 0/7] ipv4: Control SKB reprioritization after forwarding
From: Petr Machata @ 2018-07-31 22:35 UTC (permalink / raw)
To: netdev, linux-doc, linux-kselftest
Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
dsahern
After IPv4 packets are forwarded, the priority of the corresponding SKB
is updated according to the TOS field of IPv4 header. This overrides any
prioritization done earlier by e.g. an skbedit action or ingress-qos-map
defined at a vlan device.
Such overriding may not always be desirable. Even if the packet ends up
being routed, which implies this is an L3 network node, an administrator
may wish to preserve whatever prioritization was done earlier on in the
pipeline.
Therefore this patch set introduces a sysctl that controls this
behavior, net.ipv4.ip_forward_update_priority. It's value is 1 by
default to preserve the current behavior.
All of the above is implemented in patch #1.
Value changes prompt a new NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE
notification, so that the drivers can hook up whatever logic may depend
on this value. That is implemented in patch #2.
In patches #3 and #4, mlxsw is adapted to recognize the sysctl. On
initialization, the RGCR register that handles router configuration is
set in accordance with the sysctl. The new notification is listened to
and RGCR is reconfigured as necessary.
In patches #5 to #7, a selftest is added to verify that mlxsw reflects
the sysctl value as necessary. The test is expressed in terms of the
recently-introduced ieee_setapp support, and works by observing how DSCP
value gets rewritten depending on packet priority. For this reason, the
test is added to the subdirectory drivers/net/mlxsw. Even though it's
not particularly specific to mlxsw, it's not suitable for running on
soft devices (which don't support the ieee_setapp et.al.).
Changes from v1 to v2:
- In patch #1, init sysctl_ip_fwd_update_priority to 1 instead of true.
Changes from RFC to v1:
- Fix wrong sysctl name in ip-sysctl.txt
- Add notifications
- Add mlxsw support
- Add self test
Petr Machata (7):
net: ipv4: Control SKB reprioritization after forwarding
net: ipv4: Notify about changes to ip_forward_update_priority
mlxsw: spectrum: Extract work-scheduling into a new function
mlxsw: spectrum_router: Handle sysctl_ip_fwd_update_priority
selftests: forwarding: Move lldpad waiting to lib.sh
selftests: forwarding: Move DSCP capture to lib.sh
selftests: mlxsw: Add test for ip_forward_update_priority
Documentation/networking/ip-sysctl.txt | 9 +
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 56 +++--
include/net/netevent.h | 1 +
include/net/netns/ipv4.h | 1 +
net/ipv4/af_inet.c | 1 +
net/ipv4/ip_forward.c | 3 +-
net/ipv4/sysctl_net_ipv4.c | 26 +++
.../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh | 65 +-----
.../selftests/drivers/net/mlxsw/qos_dscp_router.sh | 233 +++++++++++++++++++++
tools/testing/selftests/net/forwarding/lib.sh | 63 ++++++
10 files changed, 379 insertions(+), 79 deletions(-)
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
--
2.4.11
^ permalink raw reply
* [PATCH net] net: dsa: Do not suspend/resume closed slave_dev
From: Florian Fainelli @ 2018-08-01 0:12 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
open list
If a DSA slave network device was previously disabled, there is no need
to suspend or resume it.
Fixes: 2446254915a7 ("net: dsa: allow switch drivers to implement suspend/resume hooks")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
net/dsa/slave.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 1e3b6a6d8a40..732369c80644 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1248,6 +1248,9 @@ int dsa_slave_suspend(struct net_device *slave_dev)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
+ if (!netif_running(slave_dev))
+ return 0;
+
netif_device_detach(slave_dev);
rtnl_lock();
@@ -1261,6 +1264,9 @@ int dsa_slave_resume(struct net_device *slave_dev)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
+ if (!netif_running(slave_dev))
+ return 0;
+
netif_device_attach(slave_dev);
rtnl_lock();
--
2.14.1
^ permalink raw reply related
* [PATCH] HID: Bluetooth: hidp: buffer overflow in hidp_process_report
From: Mark Salyzyn @ 2018-07-31 22:02 UTC (permalink / raw)
To: linux-kernel
Cc: Mark Salyzyn, Marcel Holtmann, Johan Hedberg, David S. Miller,
Kees Cook, Benjamin Tissoires, linux-bluetooth, netdev, security,
kernel-team, Jiri Kosina
CVE-2018-9363
The buffer length is unsigned at all layers, but gets cast to int and
checked in hidp_process_report and can lead to a buffer overflow.
Switch len parameter to unsigned int to resolve issue.
This affects 3.18 and newer kernels.
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: security@kernel.org
Cc: kernel-team@android.com
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 1036e4fa1ea2..3bba8f4b08a9 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_session *session)
del_timer(&session->timer);
}
-static void hidp_process_report(struct hidp_session *session,
- int type, const u8 *data, int len, int intr)
+static void hidp_process_report(struct hidp_session *session, int type,
+ const u8 *data, unsigned int len, int intr)
{
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related
* Re: [PATCH] HID: Bluetooth: hidp: buffer overflow in hidp_process_report
From: Kees Cook @ 2018-07-31 23:41 UTC (permalink / raw)
To: Mark Salyzyn
Cc: LKML, Marcel Holtmann, Johan Hedberg, David S. Miller,
Benjamin Tissoires, linux-bluetooth, Network Development,
Security Officers, Android Kernel Team, Jiri Kosina
In-Reply-To: <20180731220225.159741-1-salyzyn@android.com>
On Tue, Jul 31, 2018 at 3:02 PM, Mark Salyzyn <salyzyn@android.com> wrote:
> CVE-2018-9363
>
> The buffer length is unsigned at all layers, but gets cast to int and
> checked in hidp_process_report and can lead to a buffer overflow.
> Switch len parameter to unsigned int to resolve issue.
>
> This affects 3.18 and newer kernels.
>
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> Fixes: a4b1b5877b514b276f0f31efe02388a9c2836728 ("HID: Bluetooth: hidp: make sure input buffers are big enough")
nit: normally just first 12 of the sha is used.
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: linux-bluetooth@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: security@kernel.org
> Cc: kernel-team@android.com
> ---
> net/bluetooth/hidp/core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 1036e4fa1ea2..3bba8f4b08a9 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -431,8 +431,8 @@ static void hidp_del_timer(struct hidp_session *session)
> del_timer(&session->timer);
> }
>
> -static void hidp_process_report(struct hidp_session *session,
> - int type, const u8 *data, int len, int intr)
> +static void hidp_process_report(struct hidp_session *session, int type,
> + const u8 *data, unsigned int len, int intr)
> {
> if (len > HID_MAX_BUFFER_SIZE)
> len = HID_MAX_BUFFER_SIZE;
Acked-by: Kees Cook <keescook@chromium.org>
Thanks!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [PATCH] net/mlx5e: Fix uninitialized variable
From: Saeed Mahameed @ 2018-07-31 21:47 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Tariq Toukan, Saeed Mahameed, Leon Romanovsky, David S. Miller,
Linux Netdev List, RDMA mailing list, linux-kernel
In-Reply-To: <20180731142157.GA24066@embeddedor.com>
On Tue, Jul 31, 2018 at 7:21 AM, Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> There is a potential execution path in which variable *err* is returned
> without being properly initialized previously.
>
> Fix this by initializing variable *err* to 0.
>
> Addresses-Coverity-ID: 1472116 ("Uninitialized scalar variable")
> Fixes: 0ec13877ce95 ("net/mlx5e: Gather all XDP pre-requisite checks in a single function")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply
* Re: [PATCH net] enic: do not call enic_change_mtu in enic_probe
From: David Miller @ 2018-07-31 21:45 UTC (permalink / raw)
To: gvaradar; +Cc: netdev, benve
In-Reply-To: <20180730165654.3816-1-gvaradar@cisco.com>
From: Govindarajulu Varadarajan <gvaradar@cisco.com>
Date: Mon, 30 Jul 2018 09:56:54 -0700
> In commit ab123fe071c9 ("enic: handle mtu change for vf properly")
> ASSERT_RTNL() is added to _enic_change_mtu() to prevent it from being
> called without rtnl held. enic_probe() calls enic_change_mtu()
> without rtnl held. At this point netdev is not registered yet.
> Remove call to enic_change_mtu and assign the mtu to netdev->mtu.
>
> Fixes: ab123fe071c9 ("enic: handle mtu change for vf properly")
> Signed-off-by: Govindarajulu Varadarajan <gvaradar@cisco.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] ipv4: frags: handle possible skb truesize change
From: David Miller @ 2018-07-31 21:45 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180731045029.48957-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 30 Jul 2018 21:50:29 -0700
> ip_frag_queue() might call pskb_pull() on one skb that
> is already in the fragment queue.
>
> We need to take care of possible truesize change, or we
> might have an imbalance of the netns frags memory usage.
>
> IPv6 is immune to this bug, because RFC5722, Section 4,
> amended by Errata ID 3089 states :
>
> When reassembling an IPv6 datagram, if
> one or more its constituent fragments is determined to be an
> overlapping fragment, the entire datagram (and any constituent
> fragments) MUST be silently discarded.
>
> Fixes: 158f323b9868 ("net: adjust skb->truesize in pskb_expand_head()")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable.
Frankly, I think we should enforce the ipv6 rules for ipv4 too.
There is absolutely no reasonable situation where overlapping fragment
queue entries should arrive. None whatsoever.
^ permalink raw reply
* Re: [PATCH net] inet: frag: enforce memory limits earlier
From: David Miller @ 2018-07-31 21:44 UTC (permalink / raw)
To: edumazet; +Cc: netdev, jannh, eric.dumazet, fw, posk, pabeni
In-Reply-To: <20180731030911.248637-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 30 Jul 2018 20:09:11 -0700
> We currently check current frags memory usage only when
> a new frag queue is created. This allows attackers to first
> consume the memory budget (default : 4 MB) creating thousands
> of frag queues, then sending tiny skbs to exceed high_thresh
> limit by 2 to 3 order of magnitude.
>
> Note that before commit 648700f76b03 ("inet: frags: use rhashtables
> for reassembly units"), work queue could be starved under DOS,
> getting no cpu cycles.
> After commit 648700f76b03, only the per frag queue timer can eventually
> remove an incomplete frag queue and its skbs.
>
> Fixes: b13d3cbfb8e8 ("inet: frag: move eviction of queues to work queue")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Jann Horn <jannh@google.com>
Applied and queued up for -stable.
^ 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