Re: [ovs-dev] [PATCH v3] netdev-dummy: Limits the number of tx/rx queues.

[Daniele Di Proietto]

2017-01-09 21:56 GMT-08:00 nickcooper-zhangtonghao :
> This patch avoids the ovs_rcu to report WARN, caused by blocked
> for a long time, when ovs-vswitchd processes a port with many
> rx/tx queues. The number of tx/rx queues per port may be appropriate,
> because the dpdk uses it as an default max value.
>
> Signed-off-by: nickcooper-zhangtonghao 

Applied to master, thanks

> ---
> v3:
> * Limits the number of tx/rx queues in set_config().
> * Adds the WARN log when exceeds DUMMY_MAX_QUEUES_PER_PORT.
> ---
>  lib/netdev-dummy.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index bdb77e1..4a23cba 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -827,6 +827,8 @@ netdev_dummy_set_in6(struct netdev *netdev_, struct 
> in6_addr *in6,
>  return 0;
>  }
>
> +#define DUMMY_MAX_QUEUES_PER_PORT 1024
> +
>  static int
>  netdev_dummy_set_config(struct netdev *netdev_, const struct smap *args)
>  {
> @@ -870,6 +872,21 @@ netdev_dummy_set_config(struct netdev *netdev_, const 
> struct smap *args)
>
>  new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1);
>  new_n_txq = MAX(smap_get_int(args, "n_txq", NR_QUEUE), 1);
> +
> +if (new_n_rxq > DUMMY_MAX_QUEUES_PER_PORT ||
> +new_n_txq > DUMMY_MAX_QUEUES_PER_PORT) {
> +VLOG_WARN("The one or both of interface %s queues"
> +  "(rxq: %d, txq: %d) exceed %d. Sets it %d.\n",
> +  netdev->up.name,
> +  new_n_rxq,
> +  new_n_txq,
> +  DUMMY_MAX_QUEUES_PER_PORT,
> +  DUMMY_MAX_QUEUES_PER_PORT);
> +
> +new_n_rxq = MIN(DUMMY_MAX_QUEUES_PER_PORT, new_n_rxq);
> +new_n_txq = MIN(DUMMY_MAX_QUEUES_PER_PORT, new_n_txq);
> +}
> +
>  new_numa_id = smap_get_int(args, "numa_id", 0);
>  if (new_n_rxq != netdev->requested_n_rxq
>  || new_n_txq != netdev->requested_n_txq
> --
> 1.8.3.1
>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Ref:PO71887/Awaiting for draft contract letter/PI

[Firoz-Al Amani Automotech LLC]

Hello dear,
 Happy new year to you and your entire family.
 We attached our first orders for the year of 2017 and you have to make sure 
the items marked on yellow are to be treated urgently and include them with the 
first shipment.
 Please kindly make a draft of the contract letter alongside with the pro-forma 
invoice and send to us for acknowledgement and hopefully there shouldn 19t be a 
delay with the shipping schedules from your country to our country of delivery 
Dubai, U. A. E.
  
 Best regards,
  
 Firoz Ahmed
   
  
 Al Amani Automotech  LLC
P.O. Box 5810, Dubai U. A. E.
Tel:   +9714 2621126 
Fax:  +9714 2666165
Mob: +97150 6567822
www.alamanispareparts.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] intermittent ovs-vswitchd segfault

[Numan Siddique]

On Wed, Jan 11, 2017 at 5:24 AM, Joe Stringer  wrote:

> On 10 January 2017 at 00:25, Numan Siddique  wrote:
> > Hi,
> >
> > I am seeing intermittent segfault's in ovs-vswitchd. We have like 20
> > compute nodes and noticed the crash in 4 or 5 nodes. Seems to me the
> crash
> > is seen when the system is idle for a long time (as I noticed over the
> > weekend)
> >
> > We are using master of ovs (with the latest commit id 92043ab8ffd4)
> >
> > Below is the dmesg and the backtrace of the core file
> >
> > 
> > [297752.801094] revalidator239[2308]: segfault at 0 ip 7f2a8d966da3
> sp
> > 7f2a28ff6c58 error 4 in ovs-vswitchd[7f2a8d8ef000+1ea000]
> > 
> >
> >
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib64/libthread_db.so.1".
> > Core was generated by `ovs-vswitchd unix:/var/run/openvswitch/db.sock
> > -vconsole:emer -vsyslog:err -vfi'.
> > Program terminated with signal 11, Segmentation fault.
> > #0  0x7fe969b13da3 in cmap_replace__ ()
> > Missing separate debuginfos, use: debuginfo-install
> > glibc-2.17-157.el7_3.1.x86_64 keyutils-libs-1.5.8-3.el7.x86_64
> > krb5-libs-1.14.1-27.el7_3.x86_64 libcap-ng-0.7.5-4.el7.x86_64
> > libcom_err-1.42.9-9.el7.x86_64 libselinux-2.5-6.el7.x86_64
> > openssl-libs-1.0.1e-60.el7.x86_64 pcre-8.32-15.el7_2.1.x86_64
> > zlib-1.2.7-17.el7.x86_64
> > (gdb) br
> > Breakpoint 1 at 0x7fe969b13da3
> > (gdb) bt
> > #0  0x7fe969b13da3 in cmap_replace__ ()
> > #1  0x7fe969b14491 in cmap_replace ()
> > #2  0x7fe969aee9ff in ukey_delete ()
> > #3  0x7fe969aefd42 in revalidator_sweep__ ()
> > #4  0x7fe969af1bad in udpif_revalidator ()
> > #5  0x7fe969b8b2a6 in ovsthread_wrapper ()
> > #6  0x7fe968e07dc5 in start_thread () from /lib64/libpthread.so.0
> > #7  0x7fe96862c73d in clone () from /lib64/libc.so.6
> >
> > Thanks
> > Numan
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Hi Numan,
>
> Thanks for the report.
>
> First, a couple of questions:
> * Was there anything relevant in the ovs-vswitchd log?
>

​Couldn't find any. I will report it if i can find anything relevant.
​


> * Can you provide backtraces for the other threads?
>
> ​Please see this paste which has the bt of all threads -
https://paste.fedoraproject.org/525354/
​


> Jarno and I discussed this, and there's some possibility that this
> patch will fix the issue. If you're able to test, please let us know
> whether this makes a difference:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327513.html
>


​Thanks for the patch. I will try to test it out and let you know.

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

Re: [ovs-dev] *** SPAM *** [PATCH] xlate: Recirculate also when MPLS POP is implicit.

[Jarno Rajahalme]

> On Jan 6, 2017, at 9:06 AM, Ben Pfaff  wrote:
> 
> On Thu, Jan 05, 2017 at 06:19:41PM -0800, Jarno Rajahalme wrote:
>> 'ctx->was_mpls' is used to flag when an MPLS packet has been popped to
>> a non-MPLS packet, but it was not set when the MPLS POP is implicit
>> due to the 'ctx->xin->flow' being restored after a patch port
>> traversal to group bucket execution.
>> 
>> If MPLS push on a non-MPLS packet is followed by an MPLS POP in the
>> same actions list, a recirculation after the MPLS POP would not be
>> necessary, as the parsed L3/4 fields are still available.  Even so,
>> the current action validation and execution code in the Linux kernel
>> datapath implementation would need to be enhanced to support this
>> case.  For now we trigger recirculation instead.
>> 
>> Reported-by: Thomas Morin 
>> Suggested-by: Takashi YAMAMOTO 
>> Fixes: 742c0ac3c0ab ("mpls: Fix MPLS restoration after patch port and group 
>> bucket.")
>> Signed-off-by: Jarno Rajahalme 
> 
> This is complicated.
> 
> This is a bug fix, I guess.  Is this anything we can test?
> 

Testing was there, but surprisingly did not need a change, so the patch did not 
have the desired effect. Sorry for the churn, will send a new version once the 
test cases actually work as they should.

  Jarno


> Should this be implemented as a helper function?
> 

Will do.

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

Re: [ovs-dev] [PATCH] python/setup: "-fPIC" extra compile flag for C extension

[Iwase Yusuke]

Hi,

On 2017年01月11日 11:01, Ben Pfaff wrote:
> On Wed, Jan 11, 2017 at 09:38:13AM +0900, IWASE Yusuke wrote:
>> To build shared library object, it is required to built as "Position
>> Independent Code" (on x86-64, for example).
>> This patch adds "-fPIC" extra compile flag to make sure that extension
>> being PIC while building a shared library object.
>>
>> Signed-off-by: IWASE Yusuke 
>> ---
>>  python/setup.py | 9 +++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/python/setup.py b/python/setup.py
>> index 19c1f18..8f565d8 100644
>> --- a/python/setup.py
>> +++ b/python/setup.py
>> @@ -76,8 +76,13 @@ setup_args = dict(
>>  'Programming Language :: Python :: 3',
>>  'Programming Language :: Python :: 3.4',
>>  ],
>> -ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
>> -  libraries=['openvswitch'])],
>> +ext_modules=[
>> +setuptools.Extension(
>> +"ovs._json",
>> +sources=["ovs/_json.c"],
>> +libraries=['openvswitch'],
>> +extra_compile_args=["-fPIC"]),
>> +],
>>  cmdclass={'build_ext': try_build_ext},
>>  )
> 
> Are you sure that this is the right way to build a Python shared object?
> It is very ELF-specific so I imagine that it will break Windows and BSD
> builds.

Sorry, I'm not good at C cross-platform building.
I didn't test on such environment, tried it on only Ubuntu 16.04.
When I built Python shared object, I need to add "-fPIC" to extension and
also to openvswitch.a library, otherwise the followings would be output.
===
/usr/bin/ld: //usr/local/lib/libopenvswitch.a(json.o): relocation R_X86_64_32 
against `.rodata.str1.1' can not be used when making a shared object; recompile 
with -fPIC
===

I only know "shared object should be PIC", but don't understand
enough why...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] python/setup: "-fPIC" extra compile flag for C extension

[Ben Pfaff]

On Wed, Jan 11, 2017 at 09:38:13AM +0900, IWASE Yusuke wrote:
> To build shared library object, it is required to built as "Position
> Independent Code" (on x86-64, for example).
> This patch adds "-fPIC" extra compile flag to make sure that extension
> being PIC while building a shared library object.
> 
> Signed-off-by: IWASE Yusuke 
> ---
>  python/setup.py | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/python/setup.py b/python/setup.py
> index 19c1f18..8f565d8 100644
> --- a/python/setup.py
> +++ b/python/setup.py
> @@ -76,8 +76,13 @@ setup_args = dict(
>  'Programming Language :: Python :: 3',
>  'Programming Language :: Python :: 3.4',
>  ],
> -ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
> -  libraries=['openvswitch'])],
> +ext_modules=[
> +setuptools.Extension(
> +"ovs._json",
> +sources=["ovs/_json.c"],
> +libraries=['openvswitch'],
> +extra_compile_args=["-fPIC"]),
> +],
>  cmdclass={'build_ext': try_build_ext},
>  )

Are you sure that this is the right way to build a Python shared object?
It is very ELF-specific so I imagine that it will break Windows and BSD
builds.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] (no subject)

[loans]

Brauchen Sie ein Geschäft oder persönliches Darlehen. Wir geben Darlehen im 
Bereich von € 1.000 bis € 100.000.000 EURO, KONTAKTIEREN SIE UNS JETZT ÜBER 
EMAIL: jerryloa...@gmail.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] dpcls: Avoid one 8-byte chunk in subtable mask.

[Jarno Rajahalme]

Thanks for the update! Pushed to master with small edits for clarification.

  Jarno


> On Jan 6, 2017, at 1:45 AM, antonio.fische...@intel.com wrote:
> 
> This patch allows to skip the chunk comprising of dp_hash and in_port
> in the subtable mask when the packet is not recirculated.  This will
> slightly speed up the hash computation as one expensive function call
> to hash_add64() can be skipped.
> For each new netdev flow we wildcard in_port in the mask, so in the
> physical case where dp_hash is also wildcarded, the resulting 8-byte
> chunk will not be part of the subtable mask.
> 
> The idea behind is that all the packets within a given dpcls will
> have the same in_port value and typically dp_hash == 0.  So they will
> have the same 8-bytes chunk in their {dp_hash, in_port} portion of the
> miniflow. This doesn't contribute effectively in spreading hash values
> and avoiding collisions.
> 
> v2: Using ovs_assert.
> v3: Fix for dumped flows missing the in_port match.
> 
> Signed-off-by: Antonio Fischetti 
> Signed-off-by: Bhanuprakash Bodireddy 
> Co-authored-by: Bhanuprakash Bodireddy 
> Signed-off-by: Jarno Rajahalme 
> Co-authored-by: Jarno Rajahalme 
> ---
> lib/dpif-netdev.c | 13 +
> 1 file changed, 13 insertions(+)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 1a47a45..4425f85 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2120,6 +2120,9 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
> *netdev_flow,
> };
> 
> miniflow_expand(_flow->cr.mask->mf, );
> +/* in_port is exact matched, but we have left it out from the mask 
> for
> + * optimization reasons.  Add in_port back to the mask. */
> +wc.masks.in_port.odp_port = ODPP_NONE;
> 
> /* Key */
> offset = key_buf->size;
> @@ -2278,7 +2281,17 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
> struct dpcls *cls;
> odp_port_t in_port = match->flow.in_port.odp_port;
> 
> +/* As we select the dpcls based on the port number, each netdev flow
> + * belonging to the same dpcls will have the same odp_port value.
> + * For performance reasons here we wildcard odp_port in the mask.  In the
> + * physical case where dp_hash is also wildcarded, the resulting 8-byte
> + * chunk {dp_hash, in_port} will be ignored by netdev_flow_mask_init() 
> and
> + * will not be part of the subtable mask.
> + * This will speed up the hash computation during dpcls_lookup() because
> + * one call to hash_add64() will be skipped. */
> +match->wc.masks.in_port.odp_port = 0;
> netdev_flow_mask_init(, match);
> +match->wc.masks.in_port.odp_port = ODPP_NONE;
> /* Make sure wc does not have metadata. */
> ovs_assert(!FLOWMAP_HAS_FIELD(, metadata)
>&& !FLOWMAP_HAS_FIELD(, regs));
> -- 
> 2.4.11
> 

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

[ovs-dev] [PATCH] python/setup: "-fPIC" extra compile flag for C extension

[IWASE Yusuke]

To build shared library object, it is required to built as "Position
Independent Code" (on x86-64, for example).
This patch adds "-fPIC" extra compile flag to make sure that extension
being PIC while building a shared library object.

Signed-off-by: IWASE Yusuke 
---
 python/setup.py | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/python/setup.py b/python/setup.py
index 19c1f18..8f565d8 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -76,8 +76,13 @@ setup_args = dict(
 'Programming Language :: Python :: 3',
 'Programming Language :: Python :: 3.4',
 ],
-ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"],
-  libraries=['openvswitch'])],
+ext_modules=[
+setuptools.Extension(
+"ovs._json",
+sources=["ovs/_json.c"],
+libraries=['openvswitch'],
+extra_compile_args=["-fPIC"]),
+],
 cmdclass={'build_ext': try_build_ext},
 )
 
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 2/2] revalidator: Complain for more ukey transitions.

