* Re: [PATCH net-next 14/18] ionic: Add Tx and Rx handling
From: Jakub Kicinski @ 2019-06-26 0:08 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-15-snelson@pensando.io>
On Thu, 20 Jun 2019 13:24:20 -0700, Shannon Nelson wrote:
> Add both the Tx and Rx queue setup and handling. The related
> stats display come later. Instead of using the generic napi
> routines used by the slow-path command, the Tx and Rx paths
> are simplified and inlined in one file in order to get better
> compiler optimizations.
>
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> index 5ebfaa320edf..6dfcada9e822 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> @@ -351,6 +351,54 @@ int ionic_debugfs_add_qcq(struct lif *lif, struct qcq *qcq)
> desc_blob);
> }
>
> + if (qcq->flags & QCQ_F_TX_STATS) {
> + stats_dentry = debugfs_create_dir("tx_stats", q_dentry);
> + if (IS_ERR_OR_NULL(stats_dentry))
> + return PTR_ERR(stats_dentry);
> +
> + debugfs_create_u64("dma_map_err", 0400, stats_dentry,
> + &qcq->stats->tx.dma_map_err);
> + debugfs_create_u64("pkts", 0400, stats_dentry,
> + &qcq->stats->tx.pkts);
> + debugfs_create_u64("bytes", 0400, stats_dentry,
> + &qcq->stats->tx.bytes);
> + debugfs_create_u64("clean", 0400, stats_dentry,
> + &qcq->stats->tx.clean);
> + debugfs_create_u64("linearize", 0400, stats_dentry,
> + &qcq->stats->tx.linearize);
> + debugfs_create_u64("no_csum", 0400, stats_dentry,
> + &qcq->stats->tx.no_csum);
> + debugfs_create_u64("csum", 0400, stats_dentry,
> + &qcq->stats->tx.csum);
> + debugfs_create_u64("crc32_csum", 0400, stats_dentry,
> + &qcq->stats->tx.crc32_csum);
> + debugfs_create_u64("tso", 0400, stats_dentry,
> + &qcq->stats->tx.tso);
> + debugfs_create_u64("frags", 0400, stats_dentry,
> + &qcq->stats->tx.frags);
I wonder why debugfs over ethtool -S?
> +static int ionic_tx(struct queue *q, struct sk_buff *skb)
> +{
> + struct tx_stats *stats = q_to_tx_stats(q);
> + int err;
> +
> + if (skb->ip_summed == CHECKSUM_PARTIAL)
> + err = ionic_tx_calc_csum(q, skb);
> + else
> + err = ionic_tx_calc_no_csum(q, skb);
> + if (err)
> + return err;
> +
> + err = ionic_tx_skb_frags(q, skb);
> + if (err)
> + return err;
> +
> + skb_tx_timestamp(skb);
> + stats->pkts++;
> + stats->bytes += skb->len;
nit: I think counting stats on completion may be a better idea,
otherwise when you can a full ring on stop your HW counters are
guaranteed to be different than SW counters. Am I wrong?
> + ionic_txq_post(q, !netdev_xmit_more(), ionic_tx_clean, skb);
> +
> + return 0;
> +}
> +
> +static int ionic_tx_descs_needed(struct queue *q, struct sk_buff *skb)
> +{
> + struct tx_stats *stats = q_to_tx_stats(q);
> + int err;
> +
> + /* If TSO, need roundup(skb->len/mss) descs */
> + if (skb_is_gso(skb))
> + return (skb->len / skb_shinfo(skb)->gso_size) + 1;
This doesn't look correct, are you sure you don't want
skb_shinfo(skb)->gso_segs ?
> +
> + /* If non-TSO, just need 1 desc and nr_frags sg elems */
> + if (skb_shinfo(skb)->nr_frags <= IONIC_TX_MAX_SG_ELEMS)
> + return 1;
> +
> + /* Too many frags, so linearize */
> + err = skb_linearize(skb);
> + if (err)
> + return err;
> +
> + stats->linearize++;
> +
> + /* Need 1 desc and zero sg elems */
> + return 1;
> +}
> +
> +netdev_tx_t ionic_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{
> + u16 queue_index = skb_get_queue_mapping(skb);
> + struct lif *lif = netdev_priv(netdev);
> + struct queue *q;
> + int ndescs;
> + int err;
> +
> + if (unlikely(!test_bit(LIF_UP, lif->state))) {
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> + }
Surely you stop TX before taking the queues down?
> + if (likely(lif_to_txqcq(lif, queue_index)))
> + q = lif_to_txq(lif, queue_index);
> + else
> + q = lif_to_txq(lif, 0);
> +
> + ndescs = ionic_tx_descs_needed(q, skb);
> + if (ndescs < 0)
> + goto err_out_drop;
> +
> + if (!ionic_q_has_space(q, ndescs)) {
> + netif_stop_subqueue(netdev, queue_index);
> + q->stop++;
> +
> + /* Might race with ionic_tx_clean, check again */
> + smp_rmb();
> + if (ionic_q_has_space(q, ndescs)) {
> + netif_wake_subqueue(netdev, queue_index);
> + q->wake++;
> + } else {
> + return NETDEV_TX_BUSY;
> + }
> + }
> +
> + if (skb_is_gso(skb))
> + err = ionic_tx_tso(q, skb);
> + else
> + err = ionic_tx(q, skb);
> +
> + if (err)
> + goto err_out_drop;
> +
> + return NETDEV_TX_OK;
> +
> +err_out_drop:
> + q->drop++;
> + dev_kfree_skb(skb);
> + return NETDEV_TX_OK;
> +}
^ permalink raw reply
* Re: [PATCH net-next 02/10] net: dsa: sja1105: Cancel PTP delayed work on unregister
From: Willem de Bruijn @ 2019-06-26 0:11 UTC (permalink / raw)
To: Vladimir Oltean
Cc: f.fainelli, vivien.didelot, andrew, David Miller,
Network Development
In-Reply-To: <20190625233942.1946-3-olteanv@gmail.com>
On Tue, Jun 25, 2019 at 7:40 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Currently when the driver unloads and PTP is enabled, the delayed work
> that prevents the timecounter from expiring becomes a ticking time bomb.
> The kernel will schedule the work thread within 60 seconds of driver
> removal, but the work handler is no longer there, leading to this
> strange and inconclusive stack trace:
>
> [ 64.473112] Unable to handle kernel paging request at virtual address 79746970
> [ 64.480340] pgd = 008c4af9
> [ 64.483042] [79746970] *pgd=00000000
> [ 64.486620] Internal error: Oops: 80000005 [#1] SMP ARM
> [ 64.491820] Modules linked in:
> [ 64.494871] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.2.0-rc5-01634-ge3a2773ba9e5 #1246
> [ 64.503007] Hardware name: Freescale LS1021A
> [ 64.507259] PC is at 0x79746970
> [ 64.510393] LR is at call_timer_fn+0x3c/0x18c
> [ 64.514729] pc : [<79746970>] lr : [<c03bd734>] psr: 60010113
> [ 64.520965] sp : c1901de0 ip : 00000000 fp : c1903080
> [ 64.526163] r10: c1901e38 r9 : ffffe000 r8 : c19064ac
> [ 64.531363] r7 : 79746972 r6 : e98dd260 r5 : 00000100 r4 : c1a9e4a0
> [ 64.537859] r3 : c1900000 r2 : ffffa400 r1 : 79746972 r0 : e98dd260
> [ 64.544359] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> [ 64.551460] Control: 10c5387d Table: a8a2806a DAC: 00000051
> [ 64.557176] Process swapper/0 (pid: 0, stack limit = 0x1ddb27f0)
> [ 64.563147] Stack: (0xc1901de0 to 0xc1902000)
> [ 64.567481] 1de0: eb6a4918 3d60d7c3 c1a9e554 e98dd260 eb6a34c0 c1a9e4a0 ffffa400 c19064ac
> [ 64.575616] 1e00: ffffe000 c03bd95c c1901e34 c1901e34 eb6a34c0 c1901e30 c1903d00 c186f4c0
> [ 64.583751] 1e20: c1906488 29e34000 c1903080 c03bdca4 00000000 eaa6f218 00000000 eb6a45c0
> [ 64.591886] 1e40: eb6a45c0 20010193 00000003 c03c0a68 20010193 3f7231be c1903084 00000002
> [ 64.600022] 1e60: 00000082 00000001 ffffe000 c1a9e0a4 00000100 c0302298 02b64722 0000000f
> [ 64.608157] 1e80: c186b3c8 c1877540 c19064ac 0000000a c186b350 ffffa401 c1903d00 c1107348
> [ 64.616292] 1ea0: 00200102 c0d87a14 ea823c00 ffffe000 00000012 00000000 00000000 ea810800
> [ 64.624427] 1ec0: f0803000 c1876ba8 00000000 c034c784 c18774b8 c039fb50 c1906c90 c1978aac
> [ 64.632562] 1ee0: f080200c f0802000 c1901f10 c0709ca8 c03091a0 60010013 ffffffff c1901f44
> [ 64.640697] 1f00: 00000000 c1900000 c1876ba8 c0301a8c 00000000 000070a0 eb6ac1a0 c031da60
> [ 64.648832] 1f20: ffffe000 c19064ac c19064f0 00000001 00000000 c1906488 c1876ba8 00000000
> [ 64.656967] 1f40: ffffffff c1901f60 c030919c c03091a0 60010013 ffffffff 00000051 00000000
> [ 64.665102] 1f60: ffffe000 c0376aa4 c1a9da37 ffffffff 00000037 3f7231be c1ab20c0 000000cc
> [ 64.673238] 1f80: c1906488 c1906480 ffffffff 00000037 c1ab20c0 c1ab20c0 00000001 c0376e1c
> [ 64.681373] 1fa0: c1ab2118 c1700ea8 ffffffff ffffffff 00000000 c1700754 c17dfa40 ebfffd80
> [ 64.689509] 1fc0: 00000000 c17dfa40 3f7733be 00000000 00000000 c1700330 00000051 10c0387d
> [ 64.697644] 1fe0: 00000000 8f000000 410fc075 10c5387d 00000000 00000000 00000000 00000000
> [ 64.705788] [<c03bd734>] (call_timer_fn) from [<c03bd95c>] (expire_timers+0xd8/0x144)
> [ 64.713579] [<c03bd95c>] (expire_timers) from [<c03bdca4>] (run_timer_softirq+0xe4/0x1dc)
> [ 64.721716] [<c03bdca4>] (run_timer_softirq) from [<c0302298>] (__do_softirq+0x130/0x3c8)
> [ 64.729854] [<c0302298>] (__do_softirq) from [<c034c784>] (irq_exit+0xbc/0xd8)
> [ 64.737040] [<c034c784>] (irq_exit) from [<c039fb50>] (__handle_domain_irq+0x60/0xb4)
> [ 64.744833] [<c039fb50>] (__handle_domain_irq) from [<c0709ca8>] (gic_handle_irq+0x58/0x9c)
> [ 64.753143] [<c0709ca8>] (gic_handle_irq) from [<c0301a8c>] (__irq_svc+0x6c/0x90)
> [ 64.760583] Exception stack(0xc1901f10 to 0xc1901f58)
> [ 64.765605] 1f00: 00000000 000070a0 eb6ac1a0 c031da60
> [ 64.773740] 1f20: ffffe000 c19064ac c19064f0 00000001 00000000 c1906488 c1876ba8 00000000
> [ 64.781873] 1f40: ffffffff c1901f60 c030919c c03091a0 60010013 ffffffff
> [ 64.788456] [<c0301a8c>] (__irq_svc) from [<c03091a0>] (arch_cpu_idle+0x38/0x3c)
> [ 64.795816] [<c03091a0>] (arch_cpu_idle) from [<c0376aa4>] (do_idle+0x1bc/0x298)
> [ 64.803175] [<c0376aa4>] (do_idle) from [<c0376e1c>] (cpu_startup_entry+0x18/0x1c)
> [ 64.810707] [<c0376e1c>] (cpu_startup_entry) from [<c1700ea8>] (start_kernel+0x480/0x4ac)
> [ 64.818839] Code: bad PC value
> [ 64.821890] ---[ end trace e226ed97b1c584cd ]---
> [ 64.826482] Kernel panic - not syncing: Fatal exception in interrupt
> [ 64.832807] CPU1: stopping
> [ 64.835501] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D 5.2.0-rc5-01634-ge3a2773ba9e5 #1246
> [ 64.845013] Hardware name: Freescale LS1021A
> [ 64.849266] [<c0312394>] (unwind_backtrace) from [<c030cc74>] (show_stack+0x10/0x14)
> [ 64.856972] [<c030cc74>] (show_stack) from [<c0ff4138>] (dump_stack+0xb4/0xc8)
> [ 64.864159] [<c0ff4138>] (dump_stack) from [<c0310854>] (handle_IPI+0x3bc/0x3dc)
> [ 64.871519] [<c0310854>] (handle_IPI) from [<c0709ce8>] (gic_handle_irq+0x98/0x9c)
> [ 64.879050] [<c0709ce8>] (gic_handle_irq) from [<c0301a8c>] (__irq_svc+0x6c/0x90)
> [ 64.886489] Exception stack(0xea8cbf60 to 0xea8cbfa8)
> [ 64.891514] bf60: 00000000 0000307c eb6c11a0 c031da60 ffffe000 c19064ac c19064f0 00000002
> [ 64.899649] bf80: 00000000 c1906488 c1876ba8 00000000 00000000 ea8cbfb0 c030919c c03091a0
> [ 64.907780] bfa0: 600d0013 ffffffff
> [ 64.911250] [<c0301a8c>] (__irq_svc) from [<c03091a0>] (arch_cpu_idle+0x38/0x3c)
> [ 64.918609] [<c03091a0>] (arch_cpu_idle) from [<c0376aa4>] (do_idle+0x1bc/0x298)
> [ 64.925967] [<c0376aa4>] (do_idle) from [<c0376e1c>] (cpu_startup_entry+0x18/0x1c)
> [ 64.933496] [<c0376e1c>] (cpu_startup_entry) from [<803025cc>] (0x803025cc)
> [ 64.940422] Rebooting in 3 seconds..
>
> In this case, what happened is that the DSA driver failed to probe at
> boot time due to a PHY issue during phylink_connect_phy:
>
> [ 2.245607] fsl-gianfar soc:ethernet@2d90000 eth2: error -19 setting up slave phy
> [ 2.258051] sja1105 spi0.1: failed to create slave for port 0.0
>
> Fixes: bb77f36ac21d ("net: dsa: sja1105: Add support for the PTP clock")
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next 17/18] ionic: Add RSS support
From: Jakub Kicinski @ 2019-06-26 0:20 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-18-snelson@pensando.io>
On Thu, 20 Jun 2019 13:24:23 -0700, Shannon Nelson wrote:
> +static int ionic_lif_rss_init(struct lif *lif)
> +{
> + static const u8 toeplitz_symmetric_key[] = {
> + 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A,
> + 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A,
> + 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A,
> + 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A,
> + 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A, 0x6D, 0x5A,
> + };
netdev_rss_key_fill()
> + unsigned int i, tbl_sz;
> +
> + lif->rss_types = IONIC_RSS_TYPE_IPV4 |
> + IONIC_RSS_TYPE_IPV4_TCP |
> + IONIC_RSS_TYPE_IPV4_UDP |
> + IONIC_RSS_TYPE_IPV6 |
> + IONIC_RSS_TYPE_IPV6_TCP |
> + IONIC_RSS_TYPE_IPV6_UDP;
> +
> + /* Fill indirection table with 'default' values */
> + tbl_sz = le16_to_cpu(lif->ionic->ident.lif.eth.rss_ind_tbl_sz);
> + for (i = 0; i < tbl_sz; i++)
> + lif->rss_ind_tbl[i] = i % lif->nxqs;
ethtool_rxfh_indir_default()
> + return ionic_lif_rss_config(lif, lif->rss_types,
> + toeplitz_symmetric_key, NULL);
> +}
^ permalink raw reply
* Re: [PATCH net-next 01/10] net: dsa: sja1105: Build PTP support in main DSA driver
From: Willem de Bruijn @ 2019-06-26 0:25 UTC (permalink / raw)
To: Vladimir Oltean
Cc: f.fainelli, vivien.didelot, andrew, David Miller,
Network Development
In-Reply-To: <20190625233942.1946-2-olteanv@gmail.com>
On Tue, Jun 25, 2019 at 7:40 PM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> As Arnd Bergmann pointed out in commit 78fe8a28fb96 ("net: dsa: sja1105:
> fix ptp link error"), there is no point in having PTP support as a
> separate loadable kernel module.
>
> So remove the exported symbols and make sja1105.ko contain PTP support
> or not based on CONFIG_NET_DSA_SJA1105_PTP.
>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* [PATCH bpf-next] bpf: fix compiler warning with CONFIG_MODULES=n
From: Yonghong Song @ 2019-06-26 0:35 UTC (permalink / raw)
To: ast, daniel, netdev, bpf; +Cc: kernel-team, arnd
With CONFIG_MODULES=n, the following compiler warning occurs:
/data/users/yhs/work/net-next/kernel/trace/bpf_trace.c:605:13: warning:
‘do_bpf_send_signal’ defined but not used [-Wunused-function]
static void do_bpf_send_signal(struct irq_work *entry)
The __init function send_signal_irq_work_init(), which calls
do_bpf_send_signal(), is defined under CONFIG_MODULES. Hence,
when CONFIG_MODULES=n, nobody calls static function do_bpf_send_signal(),
hence the warning.
The init function send_signal_irq_work_init() should work without
CONFIG_MODULES. Moving it out of CONFIG_MODULES
code section fixed the compiler warning, and also make bpf_send_signal()
helper work without CONFIG_MODULES.
Fixes: 8b401f9ed244 ("bpf: implement bpf_send_signal() helper")
Reported-By: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/trace/bpf_trace.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index c102c240bb0b..ca1255d14576 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1431,6 +1431,20 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
return err;
}
+static int __init send_signal_irq_work_init(void)
+{
+ int cpu;
+ struct send_signal_irq_work *work;
+
+ for_each_possible_cpu(cpu) {
+ work = per_cpu_ptr(&send_signal_work, cpu);
+ init_irq_work(&work->irq_work, do_bpf_send_signal);
+ }
+ return 0;
+}
+
+subsys_initcall(send_signal_irq_work_init);
+
#ifdef CONFIG_MODULES
static int bpf_event_notify(struct notifier_block *nb, unsigned long op,
void *module)
@@ -1478,18 +1492,5 @@ static int __init bpf_event_init(void)
return 0;
}
-static int __init send_signal_irq_work_init(void)
-{
- int cpu;
- struct send_signal_irq_work *work;
-
- for_each_possible_cpu(cpu) {
- work = per_cpu_ptr(&send_signal_work, cpu);
- init_irq_work(&work->irq_work, do_bpf_send_signal);
- }
- return 0;
-}
-
fs_initcall(bpf_event_init);
-subsys_initcall(send_signal_irq_work_init);
#endif /* CONFIG_MODULES */
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v5 2/2] bpf: Add selftests for bpf_perf_event_output
From: allanzhang @ 2019-06-26 0:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
In-Reply-To: <20190626003852.163986-1-allanzhang@google.com>
Software event output is only enabled by a few prog types.
This test is to ensure that all supported types are enabled for
bpf_perf_event_output successfully.
Signed-off-by: allan zhang <allanzhang@google.com>
---
tools/testing/selftests/bpf/test_verifier.c | 12 ++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
2 files changed, 105 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..5e98d7c37eb7 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -50,7 +50,7 @@
#define MAX_INSNS BPF_MAXINSNS
#define MAX_TEST_INSNS 1000000
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS 18
+#define MAX_NR_MAPS 19
#define MAX_TEST_RUNS 8
#define POINTER_VALUE 0xcafe4all
#define TEST_DATA_LEN 64
@@ -84,6 +84,7 @@ struct bpf_test {
int fixup_map_array_wo[MAX_FIXUPS];
int fixup_map_array_small[MAX_FIXUPS];
int fixup_sk_storage_map[MAX_FIXUPS];
+ int fixup_map_event_output[MAX_FIXUPS];
const char *errstr;
const char *errstr_unpriv;
uint32_t retval, retval_unpriv, insn_processed;
@@ -627,6 +628,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_array_wo = test->fixup_map_array_wo;
int *fixup_map_array_small = test->fixup_map_array_small;
int *fixup_sk_storage_map = test->fixup_sk_storage_map;
+ int *fixup_map_event_output = test->fixup_map_event_output;
if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -788,6 +790,14 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
fixup_sk_storage_map++;
} while (*fixup_sk_storage_map);
}
+ if (*fixup_map_event_output) {
+ map_fds[18] = __create_map(BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ sizeof(int), sizeof(int), 1, 0);
+ do {
+ prog[*fixup_map_event_output].imm = map_fds[18];
+ fixup_map_event_output++;
+ } while (*fixup_map_event_output);
+ }
}
static int set_admin(bool admin)
diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
new file mode 100644
index 000000000000..059cc70addf9
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/event_output.c
@@ -0,0 +1,94 @@
+/* instructions used to output a skb based software event, produced
+ * from code snippet:
+ * struct TMP {
+ * uint64_t tmp;
+ * } tt;
+ * tt.tmp = 5;
+ * bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
+ * &tt, sizeof(tt));
+ * return 1;
+ *
+ * the bpf assembly from llvm is:
+ * 0: b7 02 00 00 05 00 00 00 r2 = 5
+ * 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
+ * 2: bf a4 00 00 00 00 00 00 r4 = r10
+ * 3: 07 04 00 00 f8 ff ff ff r4 += -8
+ * 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
+ * 6: b7 03 00 00 00 00 00 00 r3 = 0
+ * 7: b7 05 00 00 08 00 00 00 r5 = 8
+ * 8: 85 00 00 00 19 00 00 00 call 25
+ * 9: b7 00 00 00 01 00 00 00 r0 = 1
+ * 10: 95 00 00 00 00 00 00 00 exit
+ *
+ * The reason I put the code here instead of fill_helpers is that map fixup
+ * is against the insns, instead of filled prog.
+*/
+
+#define __PERF_EVENT_INSNS__ \
+ BPF_MOV64_IMM(BPF_REG_2, 5), \
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
+ BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), \
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8), \
+ BPF_LD_MAP_FD(BPF_REG_2, 0), \
+ BPF_MOV64_IMM(BPF_REG_3, 0), \
+ BPF_MOV64_IMM(BPF_REG_5, 8), \
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, \
+ BPF_FUNC_perf_event_output), \
+ BPF_MOV64_IMM(BPF_REG_0, 1), \
+ BPF_EXIT_INSN(),
+{
+ "perfevent for sockops",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCK_OPS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for tc",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for lwt out",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_LWT_OUT,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for xdp",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_XDP,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for socket filter",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for sk_skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SK_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for cgroup skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v6 1/2] bpf: Allow bpf_skb_event_output for a few prog types
From: allanzhang @ 2019-06-26 0:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
In-Reply-To: <20190626003852.163986-1-allanzhang@google.com>
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
Test bpf code is generated from code snippet:
struct TMP {
uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
&tt, sizeof(tt));
return 1;
the bpf assembly from llvm is:
0: b7 02 00 00 05 00 00 00 r2 = 5
1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
2: bf a4 00 00 00 00 00 00 r4 = r10
3: 07 04 00 00 f8 ff ff ff r4 += -8
4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
6: b7 03 00 00 00 00 00 00 r3 = 0
7: b7 05 00 00 08 00 00 00 r5 = 8
8: 85 00 00 00 19 00 00 00 call 25
9: b7 00 00 00 01 00 00 00 r0 = 1
10: 95 00 00 00 00 00 00 00 exit
Signed-off-by: allan zhang <allanzhang@google.com>
---
net/core/filter.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..b75fcf412628 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5958,6 +5958,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_socket_cookie_proto;
case BPF_FUNC_get_socket_uid:
return &bpf_get_socket_uid_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
default:
return bpf_base_func_proto(func_id);
}
@@ -5978,6 +5980,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_storage_get_proto;
case BPF_FUNC_sk_storage_delete:
return &bpf_sk_storage_delete_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_SOCK_CGROUP_DATA
case BPF_FUNC_skb_cgroup_id:
return &bpf_skb_cgroup_id_proto;
@@ -6226,6 +6230,8 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_redirect_map_proto;
case BPF_FUNC_sk_redirect_hash:
return &bpf_sk_redirect_hash_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_INET
case BPF_FUNC_sk_lookup_tcp:
return &bpf_sk_lookup_tcp_proto;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCH bpf-next v6 0/2] bpf: Allow bpf_skb_event_output for more prog types
From: allanzhang @ 2019-06-26 0:38 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
Cc: allanzhang
Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.
More prog types are enabled to access bpf_skb_event_output in this
patch.
v2 changes:
Reformating log message.
v3 changes:
Reformating log message.
v4 changes:
Reformating log message.
v5 changes:
Fix typos, reformat comments in event_output.c, move revision history to
cover letter.
v6 changes:
fix Signed-off-by, fix fixup map creation.
allanzhang (2):
bpf: Allow bpf_skb_event_output for a few prog types
bpf: Add selftests for bpf_perf_event_output
net/core/filter.c | 6 ++
tools/testing/selftests/bpf/test_verifier.c | 12 ++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
3 files changed, 111 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH v4 0/7] Hexdump Enhancements
From: Alastair D'Silva @ 2019-06-26 1:02 UTC (permalink / raw)
To: Joe Perches
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <3ae4c1a4a72f8ee6b75c45adfbe543fc0a7b5da1.camel@perches.com>
On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > Apologies for the large CC list, it's a heads up for those
> > responsible
> > for subsystems where a prototype change in generic code causes a
> > change
> > in those subsystems.
> []
> > The default behaviour of hexdump is unchanged, however, the
> > prototype
> > for hex_dump_to_buffer() has changed, and print_hex_dump() has been
> > renamed to print_hex_dump_ext(), with a wrapper replacing it for
> > compatibility with existing code, which would have been too
> > invasive to
> > change.
>
> I believe this cover letter is misleading.
>
> The point of the wrapper is to avoid unnecessary changes
> in existing
> code.
>
>
The wrapper is for print_hex_dump(), which has many callers.
The changes to existing code are for hex_dump_to_buffer(), which is
called in relatively few places.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva
Twitter: @EvilDeece
blog: http://alastair.d-silva.org
^ permalink raw reply
* Re: [PATCH] xsk: Properly terminate assignment in xskq_produce_flush_desc
From: Song Liu @ 2019-06-26 1:13 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Björn Töpel, Magnus Karlsson, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend, Networking, bpf,
xdp-newbies, open list, clang-built-linux, Nick Desaulniers,
Nathan Huckleberry
In-Reply-To: <20190625182352.13918-1-natechancellor@gmail.com>
On Tue, Jun 25, 2019 at 12:54 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns:
>
> In file included from net/xdp/xsk_queue.c:10:
> net/xdp/xsk_queue.h:292:2: warning: expression result unused
> [-Wunused-value]
> WRITE_ONCE(q->ring->producer, q->prod_tail);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/compiler.h:284:6: note: expanded from macro 'WRITE_ONCE'
> __u.__val; \
> ~~~ ^~~~~
> 1 warning generated.
>
> The q->prod_tail assignment has a comma at the end, not a semi-colon.
> Fix that so clang no longer warns and everything works as expected.
>
> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> Link: https://github.com/ClangBuiltLinux/linux/issues/544
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Cc: <stable@vger.kernel.org> # v4.18+
Acked-by: Song Liu <songliubraving@fb.com>
Thanks for the fix!
> ---
> net/xdp/xsk_queue.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index 88b9ae24658d..cba4a640d5e8 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -288,7 +288,7 @@ static inline void xskq_produce_flush_desc(struct xsk_queue *q)
> /* Order producer and data */
> smp_wmb(); /* B, matches C */
>
> - q->prod_tail = q->prod_head,
> + q->prod_tail = q->prod_head;
> WRITE_ONCE(q->ring->producer, q->prod_tail);
> }
>
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix compiler warning with CONFIG_MODULES=n
From: Song Liu @ 2019-06-26 1:16 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, Daniel Borkmann, Networking,
bpf@vger.kernel.org, Kernel Team, arnd@arndb.de
In-Reply-To: <20190626003503.1985698-1-yhs@fb.com>
> On Jun 25, 2019, at 5:35 PM, Yonghong Song <yhs@fb.com> wrote:
>
> With CONFIG_MODULES=n, the following compiler warning occurs:
> /data/users/yhs/work/net-next/kernel/trace/bpf_trace.c:605:13: warning:
> ‘do_bpf_send_signal’ defined but not used [-Wunused-function]
> static void do_bpf_send_signal(struct irq_work *entry)
>
> The __init function send_signal_irq_work_init(), which calls
> do_bpf_send_signal(), is defined under CONFIG_MODULES. Hence,
> when CONFIG_MODULES=n, nobody calls static function do_bpf_send_signal(),
> hence the warning.
>
> The init function send_signal_irq_work_init() should work without
> CONFIG_MODULES. Moving it out of CONFIG_MODULES
> code section fixed the compiler warning, and also make bpf_send_signal()
> helper work without CONFIG_MODULES.
>
> Fixes: 8b401f9ed244 ("bpf: implement bpf_send_signal() helper")
> Reported-By: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Yonghong Song <yhs@fb.com>
Thanks for the fix!
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> kernel/trace/bpf_trace.c | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index c102c240bb0b..ca1255d14576 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1431,6 +1431,20 @@ int bpf_get_perf_event_info(const struct perf_event *event, u32 *prog_id,
> return err;
> }
>
> +static int __init send_signal_irq_work_init(void)
> +{
> + int cpu;
> + struct send_signal_irq_work *work;
> +
> + for_each_possible_cpu(cpu) {
> + work = per_cpu_ptr(&send_signal_work, cpu);
> + init_irq_work(&work->irq_work, do_bpf_send_signal);
> + }
> + return 0;
> +}
> +
> +subsys_initcall(send_signal_irq_work_init);
> +
> #ifdef CONFIG_MODULES
> static int bpf_event_notify(struct notifier_block *nb, unsigned long op,
> void *module)
> @@ -1478,18 +1492,5 @@ static int __init bpf_event_init(void)
> return 0;
> }
>
> -static int __init send_signal_irq_work_init(void)
> -{
> - int cpu;
> - struct send_signal_irq_work *work;
> -
> - for_each_possible_cpu(cpu) {
> - work = per_cpu_ptr(&send_signal_work, cpu);
> - init_irq_work(&work->irq_work, do_bpf_send_signal);
> - }
> - return 0;
> -}
> -
> fs_initcall(bpf_event_init);
> -subsys_initcall(send_signal_irq_work_init);
> #endif /* CONFIG_MODULES */
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-06-26 1:27 UTC (permalink / raw)
To: Joe Perches
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <3340b520a57e00a483eae170be97316c8d18c22c.camel@perches.com>
On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > In order to support additional features, rename hex_dump_to_buffer
> > to
> > hex_dump_to_buffer_ext, and replace the ascii bool parameter with
> > flags.
> []
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/intel_engine_cs.c
> []
> > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m,
> > const void *buf, size_t len)
> > }
> >
> > WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> > - rowsize, sizeof(u32),
> > - line, sizeof(line),
> > - false) >=
> > sizeof(line));
> > + rowsize, sizeof(u32),
> > line,
> > + sizeof(line)) >=
> > sizeof(line));
>
> Huh? Why do this?
The ascii parameter was removed from the simple API as per Jani's
suggestion. The remainder was reformatted to avoid exceeding the line
length limits.
>
> > diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c
> > b/drivers/isdn/hardware/mISDN/mISDNisar.c
> []
> > @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg,
> > u8 len, u8 *msg)
> > int l = 0;
> >
> > while (l < (int)len) {
> > - hex_dump_to_buffer(msg + l, len - l,
> > 32, 1,
> > - isar->log, 256, 1);
> > + hex_dump_to_buffer_ext(msg + l, len -
> > l, 32, 1,
> > + isar->log, 256,
> > + HEXDUMP_ASCII);
>
> Again, why do any of these?
>
> The point of the wrapper is to avoid changing these.
Jani made a pretty good point that about half the callers didn't want
an ASCII dump, and presenting a simplified API makes sense.
I would actually put forward that we consider dropping rowsize from the
simplified API too, as most callers use 32, and those that use 16 would
probably be OK with 32.
Your proposal, on the other hand, only makes sense if there were many
callers, and even so, not in the form that you presented, since that
result in a mix of booleans & bitfields that you were critical of.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva
Twitter: @EvilDeece
blog: http://alastair.d-silva.org
^ permalink raw reply
* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-06-26 1:27 UTC (permalink / raw)
To: Joe Perches
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <4ba3b835fb3675ea685390903a082bf8b7f9e955.camel@perches.com>
On Mon, 2019-06-24 at 22:19 -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 15:06 +1000, Alastair D'Silva wrote:
> > The change actions Jani's suggestion:
> > https://lkml.org/lkml/2019/6/20/343
>
> I suggest not changing any of the existing uses of
> hex_dump_to_buffer and only use hex_dump_to_buffer_ext
> when necessary for your extended use cases.
>
>
I disagree, adding a wrapper for the benefit of avoiding touching a
handful of call sites that are easily amended would be adding technical
debt.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva
Twitter: @EvilDeece
blog: http://alastair.d-silva.org
^ permalink raw reply
* Re: [PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
From: Alastair D'Silva @ 2019-06-26 1:28 UTC (permalink / raw)
To: Joe Perches
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <c364c36338d385eba60c523828ad8995c792ae4d.camel@perches.com>
On Mon, 2019-06-24 at 22:37 -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> >
> > With the wider display format, it can become hard to identify how
> > many
> > bytes into the line you are looking at.
> >
> > The patch adds new flags to hex_dump_to_buffer() and
> > print_hex_dump() to
> > print vertical lines to separate every N groups of bytes.
> >
> > eg.
> > buf:00000000: 454d414e 43415053|4e495f45
> > 00584544 NAMESPAC|E_INDEX.
> > buf:00000010: 00000000 00000002|00000000
> > 00000000 ........|........
> >
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
> > include/linux/printk.h | 3 +++
> > lib/hexdump.c | 59 ++++++++++++++++++++++++++++++++++++
> > ------
> > 2 files changed, 54 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/printk.h b/include/linux/printk.h
> []
> > @@ -485,6 +485,9 @@ enum {
> >
> > #define HEXDUMP_ASCII BIT(0)
> > #define HEXDUMP_SUPPRESS_REPEATED BIT(1)
> > +#define HEXDUMP_2_GRP_LINES BIT(2)
> > +#define HEXDUMP_4_GRP_LINES BIT(3)
> > +#define HEXDUMP_8_GRP_LINES BIT(4)
>
> These aren't really bits as only one value should be set
> as 8 overrides 4 and 4 overrides 2.
This should be the other way around, as we should be emitting alternate
seperators based on the smallest grouping (2 implies 4 and 8, and 4
implies 8). I'll fix the logic.
I can't come up with a better way to represent these without making the
API more complex, if you have a suggestion, I'm happy to hear it.
>
> I would also expect this to be a value of 2 in your above
> example, rather than 8. It's described as groups not bytes.
>
> The example is showing a what would normally be a space ' '
> separator as a vertical bar '|' every 2nd grouping.
>
The above example shows a group size of 4 bytes, and
HEXDUMP_2_GRP_LINES set, with 2 groups being 8 bytes.
I'll make that clearer in the commit message.
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva
Twitter: @EvilDeece
blog: http://alastair.d-silva.org
^ permalink raw reply
* Re: [PATCH bpf-next v6 0/2] bpf: Allow bpf_skb_event_output for more prog types
From: Song Liu @ 2019-06-26 1:35 UTC (permalink / raw)
To: allanzhang
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, Networking, bpf, open list
In-Reply-To: <20190626003852.163986-1-allanzhang@google.com>
On Tue, Jun 25, 2019 at 5:39 PM allanzhang <allanzhang@google.com> wrote:
>
> Software event output is only enabled by a few prog types right now (TC,
> LWT out, XDP, sockops). Many other skb based prog types need
> bpf_skb_event_output to produce software event.
>
> More prog types are enabled to access bpf_skb_event_output in this
> patch.
>
> v2 changes:
> Reformating log message.
>
> v3 changes:
> Reformating log message.
>
> v4 changes:
> Reformating log message.
>
> v5 changes:
> Fix typos, reformat comments in event_output.c, move revision history to
> cover letter.
>
> v6 changes:
> fix Signed-off-by, fix fixup map creation.
>
> allanzhang (2):
I guess you manually fixed the name in the other two patches, but forgot
this one. You can fix it in your .gitconfig file. Make sure to get the proper
capital letters.
Also, please run ./scripts/checkpatch.pl on your patch files. This will help
you find a lot of issues.
Thanks,
Song
> bpf: Allow bpf_skb_event_output for a few prog types
> bpf: Add selftests for bpf_perf_event_output
>
> net/core/filter.c | 6 ++
> tools/testing/selftests/bpf/test_verifier.c | 12 ++-
> .../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
> 3 files changed, 111 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Willem de Bruijn @ 2019-06-26 1:36 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: David Miller, grygorii.strashko, hawk, brouer, saeedm, leon,
Alexei Starovoitov, linux-kernel, linux-omap, xdp-newbies,
ilias.apalodimas, Network Development, Daniel Borkmann,
jakub.kicinski, John Fastabend
In-Reply-To: <20190625175948.24771-2-ivan.khoronzhuk@linaro.org>
On Tue, Jun 25, 2019 at 2:00 PM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> Add user counter allowing to delete pool only when no users.
> It doesn't prevent pool from flush, only prevents freeing the
> pool instance. Helps when no need to delete the pool and now
> it's user responsibility to free it by calling page_pool_free()
> while destroying procedure. It also makes to use page_pool_free()
> explicitly, not fully hidden in xdp unreg, which looks more
> correct after page pool "create" routine.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index f07c518ef8a5..1ec838e9927e 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -101,6 +101,7 @@ struct page_pool {
> struct ptr_ring ring;
>
> atomic_t pages_state_release_cnt;
> + atomic_t user_cnt;
refcount_t?
> };
>
> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> return page->dma_addr;
> }
>
> +/* used to prevent pool from deallocation */
> +static inline void page_pool_get(struct page_pool *pool)
> +{
> + atomic_inc(&pool->user_cnt);
> +}
> +
> static inline bool is_page_pool_compiled_in(void)
> {
> #ifdef CONFIG_PAGE_POOL
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index b366f59885c1..169b0e3c870e 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
> return -ENOMEM;
>
> atomic_set(&pool->pages_state_release_cnt, 0);
> + atomic_set(&pool->user_cnt, 0);
>
> if (pool->p.flags & PP_FLAG_DMA_MAP)
> get_device(pool->p.dev);
> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
> kfree(pool);
> return ERR_PTR(err);
> }
> +
> + page_pool_get(pool);
> return pool;
> }
> EXPORT_SYMBOL(page_pool_create);
> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>
> void __page_pool_free(struct page_pool *pool)
> {
> + /* free only if no users */
> + if (!atomic_dec_and_test(&pool->user_cnt))
> + return;
> +
> WARN(pool->alloc.count, "API usage violation");
> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 829377cc83db..04bdcd784d2e 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>
> mutex_unlock(&mem_id_lock);
>
> + if (type == MEM_TYPE_PAGE_POOL)
> + page_pool_get(xdp_alloc->page_pool);
> +
need an analogous page_pool_put in xdp_rxq_info_unreg_mem_model? mlx5
does not use that inverse function, but intel drivers do.
> trace_mem_connect(xdp_alloc, xdp_rxq);
> return 0;
> err:
> --
> 2.17.1
>
^ permalink raw reply
* [PATCHv2 iproute2] ip/iptoken: fix dump error when ipv6 disabled
From: Hangbin Liu @ 2019-06-26 1:44 UTC (permalink / raw)
To: netdev
Cc: Daniel Borkmann, Phil Sutter, Stephen Hemminger, David Ahern,
Andrea Claudi, Hangbin Liu
In-Reply-To: <20190625093550.7804-1-liuhangbin@gmail.com>
When we disable IPv6 from the start up (ipv6.disable=1), there will be
no IPv6 route info in the dump message. If we return -1 when
ifi->ifi_family != AF_INET6, we will get error like
$ ip token list
Dump terminated
which will make user feel confused. There is no need to return -1 if the
dump message not match. Return 0 is enough.
v2: do not combine all the conditions together.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
ip/iptoken.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ip/iptoken.c b/ip/iptoken.c
index f1194c3e..9f356890 100644
--- a/ip/iptoken.c
+++ b/ip/iptoken.c
@@ -60,9 +60,9 @@ static int print_token(struct nlmsghdr *n, void *arg)
return -1;
if (ifi->ifi_family != AF_INET6)
- return -1;
+ return 0;
if (ifi->ifi_index == 0)
- return -1;
+ return 0;
if (ifindex > 0 && ifi->ifi_index != ifindex)
return 0;
if (ifi->ifi_flags & (IFF_LOOPBACK | IFF_NOARP))
--
2.19.2
^ permalink raw reply related
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Andrew Lunn @ 2019-06-26 1:52 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Russell King - ARM Linux admin, René van Dorst, sean.wang,
Florian Fainelli, David S. Miller, matthias.bgg, Vivien Didelot,
frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <CA+h21hrsosGVQczMWy1+WfyNGZCpeMFerUwvWb-z+TTjrSOP1Q@mail.gmail.com>
On Wed, Jun 26, 2019 at 02:13:25AM +0300, Vladimir Oltean wrote:
> On Wed, 26 Jun 2019 at 02:10, Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > > > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > > > Hi Russell,
> > > > > >
> > > > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > > > This should be removed - state->link is not for use in mac_config.
> > > > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > > > >
> > > > > >
> > > > > > What do you mean exactly that state->link is not for use, is that true in
> > > > > > general?
> > > > >
> > > > > Yes. mac_config() should not touch it; it is not always in a defined
> > > > > state. For example, if you set modes via ethtool (the
> > > > > ethtool_ksettings_set API) then state->link will probably contain
> > > > > zero irrespective of the true link state.
> > > > >
> > > >
> > > > Experimentally, state->link is zero at the same time as state->speed
> > > > is -1, so just ignoring !state->link made sense. This is not in-band
> > > > AN. What is your suggestion? Should I proceed to try and configure the
> > > > MAC for SPEED_UNKNOWN?
> > >
> > > What would you have done with a PHY when the link is down, what speed
> > > would you have configured in the phylib adjust_link callback? phylib
> > > also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
> > >
> >
> > With phylib, I'd make the driver ignore the speed and do nothing.
> > With phylink, I'd make the core not call mac_config.
> > But what happened is I saw phylink call mac_config anyway, said
> > 'weird' and proceeded to ignore it as I would have for phylib.
> > I'm just not understanding your position - it seems like you're
> > implying there's a bug in phylink and the function call with
> > MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
>
> I meant MLO_AN_PHY, sorry.
The MAC could go into a low power mode.
Andrew
^ permalink raw reply
* Re: [PATCH] net: phylink: further documentation clarifications
From: Florian Fainelli @ 2019-06-26 2:16 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Heiner Kallweit; +Cc: David S. Miller, netdev
In-Reply-To: <E1hfi09-0007Zs-Vb@rmk-PC.armlinux.org.uk>
On 6/25/2019 2:44 AM, Russell King wrote:
> Clarify the validate() behaviour in a few cases which weren't mentioned
> in the documentation, but which are necessary for users to get the
> correct behaviour.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v4 net-next 3/4] net: ethernet: ti: davinci_cpdma: return handler status
From: Willem de Bruijn @ 2019-06-26 2:17 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: David Miller, grygorii.strashko, hawk, brouer, saeedm, leon,
Alexei Starovoitov, linux-kernel, linux-omap, xdp-newbies,
ilias.apalodimas, Network Development, Daniel Borkmann,
jakub.kicinski, John Fastabend
In-Reply-To: <20190625175948.24771-4-ivan.khoronzhuk@linaro.org>
On Tue, Jun 25, 2019 at 2:00 PM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> This change is needed to return flush status of rx handler for
> flushing redirected xdp frames after processing channel packets.
> Do it as separate patch for simplicity.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> @@ -602,7 +605,8 @@ static int cpsw_tx_mq_poll(struct napi_struct *napi_tx, int budget)
> else
> cur_budget = txv->budget;
>
> - num_tx += cpdma_chan_process(txv->ch, cur_budget);
> + cpdma_chan_process(txv->ch, &cur_budget);
> + num_tx += cur_budget;
Less code change to add a new argument int *flush to communicate the
new state and leave the existing argument and return values as is?
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] libbpf: add perf buffer reading API
From: Song Liu @ 2019-06-26 2:18 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, Kernel Team
In-Reply-To: <20190625232601.3227055-2-andriin@fb.com>
On Tue, Jun 25, 2019 at 4:28 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
> to user space for additional processing. libbpf already has very low-level API
> to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
> use and requires a lot of code to set everything up. This patch adds
> perf_buffer abstraction on top of it, abstracting setting up and polling
> per-CPU logic into simple and convenient API, similar to what BCC provides.
>
> perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
> map entries. It accepts two user-provided callbacks: one for handling raw
> samples and one for get notifications of lost samples due to buffer overflow.
>
> perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
> utilizing epoll instance.
>
> perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
>
> All APIs are not thread-safe. User should ensure proper locking/coordination if
> used in multi-threaded set up.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Overall looks good. Some nit below.
> ---
> tools/lib/bpf/libbpf.c | 282 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 12 ++
> tools/lib/bpf/libbpf.map | 5 +-
> 3 files changed, 298 insertions(+), 1 deletion(-)
[...]
> +struct perf_buffer *perf_buffer__new(struct bpf_map *map, size_t page_cnt,
> + perf_buffer_sample_fn sample_cb,
> + perf_buffer_lost_fn lost_cb, void *ctx)
> +{
> + char msg[STRERR_BUFSIZE];
> + struct perf_buffer *pb;
> + int err, cpu;
> +
> + if (bpf_map__def(map)->type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
> + pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
> + bpf_map__name(map));
> + return ERR_PTR(-EINVAL);
> + }
> + if (bpf_map__fd(map) < 0) {
> + pr_warning("map '%s' doesn't have associated FD\n",
> + bpf_map__name(map));
> + return ERR_PTR(-EINVAL);
> + }
> + if (page_cnt & (page_cnt - 1)) {
> + pr_warning("page count should be power of two, but is %zu\n",
> + page_cnt);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pb = calloc(1, sizeof(*pb));
> + if (!pb)
> + return ERR_PTR(-ENOMEM);
> +
> + pb->sample_cb = sample_cb;
> + pb->lost_cb = lost_cb;
I think we need to check sample_cb != NULL && lost_cb != NULL.
> + pb->ctx = ctx;
> + pb->page_size = getpagesize();
> + pb->mmap_size = pb->page_size * page_cnt;
> + pb->mapfd = bpf_map__fd(map);
> +
> + pb->epfd = epoll_create1(EPOLL_CLOEXEC);
[...]
> +perf_buffer__process_record(struct perf_event_header *e, void *ctx)
> +{
> + struct perf_buffer *pb = ctx;
> + void *data = e;
> +
> + switch (e->type) {
> + case PERF_RECORD_SAMPLE: {
> + struct perf_sample_raw *s = data;
> +
> + pb->sample_cb(pb->ctx, s->data, s->size);
> + break;
> + }
> + case PERF_RECORD_LOST: {
> + struct perf_sample_lost *s = data;
> +
> + if (pb->lost_cb)
> + pb->lost_cb(pb->ctx, s->lost);
OK, we test lost_cb here, so not necessary at init time.
[...]
> bpf_program__attach_perf_event;
> bpf_program__attach_raw_tracepoint;
> bpf_program__attach_tracepoint;
> bpf_program__attach_uprobe;
> + btf__parse_elf;
Why move btf__parse_elf ?
Thanks,
Song
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] selftests/bpf: test perf buffer API
From: Song Liu @ 2019-06-26 2:21 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, Kernel Team
In-Reply-To: <20190625232601.3227055-3-andriin@fb.com>
On Tue, Jun 25, 2019 at 4:27 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Add test verifying perf buffer API functionality.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> .../selftests/bpf/prog_tests/perf_buffer.c | 86 +++++++++++++++++++
> .../selftests/bpf/progs/test_perf_buffer.c | 31 +++++++
> 2 files changed, 117 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> new file mode 100644
> index 000000000000..3ba3e26141ac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <pthread.h>
> +#include <sched.h>
> +#include <sys/socket.h>
> +#include <test_progs.h>
> +
> +static void on_sample(void *ctx, void *data, __u32 size)
> +{
> + cpu_set_t *cpu_seen = ctx;
> + int cpu = *(int *)data;
> +
> + CPU_SET(cpu, cpu_seen);
> +}
> +
> +void test_perf_buffer(void)
> +{
> + int err, prog_fd, prog_pfd, nr_cpus, i, duration = 0;
> + const char *prog_name = "kprobe/sys_nanosleep";
> + const char *file = "./test_perf_buffer.o";
> + struct bpf_map *perf_buf_map;
> + cpu_set_t cpu_set, cpu_seen;
> + struct bpf_program *prog;
> + struct bpf_object *obj;
> + struct perf_buffer *pb;
> +
> + nr_cpus = libbpf_num_possible_cpus();
> + if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
> + return;
> +
> + /* load program */
> + err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> + if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> + return;
> +
> + prog = bpf_object__find_program_by_title(obj, prog_name);
> + if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
> + goto out_close;
> +
> + /* load map */
> + perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
> + if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
> + goto out_close;
> +
> + /* attach kprobe */
> + prog_pfd = bpf_program__attach_kprobe(prog, false /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(prog_pfd < 0, "attach_kprobe", "err %d\n", prog_pfd))
> + goto out_close;
> +
> + /* set up perf buffer */
> + pb = perf_buffer__new(perf_buf_map, 1, on_sample, NULL, &cpu_seen);
> + if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
> + goto out_detach;
> +
> + /* trigger kprobe on every CPU */
> + CPU_ZERO(&cpu_seen);
> + for (i = 0; i < nr_cpus; i++) {
> + CPU_ZERO(&cpu_set);
> + CPU_SET(i, &cpu_set);
> +
> + err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
> + &cpu_set);
> + if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
> + i, err))
> + goto out_detach;
> +
> + usleep(1);
> + }
> +
> + /* read perf buffer */
> + err = perf_buffer__poll(pb, 100);
> + if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
> + goto out_free_pb;
> +
> + if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
> + "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
> + goto out_free_pb;
> +
> +out_free_pb:
> + perf_buffer__free(pb);
> +out_detach:
> + libbpf_perf_event_disable_and_close(prog_pfd);
> +out_close:
> + bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> new file mode 100644
> index 000000000000..ba961f608fd5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> + int type;
> + int key_size;
> + int value_size;
> + int max_entries;
> +} perf_buf_map SEC(".maps") = {
> + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = sizeof(int),
> + .value_size = sizeof(int),
> + .max_entries = 56,
> +};
> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> + int cpu = bpf_get_smp_processor_id();
> +
> + bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
> + &cpu, sizeof(cpu));
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH iproute2 1/2] devlink: fix format string warning for 32bit targets
From: Baruch Siach @ 2019-06-26 3:30 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jiri Pirko, netdev, Aya Levin, Moshe Shemesh
In-Reply-To: <20190625115806.01e29659@hermes.lan>
Hi Stephen,
On Tue, Jun 25, 2019 at 11:58:06AM -0700, Stephen Hemminger wrote:
> On Tue, 25 Jun 2019 14:49:04 +0300
> Baruch Siach <baruch@tkos.co.il> wrote:
>
> > diff --git a/devlink/devlink.c b/devlink/devlink.c
> > index 436935f88bda..b400fab17578 100644
> > --- a/devlink/devlink.c
> > +++ b/devlink/devlink.c
> > @@ -1726,9 +1726,9 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
> > jsonw_u64_field(dl->jw, name, val);
> > } else {
> > if (g_indent_newline)
> > - pr_out("%s %lu", name, val);
> > + pr_out("%s %llu", name, val);
> > else
> > - pr_out(" %s %lu", name, val);
> > + pr_out(" %s %llu", name, val);
>
> But on 64 bit target %llu expects unsigned long long which is 128bit.
Is that a problem?
> The better way to fix this is to use:
> #include <inttypes.h>
>
> And the use the macro PRIu64
> pr_out(" %s %"PRIu64, name, val);
I think it makes the code harder to read. But OK, I'll post an update to this
patch and the next.
Thanks,
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: sched: protect against stack overflow in TC act_mirred
From: Eyal Birger @ 2019-06-26 4:37 UTC (permalink / raw)
To: Jamal Hadi Salim, John Hurley
Cc: Linux Netdev List, David Miller, Florian Westphal, Simon Horman,
Jakub Kicinski, oss-drivers, shmulik
In-Reply-To: <4a4f2f81-d87a-2a45-36b9-ac101d937219@mojatatu.com>
Hi Jamal, John,
On Tue, 25 Jun 2019 07:24:37 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 2019-06-25 5:06 a.m., John Hurley wrote:
> > On Tue, Jun 25, 2019 at 9:30 AM Eyal Birger <eyal.birger@gmail.com>
> > wrote:
>
> > I'm not sure on the history of why a value of 4 was selected here
> > but it seems to fall into line with my findings.
>
> Back then we could only loop in one direction (as opposed to two right
> now) - so seeing something twice would have been suspect enough,
> so 4 seems to be a good number. I still think 4 is a good number.
I think the introduction of mirred ingress affects the 'seeing something
twice is suspicious' paradigm - see below.
> > Is there a hard requirement for >4 recursive calls here?
>
> I think this is where testcases help (which then get permanently
> added in tdc repository). Eyal - if you have a test scenario where
> this could be demonstrated it would help.
I don't have a _hard_ requirement for >4 recursive calls.
I did encounter use cases for 2 layers of stacked net devices using TC
mirred ingress. For example, first layer redirects traffic based on
incoming protocol - e.g. some tunneling criterion - and the second
layer redirects traffic based on the IP packet src/dst, something like:
+-----------+ +-----------+ +-----------+ +-----------+
| ip0 | | ip1 | | ip2 | | ip3 |
+-----------+ +-----------+ +-----------+ +-----------+
\ / \ /
\ / \ /
+-----------+ +-----------+
| proto0 | | proto1 |
+-----------+ +-----------+
\ /
\ /
+-----------+
| eth0 |
+-----------+
Where packets stem from eth0 and are redirected to the appropriate devices
using mirred ingress redirect with different criteria.
This is useful for example when each 'ip' device is part of a different
routing domain.
There are probably many other ways to do this kind of thing, but using mirred
ingress to demux the traffic provides freedom in the demux criteria while
having the benefit of a netdevice at each node allowing to use tcpdump and
other such facilities.
As such, I was concerned that a hard limit of 4 may be restrictive.
I too think Florian's suggestion of using netif_rx() in order to break
the recursion when limit is met (or always use it?) is a good approach
to try in order not to force restrictions while keeping the stack sane.
Eyal.
^ permalink raw reply
* Re: kernel panic: stack is corrupted in validate_chain
From: syzbot @ 2019-06-26 4:41 UTC (permalink / raw)
To: ast, daniel, john.fastabend, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <000000000000c7a272058c2cde21@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11d4e129a00000
start commit: 249155c2 Merge branch 'parisc-5.2-4' of git://git.kernel.o..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=13d4e129a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=15d4e129a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=9a31528e58cc12e2
dashboard link: https://syzkaller.appspot.com/bug?extid=6ba34346b252f2d497c7
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=135e34eea00000
Reported-by: syzbot+6ba34346b252f2d497c7@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ 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