Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Jim Baxter @ 2013-02-21 17:07 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CAHrpEqQMZNmzRrNQK60hokR2r5S=U65YdC-5h6aHHHzAVdEWKw@mail.gmail.com>

Is the general issue in this driver that the fec_probe function:
request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);

is called before fec_ptp_init() and fec_enet_init() have been called so that 
there is a chance the fec_enet_interrupt can occur before the hardware and data 
structures are fully setup?

^ permalink raw reply

* Your Mailbox Has Exceeded It Quota/Limit®
From: Webmail Administrator @ 2013-02-21 15:48 UTC (permalink / raw)




-- 
Your Mailbox Has Exceeded It Quota/Limit As Set By Your Administration, And You May Not Be Able To Send Or Receive New Mails Until You Re-Validate It. To Re-Validate, Please CLICK:  www.mailservices.at.tf

^ permalink raw reply

* [ANNOUNCE] iproute2 3.8.0
From: Stephen Hemminger @ 2013-02-21 17:00 UTC (permalink / raw)
  To: netdev, linux-kernel

Update to iproute2 utilities to support features in 3.8.0 release
and lots of small spelling and documentation fixes.

iproute2 package is available at:
  http://kernel.org/pub/linux/utils/net/iproute2/iproute2-3.8.0.tar.gz

You can download the source from:
  git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git

Please submit your changes now for features that are going into 3.9 kernel.

Andreas Henriksson (1):
      iproute2: fix tc ematch manpage section

Benjamin Poirier (1):
      Use pkg-config to obtain xtables.h path

Cong Wang (7):
      iproute2: add mdb sub-command to bridge
      iproute2: implement add/del mdb entry
      iproute2: add support to monitor mdb entries too
      iproute2: update usage info of bridge monitor
      bridge: distinguish permanent and temporary mdb entries
      bridge: make `bridge mdb` output consistent with input
      bridge: update help

David L Stevens (1):
      add DOVE extensions for iproute2

David Ward (1):
      ip/iptunnel: Extend TOS syntax

Eric W. Biederman (6):
      iproute2: Don't propogate mounts out of ip
      iproute2: Normalize return codes in "ip netns"
      iproute2: Improve "ip netns add" failure error message
      iproute2: Make "ip netns delete" more likely to succeed
      iproute2: Fill in the ip-netns.8 manpage
      iproute2: Add "ip netns pids" and "ip netns identify"

Jamal Hadi Salim (1):
      iproute2: act_ipt fix xtables breakage

Jan Engelhardt (1):
      build: unbreak linakge of m_xt.so

Johannes Naab (1):
      iproute2: tc netem rate: allow negative packet/cell overhead

Kees van Reeuwijk (6):
      iproute2: clarification of various man8 pages
      iproute2: clarifications in the tc-hfsc.7 man page
      iproute2: add a missing return statement
      iproute2: clarifications in the libnetlink.3 man page
      iproute2: improved error messages
      iproute2: clearer error messages for fifo and tbf qdiscs

Lutz Jaenicke (1):
      rtnl_wilddump_request: fix alignment issue for embedded platforms

Mike Frysinger (6):
      configure: move toolchain init to a function
      lib: include the Config file too
      configure: pull AR from the env too
      add man7 to subdirs list
      do not ignore errors in man subdirs
      ipxfrm: use alloca to allocate stack space

Nicolas Dichtel (15):
      ip/ip6tunnel: fix help for TCLASS
      ip/ip6tunnel: reset encap limit flag on change
      ip/ip6tunnel: fix update of tclass and flowlabel
      ip: add support of netconf messages
      ip: add support of 'ip link type ip6tnl'
      ip: add support of 'ip link type [ipip|sit]'
      ip/link_iptnl: fix indentation Logged in as shemminger
      ip: update mand pages and usage() for 'ip mroute'
      ip: update man pages for 'ip link'
      ip: term OPTIONS was used twice in 'ip route' man pages
      ip: use rtnelink to manage mroute
      ip: add man pages for netconf
      ip: update man pages and usage() for 'ip monitor'
      ipaddr: fix a typo in error msg about SIOCGIFTXQLEN
      iplink: display the value of IFLA_PROMISCUITY

Stephen Hemminger (14):
      Update kernel headers to 3.8-pre
      bridge: add if_bridge.h header
      build: indent shell functions in configure
      configure: restore old behaviour
      bridge: update kernel headers
      README: update mail address and download location
      Makefile: make warnings into errors
      libnetlink: add caveat
      ip: change format of promiscuity display
      ip: make local functions static
      lib: make string arguments const
      Makefile: turn on warnings about missing prototypes
      ip: handle flush with table > 2^31
      v3.8.0

Strake (1):
      include needed files

Zhi Yong Wu (1):
      ip: add the type 'vxlan' in the output of "ip link help"

^ permalink raw reply

* Re: [PATCH net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Jon Maloy @ 2013-02-21 16:54 UTC (permalink / raw)
  To: Neil Horman; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue, Jon Maloy
In-Reply-To: <20130221150746.GA2730@shamino.rdu.redhat.com>

On 02/21/2013 04:07 PM, Neil Horman wrote:
> On Thu, Feb 21, 2013 at 11:24:19AM +0100, Jon Maloy wrote:
>> On 02/19/2013 10:44 PM, Neil Horman wrote:
>>> 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?
>>
>> Nothing. That is why we need this extra security measure that this limit provides.
>> The normal senders will never hit it, the crazy ones will, and have their 
>> connections broken as a consequence. 
>> Sorry I didn't express this clearly enough.
> This isn't a security measure.  If a sender is well
> behaved, then as you say, they won't violate their send window, and will
> ostensibly behave properly (regardless of what limit you set on sk_rcvbuf, be it
> the large one you currently employ, or something bigger or smaller).  If they
> are malicious, then they will hit the limit, and they won't care (because
> they're malicious).  They may have their connections broken, but so what?  Its the
> receiver that needs to be able to protect itself.  The static limit you set using
> this implementation is likely fine for many systems that have sufficient memory,
> but consider a system that is memory constrained. 

The default limit is insanely high of course, but until we have introduced byte 
oriented flow control this is what we have to deal with, for reasons I explained
earlier.

If anybody want to turn it down, it will probably work well in most cases.
The risk of having connections broken rises from 0 to some low percentage. 
If somebody decides to do this, it is probably because he can accept
that risk, or because he knows the apps he is running are nice, e.g. never send 
out streams of 64k messages the way I described as a worst-case scenario.

> If a user administratively
> tunes down sk_rcvbuf to avoid over-consumption of memory on a TIPC socket, that
> condition will be ignored (or at least severly violated).  Thats bad, and may
> lead to memory exhaustion despite having administratively taken action to fix
> it.
> 
[...]

> Clearly sequential packet delivery implies the need to queue and postpone
> application delivery of packets behind the reception of a lost packet in
> sequence if it is dropped.  The problem I'm referring to here is that you seem
> to have built TIPC around the notion of a single sequence namespace per system,
> rather than one per socket.  

This is the way TIPC and other link-oriented protocols, e.g. LINX, are designed, 
and they have this well-known flip-side. They were never intended to become 
another TCP.
Many users accept this, and value that the advantages you get from this design 
more than outweigh the disadvantages. If they don't, then they should use TCP 
or SCTP instead.

Anyway the design doesn't by necessity mean that we are bound to violate the 
system limits; I think we have established that now.

> TCP, employing sequential delivery, means that
> dropping a packet stalls reception, but only for the socket in question.  The
> fact you created TIPC with a design point in which packet loss blocked an entire
> system, rather than only the intended receiving application is the issue here,
> and doing so doesn't give you the freedom to violate other control points in the
> network stack.
> 
>> I could even show that your proposal would cause almost immediate
>> deadlock, due to the order incoming and outgoing data paths are grabbing 
>> the locks. But I would rather drop this part of the discussion; we can 
>> achieve what we want anyway, with much simpler means. See below.
>>
> Perhaps you could, but if you were to deomonstrate how lowering your recieve
> buffer limit (or even just honoring whatever is set), results in a deadlock
> (ostensibly within the TIPC protocol code), how do you justify that as a reason
> why you should be allowed to simply violate your set socket limits, rather than
> recognizing the deadlock as a coding bug that needs to be fixed?  If you like,
> we can begin a separate thread on that subject and look into fixing the issue.

I wouldn't call it a bug, because it doesn't cause deadlock in the current code,
but it is clearly a design that can be improved.
We do have plans and prototypes ready to greatly simplify the locking structure, 
e.g. by getting rid of the port lock and replace most of the other ones with 
RCU-locks.
There are however a few other changes we need to get in place first, notably to
remove the remnants of the native interface, to make that possible. Stay tuned.

> 
>>>
>>>> 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.  
>>

[...]

>> The following is an approach we discussed (inside the team) earlier, but dropped
>> due to the perceived impossibility to set the sk_rcvbuf to the desired values.
>> Now this limitation is not there, as I understand, and we can revive our 
>> proposal.
>>
>> 1: When a TIPC socket is created, sk_rcvbuf is set to 64MB+ (as it was before the 
>>    latest patch). This is the limit used as last resort against connected peers
>>    not respecting the connection send window.
>>
> As I noted above, as long as there is a method to change the administrative
> default, thats fine.
> 
>> 2: Datagram messages are checked against fractions of this value, according to
>>    their importance priority. E.g. LOW->sk_rcvbuf/16,  MEDIUM->sk_rcvbuf/8
>>    HIGH->sk_rcvbuf/4 and CRITICAL->sk_rcvbuf/2.
>>
> Yes, this is what I proposed in my initial email.
> 
>> 3: In the unlikely event that anybody wants to change these limits, he can change
>>    sk_rcvbuf via setsockopt(SOL_SOCK) either by first changing
>>    /proc/sys/net/core/rmem_max as you suggest, or via a dedicated setsockopt(SOL_TIPC).
>>    The latter option would have the advantage that it could enforce a lower limit of
>>    skr_rcvbuf of 64MB, something that would be unthinkable with SOL_SOCK, because it 
>>    would obviously affect other protocols.
> Theres really no need for a private TIPC socket option here.  You do this at the
> socket level, and if you want to set it higher than rmem_max, you adjust
> rmem_max first.  Its just the way the system works.  Every other protocol
> conforms to it, theres no reason TIPC shouldn't also.
>>
>> What do you think?
>>
> Yes, this is for the most part, good.

Happy to hear that. So we'll go ahead and make this change.

Thank you for your feedback.
///jon

> Thanks
> Neil
>> ///jon
>>
>> If you set:
>>>
>>> /proc/sys/net/core/rmem_default
>>
>> But this will 
>>>
>>> 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:
>>>
>>>
>>>
>>> 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

* [PATCH 2/4] sctp: fix association hangs due to reneging packets below the cumulative TSN ACK point
From: Roberts, Lee A. @ 2013-02-21 16:44 UTC (permalink / raw)
  To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

From: Lee A. Roberts <lee.roberts@hp.com>

Resolve SCTP association hangs observed during SCTP stress
testing.  Observable symptoms include communications hangs
with data being held in the association reassembly and/or lobby
(ordering) queues.  Close examination of reassembly queue shows
missing packets.

In sctp_ulpq_renege_list(), do not renege packets below the
cumulative TSN ACK point.

Patch applies to linux-3.8 kernel.

Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
 net/sctp/ulpqueue.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+1/net/sctp/ulpqueue.c linux-3.8-SCTP+2/net/sctp/ulpqueue.c
--- linux-3.8-SCTP+1/net/sctp/ulpqueue.c	2013-02-18 16:58:34.000000000 -0700
+++ linux-3.8-SCTP+2/net/sctp/ulpqueue.c	2013-02-21 07:39:40.888281496 -0700
@@ -969,11 +969,16 @@ static __u16 sctp_ulpq_renege_list(struc
 
 	tsnmap = &ulpq->asoc->peer.tsn_map;
 
-	while ((skb = __skb_dequeue_tail(list)) != NULL) {
-		freed += skb_headlen(skb);
+	while ((skb = skb_peek_tail(list)) != NULL) {
 		event = sctp_skb2event(skb);
 		tsn = event->tsn;
 
+		/* Don't renege below the Cumulative TSN ACK Point. */
+		if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
+			break;
+
+		__skb_unlink(skb, list);
+		freed += skb_headlen(skb);
 		sctp_ulpevent_free(event);
 		sctp_tsnmap_renege(tsnmap, tsn);
 		if (freed >= needed)

^ permalink raw reply

* [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
From: Roberts, Lee A. @ 2013-02-21 16:45 UTC (permalink / raw)
  To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

From: Lee A. Roberts <lee.roberts@hp.com>

Resolve SCTP association hangs observed during SCTP stress
testing.  Observable symptoms include communications hangs
with data being held in the association reassembly and/or lobby
(ordering) queues.  Close examination of reassembly queue shows
missing packets.

In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
correct message reassembly logic for SCTP partial delivery.
Change logic to ensure that as much data as possible is sent
with the initial partial delivery and that following partial
deliveries contain all available data.

In sctp_ulpq_partial_delivery(), attempt partial delivery only
if the data on the head of the reassembly queue is at or before
the cumulative TSN ACK point.

In sctp_ulpq_renege(), adjust logic to enter partial delivery
only if the incoming chunk remains on the reassembly queue
after processing by sctp_ulpq_tail_data().  If the incoming
chunk has been delivered and data remains on the reassembly
queue, attempt to drain the queue.  Remove call to
sctp_tsnmap_mark(), as this is handled correctly in call to
sctp_ulpq_tail_data().

Patch applies to linux-3.8 kernel.

Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
 net/sctp/ulpqueue.c |   53 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 8 deletions(-)

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+3/net/sctp/ulpqueue.c linux-3.8-SCTP+4/net/sctp/ulpqueue.c
--- linux-3.8-SCTP+3/net/sctp/ulpqueue.c	2013-02-21 07:55:32.817713326 -0700
+++ linux-3.8-SCTP+4/net/sctp/ulpqueue.c	2013-02-21 08:07:41.562212475 -0700
@@ -540,14 +540,19 @@ static struct sctp_ulpevent *sctp_ulpq_r
 		ctsn = cevent->tsn;
 
 		switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
+		case SCTP_DATA_FIRST_FRAG:
+			if (!first_frag)
+				return NULL;
+			goto done;
 		case SCTP_DATA_MIDDLE_FRAG:
 			if (!first_frag) {
 				first_frag = pos;
 				next_tsn = ctsn + 1;
 				last_frag = pos;
-			} else if (next_tsn == ctsn)
+			} else if (next_tsn == ctsn) {
 				next_tsn++;
-			else
+				last_frag = pos;
+			} else
 				goto done;
 			break;
 		case SCTP_DATA_LAST_FRAG:
@@ -651,6 +656,14 @@ static struct sctp_ulpevent *sctp_ulpq_r
 			} else
 				goto done;
 			break;
+
+		case SCTP_DATA_LAST_FRAG:
+			if (!first_frag)
+				return NULL;
+			else
+				goto done;
+			break;
+
 		default:
 			return NULL;
 		}
@@ -1025,16 +1038,28 @@ void sctp_ulpq_partial_delivery(struct s
 	struct sctp_ulpevent *event;
 	struct sctp_association *asoc;
 	struct sctp_sock *sp;
+	__u32 ctsn;
+	struct sk_buff *skb;
 
 	asoc = ulpq->asoc;
 	sp = sctp_sk(asoc->base.sk);
 
 	/* If the association is already in Partial Delivery mode
-	 * we have noting to do.
+	 * we have nothing to do.
 	 */
 	if (ulpq->pd_mode)
 		return;
 
+	/* Data must be at or below the Cumulative TSN ACK Point to
+	 * start partial delivery.
+	 */
+	skb = skb_peek(&asoc->ulpq.reasm);
+	if (skb != NULL) {
+		ctsn = sctp_skb2event(skb)->tsn;
+		if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
+			return;
+	}
+
 	/* If the user enabled fragment interleave socket option,
 	 * multiple associations can enter partial delivery.
 	 * Otherwise, we can only enter partial delivery if the
@@ -1057,6 +1082,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *
 		      gfp_t gfp)
 {
 	struct sctp_association *asoc;
+	struct sk_buff *skb;
 	__u16 needed, freed;
 
 	asoc = ulpq->asoc;
@@ -1077,12 +1103,23 @@ void sctp_ulpq_renege(struct sctp_ulpq *
 	}
 	/* If able to free enough room, accept this chunk. */
 	if (chunk && (freed >= needed)) {
-		__u32 tsn;
+		__u32 tsn, ctsn;
 		tsn = ntohl(chunk->subh.data_hdr->tsn);
-		sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
-		sctp_ulpq_tail_data(ulpq, chunk, gfp);
-
-		sctp_ulpq_partial_delivery(ulpq, gfp);
+		if (sctp_ulpq_tail_data(ulpq, chunk, gfp) == 0) {
+			/*
+			 * Enter partial delivery if chunk is still on
+			 * reassembly queue; otherwise, drain the queue.
+			 */
+			skb = skb_peek(&ulpq->reasm);
+			if (skb != NULL) {
+				ctsn = sctp_skb2event(skb)->tsn;
+				if (TSN_lte(ctsn, tsn))
+					sctp_ulpq_partial_delivery(ulpq, chunk,
+						gfp);
+				else
+					sctp_ulpq_reasm_drain(ulpq);
+			}
+		}
 	}
 
 	sk_mem_reclaim(asoc->base.sk);

^ permalink raw reply

* [PATCH 3/4] sctp: fix association hangs due to errors when reneging events from the ordering queue
From: Roberts, Lee A. @ 2013-02-21 16:44 UTC (permalink / raw)
  To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

From: Lee A. Roberts <lee.roberts@hp.com>

Resolve SCTP association hangs observed during SCTP stress
testing.  Observable symptoms include communications hangs
with data being held in the association reassembly and/or lobby
(ordering) queues.  Close examination of reassembly queue shows
missing packets.

In sctp_ulpq_renege_list(), events being reneged from the
ordering queue may correspond to multiple TSNs.  Identify
all affected packets; sum freed space and renege from the
tsnmap.

Patch applies to linux-3.8 kernel.

Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
 net/sctp/ulpqueue.c |   26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+2/net/sctp/ulpqueue.c linux-3.8-SCTP+3/net/sctp/ulpqueue.c
--- linux-3.8-SCTP+2/net/sctp/ulpqueue.c	2013-02-21 07:39:40.888281496 -0700
+++ linux-3.8-SCTP+3/net/sctp/ulpqueue.c	2013-02-21 07:55:32.817713326 -0700
@@ -962,8 +962,8 @@ static __u16 sctp_ulpq_renege_list(struc
 		struct sk_buff_head *list, __u16 needed)
 {
 	__u16 freed = 0;
-	__u32 tsn;
-	struct sk_buff *skb;
+	__u32 tsn, last_tsn;
+	struct sk_buff *skb, *flist, *last;
 	struct sctp_ulpevent *event;
 	struct sctp_tsnmap *tsnmap;
 
@@ -977,10 +977,28 @@ static __u16 sctp_ulpq_renege_list(struc
 		if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
 			break;
 
-		__skb_unlink(skb, list);
+		/* Events in ordering queue may have multiple fragments
+		 * corresponding to additional TSNs.  Sum the total
+		 * freed space; find the last TSN.
+		 */
 		freed += skb_headlen(skb);
+		flist = skb_shinfo(skb)->frag_list;
+		for (last = flist; flist; flist = flist->next) {
+			last = flist;
+			freed += skb_headlen(last);
+		}
+		if (last)
+			last_tsn = sctp_skb2event(last)->tsn;
+		else
+			last_tsn = tsn;
+
+		/* Unlink the event, then renege all applicable TSNs. */
+		__skb_unlink(skb, list);
 		sctp_ulpevent_free(event);
-		sctp_tsnmap_renege(tsnmap, tsn);
+		while (TSN_lte(tsn, last_tsn)) {
+			sctp_tsnmap_renege(tsnmap, tsn);
+			tsn++;
+		}
 		if (freed >= needed)
 			return freed;
 	}

^ permalink raw reply

* [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: Roberts, Lee A. @ 2013-02-21 16:44 UTC (permalink / raw)
  To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

From: Lee A. Roberts <lee.roberts@hp.com>

Resolve SCTP association hangs observed during SCTP stress
testing.  Observable symptoms include communications hangs
with data being held in the association lobby (ordering)
queue.  Close examination of reassembly/ordering queues shows
duplicated packets.

In sctp_tsnmap_grow(), correct off-by-one errors when copying
and resizing the tsnmap.  If max_tsn_seen is in the LSB of the
word, this bit can be lost, causing the corresponding packet
to be transmitted again and to be entered as a duplicate into
the SCTP reassembly/ordering queues.

Patch applies to linux-3.8 kernel.

Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
 net/sctp/tsnmap.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-vanilla/net/sctp/tsnmap.c linux-3.8-SCTP+1/net/sctp/tsnmap.c
--- linux-3.8-vanilla/net/sctp/tsnmap.c	2013-02-18 16:58:34.000000000 -0700
+++ linux-3.8-SCTP+1/net/sctp/tsnmap.c	2013-02-20 08:01:02.555223259 -0700
@@ -369,14 +369,15 @@ static int sctp_tsnmap_grow(struct sctp_
 	if (gap >= SCTP_TSN_MAP_SIZE)
 		return 0;
 
-	inc = ALIGN((gap - map->len),BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
+	inc = ALIGN((gap - map->len + 1), BITS_PER_LONG)
+		+ SCTP_TSN_MAP_INCREMENT;
 	len = min_t(u16, map->len + inc, SCTP_TSN_MAP_SIZE);
 
 	new = kzalloc(len>>3, GFP_ATOMIC);
 	if (!new)
 		return 0;
 
-	bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn);
+	bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn + 1);
 	kfree(map->tsn_map);
 	map->tsn_map = new;
 	map->len = len;

^ permalink raw reply

* RE: [PATCH 1/3] sctp: fix association hangs due to reassembly/ordering logic
From: Roberts, Lee A. @ 2013-02-21 16:43 UTC (permalink / raw)
  To: David Miller
  Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20130220.125301.1295321158433252629.davem@davemloft.net>

David,

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, February 20, 2013 10:53 AM
> To: Roberts, Lee A.
> Cc: linux-sctp@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] sctp: fix association hangs due to reassembly/ordering logic
> 
> 
> Your email client has corrupted every one of your patches, splitting up
> new lines as well as making other alterations, making them useless for
> us.
> 
> Please read Documentation/email-clients.txt to learn how to fix this
> problem, and only resubmit your patches when you can successfully
> email a patch to yourself and cleanly apply the patch you receive
> in that email as-is.
> 
> Thanks.

I have addressed the e-mail client issue and have successfully mailed the
patches to myself and applied them.  I will be sending updated version shortly.

I have incorporated changes and recommendations from Vlad Yasevich into
the updated patches.

                                 - Lee Roberts

^ permalink raw reply

* Re: why is it not allowed to add a new socket protocol family as an external module?
From: Eric Dumazet @ 2013-02-21 16:43 UTC (permalink / raw)
  To: Chris Friesen; +Cc: Stephen Hemminger, netdev
In-Reply-To: <51264195.9060800@genband.com>

On Thu, 2013-02-21 at 09:47 -0600, Chris Friesen wrote:
> On 02/20/2013 07:39 PM, Eric Dumazet wrote:
> > On Wed, 2013-02-20 at 18:44 -0600, Chris Friesen wrote:
> >
> >> Yes, there are a number of tables sized by NPROTO/AF_MAX/PF_MAX (and the
> >> fact that we use all three within the kernel is kind of sad) but there
> >> is no technical reason why we couldn't extend those dynamically if
> >> desired (with a linked list of additional protocols, perhaps).
> >>
> >> Hence my question--is the restriction for an ideological reason or
> >> simply because nobody thought it was worth the effort?
> >>
> >
> > I guess nobody did the preliminary work.
> >
> > lockdep might be the tricky part.
> >
> > net/core/sock.c:197:static const char *const af_family_key_strings[AF_MAX+1] = {
> > net/core/sock.c:211:  "sk_lock-AF_NFC"   , "sk_lock-AF_MAX"
> > net/core/sock.c:213:static const char *const af_family_slock_key_strings[AF_MAX+1] = {
> > net/core/sock.c:227:  "slock-AF_NFC"   , "slock-AF_MAX"
> > net/core/sock.c:229:static const char *const af_family_clock_key_strings[AF_MAX+1] = {
> > net/core/sock.c:243:  "clock-AF_NFC"   , "clock-AF_MAX"
> > net/core/sock.c:250:static struct lock_class_key af_callback_keys[AF_MAX];
> 
> Unless I'm missing something that looks straightforward.
> 
> When registering dynamically the new protocol would need to specify one 
> string, the protocol name.  ("AF_MYPROTOCOL" or something).  The three 
> key strings are derived from that.
> 
> The three lock_class_key structs (corresponding to 
> af_family_keys/af_family_slock_keys/af_callback_keys) can be 
> auto-allocated by the network core at dynamic registration time.

Nope, this was the point I specifically raised but you missed it

Take a look at kernel/lockdep.c, lines 2981-2988

All the other stuff you mention seems pretty obvious.

^ permalink raw reply

* Re: why is it not allowed to add a new socket protocol family as an external module?
From: Chris Friesen @ 2013-02-21 16:19 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Eric Dumazet, netdev
In-Reply-To: <20130221080425.72e36f6d@nehalam.linuxnetplumber.net>

On 02/21/2013 10:04 AM, Stephen Hemminger wrote:

> It is not impossible to make this dynamic, you would need to make the table an allocated
> object and use proper locking like RCU. Oh, and because it is using GPL, the symbols
> would have to be EXPORT_SYMBOL_GPL(), so any dream of proprietary stacks there would
> be skating on even thinner ice.
> The lockdep stuff makes it more complicated but not impossible.

I'm not even thinking about closed-source stacks.  In our particular 
case the use-case is highly proprietary (not to mention legacy and 
obscure) but GPL-compliant.

> The bigger issue is how would you manage statically assigned id's which
> are not visible int headers or kernel source. How would you keep AF_VENDOR_PROTOCOL1 from
> not colliding with AF_VENDOR_PROTOCOL2?

I wouldn't expect many people to have multiple dynamic protocols loaded, 
so I'm not sure this would be a big problem.

For a fully generic solution I think we'd need to do dynamic numbering 
and export the mapping somewhere like /sys/class/net or 
/proc/sys/net/core/.  This would makes things a bit non-standard for the 
userspace code, but would allow arbitrary numbers of dynamic protocols 
without collision.

Or maybe we get really crazy and do protocol namespaces. :)

Chris

^ permalink raw reply

* Re: why is it not allowed to add a new socket protocol family as an external module?
From: Stephen Hemminger @ 2013-02-21 16:04 UTC (permalink / raw)
  To: Chris Friesen; +Cc: Eric Dumazet, netdev
In-Reply-To: <51264195.9060800@genband.com>

On Thu, 21 Feb 2013 09:47:33 -0600
Chris Friesen <chris.friesen@genband.com> wrote:

> On 02/20/2013 07:39 PM, Eric Dumazet wrote:
> > On Wed, 2013-02-20 at 18:44 -0600, Chris Friesen wrote:
> >
> >> Yes, there are a number of tables sized by NPROTO/AF_MAX/PF_MAX (and the
> >> fact that we use all three within the kernel is kind of sad) but there
> >> is no technical reason why we couldn't extend those dynamically if
> >> desired (with a linked list of additional protocols, perhaps).
> >>
> >> Hence my question--is the restriction for an ideological reason or
> >> simply because nobody thought it was worth the effort?
> >>
> >
> > I guess nobody did the preliminary work.
> >
> > lockdep might be the tricky part.
> >
> > net/core/sock.c:197:static const char *const af_family_key_strings[AF_MAX+1] = {
> > net/core/sock.c:211:  "sk_lock-AF_NFC"   , "sk_lock-AF_MAX"
> > net/core/sock.c:213:static const char *const af_family_slock_key_strings[AF_MAX+1] = {
> > net/core/sock.c:227:  "slock-AF_NFC"   , "slock-AF_MAX"
> > net/core/sock.c:229:static const char *const af_family_clock_key_strings[AF_MAX+1] = {
> > net/core/sock.c:243:  "clock-AF_NFC"   , "clock-AF_MAX"
> > net/core/sock.c:250:static struct lock_class_key af_callback_keys[AF_MAX];
> 
> Unless I'm missing something that looks straightforward.
> 
> When registering dynamically the new protocol would need to specify one 
> string, the protocol name.  ("AF_MYPROTOCOL" or something).  The three 
> key strings are derived from that.
> 
> The three lock_class_key structs (corresponding to 
> af_family_keys/af_family_slock_keys/af_callback_keys) can be 
> auto-allocated by the network core at dynamic registration time.
> 
> sock_lock_init(), sk_clone_lock(), and sock_init_data() would need 
> special-casing for protocol number larger than AF_MAX.  The most 
> readable would probably be to do the mapping from family to key/string 
> in helper functions rather than hard-coding a table offset right in 
> these routines.
> 
> Yes, there would be a runtime penalty, but it should be minimal.  For 
> static protocols it would be a few "sk->sk_family < AF_MAX" checks.  For 
> dynamic protocols they might need to walk a linked list, which in all 
> probability would be very short.

It is not impossible to make this dynamic, you would need to make the table an allocated
object and use proper locking like RCU. Oh, and because it is using GPL, the symbols
would have to be EXPORT_SYMBOL_GPL(), so any dream of proprietary stacks there would
be skating on even thinner ice. 
The lockdep stuff makes it more complicated but not impossible. 

The bigger issue is how would you manage statically assigned id's which
are not visible int headers or kernel source. How would you keep AF_VENDOR_PROTOCOL1 from
not colliding with AF_VENDOR_PROTOCOL2?

^ permalink raw reply

* Re: why is it not allowed to add a new socket protocol family as an external module?
From: Chris Friesen @ 2013-02-21 15:47 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
In-Reply-To: <1361410797.17413.3.camel@edumazet-glaptop>

On 02/20/2013 07:39 PM, Eric Dumazet wrote:
> On Wed, 2013-02-20 at 18:44 -0600, Chris Friesen wrote:
>
>> Yes, there are a number of tables sized by NPROTO/AF_MAX/PF_MAX (and the
>> fact that we use all three within the kernel is kind of sad) but there
>> is no technical reason why we couldn't extend those dynamically if
>> desired (with a linked list of additional protocols, perhaps).
>>
>> Hence my question--is the restriction for an ideological reason or
>> simply because nobody thought it was worth the effort?
>>
>
> I guess nobody did the preliminary work.
>
> lockdep might be the tricky part.
>
> net/core/sock.c:197:static const char *const af_family_key_strings[AF_MAX+1] = {
> net/core/sock.c:211:  "sk_lock-AF_NFC"   , "sk_lock-AF_MAX"
> net/core/sock.c:213:static const char *const af_family_slock_key_strings[AF_MAX+1] = {
> net/core/sock.c:227:  "slock-AF_NFC"   , "slock-AF_MAX"
> net/core/sock.c:229:static const char *const af_family_clock_key_strings[AF_MAX+1] = {
> net/core/sock.c:243:  "clock-AF_NFC"   , "clock-AF_MAX"
> net/core/sock.c:250:static struct lock_class_key af_callback_keys[AF_MAX];

Unless I'm missing something that looks straightforward.

When registering dynamically the new protocol would need to specify one 
string, the protocol name.  ("AF_MYPROTOCOL" or something).  The three 
key strings are derived from that.

The three lock_class_key structs (corresponding to 
af_family_keys/af_family_slock_keys/af_callback_keys) can be 
auto-allocated by the network core at dynamic registration time.

sock_lock_init(), sk_clone_lock(), and sock_init_data() would need 
special-casing for protocol number larger than AF_MAX.  The most 
readable would probably be to do the mapping from family to key/string 
in helper functions rather than hard-coding a table offset right in 
these routines.

Yes, there would be a runtime penalty, but it should be minimal.  For 
static protocols it would be a few "sk->sk_family < AF_MAX" checks.  For 
dynamic protocols they might need to walk a linked list, which in all 
probability would be very short.

Chris

^ permalink raw reply

* Re: why is it not allowed to add a new socket protocol family as an external module?
From: Chris Friesen @ 2013-02-21 15:19 UTC (permalink / raw)
  To: David Miller; +Cc: stephen, netdev
In-Reply-To: <20130220.200502.1241369422255314068.davem@davemloft.net>

On 02/20/2013 07:05 PM, David Miller wrote:
> From: Chris Friesen<chris.friesen@genband.com> Date: Wed, 20 Feb 2013
> 18:44:50 -0600
>
>> On 02/20/2013 05:23 PM, Stephen Hemminger wrote:
>>> On Wed, 20 Feb 2013 10:56:13 -0600 Chris
>>> Friesen<chris.friesen@genband.com>   wrote:
>>>
>>>> Hi,
>>>>
>>>> I was just wondering why the kernel doesn't allow a new
>>>> network protocol family to be loaded as as a kernel module
>>>> built outside the kernel source tree.
>>
>>> If you want an answer, to the question, use a tool like cscope
>>> and learn to read the kernel code. There are several tables of
>>> pointers sized by NPROTO.
>>
>> That's a bit insulting, don't you think?
>
> Absolutely not insulting at all.
>
> The list is _NOT_ a place to go when you're just too damn lazy to
> take the 10 seconds it would have taken to answer your question with
> a quick NPROTO grep on the kernel sources.
>
> Stephen's response to you was therefore %100 appropriate, and I
> would have told you likewise if I had been the first to respond.

As the rest of my reply to Stephen should have made clear, I in fact did 
read the code prior to asking the question.  My question was not about 
the details of the current implementation, but rather the rationale 
behind them.

I'm fully aware that there are statically-sized tables in the current 
code, and thus the current code does not support dynamically adding 
protocols.  However, it would be reasonably straightforward to extend 
the static tables with some form of dynamic data structure to handle 
adding new network protocols at runtime.

If the current restriction is intentional (to minimize the likelihood 
of non-GPL'd network protocols, for instance), then there's no point in 
anyone submitting patches to add support for dynamic network protocols. 
  If it's just because nobody has bothered to do it yet, then it might 
be an interesting project.

Chris

^ permalink raw reply

* Re: [PATCH net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Neil Horman @ 2013-02-21 15:07 UTC (permalink / raw)
  To: Jon Maloy; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue, Jon Maloy
In-Reply-To: <5125F5D3.1000509@ericsson.com>

On Thu, Feb 21, 2013 at 11:24:19AM +0100, Jon Maloy wrote:
> On 02/19/2013 10:44 PM, Neil Horman wrote:
> > 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?
> 
> Nothing. That is why we need this extra security measure that this limit provides.
> The normal senders will never hit it, the crazy ones will, and have their 
> connections broken as a consequence. 
> Sorry I didn't express this clearly enough.
This isn't a security measure.  If a sender is well
behaved, then as you say, they won't violate their send window, and will
ostensibly behave properly (regardless of what limit you set on sk_rcvbuf, be it
the large one you currently employ, or something bigger or smaller).  If they
are malicious, then they will hit the limit, and they won't care (because
they're
malicious).  They may have their connections broken, but so what?  Its the
receiver that needs to be able to protect itself.  The static limit you set using
this implementation is likely fine for many systems that have sufficient memory,
but consider a system that is memory constrained.  If a user administratively
tunes down sk_rcvbuf to avoid over-consumption of memory on a TIPC socket, that
condition will be ignored (or at least severly violated).  Thats bad, and may
lead to memory exhaustion despite having administratively taken action to fix
it.

> 
> > 
> >> 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).
> >>>
> [...]
> > 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.
> 
> This is not a bug, but an inherent  property of any protocol providing
> sequential, guaranteed delivery of packets, TCP inclusive.
> If you lose/drop a packet in the sequence, no subsequent packets in
> the stream can be delivered until the missing one has been retransmitted and
> delivered. To make this delivery dependent of the whims of each and any of the 
> potentially hundreds of receiving processes is simply not a good idea.
> 
Clearly sequential packet delivery implies the need to queue and postpone
application delivery of packets behind the reception of a lost packet in
sequence if it is dropped.  The problem I'm referring to here is that you seem
to have built TIPC around the notion of a single sequence namespace per system,
rather than one per socket.  TCP, employing sequential delivery, means that
dropping a packet stalls reception, but only for the socket in question.  The
fact you created TIPC with a design point in which packet loss blocked an entire
system, rather than only the intended receiving application is the issue here,
and doing so doesn't give you the freedom to violate other control points in the
network stack.

> I could even show that your proposal would cause almost immediate
> deadlock, due to the order incoming and outgoing data paths are grabbing 
> the locks. But I would rather drop this part of the discussion; we can 
> achieve what we want anyway, with much simpler means. See below.
> 
Perhaps you could, but if you were to deomonstrate how lowering your recieve
buffer limit (or even just honoring whatever is set), results in a deadlock
(ostensibly within the TIPC protocol code), how do you justify that as a reason
why you should be allowed to simply violate your set socket limits, rather than
recognizing the deadlock as a coding bug that needs to be fixed?  If you like,
we can begin a separate thread on that subject and look into fixing the issue.

> > 
> >> 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.  
> 
> Not at all. Just like you, I am trying to find a way to tune up the defaults 
> and the the limits using existing mechanisms, so that we can honour the 
> nominal value of sk_rcvbuf. 
> I think what you suggested may be a way to achieve this, but I am a little 
> worried about side effects. I'll explain further down.
> 
> > 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
> 
> [...]
> 
> >>
> >> 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.  
> 
> This is exactly what I want to avoid. To force the users to set special parameters
> during boot up just to make TIPC work would become another obstacle for the its 
> adoption. TIPC is extremely easy to install and use, and it should remain so.
> 
Theres nothing special about the default values for socket limits.  They're well
known, and well understood.  If the defaults don't work for your protocol, you
can adjust them.  Its two lines in /etc/sysctl.conf, thats it.  And if you were
to implement the memory pressue interface in the protocol, you could likely
skip doing even that.

> A programmatic solution is much better, if we can solve it from inside the module,
> and this is what I am convinced that we can.
> The following is an approach we discussed (inside the team) earlier, but dropped
> due to the perceived impossibility to set the sk_rcvbuf to the desired values.
> Now this limitation is not there, as I understand, and we can revive our 
> proposal.
> 
> 1: When a TIPC socket is created, sk_rcvbuf is set to 64MB+ (as it was before the 
>    latest patch). This is the limit used as last resort against connected peers
>    not respecting the connection send window.
> 
As I noted above, as long as there is a method to change the administrative
default, thats fine.

> 2: Datagram messages are checked against fractions of this value, according to
>    their importance priority. E.g. LOW->sk_rcvbuf/16,  MEDIUM->sk_rcvbuf/8
>    HIGH->sk_rcvbuf/4 and CRITICAL->sk_rcvbuf/2.
> 
Yes, this is what I proposed in my initial email.

> 3: In the unlikely event that anybody wants to change these limits, he can change
>    sk_rcvbuf via setsockopt(SOL_SOCK) either by first changing
>    /proc/sys/net/core/rmem_max as you suggest, or via a dedicated setsockopt(SOL_TIPC).
>    The latter option would have the advantage that it could enforce a lower limit of
>    skr_rcvbuf of 64MB, something that would be unthinkable with SOL_SOCK, because it 
>    would obviously affect other protocols.
Theres really no need for a private TIPC socket option here.  You do this at the
socket level, and if you want to set it higher than rmem_max, you adjust
rmem_max first.  Its just the way the system works.  Every other protocol
conforms to it, theres no reason TIPC shouldn't also.
> 
> What do you think?
> 
Yes, this is for the most part, good.
Thanks
Neil
> ///jon
> 
> If you set:
> > 
> > /proc/sys/net/core/rmem_default
> 
> But this will 
> > 
> > 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:
> > 
> > 
> > 
> > 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: [GIT] Networking
From: Mark Lord @ 2013-02-21 14:37 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Miller, Andrew Morton, Network Development,
	Linux Kernel Mailing List
In-Reply-To: <CA+55aFz=tzf9a-SY1CqCAxsqRDz=ebE69YoZ30at6kHeKKhsaA@mail.gmail.com>

On 13-02-20 10:05 PM, Linus Torvalds wrote:
> On Wed, Feb 20, 2013 at 2:09 PM, David Miller <davem@davemloft.net> wrote:
>>
>> 15) Orphan and delete a bunch of pre-historic networking drivers from
>>     Paul Gortmaker.
> 
> Nooo.... You killed the 3c501 and 3c503 drivers! Snif.
> 
> I wonder if they still worked..

I hope they're not really dead, because we still use them in several machines here
as secondary interfaces for test rigs and whatnot.

It's a tad early to be nuking support for such widespread devices.

^ permalink raw reply

* Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
From: Ohad Ben-Cohen @ 2013-02-21 13:53 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Dmitry Tarnyagin, Sjur Brændeland, Linus Walleij, Ido Yariv,
	linux-kernel@vger.kernel.org, Erwan Yvin, virtualization, netdev,
	David S. Miller, Sjur Brændeland
In-Reply-To: <8738wqgok6.fsf@rustcorp.com.au>

On Thu, Feb 21, 2013 at 8:37 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Hmm... I clearly jumped the gun, assuming consensus was already reached.
> I have put these patches *back* into pending-rebases, and they will not
> be merged this merge window.

Thanks.

What do you think about creating some virtio-level wrappers for the
vringh handlers?

I don't think we're going to stop with caif as the only vringh user,
and it could be nice if we follow the virtio spirit of decoupling the
drivers from the low level implementation. It sure did prove itself
when the remoteproc use cases started showing up, and it's neat.

Thanks,
Ohad.

^ permalink raw reply

* Congratulation!!! Your Email Account has won you a Prize!!
From: PROMOTION AWARD @ 2013-02-21 13:51 UTC (permalink / raw)


[-- Attachment #1: Type: text/plain, Size: 59 bytes --]

 Congratulation!!! Your Email Account has won you a Prize!!

[-- Attachment #2: MICROSOFT_AWARD_PROMOTION_2013.doc --]
[-- Type: application/msword, Size: 125440 bytes --]

^ permalink raw reply

* Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
From: Ohad Ben-Cohen @ 2013-02-21 13:47 UTC (permalink / raw)
  To: Sjur Brændeland
  Cc: Dmitry Tarnyagin, netdev, Linus Walleij,
	linux-kernel@vger.kernel.org, Erwan Yvin, virtualization,
	David S. Miller, Ido Yariv
In-Reply-To: <CAJK669bDu3SphxmEtA+h+v=btsN91t0S9Mn6ik-OKr=8gtducw@mail.gmail.com>

On Thu, Feb 21, 2013 at 1:01 AM, Sjur Brændeland
<sjur.brandeland@stericsson.com> wrote:
> [Sjur:]
>>> How do you see the in-kernel API for this? I would like to see
>>> something similar to my previous patches, where we extend
>>> the virtqueue API. E.g. something like this:
>>> struct virtqueue *vring_new_virtqueueh(...)...
>
> [Rusty:]
>>I was just going to create _kernel variants of all the _user helpers,
>>and let you drive it directly like that.
>>
>>If we get a second in-kernel user, we create wrappers (I'd prefer not to
>>overload struct virtqueue though).

The wrappers suggestion sounds good.

One of the things we truly enjoyed about virtio is how easy it was to
reuse its drivers for communication with a remote processor.

It'd be great if we can stick to this design and keep vringh virtio
drivers decoupled from the low level implementation they are developed
with (specifically caif and remoteproc in this case).

> Yes, if you insert a "malicious device" like that you can make it crash,
> but wouldn't most drivers do if you try to register a malicious device...?
>
> If we really want to protect from this, we could perhaps validate the vdev
> pointer in function rproc_virtio_new_vringh() by looking through the vdevs
> of the registered rprocs.

It sounds like we can instead just avoid this altogether if we follow
Rusty's wrappers suggestion.

The result would probably look better, and I suspect it might be very
minimal code.

> If some other driver is showing up using the vringh kernel API where
> the current API don't fit, I guess it's time to create some abstractions
> and wrappers... But I hope the current approach will do for now?

Unless I'm missing something here, adding wrappers should be straight
forward and quick. Coupling a virtio driver with remoteproc doesn't
look great to me, probably because I appreciate so much the elegance
of virtio and how easy we could have reused its vanilla drivers with a
use case it wasn't originally designed to support.

>> I have some other questions as well but maybe it's better to discuss
>> first the bigger picture.
>
> OK, but please don't hesitate to address this. I'm still aiming for this
> to go into 3.9.

I was wondering - can you please explain your motivation for using
vringh in caif ?

We have internally discussed supporting multiple remote processors
talking to each other using rpmsg, and in that scenario using vringh
can considerably simplifies the solution (no need to decide in advance
which side is the "guest" and which is the "host"). Was this the
general incentive in using vringh in caif too or something else?

Thanks,
Ohad.

^ permalink raw reply

* Re: [GIT PULL nf] IPVS fixes #2
From: Daniel Borkmann @ 2013-02-21 11:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov
In-Reply-To: <20130213201301.GA7245@localhost>

On 02/13/2013 09:13 PM, Pablo Neira Ayuso wrote:
> On Thu, Feb 07, 2013 at 07:12:13PM +0100, Pablo Neira Ayuso wrote:
>> On Wed, Feb 06, 2013 at 10:24:37AM +0900, Simon Horman wrote:
>>> Hi Pablo,
>>>
>>> Another fix. This one seems suitable for stable all the way back to
>>> 2.6.34. I suspect this change may not apply cleanly all the way back there.
>>> Please let me know if I should prepare some backports.
>
> While reviewing patches that I'll pass to -stable, I noticed that:
>
> 4b47bc9 ipvs: sctp: fix checksumming on snat and dnat handlers
>
> depends on:
>
> commit d4383f04d145cce8b855c463f40020639ef83ea0
> Author: Jesper Dangaard Brouer <brouer@redhat.com>
> Date:   Wed Sep 26 14:07:17 2012 +0200
>
>      ipvs: API change to avoid rescan of IPv6 exthdr
>
> which is too big to add as dependency (it does not fulfill -stable
> rule of not bigger patches than 100 lines).
>
> If you want to me pass this sctp fix to -stable, you or Daniel will
> have to send me a backport.

I'm currently working on it. Will send you a patch by today.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Jon Maloy @ 2013-02-21 10:24 UTC (permalink / raw)
  To: Neil Horman; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue, Jon Maloy
In-Reply-To: <20130219214439.GC31871@hmsreliant.think-freely.org>

On 02/19/2013 10:44 PM, Neil Horman wrote:
> 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?

Nothing. That is why we need this extra security measure that this limit provides.
The normal senders will never hit it, the crazy ones will, and have their 
connections broken as a consequence. 
Sorry I didn't express this clearly enough.

> 
>> 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).
>>>
[...]
> 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.

This is not a bug, but an inherent  property of any protocol providing
sequential, guaranteed delivery of packets, TCP inclusive.
If you lose/drop a packet in the sequence, no subsequent packets in
the stream can be delivered until the missing one has been retransmitted and
delivered. To make this delivery dependent of the whims of each and any of the 
potentially hundreds of receiving processes is simply not a good idea.

I could even show that your proposal would cause almost immediate
deadlock, due to the order incoming and outgoing data paths are grabbing 
the locks. But I would rather drop this part of the discussion; we can 
achieve what we want anyway, with much simpler means. See below.

> 
>> 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.  

Not at all. Just like you, I am trying to find a way to tune up the defaults 
and the the limits using existing mechanisms, so that we can honour the 
nominal value of sk_rcvbuf. 
I think what you suggested may be a way to achieve this, but I am a little 
worried about side effects. I'll explain further down.

> 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

[...]

>>
>> 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.  

This is exactly what I want to avoid. To force the users to set special parameters
during boot up just to make TIPC work would become another obstacle for the its 
adoption. TIPC is extremely easy to install and use, and it should remain so.

A programmatic solution is much better, if we can solve it from inside the module,
and this is what I am convinced that we can.
The following is an approach we discussed (inside the team) earlier, but dropped
due to the perceived impossibility to set the sk_rcvbuf to the desired values.
Now this limitation is not there, as I understand, and we can revive our 
proposal.

1: When a TIPC socket is created, sk_rcvbuf is set to 64MB+ (as it was before the 
   latest patch). This is the limit used as last resort against connected peers
   not respecting the connection send window.

2: Datagram messages are checked against fractions of this value, according to
   their importance priority. E.g. LOW->sk_rcvbuf/16,  MEDIUM->sk_rcvbuf/8
   HIGH->sk_rcvbuf/4 and CRITICAL->sk_rcvbuf/2.

3: In the unlikely event that anybody wants to change these limits, he can change
   sk_rcvbuf via setsockopt(SOL_SOCK) either by first changing
   /proc/sys/net/core/rmem_max as you suggest, or via a dedicated setsockopt(SOL_TIPC).
   The latter option would have the advantage that it could enforce a lower limit of
   skr_rcvbuf of 64MB, something that would be unthinkable with SOL_SOCK, because it 
   would obviously affect other protocols.

What do you think?

///jon

If you set:
> 
> /proc/sys/net/core/rmem_default

But this will 
> 
> 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:
> 
> 
> 
> 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

* [PATCH] ipv4: fix a bug in ping_err().
From: Li Wei @ 2013-02-21 10:09 UTC (permalink / raw)
  To: davem; +Cc: netdev, lw

We should get 'type' and 'code' from the outer ICMP header.

Signed-off-by: Li Wei <lw@cn.fujitsu.com>
---
 net/ipv4/ping.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 55c4ee1..2e91006 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -322,8 +322,8 @@ void ping_err(struct sk_buff *skb, u32 info)
 	struct iphdr *iph = (struct iphdr *)skb->data;
 	struct icmphdr *icmph = (struct icmphdr *)(skb->data+(iph->ihl<<2));
 	struct inet_sock *inet_sock;
-	int type = icmph->type;
-	int code = icmph->code;
+	int type = icmp_hdr(skb)->type;
+	int code = icmp_hdr(skb)->code;
 	struct net *net = dev_net(skb->dev);
 	struct sock *sk;
 	int harderr;
-- 
1.8.0.2

^ permalink raw reply related

* YAHOO-AWARD-KINDLY OPEN ATTACHMENT FILE
From: YAHOO-AWARD @ 2013-02-21 10:05 UTC (permalink / raw)


[-- Attachment #1: Type: text/plain, Size: 49 bytes --]





YAHOO-AWARD-KINDLY OPEN ATTACHMENT FILE

[-- Attachment #2: YAHOO-AWARD.....doc --]
[-- Type: application/msword, Size: 304640 bytes --]

^ permalink raw reply

* [PATCH net-next] be2net: Update copyright year
From: sarveshwar.bandi @ 2013-02-21  9:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam, Sarveshwar Bandi

From: Vasundhara Volam <vasundhara.volam@emulex.com>

Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
Signed-off-by: Sarveshwar Bandi <sarveshwar.bandi@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be.h         |    2 +-
 drivers/net/ethernet/emulex/benet/be_cmds.c    |    2 +-
 drivers/net/ethernet/emulex/benet/be_cmds.h    |    2 +-
 drivers/net/ethernet/emulex/benet/be_ethtool.c |    2 +-
 drivers/net/ethernet/emulex/benet/be_hw.h      |    2 +-
 drivers/net/ethernet/emulex/benet/be_main.c    |    2 +-
 drivers/net/ethernet/emulex/benet/be_roce.c    |    2 +-
 drivers/net/ethernet/emulex/benet/be_roce.h    |    2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 28ceb84..ff1efe5 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 071aea7..4512e42 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 9697086..6ef4575 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index 76b302f..053f00d 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_hw.h b/drivers/net/ethernet/emulex/benet/be_hw.h
index 541d453..c515eea 100644
--- a/drivers/net/ethernet/emulex/benet/be_hw.h
+++ b/drivers/net/ethernet/emulex/benet/be_hw.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 3860888..1f8103c 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_roce.c b/drivers/net/ethernet/emulex/benet/be_roce.c
index 55d32aa..f3d126d 100644
--- a/drivers/net/ethernet/emulex/benet/be_roce.c
+++ b/drivers/net/ethernet/emulex/benet/be_roce.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
diff --git a/drivers/net/ethernet/emulex/benet/be_roce.h b/drivers/net/ethernet/emulex/benet/be_roce.h
index db4ea80..2765729 100644
--- a/drivers/net/ethernet/emulex/benet/be_roce.h
+++ b/drivers/net/ethernet/emulex/benet/be_roce.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005 - 2011 Emulex
+ * Copyright (C) 2005 - 2013 Emulex
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or
-- 
1.7.9.5

^ permalink raw reply related

* RE: [E1000-devel] [next:akpm 16/587] drivers/net/ethernet/intel/igb/igb_main.c:6231:2: error: call to '__compiletime_assert_6235' declared with attribute error: BUILD_BUG_ON failed: SKB_WITH_OVERHEAD(IGB_RX_BUFSZ) < (NET_SKB_PAD + NET_IP_ALIGN + IGB_TS_HD
From: David Laight @ 2013-02-21  9:42 UTC (permalink / raw)
  To: Eric Dumazet, Alexander Duyck
  Cc: Allan, Bruce W, e1000-devel, netdev, Brandeburg, Jesse,
	Daniel Santos, Andrew Morton, Wu, Fengguang
In-Reply-To: <1361388156.19353.216.camel@edumazet-glaptop>

> > The problem is the 256 byte alignment for L1_CACHE_BYTES is increasing
> > the size of the data and shared info significantly pushing us past the
> > 2K limit.
> >
> > I'll look into this since it likely affects ixgbe as well.

Doesn't a 256 byte cache line cause issues almost everywhere
anything is marked as 'cache_aligned' ?

Some other structures have members part way down marked that way
so that the 2nd cache line (with smaller lines) contains a
known set of fields (either to get them together, or out of the
cache line containing the first part of the data).

With 256 byte cache lines the entire structure would probably be
a single cache line - except that it gets extended to two lines.

I know that some people have a habit of allocating mutex in their
own cache line (with nothing else), that also doesn't make sense
to me - especially with large cache lines (I don't know if there
any of those in Linux).

	David


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox