[ovs-dev] [PATCH] ovn: Support configuring the BFD params for the tunnel interfaces
From: Numan Siddique
With this commit the users can override the default values of
the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
This can be useful to debug any issues related to BFD, like BFD
state changes seen frequently.
ovn-controller checks for the options 'ovn-bfd-min-rx',
'ovn-bfd-min-tx', 'ovn-bfd-decay-min-rx' and 'ovn-bfd-mult' in the
external-ids of OpenvSwitch table row and configures these BFD
values to the tunnel interfaces.
Signed-off-by: Numan Siddique
---
v1 -> v2
---
* Addressed review comments by Miguel - added the option 'mult' to configure.
ovn/controller/bfd.c| 50 +
ovn/controller/bfd.h| 2 ++
ovn/controller/ovn-controller.8.xml | 10 ++
ovn/controller/ovn-controller.c | 10 +++---
tests/ovn.at| 26 +++
5 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index 051781f38..e2ca9cf49 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
static void
-interface_set_bfd(const struct ovsrec_interface *iface, bool bfd_setting)
+interface_set_bfd(const struct ovsrec_interface *iface, struct smap *bfd)
{
-const char *new_setting = bfd_setting ? "true":"false";
-const char *current_setting = smap_get(&iface->bfd, "enable");
-if (current_setting && !strcmp(current_setting, new_setting)) {
-/* If already set to the desired setting we skip setting it again
- * to avoid flapping to bfd initialization state */
-return;
-}
-const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting);
ovsrec_interface_verify_bfd(iface);
-ovsrec_interface_set_bfd(iface, &bfd);
-VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" : "Disabled",
-iface->name);
+ovsrec_interface_set_bfd(iface, bfd);
}
void
@@ -265,6 +255,7 @@ void
bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
const struct ovsrec_interface_table *interface_table,
+const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
const struct hmap *local_datapaths)
@@ -292,15 +283,46 @@ bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
}
}
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_table_first(ovs_table);
+const char *min_rx = NULL;
+const char *decay_min_rx = NULL;
+const char *min_tx = NULL;
+const char *mult = NULL;
+if (cfg) {
+min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
+decay_min_rx = smap_get(&cfg->external_ids, "ovn-bfd-decay-min-rx");
+min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
+mult = smap_get(&cfg->external_ids, "ovn-bfd-mult");
+}
+struct smap bfd = SMAP_INITIALIZER(&bfd);
+if (min_rx) {
+smap_add(&bfd, "min_rx", min_rx);
+}
+if (decay_min_rx) {
+smap_add(&bfd, "decay_min_rx", decay_min_rx);
+}
+if (min_tx) {
+smap_add(&bfd, "min_tx", min_tx);
+}
+if (mult) {
+smap_add(&bfd, "mult", mult);
+}
/* Enable or disable bfd */
const struct ovsrec_interface *iface;
OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
if (sset_contains(&tunnels, iface->name)) {
-interface_set_bfd(
-iface, sset_contains(&bfd_ifaces, iface->name));
+bool bfd_setting = sset_contains(&bfd_ifaces, iface->name);
+smap_replace(&bfd, "enable", bfd_setting ? "true":"false");
+if (!smap_equal(&iface->bfd, &bfd)) {
+interface_set_bfd(iface, &bfd);
+VLOG_INFO("%s BFD on interface %s",
+ bfd_setting ? "Enabled" : "Disabled", iface->name);
+}
}
}
+smap_destroy(&bfd);
sset_destroy(&tunnels);
sset_destroy(&bfd_ifaces);
sset_destroy(&bfd_chassis);
diff --git a/ovn/controller/bfd.h b/ovn/controller/bfd.h
index 7ea345a3e..9243bec69 100644
--- a/ovn/controller/bfd.h
+++ b/ovn/controller/bfd.h
@@ -21,6 +21,7 @@ struct ovsdb_idl;
struct ovsdb_idl_index;
struct ovsrec_bridge;
struct ovsrec_interface_table;
+struct ovsrec_open_vswitch_table;
struct sbrec_chassis;
struct sset;
@@ -28,6 +29,7 @@ void bfd_register_ovs_idl(struct ovsdb_idl *);
void bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
const struct ovsrec_interface_table *interface_table,
+ const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
[ovs-dev] [PATCH v2] ovn: Support configuring the BFD params for the tunnel interfaces
From: Numan Siddique
With this commit the users can override the default values of
the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
This can be useful to debug any issues related to BFD, like BFD
state changes seen frequently.
ovn-controller checks for the options 'ovn-bfd-min-rx',
'ovn-bfd-min-tx', 'ovn-bfd-decay-min-rx' and 'ovn-bfd-mult' in the
external-ids of OpenvSwitch table row and configures these BFD
values to the tunnel interfaces.
Signed-off-by: Numan Siddique
---
v1 -> v2
---
* Addressed review comments by Miguel - added the option 'mult' to configure.
ovn/controller/bfd.c| 50 +
ovn/controller/bfd.h| 2 ++
ovn/controller/ovn-controller.8.xml | 10 ++
ovn/controller/ovn-controller.c | 10 +++---
tests/ovn.at| 26 +++
5 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index 051781f38..e2ca9cf49 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
static void
-interface_set_bfd(const struct ovsrec_interface *iface, bool bfd_setting)
+interface_set_bfd(const struct ovsrec_interface *iface, struct smap *bfd)
{
-const char *new_setting = bfd_setting ? "true":"false";
-const char *current_setting = smap_get(&iface->bfd, "enable");
-if (current_setting && !strcmp(current_setting, new_setting)) {
-/* If already set to the desired setting we skip setting it again
- * to avoid flapping to bfd initialization state */
-return;
-}
-const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting);
ovsrec_interface_verify_bfd(iface);
-ovsrec_interface_set_bfd(iface, &bfd);
-VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" : "Disabled",
-iface->name);
+ovsrec_interface_set_bfd(iface, bfd);
}
void
@@ -265,6 +255,7 @@ void
bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
const struct ovsrec_interface_table *interface_table,
+const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
const struct hmap *local_datapaths)
@@ -292,15 +283,46 @@ bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
}
}
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_table_first(ovs_table);
+const char *min_rx = NULL;
+const char *decay_min_rx = NULL;
+const char *min_tx = NULL;
+const char *mult = NULL;
+if (cfg) {
+min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
+decay_min_rx = smap_get(&cfg->external_ids, "ovn-bfd-decay-min-rx");
+min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
+mult = smap_get(&cfg->external_ids, "ovn-bfd-mult");
+}
+struct smap bfd = SMAP_INITIALIZER(&bfd);
+if (min_rx) {
+smap_add(&bfd, "min_rx", min_rx);
+}
+if (decay_min_rx) {
+smap_add(&bfd, "decay_min_rx", decay_min_rx);
+}
+if (min_tx) {
+smap_add(&bfd, "min_tx", min_tx);
+}
+if (mult) {
+smap_add(&bfd, "mult", mult);
+}
/* Enable or disable bfd */
const struct ovsrec_interface *iface;
OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
if (sset_contains(&tunnels, iface->name)) {
-interface_set_bfd(
-iface, sset_contains(&bfd_ifaces, iface->name));
+bool bfd_setting = sset_contains(&bfd_ifaces, iface->name);
+smap_replace(&bfd, "enable", bfd_setting ? "true":"false");
+if (!smap_equal(&iface->bfd, &bfd)) {
+interface_set_bfd(iface, &bfd);
+VLOG_INFO("%s BFD on interface %s",
+ bfd_setting ? "Enabled" : "Disabled", iface->name);
+}
}
}
+smap_destroy(&bfd);
sset_destroy(&tunnels);
sset_destroy(&bfd_ifaces);
sset_destroy(&bfd_chassis);
diff --git a/ovn/controller/bfd.h b/ovn/controller/bfd.h
index 7ea345a3e..9243bec69 100644
--- a/ovn/controller/bfd.h
+++ b/ovn/controller/bfd.h
@@ -21,6 +21,7 @@ struct ovsdb_idl;
struct ovsdb_idl_index;
struct ovsrec_bridge;
struct ovsrec_interface_table;
+struct ovsrec_open_vswitch_table;
struct sbrec_chassis;
struct sset;
@@ -28,6 +29,7 @@ void bfd_register_ovs_idl(struct ovsdb_idl *);
void bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
const struct ovsrec_interface_table *interface_table,
+ const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
Re: [ovs-dev] [PATCH] ovn: Support configuring the BFD params for the tunnel interfaces
On Tue, Sep 25, 2018 at 10:18 PM Miguel Angel Ajo Pelayo <
majop...@redhat.com> wrote:
> Nak,
>
> Great work, this will be useful to adjust parameters based on specific
> network conditions, or while debugging network issues (to reduce flapping).
>
> I miss the "mult" parameter to setup the detection multiplier (rx_interval
> * mult).
>
>
Thanks Anil and Miguel for the review. I sent out v2 adding mult option -
https://patchwork.ozlabs.org/patch/974893/
Numan
>
> Thanks a lot,
> Miguel Ángel.
>
> On Tue, Sep 25, 2018 at 5:46 PM Anil Venkata
> wrote:
>
>> On Mon, Sep 24, 2018 at 3:12 PM wrote:
>>
>> > From: Numan Siddique
>> >
>> > With this commit the users can override the default BFD params -
>> > min_rx, min_tx and decay_min_rx if desired. This can be useful
>> > to debug any issues related to BFD, like BFD state changes are
>> > seen frequently.
>> >
>> > ovn-controller checks for the options 'ovn-bfd-min-rx', 'ovn-bfd-min-tx'
>> > and 'ovn-bfd-decay-min-rx' in the external-ids of OpenvSwitch table row
>> > and configures these BFD values to the tunnel interfaces.
>> >
>> > Signed-off-by: Numan Siddique
>> >
>> Acked-by: Venkata Anil
>>
>> > ---
>> > ovn/controller/bfd.c| 45 -
>> > ovn/controller/bfd.h| 2 ++
>> > ovn/controller/ovn-controller.8.xml | 9 ++
>> > ovn/controller/ovn-controller.c | 10 ---
>> > tests/ovn.at| 26 +
>> > 5 files changed, 74 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
>> > index 051781f38..455d7ff1f 100644
>> > --- a/ovn/controller/bfd.c
>> > +++ b/ovn/controller/bfd.c
>> > @@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>> >
>> >
>> > static void
>> > -interface_set_bfd(const struct ovsrec_interface *iface, bool
>> bfd_setting)
>> > +interface_set_bfd(const struct ovsrec_interface *iface, struct smap
>> *bfd)
>> > {
>> > -const char *new_setting = bfd_setting ? "true":"false";
>> > -const char *current_setting = smap_get(&iface->bfd, "enable");
>> > -if (current_setting && !strcmp(current_setting, new_setting)) {
>> > -/* If already set to the desired setting we skip setting it
>> again
>> > - * to avoid flapping to bfd initialization state */
>> > -return;
>> > -}
>> > -const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting);
>> > ovsrec_interface_verify_bfd(iface);
>> > -ovsrec_interface_set_bfd(iface, &bfd);
>> > -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
>> > "Disabled",
>> > -iface->name);
>> > +ovsrec_interface_set_bfd(iface, bfd);
>> > }
>> >
>> > void
>> > @@ -265,6 +255,7 @@ void
>> > bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
>> > struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> > const struct ovsrec_interface_table *interface_table,
>> > +const struct ovsrec_open_vswitch_table *ovs_table,
>> > const struct ovsrec_bridge *br_int,
>> > const struct sbrec_chassis *chassis_rec,
>> > const struct hmap *local_datapaths)
>> > @@ -292,15 +283,41 @@ bfd_run(struct ovsdb_idl_index
>> > *sbrec_chassis_by_name,
>> > }
>> > }
>> >
>> > +const struct ovsrec_open_vswitch *cfg;
>> > +cfg = ovsrec_open_vswitch_table_first(ovs_table);
>> > +const char *min_rx = NULL;
>> > +const char *decay_min_rx = NULL;
>> > +const char *min_tx = NULL;
>> > +if (cfg) {
>> > +min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
>> > +decay_min_rx = smap_get(&cfg->external_ids,
>> > "ovn-bfd-decay-min-rx");
>> > +min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
>> > +}
>> > +struct smap bfd = SMAP_INITIALIZER(&bfd);
>> > +if (min_rx) {
>> > +smap_add(&bfd, "min_rx", min_rx);
>> > +}
>> > +if (decay_min_rx) {
>> > +smap_add(&bfd, "decay_min_rx", decay_min_rx);
>> > +}
>> > +if (min_tx) {
>> > +smap_add(&bfd, "min_tx", min_tx);
>> > +}
>> > /* Enable or disable bfd */
>> > const struct ovsrec_interface *iface;
>> > OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
>> > if (sset_contains(&tunnels, iface->name)) {
>> > -interface_set_bfd(
>> > -iface, sset_contains(&bfd_ifaces,
>> iface->name));
>> > +bool bfd_setting = sset_contains(&bfd_ifaces, iface->name);
>> > +smap_replace(&bfd, "enable", bfd_setting ? "true":"false");
>> > +if (!smap_equal(&iface->bfd, &bfd)) {
>> > +interface_set_bfd(iface, &bfd);
>> > +VLOG_INFO("%s BFD on interface %s",
>> > + bfd_setting ? "Enabled" : "Disabled",
>> > iface->name);
>> > +}
>> > }
>> > }
>> >
>> > +smap_destroy(&bfd);
>> > sset_d
[ovs-dev] Fix memleaks in test-ovn.c
Hello, The attached patch plugs memory leaked by calls to ovn_extend_table_init in tests/test-ovn.c that are missing a corresponding ovn_extend_table_destroy. The patch fixes leaks for the group_table and meter_table objects. Regards, Bhargava Bhargava Shastry Security in Telecommunications TU Berlin / Telekom Innovation Laboratories Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany phone: +49 30 8353 58235 Keybase: https://keybase.io/bshastry ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH net-next] net: ovs: fix return type of ndo_start_xmit function
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing
---
net/openvswitch/vport-internal_dev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/openvswitch/vport-internal_dev.c
b/net/openvswitch/vport-internal_dev.c
index bb95c43..26f71cb 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -43,7 +43,8 @@ static struct internal_dev *internal_dev_priv(struct
net_device *netdev)
}
/* Called with rcu_read_lock_bh. */
-static int internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t
+internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
{
int len, err;
@@ -62,7 +63,7 @@ static int internal_dev_xmit(struct sk_buff *skb, struct
net_device *netdev)
} else {
netdev->stats.tx_errors++;
}
- return 0;
+ return NETDEV_TX_OK;
}
static int internal_dev_open(struct net_device *netdev)
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn: Support configuring the BFD params for the tunnel interfaces
Looks great! but you forgot the testing for the mult option. Rather simple
anyway it's obvious it works, but it's my only nit :)
On Wed, Sep 26, 2018 at 10:30 AM Numan Siddique wrote:
>
>
> On Tue, Sep 25, 2018 at 10:18 PM Miguel Angel Ajo Pelayo <
> majop...@redhat.com> wrote:
>
>> Nak,
>>
>> Great work, this will be useful to adjust parameters based on specific
>> network conditions, or while debugging network issues (to reduce flapping).
>>
>> I miss the "mult" parameter to setup the detection multiplier
>> (rx_interval * mult).
>>
>>
>
> Thanks Anil and Miguel for the review. I sent out v2 adding mult option -
> https://patchwork.ozlabs.org/patch/974893/
>
> Numan
>
>
>
>>
>> Thanks a lot,
>> Miguel Ángel.
>>
>> On Tue, Sep 25, 2018 at 5:46 PM Anil Venkata
>> wrote:
>>
>>> On Mon, Sep 24, 2018 at 3:12 PM wrote:
>>>
>>> > From: Numan Siddique
>>> >
>>> > With this commit the users can override the default BFD params -
>>> > min_rx, min_tx and decay_min_rx if desired. This can be useful
>>> > to debug any issues related to BFD, like BFD state changes are
>>> > seen frequently.
>>> >
>>> > ovn-controller checks for the options 'ovn-bfd-min-rx',
>>> 'ovn-bfd-min-tx'
>>> > and 'ovn-bfd-decay-min-rx' in the external-ids of OpenvSwitch table row
>>> > and configures these BFD values to the tunnel interfaces.
>>> >
>>> > Signed-off-by: Numan Siddique
>>> >
>>> Acked-by: Venkata Anil
>>>
>>> > ---
>>> > ovn/controller/bfd.c| 45 -
>>> > ovn/controller/bfd.h| 2 ++
>>> > ovn/controller/ovn-controller.8.xml | 9 ++
>>> > ovn/controller/ovn-controller.c | 10 ---
>>> > tests/ovn.at| 26 +
>>> > 5 files changed, 74 insertions(+), 18 deletions(-)
>>> >
>>> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
>>> > index 051781f38..455d7ff1f 100644
>>> > --- a/ovn/controller/bfd.c
>>> > +++ b/ovn/controller/bfd.c
>>> > @@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>>> >
>>> >
>>> > static void
>>> > -interface_set_bfd(const struct ovsrec_interface *iface, bool
>>> bfd_setting)
>>> > +interface_set_bfd(const struct ovsrec_interface *iface, struct smap
>>> *bfd)
>>> > {
>>> > -const char *new_setting = bfd_setting ? "true":"false";
>>> > -const char *current_setting = smap_get(&iface->bfd, "enable");
>>> > -if (current_setting && !strcmp(current_setting, new_setting)) {
>>> > -/* If already set to the desired setting we skip setting it
>>> again
>>> > - * to avoid flapping to bfd initialization state */
>>> > -return;
>>> > -}
>>> > -const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting);
>>> > ovsrec_interface_verify_bfd(iface);
>>> > -ovsrec_interface_set_bfd(iface, &bfd);
>>> > -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
>>> > "Disabled",
>>> > -iface->name);
>>> > +ovsrec_interface_set_bfd(iface, bfd);
>>> > }
>>> >
>>> > void
>>> > @@ -265,6 +255,7 @@ void
>>> > bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
>>> > struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>>> > const struct ovsrec_interface_table *interface_table,
>>> > +const struct ovsrec_open_vswitch_table *ovs_table,
>>> > const struct ovsrec_bridge *br_int,
>>> > const struct sbrec_chassis *chassis_rec,
>>> > const struct hmap *local_datapaths)
>>> > @@ -292,15 +283,41 @@ bfd_run(struct ovsdb_idl_index
>>> > *sbrec_chassis_by_name,
>>> > }
>>> > }
>>> >
>>> > +const struct ovsrec_open_vswitch *cfg;
>>> > +cfg = ovsrec_open_vswitch_table_first(ovs_table);
>>> > +const char *min_rx = NULL;
>>> > +const char *decay_min_rx = NULL;
>>> > +const char *min_tx = NULL;
>>> > +if (cfg) {
>>> > +min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
>>> > +decay_min_rx = smap_get(&cfg->external_ids,
>>> > "ovn-bfd-decay-min-rx");
>>> > +min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
>>> > +}
>>> > +struct smap bfd = SMAP_INITIALIZER(&bfd);
>>> > +if (min_rx) {
>>> > +smap_add(&bfd, "min_rx", min_rx);
>>> > +}
>>> > +if (decay_min_rx) {
>>> > +smap_add(&bfd, "decay_min_rx", decay_min_rx);
>>> > +}
>>> > +if (min_tx) {
>>> > +smap_add(&bfd, "min_tx", min_tx);
>>> > +}
>>> > /* Enable or disable bfd */
>>> > const struct ovsrec_interface *iface;
>>> > OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
>>> > if (sset_contains(&tunnels, iface->name)) {
>>> > -interface_set_bfd(
>>> > -iface, sset_contains(&bfd_ifaces,
>>> iface->name));
>>> > +bool bfd_setting = sset_contains(&bfd_ifaces,
>>> iface->name);
>>> > +smap_replace(&bfd, "enable", bfd_setting ?
>>> "true":"false");
>>> > +
Re: [ovs-dev] [dpdk-howl PATCH v4] netdev-dpdk: Upgrade to dpdk v18.08
On 09/21/2018 11:12 AM, Ophir Munk wrote:
> 1. Enable compilation and linkage with dpdk 18.08.0
> The following dpdk commits which were introduced after dpdk 17.11.x
> require OVS updates to accommodate to the dpdk changes.
> - ce17edde ("ethdev: introduce Rx queue offloads API")
> - ab3ce1e0 ("ethdev: remove old offload API")
> - c06ddf96 ("meter: add configuration profile")
> - e58638c3 ("ethdev: fix TPID handling in flow API")
> - cd8c7c7c ("ethdev: replace bus specific struct with generic dev")
> - ac8d22de ("ethdev: flatten RSS configuration in flow API")
>
> 2. Limit configured rss hash functions to only those supported
> by the eth device.
>
> 3. Set default RSS key in struct action_rss_data, required by OVS commit
> - e8a2b5bf ("netdev-dpdk: implement flow offload with rte flow")
> when configured with "other_config:hw-offload=true"
>
> 4. Update references to DPDK version 18.08 in Documentation
>
> 5. Update DPDK version 18.08 in travis' linux-build script
>
> Signed-off-by: Ophir Munk
> ---
> v1:
> First version
>
> v2:
> Avoid seg faults cases as described in
> https://patchwork.ozlabs.org/patch/965451/
> by using the patch in:
> https://github.com/kevintraynor/ovs-dpdk-
> master/commit/88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c
>
> v3:
> - rebase on latest dpdk-hwol branch
> - Updates based on latest reviews to versions v1 & v2
>
> v4:
> - update commit message
> - Set default RSS key in struct action_rss_data, required by OVS commit
> e8a2b5bf ("netdev-dpdk: implement flow offload with rte flow")
> when configured with "other_config:hw-offload=true"
>
Hi Ophir, I can't see this v4 in the ML archives or patchwork - I
suspect it got dropped during some ML issues last week. Perhaps you
could resend to the ML.
Kevin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
Hi Pravin, On Wed, 15 Aug 2018 00:19:39 -0700 Pravin Shelar wrote: > I understand fairness has cost, but we need to find right balance > between performance and fairness. Current fairness scheme is a > lockless algorithm without much computational overhead, did you try to > improve current algorithm so that it uses less number of ports. In the end, we tried harder as you suggested, and found out a way to avoid the need for per-thread sets of per-vport sockets: with a few changes, a process-wide set of per-vport sockets is actually enough to maintain the current fairness behaviour. Further, we can get rid of duplicate fd events if the EPOLLEXCLUSIVE epoll() flag is available, which improves performance noticeably. This is explained in more detail in the commit message of the ovs-vswitchd patch Matteo wrote [1], now merged. This approach solves the biggest issue (i.e. huge amount of netlink sockets) in ovs-vswitchd itself, without the need for kernel changes. It doesn't address some proposed improvements though, that is, it does nothing to improve the current fairness scheme, it won't allow neither the configurable fairness criteria proposed by Ben, nor usage of BPF maps for extensibility as suggested by William. I would however say that those topics bear definitely lower priority, and perhaps addressing them at all becomes overkill now that we don't any longer have a serious issue. [1] https://patchwork.ozlabs.org/patch/974304/ -- Stefano ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] ovn: Support configuring the BFD params for the tunnel interfaces
From: Numan Siddique
With this commit the users can override the default values of
the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
This can be useful to debug any issues related to BFD, like BFD
state changes seen frequently.
ovn-controller checks for the options 'ovn-bfd-min-rx',
'ovn-bfd-min-tx', 'ovn-bfd-decay-min-rx' and 'ovn-bfd-mult' in the
external-ids of OpenvSwitch table row and configures these BFD
values to the tunnel interfaces.
Signed-off-by: Numan Siddique
---
v2 -> v3
* Added the test case for mult option
v1 -> v2
---
* Addressed review comments by Miguel - added the option 'mult' to configure.
ovn/controller/bfd.c| 50 +
ovn/controller/bfd.h| 2 ++
ovn/controller/ovn-controller.8.xml | 10 ++
ovn/controller/ovn-controller.c | 10 +++---
tests/ovn.at| 27
5 files changed, 81 insertions(+), 18 deletions(-)
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index 051781f38..e2ca9cf49 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
static void
-interface_set_bfd(const struct ovsrec_interface *iface, bool bfd_setting)
+interface_set_bfd(const struct ovsrec_interface *iface, struct smap *bfd)
{
-const char *new_setting = bfd_setting ? "true":"false";
-const char *current_setting = smap_get(&iface->bfd, "enable");
-if (current_setting && !strcmp(current_setting, new_setting)) {
-/* If already set to the desired setting we skip setting it again
- * to avoid flapping to bfd initialization state */
-return;
-}
-const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting);
ovsrec_interface_verify_bfd(iface);
-ovsrec_interface_set_bfd(iface, &bfd);
-VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" : "Disabled",
-iface->name);
+ovsrec_interface_set_bfd(iface, bfd);
}
void
@@ -265,6 +255,7 @@ void
bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
const struct ovsrec_interface_table *interface_table,
+const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
const struct hmap *local_datapaths)
@@ -292,15 +283,46 @@ bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
}
}
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_table_first(ovs_table);
+const char *min_rx = NULL;
+const char *decay_min_rx = NULL;
+const char *min_tx = NULL;
+const char *mult = NULL;
+if (cfg) {
+min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
+decay_min_rx = smap_get(&cfg->external_ids, "ovn-bfd-decay-min-rx");
+min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
+mult = smap_get(&cfg->external_ids, "ovn-bfd-mult");
+}
+struct smap bfd = SMAP_INITIALIZER(&bfd);
+if (min_rx) {
+smap_add(&bfd, "min_rx", min_rx);
+}
+if (decay_min_rx) {
+smap_add(&bfd, "decay_min_rx", decay_min_rx);
+}
+if (min_tx) {
+smap_add(&bfd, "min_tx", min_tx);
+}
+if (mult) {
+smap_add(&bfd, "mult", mult);
+}
/* Enable or disable bfd */
const struct ovsrec_interface *iface;
OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
if (sset_contains(&tunnels, iface->name)) {
-interface_set_bfd(
-iface, sset_contains(&bfd_ifaces, iface->name));
+bool bfd_setting = sset_contains(&bfd_ifaces, iface->name);
+smap_replace(&bfd, "enable", bfd_setting ? "true":"false");
+if (!smap_equal(&iface->bfd, &bfd)) {
+interface_set_bfd(iface, &bfd);
+VLOG_INFO("%s BFD on interface %s",
+ bfd_setting ? "Enabled" : "Disabled", iface->name);
+}
}
}
+smap_destroy(&bfd);
sset_destroy(&tunnels);
sset_destroy(&bfd_ifaces);
sset_destroy(&bfd_chassis);
diff --git a/ovn/controller/bfd.h b/ovn/controller/bfd.h
index 7ea345a3e..9243bec69 100644
--- a/ovn/controller/bfd.h
+++ b/ovn/controller/bfd.h
@@ -21,6 +21,7 @@ struct ovsdb_idl;
struct ovsdb_idl_index;
struct ovsrec_bridge;
struct ovsrec_interface_table;
+struct ovsrec_open_vswitch_table;
struct sbrec_chassis;
struct sset;
@@ -28,6 +29,7 @@ void bfd_register_ovs_idl(struct ovsdb_idl *);
void bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
const struct ovsrec_interface_table *interface_table,
+ const struct ovsrec_open_vswitch_table *ovs_table,
const struct ovsrec_bridge *br_int,
[ovs-dev] [PATCH] Fixes memory leaked by call to ovn_extend_table_init that is missing a corresponding ovn_extend_table_destroy in test-ovn.c. This fixes leaks for the group_table and meter_table obje
From: Bhargava Shastry Signed-off-by: Bhargava Shastry --- tests/test-ovn.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 5e6d1c3b4..ebaf21673 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1347,6 +1347,8 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) dhcp_opts_destroy(&dhcp_opts); dhcp_opts_destroy(&dhcpv6_opts); nd_ra_opts_destroy(&nd_ra_opts); +ovn_extend_table_destroy(&group_table); +ovn_extend_table_destroy(&meter_table); exit(ok ? EXIT_SUCCESS : EXIT_FAILURE); } -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn: Support configuring the BFD params for the tunnel interfaces
Thank you,
Acked-By: Miguel Angel Ajo Pelayo
On Wed, Sep 26, 2018 at 12:46 PM wrote:
> From: Numan Siddique
>
> With this commit the users can override the default values of
> the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
> This can be useful to debug any issues related to BFD, like BFD
> state changes seen frequently.
>
> ovn-controller checks for the options 'ovn-bfd-min-rx',
> 'ovn-bfd-min-tx', 'ovn-bfd-decay-min-rx' and 'ovn-bfd-mult' in the
> external-ids of OpenvSwitch table row and configures these BFD
> values to the tunnel interfaces.
>
> Signed-off-by: Numan Siddique
> ---
>
> v2 -> v3
>
> * Added the test case for mult option
>
> v1 -> v2
> ---
> * Addressed review comments by Miguel - added the option 'mult' to
> configure.
>
> ovn/controller/bfd.c| 50 +
> ovn/controller/bfd.h| 2 ++
> ovn/controller/ovn-controller.8.xml | 10 ++
> ovn/controller/ovn-controller.c | 10 +++---
> tests/ovn.at| 27
> 5 files changed, 81 insertions(+), 18 deletions(-)
>
> diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
> index 051781f38..e2ca9cf49 100644
> --- a/ovn/controller/bfd.c
> +++ b/ovn/controller/bfd.c
> @@ -40,20 +40,10 @@ bfd_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>
>
> static void
> -interface_set_bfd(const struct ovsrec_interface *iface, bool bfd_setting)
> +interface_set_bfd(const struct ovsrec_interface *iface, struct smap *bfd)
> {
> -const char *new_setting = bfd_setting ? "true":"false";
> -const char *current_setting = smap_get(&iface->bfd, "enable");
> -if (current_setting && !strcmp(current_setting, new_setting)) {
> -/* If already set to the desired setting we skip setting it again
> - * to avoid flapping to bfd initialization state */
> -return;
> -}
> -const struct smap bfd = SMAP_CONST1(&bfd, "enable", new_setting);
> ovsrec_interface_verify_bfd(iface);
> -ovsrec_interface_set_bfd(iface, &bfd);
> -VLOG_INFO("%s BFD on interface %s", bfd_setting ? "Enabled" :
> "Disabled",
> -iface->name);
> +ovsrec_interface_set_bfd(iface, bfd);
> }
>
> void
> @@ -265,6 +255,7 @@ void
> bfd_run(struct ovsdb_idl_index *sbrec_chassis_by_name,
> struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> const struct ovsrec_interface_table *interface_table,
> +const struct ovsrec_open_vswitch_table *ovs_table,
> const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis *chassis_rec,
> const struct hmap *local_datapaths)
> @@ -292,15 +283,46 @@ bfd_run(struct ovsdb_idl_index
> *sbrec_chassis_by_name,
> }
> }
>
> +const struct ovsrec_open_vswitch *cfg;
> +cfg = ovsrec_open_vswitch_table_first(ovs_table);
> +const char *min_rx = NULL;
> +const char *decay_min_rx = NULL;
> +const char *min_tx = NULL;
> +const char *mult = NULL;
> +if (cfg) {
> +min_rx = smap_get(&cfg->external_ids, "ovn-bfd-min-rx");
> +decay_min_rx = smap_get(&cfg->external_ids,
> "ovn-bfd-decay-min-rx");
> +min_tx = smap_get(&cfg->external_ids, "ovn-bfd-min-tx");
> +mult = smap_get(&cfg->external_ids, "ovn-bfd-mult");
> +}
> +struct smap bfd = SMAP_INITIALIZER(&bfd);
> +if (min_rx) {
> +smap_add(&bfd, "min_rx", min_rx);
> +}
> +if (decay_min_rx) {
> +smap_add(&bfd, "decay_min_rx", decay_min_rx);
> +}
> +if (min_tx) {
> +smap_add(&bfd, "min_tx", min_tx);
> +}
> +if (mult) {
> +smap_add(&bfd, "mult", mult);
> +}
> /* Enable or disable bfd */
> const struct ovsrec_interface *iface;
> OVSREC_INTERFACE_TABLE_FOR_EACH (iface, interface_table) {
> if (sset_contains(&tunnels, iface->name)) {
> -interface_set_bfd(
> -iface, sset_contains(&bfd_ifaces, iface->name));
> +bool bfd_setting = sset_contains(&bfd_ifaces, iface->name);
> +smap_replace(&bfd, "enable", bfd_setting ? "true":"false");
> +if (!smap_equal(&iface->bfd, &bfd)) {
> +interface_set_bfd(iface, &bfd);
> +VLOG_INFO("%s BFD on interface %s",
> + bfd_setting ? "Enabled" : "Disabled",
> iface->name);
> +}
> }
> }
>
> +smap_destroy(&bfd);
> sset_destroy(&tunnels);
> sset_destroy(&bfd_ifaces);
> sset_destroy(&bfd_chassis);
> diff --git a/ovn/controller/bfd.h b/ovn/controller/bfd.h
> index 7ea345a3e..9243bec69 100644
> --- a/ovn/controller/bfd.h
> +++ b/ovn/controller/bfd.h
> @@ -21,6 +21,7 @@ struct ovsdb_idl;
> struct ovsdb_idl_index;
> struct ovsrec_bridge;
> struct ovsrec_interface_table;
> +struct ovsrec_open_vswitch_table;
> struct sbrec_chassis;
> struct sset;
>
> @@ -28,6 +29,7 @@ void bfd_register_ovs_idl(struct ov
Re: [ovs-dev] Fixes memory leaked by call to ovn_extend_table_init that is missing a corresponding ovn_extend_table_destroy in test-ovn.c. This fixes leaks for the group_table and meter_table objects.
Bleep bloop. Greetings Bhargava Shastry, 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 Bhargava Shastry needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Bhargava Shastry Lines checked: 27, Warnings: 1, Errors: 1 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
Re: [ovs-dev] [ovs-discuss] RX Mirroing issue with Decap in vxlan case
On Tue, Sep 25, 2018 at 10:22 PM Limaye, Namrata
wrote:
>
> Hi John,
>
> Here's the answer to your questions -
>
Thanks for the info.
> 1. Can you give more information on your setup?
> ===Nam) I have back to back connected hosts with OVS running and 2 VMs on
> each side. VxLan is configured on both sides. Port 101 carries tunneled
> traffic. This is the configuration. The traffic is tunneled between eth0.128
> and eth0.101 and mirrored to non-vxlan port eth0.129 connected to Host 2.
>
> VM1 - -- Host 1 -- (eth0.101) Host 2 (OVS) VM3 (eth0.128),
> VM2 --- VM4 (eth0.129 -
> mirror)
>
> Mirroring config is on Host 2. Tep is configured on Br1. And vxlan0 port +
> eth0.128 and eth0.129 are on br0
>
> Following is the config -
> ovs-vsctl -- --id=@p get port vxlan0 -- --id=@eth0.128 get Port eth0.128 --
> --id=@eth0.129 get Port eth0.129 -- --id=@m create
> Mirror name=mirror1 select-src-port=@eth0.128,@p output-port=@eth0.129 -- set
> Bridge br0 mirrors=@m
>
> VXLAN config for HOST2 -
>
> HOST 2:
> ovs-vsctl add-br br1
> ovs-vsctl add-port br1 eth0.101 -- set interface eth0.101 ofport_request=101
> ovs-vsctl add-port br1 tep0 -- set interface tep0 type=internal
> ip addr add 30.1.1.2/24 dev tep0
> ip link set tep0 up
> ovs-vsctl add-port br0 vxlan0 -- set interface vxlan0 type=vxlan
> options:remote_ip=30.1.1.1 options:key=5000 options:dest_port=4789
> ovs-vsctl add-port br0 eth0.128 -- set interface eth0.101 ofport_request=128
> ovs-vsctl add-port br0 eth0.129 -- set interface eth0.101 ofport_request=129
>
> 2. Above you mention that you are expecting a decap (suggesting we will hit a
> vxlan port on the bridge) but the port mirroring is on a non vxlan netdev
> (eth0.128) - is this correct?
> With this, I'm not sure how we get a TC action on a single port that includes
> both the mirror and the decap (1, 2, 3 above)?
> Nam)
> For RX mirroring the final Encap flow that is offloaded to hardware is below
> which is correct order -
> root@FPA1066GX-DA2:~# ovs-appctl dpctl/dump-flows -m type=offloaded
> ufid:157aa38b-3f13-41b8-8dac-a83ba44729c2,
> skb_priority(0/0),skb_mark(0/0),in_port(eth0.128),packet_type(ns=0/0,id=0/0),eth(src=00:e8:ca:11:bc:02,dst=00:e8:ca:11:bb:02),eth_type(0x0800),ipv4(src=1.1.3.1,dst=1.1.2.1,proto=1,tos=0/0,ttl=0/0),icmp(type=0/0,code=0/0),
> packets:3, bytes:294, used:1.360s,
> actions:eth0.129,set(tunnel(tun_id=0x1388,dst=40.1.1.2,tp_dst=4789,flags(key))),vxlan_sys_4789
> < CORRECT
>
I agree that this looks correct.
> The Decap flow that gets offloaded is -
> ufid:03b34739-c02e-4258-9162-997e4b365aad,
> skb_priority(0/0),tunnel(tun_id=0x1388,src=40.1.1.2,dst=40.1.1.1,ttl=0/0,tp_dst=4789,flags(+key)),skb_mark(0/0),in_port(vxlan_sys_4789),packet_type(ns=0/0,id=0/0),eth(src=00:e8:ca:11:bb:02,dst=00:e8:ca:11:bc:02),eth_type(0x0800),ipv4(src=1.1.2.1,dst=1.1.3.1,proto=1,tos=0/0,ttl=0/0),icmp(type=0/0,code=0/0),
> packets:3, bytes:444, used:1.360s, actions:eth0.129,eth0.128
>
> As you see it does not show the decap action in the above flow, but if you
> look at the TC code in function 'nl_msg_put_flower_acts' it does add the
> decap action netlink attribute to the flow and send it down to TC -
I think this issue here is that a decap is implied in this rule
without being stated.
If we receive data on an ingress port of type vxlan, then it must
match the vxlan params and the first thing that will happen is a
decap.
To put it another way, the RX traffic on a vxlan port can be thought
of as the decapped packets, not the vxlan encapped packets.
With this in mind, I would expect the mirrored packets to be decapped..
I ran a test similar to this, mirroring packets on a vxlan dev:
recirc_id(0),tunnel(tun_id=0x7b,src=10.0.0.2,dst=10.0.0.1,flags(-df-csum+key)),in_port(4),eth(),eth_type(0x0800),ipv4(proto=6,frag=no),
packets:266, bytes:35623, used:451.124s, flags:SFPR., actions:3,2
Where port 4 is of type vxlan, 3 is the mirrored output and 2 is a
standard output port.
I ran this both on TC and without TC (OvS kernel datapath).
In both tests decapped packets are on the mirrored port (implying
decap as a first action).
> if (flower->tunnel.tunnel) {
> act_offset = nl_msg_start_nested(request, act_index++);
> nl_msg_put_act_tunnel_key_release(request);
> nl_msg_end_nested(request, act_offset);
> }
> So, the final flow for RX case that goes down to TC looks like -
> ufid:03b34739-c02e-4258-9162-997e4b365aad,
> skb_priority(0/0),tunnel(tun_id=0x1388,src=40.1.1.2,dst=40.1.1.1,ttl=0/0,tp_dst=4789,flags(+key)),skb_mark(0/0),in_port(vxlan_sys_4789),packet_type(ns=0/0,id=0/0),eth(src=00:e8:ca:11:bb:02,dst=00:e8:ca:11:bc:02),eth_type(0x0800),ipv4(src=1.1.2.1,dst=1.1.3.1,proto=1,tos=0/0,ttl=0/0),icmp(type=0/0,code=0/0),
> packets:3, bytes:444, used:1.360s, actions:(decap_tunnel_key_release),
> eth0.129, eth0.128 < INCORRECT
>
As stated above, I do not think this is inco
[ovs-dev] [PATCH] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
This patch addresses the issue that the conntrack fields associated
with a packet are missing after a packet is resumed by NXT_RESUME.
For example, the last rule in the following OpenFlow pipeline is not
working without this patch.
table=0, arp,in_port=1 action=2
table=0, arp,in_port=2 action=1
table=0, in_port=2 icmp action=output:1
table=0, in_port=1 icmp action=ct(table=1)
table=1, icmp action=controller(pause) resubmit(,2)
table=2, in_port=1 icmp ct_state=+trk+new action=output:2
Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in
"continuations".")
VMware-BZ: #2202764
Sgned-off-by: Yi-Hung Wei
---
ofproto/ofproto-dpif-xlate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d0e7c6b9f55d..f85be6604348 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7472,6 +7472,7 @@ xlate_resume(struct ofproto_dpif *ofproto,
dp_packet_use_const(&packet, pin->base.packet,
pin->base.packet_len);
+pkt_metadata_from_flow(&packet.md, &pin->base.flow_metadata.flow);
flow_extract(&packet, flow);
struct xlate_in xin;
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
Bleep bloop. Greetings Yi-Hung Wei, 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 Yi-Hung Wei needs to sign off. Lines checked: 41, Warnings: 0, Errors: 1 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
Re: [ovs-dev] Fix memleaks in test-ovn.c
Hi, It looks like your attachment is not present. I'm not sure if you forgot to attach it or if one of the hops along the way stripped the patch off. The easiest way to send a patch to this list is to use `git send-email`. This will send the e-mail in-line. You can find more information about this command by running `git help send-email` on your command line. On 09/26/2018 04:55 AM, Bhargava Shastry wrote: Hello, The attached patch plugs memory leaked by calls to ovn_extend_table_init in tests/test-ovn.c that are missing a corresponding ovn_extend_table_destroy. The patch fixes leaks for the group_table and meter_table objects. Regards, Bhargava Bhargava Shastry Security in Telecommunications TU Berlin / Telekom Innovation Laboratories Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany phone: +49 30 8353 58235 Keybase: https://keybase.io/bshastry ___ 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 0/2] dpif-netdev: Docs and stats fixes.
Ilya Maximets (2): dpif-netdev-unixctl: Change 'masked' to 'megaflow'. dpif-netdev-perf: Print SMC statistics. lib/dpif-netdev-perf.c | 3 +++ lib/dpif-netdev-perf.h | 2 +- lib/dpif-netdev-unixctl.man | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/2] dpif-netdev-perf: Print SMC statistics.
Printing of the SMC hits missed in the 'dpif-netdev/pmd-perf-show'
appctl command.
CC: Yipeng Wang
Fixes: 60d8ccae135f ("dpif-netdev: Add SMC cache after EMC cache")
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev-perf.c | 3 +++
lib/dpif-netdev-perf.h | 2 +-
lib/dpif-netdev-unixctl.man | 3 ++-
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c
index 13f1010c9..92ac38dab 100644
--- a/lib/dpif-netdev-perf.c
+++ b/lib/dpif-netdev-perf.c
@@ -206,6 +206,7 @@ pmd_perf_format_overall_stats(struct ds *str, struct
pmd_perf_stats *s,
" Rx packets: %12"PRIu64" (%.0f Kpps, %.0f cycles/pkt)\n"
" Datapath passes: %12"PRIu64" (%.2f passes/pkt)\n"
" - EMC hits: %12"PRIu64" (%4.1f %%)\n"
+" - SMC hits: %12"PRIu64" (%4.1f %%)\n"
" - Megaflow hits: %12"PRIu64" (%4.1f %%, %.2f subtbl lookups/"
"hit)\n"
" - Upcalls: %12"PRIu64" (%4.1f %%, %.1f us/upcall)\n"
@@ -215,6 +216,8 @@ pmd_perf_format_overall_stats(struct ds *str, struct
pmd_perf_stats *s,
passes, rx_packets ? 1.0 * passes / rx_packets : 0,
stats[PMD_STAT_EXACT_HIT],
100.0 * stats[PMD_STAT_EXACT_HIT] / passes,
+stats[PMD_STAT_SMC_HIT],
+100.0 * stats[PMD_STAT_SMC_HIT] / passes,
stats[PMD_STAT_MASKED_HIT],
100.0 * stats[PMD_STAT_MASKED_HIT] / passes,
stats[PMD_STAT_MASKED_HIT]
diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h
index 299d52a98..859c05613 100644
--- a/lib/dpif-netdev-perf.h
+++ b/lib/dpif-netdev-perf.h
@@ -56,7 +56,7 @@ extern "C" {
enum pmd_stat_type {
PMD_STAT_EXACT_HIT, /* Packets that had an exact match (emc). */
-PMD_STAT_SMC_HIT,/* Packets that had a sig match hit (SMC). */
+PMD_STAT_SMC_HIT, /* Packets that had a sig match hit (SMC). */
PMD_STAT_MASKED_HIT,/* Packets that matched in the flow table. */
PMD_STAT_MISS, /* Packets that did not match and upcall was ok. */
PMD_STAT_LOST, /* Packets that did not match and upcall failed. */
diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
index c46f6b19c..417456db3 100644
--- a/lib/dpif-netdev-unixctl.man
+++ b/lib/dpif-netdev-unixctl.man
@@ -11,7 +11,7 @@ Shows performance statistics for one or all pmd threads of
the datapath
\fIdp\fR. The special thread "main" sums up the statistics of every non pmd
thread.
-The sum of "emc hits", "megaflow hits" and "miss" is the number of
+The sum of "emc hits", "smc hists", "megaflow hits" and "miss" is the number of
packet lookups performed by the datapath. Beware that a recirculated packet
experiences one additional lookup per recirculation, so there may be
more lookups than forwarded packets in the datapath.
@@ -135,6 +135,7 @@ pmd thread numa_id 0 core_id 1:
Rx packets: 2399607 (2381 Kpps, 848 cycles/pkt)
Datapath passes: 3599415 (1.50 passes/pkt)
- EMC hits:336472 ( 9.3 %)
+ - SMC hits: 0 ( 0.0 %)
- Megaflow hits: 3262943 (90.7 %, 1.00 subtbl lookups/hit)
- Upcalls: 0 ( 0.0 %, 0.0 us/upcall)
- Lost upcalls: 0 ( 0.0 %)
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/2] dpif-netdev-unixctl: Change 'masked' to 'megaflow'.
In the review process of the original patch 'masked hits' stat
was renamed to 'megaflow hits', but the man page wasn't updated.
Fixes: 6553d06bd179 ("dpif-netdev: Add dpif-netdev/pmd-stats-*
appctl commands.")
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev-unixctl.man | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
index ab8619e41..c46f6b19c 100644
--- a/lib/dpif-netdev-unixctl.man
+++ b/lib/dpif-netdev-unixctl.man
@@ -11,7 +11,7 @@ Shows performance statistics for one or all pmd threads of
the datapath
\fIdp\fR. The special thread "main" sums up the statistics of every non pmd
thread.
-The sum of "emc hits", "masked hits" and "miss" is the number of
+The sum of "emc hits", "megaflow hits" and "miss" is the number of
packet lookups performed by the datapath. Beware that a recirculated packet
experiences one additional lookup per recirculation, so there may be
more lookups than forwarded packets in the datapath.
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 2/2] rhel: fix wrong condition check for ovs-kmod-manage.sh, fedora
Hi Ben, Flavio acked this patch as well. I received the email with no problem. Martin On Tue, Sep 25, 2018 at 3:25 PM Ben Pfaff wrote: > On Thu, Sep 20, 2018 at 12:19:30PM -0700, Martin Xu wrote: > > In post-install in kmod fedora spec file, the variables storing > > different parts of kernel version numbers are renamed. The condition > > check to run ovs-kmod-manage.sh for RHEL 7.2 and 7.4 uses the older > > variables. > > > > Fixes c3570519ecaf (rhel: add 4.4 kernel in kmod build with mulitple > > versions, fedora) > > > > Signed-off-by: Martin Xu > > CC: Greg Rose > > CC: Flavio Leitner > > The list dropped some emails on Friday and I think it might have dropped > Flavio's review. Flavio, did you ack both patches? > > Thanks, > > Ben. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next] net: ovs: fix return type of ndo_start_xmit function
From: YueHaibing Date: Wed, 26 Sep 2018 17:15:38 +0800 > The method ndo_start_xmit() is defined as returning an 'netdev_tx_t', > which is a typedef for an enum type, so make sure the implementation in > this driver has returns 'netdev_tx_t' value, and change the function > return type to netdev_tx_t. > > Found by coccinelle. > > Signed-off-by: YueHaibing Applied. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-data: Drop redundant initialization from ovsdb_datum_apply_diff().
Looks good to me, thanks.
Reviewed-by: Yifeng Sun
On Mon, Sep 24, 2018 at 9:33 PM Ben Pfaff wrote:
> The call to ovsdb_datum_diff() initializes 'new', so it's not necessary to
> also do it in ovsdb_datum_apply_diff().
>
> Found by inspection.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovsdb-data.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
> index 1e523f29cb87..08b94cafa737 100644
> --- a/lib/ovsdb-data.c
> +++ b/lib/ovsdb-data.c
> @@ -2121,7 +2121,6 @@ ovsdb_datum_apply_diff(struct ovsdb_datum *new,
> const struct ovsdb_datum *diff,
> const struct ovsdb_type *type)
> {
> -ovsdb_datum_init_empty(new);
> ovsdb_datum_diff(new, old, diff, type);
>
> /* Make sure member size of 'new' conforms to type. */
> --
> 2.16.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] acinclude.m4: Really check whether GCC support -Wno-null-pointer-arithmetic.
Ben Pfaff writes:
> I've noticed recently an annoying quantity of error messages like the
> following in builds in various places:
>
> gcc: error: unrecognized command line option ‘-Wunknown-warning-option’
>
> This didn't really make sense because OVS checks whether the compiler
> supports warning options before it uses them. Looking closer, the GCC
> manual has a note that explains the issue:
>
> When an unrecognized warning option is requested (e.g.,
> '-Wunknown-warning'), GCC emits a diagnostic stating that the
> option is not recognized. However, if the '-Wno-' form is used,
> the behavior is slightly different: no diagnostic is produced for
> '-Wno-unknown-warning' unless other diagnostics are being
> produced. This allows the use of new '-Wno-' options with old
> compilers, but if something goes wrong, the compiler warns that
> an unrecognized option is present.
>
> Thus, we can properly check only for the *positive* version of a warning
> option, so this commit makes the OVS tests do that.
>
> Fixes: a7021b08b0d5 ("configure: Disable -Wnull-pointer-arithmetic Clang
> warning.")
> Signed-off-by: Ben Pfaff
> ---
I didn't get a chance to test this out, but the change looks sane.
Acked-by: Aaron Conole
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 00/20] ovn-controller incremental processing
On Fri, Sep 7, 2018 at 4:59 PM Han Zhou wrote:
>
>
>
> On Fri, Sep 7, 2018 at 2:51 PM Ben Pfaff wrote:
> >
> > On Fri, Sep 07, 2018 at 02:50:34PM -0700, Ben Pfaff wrote:
> > > On Wed, Sep 05, 2018 at 05:11:01PM -0700, Han Zhou wrote:
> > > > On Wed, Sep 5, 2018 at 2:34 PM Ben Pfaff wrote:
> > > > >
> > > > > On Mon, Aug 13, 2018 at 10:47:59AM -0700, Han Zhou wrote:
> > > > > > ovn-controller currently recomputes everything when there are
any
> > > > changes
> > > > > > of input, which leads to high CPU usages and slow in end-to-end
flow
> > > > > > enforcement in response to changes. It even wastes CPU to
recompute
> > > > flows
> > > > > > for unrelated inputs such as pinctrl events.
> > > > >
> > > > > Would you mind publishing this as a branch I can pull from?
> > > >
> > > > Hi Ben,
> > > >
> > > > The branch for the series is here:
https://github.com/hzhou8/ovs/tree/ip12
> > >
> > > Thanks. I am looking at it.
> > >
> > > I built the tip of the branch and saw only two warnings, from sparse:
> > >
> > > ../ovn/controller/ovn-controller.c:616:12: error: symbol
'sb_engine_node_names' was not declared. Should it be static?
> > > ../ovn/controller/ovn-controller.c:641:12: error: symbol
'ovs_engine_node_names' was not declared. Should it be static?
> > >
> > > Usually this warning means that the function or variable should be
> > > marked static.
> >
> > But in this case the variables can be deleted entirely:
> >
> > diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
> > index 99e901663ae7..6f22ba66eabd 100644
> > --- a/ovn/controller/ovn-controller.c
> > +++ b/ovn/controller/ovn-controller.c
> > @@ -613,12 +613,6 @@ enum sb_engine_node {
> > #undef SB_NODE
> > };
> >
> > -const char *sb_engine_node_names[] = {
> > -#define SB_NODE(NAME, NAME_STR) "SB_"NAME_STR,
> > -SB_NODES
> > -#undef SB_NODE
> > -};
> > -
> > #define SB_NODE_NAME(NAME) sb_engine_node_names[SB_##NAME]
> >
> > #define SB_NODE(NAME, NAME_STR) ENGINE_FUNC_SB(NAME);
> > @@ -638,12 +632,6 @@ enum ovs_engine_node {
> > #undef OVS_NODE
> > };
> >
> > -const char *ovs_engine_node_names[] = {
> > -#define OVS_NODE(NAME, NAME_STR) "OVS_"NAME_STR,
> > -OVS_NODES
> > -#undef OVS_NODE
> > -};
> > -
> > #define OVS_NODE_NAME(NAME) ovs_engine_node_names[OVS_##NAME]
> >
> > #define OVS_NODE(NAME, NAME_STR) ENGINE_FUNC_OVS(NAME);
>
> Hi Ben,
>
> Yes, I was using it in the beginning but refactored later and forgot to
remove them. I will remove them in next version.
> I should try sparse for warnings.
>
> Thanks,
> Han
Hi Ben,
I updated the branch https://github.com/hzhou8/ovs/tree/ip12. Apart from
removing the unused macros, there are 2 more fixes. The fixes are rebased
into the ip12 branch. I kept the fixes on top of the original ip12 branch
you reviewed in a new branch
https://github.com/hzhou8/ovs/tree/ip12_fixes_for_v5, so that you can see
the difference easily. Please take a look when you get time. I will rebase
on master and submit v6 after getting your feedback for the whole series.
Thanks,
Han
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Bug: select group with dp_hash causing recursive recirculation
Hi Zang,
Thanks for reporting this bug. As I see it, the check on dp_hash != 0 in
ofproto-dpif-xlate.c is there to guarantee that a dp_hash value has been
computed for the packet once before, not necessarily that a new one is computed
for each translated select group. That's why a check for a valid dp_hash is OK.
Bat all datapaths must adhere to the invariant that valid dp_hash !=0. Indeed
the kernel datapath implements this:
datapath/linux/actions.c:
1071 static void execute_hash(struct sk_buff *skb, struct sw_flow_key *key,
1072 const struct nlattr *attr)
1073 {
1074 struct ovs_action_hash *hash_act = nla_data(attr);
1075 u32 hash = 0;
1076
1077 /* OVS_HASH_ALG_L4 is the only possible hash algorithm. */
1078 hash = skb_get_hash(skb);
1079 hash = jhash_1word(hash, hash_act->hash_basis);
1080 if (!hash)
1081 hash = 0x1;
1082
1083 key->ovs_flow_hash = hash;
1084 }
The correct fix in my view would be to implement the same for the netdev
datapath in lib/odp-execute.c.
This requirement on the dp_hash action implementations should better be
documented properly.
BR, Jan
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org On
> Behalf Of Zang MingJie
> Sent: Tuesday, 25 September, 2018 10:45
> To: ovs dev
> Subject: [ovs-dev] Bug: select group with dp_hash causing recursive
> recirculation
>
> Hi, we found a serious problem where one pmd is stop working, I want to
> share the problem and find solution here.
>
> vswitchd log:
>
> 2018-09-13T23:36:44.377Z|40269235|dpif_netdev(pmd45)|WARN|Packet dropped.
> Max recirculation depth exceeded.
> 2018-09-13T23:36:44.387Z|40269236|dpif_netdev(pmd45)|WARN|Packet dropped.
> Max recirculation depth exceeded.
> 2018-09-13T23:36:44.391Z|40269237|dpif_netdev(pmd45)|WARN|Packet dropped.
> Max recirculation depth exceeded.
>
> problematic datapath flows:
>
>
> ct_state(+new-est),recirc_id(0x143c893),in_port(2),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(proto=6,frag=no),tcp(dst=443),
> packets:84573093, bytes:6308807903, used:0.009s,
> flags:SFPRU.ECN[200][400][800],
> actions:meter(306),hash(hash_l4(0)),recirc(0x237b09d)
>
>
> recirc_id(0x237b09d),in_port(2),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
> packets:279713339, bytes:20890205186, used:0.007s,
> flags:SFPRU.ECN[200][400][800], actions:hash(hash_l4(0)),recirc(0x237b09d)
>
> corresponding openflow:
>
> cookie=0x5b5ab65e000f0101, duration=4848269.642s, table=40,
> n_packets=974343805, n_bytes=72484367083,
> priority=10,tcp,metadata=0xf0100/0xff00,tp_dst=443
> actions=group:983297
>
>
> group_id=983297,type=select,selection_method=dp_hash,bucket=bucket_id:3057033848,weight:100,actions=ct(commit,table=70,zo
> ne=15,exec(nat(dst=10.177.251.203:443))),...``lots
> of buckets``...
>
>
>
> Following explains how select group with dp_hash works.
>
> To implement select group with dp_hash, two datapath flows are needed:
>
> 1. calculate dp_hash, recirculate to second one
> 2. select group bucket by dp_hash
>
> When encounter a datapath miss, openflow doesn't know which one is missing,
> so it depends on dp_hash value of the packet:
>
> if dp_hash == 0 generate first dp flow.
> if dp_hash != 0 generate second dp flow.
>
>
> Back to the problem.
>
> Notice that second datapath flow is a dead loop, it recirculate to itself.
> The cause of the problem is here ofproto/ofproto-dpif-xlate.c#L4429[1]:
>
> /* dp_hash value 0 is special since it means that the dp_hash has not
> been
> * computed, as all computed dp_hash values are non-zero. Therefore
> * compare to zero can be used to decide if the dp_hash value is valid
> * without masking the dp_hash field. */
> if (!dp_hash) {
>
> The comment saying that `dp_hash` shouldn't be zero, but under DPDK, it can
> be zero, at lib/odp-execute.c#L747[2]
>
>/* RSS hash can be used here instead of 5tuple for
> * performance reasons. */
>if (dp_packet_rss_valid(packet)) {
>hash = dp_packet_get_rss_hash(packet);
>hash = hash_int(hash, hash_act->hash_basis);
>} else {
>flow_extract(packet, &flow);
>hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
>}
>packet->md.dp_hash = hash;
>
> I don't know how small chance that `hash_int` returns 0, we have tested
> that if the final hash is 0, will definitely trigger the same bug. And due
> to the chance is extremely low, I'm also investigation that if there are
> other situation that will pass 0 hash to ofp.
>
>
>
> IMO, it is silly to depends on dp_hash value, maybe we need a new mechanism
> which can pass data between ofp and odp freely. And a quick solution could
> be just change the 0 hash to 1.
>
>
> [1]
> https://github.com/openvswitch/ovs/blob/master/ofproto/ofproto-dpif-xlate.c#L4429
> [2] https://github.com/openvswitch/ovs/blob
Re: [ovs-dev] [PATCH] acinclude.m4: Really check whether GCC support -Wno-null-pointer-arithmetic.
On Wed, Sep 26, 2018 at 01:26:46PM -0400, Aaron Conole wrote:
> Ben Pfaff writes:
>
> > I've noticed recently an annoying quantity of error messages like the
> > following in builds in various places:
> >
> > gcc: error: unrecognized command line option ‘-Wunknown-warning-option’
> >
> > This didn't really make sense because OVS checks whether the compiler
> > supports warning options before it uses them. Looking closer, the GCC
> > manual has a note that explains the issue:
> >
> > When an unrecognized warning option is requested (e.g.,
> > '-Wunknown-warning'), GCC emits a diagnostic stating that the
> > option is not recognized. However, if the '-Wno-' form is used,
> > the behavior is slightly different: no diagnostic is produced for
> > '-Wno-unknown-warning' unless other diagnostics are being
> > produced. This allows the use of new '-Wno-' options with old
> > compilers, but if something goes wrong, the compiler warns that
> > an unrecognized option is present.
> >
> > Thus, we can properly check only for the *positive* version of a warning
> > option, so this commit makes the OVS tests do that.
> >
> > Fixes: a7021b08b0d5 ("configure: Disable -Wnull-pointer-arithmetic Clang
> > warning.")
> > Signed-off-by: Ben Pfaff
> > ---
>
> I didn't get a chance to test this out, but the change looks sane.
>
> Acked-by: Aaron Conole
Thanks. I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-data: Drop redundant initialization from ovsdb_datum_apply_diff().
Thanks, applied to master.
On Wed, Sep 26, 2018 at 10:25:23AM -0700, Yifeng Sun wrote:
> Looks good to me, thanks.
>
> Reviewed-by: Yifeng Sun
>
> On Mon, Sep 24, 2018 at 9:33 PM Ben Pfaff wrote:
>
> > The call to ovsdb_datum_diff() initializes 'new', so it's not necessary to
> > also do it in ovsdb_datum_apply_diff().
> >
> > Found by inspection.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/ovsdb-data.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
> > index 1e523f29cb87..08b94cafa737 100644
> > --- a/lib/ovsdb-data.c
> > +++ b/lib/ovsdb-data.c
> > @@ -2121,7 +2121,6 @@ ovsdb_datum_apply_diff(struct ovsdb_datum *new,
> > const struct ovsdb_datum *diff,
> > const struct ovsdb_type *type)
> > {
> > -ovsdb_datum_init_empty(new);
> > ovsdb_datum_diff(new, old, diff, type);
> >
> > /* Make sure member size of 'new' conforms to type. */
> > --
> > 2.16.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 2/2] rhel: fix wrong condition check for ovs-kmod-manage.sh, fedora
Thanks, applied to master also. On Wed, Sep 26, 2018 at 10:19:44AM -0700, Martin Xu wrote: > Hi Ben, > > Flavio acked this patch as well. I received the email with no problem. > > Martin > > On Tue, Sep 25, 2018 at 3:25 PM Ben Pfaff wrote: > > > On Thu, Sep 20, 2018 at 12:19:30PM -0700, Martin Xu wrote: > > > In post-install in kmod fedora spec file, the variables storing > > > different parts of kernel version numbers are renamed. The condition > > > check to run ovs-kmod-manage.sh for RHEL 7.2 and 7.4 uses the older > > > variables. > > > > > > Fixes c3570519ecaf (rhel: add 4.4 kernel in kmod build with mulitple > > > versions, fedora) > > > > > > Signed-off-by: Martin Xu > > > CC: Greg Rose > > > CC: Flavio Leitner > > > > The list dropped some emails on Friday and I think it might have dropped > > Flavio's review. Flavio, did you ack both patches? > > > > Thanks, > > > > Ben. > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 09:33:27AM -0700, Yi-Hung Wei wrote:
> This patch addresses the issue that the conntrack fields associated
> with a packet are missing after a packet is resumed by NXT_RESUME.
> For example, the last rule in the following OpenFlow pipeline is not
> working without this patch.
>
> table=0, arp,in_port=1 action=2
> table=0, arp,in_port=2 action=1
> table=0, in_port=2 icmp action=output:1
> table=0, in_port=1 icmp action=ct(table=1)
> table=1, icmp action=controller(pause) resubmit(,2)
> table=2, in_port=1 icmp ct_state=+trk+new action=output:2
>
> Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in
> "continuations".")
> VMware-BZ: #2202764
> Sgned-off-by: Yi-Hung Wei
Thanks for the patch. Will you please add a test?
"Signed-off-by" is misspelled.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] datapath-windows: Move OVS_IPHELPER_INSTANCE to IpHelper.h
Move the IPHelper Instance to the main header file and update the usage to
explicitly point to POVS_IPHELPER_INSTANCE instead of PVOID. Also rename
the ipn->context to ipn->instance to make it more readable.
Found by inspection.
Signed-off-by: Sairam Venugopal
---
datapath-windows/ovsext/IpHelper.c | 32 ++-
datapath-windows/ovsext/IpHelper.h | 45 ++
2 files changed, 38 insertions(+), 39 deletions(-)
diff --git a/datapath-windows/ovsext/IpHelper.c
b/datapath-windows/ovsext/IpHelper.c
index c734b0e..876da92 100644
--- a/datapath-windows/ovsext/IpHelper.c
+++ b/datapath-windows/ovsext/IpHelper.c
@@ -40,34 +40,6 @@ static LIST_ENTRY ovsInstanceList;
static ERESOURCE ovsInstanceListLock;
/*
- * This structure is used to define each adapter instance.
- *
- * Note:
- * Only when the internal IP is configured and virtual
- * internal port is connected, the IP helper request can be
- * queued.
- *
- * We only keep internal IP for reference, it will not be used for determining
- * SRC IP of the Tunnel.
- *
- * The lock must not raise the IRQL higher than PASSIVE_LEVEL in order for the
- * route manipulation functions, i.e. GetBestRoute, to work.
- */
-typedef struct _OVS_IPHELPER_INSTANCE
-{
-LIST_ENTRY link;
-
-BOOLEAN isIpConfigured;
-UINT32 portNo;
-GUIDnetCfgId;
-MIB_IF_ROW2 internalRow;
-MIB_IPINTERFACE_ROW internalIPRow;
-UINT32 ipAddress;
-
-ERESOURCE lock;
-} OVS_IPHELPER_INSTANCE, *POVS_IPHELPER_INSTANCE;
-
-/*
* FWD_ENTRY > IPFORWARD_ENTRY
* |
* |--> IPENIGH_ENTRY
@@ -1070,7 +1042,7 @@ OvsCreateIPNeighEntry(PMIB_IPNET_ROW2 ipNeigh,
RtlCopyMemory(entry->macAddr, ipNeigh->PhysicalAddress,
ETH_ADDR_LEN);
InitializeListHead(&entry->fwdList);
-entry->context = (PVOID)instance;
+entry->instance = instance;
return entry;
}
@@ -1934,7 +1906,7 @@ OvsStartIpHelper(PVOID data)
ipAddr = ipn->ipAddr;
MIB_IPNET_ROW2 ipNeigh;
NTSTATUS status;
-POVS_IPHELPER_INSTANCE instance =
(POVS_IPHELPER_INSTANCE)ipn->context;
+POVS_IPHELPER_INSTANCE instance = ipn->instance;
NdisReleaseSpinLock(&ovsIpHelperLock);
ExAcquireResourceExclusiveLite(&ovsInstanceListLock, TRUE);
diff --git a/datapath-windows/ovsext/IpHelper.h
b/datapath-windows/ovsext/IpHelper.h
index 0556965..25adf6e 100644
--- a/datapath-windows/ovsext/IpHelper.h
+++ b/datapath-windows/ovsext/IpHelper.h
@@ -32,17 +32,44 @@
#define OVS_IPNEIGH_TIMEOUT 1 // 10 s
+ /*
+ * This structure is used to define each adapter instance.
+ *
+ * Note:
+ * Only when the internal IP is configured and virtual
+ * internal port is connected, the IP helper request can be
+ * queued.
+ *
+ * We only keep internal IP for reference, it will not be used for determining
+ * SRC IP of the Tunnel.
+ *
+ * The lock must not raise the IRQL higher than PASSIVE_LEVEL in order for the
+ * route manipulation functions, i.e. GetBestRoute, to work.
+ */
+typedef struct _OVS_IPHELPER_INSTANCE
+{
+LIST_ENTRY link;
+
+BOOLEAN isIpConfigured;
+UINT32 portNo;
+GUIDnetCfgId;
+MIB_IF_ROW2 internalRow;
+MIB_IPINTERFACE_ROW internalIPRow;
+UINT32 ipAddress;
+
+ERESOURCE lock;
+} OVS_IPHELPER_INSTANCE, *POVS_IPHELPER_INSTANCE;
typedef struct _OVS_IPNEIGH_ENTRY {
-UINT8 macAddr[ETH_ADDR_LEN];
-UINT16refCount;
-UINT32ipAddr;
-UINT32pad;
-UINT64timeout;
-LIST_ENTRYlink;
-LIST_ENTRYslink;
-LIST_ENTRYfwdList;
-PVOID context;
+UINT8 macAddr[ETH_ADDR_LEN];
+UINT16 refCount;
+UINT32 ipAddr;
+UINT32 pad;
+UINT64 timeout;
+LIST_ENTRY link;
+LIST_ENTRY slink;
+LIST_ENTRY fwdList;
+POVS_IPHELPER_INSTANCE instance;
} OVS_IPNEIGH_ENTRY, *POVS_IPNEIGH_ENTRY;
typedef struct _OVS_IPFORWARD_ENTRY {
--
2.9.0.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] Fixes memory leaked by call to ovn_extend_table_init that is missing a corresponding ovn_extend_table_destroy in test-ovn.c. This fixes leaks for the group_table and meter_table
On Wed, Sep 26, 2018 at 03:11:40PM +0200, bshas...@sect.tu-berlin.de wrote: > From: Bhargava Shastry > > Signed-off-by: Bhargava Shastry Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ovn: Support configuring the BFD params for the tunnel interfaces
On Wed, Sep 26, 2018 at 04:14:18PM +0530, nusid...@redhat.com wrote: > From: Numan Siddique > > With this commit the users can override the default values of > the BFD params - min_rx, min_tx, decay_min_rx and mult if desired. > This can be useful to debug any issues related to BFD, like BFD > state changes seen frequently. > > ovn-controller checks for the options 'ovn-bfd-min-rx', > 'ovn-bfd-min-tx', 'ovn-bfd-decay-min-rx' and 'ovn-bfd-mult' in the > external-ids of OpenvSwitch table row and configures these BFD > values to the tunnel interfaces. > > Signed-off-by: Numan Siddique Thanks for working to make OVN better. If this is primarily a debugging interface, would it be better to use ovs-appctl commands to adjust these values? If this has uses outside of debugging, would it be better to make the settings centrally configurable instead of via a local-only interface? Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVSDB IDL - tracked columns gives out of sequenced updates sometimes
There have been fixed to column tracking since then. Try a newer version. On Thu, Sep 20, 2018 at 05:10:46PM +0530, Arunkumar Rg wrote: > Forgot to mention that I'm using OVS 2.7.2. > > Thanks, > Arun. > > On Thu, Sep 20, 2018 at 5:08 PM Arunkumar Rg wrote: > > > Hi, > > > > I have a ovsdb-client using ovsdb-idl to get updates from ovsdb-server. In > > my client, I track(ovsdb_idl_track_add_column) column of a specific table > > in the DB. > > > > The issue I'm facing is - in the client, the sequence of updates that I > > get is different from the sequence of updates that actually happen on the > > DB. BTW, this does not happen always. > > > > For example: > > ovsdb-tool -m show-log /opt/ovsdb/db/vtep.db > > : > > record 1392: 2018-09-19 06:07:57.212 > > : > > table Ucast_Macs_Remote row efb03eab (efb03eab): > > delete row > > : > > record 1393: 2018-09-19 06:07:57.296 > > : > > table Ucast_Macs_Remote insert row f39fe808: > > : > > > > But in the client, I get the above updates in wrong sequence i.e. f39fe808 > > first, then I get the update for efb03eab. > > BTW, I use VTEPREC_UCAST_MACS_REMOTE_FOR_EACH_TRACKED macro to walk > > through the changes that has happened on that table. > > > > *So I'm wondering whether this out of sequenced updates given by ovsdb-idl > > is expected or a bug. * > > Please let me know your inputs. > > > > Thanks, > > Arun. > > > ___ > 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] [dpdk-howl PATCH v4] netdev-dpdk: Upgrade to dpdk v18.08
Hi Kevin,
I have reviews to be addressed in v5 which may change v4.
Therefore I suggest sending v5 (hopefully) as the final version for this PATCH.
Please let me know if you still want me to resend v4 to the ML.
Regards,
Ophir
> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Wednesday, September 26, 2018 12:47 PM
> To: Ophir Munk ; ovs-dev@openvswitch.org
> Cc: Asaf Penso ; Sugesh Chandran
> ; Ian Stokes ; Ben
> Pfaff ; Shahaf Shuler ; Thomas
> Monjalon ; Olga Shern
> Subject: Re: [dpdk-howl PATCH v4] netdev-dpdk: Upgrade to dpdk v18.08
>
> On 09/21/2018 11:12 AM, Ophir Munk wrote:
> > 1. Enable compilation and linkage with dpdk 18.08.0 The following dpdk
> > commits which were introduced after dpdk 17.11.x require OVS updates
> > to accommodate to the dpdk changes.
> > - ce17edde ("ethdev: introduce Rx queue offloads API")
> > - ab3ce1e0 ("ethdev: remove old offload API")
> > - c06ddf96 ("meter: add configuration profile")
> > - e58638c3 ("ethdev: fix TPID handling in flow API")
> > - cd8c7c7c ("ethdev: replace bus specific struct with generic dev")
> > - ac8d22de ("ethdev: flatten RSS configuration in flow API")
> >
> > 2. Limit configured rss hash functions to only those supported by the
> > eth device.
> >
> > 3. Set default RSS key in struct action_rss_data, required by OVS
> > commit
> > - e8a2b5bf ("netdev-dpdk: implement flow offload with rte flow") when
> > configured with "other_config:hw-offload=true"
> >
> > 4. Update references to DPDK version 18.08 in Documentation
> >
> > 5. Update DPDK version 18.08 in travis' linux-build script
> >
> > Signed-off-by: Ophir Munk
> > ---
> > v1:
> > First version
> >
> > v2:
> > Avoid seg faults cases as described in
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >
> chwork.ozlabs.org%2Fpatch%2F965451%2F&data=02%7C01%7Cophirm
> u%40mel
> >
> lanox.com%7C92464c8120cf4c14ebaa08d62394f775%7Ca652971c7d2e4d9b
> a6a4d14
> >
> 9256f461b%7C0%7C0%7C636735520041931061&sdata=vhHc3nMU3Gg
> MBDOgqtZWL
> > TEk%2FBnswcZM%2BZhVt6hMK00%3D&reserved=0
> > by using the patch in:
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > hub.com%2Fkevintraynor%2Fovs-dpdk-
> &data=02%7C01%7Cophirmu%40mellan
> >
> ox.com%7C92464c8120cf4c14ebaa08d62394f775%7Ca652971c7d2e4d9ba6
> a4d14925
> >
> 6f461b%7C0%7C0%7C636735520041931061&sdata=KR8HIJBkKqXwUsC
> BeOwWMUi%
> > 2BPgqOKJnu%2FCqZ1Zo3JuM%3D&reserved=0
> > master/commit/88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c
> >
> > v3:
> > - rebase on latest dpdk-hwol branch
> > - Updates based on latest reviews to versions v1 & v2
> >
> > v4:
> > - update commit message
> > - Set default RSS key in struct action_rss_data, required by OVS
> > commit e8a2b5bf ("netdev-dpdk: implement flow offload with rte flow")
> > when configured with "other_config:hw-offload=true"
> >
>
> Hi Ophir, I can't see this v4 in the ML archives or patchwork - I suspect it
> got
> dropped during some ML issues last week. Perhaps you could resend to the
> ML.
>
> Kevin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] OVN: add buffering support for ipv4 packets
On Fri, Sep 14, 2018 at 05:19:24PM +0200, Lorenzo Bianconi wrote:
> Add buffering support for IPv4 packets that will be processed
> by arp {} action when L2 address is not discovered yet since
> otherwise the packet will be substituted with an ARP frame and
> this will result in the lost of the first packet of the connection
>
> Signed-off-by: Lorenzo Bianconi
Thanks for working on this! I believe that users will find it useful.
A buffer queue depth of 64 sounds big to me. Linux appears to default
to a depth of 3 (see unres_qlen in arp(7)).
A 30-second timeout seems long to me. After that long, the sender will
have retransmitted the packet, probably multiple times. I wasn't able
to figure out how long Linux buffers packets.
Do you think it makes sense to use the same timestamp for all of the
packets buffered for a single IP? I believe that, currently, no packets
will ever expire from a buffer unless either the buffer limit is
exceeded or no new packets have been queued for the timeout interval. I
think that would mean that there could be a packet buffered for a total
of 30 + 64 seconds.
It seems odd to me to convert the IP address to a string then use that
as the key. Why not the binary form of the IP address? I see that the
following patch adds IPv6 support, which could be one reason, but in
many places in OVS we use IPv6-mapped IPv4 addresses in places where
IPv4 and IPv6 are both possible (see in6_addr_mapped_ipv4() and
in6_addr_get_mapped_ipv4(), for example).
In the buffered_send_packets() prototype, I would use struct eth_addr
instead of an unsigned char *.
Thanks,
Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] OVN: add buffering support for ipv4 packets
On Wed, Sep 26, 2018 at 03:07:52PM -0700, Ben Pfaff wrote:
> On Fri, Sep 14, 2018 at 05:19:24PM +0200, Lorenzo Bianconi wrote:
> > Add buffering support for IPv4 packets that will be processed
> > by arp {} action when L2 address is not discovered yet since
> > otherwise the packet will be substituted with an ARP frame and
> > this will result in the lost of the first packet of the connection
> >
> > Signed-off-by: Lorenzo Bianconi
>
> Thanks for working on this! I believe that users will find it useful.
>
> A buffer queue depth of 64 sounds big to me. Linux appears to default
> to a depth of 3 (see unres_qlen in arp(7)).
>
> A 30-second timeout seems long to me. After that long, the sender will
> have retransmitted the packet, probably multiple times. I wasn't able
> to figure out how long Linux buffers packets.
>
> Do you think it makes sense to use the same timestamp for all of the
> packets buffered for a single IP? I believe that, currently, no packets
> will ever expire from a buffer unless either the buffer limit is
> exceeded or no new packets have been queued for the timeout interval. I
> think that would mean that there could be a packet buffered for a total
> of 30 + 64 seconds.
>
> It seems odd to me to convert the IP address to a string then use that
> as the key. Why not the binary form of the IP address? I see that the
> following patch adds IPv6 support, which could be one reason, but in
> many places in OVS we use IPv6-mapped IPv4 addresses in places where
> IPv4 and IPv6 are both possible (see in6_addr_mapped_ipv4() and
> in6_addr_get_mapped_ipv4(), for example).
>
> In the buffered_send_packets() prototype, I would use struct eth_addr
> instead of an unsigned char *.
One more thing: I recommend making "head" and "tail" unsigned int so
that it is obvious that taking % BUFFER_QUEUE_DEPTH is efficient.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
This patch addresses the issue that the conntrack fields associated
with a packet are missing after a packet is resumed by NXT_RESUME.
For example, the last rule in the following OpenFlow pipeline is not
working without this patch.
table=0, arp,in_port=1 action=2
table=0, arp,in_port=2 action=1
table=0, in_port=2 icmp action=output:1
table=0, in_port=1 icmp action=ct(table=1)
table=1, icmp action=controller(pause) resubmit(,2)
table=2, in_port=1 icmp ct_state=+trk+new action=output:2
A unit test is added to prevent regression.
Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in
"continuations".")
VMware-BZ: #2202764
Signed-off-by: Yi-Hung Wei
---
v1->v2: A unit test is added.
---
ofproto/ofproto-dpif-xlate.c | 1 +
tests/system-traffic.at | 27 +++
2 files changed, 28 insertions(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d0e7c6b9f55d..f85be6604348 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7472,6 +7472,7 @@ xlate_resume(struct ofproto_dpif *ofproto,
dp_packet_use_const(&packet, pin->base.packet,
pin->base.packet_len);
+pkt_metadata_from_flow(&packet.md, &pin->base.flow_metadata.flow);
flow_extract(&packet, flow);
struct xlate_in xin;
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 277227bdd4cf..04f5529f8fc8 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1320,6 +1320,33 @@
udp,vlan_tci=0x,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - controller pause and resume])
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows.txt], [dnl
+table=0, arp,in_port=1 action=2
+table=0, arp,in_port=2 action=1
+table=0, in_port=2 icmp action=output:1
+table=0, in_port=1 icmp action=ct(table=1)
+table=1, icmp action=controller(pause) resubmit(,2)
+table=2, in_port=1 icmp ct_state=+trk+new action=output:2
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+ovs-ofctl monitor br0 resume --detach --no-chdir
+
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING],
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([conntrack - force commit])
CHECK_CONNTRACK()
OVS_TRAFFIC_VSWITCHD_START()
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 1:33 PM Ben Pfaff wrote:
>
> On Wed, Sep 26, 2018 at 09:33:27AM -0700, Yi-Hung Wei wrote:
> > This patch addresses the issue that the conntrack fields associated
> > with a packet are missing after a packet is resumed by NXT_RESUME.
> > For example, the last rule in the following OpenFlow pipeline is not
> > working without this patch.
> > Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal
> > in "continuations".")
> > VMware-BZ: #2202764
> > Sgned-off-by: Yi-Hung Wei
>
> Thanks for the patch. Will you please add a test?
>
> "Signed-off-by" is misspelled.
Thanks Ben for review. I send out a v2 as in here:
https://patchwork.ozlabs.org/patch/975459/
Thanks,
-Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Contribuye a la investigación científica
--- Este correo electrónico ha sido comprobado en busca de virus por AVG. http://www.avg.com ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 03:08:03PM -0700, Yi-Hung Wei wrote:
> This patch addresses the issue that the conntrack fields associated
> with a packet are missing after a packet is resumed by NXT_RESUME.
> For example, the last rule in the following OpenFlow pipeline is not
> working without this patch.
>
> table=0, arp,in_port=1 action=2
> table=0, arp,in_port=2 action=1
> table=0, in_port=2 icmp action=output:1
> table=0, in_port=1 icmp action=ct(table=1)
> table=1, icmp action=controller(pause) resubmit(,2)
> table=2, in_port=1 icmp ct_state=+trk+new action=output:2
>
> A unit test is added to prevent regression.
>
> Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in
> "continuations".")
> VMware-BZ: #2202764
> Signed-off-by: Yi-Hung Wei
> ---
> v1->v2: A unit test is added.
Thanks for adding a test. Is there a reason why a test can't be added
in the "normal" testsuite rather than system-tests? The regular
testsuite runs much more often.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Remove support for multiple queues per port.
It looks good to me, and testing shows no problem. Thanks.
Tested-by: Yifeng Sun
Reviewed-by: Yifeng Sun
On Tue, Sep 25, 2018 at 3:14 PM Ben Pfaff wrote:
> Commit 69c51582ff78 ("dpif-netlink: don't allocate per thread netlink
> sockets") removed dpif-netlink support for multiple queues per port.
> No remaining dpif provider supports multiple queues per port, so
> remove infrastructure for the feature.
>
> CC: Matteo Croce
> Signed-off-by: Ben Pfaff
> ---
> lib/dpif-netlink.c| 9 -
> lib/dpif-provider.h | 14 ++
> lib/dpif.c| 15 +++
> lib/dpif.h| 15 +--
> ofproto/ofproto-dpif-upcall.c | 7 +++
> ofproto/ofproto-dpif-xlate.c | 6 ++
> 6 files changed, 15 insertions(+), 51 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 4736d21d4abc..21315033cc66 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -234,7 +234,7 @@ static bool ovs_tunnels_out_of_tree = true;
> static int dpif_netlink_init(void);
> static int open_dpif(const struct dpif_netlink_dp *, struct dpif **);
> static uint32_t dpif_netlink_port_get_pid(const struct dpif *,
> - odp_port_t port_no, uint32_t
> hash);
> + odp_port_t port_no);
> static void dpif_netlink_handler_uninit(struct dpif_handler *handler);
> static int dpif_netlink_refresh_channels(struct dpif_netlink *,
> uint32_t n_handlers);
> @@ -991,7 +991,7 @@ dpif_netlink_port_query_by_name(const struct dpif
> *dpif_, const char *devname,
>
> static uint32_t
> dpif_netlink_port_get_pid__(const struct dpif_netlink *dpif,
> -odp_port_t port_no, uint32_t hash OVS_UNUSED)
> +odp_port_t port_no)
> OVS_REQ_RDLOCK(dpif->upcall_lock)
> {
> uint32_t port_idx = odp_to_u32(port_no);
> @@ -1015,14 +1015,13 @@ dpif_netlink_port_get_pid__(const struct
> dpif_netlink *dpif,
> }
>
> static uint32_t
> -dpif_netlink_port_get_pid(const struct dpif *dpif_, odp_port_t port_no,
> - uint32_t hash)
> +dpif_netlink_port_get_pid(const struct dpif *dpif_, odp_port_t port_no)
> {
> const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> uint32_t ret;
>
> fat_rwlock_rdlock(&dpif->upcall_lock);
> -ret = dpif_netlink_port_get_pid__(dpif, port_no, hash);
> +ret = dpif_netlink_port_get_pid__(dpif, port_no);
> fat_rwlock_unlock(&dpif->upcall_lock);
>
> return ret;
> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index debdafc42992..eb3ee50a6320 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -191,16 +191,7 @@ struct dpif_class {
>
> /* Returns the Netlink PID value to supply in
> OVS_ACTION_ATTR_USERSPACE
> * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in
> - * flows whose packets arrived on port 'port_no'. In the case where
> the
> - * provider allocates multiple Netlink PIDs to a single port, it may
> use
> - * 'hash' to spread load among them. The caller need not use a
> particular
> - * hash function; a 5-tuple hash is suitable.
> - *
> - * (The datapath implementation might use some different hash
> function for
> - * distributing packets received via flow misses among PIDs. This
> means
> - * that packets received via flow misses might be reordered relative
> to
> - * packets received via userspace actions. This is not ordinarily a
> - * problem.)
> + * flows whose packets arrived on port 'port_no'.
> *
> * A 'port_no' of UINT32_MAX should be treated as a special case. The
> * implementation should return a reserved PID, not allocated to any
> port,
> @@ -212,8 +203,7 @@ struct dpif_class {
> *
> * A dpif provider that doesn't have meaningful Netlink PIDs can use
> NULL
> * for this function. This is equivalent to always returning 0. */
> -uint32_t (*port_get_pid)(const struct dpif *dpif, odp_port_t port_no,
> - uint32_t hash);
> +uint32_t (*port_get_pid)(const struct dpif *dpif, odp_port_t port_no);
>
> /* Attempts to begin dumping the ports in a dpif. On success,
> returns 0
> * and initializes '*statep' with any data needed for iteration. On
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 85cf9000e80b..4697a4dcd519 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -737,16 +737,7 @@ dpif_port_query_by_name(const struct dpif *dpif,
> const char *devname,
>
> /* Returns the Netlink PID value to supply in OVS_ACTION_ATTR_USERSPACE
> * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in
> - * flows whose packets arrived on port 'port_no'. In the case where the
> - * provider allocates multiple Netlink PIDs to a single port, it may use
> - * 'hash' to spread load among them. The c
[ovs-dev] [PATCH v3] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
This patch addresses the issue that the conntrack fields associated
with a packet are missing after a packet is resumed by NXT_RESUME.
For example, the last rule in the following OpenFlow pipeline is not
working without this patch.
table=0, arp,in_port=1 action=2
table=0, arp,in_port=2 action=1
table=0, in_port=2 icmp action=output:1
table=0, in_port=1 icmp action=ct(table=1)
table=1, icmp action=controller(pause) resubmit(,2)
table=2, in_port=1 icmp ct_state=+trk+new action=output:2
A unit test is added to prevent regression.
Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in
"continuations".")
VMware-BZ: #2202764
Signed-off-by: Yi-Hung Wei
---
v1->v2: A unit test is added.
v2->v3: Move the unit test from system traffic test to regular unit test.
---
ofproto/ofproto-dpif-xlate.c | 1 +
tests/ofproto-dpif.at| 35 +++
2 files changed, 36 insertions(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d0e7c6b9f55d..f85be6604348 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7472,6 +7472,7 @@ xlate_resume(struct ofproto_dpif *ofproto,
dp_packet_use_const(&packet, pin->base.packet,
pin->base.packet_len);
+pkt_metadata_from_flow(&packet.md, &pin->base.flow_metadata.flow);
flow_extract(&packet, flow);
struct xlate_in xin;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 693a64cab685..a14087e6a4da 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5427,6 +5427,41 @@ AT_CHECK([test 1 = `$PYTHON
"$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap | wc
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - continuation with conntrack])
+AT_KEYWORDS([continuations pause resume])
+OVS_VSWITCHD_START
+
+add_of_ports --pcap br0 `seq 1 2`
+
+flow="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)"
+
+AT_DATA([flows.txt], [dnl
+table=0, in_port=1 icmp action=ct(table=1)
+table=1, icmp action=controller(pause) resubmit(,2)
+table=2, in_port=1 icmp ct_state=+trk+new action=output:2
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows.txt])
+
+AT_CAPTURE_FILE([ofctl_monitor0.log])
+ovs-ofctl monitor br0 resume --detach --no-chdir \
+--pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log
+
+# Run a packet through the switch.
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$flow"], [0], [stdout])
+
+# Check flow stats
+AT_CHECK([ovs-ofctl dump-flows br0], [0], [stdout])
+AT_CHECK([strip_xids < stdout | sed -n
's/duration=[[0-9]]*\.[[0-9]]*s/duration=0.0s/p' | sed -n
's/idle_age=[[0-9]]*/idle_age=0/p' | grep 'table=2'], [0], [dnl
+ cookie=0x0, duration=0.0s, table=2, n_packets=1, n_bytes=106, idle_age=0,
ct_state=+new+trk,icmp,in_port=1 actions=output:2
+])
+
+# The packet should be recieved by port 2
+AT_CHECK([test 1 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap |
wc -l`])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
# Check that pause works after the packet is cloned.
AT_SETUP([ofproto-dpif - continuation after clone])
AT_KEYWORDS([continuations clone pause resume])
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 3:27 PM Ben Pfaff wrote:
>
> On Wed, Sep 26, 2018 at 03:08:03PM -0700, Yi-Hung Wei wrote:
> > This patch addresses the issue that the conntrack fields associated
> > with a packet are missing after a packet is resumed by NXT_RESUME.
> > For example, the last rule in the following OpenFlow pipeline is not
> > working without this patch.
> >
> > table=0, arp,in_port=1 action=2
> > table=0, arp,in_port=2 action=1
> > table=0, in_port=2 icmp action=output:1
> > table=0, in_port=1 icmp action=ct(table=1)
> > table=1, icmp action=controller(pause) resubmit(,2)
> > table=2, in_port=1 icmp ct_state=+trk+new action=output:2
> >
> > A unit test is added to prevent regression.
> >
> > Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal
> > in "continuations".")
> > VMware-BZ: #2202764
> > Signed-off-by: Yi-Hung Wei
> > ---
> > v1->v2: A unit test is added.
>
> Thanks for adding a test. Is there a reason why a test can't be added
> in the "normal" testsuite rather than system-tests? The regular
> testsuite runs much more often.
Right, it makes more sense to be in the regular testsuite.
I move the test out from system traffic test to the regular testsuite in v3.
https://patchwork.ozlabs.org/patch/975486/
Thanks,
-Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
Bleep bloop. Greetings Yi-Hung Wei, 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. build: libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-netflow.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-netflow.Tpo -c ofproto/netflow.c -o ofproto/ofproto_libofproto_la-netflow.o mv -f ofproto/.deps/ofproto_libofproto_la-netflow.Tpo ofproto/.deps/ofproto_libofproto_la-netflow.Plo /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto.lo `test -f 'ofproto/ofproto.c' || echo './'`ofproto/ofproto.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c ofproto/ofproto.c -o ofproto/ofproto_libofproto_la-ofproto.o ofproto/ofproto.c: In function 'ofproto_group_mod_finish': ofproto/ofproto.c:7400:13: error: missing initializer for field 'new_buckets' of 'struct ' [-Werror=missing-field-initializers] .new_buckets = new_group ? &new_group->buckets : NULL, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:135:36: note: 'new_buckets' declared here const struct ovs_list *new_buckets; ^ ofproto/ofproto.c:7401:13: error: missing initializer for field 'group_existed' of 'struct ' [-Werror=missing-field-initializers] .group_existed = group_collection_n(&ogm->old_groups) > 0, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:140:17: note: 'group_existed' declared here int group_existed; ^ cc1: all warnings being treated as errors make[2]: *** [ofproto/ofproto_libofproto_la-ofproto.lo] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 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
Re: [ovs-dev] [PATCH v1] Load balancing not happening in case of select group action for MPLS tagged packets
On Mon, Sep 24, 2018 at 10:59:34PM +0530, Anju Thomas wrote: > OVS does not do load balancing for select group buckets in case of mpls > tagged packets. > After an MPLS pop action, the expectation is to trigger recirculation . > This recirculation will ensure an RSS re-computation which will ensure load > balancing in case of select group bucket. > Due to a missing return statement before bucket selection, the bucket > selection in case of select group happens > before the recirculation and hence no load balancing is achieved > > Signed-off-by: Anju Thomas Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Remove support for multiple queues per port.
Thanks, applied to master.
On Wed, Sep 26, 2018 at 03:29:16PM -0700, Yifeng Sun wrote:
> It looks good to me, and testing shows no problem. Thanks.
>
> Tested-by: Yifeng Sun
> Reviewed-by: Yifeng Sun
>
> On Tue, Sep 25, 2018 at 3:14 PM Ben Pfaff wrote:
>
> > Commit 69c51582ff78 ("dpif-netlink: don't allocate per thread netlink
> > sockets") removed dpif-netlink support for multiple queues per port.
> > No remaining dpif provider supports multiple queues per port, so
> > remove infrastructure for the feature.
> >
> > CC: Matteo Croce
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/dpif-netlink.c| 9 -
> > lib/dpif-provider.h | 14 ++
> > lib/dpif.c| 15 +++
> > lib/dpif.h| 15 +--
> > ofproto/ofproto-dpif-upcall.c | 7 +++
> > ofproto/ofproto-dpif-xlate.c | 6 ++
> > 6 files changed, 15 insertions(+), 51 deletions(-)
> >
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index 4736d21d4abc..21315033cc66 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -234,7 +234,7 @@ static bool ovs_tunnels_out_of_tree = true;
> > static int dpif_netlink_init(void);
> > static int open_dpif(const struct dpif_netlink_dp *, struct dpif **);
> > static uint32_t dpif_netlink_port_get_pid(const struct dpif *,
> > - odp_port_t port_no, uint32_t
> > hash);
> > + odp_port_t port_no);
> > static void dpif_netlink_handler_uninit(struct dpif_handler *handler);
> > static int dpif_netlink_refresh_channels(struct dpif_netlink *,
> > uint32_t n_handlers);
> > @@ -991,7 +991,7 @@ dpif_netlink_port_query_by_name(const struct dpif
> > *dpif_, const char *devname,
> >
> > static uint32_t
> > dpif_netlink_port_get_pid__(const struct dpif_netlink *dpif,
> > -odp_port_t port_no, uint32_t hash OVS_UNUSED)
> > +odp_port_t port_no)
> > OVS_REQ_RDLOCK(dpif->upcall_lock)
> > {
> > uint32_t port_idx = odp_to_u32(port_no);
> > @@ -1015,14 +1015,13 @@ dpif_netlink_port_get_pid__(const struct
> > dpif_netlink *dpif,
> > }
> >
> > static uint32_t
> > -dpif_netlink_port_get_pid(const struct dpif *dpif_, odp_port_t port_no,
> > - uint32_t hash)
> > +dpif_netlink_port_get_pid(const struct dpif *dpif_, odp_port_t port_no)
> > {
> > const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> > uint32_t ret;
> >
> > fat_rwlock_rdlock(&dpif->upcall_lock);
> > -ret = dpif_netlink_port_get_pid__(dpif, port_no, hash);
> > +ret = dpif_netlink_port_get_pid__(dpif, port_no);
> > fat_rwlock_unlock(&dpif->upcall_lock);
> >
> > return ret;
> > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> > index debdafc42992..eb3ee50a6320 100644
> > --- a/lib/dpif-provider.h
> > +++ b/lib/dpif-provider.h
> > @@ -191,16 +191,7 @@ struct dpif_class {
> >
> > /* Returns the Netlink PID value to supply in
> > OVS_ACTION_ATTR_USERSPACE
> > * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in
> > - * flows whose packets arrived on port 'port_no'. In the case where
> > the
> > - * provider allocates multiple Netlink PIDs to a single port, it may
> > use
> > - * 'hash' to spread load among them. The caller need not use a
> > particular
> > - * hash function; a 5-tuple hash is suitable.
> > - *
> > - * (The datapath implementation might use some different hash
> > function for
> > - * distributing packets received via flow misses among PIDs. This
> > means
> > - * that packets received via flow misses might be reordered relative
> > to
> > - * packets received via userspace actions. This is not ordinarily a
> > - * problem.)
> > + * flows whose packets arrived on port 'port_no'.
> > *
> > * A 'port_no' of UINT32_MAX should be treated as a special case. The
> > * implementation should return a reserved PID, not allocated to any
> > port,
> > @@ -212,8 +203,7 @@ struct dpif_class {
> > *
> > * A dpif provider that doesn't have meaningful Netlink PIDs can use
> > NULL
> > * for this function. This is equivalent to always returning 0. */
> > -uint32_t (*port_get_pid)(const struct dpif *dpif, odp_port_t port_no,
> > - uint32_t hash);
> > +uint32_t (*port_get_pid)(const struct dpif *dpif, odp_port_t port_no);
> >
> > /* Attempts to begin dumping the ports in a dpif. On success,
> > returns 0
> > * and initializes '*statep' with any data needed for iteration. On
> > diff --git a/lib/dpif.c b/lib/dpif.c
> > index 85cf9000e80b..4697a4dcd519 100644
> > --- a/lib/dpif.c
> > +++ b/lib/dpif.c
> > @@ -737,16 +737,7 @@ dpif_port_query_by_name(const struct dpif *dpif,
> > const char *devname,
> >
> > /* Returns the Netlink PID
Re: [ovs-dev] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
Bleep bloop. Greetings Yi-Hung Wei, 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. build: libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-netflow.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-netflow.Tpo -c ofproto/netflow.c -o ofproto/ofproto_libofproto_la-netflow.o mv -f ofproto/.deps/ofproto_libofproto_la-netflow.Tpo ofproto/.deps/ofproto_libofproto_la-netflow.Plo /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto.lo `test -f 'ofproto/ofproto.c' || echo './'`ofproto/ofproto.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c ofproto/ofproto.c -o ofproto/ofproto_libofproto_la-ofproto.o ofproto/ofproto.c: In function 'ofproto_group_mod_finish': ofproto/ofproto.c:7400:13: error: missing initializer for field 'new_buckets' of 'struct ' [-Werror=missing-field-initializers] .new_buckets = new_group ? &new_group->buckets : NULL, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:135:36: note: 'new_buckets' declared here const struct ovs_list *new_buckets; ^ ofproto/ofproto.c:7401:13: error: missing initializer for field 'group_existed' of 'struct ' [-Werror=missing-field-initializers] .group_existed = group_collection_n(&ogm->old_groups) > 0, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:140:17: note: 'group_existed' declared here int group_existed; ^ cc1: all warnings being treated as errors make[2]: *** [ofproto/ofproto_libofproto_la-ofproto.lo] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 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
Re: [ovs-dev] [PATCH v3] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 03:47:15PM -0700, Yi-Hung Wei wrote:
> This patch addresses the issue that the conntrack fields associated
> with a packet are missing after a packet is resumed by NXT_RESUME.
> For example, the last rule in the following OpenFlow pipeline is not
> working without this patch.
>
> table=0, arp,in_port=1 action=2
> table=0, arp,in_port=2 action=1
> table=0, in_port=2 icmp action=output:1
> table=0, in_port=1 icmp action=ct(table=1)
> table=1, icmp action=controller(pause) resubmit(,2)
> table=2, in_port=1 icmp ct_state=+trk+new action=output:2
>
> A unit test is added to prevent regression.
>
> Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal in
> "continuations".")
> VMware-BZ: #2202764
> Signed-off-by: Yi-Hung Wei
> ---
> v1->v2: A unit test is added.
> v2->v3: Move the unit test from system traffic test to regular unit test.
Thanks a lot, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif: Remove support for multiple queues per port.
On Wed, Sep 26, 2018 at 10:29 PM Yifeng Sun wrote: > > It looks good to me, and testing shows no problem. Thanks. > > Tested-by: Yifeng Sun > Reviewed-by: Yifeng Sun > Works fine here too. Regards, -- Matteo Croce per aspera ad upstream ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto: Fix build with some GCC versions.
GCC 4.8.x and possibly other versions don't like a designated initializer
for an anonymous struct, see e.g.
https://travis-ci.org/openvswitch/ovs/jobs/433747674
Fixes: f836888d28ec ("ofproto: Handle OpenFlow version mismatch for
requestforward with groups.")
Signed-off-by: Ben Pfaff
---
ofproto/ofproto.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a8cc4751f8c9..0f8d74747851 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -7393,13 +7393,12 @@ ofproto_group_mod_finish(struct ofproto *ofproto,
remove_groups_postponed(&ogm->old_groups);
if (req) {
-struct ofputil_requestforward rf = {
-.xid = req->request->xid,
-.reason = OFPRFR_GROUP_MOD,
-.group_mod = &ogm->gm,
-.new_buckets = new_group ? &new_group->buckets : NULL,
-.group_existed = group_collection_n(&ogm->old_groups) > 0,
-};
+struct ofputil_requestforward rf;
+rf.xid = req->request->xid;
+rf.reason = OFPRFR_GROUP_MOD;
+rf.group_mod = &ogm->gm;
+rf.new_buckets = new_group ? &new_group->buckets : NULL;
+rf.group_existed = group_collection_n(&ogm->old_groups) > 0;
connmgr_send_requestforward(ofproto->connmgr, req->ofconn, &rf);
}
}
--
2.16.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] treewide: Fix spelling of "receive".
Signed-off-by: Ben Pfaff
---
datapath-windows/ovsext/IpFragment.c | 4 ++--
datapath-windows/ovsext/IpFragment.h | 2 +-
ovn/controller/pinctrl.c | 2 +-
tests/ofproto-dpif.at| 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/datapath-windows/ovsext/IpFragment.c
b/datapath-windows/ovsext/IpFragment.c
index d59d7cf96c8d..bb2cfe02105c 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -230,7 +230,7 @@ cleanup:
/*
*
* OvsProcessIpv4Fragment
- * Reassemble the fragments once all the fragments are recieved and
+ * Reassemble the fragments once all the fragments are received and
* return NDIS_STATUS_PENDING for the pending fragments
* XXX - Instead of copying NBls, Keep the NBLs in limbo state.
*
@@ -403,7 +403,7 @@ found:
entry->tail = fragStorage;
}
-/*Update Maximum recieved Unit */
+/*Update Maximum Receive Unit */
entry->mru = entry->mru > (ETH_HEADER_LENGTH + ipHdrLen + payloadLen) ?
entry->mru : (ETH_HEADER_LENGTH + ipHdrLen + payloadLen);
entry->numFragments++;
diff --git a/datapath-windows/ovsext/IpFragment.h
b/datapath-windows/ovsext/IpFragment.h
index cd5b96033748..2b2305132fb3 100644
--- a/datapath-windows/ovsext/IpFragment.h
+++ b/datapath-windows/ovsext/IpFragment.h
@@ -57,7 +57,7 @@ typedef struct _OVS_IPFRAG_THREAD_CTX {
#define IP_FRAG_HASH_TABLE_SIZE ((UINT32)1 << 10)
#define IP_FRAG_HASH_TABLE_MASK (IP_FRAG_HASH_TABLE_SIZE - 1)
-/*30s -Sufficient time to recieve all fragments.*/
+/*30s -Sufficient time to receive all fragments.*/
#define IPFRAG_ENTRY_TIMEOUT 3LL
#define IPFRAG_CLEANUP_INTERVAL IPFRAG_ENTRY_TIMEOUT * 2 /*1m.*/
PNET_BUFFER_LIST OvsIpv4FragmentNBL(PVOID ovsContext,
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 0164696cc920..8ae4c9e522ad 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -432,7 +432,7 @@ pinctrl_handle_put_dhcp_opts(
if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN +
sizeof (struct dhcp_header) + sizeof(uint32_t) + 3)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-VLOG_WARN_RL(&rl, "Invalid or incomplete DHCP packet recieved");
+VLOG_WARN_RL(&rl, "Invalid or incomplete DHCP packet received");
goto exit;
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 5adde249a62b..851d3dc159b9 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5421,7 +5421,7 @@ AT_CHECK([strip_xids < stdout | sed -n
's/duration=[[0-9]]*\.[[0-9]]*s/duration=
cookie=0x0, duration=0.0s, table=2, n_packets=1, n_bytes=106, idle_age=0,
reg1=0x1 actions=output:2
])
-# The packet should be recieved by port 2
+# The packet should be received by port 2
AT_CHECK([test 1 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap |
wc -l`])
OVS_VSWITCHD_STOP
@@ -5492,7 +5492,7 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$flow"],
[0], [stdout])
ovs-vsctl show
ovs-ofctl dump-flows br0
-# The packet should be recieved by port 2 and not port 3
+# The packet should be received by port 2 and not port 3
AT_CHECK([test 1 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p2-tx.pcap |
wc -l`])
AT_CHECK([test 0 = `$PYTHON "$top_srcdir/utilities/ovs-pcap.in" p3-tx.pcap |
wc -l`])
--
2.16.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 03:53:17PM -0700, Yi-Hung Wei wrote:
> On Wed, Sep 26, 2018 at 3:27 PM Ben Pfaff wrote:
> >
> > On Wed, Sep 26, 2018 at 03:08:03PM -0700, Yi-Hung Wei wrote:
> > > This patch addresses the issue that the conntrack fields associated
> > > with a packet are missing after a packet is resumed by NXT_RESUME.
> > > For example, the last rule in the following OpenFlow pipeline is not
> > > working without this patch.
> > >
> > > table=0, arp,in_port=1 action=2
> > > table=0, arp,in_port=2 action=1
> > > table=0, in_port=2 icmp action=output:1
> > > table=0, in_port=1 icmp action=ct(table=1)
> > > table=1, icmp action=controller(pause) resubmit(,2)
> > > table=2, in_port=1 icmp ct_state=+trk+new action=output:2
> > >
> > > A unit test is added to prevent regression.
> > >
> > > Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal
> > > in "continuations".")
> > > VMware-BZ: #2202764
> > > Signed-off-by: Yi-Hung Wei
> > > ---
> > > v1->v2: A unit test is added.
> >
> > Thanks for adding a test. Is there a reason why a test can't be added
> > in the "normal" testsuite rather than system-tests? The regular
> > testsuite runs much more often.
>
> Right, it makes more sense to be in the regular testsuite.
>
> I move the test out from system traffic test to the regular testsuite in v3.
>
> https://patchwork.ozlabs.org/patch/975486/
Thank you.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 4:07 PM Ben Pfaff wrote:
>
> On Wed, Sep 26, 2018 at 03:47:15PM -0700, Yi-Hung Wei wrote:
> > This patch addresses the issue that the conntrack fields associated
> > with a packet are missing after a packet is resumed by NXT_RESUME.
> > For example, the last rule in the following OpenFlow pipeline is not
> > working without this patch.
> >
> > table=0, arp,in_port=1 action=2
> > table=0, arp,in_port=2 action=1
> > table=0, in_port=2 icmp action=output:1
> > table=0, in_port=1 icmp action=ct(table=1)
> > table=1, icmp action=controller(pause) resubmit(,2)
> > table=2, in_port=1 icmp ct_state=+trk+new action=output:2
> >
> > A unit test is added to prevent regression.
> >
> > Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal
> > in "continuations".")
> > VMware-BZ: #2202764
> > Signed-off-by: Yi-Hung Wei
> > ---
> > v1->v2: A unit test is added.
> > v2->v3: Move the unit test from system traffic test to regular unit test.
>
> Thanks a lot, applied to master.
Thanks for the review. The fixed patch was in 2.6, I am not sure if
this patch can be cleanly applied to older branch. Can we try to
backport it at least to 2.9 ?
Thanks,
-Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] sflow: Set agent address properly based on collector address.
On Wed, Sep 19, 2018 at 01:32:57PM -0700, Justin Pettit wrote: > If an agent address is not provided, OVS tries to choose a source > address based on the source IP that would be used to connect to the > sFlow collector. The code previously set the agent address to the > collector's address instead of using the calculated source address. > This patch properly uses the source address. > > Reported-by: Neil McKee > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ofproto-dpif-xlate: Fix conntrack fields on NXT_RESUME
On Wed, Sep 26, 2018 at 04:15:17PM -0700, Yi-Hung Wei wrote:
> On Wed, Sep 26, 2018 at 4:07 PM Ben Pfaff wrote:
> >
> > On Wed, Sep 26, 2018 at 03:47:15PM -0700, Yi-Hung Wei wrote:
> > > This patch addresses the issue that the conntrack fields associated
> > > with a packet are missing after a packet is resumed by NXT_RESUME.
> > > For example, the last rule in the following OpenFlow pipeline is not
> > > working without this patch.
> > >
> > > table=0, arp,in_port=1 action=2
> > > table=0, arp,in_port=2 action=1
> > > table=0, in_port=2 icmp action=output:1
> > > table=0, in_port=1 icmp action=ct(table=1)
> > > table=1, icmp action=controller(pause) resubmit(,2)
> > > table=2, in_port=1 icmp ct_state=+trk+new action=output:2
> > >
> > > A unit test is added to prevent regression.
> > >
> > > Fixes: 77ab5fd2a95b ("Implement serializing the state of packet traversal
> > > in "continuations".")
> > > VMware-BZ: #2202764
> > > Signed-off-by: Yi-Hung Wei
> > > ---
> > > v1->v2: A unit test is added.
> > > v2->v3: Move the unit test from system traffic test to regular unit test.
> >
> > Thanks a lot, applied to master.
>
> Thanks for the review. The fixed patch was in 2.6, I am not sure if
> this patch can be cleanly applied to older branch. Can we try to
> backport it at least to 2.9 ?
It applied cleanly back to 2.8 so I applied it that far.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] treewide: Fix spelling of "receive".
Bleep bloop. Greetings Ben Pfaff, 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. build: libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-netflow.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-netflow.Tpo -c ofproto/netflow.c -o ofproto/ofproto_libofproto_la-netflow.o mv -f ofproto/.deps/ofproto_libofproto_la-netflow.Tpo ofproto/.deps/ofproto_libofproto_la-netflow.Plo /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto.lo `test -f 'ofproto/ofproto.c' || echo './'`ofproto/ofproto.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c ofproto/ofproto.c -o ofproto/ofproto_libofproto_la-ofproto.o ofproto/ofproto.c: In function 'ofproto_group_mod_finish': ofproto/ofproto.c:7400:13: error: missing initializer for field 'new_buckets' of 'struct ' [-Werror=missing-field-initializers] .new_buckets = new_group ? &new_group->buckets : NULL, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:135:36: note: 'new_buckets' declared here const struct ovs_list *new_buckets; ^ ofproto/ofproto.c:7401:13: error: missing initializer for field 'group_existed' of 'struct ' [-Werror=missing-field-initializers] .group_existed = group_collection_n(&ogm->old_groups) > 0, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:140:17: note: 'group_existed' declared here int group_existed; ^ cc1: all warnings being treated as errors make[2]: *** [ofproto/ofproto_libofproto_la-ofproto.lo] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 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] Reminder,
Hello Please i still await your response regarding my previous email. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] treewide: Fix spelling of "receive".
> On Sep 26, 2018, at 4:12 PM, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff Ha. Acked-by: Justin Pettit --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: Fix build with some GCC versions.
> On Sep 26, 2018, at 4:11 PM, Ben Pfaff wrote:
>
> GCC 4.8.x and possibly other versions don't like a designated initializer
> for an anonymous struct, see e.g.
> https://travis-ci.org/openvswitch/ovs/jobs/433747674
>
> Fixes: f836888d28ec ("ofproto: Handle OpenFlow version mismatch for
> requestforward with groups.")
> Signed-off-by: Ben Pfaff
Acked-by: Justin Pettit
--Justin
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 1/3] dpif-netlink: Detect Out-Of-Resource condition on a netdev
Hi Simon,
Thanks for your review comments. Please see my response inline.
On Mon, Sep 24, 2018 at 8:02 PM Simon Horman wrote:
>
> On Sat, Sep 15, 2018 at 09:40:08PM +0530, Sriharsha Basavapatna via dev wrote:
> > This is the first patch in the patch-set to support dynamic rebalancing
> > of offloaded flows.
> >
> > The patch detects OOR condition on a netdev port when ENOSPC error is
> > returned by TC-Flower while adding a flow rule. A new structure is added
> > to the netdev called "netdev_hw_info", to store OOR related information
> > required to perform dynamic offload-rebalancing.
> >
> > Signed-off-by: Sriharsha Basavapatna
> > Co-authored-by: Venkat Duvvuru
> > Signed-off-by: Venkat Duvvuru
> > Reviewed-by: Sathya Perla
> > ---
> > lib/dpif-netlink.c| 11 ++-
> > lib/flow.c| 25 +
> > lib/flow.h| 1 +
> > lib/netdev-provider.h | 11 +++
> > lib/netdev.c | 26 ++
> > lib/netdev.h | 2 ++
> > 6 files changed, 75 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index e6d5a6ec5..6ffe8af25 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -2178,7 +2178,16 @@ parse_flow_put(struct dpif_netlink *dpif, struct
> > dpif_flow_put *put)
> >
> > VLOG_DBG("added flow");
> > } else if (err != EEXIST) {
> > -VLOG_ERR_RL(&rl, "failed to offload flow: %s", ovs_strerror(err));
> > +struct netdev *oor_netdev = NULL;
> > +if (err == ENOSPC) {
> > +oor_netdev = flow_get_tunnel_netdev(&match.flow.tunnel);
>
> It seems that flow_get_tunnel_netdev() determines the offload device as per
> tunnel parameters. This is confusing to me. What if the flow is not
> for a tunnel?
If the flow is not for a tunnel operation (i.e, normal flow), we just use the
input netdev ('dev' in the line below: oor_netdev = dev). When the flow is
for a tunnel operation, we need to use the underlying tunnel device for
rebalancing. I've updated comments here.
Thanks,
-Harsha
>
> > +if (!oor_netdev) {
> > +oor_netdev = dev;
> > +}
> > +netdev_set_hw_info(oor_netdev, HW_INFO_TYPE_OOR, true);
> > +}
> > +VLOG_ERR_RL(&rl, "failed to offload flow: %s: %s",
> > ovs_strerror(err),
> > +(oor_netdev ? oor_netdev->name : dev->name));
> > }
> >
> > out:
> > diff --git a/lib/flow.c b/lib/flow.c
> > index 77ed3d9df..a39807908 100644
> > --- a/lib/flow.c
> > +++ b/lib/flow.c
> > @@ -19,6 +19,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -41,6 +42,8 @@
> > #include "unaligned.h"
> > #include "util.h"
> > #include "openvswitch/nsh.h"
> > +#include "ovs-router.h"
> > +#include "lib/netdev-provider.h"
> >
> > COVERAGE_DEFINE(flow_extract);
> > COVERAGE_DEFINE(miniflow_malloc);
> > @@ -3403,3 +3406,25 @@ flow_limit_vlans(int vlan_limit)
> > flow_vlan_limit = MIN(vlan_limit, FLOW_MAX_VLAN_HEADERS);
> > }
> > }
> > +
> > +struct netdev *
> > +flow_get_tunnel_netdev(struct flow_tnl *tunnel)
> > +{
> > +char iface[IFNAMSIZ];
> > +struct in6_addr ip6;
> > +struct in6_addr gw;
> > +
> > +if (tunnel->ip_src) {
> > +in6_addr_set_mapped_ipv4(&ip6, tunnel->ip_src);
> > +} else if (ipv6_addr_is_set(&tunnel->ipv6_src)) {
> > +ip6 = tunnel->ipv6_src;
> > +} else {
> > +return NULL;
> > +}
> > +
> > +if (!ovs_router_lookup(0, &ip6, iface, NULL, &gw)) {
> > +return NULL;
> > +}
> > +
> > +return netdev_from_name(iface);
> > +}
> > diff --git a/lib/flow.h b/lib/flow.h
> > index d03f1ba9c..aca60c41a 100644
> > --- a/lib/flow.h
> > +++ b/lib/flow.h
> > @@ -73,6 +73,7 @@ void flow_extract(struct dp_packet *, struct flow *);
> > void flow_zero_wildcards(struct flow *, const struct flow_wildcards *);
> > void flow_unwildcard_tp_ports(const struct flow *, struct flow_wildcards
> > *);
> > void flow_get_metadata(const struct flow *, struct match *flow_metadata);
> > +struct netdev *flow_get_tunnel_netdev(struct flow_tnl *tunnel);
> >
> > const char *ct_state_to_string(uint32_t state);
> > uint32_t ct_state_from_string(const char *);
> > diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> > index 5a7947351..e320dad61 100644
> > --- a/lib/netdev-provider.h
> > +++ b/lib/netdev-provider.h
> > @@ -35,6 +35,15 @@ extern "C" {
> > struct netdev_tnl_build_header_params;
> > #define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC
> >
> > +/* Offload-capable (HW) netdev information */
> > +struct netdev_hw_info {
> > +bool oor;/* Out of Offload Resources ? */
> > +};
> > +
> > +enum hw_info_type {
> > +HW_INFO_TYPE_OOR = 1 /* OOR state */
> > +};
> > +
> > /* A network device (e.g. an Ethernet device).
> > *
> > * Network device implementations may read these members but shoul
Re: [ovs-dev] [PATCH v6 2/3] revalidator: Gather packets-per-second rate of flows
Hi Simon,
Thanks for your review comments; please see my response inline.
On Mon, Sep 24, 2018 at 8:07 PM Simon Horman wrote:
>
> Hi Sriharsha,
>
> thanks for your patch.
>
> On Sat, Sep 15, 2018 at 09:40:09PM +0530, Sriharsha Basavapatna via dev wrote:
> > This is the second patch in the patch-set to support dynamic rebalancing
> > of offloaded flows.
> >
> > The packets-per-second (pps) rate for each flow is computed in the context
> > of revalidator threads when the flow stats are retrieved. The pps-rate is
> > computed only after a flow is revalidated and is not scheduled for
> > deletion. The parameters used to compute pps and the pps itself are saved
> > in udpif_key since they need to be persisted across iterations of
> > rebalancing.
> >
> > Signed-off-by: Sriharsha Basavapatna
> > Co-authored-by: Venkat Duvvuru
> > Signed-off-by: Venkat Duvvuru
> > Reviewed-by: Sathya Perla
> > ---
> > lib/dpif-provider.h | 1 +
> > ofproto/ofproto-dpif-upcall.c | 56 +++
> > 2 files changed, 57 insertions(+)
> >
> > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> > index 873b6e3d4..7a71f5c0a 100644
> > --- a/lib/dpif-provider.h
> > +++ b/lib/dpif-provider.h
> > @@ -39,6 +39,7 @@ struct dpif {
> > char *full_name;
> > uint8_t netflow_engine_type;
> > uint8_t netflow_engine_id;
> > +long long int current_ms;
> > };
> >
> > void dpif_init(struct dpif *, const struct dpif_class *, const char *name,
> > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> > index 6079f..9a36dca74 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -42,6 +42,8 @@
> > #include "tunnel.h"
> > #include "unixctl.h"
> > #include "openvswitch/vlog.h"
> > +#include "lib/dpif-provider.h"
> > +#include "lib/netdev-provider.h"
> >
> > #define MAX_QUEUE_LENGTH 512
> > #define UPCALL_MAX_BATCH 64
> > @@ -304,6 +306,15 @@ struct udpif_key {
> >
> > uint32_t key_recirc_id; /* Non-zero if reference is held by the
> > ukey. */
> > struct recirc_refs recircs; /* Action recirc IDs with references
> > held. */
> > +
> > +#define OFFL_REBAL_INTVL_MSEC 3000 /* dynamic offload rebalance freq */
>
> I'd be interested to here what considerations were taken
> into account when selecting 3000ms.
Since we compute per-second packet rate (pps) and also because revalidation
runs at 500 msec, we need to have the rebalancing duration > 1 second. We also
want to provide fast response to flows with higher pps-rate that might be
pending. Hence we are using 3 seconds interval. If this seems aggressive, we
can increase it; let me know your thoughts.
>
> > +struct netdev *in_netdev;/* in_odp_port's netdev */
> > +struct netdev *out_netdev; /* out_odp_port's netdev */
>
> in_netdev, out_netdev and possibly other fields are do not
> appear to be used in this patch. I'd prefer if fields were
> added in the patch where they are first used.
done.
>
> > +bool offloaded; /* True if flow is offloaded */
> > +uint64_t flow_pps_rate; /* Packets-Per-Second rate */
> > +long long int flow_time; /* last pps update time */
> > +uint64_t flow_packets; /* #pkts seen in interval */
> > +uint64_t flow_backlog_packets; /* prev-mode #pkts (offl or kernel) */
> > };
> >
> > /* Datapath operation with optional ukey attached. */
> > @@ -1667,6 +1678,11 @@ ukey_create__(const struct nlattr *key, size_t
> > key_len,
> > ukey->stats.used = used;
> > ukey->xcache = NULL;
> >
> > +ukey->offloaded = false;
> > +ukey->flow_time = 0;
> > +ukey->in_netdev = ukey->out_netdev = NULL;
> > +ukey->flow_packets = ukey->flow_backlog_packets = 0;
> > +
> > ukey->key_recirc_id = key_recirc_id;
> > recirc_refs_init(&ukey->recircs);
> > if (xout) {
> > @@ -2442,6 +2458,42 @@ reval_op_init(struct ukey_op *op, enum reval_result
> > result,
> > }
> > }
> >
> > +static uint64_t
> > +udpif_flow_packet_delta(struct udpif_key *ukey, const struct dpif_flow *f)
> > +{
> > +return f->stats.n_packets + ukey->flow_backlog_packets -
> > +ukey->flow_packets;
> > +}
> > +
> > +static long long int
> > +udpif_flow_time_delta(struct udpif *udpif, struct udpif_key *ukey)
> > +{
> > +return (udpif->dpif->current_ms - ukey->flow_time) / 1000;
> > +}
> > +
> > +/* Gather pps-rate for the given dpif_flow and save it in its ukey */
> > +static void
> > +udpif_update_flow_pps(struct udpif *udpif, struct udpif_key *ukey,
> > + const struct dpif_flow *f)
> > +{
> > +uint64_t pps;
> > +
> > +/* Update pps-rate only when we are close to rebalance interval */
> > +if (udpif->dpif->current_ms - ukey->flow_time < OFFL_REBAL_INTVL_MSEC)
> > {
> > +return;
> > +}
> > +
> > +ovs_assert(f->stats.n_packets + ukey->flow_backlog_packets >=
> > +
Re: [ovs-dev] [PATCH v6 3/3] revalidator: Rebalance offloaded flows based on the pps rate
Hi Simon,
Thanks for your review comments; please see my response inline.
On Mon, Sep 24, 2018 at 8:16 PM Simon Horman wrote:
>
> Hi Sriharsha,
>
> thanks for your patch.
>
> I am pleased to see work in this area. I do however, have some questions
> about the implementation. Please see comments inline.
>
> On Sat, Sep 15, 2018 at 09:40:10PM +0530, Sriharsha Basavapatna via dev wrote:
> > This is the third patch in the patch-set to support dynamic rebalancing
> > of offloaded flows.
> >
> > The dynamic rebalancing functionality is implemented in this patch. The
> > ukeys that are not scheduled for deletion are obtained and passed as input
> > to the rebalancing routine. The rebalancing is done in the context of
> > revalidation leader thread, after all other revalidator threads are
> > done with gathering rebalancing data for flows.
> >
> > For each netdev that is in OOR state, a list of flows - both offloaded
> > and non-offloaded (pending) - is obtained using the ukeys. For each netdev
> > that is in OOR state, the flows are grouped and sorted into offloaded and
> > pending flows. The offloaded flows are sorted in descending order of
> > pps-rate, while pending flows are sorted in ascending order of pps-rate.
> >
> > The rebalancing is done in two phases. In the first phase, we try to
> > offload all pending flows and if that succeeds, the OOR state on the device
> > is cleared. If some (or none) of the pending flows could not be offloaded,
> > then we start replacing an offloaded flow that has a lower pps-rate than
> > a pending flow, until there are no more pending flows with a higher rate
> > than an offloaded flow. The flows that are replaced from the device are
> > added into kernel datapath.
> >
> > A new OVS configuration parameter "offload-rebalance", is added to ovsdb.
> > The default value of this is "false". To enable this feature, set the
> > value of this parameter to "true", which provides packets-per-second
> > rate based policy to dynamically offload and un-offload flows.
> >
> > Note: This option can be enabled only when 'hw-offload' policy is enabled.
> > It also requires 'tc-policy' to be set to 'skip_sw'; otherwise, flow
> > offload errors (specifically ENOSPC error this feature depends on) reported
> > by an offloaded device are supressed by TC-Flower kernel module.
> >
> > Signed-off-by: Sriharsha Basavapatna
> > Co-authored-by: Venkat Duvvuru
> > Signed-off-by: Venkat Duvvuru
> > Reviewed-by: Sathya Perla
> > ---
> > NEWS | 3 +
> > lib/dpif-netdev.c | 3 +-
> > lib/dpif-netlink.c| 20 +-
> > lib/dpif-provider.h | 8 +-
> > lib/dpif.c| 30 ++-
> > lib/dpif.h| 12 +-
> > lib/netdev-provider.h | 7 +-
> > lib/netdev.c | 71 +-
> > lib/netdev.h | 2 +
> > ofproto/ofproto-dpif-upcall.c | 468 +-
> > vswitchd/vswitch.xml | 21 ++
> > 11 files changed, 614 insertions(+), 31 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 33b4d8a23..846b46fb5 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -8,6 +8,9 @@ Post-v2.10.0
> > as the default timeout for control utilities.
> > - ovn:
> > * ovn-ctl: allow passing user:group ids to the OVN daemons.
> > + - ovs-vswitchd:
> > + * New configuration option "offload-rebalance", that enables dynamic
> > + rebalancing of offloaded flows.
> >
> >
> > v2.10.0 - xx xxx
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 7c0300cc5..1c01d2278 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -3689,7 +3689,8 @@ dpif_netdev_execute(struct dpif *dpif, struct
> > dpif_execute *execute)
> > }
> >
> > static void
> > -dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops)
> > +dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops,
> > +enum dpif_offload_type offload_type OVS_UNUSED)
> > {
> > size_t i;
> >
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index 6ffe8af25..881dbc69c 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -2179,7 +2179,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;
> > -if (err == ENOSPC) {
> > +if (err == ENOSPC && netdev_is_offload_rebalance_policy_enabled())
> > {
>
> I think that the check on netdev_is_offload_rebalance_policy_enabled()
> should be added to the patch that adds the lines immediately below.
> Perhaps via a dummy implementation of
> netdev_is_offload_rebalance_policy_enabled that always returns false.
done.
>
> > oor_netdev = flow_get_tunnel_netdev(&match.flow.tunnel);
> > if (!oor_netdev) {
> > oor_netdev = dev;
> > @@ -2274,7 +2274,8 @@
[ovs-dev] [PATCH v7 0/3] Support dynamic rebalancing of offloaded flows
With the current OVS offload design, when an offload-device fails to add a flow rule and returns an error, OVS adds the rule to the kernel datapath. The flow gets processed by the kernel datapath for the entire life of that flow. This is fine when an error is returned by the device due to lack of support for certain keys or actions. But when an error is returned due to temporary conditions such as lack of resources to add a flow rule, the flow continues to be processed by kernel even when resources become available later. That is, those flows never get offloaded again. This problem becomes more pronounced when a flow that has been initially offloaded may have a smaller packet rate than a later flow that could not be offloaded due to lack of resources. This leads to inefficient use of HW resources and wastage of host CPU cycles. This patch-set addresses this issue by providing a way to detect temporary offload resource constraints (Out-Of-Resource or OOR condition) and to selectively and dynamically offload flows with a higher packets-per-second (pps) rate. This dynamic rebalancing is done periodically on netdevs that are in OOR state until resources become available to offload all pending flows. The patch-set involves the following changes at a high level: 1. Detection of Out-Of-Resources (OOR) condition on an offload-capable netdev. 2. Gathering flow offload selection criteria for all flows on an OOR netdev; i.e, packets-per-second (pps) rate of flows for offloaded and non-offloaded (pending) flows. 3. Dynamically replacing offloaded flows with a lower pps-rate, with non-offloaded flows with a higher pps-rate, on an OOR netdev. A new OpenvSwitch configuration option - "offload-rebalance" to enable this policy. Cost/benefits data points: 1. Rough cost of the new rebalancing, in terms of CPU time: Ran a test that replaced 256 low pps-rate flows(pings) with 256 high pps-rate flows(iperf), in a system with 4 cpus (Intel Xeon E5 @ 2.40GHz; 2 cores with hw threads enabled, rest disabled). The data showed that cpu utilization increased by about ~20%. This increase occurs during the specific second in which rebalancing is done. And subsequently (from the next second), cpu utilization decreases significantly due to offloading of higher pps-rate flows. So effectively there's a bump in cpu utilization at the time of rebalancing, that is more than compensated by reduced cpu utilization once the right flows get offloaded. 2. Rough benefits to the user in terms of offload performance: The benefits to the user is reduced cpu utilization in the host, since higher pps-rate flows get offloaded, replacing lower pps-rate flows. Replacing a single offloaded flood ping flow with an iperf flow (multiple connections), shows that the cpu %used that was originally 100% on a single cpu (rebalancing disabled) goes down to 35% (rebalancing enabled). That is, cpu utilization decreased 65% after rebalancing. 3. Circumstances under which the benefits would show up: The rebalancing benefits would show up once offload resources are exhausted and new flows with higher pps-rate are initiated, that would otherwise be handled by kernel datapath costing host cpu cycles. This can be observed using 'ovs appctl dpctl/ dump-flows' command. Prior to rebalancing, any high pps-rate flows that couldn't be offloaded due to resource crunch would show up in the output of 'dump-flows type=ovs' and after rebalancing such flows would appear in the output of 'dump-flows type=offloaded'. ** v6-->v7: - Fixed comments from Simon Horman. Key changes: - Removed out_netdev and related code which is not needed. - Fixed a potential race between un-offloading and adding a rule to kernel. - Moved config option parameter check to respective patches. - Removed a couple of assertions. - Added a few comments. - Added a workaround to avoid a checkpatch warning for a pre-decrement op. v5-->v6: - Fixed a build error reported by 0-day robot in Patch-2 v4-->v5: - Fixed v4 comments from Ben Pfaff. Key changes: - Updated patch-0 comments to reflect cost/benefit of rebalancing - Release references to netdevs after rebalancing - Reduce ref holding window to rebalancing routine - Avoid collecting all flows before rebalancing (save time/space) - Skip rebalancing when no OOR netdev (new function netdev_any_oor()) - Avoid direct access to netdev members (add new netdev interfaces) - Handle tunnel-netdev as a special case of in-netdev (no separate var) - Remove floating point type for pps-rate (not needed, per-second based) - Simplify ukey_to_flow_netdevs() (remove tmp dpif_flow) - Update skip_offload code to more meaningful offload_type - dpif_operate() should validate offload_types - dpif_netlink_operate() should process all ops and set error if needed - Save time of last rebalancing in udpif (remove static vars) -
[ovs-dev] [PATCH v7 1/3] dpif-netlink: Detect Out-Of-Resource condition on a netdev
This is the first patch in the patch-set to support dynamic rebalancing
of offloaded flows.
The patch detects OOR condition on a netdev port when ENOSPC error is
returned by TC-Flower while adding a flow rule. A new structure is added
to the netdev called "netdev_hw_info", to store OOR related information
required to perform dynamic offload-rebalancing.
Signed-off-by: Sriharsha Basavapatna
Co-authored-by: Venkat Duvvuru
Signed-off-by: Venkat Duvvuru
Reviewed-by: Sathya Perla
---
lib/dpif-netlink.c| 18 +-
lib/flow.c| 25 +
lib/flow.h| 1 +
lib/netdev-provider.h | 11 +++
lib/netdev.c | 35 +++
lib/netdev.h | 3 +++
6 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e6d5a6ec5..b9ce9cbe2 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2178,7 +2178,23 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
VLOG_DBG("added flow");
} else if (err != EEXIST) {
-VLOG_ERR_RL(&rl, "failed to offload flow: %s", ovs_strerror(err));
+struct netdev *oor_netdev = NULL;
+if (err == ENOSPC && netdev_is_offload_rebalance_policy_enabled()) {
+/*
+ * We need to set OOR on the input netdev (i.e, 'dev') for the
+ * flow. But if the flow has a tunnel attribute (i.e, decap action,
+ * with a virtual device like a VxLAN interface as its in-port),
+ * then lookup and set OOR on the underlying tunnel (real) netdev.
+ */
+oor_netdev = flow_get_tunnel_netdev(&match.flow.tunnel);
+if (!oor_netdev) {
+/* Not a 'tunnel' flow */
+oor_netdev = dev;
+}
+netdev_set_hw_info(oor_netdev, HW_INFO_TYPE_OOR, true);
+}
+VLOG_ERR_RL(&rl, "failed to offload flow: %s: %s", ovs_strerror(err),
+(oor_netdev ? oor_netdev->name : dev->name));
}
out:
diff --git a/lib/flow.c b/lib/flow.c
index 77ed3d9df..a39807908 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -41,6 +42,8 @@
#include "unaligned.h"
#include "util.h"
#include "openvswitch/nsh.h"
+#include "ovs-router.h"
+#include "lib/netdev-provider.h"
COVERAGE_DEFINE(flow_extract);
COVERAGE_DEFINE(miniflow_malloc);
@@ -3403,3 +3406,25 @@ flow_limit_vlans(int vlan_limit)
flow_vlan_limit = MIN(vlan_limit, FLOW_MAX_VLAN_HEADERS);
}
}
+
+struct netdev *
+flow_get_tunnel_netdev(struct flow_tnl *tunnel)
+{
+char iface[IFNAMSIZ];
+struct in6_addr ip6;
+struct in6_addr gw;
+
+if (tunnel->ip_src) {
+in6_addr_set_mapped_ipv4(&ip6, tunnel->ip_src);
+} else if (ipv6_addr_is_set(&tunnel->ipv6_src)) {
+ip6 = tunnel->ipv6_src;
+} else {
+return NULL;
+}
+
+if (!ovs_router_lookup(0, &ip6, iface, NULL, &gw)) {
+return NULL;
+}
+
+return netdev_from_name(iface);
+}
diff --git a/lib/flow.h b/lib/flow.h
index d03f1ba9c..aca60c41a 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -73,6 +73,7 @@ void flow_extract(struct dp_packet *, struct flow *);
void flow_zero_wildcards(struct flow *, const struct flow_wildcards *);
void flow_unwildcard_tp_ports(const struct flow *, struct flow_wildcards *);
void flow_get_metadata(const struct flow *, struct match *flow_metadata);
+struct netdev *flow_get_tunnel_netdev(struct flow_tnl *tunnel);
const char *ct_state_to_string(uint32_t state);
uint32_t ct_state_from_string(const char *);
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 5a7947351..e320dad61 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -35,6 +35,15 @@ extern "C" {
struct netdev_tnl_build_header_params;
#define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC
+/* Offload-capable (HW) netdev information */
+struct netdev_hw_info {
+bool oor; /* Out of Offload Resources ? */
+};
+
+enum hw_info_type {
+HW_INFO_TYPE_OOR = 1 /* OOR state */
+};
+
/* A network device (e.g. an Ethernet device).
*
* Network device implementations may read these members but should not modify
@@ -80,6 +89,8 @@ struct netdev {
int n_rxq;
struct shash_node *node;/* Pointer to element in global map. */
struct ovs_list saved_flags_list; /* Contains "struct netdev_saved_flags".
*/
+
+struct netdev_hw_info hw_info; /* offload-capable netdev info */
};
static inline void
diff --git a/lib/netdev.c b/lib/netdev.c
index 690d28409..3e2ce7c09 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -415,6 +415,7 @@ netdev_open(const char *name, const char *type, struct
netdev **netdevp)
netdev->reconfigure_seq = seq_create();
netdev->last_reconfigure_seq =
seq_read(netdev->reconfigure_seq);
+
[ovs-dev] [PATCH v7 2/3] revalidator: Gather packets-per-second rate of flows
This is the second patch in the patch-set to support dynamic rebalancing
of offloaded flows.
The packets-per-second (pps) rate for each flow is computed in the context
of revalidator threads when the flow stats are retrieved. The pps-rate is
computed only after a flow is revalidated and is not scheduled for
deletion. The parameters used to compute pps and the pps itself are saved
in udpif_key since they need to be persisted across iterations of
rebalancing.
Signed-off-by: Sriharsha Basavapatna
Co-authored-by: Venkat Duvvuru
Signed-off-by: Venkat Duvvuru
Reviewed-by: Sathya Perla
---
lib/dpif-provider.h | 1 +
ofproto/ofproto-dpif-upcall.c | 51 +++
2 files changed, 52 insertions(+)
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 873b6e3d4..7a71f5c0a 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -39,6 +39,7 @@ struct dpif {
char *full_name;
uint8_t netflow_engine_type;
uint8_t netflow_engine_id;
+long long int current_ms;
};
void dpif_init(struct dpif *, const struct dpif_class *, const char *name,
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 6079f..a372d6252 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -42,6 +42,8 @@
#include "tunnel.h"
#include "unixctl.h"
#include "openvswitch/vlog.h"
+#include "lib/dpif-provider.h"
+#include "lib/netdev-provider.h"
#define MAX_QUEUE_LENGTH 512
#define UPCALL_MAX_BATCH 64
@@ -304,6 +306,13 @@ struct udpif_key {
uint32_t key_recirc_id; /* Non-zero if reference is held by the ukey. */
struct recirc_refs recircs; /* Action recirc IDs with references held. */
+
+#define OFFL_REBAL_INTVL_MSEC 3000/* dynamic offload rebalance freq */
+bool offloaded;/* True if flow is offloaded */
+uint64_t flow_pps_rate;/* Packets-Per-Second rate */
+long long int flow_time; /* last pps update time */
+uint64_t flow_packets; /* #pkts seen in interval */
+uint64_t flow_backlog_packets; /* prev-mode #pkts (offl or kernel) */
};
/* Datapath operation with optional ukey attached. */
@@ -1667,6 +1676,10 @@ ukey_create__(const struct nlattr *key, size_t key_len,
ukey->stats.used = used;
ukey->xcache = NULL;
+ukey->offloaded = false;
+ukey->flow_time = 0;
+ukey->flow_packets = ukey->flow_backlog_packets = 0;
+
ukey->key_recirc_id = key_recirc_id;
recirc_refs_init(&ukey->recircs);
if (xout) {
@@ -2442,6 +2455,39 @@ reval_op_init(struct ukey_op *op, enum reval_result
result,
}
}
+static uint64_t
+udpif_flow_packet_delta(struct udpif_key *ukey, const struct dpif_flow *f)
+{
+return f->stats.n_packets + ukey->flow_backlog_packets -
+ukey->flow_packets;
+}
+
+static long long int
+udpif_flow_time_delta(struct udpif *udpif, struct udpif_key *ukey)
+{
+return (udpif->dpif->current_ms - ukey->flow_time) / 1000;
+}
+
+/* Gather pps-rate for the given dpif_flow and save it in its ukey */
+static void
+udpif_update_flow_pps(struct udpif *udpif, struct udpif_key *ukey,
+ const struct dpif_flow *f)
+{
+uint64_t pps;
+
+/* Update pps-rate only when we are close to rebalance interval */
+if (udpif->dpif->current_ms - ukey->flow_time < OFFL_REBAL_INTVL_MSEC) {
+return;
+}
+
+ukey->offloaded = f->attrs.offloaded;
+pps = udpif_flow_packet_delta(ukey, f) /
+udpif_flow_time_delta(udpif, ukey);
+ukey->flow_pps_rate = pps;
+ukey->flow_packets = ukey->flow_backlog_packets + f->stats.n_packets;
+ukey->flow_time = udpif->dpif->current_ms;
+}
+
static void
revalidate(struct revalidator *revalidator)
{
@@ -2550,6 +2596,11 @@ revalidate(struct revalidator *revalidator)
}
ukey->dump_seq = dump_seq;
+if (netdev_is_offload_rebalance_policy_enabled() &&
+result != UKEY_DELETE) {
+udpif_update_flow_pps(udpif, ukey, f);
+}
+
if (result != UKEY_KEEP) {
/* Takes ownership of 'recircs'. */
reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs,
--
2.18.0.rc1.1.g6f333ff
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v7 3/3] revalidator: Rebalance offloaded flows based on the pps rate
This is the third patch in the patch-set to support dynamic rebalancing
of offloaded flows.
The dynamic rebalancing functionality is implemented in this patch. The
ukeys that are not scheduled for deletion are obtained and passed as input
to the rebalancing routine. The rebalancing is done in the context of
revalidation leader thread, after all other revalidator threads are
done with gathering rebalancing data for flows.
For each netdev that is in OOR state, a list of flows - both offloaded
and non-offloaded (pending) - is obtained using the ukeys. For each netdev
that is in OOR state, the flows are grouped and sorted into offloaded and
pending flows. The offloaded flows are sorted in descending order of
pps-rate, while pending flows are sorted in ascending order of pps-rate.
The rebalancing is done in two phases. In the first phase, we try to
offload all pending flows and if that succeeds, the OOR state on the device
is cleared. If some (or none) of the pending flows could not be offloaded,
then we start replacing an offloaded flow that has a lower pps-rate than
a pending flow, until there are no more pending flows with a higher rate
than an offloaded flow. The flows that are replaced from the device are
added into kernel datapath.
A new OVS configuration parameter "offload-rebalance", is added to ovsdb.
The default value of this is "false". To enable this feature, set the
value of this parameter to "true", which provides packets-per-second
rate based policy to dynamically offload and un-offload flows.
Note: This option can be enabled only when 'hw-offload' policy is enabled.
It also requires 'tc-policy' to be set to 'skip_sw'; otherwise, flow
offload errors (specifically ENOSPC error this feature depends on) reported
by an offloaded device are supressed by TC-Flower kernel module.
Signed-off-by: Sriharsha Basavapatna
Co-authored-by: Venkat Duvvuru
Signed-off-by: Venkat Duvvuru
Reviewed-by: Sathya Perla
---
NEWS | 3 +
lib/dpif-netdev.c | 3 +-
lib/dpif-netlink.c| 29 ++-
lib/dpif-provider.h | 8 +-
lib/dpif.c| 30 ++-
lib/dpif.h| 12 +-
lib/netdev-provider.h | 7 +-
lib/netdev.c | 62 -
lib/netdev.h | 1 +
ofproto/ofproto-dpif-upcall.c | 445 +-
vswitchd/vswitch.xml | 21 ++
11 files changed, 592 insertions(+), 29 deletions(-)
diff --git a/NEWS b/NEWS
index 33b4d8a23..846b46fb5 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,9 @@ Post-v2.10.0
as the default timeout for control utilities.
- ovn:
* ovn-ctl: allow passing user:group ids to the OVN daemons.
+ - ovs-vswitchd:
+ * New configuration option "offload-rebalance", that enables dynamic
+ rebalancing of offloaded flows.
v2.10.0 - xx xxx
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7c0300cc5..1c01d2278 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3689,7 +3689,8 @@ dpif_netdev_execute(struct dpif *dpif, struct
dpif_execute *execute)
}
static void
-dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops)
+dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops,
+enum dpif_offload_type offload_type OVS_UNUSED)
{
size_t i;
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index b9ce9cbe2..2e01f5750 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2281,7 +2281,8 @@ dpif_netlink_operate_chunks(struct dpif_netlink *dpif,
struct dpif_op **ops,
}
static void
-dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops)
+dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops,
+ enum dpif_offload_type offload_type)
{
struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
struct dpif_op *new_ops[OPERATE_MAX_OPS];
@@ -2289,7 +2290,12 @@ dpif_netlink_operate(struct dpif *dpif_, struct dpif_op
**ops, size_t n_ops)
int i = 0;
int err = 0;
-if (netdev_is_flow_api_enabled()) {
+if (offload_type == DPIF_OFFLOAD_ALWAYS && !netdev_is_flow_api_enabled()) {
+VLOG_DBG("Invalid offload_type: %d", offload_type);
+return;
+}
+
+if (offload_type != DPIF_OFFLOAD_NEVER && netdev_is_flow_api_enabled()) {
while (n_ops > 0) {
count = 0;
@@ -2298,6 +2304,23 @@ dpif_netlink_operate(struct dpif *dpif_, struct dpif_op
**ops, size_t n_ops)
err = try_send_to_netdev(dpif, op);
if (err && err != EEXIST) {
+if (offload_type == DPIF_OFFLOAD_ALWAYS) {
+/* We got an error while offloading an op. Since
+ * OFFLOAD_ALWAYS is specified, we stop further
+ * processing and return to the caller without
+ * invoking kernel datapath as fallback. But the
+
Re: [ovs-dev] dpif-netlink: Detect Out-Of-Resource condition on a netdev
Bleep bloop. Greetings Sriharsha Basavapatna via dev, 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 Sriharsha Basavapatna via dev needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Sriharsha Basavapatna Lines checked: 223, Warnings: 1, Errors: 1 build: libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-netflow.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-netflow.Tpo -c ofproto/netflow.c -o ofproto/ofproto_libofproto_la-netflow.o mv -f ofproto/.deps/ofproto_libofproto_la-netflow.Tpo ofproto/.deps/ofproto_libofproto_la-netflow.Plo /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto.lo `test -f 'ofproto/ofproto.c' || echo './'`ofproto/ofproto.c libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto.Tpo -c ofproto/ofproto.c -o ofproto/ofproto_libofproto_la-ofproto.o ofproto/ofproto.c: In function 'ofproto_group_mod_finish': ofproto/ofproto.c:7400:13: error: missing initializer for field 'new_buckets' of 'struct ' [-Werror=missing-field-initializers] .new_buckets = new_group ? &new_group->buckets : NULL, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:135:36: note: 'new_buckets' declared here const struct ovs_list *new_buckets; ^ ofproto/ofproto.c:7401:13: error: missing initializer for field 'group_existed' of 'struct ' [-Werror=missing-field-initializers] .group_existed = group_collection_n(&ogm->old_groups) > 0, ^ In file included from ofproto/ofproto.c:48:0: ./include/openvswitch/ofp-monitor.h:140:17: note: 'group_existed' declared here int group_existed; ^ cc1: all warnings being treated as errors make[2]: *** [ofproto/ofproto_libofproto_la-ofproto.lo] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 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
Re: [ovs-dev] revalidator: Rebalance offloaded flows based on the pps rate
Bleep bloop. Greetings Sriharsha Basavapatna via dev, 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. git-am: fatal: sha1 information is lacking or useless (lib/dpif-netlink.c). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 revalidator: Rebalance offloaded flows based on the pps rate The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". 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
