* Re: [PATCH net] ipv4: disable BH in set_ping_group_range()
From: Cong Wang @ 2016-10-20 21:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Eric Salo
In-Reply-To: <1476996209.7065.28.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Oct 20, 2016 at 1:43 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-10-20 at 12:44 -0700, Cong Wang wrote:
>> On Thu, Oct 20, 2016 at 12:40 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Thu, Oct 20, 2016 at 12:32 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> >> On Thu, Oct 20, 2016 at 10:26 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> >>> From: Eric Dumazet <edumazet@google.com>
>> >>>
>> >>> In commit 4ee3bd4a8c746 ("ipv4: disable BH when changing ip local port
>> >>> range") Cong added BH protection in set_local_port_range() but missed
>> >>> that same fix was needed in set_ping_group_range()
>> >>
>> >> Don't know why ping_group_range shares the same lock with local_port_range...
>> >> Perhaps just for saving a few bytes, but that is why I missed this place.
>> >
>> > Hold on... We clearly have typos there... Your fix is not correct.
>>
>> We need the attached patch, your patch should be reverted, because
>> unlike local_port_range we never read it in BH context, no need to bother _bh.
>
> Well, we do not change this sysctl very often, so I am not sure why we
> need different seqlocks to protect these ranges.
>
> Seems a waste of space really (per netns)
Error prone vs. space saving, it's up to you...
But clearly current code is still broken even after your patch. I will send
a revert + previous typo fix.
^ permalink raw reply
* Re: [PATCH net] ipv4: disable BH in set_ping_group_range()
From: Eric Dumazet @ 2016-10-20 21:08 UTC (permalink / raw)
To: Cong Wang; +Cc: David Miller, netdev, Eric Salo
In-Reply-To: <CAM_iQpX0=OnOfsGg_KGz=--_S+-1TG2=LoAmYVAOSxawCQtBUg@mail.gmail.com>
On Thu, 2016-10-20 at 14:00 -0700, Cong Wang wrote:
> Error prone vs. space saving, it's up to you...
>
> But clearly current code is still broken even after your patch. I will send
> a revert + previous typo fix.
Yes please do.
^ permalink raw reply
* Re: [PATCH net-next v5 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-10-20 21:10 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d932905c2581df9415449216eceeed1566290e3e.1476979679.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, 2016-10-20 at 22:31 +0200, Paolo Abeni wrote:
> +
> +int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
> +{
> + struct sk_buff_head *list = &sk->sk_receive_queue;
> + int rmem, delta, amt, err = -ENOMEM;
> + int size = skb->truesize;
> +
> + /* try to avoid the costly atomic add/sub pair when the receive
> + * queue is full; always allow at least a packet
> + */
> + rmem = atomic_read(&sk->sk_rmem_alloc);
> + if (rmem && (rmem + size > sk->sk_rcvbuf))
> + goto drop;
> +
> + /* we drop only if the receive buf is full and the receive
> + * queue contains some other skb
> + */
> + rmem = atomic_add_return(size, &sk->sk_rmem_alloc);
> + if ((rmem > sk->sk_rcvbuf) && (rmem > size))
> + goto uncharge_drop;
> +
> + skb_orphan(skb);
Minor point :
UDP should already have orphaned skbs ? (it uses skb_steal_sock())
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next] net/sched: em_meta: Fix 'meta vlan' to correctly recognize zero VID frames
From: Shmulik Ladkani @ 2016-10-20 21:18 UTC (permalink / raw)
To: David S . Miller
Cc: Jamal Hadi Salim, netdev, Jiri Pirko, Shmulik Ladkani,
Eric Dumazet, Stephen Hemminger
META_COLLECTOR int_vlan_tag() assumes that if the accel tag (vlan_tci)
is zero, then no vlan accel tag is present.
This is incorrect for zero VID vlan accel packets, making the following
match fail:
tc filter add ... basic match 'meta(vlan mask 0xfff eq 0)' ...
Apparently 'int_vlan_tag' was implemented prior VLAN_TAG_PRESENT was
introduced in 05423b2 "vlan: allow null VLAN ID to be used"
(and at time introduced, the 'vlan_tx_tag_get' call in em_meta was not
adapted).
Fix, testing skb_vlan_tag_present instead of testing skb_vlan_tag_get's
value.
Fixes: 05423b2413 ("vlan: allow null VLAN ID to be used")
Fixes: 1a31f2042e ("netsched: Allow meta match on vlan tag on receive")
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/em_meta.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
index a309a07ccb..41c80b6c39 100644
--- a/net/sched/em_meta.c
+++ b/net/sched/em_meta.c
@@ -176,11 +176,12 @@ META_COLLECTOR(int_vlan_tag)
{
unsigned short tag;
- tag = skb_vlan_tag_get(skb);
- if (!tag && __vlan_get_tag(skb, &tag))
- *err = -1;
- else
+ if (skb_vlan_tag_present(skb))
+ dst->value = skb_vlan_tag_get(skb);
+ else if (!__vlan_get_tag(skb, &tag))
dst->value = tag;
+ else
+ *err = -1;
}
--
2.7.4
^ permalink raw reply related
* [Patch net] ipv4: use the right lock for ping_group_range
From: Cong Wang @ 2016-10-20 21:19 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Eric Dumazet, Eric Salo
This reverts commit a681574c99be23e4d20b769bf0e543239c364af5
("ipv4: disable BH in set_ping_group_range()") because we never
read ping_group_range in BH context (unlike local_port_range).
Then, since we already have a lock for ping_group_range, those
using ip_local_ports.lock for ping_group_range are clearly typos.
We might consider to share a same lock for both ping_group_range
and local_port_range w.r.t. space saving, but that should be for
net-next.
Fixes: a681574c99be ("ipv4: disable BH in set_ping_group_range()")
Fixes: ba6b918ab234 ("ping: move ping_group_range out of CONFIG_SYSCTL")
Cc: Eric Dumazet <edumazet@google.com>
Cc: Eric Salo <salo@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/ipv4/sysctl_net_ipv4.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 500ae40..80bc36b 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -96,11 +96,11 @@ static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low
container_of(table->data, struct net, ipv4.ping_group_range.range);
unsigned int seq;
do {
- seq = read_seqbegin(&net->ipv4.ip_local_ports.lock);
+ seq = read_seqbegin(&net->ipv4.ping_group_range.lock);
*low = data[0];
*high = data[1];
- } while (read_seqretry(&net->ipv4.ip_local_ports.lock, seq));
+ } while (read_seqretry(&net->ipv4.ping_group_range.lock, seq));
}
/* Update system visible IP port range */
@@ -109,10 +109,10 @@ static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t hig
kgid_t *data = table->data;
struct net *net =
container_of(table->data, struct net, ipv4.ping_group_range.range);
- write_seqlock_bh(&net->ipv4.ip_local_ports.lock);
+ write_seqlock(&net->ipv4.ping_group_range.lock);
data[0] = low;
data[1] = high;
- write_sequnlock_bh(&net->ipv4.ip_local_ports.lock);
+ write_sequnlock(&net->ipv4.ping_group_range.lock);
}
/* Validate changes from /proc interface. */
--
2.1.0
^ permalink raw reply related
* RE: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for VF support
From: Vatsavayi, Raghu @ 2016-10-20 21:15 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, Chickles, Derek, Burla, Satananda,
Manlunas, Felix
In-Reply-To: <20161020.165720.1537348879596195765.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, October 20, 2016 1:57 PM
> To: Vatsavayi, Raghu
> Cc: netdev@vger.kernel.org; Chickles, Derek; Burla, Satananda; Manlunas,
> Felix
> Subject: Re: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for VF
> support
>
> From: "Vatsavayi, Raghu" <Raghu.Vatsavayi@cavium.com>
> Date: Thu, 20 Oct 2016 20:01:37 +0000
>
> >
> >
> >> -----Original Message-----
> >> From: David Miller [mailto:davem@davemloft.net]
> >> Sent: Thursday, October 20, 2016 11:13 AM
> >> To: Vatsavayi, Raghu
> >> Cc: netdev@vger.kernel.org; Vatsavayi, Raghu; Chickles, Derek; Burla,
> >> Satananda; Manlunas, Felix
> >> Subject: Re: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for
> >> VF support
> >>
> >> From: Raghu Vatsavayi <rvatsavayi@caviumnetworks.com>
> >> Date: Wed, 19 Oct 2016 22:40:38 -0700
> >>
> >> > +/* Default behaviour of Liquidio is to provide one queue per VF.
> >> > +But Liquidio
> >> > + * can also provide multiple queues to each VF. If user wants to
> >> > +change the
> >> > + * default behaviour HW should be provided configuration info at
> >> > +init time,
> >> > + * based on which it will create control queues for communicating
> >> > +with
> >> FW.
> >> > + */
> >> > +static u32 max_vfs[2] = { 0, 0 };
> >> > +module_param_array(max_vfs, int, NULL, 0444);
> >> > +MODULE_PARM_DESC(max_vfs, "Assign two comma-separated
> unsigned
> >> > +integers that specify max number of VFs for PF0 (left of the
> >> > +comma) and PF1 (right of the comma); for 23xx only. By default HW
> >> > +will configure as many VFs as queues after allocating PF queues.To
> >> > +increase queues for VF use this parameter. Use sysfs to create
> >> > +these VFs.");
> >> > +
> >> > +static unsigned int num_queues_per_pf[2] = { 0, 0 };
> >> > +module_param_array(num_queues_per_pf, uint, NULL, 0444);
> >> > +MODULE_PARM_DESC(num_queues_per_pf, "two comma-separated
> >> unsigned
> >> > +integers that specify number of queues per PF0 (left of the comma)
> >> > +and PF1 (right of the comma); for 23xx only");
> >> > +
> >> > static int ptp_enable = 1;
> >>
> >> We cannot continue to allow drivers to add custom module parameters
> >> to control this. It is the worst user experience possible.
> >>
> >> We need a tree-wide generic, consistent, manner in which to configure
> >> and control this kind of thing.
> >
> > Sure Dave, I will remove max_vfs module parameter and will use tree
> > wide generic sysfs interface to enable VFs.
>
> That's not what I meant.
>
> I mean there needs to be a generic mechanism that isn't a per-device knob
> (be it a module parameter or a sysctl, to me these are identical functionality
> and user experience wise).
>
> Something like ethtool or netlink.
Dave, I will remove max_vfs module parameter and will just use the generic mechanism that
all drivers do like:
" echo 10 > /sys/devices/pci0000:00/0000:00:03.0/0000:03:00.1/sriov_numvfs"
Regards
Raghu.
^ permalink raw reply
* Re: [PATCH] net: l2tp_eth: fix max_mtu
From: Asbjoern Sloth Toennesen @ 2016-10-20 21:19 UTC (permalink / raw)
To: David S . Miller; +Cc: linux-kernel, netdev, Jarod Wilson, R Parameswaran
In-Reply-To: <20161020200826.24578-1-asbjorn@asbjorn.st>
[-- Attachment #1: Type: text/plain, Size: 862 bytes --]
On Thu, 20 Oct 2016 20:08:26 +0000 (UTC), ast@asbjorn.biz wrote:
> diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
> index 965f7e3..ba82dcc 100644
> --- a/net/l2tp/l2tp_eth.c
> +++ b/net/l2tp/l2tp_eth.c
> @@ -259,6 +259,7 @@ static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 p
> session->mtu = dev->mtu - session->hdr_len;
> dev->mtu = session->mtu;
> dev->needed_headroom += session->hdr_len;
> + dev->max_mtu = ETH_MAX_MTU - dev->needed_headroom;
I forgot to add that subtracting dev->needed_headroom doesn't
give the exact maximum MTU, but one that is a bit too high.
It could also just be set to ETH_MAX_MTU, to indicate that
the encapsulation is not included.
Calculations like in R. Parameswaran's patch is needed for the
exact one.
https://lkml.org/lkml/2016/10/17/3
--
Best regards
Asbjørn Sloth Tønnesen
^ permalink raw reply
* Re: [Patch net v2] ipv6: fix a potential deadlock in do_ipv6_setsockopt()
From: Cong Wang @ 2016-10-20 21:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Baozeng Ding,
Marcelo Ricardo Leitner
In-Reply-To: <1476971226.7065.5.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Oct 20, 2016 at 6:47 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2016-10-19 at 23:35 -0700, Cong Wang wrote:
>> Baozeng reported this deadlock case:
>
> ...
>
>> +
>> +void ipv6_sock_mc_close(struct sock *sk)
>> +{
>> + struct ipv6_pinfo *np = inet6_sk(sk);
>> +
>> + if (!rcu_access_pointer(np->ipv6_mc_list))
>> + return;
>
> I wonder if rcu_dereference_protected(..., lockdep_sock_is_held(sk))
> could be used instead, to get lockdep support ?
Maybe, but this "problem" exists without my patch too, right?
^ permalink raw reply
* Re: [Patch net v2] ipv6: fix a potential deadlock in do_ipv6_setsockopt()
From: Eric Dumazet @ 2016-10-20 22:10 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, Baozeng Ding,
Marcelo Ricardo Leitner
In-Reply-To: <CAM_iQpUvU8dZPquAgtcZASO6jhjtoYQ2nc8EBceqZmSHg4ZSsQ@mail.gmail.com>
On Thu, 2016-10-20 at 14:35 -0700, Cong Wang wrote:
> On Thu, Oct 20, 2016 at 6:47 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Wed, 2016-10-19 at 23:35 -0700, Cong Wang wrote:
> >> Baozeng reported this deadlock case:
> >
> > ...
> >
> >> +
> >> +void ipv6_sock_mc_close(struct sock *sk)
> >> +{
> >> + struct ipv6_pinfo *np = inet6_sk(sk);
> >> +
> >> + if (!rcu_access_pointer(np->ipv6_mc_list))
> >> + return;
> >
> > I wonder if rcu_dereference_protected(..., lockdep_sock_is_held(sk))
> > could be used instead, to get lockdep support ?
>
> Maybe, but this "problem" exists without my patch too, right?
I used 'I wonder if' to say that we might have some better way to code
this test nowadays, but this can be done in a separate patch of course.
^ permalink raw reply
* Unexpected behaviour of suppress_prefixlength 0
From: Matthias Peter Walther @ 2016-10-20 22:00 UTC (permalink / raw)
To: netdev
Hello,
I'm Matthias and I'm new to this list. I just signed up, to ask the
following question.
I have a configuration like this:
root@des1 ~ # ip rule
0: from all lookup local
32765: from all iif lo lookup ffnet suppress_prefixlength 0
32766: from all lookup main
32767: from all lookup default
(ffnet is table 42)
root@des1 ~ # ip r s
default via 5.9.86.151 dev eth0
5.9.86.151 dev eth0 proto kernel scope link src 5.9.86.144
root@des1 ~ # ip r s t 42
blackhole default
I have the default routing table, and a routing table number 42. I could
use an ip rule filtering by destination ip, but I wanted to try
suppress_prefixlength.
Let's say I want to ping 8.8.8.8. What I expect is, that the package is
put into routing table 42 by the ip rule 32765. As there is no more
specific route for 8.8.8.8 than the default route in table 42, I expect
the suppress_prefixlength 0 option to put it back to the default routing
table and then to be send out through eth0.
Instead this configuration takes the whole machine offline:
root@des1 ~ # ping 8.8.8.8
connect: Invalid argument
When I delete the ip rule 32765 containing the suppress_prefixlength,
the machine is back online.
Do I not understand the suppress_prefixlength-feature correctly or is
this a bug? I tested with Kernel 4.7 and 4.6, both show the same
behaviour as described above.
Thanks for any replies in advance.
Regards,
Matthias
^ permalink raw reply
* linux-next: build failure after merge of the net tree
From: Stephen Rothwell @ 2016-10-20 22:27 UTC (permalink / raw)
To: David Miller, Networking; +Cc: linux-next, linux-kernel, Florian Fainelli
Hi all,
After merging the net tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:
ERROR: "kexec_in_progress" [drivers/net/dsa/bcm_sf2.ko] undefined!
Caused by commit
2399d6143f85 ("net: dsa: bcm_sf2: Prevent GPHY shutdown for kexec'd kernels")
I used the version of the net tree from next-20161020 for today.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* [PATCH net-next 1/2] bpf: add helper for retrieving current numa node id
From: Daniel Borkmann @ 2016-10-20 22:30 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, edumazet, netdev, Daniel Borkmann
In-Reply-To: <cover.1477002095.git.daniel@iogearbox.net>
Use case is mainly for soreuseport to select sockets for the local
numa node, but since generic, lets also add this for other networking
and tracing program types.
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 6 ++++++
kernel/bpf/helpers.c | 12 ++++++++++++
kernel/trace/bpf_trace.c | 2 ++
net/core/filter.c | 2 ++
5 files changed, 23 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c201017..edcd96d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -319,6 +319,7 @@ static inline struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog)
extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
+extern const struct bpf_func_proto bpf_get_numa_node_id_proto;
extern const struct bpf_func_proto bpf_tail_call_proto;
extern const struct bpf_func_proto bpf_ktime_get_ns_proto;
extern const struct bpf_func_proto bpf_get_current_pid_tgid_proto;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index f09c70b..4ae1b66 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -426,6 +426,12 @@ enum bpf_func_id {
*/
BPF_FUNC_set_hash_invalid,
+ /**
+ * bpf_get_numa_node_id()
+ * Returns the id of the current NUMA node.
+ */
+ BPF_FUNC_get_numa_node_id,
+
__BPF_FUNC_MAX_ID,
};
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 3991840..045cbe6 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -13,6 +13,7 @@
#include <linux/rcupdate.h>
#include <linux/random.h>
#include <linux/smp.h>
+#include <linux/topology.h>
#include <linux/ktime.h>
#include <linux/sched.h>
#include <linux/uidgid.h>
@@ -92,6 +93,17 @@
.ret_type = RET_INTEGER,
};
+BPF_CALL_0(bpf_get_numa_node_id)
+{
+ return numa_node_id();
+}
+
+const struct bpf_func_proto bpf_get_numa_node_id_proto = {
+ .func = bpf_get_numa_node_id,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+
BPF_CALL_0(bpf_ktime_get_ns)
{
/* NMI safe access to clock monotonic */
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 5dcb992..fa77311 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -422,6 +422,8 @@ static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
return bpf_get_trace_printk_proto();
case BPF_FUNC_get_smp_processor_id:
return &bpf_get_smp_processor_id_proto;
+ case BPF_FUNC_get_numa_node_id:
+ return &bpf_get_numa_node_id_proto;
case BPF_FUNC_perf_event_read:
return &bpf_perf_event_read_proto;
case BPF_FUNC_probe_write_user:
diff --git a/net/core/filter.c b/net/core/filter.c
index 00351cd..cd9e2ba 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2492,6 +2492,8 @@ static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
return &bpf_get_prandom_u32_proto;
case BPF_FUNC_get_smp_processor_id:
return &bpf_get_raw_smp_processor_id_proto;
+ case BPF_FUNC_get_numa_node_id:
+ return &bpf_get_numa_node_id_proto;
case BPF_FUNC_tail_call:
return &bpf_tail_call_proto;
case BPF_FUNC_ktime_get_ns:
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 0/2] Add BPF numa id helper
From: Daniel Borkmann @ 2016-10-20 22:30 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, edumazet, netdev, Daniel Borkmann
This patch set adds a helper for retrieving current numa node
id and a test case for SO_REUSEPORT.
Thanks!
Daniel Borkmann (2):
bpf: add helper for retrieving current numa node id
reuseport, bpf: add test case for bpf_get_numa_node_id
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 6 +
kernel/bpf/helpers.c | 12 ++
kernel/trace/bpf_trace.c | 2 +
net/core/filter.c | 2 +
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/Makefile | 11 +-
tools/testing/selftests/net/reuseport_bpf_numa.c | 255 +++++++++++++++++++++++
8 files changed, 286 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/net/reuseport_bpf_numa.c
--
1.9.3
^ permalink raw reply
* [PATCH net-next 2/2] reuseport, bpf: add test case for bpf_get_numa_node_id
From: Daniel Borkmann @ 2016-10-20 22:30 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, edumazet, netdev, Daniel Borkmann
In-Reply-To: <cover.1477002095.git.daniel@iogearbox.net>
The test case is very similar to reuseport_bpf_cpu, only that here
we select socket members based on current numa node id.
# numactl -H
available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 12 13 14 15 16 17
node 0 size: 128867 MB
node 0 free: 120080 MB
node 1 cpus: 6 7 8 9 10 11 18 19 20 21 22 23
node 1 size: 96765 MB
node 1 free: 87504 MB
node distances:
node 0 1
0: 10 20
1: 20 10
# ./reuseport_bpf_numa
---- IPv4 UDP ----
send node 0, receive socket 0
send node 1, receive socket 1
send node 1, receive socket 1
send node 0, receive socket 0
---- IPv6 UDP ----
send node 0, receive socket 0
send node 1, receive socket 1
send node 1, receive socket 1
send node 0, receive socket 0
---- IPv4 TCP ----
send node 0, receive socket 0
send node 1, receive socket 1
send node 1, receive socket 1
send node 0, receive socket 0
---- IPv6 TCP ----
send node 0, receive socket 0
send node 1, receive socket 1
send node 1, receive socket 1
send node 0, receive socket 0
SUCCESS
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/Makefile | 11 +-
tools/testing/selftests/net/reuseport_bpf_numa.c | 255 +++++++++++++++++++++++
3 files changed, 263 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/net/reuseport_bpf_numa.c
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 0840684..afe109e 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -3,4 +3,5 @@ psock_fanout
psock_tpacket
reuseport_bpf
reuseport_bpf_cpu
+reuseport_bpf_numa
reuseport_dualstack
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 0e53407..e24e4c8 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -1,14 +1,17 @@
# Makefile for net selftests
-CFLAGS = -Wall -O2 -g
-
+CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
CFLAGS += -I../../../../usr/include/
-NET_PROGS = socket psock_fanout psock_tpacket reuseport_bpf reuseport_bpf_cpu reuseport_dualstack
+NET_PROGS = socket
+NET_PROGS += psock_fanout psock_tpacket
+NET_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
+NET_PROGS += reuseport_dualstack
all: $(NET_PROGS)
+reuseport_bpf_numa: LDFLAGS += -lnuma
%: %.c
- $(CC) $(CFLAGS) -o $@ $^
+ $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh
TEST_FILES := $(NET_PROGS)
diff --git a/tools/testing/selftests/net/reuseport_bpf_numa.c b/tools/testing/selftests/net/reuseport_bpf_numa.c
new file mode 100644
index 0000000..6f20bc9
--- /dev/null
+++ b/tools/testing/selftests/net/reuseport_bpf_numa.c
@@ -0,0 +1,255 @@
+/*
+ * Test functionality of BPF filters with SO_REUSEPORT. Same test as
+ * in reuseport_bpf_cpu, only as one socket per NUMA node.
+ */
+
+#define _GNU_SOURCE
+
+#include <arpa/inet.h>
+#include <errno.h>
+#include <error.h>
+#include <linux/filter.h>
+#include <linux/bpf.h>
+#include <linux/in.h>
+#include <linux/unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/epoll.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#include <numa.h>
+
+static const int PORT = 8888;
+
+static void build_rcv_group(int *rcv_fd, size_t len, int family, int proto)
+{
+ struct sockaddr_storage addr;
+ struct sockaddr_in *addr4;
+ struct sockaddr_in6 *addr6;
+ size_t i;
+ int opt;
+
+ switch (family) {
+ case AF_INET:
+ addr4 = (struct sockaddr_in *)&addr;
+ addr4->sin_family = AF_INET;
+ addr4->sin_addr.s_addr = htonl(INADDR_ANY);
+ addr4->sin_port = htons(PORT);
+ break;
+ case AF_INET6:
+ addr6 = (struct sockaddr_in6 *)&addr;
+ addr6->sin6_family = AF_INET6;
+ addr6->sin6_addr = in6addr_any;
+ addr6->sin6_port = htons(PORT);
+ break;
+ default:
+ error(1, 0, "Unsupported family %d", family);
+ }
+
+ for (i = 0; i < len; ++i) {
+ rcv_fd[i] = socket(family, proto, 0);
+ if (rcv_fd[i] < 0)
+ error(1, errno, "failed to create receive socket");
+
+ opt = 1;
+ if (setsockopt(rcv_fd[i], SOL_SOCKET, SO_REUSEPORT, &opt,
+ sizeof(opt)))
+ error(1, errno, "failed to set SO_REUSEPORT");
+
+ if (bind(rcv_fd[i], (struct sockaddr *)&addr, sizeof(addr)))
+ error(1, errno, "failed to bind receive socket");
+
+ if (proto == SOCK_STREAM && listen(rcv_fd[i], len * 10))
+ error(1, errno, "failed to listen on receive port");
+ }
+}
+
+static void attach_bpf(int fd)
+{
+ static char bpf_log_buf[65536];
+ static const char bpf_license[] = "";
+
+ int bpf_fd;
+ const struct bpf_insn prog[] = {
+ /* R0 = bpf_get_numa_node_id() */
+ { BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_numa_node_id },
+ /* return R0 */
+ { BPF_JMP | BPF_EXIT, 0, 0, 0, 0 }
+ };
+ union bpf_attr attr;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+ attr.insn_cnt = sizeof(prog) / sizeof(prog[0]);
+ attr.insns = (unsigned long) &prog;
+ attr.license = (unsigned long) &bpf_license;
+ attr.log_buf = (unsigned long) &bpf_log_buf;
+ attr.log_size = sizeof(bpf_log_buf);
+ attr.log_level = 1;
+
+ bpf_fd = syscall(__NR_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
+ if (bpf_fd < 0)
+ error(1, errno, "ebpf error. log:\n%s\n", bpf_log_buf);
+
+ if (setsockopt(fd, SOL_SOCKET, SO_ATTACH_REUSEPORT_EBPF, &bpf_fd,
+ sizeof(bpf_fd)))
+ error(1, errno, "failed to set SO_ATTACH_REUSEPORT_EBPF");
+
+ close(bpf_fd);
+}
+
+static void send_from_node(int node_id, int family, int proto)
+{
+ struct sockaddr_storage saddr, daddr;
+ struct sockaddr_in *saddr4, *daddr4;
+ struct sockaddr_in6 *saddr6, *daddr6;
+ int fd;
+
+ switch (family) {
+ case AF_INET:
+ saddr4 = (struct sockaddr_in *)&saddr;
+ saddr4->sin_family = AF_INET;
+ saddr4->sin_addr.s_addr = htonl(INADDR_ANY);
+ saddr4->sin_port = 0;
+
+ daddr4 = (struct sockaddr_in *)&daddr;
+ daddr4->sin_family = AF_INET;
+ daddr4->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+ daddr4->sin_port = htons(PORT);
+ break;
+ case AF_INET6:
+ saddr6 = (struct sockaddr_in6 *)&saddr;
+ saddr6->sin6_family = AF_INET6;
+ saddr6->sin6_addr = in6addr_any;
+ saddr6->sin6_port = 0;
+
+ daddr6 = (struct sockaddr_in6 *)&daddr;
+ daddr6->sin6_family = AF_INET6;
+ daddr6->sin6_addr = in6addr_loopback;
+ daddr6->sin6_port = htons(PORT);
+ break;
+ default:
+ error(1, 0, "Unsupported family %d", family);
+ }
+
+ if (numa_run_on_node(node_id) < 0)
+ error(1, errno, "failed to pin to node");
+
+ fd = socket(family, proto, 0);
+ if (fd < 0)
+ error(1, errno, "failed to create send socket");
+
+ if (bind(fd, (struct sockaddr *)&saddr, sizeof(saddr)))
+ error(1, errno, "failed to bind send socket");
+
+ if (connect(fd, (struct sockaddr *)&daddr, sizeof(daddr)))
+ error(1, errno, "failed to connect send socket");
+
+ if (send(fd, "a", 1, 0) < 0)
+ error(1, errno, "failed to send message");
+
+ close(fd);
+}
+
+static
+void receive_on_node(int *rcv_fd, int len, int epfd, int node_id, int proto)
+{
+ struct epoll_event ev;
+ int i, fd;
+ char buf[8];
+
+ i = epoll_wait(epfd, &ev, 1, -1);
+ if (i < 0)
+ error(1, errno, "epoll_wait failed");
+
+ if (proto == SOCK_STREAM) {
+ fd = accept(ev.data.fd, NULL, NULL);
+ if (fd < 0)
+ error(1, errno, "failed to accept");
+ i = recv(fd, buf, sizeof(buf), 0);
+ close(fd);
+ } else {
+ i = recv(ev.data.fd, buf, sizeof(buf), 0);
+ }
+
+ if (i < 0)
+ error(1, errno, "failed to recv");
+
+ for (i = 0; i < len; ++i)
+ if (ev.data.fd == rcv_fd[i])
+ break;
+ if (i == len)
+ error(1, 0, "failed to find socket");
+ fprintf(stderr, "send node %d, receive socket %d\n", node_id, i);
+ if (node_id != i)
+ error(1, 0, "node id/receive socket mismatch");
+}
+
+static void test(int *rcv_fd, int len, int family, int proto)
+{
+ struct epoll_event ev;
+ int epfd, node;
+
+ build_rcv_group(rcv_fd, len, family, proto);
+ attach_bpf(rcv_fd[0]);
+
+ epfd = epoll_create(1);
+ if (epfd < 0)
+ error(1, errno, "failed to create epoll");
+ for (node = 0; node < len; ++node) {
+ ev.events = EPOLLIN;
+ ev.data.fd = rcv_fd[node];
+ if (epoll_ctl(epfd, EPOLL_CTL_ADD, rcv_fd[node], &ev))
+ error(1, errno, "failed to register sock epoll");
+ }
+
+ /* Forward iterate */
+ for (node = 0; node < len; ++node) {
+ send_from_node(node, family, proto);
+ receive_on_node(rcv_fd, len, epfd, node, proto);
+ }
+
+ /* Reverse iterate */
+ for (node = len - 1; node >= 0; --node) {
+ send_from_node(node, family, proto);
+ receive_on_node(rcv_fd, len, epfd, node, proto);
+ }
+
+ close(epfd);
+ for (node = 0; node < len; ++node)
+ close(rcv_fd[node]);
+}
+
+int main(void)
+{
+ int *rcv_fd, nodes;
+
+ if (numa_available() < 0)
+ error(1, errno, "no numa api support");
+
+ nodes = numa_max_node() + 1;
+
+ rcv_fd = calloc(nodes, sizeof(int));
+ if (!rcv_fd)
+ error(1, 0, "failed to allocate array");
+
+ fprintf(stderr, "---- IPv4 UDP ----\n");
+ test(rcv_fd, nodes, AF_INET, SOCK_DGRAM);
+
+ fprintf(stderr, "---- IPv6 UDP ----\n");
+ test(rcv_fd, nodes, AF_INET6, SOCK_DGRAM);
+
+ fprintf(stderr, "---- IPv4 TCP ----\n");
+ test(rcv_fd, nodes, AF_INET, SOCK_STREAM);
+
+ fprintf(stderr, "---- IPv6 TCP ----\n");
+ test(rcv_fd, nodes, AF_INET6, SOCK_STREAM);
+
+ free(rcv_fd);
+
+ fprintf(stderr, "SUCCESS\n");
+ return 0;
+}
--
1.9.3
^ permalink raw reply related
* Re: linux-next: build failure after merge of the net tree
From: Florian Fainelli @ 2016-10-20 22:42 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, Networking; +Cc: linux-next, linux-kernel
In-Reply-To: <20161021092710.1b56a35b@canb.auug.org.au>
On 10/20/2016 03:27 PM, Stephen Rothwell wrote:
> Hi all,
>
> After merging the net tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
>
> ERROR: "kexec_in_progress" [drivers/net/dsa/bcm_sf2.ko] undefined!
>
> Caused by commit
>
> 2399d6143f85 ("net: dsa: bcm_sf2: Prevent GPHY shutdown for kexec'd kernels")
>
> I used the version of the net tree from next-20161020 for today.
OK, seems like we need an ifdef CONFIG_KEXEC_CORE, let me fix that.
Thanks Stephen!
--
Florian
^ permalink raw reply
* Re: [PATCH 05/10] gpio: Introduce SAM gpio driver
From: Linus Walleij @ 2016-10-20 23:06 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Lee Jones, Alexandre Courbot, Rob Herring, Mark Rutland,
Frank Rowand, Wolfram Sang, David Woodhouse, Brian Norris,
Florian Fainelli, Wim Van Sebroeck, Peter Rosin, Debjit Ghosh,
Georgi Vlaev, Guenter Roeck, Maryam Seraj,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-gpio@vger.kernel.org, "linux-i2c@vger.kernel.or
In-Reply-To: <1475853518-22264-6-git-send-email-pantelis.antoniou@konsulko.com>
n Fri, Oct 7, 2016 at 5:18 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> From: Guenter Roeck <groeck@juniper.net>
>
> The SAM GPIO IP block is present in the Juniper PTX series
> of routers as part of the SAM FPGA.
>
> Signed-off-by: Georgi Vlaev <gvlaev@juniper.net>
> Signed-off-by: Guenter Roeck <groeck@juniper.net>
> Signed-off-by: Rajat Jain <rajatjain@juniper.net>
> [Ported from Juniper kernel]
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
First copy/paste my other review comments on the previous driver
I reviewed, this seems to have pretty much all the same issues.
> +config GPIO_SAM
> + tristate "SAM FPGA GPIO"
> + depends on MFD_JUNIPER_SAM
> + default y if MFD_JUNIPER_SAM
I suspect this should use
select GPIOLIB_IRQCHIP
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include <linux/gpio.h>
<linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqdomain.h>
Not needed with GPIOLIB_IRQCHIP
> +#include <linux/errno.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
Why?
> +#include <linux/mfd/sam.h>
> +
> +/* gpio status/configuration */
> +#define SAM_GPIO_NEG_EDGE (1 << 8)
> +#define SAM_GPIO_NEG_EDGE_EN (1 << 7)
> +#define SAM_GPIO_POS_EDGE (1 << 6)
> +#define SAM_GPIO_POS_EDGE_EN (1 << 5)
Interrupt triggers I suppose.
> +#define SAM_GPIO_BLINK (1 << 4)
Cool, we don't support that in gpiolib as of now.
Maybe we should.
> +#define SAM_GPIO_OUT (1 << 3)
> +#define SAM_GPIO_OUT_TS (1 << 2)
OUT_TS ... what does TS mean here?
> +#define SAM_GPIO_DEBOUNCE_EN (1 << 1)
> +#define SAM_GPIO_IN (1 << 0)
> +
> +#define SAM_GPIO_BASE 0x1000
Base into what, and why is this not coming from PCI
or the device tree?
> +#define SAM_MAX_NGPIO 512
Why do we need to know this and what does it really mean?
That is the max number the GPIO subsystem can handle by
the way.
> +#define SAM_GPIO_ADDR(addr, nr) ((addr) + SAM_GPIO_BASE + (nr) * sizeof(u32))
Why can't we just offset the address earlier, ah well it's OK.
> +struct sam_gpio_irq_group {
> + int start; /* 1st gpio pin */
> + int count; /* # of pins in group */
> + int num_enabled; /* # of enabled interrupts */
> +};
> +
> +/**
> + * struct sam_gpio - GPIO private data structure.
> + * @base: PCI base address of Memory mapped I/O register.
> + * @dev: Pointer to device structure.
> + * @gpio: Data for GPIO infrastructure.
> + * @gpio_base: 1st gpio pin
> + * @gpio_count: # of gpio pins
> + * @irq_lock: Lock used by interrupt subsystem
> + * @domain: Pointer to interrupt domain
> + * @irq: Interrupt # from parent
> + * @irq_high: Second interrupt # from parent
> + * (currently unused)
> + * @irq_group: Interrupt group descriptions
> + * (one group per interrupt bit)
> + * @irq_type: The interrupt type for each gpio pin
> + */
Why do you need to keep all of this around? Is is all really
used? gpio_base makes me nervous we generally use dynamic
allocation of GPIO numbers these days.
> +struct sam_gpio {
> + void __iomem *base;
> + struct device *dev;
> + struct gpio_chip gpio;
> + int gpio_base;
> + int gpio_count;
> + struct mutex irq_lock;
> + struct irq_domain *domain;
> + int irq;
> + int irq_high;
> + struct sam_gpio_irq_group irq_group[18];
> + u8 irq_type[SAM_MAX_NGPIO];
> + struct sam_platform_data *pdata;
> + const char **names;
> + u32 *export_flags;
> +};
> +#define to_sam(chip) container_of((chip), struct sam_gpio, gpio)
Instead of this use gpiochip_get_data(). Applies everywhere.
> +static void sam_gpio_bitop(struct sam_gpio *sam, unsigned int nr,
> + u32 bit, bool set)
> +{
> + u32 reg;
> +
> + reg = ioread32(SAM_GPIO_ADDR(sam->base, nr));
> + if (set)
> + reg |= bit;
> + else
> + reg &= ~bit;
> + iowrite32(reg, SAM_GPIO_ADDR(sam->base, nr));
> + ioread32(SAM_GPIO_ADDR(sam->base, nr));
> +}
Does that rally need a helper function?
Use BIT() and inline like I explained in the previous patch.
> +static void sam_gpio_setup(struct sam_gpio *sam)
> +{
> + struct gpio_chip *chip = &sam->gpio;
> +
> + chip->parent = sam->dev;
> + chip->label = dev_name(sam->dev);
> + chip->owner = THIS_MODULE;
> + chip->direction_input = sam_gpio_direction_input;
> + chip->get = sam_gpio_get;
> + chip->direction_output = sam_gpio_direction_output;
Implement also chip->get_direction
> + chip->set = sam_gpio_set;
> + chip->set_debounce = sam_gpio_debounce;
> + chip->dbg_show = NULL;
> + chip->base = sam->gpio_base;
Oh no, why. Use -1 please and let gpiolib decide...
> + chip->ngpio = sam->gpio_count;
> +#ifdef CONFIG_OF_GPIO
> + chip->of_node = sam->dev->of_node;
> +#endif
I doubt this #ifdef actually. If the driver needs CONFIG_OF_GPIO to
work it should just depend on it in Kconfig.
> +static int sam_of_get_exports(struct device *dev, struct sam_gpio *sam)
> +{
> + struct device_node *child, *exports;
> + int err = 0;
> +
> + if (dev->of_node == NULL)
> + return 0; /* No FDT node, we are done */
> +
> + exports = of_get_child_by_name(dev->of_node, "gpio-exports");
> + if (exports == NULL)
> + return 0; /* No exports, we are done */
> +
> + if (of_get_child_count(exports) == 0)
> + return 0; /* No children, we are done */
> +
> + sam->names = devm_kzalloc(dev, sizeof(char *) * sam->gpio_count,
> + GFP_KERNEL);
> + if (sam->names == NULL) {
> + err = -ENOMEM;
> + goto error;
> + }
> + sam->export_flags =
> + devm_kzalloc(dev, sizeof(u32) * sam->gpio_count, GFP_KERNEL);
> + if (sam->export_flags == NULL) {
> + err = -ENOMEM;
> + goto error;
> + }
> + for_each_child_of_node(exports, child) {
> + const char *label;
> + u32 pin, flags;
> +
> + label = of_get_property(child, "label", NULL) ? : child->name;
> + err = of_property_read_u32_index(child, "pin", 0, &pin);
> + if (err)
> + break;
> + if (pin >= sam->gpio_count) {
> + err = -EINVAL;
> + break;
> + }
> + err = of_property_read_u32_index(child, "pin", 1, &flags);
> + if (err)
> + break;
> + /*
> + * flags:
> + * GPIOF_DIR_IN bit 0=1
> + * GPIOF_DIR_OUT bit 0=0
> + * GPIOF_INIT_HIGH bit 1=1
> + * GPIOF_ACTIVE_LOW bit 2=1
> + * GPIOF_OPEN_DRAIN bit 3=1
> + * GPIOF_OPEN_SOURCE bit 4=1
> + * GPIOF_EXPORT bit 5=1
> + * GPIOF_EXPORT_CHANGEABLE bit 6=1
> + */
> + sam->names[pin] = label;
> + sam->export_flags[pin] = flags;
> + }
> +error:
> + of_node_put(exports);
> + return err;
> +}
What? NAK never in my life. This looks like an old hack to
export stuff to userspace. We don't do that. The kernel supports
gpio-line-names to name lines in the device tree, and you can use
the new chardev ABI to access it from userspace, sysfs is dead.
Delete this function entirely.
> +static int sam_gpio_of_init(struct device *dev, struct sam_gpio *sam)
> +{
> + int err;
> + u32 val;
> + const u32 *igroup;
> + u32 group, start, count;
> + int i, iglen, ngpio;
> +
> + if (of_have_populated_dt() && !dev->of_node) {
> + dev_err(dev, "No device node\n");
> + return -ENODEV;
> + }
So obviously this driver Kconfig should depend on OF_GPIO.
> +
> + err = of_property_read_u32(dev->of_node, "gpio-base", &val);
> + if (err)
> + val = -1;
> + sam->gpio_base = val;
NAK, No Linux bases in the device tree. Only use -1.
> + err = of_property_read_u32(dev->of_node, "gpio-count", &val);
> + if (!err) {
> + if (val > SAM_MAX_NGPIO)
> + val = SAM_MAX_NGPIO;
> + sam->gpio_count = val;
> + }
As described in the generic bindings, use "ngpios" for this if you need it.
> + igroup = of_get_property(dev->of_node, "gpio-interrupts", &iglen);
NAK on that binding.
> + if (igroup) {
> + iglen /= sizeof(u32);
> + if (iglen < 3 || iglen % 3)
> + return -EINVAL;
> + iglen /= 3;
> + for (i = 0; i < iglen; i++) {
> + group = be32_to_cpu(igroup[i * 3]);
> + if (group >= ARRAY_SIZE(sam->irq_group))
> + return -EINVAL;
> + start = be32_to_cpu(igroup[i * 3 + 1]);
> + count = be32_to_cpu(igroup[i * 3 + 2]);
> + if (start >= sam->gpio_count || count == 0 ||
> + start + count > sam->gpio_count)
> + return -EINVAL;
> + sam->irq_group[group].start = start;
> + sam->irq_group[group].count = count;
> + }
> + }
Do not invent custom interrupt bindings like this. Use the
standard device tree mechanism to resolve IRQs from the parent
controller. Maybe you also need to use hierarchical irqdomain
in Linux.
> +static int sam_gpio_pin_to_irq_bit(struct sam_gpio *sam, int pin)
> +{
> + int bit;
> +
> + for (bit = 0; bit < ARRAY_SIZE(sam->irq_group); bit++) {
> + struct sam_gpio_irq_group *irq_group = &sam->irq_group[bit];
> +
> + if (irq_group->count &&
> + pin >= irq_group->start &&
> + pin <= irq_group->start + irq_group->count)
> + return bit;
> + }
> + return -EINVAL;
> +}
> +
> +static bool sam_gpio_irq_handle_group(struct sam_gpio *sam,
> + struct sam_gpio_irq_group *irq_group)
> +{
> + unsigned int virq = 0;
> + bool handled = false;
> + bool repeat;
> + int i;
> +
> + /* no irq_group for the interrupt bit */
> + if (!irq_group->count)
> + return false;
> +
> + WARN_ON(irq_group->num_enabled == 0);
> + do {
> + repeat = false;
> + for (i = 0; i < irq_group->count; i++) {
> + int pin = irq_group->start + i;
> + bool low, high;
> + u32 regval;
> + u8 type;
> +
> + regval = ioread32(SAM_GPIO_ADDR(sam->base, pin));
> + /*
> + * write back status to clear POS_EDGE and NEG_EDGE
> + * status for this GPIO pin (status bits are
> + * clear-on-one). This is necessary to clear the
> + * high level interrupt status.
> + * Also consider the interrupt to be handled in that
> + * case, even if there is no taker.
> + */
> + if (regval & (SAM_GPIO_POS_EDGE | SAM_GPIO_NEG_EDGE)) {
> + iowrite32(regval,
> + SAM_GPIO_ADDR(sam->base, pin));
> + ioread32(SAM_GPIO_ADDR(sam->base, pin));
> + handled = true;
> + }
> +
> + /*
> + * Check if the pin changed its state.
> + * If it did, and if the expected condition applies,
> + * generate a virtual interrupt.
> + * A pin can only generate an interrupt if
> + * - interrupts are enabled for it
> + * - it is configured as input
> + */
> +
> + if (!sam->irq_type[pin])
> + continue;
> + if (!(regval & SAM_GPIO_OUT_TS))
> + continue;
> +
> + high = regval & (SAM_GPIO_IN | SAM_GPIO_POS_EDGE);
> + low = !(regval & SAM_GPIO_IN) ||
> + (regval & SAM_GPIO_NEG_EDGE);
> + type = sam->irq_type[pin];
> + if (((type & IRQ_TYPE_EDGE_RISING) &&
> + (regval & SAM_GPIO_POS_EDGE)) ||
> + ((type & IRQ_TYPE_EDGE_FALLING) &&
> + (regval & SAM_GPIO_NEG_EDGE)) ||
> + ((type & IRQ_TYPE_LEVEL_LOW) && low) ||
> + ((type & IRQ_TYPE_LEVEL_HIGH) && high)) {
This if() clause does not look sane, you have to see that.
If you think this is proper then explain with detailed comments
what is going on.
> + virq = irq_find_mapping(sam->domain, pin);
> + handle_nested_irq(virq);
> + if (type & (IRQ_TYPE_LEVEL_LOW
> + | IRQ_TYPE_LEVEL_HIGH))
> + repeat = true;
> + }
> + }
> + schedule();
Why? Voluntary preemption? Just use a threaded interrupt handler
instead.
> + } while (repeat);
> +
> + return handled;
> +}
> +
> +static irqreturn_t sam_gpio_irq_handler(int irq, void *data)
> +{
> + struct sam_gpio *sam = data;
> + struct sam_platform_data *pdata = sam->pdata;
> + irqreturn_t ret = IRQ_NONE;
> + bool handled;
> + u32 status;
> +
> + do {
> + handled = false;
> + status = pdata->irq_status(sam->dev->parent, SAM_IRQ_GPIO,
> + sam->irq);
> + pdata->irq_status_clear(sam->dev->parent, SAM_IRQ_GPIO,
> + sam->irq, status);
> + while (status) {
> + unsigned int bit;
> +
> + bit = __ffs(status);
> + status &= ~(1 << bit);
> + handled =
> + sam_gpio_irq_handle_group(sam, &sam->irq_group[bit]);
> + if (handled)
> + ret = IRQ_HANDLED;
> + }
> + } while (handled);
This handled business looks fragile. But OK.
It is a simple IRQ handler, this driver should definately use
GPIOLIB_IRQCHIP. Please look at other drivers doing that
for inspiration. It is also well described in
Documenation/gpio/driver.txt
> +static int sam_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct sam_gpio *sam = to_sam(chip);
> +
> + return irq_create_mapping(sam->domain, offset);
> +}
With GPIOLIB_IRQCHIP you do not need to implement .to_irq()
> +static void sam_irq_mask(struct irq_data *data)
> +{
> + struct sam_gpio *sam = irq_data_get_irq_chip_data(data);
> + struct sam_platform_data *pdata = sam->pdata;
> + int bit = sam_gpio_pin_to_irq_bit(sam, data->hwirq);
> +
> + if (bit < 0)
> + return;
> +
> + if (--sam->irq_group[bit].num_enabled <= 0) {
> + pdata->disable_irq(sam->dev->parent, SAM_IRQ_GPIO, sam->irq,
> + 1 << bit);
Just BIT(bit)
> +static void sam_irq_unmask(struct irq_data *data)
> +{
> + struct sam_gpio *sam = irq_data_get_irq_chip_data(data);
> + struct sam_platform_data *pdata = sam->pdata;
> + int bit = sam_gpio_pin_to_irq_bit(sam, data->hwirq);
> +
> + if (bit < 0)
> + return;
Do you expect this to happen a lot? Else just delete the check or print
an error message.
> +
> + sam->irq_group[bit].num_enabled++;
> + pdata->enable_irq(sam->dev->parent, SAM_IRQ_GPIO, sam->irq, 1 << bit);
Dito.
> +static int sam_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> + struct sam_gpio *sam = irq_data_get_irq_chip_data(data);
> + int bit = sam_gpio_pin_to_irq_bit(sam, data->hwirq);
> +
> + if (bit < 0)
> + return bit;
> +
> + sam->irq_type[data->hwirq] = type & 0x0f;
Why storing this and going to all that trouble?
> + sam_gpio_bitop(sam, data->hwirq, SAM_GPIO_OUT_TS, true);
> + sam_gpio_bitop(sam, data->hwirq, SAM_GPIO_DEBOUNCE_EN, type & 0x10);
> + sam_gpio_bitop(sam, data->hwirq,
> + SAM_GPIO_POS_EDGE_EN | SAM_GPIO_POS_EDGE,
> + type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_LEVEL_HIGH));
> + sam_gpio_bitop(sam, data->hwirq,
> + SAM_GPIO_NEG_EDGE_EN | SAM_GPIO_NEG_EDGE,
> + type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_LEVEL_LOW));
> +
> + return 0;
> +}
Just set up different handlers depending on edge. This likely simplifies your
IRQ handler code as well, as a special function (.irq_ack()) gets called for
edge IRQs.
Look how drivers/gpio/gpio-pl061.c does it.
> +static void sam_irq_bus_lock(struct irq_data *data)
> +{
> + struct sam_gpio *sam = irq_data_get_irq_chip_data(data);
> +
> + mutex_lock(&sam->irq_lock);
> +}
> +
> +static void sam_irq_bus_unlock(struct irq_data *data)
> +{
> + struct sam_gpio *sam = irq_data_get_irq_chip_data(data);
> +
> + /* Synchronize interrupts to chip */
> +
> + mutex_unlock(&sam->irq_lock);
> +}
Aha OK if it's necessary then we need to do this.
> +static struct irq_chip sam_irq_chip = {
> + .name = "gpio-sam",
Maybe this should have an instance number or something.
Just an idea no requirement.
> + .irq_mask = sam_irq_mask,
> + .irq_unmask = sam_irq_unmask,
So I think this needs .irq_ack() to solve the mess above in
the interrupt handler.
> + .irq_set_type = sam_irq_set_type,
> + .irq_bus_lock = sam_irq_bus_lock,
> + .irq_bus_sync_unlock = sam_irq_bus_unlock,
> +};
> +
> +static int sam_gpio_irq_map(struct irq_domain *domain, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_chip_data(irq, domain->host_data);
> + irq_set_chip(irq, &sam_irq_chip);
> + irq_set_nested_thread(irq, true);
> +
> + irq_set_noprobe(irq);
> +
> + return 0;
> +}
This will not be needed if you use GPIOLIB_IRQCHIP
> +static const struct irq_domain_ops sam_gpio_irq_domain_ops = {
> + .map = sam_gpio_irq_map,
> + .xlate = irq_domain_xlate_twocell,
> +};
Nor this.
> +static int sam_gpio_irq_setup(struct device *dev, struct sam_gpio *sam)
> +{
> + int ret;
> +
> + sam->domain = irq_domain_add_linear(dev->of_node,
> + sam->gpio_count,
> + &sam_gpio_irq_domain_ops,
> + sam);
> + if (sam->domain == NULL)
> + return -ENOMEM;
Nor this.
> + ret = devm_request_threaded_irq(dev, sam->irq, NULL,
> + sam_gpio_irq_handler,
> + IRQF_ONESHOT,
> + dev_name(dev), sam);
> + if (ret)
> + goto out_remove_domain;
Are you not setting .can_sleep on the gpiochip?
> +
> + sam->gpio.to_irq = sam_gpio_to_irq;
> +
> + if (!try_module_get(dev->parent->driver->owner)) {
> + ret = -EINVAL;
> + goto out_remove_domain;
> + }
Why? Is that the MFD device? Isn't the device core
handling this?
> +static int sam_gpio_unexport(struct sam_gpio *sam)
> +{
> + int i;
> +
> + if (!sam->export_flags)
> + return 0;
> +
> + /* un-export all auto-exported pins */
> + for (i = 0; i < sam->gpio_count; i++) {
> + struct gpio_desc *desc = gpio_to_desc(sam->gpio.base + i);
> +
> + if (desc == NULL)
> + continue;
> +
> + if (sam->export_flags[i] & GPIOF_EXPORT)
> + gpiochip_free_own_desc(desc);
> + }
> + return 0;
> +}
> +
> +static int sam_gpio_export(struct sam_gpio *sam)
> +{
> + int i, ret;
> +
> + if (!sam->export_flags)
> + return 0;
> +
> + /* auto-export pins as requested */
> +
> + for (i = 0; i < sam->gpio_count; i++) {
> + u32 flags = sam->export_flags[i];
> + struct gpio_desc *desc;
> +
> + /* request and initialize exported pins */
> + if (!(flags & GPIOF_EXPORT))
> + continue;
> +
> + desc = gpiochip_request_own_desc(&sam->gpio, i, "sam-export");
> + if (IS_ERR(desc)) {
> + ret = PTR_ERR(desc);
> + goto error;
> + }
> + if (flags & GPIOF_DIR_IN) {
> + ret = gpiod_direction_input(desc);
> + if (ret)
> + goto error;
> + } else {
> + ret = gpiod_direction_output(desc, flags &
> + (GPIOF_OUT_INIT_HIGH |
> + GPIOF_ACTIVE_LOW));
> + if (ret)
> + goto error;
> + }
> + ret = gpiod_export(desc, flags & GPIOF_EXPORT_CHANGEABLE);
> +
> + if (ret)
> + goto error;
> + }
> + return 0;
> +
> +error:
> + sam_gpio_unexport(sam);
> + return ret;
> +}
Just delete this. Use the new chardev ABI to access GPIOs from
userspace as explained earlier.
> +static int sam_gpio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sam_gpio *sam;
> + struct resource *res;
> + int ret;
> + struct sam_platform_data *pdata = dev_get_platdata(&pdev->dev);
> +
> + sam = devm_kzalloc(dev, sizeof(*sam), GFP_KERNEL);
> + if (sam == NULL)
> + return -ENOMEM;
if (!sam)
return -ENOMEM;
> + sam->dev = dev;
> + sam->pdata = pdata;
> + platform_set_drvdata(pdev, sam);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + sam->irq = platform_get_irq(pdev, 0);
> + sam->irq_high = platform_get_irq(pdev, 1);
> +
> + sam->base = devm_ioremap_nocache(dev, res->start, resource_size(res));
> + if (!sam->base)
> + return -ENOMEM;
> +
> + mutex_init(&sam->irq_lock);
> +
> + ret = sam_gpio_of_init(dev, sam);
> + if (ret)
> + return ret;
> +
> + sam_gpio_setup(sam);
> +
> + if (pdata && sam->irq >= 0 && of_find_property(dev->of_node,
> + "interrupt-controller", NULL)) {
> + ret = sam_gpio_irq_setup(dev, sam);
> + if (ret < 0)
> + return ret;
> + }
This is fair, but do it after adding the gpiochip and use GPIOLIB_IRQCHIP
accessors gpiochip_irqchip_add() and gpiochip_set_chained_irqchip().
Yours,
Linus Walleij
^ permalink raw reply
* RE: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for VF support
From: Vatsavayi, Raghu @ 2016-10-20 23:08 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, Chickles, Derek, Burla, Satananda,
Manlunas, Felix
In-Reply-To: <20161020.165720.1537348879596195765.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, October 20, 2016 1:57 PM
> To: Vatsavayi, Raghu
> Cc: netdev@vger.kernel.org; Chickles, Derek; Burla, Satananda; Manlunas,
> Felix
> Subject: Re: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for VF
> support
>
> From: "Vatsavayi, Raghu" <Raghu.Vatsavayi@cavium.com>
> Date: Thu, 20 Oct 2016 20:01:37 +0000
>
> >
> >
> >> -----Original Message-----
> >> From: David Miller [mailto:davem@davemloft.net]
> >> Sent: Thursday, October 20, 2016 11:13 AM
> >> To: Vatsavayi, Raghu
> >> Cc: netdev@vger.kernel.org; Vatsavayi, Raghu; Chickles, Derek; Burla,
> >> Satananda; Manlunas, Felix
> >> Subject: Re: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for
> >> VF support
> >>
> >> From: Raghu Vatsavayi <rvatsavayi@caviumnetworks.com>
> >> Date: Wed, 19 Oct 2016 22:40:38 -0700
> >>
> >> > +/* Default behaviour of Liquidio is to provide one queue per VF.
> >> > +But Liquidio
> >> > + * can also provide multiple queues to each VF. If user wants to
> >> > +change the
> >> > + * default behaviour HW should be provided configuration info at
> >> > +init time,
> >> > + * based on which it will create control queues for communicating
> >> > +with
> >> FW.
> >> > + */
> >> > +static u32 max_vfs[2] = { 0, 0 };
> >> > +module_param_array(max_vfs, int, NULL, 0444);
> >> > +MODULE_PARM_DESC(max_vfs, "Assign two comma-separated
> unsigned
> >> > +integers that specify max number of VFs for PF0 (left of the
> >> > +comma) and PF1 (right of the comma); for 23xx only. By default HW
> >> > +will configure as many VFs as queues after allocating PF queues.To
> >> > +increase queues for VF use this parameter. Use sysfs to create
> >> > +these VFs.");
> >> > +
> >> > +static unsigned int num_queues_per_pf[2] = { 0, 0 };
> >> > +module_param_array(num_queues_per_pf, uint, NULL, 0444);
> >> > +MODULE_PARM_DESC(num_queues_per_pf, "two comma-separated
> >> unsigned
> >> > +integers that specify number of queues per PF0 (left of the comma)
> >> > +and PF1 (right of the comma); for 23xx only");
> >> > +
> >> > static int ptp_enable = 1;
> >>
> >> We cannot continue to allow drivers to add custom module parameters
> >> to control this. It is the worst user experience possible.
> >>
> >> We need a tree-wide generic, consistent, manner in which to configure
> >> and control this kind of thing.
> >
> > Sure Dave, I will remove max_vfs module parameter and will use tree
> > wide generic sysfs interface to enable VFs.
>
> That's not what I meant.
>
> I mean there needs to be a generic mechanism that isn't a per-device knob
> (be it a module parameter or a sysctl, to me these are identical functionality
> and user experience wise).
>
> Something like ethtool or netlink.
Dave, I will remove max_vfs module parameter and will just use the generic mechanism that all drivers do like:
" echo 10 > /sys/devices/pci0000:00/0000:00:03.0/0000:03:00.1/sriov_numvfs"
Regarding other module parameters, in non-default case if user wants to have multiple queues then because of the way Liquidio HW works we need num_queues_per_pf and num_queues_per_vf module parameters at HW/module init time. This is because in multi-queues per VF scenario, HW has to carve these queues before FW can start communicating with PF/VF host drivers, so we must include these two parameters.
Please confirm that having these two module parameters is fine for non-default case. I will soon forward you the patches with these changes that you have recommended.
Thanks Much
Raghu.
^ permalink raw reply
* Re: [Patch net] net: saving irq context for peernet2id()
From: Cong Wang @ 2016-10-20 23:35 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, Linux Kernel Network Developers, Elad Raz,
Richard Guy Briggs
In-Reply-To: <CAHC9VhSronk914-0QZd7cPBEAOvSzN++Umc=_aueeCuOB9iG8Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2655 bytes --]
On Thu, Oct 20, 2016 at 12:07 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Oct 20, 2016 at 2:29 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Thu, Oct 20, 2016 at 7:58 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>> On 10/20/2016 02:52 AM, Cong Wang wrote:
>>>> A kernel warning inside __local_bh_enable_ip() was reported by people
>>>> running SELinux, this is caused due to some SELinux functions
>>>> (indirectly) call peernet2id() with IRQ disabled in process context,
>>>> when we re-enable BH with IRQ disabled kernel complains. Shut up this
>>>> warning by saving IRQ context in peernet2id(), BH is still implicitly
>>>> disabled.
>>>
>>> Not sure this suffices; kill_fasync() -> send_sigio() ->
>>> send_sigio_to_task() -> sigio_perm() -> security_file_send_sigiotask()
>>> -> selinux_file_send_sigiotask() -> ... -> audit_log() -> ... ->
>>> peernet2id()
>>
>> Oh, this is a new one. kill_fasync() is called in IRQ handler, so we actually
>> do multicast in IRQ context.... It makes no sense, netlink multicast could
>> be very expensive if we have many listeners.
>
> I'm sure there are a few others I don't know about, but I believe the
> only commonly used audit multicast listener is systemd.
But user-space is free to listen to this group, right? If so this is just open
for a potential DDOS attack.
>
>> I am Cc'ing Richard who added that multicast in audit_log_end(). It seems
>> not easy to just move the multicast to a workqueue, since the skb is copied
>> from audit_buffer which is freed immediately after that, probably need another
>> queue like audit_skb_queue.
>
> This approach would double the queue size which is something I want to
> avoid. I would suggest sticking with a single queue and dealing with
> the netlink message link fixup and multicast send in the existing
> netlink unicast thread; basically we would just be moving the
> multicast code from audit_log_end() into kauditd_thread(). This is
> the same approach I mentioned earlier off-list.
This is what I did in the follow up patch. I attach the updated version
in this email for you to review, I still can't make selinux-testsuites working
on my Fedora even though I have SELinux=enforcing, anyhow I don't
see any kernel warning in my dmesg at least.
>
> However, that isn't something I want to mess with as a regression fix,
> mostly because I really want to see this regression gone by -rc2 as it
> is making SELinux testing a real pain. If the patch posted at the top
> of this thread isn't a suitable fix, we really should revert the
> original patch.
Since you want to test SELinux anyway, please test the attached one.
Thanks.
[-- Attachment #2: audit-v2.diff --]
[-- Type: text/plain, Size: 2202 bytes --]
diff --git a/kernel/audit.c b/kernel/audit.c
index f1ca116..cdc5a91 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -139,6 +139,7 @@ static int audit_freelist_count;
static LIST_HEAD(audit_freelist);
static struct sk_buff_head audit_skb_queue;
+static struct sk_buff_head audit_skb_multicast_queue;
/* queue of skbs to send to auditd when/if it comes back */
static struct sk_buff_head audit_skb_hold_queue;
static struct task_struct *kauditd_task;
@@ -468,7 +469,8 @@ static void kauditd_send_multicast_skb(struct sk_buff *skb, gfp_t gfp_mask)
if (!copy)
return;
- nlmsg_multicast(sock, copy, 0, AUDIT_NLGRP_READLOG, gfp_mask);
+ skb_queue_tail(&audit_skb_multicast_queue, copy);
+ wake_up_interruptible(&kauditd_wait);
}
/*
@@ -509,6 +511,26 @@ static void flush_hold_queue(void)
consume_skb(skb);
}
+static void flush_multicast_queue(void)
+{
+ struct audit_net *aunet = net_generic(&init_net, audit_net_id);
+ struct sock *sock = aunet->nlsk;
+ struct sk_buff *skb = skb_dequeue(&audit_skb_multicast_queue);
+
+ if (!netlink_has_listeners(sock, AUDIT_NLGRP_READLOG)) {
+ while (skb) {
+ consume_skb(skb);
+ skb = skb_dequeue(&audit_skb_multicast_queue);
+ }
+ return;
+ }
+
+ while (skb) {
+ nlmsg_multicast(sock, skb, 0, AUDIT_NLGRP_READLOG, GFP_KERNEL);
+ skb = skb_dequeue(&audit_skb_multicast_queue);
+ }
+}
+
static int kauditd_thread(void *dummy)
{
set_freezable();
@@ -517,6 +539,8 @@ static int kauditd_thread(void *dummy)
flush_hold_queue();
+ flush_multicast_queue();
+
skb = skb_dequeue(&audit_skb_queue);
if (skb) {
@@ -530,7 +554,8 @@ static int kauditd_thread(void *dummy)
continue;
}
- wait_event_freezable(kauditd_wait, skb_queue_len(&audit_skb_queue));
+ wait_event_freezable(kauditd_wait, skb_queue_len(&audit_skb_queue)
+ || skb_queue_len(&audit_skb_multicast_queue));
}
return 0;
}
@@ -1197,6 +1222,7 @@ static int __init audit_init(void)
register_pernet_subsys(&audit_net_ops);
skb_queue_head_init(&audit_skb_queue);
+ skb_queue_head_init(&audit_skb_multicast_queue);
skb_queue_head_init(&audit_skb_hold_queue);
audit_initialized = AUDIT_INITIALIZED;
audit_enabled = audit_default;
^ permalink raw reply related
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2016-10-20 23:40 UTC (permalink / raw)
To: David Miller, Networking
Cc: linux-next, linux-kernel, Jarod Wilson, Thomas Falcon
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
drivers/net/ethernet/ibm/ibmvnic.c
between commit:
87737f8810db ("ibmvnic: Update MTU after device initialization")
from the net tree and commit:
d894be57ca92 ("ethernet: use net core MTU range checking in more drivers")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/net/ethernet/ibm/ibmvnic.c
index 213162df1a9b,657206be7ba9..000000000000
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@@ -3654,7 -3644,8 +3644,9 @@@ static void handle_crq_init_rsp(struct
goto task_failed;
netdev->real_num_tx_queues = adapter->req_tx_queues;
+ netdev->mtu = adapter->req_mtu;
+ netdev->min_mtu = adapter->min_mtu;
+ netdev->max_mtu = adapter->max_mtu;
if (adapter->failover) {
adapter->failover = false;
^ permalink raw reply
* Re: [Patch net v2] ipv6: fix a potential deadlock in do_ipv6_setsockopt()
From: Cong Wang @ 2016-10-20 23:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Baozeng Ding,
Marcelo Ricardo Leitner
In-Reply-To: <1477001458.7065.37.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Oct 20, 2016 at 3:10 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> I used 'I wonder if' to say that we might have some better way to code
> this test nowadays, but this can be done in a separate patch of course.
OK, let's keep one bugfix in one patch. ;)
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: Florian Fainelli @ 2016-10-21 0:30 UTC (permalink / raw)
To: Stephen Rothwell, David Miller, Networking; +Cc: linux-next, linux-kernel
In-Reply-To: <bd2fa038-2d0a-8cd1-d80d-860a7497bdf5@gmail.com>
On 10/20/2016 03:42 PM, Florian Fainelli wrote:
> On 10/20/2016 03:27 PM, Stephen Rothwell wrote:
>> Hi all,
>>
>> After merging the net tree, today's linux-next build (arm
>> multi_v7_defconfig) failed like this:
>>
>> ERROR: "kexec_in_progress" [drivers/net/dsa/bcm_sf2.ko] undefined!
>>
>> Caused by commit
>>
>> 2399d6143f85 ("net: dsa: bcm_sf2: Prevent GPHY shutdown for kexec'd kernels")
>>
>> I used the version of the net tree from next-20161020 for today.
>
> OK, seems like we need an ifdef CONFIG_KEXEC_CORE, let me fix that.
> Thanks Stephen!
Stephen, can you send me the .config file? I could not reproduce it with
next-20161020 and doing a make multi_v7_defconfig:
fainelli@fainelli-desktop:[~/../linux]$ ls -l drivers/net/dsa/*.ko
-rw-rw-r-- 1 fainelli fainelli 17K Oct 20 17:30 drivers/net/dsa/bcm_sf2.ko
Thanks
--
Florian
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: Stephen Rothwell @ 2016-10-21 0:43 UTC (permalink / raw)
To: Florian Fainelli; +Cc: David Miller, Networking, linux-next, linux-kernel
In-Reply-To: <e2380249-29b4-4769-514a-5fc5fbd8f95c@gmail.com>
Hi Florian,
On Thu, 20 Oct 2016 17:30:33 -0700 Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 10/20/2016 03:42 PM, Florian Fainelli wrote:
> > On 10/20/2016 03:27 PM, Stephen Rothwell wrote:
> >>
> >> After merging the net tree, today's linux-next build (arm
> >> multi_v7_defconfig) failed like this:
> >>
> >> ERROR: "kexec_in_progress" [drivers/net/dsa/bcm_sf2.ko] undefined!
> >>
> >> Caused by commit
> >>
> >> 2399d6143f85 ("net: dsa: bcm_sf2: Prevent GPHY shutdown for kexec'd kernels")
> >>
> >> I used the version of the net tree from next-20161020 for today.
> >
> > OK, seems like we need an ifdef CONFIG_KEXEC_CORE, let me fix that.
> > Thanks Stephen!
>
> Stephen, can you send me the .config file? I could not reproduce it with
> next-20161020 and doing a make multi_v7_defconfig:
This only started with the net tree as of today (HEAD a681574c99be).
The problem is simply that kexec_in_progress is not exported to modules.
Sorry, getting the actual .config file is a pain at this point since my
builds have moved on.
It probably had (the current .config has):
CONFIG_NET_DSA_BCM_SF2=m
CONFIG_KEXEC_CORE=y
If CONFIG_KEXEC_CORE is not set, then kexec_in_progress is defined to false.
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: linux-next: build failure after merge of the net tree
From: Florian Fainelli @ 2016-10-21 0:46 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: David Miller, Networking, linux-next, linux-kernel
In-Reply-To: <20161021114334.1eb189aa@canb.auug.org.au>
On 10/20/2016 05:43 PM, Stephen Rothwell wrote:
> Hi Florian,
>
> On Thu, 20 Oct 2016 17:30:33 -0700 Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> On 10/20/2016 03:42 PM, Florian Fainelli wrote:
>>> On 10/20/2016 03:27 PM, Stephen Rothwell wrote:
>>>>
>>>> After merging the net tree, today's linux-next build (arm
>>>> multi_v7_defconfig) failed like this:
>>>>
>>>> ERROR: "kexec_in_progress" [drivers/net/dsa/bcm_sf2.ko] undefined!
>>>>
>>>> Caused by commit
>>>>
>>>> 2399d6143f85 ("net: dsa: bcm_sf2: Prevent GPHY shutdown for kexec'd kernels")
>>>>
>>>> I used the version of the net tree from next-20161020 for today.
>>>
>>> OK, seems like we need an ifdef CONFIG_KEXEC_CORE, let me fix that.
>>> Thanks Stephen!
>>
>> Stephen, can you send me the .config file? I could not reproduce it with
>> next-20161020 and doing a make multi_v7_defconfig:
>
> This only started with the net tree as of today (HEAD a681574c99be).
> The problem is simply that kexec_in_progress is not exported to modules.
Realized that right after sending you the email.
>
> Sorry, getting the actual .config file is a pain at this point since my
> builds have moved on.
>
> It probably had (the current .config has):
>
> CONFIG_NET_DSA_BCM_SF2=m
> CONFIG_KEXEC_CORE=y
>
> If CONFIG_KEXEC_CORE is not set, then kexec_in_progress is defined to false.
Yes indeed, I have a fix ready, thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH net] udp: must lock the socket in udp_disconnect()
From: Cong Wang @ 2016-10-21 1:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Baozeng Ding, David Miller, network dev
In-Reply-To: <1476981580.7065.15.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Oct 20, 2016 at 9:39 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Baozeng Ding reported KASAN traces showing uses after free in
> udp_lib_get_port() and other related UDP functions.
>
> A CONFIG_DEBUG_PAGEALLOC=y kernel would eventually crash.
>
> I could write a reproducer with two threads doing :
>
> static int sock_fd;
> static void *thr1(void *arg)
> {
> for (;;) {
> connect(sock_fd, (const struct sockaddr *)arg,
> sizeof(struct sockaddr_in));
> }
> }
>
> static void *thr2(void *arg)
> {
> struct sockaddr_in unspec;
>
> for (;;) {
> memset(&unspec, 0, sizeof(unspec));
> connect(sock_fd, (const struct sockaddr *)&unspec,
> sizeof(unspec));
> }
> }
>
> Problem is that udp_disconnect() could run without holding socket lock,
> and this was causing list corruptions.
If this is the cause of the hashlist corruption (I am still unsure about this),
then why only UDP? Don't all of those using ip4_datagram_connect()
as ->connect() and using udp_disconnect() as ->disconnect() need this fix?
For example, after your patch,
.connect = ip4_datagram_connect,
- .disconnect = udp_disconnect,
+ .disconnect = __udp_disconnect,
Ping socket still doesn't have sock lock for ->disconnect() but has it for
->connect()? I must miss something...
^ permalink raw reply
* [PATCH] kexec: Export kexec_in_progress to modules
From: Florian Fainelli @ 2016-10-21 1:15 UTC (permalink / raw)
To: netdev; +Cc: ebiederm, kexec, linux-kernel, davem, sfr, Florian Fainelli
The bcm_sf2 driver uses kexec_in_progress to know whether it can power
down an integrated PHY during shutdown, and can be built as a module.
Other modules may be using this in the future, so export it.
Fixes: 2399d6143f85 ("net: dsa: bcm_sf2: Prevent GPHY shutdown for kexec'd kernels")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Eric, David, Stephen,
The offending commit is in David's net.git tree, so it would probably make
sense to route the fix through the same tree.
Thanks!
kernel/kexec_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 561675589511..786ab85a5452 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -59,6 +59,7 @@ size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data);
/* Flag to indicate we are going to kexec a new kernel */
bool kexec_in_progress = false;
+EXPORT_SYMBOL_GPL(kexec_in_progress);
/* Location of the reserved area for the crash kernel */
--
2.7.4
^ 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