On Thu, May 24, 2012 at 09:47:38AM -0700, Ben Pfaff wrote:
> 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.
Sorry about that. I have a bit of trouble remembering to switch
tabbing modes in my editor depending on if I am in user-space or the
datapath.
> > + 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).
I guess that it should be easy enough to set a flag if any of the parsed
configurations use ipsec and perform the pid check if so.
As it is, I wouldn't be at all surprised if my series breaks ipsec as
I haven't tested it (with or without my changes).
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