[Jarno Rajahalme]

In the worst case this patch will introduce rate-limited logging for other 
states than UKEY_OPERATIONAL that may want to transition onto themselves, so:

Acked-by: Jarno Rajahalme 

> On Jan 10, 2017, at 3:54 PM, Joe Stringer  wrote:
> 
> For most ukey transition states, only one thread should be responsible
> for transitioning the ukey into the new state. If another thread
> attempts to transition the ukey into the same state (for instance,
> evicting the datapath flow or deleting the ukey), then it is likely
> performing additional work which should only happen once. Log all cases
> of ukey transition into the current state, except for UKEY_OPERATIONAL
> -> UKEY_OPERATIONAL which regularly occurs when revalidating ukeys.
> 
> Signed-off-by: Joe Stringer 
> ---
> ofproto/ofproto-dpif-upcall.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 1ffeaabf7d8e..660383faee1c 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1672,7 +1672,7 @@ transition_ukey(struct udpif_key *ukey, enum ukey_state 
> dst)
> OVS_REQUIRES(ukey->mutex)
> {
> ovs_assert(dst >= ukey->state);
> -if (ukey->state == dst) {
> +if (ukey->state == dst && dst == UKEY_OPERATIONAL) {
> return;
> }
> 
> -- 
> 2.10.2
> 

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

Re: [ovs-dev] [PATCH 1/2] revalidator: Prevent double-delete of ukey.

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme 

> On Jan 10, 2017, at 3:54 PM, Joe Stringer  wrote:
> 
> revalidator_sweep__() splits checking for whether to delete a ukey from
> the actual deletion to prevent taking the umap lock for too long.
> However it uses information gathered from the first critical section to
> decide to call ukey_delete() - ie, the second critical section.
> 
> Since 67f08985d769 ("upcall: Replace ukeys for deleted flows."), it is
> possible for a handler thread to receive an upcall for the same flow and
> to replace the ukey which is being deleted with a new one, in between
> these critical sections. This will remove the ukey from the cmap,
> rcu-defer its deletion, and update the existing ukey.
> 
> If this occurs in between the critical sections of revalidator cleanup
> of the flow, then the revalidator will subsequently call ukey_delete()
> to delete the original ukey, which was already deleted by the handler
> thread.
> 
> Guard against this by checking the ukey state in ukey_delete().
> 
> Backtrace:
>Program terminated with signal 11, Segmentation fault.
>#0  0x7fe969b13da3 in cmap_replace__ ()
>#1  0x7fe969b14491 in cmap_replace ()
>#2  0x7fe969aee9ff in ukey_delete ()
>#3  0x7fe969aefd42 in revalidator_sweep__ ()
>#4  0x7fe969af1bad in udpif_revalidator ()
>#5  0x7fe969b8b2a6 in ovsthread_wrapper ()
>#6  0x7fe968e07dc5 in start_thread () from /lib64/libpthread.so.0
>#7  0x7fe96862c73d in clone () from /lib64/libc.so.6
> 
> Fixes: 54ebeff4c03d ("upcall: Track ukey states.")
> Fixes: 67f08985d769 ("upcall: Replace ukeys for deleted flows.")
> Reported-by: Numan Siddique 
> Signed-off-by: Joe Stringer 
> ---
> ofproto/ofproto-dpif-upcall.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index cd2445fc4ab9..1ffeaabf7d8e 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1794,9 +1794,11 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey)
> OVS_REQUIRES(umap->mutex)
> {
> ovs_mutex_lock(>mutex);
> -cmap_remove(>cmap, >cmap_node, ukey->hash);
> -ovsrcu_postpone(ukey_delete__, ukey);
> -transition_ukey(ukey, UKEY_DELETED);
> +if (ukey->state < UKEY_DELETED) {
> +cmap_remove(>cmap, >cmap_node, ukey->hash);
> +ovsrcu_postpone(ukey_delete__, ukey);
> +transition_ukey(ukey, UKEY_DELETED);
> +}
> ovs_mutex_unlock(>mutex);
> }
> 
> -- 
> 2.10.2
> 

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

Re: [ovs-dev] intermittent ovs-vswitchd segfault

[Joe Stringer]

On 10 January 2017 at 00:25, Numan Siddique  wrote:
> Hi,
>
> I am seeing intermittent segfault's in ovs-vswitchd. We have like 20
> compute nodes and noticed the crash in 4 or 5 nodes. Seems to me the crash
> is seen when the system is idle for a long time (as I noticed over the
> weekend)
>
> We are using master of ovs (with the latest commit id 92043ab8ffd4)
>
> Below is the dmesg and the backtrace of the core file
>
> 
> [297752.801094] revalidator239[2308]: segfault at 0 ip 7f2a8d966da3 sp
> 7f2a28ff6c58 error 4 in ovs-vswitchd[7f2a8d8ef000+1ea000]
> 
>
>
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> Core was generated by `ovs-vswitchd unix:/var/run/openvswitch/db.sock
> -vconsole:emer -vsyslog:err -vfi'.
> Program terminated with signal 11, Segmentation fault.
> #0  0x7fe969b13da3 in cmap_replace__ ()
> Missing separate debuginfos, use: debuginfo-install
> glibc-2.17-157.el7_3.1.x86_64 keyutils-libs-1.5.8-3.el7.x86_64
> krb5-libs-1.14.1-27.el7_3.x86_64 libcap-ng-0.7.5-4.el7.x86_64
> libcom_err-1.42.9-9.el7.x86_64 libselinux-2.5-6.el7.x86_64
> openssl-libs-1.0.1e-60.el7.x86_64 pcre-8.32-15.el7_2.1.x86_64
> zlib-1.2.7-17.el7.x86_64
> (gdb) br
> Breakpoint 1 at 0x7fe969b13da3
> (gdb) bt
> #0  0x7fe969b13da3 in cmap_replace__ ()
> #1  0x7fe969b14491 in cmap_replace ()
> #2  0x7fe969aee9ff in ukey_delete ()
> #3  0x7fe969aefd42 in revalidator_sweep__ ()
> #4  0x7fe969af1bad in udpif_revalidator ()
> #5  0x7fe969b8b2a6 in ovsthread_wrapper ()
> #6  0x7fe968e07dc5 in start_thread () from /lib64/libpthread.so.0
> #7  0x7fe96862c73d in clone () from /lib64/libc.so.6
>
> Thanks
> Numan
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Hi Numan,

Thanks for the report.

First, a couple of questions:
* Was there anything relevant in the ovs-vswitchd log?
* Can you provide backtraces for the other threads?

Jarno and I discussed this, and there's some possibility that this
patch will fix the issue. If you're able to test, please let us know
whether this makes a difference:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327513.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Windows: Implement Hyper-V VIF discovery agent.

[Yin Lin]

Signed-off-by: Yin Lin 
---
 windows/OvsDiscoveryAgent/App.config   |  18 ++
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj |  83 
 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln|  34 +++
 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs   |  67 ++
 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs  |  73 +++
 windows/OvsDiscoveryAgent/OvsVsctl.cs  |  84 
 windows/OvsDiscoveryAgent/Program.cs   |  26 +++
 .../OvsDiscoveryAgent/Properties/AssemblyInfo.cs   |  36 
 .../Properties/Settings.Designer.cs|  38 
 .../OvsDiscoveryAgent/Properties/Settings.settings |   9 +
 windows/OvsDiscoveryAgent/VirtualAdapter.cs|  36 
 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs | 228 +
 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs | 107 ++
 windows/OvsDiscoveryAgent/WmiMonitor.cs| 193 +
 windows/OvsDiscoveryAgent/WqlCondition.cs  | 168 +++
 windows/OvsDiscoveryAgent/WqlHelper.cs | 132 
 windows/OvsDiscoveryAgent/WqlObject.cs | 133 
 windows/automake.mk|  21 ++
 18 files changed, 1486 insertions(+)
 create mode 100644 windows/OvsDiscoveryAgent/App.config
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
 create mode 100644 windows/OvsDiscoveryAgent/OvsDiscoveryService.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsSwitchMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/OvsVsctl.cs
 create mode 100644 windows/OvsDiscoveryAgent/Program.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/AssemblyInfo.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.Designer.cs
 create mode 100644 windows/OvsDiscoveryAgent/Properties/Settings.settings
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapter.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterManager.cs
 create mode 100644 windows/OvsDiscoveryAgent/VirtualAdapterMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WmiMonitor.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlCondition.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlHelper.cs
 create mode 100644 windows/OvsDiscoveryAgent/WqlObject.cs

diff --git a/windows/OvsDiscoveryAgent/App.config 
b/windows/OvsDiscoveryAgent/App.config
new file mode 100644
index 000..f2c7a15
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/App.config
@@ -0,0 +1,18 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+ovs-int
+
+
+
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj 
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
new file mode 100644
index 000..ee709d3
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.csproj
@@ -0,0 +1,83 @@
+
+http://schemas.microsoft.com/developer/msbuild/2003;>
+  
+  
+Debug
+AnyCPU
+{2563C1BE-B240-4F63-84C5-01D98D015A3F}
+Exe
+Properties
+OvsDiscoveryAgent
+OvsDiscoveryAgent
+v4.5
+512
+true
+  
+  
+AnyCPU
+true
+full
+false
+bin\Debug\
+DEBUG;TRACE
+prompt
+4
+  
+  
+AnyCPU
+pdbonly
+true
+bin\Release\
+TRACE
+prompt
+4
+  
+  
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  True
+  True
+  Settings.settings
+
+
+
+  Component
+
+
+
+
+
+
+
+
+
+
+  
+  
+
+
+  SettingsSingleFileGenerator
+  Settings.Designer.cs
+
+  
+  
+  
+
\ No newline at end of file
diff --git a/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln 
b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
new file mode 100644
index 000..2fe2c81
--- /dev/null
+++ b/windows/OvsDiscoveryAgent/OvsDiscoveryAgent.sln
@@ -0,0 +1,34 @@
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio 14
+VisualStudioVersion = 14.0.25123.0
+MinimumVisualStudioVersion = 10.0.40219.1
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "OvsDiscoveryAgent", 
"OvsDiscoveryAgent.csproj", "{2563C1BE-B240-4F63-84C5-01D98D015A3F}"
+EndProject
+Global
+   GlobalSection(SolutionConfigurationPlatforms) = preSolution
+   Debug|Any CPU = Debug|Any CPU
+   Debug|x64 = Debug|x64
+   Debug|x86 = Debug|x86
+   Release|Any CPU = Release|Any CPU
+   Release|x64 = Release|x64
+   Release|x86 = Release|x86
+   EndGlobalSection
+   GlobalSection(ProjectConfigurationPlatforms) = postSolution
+   {2563C1BE-B240-4F63-84C5-01D98D015A3F}.Debug|Any CPU.ActiveCfg 
= Debug|Any CPU
+   {2563C1BE-B240-4F63-84C5-01D98D015A3F}.Debug|Any CPU.Build.0 = 

Re: [ovs-dev] [PATCH] ovn: Support ARP proxy in logical switches.

[Bruce Davie]

Han,
 Thanks - that makes sense to me. I can’t claim that this is a hugely general 
feature, but your use case seems legit to me (with the caveat that I’m no k8s 
expert).

Bruce

On Jan 9, 2017, at 4:08 PM, Han Zhou 
> wrote:

Hi Bruce,

This feature is useful for me. I had the concern because the use case for me is 
intermediate. It is for k8s integration. In k8s there is a kubeproxy running on 
each host to do service-ip NATting, and I am using OVS (programmed by OVN) to 
connect host network namespace to containers (and also connection between 
containers on the same host). When an NATted end-point of a service-ip happen 
to be on the same host (and of course, same L2, since we use bridged mode), 
then the return traffic would be forwarded directly to the source without going 
through the kubeproxy. With this ARP-proxy patch, the interface to the host 
network namespace is specified as ARP proxy port, and traffic between 
containers will go through the host for NATting.

This is an intermediate solution because a better way to do it is to utilize 
the load-balancing feature of OVN to replace kubeproxy completely, and the 
problem won't exist at all. It just takes more effort to integrate and we are 
not there yet.

Hope this clarifies my use case. But I'd like to hear if this feature would 
useful in any other circumstances.

Thanks,
Han

On Mon, Jan 9, 2017 at 2:57 PM, Bruce Davie 
> wrote:
>
> Han,
> Your comment gave me pause:
> > I have similar concerns about how useful it is.
>
> Whereas the current proxy ARP function in OVN has a pretty clear motivation & 
> tightly defined use case (to avoid needless broadcast of ARP requests across 
> the overlay when the logical router’s IP and MAC are known) it seems like 
> even you don’t really have a clear use case in mind for this additional 
> functionality. Can you try to lay our more clearly why you think this is a 
> useful enough addition to OVN to warrant the extra complexity?
>
> Thanks
> Bruce
>

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

Re: [ovs-dev] [PATCH ovs V2 12/21] dpif-netlink: Use netdev flow put api to insert a flow

[Joe Stringer]

On 10 January 2017 at 07:50, Paul Blakey  wrote:
>>> +nl_msg_put_u32(, OVS_ACTION_ATTR_OUTPUT, ifindex_out);
>>> +if (tnl_cfg && tnl_cfg->dst_port != 0) {
>>> +nl_msg_put_u32(, OVS_TUNNEL_KEY_ATTR_TP_DST,
>>> tnl_cfg->dst_port);
>>> +}
>>> +nl_msg_end_nested(, out_off);
>>> +
>>> +if (outdev) {
>>> +netdev_close(outdev);
>>> +}
>>> +} else {
>>> +nl_msg_put_unspec(, nl_attr_type(nla), nl_attr_get(nla),
>>> +  nl_attr_get_size(nla));
>>> +}
>>> +}
>>> +nl_msg_end_nested(, offset);
>>
>> Hmm. I'm a bit uneasy about this whole copying/rewriting of the
>> actions. Firstly, the tunnel stuff just looks wrong. A quick "git
>> grep" shows that OVS_TUNNEL_KEY_ATTR_TP_DST is using the wrong type,
>> and looking closer at the other places it is used, it looks like this
>> function is shortcutting a whole bunch of the tunnel config.
>
> Besides the wrong type, I'm not sure what you mean, dpif_netlink_port_add__
> seems to access the tunnel dst port the same way.

Yes, data type, bitwise differences compared to
dpif_netlink_port_add__(). Checking the ifindex.

Does tnl_cfg->exts need to be checked too? Are any of the other
'struct netdev_tunnel_config' fields relevant for output (or will
ignoring them potentially lead to a correctness issue)?

>>
>>
>> I recognise that the ODP port number needs to be translated into an
>> ifindex. Do we need to do anything else?
>
> tunnel dst port for set action was missing in ACTION_SET ,  TUNNEL pair.
> We needed that for offloading.

Broadly I was thinking that any and all fields that may be relevant to
tunnel outputting should be handled and checked - fail out if it can't
be interpreted or implemented on lower layers. Looking at 'enum
ovs_tunnel_key_attr', there's a whole bunch of fields which are
similar to OVS_TUNNEL_KEY_ATTR_TP_DST and it wasn't immediately
obvious to me why this function only serialises this one member of the
enum and doesn't need to handle any of the rest.

If I follow correctly, the answer is that for traditional OVS vports,
we configure the vport in kernel to include TP_DST (and a few other
things), then omit these from the flow. When the datapath performs
output action, it will fetch this information from the vport in
kernel. However, for the TC hardware offloads case there is no such
port configuration in kernel, so now we need to ensure that it is
encoded in the flow. Is that right?

>>> +
>>> +if (outputs > 1) {
>>> +return false;
>>> +}
>>> +
>>> +act = ofpbuf_at_assert(, offset, sizeof(struct nlattr));
>>> +dev = netdev_hmap_port_get(in_port, dpif->dpif.dpif_class);
>>> +err = netdev_flow_put(dev, , CONST_CAST(struct nlattr *,
>>> +  nl_attr_get(act)),
>>> +  nl_attr_get_size(act), put->stats,
>>> +  CONST_CAST(ovs_u128 *, put->ufid));
>>> +netdev_close(dev);
>>> +
>>> +if (!err) {
>>> +if (put->flags & DPIF_FP_MODIFY) {
>>> +struct dpif_op *opp;
>>> +struct dpif_op op;
>>> +
>>> +op.type = DPIF_OP_FLOW_DEL;
>>> +op.u.flow_del.key = put->key;
>>> +op.u.flow_del.key_len = put->key_len;
>>> +op.u.flow_del.ufid = put->ufid;
>>> +op.u.flow_del.pmd_id = put->pmd_id;
>>> +op.u.flow_del.stats = NULL;
>>> +op.u.flow_del.terse = false;
>>> +
>>> +opp = 
>>> +dpif_netlink_operate__(dpif, , 1);
>>
>> Won't this just delete the flow that was just added?
>
> The above FLOW_DEL calls the original operate (without offload) to delete it
> if it from netlink kernel datapath. This will happen if we got a modify
> request and we now
> can offload the flow and previously couldn't (so its in kernel datapath).

Ah, I missed that it was the "dpif_netlink_operate__" variation. OK.

>>
>> Is there a modify at the tc layer?
>
> Yes there is, and implemented by given a already used handle.
> So if we find the ufid in the map (ufid to handle/prio) already, we modify
> it by using
> the same handle.

OK, this seems fine at a glance.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V2 12/21] dpif-netlink: Use netdev flow put api to insert a flow

[Joe Stringer]

On 10 January 2017 at 06:36, Paul Blakey  wrote:
>
>
> On 06/01/2017 01:28, Joe Stringer wrote:
>>
>> On 25 December 2016 at 03:39, Paul Blakey  wrote:
>>>
>>> Using the new netdev flow api operate will now try and
>>> offload flows to the relevant netdev of the input port.
>>> Other operate methods flows will come in later patches.
>>>
>>> Signed-off-by: Paul Blakey 
>>> Reviewed-by: Roi Dayan 
>>> ---
>>>   lib/dpif-netlink.c | 232
>>> -
>>>   1 file changed, 228 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>>> index 3d8940e..717af90 100644
>>> --- a/lib/dpif-netlink.c
>>> +++ b/lib/dpif-netlink.c
>>> @@ -1908,15 +1908,239 @@ dpif_netlink_operate__(struct dpif_netlink
>>> *dpif,
>>>   return n_ops;
>>>   }
>>>
>>> +static int
>>> +parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
>>> +const struct nlattr *mask, size_t mask_len,
>>> +struct match *match)
>>> +{
>>> +enum odp_key_fitness fitness;
>>> +
>>> +fitness = odp_flow_key_to_flow(key, key_len, >flow);
>>> +if (fitness) {
>>> +/* This should not happen: it indicates that
>>> odp_flow_key_from_flow()
>>> + * and odp_flow_key_to_flow() disagree on the acceptable form of
>>> a
>>> + * flow.  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()) {
>>> +struct ds s;
>>> +
>>> +ds_init();
>>> +odp_flow_format(key, key_len, NULL, 0, NULL, , true);
>>> +VLOG_ERR("internal error parsing flow key %s", ds_cstr());
>>> +ds_destroy();
>>> +}
>>> +
>>> +return EINVAL;
>>> +}
>>> +
>>> +fitness = odp_flow_key_to_mask(mask, mask_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()) {
>>> +struct ds s;
>>> +
>>> +VLOG_ERR("internal error parsing flow mask %s (%s)",
>>> + ds_cstr(), odp_key_fitness_to_string(fitness));
>>> +ds_destroy();
>>> +}
>>> +
>>> +return EINVAL;
>>> +}
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static bool
>>> +parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
>>> +{
>>> +struct match match;
>>> +odp_port_t in_port;
>>> +const struct nlattr *nla;
>>> +size_t left;
>>> +int outputs = 0;
>>> +struct ofpbuf buf;
>>> +uint64_t act_stub[1024 / 8];
>>> +size_t offset;
>>> +struct nlattr *act;
>>> +struct netdev *dev;
>>> +int err;
>>> +
>>> +/* 0x1234 - fake eth type sent to probe feature */
>>> +if (put->flags & DPIF_FP_PROBE || match.flow.dl_type ==
>>> htons(0x1234)) {
>>> +return false;
>>> +}
>>> +
>>> +if (parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
>>> +put->mask_len, )) {
>>> +return false;
>>> +}
>>> +
>>> +in_port = match.flow.in_port.odp_port;
>>> +ofpbuf_use_stub(, act_stub, sizeof act_stub);
>>> +offset = nl_msg_start_nested(, OVS_FLOW_ATTR_ACTIONS);
>>> +NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) {
>>> +if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
>>> +struct netdev *outdev;
>>> +int ifindex_out;
>>> +const struct netdev_tunnel_config *tnl_cfg;
>>> +size_t out_off;
>>> +odp_port_t out_port;
>>> +
>>> +outputs++;
>>> +if (outputs > 1) {
>>> +break;
>>> +}
>>> +
>>> +out_port = nl_attr_get_u32(nla);
>>> +outdev = netdev_hmap_port_get(out_port,
>>> dpif->dpif.dpif_class);
>>> +tnl_cfg = netdev_get_tunnel_config(outdev);
>>> +
>>> +out_off = nl_msg_start_nested(, OVS_ACTION_ATTR_OUTPUT);
>>> +ifindex_out = netdev_get_ifindex(outdev);
>>> +nl_msg_put_u32(, OVS_ACTION_ATTR_OUTPUT, ifindex_out);
>>> +if (tnl_cfg && tnl_cfg->dst_port != 0) {
>>> +nl_msg_put_u32(, OVS_TUNNEL_KEY_ATTR_TP_DST,
>>> tnl_cfg->dst_port);
>>> +}
>>> +nl_msg_end_nested(, out_off);
>>> +
>>> +if (outdev) {
>>> +netdev_close(outdev);
>>> +}
>>> +} else {
>>> +nl_msg_put_unspec(, nl_attr_type(nla), 

Re: [ovs-dev] [PATCH ovs V2 09/21] dpif-netlink: Dump netdevs flows on flow dump

[Joe Stringer]

On 10 January 2017 at 03:45, Paul Blakey  wrote:
>>> +struct netdev_list_element *element;
>>> +struct ovs_list port_list;
>>> +int ports = netdev_hmap_port_get_list(dpif_->dpif_class,
>>> _list);
>>> +int i = 0;
>>> +
>>> +dump->netdev_dumps =
>>> +ports ? xzalloc(sizeof(struct netdev_flow_dump *) * ports) : 0;
>>
>> Can this be sizeof(dump->netdev_dumps)?
>
> Do you mean sizeof(*dump-netdev_dumps), or sizeof(dump->netdev_dumps[0]), if
> so yes.

Yes that's what I meant.



>>> @@ -1466,6 +1521,90 @@ dpif_netlink_flow_to_dpif_flow(struct dpif *dpif,
>>> struct dpif_flow *dpif_flow,
>>>   dpif_netlink_flow_get_stats(datapath_flow, _flow->stats);
>>>   }
>>>
>>> +static void
>>> +dpif_netlink_advance_netdev_dump(struct dpif_netlink_flow_dump_thread
>>> *thread)
>>> +{
>>> +struct dpif_netlink_flow_dump *dump = thread->dump;
>>> +
>>> +ovs_mutex_lock(>netdev_lock);
>>> +/* if we haven't finished (dumped everything) */
>>> +if (dump->netdev_given < dump->netdev_num) {
>>> +/* if we are the first to find that given dump is finished
>>> + * (for race condition, e.g 3 finish dump 0 at the same time) */
>>
>> Why is there a race condition here if this is executed under netdev_lock?
>
> The design is such that all threads are working together on the first dump
> to the last, in order. (at first they all on dump 0),
> and when one thread finds that the given dump is finished, they all move to
> the next.
> As the comment tried to explain, if 3 (or 2+) threads are working on the
> first dump, dump 0,
> (thread->netdev_cur_dump == 0) and finish at the same time, they all call
> advance func.
> Now the first one to get the lock advances the shared given dump, which
> signify which highest dump we have given
> (and all lower dumps have finished). The rest now enter and we check if the
> dump they have found to be
> finished is higher then the new one that was given, if not they catch up, so
> now all of them will work on dump 1.
>
> The race is that if 2 or more threads worked on the same dump and finished
> at the same time,
> if we just increased netdev_given without checking (thread->cur == given)
> for both of them,
> we would have increased given twice and skip one dump.

Thanks for the explanation. I think that the code would benefit from
having this written somewhere - for instance, above the
dpif_netlink_advance_netdev_dump() function.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb-data: Add support for integer ranges in database commands

[Łukasz Rząsik]

Thanks!

Please let me know if you think I can help implementing something or be
useful in some other way.

I'll also try to propose something.

2017-01-05 17:48 GMT+01:00 Ben Pfaff :

> On Thu, Dec 29, 2016 at 03:55:46PM -0700, Lukasz Rzasik wrote:
> > Adding / removing a range of integers to a column accepting a set of
> > integers requires enumarating all of the integers. This patch simplifies
> > it by introducing 'range' concept to the database commands. Two integers
> > separated by a hyphen represent an inclusive range.
> >
> > The patch adds positive and negative tests for the new syntax.
> > The patch was tested by 'make check'. Covarage was tested by
> > 'make check-lcov'.
> >
> > Signed-off-by: Lukasz Rzasik 
> > Suggested-by: 
> > Suggested-by: Ben Pfaff 
>
> Thanks.  I applied this to master.
>
> I folded in the following incremental, which is mostly stylistic.
>
> --8<--cut here-->8--
>
> diff --git a/NEWS b/NEWS
> index beee6cc..1b0aa5d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -59,6 +59,9 @@ Post-v2.6.0
>   * Ports now have a "protected" flag. Protected ports can not forward
> frames to other protected ports. Unprotected ports can receive and
> forward frames to protected and other unprotected ports.
> +   - ovs-vsctl, ovn-nbctl, ovn-sbctl, vtep-ctl:
> + * Database commands now accept integer ranges, e.g. "set port
> +   eth0 trunks=1-10" to enable trunking VLANs 1 to 10.
>
>  v2.6.0 - 27 Sep 2016
>  -
> diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man
> index 26828d6..88529b5 100644
> --- a/lib/db-ctl-base.man
> +++ b/lib/db-ctl-base.man
> @@ -31,7 +31,7 @@ columns can have an empty set of values, represented as
> \fB[]\fR, and
>  square brackets may optionally enclose other non-empty sets or single
>  values as well. For a column accepting a set of integers, database
> commands
>  accept a range. A range is represented by two integers separated by
> -\fB-\fR. A range is inclusive. A range have a maximum size of 4096
> +\fB-\fR. A range is inclusive. A range has a maximum size of 4096
>  elements. If more elements are needed, they can be specified in seperate
>  ranges.
>  .PP
> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
> index a7a6eef..840e18f 100644
> --- a/lib/ovsdb-data.c
> +++ b/lib/ovsdb-data.c
> @@ -501,9 +501,9 @@ ovsdb_atom_from_string__(union ovsdb_atom *atom,
>  long long int integer, end;
>  if (range_end_atom
>  && str_to_llong_range(s, 10, , )) {
> -if (!ovsdb_atom_range_check(integer, end)) {
> +if (end < integer) {
>  return xasprintf("\"%s\" is not a valid range. "
> -"Range start has to be less than end.", s);
> +"Range end cannot be before start.", s);
>  }
>  *range_end_atom = alloc_default_atoms(type, 1);
>  if (!(*range_end_atom)) {
> @@ -638,24 +638,22 @@ ovsdb_atom_from_string(union ovsdb_atom *atom,
>
>  if (!error && range_end_atom && *range_end_atom) {
>  /* Check range constraints */
> +int64_t start = atom->integer;
> +int64_t end = (*range_end_atom)->integer;
>  if (base->enum_) {
> -/* If enum, every atom in range has to be checked */
> -union ovsdb_atom ai = *atom;
> -++ai.integer;
> -while (!error) {
> +for (int64_t i = start + 1; i <= end; i++) {
> +union ovsdb_atom ai = { .integer = i };
>  error = ovsdb_atom_check_constraints(, base);
> -if (ai.integer == (*range_end_atom)->integer) {
> +if (error) {
>  break;
>  }
> -++ai.integer;
>  }
>  } else {
>  error = ovsdb_atom_check_constraints(*range_end_atom, base);
>  }
>
>  if (!error) {
> -error = ovsdb_atom_range_check_size(atom->integer,
> -
> (*range_end_atom)->integer);
> +error = ovsdb_atom_range_check_size(start, end);
>  }
>  }
>
> @@ -1857,11 +1855,9 @@ ovsdb_datum_add_unsafe(struct ovsdb_datum *datum,
>  datum->n += range_end_atom ?
>  (range_end_atom->integer - key->integer + 1) : 1;
>  datum->keys = xrealloc(datum->keys, datum->n * sizeof *datum->keys);
> -if (range_end_atom
> -&& ovsdb_atom_range_check(key->integer,
> range_end_atom->integer)) {
> -union ovsdb_atom ai;
> -for (ai = *key; ai.integer <= range_end_atom->integer;
> ++ai.integer) {
> -ovsdb_atom_clone(>keys[idx++], , type->key.type);
> +if (range_end_atom && key->integer <= range_end_atom->integer) {
> +for (int64_t i = key->integer; i <= range_end_atom->integer; i++)
> {
> +datum->keys[idx++].integer = i;
>  }
>  } else 

Re: [ovs-dev] [PATCH ovs V2 05/21] dpif: Save added ports in a port map for netdev flow api use

[Joe Stringer]

On 10 January 2017 at 03:20, Paul Blakey  wrote:
>
>
> On 05/01/2017 03:54, Joe Stringer wrote:
>>
>> On 25 December 2016 at 03:39, Paul Blakey  wrote:
>>>
>>> To use netdev flow offloading api, dpifs needs to iterate over
>>> added ports. This addition inserts the added dpif ports in a hash map,
>>> The map will also be used to translate dpif ports to netdevs.
>>>
>>> Signed-off-by: Paul Blakey 
>>> Reviewed-by: Roi Dayan 
>>> ---
>>>   lib/dpif.c   |  25 +++
>>>   lib/netdev.c | 133
>>> +++
>>>   lib/netdev.h |  14 +++
>>>   3 files changed, 172 insertions(+)
>>>
>>> diff --git a/lib/dpif.c b/lib/dpif.c
>>> index 53958c5..c67ea92 100644
>>> --- a/lib/dpif.c
>>> +++ b/lib/dpif.c
>>> @@ -352,7 +352,22 @@ do_open(const char *name, const char *type, bool
>>> create, struct dpif **dpifp)
>>>   error =
>>> registered_class->dpif_class->open(registered_class->dpif_class,
>>>  name, create, );
>>>   if (!error) {
>>> +struct dpif_port_dump port_dump;
>>> +struct dpif_port dpif_port;
>>> +
>>>   ovs_assert(dpif->dpif_class == registered_class->dpif_class);
>>> +
>>> +DPIF_PORT_FOR_EACH(_port, _dump, dpif) {
>>> +struct netdev *netdev;
>>> +int err = netdev_open(dpif_port.name, dpif_port.type,
>>> );
>>> +
>>> +if (!err) {
>>> +netdev_hmap_port_add(netdev, dpif->dpif_class,
>>> _port);
>>> +netdev_close(netdev);
>>> +} else {
>>> +VLOG_WARN("could not open netdev %s type %s", name,
>>> type);
>>> +}
>>> +}
>>>   } else {
>>>   dp_class_unref(registered_class);
>>>   }
>>> @@ -545,6 +560,14 @@ dpif_port_add(struct dpif *dpif, struct netdev
>>> *netdev, odp_port_t *port_nop)
>>>   if (!error) {
>>>   VLOG_DBG_RL(_rl, "%s: added %s as port %"PRIu32,
>>>   dpif_name(dpif), netdev_name, port_no);
>>> +
>>> +/* temp dpif_port, will be cloned in netdev_hmap_port_add */
>>> +struct dpif_port dpif_port;
>>> +
>>> +dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev));
>>> +dpif_port.name = CONST_CAST(char *, netdev_name);
>>> +dpif_port.port_no = port_no;
>>> +netdev_hmap_port_add(netdev, dpif->dpif_class, _port);
>>>   } else {
>>>   VLOG_WARN_RL(_rl, "%s: failed to add %s as port: %s",
>>>dpif_name(dpif), netdev_name,
>>> ovs_strerror(error));
>>> @@ -569,6 +592,8 @@ dpif_port_del(struct dpif *dpif, odp_port_t port_no)
>>>   if (!error) {
>>>   VLOG_DBG_RL(_rl, "%s: port_del(%"PRIu32")",
>>>   dpif_name(dpif), port_no);
>>> +
>>> +netdev_hmap_port_del(port_no, dpif->dpif_class->type);
>>>   } else {
>>>   log_operation(dpif, "port_del", error);
>>>   }
>>> diff --git a/lib/netdev.c b/lib/netdev.c
>>> index b289166..2630802 100644
>>> --- a/lib/netdev.c
>>> +++ b/lib/netdev.c
>>> @@ -2080,6 +2080,139 @@ netdev_init_flow_api(struct netdev *netdev)
>>>   : EOPNOTSUPP);
>>>   }
>>>
>>> +static struct hmap port_to_netdev = HMAP_INITIALIZER(_to_netdev);
>>> +static struct hmap ifindex_to_port = HMAP_INITIALIZER(_to_port);
>>> +
>>> +struct port_to_netdev_data {
>>> +struct hmap_node node;
>>> +struct netdev *netdev;
>>> +struct dpif_port dpif_port;
>>> +const void *obj;
>>> +};
>>> +
>>> +struct ifindex_to_port_data {
>>> +struct hmap_node node;
>>> +int ifindex;
>>> +odp_port_t port;
>>> +};
>>> +
>>> +int
>>> +netdev_hmap_port_add(struct netdev *netdev, const void *obj,
>>> + struct dpif_port *dpif_port)
>>> +{
>>> +size_t hash = hash_int(dpif_port->port_no, hash_pointer(obj, 0));
>>> +struct port_to_netdev_data *data = xzalloc(sizeof *data);
>>> +struct ifindex_to_port_data *ifidx = xzalloc(sizeof *ifidx);
>>> +
>>> +netdev_hmap_port_del(dpif_port->port_no, obj);
>>> +
>>> +data->netdev = netdev_ref(netdev);
>>> +data->obj = obj;
>>> +dpif_port_clone(>dpif_port, dpif_port);
>>> +
>>> +ifidx->ifindex = netdev_get_ifindex(netdev);
>>> +ifidx->port = dpif_port->port_no;
>>> +
>>> +hmap_insert(_to_netdev, >node, hash);
>>> +hmap_insert(_to_port, >node, ifidx->ifindex);
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +struct netdev *
>>> +netdev_hmap_port_get(odp_port_t port_no, const void *obj)
>>> +{
>>> +size_t hash = hash_int(port_no, hash_pointer(obj, 0));
>>> +struct port_to_netdev_data *data;
>>> +
>>> +HMAP_FOR_EACH_WITH_HASH(data, node, hash, _to_netdev) {
>>> +if (data->obj == obj && data->dpif_port.port_no == port_no)
>>> {
>>> +break;
>>> +}
>>> +}
>>> +
>>> +if (data) {
>>> +netdev_ref(data->netdev);
>>> +

Re: [ovs-dev] [PATCH v3 0/4] Fix some "clone"-related issues

[Ben Pfaff]

I applied these to master following Dong Jun's testing.

On Fri, Jan 06, 2017 at 08:33:41AM -0800, Ben Pfaff wrote:
> v1->v2:
>   - Patches 1 and 2 were applied and dropped.
>   - Patch 3 (now patch 1) no longer clears the action set or stack, although 
> it
> saves and restores them, following discussion with Jarno and Mickey.
>   - Patch 4 (now patch 2) is unchanged.
>   - Patch 5 (now patch 3) dropped unrelated changes to manpages.mk.
>   - Added a new patch to use ct_clear action in ovn-controller.
> 
> v2->v3:
>   - Fix a test bug and a nit in patch 3 (thanks Mickey!).
> 
> Ben Pfaff (4):
>   ofproto-dpif-xlate: Make "clone" save action set and stack.
>   ofproto-dpif-xlate: Make clone save "was_mpls".
>   New action "ct_clear".
>   ovn-controller: Clear conntrack state inside clone action.
> 
>  NEWS  |  2 +-
>  include/openvswitch/ofp-actions.h |  3 +-
>  lib/ofp-actions.c | 43 +++-
>  ofproto/ofproto-dpif-xlate.c  | 42 ++-
>  ovn/controller/physical.c |  3 +-
>  tests/ofp-actions.at  |  3 ++
>  tests/ofproto-dpif.at | 60 
> +++
>  utilities/ovs-ofctl.8.in  |  9 +-
>  8 files changed, 153 insertions(+), 12 deletions(-)
> 
> -- 
> 2.10.2
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Issues with the use of the clone action for resubmission to the pipeline

[Ben Pfaff]

Thank you very much for testing.  I applied all four patches to master.

On Sat, Jan 07, 2017 at 06:08:15PM +0800, Dong Jun wrote:
> 
> On 2017/1/7 17:14, Dong Jun wrote:
> >I tested my 
> >issue(https://mail.openvswitch.org/pipermail/ovs-dev/2016-December/326936.html)
> >withpatch serial v3 1-4 (http://patchwork.ozlabs.org/patch/712028/).
> It sounds a little vague, i mean all four patches from 1/4 to 4/4.
> >
> >The issue has been resolved. Gateway NAT and floating ip NAT both worked
> >well and conntrack
> >flows were completed as well.
> >Thank you.
> >
> >
> >On 2017/1/6 20:24, Numan Siddique wrote:
> >>
> >>
> >>On Fri, Jan 6, 2017 at 9:52 AM, Ben Pfaff  >>> wrote:
> >>
> >>On Thu, Jan 05, 2017 at 05:54:46PM -0800, Jarno Rajahalme wrote:
> >>>
> >>> > On Jan 5, 2017, at 4:28 PM, Ben Pfaff  >>> wrote:
> >>> >
> >>> > On Tue, Jan 03, 2017 at 02:55:19AM -0800, Mickey Spiegel wrote:
> >>> >> One of the motivations for clone is to use it as a
> >>lightweight way to
> >>> >> resubmit to an earlier table at the beginning of the
> >>pipeline, without
> >>> >> incurring all of the overhead associated with openflow patch
> >>ports.
> >>> >> One such usage is in OVN, where a recent patch set replaced the
> >>> >> use of openflow patch ports with clone, for OVN patch ports
> >>within
> >>> >> the same bridge (br-int).
> >>> >>
> >>> >> Over the holidays, some issues arose related to this usage of
> >>clone
> >>> >> (see the thread ovn ping from VM to external gateway IP
> >>failed).
> >>> >
> >>> > Thanks for bringing this up.  I had overlooked these questions
> >>and this
> >>> > issue.
> >>> >
> >>> > I guess that we should save and restore this since we're saving
> >>and
> >>> > restoring the conntrack metadata.  I've written up a patch.
> >>> >
> >>> >>   ctx->was_mpls
> >>> >
> >>> > I do not think that that this is a correctness issue, but it's
> >>a nice
> >>> > optimization to save and restore this.  I've written up a patch.
> >>>
> >>> I think the rationale is that ‘was_mpls’ is used to track the
> >>validity
> >>> of the flow key across an MPLS POP operation. Since the flow key
> >>is
> >>> saved and restored, we should save and restore ‘was_mpls’ as well.
> >>
> >>OK.  I did send a patch that adds that.  Do you want to make a
> >>suggestion for the commit message?  I may not fully understand the
> >>issue
> >>yet, so I don't think the commit message is very good.
> >>
> >>Here's the patch:
> >>https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327207.html
> >>
> >>
> >>> >>   ctx->xin->tables_version (not an issue if bridge does not
> >>change)
> >>> >
> >>> > clone doesn't change the bridge, so this shouldn't matter.
> >>> >
> >>> >>   ctx->stack
> >>> >>   ctx->action_set
> >>> >
> >>> > I think it's cleanest if a clone starts off with both of these
> >>empty and
> >>> > saves and restores them.  I've written up a patch.
> >>>
> >>> I think saving and restoring is needed, but I’m not so sure of
> >>> clearing these. However, it seems that there is no way for the
> >>action
> >>> set to be executed within the clone, so I guess it does not
> >>matter.
> >>
> >>I guess that it would also be a clean design, and consistent with
> >>my new
> >>ct_clear action, to not clear them but instead to start from a
> >>copy and
> >>allow for clearing them explicitly within the clone.
> >>
> >>There is already an instruction to clear the action set, so we
> >>wouldn't
> >>need to add anything.  I think that the action set can only affect
> >>what
> >>happens inside the clone in terms of matches or actions based on the
> >>actset_output field, though.
> >>
> >>I'm not sure of the value of an action to clear the stack, so I'd be
> >>inclined to hold off on that until we think of one.
> >>
> >>I'll revise my patch to work this way.
> >>
> >>> It would be good to add these changes to the documentation as
> >>well.
> >>
> >>My patch does update the documentation on this point.
> >>
> >>
> >>​Thanks Ben for all the fixes. We are in middle of performance testing
> >>with the version of ovn-controller which creates patch ports for router
> >>ports.
> >>
> >>Once this is done, we will be able to test with the patches you have
> >>proposed.
> >>
> >>​Dong Jun- May be if you want to test these patches and I see if it
> >>resolves the issues which you had posted.
> >>​
> >>
> >>Thanks
> >>Numan
> >>
> >>
> >>___
> >>dev mailing list
> >>d...@openvswitch.org 
> >>

Re: [ovs-dev] [PATCH] windows: Change driver and MSI company name to LF

[Sairam Venugopal]

Acked-by: Sairam Venugopal 





On 12/6/16, 3:30 PM, "ovs-dev-boun...@openvswitch.org on behalf of Alin 
Serdean"  wrote:

>Until now we used 'Open vSwitch' as the company/organization name.
>
>The project is now under The Linux Foundation ownership.
>
>This patch updates the MSI and driver attributes to reflect that ownership.
>
>Signed-off-by: Alin Gabriel Serdean 
>---
> datapath-windows/ovsext/TunnelFilter.c| 2 +-
> datapath-windows/ovsext/ovsext.inf| 2 +-
> datapath-windows/ovsext/ovsext.rc | 2 +-
> windows/ovs-windows-installer/Product.wxs | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/datapath-windows/ovsext/TunnelFilter.c 
>b/datapath-windows/ovsext/TunnelFilter.c
>index 649f5bc..f5f82cb 100644
>--- a/datapath-windows/ovsext/TunnelFilter.c
>+++ b/datapath-windows/ovsext/TunnelFilter.c
>@@ -50,7 +50,7 @@
> 
> /* The provider name should always match the provider string from the install
>  * file. */
>-#define OVS_TUNNEL_PROVIDER_NAMEL"Open vSwitch"
>+#define OVS_TUNNEL_PROVIDER_NAMEL"The Linux Foundation (R)"
> 
> /* The provider description should always contain the OVS service description
>  * string from the install file. */
>diff --git a/datapath-windows/ovsext/ovsext.inf 
>b/datapath-windows/ovsext/ovsext.inf
>index c067e72..39b3469 100644
>--- a/datapath-windows/ovsext/ovsext.inf
>+++ b/datapath-windows/ovsext/ovsext.inf
>@@ -80,6 +80,6 @@ AddReg  = Common.Params.reg
> DelService=OVSExt,0x200
> 
> [Strings]
>-OVS = "Open vSwitch"
>+OVS = "The Linux Foundation (R)"
> OVSExt_Desc = "Open vSwitch Extension"
> OVSExt_HelpText = "Open vSwitch forwarding switch extension"
>diff --git a/datapath-windows/ovsext/ovsext.rc 
>b/datapath-windows/ovsext/ovsext.rc
>index 0b92e2e..0ff49ca 100644
>--- a/datapath-windows/ovsext/ovsext.rc
>+++ b/datapath-windows/ovsext/ovsext.rc
>@@ -30,7 +30,7 @@ BEGIN
> BEGIN
> BLOCK "04b0"
> BEGIN
>-VALUE "CompanyName", "Open vSwitch"
>+VALUE "CompanyName", "The Linux Foundation (R)"
> VALUE "FileDescription", "Open vSwitch Extension"
> VALUE "FileVersion", "6.3.9600.17298"
> VALUE "InternalName", "OVSExt.SYS"
>diff --git a/windows/ovs-windows-installer/Product.wxs 
>b/windows/ovs-windows-installer/Product.wxs
>index 8b58729..c9689ae 100644
>--- a/windows/ovs-windows-installer/Product.wxs
>+++ b/windows/ovs-windows-installer/Product.wxs
>@@ -19,7 +19,7 @@
>  xmlns="https://urldefense.proofpoint.com/v2/url?u=http-3A__schemas.microsoft.com_wix_2006_wi=DgICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=B6LOluI3w76R5HzS2gJNGOLm6vxYfX_x_JDRnkpmEZk=u0Vdq3UZ2u6V36t7LiqCBK2jo0DkXq5_LgBoyT5viRk=
>  "
>  
> xmlns:util="https://urldefense.proofpoint.com/v2/url?u=http-3A__schemas.microsoft.com_wix_UtilExtension=DgICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=B6LOluI3w76R5HzS2gJNGOLm6vxYfX_x_JDRnkpmEZk=knklcwI6xyKtBkpVsHwW78sp4l_TWMKXeeKeZrRC3fo=
>  ">
>   -   Manufacturer="Open vSwitch" 
>UpgradeCode="da802b12-433d-4742-a7ae-783aa0c48222">
>+   Manufacturer="The Linux Foundation" 
>UpgradeCode="da802b12-433d-4742-a7ae-783aa0c48222">
>  InstallScope="perMachine" InstallPrivileges="elevated" Platform="x64" />
> 
> 
>-- 
>2.10.2.windows.1
>___
>dev mailing list
>d...@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DgICAg=uilaK90D4TOVoH58JNXRgQ=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo=B6LOluI3w76R5HzS2gJNGOLm6vxYfX_x_JDRnkpmEZk=4XNyMUBfzbDXWL6Ct94ojy_G7LlkoNIIEY3HzQX_95Q=
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] USA Global Youths Conference 2017

[Rhonda Watkins]

Dear Sir / Madam,
 
It is my great privilege for me to invite you to  2017 Global Youths 
conferences which will be held from 21st - 25th of February 2017 at the  
{Atlanta Georgia Convention Center United States of America, The organizing 
committee is responsible for all visa arrangements to the United States, free 
economic air round trip tickets. Registration is open and free to all 
interested participants. Hotel accommodation booking costs will be your own 
responsibility. The interested participants are to contact the Secretary 
Office: via email: rhondawatkins.hr...@hotmail.com 

We look forward to see you soon!
 
Best regards.
Mis, Rhonda Watkins


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

Re: [ovs-dev] [PATCH ovs V2 00/21] Introducing HW offload support for openvswitch

[Joe Stringer]

On 10 January 2017 at 10:07, Joe Stringer  wrote:
> On 8 January 2017 at 07:18, Paul Blakey  wrote:
>>
>> On 05/01/2017 02:11, Joe Stringer wrote:
>>
>>> On 25 December 2016 at 03:39, Paul Blakey  wrote:

 This patch series introduces rule offload functionality to dpif-netlink
 via netdev ports new flow offloading API. The user can specify whether to
 enable rule offloading or not via OVS configuration. Netdev providers
 are able to implement netdev flow offload API in order to offload rules.

 This patch series also implements one offload scheme for netdev-linux,
 using TC flower classifier, which was chosen because its sort of natrual
 to
 state OVS DP rules for this classifier. However, the code can be extended
 to support other classifiers such as U32, eBPF, etc which support offload
 as well.

 The use-case we are currently addressing is the newly sriov switchdev
 mode in the
 linux kernel which was introduced in version 4.8 [1][2]. this series was
 tested against sriov vfs
 vports representors of the Mellanox 100G ConnectX-4 series exposed by the
 mlx5 kernel driver.

 changes from V1
  - Added generic netdev flow offloads API.
  - Implemented relevant flow API in netdev-linux (and netdev-vport).
  - Added a other_config hw-offload option to enable offloading
 (defaults to false).
  - Fixed coding style to conform with OVS.
  - Policy removed for now. (Will be discussed how best implemented
 later).
>>>
>>> Hi Paul,
>>>
>>> Happy holidays to you too ;)
>>
>> Hi :)
>> Thanks for your thorough review, We are working on resubmitting the next
>> patch set and I'll
>> try and reply as much as I can right now, but this is a big review, so I'll
>> split this to several replies.
>>>
>>>
>>> A few high level comments:
>>> * Overall the patchset is looking in better state than previously.
>>> * That said, on platforms other than your specific test environment
>>> OVS fails to build.
>>> * netdev-vport seems to have acquired platform-specific code
>>
>> Thanks, We've seen your links for travis-ci and app veyor, we'll create an
>> account and fix it accordingly.
>>>
>>> * I don't think that ovs-dpctl should read the database. One of the
>>> nice things about ovs-dpctl is that it is currently standalone, ie
>>> doesn't require anything else to be running to be able to query the
>>> state of the kernel datapath. What are you trying to achieve with
>>> this? Perhaps you can extend the ovs-appctl dpctl/... commands instead
>>> to provide the functionality you need?
>>
>> We have two config options that isn't saved with the kernel datapath (and
>> dumped back on dpif open):
>> if offload is enabled to know if we even need to dump from netdevs or not
>> for dump-flows cmd (I guess we could
>> just dump the netdev anyway) and for add-flow, and tc flower flag
>> skip_hw/skip_sw that is also needed for add-flow cmd,
>> so we can know what flag we be used while inserting to flower if offloading
>> is enabled and possible.
>
> I discussed this with a few people offline and it seems that the most
> reasonable way to handle it for dump is just to dump from netdevs
> anyway. You can fetch the list of relevant ports from OVS datapath in
> kernel, then only dump those ports.
>
> For adding and deleting flows, using ovs-dpctl is not really
> supported, it's primarily for debug. ovs-vswitchd is going to override
> that behaviour anyway - if you add a flow, it may delete it; if you
> delete a flow, it may re-add it. I suggest that for debugging purposes
> the default behaviour is to install into OVS, but if you want to put
> it into TC sw/hw, provide an additional flag on the commandline.
>
> If you want ovs-vswitchd to decide how to install the flow, you can
> always use "ovs-appctl dpctl/add-flow ..." and the command will go via
> ovs-vswitchd, which will make the offloading decision based on the
> same logic as it would when handling regular upcalls.
>
>>> * It would be good to see at least some basic system tests ala
>>> tests/system-traffic.at or tests/system-ovn.at. Maybe there could be
>>> tests/system-tc.at that uses SKIP_HW to show that flows can be
>>> installed and that they work.
>>
>> Sure.
>>>
>>> * I think it would be great to get some ability to see what's
>>> happening with flow offloads; eg "ovs-appctl dpctl/show-hw-offloads"
>>> that could give you some idea of how many flows are being offloaded
>>
>> You mean a number/stats or actually dump only offloaded flows?
>
> At the time, I was thinking just stats.
>
> However I think it would also be useful to have some flag for
> ovs-dpctl that chooses which flows to fetch.
>
> Something like:
> ovs-dpctl dump-flows: Dump all flows from both OVS and TC (flower only).
> ovs-dpctl dump-flows --only-flower
> ovs-dpctl dump-flows --only-ovs
>
> Each flow when printed should also indicate 

Re: [ovs-dev] [PATCH v2 4/5] datapath-windows: Add support for OVS_KEY_ATTR_UDP set action

[Alin Serdean]

Udp checksum should not be updated in the case it was 0. I will add the changes 
in the next version.

Thanks,
Alin.

> -Original Message-
> From: Alin Serdean
> Sent: Friday, January 6, 2017 9:34 PM
> To: d...@openvswitch.org
> Cc: Alin Serdean 
> Subject: [PATCH v2 4/5] datapath-windows: Add support for
> OVS_KEY_ATTR_UDP set action
> 
> This patch adds support for set action with OVS_KEY_ATTR_UDP attribute
> (change UDP source or destination port).
> 
> If the source or destination UDP port was changed, update the UDP
> checksum.
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/6] datapath-windows: VXLAN Check for flow destination port

[Alin Serdean]

Change the UDP destination port(VXLAN header) to check if it was set by
the userspace, use it if it was set.
If the userspace did not specify a destination port, use the configured
vport destination port.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Vxlan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
index 949e069..84c2f2f 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -289,7 +289,8 @@ OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
 /* UDP header */
 udpHdr = (UDPHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
 udpHdr->source = htons(tunKey->flow_hash | MAXINT16);
-udpHdr->dest = htons(vportVxlan->dstPort);
+udpHdr->dest = tunKey->dst_port ? tunKey->dst_port :
+  htons(vportVxlan->dstPort);
 udpHdr->len = htons(NET_BUFFER_DATA_LENGTH(curNb) - headRoom +
 sizeof *udpHdr + sizeof *vxlanHdr);
 
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 3/6] datapath-windows: Add support for OVS_TUNNEL_KEY_ATTR_TP_DST

[Alin Serdean]

Add support for netlink attribute OVS_TUNNEL_KEY_ATTR_TP_DST get/set
flow functions.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Flow.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 749d83a..96ff9fa 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1737,6 +1737,9 @@ OvsTunnelAttrToIPv4TunnelKey(PNL_ATTR attr,
 case OVS_TUNNEL_KEY_ATTR_OAM:
 tunKey->flags |= OVS_TNL_F_OAM;
 break;
+case OVS_TUNNEL_KEY_ATTR_TP_DST:
+tunKey->dst_port = NlAttrGetBe16(a);
+break;
 case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS:
 if (hasOpt) {
 /* Duplicate options attribute is not allowed. */
@@ -1812,6 +1815,11 @@ MapTunAttrToFlowPut(PNL_ATTR *keyAttrs,
 destKey->tunKey.flags |= OVS_TNL_F_OAM;
 }
 
+if (tunAttrs[OVS_TUNNEL_KEY_ATTR_TP_DST]) {
+destKey->tunKey.dst_port =
+NlAttrGetU16(tunAttrs[OVS_TUNNEL_KEY_ATTR_TP_DST]);
+}
+
 if (tunAttrs[OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS]) {
 NTSTATUS status = OvsTunnelAttrToGeneveOptions(
   tunAttrs[OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS],
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/6] datapath-windows: Allow tunnel action to modify destination port

[Alin Serdean]

'OvsTunnelAttrToIPv4TunnelKey' modifies 'tunkey' with the received netlink
attributes(i.e. OVS_TUNNEL_KEY_ATTR_IPV4_DST).

Change the order of the value assignment to reflect the values received via
userspace.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Actions.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index b4a286b..1530d8b 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1492,11 +1492,11 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
 case OVS_KEY_ATTR_TUNNEL:
 {
 OvsIPv4TunnelKey tunKey;
+tunKey.flow_hash = (uint16)(hash ? *hash : OvsHashFlow(key));
+tunKey.dst_port = key->ipKey.l4.tpDst;
 NTSTATUS convertStatus = OvsTunnelAttrToIPv4TunnelKey((PNL_ATTR)a, 
);
 status = SUCCEEDED(convertStatus) ? NDIS_STATUS_SUCCESS : 
NDIS_STATUS_FAILURE;
 ASSERT(status == NDIS_STATUS_SUCCESS);
-tunKey.flow_hash = (uint16)(hash ? *hash : OvsHashFlow(key));
-tunKey.dst_port = key->ipKey.l4.tpDst;
 RtlCopyMemory(>tunKey, , sizeof ovsFwdCtx->tunKey);
 break;
 }
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/6] datapath-windows: Fix alignment in MapTunAttrToFlowPut

[Alin Serdean]

Found by inspection.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 2e8b42b..749d83a 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1809,7 +1809,7 @@ MapTunAttrToFlowPut(PNL_ATTR *keyAttrs,
 }
 
 if (tunAttrs[OVS_TUNNEL_KEY_ATTR_OAM]) {
-destKey->tunKey.flags |= OVS_TNL_F_OAM;
+destKey->tunKey.flags |= OVS_TNL_F_OAM;
 }
 
 if (tunAttrs[OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS]) {
-- 
2.10.2.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] debian; Avoid installing ovs-vswitchd.conf.db manpage as "db" language.

[Ben Pfaff]

Usually, when the name of a manpage has a two-letter extension, it means
that the manpage is written in the language designated by that language
code.

Reported-by: Michael Stapelberg 
Reported-at: https://bugs.debian.org/850631
Signed-off-by: Ben Pfaff 
---
 AUTHORS.rst  | 1 +
 debian/rules | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index add95ef..9b21bd6 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -444,6 +444,7 @@ Michael Hu  m...@nicira.com
 Michael J. Smalley  michaeljsmal...@gmail.com
 Michael Mao m...@nicira.com
 Michael Shigorinm...@osdn.org.ua
+Michael Stapelberg  stapelb...@debian.org
 Mihir Gangargang...@vmware.com
 Mike Bursellmike.burs...@citrix.com
 Mike Kruze  mkr...@nicira.com
diff --git a/debian/rules b/debian/rules
index 12fb94a..62cd14b 100755
--- a/debian/rules
+++ b/debian/rules
@@ -96,3 +96,6 @@ override_dh_strip:
dh_strip --dbg-package=openvswitch-dbg
 
 override_dh_usrlocal:
+
+override_dh_installman:
+   dh_installman --language=C
-- 
2.10.2

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

Re: [ovs-dev] [PATCH ovs V2 01/21] tc: Add tc flower interface

[Paul Blakey]

On 05/01/2017 02:57, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Add tc flower interface that will be used to offload flows via tc
flower classifier. Depending on the flag used (skip_sw/hw) flower
will pass those to HW or handle them itself.

Signed-off-by: Shahar Klein
Signed-off-by: Paul Blakey
Reviewed-by: Roi Dayan
---

Was Shahar also a co-author? Perhaps you should place the co-author tag as well.

Yes he is, We'll add that tag.

Another question: What happens if someone manually configures
additional flower (or non-flower?) filters on devices? Does OVS
complain constantly in the logs, or handle it ok? Overwrite the user
configuration?
For now additional flower are interpreted back to OVS rules and will be 
generated a new UFID.

Non flower filters are ignored by dump.


  lib/automake.mk |   2 +
  lib/tc.c| 996 
  lib/tc.h| 107 ++
  3 files changed, 1105 insertions(+)
  create mode 100644 lib/tc.c
  create mode 100644 lib/tc.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 9345cee..bcc7813 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -351,6 +351,8 @@ if LINUX
  lib_libopenvswitch_la_SOURCES += \
 lib/dpif-netlink.c \
 lib/dpif-netlink.h \
+   lib/tc.h \
+   lib/tc.c \
 lib/if-notifier.c \
 lib/if-notifier.h \
 lib/netdev-linux.c \
diff --git a/lib/tc.c b/lib/tc.c
new file mode 100644
index 000..b5f6603
--- /dev/null
+++ b/lib/tc.c
@@ -0,0 +1,996 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

I think that some of these cause userspace dependency on very new
kernel headers. Could we provide a copy of them in the OVS tree as per
discussion in cover letter?

Yes, thanks.




+/* Returns tc handle 'major':'minor'. */
+static unsigned int
+tc_make_handle(unsigned int major, unsigned int minor)
+{
+return TC_H_MAKE(major << 16, minor);
+}
+
+static struct tcmsg *
+tc_make_req(int ifindex, int type, unsigned int flags, struct ofpbuf *request)
+{
+struct tcmsg *tcmsg;
+struct nlmsghdr *nlmsghdr;
+
+ofpbuf_init(request, 512);
+
+nl_msg_reserve(request, NLMSG_HDRLEN + sizeof *tcmsg);
+nlmsghdr = nl_msg_put_uninit(request, NLMSG_HDRLEN);
+nlmsghdr->nlmsg_len = 0;
+nlmsghdr->nlmsg_type = type;
+nlmsghdr->nlmsg_flags = NLM_F_REQUEST | flags;
+nlmsghdr->nlmsg_seq = 0;
+nlmsghdr->nlmsg_pid = 0;
+
+tcmsg = ofpbuf_put_zeros(request, sizeof *tcmsg);
+tcmsg->tcm_family = AF_UNSPEC;
+tcmsg->tcm_ifindex = ifindex;
+
+return tcmsg;
+}
+
+static int
+tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
+{
+int error = nl_transact(NETLINK_ROUTE, request, replyp);
+
+ofpbuf_uninit(request);
+return error;
+}

The above few functions are practically the same as the versions in
lib/netdev-linux.c. Could you separate these into a separate patch
which refactors these functions into lib/tc.c, exports them in
lib/tc.h and reuses them frome lib/netdev-linux.c ?

Sure.

+static int
+__nl_parse_flower_eth(struct nlattr **attrs, struct tc_flow *tc_flow)

Typically ovs uses foo__() for variations on functions; but in many of
these cases it seems that the underscores are unnecessary. You can
just name thin nl_parse_flower_eth. (We don't name things specially to
indicate that they're private in the file either).


+{
+const struct eth_addr *eth = 0;
+
+if (attrs[TCA_FLOWER_KEY_ETH_SRC_MASK]) {
+eth = nl_attr_get_unspec(attrs[TCA_FLOWER_KEY_ETH_SRC], ETH_ALEN);
+memcpy(_flow->key.src_mac, eth, sizeof tc_flow->key.src_mac);
+
+eth = nl_attr_get_unspec(attrs[TCA_FLOWER_KEY_ETH_SRC_MASK], ETH_ALEN);
+memcpy(_flow->mask.src_mac, eth, sizeof tc_flow->mask.src_mac);
+}
+if (attrs[TCA_FLOWER_KEY_ETH_DST_MASK]) {
+eth = nl_attr_get_unspec(attrs[TCA_FLOWER_KEY_ETH_DST], ETH_ALEN);
+memcpy(_flow->key.dst_mac, eth, sizeof tc_flow->key.dst_mac);
+
+eth = nl_attr_get_unspec(attrs[TCA_FLOWER_KEY_ETH_DST_MASK], ETH_ALEN);
+memcpy(_flow->mask.dst_mac, eth, sizeof tc_flow->mask.dst_mac);
+}
+return 0;
+}

A bunch of these functions only ever return 0. Why return anything?

Re: [ovs-dev] [PATCH ovs V2 19/21] dpctl: read vswitch config on start

[Paul Blakey]

On 05/01/2017 23:46, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Use Open vSwitch IDL pattern to read OVS configuration on dpctl start,
needed as some functionality is dependent on that configuration.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 

Why? What functionality?
Since the hw-offload/skip-hw configurations aren't saved with kernel 
dpif and dumped back on open,
we needed "hw-offload" configuration to know if to try offloading for 
add-flow dpctl command, and then

which flag to pass on to TC (skip_hw/skip_sw) if offloading is enabled.
Also for dpctl dump-flows, if hw offload is enabled dump offloaded flows 
from netdevs.




What if OVSDB isn't running, does ovs-dpctl block?

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

[ovs-dev] Bug#850631: marked as done (Manpage ovs-vswitchd.conf.db.5 installed into wrong directory)

[Debian Bug Tracking System]

Your message dated Tue, 10 Jan 2017 16:34:13 +
with message-id 
and subject line Bug#850631: fixed in openvswitch 2.6.2~pre+git20161223-3
has caused the Debian Bug report #850631,
regarding Manpage ovs-vswitchd.conf.db.5 installed into wrong directory
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
850631: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=850631
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V2 12/21] dpif-netlink: Use netdev flow put api to insert a flow

[Paul Blakey]

On 06/01/2017 01:25, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Using the new netdev flow api operate will now try and
offload flows to the relevant netdev of the input port.
Other operate methods flows will come in later patches.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
  lib/dpif-netlink.c | 232 -
  1 file changed, 228 insertions(+), 4 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 3d8940e..717af90 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1908,15 +1908,239 @@ dpif_netlink_operate__(struct dpif_netlink *dpif,
  return n_ops;
  }

+static int
+parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
+const struct nlattr *mask, size_t mask_len,
+struct match *match)
+{
+enum odp_key_fitness fitness;
+
+fitness = odp_flow_key_to_flow(key, key_len, >flow);
+if (fitness) {
+/* This should not happen: it indicates that odp_flow_key_from_flow()
+ * and odp_flow_key_to_flow() disagree on the acceptable form of a
+ * flow.  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()) {
+struct ds s;
+
+ds_init();
+odp_flow_format(key, key_len, NULL, 0, NULL, , true);
+VLOG_ERR("internal error parsing flow key %s", ds_cstr());
+ds_destroy();
+}
+
+return EINVAL;
+}
+
+fitness = odp_flow_key_to_mask(mask, mask_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()) {
+struct ds s;
+
+VLOG_ERR("internal error parsing flow mask %s (%s)",
+ ds_cstr(), odp_key_fitness_to_string(fitness));
+ds_destroy();
+}
+
+return EINVAL;
+}
+
+return 0;
+}

Duplicated code. Please refactor and reuse. (Separate patch for
refactoring to keep the patches clear).


+
+static bool
+parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
+{
+struct match match;
+odp_port_t in_port;
+const struct nlattr *nla;
+size_t left;
+int outputs = 0;
+struct ofpbuf buf;
+uint64_t act_stub[1024 / 8];
+size_t offset;
+struct nlattr *act;
+struct netdev *dev;
+int err;
+
+/* 0x1234 - fake eth type sent to probe feature */
+if (put->flags & DPIF_FP_PROBE || match.flow.dl_type == htons(0x1234)) {
+return false;
+}

Shouldn't DPIF_FP_PROBE be sufficient?

I guess, I think we added it because we still got some probe features. I've
checked it now and I couldn't find any.

+
+if (parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
+put->mask_len, )) {
+return false;
+}
+
+in_port = match.flow.in_port.odp_port;
+ofpbuf_use_stub(, act_stub, sizeof act_stub);
+offset = nl_msg_start_nested(, OVS_FLOW_ATTR_ACTIONS);
+NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) {
+if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
+struct netdev *outdev;
+int ifindex_out;
+const struct netdev_tunnel_config *tnl_cfg;
+size_t out_off;
+odp_port_t out_port;
+
+outputs++;
+if (outputs > 1) {
+break;
+}
+
+out_port = nl_attr_get_u32(nla);
+outdev = netdev_hmap_port_get(out_port, dpif->dpif.dpif_class);
+tnl_cfg = netdev_get_tunnel_config(outdev);
+
+out_off = nl_msg_start_nested(, OVS_ACTION_ATTR_OUTPUT);
+ifindex_out = netdev_get_ifindex(outdev);

Can this fail? (port not backed by device with ifindex?)


+nl_msg_put_u32(, OVS_ACTION_ATTR_OUTPUT, ifindex_out);
+if (tnl_cfg && tnl_cfg->dst_port != 0) {
+nl_msg_put_u32(, OVS_TUNNEL_KEY_ATTR_TP_DST, 
tnl_cfg->dst_port);
+}
+nl_msg_end_nested(, out_off);
+
+if (outdev) {
+netdev_close(outdev);
+}
+} else {
+nl_msg_put_unspec(, nl_attr_type(nla), nl_attr_get(nla),
+  nl_attr_get_size(nla));
+}
+}
+nl_msg_end_nested(, offset);

Hmm. I'm a bit uneasy about this whole copying/rewriting of the
actions. Firstly, the tunnel stuff just looks wrong. A quick "git
grep" shows that OVS_TUNNEL_KEY_ATTR_TP_DST 

Re: [ovs-dev] [PATCH] ovn-ctl: Modify SYNC FROM connection default protocol to SSL

[Lance Richardson]

> From: "e" 
> To: ovs-dev@openvswitch.org
> Cc: "e" 
> Sent: Monday, January 9, 2017 9:44:43 PM
> Subject: [ovs-dev] [PATCH] ovn-ctl: Modify SYNC FROM connection default   
> protocol to SSL
> 
> This patch is used for the OVSDB HA by pacemaker.
> which the master and slave nodes connection use SSL by default
> 

Could you expand on the motivation for changing the default from
TCP to SSL? Is it expected that SSL will be more commonly used than
TCP? (I would have guessed plain TCP to be more common.)

> Signed-off-by: e 
> ---
>  ovn/utilities/ovn-ctl | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> index 90d0463..214bbc5 100755
> --- a/ovn/utilities/ovn-ctl
> +++ b/ovn/utilities/ovn-ctl
> @@ -297,7 +297,7 @@ set_defaults () {
>  DB_NB_FILE=$dbdir/ovnnb_db.db
>  DB_NB_ADDR=0.0.0.0
>  DB_NB_PORT=6641
> -DB_NB_SYNC_FROM_PROTO=tcp
> +DB_NB_SYNC_FROM_PROTO=ssl
>  DB_NB_SYNC_FROM_ADDR=
>  DB_NB_SYNC_FROM_PORT=6641
>  
> @@ -306,7 +306,7 @@ set_defaults () {
>  DB_SB_FILE=$dbdir/ovnsb_db.db
>  DB_SB_ADDR=0.0.0.0
>  DB_SB_PORT=6642
> -DB_SB_SYNC_FROM_PROTO=tcp
> +DB_SB_SYNC_FROM_PROTO=ssl
>  DB_SB_SYNC_FROM_ADDR=
>  DB_SB_SYNC_FROM_PORT=6642
>  
> @@ -409,12 +409,12 @@ File location options:
>--db-sb-port=PORTOVN Southbound db ptcp port (default: $DB_SB_PORT)
>--ovn-nb-logfile=FILE OVN Northbound log file (default: $OVN_NB_LOGFILE)
>--ovn-sb-logfile=FILE OVN Southbound log file (default: $OVN_SB_LOGFILE)
> -  --db-nb-sync-from-addr=ADDR OVN Northbound active db tcp address (default:
> $DB_NB_SYNC_FROM_ADDR)
> -  --db-nb-sync-from-port=PORT OVN Northbound active db tcp port (default:
> $DB_NB_SYNC_FROM_PORT)
> +  --db-nb-sync-from-addr=ADDR OVN Northbound active db ssl address (default:
> $DB_NB_SYNC_FROM_ADDR)

It would be better to change this to "IP address" since that is what it is.
Same applies for DB_NB_SYNC_FROM_ADDR.

> +  --db-nb-sync-from-port=PORT OVN Northbound active db ssl port (default:

It would be more correct to keep "tcp port", this parameter is actually a TCP
port number regardless of whether plain TCP or SSL is being used. Same applies
for DB_SB_SYNC_FROM_PROTO.

> $DB_NB_SYNC_FROM_PORT)
>--db-nb-sync-from-proto=PROTO OVN Northbound active db transport (default:
>$DB_NB_SYNC_FROM_PROTO)
>--db-nb-create-insecure-remote=yes|no Create ptcp OVN Northbound remote
>(default: $DB_NB_CREATE_INSECURE_REMOTE)
> -  --db-sb-sync-from-addr=ADDR OVN Southbound active db tcp address (default:
> $DB_SB_SYNC_FROM_ADDR)
> -  --db-sb-sync-from-port=ADDR OVN Southbound active db tcp port (default:
> $DB_SB_SYNC_FROM_PORT)
> +  --db-sb-sync-from-addr=ADDR OVN Southbound active db ssl address (default:
> $DB_SB_SYNC_FROM_ADDR)
> +  --db-sb-sync-from-port=ADDR OVN Southbound active db ssl port (default:
> $DB_SB_SYNC_FROM_PORT)
>--db-sb-sync-from-proto=PROTO OVN Southbound active db transport (default:
>$DB_SB_SYNC_FROM_PROTO)
>--db-sb-create-insecure-remote=yes|no Create ptcp OVN Southbound remote
>(default: $DB_SB_CREATE_INSECURE_REMOTE)
>  
> --
> 2.9.0.windows.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] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink

[Eric Garver]

On Tue, Jan 10, 2017 at 04:26:10PM +0100, Jiri Benc wrote:
> On Tue, 10 Jan 2017 15:44:07 +0100, Jiri Benc wrote:
> > create an ovs_geneve interface using rtnetlink
> 
> This can be done just once. The pseudocode at startup thus would be:
> 
> create an ovs_geneve interface using rtnetlink
> if successful {
>   delete the created interface
>   set out_of_tree flag
> }
> 
> And interface creation:
> 
> if not out_of_tree {
>   create lwtunnel geneve interface using rtnetlink
>   check parameters of the created interface
>   if it is lwtunnel {
>   done, exit
>   } else {
>   delete the created interface
>   }
> }
> create geneve interface using genetlink
> if successful {
>   done
> } else {
>   fail
> }
> 
> Is that what you want? If so, should the out_of_tree flag be queried
> and set per tunnel type, or just based globally on ovs_geneve (or a
> different kind)?
> 

With Pravin's last feedback I think we can do the lwt probe on init as
well.

== On startup/init ==

create an ovs_geneve interface using rtnetlink
if successful {
check parameters of the created interface
if it is lwtunnel {
set lwt flag
}

delete the created interface
set out_of_tree flag
}

== On interface creation ==

if not out-of-tree and is lwt {
create lwtunnel geneve interface using rtnetlink
if successful {
done, exit
}
}
create geneve interface using genetlink
if successful {
done
} else {
fail
}
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [RFC] libX: add new release / version info tags

[Aaron Conole]

This commit uses the $PACKAGE_VERSION automake variable to construct a
release and version info combination which sets the library name to be:

   libfoo-$(OVS_MAJOR_VERSION).so.$(OVS_MINOR_VERSION).0.$(OVS_MICRO_VERSION)

where formerly, it was always:

   libfoo.so.1.0.0

This allows releases of Open vSwitch libraries to reflect which specific
versions they came with, and sets up a psuedo ABI-versioning scheme.  In
this fashion, future releases of Open vSwitch could be installed
alongside older releases, allowing 3rd party utilities linked against
previous versions to continue to function.

ex:

$ ldd /path/to/utility
linux-vdso.so.1 (0x7ffe92cf6000)
libopenvswitch-2.so.6 => /lib64/libopenvswitch-2.so.6 
(0x7f733b7a3000)
libssl.so.10 => /lib64/libssl.so.10 (0x7f733b53)
...

Note the library name and version information.

CC: Ben Warren 
Signed-off-by: Aaron Conole 
---
This is labelled as RFC.  I'm not sure if it's appropriate to apply
it to master before the 2.7 branch.

 configure.ac|  1 +
 lib/automake.mk |  4 ++--
 m4/openvswitch.m4   | 18 ++
 ofproto/automake.mk |  2 +-
 ovn/lib/automake.mk |  2 +-
 ovsdb/automake.mk   |  2 +-
 vtep/automake.mk|  2 +-
 7 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index a9ae2a1..7c729ef 100644
--- a/configure.ac
+++ b/configure.ac
@@ -131,6 +131,7 @@ OVS_CHECK_ATOMIC_ALWAYS_LOCK_FREE(8)
 OVS_CHECK_POSIX_AIO
 OVS_CHECK_PTHREAD_SET_NAME
 OVS_CHECK_LINUX_HOST
+OVS_LIBTOOL_VERSIONS
 AX_FUNC_POSIX_MEMALIGN
 
 OVS_CHECK_INCLUDE_NEXT([stdio.h string.h])
diff --git a/lib/automake.mk b/lib/automake.mk
index 88344a3..b03dd2d 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -15,7 +15,7 @@ lib_libopenvswitch_la_LIBADD += ${PTHREAD_LIBS}
 endif
 
 lib_libopenvswitch_la_LDFLAGS = \
--version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) \
+$(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/lib/libopenvswitch.sym \
 $(AM_LDFLAGS)
 
@@ -328,7 +328,7 @@ CLEANFILES += $(nodist_lib_libopenvswitch_la_SOURCES)
 
 lib_LTLIBRARIES += lib/libsflow.la
 lib_libsflow_la_LDFLAGS = \
--version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) \
+$(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/lib/libsflow.sym \
 $(AM_LDFLAGS)
 lib_libsflow_la_SOURCES = \
diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 6515ed7..f316abc 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -601,3 +601,21 @@ AC_DEFUN([OVS_CHECK_PRAGMA_MESSAGE],
  [AC_DEFINE(HAVE_PRAGMA_MESSAGE,1,[Define if compiler supports #pragma
  message directive])])
   ])
+
+dnl OVS_LIBTOOL_VERSIONS sets the major, minor, micro version information for
+dnl the OVS_LT_RELINFO and OVS_LT_VERINFO variables, as well as the 
encompassing
+dnl OVS_LTINFO variable.
+AC_DEFUN([OVS_LIBTOOL_VERSIONS],
+[test ".$PACKAGE_VERSION" = "." && PACKAGE_VERSION="$VERSION"
+  AC_MSG_CHECKING(linker ouput version information)
+  OVS_MAJOR=`echo "$PACKAGE_VERSION" | sed -e 's/[[.]].*//'`
+  OVS_MINOR=`echo "$PACKAGE_VERSION" | sed -e "s/^$OVS_MAJOR//" -e 's/^.//' -e 
's/[[.]].*//'`
+  OVS_MICRO=`echo "$PACKAGE_VERSION" | sed -e "s/^$OVS_MAJOR.$OVS_MINOR//" -e 
's/^.//' -e 's/[[^0-9]].*//'`
+  OVS_LT_RELINFO="-release $OVS_MAJOR"
+  OVS_LT_VERINFO="-version-info $OVS_MINOR:$OVS_MICRO"
+  OVS_LTINFO="$OVS_LT_RELINFO $OVS_LT_VERINFO"
+  AC_MSG_RESULT([/$OVS_MAJOR/$OVS_MINOR.$OVS_MICRO (-$OVS_MAJOR.so)])
+  AC_SUBST(OVS_LT_RELINFO)
+  AC_SUBST(OVS_LT_VERINFO)
+  AC_SUBST(OVS_LTINFO)
+])
diff --git a/ofproto/automake.mk b/ofproto/automake.mk
index 5a83a60..86c24b1 100644
--- a/ofproto/automake.mk
+++ b/ofproto/automake.mk
@@ -7,7 +7,7 @@
 
 lib_LTLIBRARIES += ofproto/libofproto.la
 ofproto_libofproto_la_LDFLAGS = \
--version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) \
+$(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/ofproto/libofproto.sym \
 $(AM_LDFLAGS)
 ofproto_libofproto_la_SOURCES = \
diff --git a/ovn/lib/automake.mk b/ovn/lib/automake.mk
index 7f26de2..b86237e 100644
--- a/ovn/lib/automake.mk
+++ b/ovn/lib/automake.mk
@@ -1,6 +1,6 @@
 lib_LTLIBRARIES += ovn/lib/libovn.la
 ovn_lib_libovn_la_LDFLAGS = \
--version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) \
+$(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/ovn/lib/libovn.sym \
 $(AM_LDFLAGS)
 ovn_lib_libovn_la_SOURCES = \
diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index 099ed3c..81695f1 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -1,7 +1,7 @@
 # libovsdb
 lib_LTLIBRARIES += ovsdb/libovsdb.la
 ovsdb_libovsdb_la_LDFLAGS = \
--version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) \
+$(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/ovsdb/libovsdb.sym \
 $(AM_LDFLAGS)
 ovsdb_libovsdb_la_SOURCES = \
diff --git a/vtep/automake.mk 

Re: [ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink

[Pravin Shelar]

On Tue, Jan 10, 2017 at 8:14 PM, Jiri Benc  wrote:
> On Tue, 10 Jan 2017 19:29:21 +0530, Pravin Shelar wrote:
>> OVS kernel module has compile time checks for various kernel features,
>> if any of required tunnel feature is missing OVS kernel module
>> compiles in support for its own tunnel implementation. This compat
>> tunnel implementation is exposed as ovs_* tunnel device.
>> Therefore if ovs_geneve device availability shows that current kernel
>> tunnel device does not support all features and we should use OVS
>> compat tunnel implementation. To use compat-tunnel implementation we
>> have to use gnetlink interface. OVS compat tunnels code do not support
>> LWT interface.
>
> I've been trying to wrap my head around this for some time already but
> I'm afraid I may still not understand what you mean.
>
> By "exposed as ovs_* tunnel device", do you mean rtnetlink kind? If so,
> then the steps to create the interface (a geneve tunnel in this example)
> would be:
>
> create an ovs_geneve interface using rtnetlink
> if successful {
> delete the created interface
> create geneve interface using genetlink
> if successful {
> done
> } else {
> fail
> }
> } else {
> create lwtunnel geneve interface using rtnetlink
> check parameters of the created interface
> if it is lwtunnel {
> done
> } else {
> delete the created interface
> create geneve interface using genetlink
> if successful {
> done
> } else {
> fail
> }
> }
> }
>
> Is it what you have in mind?
>
Yes. This looks good to me.

Once it is done for very first tunnel device we do not need to repeat
it for any other tunnel device irrespective of the type. If OVS is
using LWT for one type then all other tunnel type are managed over LWT
interface and Vice Versa.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink

[Jiri Benc]

On Tue, 10 Jan 2017 15:44:07 +0100, Jiri Benc wrote:
> create an ovs_geneve interface using rtnetlink

This can be done just once. The pseudocode at startup thus would be:

create an ovs_geneve interface using rtnetlink
if successful {
delete the created interface
set out_of_tree flag
}

And interface creation:

if not out_of_tree {
create lwtunnel geneve interface using rtnetlink
check parameters of the created interface
if it is lwtunnel {
done, exit
} else {
delete the created interface
}
}
create geneve interface using genetlink
if successful {
done
} else {
fail
}

Is that what you want? If so, should the out_of_tree flag be queried
and set per tunnel type, or just based globally on ovs_geneve (or a
different kind)?

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

Re: [ovs-dev] [PATCH ovs V2 12/21] dpif-netlink: Use netdev flow put api to insert a flow

[Paul Blakey]

On 06/01/2017 01:28, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Using the new netdev flow api operate will now try and
offload flows to the relevant netdev of the input port.
Other operate methods flows will come in later patches.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
  lib/dpif-netlink.c | 232 -
  1 file changed, 228 insertions(+), 4 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 3d8940e..717af90 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1908,15 +1908,239 @@ dpif_netlink_operate__(struct dpif_netlink *dpif,
  return n_ops;
  }

+static int
+parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
+const struct nlattr *mask, size_t mask_len,
+struct match *match)
+{
+enum odp_key_fitness fitness;
+
+fitness = odp_flow_key_to_flow(key, key_len, >flow);
+if (fitness) {
+/* This should not happen: it indicates that odp_flow_key_from_flow()
+ * and odp_flow_key_to_flow() disagree on the acceptable form of a
+ * flow.  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()) {
+struct ds s;
+
+ds_init();
+odp_flow_format(key, key_len, NULL, 0, NULL, , true);
+VLOG_ERR("internal error parsing flow key %s", ds_cstr());
+ds_destroy();
+}
+
+return EINVAL;
+}
+
+fitness = odp_flow_key_to_mask(mask, mask_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()) {
+struct ds s;
+
+VLOG_ERR("internal error parsing flow mask %s (%s)",
+ ds_cstr(), odp_key_fitness_to_string(fitness));
+ds_destroy();
+}
+
+return EINVAL;
+}
+
+return 0;
+}
+
+static bool
+parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
+{
+struct match match;
+odp_port_t in_port;
+const struct nlattr *nla;
+size_t left;
+int outputs = 0;
+struct ofpbuf buf;
+uint64_t act_stub[1024 / 8];
+size_t offset;
+struct nlattr *act;
+struct netdev *dev;
+int err;
+
+/* 0x1234 - fake eth type sent to probe feature */
+if (put->flags & DPIF_FP_PROBE || match.flow.dl_type == htons(0x1234)) {
+return false;
+}
+
+if (parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
+put->mask_len, )) {
+return false;
+}
+
+in_port = match.flow.in_port.odp_port;
+ofpbuf_use_stub(, act_stub, sizeof act_stub);
+offset = nl_msg_start_nested(, OVS_FLOW_ATTR_ACTIONS);
+NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) {
+if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
+struct netdev *outdev;
+int ifindex_out;
+const struct netdev_tunnel_config *tnl_cfg;
+size_t out_off;
+odp_port_t out_port;
+
+outputs++;
+if (outputs > 1) {
+break;
+}
+
+out_port = nl_attr_get_u32(nla);
+outdev = netdev_hmap_port_get(out_port, dpif->dpif.dpif_class);
+tnl_cfg = netdev_get_tunnel_config(outdev);
+
+out_off = nl_msg_start_nested(, OVS_ACTION_ATTR_OUTPUT);
+ifindex_out = netdev_get_ifindex(outdev);
+nl_msg_put_u32(, OVS_ACTION_ATTR_OUTPUT, ifindex_out);
+if (tnl_cfg && tnl_cfg->dst_port != 0) {
+nl_msg_put_u32(, OVS_TUNNEL_KEY_ATTR_TP_DST, 
tnl_cfg->dst_port);
+}
+nl_msg_end_nested(, out_off);
+
+if (outdev) {
+netdev_close(outdev);
+}
+} else {
+nl_msg_put_unspec(, nl_attr_type(nla), nl_attr_get(nla),
+  nl_attr_get_size(nla));
+}
+}
+nl_msg_end_nested(, offset);
+
+if (outputs > 1) {
+return false;
+}
+
+act = ofpbuf_at_assert(, offset, sizeof(struct nlattr));
+dev = netdev_hmap_port_get(in_port, dpif->dpif.dpif_class);
+err = netdev_flow_put(dev, , CONST_CAST(struct nlattr *,
+  nl_attr_get(act)),
+  nl_attr_get_size(act), put->stats,
+  CONST_CAST(ovs_u128 *, put->ufid));
+netdev_close(dev);
+
+if (!err) {
+if 

Re: [ovs-dev] [RFC PATCH] datapath: allow tunnels to be created with rtnetlink

[Pravin Shelar]

On Tue, Jan 10, 2017 at 3:23 AM, Eric Garver  wrote:
> On Fri, Dec 09, 2016 at 03:12:39PM -0800, Pravin Shelar wrote:
>> On Fri, Dec 9, 2016 at 12:43 AM, Jiri Benc  wrote:
>> > On Thu, 8 Dec 2016 22:49:56 -0800, Pravin Shelar wrote:
>> >> OVS out of tree kernel module is using compat tunnel code upto kernel
>> >> 4.5 kernel even thought LWT is available in these kernels. This is due
>> >> to missing features on these kernel which are backported to OVS
>> >> module. In future we could bump up requirements of kernel again.
>> >> Therefore I think we could add compat code for GPE given it is not
>> >> that complicated.
>> >
>> > What I'm concerned about is not so much the code in the out of tree
>> > module but the added code that would have to try genetlink for
>> > VXLAN-GPE. I hoped to use rtnetlink only for this; adding genetlink
>> > would required quite a bit of code (also because it will have to be
>> > tried only for the out of tree module but never for the in kernel one;
>> > this differs from what we'll do for VXLAN itself).
>> >
>> > We'd end up with three different handling for different features:
>> >
>> > (1) genetlink+rtnetlink for both out of tree and in tree module (for
>> > e.g. plain VXLAN)
>> > (2) genetlink+rtnetlink for out of tree module, rtnetlink for in three
>> > module (for VXLAN-GPE)
>> > (3) rtnetlink for both out of tree and in tree module (new VXLAN
>> > features added in the future)
>> >
>> I see where the confusion is.
>>
>> If you are using OVS tunnel compat code then you can not use LWT
>> interfaces even if kernel supports LWT. So you just need to detect
>> what out of tree kernel module is using and accordingly use genetlink
>> or rtnetlink interface.
>> If you read this thread you would see discussion related to this and
>> you would be able to avoid this complexity.
>>
>> So from userspace you just need to detect if you can create ovs_geneve
>> interface if it is successfull use existing genetlink code as it is
>
> I don't follow why probing for ovs_geneve should be our indication to
> try genetlink first. Can you elaborate?
>

OVS kernel module has compile time checks for various kernel features,
if any of required tunnel feature is missing OVS kernel module
compiles in support for its own tunnel implementation. This compat
tunnel implementation is exposed as ovs_* tunnel device.
Therefore if ovs_geneve device availability shows that current kernel
tunnel device does not support all features and we should use OVS
compat tunnel implementation. To use compat-tunnel implementation we
have to use gnetlink interface. OVS compat tunnels code do not support
LWT interface.

>> for ALL tunnel type, if it fails move to rtnetlink interface for ALL
>> tunnel types if that fails too then fall back to existing genetlink
>> interface. This works irrespective of OVS kernel module.
>>
>> Therefore is not much difference even if we add compat interface for 
>> VXLAN-GPE.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V2 14/21] netdev-tc-offloads: Netdev flow put implementation using tc api

[Paul Blakey]

On 05/01/2017 23:29, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---

I don't think that there are errors returned for unsupported fields?

Any field that has a nonzero mask for a field that cannot be converted
into TC should result in a flow install failure.

You're right, rules are "cut" right now. We'll fix that.

  lib/netdev-tc-offloads.c | 186 +--
  1 file changed, 180 insertions(+), 6 deletions(-)

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index b4eee98..4acc8ea 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -370,15 +370,189 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
  return false;
  }

+static int
+parse_put_flow_set_action(struct tc_flow *tc_flow, const struct nlattr *set,
+  size_t set_len)
+{
+const struct nlattr *set_attr;
+size_t set_left;
+
+NL_ATTR_FOR_EACH_UNSAFE(set_attr, set_left, set, set_len) {
+if (nl_attr_type(set_attr) == OVS_KEY_ATTR_TUNNEL) {
+const struct nlattr *tunnel = nl_attr_get(set_attr);
+const size_t tunnel_len = nl_attr_get_size(set_attr);
+const struct nlattr *tun_attr;
+size_t tun_left;
+
+tc_flow->set.set = true;
+NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
+switch (nl_attr_type(tun_attr)) {
+case OVS_TUNNEL_KEY_ATTR_ID: {
+tc_flow->set.id = nl_attr_get_be64(tun_attr);
+}
+break;
+case OVS_TUNNEL_KEY_ATTR_IPV4_SRC: {
+tc_flow->set.ipv4_src = nl_attr_get_be32(tun_attr);
+}
+break;
+case OVS_TUNNEL_KEY_ATTR_IPV4_DST: {
+tc_flow->set.ipv4_dst = nl_attr_get_be32(tun_attr);
+}
+break;
+case OVS_TUNNEL_KEY_ATTR_TP_SRC: {
+tc_flow->set.tp_src = nl_attr_get_be16(tun_attr);
+}
+break;
+case OVS_TUNNEL_KEY_ATTR_TP_DST: {
+tc_flow->set.tp_dst = nl_attr_get_be16(tun_attr);
+}
+break;
+}
+}
+} else {
+VLOG_DBG("unsupported set action type: %d",
+ nl_attr_type(set_attr));
+return -1;

Usually we use positive errno values to indicate errors within OVS code.


+}
+}
+return 0;
+}
+
  int
-netdev_tc_flow_put(struct netdev *netdev OVS_UNUSED,
-  struct match *match OVS_UNUSED,
-  struct nlattr *actions OVS_UNUSED,
-  size_t actions_len OVS_UNUSED,
+netdev_tc_flow_put(struct netdev *netdev,
+  struct match *match,
+  struct nlattr *actions,
+  size_t actions_len,
struct dpif_flow_stats *stats OVS_UNUSED,
-  ovs_u128 *ufid OVS_UNUSED)
+  ovs_u128 *ufid)
  {
-return EOPNOTSUPP;
+struct tc_flow tc_flow;
+struct flow *key = >flow;
+struct flow *mask = >wc.masks;
+const struct flow_tnl *tnl = >flow.tunnel;
+struct nlattr *nla;
+size_t left;
+int prio = 0;
+int handle;
+int err;
+
+memset(_flow, 0, sizeof(tc_flow));
+
+if (tnl->tun_id) {
+VLOG_INFO("tun_id %#"PRIx64, ntohll(tnl->tun_id));
+VLOG_DBG("tun_src "IP_FMT" tun_dst "IP_FMT,
+ IP_ARGS(tnl->ip_src), IP_ARGS(tnl->ip_dst));
+VLOG_DBG("tun_tp_src %d, tun_tp_dst %d",
+ ntohs(tnl->tp_src), ntohs(tnl->tp_dst));
+tc_flow.tunnel.id = tnl->tun_id;
+tc_flow.tunnel.ipv4_src = tnl->ip_src;
+tc_flow.tunnel.ipv4_dst = tnl->ip_dst;
+tc_flow.tunnel.tp_src = tnl->tp_src;
+tc_flow.tunnel.tp_dst = tnl->tp_dst;
+tc_flow.tunnel.tunnel = true;
+}
+
+tc_flow.key.eth_type = key->dl_type;
+tc_flow.mask.eth_type = mask->dl_type;
+
+if (mask->vlan_tci) {
+ovs_be16 vid_mask = mask->vlan_tci & htons(VLAN_VID_MASK);
+ovs_be16 pcp_mask = mask->vlan_tci & htons(VLAN_PCP_MASK);
+ovs_be16 cfi = mask->vlan_tci & htons(VLAN_CFI);
+
+if (cfi && key->vlan_tci & htons(VLAN_CFI)
+&& (!vid_mask || vid_mask == htons(VLAN_VID_MASK))
+&& (!pcp_mask || pcp_mask == htons(VLAN_PCP_MASK))
+&& (vid_mask || pcp_mask)) {
+if (vid_mask) {
+tc_flow.key.vlan_id = vlan_tci_to_vid(key->vlan_tci);
+VLOG_DBG("vlan_id: %d\n", tc_flow.key.vlan_id);
+}
+if (pcp_mask) {
+tc_flow.key.vlan_prio = vlan_tci_to_pcp(key->vlan_tci);
+VLOG_DBG("vlan_prio %d\n", 

Re: [ovs-dev] [PATCH ovs V2 09/21] dpif-netlink: Dump netdevs flows on flow dump

[Paul Blakey]

On 05/01/2017 23:27, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

While dumping flows, dump flows that were offloaded to
netdev and parse them back to dpif flow.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
  lib/dpif-netlink.c | 179 +
  1 file changed, 179 insertions(+)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 36f2888..3d8940e 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -38,6 +38,7 @@
  #include "flow.h"
  #include "fat-rwlock.h"
  #include "netdev.h"
+#include "netdev-provider.h"
  #include "netdev-linux.h"
  #include "netdev-vport.h"
  #include "netlink-conntrack.h"
@@ -55,6 +56,7 @@
  #include "unaligned.h"
  #include "util.h"
  #include "openvswitch/vlog.h"
+#include "openvswitch/match.h"

  VLOG_DEFINE_THIS_MODULE(dpif_netlink);
  #ifdef _WIN32
@@ -68,6 +70,8 @@ enum { MAX_PORTS = USHRT_MAX };
   * missing if we have old headers. */
  #define ETH_FLAG_LRO  (1 << 15)/* LRO is enabled */

+#define FLOW_DUMP_MAX_BATCH 50
+
  struct dpif_netlink_dp {
  /* Generic Netlink header. */
  uint8_t cmd;
@@ -1355,6 +1359,10 @@ struct dpif_netlink_flow_dump {
  struct dpif_flow_dump up;
  struct nl_dump nl_dump;
  atomic_int status;
+struct netdev_flow_dump **netdev_dumps;
+int netdev_num;
+int netdev_given;
+struct ovs_mutex netdev_lock;

Could you add a brief comment above these variables that describes
their use? (It's also common in OVS code to mention that, eg,
netdev_lock protects the following elements. )

If there's a more descriptive name than "netdev_num", like
netdev_max_dumps or something then please use that instead. At a
glance, "given" and "num" don't provide particularly much context
about how they relate to each other or to the dump.

sure, thanks.



  };

  static struct dpif_netlink_flow_dump *
@@ -1363,6 +1371,34 @@ dpif_netlink_flow_dump_cast(struct dpif_flow_dump *dump)
  return CONTAINER_OF(dump, struct dpif_netlink_flow_dump, up);
  }

+static void start_netdev_dump(const struct dpif *dpif_,
+  struct dpif_netlink_flow_dump *dump) {
+
+if (!netdev_flow_api_enabled) {
+dump->netdev_num = 0;
+return;
+}

Typically for style we still place all variable declarations at the
top of a function, in a christmas tree long lines to short lines,
before functional code like this.


+
+struct netdev_list_element *element;
+struct ovs_list port_list;
+int ports = netdev_hmap_port_get_list(dpif_->dpif_class, _list);
+int i = 0;
+
+dump->netdev_dumps =
+ports ? xzalloc(sizeof(struct netdev_flow_dump *) * ports) : 0;

Can this be sizeof(dump->netdev_dumps)?
Do you mean sizeof(*dump-netdev_dumps), or 
sizeof(dump->netdev_dumps[0]), if so yes.



+dump->netdev_num = ports;
+dump->netdev_given = 0;
+
+LIST_FOR_EACH(element, node, _list) {
+dump->netdev_dumps[i] = netdev_flow_dump_create(element->netdev);
+dump->netdev_dumps[i]->port = element->port_no;
+i++;
+}

As a matter of style, it's easier to see that this loop is bounded by
'ports' (and that number is correct) if it's structured as

for (i = 0; i < ports; i++) {
element = get_next_node;
...
}

Also, it seems that even if the netdev doesn't support flow_dump, we
allocate a netdev_flow_dump and add it to the netdev_dumps here..
perhaps we could/should skip it for these netdevs instead?


+netdev_port_list_del(_list);
+
+ovs_mutex_init(>netdev_lock);

I don't see a corresponding ovs_mutex_destroy() call for this.

Good catch, thanks.

+}
+
  static struct dpif_flow_dump *
  dpif_netlink_flow_dump_create(const struct dpif *dpif_, bool terse)
  {
@@ -1387,6 +1423,8 @@ dpif_netlink_flow_dump_create(const struct dpif *dpif_, 
bool terse)
  atomic_init(>status, 0);
  dump->up.terse = terse;

+start_netdev_dump(dpif_, dump);
+
  return >up;
  }

@@ -1397,6 +1435,16 @@ dpif_netlink_flow_dump_destroy(struct dpif_flow_dump 
*dump_)
  unsigned int nl_status = nl_dump_done(>nl_dump);
  int dump_status;

+if (netdev_flow_api_enabled) {
+for (int i = 0; i < dump->netdev_num; i++) {
+int err = netdev_flow_dump_destroy(dump->netdev_dumps[i]);
+if (err != 0 && err != EOPNOTSUPP) {
+VLOG_ERR("failed dumping netdev: %s", ovs_strerror(err));
+}
+}
+free(dump->netdev_dumps);
+}

You don't really need to check for netdev_flow_api_enabled here;
netdev_num will be 0 if it is disabled, so that for loop turns into a
no-op; then you could initialize dump->netdev_dumps to NULL in that
case and unconditionally free it. It's a bit simpler to read the code
if you don't have to think about whether or not hardware offloads are
enabled.


+
  /* No other thread has access to 'dump' at this point. */
  

Re: [ovs-dev] [PATCH ovs V2 05/21] dpif: Save added ports in a port map for netdev flow api use

[Paul Blakey]

On 05/01/2017 03:54, Joe Stringer wrote:

On 25 December 2016 at 03:39, Paul Blakey  wrote:

To use netdev flow offloading api, dpifs needs to iterate over
added ports. This addition inserts the added dpif ports in a hash map,
The map will also be used to translate dpif ports to netdevs.

Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
  lib/dpif.c   |  25 +++
  lib/netdev.c | 133 +++
  lib/netdev.h |  14 +++
  3 files changed, 172 insertions(+)

diff --git a/lib/dpif.c b/lib/dpif.c
index 53958c5..c67ea92 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -352,7 +352,22 @@ do_open(const char *name, const char *type, bool create, 
struct dpif **dpifp)
  error = registered_class->dpif_class->open(registered_class->dpif_class,
 name, create, );
  if (!error) {
+struct dpif_port_dump port_dump;
+struct dpif_port dpif_port;
+
  ovs_assert(dpif->dpif_class == registered_class->dpif_class);
+
+DPIF_PORT_FOR_EACH(_port, _dump, dpif) {
+struct netdev *netdev;
+int err = netdev_open(dpif_port.name, dpif_port.type, );
+
+if (!err) {
+netdev_hmap_port_add(netdev, dpif->dpif_class, _port);
+netdev_close(netdev);
+} else {
+VLOG_WARN("could not open netdev %s type %s", name, type);
+}
+}
  } else {
  dp_class_unref(registered_class);
  }
@@ -545,6 +560,14 @@ dpif_port_add(struct dpif *dpif, struct netdev *netdev, 
odp_port_t *port_nop)
  if (!error) {
  VLOG_DBG_RL(_rl, "%s: added %s as port %"PRIu32,
  dpif_name(dpif), netdev_name, port_no);
+
+/* temp dpif_port, will be cloned in netdev_hmap_port_add */
+struct dpif_port dpif_port;
+
+dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev));
+dpif_port.name = CONST_CAST(char *, netdev_name);
+dpif_port.port_no = port_no;
+netdev_hmap_port_add(netdev, dpif->dpif_class, _port);
  } else {
  VLOG_WARN_RL(_rl, "%s: failed to add %s as port: %s",
   dpif_name(dpif), netdev_name, ovs_strerror(error));
@@ -569,6 +592,8 @@ dpif_port_del(struct dpif *dpif, odp_port_t port_no)
  if (!error) {
  VLOG_DBG_RL(_rl, "%s: port_del(%"PRIu32")",
  dpif_name(dpif), port_no);
+
+netdev_hmap_port_del(port_no, dpif->dpif_class->type);
  } else {
  log_operation(dpif, "port_del", error);
  }
diff --git a/lib/netdev.c b/lib/netdev.c
index b289166..2630802 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2080,6 +2080,139 @@ netdev_init_flow_api(struct netdev *netdev)
  : EOPNOTSUPP);
  }

+static struct hmap port_to_netdev = HMAP_INITIALIZER(_to_netdev);
+static struct hmap ifindex_to_port = HMAP_INITIALIZER(_to_port);
+
+struct port_to_netdev_data {
+struct hmap_node node;
+struct netdev *netdev;
+struct dpif_port dpif_port;
+const void *obj;
+};
+
+struct ifindex_to_port_data {
+struct hmap_node node;
+int ifindex;
+odp_port_t port;
+};
+
+int
+netdev_hmap_port_add(struct netdev *netdev, const void *obj,
+ struct dpif_port *dpif_port)
+{
+size_t hash = hash_int(dpif_port->port_no, hash_pointer(obj, 0));
+struct port_to_netdev_data *data = xzalloc(sizeof *data);
+struct ifindex_to_port_data *ifidx = xzalloc(sizeof *ifidx);
+
+netdev_hmap_port_del(dpif_port->port_no, obj);
+
+data->netdev = netdev_ref(netdev);
+data->obj = obj;
+dpif_port_clone(>dpif_port, dpif_port);
+
+ifidx->ifindex = netdev_get_ifindex(netdev);
+ifidx->port = dpif_port->port_no;
+
+hmap_insert(_to_netdev, >node, hash);
+hmap_insert(_to_port, >node, ifidx->ifindex);
+
+return 0;
+}
+
+struct netdev *
+netdev_hmap_port_get(odp_port_t port_no, const void *obj)
+{
+size_t hash = hash_int(port_no, hash_pointer(obj, 0));
+struct port_to_netdev_data *data;
+
+HMAP_FOR_EACH_WITH_HASH(data, node, hash, _to_netdev) {
+if (data->obj == obj && data->dpif_port.port_no == port_no) {
+break;
+}
+}
+
+if (data) {
+netdev_ref(data->netdev);
+return data->netdev;
+}
+
+return 0;
+}
+
+odp_port_t
+netdev_hmap_port_get_byifidx(int ifindex)

netdev_hmap_get_port_by_ifindex?


+{
+struct ifindex_to_port_data *data;
+
+HMAP_FOR_EACH_WITH_HASH(data, node, ifindex, _to_port) {
+if (data->ifindex == ifindex) {
+return data->port;
+}
+}
+
+return 0;
+}
+
+int
+netdev_hmap_port_del(odp_port_t port_no, const void *obj)
+{
+size_t hash = hash_int(port_no, hash_pointer(obj, 0));
+struct port_to_netdev_data *data;
+
+HMAP_FOR_EACH_WITH_HASH(data, node, hash, _to_netdev) {
+

Re: [ovs-dev] [PATCH v2 5/5] datapath-windows: Add support for OVS_KEY_ATTR_TCP set action

[Alin Serdean]

Hi Sai,

Thanks for the review! Comments inlined.

> -Original Message-
> From: Sairam Venugopal [mailto:vsai...@vmware.com]
> Sent: Tuesday, January 10, 2017 3:40 AM
> To: Alin Serdean ;
> d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v2 5/5] datapath-windows: Add support for
> OVS_KEY_ATTR_TCP set action
> 
> Please see my comment inline:
> >+
> >+ASSERT(layers->value != 0);
> >+
> >+if (!layers->isUdp) {
> 
> Sai: This is incorrect. Should be checking for !layers->isTcp. This will end 
> up
> failing for tcp set action.
> Can you consolidate TCP/UDP actions to call into 1 function and use switch
> instead. Most of the code has been copied over from UDP.
[Alin Serdean] Sorry I missed that! I think it is more readable this way and 
can be reused under different circumstances. Do you want me to try to move more 
code outside of the functions?
> 

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