* [patch v3 09/11] xfrm: pass xfrm_user module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/xfrm/xfrm_user.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 421f984..3d909f7 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2308,17 +2308,20 @@ static struct xfrm_link {
int (*doit)(struct sk_buff *, struct nlmsghdr *, struct nlattr **);
int (*dump)(struct sk_buff *, struct netlink_callback *);
int (*done)(struct netlink_callback *);
+ struct module *module;
} xfrm_dispatch[XFRM_NR_MSGTYPES] = {
[XFRM_MSG_NEWSA - XFRM_MSG_BASE] = { .doit = xfrm_add_sa },
[XFRM_MSG_DELSA - XFRM_MSG_BASE] = { .doit = xfrm_del_sa },
[XFRM_MSG_GETSA - XFRM_MSG_BASE] = { .doit = xfrm_get_sa,
.dump = xfrm_dump_sa,
- .done = xfrm_dump_sa_done },
+ .done = xfrm_dump_sa_done,
+ .module = THIS_MODULE },
[XFRM_MSG_NEWPOLICY - XFRM_MSG_BASE] = { .doit = xfrm_add_policy },
[XFRM_MSG_DELPOLICY - XFRM_MSG_BASE] = { .doit = xfrm_get_policy },
[XFRM_MSG_GETPOLICY - XFRM_MSG_BASE] = { .doit = xfrm_get_policy,
.dump = xfrm_dump_policy,
- .done = xfrm_dump_policy_done },
+ .done = xfrm_dump_policy_done,
+ .module = THIS_MODULE },
[XFRM_MSG_ALLOCSPI - XFRM_MSG_BASE] = { .doit = xfrm_alloc_userspi },
[XFRM_MSG_ACQUIRE - XFRM_MSG_BASE] = { .doit = xfrm_add_acquire },
[XFRM_MSG_EXPIRE - XFRM_MSG_BASE] = { .doit = xfrm_add_sa_expire },
@@ -2362,6 +2365,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
struct netlink_dump_control c = {
.dump = link->dump,
.done = link->done,
+ .module = link->module,
};
return netlink_dump_start(net->xfrm.nlsk, skb, nlh, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 10/11] ipset: pass ipset module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem, eric.dumazet, steffen.klassert
Cc: netfilter-devel, linux-rdma, netdev, linux-crypto, pablo,
stephen.hemminger, jengelh, Gao feng, Jozsef Kadlecsik
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
net/netfilter/ipset/ip_set_core.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 778465f..6f2b55c 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1185,6 +1185,7 @@ ip_set_dump(struct sock *ctnl, struct sk_buff *skb,
struct netlink_dump_control c = {
.dump = ip_set_dump_start,
.done = ip_set_dump_done,
+ .module = THIS_MODULE,
};
return netlink_dump_start(ctnl, skb, nlh, &c);
}
--
1.7.7.6
^ permalink raw reply related
* [patch v3 11/11] infiniband: pass rdma_cm module to netlink_dump_start
From: Gao feng @ 2012-10-04 4:41 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ
Cc: netfilter-devel-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, pablo-Cap9r6Oaw4JrovVCs/uTlw,
stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA, jengelh-9+2X+4sQBs8,
Gao feng, Roland Dreier, Sean Hefty
In-Reply-To: <1349325689-16104-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/core/cma.c | 3 ++-
drivers/infiniband/core/netlink.c | 1 +
include/rdma/rdma_netlink.h | 1 +
3 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 26b3760..4fff27a 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -3498,7 +3498,8 @@ out:
}
static const struct ibnl_client_cbs cma_cb_table[] = {
- [RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats },
+ [RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats,
+ .module = THIS_MODULE },
};
static int __init cma_init(void)
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index fe10a94..da06abd 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -154,6 +154,7 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct netlink_dump_control c = {
.dump = client->cb_table[op].dump,
+ .module = client->cb_table[op].module,
};
return netlink_dump_start(nls, skb, nlh, &c);
}
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 3c5363a..bd3d8b2 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -39,6 +39,7 @@ struct rdma_cm_id_stats {
struct ibnl_client_cbs {
int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
+ struct module *module;
};
int ibnl_init(void);
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 related
* Re: [RFC] vxlan: use ether header as fallback hash
From: Eric Dumazet @ 2012-10-04 4:57 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jesse Gross, davem, netdev
In-Reply-To: <20121003213906.09b57539@nehalam.linuxnetplumber.net>
On Wed, 2012-10-03 at 21:39 -0700, Stephen Hemminger wrote:
> VXLAN bases source UDP port based on flow to help the
> receiver to be able to load balance based on outer header
> contents.
>
> This patches changes the algorithm to better handle packets
> that can not be categorized by the rxhash() function.
> It adds a fallback to use jhash on the Ether header.
>
> It also fixes a bug where the old code could assign 0 as a port
> value.
>
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
> ---
> RFC for now, compile tested only
>
> --- a/drivers/net/vxlan.c 2012-10-03 21:25:43.747968165 -0700
> +++ b/drivers/net/vxlan.c 2012-10-03 21:36:10.213805422 -0700
> @@ -622,12 +622,30 @@ static inline u8 vxlan_ecn_encap(u8 tos,
> return INET_ECN_encapsulate(tos, inner);
> }
>
#include <linux/jhash.h>
> +/* Compute hash to use for source port
> + * first choice to use L4 flow hash since it will spread
> + * better and maybe available from hardware
> + * secondary choice is to use jhash on the Ethernet header
> + * Always returns non-zero value
> + */
> +static u16 vxlan_flow_hash(struct sk_buff *skb)
> +{
> + u16 hash = skb_get_rxhash(skb);
skb_get_rxhash(skb) returns an u32, that could have low order 16bits set
to 0.
So I would use u32 hash = skb_get_rxhash(skb);
> +
> + if (!hash)
> + hash = jhash(skb->data, 3, skb->protocol);
> +
then here, do :
hash ^= hash >> 16;
hash &= 0xffff;
> + if (!hash)
> + hash = 1;
> +
> + return hash;
> +}
> +
^ permalink raw reply
* Re: [RFC] vxlan: use ether header as fallback hash
From: Stephen Hemminger @ 2012-10-04 5:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jesse Gross, davem, netdev
In-Reply-To: <1349326673.16011.5.camel@edumazet-glaptop>
> On Wed, 2012-10-03 at 21:39 -0700, Stephen Hemminger wrote:
> > VXLAN bases source UDP port based on flow to help the
> > receiver to be able to load balance based on outer header
> > contents.
> >
> > This patches changes the algorithm to better handle packets
> > that can not be categorized by the rxhash() function.
> > It adds a fallback to use jhash on the Ether header.
> >
> > It also fixes a bug where the old code could assign 0 as a port
> > value.
> >
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> > ---
> > RFC for now, compile tested only
> >
> > --- a/drivers/net/vxlan.c 2012-10-03 21:25:43.747968165 -0700
> > +++ b/drivers/net/vxlan.c 2012-10-03 21:36:10.213805422 -0700
> > @@ -622,12 +622,30 @@ static inline u8 vxlan_ecn_encap(u8 tos,
> > return INET_ECN_encapsulate(tos, inner);
> > }
> >
>
> #include <linux/jhash.h>
>
> > +/* Compute hash to use for source port
> > + * first choice to use L4 flow hash since it will spread
> > + * better and maybe available from hardware
> > + * secondary choice is to use jhash on the Ethernet header
> > + * Always returns non-zero value
> > + */
> > +static u16 vxlan_flow_hash(struct sk_buff *skb)
> > +{
> > + u16 hash = skb_get_rxhash(skb);
>
> skb_get_rxhash(skb) returns an u32, that could have low order 16bits
> set
> to 0.
>
> So I would use u32 hash = skb_get_rxhash(skb);
Turns out that 0 is a valid source port if no reply is requested
(per RFC and wikipedia)
^ permalink raw reply
* Re: [patch v3 02/11] inet_diag: pass inet_diag module to netlink_dump_start
From: Eric Dumazet @ 2012-10-04 5:08 UTC (permalink / raw)
To: Gao feng
Cc: davem, steffen.klassert, netfilter-devel, linux-rdma, netdev,
linux-crypto, pablo, stephen.hemminger, jengelh
In-Reply-To: <1349325689-16104-2-git-send-email-gaofeng@cn.fujitsu.com>
On Thu, 2012-10-04 at 12:41 +0800, Gao feng wrote:
> set netlink_dump_control.module to avoid panic.
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
> net/ipv4/inet_diag.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index 535584c..5ffd7bc 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -981,6 +981,7 @@ static int inet_diag_rcv_msg_compat(struct sk_buff *skb, struct nlmsghdr *nlh)
> {
> struct netlink_dump_control c = {
> .dump = inet_diag_dump_compat,
> + .module = THIS_MODULE,
> };
> return netlink_dump_start(net->diag_nlsk, skb, nlh, &c);
> }
> @@ -1010,6 +1011,7 @@ static int inet_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
> {
> struct netlink_dump_control c = {
> .dump = inet_diag_dump,
> + .module = THIS_MODULE,
> };
> return netlink_dump_start(net->diag_nlsk, skb, h, &c);
> }
I believe Pablo suggestion was to make netlink_dump_start()
automatically pass THIS_MODULE so that we dont need to change all call
sites ?
extern int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
struct netlink_dump_control *control);
static inline int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
struct netlink_dump_control *control)
{
control->module = THIS_MODULE;
return __netlink_dump_start(ssk, skb, nlh, control);
}
or :
extern int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
struct netlink_dump_control *control,
struct module *module);
static inline int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
struct netlink_dump_control *control)
{
return __netlink_dump_start(ssk, skb, nlh, control, THIS_MODULE);
}
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-04 5:17 UTC (permalink / raw)
To: Anthony Liguori, Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87sj9vxbnf.fsf@codemonkey.ws>
Anthony Liguori <anthony@codemonkey.ws> writes:
> Rusty Russell <rusty@rustcorp.com.au> writes:
>
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>
>> There's a reason I haven't done this. I really, really dislike "my
>> implemention isn't broken" feature bits. We could have an infinite
>> number of them, for each bug in each device.
>>
>> So my plan was to tie this assumption to the new PCI layout. And have a
>> stress-testing patch like the one below in the kernel (see my virtio-wip
>> branch for stuff like this). Turn it on at boot with
>> "virtio_ring.torture" on the kernel commandline.
>>
>> BTW, I've fixed lguest, but my kvm here (Ubuntu precise, kvm-qemu 1.0)
>> is too old. Building the latest git now...
>>
>> Cheers,
>> Rusty.
>>
>> Subject: virtio: CONFIG_VIRTIO_DEVICE_TORTURE
>>
>> Virtio devices are not supposed to depend on the framing of the scatter-gather
>> lists, but various implementations did. Safeguard this in future by adding
>> an option to deliberately create perverse descriptors.
>>
>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> Ignore framing is really a bad idea. You want backends to enforce
> reasonable framing because guest's shouldn't do silly things with framing.
>
> For instance, with virtio-blk, if you want decent performance, you
> absolutely want to avoid bouncing the data. If you're using O_DIRECT in
> the host to submit I/O requests, then it's critical that all of the s/g
> elements are aligned to a sector boundary and sized to a sector
> boundary.
>
> Yes, QEMU can handle if that's not the case, but it would be insanely
> stupid for a guest not to do this. This is the sort of thing that ought
> to be enforced in the specification because a guest cannot perform well
> if it doesn't follow these rules.
Lack of imagination is what got us into trouble in the first place; when
presented with one counter-example, it's useful to look for others.
That's our job, not to dismiss them a "insanely stupid".
For example:
1) Perhaps the guest isn't trying to perform well, it's trying to be a
tiny bootloader?
2) Perhaps the guest is the direct consumer, and aligning buffers is
redundant.
> A spec isn't terribly useful if the result is guest drivers that are
> slow. There's very little to gain by not enforcing rules around framing
> and there's a lot to lose if a guest frames incorrectly.
The guest has the flexibility, and gets to decide. The spec is not
forcing them to perform badly.
> In the rare case where we want to make a framing change, we should use
> feature bits like Michael is proposing.
>
> In this case, we should simply say that with the feature bit, the vnet
> header can be in the same element as the data but not allow the header
> to be spread across multiple elements.
I'd love to split struct virtio_net_hdr_mrg_rxbuf, so the num_buffers
ends up somewhere else.
The simplest rules are "never" or "always".
Cheers,
Rusty.
PS. Inserting zero-length buffers is something I'd be prepared to rule
out, my current patch does it just for yuks...
^ permalink raw reply
* Re: [PATCH 3/3] vxlan: virtual extensible lan
From: Eric Dumazet @ 2012-10-04 6:34 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jesse Gross, davem, netdev
In-Reply-To: <20121003210220.5e74aa5d@nehalam.linuxnetplumber.net>
On Wed, 2012-10-03 at 21:02 -0700, Stephen Hemminger wrote:
>
> Since port is always a pair, there is no need to restrict range, unless
> there was a broken firewall in the way. One bug there is that random32()
> can return 0 which is not a valid port number. A better fallback
> would be a hash of the MAC header.
But using up to 65536 values for the port means that if vxlan traffic
crosses a statefull firewall (eg netfilter with conntrack), this might
need 65536 contexts/sessions.
So random32() is not needed.
What might be needed is additional parameters for a vxlan tunnel, to
give an optional range for the source port.
^ permalink raw reply
* IPVS-DR problem with neigh lookup in 3.6
From: Julian Anastasov @ 2012-10-04 6:52 UTC (permalink / raw)
To: netdev
Hello,
I suspect commit a263b3093641fb1ec377582c90986a7fd0625184
is causing problem for the IPVS Direct Routing mode which
requests output route for real server IP (RIP) but packets
contain virtual IP (VIP). Before 3.6 ip_finish_output2 used
the neighbour address while now we use iph daddr. The
result is that now we send ARP query for VIP instead of RIP.
Is it a good idea to keep rt_gateway always
valid and to check RTCF_REDIRECTED where needed?
While checking this option I see that the value of
0 is not good for matching, for example, callforward_do_filter
in net/netfilter/nf_conntrack_h323_main.c will fail
too. If we do such change there is a chance some rt_gateway
checks to disappear, other will be replaced with check for
RTCF_REDIRECTED flag. Should I try such option?
Because adding rt_dst does not look good, it will not
help to callforward_do_filter too.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: IPVS-DR problem with neigh lookup in 3.6
From: David Miller @ 2012-10-04 6:58 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1210040923010.1618@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Thu, 4 Oct 2012 09:52:47 +0300 (EEST)
> Is it a good idea to keep rt_gateway always
> valid and to check RTCF_REDIRECTED where needed?
What exactly do you mean by this? rt_gateway can only
be explicit nexthop or zero for local subnet.
It cannot take on any other value, otherwise routes are
not properly sharable.
> Because adding rt_dst does not look good, it will not
> help to callforward_do_filter too.
Adding rt_dst is not to be seriously considered.
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Paolo Bonzini @ 2012-10-04 7:09 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, Thomas Lendacky, kvm, netdev, linux-kernel,
virtualization, avi, Sasha Levin
In-Reply-To: <87bogj2j1b.fsf@rustcorp.com.au>
Il 04/10/2012 02:11, Rusty Russell ha scritto:
> > > There's a reason I haven't done this. I really, really dislike "my
> > > implemention isn't broken" feature bits. We could have an infinite
> > > number of them, for each bug in each device.
> >
> > However, this bug affects (almost) all implementations and (almost) all
> > devices. It even makes sense to reserve a transport feature bit for it
> > instead of a device feature bit.
>
> Perhaps, but we have to fix the bugs first!
Yes. :) Isn't that what mst's patch does?
> As I said, my torture patch broke qemu immediately. Since noone has
> leapt onto fixing that, I'll take a look now...
I can look at virtio-scsi.
Paolo
^ permalink raw reply
* Good News---Please Reply Now.
From: roberta.miller @ 2012-10-04 4:57 UTC (permalink / raw)
Greeting, I want you to stand as next of kin so that we can transfer very
huge amount of money and I have all the legal documents to back it up.
Regards,
Barrister Ulrich Claypole.
^ permalink raw reply
* Re: [RFC PATCH 1/2] sctp: fix a typo in prototype of __sctp_rcv_lookup()
From: Nicolas Dichtel @ 2012-10-04 7:17 UTC (permalink / raw)
To: David Miller; +Cc: linux-sctp, vyasevich, netdev
In-Reply-To: <20121003.162847.312544751568068374.davem@davemloft.net>
Le 03/10/2012 22:28, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Wed, 3 Oct 2012 17:43:21 +0200
>
>> Just to avoid confusion when people only reads this prototype.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> I think we should apply this one, regardless of what happens
> to patch #2 in this series.
>
Yes, I catch it when working on the second patch, but I should send two seperate
patches.
^ permalink raw reply
* Re: [patch net] sky2: fix rx filter setup on link up
From: Jiri Pirko @ 2012-10-04 7:40 UTC (permalink / raw)
To: Mirko Lindner
Cc: Stephen Hemminger, netdev@vger.kernel.org, davem@davemloft.net,
linux-kernel@vger.kernel.org
In-Reply-To: <175CCF5F49938B4D99B2E3EF7F558EBE1B63898F0B@SC-VEXCH4.marvell.com>
Tue, Sep 18, 2012 at 02:38:52AM CEST, mlindner@marvell.com wrote:
>>Mon, Sep 17, 2012 at 06:12:14PM CEST, shemminger@vyatta.com wrote:
>>>On Mon, 17 Sep 2012 17:10:17 +0200
>>>Jiri Pirko <jiri@resnulli.us> wrote:
>>>
>>>> In my case I have following problem. sky2_set_multicast() sets registers
>>>> GM_MC_ADDR_H[1-4] correctly to:
>>>> 0000 0800 0001 0410
>>>> However, when adapter gets link and sky2_link_up() is called, the values
>>>> are for some reason different:
>>>> 0000 0800 0016 0410
>>>
>>>Rather than papering over the problem, it would be better to
>>>trace back what is setting those registers and fix that code.
>
>>Yes, I did that. No code at sky2.[ch] is writing to this registers other
>>than sky2_set_multicast() and sky2_gmac_reset() (I hooked on sky2_write*()).
>>So I strongly believe this is a HW issue (maybe only issue of my revision
>>"Yukon-2 EC chip revision 2")
>
>I would like to check the registers as soon as I'm back in my office next week and report my findings.
>Could you also please check the hint from Stephen?
Mirko, did you have a chance to look at this?
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-04 7:44 UTC (permalink / raw)
To: Anthony Liguori, Michael S. Tsirkin, Thomas Lendacky
Cc: Sasha Levin, virtualization, linux-kernel, avi, kvm, netdev
In-Reply-To: <87wqz6kgg4.fsf@codemonkey.ws>
Anthony Liguori <anthony@codemonkey.ws> writes:
>> lguest fix is pending in my queue. lkvm and qemu are broken; lkvm isn't
>> ever going to be merged, so I'm not sure what its status is? But I'm
>> determined to fix qemu, and hence my torture patch to make sure this
>> doesn't creep in again.
>
> There are even more implementations out there and I'd wager they all
> rely on framing.
Worse, both virtio_blk (for scsi commands) and virtio_scsi explicitly
and inescapably rely on framing. The spec conflicts clearly with
itself.
Such layering violations are always a mistake, but I can't blame anyone
else for my lack of attention :(
Here's the spec change:
commit 7e74459bb966ccbaad9e4bf361d1178b7f400b79
Author: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu Oct 4 17:11:27 2012 +0930
No longer assume framing is independent of messages. *sniff*
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
--- virtio-spec.txt 2012-10-04 17:13:04.988259234 +0930
+++ virtio-spec.txt.new 2012-10-04 17:12:54.624258969 +0930
@@ -880,19 +880,19 @@
Message Framing
-The descriptors used for a buffer should not effect the semantics
-of the message, except for the total length of the buffer. For
-example, a network buffer consists of a 10 byte header followed
-by the network packet. Whether this is presented in the ring
-descriptor chain as (say) a 10 byte buffer and a 1514 byte
-buffer, or a single 1524 byte buffer, or even three buffers,
-should have no effect.
+Unless stated otherwise, it is expected that headers within a
+message are contained within their own descriptors. For example,
+a network buffer consists of a 10 or 12 byte header followed by
+the network packet. An implementation should expect that this
+header will be within the first descriptor, and that the
+remainder of the data will begin on the second descriptor.
-In particular, no implementation should use the descriptor
-boundaries to determine the size of any header in a request.[footnote:
-The current qemu device implementations mistakenly insist that
-the first descriptor cover the header in these cases exactly, so
-a cautious driver should arrange it so.
+[footnote:
+It was previously asserted that framing should be independent of
+message contents, yet invariably drivers layed out messages in
+reliable ways and devices assumed it. In addition, the
+specifications for virtio_blk and virtio_scsi require intuiting
+field lengths from frame boundaries.
]
Device Improvements
^ permalink raw reply
* Re: [patch v3 02/11] inet_diag: pass inet_diag module to netlink_dump_start
From: Gao feng @ 2012-10-04 8:03 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, steffen.klassert, netfilter-devel, linux-rdma, netdev,
linux-crypto, pablo, stephen.hemminger, jengelh
In-Reply-To: <1349327316.16011.11.camel@edumazet-glaptop>
于 2012年10月04日 13:08, Eric Dumazet 写道:
> I believe Pablo suggestion was to make netlink_dump_start()
> automatically pass THIS_MODULE so that we dont need to change all call
> sites ?
Yes, since you and Pablo both think this should be better,
I will resend this patchset.
Thanks!
^ permalink raw reply
* Question: Proper place for port details?
From: Stefan Raspl @ 2012-10-04 8:28 UTC (permalink / raw)
To: netdev
Some network device drivers are capable of collecting further
information on the link partner, e.g. reporting details on the port
in the adjacent switch. Is there a good place to make these
available outside of the kernel? ethtool gives a bit of information
like the link partner's speed or duplex mode. But is adding further
information about the link partner to ethtool desired, or is there a
better place somewhere else?
Ciao,
Stefan
^ permalink raw reply
* [PATCH] bonding: set qdisc_tx_busylock to avoid LOCKDEP splat
From: Eric Dumazet @ 2012-10-04 9:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
From: Eric Dumazet <edumazet@google.com>
If a qdisc is installed on a bonding device, its possible to get
following lockdep splat under stress :
=============================================
[ INFO: possible recursive locking detected ]
3.6.0+ #211 Not tainted
---------------------------------------------
ping/4876 is trying to acquire lock:
(dev->qdisc_tx_busylock ?: &qdisc_tx_busylock){+.-...}, at: [<ffffffff8157a191>] dev_queue_xmit+0xe1/0x830
but task is already holding lock:
(dev->qdisc_tx_busylock ?: &qdisc_tx_busylock){+.-...}, at: [<ffffffff8157a191>] dev_queue_xmit+0xe1/0x830
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(dev->qdisc_tx_busylock ?: &qdisc_tx_busylock);
lock(dev->qdisc_tx_busylock ?: &qdisc_tx_busylock);
*** DEADLOCK ***
May be due to missing lock nesting notation
6 locks held by ping/4876:
#0: (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff815e5030>] raw_sendmsg+0x600/0xc30
#1: (rcu_read_lock_bh){.+....}, at: [<ffffffff815ba4bd>] ip_finish_output+0x12d/0x870
#2: (rcu_read_lock_bh){.+....}, at: [<ffffffff8157a0b0>] dev_queue_xmit+0x0/0x830
#3: (dev->qdisc_tx_busylock ?: &qdisc_tx_busylock){+.-...}, at: [<ffffffff8157a191>] dev_queue_xmit+0xe1/0x830
#4: (&bond->lock){++.?..}, at: [<ffffffffa02128c1>] bond_start_xmit+0x31/0x4b0 [bonding]
#5: (rcu_read_lock_bh){.+....}, at: [<ffffffff8157a0b0>] dev_queue_xmit+0x0/0x830
stack backtrace:
Pid: 4876, comm: ping Not tainted 3.6.0+ #211
Call Trace:
[<ffffffff810a0145>] __lock_acquire+0x715/0x1b80
[<ffffffff810a256b>] ? mark_held_locks+0x9b/0x100
[<ffffffff810a1bf2>] lock_acquire+0x92/0x1d0
[<ffffffff8157a191>] ? dev_queue_xmit+0xe1/0x830
[<ffffffff81726b7c>] _raw_spin_lock+0x3c/0x50
[<ffffffff8157a191>] ? dev_queue_xmit+0xe1/0x830
[<ffffffff8106264d>] ? rcu_read_lock_bh_held+0x5d/0x90
[<ffffffff8157a191>] dev_queue_xmit+0xe1/0x830
[<ffffffff8157a0b0>] ? netdev_pick_tx+0x570/0x570
[<ffffffffa0212a6a>] bond_start_xmit+0x1da/0x4b0 [bonding]
[<ffffffff815796d0>] dev_hard_start_xmit+0x240/0x6b0
[<ffffffff81597c6e>] sch_direct_xmit+0xfe/0x2a0
[<ffffffff8157a249>] dev_queue_xmit+0x199/0x830
[<ffffffff8157a0b0>] ? netdev_pick_tx+0x570/0x570
[<ffffffff815ba96f>] ip_finish_output+0x5df/0x870
[<ffffffff815ba4bd>] ? ip_finish_output+0x12d/0x870
[<ffffffff815bb964>] ip_output+0x54/0xf0
[<ffffffff815bad48>] ip_local_out+0x28/0x90
[<ffffffff815bc444>] ip_send_skb+0x14/0x50
[<ffffffff815bc4b2>] ip_push_pending_frames+0x32/0x40
[<ffffffff815e536a>] raw_sendmsg+0x93a/0xc30
[<ffffffff8128d570>] ? selinux_file_send_sigiotask+0x1f0/0x1f0
[<ffffffff8109ddb4>] ? __lock_is_held+0x54/0x80
[<ffffffff815f6730>] ? inet_recvmsg+0x220/0x220
[<ffffffff8109ddb4>] ? __lock_is_held+0x54/0x80
[<ffffffff815f6855>] inet_sendmsg+0x125/0x240
[<ffffffff815f6730>] ? inet_recvmsg+0x220/0x220
[<ffffffff8155cddb>] sock_sendmsg+0xab/0xe0
[<ffffffff810a1650>] ? lock_release_non_nested+0xa0/0x2e0
[<ffffffff810a1650>] ? lock_release_non_nested+0xa0/0x2e0
[<ffffffff8155d18c>] __sys_sendmsg+0x37c/0x390
[<ffffffff81195b2a>] ? fsnotify+0x2ca/0x7e0
[<ffffffff811958e8>] ? fsnotify+0x88/0x7e0
[<ffffffff81361f36>] ? put_ldisc+0x56/0xd0
[<ffffffff8116f98a>] ? fget_light+0x3da/0x510
[<ffffffff8155f6c4>] sys_sendmsg+0x44/0x80
[<ffffffff8172fc22>] system_call_fastpath+0x16/0x1b
Avoid this problem using a distinct lock_class_key for bonding
devices.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/bonding/bond_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7858c58..b721902 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4826,6 +4826,7 @@ static int bond_check_params(struct bond_params *params)
static struct lock_class_key bonding_netdev_xmit_lock_key;
static struct lock_class_key bonding_netdev_addr_lock_key;
+static struct lock_class_key bonding_tx_busylock_key;
static void bond_set_lockdep_class_one(struct net_device *dev,
struct netdev_queue *txq,
@@ -4840,6 +4841,7 @@ static void bond_set_lockdep_class(struct net_device *dev)
lockdep_set_class(&dev->addr_list_lock,
&bonding_netdev_addr_lock_key);
netdev_for_each_tx_queue(dev, bond_set_lockdep_class_one, NULL);
+ dev->qdisc_tx_busylock = &bonding_tx_busylock_key;
}
/*
^ permalink raw reply related
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-04 9:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <20121003092508.6a7da662@nehalam.linuxnetplumber.net>
>
> Thanks for looking into these kind of problems. The contents
> of the patch are correct, but the automated commit message is useless.
> You shouldn't just blindly say what the automated
> script was looking for, you should describe what the bug is so that evaluators
> can decide what the impact is and if it should be backported to stable
> and vendor kernels.
>
> Please resubmit the patchs with a reasonable analysis in the commit message.
> Something like:
>
> There is a bug in skge driver. If alloc_etherdev() fails, then
> skge_devinit() will return NULL, and the skge_probe function incorrectly
> returns success 0. It should return -ENOMEM instead.
>
>
Stephen, I do not want to include function names on the commit
message. What do you think about this updated message, is it
acceptable?
--- --- ---
This patch fixes a bug related to the return value of the function. In some
error cases, the function return non-negative SUCCESS values, when the
correct would be a negative ERROR value.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
{ ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != &ret
*if(...)
{
... when != ret = e2
when forall
return ret;
}
// </smpl>
--- --- ---
Thanks,
Peter
--
Peter
^ permalink raw reply
* Re: IPVS-DR problem with neigh lookup in 3.6
From: Julian Anastasov @ 2012-10-04 9:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20121004.025808.2302904472507419433.davem@davemloft.net>
Hello,
On Thu, 4 Oct 2012, David Miller wrote:
> From: Julian Anastasov <ja@ssi.bg>
> Date: Thu, 4 Oct 2012 09:52:47 +0300 (EEST)
>
> > Is it a good idea to keep rt_gateway always
> > valid and to check RTCF_REDIRECTED where needed?
>
> What exactly do you mean by this? rt_gateway can only
> be explicit nexthop or zero for local subnet.
>
> It cannot take on any other value, otherwise routes are
> not properly sharable.
Oh, yes, that was the reason for value 0.
> > Because adding rt_dst does not look good, it will not
> > help to callforward_do_filter too.
>
> Adding rt_dst is not to be seriously considered.
OK, I'll think more for alternatives. May be
if there is a way to request output route that should
not be cached. Then we can set valid rt_gateway for
such private routes, fnhe can be used for info too
but this route should not be cached there. Or we
should cache the entry in fnhe, so that we can detect
that fnhe is reclaimed.
Currently, we handle NETDEV_UNREGISTER
both for cached and uncached routes, so users like IPVS
can safely request such special routes. IPVS does the
rt caching itself.
Now the question is how to request such host routes
via flowi. Archives remember for FLOWI_FLAG_RT_NOCACHE, may be
we can port it to the new caching as FLOWI_FLAG_RT_HOST?
FLOWI_FLAG_RT_HOST will mean: can cache in fnhe if present
or return uncached result as second option.
BTW, I see another place with problem, ip_forward():
if (opt->is_strictroute && opt->nexthop != rt->rt_gateway)
may be should be
if (opt->is_strictroute && opt->nexthop != rt->rt_gateway &&
rt->rt_gateway)
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* [PATCH] tipc: flow control should not account for sk_rcvbuf
From: erik.hugne @ 2012-10-04 9:14 UTC (permalink / raw)
To: netdev, jon.maloy; +Cc: ying.xue, paul.gortmaker, Erik Hugne
From: Erik Hugne <erik.hugne@ericsson.com>
The TIPC flow control is design around message count, and it should not
account for the sk_rcvbuf when enqueueing messages to the socket
receive queue.
This fixes a problem when the sk_add_backlog fails due to this check
and TIPC_ERR_OVERLOAD is reported back to the sender.
The sender would then drop it's side of the connection only, leaving
a stale connection on the other end.
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/socket.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 09dc5b9..02fed90 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1269,10 +1269,8 @@ static u32 dispatch(struct tipc_port *tport, struct sk_buff *buf)
if (!sock_owned_by_user(sk)) {
res = filter_rcv(sk, buf);
} else {
- if (sk_add_backlog(sk, buf, sk->sk_rcvbuf))
- res = TIPC_ERR_OVERLOAD;
- else
- res = TIPC_OK;
+ __sk_add_backlog(sk, buf);
+ res = TIPC_OK;
}
bh_unlock_sock(sk);
--
1.7.5.4
^ permalink raw reply related
* [PATCH] team: set qdisc_tx_busylock to avoid LOCKDEP splat
From: Eric Dumazet @ 2012-10-04 9:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Jiri Pirko
From: Eric Dumazet <edumazet@google.com>
If a qdisc is installed on a team device, its possible to get
a lockdep splat under stress, because nested dev_queue_xmit() can
lock busylock a second time (on a different device, so its a false
positive)
Avoid this problem using a distinct lock_class_key for team
devices.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 5c7547c..d44cca3 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1315,6 +1315,7 @@ static const struct team_option team_options[] = {
static struct lock_class_key team_netdev_xmit_lock_key;
static struct lock_class_key team_netdev_addr_lock_key;
+static struct lock_class_key team_tx_busylock_key;
static void team_set_lockdep_class_one(struct net_device *dev,
struct netdev_queue *txq,
@@ -1327,6 +1328,7 @@ static void team_set_lockdep_class(struct net_device *dev)
{
lockdep_set_class(&dev->addr_list_lock, &team_netdev_addr_lock_key);
netdev_for_each_tx_queue(dev, team_set_lockdep_class_one, NULL);
+ dev->qdisc_tx_busylock = &team_tx_busylock_key;
}
static int team_init(struct net_device *dev)
^ permalink raw reply related
* Re: [PATCH] tipc: flow control should not account for sk_rcvbuf
From: Eric Dumazet @ 2012-10-04 9:21 UTC (permalink / raw)
To: erik.hugne; +Cc: netdev, jon.maloy, ying.xue, paul.gortmaker
In-Reply-To: <1349342067-27586-1-git-send-email-erik.hugne@ericsson.com>
On Thu, 2012-10-04 at 11:14 +0200, erik.hugne@ericsson.com wrote:
> From: Erik Hugne <erik.hugne@ericsson.com>
>
> The TIPC flow control is design around message count, and it should not
> account for the sk_rcvbuf when enqueueing messages to the socket
> receive queue.
>
> This fixes a problem when the sk_add_backlog fails due to this check
> and TIPC_ERR_OVERLOAD is reported back to the sender.
> The sender would then drop it's side of the connection only, leaving
> a stale connection on the other end.
>
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
> ---
> net/tipc/socket.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 09dc5b9..02fed90 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1269,10 +1269,8 @@ static u32 dispatch(struct tipc_port *tport, struct sk_buff *buf)
> if (!sock_owned_by_user(sk)) {
> res = filter_rcv(sk, buf);
> } else {
> - if (sk_add_backlog(sk, buf, sk->sk_rcvbuf))
> - res = TIPC_ERR_OVERLOAD;
> - else
> - res = TIPC_OK;
> + __sk_add_backlog(sk, buf);
> + res = TIPC_OK;
> }
> bh_unlock_sock(sk);
>
What guarantee do we have this cannot use all kernel memory ?
If sk->sk_rcvbuf is not an acceptable limit here, you must use a
different limit, but not infinity.
^ permalink raw reply
* Re: [PATCH] team: set qdisc_tx_busylock to avoid LOCKDEP splat
From: Jiri Pirko @ 2012-10-04 9:22 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Jiri Pirko
In-Reply-To: <1349342319.16011.30.camel@edumazet-glaptop>
Thu, Oct 04, 2012 at 11:18:39AM CEST, eric.dumazet@gmail.com wrote:
>From: Eric Dumazet <edumazet@google.com>
>
>If a qdisc is installed on a team device, its possible to get
>a lockdep splat under stress, because nested dev_queue_xmit() can
>lock busylock a second time (on a different device, so its a false
>positive)
>
>Avoid this problem using a distinct lock_class_key for team
>devices.
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Jiri Pirko <jpirko@redhat.com>
>---
> drivers/net/team/team.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index 5c7547c..d44cca3 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -1315,6 +1315,7 @@ static const struct team_option team_options[] = {
>
> static struct lock_class_key team_netdev_xmit_lock_key;
> static struct lock_class_key team_netdev_addr_lock_key;
>+static struct lock_class_key team_tx_busylock_key;
>
> static void team_set_lockdep_class_one(struct net_device *dev,
> struct netdev_queue *txq,
>@@ -1327,6 +1328,7 @@ static void team_set_lockdep_class(struct net_device *dev)
> {
> lockdep_set_class(&dev->addr_list_lock, &team_netdev_addr_lock_key);
> netdev_for_each_tx_queue(dev, team_set_lockdep_class_one, NULL);
>+ dev->qdisc_tx_busylock = &team_tx_busylock_key;
> }
>
> static int team_init(struct net_device *dev)
>
>
>--
>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
Thanks Eric.
Acked-by: Jiri Pirko <jiri@resnulli.us>
^ permalink raw reply
* Re: [PATCH 4/20] drivers/net/can/sja1000/peak_pcmcia.c: fix error return code
From: Marc Kleine-Budde @ 2012-10-04 9:30 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: wg, kernel-janitors, linux, linux-can, netdev, linux-kernel
In-Reply-To: <1349281090-10013-3-git-send-email-peter.senna@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 954 bytes --]
On 10/03/2012 06:17 PM, Peter Senna Tschudin wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> Convert a nonnegative error return code to a negative one, as returned
> elsewhere in the function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> (
> if@p1 (\(ret < 0\|ret != 0\))
> { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
> when != &ret
> *if(...)
> {
> ... when != ret = e2
> when forall
> return ret;
> }
> // </smpl>
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Applied to can.
tnx, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox