* Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses
From: David Miller @ 2019-02-21 4:07 UTC (permalink / raw)
To: viro; +Cc: netdev
In-Reply-To: <20190215200934.GM2217@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
Date: Fri, 15 Feb 2019 20:09:35 +0000
> Several u->addr and u->path users are not holding any locks in
> common with unix_bind(). unix_state_lock() is useless for those
> purposes.
>
> u->addr is assign-once and *(u->addr) is fully set up by the time
> we set u->addr (all under unix_table_lock). u->path is also
> set in the same critical area, also before setting u->addr, and
> any unix_sock with ->path filled will have non-NULL ->addr.
>
> So setting ->addr with smp_store_release() is all we need for those
> "lockless" users - just have them fetch ->addr with smp_load_acquire()
> and don't even bother looking at ->path if they see NULL ->addr.
>
> Users of ->addr and ->path fall into several classes now:
> 1) ones that do smp_load_acquire(u->addr) and access *(u->addr)
> and u->path only if smp_load_acquire() has returned non-NULL.
> 2) places holding unix_table_lock. These are guaranteed that
> *(u->addr) is seen fully initialized. If unix_sock is in one of the
> "bound" chains, so's ->path.
> 3) unix_sock_destructor() using ->addr is safe. All places
> that set u->addr are guaranteed to have seen all stores *(u->addr)
> while holding a reference to u and unix_sock_destructor() is called
> when (atomic) refcount hits zero.
> 4) unix_release_sock() using ->path is safe. unix_bind()
> is serialized wrt unix_release() (normally - by struct file
> refcount), and for the instances that had ->path set by unix_bind()
> unix_release_sock() comes from unix_release(), so they are fine.
> Instances that had it set in unix_stream_connect() either end up
> attached to a socket (in unix_accept()), in which case the call
> chain to unix_release_sock() and serialization are the same as in
> the previous case, or they never get accept'ed and unix_release_sock()
> is called when the listener is shut down and its queue gets purged.
> In that case the listener's queue lock provides the barriers needed -
> unix_stream_connect() shoves our unix_sock into listener's queue
> under that lock right after having set ->path and eventual
> unix_release_sock() caller picks them from that queue under the
> same lock right before calling unix_release_sock().
> 5) unix_find_other() use of ->path is pointless, but safe -
> it happens with successful lookup by (abstract) name, so ->path.dentry
> is guaranteed to be NULL there.
>
> earlier-variant-reviewed-by: "Paul E. McKenney" <paulmck@linux.ibm.com>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Applied and queued up for -stable, thanks Al.
^ permalink raw reply
* Re: [Patch net-next 1/2] net_sched: fix a race condition in tcindex_destroy()
From: David Miller @ 2019-02-21 4:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, bugs, ben, jhs, jiri
In-Reply-To: <20190216185827.21535-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Sat, 16 Feb 2019 10:58:26 -0800
> (cherry picked from commit 8015d93ebd27484418d4952284fd02172fa4b0b2)
>
> tcindex_destroy() invokes tcindex_destroy_element() via
> a walker to delete each filter result in its perfect hash
> table, and tcindex_destroy_element() calls tcindex_delete()
> which schedules tcf RCU works to do the final deletion work.
> Unfortunately this races with the RCU callback
> __tcindex_destroy(), which could lead to use-after-free as
> reported by Adrian.
>
> Fix this by migrating this RCU callback to tcf RCU work too,
> as that workqueue is ordered, we will not have use-after-free.
>
> Note, we don't need to hold netns refcnt because we don't call
> tcf_exts_destroy() here.
>
> Fixes: 27ce4f05e2ab ("net_sched: use tcf_queue_work() in tcindex filter")
> Reported-by: Adrian <bugs@abtelecom.ro>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply
* Re: [Patch net-next 2/2] net_sched: fix a memory leak in cls_tcindex
From: David Miller @ 2019-02-21 4:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, jhs, jiri
In-Reply-To: <20190216185827.21535-2-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Sat, 16 Feb 2019 10:58:27 -0800
> (cherry picked from commit 033b228e7f26b29ae37f8bfa1bc6b209a5365e9f)
>
> When tcindex_destroy() destroys all the filter results in
> the perfect hash table, it invokes the walker to delete
> each of them. However, results with class==0 are skipped
> in either tcindex_walk() or tcindex_delete(), which causes
> a memory leak reported by kmemleak.
>
> This patch fixes it by skipping the walker and directly
> deleting these filter results so we don't miss any filter
> result.
>
> As a result of this change, we have to initialize exts->net
> properly in tcindex_alloc_perfect_hash(). For net-next, we
> need to consider whether we should initialize ->net in
> tcf_exts_init() instead, before that just directly test
> CONFIG_NET_CLS_ACT=y.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply
* Re: [pull request][net-next 00/11] Mellanox, mlx5 updates 2019-02-19
From: David Miller @ 2019-02-21 4:15 UTC (permalink / raw)
To: saeedm; +Cc: netdev, ariela
In-Reply-To: <20190219221957.2899-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Tue, 19 Feb 2019 14:19:46 -0800
> This series provides some updates to mlx5 driver.
> In addition, there is one patch that defines new 50Gbps per lane link
> modes in include/uapi/linux/ethtool.h:
> ("ethtool: Added support for 50Gbps per lane link modes")
>
> For more information please see tag log below.
>
> Please pull and let me know if there is any problem.
Pulled, thanks Saeed.
^ permalink raw reply
* Re: [PATCH v2 net-next 0/2] net: phy: disable aneg in genphy_c45_pma_setup_forced
From: David Miller @ 2019-02-21 4:19 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, linux, netdev
In-Reply-To: <c029e3b6-5fbe-08fb-a012-90ceef3be100@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 16 Feb 2019 20:39:57 +0100
> When genphy_c45_pma_setup_forced() is called the "aneg enabled" bit
> may still be set, therefore clear it. This is also in line with what
> genphy_setup_forced() does for Clause 22.
>
> v2:
> - fix a typo in patch 1
Applied, thanks Heiner.
^ permalink raw reply
* Re: [PATCH bpf-next v2] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Kees Cook @ 2019-02-21 5:31 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, Jann Horn, Andy Lutomirski, Alexei Starovoitov,
Network Development
In-Reply-To: <CAADnVQJYjXKe7NKwjiCDt-tsgejZ1S0ApA4aJUw6se5XsWY5KQ@mail.gmail.com>
On Wed, Feb 20, 2019 at 8:03 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Feb 20, 2019 at 3:59 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, Feb 21, 2019 at 12:01:35AM +0100, Daniel Borkmann wrote:
> > > In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> > > a check was added for BPF_PROG_RUN() that for every invocation preemption is
> > > disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
> > > not count for seccomp because only cBPF -> eBPF is loaded here and it does
> > > not make use of any functionality that would require this assertion. Fix this
> > > false positive by adding and using SECCOMP_RUN() variant that does not have
> > > the cant_sleep(); check.
> > >
> > > Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> > > Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> > > Acked-by: Kees Cook <keescook@chromium.org>
> >
> > Applied, Thanks
>
> Actually I think it's a wrong approach to go long term.
> I'm thinking to revert it.
> I think it's better to disable preemption for duration of
> seccomp cbpf prog.
> It's short and there is really no reason for it to be preemptible.
> When seccomp switches to ebpf we'll have this weird inconsistency.
> Let's just disable preemption for seccomp as well.
A lot of changes will be needed for seccomp ebpf -- not the least of
which is convincing me there is a use-case. ;)
But the main issue is that I'm not a huge fan of dropping two
barriers() across syscall entry. That seems pretty heavy-duty for
something that is literally not needed right now.
--
Kees Cook
^ permalink raw reply
* Re: linux-next: Fixes tag needs some work in the net-next tree
From: Vinod Koul @ 2019-02-21 5:34 UTC (permalink / raw)
To: Stefano Brivio
Cc: David Miller, jiri, sfr, netdev, linux-next, linux-kernel, andrew
In-Reply-To: <20190220205910.7bd7fd40@redhat.com>
On 20-02-19, 20:59, Stefano Brivio wrote:
> On Wed, 20 Feb 2019 11:02:01 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Jiri Pirko <jiri@resnulli.us>
> > Date: Wed, 20 Feb 2019 09:36:11 +0100
> >
> > > Would be good to have some robot checking "Fixes" sanity...
> >
> > I want to add a script to my trees that locally do it for me but the
> > backlog for patch review for me is so huge that I never get to "fun"
> > tasks like that....
>
> If it helps, this is what I use after being bitten once:
>
> #!/bin/sh
>
> [ ${#} -ne 2 ] && echo "Usage: %0 PATCH_FILE GIT_TREE" && exit 1
> grep "^Fixes: " "${1}" | while read -r f; do
> sha="$(echo "${f}" | cut -d' ' -f2)"
> if [ -z "${sha}" ] || [ "${f}" != "$(git -C "${2}" show -s --abbrev=12 --pretty=format:"Fixes: %h (\"%s\")" "${sha}" 2>/dev/null)" ]; then
> echo "Bad tag: ${f}" && exit 1
> fi
> done
Awesome thanks, I am adding this into my patch commit script and well as
send script
--
~Vinod
^ permalink raw reply
* [Patch net-next] net_sched: initialize net pointer inside tcf_exts_init()
From: Cong Wang @ 2019-02-21 5:37 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, Jiri Pirko
For tcindex filter, it is too late to initialize the
net pointer in tcf_exts_validate(), as tcf_exts_get_net()
requires a non-NULL net pointer. We can just move its
initialization into tcf_exts_init(), which just requires
an additional parameter.
This makes the code in tcindex_alloc_perfect_hash()
prettier.
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/pkt_cls.h | 5 +++--
net/sched/cls_api.c | 1 -
net/sched/cls_basic.c | 2 +-
net/sched/cls_bpf.c | 2 +-
net/sched/cls_cgroup.c | 2 +-
net/sched/cls_flow.c | 2 +-
net/sched/cls_flower.c | 2 +-
net/sched/cls_fw.c | 5 +++--
net/sched/cls_matchall.c | 2 +-
net/sched/cls_route.c | 2 +-
net/sched/cls_rsvp.h | 7 ++++---
net/sched/cls_tcindex.c | 19 +++++++++----------
net/sched/cls_u32.c | 8 ++++----
13 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 6a530bef9253..58ea48e1221c 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -289,12 +289,13 @@ struct tcf_exts {
int police;
};
-static inline int tcf_exts_init(struct tcf_exts *exts, int action, int police)
+static inline int tcf_exts_init(struct tcf_exts *exts, struct net *net,
+ int action, int police)
{
#ifdef CONFIG_NET_CLS_ACT
exts->type = 0;
exts->nr_actions = 0;
- exts->net = NULL;
+ exts->net = net;
exts->actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *),
GFP_KERNEL);
if (!exts->actions)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 9ad53895e604..2ca2bcd7da1c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3049,7 +3049,6 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
return err;
exts->nr_actions = err;
}
- exts->net = net;
}
#else
if ((exts->action && tb[exts->action]) ||
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 2383f449d2bc..687b0af67878 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -199,7 +199,7 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
if (!fnew)
return -ENOBUFS;
- err = tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
+ err = tcf_exts_init(&fnew->exts, net, TCA_BASIC_ACT, TCA_BASIC_POLICE);
if (err < 0)
goto errout;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 062350c6621c..b4ac58039cb1 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -477,7 +477,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
if (!prog)
return -ENOBUFS;
- ret = tcf_exts_init(&prog->exts, TCA_BPF_ACT, TCA_BPF_POLICE);
+ ret = tcf_exts_init(&prog->exts, net, TCA_BPF_ACT, TCA_BPF_POLICE);
if (ret < 0)
goto errout;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 02b05066b635..4c1567854f95 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -99,7 +99,7 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
if (!new)
return -ENOBUFS;
- err = tcf_exts_init(&new->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
+ err = tcf_exts_init(&new->exts, net, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
if (err < 0)
goto errout;
new->handle = handle;
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 204e2edae8d5..eece1ee26930 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -441,7 +441,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
if (err < 0)
goto err1;
- err = tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE);
+ err = tcf_exts_init(&fnew->exts, net, TCA_FLOW_ACT, TCA_FLOW_POLICE);
if (err < 0)
goto err2;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 640f83e7f93f..27300a3e76c7 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1344,7 +1344,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout_tb;
}
- err = tcf_exts_init(&fnew->exts, TCA_FLOWER_ACT, 0);
+ err = tcf_exts_init(&fnew->exts, net, TCA_FLOWER_ACT, 0);
if (err < 0)
goto errout;
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 4e34966f2ae2..456ee6e62dfa 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -285,7 +285,8 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
#endif /* CONFIG_NET_CLS_IND */
fnew->tp = f->tp;
- err = tcf_exts_init(&fnew->exts, TCA_FW_ACT, TCA_FW_POLICE);
+ err = tcf_exts_init(&fnew->exts, net, TCA_FW_ACT,
+ TCA_FW_POLICE);
if (err < 0) {
kfree(fnew);
return err;
@@ -334,7 +335,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
if (f == NULL)
return -ENOBUFS;
- err = tcf_exts_init(&f->exts, TCA_FW_ACT, TCA_FW_POLICE);
+ err = tcf_exts_init(&f->exts, net, TCA_FW_ACT, TCA_FW_POLICE);
if (err < 0)
goto errout;
f->id = handle;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 1f9d481b0fbb..459921bd3d87 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -191,7 +191,7 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
if (!new)
return -ENOBUFS;
- err = tcf_exts_init(&new->exts, TCA_MATCHALL_ACT, 0);
+ err = tcf_exts_init(&new->exts, net, TCA_MATCHALL_ACT, 0);
if (err)
goto err_exts_init;
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 444d15a75d98..f006af23b64a 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -497,7 +497,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
if (!f)
goto errout;
- err = tcf_exts_init(&f->exts, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
+ err = tcf_exts_init(&f->exts, net, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
if (err < 0)
goto errout;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 4d3836178fa5..0719a21d9c41 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -501,7 +501,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
if (err < 0)
return err;
- err = tcf_exts_init(&e, TCA_RSVP_ACT, TCA_RSVP_POLICE);
+ err = tcf_exts_init(&e, net, TCA_RSVP_ACT, TCA_RSVP_POLICE);
if (err < 0)
return err;
err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr, true,
@@ -523,7 +523,8 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
goto errout2;
}
- err = tcf_exts_init(&n->exts, TCA_RSVP_ACT, TCA_RSVP_POLICE);
+ err = tcf_exts_init(&n->exts, net, TCA_RSVP_ACT,
+ TCA_RSVP_POLICE);
if (err < 0) {
kfree(n);
goto errout2;
@@ -551,7 +552,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
if (f == NULL)
goto errout2;
- err = tcf_exts_init(&f->exts, TCA_RSVP_ACT, TCA_RSVP_POLICE);
+ err = tcf_exts_init(&f->exts, net, TCA_RSVP_ACT, TCA_RSVP_POLICE);
if (err < 0)
goto errout;
h2 = 16;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index fbf3519a12d8..24e0a62a65cc 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -246,10 +246,12 @@ static const struct nla_policy tcindex_policy[TCA_TCINDEX_MAX + 1] = {
[TCA_TCINDEX_CLASSID] = { .type = NLA_U32 },
};
-static int tcindex_filter_result_init(struct tcindex_filter_result *r)
+static int tcindex_filter_result_init(struct tcindex_filter_result *r,
+ struct net *net)
{
memset(r, 0, sizeof(*r));
- return tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
+ return tcf_exts_init(&r->exts, net, TCA_TCINDEX_ACT,
+ TCA_TCINDEX_POLICE);
}
static void tcindex_partial_destroy_work(struct work_struct *work)
@@ -281,13 +283,10 @@ static int tcindex_alloc_perfect_hash(struct net *net, struct tcindex_data *cp)
return -ENOMEM;
for (i = 0; i < cp->hash; i++) {
- err = tcf_exts_init(&cp->perfect[i].exts,
+ err = tcf_exts_init(&cp->perfect[i].exts, net,
TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
if (err < 0)
goto errout;
-#ifdef CONFIG_NET_CLS_ACT
- cp->perfect[i].exts.net = net;
-#endif
}
return 0;
@@ -310,7 +309,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
int err, balloc = 0;
struct tcf_exts e;
- err = tcf_exts_init(&e, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
+ err = tcf_exts_init(&e, net, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
if (err < 0)
return err;
err = tcf_exts_validate(net, tp, tb, est, &e, ovr, true, extack);
@@ -344,7 +343,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
}
cp->h = p->h;
- err = tcindex_filter_result_init(&new_filter_result);
+ err = tcindex_filter_result_init(&new_filter_result, net);
if (err < 0)
goto errout1;
if (old_r)
@@ -431,7 +430,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
goto errout_alloc;
f->key = handle;
f->next = NULL;
- err = tcindex_filter_result_init(&f->result);
+ err = tcindex_filter_result_init(&f->result, net);
if (err < 0) {
kfree(f);
goto errout_alloc;
@@ -444,7 +443,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
}
if (old_r && old_r != r) {
- err = tcindex_filter_result_init(old_r);
+ err = tcindex_filter_result_init(old_r, net);
if (err < 0) {
kfree(f);
goto errout_alloc;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 27d29c04dcc9..48e76a3acf8a 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -804,7 +804,7 @@ static void u32_replace_knode(struct tcf_proto *tp, struct tc_u_common *tp_c,
rcu_assign_pointer(*ins, n);
}
-static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
+static struct tc_u_knode *u32_init_knode(struct net *net, struct tcf_proto *tp,
struct tc_u_knode *n)
{
struct tc_u_hnode *ht = rtnl_dereference(n->ht_down);
@@ -849,7 +849,7 @@ static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
#endif
memcpy(&new->sel, s, sizeof(*s) + s->nkeys*sizeof(struct tc_u32_key));
- if (tcf_exts_init(&new->exts, TCA_U32_ACT, TCA_U32_POLICE)) {
+ if (tcf_exts_init(&new->exts, net, TCA_U32_ACT, TCA_U32_POLICE)) {
kfree(new);
return NULL;
}
@@ -911,7 +911,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return -EINVAL;
}
- new = u32_init_knode(tp, n);
+ new = u32_init_knode(net, tp, n);
if (!new)
return -ENOMEM;
@@ -1061,7 +1061,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
n->fshift = s->hmask ? ffs(ntohl(s->hmask)) - 1 : 0;
n->flags = flags;
- err = tcf_exts_init(&n->exts, TCA_U32_ACT, TCA_U32_POLICE);
+ err = tcf_exts_init(&n->exts, net, TCA_U32_ACT, TCA_U32_POLICE);
if (err < 0)
goto errout;
--
2.20.1
^ permalink raw reply related
* [PATCH RFC 1/5] net: rtnetlink: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-21 5:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
In-Reply-To: <20190221054942.132388-1-joel@joelfernandes.org>
rtnl_register_internal() and rtnl_unregister_all tries to directly
dereference an RCU protected pointed outside RCU read side section.
While this is Ok to do since a lock is held, let us use the correct
API to avoid programmer bugs in the future.
This also fixes sparse warnings arising from not using RCU API.
net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
(different address spaces) net/core/rtnetlink.c:332:13: expected
struct rtnl_link **tab net/core/rtnetlink.c:332:13: got struct
rtnl_link *[noderef] <asn:4>*<noident>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
net/core/rtnetlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ea1bed08ede..98be4b4818a9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
msgindex = rtm_msgindex(msgtype);
rtnl_lock();
- tab = rtnl_msg_handlers[protocol];
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
if (tab == NULL) {
tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
if (!tab)
@@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
rtnl_lock();
- tab = rtnl_msg_handlers[protocol];
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
if (!tab) {
rtnl_unlock();
return;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH RFC 5/5] rcuwait: Replace rcu_assign_pointer() with WRITE_ONCE
From: Joel Fernandes (Google) @ 2019-02-21 5:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
In-Reply-To: <20190221054942.132388-1-joel@joelfernandes.org>
This suppresses a sparse error generated due to the recently added
rcu_assign_pointer sparse check below. It seems WRITE_ONCE should be
sufficient here.
>> kernel//locking/percpu-rwsem.c:162:9: sparse: error: incompatible
types in comparison expression (different address spaces)
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
include/linux/rcuwait.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
index 90bfa3279a01..9e5b4760e6c2 100644
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -44,7 +44,7 @@ extern void rcuwait_wake_up(struct rcuwait *w);
*/ \
WARN_ON(current->exit_state); \
\
- rcu_assign_pointer((w)->task, current); \
+ WRITE_ONCE((w)->task, current); \
for (;;) { \
/* \
* Implicit barrier (A) pairs with (B) in \
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH RFC 4/5] sched/topology: Annonate RCU pointers properly
From: Joel Fernandes (Google) @ 2019-02-21 5:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
In-Reply-To: <20190221054942.132388-1-joel@joelfernandes.org>
The scheduler's topology code uses rcu_assign_pointer() to initialize
various pointers.
Let us annotate the pointers correctly which also help avoid future
bugs. This suppresses the new sparse errors caused by an annotation
check I added to rcu_assign_pointer().
Also replace rcu_assign_pointer call on rq->sd with WRITE_ONCE. This
should be sufficient for the rq->sd initialization.
This fixes sparse errors:
kernel//sched/topology.c:378:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:387:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:612:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:615:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:618:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:621:9: sparse: error: incompatible types in
comparison expression (different address spaces)
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/sched/sched.h | 12 ++++++------
kernel/sched/topology.c | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 2ab545d40381..806703afd4b0 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -780,7 +780,7 @@ struct root_domain {
* NULL-terminated list of performance domains intersecting with the
* CPUs of the rd. Protected by RCU.
*/
- struct perf_domain *pd;
+ struct perf_domain __rcu *pd;
};
extern struct root_domain def_root_domain;
@@ -1305,13 +1305,13 @@ static inline struct sched_domain *lowest_flag_domain(int cpu, int flag)
return sd;
}
-DECLARE_PER_CPU(struct sched_domain *, sd_llc);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DECLARE_PER_CPU(int, sd_llc_size);
DECLARE_PER_CPU(int, sd_llc_id);
-DECLARE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
-DECLARE_PER_CPU(struct sched_domain *, sd_numa);
-DECLARE_PER_CPU(struct sched_domain *, sd_asym_packing);
-DECLARE_PER_CPU(struct sched_domain *, sd_asym_cpucapacity);
+DECLARE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
extern struct static_key_false sched_asym_cpucapacity;
struct sched_group_capacity {
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 3f35ba1d8fde..2eab2e16ded5 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -586,13 +586,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
* the cpumask of the domain), this allows us to quickly tell if
* two CPUs are in the same cache domain, see cpus_share_cache().
*/
-DEFINE_PER_CPU(struct sched_domain *, sd_llc);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DEFINE_PER_CPU(int, sd_llc_size);
DEFINE_PER_CPU(int, sd_llc_id);
-DEFINE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
-DEFINE_PER_CPU(struct sched_domain *, sd_numa);
-DEFINE_PER_CPU(struct sched_domain *, sd_asym_packing);
-DEFINE_PER_CPU(struct sched_domain *, sd_asym_cpucapacity);
+DEFINE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
DEFINE_STATIC_KEY_FALSE(sched_asym_cpucapacity);
static void update_top_cache_domain(int cpu)
@@ -668,7 +668,7 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
rq_attach_root(rq, rd);
tmp = rq->sd;
- rcu_assign_pointer(rq->sd, sd);
+ WRITE_ONCE(rq->sd, sd);
dirty_sched_domain_sysctl(cpu);
destroy_sched_domains(tmp);
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH RFC 2/5] ixgbe: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-21 5:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
In-Reply-To: <20190221054942.132388-1-joel@joelfernandes.org>
Recently, I added an RCU annotation check in rcu_assign_pointer. This
caused a sparse error to be reported by the ixgbe driver.
Further looking, it seems the adapter->xdp_prog pointer is not annotated
with __rcu. Annonating it fixed the error, but caused a bunch of other
warnings.
This patch tries to fix all warnings by using RCU API properly. This
makes sense to do because not using RCU properly can result in various
hard to find bugs. This is a best effort fix and is only build tested.
The sparse errors and warnings go away with the change. I request
maintainers / developers in this area to test it properly.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 ++++++++++++-----
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 08d85e336bd4..3b14daf27516 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -311,7 +311,7 @@ struct ixgbe_ring {
struct ixgbe_ring *next; /* pointer to next ring in q_vector */
struct ixgbe_q_vector *q_vector; /* backpointer to host q_vector */
struct net_device *netdev; /* netdev ring belongs to */
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct device *dev; /* device for DMA mapping */
void *desc; /* descriptor ring memory */
union {
@@ -560,7 +560,7 @@ struct ixgbe_adapter {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
/* OS defined structs */
struct net_device *netdev;
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct pci_dev *pdev;
struct mii_bus *mii_bus;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..6aa59bb13a14 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2199,7 +2199,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
u32 act;
rcu_read_lock();
- xdp_prog = READ_ONCE(rx_ring->xdp_prog);
+ xdp_prog = rcu_dereference(rx_ring->xdp_prog);
if (!xdp_prog)
goto xdp_out;
@@ -6547,7 +6547,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
rx_ring->queue_index) < 0)
goto err;
- rx_ring->xdp_prog = adapter->xdp_prog;
+ rcu_assign_pointer(rx_ring->xdp_prog, adapter->xdp_prog);
return 0;
err:
@@ -10246,7 +10246,10 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (nr_cpu_ids > MAX_XDP_QUEUES)
return -ENOMEM;
- old_prog = xchg(&adapter->xdp_prog, prog);
+ rcu_read_lock();
+ old_prog = rcu_dereference(adapter->xdp_prog);
+ rcu_assign_pointer(adapter->xdp_prog, prog);
+ rcu_read_unlock();
/* If transitioning XDP modes reconfigure rings */
if (!!prog != !!old_prog) {
@@ -10271,13 +10274,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
+ struct bpf_prog *prog;
switch (xdp->command) {
case XDP_SETUP_PROG:
return ixgbe_xdp_setup(dev, xdp->prog);
case XDP_QUERY_PROG:
- xdp->prog_id = adapter->xdp_prog ?
- adapter->xdp_prog->aux->id : 0;
+ rcu_read_lock();
+ prog = rcu_dereference(adapter->xdp_prog);
+ xdp->prog_id = prog ? prog->aux->id : 0;
+ rcu_read_unlock();
+
return 0;
case XDP_QUERY_XSK_UMEM:
return ixgbe_xsk_umem_query(adapter, &xdp->xsk.umem,
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-21 5:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
In-Reply-To: <20190221054942.132388-1-joel@joelfernandes.org>
Recently I added an RCU annotation check to rcu_assign_pointer(). All
pointers assigned to RCU protected data are to be annotated with __rcu
inorder to be able to use rcu_assign_pointer() similar to checks in
other RCU APIs.
This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
error: incompatible types in comparison expression (different address
spaces)
Fix this by using the correct APIs for RCU accesses. This will
potentially avoid any future bugs in the code. If it is felt that RCU
protection is not needed here, then the rcu_assign_pointer call can be
dropped and replaced with, say, WRITE_ONCE or smp_store_release. Or, may
be we add a new API to do it. But calls rcu_assign_pointer seems an
abuse of the RCU API unless RCU is being used.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/sched/cpufreq.c | 8 ++++++--
kernel/sched/sched.h | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
index 22bd8980f32f..c9aeb3bf5dc2 100644
--- a/kernel/sched/cpufreq.c
+++ b/kernel/sched/cpufreq.c
@@ -7,7 +7,7 @@
*/
#include "sched.h"
-DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
/**
* cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
@@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
if (WARN_ON(!data || !func))
return;
- if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
+ rcu_read_lock();
+ if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
+ rcu_read_unlock();
return;
+ }
+ rcu_read_unlock();
data->func = func;
rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d04530bf251f..2ab545d40381 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2166,7 +2166,7 @@ static inline u64 irq_time_read(int cpu)
#endif /* CONFIG_IRQ_TIME_ACCOUNTING */
#ifdef CONFIG_CPU_FREQ
-DECLARE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
/**
* cpufreq_update_util - Take a note about CPU utilization changes.
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH RFC 0/5] RCU fixes for rcu_assign_pointer() usage
From: Joel Fernandes (Google) @ 2019-02-21 5:49 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
These patches fix various RCU API usage issues found due to sparse errors as a
result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().
The errors in many cases seem to indicate either an incorrect API usage, or
missing annotations. The annotations added can also help avoid future incorrect
usages and bugs so it is a good idea to do in any case.
These are only build/boot tested and I request for feedback from maintainers
and developers in the various areas the patches touch. Thanks for any feedback!
(There are still errors in rbtree.h but I have kept those for a later time
since fixing them is a bit more involved).
Joel Fernandes (Google) (5):
net: rtnetlink: Fix incorrect RCU API usage
ixgbe: Fix incorrect RCU API usage
sched/cpufreq: Fix incorrect RCU API usage
sched/topology: Annonate RCU pointers properly
rcuwait: Replace rcu_assign_pointer() with WRITE_ONCE
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 ++++++++++++-----
include/linux/rcuwait.h | 2 +-
kernel/sched/cpufreq.c | 8 ++++++--
kernel/sched/sched.h | 14 +++++++-------
kernel/sched/topology.c | 12 ++++++------
net/core/rtnetlink.c | 4 ++--
7 files changed, 36 insertions(+), 25 deletions(-)
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply
* Re: [PATCH bpf-next v5 1/3] libbpf: add support for using AF_XDP sockets
From: Ye Xiaolong @ 2019-02-21 5:59 UTC (permalink / raw)
To: Magnus Karlsson
Cc: bjorn.topel, ast, daniel, netdev, jakub.kicinski, bjorn.topel,
qi.z.zhang, brouer
In-Reply-To: <1550585838-15630-2-git-send-email-magnus.karlsson@intel.com>
Hi Magnus
On 02/19, Magnus Karlsson wrote:
[snip]
>+static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
>+{
>+ bool prog_attached = false;
>+ __u32 prog_id = 0;
>+ int err;
>+
>+ err = bpf_get_link_xdp_id(xsk->ifindex, &prog_id,
>+ xsk->config.xdp_flags);
>+ if (err)
>+ return err;
>+
>+ if (!prog_id) {
>+ prog_attached = true;
>+ err = xsk_create_bpf_maps(xsk);
>+ if (err)
>+ return err;
>+
>+ err = xsk_load_xdp_prog(xsk);
>+ if (err)
>+ goto out_maps;
>+ } else {
>+ xsk->fd = bpf_prog_get_fd_by_id(prog_id);
I suppose it should be
xsk->prog_fd = bpf_prog_get_fd_by_id(prog_id);
>+ }
>+
>+ err = xsk_update_bpf_maps(xsk, true, xsk->fd);
>+ if (err)
>+ goto out_load;
>+
>+ return 0;
>+
>+out_load:
>+ if (prog_attached)
>+ close(xsk->prog_fd);
>+out_maps:
>+ if (prog_attached)
>+ xsk_delete_bpf_maps(xsk);
>+ return err;
>+}
>+
>+int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>+ __u32 queue_id, struct xsk_umem *umem,
>+ struct xsk_ring_cons *rx, struct xsk_ring_prod *tx,
>+ const struct xsk_socket_config *usr_config)
>+{
>+ struct sockaddr_xdp sxdp = {};
>+ struct xdp_mmap_offsets off;
>+ struct xsk_socket *xsk;
>+ socklen_t optlen;
>+ void *map;
>+ int err;
>+
>+ if (!umem || !xsk_ptr || !rx || !tx)
>+ return -EFAULT;
>+
>+ if (umem->refcount) {
>+ pr_warning("Error: shared umems not supported by libbpf.\n");
>+ return -EBUSY;
>+ }
>+
>+ xsk = calloc(1, sizeof(*xsk));
>+ if (!xsk)
>+ return -ENOMEM;
>+
>+ if (umem->refcount++ > 0) {
>+ xsk->fd = socket(AF_XDP, SOCK_RAW, 0);
>+ if (xsk->fd < 0) {
>+ err = -errno;
>+ goto out_xsk_alloc;
>+ }
>+ } else {
>+ xsk->fd = umem->fd;
>+ }
>+
>+ xsk->outstanding_tx = 0;
>+ xsk->queue_id = queue_id;
>+ xsk->umem = umem;
>+ xsk->ifindex = if_nametoindex(ifname);
>+ if (!xsk->ifindex) {
>+ err = -errno;
>+ goto out_socket;
>+ }
>+ strncpy(xsk->ifname, ifname, IFNAMSIZ);
>+
>+ xsk_set_xdp_socket_config(&xsk->config, usr_config);
>+
>+ if (rx) {
>+ err = setsockopt(xsk->fd, SOL_XDP, XDP_RX_RING,
>+ &xsk->config.rx_size,
>+ sizeof(xsk->config.rx_size));
>+ if (err) {
>+ err = -errno;
>+ goto out_socket;
>+ }
>+ }
>+ if (tx) {
>+ err = setsockopt(xsk->fd, SOL_XDP, XDP_TX_RING,
>+ &xsk->config.tx_size,
>+ sizeof(xsk->config.tx_size));
>+ if (err) {
>+ err = -errno;
>+ goto out_socket;
>+ }
>+ }
>+
>+ optlen = sizeof(off);
>+ err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
>+ if (err) {
>+ err = -errno;
>+ goto out_socket;
>+ }
>+
>+ if (rx) {
>+ map = xsk_mmap(NULL, off.rx.desc +
>+ xsk->config.rx_size * sizeof(struct xdp_desc),
>+ PROT_READ | PROT_WRITE,
>+ MAP_SHARED | MAP_POPULATE,
>+ xsk->fd, XDP_PGOFF_RX_RING);
>+ if (map == MAP_FAILED) {
>+ err = -errno;
>+ goto out_socket;
>+ }
>+
>+ rx->mask = xsk->config.rx_size - 1;
>+ rx->size = xsk->config.rx_size;
>+ rx->producer = map + off.rx.producer;
>+ rx->consumer = map + off.rx.consumer;
>+ rx->ring = map + off.rx.desc;
>+ }
>+ xsk->rx = rx;
>+
>+ if (tx) {
>+ map = xsk_mmap(NULL, off.tx.desc +
>+ xsk->config.tx_size * sizeof(struct xdp_desc),
>+ PROT_READ | PROT_WRITE,
>+ MAP_SHARED | MAP_POPULATE,
>+ xsk->fd, XDP_PGOFF_TX_RING);
>+ if (map == MAP_FAILED) {
>+ err = -errno;
>+ goto out_mmap_rx;
>+ }
>+
>+ tx->mask = xsk->config.tx_size - 1;
>+ tx->size = xsk->config.tx_size;
>+ tx->producer = map + off.tx.producer;
>+ tx->consumer = map + off.tx.consumer;
>+ tx->ring = map + off.tx.desc;
>+ tx->cached_cons = xsk->config.tx_size;
>+ }
>+ xsk->tx = tx;
>+
>+ sxdp.sxdp_family = PF_XDP;
>+ sxdp.sxdp_ifindex = xsk->ifindex;
>+ sxdp.sxdp_queue_id = xsk->queue_id;
>+ sxdp.sxdp_flags = xsk->config.bind_flags;
>+
>+ err = bind(xsk->fd, (struct sockaddr *)&sxdp, sizeof(sxdp));
>+ if (err) {
>+ err = -errno;
>+ goto out_mmap_tx;
>+ }
>+
>+ if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
>+ err = xsk_setup_xdp_prog(xsk);
>+ if (err)
>+ goto out_mmap_tx;
>+ }
>+
>+ *xsk_ptr = xsk;
>+ return 0;
>+
>+out_mmap_tx:
>+ if (tx)
>+ munmap(xsk->tx,
>+ off.tx.desc +
>+ xsk->config.tx_size * sizeof(struct xdp_desc));
>+out_mmap_rx:
>+ if (rx)
>+ munmap(xsk->rx,
>+ off.rx.desc +
>+ xsk->config.rx_size * sizeof(struct xdp_desc));
>+out_socket:
>+ if (--umem->refcount)
>+ close(xsk->fd);
>+out_xsk_alloc:
>+ free(xsk);
>+ return err;
>+}
>+
>+int xsk_umem__delete(struct xsk_umem *umem)
>+{
>+ struct xdp_mmap_offsets off;
>+ socklen_t optlen;
>+ int err;
>+
>+ if (!umem)
>+ return 0;
>+
>+ if (umem->refcount)
>+ return -EBUSY;
>+
>+ optlen = sizeof(off);
>+ err = getsockopt(umem->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
>+ if (!err) {
>+ munmap(umem->fill->ring,
>+ off.fr.desc + umem->config.fill_size * sizeof(__u64));
>+ munmap(umem->comp->ring,
>+ off.cr.desc + umem->config.comp_size * sizeof(__u64));
>+ }
>+
>+ close(umem->fd);
>+ free(umem);
>+
>+ return 0;
>+}
>+
>+void xsk_socket__delete(struct xsk_socket *xsk)
>+{
>+ struct xdp_mmap_offsets off;
>+ socklen_t optlen;
>+ int err;
>+
>+ if (!xsk)
>+ return;
>+
>+ (void)xsk_update_bpf_maps(xsk, 0, 0);
>+
>+ optlen = sizeof(off);
>+ err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
>+ if (!err) {
>+ if (xsk->rx)
>+ munmap(xsk->rx->ring,
>+ off.rx.desc +
>+ xsk->config.rx_size * sizeof(struct xdp_desc));
>+ if (xsk->tx)
>+ munmap(xsk->tx->ring,
>+ off.tx.desc +
>+ xsk->config.tx_size * sizeof(struct xdp_desc));
>+ }
>+
>+ xsk->umem->refcount--;
>+ /* Do not close an fd that also has an associated umem connected
>+ * to it.
>+ */
>+ if (xsk->fd != xsk->umem->fd)
>+ close(xsk->fd);
>+ free(xsk);
>+}
>diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>new file mode 100644
>index 0000000..a497f00
>--- /dev/null
>+++ b/tools/lib/bpf/xsk.h
>@@ -0,0 +1,203 @@
>+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
>+
>+/*
>+ * AF_XDP user-space access library.
>+ *
>+ * Copyright(c) 2018 - 2019 Intel Corporation.
>+ *
>+ * Author(s): Magnus Karlsson <magnus.karlsson@intel.com>
>+ */
>+
>+#ifndef __LIBBPF_XSK_H
>+#define __LIBBPF_XSK_H
>+
>+#include <stdio.h>
>+#include <stdint.h>
>+#include <linux/if_xdp.h>
>+
>+#include "libbpf.h"
>+
>+#ifdef __cplusplus
>+extern "C" {
>+#endif
>+
>+/* Do not access these members directly. Use the functions below. */
>+#define DEFINE_XSK_RING(name) \
>+struct name { \
>+ __u32 cached_prod; \
>+ __u32 cached_cons; \
>+ __u32 mask; \
>+ __u32 size; \
>+ __u32 *producer; \
>+ __u32 *consumer; \
>+ void *ring; \
>+}
>+
>+DEFINE_XSK_RING(xsk_ring_prod);
>+DEFINE_XSK_RING(xsk_ring_cons);
>+
>+struct xsk_umem;
>+struct xsk_socket;
>+
>+static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>+ __u32 idx)
>+{
>+ __u64 *addrs = (__u64 *)fill->ring;
>+
>+ return &addrs[idx & fill->mask];
>+}
>+
>+static inline const __u64 *
>+xsk_ring_cons__comp_addr(const struct xsk_ring_cons *comp, __u32 idx)
>+{
>+ const __u64 *addrs = (const __u64 *)comp->ring;
>+
>+ return &addrs[idx & comp->mask];
>+}
>+
>+static inline struct xdp_desc *xsk_ring_prod__tx_desc(struct xsk_ring_prod *tx,
>+ __u32 idx)
>+{
>+ struct xdp_desc *descs = (struct xdp_desc *)tx->ring;
>+
>+ return &descs[idx & tx->mask];
>+}
>+
>+static inline const struct xdp_desc *
>+xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
>+{
>+ const struct xdp_desc *descs = (const struct xdp_desc *)rx->ring;
>+
>+ return &descs[idx & rx->mask];
>+}
>+
>+static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
>+{
>+ __u32 free_entries = r->cached_cons - r->cached_prod;
>+
>+ if (free_entries >= nb)
>+ return free_entries;
>+
>+ /* Refresh the local tail pointer.
>+ * cached_cons is r->size bigger than the real consumer pointer so
>+ * that this addition can be avoided in the more frequently
>+ * executed code that computs free_entries in the beginning of
>+ * this function. Without this optimization it whould have been
>+ * free_entries = r->cached_prod - r->cached_cons + r->size.
>+ */
>+ r->cached_cons = *r->consumer + r->size;
>+
>+ return r->cached_cons - r->cached_prod;
>+}
>+
>+static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
>+{
>+ __u32 entries = r->cached_prod - r->cached_cons;
>+
>+ if (entries == 0) {
>+ r->cached_prod = *r->producer;
>+ entries = r->cached_prod - r->cached_cons;
>+ }
>+
>+ return (entries > nb) ? nb : entries;
>+}
>+
>+static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
>+ size_t nb, __u32 *idx)
>+{
>+ if (unlikely(xsk_prod_nb_free(prod, nb) < nb))
>+ return 0;
>+
>+ *idx = prod->cached_prod;
>+ prod->cached_prod += nb;
>+
>+ return nb;
>+}
>+
>+static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>+{
>+ /* Make sure everything has been written to the ring before signalling
>+ * this to the kernel.
>+ */
>+ smp_wmb();
>+
>+ *prod->producer += nb;
>+}
>+
>+static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>+ size_t nb, __u32 *idx)
>+{
>+ size_t entries = xsk_cons_nb_avail(cons, nb);
>+
>+ if (likely(entries > 0)) {
>+ /* Make sure we do not speculatively read the data before
>+ * we have received the packet buffers from the ring.
>+ */
>+ smp_rmb();
>+
>+ *idx = cons->cached_cons;
>+ cons->cached_cons += entries;
>+ }
>+
>+ return entries;
>+}
>+
>+static inline void xsk_ring_cons__release(struct xsk_ring_cons *cons, size_t nb)
>+{
>+ *cons->consumer += nb;
>+}
>+
>+static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
>+{
>+ return &((char *)umem_area)[addr];
>+}
>+
>+LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem);
>+LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk);
>+
>+#define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048
>+#define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048
>+#define XSK_UMEM__DEFAULT_FRAME_SHIFT 11 /* 2048 bytes */
>+#define XSK_UMEM__DEFAULT_FRAME_SIZE (1 << XSK_UMEM__DEFAULT_FRAME_SHIFT)
>+#define XSK_UMEM__DEFAULT_FRAME_HEADROOM 0
>+
>+struct xsk_umem_config {
>+ __u32 fill_size;
>+ __u32 comp_size;
>+ __u32 frame_size;
>+ __u32 frame_headroom;
>+};
>+
>+/* Flags for the libbpf_flags field. */
>+#define XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD (1 << 0)
>+
>+struct xsk_socket_config {
>+ __u32 rx_size;
>+ __u32 tx_size;
>+ __u32 libbpf_flags;
>+ __u32 xdp_flags;
>+ __u16 bind_flags;
>+};
>+
>+/* Set config to NULL to get the default configuration. */
>+LIBBPF_API int xsk_umem__create(struct xsk_umem **umem,
>+ void *umem_area, __u64 size,
>+ struct xsk_ring_prod *fill,
>+ struct xsk_ring_cons *comp,
>+ const struct xsk_umem_config *config);
>+LIBBPF_API int xsk_socket__create(struct xsk_socket **xsk,
>+ const char *ifname, __u32 queue_id,
>+ struct xsk_umem *umem,
>+ struct xsk_ring_cons *rx,
>+ struct xsk_ring_prod *tx,
>+ const struct xsk_socket_config *config);
>+
>+/* Returns 0 for success and -EBUSY if the umem is still in use. */
>+LIBBPF_API int xsk_umem__delete(struct xsk_umem *umem);
>+LIBBPF_API void xsk_socket__delete(struct xsk_socket *xsk);
>+
>+#ifdef __cplusplus
>+} /* extern "C" */
>+#endif
>+
>+#endif /* __LIBBPF_XSK_H */
>--
>2.7.4
>
^ permalink raw reply
* [PATCH] perf record: Add support for limit perf output file size
From: Jiwei Sun @ 2019-02-21 6:44 UTC (permalink / raw)
To: peterz, mingo, acme, alexander.shishkin, jolsa, namhyung
Cc: ast, daniel, kafai, songliubraving, yhs, linux-kernel, netdev
The patch adds a new option to limit the output file size, then based
on it, we can create a wrapper of the perf command that uses the option
to avoid exhausting the disk space by the unconscious user.
Signed-off-by: Jiwei Sun <jiwei.sun@windriver.com>
---
tools/perf/builtin-record.c | 39 +++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 882285fb9f64..28a03929166d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -81,6 +81,7 @@ struct record {
bool timestamp_boundary;
struct switch_output switch_output;
unsigned long long samples;
+ unsigned long output_max_size; /* = 0: unlimited */
};
static volatile int auxtrace_record__snapshot_started;
@@ -106,6 +107,12 @@ static bool switch_output_time(struct record *rec)
trigger_is_ready(&switch_output_trigger);
}
+static bool record__output_max_size_exceeded(struct record *rec)
+{
+ return (rec->output_max_size &&
+ rec->bytes_written >= rec->output_max_size);
+}
+
static int record__write(struct record *rec, struct perf_mmap *map __maybe_unused,
void *bf, size_t size)
{
@@ -118,6 +125,9 @@ static int record__write(struct record *rec, struct perf_mmap *map __maybe_unuse
rec->bytes_written += size;
+ if (record__output_max_size_exceeded(rec))
+ raise(SIGTERM);
+
if (switch_output_size(rec))
trigger_hit(&switch_output_trigger);
@@ -1639,6 +1649,33 @@ static int parse_clockid(const struct option *opt, const char *str, int unset)
return -1;
}
+static int parse_output_max_size(const struct option *opt, const char *str,
+ int unset)
+{
+ unsigned long *s = (unsigned long *)opt->value;
+ static struct parse_tag tags_size[] = {
+ { .tag = 'B', .mult = 1 },
+ { .tag = 'K', .mult = 1 << 10 },
+ { .tag = 'M', .mult = 1 << 20 },
+ { .tag = 'G', .mult = 1 << 30 },
+ { .tag = 0 },
+ };
+ unsigned long val;
+
+ if (unset) {
+ *s = 0;
+ return 0;
+ }
+
+ val = parse_tag_value(str, tags_size);
+ if (val != (unsigned long) -1) {
+ *s = val;
+ return 0;
+ }
+
+ return -1;
+}
+
static int record__parse_mmap_pages(const struct option *opt,
const char *str,
int unset __maybe_unused)
@@ -1946,6 +1983,8 @@ static struct option __record_options[] = {
&nr_cblocks_default, "n", "Use <n> control blocks in asynchronous trace writing mode (default: 1, max: 4)",
record__aio_parse),
#endif
+ OPT_CALLBACK(0, "output-max-size", &record.output_max_size,
+ "size", "Output file maximum size", parse_output_max_size),
OPT_END()
};
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next v5 1/3] libbpf: add support for using AF_XDP sockets
From: Magnus Karlsson @ 2019-02-21 7:09 UTC (permalink / raw)
To: Ye Xiaolong
Cc: Magnus Karlsson, Björn Töpel, ast, Daniel Borkmann,
Network Development, Jakub Kicinski, Björn Töpel,
Zhang, Qi Z, Jesper Dangaard Brouer
In-Reply-To: <20190221055907.GB81905@intel.com>
On Thu, Feb 21, 2019 at 7:06 AM Ye Xiaolong <xiaolong.ye@intel.com> wrote:
>
> Hi Magnus
>
> On 02/19, Magnus Karlsson wrote:
> [snip]
> >+static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
> >+{
> >+ bool prog_attached = false;
> >+ __u32 prog_id = 0;
> >+ int err;
> >+
> >+ err = bpf_get_link_xdp_id(xsk->ifindex, &prog_id,
> >+ xsk->config.xdp_flags);
> >+ if (err)
> >+ return err;
> >+
> >+ if (!prog_id) {
> >+ prog_attached = true;
> >+ err = xsk_create_bpf_maps(xsk);
> >+ if (err)
> >+ return err;
> >+
> >+ err = xsk_load_xdp_prog(xsk);
> >+ if (err)
> >+ goto out_maps;
> >+ } else {
> >+ xsk->fd = bpf_prog_get_fd_by_id(prog_id);
>
> I suppose it should be
>
> xsk->prog_fd = bpf_prog_get_fd_by_id(prog_id);
My bad, that is correct. Will spin a v6.
Thanks: Magnus
> >+ }
> >+
> >+ err = xsk_update_bpf_maps(xsk, true, xsk->fd);
> >+ if (err)
> >+ goto out_load;
> >+
> >+ return 0;
> >+
> >+out_load:
> >+ if (prog_attached)
> >+ close(xsk->prog_fd);
> >+out_maps:
> >+ if (prog_attached)
> >+ xsk_delete_bpf_maps(xsk);
> >+ return err;
> >+}
> >+
> >+int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
> >+ __u32 queue_id, struct xsk_umem *umem,
> >+ struct xsk_ring_cons *rx, struct xsk_ring_prod *tx,
> >+ const struct xsk_socket_config *usr_config)
> >+{
> >+ struct sockaddr_xdp sxdp = {};
> >+ struct xdp_mmap_offsets off;
> >+ struct xsk_socket *xsk;
> >+ socklen_t optlen;
> >+ void *map;
> >+ int err;
> >+
> >+ if (!umem || !xsk_ptr || !rx || !tx)
> >+ return -EFAULT;
> >+
> >+ if (umem->refcount) {
> >+ pr_warning("Error: shared umems not supported by libbpf.\n");
> >+ return -EBUSY;
> >+ }
> >+
> >+ xsk = calloc(1, sizeof(*xsk));
> >+ if (!xsk)
> >+ return -ENOMEM;
> >+
> >+ if (umem->refcount++ > 0) {
> >+ xsk->fd = socket(AF_XDP, SOCK_RAW, 0);
> >+ if (xsk->fd < 0) {
> >+ err = -errno;
> >+ goto out_xsk_alloc;
> >+ }
> >+ } else {
> >+ xsk->fd = umem->fd;
> >+ }
> >+
> >+ xsk->outstanding_tx = 0;
> >+ xsk->queue_id = queue_id;
> >+ xsk->umem = umem;
> >+ xsk->ifindex = if_nametoindex(ifname);
> >+ if (!xsk->ifindex) {
> >+ err = -errno;
> >+ goto out_socket;
> >+ }
> >+ strncpy(xsk->ifname, ifname, IFNAMSIZ);
> >+
> >+ xsk_set_xdp_socket_config(&xsk->config, usr_config);
> >+
> >+ if (rx) {
> >+ err = setsockopt(xsk->fd, SOL_XDP, XDP_RX_RING,
> >+ &xsk->config.rx_size,
> >+ sizeof(xsk->config.rx_size));
> >+ if (err) {
> >+ err = -errno;
> >+ goto out_socket;
> >+ }
> >+ }
> >+ if (tx) {
> >+ err = setsockopt(xsk->fd, SOL_XDP, XDP_TX_RING,
> >+ &xsk->config.tx_size,
> >+ sizeof(xsk->config.tx_size));
> >+ if (err) {
> >+ err = -errno;
> >+ goto out_socket;
> >+ }
> >+ }
> >+
> >+ optlen = sizeof(off);
> >+ err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
> >+ if (err) {
> >+ err = -errno;
> >+ goto out_socket;
> >+ }
> >+
> >+ if (rx) {
> >+ map = xsk_mmap(NULL, off.rx.desc +
> >+ xsk->config.rx_size * sizeof(struct xdp_desc),
> >+ PROT_READ | PROT_WRITE,
> >+ MAP_SHARED | MAP_POPULATE,
> >+ xsk->fd, XDP_PGOFF_RX_RING);
> >+ if (map == MAP_FAILED) {
> >+ err = -errno;
> >+ goto out_socket;
> >+ }
> >+
> >+ rx->mask = xsk->config.rx_size - 1;
> >+ rx->size = xsk->config.rx_size;
> >+ rx->producer = map + off.rx.producer;
> >+ rx->consumer = map + off.rx.consumer;
> >+ rx->ring = map + off.rx.desc;
> >+ }
> >+ xsk->rx = rx;
> >+
> >+ if (tx) {
> >+ map = xsk_mmap(NULL, off.tx.desc +
> >+ xsk->config.tx_size * sizeof(struct xdp_desc),
> >+ PROT_READ | PROT_WRITE,
> >+ MAP_SHARED | MAP_POPULATE,
> >+ xsk->fd, XDP_PGOFF_TX_RING);
> >+ if (map == MAP_FAILED) {
> >+ err = -errno;
> >+ goto out_mmap_rx;
> >+ }
> >+
> >+ tx->mask = xsk->config.tx_size - 1;
> >+ tx->size = xsk->config.tx_size;
> >+ tx->producer = map + off.tx.producer;
> >+ tx->consumer = map + off.tx.consumer;
> >+ tx->ring = map + off.tx.desc;
> >+ tx->cached_cons = xsk->config.tx_size;
> >+ }
> >+ xsk->tx = tx;
> >+
> >+ sxdp.sxdp_family = PF_XDP;
> >+ sxdp.sxdp_ifindex = xsk->ifindex;
> >+ sxdp.sxdp_queue_id = xsk->queue_id;
> >+ sxdp.sxdp_flags = xsk->config.bind_flags;
> >+
> >+ err = bind(xsk->fd, (struct sockaddr *)&sxdp, sizeof(sxdp));
> >+ if (err) {
> >+ err = -errno;
> >+ goto out_mmap_tx;
> >+ }
> >+
> >+ if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
> >+ err = xsk_setup_xdp_prog(xsk);
> >+ if (err)
> >+ goto out_mmap_tx;
> >+ }
> >+
> >+ *xsk_ptr = xsk;
> >+ return 0;
> >+
> >+out_mmap_tx:
> >+ if (tx)
> >+ munmap(xsk->tx,
> >+ off.tx.desc +
> >+ xsk->config.tx_size * sizeof(struct xdp_desc));
> >+out_mmap_rx:
> >+ if (rx)
> >+ munmap(xsk->rx,
> >+ off.rx.desc +
> >+ xsk->config.rx_size * sizeof(struct xdp_desc));
> >+out_socket:
> >+ if (--umem->refcount)
> >+ close(xsk->fd);
> >+out_xsk_alloc:
> >+ free(xsk);
> >+ return err;
> >+}
> >+
> >+int xsk_umem__delete(struct xsk_umem *umem)
> >+{
> >+ struct xdp_mmap_offsets off;
> >+ socklen_t optlen;
> >+ int err;
> >+
> >+ if (!umem)
> >+ return 0;
> >+
> >+ if (umem->refcount)
> >+ return -EBUSY;
> >+
> >+ optlen = sizeof(off);
> >+ err = getsockopt(umem->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
> >+ if (!err) {
> >+ munmap(umem->fill->ring,
> >+ off.fr.desc + umem->config.fill_size * sizeof(__u64));
> >+ munmap(umem->comp->ring,
> >+ off.cr.desc + umem->config.comp_size * sizeof(__u64));
> >+ }
> >+
> >+ close(umem->fd);
> >+ free(umem);
> >+
> >+ return 0;
> >+}
> >+
> >+void xsk_socket__delete(struct xsk_socket *xsk)
> >+{
> >+ struct xdp_mmap_offsets off;
> >+ socklen_t optlen;
> >+ int err;
> >+
> >+ if (!xsk)
> >+ return;
> >+
> >+ (void)xsk_update_bpf_maps(xsk, 0, 0);
> >+
> >+ optlen = sizeof(off);
> >+ err = getsockopt(xsk->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
> >+ if (!err) {
> >+ if (xsk->rx)
> >+ munmap(xsk->rx->ring,
> >+ off.rx.desc +
> >+ xsk->config.rx_size * sizeof(struct xdp_desc));
> >+ if (xsk->tx)
> >+ munmap(xsk->tx->ring,
> >+ off.tx.desc +
> >+ xsk->config.tx_size * sizeof(struct xdp_desc));
> >+ }
> >+
> >+ xsk->umem->refcount--;
> >+ /* Do not close an fd that also has an associated umem connected
> >+ * to it.
> >+ */
> >+ if (xsk->fd != xsk->umem->fd)
> >+ close(xsk->fd);
> >+ free(xsk);
> >+}
> >diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> >new file mode 100644
> >index 0000000..a497f00
> >--- /dev/null
> >+++ b/tools/lib/bpf/xsk.h
> >@@ -0,0 +1,203 @@
> >+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> >+
> >+/*
> >+ * AF_XDP user-space access library.
> >+ *
> >+ * Copyright(c) 2018 - 2019 Intel Corporation.
> >+ *
> >+ * Author(s): Magnus Karlsson <magnus.karlsson@intel.com>
> >+ */
> >+
> >+#ifndef __LIBBPF_XSK_H
> >+#define __LIBBPF_XSK_H
> >+
> >+#include <stdio.h>
> >+#include <stdint.h>
> >+#include <linux/if_xdp.h>
> >+
> >+#include "libbpf.h"
> >+
> >+#ifdef __cplusplus
> >+extern "C" {
> >+#endif
> >+
> >+/* Do not access these members directly. Use the functions below. */
> >+#define DEFINE_XSK_RING(name) \
> >+struct name { \
> >+ __u32 cached_prod; \
> >+ __u32 cached_cons; \
> >+ __u32 mask; \
> >+ __u32 size; \
> >+ __u32 *producer; \
> >+ __u32 *consumer; \
> >+ void *ring; \
> >+}
> >+
> >+DEFINE_XSK_RING(xsk_ring_prod);
> >+DEFINE_XSK_RING(xsk_ring_cons);
> >+
> >+struct xsk_umem;
> >+struct xsk_socket;
> >+
> >+static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
> >+ __u32 idx)
> >+{
> >+ __u64 *addrs = (__u64 *)fill->ring;
> >+
> >+ return &addrs[idx & fill->mask];
> >+}
> >+
> >+static inline const __u64 *
> >+xsk_ring_cons__comp_addr(const struct xsk_ring_cons *comp, __u32 idx)
> >+{
> >+ const __u64 *addrs = (const __u64 *)comp->ring;
> >+
> >+ return &addrs[idx & comp->mask];
> >+}
> >+
> >+static inline struct xdp_desc *xsk_ring_prod__tx_desc(struct xsk_ring_prod *tx,
> >+ __u32 idx)
> >+{
> >+ struct xdp_desc *descs = (struct xdp_desc *)tx->ring;
> >+
> >+ return &descs[idx & tx->mask];
> >+}
> >+
> >+static inline const struct xdp_desc *
> >+xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
> >+{
> >+ const struct xdp_desc *descs = (const struct xdp_desc *)rx->ring;
> >+
> >+ return &descs[idx & rx->mask];
> >+}
> >+
> >+static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
> >+{
> >+ __u32 free_entries = r->cached_cons - r->cached_prod;
> >+
> >+ if (free_entries >= nb)
> >+ return free_entries;
> >+
> >+ /* Refresh the local tail pointer.
> >+ * cached_cons is r->size bigger than the real consumer pointer so
> >+ * that this addition can be avoided in the more frequently
> >+ * executed code that computs free_entries in the beginning of
> >+ * this function. Without this optimization it whould have been
> >+ * free_entries = r->cached_prod - r->cached_cons + r->size.
> >+ */
> >+ r->cached_cons = *r->consumer + r->size;
> >+
> >+ return r->cached_cons - r->cached_prod;
> >+}
> >+
> >+static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
> >+{
> >+ __u32 entries = r->cached_prod - r->cached_cons;
> >+
> >+ if (entries == 0) {
> >+ r->cached_prod = *r->producer;
> >+ entries = r->cached_prod - r->cached_cons;
> >+ }
> >+
> >+ return (entries > nb) ? nb : entries;
> >+}
> >+
> >+static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
> >+ size_t nb, __u32 *idx)
> >+{
> >+ if (unlikely(xsk_prod_nb_free(prod, nb) < nb))
> >+ return 0;
> >+
> >+ *idx = prod->cached_prod;
> >+ prod->cached_prod += nb;
> >+
> >+ return nb;
> >+}
> >+
> >+static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
> >+{
> >+ /* Make sure everything has been written to the ring before signalling
> >+ * this to the kernel.
> >+ */
> >+ smp_wmb();
> >+
> >+ *prod->producer += nb;
> >+}
> >+
> >+static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
> >+ size_t nb, __u32 *idx)
> >+{
> >+ size_t entries = xsk_cons_nb_avail(cons, nb);
> >+
> >+ if (likely(entries > 0)) {
> >+ /* Make sure we do not speculatively read the data before
> >+ * we have received the packet buffers from the ring.
> >+ */
> >+ smp_rmb();
> >+
> >+ *idx = cons->cached_cons;
> >+ cons->cached_cons += entries;
> >+ }
> >+
> >+ return entries;
> >+}
> >+
> >+static inline void xsk_ring_cons__release(struct xsk_ring_cons *cons, size_t nb)
> >+{
> >+ *cons->consumer += nb;
> >+}
> >+
> >+static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
> >+{
> >+ return &((char *)umem_area)[addr];
> >+}
> >+
> >+LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem);
> >+LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk);
> >+
> >+#define XSK_RING_CONS__DEFAULT_NUM_DESCS 2048
> >+#define XSK_RING_PROD__DEFAULT_NUM_DESCS 2048
> >+#define XSK_UMEM__DEFAULT_FRAME_SHIFT 11 /* 2048 bytes */
> >+#define XSK_UMEM__DEFAULT_FRAME_SIZE (1 << XSK_UMEM__DEFAULT_FRAME_SHIFT)
> >+#define XSK_UMEM__DEFAULT_FRAME_HEADROOM 0
> >+
> >+struct xsk_umem_config {
> >+ __u32 fill_size;
> >+ __u32 comp_size;
> >+ __u32 frame_size;
> >+ __u32 frame_headroom;
> >+};
> >+
> >+/* Flags for the libbpf_flags field. */
> >+#define XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD (1 << 0)
> >+
> >+struct xsk_socket_config {
> >+ __u32 rx_size;
> >+ __u32 tx_size;
> >+ __u32 libbpf_flags;
> >+ __u32 xdp_flags;
> >+ __u16 bind_flags;
> >+};
> >+
> >+/* Set config to NULL to get the default configuration. */
> >+LIBBPF_API int xsk_umem__create(struct xsk_umem **umem,
> >+ void *umem_area, __u64 size,
> >+ struct xsk_ring_prod *fill,
> >+ struct xsk_ring_cons *comp,
> >+ const struct xsk_umem_config *config);
> >+LIBBPF_API int xsk_socket__create(struct xsk_socket **xsk,
> >+ const char *ifname, __u32 queue_id,
> >+ struct xsk_umem *umem,
> >+ struct xsk_ring_cons *rx,
> >+ struct xsk_ring_prod *tx,
> >+ const struct xsk_socket_config *config);
> >+
> >+/* Returns 0 for success and -EBUSY if the umem is still in use. */
> >+LIBBPF_API int xsk_umem__delete(struct xsk_umem *umem);
> >+LIBBPF_API void xsk_socket__delete(struct xsk_socket *xsk);
> >+
> >+#ifdef __cplusplus
> >+} /* extern "C" */
> >+#endif
> >+
> >+#endif /* __LIBBPF_XSK_H */
> >--
> >2.7.4
> >
^ permalink raw reply
* Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink
From: Jiri Pirko @ 2019-02-21 7:00 UTC (permalink / raw)
To: Florian Fainelli
Cc: Jakub Kicinski, davem, netdev, oss-drivers, mkubecek, andrew
In-Reply-To: <befeae18-f80e-6f2f-3e97-28629200cf92@gmail.com>
Thu, Feb 21, 2019 at 03:59:05AM CET, f.fainelli@gmail.com wrote:
>
>
>On 2/19/2019 4:49 PM, Jakub Kicinski wrote:
>> On Tue, 19 Feb 2019 10:19:42 +0100, Jiri Pirko wrote:
>>> Fri, Feb 15, 2019 at 04:44:29PM CET, jakub.kicinski@netronome.com wrote:
>>>> On Fri, 15 Feb 2019 11:15:14 +0100, Jiri Pirko wrote:
>>>>>> static const struct ethtool_ops nfp_net_ethtool_ops = {
>>>>>
>>>>> Why don't you use the compat fallback? I think you should.
>>>>
>>>> You and Michal both asked the same so let me answer the first to ask :)
>>>> - if devlink is built as a module the fallback is not reachable.
>>>
>>> So the fallback is not really good as you can't use it for real drivers
>>> anyway. Odd. Maybe we should compile devlink in without possibility to
>>> have it as module.
>>
>> Ack, I'll make devlink a bool.
>
>Meh how about those poor and memory constrained embedded systems?
>Ideally ethtool should/could have been modular as well, but that ship
>has now sailed.
>
>>
>> I need a little extra time, I forgot that nfp's flower offload still
>> doesn't register all ports (using your port flavour infrastructure).
>>
>
>We have had similar issues with PHYLIB before where we wanted
>net/core/ethtool.c to be able to call into generic PHYLIB functions to
>obtain PHY statistics, an inline helper that de-references the PHY
>device's driver function pointers solved that (look for
>phy_ethtool_get_{strings,sset,stats}) while letting PHYLIB remain modular.
>
>devlink_compat_flash_update() is a bit big to be inlined, but why not?
Others compat functions are going to come.
>
>If we make sure we always provide a devlink_mutex and devlink_list that
>symbols such that this builds wheter CONFIG_DEVLINK=y|m then everything
>else can be determined at runtime whether devlink.ko is loaded or not.
>
>Does that make sense?
>--
>Florian
^ permalink raw reply
* Re: [patch iproute2] devlink: relax dpipe table show dependency on resources
From: Jiri Pirko @ 2019-02-21 7:02 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, dsahern, mlxsw
In-Reply-To: <20190220180117.2ad552d0@shemminger-XPS-13-9360>
Thu, Feb 21, 2019 at 03:01:17AM CET, stephen@networkplumber.org wrote:
>On Wed, 20 Feb 2019 10:21:09 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> - resource_valid = !!nla_table[DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID];
>> + resource_valid = !!nla_table[DEVLINK_ATTR_DPIPE_TABLE_RESOURCE_ID] &&
>> + ctx->resources;
>
>When you add a logical AND operator the !! is no longer needed. The !! convention
>is used to turn a value of not zero (ie not NULL) into a boolean. When you add the
>additional operator the result is already boolean.
Okay. Will send v2.
^ permalink raw reply
* Re: [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink
From: Michal Kubecek @ 2019-02-21 7:17 UTC (permalink / raw)
To: Florian Fainelli
Cc: Jakub Kicinski, Jiri Pirko, davem, netdev, oss-drivers, andrew
In-Reply-To: <befeae18-f80e-6f2f-3e97-28629200cf92@gmail.com>
On Wed, Feb 20, 2019 at 06:59:05PM -0800, Florian Fainelli wrote:
> On 2/19/2019 4:49 PM, Jakub Kicinski wrote:
> > On Tue, 19 Feb 2019 10:19:42 +0100, Jiri Pirko wrote:
> >> Fri, Feb 15, 2019 at 04:44:29PM CET, jakub.kicinski@netronome.com wrote:
> >>> On Fri, 15 Feb 2019 11:15:14 +0100, Jiri Pirko wrote:
> >>>>> static const struct ethtool_ops nfp_net_ethtool_ops = {
> >>>>
> >>>> Why don't you use the compat fallback? I think you should.
> >>>
> >>> You and Michal both asked the same so let me answer the first to ask :)
> >>> - if devlink is built as a module the fallback is not reachable.
> >>
> >> So the fallback is not really good as you can't use it for real drivers
> >> anyway. Odd. Maybe we should compile devlink in without possibility to
> >> have it as module.
> >
> > Ack, I'll make devlink a bool.
>
> Meh how about those poor and memory constrained embedded systems?
> Ideally ethtool should/could have been modular as well, but that ship
> has now sailed.
I would certainly like to make the ioctl interface optional once we
reach the end of "phase one", i.e. make ioctl-less ethtool possible.
Looking at the code, I don't see an obvious reason why it couldn't be
modular.
There seem to be only few functions in net/core/ethtool.c which are
called from outside and all seem to be simple helpers not really tied to
the rest of the code, except for netdev_rss_key variable (needed for
/proc/sys/net/core/netdev_rss_key). Some of them could even be inline.
We could always put these into some net/ethtool/stub.c (ethtool-stub.c)
and build unconditionally.
So if keeping the option to have devlink (and ethtool) as a module is
really desirable, I believe it can be done even now (unless I missed
something important).
> We have had similar issues with PHYLIB before where we wanted
> net/core/ethtool.c to be able to call into generic PHYLIB functions to
> obtain PHY statistics, an inline helper that de-references the PHY
> device's driver function pointers solved that (look for
> phy_ethtool_get_{strings,sset,stats}) while letting PHYLIB remain modular.
There is also something similar in netfilter - nf_ct_hook, nfnl_ct_hook
or nf_ipv6_ops.
> devlink_compat_flash_update() is a bit big to be inlined, but why not?
Most of it seems to be the lookup which I'm planning to factor out as
a separate helper. But that would also need to be available to external
code, of course.
Michal
^ permalink raw reply
* Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Luciano Coelho @ 2019-02-21 7:33 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann
Cc: Johannes Berg, Emmanuel Grumbach, Intel Linux Wireless,
Kalle Valo, linux-wireless, Networking, Linux Kernel Mailing List,
Nick Desaulniers
In-Reply-To: <20190220175615.GA1312@archlinux-ryzen>
On Wed, 2019-02-20 at 10:56 -0700, Nathan Chancellor wrote:
> On Wed, Feb 20, 2019 at 11:51:34AM +0100, Arnd Bergmann wrote:
> > On Tue, Feb 19, 2019 at 7:22 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-
> > > initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-
> > > initiator.c
> > > index e9822a3ec373..92b22250eb7d 100644
> > > --- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > > @@ -462,7 +462,7 @@ static void iwl_mvm_debug_range_resp(struct
> > > iwl_mvm *mvm, u8 index,
> > > {
> > > s64 rtt_avg = res->ftm.rtt_avg * 100;
> > >
> > > - do_div(rtt_avg, 6666);
> > > + div64_s64(rtt_avg, 6666);
> >
> > This is wrong: div64_s64 does not modify its argument like
> > do_div(), but
> > it returns the result instead. You also don't want to divide by a
> > 64-bit
> > value when the second argument is a small constant.
> >
> > I think the correct way should be
> >
> > s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, 6666);
> >
> > If you know that the value is positive, using unsigned types
> > and div_u64() would be slightly faster.
> >
> > Arnd
>
> Thanks for the review and explanation, Arnd.
>
> Luca, could you drop this version so I can resend it?
Sure, please do! I already applied this internally, but I can just fix
it with your new patch and that will be squashed before being sent
upstream, so it will look like your second patch.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH iproute2] ss: fix compilation under glibc < 2.18
From: Thomas De Schampheleire @ 2019-02-21 7:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Thomas De Schampheleire
In-Reply-To: <20190220175345.34de8462@shemminger-XPS-13-9360>
El jue., 21 feb. 2019 a las 2:53, Stephen Hemminger
(<stephen@networkplumber.org>) escribió:
>
> On Wed, 20 Feb 2019 15:41:51 +0100
> Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
>
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > Commit c759116a0b2b6da8df9687b0a40ac69050132c77 introduced support for
> > AF_VSOCK. This define is only provided since glibc version 2.18, so
> > compilation fails when using older toolchains.
> >
> > Provide the necessary definitions if needed.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> Not sure why you would want new iproute2 with a 5 year old version of glibc?
> Yes that means update your tool chain.
>
This problem is noticed for an embedded system. It has up-to-date
applications and libraries (built via Buildroot) but the toolchain and
kernel are supplied by the SoC vendor (in this case Marvell, formerly
Cavium Networks). Unfortunately their toolchain is lagging behind and
is still using glibc 2.16.
I could handle this patch locally, but I think there may be other
people in a similar situation which would benefit from an upstream
fix.
Best regards,
Thomas
^ permalink raw reply
* Re: [PATCH] net: dsa: add missing phy address offset
From: Marcel Reichmuth @ 2019-02-21 7:53 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, vivien.didelot@gmail.com,
davem@davemloft.net
In-Reply-To: <20190220193122.GA5302@lunn.ch>
On Wed, Feb 20, 2019 at 08:31:22PM +0100, Andrew Lunn wrote:
> On Wed, Feb 20, 2019 at 11:27:16AM -0800, Florian Fainelli wrote:
> > On 2/20/19 10:15 AM, Marcel Reichmuth wrote:
> >
> > You are supposed to describe the port to PHY mapping using the binding,
> > so for instance:
> >
> > ports {
> > port@0 {
> > reg = <0>;
> > phy-handle = <&phy1>;
> > };
> >
> > };
> >
> > mdio {
> > phy1: phy@1 {
> > reg = <1>;
> > };
> > };
> >
> > etc. is not that working for you?
>
>
> The Espressobin does exactly this:
>
> arch/arm64/boot/dts/marvell/armada-3720-espressobin.dts
>
> It also uses the 6341.
>
Thank you very much for your hints. Yes that works indeed too. I
just assumed it was intended to work automatically with the
built-in phys as it does with the other switches I am using.
^ permalink raw reply
* Re: [Bridge] [PATCH net] net: bridge: remove ipv6 zero address check in mcast queries
From: Hangbin Liu @ 2019-02-21 8:01 UTC (permalink / raw)
To: Linus Lüssing
Cc: Nikolay Aleksandrov, netdev, roopa, bridge, davem, yinxu,
Sebastian Gottschall
In-Reply-To: <20181213161027.GC1713@otheros>
Hi Linus,
Sorry, my mail client somehow droped your message and I didn't see your reply.
I find this mail after Nikolay pointed out yesterday.
> Hi and thanks for your reply!
>
> On Fri, Dec 14, 2018 at 10:32:16AM +0800, Ying Xu wrote:
> > I think the scenario mentioned above is abnormal.
>
> Can we agree, that this scenario, if switch A and B were using the
> current bridge code, has issues right now which it did
> not have before that patch?
Yes, I agree. But this "regression" could be fixed by setting up correct
switch configuration. See more explains below.
>
> I also do not quite understand what you mean with "abnormal". Do
> you think that it is unlikely to have two snooping switches and
> general queries with a 0.0.0.0 source?
The "abnormal" means we shouldn't use this kind of configuration on
snooping switch.
We could have general queries with a 0.0.0.0 source, but these queries
are only from proxy Querier, not from real Querier.
Based on RFC 4541,
2.1.1. IGMP Forwarding Rules
b) The arrival port for IGMP Queries (sent by multicast routers)
where the source address is not 0.0.0.0.
The 0.0.0.0 address represents a special case where the switch
is proxying IGMP Queries for faster network convergence, but is
not itself the Querier. The switch does not use its own IP
address (even if it has one), because this would cause the
Queries to be seen as coming from a newly elected Querier. The
0.0.0.0 address is used to indicate that the Query packets are
NOT from a multicast router.
This paragraph specified "0.0.0.0" is only for switch proxying queries.
It should not be used as a IGMP Querier.
"because this would cause the Queries to be seen as coming from a newly
elected Querier" means other address could be elected as a Querier but
"0.0.0.0" should not.
AFAIK, most verdors(Cisco, HW, etc. expect H3C) use none-zero address
as default address on proxying switches. But H3C also respect the
b) section.
>
> Note that with the current bridge code and according to RFC3376
> and RFC2236, as soon as a query with a 0.0.0.0 source is sent somewhere
> in the broadcast domain, it will become the selected querier [*].
Yes, In RFC 3376
6.6.2. Querier Election
IGMPv3 elects a single querier per subnet using the same querier
election mechanism as IGMPv2, namely by IP address. When a router
receives a query with a lower IP address, it sets the Other-Querier-
Present timer to Other Querier Present Interval and ceases to send
queries on the network if it was the previously elected querier.
After its Other-Querier Present timer expires, it should begin
sending General Queries.
It only said we should select lower IP address as Querier, but didn't specify
the "0.0.0.0" address.
While in the later RFC 4541
Abstract
This memo describes the recommendations for Internet Group Management
Protocol (IGMP) and Multicast Listener Discovery (MLD) snooping
switches. These are based on best current practices for IGMPv2, with
further considerations for IGMPv3- and MLDv2-snooping.
And section 2.1.1. IGMP Forwarding Rules(I just pasted above). It did
specify the "0.0.0.0" address issue.
I think we should respect the later and accurater standard, shouldn't we?
>
>
> > The source of query indicats that is a real router or only a switch.(0.0.0.0
> > means switch,non-zero means router).
> > In the scenario above,the switch A was selected to be a querier that means A
> > performs as a router,
> > so switch A should config its query source address to non-zero,and then Host A
> > can recieve the traffic from B.
>
> Even if in the described scenario switch A were configured to use a a non-zero
> source address to become a router, so that switch B would mark the port
> to switch A as a multicast router port, switch A would still loose
> in the querier election right now, as 0.0.0.0 is lower (RFC3376, RFC2236). So
> switch B would then become the selected querier with its 0.0.0.0 source
> and switch A would become silent even though it had a non-zero
> source address.
>
> And then we would have the same issue again, only swapped between
> host+switch A and host+switch B.
>
> Would you agree, does that make sense?
>
Yes, that's why Ying Xu said the topology is "abnormal", or illegal. Because
in a correct configured topology, there should has a Querier with none-zero
IP address.
> Regards, Linus
>
>
> [*]: Looking at br_ip4_multicast_select_querier():
>
> If previously selected querier were 0.0.0.0, it would accept any
> source as a new querier ("!br->ip4_querier.addr.u.ip4"). However,
> if the previously selected querier were non-zero, a query with
> 0.0.0.0 would win, too
> ("ntohl(saddr) <= ntohl(br->ip4_querier.addr.u.ip4)").
In RFC 3810
7.6.2. Querier Election
When a router starts operating on a subnet, by default it
considers itself as being the Querier.
When a router receives a query with a lower IPv6 address than its
own, it sets the Other Querier Present timer to Other Querier Present
Timeout; if it was previously in Querier state, it switches to Non-
Querier state and ceases to send queries on the link.
All MLDv2 queries MUST be sent with the FE80::/64 link-local source
address prefix.
MLDv2 also sepcified the default Querier is itself and electe a lower adddress
as new Querier. And the source address should be link-loacl source address
instead of 0("::"). This is another evidence that we should not use "0.0.0.0"
as a Querier. So I think we should fix the IPv4 election in function
br_ip4_multicast_select_querier().
This looks like a little extreme and may have "regression", but the "regression"
should be fixed by setting up correct router/switch configuration.
What do you think?
Regards
Hangbin
^ permalink raw reply
* [PATCH v2] iwlwifi: mvm: Use div_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Nathan Chancellor @ 2019-02-21 8:06 UTC (permalink / raw)
To: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Nick Desaulniers,
Nathan Chancellor, Arnd Bergmann
In-Reply-To: <20190219182105.19933-1-natechancellor@gmail.com>
Clang warns:
drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c:465:2: warning:
comparison of distinct pointer types ('typeof ((rtt_avg)) *' (aka 'long
long *') and 'uint64_t *' (aka 'unsigned long long *'))
[-Wcompare-distinct-pointer-types]
do_div(rtt_avg, 6666);
^~~~~~~~~~~~~~~~~~~~~
include/asm-generic/div64.h:222:28: note: expanded from macro 'do_div'
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
1 warning generated.
do_div expects an unsigned dividend. Use div_s64, which expects a signed
dividend.
Fixes: 937b10c0de68 ("iwlwifi: mvm: add debug prints for FTM")
Link: https://github.com/ClangBuiltLinux/linux/issues/372
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Fix logic (as the return value of div{,64}_s64 must be used), thanks
to Arnd for the review.
drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
index e9822a3ec373..94132cfd1f56 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
@@ -460,9 +460,7 @@ static int iwl_mvm_ftm_range_resp_valid(struct iwl_mvm *mvm, u8 request_id,
static void iwl_mvm_debug_range_resp(struct iwl_mvm *mvm, u8 index,
struct cfg80211_pmsr_result *res)
{
- s64 rtt_avg = res->ftm.rtt_avg * 100;
-
- do_div(rtt_avg, 6666);
+ s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, 6666);
IWL_DEBUG_INFO(mvm, "entry %d\n", index);
IWL_DEBUG_INFO(mvm, "\tstatus: %d\n", res->status);
--
2.21.0.rc1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox