* [v2 088/115] RFC: sysctl: convert read-write lock to RCU
From: Lucian Adrian Grijincu @ 2011-05-08 22:39 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Lucian Adrian Grijincu
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com>
Apologies to reviewers who will feel insulted reading this. This patch
is just for kicks - and by kicks I mean ass-kicks for such an awful
misuse of the RCU API. I haven't done anything with RCUs until now and
I'm very unsure about the sanity of this patch.
This patch replaces the reader-writer lock protected lists ctl_subdirs
and ctl_tables with RCU protected lists.
Unlike in the RCU sniplets I found, where the Reader part only read
data from the object - Updates were done on a separate Copy (RCU ...),
here readers do change some data in the list elements (data access
protected by a separate spin lock), but does not touch the list_head.
read-side:
- uses the for...rcu list traversal for DEC Alpha memory whatever
- rcu_read_(un)lock make sure the grace period is as long as needed
write-site:
- writers are synchronized with a spin-lock
- list adding/removing is done with list_add_tail_rcu/list_del_rcu
- freeing of elements is done after the grace period has ended (call_rcu)
Also note that there may be unwanted interactions with the RCU
protected VFS routines: ctl_table_header elements are scheduled to be
freed when all references to them have disappeared. This means after
removing the element from the list of at a later time (also with
call_rcu). I don't think that delaying free-ing some more would be a
problem, but I may be very wrong.
Free-ing of ctl_table_header is done with free_head. This is
scheduled to be called with call_rcu in two places:
- sysctl_proc_inode_put() called from the VFS by proc_evict_inode which uses
rcu_assign_pointer(PROC_I(inode)->sysctl, NULL)
to delete the VFS's last reference to the object
- unregister_sysctl_table (no connection to the VFS).
Each of them determines if all references to that object have
disappeared, and if so, schedule the object to be freed with call_rcu.
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
fs/proc/proc_sysctl.c | 8 ++++----
kernel/sysctl.c | 29 ++++++++++++-----------------
kernel/sysctl_check.c | 7 ++++---
3 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 9337149..b3e2453 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -81,7 +81,7 @@ retry:
sysctl_read_lock_head(head);
/* first check whether a subdirectory has the searched-for name */
- list_for_each_entry(h, &head->ctl_subdirs, ctl_entry) {
+ list_for_each_entry_rcu(h, &head->ctl_subdirs, ctl_entry) {
if (IS_ERR(sysctl_use_header(h)))
continue;
@@ -93,7 +93,7 @@ retry:
}
/* no subdir with that name, look for the file in the ctl_tables */
- list_for_each_entry(h, &head->ctl_tables, ctl_entry) {
+ list_for_each_entry_rcu(h, &head->ctl_tables, ctl_entry) {
if (IS_ERR(sysctl_use_header(h)))
continue;
@@ -234,7 +234,7 @@ static int scan(struct ctl_table_header *head,
sysctl_read_lock_head(head);
- list_for_each_entry(h, &head->ctl_subdirs, ctl_entry) {
+ list_for_each_entry_rcu(h, &head->ctl_subdirs, ctl_entry) {
if (*pos < file->f_pos) {
(*pos)++;
continue;
@@ -252,7 +252,7 @@ static int scan(struct ctl_table_header *head,
(*pos)++;
}
- list_for_each_entry(h, &head->ctl_tables, ctl_entry) {
+ list_for_each_entry_rcu(h, &head->ctl_tables, ctl_entry) {
ctl_table *t;
if (IS_ERR(sysctl_use_header(h)))
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 9e50334..26c2bc6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -56,7 +56,6 @@
#include <linux/kprobes.h>
#include <linux/pipe_fs_i.h>
#include <linux/oom.h>
-#include <linux/rwsem.h>
#include <asm/uaccess.h>
#include <asm/processor.h>
@@ -1616,30 +1615,25 @@ struct ctl_table_header *sysctl_use_netns_corresp(struct ctl_table_header *h)
}
-/* This semaphore protects the ctl_subdirs and ctl_tables lists. You
- * must also have incremented the _use_refs of the header before
- * accessing any field of the header including these lists. If it's
- * deemed necessary, we can create a per-header rwsem. For now a
- * global one will do. */
-static DECLARE_RWSEM(sysctl_rwsem);
+/* protection for the headers' ctl_subdirs/ctl_tables lists */
+static DEFINE_SPINLOCK(sysctl_list_lock);
void sysctl_write_lock_head(struct ctl_table_header *head)
{
- down_write(&sysctl_rwsem);
+ spin_lock(&sysctl_list_lock);
}
void sysctl_write_unlock_head(struct ctl_table_header *head)
{
- up_write(&sysctl_rwsem);
+ spin_unlock(&sysctl_list_lock);
}
void sysctl_read_lock_head(struct ctl_table_header *head)
{
- down_read(&sysctl_rwsem);
+ rcu_read_lock();
}
void sysctl_read_unlock_head(struct ctl_table_header *head)
{
- up_read(&sysctl_rwsem);
+ rcu_read_unlock();
}
-
/*
* sysctl_perm does NOT grant the superuser all rights automatically, because
* some sysctl variables are readonly even to root.
@@ -1777,6 +1771,7 @@ static struct ctl_table_header *alloc_sysctl_header(struct ctl_table_group *grou
h->ctl_table_arg = NULL;
h->unregistering = NULL;
h->ctl_group = group;
+ INIT_LIST_HEAD(&h->ctl_entry);
return h;
}
@@ -1788,7 +1783,7 @@ static struct ctl_table_header *mkdir_existing_dir(struct ctl_table_header *pare
const char *name)
{
struct ctl_table_header *h;
- list_for_each_entry(h, &parent->ctl_subdirs, ctl_entry) {
+ list_for_each_entry_rcu(h, &parent->ctl_subdirs, ctl_entry) {
spin_lock(&sysctl_lock);
if (likely(!h->unregistering)) {
if (strcmp(name, h->ctl_dirname) == 0) {
@@ -1844,7 +1839,7 @@ static struct ctl_table_header *mkdir_new_dir(struct ctl_table_header *parent,
{
dir->parent = parent;
header_refs_inc(dir);
- list_add_tail(&dir->ctl_entry, &parent->ctl_subdirs);
+ list_add_tail_rcu(&dir->ctl_entry, &parent->ctl_subdirs);
return dir;
}
@@ -2049,7 +2044,7 @@ struct ctl_table_header *__register_sysctl_paths(struct ctl_table_group *group,
failed_duplicate_check = sysctl_check_duplicates(header);
#endif
if (!failed_duplicate_check)
- list_add_tail(&header->ctl_entry, &header->parent->ctl_tables);
+ list_add_tail_rcu(&header->ctl_entry, &header->parent->ctl_tables);
sysctl_write_unlock_head(header->parent);
@@ -2119,14 +2114,14 @@ void unregister_sysctl_table(struct ctl_table_header *header)
* specific ctl_table_group list. For not that
* list is protected by sysctl_lock. */
spin_lock(&sysctl_lock);
- list_del_init(&header->ctl_entry);
+ list_del_rcu(&header->ctl_entry);
spin_unlock(&sysctl_lock);
} else {
/* ctl_entry is a member of the parent's
* ctl_tables/subdirs lists which are
* protected by the parent's write lock. */
sysctl_write_lock_head(parent);
- list_del_init(&header->ctl_entry);
+ list_del_rcu(&header->ctl_entry);
sysctl_write_unlock_head(parent);
}
diff --git a/kernel/sysctl_check.c b/kernel/sysctl_check.c
index 55e797a..b9573e0 100644
--- a/kernel/sysctl_check.c
+++ b/kernel/sysctl_check.c
@@ -1,5 +1,6 @@
#include <linux/sysctl.h>
#include <linux/string.h>
+#include <linux/rculist.h>
/*
* @path: the path to the offender
@@ -124,7 +125,7 @@ int sysctl_check_duplicates(struct ctl_table_header *header)
struct ctl_table_header *dir = header->parent;
struct ctl_table_header *h;
- list_for_each_entry(h, &dir->ctl_subdirs, ctl_entry) {
+ list_for_each_entry_rcu(h, &dir->ctl_subdirs, ctl_entry) {
if (IS_ERR(sysctl_use_header(h)))
continue;
@@ -136,7 +137,7 @@ int sysctl_check_duplicates(struct ctl_table_header *header)
sysctl_unuse_header(h);
}
- list_for_each_entry(h, &dir->ctl_tables, ctl_entry) {
+ list_for_each_entry_rcu(h, &dir->ctl_tables, ctl_entry) {
ctl_table *t;
if (IS_ERR(sysctl_use_header(h)))
@@ -188,7 +189,7 @@ int sysctl_check_netns_correspondents(struct ctl_table_header *header,
/* see if the netns_correspondent has a subdir
* with the same as this non-netns specific header */
sysctl_read_lock_head(netns_corresp);
- list_for_each_entry(h, &netns_corresp->ctl_subdirs, ctl_entry) {
+ list_for_each_entry_rcu(h, &netns_corresp->ctl_subdirs, ctl_entry) {
if (IS_ERR(sysctl_use_header(h)))
continue;
if (strcmp(header->ctl_dirname, h->ctl_dirname) == 0) {
--
1.7.5.134.g1c08b
^ permalink raw reply related
* [v2 089/115] RFC: sysctl: change type of ctl_procfs_refs to u8
From: Lucian Adrian Grijincu @ 2011-05-08 22:39 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Lucian Adrian Grijincu
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com>
255 files per registered header should be enough for everyone.
If not, either:
- register another header (and another, and another) each with max 255 files
- change the type of ctl_procfs_refs to something bigger (e.g. u16)
This patch makes two assumptions:
- there will be at max a single inode created for each sysctl
file. That means that the ctl_table_header will be (at max)
incremented once for each of it's files. For directories the counter
will be incremented only once (when creating an inode for the
directory itself).
- there are no sysctl tables in the kernel with more than 255 entries.
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
fs/proc/proc_sysctl.c | 12 +++++++++---
include/linux/sysctl.h | 11 +++++++----
kernel/sysctl.c | 10 +++++++++-
kernel/sysctl_check.c | 12 ++++++++++++
4 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index b3e2453..9580794 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -20,13 +20,15 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
struct inode *inode;
struct proc_inode *ei;
+ if (sysctl_proc_inode_get(head))
+ goto err_get;
+
inode = new_inode(sb);
if (!inode)
- goto out;
+ goto err_new_inode;
inode->i_ino = get_next_ino();
- sysctl_proc_inode_get(head);
ei = PROC_I(inode);
ei->sysctl = head;
ei->sysctl_entry = table;
@@ -44,8 +46,12 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
inode->i_op = &proc_sys_dir_operations;
inode->i_fop = &proc_sys_dir_file_operations;
}
-out:
return inode;
+
+err_new_inode:
+ sysctl_proc_inode_put(head);
+err_get:
+ return NULL;
}
static struct ctl_table *find_in_table(struct ctl_table *p, struct qstr *name)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 036d1aa..d5d9b66f 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -947,7 +947,7 @@ extern void sysctl_init_group(struct ctl_table_group *group,
/* get/put a reference to this header that
* will be/was embedded in a procfs proc_inode */
-extern void sysctl_proc_inode_get(struct ctl_table_header *);
+extern int sysctl_proc_inode_get(struct ctl_table_header *);
extern void sysctl_proc_inode_put(struct ctl_table_header *);
extern int sysctl_is_seen(struct ctl_table_header *);
@@ -1059,13 +1059,16 @@ struct ctl_table_header {
/* references to this header from contexts that
* can access fields of this header */
int ctl_use_refs;
- /* references to this header from procfs inodes.
- * procfs embeds a pointer to the header in proc_inode */
- int ctl_procfs_refs;
/* counts references to this header from other
* headers (through ->parent) plus the reference
* returned by __register_sysctl_paths */
int ctl_header_refs;
+ /* references to this header from procfs inodes.
+ * procfs embeds a pointer to the header in proc_inode.
+ * If there's at max one inode created per file then
+ * the max value of this is the number of files in the
+ * ctl_table array, or 1 for directories. */
+ u8 ctl_procfs_refs;
};
struct rcu_head rcu;
};
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 26c2bc6..3e30e78 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1546,11 +1546,19 @@ static void start_unregistering(struct ctl_table_header *p)
}
}
-void sysctl_proc_inode_get(struct ctl_table_header *head)
+int sysctl_proc_inode_get(struct ctl_table_header *head)
{
+ int err = 0;
spin_lock(&sysctl_lock);
head->ctl_procfs_refs++;
+ if (unlikely(head->ctl_procfs_refs == 0)) {
+ /* restore old value */
+ head->ctl_procfs_refs--;
+ err = 1;
+ WARN(head->ctl_procfs_refs == 0, "sysctl: ctl_procfs_refs overflow");
+ }
spin_unlock(&sysctl_lock);
+ return err;
}
static void free_head(struct rcu_head *rcu)
diff --git a/kernel/sysctl_check.c b/kernel/sysctl_check.c
index b9573e0..205f721 100644
--- a/kernel/sysctl_check.c
+++ b/kernel/sysctl_check.c
@@ -29,6 +29,8 @@ int sysctl_check_table(const struct ctl_path *path,
struct ctl_table *table)
{
struct ctl_table *t;
+ unsigned int max_bits, max_files;
+ unsigned int nr_files = 0;
int error = 0;
if (nr_dirs > CTL_MAXNAME - 1) {
@@ -37,6 +39,7 @@ int sysctl_check_table(const struct ctl_path *path,
}
for(t = table; t->procname; t++) {
+ nr_files ++;
if ((t->proc_handler == proc_dostring) ||
(t->proc_handler == proc_dointvec) ||
(t->proc_handler == proc_dointvec_minmax) ||
@@ -58,6 +61,15 @@ int sysctl_check_table(const struct ctl_path *path,
FAIL("bogus .mode");
}
+ /* make sure we can increment the header's ctl_procfs_refs
+ * counter for each file in the table. If this fails we either
+ * need to change the type of the ctl_procfs_refs variable, or
+ * register more tables in the same directory. */
+ max_bits = 8 * sizeof(((struct ctl_table_header *) 0)->ctl_procfs_refs);
+ max_files = 1 << max_bits;
+ if (nr_files >= max_files)
+ FAIL("too many files in registered table");
+
if (error)
dump_stack();
--
1.7.5.134.g1c08b
^ permalink raw reply related
* [v2 090/115] sysctl: warn if registration/unregistration order is not respected
From: Lucian Adrian Grijincu @ 2011-05-08 22:39 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Lucian Adrian Grijincu
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com>
This patch sends a warning for each sysctl unregistration that cannot
delete all the directories that it created.
For example:
- register /existingdir/newdir/file-a
- register /existingdir/newdir/dir3/file-b
- unregister /existingdir/newdir/file-a
- unregister /existingdir/newdir/dir3/file-b
Here the order is violated because the first unregister operation
cannot delete all the directories it has created (namely 'newdir')
because they are used by another registered path.
This rule violation can be fixed in (at least) two ways:
- enforce order of unregistration:
- register /existingdir/newdir/file-a
- register /existingdir/newdir/dir3/file-b
- unregister /existingdir/newdir/dir3/file-b
- unregister /existingdir/newdir/file-a
- have a third party register the common part:
- register /existingdir/newdir/
- register /existingdir/newdir/file-a
- register /existingdir/newdir/dir3/file-b
- unregister /existingdir/newdir/file-a
- unregister /existingdir/newdir/dir3/file-b
- unregister /existingdir/newdir/
The current implementation works well regardless of this order being
respected. In the future, other sysctl implementations may only work
if this rule is respected.
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
include/linux/sysctl.h | 15 ++++++++++++---
kernel/sysctl.c | 39 +++++++++++++++++++++++++++++++++++----
2 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index d5d9b66f..322246d 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -1036,12 +1036,14 @@ struct ctl_table_group_ops {
};
struct ctl_table_group {
+ /* has initialization for this group finished? */
+ int is_initialized:1;
+ /* does this group use the @corresp_list? */
+ int has_netns_corresp:1;
+ struct list_head corresp_list;
const struct ctl_table_group_ops *ctl_ops;
/* A list of ctl_table_header elements that represent the
* netns-specific correspondents of some sysctl directories */
- struct list_head corresp_list;
- /* binary: whether this group uses the @corresp_list */
- char has_netns_corresp;
};
/* struct ctl_table_header is used to maintain dynamic lists of
@@ -1069,6 +1071,13 @@ struct ctl_table_header {
* the max value of this is the number of files in the
* ctl_table array, or 1 for directories. */
u8 ctl_procfs_refs;
+ /* how many dirs were created when this header was
+ * registered. Rule: the header which created a directory
+ * should be the one that deletes it. This counter is
+ * used to signal violations of this rule. The counter's
+ * max value is CTL_MAXNAME (currently=10) so we use
+ * only 4 bits of the 8 available. */
+ u8 ctl_owned_dirs_refs;
};
struct rcu_head rcu;
};
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 3e30e78..94fff4e 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -204,8 +204,9 @@ static struct kmem_cache *sysctl_header_cachep;
static const struct ctl_table_group_ops root_table_group_ops = { };
static struct ctl_table_group root_table_group = {
- .has_netns_corresp = 0,
- .ctl_ops = &root_table_group_ops,
+ .is_initialized = 1,
+ .has_netns_corresp = 0,
+ .ctl_ops = &root_table_group_ops,
};
static struct ctl_table_header root_table_header = {
@@ -1617,6 +1618,14 @@ out:
struct ctl_table_header *sysctl_use_netns_corresp(struct ctl_table_header *h)
{
struct ctl_table_group *g = ¤t->nsproxy->net_ns->netns_ctl_group;
+
+ /* this function may be called to check whether the
+ * netns-specific vs. non-netns-specific registration order is
+ * respected. Those checks may be done early during init when
+ * nor init_net is not initialized, nor it's netns-specific group. */
+ if (!g->is_initialized)
+ return NULL;
+
/* dflt == NULL means: if there's a netns corresp return it,
* if there isn't, just return NULL */
return sysctl_use_netns_corresp_dflt(g, h, NULL);
@@ -1869,13 +1878,14 @@ static struct ctl_table_header *mkdir_new_dir(struct ctl_table_header *parent,
static struct ctl_table_header *sysctl_mkdirs(struct ctl_table_header *parent,
struct ctl_table_group *group,
const struct ctl_path *path,
- int nr_dirs)
+ int nr_dirs, int *p_dirs_created)
{
struct ctl_table_header *dirs[CTL_MAXNAME];
struct ctl_table_header *__netns_corresp = NULL;
int create_first_netns_corresp = group->has_netns_corresp;
int i;
+ *p_dirs_created = 0;
/* We create excess ctl_table_header for directory entries.
* We do so because we may need new headers while under a lock
* where we will not be able to allocate entries (sleeping).
@@ -1929,6 +1939,7 @@ static struct ctl_table_header *sysctl_mkdirs(struct ctl_table_header *parent,
goto err_check_netns_correspondents;
}
#endif
+ (*p_dirs_created)++;
continue;
}
@@ -1945,8 +1956,12 @@ static struct ctl_table_header *sysctl_mkdirs(struct ctl_table_header *parent,
if (create_first_netns_corresp)
parent = mkdir_netns_corresp(parent, group, &__netns_corresp);
+ /* if mkdir_netns_corresp used it, it's NULL */
if (__netns_corresp)
kmem_cache_free(sysctl_header_cachep, __netns_corresp);
+ else
+ (*p_dirs_created)++;
+
/* free unused pre-allocated entries */
for (i = 0; i < nr_dirs; i++)
@@ -2027,6 +2042,7 @@ struct ctl_table_header *__register_sysctl_paths(struct ctl_table_group *group,
struct ctl_table_header *header;
int failed_duplicate_check = 0;
int nr_dirs = ctl_path_items(path);
+ int dirs_created = 0;
#ifdef CONFIG_SYSCTL_SYSCALL_CHECK
if (sysctl_check_table(path, nr_dirs, table))
@@ -2037,7 +2053,8 @@ struct ctl_table_header *__register_sysctl_paths(struct ctl_table_group *group,
if (!header)
return NULL;
- header->parent = sysctl_mkdirs(&root_table_header, group, path, nr_dirs);
+ header->parent = sysctl_mkdirs(&root_table_header, group, path,
+ nr_dirs, &dirs_created);
if (!header->parent) {
kmem_cache_free(sysctl_header_cachep, header);
return NULL;
@@ -2045,6 +2062,7 @@ struct ctl_table_header *__register_sysctl_paths(struct ctl_table_group *group,
header->ctl_table_arg = table;
header->ctl_header_refs = 1;
+ header->ctl_owned_dirs_refs = dirs_created;
sysctl_write_lock_head(header->parent);
@@ -2089,6 +2107,7 @@ struct ctl_table_header *register_sysctl_paths(const struct ctl_path *path,
*/
void unregister_sysctl_table(struct ctl_table_header *header)
{
+ int dirs_to_delete = header->ctl_owned_dirs_refs;
might_sleep();
while(header->parent) {
@@ -2098,6 +2117,13 @@ void unregister_sysctl_table(struct ctl_table_header *header)
* and ctl_use_refs) are protected by the spin lock. */
spin_lock(&sysctl_lock);
if (header->ctl_header_refs > 1) {
+ if (WARN(dirs_to_delete != 0, "directory that we "
+ "created is still used by another header.")) {
+ /* if one element of the path is still used it's
+ * parents will be too. Stop sending warnings */
+ dirs_to_delete = 0;
+ }
+
/* other headers need a reference to this one. Just
* mark that we don't need it and leave it as it is. */
header->ctl_header_refs --;
@@ -2116,6 +2142,10 @@ void unregister_sysctl_table(struct ctl_table_header *header)
start_unregistering(header);
spin_unlock(&sysctl_lock);
+ /* don't go negative */
+ if (dirs_to_delete)
+ dirs_to_delete --;
+
if (!header->ctl_dirname) {
/* the header is a netns correspondent of it's
* parent. It is a member of it's netns
@@ -2173,6 +2203,7 @@ void sysctl_init_group(struct ctl_table_group *group,
group->has_netns_corresp = has_netns_corresp;
if (has_netns_corresp)
INIT_LIST_HEAD(&group->corresp_list);
+ group->is_initialized = 1;
}
#else /* !CONFIG_SYSCTL */
--
1.7.5.134.g1c08b
^ permalink raw reply related
* [v2 093/115] sysctl: ax25: create empty dir with register_sysctl_dir
From: Lucian Adrian Grijincu @ 2011-05-08 22:39 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Lucian Adrian Grijincu
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com>
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
net/ax25/af_ax25.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 965662d..d8a4ea4 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1996,7 +1996,6 @@ static const struct __initdata ctl_path ax25_path[] = {
{ .procname = "ax25" },
{ }
};
-static struct ctl_table empty;
static struct ctl_table_header *ax25_root_header;
#endif /* CONFIG_SYSCTL */
@@ -2014,7 +2013,7 @@ static int __init ax25_init(void)
/* XXX: no error checking done in initializer */
#ifdef CONFIG_SYSCTL
- ax25_root_header = register_sysctl_paths(ax25_path, &empty);
+ ax25_root_header = register_sysctl_dir(ax25_path);
#endif
proc_net_fops_create(&init_net, "ax25_route", S_IRUGO, &ax25_route_fops);
--
1.7.5.134.g1c08b
^ permalink raw reply related
* [v2 094/115] sysctl: net/core: create empty dir with register_sysctl_dir
From: Lucian Adrian Grijincu @ 2011-05-08 22:39 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Lucian Adrian Grijincu
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com>
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
net/core/sysctl_net_core.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 385b609..6d2fe6e 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -239,9 +239,7 @@ static __net_initdata struct pernet_operations sysctl_core_ops = {
static __init int sysctl_core_init(void)
{
- static struct ctl_table empty[1];
-
- register_sysctl_paths(net_core_path, empty);
+ register_sysctl_dir(net_core_path);
register_net_sysctl_rotable(net_core_path, net_core_table);
return register_pernet_subsys(&sysctl_core_ops);
}
--
1.7.5.134.g1c08b
^ permalink raw reply related
* [v2 095/115] sysctl: net/ipv4/neigh: create empty dir with register_sysctl_dir
From: Lucian Adrian Grijincu @ 2011-05-08 22:39 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, Lucian Adrian Grijincu
In-Reply-To: <1304894407-32201-1-git-send-email-lucian.grijincu@gmail.com>
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
net/ipv4/route.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 46c7b3d..092f3d1 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -3151,8 +3151,6 @@ static ctl_table ipv4_route_table[] = {
{ }
};
-static struct ctl_table empty[1];
-
static __net_initdata struct ctl_path ipv4_neigh_path[] = {
{ .procname = "net", },
{ .procname = "ipv4", },
@@ -3310,6 +3308,6 @@ int __init ip_rt_init(void)
void __init ip_static_sysctl_init(void)
{
register_sysctl_paths(ipv4_route_path, ipv4_route_table);
- register_sysctl_paths(ipv4_neigh_path, empty);
+ register_sysctl_dir(ipv4_neigh_path);
}
#endif
--
1.7.5.134.g1c08b
^ permalink raw reply related
* Re: [PATCH net-next] ethtool: Add 20G bit definitions
From: David Miller @ 2011-05-08 22:43 UTC (permalink / raw)
To: yanivr; +Cc: netdev, eilong, bhutchings
In-Reply-To: <1304407809.2518.1.camel@lb-tlvb-dmitry>
From: "Yaniv Rosner" <yanivr@broadcom.com>
Date: Tue, 3 May 2011 10:30:08 +0300
> Add 20G supported and advertising bit definitions.
> 20G will be supported with the 57840 chips.
>
>
> Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Applied to net-next-2.6, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipheth: Properly distinguish length and alignment in URBs and skbs
From: David Miller @ 2011-05-08 22:46 UTC (permalink / raw)
To: bhutchings
Cc: david.hill, agimenez, linux-kernel, dgiagio, dborca, pmcenery,
linux-usb, netdev
In-Reply-To: <1304444965.2873.11.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 03 May 2011 18:49:25 +0100
> The USB protocol this driver implements appears to require 2 bytes of
> padding in front of each received packet. This used to be equal to
> the value of NET_IP_ALIGN on x86, so the driver abused that constant
> and mostly worked, but this is no longer the case. The driver also
> mixed up the URB and packet lengths, resulting in 2 bytes of junk at
> the end of the skb.
>
> Introduce a private constant for the 2 bytes of padding; fix this
> confusion and check for the under-length case.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> Compile-tested only, as I'm not cool enough for an iPhone either.
> This is applicable to net-next-2.6 or v2.6.38.
I've applied this to net-2.6 and will conditionally queue it up for
-stable, if we need further fixups we can add relative patches.
Thanks.
^ permalink raw reply
* Re: [PATCH] pktgen: use %pI6c for printing IPv6 addresses
From: David Miller @ 2011-05-08 22:50 UTC (permalink / raw)
To: joe; +Cc: adobriyan, netdev
In-Reply-To: <1304460714.1788.40.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Tue, 03 May 2011 15:11:54 -0700
> On Wed, 2011-05-04 at 00:23 +0300, Alexey Dobriyan wrote:
>> I don't know why %pI6 doesn't compress, but the format specifier is
>> kernel-standard, so use it.
>
> Because seq_printf output using %pI6 has a known
> output style and shouldn't be changed to avoid
> breaking user applications.
I've applied Alexey's patch to net-next-2.6
^ permalink raw reply
* Re: [PATCH] tcp_cubic: limit delayed_ack ratio to prevent divide error
From: David Miller @ 2011-05-08 22:52 UTC (permalink / raw)
To: shemminger
Cc: sangtae.ha, injongrhee, Valdis.Kletnieks, rdunlap, lkml, netdev,
linux-kernel
In-Reply-To: <20110504130456.425dee68@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 4 May 2011 13:04:56 -0700
> TCP Cubic keeps a metric that estimates the amount of delayed
> acknowledgements to use in adjusting the window. If an abnormally
> large number of packets are acknowledged at once, then the update
> could wrap and reach zero. This kind of ACK could only
> happen when there was a large window and huge number of
> ACK's were lost.
>
> This patch limits the value of delayed ack ratio. The choice of 32
> is just a conservative value since normally it should be range of
> 1 to 4 packets.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied, thanks Stephen.
^ permalink raw reply
* Re: [PATCHv2] forcedeth: Allow ethtool to enable/disable loopback.
From: David Miller @ 2011-05-08 22:57 UTC (permalink / raw)
To: maheshb; +Cc: netdev, therbert
In-Reply-To: <1304559400-16257-1-git-send-email-maheshb@google.com>
From: Mahesh Bandewar <maheshb@google.com>
Date: Wed, 4 May 2011 18:36:40 -0700
> @@ -4502,6 +4549,9 @@ static int nv_set_features(struct net_device *dev, u32 features)
>
> spin_unlock_irq(&np->lock);
> }
> + if ((changed & NETIF_F_LOOPBACK) && netif_running(dev)) {
> + nv_set_loopback(dev, features);
> + }
>
Please do not create a braced basic block just for a single
line of code. Simply:
> + if ((changed & NETIF_F_LOOPBACK) && netif_running(dev))
> + nv_set_loopback(dev, features);
is sufficient.
^ permalink raw reply
* Re: [PATCHv2 1/2] net: Allow ethtool to set interface in loopback mode.
From: David Miller @ 2011-05-08 23:00 UTC (permalink / raw)
To: bhutchings; +Cc: maheshb, netdev, mirq-linux, therbert
In-Reply-To: <1304560369.3203.44.camel@localhost>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 05 May 2011 02:52:48 +0100
> On Wed, 2011-05-04 at 18:30 -0700, Mahesh Bandewar wrote:
>> This patch enables ethtool to set the loopback mode on a given interface.
>> By configuring the interface in loopback mode in conjunction with a policy
>> route / rule, a userland application can stress the egress / ingress path
>> exposing the flows of the change in progress and potentially help developer(s)
>> understand the impact of those changes without even sending a packet out
>> on the network.
>>
>> Following set of commands illustrates one such example -
>> a) ip -4 addr add 192.168.1.1/24 dev eth1
>> b) ip -4 rule add from all iif eth1 lookup 250
>> c) ip -4 route add local 0/0 dev lo proto kernel scope host table 250
>> d) arp -Ds 192.168.1.100 eth1
>> e) arp -Ds 192.168.1.200 eth1
>> f) sysctl -w net.ipv4.ip_nonlocal_bind=1
>> g) sysctl -w net.ipv4.conf.all.accept_local=1
>> # Assuming that the machine has 8 cores
>> h) taskset 000f netserver -L 192.168.1.200
>> i) taskset 00f0 netperf -t TCP_CRR -L 192.168.1.100 -H 192.168.1.200 -l 30
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> Acked-by: Ben Hutchings <bhutchings@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Håkon Løvdal @ 2011-05-08 23:08 UTC (permalink / raw)
To: matt mooney
Cc: Joe Perches, aquini, kernel-janitors, David Miller, Jay Vosburgh,
Andy Gospodarek, shemminger, netdev, Nicolas Kaiser
In-Reply-To: <BANLkTimKs39m_htMFEygR+3Bfu7Butc-zQ@mail.gmail.com>
On 7 May 2011 21:35, matt mooney <mfmooney@gmail.com> wrote:
> But isn't the preferred style to have a single exit point?
This is generally considered to be a bad advice, see
http://stackoverflow.com/questions/1701686/why-should-methods-have-a-single-entry-and-exit-points/1701721#1701721
for instance.
BR Håkon Løvdal
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: David Miller @ 2011-05-08 23:10 UTC (permalink / raw)
To: hlovdal
Cc: mfmooney, joe, aquini, kernel-janitors, fubar, andy, shemminger,
netdev, nikai
In-Reply-To: <BANLkTik_gMCHYp4sOnzzR1Gs=bn2pkpjfQ@mail.gmail.com>
From: Håkon Løvdal <hlovdal@gmail.com>
Date: Mon, 9 May 2011 01:08:44 +0200
> On 7 May 2011 21:35, matt mooney <mfmooney@gmail.com> wrote:
>> But isn't the preferred style to have a single exit point?
>
> This is generally considered to be a bad advice, see
> http://stackoverflow.com/questions/1701686/why-should-methods-have-a-single-entry-and-exit-points/1701721#1701721
> for instance.
That article totally ignores the issue of locking and how hard it is
to get right without single exit points, and how unlocking in
multiple spots bloats up the code.
Definitely don't take that article's advice when working on the
kernel.
^ permalink raw reply
* [GIT PULL nf-2.6] IPVS (take III)
From: Simon Horman @ 2011-05-08 23:17 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman
please consider pulling
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-2.6.git for-nf-2.6
to get the following fix from Hans. They resolve some problems related
to his netns for IPVS work which was incorporated into 2.6.39-rc1.
The pull request is based on nf-2.6/master.
There are other less-pressing changes from Hans which
I plan to get you to pull into nf-next-2.6 once these
changes make it there (presumably via net-2.6 and then net-next-2.6).
There are minor changes since take II.
Hans Schillstrom (2):
IPVS: Change of socket usage to enable name space exit.
IPVS: init and cleanup restructuring
include/net/ip_vs.h | 17 +++++
net/netfilter/ipvs/ip_vs_app.c | 15 +----
net/netfilter/ipvs/ip_vs_conn.c | 12 +---
net/netfilter/ipvs/ip_vs_core.c | 103 +++++++++++++++++++++++++++++---
net/netfilter/ipvs/ip_vs_ctl.c | 120 ++++++++++++++++++++++++++++++++------
net/netfilter/ipvs/ip_vs_est.c | 14 +----
net/netfilter/ipvs/ip_vs_proto.c | 11 +---
net/netfilter/ipvs/ip_vs_sync.c | 65 ++++++++++++---------
8 files changed, 258 insertions(+), 99 deletions(-)
^ permalink raw reply
* [PATCH 1/2] IPVS: Change of socket usage to enable name space exit.
From: Simon Horman @ 2011-05-08 23:17 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman,
Simon Horman
In-Reply-To: <1304896671-29384-1-git-send-email-horms@verge.net.au>
From: Hans Schillstrom <hans@schillstrom.com>
If the sync daemons run in a name space while it crashes
or get killed, there is no way to stop them except for a reboot.
When all patches are there, ip_vs_core will handle register_pernet_(),
i.e. ip_vs_sync_init() and ip_vs_sync_cleanup() will be removed.
Kernel threads should not increment the use count of a socket.
By calling sk_change_net() after creating a socket this is avoided.
sock_release cant be used intead sk_release_kernel() should be used.
Thanks Eric W Biederman for your advices.
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
[horms@verge.net.au: minor edit to changelog]
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 2 +-
net/netfilter/ipvs/ip_vs_sync.c | 58 +++++++++++++++++++++++++--------------
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 07accf6..a0791dc 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1896,7 +1896,7 @@ static int __net_init __ip_vs_init(struct net *net)
static void __net_exit __ip_vs_cleanup(struct net *net)
{
- IP_VS_DBG(10, "ipvs netns %d released\n", net_ipvs(net)->gen);
+ IP_VS_DBG(2, "ipvs netns %d released\n", net_ipvs(net)->gen);
}
static struct pernet_operations ipvs_core_ops = {
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 3e7961e..0cce953 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1303,13 +1303,18 @@ static struct socket *make_send_sock(struct net *net)
struct socket *sock;
int result;
- /* First create a socket */
- result = __sock_create(net, PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock, 1);
+ /* First create a socket move it to right name space later */
+ result = sock_create_kern(PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
if (result < 0) {
pr_err("Error during creation of socket; terminating\n");
return ERR_PTR(result);
}
-
+ /*
+ * Kernel sockets that are a part of a namespace, should not
+ * hold a reference to a namespace in order to allow to stop it.
+ * After sk_change_net should be released using sk_release_kernel.
+ */
+ sk_change_net(sock->sk, net);
result = set_mcast_if(sock->sk, ipvs->master_mcast_ifn);
if (result < 0) {
pr_err("Error setting outbound mcast interface\n");
@@ -1334,8 +1339,8 @@ static struct socket *make_send_sock(struct net *net)
return sock;
- error:
- sock_release(sock);
+error:
+ sk_release_kernel(sock->sk);
return ERR_PTR(result);
}
@@ -1350,12 +1355,17 @@ static struct socket *make_receive_sock(struct net *net)
int result;
/* First create a socket */
- result = __sock_create(net, PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock, 1);
+ result = sock_create_kern(PF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
if (result < 0) {
pr_err("Error during creation of socket; terminating\n");
return ERR_PTR(result);
}
-
+ /*
+ * Kernel sockets that are a part of a namespace, should not
+ * hold a reference to a namespace in order to allow to stop it.
+ * After sk_change_net should be released using sk_release_kernel.
+ */
+ sk_change_net(sock->sk, net);
/* it is equivalent to the REUSEADDR option in user-space */
sock->sk->sk_reuse = 1;
@@ -1377,8 +1387,8 @@ static struct socket *make_receive_sock(struct net *net)
return sock;
- error:
- sock_release(sock);
+error:
+ sk_release_kernel(sock->sk);
return ERR_PTR(result);
}
@@ -1473,7 +1483,7 @@ static int sync_thread_master(void *data)
ip_vs_sync_buff_release(sb);
/* release the sending multicast socket */
- sock_release(tinfo->sock);
+ sk_release_kernel(tinfo->sock->sk);
kfree(tinfo);
return 0;
@@ -1513,7 +1523,7 @@ static int sync_thread_backup(void *data)
}
/* release the sending multicast socket */
- sock_release(tinfo->sock);
+ sk_release_kernel(tinfo->sock->sk);
kfree(tinfo->buf);
kfree(tinfo);
@@ -1601,7 +1611,7 @@ outtinfo:
outbuf:
kfree(buf);
outsocket:
- sock_release(sock);
+ sk_release_kernel(sock->sk);
out:
return result;
}
@@ -1610,6 +1620,7 @@ out:
int stop_sync_thread(struct net *net, int state)
{
struct netns_ipvs *ipvs = net_ipvs(net);
+ int retc = -EINVAL;
IP_VS_DBG(7, "%s(): pid %d\n", __func__, task_pid_nr(current));
@@ -1629,7 +1640,7 @@ int stop_sync_thread(struct net *net, int state)
spin_lock_bh(&ipvs->sync_lock);
ipvs->sync_state &= ~IP_VS_STATE_MASTER;
spin_unlock_bh(&ipvs->sync_lock);
- kthread_stop(ipvs->master_thread);
+ retc = kthread_stop(ipvs->master_thread);
ipvs->master_thread = NULL;
} else if (state == IP_VS_STATE_BACKUP) {
if (!ipvs->backup_thread)
@@ -1639,16 +1650,14 @@ int stop_sync_thread(struct net *net, int state)
task_pid_nr(ipvs->backup_thread));
ipvs->sync_state &= ~IP_VS_STATE_BACKUP;
- kthread_stop(ipvs->backup_thread);
+ retc = kthread_stop(ipvs->backup_thread);
ipvs->backup_thread = NULL;
- } else {
- return -EINVAL;
}
/* decrease the module use count */
ip_vs_use_count_dec();
- return 0;
+ return retc;
}
/*
@@ -1670,8 +1679,15 @@ static int __net_init __ip_vs_sync_init(struct net *net)
static void __ip_vs_sync_cleanup(struct net *net)
{
- stop_sync_thread(net, IP_VS_STATE_MASTER);
- stop_sync_thread(net, IP_VS_STATE_BACKUP);
+ int retc;
+
+ retc = stop_sync_thread(net, IP_VS_STATE_MASTER);
+ if (retc && retc != -ESRCH)
+ pr_err("Failed to stop Master Daemon\n");
+
+ retc = stop_sync_thread(net, IP_VS_STATE_BACKUP);
+ if (retc && retc != -ESRCH)
+ pr_err("Failed to stop Backup Daemon\n");
}
static struct pernet_operations ipvs_sync_ops = {
@@ -1682,10 +1698,10 @@ static struct pernet_operations ipvs_sync_ops = {
int __init ip_vs_sync_init(void)
{
- return register_pernet_subsys(&ipvs_sync_ops);
+ return register_pernet_device(&ipvs_sync_ops);
}
void ip_vs_sync_cleanup(void)
{
- unregister_pernet_subsys(&ipvs_sync_ops);
+ unregister_pernet_device(&ipvs_sync_ops);
}
--
1.7.4.4
^ permalink raw reply related
* [PATCH 2/2] IPVS: init and cleanup restructuring
From: Simon Horman @ 2011-05-08 23:17 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy,
Hans Schillstrom, Hans Schillstrom, Eric W. Biederman,
Simon Horman
In-Reply-To: <1304896671-29384-1-git-send-email-horms@verge.net.au>
From: Hans Schillstrom <hans@schillstrom.com>
DESCRIPTION
This patch tries to restore the initial init and cleanup
sequences that was before namspace patch.
Netns also requires action when net devices unregister
which has never been implemented. I.e this patch also
covers when a device moves into a network namespace,
and has to be released.
IMPLEMENTATION
The number of calls to register_pernet_device have been
reduced to one for the ip_vs.ko
Schedulers still have their own calls.
This patch adds a function __ip_vs_service_cleanup()
and an enable flag for the netfilter hooks.
The nf hooks will be enabled when the first service is loaded
and never disabled again, except when a namespace exit starts.
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
[horms@verge.net.au: minor edit to changelog]
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 17 +++++
net/netfilter/ipvs/ip_vs_app.c | 15 +----
net/netfilter/ipvs/ip_vs_conn.c | 12 +---
net/netfilter/ipvs/ip_vs_core.c | 101 +++++++++++++++++++++++++++++---
net/netfilter/ipvs/ip_vs_ctl.c | 120 ++++++++++++++++++++++++++++++++------
net/netfilter/ipvs/ip_vs_est.c | 14 +----
net/netfilter/ipvs/ip_vs_proto.c | 11 +---
net/netfilter/ipvs/ip_vs_sync.c | 13 +---
8 files changed, 223 insertions(+), 80 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 4d1b71a..02f6702 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -791,6 +791,7 @@ struct ip_vs_app {
/* IPVS in network namespace */
struct netns_ipvs {
int gen; /* Generation */
+ int enable; /* enable like nf_hooks do */
/*
* Hash table: for real service lookups
*/
@@ -1089,6 +1090,22 @@ ip_vs_control_add(struct ip_vs_conn *cp, struct ip_vs_conn *ctl_cp)
atomic_inc(&ctl_cp->n_control);
}
+/*
+ * IPVS netns init & cleanup functions
+ */
+extern int __ip_vs_estimator_init(struct net *net);
+extern int __ip_vs_control_init(struct net *net);
+extern int __ip_vs_protocol_init(struct net *net);
+extern int __ip_vs_app_init(struct net *net);
+extern int __ip_vs_conn_init(struct net *net);
+extern int __ip_vs_sync_init(struct net *net);
+extern void __ip_vs_conn_cleanup(struct net *net);
+extern void __ip_vs_app_cleanup(struct net *net);
+extern void __ip_vs_protocol_cleanup(struct net *net);
+extern void __ip_vs_control_cleanup(struct net *net);
+extern void __ip_vs_estimator_cleanup(struct net *net);
+extern void __ip_vs_sync_cleanup(struct net *net);
+extern void __ip_vs_service_cleanup(struct net *net);
/*
* IPVS application functions
diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c
index 2dc6de1..51f3af7 100644
--- a/net/netfilter/ipvs/ip_vs_app.c
+++ b/net/netfilter/ipvs/ip_vs_app.c
@@ -576,7 +576,7 @@ static const struct file_operations ip_vs_app_fops = {
};
#endif
-static int __net_init __ip_vs_app_init(struct net *net)
+int __net_init __ip_vs_app_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -585,26 +585,17 @@ static int __net_init __ip_vs_app_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_app_cleanup(struct net *net)
+void __net_exit __ip_vs_app_cleanup(struct net *net)
{
proc_net_remove(net, "ip_vs_app");
}
-static struct pernet_operations ip_vs_app_ops = {
- .init = __ip_vs_app_init,
- .exit = __ip_vs_app_cleanup,
-};
-
int __init ip_vs_app_init(void)
{
- int rv;
-
- rv = register_pernet_subsys(&ip_vs_app_ops);
- return rv;
+ return 0;
}
void ip_vs_app_cleanup(void)
{
- unregister_pernet_subsys(&ip_vs_app_ops);
}
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index f289306..5092505 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1258,22 +1258,17 @@ int __net_init __ip_vs_conn_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_conn_cleanup(struct net *net)
+void __net_exit __ip_vs_conn_cleanup(struct net *net)
{
/* flush all the connection entries first */
ip_vs_conn_flush(net);
proc_net_remove(net, "ip_vs_conn");
proc_net_remove(net, "ip_vs_conn_sync");
}
-static struct pernet_operations ipvs_conn_ops = {
- .init = __ip_vs_conn_init,
- .exit = __ip_vs_conn_cleanup,
-};
int __init ip_vs_conn_init(void)
{
int idx;
- int retc;
/* Compute size and mask */
ip_vs_conn_tab_size = 1 << ip_vs_conn_tab_bits;
@@ -1309,17 +1304,14 @@ int __init ip_vs_conn_init(void)
rwlock_init(&__ip_vs_conntbl_lock_array[idx].l);
}
- retc = register_pernet_subsys(&ipvs_conn_ops);
-
/* calculate the random value for connection hash */
get_random_bytes(&ip_vs_conn_rnd, sizeof(ip_vs_conn_rnd));
- return retc;
+ return 0;
}
void ip_vs_conn_cleanup(void)
{
- unregister_pernet_subsys(&ipvs_conn_ops);
/* Release the empty cache */
kmem_cache_destroy(ip_vs_conn_cachep);
vfree(ip_vs_conn_tab);
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index a0791dc..a74dae6 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1113,6 +1113,9 @@ ip_vs_out(unsigned int hooknum, struct sk_buff *skb, int af)
return NF_ACCEPT;
net = skb_net(skb);
+ if (!net_ipvs(net)->enable)
+ return NF_ACCEPT;
+
ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
#ifdef CONFIG_IP_VS_IPV6
if (af == AF_INET6) {
@@ -1343,6 +1346,7 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
return NF_ACCEPT; /* The packet looks wrong, ignore */
net = skb_net(skb);
+
pd = ip_vs_proto_data_get(net, cih->protocol);
if (!pd)
return NF_ACCEPT;
@@ -1529,6 +1533,11 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
IP_VS_DBG_ADDR(af, &iph.daddr), hooknum);
return NF_ACCEPT;
}
+ /* ipvs enabled in this netns ? */
+ net = skb_net(skb);
+ if (!net_ipvs(net)->enable)
+ return NF_ACCEPT;
+
ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
/* Bad... Do not break raw sockets */
@@ -1562,7 +1571,6 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
}
- net = skb_net(skb);
/* Protocol supported? */
pd = ip_vs_proto_data_get(net, iph.protocol);
if (unlikely(!pd))
@@ -1588,7 +1596,6 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
}
IP_VS_DBG_PKT(11, af, pp, skb, 0, "Incoming packet");
- net = skb_net(skb);
ipvs = net_ipvs(net);
/* Check the server status */
if (cp->dest && !(cp->dest->flags & IP_VS_DEST_F_AVAILABLE)) {
@@ -1743,10 +1750,16 @@ ip_vs_forward_icmp(unsigned int hooknum, struct sk_buff *skb,
int (*okfn)(struct sk_buff *))
{
int r;
+ struct net *net;
if (ip_hdr(skb)->protocol != IPPROTO_ICMP)
return NF_ACCEPT;
+ /* ipvs enabled in this netns ? */
+ net = skb_net(skb);
+ if (!net_ipvs(net)->enable)
+ return NF_ACCEPT;
+
return ip_vs_in_icmp(skb, &r, hooknum);
}
@@ -1757,10 +1770,16 @@ ip_vs_forward_icmp_v6(unsigned int hooknum, struct sk_buff *skb,
int (*okfn)(struct sk_buff *))
{
int r;
+ struct net *net;
if (ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6)
return NF_ACCEPT;
+ /* ipvs enabled in this netns ? */
+ net = skb_net(skb);
+ if (!net_ipvs(net)->enable)
+ return NF_ACCEPT;
+
return ip_vs_in_icmp_v6(skb, &r, hooknum);
}
#endif
@@ -1884,21 +1903,72 @@ static int __net_init __ip_vs_init(struct net *net)
pr_err("%s(): no memory.\n", __func__);
return -ENOMEM;
}
+ /* Hold the beast until a service is registerd */
+ ipvs->enable = 0;
ipvs->net = net;
/* Counters used for creating unique names */
ipvs->gen = atomic_read(&ipvs_netns_cnt);
atomic_inc(&ipvs_netns_cnt);
net->ipvs = ipvs;
+
+ if (__ip_vs_estimator_init(net) < 0)
+ goto estimator_fail;
+
+ if (__ip_vs_control_init(net) < 0)
+ goto control_fail;
+
+ if (__ip_vs_protocol_init(net) < 0)
+ goto protocol_fail;
+
+ if (__ip_vs_app_init(net) < 0)
+ goto app_fail;
+
+ if (__ip_vs_conn_init(net) < 0)
+ goto conn_fail;
+
+ if (__ip_vs_sync_init(net) < 0)
+ goto sync_fail;
+
printk(KERN_INFO "IPVS: Creating netns size=%zu id=%d\n",
sizeof(struct netns_ipvs), ipvs->gen);
return 0;
+/*
+ * Error handling
+ */
+
+sync_fail:
+ __ip_vs_conn_cleanup(net);
+conn_fail:
+ __ip_vs_app_cleanup(net);
+app_fail:
+ __ip_vs_protocol_cleanup(net);
+protocol_fail:
+ __ip_vs_control_cleanup(net);
+control_fail:
+ __ip_vs_estimator_cleanup(net);
+estimator_fail:
+ return -ENOMEM;
}
static void __net_exit __ip_vs_cleanup(struct net *net)
{
+ __ip_vs_service_cleanup(net); /* ip_vs_flush() with locks */
+ __ip_vs_conn_cleanup(net);
+ __ip_vs_app_cleanup(net);
+ __ip_vs_protocol_cleanup(net);
+ __ip_vs_control_cleanup(net);
+ __ip_vs_estimator_cleanup(net);
IP_VS_DBG(2, "ipvs netns %d released\n", net_ipvs(net)->gen);
}
+static void __net_exit __ip_vs_dev_cleanup(struct net *net)
+{
+ EnterFunction(2);
+ net_ipvs(net)->enable = 0; /* Disable packet reception */
+ __ip_vs_sync_cleanup(net);
+ LeaveFunction(2);
+}
+
static struct pernet_operations ipvs_core_ops = {
.init = __ip_vs_init,
.exit = __ip_vs_cleanup,
@@ -1906,6 +1976,10 @@ static struct pernet_operations ipvs_core_ops = {
.size = sizeof(struct netns_ipvs),
};
+static struct pernet_operations ipvs_core_dev_ops = {
+ .exit = __ip_vs_dev_cleanup,
+};
+
/*
* Initialize IP Virtual Server
*/
@@ -1913,10 +1987,6 @@ static int __init ip_vs_init(void)
{
int ret;
- ret = register_pernet_subsys(&ipvs_core_ops); /* Alloc ip_vs struct */
- if (ret < 0)
- return ret;
-
ip_vs_estimator_init();
ret = ip_vs_control_init();
if (ret < 0) {
@@ -1944,15 +2014,28 @@ static int __init ip_vs_init(void)
goto cleanup_conn;
}
+ ret = register_pernet_subsys(&ipvs_core_ops); /* Alloc ip_vs struct */
+ if (ret < 0)
+ goto cleanup_sync;
+
+ ret = register_pernet_device(&ipvs_core_dev_ops);
+ if (ret < 0)
+ goto cleanup_sub;
+
ret = nf_register_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
if (ret < 0) {
pr_err("can't register hooks.\n");
- goto cleanup_sync;
+ goto cleanup_dev;
}
pr_info("ipvs loaded.\n");
+
return ret;
+cleanup_dev:
+ unregister_pernet_device(&ipvs_core_dev_ops);
+cleanup_sub:
+ unregister_pernet_subsys(&ipvs_core_ops);
cleanup_sync:
ip_vs_sync_cleanup();
cleanup_conn:
@@ -1964,20 +2047,20 @@ cleanup_sync:
ip_vs_control_cleanup();
cleanup_estimator:
ip_vs_estimator_cleanup();
- unregister_pernet_subsys(&ipvs_core_ops); /* free ip_vs struct */
return ret;
}
static void __exit ip_vs_cleanup(void)
{
nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
+ unregister_pernet_device(&ipvs_core_dev_ops);
+ unregister_pernet_subsys(&ipvs_core_ops); /* free ip_vs struct */
ip_vs_sync_cleanup();
ip_vs_conn_cleanup();
ip_vs_app_cleanup();
ip_vs_protocol_cleanup();
ip_vs_control_cleanup();
ip_vs_estimator_cleanup();
- unregister_pernet_subsys(&ipvs_core_ops); /* free ip_vs struct */
pr_info("ipvs unloaded.\n");
}
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index ae47090..ea72281 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -69,6 +69,11 @@ int ip_vs_get_debug_level(void)
}
#endif
+
+/* Protos */
+static void __ip_vs_del_service(struct ip_vs_service *svc);
+
+
#ifdef CONFIG_IP_VS_IPV6
/* Taken from rt6_fill_node() in net/ipv6/route.c, is there a better way? */
static int __ip_vs_addr_is_local_v6(struct net *net,
@@ -1214,6 +1219,8 @@ ip_vs_add_service(struct net *net, struct ip_vs_service_user_kern *u,
write_unlock_bh(&__ip_vs_svc_lock);
*svc_p = svc;
+ /* Now there is a service - full throttle */
+ ipvs->enable = 1;
return 0;
@@ -1472,6 +1479,84 @@ static int ip_vs_flush(struct net *net)
return 0;
}
+/*
+ * Delete service by {netns} in the service table.
+ * Called by __ip_vs_cleanup()
+ */
+void __ip_vs_service_cleanup(struct net *net)
+{
+ EnterFunction(2);
+ /* Check for "full" addressed entries */
+ mutex_lock(&__ip_vs_mutex);
+ ip_vs_flush(net);
+ mutex_unlock(&__ip_vs_mutex);
+ LeaveFunction(2);
+}
+/*
+ * Release dst hold by dst_cache
+ */
+static inline void
+__ip_vs_dev_reset(struct ip_vs_dest *dest, struct net_device *dev)
+{
+ spin_lock_bh(&dest->dst_lock);
+ if (dest->dst_cache && dest->dst_cache->dev == dev) {
+ IP_VS_DBG_BUF(3, "Reset dev:%s dest %s:%u ,dest->refcnt=%d\n",
+ dev->name,
+ IP_VS_DBG_ADDR(dest->af, &dest->addr),
+ ntohs(dest->port),
+ atomic_read(&dest->refcnt));
+ ip_vs_dst_reset(dest);
+ }
+ spin_unlock_bh(&dest->dst_lock);
+
+}
+/*
+ * Netdev event receiver
+ * Currently only NETDEV_UNREGISTER is handled, i.e. if we hold a reference to
+ * a device that is "unregister" it must be released.
+ */
+static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
+ void *ptr)
+{
+ struct net_device *dev = ptr;
+ struct net *net = dev_net(dev);
+ struct ip_vs_service *svc;
+ struct ip_vs_dest *dest;
+ unsigned int idx;
+
+ if (event != NETDEV_UNREGISTER)
+ return NOTIFY_DONE;
+ IP_VS_DBG(3, "%s() dev=%s\n", __func__, dev->name);
+ EnterFunction(2);
+ mutex_lock(&__ip_vs_mutex);
+ for (idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
+ list_for_each_entry(svc, &ip_vs_svc_table[idx], s_list) {
+ if (net_eq(svc->net, net)) {
+ list_for_each_entry(dest, &svc->destinations,
+ n_list) {
+ __ip_vs_dev_reset(dest, dev);
+ }
+ }
+ }
+
+ list_for_each_entry(svc, &ip_vs_svc_fwm_table[idx], f_list) {
+ if (net_eq(svc->net, net)) {
+ list_for_each_entry(dest, &svc->destinations,
+ n_list) {
+ __ip_vs_dev_reset(dest, dev);
+ }
+ }
+
+ }
+ }
+
+ list_for_each_entry(dest, &net_ipvs(net)->dest_trash, n_list) {
+ __ip_vs_dev_reset(dest, dev);
+ }
+ mutex_unlock(&__ip_vs_mutex);
+ LeaveFunction(2);
+ return NOTIFY_DONE;
+}
/*
* Zero counters in a service or all services
@@ -3588,6 +3673,10 @@ void __net_init __ip_vs_control_cleanup_sysctl(struct net *net) { }
#endif
+static struct notifier_block ip_vs_dst_notifier = {
+ .notifier_call = ip_vs_dst_event,
+};
+
int __net_init __ip_vs_control_init(struct net *net)
{
int idx;
@@ -3626,7 +3715,7 @@ err:
return -ENOMEM;
}
-static void __net_exit __ip_vs_control_cleanup(struct net *net)
+void __net_exit __ip_vs_control_cleanup(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -3639,11 +3728,6 @@ static void __net_exit __ip_vs_control_cleanup(struct net *net)
free_percpu(ipvs->tot_stats.cpustats);
}
-static struct pernet_operations ipvs_control_ops = {
- .init = __ip_vs_control_init,
- .exit = __ip_vs_control_cleanup,
-};
-
int __init ip_vs_control_init(void)
{
int idx;
@@ -3657,33 +3741,32 @@ int __init ip_vs_control_init(void)
INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
}
- ret = register_pernet_subsys(&ipvs_control_ops);
- if (ret) {
- pr_err("cannot register namespace.\n");
- goto err;
- }
-
smp_wmb(); /* Do we really need it now ? */
ret = nf_register_sockopt(&ip_vs_sockopts);
if (ret) {
pr_err("cannot register sockopt.\n");
- goto err_net;
+ goto err_sock;
}
ret = ip_vs_genl_register();
if (ret) {
pr_err("cannot register Generic Netlink interface.\n");
- nf_unregister_sockopt(&ip_vs_sockopts);
- goto err_net;
+ goto err_genl;
}
+ ret = register_netdevice_notifier(&ip_vs_dst_notifier);
+ if (ret < 0)
+ goto err_notf;
+
LeaveFunction(2);
return 0;
-err_net:
- unregister_pernet_subsys(&ipvs_control_ops);
-err:
+err_notf:
+ ip_vs_genl_unregister();
+err_genl:
+ nf_unregister_sockopt(&ip_vs_sockopts);
+err_sock:
return ret;
}
@@ -3691,7 +3774,6 @@ err:
void ip_vs_control_cleanup(void)
{
EnterFunction(2);
- unregister_pernet_subsys(&ipvs_control_ops);
ip_vs_genl_unregister();
nf_unregister_sockopt(&ip_vs_sockopts);
LeaveFunction(2);
diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c
index 8c8766c..508cce9 100644
--- a/net/netfilter/ipvs/ip_vs_est.c
+++ b/net/netfilter/ipvs/ip_vs_est.c
@@ -192,7 +192,7 @@ void ip_vs_read_estimator(struct ip_vs_stats_user *dst,
dst->outbps = (e->outbps + 0xF) >> 5;
}
-static int __net_init __ip_vs_estimator_init(struct net *net)
+int __net_init __ip_vs_estimator_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -203,24 +203,16 @@ static int __net_init __ip_vs_estimator_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_estimator_exit(struct net *net)
+void __net_exit __ip_vs_estimator_cleanup(struct net *net)
{
del_timer_sync(&net_ipvs(net)->est_timer);
}
-static struct pernet_operations ip_vs_app_ops = {
- .init = __ip_vs_estimator_init,
- .exit = __ip_vs_estimator_exit,
-};
int __init ip_vs_estimator_init(void)
{
- int rv;
-
- rv = register_pernet_subsys(&ip_vs_app_ops);
- return rv;
+ return 0;
}
void ip_vs_estimator_cleanup(void)
{
- unregister_pernet_subsys(&ip_vs_app_ops);
}
diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
index 17484a4..eb86028 100644
--- a/net/netfilter/ipvs/ip_vs_proto.c
+++ b/net/netfilter/ipvs/ip_vs_proto.c
@@ -316,7 +316,7 @@ ip_vs_tcpudp_debug_packet(int af, struct ip_vs_protocol *pp,
/*
* per network name-space init
*/
-static int __net_init __ip_vs_protocol_init(struct net *net)
+int __net_init __ip_vs_protocol_init(struct net *net)
{
#ifdef CONFIG_IP_VS_PROTO_TCP
register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
@@ -336,7 +336,7 @@ static int __net_init __ip_vs_protocol_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_protocol_cleanup(struct net *net)
+void __net_exit __ip_vs_protocol_cleanup(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
struct ip_vs_proto_data *pd;
@@ -349,11 +349,6 @@ static void __net_exit __ip_vs_protocol_cleanup(struct net *net)
}
}
-static struct pernet_operations ipvs_proto_ops = {
- .init = __ip_vs_protocol_init,
- .exit = __ip_vs_protocol_cleanup,
-};
-
int __init ip_vs_protocol_init(void)
{
char protocols[64];
@@ -382,7 +377,6 @@ int __init ip_vs_protocol_init(void)
REGISTER_PROTOCOL(&ip_vs_protocol_esp);
#endif
pr_info("Registered protocols (%s)\n", &protocols[2]);
- return register_pernet_subsys(&ipvs_proto_ops);
return 0;
}
@@ -393,7 +387,6 @@ void ip_vs_protocol_cleanup(void)
struct ip_vs_protocol *pp;
int i;
- unregister_pernet_subsys(&ipvs_proto_ops);
/* unregister all the ipvs protocols */
for (i = 0; i < IP_VS_PROTO_TAB_SIZE; i++) {
while ((pp = ip_vs_proto_table[i]) != NULL)
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 0cce953..e292e5b 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1663,7 +1663,7 @@ int stop_sync_thread(struct net *net, int state)
/*
* Initialize data struct for each netns
*/
-static int __net_init __ip_vs_sync_init(struct net *net)
+int __net_init __ip_vs_sync_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -1677,7 +1677,7 @@ static int __net_init __ip_vs_sync_init(struct net *net)
return 0;
}
-static void __ip_vs_sync_cleanup(struct net *net)
+void __ip_vs_sync_cleanup(struct net *net)
{
int retc;
@@ -1690,18 +1690,11 @@ static void __ip_vs_sync_cleanup(struct net *net)
pr_err("Failed to stop Backup Daemon\n");
}
-static struct pernet_operations ipvs_sync_ops = {
- .init = __ip_vs_sync_init,
- .exit = __ip_vs_sync_cleanup,
-};
-
^ permalink raw reply related
* [PATCH] udp: Use flow key information instead of rt->rt_{src,dst}
From: David Miller @ 2011-05-08 23:40 UTC (permalink / raw)
To: netdev
We have two cases.
Either the socket is in TCP_ESTABLISHED state and connect() filled
in the inet socket cork flow, or we looked up the route here and
used an on-stack flow.
Track which one it was, and use it to obtain src/dst addrs.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
This is a prelude to passing a flow key down into the datagram
packet sending engine interfaces in ip_output.c
net/ipv4/udp.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 544f435..ba9f137 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -791,6 +791,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
{
struct inet_sock *inet = inet_sk(sk);
struct udp_sock *up = udp_sk(sk);
+ struct flowi4 fl4_stack;
struct flowi4 *fl4;
int ulen = len;
struct ipcm_cookie ipc;
@@ -919,17 +920,18 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
if (connected)
rt = (struct rtable *)sk_dst_check(sk, 0);
+ fl4 = &inet->cork.fl.u.ip4;
if (rt == NULL) {
- struct flowi4 fl4;
struct net *net = sock_net(sk);
- flowi4_init_output(&fl4, ipc.oif, sk->sk_mark, tos,
+ fl4 = &fl4_stack;
+ flowi4_init_output(fl4, ipc.oif, sk->sk_mark, tos,
RT_SCOPE_UNIVERSE, sk->sk_protocol,
inet_sk_flowi_flags(sk)|FLOWI_FLAG_CAN_SLEEP,
faddr, saddr, dport, inet->inet_sport);
- security_sk_classify_flow(sk, flowi4_to_flowi(&fl4));
- rt = ip_route_output_flow(net, &fl4, sk);
+ security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
+ rt = ip_route_output_flow(net, fl4, sk);
if (IS_ERR(rt)) {
err = PTR_ERR(rt);
rt = NULL;
@@ -950,9 +952,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
goto do_confirm;
back_from_confirm:
- saddr = rt->rt_src;
+ saddr = fl4->saddr;
if (!ipc.addr)
- daddr = ipc.addr = rt->rt_dst;
+ daddr = ipc.addr = fl4->daddr;
/* Lockless fast path for the non-corking case. */
if (!corkreq) {
--
1.7.5.1
^ permalink raw reply related
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Håkon Løvdal @ 2011-05-09 0:08 UTC (permalink / raw)
To: David Miller
Cc: mfmooney, joe, aquini, kernel-janitors, fubar, andy, shemminger,
netdev, nikai
In-Reply-To: <20110508.161012.258121848.davem@davemloft.net>
On 9 May 2011 01:10, David Miller <davem@davemloft.net> wrote:
> From: Håkon Løvdal <hlovdal@gmail.com>
> Date: Mon, 9 May 2011 01:08:44 +0200
>
>> On 7 May 2011 21:35, matt mooney <mfmooney@gmail.com> wrote:
>>> But isn't the preferred style to have a single exit point?
>>
>> This is generally considered to be a bad advice, see
>> http://stackoverflow.com/questions/1701686/why-should-methods-have-a-single-entry-and-exit-points/1701721#1701721
>> for instance.
>
> That article totally ignores the issue of locking and how hard it is
> to get right without single exit points, and how unlocking in
> multiple spots bloats up the code.
>
> Definitely don't take that article's advice when working on the
> kernel.
>
I think we agree, but my answer was probably too short, unclear and
imprecise. In the case of locking and single exit points in the kernel,
they are (almost always) reached through goto/labels, and this is a fine
way of handling exiting a function, e.g.
void bond_3ad_state_machine_handler(struct work_struct *work)
{
struct bonding *bond = container_of(work, struct bonding,
ad_work.work);
struct port *port;
struct aggregator *aggregator;
read_lock(&bond->lock);
if (bond->kill_timers)
goto out;
//check if there are any slaves
if (bond->slave_cnt == 0)
goto re_arm;
...
re_arm:
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
out:
read_unlock(&bond->lock);
}
I often advocate usage of goto to achive this kind of style.
What I assosiate with "writing a function as single exit style" would
be something like the following (and usually littered with
temporary remember-this-for-later variables).
void bond_3ad_state_machine_handler(struct work_struct *work)
{
struct bonding *bond = container_of(work, struct bonding,
ad_work.work);
struct port *port;
struct aggregator *aggregator;
read_lock(&bond->lock);
if (! bond->kill_timers) {
//check if there are any slaves
if (bond->slave_cnt != 0) {
...
}
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
}
read_unlock(&bond->lock);
}
And this was what I trying to reccommend against (and which the
stackoverflow question is about). So most probably my assosiasion was too
implicit to make my reply useful. That was not the intention, hopefully
this followup clears up a little.
BR Håkon Løvdal
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: David Miller @ 2011-05-09 0:12 UTC (permalink / raw)
To: hlovdal
Cc: mfmooney, joe, aquini, kernel-janitors, fubar, andy, shemminger,
netdev, nikai
In-Reply-To: <BANLkTimao-Zyk6ypW3UxzqtsgmQ-6qvYng@mail.gmail.com>
From: Håkon Løvdal <hlovdal@gmail.com>
Date: Mon, 9 May 2011 02:08:41 +0200
> void bond_3ad_state_machine_handler(struct work_struct *work)
> {
> struct bonding *bond = container_of(work, struct bonding,
> ad_work.work);
> struct port *port;
> struct aggregator *aggregator;
>
> read_lock(&bond->lock);
>
> if (! bond->kill_timers) {
>
> //check if there are any slaves
> if (bond->slave_cnt != 0) {
> ...
> }
> queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
> }
> read_unlock(&bond->lock);
> }
>
>
> And this was what I trying to reccommend against (and which the
> stackoverflow question is about).
I really don't see anything wrong with either approach.
^ permalink raw reply
* [PATCH 0/2] Get flow key down into ipv4 datagram packet engine
From: David Miller @ 2011-05-09 1:07 UTC (permalink / raw)
To: netdev
The really nice part about this patch set is that it will subsequently
allow us to kill a lot of on-stack stuff.
Parts of what we pass around in the flow key is also passed around in
this "ipcm_cookie" object we use for datagram/raw/icmp packet
construction.
So now we can perform some minimizations on ipcm_cookie as a result.
^ permalink raw reply
* [PATCH 1/2] ipv4: Pass flow keys down into datagram packet building engine.
From: David Miller @ 2011-05-09 1:07 UTC (permalink / raw)
To: netdev
This way ip_output.c no longer needs rt->rt_{src,dst}.
We already have these keys sitting, ready and waiting, on the stack or
in a socket structure.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/ip.h | 8 +++--
net/ipv4/icmp.c | 74 ++++++++++++++++++++++++-------------------------
net/ipv4/ip_output.c | 39 +++++++++++++-------------
net/ipv4/raw.c | 59 +++++++++++++++++++--------------------
net/ipv4/udp.c | 4 +-
5 files changed, 91 insertions(+), 93 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index acf8b78..a425379 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -117,12 +117,14 @@ extern int ip_generic_getfrag(void *from, char *to, int offset, int len, int od
extern ssize_t ip_append_page(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
extern struct sk_buff *__ip_make_skb(struct sock *sk,
+ struct flowi4 *fl4,
struct sk_buff_head *queue,
struct inet_cork *cork);
extern int ip_send_skb(struct sk_buff *skb);
-extern int ip_push_pending_frames(struct sock *sk);
+extern int ip_push_pending_frames(struct sock *sk, struct flowi4 *fl4);
extern void ip_flush_pending_frames(struct sock *sk);
extern struct sk_buff *ip_make_skb(struct sock *sk,
+ struct flowi4 *fl4,
int getfrag(void *from, char *to, int offset, int len,
int odd, struct sk_buff *skb),
void *from, int length, int transhdrlen,
@@ -130,9 +132,9 @@ extern struct sk_buff *ip_make_skb(struct sock *sk,
struct rtable **rtp,
unsigned int flags);
-static inline struct sk_buff *ip_finish_skb(struct sock *sk)
+static inline struct sk_buff *ip_finish_skb(struct sock *sk, struct flowi4 *fl4)
{
- return __ip_make_skb(sk, &sk->sk_write_queue, &inet_sk(sk)->cork.base);
+ return __ip_make_skb(sk, fl4, &sk->sk_write_queue, &inet_sk(sk)->cork.base);
}
/* datagram.c */
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index cfeca3c..b3dc6de 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -290,6 +290,7 @@ static int icmp_glue_bits(void *from, char *to, int offset, int len, int odd,
}
static void icmp_push_reply(struct icmp_bxm *icmp_param,
+ struct flowi4 *fl4,
struct ipcm_cookie *ipc, struct rtable **rt)
{
struct sock *sk;
@@ -315,7 +316,7 @@ static void icmp_push_reply(struct icmp_bxm *icmp_param,
icmp_param->head_len, csum);
icmph->checksum = csum_fold(csum);
skb->ip_summed = CHECKSUM_NONE;
- ip_push_pending_frames(sk);
+ ip_push_pending_frames(sk, fl4);
}
}
@@ -328,6 +329,7 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
struct ipcm_cookie ipc;
struct rtable *rt = skb_rtable(skb);
struct net *net = dev_net(rt->dst.dev);
+ struct flowi4 fl4;
struct sock *sk;
struct inet_sock *inet;
__be32 daddr;
@@ -351,57 +353,52 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
if (ipc.opt->opt.srr)
daddr = icmp_param->replyopts.opt.opt.faddr;
}
- {
- struct flowi4 fl4 = {
- .daddr = daddr,
- .saddr = rt->rt_spec_dst,
- .flowi4_tos = RT_TOS(ip_hdr(skb)->tos),
- .flowi4_proto = IPPROTO_ICMP,
- };
- security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
- rt = ip_route_output_key(net, &fl4);
- if (IS_ERR(rt))
- goto out_unlock;
- }
+ memset(&fl4, 0, sizeof(fl4));
+ fl4.daddr = daddr;
+ fl4.saddr = rt->rt_spec_dst;
+ fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
+ fl4.flowi4_proto = IPPROTO_ICMP;
+ security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
+ rt = ip_route_output_key(net, &fl4);
+ if (IS_ERR(rt))
+ goto out_unlock;
if (icmpv4_xrlim_allow(net, rt, icmp_param->data.icmph.type,
icmp_param->data.icmph.code))
- icmp_push_reply(icmp_param, &ipc, &rt);
+ icmp_push_reply(icmp_param, &fl4, &ipc, &rt);
ip_rt_put(rt);
out_unlock:
icmp_xmit_unlock(sk);
}
-static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
+static struct rtable *icmp_route_lookup(struct net *net,
+ struct flowi4 *fl4,
+ struct sk_buff *skb_in,
const struct iphdr *iph,
__be32 saddr, u8 tos,
int type, int code,
struct icmp_bxm *param)
{
- struct flowi4 fl4 = {
- .daddr = (param->replyopts.opt.opt.srr ?
- param->replyopts.opt.opt.faddr : iph->saddr),
- .saddr = saddr,
- .flowi4_tos = RT_TOS(tos),
- .flowi4_proto = IPPROTO_ICMP,
- .fl4_icmp_type = type,
- .fl4_icmp_code = code,
- };
struct rtable *rt, *rt2;
int err;
- security_skb_classify_flow(skb_in, flowi4_to_flowi(&fl4));
- rt = __ip_route_output_key(net, &fl4);
+ memset(&fl4, 0, sizeof(fl4));
+ fl4->daddr = (param->replyopts.opt.opt.srr ?
+ param->replyopts.opt.opt.faddr : iph->saddr);
+ fl4->saddr = saddr;
+ fl4->flowi4_tos = RT_TOS(tos);
+ fl4->flowi4_proto = IPPROTO_ICMP;
+ fl4->fl4_icmp_type = type;
+ fl4->fl4_icmp_code = code;
+ security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
+ rt = __ip_route_output_key(net, fl4);
if (IS_ERR(rt))
return rt;
/* No need to clone since we're just using its address. */
rt2 = rt;
- if (!fl4.saddr)
- fl4.saddr = rt->rt_src;
-
rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
- flowi4_to_flowi(&fl4), NULL, 0);
+ flowi4_to_flowi(fl4), NULL, 0);
if (!IS_ERR(rt)) {
if (rt != rt2)
return rt;
@@ -410,19 +407,19 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
} else
return rt;
- err = xfrm_decode_session_reverse(skb_in, flowi4_to_flowi(&fl4), AF_INET);
+ err = xfrm_decode_session_reverse(skb_in, flowi4_to_flowi(fl4), AF_INET);
if (err)
goto relookup_failed;
- if (inet_addr_type(net, fl4.saddr) == RTN_LOCAL) {
- rt2 = __ip_route_output_key(net, &fl4);
+ if (inet_addr_type(net, fl4->saddr) == RTN_LOCAL) {
+ rt2 = __ip_route_output_key(net, fl4);
if (IS_ERR(rt2))
err = PTR_ERR(rt2);
} else {
struct flowi4 fl4_2 = {};
unsigned long orefdst;
- fl4_2.daddr = fl4.saddr;
+ fl4_2.daddr = fl4->saddr;
rt2 = ip_route_output_key(net, &fl4_2);
if (IS_ERR(rt2)) {
err = PTR_ERR(rt2);
@@ -430,7 +427,7 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
}
/* Ugh! */
orefdst = skb_in->_skb_refdst; /* save old refdst */
- err = ip_route_input(skb_in, fl4.daddr, fl4.saddr,
+ err = ip_route_input(skb_in, fl4->daddr, fl4->saddr,
RT_TOS(tos), rt2->dst.dev);
dst_release(&rt2->dst);
@@ -442,7 +439,7 @@ static struct rtable *icmp_route_lookup(struct net *net, struct sk_buff *skb_in,
goto relookup_failed;
rt2 = (struct rtable *) xfrm_lookup(net, &rt2->dst,
- flowi4_to_flowi(&fl4), NULL,
+ flowi4_to_flowi(fl4), NULL,
XFRM_LOOKUP_ICMP);
if (!IS_ERR(rt2)) {
dst_release(&rt->dst);
@@ -481,6 +478,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
struct icmp_bxm icmp_param;
struct rtable *rt = skb_rtable(skb_in);
struct ipcm_cookie ipc;
+ struct flowi4 fl4;
__be32 saddr;
u8 tos;
struct net *net;
@@ -599,7 +597,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
ipc.opt = &icmp_param.replyopts.opt;
ipc.tx_flags = 0;
- rt = icmp_route_lookup(net, skb_in, iph, saddr, tos,
+ rt = icmp_route_lookup(net, &fl4, skb_in, iph, saddr, tos,
type, code, &icmp_param);
if (IS_ERR(rt))
goto out_unlock;
@@ -620,7 +618,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
icmp_param.data_len = room;
icmp_param.head_len = sizeof(struct icmphdr);
- icmp_push_reply(&icmp_param, &ipc, &rt);
+ icmp_push_reply(&icmp_param, &fl4, &ipc, &rt);
ende:
ip_rt_put(rt);
out_unlock:
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index b88ee5f..dca637b 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1267,6 +1267,7 @@ static void ip_cork_release(struct inet_cork *cork)
* and push them out.
*/
struct sk_buff *__ip_make_skb(struct sock *sk,
+ struct flowi4 *fl4,
struct sk_buff_head *queue,
struct inet_cork *cork)
{
@@ -1333,8 +1334,8 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
ip_select_ident(iph, &rt->dst, sk);
iph->ttl = ttl;
iph->protocol = sk->sk_protocol;
- iph->saddr = rt->rt_src;
- iph->daddr = rt->rt_dst;
+ iph->saddr = fl4->saddr;
+ iph->daddr = fl4->daddr;
skb->priority = sk->sk_priority;
skb->mark = sk->sk_mark;
@@ -1370,11 +1371,11 @@ int ip_send_skb(struct sk_buff *skb)
return err;
}
-int ip_push_pending_frames(struct sock *sk)
+int ip_push_pending_frames(struct sock *sk, struct flowi4 *fl4)
{
struct sk_buff *skb;
- skb = ip_finish_skb(sk);
+ skb = ip_finish_skb(sk, fl4);
if (!skb)
return 0;
@@ -1403,6 +1404,7 @@ void ip_flush_pending_frames(struct sock *sk)
}
struct sk_buff *ip_make_skb(struct sock *sk,
+ struct flowi4 *fl4,
int getfrag(void *from, char *to, int offset,
int len, int odd, struct sk_buff *skb),
void *from, int length, int transhdrlen,
@@ -1432,7 +1434,7 @@ struct sk_buff *ip_make_skb(struct sock *sk,
return ERR_PTR(err);
}
- return __ip_make_skb(sk, &queue, &cork);
+ return __ip_make_skb(sk, fl4, &queue, &cork);
}
/*
@@ -1461,6 +1463,7 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
struct inet_sock *inet = inet_sk(sk);
struct ip_options_data replyopts;
struct ipcm_cookie ipc;
+ struct flowi4 fl4;
__be32 daddr;
struct rtable *rt = skb_rtable(skb);
@@ -1478,20 +1481,16 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
daddr = replyopts.opt.opt.faddr;
}
- {
- struct flowi4 fl4;
-
- flowi4_init_output(&fl4, arg->bound_dev_if, 0,
- RT_TOS(ip_hdr(skb)->tos),
- RT_SCOPE_UNIVERSE, sk->sk_protocol,
- ip_reply_arg_flowi_flags(arg),
- daddr, rt->rt_spec_dst,
- tcp_hdr(skb)->source, tcp_hdr(skb)->dest);
- security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
- rt = ip_route_output_key(sock_net(sk), &fl4);
- if (IS_ERR(rt))
- return;
- }
+ flowi4_init_output(&fl4, arg->bound_dev_if, 0,
+ RT_TOS(ip_hdr(skb)->tos),
+ RT_SCOPE_UNIVERSE, sk->sk_protocol,
+ ip_reply_arg_flowi_flags(arg),
+ daddr, rt->rt_spec_dst,
+ tcp_hdr(skb)->source, tcp_hdr(skb)->dest);
+ security_skb_classify_flow(skb, flowi4_to_flowi(&fl4));
+ rt = ip_route_output_key(sock_net(sk), &fl4);
+ if (IS_ERR(rt))
+ return;
/* And let IP do all the hard work.
@@ -1512,7 +1511,7 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
arg->csumoffset) = csum_fold(csum_add(skb->csum,
arg->csum));
skb->ip_summed = CHECKSUM_NONE;
- ip_push_pending_frames(sk);
+ ip_push_pending_frames(sk, &fl4);
}
bh_unlock_sock(sk);
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index a8659e0..6fee91f 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -314,9 +314,10 @@ int raw_rcv(struct sock *sk, struct sk_buff *skb)
return 0;
}
-static int raw_send_hdrinc(struct sock *sk, void *from, size_t length,
- struct rtable **rtp,
- unsigned int flags)
+static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
+ void *from, size_t length,
+ struct rtable **rtp,
+ unsigned int flags)
{
struct inet_sock *inet = inet_sk(sk);
struct net *net = sock_net(sk);
@@ -327,7 +328,7 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length,
struct rtable *rt = *rtp;
if (length > rt->dst.dev->mtu) {
- ip_local_error(sk, EMSGSIZE, rt->rt_dst, inet->inet_dport,
+ ip_local_error(sk, EMSGSIZE, fl4->daddr, inet->inet_dport,
rt->dst.dev->mtu);
return -EMSGSIZE;
}
@@ -372,7 +373,7 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length,
if (iphlen >= sizeof(*iph)) {
if (!iph->saddr)
- iph->saddr = rt->rt_src;
+ iph->saddr = fl4->saddr;
iph->check = 0;
iph->tot_len = htons(length);
if (!iph->id)
@@ -455,6 +456,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
struct inet_sock *inet = inet_sk(sk);
struct ipcm_cookie ipc;
struct rtable *rt = NULL;
+ struct flowi4 fl4;
int free = 0;
__be32 daddr;
__be32 saddr;
@@ -558,27 +560,23 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
saddr = inet->mc_addr;
}
- {
- struct flowi4 fl4;
+ flowi4_init_output(&fl4, ipc.oif, sk->sk_mark, tos,
+ RT_SCOPE_UNIVERSE,
+ inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
+ FLOWI_FLAG_CAN_SLEEP, daddr, saddr, 0, 0);
- flowi4_init_output(&fl4, ipc.oif, sk->sk_mark, tos,
- RT_SCOPE_UNIVERSE,
- inet->hdrincl ? IPPROTO_RAW : sk->sk_protocol,
- FLOWI_FLAG_CAN_SLEEP, daddr, saddr, 0, 0);
-
- if (!inet->hdrincl) {
- err = raw_probe_proto_opt(&fl4, msg);
- if (err)
- goto done;
- }
-
- security_sk_classify_flow(sk, flowi4_to_flowi(&fl4));
- rt = ip_route_output_flow(sock_net(sk), &fl4, sk);
- if (IS_ERR(rt)) {
- err = PTR_ERR(rt);
- rt = NULL;
+ if (!inet->hdrincl) {
+ err = raw_probe_proto_opt(&fl4, msg);
+ if (err)
goto done;
- }
+ }
+
+ security_sk_classify_flow(sk, flowi4_to_flowi(&fl4));
+ rt = ip_route_output_flow(sock_net(sk), &fl4, sk);
+ if (IS_ERR(rt)) {
+ err = PTR_ERR(rt);
+ rt = NULL;
+ goto done;
}
err = -EACCES;
@@ -590,19 +588,20 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
back_from_confirm:
if (inet->hdrincl)
- err = raw_send_hdrinc(sk, msg->msg_iov, len,
- &rt, msg->msg_flags);
+ err = raw_send_hdrinc(sk, &fl4, msg->msg_iov, len,
+ &rt, msg->msg_flags);
else {
if (!ipc.addr)
- ipc.addr = rt->rt_dst;
+ ipc.addr = fl4.daddr;
lock_sock(sk);
- err = ip_append_data(sk, ip_generic_getfrag, msg->msg_iov, len, 0,
- &ipc, &rt, msg->msg_flags);
+ err = ip_append_data(sk, ip_generic_getfrag,
+ msg->msg_iov, len, 0,
+ &ipc, &rt, msg->msg_flags);
if (err)
ip_flush_pending_frames(sk);
else if (!(msg->msg_flags & MSG_MORE)) {
- err = ip_push_pending_frames(sk);
+ err = ip_push_pending_frames(sk, &fl4);
if (err == -ENOBUFS && !inet->recverr)
err = 0;
}
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index ba9f137..006e2cc 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -774,7 +774,7 @@ static int udp_push_pending_frames(struct sock *sk)
struct sk_buff *skb;
int err = 0;
- skb = ip_finish_skb(sk);
+ skb = ip_finish_skb(sk, fl4);
if (!skb)
goto out;
@@ -958,7 +958,7 @@ back_from_confirm:
/* Lockless fast path for the non-corking case. */
if (!corkreq) {
- skb = ip_make_skb(sk, getfrag, msg->msg_iov, ulen,
+ skb = ip_make_skb(sk, fl4, getfrag, msg->msg_iov, ulen,
sizeof(struct udphdr), &ipc, &rt,
msg->msg_flags);
err = PTR_ERR(skb);
--
1.7.5.1
^ permalink raw reply related
* [PATCH 2/2] ipv4: Pass flow key down into ip_append_*().
From: David Miller @ 2011-05-09 1:07 UTC (permalink / raw)
To: netdev
This way rt->rt_dst accesses are unnecessary.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/ip.h | 4 ++--
net/ipv4/icmp.c | 2 +-
net/ipv4/ip_output.c | 18 ++++++++++--------
net/ipv4/raw.c | 2 +-
net/ipv4/udp.c | 12 +++++++-----
5 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index a425379..0b30d3a 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -106,7 +106,7 @@ extern int __ip_local_out(struct sk_buff *skb);
extern int ip_local_out(struct sk_buff *skb);
extern int ip_queue_xmit(struct sk_buff *skb, struct flowi *fl);
extern void ip_init(void);
-extern int ip_append_data(struct sock *sk,
+extern int ip_append_data(struct sock *sk, struct flowi4 *fl4,
int getfrag(void *from, char *to, int offset, int len,
int odd, struct sk_buff *skb),
void *from, int len, int protolen,
@@ -114,7 +114,7 @@ extern int ip_append_data(struct sock *sk,
struct rtable **rt,
unsigned int flags);
extern int ip_generic_getfrag(void *from, char *to, int offset, int len, int odd, struct sk_buff *skb);
-extern ssize_t ip_append_page(struct sock *sk, struct page *page,
+extern ssize_t ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
int offset, size_t size, int flags);
extern struct sk_buff *__ip_make_skb(struct sock *sk,
struct flowi4 *fl4,
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index b3dc6de..67ad66a 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -297,7 +297,7 @@ static void icmp_push_reply(struct icmp_bxm *icmp_param,
struct sk_buff *skb;
sk = icmp_sk(dev_net((*rt)->dst.dev));
- if (ip_append_data(sk, icmp_glue_bits, icmp_param,
+ if (ip_append_data(sk, fl4, icmp_glue_bits, icmp_param,
icmp_param->data_len+icmp_param->head_len,
icmp_param->head_len,
ipc, rt, MSG_DONTWAIT) < 0) {
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index dca637b..cd89d22 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -776,7 +776,9 @@ static inline int ip_ufo_append_data(struct sock *sk,
(length - transhdrlen));
}
-static int __ip_append_data(struct sock *sk, struct sk_buff_head *queue,
+static int __ip_append_data(struct sock *sk,
+ struct flowi4 *fl4,
+ struct sk_buff_head *queue,
struct inet_cork *cork,
int getfrag(void *from, char *to, int offset,
int len, int odd, struct sk_buff *skb),
@@ -808,7 +810,7 @@ static int __ip_append_data(struct sock *sk, struct sk_buff_head *queue,
maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;
if (cork->length + length > 0xFFFF - fragheaderlen) {
- ip_local_error(sk, EMSGSIZE, rt->rt_dst, inet->inet_dport,
+ ip_local_error(sk, EMSGSIZE, fl4->daddr, inet->inet_dport,
mtu-exthdrlen);
return -EMSGSIZE;
}
@@ -1083,7 +1085,7 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
*
* LATER: length must be adjusted by pad at tail, when it is required.
*/
-int ip_append_data(struct sock *sk,
+int ip_append_data(struct sock *sk, struct flowi4 *fl4,
int getfrag(void *from, char *to, int offset, int len,
int odd, struct sk_buff *skb),
void *from, int length, int transhdrlen,
@@ -1104,11 +1106,11 @@ int ip_append_data(struct sock *sk,
transhdrlen = 0;
}
- return __ip_append_data(sk, &sk->sk_write_queue, &inet->cork.base, getfrag,
+ return __ip_append_data(sk, fl4, &sk->sk_write_queue, &inet->cork.base, getfrag,
from, length, transhdrlen, flags);
}
-ssize_t ip_append_page(struct sock *sk, struct page *page,
+ssize_t ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
int offset, size_t size, int flags)
{
struct inet_sock *inet = inet_sk(sk);
@@ -1146,7 +1148,7 @@ ssize_t ip_append_page(struct sock *sk, struct page *page,
maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;
if (cork->length + size > 0xFFFF - fragheaderlen) {
- ip_local_error(sk, EMSGSIZE, rt->rt_dst, inet->inet_dport, mtu);
+ ip_local_error(sk, EMSGSIZE, fl4->daddr, inet->inet_dport, mtu);
return -EMSGSIZE;
}
@@ -1427,7 +1429,7 @@ struct sk_buff *ip_make_skb(struct sock *sk,
if (err)
return ERR_PTR(err);
- err = __ip_append_data(sk, &queue, &cork, getfrag,
+ err = __ip_append_data(sk, fl4, &queue, &cork, getfrag,
from, length, transhdrlen, flags);
if (err) {
__ip_flush_pending_frames(sk, &queue, &cork);
@@ -1503,7 +1505,7 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
sk->sk_priority = skb->priority;
sk->sk_protocol = ip_hdr(skb)->protocol;
sk->sk_bound_dev_if = arg->bound_dev_if;
- ip_append_data(sk, ip_reply_glue_bits, arg->iov->iov_base, len, 0,
+ ip_append_data(sk, &fl4, ip_reply_glue_bits, arg->iov->iov_base, len, 0,
&ipc, &rt, MSG_DONTWAIT);
if ((skb = skb_peek(&sk->sk_write_queue)) != NULL) {
if (arg->csumoffset >= 0)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 6fee91f..11e17804 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -595,7 +595,7 @@ back_from_confirm:
if (!ipc.addr)
ipc.addr = fl4.daddr;
lock_sock(sk);
- err = ip_append_data(sk, ip_generic_getfrag,
+ err = ip_append_data(sk, &fl4, ip_generic_getfrag,
msg->msg_iov, len, 0,
&ipc, &rt, msg->msg_flags);
if (err)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 006e2cc..66341a3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -822,6 +822,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
getfrag = is_udplite ? udplite_getfrag : ip_generic_getfrag;
+ fl4 = &inet->cork.fl.u.ip4;
if (up->pending) {
/*
* There are pending frames.
@@ -920,7 +921,6 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
if (connected)
rt = (struct rtable *)sk_dst_check(sk, 0);
- fl4 = &inet->cork.fl.u.ip4;
if (rt == NULL) {
struct net *net = sock_net(sk);
@@ -989,9 +989,9 @@ back_from_confirm:
do_append_data:
up->len += ulen;
- err = ip_append_data(sk, getfrag, msg->msg_iov, ulen,
- sizeof(struct udphdr), &ipc, &rt,
- corkreq ? msg->msg_flags|MSG_MORE : msg->msg_flags);
+ err = ip_append_data(sk, fl4, getfrag, msg->msg_iov, ulen,
+ sizeof(struct udphdr), &ipc, &rt,
+ corkreq ? msg->msg_flags|MSG_MORE : msg->msg_flags);
if (err)
udp_flush_pending_frames(sk);
else if (!corkreq)
@@ -1031,6 +1031,7 @@ EXPORT_SYMBOL(udp_sendmsg);
int udp_sendpage(struct sock *sk, struct page *page, int offset,
size_t size, int flags)
{
+ struct inet_sock *inet = inet_sk(sk);
struct udp_sock *up = udp_sk(sk);
int ret;
@@ -1055,7 +1056,8 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
return -EINVAL;
}
- ret = ip_append_page(sk, page, offset, size, flags);
+ ret = ip_append_page(sk, &inet->cork.fl.u.ip4,
+ page, offset, size, flags);
if (ret == -EOPNOTSUPP) {
release_sock(sk);
return sock_no_sendpage(sk->sk_socket, page, offset,
--
1.7.5.1
^ permalink raw reply related
* Re: [PATCH 1/2] ipv4: Pass flow keys down into datagram packet building engine.
From: Joe Perches @ 2011-05-09 1:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110508.180753.241908549.davem@davemloft.net>
On Sun, 8 May 2011, David Miller wrote:
> +static struct rtable *icmp_route_lookup(struct net *net,
> + struct flowi4 *fl4,
> + struct sk_buff *skb_in,
> const struct iphdr *iph,
> __be32 saddr, u8 tos,
> int type, int code,
> struct icmp_bxm *param)
> {
[]
> + memset(&fl4, 0, sizeof(fl4));
memset(fl4, 0, sizeof(*fl4));
> + fl4->daddr = (param->replyopts.opt.opt.srr ?
> + param->replyopts.opt.opt.faddr : iph->saddr);
^ permalink raw reply
* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Håkon Løvdal @ 2011-05-09 1:30 UTC (permalink / raw)
To: David Miller
Cc: mfmooney, joe, aquini, kernel-janitors, fubar, andy, shemminger,
netdev, nikai
In-Reply-To: <20110508.171204.27789908.davem@davemloft.net>
On 9 May 2011 02:12, David Miller <davem@davemloft.net> wrote:
> From: Håkon Løvdal <hlovdal@gmail.com>
> Date: Mon, 9 May 2011 02:08:41 +0200
>
>> void bond_3ad_state_machine_handler(struct work_struct *work)
>> {
>> struct bonding *bond = container_of(work, struct bonding,
>> ad_work.work);
>> struct port *port;
>> struct aggregator *aggregator;
>>
>> read_lock(&bond->lock);
>>
>> if (! bond->kill_timers) {
>>
>> //check if there are any slaves
>> if (bond->slave_cnt != 0) {
>> ...
>> }
>> queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
>> }
>> read_unlock(&bond->lock);
>> }
>>
>>
>> And this was what I trying to reccommend against (and which the
>> stackoverflow question is about).
>
> I really don't see anything wrong with either approach.
The example above looks innocent since it is so small. But written
this way, the all of core of the function ("...") will have two extra,
unnecessary indentation levels. My rule of thumb is to always test for
exceptions, not the normal case.
Expanding the example to cover the full function, it could be like the
following:
void bond_3ad_state_machine_handler(struct work_struct *work)
{
struct bonding *bond = container_of(work, struct bonding,
ad_work.work);
struct port *port;
struct aggregator *aggregator;
read_lock(&bond->lock);
if (! bond->kill_timers)
{
//check if there are any slaves
if (bond->slave_cnt != 0)
{
int uninitialized = 0;
// check if agg_select_timer timer after
initialize is timed out
if (BOND_AD_INFO(bond).agg_select_timer &&
!(--BOND_AD_INFO(bond).agg_select_timer)) {
// select the active aggregator for the bond
if ((port = __get_first_port(bond))) {
if (port->slave) {
aggregator =
__get_first_agg(port);
ad_agg_selection_logic(aggregator);
} else {
pr_warning("%s:
Warning: bond's first port is uninitialized\n",
bond->dev->name);
uninitialized = 1;
}
}
if (! uninitialized)|
bond_3ad_set_carrier(bond);
}
if (! uninitialized) {
// for each port run the state machines
for (port = __get_first_port(bond);
port; port = __get_next_port(port)) {
if (port->slave) {
/* Lock around state
machines to protect data accessed
* by all (e.g.,
port->sm_vars). ad_rx_machine may run
* concurrently due to
incoming LACPDU.
*/
__get_state_machine_lock(port);
ad_rx_machine(NULL, port);
ad_periodic_machine(port);
ad_port_selection_logic(port);
ad_mux_machine(port);
ad_tx_machine(port);
// turn off the BEGIN
bit, since we already handled it
if (port->sm_vars &
AD_PORT_BEGIN)
port->sm_vars
&= ~AD_PORT_BEGIN;
__release_state_machine_lock(port);
} else {
pr_warning("%s:
Warning: Found an uninitialized port\n",
bond->dev->name);
}
}
}
}
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
}
read_unlock(&bond->lock);
}
Notice for instance "aggregator = __get_first_agg(port);" now have 5
levels of indentation in addition to the function body level, compared
to just 2 in the original.
That extra indentation is one thing, but the most important in my opinion
is that layout of the code becomes harder to read. This is because the
error handling code will typically be much more intrusive in the "layout".
Good code written like
if (error_check) {
error
handling
code
}
normal
code
here
is possible to quickly scan/read, but if code is written like
if (some_check) {
error
handling
code
} else {
normal
code
here
}
or
if (some_check) {
normal
code
here
} else {
error
handling
code
}
you have to go into slow gear and read and mentally process (relatively)
much more of the code to get the same picture. Combine a few such if/else,
nested - now everything is tightly intervened and impossible to quickly
grasp and/or hold in your head.
And the "uninitialized" variable is a crutch.
BR Håkon Løvdal
^ 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