* Re: [PATCH v2] isdn: hfc_{pci,sx}: Avoid empty body if statements
From: David Miller @ 2018-10-19 0:23 UTC (permalink / raw)
To: natechancellor; +Cc: isdn, netdev, linux-kernel, yamada.masahiro
In-Reply-To: <20181019002117.GA10161@flashbox>
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Thu, 18 Oct 2018 17:21:17 -0700
> Thanks for the review, I went ahead and compiled with the following diff
> on top of v2 and got no warnings from Clang, GCC, or sparse, does this
> seem satisfactory for v3?
Well, one thing I notice.
> @@ -86,7 +86,7 @@ release_io_hfcpci(struct IsdnCardState *cs)
> pci_free_consistent(cs->hw.hfcpci.dev, 0x8000,
> cs->hw.hfcpci.fifos, cs->hw.hfcpci.dma);
> cs->hw.hfcpci.fifos = NULL;
> - iounmap((void *)cs->hw.hfcpci.pci_io);
> + iounmap(cs->hw.hfcpci.pci_io);
> }
Driver uses iounmap().
> @@ -1692,7 +1692,7 @@ setup_hfcpci(struct IsdnCard *card)
> printk(KERN_WARNING "HFC-PCI: No IRQ for PCI card found\n");
> return (0);
> }
> - cs->hw.hfcpci.pci_io = (char *)(unsigned long)dev_hfcpci->resource[1].start;
> + cs->hw.hfcpci.pci_io = (void __iomem *)(unsigned long)dev_hfcpci->resource[1].start;
> printk(KERN_INFO "HiSax: HFC-PCI card manufacturer: %s card name: %s\n", id_list[i].vendor_name, id_list[i].card_name);
But does not use iomap(). You won't need any cast here if it did use
iomap() properly.
Thanks.
^ permalink raw reply
* Re: [PATCH v2] isdn: hfc_{pci,sx}: Avoid empty body if statements
From: Nathan Chancellor @ 2018-10-19 0:21 UTC (permalink / raw)
To: David Miller; +Cc: isdn, netdev, linux-kernel, yamada.masahiro
In-Reply-To: <20181018.154219.1246620018001438422.davem@davemloft.net>
On Thu, Oct 18, 2018 at 03:42:19PM -0700, David Miller wrote:
> From: Nathan Chancellor <natechancellor@gmail.com>
> Date: Wed, 17 Oct 2018 20:49:36 -0700
>
> > @@ -228,8 +228,8 @@ typedef union {
> > } fifo_area;
> >
> >
> > -#define Write_hfc(a, b, c) (*(((u_char *)a->hw.hfcpci.pci_io) + b) = c)
> > -#define Read_hfc(a, b) (*(((u_char *)a->hw.hfcpci.pci_io) + b))
> > +#define Write_hfc(a, b, c) (writeb(c, (a->hw.hfcpci.pci_io) + b))
> > +#define Read_hfc(a, b) (readb((a->hw.hfcpci.pci_io) + b))
>
> This will add new kinds of warnings.
>
> The problem is that readb/writeb/etc. take an __iomem pointer, but pci_io
> is declared as plain "unsigned char *". It should be something like
> "void * __iomem" of similar.
Thanks for the review, I went ahead and compiled with the following diff
on top of v2 and got no warnings from Clang, GCC, or sparse, does this
seem satisfactory for v3?
Nathan
========================================================================
diff --git a/drivers/isdn/hisax/hfc_pci.c b/drivers/isdn/hisax/hfc_pci.c
index 7bcd104e9dfe..3dbaee8c604f 100644
--- a/drivers/isdn/hisax/hfc_pci.c
+++ b/drivers/isdn/hisax/hfc_pci.c
@@ -86,7 +86,7 @@ release_io_hfcpci(struct IsdnCardState *cs)
pci_free_consistent(cs->hw.hfcpci.dev, 0x8000,
cs->hw.hfcpci.fifos, cs->hw.hfcpci.dma);
cs->hw.hfcpci.fifos = NULL;
- iounmap((void *)cs->hw.hfcpci.pci_io);
+ iounmap(cs->hw.hfcpci.pci_io);
}
/********************************************************************************/
@@ -1692,7 +1692,7 @@ setup_hfcpci(struct IsdnCard *card)
printk(KERN_WARNING "HFC-PCI: No IRQ for PCI card found\n");
return (0);
}
- cs->hw.hfcpci.pci_io = (char *)(unsigned long)dev_hfcpci->resource[1].start;
+ cs->hw.hfcpci.pci_io = (void __iomem *)(unsigned long)dev_hfcpci->resource[1].start;
printk(KERN_INFO "HiSax: HFC-PCI card manufacturer: %s card name: %s\n", id_list[i].vendor_name, id_list[i].card_name);
if (!cs->hw.hfcpci.pci_io) {
diff --git a/drivers/isdn/hisax/hisax.h b/drivers/isdn/hisax/hisax.h
index 338d0408b377..40080e06421c 100644
--- a/drivers/isdn/hisax/hisax.h
+++ b/drivers/isdn/hisax/hisax.h
@@ -703,7 +703,7 @@ struct hfcPCI_hw {
unsigned char nt_mode;
int nt_timer;
struct pci_dev *dev;
- unsigned char *pci_io; /* start of PCI IO memory */
+ void __iomem *pci_io; /* start of PCI IO memory */
dma_addr_t dma; /* dma handle for Fifos */
void *fifos; /* FIFO memory */
int last_bfifo_cnt[2]; /* marker saving last b-fifo frame count */
^ permalink raw reply related
* [GIT] Networking
From: David Miller @ 2018-10-19 0:19 UTC (permalink / raw)
To: gregkh; +Cc: akpm, netdev, linux-kernel
1) Fix gro_cells leak in xfrm layer, from Li RongQing.
2) BPF selftests change RLIMIT_MEMLOCK blindly, don't do that.
From Eric Dumazet.
3) AF_XDP calls synchronize_net() under RCU lock, fix from Björn
Töpel.
4) Out of bounds packet access in _decode_session6(), from Alexei
Starovoitov.
5) Several ethtool bugs, where we copy a struct into the kernel
twice and our validations of the values in the first copy can
be invalidated by the second copy due to asynchronous updates
to the memory by the user. From Wenwen Wang.
6) Missing netlink attribute validation in cls_api, from Davide
Caratti.
7) LLC SAP sockets neet to be SOCK_RCU FREE, from Cong Wang.
8) rxrpc operates on wrong kvec, from Yue Haibing.
9) A regression was introduced by the disassosciation of route
neighbour references in rt6_probe(), causing probe for
neighbourless routes to not be properly rate limited. Fix
from Sabrina Dubroca.
10) Unsafe RCU locking in tipc, from Tung Nguyen.
11) Use after free in inet6_mc_check(), from Eric Dumazet.
12) PMTU from icmp packets should update the SCTP transport
pathmtu, from Xin Long.
13) Missing peer put on error in rxrpc, from David Howells.
14) Fix pedit in nfp driver, from Pieter Jansen van Vuuren.
15) Fix overflowing shift statement in qla3xxx driver, from Nathan
Chancellor.
16) Fix Spectre v1 in ptp code, from Gustavo A. R. Silva.
17) udp6_unicast_rcv_skb() interprets udpv6_queue_rcv_skb() return
value in an inverted manner, fix from Paolo Abeni.
18) Fix missed unresolved entries in ipmr dumps, from Nikolay
Aleksandrov.
19) Fix NAPI handling under high load, we can completely miss events
when NAPI has to loop more than one time in a cycle. From Heiner
Kallweit.
Please pull, thanks a lot!
The following changes since commit bab5c80b211035739997ebd361a679fa85b39465:
Merge tag 'armsoc-fixes-4.19' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc (2018-10-12 17:41:27 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
for you to fetch changes up to d4d576f5ab7edcb757bb33e6a5600666a0b1232d:
ip6_tunnel: Fix encapsulation layout (2018-10-18 16:54:40 -0700)
----------------------------------------------------------------
Ake Koomsin (1):
virtio_net: avoid using netif_tx_disable() for serializing tx routine
Alexei Starovoitov (1):
net/xfrm: fix out-of-bounds packet access
Björn Töpel (1):
xsk: do not call synchronize_net() under RCU read lock
Colin Ian King (1):
qed: fix spelling mistake "Ireelevant" -> "Irrelevant"
Cong Wang (1):
llc: set SOCK_RCU_FREE in llc_sap_add_socket()
David Howells (3):
rxrpc: Fix an uninitialised variable
rxrpc: Fix incorrect conditional on IPV6
rxrpc: Fix a missing rxrpc_put_peer() in the error_report handler
David S. Miller (5):
Merge git://git.kernel.org/.../bpf/bpf
Merge tag 'mlx5-fixes-2018-10-10' of git://git.kernel.org/.../saeed/linux
Merge branch 'nfp-fix-pedit-set-action-offloads'
Merge branch 'geneve-vxlan-mtu'
Merge branch 'master' of git://git.kernel.org/.../klassert/ipsec
Davide Caratti (1):
net/sched: cls_api: add missing validation of netlink attributes
Eric Dumazet (2):
bpf: do not blindly change rlimit in reuseport net selftest
ipv6: mcast: fix a use-after-free in inet6_mc_check
Florian Fainelli (1):
net: bcmgenet: Poll internal PHY for GENETv5
Florian Westphal (1):
xfrm: policy: use hlist rcu variants on insert
Fugang Duan (1):
net: fec: don't dump RX FIFO register when not available
Gregory CLEMENT (1):
net: mscc: ocelot: Fix comment in ocelot_vlant_wait_for_completion()
Gustavo A. R. Silva (1):
ptp: fix Spectre v1 vulnerability
Heiner Kallweit (2):
r8169: re-enable MSI-X on RTL8168g
r8169: fix NAPI handling under high load
Huy Nguyen (1):
net/mlx5: Take only bit 24-26 of wqe.pftype_wq for page fault type
Ido Schimmel (1):
mlxsw: core: Fix use-after-free when flashing firmware during init
Jian-Hong Pan (1):
r8169: Enable MSI-X on RTL8106e
Jon Maloy (2):
tipc: initialize broadcast link stale counter correctly
tipc: fix info leak from kernel tipc_event
Li RongQing (1):
xfrm: fix gro_cells leak when remove virtual xfrm interfaces
Marcelo Ricardo Leitner (1):
sctp: fix race on sctp_id2asoc
Nathan Chancellor (1):
net: qla3xxx: Remove overflowing shift statement
Nikolay Aleksandrov (1):
net: ipmr: fix unresolved entry dumps
Paolo Abeni (1):
udp6: fix encap return code for resubmitting
Phil Sutter (1):
net: sched: Fix for duplicate class dump
Pieter Jansen van Vuuren (3):
nfp: flower: fix pedit set actions for multiple partial masks
nfp: flower: fix multiple keys per pedit action
nfp: flower: use offsets provided by pedit instead of index for ipv6
Sabrina Dubroca (1):
ipv6: rate-limit probes for neighbourless routes
Stefano Brivio (3):
geneve, vxlan: Don't check skb_dst() twice
geneve, vxlan: Don't set exceptions if skb->len < mtu
ip6_tunnel: Fix encapsulation layout
Steffen Klassert (1):
MAINTAINERS: Remove net/core/flow.c
Taehee Yoo (1):
net: bpfilter: use get_pid_task instead of pid_task
Talat Batheesh (1):
net/mlx5: Fix memory leak when setting fpga ipsec caps
Tariq Toukan (1):
net/mlx5: WQ, fixes for fragmented WQ buffers API
Tung Nguyen (1):
tipc: fix unsafe rcu locking when accessing publication list
Wenwen Wang (3):
ethtool: fix a missing-check bug
ethtool: fix a privilege escalation bug
net: socket: fix a missing-check bug
Xin Long (4):
sctp: use the pmtu from the icmp packet to update transport pathmtu
sctp: get pr_assoc and pr_stream all status with SCTP_PR_SCTP_ALL instead
sctp: not free the new asoc when sctp_wait_for_connect returns err
sctp: fix the data size calculation in sctp_data_size
YueHaibing (1):
rxrpc: use correct kvec num when sending BUSY response packet
MAINTAINERS | 1 -
drivers/net/ethernet/broadcom/genet/bcmmii.c | 7 +++++--
drivers/net/ethernet/freescale/fec.h | 4 ++++
drivers/net/ethernet/freescale/fec_main.c | 16 ++++++++++++----
drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 12 +++++-------
drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 22 +++++++++++-----------
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/fpga/ipsec.c | 9 ++++-----
drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.h | 5 ++---
drivers/net/ethernet/mellanox/mlx5/core/wq.c | 5 -----
drivers/net/ethernet/mellanox/mlx5/core/wq.h | 11 +++++------
drivers/net/ethernet/mellanox/mlxsw/core.c | 2 ++
drivers/net/ethernet/mellanox/mlxsw/core.h | 4 ++++
drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c | 17 +++++++++++------
drivers/net/ethernet/mscc/ocelot.c | 6 +++---
drivers/net/ethernet/netronome/nfp/flower/action.c | 51 +++++++++++++++++++++++++++++++++------------------
drivers/net/ethernet/qlogic/qed/qed_int.c | 2 +-
drivers/net/ethernet/qlogic/qla3xxx.c | 2 --
drivers/net/ethernet/realtek/r8169.c | 20 +++++---------------
drivers/net/geneve.c | 14 +++-----------
drivers/net/virtio_net.c | 5 ++++-
drivers/net/vxlan.c | 12 ++----------
drivers/ptp/ptp_chardev.c | 4 ++++
include/linux/mlx5/driver.h | 8 ++++++++
include/net/dst.h | 10 ++++++++++
include/net/ip6_fib.h | 4 ++++
include/net/sctp/sm.h | 2 +-
include/net/sctp/structs.h | 2 ++
include/uapi/linux/sctp.h | 1 +
kernel/bpf/xskmap.c | 10 ++--------
net/bpfilter/bpfilter_kern.c | 6 ++++--
net/core/ethtool.c | 11 +++++++++--
net/ipv4/ipmr_base.c | 2 --
net/ipv6/ip6_tunnel.c | 10 +++++-----
net/ipv6/mcast.c | 16 ++++++++--------
net/ipv6/route.c | 12 ++++++------
net/ipv6/udp.c | 6 ++----
net/ipv6/xfrm6_policy.c | 4 ++--
net/llc/llc_conn.c | 1 +
net/rxrpc/call_accept.c | 2 +-
net/rxrpc/local_object.c | 2 +-
net/rxrpc/output.c | 3 ++-
net/rxrpc/peer_event.c | 1 +
net/sched/cls_api.c | 13 ++++++++-----
net/sched/sch_api.c | 11 ++++++-----
net/sctp/associola.c | 3 ++-
net/sctp/input.c | 1 +
net/sctp/output.c | 6 ++++++
net/sctp/socket.c | 17 +++++++++--------
net/socket.c | 11 ++++++++---
net/tipc/group.c | 1 +
net/tipc/link.c | 1 +
net/tipc/name_distr.c | 4 ++--
net/xdp/xsk.c | 2 ++
net/xfrm/xfrm_interface.c | 3 +++
net/xfrm/xfrm_policy.c | 8 ++++----
tools/testing/selftests/net/reuseport_bpf.c | 13 +++++++++----
57 files changed, 253 insertions(+), 187 deletions(-)
^ permalink raw reply
* Re: [iproute PATCH] rdma: Fix for ineffective check in add_filter()
From: Phil Sutter @ 2018-10-18 16:17 UTC (permalink / raw)
To: David Ahern
Cc: Stephen Hemminger, netdev, Arkadi Sharshevsky, Leon Romanovsky
In-Reply-To: <6b365665-100c-d6e2-56bd-dfac39b55806@gmail.com>
Hi,
On Thu, Oct 18, 2018 at 09:27:47AM -0600, David Ahern wrote:
> On 10/18/18 5:41 AM, Phil Sutter wrote:
> > With 'name' field defined as array in struct filters, it will always
> > contain a value irrespective of whether a name was assigned or not.
> >
> > Fix this by turning the field into a const char pointer.
> >
> > Fixes: 8cd644095842a ("devlink: Add support for devlink resource abstraction")
>
> Stale paste buffer? Seems like the correct tag is
> Fixes: 1174be72d1b4c ("rdma: Add filtering infrastructure")
Oh, indeed. Also the wrong person in Cc: Arkadi is innocent, culprit
patch came from Leon.
Stephen, should I respin or will you correct the Fixes tag?
Thanks, Phil
^ permalink raw reply
* linux-next: build failure after merge of the net-next tree
From: Stephen Rothwell @ 2018-10-19 0:18 UTC (permalink / raw)
To: David Miller, Networking, Doug Ledford, Jason Gunthorpe
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Mark Bloch,
Leon Romanovsky, Paul Blakey, Saeed Mahameed
[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]
Hi all,
After merging the net-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:
drivers/infiniband/hw/mlx5/flow.c: In function 'mlx5_ib_handler_MLX5_IB_METHOD_CREATE_FLOW':
drivers/infiniband/hw/mlx5/flow.c:163:12: error: 'struct mlx5_flow_act' has no member named 'has_flow_tag'; did you mean 'flow_tag'?
flow_act.has_flow_tag = true;
^~~~~~~~~~~~
flow_tag
Caused by commit
d5634fee245f ("net/mlx5: Add a no-append flow insertion mode")
interacting with commit
ba4a41198324 ("RDMA/mlx5: Add support for flow tag to raw create flow")
from the rdma tree.
I have applied the following merge fix patch for today:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 19 Oct 2018 11:10:39 +1100
Subject: [PATCH] net/mlx5: fix up for has_flow_tag changing to a flag
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
drivers/infiniband/hw/mlx5/flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx5/flow.c b/drivers/infiniband/hw/mlx5/flow.c
index e57435cb6d96..f86cdcafdafc 100644
--- a/drivers/infiniband/hw/mlx5/flow.c
+++ b/drivers/infiniband/hw/mlx5/flow.c
@@ -160,7 +160,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(
ret = -EINVAL;
goto err_out;
}
- flow_act.has_flow_tag = true;
+ flow_act.flags |= FLOW_ACT_HAS_TAG;
}
flow_handler = mlx5_ib_raw_fs_rule_add(dev, fs_matcher, &flow_act,
--
2.18.0
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
* [PATCH net-next] tcp: fix TCP_REPAIR xmit queue setup
From: Eric Dumazet @ 2018-10-18 16:12 UTC (permalink / raw)
To: David S . Miller, Neal Cardwell, Soheil Hassas Yeganeh,
Andrey Vagin
Cc: netdev, Eric Dumazet, Eric Dumazet
Andrey reported the following warning triggered while running CRIU tests:
tcp_clean_rtx_queue()
...
last_ackt = tcp_skb_timestamp_us(skb);
WARN_ON_ONCE(last_ackt == 0);
This is caused by 5f6188a8003d ("tcp: do not change tcp_wstamp_ns
in tcp_mstamp_refresh"), as we end up having skbs in retransmit queue
with a zero skb->skb_mstamp_ns field.
We could fix this bug in different ways, like making sure
tp->tcp_wstamp_ns is not zero at socket creation, but as Neal pointed
out, we also do not want that pacing status of a repaired socket
could push tp->tcp_wstamp_ns far ahead in the future.
So we prefer changing tcp_write_xmit() to not call tcp_update_skb_after_send()
and instead do what is requested by TCP_REPAIR logic.
Fixes: 5f6188a8003d ("tcp: do not change tcp_wstamp_ns in tcp_mstamp_refresh")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Andrey Vagin <avagin@openvz.org>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
net/ipv4/tcp_output.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index d212e4cbc68902e873afb4a12b43b467ccd6069b..c07990a35ff3bd9438d32c82863ef207c93bdb9e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2321,18 +2321,19 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
while ((skb = tcp_send_head(sk))) {
unsigned int limit;
- if (tcp_pacing_check(sk))
- break;
-
- tso_segs = tcp_init_tso_segs(skb, mss_now);
- BUG_ON(!tso_segs);
-
if (unlikely(tp->repair) && tp->repair_queue == TCP_SEND_QUEUE) {
- /* "skb_mstamp" is used as a start point for the retransmit timer */
- tcp_update_skb_after_send(sk, skb, tp->tcp_wstamp_ns);
+ /* "skb_mstamp_ns" is used as a start point for the retransmit timer */
+ skb->skb_mstamp_ns = tp->tcp_wstamp_ns = tp->tcp_clock_cache;
+ list_move_tail(&skb->tcp_tsorted_anchor, &tp->tsorted_sent_queue);
goto repair; /* Skip network transmission */
}
+ if (tcp_pacing_check(sk))
+ break;
+
+ tso_segs = tcp_init_tso_segs(skb, mss_now);
+ BUG_ON(!tso_segs);
+
cwnd_quota = tcp_cwnd_test(tp, skb);
if (!cwnd_quota) {
if (push_one == 2)
--
2.19.1.331.ge82ca0e54c-goog
^ permalink raw reply related
* Photos 39
From: Nancy @ 2018-10-18 10:01 UTC (permalink / raw)
To: netdev
We are an image team who can process 400+ images each day.
If you need any image editing service, please let us know.
Image cut out and clipping path, masking.
Such as for ecommerce photos, jewelry photos retouching, beauty and skin
images
and wedding photos.
We give test editing for your photos if you send us some.
Thanks,
Nancy
^ permalink raw reply
* Photos 39
From: Nancy @ 2018-10-18 11:24 UTC (permalink / raw)
To: netdev
We are an image team who can process 400+ images each day.
If you need any image editing service, please let us know.
Image cut out and clipping path, masking.
Such as for ecommerce photos, jewelry photos retouching, beauty and skin
images
and wedding photos.
We give test editing for your photos if you send us some.
Thanks,
Nancy
^ permalink raw reply
* [PATCH v4 bpf-next 0/2] bpf: add cg_skb_is_valid_access
From: Song Liu @ 2018-10-18 16:06 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, Song Liu
Changes v3 -> v4:
1. Fixed crash issue reported by Alexei.
Changes v2 -> v3:
1. Added helper function bpf_compute_and_save_data_pointers() and
bpf_restore_data_pointers().
Changes v1 -> v2:
1. Updated the list of read-only fields, and read-write fields.
2. Added dummy sk to bpf_prog_test_run_skb().
This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access
some __skb_buff data directly.
Song Liu (2):
bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
bpf: add tests for direct packet access from CGROUP_SKB
include/linux/filter.h | 24 +++
kernel/bpf/cgroup.c | 6 +
net/bpf/test_run.c | 7 +
net/core/filter.c | 36 ++++-
tools/testing/selftests/bpf/test_verifier.c | 170 ++++++++++++++++++++
5 files changed, 242 insertions(+), 1 deletion(-)
^ permalink raw reply
* [PATCH v4 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB
From: Song Liu @ 2018-10-18 16:06 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20181018160649.1611530-1-songliubraving@fb.com>
Tests are added to make sure CGROUP_SKB cannot access:
tc_classid, data_meta, flow_keys
and can read and write:
mark, prority, and cb[0-4]
and can read other fields.
To make selftest with skb->sk work, a dummy sk is added in
bpf_prog_test_run_skb().
Signed-off-by: Song Liu <songliubraving@fb.com>
---
net/bpf/test_run.c | 7 +
tools/testing/selftests/bpf/test_verifier.c | 170 ++++++++++++++++++++
2 files changed, 177 insertions(+)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0c423b8cd75c..8dccac305268 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,8 @@
#include <linux/etherdevice.h>
#include <linux/filter.h>
#include <linux/sched/signal.h>
+#include <net/sock.h>
+#include <net/tcp.h>
static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
@@ -115,6 +117,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
u32 retval, duration;
int hh_len = ETH_HLEN;
struct sk_buff *skb;
+ struct sock sk = {0};
void *data;
int ret;
@@ -137,11 +140,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
break;
}
+ sock_net_set(&sk, &init_net);
+ sock_init_data(NULL, &sk);
+
skb = build_skb(data, 0);
if (!skb) {
kfree(data);
return -ENOMEM;
}
+ skb->sk = &sk;
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
__skb_put(skb, size);
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index cf4cd32b6772..5bfba7e8afd7 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4862,6 +4862,176 @@ static struct bpf_test tests[] = {
.result = REJECT,
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
+ {
+ "direct packet read test#1 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, data_end)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, len)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, pkt_type)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, mark)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, mark)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, queue_mapping)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, protocol)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_present)),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_0, BPF_REG_3, 1),
+ BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#2 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_tci)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_proto)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, priority)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, priority)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, ingress_ifindex)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, tc_index)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, hash)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#3 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, cb[4])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_4,
+ offsetof(struct __sk_buff, cb[0])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_5,
+ offsetof(struct __sk_buff, cb[1])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, cb[2])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_7,
+ offsetof(struct __sk_buff, cb[3])),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_8,
+ offsetof(struct __sk_buff, cb[4])),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#4 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, family)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip4)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip4)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_ip6[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, local_ip6[3])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, remote_port)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, local_port)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of tc_classid for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, tc_classid)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of data_meta for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, data_meta)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid access of flow_keys for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, flow_keys)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "invalid write access to napi_id for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_9,
+ offsetof(struct __sk_buff, napi_id)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = REJECT,
+ .errstr = "invalid bpf_context access",
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
{
"valid cgroup storage access",
.insns = {
--
2.17.1
^ permalink raw reply related
* [PATCH v4 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
From: Song Liu @ 2018-10-18 16:06 UTC (permalink / raw)
To: netdev; +Cc: ast, daniel, kernel-team, Song Liu
In-Reply-To: <20181018160649.1611530-1-songliubraving@fb.com>
BPF programs of BPF_PROG_TYPE_CGROUP_SKB need to access headers in the
skb. This patch enables direct access of skb for these programs.
Two helper functions bpf_compute_and_save_data_pointers() and
bpf_restore_data_pointers() are introduced. There are used in
__cgroup_bpf_run_filter_skb(), to compute proper data_end for the
BPF program, and restore original data afterwards.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
include/linux/filter.h | 24 ++++++++++++++++++++++++
kernel/bpf/cgroup.c | 6 ++++++
net/core/filter.c | 36 +++++++++++++++++++++++++++++++++++-
3 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 5771874bc01e..96b3ee7f14c9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -548,6 +548,30 @@ static inline void bpf_compute_data_pointers(struct sk_buff *skb)
cb->data_end = skb->data + skb_headlen(skb);
}
+/* Similar to bpf_compute_data_pointers(), except that save orginal
+ * data in cb->data and cb->meta_data for restore.
+ */
+static inline void bpf_compute_and_save_data_pointers(
+ struct sk_buff *skb, void *saved_pointers[2])
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ saved_pointers[0] = cb->data_meta;
+ saved_pointers[1] = cb->data_end;
+ cb->data_meta = skb->data - skb_metadata_len(skb);
+ cb->data_end = skb->data + skb_headlen(skb);
+}
+
+/* Restore data saved by bpf_compute_data_pointers(). */
+static inline void bpf_restore_data_pointers(
+ struct sk_buff *skb, void *saved_pointers[2])
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ cb->data_meta = saved_pointers[0];
+ cb->data_end = saved_pointers[1];;
+}
+
static inline u8 *bpf_skb_cb(struct sk_buff *skb)
{
/* eBPF programs may read/write skb->cb[] area to transfer meta
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 00f6ed2e4f9a..5f5180104ddc 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -554,6 +554,7 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
unsigned int offset = skb->data - skb_network_header(skb);
struct sock *save_sk;
struct cgroup *cgrp;
+ void *saved_pointers[2];
int ret;
if (!sk || !sk_fullsock(sk))
@@ -566,8 +567,13 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
save_sk = skb->sk;
skb->sk = sk;
__skb_push(skb, offset);
+
+ /* compute pointers for the bpf prog */
+ bpf_compute_and_save_data_pointers(skb, saved_pointers);
+
ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[type], skb,
bpf_prog_run_save_cb);
+ bpf_restore_data_pointers(skb, saved_pointers);
__skb_pull(skb, offset);
skb->sk = save_sk;
return ret == 1 ? 0 : -EPERM;
diff --git a/net/core/filter.c b/net/core/filter.c
index 1a3ac6c46873..e3ca30bd6840 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5346,6 +5346,40 @@ static bool sk_filter_is_valid_access(int off, int size,
return bpf_skb_is_valid_access(off, size, type, prog, info);
}
+static bool cg_skb_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, tc_classid):
+ case bpf_ctx_range(struct __sk_buff, data_meta):
+ case bpf_ctx_range(struct __sk_buff, flow_keys):
+ return false;
+ }
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, mark):
+ case bpf_ctx_range(struct __sk_buff, priority):
+ case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+ break;
+ default:
+ return false;
+ }
+ }
+
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, data):
+ info->reg_type = PTR_TO_PACKET;
+ break;
+ case bpf_ctx_range(struct __sk_buff, data_end):
+ info->reg_type = PTR_TO_PACKET_END;
+ break;
+ }
+
+ return bpf_skb_is_valid_access(off, size, type, prog, info);
+}
+
static bool lwt_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -7038,7 +7072,7 @@ const struct bpf_prog_ops xdp_prog_ops = {
const struct bpf_verifier_ops cg_skb_verifier_ops = {
.get_func_proto = cg_skb_func_proto,
- .is_valid_access = sk_filter_is_valid_access,
+ .is_valid_access = cg_skb_is_valid_access,
.convert_ctx_access = bpf_convert_ctx_access,
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next] net: ethernet: ti: cpsw: don't flush mcast entries while switch promisc mode
From: Grygorii Strashko @ 2018-10-19 0:03 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem; +Cc: linux-omap, netdev, linux-kernel
In-Reply-To: <20181018180006.7065-1-ivan.khoronzhuk@linaro.org>
On 10/18/18 1:00 PM, Ivan Khoronzhuk wrote:
> No need now to flush mcast entries in switch mode while toggling to
> promiscuous mode. It's not needed as vlan reg_mcast = ALL_PORTS
> and mcast/vlan ports = ALL_PORTS, the same happening for vlan
> unreg_mcast, it's set to ALL_PORT_MASK just after calling promisc
> mode routine by calling set allmulti. I suppose main reason to flush
> them is to use unreg_mcast to receive all to host port. Thus, now, all
> mcast packets are received anyway and no reason to flush mcast entries
> unsafely, as they were synced with __dev_mc_sync() previously and are
> not restored. Another way is to _dev_mc_unsync() them, but no need.
User have possibility to add additional mcast entries or edit existing
in switch-mode, which is now done using custom tool. So, Host in promisc
mode will not receive packets for mcast address X if port mask for this
addr set to (ALL_PORTS - HOST_PORT). Am I missing smth?
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>
> Based on net-next/master
> Tasted on am572x EVM and BBB
>
> drivers/net/ethernet/ti/cpsw.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 226be2a56c1f..0e475020a674 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -638,9 +638,6 @@ static void cpsw_set_promiscious(struct net_device *ndev, bool enable)
> } while (time_after(timeout, jiffies));
> cpsw_ale_control_set(ale, 0, ALE_AGEOUT, 1);
>
> - /* Clear all mcast from ALE */
> - cpsw_ale_flush_multicast(ale, ALE_ALL_PORTS, -1);
> -
> /* Flood All Unicast Packets to Host port */
> cpsw_ale_control_set(ale, 0, ALE_P0_UNI_FLOOD, 1);
> dev_dbg(&ndev->dev, "promiscuity enabled\n");
>
--
regards,
-grygorii
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2018-10-18 23:56 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Nikolay Aleksandrov, David Ahern
[-- Attachment #1: Type: text/plain, Size: 3068 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/ipv4/ipmr_base.c
between commit:
eddf016b9104 ("net: ipmr: fix unresolved entry dumps")
from the net tree and commit:
e1cedae1ba6b ("ipmr: Refactor mr_rtm_dumproute")
from the net-next tree.
I fixed it up (I think - see below) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/ipv4/ipmr_base.c
index eab8cd5ec2f5,844806120f44..000000000000
--- a/net/ipv4/ipmr_base.c
+++ b/net/ipv4/ipmr_base.c
@@@ -268,6 -268,83 +268,81 @@@ int mr_fill_mroute(struct mr_table *mrt
}
EXPORT_SYMBOL(mr_fill_mroute);
+ static bool mr_mfc_uses_dev(const struct mr_table *mrt,
+ const struct mr_mfc *c,
+ const struct net_device *dev)
+ {
+ int ct;
+
+ for (ct = c->mfc_un.res.minvif; ct < c->mfc_un.res.maxvif; ct++) {
+ if (VIF_EXISTS(mrt, ct) && c->mfc_un.res.ttls[ct] < 255) {
+ const struct vif_device *vif;
+
+ vif = &mrt->vif_table[ct];
+ if (vif->dev == dev)
+ return true;
+ }
+ }
+ return false;
+ }
+
+ int mr_table_dump(struct mr_table *mrt, struct sk_buff *skb,
+ struct netlink_callback *cb,
+ int (*fill)(struct mr_table *mrt, struct sk_buff *skb,
+ u32 portid, u32 seq, struct mr_mfc *c,
+ int cmd, int flags),
+ spinlock_t *lock, struct fib_dump_filter *filter)
+ {
+ unsigned int e = 0, s_e = cb->args[1];
+ unsigned int flags = NLM_F_MULTI;
+ struct mr_mfc *mfc;
+ int err;
+
+ if (filter->filter_set)
+ flags |= NLM_F_DUMP_FILTERED;
+
+ list_for_each_entry_rcu(mfc, &mrt->mfc_cache_list, list) {
+ if (e < s_e)
+ goto next_entry;
+ if (filter->dev &&
+ !mr_mfc_uses_dev(mrt, mfc, filter->dev))
+ goto next_entry;
+
+ err = fill(mrt, skb, NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq, mfc, RTM_NEWROUTE, flags);
+ if (err < 0)
+ goto out;
+ next_entry:
+ e++;
+ }
- e = 0;
- s_e = 0;
+
+ spin_lock_bh(lock);
+ list_for_each_entry(mfc, &mrt->mfc_unres_queue, list) {
+ if (e < s_e)
+ goto next_entry2;
+ if (filter->dev &&
+ !mr_mfc_uses_dev(mrt, mfc, filter->dev))
+ goto next_entry2;
+
+ err = fill(mrt, skb, NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq, mfc, RTM_NEWROUTE, flags);
+ if (err < 0) {
+ spin_unlock_bh(lock);
+ goto out;
+ }
+ next_entry2:
+ e++;
+ }
+ spin_unlock_bh(lock);
+ err = 0;
+ e = 0;
+
+ out:
+ cb->args[1] = e;
+ return err;
+ }
+ EXPORT_SYMBOL(mr_table_dump);
+
int mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb,
struct mr_table *(*iter)(struct net *net,
struct mr_table *mrt),
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2] net-next/hinic: add checksum offload and TSO support
From: David Miller @ 2018-10-18 23:45 UTC (permalink / raw)
To: xuechaojing
Cc: linux-kernel, netdev, wulike1, chiqijun, fy.wang, zhaochen6,
tony.qu, luoshaokai
In-Reply-To: <20181018150251.28802-1-xuechaojing@huawei.com>
From: Xue Chaojing <xuechaojing@huawei.com>
Date: Thu, 18 Oct 2018 15:02:51 +0000
> From: Zhao Chen <zhaochen6@huawei.com>
>
> This patch adds checksum offload and TSO support for the HiNIC
> driver. Perfomance test (Iperf) shows more than 100% improvement
> in TCP streams.
>
> Signed-off-by: Zhao Chen <zhaochen6@huawei.com>
> Signed-off-by: Xue Chaojing <xuechaojing@huawei.com>
> ---
> v2: made all the local variables from longest to shortest line.
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] net: socket: fix a missing-check bug
From: David Miller @ 2018-10-18 23:43 UTC (permalink / raw)
To: wang6495; +Cc: kjlu, netdev, linux-kernel
In-Reply-To: <1539873406-5967-1-git-send-email-wang6495@umn.edu>
From: Wenwen Wang <wang6495@umn.edu>
Date: Thu, 18 Oct 2018 09:36:46 -0500
> In ethtool_ioctl(), the ioctl command 'ethcmd' is checked through a switch
> statement to see whether it is necessary to pre-process the ethtool
> structure, because, as mentioned in the comment, the structure
> ethtool_rxnfc is defined with padding. If yes, a user-space buffer 'rxnfc'
> is allocated through compat_alloc_user_space(). One thing to note here is
> that, if 'ethcmd' is ETHTOOL_GRXCLSRLALL, the size of the buffer 'rxnfc' is
> partially determined by 'rule_cnt', which is actually acquired from the
> user-space buffer 'compat_rxnfc', i.e., 'compat_rxnfc->rule_cnt', through
> get_user(). After 'rxnfc' is allocated, the data in the original user-space
> buffer 'compat_rxnfc' is then copied to 'rxnfc' through copy_in_user(),
> including the 'rule_cnt' field. However, after this copy, no check is
> re-enforced on 'rxnfc->rule_cnt'. So it is possible that a malicious user
> race to change the value in the 'compat_rxnfc->rule_cnt' between these two
> copies. Through this way, the attacker can bypass the previous check on
> 'rule_cnt' and inject malicious data. This can cause undefined behavior of
> the kernel and introduce potential security risk.
>
> This patch avoids the above issue via copying the value acquired by
> get_user() to 'rxnfc->rule_cn', if 'ethcmd' is ETHTOOL_GRXCLSRLALL.
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
This isn't pretty, but I can't come up with a better fix.
Note that we check and validate the rule count value even a third time
when we copy the rules back out to userspace.
Applied and queued up for -stable, thank you.
^ permalink raw reply
* [net 1/1] tipc: fix info leak from kernel tipc_event
From: Jon Maloy @ 2018-10-18 15:38 UTC (permalink / raw)
To: davem, netdev
Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, jon.maloy,
canh.d.luu, ying.xue, tipc-discussion
We initialize a struct tipc_event allocated on the kernel stack to
zero to avert info leak to user space.
Reported-by: syzbot+057458894bc8cada4dee@syzkaller.appspotmail.com
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/group.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/tipc/group.c b/net/tipc/group.c
index e82f13c..06fee14 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -666,6 +666,7 @@ static void tipc_group_create_event(struct tipc_group *grp,
struct sk_buff *skb;
struct tipc_msg *hdr;
+ memset(&evt, 0, sizeof(evt));
evt.event = event;
evt.found_lower = m->instance;
evt.found_upper = m->instance;
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v4] Wait for running BPF programs when updating map-in-map
From: Joel Fernandes @ 2018-10-18 23:36 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Colascione, Joel Fernandes, LKML, Tim Murray, netdev,
Lorenzo Colitti, Chenbo Feng, Mathieu Desnoyers,
Alexei Starovoitov, Daniel Borkmann, stable
In-Reply-To: <20181018154657.ehgj3ozy62zy47hi@ast-mbp.dhcp.thefacebook.com>
On Thu, Oct 18, 2018 at 08:46:59AM -0700, Alexei Starovoitov wrote:
> On Tue, Oct 16, 2018 at 10:39:57AM -0700, Joel Fernandes wrote:
> > On Fri, Oct 12, 2018 at 7:31 PM, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > On Fri, Oct 12, 2018 at 03:54:27AM -0700, Daniel Colascione wrote:
> > >> The map-in-map frequently serves as a mechanism for atomic
> > >> snapshotting of state that a BPF program might record. The current
> > >> implementation is dangerous to use in this way, however, since
> > >> userspace has no way of knowing when all programs that might have
> > >> retrieved the "old" value of the map may have completed.
> > >>
> > >> This change ensures that map update operations on map-in-map map types
> > >> always wait for all references to the old map to drop before returning
> > >> to userspace.
> > >>
> > >> Signed-off-by: Daniel Colascione <dancol@google.com>
> > >> ---
> > >> kernel/bpf/syscall.c | 14 ++++++++++++++
> > >> 1 file changed, 14 insertions(+)
> > >>
> > >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > >> index 8339d81cba1d..d7c16ae1e85a 100644
> > >> --- a/kernel/bpf/syscall.c
> > >> +++ b/kernel/bpf/syscall.c
> > >> @@ -741,6 +741,18 @@ static int map_lookup_elem(union bpf_attr *attr)
> > >> return err;
> > >> }
> > >>
> > >> +static void maybe_wait_bpf_programs(struct bpf_map *map)
> > >> +{
> > >> + /* Wait for any running BPF programs to complete so that
> > >> + * userspace, when we return to it, knows that all programs
> > >> + * that could be running use the new map value.
> > >> + */
> > >> + if (map->map_type == BPF_MAP_TYPE_HASH_OF_MAPS ||
> > >> + map->map_type == BPF_MAP_TYPE_ARRAY_OF_MAPS) {
> > >> + synchronize_rcu();
> > >> + }
> > >
> > > extra {} were not necessary. I removed them while applying to bpf-next.
> > > Please run checkpatch.pl next time.
> > > Thanks
> >
> > Thanks Alexei for taking it. Me and Lorenzo were discussing that not
> > having this causes incorrect behavior for apps using map-in-map for
> > this. So I CC'd stable as well.
>
> It is too late in the release cycle.
> We can submit it to stable releases after the merge window.
>
Sounds good, thanks.
- Joel
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] tools, perf: use smp_{rmb,mb} barriers instead of {rmb,mb}
From: Alexei Starovoitov @ 2018-10-18 15:33 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Peter Zijlstra, paulmck, will.deacon, acme, yhs, john.fastabend,
netdev
In-Reply-To: <df1da9c7-f9b4-a482-0bbc-a6455b57a476@iogearbox.net>
On Thu, Oct 18, 2018 at 05:04:34PM +0200, Daniel Borkmann wrote:
> #endif /* _TOOLS_LINUX_ASM_IA64_BARRIER_H */
> diff --git a/tools/arch/powerpc/include/asm/barrier.h b/tools/arch/powerpc/include/asm/barrier.h
> index a634da0..905a2c6 100644
> --- a/tools/arch/powerpc/include/asm/barrier.h
> +++ b/tools/arch/powerpc/include/asm/barrier.h
> @@ -27,4 +27,20 @@
> #define rmb() __asm__ __volatile__ ("sync" : : : "memory")
> #define wmb() __asm__ __volatile__ ("sync" : : : "memory")
>
> +#if defined(__powerpc64__)
> +#define smp_lwsync() __asm__ __volatile__ ("lwsync" : : : "memory")
> +
> +#define smp_store_release(p, v) \
> +do { \
> + smp_lwsync(); \
> + WRITE_ONCE(*p, v); \
> +} while (0)
> +
> +#define smp_load_acquire(p) \
> +({ \
> + typeof(*p) ___p1 = READ_ONCE(*p); \
> + smp_lwsync(); \
> + ___p1; \
I don't like this proliferation of asm.
Why do we think that we can do better job than compiler?
can we please use gcc builtins instead?
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html
__atomic_load_n(ptr, __ATOMIC_ACQUIRE);
__atomic_store_n(ptr, val, __ATOMIC_RELEASE);
are done specifically for this use case if I'm not mistaken.
I think it pays to learn what compiler provides.
^ permalink raw reply
* Re: [PATCH net v2] net/sched: act_gact: properly init 'goto chain'
From: Davide Caratti @ 2018-10-18 15:30 UTC (permalink / raw)
To: Jamal Hadi Salim, Jiri Pirko, Cong Wang, David S. Miller, netdev
In-Reply-To: <b27083a6-ad14-0f44-f85c-1ee73b8ddb8a@mojatatu.com>
hello Jamal,
On Thu, 2018-10-18 at 08:52 -0400, Jamal Hadi Salim wrote:
> Rejection is a good solution[1].
> Would be helpful to set an ext_ack to something like
> "only one goto chain is supported currently"
ok, that's material for a v3, if you / Cong / others are ok with the other
hunks.
On Thu, 2018-10-18 at 08:52 -0400, Jamal Hadi Salim wrote:
> I didnt follow why you needed to introduce tcfg_caction...
it's a trick (maybe an abuse, up to you to decide): when the user does
# tc f a dev v0 egress matchall action pass random determ goto chain 4 5
tcfg_caction contains 'pass', tcfg_paction and tcf_action both contain
'goto chain 4'. On the opposite, when the user does
# tc f a dev v0 egress matchall action goto chain 4 random determ drop 5
tcfg_caction and tcf_action both contain 'goto chain 4', and tcf_paction
contains 'drop'. In this way, the 'goto chain' is always stored in
tcf_action, so that TC generic code can always initialize a->goto_chain.
(we need tcf_caction, becaue tcf_gact_dump() and helpers in tc_gact.h need
to read the control action on the left of "random")
The alternative is, we systematically forbid usage of 'goto chain' in
tcfg_paction, so that:
# tc f a dev v0 egress matchall action <whatever> random determ goto chain 4 5
is systematically rejected with -EINVAL. This comand never worked, so we
are not breaking anything in userspace.
thanks for reviewing!
--
davide
^ permalink raw reply
* Re: [iproute PATCH] rdma: Fix for ineffective check in add_filter()
From: David Ahern @ 2018-10-18 15:27 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger; +Cc: netdev, Arkadi Sharshevsky
In-Reply-To: <20181018114154.8886-1-phil@nwl.cc>
On 10/18/18 5:41 AM, Phil Sutter wrote:
> With 'name' field defined as array in struct filters, it will always
> contain a value irrespective of whether a name was assigned or not.
>
> Fix this by turning the field into a const char pointer.
>
> Fixes: 8cd644095842a ("devlink: Add support for devlink resource abstraction")
Stale paste buffer? Seems like the correct tag is
Fixes: 1174be72d1b4c ("rdma: Add filtering infrastructure")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> rdma/rdma.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rdma/rdma.h b/rdma/rdma.h
> index d4b7ba1918b13..c3b7530b6cc71 100644
> --- a/rdma/rdma.h
> +++ b/rdma/rdma.h
> @@ -34,7 +34,7 @@
>
> #define MAX_NUMBER_OF_FILTERS 64
> struct filters {
> - char name[32];
> + const char *name;
> bool is_number;
> };
>
>
^ permalink raw reply
* Re: [PATCH] virtio_net: add local_bh_disable() around u64_stats_update_begin
From: David Miller @ 2018-10-18 23:23 UTC (permalink / raw)
To: bigeasy; +Cc: mst, netdev, virtualization, tglx
In-Reply-To: <20181018084313.oopu34iwfwgkcwwc@linutronix.de>
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Thu, 18 Oct 2018 10:43:13 +0200
> on 32bit, lockdep notices that virtnet_open() and refill_work() invoke
> try_fill_recv() from process context while virtnet_receive() invokes the
> same function from BH context. The problem that the seqcounter within
> u64_stats_update_begin() may deadlock if it is interrupted by BH and
> then acquired again.
>
> Introduce u64_stats_update_begin_bh() which disables BH on 32bit
> architectures. Since the BH might interrupt the worker, this new
> function should not limited to SMP like the others which are expected
> to be used in softirq.
>
> With this change we might lose increments but this is okay. The
> important part that the two 32bit parts of the 64bit counter are not
> corrupted.
>
> Fixes: 461f03dc99cf6 ("virtio_net: Add kick stats").
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Trying to get down to the bottom of this:
1) virtnet_receive() runs from softirq but only if NAPI is active and
enabled. It is in this context that it invokes try_fill_recv().
2) refill_work() runs from process context, but disables NAPI (and
thus invocation of virtnet_receive()) before calling
try_fill_recv().
3) virtnet_open() invokes from process context as well, but before the
NAPI instances are enabled, it is same as case #2.
4) virtnet_restore_up() is the same situations as #3.
Therefore I agree that this is a false positive, and simply lockdep
cannot see the NAPI synchronization done by case #2.
I think we shouldn't add unnecessary BH disabling here, and instead
find some way to annotate this for lockdep's sake.
Thank you.
^ permalink raw reply
* [PATCH net-next v2] netpoll: allow cleanup to be synchronous
From: Debabrata Banerjee @ 2018-10-18 15:18 UTC (permalink / raw)
To: David S . Miller, Neil Horman, netdev; +Cc: dbanerje
This fixes a problem introduced by:
commit 2cde6acd49da ("netpoll: Fix __netpoll_rcu_free so that it can hold the rtnl lock")
When using netconsole on a bond, __netpoll_cleanup can asynchronously
recurse multiple times, each __netpoll_free_async call can result in
more __netpoll_free_async's. This means there is now a race between
cleanup_work queues on multiple netpoll_info's on multiple devices and
the configuration of a new netpoll. For example if a netconsole is set
to enable 0, reconfigured, and enable 1 immediately, this netconsole
will likely not work.
Given the reason for __netpoll_free_async is it can be called when rtnl
is not locked, if it is locked, we should be able to execute
synchronously. It appears to be locked everywhere it's called from.
Generalize the design pattern from the teaming driver for current
callers of __netpoll_free_async.
CC: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Debabrata Banerjee <dbanerje@akamai.com>
---
drivers/net/bonding/bond_main.c | 3 ++-
drivers/net/macvlan.c | 2 +-
drivers/net/team/team.c | 5 +----
include/linux/netpoll.h | 4 +---
net/8021q/vlan_dev.c | 3 +--
net/bridge/br_device.c | 2 +-
net/core/netpoll.c | 20 +++++---------------
net/dsa/slave.c | 2 +-
8 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ee28ec9e0aba..ffa37adb7681 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -963,7 +963,8 @@ static inline void slave_disable_netpoll(struct slave *slave)
return;
slave->np = NULL;
- __netpoll_free_async(np);
+
+ __netpoll_free(np);
}
static void bond_poll_controller(struct net_device *bond_dev)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index cfda146f3b3b..fc8d5f1ee1ad 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1077,7 +1077,7 @@ static void macvlan_dev_netpoll_cleanup(struct net_device *dev)
vlan->netpoll = NULL;
- __netpoll_free_async(netpoll);
+ __netpoll_free(netpoll);
}
#endif /* CONFIG_NET_POLL_CONTROLLER */
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index d887016e54b6..db633ae9f784 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1104,10 +1104,7 @@ static void team_port_disable_netpoll(struct team_port *port)
return;
port->np = NULL;
- /* Wait for transmitting packets to finish before freeing. */
- synchronize_rcu_bh();
- __netpoll_cleanup(np);
- kfree(np);
+ __netpoll_free(np);
}
#else
static int team_port_enable_netpoll(struct team_port *port)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 3ef82d3a78db..676f1ff161a9 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -31,8 +31,6 @@ struct netpoll {
bool ipv6;
u16 local_port, remote_port;
u8 remote_mac[ETH_ALEN];
-
- struct work_struct cleanup_work;
};
struct netpoll_info {
@@ -63,7 +61,7 @@ int netpoll_parse_options(struct netpoll *np, char *opt);
int __netpoll_setup(struct netpoll *np, struct net_device *ndev);
int netpoll_setup(struct netpoll *np);
void __netpoll_cleanup(struct netpoll *np);
-void __netpoll_free_async(struct netpoll *np);
+void __netpoll_free(struct netpoll *np);
void netpoll_cleanup(struct netpoll *np);
void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
struct net_device *dev);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 546af0e73ac3..ff720f1ebf73 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -756,8 +756,7 @@ static void vlan_dev_netpoll_cleanup(struct net_device *dev)
return;
vlan->netpoll = NULL;
-
- __netpoll_free_async(netpoll);
+ __netpoll_free(netpoll);
}
#endif /* CONFIG_NET_POLL_CONTROLLER */
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index e053a4e43758..c6abf927f0c9 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -344,7 +344,7 @@ void br_netpoll_disable(struct net_bridge_port *p)
p->np = NULL;
- __netpoll_free_async(np);
+ __netpoll_free(np);
}
#endif
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index de1d1ba92f2d..6ac71624ead4 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -591,7 +591,6 @@ int __netpoll_setup(struct netpoll *np, struct net_device *ndev)
np->dev = ndev;
strlcpy(np->dev_name, ndev->name, IFNAMSIZ);
- INIT_WORK(&np->cleanup_work, netpoll_async_cleanup);
if (ndev->priv_flags & IFF_DISABLE_NETPOLL) {
np_err(np, "%s doesn't support polling, aborting\n",
@@ -790,10 +789,6 @@ void __netpoll_cleanup(struct netpoll *np)
{
struct netpoll_info *npinfo;
- /* rtnl_dereference would be preferable here but
- * rcu_cleanup_netpoll path can put us in here safely without
- * holding the rtnl, so plain rcu_dereference it is
- */
npinfo = rtnl_dereference(np->dev->npinfo);
if (!npinfo)
return;
@@ -814,21 +809,16 @@ void __netpoll_cleanup(struct netpoll *np)
}
EXPORT_SYMBOL_GPL(__netpoll_cleanup);
-static void netpoll_async_cleanup(struct work_struct *work)
+void __netpoll_free(struct netpoll *np)
{
- struct netpoll *np = container_of(work, struct netpoll, cleanup_work);
+ ASSERT_RTNL();
- rtnl_lock();
+ /* Wait for transmitting packets to finish before freeing. */
+ synchronize_rcu_bh();
__netpoll_cleanup(np);
- rtnl_unlock();
kfree(np);
}
-
-void __netpoll_free_async(struct netpoll *np)
-{
- schedule_work(&np->cleanup_work);
-}
-EXPORT_SYMBOL_GPL(__netpoll_free_async);
+EXPORT_SYMBOL_GPL(__netpoll_free);
void netpoll_cleanup(struct netpoll *np)
{
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3f840b6eea69..3679e13b2ead 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -722,7 +722,7 @@ static void dsa_slave_netpoll_cleanup(struct net_device *dev)
p->netpoll = NULL;
- __netpoll_free_async(netpoll);
+ __netpoll_free(netpoll);
}
static void dsa_slave_poll_controller(struct net_device *dev)
--
2.19.1
^ permalink raw reply related
* RE: [PATCH net-next] netpoll: allow cleanup to be synchronous
From: Banerjee, Debabrata @ 2018-10-18 15:17 UTC (permalink / raw)
To: 'Neil Horman', David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <20181018112929.GA2601@hmswarspite.think-freely.org>
> From: Neil Horman <nhorman@tuxdriver.com>
> Agreed, this doesn't make sense. If you want a synchronous cleanup, create
> a wrapper function that creates a wait queue, calls __netpoll_free_async,
> and blocks on the wait queue completion. Modify the cleanup_work
> method(s) to complete the wait queue, and you've got what you want.
>
> Neil
Actually the patch looks bad, but it seems to turn out the rtnl is always
held by the parent when it is called, and asynchronous cleanup doesn't
seem to be necessary at all. Perhaps you could share your deadlock
test case for 2cde6acd49da?
Patch v2 coming next.
-Deb
^ permalink raw reply
* Re: [PATCH net] net: ipmr: fix unresolved entry dumps
From: David Ahern @ 2018-10-18 15:16 UTC (permalink / raw)
To: David Miller, nikolay; +Cc: netdev
In-Reply-To: <20181017.223629.857704876875928943.davem@davemloft.net>
On 10/17/18 11:36 PM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Wed, 17 Oct 2018 22:34:34 +0300
>
>> If the skb space ends in an unresolved entry while dumping we'll miss
>> some unresolved entries. The reason is due to zeroing the entry counter
>> between dumping resolved and unresolved mfc entries. We should just
>> keep counting until the whole table is dumped and zero when we move to
>> the next as we have a separate table counter.
>>
>> Reported-by: Colin Ian King <colin.king@canonical.com>
>> Fixes: 8fb472c09b9d ("ipmr: improve hash scalability")
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>
> Applied and queued up for -stable.
>
FYI: on the net to net-next merge, those 2 lines were moved to
mr_table_dump.
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] tools, perf: use smp_{rmb,mb} barriers instead of {rmb,mb}
From: Daniel Borkmann @ 2018-10-18 15:04 UTC (permalink / raw)
To: Peter Zijlstra
Cc: alexei.starovoitov, paulmck, will.deacon, acme, yhs,
john.fastabend, netdev
In-Reply-To: <20181018081434.GT3121@hirez.programming.kicks-ass.net>
On 10/18/2018 10:14 AM, Peter Zijlstra wrote:
> On Thu, Oct 18, 2018 at 01:10:15AM +0200, Daniel Borkmann wrote:
>
>> Wouldn't this then also allow the kernel side to use smp_store_release()
>> when it updates the head? We'd be pretty much at the model as described
>> in Documentation/core-api/circular-buffers.rst.
>>
>> Meaning, rough pseudo-code diff would look as:
>>
>> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
>> index 5d3cf40..3d96275 100644
>> --- a/kernel/events/ring_buffer.c
>> +++ b/kernel/events/ring_buffer.c
>> @@ -84,8 +84,9 @@ static void perf_output_put_handle(struct perf_output_handle *handle)
>> *
>> * See perf_output_begin().
>> */
>> - smp_wmb(); /* B, matches C */
>> - rb->user_page->data_head = head;
>> +
>> + /* B, matches C */
>> + smp_store_release(&rb->user_page->data_head, head);
>
> Yes, this would be correct.
>
> The reason we didn't do this is because smp_store_release() ends up
> being smp_mb() + WRITE_ONCE() for a fair number of platforms, even if
> they have a cheaper smp_wmb(). Most notably ARM.
Yep agree, that would be worse..
> (ARM64 OTOH would like to have smp_store_release() there I imagine;
> while x86 doesn't care either way around).
>
> A similar concern exists for the smp_load_acquire() I proposed for the
> userspace side, ARM would have to resort to smp_mb() in that situation,
> instead of the cheaper smp_rmb().
>
> The smp_store_release() on the userspace side will actually be of equal
> cost or cheaper, since it already has an smp_mb(). Most notably, x86 can
> avoid barrier entirely, because TSO doesn't allow the LOAD-STORE reorder
> (it only allows the STORE-LOAD reorder). And PowerPC can use LWSYNC
> instead of SYNC.
Ok, thanks a lot for your feedback, Peter! I've changed the user space
side now to the following diff (also moving to a small helper so it can
be reused by libbpf in the subsequent fix I had in the series):
tools/arch/arm64/include/asm/barrier.h | 70 +++++++++++++++++++++++++++++++
tools/arch/ia64/include/asm/barrier.h | 13 ++++++
tools/arch/powerpc/include/asm/barrier.h | 16 +++++++
tools/arch/s390/include/asm/barrier.h | 13 ++++++
tools/arch/sparc/include/asm/barrier_64.h | 13 ++++++
tools/arch/x86/include/asm/barrier.h | 14 +++++++
tools/include/asm/barrier.h | 35 ++++++++++++++++
tools/include/linux/ring_buffer.h | 69 ++++++++++++++++++++++++++++++
tools/perf/util/mmap.h | 15 ++-----
9 files changed, 246 insertions(+), 12 deletions(-)
create mode 100644 tools/include/linux/ring_buffer.h
diff --git a/tools/arch/arm64/include/asm/barrier.h b/tools/arch/arm64/include/asm/barrier.h
index 40bde6b..12835ea 100644
--- a/tools/arch/arm64/include/asm/barrier.h
+++ b/tools/arch/arm64/include/asm/barrier.h
@@ -14,4 +14,74 @@
#define wmb() asm volatile("dmb ishst" ::: "memory")
#define rmb() asm volatile("dmb ishld" ::: "memory")
+#define smp_store_release(p, v) \
+do { \
+ union { typeof(*p) __val; char __c[1]; } __u = \
+ { .__val = (__force typeof(*p)) (v) }; \
+ \
+ switch (sizeof(*p)) { \
+ case 1: \
+ asm volatile ("stlrb %w1, %0" \
+ : "=Q" (*p) \
+ : "r" (*(__u8 *)__u.__c) \
+ : "memory"); \
+ break; \
+ case 2: \
+ asm volatile ("stlrh %w1, %0" \
+ : "=Q" (*p) \
+ : "r" (*(__u16 *)__u.__c) \
+ : "memory"); \
+ break; \
+ case 4: \
+ asm volatile ("stlr %w1, %0" \
+ : "=Q" (*p) \
+ : "r" (*(__u32 *)__u.__c) \
+ : "memory"); \
+ break; \
+ case 8: \
+ asm volatile ("stlr %1, %0" \
+ : "=Q" (*p) \
+ : "r" (*(__u64 *)__u.__c) \
+ : "memory"); \
+ break; \
+ default: \
+ /* Only to shut up gcc ... */ \
+ mb(); \
+ break; \
+ } \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ union { typeof(*p) __val; char __c[1]; } __u; \
+ \
+ switch (sizeof(*p)) { \
+ case 1: \
+ asm volatile ("ldarb %w0, %1" \
+ : "=r" (*(__u8 *)__u.__c) \
+ : "Q" (*p) : "memory"); \
+ break; \
+ case 2: \
+ asm volatile ("ldarh %w0, %1" \
+ : "=r" (*(__u16 *)__u.__c) \
+ : "Q" (*p) : "memory"); \
+ break; \
+ case 4: \
+ asm volatile ("ldar %w0, %1" \
+ : "=r" (*(__u32 *)__u.__c) \
+ : "Q" (*p) : "memory"); \
+ break; \
+ case 8: \
+ asm volatile ("ldar %0, %1" \
+ : "=r" (*(__u64 *)__u.__c) \
+ : "Q" (*p) : "memory"); \
+ break; \
+ default: \
+ /* Only to shut up gcc ... */ \
+ mb(); \
+ break; \
+ } \
+ __u.__val; \
+})
+
#endif /* _TOOLS_LINUX_ASM_AARCH64_BARRIER_H */
diff --git a/tools/arch/ia64/include/asm/barrier.h b/tools/arch/ia64/include/asm/barrier.h
index d808ee0..4d471d9 100644
--- a/tools/arch/ia64/include/asm/barrier.h
+++ b/tools/arch/ia64/include/asm/barrier.h
@@ -46,4 +46,17 @@
#define rmb() mb()
#define wmb() mb()
+#define smp_store_release(p, v) \
+do { \
+ barrier(); \
+ WRITE_ONCE(*p, v); \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = READ_ONCE(*p); \
+ barrier(); \
+ ___p1; \
+})
+
#endif /* _TOOLS_LINUX_ASM_IA64_BARRIER_H */
diff --git a/tools/arch/powerpc/include/asm/barrier.h b/tools/arch/powerpc/include/asm/barrier.h
index a634da0..905a2c6 100644
--- a/tools/arch/powerpc/include/asm/barrier.h
+++ b/tools/arch/powerpc/include/asm/barrier.h
@@ -27,4 +27,20 @@
#define rmb() __asm__ __volatile__ ("sync" : : : "memory")
#define wmb() __asm__ __volatile__ ("sync" : : : "memory")
+#if defined(__powerpc64__)
+#define smp_lwsync() __asm__ __volatile__ ("lwsync" : : : "memory")
+
+#define smp_store_release(p, v) \
+do { \
+ smp_lwsync(); \
+ WRITE_ONCE(*p, v); \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = READ_ONCE(*p); \
+ smp_lwsync(); \
+ ___p1; \
+})
+#endif /* defined(__powerpc64__) */
#endif /* _TOOLS_LINUX_ASM_POWERPC_BARRIER_H */
diff --git a/tools/arch/s390/include/asm/barrier.h b/tools/arch/s390/include/asm/barrier.h
index 5030c99..de362fa6 100644
--- a/tools/arch/s390/include/asm/barrier.h
+++ b/tools/arch/s390/include/asm/barrier.h
@@ -28,4 +28,17 @@
#define rmb() mb()
#define wmb() mb()
+#define smp_store_release(p, v) \
+do { \
+ barrier(); \
+ WRITE_ONCE(*p, v); \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = READ_ONCE(*p); \
+ barrier(); \
+ ___p1; \
+})
+
#endif /* __TOOLS_LIB_ASM_BARRIER_H */
diff --git a/tools/arch/sparc/include/asm/barrier_64.h b/tools/arch/sparc/include/asm/barrier_64.h
index ba61344..cfb0fdc 100644
--- a/tools/arch/sparc/include/asm/barrier_64.h
+++ b/tools/arch/sparc/include/asm/barrier_64.h
@@ -40,4 +40,17 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define rmb() __asm__ __volatile__("":::"memory")
#define wmb() __asm__ __volatile__("":::"memory")
+#define smp_store_release(p, v) \
+do { \
+ barrier(); \
+ WRITE_ONCE(*p, v); \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = READ_ONCE(*p); \
+ barrier(); \
+ ___p1; \
+})
+
#endif /* !(__TOOLS_LINUX_SPARC64_BARRIER_H) */
diff --git a/tools/arch/x86/include/asm/barrier.h b/tools/arch/x86/include/asm/barrier.h
index 8774dee..5891986 100644
--- a/tools/arch/x86/include/asm/barrier.h
+++ b/tools/arch/x86/include/asm/barrier.h
@@ -26,4 +26,18 @@
#define wmb() asm volatile("sfence" ::: "memory")
#endif
+#if defined(__x86_64__)
+#define smp_store_release(p, v) \
+do { \
+ barrier(); \
+ WRITE_ONCE(*p, v); \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = READ_ONCE(*p); \
+ barrier(); \
+ ___p1; \
+})
+#endif /* defined(__x86_64__) */
#endif /* _TOOLS_LINUX_ASM_X86_BARRIER_H */
diff --git a/tools/include/asm/barrier.h b/tools/include/asm/barrier.h
index 391d942..8d378c5 100644
--- a/tools/include/asm/barrier.h
+++ b/tools/include/asm/barrier.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/compiler.h>
#if defined(__i386__) || defined(__x86_64__)
#include "../../arch/x86/include/asm/barrier.h"
#elif defined(__arm__)
@@ -26,3 +27,37 @@
#else
#include <asm-generic/barrier.h>
#endif
+
+/*
+ * Generic fallback smp_*() definitions for archs that haven't
+ * been updated yet.
+ */
+
+#ifndef smp_rmb
+# define smp_rmb() rmb()
+#endif
+
+#ifndef smp_wmb
+# define smp_wmb() wmb()
+#endif
+
+#ifndef smp_mb
+# define smp_mb() mb()
+#endif
+
+#ifndef smp_store_release
+# define smp_store_release(p, v) \
+do { \
+ smp_mb(); \
+ WRITE_ONCE(*p, v); \
+} while (0)
+#endif
+
+#ifndef smp_load_acquire
+# define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = READ_ONCE(*p); \
+ smp_mb(); \
+ ___p1; \
+})
+#endif
diff --git a/tools/include/linux/ring_buffer.h b/tools/include/linux/ring_buffer.h
new file mode 100644
index 0000000..48200e0
--- /dev/null
+++ b/tools/include/linux/ring_buffer.h
@@ -0,0 +1,69 @@
+#ifndef _TOOLS_LINUX_RING_BUFFER_H_
+#define _TOOLS_LINUX_RING_BUFFER_H_
+
+#include <linux/compiler.h>
+#include <asm/barrier.h>
+
+/*
+ * Below barriers pair as follows (kernel/events/ring_buffer.c):
+ *
+ * Since the mmap() consumer (userspace) can run on a different CPU:
+ *
+ * kernel user
+ *
+ * if (LOAD ->data_tail) { LOAD ->data_head
+ * (A) smp_rmb() (C)
+ * STORE $data LOAD $data
+ * smp_wmb() (B) smp_mb() (D)
+ * STORE ->data_head STORE ->data_tail
+ * }
+ *
+ * Where A pairs with D, and B pairs with C.
+ *
+ * In our case A is a control dependency that separates the load
+ * of the ->data_tail and the stores of $data. In case ->data_tail
+ * indicates there is no room in the buffer to store $data we do not.
+ *
+ * D needs to be a full barrier since it separates the data READ
+ * from the tail WRITE.
+ *
+ * For B a WMB is sufficient since it separates two WRITEs, and for
+ * C an RMB is sufficient since it separates two READs.
+ */
+
+/*
+ * Note, instead of B, C, D we could also use smp_store_release()
+ * in B and D as well as smp_load_acquire() in C. However, this
+ * optimization makes sense not for all architectures since it
+ * would resolve into READ_ONCE() + smp_mb() pair for smp_load_acquire()
+ * and smp_mb() + WRITE_ONCE() pair for smp_store_release(), thus
+ * for those smp_wmb() in B and smp_rmb() in C would still be less
+ * expensive. For the case of D this has either the same cost or
+ * is less expensive. For example, due to TSO (total store order),
+ * x86 can avoid the CPU barrier entirely.
+ */
+
+static inline u64 ring_buffer_read_head(struct perf_event_mmap_page *base)
+{
+/*
+ * Architectures where smp_load_acquire() does not fallback to
+ * READ_ONCE() + smp_mb() pair.
+ */
+#if defined(__x86_64__) || defined(__aarch64__) || defined(__powerpc64__) || \
+ defined(__ia64__) || defined(__sparc__) && defined(__arch64__)
+ return smp_load_acquire(&base->data_head);
+#else
+ u64 head = READ_ONCE(base->data_head);
+
+ smp_rmb();
+ return head;
+#endif
+}
+
+static inline void ring_buffer_write_tail(struct perf_event_mmap_page *base,
+ u64 tail)
+{
+ smp_store_release(&base->data_tail, tail);
+}
+
+#endif /* _TOOLS_LINUX_RING_BUFFER_H_ */
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 05a6d47..8f6531f 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -4,7 +4,7 @@
#include <linux/compiler.h>
#include <linux/refcount.h>
#include <linux/types.h>
-#include <asm/barrier.h>
+#include <linux/ring_buffer.h>
#include <stdbool.h>
#include "auxtrace.h"
#include "event.h"
@@ -71,21 +71,12 @@ void perf_mmap__consume(struct perf_mmap *map);
static inline u64 perf_mmap__read_head(struct perf_mmap *mm)
{
- struct perf_event_mmap_page *pc = mm->base;
- u64 head = READ_ONCE(pc->data_head);
- rmb();
- return head;
+ return ring_buffer_read_head(mm->base);
}
static inline void perf_mmap__write_tail(struct perf_mmap *md, u64 tail)
{
- struct perf_event_mmap_page *pc = md->base;
-
- /*
- * ensure all reads are done before we write the tail out.
- */
- mb();
- pc->data_tail = tail;
+ ring_buffer_write_tail(md->base, tail);
}
union perf_event *perf_mmap__read_forward(struct perf_mmap *map);
--
2.9.5
^ permalink raw reply related
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