Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-27 Thread pravin shelar
On Mon, Sep 26, 2016 at 1:15 PM, pravin shelar  wrote:
> On Mon, Sep 26, 2016 at 11:49 AM, Ansis Atteka  wrote:
>>
>>
>> On 26 September 2016 at 03:48, Pravin B Shelar  wrote:
>>>
>>> OVS GRE IPsec tunnel support has multiple issues, Therefore
>>
>> s/issues,/issues.
>>>
>>> it was deprecated in OVS 2.6.
>>>
>>> Following patch removes support GRE IPsec and allow external
>>
>> s/support/support for
>> s/allow/allows
>>>
>>> IPsec tunnel management for any type of tunnel not just GRE.
>>>
>>> e.g. user can encrpt Geneve or VxLan traffic.
>>
>> s/encrpt/encrypt
>>>
>>>
>>> It can be done by using openflow pipeline to set skb-mark
>>> and using xfrm to implement IPsec tunnels. xfrm can match
>>> on the skb-mark to encrypt selective tunnel traffic.
>>
>>
>> Some folks may misinterpret the paragraph above that we are recommending
>> them to use XFRM *directly* as an alternative. XFRM is just NetLink
>> interface to linux kernel to install IPsec keys after these keys have been
>> negotiated by IPsec keying daemon, such as strongSwan, openSwan/libreswan or
>> racoon.
>>
>> Instead I would recommend users to use one of the IPsec keying daemons
>> rather than XFRM directly.
>>
> ok, sounds good, I will update commit msg.
>
>>> VMware-BZ: 1710701
>>> Signed-off-by: Pravin B Shelar 
>>> ---
>>> This is targeted for OVS master branch only.
>>> ---
>>>  NEWS |   1 +
>>>  README.md|   2 +-
>>>
>>>  debian/automake.mk   |   7 -
>>>  debian/control   |  24 --
>>>  debian/openvswitch-ipsec.dirs|   1 -
>>>  debian/openvswitch-ipsec.init| 203 
>>>  debian/openvswitch-ipsec.install |   1 -
>>>  debian/ovs-monitor-ipsec | 507
>>> ---
>>>  lib/netdev-vport.c   |  67 +-
>>>  lib/netdev.h |   1 -
>>>  ofproto/ofproto-dpif-ipfix.c |  15 --
>>>  ofproto/ofproto-dpif-sflow.c |   7 -
>>>  ofproto/tunnel.c |  13 -
>>>  tests/automake.mk|   1 -
>>>  tests/ofproto-macros.at  |  49 
>>>  tests/ovn-controller.at  |   2 +-
>>>  tests/ovs-monitor-ipsec.at   | 271 -
>>>  tests/testsuite.at   |   1 -
>>>  tests/tunnel-push-pop-ipv6.at|   2 +-
>>>  tests/tunnel-push-pop.at |   2 +-
>>>  tests/tunnel.at  |  87 +--
>>>  utilities/bugtool/ovs-bugtool.in |   2 +-
>>>  utilities/ovs-appctl.8.in|   4 +-
>>>  vswitchd/vswitch.xml |  57 +
>>>  24 files changed, 23 insertions(+), 1304 deletions(-)
>>>  delete mode 100644 debian/openvswitch-ipsec.dirs
>>>  delete mode 100755 debian/openvswitch-ipsec.init
>>>  delete mode 100644 debian/openvswitch-ipsec.install
>>>  delete mode 100755 debian/ovs-monitor-ipsec
>>>  delete mode 100644 tests/ovs-monitor-ipsec.at
>>
>>
>> Assuming you were able to build all other debian packages with "fakeroot
>> debian/rules binary" after removing and editing those files, then
>> Acked-by: Ansis Atteka 
>>
> Thanks for review.
>
>> Let me know, if you want me to independently verify that as well?
>
> I will test this but it will be nice if you verify it independently.

I tested it on Debian, It was pretty straight forward to build Debian
packages. I did not see any issue with the patch. so I pushed the
patch to master.

