date:20170102

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

2017-01-02 Thread Dong Jun

On 2017/1/3 12:59, Numan Siddique wrote:

On Tue, Jan 3, 2017 at 2:06 AM, Mickey Spiegel > wrote:

On Mon, Jan 2, 2017 at 3:46 AM, Numan Siddique
mailto:nusid...@redhat.com>> wrote:

On Mon, Jan 2, 2017 at 2:07 AM, Mickey Spiegel
mailto:mickeys@gmail.com>> wrote:

On Sun, Jan 1, 2017 at 10:31 AM, Numan Siddique
mailto:nusid...@redhat.com>> wrote:

On Sun, Jan 1, 2017 at 6:39 AM, Mickey Spiegel
mailto:mickeys@gmail.com>>
wrote:

On Sat, Dec 31, 2016 at 1:19 AM, Mickey Spiegel
mailto:mickeys@gmail.com>> wrote:

On Fri, Dec 30, 2016 at 11:37 AM, Mickey
Spiegel mailto:mickeys@gmail.com>> wrote:

On Fri, Dec 30, 2016 at 7:46 AM, Numan
Siddique mailto:nusid...@redhat.com>> wrote:

On Fri, Dec 30, 2016 at 5:36 PM, Dong
Jun mailto:do...@dtdream.com>> wrote:

Hi Dong Jun, I am also facing the same
issue on my setup.

These are the findings of my
investigation so far

Looks like this issue is seen after
the commit

https://github.com/openvswitch/ovs/commit/f1a8bd06d58f2c5312622fbaeacbc6ce7576e347

which removes the usage of patch ports
and uses the clone action instead.

I reverted to the commit just before
it and SNAT/DNAT is working as
expected.

In my case, the gateway router is
hosted on node 1 and the I am trying to
reach a VM (192.168.0.5) hosted on
node 2 using the external ip
(10.2.7.105) associated with it. I
could see that the node 1 is sending
the packet to node 2 through the
geneve tunnel, but it is dropped by node 2
flows.

Below is the tcpdump of the packet

**
19:39:44.709907 IP 182.16.0.16.60069 >
182.16.0.15.geneve: Geneve, Flags
[none], vni 0x1: IP
nusiddiq.blr.redhat.com
 >
192.168.0.5 : ICMP
echo
request, id 13240, seq 1, length 64
***

Below is the tcpdump of the packet
with the ovn-controller (without the
above commit) in the working case

**
19:41:56.783570 IP 182.16.0.12.29778 >
182.16.0.15.geneve: Geneve, Flags
[C], vni 0x1, options [8 bytes]: IP
nusiddiq.blr.redhat.com
 >
192.168.0.5 :
ICMP echo request, id 13308, seq 1,
length 64
19:41:56.784270 IP 182.16.0.15.14539 >
182.16.0.12.geneve: Geneve, Flags
[C], vni 0xf, options [8 bytes]: IP
192.168.0.5 > nusiddiq.blr.redhat.com
:
ICMP echo reply, id 13308, seq 1,
length 64
**

The options data has - 00030005

From the packet, I could see that the
packet from node 1 is missing the
geneve option fields which has inport

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

2017-01-02 Thread Mickey Spiegel

On Mon, Jan 2, 2017 at 8:59 PM, Numan Siddique  wrote:

>
>
> On Tue, Jan 3, 2017 at 2:06 AM, Mickey Spiegel 
> wrote:
>
>>
>> On Mon, Jan 2, 2017 at 3:46 AM, Numan Siddique 
>> wrote:
>>
>>>
>>>
>>> On Mon, Jan 2, 2017 at 2:07 AM, Mickey Spiegel 
>>> wrote:
>>>

 On Sun, Jan 1, 2017 at 10:31 AM, Numan Siddique 
 wrote:

>
>
> On Sun, Jan 1, 2017 at 6:39 AM, Mickey Spiegel 
> wrote:
>
>>
>> On Sat, Dec 31, 2016 at 1:19 AM, Mickey Spiegel <
>> mickeys@gmail.com> wrote:
>>
>>>
>>> On Fri, Dec 30, 2016 at 11:37 AM, Mickey Spiegel <
>>> mickeys@gmail.com> wrote:
>>>

 On Fri, Dec 30, 2016 at 7:46 AM, Numan Siddique <
 nusid...@redhat.com> wrote:

> On Fri, Dec 30, 2016 at 5:36 PM, Dong Jun 
> wrote:
>

 


> 
> Hi Dong Jun, I am also facing the same issue on my setup.
> 
> These are the findings of my investigation so far
>
> Looks like this issue is seen after the commit
> https://github.com/openvswitch/ovs/commit/f1a8bd06d58f2c5312
> 622fbaeacbc6ce7576e347
> 
> which removes the usage of patch ports and uses the clone action
> instead.
> 
>
> I reverted to the commit just before it and SNAT/DNAT is working as
> expected.
>
> In my case, the gateway router is hosted on node 1 and the I am
> trying to
> reach a VM (192.168.0.5) hosted on node 2 using the external ip
> (10.2.7.105) associated with it. I could see that the node 1 is
> sending
> the packet to node 2 through the geneve tunnel, but it is dropped
> by node 2
> flows.
>
> Below is the tcpdump of the packet
>
> **
> 19:39:44.709907 IP 182.16.0.16.60069 > 182.16.0.15.geneve: Geneve,
> Flags
> [none], vni 0x1: IP nusiddiq.blr.redhat.com > 192.168.0.5: ICMP
> echo
> request, id 13240, seq 1, length 64
> ***
>
> Below is the tcpdump of the packet with the ovn-controller
> (without the
> above commit) in the working case
>
> **
> 19:41:56.783570 IP 182.16.0.12.29778 > 182.16.0.15.geneve: Geneve,
> Flags
> [C], vni 0x1, options [8 bytes]: IP nusiddiq.blr.redhat.com >
> 192.168.0.5:
> ICMP echo request, id 13308, seq 1, length 64
> 19:41:56.784270 IP 182.16.0.15.14539 > 182.16.0.12.geneve: Geneve,
> Flags
> [C], vni 0xf, options [8 bytes]: IP 192.168.0.5 >
> nusiddiq.blr.redhat.com:
> ICMP echo reply, id 13308, seq 1, length 64
> **
>
> The options data has - 00030005
>
> From the packet, I could see that the packet from node 1 is
> missing the
> geneve option fields which has inport and outport keys.
>

 I am facing the same issue running my distributed NAT patch set.
 Between UNSNAT recirc and output to tunnel, a megaflow is installed
 that
 is missing the geneve option fields.

 I verified that the table=32 openflow rule has the geneve option
 fields.
 ofproto/trace shows geneve in the "Datapath actions" at the end, so
 no
 problem with whatever ofproto/trace is using.

>>>
>>> Throwing some logs in, I see that flow->metadata.present.map is 0
>>> rather
>>> than 1 coming into tun_metadata_to_geneve_nlattr() in
>>> lib/tun-metadata.c,
>>> when the problem occurs. That is why the geneve option fields are
>>> missing.
>>>
>>> I have not yet figured out why flow->metadata.present.map is 0. It
>>> should
>>> be modified when tun_metadata_write() is called due to actions
>>> setting
>>> tunnel metadata values. I have not checked that yet.
>>>
>>
>> I just posted a fix. I did not try it with the gateway router or with
>> OpenStack,
>> but with this bug fix all distributed NAT manual test cases are now
>> passing.
>>
>>
> Thanks for the fix. I just tested it. Its working when I am trying to
> reach the VM using its floating ip. But not when trying to ping
> www.google.com from the VM (SNAT use case)
>

 With distributed NAT, most of my debugging and tests were using SNAT.
 The bug fix that I posted fixed the problem that was causing ICMP echo
 replies to be dropped. The openflow path for distributed SNAT is similar to
 that for SNAT on gateway routers, but there are still some differences,
 notably one router instead of two routers and no "join" switch. Also I did
 not try it with DNS.

 Are you able to debug further,

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: After thawing, retrieve tunnel table from thawed xbridge

2017-01-02 Thread Numan Siddique

On Sun, Jan 1, 2017 at 6:35 AM, Mickey Spiegel 
wrote:

> In xlate_actions in ofproto-dpif-xlate.c, after thawing from frozen state,
> it currently retrieves the tunnel metadata table from the original xbridge.
> It should retrieve the tunnel metadata table from the thawed xbridge.
>
> In OVN, this manifested as missing geneve option fields when receiving a
> packet from localnet to br-int, then freezing (e.g. for NAT on a gateway
> router or for distributed NAT), then attempting to send out a tunnel.
>
> Signed-off-by: Mickey Spiegel 
>

Tested-by: Numan Siddique 

Numan



> ---
>  ofproto/ofproto-dpif-xlate.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 2977be5..44fe3d1 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -5543,7 +5543,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
>
>  /* Tunnel metadata in udpif format must be normalized before
> translation. */
>  if (flow->tunnel.flags & FLOW_TNL_F_UDPIF) {
> -const struct tun_table *tun_tab = ofproto_dpif_get_tun_tab(xin->
> ofproto);
> +const struct tun_table *tun_tab =
> +ofproto_dpif_get_tun_tab(ctx.
> xbridge->ofproto);
>  int err;
>
>  err = tun_metadata_from_geneve_udpif(tun_tab,
> &xin->upcall_flow->tunnel,
> @@ -5558,7 +5559,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
>  /* If the original flow did not come in on a tunnel, then it
> won't have
>   * FLOW_TNL_F_UDPIF set. However, we still need to have a metadata
>   * table in case we generate tunnel actions. */
> -flow->tunnel.metadata.tab = ofproto_dpif_get_tun_tab(xin->
> ofproto);
> +flow->tunnel.metadata.tab =
> +ofproto_dpif_get_tun_tab(ctx.
> xbridge->ofproto);
>  }
>  ctx.wc->masks.tunnel.metadata.tab = flow->tunnel.metadata.tab;
>
> --
> 1.9.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 11/17] userspace: add non-tap (l3) support to GRE vports

2017-01-02 Thread Yang, Yi Y

Jan, which patch is the final solution you mentioned? Has It been merged into 
net-next? Or it isn't ready at all?  From my understanding, the final solution 
you mentioned will also wait for long time to merge, it is just to add 
packet_type match field, this won't have any big impact on current patchset 
from user perspective.

-Original Message-
From: Jan Scheurich [mailto:jan.scheur...@web.de] 
Sent: Friday, December 30, 2016 6:59 PM
To: Yang, Yi Y ; d...@openvswitch.org
Cc: Simon Horman ; Jiri Benc 
Subject: Re: [ovs-dev] [PATCH v2 11/17] userspace: add non-tap (l3) support to 
GRE vports

This patch is not in line with the ongoing work to support L3 tunnels on legacy 
(non packet type-aware) OVS bridges as specified in 
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8/edit?usp=sharing

To avoid extensive rework, we suggest to replace the patch with the final 
solution based on explicit packet_type fields in dp_packet and struct flow.

Regards, Jan

On 2016-12-28 13:26, Yi Yang wrote:
> Add support for layer 3 GRE vports (non-tap aka non-VTEP).
>
> This makes use of a vport mode configuration for the existing 
> (tap/VTEP) GRE vports.
>
> In order to differentiate packets for two different types of GRE 
> vports a new flow key attribute, OVS_KEY_ATTR_NEXT_BASE_LAYER, is 
> used.  It is intended that this attribute is only used in userspace as 
> there appears to be no need for it to be used in the kernel datapath.
>
> It is envisaged that this attribute may be used for other 
> encapsulation protocols that support both layer3 and layer2 inner-packets.
>
> Signed-off-by: Simon Horman 
> Signed-off-by: Jiri Benc 
> Signed-off-by: Yi Yang 
> ---
>   datapath/linux/compat/include/linux/openvswitch.h |  3 ++
>   include/openvswitch/flow.h| 12 --
>   lib/flow.c| 34 
>   lib/match.c   |  6 ++-
>   lib/netdev-linux.c|  3 +-
>   lib/netdev-native-tnl.c   | 26 +---
>   lib/netdev-vport.c| 22 --
>   lib/netdev.h  |  1 +
>   lib/nx-match.c|  2 +-
>   lib/odp-execute.c |  2 +
>   lib/odp-util.c| 22 ++
>   lib/odp-util.h|  4 +-
>   lib/ofp-util.c|  2 +-
>   lib/tnl-ports.c   | 49 
> +--
>   lib/tnl-ports.h   |  3 +-
>   ofproto/ofproto-dpif-rid.h|  2 +-
>   ofproto/ofproto-dpif-sflow.c  |  1 +
>   ofproto/ofproto-dpif-xlate.c  |  2 +-
>   ofproto/ofproto-dpif.c|  2 +
>   ofproto/tunnel.c  |  4 +-
>   tests/tunnel-push-pop-ipv6.at | 12 --
>   tests/tunnel-push-pop.at  | 26 ++--
>   vswitchd/vswitch.xml  | 13 ++
>   23 files changed, 202 insertions(+), 51 deletions(-)
>
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index af4ee5c..e477d35 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -360,6 +360,9 @@ enum ovs_key_attr {
>   #ifdef __KERNEL__
>   /* Only used within kernel data path. */
>   OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
> +#else
> + /* Only used within user-space data path. */
> + OVS_KEY_ATTR_NEXT_BASE_LAYER, /* base layer of encapsulated packet 
> +*/
>   #endif
>   __OVS_KEY_ATTR_MAX
>   };
> diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h 
> index 93ed37e..46ef87e 100644
> --- a/include/openvswitch/flow.h
> +++ b/include/openvswitch/flow.h
> @@ -23,7 +23,7 @@
>   /* This sequence number should be incremented whenever anything involving 
> flows
>* or the wildcarding of flows changes.  This will cause build assertion
>* failures in places which likely need to be updated. */ -#define 
> FLOW_WC_SEQ 37
> +#define FLOW_WC_SEQ 38
>   
>   /* Number of Open vSwitch extension 32-bit registers. */
>   #define FLOW_N_REGS 16
> @@ -138,6 +138,10 @@ struct flow {
>   ovs_be16 tp_dst;/* TCP/UDP/SCTP destination port/ICMP code. 
> */
>   ovs_be32 igmp_group_ip4;/* IGMP group IPv4 address.
>* Keep last for BUILD_ASSERT_DECL 
> below. */
> +
> +uint8_t next_base_layer;/* Fields of encapsulated packet, if any,
> + * start at this layer */
> +uint8_t pad4[7];
>   };
>   BUILD_ASSERT_DECL(sizeof(struct flow) % sizeof(uint64_t) == 0);
>

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

2017-01-02 Thread Numan Siddique

On Tue, Jan 3, 2017 at 2:06 AM, Mickey Spiegel 
wrote:

>
> On Mon, Jan 2, 2017 at 3:46 AM, Numan Siddique 
> wrote:
>
>>
>>
>> On Mon, Jan 2, 2017 at 2:07 AM, Mickey Spiegel 
>> wrote:
>>
>>>
>>> On Sun, Jan 1, 2017 at 10:31 AM, Numan Siddique 
>>> wrote:
>>>


 On Sun, Jan 1, 2017 at 6:39 AM, Mickey Spiegel 
 wrote:

>
> On Sat, Dec 31, 2016 at 1:19 AM, Mickey Spiegel  > wrote:
>
>>
>> On Fri, Dec 30, 2016 at 11:37 AM, Mickey Spiegel <
>> mickeys@gmail.com> wrote:
>>
>>>
>>> On Fri, Dec 30, 2016 at 7:46 AM, Numan Siddique >> > wrote:
>>>
 On Fri, Dec 30, 2016 at 5:36 PM, Dong Jun 
 wrote:

>>>
>>> 
>>>
>>>
 
 Hi Dong Jun, I am also facing the same issue on my setup.
 
 These are the findings of my investigation so far

 Looks like this issue is seen after the commit
 https://github.com/openvswitch/ovs/commit/f1a8bd06d58f2c5312
 622fbaeacbc6ce7576e347
 
 which removes the usage of patch ports and uses the clone action
 instead.
 

 I reverted to the commit just before it and SNAT/DNAT is working as
 expected.

 In my case, the gateway router is hosted on node 1 and the I am
 trying to
 reach a VM (192.168.0.5) hosted on node 2 using the external ip
 (10.2.7.105) associated with it. I could see that the node 1 is
 sending
 the packet to node 2 through the geneve tunnel, but it is dropped
 by node 2
 flows.

 Below is the tcpdump of the packet

 **
 19:39:44.709907 IP 182.16.0.16.60069 > 182.16.0.15.geneve: Geneve,
 Flags
 [none], vni 0x1: IP nusiddiq.blr.redhat.com > 192.168.0.5: ICMP
 echo
 request, id 13240, seq 1, length 64
 ***

 Below is the tcpdump of the packet with the ovn-controller (without
 the
 above commit) in the working case

 **
 19:41:56.783570 IP 182.16.0.12.29778 > 182.16.0.15.geneve: Geneve,
 Flags
 [C], vni 0x1, options [8 bytes]: IP nusiddiq.blr.redhat.com >
 192.168.0.5:
 ICMP echo request, id 13308, seq 1, length 64
 19:41:56.784270 IP 182.16.0.15.14539 > 182.16.0.12.geneve: Geneve,
 Flags
 [C], vni 0xf, options [8 bytes]: IP 192.168.0.5 >
 nusiddiq.blr.redhat.com:
 ICMP echo reply, id 13308, seq 1, length 64
 **

 The options data has - 00030005

 From the packet, I could see that the packet from node 1 is missing
 the
 geneve option fields which has inport and outport keys.

>>>
>>> I am facing the same issue running my distributed NAT patch set.
>>> Between UNSNAT recirc and output to tunnel, a megaflow is installed
>>> that
>>> is missing the geneve option fields.
>>>
>>> I verified that the table=32 openflow rule has the geneve option
>>> fields.
>>> ofproto/trace shows geneve in the "Datapath actions" at the end, so
>>> no
>>> problem with whatever ofproto/trace is using.
>>>
>>
>> Throwing some logs in, I see that flow->metadata.present.map is 0
>> rather
>> than 1 coming into tun_metadata_to_geneve_nlattr() in
>> lib/tun-metadata.c,
>> when the problem occurs. That is why the geneve option fields are
>> missing.
>>
>> I have not yet figured out why flow->metadata.present.map is 0. It
>> should
>> be modified when tun_metadata_write() is called due to actions setting
>> tunnel metadata values. I have not checked that yet.
>>
>
> I just posted a fix. I did not try it with the gateway router or with
> OpenStack,
> but with this bug fix all distributed NAT manual test cases are now
> passing.
>
>
 Thanks for the fix. I just tested it. Its working when I am trying to
 reach the VM using its floating ip. But not when trying to ping
 www.google.com from the VM (SNAT use case)

>>>
>>> With distributed NAT, most of my debugging and tests were using SNAT.
>>> The bug fix that I posted fixed the problem that was causing ICMP echo
>>> replies to be dropped. The openflow path for distributed SNAT is similar to
>>> that for SNAT on gateway routers, but there are still some differences,
>>> notably one router instead of two routers and no "join" switch. Also I did
>>> not try it with DNS.
>>>
>>> Are you able to debug further, to see whether a missing geneve options
>>> field is still the culprit?
>>> It is possible that removal of patch ports within br-int uncovered other
>>> issues.
>>>
>>
>>
>> With some testing I could see that in the node where the gateway is
>> hosted
>>  -

Re: [ovs-dev] Sync on PTAP, EXT-382 and NSH

2017-01-02 Thread Yang, Yi Y

Jan, we can't always be waiting endlessly, L3 patchset has been in Linux kernel 
no matter you like it or not, we're just to make sure ovs can work with it, 
VxLAN-gpe is not only for NSH.

If you think L3 patchset in Linux kernel has any issue, please send out your 
fix patches as soon as possible, it can be ported to ovs, this isn't an excuse 
we wait endlessly. I don't think we need to wait it.

About several NSH-related issues you mentioned, I don't think they are big 
issues,


Jan: "The NXM fields for NSH are used both as packet match fields and as packet 
metadata fields (after decap). This ambiguity leads to problems, latest when 
dealing with NSH in NSH packets."

Yi: VxLAN tunnel metadata used the same way, isn't it? What problems do you 
mean? I believe we can fix them even if they are really so-called "problems".
Jan: "They introduce push/pop_nsh OpenFlow actions without dealing with the 
resulting non-Ethernet packets in the pipeline. The behavior is not at all well 
defined."
Yi: L3 patchset is just for this, isn't it? Your new proposal will also depend 
on L3 patchset, right?
Jan: "The re-use of the Geneve tunnel metada fields for NSH MD2 TLVs is 
problematic because
a) it again mixes packet metadata and header fields and
b) it couldn't handle NSH MD2 in Geneve tunnels."
Yi: You have to admit this is the existing best solution for MD type 2, it is 
not perfect, but it is ready for use. I don't think people will use GENEVE for 
NSH now, we can modify it to adapt to such use case if people really would like 
to do that way.

Jan, I don't think the new proposal fixed the above issues you mentioned, on 
the contrary, it will make things more complicated. Why don't we go fats path 
instead take a from-a-scratch way?



From: Jan Scheurich [mailto:jan.scheur...@web.de]
Sent: Friday, December 30, 2016 6:34 PM
To: Yang, Yi Y ; Jan Scheurich 
; Zoltán Balogh ; Jiri 
Benc (jb...@redhat.com) ; Pravin Shelar ; 
Simon Horman (simon.hor...@netronome.com) ; 
'jpet...@ovn.org' ; 'ja...@ovn.org' ; 'Ben 
Pfaff' ; 'ben.mackcr...@corsa.com' 
; d...@openvswitch.org; Zhou, Danny 

Subject: Re: [ovs-dev] Sync on PTAP, EXT-382 and NSH


Hi Yi,

Thanks for the confirmation and for rebasing the existing L3 tunneling patches 
to include VXLAN-GPE.

Unfortunately, Simon's original user-space implementation in patches 9/17 
through 11/17 using base_layer and offset fields in dp_packet is not compatible 
to our ongoing implementation of versatile tunnel ports in PTAP and non-PTAP 
bridges, which is based on an explicit packet_type field.

To avoid extensive rework, I would rather not merge these changes into master 
now but substitute them with the final implementation. This is work-package "3 
- L3 ports in non-PTAP pipeline" in our Google doc and Zoltan and I will have 
that ready soon.

Regarding the implementation of NSH support, we should work together to 
implement what is described in the Google doc: 
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8/edit#heading=h.wp4o2op1lp9z

In our opinion the earlier NSH patches cannot be upstreamed because of a couple 
of fundamental conceptual problems:

  1.  The NXM fields for NSH are used both as packet match fields and as packet 
metadata fields (after decap).
This ambiguity leads to problems, latest when dealing with NSH in NSH packets.
  2.  They introduce push/pop_nsh OpenFlow actions without dealing with the 
resulting non-Ethernet packets in the pipeline. The behavior is not at all well 
defined.
  3.  The re-use of the Geneve tunnel metada fields for NSH MD2 TLVs is 
problematic because
a) it again mixes packet metadata and header fields and
b) it couldn't handle NSH MD2 in Geneve tunnels.
All these issues are addressed in the proposed new solution built on PTAP and 
EXT-382. The fundamentals are aligned with ONF (OXM classes already assigned), 
so that there is a good chance that we can feed the OVS implementation of NSH 
into the next OpenFlow standard. The first phase covering the fixed MD1 NSH 
header should also be possible to upstream in Q1/17, quite soon after the basic 
patches for PTAP and EXT-382.

Let's have a direct talk when I'm back in office after New Year.

Regards, Jan

On 2016-12-23 01:51, Yang, Yi Y wrote:

Hi, Jan



I confirm I can take VxLAN-gpe and NSH  related work, now I'm pushing Jiri's L3 
patches ot ovs in order that it can be ported into ovs as early as possible, 
Pravin, Joe and Jarno found some vlan-related issues in Jiri's L3 patches in 
net-next and worked out several patches for net-next, but they are not merged 
yet.



But I have had  a workable local ovs version with Jiri's l3 patches and Jarno's 
fix patches merged, I have worked out several patches to make sure VxLAN-gpe 
can work in layer3 and layer 2 mode, now they are ready except DPDK userspace 
has some issues which I'm debugging.



So I think L3 patches and VxLAN-gpe will be ready very soon.



I remember every guys agreed our old NSH impl

Re: [ovs-dev] [PATCH v2 14/17] datapath: Fix skb->protocol for vlan frames

2017-01-02 Thread Yang, Yi Y

Pravin, the issue is current ovs has too many differences from net-next tree, 
the best way is to apply all the patches before your patch, but it seems a 
super huge work, it is out of my capability :-) Anybody of you is working on 
this?

-Original Message-
From: Pravin Shelar [mailto:pshe...@ovn.org] 
Sent: Sunday, January 1, 2017 3:45 PM
To: Yang, Yi Y 
Cc: ovs dev ; Jarno Rajahalme 
Subject: Re: [PATCH v2 14/17] datapath: Fix skb->protocol for vlan frames

On Wed, Dec 28, 2016 at 5:56 PM, Yi Yang  wrote:
> Do not always set skb->protocol to be the ethertype of the L3 header.
> For a packet with non-accelerated VLAN tags skb->protocol needs to be the 
> ethertype of the outermost non-accelerated VLAN ethertype.
>
> Any VLAN offloading is undone on the OVS netlink interface, and any VLAN tags 
> added by userspace are non-accelerated, as are double tagged VLAN packets.
>
> Fixes: 018c1dda5f ("openvswitch: 802.1AD Flow handling, actions, vlan 
> parsing, netlink attributes")
> Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets")
> Signed-off-by: Jarno Rajahalme 
> Signed-off-by: Yi Yang 
This is not upstream patch. So even though it fixes the issue we can not apply 
it to out of tree kernel module.

Can you look at the net branch for the correct patch.

Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v8] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

2017-01-02 Thread Chandran, Sugesh



Regards
_Sugesh

> -Original Message-
> From: Pravin Shelar [mailto:pshe...@ovn.org]
> Sent: Sunday, January 1, 2017 4:53 PM
> To: Chandran, Sugesh 
> Cc: ovs dev ; Jesse Gross ; Ben
> Pfaff 
> Subject: Re: [PATCH v8] netdev-dpdk: Enable Rx checksum offloading
> feature on DPDK physical ports.
> 
> On Fri, Dec 30, 2016 at 1:58 AM, Chandran, Sugesh
>  wrote:
> >
> >
> > Regards
> > _Sugesh
> >
> >
> >> -Original Message-
> >> From: Pravin Shelar [mailto:pshe...@ovn.org]
> >> Sent: Saturday, December 24, 2016 1:13 PM
> >> To: Chandran, Sugesh 
> >> Cc: ovs dev ; Jesse Gross ;
> >> Ben Pfaff 
> >> Subject: Re: [PATCH v8] netdev-dpdk: Enable Rx checksum offloading
> >> feature on DPDK physical ports.
> >>
> >> On Fri, Dec 23, 2016 at 8:16 PM, Sugesh Chandran
> >>  wrote:
> >> > Add Rx checksum offloading feature support on DPDK physical ports.
> >> > By default, the Rx checksum offloading is enabled if NIC supports.
> >> > However, the checksum offloading can be turned OFF either while
> >> > adding a new DPDK physical port to OVS or at runtime.
> >> >
> >> > The rx checksum offloading can be turned off by setting the
> >> > parameter to 'false'. For eg: To disable the rx checksum offloading
> >> > when adding a port,
> >> >
> >> >  'ovs-vsctl add-port br0 dpdk0 -- \
> >> >   set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'
> >> >
> >> > OR (to disable at run time after port is being added to OVS)
> >> >
> >> > 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'
> >> >
> >> > Similarly to turn ON rx checksum offloading at run time,
> >> > 'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'
> >> >
> >> > The Tx checksum offloading support is not implemented due to the
> >> > following reasons.
> >> >
> >> > 1) Checksum offloading and vectorization are mutually exclusive in
> >> > DPDK poll mode driver. Vector packet processing is turned OFF when
> >> > checksum offloading is enabled which causes significant performance
> >> > drop
> >> at Tx side.
> >> >
> >> > 2) Normally, OVS generates checksum for tunnel packets in software
> >> > at the 'tunnel push' operation, where the tunnel headers are created.
> >> > However enabling Tx checksum offloading involves,
> >> >
> >> > *) Mark every packets for tx checksum offloading at 'tunnel_push'
> >> > and recirculate.
> >> > *) At the time of xmit, validate the same flag and instruct the NIC
> >> > to do the checksum calculation.  In case NIC doesnt support Tx
> >> > checksum offloading, the checksum calculation has to be done in
> >> > software before sending out the packets.
> >> >
> >> > No significant performance improvement noticed with Tx checksum
> >> > offloading due to the e overhead of additional validations + non
> >> > vector
> >> packet processing.
> >> > In some test scenarios, it introduces performance drop too.
> >> >
> >> > Rx checksum offloading still offers 8-9% of improvement on VxLAN
> >> > tunneling decapsulation even though the SSE vector Rx function is
> >> > disabled in DPDK poll mode driver.
> >> >
> >> > Signed-off-by: Sugesh Chandran 
> >> > Acked-by: Jesse Gross 
> >> >
> >> > ---
> >> > v8
> >> > - Update error log to clearly mention when the checksum offload is
> >> unsupported.
> >> > - Rebase to OVS latest master.
> >>
> >> As commented in earlier review, can you update get-config to reflect
> >> the status of offload for the device?
> > [Sugesh] The checksum offload is displayed as part of 'ovs-vsctl show'
> when configured.
> > Do you think it still needs another get-config?
> >
> Yes.
> The ovs-vsctl is showing the value as user request. I am looking for current
> device setting via get-config.
[Sugesh] sure, will update the get config as well.
> 
> > ~/repo/ovs-apr16$ sudo ./utilities/ovs-vsctl show
> > d26b6bac-a397-4dc3-a514-c4e86e07ad69
> > Bridge "br0"
> > Port "br0"
> > Interface "br0"
> > type: internal
> > Port "dpdk1"
> > Interface "dpdk1"
> > type: dpdk
> > Port "dpdk0"
> > Interface "dpdk0"
> > type: dpdk
> > options: {rx-checksum-offload="false"}
> > ~/repo/ovs-apr16$
> >>
> >> Thanks,
> >> Pravin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovsdb: fix data loss when OVSDB replication from itself

2017-01-02 Thread Guoshuai Li

Delete the local database after receiving the master data,
this is safer for data.
This patch is used by HA cluster that have no way to
control the order of resources, such as kubernetes.

Signed-off-by: Guoshuai Li 
---
 ovsdb/replication.c | 73 +
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 1c77b18..109ae98 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -252,56 +252,53 @@ replication_run(void)
 }
 ovsdb_schema_destroy(schema);
 
-/* After receiving schemas, reset the local databases that
- * will be monitored and send out monitor requests for them. */
+/* After receiving schemas, send out monitor requests for DB. 
*/
 if (hmap_is_empty(&request_ids)) {
-struct shash_node *node, *next;
-
-SHASH_FOR_EACH_SAFE (node, next, replication_dbs) {
+struct shash_node *node;
+SHASH_FOR_EACH (node, replication_dbs) {
 db = node->data;
-struct ovsdb_error *error = reset_database(db);
-if (error) {
-const char *db_name = db->schema->name;
-shash_find_and_delete(replication_dbs, db_name);
-ovsdb_error_assert(error);
-VLOG_WARN("Failed to reset database, "
-  "%s not replicated.", db_name);
-}
-}
-
-if (shash_is_empty(replication_dbs)) {
-VLOG_WARN("Nothing to replicate.");
-state = RPL_S_ERR;
-} else {
-SHASH_FOR_EACH (node, replication_dbs) {
-db = node->data;
-struct ovsdb *db = node->data;
-struct jsonrpc_msg *request =
-create_monitor_request(db);
-
-request_ids_add(request->id, db);
-jsonrpc_session_send(session, request);
-VLOG_DBG("Send monitor requests");
-state = RPL_S_MONITOR_REQUESTED;
-}
+struct ovsdb *db = node->data;
+struct jsonrpc_msg *request =
+create_monitor_request(db);
+
+request_ids_add(request->id, db);
+jsonrpc_session_send(session, request);
+VLOG_DBG("Send monitor requests");
+state = RPL_S_MONITOR_REQUESTED;
 }
 }
 break;
 }
 
 case RPL_S_MONITOR_REQUESTED: {
-/* Reply to monitor requests. */
-struct ovsdb_error *error;
-error = process_notification(msg->result, db);
+/* After receiving monitor response, reset the local database
+ * that will be monitored and process message. */
+struct ovsdb_error *error = reset_database(db);
 if (error) {
+const char *db_name = db->schema->name;
+shash_find_and_delete(replication_dbs, db_name);
 ovsdb_error_assert(error);
+VLOG_WARN("Failed to reset database, "
+  "%s not replicated.", db_name);
+}
+
+if (shash_is_empty(replication_dbs)) {
+VLOG_WARN("Nothing to replicate.");
 state = RPL_S_ERR;
 } else {
-/* Transition to replicating state after receiving
- * all replies of "monitor" requests. */
-if (hmap_is_empty(&request_ids)) {
-VLOG_DBG("Listening to monitor updates");
-state = RPL_S_REPLICATING;
+/* Reply to monitor requests. */
+struct ovsdb_error *error;
+error = process_notification(msg->result, db);
+if (error) {
+ovsdb_error_assert(error);
+state = RPL_S_ERR;
+} else {
+/* Transition to replicating state after receiving
+ * all replies of "monitor" requests. */
+if (hmap_is_empty(&request_ids)) {
+VLOG_DBG("Listening to monitor updates");
+state = RPL_S_REPLICATING;
+}
 }
 }
 break;
-- 
2.10.1.windows.1

[ovs-dev] [PATCH v9] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

2017-01-02 Thread Sugesh Chandran

Add Rx checksum offloading feature support on DPDK physical ports. By default,
the Rx checksum offloading is enabled if NIC supports. However,
the checksum offloading can be turned OFF either while adding a new DPDK
physical port to OVS or at runtime.

The rx checksum offloading can be turned off by setting the parameter to
'false'. For eg: To disable the rx checksum offloading when adding a port,

 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'

OR (to disable at run time after port is being added to OVS)

'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'

Similarly to turn ON rx checksum offloading at run time,
'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'

The Tx checksum offloading support is not implemented due to the following
reasons.

1) Checksum offloading and vectorization are mutually exclusive in DPDK poll
mode driver. Vector packet processing is turned OFF when checksum offloading
is enabled which causes significant performance drop at Tx side.

2) Normally, OVS generates checksum for tunnel packets in software at the
'tunnel push' operation, where the tunnel headers are created. However
enabling Tx checksum offloading involves,

*) Mark every packets for tx checksum offloading at 'tunnel_push' and
recirculate.
*) At the time of xmit, validate the same flag and instruct the NIC to do the
checksum calculation.  In case NIC doesnt support Tx checksum offloading,
the checksum calculation has to be done in software before sending out the
packets.

No significant performance improvement noticed with Tx checksum offloading
due to the e overhead of additional validations + non vector packet processing.
In some test scenarios, it introduces performance drop too.

Rx checksum offloading still offers 8-9% of improvement on VxLAN tunneling
decapsulation even though the SSE vector Rx function is disabled in DPDK poll
mode driver.

Signed-off-by: Sugesh Chandran 
Acked-by: Jesse Gross 
---
v9
- Updated get-config to report rx-checksum-offload when configured.
- Rebase to the OVS latest master

v8
- Update error log to clearly mention when the checksum offload is unsupported.
- Rebase to OVS latest master.

v7
- Update documentation with details of performance impact of having DPDK
vectorization in disabled state.
- Merge to latest OVS master.

v6
- Avoid unnecessary reconfiguration of DPDK ports when Rx checksum offload
support is unavailable.
- Limit the 'Rx checksum offload is unsupported' message in logging.
- Merge to latest OVS for DPDK 16.11 support. The DPDK checksum flags that used
- in the patch are only introduced in 16.11.

v5
- Reset the checksum flag in common tunnel pop function than in
'udp_extract_tnl_md' function.

v4
- Unconditonally clear off the checksum flag one time in pop operation than
doing separately in IP and UDP layers.

v3
- Reset the checksum offload flags in tunnel pop operation after the validation.
- Reconfigure the dpdk port with rx checksum offload only if new configuration
is different than current one.

v2
- Set Rx checksum enabled by default.
- Modified commit message, explaining the tradeoff with tx checksum offloading.
- Use dpdk mbuf checksum offload flags  instead of defining new metadata field
in OVS dp_packet.
- validate udp checksum mbuf flag only if the checksum present in the packet.
- Doc update with Rx checksum offloading feature.
---
---
 Documentation/howto/dpdk.rst | 25 ++
 lib/dp-packet.h  | 29 +
 lib/netdev-dpdk.c| 50 
 lib/netdev-native-tnl.c  | 35 ++-
 lib/netdev.c |  4 
 vswitchd/vswitch.xml | 14 +
 6 files changed, 142 insertions(+), 15 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index e96ce56..db92a8e 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -273,6 +273,31 @@ largest frame size supported by Fortville NIC using the 
DPDK i40e driver, but
 larger frames and other DPDK NIC drivers may be supported. These cases are
 common for use cases involving East-West traffic only.
 
+Rx Checksum Offload
+---
+
+By default, DPDK physical ports are enabled with Rx checksum offload. Rx
+checksum offload can be configured on a DPDK physical port either when adding
+or at run time.
+
+To disable Rx checksum offload when adding a DPDK port dpdk0::
+
+$ ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
+  options:rx-checksum-offload=false
+
+Similarly to disable the Rx checksum offloading on a existing DPDK port dpdk0::
+
+$ ovs-vsctl set Interface dpdk0 type=dpdk options:rx-checksum-offload=false
+
+Rx checksum offload can offer performance improvement only for tunneling
+traffic in OVS-DPDK because the checksum validation of tunnel packets is
+offloaded to the N

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

2017-01-02 Thread Mickey Spiegel

On Mon, Jan 2, 2017 at 3:46 AM, Numan Siddique  wrote:

>
>
> On Mon, Jan 2, 2017 at 2:07 AM, Mickey Spiegel 
> wrote:
>
>>
>> On Sun, Jan 1, 2017 at 10:31 AM, Numan Siddique 
>> wrote:
>>
>>>
>>>
>>> On Sun, Jan 1, 2017 at 6:39 AM, Mickey Spiegel 
>>> wrote:
>>>

 On Sat, Dec 31, 2016 at 1:19 AM, Mickey Spiegel 
 wrote:

>
> On Fri, Dec 30, 2016 at 11:37 AM, Mickey Spiegel <
> mickeys@gmail.com> wrote:
>
>>
>> On Fri, Dec 30, 2016 at 7:46 AM, Numan Siddique 
>> wrote:
>>
>>> On Fri, Dec 30, 2016 at 5:36 PM, Dong Jun  wrote:
>>>
>>
>> 
>>
>>
>>> 
>>> Hi Dong Jun, I am also facing the same issue on my setup.
>>> 
>>> These are the findings of my investigation so far
>>>
>>> Looks like this issue is seen after the commit
>>> https://github.com/openvswitch/ovs/commit/f1a8bd06d58f2c5312
>>> 622fbaeacbc6ce7576e347
>>> 
>>> which removes the usage of patch ports and uses the clone action
>>> instead.
>>> 
>>>
>>> I reverted to the commit just before it and SNAT/DNAT is working as
>>> expected.
>>>
>>> In my case, the gateway router is hosted on node 1 and the I am
>>> trying to
>>> reach a VM (192.168.0.5) hosted on node 2 using the external ip
>>> (10.2.7.105) associated with it. I could see that the node 1 is
>>> sending
>>> the packet to node 2 through the geneve tunnel, but it is dropped by
>>> node 2
>>> flows.
>>>
>>> Below is the tcpdump of the packet
>>>
>>> **
>>> 19:39:44.709907 IP 182.16.0.16.60069 > 182.16.0.15.geneve: Geneve,
>>> Flags
>>> [none], vni 0x1: IP nusiddiq.blr.redhat.com > 192.168.0.5: ICMP echo
>>> request, id 13240, seq 1, length 64
>>> ***
>>>
>>> Below is the tcpdump of the packet with the ovn-controller (without
>>> the
>>> above commit) in the working case
>>>
>>> **
>>> 19:41:56.783570 IP 182.16.0.12.29778 > 182.16.0.15.geneve: Geneve,
>>> Flags
>>> [C], vni 0x1, options [8 bytes]: IP nusiddiq.blr.redhat.com >
>>> 192.168.0.5:
>>> ICMP echo request, id 13308, seq 1, length 64
>>> 19:41:56.784270 IP 182.16.0.15.14539 > 182.16.0.12.geneve: Geneve,
>>> Flags
>>> [C], vni 0xf, options [8 bytes]: IP 192.168.0.5 >
>>> nusiddiq.blr.redhat.com:
>>> ICMP echo reply, id 13308, seq 1, length 64
>>> **
>>>
>>> The options data has - 00030005
>>>
>>> From the packet, I could see that the packet from node 1 is missing
>>> the
>>> geneve option fields which has inport and outport keys.
>>>
>>
>> I am facing the same issue running my distributed NAT patch set.
>> Between UNSNAT recirc and output to tunnel, a megaflow is installed
>> that
>> is missing the geneve option fields.
>>
>> I verified that the table=32 openflow rule has the geneve option
>> fields.
>> ofproto/trace shows geneve in the "Datapath actions" at the end, so no
>> problem with whatever ofproto/trace is using.
>>
>
> Throwing some logs in, I see that flow->metadata.present.map is 0
> rather
> than 1 coming into tun_metadata_to_geneve_nlattr() in
> lib/tun-metadata.c,
> when the problem occurs. That is why the geneve option fields are
> missing.
>
> I have not yet figured out why flow->metadata.present.map is 0. It
> should
> be modified when tun_metadata_write() is called due to actions setting
> tunnel metadata values. I have not checked that yet.
>

 I just posted a fix. I did not try it with the gateway router or with
 OpenStack,
 but with this bug fix all distributed NAT manual test cases are now
 passing.


>>> Thanks for the fix. I just tested it. Its working when I am trying to
>>> reach the VM using its floating ip. But not when trying to ping
>>> www.google.com from the VM (SNAT use case)
>>>
>>
>> With distributed NAT, most of my debugging and tests were using SNAT. The
>> bug fix that I posted fixed the problem that was causing ICMP echo replies
>> to be dropped. The openflow path for distributed SNAT is similar to that
>> for SNAT on gateway routers, but there are still some differences, notably
>> one router instead of two routers and no "join" switch. Also I did not try
>> it with DNS.
>>
>> Are you able to debug further, to see whether a missing geneve options
>> field is still the culprit?
>> It is possible that removal of patch ports within br-int uncovered other
>> issues.
>>
>
>
> With some testing I could see that in the node where the gateway is
> hosted
>  - The reply packet reaches the gateway router pipeline -> to the otls
> switch pipeline (via clone) -> to the router pipeline -> to the peer port
> of the switch.
> The packet gets dropped at table 22
>
>  table=22, n_packets=275,

[ovs-dev] [PATCH v2 4/5] make: Ensure flake8, sphinx run when required

2017-01-02 Thread Stephen Finucane

If someone makes changes to documentation or Python scripts, they should
validate these changes using the relevant targets. However, said targets
use optional dependencies and are not guaranteed to be enabled. Enforce
running of these checks whenever changes are made to select files,
ensuring the user has installed the changes.

Signed-off-by: Stephen Finucane 
---
 Documentation/automake.mk| 12 
 Makefile.am  |  5 -
 build-aux/check-file-changes | 19 +++
 3 files changed, 31 insertions(+), 5 deletions(-)
 create mode 100755 build-aux/check-file-changes

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 51abd55..aac21d5 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -103,13 +103,17 @@ htmldocs:
$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html
 ALL_LOCAL += htmldocs
 
-check-docs:
-   $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
-
 clean-docs:
rm -rf $(SPHINXBUILDDIR)/*
 CLEAN_LOCAL += clean-docs
 endif
 .PHONY: htmldocs
-.PHONY: check-docs
 .PHONY: clean-docs
+
+check-docs:
+if HAVE_SPHINX
+   $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
+else
+   $(srcdir)/build-aux/check-file-changes "Documentation/" "sphinx"
+endif
+.PHONY: check-docs
diff --git a/Makefile.am b/Makefile.am
index 74839e1..7969fd1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -79,6 +79,7 @@ EXTRA_DIST = \
build-aux/cccl \
build-aux/cksum-schema-check \
build-aux/calculate-schema-cksum \
+   build-aux/check-file-changes \
build-aux/dist-docs \
build-aux/sodepends.pl \
build-aux/soexpand.pl \
@@ -319,7 +320,6 @@ manpage-check: $(man_MANS) $(dist_man_MANS) 
$(noinst_man_MANS)
 CLEANFILES += manpage-check
 endif
 
-if HAVE_FLAKE8
 ALL_LOCAL += flake8-check
 # http://flake8.readthedocs.org/en/latest/warnings.html
 # All warnings explicitly selected or ignored should be listed below.
@@ -343,9 +343,12 @@ ALL_LOCAL += flake8-check
 #   H233 Python 3.x incompatible use of print operator
 #   H238 old style class declaration, use new style (inherit from `object`)
 flake8-check: $(FLAKE8_PYFILES)
+if HAVE_FLAKE8
$(AM_V_GEN) if flake8 $^ --select=H231,H232,H233,H238 ${FLAKE8_FLAGS} 
&& \
flake8 $^ 
--ignore=E121,E123,E125,E126,E127,E128,E129,E131,W503,F811,D,H ${FLAKE8_FLAGS}; 
then \
touch $@; else exit 1; fi
+else
+   $(srcdir)/build-aux/check-file-changes "python/" "flake8"
 endif
 CLEANFILES += flake8-check
 
diff --git a/build-aux/check-file-changes b/build-aux/check-file-changes
new file mode 100755
index 000..5c0b809
--- /dev/null
+++ b/build-aux/check-file-changes
@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+
+directory="$1"
+dependency="$2"
+
+# ensure git is actually installed
+if ! type "git" > /dev/null; then
+printf "git is not available - skipping check-file-changes"
+exit 0
+fi
+
+file_changes=$(git diff --name-only master..HEAD "$directory")
+
+if [ -n "$file_changes" ]; then
+printf "There are changes in the '$directory' directory, but the "
+printf "'$dependency' dependency is missing. Install missing dependencies "
+printf "before continuing.\n"
+exit 1
+fi
-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 1/5] make: Validate documents on build

2017-01-02 Thread Stephen Finucane

Build documentation as part of every build. This ensures any syntax
errors are caught early.

In addition, a 'check-docs' target is added to validates all external
links.

The nitpick ('-n') flag is added to ensure all possible warnings are
raised.

Signed-off-by: Stephen Finucane 
Cc: Ben Pfaff 
---
v2:
- Use 'htmldocs' target to validate docs, rather than 'check-docs'
- Don't silence output of 'check-docs' target
---
 Documentation/automake.mk | 9 +++--
 Documentation/conf.py | 2 ++
 Makefile.am   | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 98adca7..cb41d11 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -96,9 +96,14 @@ SPHINXBUILDDIR = $(builddir)/Documentation/_build
 # Internal variables.
 PAPEROPT_a4 = -D latex_paper_size=a4
 PAPEROPT_letter = -D latex_paper_size=letter
-ALLSPHINXOPTS = -W -d $(SPHINXBUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) 
$(SPHINXOPTS) $(SPHINXSRCDIR)
+ALLSPHINXOPTS = -W -n -d $(SPHINXBUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) 
$(SPHINXOPTS) $(SPHINXSRCDIR)
 
-.PHONY: htmldocs
 htmldocs:
rm -rf $(SPHINXBUILDDIR)/*
$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html
+ALL_LOCAL += htmldocs
+.PHONY: htmldocs
+
+check-docs:
+   $(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
+.PHONY: check-docs
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 63f5877..06f50bc 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -113,6 +113,8 @@ pygments_style = 'sphinx'
 # If true, `todo` and `todoList` produce output, else they produce nothing.
 todo_include_todos = False
 
+# If true, check the validity of #anchors in links.
+linkcheck_anchors = False
 
 # -- Options for HTML output --
 
diff --git a/Makefile.am b/Makefile.am
index 9c429b2..74839e1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -177,10 +177,10 @@ SUFFIXES += .xml
sbindir='$(sbindir)'
$(AM_v_at)mv $@.tmp $@
 
-.PHONY: clean-pycov
 clean-pycov:
cd $(srcdir) && rm -f $(PYCOV_CLEAN_FILES)
 CLEAN_LOCAL += clean-pycov
+.PHONY: clean-pycov
 
 # If we're checked out from a Git repository, make sure that every
 # file that is in Git is distributed.
-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 5/5] make: Standardize indentation

2017-01-02 Thread Stephen Finucane

If we're going to mix tabs and spaces, let's do it consistently.

Signed-off-by: Stephen Finucane 
---
 Makefile.am | 178 ++--
 1 file changed, 89 insertions(+), 89 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 7969fd1..96917c5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -138,44 +138,44 @@ ro_shell = printf '\043 Generated automatically -- do not 
modify!-*- buffer-
 SUFFIXES += .in
 .in:
$(AM_V_GEN)$(PERL) $(srcdir)/build-aux/soexpand.pl -I$(srcdir) < $< | \
-   sed \
-   -e 's,[@]PKIDIR[@],$(PKIDIR),g' \
--e 's,[@]LOGDIR[@],$(LOGDIR),g' \
--e 's,[@]DBDIR[@],$(DBDIR),g' \
--e 's,[@]PERL[@],$(PERL),g' \
--e 's,[@]PYTHON[@],$(PYTHON),g' \
--e 's,[@]RUNDIR[@],$(RUNDIR),g' \
--e 's,[@]VERSION[@],$(VERSION),g' \
--e 's,[@]localstatedir[@],$(localstatedir),g' \
--e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
--e 's,[@]sysconfdir[@],$(sysconfdir),g' \
--e 's,[@]bindir[@],$(bindir),g' \
--e 's,[@]sbindir[@],$(sbindir),g' \
--e 's,[@]abs_builddir[@],$(abs_builddir),g' \
--e 's,[@]abs_top_srcdir[@],$(abs_top_srcdir),g' \
-> $@.tmp
+ sed \
+   -e 's,[@]PKIDIR[@],$(PKIDIR),g' \
+   -e 's,[@]LOGDIR[@],$(LOGDIR),g' \
+   -e 's,[@]DBDIR[@],$(DBDIR),g' \
+   -e 's,[@]PERL[@],$(PERL),g' \
+   -e 's,[@]PYTHON[@],$(PYTHON),g' \
+   -e 's,[@]RUNDIR[@],$(RUNDIR),g' \
+   -e 's,[@]VERSION[@],$(VERSION),g' \
+   -e 's,[@]localstatedir[@],$(localstatedir),g' \
+   -e 's,[@]pkgdatadir[@],$(pkgdatadir),g' \
+   -e 's,[@]sysconfdir[@],$(sysconfdir),g' \
+   -e 's,[@]bindir[@],$(bindir),g' \
+   -e 's,[@]sbindir[@],$(sbindir),g' \
+   -e 's,[@]abs_builddir[@],$(abs_builddir),g' \
+   -e 's,[@]abs_top_srcdir[@],$(abs_top_srcdir),g' \
+ > $@.tmp
@if head -n 1 $@.tmp | grep '#!' > /dev/null; then \
-   chmod +x $@.tmp; \
+ chmod +x $@.tmp; \
fi
$(AM_V_at) mv $@.tmp $@
 
 SUFFIXES += .xml
 %: %.xml
$(AM_V_GEN)$(run_python) $(srcdir)/build-aux/xml2nroff $< > $@.tmp \
-   -I $(srcdir) \
-   --version=$(VERSION) \
-   PKIDIR='$(PKIDIR)' \
-   LOGDIR='$(LOGDIR)' \
-   DBDIR='$(DBDIR)' \
-   PERL='$(PERL)' \
-   PYTHON='$(PYTHON)' \
-   RUNDIR='$(RUNDIR)' \
-   VERSION='$(VERSION)' \
-   localstatedir='$(localstatedir)' \
-   pkgdatadir='$(pkgdatadir)' \
-   sysconfdir='$(sysconfdir)' \
-   bindir='$(bindir)' \
-   sbindir='$(sbindir)'
+ -I $(srcdir) \
+ --version=$(VERSION) \
+ PKIDIR='$(PKIDIR)' \
+ LOGDIR='$(LOGDIR)' \
+ DBDIR='$(DBDIR)' \
+ PERL='$(PERL)' \
+ PYTHON='$(PYTHON)' \
+ RUNDIR='$(RUNDIR)' \
+ VERSION='$(VERSION)' \
+ localstatedir='$(localstatedir)' \
+ pkgdatadir='$(pkgdatadir)' \
+ sysconfdir='$(sysconfdir)' \
+ bindir='$(bindir)' \
+ sbindir='$(sbindir)'
$(AM_v_at)mv $@.tmp $@
 
 clean-pycov:
@@ -193,17 +193,17 @@ if GNU_MAKE
 ALL_LOCAL += dist-hook-git
 dist-hook-git: distfiles
@if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1; then \
- (cd datapath && $(MAKE) distfiles);   \
- (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) |\
-   LC_ALL=C sort -u > all-distfiles;   \
- (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' |\
-   LC_ALL=C sort -u > all-gitfiles;\
+ (cd datapath && $(MAKE) distfiles); \
+ (cat distfiles; sed 's|^|datapath/|' datapath/distfiles) | \
+   LC_ALL=C sort -u > all-distfiles; \
+ (cd $(srcdir) && git ls-files) | grep -v '\.gitignore$$' | \
+   LC_ALL=C sort -u > all-gitfiles; \
  LC_ALL=C comm -1 -3 all-distfiles all-gitfiles > missing-distfiles; \
- if test -s missing-distfiles; then\
+ if test -s missing-distfiles; then \
echo "The following files are in git but not the distribution:"; \
-   cat missing-distfiles;  \
-   exit 1; \
- fi;   \
+   cat missing-distfiles; \
+   exit 1; \
+ fi; \
fi
 CLEANFILES += all-distfiles all-gitfiles missing-distfiles
 # The following is based on commands for the Automake "distdir" target.
@@ -214,7 +214,7

[ovs-dev] [PATCH v2 3/5] make: Check for Sphinx before checking docs

2017-01-02 Thread Stephen Finucane

Signed-off-by: Stephen Finucane 
---
 Documentation/automake.mk |  6 --
 configure.ac  |  1 +
 m4/openvswitch.m4 | 14 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index ac147a7..51abd55 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -98,16 +98,18 @@ PAPEROPT_a4 = -D latex_paper_size=a4
 PAPEROPT_letter = -D latex_paper_size=letter
 ALLSPHINXOPTS = -W -n -d $(SPHINXBUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) 
$(SPHINXOPTS) $(SPHINXSRCDIR)
 
+if HAVE_SPHINX
 htmldocs:
$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html
 ALL_LOCAL += htmldocs
-.PHONY: htmldocs
 
 check-docs:
$(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
-.PHONY: check-docs
 
 clean-docs:
rm -rf $(SPHINXBUILDDIR)/*
 CLEAN_LOCAL += clean-docs
+endif
+.PHONY: htmldocs
+.PHONY: check-docs
 .PHONY: clean-docs
diff --git a/configure.ac b/configure.ac
index 4414230..a9ae2a1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -97,6 +97,7 @@ OVS_CHECK_LOGDIR
 OVS_CHECK_PYTHON
 OVS_CHECK_PYTHON3
 OVS_CHECK_FLAKE8
+OVS_CHECK_SPHINX
 OVS_CHECK_DOT
 OVS_CHECK_IF_PACKET
 OVS_CHECK_IF_DL
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index a448223..6515ed7 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -400,7 +400,7 @@ else:
AM_CONDITIONAL([HAVE_PYTHON3], [test "$HAVE_PYTHON3" = yes])])
 
 
-dnl Checks for dot.
+dnl Checks for flake8.
 AC_DEFUN([OVS_CHECK_FLAKE8],
   [AC_CACHE_CHECK(
 [for flake8],
@@ -412,6 +412,18 @@ AC_DEFUN([OVS_CHECK_FLAKE8],
  fi])
AM_CONDITIONAL([HAVE_FLAKE8], [test "$ovs_cv_flake8" = yes])])
 
+dnl Checks for sphinx.
+AC_DEFUN([OVS_CHECK_SPHINX],
+  [AC_CACHE_CHECK(
+[for sphinx],
+[ovs_cv_sphinx],
+[if sphinx-build --version >/dev/null 2>&1; then
+   ovs_cv_sphinx=yes
+ else
+   ovs_cv_sphinx=no
+ fi])
+   AM_CONDITIONAL([HAVE_SPHINX], [test "$ovs_cv_sphinx" = yes])])
+
 dnl Checks for dot.
 AC_DEFUN([OVS_CHECK_DOT],
   [AC_CACHE_CHECK(
-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 2/5] make: Add distinct clean-docs target

2017-01-02 Thread Stephen Finucane

Speed things up by not rebuilding documents every time.

Signed-off-by: Stephen Finucane 
---
 Documentation/automake.mk | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index cb41d11..ac147a7 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -99,7 +99,6 @@ PAPEROPT_letter = -D latex_paper_size=letter
 ALLSPHINXOPTS = -W -n -d $(SPHINXBUILDDIR)/doctrees $(PAPEROPT_$(PAPER)) 
$(SPHINXOPTS) $(SPHINXSRCDIR)
 
 htmldocs:
-   rm -rf $(SPHINXBUILDDIR)/*
$(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/html
 ALL_LOCAL += htmldocs
 .PHONY: htmldocs
@@ -107,3 +106,8 @@ ALL_LOCAL += htmldocs
 check-docs:
$(SPHINXBUILD) -b linkcheck $(ALLSPHINXOPTS) $(SPHINXBUILDDIR)/linkcheck
 .PHONY: check-docs
+
+clean-docs:
+   rm -rf $(SPHINXBUILDDIR)/*
+CLEAN_LOCAL += clean-docs
+.PHONY: clean-docs
-- 
2.9.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

2017-01-02 Thread Numan Siddique

On Mon, Jan 2, 2017 at 2:07 AM, Mickey Spiegel 
wrote:

>
> On Sun, Jan 1, 2017 at 10:31 AM, Numan Siddique 
> wrote:
>
>>
>>
>> On Sun, Jan 1, 2017 at 6:39 AM, Mickey Spiegel 
>> wrote:
>>
>>>
>>> On Sat, Dec 31, 2016 at 1:19 AM, Mickey Spiegel 
>>> wrote:
>>>

 On Fri, Dec 30, 2016 at 11:37 AM, Mickey Spiegel >>> > wrote:

>
> On Fri, Dec 30, 2016 at 7:46 AM, Numan Siddique 
> wrote:
>
>> On Fri, Dec 30, 2016 at 5:36 PM, Dong Jun  wrote:
>>
>
> 
>
>
>> 
>> Hi Dong Jun, I am also facing the same issue on my setup.
>> 
>> These are the findings of my investigation so far
>>
>> Looks like this issue is seen after the commit
>> https://github.com/openvswitch/ovs/commit/f1a8bd06d58f2c5312
>> 622fbaeacbc6ce7576e347
>> 
>> which removes the usage of patch ports and uses the clone action
>> instead.
>> 
>>
>> I reverted to the commit just before it and SNAT/DNAT is working as
>> expected.
>>
>> In my case, the gateway router is hosted on node 1 and the I am
>> trying to
>> reach a VM (192.168.0.5) hosted on node 2 using the external ip
>> (10.2.7.105) associated with it. I could see that the node 1 is
>> sending
>> the packet to node 2 through the geneve tunnel, but it is dropped by
>> node 2
>> flows.
>>
>> Below is the tcpdump of the packet
>>
>> **
>> 19:39:44.709907 IP 182.16.0.16.60069 > 182.16.0.15.geneve: Geneve,
>> Flags
>> [none], vni 0x1: IP nusiddiq.blr.redhat.com > 192.168.0.5: ICMP echo
>> request, id 13240, seq 1, length 64
>> ***
>>
>> Below is the tcpdump of the packet with the ovn-controller (without
>> the
>> above commit) in the working case
>>
>> **
>> 19:41:56.783570 IP 182.16.0.12.29778 > 182.16.0.15.geneve: Geneve,
>> Flags
>> [C], vni 0x1, options [8 bytes]: IP nusiddiq.blr.redhat.com >
>> 192.168.0.5:
>> ICMP echo request, id 13308, seq 1, length 64
>> 19:41:56.784270 IP 182.16.0.15.14539 > 182.16.0.12.geneve: Geneve,
>> Flags
>> [C], vni 0xf, options [8 bytes]: IP 192.168.0.5 >
>> nusiddiq.blr.redhat.com:
>> ICMP echo reply, id 13308, seq 1, length 64
>> **
>>
>> The options data has - 00030005
>>
>> From the packet, I could see that the packet from node 1 is missing
>> the
>> geneve option fields which has inport and outport keys.
>>
>
> I am facing the same issue running my distributed NAT patch set.
> Between UNSNAT recirc and output to tunnel, a megaflow is installed
> that
> is missing the geneve option fields.
>
> I verified that the table=32 openflow rule has the geneve option
> fields.
> ofproto/trace shows geneve in the "Datapath actions" at the end, so no
> problem with whatever ofproto/trace is using.
>

 Throwing some logs in, I see that flow->metadata.present.map is 0 rather
 than 1 coming into tun_metadata_to_geneve_nlattr() in
 lib/tun-metadata.c,
 when the problem occurs. That is why the geneve option fields are
 missing.

 I have not yet figured out why flow->metadata.present.map is 0. It
 should
 be modified when tun_metadata_write() is called due to actions setting
 tunnel metadata values. I have not checked that yet.

>>>
>>> I just posted a fix. I did not try it with the gateway router or with
>>> OpenStack,
>>> but with this bug fix all distributed NAT manual test cases are now
>>> passing.
>>>
>>>
>> Thanks for the fix. I just tested it. Its working when I am trying to
>> reach the VM using its floating ip. But not when trying to ping
>> www.google.com from the VM (SNAT use case)
>>
>
> With distributed NAT, most of my debugging and tests were using SNAT. The
> bug fix that I posted fixed the problem that was causing ICMP echo replies
> to be dropped. The openflow path for distributed SNAT is similar to that
> for SNAT on gateway routers, but there are still some differences, notably
> one router instead of two routers and no "join" switch. Also I did not try
> it with DNS.
>
> Are you able to debug further, to see whether a missing geneve options
> field is still the culprit?
> It is possible that removal of patch ports within br-int uncovered other
> issues.
>


With some testing I could see that in the node where the gateway is hosted
 - The reply packet reaches the gateway router pipeline -> to the otls
switch pipeline (via clone) -> to the router pipeline -> to the peer port
of the switch.
The packet gets dropped at table 22

 table=22, n_packets=275, n_bytes=26686,
priority=65535,ct_state=+inv+trk,metadata=0x1 actions=drop

Not sure why it is happening. I will try to debug further.

Numan



> I primarily used ovs-dpctl dump-flows to see installed megaflows, ovs-appctl
> ofproto/trace (wit

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: After thawing, retrieve tunnel table from thawed xbridge

Re: [ovs-dev] [PATCH v2 11/17] userspace: add non-tap (l3) support to GRE vports

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

Re: [ovs-dev] Sync on PTAP, EXT-382 and NSH

Re: [ovs-dev] [PATCH v2 14/17] datapath: Fix skb->protocol for vlan frames

Re: [ovs-dev] [PATCH v8] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[ovs-dev] [PATCH] ovsdb: fix data loss when OVSDB replication from itself

[ovs-dev] [PATCH v9] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

[ovs-dev] [PATCH v2 4/5] make: Ensure flake8, sphinx run when required

[ovs-dev] [PATCH v2 1/5] make: Validate documents on build

[ovs-dev] [PATCH v2 5/5] make: Standardize indentation

[ovs-dev] [PATCH v2 3/5] make: Check for Sphinx before checking docs

[ovs-dev] [PATCH v2 2/5] make: Add distinct clean-docs target

Re: [ovs-dev] ovn ping from VM to external gateway IP failed.

17 matches

Site Navigation

Mail list logo

Footer information