Re: [ovs-dev] [PATCH net-next v2 2/5] net: openvswitch: set max limitation to meters

2020-04-20 Thread Pravin Shelar 
On Sun, Apr 19, 2020 at 5:28 PM Tonghao Zhang  wrote:
>
> On Mon, Apr 20, 2020 at 1:31 AM Pravin Shelar  wrote:
> >
> > On Sat, Apr 18, 2020 at 10:25 AM  wrote:
> > >
> > > From: Tonghao Zhang 
> > >
> > > Don't allow user to create meter unlimitedly,
> > > which may cause to consume a large amount of kernel memory.
> > > The 200,000 meters may be fine in general case.
> > >
> > > Cc: Pravin B Shelar 
> > > Cc: Andy Zhou 
> > > Signed-off-by: Tonghao Zhang 
> > > ---
> > >  net/openvswitch/meter.c | 21 +++--
> > >  net/openvswitch/meter.h |  1 +
> > >  2 files changed, 16 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> > > index 494a0014ecd8..1b6776f9c109 100644
> > > --- a/net/openvswitch/meter.c
> > > +++ b/net/openvswitch/meter.c
> > > @@ -137,6 +137,7 @@ static int attach_meter(struct dp_meter_table *tbl, 
> > > struct dp_meter *meter)
> > >  {
> > > struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
> > > u32 hash = meter_hash(ti, meter->id);
> > > +   int err;
> > >
> > > /*
> > >  * In generally, slot selected should be empty, because
> > > @@ -148,16 +149,24 @@ static int attach_meter(struct dp_meter_table *tbl, 
> > > struct dp_meter *meter)
> > > dp_meter_instance_insert(ti, meter);
> > >
> > > /* That function is thread-safe. */
> > > -   if (++tbl->count >= ti->n_meters)
> > > -   if (dp_meter_instance_realloc(tbl, ti->n_meters * 2))
> > > -   goto expand_err;
> > > +   tbl->count++;
> > > +   if (tbl->count > DP_METER_NUM_MAX) {
> > > +   err = -EFBIG;
> > > +   goto attach_err;
> > > +   }
> > > +
> > > +   if (tbl->count >= ti->n_meters &&
> > > +   dp_meter_instance_realloc(tbl, ti->n_meters * 2)) {
> > > +   err = -ENOMEM;
> > > +   goto attach_err;
> > > +   }
> > >
> > > return 0;
> > >
> > > -expand_err:
> > > +attach_err:
> > > dp_meter_instance_remove(ti, meter);
> > > tbl->count--;
> > > -   return -ENOMEM;
> > > +   return err;
> > >  }
> > >
> > >  static void detach_meter(struct dp_meter_table *tbl, struct dp_meter 
> > > *meter)
> > > @@ -264,7 +273,7 @@ static int ovs_meter_cmd_features(struct sk_buff 
> > > *skb, struct genl_info *info)
> > > if (IS_ERR(reply))
> > > return PTR_ERR(reply);
> > >
> > > -   if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
> > > +   if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, 
> > > DP_METER_NUM_MAX) ||
> > > nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
> > > goto nla_put_failure;
> > >
> > > diff --git a/net/openvswitch/meter.h b/net/openvswitch/meter.h
> > > index d91940383bbe..cdfc6b9dbd42 100644
> > > --- a/net/openvswitch/meter.h
> > > +++ b/net/openvswitch/meter.h
> > > @@ -19,6 +19,7 @@ struct datapath;
> > >
> > >  #define DP_MAX_BANDS   1
> > >  #define DP_METER_ARRAY_SIZE_MIN(1ULL << 10)
> > > +#define DP_METER_NUM_MAX   (20ULL)
> > >
> > Lets make it configurable and default could 200k to allow
> > customization on different memory configurations.
> Great, set different limit depend on current system memory size like tcp ?

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

Re: [ovs-dev] [PATCH net-next v2 1/5] net: openvswitch: expand the meters supported number

2020-04-20 Thread Pravin Shelar 
On Sun, Apr 19, 2020 at 5:23 PM Tonghao Zhang  wrote:
>
> On Mon, Apr 20, 2020 at 1:29 AM Pravin Shelar  wrote:
> >
> > On Sat, Apr 18, 2020 at 10:25 AM  wrote:
> > >
> > > From: Tonghao Zhang 
> > >
> > > In kernel datapath of Open vSwitch, there are only 1024
> > > buckets of meter in one dp. If installing more than 1024
> > > (e.g. 8192) meters, it may lead to the performance drop.
> > > But in some case, for example, Open vSwitch used as edge
> > > gateway, there should be 200,000+ at least, meters used for
> > > IP address bandwidth limitation.
> > >
> > > [Open vSwitch userspace datapath has this issue too.]
> > >
> > > For more scalable meter, this patch expands the buckets
> > > when necessary, so we can install more meters in the datapath.
> > > Introducing the struct *dp_meter_instance*, it's easy to
> > > expand meter though changing the *ti* point in the struct
> > > *dp_meter_table*.
> > >
> > > Cc: Pravin B Shelar 
> > > Cc: Andy Zhou 
> > > Signed-off-by: Tonghao Zhang 
> > > ---
> > >  net/openvswitch/datapath.h |   2 +-
> > >  net/openvswitch/meter.c| 200 +
> > >  net/openvswitch/meter.h|  15 ++-
> > >  3 files changed, 169 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
> > > index e239a46c2f94..785105578448 100644
> > > --- a/net/openvswitch/datapath.h
> > > +++ b/net/openvswitch/datapath.h
> > > @@ -82,7 +82,7 @@ struct datapath {
> > > u32 max_headroom;
> > >
> > > /* Switch meters. */
> > > -   struct hlist_head *meters;
> > > +   struct dp_meter_table *meters;
> > lets define it as part of this struct to avoid indirection.
> >
> > >  };
> > >
> > >  /**
> > > diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> > > index 5010d1ddd4bd..494a0014ecd8 100644
> > > --- a/net/openvswitch/meter.c
> > > +++ b/net/openvswitch/meter.c
> > > @@ -19,8 +19,6 @@
> > >  #include "datapath.h"
> > >  #include "meter.h"
> > >
> > > -#define METER_HASH_BUCKETS 1024
> > > -
> > >  static const struct nla_policy meter_policy[OVS_METER_ATTR_MAX + 1] = {
> > > [OVS_METER_ATTR_ID] = { .type = NLA_U32, },
> > > [OVS_METER_ATTR_KBPS] = { .type = NLA_FLAG },
> > > @@ -39,6 +37,11 @@ static const struct nla_policy 
> > > band_policy[OVS_BAND_ATTR_MAX + 1] = {
> > > [OVS_BAND_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
> > >  };
> > >
> > > +static u32 meter_hash(struct dp_meter_instance *ti, u32 id)
> > > +{
> > > +   return id % ti->n_meters;
> > > +}
> > > +
> > >  static void ovs_meter_free(struct dp_meter *meter)
> > >  {
> > > if (!meter)
> > > @@ -47,40 +50,141 @@ static void ovs_meter_free(struct dp_meter *meter)
> > > kfree_rcu(meter, rcu);
> > >  }
> > >
> > > -static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
> > > -   u32 meter_id)
> > > -{
> > > -   return >meters[meter_id & (METER_HASH_BUCKETS - 1)];
> > > -}
> > > -
> > >  /* Call with ovs_mutex or RCU read lock. */
> > > -static struct dp_meter *lookup_meter(const struct datapath *dp,
> > > +static struct dp_meter *lookup_meter(const struct dp_meter_table *tbl,
> > >  u32 meter_id)
> > >  {
> > > +   struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
> > > +   u32 hash = meter_hash(ti, meter_id);
> > > struct dp_meter *meter;
> > > -   struct hlist_head *head;
> > >
> > > -   head = meter_hash_bucket(dp, meter_id);
> > > -   hlist_for_each_entry_rcu(meter, head, dp_hash_node,
> > > -   lockdep_ovsl_is_held()) {
> > > -   if (meter->id == meter_id)
> > > -   return meter;
> > > -   }
> > > +   meter = rcu_dereference_ovsl(ti->dp_meters[hash]);
> > > +   if (meter && likely(meter->id == meter_id))
> > > +   return meter;
> > > +
> > > return NULL;
> > >  }
> > >
> > > -static void attach_meter(struct datapath *dp, struct dp_meter *meter)
> > > +static struct dp_meter_instance *dp_meter_instance_alloc(const u32 size)
> > > +{
> > > +   struct dp_meter_instance *ti;
> > > +
> > > +   ti = kvzalloc(sizeof(*ti) +
> > > + sizeof(struct dp_meter *) * size,
> > > + GFP_KERNEL);
> > > +   if (!ti)
> > > +   return NULL;
> > Given this is a kernel space array we need to have hard limit inplace.
> In patch 2, I limited the meter number, should we add hard limit here ?
I guess its not needed here.
...

> > >  static struct sk_buff *
> > > @@ -303,9 +407,13 @@ static int ovs_meter_cmd_set(struct sk_buff *skb, 
> > > struct genl_info *info)
> > > meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
> > >
> > > /* Cannot fail after this. */
> > > -   old_meter = lookup_meter(dp, meter_id);
> > > -   detach_meter(old_meter);
> > > -   attach_meter(dp,

[ovs-dev] se former a la maison

2020-04-20 Thread Fabien COUTARIER 

* {line-height: inherit;} .ExternalClass * { line-height: 100%; } body,
p{margin:0; padding:0; margin-bottom:0; -webkit-text-size-adjust:none;
-ms-text-size-adjust:none;} img{line-height:100%; outline:none;
text-decoration:none; -ms-interpolation-mode: bicubic;} a img{border:
none;} a, a:link, .no-detect-local a, .appleLinks a{color:#97c21d
!important; text-decoration: underline;} .ExternalClass {display: block
!important; width:100%;} .ExternalClass, .ExternalClass p,
.ExternalClass span, .ExternalClass font, .ExternalClass td,
.ExternalClass div { line-height: inherit; } table td
{border-collapse:collapse;mso-table-lspace: 0pt; mso-table-rspace:
0pt;} sup{position: relative; top: 4px; line-height:7px
!important;font-size:11px !important;} .mobile_link a[href^="tel"],
.mobile_link a[href^="sms"] {text-decoration: default; color: #97c21d
!important; pointer-events: auto; cursor: default;} .no-detect
a{text-decoration: none; color: #97c21d; pointer-events: auto; cursor:
default;} {color: #97c21d;} span {color: inherit; border-bottom: none;}
span:hover { background-color: transparent; } .nounderline
{text-decoration: none !important;} h1, h2, h3 { margin:0; padding:0; }
p {Margin: 0px !important; } table[class="email-root-wrapper"] { width:
600px !important; } body { } body { min-width: 280px; width: 100%;}
td[class="pattern"] .c56p10r { width: 10%;} td[class="pattern"]
.c448p80r { width: 80%;} @media only screen and (max-width: 599px),
only screen and (max-device-width: 599px), only screen and (max-width:
400px), only screen and (max-device-width: 400px) { .email-root-wrapper
{ width: 100% !important; } .full-width { width: 100% !important;
height: auto !important; text-align:center;} .fullwidthhalfleft
{width:100% !important;} .fullwidthhalfright {width:100% !important;}
.fullwidthhalfinner {width:100% !important; margin: 0 auto !important;
float: none !important; margin-left: auto !important; margin-right:
auto !important; clear:both !important; } .hide {display:none
!important; width:0px !important;height:0px !important;
overflow:hidden; } .desktop-hide { display:block !important; width:100%
!important;height:auto !important; overflow:hidden; max-height: inherit
!important; } .c56p10r { width: 100% !important; float:none;} .c448p80r
{ width: 100% !important; float:none;} } @media only screen and
(min-width: 600px) { td[class="pattern"] .c56p10r { width: 56px
!important;} td[class="pattern"] .c448p80r { width: 448px !important;}
} @media only screen and (max-width: 599px), only screen and
(max-device-width: 599px), only screen and (max-width: 400px), only
screen and (max-device-width: 400px) {
table[class="email-root-wrapper"] { width: 100% !important; }
td[class="wrap"] .full-width { width: 100% !important; height: auto
!important;} td[class="wrap"] .fullwidthhalfleft
{width:100%!important;} td[class="wrap"] .fullwidthhalfright
{width:100% !important;} td[class="wrap"] .fullwidthhalfinner
{width:100% !important; margin: 0 auto !important; float: none
!important; margin-left: auto !important; margin-right: auto
!important; clear:both !important; } td[class="wrap"] .hide {
display:none !important; width:0px;height:0px; overflow:hidden; }
td[class="pattern"] .c56p10r { width: 100% !important; }
td[class="pattern"] .c448p80r { width: 100% !important; } } html, body
{margin:0 !important; padding:0px !important;} img.full-width {
position: relative !important; } .img227x227 { width: 227px !important;
height: 227px !important;} .mso-font-fix-arial { font-family: Arial,
sans-serif;} .mso-font-fix-georgia { font-family: Georgia, sans-serif;}
.mso-font-fix-tahoma { font-family: Tahoma, sans-serif;}
.mso-font-fix-times_new_roman { font-family: 'Times New Roman',
sans-serif;} .mso-font-fix-trebuchet_ms { font-family: 'Trebuchet MS',
sans-serif;} .mso-font-fix-verdana { font-family: Verdana, sans-serif;}
table, td { border-collapse: collapse !important; mso-table-lspace: 0px
!important; mso-table-rspace: 0px !important; } .email-root-wrapper {
width 600px !important;} .imglink { font-size: 0px; } .edm_button {
font-size: 0px; } table { font-size:0px; mso-margin-top-alt:0px; }
.fullwidthhalfleft { width: 49% !important; float:left !important; }
.fullwidthhalfright { width: 50% !important; float:right !important; }
html, body {background-image: none !important; background-color:
transparent !important; margin:0 !important; padding:0 !important;}
.Style1 {font-size: 24px} .Style2 { font-family: arial; font-size:
14px; color: #97c21d; font-weight: bold; } [Seconds]















Formations en Magnétisme Thérapeutique



















Le Magnétisme Thérapeutique permet par simple imposition des mains de
soulager en ré-harmonisant les énergies. Découvrez, comprenez et
contrôlez ces incroyables pouvoirs qui sont cachés en vous !











































Cette formation Certifiante de Praticien en Magnétisme Thérapeutique
vise dans un premier temps, à faire prendre conscience à chaque
participant de

Re: [ovs-dev] OVS DPDK VF port representors not working

2020-04-20 Thread Ravi Kerur 
On Mon, Apr 20, 2020 at 10:37 AM Ravi Kerur  wrote:

>
>
> On Mon, Apr 20, 2020 at 3:14 AM Ilya Maximets  wrote:
>
>> On 4/19/20 6:08 PM, Ravi Kerur wrote:
>> >
>> >
>> > On Sun, Apr 19, 2020 at 6:14 AM Ilya Maximets > > wrote:
>> >
>> > On 4/18/20 2:33 PM, Ravi Kerur wrote:
>> > >
>> > >
>> > > On Fri, Apr 17, 2020 at 9:57 AM Ilya Maximets >   i.maxim...@ovn.org>>> wrote:
>> > >
>> > > On 4/16/20 11:00 PM, Ravi Kerur wrote:
>> > > > Hello OvS-DPDK team,
>> > > >
>> > > > We are expanding our usage of OvS-DPDK and want to use VF
>> port representors
>> > > > for one use-case. Going through the following document it
>> is unclear how
>> > > > this functionality can be achieved
>> > > >
>> > > > http://docs.openvswitch.org/en/latest/topics/dpdk/phy/
>> > > >
>> > > > I believe following things need to be configured before
>> configuring OvS, I
>> > > > am using Intel NICs for evaluation
>> > > >
>> > > > (1) Create VFs on Intel NICs(82599). This can only happen
>> when Intel NIC is
>> > > > bound to its respective kernel driver (ixgbe) via
>> > > >
>> > > > echo 2 > /sys/devices//sriov_numvfs
>> > > >
>> > > >
>> > > > It creates 2 VF interfaces bound to ixgbevf driver
>> > > >
>> > > >
>> > > > (2) Using driverctl, bind VF and PF interfaces to vfio-pci.
>> Moment PF
>> > > > device is bound to vfio-pci, VF interfaces vanish
>> > > >
>> > > >
>> > > > Question based on the OvS document shown above
>> > > >
>> > > >
>> > > > (1) Binding PF interface?
>> > > >
>> > > >
>> > > > ovs-vsctl add-port br0 dpdk-pf -- set Interface dpdk-pf
>> type=dpdk
>> > > > options:dpdk-devargs=:08:00.0
>> > > >
>> > > >
>> > > > What driver interface <:08:00.0> is bound to? igb_uio,
>> vfio-pci or
>> > > > kernel driver?
>> > >
>> > > I believe that it was tested with igb_uio driver, but I never
>> tried it myself.
>> > >
>> > > In order to have sr-iov configuration support with vfio-pci
>> driver you need
>> > > a brand new v5.7-rc1 kernel and DPDK changes that are not
>> merged yet:
>> > > https://patchwork.dpdk.org/project/dpdk/list/?series=9356
>> > > Not sure if it will work smoothly with OVS out of the box,
>> though.
>> > >
>> > >
>> > > Thanks Ilya for the link. Will look into it.
>> > >
>> > > Unfortunately igb_uio doesn't work either. Basics of creating
>> VF's out of PF and assigning them to DPDK driver itself is not working.
>> > > My understanding is
>> > > (1) In order to create VF's NIC should be bound to its kernel
>> driver. Cannot create VFs when NIC is bound to igb_uio or vfio-pci
>> > > (2) No mechanism exists to bind both PF and VF to igb_uio to use
>> this feature
>> > >
>> > > Output below shows what I am observing on :18:00.0
>> > >
>> > > (1) PCI device has PF and 1 VF, both assigned to kernel driver
>> > >
>> > > driverctl list-devices network
>> > >
>> > > :04:00.0 igb
>> > >
>> > > *:18:00.0 ixgbe*
>> > >
>> > > :18:00.1 ixgbe
>> > >
>> > > *:18:10.0 ixgbevf*
>> > >
>> > > :b3:00.0 vfio-pci [*]
>> > >
>> > > :b3:00.1 ixgbe
>> > >
>> > >
>> > > (2) Bind :18:00.0 to igb_uio, VF device is gone
>> > > # driverctl set-override :18:00.0 igb_uio
>> > >
>> > > #driverctl list-devices network
>> > >
>> > > :04:00.0 igb
>> > >
>> > > *:18:00.0 igb_uio [*]*
>> > >
>> > > :18:00.1 ixgbe
>> > >
>> > > :b3:00.0 vfio-pci [*]
>> > >
>> > > :b3:00.1 ixgbe
>> > >
>> > >
>> > > (3) Try to create VF when bound to igb_uio
>> > >
>> > >
>> > > echo 1 > /sys/bus/pci/devices/\:18\:00.0/sriov_numvfs
>> > >
>> > > bash: echo: write error: No such file or directory
>> >
>> > OVS and DPDK docs both says that knob should be "max_vfs", not
>> "sriov_numvfs".
>> > I didn't test, but looking at the code of igb_uio driver it
>> actually creates
>> > this sysfs entry.
>> >
>> >
>> > Thanks for your kind help. My mistake. On Ubuntu max_vfs is created in
>> >
>> > /sys/devices/pci:16/:16:02.0/:18:00.0/max_vfs instead
>> of usual
>> >
>> > /sys/bus/pci/devices/\:18\:00.0/
>> >
>> >  I am able to create VF's when PF is attached to igb_uio.
>> > When adding dpdk-pf port to bridge I get following errors (OvS 2.13.90,
>> DPDK 19.11). Please note I have started with empty conf.db in OvS.
>> >
>> > driverctl list-devices network
>> >
>> > :04:00.0 igb
>> >
>> > :18:00.0 igb_uio [*]
>> >
>> > :18:00.1

Re: [ovs-dev] [PATCH] net: openvswitch: ovs_ct_exit to be done under ovs_lock

2020-04-20 Thread David Miller 
From: xiangxia.m@gmail.com
Date: Fri, 17 Apr 2020 02:57:31 +0800

> From: Tonghao Zhang 
> 
> syzbot wrote:
 ...
> To avoid that warning, invoke the ovs_ct_exit under ovs_lock and add
> lockdep_ovsl_is_held as optional lockdep expression.
> 
> Link: https://lore.kernel.org/lkml/e642a905a0cbe...@google.com
> Fixes: 11efd5cb04a1 ("openvswitch: Support conntrack zone limit")
> Cc: Pravin B Shelar  
> Cc: Yi-Hung Wei 
> Reported-by: syzbot+7ef50afd3a211f879...@syzkaller.appspotmail.com
> Signed-off-by: Tonghao Zhang 

Applied and queued up for -stable, thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/1] ofproto: Fix ofgroup ref_count in the group stats.

2020-04-20 Thread 0-day Robot 
Bleep bloop.  Greetings Pier Luigi Ventre, 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:
ERROR: Author Pier Luigi Ventre  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Pier Luigi Ventre 
Lines checked: 345, Warnings: 1, Errors: 1


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

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

Re: [ovs-dev] [PATCH] Utilities: Add the ovs_dump_ofpacts command to gdb

2020-04-20 Thread William Tu 
On Mon, Apr 20, 2020 at 03:37:03PM +0200, Eelco Chaudron wrote:
> William,
> 
> Can you try adding this somewhere at the top after the other import section?
> 
> from __future__ import print_function
> 
> And let me know if it works?
> 
> Thanks,
> 
> Eelco

Hi Eelco,

Thanks, it work, but I have to put it "before" other import

diff --git a/utilities/gdb/ovs_gdb.py b/utilities/gdb/ovs_gdb.py
index befc2b4a4b45..3a23df452816 100644
--- a/utilities/gdb/ovs_gdb.py
+++ b/utilities/gdb/ovs_gdb.py
@@ -55,11 +55,14 @@
 #...
 #...
 #
+
+from __future__ import print_function
 import gdb
 import sys
 import uuid


Putting it after import below has the issue

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
0x7f57c81f774d in poll () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) source utilities/gdb/ovs_gdb.py 
  File "utilities/gdb/ovs_gdb.py", line 62
from __future__ import print_function
SyntaxError: from __future__ imports must occur at the beginning of the file
(gdb) 


William

> 
> 
> On 20 Apr 2020, at 15:27, Eelco Chaudron wrote:
> 
> >On 20 Apr 2020, at 15:06, William Tu wrote:
> >
> >
> Is this due to python2/3? My python version:
> boxes:~/ovs# python
> Python 3.5.2 (default, Oct  8 2019, 13:06:37)
> [GCC 5.4.0 20160609] on linux
> Type "help", "copyright", "credits" or "license" for more
> information.
> >>>
> >>Hi Eelco,
> >>That's the only error I got
> >>
> >>(gdb) source ./utilities/gdb/ovs_gdb.py
> >>  File "./utilities/gdb/ovs_gdb.py", line 164
> >>print(self.message, end='')
> >>   ^
> >>SyntaxError: invalid syntax
> >
> >Looks like the end=‘’ option is only supported in later version of 3.x.
> >I’ll try to find a way to make this work for all version of Python and
> >sent a patch later this week/early next week.
> >
> >//Eelco
> >
> >___
> >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

[ovs-dev] [PATCH ovn] DNS: Make DNS lookups case insensitive.

2020-04-20 Thread Mark Michelson 
>From RFC 1035 Section 2.3.3:

"For all parts of the DNS that are part of the official protocol, all
comparisons between character strings (e.g., labels, domain names, etc.)
are done in a case-insensitive manner."

OVN was using case-sensitive lookups and therefore was not complying.
This change makes lookups case insensitive by storing lowercase record
names in the southbound database and converting incoming query names to
lowercase.

Signed-off-by: Mark Michelson 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1819069
Reported-by: Jianlin Shi 
---
v1 -> v2
Fixed 0-day Robot's complaint about line length in pinctrl.c
---
 controller/pinctrl.c |  7 -
 lib/ovn-util.c   | 15 +++
 lib/ovn-util.h   |  5 
 northd/ovn-northd.c  | 15 ++-
 ovn-sb.xml   |  3 ++-
 tests/ovn.at | 61 
 6 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 8703641c2..8592d4e3f 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2368,7 +2368,12 @@ pinctrl_handle_dns_lookup(
 struct dns_data *d = iter->data;
 for (size_t i = 0; i < d->n_dps; i++) {
 if (d->dps[i] == dp_key) {
-answer_ips = smap_get(>records, ds_cstr(_name));
+/* DNS records in SBDB are stored in lowercase. Convert to
+ * lowercase to perform case insensitive lookup
+ */
+char *query_name_lower = str_tolower(ds_cstr(_name));
+answer_ips = smap_get(>records, query_name_lower);
+free(query_name_lower);
 if (answer_ips) {
 break;
 }
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 514e2489f..1b30c2e9a 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -21,6 +21,7 @@
 #include "openvswitch/ofp-parse.h"
 #include "ovn-nb-idl.h"
 #include "ovn-sb-idl.h"
+#include 
 
 VLOG_DEFINE_THIS_MODULE(ovn_util);
 
@@ -550,3 +551,17 @@ ip46_equals(const struct v46_ip *addr1, const struct 
v46_ip *addr2)
 (addr1->family == AF_INET ? addr1->ipv4 == addr2->ipv4 :
  IN6_ARE_ADDR_EQUAL(>ipv6, >ipv6)));
 }
+
+char *
+str_tolower(const char *orig)
+{
+char *copy = xmalloc(strlen(orig) + 1);
+char *p = copy;
+
+while (*orig) {
+*p++ = tolower(*orig++);
+}
+*p = '\0';
+
+return copy;
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 11238f61c..4076e8b9a 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -124,4 +124,9 @@ struct v46_ip {
 bool ip46_parse_cidr(const char *str, struct v46_ip *prefix,
  unsigned int *plen);
 bool ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2);
+
+/* Returns a lowercase copy of orig.
+ * Caller must free the returned string.
+ */
+char *str_tolower(const char *orig);
 #endif
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 616e52bb2..0219a4bb0 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -10706,7 +10706,20 @@ sync_dns_entries(struct northd_context *ctx, struct 
hmap *datapaths)
 dns_info->sb_dns,
 (struct sbrec_datapath_binding **)dns_info->sbs,
 dns_info->n_sbs);
-sbrec_dns_set_records(dns_info->sb_dns, _info->nb_dns->records);
+
+/* DNS lookups are case-insensitive. Convert records to lowercase so
+ * we can do consistent lookups when DNS requests arrive
+ */
+struct smap lower_records = SMAP_INITIALIZER(_records);
+struct smap_node *node;
+SMAP_FOR_EACH (node, _info->nb_dns->records) {
+smap_add_nocopy(_records, xstrdup(node->key),
+str_tolower(node->value));
+}
+
+sbrec_dns_set_records(dns_info->sb_dns, _records);
+
+smap_destroy(_records);
 free(dns_info->sbs);
 free(dns_info);
 }
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 72466b97e..5f8da534c 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3597,7 +3597,8 @@ tcp.flags = RST;
 
   Key-value pair of DNS records with DNS query name as the key
   and a string of IP address(es) separated by comma or space as the
-  value.
+  value. ovn-northd stores the DNS query name in all lowercase in order to
+  facilitate case-insensitive lookups.
 
   Example:  "vm1.ovn.org" = "10.0.0.4 aef0::4"
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 7858c496e..a52e644f0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8357,6 +8357,12 @@ set_dns_params() {
 # IPv4 address - 10.0.0.4
 expected_dns_answer=${query_name}00010001${ttl}00040a04
 ;;
+VM1)
+# VM1.OVN.ORG
+query_name=03564d31034f564e034f524700
+# IPv4 address - 10.0.0.4
+expected_dns_answer=${query_name}00010001${ttl}00040a04
+;;
 vm2)
 # vm2.ovn.org
 query_name=03766d32036f766e036f726700
@@

Re: [ovs-dev] [PATCH ovn] ovn-northd: Limit IPv6 ND NS/RA/RS to the local network.

2020-04-20 Thread Numan Siddique 
On Sat, Apr 18, 2020 at 3:24 AM Dumitru Ceara  wrote:
>
> Neighbor solicitation packets for router owned IPs are replied to in
> table IN_IP_INPUT at a higher priority than flows relay IPv6 multicast
> traffic when needed. All other NS/NA packets received at this point can
> be safely dropped.
>
> However, router advertisement and router solicitation packets are
> processed at a later stage, in ND_RA_OPTIONS/ND_RA_RESPONSE. These
> packets need to be allowed in table IN_IP_INPUT.
>
> Commit 677a3ba4d66b incorrectly allowed all IPv6 multicast traffic
> destined to all-nodes in table IN_IP_INPUT. Instead, only ND_RA and
> ND_RS packets should be allowed. All others were either already
> processed or should be dropped. If multicast relay is enabled then IPv6
> multicast traffic that's not destined to reserved groups should also be
> allowed.
>
> Furthermore, router solicitation and advertisement packets that don't
> get processed in tables ND_RA_OPTIONS/ND_RA_RESPONSE should be dropped
> in IN_IP_ROUTING because they should never be routed.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1825334
> Reported-by: Jakub Libosvar 
> Fixes: 677a3ba4d66b ("ovn: Add MLD support.")
> Signed-off-by: Dumitru Ceara 
> ---
>  northd/ovn-northd.8.xml | 49 
> -
>  northd/ovn-northd.c | 43 ++-
>  2 files changed, 62 insertions(+), 30 deletions(-)

Thanks Dumitru for the fix.
I tested locally too and confirm that IPv6 RA packets  which entered
the router pipeline
are dropped.

I applied this patch to master and branch-20.03.

Thanks
Numan

>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 82c86f6..efcc4b7 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1670,22 +1670,6 @@ next;
>
>
>  
> -  A priority-87 flow explicitly allows IPv6 multicast traffic that is
> -  supposed to reach the router pipeline (e.g., neighbor solicitations
> -  and traffic destined to the All-Routers multicast group).
> -
> -  
> -
> -  
> -
> -  A priority-86 flow allows IP multicast traffic if
> -  :mcast_relay='true',
> -  otherwise drops it.
> -
> -  
> -
> -  
> -
>ICMP echo reply.  These flows reply to ICMP echo requests received
>for the router's IP address.  Let A be an IP address
>owned by a router port.  Then, for each A that is
> @@ -1946,6 +1930,29 @@ nd.tll = external_mac;
>
>
>  
> +  A priority-84 flow explicitly allows IPv6 multicast traffic that is
> +  supposed to reach the router pipeline (i.e., router solicitation
> +  and router advertisement packets).
> +
> +  
> +
> +  
> +
> +  A priority-83 flow explicitly drops IPv6 multicast traffic that is
> +  destined to reserved multicast groups.
> +
> +  
> +
> +  
> +
> +  A priority-82 flow allows IP multicast traffic if
> +  :mcast_relay='true',
> +  otherwise drops it.
> +
> +  
> +
> +  
> +
>UDP port unreachable.  Priority-80 flows generate ICMP port
>unreachable messages in reply to UDP datagrams directed to the
>router's IP address, except in the special case of gateways,
> @@ -2442,6 +2449,13 @@ output;
>  
>
>  
> +  Priority-550 flow that drops IPv6 Router Solicitation/Advertisement
> +  packets that were not processed in previous tables.
> +
> +  
> +
> +  
> +
>Priority-500 flows that match IP multicast traffic destined to
>groups registered on any of the attached switches and sets
>outport to the associated multicast group that will
> @@ -2457,7 +2471,8 @@ output;
>multicast group, which ovn-northd populates with the
>logical ports that have
>
> -  :mcast_flood='true'.
> +  :mcast_flood='true'. If no router ports are configured
> +  to flood multicast traffic the packets are dropped.
>  
>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 616e52b..3248350 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -8006,17 +8006,6 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>
>  /* Priority-90 flows reply to ARP requests and ND packets. */
>
> -/* Allow IPv6 multicast traffic that's supposed to reach the
> - * router pipeline (e.g., neighbor solicitations).
> - */
> -ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 87, 
> "ip6.mcast_flood",
> -  "next;");
> -
> -/* Allow multicast if relay enabled (priority 86). */
> -ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 86,
> -

Re: [ovs-dev] [PATCH ovn] DNS: Make DNS lookups case insensitive.

2020-04-20 Thread 0-day Robot 
Bleep bloop.  Greetings Mark Michelson, 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 84 characters long (recommended limit is 79)
#38 FILE: controller/pinctrl.c:2371:
/* DNS records in SBDB are stored in lowercase. Convert to 
lowercase

Lines checked: 268, Warnings: 1, Errors: 0


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

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

[ovs-dev] [PATCH 1/1] ofproto: Fix ofgroup ref_count in the group stats.

2020-04-20 Thread Pier Luigi Ventre 
Hi everyone.

This patch is a followup to this message sent in ovs-discuss:
https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049897.html 


This is the brief description of the change:

The patch fixes ofgroup ref_count in the group stats. Previously, only the 
rules were counted.
However, ofgroups can reference other ofgroups.

Updates tests that were failing: groups were not created in order. Adds new 
unit tests for reference count

Signed-off-by: Pier Luigi Ventre 

—

diff --git a/include/openvswitch/ofp-group.h b/include/openvswitch/ofp-group.h
index cd7af0ebf..e5dd3a9ec 100644
--- a/include/openvswitch/ofp-group.h
+++ b/include/openvswitch/ofp-group.h
@@ -58,6 +58,9 @@ struct ofputil_bucket {
 struct ofpact *ofpacts; /* Series of "struct ofpact"s. */
 size_t ofpacts_len; /* Length of ofpacts, in bytes. */
 
+bool has_groups;/* 'has_groups' is true if 'ofpacts' contains
+ * an OFPACT_GROUP action .*/
+
 struct bucket_counter stats;
 };
 
diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index bf0f8af54..6cb904f79 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -1375,6 +1375,10 @@ ofputil_pull_ofp11_buckets(struct ofpbuf *msg, size_t 
buckets_length,
 
 bucket->ofpacts = ofpbuf_steal_data();
 bucket->ofpacts_len = ofpacts.size;
+bucket->has_groups =
+(ofpact_find_type_flattened(bucket->ofpacts, OFPACT_GROUP,
+ofpact_end(bucket->ofpacts, bucket->ofpacts_len))
+!= NULL);
 ovs_list_push_back(buckets, >list_node);
 }
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index afecb24cb..b0779996e 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -586,6 +586,7 @@ struct ofgroup {
 struct ofputil_group_props props;
 
 struct rule_collection rules OVS_GUARDED;   /* Referring rules. */
+uint32_t rgroups OVS_GUARDED; /* Referring groups. */
 };
 
 struct pkt_stats {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0fbd6c380..f31d4160e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3075,7 +3075,9 @@ ofproto_group_unref(struct ofgroup *group)
 OVS_NO_THREAD_SAFETY_ANALYSIS
 {
 if (group && ovs_refcount_unref_relaxed(>ref_count) == 1) {
+/* There are no rules nor groups still referencing it. */
 ovs_assert(rule_collection_n(>rules) == 0);
+ovs_assert(group->rgroups == 0);
 ovsrcu_postpone(group_destroy_cb, group);
 }
 }
@@ -7100,6 +7102,45 @@ group_remove_rule(struct ofgroup *group, struct rule 
*rule)
 rule_collection_remove(>rules, rule);
 }
 
+static void
+group_add_rgroup(struct ofgroup *group)
+{
+group->rgroups++;
+}
+
+static void
+group_remove_rgroup(struct ofgroup *group)
+{
+group->rgroups--;
+}
+
+/* Iterates over the buckets and updates the rgroups count
+ * in the referenced groups. */
+static void
+update_group_references(struct ofproto *ofproto, struct ofgroup *ogroup,
+bool add)
+{
+struct ofputil_bucket *bucket;
+LIST_FOR_EACH (bucket, list_node, >buckets) {
+if (bucket->has_groups) {
+/* Iterate over the group actions and update the references */
+const struct ofpact_group *a;
+OFPACT_FOR_EACH_TYPE_FLATTENED (a, GROUP, bucket->ofpacts,
+bucket->ofpacts_len) {
+struct ofgroup *group;
+group = ofproto_group_lookup(ofproto, a->group_id,
+ OVS_VERSION_MAX, false);
+ovs_assert(group != NULL);
+if (add) {
+group_add_rgroup(group);
+} else {
+group_remove_rgroup(group);
+}
+}
+}
+}
+}
+
 static void
 append_group_stats(struct ofgroup *group, struct ovs_list *replies)
 OVS_REQUIRES(ofproto_mutex)
@@ -7112,7 +7153,7 @@ append_group_stats(struct ofgroup *group, struct ovs_list 
*replies)
 ogs.bucket_stats = xmalloc(group->n_buckets * sizeof *ogs.bucket_stats);
 
 /* Provider sets the packet and byte counts, we do the rest. */
-ogs.ref_count = rule_collection_n(>rules);
+ogs.ref_count = rule_collection_n(>rules) + group->rgroups;
 ogs.n_buckets = group->n_buckets;
 
 error = (ofproto->ofproto_class->group_get_stats
@@ -7349,6 +7390,7 @@ init_group(struct ofproto *ofproto, const struct 
ofputil_group_mod *gm,
  &(*ofgroup)->props),
   >props);
 rule_collection_init(&(*ofgroup)->rules);
+*CONST_CAST(uint32_t *, &((*ofgroup)->rgroups)) = 0;
 
 /* Make group visible from 'version'. */
 (*ofgroup)->versions = VERSIONS_INITIALIZER(version,
@@ -7391,6 +7433,8 @@ add_group_start(struct ofproto *ofproto,

[ovs-dev] [PATCH ovn] DNS: Make DNS lookups case insensitive.

2020-04-20 Thread Mark Michelson 
>From RFC 1035 Section 2.3.3:

"For all parts of the DNS that are part of the official protocol, all
comparisons between character strings (e.g., labels, domain names, etc.)
are done in a case-insensitive manner."

OVN was using case-sensitive lookups and therefore was not complying.
This change makes lookups case insensitive by storing lowercase record
names in the southbound database and converting incoming query names to
lowercase.

Signed-off-by: Mark Michelson 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1819069
Reported-by: Jianlin Shi 
---
 controller/pinctrl.c |  7 -
 lib/ovn-util.c   | 15 +++
 lib/ovn-util.h   |  5 
 northd/ovn-northd.c  | 15 ++-
 ovn-sb.xml   |  3 ++-
 tests/ovn.at | 61 
 6 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 8703641c2..79ae86968 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2368,7 +2368,12 @@ pinctrl_handle_dns_lookup(
 struct dns_data *d = iter->data;
 for (size_t i = 0; i < d->n_dps; i++) {
 if (d->dps[i] == dp_key) {
-answer_ips = smap_get(>records, ds_cstr(_name));
+/* DNS records in SBDB are stored in lowercase. Convert to 
lowercase
+ * to perform case insensitive lookup
+ */
+char *query_name_lower = str_tolower(ds_cstr(_name));
+answer_ips = smap_get(>records, query_name_lower);
+free(query_name_lower);
 if (answer_ips) {
 break;
 }
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 514e2489f..1b30c2e9a 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -21,6 +21,7 @@
 #include "openvswitch/ofp-parse.h"
 #include "ovn-nb-idl.h"
 #include "ovn-sb-idl.h"
+#include 
 
 VLOG_DEFINE_THIS_MODULE(ovn_util);
 
@@ -550,3 +551,17 @@ ip46_equals(const struct v46_ip *addr1, const struct 
v46_ip *addr2)
 (addr1->family == AF_INET ? addr1->ipv4 == addr2->ipv4 :
  IN6_ARE_ADDR_EQUAL(>ipv6, >ipv6)));
 }
+
+char *
+str_tolower(const char *orig)
+{
+char *copy = xmalloc(strlen(orig) + 1);
+char *p = copy;
+
+while (*orig) {
+*p++ = tolower(*orig++);
+}
+*p = '\0';
+
+return copy;
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 11238f61c..4076e8b9a 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -124,4 +124,9 @@ struct v46_ip {
 bool ip46_parse_cidr(const char *str, struct v46_ip *prefix,
  unsigned int *plen);
 bool ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2);
+
+/* Returns a lowercase copy of orig.
+ * Caller must free the returned string.
+ */
+char *str_tolower(const char *orig);
 #endif
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 616e52bb2..0219a4bb0 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -10706,7 +10706,20 @@ sync_dns_entries(struct northd_context *ctx, struct 
hmap *datapaths)
 dns_info->sb_dns,
 (struct sbrec_datapath_binding **)dns_info->sbs,
 dns_info->n_sbs);
-sbrec_dns_set_records(dns_info->sb_dns, _info->nb_dns->records);
+
+/* DNS lookups are case-insensitive. Convert records to lowercase so
+ * we can do consistent lookups when DNS requests arrive
+ */
+struct smap lower_records = SMAP_INITIALIZER(_records);
+struct smap_node *node;
+SMAP_FOR_EACH (node, _info->nb_dns->records) {
+smap_add_nocopy(_records, xstrdup(node->key),
+str_tolower(node->value));
+}
+
+sbrec_dns_set_records(dns_info->sb_dns, _records);
+
+smap_destroy(_records);
 free(dns_info->sbs);
 free(dns_info);
 }
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 72466b97e..5f8da534c 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3597,7 +3597,8 @@ tcp.flags = RST;
 
   Key-value pair of DNS records with DNS query name as the key
   and a string of IP address(es) separated by comma or space as the
-  value.
+  value. ovn-northd stores the DNS query name in all lowercase in order to
+  facilitate case-insensitive lookups.
 
   Example:  "vm1.ovn.org" = "10.0.0.4 aef0::4"
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 7858c496e..a52e644f0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8357,6 +8357,12 @@ set_dns_params() {
 # IPv4 address - 10.0.0.4
 expected_dns_answer=${query_name}00010001${ttl}00040a04
 ;;
+VM1)
+# VM1.OVN.ORG
+query_name=03564d31034f564e034f524700
+# IPv4 address - 10.0.0.4
+expected_dns_answer=${query_name}00010001${ttl}00040a04
+;;
 vm2)
 # vm2.ovn.org
 query_name=03766d32036f766e036f726700
@@ -8519,6 +8525,29 @@ reset_pcap_file hv1-vif2 hv1/vif2
 rm -f 1.expected

[ovs-dev] [PATCH] ofproto: report coverage on hitting datapath flow limit

2020-04-20 Thread Gowrishankar Muthukrishnan 
Whenever the number of flows in the datapath crosses above
the flow limit set/autoconfigured, it is helpful to report
this event through coverage counter for an operator/devops
engineer to know and take proactive corrections in the
switch configuration.

Today, these events are reported in ovs vswitch log when
a new flow can not be inserted in upcall processing in which
case ovs writes a warning, otherwise an auto correction
made by ovs to flush old flows without any intimation at all.

Signed-off-by: Gowrishankar Muthukrishnan 
---
 ofproto/ofproto-dpif-upcall.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5e08ef10d..a76532ec7 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -56,6 +56,7 @@ COVERAGE_DEFINE(handler_duplicate_upcall);
 COVERAGE_DEFINE(upcall_ukey_contention);
 COVERAGE_DEFINE(upcall_ukey_replace);
 COVERAGE_DEFINE(revalidate_missed_dp_flow);
+COVERAGE_DEFINE(upcall_flow_limit_hit);
 
 /* A thread that reads upcalls from dpif, forwards each upcall's packet,
  * and possibly sets up a kernel flow as a cache. */
@@ -1281,6 +1282,7 @@ should_install_flow(struct udpif *udpif, struct upcall 
*upcall)
 
 atomic_read_relaxed(>flow_limit, _limit);
 if (udpif_get_n_flows(udpif) >= flow_limit) {
+COVERAGE_INC(upcall_flow_limit_hit);
 VLOG_WARN_RL(, "upcall: datapath flow limit reached");
 return false;
 }
@@ -2624,6 +2626,10 @@ revalidate(struct revalidator *revalidator)
  *   datapath flows, so we will recover before all the flows are
  *   gone.) */
 n_dp_flows = udpif_get_n_flows(udpif);
