* Re: [PATCHv2 net] netpoll: fix rx_hook() interface by passing the skb
From: Antonio Quartulli @ 2013-10-24 12:01 UTC (permalink / raw)
To: David Laight; +Cc: David Miller, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73A5@saturn3.aculab.com>
[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]
On Thu, Oct 24, 2013 at 09:43:38AM +0100, David Laight wrote:
> > Subject: [PATCHv2 net] netpoll: fix rx_hook() interface by passing the skb
> > @@ -820,7 +823,10 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
> >
> > len -= iph->ihl*4;
> > uh = (struct udphdr *)(((char *)iph) + iph->ihl*4);
> > + offset = (unsigned char *)(uh + 1) - skb->data;
> > ulen = ntohs(uh->len);
> > + data_len = skb->len - offset;
> > + source = ntohs(uh->source);
> >
> > if (ulen != len)
> > goto out;
> > @@ -834,9 +840,7 @@ int __netpoll_rx(struct sk_buff *skb, struct netpoll_info *npinfo)
> > if (np->local_port && np->local_port != ntohs(uh->dest))
> > continue;
> >
> > - np->rx_hook(np, ntohs(uh->source),
> > - (char *)(uh+1),
> > - ulen - sizeof(struct udphdr));
> > + np->rx_skb_hook(np, source, skb, offset, data_len);
> > hits++;
> > }
> > } else {
>
> From a code optimisation point of view you probably don't want to be
> calculating the source, offset and length early.
> It is quite likely that the local variables will have to be written
> to the stack (because of the function calls) - so it is almost
> certainly more efficient to calculate them just before the call.
I thought that computing them once outside the loop was better than
re-computing them during each iteration.
Having them outside makes it also clear that they always have the same value.
Regards,
--
Antonio Quartulli
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* FYI!!
From: Mr. Tan Kong @ 2013-10-24 12:15 UTC (permalink / raw)
To: tan
Hello,
I am asking for your partnership in re-profiling funds.
Regards,
Tan Kong
^ permalink raw reply
* Re: Bluetooth 6LoWPAN and routing
From: Alexander Aring @ 2013-10-24 12:25 UTC (permalink / raw)
To: Jukka Rissanen; +Cc: netdev
In-Reply-To: <5268C214.2040307@linux.intel.com>
Hi Jukka,
On Thu, Oct 24, 2013 at 09:45:40AM +0300, Jukka Rissanen wrote:
> Hi,
>
> I have been prototyping with BT 6LoWPAN support (using this draft
> http://tools.ietf.org/html/draft-ietf-6lowpan-btle-12 as a
> reference). I sent first version yesterday to linux-bluetooth ml
> http://thread.gmane.org/gmane.linux.bluez.kernel/39394
>
I see you take many code from the 6lowpan ieee802154 implementation.
(Just notice you drop the original authors from there)
I have a couple of patches to fix a lot of bugs in the current 6LoWPAN
ieee802154 implementation.
Some bugs which I found:
- Fix race conditions in fragmentation handling
- Fix UDP compression/uncompressionm, which is completly broken
- Fragmentation handling isn't rfc4944 compatible
And some other improvements. I see your rfc has the same issues (e.g.
fragmentation race conditions).
Currently I preparing these patches for mainlining.
But my question is:
What we do now, make a generic 6LoWPAN implementation. Or bluetooth,
ieee802154 makes his own implementation?
- Alex
^ permalink raw reply
* Re: [PACTH net-next] SUNRPC: remove an unnecessary if statement
From: Ding Tianhong @ 2013-10-24 12:29 UTC (permalink / raw)
To: wangweidong; +Cc: davem, Trond.Myklebust, bfields, netdev, linux-nfs
In-Reply-To: <52688768.9020703@huawei.com>
On 2013/10/24 10:35, wangweidong wrote:
> If req allocated failed just goto out_free, no need to check the
> 'i < num_prealloc'. There is just code simplification, no
> functional changes.
>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> ---
> net/sunrpc/xprt.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index 095363e..a8e20de 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -1087,11 +1087,9 @@ struct rpc_xprt *xprt_alloc(struct net *net, size_t size,
> for (i = 0; i < num_prealloc; i++) {
> req = kzalloc(sizeof(struct rpc_rqst), GFP_KERNEL);
> if (!req)
> - break;
> + goto out_free;
> list_add(&req->rq_list, &xprt->free);
> }
> - if (i < num_prealloc)
> - goto out_free;
> if (max_alloc > num_prealloc)
> xprt->max_reqs = max_alloc;
> else
> -- 1.7.12
>
>
Acked-by: Ding Tianhong <dingtianhong@huawei.com>
>
>
> .
>
^ permalink raw reply
* crypto: skcipher - Use eseqiv even on UP machines
From: Herbert Xu @ 2013-10-24 12:41 UTC (permalink / raw)
To: David S. Miller, Steffen Klassert, netdev,
Linux Crypto Mailing List
Hi:
Previously we would use eseqiv on all async ciphers in all cases,
and sync ciphers if we have more than one CPU. This meant that
chainiv is only used in the case of sync ciphers on a UP machine.
As chainiv may aid attackers by making the IV predictable, even
though this risk itself is small, the above usage pattern causes
it to further leak information about the host.
This patch addresses these issues by using eseqiv even if we're
on a UP machine.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
--git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index 7d4a8d2..40886c4 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -16,9 +16,7 @@
#include <crypto/internal/skcipher.h>
#include <linux/cpumask.h>
#include <linux/err.h>
-#include <linux/init.h>
#include <linux/kernel.h>
-#include <linux/module.h>
#include <linux/rtnetlink.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -30,8 +28,6 @@
#include "internal.h"
-static const char *skcipher_default_geniv __read_mostly;
-
struct ablkcipher_buffer {
struct list_head entry;
struct scatter_walk dst;
@@ -527,8 +523,7 @@ const char *crypto_default_geniv(const struct crypto_alg *alg)
alg->cra_blocksize)
return "chainiv";
- return alg->cra_flags & CRYPTO_ALG_ASYNC ?
- "eseqiv" : skcipher_default_geniv;
+ return "eseqiv";
}
static int crypto_givcipher_default(struct crypto_alg *alg, u32 type, u32 mask)
@@ -709,17 +704,3 @@ err:
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(crypto_alloc_ablkcipher);
-
-static int __init skcipher_module_init(void)
-{
- skcipher_default_geniv = num_possible_cpus() > 1 ?
- "eseqiv" : "chainiv";
- return 0;
-}
-
-static void skcipher_module_exit(void)
-{
-}
-
-module_init(skcipher_module_init);
-module_exit(skcipher_module_exit);
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: Bluetooth 6LoWPAN and routing
From: Jukka Rissanen @ 2013-10-24 12:48 UTC (permalink / raw)
To: Alexander Aring; +Cc: netdev
In-Reply-To: <20131024122452.GA5491@omega>
Hi Alexander,
On 24.10.2013 15:25, Alexander Aring wrote:
> Hi Jukka,
>
> On Thu, Oct 24, 2013 at 09:45:40AM +0300, Jukka Rissanen wrote:
>> Hi,
>>
>> I have been prototyping with BT 6LoWPAN support (using this draft
>> http://tools.ietf.org/html/draft-ietf-6lowpan-btle-12 as a
>> reference). I sent first version yesterday to linux-bluetooth ml
>> http://thread.gmane.org/gmane.linux.bluez.kernel/39394
>>
> I see you take many code from the 6lowpan ieee802154 implementation.
> (Just notice you drop the original authors from there)
Hmm, those got dropped, I am sorry about that. I will add the original
authors information of course.
>
> I have a couple of patches to fix a lot of bugs in the current 6LoWPAN
> ieee802154 implementation.
>
> Some bugs which I found:
>
> - Fix race conditions in fragmentation handling
> - Fix UDP compression/uncompressionm, which is completly broken
> - Fragmentation handling isn't rfc4944 compatible
>
> And some other improvements. I see your rfc has the same issues (e.g.
> fragmentation race conditions).
>
> Currently I preparing these patches for mainlining.
Excellent news!
>
> But my question is:
>
> What we do now, make a generic 6LoWPAN implementation. Or bluetooth,
> ieee802154 makes his own implementation?
At least the compression/uncompression code and fragmentation could be
shared. Could it possible to isolate relevant functions to separate file
that would be usable by both BT and ieee802154?
>
> - Alex
>
--
Cheers,
Jukka
^ permalink raw reply
* Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace
From: François Cachereul @ 2013-10-24 13:41 UTC (permalink / raw)
To: James Chapman; +Cc: Paul Mackerras, netdev, linux-ppp
In-Reply-To: <5268FCB1.7020903@katalix.com>
On 10/24/2013 12:55 PM, James Chapman wrote:
> On 24/10/13 11:30, François Cachereul wrote:
>> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
>> if the device is connected to a l2tp session socket.
>> Restore the flag in ppp_disconnect_channel().
>
> What about pppd's network namespace? Also, L2TP's tunnel socket (UDP or
> L2TP/IP) will be in a different namespace if the ppp interface is moved.
That's what I'm trying to achieve. I'm not using pppd and my problem is
as follow: I need to isolate ppp devices from each other, even when
they are connected to sessions carried by the same L2TP tunnel. Also, I
need the authentication to be terminated to know the namespace in which
the ppp will be moved. For that, the process runs in a namespace with
its l2tp sockets (tunnel and session) in that same namespace and each
ppp device is moved in a specific namespace after authentication.
Regards
François
^ permalink raw reply
* [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Jiri Pirko @ 2013-10-24 13:45 UTC (permalink / raw)
To: netdev; +Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen
This is needed in order to implement userspace address configuration,
namely ip6-privacy (rfc4941) in NetworkManager.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
net/ipv6/addrconf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cd3fb30..962c7c9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
return -ENODEV;
/* We ignore other flags so far. */
- ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
+ ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
+ IFA_F_TEMPORARY);
ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
if (ifa == NULL) {
--
1.8.3.1
^ permalink raw reply related
* [patch iproute2] allow to create temporary addresses
From: Jiri Pirko @ 2013-10-24 13:48 UTC (permalink / raw)
To: netdev; +Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen
In-Reply-To: <1382622355-6500-1-git-send-email-jiri@resnulli.us>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
ip/ipaddress.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1c3e4da..3ca774d 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1332,6 +1332,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
req.ifa.ifa_flags |= IFA_F_HOMEADDRESS;
} else if (strcmp(*argv, "nodad") == 0) {
req.ifa.ifa_flags |= IFA_F_NODAD;
+ } else if (strcmp(*argv, "temporary") == 0 ||
+ strcmp(*argv, "secondary") == 0) {
+ req.ifa.ifa_flags |= IFA_F_SECONDARY;
} else {
if (strcmp(*argv, "local") == 0) {
NEXT_ARG();
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: Hannes Frederic Sowa @ 2013-10-24 14:02 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen
In-Reply-To: <1382622355-6500-1-git-send-email-jiri@resnulli.us>
On Thu, Oct 24, 2013 at 03:45:55PM +0200, Jiri Pirko wrote:
> This is needed in order to implement userspace address configuration,
> namely ip6-privacy (rfc4941) in NetworkManager.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> net/ipv6/addrconf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index cd3fb30..962c7c9 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3715,7 +3715,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
> return -ENODEV;
>
> /* We ignore other flags so far. */
> - ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
> + ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS |
> + IFA_F_TEMPORARY);
>
> ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
> if (ifa == NULL) {
Hm, the kernel will pick up IFA_F_TEMPORARY marked addresses and do ipv6 address
regeneration (depending on lifetimes). Is this intended?
Btw. I am very interested in this topic as there is currently work in the IETF
to move away from eui-48 generation of addresses:
https://datatracker.ietf.org/doc/draft-ietf-6man-stable-privacy-addresses/
This needs to be done in userspace as we depend on a secret generated at
system install time.
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH 1/1] net:sched fix a bug about memery leak
From: Sergei Shtylyov @ 2013-10-24 14:05 UTC (permalink / raw)
To: Jing Wang, netdev
In-Reply-To: <1382605580-2629-1-git-send-email-windsdaemon@gmail.com>
Hello.
On 10/24/2013 01:06 PM, Jing Wang wrote:
> From: Jing Wang <windsdaemon@gmail.com>
> the code isn't properly release memory
> Signed-off-by: Jing Wang <windsdaemon@gmail.com>
> ---
> net/sched/cls_route.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 37da567..118f8d5 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
[...]
> @@ -517,6 +517,9 @@ reinsert:
>
> errout:
> kfree(f);
> +errflt:
> + kfree(head);
Please indent with tab, not spaces.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/1] net:sched fix a bug about memery leak
From: Christoph Paasch @ 2013-10-24 14:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jing Wang, davem, jhs, netdev
In-Reply-To: <1382607218.7572.45.camel@edumazet-glaptop.roam.corp.google.com>
On 24/10/13 - 02:33:38, Eric Dumazet wrote:
> On Thu, 2013-10-24 at 17:12 +0800, Jing Wang wrote:
> > From: Jing Wang <windsdaemon@gmail.com>
> >
> > the code isn't properly release memory
> >
> > Signed-off-by: Jing Wang <windsdaemon@gmail.com>
> > ---
> > net/sched/cls_route.c | 9 ++++++---
> > 1 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> > index 37da567..118f8d5 100644
> > --- a/net/sched/cls_route.c
> > +++ b/net/sched/cls_route.c
> > @@ -466,11 +466,11 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> > goto reinsert;
> > }
> >
> > - err = -ENOBUFS;
> > + err = -ENOMEM;
> > if (head == NULL) {
> > head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> > if (head == NULL)
> > - goto errout;
> > + goto errhead;
> >
> > tcf_tree_lock(tp);
> > tp->root = head;
> > @@ -479,7 +479,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> >
> > f = kzalloc(sizeof(struct route4_filter), GFP_KERNEL);
> > if (f == NULL)
> > - goto errout;
> > + goto errflt;
> >
> > err = route4_set_parms(net, tp, base, f, handle, head, tb,
> > tca[TCA_RATE], 1);
> > @@ -517,6 +517,9 @@ reinsert:
> >
> > errout:
> > kfree(f);
> > +errflt:
> > + kfree(head);
> > +errhead:
> > return err;
> > }
> >
>
> I don't think this patch is needed or correct.
>
> tp->root is the head, you cannot free it like that.
>
> It will be freed properly in route4_destroy()
>
> Please elaborate, thanks.
I think there is something else wrong in route4_change:
----
>From 1409402bf964bef79667755a5d0d5e0c2bd663f3 Mon Sep 17 00:00:00 2001
From: Christoph Paasch <christoph.paasch@uclouvain.be>
Date: Thu, 24 Oct 2013 15:33:28 +0200
Subject: [PATCH net] net: sched: Don't free f before it is allocated in
route4_change
f is set to *arg in route4_change at the beginning, which points to a
route4_filter in the hash-table (gotten through route4_get, called by
tc_ctl_filter). If the alloc of head fails, we should not goto errout,
because this will free f and thus freed memory will be referenced by
the hash-table.
Only later the pointer f will change to an allocated route4_filter.
This patch returns err if the allocation of head fails as f has not yet
been allocated inside route4_change.
Seems the code has been like this since Linus's original git-commit.
Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
net/sched/cls_route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 37da567..f17c67f 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
if (head == NULL) {
head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
if (head == NULL)
- goto errout;
+ return err;
tcf_tree_lock(tp);
tp->root = head;
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH 1/1] net:sched fix a bug about memery leak
From: Sergei Shtylyov @ 2013-10-24 14:10 UTC (permalink / raw)
To: Jing Wang, davem, jhs, netdev
In-Reply-To: <1382605964-2693-1-git-send-email-windsdaemon@gmail.com>
Hello.
On 10/24/2013 01:12 PM, Jing Wang wrote:
s/mememry/memory/ in the subject.
> From: Jing Wang <windsdaemon@gmail.com>
> the code isn't properly release memory
s/release/releasing/.
> Signed-off-by: Jing Wang <windsdaemon@gmail.com>
WBR, Sergei
^ permalink raw reply
* Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace
From: Sergei Shtylyov @ 2013-10-24 14:23 UTC (permalink / raw)
To: François Cachereul, Paul Mackerras, James Chapman; +Cc: netdev, linux-ppp
In-Reply-To: <5268F6CD.9070600@alphalink.fr>
Hello.
On 10/24/2013 02:30 PM, François Cachereul wrote:
> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
> if the device is connected to a l2tp session socket.
> Restore the flag in ppp_disconnect_channel().
> Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
> ---
> I'm trying to get rid of this flag for ppp device connected to l2tp
> session, it's seem to be safe to do it for as l2tp_ppp module hasn't any
> reference to the ppp device except to the device name. We can probably
> do it for others modules but pppoe and pptp will require more work.
> I remove the flag for l2tp in ppp_generic.c because I couldn't find a
> place like a callback to do it in l2tp_ppp.c. The best will be to
> remove the flag for all ppp devices.
> François
> drivers/net/ppp/ppp_generic.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index 72ff14b..7ccf2ae 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
[...]
> @@ -2883,6 +2886,13 @@ ppp_connect_channel(struct channel *pch, int unit)
> ++ppp->n_channels;
> pch->ppp = ppp;
> atomic_inc(&ppp->file.refcnt);
> +
> + /* allow ppp net device to be moved in another network namespace
> + * if it's connected to an l2tp session */
Acording to Documentation/CodingStyle, the preferred comment style in the
networking code is:
/* bla
* bla
*/
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/1] net:sched fix a bug about memery leak
From: Christoph Paasch @ 2013-10-24 14:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jing Wang, davem, jhs, netdev
In-Reply-To: <20131024140709.GA19470@cpaasch-mac>
On 24/10/13 - 16:07:09, Christoph Paasch wrote:
> On 24/10/13 - 02:33:38, Eric Dumazet wrote:
> > On Thu, 2013-10-24 at 17:12 +0800, Jing Wang wrote:
> > > From: Jing Wang <windsdaemon@gmail.com>
> > >
> > > the code isn't properly release memory
> > >
> > > Signed-off-by: Jing Wang <windsdaemon@gmail.com>
> > > ---
> > > net/sched/cls_route.c | 9 ++++++---
> > > 1 files changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> > > index 37da567..118f8d5 100644
> > > --- a/net/sched/cls_route.c
> > > +++ b/net/sched/cls_route.c
> > > @@ -466,11 +466,11 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> > > goto reinsert;
> > > }
> > >
> > > - err = -ENOBUFS;
> > > + err = -ENOMEM;
> > > if (head == NULL) {
> > > head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> > > if (head == NULL)
> > > - goto errout;
> > > + goto errhead;
> > >
> > > tcf_tree_lock(tp);
> > > tp->root = head;
> > > @@ -479,7 +479,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> > >
> > > f = kzalloc(sizeof(struct route4_filter), GFP_KERNEL);
> > > if (f == NULL)
> > > - goto errout;
> > > + goto errflt;
> > >
> > > err = route4_set_parms(net, tp, base, f, handle, head, tb,
> > > tca[TCA_RATE], 1);
> > > @@ -517,6 +517,9 @@ reinsert:
> > >
> > > errout:
> > > kfree(f);
> > > +errflt:
> > > + kfree(head);
> > > +errhead:
> > > return err;
> > > }
> > >
> >
> > I don't think this patch is needed or correct.
> >
> > tp->root is the head, you cannot free it like that.
> >
> > It will be freed properly in route4_destroy()
> >
> > Please elaborate, thanks.
>
> I think there is something else wrong in route4_change:
>
> ----
> From 1409402bf964bef79667755a5d0d5e0c2bd663f3 Mon Sep 17 00:00:00 2001
> From: Christoph Paasch <christoph.paasch@uclouvain.be>
> Date: Thu, 24 Oct 2013 15:33:28 +0200
> Subject: [PATCH net] net: sched: Don't free f before it is allocated in
> route4_change
>
> f is set to *arg in route4_change at the beginning, which points to a
> route4_filter in the hash-table (gotten through route4_get, called by
> tc_ctl_filter). If the alloc of head fails, we should not goto errout,
> because this will free f and thus freed memory will be referenced by
> the hash-table.
> Only later the pointer f will change to an allocated route4_filter.
>
> This patch returns err if the allocation of head fails as f has not yet
> been allocated inside route4_change.
>
> Seems the code has been like this since Linus's original git-commit.
>
> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> ---
> net/sched/cls_route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 37da567..f17c67f 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> if (head == NULL) {
> head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> if (head == NULL)
> - goto errout;
> + return err;
Oh... copy-paste from vim into mutt actually replaced the tabs by spaces...
I will resend. Sorry.
Christoph
>
> tcf_tree_lock(tp);
> tp->root = head;
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH NEXT 1/5] staging: r8188eu: Fix sparse warnings in ioctl_linux.c
From: Larry Finger @ 2013-10-24 14:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: devel, gregkh, netdev
In-Reply-To: <20131024101001.GD5871@mwanda>
On 10/24/2013 05:10 AM, Dan Carpenter wrote:
> I have looked at how this is called from ioctl_private_call() and it
> seems like these are actual user pointers and the code is buggy. The
> patch silences the warnings instead of fixing the bugs.
Thanks for the review.
Greg - please drop this one and take only 2-5. Do you want a resubmission?
Larry
^ permalink raw reply
* Re: [PATCH NEXT 1/5] staging: r8188eu: Fix sparse warnings in ioctl_linux.c
From: Greg KH @ 2013-10-24 14:51 UTC (permalink / raw)
To: Larry Finger; +Cc: Dan Carpenter, devel, netdev
In-Reply-To: <526932DD.2080602@lwfinger.net>
On Thu, Oct 24, 2013 at 09:46:53AM -0500, Larry Finger wrote:
> On 10/24/2013 05:10 AM, Dan Carpenter wrote:
> > I have looked at how this is called from ioctl_private_call() and it
> > seems like these are actual user pointers and the code is buggy. The
> > patch silences the warnings instead of fixing the bugs.
>
> Thanks for the review.
>
> Greg - please drop this one and take only 2-5. Do you want a resubmission?
I can just drop it. I'm way behind on patches due to the kernel summit
and LinuxCon, so it will be a week or so before I can catch up, sorry.
greg k-h
^ permalink raw reply
* [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Christoph Paasch @ 2013-10-24 14:50 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, David Miller, Jamal Hadi Salim, Jing Wang
f is set to *arg in route4_change at the beginning, which points to a
route4_filter in the hash-table (gotten through route4_get, called by
tc_ctl_filter). If the alloc of head fails, we should not goto errout,
because this will free f and thus freed memory will be referenced by
the hash-table.
Only later the pointer f will change to an allocated route4_filter.
This patch returns err if the allocation of head fails as f has not yet
been allocated inside route4_change.
Seems the code has been like this since Linus's original git-commit.
Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
net/sched/cls_route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 37da567..f17c67f 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
if (head == NULL) {
head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
if (head == NULL)
- goto errout;
+ return err;
tcf_tree_lock(tp);
tp->root = head;
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Eric Dumazet @ 2013-10-24 14:54 UTC (permalink / raw)
To: Christoph Paasch; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <1382626250-15676-1-git-send-email-christoph.paasch@uclouvain.be>
On Thu, 2013-10-24 at 16:50 +0200, Christoph Paasch wrote:
> f is set to *arg in route4_change at the beginning, which points to a
> route4_filter in the hash-table (gotten through route4_get, called by
> tc_ctl_filter). If the alloc of head fails, we should not goto errout,
> because this will free f and thus freed memory will be referenced by
> the hash-table.
> Only later the pointer f will change to an allocated route4_filter.
>
> This patch returns err if the allocation of head fails as f has not yet
> been allocated inside route4_change.
>
> Seems the code has been like this since Linus's original git-commit.
>
> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> ---
> net/sched/cls_route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 37da567..f17c67f 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> if (head == NULL) {
> head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> if (head == NULL)
> - goto errout;
> + return err;
>
> tcf_tree_lock(tp);
> tp->root = head;
I see no bug here, you missed the "goto reinsert;"
Guys, if there are no bugs, could we calm down ?
Thanks !
^ permalink raw reply
* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Christoph Paasch @ 2013-10-24 14:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <1382626473.7572.58.camel@edumazet-glaptop.roam.corp.google.com>
On 24/10/13 - 07:54:33, Eric Dumazet wrote:
> On Thu, 2013-10-24 at 16:50 +0200, Christoph Paasch wrote:
> > f is set to *arg in route4_change at the beginning, which points to a
> > route4_filter in the hash-table (gotten through route4_get, called by
> > tc_ctl_filter). If the alloc of head fails, we should not goto errout,
> > because this will free f and thus freed memory will be referenced by
> > the hash-table.
> > Only later the pointer f will change to an allocated route4_filter.
> >
> > This patch returns err if the allocation of head fails as f has not yet
> > been allocated inside route4_change.
> >
> > Seems the code has been like this since Linus's original git-commit.
> >
> > Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
> > ---
> > net/sched/cls_route.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> > index 37da567..f17c67f 100644
> > --- a/net/sched/cls_route.c
> > +++ b/net/sched/cls_route.c
> > @@ -470,7 +470,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
> > if (head == NULL) {
> > head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> > if (head == NULL)
> > - goto errout;
> > + return err;
> >
> > tcf_tree_lock(tp);
> > tp->root = head;
>
> I see no bug here, you missed the "goto reinsert;"
Ups - sorry...
^ permalink raw reply
* Re: [PATCH net] net: sched: Don't free f before it is allocated in route4_change
From: Eric Dumazet @ 2013-10-24 15:05 UTC (permalink / raw)
To: Christoph Paasch; +Cc: netdev, David Miller, Jamal Hadi Salim, Jing Wang
In-Reply-To: <20131024145936.GB15936@cpaasch-mac>
On Thu, 2013-10-24 at 16:59 +0200, Christoph Paasch wrote:
> On 24/10/13 - 07:54:33, Eric Dumazet wrote:
> > I see no bug here, you missed the "goto reinsert;"
>
> Ups - sorry...
>
Yeah, trying to find bugs in Alexey code is really tricky ;)
^ permalink raw reply
* RE: transmit lockup using smsc95xx ethernet on usb3
From: David Laight @ 2013-10-24 15:05 UTC (permalink / raw)
To: Sarah Sharp; +Cc: netdev, linux-usb, Xenia Ragiadakou
In-Reply-To: <20131017000758.GL7082@xanatos>
> Have you tried the latest stable kernel or the latest -rc kernel?
I've built a kernel based on Linus's tree from last Friday, 3.12-rc6 ish.
Commented out the trace for short reads - happens all the time.
I've not seen an error on a Bo yet, the failure rate is depressingly low.
I have had an error that started with an interrupt completion (I think
from the root), that leads to a full unplug-replug sequence that
overfilled the kernel message buffer (rebuilt with a much bigger buffer).
I've just got a error -71 from a Bi. This generates:
(There are actually two separated by a Bo completion and setup.
[175908.563068] xhci_hcd 0000:00:14.0: Transfer error on endpoint
[175908.563070] xhci_hcd 0000:00:14.0: Cleaning up stalled endpoint ring
[175908.563072] xhci_hcd 0000:00:14.0: Finding segment containing stopped TRB.
[175908.563074] xhci_hcd 0000:00:14.0: Finding endpoint context
[175908.563076] xhci_hcd 0000:00:14.0: Finding segment containing last TRB in TD.
[175908.563078] xhci_hcd 0000:00:14.0: Cycle state = 0x1
[175908.563081] xhci_hcd 0000:00:14.0: New dequeue segment = ffff8800d6a817e0 (virtual)
[175908.563089] xhci_hcd 0000:00:14.0: New dequeue pointer = 0x2137a4b90 (DMA)
[175908.563096] xhci_hcd 0000:00:14.0: Queueing new dequeue state
[175908.563104] xhci_hcd 0000:00:14.0: Set TR Deq Ptr cmd, new deq seg = ffff8800d6a817e0 (0x2137a4800 dma), new deq ptr = ffff8802137a4b90 (0x2137a4b90 dma), new cycle = 1
[175908.563110] xhci_hcd 0000:00:14.0: // Ding dong!
[175908.563119] xhci_hcd 0000:00:14.0: Giveback URB ffff8802114fd9c0, len = 0, expected = 18944, status = -71
[175908.563122] xhci_hcd 0000:00:14.0: Ignoring reset ep completion code of 1
[175908.563125] xhci_hcd 0000:00:14.0: Successful Set TR Deq Ptr cmd, deq = @2137a4b91
What is interesting is that the usbmon trace then shows only 2 Bi URB being used
(rather than 4). After 125ms (assuming the usbmon timestamps are us)
another 2 URB are added.
Since the URB are usually recycled (or at least freed and immediately realloced
so getting the same address) I wonder if this is a memory leak?
In any case waiting that long before adding the URB back doesn't seem right.
I don't think I've seen an error that only affected the Bi side before, so
don't know whether the recover behaviour has changed.
FWIW we have identified something 'sub-optimal' with the pcb layout that might
be responsible for noise on the USB data/clock source. However the xhci driver
should recover from such errors.
If the hw guys fix the pcb I'm going to need to keep a failing system!
David
^ permalink raw reply
* [net-next 00/11][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to igb, igbvf, i40e, ixgbe and ixgbevf.
Dan Carpenter provides a patch for igbvf to fix a bug found by a static
checker. If the new MTU is very large, then "new_mtu + ETH_HLEN +
ETH_FCS_LEN" can wrap and the check on the next line can underflow.
Wei Yongjun provides 2 patches, the first against igbvf adds a missing
iounmap() before the return from igbvf_probe(). The second against
i40e, removes the include <linux/version.h> because it is not needed.
Carolyn provides a patch for igb to fix a call to set the master/slave
mode for all m88 generation 2 PHY's and removes the call for I210
devices which do not need it.
Stefan Assmann provides a patch for igb to fix an issue which was broke
by:
commit fa44f2f185f7f9da19d331929bb1b56c1ccd1d93
Author: Greg Rose <gregory.v.rose@intel.com>
Date: Thu Jan 17 01:03:06 2013 -0800
igb: Enable SR-IOV configuration via PCI sysfs interface
which breaks the reloading of igb when VFs are assigned to a guest, in
several ways.
Jacob provides a patch for ixgbe and ixgbevf. First, against ixgbe,
cleans up ixgbe_enumerate_functions to reduce code complexity. The
second, against ixgbevf, adds support for ethtool's get_coalesce and
set_coalesce command for the ixgbevf driver.
Yijing Wang provides a patch for ixgbe to use pcie_capability_read_word()
to simplify the code.
Emil provides a ixgbe patch to fix an issue where the logic used to
detect changes in rx-usecs was incorrect and was masked by the call to
ixgbe_update_rsc().
Don provides 2 patches for ixgbevf. First creates a new function to set
PSRTYPE. The second bumps the ixgbevf driver version.
The following are changes since commit b45bd46decd947eaa3497699d450e0851d247534:
Merge tag 'batman-adv-for-davem' of git://git.open-mesh.org/linux-merge
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Carolyn Wyborny (1):
igb: Fix master/slave mode for all m88 i354 PHY's
Dan Carpenter (1):
igbvf: integer wrapping bug setting the mtu
Don Skidmore (2):
ixgbevf: Adds function to set PSRTYPE register
ixgbevf: bump driver version
Emil Tantilov (1):
ixgbe: fix rx-usecs range checks for BQL
Jacob Keller (2):
ixgbe: cleanup ixgbe_enumerate_functions
ixgbevf: implement ethtool get/set coalesce
Stefan Assmann (1):
igb: fix driver reload with VF assigned to guest
Wei Yongjun (2):
igbvf: add missing iounmap() on error in igbvf_probe()
i40e: remove unused including <linux/version.h>
Yijing Wang (1):
ixgbe: use pcie_capability_read_word() to simplify code
drivers/net/ethernet/intel/i40e/i40e.h | 1 -
drivers/net/ethernet/intel/igb/e1000_phy.c | 8 +--
drivers/net/ethernet/intel/igb/igb_main.c | 37 +++++------
drivers/net/ethernet/intel/igbvf/netdev.c | 8 +--
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 6 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 19 ++----
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 81 +++++++++++++++++++++++
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 ++++--
9 files changed, 134 insertions(+), 50 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [net-next 02/11] igbvf: add missing iounmap() on error in igbvf_probe()
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
To: davem; +Cc: Wei Yongjun, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Add the missing iounmap() before return from igbvf_probe()
in the error handling case.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Tested-by: Sibai Li <Sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igbvf/netdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index f48ae71..9fadbb2 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2698,7 +2698,7 @@ static int igbvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ei->get_variants) {
err = ei->get_variants(adapter);
if (err)
- goto err_ioremap;
+ goto err_get_variants;
}
/* setup adapter struct */
@@ -2795,6 +2795,7 @@ err_hw_init:
kfree(adapter->rx_ring);
err_sw_init:
igbvf_reset_interrupt_capability(adapter);
+err_get_variants:
iounmap(adapter->hw.hw_addr);
err_ioremap:
free_netdev(netdev);
--
1.8.3.1
^ permalink raw reply related
* [net-next 01/11] igbvf: integer wrapping bug setting the mtu
From: Jeff Kirsher @ 2013-10-24 15:27 UTC (permalink / raw)
To: davem; +Cc: Dan Carpenter, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1382628458-26601-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Dan Carpenter <dan.carpenter@oracle.com>
If new_mtu is very large then "new_mtu + ETH_HLEN + ETH_FCS_LEN" can
wrap and the check on the next line can underflow. This is one of those
bugs which can be triggered by the user if you have namespaces
configured.
Also since this is something the user can trigger then we don't want to
have dev_err() message.
This is a static checker fix and I'm not sure what the impact is.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Tested-by: Sibai Li Sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igbvf/netdev.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 93eb7ee..f48ae71 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2343,10 +2343,9 @@ static int igbvf_change_mtu(struct net_device *netdev, int new_mtu)
struct igbvf_adapter *adapter = netdev_priv(netdev);
int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN;
- if ((new_mtu < 68) || (max_frame > MAX_JUMBO_FRAME_SIZE)) {
- dev_err(&adapter->pdev->dev, "Invalid MTU setting\n");
+ if (new_mtu < 68 || new_mtu > INT_MAX - ETH_HLEN - ETH_FCS_LEN ||
+ max_frame > MAX_JUMBO_FRAME_SIZE)
return -EINVAL;
- }
#define MAX_STD_JUMBO_FRAME_SIZE 9234
if (max_frame > MAX_STD_JUMBO_FRAME_SIZE) {
--
1.8.3.1
^ 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