* netlink, RTM_NEWTCLASS, nested attributes
From: Denys Fedoryshchenko @ 2013-02-19 21:45 UTC (permalink / raw)
To: netdev, pablo, jhs, davem, linux-kernel
Hi
I tried recently to write my own tool based on amazing libmnl (which
makes understanding of netlink - easy), written
by Pablo Neira Ayuso, to manage QoS in Linux and faced problem, which i
think probably
a bug in handling netlink messages in kernel.
For example if i send message, RTM_NEWTCLASS, after attribute
TCA_OPTIONS i have nested attributes,
for example in HTB: TCA_HTB_PARMS, TCA_HTB_RTAB, TCA_HTB_CTAB.
libmnl, if i use nested attribute, adding a bit to it, by OR -
NLA_F_NESTED(1 << 15).
If i remove this flag - everything works fine. And here is the case,
iproute2 tools
just update length of TCA_OPTIONS, without setting flag, and it works
because of that fine too.
So there is basically 3 solutions:
1)New function in libmnl to do nested attributes without setting by OR
flag
2)AND-ing attribute type in kernel to ignore nested flag
3)Keeping as is, who cares?
Thank you for your attention.
---
Denys Fedoryshchenko, Network Engineer, Virtual ISP S.A.L.
^ permalink raw reply
* Re: [PATCH net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Neil Horman @ 2013-02-19 21:44 UTC (permalink / raw)
To: Jon Maloy; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue
In-Reply-To: <5123DDA8.5090202@ericsson.com>
On Tue, Feb 19, 2013 at 09:16:40PM +0100, Jon Maloy wrote:
> On 02/19/2013 08:18 PM, Neil Horman wrote:
> > On Tue, Feb 19, 2013 at 06:54:14PM +0100, Jon Maloy wrote:
> >> On 02/19/2013 03:26 PM, Neil Horman wrote:
> >>> On Tue, Feb 19, 2013 at 09:07:54AM +0100, Jon Maloy wrote:
> >>>> On 02/18/2013 09:47 AM, Neil Horman wrote:
> >>>>> On Fri, Feb 15, 2013 at 05:57:46PM -0500, Paul Gortmaker wrote:
> >>>>>> From: Ying Xue <ying.xue@windriver.com>
><snip>
> >>
> >> There are two reasons for this.
> >> The first one due to the message oriented nature of the flow control for
> >> connections. Max message size is 65k, and max number of unacked messages
> >> (at socket level, that is) before the sending process will take a break
> >> is 1024.
> >> So, simple maths gives that we must allow for 64MB + sk_overhead to guarantee
> >> that a connection never is broken because of receiver overload. Contrary to TCP,
> > Note, this is false, due to the fact that, it presumes that the sender will
> > honor the congestion window. Granted, that would be a sender violation, but its
> > still possible for packets to get lost due to receiver overrun.
>
> The reason for this high limit is exactly to guard against crazy or malevolent
> senders. If they respect their send window they will never hit this limit on
> connections.
>
Nope, You don't get to have it both ways - If a sender is malevolent or crazy,
what makes you think it will respect its send window?
> The fact that
> > you ack packets before accepting them to the receive queue is the problem that
> > needs fixing in this case, but that looks like it can be easily accomplished
> > (see below).
> >
> >> we don't have the luxury to just drop packets and expect them to be retransmitted,
> >> because in TIPC they have already passed through the retransmission and
> >> defragmentation layers, and are committed for delivery when they reach the
> >> receiving socket.
> > The problem isn't that you don't have the luxury of droping packets, the problem
> > is that you've decided to ack an incomming packet before you've determined if
> > you have the space to accept it (I sympathise with you here, I had to go through
> > this same exercise with SCTP a few years ago). Just trying to balance your
> > rcvbuf space so that you don't have to worry about it causes more problems than
> > it solves in the end.
>
> I think you are missing an important point here. There is no one-to-one relation
> between a link and a socket. The link serves *all* socket on a node, with one shared
> send/retransmission queue and one shared packet number sequence. So dropping
> packets on arrival as you suggest will not only hurt the process sending too many
> messages, but everybody else sending anything between the two nodes.
No, I get that, but the fact that TIPC has a shared retransmit queue between all
sockets on the link isn't an excuse to violate the limits set on an individual
socket.
> Granted, the retransmission will take care of the dropped packet, but in the
> meantime no other packets can be delivered through from that link, to any
> socket. Basically, all TIPC communication between the two involved
> nodes in the given direction would grind to a halt until the slow or overwhelmed
> receiver process has decided to work off his receive queue, something that may
> never happen if it is faulty.
Sounds like a bug. That should be fixed.
> You may see this as a flaw, but it is a consequence of that TIPC is a radically
> different design than IP based protocols, designed for completely different needs.
> The shared link concept is a fundamental feature that has a lot of other advantages
> which I won't elaborate on here.
>
Very well. While I'm thinking of it though, you also seem to be making a large
leap in reasoning - you seem to be repeatedly equating my request to have you
honor the limits of sk_rcvbuf, with a desire to have that limit be smaller than
what you currently have it set to. Thats absolutely untrue. I don't care if
you set your limit to UINT_MAX/2 on a socket, I just want what you set to be
honored. If you set your sk_rcvbuf high enough (using setsockopt or the proc
tunables I mentioned below), checking to make sure you're complying with those values
should do nothing from a rx path standpoint (i.e. if you set your rcvbuf high
enough, you will never hit the limit anyway, unless of course you have a
malevolent sender, in which case all bets are off anyway).
> >
> > The thing to do is just move your rcvbuf check lower in the stack, in this case
> > to tipc_recv_msg. You have a tipc_msg struct there, from which you can
> > determine if the message is data, get the desination port, and convert that to a
> > tipc_port pointer, which contains a usr_handle pointer. From my read, the
> > usr_handle pointer is only ever set to a socket structure,
>
> Unfortunately not. There are still remnants of the "native" interface present,
> which is not socket based at all. We are working on that.
>
Ugh, you're right. Well, thats still just two possible types in usr_handle, you
could easily add a boolean flag to mark those ports which had sockets assigned
to them, at least until you cleaned out the native interface
> so if you have a non
> > NULL usr_handle, cast it to a struct sock, and read is sk_rcvbuf. If its over
> > capacity, drop the packet before you ack it.
>
> In theory this is possible, but extremely awkward with the current locking
> structure (We are working on that too). Anyway, I hope I have convinced
> you with the above that dropping packets at the link level is not a viable
> option.
>
The only locking that I see you doing to check the sk_rcvbuf value in
its current location is the tipc_port_lock, and you do that with all the same
locks held farther up the call stack (tipc_net_lock, tipc_node_lock). The fact
of the matter is, moving that check lower down into tipc_recv_msg doesn't
require any nesting changes to your locking at all.
> >
> >> You may question the wisdom of having a message oriented flow control algorithm,
> >> but this the way it is now. We do actually have a working prototype where we
> >> introduce purely byte-based flow control, but it is not ready for delivery yet,
> >> and compatibility constraints will require that we still keep this high limit
> >> in some cases.
> >>
> > I do believe that a byte oriented algorithm is better, but for now a message
> > based mechanism is fine by me. My only concern is that you honor the limits
> > placed on the socket. I'm not sure what compatibilty requirement enjoin you
> > from having a lower rcvbuf limit, but it would seem to me, just not acking a
> > message until you are sure you can accept it would fix those problems. Even if
> > that isn't the case, honor the limits, tie them to the tunables in /proc (or
> > create your own), and document what the required limits are.
> >
> >> The second reason is that TIPC provides SOCK_RDM instead of SOCK_DGRAM as its
> >> basic datagram service. (It is the only transport service doing this afaik.)
> >> So, the risk of having datagram messages rejected (not dropped), for any reason,
> >> must be extremely low. ("Rejected" here means that we notify the sender when a
> >> message cannot be delivered, we don't just silently drop it.)
> > Thats fine, I'm not talking about rejecting messages, I'm specifically referring
> > to dropping them - i.e. behaving as if they never arrived at the host at all
> > (save for maybe increasing a stats counter, so the admin knows whats going on).
> > One way or another the sender has to be prepared to handle an unacked frame. If
> > you can't, then the protocol is broken.
> >
> >> Given that we also often have seen designs with many clients sending towards
> >> one server we have empirical experience that we must have a high threshold here.
> >> One can discuss whether 2MB or 5MB is the adequate limit for the lowest level,
> >> we don't really now since this a new design, but we have every reason to believe
> >> that the upper limit permitted by setsockopt(SOL_SOCK) (425,984 bytes according
> >> to a quick test I made) is not enough for us.
> > I'm not trying to force you to a lower sk_rcvbuf value, I'm fine with whatever
> > you want to set it to, my only request here is that you honor the limits set on
> > your socket at least semi accurately. If you need to set that limit higher by
> > default, do so (theres sysctl values for that already:
> > sysctl_rmem_[min|default|max], or you can build your own, though I would
> > recommend you use the former).
>
> Ok, I wasn't aware of that. Now, if we could set these parameters from inside
> the module, when a socket is created, we I think we have what we need. We
> don't want to force every socket creator to set these limits explicitly, unless
> he has some very particular needs and knows what he is doing.
>
I've tried to explain this several times now. You don't have to have set this value to
what you want programatically, nor do you need to force the value from within
the module code itself, you can do it administratively. If you set:
/proc/sys/net/core/rmem_default
to the value that you want all your sockets to have, any socket that gets
initalized with sock_init_data will inherit that value. Note that, when doing
so, you may also have to set:
/proc/sys/net/core/rmem_max
As you can't adminstratively set your default socket rcvbuf value to something
larger than the maximum allowed value without raising the maximum allowed value
first.
Then all you have to do is make sure those values are set during boot up, and for
users, it will appear as though nothing has changed.
Neil
>
^ permalink raw reply
* Re: Problems finding ethtool source
From: Ben Hutchings @ 2013-02-19 21:18 UTC (permalink / raw)
To: Jeff Garzik, chris packham; +Cc: netdev
In-Reply-To: <51238D03020000680002CB63@gwia.alliedtelesyn.co.nz>
On Tue, 2013-02-19 at 14:32 +1300, chris packham wrote:
> Hi Ben,
>
> Just writing to let you know about the trouble I had locating the source
> tarballs for ethtool.
>
> I didn't have any particular problem I just wanted to make sure the
> version of ethtool we use on our embedded target was compatible with
> our kernel version.
You shouldn't see compatibility issues between differing ethtool and
kernel versions, as the ethtool ABI is intended to be stable. (That's
not to say there aren't bugs.) The ethtool version number is a rough
indicator that it can exercise the features implemented in the
corresponding kernel version.
> Being lazy I headed to google and searched for
> "ethtool source" which took me to the sourceforge.net page with the
> older versions. I actually had to try pretty hard to find the
> http://www.kernel.org/pub/software/network/ethtool/ page (eventually
> got there via debian.org).
http://tinyurl.com/ethtool-home also works.
> Are you (or anyone on netdev) in contact with Jeff Garzik? If so could you
> ask for a link to the newer versions to be added to the sf.net page, I think
> that'd help others searching for the real upstream.
Jeff, would you mind adding a link to the current ethtool page in or
under the gkernel project description?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC PATCH] ipv6: Split from and expires field in dst_entry out of union [net-next]
From: Eric Dumazet @ 2013-02-19 21:17 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David Miller, Gao feng, Jiri Bohac
In-Reply-To: <1361305694-8303-1-git-send-email-nhorman@tuxdriver.com>
On Tue, 2013-02-19 at 15:28 -0500, Neil Horman wrote:
> static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
> {
> if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> - if (rt->dst.from)
> - dst_release(rt->dst.from);
> + dst_release(rt->dst.from);
> /* dst_set_expires relies on expires == 0
> * if it has not been set previously.
> */
> rt->dst.expires = 0;
> + rt6->dst.from = NULL;
> }
>
Sorry you didnt really address the problem, only reduce the race window.
^ permalink raw reply
* Re: Upcoming cross-tree build breakage on merge window
From: John Crispin @ 2013-02-19 20:45 UTC (permalink / raw)
To: Rafał Miłecki
Cc: ralf, David S. Miller, linux-mips, Network Development,
Hauke Mehrtens
In-Reply-To: <CACna6rwYZWsGhb8ksko+XGvod=hVyVHu2QrCfEisTG=YAEfBRQ@mail.gmail.com>
On 19/02/13 20:44, Rafał Miłecki wrote:
> 2013/1/23 Rafał Miłecki<zajec5@gmail.com>:
>> I've noticed possible build breakage when two trees get merged:
>> net-next and linux-john (MIPS).
>>
>> This is about two following commits:
>> http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commit;h=dd4544f05469aaaeee891d7dc54d66430344321e
>> http://git.linux-mips.org/?p=john/linux-john.git;a=commit;h=a008ca117bc85a9d66c47cd5ab18a6c332411919
>>
>> The first one adds "bgmac" driver which uses asm/mach-bcm47xx/nvram.h
>> and nvram_getenv. The second one renames them.
>>
>> Can you handle this in some clever way during merge window, please?
>>
>> The fix is trivial:
>> 1) Use<bcm47xx_nvram.h>
>> 2) Use bcm47xx_nvram_getenv
>
> Just a reminder.
>
Hi,
Ralf told me he will pull the fix into his upstream-sfr.git today so
that the upcoming linux-next should not build break due to this patch
John
^ permalink raw reply
* [RFC PATCH] ipv6: Split from and expires field in dst_entry out of union [net-next]
From: Neil Horman @ 2013-02-19 20:28 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, eric.dumazet, David Miller, Gao feng, Jiri Bohac
In-Reply-To: <1361231718.19353.117.camel@edumazet-glaptop>
I've been looking into some random ipv6 crashes lately that occur around
ip6_link_failure. This and simmilar partial stack traces:
ip6_link_failure+0xbe/0xd0
ipip6_tunnel_xmit+0x7e7/0x860 [sit]
dev_hard_start_xmit+0x3e3/0x690
dev_queue_xmit+0x38f/0x610
neigh_direct_output+0x11/0x20
ip6_finish_output2+0x90/0x340
? ac6_proc_exit+0x20/0x20
ip6_finish_output+0x98/0xc0
ip6_output
? __ip6_local_out
ip6_local_out
ip6_push_pending
? ip6_append_data
udp_v6_push_pending_frames
udpv6_sendmsg
were all I had. Eric D. Made this click with me this morning however, noting a
possible/likely race condition in the ipv6 code. It appears that, if a dst
entry is accessed by multiple cpus (and it appears that certainly can happen, as
routes created via ip6_rt_copy are hashed back into the fib, for future
lookups), then the use of the rt6i_flags field can race, leading to multiple
conflicting uses of the aforementioned union (jiffies being interpreted as the
from pointer and vice versa).
Eric suggested that this be fixed in rt6_update_expires, but looking at the
other uses of this union I don't think thats a complete fix. All the accessors
for the expired|from union in the dst entry seem to rely on the RTF_EXPIRES flag
being accessed and updated atomically, and thats just not the case.
I think the only fix here, that doesn't involve additional locking, is to
separate the expires and from fields into their own storage, so as not to
trample one another.
This is currently untested, but I've given it to several people who can
reproduce this problem and are testing it now. I'll post again when I have
results from them.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: eric.dumazet@gmail.com
CC: David Miller <davem@davemloft.net>
CC: Gao feng <gaofeng@cn.fujitsu.com>
CC: Jiri Bohac <jbohac@suse.cz>
---
include/net/dst.h | 9 +++------
include/net/ip6_fib.h | 13 ++++++++-----
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index 3da47e0..6b7ebcf 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -36,13 +36,10 @@ struct dst_entry {
struct net_device *dev;
struct dst_ops *ops;
unsigned long _metrics;
- union {
- unsigned long expires;
- /* point to where the dst_entry copied from */
- struct dst_entry *from;
- };
+ unsigned long expires;
+ /* point to where the dst_entry copied from */
+ struct dst_entry *from;
struct dst_entry *path;
- void *__pad0;
#ifdef CONFIG_XFRM
struct xfrm_state *xfrm;
#else
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 6919a50..a285e37 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -164,31 +164,33 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
static inline void rt6_clean_expires(struct rt6_info *rt)
{
- if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+ if (!rt->rt6i_flags & RTF_EXPIRES)
dst_release(rt->dst.from);
rt->rt6i_flags &= ~RTF_EXPIRES;
rt->dst.from = NULL;
+ rt->dst.expires = 0;
}
static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
{
- if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+ if (!rt->rt6i_flags & RTF_EXPIRES)
dst_release(rt->dst.from);
rt->rt6i_flags |= RTF_EXPIRES;
+ rt->dst.from = NULL;
rt->dst.expires = expires;
}
static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
{
if (!(rt->rt6i_flags & RTF_EXPIRES)) {
- if (rt->dst.from)
- dst_release(rt->dst.from);
+ dst_release(rt->dst.from);
/* dst_set_expires relies on expires == 0
* if it has not been set previously.
*/
rt->dst.expires = 0;
+ rt6->dst.from = NULL;
}
dst_set_expires(&rt->dst, timeout);
@@ -199,7 +201,7 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
{
struct dst_entry *new = (struct dst_entry *) from;
- if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+ if (!rt->rt6i_flags & RTF_EXPIRES) {
if (new == rt->dst.from)
return;
dst_release(rt->dst.from);
@@ -207,6 +209,7 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
rt->rt6i_flags &= ~RTF_EXPIRES;
rt->dst.from = new;
+ rt->dst.expires = 0;
dst_hold(new);
}
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH 7/7] netfilter: nf_ct_helper: Fix logging for dropped packets
From: Pablo Neira Ayuso @ 2013-02-19 20:17 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, davem, netfilter-devel
In-Reply-To: <1361260267.5920.15.camel@joe-AO722>
On Mon, Feb 18, 2013 at 11:51:07PM -0800, Joe Perches wrote:
>
> Sorry, I didn't look at the function implementation.
>
> It doesn't use the var args that follow fmt.
> Two current uses have format and args aren't emitted correctly.
Thanks Joe, will pass this to David asap.
Regards.
^ permalink raw reply
* Re: [PATCH net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Jon Maloy @ 2013-02-19 20:16 UTC (permalink / raw)
To: Neil Horman; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue
In-Reply-To: <20130219191833.GB31871@hmsreliant.think-freely.org>
On 02/19/2013 08:18 PM, Neil Horman wrote:
> On Tue, Feb 19, 2013 at 06:54:14PM +0100, Jon Maloy wrote:
>> On 02/19/2013 03:26 PM, Neil Horman wrote:
>>> On Tue, Feb 19, 2013 at 09:07:54AM +0100, Jon Maloy wrote:
>>>> On 02/18/2013 09:47 AM, Neil Horman wrote:
>>>>> On Fri, Feb 15, 2013 at 05:57:46PM -0500, Paul Gortmaker wrote:
>>>>>> From: Ying Xue <ying.xue@windriver.com>
>>>>>>
>>>>>> Change overload control to be purely byte-based, using
>>>>>> sk->sk_rmem_alloc as byte counter, and compare it to a calculated
>>>>>> upper limit for the socket receive queue.
>>>>
>>>> [...]
>>>>
>>>>>> + *
>>>>>> + * For all connectionless messages, by default new queue limits are
>>>>>> + * as belows:
>>>>>> + *
>>>>>> + * TIPC_LOW_IMPORTANCE (5MB)
>>>>>> + * TIPC_MEDIUM_IMPORTANCE (10MB)
>>>>>> + * TIPC_HIGH_IMPORTANCE (20MB)
>>>>>> + * TIPC_CRITICAL_IMPORTANCE (40MB)
>>>>>> + *
>>>>>> + * Returns overload limit according to corresponding message importance
>>>>>> + */
>>>>>> +static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *buf)
>>>>>> +{
>>>>>> + struct tipc_msg *msg = buf_msg(buf);
>>>>>> + unsigned int limit;
>>>>>> +
>>>>>> + if (msg_connected(msg))
>>>>>> + limit = CONN_OVERLOAD_LIMIT;
>>>>> This still strikes me as a bit wierd. If you really can't tolerate the default
>>>>> rmem settings in proc, have you considered separating the rmem and wmem values
>>>>> out into their own sysctls?
>>>>
>>>> Initially we tried to set this value as default for sk_rcvbuf, and then use
>>>> fractions of it as limits, as you suggest below. The problem we found was that
>>>> if we want to change this via setsockopt(SOL_SOCKET, SO_RCVBUF) the value range
>>>> we can use is very limited, and doesn't fit our purposes.
>>>>
>>> Can you elaborate on this please? The above doesn't really explain why you
>>> can't do what I suggested. Not asserting that what you say is untrue, mind you,
>>> I'm just trying to understand what it is about TIPC that requires such a
>>> specific reception buffer envelope,
>>
>> There are two reasons for this.
>> The first one due to the message oriented nature of the flow control for
>> connections. Max message size is 65k, and max number of unacked messages
>> (at socket level, that is) before the sending process will take a break
>> is 1024.
>> So, simple maths gives that we must allow for 64MB + sk_overhead to guarantee
>> that a connection never is broken because of receiver overload. Contrary to TCP,
> Note, this is false, due to the fact that, it presumes that the sender will
> honor the congestion window. Granted, that would be a sender violation, but its
> still possible for packets to get lost due to receiver overrun.
The reason for this high limit is exactly to guard against crazy or malevolent
senders. If they respect their send window they will never hit this limit on
connections.
The fact that
> you ack packets before accepting them to the receive queue is the problem that
> needs fixing in this case, but that looks like it can be easily accomplished
> (see below).
>
>> we don't have the luxury to just drop packets and expect them to be retransmitted,
>> because in TIPC they have already passed through the retransmission and
>> defragmentation layers, and are committed for delivery when they reach the
>> receiving socket.
> The problem isn't that you don't have the luxury of droping packets, the problem
> is that you've decided to ack an incomming packet before you've determined if
> you have the space to accept it (I sympathise with you here, I had to go through
> this same exercise with SCTP a few years ago). Just trying to balance your
> rcvbuf space so that you don't have to worry about it causes more problems than
> it solves in the end.
I think you are missing an important point here. There is no one-to-one relation
between a link and a socket. The link serves *all* socket on a node, with one shared
send/retransmission queue and one shared packet number sequence. So dropping
packets on arrival as you suggest will not only hurt the process sending too many
messages, but everybody else sending anything between the two nodes.
Granted, the retransmission will take care of the dropped packet, but in the
meantime no other packets can be delivered through from that link, to any
socket. Basically, all TIPC communication between the two involved
nodes in the given direction would grind to a halt until the slow or overwhelmed
receiver process has decided to work off his receive queue, something that may
never happen if it is faulty.
You may see this as a flaw, but it is a consequence of that TIPC is a radically
different design than IP based protocols, designed for completely different needs.
The shared link concept is a fundamental feature that has a lot of other advantages
which I won't elaborate on here.
>
> The thing to do is just move your rcvbuf check lower in the stack, in this case
> to tipc_recv_msg. You have a tipc_msg struct there, from which you can
> determine if the message is data, get the desination port, and convert that to a
> tipc_port pointer, which contains a usr_handle pointer. From my read, the
> usr_handle pointer is only ever set to a socket structure,
Unfortunately not. There are still remnants of the "native" interface present,
which is not socket based at all. We are working on that.
so if you have a non
> NULL usr_handle, cast it to a struct sock, and read is sk_rcvbuf. If its over
> capacity, drop the packet before you ack it.
In theory this is possible, but extremely awkward with the current locking
structure (We are working on that too). Anyway, I hope I have convinced
you with the above that dropping packets at the link level is not a viable
option.
>
>> You may question the wisdom of having a message oriented flow control algorithm,
>> but this the way it is now. We do actually have a working prototype where we
>> introduce purely byte-based flow control, but it is not ready for delivery yet,
>> and compatibility constraints will require that we still keep this high limit
>> in some cases.
>>
> I do believe that a byte oriented algorithm is better, but for now a message
> based mechanism is fine by me. My only concern is that you honor the limits
> placed on the socket. I'm not sure what compatibilty requirement enjoin you
> from having a lower rcvbuf limit, but it would seem to me, just not acking a
> message until you are sure you can accept it would fix those problems. Even if
> that isn't the case, honor the limits, tie them to the tunables in /proc (or
> create your own), and document what the required limits are.
>
>> The second reason is that TIPC provides SOCK_RDM instead of SOCK_DGRAM as its
>> basic datagram service. (It is the only transport service doing this afaik.)
>> So, the risk of having datagram messages rejected (not dropped), for any reason,
>> must be extremely low. ("Rejected" here means that we notify the sender when a
>> message cannot be delivered, we don't just silently drop it.)
> Thats fine, I'm not talking about rejecting messages, I'm specifically referring
> to dropping them - i.e. behaving as if they never arrived at the host at all
> (save for maybe increasing a stats counter, so the admin knows whats going on).
> One way or another the sender has to be prepared to handle an unacked frame. If
> you can't, then the protocol is broken.
>
>> Given that we also often have seen designs with many clients sending towards
>> one server we have empirical experience that we must have a high threshold here.
>> One can discuss whether 2MB or 5MB is the adequate limit for the lowest level,
>> we don't really now since this a new design, but we have every reason to believe
>> that the upper limit permitted by setsockopt(SOL_SOCK) (425,984 bytes according
>> to a quick test I made) is not enough for us.
> I'm not trying to force you to a lower sk_rcvbuf value, I'm fine with whatever
> you want to set it to, my only request here is that you honor the limits set on
> your socket at least semi accurately. If you need to set that limit higher by
> default, do so (theres sysctl values for that already:
> sysctl_rmem_[min|default|max], or you can build your own, though I would
> recommend you use the former).
Ok, I wasn't aware of that. Now, if we could set these parameters from inside
the module, when a socket is created, we I think we have what we need. We
don't want to force every socket creator to set these limits explicitly, unless
he has some very particular needs and knows what he is doing.
///jon
>
> Note also, that the upper limit of sk_rcvbuf isn't 425,984 bytes. That sounds
> more like what you have sysctl_rmem_max set to on your system at the moment.
> Bump that up if you need it higher (do the same with sysctl_rmem_default, for
> apps that need to expect your old buffer size value.
>
>>
>>> and how enforcing queue limits here is so
>>> important when packets could just as easily be dropped at the ip layer (with
>>> ostensibly no fatal failure).
>>
>> There is no IP layer. TIPC and its retransmission layer sits directly on top of
>> L2/Ethernet. Nothing can be dropped below that layer, for obvious reasons,
>> and to drop anything above that layer *has* fatal consequences, because TIPC
>> guarantees delivery both for connection oriented and connectionless (as far as
>> ever possible) services.
> Sorry, I forgot TIPC sat directly on top of L2. Regardless however, as noted
> above, you can certainly still do discards below your ack point in your stack.
>
>> Anyway, only the receiving socket contains the info making it possible to
>> select which messages to reject, in the rare cases where that becomes
>> unavoidable.
>>
> Yup, and you can interrogate that from tipc_recv_msg
>
>>>
>>>> We did consider to introduce a separate setsockopt at TIPC level for this,
>>>> but thought it had a value in itself to use the mechanism that is already there.
>>>> Hence the "re-interpretation" of sk_rcvbuf as we do below.
>>>> Considering the weird doubling of this parameter that is done elsewhere in the
>>>> code we thought that having our own interpretation might be acceptable.
>>> Thats quite different IMHO. The comments in sock_setsockopt make it pretty
>>> clear that the doubling of the rcvbuf value is done to account for the sk_buff
>>> overhead of packet reception, and thats documented in the socket(7) man page.
>>> What you have here is the ability to set sk_rcvbuf, and then have that setting
>>> be ignored, but only to within several different limits, depending on various
>>> conditions, all of which are not visible to user space.
>>>
>>>> We did of course see the potential issue with this, that is why we cc-ed
>>>> you for comments.
>>> I appreciate that.
>>>
>>>> Now I see that David already pulled the series, so I am a little uncertain
>>>> about how to proceed.
>>> I saw that too, and asked him about this. A follow-on patch (if we wind up
>>> deciding one is warranted) is the way to go here.
>>
>> Good. The best solution I see now, if you think the times-32 scaling is
>> unacceptable, would be to introduce a setsockopt() at SOL_TIPC level, and allow
>> for much wider limits than SOL_SOCK permits now. That we need these wider limits
>> is beyond doubt, as I see it.
>>
> No See above, don't do a special socket option. Just use the existing sysctl
> to define your default and upper limits, and if they're not big enough, set them
> higher, you can use /etc/sysctl.conf to automate this.
>
> Neil
>
>> Regards
>> ///jon
>>
>>>
>>> Regards
>>> Neil
>>>
>>
>>
^ permalink raw reply
* Re: [PATCH] b43: Increase number of RX DMA slots
From: Bastian Bittorf @ 2013-02-19 20:01 UTC (permalink / raw)
To: Larry Finger, openwrt-devel; +Cc: netdev, linux-wireless, linville, Stable
In-Reply-To: <51228ACF.3060500@lwfinger.net>
* Larry Finger <Larry.Finger@lwfinger.net> [18.02.2013 21:17]:
> (14e4:4315) units. I think the firmware and DMA can handle it. After
> all, all the TX rings have 256 slots. There is, however, a question
> of the memory. TX only acquires the buffers when needed, but RX has
> to get them in advance, thus 256 slots there will waste a lot of
> memory.
I did some testing with 256 slots and the
"b43-workaround-rx-fifo-overflow.patch" on a very slow board,
an i'am sure, that 128 slots are not enough. kernel-log is here:
http://intercity-vpn.de/files/openwrt/b43test.dmesg.txt
this was a normal download (100mb @ 1 megabyte/s), but if you
do some udp-magic (netperf) you run soon into trouble.
thanks for your work!
bye, bastian / wireless.subsignal.org / weimarnetz e.V.
^ permalink raw reply
* Re: Upcoming cross-tree build breakage on merge window
From: Rafał Miłecki @ 2013-02-19 19:44 UTC (permalink / raw)
To: john, ralf, David S. Miller
Cc: linux-mips, Network Development, Hauke Mehrtens
In-Reply-To: <CACna6ryD3SjLN-oauvVuRa+q7an8DaULj+Uj4bwFSzQf2WCvMw@mail.gmail.com>
2013/1/23 Rafał Miłecki <zajec5@gmail.com>:
> I've noticed possible build breakage when two trees get merged:
> net-next and linux-john (MIPS).
>
> This is about two following commits:
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commit;h=dd4544f05469aaaeee891d7dc54d66430344321e
> http://git.linux-mips.org/?p=john/linux-john.git;a=commit;h=a008ca117bc85a9d66c47cd5ab18a6c332411919
>
> The first one adds "bgmac" driver which uses asm/mach-bcm47xx/nvram.h
> and nvram_getenv. The second one renames them.
>
> Can you handle this in some clever way during merge window, please?
>
> The fix is trivial:
> 1) Use <bcm47xx_nvram.h>
> 2) Use bcm47xx_nvram_getenv
Just a reminder.
--
Rafał
^ permalink raw reply
* Re: PPPOE lockdep report in dev_queue_xmit+0x8b8/0x900
From: David Miller @ 2013-02-19 19:34 UTC (permalink / raw)
To: eric.dumazet; +Cc: yaneti, netdev
In-Reply-To: <1361299323.19353.156.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 19 Feb 2013 10:42:03 -0800
> [PATCH v2] ppp: set qdisc_tx_busylock to avoid LOCKDEP splat
>
> If a qdisc is installed on a ppp 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 ppp
> devices.
>
> Reported-by: Yanko Kaneti <yaneti@declera.com>
> Tested-by: Yanko Kaneti <yaneti@declera.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply
* [PATCH] phy: fix phy_device_free memory leak
From: Petr Malat @ 2013-02-19 19:36 UTC (permalink / raw)
To: netdev; +Cc: Petr Malat
From: Petr Malat <oss@malat.biz>
Fix memory leak in phy_device_free() for the case when phy_device*
returned by phy_device_create() is not registered in the system.
Bug description:
phy_device_create() sets name of kobject using dev_set_name(), which
allocates memory using kvasprintf(), but this memory isn't freed if
the underlying device isn't registered properly, because kobject_cleanup()
is not called in that case. This can happen (and actually is happening on
our machines) if phy_device_register(), called by mdiobus_scan(), fails.
Patch description:
Name is freed by phy_device_free(). In the case a device is released
trough kobject_cleanup()->device_release()->phy_device_release(), the name
is set to NULL and it is not freed by phy_device_free(), because it will
be freed later by kobject_cleanup().
Signed-off-by: Petr Malat <oss@malat.biz>
---
Please put me on CC, I'm not signed into the mailing list.
--- linux-v2.6.32.60.orig/drivers/net/phy/phy_device.c 2013-02-06 19:44:11.000000000 +0100
+++ linux-v2.6.32.60/drivers/net/phy/phy_device.c 2013-02-06 20:56:57.000000000 +0100
@@ -41,12 +41,16 @@ MODULE_LICENSE("GPL");
void phy_device_free(struct phy_device *phydev)
{
+ kfree(phydev->dev.kobj.name);
kfree(phydev);
}
EXPORT_SYMBOL(phy_device_free);
static void phy_device_release(struct device *dev)
{
+ /* Name will be freed by kobject_cleanup() */
+ dev->kobj.name = NULL;
+
phy_device_free(to_phy_device(dev));
}
^ permalink raw reply
* RE: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
From: Dmitry Kravkov @ 2013-02-19 19:31 UTC (permalink / raw)
To: pravin; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <CANvJab2wKiQjZ=ZEuJO1A=jCnja==a0qL8XULSAU2dc6W-rh3A@mail.gmail.com>
> -----Original Message-----
> From: pravin [mailto:pravin.shelar@gmail.com]
> Sent: Tuesday, February 19, 2013 8:40 PM
> To: Dmitry Kravkov
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO
> capability to the tunnel device
>
> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> wrote:
> >
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > ---
> > Changes from v1: fixed email address
> >
> >
> > net/ipv4/ip_gre.c | 10 ++++++++--
> > 1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index cdc31ac..31bc941 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device
> *dev)
> > tunnel->hlen = addend;
> > /* TCP offload with GRE SEQ is not supported. */
> > if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> > - dev->features |= NETIF_F_GSO_SOFTWARE;
> > - dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> > + /* device supports enc gso offload*/
> > + if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> > + dev->features |= NETIF_F_TSO;
> > + dev->hw_features |= NETIF_F_TSO;
> > + } else {
> > + dev->features |= NETIF_F_GSO_SOFTWARE;
> > + dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> > + }
> > }
>
> I am not sure about this change, Are you trying to limit GRE TSO to
> just IPV4 in case of GRE offload in hardware?
>
You're right,
It probably should be fixed to:
if (tdev && tdev->hw_enc_features & NETIF_F_GRE_GSO) {
dev->features |= tdev-> hw_enc_features & NETIF_F_GSO_MASK;
dev->hw_features |= tdev-> hw_enc_features & NETIF_F_GSO_MASK;
> >
> > return mtu;
> > --
> > 1.7.7.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 net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
From: Dmitry Kravkov @ 2013-02-19 19:20 UTC (permalink / raw)
To: pravin; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <CANvJab2uOKaGG6SrJCjvhe39oPqQbLBuc44j_nkShz_NW8G98w@mail.gmail.com>
> -----Original Message-----
> From: pravin [mailto:pravin.shelar@gmail.com]
> Sent: Tuesday, February 19, 2013 8:28 PM
> To: Dmitry Kravkov
> Cc: davem@davemloft.net; netdev@vger.kernel.org
> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
> handle packets
>
> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
> wrote:
> > If device is not able to handle checksumming it will
> > be handled in dev_xmit
> >
> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> > ---
> > Changes from v1: fixed email address
> >
> > net/ipv4/ip_gre.c | 7 ++-----
> > 1 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index a56f118..cdc31ac 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
> *skb)
> > goto error;
> > skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> > return skb;
> > - } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > - err = skb_checksum_help(skb);
> > - if (unlikely(err))
> > - goto error;
> > }
> > - skb->ip_summed = CHECKSUM_NONE;
> > + if (skb->ip_summed != CHECKSUM_PARTIAL)
> > + skb->ip_summed = CHECKSUM_NONE;
> >
> > return skb;
> >
> > --
> > 1.7.7.2
> >
> >
>
> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
> complete IP packet to checksum entire GRE payload.
Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
This is the only problematic case, right?
> > --
> > 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 net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Neil Horman @ 2013-02-19 19:18 UTC (permalink / raw)
To: Jon Maloy; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue
In-Reply-To: <5123BC46.40909@ericsson.com>
On Tue, Feb 19, 2013 at 06:54:14PM +0100, Jon Maloy wrote:
> On 02/19/2013 03:26 PM, Neil Horman wrote:
> > On Tue, Feb 19, 2013 at 09:07:54AM +0100, Jon Maloy wrote:
> >> On 02/18/2013 09:47 AM, Neil Horman wrote:
> >>> On Fri, Feb 15, 2013 at 05:57:46PM -0500, Paul Gortmaker wrote:
> >>>> From: Ying Xue <ying.xue@windriver.com>
> >>>>
> >>>> Change overload control to be purely byte-based, using
> >>>> sk->sk_rmem_alloc as byte counter, and compare it to a calculated
> >>>> upper limit for the socket receive queue.
> >>
> >> [...]
> >>
> >>>> + *
> >>>> + * For all connectionless messages, by default new queue limits are
> >>>> + * as belows:
> >>>> + *
> >>>> + * TIPC_LOW_IMPORTANCE (5MB)
> >>>> + * TIPC_MEDIUM_IMPORTANCE (10MB)
> >>>> + * TIPC_HIGH_IMPORTANCE (20MB)
> >>>> + * TIPC_CRITICAL_IMPORTANCE (40MB)
> >>>> + *
> >>>> + * Returns overload limit according to corresponding message importance
> >>>> + */
> >>>> +static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *buf)
> >>>> +{
> >>>> + struct tipc_msg *msg = buf_msg(buf);
> >>>> + unsigned int limit;
> >>>> +
> >>>> + if (msg_connected(msg))
> >>>> + limit = CONN_OVERLOAD_LIMIT;
> >>> This still strikes me as a bit wierd. If you really can't tolerate the default
> >>> rmem settings in proc, have you considered separating the rmem and wmem values
> >>> out into their own sysctls?
> >>
> >> Initially we tried to set this value as default for sk_rcvbuf, and then use
> >> fractions of it as limits, as you suggest below. The problem we found was that
> >> if we want to change this via setsockopt(SOL_SOCKET, SO_RCVBUF) the value range
> >> we can use is very limited, and doesn't fit our purposes.
> >>
> > Can you elaborate on this please? The above doesn't really explain why you
> > can't do what I suggested. Not asserting that what you say is untrue, mind you,
> > I'm just trying to understand what it is about TIPC that requires such a
> > specific reception buffer envelope,
>
> There are two reasons for this.
> The first one due to the message oriented nature of the flow control for
> connections. Max message size is 65k, and max number of unacked messages
> (at socket level, that is) before the sending process will take a break
> is 1024.
> So, simple maths gives that we must allow for 64MB + sk_overhead to guarantee
> that a connection never is broken because of receiver overload. Contrary to TCP,
Note, this is false, due to the fact that, it presumes that the sender will
honor the congestion window. Granted, that would be a sender violation, but its
still possible for packets to get lost due to receiver overrun. The fact that
you ack packets before accepting them to the receive queue is the problem that
needs fixing in this case, but that looks like it can be easily accomplished
(see below).
> we don't have the luxury to just drop packets and expect them to be retransmitted,
> because in TIPC they have already passed through the retransmission and
> defragmentation layers, and are committed for delivery when they reach the
> receiving socket.
The problem isn't that you don't have the luxury of droping packets, the problem
is that you've decided to ack an incomming packet before you've determined if
you have the space to accept it (I sympathise with you here, I had to go through
this same exercise with SCTP a few years ago). Just trying to balance your
rcvbuf space so that you don't have to worry about it causes more problems than
it solves in the end.
The thing to do is just move your rcvbuf check lower in the stack, in this case
to tipc_recv_msg. You have a tipc_msg struct there, from which you can
determine if the message is data, get the desination port, and convert that to a
tipc_port pointer, which contains a usr_handle pointer. From my read, the
usr_handle pointer is only ever set to a socket structure, so if you have a non
NULL usr_handle, cast it to a struct sock, and read is sk_rcvbuf. If its over
capacity, drop the packet before you ack it.
> You may question the wisdom of having a message oriented flow control algorithm,
> but this the way it is now. We do actually have a working prototype where we
> introduce purely byte-based flow control, but it is not ready for delivery yet,
> and compatibility constraints will require that we still keep this high limit
> in some cases.
>
I do believe that a byte oriented algorithm is better, but for now a message
based mechanism is fine by me. My only concern is that you honor the limits
placed on the socket. I'm not sure what compatibilty requirement enjoin you
from having a lower rcvbuf limit, but it would seem to me, just not acking a
message until you are sure you can accept it would fix those problems. Even if
that isn't the case, honor the limits, tie them to the tunables in /proc (or
create your own), and document what the required limits are.
> The second reason is that TIPC provides SOCK_RDM instead of SOCK_DGRAM as its
> basic datagram service. (It is the only transport service doing this afaik.)
> So, the risk of having datagram messages rejected (not dropped), for any reason,
> must be extremely low. ("Rejected" here means that we notify the sender when a
> message cannot be delivered, we don't just silently drop it.)
Thats fine, I'm not talking about rejecting messages, I'm specifically referring
to dropping them - i.e. behaving as if they never arrived at the host at all
(save for maybe increasing a stats counter, so the admin knows whats going on).
One way or another the sender has to be prepared to handle an unacked frame. If
you can't, then the protocol is broken.
> Given that we also often have seen designs with many clients sending towards
> one server we have empirical experience that we must have a high threshold here.
> One can discuss whether 2MB or 5MB is the adequate limit for the lowest level,
> we don't really now since this a new design, but we have every reason to believe
> that the upper limit permitted by setsockopt(SOL_SOCK) (425,984 bytes according
> to a quick test I made) is not enough for us.
I'm not trying to force you to a lower sk_rcvbuf value, I'm fine with whatever
you want to set it to, my only request here is that you honor the limits set on
your socket at least semi accurately. If you need to set that limit higher by
default, do so (theres sysctl values for that already:
sysctl_rmem_[min|default|max], or you can build your own, though I would
recommend you use the former).
Note also, that the upper limit of sk_rcvbuf isn't 425,984 bytes. That sounds
more like what you have sysctl_rmem_max set to on your system at the moment.
Bump that up if you need it higher (do the same with sysctl_rmem_default, for
apps that need to expect your old buffer size value.
>
> > and how enforcing queue limits here is so
> > important when packets could just as easily be dropped at the ip layer (with
> > ostensibly no fatal failure).
>
> There is no IP layer. TIPC and its retransmission layer sits directly on top of
> L2/Ethernet. Nothing can be dropped below that layer, for obvious reasons,
> and to drop anything above that layer *has* fatal consequences, because TIPC
> guarantees delivery both for connection oriented and connectionless (as far as
> ever possible) services.
Sorry, I forgot TIPC sat directly on top of L2. Regardless however, as noted
above, you can certainly still do discards below your ack point in your stack.
> Anyway, only the receiving socket contains the info making it possible to
> select which messages to reject, in the rare cases where that becomes
> unavoidable.
>
Yup, and you can interrogate that from tipc_recv_msg
> >
> >> We did consider to introduce a separate setsockopt at TIPC level for this,
> >> but thought it had a value in itself to use the mechanism that is already there.
> >> Hence the "re-interpretation" of sk_rcvbuf as we do below.
> >> Considering the weird doubling of this parameter that is done elsewhere in the
> >> code we thought that having our own interpretation might be acceptable.
> > Thats quite different IMHO. The comments in sock_setsockopt make it pretty
> > clear that the doubling of the rcvbuf value is done to account for the sk_buff
> > overhead of packet reception, and thats documented in the socket(7) man page.
> > What you have here is the ability to set sk_rcvbuf, and then have that setting
> > be ignored, but only to within several different limits, depending on various
> > conditions, all of which are not visible to user space.
> >
> >> We did of course see the potential issue with this, that is why we cc-ed
> >> you for comments.
> > I appreciate that.
> >
> >> Now I see that David already pulled the series, so I am a little uncertain
> >> about how to proceed.
> > I saw that too, and asked him about this. A follow-on patch (if we wind up
> > deciding one is warranted) is the way to go here.
>
> Good. The best solution I see now, if you think the times-32 scaling is
> unacceptable, would be to introduce a setsockopt() at SOL_TIPC level, and allow
> for much wider limits than SOL_SOCK permits now. That we need these wider limits
> is beyond doubt, as I see it.
>
No See above, don't do a special socket option. Just use the existing sysctl
to define your default and upper limits, and if they're not big enough, set them
higher, you can use /etc/sysctl.conf to automate this.
Neil
> Regards
> ///jon
>
> >
> > Regards
> > Neil
> >
>
>
^ permalink raw reply
* [PATCH 60/81] sctp: refactor sctp_outq_teardown to insure proper re-initalization
From: Herton Ronaldo Krzesinski @ 2013-02-19 18:49 UTC (permalink / raw)
To: linux-kernel, stable, kernel-team
Cc: Jamie Parsons, Neil Horman, netdev, Vlad Yasevich,
David S. Miller
In-Reply-To: <1361299784-8830-1-git-send-email-herton.krzesinski@canonical.com>
3.5.7.6 -stable review patch. If anyone has any objections, please let me know.
------------------
From: Neil Horman <nhorman@tuxdriver.com>
commit 2f94aabd9f6c925d77aecb3ff020f1cc12ed8f86 upstream.
Jamie Parsons reported a problem recently, in which the re-initalization of an
association (The duplicate init case), resulted in a loss of receive window
space. He tracked down the root cause to sctp_outq_teardown, which discarded
all the data on an outq during a re-initalization of the corresponding
association, but never reset the outq->outstanding_data field to zero. I wrote,
and he tested this fix, which does a proper full re-initalization of the outq,
fixing this problem, and hopefully future proofing us from simmilar issues down
the road.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Reported-by: Jamie Parsons <Jamie.Parsons@metaswitch.com>
Tested-by: Jamie Parsons <Jamie.Parsons@metaswitch.com>
CC: Jamie Parsons <Jamie.Parsons@metaswitch.com>
CC: Vlad Yasevich <vyasevich@gmail.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netdev@vger.kernel.org
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
net/sctp/outqueue.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index a0fa19f..0716290 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -223,7 +223,7 @@ void sctp_outq_init(struct sctp_association *asoc, struct sctp_outq *q)
/* Free the outqueue structure and any related pending chunks.
*/
-void sctp_outq_teardown(struct sctp_outq *q)
+static void __sctp_outq_teardown(struct sctp_outq *q)
{
struct sctp_transport *transport;
struct list_head *lchunk, *temp;
@@ -276,8 +276,6 @@ void sctp_outq_teardown(struct sctp_outq *q)
sctp_chunk_free(chunk);
}
- q->error = 0;
-
/* Throw away any leftover control chunks. */
list_for_each_entry_safe(chunk, tmp, &q->control_chunk_list, list) {
list_del_init(&chunk->list);
@@ -285,11 +283,17 @@ void sctp_outq_teardown(struct sctp_outq *q)
}
}
+void sctp_outq_teardown(struct sctp_outq *q)
+{
+ __sctp_outq_teardown(q);
+ sctp_outq_init(q->asoc, q);
+}
+
/* Free the outqueue structure and any related pending chunks. */
void sctp_outq_free(struct sctp_outq *q)
{
/* Throw away leftover chunks. */
- sctp_outq_teardown(q);
+ __sctp_outq_teardown(q);
/* If we were kmalloc()'d, free the memory. */
if (q->malloced)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 48/81] net: calxedaxgmac: throw away overrun frames
From: Herton Ronaldo Krzesinski @ 2013-02-19 18:49 UTC (permalink / raw)
To: linux-kernel, stable, kernel-team; +Cc: netdev, David S. Miller, Rob Herring
In-Reply-To: <1361299784-8830-1-git-send-email-herton.krzesinski@canonical.com>
3.5.7.6 -stable review patch. If anyone has any objections, please let me know.
------------------
From: Rob Herring <rob.herring@calxeda.com>
commit d6fb3be544b46a7611a3373fcaa62b5b0be01888 upstream.
The xgmac driver assumes 1 frame per descriptor. If a frame larger than
the descriptor's buffer size is received, the frame will spill over into
the next descriptor. So check for received frames that span more than one
descriptor and discard them. This prevents a crash if we receive erroneous
large packets.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
drivers/net/ethernet/calxeda/xgmac.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 8b0a0e4..8a3cd87 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -546,6 +546,10 @@ static int desc_get_rx_status(struct xgmac_priv *priv, struct xgmac_dma_desc *p)
return -1;
}
+ /* All frames should fit into a single buffer */
+ if (!(status & RXDESC_FIRST_SEG) || !(status & RXDESC_LAST_SEG))
+ return -1;
+
/* Check if packet has checksum already */
if ((status & RXDESC_FRAME_TYPE) && (status & RXDESC_EXT_STATUS) &&
!(ext_status & RXDESC_IP_PAYLOAD_MASK))
--
1.7.9.5
^ permalink raw reply related
* Re: PPPOE lockdep report in dev_queue_xmit+0x8b8/0x900
From: Eric Dumazet @ 2013-02-19 18:42 UTC (permalink / raw)
To: Yanko Kaneti; +Cc: David Miller, netdev
In-Reply-To: <1361272221.2282.5.camel@d2>
From: Eric Dumazet <edumazet@google.com>
On Tue, 2013-02-19 at 13:10 +0200, Yanko Kaneti wrote:
>
> It looks like it has done the job. I am running a kernel with the patch
> and workload that otherwise inevitably triggers the splat and so far it
> has been quiet.
>
Thanks for testing.
David, there was a typo in the changelog : team should be replaced by
ppp.
[PATCH v2] ppp: set qdisc_tx_busylock to avoid LOCKDEP splat
If a qdisc is installed on a ppp 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 ppp
devices.
Reported-by: Yanko Kaneti <yaneti@declera.com>
Tested-by: Yanko Kaneti <yaneti@declera.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
drivers/net/ppp/ppp_generic.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 4fd754e..3db9131 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -1058,7 +1058,15 @@ ppp_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats64)
return stats64;
}
+static struct lock_class_key ppp_tx_busylock;
+static int ppp_dev_init(struct net_device *dev)
+{
+ dev->qdisc_tx_busylock = &ppp_tx_busylock;
+ return 0;
+}
+
static const struct net_device_ops ppp_netdev_ops = {
+ .ndo_init = ppp_dev_init,
.ndo_start_xmit = ppp_start_xmit,
.ndo_do_ioctl = ppp_net_ioctl,
.ndo_get_stats64 = ppp_get_stats64,
^ permalink raw reply related
* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
From: pravin @ 2013-02-19 18:39 UTC (permalink / raw)
To: Dmitry Kravkov; +Cc: davem, netdev
In-Reply-To: <1361217053-16984-2-git-send-email-dmitry@broadcom.com>
On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> ---
> Changes from v1: fixed email address
>
>
> net/ipv4/ip_gre.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index cdc31ac..31bc941 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
> tunnel->hlen = addend;
> /* TCP offload with GRE SEQ is not supported. */
> if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> - dev->features |= NETIF_F_GSO_SOFTWARE;
> - dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> + /* device supports enc gso offload*/
> + if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> + dev->features |= NETIF_F_TSO;
> + dev->hw_features |= NETIF_F_TSO;
> + } else {
> + dev->features |= NETIF_F_GSO_SOFTWARE;
> + dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> + }
> }
I am not sure about this change, Are you trying to limit GRE TSO to
just IPV4 in case of GRE offload in hardware?
>
> return mtu;
> --
> 1.7.7.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 v5 29/45] x86/xen: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-02-19 18:29 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
rostedt, wangyun, xiaoguangrong, rjw, sbw, fweisbec, linux,
nikunj, linux-pm, linux-arch, linux-arm-kernel, linuxppc-dev,
netdev, linux-doc, linux-kernel
In-Reply-To: <20130219181038.GB18244@phenom.dumpdata.com>
On 02/19/2013 11:40 PM, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 22, 2013 at 01:10:51PM +0530, Srivatsa S. Bhat wrote:
>> Once stop_machine() is gone from the CPU offline path, we won't be able to
>> depend on preempt_disable() or local_irq_disable() to prevent CPUs from
>> going offline from under us.
>>
>> Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going offline,
>> while invoking from atomic context.
>>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> Weird. I see this in the patch but I don't see it in the header?
Meaning, you didn't get this email at all?
> Did you
> explicitly suppress the CC part?
>
No.. I sent the entire patchset to a set of email ids and in addition to
that I CC'ed individual patches to the respective maintainers/lists (the
CC: list in the changelog). I used the --auto knob from stgit to do that.
>
> Anyhow, the patch looks sane enough, thought I need to to run it through
> a test framework just to be on a sure side.
>
Sure, thank you. But you might want to test the v6 that I sent out
yesterday instead of v5. Oh, wait a min, you didn't get the v6 mail also?
Here it is, for your reference:
http://marc.info/?l=linux-kernel&m=136119260122255&w=2
Regards,
Srivatsa S. Bhat
>> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: x86@kernel.org
>> Cc: xen-devel@lists.xensource.com
>> Cc: virtualization@lists.linux-foundation.org
>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>> ---
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
From: pravin @ 2013-02-19 18:28 UTC (permalink / raw)
To: Dmitry Kravkov; +Cc: davem, netdev
In-Reply-To: <1361217053-16984-1-git-send-email-dmitry@broadcom.com>
On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> If device is not able to handle checksumming it will
> be handled in dev_xmit
>
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> ---
> Changes from v1: fixed email address
>
> net/ipv4/ip_gre.c | 7 ++-----
> 1 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index a56f118..cdc31ac 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
> goto error;
> skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> return skb;
> - } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> - err = skb_checksum_help(skb);
> - if (unlikely(err))
> - goto error;
> }
> - skb->ip_summed = CHECKSUM_NONE;
> + if (skb->ip_summed != CHECKSUM_PARTIAL)
> + skb->ip_summed = CHECKSUM_NONE;
>
> return skb;
>
> --
> 1.7.7.2
>
>
This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
complete IP packet to checksum entire GRE payload.
> --
> 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] b43: Increase number of RX DMA slots
From: Larry Finger @ 2013-02-19 18:28 UTC (permalink / raw)
To: David Miller
Cc: David.Laight, linville, linux-wireless, netdev, bittorf, stable
In-Reply-To: <20130219.131553.787630407148880340.davem@davemloft.net>
On 02/19/2013 12:15 PM, David Miller wrote:
> I understand your constraints, but this is a trivially remotely
> DoS'able condition even on slow CPU atom laptops.
>
> Send an "expansive" full sized frame followed by 300 or so 64-byte UDP
> packets --> instant hang.
Thanks for the suggestion for a test. I think the reset solution should survive
with only some packet loss, but we will find out.
Larry
^ permalink raw reply
* Re: [PATCH] atl1c: restore buffer state
From: David Miller @ 2013-02-19 18:19 UTC (permalink / raw)
To: xiong; +Cc: netdev, linux-kernel, qca-linux-team, nic-devel, jwboyer
In-Reply-To: <1361294589-7316-1-git-send-email-xiong@qca.qualcomm.com>
From: xiong <xiong@qca.qualcomm.com>
Date: Wed, 20 Feb 2013 01:23:09 +0800
> in the previous commit : f1f220ea1dda078, the BUSY state of buffer is wrongly
> deleted. this patch just restore it.
>
> Signed-off-by: xiong <xiong@qca.qualcomm.com>
Applied.
^ permalink raw reply
* Re: [Patch] net: fix a build failure when !CONFIG_PROC_FS
From: David Miller @ 2013-02-19 18:19 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, gaofeng
In-Reply-To: <1361278025-22864-1-git-send-email-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue, 19 Feb 2013 20:47:05 +0800
> From: Cong Wang <xiyou.wangcong@gmail.com>
>
> When !CONFIG_PROC_FS dev_mcast_init() is not defined,
> actually we can just merge dev_mcast_init() into
> dev_proc_init().
>
> Reported-by: Gao feng <gaofeng@cn.fujitsu.com>
> Cc: Gao feng <gaofeng@cn.fujitsu.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
I definitely prefer this version of this fix, applied, thanks.
^ permalink raw reply
* Re: [PATCH 3/3] net: ipv4: fix waring -Wunused-variable
From: David Miller @ 2013-02-19 18:19 UTC (permalink / raw)
To: gaofeng; +Cc: netdev
In-Reply-To: <1361270592-19492-3-git-send-email-gaofeng@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Tue, 19 Feb 2013 18:43:12 +0800
> the vars ip_rt_gc_timeout is used only when
> CONFIG_SYSCTL is selected.
>
> move these vars into CONFIG_SYSCTL.
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Applied.
^ 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