+if (n_dp_flows >= flow_limit) {
+COVERAGE_INC(upcall_flow_limit_hit);
+}
+
 kill_them_all = n_dp_flows > flow_limit * 2;
 max_idle = n_dp_flows > flow_limit ? 100 : ofproto_max_idle;
 
-- 
2.21.1

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

Re: [ovs-dev] [PATCH] Utilities: Add the ovs_dump_ofpacts command to gdb

2020-04-20 Thread Eelco Chaudron 

William,

Can you try adding this somewhere at the top after the other import 
section?


from __future__ import print_function

And let me know if it works?

Thanks,

Eelco


On 20 Apr 2020, at 15:27, Eelco Chaudron wrote:


On 20 Apr 2020, at 15:06, William Tu wrote:



Is this due to python2/3? My python version:
boxes:~/ovs# python
Python 3.5.2 (default, Oct  8 2019, 13:06:37)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more 
information.



Hi Eelco,
That's the only error I got

(gdb) source ./utilities/gdb/ovs_gdb.py
  File "./utilities/gdb/ovs_gdb.py", line 164
print(self.message, end='')
   ^
SyntaxError: invalid syntax


Looks like the end=‘’ option is only supported in later version of 
3.x.
I’ll try to find a way to make this work for all version of Python 
and sent a patch later this week/early next week.


//Eelco

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

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

Re: [ovs-dev] [PATCH] Utilities: Add the ovs_dump_ofpacts command to gdb

2020-04-20 Thread Eelco Chaudron 



On 20 Apr 2020, at 15:06, William Tu wrote:



Is this due to python2/3? My python version:
boxes:~/ovs# python
Python 3.5.2 (default, Oct  8 2019, 13:06:37)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more 
information.



Hi Eelco,
That's the only error I got

(gdb) source ./utilities/gdb/ovs_gdb.py
  File "./utilities/gdb/ovs_gdb.py", line 164
print(self.message, end='')
   ^
SyntaxError: invalid syntax


Looks like the end=‘’ option is only supported in later version of 
3.x.
I’ll try to find a way to make this work for all version of Python and 
sent a patch later this week/early next week.


//Eelco

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

Re: [ovs-dev] [PATCH] AB bonding: Add "primary" interface concept

2020-04-20 Thread Aaron Conole 
Hi Jeff,

Jeff Squyres via dev  writes:

> A "primary" slave interface will always become active if it is
> enabled.  The "primary" concept exists in Linux kernel network
> interface bonding, but did not previously exist in OVS bonding.
>
> Only one "primary" slave inteface is supported per bond, and
> is only supported for active/backup bonding.
>
> The primary slave interface is designated via
> "other_config:bond-primary" when creating a bond.
>
> Signed-off-by: Jeff Squyres 
> ---

I only did a cursory review (sorry).  I plan to test this tomorrow.

