* Re: [PATCH v2 14/29] net: simplify eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-08-10 16:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Corbet, Sekhar Nori, Kevin Hilman, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger,
Grygorii Strashko, David S . Miller, Srinivas Kandagatla, Naren,
Mauro Carvalho Chehab, Andrew Morton, Lukas Wunner, Dan Carpenter
In-Reply-To: <CAHp75Ve28aNe+FCK3p4OLsDf7dNo-iJ1VR=pyx+Y9OUn3sjYBw@mail.gmail.com>
2018-08-10 16:39 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Fri, Aug 10, 2018 at 11:05 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>
>> We don't need to use pci_device_to_OF_node() - we can retrieve
>> dev->of_node directly even for pci devices.
>>
>
>> struct device_node *dp;
>>
>> - if (dev_is_pci(dev))
>> - dp = pci_device_to_OF_node(to_pci_dev(dev));
>> - else
>> - dp = dev->of_node;
>> -
>> + dp = dev->of_node;
>> addr = NULL;
>> if (dp)
>> addr = of_get_mac_address(dp);
>
> Looking more at this I could even propose to change all above by
>
> addr = device_get_mac_address(dev, mac_addr, ETH_ALEN);
>
> Thoughts?
>
> --
> With Best Regards,
> Andy Shevchenko
Indeed seems like it's even more generalized. Thanks for spotting that.
Bart
^ permalink raw reply
* Motorcycle Owners List
From: Brenda Calkins @ 2018-08-10 14:51 UTC (permalink / raw)
To: netdev
--
Hi,
Greetings of the day!
Would you be interested in acquiring an email list of "Motorcycle Owners List" from USA?
We also have data for Luxury Car Owners List, Luxury Brand Buyers List, Mercedes-Benz Owners, BMW Owners, Audi Owners, Hyundai Owners, Porsche Owners, Toyota Owners, Harley Davidson Owners List, Car Owners, HNI list, RV Owners and many more...
All the contacts are opt-in verified, 100% permission based and can be used for unlimited multi-channel marketing.
Please let me know your thoughts towards procuring the Motorcycle Owners List.
Best Regards,
Brenda Calkins
Research Analyst
We respect your privacy, if you do not wish to receive any further emails from our end, please reply with a subject “Leave Out”.
^ permalink raw reply
* Re: [PATCH net] l2tp: use sk_dst_check() to avoid race on sk->sk_dst_cache
From: Guillaume Nault @ 2018-08-10 15:59 UTC (permalink / raw)
To: Wei Wang; +Cc: David Miller, netdev, Martin KaFai Lau, David Ahern, Cong Wang
In-Reply-To: <20180809185405.38744-1-tracywwnj@gmail.com>
On Thu, Aug 09, 2018 at 11:54:05AM -0700, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
>
> In l2tp code, if it is a L2TP_UDP_ENCAP tunnel, tunnel->sk points to a
> UDP socket. User could call sendmsg() on both this tunnel and the UDP
> socket itself concurrently. As l2tp_xmit_skb() holds socket lock and call
> __sk_dst_check() to refresh sk->sk_dst_cache, while udpv6_sendmsg() is
> lockless and call sk_dst_check() to refresh sk->sk_dst_cache, there
> could be a race and cause the dst cache to be freed multiple times.
> So we fix l2tp side code to always call sk_dst_check() to garantee
> xchg() is called when refreshing sk->sk_dst_cache to avoid race
> conditions.
>
> Syzkaller reported stack trace:
> BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
> BUG: KASAN: use-after-free in atomic_fetch_add_unless include/linux/atomic.h:575 [inline]
> BUG: KASAN: use-after-free in atomic_add_unless include/linux/atomic.h:597 [inline]
> BUG: KASAN: use-after-free in dst_hold_safe include/net/dst.h:308 [inline]
> BUG: KASAN: use-after-free in ip6_hold_safe+0xe6/0x670 net/ipv6/route.c:1029
> Read of size 4 at addr ffff8801aea9a880 by task syz-executor129/4829
>
> CPU: 0 PID: 4829 Comm: syz-executor129 Not tainted 4.18.0-rc7-next-20180802+ #30
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
> print_address_description+0x6c/0x20b mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
> check_memory_region_inline mm/kasan/kasan.c:260 [inline]
> check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
> kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272
> atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
> atomic_fetch_add_unless include/linux/atomic.h:575 [inline]
> atomic_add_unless include/linux/atomic.h:597 [inline]
> dst_hold_safe include/net/dst.h:308 [inline]
> ip6_hold_safe+0xe6/0x670 net/ipv6/route.c:1029
> rt6_get_pcpu_route net/ipv6/route.c:1249 [inline]
> ip6_pol_route+0x354/0xd20 net/ipv6/route.c:1922
> ip6_pol_route_output+0x54/0x70 net/ipv6/route.c:2098
> fib6_rule_lookup+0x283/0x890 net/ipv6/fib6_rules.c:122
> ip6_route_output_flags+0x2c5/0x350 net/ipv6/route.c:2126
> ip6_dst_lookup_tail+0x1278/0x1da0 net/ipv6/ip6_output.c:978
> ip6_dst_lookup_flow+0xc8/0x270 net/ipv6/ip6_output.c:1079
> ip6_sk_dst_lookup_flow+0x5ed/0xc50 net/ipv6/ip6_output.c:1117
> udpv6_sendmsg+0x2163/0x36b0 net/ipv6/udp.c:1354
> inet_sendmsg+0x1a1/0x690 net/ipv4/af_inet.c:798
> sock_sendmsg_nosec net/socket.c:622 [inline]
> sock_sendmsg+0xd5/0x120 net/socket.c:632
> ___sys_sendmsg+0x51d/0x930 net/socket.c:2115
> __sys_sendmmsg+0x240/0x6f0 net/socket.c:2210
> __do_sys_sendmmsg net/socket.c:2239 [inline]
> __se_sys_sendmmsg net/socket.c:2236 [inline]
> __x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2236
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x446a29
> Code: e8 ac b8 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f4de5532db8 EFLAGS: 00000246 ORIG_RAX: 0000000000000133
> RAX: ffffffffffffffda RBX: 00000000006dcc38 RCX: 0000000000446a29
> RDX: 00000000000000b8 RSI: 0000000020001b00 RDI: 0000000000000003
> RBP: 00000000006dcc30 R08: 00007f4de5533700 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dcc3c
> R13: 00007ffe2b830fdf R14: 00007f4de55339c0 R15: 0000000000000001
>
> Fixes: 71b1391a4128 ("l2tp: ensure sk->dst is still valid")
> Reported-by: syzbot+05f840f3b04f211bad55@syzkaller.appspotmail.com
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/l2tp/l2tp_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index 40261cb68e83..7166b61338d4 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1110,7 +1110,7 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
>
> /* Get routing info from the tunnel socket */
> skb_dst_drop(skb);
> - skb_dst_set(skb, dst_clone(__sk_dst_check(sk, 0)));
> + skb_dst_set(skb, dst_clone(sk_dst_check(sk, 0)));
>
With sk_dst_check(), we're now holding an extra reference on the cached
dst. How is it dropped? Shouldn't we remove dst_clone()?
Also, does syzbot have a reproducer? I haven't found the original
report on the mailing list.
^ permalink raw reply
* [PATCH v2] Add icmp_echo_ignore_all support for ICMPv6
From: Virgile Jarry @ 2018-08-10 15:48 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
linux-kernel
Preventing the kernel from responding to ICMP Echo Requests messages
can be useful in several ways. The sysctl parameter
'icmp_echo_ignore_all' can be used to prevent the kernel from
responding to IPv4 ICMP echo requests. For IPv6 pings, such
a sysctl kernel parameter did not exist.
Add the ability to prevent the kernel from responding to IPv6
ICMP echo requests through the use of the following sysctl
parameter : /proc/sys/net/ipv6/icmp/echo_ignore_all.
Update the documentation to reflect this change.
Signed-off-by: Virgile Jarry <virgile@acceis.fr>
---
Changes in v2:
- Added support for sysctl interface (and not just /proc filesystem)
Documentation/networking/ip-sysctl.txt | 5 +++++
include/net/netns/ipv6.h | 1 +
include/uapi/linux/sysctl.h | 3 ++-
net/ipv6/af_inet6.c | 1 +
net/ipv6/icmp.c | 16 +++++++++++++---
5 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ce8fbf5..5042a13 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1863,6 +1863,11 @@ ratelimit - INTEGER
otherwise the minimal space between responses in milliseconds.
Default: 1000
+echo_ignore_all - BOOLEAN
+ If set non-zero, then the kernel will ignore all ICMP ECHO
+ requests sent to it over the IPv6 protocol.
+ Default: 0
+
xfrm6_gc_thresh - INTEGER
The threshold at which we will start garbage collecting for IPv6
destination cache entries. At twice this value the system will
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 762ac99..f0e396a 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -32,6 +32,7 @@ struct netns_sysctl_ipv6 {
int flowlabel_consistency;
int auto_flowlabels;
int icmpv6_time;
+ int icmpv6_echo_ignore_all;
int anycast_src_echo_reply;
int ip_nonlocal_bind;
int fwmark_reflect;
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 6b58371..d71013f 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -575,7 +575,8 @@ enum {
/* /proc/sys/net/ipv6/icmp */
enum {
- NET_IPV6_ICMP_RATELIMIT=1
+ NET_IPV6_ICMP_RATELIMIT = 1,
+ NET_IPV6_ICMP_ECHO_IGNORE_ALL = 2
};
/* /proc/sys/net/<protocol>/neigh/<dev> */
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 9ed0eae..d0033b3 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -833,6 +833,7 @@ static int __net_init inet6_net_init(struct net *net)
net->ipv6.sysctl.bindv6only = 0;
net->ipv6.sysctl.icmpv6_time = 1*HZ;
+ net->ipv6.sysctl.icmpv6_echo_ignore_all = 0;
net->ipv6.sysctl.flowlabel_consistency = 1;
net->ipv6.sysctl.auto_flowlabels = IP6_DEFAULT_AUTO_FLOW_LABELS;
net->ipv6.sysctl.idgen_retries = 3;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index ef2505a..1d869f1 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -800,6 +800,7 @@ void icmpv6_notify(struct sk_buff *skb, u8 type, u8 code, __be32 info)
static int icmpv6_rcv(struct sk_buff *skb)
{
+ struct net *net = dev_net(skb->dev);
struct net_device *dev = skb->dev;
struct inet6_dev *idev = __in6_dev_get(dev);
const struct in6_addr *saddr, *daddr;
@@ -849,7 +850,8 @@ static int icmpv6_rcv(struct sk_buff *skb)
switch (type) {
case ICMPV6_ECHO_REQUEST:
- icmpv6_echo_reply(skb);
+ if (!net->ipv6.sysctl.icmpv6_echo_ignore_all)
+ icmpv6_echo_reply(skb);
break;
case ICMPV6_ECHO_REPLY:
@@ -1110,6 +1112,13 @@ static struct ctl_table ipv6_icmp_table_template[] = {
.mode = 0644,
.proc_handler = proc_dointvec_ms_jiffies,
},
+ {
+ .procname = "echo_ignore_all",
+ .data = &init_net.ipv6.sysctl.icmpv6_echo_ignore_all,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
{ },
};
@@ -1121,9 +1130,10 @@ struct ctl_table * __net_init ipv6_icmp_sysctl_init(struct net *net)
sizeof(ipv6_icmp_table_template),
GFP_KERNEL);
- if (table)
+ if (table) {
table[0].data = &net->ipv6.sysctl.icmpv6_time;
-
+ table[1].data = &net->ipv6.sysctl.icmpv6_echo_ignore_all;
+ }
return table;
}
#endif
--
2.7.4
^ permalink raw reply related
* Re: Error running AF_XDP sample application
From: Björn Töpel @ 2018-08-10 15:45 UTC (permalink / raw)
To: kdjimeli; +Cc: Jakub Kicinski, Netdev, netdev-owner
In-Reply-To: <9a31684912d059772cfbb8e2062c38af@igalia.com>
Den fre 10 aug. 2018 kl 15:23 skrev Konrad Djimeli <kdjimeli@igalia.com>:
>
> On 2018-08-10 11:58, Konrad Djimeli wrote:
> > On 2018-08-10 03:51, Jakub Kicinski wrote:
> >> On Thu, 09 Aug 2018 18:18:08 +0200, kdjimeli wrote:
> >>> Hello,
> >>>
> >>> I have been trying to test a sample AF_XDP program, but I have been
> >>> experiencing some issues.
> >>> After building the sample code
> >>> https://github.com/torvalds/linux/tree/master/samples/bpf,
> >>> when running the xdpsock binary, I get the errors
> >>> "libbpf: failed to create map (name: 'xsks_map'): Invalid argument"
> >>> "libbpf: failed to load object './xdpsock_kern.o"
> >>>
> >>> I tried to figure out the cause of the error but all I know is that it
> >>> occurs at line 910 with the function
> >>> call "bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd)".
> >>>
> >>> Please I would like to inquire what could be a possible for this error.
> >>
> >> which kernel version are you running?
> >
> > My kernel version is 4.18.0-rc8+. I cloned it from
> > https://github.com/torvalds/linux before building a running.
> >
> > My commit head(git show-ref --head) is at
> > 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc HEAD
> > 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc refs/heads/master
> > 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc refs/remotes/origin/HEAD
> > 1236568ee3cbb0d3ac62d0074a29b97ecf34cbbc refs/remotes/origin/master
> > ...
> >
> >
> > I also applied the patch https://patchwork.ozlabs.org/patch/949884/
> > (samples: bpf: convert xdpsock_user.c to libbpf ), as the error was
> > initially in the form show below:
> > "failed to create a map: 22 Invalid argument"
> > "ERROR: load_bpf_file"
> >
> > Thanks
> > Konrad
>
> Also other sample applications that make use of other bpf maps, such as
> BPF_MAP_TYPE_CPUMAP in xdp_redirect_cpu work fine. But the application
> with BPF_MAP_TYPE_XSKMAP fails producing the error mentioned above.
>
> Thanks
> Konrad
Thanks for taking AF_XDP for a spin!
Before I start digging into details; Do you have CONFIG_XDP_SOCKETS=y
in your config? :-)
Björn
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: core: remove unnecessary function mlxsw_core_driver_put
From: Ido Schimmel @ 2018-08-10 15:26 UTC (permalink / raw)
To: YueHaibing; +Cc: davem, jiri, linux-kernel, netdev
In-Reply-To: <20180810023730.13140-1-yuehaibing@huawei.com>
On Fri, Aug 10, 2018 at 10:37:30AM +0800, YueHaibing wrote:
> The function mlxsw_core_driver_put only traverse mlxsw_core_driver_list
> to find the matched mlxsw_driver,but never used it.
> So it can be removed safely.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
FWIW, this function used to call module_put(), but it was removed in
commit 1d20d23c59c9 ("mlxsw: Move PCI id table definitions into driver
modules").
Thanks!
^ permalink raw reply
* Re: [PATCH 0/2] net: ethernet: ti: cpsw: fix runtime pm while add/del reserved vid
From: Grygorii Strashko @ 2018-08-10 17:53 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem; +Cc: linux-omap, netdev, linux-kernel
In-Reply-To: <20180810124709.25089-1-ivan.khoronzhuk@linaro.org>
On 08/10/2018 07:47 AM, Ivan Khoronzhuk wrote:
> Here 2 not critical fixes for:
> - vlan ale table leak while error if deleting vlan (simplifies next fix)
> - runtime pm while try to set reserved vlan
>
> Based on net/master
>
> Ivan Khoronzhuk (2):
> net: ethernet: ti: cpsw: clear all entries when delete vid
> net: ethernet: ti: cpsw: fix runtime_pm while add/kill vlan
>
> drivers/net/ethernet/ti/cpsw.c | 25 +++++++++++--------------
> drivers/net/ethernet/ti/cpsw_ale.c | 2 +-
> 2 files changed, 12 insertions(+), 15 deletions(-)
>
Thank you.
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
--
regards,
-grygorii
^ permalink raw reply
* Re: [PATCH bpf-next 0/4] Convert filter.txt to RST
From: Alexei Starovoitov @ 2018-08-10 17:51 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Tobin C. Harding, Daniel Borkmann, Alexei Starovoitov,
David S. Miller, Kees Cook, Andy Lutomirski, Will Drewry,
linux-doc, Network Development, LKML
In-Reply-To: <20180810065752.2ab5473d@lwn.net>
On Fri, Aug 10, 2018 at 5:57 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> The objective actually is to have SPDX tags in all files in the kernel.
> That includes documentation, even though people, as always, care less
> about the docs than they do the code.
right, but let's do that as a separate patch set.
In the current set I'd focus on reviewing the actual doc changes.
In particular completely removing
Documentation/networking/filter.txt
feels wrong, since lots of websites point directly there.
Can we have at least few words there pointing to new location?
^ permalink raw reply
* Re: C45 support and mdiobus_scan
From: Tom Lendacky @ 2018-08-10 15:20 UTC (permalink / raw)
To: Andrew Lunn, Jose Abreu; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180809152502.GC20006@lunn.ch>
On 8/9/2018 10:25 AM, Andrew Lunn wrote:
>>> The PCIe core will look in the device tree and when it creates the
>>> platform device for the i210 on the pcie bus, it points
>>> pdev->dev.of_node at this node. So long as you are using a platform
>>> with DT, you can do this. I hope you are not using x86..
>>
>> Yes I am :( Any possible solution for this?
I haven't looked too closely, but maybe you can add a new mdiobus_scan
function for 10G that attempts get_phy_device() with is_c45 set to true
and if nothing is found falls back to get_phy_device() with is_c45 set to
false. I don't know what would happen if you have a non-c45 phy attached,
but it's worth a shot to try it and see for each situation.
Thanks,
Tom
>
> Well, DT can be used with x86. I think Edison did that. But i assume
> your PCIe host is in ACPI, not DT. So getting this linking working
> will not be easy.
>
> There has been some work to add an ACPI binding for PHYs. I don't know
> if it actually got far enough that you can hack your DSDT to add a
> PHY. But i'm sure it did not get far enough that you can describe an
> MDIO bus in DSDT, so it probably is not going to help you.
>
>> I guess in ultimate case I will have to switch to ARM based setup.
>
> Yes, or MIPS.
>
> Andrew
>
^ permalink raw reply
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
From: Björn Töpel @ 2018-08-10 15:16 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Netdev, Taehee Yoo, Jesper Dangaard Brouer,
Björn Töpel, Karlsson, Magnus, Magnus Karlsson, kafai
In-Reply-To: <5d07c5ce-2dea-7ab2-f5cf-7ed22d5da783@iogearbox.net>
Den fre 10 aug. 2018 kl 16:10 skrev Daniel Borkmann <daniel@iogearbox.net>:
>
> On 08/10/2018 11:28 AM, Björn Töpel wrote:
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> >
> > The reverted commit adds a WARN to check against NULL entries in the
> > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > driver) fast path is required to make a paired
> > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > addition, a driver using a different allocation scheme than the
> > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > xdp_rxq_info_reg_mem_model.
> >
> > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > preferred to the WARN.
> >
> > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>
> Given the last bpf pr went out yesterday night, I've applied this to
> bpf-next (worst case we can just route it via stable), thanks!
Ah, right! Thanks!
bpf-next is OK. (Since this path is currently not used yet by any driver... :-()
Björn
^ permalink raw reply
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
From: Björn Töpel @ 2018-08-10 15:15 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexei Starovoitov, Daniel Borkmann, Netdev, Taehee Yoo,
Björn Töpel, Karlsson, Magnus, Magnus Karlsson, kafai,
Jakub Kicinski
In-Reply-To: <20180810121804.22aec831@redhat.com>
Den fre 10 aug. 2018 kl 12:18 skrev Jesper Dangaard Brouer <brouer@redhat.com>:
>
> On Fri, 10 Aug 2018 11:28:02 +0200
> Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> >
> > The reverted commit adds a WARN to check against NULL entries in the
> > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > driver) fast path is required to make a paired
> > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > addition, a driver using a different allocation scheme than the
> > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > xdp_rxq_info_reg_mem_model.
> >
> > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > preferred to the WARN.
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> As a comment says in the code: /* NB! Only valid from an xdp_buff! */
> Which is (currently) guarded by the return/exit in convert_to_xdp_frame().
>
> This means that this code path can only be invoked while the driver is
> still running under the RX NAPI process. Thus, there is no chance that
> the allocator-id is gone (via calling xdp_rxq_info_unreg) for this code
> path.
>
> But I really hope we at somepoint can convert a MEM_TYPE_ZERO_COPY into
> a form of xdp_frame, that can travel further into the redirect-core.
> In which case, we likely need to handle the NULL case (but also need
> other code to handle what to do with the memory backing the frame)
>
> (I'm my vision here:)
>
> I really dislike that the current Zero-Copy mode steal ALL packets,
> when ZC is enabled on a RX-queue. This is not better than the existing
> bypass solutions, which have ugly ways of re-injecting packet back into
> the network stack. With the integration with XDP, we have the
> flexibility of selecting frames, that we don't want to be "bypassed"
> into AF_XDP, and want the kernel process these. (The most common
> use-case is letting the kernel handle the arptable). IHMO this is what
> will/would make AF_XDP superior to other bypass solutions.
>
>
Thanks for putting your visions/ideas here! I agree with both of your
last sections, and this is what we're working towards. AF_XDP ZC has
to play nicer with XDP. The current (well, the soon-to-be-published
[1] ;-)) ZC scheme is just a first step, and should be seen as a
starting point so people can start playing using AF_XDP. Jakub also
mentioned these issues a couple of threads ago, so there are
definitely more people feeling the ZC allocator pains. Mid-term a
sophisticated/proper and generic (for inter-driver reuse) ZC allocator
is needed; Converting xdp_buffs to xdp_frames cheaply for multi-CPU
completion, and hopefully dito for the XDP_PASS/kernel stack path. But
let's start with something simple that works, and take it from there.
Björn
[1] WIP: https://github.com/bjoto/linux/tree/af-xdp-i40e-zc
> > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> > ---
> > net/core/xdp.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 6771f1855b96..9d1f22072d5d 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -345,8 +345,7 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > rcu_read_lock();
> > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > - if (!WARN_ON_ONCE(!xa))
> > - xa->zc_alloc->free(xa->zc_alloc, handle);
> > + xa->zc_alloc->free(xa->zc_alloc, handle);
> > rcu_read_unlock();
> > default:
> > /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] net: chelsio: cxgb2: remove unused array pci_speed
From: David Miller @ 2018-08-10 17:25 UTC (permalink / raw)
To: colin.king; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180810080217.26758-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 10 Aug 2018 09:02:17 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Array pci_speed is defined but is never used hence it is redundant
> and can be removed.
>
> Cleans up clang warning:
> warning: 'pci_speed' defined but not used [-Wunused-const-variable=]
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] mlxsw: remove unused arrays mlxsw_i2c_driver_name and mlxsw_pci_driver_name
From: David Miller @ 2018-08-10 17:25 UTC (permalink / raw)
To: colin.king; +Cc: jiri, idosch, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20180810075328.26400-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Fri, 10 Aug 2018 08:53:28 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Arrays mlxsw_i2c_driver_name and mlxsw_pci_driver_name are defined
> but never used hence they are redundant and can be removed.
>
> Cleans up clang warnings:
> warning: 'mlxsw_i2c_driver_name' defined but not used
> warning: 'mlxsw_pci_driver_name' defined but not used
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: Provide stub for __netif_set_xps_queue if there is no CONFIG_XPS
From: David Miller @ 2018-08-10 17:22 UTC (permalink / raw)
To: krzk; +Cc: netdev, linux-kernel, avagin
In-Reply-To: <1533890863-32341-1-git-send-email-krzk@kernel.org>
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: Fri, 10 Aug 2018 10:47:43 +0200
> Building virtio_net driver without CONFIG_XPS fails with:
>
> drivers/net/virtio_net.c: In function ‘virtnet_set_affinity’:
> drivers/net/virtio_net.c:1910:3: error: implicit declaration of function ‘__netif_set_xps_queue’ [-Werror=implicit-function-declaration]
> __netif_set_xps_queue(vi->dev, mask, i, false);
> ^
> Fixes: 4d99f6602cb5 ("net: allow to call netif_reset_xps_queues() under cpus_read_lock")
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH iproute2/net-next v2] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
From: Eelco Chaudron @ 2018-08-10 14:48 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, davem
In-Reply-To: <20180810074454.4af9e5cd@xeon-e3>
On 10 Aug 2018, at 16:44, Stephen Hemminger wrote:
> On Fri, 10 Aug 2018 07:59:30 -0400
> Eelco Chaudron <echaudro@redhat.com> wrote:
>
>> + if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
>> + print_string(PRINT_FP, NULL, "%s", _SL_);
>> + print_string(PRINT_FP, NULL, "%s", prefix);
>> + print_lluint(PRINT_ANY, "sw_bytes",
>> + "Sent software %llu bytes",
>> + bs.bytes - bs_hw.bytes);
>> + print_uint(PRINT_ANY, "sw_packets", " %u pkt",
>> + bs.packets - bs_hw.packets);
>> + }
>> + }
>> +
>> + print_string(PRINT_FP, NULL, "%s", _SL_);
>> + print_string(PRINT_FP, NULL, "%s", prefix);
>> + print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
>> + bs_hw.bytes);
>
> What does the output look like?
See the two +’es below:
$ tc -s filter show dev enp3s0np0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
eth_type ipv4
dst_ip 2.0.0.0
src_ip 1.0.0.0
ip_flags nofrag
in_hw
action order 1: mirred (Egress Redirect to device eth1) stolen
index 1 ref 1 bind 1 installed 0 sec used 0 sec
Action statistics:
Sent 534884742 bytes 8915697 pkt (dropped 0, overlimits 0 requeues 0)
+ Sent software 187542 bytes 4077 pkt
+ Sent hardware 534697200 bytes 8911620 pkt
backlog 0b 0p requeues 0
cookie 89173e6a44447001becfd486bda17e29
^ permalink raw reply
* Re: [PATCH iproute2/net-next v2] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
From: Stephen Hemminger @ 2018-08-10 14:44 UTC (permalink / raw)
To: Eelco Chaudron; +Cc: netdev, davem
In-Reply-To: <20180810115846.15762.3693.stgit@wsfd-netdev20.ntdv.lab.eng.bos.redhat.com>
On Fri, 10 Aug 2018 07:59:30 -0400
Eelco Chaudron <echaudro@redhat.com> wrote:
> + if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
> + print_string(PRINT_FP, NULL, "%s", _SL_);
> + print_string(PRINT_FP, NULL, "%s", prefix);
> + print_lluint(PRINT_ANY, "sw_bytes",
> + "Sent software %llu bytes",
> + bs.bytes - bs_hw.bytes);
> + print_uint(PRINT_ANY, "sw_packets", " %u pkt",
> + bs.packets - bs_hw.packets);
> + }
> + }
> +
> + print_string(PRINT_FP, NULL, "%s", _SL_);
> + print_string(PRINT_FP, NULL, "%s", prefix);
> + print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
> + bs_hw.bytes);
What does the output look like?
^ permalink raw reply
* Re: [PATCH v2 14/29] net: simplify eth_platform_get_mac_address()
From: Andy Shevchenko @ 2018-08-10 14:39 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jonathan Corbet, Sekhar Nori, Kevin Hilman, Russell King,
Arnd Bergmann, Greg Kroah-Hartman, David Woodhouse, Brian Norris,
Boris Brezillon, Marek Vasut, Richard Weinberger,
Grygorii Strashko, David S . Miller, Srinivas Kandagatla, Naren,
Mauro Carvalho Chehab, Andrew Morton, Lukas Wunner, Dan Carpenter
In-Reply-To: <20180810080526.27207-15-brgl@bgdev.pl>
On Fri, Aug 10, 2018 at 11:05 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> We don't need to use pci_device_to_OF_node() - we can retrieve
> dev->of_node directly even for pci devices.
>
> struct device_node *dp;
>
> - if (dev_is_pci(dev))
> - dp = pci_device_to_OF_node(to_pci_dev(dev));
> - else
> - dp = dev->of_node;
> -
> + dp = dev->of_node;
> addr = NULL;
> if (dp)
> addr = of_get_mac_address(dp);
Looking more at this I could even propose to change all above by
addr = device_get_mac_address(dev, mac_addr, ETH_ALEN);
Thoughts?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* KMSAN: uninit-value in ip_tunnel_xmit (2)
From: syzbot @ 2018-08-10 14:30 UTC (permalink / raw)
To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 80ecacc456c1 kmsan: fix CONFIG_KMSAN=n build
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=163b7794400000
kernel config: https://syzkaller.appspot.com/x/.config?x=93d57043084eee38
dashboard link: https://syzkaller.appspot.com/bug?extid=4a2c52677a8a1aa283cb
compiler: clang version 7.0.0 (trunk 334104)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=17cec9dc400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=145dcf0c400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KMSAN: uninit-value in ip_tunnel_xmit+0x63a/0x3b10
net/ipv4/ip_tunnel.c:645
CPU: 1 PID: 4603 Comm: syz-executor712 Not tainted 4.18.0-rc4+ #27
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x185/0x1e0 lib/dump_stack.c:113
kmsan_report+0x195/0x2c0 mm/kmsan/kmsan.c:982
__msan_warning+0x7d/0xe0 mm/kmsan/kmsan_instr.c:645
ip_tunnel_xmit+0x63a/0x3b10 net/ipv4/ip_tunnel.c:645
__gre_xmit net/ipv4/ip_gre.c:449 [inline]
ipgre_xmit+0xe16/0xef0 net/ipv4/ip_gre.c:701
__netdev_start_xmit include/linux/netdevice.h:4148 [inline]
netdev_start_xmit include/linux/netdevice.h:4157 [inline]
xmit_one net/core/dev.c:3034 [inline]
dev_hard_start_xmit+0x60f/0xcc0 net/core/dev.c:3050
__dev_queue_xmit+0x3060/0x3c70 net/core/dev.c:3569
dev_queue_xmit+0x4b/0x60 net/core/dev.c:3602
packet_snd net/packet/af_packet.c:2919 [inline]
packet_sendmsg+0x8469/0x9010 net/packet/af_packet.c:2944
sock_sendmsg_nosec net/socket.c:641 [inline]
sock_sendmsg net/socket.c:651 [inline]
___sys_sendmsg+0xed9/0x1350 net/socket.c:2125
__sys_sendmsg net/socket.c:2163 [inline]
__do_sys_sendmsg net/socket.c:2172 [inline]
__se_sys_sendmsg net/socket.c:2170 [inline]
__x64_sys_sendmsg+0x3b0/0x520 net/socket.c:2170
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x4412f9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 db 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffdc77f10d8 EFLAGS: 00000213 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004412f9
RDX: 0000000000000000 RSI: 0000000020000600 RDI: 0000000000000003
RBP: 00000000006cc018 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000020 R11: 0000000000000213 R12: 0000000000402260
R13: 00000000004022f0 R14: 0000000000000000 R15: 0000000000000000
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
kmsan_internal_poison_shadow+0xc8/0x1d0 mm/kmsan/kmsan.c:181
kmsan_kmalloc+0xa1/0x120 mm/kmsan/kmsan_hooks.c:91
kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan_hooks.c:100
slab_post_alloc_hook mm/slab.h:446 [inline]
slab_alloc_node mm/slub.c:2720 [inline]
__kmalloc_node_track_caller+0xb48/0x11d0 mm/slub.c:4353
__kmalloc_reserve net/core/skbuff.c:138 [inline]
__alloc_skb+0x2cb/0x9e0 net/core/skbuff.c:206
alloc_skb include/linux/skbuff.h:988 [inline]
alloc_skb_with_frags+0x1e6/0xb80 net/core/skbuff.c:5271
sock_alloc_send_pskb+0xb59/0x11e0 net/core/sock.c:2095
packet_alloc_skb net/packet/af_packet.c:2776 [inline]
packet_snd net/packet/af_packet.c:2867 [inline]
packet_sendmsg+0x68cb/0x9010 net/packet/af_packet.c:2944
sock_sendmsg_nosec net/socket.c:641 [inline]
sock_sendmsg net/socket.c:651 [inline]
___sys_sendmsg+0xed9/0x1350 net/socket.c:2125
__sys_sendmsg net/socket.c:2163 [inline]
__do_sys_sendmsg net/socket.c:2172 [inline]
__se_sys_sendmsg net/socket.c:2170 [inline]
__x64_sys_sendmsg+0x3b0/0x520 net/socket.c:2170
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x63/0xe7
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v8 bpf-next 00/10] veth: Driver XDP
From: Daniel Borkmann @ 2018-08-10 14:24 UTC (permalink / raw)
To: Toshiaki Makita, Alexei Starovoitov
Cc: netdev, Jesper Dangaard Brouer, Jakub Kicinski, John Fastabend
In-Reply-To: <1533283098-2397-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
On 08/03/2018 09:58 AM, Toshiaki Makita wrote:
> This patch set introduces driver XDP for veth.
> Basically this is used in conjunction with redirect action of another XDP
> program.
>
> NIC -----------> veth===veth
> (XDP) (redirect) (XDP)
>
> In this case xdp_frame can be forwarded to the peer veth without
> modification, so we can expect far better performance than generic XDP.
>
>
> Envisioned use-cases
> --------------------
>
> * Container managed XDP program
> Container host redirects frames to containers by XDP redirect action, and
> privileged containers can deploy their own XDP programs.
>
> * XDP program cascading
> Two or more XDP programs can be called for each packet by redirecting
> xdp frames to veth.
>
> * Internal interface for an XDP bridge
> When using XDP redirection to create a virtual bridge, veth can be used
> to create an internal interface for the bridge.
>
>
> Implementation
> --------------
>
> This changeset is making use of NAPI to implement ndo_xdp_xmit and
> XDP_TX/REDIRECT. This is mainly because XDP heavily relies on NAPI
> context.
> - patch 1: Export a function needed for veth XDP.
> - patch 2-3: Basic implementation of veth XDP.
> - patch 4-6: Add ndo_xdp_xmit.
> - patch 7-9: Add XDP_TX and XDP_REDIRECT.
> - patch 10: Performance optimization for multi-queue env.
>
>
> Tests and performance numbers
> -----------------------------
>
> Tested with a simple XDP program which only redirects packets between
> NIC and veth. I used i40e 25G NIC (XXV710) for the physical NIC. The
> server has 20 of Xeon Silver 2.20 GHz cores.
>
> pktgen --(wire)--> XXV710 (i40e) <--(XDP redirect)--> veth===veth (XDP)
>
> The rightmost veth loads XDP progs and just does DROP or TX. The number
> of packets is measured in the XDP progs. The leftmost pktgen sends
> packets at 37.1 Mpps (almost 25G wire speed).
>
> veth XDP action Flows Mpps
> ================================
> DROP 1 10.6
> DROP 2 21.2
> DROP 100 36.0
> TX 1 5.0
> TX 2 10.0
> TX 100 31.0
>
> I also measured netperf TCP_STREAM but was not so great performance due
> to lack of tx/rx checksum offload and TSO, etc.
>
> netperf <--(wire)--> XXV710 (i40e) <--(XDP redirect)--> veth===veth (XDP PASS)
>
> Direction Flows Gbps
> ==============================
> external->veth 1 20.8
> external->veth 2 23.5
> external->veth 100 23.6
> veth->external 1 9.0
> veth->external 2 17.8
> veth->external 100 22.9
>
> Also tested doing ifup/down or load/unload a XDP program repeatedly
> during processing XDP packets in order to check if enabling/disabling
> NAPI is working as expected, and found no problems.
>
> v8:
> - Don't use xdp_frame pointer address to calculate skb->head, headroom,
> and xdp_buff.data_hard_start.
>
> v7:
> - Introduce xdp_scrub_frame() to clear kernel pointers in xdp_frame and
> use it instead of memset().
>
> v6:
> - Check skb->len only if reallocation is needed.
> - Add __GFP_NOWARN to alloc_page() since it can be triggered by external
> events.
> - Fix sparse warning around EXPORT_SYMBOL.
>
> v5:
> - Fix broken SOBs.
>
> v4:
> - Don't adjust MTU automatically.
> - Skip peer IFF_UP check on .ndo_xdp_xmit() because it is unnecessary.
> Add comments to explain that.
> - Use redirect_info instead of xdp_mem_info for storing no_direct flag
> to avoid per packet copy cost.
>
> v3:
> - Drop skb bulk xmit patch since it makes little performance
> difference. The hotspot in TCP skb xmit at this point is checksum
> computation in skb_segment and packet copy on XDP_REDIRECT due to
> cloned/nonlinear skb.
> - Fix race on closing device.
> - Add extack messages in ndo_bpf.
>
> v2:
> - Squash NAPI patch with "Add driver XDP" patch.
> - Remove conversion from xdp_frame to skb when NAPI is not enabled.
> - Introduce per-queue XDP ring (patch 8).
> - Introduce bulk skb xmit when XDP is enabled on the peer (patch 9).
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>
> Toshiaki Makita (10):
> net: Export skb_headers_offset_update
> veth: Add driver XDP
> veth: Avoid drops by oversized packets when XDP is enabled
> xdp: Helper function to clear kernel pointers in xdp_frame
> veth: Handle xdp_frames in xdp napi ring
> veth: Add ndo_xdp_xmit
> bpf: Make redirect_info accessible from modules
> xdp: Helpers for disabling napi_direct of xdp_return_frame
> veth: Add XDP TX and REDIRECT
> veth: Support per queue XDP ring
>
> drivers/net/veth.c | 750 ++++++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/filter.h | 35 +++
> include/linux/skbuff.h | 1 +
> include/net/xdp.h | 7 +
> net/core/filter.c | 29 +-
> net/core/skbuff.c | 3 +-
> net/core/xdp.c | 6 +-
> 7 files changed, 801 insertions(+), 30 deletions(-)
>
Applied to bpf-next, thanks Toshiaki!
^ permalink raw reply
* Re: [bpf-next V2 PATCH 0/2] Implement sample code for XDP cpumap IP-pair load-balancing
From: Daniel Borkmann @ 2018-08-10 14:10 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev
Cc: victor, eric, Daniel Borkmann, Alexei Starovoitov, jhsiao
In-Reply-To: <153390254528.10825.9125109091513926669.stgit@firesoul>
On 08/10/2018 02:02 PM, Jesper Dangaard Brouer wrote:
> Background: cpumap moves the SKB allocation out of the driver code,
> and instead allocate it on the remote CPU, and invokes the regular
> kernel network stack with the newly allocated SKB.
>
> The idea behind the XDP CPU redirect feature, is to use XDP as a
> load-balancer step in-front of regular kernel network stack. But the
> current sample code does not provide a good example of this. Part of
> the reason is that, I have implemented this as part of Suricata XDP
> load-balancer.
>
> Given this is the most frequent feature request I get. This patchset
> implement the same XDP load-balancing as Suricata does, which is a
> symmetric hash based on the IP-pairs + L4-protocol.
>
> The expected setup for the use-case is to reduce the number of NIC RX
> queues via ethtool (as XDP can handle more per core), and via
> smp_affinity assign these RX queues to a set of CPUs, which will be
> handling RX packets. The CPUs that runs the regular network stack is
> supplied to the sample xdp_redirect_cpu tool by specifying
> the --cpu option multiple times on the cmdline.
>
> I do note that cpumap SKB creation is not feature complete yet, and
> more work is coming. E.g. given GRO is not implemented yet, do expect
> TCP workloads to be slower. My measurements do indicate UDP workloads
> are faster.
Applied to bpf-next, thanks Jesper!
^ permalink raw reply
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
From: Daniel Borkmann @ 2018-08-10 14:10 UTC (permalink / raw)
To: Björn Töpel, ast, netdev, ap420073, brouer
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson, kafai
In-Reply-To: <20180810092802.5948-1-bjorn.topel@gmail.com>
On 08/10/2018 11:28 AM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
>
> The reverted commit adds a WARN to check against NULL entries in the
> mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> driver) fast path is required to make a paired
> xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> addition, a driver using a different allocation scheme than the
> default MEM_TYPE_PAGE_SHARED is required to additionally call
> xdp_rxq_info_reg_mem_model.
>
> For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> that the mem_id_ht rhashtable has a properly inserted allocator id. If
> not, this would be a driver bug. A NULL pointer kernel OOPS is
> preferred to the WARN.
>
> Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Given the last bpf pr went out yesterday night, I've applied this to
bpf-next (worst case we can just route it via stable), thanks!
^ permalink raw reply
* [PATCH net-next 1/1] tc: Update README and add config
From: Keara Leibovitz @ 2018-08-10 14:09 UTC (permalink / raw)
To: davem; +Cc: netdev, jhs, xiyou.wangcong, jiri, lucasb, Keara Leibovitz
Updated README.
Added config file that contains the minimum required features enabled to
run the tests currently present in the kernel.
This must be updated when new unittests are created and require their own
modules.
Signed-off-by: Keara Leibovitz <kleib@mojatatu.com>
---
tools/testing/selftests/tc-testing/README | 16 +++++++----
tools/testing/selftests/tc-testing/config | 48 +++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 5 deletions(-)
create mode 100644 tools/testing/selftests/tc-testing/config
diff --git a/tools/testing/selftests/tc-testing/README b/tools/testing/selftests/tc-testing/README
index 3a0336782d2d..49a6f8c3fdae 100644
--- a/tools/testing/selftests/tc-testing/README
+++ b/tools/testing/selftests/tc-testing/README
@@ -17,6 +17,10 @@ REQUIREMENTS
* The kernel must have veth support available, as a veth pair is created
prior to running the tests.
+* The kernel must have the appropriate infrastructure enabled to run all tdc
+ unit tests. See the config file in this directory for minimum required
+ features. As new tests will be added, config options list will be updated.
+
* All tc-related features being tested must be built in or available as
modules. To check what is required in current setup run:
./tdc.py -c
@@ -109,8 +113,8 @@ COMMAND LINE ARGUMENTS
Run tdc.py -h to see the full list of available arguments.
usage: tdc.py [-h] [-p PATH] [-D DIR [DIR ...]] [-f FILE [FILE ...]]
- [-c [CATG [CATG ...]]] [-e ID [ID ...]] [-l] [-s] [-i] [-v]
- [-d DEVICE] [-n NS] [-V]
+ [-c [CATG [CATG ...]]] [-e ID [ID ...]] [-l] [-s] [-i] [-v] [-N]
+ [-d DEVICE] [-P] [-n] [-V]
Linux TC unit tests
@@ -118,8 +122,10 @@ optional arguments:
-h, --help show this help message and exit
-p PATH, --path PATH The full path to the tc executable to use
-v, --verbose Show the commands that are being run
+ -N, --notap Suppress tap results for command under test
-d DEVICE, --device DEVICE
Execute the test case in flower category
+ -P, --pause Pause execution just before post-suite stage
selection:
select which test cases: files plus directories; filtered by categories
@@ -146,10 +152,10 @@ action:
-i, --id Generate ID numbers for new test cases
netns:
- options for nsPlugin(run commands in net namespace)
+ options for nsPlugin (run commands in net namespace)
- -n NS, --namespace NS
- Run commands in namespace NS
+ -n, --namespace
+ Run commands in namespace as specified in tdc_config.py
valgrind:
options for valgrindPlugin (run command under test under Valgrind)
diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
new file mode 100644
index 000000000000..203302065458
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/config
@@ -0,0 +1,48 @@
+CONFIG_NET_SCHED=y
+
+#
+# Queueing/Scheduling
+#
+CONFIG_NET_SCH_PRIO=m
+CONFIG_NET_SCH_INGRESS=m
+
+#
+# Classification
+#
+CONFIG_NET_CLS=y
+CONFIG_NET_CLS_FW=m
+CONFIG_NET_CLS_U32=m
+CONFIG_CLS_U32_PERF=y
+CONFIG_CLS_U32_MARK=y
+CONFIG_NET_EMATCH=y
+CONFIG_NET_EMATCH_STACK=32
+CONFIG_NET_EMATCH_CMP=m
+CONFIG_NET_EMATCH_NBYTE=m
+CONFIG_NET_EMATCH_U32=m
+CONFIG_NET_EMATCH_META=m
+CONFIG_NET_EMATCH_TEXT=m
+CONFIG_NET_EMATCH_IPSET=m
+CONFIG_NET_EMATCH_IPT=m
+CONFIG_NET_CLS_ACT=y
+CONFIG_NET_ACT_POLICE=m
+CONFIG_NET_ACT_GACT=m
+CONFIG_GACT_PROB=y
+CONFIG_NET_ACT_MIRRED=m
+CONFIG_NET_ACT_SAMPLE=m
+CONFIG_NET_ACT_IPT=m
+CONFIG_NET_ACT_NAT=m
+CONFIG_NET_ACT_PEDIT=m
+CONFIG_NET_ACT_SIMP=m
+CONFIG_NET_ACT_SKBEDIT=m
+CONFIG_NET_ACT_CSUM=m
+CONFIG_NET_ACT_VLAN=m
+CONFIG_NET_ACT_BPF=m
+CONFIG_NET_ACT_CONNMARK=m
+CONFIG_NET_ACT_SKBMOD=m
+CONFIG_NET_ACT_IFE=m
+CONFIG_NET_ACT_TUNNEL_KEY=m
+CONFIG_NET_IFE_SKBMARK=m
+CONFIG_NET_IFE_SKBPRIO=m
+CONFIG_NET_IFE_SKBTCINDEX=m
+CONFIG_NET_CLS_IND=y
+CONFIG_NET_SCH_FIFO=y
--
2.7.4
^ permalink raw reply related
* Re: KMSAN: uninit-value in ip_tunnel_xmit (2)
From: William Tu @ 2018-08-10 16:20 UTC (permalink / raw)
To: syzbot
Cc: David Miller, Alexey Kuznetsov, LKML,
Linux Kernel Network Developers, syzkaller-bugs,
Hideaki YOSHIFUJI
In-Reply-To: <0000000000005012b605731594e3@google.com>
On Fri, Aug 10, 2018 at 7:30 AM, syzbot
<syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 80ecacc456c1 kmsan: fix CONFIG_KMSAN=n build
> git tree: https://github.com/google/kmsan.git/master
> console output: https://syzkaller.appspot.com/x/log.txt?x=163b7794400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=93d57043084eee38
> dashboard link: https://syzkaller.appspot.com/bug?extid=4a2c52677a8a1aa283cb
> compiler: clang version 7.0.0 (trunk 334104)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=17cec9dc400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=145dcf0c400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+4a2c52677a8a1aa283cb@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KMSAN: uninit-value in ip_tunnel_xmit+0x63a/0x3b10 net/ipv4/ip_tunnel.c:645
It complains about
644 dst = tnl_params->daddr;
645 if (dst == 0) {
I guess at __gre_tunnel_init, we have to zero the tunne->parms.iph.
I will test it to see if the issue go away.
Thanks
William
> CPU: 1 PID: 4603 Comm: syz-executor712 Not tainted 4.18.0-rc4+ #27
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x185/0x1e0 lib/dump_stack.c:113
> kmsan_report+0x195/0x2c0 mm/kmsan/kmsan.c:982
> __msan_warning+0x7d/0xe0 mm/kmsan/kmsan_instr.c:645
> ip_tunnel_xmit+0x63a/0x3b10 net/ipv4/ip_tunnel.c:645
> __gre_xmit net/ipv4/ip_gre.c:449 [inline]
> ipgre_xmit+0xe16/0xef0 net/ipv4/ip_gre.c:701
> __netdev_start_xmit include/linux/netdevice.h:4148 [inline]
> netdev_start_xmit include/linux/netdevice.h:4157 [inline]
> xmit_one net/core/dev.c:3034 [inline]
> dev_hard_start_xmit+0x60f/0xcc0 net/core/dev.c:3050
> __dev_queue_xmit+0x3060/0x3c70 net/core/dev.c:3569
> dev_queue_xmit+0x4b/0x60 net/core/dev.c:3602
> packet_snd net/packet/af_packet.c:2919 [inline]
> packet_sendmsg+0x8469/0x9010 net/packet/af_packet.c:2944
> sock_sendmsg_nosec net/socket.c:641 [inline]
> sock_sendmsg net/socket.c:651 [inline]
> ___sys_sendmsg+0xed9/0x1350 net/socket.c:2125
> __sys_sendmsg net/socket.c:2163 [inline]
> __do_sys_sendmsg net/socket.c:2172 [inline]
> __se_sys_sendmsg net/socket.c:2170 [inline]
> __x64_sys_sendmsg+0x3b0/0x520 net/socket.c:2170
> do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x63/0xe7
> RIP: 0033:0x4412f9
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffdc77f10d8 EFLAGS: 00000213 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004412f9
> RDX: 0000000000000000 RSI: 0000000020000600 RDI: 0000000000000003
> RBP: 00000000006cc018 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000020 R11: 0000000000000213 R12: 0000000000402260
> R13: 00000000004022f0 R14: 0000000000000000 R15: 0000000000000000
>
> Uninit was created at:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
> kmsan_internal_poison_shadow+0xc8/0x1d0 mm/kmsan/kmsan.c:181
> kmsan_kmalloc+0xa1/0x120 mm/kmsan/kmsan_hooks.c:91
> kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan_hooks.c:100
> slab_post_alloc_hook mm/slab.h:446 [inline]
> slab_alloc_node mm/slub.c:2720 [inline]
> __kmalloc_node_track_caller+0xb48/0x11d0 mm/slub.c:4353
> __kmalloc_reserve net/core/skbuff.c:138 [inline]
> __alloc_skb+0x2cb/0x9e0 net/core/skbuff.c:206
> alloc_skb include/linux/skbuff.h:988 [inline]
> alloc_skb_with_frags+0x1e6/0xb80 net/core/skbuff.c:5271
> sock_alloc_send_pskb+0xb59/0x11e0 net/core/sock.c:2095
> packet_alloc_skb net/packet/af_packet.c:2776 [inline]
> packet_snd net/packet/af_packet.c:2867 [inline]
> packet_sendmsg+0x68cb/0x9010 net/packet/af_packet.c:2944
> sock_sendmsg_nosec net/socket.c:641 [inline]
> sock_sendmsg net/socket.c:651 [inline]
> ___sys_sendmsg+0xed9/0x1350 net/socket.c:2125
> __sys_sendmsg net/socket.c:2163 [inline]
> __do_sys_sendmsg net/socket.c:2172 [inline]
> __se_sys_sendmsg net/socket.c:2170 [inline]
> __x64_sys_sendmsg+0x3b0/0x520 net/socket.c:2170
> do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x63/0xe7
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* this here for you
From: Kelly @ 2018-08-10 10:33 UTC (permalink / raw)
To: netdev
Your photos need editing. We can do it for you.
We do editing for e-commerce photos, jewelries images and portrait photos
etc.
This will include cutout and clipping path etc , also retouching if needed.
Let;s know if you want to send photos for working.
We can do test on your photos.
Thanks,
Kelly
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: enable btf for use in all maps
From: Daniel Borkmann @ 2018-08-10 13:30 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: Alexei Starovoitov, ast, netdev, yhs
In-Reply-To: <20180810125450.qsurzvu4zuy6niqt@kafai-mbp.dhcp.thefacebook.com>
On 08/10/2018 02:54 PM, Martin KaFai Lau wrote:
> On Fri, Aug 10, 2018 at 09:55:35AM +0200, Daniel Borkmann wrote:
>> On 08/10/2018 04:13 AM, Alexei Starovoitov wrote:
>>> On Fri, Aug 10, 2018 at 12:43:20AM +0200, Daniel Borkmann wrote:
>>>> On 08/09/2018 11:44 PM, Alexei Starovoitov wrote:
>>>>> On Thu, Aug 09, 2018 at 11:30:52PM +0200, Daniel Borkmann wrote:
>>>>>> On 08/09/2018 11:14 PM, Alexei Starovoitov wrote:
>>>>>>> On Thu, Aug 09, 2018 at 09:42:20PM +0200, Daniel Borkmann wrote:
>>>>>>>> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
>>>>>>>> the basic arraymap") enabled support for BTF and dumping via
>>>>>>>> BPF fs for arraymap. However, both can be decoupled from each
>>>>>>>> other such that all BPF maps can be supported for attaching
>>>>>>>> BTF key/value information, while not all maps necessarily
>>>>>>>> need to dump via map_seq_show_elem() callback.
>>>>>>>>
>>>>>>>> The check in array_map_check_btf() can be generalized as
>>>>>>>> ultimatively the key and value size is the only contraint
>>>>>>>> that needs to match for the map. The fact that the key needs
>>>>>>>> to be of type int is optional; it could be any data type as
>>>>>>>> long as it matches the 4 byte key size, just like hash table
>>>>>>>> key or others could be of any data type as well.
>>>>>>>>
>>>>>>>> Minimal example of a hash table dump which then works out
>>>>>>>> of the box for bpftool:
>>>>>>>>
>>>>>>>> # bpftool map dump id 19
>>>>>>>> [{
>>>>>>>> "key": {
>>>>>>>> "": {
>>>>>>>> "vip": 0,
>>>>>>>> "vipv6": []
>>>>>>>> },
>>>>>>>> "port": 0,
>>>>>>>> "family": 0,
>>>>>>>> "proto": 0
>>>>>>>> },
>>>>>>>> "value": {
>>>>>>>> "flags": 0,
>>>>>>>> "vip_num": 0
>>>>>>>> }
>>>>>>>> }
>>>>>>>> ]
>>>>>>>>
>>>>>>>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>>>>>>>> Cc: Yonghong Song <yhs@fb.com>
>>>>>>>> ---
>>>>>>>> include/linux/bpf.h | 4 +---
>>>>>>>> kernel/bpf/arraymap.c | 27 ---------------------------
>>>>>>>> kernel/bpf/inode.c | 3 ++-
>>>>>>>> kernel/bpf/syscall.c | 24 ++++++++++++++++++++----
>>>>>>>> 4 files changed, 23 insertions(+), 35 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>>>>>>> index cd8790d..eb76e8e 100644
>>>>>>>> --- a/include/linux/bpf.h
>>>>>>>> +++ b/include/linux/bpf.h
>>>>>>>> @@ -48,8 +48,6 @@ struct bpf_map_ops {
>>>>>>>> u32 (*map_fd_sys_lookup_elem)(void *ptr);
>>>>>>>> void (*map_seq_show_elem)(struct bpf_map *map, void *key,
>>>>>>>> struct seq_file *m);
>>>>>>>> - int (*map_check_btf)(const struct bpf_map *map, const struct btf *btf,
>>>>>>>> - u32 key_type_id, u32 value_type_id);
>>>>>>>> };
>>>>>>>>
>>>>>>>> struct bpf_map {
>>>>>>>> @@ -118,7 +116,7 @@ static inline bool bpf_map_offload_neutral(const struct bpf_map *map)
>>>>>>>>
>>>>>>>> static inline bool bpf_map_support_seq_show(const struct bpf_map *map)
>>>>>>>> {
>>>>>>>> - return map->ops->map_seq_show_elem && map->ops->map_check_btf;
>>>>>>>> + return map->btf && map->ops->map_seq_show_elem;
>>>>>>>> }
>>>>>>>>
>>>>>>>> extern const struct bpf_map_ops bpf_map_offload_ops;
>>>>>>>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>>>>>>>> index 2aa55d030..67f0bdf 100644
>>>>>>>> --- a/kernel/bpf/arraymap.c
>>>>>>>> +++ b/kernel/bpf/arraymap.c
>>>>>>>> @@ -358,32 +358,6 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
>>>>>>>> rcu_read_unlock();
>>>>>>>> }
>>>>>>>>
>>>>>>>> -static int array_map_check_btf(const struct bpf_map *map, const struct btf *btf,
>>>>>>>> - u32 btf_key_id, u32 btf_value_id)
>>>>>>>> -{
>>>>>>>> - const struct btf_type *key_type, *value_type;
>>>>>>>> - u32 key_size, value_size;
>>>>>>>> - u32 int_data;
>>>>>>>> -
>>>>>>>> - key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
>>>>>>>> - if (!key_type || BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
>>>>>>>> - return -EINVAL;
>>>>>>>> -
>>>>>>>> - int_data = *(u32 *)(key_type + 1);
>>>>>>>> - /* bpf array can only take a u32 key. This check makes
>>>>>>>> - * sure that the btf matches the attr used during map_create.
>>>>>>>> - */
>>>>>>>> - if (BTF_INT_BITS(int_data) != 32 || key_size != 4 ||
>>>>>>>> - BTF_INT_OFFSET(int_data))
>>>>>>>> - return -EINVAL;
>>>>>>>
>>>>>>> I think most of these checks are still necessary for array type.
>>>>>>> Relaxing BTF array key from BTF_KIND_INT to, for example, BTF_KIND_ENUM
>>>>>>> is probably ok, but key being BTF_KIND_PTR or BTF_KIND_ARRAY doesn't makes sense.
>>>>>>
>>>>>> Hmm, so on 64 bit archs BTF_KIND_PTR would get rejected for array,
>>>>>> on 32 bit it may be allowed due to sizeof(void *) == 4. BTF_KIND_ARRAY
>>>>>> could be array of u8 foo[4], for example, or u16 foo[2]. But how would
>>>>>> it ultimately be different from e.g. having 'struct a' versus 'struct b'
>>>>>> where both are of same size and while actual key has 'struct a', the one
>>>>>> who writes the prog resp. loads the BTF into the kernel would lie about
>>>>>> it stating it's of type 'struct b' instead? It's basically trusting the
>>>>>> app that it advertised sane key types which kernel is propagating back.
>>>>>
>>>>> for hash map - yes. the kernel cannot yet catch the lie that
>>>>> key == 'struct a' that user said in BTF is not what program used
>>>>> (which used 'struct b' of the same size).
>>>>> Eventually we will annotate all load/store in the program and will
>>>>> make sure that memory access match what BTF said.
>>>>
>>>> But in that case, would you reject the program? E.g. from prog point of
>>>> view, it's just a buffer of x bytes, so key could be casted to different
>>>> struct/types potentially and used for lookup; similar with value if you
>>>> would go the route to annotate all access into it. I don't think this
>>>> serves as a security feature (since you might as well just load the prog
>>>> without BTF just fine), but it can be used to help verifier to perform
>>>> rewrites like in tracing for implicit bpf_probe_read() based on member
>>>> access. But also in that case, if you might have e.g. stale or wrong BTF
>>>> data e.g. of the whole kernel or some application it would follow/walk
>>>> that one instead. But such user error would be "acceptable" since it serves
>>>> as a hint, roughly similar to (explicitly) walking the data structures
>>>> based on the headers today, you do have better control in terms of header
>>>> mismatches in that you can ship the BTF directly from the build, but there's
>>>> still no guarantee in that sense that you "verified" that these bytes
>>>> originally were mapped to struct foo somewhere in a C program.
>>>
>>> I wouldn't view such key checks as safety feature, but rather
>>> as honesty check. Meaning that user space shouldn't be able to cheat
>>> by passing completely bogus BTF into the kernel that only
>>> statisfies single size check.
>>
>> Ok, meaning, we agree that BTF is passed as a hint to the kernel. It is
>> a 'should not cheat' requirement but not 'cannot cheat', meaning if it
>> was the latter, BTF is core part of the verifier's safety analysis /
>> checks, but as it's optional on top of it (well, due to compatibility it
>> kind of needs to be anyway), it doesn't affect kernel's safety in
>> relation to what the program is doing internally.
>>
>>> Say, BTF has a key that is a 4 byte struct { char a,b,c,d; };
>>> but program operates with it as u32. I very much would like
>>> the verifier to notice and reject such program, since if C code
>>> has struct { char a,b,c,d; }; the compiler won't generate u32 access
>>> to it unless C code type casts, but then llvm will warn. So for u32
>>> to legitimately appear in the generated code the struct should be:
>>> union {
>>> struct { char a,b,c,d;}
>>> u32 e;
>>> };
>>> Narrow access (like u8 load/store in the bpf prog form u32 BTF field)
>>> is ok, since that's normal compiler optimization, but any other
>>> field/size mismatch the verifier should reject to prevent cheating.
>>>
>>> In other words even proprietary bpf programs should not be able to cheat.
>>> If they provide BTF to the kernel, it should correspond exactly to the program.
>>> That's the key to _trusted_ introspection.
>>
>> But then wouldn't this artificially force users into programming /
>> thinking style that verifier dictates upon them similar as we have
>> today as opposed to further moving away from it to allow more C-style
>> programs to be accepted?
>>
>> But even if that is the goal, it is still used as a hint today, e.g.
>> even for an array I could tell the compiler that the key is '__u32'
>> for BTF sake while using the underlying key differently (e.g. as struct,
>> enum, etc) in order to pass the array_map_check_btf() check in the
>> kernel. Those type of programs would still need to be accepted due
>> to compatibility reasons. Same if we only test on size match in other
>> maps, it's nothing different. I don't see how verifier would start
>> enforcing programs to be rejected based on access patterns on the
>> types; while it might work for newly added program types that this
>> could be enforced, it cannot for all the many existing program types,
>> but I also don't really think it needs to be. Unless it is declared
>> a safety feature for future program types.
>>
>>> Admin that ssh-es into the box and operates with bpftool should be
>>> certain that the map introspection represent the real situation of
>>> the program. If proprietary prog is paranoid about layout of map
>>> it shouldn't be using BTF, but if it does, BTF should match.
>>> In the future I'd like to enfore availability of BTF for
>>> new program types.
>>
>> Sure, and in vast majority / normal cases it will represent the real
>> situation. Do you mistrust DWARF debugging data that gets shipped with
>> your distro, for example? pahole and other tools on top of it? It is
>> pretty much the same situation. Given gdb example below, does gdb do
>> any verification of the binary in order to analyze load/store patterns
>> and whether it matches with what sits in DWARF as types? It would just
>> as well map the types into memory and dump it as pretty print instead.
>> And that's okay, the one thing that needs to be verified for correctness
>> resp. validity is the debugging format itself so it can be used for
>> that.
> I think decoupling map_seq_show_elem() requirement from BTF support
> during map_create() is ok. There is plan to do that going forward if
> it ever hits some map types that is difficult to do map_seq_show_elem().
> However, I think most of the maps should have pretty straight
> forward seq_show implementation considering most of the
> heavy lifting has already been done in btf_*_seq_show()
> and bpffs_map_seq_ops.
I think potentially this could also be made generic for majority of maps
in that you do the lookup and then the key/value btf_type_seq_show()
dump from it under RCU.
> Regarding one general map_check_btf() for all maps' purpose,
> I think not all bpf maps are behaving the same.
>
> There are some property that the kernel implies on a particular
> map type. Some of them even have kernel generated value in it.
> For example, map-in-map, the lookup_elem() value is a map's id
> which must be an integer generated by kernel's idr_alloc_cyclic().
> Treating it as enum/struct/array...etc seems wrong.
> Hence, I think each map type should be able to provide
> its own map_check_btf(). If a map does not have
> specific key/value requirement, it can use the default one which
> is the min requirement for kernel safety reason.
Yeah, fair point, I think potentially for the in-kernel generated ones at
some point they could provide their own in-kernel BTF description. Ok,
I'll rework to have a default and stricter map specific override if present
which those in-kernel ones will reject then.
Thanks,
Daniel
^ 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