Thanks.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-26 Thread pravin shelar
On Mon, Sep 26, 2016 at 11:49 AM, Ansis Atteka  wrote:
>
>
> On 26 September 2016 at 03:48, Pravin B Shelar  wrote:
>>
>> OVS GRE IPsec tunnel support has multiple issues, Therefore
>
> s/issues,/issues.
>>
>> it was deprecated in OVS 2.6.
>>
>> Following patch removes support GRE IPsec and allow external
>
> s/support/support for
> s/allow/allows
>>
>> IPsec tunnel management for any type of tunnel not just GRE.
>>
>> e.g. user can encrpt Geneve or VxLan traffic.
>
> s/encrpt/encrypt
>>
>>
>> It can be done by using openflow pipeline to set skb-mark
>> and using xfrm to implement IPsec tunnels. xfrm can match
>> on the skb-mark to encrypt selective tunnel traffic.
>
>
> Some folks may misinterpret the paragraph above that we are recommending
> them to use XFRM *directly* as an alternative. XFRM is just NetLink
> interface to linux kernel to install IPsec keys after these keys have been
> negotiated by IPsec keying daemon, such as strongSwan, openSwan/libreswan or
> racoon.
>
> Instead I would recommend users to use one of the IPsec keying daemons
> rather than XFRM directly.
>
ok, sounds good, I will update commit msg.

>> VMware-BZ: 1710701
>> Signed-off-by: Pravin B Shelar 
>> ---
>> This is targeted for OVS master branch only.
>> ---
>>  NEWS |   1 +
>>  README.md|   2 +-
>>
>>  debian/automake.mk   |   7 -
>>  debian/control   |  24 --
>>  debian/openvswitch-ipsec.dirs|   1 -
>>  debian/openvswitch-ipsec.init| 203 
>>  debian/openvswitch-ipsec.install |   1 -
>>  debian/ovs-monitor-ipsec | 507
>> ---
>>  lib/netdev-vport.c   |  67 +-
>>  lib/netdev.h |   1 -
>>  ofproto/ofproto-dpif-ipfix.c |  15 --
>>  ofproto/ofproto-dpif-sflow.c |   7 -
>>  ofproto/tunnel.c |  13 -
>>  tests/automake.mk|   1 -
>>  tests/ofproto-macros.at  |  49 
>>  tests/ovn-controller.at  |   2 +-
>>  tests/ovs-monitor-ipsec.at   | 271 -
>>  tests/testsuite.at   |   1 -
>>  tests/tunnel-push-pop-ipv6.at|   2 +-
>>  tests/tunnel-push-pop.at |   2 +-
>>  tests/tunnel.at  |  87 +--
>>  utilities/bugtool/ovs-bugtool.in |   2 +-
>>  utilities/ovs-appctl.8.in|   4 +-
>>  vswitchd/vswitch.xml |  57 +
>>  24 files changed, 23 insertions(+), 1304 deletions(-)
>>  delete mode 100644 debian/openvswitch-ipsec.dirs
>>  delete mode 100755 debian/openvswitch-ipsec.init
>>  delete mode 100644 debian/openvswitch-ipsec.install
>>  delete mode 100755 debian/ovs-monitor-ipsec
>>  delete mode 100644 tests/ovs-monitor-ipsec.at
>
>
> Assuming you were able to build all other debian packages with "fakeroot
> debian/rules binary" after removing and editing those files, then
> Acked-by: Ansis Atteka 
>
Thanks for review.

> Let me know, if you want me to independently verify that as well?

I will test this but it will be nice if you verify it independently.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-26 Thread Ansis Atteka
On 26 September 2016 at 03:48, Pravin B Shelar  wrote:

> OVS GRE IPsec tunnel support has multiple issues, Therefore
>
s/issues,/issues.

> it was deprecated in OVS 2.6.
>
> Following patch removes support GRE IPsec and allow external
>
s/support/support for
s/allow/allows

> IPsec tunnel management for any type of tunnel not just GRE.

e.g. user can encrpt Geneve or VxLan traffic.
>
s/encrpt/encrypt