>  ofproto/bond.c| 58 +++-
>  ofproto/bond.h|  1 +
>  tests/lacp.at |  1 +
>  tests/ofproto-dpif.at | 78 ++-
>  vswitchd/bridge.c |  5 +++
>  vswitchd/vswitch.xml  |  8 +
>  6 files changed, 149 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index 405202fb6..2437246ca 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -93,6 +93,7 @@ struct bond_slave {
>  /* Link status. */
>  bool enabled;   /* May be chosen for flows? */
>  bool may_enable;/* Client considers this slave bondable. */
> +bool is_primary;/* This slave is preferred over others. */
>  long long delay_expires;/* Time after which 'enabled' may change. */
>  
>  /* Rebalancing info.  Used only by bond_rebalance(). */
> @@ -126,6 +127,7 @@ struct bond {
>  enum lacp_status lacp_status; /* Status of LACP negotiations. */
>  bool bond_revalidate;   /* True if flows need revalidation. */
>  uint32_t basis; /* Basis for flow hash function. */
> +char *primary;  /* Name of the primary slave interface. */
>  
>  /* SLB specific bonding info. */
>  struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */
> @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct 
> ofproto_dpif *ofproto)
>  
>  bond->active_slave_mac = eth_addr_zero;
>  bond->active_slave_changed = false;
> +bond->primary = NULL;
>  
>  bond_reconfigure(bond, s);
>  return bond;
> @@ -290,6 +293,7 @@ bond_unref(struct bond *bond)
>  update_recirc_rules__(bond);
>  
>  hmap_destroy(>pr_rule_ops);
> +free(bond->primary);
>  free(bond->name);
>  free(bond);
>  }
> @@ -459,6 +463,31 @@ bond_reconfigure(struct bond *bond, const struct 
> bond_settings *s)
>  bond->bond_revalidate = false;
>  }
>  
> +/*
> + * If a primary interface is set on the new settings:
> + * 1. If the bond has no primary previously set, save it and
> + * revalidate.
> + * 2. If the bond has a different primary previously set, save the
> + * new one and revalidate.
> + * 3. If the bond has the same primary previously set, do nothing.
> + */
> +if (s->primary) {
> +bool changed = false;
> +if (bond->primary) {
> +if (strcmp(bond->primary, s->primary) != 0) {

   ^
   through the codebase we typically use if(strcmp(...))

> +free(bond->primary);
> +changed = true;
> +}
> +} else {
> +changed = true;
> +}
> +
> +if (changed) {
> +bond->primary = xstrdup(s->primary);
> +revalidate = true;
> +}
> +}
> +
>  if (bond->balance != BM_AB) {
>  if (!bond->recirc_id) {
>  bond->recirc_id = recirc_alloc_id(bond->ofproto);
> @@ -549,6 +578,12 @@ bond_slave_register(struct bond *bond, void *slave_,
>  slave->name = xstrdup(netdev_get_name(netdev));
>  bond->bond_revalidate = true;
>  
> +if (bond->primary && strcmp(bond->primary, slave->name) == 0) {

^ as above, !strcmp()

> +slave->is_primary = true;
> +} else {
> +slave->is_primary = false;
> +}
> +
>  slave->enabled = false;
>  bond_enable_slave(slave, netdev_get_carrier(netdev));
>  }
> @@ -644,6 +679,7 @@ bond_run(struct bond *bond, enum lacp_status lacp_status)
>  {
>  struct bond_slave *slave;
>  bool revalidate;
> +struct bond_slave *primary;
>  
>  ovs_rwlock_wrlock();
>  if (bond->lacp_status != lacp_status) {
> @@ -659,11 +695,19 @@ bond_run(struct bond *bond, enum lacp_status 
> lacp_status)
>  }
>  
>  /* Enable slaves based on link status and LACP feedback. */
> +primary = NULL;
>  HMAP_FOR_EACH (slave, hmap_node, >slaves) {
>  bond_link_status_update(slave);
>  slave->change_seq = seq_read(connectivity_seq_get());
> +
> +/* Discover if there is an active slave marked "primary". */
> +if (bond->balance == BM_AB && slave->is_primary && slave->enabled) {
> +primary = slave;
> +}
>  }
> -if (!bond->active_slave || !bond->active_slave->enabled) {
> +
> +

Re: [ovs-dev] [PATCH] Utilities: Add the ovs_dump_ofpacts command to gdb

2020-04-20 Thread William Tu 
On Mon, Apr 20, 2020 at 10:02:15AM +0200, Eelco Chaudron wrote:
> 
> 
> On 19 Apr 2020, at 17:58, William Tu wrote:
> 
> >On Fri, Apr 17, 2020 at 02:51:34PM +0200, Eelco Chaudron wrote:
> >>This adds the ovs_dump_ifpacts command:
> >>
> >>(gdb) help ovs_dump_ofpacts
> >>Dump all actions in an ofpacts set
> >>Usage: ovs_dump_ofpacts  
> >>
> >>: Pointer to set of ofpact structures.
> >>: Total length of the set.
> >>
> >>Example dumping all actions when in the clone_xlate_actions()
> >>function:
> >>
> >>(gdb) ovs_dump_ofpacts actions actions_len
> >>(struct ofpact *) 0x561c7be487c8: {type = OFPACT_SET_FIELD, raw =
> >>255 '', len = 24}
> >>(struct ofpact *) 0x561c7be487e0: {type = OFPACT_SET_FIELD, raw =
> >>255 '', len = 24}
> >>(struct ofpact *) 0x561c7be487f8: {type = OFPACT_SET_FIELD, raw =
> >>255 '', len = 24}
> >>(struct ofpact *) 0x561c7be48810: {type = OFPACT_SET_FIELD, raw =
> >>255 '', len = 32}
> >>(struct ofpact *) 0x561c7be48830: {type = OFPACT_SET_FIELD, raw =
> >>255 '', len = 24}
> >>(struct ofpact *) 0x561c7be48848: {type = OFPACT_RESUBMIT, raw = 38
> >>'&', len = 16}
> >>
> >>Signed-off-by: Eelco Chaudron 
> >>---
> >Hi Eelco,
> >
> >I'm trying to test this patch. But I hit another problem.
> >Can you help me with the issue? Sorry this is not related to your patch.
> >
> >root@osboxes:~/ovs# gdb $(which ovs-vswitchd) $(pidof ovs-vswitchd)
> >GNU gdb (GDB) 8.0.50.20170501-git
> >Copyright (C) 2017 Free Software Foundation, Inc.
> >License GPLv3+: GNU GPL version 3 or later
> >
> >This is free software: you are free to change and redistribute it.
> >There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> >and "show warranty" for details.
> >This GDB was configured as "x86_64-pc-linux-gnu".
> >Type "show configuration" for configuration details.
> >For bug reporting instructions, please see:
> >.
> >Find the GDB manual and other documentation resources online at:
> >.
> >For help, type "help".
> >Type "apropos word" to search for commands related to "word"...
> >Reading symbols from /usr/local/sbin/ovs-vswitchd...done.
> >Attaching to program: /usr/local/sbin/ovs-vswitchd, process 58469
> >[New LWP 58473]
> >[New LWP 58474]
> >[New LWP 58475]
> >[New LWP 58484]
> >[New LWP 58485]
> >[New LWP 58486]
> >[New LWP 58487]
> >[New LWP 58578]
> >[New LWP 58579]
> >[Thread debugging using libthread_db enabled]
> >Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> >0x7fe63d33774d in poll () from /lib/x86_64-linux-gnu/libc.so.6
> >(gdb) source ./utilities/gdb/ovs_gdb.py
> >  File "./utilities/gdb/ovs_gdb.py", line 164
> >print(self.message, end='')
> 
> The script should work with both versions of python. Is the above the only
> error you get? Or did you forget to cut/paste the actual error message?

> 
> >Is this due to python2/3? My python version:
> >boxes:~/ovs# python
> >Python 3.5.2 (default, Oct  8 2019, 13:06:37)
> >[GCC 5.4.0 20160609] on linux
> >Type "help", "copyright", "credits" or "license" for more information.
> 
Hi Eelco,
That's the only error I got

(gdb) source ./utilities/gdb/ovs_gdb.py
  File "./utilities/gdb/ovs_gdb.py", line 164
print(self.message, end='')
   ^
SyntaxError: invalid syntax
(gdb) 

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

Re: [ovs-dev] OVS DPDK VF port representors not working

2020-04-20 Thread Ilya Maximets 
On 4/19/20 6:08 PM, Ravi Kerur wrote:
> 
> 
> On Sun, Apr 19, 2020 at 6:14 AM Ilya Maximets  > wrote:
> 
> On 4/18/20 2:33 PM, Ravi Kerur wrote:
> >
> >
> > On Fri, Apr 17, 2020 at 9:57 AM Ilya Maximets    >> wrote:
> >
> >     On 4/16/20 11:00 PM, Ravi Kerur wrote:
> >     > Hello OvS-DPDK team,
> >     >
> >     > We are expanding our usage of OvS-DPDK and want to use VF port 
> representors
> >     > for one use-case. Going through the following document it is 
> unclear how
> >     > this functionality can be achieved
> >     >
> >     > http://docs.openvswitch.org/en/latest/topics/dpdk/phy/
> >     >
> >     > I believe following things need to be configured before 
> configuring OvS, I
> >     > am using Intel NICs for evaluation
> >     >
> >     > (1) Create VFs on Intel NICs(82599). This can only happen when 
> Intel NIC is
> >     > bound to its respective kernel driver (ixgbe) via
> >     >
> >     > echo 2 > /sys/devices//sriov_numvfs
> >     >
> >     >
> >     > It creates 2 VF interfaces bound to ixgbevf driver
> >     >
> >     >
> >     > (2) Using driverctl, bind VF and PF interfaces to vfio-pci. 
> Moment PF
> >     > device is bound to vfio-pci, VF interfaces vanish
> >     >
> >     >
> >     > Question based on the OvS document shown above
> >     >
> >     >
> >     > (1) Binding PF interface?
> >     >
> >     >
> >     > ovs-vsctl add-port br0 dpdk-pf -- set Interface dpdk-pf type=dpdk
> >     > options:dpdk-devargs=:08:00.0
> >     >
> >     >
> >     > What driver interface <:08:00.0> is bound to? igb_uio, 
> vfio-pci or
> >     > kernel driver?
> >
> >     I believe that it was tested with igb_uio driver, but I never tried 
> it myself.
> >
> >     In order to have sr-iov configuration support with vfio-pci driver 
> you need
> >     a brand new v5.7-rc1 kernel and DPDK changes that are not merged 
> yet:
> >     https://patchwork.dpdk.org/project/dpdk/list/?series=9356
> >     Not sure if it will work smoothly with OVS out of the box, though.
> >
> >
> > Thanks Ilya for the link. Will look into it. 
> >
> > Unfortunately igb_uio doesn't work either. Basics of creating VF's out 
> of PF and assigning them to DPDK driver itself is not working.
> > My understanding is
> > (1) In order to create VF's NIC should be bound to its kernel driver. 
> Cannot create VFs when NIC is bound to igb_uio or vfio-pci
> > (2) No mechanism exists to bind both PF and VF to igb_uio to use this 
> feature
> >
> > Output below shows what I am observing on :18:00.0
> >
> > (1) PCI device has PF and 1 VF, both assigned to kernel driver
> >
> > driverctl list-devices network
> >
> > :04:00.0 igb
> >
> > *:18:00.0 ixgbe*
> >
> > :18:00.1 ixgbe
> >
> > *:18:10.0 ixgbevf*
> >
> > :b3:00.0 vfio-pci [*]
> >
> > :b3:00.1 ixgbe
> >
> >
> > (2) Bind :18:00.0 to igb_uio, VF device is gone
> > # driverctl set-override :18:00.0 igb_uio 
> >
> > #driverctl list-devices network
> >
> > :04:00.0 igb
> >
> > *:18:00.0 igb_uio [*]*
> >
> > :18:00.1 ixgbe
> >
> > :b3:00.0 vfio-pci [*]
> >
> > :b3:00.1 ixgbe
> >
> >
> > (3) Try to create VF when bound to igb_uio
> >
> >
> > echo 1 > /sys/bus/pci/devices/\:18\:00.0/sriov_numvfs
> >
> > bash: echo: write error: No such file or directory
> 
> OVS and DPDK docs both says that knob should be "max_vfs", not 
> "sriov_numvfs".
> I didn't test, but looking at the code of igb_uio driver it actually 
> creates
> this sysfs entry.
> 
> 
> Thanks for your kind help. My mistake. On Ubuntu max_vfs is created in 
> 
> /sys/devices/pci:16/:16:02.0/:18:00.0/max_vfs instead of usual
> 
> /sys/bus/pci/devices/\:18\:00.0/
> 
>  I am able to create VF's when PF is attached to igb_uio. 
> When adding dpdk-pf port to bridge I get following errors (OvS 2.13.90, DPDK 
> 19.11). Please note I have started with empty conf.db in OvS.
> 
> driverctl list-devices network
> 
> :04:00.0 igb
> 
> :18:00.0 igb_uio [*]
> 
> :18:00.1 ixgbe
> 
> :18:10.0 vfio-pci [*]
> 
> :18:10.2 vfio-pci [*]
> 
> :b3:00.0 vfio-pci [*]
> 
> :b3:00.1 ixgbe
> 
> 
> ovs-vsctl add-br br-phy -- set bridge br-phy datapath_type=netdev
> 
> ovs-vsctl add-port br-phy dpdk-pf -- set Interface dpdk-pf type=dpdk 
> options:dpdk-devargs=:18:00.0
> 
> ovs-vsctl: Error detected while setting up 'dpdk-pf': could not add network 
> device dpdk-pf to ofproto (Invalid argument).  See ovs-vswitchd log for 
> details.
> 
>

Re: [ovs-dev] [PATCH] Utilities: Add the ovs_dump_ofpacts command to gdb

2020-04-20 Thread Eelco Chaudron 




On 19 Apr 2020, at 17:58, William Tu wrote:


On Fri, Apr 17, 2020 at 02:51:34PM +0200, Eelco Chaudron wrote:

This adds the ovs_dump_ifpacts command:

(gdb) help ovs_dump_ofpacts
Dump all actions in an ofpacts set
Usage: ovs_dump_ofpacts  

: Pointer to set of ofpact structures.
: Total length of the set.

Example dumping all actions when in the clone_xlate_actions() 
function:


(gdb) ovs_dump_ofpacts actions actions_len
(struct ofpact *) 0x561c7be487c8: {type = OFPACT_SET_FIELD, raw = 
255 '', len = 24}
(struct ofpact *) 0x561c7be487e0: {type = OFPACT_SET_FIELD, raw = 
255 '', len = 24}
(struct ofpact *) 0x561c7be487f8: {type = OFPACT_SET_FIELD, raw = 
255 '', len = 24}
(struct ofpact *) 0x561c7be48810: {type = OFPACT_SET_FIELD, raw = 
255 '', len = 32}
(struct ofpact *) 0x561c7be48830: {type = OFPACT_SET_FIELD, raw = 
255 '', len = 24}
(struct ofpact *) 0x561c7be48848: {type = OFPACT_RESUBMIT, raw = 
38 '&', len = 16}


Signed-off-by: Eelco Chaudron 
---

Hi Eelco,

I'm trying to test this patch. But I hit another problem.
Can you help me with the issue? Sorry this is not related to your 
patch.


root@osboxes:~/ovs# gdb $(which ovs-vswitchd) $(pidof ovs-vswitchd)
GNU gdb (GDB) 8.0.50.20170501-git
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 


This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show 
copying"

and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
.
Find the GDB manual and other documentation resources online at:
.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/local/sbin/ovs-vswitchd...done.
Attaching to program: /usr/local/sbin/ovs-vswitchd, process 58469
[New LWP 58473]
[New LWP 58474]
[New LWP 58475]
[New LWP 58484]
[New LWP 58485]
[New LWP 58486]
[New LWP 58487]
[New LWP 58578]
[New LWP 58579]
[Thread debugging using libthread_db enabled]
Using host libthread_db library 
"/lib/x86_64-linux-gnu/libthread_db.so.1".

0x7fe63d33774d in poll () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) source ./utilities/gdb/ovs_gdb.py
  File "./utilities/gdb/ovs_gdb.py", line 164
print(self.message, end='')


The script should work with both versions of python. Is the above the 
only error you get? Or did you forget to cut/paste the actual error 
message?



Is this due to python2/3? My python version:
boxes:~/ovs# python
Python 3.5.2 (default, Oct  8 2019, 13:06:37)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.


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

Re: [ovs-dev] [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces

2020-04-20 Thread Roni Bar Yanai 
Hi Ben, Ilya

Going back to this thread. We've tried app-ctl approach and it fails on 
consistency
problem. Orchestrator can configure on full system init, but now executing 
local 
restart will lose the configuration (again for bifurcated driver it is not 
problem).

The requirement for setting VF mac through the representor, comes from different
use cases such as Open Stack and Kubernetes. VF is used with pass-through and
untrusted user VM/Pod/Container . The MAC address is set by the orchestrator
only. For Linux use case there is ip tool for doing that, and Linux takes care 
of the
consistency. For OVS-DPDK with port representor, there is no way to configure 
VF MAC
address (except of for bifurcated drivers).

The requirement is to enable orchestrator configuration of VF MAC address in 
DPDK,
when working with port representor.
The solution should handle the generic use case and not bifurcated drivers only.
The MAC should be kept and configured in case of OVS restart. 

Maybe we can go back to the first solution and open set MAC to port representors
only. We treat the solution as a generic solution and ignore bifurcated 
drivers. 
When user configure the representor MAC (which is a reflection of the VF), the 
VF MAC is configured. The MAC address is saved in the DB. In case of returning 
back
to kernel there is no problem because the DB MAC applies only for DPDK 
representor.
For bifurcated driver, user can cause in consistency, but this is a unique case 
and 
we cannot defend against misuse of bifurcated drivers.

Any thoughts?

>-Original Message-
>From: dev  On Behalf Of Roni Bar Yanai
>Sent: Sunday, January 26, 2020 9:47 AM
>To: Ben Pfaff 
>Cc: d...@openvswitch.org; Adrian Chiris ; Ilya Maximets
>; Ameer Mahagneh ; Eveline
>Raine 
>Subject: Re: [ovs-dev] [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>interfaces
>
>Thanks Ben.
>
>>-Original Message-
>>From: Ben Pfaff 
>>Sent: Thursday, January 23, 2020 11:33 PM
>>To: Roni Bar Yanai 
>>Cc: Ilya Maximets ; Ophir Munk
>>; Eveline Raine ;
>>d...@openvswitch.org; Moshe Levi ; Adrian Chiris
>>; Majd Dibbiny ; Ameer
>>Mahagneh 
>>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>>
>>The main problem is that the database is stateful.  I would not have
>>the same objection to an RPC to set an Ethernet address.  This could be
>>implemented via the ovs-appctl interface, if local-only is acceptable,
>>or via OpenFlow, if the controller needs to do it.
>>
>>On Wed, Jan 22, 2020 at 02:49:47PM +, Roni Bar Yanai wrote:
>>> Hi Ilya, Ben
>>>
>>> What is the plan to manage MAC addresses of DPDK ports with representors?
>>> The solution should be generic and support non bifurcated drivers.
>>> Currently we cannot integrate DPDK with port representors in
>>> platforms such as open stack.
>>>
>>> Thanks,
>>> Roni
>>> >-Original Message-
>>> >From: Roni Bar Yanai
>>> >Sent: Monday, January 13, 2020 1:24 PM
>>> >To: Ilya Maximets ; Ophir Munk
>>> >; Ben Pfaff 
>>> >Cc: Eveline Raine ; d...@openvswitch.org;
>>> >Moshe Levi ; Adrian Chiris
>>> >; Majd Dibbiny ; Ameer
>>> >Mahagneh 
>>> >Subject: RE: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>> >interfaces
>>> >
>>> >
>>> >
>>> >>-Original Message-
>>> >>From: Ilya Maximets 
>>> >>Sent: Thursday, January 9, 2020 3:28 PM
>>> >>To: Ophir Munk ; Ben Pfaff ;
>>> >>Ilya Maximets 
>>> >>Cc: Eveline Raine ; d...@openvswitch.org;
>>> >>Moshe Levi ; Adrian Chiris
>>> >>; Majd Dibbiny ; Roni Bar
>>> >>Yanai ; Ameer Mahagneh
>>
>>> >>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>> >>interfaces
>>> >>
>>> >>On 08.01.2020 11:48, Ophir Munk wrote:
>>> >>> Hi,
>>> >>> There is an important use case for having OVS change MAC
>>> >>> addresses of dpdk
>>> >>interfaces.
>>> >>> OpenStack for example needs to update the MAC address of a VF
>>> >>> assigned to a
>>> >>VM, where the corresponding VF representor is owned by dpdk.
>>> >>> For some NIC vendors using "ifconfig" or "ip" commands - is not
>>> >>> an option (if the
>>> >>NIC is not bifurcated).
>>> >>> Therefore OpenStack should use the OVS API to set the MAC address
>>> >>> for dpdk
>>> >>interfaces.
>>> >>> Along with Ben's explanation it seems right to only allow
>>> >>> "internal" or
>>"dpdk"
>>> >>port types to set the MAC.
>>> >>
>>> >>There are still same issues for DPDK ports.  Just because OVS
>>> >>doesn't necessarily "owns" them.  In case of Mellanox NICs, ports
>>> >>are still could be
>>> >configured by 'ip'
>>> >>tool due to bifurcated drivers.  And this produces the same confusion.
>>> >>
>>> >This makes it hard to integrate OVS-DPDK in cloud with port
>>> >representor. There Is no way to configure MAC address for untrusted
>>> >VM. If the driver is bifurcated there is some WA,  but this is not a
>>> >generic
>>solution and will not be used.
>>> >Any thoughts on how we can configure MAC for untrusted VM on such
>>> >topologies?
>>> >>>
>>> >>> Testing both patches [1] and [2] - passed

Re: [ovs-dev] [PATCH] ofproto: Fix for frequent invalidation of flows due to mismatch in mask bits

2020-04-20 Thread Vishal Deep Ajmera via dev 
> >
> > Any thoughts on this? Should I write an equivalent of flow_wc_map() and
> use
> > it in xlate_wc_finish()
> > or we use a stricter version i.e. flow_wildcards_init_for_packet() and
use
> > in revalidate_ukey__?
> 
> My big problem for review here is that I don't understand the motivation
> for moving the code around.  Sure, it *works*, but why?  Why did you
> think to do it?

The reason to move the code from xlate_wc_finish() to revalidate_ukey__() is
as it caused quite a few unit tests to fail.
After looking at the failures I notice that Megaflow is not matching the
'expected' value.

For e.g. failure for test 0738 is:

-Megaflow:
recirc_id=0,eth,ip,tun_id=0,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_fl
ags=-df-csum-key,in_port=1,nw_ecn=3,nw_frag=no
+Megaflow:
recirc_id=0,eth,ip,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=3,tun_flags=-df-c
sum-key,in_port=1,nw_ecn=3,nw_frag=no

flow_wildcards_init_for_packet() unwildcards 'tun_id' only when either
FLOW_TNL_F_KEY is set in tunnel flags or tun->ip_dst is not set & tun_id is
non-zero.

However the 'expected' Megaflow itself is not correct in this case as
tun_flags = -key and tun_dst is set, so tun_id should have been wildcarded
but it is not.

Let me know if it makes sense.

Warm Regards,
Vishal Ajmera
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev