* Re: [PATCH V2 0/3] net-next: Broadcom PHY driver cleanup
From: David Miller @ 2017-01-26 4:14 UTC (permalink / raw)
To: zajec5; +Cc: f.fainelli, xow, joel, jon.mason, jaedon.shin, netdev, rafal
In-Reply-To: <20170125200027.14387-1-zajec5@gmail.com>
From: Rafał Miłecki <zajec5@gmail.com>
Date: Wed, 25 Jan 2017 21:00:24 +0100
> I will probably need to use broadcom.ko for PHY connected to interface
> of bgmac supported device so I started looking at it willing to
> understand it better.
>
> I found AUXCTL part of the driver / lib a bit confusing and hard to read
> so I'm trying to clean it up a bit. I hope this patchset makes following
> AUXCTL operations much easier making it clear which defines are for
> registers and which for values.
>
> There is no functional change in this pachset.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] bpf: use prefix_len in test_tag when reading fdinfo
From: David Miller @ 2017-01-26 4:15 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev
In-Reply-To: <a4146cba5583102419763e59d8feff5e91802525.1485387682.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 26 Jan 2017 00:42:49 +0100
> We currently used len instead of prefix_len for the strncmp() in
> fdinfo on the prog_tag. It still worked as we matched on the correct
> output line also with first 8 instead of 10 chars, but lets fix it
> properly to use the intended length.
>
> Fixes: 62b64660262a ("bpf: add prog tag test case to bpf selftests")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Applied.
^ permalink raw reply
* Re: [PATCH] [net-next] bridge: move maybe_deliver_addr() inside #ifdef
From: David Miller @ 2017-01-26 4:16 UTC (permalink / raw)
To: arnd; +Cc: stephen, linus.luessing, nikolay, nbd, bridge, netdev,
linux-kernel
In-Reply-To: <20170125222950.1352566-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 25 Jan 2017 23:29:33 +0100
> The only caller of this new function is inside of an #ifdef checking
> for CONFIG_BRIDGE_IGMP_SNOOPING, so we should move the implementation
> there too, in order to avoid this harmless warning:
>
> net/bridge/br_forward.c:177:13: error: 'maybe_deliver_addr' defined but not used [-Werror=unused-function]
>
> Fixes: 6db6f0eae605 ("bridge: multicast to unicast")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thank you.
^ permalink raw reply
* [net-next] openvswitch: Simplify do_execute_actions().
From: Andy Zhou @ 2017-01-26 5:24 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou
do_execute_actions() implements a worthwhile optimization: in case
an output action is the last action in an action list, skb_clone()
can be avoided by outputing the current skb. However, the
implementation is more complicated than necessary. This patch
simplify this logic.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
net/openvswitch/actions.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 514f7bc..3866608 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -830,6 +830,9 @@ static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port,
{
struct vport *vport = ovs_vport_rcu(dp, out_port);
+ if (unlikely(!skb))
+ return;
+
if (likely(vport)) {
u16 mru = OVS_CB(skb)->mru;
u32 cutlen = OVS_CB(skb)->cutlen;
@@ -1141,12 +1144,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
struct sw_flow_key *key,
const struct nlattr *attr, int len)
{
- /* Every output action needs a separate clone of 'skb', but the common
- * case is just a single output action, so that doing a clone and
- * then freeing the original skbuff is wasteful. So the following code
- * is slightly obscure just to avoid that.
- */
- int prev_port = -1;
const struct nlattr *a;
int rem;
@@ -1154,20 +1151,25 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
a = nla_next(a, &rem)) {
int err = 0;
- if (unlikely(prev_port != -1)) {
- struct sk_buff *out_skb = skb_clone(skb, GFP_ATOMIC);
+ switch (nla_type(a)) {
+ case OVS_ACTION_ATTR_OUTPUT: {
+ int port = nla_get_u32(a);
- if (out_skb)
- do_output(dp, out_skb, prev_port, key);
+ /* Every output action needs a separate clone
+ * of 'skb', In case the output action is the
+ * last action, cloning can be avoided.
+ */
+ if (nla_is_last(a, rem)) {
+ do_output(dp, skb, port, key);
+ /* 'skb' has been used for output.
+ */
+ return 0;
+ }
+ do_output(dp, skb_clone(skb, GFP_ATOMIC), port, key);
OVS_CB(skb)->cutlen = 0;
- prev_port = -1;
- }
-
- switch (nla_type(a)) {
- case OVS_ACTION_ATTR_OUTPUT:
- prev_port = nla_get_u32(a);
break;
+ }
case OVS_ACTION_ATTR_TRUNC: {
struct ovs_action_trunc *trunc = nla_data(a);
@@ -1257,11 +1259,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
}
}
- if (prev_port != -1)
- do_output(dp, skb, prev_port, key);
- else
- consume_skb(skb);
-
+ consume_skb(skb);
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] bpf: expose netns inode to bpf programs
From: Eric W. Biederman @ 2017-01-26 5:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S . Miller, Daniel Borkmann, David Ahern, Tejun Heo,
Andy Lutomirski, Thomas Graf, netdev
In-Reply-To: <1485401274-2836524-1-git-send-email-ast@fb.com>
Alexei Starovoitov <ast@fb.com> writes:
> in cases where bpf programs are looking at sockets and packets
> that belong to different netns, it could be useful to read netns inode,
> so that programs can make intelligent decisions.
> For example to disallow raw sockets in all non-init netns the program can do:
> if (sk->type == SOCK_RAW && sk->netns_inum != 0xf0000075)
> return 0;
> where 0xf0000075 inode comes from /proc/pid/ns/net
>
> Similarly TC cls_bpf/act_bpf and socket filters can do
> if (skb->netns_inum == expected_inode)
Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Particularly you need to compare more than the inode number.
Further I have never guaranteed there will be exactly one inode
per network namespace, just that if the device number and the inode
number pair match they are the same.
Beyond that the entire concept is complete rubbish.
The only sane thing is to interpret whatever your bpf program in the
context of the program which installs it.
If you can't do that you have a very broken piece of userspace
interface. Which appears to be the case here.
Eric
^ permalink raw reply
* Re: [PATCH net] bpf: expose netns inode to bpf programs
From: Ying Xue @ 2017-01-26 6:00 UTC (permalink / raw)
To: ying.xue
Cc: Daniel Borkmann, David Ahern, Tejun Heo, Andy Lutomirski,
Thomas Graf, netdev
In-Reply-To: <87efzq8jbi.fsf@xmission.com>
tetst teste tetet tetest tetetttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt
On 01/26/2017 01:46 PM, Eric W. Biederman wrote:
> Alexei Starovoitov <ast@fb.com> writes:
>
>> in cases where bpf programs are looking at sockets and packets
>> that belong to different netns, it could be useful to read netns inode,
>> so that programs can make intelligent decisions.
>> For example to disallow raw sockets in all non-init netns the program can do:
>> if (sk->type == SOCK_RAW && sk->netns_inum != 0xf0000075)
>> return 0;
>> where 0xf0000075 inode comes from /proc/pid/ns/net
>>
>> Similarly TC cls_bpf/act_bpf and socket filters can do
>> if (skb->netns_inum == expected_inode)
>
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> Particularly you need to compare more than the inode number.
> Further I have never guaranteed there will be exactly one inode
> per network namespace, just that if the device number and the inode
> number pair match they are the same.
>
> Beyond that the entire concept is complete rubbish.
>
> The only sane thing is to interpret whatever your bpf program in the
> context of the program which installs it.
>
> If you can't do that you have a very broken piece of userspace
> interface. Which appears to be the case here.
>
> Eric
>
^ permalink raw reply
* Re: [PATCH net] bpf: expose netns inode to bpf programs
From: Alexei Starovoitov @ 2017-01-26 6:23 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David S . Miller, Daniel Borkmann, David Ahern, Tejun Heo,
Andy Lutomirski, Thomas Graf, netdev
In-Reply-To: <87efzq8jbi.fsf@xmission.com>
On 1/25/17 9:46 PM, Eric W. Biederman wrote:
> Alexei Starovoitov <ast@fb.com> writes:
>
>> in cases where bpf programs are looking at sockets and packets
>> that belong to different netns, it could be useful to read netns inode,
>> so that programs can make intelligent decisions.
>> For example to disallow raw sockets in all non-init netns the program can do:
>> if (sk->type == SOCK_RAW && sk->netns_inum != 0xf0000075)
>> return 0;
>> where 0xf0000075 inode comes from /proc/pid/ns/net
>>
>> Similarly TC cls_bpf/act_bpf and socket filters can do
>> if (skb->netns_inum == expected_inode)
>
> Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>
I very much value your opinion, but your ack or nack doesn't apply here.
Exposing existing inode has nothing to do with what you maintain.
It's a bpf feature that is exposing already visible to user space id.
Period.
If I was proposing to expose some internal namespace id, then yes,
we'd need to have a discussion. ns.inum is already visible.
> Particularly you need to compare more than the inode number.
> Further I have never guaranteed there will be exactly one inode
> per network namespace, just that if the device number and the inode
> number pair match they are the same.
people already rely on inodes for _all_ namespaces.
The current implementation of
net_ns_net_init->..>ida_get_new is stuck the way it is.
We can change ids, generation algorithm, but uniqueness is
already assumed by user space.
> Beyond that the entire concept is complete rubbish.
care to explain what you think the 'entire concept' is?
> The only sane thing is to interpret whatever your bpf program in the
> context of the program which installs it.
that's impossible. The programs are operating in the context that
is disjoined from the app that installs it.
> If you can't do that you have a very broken piece of userspace
> interface. Which appears to be the case here.
Call it rubbish, but this is how it is.
cls_bpf is operating on packets. xdp_bpf is operating on raw dma buffers
and there we might need eventually lookup sockets and net namespaces.
Think of bpf programs as safe kernel modules. They don't have
confined boundaries and program authors, if not careful, can shoot
themselves in the foot. We're not trying to prevent that because
it's impossible to check that the program is sane. Just like
it's impossible to check that kernel module is sane.
But in case of bpf we check that bpf program is _safe_ from the kernel
point of view. If it's doing some garbage, it's program's business.
Does it make more sense now?
^ permalink raw reply
* [PATCH RFC ipsec-next 2/2] xfrm: Add a device independent napi instance
From: Steffen Klassert @ 2017-01-26 6:50 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, David Miller, Eric Dumazet, Sowmini Varadhan,
Ilan Tayari
In-Reply-To: <1485413456-31197-1-git-send-email-steffen.klassert@secunet.com>
This patch adds a napi instance for IPsec GRO.
With this, we handle IPsec GRO with no impact
on the generic networking code.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_input.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 86 insertions(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 6e3f025..c5fc12d 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -21,6 +21,90 @@
static DEFINE_SPINLOCK(xfrm_input_afinfo_lock);
static struct xfrm_input_afinfo __rcu *xfrm_input_afinfo[NPROTO];
+struct xfrm_napi {
+ struct sk_buff_head napi_skbs;
+ struct napi_struct napi;
+};
+
+static struct xfrm_napi __percpu *napis;
+
+static void xfrm_gro_receive(struct sk_buff *skb)
+{
+ struct xfrm_napi *xnapi;
+ struct net_device *dev = skb->dev;
+
+ if (!napis || skb_cloned(skb) || !(dev->features & NETIF_F_GRO)) {
+ netif_rx(skb);
+ return;
+ }
+
+ xnapi = this_cpu_ptr(napis);
+
+ if (skb_queue_len(&xnapi->napi_skbs) > netdev_max_backlog) {
+ atomic_long_inc(&dev->rx_dropped);
+ kfree_skb(skb);
+ return;
+ }
+
+ __skb_queue_tail(&xnapi->napi_skbs, skb);
+ if (skb_queue_len(&xnapi->napi_skbs) == 1)
+ napi_schedule(&xnapi->napi);
+}
+
+static int xfrm_gro_poll(struct napi_struct *napi, int budget)
+{
+ unsigned long flags;
+ struct sk_buff *skb;
+ int work_done = 0;
+ struct xfrm_napi *xnapi = container_of(napi, struct xfrm_napi, napi);
+
+ while (work_done < budget) {
+ skb = __skb_dequeue(&xnapi->napi_skbs);
+ if (!skb)
+ break;
+ napi_gro_receive(napi, skb);
+ work_done++;
+ }
+
+ if (work_done < budget) {
+ napi_gro_flush(napi, false);
+
+ if (likely(list_empty(&napi->poll_list))) {
+ WARN_ON_ONCE(!test_and_clear_bit(NAPI_STATE_SCHED, &napi->state));
+ } else {
+ /* If n->poll_list is not empty, we need to mask irqs */
+ local_irq_save(flags);
+ __napi_complete(napi);
+ local_irq_restore(flags);
+ }
+ }
+
+ return work_done;
+}
+
+static void xfrm_gro_init(void)
+{
+ int i;
+
+ /* Napi remains disabled if we can't alloc memory. */
+ napis = alloc_percpu(struct xfrm_napi);
+
+ for_each_possible_cpu(i) {
+ struct xfrm_napi *xnapi = per_cpu_ptr(napis, i);
+
+ __skb_queue_head_init(&xnapi->napi_skbs);
+
+ set_bit(NAPI_STATE_NO_BUSY_POLL, &xnapi->napi.state);
+
+ INIT_LIST_HEAD(&xnapi->napi.poll_list);
+ xnapi->napi.poll = xfrm_gro_poll;
+ xnapi->napi.weight = 64;
+ xnapi->napi.gro_list = NULL;
+ xnapi->napi.gro_count = 0;
+ xnapi->napi.skb = NULL;
+ }
+}
+
int xfrm_input_register_afinfo(struct xfrm_input_afinfo *afinfo)
{
int err = 0;
@@ -371,7 +455,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
if (decaps) {
skb_dst_drop(skb);
- netif_rx(skb);
+ xfrm_gro_receive(skb);
return 0;
} else {
return x->inner_mode->afinfo->transport_finish(skb, async);
@@ -394,6 +478,7 @@ int xfrm_input_resume(struct sk_buff *skb, int nexthdr)
void __init xfrm_input_init(void)
{
+ xfrm_gro_init();
secpath_cachep = kmem_cache_create("secpath_cache",
sizeof(struct sec_path),
0, SLAB_HWCACHE_ALIGN|SLAB_PANIC,
--
1.9.1
^ permalink raw reply related
* [PATCH RFC ipsec-next 1/2] net: Drop secpath on free after gro merge.
From: Steffen Klassert @ 2017-01-26 6:50 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, David Miller, Eric Dumazet, Sowmini Varadhan,
Ilan Tayari
In-Reply-To: <1485413456-31197-1-git-send-email-steffen.klassert@secunet.com>
With a followup patch, a gro merged skb can have a secpath.
So drop it before freeing or reusing the skb.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/core/dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 56818f7..c9e541e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4623,6 +4623,7 @@ static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
case GRO_MERGED_FREE:
if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD) {
skb_dst_drop(skb);
+ secpath_put(skb->sp);
kmem_cache_free(skbuff_head_cache, skb);
} else {
__kfree_skb(skb);
@@ -4663,6 +4664,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
skb->encapsulation = 0;
skb_shinfo(skb)->gso_type = 0;
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
+ secpath_put(skb->sp);
napi->skb = skb;
}
--
1.9.1
^ permalink raw reply related
* [PATCH RFC ipsec-next] IPsec GRO
From: Steffen Klassert @ 2017-01-26 6:50 UTC (permalink / raw)
To: netdev
Cc: Steffen Klassert, David Miller, Eric Dumazet, Sowmini Varadhan,
Ilan Tayari
This introduces a device independent napi instance that
handles GRO for IPsec. I was not able to use gro cells
directly because I don't have a netdevice where I can
hang off the napi instance. We now may have a secpath
at a GRO merged skb, so we need to drop it. This is the
only cange to the generic networking code.
The packet still travels two times through the stack,
but might be aggregated in the second round. We can
avoid the second round with implementing GRO callbacks
for the IPsec protocols. This will be a separate patchset
as this needs some more generic networking changes because
of the asynchronous nature of IPsec.
^ permalink raw reply
* Re: [PATCH v2 iproute2] f_flower: don't set TCA_FLOWER_KEY_ETH_TYPE for "protocol all"
From: Roi Dayan @ 2017-01-26 7:13 UTC (permalink / raw)
To: Benjamin LaHaise, Stephen Hemminger; +Cc: Jamal Hadi Salim, netdev, bcrl
In-Reply-To: <20170120190738.GA17163@nvt-d.home.kvack.org>
On 20/01/2017 21:07, Benjamin LaHaise wrote:
> v2 - update to address changes in 00697ca19ae3e1118f2af82c3b41ac4335fe918b.
>
> When using the tc flower filter, rules marked with "protocol all" do not
> actually match all packets. This is due to a bug in f_flower.c that passes
> in ETH_P_ALL in the TCA_FLOWER_KEY_ETH_TYPE attribute when adding a rule.
> Fix this by omitting TCA_FLOWER_KEY_ETH_TYPE if the protocol is set to
> ETH_P_ALL.
>
> Fixes: 488b41d020fb ("tc: flower no need to specify the ethertype")
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Benjamin LaHaise <benjamin.lahaise@netronome.com>
> Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>
> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index 314c2dd..145a856 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c
> @@ -529,9 +529,11 @@ parse_done:
> if (ret)
> return ret;
>
> - ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
> - if (ret)
> - return ret;
> + if (eth_type != htons(ETH_P_ALL)) {
> + ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
> + if (ret)
> + return ret;
> + }
>
> tail->rta_len = (((void *)n)+n->nlmsg_len) - (void *)tail;
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* Re: [PATCH 0/6 v3] kvmalloc
From: Michal Hocko @ 2017-01-26 7:43 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org
In-Reply-To: <588907AA.1020704@iogearbox.net>
On Wed 25-01-17 21:16:42, Daniel Borkmann wrote:
> On 01/25/2017 07:14 PM, Alexei Starovoitov wrote:
> > On Wed, Jan 25, 2017 at 5:21 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > On Wed 25-01-17 14:10:06, Michal Hocko wrote:
> > > > On Tue 24-01-17 11:17:21, Alexei Starovoitov wrote:
> [...]
> > > > > > Are there any more comments? I would really appreciate to hear from
> > > > > > networking folks before I resubmit the series.
> > > > >
> > > > > while this patchset was baking the bpf side switched to use bpf_map_area_alloc()
> > > > > which fixes the issue with missing __GFP_NORETRY that we had to fix quickly.
> > > > > See commit d407bd25a204 ("bpf: don't trigger OOM killer under pressure with map alloc")
> > > > > it covers all kmalloc/vmalloc pairs instead of just one place as in this set.
> > > > > So please rebase and switch bpf_map_area_alloc() to use kvmalloc().
> > > >
> > > > OK, will do. Thanks for the heads up.
> > >
> > > Just for the record, I will fold the following into the patch 1
> > > ---
> > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > > index 19b6129eab23..8697f43cf93c 100644
> > > --- a/kernel/bpf/syscall.c
> > > +++ b/kernel/bpf/syscall.c
> > > @@ -53,21 +53,7 @@ void bpf_register_map_type(struct bpf_map_type_list *tl)
> > >
> > > void *bpf_map_area_alloc(size_t size)
> > > {
> > > - /* We definitely need __GFP_NORETRY, so OOM killer doesn't
> > > - * trigger under memory pressure as we really just want to
> > > - * fail instead.
> > > - */
> > > - const gfp_t flags = __GFP_NOWARN | __GFP_NORETRY | __GFP_ZERO;
> > > - void *area;
> > > -
> > > - if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
> > > - area = kmalloc(size, GFP_USER | flags);
> > > - if (area != NULL)
> > > - return area;
> > > - }
> > > -
> > > - return __vmalloc(size, GFP_KERNEL | __GFP_HIGHMEM | flags,
> > > - PAGE_KERNEL);
> > > + return kvzalloc(size, GFP_USER);
> > > }
> > >
> > > void bpf_map_area_free(void *area)
> >
> > Looks fine by me.
> > Daniel, thoughts?
>
> I assume that kvzalloc() is still the same from [1], right? If so, then
> it would unfortunately (partially) reintroduce the issue that was fixed.
> If you look above at flags, they're also passed to __vmalloc() to not
> trigger OOM in these situations I've experienced.
Pushing __GFP_NORETRY to __vmalloc doesn't have the effect you might
think it would. It can still trigger the OOM killer becauset the flags
are no propagated all the way down to all allocations requests (e.g.
page tables). This is the same reason why GFP_NOFS is not supported in
vmalloc.
> This is effectively the
> same requirement as in other networking areas f.e. that 5bad87348c70
> ("netfilter: x_tables: avoid warn and OOM killer on vmalloc call") has.
> In your comment in kvzalloc() you eventually say that some of the above
> modifiers are not supported. So there would be two options, i) just leave
> out the kvzalloc() chunk for BPF area to avoid the merge conflict and tackle
> it later (along with similar code from 5bad87348c70), or ii) implement
> support for these modifiers as well to your original set. I guess it's not
> too urgent, so we could also proceed with i) if that is easier for you to
> proceed (I don't mind either way).
Could you clarify why the oom killer in vmalloc matters actually?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH] net: ethernet: mvneta: add support for 2.5G DRSGMII mode
From: Jan Lübbe @ 2017-01-26 8:17 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20170125.123806.790914338952471404.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Mi, 2017-01-25 at 12:38 -0500, David Miller wrote:
> I still haven't seen a sufficient explanation as to why this change
> works without any explicit MAC programming changes to this driver.
>
> That really needs to be explained before I will apply this patch.
I'll need to recheck this with the documentation and will also check
what/if changes need to be done for ethtool and adjust_link as Andrew
Lunn mentioned. So in any case there will be a v2 as soon as I figure
out those details.
Thanks,
Jan
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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
* Re: [PATCH 1/2] mac80211: use helper function to access ieee802_1d_to_ac[]
From: Johannes Berg @ 2017-01-26 8:51 UTC (permalink / raw)
To: Amadeusz Sławiński, linux-wireless
Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <1485272531-11587-1-git-send-email-amadeusz.slawinski@tieto.com>
On Tue, 2017-01-24 at 16:42 +0100, Amadeusz Sławiński wrote:
> cleanup patch to make use of ieee80211_ac_from_tid() to retrieve ac
> from
> ieee802_1d_to_ac[]
Applied.
johannes
^ permalink raw reply
* Re: [PATCH RFC ipsec-next 1/2] net: Drop secpath on free after gro merge.
From: Steffen Klassert @ 2017-01-26 8:52 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Sowmini Varadhan, Ilan Tayari
In-Reply-To: <1485413456-31197-2-git-send-email-steffen.klassert@secunet.com>
On Thu, Jan 26, 2017 at 07:50:55AM +0100, Steffen Klassert wrote:
> With a followup patch, a gro merged skb can have a secpath.
> So drop it before freeing or reusing the skb.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> net/core/dev.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 56818f7..c9e541e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4623,6 +4623,7 @@ static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
> case GRO_MERGED_FREE:
> if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD) {
> skb_dst_drop(skb);
> + secpath_put(skb->sp);
> kmem_cache_free(skbuff_head_cache, skb);
> } else {
> __kfree_skb(skb);
> @@ -4663,6 +4664,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
> skb->encapsulation = 0;
> skb_shinfo(skb)->gso_type = 0;
> skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
> + secpath_put(skb->sp);
I have to use secpath_reset(skb) here of course.
skb->sp is not even defined if CONFIG_XFRM is not set.
Thanks to "kbuild test robot <lkp@intel.com>" for
pointing to that.
^ permalink raw reply
* [patch net-next] smc: some potential use after free bugs
From: Dan Carpenter @ 2017-01-26 9:05 UTC (permalink / raw)
To: Ursula Braun; +Cc: David S. Miller, linux-s390, netdev, kernel-janitors
Say we got really unlucky and these failed on the last iteration, then
it could lead to a use after free bug.
Fixes: cd6851f30386 ("smc: remote memory buffers (RMBs)")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 8b1d34378829..941279e1504e 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -535,6 +535,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
/* if send buffer allocation has failed,
* try a smaller one
*/
+ sndbuf_desc = NULL;
continue;
}
rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
@@ -543,6 +544,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
if (rc) {
kfree(sndbuf_desc->cpu_addr);
kfree(sndbuf_desc);
+ sndbuf_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
sndbuf_desc->used = 1;
@@ -599,6 +601,7 @@ int smc_rmb_create(struct smc_sock *smc)
/* if RMB allocation has failed,
* try a smaller one
*/
+ rmb_desc = NULL;
continue;
}
rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
@@ -607,6 +610,7 @@ int smc_rmb_create(struct smc_sock *smc)
if (rc) {
kfree(rmb_desc->cpu_addr);
kfree(rmb_desc);
+ rmb_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd,
@@ -619,6 +623,7 @@ int smc_rmb_create(struct smc_sock *smc)
DMA_FROM_DEVICE);
kfree(rmb_desc->cpu_addr);
kfree(rmb_desc);
+ rmb_desc = NULL;
continue;
}
rmb_desc->used = 1;
^ permalink raw reply related
* Re: [PATCH 0/6 v3] kvmalloc
From: Daniel Borkmann @ 2017-01-26 9:36 UTC (permalink / raw)
To: Michal Hocko
Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org,
marcelo.leitner
In-Reply-To: <20170126074354.GB8456@dhcp22.suse.cz>
On 01/26/2017 08:43 AM, Michal Hocko wrote:
> On Wed 25-01-17 21:16:42, Daniel Borkmann wrote:
>> On 01/25/2017 07:14 PM, Alexei Starovoitov wrote:
>>> On Wed, Jan 25, 2017 at 5:21 AM, Michal Hocko <mhocko@kernel.org> wrote:
>>>> On Wed 25-01-17 14:10:06, Michal Hocko wrote:
>>>>> On Tue 24-01-17 11:17:21, Alexei Starovoitov wrote:
>> [...]
>>>>>>> Are there any more comments? I would really appreciate to hear from
>>>>>>> networking folks before I resubmit the series.
>>>>>>
>>>>>> while this patchset was baking the bpf side switched to use bpf_map_area_alloc()
>>>>>> which fixes the issue with missing __GFP_NORETRY that we had to fix quickly.
>>>>>> See commit d407bd25a204 ("bpf: don't trigger OOM killer under pressure with map alloc")
>>>>>> it covers all kmalloc/vmalloc pairs instead of just one place as in this set.
>>>>>> So please rebase and switch bpf_map_area_alloc() to use kvmalloc().
>>>>>
>>>>> OK, will do. Thanks for the heads up.
>>>>
>>>> Just for the record, I will fold the following into the patch 1
>>>> ---
>>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>>> index 19b6129eab23..8697f43cf93c 100644
>>>> --- a/kernel/bpf/syscall.c
>>>> +++ b/kernel/bpf/syscall.c
>>>> @@ -53,21 +53,7 @@ void bpf_register_map_type(struct bpf_map_type_list *tl)
>>>>
>>>> void *bpf_map_area_alloc(size_t size)
>>>> {
>>>> - /* We definitely need __GFP_NORETRY, so OOM killer doesn't
>>>> - * trigger under memory pressure as we really just want to
>>>> - * fail instead.
>>>> - */
>>>> - const gfp_t flags = __GFP_NOWARN | __GFP_NORETRY | __GFP_ZERO;
>>>> - void *area;
>>>> -
>>>> - if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
>>>> - area = kmalloc(size, GFP_USER | flags);
>>>> - if (area != NULL)
>>>> - return area;
>>>> - }
>>>> -
>>>> - return __vmalloc(size, GFP_KERNEL | __GFP_HIGHMEM | flags,
>>>> - PAGE_KERNEL);
>>>> + return kvzalloc(size, GFP_USER);
>>>> }
>>>>
>>>> void bpf_map_area_free(void *area)
>>>
>>> Looks fine by me.
>>> Daniel, thoughts?
>>
>> I assume that kvzalloc() is still the same from [1], right? If so, then
>> it would unfortunately (partially) reintroduce the issue that was fixed.
>> If you look above at flags, they're also passed to __vmalloc() to not
>> trigger OOM in these situations I've experienced.
>
> Pushing __GFP_NORETRY to __vmalloc doesn't have the effect you might
> think it would. It can still trigger the OOM killer becauset the flags
> are no propagated all the way down to all allocations requests (e.g.
> page tables). This is the same reason why GFP_NOFS is not supported in
> vmalloc.
Ok, good to know, is that somewhere clearly documented (like for the
case with kmalloc())? If not, could we do that for non-mm folks, or
at least add a similar WARN_ON_ONCE() as you did for kvmalloc() to make
it obvious to users that a given flag combination is not supported all
the way down?
>> This is effectively the
>> same requirement as in other networking areas f.e. that 5bad87348c70
>> ("netfilter: x_tables: avoid warn and OOM killer on vmalloc call") has.
>> In your comment in kvzalloc() you eventually say that some of the above
>> modifiers are not supported. So there would be two options, i) just leave
>> out the kvzalloc() chunk for BPF area to avoid the merge conflict and tackle
>> it later (along with similar code from 5bad87348c70), or ii) implement
>> support for these modifiers as well to your original set. I guess it's not
>> too urgent, so we could also proceed with i) if that is easier for you to
>> proceed (I don't mind either way).
>
> Could you clarify why the oom killer in vmalloc matters actually?
For both mentioned commits, (privileged) user space can potentially
create large allocation requests, where we thus switch to vmalloc()
flavor eventually and then OOM starts killing processes to try to
satisfy the allocation request. This is bad, because we want the
request to just fail instead as it's non-critical and f.e. not kill
ssh connection et al. Failing is totally fine in this case, whereas
triggering OOM is not. In my testing, __GFP_NORETRY did satisfy this
just fine, but as you say it seems it's not enough. Given there are
multiple places like these in the kernel, could we instead add an
option such as __GFP_NOOOM, or just make __GFP_NORETRY supported?
Thanks,
Daniel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* RE: [PATCH 0/6 v3] kvmalloc
From: David Laight @ 2017-01-26 9:48 UTC (permalink / raw)
To: 'Daniel Borkmann', Michal Hocko
Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org,
marcelo.leitner@gmail.com
In-Reply-To: <5889C331.7020101@iogearbox.net>
From: Daniel Borkmann
> Sent: 26 January 2017 09:37
...
> >> I assume that kvzalloc() is still the same from [1], right? If so, then
> >> it would unfortunately (partially) reintroduce the issue that was fixed.
> >> If you look above at flags, they're also passed to __vmalloc() to not
> >> trigger OOM in these situations I've experienced.
> >
> > Pushing __GFP_NORETRY to __vmalloc doesn't have the effect you might
> > think it would. It can still trigger the OOM killer becauset the flags
> > are no propagated all the way down to all allocations requests (e.g.
> > page tables). This is the same reason why GFP_NOFS is not supported in
> > vmalloc.
>
> Ok, good to know, is that somewhere clearly documented (like for the
> case with kmalloc())? If not, could we do that for non-mm folks, or
> at least add a similar WARN_ON_ONCE() as you did for kvmalloc() to make
> it obvious to users that a given flag combination is not supported all
> the way down?
ISTM that requests for the relatively small memory blocks needed for page
tables aren't really likely to invoke the OOM killer when it isn't already
being invoked by other actions. So that isn't really a problem.
More of a problem is that requests that you really don't mind failing
can use the last 'reasonably available' memory.
This will cause the next allocate to fail when it would be better for
the earlier one to fail instead.
David
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 0/6 v3] kvmalloc
From: Michal Hocko @ 2017-01-26 10:08 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org,
marcelo.leitner
In-Reply-To: <5889C331.7020101@iogearbox.net>
On Thu 26-01-17 10:36:49, Daniel Borkmann wrote:
> On 01/26/2017 08:43 AM, Michal Hocko wrote:
> > On Wed 25-01-17 21:16:42, Daniel Borkmann wrote:
[...]
> > > I assume that kvzalloc() is still the same from [1], right? If so, then
> > > it would unfortunately (partially) reintroduce the issue that was fixed.
> > > If you look above at flags, they're also passed to __vmalloc() to not
> > > trigger OOM in these situations I've experienced.
> >
> > Pushing __GFP_NORETRY to __vmalloc doesn't have the effect you might
> > think it would. It can still trigger the OOM killer becauset the flags
> > are no propagated all the way down to all allocations requests (e.g.
> > page tables). This is the same reason why GFP_NOFS is not supported in
> > vmalloc.
>
> Ok, good to know, is that somewhere clearly documented (like for the
> case with kmalloc())?
I am afraid that we really suck on this front. I will add something.
> If not, could we do that for non-mm folks, or
> at least add a similar WARN_ON_ONCE() as you did for kvmalloc() to make
> it obvious to users that a given flag combination is not supported all
> the way down?
I am not sure that triggering a warning that somebody has used
__GFP_NOWARN is very helpful ;). I also do not think that covering all the
supported flags is really feasible. Most of them will not have bad side
effects. I have added the warning because this API is new and I wanted
to catch new abusers. Old ones would have to die slowly.
> > > This is effectively the
> > > same requirement as in other networking areas f.e. that 5bad87348c70
> > > ("netfilter: x_tables: avoid warn and OOM killer on vmalloc call") has.
> > > In your comment in kvzalloc() you eventually say that some of the above
> > > modifiers are not supported. So there would be two options, i) just leave
> > > out the kvzalloc() chunk for BPF area to avoid the merge conflict and tackle
> > > it later (along with similar code from 5bad87348c70), or ii) implement
> > > support for these modifiers as well to your original set. I guess it's not
> > > too urgent, so we could also proceed with i) if that is easier for you to
> > > proceed (I don't mind either way).
> >
> > Could you clarify why the oom killer in vmalloc matters actually?
>
> For both mentioned commits, (privileged) user space can potentially
> create large allocation requests, where we thus switch to vmalloc()
> flavor eventually and then OOM starts killing processes to try to
> satisfy the allocation request. This is bad, because we want the
> request to just fail instead as it's non-critical and f.e. not kill
> ssh connection et al. Failing is totally fine in this case, whereas
> triggering OOM is not.
I see your intention but does it really make any real difference?
Consider you would back off right before you would have OOMed. Any
parallel request would just hit the OOM for you. You are (almost) never
doing an allocation in an isolation.
> In my testing, __GFP_NORETRY did satisfy this
> just fine, but as you say it seems it's not enough.
Yeah, ptes have been most probably popullated already.
> Given there are
> multiple places like these in the kernel, could we instead add an
> option such as __GFP_NOOOM, or just make __GFP_NORETRY supported?
As said above I do not really think that suppressing the OOM killer
makes any difference because it might be just somebody else doing that
for you. Also the OOM killer is the MM internal implementation "detail"
users shouldn't really care. I agree that callers should have a way to
say they do not want to try really hard and that is not that simple
for vmalloc unfortunatelly. The main problem here is that gfp mask
propagation is not that easy to fix without a lot of code churn as some
of those hardcoded allocation requests are deep in call chains.
I know this sucks and it would be great to support __GFP_NORETRY to
[k]vmalloc and maybe we will get there eventually. But for the mean time
I really think that using kvmalloc wherever possible is much better than
open coded variants whith expectations which do not hold sometimes.
If you disagree I can drop the bpf part of course...
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH 0/5] GTP: Fine-tuning for some function implementations
From: SF Markus Elfring @ 2017-01-26 10:15 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, David S. Miller, Harald Welte,
Johannes Berg, Pablo Neira
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 26 Jan 2017 11:10:01 +0100
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (5):
Use kmalloc_array() in gtp_hashtable_new()
Improve another size determination in ipv4_pdp_add()
Adjust 12 checks for null pointers
Rename jump labels in gtp_encap_enable()
Rename jump labels in gtp_hashtable_new()
drivers/net/gtp.c | 46 ++++++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 22 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH 1/5] gtp: Use kmalloc_array() in gtp_hashtable_new()
From: SF Markus Elfring @ 2017-01-26 10:17 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, David S. Miller, Harald Welte,
Johannes Berg, Pablo Neira
Cc: LKML, kernel-janitors
In-Reply-To: <698070a4-e08e-78e5-0214-df69783cce22@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 25 Jan 2017 22:01:00 +0100
* Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
* Replace the specification of data structures by pointer dereferences
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/gtp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8b6810bad54b..5d0d520ae40f 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -783,11 +783,13 @@ static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
{
int i;
- gtp->addr_hash = kmalloc(sizeof(struct hlist_head) * hsize, GFP_KERNEL);
+ gtp->addr_hash = kmalloc_array(hsize, sizeof(*gtp->addr_hash),
+ GFP_KERNEL);
if (gtp->addr_hash == NULL)
return -ENOMEM;
- gtp->tid_hash = kmalloc(sizeof(struct hlist_head) * hsize, GFP_KERNEL);
+ gtp->tid_hash = kmalloc_array(hsize, sizeof(*gtp->tid_hash),
+ GFP_KERNEL);
if (gtp->tid_hash == NULL)
goto err1;
--
2.11.0
^ permalink raw reply related
* [PATCH 2/5] gtp: Improve another size determination in ipv4_pdp_add()
From: SF Markus Elfring @ 2017-01-26 10:18 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, David S. Miller, Harald Welte,
Johannes Berg, Pablo Neira
Cc: LKML, kernel-janitors
In-Reply-To: <698070a4-e08e-78e5-0214-df69783cce22@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 25 Jan 2017 22:24:08 +0100
Replace the specification of a data type by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/gtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 5d0d520ae40f..f0c123101aa9 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -956,7 +956,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info)
}
- pctx = kmalloc(sizeof(struct pdp_ctx), GFP_KERNEL);
+ pctx = kmalloc(sizeof(*pctx), GFP_KERNEL);
if (pctx == NULL)
return -ENOMEM;
--
2.11.0
^ permalink raw reply related
* [PATCH 3/5] gtp: Adjust 12 checks for null pointers
From: SF Markus Elfring @ 2017-01-26 10:19 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, David S. Miller, Harald Welte,
Johannes Berg, Pablo Neira
Cc: LKML, kernel-janitors
In-Reply-To: <698070a4-e08e-78e5-0214-df69783cce22@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 25 Jan 2017 22:50:23 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script "checkpatch.pl" pointed information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/gtp.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index f0c123101aa9..7e38786361d8 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -785,12 +785,12 @@ static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
gtp->addr_hash = kmalloc_array(hsize, sizeof(*gtp->addr_hash),
GFP_KERNEL);
- if (gtp->addr_hash == NULL)
+ if (!gtp->addr_hash)
return -ENOMEM;
gtp->tid_hash = kmalloc_array(hsize, sizeof(*gtp->tid_hash),
GFP_KERNEL);
- if (gtp->tid_hash == NULL)
+ if (!gtp->tid_hash)
goto err1;
gtp->hash_size = hsize;
@@ -832,7 +832,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
netdev_dbg(dev, "enable gtp on %d, %d\n", fd_gtp0, fd_gtp1);
sock0 = sockfd_lookup(fd_gtp0, &err);
- if (sock0 == NULL) {
+ if (!sock0) {
netdev_dbg(dev, "socket fd=%d not found (gtp0)\n", fd_gtp0);
return -ENOENT;
}
@@ -844,7 +844,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
}
sock1u = sockfd_lookup(fd_gtp1, &err);
- if (sock1u == NULL) {
+ if (!sock1u) {
netdev_dbg(dev, "socket fd=%d not found (gtp1u)\n", fd_gtp1);
err = -ENOENT;
goto err1;
@@ -957,7 +957,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info)
}
pctx = kmalloc(sizeof(*pctx), GFP_KERNEL);
- if (pctx == NULL)
+ if (!pctx)
return -ENOMEM;
ipv4_pdp_fill(pctx, info);
@@ -1029,7 +1029,7 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
/* Check if there's an existing gtpX device to configure */
dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK]));
- if (dev == NULL) {
+ if (!dev) {
put_net(net);
return -ENODEV;
}
@@ -1055,7 +1055,7 @@ static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info)
/* Check if there's an existing gtpX device to configure */
dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK]));
- if (dev == NULL) {
+ if (!dev) {
put_net(net);
return -ENODEV;
}
@@ -1079,7 +1079,7 @@ static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}
- if (pctx == NULL)
+ if (!pctx)
return -ENOENT;
if (pctx->gtp_version == GTP_V0)
@@ -1105,7 +1105,7 @@ static int gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq,
genlh = genlmsg_put(skb, snd_portid, snd_seq, >p_genl_family, 0,
type);
- if (genlh == NULL)
+ if (!genlh)
goto nlmsg_failure;
if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) ||
@@ -1163,7 +1163,7 @@ static int gtp_genl_get_pdp(struct sk_buff *skb, struct genl_info *info)
/* Check if there's an existing gtpX device to configure */
dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK]));
- if (dev == NULL) {
+ if (!dev) {
put_net(net);
return -ENODEV;
}
@@ -1188,13 +1188,13 @@ static int gtp_genl_get_pdp(struct sk_buff *skb, struct genl_info *info)
pctx = ipv4_pdp_find(gtp, ip);
}
- if (pctx == NULL) {
+ if (!pctx) {
err = -ENOENT;
goto err_unlock;
}
skb2 = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
- if (skb2 == NULL) {
+ if (!skb2) {
err = -ENOMEM;
goto err_unlock;
}
--
2.11.0
^ permalink raw reply related
* [PATCH 4/5] gtp: Rename jump labels in gtp_encap_enable()
From: SF Markus Elfring @ 2017-01-26 10:20 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, David S. Miller, Harald Welte,
Johannes Berg, Pablo Neira
Cc: LKML, kernel-janitors
In-Reply-To: <698070a4-e08e-78e5-0214-df69783cce22@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 26 Jan 2017 10:48:40 +0100
Adjust jump labels according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/gtp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 7e38786361d8..2d4ddc000919 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -840,20 +840,20 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
if (sock0->sk->sk_protocol != IPPROTO_UDP) {
netdev_dbg(dev, "socket fd=%d not UDP\n", fd_gtp0);
err = -EINVAL;
- goto err1;
+ goto put_socket;
}
sock1u = sockfd_lookup(fd_gtp1, &err);
if (!sock1u) {
netdev_dbg(dev, "socket fd=%d not found (gtp1u)\n", fd_gtp1);
err = -ENOENT;
- goto err1;
+ goto put_socket;
}
if (sock1u->sk->sk_protocol != IPPROTO_UDP) {
netdev_dbg(dev, "socket fd=%d not UDP\n", fd_gtp1);
err = -EINVAL;
- goto err2;
+ goto put_socket_u;
}
netdev_dbg(dev, "enable gtp on %p, %p\n", sock0, sock1u);
@@ -873,9 +873,9 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
setup_udp_tunnel_sock(sock_net(gtp->sock1u->sk), gtp->sock1u, &tuncfg);
err = 0;
-err2:
+put_socket_u:
sockfd_put(sock1u);
-err1:
+put_socket:
sockfd_put(sock0);
return err;
}
--
2.11.0
^ permalink raw reply related
* [PATCH 5/5] gtp: Rename jump labels in gtp_hashtable_new()
From: SF Markus Elfring @ 2017-01-26 10:22 UTC (permalink / raw)
To: netdev, Alexey Dobriyan, David S. Miller, Harald Welte,
Johannes Berg, Pablo Neira
Cc: LKML, kernel-janitors
In-Reply-To: <698070a4-e08e-78e5-0214-df69783cce22@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 26 Jan 2017 10:54:20 +0100
Adjust a jump label according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/gtp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 2d4ddc000919..1571d6c7fe81 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -791,7 +791,7 @@ static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
gtp->tid_hash = kmalloc_array(hsize, sizeof(*gtp->tid_hash),
GFP_KERNEL);
if (!gtp->tid_hash)
- goto err1;
+ goto free_hash;
gtp->hash_size = hsize;
@@ -800,7 +800,7 @@ static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
INIT_HLIST_HEAD(>p->tid_hash[i]);
}
return 0;
-err1:
+free_hash:
kfree(gtp->addr_hash);
return -ENOMEM;
}
--
2.11.0
^ 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