Re: [ovs-dev] [PATCH] FAQ: Add entry about different datapaths features.

2015-12-09 Thread Gray, Mark D
> From: Justin Pettit [mailto:jpet...@ovn.org]
> Sent: Tuesday, December 8, 2015 5:03 PM
> 
> > On Dec 8, 2015, at 1:01 AM, Gray, Mark D  wrote:
> >
> >> +Feature   | Linux upstream | Linux OVS tree | Userspace | 
> >> Hyper-V
> |
> >> +--|:--:|:--:|:-:|:-
> >> +--|--:|
> >> +Connection tracking   |  4.3   |   3.10 |NO |   
> >> NO|
> >> +Tunnel - LISP |  NO|   YES  |NO |   
> >> NO|
> >> +Tunnel - STT  |  NO|   3.5  |NO |   
> >> YES   |
> >> +Tunnel - GRE  |  3.11  |   YES  |YES|   
> >> NO|
> >> +Tunnel - VXLAN|  3.12  |   YES  |YES|   
> >> YES   |
> >> +Tunnel - Geneve   |  3.18  |   YES  |YES|   
> >> NO|
> >> +QoS   |  YES   |   YES  |NO |   
> >> NO|
> >
> > [Gray, Mark D]
> > QoS is such a loaded term. It may be good to expand on this a bit.
> 
> What about breaking it into "Traffic Policing" and "Traffic Shaping"?  Those
> are the two dimensions we usually focus on.

That helps. There is also Ingress Policing in the other direction and
the metering work being done by Jarno.

All this might make the table quite complicated though! 

> 
> --Justin
> 
> 

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


[ovs-dev] 802.1x authentication support in OVS

2015-12-09 Thread gayathri.manepalli
Hi Team,

I would like to configure the 802.1x authentication in OVS. As of my knowledge, 
current OVS does not support 802.1x authentication. If any support already 
exist or any one provided the patch, please let me know the 
details/configuration.

Thanks & Regards,
Gayathri

The information contained in this electronic message and any attachments to 
this message are intended for the exclusive use of the addressee(s) and may 
contain proprietary, confidential or privileged information. If you are not the 
intended recipient, you should not disseminate, distribute or copy this e-mail. 
Please notify the sender immediately and destroy all copies of this message and 
any attachments. WARNING: Computer viruses can be transmitted via email. The 
recipient should check this email and any attachments for the presence of 
viruses. The company accepts no liability for any damage caused by any virus 
transmitted by this email. www.wipro.com
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.

2015-12-09 Thread Zoltan Kiss



On 08/12/15 17:31, Gray, Mark D wrote:




-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org]
Sent: Tuesday, December 8, 2015 4:50 PM
To: Gray, Mark D
Cc: Traynor, Kevin; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.

On Tue, Dec 08, 2015 at 04:41:37PM +, Gray, Mark D wrote:

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Kevin
Traynor
Sent: Monday, December 7, 2015 5:58 PM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] INSTALL.DPDK.md: Clarify DPDK arguments.

Add some information about the DPDK -c and -n parameters.

Signed-off-by: Kevin Traynor 
Reported-by: Zoltan Kiss 
---
  INSTALL.DPDK.md |   14 --
  1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index
96b686c..ee016da
100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -145,8 +145,18 @@ Using the DPDK with ovs-vswitchd:

 DPDK configuration arguments can be passed to vswitchd via `--dpdk`
 argument. This needs to be first argument passed to vswitchd process.
-   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
-   for dpdk initialization.
+   The DPDK configuration arguments are passed to DPDK during DPDK
+   initialization.
+
+   The DPDK -c coremask is a required argument. To avoid wasted

resources

+   only one core should be set. The standard OVS threads (e.g. main
+   process, handler, revalidator) will run on the core that is specified.


Might be worth mentioning that then there is a corresponding potential
decrease in performance of revalidation and flow handling.


With the kernel datapath, OVS sets up flows and revalidates them on
multiple cores.  You're saying that with DPDK it only uses one core?
Why?


In my opinion, the core mask should define the affinities of the other
threads (main, process, handler, revalidator) and the pmd-cpu-mask


That would sound good, but I'm afraid you can't really influence how 
DPDK interprets the -c parameter. Maybe you can delegate the tasks of 
the non-pmd threads to the lcore threads (via 
rte_eal_[mp_]remote_launch() functions), but I'm not sure if its viable.



should define the affinities of the packet processing threads. However,
I don't know if this was the intended behavior because the name is a little
too generic ("core mask").  If this was the intended behavior, Kevin and I
just did some tests, and it is not behaving like this.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


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


Re: [ovs-dev] [PATCH net-next v4 4/8] openvswitch: Update the CT state key only after nf_conntrack_in().

2015-12-09 Thread Sergei Shtylyov

Hello.

On 12/9/2015 4:01 AM, Jarno Rajahalme wrote:


Only a successful nf_conntrack_in() call can effect a connection state
change, so if suffices to update the key only after the
nf_conntrack_in() returns.

This change is needed for the later NAT patches.

Signed-off-by: Jarno Rajahalme 
---
  net/openvswitch/conntrack.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index a28a819..10f4a6e 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -194,7 +194,6 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct 
sw_flow_key *key,
struct nf_conn *ct;
u32 new_mark;

-


   Unrelated whitespace change?

[...]

MBR, Sergei

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


[ovs-dev] Your order #72307442 - Corresponding Invoice #0BEA7625

2015-12-09 Thread Mavis Caldwell
Dear Valued Customer,

We are pleased to inform you that your order #72307442 has been processed 
andready to be dispatched. However, according to our records, above 
mentionedinvoice is still unpaid.
We would highly appreciate if you sent your paymentpromptly. For your 
information, don't hesitate to check the invoice enclosed tothis letter or 
contact us directly.
In case if you have already sent yourpayment, please disregards this letter and 
kindly allow us up to 3 business daysto clear the incoming payment.

We look forward to your remittance and will the dispatch the goods.

Thank you for choosing our services we sincerely hope to continue doing 
businesswith you again.

Sincerely,
Mavis Caldwell

Sales Department Manager
Fretter Inc.
2715 Sycamore Road
Nyssa, OR 97913
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Fix ct_state bit mappings in OVN symtab.

2015-12-09 Thread Russell Bryant
On 12/08/2015 06:20 PM, Joe Stringer wrote:
> On 8 December 2015 at 14:34, Russell Bryant  wrote:
>> The OVN symbol table contained outdated mappings between connection
>> states and the corresponding bit in the ct_state field.  This patch
>> updates the symbol table with the proper values as defined in
>> lib/packets.h.
>>
>> Signed-off-by: Russell Bryant 
> 
> Thanks for figuring this out!
> 
> Is there a way for these to use the correct constants from OVS? Maybe
> they're not exported in an appropriate header right now..

I guess I was assuming they weren't going to change again.  :-)

I'll look at another patch for this.

> Fixes: 63bc9fb1c69f ("packets: Reorder CS_* flags to remove gap.")
> Acked-by: Joe Stringer 

Thanks!  I applied this patch to master and branch-2.5.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Your order #83274151 - Corresponding Invoice #386D3034

2015-12-09 Thread Kenya Rodriquez
Dear Valued Customer,

We are pleased to inform you that your order #83274151 has been processed 
andready to be dispatched. However, according to our records, above 
mentionedinvoice is still unpaid.
We would highly appreciate if you sent your paymentpromptly. For your 
information, don't hesitate to check the invoice enclosed tothis letter or 
contact us directly.
In case if you have already sent yourpayment, please disregards this letter and 
kindly allow us up to 3 business daysto clear the incoming payment.

We look forward to your remittance and will the dispatch the goods.

Thank you for choosing our services we sincerely hope to continue doing 
businesswith you again.

Sincerely,
Kenya Rodriquez

Sales Department Manager
Fretter Inc.
2715 Sycamore Road
Nyssa, OR 97913
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] tests: Fix typo in comment.

2015-12-09 Thread Ben Pfaff
Signed-off-by: Ben Pfaff 
---
 tests/ofproto-dpif.at | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 60734db..4c2a995 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -3354,7 +3354,7 @@ 
mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,
 ])
 
 dnl Modified MPLS controller action.
-dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl In this test, the input packet is vlan-tagged, which should be stripped
 dnl before we push the MPLS and VLAN tags.
 AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
--detach --pidfile 2> ofctl_monitor.log])
 
@@ -3425,7 +3425,7 @@ 
mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,
 ])
 
 dnl Modified MPLS controller action.
-dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl In this test, the input packet is vlan-tagged, which should be stripped
 dnl before we push the MPLS and VLAN tags.
 AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
--detach --pidfile 2> ofctl_monitor.log])
 
@@ -3496,7 +3496,7 @@ 
mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,
 ])
 
 dnl Modified MPLS controller action.
-dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl In this test, the input packet is vlan-tagged, which should be stripped
 dnl before we push the MPLS and VLAN tags.
 AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
--detach --pidfile 2> ofctl_monitor.log])
 
@@ -3567,7 +3567,7 @@ 
mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,
 ])
 
 dnl Modified MPLS controller action.
-dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl In this test, the input packet is vlan-tagged, which should be stripped
 dnl before we push the MPLS and VLAN tags.
 AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 -m 65534 -P nxm 
--detach --pidfile 2> ofctl_monitor.log])
 
