* Re: netlink: 16 bytes leftover after parsing attributes in process `ip'.
From: Jiri Benc @ 2018-09-25 14:47 UTC (permalink / raw)
To: Christian Brauner; +Cc: David Ahern, netdev@vger.kernel.org, David Miller
In-Reply-To: <20180925094908.yuu5bsrazkiq3ihy@brauner.io>
On Tue, 25 Sep 2018 11:49:10 +0200, Christian Brauner wrote:
> So if people really want to hide this issue as much as we can then we
> can play the guessing game. I could send a patch that roughly does the
> following:
>
> if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg))
> guessed_header_len = sizeof(struct ifaddrmsg);
> else
> guessed_header_len = sizeof(struct ifinfomsg);
>
> This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16.
> The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID.
> This propert is a __s32 which should bring the message up to 12 bytes
> (not sure about alignment requiremnts and where we might wend up ten)
> which is still less than the 16 bytes without that property from
> ifinfomsg. That's a hacky hacky hack-hack and will likely work but will
> break when ifaddrmsg grows a new member or we introduce another property
> that is valid in RTM_GETADDR requests. It also will not work cleanly
> when users stuff additional properties in there that are vaif (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,lid for the
> address family but are not used int RTM_GETADDR requests.
I'd expect that any potential existing code that makes use of other
attributes already assumes ifaddrmsg. Hence, if the nlmsg_len is >
sizeof(ifinfomsg), you can be sure that there are attributes and thus
the struct used was ifaddrmsg.
So, in order for RTM_GETADDR to work reliably with attributes, you have
to ensure that the length is > sizeof(ifinfomsg).
This can be achieved by putting IFA_TARGET_NETNSID into a nested
attribute. Just define IFA_EXTENDED (feel free to invent a better name,
of course) and put IFA_TARGET_NETNSID inside. Then in the code, attempt
to parse only when the size is large enough:
if (nlmsg_len(cb->nlh) > sizeof(struct ifinfomsg)) {
int err;
err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
IFA_MAX, ifa_ipv6_policy, NULL);
if (err < 0)
return err;
if (tb[IFA_EXTENDED]) {
...parse the nested attribute...
if (tb_nested[IFA_TARGET_NETNSID]) {
...etc...
}
}
}
Another option is forcing the user space to add another attribute, for
example, IFA_FLAGS_PRESENT, and attempt parsing only when it is
present. The logic would then be:
if (nlmsg_len(cb->nlh) > sizeof(struct ifinfomsg)) {
int err;
err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
IFA_MAX, ifa_ipv6_policy, NULL);
if (err < 0)
return err;
if (tb[IFA_FLAGS_PRESENT] && tb[IFA_TARGET_NETNSID]) {
...etc...
}
}
Jiri
^ permalink raw reply
* Re: [PATCH net-next v6 02/23] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-25 20:54 UTC (permalink / raw)
To: Joe Perches
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <abedae13e59e01200531931e530b46e937501c4a.camel@perches.com>
Hi Joe,
On Tue, Sep 25, 2018 at 10:21 PM Joe Perches <joe@perches.com> wrote:
> I'd still prefer as all these are effectively
> debugging output that you convert the pr_info
> uses pr_debug and so avoid using pr_fmt altogether.
I started to write back, "sure no problem, this will be in v7," but
then I gave it a closer look, and I think actually I'll be changing
these into pr_err/pr_info and not naming this "DEBUG" but rather
"SELFTEST", since I think these are actually very good to have beyond
mere debugging.
Jason
^ permalink raw reply
* Re: [PATCH net 00/15] netpoll: avoid capture effects for NAPI drivers
From: Michael Chan @ 2018-09-25 14:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Song Liu, David Miller, Netdev, Ariel Elior, Eric Dumazet, tariqt,
Saeed Mahameed, jeffrey.t.kirsher, Jakub Kicinski, j.vosburgh,
vfalico, Andy Gospodarek
In-Reply-To: <CANn89i+rp-fAfLDSBpLN61X9R9hN4JQyoJgP8T=SUJ8PcSva-Q@mail.gmail.com>
On Tue, Sep 25, 2018 at 7:20 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Sep 25, 2018 at 7:02 AM Michael Chan <michael.chan@broadcom.com> wrote:
> >
> > On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubraving@fb.com> wrote:
> > >
> > >
> > >
> > > > On Sep 24, 2018, at 2:05 PM, Eric Dumazet <edumazet@google.com> wrote:
> > > >
> > > >>
> > > >> Interesting, maybe a bnxt specific issue.
> > > >>
> > > >> It seems their model is to process TX/RX notification in the same queue,
> > > >> they throw away RX events if budget == 0
> > > >>
> > > >> It means commit e7b9569102995ebc26821789628eef45bd9840d8 is wrong and
> > > >> must be reverted.
> > > >>
> > > >> Otherwise, we have a possibility of blocking a queue under netpoll pressure.
> > > >
> > > > Hmm, actually a revert might not be enough, since code at lines 2030-2031
> > > > would fire and we might not call napi_complete_done() anyway.
> > > >
> > > > Unfortunately this driver logic is quite complex.
> > > >
> > > > Could you test on other NIC eventually ?
> > > >
> > >
> > > It actually runs OK on ixgbe.
> > >
> > > @Michael, could you please help us with this?
> > >
> > I've taken a quick look using today's net tree plus Eric's
> > poll_one_napi() patch. The problem I'm seeing is that netpoll calls
> > bnxt_poll() with budget 0. And since work_done >= budget of 0, we
> > return without calling napi_complete_done() and without arming the
> > interrupt. netpoll doesn't always call us back until we call
> > napi_complete_done(), right? So I think if there are in-flight TX
> > completions, we'll miss those.
>
> That's the whole point of netpoll :
>
> We drain the TX queues, without interrupts being involved at all,
> by calling ->napi() with a zero budget.
>
> napi_complete(), even if called from ->napi() while budget was zero,
> should do nothing but return early.
>
> budget==0 means that ->napi() should process all TX completions.
All TX completions that we can see. We cannot see the in-flight ones.
If budget is exceeded, I think the assumption is that poll will always
be called again.
>
> So it looks like bnxt has a bug, that is showing up after the latest
> poll_one_napi() patch.
> This latest patch is needed otherwise the cpu attempting the
> netpoll-TX-drain might drain nothing at all,
> since it does not anymore call ndo_poll_controller() that was grabbing
> SCHED bits on all queues (napi_schedule() like calls)
I think the latest patch is preventing the normal interrupt -> NAPI
path from coming in and cleaning the remaining TX completions and
arming the interrupt.
^ permalink raw reply
* Re: [PATCH net-next v5 02/20] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-25 14:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ard Biesheuvel, Eric Biggers, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Jean-Philippe Aumasson
In-Reply-To: <CAK8P3a3j0eqAPc-Vkp7YFxT56R2-LrqL_EcULQFam5Bm93gPyw@mail.gmail.com>
Hey Arnd,
On Tue, Sep 25, 2018 at 9:18 AM Arnd Bergmann <arnd@arndb.de> wrote:
> I think I misread your earlier sentence and thought you had said the
> exact opposite.
>
> For confirmation, I've downloaded your git tree and built it with my
> collection of compilers (gcc-4.6 through 8.1) and tried building it
> in various configurations. Nothing alarming stood out, the only
> thing that I think would might warrant some investigation is this one:
>
> lib/zinc/curve25519/curve25519-hacl64.h: In function 'curve25519_generic':
> lib/zinc/curve25519/curve25519-hacl64.h:785:1: warning: the frame size
> of 1536 bytes is larger than 500 bytes [-Wframe-larger-than=]
>
> Without KASAN, this takes 832 bytes, which is still more than it should
> use from a look at the source code.
>
> I first suspected some misoptimization around the get/put_unaligned_le64()
> calls, but playing around with it some more led me to this patch:
Excellent detective work. Thanks for spotting that before I had a time
to respond here. I'll also send that same fix to the HACL* team at
INRIA, so they can add it as a heuristic.
Jason
^ permalink raw reply
* [bpf-next PATCH 3/3] selftests/bpf: add XDP selftests for modifying and popping VLAN headers
From: Jesper Dangaard Brouer @ 2018-09-25 14:25 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer; +Cc: Daniel Borkmann, yoel, Alexei Starovoitov
In-Reply-To: <153788541527.7040.8722029216689268562.stgit@firesoul>
This XDP selftest also contain a small TC-bpf component. It provoke
the generic-XDP bug fixed in previous commit.
The selftest itself shows how to do VLAN manipulation from XDP and TC.
The test demonstrate how XDP ingress can remove a VLAN tag, and how TC
egress can add back a VLAN tag.
This use-case originates from a production need by ISP (kviknet.dk),
who gets DSL-lines terminated as VLAN Q-in-Q tagged packets, and want
to avoid having an net_device for every end-customer on the box doing
the L2 to L3 termination.
The test-setup is done via a veth-pair and creating two network
namespaces (ns1 and ns2). The 'ns1' simulate the ISP network that are
loading the BPF-progs stripping and adding VLAN IDs. The 'ns2'
simulate the DSL-customer that are using VLAN tagged packets.
Running the script with --interactive, will simply not call the
cleanup function. This gives the effect of creating a testlab, that
the users can inspect and play with. The --verbose option will simply
request that the shell will print input lines as they are read, this
include comments, which in effect make the comments visible docs.
Reported-by: Yoel Caspersen <yoel@kviknet.dk>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
tools/testing/selftests/bpf/Makefile | 5
tools/testing/selftests/bpf/test_xdp_vlan.c | 292 ++++++++++++++++++++++++++
tools/testing/selftests/bpf/test_xdp_vlan.sh | 195 +++++++++++++++++
3 files changed, 490 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/test_xdp_vlan.c
create mode 100755 tools/testing/selftests/bpf/test_xdp_vlan.sh
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index fd3851d5c079..b2c80a73b148 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -35,7 +35,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
- test_skb_cgroup_id_kern.o bpf_flow.o
+ test_skb_cgroup_id_kern.o bpf_flow.o test_xdp_vlan.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
@@ -48,7 +48,8 @@ TEST_PROGS := test_kmod.sh \
test_lwt_seg6local.sh \
test_lirc_mode2.sh \
test_skb_cgroup_id.sh \
- test_flow_dissector.sh
+ test_flow_dissector.sh \
+ test_xdp_vlan.sh
# Compile but not part of 'make run_tests'
TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user \
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan.c b/tools/testing/selftests/bpf/test_xdp_vlan.c
new file mode 100644
index 000000000000..365a7d2d9f5c
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_vlan.c
@@ -0,0 +1,292 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * Copyright(c) 2018 Jesper Dangaard Brouer.
+ *
+ * XDP/TC VLAN manipulation example
+ *
+ * GOTCHA: Remember to disable NIC hardware offloading of VLANs,
+ * else the VLAN tags are NOT inlined in the packet payload:
+ *
+ * # ethtool -K ixgbe2 rxvlan off
+ *
+ * Verify setting:
+ * # ethtool -k ixgbe2 | grep rx-vlan-offload
+ * rx-vlan-offload: off
+ *
+ */
+#include <stddef.h>
+#include <stdbool.h>
+#include <string.h>
+#include <linux/bpf.h>
+#include <linux/if_ether.h>
+#include <linux/if_vlan.h>
+#include <linux/in.h>
+#include <linux/pkt_cls.h>
+
+#include "bpf_helpers.h"
+#include "bpf_endian.h"
+
+/* linux/if_vlan.h have not exposed this as UAPI, thus mirror some here
+ *
+ * struct vlan_hdr - vlan header
+ * @h_vlan_TCI: priority and VLAN ID
+ * @h_vlan_encapsulated_proto: packet type ID or len
+ */
+struct _vlan_hdr {
+ __be16 h_vlan_TCI;
+ __be16 h_vlan_encapsulated_proto;
+};
+#define VLAN_PRIO_MASK 0xe000 /* Priority Code Point */
+#define VLAN_PRIO_SHIFT 13
+#define VLAN_CFI_MASK 0x1000 /* Canonical Format Indicator */
+#define VLAN_TAG_PRESENT VLAN_CFI_MASK
+#define VLAN_VID_MASK 0x0fff /* VLAN Identifier */
+#define VLAN_N_VID 4096
+
+struct parse_pkt {
+ __u16 l3_proto;
+ __u16 l3_offset;
+ __u16 vlan_outer;
+ __u16 vlan_inner;
+ __u8 vlan_outer_offset;
+ __u8 vlan_inner_offset;
+};
+
+char _license[] SEC("license") = "GPL";
+
+static __always_inline
+bool parse_eth_frame(struct ethhdr *eth, void *data_end, struct parse_pkt *pkt)
+{
+ __u16 eth_type;
+ __u8 offset;
+
+ offset = sizeof(*eth);
+ /* Make sure packet is large enough for parsing eth + 2 VLAN headers */
+ if ((void *)eth + offset + (2*sizeof(struct _vlan_hdr)) > data_end)
+ return false;
+
+ eth_type = eth->h_proto;
+
+ /* Handle outer VLAN tag */
+ if (eth_type == bpf_htons(ETH_P_8021Q)
+ || eth_type == bpf_htons(ETH_P_8021AD)) {
+ struct _vlan_hdr *vlan_hdr;
+
+ vlan_hdr = (void *)eth + offset;
+ pkt->vlan_outer_offset = offset;
+ pkt->vlan_outer = bpf_ntohs(vlan_hdr->h_vlan_TCI)
+ & VLAN_VID_MASK;
+ eth_type = vlan_hdr->h_vlan_encapsulated_proto;
+ offset += sizeof(*vlan_hdr);
+ }
+
+ /* Handle inner (double) VLAN tag */
+ if (eth_type == bpf_htons(ETH_P_8021Q)
+ || eth_type == bpf_htons(ETH_P_8021AD)) {
+ struct _vlan_hdr *vlan_hdr;
+
+ vlan_hdr = (void *)eth + offset;
+ pkt->vlan_inner_offset = offset;
+ pkt->vlan_inner = bpf_ntohs(vlan_hdr->h_vlan_TCI)
+ & VLAN_VID_MASK;
+ eth_type = vlan_hdr->h_vlan_encapsulated_proto;
+ offset += sizeof(*vlan_hdr);
+ }
+
+ pkt->l3_proto = bpf_ntohs(eth_type); /* Convert to host-byte-order */
+ pkt->l3_offset = offset;
+
+ return true;
+}
+
+/* Hint, VLANs are choosen to hit network-byte-order issues */
+#define TESTVLAN 4011 /* 0xFAB */
+// #define TO_VLAN 4000 /* 0xFA0 (hint 0xOA0 = 160) */
+
+SEC("xdp_drop_vlan_4011")
+int xdp_prognum0(struct xdp_md *ctx)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct parse_pkt pkt = { 0 };
+
+ if (!parse_eth_frame(data, data_end, &pkt))
+ return XDP_ABORTED;
+
+ /* Drop specific VLAN ID example */
+ if (pkt.vlan_outer == TESTVLAN)
+ return XDP_ABORTED;
+ /*
+ * Using XDP_ABORTED makes it possible to record this event,
+ * via tracepoint xdp:xdp_exception like:
+ * # perf record -a -e xdp:xdp_exception
+ * # perf script
+ */
+ return XDP_PASS;
+}
+/*
+Commands to setup VLAN on Linux to test packets gets dropped:
+
+ export ROOTDEV=ixgbe2
+ export VLANID=4011
+ ip link add link $ROOTDEV name $ROOTDEV.$VLANID type vlan id $VLANID
+ ip link set dev $ROOTDEV.$VLANID up
+
+ ip link set dev $ROOTDEV mtu 1508
+ ip addr add 100.64.40.11/24 dev $ROOTDEV.$VLANID
+
+Load prog with ip tool:
+
+ ip link set $ROOTDEV xdp off
+ ip link set $ROOTDEV xdp object xdp_vlan01_kern.o section xdp_drop_vlan_4011
+
+*/
+
+/* Changing VLAN to zero, have same practical effect as removing the VLAN. */
+#define TO_VLAN 0
+
+SEC("xdp_vlan_change")
+int xdp_prognum1(struct xdp_md *ctx)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct parse_pkt pkt = { 0 };
+
+ if (!parse_eth_frame(data, data_end, &pkt))
+ return XDP_ABORTED;
+
+ /* Change specific VLAN ID */
+ if (pkt.vlan_outer == TESTVLAN) {
+ struct _vlan_hdr *vlan_hdr = data + pkt.vlan_outer_offset;
+
+ /* Modifying VLAN, preserve top 4 bits */
+ vlan_hdr->h_vlan_TCI =
+ bpf_htons((bpf_ntohs(vlan_hdr->h_vlan_TCI) & 0xf000)
+ | TO_VLAN);
+ }
+
+ return XDP_PASS;
+}
+
+/*
+ * Show XDP+TC can cooperate, on creating a VLAN rewriter.
+ * 1. Create a XDP prog that can "pop"/remove a VLAN header.
+ * 2. Create a TC-bpf prog that egress can add a VLAN header.
+ */
+
+#ifndef ETH_ALEN /* Ethernet MAC address length */
+#define ETH_ALEN 6 /* bytes */
+#endif
+#define VLAN_HDR_SZ 4 /* bytes */
+
+SEC("xdp_vlan_remove_outer")
+int xdp_prognum2(struct xdp_md *ctx)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct parse_pkt pkt = { 0 };
+ char *dest;
+
+ if (!parse_eth_frame(data, data_end, &pkt))
+ return XDP_ABORTED;
+
+ /* Skip packet if no outer VLAN was detected */
+ if (pkt.vlan_outer_offset == 0)
+ return XDP_PASS;
+
+ /* Moving Ethernet header, dest overlap with src, memmove handle this */
+ dest = data;
+ dest+= VLAN_HDR_SZ;
+ /*
+ * Notice: Taking over vlan_hdr->h_vlan_encapsulated_proto, by
+ * only moving two MAC addrs (12 bytes), not overwriting last 2 bytes
+ */
+ __builtin_memmove(dest, data, ETH_ALEN * 2);
+ /* Note: LLVM built-in memmove inlining require size to be constant */
+
+ /* Move start of packet header seen by Linux kernel stack */
+ bpf_xdp_adjust_head(ctx, VLAN_HDR_SZ);
+
+ return XDP_PASS;
+}
+
+static __always_inline
+void shift_mac_4bytes_16bit(void *data)
+{
+ __u16 *p = data;
+
+ p[7] = p[5]; /* delete p[7] was vlan_hdr->h_vlan_TCI */
+ p[6] = p[4]; /* delete p[6] was ethhdr->h_proto */
+ p[5] = p[3];
+ p[4] = p[2];
+ p[3] = p[1];
+ p[2] = p[0];
+}
+
+static __always_inline
+void shift_mac_4bytes_32bit(void *data)
+{
+ __u32 *p = data;
+
+ /* Assuming VLAN hdr present. The 4 bytes in p[3] that gets
+ * overwritten, is ethhdr->h_proto and vlan_hdr->h_vlan_TCI.
+ * The vlan_hdr->h_vlan_encapsulated_proto take over role as
+ * ethhdr->h_proto.
+ */
+ p[3] = p[2];
+ p[2] = p[1];
+ p[1] = p[0];
+}
+
+SEC("xdp_vlan_remove_outer2")
+int xdp_prognum3(struct xdp_md *ctx)
+{
+ void *data_end = (void *)(long)ctx->data_end;
+ void *data = (void *)(long)ctx->data;
+ struct ethhdr *orig_eth = data;
+ struct parse_pkt pkt = { 0 };
+
+ if (!parse_eth_frame(orig_eth, data_end, &pkt))
+ return XDP_ABORTED;
+
+ /* Skip packet if no outer VLAN was detected */
+ if (pkt.vlan_outer_offset == 0)
+ return XDP_PASS;
+
+ /* Simply shift down MAC addrs 4 bytes, overwrite h_proto + TCI */
+ shift_mac_4bytes_32bit(data);
+
+ /* Move start of packet header seen by Linux kernel stack */
+ bpf_xdp_adjust_head(ctx, VLAN_HDR_SZ);
+
+ return XDP_PASS;
+}
+
+/*=====================================
+ * BELOW: TC-hook based ebpf programs
+ * ====================================
+ * The TC-clsact eBPF programs (currently) need to be attach via TC commands
+ */
+
+SEC("tc_vlan_push")
+int _tc_progA(struct __sk_buff *ctx)
+{
+ bpf_skb_vlan_push(ctx, bpf_htons(ETH_P_8021Q), TESTVLAN);
+
+ return TC_ACT_OK;
+}
+/*
+Commands to setup TC to use above bpf prog:
+
+export ROOTDEV=ixgbe2
+export FILE=xdp_vlan01_kern.o
+
+# Re-attach clsact to clear/flush existing role
+tc qdisc del dev $ROOTDEV clsact 2> /dev/null ;\
+tc qdisc add dev $ROOTDEV clsact
+
+# Attach BPF prog EGRESS
+tc filter add dev $ROOTDEV egress \
+ prio 1 handle 1 bpf da obj $FILE sec tc_vlan_push
+
+tc filter show dev $ROOTDEV egress
+*/
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan.sh b/tools/testing/selftests/bpf/test_xdp_vlan.sh
new file mode 100755
index 000000000000..51a3a31d1aac
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_vlan.sh
@@ -0,0 +1,195 @@
+#!/bin/bash
+
+TESTNAME=xdp_vlan
+
+usage() {
+ echo "Testing XDP + TC eBPF VLAN manipulations: $TESTNAME"
+ echo ""
+ echo "Usage: $0 [-vfh]"
+ echo " -v | --verbose : Verbose"
+ echo " --flush : Flush before starting (e.g. after --interactive)"
+ echo " --interactive : Keep netns setup running after test-run"
+ echo ""
+}
+
+cleanup()
+{
+ local status=$?
+
+ if [ "$status" = "0" ]; then
+ echo "selftests: $TESTNAME [PASS]";
+ else
+ echo "selftests: $TESTNAME [FAILED]";
+ fi
+
+ if [ -n "$INTERACTIVE" ]; then
+ echo "Namespace setup still active explore with:"
+ echo " ip netns exec ns1 bash"
+ echo " ip netns exec ns2 bash"
+ exit $status
+ fi
+
+ set +e
+ ip link del veth1 2> /dev/null
+ ip netns del ns1 2> /dev/null
+ ip netns del ns2 2> /dev/null
+}
+
+# Using external program "getopt" to get --long-options
+OPTIONS=$(getopt -o hvfi: \
+ --long verbose,flush,help,interactive,debug -- "$@")
+if (( $? != 0 )); then
+ usage
+ echo "selftests: $TESTNAME [FAILED] Error calling getopt, unknown option?"
+ exit 2
+fi
+eval set -- "$OPTIONS"
+
+## --- Parse command line arguments / parameters ---
+while true; do
+ case "$1" in
+ -v | --verbose)
+ export VERBOSE=yes
+ shift
+ ;;
+ -i | --interactive | --debug )
+ INTERACTIVE=yes
+ shift
+ ;;
+ -f | --flush )
+ cleanup
+ shift
+ ;;
+ -- )
+ shift
+ break
+ ;;
+ -h | --help )
+ usage;
+ echo "selftests: $TESTNAME [SKIP] usage help info requested"
+ exit 0
+ ;;
+ * )
+ shift
+ break
+ ;;
+ esac
+done
+
+if [ "$EUID" -ne 0 ]; then
+ echo "selftests: $TESTNAME [FAILED] need root privileges"
+ exit 1
+fi
+
+ip link set dev lo xdp off 2>/dev/null > /dev/null
+if [ $? -ne 0 ];then
+ echo "selftests: $TESTNAME [SKIP] need ip xdp support"
+ exit 0
+fi
+
+# Interactive mode likely require us to cleanup netns
+if [ -n "$INTERACTIVE" ]; then
+ ip link del veth1 2> /dev/null
+ ip netns del ns1 2> /dev/null
+ ip netns del ns2 2> /dev/null
+fi
+
+# Exit on failure
+set -e
+
+# Some shell-tools dependencies
+which ip > /dev/null
+which tc > /dev/null
+which ethtool > /dev/null
+
+# Make rest of shell verbose, showing comments as doc/info
+if [ -n "$VERBOSE" ]; then
+ set -v
+fi
+
+# Create two namespaces
+ip netns add ns1
+ip netns add ns2
+
+# Run cleanup if failing or on kill
+trap cleanup 0 2 3 6 9
+
+# Create veth pair
+ip link add veth1 type veth peer name veth2
+
+# Move veth1 and veth2 into the respective namespaces
+ip link set veth1 netns ns1
+ip link set veth2 netns ns2
+
+# NOTICE: XDP require VLAN header inside packet payload
+# - Thus, disable VLAN offloading driver features
+# - For veth REMEMBER TX side VLAN-offload
+#
+# Disable rx-vlan-offload (mostly needed on ns1)
+ip netns exec ns1 ethtool -K veth1 rxvlan off
+ip netns exec ns2 ethtool -K veth2 rxvlan off
+#
+# Disable tx-vlan-offload (mostly needed on ns2)
+ip netns exec ns2 ethtool -K veth2 txvlan off
+ip netns exec ns1 ethtool -K veth1 txvlan off
+
+export IPADDR1=100.64.41.1
+export IPADDR2=100.64.41.2
+
+# In ns1/veth1 add IP-addr on plain net_device
+ip netns exec ns1 ip addr add ${IPADDR1}/24 dev veth1
+ip netns exec ns1 ip link set veth1 up
+
+# In ns2/veth2 create VLAN device
+export VLAN=4011
+export DEVNS2=veth2
+ip netns exec ns2 ip link add link $DEVNS2 name $DEVNS2.$VLAN type vlan id $VLAN
+ip netns exec ns2 ip addr add ${IPADDR2}/24 dev $DEVNS2.$VLAN
+ip netns exec ns2 ip link set $DEVNS2 up
+ip netns exec ns2 ip link set $DEVNS2.$VLAN up
+
+# Bringup lo in netns (to avoids confusing people using --interactive)
+ip netns exec ns1 ip link set lo up
+ip netns exec ns2 ip link set lo up
+
+# At this point, the hosts cannot reach each-other,
+# because ns2 are using VLAN tags on the packets.
+
+ip netns exec ns2 sh -c 'ping -W 1 -c 1 100.64.41.1 || echo "Okay ping fails"'
+
+
+# Now we can use the test_xdp_vlan.c program to pop/push these VLAN tags
+# ----------------------------------------------------------------------
+# In ns1: ingress use XDP to remove VLAN tags
+export DEVNS1=veth1
+export FILE=test_xdp_vlan.o
+
+# First test: Remove VLAN by setting VLAN ID 0, using "xdp_vlan_change"
+export XDP_PROG=xdp_vlan_change
+ip netns exec ns1 ip link set $DEVNS1 xdp object $FILE section $XDP_PROG
+
+# In ns1: egress use TC to add back VLAN tag 4011
+# (del cmd)
+# tc qdisc del dev $DEVNS1 clsact 2> /dev/null
+#
+ip netns exec ns1 tc qdisc add dev $DEVNS1 clsact
+ip netns exec ns1 tc filter add dev $DEVNS1 egress \
+ prio 1 handle 1 bpf da obj $FILE sec tc_vlan_push
+
+# Now the namespaces can reach each-other, test with ping:
+ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
+ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
+
+# Second test: Replace xdp prog, that fully remove vlan header
+#
+# Catch kernel bug for generic-XDP, that does didn't allow us to
+# remove a VLAN header, because skb->protocol still contain VLAN
+# ETH_P_8021Q indication, and this cause overwriting of our changes.
+#
+export XDP_PROG=xdp_vlan_remove_outer2
+ip netns exec ns1 ip link set $DEVNS1 xdp off
+ip netns exec ns1 ip link set $DEVNS1 xdp object $FILE section $XDP_PROG
+
+# Now the namespaces should still be able reach each-other, test with ping:
+ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
+ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
^ permalink raw reply related
* [bpf-next PATCH 2/3] bpf: make TC vlan bpf_helpers avail to selftests
From: Jesper Dangaard Brouer @ 2018-09-25 14:25 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer; +Cc: Daniel Borkmann, yoel, Alexei Starovoitov
In-Reply-To: <153788541527.7040.8722029216689268562.stgit@firesoul>
The helper bpf_skb_vlan_push is needed by next patch, and the helper
bpf_skb_vlan_pop is added for completeness, regarding VLAN helpers.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index e4be7730222d..d057e6891a6b 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -143,6 +143,10 @@ static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
(void *) BPF_FUNC_skb_cgroup_id;
static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
(void *) BPF_FUNC_skb_ancestor_cgroup_id;
+static int (*bpf_skb_vlan_push)(void *ctx, __be16 vlan_proto, __u16 vlan_tci) =
+ (void *) BPF_FUNC_skb_vlan_push;
+static int (*bpf_skb_vlan_pop)(void *ctx) =
+ (void *) BPF_FUNC_skb_vlan_pop;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
^ permalink raw reply related
* [bpf-next PATCH 1/3] net: fix generic XDP to handle if eth header was mangled
From: Jesper Dangaard Brouer @ 2018-09-25 14:25 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer; +Cc: Daniel Borkmann, yoel, Alexei Starovoitov
In-Reply-To: <153788541527.7040.8722029216689268562.stgit@firesoul>
XDP can modify (and resize) the Ethernet header in the packet.
There is a bug in generic-XDP, because skb->protocol and skb->pkt_type
are setup before reaching (netif_receive_)generic_xdp.
This bug was hit when XDP were popping VLAN headers (changing
eth->h_proto), as skb->protocol still contains VLAN-indication
(ETH_P_8021Q) causing invocation of skb_vlan_untag(skb), which corrupt
the packet (basically popping the VLAN again).
This patch catch if XDP changed eth header in such a way, that SKB
fields needs to be updated.
Fixes: d445516966dc ("net: xdp: support xdp generic on virtual devices")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/dev.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index ca78dc5a79a3..db6d89f536cb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4258,6 +4258,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
struct netdev_rx_queue *rxqueue;
void *orig_data, *orig_data_end;
u32 metalen, act = XDP_DROP;
+ __be16 orig_eth_type;
+ struct ethhdr *eth;
+ bool orig_bcast;
int hlen, off;
u32 mac_len;
@@ -4298,6 +4301,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
xdp->data_hard_start = skb->data - skb_headroom(skb);
orig_data_end = xdp->data_end;
orig_data = xdp->data;
+ eth = (struct ethhdr *)xdp->data;
+ orig_bcast = is_multicast_ether_addr_64bits(eth->h_dest);
+ orig_eth_type = eth->h_proto;
rxqueue = netif_get_rxqueue(skb);
xdp->rxq = &rxqueue->xdp_rxq;
@@ -4321,6 +4327,14 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
}
+ /* check if XDP changed eth hdr such SKB needs update */
+ eth = (struct ethhdr *)xdp->data;
+ if ((orig_eth_type != eth->h_proto) ||
+ (orig_bcast != is_multicast_ether_addr_64bits(eth->h_dest))) {
+ __skb_push(skb, mac_len);
+ skb->protocol = eth_type_trans(skb, skb->dev);
+ }
+
switch (act) {
case XDP_REDIRECT:
case XDP_TX:
^ permalink raw reply related
* [bpf-next PATCH 0/3] bpf/xdp: fix generic-XDP and demonstrate VLAN manipulation
From: Jesper Dangaard Brouer @ 2018-09-25 14:25 UTC (permalink / raw)
To: netdev, Jesper Dangaard Brouer; +Cc: Daniel Borkmann, yoel, Alexei Starovoitov
While implementing PoC building blocks for eBPF code XDP+TC that can
manipulate VLANs headers, I discovered a bug in generic-XDP.
The fix should be backported to stable kernels. Even-though
generic-XDP were introduced in v4.12, I think the bug is not exposed
until v4.14 in the mentined fixes commit.
---
p.s. send from Embedded Recipes 2018 in Paris
Jesper Dangaard Brouer (3):
net: fix generic XDP to handle if eth header was mangled
bpf: make TC vlan bpf_helpers avail to selftests
selftests/bpf: add XDP selftests for modifying and popping VLAN headers
net/core/dev.c | 14 +
tools/testing/selftests/bpf/Makefile | 5
tools/testing/selftests/bpf/bpf_helpers.h | 4
tools/testing/selftests/bpf/test_xdp_vlan.c | 292 ++++++++++++++++++++++++++
tools/testing/selftests/bpf/test_xdp_vlan.sh | 195 +++++++++++++++++
5 files changed, 508 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/test_xdp_vlan.c
create mode 100755 tools/testing/selftests/bpf/test_xdp_vlan.sh
^ permalink raw reply
* Re: [PATCH net 00/15] netpoll: avoid capture effects for NAPI drivers
From: Eric Dumazet @ 2018-09-25 14:20 UTC (permalink / raw)
To: michael.chan
Cc: songliubraving, David Miller, netdev, Ariel Elior, Eric Dumazet,
Tariq Toukan, Saeed Mahameed, Jeff Kirsher, jakub.kicinski,
Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <CACKFLinRzz8h+Q12uTU6cORf9f8-hRTTAT2vi+GQskPM9MT55A@mail.gmail.com>
On Tue, Sep 25, 2018 at 7:02 AM Michael Chan <michael.chan@broadcom.com> wrote:
>
> On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubraving@fb.com> wrote:
> >
> >
> >
> > > On Sep 24, 2018, at 2:05 PM, Eric Dumazet <edumazet@google.com> wrote:
> > >
> > >>
> > >> Interesting, maybe a bnxt specific issue.
> > >>
> > >> It seems their model is to process TX/RX notification in the same queue,
> > >> they throw away RX events if budget == 0
> > >>
> > >> It means commit e7b9569102995ebc26821789628eef45bd9840d8 is wrong and
> > >> must be reverted.
> > >>
> > >> Otherwise, we have a possibility of blocking a queue under netpoll pressure.
> > >
> > > Hmm, actually a revert might not be enough, since code at lines 2030-2031
> > > would fire and we might not call napi_complete_done() anyway.
> > >
> > > Unfortunately this driver logic is quite complex.
> > >
> > > Could you test on other NIC eventually ?
> > >
> >
> > It actually runs OK on ixgbe.
> >
> > @Michael, could you please help us with this?
> >
> I've taken a quick look using today's net tree plus Eric's
> poll_one_napi() patch. The problem I'm seeing is that netpoll calls
> bnxt_poll() with budget 0. And since work_done >= budget of 0, we
> return without calling napi_complete_done() and without arming the
> interrupt. netpoll doesn't always call us back until we call
> napi_complete_done(), right? So I think if there are in-flight TX
> completions, we'll miss those.
That's the whole point of netpoll :
We drain the TX queues, without interrupts being involved at all,
by calling ->napi() with a zero budget.
napi_complete(), even if called from ->napi() while budget was zero,
should do nothing but return early.
budget==0 means that ->napi() should process all TX completions.
So it looks like bnxt has a bug, that is showing up after the latest
poll_one_napi() patch.
This latest patch is needed otherwise the cpu attempting the
netpoll-TX-drain might drain nothing at all,
since it does not anymore call ndo_poll_controller() that was grabbing
SCHED bits on all queues (napi_schedule() like calls)
^ permalink raw reply
* Re: [PATCH net-next v6 02/23] zinc: introduce minimal cryptography library
From: Joe Perches @ 2018-09-25 20:21 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <CAHmME9peVqXDq7hmFZ+4cus+Y97NHS-TyBKF9GkdLvixdmq6hw@mail.gmail.com>
On Tue, 2018-09-25 at 22:12 +0200, Jason A. Donenfeld wrote:
> Hi Joe,
>
> On Tue, Sep 25, 2018 at 10:05 PM Joe Perches <joe@perches.com> wrote:
> > As far as I can tell, zinc doesn't use pr_err, just
> > pr_info
>
> Yes, pr_info, not pr_err. Apologies for my imprecision. But the
> distinction does not matter at all, since they both use pr_fmt in
> exactly the same way, which is sufficient for the purposes of this
> discussion.
>
> > > why can't I just do this from the makefile, since
> > > I want that same pr_fmt to copy the whole directory?
> >
> > Ideally all of those
> >
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > uses will be removed one day soon and the default
> > will change.
> >
> > https://lore.kernel.org/patchwork/cover/904507/
> >
> > This slightly odd use might complicate that.
>
> Oh, that's good to see. In this case, I'm prefixing all of them with
> zinc: instead of zinc_chacha20: (the modname), but I wouldn't object
> to that changing to KBUILD_MODNAME if your patch lands. IOW, once your
> patch lands, I'm happy to revisit this discussion and fall back to
> using the sane defaults you're in the process of setting up for
> everyone.
I'd still prefer as all these are effectively
debugging output that you convert the pr_info
uses pr_debug and so avoid using pr_fmt altogether.
^ permalink raw reply
* Editing for 4
From: Heather @ 2018-09-25 11:18 UTC (permalink / raw)
To: netdev
Want editing for your photos? We can help you for this.
We can do cutting out and retouching for your ecommerce photos, jewelry
photos retouching,
portrait photos or wedding photos.
Turnaround time is fast
Send us one photo, we wil do testing for you.
Thanks,
Heather
^ permalink raw reply
* Re: [PATCH v2 net-next 0/9] bnxt_en: devlink param updates
From: Jakub Kicinski @ 2018-09-25 14:06 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: David Miller, michael.chan@broadcom.com, Netdev
In-Reply-To: <CAACQVJoR1LKyZmeNbRmjOL6w5iKfE2cnshRTs92r1zCXjiS_pg@mail.gmail.com>
On Tue, 25 Sep 2018 09:44:41 +0530, Vasundhara Volam wrote:
> On Mon, Sep 24, 2018 at 10:11 PM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
> >
> > On Mon, 24 Sep 2018 10:46:12 +0530, Vasundhara Volam wrote:
> > > This patchset adds support for 3 generic and 1 driver-specific devlink
> > > parameters. Add documentation for these configuration parameters.
> > >
> > > Also, this patchset adds support to return proper error code if
> > > HWRM_NVM_GET/SET_VARIABLE commands return error code
> > > HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.
> > >
> > > v1->v2:
> > > -Remove hw_tc_offload parameter.
> > > -Update all patches with Cc of MAINTAINERS.
> > > -Add more description in commit message for device specific parameter.
> > > -Add a new Documentation/networking/devlink-params.txt with some
> > > generic devlink parameters information.
> > > -Add a new Documentation/networking/devlink-params-bnxt_en.txt with devlink
> > > parameters information that are supported by bnxt_en driver.
> > >
> > > Vasundhara Volam (9):
> > > devlink: Add generic parameter ignore_ari
> > > devlink: Add generic parameter msix_vec_per_pf_max
> > > devlink: Add generic parameter msix_vec_per_pf_min
> >
> > Nobody agreed with me that we need structure the PCIe bits better so
> > I'll let go...
> >
> > > bnxt_en: Use ignore_ari devlink parameter
> > > bnxt_en: return proper error when FW returns
> > > HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
> > > bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink
> > > params.
> > > bnxt_en: Add a driver specific gre_ver_check devlink parameter.
> >
> > This looks like configuring forwarding rules with devlink, but again,
> Do you think, this parameter should be made generic?
By no means.
> > I won't object if I'm the only one who finds this inappropriate.
> >
> > You should CC people who gave you feedback on the previous version.
> Sorry, I will add in the next version of the patchset. Thanks.
> >
> > > devlink: Add Documentation/networking/devlink-params.txt
> > > devlink: Add Documentation/networking/devlink-params-bnxt.txt
^ permalink raw reply
* Re: [PATCH v2 net-next 9/9] devlink: Add Documentation/networking/devlink-params-bnxt.txt
From: Jakub Kicinski @ 2018-09-25 14:05 UTC (permalink / raw)
To: Vasundhara Volam; +Cc: David Miller, michael.chan@broadcom.com, Netdev
In-Reply-To: <CAACQVJpxp=C8TdiAbOZCKhZ=Pgo-kX8aD148pYMM2613GdEQjQ@mail.gmail.com>
On Tue, 25 Sep 2018 09:37:20 +0530, Vasundhara Volam wrote:
> > Why duplicate the description of the generic parameters?
> Not all generic parameters are used by all drivers. So, I want to add
> information about
> type and configuration mode about generic parameters used by bnxt_en driver.
> I can remove description part keeping type and configuration mode, if
> it looks duplication.
That'd be better.
^ permalink raw reply
* Re: [PATCH net-next v6 02/23] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-25 20:12 UTC (permalink / raw)
To: Joe Perches
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <748429eee2fc77afd01630c24bd2cf66a545aec5.camel@perches.com>
Hi Joe,
On Tue, Sep 25, 2018 at 10:05 PM Joe Perches <joe@perches.com> wrote:
> As far as I can tell, zinc doesn't use pr_err, just
> pr_info
Yes, pr_info, not pr_err. Apologies for my imprecision. But the
distinction does not matter at all, since they both use pr_fmt in
exactly the same way, which is sufficient for the purposes of this
discussion.
> > why can't I just do this from the makefile, since
> > I want that same pr_fmt to copy the whole directory?
>
> Ideally all of those
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> uses will be removed one day soon and the default
> will change.
>
> https://lore.kernel.org/patchwork/cover/904507/
>
> This slightly odd use might complicate that.
Oh, that's good to see. In this case, I'm prefixing all of them with
zinc: instead of zinc_chacha20: (the modname), but I wouldn't object
to that changing to KBUILD_MODNAME if your patch lands. IOW, once your
patch lands, I'm happy to revisit this discussion and fall back to
using the sane defaults you're in the process of setting up for
everyone.
Jason
^ permalink raw reply
* Re: [PATCH net 00/15] netpoll: avoid capture effects for NAPI drivers
From: Michael Chan @ 2018-09-25 14:02 UTC (permalink / raw)
To: Song Liu
Cc: Eric Dumazet, David Miller, Netdev, Ariel Elior, Eric Dumazet,
tariqt, Saeed Mahameed, jeffrey.t.kirsher, Jakub Kicinski,
j.vosburgh, vfalico, Andy Gospodarek
In-Reply-To: <ACD08072-B5A0-4A5D-8E02-30E5F763124D@fb.com>
On Mon, Sep 24, 2018 at 2:18 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Sep 24, 2018, at 2:05 PM, Eric Dumazet <edumazet@google.com> wrote:
> >
> >>
> >> Interesting, maybe a bnxt specific issue.
> >>
> >> It seems their model is to process TX/RX notification in the same queue,
> >> they throw away RX events if budget == 0
> >>
> >> It means commit e7b9569102995ebc26821789628eef45bd9840d8 is wrong and
> >> must be reverted.
> >>
> >> Otherwise, we have a possibility of blocking a queue under netpoll pressure.
> >
> > Hmm, actually a revert might not be enough, since code at lines 2030-2031
> > would fire and we might not call napi_complete_done() anyway.
> >
> > Unfortunately this driver logic is quite complex.
> >
> > Could you test on other NIC eventually ?
> >
>
> It actually runs OK on ixgbe.
>
> @Michael, could you please help us with this?
>
I've taken a quick look using today's net tree plus Eric's
poll_one_napi() patch. The problem I'm seeing is that netpoll calls
bnxt_poll() with budget 0. And since work_done >= budget of 0, we
return without calling napi_complete_done() and without arming the
interrupt. netpoll doesn't always call us back until we call
napi_complete_done(), right? So I think if there are in-flight TX
completions, we'll miss those.
^ permalink raw reply
* Re: [PATCH net-next v6 02/23] zinc: introduce minimal cryptography library
From: Joe Perches @ 2018-09-25 20:05 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <CAHmME9qLzFHPm2aaEKOmjBA95-c3CZuT+kQiuksLRRWQ_ad2sw@mail.gmail.com>
On Tue, 2018-09-25 at 21:43 +0200, Jason A. Donenfeld wrote:
> On Tue, Sep 25, 2018 at 8:33 PM Joe Perches <joe@perches.com> wrote:
> > I think the -Dpr_fmt is especially odd and not
> > really acceptable as it not used anywhere else
> > in the kernel.
>
> There are about 2000 cases in the kernel of the same '#define
> pr_fmt...' being pasted into the top of the file, which seems a bit
> cumbersome. Rather than having to paste that into each and every file
> that I pr_err from,
As far as I can tell, zinc doesn't use pr_err, just
pr_info in all the selftest blocks which are only
used when DEBUG is #defined.
Perhaps all the pr_info uses should use pr_debug
instead as well as dynamic debugging which already
optionally adds KBUILD_MODINFO.
> why can't I just do this from the makefile, since
> I want that same pr_fmt to copy the whole directory?
Ideally all of those
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
uses will be removed one day soon and the default
will change.
https://lore.kernel.org/patchwork/cover/904507/
This slightly odd use might complicate that.
^ permalink raw reply
* Re: [PATCH net-next v6 02/23] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-25 20:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Joe Perches, LKML, Netdev, Linux Crypto Mailing List,
David Miller, Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <1BA7DB23-7BB6-46BA-A3E2-D65BC3CAF11C@amacapital.net>
On Tue, Sep 25, 2018 at 10:00 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > On Sep 25, 2018, at 12:43 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> >> On Tue, Sep 25, 2018 at 8:33 PM Joe Perches <joe@perches.com> wrote:
> >> I think the -Dpr_fmt is especially odd and not
> >> really acceptable as it not used anywhere else
> >> in the kernel.
> >
> > There are about 2000 cases in the kernel of the same '#define
> > pr_fmt...' being pasted into the top of the file, which seems a bit
> > cumbersome. Rather than having to paste that into each and every file
> > that I pr_err from, why can't I just do this from the makefile, since
> > I want that same pr_fmt to copy the whole directory?
>
> Because people like to be able to just read the C file to figure out what it does. Or we could adopt the Makefile approach kernel-wide, since this use of it seems reasonable.
It would indeed be nice to see this done tree-wide. I can recall
various random bugs over the year where some dmesg messages are
missing a prefix because the author forgot to copy and paste the thing
to /yet another file/ in the same directory.
^ permalink raw reply
* [net 1/1] tipc: reset bearer if device carrier not ok
From: Jon Maloy @ 2018-09-25 19:56 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion
From: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
If we detect that under lying carrier detects errors and goes down,
we reset the bearer.
Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/bearer.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 418f03d..645c160 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -609,16 +609,18 @@ static int tipc_l2_device_event(struct notifier_block *nb, unsigned long evt,
switch (evt) {
case NETDEV_CHANGE:
- if (netif_carrier_ok(dev))
+ if (netif_carrier_ok(dev) && netif_oper_up(dev)) {
+ test_and_set_bit_lock(0, &b->up);
break;
- /* else: fall through */
- case NETDEV_UP:
- test_and_set_bit_lock(0, &b->up);
- break;
+ }
+ /* fall through */
case NETDEV_GOING_DOWN:
clear_bit_unlock(0, &b->up);
tipc_reset_bearer(net, b);
break;
+ case NETDEV_UP:
+ test_and_set_bit_lock(0, &b->up);
+ break;
case NETDEV_CHANGEMTU:
if (tipc_mtu_bad(dev, 0)) {
bearer_disable(net, b);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next v6 02/23] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-25 19:43 UTC (permalink / raw)
To: Joe Perches
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <2b5f2df2f1278c118a913259a44efdf589719293.camel@perches.com>
On Tue, Sep 25, 2018 at 8:33 PM Joe Perches <joe@perches.com> wrote:
> I think the -Dpr_fmt is especially odd and not
> really acceptable as it not used anywhere else
> in the kernel.
There are about 2000 cases in the kernel of the same '#define
pr_fmt...' being pasted into the top of the file, which seems a bit
cumbersome. Rather than having to paste that into each and every file
that I pr_err from, why can't I just do this from the makefile, since
I want that same pr_fmt to copy the whole directory?
^ permalink raw reply
* Re: r8169 hang on 4.18
From: Gabriel C @ 2018-09-25 19:37 UTC (permalink / raw)
To: hkallweit1; +Cc: odi, LKML, netdev
In-Reply-To: <9fb89abb-917a-44b6-5dff-9c4a0ed1fd6b@gmail.com>
Am Di., 25. Sep. 2018 um 20:44 Uhr schrieb Heiner Kallweit
<hkallweit1@gmail.com>:
> As I said, you could try running 4.18.4 instead of 4.18.8 for a while (if you have this option).
> You faced the hang roughly once per day, so after a few days you should be sure.
I don't think this will fix anything.
See: https://lkml.org/lkml/2018/9/4/174
Probably all broken by 4fd48c4ac0a0 ("r8169: move common
initializations to tp->hw_start").
BR
^ permalink raw reply
* Re: requesting stable backport of BPF security fix (commit dd066823db2ac4e22f721ec85190817b58059a54)
From: Daniel Borkmann @ 2018-09-25 13:27 UTC (permalink / raw)
To: Jann Horn, Network Development, David S. Miller
Cc: Alexei Starovoitov, security
In-Reply-To: <CAG48ez1=zOgmdsuE38HkG73EA4en+5QOTLTMZMe+PGcthhwt8g@mail.gmail.com>
On 09/25/2018 02:46 PM, Jann Horn wrote:
> Hi!
>
> Per the policy at Documentation/networking/netdev-FAQ.rst, I'm sending
> this to netdev@ and davem, rather than stable@; with a CC to security@
> because I believe that this is a security process issue.
>
> Upstream commit dd066823db2ac4e22f721ec85190817b58059a54
> ("bpf/verifier: disallow pointer subtraction") fixes a security bug
> (kernel pointer leak to unprivileged userspace). The fix has been in
> Linus' tree since about a week ago, but the patch still doesn't appear
> in Greg's linux-4.18.y linux-stable-rc repo, in Greg's 4.18
> stable-queue, or in davem's stable queue at
> http://patchwork.ozlabs.org/bundle/davem/stable/?state=* .
>
> Please queue it up for backporting.
Done & flushed out now, sorry for the delay.
Thanks,
Daniel
^ permalink raw reply
* Re: netlink: 16 bytes leftover after parsing attributes in process `ip'.
From: Stephen Hemminger @ 2018-09-25 13:16 UTC (permalink / raw)
To: Christian Brauner; +Cc: David Ahern, netdev@vger.kernel.org, David Miller
In-Reply-To: <CAHrFyr55vt78akhC+WE5maDPLzmurUeB6wT-DmHcCmnAfPKrgw@mail.gmail.com>
On Tue, 25 Sep 2018 14:34:08 +0200
Christian Brauner <christian@brauner.io> wrote:
> On Tue, Sep 25, 2018, 14:07 Stephen Hemminger <stephen@networkplumber.org>
> wrote:
>
> > On Tue, 25 Sep 2018 11:49:10 +0200
> > Christian Brauner <christian@brauner.io> wrote:
> >
> > > On Mon, Sep 24, 2018 at 09:19:06PM -0600, David Ahern wrote:
> > > > On top of net-next I am see a dmesg error:
> > > >
> > > > netlink: 16 bytes leftover after parsing attributes in process `ip'.
> > > >
> > > > I traced it to address lists and commit:
> > > >
> > > > commit 6ecf4c37eb3e89b0832c9616089a5cdca3747da7
> > > > Author: Christian Brauner <christian@brauner.io>
> > > > Date: Tue Sep 4 21:53:50 2018 +0200
> > > >
> > > > ipv6: enable IFA_TARGET_NETNSID for RTM_GETADDR
> > > >
> > > > Per the commit you are trying to guess whether the ancillary header is
> > > > an ifinfomsg or a ifaddrmsg. I am guessing you are guessing wrong.
> > :-)
> > >
> > > Well, I currently don't guess at all. :) I'm parsing with struct
> > > ifaddrmsg as assumed header size but ignore parsing errors when that
> > > fails. You don't get the niceties of the new property if you don't pack
> >
> > There are legacy parts of netlink interface with kernel.
> > The ABI has evolved over time but some old parts are stuck in the past.
> >
> >
> > > > I don't have time to take this to ground, but address listing is not
> > the
> > > > only area subject to iproute2's SNAFU of infomsg everywhere on dumps. I
> > > > have thought about this for route dumps, but its solution does not work
> > > > here. You'll need to find something because the current warning on
> > every
> > > > address dump is not acceptable.
> > >
> > > Two points before I propose a migitation:
> > >
> > > 1. The burded of seeing pr_warn_ratelimited() messages in dmesg when
> > > userspace is doing something wrong is imho justifiable.
> > > Actually, I would argue that we should not hide the problem from
> > > userspace at all. The rate-limited (so no logging DOS afaict) warning
> > > messages are a perfect indicator that a tool is doing something wrong
> > > *without* introducing any regressions.
> > > The rtnetlink manpage clearly indicates that ifaddrmsg is supposed to
> > > be used too. Additionally, userspace stuffs an ifinfomsg in there but
> > > expects to receive ifaddrmsg. They should be warned loudly. :) So I
> > > actually like the warning messages.
> > > 2. Userspace should be fixed. Especially such an important standard tool
> > > as iproute2 that is maintained on git.kernel.org (glibc is already
> > > doing the right.).
> > >
> > > So if people really want to hide this issue as much as we can then we
> > > can play the guessing game. I could send a patch that roughly does the
> > > following:
> > >
> > > if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg))
> > > guessed_header_len = sizeof(struct ifaddrmsg);
> > > else
> > > guessed_header_len = sizeof(struct ifinfomsg);
> > >
> > > This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16.
> > > The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID.
> > > This propert is a __s32 which should bring the message up to 12 bytes
> > > (not sure about alignment requiremnts and where we might wend up ten)
> > > which is still less than the 16 bytes without that property from
> > > ifinfomsg. That's a hacky hacky hack-hack and will likely work but will
> > > break when ifaddrmsg grows a new member or we introduce another property
> > > that is valid in RTM_GETADDR requests. It also will not work cleanly
> > > when users stuff additional properties in there that are valid for the
> > > address family but are not used int RTM_GETADDR requests.
> > >
> > > I would like to hear what other people and davem think we should do.
> > > Patch it away or print the warning.
> > >
> > > Christian
> >
> > You can't break old programs. That is one of the rules of kernel.
> > Therefore, please either revert the kernel change or put the new attribute
> > in a place where old versions do not cause problem.
> >
>
> Sorry, there's a misunderstanding here. The code doesn't regress anything.
> The patch was written in a backward compatible way. The only thing it
> causes are rate-limited logging messages when the wrong struct is passed.
> But the request still succeeds. The issue is with the logging afaict.
>
> Christian
That still means enterprise distributions that use the current kernel will
get customer complaints. You need to remove the warning.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 00/10] bpf: per-cpu cgroup local storage
From: Song Liu @ 2018-09-25 19:05 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Kernel Team
In-Reply-To: <20180925152114.13537-1-guro@fb.com>
> On Sep 25, 2018, at 8:21 AM, Roman Gushchin <guro@fb.com> wrote:
>
> This patchset implements per-cpu cgroup local storage and provides
> an example how per-cpu and shared cgroup local storage can be used
> for efficient accounting of network traffic.
>
> v2->v1:
> 1) added a selftest implementing network counters
> 2) added a missing free() in cgroup local storage selftest
>
> Roman Gushchin (10):
> bpf: extend cgroup bpf core to allow multiple cgroup storage types
> bpf: rework cgroup storage pointer passing
> bpf: introduce per-cpu cgroup local storage
> bpf: don't allow create maps of per-cpu cgroup local storages
> bpf: sync include/uapi/linux/bpf.h to tools/include/uapi/linux/bpf.h
> bpftool: add support for PERCPU_CGROUP_STORAGE maps
> selftests/bpf: add verifier per-cpu cgroup storage tests
> selftests/bpf: extend the storage test to test per-cpu cgroup storage
> samples/bpf: extend test_cgrp2_attach2 test to use per-cpu cgroup
> storage
> selftests/bpf: cgroup local storage-based network counters
>
> include/linux/bpf-cgroup.h | 55 ++++--
> include/linux/bpf.h | 12 +-
> include/linux/bpf_types.h | 1 +
> include/uapi/linux/bpf.h | 1 +
> kernel/bpf/cgroup.c | 74 +++++---
> kernel/bpf/helpers.c | 25 ++-
> kernel/bpf/local_storage.c | 167 +++++++++++++++---
> kernel/bpf/map_in_map.c | 3 +-
> kernel/bpf/syscall.c | 20 ++-
> kernel/bpf/verifier.c | 23 ++-
> net/bpf/test_run.c | 20 ++-
> samples/bpf/test_cgrp2_attach2.c | 19 +-
> tools/bpf/bpftool/map.c | 4 +-
> tools/include/uapi/linux/bpf.h | 1 +
> tools/testing/selftests/bpf/Makefile | 6 +-
> tools/testing/selftests/bpf/netcnt_common.h | 23 +++
> tools/testing/selftests/bpf/netcnt_prog.c | 71 ++++++++
> .../selftests/bpf/test_cgroup_storage.c | 60 ++++++-
> tools/testing/selftests/bpf/test_netcnt.c | 153 ++++++++++++++++
> tools/testing/selftests/bpf/test_verifier.c | 139 ++++++++++++++-
> 20 files changed, 778 insertions(+), 99 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/netcnt_common.h
> create mode 100644 tools/testing/selftests/bpf/netcnt_prog.c
> create mode 100644 tools/testing/selftests/bpf/test_netcnt.c
>
> --
> 2.17.1
>
For 04/10 to 10/10:
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply
* [PATCH iproute2 1/1] DEBUG: Fix make check when need build generate_nlmsg
From: Petr Vorel @ 2018-09-25 12:49 UTC (permalink / raw)
To: netdev; +Cc: Petr Vorel, Stephen Hemminger
make check from top level Makefile defines several flags which break
building generate_nlmsg:
$ make check
make -C tools
gcc -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wold-style-definition -Wformat=2 -O2 -I../include -I../include/uapi -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\" -DNETNS_ETC_DIR=\"/etc/netns\" -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS -DHAVE_SELINUX -DHAVE_ELF -DHAVE_LIBMNL -I/usr/include/libmnl -DNEED_STRLCPY -DHAVE_LIBCAP ../lib/libutil.a ../lib/libnetlink.a -lselinux -lelf -lmnl -lcap -I../../include -include../../include/uapi/linux/netlink.h -o generate_nlmsg generate_nlmsg.c ../../lib/libnetlink.c -lmnl
gcc: error: ../lib/libutil.a: No such file or directory
gcc: error: ../lib/libnetlink.a: No such file or directory
make[2]: *** [Makefile:5: generate_nlmsg] Error 1
make[1]: *** [Makefile:40: generate_nlmsg] Error 2
To fix it reset CFLAGS in sub Makefile and remove LDLIBS entirely (as
required -lmnl flag was specified in 5dc2204c ("testsuite: add libmnl").
Fixes: 8804a8c0 ("Makefile: Add check target")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi Stephen,
I'm sorry for this regression.
Kind regards,
Petr
---
testsuite/tools/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/testsuite/tools/Makefile b/testsuite/tools/Makefile
index 7d53d226..e1d9bfef 100644
--- a/testsuite/tools/Makefile
+++ b/testsuite/tools/Makefile
@@ -1,8 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
+CFLAGS=
include ../../config.mk
generate_nlmsg: generate_nlmsg.c ../../lib/libnetlink.c
- $(CC) $(CPPFLAGS) $(CFLAGS) $(LDLIBS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
+ $(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -I../../include -include../../include/uapi/linux/netlink.h -o $@ $^ -lmnl
clean:
rm -f generate_nlmsg
--
2.19.0
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 03/10] bpf: introduce per-cpu cgroup local storage
From: Song Liu @ 2018-09-25 18:54 UTC (permalink / raw)
To: Roman Gushchin
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Kernel Team,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180925152114.13537-4-guro@fb.com>
> On Sep 25, 2018, at 8:21 AM, Roman Gushchin <guro@fb.com> wrote:
>
> This commit introduced per-cpu cgroup local storage.
>
> Per-cpu cgroup local storage is very similar to simple cgroup storage
> (let's call it shared), except all the data is per-cpu.
>
> The main goal of per-cpu variant is to implement super fast
> counters (e.g. packet counters), which don't require neither
> lookups, neither atomic operations.
>
> From userspace's point of view, accessing a per-cpu cgroup storage
> is similar to other per-cpu map types (e.g. per-cpu hashmaps and
> arrays).
>
> Writing to a per-cpu cgroup storage is not atomic, but is performed
> by copying longs, so some minimal atomicity is here, exactly
> as with other per-cpu maps.
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> ---
> include/linux/bpf-cgroup.h | 20 ++++-
> include/linux/bpf.h | 1 +
> include/linux/bpf_types.h | 1 +
> include/uapi/linux/bpf.h | 1 +
> kernel/bpf/helpers.c | 8 +-
> kernel/bpf/local_storage.c | 148 ++++++++++++++++++++++++++++++++-----
> kernel/bpf/syscall.c | 11 ++-
> kernel/bpf/verifier.c | 15 +++-
> 8 files changed, 177 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 7e0c9a1d48b7..9bd907657f9b 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -37,7 +37,10 @@ struct bpf_storage_buffer {
> };
>
> struct bpf_cgroup_storage {
> - struct bpf_storage_buffer *buf;
> + union {
> + struct bpf_storage_buffer *buf;
> + char __percpu *percpu_buf;
"char *" here looks weird. Did you mean to use "void *"?
> + };
> struct bpf_cgroup_storage_map *map;
> struct bpf_cgroup_storage_key key;
> struct list_head list;
> @@ -109,6 +112,9 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> static inline enum bpf_cgroup_storage_type cgroup_storage_type(
> struct bpf_map *map)
> {
> + if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> + return BPF_CGROUP_STORAGE_PERCPU;
> +
> return BPF_CGROUP_STORAGE_SHARED;
> }
>
> @@ -131,6 +137,10 @@ void bpf_cgroup_storage_unlink(struct bpf_cgroup_storage *storage);
> int bpf_cgroup_storage_assign(struct bpf_prog *prog, struct bpf_map *map);
> void bpf_cgroup_storage_release(struct bpf_prog *prog, struct bpf_map *map);
>
> +int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key, void *value);
> +int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
> + void *value, u64 flags);
> +
> /* Wrappers for __cgroup_bpf_run_filter_skb() guarded by cgroup_bpf_enabled. */
> #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk, skb) \
> ({ \
> @@ -285,6 +295,14 @@ static inline struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(
> struct bpf_prog *prog, enum bpf_cgroup_storage_type stype) { return 0; }
> static inline void bpf_cgroup_storage_free(
> struct bpf_cgroup_storage *storage) {}
> +static inline int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key,
> + void *value) {
> + return 0;
> +}
> +static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
> + void *key, void *value, u64 flags) {
> + return 0;
> +}
>
> #define cgroup_bpf_enabled (0)
> #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b457fbe7b70b..018299a595c8 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -274,6 +274,7 @@ struct bpf_prog_offload {
>
> enum bpf_cgroup_storage_type {
> BPF_CGROUP_STORAGE_SHARED,
> + BPF_CGROUP_STORAGE_PERCPU,
> __BPF_CGROUP_STORAGE_MAX
> };
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index c9bd6fb765b0..5432f4c9f50e 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -43,6 +43,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_CGROUP_ARRAY, cgroup_array_map_ops)
> #endif
> #ifdef CONFIG_CGROUP_BPF
> BPF_MAP_TYPE(BPF_MAP_TYPE_CGROUP_STORAGE, cgroup_storage_map_ops)
> +BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE, cgroup_storage_map_ops)
> #endif
> BPF_MAP_TYPE(BPF_MAP_TYPE_HASH, htab_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_HASH, htab_percpu_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index aa5ccd2385ed..e2070d819e04 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -127,6 +127,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_SOCKHASH,
> BPF_MAP_TYPE_CGROUP_STORAGE,
> BPF_MAP_TYPE_REUSEPORT_SOCKARRAY,
> + BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
> };
>
> enum bpf_prog_type {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e42f8789b7ea..1f21ef1c4ad3 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -206,10 +206,16 @@ BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
> */
> enum bpf_cgroup_storage_type stype = cgroup_storage_type(map);
> struct bpf_cgroup_storage *storage;
> + void *ptr = NULL;
Not necessary to initialize to NULL.
>
> storage = this_cpu_read(bpf_cgroup_storage[stype]);
>
> - return (unsigned long)&READ_ONCE(storage->buf)->data[0];
> + if (stype == BPF_CGROUP_STORAGE_SHARED)
> + ptr = &READ_ONCE(storage->buf)->data[0];
> + else
> + ptr = this_cpu_ptr(storage->percpu_buf);
> +
> + return (unsigned long)ptr;
> }
>
> const struct bpf_func_proto bpf_get_local_storage_proto = {
> diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
> index 6742292fb39e..d991355b5b46 100644
> --- a/kernel/bpf/local_storage.c
> +++ b/kernel/bpf/local_storage.c
> @@ -152,6 +152,71 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *_key,
> return 0;
> }
>
> +int bpf_percpu_cgroup_storage_copy(struct bpf_map *_map, void *_key,
> + void *value)
> +{
> + struct bpf_cgroup_storage_map *map = map_to_storage(_map);
> + struct bpf_cgroup_storage_key *key = _key;
> + struct bpf_cgroup_storage *storage;
> + int cpu, off = 0;
> + u32 size;
> +
> + rcu_read_lock();
> + storage = cgroup_storage_lookup(map, key, false);
> + if (!storage) {
> + rcu_read_unlock();
> + return -ENOENT;
> + }
> +
> + /* per_cpu areas are zero-filled and bpf programs can only
> + * access 'value_size' of them, so copying rounded areas
> + * will not leak any kernel data
> + */
> + size = round_up(_map->value_size, 8);
> + for_each_possible_cpu(cpu) {
> + bpf_long_memcpy(value + off,
> + per_cpu_ptr(storage->percpu_buf, cpu), size);
> + off += size;
> + }
> + rcu_read_unlock();
> + return 0;
> +}
> +
> +int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *_key,
> + void *value, u64 map_flags)
> +{
> + struct bpf_cgroup_storage_map *map = map_to_storage(_map);
> + struct bpf_cgroup_storage_key *key = _key;
> + struct bpf_cgroup_storage *storage;
> + int cpu, off = 0;
> + u32 size;
> +
> + if (unlikely(map_flags & BPF_EXIST))
> + return -EINVAL;
> +
> + rcu_read_lock();
> + storage = cgroup_storage_lookup(map, key, false);
> + if (!storage) {
> + rcu_read_unlock();
> + return -ENOENT;
> + }
> +
> + /* the user space will provide round_up(value_size, 8) bytes that
> + * will be copied into per-cpu area. bpf programs can only access
> + * value_size of it. During lookup the same extra bytes will be
> + * returned or zeros which were zero-filled by percpu_alloc,
> + * so no kernel data leaks possible
> + */
> + size = round_up(_map->value_size, 8);
> + for_each_possible_cpu(cpu) {
> + bpf_long_memcpy(per_cpu_ptr(storage->percpu_buf, cpu),
> + value + off, size);
> + off += size;
> + }
> + rcu_read_unlock();
> + return 0;
> +}
> +
> static int cgroup_storage_get_next_key(struct bpf_map *_map, void *_key,
> void *_next_key)
> {
> @@ -292,55 +357,98 @@ struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(struct bpf_prog *prog,
> {
> struct bpf_cgroup_storage *storage;
> struct bpf_map *map;
> + gfp_t flags;
> + size_t size;
> u32 pages;
>
> map = prog->aux->cgroup_storage[stype];
> if (!map)
> return NULL;
>
> - pages = round_up(sizeof(struct bpf_cgroup_storage) +
> - sizeof(struct bpf_storage_buffer) +
> - map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
> + if (stype == BPF_CGROUP_STORAGE_SHARED) {
> + size = sizeof(struct bpf_storage_buffer) + map->value_size;
> + pages = round_up(sizeof(struct bpf_cgroup_storage) + size,
> + PAGE_SIZE) >> PAGE_SHIFT;
> + } else {
> + size = map->value_size;
> + pages = round_up(round_up(size, 8) * num_possible_cpus(),
> + PAGE_SIZE) >> PAGE_SHIFT;
> + }
> +
> if (bpf_map_charge_memlock(map, pages))
> return ERR_PTR(-EPERM);
>
> storage = kmalloc_node(sizeof(struct bpf_cgroup_storage),
> __GFP_ZERO | GFP_USER, map->numa_node);
> - if (!storage) {
> - bpf_map_uncharge_memlock(map, pages);
> - return ERR_PTR(-ENOMEM);
> - }
> + if (!storage)
> + goto enomem;
>
> - storage->buf = kmalloc_node(sizeof(struct bpf_storage_buffer) +
> - map->value_size, __GFP_ZERO | GFP_USER,
> - map->numa_node);
> - if (!storage->buf) {
> - bpf_map_uncharge_memlock(map, pages);
> - kfree(storage);
> - return ERR_PTR(-ENOMEM);
> + flags = __GFP_ZERO | GFP_USER;
> +
> + if (stype == BPF_CGROUP_STORAGE_SHARED) {
> + storage->buf = kmalloc_node(size, flags, map->numa_node);
> + if (!storage->buf)
> + goto enomem;
> + } else {
> + storage->percpu_buf = __alloc_percpu_gfp(size, 8, flags);
> + if (!storage->percpu_buf)
> + goto enomem;
> }
>
> storage->map = (struct bpf_cgroup_storage_map *)map;
>
> return storage;
> +
> +enomem:
> + bpf_map_uncharge_memlock(map, pages);
> + kfree(storage);
> + return ERR_PTR(-ENOMEM);
> +}
> +
> +static void free_cgroup_storage_rcu(struct rcu_head *rcu)
Maybe rename as free_shared_cgroup_storage_rcu()?
> +{
> + struct bpf_cgroup_storage *storage =
> + container_of(rcu, struct bpf_cgroup_storage, rcu);
> +
> + kfree(storage->buf);
> + kfree(storage);
> +}
> +
> +static void free_percpu_cgroup_storage_rcu(struct rcu_head *rcu)
> +{
> + struct bpf_cgroup_storage *storage =
> + container_of(rcu, struct bpf_cgroup_storage, rcu);
> +
> + free_percpu(storage->percpu_buf);
> + kfree(storage);
> }
>
> void bpf_cgroup_storage_free(struct bpf_cgroup_storage *storage)
> {
> - u32 pages;
> + enum bpf_cgroup_storage_type stype;
> struct bpf_map *map;
> + u32 pages;
>
> if (!storage)
> return;
>
> map = &storage->map->map;
> - pages = round_up(sizeof(struct bpf_cgroup_storage) +
> - sizeof(struct bpf_storage_buffer) +
> - map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
> + stype = cgroup_storage_type(map);
> + if (stype == BPF_CGROUP_STORAGE_SHARED)
> + pages = round_up(sizeof(struct bpf_cgroup_storage) +
> + sizeof(struct bpf_storage_buffer) +
> + map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
> + else
> + pages = round_up(round_up(map->value_size, 8) *
> + num_possible_cpus(),
> + PAGE_SIZE) >> PAGE_SHIFT;
> +
> bpf_map_uncharge_memlock(map, pages);
>
> - kfree_rcu(storage->buf, rcu);
> - kfree_rcu(storage, rcu);
> + if (stype == BPF_CGROUP_STORAGE_SHARED)
> + call_rcu(&storage->rcu, free_cgroup_storage_rcu);
> + else
> + call_rcu(&storage->rcu, free_percpu_cgroup_storage_rcu);
> }
>
> void bpf_cgroup_storage_link(struct bpf_cgroup_storage *storage,
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8c91d2b41b1e..5742df21598c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -686,7 +686,8 @@ static int map_lookup_elem(union bpf_attr *attr)
>
> if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
> map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> - map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
> + map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> + map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> value_size = round_up(map->value_size, 8) * num_possible_cpus();
> else if (IS_FD_MAP(map))
> value_size = sizeof(u32);
> @@ -705,6 +706,8 @@ static int map_lookup_elem(union bpf_attr *attr)
> err = bpf_percpu_hash_copy(map, key, value);
> } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
> err = bpf_percpu_array_copy(map, key, value);
> + } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
> + err = bpf_percpu_cgroup_storage_copy(map, key, value);
> } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
> err = bpf_stackmap_copy(map, key, value);
> } else if (IS_FD_ARRAY(map)) {
> @@ -774,7 +777,8 @@ static int map_update_elem(union bpf_attr *attr)
>
> if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
> map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> - map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
> + map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> + map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> value_size = round_up(map->value_size, 8) * num_possible_cpus();
> else
> value_size = map->value_size;
> @@ -809,6 +813,9 @@ static int map_update_elem(union bpf_attr *attr)
> err = bpf_percpu_hash_update(map, key, value, attr->flags);
> } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
> err = bpf_percpu_array_update(map, key, value, attr->flags);
> + } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
> + err = bpf_percpu_cgroup_storage_update(map, key, value,
> + attr->flags);
> } else if (IS_FD_ARRAY(map)) {
> rcu_read_lock();
> err = bpf_fd_array_map_update_elem(map, f.file, key, value,
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e75f36de91d6..d94073deb68a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2074,6 +2074,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> goto error;
> break;
> case BPF_MAP_TYPE_CGROUP_STORAGE:
> + case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
> if (func_id != BPF_FUNC_get_local_storage)
> goto error;
> break;
> @@ -2164,7 +2165,8 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> goto error;
> break;
> case BPF_FUNC_get_local_storage:
> - if (map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE)
> + if (map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE &&
> + map->map_type != BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
> goto error;
> break;
> case BPF_FUNC_sk_select_reuseport:
> @@ -5049,6 +5051,12 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
> return 0;
> }
>
> +static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
> +{
> + return (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
> + map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
> +}
> +
> /* look for pseudo eBPF instructions that access map FDs and
> * replace them with actual map pointers
> */
> @@ -5139,10 +5147,9 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
> }
> env->used_maps[env->used_map_cnt++] = map;
>
> - if (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE &&
> + if (bpf_map_is_cgroup_storage(map) &&
> bpf_cgroup_storage_assign(env->prog, map)) {
> - verbose(env,
> - "only one cgroup storage is allowed\n");
> + verbose(env, "only one cgroup storage of each type is allowed\n");
> fdput(f);
> return -EBUSY;
> }
> --
> 2.17.1
>
^ 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