>
> It can be done by using openflow pipeline to set skb-mark
> and using xfrm to implement IPsec tunnels. xfrm can match
> on the skb-mark to encrypt selective tunnel traffic.
>

Some folks may misinterpret the paragraph above that we are recommending
them to use XFRM *directly* as an alternative. XFRM is just NetLink
interface to linux kernel to install IPsec keys after these keys have been
negotiated by IPsec keying daemon, such as strongSwan, openSwan/libreswan
or racoon.

Instead I would recommend users to use one of the IPsec keying daemons
rather than XFRM directly.

VMware-BZ: 1710701
> Signed-off-by: Pravin B Shelar 
> ---
> This is targeted for OVS master branch only.
> ---
>  NEWS |   1 +
>  README.md|   2 +-

 debian/automake.mk   |   7 -
>  debian/control   |  24 --
>  debian/openvswitch-ipsec.dirs|   1 -
>  debian/openvswitch-ipsec.init| 203 
>  debian/openvswitch-ipsec.install |   1 -
>  debian/ovs-monitor-ipsec | 507 --
> -
>  lib/netdev-vport.c   |  67 +-
>  lib/netdev.h |   1 -
>  ofproto/ofproto-dpif-ipfix.c |  15 --
>  ofproto/ofproto-dpif-sflow.c |   7 -
>  ofproto/tunnel.c |  13 -
>  tests/automake.mk|   1 -
>  tests/ofproto-macros.at  |  49 
>  tests/ovn-controller.at  |   2 +-
>  tests/ovs-monitor-ipsec.at   | 271 -
>  tests/testsuite.at   |   1 -
>  tests/tunnel-push-pop-ipv6.at|   2 +-
>  tests/tunnel-push-pop.at |   2 +-
>  tests/tunnel.at  |  87 +--
>  utilities/bugtool/ovs-bugtool.in |   2 +-
>  utilities/ovs-appctl.8.in|   4 +-
>  vswitchd/vswitch.xml |  57 +
>  24 files changed, 23 insertions(+), 1304 deletions(-)
>  delete mode 100644 debian/openvswitch-ipsec.dirs
>  delete mode 100755 debian/openvswitch-ipsec.init
>  delete mode 100644 debian/openvswitch-ipsec.install
>  delete mode 100755 debian/ovs-monitor-ipsec
>  delete mode 100644 tests/ovs-monitor-ipsec.at


Assuming you were able to build all other debian packages with "fakeroot
debian/rules binary" after removing and editing those files, then
Acked-by: Ansis Atteka 

Let me know, if you want me to independently verify that as well?

>
>
> diff --git a/NEWS b/NEWS
> index 6e284aa..069ab42 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -25,6 +25,7 @@ Post-v2.6.0
>   * TLV mappings for protocols such as Geneve are now segregated on
> a per-OpenFlow bridge basis rather than globally. (The interface
> has not changed.)
> + * Removed support for IPsec tunnels.
>
>  v2.6.0 - xx xxx 
>  -
> diff --git a/README.md b/README.md
> index cf53437..53b0faf 100644
> --- a/README.md
> +++ b/README.md
> @@ -30,7 +30,7 @@ vSwitch supports the following features:
>  * NIC bonding with or without LACP on upstream switch
>  * NetFlow, sFlow(R), and mirroring for increased visibility
>  * QoS (Quality of Service) configuration, plus policing
> -* Geneve, GRE, GRE over IPSEC, VXLAN, and LISP tunneling
> +* Geneve, GRE, VXLAN, STT, and LISP tunneling
>  * 802.1ag connectivity fault management
>  * OpenFlow 1.0 plus numerous extensions
>  * Transactional configuration database with C and Python bindings
> diff --git a/debian/automake.mk b/debian/automake.mk
> index 73b4d00..2da7055 100644
> --- a/debian/automake.mk
> +++ b/debian/automake.mk
> @@ -19,9 +19,6 @@ EXTRA_DIST += \
> debian/openvswitch-datapath-source.dirs \
> debian/openvswitch-datapath-source.install \
> debian/openvswitch-dev.install \
> -   debian/openvswitch-ipsec.dirs \
> -   debian/openvswitch-ipsec.init \
> -   debian/openvswitch-ipsec.install \
> debian/openvswitch-pki.dirs \
> debian/openvswitch-pki.postinst \
> debian/openvswitch-pki.postrm \
> @@ -71,7 +68,6 @@ EXTRA_DIST += \
> debian/ovn-host.postinst \
> debian/ovn-host.postrm \
> debian/ovn-host.template \
> -   debian/ovs-monitor-ipsec \
> debian/python-openvswitch.dirs \
> debian/python-openvswitch.install \
> debian/rules \
> @@ -79,9 +75,6 @@ EXTRA_DIST += \
> debian/ifupdown.sh \
> debian/source/format
>
> -FLAKE8_PYFILES += \
> -   debian/ovs-monitor-ipsec
> -
>  check-debian-changelog-version:
> @DEB_VERSION=`echo '$(VERSION)' | sed 's/pre/~pre/'`;
>   

[ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-25 Thread Pravin B Shelar
OVS GRE IPsec tunnel support has multiple issues, Therefore
it was deprecated in OVS 2.6.

Following patch removes support GRE IPsec and allow external
IPsec tunnel management for any type of tunnel not just GRE.
e.g. user can encrpt Geneve or VxLan traffic.

It can be done by using openflow pipeline to set skb-mark
and using xfrm to implement IPsec tunnels. xfrm can match
on the skb-mark to encrypt selective tunnel traffic.

VMware-BZ: 1710701
Signed-off-by: Pravin B Shelar 
---
This is targeted for OVS master branch only.
---
 NEWS |   1 +
 README.md|   2 +-
 debian/automake.mk   |   7 -
 debian/control   |  24 --
 debian/openvswitch-ipsec.dirs|   1 -
 debian/openvswitch-ipsec.init| 203 
 debian/openvswitch-ipsec.install |   1 -
 debian/ovs-monitor-ipsec | 507 ---
 lib/netdev-vport.c   |  67 +-
 lib/netdev.h |   1 -
 ofproto/ofproto-dpif-ipfix.c |  15 --
 ofproto/ofproto-dpif-sflow.c |   7 -
 ofproto/tunnel.c |  13 -
 tests/automake.mk|   1 -
 tests/ofproto-macros.at  |  49 
 tests/ovn-controller.at  |   2 +-
 tests/ovs-monitor-ipsec.at   | 271 -
 tests/testsuite.at   |   1 -
 tests/tunnel-push-pop-ipv6.at|   2 +-
 tests/tunnel-push-pop.at |   2 +-
 tests/tunnel.at  |  87 +--
 utilities/bugtool/ovs-bugtool.in |   2 +-
 utilities/ovs-appctl.8.in|   4 +-
 vswitchd/vswitch.xml |  57 +
 24 files changed, 23 insertions(+), 1304 deletions(-)
 delete mode 100644 debian/openvswitch-ipsec.dirs
 delete mode 100755 debian/openvswitch-ipsec.init
 delete mode 100644 debian/openvswitch-ipsec.install
 delete mode 100755 debian/ovs-monitor-ipsec
 delete mode 100644 tests/ovs-monitor-ipsec.at

diff --git a/NEWS b/NEWS
index 6e284aa..069ab42 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,7 @@ Post-v2.6.0
  * TLV mappings for protocols such as Geneve are now segregated on
a per-OpenFlow bridge basis rather than globally. (The interface
has not changed.)
+ * Removed support for IPsec tunnels.
 
 v2.6.0 - xx xxx 
 -
diff --git a/README.md b/README.md
index cf53437..53b0faf 100644
--- a/README.md
+++ b/README.md
@@ -30,7 +30,7 @@ vSwitch supports the following features:
 * NIC bonding with or without LACP on upstream switch
 * NetFlow, sFlow(R), and mirroring for increased visibility
 * QoS (Quality of Service) configuration, plus policing
-* Geneve, GRE, GRE over IPSEC, VXLAN, and LISP tunneling
+* Geneve, GRE, VXLAN, STT, and LISP tunneling
 * 802.1ag connectivity fault management
 * OpenFlow 1.0 plus numerous extensions
 * Transactional configuration database with C and Python bindings
diff --git a/debian/automake.mk b/debian/automake.mk
index 73b4d00..2da7055 100644
--- a/debian/automake.mk
+++ b/debian/automake.mk
@@ -19,9 +19,6 @@ EXTRA_DIST += \
debian/openvswitch-datapath-source.dirs \
debian/openvswitch-datapath-source.install \
debian/openvswitch-dev.install \
-   debian/openvswitch-ipsec.dirs \
-   debian/openvswitch-ipsec.init \
-   debian/openvswitch-ipsec.install \
debian/openvswitch-pki.dirs \
debian/openvswitch-pki.postinst \
debian/openvswitch-pki.postrm \
@@ -71,7 +68,6 @@ EXTRA_DIST += \
debian/ovn-host.postinst \
debian/ovn-host.postrm \
debian/ovn-host.template \
-   debian/ovs-monitor-ipsec \
debian/python-openvswitch.dirs \
debian/python-openvswitch.install \
debian/rules \
@@ -79,9 +75,6 @@ EXTRA_DIST += \
debian/ifupdown.sh \
debian/source/format
 
-FLAKE8_PYFILES += \
-   debian/ovs-monitor-ipsec
-
 check-debian-changelog-version:
@DEB_VERSION=`echo '$(VERSION)' | sed 's/pre/~pre/'`;\
if $(FGREP) '($(DEB_VERSION)' $(srcdir)/debian/changelog >/dev/null; \
diff --git a/debian/control b/debian/control
index da86fe9..813721a 100644
--- a/debian/control
+++ b/debian/control
@@ -178,30 +178,6 @@ Description: OVN Docker drivers
  .
  ovn-docker provides the docker drivers for OVN.
 
-Package: openvswitch-ipsec
-Architecture: linux-any
-Depends: ipsec-tools (>=0.8~alpha20101208),
- iproute2,
- openvswitch-common (= ${binary:Version}),
- openvswitch-switch (= ${binary:Version}),
- python,
- python-openvswitch (= ${source:Version}),
- racoon (>=0.8~alpha20101208),
- ${misc:Depends},
- ${shlibs:Depends}
-Description: Open vSwitch GRE-over-IPsec support
- Open vSwitch is a production quality, multilayer, software-based,
- Ethernet virtual switch. It is designed to enable massive network
- automation through programmatic extension, while still supporting
- standard management interfaces and protocols 

Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-23 Thread pravin shelar
On Fri, Sep 23, 2016 at 12:54 PM, Ansis Atteka  wrote:
> On Fri, Sep 23, 2016 at 1:12 AM, pravin shelar  wrote:
>> On Thu, Sep 22, 2016 at 11:59 AM, Ansis Atteka  wrote:
>>>
>>>
>>> On 20 September 2016 at 20:52, Pravin B Shelar  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
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-23 Thread Ansis Atteka
On Fri, Sep 23, 2016 at 1:12 AM, pravin shelar  wrote:
> On Thu, Sep 22, 2016 at 11:59 AM, Ansis Atteka  wrote:
>>
>>
>> On 20 September 2016 at 20:52, Pravin B Shelar  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?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-22 Thread pravin shelar
On Thu, Sep 22, 2016 at 11:59 AM, Ansis Atteka  wrote:
>
>
> On 20 September 2016 at 20:52, Pravin B Shelar  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.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-22 Thread Ansis Atteka
On 20 September 2016 at 20:52, Pravin B Shelar  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 
> ---
>  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 
>   * 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();
>  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 

[ovs-dev] [PATCH] openvswitch: Allow external IPsec tunnel management.

2016-09-20 Thread Pravin B Shelar
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 
---
 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 
  * 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();
 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(>masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark);
-
+if (enable_ipsec_tnl) {
+