LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [*v3 PATCH 01/22] IPVS: netns, add basic init per netns.

To: hans@xxxxxxxxxxxxxxx
Subject: Re: [*v3 PATCH 01/22] IPVS: netns, add basic init per netns.
Cc: horms@xxxxxxxxxxxx, ja@xxxxxx, daniel.lezcano@xxxxxxx, wensong@xxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, Hans Schillstrom <hans.schillstrom@xxxxxxxxxxxx>
From: Jan Engelhardt <jengelh@xxxxxxxxxx>
Date: Fri, 31 Dec 2010 00:58:35 +0100 (CET)
On Thursday 2010-12-30 11:50, hans@xxxxxxxxxxxxxxx wrote:
>+++ b/include/net/ip_vs.h
>@@ -28,6 +28,15 @@
> #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
> #include <net/netfilter/nf_conntrack.h>
> #endif
>+#include <net/net_namespace.h>                /* Netw namespace */
>+
>+/*
>+ * Generic access of ipvs struct
>+ */
>+static inline struct netns_ipvs * net_ipvs(struct net* net)

Balancing the '*' between the ' ' would be nice... I know it's hard at this
time of the year ;-)

>diff --git a/include/net/netns/ip_vs.h b/include/net/netns/ip_vs.h
>new file mode 100644
>index 0000000..9068d95
>--- /dev/null
>+++ b/include/net/netns/ip_vs.h
>@@ -0,0 +1,26 @@
>+/*
>+ * ip_vs.h
>+ *
>+ *  Created on: Nov 23, 2010
>+ *  Author: hans
>+ */

Filenames, creation dates, author names (when they don't serve a copyright
notice), CVS $Id$ tags, etc. don't belong into files. The git log exists for
that very purpose.

>+struct netns_ipvs {
>+      int                     inc;            /* Incarnation */
>+};

Just my thoughts: Incarnation - haven't heard that in a while. ("inc"
is also used as an abbreviation for increment, so ~ ~). Other places
use "generation" as a term (such as the VFS on inode generation). [I
also get the feeling that (re)"incarnation" may require a death, so
it is different from "generation" after all -]

>+++ b/net/netfilter/ipvs/ip_vs_core.c
>@@ -1813,6 +1820,44 @@ static struct nf_hook_ops ip_vs_ops[] __read_mostly = {
> #endif
> };
>
>+/*
>+ *    Initialize IP Virtual Server netns mem.
>+ */
>+static int __net_init  __ip_vs_init(struct net *net)
>+{
>+      struct netns_ipvs *ipvs;
>+
>+      if (!net_eq(net, &init_net)) {
>+              pr_err("The final patch for enabling netns is missing\n");
>+              return -EPERM;
>+      }
>+      ipvs = (struct netns_ipvs *)net_generic(net, ip_vs_net_id);

Pointless cast is pointless.

>index c1c167a..ea390f8 100644
>--- a/net/netfilter/ipvs/ip_vs_sync.c
>+++ b/net/netfilter/ipvs/ip_vs_sync.c
>@@ -1639,3 +1639,31 @@ int stop_sync_thread(int state)
>
>       return 0;
> }
>+
>+/*
>+ * Initialize data struct for each netns
>+ */
>+static int __net_init __ip_vs_sync_init(struct net *net)
>+{
>+      return 0;
>+}
>+
>+static void __ip_vs_sync_cleanup(struct net *net)
>+{
>+      return;
>+}

The trailing return; in functions returning nothing can be (and
generally, is) omitted.

--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>