@@ -3601,7 +3601,7 @@ 
mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,
 ])
 
 dnl Modified MPLS controller action.
-dnl In this test, the input packet in vlan-tagged, which should be stripped
+dnl In this test, the input packet is vlan-tagged, which should be stripped
 dnl before we push the MPLS and VLAN tags.
 AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
--detach --pidfile 2> ofctl_monitor.log])
 
@@ -3635,7 +3635,7 @@ 
mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,
 ])
 
 dnl Modified MPLS controller action.
-dnl In this test, the input packet in vlan-tagged, which should be modified
+dnl In this test, the input packet is vlan-tagged, which should be modified
 dnl before we push MPLS and VLAN tags.
 AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
--detach --pidfile 2> ofctl_monitor.log])
 
-- 
2.1.3

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


Re: [ovs-dev] 802.1x authentication support in OVS

2015-12-09 Thread Justin Pettit

> On Dec 9, 2015, at 4:07 AM,  
>  wrote:
> 
> Hi Team,
> 
> I would like to configure the 802.1x authentication in OVS. As of my 
> knowledge, current OVS does not support 802.1x authentication. If any support 
> already exist or any one provided the patch, please let me know the 
> details/configuration.

I'm not aware of any.

--Justin


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


Re: [ovs-dev] [PATCH] tests: Fix typo in comment.

2015-12-09 Thread Justin Pettit
Acked-by: Justin Pettit 

--Justin


> On Dec 9, 2015, at 9:31 AM, Ben Pfaff  wrote:
> 
> Signed-off-by: Ben Pfaff 
> ---
> tests/ofproto-dpif.at | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 60734db..4c2a995 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -3354,7 +3354,7 @@ 
> mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:50,dl_dst=50:54:00:00:00:07,
> ])
> 
> dnl Modified MPLS controller action.
> -dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl In this test, the input packet is vlan-tagged, which should be stripped
> dnl before we push the MPLS and VLAN tags.
> AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
> --detach --pidfile 2> ofctl_monitor.log])
> 
> @@ -3425,7 +3425,7 @@ 
> mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:52,dl_dst=52:54:00:00:00:07,
> ])
> 
> dnl Modified MPLS controller action.
> -dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl In this test, the input packet is vlan-tagged, which should be stripped
> dnl before we push the MPLS and VLAN tags.
> AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
> --detach --pidfile 2> ofctl_monitor.log])
> 
> @@ -3496,7 +3496,7 @@ 
> mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:54,dl_dst=50:54:00:00:00:07,
> ])
> 
> dnl Modified MPLS controller action.
> -dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl In this test, the input packet is vlan-tagged, which should be stripped
> dnl before we push the MPLS and VLAN tags.
> AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
> --detach --pidfile 2> ofctl_monitor.log])
> 
> @@ -3567,7 +3567,7 @@ 
> mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:56,dl_dst=50:54:00:00:00:07,
> ])
> 
> dnl Modified MPLS controller action.
> -dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl In this test, the input packet is vlan-tagged, which should be stripped
> dnl before we push the MPLS and VLAN tags.
> AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 -m 65534 -P nxm 
> --detach --pidfile 2> ofctl_monitor.log])
> 
> @@ -3601,7 +3601,7 @@ 
> mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:57,dl_dst=50:54:00:00:00:07,
> ])
> 
> dnl Modified MPLS controller action.
> -dnl In this test, the input packet in vlan-tagged, which should be stripped
> +dnl In this test, the input packet is vlan-tagged, which should be stripped
> dnl before we push the MPLS and VLAN tags.
> AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
> --detach --pidfile 2> ofctl_monitor.log])
> 
> @@ -3635,7 +3635,7 @@ 
> mpls,dl_vlan=99,dl_vlan_pcp=1,dl_src=40:44:44:44:54:58,dl_dst=50:54:00:00:00:07,
> ])
> 
> dnl Modified MPLS controller action.
> -dnl In this test, the input packet in vlan-tagged, which should be modified
> +dnl In this test, the input packet is vlan-tagged, which should be modified
> dnl before we push MPLS and VLAN tags.
> AT_CHECK([ovs-ofctl --protocols=OpenFlow12 monitor br0 65534 -m -P nxm 
> --detach --pidfile 2> ofctl_monitor.log])
> 
> -- 
> 2.1.3
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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


[ovs-dev] [PATCH] CONTRIBUTING: Document the Fixes header.

2015-12-09 Thread Russell Bryant
Document the use of the Fixes header to refer to a commit that
introduced a bug being fixed.

Signed-off-by: Russell Bryant 
---
 CONTRIBUTING.md | 8 
 1 file changed, 8 insertions(+)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 97ab9cb..247c1d9 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -231,6 +231,14 @@ Examples of common tags follow.
 in old change log entries.  (They are obsolete because they do
 not tell the reader what bug tracker is referred to.)
 
+Fixes: 63bc9fb1c69f (“packets: Reorder CS_* flags to remove gap.”)
+
+If you would like to record which commit introduced a bug being fixed,
+you may do that with a “Fixes” header.  The easiest way to generate the
+header in the proper format is with this git command:
+
+git log -1 --pretty=format:”Fixes: %h (\”%s\”)” --abbrev=12 COMMIT_REF
+
 Developer's Certificate of Origin
 -
 
-- 
2.5.0

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


Re: [ovs-dev] [PATCH] CONTRIBUTING: Document the Fixes header.

2015-12-09 Thread Joe Stringer
On 9 December 2015 at 11:05, Russell Bryant  wrote:
> Document the use of the Fixes header to refer to a commit that
> introduced a bug being fixed.
>
> Signed-off-by: Russell Bryant 
> ---
>  CONTRIBUTING.md | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 97ab9cb..247c1d9 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -231,6 +231,14 @@ Examples of common tags follow.
>  in old change log entries.  (They are obsolete because they do
>  not tell the reader what bug tracker is referred to.)
>
> +Fixes: 63bc9fb1c69f (“packets: Reorder CS_* flags to remove gap.”)
> +
> +If you would like to record which commit introduced a bug being 
> fixed,
> +you may do that with a “Fixes” header.  The easiest way to generate 
> the
> +header in the proper format is with this git command:
> +
> +git log -1 --pretty=format:”Fixes: %h (\”%s\”)” --abbrev=12 
> COMMIT_REF
> +

I tried copy/pasting/using this command, and it failed. Seems like the
speech marks are the wrong kind, you need these instead: "

A single sentence something like "This assists in determining which
OVS releases have the bug, so the patch can be applied to all affected
versions" might be handy so people have more context on when it should
be used.

Other than that, looks good to me, thanks for the patch!

Acked-by: Joe Stringer 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Invoice #46189846 from DataCorp Inc.

2015-12-09 Thread Ora Booker
Dear Customer,

Reference nr. 46189846-1110

Our internal records show that you have an outstanding balance dating on 
youraccount. Previous invoice was for $618.73 and have yet to receive your 
payment.
You can find the copy of the invoice enclosed to this letter.

In case if you have already transferred the payment you can disregards 
thispayment notice. In all other case, please be so kind and forward us the 
amountstated in full until the end of the month.
As our agreement indicates, alloutstanding balances after 30 days are subject 
to the 7% interest fee.

Thank you in advance for your cooperation.

Sincerely,
Ora Booker

Junior Accountant
DataCorp Inc.
4851 Sugar Camp Road
Owatonna, MN 55060
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] CONTRIBUTING: Document the Fixes header.

2015-12-09 Thread Russell Bryant
On 12/09/2015 02:35 PM, Joe Stringer wrote:
> On 9 December 2015 at 11:05, Russell Bryant  wrote:
>> Document the use of the Fixes header to refer to a commit that
>> introduced a bug being fixed.
>>
>> Signed-off-by: Russell Bryant 
>> ---
>>  CONTRIBUTING.md | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
>> index 97ab9cb..247c1d9 100644
>> --- a/CONTRIBUTING.md
>> +++ b/CONTRIBUTING.md
>> @@ -231,6 +231,14 @@ Examples of common tags follow.
>>  in old change log entries.  (They are obsolete because they do
>>  not tell the reader what bug tracker is referred to.)
>>
>> +Fixes: 63bc9fb1c69f (“packets: Reorder CS_* flags to remove gap.”)
>> +
>> +If you would like to record which commit introduced a bug being 
>> fixed,
>> +you may do that with a “Fixes” header.  The easiest way to generate 
>> the
>> +header in the proper format is with this git command:
>> +
>> +git log -1 --pretty=format:”Fixes: %h (\”%s\”)” --abbrev=12 
>> COMMIT_REF
>> +
> 
> I tried copy/pasting/using this command, and it failed. Seems like the
> speech marks are the wrong kind, you need these instead: "

Weird.  Something automatically converted that copy/pasting it around.
I'll fix.

> A single sentence something like "This assists in determining which
> OVS releases have the bug, so the patch can be applied to all affected
> versions" might be handy so people have more context on when it should
> be used.

Great point.  I'll use your suggested text.

> Other than that, looks good to me, thanks for the patch!
> 
> Acked-by: Joe Stringer 

Thanks!  I'll apply this with your suggested fixes in a moment.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovn: Fix ct_state bit mappings in OVN symtab.

2015-12-09 Thread Joe Stringer
On 9 December 2015 at 08:19, Russell Bryant  wrote:
> On 12/08/2015 06:20 PM, Joe Stringer wrote:
>> On 8 December 2015 at 14:34, Russell Bryant  wrote:
>>> The OVN symbol table contained outdated mappings between connection
>>> states and the corresponding bit in the ct_state field.  This patch
>>> updates the symbol table with the proper values as defined in
>>> lib/packets.h.
>>>
>>> Signed-off-by: Russell Bryant 
>>
>> Thanks for figuring this out!
>>
>> Is there a way for these to use the correct constants from OVS? Maybe
>> they're not exported in an appropriate header right now..
>
> I guess I was assuming they weren't going to change again.  :-)

I thought that before they changed recently ;)

It's more a matter of style and greppability.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 01/14] sparse: Define INET_ADDRSTRLEN.

2015-12-09 Thread Justin Pettit
Acked-by: Justin Pettit 

--Justin


> On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> 
> POSIX defines this but it was missing from the OVS header file definitions
> for "sparse".
> 
> Signed-off-by: Ben Pfaff 
> ---
> include/sparse/netinet/in.h | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/include/sparse/netinet/in.h b/include/sparse/netinet/in.h
> index 1223553..78e5981 100644
> --- a/include/sparse/netinet/in.h
> +++ b/include/sparse/netinet/in.h
> @@ -116,6 +116,7 @@ struct sockaddr_in6 {
>  (X)->s6_addr[10] == 0xff &&\
>  (X)->s6_addr[11] == 0xff)
> 
> +#define INET_ADDRSTRLEN 16
> #define INET6_ADDRSTRLEN 46
> 
> #define IPV6_TCLASS   67
> -- 
> 2.1.3
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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


Re: [ovs-dev] [PATCH 02/14] packets: New macro ETH_ADDR_STRLEN.

2015-12-09 Thread Justin Pettit
Acked-by: Justin Pettit 

--Justin


> On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> 
> An upcoming commit will introduce another user.
> 
> Signed-off-by: Ben Pfaff 
> ---
> lib/packets.h | 1 +
> vswitchd/bridge.c | 5 +++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/packets.h b/lib/packets.h
> index edf140b..1e0417a 100644
> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -330,6 +330,7 @@ ovs_be32 set_mpls_lse_values(uint8_t ttl, uint8_t tc, 
> uint8_t bos,
> #define ETH_ADDR_ARGS(EA) ETH_ADDR_BYTES_ARGS((EA).ea)
> #define ETH_ADDR_BYTES_ARGS(EAB) \
>  (EAB)[0], (EAB)[1], (EAB)[2], (EAB)[3], (EAB)[4], (EAB)[5]
> +#define ETH_ADDR_STRLEN 17
> 
> /* Example:
>  *
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index b966d92..af10352 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2220,9 +2220,10 @@ iface_refresh_netdev_status(struct iface *iface)
> 
> error = netdev_get_etheraddr(iface->netdev, &mac);
> if (!error) {
> -char mac_string[32];
> +char mac_string[ETH_ADDR_STRLEN + 1];
> 
> -sprintf(mac_string, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
> +snprintf(mac_string, sizeof mac_string,
> + ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
> ovsrec_interface_set_mac_in_use(iface->cfg, mac_string);
> } else {
> ovsrec_interface_set_mac_in_use(iface->cfg, NULL);
> -- 
> 2.1.3
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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


Re: [ovs-dev] [PATCH 03/14] packets: Add new functions for IPv4 and IPv6 address parsing.

2015-12-09 Thread Justin Pettit
Acked-by: Justin Pettit 

--Justin


> On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> 
> These will be used in an upcoming patch to reduce duplicated code.
> 
> Signed-off-by: Ben Pfaff 
> ---
> lib/packets.c | 132 ++
> lib/packets.h |   7 
> 2 files changed, 102 insertions(+), 37 deletions(-)
> 
> diff --git a/lib/packets.c b/lib/packets.c
> index 0ad5073..309ec0c 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -416,6 +416,14 @@ ip_format_masked(ovs_be32 ip, ovs_be32 mask, struct ds 
> *s)
> }
> }
> 
> +/* Parses string 's', which must be an IP address.  Stores the IP address 
> into
> + * '*ip'.  Returns true if successful, otherwise false. */
> +bool
> +ip_parse(const char *s, ovs_be32 *ip)
> +{
> +return inet_pton(AF_INET, s, ip) == 1;
> +}
> +
> /* Parses string 's', which must be an IP address with an optional netmask or
>  * CIDR prefix length.  Stores the IP address into '*ip' and the netmask into
>  * '*mask'.  (If 's' does not contain a netmask, 255.255.255.255 is
> @@ -447,6 +455,93 @@ ip_parse_masked(const char *s, ovs_be32 *ip, ovs_be32 
> *mask)
> return NULL;
> }
> 
> +/* Similar to ip_parse_masked(), but the mask, if present, must be a CIDR 
> mask
> + * and is returned as a prefix length in '*plen'. */
> +char * OVS_WARN_UNUSED_RESULT
> +ip_parse_cidr(const char *s, ovs_be32 *ip, unsigned int *plen)
> +{
> +ovs_be32 mask;
> +char *error;
> +
> +error = ip_parse_masked(s, ip, &mask);
> +if (error) {
> +return error;
> +}
> +
> +if (!ip_is_cidr(mask)) {
> +return xasprintf("%s: CIDR network required", s);
> +}
> +*plen = ip_count_cidr_bits(mask);
> +return NULL;
> +}
> +
> +/* Parses string 's', which must be an IPv6 address.  Stores the IPv6 address
> + * into '*ip'.  Returns true if successful, otherwise false. */
> +bool
> +ipv6_parse(const char *s, struct in6_addr *ip)
> +{
> +return inet_pton(AF_INET6, s, ip) == 1;
> +}
> +
> +/* Parses string 's', which must be an IPv6 address with an optional netmask 
> or
> + * CIDR prefix length.  Stores the IPv6 address into '*ip' and the netmask 
> into
> + * '*mask'.  (If 's' does not contain a netmask, all-one-bits is assumed.)
> + *
> + * Returns NULL if successful, otherwise an error message that the caller 
> must
> + * free(). */
> +char * OVS_WARN_UNUSED_RESULT
> +ipv6_parse_masked(const char *s, struct in6_addr *ip, struct in6_addr *mask)
> +{
> +char ipv6_s[IPV6_SCAN_LEN + 1];
> +int prefix;
> +int n;
> +
> +if (ovs_scan(s, IPV6_SCAN_FMT"%n", ipv6_s, &n) && ipv6_parse(ipv6_s, 
> ip)) {
> +s += n;
> +if (!*s) {
> +*mask = in6addr_exact;
> +} else if (ovs_scan(s, "/%d%n", &prefix, &n) && !s[n]) {
> +if (prefix <= 0 || prefix > 128) {
> +return xasprintf("%s: IPv6 network prefix bits not between 0 
> "
> + "and 128", s);
> +}
> +*mask = ipv6_create_mask(prefix);
> +} else if (ovs_scan(s, "/"IPV6_SCAN_FMT"%n", ipv6_s, &n)
> +   && !s[n]
> +   && ipv6_parse(ipv6_s, mask)) {
> +/* OK. */
> +} else {
> +return xasprintf("%s: syntax error expecting IPv6 prefix length "
> + "or mask", s);
> +}
> +return NULL;
> +}
> +return xasprintf("%s: invalid IPv6 address", s);
> +}
> +
> +/* Similar to ipv6_parse_masked(), but the mask, if present, must be a CIDR
> + * mask and is returned as a prefix length in '*plen'. */
> +char * OVS_WARN_UNUSED_RESULT
> +ipv6_parse_cidr(const char *s, struct in6_addr *ip, unsigned int *plen)
> +{
> +struct in6_addr mask;
> +char *error;
> +
> +error = ipv6_parse_masked(s, ip, &mask);
> +if (error) {
> +return error;
> +}
> +
> +if (!ipv6_is_cidr(&mask)) {
> +return xasprintf("%s: IPv6 CIDR network required", s);
> +}
> +*plen = ipv6_count_cidr_bits(&mask);
> +return NULL;
> +}
> +
> +/* Stores the string representation of the IPv6 address 'addr' into the
> + * character array 'addr_str', which must be at least INET6_ADDRSTRLEN
> + * bytes long. */
> void
> ipv6_format_addr(const struct in6_addr *addr, struct ds *s)
> {
> @@ -612,43 +707,6 @@ ipv6_is_cidr(const struct in6_addr *netmask)
> return true;
> }
> 
> -/* Parses string 's', which must be an IPv6 address with an optional
> - * CIDR prefix length.  Stores the IP address into '*ipv6' and the CIDR
> - * prefix in '*prefix'.  (If 's' does not contain a CIDR length, all-ones
> - * is assumed.)
> - *
> - * Returns NULL if successful, otherwise an error message that the caller 
> must
> - * free(). */
> -char * OVS_WARN_UNUSED_RESULT
> -ipv6_parse_masked(const char *s, struct in6_addr *ipv6, struct in6_addr 
> *mask)
> -{
> -char ipv6_s[IPV6_SCAN_LEN + 1];
> -char mask_s[IPV6_SCAN_LEN + 1];
> -int prefix;
> -int

Re: [ovs-dev] [PATCH 04/14] Use ip_parse() and ipv6_parse() and variants in more places.

2015-12-09 Thread Justin Pettit

> On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> 
> 
> -gw6 = in6addr_any;
> +gw6 = in6addr_exact;

Did you mean to change this from "any" to "exact"?

This is a nice improvement.  Other than the above comment:

Acked-by: Justin Pettit 

--Justin


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


[ovs-dev] [PATCH] datapath-windows: Fix compilation issue

2015-12-09 Thread Sairam Venugopal
The previous patch was missing a ;

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Actions.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 7d34458..f6c029b 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1363,7 +1363,7 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
 
 default:
 OVS_LOG_INFO("Unhandled attribute %#x", type);
-break
+break;
 }
 return status;
 }
-- 
2.5.0.windows.1

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


Re: [ovs-dev] [PATCH] datapath-windows: Fix compilation issue

2015-12-09 Thread Nithin Raju
That was my patch that broke it. I had fixed it in my local tree but
forgot to send out the updated patch.

Thanks for fixing this.

Acked-by: Nithin Raju 



-Original Message-
From: dev  on behalf of Sairam Venugopal

Date: Wednesday, December 9, 2015 at 5:31 PM
To: "dev@openvswitch.org" 
Subject: [ovs-dev] [PATCH] datapath-windows: Fix compilation issue

>The previous patch was missing a ;
>
>Signed-off-by: Sairam Venugopal 
>---
> datapath-windows/ovsext/Actions.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c
>b/datapath-windows/ovsext/Actions.c
>index 7d34458..f6c029b 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -1363,7 +1363,7 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
> 
> default:
> OVS_LOG_INFO("Unhandled attribute %#x", type);
>-break
>+break;
> }
> return status;
> }
>-- 
>2.5.0.windows.1
>
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=7iz4DcNm8wg0BOhqisR1EIdYEFjOrx
>kl1Y_lEwrHYps&s=fmzWIoWqFq39D5lfaCK-UO9R2GBRD49X5kR9baZWKOA&e= 

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


Re: [ovs-dev] [PATCH] datapath-windows: Added recirculation support.

2015-12-09 Thread Sairam Venugopal
Hey Sorin,

Thanks for sending this out. I am currently working on a feature that
relies on recirculation. I tried applying the patch and encountered the
following errors in the log:

NlAttrParse:1103 Required attr:1 missing
_FlowNlGetCmdHandler:460 Attr Parsing failed for msg: E796E400
NlAttrParse:1103 Required attr:1 missing
_FlowNlGetCmdHandler:460 Attr Parsing failed for msg: E796E400
NlAttrParse:1103 Required attr:1 missing
_FlowNlGetCmdHandler:460 Attr Parsing failed for msg: E35EA240
NlAttrParse:1103 Required attr:1 missing
_FlowNlGetCmdHandler:460 Attr Parsing failed for msg: E2F17E40
NlAttrParse:1103 Required attr:1 missing
_FlowNlGetCmdHandler:460 Attr Parsing failed for msg: E2F17E40
NlAttrParse:1103 Required attr:1 missing
_FlowNlGetCmdHandler:460 Attr Parsing failed for msg: E2F17E40

Moreover, pings seem to fail after applying this. Are you encountering
something similar?


Thanks,
Sairam Venugopal

On 12/3/15, 1:14 AM, "Sorin Vinturis" 
wrote:

>Recirculation support for the OVS extension.
>
>Signed-off-by: Sorin Vinturis 
>Reported-by: Sorin Vinturis 
>Reported-at: 
>https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitc
>h_ovs-2Dissues_issues_104&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNt
>Xt-uEs&r=Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=Zs90ntv9lKYhlT7rupj
>F5zlQhJthjBirTyHJcaPmaVM&s=RrejKSD4SCjSvjEmKmYkDxL_mqrp1jC7iCDZCpUmqQM&e=
>---
> datapath-windows/ovsext/Actions.c | 202
>+++---
> datapath-windows/ovsext/Actions.h |  54 
> datapath-windows/ovsext/DpInternal.h  |   1 +
> datapath-windows/ovsext/Flow.c|  49 
> datapath-windows/ovsext/Flow.h|   5 +-
> datapath-windows/ovsext/Netlink/Netlink.h |  11 ++
> datapath-windows/ovsext/PacketIO.c|   1 +
> datapath-windows/ovsext/PacketIO.h|  10 --
> datapath-windows/ovsext/Recirc.c  | 140 +
> datapath-windows/ovsext/Recirc.h  | 106 
> datapath-windows/ovsext/Tunnel.c  |   1 +
> datapath-windows/ovsext/User.c|   1 +
> datapath-windows/ovsext/ovsext.vcxproj|   3 +
> 13 files changed, 553 insertions(+), 31 deletions(-)
> create mode 100644 datapath-windows/ovsext/Actions.h
> create mode 100644 datapath-windows/ovsext/Recirc.c
> create mode 100644 datapath-windows/ovsext/Recirc.h
>
>diff --git a/datapath-windows/ovsext/Actions.c
>b/datapath-windows/ovsext/Actions.c
>index e902983..fbc34de 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -14,7 +14,7 @@
>  * limitations under the License.
>  */
> 
>-#include "precomp.h"
>+#include "Actions.h"
> 
> #include "Switch.h"
> #include "Vport.h"
>@@ -26,6 +26,7 @@
> #include "Stt.h"
> #include "Checksum.h"
> #include "PacketIO.h"
>+#include "Recirc.h"
> 
> #ifdef OVS_DBG_MOD
> #undef OVS_DBG_MOD
>@@ -33,6 +34,8 @@
> #define OVS_DBG_MOD OVS_DBG_ACTION
> #include "Debug.h"
> 
>+#define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2
>+
> typedef struct _OVS_ACTION_STATS {
> UINT64 rxVxlan;
> UINT64 txVxlan;
>@@ -62,7 +65,6 @@ OVS_ACTION_STATS ovsActionStats;
>  * exercised while adding new members to the structure - only add ones
>that get
>  * used across multiple stages in the pipeline/get used in multiple
>functions.
>  */
>-#define OVS_DEST_PORTS_ARRAY_MIN_SIZE 2
> typedef struct OvsForwardingContext {
> POVS_SWITCH_CONTEXT switchContext;
> /* The NBL currently used in the pipeline. */
>@@ -95,7 +97,7 @@ typedef struct OvsForwardingContext {
>  */
> OvsIPv4TunnelKey tunKey;
> 
>- /*
>+/*
>  * Tunneling - Tx:
>  * To store the output port, when it is a tunneled port. We don't
>foresee
>  * multiple tunneled ports as outport for any given NBL.
>@@ -113,7 +115,6 @@ typedef struct OvsForwardingContext {
> OVS_PACKET_HDR_INFO layers;
> } OvsForwardingContext;
> 
>-
> /*
>  * 
>--
>  * OvsInitForwardingCtx --
>@@ -581,11 +582,13 @@ OvsDoFlowLookupOutput(OvsForwardingContext
>*ovsFwdCtx)
> if (flow) {
> OvsFlowUsed(flow, ovsFwdCtx->curNbl, &ovsFwdCtx->layers);
> ovsFwdCtx->switchContext->datapath.hits++;
>-status = OvsActionsExecute(ovsFwdCtx->switchContext,
>-   ovsFwdCtx->completionList,
>ovsFwdCtx->curNbl,
>-   ovsFwdCtx->srcVportNo,
>ovsFwdCtx->sendFlags,
>-   &key, &hash, &ovsFwdCtx->layers,
>-   flow->actions, flow->actionsLen);
>+status = OvsDoExecuteActions(ovsFwdCtx->switchContext,
>+ ovsFwdCtx->completionList,
>+ ovsFwdCtx->curNbl,
>+ ovsFwdCtx->srcVportNo,
>+ ovsFwdCtx->sendFlags,
>+  

Re: [ovs-dev] [PATCH 05/14] Add OpenFlow support for new "arp" action.

2015-12-09 Thread Justin Pettit

> On Dec 8, 2015, at 5:08 PM, Ben Pfaff  wrote:
> 
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,7 @@
> Post-v2.5.0
> -
> +   - OpenFlow:
> + * New "arp" OpenFlow extension action for generating ARP packets.

Once you have a design for packet injection from a controller, I think we 
should revisit this patch.  My initial reaction is that I'd prefer to not 
introduce changes to OVS unless they're really needed, and it's not clear to me 
whether that's the case here.  We're going to be handling the IPv6 equivalent 
from the controller, so there'd be some parity.

> + * @OVS_ACTION_ATTR_ARP: Transform IPv4 packet into ARP packet, execute 
> nested
> + * actions, restore IPv4 packet.

Another advantage of initiating the ARP in the controller is that we can buffer 
the packet, since this means that an ARP miss will always require a 
retransmission of the original packet.  We'd also have the ability to 
rate-limit ARP generation from the controller.

> +/* Parses 'arg' as the argument to a "arp" action, and appends such an
> + * action to 'ofpacts'.

s/a "arp"/an "arp"/

> --- a/lib/packets.h
> +++ b/lib/packets.h
> @@ -1041,6 +1041,7 @@ void packet_set_nd(struct dp_packet *, const ovs_be32 
> target[4],
> 
> void packet_format_tcp_flags(struct ds *, uint16_t);
> const char *packet_tcp_flag_to_string(uint32_t flag);
> +void compose_arp__(struct dp_packet *);

Somehow it seems kind of strange to have a "__" function defined in a header 
file for external users.

Acked-by: Justin Pettit 

--Justin


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


Re: [ovs-dev] [PATCH 3/3 v2] datapath-windows: Add GRE TEB support for windows datapath

2015-12-09 Thread Nithin Raju
Alin,
Thanks for adding GRE support. The patch looks good for the most part.

RFC 2890 calls for a much more elaborate handling of key and sequence
numbers in the GRE header. Eg. Buffering by handling out of order sequence
packets. In your patch, none of that is being done. Also, there¹s no way
to program the sequence number since OVS userspace does not support it as
a tunnel attribute. So, I don¹t see much value in adding support for
Œsequence number¹.

Pls. see my comments inline.

-- Nithin

-Original Message-
From: dev  on behalf of Alin Serdean

Date: Wednesday, December 2, 2015 at 12:19 PM
To: "dev@openvswitch.org" 
Subject: [ovs-dev] [PATCH 3/3 v2] datapath-windows: Add GRE TEB support
for windows datapath

>This patch introduces the support for GRE TEB (trasparent ethernet
>bridging)
>for the windows datapath.
>
>The GRE support is based on
>https://urldefense.proofpoint.com/v2/url?u=http-3A__tools.ietf.org_html_rf
>c2890&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40
>b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=YpArQ71DPRuRR2-mvTtpkpuAMzT30iOnh3Ajl-V
>HSVE&s=tzvZh5eEQknfpMgejq-iyLf6HNx_sqmZSKeTN6L8N54&e=  and supports
>only the GRE protocol type 6558 (trasparent ethernet bridging) like its
>linux
>counterpart.
>
>Util.h: define the GRE pool tag
>Vport.c/h: sort the includes alphabetically
>   add the function OvsFindTunnelVportByPortType which searches
>the
>   tunnelVportsArray for a given port type
>Actions.c : sort the includes alphabetically
>call the GRE encapsulation / decapsulation functions when
>needed
>Gre.c/h : add GRE type defines
>  add initialization/cleanup functions
>  add encapsulation / decapsulation functions with software
>offloads
>  (hardware offloads will be added in a separate patch) with
>LSO(TSO)
>  support
>
>Tested using: PSPING
>  
>(https://urldefense.proofpoint.com/v2/url?u=https-3A__technet.microsoft.co
>m_en-2Dus_sysinternals_psping.aspx&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeA
>w-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=YpArQ71DPR
>uRR2-mvTtpkpuAMzT30iOnh3Ajl-VHSVE&s=MzyMozrD9wsPavMwqZMJsiDWNsTNyz8hZnZ6gl
>jGNzw&e= )
>  (ICMP, TCP, UDP) with various packet lengths
>  IPERF3
>  
>(https://urldefense.proofpoint.com/v2/url?u=https-3A__iperf.fr_iperf-2Ddow
>nload.php&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr
>7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=YpArQ71DPRuRR2-mvTtpkpuAMzT30iOnh3A
>jl-VHSVE&s=whqEL6zIeu2p7KE_-cN8HL7GI1x8G6hp0wTcN-rLPe0&e= )
>  (TCP, UDP) with various options
>
>Signed-off-by: Alin Gabriel Serdean 
>---
>v2: add Gre.c/h to automake.mk EXTRA_DIST
>---
> datapath-windows/automake.mk   |  20 +-
> datapath-windows/ovsext/Actions.c  |  71 +++--
> datapath-windows/ovsext/Gre.c  | 456
>+
> datapath-windows/ovsext/Gre.h  | 113 
> datapath-windows/ovsext/Util.h |   1 +
> datapath-windows/ovsext/Vport.c|  43 +++-
> datapath-windows/ovsext/Vport.h|  14 +-
> datapath-windows/ovsext/ovsext.vcxproj |   2 +
> 8 files changed, 676 insertions(+), 44 deletions(-)
> create mode 100644 datapath-windows/ovsext/Gre.c
> create mode 100644 datapath-windows/ovsext/Gre.h
>
>diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
>index ed48c69..7f12d92 100644
>--- a/datapath-windows/automake.mk
>+++ b/datapath-windows/automake.mk
>@@ -4,45 +4,49 @@ EXTRA_DIST += \
>   datapath-windows/Package/package.VcxProj \
>   datapath-windows/Package/package.VcxProj.user \
>   datapath-windows/include/OvsDpInterfaceExt.h \
>+  datapath-windows/misc/OVS.psm1 \
>   datapath-windows/misc/install.cmd \
>   datapath-windows/misc/uninstall.cmd \
>-  datapath-windows/misc/OVS.psm1 \
>   datapath-windows/ovsext.sln \
>-  datapath-windows/ovsext/Datapath.c \
>-  datapath-windows/ovsext/Datapath.h \
>-  datapath-windows/ovsext/DpInternal.h\
>   datapath-windows/ovsext/Actions.c \
>   datapath-windows/ovsext/Atomic.h \
>   datapath-windows/ovsext/BufferMgmt.c \
>   datapath-windows/ovsext/BufferMgmt.h \
>   datapath-windows/ovsext/Checksum.c \
>   datapath-windows/ovsext/Checksum.h \
>+  datapath-windows/ovsext/Datapath.c \
>+  datapath-windows/ovsext/Datapath.h \
>   datapath-windows/ovsext/Debug.c \
>   datapath-windows/ovsext/Debug.h \
>+  datapath-windows/ovsext/DpInternal.h\
>   datapath-windows/ovsext/Driver.c \
>   datapath-windows/ovsext/Ethernet.h \
>   datapath-windows/ovsext/Event.c \
>   datapath-windows/ovsext/Event.h \
>   datapath-windows/ovsext/Flow.c \
>   datapath-windows/ovsext/Flow.h \
>+  datapath-windows/ovsext/Gre.h \
>+  datapath-windows/ovsext/Gre.c \
>   datapath-windows/ovsext/IpHelper.c \
>   datapath-windows/ovsext/IpHelper.h \
>   datapath-windows/ovse

Re: [ovs-dev] [PATCH 1/3] datapath-windows: Add sequence tunneling information define

2015-12-09 Thread Nithin Raju
I suggested we get rid of ŒOVS_TNL_F_SEQ¹ in the 3/3 patch, since there¹s
no way of using it.

-- Nithin

-Original Message-
From: dev  on behalf of Alin Serdean

Date: Wednesday, December 2, 2015 at 9:50 AM
To: "dev@openvswitch.org" 
Subject: [ovs-dev] [PATCH 1/3] datapath-windows: Add sequence tunneling
information define

>This patch adds the sequence tunneling information which will be needed
>for
>GRE tunneling.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> datapath-windows/ovsext/Flow.h | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/datapath-windows/ovsext/Flow.h
>b/datapath-windows/ovsext/Flow.h
>index 74b9dfb..0d4009a 100644
>--- a/datapath-windows/ovsext/Flow.h
>+++ b/datapath-windows/ovsext/Flow.h
>@@ -83,5 +83,6 @@ UINT32 OvsTunKeyAttrSize(void);
> #define OVS_TNL_F_DONT_FRAGMENT (1 << 0)
> #define OVS_TNL_F_CSUM  (1 << 1)
> #define OVS_TNL_F_KEY   (1 << 2)
>+#define OVS_TNL_F_SEQ   (1 << 3)
> 
> #endif /* __FLOW_H_ */
>-- 
>1.9.5.msysgit.0
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=waN6REJQ8BPz5NGnNO2kn4Z21UcO_W
>u1Dy8MG9aDSn8&s=J4-_IyUF3p2E00rqJC7u8swnlxXI6Swik7XuoSTsYtE&e= 

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


[ovs-dev] [PATCH 0/9] Translation fixes for revalidation

2015-12-09 Thread Daniele Di Proietto
Sometimes the ofproto layer creates a flow which is not liked by the
revalidation for various reasons.  This behavior, while not critical
might impact the performance.  This series aims to fix a lot of these
bugs.

The detection has been done by modifying OVS to revalidate a flow as
soon as it is installed (this is not included in the series, I'd be
happy to discuss strategies to merge something like that upstream).
If the revalidation complains there's a bug. This series fixes all the
bugs found in the testsuite.

The first commits are trivial fixes to various components in OVS. The
last three commits address more complicated problems and I'd be happy
to discuss alternative (maybe simpler) solutions.

Daniele Di Proietto (9):
  dpif-netdev: Initialize match.tun_md in various places.
  ofproto-dpif-xlate: Fix revalidation in execute_controller_action().
  tnl-ports: Generate mask with correct prerequisites.
  odp-util: Commit ICMP set only for ICMP packets.
  odp-util: Return exact mask if netlink mask attribute is missing.
  odp-util: Correctly [de]serialize mask for ND attributes.
  ofproto-dpif-xlate: Generate right mask when checking prereqs.
  ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.
  ofproto-dpif-xlate: Do not include non existing MPLS lse in wc.

 lib/dpctl.c   |   2 +-
 lib/dpif-netdev.c |  48 +-
 lib/flow.h|  38 ++--
 lib/meta-flow.c   | 202 +-
 lib/meta-flow.h   |   6 +-
 lib/nx-match.c|  12 +--
 lib/odp-util.c|  53 ---
 lib/odp-util.h|   4 +-
 lib/tnl-ports.c   |   6 +-
 ofproto/ofproto-dpif-upcall.c |   2 +-
 ofproto/ofproto-dpif-xlate.c  |  26 +-
 tests/mpls-xlate.at   |   2 +-
 tests/test-odp.c  |   2 +-
 13 files changed, 278 insertions(+), 125 deletions(-)

-- 
2.1.4

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


[ovs-dev] [PATCH 5/9] odp-util: Return exact mask if netlink mask attribute is missing.

2015-12-09 Thread Daniele Di Proietto
In the ODP context an empty mask netlink attribute usually means that
the flow should be an exact match.

odp_flow_key_to_mask{,_udpif}() instead return a struct flow_wildcards
with matches only on recirc_id and vlan_tci.

A more appropriate behavior is to handle a missing (zero length) netlink
mask specially (like we do in userspace and Linux datapath) and create
an exact match flow_wildcards from the original flow.

This fixes a bug in revalidate_ukey(): every flow created with
megaflows disabled would be revalidated away, because the mask would
seem too generic. (Another possible fix would be to handle the special
case of a missing mask in revalidate_ukey(), but this seems a more
generic solution).

Signed-off-by: Daniele Di Proietto 
---
 lib/dpctl.c   |  2 +-
 lib/dpif-netdev.c | 46 ---
 lib/odp-util.c| 35 ++--
 lib/odp-util.h|  4 ++--
 ofproto/ofproto-dpif-upcall.c |  2 +-
 tests/test-odp.c  |  2 +-
 6 files changed, 55 insertions(+), 36 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 438bfd3..26de23f 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -797,7 +797,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 
 odp_flow_key_to_flow(f.key, f.key_len, &flow);
 odp_flow_key_to_mask(f.mask, f.mask_len, f.key, f.key_len,
- &wc.masks, &flow);
+ &wc, &flow);
 match_init(&match, &flow, &wc);
 
 match_init(&match_filter, &flow_filter, &wc);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3bf130d..8794ca0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1858,33 +1858,29 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, 
uint32_t key_len,
   uint32_t mask_key_len, const struct flow *flow,
   struct flow_wildcards *wc)
 {
-if (mask_key_len) {
-enum odp_key_fitness fitness;
-
-fitness = odp_flow_key_to_mask_udpif(mask_key, mask_key_len, key,
- key_len, &wc->masks, flow);
-if (fitness) {
-/* This should not happen: it indicates that
- * odp_flow_key_from_mask() and odp_flow_key_to_mask()
- * disagree on the acceptable form of a mask.  Log the problem
- * as an error, with enough details to enable debugging. */
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-if (!VLOG_DROP_ERR(&rl)) {
-struct ds s;
-
-ds_init(&s);
-odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
-true);
-VLOG_ERR("internal error parsing flow mask %s (%s)",
- ds_cstr(&s), odp_key_fitness_to_string(fitness));
-ds_destroy(&s);
-}
+enum odp_key_fitness fitness;
+
+fitness = odp_flow_key_to_mask_udpif(mask_key, mask_key_len, key,
+ key_len, wc, flow);
+if (fitness) {
+/* This should not happen: it indicates that
+ * odp_flow_key_from_mask() and odp_flow_key_to_mask()
+ * disagree on the acceptable form of a mask.  Log the problem
+ * as an error, with enough details to enable debugging. */
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+if (!VLOG_DROP_ERR(&rl)) {
+struct ds s;
 
-return EINVAL;
+ds_init(&s);
+odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
+true);
+VLOG_ERR("internal error parsing flow mask %s (%s)",
+ ds_cstr(&s), odp_key_fitness_to_string(fitness));
+ds_destroy(&s);
 }
-} else {
-flow_wildcards_init_for_packet(wc, flow);
+
+return EINVAL;
 }
 
 return 0;
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 4db371d..3717aee 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -5152,6 +5152,26 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t 
key_len,
return odp_flow_key_to_flow__(key, key_len, NULL, 0, flow, flow, false);
 }
 
+static enum odp_key_fitness
+odp_flow_key_to_mask__(const struct nlattr *mask_key, size_t mask_key_len,
+   const struct nlattr *flow_key, size_t flow_key_len,
+   struct flow_wildcards *mask,
+   const struct flow *src_flow,
+   bool udpif)
+{
+if (mask_key_len) {
+return odp_flow_key_to_flow__(mask_key, mask_key_len,
+  flow_key, flow_key_len,
+  &mask->masks, src_flow, udpif);
+
+} else {
+/* A missing mask means that the flow should be exact matched

[ovs-dev] [PATCH 1/9] dpif-netdev: Initialize match.tun_md in various places.

2015-12-09 Thread Daniele Di Proietto
This solves a crash in dp_netdev_flow_add(), when log level is debug.

Signed-off-by: Daniele Di Proietto 
---
 lib/dpif-netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a67ef05..3bf130d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1993,6 +1993,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
 struct match match;
 struct ds ds = DS_EMPTY_INITIALIZER;
 
+match.tun_md.valid = false;
 match.flow = flow->flow;
 miniflow_expand(&flow->cr.mask->mf, &match.wc.masks);
 
@@ -3300,6 +3301,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
 
 miss_cnt++;
 
+match.tun_md.valid = false;
 miniflow_expand(&keys[i].mf, &match.flow);
 
 ofpbuf_clear(&actions);
-- 
2.1.4

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


[ovs-dev] [PATCH 9/9] ofproto-dpif-xlate: Do not include non existing MPLS lse in wc.

2015-12-09 Thread Daniele Di Proietto
An action list like

actions=push_mpls:0x8847,load:10->OXM_OF_MPLS_LABEL[]

will generate an exact match on the newly pushed mpls label, because the
load action needs to unwildcard its target to work properly.  This
doesn't make sense, because the original flow didn't have that label. An
invalid mask like this will cause the flow to be deleted by
revalidation.

Fix the problem by storing the original number of mpls labels and
clearing the ones that do not make sense in xlate_wc_finish().

Signed-off-by: Daniele Di Proietto 
---
 ofproto/ofproto-dpif-xlate.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index eff938b..dc7d3af 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -309,6 +309,9 @@ struct xlate_ctx {
 /* Pointer to an embedded NAT action in a conntrack action, or NULL. */
 struct ofpact_nat *ct_nat_action;
 
+/* Remember the number of mpls labels in the original flow. */
+unsigned orig_n_mpls_lse;
+
 /* OpenFlow 1.1+ action set.
  *
  * 'action_set' accumulates "struct ofpact"s added by OFPACT_WRITE_ACTIONS.
@@ -4999,6 +5002,8 @@ xlate_wc_init(struct xlate_ctx *ctx)
 static void
 xlate_wc_finish(struct xlate_ctx *ctx)
 {
+size_t i;
+
 /* Clear the metadata and register wildcard masks, because we won't
  * use non-header fields as part of the cache. */
 flow_wildcards_clear_non_packet_fields(ctx->wc);
@@ -5021,6 +5026,15 @@ xlate_wc_finish(struct xlate_ctx *ctx)
 if (ctx->wc->masks.vlan_tci) {
 ctx->wc->masks.vlan_tci |= htons(VLAN_CFI);
 }
+
+/* A set action on an MPLS label after a push_mpls action might
+ * unwildcard a label that was not in the original flow. We must
+ * make sure that the generated wildcard doesn't try to match on
+ * non existing labels. */
+for (i = ctx->orig_n_mpls_lse; i < FLOW_MAX_MPLS_LABELS; i++) {
+ctx->wc->masks.mpls_lse[i] = 0;
+}
+
 }
 
 /* Translates the flow, actions, or rule in 'xin' into datapath actions in
@@ -5118,6 +5132,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out 
*xout)
 xlate_wc_init(&ctx);
 }
 
+/* Store the number of MPLS lse in the original flow */
+ctx.orig_n_mpls_lse = flow_count_mpls_labels(flow, ctx.wc);
+
 COVERAGE_INC(xlate_actions);
 
 if (xin->recirc) {
-- 
2.1.4

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


[ovs-dev] [PATCH 7/9] ofproto-dpif-xlate: Generate right mask when checking prereqs.

2015-12-09 Thread Daniele Di Proietto
During translation we need to unwildcard each member of the flow that we
look at.  When setting or moving a field, we need to look at (and
consequently unwildcard) the field itself an all the prerequisites.

The current code unwildcards the field and all the prerequisites and
then reads them.  This behavior is wrong, because if prerequisites are
not met we might end up unwildcarding more fields than necessary,
generating masks that don't make sense.

The bug can be triggered with an action that sets the tcp src port and
a fragmented IP packet.  The translation will not generate a set action,
but it will match on the target field (tcp src port), even though the
prerequisites for that are not met.

The wrong mask causes the flow to get deleted by revalidation.

Fix this by introducing mf_check_prereqs_and_set_mask(), which checks
the prerequisites while masking the field. It is based on the behavior
of mf_are_prereqs_ok().

mf_mask_field_and_prereqs() is not used anymore, so it can be removed.
mf_are_prereqs_ok() can be rewritten using
mf_check_prereqs_and_set_mask(), avoiding code duplication

Signed-off-by: Daniele Di Proietto 
---
 lib/flow.h   |  38 +--
 lib/meta-flow.c  | 113 +++
 lib/meta-flow.h  |   5 +-
 lib/nx-match.c   |  10 ++--
 ofproto/ofproto-dpif-xlate.c |   4 +-
 5 files changed, 92 insertions(+), 78 deletions(-)

diff --git a/lib/flow.h b/lib/flow.h
index 5d78615..6f99297 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -999,21 +999,49 @@ pkt_metadata_from_flow(struct pkt_metadata *md, const 
struct flow *flow)
 md->ct_label = flow->ct_label;
 }
 
+/* Often, during translation we need to read a value from a flow and
+ * unwildcard the corresponding bits in the wildcards. */
+
+#define FLOW_WC_GET_AND_MASK(FLOW, WC, FIELD) \
+(((WC) ? WC_MASK_FIELD(WC, FIELD) : NULL), (FLOW)->FIELD)
+
+#define FLOW_WC_GET_AND_MASK_MASKED(FLOW, WC, FIELD, MASK) \
+(((WC) ? WC_MASK_FIELD_MASK(WC, FIELD, MASK) : 0), \
+ (((FLOW)->FIELD) & (MASK)))
+
+static inline bool check_and_mask_ip_any(const struct flow *flow,
+ struct flow_wildcards *wc)
+{
+return dl_type_is_ip_any(FLOW_WC_GET_AND_MASK(flow, wc, dl_type));
+}
+
+static inline bool check_and_mask_icmpv4(const struct flow *flow,
+ struct flow_wildcards *wc)
+{
+return (FLOW_WC_GET_AND_MASK(flow, wc, dl_type) == htons(ETH_TYPE_IP)
+&& FLOW_WC_GET_AND_MASK(flow, wc, nw_proto) == IPPROTO_ICMP);
+}
+
+static inline bool check_and_mask_icmpv6(const struct flow *flow,
+ struct flow_wildcards *wc)
+{
+return (FLOW_WC_GET_AND_MASK(flow, wc, dl_type) == htons(ETH_TYPE_IPV6)
+&& FLOW_WC_GET_AND_MASK(flow, wc, nw_proto) == IPPROTO_ICMPV6);
+}
+
 static inline bool is_ip_any(const struct flow *flow)
 {
-return dl_type_is_ip_any(flow->dl_type);
+return check_and_mask_ip_any(flow, NULL);
 }
 
 static inline bool is_icmpv4(const struct flow *flow)
 {
-return (flow->dl_type == htons(ETH_TYPE_IP)
-&& flow->nw_proto == IPPROTO_ICMP);
+return check_and_mask_icmpv4(flow, NULL);
 }
 
 static inline bool is_icmpv6(const struct flow *flow)
 {
-return (flow->dl_type == htons(ETH_TYPE_IPV6)
-&& flow->nw_proto == IPPROTO_ICMPV6);
+return check_and_mask_icmpv6(flow, NULL);
 }
 
 static inline bool is_igmp(const struct flow *flow)
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 6bd0b99..398b2f2 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -356,94 +356,79 @@ mf_is_mask_valid(const struct mf_field *mf, const union 
mf_value *mask)
 bool
 mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow)
 {
+return mf_check_prereqs_and_set_mask(mf, flow, NULL);
+}
+
+/* Returns true if 'flow' meets the prerequisites for 'mf', false otherwise.
+ * If an attribute is read from 'flow', the corresponding attribute in 'wc'
+ * will be unwildcarded. */
+bool
+mf_check_prereqs_and_set_mask(const struct mf_field *mf,
+  const struct flow *flow,
+  struct flow_wildcards *wc)
+{
 switch (mf->prereqs) {
 case MFP_NONE:
 return true;
 
 case MFP_ARP:
-  return (flow->dl_type == htons(ETH_TYPE_ARP) ||
-  flow->dl_type == htons(ETH_TYPE_RARP));
+return FLOW_WC_GET_AND_MASK(flow, wc, dl_type) == htons(ETH_TYPE_ARP)
+|| FLOW_WC_GET_AND_MASK(flow, wc, dl_type) == htons(ETH_TYPE_RARP);
 case MFP_IPV4:
-return flow->dl_type == htons(ETH_TYPE_IP);
+return FLOW_WC_GET_AND_MASK(flow, wc, dl_type) == htons(ETH_TYPE_IP);
 case MFP_IPV6:
-return flow->dl_type == htons(ETH_TYPE_IPV6);
+return FLOW_WC_GET_AND_MASK(flow, wc, dl_type) == htons(ETH_TYPE_IPV6);
 case MFP_VLAN_VID:
-return (flow->vlan_tci & htons(VLAN_CFI)) 

[ovs-dev] [PATCH 3/9] tnl-ports: Generate mask with correct prerequisites.

2015-12-09 Thread Daniele Di Proietto
We should match on the transport ports only if the tunnel has a UDP
header.  It doesn't make sense to match on transport port for GRE
tunnels.

Also, to match on fragment bits we should use FLOW_NW_FRAG_MASK instead
of 0xFF.  FLOW_NW_FRAG_MASK is what we get if we convert to the ODP
netlink format and back.

Adding the correct masks in the tunnel router classifier helps in making
sure that the translation generates a masks that respects prerequisites.

If the mask has some fields that do not respect prerequisites, the flow
will get deleted by revalidation, because translating to ODP format and
back will generate a more generic mask, which will be perceived as too
generic (compared with the one generated by the translation).

Signed-off-by: Daniele Di Proietto 
---
 lib/tnl-ports.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 3006a8b..890a8b2 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -126,8 +126,10 @@ map_insert(odp_port_t port, struct eth_addr mac, struct 
in6_addr *addr,
 
 match.wc.masks.dl_type = OVS_BE16_MAX;
 match.wc.masks.nw_proto = 0xff;
-match.wc.masks.nw_frag = 0xff;  /* XXX: No fragments support. */
-match.wc.masks.tp_dst = OVS_BE16_MAX;
+match.wc.masks.nw_frag = FLOW_NW_FRAG_MASK;  /* XXX: No fragments 
support. */
+if (udp_port) {
+match.wc.masks.tp_dst = OVS_BE16_MAX;
+}
 if (IN6_IS_ADDR_V4MAPPED(addr)) {
 match.wc.masks.nw_dst = OVS_BE32_MAX;
 } else {
-- 
2.1.4

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


[ovs-dev] [PATCH 8/9] ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.

2015-12-09 Thread Daniele Di Proietto
When translating a set action we also unwildcard the field in question.
This is done to correctly translate set actions with the value identical
to the ingress flow, like in the following example:

flow table:

tcp,actions=set_field:80->tcp_dst,output:5

ingress packet:

...,tcp,tcp_dst=80

datapath flow

...,tcp(dst=80) actions:5

The datapath flow must exact match the target field, because the actions
do not include a set field. (Otherwise a packet coming in with a
different tcp_dst would be matched, and its port wouldn't be altered).

Tunnel attributes behave differently: at the datapath layer, before
action processing they're cleared (we do the same at the ofproto layer
in xlate_actions()).  Therefore there's no need to unwildcard them,
because a set action would always be detected (since we zero them at the
beginning of xlate_ations()).

This fixes a problem related to the handling of Geneve options.
Unwildcarding non existing Geneve options (as done by a
set_field:X->tun_metadata on a packet coming from a non-tunnel
interface) would be problematic for the datapaths: the ODP translation
functions cannot express a match on non existing Geneve options (unlike
on other attributes), and the userspace datapath wouldn't be able to
translate them to "userspace datapath format".  In both cases the
installed flow would be deleted by revalidation at the first
opportunity.

Signed-off-by: Daniele Di Proietto 
---
 lib/meta-flow.c  | 89 
 lib/meta-flow.h  |  1 +
 lib/nx-match.c   |  4 +-
 ofproto/ofproto-dpif-xlate.c |  4 +-
 4 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 398b2f2..e6287d5 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1421,6 +1421,95 @@ mf_is_tun_metadata(const struct mf_field *mf)
mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
 }
 
+/* Returns true if a field 'mf' should be exact matched before being set
+ * by the action translation, false otherwise.  Most of the fields need
+ * an exact match.*/
+bool
+mf_set_must_mask(const struct mf_field *mf)
+{
+/* Tunnel attributes don't need an exact match, because they are
+ * cleared by the datapath between ingress and egress. Also, an
+ * exact match on tunnel metadata might be problematic, because
+ * it is not possible to express it if the metadata didn't exist
+ * on ingress. */
+switch (mf->id) {
+case MFF_TUN_ID:
+case MFF_TUN_SRC:
+case MFF_TUN_DST:
+case MFF_TUN_IPV6_SRC:
+case MFF_TUN_IPV6_DST:
+case MFF_TUN_FLAGS:
+case MFF_TUN_GBP_ID:
+case MFF_TUN_GBP_FLAGS:
+case MFF_TUN_TOS:
+case MFF_TUN_TTL:
+CASE_MFF_TUN_METADATA:
+return false;
+
+case MFF_DP_HASH:
+case MFF_RECIRC_ID:
+case MFF_CONJ_ID:
+case MFF_METADATA:
+case MFF_IN_PORT:
+case MFF_IN_PORT_OXM:
+case MFF_ACTSET_OUTPUT:
+case MFF_SKB_PRIORITY:
+case MFF_PKT_MARK:
+case MFF_CT_STATE:
+case MFF_CT_ZONE:
+case MFF_CT_MARK:
+case MFF_CT_LABEL:
+CASE_MFF_REGS:
+CASE_MFF_XREGS:
+case MFF_ETH_SRC:
+case MFF_ETH_DST:
+case MFF_ETH_TYPE:
+case MFF_VLAN_TCI:
+case MFF_DL_VLAN:
+case MFF_VLAN_VID:
+case MFF_DL_VLAN_PCP:
+case MFF_VLAN_PCP:
+case MFF_MPLS_LABEL:
+case MFF_MPLS_TC:
+case MFF_MPLS_BOS:
+case MFF_IPV4_SRC:
+case MFF_ARP_SPA:
+case MFF_IPV4_DST:
+case MFF_ARP_TPA:
+case MFF_IPV6_SRC:
+case MFF_IPV6_DST:
+case MFF_IPV6_LABEL:
+case MFF_IP_PROTO:
+case MFF_IP_DSCP:
+case MFF_IP_DSCP_SHIFTED:
+case MFF_IP_ECN:
+case MFF_IP_TTL:
+case MFF_IP_FRAG:
+case MFF_ARP_OP:
+case MFF_ARP_SHA:
+case MFF_ND_SLL:
+case MFF_ARP_THA:
+case MFF_ND_TLL:
+case MFF_TCP_SRC:
+case MFF_UDP_SRC:
+case MFF_SCTP_SRC:
+case MFF_ICMPV4_TYPE:
+case MFF_ICMPV6_TYPE:
+case MFF_TCP_DST:
+case MFF_UDP_DST:
+case MFF_SCTP_DST:
+case MFF_ICMPV4_CODE:
+case MFF_ICMPV6_CODE:
+case MFF_TCP_FLAGS:
+case MFF_ND_TARGET:
+return true;
+
+case MFF_N_IDS:
+default:
+OVS_NOT_REACHED();
+}
+}
+
 /* Returns true if 'mf' has previously been set in 'flow', false if
  * it contains a non-default value.
  *
diff --git a/lib/meta-flow.h b/lib/meta-flow.h
index 1e6055b..b18cd97 100644
--- a/lib/meta-flow.h
+++ b/lib/meta-flow.h
@@ -1993,6 +1993,7 @@ void mf_set_flow_value_masked(const struct mf_field *,
   const union mf_value *mask,
   struct flow *);
 bool mf_is_tun_metadata(const struct mf_field *);
+bool mf_set_must_mask(const struct mf_field *);
 bool mf_is_set(const struct mf_field *, const struct flow *);
 void mf_mask_field(const struct mf_field *, struct flow *);
 int mf_field_len(const struct mf_field *, const union mf_value *value,
diff --git a/lib/nx-match.c b/lib/nx-match.c
index f56c9

[ovs-dev] [PATCH 2/9] ofproto-dpif-xlate: Fix revalidation in execute_controller_action().

2015-12-09 Thread Daniele Di Proietto
If there's no actual packet (e.g. during revalidation),
execute_controller_action() exits right away, without calling
xlate_commit_actions().

xlate_commit_actions() might have an influence on slow_path reason
(which is included in the generated ODP actions), meaning that the
revalidation will not generate the same actions than the original
translation.

Fix the problem my making execute_controller_action() call
xlate_commit_actions() even without a packet.

Signed-off-by: Daniele Di Proietto 
---
 ofproto/ofproto-dpif-xlate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index cf184e4..dab64b9 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3572,14 +3572,13 @@ execute_controller_action(struct xlate_ctx *ctx, int 
len,
 struct dp_packet *packet;
 
 ctx->xout->slow |= SLOW_CONTROLLER;
+xlate_commit_actions(ctx);
 if (!ctx->xin->packet) {
 return;
 }
 
 packet = dp_packet_clone(ctx->xin->packet);
 
-xlate_commit_actions(ctx);
-
 odp_execute_actions(NULL, &packet, 1, false,
 ctx->odp_actions->data, ctx->odp_actions->size, NULL);
 
-- 
2.1.4

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


[ovs-dev] [PATCH 6/9] odp-util: Correctly [de]serialize mask for ND attributes.

2015-12-09 Thread Daniele Di Proietto
When converting between ODP attributes and struct flow_wildcards, we
check that all the prerequisites are exact matched on the mask.

For ND(ICMPv6) attributes, an exact match on tp_src and tp_dst
(which in this context are the icmp type and code) shold look like
htons(0xff), not htons(0x).  Fix this in two places.

The consequences were that the datapath wouldn't match on the ND
attributes, and the flow would get revalidated away.

Signed-off-by: Daniele Di Proietto 
---
 lib/odp-util.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 3717aee..fbc0788 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -4358,8 +4358,8 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms 
*parms,
 if (flow->tp_dst == htons(0)
 && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT)
 || flow->tp_src == htons(ND_NEIGHBOR_ADVERT))
-&& (!export_mask || (data->tp_src == htons(0x)
- && data->tp_dst == htons(0x {
+&& (!export_mask || (data->tp_src == htons(0xff)
+ && data->tp_dst == htons(0xff {
 
 struct ovs_key_nd *nd_key;
 
@@ -4905,8 +4905,8 @@ parse_l2_5_onward(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
 flow->arp_tha = nd_key->nd_tll;
 if (is_mask) {
 if (!is_all_zeros(nd_key, sizeof *nd_key) &&
-(flow->tp_src != htons(0x) ||
- flow->tp_dst != htons(0x))) {
+(flow->tp_src != htons(0xff) ||
+ flow->tp_dst != htons(0xff))) {
 return ODP_FIT_ERROR;
 } else {
 expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
-- 
2.1.4

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


[ovs-dev] [PATCH 4/9] odp-util: Commit ICMP set only for ICMP packets.

2015-12-09 Thread Daniele Di Proietto
commit_set_icmp_action() should do its job only if the packet is ICMP,
otherwise there will be two problems:

* A set ICMP action will be inserted in the ODP actions and the flow
  will be slow pathed.
* The tp_src and tp_dst field will be unwildcarded.

Normal TCP or UDP packets won't be impacted, because
commit_set_port_action() is called before commit_set_icmp_actions().

MPLS packets though will hit the bug, causing a nonsensical set action
(which will end up zeroing the transport source port) and an invalid
mask to be generated.

The commit also alters an MPLS testcase to trigger the bug.

Signed-off-by: Daniele Di Proietto 
---
 lib/odp-util.c  | 10 --
 tests/mpls-xlate.at |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 90942c7..4db371d 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -5651,12 +5651,18 @@ commit_set_icmp_action(const struct flow *flow, struct 
flow *base_flow,
 struct ovs_key_icmp key, mask, base;
 enum ovs_key_attr attr;
 
+if (is_icmpv4(flow)) {
+attr = OVS_KEY_ATTR_ICMP;
+} else if (is_icmpv6(flow)) {
+attr = OVS_KEY_ATTR_ICMPV6;
+} else {
+return 0;
+}
+
 get_icmp_key(flow, &key);
 get_icmp_key(base_flow, &base);
 get_icmp_key(&wc->masks, &mask);
 
-attr = flow->dl_type == htons(ETH_TYPE_IP) ? OVS_KEY_ATTR_ICMP
-   : OVS_KEY_ATTR_ICMPV6;
 if (commit(attr, false, &key, &base, &mask, sizeof key, odp_actions)) {
 put_icmp_key(&base, base_flow);
 put_icmp_key(&mask, &wc->masks);
diff --git a/tests/mpls-xlate.at b/tests/mpls-xlate.at
index 8f286c3..38790ea 100644
--- a/tests/mpls-xlate.at
+++ b/tests/mpls-xlate.at
@@ -16,7 +16,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 
in_port=local,dl_type=0x0800,acti
 AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 
dl_type=0x8847,in_port=1,mpls_label=20,action=pop_mpls:0x0800,output:LOCAL])
 
 dnl Test MPLS push
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=1.1.2.92,dst=1.1.2.88,proto=47,tos=0,ttl=64,frag=no)'],
 [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(100),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=1.1.2.92,dst=1.1.2.88,proto=17,tos=0,ttl=64,frag=no),udp(src=,dst=80)'],
 [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
   [Datapath actions: push_mpls(label=10,tc=0,ttl=64,bos=1,eth_type=0x8847),1
 ])
-- 
2.1.4

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


Re: [ovs-dev] [ovs] ovn-nbctl:add db commands help (#93)

2015-12-09 Thread Wei Li

在 2015/12/8 22:57, Russell Bryant 写道:

On 12/08/2015 01:06 AM, Wei Li wrote:


在 2015/12/7 23:30, Russell Bryant 写道:

Can you update the man page, as well?


I would like to do this,but have a question

There is a reference to /db-ctl-base.man/ in /ovn-sbctl.8.in/  implement
by "/.so lib/db-ctl-base.man/"

but ovn-nbctl`s manpage is xml format

How to add a reference to another manual page in xml file?

Good question.  I don't know of an example you can copy so we may have
to create it.  ovn-sbctl needs the same content, but is written in nroff
instead of the XML format.

Perhaps we need an XML equivalent to db-ctl-base.man and then include it
using xi:include.

...
   
...

xml2nroff use xml.dom.minidom to parse xml, does it support Xinclude?

any one can provide a example?




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


[ovs-dev] RETURNED MAIL: SEE TRANSCRIPT FOR DETAILS

2015-12-09 Thread Bounced mail
The message could not be delivered

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