| Message ID | 1474393978-27536-1-git-send-email-pshelar@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
On 20 September 2016 at 20:52, Pravin B Shelar <pshelar@ovn.org> wrote: > OVS IPsec tunnel support has issues: > 1. It only works for GRE. 2. only works on Debian. 3. It does not allow user to match on packet-mark > on packet received on tunnel ports. > Therefore following patch provide alternative to completely > disable ipsec-tunnel support by vswitchd command line option. > This way user can use external daemon to manage IPsec tunnel > traffic and stir it using skb-mark match action in OVS bridge. > This patch deprecates support for IPsec tunnel port. > There are other alternative solutions worth to mention: 1) remove the special meaning of skb_mark bit #0 and update ovs-monitor-ipsec not to depend on harcoded skb_mark value at all (I think this can be done with some trickery); 2) allow users to chose OVS mode where OVS can be explicitly told to either use skb_mark for its own needs (e.g. IPsec) OR to pass skb_mark to OpenFlow pipeline as-is; 3) leave bit #0 assigned to IPsec and let OpenFlow to match only on bits #1-32. Your solutions is kinda like 2), except it discourages uses to configure OVS in a way where it consumes skb_mark for itself. I think solutions 1) could be implemented even after your patch. Except, maybe then we should not mention that IPsec will be deprecated in the next release. Also, I would need to think how to address corner cases if ovs-monitor-ipsec can't use skb_mark anymore. Solution 3) would be great from ovs-monitor-ipsec perspective because it would not need to change. However, it possibly would make OpenFlow skb_mark matching look weird compared to other fields that OVS can match on. > Signed-off-by: Pravin B Shelar <pshelar@ovn.org> > --- > NEWS | 2 ++ > debian/changelog | 2 ++ > debian/control | 1 + > lib/netdev-vport.c | 3 +++ > lib/netdev.c | 1 + > lib/netdev.h | 1 + > ofproto/tunnel.c | 30 ++++++++++++++++++++++-------- > ofproto/tunnel.h | 2 ++ > vswitchd/ovs-vswitchd.c | 7 +++++++ > vswitchd/vswitch.xml | 8 ++++++++ > 10 files changed, 49 insertions(+), 8 deletions(-) > > diff --git a/NEWS b/NEWS > index 21ab538..057edfd 100644 > --- a/NEWS > +++ b/NEWS > @@ -149,6 +149,8 @@ v2.6.0 - xx xxx xxxx > * Flow based tunnel match and action can be used for IPv6 address > using > tun_ipv6_src, tun_ipv6_dst fields. > * Added support for IPv6 tunnels, for details checkout FAQ. > + * Allow external IPsec tunnel management. Deprecated support for > IPsec > + tunnels ports. > s/tunnels/tunnel - A wrapper script, 'ovs-tcpdump', to easily port-mirror an OVS port and > watch with tcpdump > - Introduce --no-self-confinement flag that allows daemons to work with > diff --git a/debian/changelog b/debian/changelog > index d73e636..8add140 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -108,6 +108,8 @@ openvswitch (2.6.0-1) unstable; urgency=low > * Flow based tunnel match and action can be used for IPv6 address > using > tun_ipv6_src, tun_ipv6_dst fields. > * Added support for IPv6 tunnels, for details checkout FAQ. > + * Allow external IPsec tunnel management. Deprecated support for > IPsec > + tunnels ports. > same here > - A wrapper script, 'ovs-tcpdump', to easily port-mirror an OVS port > and > watch with tcpdump > - Introduce --no-self-confinement flag that allows daemons to work with > diff --git a/debian/control b/debian/control > index 6e704f1..da86fe9 100644 > --- a/debian/control > +++ b/debian/control > @@ -200,6 +200,7 @@ Description: Open vSwitch GRE-over-IPsec support > . > The ovs-monitor-ipsec script provides support for encrypting GRE > tunnels with IPsec. > + IPsec tunnels support is deprecated. > s/tunnels/tunneling > > Package: openvswitch-pki > Architecture: all > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index 8d22cf5..6bf4d2d 100755 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -543,6 +543,9 @@ set_tunnel_config(struct netdev *dev_, const struct > smap *args) > static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; > static pid_t pid = 0; > > + VLOG_ERR("%s: OVS IPsec tunnel support is deprecated. " > + "See man page for details", name); > + > I believe IPsec does not work anymore with the command line argument you introduced. Should you give a special warning message in that case? > #ifndef _WIN32 > ovs_mutex_lock(&mutex); > if (pid <= 0) { > diff --git a/lib/netdev.c b/lib/netdev.c > index 6c4c657..a626f18 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -98,6 +98,7 @@ static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 20); > > static void restore_all_flags(void *aux OVS_UNUSED); > void update_device_args(struct netdev *, const struct shash *args); > +bool enable_ipsec_tnl = true; > Wouldn't it be preferred that enable_ipsec_tnl is set to false by default? Otherwise, Open vSwitch users installing OVS from rpm and deb package may not make use of your change (see init.d script and systemd configuration file that actually starts ovs-vswitchd process). > > int > netdev_n_txq(const struct netdev *netdev) > diff --git a/lib/netdev.h b/lib/netdev.h > index 634c665..870ce22 100644 > --- a/lib/netdev.h > +++ b/lib/netdev.h > @@ -299,6 +299,7 @@ int netdev_dump_queue_stats(const struct netdev *, > netdev_dump_queue_stats_cb *, void *aux); > > extern struct seq *tnl_conf_seq; > +extern bool enable_ipsec_tnl; > > #ifndef _WIN32 > void netdev_get_addrs_list_flush(void); > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c > index 9a69071..595a1bd 100644 > --- a/ofproto/tunnel.c > +++ b/ofproto/tunnel.c > @@ -164,7 +164,10 @@ tnl_port_add__(const struct ofport_dpif *ofport, > const struct netdev *netdev, > tnl_port->match.ipv6_dst = cfg->ipv6_dst; > tnl_port->match.ip_src_flow = cfg->ip_src_flow; > tnl_port->match.ip_dst_flow = cfg->ip_dst_flow; > - tnl_port->match.pkt_mark = cfg->ipsec ? IPSEC_MARK : 0; > + > + if (enable_ipsec_tnl) { > + tnl_port->match.pkt_mark = cfg->ipsec ? IPSEC_MARK : 0; > + } > tnl_port->match.in_key_flow = cfg->in_key_flow; > tnl_port->match.odp_port = odp_port; > > @@ -357,7 +360,9 @@ tnl_process_ecn(struct flow *flow) > flow->nw_tos |= IP_ECN_CE; > } > > - flow->pkt_mark &= ~IPSEC_MARK; > + if (enable_ipsec_tnl) { > + flow->pkt_mark &= ~IPSEC_MARK; > + } > return true; > } > > @@ -383,8 +388,11 @@ tnl_wc_init(struct flow *flow, struct flow_wildcards > *wc) > wc->masks.tunnel.tp_src = 0; > wc->masks.tunnel.tp_dst = 0; > > - memset(&wc->masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark); > - > + if (enable_ipsec_tnl) { > + memset(&wc->masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark); > + } else { > + memset(&wc->masks.pkt_mark, 0, sizeof wc->masks.pkt_mark); > + } > if (is_ip_any(flow) > && IP_ECN_is_ce(flow->tunnel.ip_tos)) { > wc->masks.nw_tos |= IP_ECN_MASK; > @@ -435,8 +443,10 @@ tnl_port_send(const struct ofport_dpif *ofport, > struct flow *flow, > flow->tunnel.ipv6_dst = in6addr_any; > } > } > - flow->pkt_mark |= tnl_port->match.pkt_mark; > - wc->masks.pkt_mark |= tnl_port->match.pkt_mark; > + if (enable_ipsec_tnl) { > + flow->pkt_mark |= tnl_port->match.pkt_mark; > + wc->masks.pkt_mark |= tnl_port->match.pkt_mark; > + } > > if (!cfg->out_key_flow) { > flow->tunnel.tun_id = cfg->out_key; > @@ -561,7 +571,9 @@ tnl_find(const struct flow *flow) > OVS_REQ_RDLOCK(rwlock) > match.ipv6_dst = flow_tnl_src(&flow->tunnel); > } > match.odp_port = flow->in_port.odp_port; > - match.pkt_mark = flow->pkt_mark; > + if (enable_ipsec_tnl) { > + match.pkt_mark = flow->pkt_mark; > + } > match.in_key_flow = in_key_flow; > match.ip_dst_flow = ip_dst_flow; > match.ip_src_flow = ip_src == IP_SRC_FLOW; > @@ -616,7 +628,9 @@ tnl_match_fmt(const struct tnl_match *match, struct ds > *ds) > } > > ds_put_format(ds, ", dp port=%"PRIu32, match->odp_port); > - ds_put_format(ds, ", pkt mark=%"PRIu32, match->pkt_mark); > + if (enable_ipsec_tnl) { > + ds_put_format(ds, ", pkt mark=%"PRIu32, match->pkt_mark); > + } > } > > static void > diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h > index b0ec67c..b2f5590 100644 > --- a/ofproto/tunnel.h > +++ b/ofproto/tunnel.h > @@ -30,6 +30,8 @@ struct ofport_dpif; > struct netdev; > struct netdev_tnl_build_header_params; > > +extern bool enable_ipsec_tnl; > + > void ofproto_tunnel_init(void); > bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev > *, > odp_port_t, bool native_tnl, const char name[]); > diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c > index 72448bb..84c9b96 100644 > --- a/vswitchd/ovs-vswitchd.c > +++ b/vswitchd/ovs-vswitchd.c > @@ -146,6 +146,7 @@ parse_options(int argc, char *argv[], char > **unixctl_pathp) > DAEMON_OPTION_ENUMS, > OPT_DPDK, > OPT_DUMMY_NUMA, > + OPT_DISABLE_IPSEC_TNL, }; > static const struct option long_options[] = { > {"help", no_argument, NULL, 'h'}, > @@ -161,6 +162,7 @@ parse_options(int argc, char *argv[], char > **unixctl_pathp) > {"disable-system", no_argument, NULL, OPT_DISABLE_SYSTEM}, > {"dpdk", optional_argument, NULL, OPT_DPDK}, > {"dummy-numa", required_argument, NULL, OPT_DUMMY_NUMA}, > + {"external-ipsec-tunneling", no_argument, NULL, > OPT_DISABLE_IPSEC_TNL}, > How about having two Open vSwitch modes: 1. mode where skb mark passes through as-is and can be matched with OpenFlow rules; 2. mode where Open vSwitch uses all bits in skb_mark and uses them for features like IPsec. Anyway, this may unnecessarily complicate OVS, especially, if there are no other services besides IPsec that would want to make OVS-specific use of skb_mark. Also, unless ovs-vswitchd process is started from command line, it may not be easy to override these flags with "systemctl restart openvswitch" command. > {NULL, 0, NULL, 0}, > }; > char *short_options = ovs_cmdl_long_options_to_short > _options(long_options); > @@ -220,6 +222,10 @@ parse_options(int argc, char *argv[], char > **unixctl_pathp) > ovs_numa_set_dummy(optarg); > break; > > + case OPT_DISABLE_IPSEC_TNL: > + enable_ipsec_tnl = false; > Depending on what you want to be default behavior, this may have to be inverted to true (see my other comment where you set the default value) > + break; > + > default: > abort(); > } > @@ -259,6 +265,7 @@ usage(void) > ); > printf("\nOther options:\n" > " --unixctl=SOCKET override default control socket > name\n" > + " --external-ipsec-tunneling Disable native IPsec > tunnel support to allow external IPsec tunnel management\n" > " -h, --help display this help message\n" > " -V, --version display version information\n"); > exit(EXIT_SUCCESS); > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index e73023d..d6fc508 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -2008,6 +2008,14 @@ > <dd> > An Ethernet over RFC 2890 Generic Routing Encapsulation over > IPv4/IPv6 > IPsec tunnel. > + IPsec tunnel port are deprecated. The support will be > completely > s/port/ports s/The support/IPsec support > + removed in next version. > s/next version/the next release (Maybe mention exact release here as well). > + Better way to implement IPsec Tunnel is to use external > daemon to manage s/Better way to implement IPsec Tunnel/A better way to secure OVS tunnels through IPsec > + IPsec tunnel traffic using strongswan and stir it using > skb-mark match > s/strongswan/strongSwan I am not sure what you meant here with word "stir"? Did you mean "steer"? If so, then how about some other word like "differentiate"? > + action in OVS bridge. To match on skb-marked tunnel packet, > OVS IPsec + tunnel port support needs to be disabled using command line > ovs-vswitchd > + daemon option (--external-ipsec-tunneling). + > </dd> > > <dt><code>vxlan</code></dt> > -- > 1.9.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev >
On Thu, Sep 22, 2016 at 11:59 AM, Ansis Atteka <ansisatteka@gmail.com> wrote: > > > On 20 September 2016 at 20:52, Pravin B Shelar <pshelar@ovn.org> wrote: >> >> OVS IPsec tunnel support has issues: >> 1. It only works for GRE. >> >> 2. only works on Debian. >> >> 3. It does not allow user to match on packet-mark >> on packet received on tunnel ports. >> >> >> >> >> Therefore following patch provide alternative to completely >> disable ipsec-tunnel support by vswitchd command line option. >> This way user can use external daemon to manage IPsec tunnel >> traffic and stir it using skb-mark match action in OVS bridge. >> >> >> This patch deprecates support for IPsec tunnel port. > > > There are other alternative solutions worth to mention: > 1) remove the special meaning of skb_mark bit #0 and update > ovs-monitor-ipsec not to depend on harcoded skb_mark value at all (I think > this can be done with some trickery); I am not sure what does this mean. How are you going match on IPsec traffic? > 2) allow users to chose OVS mode where OVS can be explicitly told to either > use skb_mark for its own needs (e.g. IPsec) OR to pass skb_mark to OpenFlow > pipeline as-is; This was basically this patch does but I have sent another patch to just deprecate IPsec support. I have mentioned reasoning for the change there. http://openvswitch.org/pipermail/dev/2016-September/079770.html > 3) leave bit #0 assigned to IPsec and let OpenFlow to match only on bits > #1-32. > > Your solutions is kinda like 2), except it discourages uses to configure OVS > in a way where it consumes skb_mark for itself. > > I think solutions 1) could be implemented even after your patch. Except, > maybe then we should not mention that IPsec will be deprecated in the next > release. Also, I would need to think how to address corner cases if > ovs-monitor-ipsec can't use skb_mark anymore. > > Solution 3) would be great from ovs-monitor-ipsec perspective because it > would not need to change. However, it possibly would make OpenFlow skb_mark > matching look weird compared to other fields that OVS can match on. > I do not like solution 3. It does not allows OVS user to use all bits of skb-mark even when there is no IPSEC involved which is what linux networking stack provide.
On Fri, Sep 23, 2016 at 1:12 AM, pravin shelar <pshelar@ovn.org> wrote: > On Thu, Sep 22, 2016 at 11:59 AM, Ansis Atteka <ansisatteka@gmail.com> wrote: >> >> >> On 20 September 2016 at 20:52, Pravin B Shelar <pshelar@ovn.org> wrote: >>> >>> OVS IPsec tunnel support has issues: >>> 1. It only works for GRE. >>> >>> 2. only works on Debian. >>> >>> 3. It does not allow user to match on packet-mark >>> on packet received on tunnel ports. >>> >>> >>> >>> >>> Therefore following patch provide alternative to completely >>> disable ipsec-tunnel support by vswitchd command line option. >>> This way user can use external daemon to manage IPsec tunnel >>> traffic and stir it using skb-mark match action in OVS bridge. >>> >>> >>> This patch deprecates support for IPsec tunnel port. >> >> >> There are other alternative solutions worth to mention: >> 1) remove the special meaning of skb_mark bit #0 and update >> ovs-monitor-ipsec not to depend on harcoded skb_mark value at all (I think >> this can be done with some trickery); > > I am not sure what does this mean. How are you going match on IPsec traffic? > >> 2) allow users to chose OVS mode where OVS can be explicitly told to either >> use skb_mark for its own needs (e.g. IPsec) OR to pass skb_mark to OpenFlow >> pipeline as-is; > > This was basically this patch does but I have sent another patch to > just deprecate IPsec support. I have mentioned reasoning for the > change there. > > http://openvswitch.org/pipermail/dev/2016-September/079770.html > >> 3) leave bit #0 assigned to IPsec and let OpenFlow to match only on bits >> #1-32. >> >> Your solutions is kinda like 2), except it discourages uses to configure OVS >> in a way where it consumes skb_mark for itself. >> >> I think solutions 1) could be implemented even after your patch. Except, >> maybe then we should not mention that IPsec will be deprecated in the next >> release. Also, I would need to think how to address corner cases if >> ovs-monitor-ipsec can't use skb_mark anymore. >> >> Solution 3) would be great from ovs-monitor-ipsec perspective because it >> would not need to change. However, it possibly would make OpenFlow skb_mark >> matching look weird compared to other fields that OVS can match on. >> > > I do not like solution 3. It does not allows OVS user to use all bits > of skb-mark even when there is no IPSEC involved which is what linux > networking stack provide. The reason why IPsec needed this one skb mark bit was because, otherwise, Linux IP stack (in particular "xfrm lookup" hook - https://upload.wikimedia.org/wikipedia/commons/3/37/Netfilter-packet-flow.svg) really does not have slightest idea whether GRE packet came from gre or ipsec_gre port. If this bit is taken away from ovs-monitor-ipsec, because we want OVS users to be able to use all 32 bits of skb mark in an arbitrary manner, then, yes, ipsec_* tunnel support must be removed, because, then from Linux IP stack point of view ipsec_gre and gre would look the same. So let's just move on with your patch then. I guess you will send V2 after addressing implementation related comments that I had?
On Fri, Sep 23, 2016 at 12:54 PM, Ansis Atteka <aatteka@nicira.com> wrote: > On Fri, Sep 23, 2016 at 1:12 AM, pravin shelar <pshelar@ovn.org> wrote: >> On Thu, Sep 22, 2016 at 11:59 AM, Ansis Atteka <ansisatteka@gmail.com> wrote: >>> >>> >>> On 20 September 2016 at 20:52, Pravin B Shelar <pshelar@ovn.org> wrote: >>>> >>>> OVS IPsec tunnel support has issues: >>>> 1. It only works for GRE. >>>> >>>> 2. only works on Debian. >>>> >>>> 3. It does not allow user to match on packet-mark >>>> on packet received on tunnel ports. >>>> >>>> >>>> >>>> >>>> Therefore following patch provide alternative to completely >>>> disable ipsec-tunnel support by vswitchd command line option. >>>> This way user can use external daemon to manage IPsec tunnel >>>> traffic and stir it using skb-mark match action in OVS bridge. >>>> >>>> >>>> This patch deprecates support for IPsec tunnel port. >>> >>> >>> There are other alternative solutions worth to mention: >>> 1) remove the special meaning of skb_mark bit #0 and update >>> ovs-monitor-ipsec not to depend on harcoded skb_mark value at all (I think >>> this can be done with some trickery); >> >> I am not sure what does this mean. How are you going match on IPsec traffic? >> >>> 2) allow users to chose OVS mode where OVS can be explicitly told to either >>> use skb_mark for its own needs (e.g. IPsec) OR to pass skb_mark to OpenFlow >>> pipeline as-is; >> >> This was basically this patch does but I have sent another patch to >> just deprecate IPsec support. I have mentioned reasoning for the >> change there. >> >> http://openvswitch.org/pipermail/dev/2016-September/079770.html >> >>> 3) leave bit #0 assigned to IPsec and let OpenFlow to match only on bits >>> #1-32. >>> >>> Your solutions is kinda like 2), except it discourages uses to configure OVS >>> in a way where it consumes skb_mark for itself. >>> >>> I think solutions 1) could be implemented even after your patch. Except, >>> maybe then we should not mention that IPsec will be deprecated in the next >>> release. Also, I would need to think how to address corner cases if >>> ovs-monitor-ipsec can't use skb_mark anymore. >>> >>> Solution 3) would be great from ovs-monitor-ipsec perspective because it >>> would not need to change. However, it possibly would make OpenFlow skb_mark >>> matching look weird compared to other fields that OVS can match on. >>> >> >> I do not like solution 3. It does not allows OVS user to use all bits >> of skb-mark even when there is no IPSEC involved which is what linux >> networking stack provide. > > The reason why IPsec needed this one skb mark bit was because, > otherwise, Linux IP stack (in particular "xfrm lookup" hook - > https://upload.wikimedia.org/wikipedia/commons/3/37/Netfilter-packet-flow.svg) > really does not have slightest idea whether GRE packet came from gre > or ipsec_gre port. > > If this bit is taken away from ovs-monitor-ipsec, because we want OVS > users to be able to use all 32 bits of skb mark in an arbitrary > manner, then, yes, ipsec_* tunnel support must be removed, because, > then from Linux IP stack point of view ipsec_gre and gre would look > the same. So let's just move on with your patch then. > I am not objecting to use one bit for IPsec tunnels. That is required to make IPsec tunnel work on linux. My proposal is to let user set skb mark using open-flow pipeline. So that he has complete control over all bits in skb-mark. In this scheme user configure skb-mark and xfrm to implement IPsec tunnels. OVS does not need to support this port type. > I guess you will send V2 after addressing implementation related > comments that I had? I have posted another patch to deprecate IPsec. http://openvswitch.org/pipermail/dev/2016-September/079770.html
diff --git a/NEWS b/NEWS index 21ab538..057edfd 100644 --- a/NEWS +++ b/NEWS @@ -149,6 +149,8 @@ v2.6.0 - xx xxx xxxx * Flow based tunnel match and action can be used for IPv6 address using tun_ipv6_src, tun_ipv6_dst fields. * Added support for IPv6 tunnels, for details checkout FAQ. + * Allow external IPsec tunnel management. Deprecated support for IPsec + tunnels ports. - A wrapper script, 'ovs-tcpdump', to easily port-mirror an OVS port and watch with tcpdump - Introduce --no-self-confinement flag that allows daemons to work with diff --git a/debian/changelog b/debian/changelog index d73e636..8add140 100644 --- a/debian/changelog +++ b/debian/changelog @@ -108,6 +108,8 @@ openvswitch (2.6.0-1) unstable; urgency=low * Flow based tunnel match and action can be used for IPv6 address using tun_ipv6_src, tun_ipv6_dst fields. * Added support for IPv6 tunnels, for details checkout FAQ. + * Allow external IPsec tunnel management. Deprecated support for IPsec + tunnels ports. - A wrapper script, 'ovs-tcpdump', to easily port-mirror an OVS port and watch with tcpdump - Introduce --no-self-confinement flag that allows daemons to work with diff --git a/debian/control b/debian/control index 6e704f1..da86fe9 100644 --- a/debian/control +++ b/debian/control @@ -200,6 +200,7 @@ Description: Open vSwitch GRE-over-IPsec support . The ovs-monitor-ipsec script provides support for encrypting GRE tunnels with IPsec. + IPsec tunnels support is deprecated. Package: openvswitch-pki Architecture: all diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 8d22cf5..6bf4d2d 100755 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -543,6 +543,9 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; static pid_t pid = 0; + VLOG_ERR("%s: OVS IPsec tunnel support is deprecated. " + "See man page for details", name); + #ifndef _WIN32 ovs_mutex_lock(&mutex); if (pid <= 0) { diff --git a/lib/netdev.c b/lib/netdev.c index 6c4c657..a626f18 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -98,6 +98,7 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); static void restore_all_flags(void *aux OVS_UNUSED); void update_device_args(struct netdev *, const struct shash *args); +bool enable_ipsec_tnl = true; int netdev_n_txq(const struct netdev *netdev) diff --git a/lib/netdev.h b/lib/netdev.h index 634c665..870ce22 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -299,6 +299,7 @@ int netdev_dump_queue_stats(const struct netdev *, netdev_dump_queue_stats_cb *, void *aux); extern struct seq *tnl_conf_seq; +extern bool enable_ipsec_tnl; #ifndef _WIN32 void netdev_get_addrs_list_flush(void); diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c index 9a69071..595a1bd 100644 --- a/ofproto/tunnel.c +++ b/ofproto/tunnel.c @@ -164,7 +164,10 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev, tnl_port->match.ipv6_dst = cfg->ipv6_dst; tnl_port->match.ip_src_flow = cfg->ip_src_flow; tnl_port->match.ip_dst_flow = cfg->ip_dst_flow; - tnl_port->match.pkt_mark = cfg->ipsec ? IPSEC_MARK : 0; + + if (enable_ipsec_tnl) { + tnl_port->match.pkt_mark = cfg->ipsec ? IPSEC_MARK : 0; + } tnl_port->match.in_key_flow = cfg->in_key_flow; tnl_port->match.odp_port = odp_port; @@ -357,7 +360,9 @@ tnl_process_ecn(struct flow *flow) flow->nw_tos |= IP_ECN_CE; } - flow->pkt_mark &= ~IPSEC_MARK; + if (enable_ipsec_tnl) { + flow->pkt_mark &= ~IPSEC_MARK; + } return true; } @@ -383,8 +388,11 @@ tnl_wc_init(struct flow *flow, struct flow_wildcards *wc) wc->masks.tunnel.tp_src = 0; wc->masks.tunnel.tp_dst = 0; - memset(&wc->masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark); - + if (enable_ipsec_tnl) { + memset(&wc->masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark); + } else { + memset(&wc->masks.pkt_mark, 0, sizeof wc->masks.pkt_mark); + } if (is_ip_any(flow) && IP_ECN_is_ce(flow->tunnel.ip_tos)) { wc->masks.nw_tos |= IP_ECN_MASK; @@ -435,8 +443,10 @@ tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow, flow->tunnel.ipv6_dst = in6addr_any; } } - flow->pkt_mark |= tnl_port->match.pkt_mark; - wc->masks.pkt_mark |= tnl_port->match.pkt_mark; + if (enable_ipsec_tnl) { + flow->pkt_mark |= tnl_port->match.pkt_mark; + wc->masks.pkt_mark |= tnl_port->match.pkt_mark; + } if (!cfg->out_key_flow) { flow->tunnel.tun_id = cfg->out_key; @@ -561,7 +571,9 @@ tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock) match.ipv6_dst = flow_tnl_src(&flow->tunnel); } match.odp_port = flow->in_port.odp_port; - match.pkt_mark = flow->pkt_mark; + if (enable_ipsec_tnl) { + match.pkt_mark = flow->pkt_mark; + } match.in_key_flow = in_key_flow; match.ip_dst_flow = ip_dst_flow; match.ip_src_flow = ip_src == IP_SRC_FLOW; @@ -616,7 +628,9 @@ tnl_match_fmt(const struct tnl_match *match, struct ds *ds) } ds_put_format(ds, ", dp port=%"PRIu32, match->odp_port); - ds_put_format(ds, ", pkt mark=%"PRIu32, match->pkt_mark); + if (enable_ipsec_tnl) { + ds_put_format(ds, ", pkt mark=%"PRIu32, match->pkt_mark); + } } static void diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h index b0ec67c..b2f5590 100644 --- a/ofproto/tunnel.h +++ b/ofproto/tunnel.h @@ -30,6 +30,8 @@ struct ofport_dpif; struct netdev; struct netdev_tnl_build_header_params; +extern bool enable_ipsec_tnl; + void ofproto_tunnel_init(void); bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev *, odp_port_t, bool native_tnl, const char name[]); diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c index 72448bb..84c9b96 100644 --- a/vswitchd/ovs-vswitchd.c +++ b/vswitchd/ovs-vswitchd.c @@ -146,6 +146,7 @@ parse_options(int argc, char *argv[], char **unixctl_pathp) DAEMON_OPTION_ENUMS, OPT_DPDK, OPT_DUMMY_NUMA, + OPT_DISABLE_IPSEC_TNL, }; static const struct option long_options[] = { {"help", no_argument, NULL, 'h'}, @@ -161,6 +162,7 @@ parse_options(int argc, char *argv[], char **unixctl_pathp) {"disable-system", no_argument, NULL, OPT_DISABLE_SYSTEM}, {"dpdk", optional_argument, NULL, OPT_DPDK}, {"dummy-numa", required_argument, NULL, OPT_DUMMY_NUMA}, + {"external-ipsec-tunneling", no_argument, NULL, OPT_DISABLE_IPSEC_TNL}, {NULL, 0, NULL, 0}, }; char *short_options = ovs_cmdl_long_options_to_short_options(long_options); @@ -220,6 +222,10 @@ parse_options(int argc, char *argv[], char **unixctl_pathp) ovs_numa_set_dummy(optarg); break; + case OPT_DISABLE_IPSEC_TNL: + enable_ipsec_tnl = false; + break; + default: abort(); } @@ -259,6 +265,7 @@ usage(void) ); printf("\nOther options:\n" " --unixctl=SOCKET override default control socket name\n" + " --external-ipsec-tunneling Disable native IPsec tunnel support to allow external IPsec tunnel management\n" " -h, --help display this help message\n" " -V, --version display version information\n"); exit(EXIT_SUCCESS); diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index e73023d..d6fc508 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -2008,6 +2008,14 @@ <dd> An Ethernet over RFC 2890 Generic Routing Encapsulation over IPv4/IPv6 IPsec tunnel. + IPsec tunnel port are deprecated. The support will be completely + removed in next version. + Better way to implement IPsec Tunnel is to use external daemon to manage + IPsec tunnel traffic using strongswan and stir it using skb-mark match + action in OVS bridge. To match on skb-marked tunnel packet, OVS IPsec + tunnel port support needs to be disabled using command line ovs-vswitchd + daemon option (--external-ipsec-tunneling). + </dd> <dt><code>vxlan</code></dt>
OVS IPsec tunnel support has issues: 1. It only works for GRE. 2. only works on Debian. 3. It does not allow user to match on packet-mark on packet received on tunnel ports. Therefore following patch provide alternative to completely disable ipsec-tunnel support by vswitchd command line option. This way user can use external daemon to manage IPsec tunnel traffic and stir it using skb-mark match action in OVS bridge. This patch deprecates support for IPsec tunnel port. Signed-off-by: Pravin B Shelar <pshelar@ovn.org> --- NEWS | 2 ++ debian/changelog | 2 ++ debian/control | 1 + lib/netdev-vport.c | 3 +++ lib/netdev.c | 1 + lib/netdev.h | 1 + ofproto/tunnel.c | 30 ++++++++++++++++++++++-------- ofproto/tunnel.h | 2 ++ vswitchd/ovs-vswitchd.c | 7 +++++++ vswitchd/vswitch.xml | 8 ++++++++ 10 files changed, 49 insertions(+), 8 deletions(-)