* Re: [patch net] devlink: convert occ_get op to separate registration
From: Jiri Pirko @ 2018-04-05 21:17 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, idosch, jakub.kicinski, mlxsw
In-Reply-To: <0880af20-f0a2-bc19-6b1d-0e42a7937184@gmail.com>
Thu, Apr 05, 2018 at 10:55:58PM CEST, dsahern@gmail.com wrote:
>On 4/5/18 2:13 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> This resolves race during initialization where the resources with
>> ops are registered before driver and the structures used by occ_get
>> op is initialized. So keep occ_get callbacks registered only when
>> all structs are initialized.
>>
>> The example flows, as it is in mlxsw:
>> 1) driver load/asic probe:
>> mlxsw_core
>> -> mlxsw_sp_resources_register
>> -> mlxsw_sp_kvdl_resources_register
>> -> devlink_resource_register IDX
>> mlxsw_spectrum
>> -> mlxsw_sp_kvdl_init
>> -> mlxsw_sp_kvdl_parts_init
>> -> mlxsw_sp_kvdl_part_init
>> -> devlink_resource_size_get IDX (to get the current setup
>> size from devlink)
>> -> devlink_resource_occ_get_register IDX (register current
>> occupancy getter)
>> 2) reload triggered by devlink command:
>> -> mlxsw_devlink_core_bus_device_reload
>> -> mlxsw_sp_fini
>> -> mlxsw_sp_kvdl_fini
>> -> devlink_resource_occ_get_unregister IDX
>> (struct mlxsw_sp *mlxsw_sp is freed at this point, call to occ get
>> which is using mlxsw_sp would cause use-after free)
>> -> mlxsw_sp_init
>> -> mlxsw_sp_kvdl_init
>> -> mlxsw_sp_kvdl_parts_init
>> -> mlxsw_sp_kvdl_part_init
>> -> devlink_resource_size_get IDX (to get the current setup
>> size from devlink)
>> -> devlink_resource_occ_get_register IDX (register current
>> occupancy getter)
>>
>> Fixes: d9f9b9a4d05f ("devlink: Add support for resource abstraction")
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>
>
>Why won't something like the attached work as opposed to playing
>register / unregister games?
Because it is quite driver-specific. For example in netdevsim, with
current implementation you don't have to unreg as the priv is there the
whole time.
Also, I think it is more correct to register getter with the priv
directly. Not to depend on getting with via devlink struct somehow.
I'm not saying that my solution is super nice. But what you suggest
seems much more uglier to me.
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
>index 93ea56620a24..dcded613faa6 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
>@@ -113,6 +113,7 @@ struct mlxsw_core {
> struct mlxsw_thermal *thermal;
> struct mlxsw_core_port *ports;
> unsigned int max_ports;
>+ bool reload_in_progress;
> bool reload_fail;
> unsigned long driver_priv[0];
> /* driver_priv has to be always the last item */
>@@ -154,6 +155,12 @@ void *mlxsw_core_driver_priv(struct mlxsw_core *mlxsw_core)
> }
> EXPORT_SYMBOL(mlxsw_core_driver_priv);
>
>+bool mlxsw_core_reload_in_progress(struct mlxsw_core *mlxsw_core)
>+{
>+ return mlxsw_core->mlxsw_core_driver_priv;
>+}
>+EXPORT_SYMBOL(mlxsw_core_reload_in_progress);
>+
> struct mlxsw_rx_listener_item {
> struct list_head list;
> struct mlxsw_rx_listener rxl;
>@@ -972,12 +979,16 @@ static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink)
> if (!mlxsw_bus->reset)
> return -EOPNOTSUPP;
>
>+ mlxsw_core->reload_in_progress = true;
>+
> mlxsw_core_bus_device_unregister(mlxsw_core, true);
> mlxsw_bus->reset(mlxsw_core->bus_priv);
> err = mlxsw_core_bus_device_register(mlxsw_core->bus_info,
> mlxsw_core->bus,
> mlxsw_core->bus_priv, true,
> devlink);
>+ mlxsw_core->reload_in_progress = false;
>+
> if (err)
> mlxsw_core->reload_fail = true;
> return err;
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
>index 092d39399f3c..ffa1db154757 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
>+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
>@@ -60,6 +60,7 @@ struct mlxsw_bus_info;
> unsigned int mlxsw_core_max_ports(const struct mlxsw_core *mlxsw_core);
>
> void *mlxsw_core_driver_priv(struct mlxsw_core *mlxsw_core);
>+bool mlxsw_core_reload_in_progress(struct mlxsw_core *mlxsw_core);
>
> int mlxsw_core_driver_register(struct mlxsw_driver *mlxsw_driver);
> void mlxsw_core_driver_unregister(struct mlxsw_driver *mlxsw_driver);
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>index 53fffd09d133..09b89af37d8a 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>@@ -3808,8 +3808,12 @@ static const struct mlxsw_config_profile mlxsw_sp_config_profile = {
> static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
> {
> struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
>+ struct mlxsw_sp *mlxsw_sp;
>+
>+ if (mlxsw_core_reload_in_progress(mlxsw_core))
>+ return 0;
>
>+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> return mlxsw_sp_kvdl_occ_get(mlxsw_sp);
> }
>
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
>index 8796db44dcc3..dd66285bafb5 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
>@@ -329,9 +329,13 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
> static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
> {
> struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> struct mlxsw_sp_kvdl_part *part;
>+ struct mlxsw_sp *mlxsw_sp;
>
>+ if (mlxsw_core_reload_in_progress(mlxsw_core))
>+ return 0;
>+
>+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
> return mlxsw_sp_kvdl_part_occ(part);
> }
>@@ -339,8 +343,13 @@ static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
> static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
> {
> struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> struct mlxsw_sp_kvdl_part *part;
>+ struct mlxsw_sp *mlxsw_sp;
>+
>+ if (mlxsw_core_reload_in_progress(mlxsw_core))
>+ return 0;
>+
>+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
>
> part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
> return mlxsw_sp_kvdl_part_occ(part);
>@@ -349,9 +358,13 @@ static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
> static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
> {
> struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> struct mlxsw_sp_kvdl_part *part;
>+ struct mlxsw_sp *mlxsw_sp;
>+
>+ if (mlxsw_core_reload_in_progress(mlxsw_core))
>+ return 0;
>
>+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
> part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
> return mlxsw_sp_kvdl_part_occ(part);
> }
^ permalink raw reply
* [PATCH net] net/ipv6: Increment OUTxxx counters after netfilter hook
From: Jeff Barnhill @ 2018-04-05 21:29 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Jeff Barnhill
At the end of ip6_forward(), IPSTATS_MIB_OUTFORWDATAGRAMS and
IPSTATS_MIB_OUTOCTETS are incremented immediately before the NF_HOOK call
for NFPROTO_IPV6 / NF_INET_FORWARD. As a result, these counters get
incremented regardless of whether or not the netfilter hook allows the
packet to continue being processed. This change increments the counters
in ip6_forward_finish() so that it will not happen if the netfilter hook
chooses to terminate the packet, which is similar to how IPv4 works.
Signed-off-by: Jeff Barnhill <0xeffeff@gmail.com>
---
net/ipv6/ip6_output.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index b8ee50e94af3..2e891d2c30ef 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -375,6 +375,11 @@ static int ip6_forward_proxy_check(struct sk_buff *skb)
static inline int ip6_forward_finish(struct net *net, struct sock *sk,
struct sk_buff *skb)
{
+ struct dst_entry *dst = skb_dst(skb);
+
+ __IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
+ __IP6_ADD_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTOCTETS, skb->len);
+
return dst_output(net, sk, skb);
}
@@ -569,8 +574,6 @@ int ip6_forward(struct sk_buff *skb)
hdr->hop_limit--;
- __IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTFORWDATAGRAMS);
- __IP6_ADD_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTOCTETS, skb->len);
return NF_HOOK(NFPROTO_IPV6, NF_INET_FORWARD,
net, NULL, skb, skb->dev, dst->dev,
ip6_forward_finish);
--
2.14.1
^ permalink raw reply related
* Re: Best userspace programming API for XDP features query to kernel?
From: Jakub Kicinski via iovisor-dev @ 2018-04-05 21:47 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: oisf-devel-ZwoEplunGu2j570ONfqVQLVmwVP6tfMwSoIsB4E12gc,
Alexei Starovoitov, Victor Julien, netdev-u79uwXL29TY76Z2rM5mHXA,
iovisor-dev-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org,
Peter Manev, Jiri Benc, Saeed Mahameed, Eric Leblond,
Daniel Borkmann
In-Reply-To: <20180405225133.18a09883-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, 5 Apr 2018 22:51:33 +0200, Jesper Dangaard Brouer wrote:
> > What about nfp in terms of XDP
> > offload capabilities, should they be included as well or is probing to load
> > the program and see if it loads/JITs as we do today just fine (e.g. you'd
> > otherwise end up with extra flags on a per BPF helper basis)?
>
> No, flags per BPF helper basis. As I've described above, helper belong
> to the BPF core, not the driver. Here I want to know what the specific
> driver support.
I think Daniel meant for nfp offload. The offload restrictions are
quite involved, are we going to be able to express those?
This is a bit simpler but reminds me of the TC flower capability
discussion. Expressing features and capabilities gets messy quickly.
I have a gut feeling that a good starting point would be defining and
building a test suite or a set of probing tests to check things work at
system level (incl. redirects to different ports etc.) I think having
a concrete set of litmus tests that confirm the meaning of a given
feature/capability would go a long way in making people more comfortable
with accepting any form of BPF driver capability. And serious BPF
projects already do probing so it's just centralizing this in the
kernel.
That's my two cents.
^ permalink raw reply
* Re: [PATCH net] net: dsa: Discard frames from unused ports
From: Florian Fainelli @ 2018-04-05 22:20 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Vivien Didelot
In-Reply-To: <20180405021757.GA1838@lunn.ch>
On 04/04/2018 07:17 PM, Andrew Lunn wrote:
> On Wed, Apr 04, 2018 at 05:49:10PM -0700, Florian Fainelli wrote:
>> On 04/04/2018 04:56 PM, Andrew Lunn wrote:
>>> The Marvell switches under some conditions will pass a frame to the
>>> host with the port being the CPU port. Such frames are invalid, and
>>> should be dropped. Not dropping them can result in a crash when
>>> incrementing the receive statistics for an invalid port.
>>>
>>> Reported-by: Chris Healy <cphealy@gmail.com>
>>> Fixes: 5f6b4e14cada ("net: dsa: User per-cpu 64-bit statistics")
>>
>> Are you sure this is the commit that introduced the problem?
>
> Hi Florian
>
> Well, the problem is it crashes when trying to update the
> statistics. The CPU port is not allocated a p->stats64, only slave
> ports get those. So before this patch, there was no crash and the
> frame would be delivered to the master interface. This in itself is
> probably not correct, but also not fatal. Talking to Chris, it seems
> this behaviour has existing for a long while. I needed to use lldpd to
> trigger the issue, because i assume the Marvell switch sees these as
> special frames and forwards them to the CPU. The other thing is, the
> code got refactored recently. So this fix will not rebase to too many
> earlier versions. It needs a fix per tagging protocol for before the
> common dsa_master_find_slave() was added.
Yes what you are explaining makes sense, but does not that mean we would
just be accessing a garbage memory location before as well? It seems to
me like this problem dates back from long before that particular
changeset, and this changeset just made it more obvious. Would that be
accurate?
>
>>> - return ds->ports[port].slave;
>>> + slave_port = &ds->ports[port];
>>> +
>>> + if (slave_port->type != DSA_PORT_TYPE_USER)
>>
>> Can we optimize this with an unlikely()?
>
> Yes, that would make sense.
>
> Andrew
>
--
Florian
^ permalink raw reply
* Re: Enable and configure storm prevention in a network device
From: Florian Fainelli @ 2018-04-05 22:35 UTC (permalink / raw)
To: David Miller, m-karicheri2; +Cc: netdev, andrew
In-Reply-To: <20180405.162031.2009953983418308744.davem@davemloft.net>
On 04/05/2018 01:20 PM, David Miller wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>
> Date: Thu, 5 Apr 2018 16:14:49 -0400
>
>> Is there a standard way to implement and configure storm prevention
>> in a Linux network device?
>
> What kind of "storm", an interrupt storm?
>
I would assume Murali is referring to L2 broadcast storms which is
common in switches. There is not an API for that AFAICT and I am not
sure what a proper API would look like.
--
Florian
^ permalink raw reply
* [PATCH net] net/sched: fix NULL dereference in the error path of tcf_bpf_init()
From: Davide Caratti @ 2018-04-05 23:19 UTC (permalink / raw)
To: David S. Miller; +Cc: Jamal Hadi Salim, Cong Wang, netdev, Lucas Bates
when tcf_bpf_init_from_ops() fails (e.g. because of program having invalid
number of instructions), tcf_bpf_cfg_cleanup() calls bpf_prog_put(NULL) or
bpf_prog_destroy(NULL). Unless CONFIG_BPF_SYSCALL is unset, this causes
the following error:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
PGD 800000007345a067 P4D 800000007345a067 PUD 340e1067 PMD 0
Oops: 0000 [#1] SMP PTI
Modules linked in: act_bpf(E) ip6table_filter ip6_tables iptable_filter binfmt_misc ext4 mbcache jbd2 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_codec_generic pcbc snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm aesni_intel crypto_simd glue_helper cryptd joydev snd_timer snd virtio_balloon pcspkr soundcore i2c_piix4 nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c ata_generic pata_acpi qxl drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm virtio_blk drm virtio_net virtio_console i2c_core crc32c_intel serio_raw virtio_pci ata_piix libata virtio_ring floppy virtio dm_mirror dm_region_hash dm_log dm_mod [last unloaded: act_bpf]
CPU: 3 PID: 5654 Comm: tc Tainted: G E 4.16.0.bpf_test+ #408
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
RIP: 0010:__bpf_prog_put+0xc/0xc0
RSP: 0018:ffff9594003ef728 EFLAGS: 00010202
RAX: 0000000000000000 RBX: ffff9594003ef758 RCX: 0000000000000024
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000044
R10: 0000000000000220 R11: ffff8a7ab9f17131 R12: 0000000000000000
R13: ffff8a7ab7c3c8e0 R14: 0000000000000001 R15: ffff8a7ab88f1054
FS: 00007fcb2f17c740(0000) GS:ffff8a7abfd80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000020 CR3: 000000007c888006 CR4: 00000000001606e0
Call Trace:
tcf_bpf_cfg_cleanup+0x2f/0x40 [act_bpf]
tcf_bpf_cleanup+0x4c/0x70 [act_bpf]
__tcf_idr_release+0x79/0x140
tcf_bpf_init+0x125/0x330 [act_bpf]
tcf_action_init_1+0x2cc/0x430
? get_page_from_freelist+0x3f0/0x11b0
tcf_action_init+0xd3/0x1b0
tc_ctl_action+0x18b/0x240
rtnetlink_rcv_msg+0x29c/0x310
? _cond_resched+0x15/0x30
? __kmalloc_node_track_caller+0x1b9/0x270
? rtnl_calcit.isra.29+0x100/0x100
netlink_rcv_skb+0xd2/0x110
netlink_unicast+0x17c/0x230
netlink_sendmsg+0x2cd/0x3c0
sock_sendmsg+0x30/0x40
___sys_sendmsg+0x27a/0x290
? mem_cgroup_commit_charge+0x80/0x130
? page_add_new_anon_rmap+0x73/0xc0
? do_anonymous_page+0x2a2/0x560
? __handle_mm_fault+0xc75/0xe20
__sys_sendmsg+0x58/0xa0
do_syscall_64+0x6e/0x1a0
entry_SYSCALL_64_after_hwframe+0x3d/0xa2
RIP: 0033:0x7fcb2e58eba0
RSP: 002b:00007ffc93c496c8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007ffc93c497f0 RCX: 00007fcb2e58eba0
RDX: 0000000000000000 RSI: 00007ffc93c49740 RDI: 0000000000000003
RBP: 000000005ac6a646 R08: 0000000000000002 R09: 0000000000000000
R10: 00007ffc93c49120 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffc93c49804 R14: 0000000000000001 R15: 000000000066afa0
Code: 5f 00 48 8b 43 20 48 c7 c7 70 2f 7c b8 c7 40 10 00 00 00 00 5b e9 a5 8b 61 00 0f 1f 44 00 00 0f 1f 44 00 00 41 54 55 48 89 fd 53 <48> 8b 47 20 f0 ff 08 74 05 5b 5d 41 5c c3 41 89 f4 0f 1f 44 00
RIP: __bpf_prog_put+0xc/0xc0 RSP: ffff9594003ef728
CR2: 0000000000000020
Fix it in tcf_bpf_cfg_cleanup(), ensuring that bpf_prog_{put,destroy}(f)
is called only when f is not NULL.
Fixes: bbc09e7842a5 ("net/sched: fix idr leak on the error path of tcf_bpf_init()")
Reported-by: Lucas Bates <lucasb@mojatatu.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_bpf.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 9092531d45d8..18089c02e557 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -248,10 +248,14 @@ static int tcf_bpf_init_from_efd(struct nlattr **tb, struct tcf_bpf_cfg *cfg)
static void tcf_bpf_cfg_cleanup(const struct tcf_bpf_cfg *cfg)
{
- if (cfg->is_ebpf)
- bpf_prog_put(cfg->filter);
- else
- bpf_prog_destroy(cfg->filter);
+ struct bpf_prog *filter = cfg->filter;
+
+ if (filter) {
+ if (cfg->is_ebpf)
+ bpf_prog_put(filter);
+ else
+ bpf_prog_destroy(filter);
+ }
kfree(cfg->bpf_ops);
kfree(cfg->bpf_name);
--
2.14.3
^ permalink raw reply related
* Packet corrupts to guest system system from host kernel 4.16.x
From: Алексей Болдырев @ 2018-04-06 0:49 UTC (permalink / raw)
To: qemu-discuss, netdev
Why, when using the 4.16 kernel on the host system, is there such a strange behavior of virtual machines? What is the problem? He rolled back to 4.9.c - the problem was gone.
tcpdump on router:
tcpdump: listening on vlan-00110013, link-type EN10MB (Ethernet), capture size 262144 bytes
23:59:08.331875 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype ARP (0x0806), length 38: [|ARP]
0x0000: 0001 0800 0604 0001 5254 0038 cf94 c0a8 ........RT.8....
0x0010: b402 0000 0000 0000 ........
23:59:08.454364 52:54:00:4e:9c:5f > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 42: Ethernet (len 6), IPv4 (len 4), Request who-has 192.168.180.2 tell 192.168.180.1, length 28
23:59:08.454754 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype ARP (0x0806), length 38: [|ARP]
0x0000: 0001 0800 0604 0002 5254 0038 cf94 c0a8 ........RT.8....
0x0010: b402 5254 004e 9c5f ..RT.N._
23:59:09.462383 52:54:00:4e:9c:5f > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 42: Ethernet (len 6), IPv4 (len 4), Request who-has 192.168.180.2 tell 192.168.180.1, length 28
23:59:09.463607 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype ARP (0x0806), length 38: [|ARP]
0x0000: 0001 0800 0604 0002 5254 0038 cf94 c0a8 ........RT.8....
0x0010: b402 5254 004e 9c5f ..RT.N._
23:59:10.486352 52:54:00:4e:9c:5f > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 42: Ethernet (len 6), IPv4 (len 4), Request who-has 192.168.180.2 tell 192.168.180.1, length 28
23:59:10.487303 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype ARP (0x0806), length 38: [|ARP]
0x0000: 0001 0800 0604 0002 5254 0038 cf94 c0a8 ........RT.8....
0x0010: b402 5254 004e 9c5f ..RT.N._
23:59:11.051570 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype IPv4 (0x0800), length 77: truncated-ip - 4 bytes missing! (tos 0x0, ttl 64, id 54539, offset 0, flags [DF], proto UDP (17), length 67)
192.168.180.2.59917 > 198.18.120.1.53: 1196+[|domain]
23:59:11.051585 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype IPv4 (0x0800), length 77: truncated-ip - 4 bytes missing! (tos 0x0, ttl 64, id 54540, offset 0, flags [DF], proto UDP (17), length 67)
192.168.180.2.59917 > 198.18.120.1.53: 5849+[|domain]
23:59:11.527109 52:54:00:4e:9c:5f > ff:ff:ff:ff:ff:ff, ethertype ARP (0x0806), length 42: Ethernet (len 6), IPv4 (len 4), Request who-has 192.168.180.2 tell 192.168.180.1, length 28
23:59:11.527707 52:54:00:38:cf:94 > 52:54:00:4e:9c:5f, ethertype ARP (0x0806), length 38: [|ARP]
0x0000: 0001 0800 0604 0002 5254 0038 cf94 c0a8 ........RT.8....
0x0010: b402 5254 004e 9c5f ..RT.N._
^C
11 packets captured
11 packets received by filter
0 packets dropped by kernel
^ permalink raw reply
* Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh
From: Masahiro Yamada @ 2018-04-06 0:59 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, lkml, netdev,
Linux Kbuild mailing list, Quentin Monnet, Eugene Syromiatnikov,
Jiri Benc, Stanislav Kozina, Jerome Marchand,
Arnaldo Carvalho de Melo, Michal Marek, Jiri Kosina
In-Reply-To: <20180405185950.GA3668@krava>
2018-04-06 3:59 GMT+09:00 Jiri Olsa <jolsa@redhat.com>:
> On Fri, Apr 06, 2018 at 12:50:00AM +0900, Masahiro Yamada wrote:
>> 2018-04-06 0:16 GMT+09:00 Jiri Olsa <jolsa@kernel.org>:
>> > There's no need to pass LD* arguments to link-vmlinux.sh,
>> > because they are passed as variables. The only argument
>> > the link-vmlinux.sh supports is the 'clean' argument.
>> >
>> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>> > ---
>>
>> Wrong.
>>
>> $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
>> exist here so that any change in them
>> invokes scripts/linkk-vmlinux.sh
>
> sry, I can't see that.. but it's just a side fix,
> which is actually not needed for the rest
>
> I'll check on more and address this separately
The link command is recorded in .vmlinux.cmd
then, it is checked by if_changed in the next rebuild
because link-vmlinux is invoked by $(call if_changed,...)
+$(call if_changed,link-vmlinux)
> thanks,
> jirka
>
>>
>>
>>
>>
>> > Makefile | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index d3300e46f925..a65a3919c6ad 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>> >
>> > # Final link of vmlinux with optional arch pass after final link
>> > cmd_link-vmlinux = \
>> > - $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ; \
>> > + $(CONFIG_SHELL) $< ; \
>> > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>> >
>> > vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
>>
>>
>>
>>
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/3] bpf/verifier: subprog/func_call simplifications
From: Alexei Starovoitov @ 2018-04-06 1:20 UTC (permalink / raw)
To: Jiong Wang; +Cc: Edward Cree, Daniel Borkmann, netdev
In-Reply-To: <2ff89131-c6ea-5ddf-156c-c6f6e455fbdd@netronome.com>
On Thu, Apr 05, 2018 at 04:50:03PM +0100, Jiong Wang wrote:
> On 03/04/2018 02:08, Alexei Starovoitov wrote:
> > Combining subprog pass with do_check is going into opposite direction
> > of this long term work. Divide and conquer. Combining more things into
> > do_check is the opposite of this programming principle.
>
> Agree. And for the redundant insn traversal issue in check_subprogs that
> Edward trying to fix, I am thinking we could do it by utilizing the
> existing DFS traversal in check_cfg.
>
> The current DFS probably could be improved into an generic instruction
> information collection pass.
>
> This won't make the existing DFS complexer as it only does information
> collection as a side job during traversal. These collected information
> then could be used to build any other information to be consumed later,
> for example subprog, basic blocks etc.
>
> For the redundant insn traversal issue during subprog detection, the
> Like how we mark STATE_LIST_MARK in DFS, we could just call add_subprog
> for BPF_PSEUDO_CALL insn during DFS.
>
> i.e we change the code logic of check_cfg into:
>
> check_cfg
> {
> * DFS traversal:
> - detect back-edge.
> - collect STATE_LIST_MARK.
> - collect subprog destination.
>
> * check all insns are reachable.
> * check_subprogs (insn traversal removed).
> }
I don't think that will work.
Functions are independent from CFG.
With bpf libraries we will have disjoint functions sitting in the kernel
and check_cfg would need to be done separately from function boundaries.
^ permalink raw reply
* (unknown),
From: venkatvenkatsubra @ 2018-04-06 1:18 UTC (permalink / raw)
To: netdev
Hi Netdev https://goo.gl/5bDZtk
^ permalink raw reply
* Re: [Cluster-devel] [PATCH v2 0/2] gfs2: Stop using rhashtable_walk_peek
From: NeilBrown @ 2018-04-06 1:33 UTC (permalink / raw)
To: Andreas Grünbacher, Herbert Xu
Cc: Andreas Gruenbacher, Bob Peterson, Thomas Graf, Tom Herbert,
cluster-devel, linux-kernel, netdev
In-Reply-To: <CAHpGcMKxDNOkMMKB1_9H0ob-502cD89E-94a18prp91y_a52GA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2687 bytes --]
On Wed, Apr 04 2018, Andreas Grünbacher wrote:
> Herbert Xu <herbert@gondor.apana.org.au> schrieb am Mi. 4. Apr. 2018 um
> 17:51:
>
>> On Wed, Apr 04, 2018 at 11:46:28AM -0400, Bob Peterson wrote:
>> >
>> > The patches look good. The big question is whether to add them to this
>> > merge window while it's still open. Opinions?
>>
>> We're still hashing out the rhashtable interface so I don't think now is
>> the time to rush things.
>
>
> Fair enough. No matter how rhashtable_walk_peek changes, we‘ll still need
> these two patches to fix the glock dump though.
Those two patches look fine to me and don't depend on changes to
rhashtable, so it is up to you when they go upstream.
However, I think the code can be substantially simplified, particularly
once we make rhashtable a little cleverer.
So this is what I'll probably be doing for a similar situation in
lustre....
Having examined seqfile closely, it is apparent that if ->start never
changes *ppos, and if ->next always increments it (except maybe on error)
then
1/ ->next is only ever given a 'pos' that was returned by the previous
call to ->start or ->next. So it should *always* return the next
object, after the one previously returned by ->start or ->next. It
never needs to 'seek'. The 'traverse()' function in seq_file.c does
any seeking needed. ->next needs to increase 'pos' and when walking
a dynamic list, it is easiest if it just increments it.
2/ ->start is only called with a pos of:
0 - in this case it should rewind to the start
the last pos passed to ->start of ->next
In this case it should return the same thing that was
returned last time. If it no longer exists, then
the following one should be returned.
one more than the last pos passed to ->start or ->next
In this case it should return the object after the
last one returned.
The proposed enhancement to rhashtable_walk* is to add a
rhashtable_walk_prev() which returns the previously returned object,
if it is still in the table, or NULL. It also enhances
rhashtable_walk_start() so that if the previously returned object is
still in the table, it is preserved as the current cursor.
This means that if you take some action to ensure that the
previously returned object remains in the table until the next ->start,
then you can reliably walk the table with no duplicates or omissions
(unless a concurrent rehash causes duplicates)
If you don't keep the object in the table and it gets removed, then
the 'skip' counter is used to find your place, and you might get
duplicates or omissions.
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [RFC 0/9] bpf: Add buildid check support
From: Alexei Starovoitov @ 2018-04-06 1:37 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, lkml, netdev, linux-kbuild,
Quentin Monnet, Eugene Syromiatnikov, Jiri Benc, Stanislav Kozina,
Jerome Marchand, Arnaldo Carvalho de Melo, Masahiro Yamada,
Michal Marek, Jiri Kosina
In-Reply-To: <20180405151645.19130-1-jolsa@kernel.org>
On Thu, Apr 05, 2018 at 05:16:36PM +0200, Jiri Olsa wrote:
> hi,
> eBPF programs loaded for kprobes are allowed to read kernel
> internal structures. We check the provided kernel version
> to ensure that the program is loaded for the proper kernel.
>
> The problem is that the version check is not enough, because
> it only follows the version setup from kernel's Makefile.
> However, the internal kernel structures change based on the
> .config data, so in practise we have different kernels with
> same version.
>
> The eBPF kprobe program thus then get loaded for different
> kernel than it's been built for, get wrong data (silently)
> and provide misleading output.
>
> This patchset implements additional check in eBPF loading code
> on provided build ID (from kernel's elf image, .notes section
> GNU build ID) to ensure we load the eBPF program on correct
> kernel.
>
> Also available in here (based on bpf-next/master):
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> bpf/checksum
>
> This patchset consists of several changes:
>
> - adding CONFIG_BUILDID_H option that instructs the build
> to generate uapi header file with build ID data, that
> will be included by eBPF program
>
> - adding CONFIG_BPF_BUILDID_CHECK option and new bpf_attr
> field to allow build ID checking when loading the eBPF
> program
>
> - changing libbpf to read and pass build ID to the kernel
>
> - several small side fixes
>
> - example perf eBPF code in bpf-samples/bpf-stdout-example.c
> to show the build ID support/usage.
>
> # perf record -vv -e ./bpf-samples/bpf-stdout-example.c kill 2>&1 | grep buildid
> libbpf: section(7) buildid, size 21, link 0, flags 3, type=1
> libbpf: kernel buildid of ./bpf-samples/bpf-stdout-example.c is: 6e25edeb408513184e2753bebad25d42314501a0
>
> The buildid is provided the same way we provide kernel
> version, in a special "buildid" section:
>
> # cat ./bpf-samples/bpf-stdout-example.c
> ...
> #include <linux/buildid.h>
>
> char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA;
> ...
>
> where LINUX_BUILDID_DATA is defined in the generated buildid.h.
>
> please note it's an RFC ;-) any comments and suggestions are welcome
I think this is overkill.
We're very heavy users of kprobe+bpf. It's used for lots
of different cases and usage is constantly growing,
but I haven't seen a single case of :
> The eBPF kprobe program thus then get loaded for different
> kernel than it's been built for, get wrong data (silently)
> and provide misleading output.
but I saw plenty of the opposite. People pre-compile the program
and hack kernel version when they load, since they know in advance
that kprobe+bpf doesn't use any kernel specific things.
The existing kernel version check for kprobe+bpf is already annoying
to them.
This buildid check they can easily bypass the same way.
imo the whole thing just adds complexity and doesn't solve anything.
Sorry, this is a Nack.
^ permalink raw reply
* [PATCH] make net_gso_ok return false when gso_type is zero(invalid)
From: Wenhua Shi @ 2018-04-06 1:43 UTC (permalink / raw)
Cc: David S. Miller, netdev, linux-kernel
Signed-off-by: Wenhua Shi <march511@gmail.com>
---
include/linux/netdevice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cf44503e..1f26cbcf 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4187,7 +4187,7 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
BUILD_BUG_ON(SKB_GSO_ESP != (NETIF_F_GSO_ESP >> NETIF_F_GSO_SHIFT));
BUILD_BUG_ON(SKB_GSO_UDP != (NETIF_F_GSO_UDP >> NETIF_F_GSO_SHIFT));
- return (features & feature) == feature;
+ return feature && (features & feature) == feature;
}
static inline bool skb_gso_ok(struct sk_buff *skb, netdev_features_t features)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] netns: filter uevents correctly
From: David Miller @ 2018-04-06 2:02 UTC (permalink / raw)
To: christian.brauner
Cc: ebiederm, gregkh, netdev, linux-kernel, avagin, ktkhai, serge
In-Reply-To: <CAPP7u0VQc33MiYP8Ene6K1Bqkx4LtBtQ0ugFYp_36UsxuusT1g@mail.gmail.com>
From: Christian Brauner <christian.brauner@canonical.com>
Date: Thu, 05 Apr 2018 01:27:16 +0000
> David, is it ok to queue this or would you prefer I resend when net-next
> reopens?
This definitely needs more discussion, and after the discussion some
further clarification in the commit log message based upon that
discussion if we move forward with this change at all.
Thank you.
^ permalink raw reply
* Re: [PATCH net] net: dsa: Discard frames from unused ports
From: David Miller @ 2018-04-06 2:04 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, vivien.didelot
In-Reply-To: <1522886204-1545-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu, 5 Apr 2018 01:56:44 +0200
> The Marvell switches under some conditions will pass a frame to the
> host with the port being the CPU port. Such frames are invalid, and
> should be dropped. Not dropping them can result in a crash when
> incrementing the receive statistics for an invalid port.
>
> Reported-by: Chris Healy <cphealy@gmail.com>
> Fixes: 5f6b4e14cada ("net: dsa: User per-cpu 64-bit statistics")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
...
> + slave_port = &ds->ports[port];
> +
> + if (slave_port->type != DSA_PORT_TYPE_USER)
> + return NULL;
Look like we need a Fixes: update and an adjustment to use unlikely()
here based upon Florian's feedback.
^ permalink raw reply
* Re: [PATCH net] arp: fix arp_filter on l3slave devices
From: David Miller @ 2018-04-06 2:05 UTC (permalink / raw)
To: dsahern; +Cc: mfadon, netdev
In-Reply-To: <8ea161a0-b164-8f05-5026-d416f17fbdfb@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Thu, 5 Apr 2018 08:40:48 -0600
> On 4/5/18 2:25 AM, Miguel Fadon Perlines wrote:
>> arp_filter performs an ip_route_output search for arp source address and
>> checks if output device is the same where the arp request was received,
>> if it is not, the arp request is not answered.
>>
>> This route lookup is always done on main route table so l3slave devices
>> never find the proper route and arp is not answered.
>>
>> Passing l3mdev_master_ifindex_rcu(dev) return value as oif fixes the
>> lookup for l3slave devices while maintaining same behavior for non
>> l3slave devices as this function returns 0 in that case.
>>
>> Signed-off-by: Miguel Fadon Perlines <mfadon@teldat.com>
>> ---
>> net/ipv4/arp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>
> Acked-by: David Ahern <dsa@cumulusnetworks.com>
>
> DaveM: this is a day 1 bug for VRF. Best guess at a Fixes tag would be:
>
> Fixes: 613d09b30f8b ("net: Use VRF device index for lookups on TX")
>
> It would be good to get this into stable releases 4.9 and up. Thanks,
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net: mvpp2: Fix parser entry init boundary check
From: David Miller @ 2018-04-06 2:14 UTC (permalink / raw)
To: maxime.chevallier
Cc: netdev, linux-kernel, antoine.tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180405095548.14433-1-maxime.chevallier@bootlin.com>
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Thu, 5 Apr 2018 11:55:48 +0200
> Boundary check in mvpp2_prs_init_from_hw must be done according to the
> passed "tid" parameter, not the mvpp2_prs_entry index, which is not yet
> initialized at the time of the check.
>
> Fixes: 47e0e14eb1a6 ("net: mvpp2: Make mvpp2_prs_hw_read a parser entry init function")
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation
From: Al Viro @ 2018-04-06 2:15 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
linux-kernel
In-Reply-To: <20180330150809.28094-9-hch@lst.de>
On Fri, Mar 30, 2018 at 05:07:45PM +0200, Christoph Hellwig wrote:
> The current kiocb_set_cancel_fn implementation assumes the kiocb is
> embedded into an aio_kiocb, which is fundamentally unsafe as it might
> have been submitted by non-aio callers. Instead add a cancel_kiocb
> file operation that replaced the ki_cancel function pointer set by
> kiocb_set_cancel_fn, and only adds iocbs to the active list when
> the read/write_iter methods return -EIOCBQUEUED and the file has
> a cancel_kiocb method.
> @@ -1440,6 +1423,16 @@ static inline ssize_t aio_rw_ret(struct kiocb *req, ssize_t ret)
> {
> switch (ret) {
> case -EIOCBQUEUED:
> + if (req->ki_filp->f_op->cancel_kiocb) {
... and by that point req might've been already freed by IO completion on
another CPU.
> + struct aio_kiocb *iocb =
> + container_of(req, struct aio_kiocb, rw);
> + struct kioctx *ctx = iocb->ki_ctx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ctx->ctx_lock, flags);
> + list_add_tail(&iocb->ki_list, &ctx->active_reqs);
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH net-next] hv_netvsc: Add NetVSP v6 into version negotiation
From: David Miller @ 2018-04-06 2:17 UTC (permalink / raw)
To: haiyangz, haiyangz
Cc: netdev, kys, sthemmin, olaf, vkuznets, devel, linux-kernel
In-Reply-To: <20180405184222.3167-1-haiyangz@linuxonhyperv.com>
From: Haiyang Zhang <haiyangz@linuxonhyperv.com>
Date: Thu, 5 Apr 2018 11:42:22 -0700
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> This patch adds the NetVSP v6 message structures, and includes this
> version into NetVSC/NetVSP version negotiation process.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
The net-next tree is closed, please resubmit this when the net-next
tree reopens.
Thank you.
^ permalink raw reply
* Re: [PATCH 0/4] hv_netvsc: Fix shutdown issues on older Windows hosts
From: David Miller @ 2018-04-06 2:21 UTC (permalink / raw)
To: mgamal
Cc: netdev, sthemmin, devel, linux-kernel, kys, haiyangz, vkuznets,
otubo
In-Reply-To: <1522955361-14704-1-git-send-email-mgamal@redhat.com>
From: Mohammed Gamal <mgamal@redhat.com>
Date: Thu, 5 Apr 2018 21:09:17 +0200
> Guests running on WS2012 hosts would not shutdown when changing network
> interface setting (e.g. Number of channels, MTU ... etc).
>
> This patch series addresses these shutdown issues we enecountered with WS2012
> hosts. It's essentialy a rework of the series sent in
> https://lkml.org/lkml/2018/1/23/111 on top of latest upstream
>
> Fixes: 0ef58b0a05c1 ("hv_netvsc: change GPAD teardown order on older versions")
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: dp83640: Read strapped configuration settings
From: David Miller @ 2018-04-06 2:24 UTC (permalink / raw)
To: andrew
Cc: esben, f.fainelli, richardcochran, netdev, linux-kernel,
rasmus.villemoes
In-Reply-To: <20180405204049.GD17495@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu, 5 Apr 2018 22:40:49 +0200
> Or could it still contain whatever state the last boot of Linux, or
> maybe the bootloader, left the PHY in?
Right, this is my concern as well.
^ permalink raw reply
* Re: [PATCH net] net/ipv6: Increment OUTxxx counters after netfilter hook
From: David Miller @ 2018-04-06 2:24 UTC (permalink / raw)
To: 0xeffeff; +Cc: netdev, dsahern
In-Reply-To: <20180405212947.17858-1-0xeffeff@gmail.com>
From: Jeff Barnhill <0xeffeff@gmail.com>
Date: Thu, 5 Apr 2018 21:29:47 +0000
> At the end of ip6_forward(), IPSTATS_MIB_OUTFORWDATAGRAMS and
> IPSTATS_MIB_OUTOCTETS are incremented immediately before the NF_HOOK call
> for NFPROTO_IPV6 / NF_INET_FORWARD. As a result, these counters get
> incremented regardless of whether or not the netfilter hook allows the
> packet to continue being processed. This change increments the counters
> in ip6_forward_finish() so that it will not happen if the netfilter hook
> chooses to terminate the packet, which is similar to how IPv4 works.
>
> Signed-off-by: Jeff Barnhill <0xeffeff@gmail.com>
Yep, this is more consistent with ipv4.
Applied, thank you.
^ permalink raw reply
* Re: [PATCH v15 ] net/veth/XDP: Line-rate packet forwarding in kernel
From: David Ahern @ 2018-04-06 2:55 UTC (permalink / raw)
To: Md. Islam
Cc: netdev, David Miller, Stephen Hemminger, Anton Gary Ceph,
Pavel Emelyanov, Eric Dumazet, alexei.starovoitov, brouer
In-Reply-To: <CAFgPn1D2SE_wvCvLJU9uRJbwa4hy4NVj2as_C=VzmDLqyBeGbQ@mail.gmail.com>
On 4/3/18 9:15 PM, Md. Islam wrote:
>> Have you looked at what I would consider a more interesting use case of
>> packets into a node and delivered to a namespace via veth?
>>
>> +--------------------------+---------------
>> | Host | container
>> | |
>> | +-------{ veth1 }-|-{veth2}----
>> | | |
>> +----{ eth1 }------------------
>>
>> Can xdp / bpf on eth1 be used to speed up delivery to the container?
>
> I didn't consider that, but it sounds like an important use case. How
> do we determine which namespace gets the packet?
>
FIB lookups of course. Starting with my patch set that handles
forwarding on eth1, what is needed for XDP with veth? ie., a program on
eth1 does the lookup and redirects the packet to veth1 for Tx.
ndo_xdp_xmit for veth knows the packet needs to be forwarded to veth2
internally and there is no skb allocated for the packet yet.
^ permalink raw reply
* Re: [PATCH 5/6] rhashtable: support guaranteed successful insertion.
From: NeilBrown @ 2018-04-06 3:11 UTC (permalink / raw)
To: Herbert Xu
Cc: Thomas Graf, netdev, linux-kernel, David S. Miller, Eric Dumazet
In-Reply-To: <20180329052231.GA21028@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 5902 bytes --]
On Thu, Mar 29 2018, Herbert Xu wrote:
> On Thu, Mar 29, 2018 at 08:26:21AM +1100, NeilBrown wrote:
>>
>> I say "astronomically unlikely", you say "probability .. is extremely
>> low". I think we are in agreement here.
>>
>> The point remains that if an error *can* be returned then I have to
>> write code to handle it and test that code. I'd rather not.
>
> You have be able to handle errors anyway because of memory allocation
> failures. Ultimately if you keep inserting you will eventually
> fail with ENOMEM. So I don't see the issue with an additional
> error value.
You don't need to handle memory allocation failures at the point where
you insert into the table - adding to a linked list requires no new
memory.
If you are running out of memory, you will fail to allocate new objects,
so you won't be able to add them to the rhashtable. Obviously you have
to handle that failure, and that is likely to save rhashtable from
getting many mem-alloc failures.
But once you have allocated a new object, rhashtable should just accept
it. It doesn't help anyone to have the insertion fail.
The insertion can currently fail even when allocating a new object would
succeed, as assertion can require a GFP_ATOMIC allocation to succeed,
while allocating a new object might only need a GFP_KERNEL allocation to
succeed.
>
>> > Even if it does happen we won't fail because we will perform
>> > an immediate rehash. We only fail if it happens right away
>> > after the rehash (that is, at least another 16 elements have
>> > been inserted and you're trying to insert a 17th element, all
>> > while the new hash table has not been completely populated),
>> > which means that somebody has figured out our hash secret and
>> > failing in that case makes sense.
>
> BTW, you didn't acknowledge this bit which I think is crucial to
> how likely such an error is.
The likelihood of the error isn't really the issue - it either can
happen or it cannot. If it can, it requires code to handle it.
>
>> I never suggested retrying, but I would have to handle it somehow. I'd
>> rather not.
>
> ...
>
>> While I have no doubt that there are hashtables where someone could try
>> to attack the hash, I am quite sure there are others where is such an
>> attack is meaningless - any code which could generate the required range of
>> keys, could do far worse things more easily.
>
> Our network hashtable has to be secure against adversaries. I
> understand that this may not be important to your use-case. However,
> given the fact that the failure would only occur if an adversary
> is present and actively attacking your hash table, I don't think
> it has that much of a negative effect on your use-case either.
I certainly accept that there are circumstances where it is a real
possibility that an adversary might succeed in attacking the hash
function, and changing the seed for each table is an excellent idea.
It isn't clear to me that failing insertions is needed - it is done
rather late and so doesn't throttle much, and could give the attacker
feedback that they had succeeded (?).
Not all hash tables are susceptible to attack. It would be nice if
developers working in less exposed areas could use rhashtable without ever
having to check for errors.
Error codes are messages from the implementation to the caller.
They should be chosen to help the caller make useful choices, not to
allow the implementation to avoid awkward situations.
>
> Of course if you can reproduce the EBUSY error without your
> disable_count patch or even after you have fixed the issue I
> have pointed out in your disable_count patch you can still reproduce
> it then that would suggest a real bug and we would need to fix it,
> for everyone.
I suspect that I cannot. However experience tells me that customers do
things that I cannot and would never expect - they can often trigger
errors that I cannot. It is best if the error cannot be returned.
>
>> Yes, storing a sharded count in the spinlock table does seem like an
>> appropriate granularity. However that leads me to ask: why do we have
>> the spinlock table? Why not bit spinlocks in the hashchain head like
>> include/linux/list_bl uses?
>
> The spinlock table predates rhashtable. Perhaps Thomas/Eric/Dave
> can elucidate this.
Maybe I'll submit an RFC patch to change it - if I can make it work.
>
>> I don't understand how it can ever be "too late", though I appreciate
>> that in some cases "sooner" is better than "later"
>> If we give up on the single atomic_t counter, then we must accept that
>> the number of elements could exceed any given value. The only promise
>> we can provide is that it wont exceed N% of the table size for more than
>> T seconds.
>
> Sure. However, assuming we use an estimate that is hash-based,
> that *should* be fairly accurate assuming that your hash function
> is working in the first place. It's completely different vs.
> estimating based on a per-cpu count which could be wildly inaccurate.
Ahhh... I see what you are thinking now. You aren't suggestion a
sharded count that is summed periodically. Rather you are suggesting that
we divide the hash space into N regions each with their own independent
counter, and resize the table if any one region reaches 70% occupancy -
on the assumption that the other regions are likely to be close. Is
that right?
It would probably be dangerous to allow automatic shrinking (when just
one drops below 30%) in that case, but it might be OK for growing.
I'm not sure I like the idea, but it is worth thinking about.
Thanks,
NeilBrown
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: KASAN: use-after-free Read in worker_thread (2)
From: Eric Biggers @ 2018-04-06 3:12 UTC (permalink / raw)
To: syzbot
Cc: davem, dvyukov, ebiggers, jiangshanlai, linux-kernel, mingo,
netdev, syzkaller-bugs, tj, tklauser, tom, xiyou.wangcong
In-Reply-To: <001a1140ccfee824f8055db71210@google.com>
On Sat, Nov 11, 2017 at 07:56:01AM -0800, syzbot wrote:
> syzkaller has found reproducer for the following crash on
> d9e0e63d9a6f88440eb201e1491fcf730272c706
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
>
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
>
>
> BUG: KASAN: use-after-free in worker_thread+0x15bb/0x1990
> kernel/workqueue.c:2244
> Read of size 8 at addr ffff88002d0e3de0 by task kworker/u8:1/1209
>
> CPU: 0 PID: 1209 Comm: kworker/u8:1 Not tainted 4.14.0-rc8-next-20171110+
> #12
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:17 [inline]
> dump_stack+0x194/0x257 lib/dump_stack.c:53
> print_address_description+0x73/0x250 mm/kasan/report.c:252
> kasan_report_error mm/kasan/report.c:351 [inline]
> kasan_report+0x25b/0x340 mm/kasan/report.c:409
> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430
> worker_thread+0x15bb/0x1990 kernel/workqueue.c:2244
> kthread+0x37a/0x440 kernel/kthread.c:238
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:437
>
> Allocated by task 11866:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459 [inline]
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:489
> kmem_cache_alloc+0x12e/0x760 mm/slab.c:3548
> kmem_cache_zalloc include/linux/slab.h:693 [inline]
> kcm_attach net/kcm/kcmsock.c:1394 [inline]
> kcm_attach_ioctl net/kcm/kcmsock.c:1460 [inline]
> kcm_ioctl+0x2d1/0x1610 net/kcm/kcmsock.c:1695
> sock_do_ioctl+0x65/0xb0 net/socket.c:960
> sock_ioctl+0x2c2/0x440 net/socket.c:1057
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> entry_SYSCALL_64_fastpath+0x1f/0x96
>
> Freed by task 11867:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459 [inline]
> kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
> __cache_free mm/slab.c:3492 [inline]
> kmem_cache_free+0x77/0x280 mm/slab.c:3750
> kcm_unattach+0xe50/0x1510 net/kcm/kcmsock.c:1563
> kcm_unattach_ioctl net/kcm/kcmsock.c:1608 [inline]
> kcm_ioctl+0xdf0/0x1610 net/kcm/kcmsock.c:1705
> sock_do_ioctl+0x65/0xb0 net/socket.c:960
> sock_ioctl+0x2c2/0x440 net/socket.c:1057
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1530 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> entry_SYSCALL_64_fastpath+0x1f/0x96
>
> The buggy address belongs to the object at ffff88002d0e3d00
> which belongs to the cache kcm_psock_cache of size 576
> The buggy address is located 224 bytes inside of
> 576-byte region [ffff88002d0e3d00, ffff88002d0e3f40)
> The buggy address belongs to the page:
> page:ffffea0000b43880 count:1 mapcount:0 mapping:ffff88002d0e2180 index:0x0
> compound_mapcount: 0
> flags: 0x100000000008100(slab|head)
> raw: 0100000000008100 ffff88002d0e2180 0000000000000000 000000010000000b
> raw: ffffea0000b14920 ffffea0000b27e20 ffff88002b0089c0 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff88002d0e3c80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff88002d0e3d00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > ffff88002d0e3d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff88002d0e3e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff88002d0e3e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
No longer occurring, the fix seems to have been commit 7e9964574ee97:
#syz fix: kcm: Only allow TCP sockets to be attached to a KCM mux
- Eric
^ 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