Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

2019-03-21 Thread Gregory Rose 



On 3/21/2019 5:38 PM, Gregory Rose wrote:



On 3/21/2019 10:37 AM, Numan Siddique wrote:

This is the datapath patch - https://patchwork.ozlabs.org/patch/1046370/
and this is the corresponding ovs-vswitchd patch - 
https://patchwork.ozlabs.org/patch/1059081/ (this is part of the 
series - 
https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190, 
but probably you would be interested in only ovs patch)


Sharing the links so that you can find it easily.

Thanks
Numan




This patch:

https://patchwork.ozlabs.org/patch/1059081/

shows this when applied:

Applying: Add a new OVS action check_pkt_larger
.git/rebase-apply/patch:1097: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

In regards to the datapath patch 1046370 



In execute_check_pkt_len():

+
+   actual_pkt_len = skb->len + (skb_vlan_tag_present(skb) ? 
VLAN_HLEN : 0);

+

This doesn't seem right to me - the skb length should include the 
length of the entire packet, including any

VLAN tags, or at least that is my understanding.  Please check it.

In validate_and_copy_check_pkt_len() in flow_netlink.c:

+   static const struct nla_policy pol[OVS_CHECK_PKT_LEN_ATTR_MAX 
+ 1] = {

+   [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NLA_U16 },
+   [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {
+   .type = NLA_NESTED },
+   [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {
+   .type = NLA_NESTED },
+   };

I don't care for declaring these things within function scope and it 
is not generally done.  I see that
flow_netlink.c has one other instance of the nla_policy structure 
statically declared within the function scope
but if you look at datapath.c none of them are.  I prefer the way it's 
done in datapath.c.  I also grepped around
in other kernel code in the ./net tree and that is also the way it's 
done there, i.e. I didn't see any other

instances of it declared within function scope.

I compiled both the ovs-vswitchd and openvswitch kernel module 
components with no issues.  I wanted to use
clang but the version of clang on Ubuntu right now doesn't have 
retpoline support so it won't compile

kernel modules.

:-/

I did some quick regression testing and found no problems.  If you can 
address the two coding issues I brought up

then I'd be glad to add my reviewed and tested by tags.


Oh wait, this is just an RFC.

I'll review and test the patches again when they officially come out.  
Maybe clang will have retpoline support

by then.

- Greg

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

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

2019-03-21 Thread Gregory Rose 



On 3/21/2019 10:37 AM, Numan Siddique wrote:

This is the datapath patch - https://patchwork.ozlabs.org/patch/1046370/
and this is the corresponding ovs-vswitchd patch - 
https://patchwork.ozlabs.org/patch/1059081/ (this is part of the 
series - 
https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190, 
but probably you would be interested in only ovs patch)


Sharing the links so that you can find it easily.

Thanks
Numan




This patch:

https://patchwork.ozlabs.org/patch/1059081/

shows this when applied:

Applying: Add a new OVS action check_pkt_larger
.git/rebase-apply/patch:1097: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

In regards to the datapath patch 1046370 



In execute_check_pkt_len():

+
+   actual_pkt_len = skb->len + (skb_vlan_tag_present(skb) ? 
VLAN_HLEN : 0);

+

This doesn't seem right to me - the skb length should include the length 
of the entire packet, including any

VLAN tags, or at least that is my understanding.  Please check it.

In validate_and_copy_check_pkt_len() in flow_netlink.c:

+   static const struct nla_policy pol[OVS_CHECK_PKT_LEN_ATTR_MAX + 
1] = {

+   [OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = {.type = NLA_U16 },
+   [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = {
+   .type = NLA_NESTED },
+   [OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL] = {
+   .type = NLA_NESTED },
+   };

I don't care for declaring these things within function scope and it is 
not generally done.  I see that
flow_netlink.c has one other instance of the nla_policy structure 
statically declared within the function scope
but if you look at datapath.c none of them are.  I prefer the way it's 
done in datapath.c.  I also grepped around
in other kernel code in the ./net tree and that is also the way it's 
done there, i.e. I didn't see any other

instances of it declared within function scope.

I compiled both the ovs-vswitchd and openvswitch kernel module 
components with no issues.  I wanted to use
clang but the version of clang on Ubuntu right now doesn't have 
retpoline support so it won't compile

kernel modules.

:-/

I did some quick regression testing and found no problems.  If you can 
address the two coding issues I brought up

then I'd be glad to add my reviewed and tested by tags.

Thanks!

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

Re: [ovs-dev] [ovs-dev,1/2] ovn-ctl: Unify OVN_RUNDIR usage.

2019-03-21 Thread 0-day Robot 
Bleep bloop.  Greetings Han Zhou, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 113 characters long (recommended limit is 79)
#41 FILE: ovn/utilities/ovn-ctl:64:
ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`

WARNING: Line is 87 characters long (recommended limit is 79)
#42 FILE: ovn/utilities/ovn-ctl:65:
ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/connect-active-ovsdb-server

WARNING: Line is 113 characters long (recommended limit is 79)
#52 FILE: ovn/utilities/ovn-ctl:78:
ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`

WARNING: Line is 87 characters long (recommended limit is 79)
#53 FILE: ovn/utilities/ovn-ctl:79:
ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/connect-active-ovsdb-server

WARNING: Line is 86 characters long (recommended limit is 79)
#62 FILE: ovn/utilities/ovn-ctl:88:
ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server

WARNING: Line is 86 characters long (recommended limit is 79)
#68 FILE: ovn/utilities/ovn-ctl:93:
ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server

WARNING: Line is 98 characters long (recommended limit is 79)
#77 FILE: ovn/utilities/ovn-ctl:247:
ovs-appctl -t $OVN_RUNDIR/ovn${1}_db.ctl ovsdb-server/sync-status | awk 
'{if(NR==1) print $2}'

Lines checked: 127, Warnings: 7, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

[ovs-dev] [PATCH 2/2] ovn-ctl: Make sure OVN_RUNDIR is created for central nodes.

2019-03-21 Thread Han Zhou 
From: Han Zhou 

When ovn-ctl tries to start ovsdb, it didn't ensure the rundir
(e.g. /var/run/openvswitch) exist, because it is not calling
start_daemon(). Usually, if OVS is started by ovs-ctl before
on the same node, the folder is created already. However, for
OVN central node, OVS is usually not needed. If the folder is
not created (it is common case when system restarted because
/var/run is usually tmpfs), ovn-ctl will fail to start ovsdb.
This patch always ensures the OVN_RUNDIR is created.

Signed-off-by: Han Zhou 
---
 ovn/utilities/ovn-ctl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 83f026e..cca5fac 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -144,6 +144,7 @@ start_ovsdb__() {
 eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
 eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
 
+install_dir "$OVN_RUNDIR"
 # Check and eventually start ovsdb-server for DB
 if pidfile_is_running $db_pid_file; then
 return
-- 
2.1.0

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

[ovs-dev] [PATCH 1/2] ovn-ctl: Unify OVN_RUNDIR usage.

2019-03-21 Thread Han Zhou 
From: Han Zhou 

In this script $rundir and $OVN_RUNDIR is used in a mixed way, which
can cause different folders used for different runtime files. This
patch unifies the usage to the correct one.

Signed-off-by: Han Zhou 
---
 ovn/utilities/ovn-ctl | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 9e49d1d..83f026e 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -40,13 +40,13 @@ pidfile_is_running () {
 
 stop_nb_ovsdb() {
 if pidfile_is_running $DB_NB_PID; then
-ovs-appctl -t $rundir/ovnnb_db.ctl exit
+ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl exit
 fi
 }
 
 stop_sb_ovsdb() {
 if pidfile_is_running $DB_SB_PID; then
-ovs-appctl -t $rundir/ovnsb_db.ctl exit
+ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl exit
 fi
 }
 
@@ -61,8 +61,8 @@ demote_ovnnb() {
 fi
 
 if test -e $ovnnb_active_conf_file; then
-ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`
-ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
+ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`
+ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
 else
 echo >&2 "$0: active server details not set"
 exit 1
@@ -75,8 +75,8 @@ demote_ovnsb() {
 fi
 
 if test -e $ovnsb_active_conf_file; then
-ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`
-ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
+ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`
+ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/connect-active-ovsdb-server
 else
 echo >&2 "$0: active server details not set"
 exit 1
@@ -85,12 +85,12 @@ demote_ovnsb() {
 
 promote_ovnnb() {
 rm -f $ovnnb_active_conf_file
-ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+ovs-appctl -t $OVN_RUNDIR/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
 }
 
 promote_ovnsb() {
 rm -f $ovnsb_active_conf_file
-ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+ovs-appctl -t $OVN_RUNDIR/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
 }
 
 start_ovsdb__() {
@@ -244,7 +244,7 @@ start_ovsdb () {
 }
 
 sync_status() {
-ovs-appctl -t $rundir/ovn${1}_db.ctl ovsdb-server/sync-status | awk 
'{if(NR==1) print $2}'
+ovs-appctl -t $OVN_RUNDIR/ovn${1}_db.ctl ovsdb-server/sync-status | awk 
'{if(NR==1) print $2}'
 }
 
 status_ovnnb() {
@@ -428,8 +428,11 @@ restart_sb_ovsdb () {
 set_defaults () {
 OVN_MANAGE_OVSDB=yes
 
-DB_NB_SOCK=$rundir/ovnnb_db.sock
-DB_NB_PID=$rundir/ovnnb_db.pid
+OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
+OVN_RUNDIR=${OVN_RUNDIR:-${OVS_RUNDIR}}
+
+DB_NB_SOCK=$OVN_RUNDIR/ovnnb_db.sock
+DB_NB_PID=$OVN_RUNDIR/ovnnb_db.pid
 DB_NB_FILE=$dbdir/ovnnb_db.db
 DB_NB_ADDR=0.0.0.0
 DB_NB_PORT=6641
@@ -437,8 +440,8 @@ set_defaults () {
 DB_NB_SYNC_FROM_ADDR=
 DB_NB_SYNC_FROM_PORT=6641
 
-DB_SB_SOCK=$rundir/ovnsb_db.sock
-DB_SB_PID=$rundir/ovnsb_db.pid
+DB_SB_SOCK=$OVN_RUNDIR/ovnsb_db.sock
+DB_SB_PID=$OVN_RUNDIR/ovnsb_db.pid
 DB_SB_FILE=$dbdir/ovnsb_db.db
 DB_SB_ADDR=0.0.0.0
 DB_SB_PORT=6642
@@ -449,7 +452,7 @@ set_defaults () {
 DB_NB_SCHEMA=$datadir/ovn-nb.ovsschema
 DB_SB_SCHEMA=$datadir/ovn-sb.ovsschema
 
-DB_SOCK=$rundir/db.sock
+DB_SOCK=$OVN_RUNDIR/db.sock
 DB_CONF_FILE=$dbdir/conf.db
 
 OVN_NORTHD_PRIORITY=-10
@@ -457,8 +460,6 @@ set_defaults () {
 OVN_CONTROLLER_PRIORITY=-10
 OVN_CONTROLLER_WRAPPER=
 
-OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
-OVN_RUNDIR=${OVN_RUNDIR:-${OVS_RUNDIR}}
 OVN_USER=
 OVS_USER=
 
-- 
2.1.0

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

[ovs-dev] (no subject)

2019-03-21 Thread Golden Gate Company via dev 
I am Li Xu from China. I am looking for your partnership because of the 
deteriorating security situation and the prospect of a civil war, urgent 
removal of my family from Iraq if the US finally withdraws their troops 
Partnership for me to leave Iraq with my family For your interest show or 
request more information, contact me
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Hi

2019-03-21 Thread Espoir 

Hello ,
could we be friends? I would love to meet you ..
Greeting

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

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

2019-03-21 Thread Numan Siddique 
On Thu, Mar 21, 2019 at 11:01 PM Gregory Rose  wrote:

>
> On 3/21/2019 8:23 AM, Numan Siddique wrote:
>
>
>
> On Mon, Feb 25, 2019 at 12:09 PM Numan Siddique 
> wrote:
>
>>
>>
>> On Sat, Feb 23, 2019 at 2:35 AM Gregory Rose 
>> wrote:
>>
>>> Numan,
>>>
>>> I intend to test and review this on my local net-next branch but I'm not
>>> feeling well so it will probably be
>>> early next week before I can do that.  Sorry for the delay...
>>>
>>>
>> No worries. Please take your time.
>> If you want to test it out, you probably need the corresponding
>> ovs-vswitchd patch.
>> I still need to address the review comments from Ben. But you can use the
>> one from here -
>> https://github.com/numansiddique/ovs/tree/ovn_mtu_fix/rfc_v4_p4
>> https://github.com/numansiddique/ovs/commits/ovn_mtu_fix/rfc_v4_p4 for
>> your testing.
>>
>>
>
> Hi Greg,
> Gentle ping to see if you got a chance to try this out.
>
> FYI - RFC v4 of the corresponding ovs patch is submitted here -
> https://patchwork.ozlabs.org/patch/1059081/
>
> Thanks
> Numan
>
>
> Gah!  No, that's my bad.  It fell off my radar.  I'll look at the new
> patch.
>
> Thanks for the reminder.
>


Thanks.
This is the datapath patch - https://patchwork.ozlabs.org/patch/1046370/
and this is the corresponding ovs-vswitchd patch -
https://patchwork.ozlabs.org/patch/1059081/ (this is part of the series -
https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190, but
probably you would be interested in only ovs patch)

Sharing the links so that you can find it easily.

Thanks
Numan


> - Greg
>
>
> Thanks
>> Numan
>>
>>
>>
>> - Greg
>>>
>>> On 2/21/2019 10:42 AM, nusid...@redhat.com wrote:
>>> > From: Numan Siddique 
>>> >
>>> > [Please note, this patch is submitted as RFC in ovs-dev ML to
>>> > get feedback before submitting to netdev ML. You need net-next tree
>>> > to apply this patch]
>>> >
>>> > This patch adds a new action - 'check_pkt_len' which checks the
>>> > packet length and executes a set of actions if the packet
>>> > length is greater than the specified length or executes
>>> > another set of actions if the packet length is lesser or equal to.
>>> >
>>> > This action takes below nlattrs
>>> >* OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for
>>> >
>>> >* OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER - Nested actions
>>> >  to apply if the packet length is greater than the specified
>>> 'pkt_len'
>>> >
>>> >* OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
>>> >  actions to apply if the packet length is lesser or equal to the
>>> >  specified 'pkt_len'.
>>> >
>>> > The main use case for adding this action is to solve the packet
>>> > drops because of MTU mismatch in OVN virtual networking solution.
>>> > When a VM (which belongs to a logical switch of OVN) sends a packet
>>> > destined to go via the gateway router and if the nic which provides
>>> > external connectivity, has a lesser MTU, OVS drops the packet
>>> > if the packet length is greater than this MTU.
>>> >
>>> > With the help of this action, OVN will check the packet length
>>> > and if it is greater than the MTU size, it will generate an
>>> > ICMP packet (type 3, code 4) and includes the next hop mtu in it
>>> > so that the sender can fragment the packets.
>>> >
>>> > Reported-at:
>>> >
>>> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
>>> > Suggested-by: Ben Pfaff 
>>> > Signed-off-by: Numan Siddique 
>>> > CC: Ben Pfaff 
>>> > CC: Greg Rose 
>>> > CC: Lorenzo Bianconi 
>>> > ---
>>> >
>>> > v3 -> v4
>>> > 
>>> >   * v4 only has 1 patch - datapath patch which implements the
>>> >   * check_pkt_len action
>>> >   * Addressed the review comments from Lorenzo, Ben and Greg
>>> >
>>> >
>>> >   include/uapi/linux/openvswitch.h |  42 
>>> >   net/openvswitch/actions.c|  49 +
>>> >   net/openvswitch/flow_netlink.c   | 171
>>> +++
>>> >   3 files changed, 262 insertions(+)
>>> >
>>> > diff --git a/include/uapi/linux/openvswitch.h
>>> b/include/uapi/linux/openvswitch.h
>>> > index dbe0cbe4f1b7..05ab885c718d 100644
>>> > --- a/include/uapi/linux/openvswitch.h
>>> > +++ b/include/uapi/linux/openvswitch.h
>>> > @@ -798,6 +798,44 @@ struct ovs_action_push_eth {
>>> >   struct ovs_key_ethernet addresses;
>>> >   };
>>> >
>>> > +/*
>>> > + * enum ovs_check_pkt_len_attr - Attributes for
>>> %OVS_ACTION_ATTR_CHECK_PKT_LEN.
>>> > + *
>>> > + * @OVS_CHECK_PKT_LEN_ATTR_PKT_LEN: u16 Packet length to check for.
>>> > + * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER: Nested
>>> OVS_ACTION_ATTR_*
>>> > + * actions to apply if the packer length is greater than the specified
>>> > + * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
>>> > + * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
>>> OVS_ACTION_ATTR_*
>>> > + * actions to apply if the packer length is lesser or equal to the
>>> specified
>>> > + * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
>>> > + */
>>> >

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

2019-03-21 Thread Gregory Rose 


On 3/21/2019 8:23 AM, Numan Siddique wrote:



On Mon, Feb 25, 2019 at 12:09 PM Numan Siddique > wrote:




On Sat, Feb 23, 2019 at 2:35 AM Gregory Rose mailto:gvrose8...@gmail.com>> wrote:

Numan,

I intend to test and review this on my local net-next branch
but I'm not
feeling well so it will probably be
early next week before I can do that.  Sorry for the delay...


No worries. Please take your time.
If you want to test it out, you probably need the corresponding
ovs-vswitchd patch.
I still need to address the review comments from Ben. But you can
use the one from here -
https://github.com/numansiddique/ovs/tree/ovn_mtu_fix/rfc_v4_p4
https://github.com/numansiddique/ovs/commits/ovn_mtu_fix/rfc_v4_p4
for your testing.



Hi Greg,
Gentle ping to see if you got a chance to try this out.

FYI - RFC v4 of the corresponding ovs patch is submitted here - 
https://patchwork.ozlabs.org/patch/1059081/


Thanks
Numan


Gah!  No, that's my bad.  It fell off my radar.  I'll look at the new patch.

Thanks for the reminder.

- Greg



Thanks
Numan


- Greg

On 2/21/2019 10:42 AM, nusid...@redhat.com
 wrote:
> From: Numan Siddique mailto:nusid...@redhat.com>>
>
> [Please note, this patch is submitted as RFC in ovs-dev ML to
> get feedback before submitting to netdev ML. You need
net-next tree
> to apply this patch]
>
> This patch adds a new action - 'check_pkt_len' which checks the
> packet length and executes a set of actions if the packet
> length is greater than the specified length or executes
> another set of actions if the packet length is lesser or
equal to.
>
> This action takes below nlattrs
>    * OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for
>
>    * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER - Nested actions
>      to apply if the packet length is greater than the
specified 'pkt_len'
>
>    * OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
>      actions to apply if the packet length is lesser or
equal to the
>      specified 'pkt_len'.
>
> The main use case for adding this action is to solve the packet
> drops because of MTU mismatch in OVN virtual networking
solution.
> When a VM (which belongs to a logical switch of OVN) sends a
packet
> destined to go via the gateway router and if the nic which
provides
> external connectivity, has a lesser MTU, OVS drops the packet
> if the packet length is greater than this MTU.
>
> With the help of this action, OVN will check the packet length
> and if it is greater than the MTU size, it will generate an
> ICMP packet (type 3, code 4) and includes the next hop mtu in it
> so that the sender can fragment the packets.
>
> Reported-at:
>
https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
> Suggested-by: Ben Pfaff mailto:b...@ovn.org>>
> Signed-off-by: Numan Siddique mailto:nusid...@redhat.com>>
> CC: Ben Pfaff mailto:b...@ovn.org>>
> CC: Greg Rose mailto:gvrose8...@gmail.com>>
> CC: Lorenzo Bianconi mailto:lorenzo.bianc...@redhat.com>>
> ---
>
> v3 -> v4
> 
>   * v4 only has 1 patch - datapath patch which implements the
>   * check_pkt_len action
>   * Addressed the review comments from Lorenzo, Ben and Greg
>
>
>   include/uapi/linux/openvswitch.h |  42 
>   net/openvswitch/actions.c        |  49 +
>   net/openvswitch/flow_netlink.c   | 171
+++
>   3 files changed, 262 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h
b/include/uapi/linux/openvswitch.h
> index dbe0cbe4f1b7..05ab885c718d 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -798,6 +798,44 @@ struct ovs_action_push_eth {
>       struct ovs_key_ethernet addresses;
>   };
>
> +/*
> + * enum ovs_check_pkt_len_attr - Attributes for
%OVS_ACTION_ATTR_CHECK_PKT_LEN.
> + *
> + * @OVS_CHECK_PKT_LEN_ATTR_PKT_LEN: u16 Packet length to
check for.
> + * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER: Nested
OVS_ACTION_ATTR_*
> + * actions to apply if the packer length is greater than
the specified
> + * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
> + * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
OVS_ACTION_ATTR_*
> + *

Re: [ovs-dev] [ovs-dev, v4] ovsdb raft: Sync commit index to followers without delay.

2019-03-21 Thread Han Zhou 
On Wed, Mar 20, 2019 at 11:07 AM Ben Pfaff  wrote:
>
> On Wed, Mar 20, 2019 at 10:44:55AM -0700, Han Zhou wrote:
> > On Wed, Mar 20, 2019 at 9:56 AM Ben Pfaff  wrote:
> > >
> > > On Wed, Mar 20, 2019 at 08:28:50AM -0700, Han Zhou wrote:
> > > > On Wed, Mar 20, 2019 at 5:02 AM Ilya Maximets  
> > > > wrote:
> > > > >
> > > > > On 20.03.2019 8:56, Han Zhou wrote:
> > > > > > From: Han Zhou 
> > > > > >
> > > > > > When update is requested from follower, the leader sends 
> > > > > > AppendRequest
> > > > > > to all followers and wait until AppendReply received from majority, 
> > > > > > and
> > > > > > then it will update commit index - the new entry is regarded as 
> > > > > > committed
> > > > > > in raft log. However, this commit will not be notified to followers
> > > > > > (including the one initiated the request) until next heartbeat (ping
> > > > > > timeout), if no other pending requests. This results in long latency
> > > > > > for updates made through followers, especially when a batch of 
> > > > > > updates
> > > > > > are requested through the same follower.
> > > > > >
> > > > > > $ time for i in `seq 1 100`; do ovn-nbctl ls-add ls$i; done
> > > > > >
> > > > > > real0m34.154s
> > > > > > user0m0.083s
> > > > > > sys 0m0.250s
> > > > > >
> > > > > > This patch solves the problem by sending heartbeat as soon as the 
> > > > > > commit
> > > > > > index is updated in leader. It also avoids unnessary heartbeat by 
> > > > > > resetting
> > > > > > the ping timer whenever AppendRequest is broadcasted. With this 
> > > > > > patch
> > > > > > the performance is improved more than 50 times in same test:
> > > > > >
> > > > > > $ time for i in `seq 1 100`; do ovn-nbctl ls-add ls$i; done
> > > > > >
> > > > > > real0m0.564s
> > > > > > user0m0.080s
> > > > > > sys 0m0.199s
> > > > > >
> > > > > > Torture test cases are also updated because otherwise the tests will
> > > > > > all be skipped because of the improved performance.
> > > > > >
> > > > > > Signed-off-by: Han Zhou 
> > > > > > ---
> > > > > >
> > > > > > Notes:
> > > > > > v3->v4: Update torture tests again. Instead of sleeping, the 
> > > > > > size of
> > > > > > transaction of each client is increased to slow down the 
> > > > > > execution so that the
> > > > > > chance of parallel executions are not reduced.
> > > > > >
> > > > >
> > > > > Unfortunately, this patch fails all the testsuite runs on TravisCI:
> > > > >
> > > > >   https://travis-ci.org/ovsrobot/ovs/builds/508777615
> > > > >
> > > > > And some on CirrusCI too:
> > > > >
> > > > >   https://cirrus-ci.com/build/5201766546145280
> > > > >
> > > > > Best regards, Ilya Maximets.
> > > > >
> > > >
> > > > Does the CI retry failed tests? The failed ones are some of the
> > > > torture tests in ovsdb-cluster.at, which was discussed here:
> > > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357373.html
> > > >
> > > > Basically, the failures are real bugs that are not caused by this
> > > > patch code itself, but triggered by the test case change in this
> > > > patch.
> > > >
> > > > The test cases are improved in this patch so that can now find the bug
> > > > that was not found before. To avoid CI failure, we can either merge V3
> > > > (the tests were less effective), or wait until the bug is fixed.
> > >
> > > Both of these do retry failed tests.  You can see the details from the
> > > logs at the URLs that Ilya cited.
> >
> > Yes, checking the log again, I saw the failed torture tests are
> > retried once, and some of them failed again when retrying, which make
> > me more confident for the effectiveness of the updated test cases. I
> > may be distracted today but I will continue debugging tomorrow. I am
> > pretty confident that the bug is not related to the current patch,
> > because it is easy to reproduce the failures such as test 2528 and
> > 2533 with current master applying only the torture test case change.
>
> By the way, I totally support this effort and I'm really looking forward
> to applying the fixes when we figure out how to make the tests both
> effective and pass in the normal case.

Hi Ben, I fixed a reconnection bug which was causing the client IDL
reconnect to same old server after the server is disconnected from
cluster in the torture test:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357443.html
When server was disconnected, it sent out monitor_cancelled message to
client, so the reconnect FSM transitioned back to ACTIVE with same old
server because of the activity on the session. After the fix, the
torture tests all passed in several runs on my laptop with current
patch V4. Please try it and let me know.

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

[ovs-dev] [PATCH] reconnect.c: Don't transition back to ACTIVE when forced to RECONNECT.

2019-03-21 Thread Han Zhou 
From: Han Zhou 

Currently, whenever there is activity on the session, the FSM is
transitioned to ACTIVE. However, this causes reconnect_force_reconnect()
failed to work once there are traffic received from remote after
transition to RECONNECT, it will skip the reconnection phase and directly
go back to ACTIVE for the old session. This patch fixes it so that
when FSM is in RECONNECT state, it doesn't transition back to ACTIVE
directly.

Signed-off-by: Han Zhou 
---
 lib/reconnect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/reconnect.c b/lib/reconnect.c
index 0f21378..e2901ab 100644
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -495,7 +495,7 @@ reconnect_connect_failed(struct reconnect *fsm, long long 
int now, int error)
 void
 reconnect_activity(struct reconnect *fsm, long long int now)
 {
-if (fsm->state != S_ACTIVE) {
+if (fsm->state != S_ACTIVE && fsm->state != S_RECONNECT) {
 reconnect_transition__(fsm, now, S_ACTIVE);
 }
 fsm->last_activity = now;
-- 
2.1.0

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

[ovs-dev] [PATCH net-next] openvswitch: add seqadj extension when NAT is used.

2019-03-21 Thread Flavio Leitner 
When the conntrack is initialized, there is no helper attached
yet so the nat info initialization (nf_nat_setup_info) skips
adding the seqadj ext.

A helper is attached later when the conntrack is not confirmed
but is going to be committed. In this case, if NAT is needed then
adds the seqadj ext as well.

Fixes: 16ec3d4fbb96 ("openvswitch: Fix cached ct with helper.")
Signed-off-by: Flavio Leitner 
---
 net/openvswitch/conntrack.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 1b6896896fff..a7664515c943 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -990,6 +990,11 @@ static int __ovs_ct_lookup(struct net *net, struct 
sw_flow_key *key,
GFP_ATOMIC);
if (err)
return err;
+
+   if (info->nat && nfct_help(ct) && !nfct_seqadj(ct)) {
+   if (!nfct_seqadj_ext_add(ct))
+   return -EINVAL;
+   }
}
 
/* Call the helper only if:
-- 
2.20.1



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

Re: [ovs-dev] [RFC v4 1/1] datapath: Add a new action check_pkt_len

2019-03-21 Thread Numan Siddique 
On Mon, Feb 25, 2019 at 12:09 PM Numan Siddique  wrote:

>
>
> On Sat, Feb 23, 2019 at 2:35 AM Gregory Rose  wrote:
>
>> Numan,
>>
>> I intend to test and review this on my local net-next branch but I'm not
>> feeling well so it will probably be
>> early next week before I can do that.  Sorry for the delay...
>>
>>
> No worries. Please take your time.
> If you want to test it out, you probably need the corresponding
> ovs-vswitchd patch.
> I still need to address the review comments from Ben. But you can use the
> one from here -
> https://github.com/numansiddique/ovs/tree/ovn_mtu_fix/rfc_v4_p4
> https://github.com/numansiddique/ovs/commits/ovn_mtu_fix/rfc_v4_p4 for
> your testing.
>
>

Hi Greg,
Gentle ping to see if you got a chance to try this out.

FYI - RFC v4 of the corresponding ovs patch is submitted here -
https://patchwork.ozlabs.org/patch/1059081/

Thanks
Numan

Thanks
> Numan
>
>
>
> - Greg
>>
>> On 2/21/2019 10:42 AM, nusid...@redhat.com wrote:
>> > From: Numan Siddique 
>> >
>> > [Please note, this patch is submitted as RFC in ovs-dev ML to
>> > get feedback before submitting to netdev ML. You need net-next tree
>> > to apply this patch]
>> >
>> > This patch adds a new action - 'check_pkt_len' which checks the
>> > packet length and executes a set of actions if the packet
>> > length is greater than the specified length or executes
>> > another set of actions if the packet length is lesser or equal to.
>> >
>> > This action takes below nlattrs
>> >* OVS_CHECK_PKT_LEN_ATTR_PKT_LEN - 'pkt_len' to check for
>> >
>> >* OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER - Nested actions
>> >  to apply if the packet length is greater than the specified
>> 'pkt_len'
>> >
>> >* OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
>> >  actions to apply if the packet length is lesser or equal to the
>> >  specified 'pkt_len'.
>> >
>> > The main use case for adding this action is to solve the packet
>> > drops because of MTU mismatch in OVN virtual networking solution.
>> > When a VM (which belongs to a logical switch of OVN) sends a packet
>> > destined to go via the gateway router and if the nic which provides
>> > external connectivity, has a lesser MTU, OVS drops the packet
>> > if the packet length is greater than this MTU.
>> >
>> > With the help of this action, OVN will check the packet length
>> > and if it is greater than the MTU size, it will generate an
>> > ICMP packet (type 3, code 4) and includes the next hop mtu in it
>> > so that the sender can fragment the packets.
>> >
>> > Reported-at:
>> >
>> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-July/047039.html
>> > Suggested-by: Ben Pfaff 
>> > Signed-off-by: Numan Siddique 
>> > CC: Ben Pfaff 
>> > CC: Greg Rose 
>> > CC: Lorenzo Bianconi 
>> > ---
>> >
>> > v3 -> v4
>> > 
>> >   * v4 only has 1 patch - datapath patch which implements the
>> >   * check_pkt_len action
>> >   * Addressed the review comments from Lorenzo, Ben and Greg
>> >
>> >
>> >   include/uapi/linux/openvswitch.h |  42 
>> >   net/openvswitch/actions.c|  49 +
>> >   net/openvswitch/flow_netlink.c   | 171 +++
>> >   3 files changed, 262 insertions(+)
>> >
>> > diff --git a/include/uapi/linux/openvswitch.h
>> b/include/uapi/linux/openvswitch.h
>> > index dbe0cbe4f1b7..05ab885c718d 100644
>> > --- a/include/uapi/linux/openvswitch.h
>> > +++ b/include/uapi/linux/openvswitch.h
>> > @@ -798,6 +798,44 @@ struct ovs_action_push_eth {
>> >   struct ovs_key_ethernet addresses;
>> >   };
>> >
>> > +/*
>> > + * enum ovs_check_pkt_len_attr - Attributes for
>> %OVS_ACTION_ATTR_CHECK_PKT_LEN.
>> > + *
>> > + * @OVS_CHECK_PKT_LEN_ATTR_PKT_LEN: u16 Packet length to check for.
>> > + * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER: Nested OVS_ACTION_ATTR_*
>> > + * actions to apply if the packer length is greater than the specified
>> > + * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
>> > + * @OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL - Nested
>> OVS_ACTION_ATTR_*
>> > + * actions to apply if the packer length is lesser or equal to the
>> specified
>> > + * length in the attr - OVS_CHECK_PKT_LEN_ATTR_PKT_LEN.
>> > + */
>> > +enum ovs_check_pkt_len_attr {
>> > + OVS_CHECK_PKT_LEN_ATTR_UNSPEC,
>> > + OVS_CHECK_PKT_LEN_ATTR_PKT_LEN,
>> > + OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER,
>> > + OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL,
>> > + __OVS_CHECK_PKT_LEN_ATTR_MAX,
>> > +
>> > +#ifdef __KERNEL__
>> > + OVS_CHECK_PKT_LEN_ATTR_ARG  /* struct check_pkt_len_arg
>> */
>> > +#endif
>> > +};
>> > +
>> > +#define OVS_CHECK_PKT_LEN_ATTR_MAX (__OVS_CHECK_PKT_LEN_ATTR_MAX - 1)
>> > +
>> > +#ifdef __KERNEL__
>> > +struct check_pkt_len_arg {
>> > + u16 pkt_len;/* Same value as OVS_CHECK_PKT_LEN_ATTR_PKT_LEN'.
>> */
>> > + bool exec_for_greater;  /* When true, actions in IF_GREATE will
>> > +  * not change flow keys. False

Re: [ovs-dev] [ovs-dev, RFC] vswitchd: warn about misconfigured interface type on netdev bridge

2019-03-21 Thread Ilya Maximets 
On 20.03.2019 21:10, Ben Pfaff wrote:
> On Wed, Mar 20, 2019 at 06:46:22PM +0300, Ilya Maximets wrote:
>> On 20.03.2019 4:20, Ben Pfaff wrote:
>>> On Tue, Mar 19, 2019 at 07:59:55PM +0300, Ilya Maximets wrote:
 On 19.03.2019 18:51, Ilya Maximets wrote:
> On 19.03.2019 12:23, Eelco Chaudron wrote:
>> When debugging an issue we noticed that by accident someone has changes 
>> the
>> bridge datapath_type to netdev, where it's clearly a kernel (system 
>> bridge)
>> with physical and tap devices. Unfortunately, this is not something you
>> will easily spot, as the bridge datapath type value is not shown by 
>> default.
>>
>> In addition, OVS is not warning you about this potential mismatch in
>> interface and bridge datapath.
>>
>> I'm sending out this patch as an RFC as I could not find a clear
>> demarcation between bridge datapath types and interface datapath types.
>> The patch below will at least warn for netdev bridges with system
>> interfaces. But no warning will be given for some unsupported virtual
>> interfaces. For system bridges, the dpdk types will no be recognized as
>> system/virtual interfaces (unless the name exists) which will result in
>> an error.
>>
>> Signed-off-by: Eelco Chaudron 
>> ---
>
> Hi.
>
> Hmm. What do you mean under misconfiguration?
> In practice, userspace datapath is able to open "system" type netdevs.
> It's supported by netdev-linux to open system network devices via socket.
> And these devices has "system" type.
> On 'datapath_type' changes, bridge/ofproto will be destroyed and 
> re-created.
> All the ports from kernel datapath will be destroyed and new ones created
> in userspace. All the ports should still work as usual.
>
> The only possible issue will be that this bridge will no longer 
> communicate
> with other bridges, because they're in different datapaths now.

 For this issue, probably, following warning will give a clue to a user:

 diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
 index 21dd54bab..df51aaa3a 100644
 --- a/ofproto/ofproto-dpif.c
 +++ b/ofproto/ofproto-dpif.c
 @@ -3557,6 +3557,10 @@ ofport_update_peer(struct ofport_dpif *ofport)
  
  break;
  }
 +if (!ofport->peer) {
 +VLOG_WARN_RL(, "No usable peer '%s' exist for patch port 
 '%s'.",
 + peer_name, netdev_get_name(ofport->up.netdev));
 +}
  free(peer_name);
  }
  
 ---

 I could send this as a proper patch.
>>>
>>> In the log message, s/exist/exists/ for English grammar reasons.
>>
>> Sure. Thanks for spotting.
>>
>>>
>>> Even with the log message, the error might not be obvious to the reader.
>>> It would be nice, if a port with the given name existed on a different
>>> backer, to somehow point that out.  It might be hard for this code to
>>> figure it out though.  Maybe it would be helpful to simply note the
>>> datapath type; that might give the reader the hint that the datapath
>>> type could be relevant.
>>
>> I thought about this a little bit more and now I think that message
>> like that will be more confusing than helpful. It's because it will
>> be printed each time while one side of the patch already created but
>> the peer is not added yet. Logs could be confusing unless we reporting
>> successful pairing.
> 
> OK.
> 
>> Maybe it's more appropriate to propagate issue like this to the 'error'
>> column of the interface. But I didn't figure out yet how to do that
>> in a good way.
> 
> You're right, that would be better.  (I don't remember how that
> propagation works.)
> 
>> BTW, maybe the following small change will help for debugging issues
>> like that:
>>
>> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c   
>> 
>> index a36905186..4948137ef 100644
>> 
>> --- a/utilities/ovs-vsctl.c  
>> 
>> +++ b/utilities/ovs-vsctl.c  
>> 
>> @@ -1001,6 +1001,7 @@ static struct cmd_show_table cmd_show_tables[] = { 
>> 
>>   _bridge_col_name,   
>> 
>>   {_bridge_col_controller,
>> 
>>_bridge_col_fail_mode, 
>> 
>> +  _bridge_col_datapath_type,

Re: [ovs-dev] [PATCH] ovs-vsctl: Add datapath_type column to show command.

2019-03-21 Thread Eelco Chaudron 



On 21 Mar 2019, at 11:56, Ilya Maximets wrote:

> Sometimes it's unclear which datapath type is in use by particular
> bridge. For example, if all the interfaces supported by both system
> and netdev datapaths it needs a DB query or log analysis to find out
> which 'datapath_type' is in use.
> Another case is that it's hard to figure out if patch ports are really
> connected to each other. They are definitely not connected if datapath
> types of their bridges differs.
>
> With this change non-default 'datapath_type's will be exposed to
> 'ovs-vsctl show' command, so it'll be easier to spot misconfiguration.
>
>   $ ovs-vsctl show
>   ...
>   Bridge "br0"
>   datapath_type: netdev
>   Port "br0"
>   Interface "br0"
>   type: internal
>   ...
>
> Signed-off-by: Ilya Maximets 

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

[ovs-dev] [PATCH] ovs-vsctl: Add datapath_type column to show command.

2019-03-21 Thread Ilya Maximets 
Sometimes it's unclear which datapath type is in use by particular
bridge. For example, if all the interfaces supported by both system
and netdev datapaths it needs a DB query or log analysis to find out
which 'datapath_type' is in use.
Another case is that it's hard to figure out if patch ports are really
connected to each other. They are definitely not connected if datapath
types of their bridges differs.

With this change non-default 'datapath_type's will be exposed to
'ovs-vsctl show' command, so it'll be easier to spot misconfiguration.

  $ ovs-vsctl show
  ...
  Bridge "br0"
  datapath_type: netdev
  Port "br0"
  Interface "br0"
  type: internal
  ...

Signed-off-by: Ilya Maximets 
---
 utilities/ovs-vsctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index a36905186..4948137ef 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1001,6 +1001,7 @@ static struct cmd_show_table cmd_show_tables[] = {
  _bridge_col_name,
  {_bridge_col_controller,
   _bridge_col_fail_mode,
+  _bridge_col_datapath_type,
   _bridge_col_ports},
  {NULL, NULL, NULL}
 },
-- 
2.17.1

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

[ovs-dev] [PATCH net-next] openvswitch: Make metadata_dst tunnel work in IP_TUNNEL_INFO_BRIDGE mode

2019-03-21 Thread wenxu 
From: wenxu 

There is currently no support for the multicasti/broadcst aspects
of VXLAN in ovs. In the datapath flow the tun_dst must specific.
But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
And the packet can forward through the fdb of vxlan devcice. In
this mode the broadcast/multicast packet can be sent through the
following ways in ovs.

ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
options:key=1000 options:remote_ip=flow
ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff,\
action=output:vxlan

bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
src_vni 1000 vni 1000 self
bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
src_vni 1000 vni 1000 self

Signed-off-by: wenxu 
---
 include/uapi/linux/openvswitch.h |  1 +
 net/openvswitch/flow_netlink.c   | 32 ++--
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index dbe0cbe..696a308 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -364,6 +364,7 @@ enum ovs_tunnel_key_attr {
OVS_TUNNEL_KEY_ATTR_IPV6_DST,   /* struct in6_addr dst IPv6 
address. */
OVS_TUNNEL_KEY_ATTR_PAD,
OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,/* struct erspan_metadata */
+   OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST,/* No argument. No dst IP 
address. */
__OVS_TUNNEL_KEY_ATTR_MAX
 };
 
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 691da85..033df5c 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -403,6 +403,7 @@ size_t ovs_key_attr_size(void)
[OVS_TUNNEL_KEY_ATTR_IPV6_SRC]  = { .len = sizeof(struct in6_addr) 
},
[OVS_TUNNEL_KEY_ATTR_IPV6_DST]  = { .len = sizeof(struct in6_addr) 
},
[OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS]   = { .len = OVS_ATTR_VARIABLE },
+   [OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST]   = { .len = 0 },
 };
 
 static const struct ovs_len_tbl
@@ -663,7 +664,7 @@ static int erspan_tun_opt_from_nlattr(const struct nlattr 
*a,
 
 static int ip_tun_from_nlattr(const struct nlattr *attr,
  struct sw_flow_match *match, bool is_mask,
- bool log)
+ bool log, bool *no_ipv4_dst)
 {
bool ttl = false, ipv4 = false, ipv6 = false;
__be16 tun_flags = 0;
@@ -671,6 +672,9 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
struct nlattr *a;
int rem;
 
+   if (no_ipv4_dst)
+   *no_ipv4_dst = false;
+
nla_for_each_nested(a, attr, rem) {
int type = nla_type(a);
int err;
@@ -782,6 +786,12 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
tun_flags |= TUNNEL_ERSPAN_OPT;
opts_type = type;
break;
+   case OVS_TUNNEL_KEY_ATTR_NO_IPV4_DST:
+   if (no_ipv4_dst) {
+   *no_ipv4_dst = true;
+   ipv4 = true;
+   }
+   break;
default:
OVS_NLERR(log, "Unknown IP tunnel attribute %d",
  type);
@@ -812,9 +822,16 @@ static int ip_tun_from_nlattr(const struct nlattr *attr,
OVS_NLERR(log, "IP tunnel dst address not specified");
return -EINVAL;
}
-   if (ipv4 && !match->key->tun_key.u.ipv4.dst) {
-   OVS_NLERR(log, "IPv4 tunnel dst address is zero");
-   return -EINVAL;
+   if (ipv4) {
+   bool no_dst = no_ipv4_dst ? *no_ipv4_dst : false;
+
+   if (no_dst && match->key->tun_key.u.ipv4.dst) {
+   OVS_NLERR(log, "IPv4 tunnel dst address is not 
zero");
+   return -EINVAL;
+   } else if (!no_dst && !match->key->tun_key.u.ipv4.dst) {
+   OVS_NLERR(log, "IPv4 tunnel dst address is 
zero");
+   return -EINVAL;
+   }
}
if (ipv6 && ipv6_addr_any(>key->tun_key.u.ipv6.dst)) {
OVS_NLERR(log, "IPv6 tunnel dst address is zero");
@@ -1178,7 +1195,7 @@ static int metadata_from_nlattrs(struct net *net, struct 
sw_flow_match *match,
}
if (*attrs & (1 << OVS_KEY_ATTR_TUNNEL)) {
if (ip_tun_from_nlattr(a[OVS_KEY_ATTR_TUNNEL], match,
-  is_mask, log) < 0)
+  is_mask, log, NULL) < 0)
return -EINVAL;
*attrs &= ~(1 << OVS_KEY_ATTR_TUNNEL);
}
@@ -2551,10 +2568,11 @@ static

Re: [ovs-dev] [PATCH] netdev-rte-offloads: Add thread-safety notes.

2019-03-21 Thread Ian Stokes 

On 3/21/2019 7:35 AM, Roni Bar Yanai wrote:

Looks good.


-Original Message-
From: Ilya Maximets 
Sent: Wednesday, March 20, 2019 1:15 PM
To: ovs-dev@openvswitch.org; Ian Stokes 
Cc: Flavio Leitner ; Ophir Munk
; Kevin Traynor ; Roni Bar
Yanai ; Finn Christensen ; Asaf
Penso ; Ilya Maximets 
Subject: [PATCH] netdev-rte-offloads: Add thread-safety notes.

DPDK_FLOW_OFFLOAD_API is not safe in a variety of ways.
This should be documented.



Thanks Ilya, LGTM, pushed to master.

Ian

Signed-off-by: Ilya Maximets 
---

Wording/spelling suggestions are welcome.

  lib/netdev-rte-offloads.h | 17 +
  1 file changed, 17 insertions(+)

diff --git a/lib/netdev-rte-offloads.h b/lib/netdev-rte-offloads.h
index 3f7d15f45..18c8a7558 100644
--- a/lib/netdev-rte-offloads.h
+++ b/lib/netdev-rte-offloads.h
@@ -25,6 +25,23 @@ struct nlattr;
  struct offload_info;
  struct dpif_flow_stats;

+/* Thread-safety
+ * =
+ *
+ * Below API is NOT thread safe in following terms:
+ *
+ *  - The caller must be sure that none of these functions will be called
+ *simultaneously.  Even for different 'netdev's.
+ *
+ *  - The caller must be sure that 'netdev' will not be
destructed/deallocated.
+ *
+ *  - The caller must be sure that 'netdev' configuration will not be changed.
+ *For example, simultaneous call of 'netdev_reconfigure()' for the same
+ *'netdev' is forbidden.
+ *
+ * For current implementation all above restrictions could be fulfilled by
+ * taking the datapath 'port_mutex' in lib/dpif-netdev.c.  */
+
  int netdev_rte_offloads_flow_put(struct netdev *netdev, struct match
*match,
   struct nlattr *actions, size_t actions_len,
   const ovs_u128 *ufid,
--
2.17.1




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

Re: [ovs-dev] [ovs-dev, V7, 1/2] Makefiles: Generate datapath ovs key fields macros

2019-03-21 Thread 0-day Robot 
Bleep bloop.  Greetings Eli Britstein, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 107 characters long (recommended limit is 79)
#64 FILE: build-aux/extract-odp-netlink-macros-h:29:
awk_cmd=$awk_cmd'print "{offsetof(struct '"${struct_name}"', 
"$3"), sizeof("$1,$2")}, \\";'

WARNING: Line is 107 characters long (recommended limit is 79)
#66 FILE: build-aux/extract-odp-netlink-macros-h:31:
awk_cmd=$awk_cmd'print "{offsetof(struct '"${struct_name}"', 
"$2"), sizeof("   $1")}, \\";'

WARNING: Line is 94 characters long (recommended limit is 79)
#70 FILE: build-aux/extract-odp-netlink-macros-h:35:
awk -F ";" "NR>${line_start} && NR<=${line_end}"' {print $1}' $hfile | eval 
"awk $awk_cmd"

WARNING: Line is 83 characters long (recommended limit is 79)
#76 FILE: build-aux/extract-odp-netlink-macros-h:41:
echo "/* Generated automatically from  -- do not modify! 
*/"

Lines checked: 122, Warnings: 4, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@bytheb.org

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

[ovs-dev] [PATCH V7 2/2] odp-util: Do not rewrite fields with the same values as matched

2019-03-21 Thread Eli Britstein 
To improve performance and avoid wasting resources for HW offloaded
flows, do not rewrite fields that are matched with the same value.

Signed-off-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 lib/odp-util.c| 93 +--
 tests/mpls-xlate.at   |  2 +-
 tests/ofproto-dpif.at | 14 +++
 3 files changed, 88 insertions(+), 21 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 0716161dd..291c05f84 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -43,6 +43,7 @@
 #include "uuid.h"
 #include "openvswitch/vlog.h"
 #include "openvswitch/match.h"
+#include "odp-netlink-macros.h"
 
 VLOG_DEFINE_THIS_MODULE(odp_util);
 
@@ -7361,12 +7362,51 @@ commit_odp_tunnel_action(const struct flow *flow, 
struct flow *base,
 }
 }
 
+struct offsetof_sizeof {
+int offset;
+int size;
+};
+
+/* Compare the keys similary to memcmp, but each field separately.
+ * The offsets and sizes of each field is provided by offsetof_sizeof_arr
+ * argument.
+ * For fields with the same value, zero out their mask part in order not to
+ * rewrite them as it's unnecessary */
+static bool
+keycmp_mask(const void *key0, const void *key1,
+struct offsetof_sizeof *offsetof_sizeof_arr, void *mask)
+{
+int field;
+bool differ = false;
+
+for (field = 0 ; ; field++) {
+int size = offsetof_sizeof_arr[field].size;
+int offset = offsetof_sizeof_arr[field].offset;
+char *pkey0 = ((char *)key0) + offset;
+char *pkey1 = ((char *)key1) + offset;
+char *pmask = ((char *)mask) + offset;
+
+if (size == 0) {
+break;
+}
+
+if (memcmp(pkey0, pkey1, size) == 0) {
+memset(pmask, 0, size);
+} else {
+differ = true;
+}
+}
+
+return differ;
+}
+
 static bool
 commit(enum ovs_key_attr attr, bool use_masked_set,
const void *key, void *base, void *mask, size_t size,
+   struct offsetof_sizeof *offsetof_sizeof_arr,
struct ofpbuf *odp_actions)
 {
-if (memcmp(key, base, size)) {
+if (keycmp_mask(key, base, offsetof_sizeof_arr, mask)) {
 bool fully_masked = odp_mask_is_exact(attr, mask, size);
 
 if (use_masked_set && !fully_masked) {
@@ -7408,7 +7448,8 @@ commit_set_ether_action(const struct flow *flow, struct 
flow *base_flow,
 bool use_masked)
 {
 struct ovs_key_ethernet key, base, mask;
-
+struct offsetof_sizeof ovs_key_ethernet_offsetof_sizeof_arr[] =
+OVS_KEY_ETHERNET_OFFSETOF_SIZEOF_ARR;
 if (flow->packet_type != htonl(PT_ETH)) {
 return;
 }
@@ -7418,7 +7459,8 @@ commit_set_ether_action(const struct flow *flow, struct 
flow *base_flow,
 get_ethernet_key(>masks, );
 
 if (commit(OVS_KEY_ATTR_ETHERNET, use_masked,
-   , , , sizeof key, odp_actions)) {
+   , , , sizeof key,
+   ovs_key_ethernet_offsetof_sizeof_arr, odp_actions)) {
 put_ethernet_key(, base_flow);
 put_ethernet_key(, >masks);
 }
@@ -7544,6 +7586,8 @@ commit_set_ipv4_action(const struct flow *flow, struct 
flow *base_flow,
bool use_masked)
 {
 struct ovs_key_ipv4 key, mask, base;
+struct offsetof_sizeof ovs_key_ipv4_offsetof_sizeof_arr[] =
+OVS_KEY_IPV4_OFFSETOF_SIZEOF_ARR;
 
 /* Check that nw_proto and nw_frag remain unchanged. */
 ovs_assert(flow->nw_proto == base_flow->nw_proto &&
@@ -7561,7 +7605,7 @@ commit_set_ipv4_action(const struct flow *flow, struct 
flow *base_flow,
 }
 
 if (commit(OVS_KEY_ATTR_IPV4, use_masked, , , , sizeof key,
-   odp_actions)) {
+   ovs_key_ipv4_offsetof_sizeof_arr, odp_actions)) {
 put_ipv4_key(, base_flow, false);
 if (mask.ipv4_proto != 0) { /* Mask was changed by commit(). */
 put_ipv4_key(, >masks, true);
@@ -7599,6 +7643,8 @@ commit_set_ipv6_action(const struct flow *flow, struct 
flow *base_flow,
bool use_masked)
 {
 struct ovs_key_ipv6 key, mask, base;
+struct offsetof_sizeof ovs_key_ipv6_offsetof_sizeof_arr[] =
+OVS_KEY_IPV6_OFFSETOF_SIZEOF_ARR;
 
 /* Check that nw_proto and nw_frag remain unchanged. */
 ovs_assert(flow->nw_proto == base_flow->nw_proto &&
@@ -7617,7 +7663,7 @@ commit_set_ipv6_action(const struct flow *flow, struct 
flow *base_flow,
 }
 
 if (commit(OVS_KEY_ATTR_IPV6, use_masked, , , , sizeof key,
-   odp_actions)) {
+   ovs_key_ipv6_offsetof_sizeof_arr, odp_actions)) {
 put_ipv6_key(, base_flow, false);
 if (mask.ipv6_proto != 0) { /* Mask was changed by commit(). */
 put_ipv6_key(, >masks, true);
@@ -7653,13 +7699,15 @@ commit_set_arp_action(const struct flow *flow, struct 
flow *base_flow,
   struct ofpbuf *odp_actions, struct flow_wildcards *wc)
 {
 struct ovs_key_arp key, mask, base;
+struct offsetof_sizeof

[ovs-dev] [PATCH V7 1/2] Makefiles: Generate datapath ovs key fields macros

2019-03-21 Thread Eli Britstein 
Generate datapath ovs key fields offset and size array macros as a
pre-step for bit-wise comparing fields, with no functional change.

Signed-off-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 .gitignore |  1 +
 build-aux/extract-odp-netlink-macros-h | 60 ++
 include/automake.mk| 11 +++--
 3 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100755 build-aux/extract-odp-netlink-macros-h

diff --git a/.gitignore b/.gitignore
index 60e7818c3..2ac9cdac7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,6 +68,7 @@ cscope.*
 tags
 _debian
 odp-netlink.h
+odp-netlink-macros.h
 OvsDpInterface.h
 /.vagrant/
 testsuite.tmp.orig
diff --git a/build-aux/extract-odp-netlink-macros-h 
b/build-aux/extract-odp-netlink-macros-h
new file mode 100755
index 0..7152f298c
--- /dev/null
+++ b/build-aux/extract-odp-netlink-macros-h
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+hfile=$1
+
+generate_fields_macros () {
+local struct_name=$1
+local line_start
+local num_lines
+local line_end
+local awk_cmd
+local STRUCT
+
+# line_start is the line number where the definition of the struct begin
+# line_end is the line number where the definition of the struct ends
+line_start=`grep -nw $struct_name $hfile | grep { | cut -d ":" -f1`
+num_lines=`tail -n +${line_start} $hfile | grep -n -m1 } | cut -d ":" -f1`
+line_end=$((line_start+num_lines-1))
+
+
+STRUCT=`echo $struct_name | tr [a-z] [A-Z]`
+echo "#define ${STRUCT}_OFFSETOF_SIZEOF_ARR { \\"
+# for all the field lines, including the terminating }, remove ";" and
+# replace with an item of {offsetof, sizeof}.
+# 3 awk fields are for type struct  and field 
+# 2 awk fields are for type  and field 
+# else - terminating the array by item {0, 0}
+awk_cmd="'{"
+awk_cmd=$awk_cmd'if (NF == 3)'
+awk_cmd=$awk_cmd'print "{offsetof(struct '"${struct_name}"', 
"$3"), sizeof("$1,$2")}, \\";'
+awk_cmd=$awk_cmd'else if (NF == 2)'
+awk_cmd=$awk_cmd'print "{offsetof(struct '"${struct_name}"', 
"$2"), sizeof("   $1")}, \\";'
+awk_cmd=$awk_cmd'else'
+awk_cmd=$awk_cmd'print "{0, 0}}";'
+awk_cmd=$awk_cmd"}'"
+awk -F ";" "NR>${line_start} && NR<=${line_end}"' {print $1}' $hfile | 
eval "awk $awk_cmd"
+
+echo
+echo
+}
+
+echo "/* Generated automatically from  -- do not 
modify! */"
+echo "#ifndef ODP_NETLINK_MACROS_H"
+echo "#define ODP_NETLINK_MACROS_H"
+echo
+echo
+
+generate_fields_macros "ovs_key_ethernet"
+generate_fields_macros "ovs_key_ipv4"
+generate_fields_macros "ovs_key_ipv6"
+generate_fields_macros "ovs_key_tcp"
+generate_fields_macros "ovs_key_udp"
+generate_fields_macros "ovs_key_sctp"
+generate_fields_macros "ovs_key_icmp"
+generate_fields_macros "ovs_key_icmpv6"
+generate_fields_macros "ovs_key_arp"
+generate_fields_macros "ovs_key_nd"
+generate_fields_macros "ovs_key_nd_extensions"
+
+echo
+echo "#endif"
diff --git a/include/automake.mk b/include/automake.mk
index 3f3ed1ccd..01031e88d 100644
--- a/include/automake.mk
+++ b/include/automake.mk
@@ -1,10 +1,15 @@
-BUILT_SOURCES += include/odp-netlink.h
+BUILT_SOURCES += include/odp-netlink.h include/odp-netlink-macros.h
 
 include/odp-netlink.h: datapath/linux/compat/include/linux/openvswitch.h \
build-aux/extract-odp-netlink-h
$(AM_V_GEN)sed -f $(srcdir)/build-aux/extract-odp-netlink-h < $< > $@
-EXTRA_DIST += build-aux/extract-odp-netlink-h
-CLEANFILES += include/odp-netlink.h
+
+include/odp-netlink-macros.h: include/odp-netlink.h \
+  build-aux/extract-odp-netlink-macros-h
+   $(AM_V_GEN)sh -f $(srcdir)/build-aux/extract-odp-netlink-macros-h $< > 
$@
+
+EXTRA_DIST += build-aux/extract-odp-netlink-h 
build-aux/extract-odp-netlink-macros-h
+CLEANFILES += include/odp-netlink.h include/odp-netlink-macros.h
 
 include include/ovn/automake.mk
 include include/openflow/automake.mk
-- 
2.17.2

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

[ovs-dev] [PATCH V7 0/2] Do not rewrite fields with the same values as matched

2019-03-21 Thread Eli Britstein 
This patch set avoids unnecessary rewrite actions to fields with the
same values as matched on.

Patch 1 is a pre-step of generating ovs key fields macros
Patch 2 avoids the unnecessary rewrites and adapts the tests accordingly

Differences from V6:
Remove bash dependency.

Thanks,
Eli

Eli Britstein (2):
  Makefiles: Generate datapath ovs key fields macros
  odp-util: Do not rewrite fields with the same values as matched

 .gitignore |  1 +
 build-aux/extract-odp-netlink-macros-h | 60 +
 include/automake.mk| 11 ++-
 lib/odp-util.c | 93 ++
 tests/mpls-xlate.at|  2 +-
 tests/ofproto-dpif.at  | 14 ++--
 6 files changed, 157 insertions(+), 24 deletions(-)
 create mode 100755 build-aux/extract-odp-netlink-macros-h

-- 
2.17.2

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

Re: [ovs-dev] [PATCH] netdev-rte-offloads: Add thread-safety notes.

2019-03-21 Thread Roni Bar Yanai 
Looks good.

> -Original Message-
> From: Ilya Maximets 
> Sent: Wednesday, March 20, 2019 1:15 PM
> To: ovs-dev@openvswitch.org; Ian Stokes 
> Cc: Flavio Leitner ; Ophir Munk
> ; Kevin Traynor ; Roni Bar
> Yanai ; Finn Christensen ; Asaf
> Penso ; Ilya Maximets 
> Subject: [PATCH] netdev-rte-offloads: Add thread-safety notes.
> 
> DPDK_FLOW_OFFLOAD_API is not safe in a variety of ways.
> This should be documented.
> 
> Signed-off-by: Ilya Maximets 
> ---
> 
> Wording/spelling suggestions are welcome.
> 
>  lib/netdev-rte-offloads.h | 17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/lib/netdev-rte-offloads.h b/lib/netdev-rte-offloads.h
> index 3f7d15f45..18c8a7558 100644
> --- a/lib/netdev-rte-offloads.h
> +++ b/lib/netdev-rte-offloads.h
> @@ -25,6 +25,23 @@ struct nlattr;
>  struct offload_info;
>  struct dpif_flow_stats;
> 
> +/* Thread-safety
> + * =
> + *
> + * Below API is NOT thread safe in following terms:
> + *
> + *  - The caller must be sure that none of these functions will be called
> + *simultaneously.  Even for different 'netdev's.
> + *
> + *  - The caller must be sure that 'netdev' will not be
> destructed/deallocated.
> + *
> + *  - The caller must be sure that 'netdev' configuration will not be 
> changed.
> + *For example, simultaneous call of 'netdev_reconfigure()' for the same
> + *'netdev' is forbidden.
> + *
> + * For current implementation all above restrictions could be fulfilled by
> + * taking the datapath 'port_mutex' in lib/dpif-netdev.c.  */
> +
>  int netdev_rte_offloads_flow_put(struct netdev *netdev, struct match
> *match,
>   struct nlattr *actions, size_t actions_len,
>   const ovs_u128 *ufid,
> --
> 2.17.1

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

Re: [ovs-dev] [PATCH] dpif-netlink: make offload failed EOPNOTSUPP and ENOSPC cases lower priority level log

2019-03-21 Thread Simon Horman 
On Wed, Mar 20, 2019 at 03:10:48PM +0100, Simon Horman wrote:
> On Tue, Mar 19, 2019 at 08:47:31PM +0800, we...@ucloud.cn wrote:
> > From: wenxu 
> > 
> > Offload flow failed for EOPNOTSUPP and ENOSPC which should not
> > be a err. It should e lower priority level log for this two
> > failure case.
> 
> Thanks,
> 
> this looks good to me.
> 
> I am running it through travis-ci to see if it picks up any problems
> https://travis-ci.org/horms2/ovs/builds/508950501

Thanks again,

travis-ci tests passed cleanly and I have applied this
change to the master branch.

> 
> > 
> > Signed-off-by: wenxu 
> > ---
> >  lib/dpif-netlink.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index 00538e5..c554666 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -2068,6 +2068,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
> > dpif_flow_put *put)
> >  VLOG_DBG("added flow");
> >  } else if (err != EEXIST) {
> >  struct netdev *oor_netdev = NULL;
> > +enum vlog_level level;
> >  if (err == ENOSPC && netdev_is_offload_rebalance_policy_enabled()) 
> > {
> >  /*
> >   * We need to set OOR on the input netdev (i.e, 'dev') for the
> > @@ -2082,8 +2083,10 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
> > dpif_flow_put *put)
> >  }
> >  netdev_set_hw_info(oor_netdev, HW_INFO_TYPE_OOR, true);
> >  }
> > -VLOG_ERR_RL(, "failed to offload flow: %s: %s", 
> > ovs_strerror(err),
> > -(oor_netdev ? oor_netdev->name : dev->name));
> > +level = (err == ENOSPC || err == EOPNOTSUPP) ? VLL_DBG : VLL_ERR;
> > +VLOG_RL(, level, "failed to offload flow: %s: %s",
> > +ovs_strerror(err),
> > +(oor_netdev ? oor_netdev->name : dev->name));
> >  }
> >  
> >  out:
> > -- 
> > 1.8.3.1
> > 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.10 0/2] Fix syslog and make travis great again.

2019-03-21 Thread Simon Horman 
On Wed, Mar 20, 2019 at 09:39:34AM -0700, Ben Pfaff wrote:
> On Wed, Mar 20, 2019 at 09:37:22AM -0700, Ben Pfaff wrote:
> > On Wed, Mar 20, 2019 at 10:49:40AM +0300, Ilya Maximets wrote:
> > > On 18.03.2019 23:38, Ben Pfaff wrote:
> > > > On Mon, Mar 18, 2019 at 11:31:32AM +0300, Ilya Maximets wrote:
> > > >> On 18.03.2019 11:03, Simon Horman wrote:
> > > >>> On Fri, Mar 15, 2019 at 01:12:09PM +0300, Ilya Maximets wrote:
> > >  On 15.03.2019 12:39, Simon Horman wrote:
> > > > On Fri, Mar 15, 2019 at 12:28:03PM +0300, Ilya Maximets wrote:
> > > >> On 15.03.2019 11:50, Simon Horman wrote:
> > > >>> On Thu, Mar 14, 2019 at 01:14:05PM +0300, Ilya Maximets wrote:
> > >  On 14.03.2019 11:48, Simon Horman wrote:
> > > > On Wed, Mar 13, 2019 at 12:06:51PM +0100, Simon Horman wrote:
> > > >> On Tue, Feb 26, 2019 at 04:00:02PM +0300, Ilya Maximets wrote:
> > > >>> First patch is a bugfix backport. Second one fixes the 
> > > >>> testsuite
> > > >>> jobs on TravisCI failure due to 50 minutes timeout.
> > > >>>
> > > >>> Both patches needs to be applied to branch-2.10 and 
> > > >>> backported as far
> > > >>> as possible. I tested them on branches from 2.10 down to 2.6.
> > > >>>
> > > >>> The second patch disables rsyslog daemon on travis, so the 
> > > >>> first
> > > >>> fix required to make it work fine in this configuration.
> > > >>>
> > > >>> Branch 2.11 and master are not affected by the slow syslog 
> > > >>> issue
> > > >>> because syslog-null is in use for the testsuite invocations.
> > > >>>
> > > >>> Ilya Maximets (2):
> > > >>>   vlog: Better handle syslog handler exceptions.
> > > >>>   travis: Stop rsyslog before start.
> > > >>
> > > >> Thanks Ilya,
> > > >>
> > > >> I have tested these patches and with both applied I now see
> > > >> that travis-ci runs successfully while this was not the case
> > > >> without these patches.
> > > >>
> > > >> https://travis-ci.org/horms2/ovs/builds/505654239
> > > >>
> > > >> Tested-by: Simon Horman 
> > > >>
> > > >> Ben, I'd be happy to go ahead and apply these if there are no 
> > > >> objections.
> > > >> Or I'm just as happy for someone else to apply them.
> > > >
> > > > I have gone ahead and push this series to branch-2.10.
> > > 
> > >  Thanks! branch-2.10 is green now.
> > > 
> > >  BTW, It'll be good to have these patches down to branch-2.6.
> > > 
> > >  Best regards, Ilya Maximets.
> > > >>>
> > > >>> Thanks,
> > > >>>
> > > >>> I've gone ahead and pushed these to branch-2.9, branch-2.8, 
> > > >>> branch-2.7
> > > >>> and branch-2.6.
> > > >>>
> > > >>> In the case of branch-2.9 and branch-2.8 my testing indicates that
> > > >>> travis-ci should now turn green (hooray!).
> > > >>>
> > > >>> https://travis-ci.org/horms2/ovs/builds/506251905
> > > >>> https://travis-ci.org/horms2/ovs/builds/506252393
> > > >>>
> > > >>> In the case of branch-2.7 and branch-2.6, I had to resolve a minor
> > > >>> conflict when applying the patches. You may want to check to make 
> > > >>> sure
> > > >>> that I got that right.
> > > >>
> > > >> Thanks fro working on this!
> > > >> Rebase is minor, looks good.
> > > >
> > > > Likewise, thanks for your help.
> > > > We seem to be making good progress.
> > > >
> > > >>> For these branches it seems to make travis-ci happier and there 
> > > >>> are
> > > >>> no more aborted ("!") builds. However, there does still seem to 
> > > >>> be an
> > > >>> unrelated failure so those branches do not turn green.
> > > >>>
> > > >>> https://travis-ci.org/horms2/ovs/builds/506253422
> > > >>> https://travis-ci.org/horms2/ovs/jobs/506253451
> > > >>>
> > > >>> https://travis-ci.org/horms2/ovs/builds/506254580
> > > >>> https://travis-ci.org/horms2/ovs/jobs/506254614
> > > >>
> > > >>
> > > >> For OSX build we probably need to backport following patch to 2.6 
> > > >> and 2.7:
> > > >>
> > > >> commit 10fd9f6e477555ca93d28094c2976b2ea0198798
> > > >> Author: Richard Oliver 
> > > >> Date:   Sat Oct 28 16:38:30 2017 +0100
> > > >>
> > > >> timeval: Check for OS-provided clock_gettime on macOS
> > > >> 
> > > >> [Problem]
> > > >> Compilation error on newer versions of macOS (Sierra onwards) 
> > > >> due to
> > > >> multiple declarations of clock_gettime.
> > > >> 
> > > >> [Solution]
> > > >> Have configure check for clock_gettime and check this result in
> > > >> timeval to avoid incorrectly declaring/defining clock_gettime 
> > >

Re: [ovs-dev] [PATCH v5] Monitor Database table to manage lifecycle of IDL client.

2019-03-21 Thread Numan Siddique 
On Sat, Jan 26, 2019 at 12:41 AM Ted Elhourani 
wrote:

> The Python IDL implementation supports ovsdb cluster connections.
> This patch is a follow up to commit 31e434fc98, it adds the option of
> connecting to the leader (the default) in the Raft-based cluster. It mimics
> the exisiting C IDL support for clusters introduced in commit 1b1d2e6daa.
>
> The _Server database schema is first requested, then a monitor of the
> Database table in the _Server Database. Method __check_server_db verifies
> the eligibility of the server. If the attempt to obtain a monitor of the
> _Server database fails and a cluster id was not provided this
> implementation
> proceeds to request the data monitor. If a cluster id was provided via the
> set_cluster_id method then the connection is aborted and a connection to a
> different node is instead attempted, until a valid cluster node is found.
> Thus, when supplied, cluster id is interpreted as the intention to only
> allow connections to a clustered database. If not supplied, connections to
> standalone nodes, or nodes that do not have the _Server database are
> allowed. change_seqno is not incremented in the case of Database table
> updates.
>
> Signed-off-by: Ted Elhourani 
>

Acked-by: Numan Siddique 

Numan

---
>
> v4 -> v5
> 
> * Increase test timeout.
> * Spell out list of files to cat for shell compatibility.
>
> v3 -> v4
> 
> * export -f is not compatible with FreeBSD, modify tests to avoid shell
> function export.
> * Re-add a line that was removed by mistake.
>
> v2 -> v3
> 
> * Add 2 tests, treat cluster_id as a string, mv arg till end, pep8 fixes.
>
> v1 -> v2
> 
> * Modify for backward compatibility with _Server-less ovsdb servers.
>
>  python/ovs/db/idl.py| 219
> 
>  python/ovs/reconnect.py |   3 +
>  tests/ovsdb-idl.at  | 129 +---
>  tests/test-ovsdb.py |  67 ++-
>  4 files changed, 372 insertions(+), 46 deletions(-)
>
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index 250e897..84af978 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -38,6 +38,8 @@ ROW_DELETE = "delete"
>  OVSDB_UPDATE = 0
>  OVSDB_UPDATE2 = 1
>
> +CLUSTERED = "clustered"
> +
>
>  class Idl(object):
>  """Open vSwitch Database Interface Definition Language (OVSDB IDL).
> @@ -92,10 +94,13 @@ class Idl(object):
>  """
>
>  IDL_S_INITIAL = 0
> -IDL_S_MONITOR_REQUESTED = 1
> -IDL_S_MONITOR_COND_REQUESTED = 2
> +IDL_S_SERVER_SCHEMA_REQUESTED = 1
> +IDL_S_SERVER_MONITOR_REQUESTED = 2
> +IDL_S_DATA_MONITOR_REQUESTED = 3
> +IDL_S_DATA_MONITOR_COND_REQUESTED = 4
>
> -def __init__(self, remote, schema_helper, probe_interval=None):
> +def __init__(self, remote, schema_helper, probe_interval=None,
> + leader_only=True):
>  """Creates and returns a connection to the database named
> 'db_name' on
>  'remote', which should be in a form acceptable to
>  ovs.jsonrpc.session.open().  The connection will maintain an
> in-memory
> @@ -119,6 +124,9 @@ class Idl(object):
>
>  The IDL uses and modifies 'schema' directly.
>
> +If 'leader_only' is set to True (default value) the IDL will only
> +monitor and transact with the leader of the cluster.
> +
>  If "probe_interval" is zero it disables the connection keepalive
>  feature. If non-zero the value will be forced to at least 1000
>  milliseconds. If None it will just use the default value in OVS.
> @@ -137,6 +145,20 @@ class Idl(object):
>  self._last_seqno = None
>  self.change_seqno = 0
>  self.uuid = uuid.uuid1()
> +
> +# Server monitor.
> +self._server_schema_request_id = None
> +self._server_monitor_request_id = None
> +self._db_change_aware_request_id = None
> +self._server_db_name = '_Server'
> +self._server_db_table = 'Database'
> +self.server_tables = None
> +self._server_db = None
> +self.server_monitor_uuid = uuid.uuid1()
> +self.leader_only = leader_only
> +self.cluster_id = None
> +self._min_index = 0
> +
>  self.state = self.IDL_S_INITIAL
>
>  # Database locking.
> @@ -172,6 +194,12 @@ class Idl(object):
>  remotes.append(r)
>  return remotes
>
> +def set_cluster_id(self, cluster_id):
> +"""Set the id of the cluster that this idl must connect to."""
> +self.cluster_id = cluster_id
> +if self.state != self.IDL_S_INITIAL:
> +self.force_reconnect()
> +
>  def index_create(self, table, name):
>  """Create a named multi-column index on a table"""
>  return self.tables[table].rows.index_create(name)
> @@ -222,7 +250,7 @@ class Idl(object):
>  if seqno != self._last_seqno:
>  self._last_seqno = seqno
>

[ovs-dev] PRIVATE...

2019-03-21 Thread out--- via dev 
I have a business Proposal that will be of benefit to the both of us.Kindly 
contact me on mrmichealwu...@yahoo.com.hk should this be of interest to you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev