Re: [PATCH 04/21] vswitchd: Add iface_parse_tunnel

The concept seems OK to me here.  I have only a few minor comments.

On Thu, May 24, 2012 at 06:08:57PM +0900, Simon Horman wrote:

+#define TNL_F_CSUM          (1 << 0) /* Checksum packets. */
+#define TNL_F_TOS_INHERIT	(1 << 1) /* Inherit ToS from inner packet. */
+#define TNL_F_TTL_INHERIT	(1 << 2) /* Inherit TTL from inner packet. */
+#define TNL_F_DF_INHERIT	(1 << 3) /* Inherit DF bit from inner packet. */
+#define TNL_F_DF_DEFAULT	(1 << 4) /* Set DF bit if inherit off or
+                                      * not IP. */
+#define TNL_F_PMTUD		    (1 << 5) /* Enable path MTU discovery. */
+#define TNL_F_HDR_CACHE		(1 << 6) /* Enable tunnel header caching. */
+#define TNL_F_IPSEC		    (1 << 7) /* Traffic is IPsec encrypted. */
+#define TNL_F_IN_KEY	    (1 << 8) /* Tunnel port has input key. */
+#define TNL_F_OUT_KEY	    (1 << 9) /* Tunnel port has output key. */

Some of the above definitions use all spaces, others use tabs.  It's
OVS userspace code so it's better to use all spaces, I think.

+    if (is_ipsec) {
+        char *file_name = xasprintf("%s/%s", ovs_rundir(),
+                "ovs-monitor-ipsec.pid");
+        pid_t pid = read_pidfile(file_name);
+        free(file_name);
+        if (pid < 0) {
+            VLOG_ERR("%s: IPsec requires the ovs-monitor-ipsec daemon",
+                     iface_cfg->name);
+            goto err;
+        }

I just noticed that we re-read this pidfile every time we parse an
IPsec tunnel.  I guess that would be a big waste of time if we have a
lot of IPsec tunnels.  I'll make a note to consider fixing this
separately (it's not your problem).