04 Nov, 2012
1 commit
-
If tasklet_disable() is called before related tasklet handled,
tasklet_kill will never be finished. tasklet_kill is enough.Signed-off-by: Xiaotian Feng
Cc: Jon Maloy
Cc: Allan Stephens
Cc: "David S. Miller"
Cc: netdev@vger.kernel.org
Cc: tipc-discussion@lists.sourceforge.net
Signed-off-by: David S. Miller
05 Oct, 2012
1 commit
-
When large buffers are sent over connected TIPC sockets, it
is likely that the sk_backlog will be filled up on the
receiver side, but the TIPC flow control mechanism is happily
unaware of this since that is based on message count.The sender will receive a TIPC_ERR_OVERLOAD message when this occurs
and drop it's side of the connection, leaving it stale on
the receiver end.By increasing the sk_rcvbuf to a 'worst case' value, we avoid the
overload caused by a full backlog queue and the flow control
will work properly.This worst case value is the max TIPC message size times
the flow control window, multiplied by two because a sender
will transmit up to double the window size before a port is marked
congested.
We multiply this by 2 to account for the sk_buff and other overheads.Signed-off-by: Erik Hugne
Signed-off-by: David S. Miller
19 Sep, 2012
1 commit
-
Found by http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin
Signed-off-by: David S. Miller
11 Sep, 2012
1 commit
-
It is a frequent mistake to confuse the netlink port identifier with a
process identifier. Try to reduce this confusion by renaming fields
that hold port identifiers portid instead of pid.I have carefully avoided changing the structures exported to
userspace to avoid changing the userspace API.I have successfully built an allyesconfig kernel with this change.
Signed-off-by: "Eric W. Biederman"
Acked-by: Stephen Hemminger
Signed-off-by: David S. Miller
20 Aug, 2012
9 commits
-
Gets rid of the need for users to specify the maximum number of
name publications supported by TIPC. TIPC now automatically provides
support for the maximum number of name publications to 65535.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
Gets rid of the need for users to specify the maximum number of
name subscriptions supported by TIPC. TIPC now automatically provides
support for the maximum number of name subscriptions to 65535.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
Added to the following:
- tipc_random
- tipc_own_addr
- tipc_max_ports
- tipc_net_id
- tipc_remote_management
- handler_enabledThe above global variables are read often, but written rarely. Use
__read_mostly to prevent them being on the same cacheline as another
variable which is written to often, which would cause cacheline
bouncing.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
There is nothing changing this variable dynamically, so change
it to a macro to make that more obvious when reading the code.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
Since now tipc_net_start() always returns a success code - 0, its
return value type should be changed from integer to void, which can
avoid unnecessary check for its return value.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
After eliminating the mechanism which checks whether all letters
in media name string are within a given character set, the
media_name_valid routine becomes trivial. It is also only
used once, so it is unnecessary to keep it as a separate function.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
There is no real reason to check whether all letters in the given
media name and network interface name are within the character set
defined in tipc_alphabet array. Even if we eliminate the checking,
the rest of checking conditions in tipc_enable_bearer() can ensure
we do not enable an invalid or illegal bearer.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
When the lockdep validator is enabled, it will report the below
warning when we enable a TIPC bearer:[ INFO: possible irq lock inversion dependency detected ]
---------------------------------------------------------
Possible interrupt unsafe locking scenario:CPU0 CPU1
---- ----
lock(ptype_lock);
local_irq_disable();
lock(tipc_net_lock);
lock(ptype_lock);
lock(tipc_net_lock);*** DEADLOCK ***
the shortest dependencies between 2nd lock and 1st lock:
-> (ptype_lock){+.+...} ops: 10 {
[...]
SOFTIRQ-ON-W at:
[] __lock_acquire+0x528/0x13e0
[] lock_acquire+0x90/0x100
[] _raw_spin_lock+0x38/0x50
[] dev_add_pack+0x3a/0x60
[] arp_init+0x1a/0x48
[] inet_init+0x181/0x27e
[] do_one_initcall+0x34/0x170
[] kernel_init+0x110/0x1b2
[] kernel_thread_helper+0x6/0x10
[...]
... key at: [] ptype_lock+0x10/0x20
... acquired at:
[] lock_acquire+0x90/0x100
[] _raw_spin_lock+0x38/0x50
[] dev_add_pack+0x3a/0x60
[] enable_bearer+0xf2/0x140 [tipc]
[] tipc_enable_bearer+0x1ba/0x450 [tipc]
[] tipc_cfg_do_cmd+0x5c4/0x830 [tipc]
[] handle_cmd+0x42/0xd0 [tipc]
[] genl_rcv_msg+0x232/0x280
[] netlink_rcv_skb+0x86/0xb0
[] genl_rcv+0x1c/0x30
[] netlink_unicast+0x174/0x1f0
[] netlink_sendmsg+0x1eb/0x2d0
[] sock_aio_write+0x161/0x170
[] do_sync_write+0xac/0xf0
[] vfs_write+0x156/0x170
[] sys_write+0x42/0x70
[] sysenter_do_call+0x12/0x38
[...]
}
-> (tipc_net_lock){+..-..} ops: 4 {
[...]
IN-SOFTIRQ-R at:
[] __lock_acquire+0x64a/0x13e0
[] lock_acquire+0x90/0x100
[] _raw_read_lock_bh+0x3d/0x50
[] tipc_recv_msg+0x1d/0x830 [tipc]
[] recv_msg+0x3f/0x50 [tipc]
[] __netif_receive_skb+0x22a/0x590
[] netif_receive_skb+0x2b/0xf0
[] pcnet32_poll+0x292/0x780
[] net_rx_action+0xfa/0x1e0
[] __do_softirq+0xae/0x1e0
[...]
}>From the log, we can see three different call chains between
CPU0 and CPU1:Time 0 on CPU0:
kernel_init()->inet_init()->dev_add_pack()
At time 0, the ptype_lock is held by CPU0 in dev_add_pack();
Time 1 on CPU1:
tipc_enable_bearer()->enable_bearer()->dev_add_pack()
At time 1, tipc_enable_bearer() first holds tipc_net_lock, and then
wants to take ptype_lock to register TIPC protocol handler into the
networking stack. But the ptype_lock has been taken by dev_add_pack()
on CPU0, so at this time the dev_add_pack() running on CPU1 has to be
busy looping.Time 2 on CPU0:
netif_receive_skb()->recv_msg()->tipc_recv_msg()
At time 2, an incoming TIPC packet arrives at CPU0, hence
tipc_recv_msg() will be invoked. In tipc_recv_msg(), it first wants
to hold tipc_net_lock. At the moment, below scenario happens:On CPU0, below is our sequence of taking locks:
lock(ptype_lock)->lock(tipc_net_lock)
On CPU1, our sequence of taking locks looks like:
lock(tipc_net_lock)->lock(ptype_lock)
Obviously deadlock may happen in this case.
But please note the deadlock possibly doesn't occur at all when the
first TIPC bearer is enabled. Before enable_bearer() -- running on
CPU1 does not hold ptype_lock, so the TIPC receive handler (i.e.
recv_msg()) is not registered successfully via dev_add_pack(), so
the tipc_recv_msg() cannot be called by recv_msg() even if a TIPC
message comes to CPU0. But when the second TIPC bearer is
registered, the deadlock can perhaps really happen.To fix it, we will push the work of registering TIPC protocol
handler into workqueue context. After the change, both paths taking
ptype_lock are always in process contexts, thus, the deadlock should
never occur.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
Ethernet media initialization is only done when TIPC is started or
switched to network mode. So the initialization of the network device
notifier structure can be moved out of this function and done
statically instead.Signed-off-by: Ying Xue
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller
14 Jul, 2012
6 commits
-
The internal log buffer handling functions can now safely be
removed since there is no code using it anymore. Requests to
interact with the internal tipc log buffer over netlink (in
config.c) will report 'obsolete command'.This represents the final removal of any references to a
struct print_buf, and the removal of the struct itself.
We also get rid of a TIPC specific Kconfig in the process.Finally, log.h is removed since it is not needed anymore.
Signed-off-by: Erik Hugne
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker -
The tipc_printf is renamed to tipc_snprintf, as the new name
describes more what the function actually does. It is also
changed to take a buffer and length parameter and return
number of characters written to the buffer. All callers of
this function that used to pass a print_buf are updated.Final removal of the struct print_buf itself will be done
synchronously with the pending removal of the deprecated
logging code that also was using it.Functions that build up a response message with a list of
ports, nametable contents etc. are changed to return the number
of characters written to the output buffer. This information
was previously hidden in a field of the print_buf struct, and
the number of chars written was fetched with a call to
tipc_printbuf_validate. This function is removed since it
is no longer referenced nor needed.A generic max size ULTRA_STRING_MAX_LEN is defined, named
in keeping with the existing TIPC_TLV_ULTRA_STRING, and the
various definitions in port, link and nametable code that
largely duplicated this information are removed. This means
that amount of link statistics that can be returned is now
increased from 2k to 32k.The buffer overflow check is now done just before the reply
message is passed over netlink or TIPC to a remote node and
the message indicating a truncated buffer is changed to a less
dramatic one (less CAPS), placed at the end of the message.Signed-off-by: Erik Hugne
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker -
tipc_printf was previously used both to construct debug traces
and to append data to buffers that should be sent over netlink
to the tipc-config application. A global print_buffer was
used to format the string before it was copied to the actual
output buffer. This could lead to concurrent access of the
global print_buffer, which then had to be lock protected.
This is simplified by changing tipc_printf to append data
directly to the output buffer using vscnprintf.With the new implementation of tipc_printf, there is no longer
any risk of concurrent access to the internal log buffer, so
the lock (and the comments describing it) are no longer
strictly necessary. However, there are still a few functions
that do grab this lock before resizing/dumping the log
buffer. We leave the lock, and these functions untouched since
they will be removed with a subsequent commit that drops the
deprecated log buffer handling codeSigned-off-by: Erik Hugne
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker -
To pave the way for a pending cleanup of tipc_printf, and
removal of struct print_buf entirely, we make that task simpler
by converting link_print to issue its messages with standard
printk infrastructure. [Original idea separated from a larger
patch from Erik Hugne ]Cc: Erik Hugne
Signed-off-by: Paul Gortmaker -
The link queue traces and packet level debug functions served
a purpose during early development, but are now redundant
since there are other, more capable tools available for
debugging at the packet level.The TIPC_DEBUG Kconfig option is removed since it does not
provide any extra debugging features anymore.This gets rid of a lot of tipc_printf usages, which will
make the pending cleanup work of that function easier.Signed-off-by: Erik Hugne
Signed-off-by: Jon Maloy
Signed-off-by: Paul Gortmaker -
All messages should go directly to the kernel log. The TIPC
specific error, warning, info and debug trace macro's are
removed and all references replaced with pr_err, pr_warn,
pr_info and pr_debug.Commonly used sub-strings are explicitly declared as a const
char to reduce .text size.Note that this means the debug messages (changed to pr_debug),
are now enabled through dynamic debugging, instead of a TIPC
specific Kconfig option (TIPC_DEBUG). The latter will be
phased out completelySigned-off-by: Erik Hugne
Signed-off-by: Jon Maloy
[PG: use pr_fmt as suggested by Joe Perches ]
Signed-off-by: Paul Gortmaker
12 Jul, 2012
2 commits
-
With the default name table size of 1024, it is possible that
the sanity check in tipc_nametbl_stop could spam out 1024
essentially identical error messages if memory was corrupted
or similar. Limit it to issuing no more than a single message.The actual chain number (i.e. 0 --> 1023) wouldn't provide any
useful insight if/when such an instance happened, so don't
bother printing out that value.Signed-off-by: Paul Gortmaker
-
This is done to improve readability, and so that we can give
the struct a name that will allow us to declare a local
pointer to it in code, instead of having to always redirect
through the link struct to get to it.Signed-off-by: Paul Gortmaker
11 Jul, 2012
2 commits
-
Signed-off-by: Ben Hutchings
Signed-off-by: David S. Miller -
Fix incorrect start markers, wrapped summary lines, missing section
breaks, incorrect separators, and some name mismatches.Signed-off-by: Ben Hutchings
Signed-off-by: David S. Miller
04 Jun, 2012
1 commit
-
Adding casts of objects to the same type is unnecessary
and confusing for a human reader.For example, this cast:
int y;
int *p = (int *)&y;I used the coccinelle script below to find and remove these
unnecessary casts. I manually removed the conversions this
script produces of casts with __force and __user.@@
type T;
T *p;
@@- (T *)p
+ pSigned-off-by: Joe Perches
Signed-off-by: David S. Miller
01 May, 2012
1 commit
-
Some of the comment blocks are floating in limbo between two
functions, or between blocks of code. Delete the extra line
feeds between any comment and its associated following block
of code, to be consistent with the majority of the rest of
the kernel. Also delete trailing newlines at EOF and fix
a couple trivial typos in existing comments.This is a 100% cosmetic change with no runtime impact. We get
rid of over 500 lines of non-code, and being blank line deletes,
they won't even show up as noise in git blame.Signed-off-by: Paul Gortmaker
27 Apr, 2012
9 commits
-
Adds check to ensure TIPC sockets reject incoming payload messages
that have an unrecognized message type.Remove the old open question about whether TIPC_ERR_NO_PORT is
the proper return value. It is appropriate here since there are
valid instances where another node can make use of the reply,
and at this point in time the host is already broadcasting TIPC
data, so there are no real security concerns.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Consolidates validation of scope and name sequence range values into
a single routine where it applies both to local name publications
and to name publications issued by other nodes in the network. This
change means that the scope value for non-local publications is now
validated and the name sequence range for local publications is now
validated only once. Additionally, a publication attempt that fails
validation now creates an entry in the system log file only if debugging
capabilities have been enabled; this prevents the system log from being
cluttered up with messages caused by a defective application or network
node.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Replaces two identical chunks of code that delete an unused name
sequence structure from TIPC's name table with calls to a new routine
that performs this operation.This change is cosmetic and doesn't impact the operation of TIPC.
Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Eliminate code to zero-out the main topology service structure,
which is already zeroed-out.Get rid of a comment documenting a field of the main topology
service structure that no longer exists.Both are cosmetic changes with no impact on runtime behaviour.
Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Initialization now occurs in the calling thread of control,
rather than being deferred to the TIPC tasklet. With the
current codebase, the deferral is no longer necessary.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Streamlines the job of re-initializing TIPC's network topology service
when a node's network address is first assigned. Rather than destroying
the topology server port and breaking its connections to existing
subscribers, TIPC now simply lets the service continue running (since
the change to the port identifier of each port used by the topology
service no longer impacts the flow of messages between the service and
its subscribers).This enhancement means that applications that utilize the topology
service prior to the assignment of TIPC's network address no longer need
to re-establish their subscriptions when the address is finally assigned.However, it is worth noting that any subsequent events for existing
subscriptions report the new port identifier of the publishing port,
rather than the original port identifier. (For example, a name that was
previously reported as being published by may be subsequently
withdrawn by .)This doesn't impact any of the existing known userspace in tipc-utils,
since (a) TIPC continues to treat references to the original port ID
correctly and (b) normal use cases assign an address before active use.However if there does happen to be some rare/custom application out
there that was relying on this, they can simply bypass the enhancement
by issuing a subscription to {0,0} and break its connection to the
topology service, if an associated withdrawal event occurs.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Termination no longer tests to see if the configuration service
port was successfully created or not. In the unlikely event that the
port was not created, attempting to delete the non-existent port is
detected gracefully and causes no harm.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Initialization now occurs in the calling thread of control,
rather than being deferred to the TIPC tasklet. With the
current codebase, the deferral is no longer necessary.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Streamlines the job of re-initializing TIPC's configuration service
when a node's network address is first assigned. Rather than destroying
the configuration server port and then recreating it, TIPC now simply
withdraws the existing {0,} name publication and creates a new
{0,} name publication that identifies the node's network address
to interested subscribers.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker
24 Apr, 2012
2 commits
-
Untie gcc's hands and let it do what it wants within the
individual source files. There are two files, node.c and
port.c -- only the latter effectively changes (gcc-4.5.2).
Objdump shows gcc deciding to not inline port_peernode().Suggested-by: David S. Miller
Signed-off-by: Paul Gortmaker
Signed-off-by: David S. Miller -
sk_add_backlog() & sk_rcvqueues_full() hard coded sk_rcvbuf as the
memory limit. We need to make this limit a parameter for TCP use.No functional change expected in this patch, all callers still using the
old sk_rcvbuf limit.Signed-off-by: Eric Dumazet
Cc: Neal Cardwell
Cc: Tom Herbert
Cc: Maciej Żenczykowski
Cc: Yuchung Cheng
Cc: Ilpo Järvinen
Cc: Rick Jones
Signed-off-by: David S. Miller
20 Apr, 2012
4 commits
-
Enhances command validation done by TIPC's configuration service so
that it works properly even if the node's network address is changed in
mid-operation. The default node address of is now recognized as an
alias for "this node" even after a new network address has been assigned.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Revises handling of a rejected message to ensure that a locally
originated message is returned properly even if the node's network
address is changed in mid-operation. The routine now treats the
default node address of as an alias for "this node" when
determining where to send a returned message.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
Revises handling of send routines for payload messages to ensure that
they are processed properly even if the node's network address is
changed in mid-operation. The routines now treat the default node
address of as an alias for "this node" when determining where
to send an outgoing message.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker -
There are two send routines that might conceivably be asked by an
application to send a message off-node when the node is still using
the default network address. These now have an added check that
detects this and rejects the message gracefully.Signed-off-by: Allan Stephens
Signed-off-by: Paul Gortmaker