* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Jesper Dangaard Brouer @ 2019-07-02 18:29 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap, brouer
In-Reply-To: <20190702152112.GG4510@khorivan>
On Tue, 2 Jul 2019 18:21:13 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> On Tue, Jul 02, 2019 at 05:10:29PM +0200, Jesper Dangaard Brouer wrote:
> >On Tue, 2 Jul 2019 17:56:13 +0300
> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >
> >> On Tue, Jul 02, 2019 at 04:52:30PM +0200, Jesper Dangaard Brouer wrote:
> >> >On Tue, 2 Jul 2019 17:44:27 +0300
> >> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >> >
> >> >> On Tue, Jul 02, 2019 at 04:31:39PM +0200, Jesper Dangaard Brouer wrote:
> >> >> >From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> >> >
> >> >> >Jesper recently removed page_pool_destroy() (from driver invocation) and
> >> >> >moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
> >> >> >handle in-flight packets/pages. This created an asymmetry in drivers
> >> >> >create/destroy pairs.
> >> >> >
> >> >> >This patch add page_pool user refcnt and reintroduce page_pool_destroy.
> >> >> >This serves two purposes, (1) simplify drivers error handling as driver now
> >> >> >drivers always calls page_pool_destroy() and don't need to track if
> >> >> >xdp_rxq_info_reg_mem_model() was unsuccessful. (2) allow special cases
> >> >> >where a single RX-queue (with a single page_pool) provides packets for two
> >> >> >net_device'es, and thus needs to register the same page_pool twice with two
> >> >> >xdp_rxq_info structures.
> >> >>
> >> >> As I tend to use xdp level patch there is no more reason to mention (2) case
> >> >> here. XDP patch serves it better and can prevent not only obj deletion but also
> >> >> pool flush, so, this one patch I could better leave only for (1) case.
> >> >
> >> >I don't understand what you are saying.
> >> >
> >> >Do you approve this patch, or do you reject this patch?
> >> >
> >> It's not reject, it's proposition to use both, XDP and page pool patches,
> >> each having its goal.
> >
> >Just to be clear, if you want this patch to get accepted you have to
> >reply with your Signed-off-by (as I wrote).
> >
> >Maybe we should discuss it in another thread, about why you want two
> >solutions to the same problem.
>
> If it solves same problem I propose to reject this one and use this:
> https://lkml.org/lkml/2019/7/2/651
No, I propose using this one, and rejecting the other one.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH bpf-next] selftests: bpf: standardize to static __always_inline
From: Jiri Benc @ 2019-07-02 18:26 UTC (permalink / raw)
To: netdev, bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Song Liu, Y Song
The progs for bpf selftests use several different notations to force
function inlining. Standardize to what most of them use,
static __always_inline.
Suggested-by: Song Liu <liu.song.a23@gmail.com>
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
tools/testing/selftests/bpf/progs/pyperf.h | 9 +++--
.../testing/selftests/bpf/progs/strobemeta.h | 36 ++++++++++---------
.../testing/selftests/bpf/progs/test_jhash.h | 3 +-
.../selftests/bpf/progs/test_seg6_loop.c | 23 ++++++------
.../selftests/bpf/progs/test_verif_scale2.c | 2 +-
5 files changed, 38 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/pyperf.h b/tools/testing/selftests/bpf/progs/pyperf.h
index 6b0781391be5..abf6224649be 100644
--- a/tools/testing/selftests/bpf/progs/pyperf.h
+++ b/tools/testing/selftests/bpf/progs/pyperf.h
@@ -75,8 +75,7 @@ typedef struct {
void* co_name; // PyCodeObject.co_name
} FrameData;
-static inline __attribute__((__always_inline__)) void*
-get_thread_state(void* tls_base, PidData* pidData)
+static __always_inline void *get_thread_state(void *tls_base, PidData *pidData)
{
void* thread_state;
int key;
@@ -87,8 +86,8 @@ get_thread_state(void* tls_base, PidData* pidData)
return thread_state;
}
-static inline __attribute__((__always_inline__)) bool
-get_frame_data(void* frame_ptr, PidData* pidData, FrameData* frame, Symbol* symbol)
+static __always_inline bool get_frame_data(void *frame_ptr, PidData *pidData,
+ FrameData *frame, Symbol *symbol)
{
// read data from PyFrameObject
bpf_probe_read(&frame->f_back,
@@ -161,7 +160,7 @@ struct bpf_elf_map SEC("maps") stackmap = {
.max_elem = 1000,
};
-static inline __attribute__((__always_inline__)) int __on_event(struct pt_regs *ctx)
+static __always_inline int __on_event(struct pt_regs *ctx)
{
uint64_t pid_tgid = bpf_get_current_pid_tgid();
pid_t pid = (pid_t)(pid_tgid >> 32);
diff --git a/tools/testing/selftests/bpf/progs/strobemeta.h b/tools/testing/selftests/bpf/progs/strobemeta.h
index 1ff73f60a3e4..553bc3b62e89 100644
--- a/tools/testing/selftests/bpf/progs/strobemeta.h
+++ b/tools/testing/selftests/bpf/progs/strobemeta.h
@@ -266,8 +266,8 @@ struct tls_index {
uint64_t offset;
};
-static inline __attribute__((always_inline))
-void *calc_location(struct strobe_value_loc *loc, void *tls_base)
+static __always_inline void *calc_location(struct strobe_value_loc *loc,
+ void *tls_base)
{
/*
* tls_mode value is:
@@ -327,10 +327,10 @@ void *calc_location(struct strobe_value_loc *loc, void *tls_base)
: NULL;
}
-static inline __attribute__((always_inline))
-void read_int_var(struct strobemeta_cfg *cfg, size_t idx, void *tls_base,
- struct strobe_value_generic *value,
- struct strobemeta_payload *data)
+static __always_inline void read_int_var(struct strobemeta_cfg *cfg,
+ size_t idx, void *tls_base,
+ struct strobe_value_generic *value,
+ struct strobemeta_payload *data)
{
void *location = calc_location(&cfg->int_locs[idx], tls_base);
if (!location)
@@ -342,10 +342,11 @@ void read_int_var(struct strobemeta_cfg *cfg, size_t idx, void *tls_base,
data->int_vals_set_mask |= (1 << idx);
}
-static inline __attribute__((always_inline))
-uint64_t read_str_var(struct strobemeta_cfg* cfg, size_t idx, void *tls_base,
- struct strobe_value_generic *value,
- struct strobemeta_payload *data, void *payload)
+static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,
+ size_t idx, void *tls_base,
+ struct strobe_value_generic *value,
+ struct strobemeta_payload *data,
+ void *payload)
{
void *location;
uint32_t len;
@@ -371,10 +372,11 @@ uint64_t read_str_var(struct strobemeta_cfg* cfg, size_t idx, void *tls_base,
return len;
}
-static inline __attribute__((always_inline))
-void *read_map_var(struct strobemeta_cfg *cfg, size_t idx, void *tls_base,
- struct strobe_value_generic *value,
- struct strobemeta_payload* data, void *payload)
+static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
+ size_t idx, void *tls_base,
+ struct strobe_value_generic *value,
+ struct strobemeta_payload *data,
+ void *payload)
{
struct strobe_map_descr* descr = &data->map_descrs[idx];
struct strobe_map_raw map;
@@ -435,9 +437,9 @@ void *read_map_var(struct strobemeta_cfg *cfg, size_t idx, void *tls_base,
* read_strobe_meta returns NULL, if no metadata was read; otherwise returns
* pointer to *right after* payload ends
*/
-static inline __attribute__((always_inline))
-void *read_strobe_meta(struct task_struct* task,
- struct strobemeta_payload* data) {
+static __always_inline void *read_strobe_meta(struct task_struct *task,
+ struct strobemeta_payload *data)
+{
pid_t pid = bpf_get_current_pid_tgid() >> 32;
struct strobe_value_generic value = {0};
struct strobemeta_cfg *cfg;
diff --git a/tools/testing/selftests/bpf/progs/test_jhash.h b/tools/testing/selftests/bpf/progs/test_jhash.h
index 3d12c11a8d47..c300734d26f6 100644
--- a/tools/testing/selftests/bpf/progs/test_jhash.h
+++ b/tools/testing/selftests/bpf/progs/test_jhash.h
@@ -1,9 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2019 Facebook
+#include <features.h>
typedef unsigned int u32;
-static __attribute__((always_inline)) u32 rol32(u32 word, unsigned int shift)
+static __always_inline u32 rol32(u32 word, unsigned int shift)
{
return (word << shift) | (word >> ((-shift) & 31));
}
diff --git a/tools/testing/selftests/bpf/progs/test_seg6_loop.c b/tools/testing/selftests/bpf/progs/test_seg6_loop.c
index 463964d79f73..1dbe1d4d467e 100644
--- a/tools/testing/selftests/bpf/progs/test_seg6_loop.c
+++ b/tools/testing/selftests/bpf/progs/test_seg6_loop.c
@@ -54,7 +54,7 @@ struct sr6_tlv_t {
unsigned char value[0];
} BPF_PACKET_HEADER;
-static __attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
+static __always_inline struct ip6_srh_t *get_srh(struct __sk_buff *skb)
{
void *cursor, *data_end;
struct ip6_srh_t *srh;
@@ -88,9 +88,9 @@ static __attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff
return srh;
}
-static __attribute__((always_inline))
-int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
- uint32_t old_pad, uint32_t pad_off)
+static __always_inline int update_tlv_pad(struct __sk_buff *skb,
+ uint32_t new_pad, uint32_t old_pad,
+ uint32_t pad_off)
{
int err;
@@ -118,10 +118,11 @@ int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
return 0;
}
-static __attribute__((always_inline))
-int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
- uint32_t *tlv_off, uint32_t *pad_size,
- uint32_t *pad_off)
+static __always_inline int is_valid_tlv_boundary(struct __sk_buff *skb,
+ struct ip6_srh_t *srh,
+ uint32_t *tlv_off,
+ uint32_t *pad_size,
+ uint32_t *pad_off)
{
uint32_t srh_off, cur_off;
int offset_valid = 0;
@@ -177,9 +178,9 @@ int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
return 0;
}
-static __attribute__((always_inline))
-int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
- struct sr6_tlv_t *itlv, uint8_t tlv_size)
+static __always_inline int add_tlv(struct __sk_buff *skb,
+ struct ip6_srh_t *srh, uint32_t tlv_off,
+ struct sr6_tlv_t *itlv, uint8_t tlv_size)
{
uint32_t srh_off = (char *)srh - (char *)(long)skb->data;
uint8_t len_remaining, new_pad;
diff --git a/tools/testing/selftests/bpf/progs/test_verif_scale2.c b/tools/testing/selftests/bpf/progs/test_verif_scale2.c
index 77830693eccb..9897150ed516 100644
--- a/tools/testing/selftests/bpf/progs/test_verif_scale2.c
+++ b/tools/testing/selftests/bpf/progs/test_verif_scale2.c
@@ -2,7 +2,7 @@
// Copyright (c) 2019 Facebook
#include <linux/bpf.h>
#include "bpf_helpers.h"
-#define ATTR __attribute__((always_inline))
+#define ATTR __always_inline
#include "test_jhash.h"
SEC("scale90_inline")
--
2.18.1
^ permalink raw reply related
* -Wsometimes-uninitialized warning after 8b97b055dc9db09b48d5a9a37d847900dd00d3cc
From: Nathan Chancellor @ 2019-07-02 18:18 UTC (permalink / raw)
To: Miaoqing Pan, Kalle Valo
Cc: ath10k, linux-wireless, netdev, linux-kernel, clang-built-linux
Hi all,
After commit 8b97b055dc9d ("ath10k: fix failure to set multiple fixed
rate") in -next, clang warns:
../drivers/net/wireless/ath/ath10k/mac.c:7528:7: warning: variable 'vht_pfr' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
arvif->vht_pfr = vht_pfr;
^~~~~~~
../drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
u8 vht_pfr;
^
= '\0'
1 warning generated.
This definitely seems legitimate as the call to
ath10k_mac_can_set_bitrate_mask might fail and vht_pfr
won't be initialized. I would fix this myself but I assume
there is a sane default value for vht_pfr other than just
0 that should be used?
Please look into this when you get a chance. Thanks,
Nathan
^ permalink raw reply
* Re: [PATCH net-next] net: stmmac: Re-word Kconfig entry
From: David Miller @ 2019-07-02 18:13 UTC (permalink / raw)
To: Jose.Abreu
Cc: linux-kernel, netdev, Joao.Pinto, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <eac9ac857255993581bec265fb5ce7e3bdd20c78.1562058669.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Tue, 2 Jul 2019 11:12:10 +0200
> We support many speeds and it doesn't make much sense to list them all
> in the Kconfig. Let's just call it Multi-Gigabit.
>
> Suggested-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] hinic: remove standard netdev stats
From: David Miller @ 2019-07-02 18:12 UTC (permalink / raw)
To: xuechaojing
Cc: linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun, chiqijun,
wulike1
In-Reply-To: <20190701234000.31738-1-xuechaojing@huawei.com>
From: Xue Chaojing <xuechaojing@huawei.com>
Date: Mon, 1 Jul 2019 23:40:00 +0000
> This patch removes standard netdev stats in ethtool -S.
>
> Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Signed-off-by: Xue Chaojing <xuechaojing@huawei.com>
Applied, thanks for following up on this.
^ permalink raw reply
* [PATCH] netfilter: nf_log: Replace a seq_printf() call by seq_puts() in seq_show()
From: Markus Elfring @ 2019-07-02 18:11 UTC (permalink / raw)
To: netfilter-devel, coreteam, netdev, David S. Miller,
Florian Westphal, Jozsef Kadlecsik, Pablo Neira Ayuso
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 Jul 2019 20:06:30 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function “seq_puts”.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/netfilter/nf_log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 3574a212bdc2..bb25d4c794c7 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -374,7 +374,7 @@ static int seq_show(struct seq_file *s, void *v)
continue;
logger = nft_log_dereference(loggers[*pos][i]);
- seq_printf(s, "%s", logger->name);
+ seq_puts(s, logger->name);
if (i == 0 && loggers[*pos][i + 1] != NULL)
seq_puts(s, ",");
--
2.22.0
^ permalink raw reply related
* Re: [PATCH net-next] iavf: remove unused debug function iavf_debug_d
From: David Miller @ 2019-07-02 18:06 UTC (permalink / raw)
To: yuehaibing; +Cc: jeffrey.t.kirsher, intel-wired-lan, linux-kernel, netdev
In-Reply-To: <20190702062021.41524-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Tue, 2 Jul 2019 14:20:21 +0800
> There is no caller of function iavf_debug_d() in tree since
> commit 75051ce4c5d8 ("iavf: Fix up debug print macro"),
> so it can be removed.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Jeff, please queue this up or handle however is otherwise appropriate.
Thanks.
^ permalink raw reply
* Re: [PATCH] bpf: Replace a seq_printf() call by seq_puts() in btf_enum_seq_show()
From: Yonghong Song @ 2019-07-02 18:02 UTC (permalink / raw)
To: Markus Elfring, bpf@vger.kernel.org, netdev@vger.kernel.org,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu
Cc: LKML, kernel-janitors@vger.kernel.org
In-Reply-To: <93898abe-9a7d-0c64-0856-094b62e07ba2@web.de>
On 7/2/19 10:13 AM, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 2 Jul 2019 19:04:08 +0200
>
> A string which did not contain a data format specification should be put
> into a sequence. Thus use the corresponding function “seq_puts”.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> kernel/bpf/btf.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 546ebee39e2a..679a19968f29 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2426,9 +2426,8 @@ static void btf_enum_seq_show(const struct btf *btf, const struct btf_type *t,
>
> for (i = 0; i < nr_enums; i++) {
> if (v == enums[i].val) {
> - seq_printf(m, "%s",
> - __btf_name_by_offset(btf,
> - enums[i].name_off));
> + seq_puts(m,
> + __btf_name_by_offset(btf, enums[i].name_off));
> return;
> }
> }
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH bpf v2] selftests: bpf: fix inlines in test_lwt_seg6local
From: Y Song @ 2019-07-02 17:58 UTC (permalink / raw)
To: Jiri Benc
Cc: netdev, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux, Song Liu
In-Reply-To: <bf60860191c7d4ab0f50fe3143f3d175bd6ee112.1562089104.git.jbenc@redhat.com>
On Tue, Jul 2, 2019 at 10:41 AM Jiri Benc <jbenc@redhat.com> wrote:
>
> Selftests are reporting this failure in test_lwt_seg6local.sh:
>
> + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> Error fetching program/map!
> Failed to parse eBPF program: Operation not permitted
>
> The problem is __attribute__((always_inline)) alone is not enough to prevent
> clang from inserting those functions in .text. In that case, .text is not
> marked as relocateable.
>
> See the output of objdump -h test_lwt_seg6local.o:
>
> Idx Name Size VMA LMA File off Algn
> 0 .text 00003530 0000000000000000 0000000000000000 00000040 2**3
> CONTENTS, ALLOC, LOAD, READONLY, CODE
>
> This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> relocateable .text section in the file.
>
> To fix this, convert to 'static __always_inline'.
>
> v2: Use 'static __always_inline' instead of 'static inline
> __attribute__((always_inline))'
>
> Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> .../testing/selftests/bpf/progs/test_lwt_seg6local.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
> index 0575751bc1bc..e2f6ed0a583d 100644
> --- a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
> +++ b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
> @@ -61,7 +61,7 @@ struct sr6_tlv_t {
> unsigned char value[0];
> } BPF_PACKET_HEADER;
>
> -__attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
> +static __always_inline struct ip6_srh_t *get_srh(struct __sk_buff *skb)
> {
> void *cursor, *data_end;
> struct ip6_srh_t *srh;
> @@ -95,7 +95,7 @@ __attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
> return srh;
> }
>
> -__attribute__((always_inline))
> +static __always_inline
> int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
> uint32_t old_pad, uint32_t pad_off)
> {
> @@ -125,7 +125,7 @@ int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
> return 0;
> }
>
> -__attribute__((always_inline))
> +static __always_inline
> int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
> uint32_t *tlv_off, uint32_t *pad_size,
> uint32_t *pad_off)
> @@ -184,7 +184,7 @@ int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
> return 0;
> }
>
> -__attribute__((always_inline))
> +static __always_inline
> int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
> struct sr6_tlv_t *itlv, uint8_t tlv_size)
> {
> @@ -228,7 +228,7 @@ int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
> return update_tlv_pad(skb, new_pad, pad_size, pad_off);
> }
>
> -__attribute__((always_inline))
> +static __always_inline
> int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
> uint32_t tlv_off)
> {
> @@ -266,7 +266,7 @@ int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
> return update_tlv_pad(skb, new_pad, pad_size, pad_off);
> }
>
> -__attribute__((always_inline))
> +static __always_inline
> int has_egr_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh)
> {
> int tlv_offset = sizeof(struct ip6_t) + sizeof(struct ip6_srh_t) +
> --
> 2.18.1
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/8] bpf: TCP RTT sock_ops bpf callback
From: Y Song @ 2019-07-02 17:55 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, bpf, David Miller, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Priyaranjan Jha, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <20190702161403.191066-1-sdf@google.com>
On Tue, Jul 2, 2019 at 9:14 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Congestion control team would like to have a periodic callback to
> track some TCP statistics. Let's add a sock_ops callback that can be
> selectively enabled on a socket by socket basis and is executed for
> every RTT. BPF program frequency can be further controlled by calling
> bpf_ktime_get_ns and bailing out early.
>
> I run neper tcp_stream and tcp_rr tests with the sample program
> from the last patch and didn't observe any noticeable performance
> difference.
>
> v2:
> * add a comment about second accept() in selftest (Yonghong Song)
> * refer to tcp_bpf.readme in sample program (Yonghong Song)
>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Priyaranjan Jha <priyarjha@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
Ack for the whole series.
Acked-by: Yonghong Song <yhs@fb.com>
>
> Stanislav Fomichev (8):
> bpf: add BPF_CGROUP_SOCK_OPS callback that is executed on every RTT
> bpf: split shared bpf_tcp_sock and bpf_sock_ops implementation
> bpf: add dsack_dups/delivered{,_ce} to bpf_tcp_sock
> bpf: add icsk_retransmits to bpf_tcp_sock
> bpf/tools: sync bpf.h
> selftests/bpf: test BPF_SOCK_OPS_RTT_CB
> samples/bpf: add sample program that periodically dumps TCP stats
> samples/bpf: fix tcp_bpf.readme detach command
>
> include/net/tcp.h | 8 +
> include/uapi/linux/bpf.h | 12 +-
> net/core/filter.c | 207 +++++++++++-----
> net/ipv4/tcp_input.c | 4 +
> samples/bpf/Makefile | 1 +
> samples/bpf/tcp_bpf.readme | 2 +-
> samples/bpf/tcp_dumpstats_kern.c | 68 ++++++
> tools/include/uapi/linux/bpf.h | 12 +-
> tools/testing/selftests/bpf/Makefile | 3 +-
> tools/testing/selftests/bpf/progs/tcp_rtt.c | 61 +++++
> tools/testing/selftests/bpf/test_tcp_rtt.c | 254 ++++++++++++++++++++
> 11 files changed, 574 insertions(+), 58 deletions(-)
> create mode 100644 samples/bpf/tcp_dumpstats_kern.c
> create mode 100644 tools/testing/selftests/bpf/progs/tcp_rtt.c
> create mode 100644 tools/testing/selftests/bpf/test_tcp_rtt.c
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: fix compiling loop{1,2,3}.c on s390
From: Y Song @ 2019-07-02 17:54 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev
In-Reply-To: <20190702153908.41562-1-iii@linux.ibm.com>
On Tue, Jul 2, 2019 at 8:40 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Use PT_REGS_RC(ctx) instead of ctx->rax, which is not present on s390.
>
> Pass -D__TARGET_ARCH_$(ARCH) to selftests in order to choose a proper
> PT_REGS_RC variant.
>
> Fix s930 -> s390 typo.
>
> On s390, provide the forward declaration of struct pt_regs and cast it
> to user_pt_regs in PT_REGS_* macros. This is necessary, because instead
> of the full struct pt_regs, s390 exposes only its first field
> user_pt_regs to userspace, and bpf_helpers.h is used with both userspace
> (in selftests) and kernel (in samples) headers.
>
> On x86, provide userspace versions of PT_REGS_* macros. Unlike s390, x86
> provides struct pt_regs to both userspace and kernel, however, with
> different field names.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/testing/selftests/bpf/Makefile | 4 +-
> tools/testing/selftests/bpf/bpf_helpers.h | 46 +++++++++++++++--------
> tools/testing/selftests/bpf/progs/loop1.c | 2 +-
> tools/testing/selftests/bpf/progs/loop2.c | 2 +-
> tools/testing/selftests/bpf/progs/loop3.c | 2 +-
> 5 files changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index d60fee59fbd1..599b320bef65 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> include ../../../../scripts/Kbuild.include
> +include ../../../scripts/Makefile.arch
>
> LIBDIR := ../../../lib
> BPFDIR := $(LIBDIR)/bpf
> @@ -138,7 +139,8 @@ CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
>
> CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> $(CLANG_SYS_INCLUDES) \
> - -Wno-compare-distinct-pointer-types
> + -Wno-compare-distinct-pointer-types \
> + -D__TARGET_ARCH_$(ARCH)
>
> $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
> $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 1a5b1accf091..faf86d83301a 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -312,8 +312,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #if defined(__TARGET_ARCH_x86)
> #define bpf_target_x86
> #define bpf_target_defined
> -#elif defined(__TARGET_ARCH_s930x)
> - #define bpf_target_s930x
> +#elif defined(__TARGET_ARCH_s390)
> + #define bpf_target_s390
> #define bpf_target_defined
> #elif defined(__TARGET_ARCH_arm)
> #define bpf_target_arm
> @@ -338,8 +338,8 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #ifndef bpf_target_defined
> #if defined(__x86_64__)
> #define bpf_target_x86
> -#elif defined(__s390x__)
> - #define bpf_target_s930x
> +#elif defined(__s390__)
> + #define bpf_target_s390
> #elif defined(__arm__)
> #define bpf_target_arm
> #elif defined(__aarch64__)
> @@ -355,6 +355,7 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
>
> #if defined(bpf_target_x86)
>
> +#ifdef __KERNEL__
> #define PT_REGS_PARM1(x) ((x)->di)
> #define PT_REGS_PARM2(x) ((x)->si)
> #define PT_REGS_PARM3(x) ((x)->dx)
> @@ -365,19 +366,34 @@ static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> #define PT_REGS_RC(x) ((x)->ax)
> #define PT_REGS_SP(x) ((x)->sp)
> #define PT_REGS_IP(x) ((x)->ip)
> +#else
> +#define PT_REGS_PARM1(x) ((x)->rdi)
> +#define PT_REGS_PARM2(x) ((x)->rsi)
> +#define PT_REGS_PARM3(x) ((x)->rdx)
> +#define PT_REGS_PARM4(x) ((x)->rcx)
> +#define PT_REGS_PARM5(x) ((x)->r8)
> +#define PT_REGS_RET(x) ((x)->rsp)
> +#define PT_REGS_FP(x) ((x)->rbp)
> +#define PT_REGS_RC(x) ((x)->rax)
> +#define PT_REGS_SP(x) ((x)->rsp)
> +#define PT_REGS_IP(x) ((x)->rip)
> +#endif
>
> -#elif defined(bpf_target_s390x)
> +#elif defined(bpf_target_s390)
>
> -#define PT_REGS_PARM1(x) ((x)->gprs[2])
> -#define PT_REGS_PARM2(x) ((x)->gprs[3])
> -#define PT_REGS_PARM3(x) ((x)->gprs[4])
> -#define PT_REGS_PARM4(x) ((x)->gprs[5])
> -#define PT_REGS_PARM5(x) ((x)->gprs[6])
> -#define PT_REGS_RET(x) ((x)->gprs[14])
> -#define PT_REGS_FP(x) ((x)->gprs[11]) /* Works only with CONFIG_FRAME_POINTER */
> -#define PT_REGS_RC(x) ((x)->gprs[2])
> -#define PT_REGS_SP(x) ((x)->gprs[15])
> -#define PT_REGS_IP(x) ((x)->psw.addr)
> +/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
> +struct pt_regs;
> +#define PT_REGS_PARM1(x) (((const volatile user_pt_regs *)(x))->gprs[2])
> +#define PT_REGS_PARM2(x) (((const volatile user_pt_regs *)(x))->gprs[3])
> +#define PT_REGS_PARM3(x) (((const volatile user_pt_regs *)(x))->gprs[4])
> +#define PT_REGS_PARM4(x) (((const volatile user_pt_regs *)(x))->gprs[5])
> +#define PT_REGS_PARM5(x) (((const volatile user_pt_regs *)(x))->gprs[6])
> +#define PT_REGS_RET(x) (((const volatile user_pt_regs *)(x))->gprs[14])
> +/* Works only with CONFIG_FRAME_POINTER */
> +#define PT_REGS_FP(x) (((const volatile user_pt_regs *)(x))->gprs[11])
> +#define PT_REGS_RC(x) (((const volatile user_pt_regs *)(x))->gprs[2])
> +#define PT_REGS_SP(x) (((const volatile user_pt_regs *)(x))->gprs[15])
> +#define PT_REGS_IP(x) (((const volatile user_pt_regs *)(x))->psw.addr)
>
> #elif defined(bpf_target_arm)
>
> diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c
> index dea395af9ea9..7cdb7f878310 100644
> --- a/tools/testing/selftests/bpf/progs/loop1.c
> +++ b/tools/testing/selftests/bpf/progs/loop1.c
> @@ -18,7 +18,7 @@ int nested_loops(volatile struct pt_regs* ctx)
> for (j = 0; j < 300; j++)
> for (i = 0; i < j; i++) {
> if (j & 1)
> - m = ctx->rax;
> + m = PT_REGS_RC(ctx);
> else
> m = j;
> sum += i * m;
> diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c
> index 0637bd8e8bcf..9b2f808a2863 100644
> --- a/tools/testing/selftests/bpf/progs/loop2.c
> +++ b/tools/testing/selftests/bpf/progs/loop2.c
> @@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx)
> int i = 0;
>
> while (true) {
> - if (ctx->rax & 1)
> + if (PT_REGS_RC(ctx) & 1)
> i += 3;
> else
> i += 7;
> diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c
> index 30a0f6cba080..d727657d51e2 100644
> --- a/tools/testing/selftests/bpf/progs/loop3.c
> +++ b/tools/testing/selftests/bpf/progs/loop3.c
> @@ -16,7 +16,7 @@ int while_true(volatile struct pt_regs* ctx)
> __u64 i = 0, sum = 0;
> do {
> i++;
> - sum += ctx->rax;
> + sum += PT_REGS_RC(ctx);
> } while (i < 0x100000000ULL);
> return sum;
> }
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: fix compiling loop{1,2,3}.c on s390
From: Y Song @ 2019-07-02 17:53 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: bpf, netdev
In-Reply-To: <1AE29825-8FB2-4682-8822-5F3D16965657@linux.ibm.com>
On Tue, Jul 2, 2019 at 9:58 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> > Am 02.07.2019 um 18:42 schrieb Y Song <ys114321@gmail.com>:
> >
> > On Tue, Jul 2, 2019 at 8:40 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >>
> >> -#elif defined(__s390x__)
> >> - #define bpf_target_s930x
> >
> > I see in some other places (e.g., bcc) where
> > macro __s390x__ is also used to indicate a s390 architecture.
> > Could you explain the difference between __s390__ and
> > __s390x__?
>
> __s390__ is defined for 32-bit and 64-bit variants, __s390x__ is defined
> for 64-bit variant only.
Thanks.
>
> >> #if defined(bpf_target_x86)
> >>
> >> +#ifdef __KERNEL__
> >
> > In samples/bpf/, __KERNEL__ is defined at clang options and
> > in selftests/bpf/, the __KERNEL__ is not defined.
> >
> > I checked x86 pt_regs definition with and without __KERNEL__.
> > They are identical except some register name difference.
> > I am wondering whether we can unify into all without
> > __KERNEL__. Is __KERNEL__ really needed?
>
> Right now removing it causes the build to fail, but the errors look
> fixable. However, I wonder whether there is a plan regarding this:
> should eBPF programs be built with user headers, kernel headers,
> or both? Status quo appears to be "both", so I’ve decided to stick with
> that in this patch.
Your patch is okay in the sense it maintains the current behavor.
I think it is okay since user level and kernel pt_regs layout are the same
except certain names are different.
>
> >> +/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
> >> +struct pt_regs;
> >> +#define PT_REGS_PARM1(x) (((const volatile user_pt_regs *)(x))->gprs[2])
> >
> > Is user_pt_regs a recent change or has been there for quite some time?
> > I am asking since bcc did not use user_pt_regs yet.
>
> It was added in late 2017 in commit 466698e654e8 ("s390/bpf: correct
> broken uapi for BPF_PROG_TYPE_PERF_EVENT program type“).
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and port attribute
From: Jakub Kicinski @ 2019-07-02 17:47 UTC (permalink / raw)
To: Parav Pandit; +Cc: Jiri Pirko, netdev@vger.kernel.org, Saeed Mahameed
In-Reply-To: <AM0PR05MB4866085BC8B082EFD5B59DD2D1F80@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, 2 Jul 2019 04:26:47 +0000, Parav Pandit wrote:
> > On Mon, 1 Jul 2019 07:27:32 -0500, Parav Pandit wrote:
> > > In an eswitch, PCI PF may have port which is normally represented
> > > using a representor netdevice.
> > > To have better visibility of eswitch port, its association with PF, a
> > > representor netdevice and port number, introduce a PCI PF port flavour
> > > and port attriute.
> > >
> > > When devlink port flavour is PCI PF, fill up PCI PF attributes of the
> > > port.
> > >
> > > Extend port name creation using PCI PF number on best effort basis.
> > > So that vendor drivers can skip defining their own scheme.
> > >
> > > $ devlink port show
> > > pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
> > >
> > > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > > diff --git a/include/net/devlink.h b/include/net/devlink.h index
> > > 6625ea068d5e..8db9c0e83fb5 100644
> > > --- a/include/net/devlink.h
> > > +++ b/include/net/devlink.h
> > > @@ -38,6 +38,10 @@ struct devlink {
> > > char priv[0] __aligned(NETDEV_ALIGN); };
> > >
> > > +struct devlink_port_pci_pf_attrs {
> >
> > Why the named structure? Anonymous one should be just fine?
> >
> No specific reason for this patch. But named structure allows to
> extend it more easily with code readability.
I'd argue the readability - I hove to scroll up/look up the structure
just to see it has a single member. But no big deal :)
> Such as subsequently we want to add the peer_mac etc port attributes.
> Named structure to store those attributes are helpful.
It remains to be seen if peer attributes are flavour specific 🤔
I'd imagine most port types would have some form of a peer (other
than a network port, perhaps). But perhaps different peer attributes.
> > > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > > 89c533778135..001f9e2c96f0 100644
> > > --- a/net/core/devlink.c
> > > +++ b/net/core/devlink.c
> > > @@ -517,6 +517,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> > > return -EMSGSIZE;
> > > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
> > > return -EMSGSIZE;
> >
> > Why would we report network port information for PF and VF port
> > flavours?
>
> I didn't see any immediate need to report, at the same time didn't
> find any reason to treat such port flavours differently than existing
> one. It just gives a clear view of the device's eswitch. Might find
> it useful during debugging while inspecting device internal tables..
PFs and VFs ports are not tied to network ports in switchdev mode.
You have only one network port under a devlink instance AFAIR, anyway.
> > > + if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> > > + if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> > > + attrs->pci_pf.pf))
> > > + return -EMSGSIZE;
> > > + }
> > > if (!attrs->split)
> > > return 0;
> > > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
^ permalink raw reply
* [PATCH net-next v2] bonding: add an option to specify a delay between peer notifications
From: Vincent Bernat @ 2019-07-02 17:43 UTC (permalink / raw)
To: Jiri Pirko, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David S. Miller, netdev
Cc: Vincent Bernat
In-Reply-To: <20190701092758.GA2250@nanopsycho>
Currently, gratuitous ARP/ND packets are sent every `miimon'
milliseconds. This commit allows a user to specify a custom delay
through a new option, `peer_notif_delay'.
Like for `updelay' and `downdelay', this delay should be a multiple of
`miimon' to avoid managing an additional work queue. The configuration
logic is copied from `updelay' and `downdelay'. However, the default
value cannot be set using a module parameter: Netlink or sysfs should
be used to configure this feature.
When setting `miimon' to 100 and `peer_notif_delay' to 500, we can
observe the 500 ms delay is respected:
20:30:19.354693 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
20:30:19.874892 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
20:30:20.394919 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
20:30:20.914963 ARP, Request who-has 203.0.113.10 tell 203.0.113.10, length 28
In bond_mii_monitor(), I have tried to keep the lock logic readable.
The change is due to the fact we cannot rely on a notification to
lower the value of `bond->send_peer_notif' as `NETDEV_NOTIFY_PEERS' is
only triggered once every N times, while we need to decrement the
counter each time.
iproute2 also needs to be updated to be able to specify this new
attribute through `ip link'.
Signed-off-by: Vincent Bernat <vincent@bernat.ch>
---
drivers/net/bonding/bond_main.c | 31 ++++++++-----
drivers/net/bonding/bond_netlink.c | 14 ++++++
drivers/net/bonding/bond_options.c | 71 +++++++++++++++++++-----------
drivers/net/bonding/bond_procfs.c | 2 +
drivers/net/bonding/bond_sysfs.c | 13 ++++++
include/net/bond_options.h | 1 +
include/net/bonding.h | 1 +
include/uapi/linux/if_link.h | 1 +
tools/include/uapi/linux/if_link.h | 1 +
9 files changed, 98 insertions(+), 37 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 4f5b3baf04c3..74e90381a317 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -796,6 +796,8 @@ static bool bond_should_notify_peers(struct bonding *bond)
slave ? slave->dev->name : "NULL");
if (!slave || !bond->send_peer_notif ||
+ bond->send_peer_notif %
+ max(1, bond->params.peer_notif_delay) != 0 ||
!netif_carrier_ok(bond->dev) ||
test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
return false;
@@ -886,15 +888,18 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
if (netif_running(bond->dev)) {
bond->send_peer_notif =
- bond->params.num_peer_notif;
+ bond->params.num_peer_notif *
+ max(1, bond->params.peer_notif_delay);
should_notify_peers =
bond_should_notify_peers(bond);
}
call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
- if (should_notify_peers)
+ if (should_notify_peers) {
+ bond->send_peer_notif--;
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
bond->dev);
+ }
}
}
@@ -2279,6 +2284,7 @@ static void bond_mii_monitor(struct work_struct *work)
struct bonding *bond = container_of(work, struct bonding,
mii_work.work);
bool should_notify_peers = false;
+ bool commit;
unsigned long delay;
struct slave *slave;
struct list_head *iter;
@@ -2289,12 +2295,19 @@ static void bond_mii_monitor(struct work_struct *work)
goto re_arm;
rcu_read_lock();
-
should_notify_peers = bond_should_notify_peers(bond);
-
- if (bond_miimon_inspect(bond)) {
+ commit = !!bond_miimon_inspect(bond);
+ if (bond->send_peer_notif) {
+ rcu_read_unlock();
+ if (rtnl_trylock()) {
+ bond->send_peer_notif--;
+ rtnl_unlock();
+ }
+ } else {
rcu_read_unlock();
+ }
+ if (commit) {
/* Race avoidance with bond_close cancel of workqueue */
if (!rtnl_trylock()) {
delay = 1;
@@ -2308,8 +2321,7 @@ static void bond_mii_monitor(struct work_struct *work)
bond_miimon_commit(bond);
rtnl_unlock(); /* might sleep, hold no other locks */
- } else
- rcu_read_unlock();
+ }
re_arm:
if (bond->params.miimon)
@@ -3065,10 +3077,6 @@ static int bond_master_netdev_event(unsigned long event,
case NETDEV_REGISTER:
bond_create_proc_entry(event_bond);
break;
- case NETDEV_NOTIFY_PEERS:
- if (event_bond->send_peer_notif)
- event_bond->send_peer_notif--;
- break;
default:
break;
}
@@ -4691,6 +4699,7 @@ static int bond_check_params(struct bond_params *params)
params->arp_all_targets = arp_all_targets_value;
params->updelay = updelay;
params->downdelay = downdelay;
+ params->peer_notif_delay = 0;
params->use_carrier = use_carrier;
params->lacp_fast = lacp_fast;
params->primary[0] = 0;
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index b24cce48ae35..a259860a7208 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -108,6 +108,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
[IFLA_BOND_AD_ACTOR_SYSTEM] = { .type = NLA_BINARY,
.len = ETH_ALEN },
[IFLA_BOND_TLB_DYNAMIC_LB] = { .type = NLA_U8 },
+ [IFLA_BOND_PEER_NOTIF_DELAY] = { .type = NLA_U32 },
};
static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
@@ -215,6 +216,14 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
if (err)
return err;
}
+ if (data[IFLA_BOND_PEER_NOTIF_DELAY]) {
+ int delay = nla_get_u32(data[IFLA_BOND_PEER_NOTIF_DELAY]);
+
+ bond_opt_initval(&newval, delay);
+ err = __bond_opt_set(bond, BOND_OPT_PEER_NOTIF_DELAY, &newval);
+ if (err)
+ return err;
+ }
if (data[IFLA_BOND_USE_CARRIER]) {
int use_carrier = nla_get_u8(data[IFLA_BOND_USE_CARRIER]);
@@ -494,6 +503,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
nla_total_size(sizeof(u16)) + /* IFLA_BOND_AD_USER_PORT_KEY */
nla_total_size(ETH_ALEN) + /* IFLA_BOND_AD_ACTOR_SYSTEM */
nla_total_size(sizeof(u8)) + /* IFLA_BOND_TLB_DYNAMIC_LB */
+ nla_total_size(sizeof(u32)) + /* IFLA_BOND_PEER_NOTIF_DELAY */
0;
}
@@ -536,6 +546,10 @@ static int bond_fill_info(struct sk_buff *skb,
bond->params.downdelay * bond->params.miimon))
goto nla_put_failure;
+ if (nla_put_u32(skb, IFLA_BOND_PEER_NOTIF_DELAY,
+ bond->params.downdelay * bond->params.miimon))
+ goto nla_put_failure;
+
if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, bond->params.use_carrier))
goto nla_put_failure;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 0d852fe9da7c..ddb3916d3506 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -24,6 +24,8 @@ static int bond_option_updelay_set(struct bonding *bond,
const struct bond_opt_value *newval);
static int bond_option_downdelay_set(struct bonding *bond,
const struct bond_opt_value *newval);
+static int bond_option_peer_notif_delay_set(struct bonding *bond,
+ const struct bond_opt_value *newval);
static int bond_option_use_carrier_set(struct bonding *bond,
const struct bond_opt_value *newval);
static int bond_option_arp_interval_set(struct bonding *bond,
@@ -424,6 +426,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
.desc = "Number of peer notifications to send on failover event",
.values = bond_num_peer_notif_tbl,
.set = bond_option_num_peer_notif_set
+ },
+ [BOND_OPT_PEER_NOTIF_DELAY] = {
+ .id = BOND_OPT_PEER_NOTIF_DELAY,
+ .name = "peer_notif_delay",
+ .desc = "Delay between each peer notification on failover event, in milliseconds",
+ .values = bond_intmax_tbl,
+ .set = bond_option_peer_notif_delay_set
}
};
@@ -841,6 +850,9 @@ static int bond_option_miimon_set(struct bonding *bond,
if (bond->params.downdelay)
netdev_dbg(bond->dev, "Note: Updating downdelay (to %d) since it is a multiple of the miimon value\n",
bond->params.downdelay * bond->params.miimon);
+ if (bond->params.peer_notif_delay)
+ netdev_dbg(bond->dev, "Note: Updating peer_notif_delay (to %d) since it is a multiple of the miimon value\n",
+ bond->params.peer_notif_delay * bond->params.miimon);
if (newval->value && bond->params.arp_interval) {
netdev_dbg(bond->dev, "MII monitoring cannot be used with ARP monitoring - disabling ARP monitoring...\n");
bond->params.arp_interval = 0;
@@ -864,52 +876,59 @@ static int bond_option_miimon_set(struct bonding *bond,
return 0;
}
-/* Set up and down delays. These must be multiples of the
- * MII monitoring value, and are stored internally as the multiplier.
- * Thus, we must translate to MS for the real world.
+/* Set up, down and peer notification delays. These must be multiples
+ * of the MII monitoring value, and are stored internally as the
+ * multiplier. Thus, we must translate to MS for the real world.
*/
-static int bond_option_updelay_set(struct bonding *bond,
- const struct bond_opt_value *newval)
+static int _bond_option_delay_set(struct bonding *bond,
+ const struct bond_opt_value *newval,
+ const char *name,
+ int *target)
{
int value = newval->value;
if (!bond->params.miimon) {
- netdev_err(bond->dev, "Unable to set up delay as MII monitoring is disabled\n");
+ netdev_err(bond->dev, "Unable to set %s as MII monitoring is disabled\n",
+ name);
return -EPERM;
}
if ((value % bond->params.miimon) != 0) {
- netdev_warn(bond->dev, "up delay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
+ netdev_warn(bond->dev,
+ "%s (%d) is not a multiple of miimon (%d), value rounded to %d ms\n",
+ name,
value, bond->params.miimon,
(value / bond->params.miimon) *
bond->params.miimon);
}
- bond->params.updelay = value / bond->params.miimon;
- netdev_dbg(bond->dev, "Setting up delay to %d\n",
- bond->params.updelay * bond->params.miimon);
+ *target = value / bond->params.miimon;
+ netdev_dbg(bond->dev, "Setting %s to %d\n",
+ name,
+ *target * bond->params.miimon);
return 0;
}
+static int bond_option_updelay_set(struct bonding *bond,
+ const struct bond_opt_value *newval)
+{
+ return _bond_option_delay_set(bond, newval, "up delay",
+ &bond->params.updelay);
+}
+
static int bond_option_downdelay_set(struct bonding *bond,
const struct bond_opt_value *newval)
{
- int value = newval->value;
-
- if (!bond->params.miimon) {
- netdev_err(bond->dev, "Unable to set down delay as MII monitoring is disabled\n");
- return -EPERM;
- }
- if ((value % bond->params.miimon) != 0) {
- netdev_warn(bond->dev, "down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
- value, bond->params.miimon,
- (value / bond->params.miimon) *
- bond->params.miimon);
- }
- bond->params.downdelay = value / bond->params.miimon;
- netdev_dbg(bond->dev, "Setting down delay to %d\n",
- bond->params.downdelay * bond->params.miimon);
+ return _bond_option_delay_set(bond, newval, "down delay",
+ &bond->params.downdelay);
+}
- return 0;
+static int bond_option_peer_notif_delay_set(struct bonding *bond,
+ const struct bond_opt_value *newval)
+{
+ int ret = _bond_option_delay_set(bond, newval,
+ "peer notification delay",
+ &bond->params.peer_notif_delay);
+ return ret;
}
static int bond_option_use_carrier_set(struct bonding *bond,
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 9f7d83e827c3..fd5c9cbe45b1 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -104,6 +104,8 @@ static void bond_info_show_master(struct seq_file *seq)
bond->params.updelay * bond->params.miimon);
seq_printf(seq, "Down Delay (ms): %d\n",
bond->params.downdelay * bond->params.miimon);
+ seq_printf(seq, "Peer Notification Delay (ms): %d\n",
+ bond->params.peer_notif_delay * bond->params.miimon);
/* ARP information */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 94214eaf53c5..2d615a93685e 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -327,6 +327,18 @@ static ssize_t bonding_show_updelay(struct device *d,
static DEVICE_ATTR(updelay, 0644,
bonding_show_updelay, bonding_sysfs_store_option);
+static ssize_t bonding_show_peer_notif_delay(struct device *d,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct bonding *bond = to_bond(d);
+
+ return sprintf(buf, "%d\n",
+ bond->params.peer_notif_delay * bond->params.miimon);
+}
+static DEVICE_ATTR(peer_notif_delay, 0644,
+ bonding_show_peer_notif_delay, bonding_sysfs_store_option);
+
/* Show the LACP interval. */
static ssize_t bonding_show_lacp(struct device *d,
struct device_attribute *attr,
@@ -718,6 +730,7 @@ static struct attribute *per_bond_attrs[] = {
&dev_attr_arp_ip_target.attr,
&dev_attr_downdelay.attr,
&dev_attr_updelay.attr,
+ &dev_attr_peer_notif_delay.attr,
&dev_attr_lacp_rate.attr,
&dev_attr_ad_select.attr,
&dev_attr_xmit_hash_policy.attr,
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 2a05cc349018..9d382f2f0bc5 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -63,6 +63,7 @@ enum {
BOND_OPT_AD_ACTOR_SYSTEM,
BOND_OPT_AD_USER_PORT_KEY,
BOND_OPT_NUM_PEER_NOTIF_ALIAS,
+ BOND_OPT_PEER_NOTIF_DELAY,
BOND_OPT_LAST
};
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 676e7fae05a3..f7fe45689142 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -123,6 +123,7 @@ struct bond_params {
int fail_over_mac;
int updelay;
int downdelay;
+ int peer_notif_delay;
int lacp_fast;
unsigned int min_links;
int ad_select;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6f75bda2c2d7..4a8c02cafa9a 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -636,6 +636,7 @@ enum {
IFLA_BOND_AD_USER_PORT_KEY,
IFLA_BOND_AD_ACTOR_SYSTEM,
IFLA_BOND_TLB_DYNAMIC_LB,
+ IFLA_BOND_PEER_NOTIF_DELAY,
__IFLA_BOND_MAX,
};
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index 5b225ff63b48..7d113a9602f0 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -636,6 +636,7 @@ enum {
IFLA_BOND_AD_USER_PORT_KEY,
IFLA_BOND_AD_ACTOR_SYSTEM,
IFLA_BOND_TLB_DYNAMIC_LB,
+ IFLA_BOND_PEER_NOTIF_DELAY,
__IFLA_BOND_MAX,
};
--
2.20.1
^ permalink raw reply related
* [PATCH bpf v2] selftests: bpf: fix inlines in test_lwt_seg6local
From: Jiri Benc @ 2019-07-02 17:40 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Mathieu Xhonneux, Song Liu,
Y Song
Selftests are reporting this failure in test_lwt_seg6local.sh:
+ ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
Error fetching program/map!
Failed to parse eBPF program: Operation not permitted
The problem is __attribute__((always_inline)) alone is not enough to prevent
clang from inserting those functions in .text. In that case, .text is not
marked as relocateable.
See the output of objdump -h test_lwt_seg6local.o:
Idx Name Size VMA LMA File off Algn
0 .text 00003530 0000000000000000 0000000000000000 00000040 2**3
CONTENTS, ALLOC, LOAD, READONLY, CODE
This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
relocateable .text section in the file.
To fix this, convert to 'static __always_inline'.
v2: Use 'static __always_inline' instead of 'static inline
__attribute__((always_inline))'
Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
.../testing/selftests/bpf/progs/test_lwt_seg6local.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
index 0575751bc1bc..e2f6ed0a583d 100644
--- a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
+++ b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
@@ -61,7 +61,7 @@ struct sr6_tlv_t {
unsigned char value[0];
} BPF_PACKET_HEADER;
-__attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
+static __always_inline struct ip6_srh_t *get_srh(struct __sk_buff *skb)
{
void *cursor, *data_end;
struct ip6_srh_t *srh;
@@ -95,7 +95,7 @@ __attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
return srh;
}
-__attribute__((always_inline))
+static __always_inline
int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
uint32_t old_pad, uint32_t pad_off)
{
@@ -125,7 +125,7 @@ int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
return 0;
}
-__attribute__((always_inline))
+static __always_inline
int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
uint32_t *tlv_off, uint32_t *pad_size,
uint32_t *pad_off)
@@ -184,7 +184,7 @@ int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
return 0;
}
-__attribute__((always_inline))
+static __always_inline
int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
struct sr6_tlv_t *itlv, uint8_t tlv_size)
{
@@ -228,7 +228,7 @@ int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
return update_tlv_pad(skb, new_pad, pad_size, pad_off);
}
-__attribute__((always_inline))
+static __always_inline
int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
uint32_t tlv_off)
{
@@ -266,7 +266,7 @@ int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
return update_tlv_pad(skb, new_pad, pad_size, pad_off);
}
-__attribute__((always_inline))
+static __always_inline
int has_egr_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh)
{
int tlv_offset = sizeof(struct ip6_t) + sizeof(struct ip6_srh_t) +
--
2.18.1
^ permalink raw reply related
* Re: [PATCH bpf-next v2] bpftool: Add BPF_F_QUERY_EFFECTIVE support in bpftool cgroup [show|tree]
From: Jakub Kicinski @ 2019-07-02 17:31 UTC (permalink / raw)
To: Takshak Chahande; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <20190702164843.3662715-1-ctakshak@fb.com>
On Tue, 2 Jul 2019 09:48:43 -0700, Takshak Chahande wrote:
> + cgroup_fd = open(cgroup_path, O_RDONLY);
> if (cgroup_fd < 0) {
> - p_err("can't open cgroup %s", argv[1]);
> + p_err("can't open cgroup %s", cgroup_path);
AFAICS the bpf tree has not been merged into bpf-next yet, and there
will be a conflict here. For no good reason. Please wait, I will
post my patch for inclusion when the time is right.
> goto exit;
> }
^ permalink raw reply
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Jiri Benc @ 2019-07-02 17:29 UTC (permalink / raw)
To: Y Song
Cc: Song Liu, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
In-Reply-To: <CAH3MdRUbkswKAYiDSmhe9cdd-Jd=YmC0_PSLhzfY7vKv-zxCCA@mail.gmail.com>
On Mon, 1 Jul 2019 11:39:27 -0700, Y Song wrote:
> By default, we have
> # define __always_inline inline __attribute__((always_inline))
>
> So just use __always_inline should be less verbose in your patch.
I'll resubmit, converting everything to __always_inline, targeting
bpf-next.
> BTW, what compiler did you use have this behavior?
clang version 7.0.1 (tags/RELEASE_701/final)
LLVM version 7.0.1
> Did you have issues with `static __attribute__((always_inline))`?
That seems to work, too.
Jiri
^ permalink raw reply
* RE: [PATCH net-next 1/1] devlink: Add APIs to publish/unpublish the port parameters.
From: Parav Pandit @ 2019-07-02 17:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: Sudarsana Reddy Kalluru, davem@davemloft.net,
netdev@vger.kernel.org, mkalderon@marvell.com, aelior@marvell.com,
jiri@resnulli.us
In-Reply-To: <20190702164844.GA28471@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, July 2, 2019 10:19 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: Sudarsana Reddy Kalluru <skalluru@marvell.com>; davem@davemloft.net;
> netdev@vger.kernel.org; mkalderon@marvell.com; aelior@marvell.com;
> jiri@resnulli.us
> Subject: Re: [PATCH net-next 1/1] devlink: Add APIs to publish/unpublish the
> port parameters.
>
> > A vendor driver calling these APIs is needed at minimum.
>
> Not a vendor driver, but a mainline driver.
>
My apologies for terminology.
I meant to say that a NIC/hw driver from the kernel tree that created the devlink port should call this API (as user) in the patch.
You said it rightly below. Thanks.
> But yes, a new API should not be added without at least one user.
>
> Andrew
^ permalink raw reply
* [PATCH] bpf: Replace a seq_printf() call by seq_puts() in btf_enum_seq_show()
From: Markus Elfring @ 2019-07-02 17:13 UTC (permalink / raw)
To: bpf, netdev, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 2 Jul 2019 19:04:08 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function “seq_puts”.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
kernel/bpf/btf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 546ebee39e2a..679a19968f29 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2426,9 +2426,8 @@ static void btf_enum_seq_show(const struct btf *btf, const struct btf_type *t,
for (i = 0; i < nr_enums; i++) {
if (v == enums[i].val) {
- seq_printf(m, "%s",
- __btf_name_by_offset(btf,
- enums[i].name_off));
+ seq_puts(m,
+ __btf_name_by_offset(btf, enums[i].name_off));
return;
}
}
--
2.22.0
^ permalink raw reply related
* Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Ido Schimmel @ 2019-07-02 17:11 UTC (permalink / raw)
To: Linus Lüssing
Cc: Russell King - ARM Linux admin, nikolay, Ido Schimmel,
Vivien Didelot, Florian Fainelli, netdev@vger.kernel.org,
Jiri Pirko, andrew@lunn.ch, davem@davemloft.net, bridge,
b.a.t.m.a.n
In-Reply-To: <20190630165601.GC2500@otheros>
On Sun, Jun 30, 2019 at 06:56:01PM +0200, Linus Lüssing wrote:
> > On Sun, Jun 23, 2019 at 10:44:27AM +0300, Ido Schimmel wrote:
> > > See commit b00589af3b04 ("bridge: disable snooping if there is no
> > > querier"). I think that's unfortunate behavior that we need because
> > > multicast snooping is enabled by default. If it weren't enabled by
> > > default, then anyone enabling it would also make sure there's a querier
> > > in the network.
>
> I do not quite understand that point. In a way, that's what we
> have right now, isn't it? By default it's disabled, because by
> default there is no querier on the link. So anyone wanting to use
> multicast snooping will need to make sure there's a querier in the
> network.
Hi Linus,
Querier state is not reflected to drivers ATM, so drivers believe the
bridge is multicast aware and unregistered multicast packets are only
flooded to mrouter ports. Hosts that are silent (because there is no
querier) never get the traffic addressed to them (f.e., IPv6 neighbour
solicitation).
> Overall I think the querier (election) mechanism in the standards could
> need an update. While the lowest-address first might have
> worked well back then, in uniform, fully wired networks where the
> position of the querier did not matter, this is not a good
> solution anymore in networks involving wireless, dynamic connections.
> Especially in wireless mesh networks this is a bit of an issue for
> us. Ideally, the querier mechanism were dismissed in favour of simply
> unsolicited, periodic IGMP/MLD reports...
>
> But of course, updating IETF standards is no solution for now.
>
> While more complicated, it would not be impossible to consider the
> querier state, would it? I mean you probably already need to
> consider the case of a user disabling multicast snooping during
> runtime, right?
Sure, this is implemented.
> So similarly, you could react to appearing or disappearing queriers?
Yes, but it's a bit more complicated since we need to differentiate
between IPv4 and IPv6. If the bridge is multicast aware, but there is
only IPv4 querier on the link, then:
1. All the IPv6 MDB entries need to be removed from the device. At least
in mlxsw, we do not have a way to ignore only IPv6 entries. From the
device's perspective, an MDB entry is just a multicast DMAC with a
bitmap of ports packets should be replicated to.
2. We need to split the flood tables used for IPv4 and IPv6 unregistered
multicast packets. For IPv4, packets should only be flooded to mrouter
ports whereas for IPv6 packets should be flooded to all the member
ports.
Do you differentiate between IPv4 and IPv6 in batman-adv?
> Cheers, Linus
Thanks for the feedback!
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: fix compiling loop{1,2,3}.c on s390
From: Ilya Leoshkevich @ 2019-07-02 16:58 UTC (permalink / raw)
To: Y Song; +Cc: bpf, netdev
In-Reply-To: <CAH3MdRUk5x2D9yRuKpGpVuDMFF0JbYeB+Y0Qz6chtPgfm-1vxA@mail.gmail.com>
> Am 02.07.2019 um 18:42 schrieb Y Song <ys114321@gmail.com>:
>
> On Tue, Jul 2, 2019 at 8:40 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> -#elif defined(__s390x__)
>> - #define bpf_target_s930x
>
> I see in some other places (e.g., bcc) where
> macro __s390x__ is also used to indicate a s390 architecture.
> Could you explain the difference between __s390__ and
> __s390x__?
__s390__ is defined for 32-bit and 64-bit variants, __s390x__ is defined
for 64-bit variant only.
>> #if defined(bpf_target_x86)
>>
>> +#ifdef __KERNEL__
>
> In samples/bpf/, __KERNEL__ is defined at clang options and
> in selftests/bpf/, the __KERNEL__ is not defined.
>
> I checked x86 pt_regs definition with and without __KERNEL__.
> They are identical except some register name difference.
> I am wondering whether we can unify into all without
> __KERNEL__. Is __KERNEL__ really needed?
Right now removing it causes the build to fail, but the errors look
fixable. However, I wonder whether there is a plan regarding this:
should eBPF programs be built with user headers, kernel headers,
or both? Status quo appears to be "both", so I’ve decided to stick with
that in this patch.
>> +/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
>> +struct pt_regs;
>> +#define PT_REGS_PARM1(x) (((const volatile user_pt_regs *)(x))->gprs[2])
>
> Is user_pt_regs a recent change or has been there for quite some time?
> I am asking since bcc did not use user_pt_regs yet.
It was added in late 2017 in commit 466698e654e8 ("s390/bpf: correct
broken uapi for BPF_PROG_TYPE_PERF_EVENT program type“).
^ permalink raw reply
* Re: [PATCH net-next] r8169: add random MAC address fallback
From: Heiner Kallweit @ 2019-07-02 16:49 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <61a7754f-bdf9-f69a-296d-47353a78c8b4@gmail.com>
On 02.07.2019 08:18, Heiner Kallweit wrote:
>>From 1c8bacf724f1450e5256c68fbff407305faf9cbd Mon Sep 17 00:00:00 2001
>
>
>
Sorry, something went wrong when preparing the commit message. I'll resubmit.
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/net/ethernet/realtek/r8169_main.c | 40 +++++++++++++++--------
> 1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 450c74dc1..d6c137b7f 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -6651,13 +6651,36 @@ static int rtl_get_ether_clk(struct rtl8169_private *tp)
> return rc;
> }
>
> +static void rtl_init_mac_address(struct rtl8169_private *tp)
> +{
> + struct net_device *dev = tp->dev;
> + u8 *mac_addr = dev->dev_addr;
> + int rc, i;
> +
> + rc = eth_platform_get_mac_address(tp_to_dev(tp), mac_addr);
> + if (!rc)
> + goto done;
> +
> + rtl_read_mac_address(tp, mac_addr);
> + if (is_valid_ether_addr(mac_addr))
> + goto done;
> +
> + for (i = 0; i < ETH_ALEN; i++)
> + mac_addr[i] = RTL_R8(tp, MAC0 + i);
> + if (is_valid_ether_addr(mac_addr))
> + goto done;
> +
> + eth_hw_addr_random(dev);
> + dev_warn(tp_to_dev(tp), "can't read MAC address, setting random one\n");
> +done:
> + rtl_rar_set(tp, mac_addr);
> +}
> +
> static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> - /* align to u16 for is_valid_ether_addr() */
> - u8 mac_addr[ETH_ALEN] __aligned(2) = {};
> struct rtl8169_private *tp;
> struct net_device *dev;
> - int chipset, region, i;
> + int chipset, region;
> int jumbo_max, rc;
>
> dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
> @@ -6749,16 +6772,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> u64_stats_init(&tp->rx_stats.syncp);
> u64_stats_init(&tp->tx_stats.syncp);
>
> - /* get MAC address */
> - rc = eth_platform_get_mac_address(&pdev->dev, mac_addr);
> - if (rc)
> - rtl_read_mac_address(tp, mac_addr);
> -
> - if (is_valid_ether_addr(mac_addr))
> - rtl_rar_set(tp, mac_addr);
> -
> - for (i = 0; i < ETH_ALEN; i++)
> - dev->dev_addr[i] = RTL_R8(tp, MAC0 + i);
> + rtl_init_mac_address(tp);
>
> dev->ethtool_ops = &rtl8169_ethtool_ops;
>
>
^ permalink raw reply
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: santosh.shilimkar @ 2019-07-02 16:49 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <6ff00a46-07f6-7be2-8e75-c87448568aa4@oracle.com>
On 7/1/19 10:11 PM, Gerd Rausch wrote:
> Hi Santosh,
>
> On 01/07/2019 19.28, santosh.shilimkar@oracle.com wrote:
>>>
>> Below. All command timeouts are 60 seconds.
>>
>> enum {
>> MLX4_CMD_TIME_CLASS_A = 60000,
>> MLX4_CMD_TIME_CLASS_B = 60000,
>> MLX4_CMD_TIME_CLASS_C = 60000,
>> };
>>
>
> Thank you for the pointer.
>
>> But having said that, I re-looked the code you are patching
>> and thats actually only FRWR code which is purely work-request
>> based so this command timeout shouldn't matter.
>>
>
> Which brings us back full circle to the question of
> what the timeout ought to be?
>
> Please keep in mind that prior to this fix,
> the RDS code didn't wait at all:
>
> It simply posted those registration (IB_WR_REG_MR)
> and invalidation (IB_WR_LOCAL_INV)
> work-requests, with no regards to when the firmware
> would execute them.
>
> Arguably, waiting any amount time greater than zero
> for the operation to complete is better than not waiting at all.
>
> We can change the timeout to a high value, or even make it infinite
> by using "wait_event" instead of "wait_event_timeout".
>
> For the registration work-requests there is a benefit to wait a short
> amount of time only (the trade-off described in patch #1 of this series).
>
Actually we should just switch this code to what Avinash has
finally made in downstream code. That keeps the RDS_GET_MR
semantics and makes sure MR is really valid before handing over
the key to userland. There is no need for any timeout
for registration case.
> For de-registration work-requests, it is beneficial to wait
> until they are truly done.
> But: Function "rds_ib_unreg_frmr" prior and post this change
> simply moves on after a failed de-registration attempt,
> and releases the pages owned by the memory region.
>
> This patch does _not_ change that behavior.
>
>> If the work request fails, then it will lead to flush errors and
>> MRs will be marked as STALE. So this wait may not be necessary
>>
>
> This wait is necessary to avoid the 2 scenarios described
> in the commit-log message:
>
> #1) Memory regions bouncing between "drop_list" and "clean_list"
> as items on the "clean_list" aren't really clean until
> their state transitions to "FRMR_IS_FREE".
>
> #2) Prevent an access error as "rds_ib_post_inv" is called
> just prior to de-referencing pages via "__rds_ib_teardown_mr".
> And you certainly don't want those pages populated in the
> HCA's memory-translation-table with full access, while
> the Linux kernel 'thinks' you gave them back already
> and starts re-purposing them.
>
>> RDS_GET_MR case is what actually showing the issue you saw
>> and the fix for that Avinash has it in production kernel.
>
> Actually, no:
> Socket option RDS_GET_MR wasn't even in the code-path of the
> tests I performed:
>
> It were there RDS_CMSG_RDMA_MAP / RDS_CMSG_RDMA_DEST control
> messages that ended up calling '__rds_rdma_map".
>
What option did you use ? Default option with rds-stress is
RDS_GET_MR and hence the question.
>>
>> I believe with that change, registration issue becomes non-issue
>> already.
>>
>
> Please explain how that is related to this fix-suggestion?
>
> I submitted this patch #3 and the others in this series in order
> to fix bugs in the RDS that is currently shipped with Linux.
>
> It may very well be the case that there are other changes
> that Avinash put into production kernels that would be better
> suited to fix this and other problems.
>
> But that should not eliminate the need to fix what is currently broken.
>
> Fixing what's broken does not preclude replacing the fixed code
> with newer or better versions of the same.
>
>> And as far as invalidation concerned with proxy qp, it not longer
>> races with data path qp.
>>
>
> I don't understand, please elaborate.
>
>> May be you can try those changes if not already to see if it
>> addresses the couple of cases where you ended up adding
>> timeouts.
>>
>
> I don't understand, please elaborate:
> a) Are you saying this issue should not be fixed?
> b) Or are you suggesting to replace this fix with a different fix?
> If it's the later, please point out what you have in mind.
> c) ???
>
All am saying is the code got changed for good reason and that changed
code makes some of these race conditions possibly not applicable.
So instead of these timeout fixes, am suggesting to use that
code as fix. At least test it with those changes and see whats
the behavior.
Regards,
Santosh
^ permalink raw reply
* [PATCH bpf-next v2] bpftool: Add BPF_F_QUERY_EFFECTIVE support in bpftool cgroup [show|tree]
From: Takshak Chahande @ 2019-07-02 16:48 UTC (permalink / raw)
To: netdev; +Cc: ast, jakub.kicinski, daniel, ctakshak, kernel-team
With different bpf attach_flags available to attach bpf programs specially
with BPF_F_ALLOW_OVERRIDE and BPF_F_ALLOW_MULTI, the list of effective
bpf-programs available to any sub-cgroups really needs to be available for
easy debugging.
Using BPF_F_QUERY_EFFECTIVE flag, one can get the list of not only attached
bpf-programs to a cgroup but also the inherited ones from parent cgroup.
So additional "effective" option is introduced to use BPF_F_QUERY_EFFECTIVE
query flag here to list all the effective bpf-programs available for execution
at a specified cgroup.
Reused modified test program test_cgroup_attach from tools/testing/selftests/bpf:
# ./test_cgroup_attach
With old bpftool (without 'effective' option)
# bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/
ID AttachType AttachFlags Name
129 egress multi pkt_cntr_1
130 egress multi pkt_cntr_2
Attached new program pkt_cntr_4 in cg2 gives following:
# bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2/
ID AttachType AttachFlags Name
131 egress override pkt_cntr_4
And with 'effective' option it shows all effective programs for cg2:
# bpftool cgroup show /sys/fs/cgroup/cgroup-test-work-dir/cg1/cg2/ effective
ID AttachType AttachFlags Name
131 egress override pkt_cntr_4
129 egress override pkt_cntr_1
130 egress override pkt_cntr_2
Signed-off-by: Takshak Chahande <ctakshak@fb.com>
---
Changelog:
v2:
- Removed global '-e|--effective' query flag
- Added 'effective' option after cgroup path argument as proposed by Jakub
.../bpftool/Documentation/bpftool-cgroup.rst | 17 +++--
tools/bpf/bpftool/bash-completion/bpftool | 15 +++--
tools/bpf/bpftool/cgroup.c | 66 ++++++++++++-------
3 files changed, 66 insertions(+), 32 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
index 585f270c2d25..8c12e8bb5dd4 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-cgroup.rst
@@ -20,8 +20,8 @@ SYNOPSIS
CGROUP COMMANDS
===============
-| **bpftool** **cgroup { show | list }** *CGROUP*
-| **bpftool** **cgroup tree** [*CGROUP_ROOT*]
+| **bpftool** **cgroup { show | list }** *CGROUP* [**effective**]
+| **bpftool** **cgroup tree** [*CGROUP_ROOT*] [**effective**]
| **bpftool** **cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
| **bpftool** **cgroup detach** *CGROUP* *ATTACH_TYPE* *PROG*
| **bpftool** **cgroup help**
@@ -35,13 +35,18 @@ CGROUP COMMANDS
DESCRIPTION
===========
- **bpftool cgroup { show | list }** *CGROUP*
+ **bpftool cgroup { show | list }** *CGROUP* [**effective**]
List all programs attached to the cgroup *CGROUP*.
Output will start with program ID followed by attach type,
attach flags and program name.
- **bpftool cgroup tree** [*CGROUP_ROOT*]
+ If **effective** is specified, then it retrieves all the
+ effective programs that will be executed for events within
+ a cgroup. This includes inherited along with attached ones
+ to the cgroup.
+
+ **bpftool cgroup tree** [*CGROUP_ROOT*] [**effective**]
Iterate over all cgroups in *CGROUP_ROOT* and list all
attached programs. If *CGROUP_ROOT* is not specified,
bpftool uses cgroup v2 mountpoint.
@@ -50,6 +55,10 @@ DESCRIPTION
commands: it starts with absolute cgroup path, followed by
program ID, attach type, attach flags and program name.
+ With **effective** it retrieves all the effective programs
+ that will be executed for events at each cgroup level as
+ similar to show/list command output for a cgroup.
+
**bpftool cgroup attach** *CGROUP* *ATTACH_TYPE* *PROG* [*ATTACH_FLAGS*]
Attach program *PROG* to the cgroup *CGROUP* with attach type
*ATTACH_TYPE* and optional *ATTACH_FLAGS*.
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index ba37095e1f62..b9b76b812dda 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -679,12 +679,15 @@ _bpftool()
;;
cgroup)
case $command in
- show|list)
- _filedir
- return 0
- ;;
- tree)
- _filedir
+ show|list|tree)
+ case $cword in
+ 3)
+ _filedir
+ ;;
+ 4)
+ COMPREPLY=( $( compgen -W 'effective' -- "$cur" ) )
+ ;;
+ esac
return 0
;;
attach|detach)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 390b89a224f1..09040dbdc20a 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -29,6 +29,7 @@
" recvmsg4 | recvmsg6 | sysctl |\n" \
" getsockopt | setsockopt }"
+static unsigned int query_flags;
static const char * const attach_type_strings[] = {
[BPF_CGROUP_INET_INGRESS] = "ingress",
[BPF_CGROUP_INET_EGRESS] = "egress",
@@ -107,7 +108,8 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
__u32 prog_cnt = 0;
int ret;
- ret = bpf_prog_query(cgroup_fd, type, 0, NULL, NULL, &prog_cnt);
+ ret = bpf_prog_query(cgroup_fd, type, query_flags, NULL, NULL,
+ &prog_cnt);
if (ret)
return -1;
@@ -125,8 +127,8 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
int ret;
prog_cnt = ARRAY_SIZE(prog_ids);
- ret = bpf_prog_query(cgroup_fd, type, 0, &attach_flags, prog_ids,
- &prog_cnt);
+ ret = bpf_prog_query(cgroup_fd, type, query_flags, &attach_flags,
+ prog_ids, &prog_cnt);
if (ret)
return ret;
@@ -158,20 +160,32 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
static int do_show(int argc, char **argv)
{
enum bpf_attach_type type;
+ char *cgroup_path;
int cgroup_fd;
int ret = -1;
- if (argc < 1) {
+ if (!REQ_ARGS(1)) {
p_err("too few parameters for cgroup show");
goto exit;
- } else if (argc > 1) {
- p_err("too many parameters for cgroup show");
- goto exit;
}
- cgroup_fd = open(argv[0], O_RDONLY);
+ query_flags = 0;
+ cgroup_path = GET_ARG();
+
+ if (argc) {
+ if (argc == 1 && is_prefix(*argv, "effective")) {
+ query_flags |= BPF_F_QUERY_EFFECTIVE;
+ NEXT_ARG();
+ } else {
+ p_err("invalid argument after cgroup path, "
+ "got: '%s'?, expect only effective", *argv);
+ goto exit;
+ }
+ }
+
+ cgroup_fd = open(cgroup_path, O_RDONLY);
if (cgroup_fd < 0) {
- p_err("can't open cgroup %s", argv[1]);
+ p_err("can't open cgroup %s", cgroup_path);
goto exit;
}
@@ -294,26 +308,34 @@ static char *find_cgroup_root(void)
static int do_show_tree(int argc, char **argv)
{
+ bool free_cg_root = false;
char *cgroup_root;
int ret;
- switch (argc) {
- case 0:
+ query_flags = 0;
+
+ if (!argc) {
cgroup_root = find_cgroup_root();
if (!cgroup_root) {
p_err("cgroup v2 isn't mounted");
return -1;
}
- break;
- case 1:
- cgroup_root = argv[0];
- break;
- default:
- p_err("too many parameters for cgroup tree");
- return -1;
+ free_cg_root = true;
+ } else {
+ cgroup_root = GET_ARG();
+
+ if (argc) {
+ if (argc == 1 && is_prefix(*argv, "effective")) {
+ query_flags |= BPF_F_QUERY_EFFECTIVE;
+ NEXT_ARG();
+ } else {
+ p_err("invalid argument after cgroup path, "
+ "got: '%s'? expect only effective", *argv);
+ return -1;
+ }
+ }
}
-
if (json_output)
jsonw_start_array(json_wtr);
else
@@ -338,7 +360,7 @@ static int do_show_tree(int argc, char **argv)
if (json_output)
jsonw_end_array(json_wtr);
- if (argc == 0)
+ if (free_cg_root)
free(cgroup_root);
return ret;
@@ -459,8 +481,8 @@ static int do_help(int argc, char **argv)
}
fprintf(stderr,
- "Usage: %s %s { show | list } CGROUP\n"
- " %s %s tree [CGROUP_ROOT]\n"
+ "Usage: %s %s { show | list } CGROUP [effective]\n"
+ " %s %s tree [CGROUP_ROOT] [effective]\n"
" %s %s attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS]\n"
" %s %s detach CGROUP ATTACH_TYPE PROG\n"
" %s %s help\n"
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 1/1] devlink: Add APIs to publish/unpublish the port parameters.
From: Andrew Lunn @ 2019-07-02 16:48 UTC (permalink / raw)
To: Parav Pandit
Cc: Sudarsana Reddy Kalluru, davem@davemloft.net,
netdev@vger.kernel.org, mkalderon@marvell.com, aelior@marvell.com,
jiri@resnulli.us
In-Reply-To: <AM0PR05MB4866D7B26F48AF0BED9055EED1F80@AM0PR05MB4866.eurprd05.prod.outlook.com>
> A vendor driver calling these APIs is needed at minimum.
Not a vendor driver, but a mainline driver.
But yes, a new API should not be added without at least one user.
Andrew
^ 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