Re: [ovs-dev] [PATCH ovn] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

[Numan Siddique]

On Mon, Dec 2, 2019 at 12:44 PM Han Zhou  wrote:
>
> On Fri, Nov 29, 2019 at 1:08 AM  wrote:
> >
> > From: Numan Siddique 
> >
> > If ofctrl_check_and_add_flow(F') is called where flow F' has
> match-actions (M, A2)
> > and if there already exists a flow F with match-actions (M, A1) in the
> desired flow
> > table, then this function  doesn't update the existing flow F with new
> actions
> > actions A2.
> >
> > This patch fixes it. Presently we don't see any issues because of this
> behaviour.
> > But it's better to fix it.
> >
>
> Hi Numan, could you explain why do you think the F' should override the F?
> For my understanding, this is a problem of duplicated logical flows
> generated by ovn-northd and can't be solved in ovn-controller. The desired
> flows have conflict and there is no way to judge which one should be
> applied.
>

Hi Han,

I probably should have given the context in which I observed this issue.
I am working on making incremental processing for datapath additions/deletions.

In my testing I observed that the test case -  84: ovn.at:10767
ovn -- send gratuitous ARP for NAT rules on HA distributed router
is failing 100% of the time.

Upon investigation I found that it's failing for below logical flow

 table=17(ls_in_l2_lkup  ), priority=80   , match=(eth.src == {
00:00:00:00:ff:01} && (arp.op == 1 || nd_ns)), action=(outport =
"_MC_flood"; output;)

If you see the test code here -
https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10901

ovn-nbctl --wait=hv clear logical_switch_port lrp0-rp options

When the above command is executed, the logical switch "ls0" is
removed from the "local_datapaths" hmap.
When ls0 is removed, ovsdb-server sends monitor condition to remove
the "Multicast_Group" row ls0.

Later when this command is executed  - ovn-nbctl --wait=hv
lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router"

ovn-controller gets this update from sb ovsdb-server and it adds back
ls0 to "local_datapaths". At this point, "Multicast_Group" row
for ls0 is empty and the above logical flow  translates to "set:0->reg15".

Soon after ovn-controller receives monitor update message to add back
the "Multicast_Group" row for ls0.
With the patch I am working, calculates logical flows for the logical
switch sw0. But it doesn't add the right flow. The action should have
been set to "set:0x8000->reg15".

The patch I am working on can be found here - [1] -
https://github.com/numansiddique/ovn/commit/024812e76f4d0628612b1885cca65302a64038c8
https://github.com/numansiddique/ovn/tree/lflow_reloaded/v1/p1

Please note I am still testing it out and doing some scale testing
before submitting the patch for review.
[1] adds  a new table - "Datapath_Logical_Flow" in south db which is a
weak ref and its references in Datapath_Binding.

We can discuss more about the approach later.

But I think the proposed patch here makes sense. We don't hit this
issue in the present code because when ever any datapath_binding
change happens
we do full recompute and this flushes out the old  logical flow in the
desired_flow_table.

Thanks
Numan


> > Signed-off-by: Numan Siddique 
> > ---
> >  controller/ofctrl.c | 23 ---
> >  1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > index 10edd84fb..5a174da48 100644
> > --- a/controller/ofctrl.c
> > +++ b/controller/ofctrl.c
> > @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct
> ovn_desired_flow_table *flow_table,
> >
> >  ovn_flow_log(f, "ofctrl_add_flow");
> >
> > -if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) {
> > -if (log_duplicate_flow) {
> > -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 5);
> > -if (!VLOG_DROP_DBG(&rl)) {
> > -char *s = ovn_flow_to_string(f);
> > -VLOG_DBG("dropping duplicate flow: %s", s);
> > -free(s);
> > +struct ovn_flow *existing_f =
> > +ovn_flow_lookup(&flow_table->match_flow_table, f, true);
> > +if (existing_f) {
> > +if (ofpacts_equal(f->ofpacts, f->ofpacts_len,
> > +  existing_f->ofpacts, existing_f->ofpacts_len))
> {
> > +if (log_duplicate_flow) {
> > +static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(5, 5);
> > +if (!VLOG_DROP_DBG(&rl)) {
> > +char *s = ovn_flow_to_string(f);
> > +VLOG_DBG("dropping duplicate flow: %s", s);
> > +free(s);
> > +}
> >  }
> > +} else {
> > +free(existing_f->ofpacts);
> > +existing_f->ofpacts = xmemdup(f->ofpacts, f->ofpacts_len);
> > +existing_f->ofpacts_len = f->ofpacts_len;
> >  }
> >  ovn_flow_destroy(f);
> >  return;
> > --
> > 2.23.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > 

Re: [ovs-dev] [PATCH ovn] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

[Han Zhou]

On Sun, Dec 1, 2019 at 11:59 PM Numan Siddique  wrote:
>
> On Mon, Dec 2, 2019 at 12:44 PM Han Zhou  wrote:
> >
> > On Fri, Nov 29, 2019 at 1:08 AM  wrote:
> > >
> > > From: Numan Siddique 
> > >
> > > If ofctrl_check_and_add_flow(F') is called where flow F' has
> > match-actions (M, A2)
> > > and if there already exists a flow F with match-actions (M, A1) in the
> > desired flow
> > > table, then this function  doesn't update the existing flow F with new
> > actions
> > > actions A2.
> > >
> > > This patch fixes it. Presently we don't see any issues because of this
> > behaviour.
> > > But it's better to fix it.
> > >
> >
> > Hi Numan, could you explain why do you think the F' should override the
F?
> > For my understanding, this is a problem of duplicated logical flows
> > generated by ovn-northd and can't be solved in ovn-controller. The
desired
> > flows have conflict and there is no way to judge which one should be
> > applied.
> >
>
> Hi Han,
>
> I probably should have given the context in which I observed this issue.
> I am working on making incremental processing for datapath
additions/deletions.
>
> In my testing I observed that the test case -  84: ovn.at:10767
> ovn -- send gratuitous ARP for NAT rules on HA distributed router
> is failing 100% of the time.
>
> Upon investigation I found that it's failing for below logical flow
>
>  table=17(ls_in_l2_lkup  ), priority=80   , match=(eth.src == {
> 00:00:00:00:ff:01} && (arp.op == 1 || nd_ns)), action=(outport =
> "_MC_flood"; output;)
>
> If you see the test code here -
> https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10901
>
> ovn-nbctl --wait=hv clear logical_switch_port lrp0-rp options
>
> When the above command is executed, the logical switch "ls0" is
> removed from the "local_datapaths" hmap.
> When ls0 is removed, ovsdb-server sends monitor condition to remove
> the "Multicast_Group" row ls0.
>
> Later when this command is executed  - ovn-nbctl --wait=hv
> lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router"
>
> ovn-controller gets this update from sb ovsdb-server and it adds back
> ls0 to "local_datapaths". At this point, "Multicast_Group" row
> for ls0 is empty and the above logical flow  translates to "set:0->reg15".
>
> Soon after ovn-controller receives monitor update message to add back
> the "Multicast_Group" row for ls0.
> With the patch I am working, calculates logical flows for the logical
> switch sw0. But it doesn't add the right flow. The action should have
> been set to "set:0x8000->reg15".
>
> The patch I am working on can be found here - [1] -
>
https://github.com/numansiddique/ovn/commit/024812e76f4d0628612b1885cca65302a64038c8
> https://github.com/numansiddique/ovn/tree/lflow_reloaded/v1/p1
>
> Please note I am still testing it out and doing some scale testing
> before submitting the patch for review.
> [1] adds  a new table - "Datapath_Logical_Flow" in south db which is a
> weak ref and its references in Datapath_Binding.
>
> We can discuss more about the approach later.
>
> But I think the proposed patch here makes sense. We don't hit this
> issue in the present code because when ever any datapath_binding
> change happens
> we do full recompute and this flushes out the old  logical flow in the
> desired_flow_table.
>
Hi Numan,

Thanks for explaining the context. The principle in I-P for handling
changes is always handling deletions first and then creations, and for
updates, we always delete the old entries and then add the new ones.
In your context, if a mc_group is updated, we should delete the old flows
related to that mc_group and then create the new flows. Would the problem
be solved if we follow this principle?
In my view, ofctrl_check_and_add_flow() is not the right place for this,
because it already lose the context whether it is duplicated logical flows
generated by northd (e.g. conflict ACLs, or bugs), or it is just transient
status during ovn-controller processing, which is the case you encountered.

Thanks,
Han
>
> > > Signed-off-by: Numan Siddique 
> > > ---
> > >  controller/ofctrl.c | 23 ---
> > >  1 file changed, 16 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c
> > > index 10edd84fb..5a174da48 100644
> > > --- a/controller/ofctrl.c
> > > +++ b/controller/ofctrl.c
> > > @@ -667,14 +667,23 @@ ofctrl_check_and_add_flow(struct
> > ovn_desired_flow_table *flow_table,
> > >
> > >  ovn_flow_log(f, "ofctrl_add_flow");
> > >
> > > -if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) {
> > > -if (log_duplicate_flow) {
> > > -static struct vlog_rate_limit rl =
VLOG_RATE_LIMIT_INIT(5,
> > 5);
> > > -if (!VLOG_DROP_DBG(&rl)) {
> > > -char *s = ovn_flow_to_string(f);
> > > -VLOG_DBG("dropping duplicate flow: %s", s);
> > > -free(s);
> > > +struct ovn_flow *existing_f =
> > > +ovn_flow_lookup(&flow_table->match_flow_tab

Re: [ovs-dev] [OVN][RAFT] Follower refusing new entries from leader

[Han Zhou]

Sorry for the late reply. It was holiday here.
I didn't see such problem when there is no compaction. Did you see this
problem when DB compaction didn't happen? The difference is that after
compaction the RAFT log doesn't have any entries and all the data is in the
snapshot.

On Fri, Nov 29, 2019 at 12:11 AM taoyunupt  wrote:

> Hi,Han
>   Hope to receive your reply.
>
>
> Thanks,
> Yun
>
>
>
> 在 2019-11-28 16:17:07，"taoyunupt"  写道：
>
> Hi,Han
>  Another question. NO COMPACT. If restart a follower , leader
> sender some entries during the  break time, when it has started, if it also
> happend to this problem?  What is the difference between simply restart and
> COMPACT with restart ?
>
>
> Thanks,
> Yun
>
>
>
>
>
>
>
>
> 在 2019-11-28 13:58:36，"taoyunupt"  写道：
>
> Hi,Han
>  Thanks for your reply.  I think maybe we can disconnect the
> failed follower from the Haproxy then synchronize the date, after all
> completed, reconnect it to Haproxy again. But I do not know how to
> synchronize actually.
>  It is just my naive idea. Do you have some suggestion about how
> to fix this problem.  If not very completed, I wii have a try.
>
>
> Thanks
> Yun
>
>
>
>
>
>
> 在 2019-11-28 11:47:55，"Han Zhou"  写道：
>
>
>
> On Wed, Nov 27, 2019 at 7:22 PM taoyunupt  wrote:
> >
> > Hi,
> > My OVN cluster has 3 OVN-northd nodes, They are proxied by Haproxy
> with a VIP. Recently, I restart OVN cluster frequently.  One of the members
> report the logs below.
> > After read the code and paper of RAFT, it seems normal process ,If
> the follower does not find an entry in its log with the same index and
> term, then it refuses the new entries.
> > I think it's reasonable to refuse. But, as we could not control
> Haproxy or some proxy maybe, so it will happen error when an session
> assignate to the failed follower.
> >
> > Does have some means or ways to solve this problem. Maybe we can
> kick off the failed follower or disconnect it from the haproxy then
> synchronize the date ?  Hope to hear your suggestion.
> >
> >
> > 2019-11-27T14:22:17.060Z|00240|raft|INFO|rejecting append_request
> because previous entry 1103,50975 not in local log (mismatch past end of
> log)
> > 2019-11-27T14:22:17.064Z|00241|raft|ERR|Dropped 34 log messages in last
> 12 seconds (most recently, 0 seconds ago) due to excessive rate
> > 2019-11-27T14:22:17.064Z|00242|raft|ERR|internal error: deferred
> append_reply message completed but not ready to send because message index
> 14890 is past last synced index 0: a2b2 append_reply "mismatch past end of
> log": term=1103 log_end=14891 result="inconsistency"
> > 2019-11-27T14:22:17.402Z|00243|raft|INFO|rejecting append_request
> because previous entry 1103,50975 not in local log (mismatch past end of
> log)
> >
> >
> > [root@ovn1 ~]#  ovs-appctl -t /var/run/openvswitch/ovnsb_db.ctl
> cluster/status OVN_Southbound
> > a2b2
> > Name: OVN_Southbound
> > Cluster ID: 4c54 (4c546513-77e3-4602-b211-2e200014ad79)
> > Server ID: a2b2 (a2b2a9c5-cf58-4724-8421-88fd5ca5d94d)
> > Address: tcp:10.254.8.209:6644
> > Status: cluster member
> > Role: leader
> > Term: 1103
> > Leader: self
> > Vote: self
> >
> > Log: [42052, 51009]
> > Entries not yet committed: 0
> > Entries not yet applied: 0
> > Connections: ->beaf ->9a33 <-9a33 <-beaf
> > Servers:
> > a2b2 (a2b2 at tcp:10.254.8.209:6644) (self) next_index=15199
> match_index=51008
> > beaf (beaf at tcp:10.254.8.208:6644) next_index=51009 match_index=0
> > 9a33 (9a33 at tcp:10.254.8.210:6644) next_index=51009
> match_index=51008
>
> >
>
>
> I think it is a bug. I noticed that this problem happens when the cluster
> is restarted after DB compaction. I mentioned it in one of the test cases:
> https://github.com/openvswitch/ovs/blob/master/tests/ovsdb-cluster.at#L252
> I also mentioned another problem related to compaction:
> https://github.com/openvswitch/ovs/blob/master/tests/ovsdb-cluster.at#L239
> I was planning to debug these but didn't get the time yet. I will try to
> find some time next week (it would be great if you could figure it out and
> submit patches).
>
>
>
> Thanks,
> Han
> ___
> 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 ovn] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

[Numan Siddique]

On Mon, Dec 2, 2019 at 1:53 PM Han Zhou  wrote:
>
> On Sun, Dec 1, 2019 at 11:59 PM Numan Siddique  wrote:
> >
> > On Mon, Dec 2, 2019 at 12:44 PM Han Zhou  wrote:
> > >
> > > On Fri, Nov 29, 2019 at 1:08 AM  wrote:
> > > >
> > > > From: Numan Siddique 
> > > >
> > > > If ofctrl_check_and_add_flow(F') is called where flow F' has
> > > match-actions (M, A2)
> > > > and if there already exists a flow F with match-actions (M, A1) in the
> > > desired flow
> > > > table, then this function  doesn't update the existing flow F with new
> > > actions
> > > > actions A2.
> > > >
> > > > This patch fixes it. Presently we don't see any issues because of this
> > > behaviour.
> > > > But it's better to fix it.
> > > >
> > >
> > > Hi Numan, could you explain why do you think the F' should override the
> F?
> > > For my understanding, this is a problem of duplicated logical flows
> > > generated by ovn-northd and can't be solved in ovn-controller. The
> desired
> > > flows have conflict and there is no way to judge which one should be
> > > applied.
> > >
> >
> > Hi Han,
> >
> > I probably should have given the context in which I observed this issue.
> > I am working on making incremental processing for datapath
> additions/deletions.
> >
> > In my testing I observed that the test case -  84: ovn.at:10767
> > ovn -- send gratuitous ARP for NAT rules on HA distributed router
> > is failing 100% of the time.
> >
> > Upon investigation I found that it's failing for below logical flow
> >
> >  table=17(ls_in_l2_lkup  ), priority=80   , match=(eth.src == {
> > 00:00:00:00:ff:01} && (arp.op == 1 || nd_ns)), action=(outport =
> > "_MC_flood"; output;)
> >
> > If you see the test code here -
> > https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10901
> >
> > ovn-nbctl --wait=hv clear logical_switch_port lrp0-rp options
> >
> > When the above command is executed, the logical switch "ls0" is
> > removed from the "local_datapaths" hmap.
> > When ls0 is removed, ovsdb-server sends monitor condition to remove
> > the "Multicast_Group" row ls0.
> >
> > Later when this command is executed  - ovn-nbctl --wait=hv
> > lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router"
> >
> > ovn-controller gets this update from sb ovsdb-server and it adds back
> > ls0 to "local_datapaths". At this point, "Multicast_Group" row
> > for ls0 is empty and the above logical flow  translates to "set:0->reg15".
> >
> > Soon after ovn-controller receives monitor update message to add back
> > the "Multicast_Group" row for ls0.
> > With the patch I am working, calculates logical flows for the logical
> > switch sw0. But it doesn't add the right flow. The action should have
> > been set to "set:0x8000->reg15".
> >
> > The patch I am working on can be found here - [1] -
> >
> https://github.com/numansiddique/ovn/commit/024812e76f4d0628612b1885cca65302a64038c8
> > https://github.com/numansiddique/ovn/tree/lflow_reloaded/v1/p1
> >
> > Please note I am still testing it out and doing some scale testing
> > before submitting the patch for review.
> > [1] adds  a new table - "Datapath_Logical_Flow" in south db which is a
> > weak ref and its references in Datapath_Binding.
> >
> > We can discuss more about the approach later.
> >
> > But I think the proposed patch here makes sense. We don't hit this
> > issue in the present code because when ever any datapath_binding
> > change happens
> > we do full recompute and this flushes out the old  logical flow in the
> > desired_flow_table.
> >
> Hi Numan,
>
> Thanks for explaining the context. The principle in I-P for handling
> changes is always handling deletions first and then creations, and for
> updates, we always delete the old entries and then add the new ones.
> In your context, if a mc_group is updated, we should delete the old flows
> related to that mc_group and then create the new flows. Would the problem
> be solved if we follow this principle?

In principle, yes this would work. But I am not sure how to associate
the logical flow/OF flows
related to mc_group.

The action - "outport = _MC_Flood" (or any other _MC_*) can be used in
any pipeline of the
logical switches/logical routers. I couldn't figure out how to apply
incremental processing
for mc_group changes. The approach I have taken now is to recalculate
flows for only the
relevant datapaths (based on the datapath column of Multicast_Group table).


> In my view, ofctrl_check_and_add_flow() is not the right place for this,
> because it already lose the context whether it is duplicated logical flows
> generated by northd (e.g. conflict ACLs, or bugs), or it is just transient
> status during ovn-controller processing, which is the case you encountered.
>

Suppose if CMS has added below ACLs

match - "tcp.src > 0 && tcp.dst < 34555" action = "drop"
match - "tcp.src > 0 && tcp.dst < 34555" action = "allow"

In the present code, we log the 2nd flow as duplicate and ignore it.
In the proposed patch, we override the first flow with the 2nd on

[ovs-dev] [PATCH V2 07/19] netdev-offload-dpdk: Introduce flow flush callback

[Eli Britstein]

Introduce flow flush callback for dpdk offloaded flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 6e1ca8a0d..64873759d 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -307,6 +307,21 @@ netdev_offload_dpdk_flow_del(struct netdev *netdev, const 
ovs_u128 *ufid,
 return netdev_offload_dpdk_destroy_flow(netdev, ufid, rte_flow);
 }
 
+static int
+netdev_offload_dpdk_flow_flush(struct netdev *netdev)
+{
+struct rte_flow_error error;
+int ret;
+
+ret = netdev_dpdk_rte_flow_flush(netdev, &error);
+if (ret) {
+VLOG_ERR("%s: rte flow flush error: %u : message : %s\n",
+ netdev_get_name(netdev), error.type, error.message);
+}
+
+return ret;
+}
+
 static int
 netdev_offload_dpdk_init_flow_api(struct netdev *netdev)
 {
@@ -315,6 +330,7 @@ netdev_offload_dpdk_init_flow_api(struct netdev *netdev)
 
 const struct netdev_flow_api netdev_offload_dpdk = {
 .type = "dpdk_flow_api",
+.flow_flush = netdev_offload_dpdk_flow_flush,
 .flow_put = netdev_offload_dpdk_flow_put,
 .flow_del = netdev_offload_dpdk_flow_del,
 .init_flow_api = netdev_offload_dpdk_init_flow_api,
-- 
2.14.5

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

[ovs-dev] [PATCH V2 09/19] netdev-dpdk: Introduce rte flow query function

[Eli Britstein]

Introduce a rte flow query function as a pre-step towards reading HW
statistics of fully offloaded flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-dpdk.c | 25 +
 lib/netdev-dpdk.h |  6 ++
 2 files changed, 31 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 06eca848c..c891d3ed9 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4505,6 +4505,31 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 return flow;
 }
 
+int
+netdev_dpdk_rte_flow_query(struct netdev *netdev,
+   struct rte_flow *rte_flow,
+   struct rte_flow_query_count *query,
+   struct rte_flow_error *error)
+{
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+struct rte_flow_action_count count = {};
+const struct rte_flow_action actions[] = {
+{
+.type = RTE_FLOW_ACTION_TYPE_COUNT,
+.conf = &count,
+},
+{
+.type = RTE_FLOW_ACTION_TYPE_END,
+},
+};
+int ret;
+
+ovs_mutex_lock(&dev->mutex);
+ret = rte_flow_query(dev->port_id, rte_flow, actions, query, error);
+ovs_mutex_unlock(&dev->mutex);
+return ret;
+}
+
 int
 netdev_dpdk_rte_flow_flush(struct netdev *netdev,
struct rte_flow_error *error)
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 960aec7a8..8e79bcdf8 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -31,6 +31,7 @@ struct rte_flow_error;
 struct rte_flow_attr;
 struct rte_flow_item;
 struct rte_flow_action;
+struct rte_flow_query_count;
 
 void netdev_dpdk_register(void);
 void free_dpdk_buf(struct dp_packet *);
@@ -50,6 +51,11 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 int
 netdev_dpdk_rte_flow_flush(struct netdev *netdev,
struct rte_flow_error *error);
+int
+netdev_dpdk_rte_flow_query(struct netdev *netdev,
+   struct rte_flow *rte_flow,
+   struct rte_flow_query_count *query,
+   struct rte_flow_error *error);
 
 #else
 
-- 
2.14.5

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

[ovs-dev] [PATCH V2 11/19] dpif-netdev: Read hw stats during flow_dump_next() call

[Eli Britstein]

From: Ophir Munk 

Use netdev dump flow next API in order to update the statistics of fully
offloaded flows.

Co-authored-by: Eli Britstein 
Signed-off-by: Ophir Munk 
Reviewed-by: Oz Shlomo 
Signed-off-by: Eli Britstein 
---
 lib/dpif-netdev.c | 42 --
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5142bad1d..bfeb1e7b0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -527,6 +527,7 @@ struct dp_netdev_flow {
 
 bool dead;
 uint32_t mark;   /* Unique flow mark assigned to a flow */
+bool actions_offloaded;  /* true if flow is fully offloaded */
 
 /* Statistics. */
 struct dp_netdev_flow_stats stats;
@@ -2410,6 +2411,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item 
*offload)
 }
 }
 info.flow_mark = mark;
+info.actions_offloaded = &flow->actions_offloaded;
 
 ovs_mutex_lock(&pmd->dp->port_mutex);
 port = dp_netdev_lookup_port(pmd->dp, in_port);
@@ -3073,8 +3075,8 @@ dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow 
*netdev_flow,
 flow->pmd_id = netdev_flow->pmd_id;
 get_dpif_flow_stats(netdev_flow, &flow->stats);
 
-flow->attrs.offloaded = false;
-flow->attrs.dp_layer = "ovs";
+flow->attrs.offloaded = netdev_flow->actions_offloaded;
+flow->attrs.dp_layer = flow->attrs.offloaded ? "in_hw" : "ovs";
 }
 
 static int
@@ -3244,6 +3246,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
 flow->dead = false;
 flow->batch = NULL;
 flow->mark = INVALID_FLOW_MARK;
+flow->actions_offloaded = false;
 *CONST_CAST(unsigned *, &flow->pmd_id) = pmd->core_id;
 *CONST_CAST(struct flow *, &flow->flow) = match->flow;
 *CONST_CAST(ovs_u128 *, &flow->ufid) = *ufid;
@@ -3598,6 +3601,37 @@ dpif_netdev_flow_dump_thread_destroy(struct 
dpif_flow_dump_thread *thread_)
 free(thread);
 }
 
+static int
+dpif_netdev_offload_used(struct dp_netdev_flow *netdev_flow,
+ struct dp_netdev_pmd_thread *pmd)
+{
+struct netdev_flow_dump netdev_dump;
+struct dpif_flow_stats stats;
+ovs_u128 ufid;
+bool has_next;
+
+netdev_dump.port = netdev_flow->flow.in_port.odp_port;
+netdev_dump.netdev = netdev_ports_get(netdev_dump.port, pmd->dp->class);
+if (!netdev_dump.netdev) {
+return -1;
+}
+/* get offloaded stats */
+ufid = netdev_flow->mega_ufid;
+has_next = netdev_flow_dump_next(&netdev_dump, NULL, NULL, &stats, NULL,
+ &ufid, NULL, NULL);
+netdev_close(netdev_dump.netdev);
+if (!has_next) {
+return -1;
+}
+if (stats.n_packets) {
+atomic_store_relaxed(&netdev_flow->stats.used, pmd->ctx.now / 1000);
+non_atomic_ullong_add(&netdev_flow->stats.packet_count, 
stats.n_packets);
+non_atomic_ullong_add(&netdev_flow->stats.byte_count, stats.n_bytes);
+}
+
+return 0;
+}
+
 static int
 dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread *thread_,
struct dpif_flow *flows, int max_flows)
@@ -3638,6 +3672,10 @@ dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread 
*thread_,
 netdev_flows[n_flows] = CONTAINER_OF(node,
  struct dp_netdev_flow,
  node);
+/* Read hardware stats in case of hardware offload */
+if (netdev_flows[n_flows]->actions_offloaded) {
+dpif_netdev_offload_used(netdev_flows[n_flows], pmd);
+}
 }
 /* When finishing dumping the current pmd thread, moves to
  * the next. */
-- 
2.14.5

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

[ovs-dev] [PATCH V2 06/19] netdev-dpdk: Introduce flow_flush method

[Eli Britstein]

Introduce this method to orderly flush the rules when upper layers
request it.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-dpdk.c | 13 +
 lib/netdev-dpdk.h |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5fea08301..06eca848c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4505,6 +4505,19 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 return flow;
 }
 
+int
+netdev_dpdk_rte_flow_flush(struct netdev *netdev,
+   struct rte_flow_error *error)
+{
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+int ret;
+
+ovs_mutex_lock(&dev->mutex);
+ret = rte_flow_flush(dev->port_id, error);
+ovs_mutex_unlock(&dev->mutex);
+return ret;
+}
+
 #define NETDEV_DPDK_CLASS_COMMON\
 .is_pmd = true, \
 .alloc = netdev_dpdk_alloc, \
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 60631c4f0..960aec7a8 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -47,6 +47,9 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 const struct rte_flow_item *items,
 const struct rte_flow_action *actions,
 struct rte_flow_error *error);
+int
+netdev_dpdk_rte_flow_flush(struct netdev *netdev,
+   struct rte_flow_error *error);
 
 #else
 
-- 
2.14.5

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

[ovs-dev] [PATCH V2 10/19] netdev-offload-dpdk: Implement flow dump next method

[Eli Britstein]

Implement the flow dump next method for DPDK, to get the statistics of
the provided ufid, towards reading statistics of fully offloaded flows.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 8561a9b77..0f9c51aa7 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -375,10 +375,46 @@ netdev_offload_dpdk_init_flow_api(struct netdev *netdev)
 return netdev_dpdk_flow_api_supported(netdev) ? 0 : EOPNOTSUPP;
 }
 
+static bool
+netdev_offload_dpdk_flow_dump_next(struct netdev_flow_dump *dump,
+   struct match *match OVS_UNUSED,
+   struct nlattr **actions OVS_UNUSED,
+   struct dpif_flow_stats *stats,
+   struct dpif_flow_attrs *attrs OVS_UNUSED,
+   ovs_u128 *ufid,
+   struct ofpbuf *rbuffer OVS_UNUSED,
+   struct ofpbuf *wbuffer OVS_UNUSED)
+{
+struct rte_flow_query_count query = { .reset = 1 };
+struct rte_flow_error error;
+struct rte_flow *rte_flow;
+int ret;
+
+rte_flow = ufid_to_rte_flow_find(ufid);
+if (!rte_flow) {
+return false;
+}
+
+memset(stats, 0, sizeof *stats);
+ret = netdev_dpdk_rte_flow_query(dump->netdev, rte_flow, &query, &error);
+if (ret) {
+VLOG_DBG("ufid "UUID_FMT
+ " flow %p query for '%s' failed\n",
+ UUID_ARGS((struct uuid *)ufid), rte_flow,
+ netdev_get_name(dump->netdev));
+return false;
+}
+stats->n_packets += (query.hits_set) ? query.hits : 0;
+stats->n_bytes += (query.bytes_set) ? query.bytes : 0;
+
+return true;
+}
+
 const struct netdev_flow_api netdev_offload_dpdk = {
 .type = "dpdk_flow_api",
 .flow_flush = netdev_offload_dpdk_flow_flush,
 .flow_put = netdev_offload_dpdk_flow_put,
 .flow_del = netdev_offload_dpdk_flow_del,
 .init_flow_api = netdev_offload_dpdk_init_flow_api,
+.flow_dump_next = netdev_offload_dpdk_flow_dump_next,
 };
-- 
2.14.5

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

[ovs-dev] [PATCH V2 00/19] netdev datapath actions offload

[Eli Britstein]

Currently, netdev datapath offload only accelerates the flow match
sequence by associating a mark per flow. This series introduces the full
offload of netdev datapath flows by having the HW also perform the flow
actions.

This series adds HW offload for output, drop, set MAC, set IPv4, set
TCP/UDP ports and tunnel push actions using the DPDK rte_flow API.

v1 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/614511472
v2 Travis passed: https://travis-ci.org/elibritstein/OVS/builds/619262148

v1-v2:
- fallback to mark/rss in case of any failure of action offload.
- dp_layer changed to "in_hw" in case the flow is offloaded
- using netdev_ports_get instead of dp_netdev_lookup_port in stats reading
- using flow_dump_next API instead of a new API
- validity checks for last action of output and clone
- count action should be done for drop as well
- validity check for output action
- added doc in Documentation/howto/dpdk.rst

Eli Britstein (18):
  sparse: rte_flow.h: Adapt according to DPDK 18.11.2
  netdev-offload-dpdk: Refactor flow patterns and actions code
  netdev-offload-dpdk-flow: Dynamically allocate pattern items
  netdev-offload-dpdk: Fix typo
  netdev-dpdk: Improve HW offload flow debuggability
  netdev-dpdk: Introduce flow_flush method
  netdev-offload-dpdk: Introduce flow flush callback
  netdev-offload-dpdk: Framework for actions offload
  netdev-dpdk: Introduce rte flow query function
  netdev-offload-dpdk: Implement flow dump next method
  dpif-netdev: Populate dpif class field in offload struct
  netdev-dpdk: Getter function for dpdk port id API
  netdev-offload-dpdk-flow: Support offload of output action
  netdev-offload-dpdk-flow: Support offload of drop action
  netdev-offload-dpdk-flow: Support offload of set MAC actions
  netdev-offload-dpdk-flow: Support offload of set IPv4 actions
  netdev-offload-dpdk-flow: Support offload of clone tnl_push/output
actions
  netdev-offload-dpdk-flow: Support offload of set TCP/UDP ports actions

Ophir Munk (1):
  dpif-netdev: Read hw stats during flow_dump_next() call

 Documentation/howto/dpdk.rst  |   8 +-
 NEWS  |   2 +
 include/sparse/rte_flow.h | 810 ++---
 lib/automake.mk   |   4 +-
 lib/dpif-netdev.c |  44 +-
 lib/netdev-dpdk.c |  76 
 lib/netdev-dpdk.h |  11 +
 lib/netdev-offload-dpdk-flow.c| 922 ++
 lib/netdev-offload-dpdk-private.h |  65 +++
 lib/netdev-offload-dpdk.c | 575 +---
 lib/netdev-offload.h  |   1 +
 11 files changed, 1999 insertions(+), 519 deletions(-)
 create mode 100644 lib/netdev-offload-dpdk-flow.c
 create mode 100644 lib/netdev-offload-dpdk-private.h

-- 
2.14.5

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

[ovs-dev] [PATCH V2 01/19] sparse: rte_flow.h: Adapt according to DPDK 18.11.2

[Eli Britstein]

In dpdk-latest branch, this file was removed. This patch is temporary,
should not be merged, and only exist to pass sparse travis tests.

Signed-off-by: Eli Britstein 
---
 include/sparse/rte_flow.h | 810 +++---
 1 file changed, 759 insertions(+), 51 deletions(-)

diff --git a/include/sparse/rte_flow.h b/include/sparse/rte_flow.h
index 02fa523b4..0c55e68cd 100644
--- a/include/sparse/rte_flow.h
+++ b/include/sparse/rte_flow.h
@@ -64,18 +64,20 @@ extern "C" {
 /**
  * Flow rule attributes.
  *
- * Priorities are set on two levels: per group and per rule within groups.
+ * Priorities are set on a per rule based within groups.
  *
- * Lower values denote higher priority, the highest priority for both levels
- * is 0, so that a rule with priority 0 in group 8 is always matched after a
- * rule with priority 8 in group 0.
+ * Lower values denote higher priority, the highest priority for a flow rule
+ * is 0, so that a flow that matches for than one rule, the rule with the
+ * lowest priority value will always be matched.
  *
  * Although optional, applications are encouraged to group similar rules as
  * much as possible to fully take advantage of hardware capabilities
  * (e.g. optimized matching) and work around limitations (e.g. a single
- * pattern type possibly allowed in a given group).
+ * pattern type possibly allowed in a given group). Applications should be
+ * aware that groups are not linked by default, and that they must be
+ * explicitly linked by the application using the JUMP action.
  *
- * Group and priority levels are arbitrary and up to the application, they
+ * Priority levels are arbitrary and up to the application, they
  * do not need to be contiguous nor start from 0, however the maximum number
  * varies between devices and may be affected by existing flow rules.
  *
@@ -98,10 +100,29 @@ extern "C" {
  */
 struct rte_flow_attr {
uint32_t group; /**< Priority group. */
-   uint32_t priority; /**< Priority level within group. */
+   uint32_t priority; /**< Rule priority level within group. */
uint32_t ingress:1; /**< Rule applies to ingress traffic. */
uint32_t egress:1; /**< Rule applies to egress traffic. */
-   uint32_t reserved:30; /**< Reserved, must be zero. */
+   /**
+* Instead of simply matching the properties of traffic as it would
+* appear on a given DPDK port ID, enabling this attribute transfers
+* a flow rule to the lowest possible level of any device endpoints
+* found in the pattern.
+*
+* When supported, this effectively enables an application to
+* re-route traffic not necessarily intended for it (e.g. coming
+* from or addressed to different physical ports, VFs or
+* applications) at the device level.
+*
+* It complements the behavior of some pattern items such as
+* RTE_FLOW_ITEM_TYPE_PHY_PORT and is meaningless without them.
+*
+* When transferring flow rules, ingress and egress attributes keep
+* their original meaning, as if processing traffic emitted or
+* received by the application.
+*/
+   uint32_t transfer:1;
+   uint32_t reserved:29; /**< Reserved, must be zero. */
 };
 
 /**
@@ -109,15 +130,13 @@ struct rte_flow_attr {
  *
  * Pattern items fall in two categories:
  *
- * - Matching protocol headers and packet data (ANY, RAW, ETH, VLAN, IPV4,
- *   IPV6, ICMP, UDP, TCP, SCTP, VXLAN and so on), usually associated with a
+ * - Matching protocol headers and packet data, usually associated with a
  *   specification structure. These must be stacked in the same order as the
- *   protocol layers to match, starting from the lowest.
+ *   protocol layers to match inside packets, starting from the lowest.
  *
- * - Matching meta-data or affecting pattern processing (END, VOID, INVERT,
- *   PF, VF, PORT and so on), often without a specification structure. Since
- *   they do not match packet contents, these can be specified anywhere
- *   within item lists without affecting others.
+ * - Matching meta-data or affecting pattern processing, often without a
+ *   specification structure. Since they do not match packet contents, their
+ *   position in the list is usually not relevant.
  *
  * See the description of individual types for more information. Those
  * marked with [META] fall into the second category.
@@ -164,13 +183,8 @@ enum rte_flow_item_type {
/**
 * [META]
 *
-* Matches packets addressed to the physical function of the device.
-*
-* If the underlying device function differs from the one that would
-* normally receive the matched traffic, specifying this item
-* prevents it from reaching that device unless the flow rule
-* contains a PF action. Packets are not duplicated between device
-* instances by default.
+* Matches traffic originating from (

[ovs-dev] [PATCH V2 03/19] netdev-offload-dpdk-flow: Dynamically allocate pattern items

[Eli Britstein]

Instead of statically allocated pattern items on the stack, dynamically
allocate only the required items while parsing the required matches, to
simplify the parsing and make it self-contained, without need of
external types.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk-flow.c| 163 +-
 lib/netdev-offload-dpdk-private.h |  14 
 lib/netdev-offload-dpdk.c |   6 +-
 3 files changed, 90 insertions(+), 93 deletions(-)

diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index d1d5ce2c6..c8c3e28ea 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -29,6 +29,16 @@ void
 netdev_dpdk_flow_patterns_free(struct flow_patterns *patterns)
 {
 /* When calling this function 'patterns' must be valid */
+int i;
+
+for (i = 0; i < patterns->cnt; i++) {
+if (patterns->items[i].spec) {
+free((void *)patterns->items[i].spec);
+}
+if (patterns->items[i].mask) {
+free((void *)patterns->items[i].mask);
+}
+}
 free(patterns->items);
 patterns->items = NULL;
 patterns->cnt = 0;
@@ -333,8 +343,6 @@ netdev_dpdk_flow_actions_add_mark_rss(struct flow_actions 
*actions,
 
 int
 netdev_dpdk_flow_patterns_add(struct flow_patterns *patterns,
-  struct flow_pattern_items *spec,
-  struct flow_pattern_items *mask,
   const struct match *match)
 {
 uint8_t proto = 0;
@@ -342,16 +350,20 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns 
*patterns,
 /* Eth */
 if (!eth_addr_is_zero(match->wc.masks.dl_src) ||
 !eth_addr_is_zero(match->wc.masks.dl_dst)) {
-memcpy(&spec->eth.dst, &match->flow.dl_dst, sizeof spec->eth.dst);
-memcpy(&spec->eth.src, &match->flow.dl_src, sizeof spec->eth.src);
-spec->eth.type = match->flow.dl_type;
+struct rte_flow_item_eth *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
 
-memcpy(&mask->eth.dst, &match->wc.masks.dl_dst, sizeof mask->eth.dst);
-memcpy(&mask->eth.src, &match->wc.masks.dl_src, sizeof mask->eth.src);
-mask->eth.type = match->wc.masks.dl_type;
+memcpy(&spec->dst, &match->flow.dl_dst, sizeof spec->dst);
+memcpy(&spec->src, &match->flow.dl_src, sizeof spec->src);
+spec->type = match->flow.dl_type;
 
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH,
- &spec->eth, &mask->eth);
+memcpy(&mask->dst, &match->wc.masks.dl_dst, sizeof mask->dst);
+memcpy(&mask->src, &match->wc.masks.dl_src, sizeof mask->src);
+mask->type = match->wc.masks.dl_type;
+
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ETH, spec, mask);
 } else {
 /*
  * If user specifies a flow (like UDP flow) without L2 patterns,
@@ -365,36 +377,43 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns 
*patterns,
 
 /* VLAN */
 if (match->wc.masks.vlans[0].tci && match->flow.vlans[0].tci) {
-spec->vlan.tci  = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
-mask->vlan.tci  = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
+struct rte_flow_item_vlan *spec, *mask;
+
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
+
+spec->tci = match->flow.vlans[0].tci & ~htons(VLAN_CFI);
+mask->tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI);
 
 /* Match any protocols. */
-mask->vlan.inner_type = 0;
+mask->inner_type = 0;
 
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN,
- &spec->vlan, &mask->vlan);
+add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_VLAN, spec, mask);
 }
 
 /* IP v4 */
 if (match->flow.dl_type == htons(ETH_TYPE_IP)) {
-spec->ipv4.hdr.type_of_service = match->flow.nw_tos;
-spec->ipv4.hdr.time_to_live= match->flow.nw_ttl;
-spec->ipv4.hdr.next_proto_id   = match->flow.nw_proto;
-spec->ipv4.hdr.src_addr= match->flow.nw_src;
-spec->ipv4.hdr.dst_addr= match->flow.nw_dst;
+struct rte_flow_item_ipv4 *spec, *mask;
 
-mask->ipv4.hdr.type_of_service = match->wc.masks.nw_tos;
-mask->ipv4.hdr.time_to_live= match->wc.masks.nw_ttl;
-mask->ipv4.hdr.next_proto_id   = match->wc.masks.nw_proto;
-mask->ipv4.hdr.src_addr= match->wc.masks.nw_src;
-mask->ipv4.hdr.dst_addr= match->wc.masks.nw_dst;
+spec = xzalloc(sizeof *spec);
+mask = xzalloc(sizeof *mask);
 
-add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_IPV4,
- &spec->ipv4, &mask->ipv4);
+spec->hdr.type_of_service = match->flow.nw_tos;
+spec->hdr.time_to_live= match->flow.nw_ttl;
+spec->hdr.next_proto_id   = match->flow.nw_proto;

[ovs-dev] [PATCH V2 04/19] netdev-offload-dpdk: Fix typo

[Eli Britstein]

Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 9882e1d23..6e1ca8a0d 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -150,7 +150,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
actions.actions, &error);
 
 if (!flow) {
-VLOG_ERR("%s: rte flow creat error: %u : message : %s\n",
+VLOG_ERR("%s: rte flow create error: %u : message : %s\n",
  netdev_get_name(netdev), error.type, error.message);
 ret = -1;
 goto out;
-- 
2.14.5

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

[ovs-dev] [PATCH V2 02/19] netdev-offload-dpdk: Refactor flow patterns and actions code

[Eli Britstein]

Refactor the flow patterns and actions code to a new source file for
better readability and towards adding more code to it.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/automake.mk   |   4 +-
 lib/netdev-offload-dpdk-flow.c| 479 ++
 lib/netdev-offload-dpdk-private.h |  69 ++
 lib/netdev-offload-dpdk.c | 466 +---
 4 files changed, 562 insertions(+), 456 deletions(-)
 create mode 100644 lib/netdev-offload-dpdk-flow.c
 create mode 100644 lib/netdev-offload-dpdk-private.h

diff --git a/lib/automake.mk b/lib/automake.mk
index 17b36b43d..3a813af85 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -142,6 +142,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/netdev-dummy.c \
lib/netdev-offload.c \
lib/netdev-offload.h \
+   lib/netdev-offload-dpdk-private.h \
lib/netdev-offload-provider.h \
lib/netdev-provider.h \
lib/netdev-vport.c \
@@ -426,7 +427,8 @@ if DPDK_NETDEV
 lib_libopenvswitch_la_SOURCES += \
lib/dpdk.c \
lib/netdev-dpdk.c \
-   lib/netdev-offload-dpdk.c
+   lib/netdev-offload-dpdk.c \
+   lib/netdev-offload-dpdk-flow.c
 else
 lib_libopenvswitch_la_SOURCES += \
lib/dpdk-stub.c
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
new file mode 100644
index 0..d1d5ce2c6
--- /dev/null
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -0,0 +1,479 @@
+/*
+ * Copyright (c) 2019 Mellanox Technologies, Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include 
+
+#include 
+
+#include "dpif-netdev.h"
+#include "netdev-offload-dpdk-private.h"
+#include "openvswitch/match.h"
+#include "openvswitch/vlog.h"
+#include "packets.h"
+
+VLOG_DEFINE_THIS_MODULE(netdev_offload_dpdk_flow);
+
+void
+netdev_dpdk_flow_patterns_free(struct flow_patterns *patterns)
+{
+/* When calling this function 'patterns' must be valid */
+free(patterns->items);
+patterns->items = NULL;
+patterns->cnt = 0;
+}
+
+void
+netdev_dpdk_flow_actions_free(struct flow_actions *actions)
+{
+/* When calling this function 'actions' must be valid */
+int i;
+
+for (i = 0; i < actions->cnt; i++) {
+if (actions->actions[i].conf) {
+free((void *)actions->actions[i].conf);
+}
+}
+free(actions->actions);
+actions->actions = NULL;
+actions->cnt = 0;
+}
+
+static void
+dump_flow_pattern(struct rte_flow_item *item)
+{
+struct ds s;
+
+if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
+return;
+}
+
+ds_init(&s);
+
+if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
+const struct rte_flow_item_eth *eth_spec = item->spec;
+const struct rte_flow_item_eth *eth_mask = item->mask;
+
+ds_put_cstr(&s, "rte flow eth pattern:\n");
+if (eth_spec) {
+ds_put_format(&s,
+  "  Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
+  "type=0x%04" PRIx16"\n",
+  ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
+  ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
+  ntohs(eth_spec->type));
+} else {
+ds_put_cstr(&s, "  Spec = null\n");
+}
+if (eth_mask) {
+ds_put_format(&s,
+  "  Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
+  "type=0x%04"PRIx16"\n",
+  ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
+  ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
+  ntohs(eth_mask->type));
+} else {
+ds_put_cstr(&s, "  Mask = null\n");
+}
+}
+
+if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
+const struct rte_flow_item_vlan *vlan_spec = item->spec;
+const struct rte_flow_item_vlan *vlan_mask = item->mask;
+
+ds_put_cstr(&s, "rte flow vlan pattern:\n");
+if (vlan_spec) {
+ds_put_format(&s,
+  "  Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n",
+  ntohs(vlan_spec->inner_type), ntohs(vlan_spec->tci));
+} else {
+ds_put_cstr(&s, "  Spec = null\n");
+}
+
+if (vlan_mask) {
+ds_put_format(&s,
+  "  Mask: inner_type=0x%"PRIx16", tci=

[ovs-dev] [PATCH V2 14/19] netdev-offload-dpdk-flow: Support offload of output action

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 Documentation/howto/dpdk.rst   |  8 ++--
 NEWS   |  1 +
 lib/netdev-offload-dpdk-flow.c | 91 --
 3 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 766a7950c..de779d78f 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -370,8 +370,10 @@ The flow hardware offload is disabled by default and can 
be enabled by::
 
 $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
 
-So far only partial flow offload is implemented. Moreover, it only works
-with PMD drivers have the rte_flow action "MARK + RSS" support.
+Matches and actions are programmed into HW to achieve full offload of
+the flow. If not all actions are supported, fallback to partial flow
+offload (offloading matches only). Moreover, it only works with PMD
+drivers that have the relevant rte_flow actions support.
 
 The validated NICs are:
 
@@ -380,7 +382,7 @@ The validated NICs are:
 
 Supported protocols for hardware offload are:
 - L2: Ethernet, VLAN
-- L3: IPv4, IPv6
+- L3: IPv4
 - L4: TCP, UDP, SCTP, ICMP
 
 Further Reading
diff --git a/NEWS b/NEWS
index 80b1a..33882d2af 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
+ * Add hardware offload support for output actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index b03ce2c41..b633bc129 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_cstr(s, "  RSS = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
+const struct rte_flow_action_count *count = actions->conf;
+
+ds_put_cstr(s, "rte flow count action:\n");
+if (count) {
+ds_put_format(s,
+  "  Count: shared=%d, id=%d\n",
+  count->shared, count->id);
+} else {
+ds_put_cstr(s, "  Count = null\n");
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
+const struct rte_flow_action_port_id *port_id = actions->conf;
+
+ds_put_cstr(s, "rte flow port-id action:\n");
+if (port_id) {
+ds_put_format(s,
+  "  Port-id: original=%d, id=%d\n",
+  port_id->original, port_id->id);
+} else {
+ds_put_cstr(s, "  Port-id = null\n");
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -531,19 +553,80 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns 
*patterns,
 return 0;
 }
 
+static void
+netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
+{
+struct rte_flow_action_count *count = xzalloc(sizeof *count);
+
+count->shared = 0;
+/* Each flow has a single count action, so no need of id */
+count->id = 0;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
+}
+
+static int
+netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
+struct netdev *outdev)
+{
+struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
+int outdev_id;
+
+outdev_id = netdev_dpdk_get_port_id(outdev);
+if (outdev_id < 0) {
+return -1;
+}
+port_id->id = outdev_id;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+return 0;
+}
+
+static int
+netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
+   const struct nlattr *nla,
+   struct offload_info *info)
+{
+struct netdev *outdev;
+odp_port_t port;
+int ret = 0;
+
+port = nl_attr_get_odp_port(nla);
+outdev = netdev_ports_get(port, info->dpif_class);
+if (outdev == NULL) {
+VLOG_DBG_RL(&error_rl,
+"Cannot find netdev for odp port %d", port);
+return -1;
+}
+if (netdev_dpdk_flow_add_port_id_action(actions, outdev)) {
+VLOG_DBG_RL(&error_rl,
+"Output to %s cannot be offloaded",
+netdev_get_name(outdev));
+ret = -1;
+}
+netdev_close(outdev);
+return ret;
+}
+
 int
 netdev_dpdk_flow_actions_add(struct flow_actions *actions,
  struct nlattr *nl_actions,
  size_t nl_actions_len,
- struct offload_info *info OVS_UNUSED)
+ struct offload_info *info)
 {
 struct nlattr *nla;

[ovs-dev] [PATCH V2 05/19] netdev-dpdk: Improve HW offload flow debuggability

[Eli Britstein]

Add debug prints when creating and destroying rte flows, with all the
flow details (attributes, patterns, actions).

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-dpdk.c |  29 +++
 lib/netdev-offload-dpdk-flow.c| 168 +++---
 lib/netdev-offload-dpdk-private.h |   5 ++
 3 files changed, 136 insertions(+), 66 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4c9f122b0..5fea08301 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -67,6 +67,7 @@
 #include "unixctl.h"
 #include "util.h"
 #include "uuid.h"
+#include "netdev-offload-dpdk-private.h"
 
 enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 
@@ -4456,6 +4457,15 @@ netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
 
 ovs_mutex_lock(&dev->mutex);
 ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+if (!ret) {
+VLOG_DBG("Destroy rte_flow %p: netdev=%s, port=%d\n",
+ rte_flow, netdev_get_name(netdev), dev->port_id);
+} else {
+VLOG_ERR("Destroy rte_flow %p: netdev=%s, port=%d\n"
+ "FAILED. error %u : message : %s",
+ rte_flow, netdev_get_name(netdev), dev->port_id,
+ error->type, error->message);
+}
 ovs_mutex_unlock(&dev->mutex);
 return ret;
 }
@@ -4469,9 +4479,28 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
 {
 struct rte_flow *flow;
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+struct ds s;
 
 ovs_mutex_lock(&dev->mutex);
 flow = rte_flow_create(dev->port_id, attr, items, actions, error);
+ds_init(&s);
+if (flow) {
+VLOG_DBG("Create rte_flow: netdev=%s, port=%d\n"
+ "%s"
+ "Flow handle=%p\n",
+ netdev_get_name(netdev), dev->port_id,
+ ds_cstr(netdev_dpdk_flow_ds_put_flow(&s, attr, items,
+  actions)), flow);
+} else {
+VLOG_ERR("Create rte_flow: netdev=%s, port=%d\n"
+ "%s"
+ "FAILED. error %u : message : %s\n",
+ netdev_get_name(netdev), dev->port_id,
+ ds_cstr(netdev_dpdk_flow_ds_put_flow(&s, attr, items,
+  actions)),
+ error->type, error->message);
+}
+ds_destroy(&s);
 ovs_mutex_unlock(&dev->mutex);
 return flow;
 }
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index c8c3e28ea..19c933932 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -61,73 +61,71 @@ netdev_dpdk_flow_actions_free(struct flow_actions *actions)
 }
 
 static void
-dump_flow_pattern(struct rte_flow_item *item)
+ds_put_flow_attr(struct ds *s, const struct rte_flow_attr *attr)
 {
-struct ds s;
-
-if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
-return;
-}
-
-ds_init(&s);
+ds_put_format(s,
+  "  Attributes: "
+  "ingress=%d, egress=%d, prio=%d, group=%d, transfer=%d\n",
+  attr->ingress, attr->egress, attr->priority, attr->group,
+  attr->transfer);
+}
 
+static void
+ds_put_flow_pattern(struct ds *s, const struct rte_flow_item *item)
+{
 if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
 const struct rte_flow_item_eth *eth_spec = item->spec;
 const struct rte_flow_item_eth *eth_mask = item->mask;
 
-ds_put_cstr(&s, "rte flow eth pattern:\n");
+ds_put_cstr(s, "rte flow eth pattern:\n");
 if (eth_spec) {
-ds_put_format(&s,
+ds_put_format(s,
   "  Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
   "type=0x%04" PRIx16"\n",
   ETH_ADDR_BYTES_ARGS(eth_spec->src.addr_bytes),
   ETH_ADDR_BYTES_ARGS(eth_spec->dst.addr_bytes),
   ntohs(eth_spec->type));
 } else {
-ds_put_cstr(&s, "  Spec = null\n");
+ds_put_cstr(s, "  Spec = null\n");
 }
 if (eth_mask) {
-ds_put_format(&s,
+ds_put_format(s,
   "  Mask: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
   "type=0x%04"PRIx16"\n",
   ETH_ADDR_BYTES_ARGS(eth_mask->src.addr_bytes),
   ETH_ADDR_BYTES_ARGS(eth_mask->dst.addr_bytes),
   ntohs(eth_mask->type));
 } else {
-ds_put_cstr(&s, "  Mask = null\n");
+ds_put_cstr(s, "  Mask = null\n");
 }
-}
-
-if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
+} else if (item->type == RTE_FLOW_ITEM_TYPE_VLAN) {
 const struct rte_flow_item_vlan *vlan_spec = item->spec;
 const struct rte_flow_item_vlan *vlan_mask = item->mask;
 
-ds_put_cstr(&s, "rte flow vlan pattern:\n");
+

[ovs-dev] [PATCH V2 12/19] dpif-netdev: Populate dpif class field in offload struct

[Eli Britstein]

Populate dpif class field in offload struct to be used in offloading
flow put.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/dpif-netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bfeb1e7b0..22f032ab0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2375,6 +2375,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item 
*offload)
 {
 struct dp_netdev_port *port;
 struct dp_netdev_pmd_thread *pmd = offload->pmd;
+const struct dpif_class *dpif_class = pmd->dp->class;
 struct dp_netdev_flow *flow = offload->flow;
 odp_port_t in_port = flow->flow.in_port.odp_port;
 bool modification = offload->op == DP_NETDEV_FLOW_OFFLOAD_OP_MOD;
@@ -2412,6 +2413,7 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item 
*offload)
 }
 info.flow_mark = mark;
 info.actions_offloaded = &flow->actions_offloaded;
+info.dpif_class = dpif_class;
 
 ovs_mutex_lock(&pmd->dp->port_mutex);
 port = dp_netdev_lookup_port(pmd->dp, in_port);
-- 
2.14.5

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

[ovs-dev] [PATCH V2 15/19] netdev-offload-dpdk-flow: Support offload of drop action

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 NEWS   | 2 +-
 lib/netdev-offload-dpdk-flow.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 33882d2af..45eef591f 100644
--- a/NEWS
+++ b/NEWS
@@ -26,7 +26,7 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
- * Add hardware offload support for output actions (experimental).
+ * Add hardware offload support for output and drop actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index b633bc129..a73d9522d 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -296,6 +296,8 @@ ds_put_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_cstr(s, "  Port-id = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
+ds_put_cstr(s, "rte flow drop action\n");
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -630,9 +632,7 @@ netdev_dpdk_flow_actions_add(struct flow_actions *actions,
 }
 
 if (nl_actions_len == 0) {
-VLOG_DBG_RL(&error_rl,
-"Unsupported action type drop");
-return -1;
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_DROP, NULL);
 }
 
 add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
-- 
2.14.5

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

[ovs-dev] [PATCH V2 18/19] netdev-offload-dpdk-flow: Support offload of clone tnl_push/output actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 NEWS   |  4 +--
 lib/netdev-offload-dpdk-flow.c | 61 ++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 19fe2b72b..2ccc19b90 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
- * Add hardware offload support for output, drop and set actions of
-   MAC and IPv4 (experimental).
+ * Add hardware offload support for output, drop, set of MAC, IPv4
+   and tunnel push-output actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index 7b1ea3e08..aa976d62e 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -337,6 +337,20 @@ ds_put_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_cstr(s, "  Set-ttl = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_RAW_ENCAP) {
+const struct rte_flow_action_raw_encap *raw_encap = actions->conf;
+
+ds_put_cstr(s, "rte flow raw-encap action:\n");
+if (raw_encap) {
+ds_put_format(s,
+  "  Raw-encap: size=%ld\n",
+  raw_encap->size);
+ds_put_format(s,
+  "  Raw-encap: encap=\n");
+ds_put_hex_dump(s, raw_encap->data, raw_encap->size, 0, false);
+} else {
+ds_put_cstr(s, "  Raw-encap = null\n");
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -770,6 +784,44 @@ netdev_dpdk_flow_add_set_actions(struct flow_actions 
*actions,
 return 0;
 }
 
+static int
+netdev_dpdk_flow_add_clone_actions(struct flow_actions *actions,
+   const struct nlattr *clone_actions,
+   const size_t clone_actions_len,
+   struct offload_info *info)
+{
+const struct nlattr *ca;
+unsigned int cleft;
+
+NL_ATTR_FOR_EACH_UNSAFE (ca, cleft, clone_actions, clone_actions_len) {
+int clone_type = nl_attr_type(ca);
+
+if (clone_type == OVS_ACTION_ATTR_TUNNEL_PUSH) {
+const struct ovs_action_push_tnl *tnl_push = nl_attr_get(ca);
+struct rte_flow_action_raw_encap *raw_encap =
+xzalloc(sizeof *raw_encap);
+
+raw_encap->data = (uint8_t *)tnl_push->header;
+raw_encap->preserve = NULL;
+raw_encap->size = tnl_push->header_len;
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RAW_ENCAP,
+raw_encap);
+} else if (clone_type == OVS_ACTION_ATTR_OUTPUT) {
+if (!(cleft <= NLA_ALIGN(ca->nla_len)) ||
+netdev_dpdk_flow_add_output_action(actions, ca, info)) {
+return -1;
+}
+} else {
+VLOG_DBG_RL(&error_rl,
+"Unsupported clone action. clone_type=%d", clone_type);
+return -1;
+}
+}
+
+return 0;
+}
+
 int
 netdev_dpdk_flow_actions_add(struct flow_actions *actions,
  struct nlattr *nl_actions,
@@ -796,6 +848,15 @@ netdev_dpdk_flow_actions_add(struct flow_actions *actions,
  set_actions_len, masked)) {
 return -1;
 }
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CLONE) {
+const struct nlattr *clone_actions = nl_attr_get(nla);
+size_t clone_actions_len = nl_attr_get_size(nla);
+
+if (!(left <= NLA_ALIGN(nla->nla_len)) ||
+netdev_dpdk_flow_add_clone_actions(actions, clone_actions,
+   clone_actions_len, info)) {
+return -1;
+}
 } else {
 VLOG_DBG_RL(&error_rl,
 "Unsupported action type %d", nl_attr_type(nla));
-- 
2.14.5

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

[ovs-dev] [PATCH V2 17/19] netdev-offload-dpdk-flow: Support offload of set IPv4 actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 NEWS   |  4 ++--
 lib/netdev-offload-dpdk-flow.c | 50 ++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index b372e703a..19fe2b72b 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
- * Add hardware offload support for output, drop and set MAC actions
-   (experimental).
+ * Add hardware offload support for output, drop and set actions of
+   MAC and IPv4 (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index b9ceb1aaf..7b1ea3e08 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -313,6 +313,30 @@ ds_put_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_format(s, "  Set-mac-%s = null\n", dirstr);
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ||
+   actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DST) {
+const struct rte_flow_action_set_ipv4 *set_ipv4 = actions->conf;
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
+   ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-ipv4-%s action:\n", dirstr);
+if (set_ipv4) {
+ds_put_format(s,
+  "  Set-ipv4-%s: "IP_FMT"\n",
+  dirstr, IP_ARGS(set_ipv4->ipv4_addr));
+} else {
+ds_put_format(s, "  Set-ipv4-%s = null\n", dirstr);
+}
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TTL) {
+const struct rte_flow_action_set_ttl *set_ttl = actions->conf;
+
+ds_put_cstr(s, "rte flow set-ttl action:\n");
+if (set_ttl) {
+ds_put_format(s,
+  "  Set-ttl: %d\n", set_ttl->ttl_value);
+} else {
+ds_put_cstr(s, "  Set-ttl = null\n");
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -706,6 +730,32 @@ netdev_dpdk_flow_add_set_actions(struct flow_actions 
*actions,
 RTE_FLOW_ACTION_TYPE_SET_MAC_DST),
 };
 
+if (add_set_flow_action(actions, sa_info_arr,
+ARRAY_SIZE(sa_info_arr))) {
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_IPV4) {
+const struct ovs_key_ipv4 *key = nl_attr_get(sa);
+const struct ovs_key_ipv4 *mask = masked ?
+get_mask(sa, struct ovs_key_ipv4) : NULL;
+struct rte_flow_action_set_ipv4 *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_ipv4 *dst = xzalloc(sizeof *dst);
+struct rte_flow_action_set_ttl *ttl = xzalloc(sizeof *ttl);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(ipv4_src, src->ipv4_addr,
+RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC),
+SA_INFO(ipv4_dst, dst->ipv4_addr,
+RTE_FLOW_ACTION_TYPE_SET_IPV4_DST),
+SA_INFO(ipv4_ttl, ttl->ttl_value,
+RTE_FLOW_ACTION_TYPE_SET_TTL),
+};
+
+if (mask && (mask->ipv4_proto || mask->ipv4_tos ||
+mask->ipv4_frag)) {
+VLOG_DBG_RL(&error_rl, "Unsupported IPv4 set action");
+return -1;
+}
+
 if (add_set_flow_action(actions, sa_info_arr,
 ARRAY_SIZE(sa_info_arr))) {
 return -1;
-- 
2.14.5

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

[ovs-dev] [PATCH V2 13/19] netdev-dpdk: Getter function for dpdk port id API

[Eli Britstein]

Add a getter function for using the dpdk port id outside the scope of
netdev-dpdk.c to be used for HW offload.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-dpdk.c | 9 +
 lib/netdev-dpdk.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c891d3ed9..d047f4439 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4426,6 +4426,15 @@ unlock:
 return err;
 }
 
+int
+netdev_dpdk_get_port_id(const struct netdev *netdev)
+{
+if (!is_dpdk_class(netdev->netdev_class)) {
+return -1;
+}
+return CONTAINER_OF(netdev, struct netdev_dpdk, up)->port_id;
+}
+
 bool
 netdev_dpdk_flow_api_supported(struct netdev *netdev)
 {
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index 8e79bcdf8..54582ed90 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -56,6 +56,8 @@ netdev_dpdk_rte_flow_query(struct netdev *netdev,
struct rte_flow *rte_flow,
struct rte_flow_query_count *query,
struct rte_flow_error *error);
+int
+netdev_dpdk_get_port_id(const struct netdev *netdev);
 
 #else
 
-- 
2.14.5

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

[ovs-dev] [PATCH V2 16/19] netdev-offload-dpdk-flow: Support offload of set MAC actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 NEWS   |   3 +-
 lib/netdev-offload-dpdk-flow.c | 122 +
 2 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 45eef591f..b372e703a 100644
--- a/NEWS
+++ b/NEWS
@@ -26,7 +26,8 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
- * Add hardware offload support for output and drop actions (experimental).
+ * Add hardware offload support for output, drop and set MAC actions
+   (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index a73d9522d..b9ceb1aaf 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -298,6 +298,21 @@ ds_put_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 }
 } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
 ds_put_cstr(s, "rte flow drop action\n");
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ||
+   actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) {
+const struct rte_flow_action_set_mac *set_mac = actions->conf;
+
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST
+   ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr);
+if (set_mac) {
+ds_put_format(s,
+  "  Set-mac-%s: "ETH_ADDR_FMT"\n",
+  dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr));
+} else {
+ds_put_format(s, "  Set-mac-%s = null\n", dirstr);
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -608,6 +623,103 @@ netdev_dpdk_flow_add_output_action(struct flow_actions 
*actions,
 return ret;
 }
 
+struct set_action_info {
+const uint8_t *value, *mask;
+const uint8_t size;
+uint8_t *spec;
+const int attr;
+};
+
+static int
+add_set_flow_action(struct flow_actions *actions,
+struct set_action_info *sa_info_arr,
+size_t sa_info_arr_size)
+{
+int field, i;
+
+for (field = 0; field < sa_info_arr_size; field++) {
+if (sa_info_arr[field].mask) {
+/* DPDK does not support partially masked set actions. In such
+ * case, fail the offload.
+ */
+if (sa_info_arr[field].mask[0] != 0x00 &&
+sa_info_arr[field].mask[0] != 0xFF) {
+VLOG_DBG_RL(&error_rl,
+"Partial mask is not supported");
+return -1;
+}
+
+for (i = 1; i < sa_info_arr[field].size; i++) {
+if (sa_info_arr[field].mask[i] !=
+sa_info_arr[field].mask[i - 1]) {
+VLOG_DBG_RL(&error_rl,
+"Partial mask is not supported");
+return -1;
+}
+}
+
+if (sa_info_arr[field].mask[0] == 0x00) {
+/* mask bytes are all 0 - no rewrite action required */
+continue;
+}
+}
+
+memcpy(sa_info_arr[field].spec, sa_info_arr[field].value,
+   sa_info_arr[field].size);
+add_flow_action(actions, sa_info_arr[field].attr,
+sa_info_arr[field].spec);
+}
+
+return 0;
+}
+
+/* Mask is at the midpoint of the data. */
+#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1)
+
+#define SA_INFO(_field, _spec, _attr) { \
+.value = (uint8_t *)&key->_field, \
+.mask = (masked) ? (uint8_t *)&mask->_field : NULL, \
+.size = sizeof key->_field, \
+.spec = (uint8_t *)&_spec, \
+.attr = _attr }
+
+static int
+netdev_dpdk_flow_add_set_actions(struct flow_actions *actions,
+ const struct nlattr *set_actions,
+ const size_t set_actions_len,
+ bool masked)
+{
+const struct nlattr *sa;
+unsigned int sleft;
+
+NL_ATTR_FOR_EACH_UNSAFE (sa, sleft, set_actions, set_actions_len) {
+if (nl_attr_type(sa) == OVS_KEY_ATTR_ETHERNET) {
+const struct ovs_key_ethernet *key = nl_attr_get(sa);
+const struct ovs_key_ethernet *mask = masked ?
+get_mask(sa, struct ovs_key_ethernet) : NULL;
+struct rte_flow_action_set_mac *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(eth_src, src->mac_addr[0],
+RTE_FLOW_ACTION_TYPE_SET_MAC_SRC),
+SA_INFO(eth_dst, dst->mac_addr[0],
+RTE

[ovs-dev] [PATCH V2 19/19] netdev-offload-dpdk-flow: Support offload of set TCP/UDP ports actions

[Eli Britstein]

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 NEWS   |  4 ++--
 lib/netdev-offload-dpdk-flow.c | 48 ++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 2ccc19b90..cbdaaeb43 100644
--- a/NEWS
+++ b/NEWS
@@ -26,8 +26,8 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
- * Add hardware offload support for output, drop, set of MAC, IPv4
-   and tunnel push-output actions (experimental).
+ * Add hardware offload support for output, drop, set of MAC, IPv4,
+   TCP/UDP ports and tunnel push-output actions (experimental).
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index aa976d62e..457facd27 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -351,6 +351,20 @@ ds_put_flow_action(struct ds *s, const struct 
rte_flow_action *actions)
 } else {
 ds_put_cstr(s, "  Raw-encap = null\n");
 }
+} else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_SRC ||
+   actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST) {
+const struct rte_flow_action_set_tp *set_tp = actions->conf;
+char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST
+   ? "dst" : "src";
+
+ds_put_format(s, "rte flow set-tcp/udp-port-%s action:\n", dirstr);
+if (set_tp) {
+ds_put_format(s,
+  "  Set-%s-tcp/udp-port: %"PRIu16"\n",
+  dirstr, ntohs(set_tp->port));
+} else {
+ds_put_format(s, "  Set-%s-tcp/udp-port = null\n", dirstr);
+}
 } else {
 ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
 }
@@ -770,6 +784,40 @@ netdev_dpdk_flow_add_set_actions(struct flow_actions 
*actions,
 return -1;
 }
 
+if (add_set_flow_action(actions, sa_info_arr,
+ARRAY_SIZE(sa_info_arr))) {
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_TCP) {
+const struct ovs_key_tcp *key = nl_attr_get(sa);
+const struct ovs_key_tcp *mask = masked ?
+get_mask(sa, struct ovs_key_tcp) : NULL;
+struct rte_flow_action_set_tp *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_tp *dst = xzalloc(sizeof *dst);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(tcp_src, src->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_SRC),
+SA_INFO(tcp_dst, dst->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_DST),
+};
+
+if (add_set_flow_action(actions, sa_info_arr,
+ARRAY_SIZE(sa_info_arr))) {
+return -1;
+}
+} else if (nl_attr_type(sa) == OVS_KEY_ATTR_UDP) {
+const struct ovs_key_udp *key = nl_attr_get(sa);
+const struct ovs_key_udp *mask = masked ?
+get_mask(sa, struct ovs_key_udp) : NULL;
+struct rte_flow_action_set_tp *src = xzalloc(sizeof *src);
+struct rte_flow_action_set_tp *dst = xzalloc(sizeof *dst);
+struct set_action_info sa_info_arr[] = {
+SA_INFO(udp_src, src->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_SRC),
+SA_INFO(udp_dst, dst->port,
+RTE_FLOW_ACTION_TYPE_SET_TP_DST),
+};
+
 if (add_set_flow_action(actions, sa_info_arr,
 ARRAY_SIZE(sa_info_arr))) {
 return -1;
-- 
2.14.5

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

[ovs-dev] [PATCH V2 08/19] netdev-offload-dpdk: Framework for actions offload

[Eli Britstein]

Currently HW offload is accelerating only the rule matching sequence.
Introduce a framework for offloading rule actions as a pre-step for
processing the rule actions in HW. In case of a failure, fallback to the
legacy partial offload scheme.

Note: a flow will be fully offloaded only if it can process all its
actions in HW.

Signed-off-by: Eli Britstein 
Reviewed-by: Oz Shlomo 
---
 lib/netdev-offload-dpdk-flow.c| 28 +
 lib/netdev-offload-dpdk-private.h |  5 +++
 lib/netdev-offload-dpdk.c | 83 ++-
 lib/netdev-offload.h  |  1 +
 4 files changed, 99 insertions(+), 18 deletions(-)

diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
index 19c933932..b03ce2c41 100644
--- a/lib/netdev-offload-dpdk-flow.c
+++ b/lib/netdev-offload-dpdk-flow.c
@@ -18,6 +18,7 @@
 #include 
 
 #include "dpif-netdev.h"
+#include "netdev-offload-provider.h"
 #include "netdev-offload-dpdk-private.h"
 #include "openvswitch/match.h"
 #include "openvswitch/vlog.h"
@@ -25,6 +26,8 @@
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload_dpdk_flow);
 
+static struct vlog_rate_limit error_rl = VLOG_RATE_LIMIT_INIT(100, 5);
+
 void
 netdev_dpdk_flow_patterns_free(struct flow_patterns *patterns)
 {
@@ -528,3 +531,28 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns 
*patterns,
 return 0;
 }
 
+int
+netdev_dpdk_flow_actions_add(struct flow_actions *actions,
+ struct nlattr *nl_actions,
+ size_t nl_actions_len,
+ struct offload_info *info OVS_UNUSED)
+{
+struct nlattr *nla;
+size_t left;
+
+NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
+VLOG_DBG_RL(&error_rl,
+"Unsupported action type %d", nl_attr_type(nla));
+return -1;
+}
+
+if (nl_actions_len == 0) {
+VLOG_DBG_RL(&error_rl,
+"Unsupported action type drop");
+return -1;
+}
+
+add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
+return 0;
+}
+
diff --git a/lib/netdev-offload-dpdk-private.h 
b/lib/netdev-offload-dpdk-private.h
index 68caa7144..b69b76dff 100644
--- a/lib/netdev-offload-dpdk-private.h
+++ b/lib/netdev-offload-dpdk-private.h
@@ -51,6 +51,11 @@ void
 netdev_dpdk_flow_actions_add_mark_rss(struct flow_actions *actions,
   struct netdev *netdev,
   uint32_t mark_id);
+int
+netdev_dpdk_flow_actions_add(struct flow_actions *actions,
+ struct nlattr *nl_actions,
+ size_t nl_actions_len,
+ struct offload_info *info);
 struct ds *
 netdev_dpdk_flow_ds_put_flow(struct ds *s,
  const struct rte_flow_attr *attr,
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 64873759d..8561a9b77 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -115,13 +115,9 @@ ufid_to_rte_flow_disassociate(const ovs_u128 *ufid)
   UUID_ARGS((struct uuid *) ufid));
 }
 
-static int
-netdev_offload_dpdk_add_flow(struct netdev *netdev,
- const struct match *match,
- struct nlattr *nl_actions OVS_UNUSED,
- size_t actions_len OVS_UNUSED,
- const ovs_u128 *ufid,
- struct offload_info *info)
+static struct rte_flow *
+netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
+ struct netdev *netdev, uint32_t flow_mark)
 {
 const struct rte_flow_attr flow_attr = {
 .group = 0,
@@ -129,10 +125,63 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 .ingress = 1,
 .egress = 0
 };
-struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
 struct flow_actions actions = { .actions = NULL, .cnt = 0 };
 struct rte_flow_error error;
 struct rte_flow *flow;
+
+netdev_dpdk_flow_actions_add_mark_rss(&actions, netdev, flow_mark);
+flow = netdev_dpdk_rte_flow_create(netdev, &flow_attr,
+   patterns->items,
+   actions.actions, &error);
+if (!flow) {
+VLOG_ERR("%s: rte flow create error: %u : message : %s\n",
+ netdev_get_name(netdev), error.type, error.message);
+}
+netdev_dpdk_flow_actions_free(&actions);
+return flow;
+}
+
+static struct rte_flow *
+netdev_offload_dpdk_actions(struct netdev *netdev,
+struct flow_patterns *patterns,
+struct nlattr *nl_actions,
+size_t actions_len,
+struct offload_info *info)
+{
+const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
+struct flow_actions actions = { .actions = NULL, .cnt = 0 };
+ 

Re: [ovs-dev] [PATCH ovn v1] northd: Remove misleading warning log message

[Numan Siddique]

On Mon, Dec 2, 2019 at 8:54 AM Russell Bryant  wrote:
>
> While debugging an ovn-kubernetes cluster, I spotted several
> "Duplicate MAC set" warning messages in the ovn-northd log.  It looks
> like this message was emitted from this code path by mistake, where
> it correctly avoided assigning a duplicate MAC address.  This patch
> turns off the warning for that case.
>
> Signed-off-by: Russell Bryant 

Acked-by: Numan Siddique 

Numan

> ---
>  northd/ovn-northd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index a943e1037..9f558c628 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1395,7 +1395,7 @@ ipam_get_unused_mac(ovs_be32 ip)
>  mac_addr_suffix = ((base_addr + i) % (MAC_ADDR_SPACE - 1)) + 1;
>  mac64 =  eth_addr_to_uint64(mac_prefix) | mac_addr_suffix;
>  eth_addr_from_uint64(mac64, &mac);
> -if (!ipam_is_duplicate_mac(&mac, mac64, true)) {
> +if (!ipam_is_duplicate_mac(&mac, mac64, false)) {
>  break;
>  }
>  }
> --
> 2.23.0
>
> ___
> 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 V2 11/19] dpif-netdev: Read hw stats during flow_dump_next() call

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Line is 81 characters long (recommended limit is 79)
#83 FILE: lib/dpif-netdev.c:3628:
non_atomic_ullong_add(&netdev_flow->stats.packet_count, 
stats.n_packets);

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


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

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

Re: [ovs-dev] [PATCH V2 08/19] netdev-offload-dpdk: Framework for actions offload

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#144 FILE: lib/netdev-offload-dpdk.c:157:
ret = netdev_dpdk_flow_actions_add(&actions, nl_actions, actions_len, info);

WARNING: Line is 80 characters long (recommended limit is 79)
#192 FILE: lib/netdev-offload-dpdk.c:202:
flow = netdev_offload_dpdk_mark_rss(&patterns, netdev, info->flow_mark);

Lines checked: 222, Warnings: 2, Errors: 0


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

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

[ovs-dev] [PATCH ovn] ovn-controller: Add missing port group lflow references.

[Dumitru Ceara]

The commit that adds incremental processing for port-group changes
doesn't store logical flow references for port groups. If a port group
is updated (e.g., a port is added) no logical flow recalculation will be
performed.

To fix this, when parsing the flow expression also store the referenced
port groups and bind them to the logical flows that depend on them. If
the port group is updated then the logical flows referring them will
also be reinstalled.

Reported-by: Daniel Alvarez 
Reported-at: https://bugzilla.redhat.com/1778164
CC: Han Zhou 
Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for port-group 
changes.")
Signed-off-by: Dumitru Ceara 

Change-Id: Id39695f022f16b598fd8416cd2494e7dab3bf11b
---
 controller/lflow.c|  9 -
 include/ovn/expr.h|  4 +++-
 lib/actions.c |  4 ++--
 lib/expr.c| 24 +---
 tests/test-ovn.c  |  8 
 utilities/ovn-trace.c |  2 +-
 6 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 36150bd..a689320 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -616,14 +616,21 @@ consider_logical_flow(
 struct expr *expr;
 
 struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
+struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
 expr = expr_parse_string(lflow->match, &symtab, addr_sets, port_groups,
- &addr_sets_ref, &error);
+ &addr_sets_ref, &port_groups_ref, &error);
 const char *addr_set_name;
 SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
 lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
&lflow->header_.uuid);
 }
+const char *port_group_name;
+SSET_FOR_EACH (port_group_name, &port_groups_ref) {
+lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
+   &lflow->header_.uuid);
+}
 sset_destroy(&addr_sets_ref);
+sset_destroy(&port_groups_ref);
 
 if (!error) {
 if (prereqs) {
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 22f633e..21bf51c 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -390,11 +390,13 @@ void expr_print(const struct expr *);
 struct expr *expr_parse(struct lexer *, const struct shash *symtab,
 const struct shash *addr_sets,
 const struct shash *port_groups,
-struct sset *addr_sets_ref);
+struct sset *addr_sets_ref,
+struct sset *port_groups_ref);
 struct expr *expr_parse_string(const char *, const struct shash *symtab,
const struct shash *addr_sets,
const struct shash *port_groups,
struct sset *addr_sets_ref,
+   struct sset *port_groups_ref,
char **errorp);
 
 struct expr *expr_clone(struct expr *);
diff --git a/lib/actions.c b/lib/actions.c
index 586d7b7..051e6c8 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -240,8 +240,8 @@ add_prerequisite(struct action_context *ctx, const char 
*prerequisite)
 struct expr *expr;
 char *error;
 
-expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL, NULL,
- &error);
+expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
+ NULL, NULL, &error);
 ovs_assert(!error);
 ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
 }
diff --git a/lib/expr.c b/lib/expr.c
index 71de615..e5e4d21 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -480,7 +480,8 @@ struct expr_context {
 const struct shash *symtab;/* Symbol table. */
 const struct shash *addr_sets; /* Address set table. */
 const struct shash *port_groups; /* Port group table. */
-struct sset *addr_sets_ref;   /* The set of address set referenced. */
+struct sset *addr_sets_ref;  /* The set of address set referenced. */
+struct sset *port_groups_ref;/* The set of port groups referenced. */
 bool not;/* True inside odd number of NOT operators. */
 unsigned int paren_depth;/* Depth of nested parentheses. */
 };
@@ -782,6 +783,10 @@ static bool
 parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,
  size_t *allocated_values)
 {
+if (ctx->port_groups_ref) {
+sset_add(ctx->port_groups_ref, ctx->lexer->token.s);
+}
+
 struct expr_constant_set *port_group
 = (ctx->port_groups
? shash_find_data(ctx->port_groups, ctx->lexer->token.s)
@@ -1296,13 +1301,15 @@ struct expr *
 expr_parse(struct lexer *lexer, const struct shash *symtab,
const struct shash *addr_sets,
const struct shash *port_groups,
-   struct sset *addr_sets_ref)
+   struct sset 

Re: [ovs-dev] [PATCH] dpif-netdev: Use netdev-offload API for port lookup while offloading.

[Ophir Munk]

> -Original Message-
> From: Ilya Maximets 
> Sent: Saturday, November 30, 2019 2:07 PM
> To: Ophir Munk ; Ilya Maximets
> ; ovs-dev@openvswitch.org
> Cc: Roni Bar Yanai ; Eli Britstein
> ; Ian Stokes ; Ameer
> Mahagneh 
> Subject: Re: [PATCH] dpif-netdev: Use netdev-offload API for port lookup
> while offloading.
> 
> On 29.11.2019 15:46, Ophir Munk wrote:
> > Hi Ilya,
> > The first two RFC series are confirmed. They require the third RFC series to
> enable user space vport offloading.
> >
> > 1.
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D14320
> 9&a
> >
> mp;data=02%7C01%7Cophirmu%40mellanox.com%7C0d4c9b5abcef4d0796b5
> 08d7758
> >
> ddfc7%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371071245400
> 95619&
> >
> amp;sdata=OyBtcrlldUxUPZTNy6V3zArvjas1cIAA6EzBjkcXJsY%3D&reser
> ved=
> > 0
> > ("netdev-offload: Prerequisites of vport offloading via DPDK")
> >
> > 2.
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D14455
> 9&a
> >
> mp;data=02%7C01%7Cophirmu%40mellanox.com%7C0d4c9b5abcef4d0796b5
> 08d7758
> >
> ddfc7%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371071245400
> 95619&
> >
> amp;sdata=PBp1gVHPJoRWhTVDGzLDsi4EPRcSlbQRZeVS9K99APE%3D&
> reserved=
> > 0
> > ("dpif-netdev: Use netdev-offload API for port lookup while
> > offloading.")
> >
> > 3.
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D14482
> 6&a
> >
> mp;data=02%7C01%7Cophirmu%40mellanox.com%7C0d4c9b5abcef4d0796b5
> 08d7758
> >
> ddfc7%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371071245400
> 95619&
> >
> amp;sdata=OoZ0OfQunKaPqjKzOhIScIfAJwR%2B1g36qZcQITvAX60%3D&
> ;reserve
> > d=0 ("[ovs-dev,RFC,v1] netdev-dpdk: Add flow_api support for netdev
> > vports")
> >
> > Can you please merge all the three series into one patch set?
> > Then we will be able  to test it and send our feedback.
> 
> Hi.
> 
> This patch (series #2) is more like a bug fix that doesn't relate to vport
> offloading.  I'd like it to be reviewed/ACKed as a standalone fix, so I could
> push it directly to master.  After that I could rebase series #1.

Patch (series #2) tested successfully. 
Acked-by: Ophir Munk 

Looking forward to the rebase of series #1

> 
> I don't see the reason for changes in series #3, the doesn't make sense for
> me without looking at your vport offloading code, so I'd suggest to make it a
> part of your vport offlloading patch-set.  However, I'm suspecting that you're
> going to use this similarly to what happens in there:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.
> openvswitch.org%2Fpipermail%2Fovs-dev%2F2019-
> November%2F364931.html&data=02%7C01%7Cophirmu%40mellanox.c
> om%7C0d4c9b5abcef4d0796b508d7758ddfc7%7Ca652971c7d2e4d9ba6a4d149
> 256f461b%7C0%7C0%7C637107124540095619&sdata=atkVKubOzY0SeeZ
> Vm0inL9Tsv1UVu4pWC1k1J%2FoX1Es%3D&reserved=0
> So, you may take the same suggestion into account:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.
> openvswitch.org%2Fpipermail%2Fovs-dev%2F2019-
> November%2F365294.html&data=02%7C01%7Cophirmu%40mellanox.c
> om%7C0d4c9b5abcef4d0796b508d7758ddfc7%7Ca652971c7d2e4d9ba6a4d149
> 256f461b%7C0%7C0%7C637107124540095619&sdata=vsVJlVPcxzoRTWA
> OSCNudKzAjzI9cRbmXeHyMGrxDC8%3D&reserved=0
> 
> Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovn-controller: Add missing port group lflow references.

[0-day Robot]

Bleep bloop.  Greetings Dumitru Ceara, 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: Remove Gerrit Change-Id's before submitting upstream.
21: Change-Id: Id39695f022f16b598fd8416cd2494e7dab3bf11b

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


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

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

[ovs-dev] [PATCH ovn v2] ovn-controller: Add missing port group lflow references.

[Dumitru Ceara]

The commit that adds incremental processing for port-group changes
doesn't store logical flow references for port groups. If a port group
is updated (e.g., a port is added) no logical flow recalculation will be
performed.

To fix this, when parsing the flow expression also store the referenced
port groups and bind them to the logical flows that depend on them. If
the port group is updated then the logical flows referring them will
also be reinstalled.

Reported-by: Daniel Alvarez 
Reported-at: https://bugzilla.redhat.com/1778164
CC: Han Zhou 
Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for port-group 
changes.")
Signed-off-by: Dumitru Ceara 

---
v2: remove Change-Id from commit message.
---
 controller/lflow.c|  9 -
 include/ovn/expr.h|  4 +++-
 lib/actions.c |  4 ++--
 lib/expr.c| 24 +---
 tests/test-ovn.c  |  8 
 utilities/ovn-trace.c |  2 +-
 6 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 36150bd..a689320 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -616,14 +616,21 @@ consider_logical_flow(
 struct expr *expr;
 
 struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
+struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
 expr = expr_parse_string(lflow->match, &symtab, addr_sets, port_groups,
- &addr_sets_ref, &error);
+ &addr_sets_ref, &port_groups_ref, &error);
 const char *addr_set_name;
 SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
 lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
&lflow->header_.uuid);
 }
+const char *port_group_name;
+SSET_FOR_EACH (port_group_name, &port_groups_ref) {
+lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
+   &lflow->header_.uuid);
+}
 sset_destroy(&addr_sets_ref);
+sset_destroy(&port_groups_ref);
 
 if (!error) {
 if (prereqs) {
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 22f633e..21bf51c 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -390,11 +390,13 @@ void expr_print(const struct expr *);
 struct expr *expr_parse(struct lexer *, const struct shash *symtab,
 const struct shash *addr_sets,
 const struct shash *port_groups,
-struct sset *addr_sets_ref);
+struct sset *addr_sets_ref,
+struct sset *port_groups_ref);
 struct expr *expr_parse_string(const char *, const struct shash *symtab,
const struct shash *addr_sets,
const struct shash *port_groups,
struct sset *addr_sets_ref,
+   struct sset *port_groups_ref,
char **errorp);
 
 struct expr *expr_clone(struct expr *);
diff --git a/lib/actions.c b/lib/actions.c
index 586d7b7..051e6c8 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -240,8 +240,8 @@ add_prerequisite(struct action_context *ctx, const char 
*prerequisite)
 struct expr *expr;
 char *error;
 
-expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL, NULL,
- &error);
+expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
+ NULL, NULL, &error);
 ovs_assert(!error);
 ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
 }
diff --git a/lib/expr.c b/lib/expr.c
index 71de615..e5e4d21 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -480,7 +480,8 @@ struct expr_context {
 const struct shash *symtab;/* Symbol table. */
 const struct shash *addr_sets; /* Address set table. */
 const struct shash *port_groups; /* Port group table. */
-struct sset *addr_sets_ref;   /* The set of address set referenced. */
+struct sset *addr_sets_ref;  /* The set of address set referenced. */
+struct sset *port_groups_ref;/* The set of port groups referenced. */
 bool not;/* True inside odd number of NOT operators. */
 unsigned int paren_depth;/* Depth of nested parentheses. */
 };
@@ -782,6 +783,10 @@ static bool
 parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,
  size_t *allocated_values)
 {
+if (ctx->port_groups_ref) {
+sset_add(ctx->port_groups_ref, ctx->lexer->token.s);
+}
+
 struct expr_constant_set *port_group
 = (ctx->port_groups
? shash_find_data(ctx->port_groups, ctx->lexer->token.s)
@@ -1296,13 +1301,15 @@ struct expr *
 expr_parse(struct lexer *lexer, const struct shash *symtab,
const struct shash *addr_sets,
const struct shash *port_groups,
-   struct sset *addr_sets_ref)
+   struct sset *addr_s

[ovs-dev] [PATCH 0/4] Add support for TSO with DPDK

[Flavio Leitner]

Abbreviated as TSO, TCP Segmentation Offload is a feature which enables
the network stack to delegate the TCP segmentation to the NIC reducing
the per packet CPU overhead.

A guest using vhost-user interface with TSO enabled can send TCP packets
much bigger than the MTU, which saves CPU cycles normally used to break
the packets down to MTU size and to calculate checksums.

It also saves CPU cycles used to parse multiple packets/headers during
the packet processing inside virtual switch.

If the destination of the packet is another guest in the same host, then
the same big packet can be sent through a vhost-user interface skipping
the segmentation completely. However, if the destination is not local,
the NIC hardware is instructed to do the TCP segmentation and checksum
calculation.

The first 3 patches are not really part of TSO support, but they are
required to make sure everything works.

The TSO is only enabled in the vhost-user ports in client mode which
means DPDK is required. The other ports (dpdk or linux) can receive
those packets.

This patchset is based on branch dpdk-latest (v19.11 required).

The numbers look good and I noticed no regression so far. However, my
environment is tuned but not well fined tuned, so consider those as
ball park numbers only.

This is VM-to-external host (Gbits/sec)
   | No patch  |  TSO off  |  TSO on
  -
TCPv4  |10 |10 |   38  (line rate)
---
TCPv6  | 9 | 9 |   38  (line rate)
---
V4 Conntrack   | 1 | 1 |   33


This is VM-to-VM in the same host (Gbits/sec)
   | No patch  |  TSO off  |  TSO on
  -
TCPv4  | 9.4   |9.4|   31
---
TCPv6  | 8 |8  |   30
---

There are good improvements sending to veth pairs or tap devices too.

Flavio Leitner (4):
  dp-packet: preserve headroom when cloning a pkt batch
  vhost: Disable multi-segmented buffers
  dp-packet: handle new dpdk buffer flags
  netdev-dpdk: Add TCP Segmentation Offload support

 Documentation/automake.mk   |   1 +
 Documentation/topics/dpdk/index.rst |   1 +
 Documentation/topics/dpdk/tso.rst   |  83 +
 NEWS|   1 +
 lib/automake.mk |   2 +
 lib/dp-packet.c |  50 -
 lib/dp-packet.h |  38 --
 lib/netdev-bsd.c|   7 ++
 lib/netdev-dpdk.c   | 112 +++-
 lib/netdev-dummy.c  |   7 ++
 lib/netdev-linux.c  |  71 --
 lib/tso.c   |  54 ++
 lib/tso.h   |  23 ++
 vswitchd/bridge.c   |   2 +
 vswitchd/vswitch.xml|  14 
 15 files changed, 430 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/topics/dpdk/tso.rst
 create mode 100644 lib/tso.c
 create mode 100644 lib/tso.h

-- 
2.23.0

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

[ovs-dev] [PATCH 1/4] dp-packet: preserve headroom when cloning a pkt batch

[Flavio Leitner]

The headroom is useful if the packet needs to insert additional
header, so preserve the original headroom when cloning the batch.

Signed-off-by: Flavio Leitner 
---
 lib/dp-packet.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 14f0897fa..1e5362304 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -874,7 +874,11 @@ dp_packet_batch_clone(struct dp_packet_batch *dst,
 
 dp_packet_batch_init(dst);
 DP_PACKET_BATCH_FOR_EACH (i, packet, src) {
-dp_packet_batch_add(dst, dp_packet_clone(packet));
+uint32_t headroom = dp_packet_headroom(packet);
+struct dp_packet *pkt_clone;
+
+pkt_clone  = dp_packet_clone_with_headroom(packet, headroom);
+dp_packet_batch_add(dst, pkt_clone);
 }
 dst->trunc = src->trunc;
 }
-- 
2.23.0

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

[ovs-dev] [PATCH 3/4] dp-packet: handle new dpdk buffer flags

[Flavio Leitner]

DPDK included a couple flags EXT_ATTACHED_MBUF and IND_ATTACHED_MBUF
which are not really offloading flags, so this patch fixes to reset
only offloading flags or to reset only those flags when needed.

Signed-off-by: Flavio Leitner 
---
 lib/dp-packet.c | 4 +++-
 lib/dp-packet.h | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 62d7faa4c..e02891449 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -193,7 +193,9 @@ dp_packet_clone_with_headroom(const struct dp_packet 
*buffer, size_t headroom)
 offsetof(struct dp_packet, l2_pad_size));
 
 #ifdef DPDK_NETDEV
-new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags;
+/* The new packet is linear, so copy only the offloading flags */
+new_buffer->mbuf.ol_flags = buffer->mbuf.ol_flags
+& ~(EXT_ATTACHED_MBUF | IND_ATTACHED_MBUF);
 #endif
 
 if (dp_packet_rss_valid(buffer)) {
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 1e5362304..325924eaa 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -538,7 +538,7 @@ dp_packet_rss_valid(const struct dp_packet *p)
 static inline void
 dp_packet_reset_offload(struct dp_packet *p)
 {
-p->mbuf.ol_flags = 0;
+p->mbuf.ol_flags &= (EXT_ATTACHED_MBUF | IND_ATTACHED_MBUF);
 }
 
 static inline bool
-- 
2.23.0

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

[ovs-dev] [PATCH 2/4] vhost: Disable multi-segmented buffers

[Flavio Leitner]

There is no support for multi-segmented buffers, so flag
that to vhost library.

Signed-off-by: Flavio Leitner 
---
 lib/netdev-dpdk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2423d26ee..cd035f76e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4355,6 +4355,9 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev 
*netdev)
 /* Register client-mode device. */
 vhost_flags |= RTE_VHOST_USER_CLIENT;
 
+/* There is no support for multi-segments buffers */
+vhost_flags |= RTE_VHOST_USER_LINEARBUF_SUPPORT;
+
 /* Enable IOMMU support, if explicitly requested. */
 if (dpdk_vhost_iommu_enabled()) {
 vhost_flags |= RTE_VHOST_USER_IOMMU_SUPPORT;
-- 
2.23.0

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

[ovs-dev] [PATCH 4/4] netdev-dpdk: Add TCP Segmentation Offload support

[Flavio Leitner]

Abbreviated as TSO, TCP Segmentation Offload is a feature which enables
the network stack to delegate the TCP segmentation to the NIC reducing
the per packet CPU overhead.

A guest using vhostuser interface with TSO enabled can send TCP packets
much bigger than the MTU, which saves CPU cycles normally used to break
the packets down to MTU size and to calculate checksums.

It also saves CPU cycles used to parse multiple packets/headers during
the packet processing inside virtual switch.

If the destination of the packet is another guest in the same host, then
the same big packet can be sent through a vhostuser interface skipping
the segmentation completely. However, if the destination is not local,
the NIC hardware is instructed to do the TCP segmentation and checksum
calculation.

It is recommended to check if NIC hardware supports TSO before enabling
the feature, which is off by default.

Signed-off-by: Flavio Leitner 
---
 Documentation/automake.mk   |   1 +
 Documentation/topics/dpdk/index.rst |   1 +
 Documentation/topics/dpdk/tso.rst   |  83 +
 NEWS|   1 +
 lib/automake.mk |   2 +
 lib/dp-packet.c |  46 
 lib/dp-packet.h |  30 ++--
 lib/netdev-bsd.c|   7 ++
 lib/netdev-dpdk.c   | 109 +++-
 lib/netdev-dummy.c  |   7 ++
 lib/netdev-linux.c  |  71 --
 lib/tso.c   |  54 ++
 lib/tso.h   |  23 ++
 vswitchd/bridge.c   |   2 +
 vswitchd/vswitch.xml|  14 
 15 files changed, 418 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/topics/dpdk/tso.rst
 create mode 100644 lib/tso.c
 create mode 100644 lib/tso.h

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 5f7c3e07b..abe5aaed1 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -35,6 +35,7 @@ DOC_SOURCE = \
Documentation/topics/dpdk/index.rst \
Documentation/topics/dpdk/bridge.rst \
Documentation/topics/dpdk/jumbo-frames.rst \
+   Documentation/topics/dpdk/tso.rst \
Documentation/topics/dpdk/memory.rst \
Documentation/topics/dpdk/pdump.rst \
Documentation/topics/dpdk/phy.rst \
diff --git a/Documentation/topics/dpdk/index.rst 
b/Documentation/topics/dpdk/index.rst
index f2862ea70..400d56051 100644
--- a/Documentation/topics/dpdk/index.rst
+++ b/Documentation/topics/dpdk/index.rst
@@ -40,4 +40,5 @@ DPDK Support
/topics/dpdk/qos
/topics/dpdk/pdump
/topics/dpdk/jumbo-frames
+   /topics/dpdk/tso
/topics/dpdk/memory
diff --git a/Documentation/topics/dpdk/tso.rst 
b/Documentation/topics/dpdk/tso.rst
new file mode 100644
index 0..92393a399
--- /dev/null
+++ b/Documentation/topics/dpdk/tso.rst
@@ -0,0 +1,83 @@
+..
+  Copyright 2019, Red Hat, Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+===
+TSO
+===
+
+**Note:** This feature is considered experimental.
+
+TCP Segmentation Offload (TSO) enables a network stack to delegate segmentation
+of an oversized TCP segment to the underlying physical NIC. Offload of frame
+segmentation achieves computational savings in the core, freeing up CPU cycles
+for more useful work.
+
+A common use case for TSO is when using virtualization, where traffic that's
+coming in from a VM can offload the TCP segmentation, thus avoiding the
+fragmentation in software. Additionally, if the traffic is headed to a VM
+within the same host further optimization can be expected. As the traffic never
+leaves the machine, no MTU needs to be accounted for, and thus no segmentation
+and checksum calculations are required, which saves yet more cycles. Only when
+the traffic actually leaves the host the segmentation needs to happen, in which
+case it will be performed by the egress NIC. Consult your controller's 
datasheet
+for compatibility. Secondly, the NIC must have an associated DPDK Poll Mode
+Driver (PMD) which supports `TSO`.
+
+Enabling 

Re: [ovs-dev] [PATCH v2 03/10] tc: Introduce tc_id to specify a tc filter

[Marcelo Ricardo Leitner]

On Sun, Dec 01, 2019 at 07:38:35AM +, Paul Blakey wrote:
> 
> On 11/27/2019 4:07 PM, Marcelo Ricardo Leitner wrote:
> > On Wed, Nov 27, 2019 at 02:55:09PM +0200, Roi Dayan wrote: >> From: Paul 
> > Blakey  >> >> Move all that 
> > is needed to identify a tc filter to a new structure, >> tc_id. This 
> > removes a lot of duplication in accessing/creating tc >> filters. > > Ok, 
> > gotta say, 'tc_id' is a bit confusing here. Every time I read > it, I try 
> > to find a correlating id in the kernel land but it doesn't > exist, because 
> > 'tc' itself refers to a much bigger thing and not > just filters. > > That 
> > said, I was thinking, what about 'tcf_id', or even a longer > one, 
> > 'tc_filter_id'? But before answering, please read below. :-)
> I don't mind either way, but from this I'd rater have the short one tcf_id.
> 
> > >> >> --- >> >> Changelog: V1->V2: In tc_del_matchall_policer - reverse 
> > >> >> xmas param >> order Added and used helper is_tc_id_eq(id1, id2) in 
> > >> >> find_ufid In >> netdev_tc_flow_dump_next - use make_tc_id() instead 
> > >> >> of manualy >> filling id > ^^ > >> In netdev_tc_flow_put - 
> > >> >> use if (get_ufid_tc_mapping(ufid, &id) == >> 0) to be mor explict we 
> > >> >> found the mapping not failed to get In >> make_tc_id - fill id 
> > >> >> explictily and removed memset. Moved >> tc_make_id to be static 
> > >> >> inline in tc.h > ^^ > > The patch is currently using 
> > >> >> make_tc_id. I see other functions > already using the latter one, 
> > >> >> tc_make_*, like tc_make_handle and > tc_make_request. Though it also 
> > >> >> already has make_tc_id_chain. Which > then, with this last one, it 
> > >> >> gets easier to understand what a 'tc_id' > means. > > I don't have a 
> > >> >> strong opinion here, so I'm just highlighting the > topic.
> Not sure what you meant here.
> 
> 
> Would you rather  we have tc_make_tcf_id() and tc_make_tcf_id_chain() (in 
> later patch) to be more consistent?

Yes. That seems the best to me.

Thanks,
  Marcelo

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

Re: [ovs-dev] [PATCH 3/4] dp-packet: handle new dpdk buffer flags

[0-day Robot]

Bleep bloop.  Greetings Flavio Leitner, 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/dp-packet.h).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 dp-packet: handle new dpdk buffer flags
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...@redhat.com

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

Re: [ovs-dev] [PATCH 15/20] netdev-offload-dpdk-flow: Support offload of output action

[Ilya Maximets]

On 01.12.2019 9:29, Eli Britstein wrote:
> 
> On 11/30/2019 1:59 PM, Ilya Maximets wrote:
>> On 24.11.2019 14:22, Eli Britstein wrote:
>>> On 11/22/2019 6:19 PM, Ilya Maximets wrote:
 On 20.11.2019 16:28, Eli Britstein wrote:
> Signed-off-by: Eli Britstein 
> Reviewed-by: Oz Shlomo 
> ---
>NEWS   |  2 +
>lib/netdev-offload-dpdk-flow.c | 87 
> --
>2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 88b818948..ca9c2b230 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,8 @@ Post-v2.12.0
>   if supported by libbpf.
> * Add option to enable, disable and query TCP sequence checking in
>   conntrack.
> +   - DPDK:
> + * Add hardware offload support for output actions.
>
>v2.12.0 - 03 Sep 2019
>-
> diff --git a/lib/netdev-offload-dpdk-flow.c 
> b/lib/netdev-offload-dpdk-flow.c
> index dbbc45e99..6e7efb315 100644
> --- a/lib/netdev-offload-dpdk-flow.c
> +++ b/lib/netdev-offload-dpdk-flow.c
> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct 
> rte_flow_action *actions)
>} else {
>ds_put_cstr(s, "  RSS = null\n");
>}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
> +const struct rte_flow_action_count *count = actions->conf;
> +
> +ds_put_cstr(s, "rte flow count action:\n");
> +if (count) {
> +ds_put_format(s,
> +  "  Count: shared=%d, id=%d\n",
> +  count->shared, count->id);
> +} else {
> +ds_put_cstr(s, "  Count = null\n");
> +}
> +} else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
> +const struct rte_flow_action_port_id *port_id = actions->conf;
> +
> +ds_put_cstr(s, "rte flow port-id action:\n");
> +if (port_id) {
> +ds_put_format(s,
> +  "  Port-id: original=%d, id=%d\n",
> +  port_id->original, port_id->id);
> +} else {
> +ds_put_cstr(s, "  Port-id = null\n");
> +}
>} else {
>ds_put_format(s, "unknown rte flow action (%d)\n", 
> actions->type);
>}
> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns 
> *patterns,
>return 0;
>}
>
> +static void
> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
> +{
> +struct rte_flow_action_count *count = xzalloc(sizeof *count);
> +
> +count->shared = 0;
> +/* Each flow has a single count action, so no need of id */
> +count->id = 0;
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
> +}
> +
> +static void
> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
> +struct netdev *outdev)
> +{
> +struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
> +
> +port_id->id = netdev_dpdk_get_port_id(outdev);
> +add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
> +}
> +
> +static int
> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
> +   const struct nlattr *nla,
> +   struct offload_info *info)
> +{
> +struct netdev *outdev;
> +odp_port_t port;
> +
> +port = nl_attr_get_odp_port(nla);
> +outdev = netdev_ports_get(port, info->dpif_class);
> +if (outdev == NULL) {
> +VLOG_DBG_RL(&error_rl,
> +"Cannot find netdev for odp port %d", port);
> +return -1;
> +}
> +if (!netdev_dpdk_flow_api_supported(outdev)) {
 This doesn't look correct.  I mean, maybe we need to check if port is
 really the port on a same piece of hardware.  Will the flow setup fail
 in this case?  Will code fallback to the MARK+RSS?
>>> We cannot check if the port is on the same HW, and it is also wrong from
>>> the application point of view. If the operation cannot be supported, it
>>> is in the driver (DPDK) scope to fail it.
>> BTW, function netdev_dpdk_flow_api_supported() is not intended to be called
>> in the offloading process.  It's only for initialization phase.  You can see
>> the "/* TODO: Check if we able to offload some minimal flow. */" in the code
>> and that might be destructive and unwanted for offloading process.
> 
> Actually, there is no need to call it at all, as we get here only if we 
> found the netdev by netdev_ports_get, which means we found a device that 
> can have offloads

Re: [ovs-dev] [PATCH] dpif-netdev: Use netdev-offload API for port

[Ilya Maximets]

> Acked-by: Eli Britstein 

Thanks. Assuming this was sent for the patch
"dpif-netdev: Use netdev-offload API for port lookup while offloading.".

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Contratación de mandos operativos y medios

[Cierre de Inscripciones]

11 de Diciembre | Horario de 10:00 a 17:00 hrs.  |  (hora del centro de México) 

- Contratación eficiente de mandos operativos y medios - 


Una empresa puede tener las mejores instalaciones, los mejores instrumentos de 
trabajo, incluso, lo mejores directivos, pero le haría falta un elemento 
esencial: la fuerza laboral aquellas personas que día a día hacen realidad los 
objetivos estratégicos de una organización, ponen en marcha los
planes a través de acciones concretas, es decir, están en la operación.

¿Qué aprenderás?:

- El participante conocerá el panorama actual del reclutamiento y contratación 
dentro de las organizaciones.
- El participante conocerá las herramientas de reclutamiento y selección 
másutilizadas para los puestos
operativos y mandos medios.
- El participante conocerá las tendencias de Recursos Humanos 3.0, así como 
parala Gestión del personal.
- El participante obtendrá herramientas prácticas para trabajar la 
incorporación de talento humano de diferentes niveles.

Solicita información respondiendo a este correo con la palabra Operativosn los 
siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Dirigido a: Ejecutivos, jefes, gerentes y directores de RRHH, Administración, 
Desarrollo
Humano o toda área relacionada con el manejo de personal. Profesionales y 
empresarios con
personal a su cargo.

Números de Atención: 

(045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder con la palabra baja o 
enviar un correo a bajas@ innovalearn.net



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

[ovs-dev] [PATCH] dpdk: Support running PMD threads on cores > RTE_MAX_LCORE.

[David Marchand]

Most DPDK components make the assumption that rte_lcore_id() returns
a valid lcore_id in [0..RTE_MAX_LCORE] range (with the exception of
the LCORE_ID_ANY special value).

OVS does not currently check which value is set in
RTE_PER_LCORE(_lcore_id) which exposes us to potential crashes on DPDK
side.

Introduce a lcore allocator in OVS for PMD threads and map them to
unused lcores from DPDK à la --lcores.

The physical cores on which the PMD threads are running still
constitutes an important information when debugging, so still keep
those in the PMD thread names but add a new debug log when starting
them.

Synchronize DPDK internals on numa and cpuset for the PMD threads by
registering them via the rte_thread_set_affinity() helper.

Signed-off-by: David Marchand 
---
 lib/dpdk-stub.c   |  8 +-
 lib/dpdk.c| 69 +++
 lib/dpdk.h|  3 ++-
 lib/dpif-netdev.c |  3 ++-
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
index c332c217c..90473bc8e 100644
--- a/lib/dpdk-stub.c
+++ b/lib/dpdk-stub.c
@@ -39,7 +39,13 @@ dpdk_init(const struct smap *ovs_other_config)
 }
 
 void
-dpdk_set_lcore_id(unsigned cpu OVS_UNUSED)
+dpdk_init_thread_context(unsigned cpu OVS_UNUSED)
+{
+/* Nothing */
+}
+
+void
+dpdk_uninit_thread_context(void)
 {
 /* Nothing */
 }
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 21dd47e80..771baa413 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -33,6 +33,7 @@
 
 #include "dirs.h"
 #include "fatal-signal.h"
+#include "id-pool.h"
 #include "netdev-dpdk.h"
 #include "netdev-offload-provider.h"
 #include "openvswitch/dynamic-string.h"
@@ -55,6 +56,9 @@ static bool dpdk_initialized = false; /* Indicates successful 
initialization
* of DPDK. */
 static bool per_port_memory = false; /* Status of per port memory support */
 
+static struct id_pool *lcore_id_pool;
+static struct ovs_mutex lcore_id_pool_mutex = OVS_MUTEX_INITIALIZER;
+
 static int
 process_vhost_flags(char *flag, const char *default_val, int size,
 const struct smap *ovs_other_config,
@@ -346,7 +350,8 @@ dpdk_init__(const struct smap *ovs_other_config)
 }
 }
 
-if (args_contains(&args, "-c") || args_contains(&args, "-l")) {
+if (args_contains(&args, "-c") || args_contains(&args, "-l") ||
+args_contains(&args, "--lcores")) {
 auto_determine = false;
 }
 
@@ -372,8 +377,8 @@ dpdk_init__(const struct smap *ovs_other_config)
  * thread affintity - default to core #0 */
 VLOG_ERR("Thread getaffinity failed. Using core #0");
 }
-svec_add(&args, "-l");
-svec_add_nocopy(&args, xasprintf("%d", cpu));
+svec_add(&args, "--lcores");
+svec_add_nocopy(&args, xasprintf("0@%d", cpu));
 }
 
 svec_terminate(&args);
@@ -429,6 +434,23 @@ dpdk_init__(const struct smap *ovs_other_config)
 }
 }
 
+ovs_mutex_lock(&lcore_id_pool_mutex);
+lcore_id_pool = id_pool_create(0, RTE_MAX_LCORE);
+/* Empty the whole pool... */
+for (uint32_t lcore = 0; lcore < RTE_MAX_LCORE; lcore++) {
+uint32_t lcore_id;
+
+id_pool_alloc_id(lcore_id_pool, &lcore_id);
+}
+/* ...and release the unused spots. */
+for (uint32_t lcore = 0; lcore < RTE_MAX_LCORE; lcore++) {
+if (rte_eal_lcore_role(lcore) != ROLE_OFF) {
+ continue;
+}
+id_pool_free_id(lcore_id_pool, lcore);
+}
+ovs_mutex_unlock(&lcore_id_pool_mutex);
+
 /* We are called from the main thread here */
 RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
 
@@ -522,11 +544,48 @@ dpdk_available(void)
 }
 
 void
-dpdk_set_lcore_id(unsigned cpu)
+dpdk_init_thread_context(unsigned cpu)
 {
+cpu_set_t cpuset;
+unsigned lcore;
+int err;
+
 /* NON_PMD_CORE_ID is reserved for use by non pmd threads. */
 ovs_assert(cpu != NON_PMD_CORE_ID);
-RTE_PER_LCORE(_lcore_id) = cpu;
+
+ovs_mutex_lock(&lcore_id_pool_mutex);
+if (lcore_id_pool == NULL || !id_pool_alloc_id(lcore_id_pool, &lcore)) {
+lcore = NON_PMD_CORE_ID;
+}
+ovs_mutex_unlock(&lcore_id_pool_mutex);
+
+RTE_PER_LCORE(_lcore_id) = lcore;
+
+/* DPDK is not initialised, nothing more to do. */
+if (lcore == NON_PMD_CORE_ID) {
+return;
+}
+
+CPU_ZERO(&cpuset);
+err = pthread_getaffinity_np(pthread_self(), sizeof cpuset, &cpuset);
+if (err) {
+VLOG_ABORT("Thread getaffinity error: %s", ovs_strerror(err));
+}
+
+rte_thread_set_affinity(&cpuset);
+VLOG_INFO("Initialised lcore %u for core %u", lcore, cpu);
+}
+
+void
+dpdk_uninit_thread_context(void)
+{
+if (RTE_PER_LCORE(_lcore_id) == NON_PMD_CORE_ID) {
+return;
+}
+
+ovs_mutex_lock(&lcore_id_pool_mutex);
+id_pool_free_id(lcore_id_pool, RTE_PER_LCORE(_lcore_id));
+ovs_mutex_unlock(&lcore_id_pool_mutex);
 }
 
 void
diff --git a/lib/dpdk.h b/lib/dpdk.

[ovs-dev] [PATCH ovn] ovn-controller: Add command to trigger an I-P full recompute.

[Dumitru Ceara]

Incremental processing tries to minimize the number of times
ovn-controller has to fully reprocess the contents of the southbound
database. However, if a bug in the I-P code causes ovn-controller to
end up in an inconsistent state, we have no easy way to force a full
recalculation of the openflow entries.

This commit adds a new command to ovn-controller, "recompute", which
allows users to force a full recompute of the database. It can be
triggered by the user in the following way:

ovn-appctl -t ovn-controller recompute

Signed-off-by: Dumitru Ceara 
---
 controller/ovn-controller.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index c56190f..04d9dea 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -73,6 +73,7 @@ static unixctl_cb_func meter_table_list;
 static unixctl_cb_func group_table_list;
 static unixctl_cb_func inject_pkt;
 static unixctl_cb_func ovn_controller_conn_show;
+static unixctl_cb_func engine_recompute_cmd;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
@@ -1941,6 +1942,9 @@ main(int argc, char *argv[])
 unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
  &pending_pkt);
 
+unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
+ NULL);
+
 uint64_t engine_run_id = 0;
 bool engine_run_done = true;
 
@@ -2442,3 +2446,13 @@ ovn_controller_conn_show(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
 }
 unixctl_command_reply(conn, result);
 }
+
+static void
+engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
+ const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
+{
+VLOG_INFO("User triggered force recompute.");
+engine_set_force_recompute(true);
+poll_immediate_wake();
+unixctl_command_reply(conn, NULL);
+}
-- 
1.8.3.1

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

[ovs-dev] [RFC PATCH] netdev-dpdk: Narrow down txq critical section.

[David Marchand]

tx_lock protects the NIC/vhost queue from concurrent access.
Move it closer to the parts it protects and let packets duplication (when
source is not DPDK) and the egress policer run out of it.

Signed-off-by: David Marchand 
---
I caught this by code review, but I imagine this could make the
contention on the tx lock even worse if broadcasting packets.

---
 lib/netdev-dpdk.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4c9f122b0..9dd43f2a9 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2053,10 +2053,15 @@ netdev_dpdk_rxq_dealloc(struct netdev_rxq *rxq)
  * Returns the number of packets that weren't transmitted. */
 static inline int
 netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid,
- struct rte_mbuf **pkts, int cnt)
+ struct rte_mbuf **pkts, int cnt, bool concurrent_txq)
 {
 uint32_t nb_tx = 0;
 
+if (OVS_UNLIKELY(concurrent_txq)) {
+qid = qid % dev->up.n_txq;
+rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
+}
+
 while (nb_tx != cnt) {
 uint32_t ret;
 
@@ -2068,6 +2073,10 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int 
qid,
 nb_tx += ret;
 }
 
+if (OVS_UNLIKELY(concurrent_txq)) {
+rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
+}
+
 if (OVS_UNLIKELY(nb_tx != cnt)) {
 /* Free buffers, which we couldn't transmit, one at a time (each
  * packet could come from a different mempool) */
@@ -2412,11 +2421,6 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 goto out;
 }
 
-if (OVS_UNLIKELY(!rte_spinlock_trylock(&dev->tx_q[qid].tx_lock))) {
-COVERAGE_INC(vhost_tx_contention);
-rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
-}
-
 cnt = netdev_dpdk_filter_packet_len(dev, cur_pkts, cnt);
 sw_stats_add.tx_mtu_exceeded_drops = total_packets - cnt;
 
@@ -2427,6 +2431,11 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 
 n_packets_to_free = cnt;
 
+if (OVS_UNLIKELY(!rte_spinlock_trylock(&dev->tx_q[qid].tx_lock))) {
+COVERAGE_INC(vhost_tx_contention);
+rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
+}
+
 do {
 int vhost_qid = qid * VIRTIO_QNUM + VIRTIO_RXQ;
 unsigned int tx_pkts;
@@ -2468,7 +2477,8 @@ out:
 
 /* Tx function. Transmit packets indefinitely */
 static void
-dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch)
+dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch,
+bool concurrent_txq)
 OVS_NO_THREAD_SAFETY_ANALYSIS
 {
 const size_t batch_cnt = dp_packet_batch_size(batch);
@@ -2527,7 +2537,8 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct 
dp_packet_batch *batch)
 __netdev_dpdk_vhost_send(netdev, qid, (struct dp_packet **) pkts,
  txcnt);
 } else {
-tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, txcnt);
+tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, txcnt,
+  concurrent_txq);
 }
 }
 
@@ -2549,7 +2560,7 @@ netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 {
 
 if (OVS_UNLIKELY(batch->packets[0]->source != DPBUF_DPDK)) {
-dpdk_do_tx_copy(netdev, qid, batch);
+dpdk_do_tx_copy(netdev, qid, batch, true);
 dp_packet_delete_batch(batch, true);
 } else {
 __netdev_dpdk_vhost_send(netdev, qid, batch->packets,
@@ -2568,15 +2579,10 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
 return;
 }
 
-if (OVS_UNLIKELY(concurrent_txq)) {
-qid = qid % dev->up.n_txq;
-rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
-}
-
 if (OVS_UNLIKELY(batch->packets[0]->source != DPBUF_DPDK)) {
 struct netdev *netdev = &dev->up;
 
-dpdk_do_tx_copy(netdev, qid, batch);
+dpdk_do_tx_copy(netdev, qid, batch, concurrent_txq);
 dp_packet_delete_batch(batch, true);
 } else {
 struct netdev_dpdk_sw_stats *sw_stats = dev->sw_stats;
@@ -2591,7 +2597,8 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
 tx_cnt = netdev_dpdk_qos_run(dev, pkts, tx_cnt, true);
 qos_drops -= tx_cnt;
 
-tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, tx_cnt);
+tx_failure = netdev_dpdk_eth_tx_burst(dev, qid, pkts, tx_cnt,
+  concurrent_txq);
 
 dropped = tx_failure + mtu_drops + qos_drops;
 if (OVS_UNLIKELY(dropped)) {
@@ -2603,10 +2610,6 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
 rte_spinlock_unlock(&dev->stats_lock);
 }
 }
-
-if (OVS_UNLIKELY(concurrent_txq)) {
-rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
-}
 }
 
 static int
-- 
2.23.0

__

Re: [ovs-dev] [PATCH ovn] ovn-controller: Add command to trigger an I-P full recompute.

[Daniel Alvarez Sanchez]

This is very handy! Can you please add the command to [0]?

[0]
https://github.com/ovn-org/ovn/blob/master/controller/ovn-controller.8.xml#L403

On Mon, Dec 2, 2019 at 5:19 PM Dumitru Ceara  wrote:

> Incremental processing tries to minimize the number of times
> ovn-controller has to fully reprocess the contents of the southbound
> database. However, if a bug in the I-P code causes ovn-controller to
> end up in an inconsistent state, we have no easy way to force a full
> recalculation of the openflow entries.
>
> This commit adds a new command to ovn-controller, "recompute", which
> allows users to force a full recompute of the database. It can be
> triggered by the user in the following way:
>
> ovn-appctl -t ovn-controller recompute
>
> Signed-off-by: Dumitru Ceara 
> ---
>  controller/ovn-controller.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index c56190f..04d9dea 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -73,6 +73,7 @@ static unixctl_cb_func meter_table_list;
>  static unixctl_cb_func group_table_list;
>  static unixctl_cb_func inject_pkt;
>  static unixctl_cb_func ovn_controller_conn_show;
> +static unixctl_cb_func engine_recompute_cmd;
>
>  #define DEFAULT_BRIDGE_NAME "br-int"
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> @@ -1941,6 +1942,9 @@ main(int argc, char *argv[])
>  unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
>   &pending_pkt);
>
> +unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
> + NULL);
> +
>  uint64_t engine_run_id = 0;
>  bool engine_run_done = true;
>
> @@ -2442,3 +2446,13 @@ ovn_controller_conn_show(struct unixctl_conn *conn,
> int argc OVS_UNUSED,
>  }
>  unixctl_command_reply(conn, result);
>  }
> +
> +static void
> +engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc
> OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
> +{
> +VLOG_INFO("User triggered force recompute.");
> +engine_set_force_recompute(true);
> +poll_immediate_wake();
> +unixctl_command_reply(conn, NULL);
> +}
> --
> 1.8.3.1
>

Reviewed-by: Daniel Alvarez 

>
> ___
> 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 ovn] ovn-controller: Add missing port group lflow references.

[Daniel Alvarez Sanchez]

Thanks for this patch. This can be a security issue as ACLs applied to a
Port Group may not be taking effect. Tested this patch on an OpenStack
environment that recreated the issue and I confirm that it fixes the
problem.

On Mon, Dec 2, 2019 at 1:40 PM Dumitru Ceara  wrote:

> The commit that adds incremental processing for port-group changes
> doesn't store logical flow references for port groups. If a port group
> is updated (e.g., a port is added) no logical flow recalculation will be
> performed.
>
> To fix this, when parsing the flow expression also store the referenced
> port groups and bind them to the logical flows that depend on them. If
> the port group is updated then the logical flows referring them will
> also be reinstalled.
>
> Reported-by: Daniel Alvarez 
> Reported-at: https://bugzilla.redhat.com/1778164
> CC: Han Zhou 
> Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for
> port-group changes.")
> Signed-off-by: Dumitru Ceara 
>
> Change-Id: Id39695f022f16b598fd8416cd2494e7dab3bf11b
> ---
>  controller/lflow.c|  9 -
>  include/ovn/expr.h|  4 +++-
>  lib/actions.c |  4 ++--
>  lib/expr.c| 24 +---
>  tests/test-ovn.c  |  8 
>  utilities/ovn-trace.c |  2 +-
>  6 files changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 36150bd..a689320 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -616,14 +616,21 @@ consider_logical_flow(
>  struct expr *expr;
>
>  struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
> +struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
>  expr = expr_parse_string(lflow->match, &symtab, addr_sets,
> port_groups,
> - &addr_sets_ref, &error);
> + &addr_sets_ref, &port_groups_ref, &error);
>  const char *addr_set_name;
>  SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
>  lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
> &lflow->header_.uuid);
>  }
> +const char *port_group_name;
> +SSET_FOR_EACH (port_group_name, &port_groups_ref) {
> +lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
> +   &lflow->header_.uuid);
> +}
>  sset_destroy(&addr_sets_ref);
> +sset_destroy(&port_groups_ref);
>
>  if (!error) {
>  if (prereqs) {
> diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> index 22f633e..21bf51c 100644
> --- a/include/ovn/expr.h
> +++ b/include/ovn/expr.h
> @@ -390,11 +390,13 @@ void expr_print(const struct expr *);
>  struct expr *expr_parse(struct lexer *, const struct shash *symtab,
>  const struct shash *addr_sets,
>  const struct shash *port_groups,
> -struct sset *addr_sets_ref);
> +struct sset *addr_sets_ref,
> +struct sset *port_groups_ref);
>  struct expr *expr_parse_string(const char *, const struct shash *symtab,
> const struct shash *addr_sets,
> const struct shash *port_groups,
> struct sset *addr_sets_ref,
> +   struct sset *port_groups_ref,
> char **errorp);
>
>  struct expr *expr_clone(struct expr *);
> diff --git a/lib/actions.c b/lib/actions.c
> index 586d7b7..051e6c8 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -240,8 +240,8 @@ add_prerequisite(struct action_context *ctx, const
> char *prerequisite)
>  struct expr *expr;
>  char *error;
>
> -expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
> NULL,
> - &error);
> +expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
> + NULL, NULL, &error);
>  ovs_assert(!error);
>  ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
>  }
> diff --git a/lib/expr.c b/lib/expr.c
> index 71de615..e5e4d21 100644
> --- a/lib/expr.c
> +++ b/lib/expr.c
> @@ -480,7 +480,8 @@ struct expr_context {
>  const struct shash *symtab;/* Symbol table. */
>  const struct shash *addr_sets; /* Address set table. */
>  const struct shash *port_groups; /* Port group table. */
> -struct sset *addr_sets_ref;   /* The set of address set
> referenced. */
> +struct sset *addr_sets_ref;  /* The set of address set
> referenced. */
> +struct sset *port_groups_ref;/* The set of port groups
> referenced. */
>  bool not;/* True inside odd number of NOT
> operators. */
>  unsigned int paren_depth;/* Depth of nested parentheses. */
>  };
> @@ -782,6 +783,10 @@ static bool
>  parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,
>   size_t *allocated_values)
>  {
> +   

Re: [ovs-dev] [PATCH] rhel: Support RHEL7.7 build and packaging

[Yifeng Sun]

Thanks Ben. Can you please backport this patch to 2.12?

Yifeng

On Thu, Oct 24, 2019 at 3:12 PM Ben Pfaff  wrote:
>
> On Fri, Oct 11, 2019 at 02:49:14PM -0700, Yifeng Sun wrote:
> > This patch provides essential fixes for OVS to support
> > RHEL7.7's new kernel.
> >
> > make rpm-fedora-kmod \
> > RPMBUILD_OPT='-D "kversion 3.10.0-1062.1.2.el7.x86_64"'
> >
> > Signed-off-by: Yifeng Sun 
>
> Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netdev: Use netdev-offload API for port lookup while offloading.

[Ilya Maximets]

On 02.12.2019 13:43, Ophir Munk wrote:
> 
> 
>> -Original Message-
>> From: Ilya Maximets 
>> Sent: Saturday, November 30, 2019 2:07 PM
>> To: Ophir Munk ; Ilya Maximets
>> ; ovs-dev@openvswitch.org
>> Cc: Roni Bar Yanai ; Eli Britstein
>> ; Ian Stokes ; Ameer
>> Mahagneh 
>> Subject: Re: [PATCH] dpif-netdev: Use netdev-offload API for port lookup
>> while offloading.
>>
>> On 29.11.2019 15:46, Ophir Munk wrote:
>>> Hi Ilya,
>>> The first two RFC series are confirmed. They require the third RFC series to
>> enable user space vport offloading.
>>>
>>> 1.
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>>
>> hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D14320
>> 9&a
>>>
>> mp;data=02%7C01%7Cophirmu%40mellanox.com%7C0d4c9b5abcef4d0796b5
>> 08d7758
>>>
>> ddfc7%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371071245400
>> 95619&
>>>
>> amp;sdata=OyBtcrlldUxUPZTNy6V3zArvjas1cIAA6EzBjkcXJsY%3D&reser
>> ved=
>>> 0
>>> ("netdev-offload: Prerequisites of vport offloading via DPDK")
>>>
>>> 2.
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>>
>> hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D14455
>> 9&a
>>>
>> mp;data=02%7C01%7Cophirmu%40mellanox.com%7C0d4c9b5abcef4d0796b5
>> 08d7758
>>>
>> ddfc7%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371071245400
>> 95619&
>>>
>> amp;sdata=PBp1gVHPJoRWhTVDGzLDsi4EPRcSlbQRZeVS9K99APE%3D&
>> reserved=
>>> 0
>>> ("dpif-netdev: Use netdev-offload API for port lookup while
>>> offloading.")
>>>
>>> 3.
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>>
>> hwork.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D14482
>> 6&a
>>>
>> mp;data=02%7C01%7Cophirmu%40mellanox.com%7C0d4c9b5abcef4d0796b5
>> 08d7758
>>>
>> ddfc7%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371071245400
>> 95619&
>>>
>> amp;sdata=OoZ0OfQunKaPqjKzOhIScIfAJwR%2B1g36qZcQITvAX60%3D&
>> ;reserve
>>> d=0 ("[ovs-dev,RFC,v1] netdev-dpdk: Add flow_api support for netdev
>>> vports")
>>>
>>> Can you please merge all the three series into one patch set?
>>> Then we will be able  to test it and send our feedback.
>>
>> Hi.
>>
>> This patch (series #2) is more like a bug fix that doesn't relate to vport
>> offloading.  I'd like it to be reviewed/ACKed as a standalone fix, so I could
>> push it directly to master.  After that I could rebase series #1.
> 
> Patch (series #2) tested successfully. 
> Acked-by: Ophir Munk 

Thanks Ophir and Eli!  Applied to master.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovn-controller: Add command to trigger an I-P full recompute.

[Dumitru Ceara]

On Mon, Dec 2, 2019 at 5:48 PM Daniel Alvarez Sanchez
 wrote:
>
> This is very handy! Can you please add the command to [0]?
>
> [0] 
> https://github.com/ovn-org/ovn/blob/master/controller/ovn-controller.8.xml#L403

Thanks for the review Daniel! Yes, i'll add it to the documentation in
v2, sorry I completely forgot about it.

Regards,
Dumitru

>
> On Mon, Dec 2, 2019 at 5:19 PM Dumitru Ceara  wrote:
>>
>> Incremental processing tries to minimize the number of times
>> ovn-controller has to fully reprocess the contents of the southbound
>> database. However, if a bug in the I-P code causes ovn-controller to
>> end up in an inconsistent state, we have no easy way to force a full
>> recalculation of the openflow entries.
>>
>> This commit adds a new command to ovn-controller, "recompute", which
>> allows users to force a full recompute of the database. It can be
>> triggered by the user in the following way:
>>
>> ovn-appctl -t ovn-controller recompute
>>
>> Signed-off-by: Dumitru Ceara 
>> ---
>>  controller/ovn-controller.c | 14 ++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> index c56190f..04d9dea 100644
>> --- a/controller/ovn-controller.c
>> +++ b/controller/ovn-controller.c
>> @@ -73,6 +73,7 @@ static unixctl_cb_func meter_table_list;
>>  static unixctl_cb_func group_table_list;
>>  static unixctl_cb_func inject_pkt;
>>  static unixctl_cb_func ovn_controller_conn_show;
>> +static unixctl_cb_func engine_recompute_cmd;
>>
>>  #define DEFAULT_BRIDGE_NAME "br-int"
>>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>> @@ -1941,6 +1942,9 @@ main(int argc, char *argv[])
>>  unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
>>   &pending_pkt);
>>
>> +unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
>> + NULL);
>> +
>>  uint64_t engine_run_id = 0;
>>  bool engine_run_done = true;
>>
>> @@ -2442,3 +2446,13 @@ ovn_controller_conn_show(struct unixctl_conn *conn, 
>> int argc OVS_UNUSED,
>>  }
>>  unixctl_command_reply(conn, result);
>>  }
>> +
>> +static void
>> +engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc 
>> OVS_UNUSED,
>> + const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
>> +{
>> +VLOG_INFO("User triggered force recompute.");
>> +engine_set_force_recompute(true);
>> +poll_immediate_wake();
>> +unixctl_command_reply(conn, NULL);
>> +}
>> --
>> 1.8.3.1
>
>
> Reviewed-by: Daniel Alvarez 
>>
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>

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

[ovs-dev] [PATCH ovn v2] ovn-controller: Add command to trigger an I-P full recompute.

[Dumitru Ceara]

Incremental processing tries to minimize the number of times
ovn-controller has to fully reprocess the contents of the southbound
database. However, if a bug in the I-P code causes ovn-controller to
end up in an inconsistent state, we have no easy way to force a full
recalculation of the openflow entries.

This commit adds a new command to ovn-controller, "recompute", which
allows users to force a full recompute of the database. It can be
triggered by the user in the following way:

ovn-appctl -t ovn-controller recompute

Reviewed-by: Daniel Alvarez 
Signed-off-by: Dumitru Ceara 

---
v2:
- Add command description to manpage (suggested by Daniel).
- Add Reviewed-by tag.
---
 controller/ovn-controller.8.xml | 14 ++
 controller/ovn-controller.c | 14 ++
 2 files changed, 28 insertions(+)

diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index 780625f..a226802 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -450,6 +450,20 @@
   
 Show OVN SBDB connection status for the chassis.
   
+
+  recompute
+  
+  
+Trigger a full compute iteration in ovn-controller based
+on the contents of the Southbound database and local OVS database.
+  
+  
+This command is intended to use only in the event of a bug in the
+incremental processing engine in ovn-controller to avoid
+inconsistent states. It should therefore be used with care as full
+recomputes are cpu intensive.
+  
+  
   
 
 
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index c56190f..04d9dea 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -73,6 +73,7 @@ static unixctl_cb_func meter_table_list;
 static unixctl_cb_func group_table_list;
 static unixctl_cb_func inject_pkt;
 static unixctl_cb_func ovn_controller_conn_show;
+static unixctl_cb_func engine_recompute_cmd;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
@@ -1941,6 +1942,9 @@ main(int argc, char *argv[])
 unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
  &pending_pkt);
 
+unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
+ NULL);
+
 uint64_t engine_run_id = 0;
 bool engine_run_done = true;
 
@@ -2442,3 +2446,13 @@ ovn_controller_conn_show(struct unixctl_conn *conn, int 
argc OVS_UNUSED,
 }
 unixctl_command_reply(conn, result);
 }
+
+static void
+engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc OVS_UNUSED,
+ const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
+{
+VLOG_INFO("User triggered force recompute.");
+engine_set_force_recompute(true);
+poll_immediate_wake();
+unixctl_command_reply(conn, NULL);
+}
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH] rhel: Support RHEL7.7 build and packaging

[Ben Pfaff]

Done.

On Mon, Dec 02, 2019 at 09:15:03AM -0800, Yifeng Sun wrote:
> Thanks Ben. Can you please backport this patch to 2.12?
> 
> Yifeng
> 
> On Thu, Oct 24, 2019 at 3:12 PM Ben Pfaff  wrote:
> >
> > On Fri, Oct 11, 2019 at 02:49:14PM -0700, Yifeng Sun wrote:
> > > This patch provides essential fixes for OVS to support
> > > RHEL7.7's new kernel.
> > >
> > > make rpm-fedora-kmod \
> > > RPMBUILD_OPT='-D "kversion 3.10.0-1062.1.2.el7.x86_64"'
> > >
> > > Signed-off-by: Yifeng Sun 
> >
> > Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] rhel: Support RHEL7.7 build and packaging

[Yifeng Sun]

Thanks.

On Mon, Dec 2, 2019 at 10:42 AM Ben Pfaff  wrote:
>
> Done.
>
> On Mon, Dec 02, 2019 at 09:15:03AM -0800, Yifeng Sun wrote:
> > Thanks Ben. Can you please backport this patch to 2.12?
> >
> > Yifeng
> >
> > On Thu, Oct 24, 2019 at 3:12 PM Ben Pfaff  wrote:
> > >
> > > On Fri, Oct 11, 2019 at 02:49:14PM -0700, Yifeng Sun wrote:
> > > > This patch provides essential fixes for OVS to support
> > > > RHEL7.7's new kernel.
> > > >
> > > > make rpm-fedora-kmod \
> > > > RPMBUILD_OPT='-D "kversion 3.10.0-1062.1.2.el7.x86_64"'
> > > >
> > > > Signed-off-by: Yifeng Sun 
> > >
> > > Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v1] northd: Remove misleading warning log message

[Russell Bryant]

On Mon, Dec 2, 2019 at 3:45 AM Numan Siddique  wrote:
>
> On Mon, Dec 2, 2019 at 8:54 AM Russell Bryant  wrote:
> >
> > While debugging an ovn-kubernetes cluster, I spotted several
> > "Duplicate MAC set" warning messages in the ovn-northd log.  It looks
> > like this message was emitted from this code path by mistake, where
> > it correctly avoided assigning a duplicate MAC address.  This patch
> > turns off the warning for that case.
> >
> > Signed-off-by: Russell Bryant 
>
> Acked-by: Numan Siddique 

Thanks.  I applied this to master.


>
> Numan
>
> > ---
> >  northd/ovn-northd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index a943e1037..9f558c628 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -1395,7 +1395,7 @@ ipam_get_unused_mac(ovs_be32 ip)
> >  mac_addr_suffix = ((base_addr + i) % (MAC_ADDR_SPACE - 1)) + 1;
> >  mac64 =  eth_addr_to_uint64(mac_prefix) | mac_addr_suffix;
> >  eth_addr_from_uint64(mac64, &mac);
> > -if (!ipam_is_duplicate_mac(&mac, mac64, true)) {
> > +if (!ipam_is_duplicate_mac(&mac, mac64, false)) {
> >  break;
> >  }
> >  }
> > --
> > 2.23.0
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >



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

[ovs-dev] [patch v1] conntrack: Support zone limits.

[Darrell Ball]

Signed-off-by: Darrell Ball 
---
 Documentation/faq/releases.rst   |   4 +-
 NEWS |   1 +
 lib/conntrack-private.h  |   1 +
 lib/conntrack.c  | 142 +++
 lib/conntrack.h  |  17 +
 lib/dpif-netdev.c|  88 +++-
 tests/system-kmod-macros.at  |   7 --
 tests/system-traffic.at  |   1 -
 tests/system-userspace-macros.at |   9 ---
 9 files changed, 248 insertions(+), 22 deletions(-)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index e02dda1..6702c58 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -116,9 +116,9 @@ Q: Are all features available with all datapaths?
 FeatureLinux upstream Linux OVS tree Userspace Hyper-V
 == == == = ===
 Connection tracking 4.32.5  2.6  YES
-Conntrack Fragment Reass.   4.32.6  2.10 YES
+Conntrack Fragment Reass.   4.32.6  2.12 YES
 Conntrack Timeout Policies  5.22.12 NO   NO
-Conntrack Zone Limit4.18   2.10 NO   YES
+Conntrack Zone Limit4.18   2.10 2.13 YES
 Conntrack NAT   4.62.6  2.8  YES
 Tunnel - LISP   NO 2.11 NO   NO
 Tunnel - STTNO 2.4  NO   YES
diff --git a/NEWS b/NEWS
index 80b..17f92ba 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Post-v2.12.0
- Userspace datapath:
  * Add option to enable, disable and query TCP sequence checking in
conntrack.
+ * Add support for conntrack zone limits.
- AF_XDP:
  * New option 'use-need-wakeup' for netdev-afxdp to control enabling
of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by default
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 590f139..22823cb 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -155,6 +155,7 @@ struct conntrack {
 struct ovs_mutex ct_lock; /* Protects 2 following fields. */
 struct cmap conns OVS_GUARDED;
 struct ovs_list exp_lists[N_CT_TM] OVS_GUARDED;
+struct hmap zone_limits OVS_GUARDED;
 uint32_t hash_basis; /* Salt for hashing a connection key. */
 pthread_t clean_thread; /* Periodically cleans up connection tracker. */
 struct latch clean_thread_exit; /* To destroy the 'clean_thread'. */
diff --git a/lib/conntrack.c b/lib/conntrack.c
index df7b9fa..59e1c51 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -76,6 +76,13 @@ enum ct_alg_ctl_type {
 CT_ALG_CTL_SIP,
 };
 
+struct zone_limit {
+struct hmap_node node;
+int32_t zone;
+uint32_t limit;
+uint32_t count;
+};
+
 static bool conn_key_extract(struct conntrack *, struct dp_packet *,
  ovs_be16 dl_type, struct conn_lookup_ctx *,
  uint16_t zone);
@@ -305,6 +312,7 @@ conntrack_init(void)
 for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) {
 ovs_list_init(&ct->exp_lists[i]);
 }
+hmap_init(&ct->zone_limits);
 ovs_mutex_unlock(&ct->ct_lock);
 
 ct->hash_basis = random_uint32();
@@ -318,6 +326,111 @@ conntrack_init(void)
 return ct;
 }
 
+static uint32_t
+zone_key_hash(int32_t zone, uint32_t basis)
+{
+size_t hash = hash_int((OVS_FORCE uint32_t) zone, basis);
+return hash;
+}
+
+static struct zone_limit *
+zone_limit_lookup(struct conntrack *ct, int32_t zone)
+OVS_REQUIRES(ct->ct_lock)
+{
+uint32_t hash = zone_key_hash(zone, ct->hash_basis);
+struct zone_limit *zl;
+HMAP_FOR_EACH_WITH_HASH (zl, node, hash, &ct->zone_limits) {
+if (zl->zone == zone) {
+return zl;
+}
+}
+return NULL;
+}
+
+struct conntrack_zone_limit
+zone_limit_get(struct conntrack *ct, int32_t zone)
+{
+struct conntrack_zone_limit czl = {INVALID_ZONE, 0, 0};
+ovs_mutex_lock(&ct->ct_lock);
+struct zone_limit *zl = zone_limit_lookup(ct, zone);
+if (zl) {
+czl.zone = zl->zone;
+czl.limit = zl->limit;
+czl.count = zl->count;
+} else {
+zl = zone_limit_lookup(ct, DEFAULT_ZONE);
+if (zl) {
+czl.zone = zl->zone;
+czl.limit = zl->limit;
+czl.count = zl->count;
+}
+}
+ovs_mutex_unlock(&ct->ct_lock);
+return czl;
+}
+
+static int
+zone_limit_create(struct conntrack *ct, int32_t zone, uint32_t limit)
+OVS_REQUIRES(ct->ct_lock)
+{
+if (zone >= DEFAULT_ZONE && zone <= MAX_ZONE) {
+struct zone_limit *zl = xzalloc(sizeof *zl);
+zl->limit = limit;
+zl->zone = zone;
+uint32_t hash = zone_key_hash(zone, ct->hash_basis);
+  

Re: [ovs-dev] [PATCH v2 1/2] sparse: Get rid of obsolete rte_flow header.

[Stokes, Ian]

On 10/15/2019 1:46 PM, Stokes, Ian wrote:



On 10/3/2019 7:15 PM, David Marchand wrote:
On Thu, Oct 3, 2019 at 8:12 PM David Marchand 
 wrote:


This header had been copied to cope with issues on the dpdk side.
Now that the problems have been fixed [1], let's drop this file as it is
now out of sync with dpdk.

1: https://git.dpdk.org/dpdk/commit/?id=fbb25a3878cc

Signed-off-by: David Marchand 


Sorry, those patches are for the dpdk-latest branch... do you want me
to resend them?



No need, I can apply directly to dpdk-latest. I see this change is also 
part of DPDK 18.11.3. Would it make sense to apply this patch to OVS 
master once we update OVS to use 18.11.3?


FYI we now support DPDK 18.11.5 on master, 2.12 and 2.11 so I've applied 
this patch to those branches.


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

Re: [ovs-dev] [patch v1] conntrack: Support zone limits.

[Ben Pfaff]

On Mon, Dec 02, 2019 at 11:41:27AM -0800, Darrell Ball wrote:
> Signed-off-by: Darrell Ball 

Thanks.  I'm glad to see this code growing closer to parity with the
kernel implementation.  The implementation also looks pretty clean.

I'm appending some style suggestions.  They should not change behavior.

I do have one concern about correctness.  It looks to me that there is a
separate lookup for the zone at the time that a connection is created
(to increment the counter) and at the time it is destroyed (to decrement
the counter).  I believe that this can lead to inconsistencies.  For
example, suppose that initially a connection has no associated zone, but
at time of destruction a zone does exist.  In that case, I believe that
a counter would get decremented that was never incremented.  I think
that there are similar potential issues related to finding a specific
zone versus the default zone.

-8<--cut here-->8--

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 59e1c51c0389..c90da2b4e32e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -78,9 +78,7 @@ enum ct_alg_ctl_type {
 
 struct zone_limit {
 struct hmap_node node;
-int32_t zone;
-uint32_t limit;
-uint32_t count;
+struct conntrack_zone_limit czl;
 };
 
 static bool conn_key_extract(struct conntrack *, struct dp_packet *,
@@ -339,31 +337,30 @@ zone_limit_lookup(struct conntrack *ct, int32_t zone)
 {
 uint32_t hash = zone_key_hash(zone, ct->hash_basis);
 struct zone_limit *zl;
-HMAP_FOR_EACH_WITH_HASH (zl, node, hash, &ct->zone_limits) {
-if (zl->zone == zone) {
+HMAP_FOR_EACH_IN_BUCKET (zl, node, hash, &ct->zone_limits) {
+if (zl->czl.zone == zone) {
 return zl;
 }
 }
 return NULL;
 }
 
+static struct zone_limit *
+zone_limit_lookup_or_default(struct conntrack *ct, int32_t zone)
+OVS_REQUIRES(ct->ct_lock)
+{
+struct zone_limit *zl = zone_limit_lookup(ct, zone);
+return zl ? zl : zone_limit_lookup(ct, DEFAULT_ZONE);
+}
+
 struct conntrack_zone_limit
 zone_limit_get(struct conntrack *ct, int32_t zone)
 {
 struct conntrack_zone_limit czl = {INVALID_ZONE, 0, 0};
 ovs_mutex_lock(&ct->ct_lock);
-struct zone_limit *zl = zone_limit_lookup(ct, zone);
+struct zone_limit *zl = zone_limit_lookup_or_default(ct, zone);
 if (zl) {
-czl.zone = zl->zone;
-czl.limit = zl->limit;
-czl.count = zl->count;
-} else {
-zl = zone_limit_lookup(ct, DEFAULT_ZONE);
-if (zl) {
-czl.zone = zl->zone;
-czl.limit = zl->limit;
-czl.count = zl->count;
-}
+czl = zl->czl;
 }
 ovs_mutex_unlock(&ct->ct_lock);
 return czl;
@@ -375,8 +372,8 @@ zone_limit_create(struct conntrack *ct, int32_t zone, 
uint32_t limit)
 {
 if (zone >= DEFAULT_ZONE && zone <= MAX_ZONE) {
 struct zone_limit *zl = xzalloc(sizeof *zl);
-zl->limit = limit;
-zl->zone = zone;
+zl->czl.limit = limit;
+zl->czl.zone = zone;
 uint32_t hash = zone_key_hash(zone, ct->hash_basis);
 hmap_insert(&ct->zone_limits, &zl->node, hash);
 return 0;
@@ -392,7 +389,7 @@ zone_limit_update(struct conntrack *ct, int32_t zone, 
uint32_t limit)
 ovs_mutex_lock(&ct->ct_lock);
 struct zone_limit *zl = zone_limit_lookup(ct, zone);
 if (zl) {
-zl->limit = limit;
+zl->czl.limit = limit;
 VLOG_INFO("Changed zone limit of %u for zone %d", limit, zone);
 } else {
 err = zone_limit_create(ct, zone, limit);
@@ -444,7 +441,7 @@ conn_clean_cmn(struct conntrack *ct, struct conn *conn)
 
 struct zone_limit *zl = zone_limit_lookup(ct, conn->key.zone);
 if (zl) {
-zl->count--;
+zl->czl.count--;
 }
 }
 
@@ -984,16 +981,9 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
 return nc;
 }
 
-struct zone_limit *zl = zone_limit_lookup(ct, ctx->key.zone);
-if (zl) {
-if (zl->count >= zl->limit) {
-return nc;
-}
-} else {
-zl = zone_limit_lookup(ct, DEFAULT_ZONE);
-if (zl && zl->count >= zl->limit) {
-return nc;
-}
+struct zone_limit *zl = zone_limit_lookup_or_default(ct, 
ctx->key.zone);
+if (zl && zl->czl.count >= zl->czl.limit) {
+return nc;
 }
 
 nc = new_conn(ct, pkt, &ctx->key, now);
@@ -1055,7 +1045,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet 
*pkt,
 atomic_count_inc(&ct->n_conn);
 ctx->conn = nc; /* For completeness. */
 if (zl) {
-zl->count++;
+zl->czl.count++;
 }
 }
 
diff --git a/lib/conntrack.h b/lib/conntrack.h
index e407228054a3..71272371c7c7 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -110,7 +110,7 @@ struct conntrack_zone_limit {
 uint32_t count;
 };
 
-e

Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow

[Ben Pfaff]

On Fri, Nov 29, 2019 at 04:15:59PM +0530, Numan Siddique wrote:
> On Fri, Nov 29, 2019 at 11:44 AM Linhaifeng  wrote:
> >
> > Should use flow->actions not &flow->actions.
> >
> > here is ASAN report:
> > =
> > ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address 
> > 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 
> > READ of size 196 at 0x428fa0e8 thread T150 (revalidator22)
> > #0 0x7f61a51f in __interceptor_memcpy (/lib64/libasan.so.4+0xa251f)
> > #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426
> > #2 0xd26a30cb in ofpbuf_clone_data_with_headroom lib/ofpbuf.c:248
> > #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218
> > #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208
> > #5 0xd23e3993 in ukey_set_actions ofproto/ofproto-dpif-upcall.c:1640
> > #6 0xd23e3f03 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1696
> > #7 0xd23e553f in ukey_create_from_dpif_flow 
> > ofproto/ofproto-dpif-upcall.c:1806
> > #8 0xd23e65fb in ukey_acquire ofproto/ofproto-dpif-upcall.c:1984
> > #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625
> > #10 0xd23dee5f in udpif_revalidator 
> > ofproto/ofproto-dpif-upcall.c:1076
> > #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708
> > #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb)
> > #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb)
> >
> > Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) 
> > at offset 328 in frame
> > #0 0xd23e4cab in ukey_create_from_dpif_flow 
> > ofproto/ofproto-dpif-upcall.c:1762
> >
> >   This frame has 4 object(s):
> > [32, 96) 'actions'
> > [128, 192) 'buf'
> > [224, 328) 'full_flow'
> > [384, 2432) 'stub' <== Memory access at offset 328 partially underflows 
> > this variable
> > HINT: this may be a false positive if your program uses some custom stack 
> > unwind mechanism or swapcontext
> >   (longjmp and C++ exceptions *are* supported) Thread T150 
> > (revalidator22) created by T0 here:
> > #0 0x7f5b0f7f in __interceptor_pthread_create 
> > (/lib64/libasan.so.4+0x38f7f)
> > #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792
> > #2 0xd23dc62f in udpif_start_threads 
> > ofproto/ofproto-dpif-upcall.c:639
> > #3 0xd23daf87 in ofproto_set_flow_table 
> > ofproto/ofproto-dpif-upcall.c:446
> > #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134
> > #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148
> > #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944
> > #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240
> > #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf)
> > #9 0xd230a3d3  
> > (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3)
> >
> > SUMMARY: AddressSanitizer: stack-buffer-overflow 
> > (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around 
> > the buggy address:
> >   0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00
> >   0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >   0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >   0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
> >   0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2
> > =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2
> >   0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
> >   0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >   0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >   0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >   0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow 
> > byte legend (one shadow byte represents 8 application bytes):
> >   Addressable:   00
> >   Partially addressable: 01 02 03 04 05 06 07
> >   Heap left redzone:   fa
> >   Freed heap region:   fd
> >   Stack left redzone:  f1
> >   Stack mid redzone:   f2
> >   Stack right redzone: f3
> >   Stack after return:  f5
> >   Stack use after scope:   f8
> >   Global redzone:  f9
> >   Global init order:   f6
> >   Poisoned by user:f7
> >   Container overflow:  fc
> >   Array cookie:ac
> >   Intra object redzone:bb
> >   ASan internal:   fe
> >   Left alloca redzone: ca
> >   Right alloca redzone:cb
> > ==57189==ABORTING
> >
> > Signed-off-by: Linhaifeng 
> 
> Acked-by: Numan Siddique 

Linhaifeng, this is a great catch.  Thank you.  I applied this to master
and backported it.

I also checked through the source tree for other, similar issues.  I did
not find any.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Documentation: Convert multiple manpages to ReST.

[Ben Pfaff]

On Thu, Nov 28, 2019 at 12:57:16PM +0530, Numan Siddique wrote:
> On Thu, Nov 28, 2019 at 12:49 PM Numan Siddique  wrote:
> >
> > On Wed, Nov 27, 2019 at 4:10 AM Ben Pfaff  wrote:
> > >
> > > On Thu, Oct 10, 2019 at 02:29:42PM -0700, Ben Pfaff wrote:
> > > > Signed-off-by: Ben Pfaff 
> > >
> > > Still needs review.
> >
> > Hi Ben,
> >
> > Can you please rebase this patch. It doesn't apply.
> 
> I was able to apply this patch to an older commit.
> 
> Tested-by: Numan Siddique 
> Acked-by: Numan Siddique 

Thanks.  I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] debian: Update list of copyright holders.

[Ben Pfaff]

On Wed, Nov 27, 2019 at 10:25:36AM -0800, Yi-Hung Wei wrote:
> On Wed, Oct 9, 2019 at 10:35 AM Ben Pfaff  wrote:
> >
> > The list of copyright holders was incomplete and out of date.  This
> > updates it based on a "grep" for copyright notices, which I reviewed by
> > hand.
> >
> > CC: 942...@bugs.debian.org
> > Reported-by: Chris Lamb 
> > Reported-at: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=942056
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> LGTM.
> 
> Acked-by: Yi-Hung Wei 

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

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Restore table ID on error in xlate_table_action().

[Ben Pfaff]

On Wed, Nov 27, 2019 at 10:04:24AM -0800, Yi-Hung Wei wrote:
> On Mon, Oct 14, 2019 at 4:33 PM Ben Pfaff  wrote:
> >
> > Found by inspection.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> >  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 f92cb62c80ce..0fa5d8a7c61b 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -4369,6 +4369,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t 
> > in_port, uint8_t table_id,
> >  !is_ip_any(&ctx->xin->flow)) {
> >  xlate_report_error(ctx,
> > "resubmit(ct) with non-tracked or 
> > non-IP packet!");
> > +ctx->table_id = old_table_id;
> >  return;
> >  }
> >  tuple_swap(&ctx->xin->flow, ctx->wc);
> > --
> 
> Thanks for the patch. It looks good to me.
> 
> Acked-by: Yi-Hung Wei 

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

Re: [ovs-dev] [PATCH v2 1/2] sparse: Get rid of obsolete rte_flow header.

[David Marchand]

On Mon, Dec 2, 2019 at 9:19 PM Stokes, Ian  wrote:
> >>> This header had been copied to cope with issues on the dpdk side.
> >>> Now that the problems have been fixed [1], let's drop this file as it is
> >>> now out of sync with dpdk.
> >>>
> >>> 1: https://git.dpdk.org/dpdk/commit/?id=fbb25a3878cc
> >>>
> >>> Signed-off-by: David Marchand 
> FYI we now support DPDK 18.11.5 on master, 2.12 and 2.11 so I've applied
> this patch to those branches.

Thanks Ian.


--
David Marchand

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

[ovs-dev] Congratulation

[Nations]

Address: Ring Rd E, Accra
Hours: Closed · Opens 7:30AM
Phone: 30 221 5665

Attention:

Congratulation! United Nations has approved Two Million Dollars ($2m) 
compensation for you for more details contact Mr.John Guaranty Trust Bank 
Director West Africa Ghana. with this email :jduggan...@gmail.com  

Thanks
Antуnio Guterres
Un Secretary General
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] New Purchase order

[De Bruyn, Jacques La Mont]

Find attached copy of new purchase order.

 

Regards,

Jacques De Bruyn  |  Specialist - Procurement & Purchasing  |  +27 11 741 3990
Timken South Africa PTY Ltd  |  Mail Code: TSA-01  |  Cnr Great North Road and 
Elgin Street

Pomona, Kempton Park | Johannesburg, Gauteng,  1619 |  Stronger. By Design.



 

This message and any attachments are intended for the individual or entity 
named above. If you are not the intended recipient, please do not forward, 
copy, print, use or disclose this communication to others; also please notify 
the sender by replying to this message, and then delete it from your system. 
The Timken Company / The Timken Corporation
This message and any attachments are intended for the individual or entity 
named above. If you are not the intended recipient, please do not forward, 
copy, print, use or disclose this com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCHv3] userspace: Add GTP-U support.

[William Tu]

GTP, GPRS Tunneling Protocol, is a group of IP-based communications
protocols used to carry general packet radio service (GPRS) within
GSM, UMTS and LTE networks.  GTP protocol has two parts: Signalling
(GTP-Control, GTP-C) and User data (GTP-User, GTP-U). GTP-C is used
for setting up GTP-U protocol, which is an IP-in-UDP tunneling
protocol. Usually GTP is used in connecting between base station for
radio, Serving Gateway (S-GW), and PDN Gateway (P-GW).

This patch implements GTP-U protocol for userspace datapath,
supporting only required header fields and G-PDU message type.
See spec in:
https://tools.ietf.org/html/draft-hmm-dmm-5g-uplane-analysis-00

Signed-off-by: Yi Yang 
Co-authored-by: Yi Yang 
Signed-off-by: William Tu 

---
v2 -> v3:
  - pick up the code from v2, rebase to master
  - many fixes in code, docs, and add more tests
  - travis: https://travis-ci.org/williamtu/ovs-travis/builds/619799361

v1 -> v2:
  - Add new packet_type PT_GTPU_MSG to handle GTP-U signaling message,
GTP-U signaling message should be steered into a control plane handler
by explicit openflow entry in flow table.
  - Fix gtpu_flags and gptu_msgtype set issue.
  - Verify communication with kernel GTP implementation from Jiannan
Ouyang.
  - Fix unit test issue and make sure all the unit tests can pass.
  - This patch series add GTP-U tunnel support in DPDK userspace,
GTP-U is used for carrying user data within the GPRS core network and
between the radio access network and the core network. The user data
transported can be packets in any of IPv4, IPv6, or PPP formats.
---
 Documentation/faq/configuration.rst   |  13 +++
 Documentation/faq/releases.rst|   1 +
 NEWS  |   3 +
 datapath/linux/compat/include/linux/openvswitch.h |   2 +
 include/openvswitch/match.h   |   6 ++
 include/openvswitch/meta-flow.h   |  28 ++
 include/openvswitch/packets.h |   4 +-
 lib/dpif-netlink-rtnl.c   |   5 +
 lib/dpif-netlink.c|   5 +
 lib/match.c   |  28 ++
 lib/meta-flow.c   |  38 
 lib/meta-flow.xml |  75 ++-
 lib/netdev-native-tnl.c   |  85 +
 lib/netdev-native-tnl.h   |   8 ++
 lib/netdev-vport.c|  25 -
 lib/nx-match.c|   6 ++
 lib/odp-util.c| 108 ++
 lib/packets.h |  59 
 lib/tnl-ports.c   |   3 +
 ofproto/ofproto-dpif-xlate.c  |   4 +-
 ofproto/tunnel.c  |   3 +-
 tests/ofproto.at  |   4 +-
 tests/tunnel-push-pop.at  |  22 +
 tests/tunnel.at   |  78 
 vswitchd/vswitch.xml  |  18 
 25 files changed, 624 insertions(+), 7 deletions(-)

diff --git a/Documentation/faq/configuration.rst 
b/Documentation/faq/configuration.rst
index ff3b71a5d4ef..a9ac9354d0dc 100644
--- a/Documentation/faq/configuration.rst
+++ b/Documentation/faq/configuration.rst
@@ -225,6 +225,19 @@ Q: Does Open vSwitch support IPv6 GRE?
 options:remote_ip=fc00:100::1 \
 options:packet_type=legacy_l2
 
+Q: Does Open vSwitch support GTP-U?
+
+A: Yes. GTP-U (GPRS Tunnelling Protocol User Plane (GTPv1-U))
+is supported in userspace datapath. TEID is set by using tunnel
+key field.
+
+::
+
+$ ovs-vsctl add-br br0
+$ ovs-vsctl add-port br0 gtpu0 -- \
+set int gtpu0 type=gtpu options:key= \
+options:remote_ip=172.31.1.1
+
 Q: How do I connect two bridges?
 
 A: First, why do you want to do this?  Two connected bridges are not much
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index e02dda150a34..a181bc39ab7a 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -130,6 +130,7 @@ Q: Are all features available with all datapaths?
 Tunnel - Geneve-IPv64.42.6  2.6  NO
 Tunnel - ERSPAN 4.18   2.10 2.10 NO
 Tunnel - ERSPAN-IPv64.18   2.10 2.10 NO
+Tunnel - GTP-U  NO NO   2.13 NO
 QoS - Policing  YES1.1  2.6  NO
 QoS - Shaping   YES1.1  NO   NO
 sFlow   YES1.0  1.0  NO
diff --git a/NEWS b/NEWS
index 80b1a2ca..0e079b1cce2f 100644
--- a/NEWS
+++ b/N

[ovs-dev] [PATCH] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.

[Ben Pfaff]

Requested-by: Leonid Ryzhyk 
Signed-off-by: Ben Pfaff 
---
RFC->v1: Rebase.

 Documentation/ref/ovsdb-server.7.rst |  9 +
 NEWS |  1 +
 ovsdb/execution.c| 26 ++
 ovsdb/transaction.c  | 22 +-
 ovsdb/transaction.h  |  5 -
 tests/ovsdb-execution.at | 15 +++
 tests/uuidfilt.py| 18 --
 7 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/Documentation/ref/ovsdb-server.7.rst 
b/Documentation/ref/ovsdb-server.7.rst
index bc533611cb4a..967761bdfb84 100644
--- a/Documentation/ref/ovsdb-server.7.rst
+++ b/Documentation/ref/ovsdb-server.7.rst
@@ -546,6 +546,15 @@ condition can be either a 3-element JSON array as 
described in the RFC or a
 boolean value. In case of an empty array an implicit true boolean value will be
 considered.
 
+5.2.1 Insert
+
+
+As an extension, Open vSwitch 2.13 and later allow an optional ``uuid`` member
+to specify the UUID for the new row.  The specified UUID must be unique within
+the table when it is inserted and not the UUID of a row previously deleted
+within the transaction.  If the UUID violates these rules, then the operation
+fails with a ``duplicate uuid`` error.
+
 5.2.6 Wait, 5.2.7 Commit, 5.2.9 Comment
 ---
 
diff --git a/NEWS b/NEWS
index 80b1a2ca..94261f8410a6 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.12.0
releases.
  * OVS validated with DPDK 18.11.5, due to the inclusion of a fix for
CVE-2019-14818, this DPDK version is strongly recommended to be used.
+   - ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
 
 v2.12.0 - 03 Sep 2019
 -
diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index f2cf3d72d45f..e45f3d6796a7 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2017, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -329,11 +329,12 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct 
ovsdb_parser *parser,
 {
 struct ovsdb_table *table;
 struct ovsdb_row *row = NULL;
-const struct json *uuid_name, *row_json;
+const struct json *uuid_json, *uuid_name, *row_json;
 struct ovsdb_error *error;
 struct uuid row_uuid;
 
 table = parse_table(x, parser, "table");
+uuid_json = ovsdb_parser_member(parser, "uuid", OP_STRING | OP_OPTIONAL);
 uuid_name = ovsdb_parser_member(parser, "uuid-name", OP_ID | OP_OPTIONAL);
 row_json = ovsdb_parser_member(parser, "row", OP_OBJECT);
 error = ovsdb_parser_get_error(parser);
@@ -341,6 +342,19 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct 
ovsdb_parser *parser,
 return error;
 }
 
+if (uuid_json) {
+if (!uuid_from_string(&row_uuid, json_string(uuid_json))) {
+return ovsdb_syntax_error(uuid_json, NULL, "bad uuid");
+}
+
+if (!ovsdb_txn_may_create_row(table, &row_uuid)) {
+return ovsdb_syntax_error(uuid_json, "duplicate uuid",
+  "This UUID would duplicate a UUID "
+  "already present within the table or "
+  "deleted within the same transaction.");
+}
+}
+
 if (uuid_name) {
 struct ovsdb_symbol *symbol;
 
@@ -350,9 +364,13 @@ ovsdb_execute_insert(struct ovsdb_execution *x, struct 
ovsdb_parser *parser,
   "This \"uuid-name\" appeared on an "
   "earlier \"insert\" operation.");
 }
-row_uuid = symbol->uuid;
+if (uuid_json) {
+symbol->uuid = row_uuid;
+} else {
+row_uuid = symbol->uuid;
+}
 symbol->created = true;
-} else {
+} else if (!uuid_json) {
 uuid_generate(&row_uuid);
 }
 
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 866fabe8df57..369436bffbf5 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019 Nicira, 
Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -1287,6 +1287,26 @@ ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct 
ovsdb_row *row_)
 }
 }
 
+/* Returns true if 'row_uuid' may be used as the UUID for a newly created row
+ * in 'table' (that is, that it is unique within 'table'), false otherwise. */
+bool
+ovsdb_txn_may_create_row(const struct ovsdb_table *t

Re: [ovs-dev] [patch v1] faq: Correct fragment reassembly release.

[Ben Pfaff]

On Mon, Nov 25, 2019 at 06:39:34PM -0800, Darrell Ball wrote:
> Correct fragment reassembly release for the userspace datapath.
> 
> Signed-off-by: Darrell Ball 

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

Re: [ovs-dev] [PATCH] datapath: make generic netlink group const

[Ben Pfaff]

On Mon, Nov 25, 2019 at 02:20:44PM -0800, Greg Rose wrote:
> Upstream commit:
> commit 48e48a70c08a8a68f8697f8b30cb83775bda8001
> Author: stephen hemminger 
> Date:   Wed Jul 16 11:25:52 2014 -0700
> 
> openvswitch: make generic netlink group const
> 
> Generic netlink tables can be const.
> 
> Signed-off-by: Stephen Hemminger 
> Signed-off-by: David S. Miller 
> 
> The equivalent tables in meter.c and conntrack.c are constified so
> it should be safe to do the same for these and will improve
> security as well.
> 
> Original patch slightly modified for out of tree module.
> 
> Passes check-kmod.
> Passes Travis.
> https://travis-ci.org/gvrose8192/ovs-experimental/builds/616880002
> 
> Cc: Stephen Hemminger 
> Signed-off-by: Greg Rose 

Thanks, applied to (OVS) master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] ovn-controller: Add missing port group lflow references.

[Han Zhou]

On Mon, Dec 2, 2019 at 5:20 AM Dumitru Ceara  wrote:
>
> The commit that adds incremental processing for port-group changes
> doesn't store logical flow references for port groups. If a port group
> is updated (e.g., a port is added) no logical flow recalculation will be
> performed.
>
> To fix this, when parsing the flow expression also store the referenced
> port groups and bind them to the logical flows that depend on them. If
> the port group is updated then the logical flows referring them will
> also be reinstalled.
>
> Reported-by: Daniel Alvarez 
> Reported-at: https://bugzilla.redhat.com/1778164
> CC: Han Zhou 
> Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for
port-group changes.")
> Signed-off-by: Dumitru Ceara 
>
> ---
> v2: remove Change-Id from commit message.
> ---
>  controller/lflow.c|  9 -
>  include/ovn/expr.h|  4 +++-
>  lib/actions.c |  4 ++--
>  lib/expr.c| 24 +---
>  tests/test-ovn.c  |  8 
>  utilities/ovn-trace.c |  2 +-
>  6 files changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 36150bd..a689320 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -616,14 +616,21 @@ consider_logical_flow(
>  struct expr *expr;
>
>  struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
> +struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
>  expr = expr_parse_string(lflow->match, &symtab, addr_sets,
port_groups,
> - &addr_sets_ref, &error);
> + &addr_sets_ref, &port_groups_ref, &error);
>  const char *addr_set_name;
>  SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
>  lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
> &lflow->header_.uuid);
>  }
> +const char *port_group_name;
> +SSET_FOR_EACH (port_group_name, &port_groups_ref) {
> +lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
> +   &lflow->header_.uuid);
> +}
>  sset_destroy(&addr_sets_ref);
> +sset_destroy(&port_groups_ref);
>
>  if (!error) {
>  if (prereqs) {
> diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> index 22f633e..21bf51c 100644
> --- a/include/ovn/expr.h
> +++ b/include/ovn/expr.h
> @@ -390,11 +390,13 @@ void expr_print(const struct expr *);
>  struct expr *expr_parse(struct lexer *, const struct shash *symtab,
>  const struct shash *addr_sets,
>  const struct shash *port_groups,
> -struct sset *addr_sets_ref);
> +struct sset *addr_sets_ref,
> +struct sset *port_groups_ref);
>  struct expr *expr_parse_string(const char *, const struct shash *symtab,
> const struct shash *addr_sets,
> const struct shash *port_groups,
> struct sset *addr_sets_ref,
> +   struct sset *port_groups_ref,
> char **errorp);
>
>  struct expr *expr_clone(struct expr *);
> diff --git a/lib/actions.c b/lib/actions.c
> index 586d7b7..051e6c8 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -240,8 +240,8 @@ add_prerequisite(struct action_context *ctx, const
char *prerequisite)
>  struct expr *expr;
>  char *error;
>
> -expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
NULL,
> - &error);
> +expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
> + NULL, NULL, &error);
>  ovs_assert(!error);
>  ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
>  }
> diff --git a/lib/expr.c b/lib/expr.c
> index 71de615..e5e4d21 100644
> --- a/lib/expr.c
> +++ b/lib/expr.c
> @@ -480,7 +480,8 @@ struct expr_context {
>  const struct shash *symtab;/* Symbol table. */
>  const struct shash *addr_sets; /* Address set table. */
>  const struct shash *port_groups; /* Port group table. */
> -struct sset *addr_sets_ref;   /* The set of address set
referenced. */
> +struct sset *addr_sets_ref;  /* The set of address set
referenced. */
> +struct sset *port_groups_ref;/* The set of port groups
referenced. */
>  bool not;/* True inside odd number of NOT
operators. */
>  unsigned int paren_depth;/* Depth of nested parentheses. */
>  };
> @@ -782,6 +783,10 @@ static bool
>  parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,
>   size_t *allocated_values)
>  {
> +if (ctx->port_groups_ref) {
> +sset_add(ctx->port_groups_ref, ctx->lexer->token.s);
> +}
> +
>  struct expr_constant_set *port_group
>  = (ctx->port_groups
> ? shash_find_data(ctx->port_groups, ctx->lexer->

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Prevent duplicating of traffic to a mirror port

[Ben Pfaff]

On Sun, Nov 03, 2019 at 11:11:53AM +0200, Roi Dayan wrote:
> From: Dmytro Linkin 
> 
> Currently ofproto design disallow duplicating output packet on forwarding
> and mirroring to/from same ovs port. Next scenario reveal lack of design:
> 1. Send ping between regular ovs ports (VFs, for ex.), stop it.
> 2. While rule still exist, make mirror for one of the ports.
> Prevent duplicating of traffic to a mirror port.
> 
> Fixes: 86e2dcddce85 ("dpif-xlate: Snoop multicast packets and send them 
> properly")
> Signed-off-by: Dmytro Linkin 
> Acked-by: Roi Dayan 

Thanks for the patch!

I don't think that the following message is correct, because the tests
here are not concerned with the input port.  I think that this message
should be dropped:
> +if (ctx->xin->packet != NULL) {
> +xlate_report_error(ctx, "dropping packet received on 
> port %s, "
> +   "which is reserved exclusively for 
> mirroring",
> +   mac_xbundle->name);
> +}

This one might better be phrased as "learned port" rather than "output
port":

> +xlate_report(ctx, OFT_WARN,
> + "output port is a mirror port, dropping");
> +return;
> +}

Thanks,

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

Re: [ovs-dev] [PATCH v2] netdev: use acquire-release semantics for change_seq in netdev

[Ben Pfaff]

On Tue, Nov 26, 2019 at 03:35:23PM +0800, Yanqin Wei wrote:
> "rxq_enabled" of netdev is writen in the vhost thread and read by pmd
> thread once it observes 'change_seq' is updated. This patch is to keep
> order on aarch64 or other weak memory model CPU to ensure 'rxq_enabled' is
> observed before 'change_seq'.
> 
> Reviewed-by: Gavin Hu 
> Signed-off-by: Yanqin Wei 

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

Re: [ovs-dev] [PATCH ovn v2] ovn-controller: Add command to trigger an I-P full recompute.

[Han Zhou]

On Mon, Dec 2, 2019 at 10:03 AM Dumitru Ceara  wrote:
>
> Incremental processing tries to minimize the number of times
> ovn-controller has to fully reprocess the contents of the southbound
> database. However, if a bug in the I-P code causes ovn-controller to
> end up in an inconsistent state, we have no easy way to force a full
> recalculation of the openflow entries.
>
> This commit adds a new command to ovn-controller, "recompute", which
> allows users to force a full recompute of the database. It can be
> triggered by the user in the following way:
>
> ovn-appctl -t ovn-controller recompute
>
> Reviewed-by: Daniel Alvarez 
> Signed-off-by: Dumitru Ceara 
>
> ---
> v2:
> - Add command description to manpage (suggested by Daniel).
> - Add Reviewed-by tag.
> ---
>  controller/ovn-controller.8.xml | 14 ++
>  controller/ovn-controller.c | 14 ++
>  2 files changed, 28 insertions(+)
>
> diff --git a/controller/ovn-controller.8.xml
b/controller/ovn-controller.8.xml
> index 780625f..a226802 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -450,6 +450,20 @@
>
>  Show OVN SBDB connection status for the chassis.
>
> +
> +  recompute
> +  
> +  
> +Trigger a full compute iteration in ovn-controller
based
> +on the contents of the Southbound database and local OVS
database.
> +  
> +  
> +This command is intended to use only in the event of a bug in the
> +incremental processing engine in ovn-controller to
avoid
> +inconsistent states. It should therefore be used with care as
full
> +recomputes are cpu intensive.
> +  
> +  
>
>  
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index c56190f..04d9dea 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -73,6 +73,7 @@ static unixctl_cb_func meter_table_list;
>  static unixctl_cb_func group_table_list;
>  static unixctl_cb_func inject_pkt;
>  static unixctl_cb_func ovn_controller_conn_show;
> +static unixctl_cb_func engine_recompute_cmd;
>
>  #define DEFAULT_BRIDGE_NAME "br-int"
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> @@ -1941,6 +1942,9 @@ main(int argc, char *argv[])
>  unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
>   &pending_pkt);
>
> +unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
> + NULL);
> +
>  uint64_t engine_run_id = 0;
>  bool engine_run_done = true;
>
> @@ -2442,3 +2446,13 @@ ovn_controller_conn_show(struct unixctl_conn
*conn, int argc OVS_UNUSED,
>  }
>  unixctl_command_reply(conn, result);
>  }
> +
> +static void
> +engine_recompute_cmd(struct unixctl_conn *conn OVS_UNUSED, int argc
OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
> +{
> +VLOG_INFO("User triggered force recompute.");
> +engine_set_force_recompute(true);
> +poll_immediate_wake();
> +unixctl_command_reply(conn, NULL);
> +}
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Dumitru for adding this handy command. I applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] ovn-controller: Add missing port group lflow references.

[Han Zhou]

On Mon, Dec 2, 2019 at 2:38 PM Han Zhou  wrote:
>
>
>
> On Mon, Dec 2, 2019 at 5:20 AM Dumitru Ceara  wrote:
> >
> > The commit that adds incremental processing for port-group changes
> > doesn't store logical flow references for port groups. If a port group
> > is updated (e.g., a port is added) no logical flow recalculation will be
> > performed.
> >
> > To fix this, when parsing the flow expression also store the referenced
> > port groups and bind them to the logical flows that depend on them. If
> > the port group is updated then the logical flows referring them will
> > also be reinstalled.
> >
> > Reported-by: Daniel Alvarez 
> > Reported-at: https://bugzilla.redhat.com/1778164
> > CC: Han Zhou 
> > Fixes: 978f5e90af0a ("ovn-controller: Incremental processing for
port-group changes.")
> > Signed-off-by: Dumitru Ceara 
> >
> > ---
> > v2: remove Change-Id from commit message.
> > ---
> >  controller/lflow.c|  9 -
> >  include/ovn/expr.h|  4 +++-
> >  lib/actions.c |  4 ++--
> >  lib/expr.c| 24 +---
> >  tests/test-ovn.c  |  8 
> >  utilities/ovn-trace.c |  2 +-
> >  6 files changed, 35 insertions(+), 16 deletions(-)
> >
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index 36150bd..a689320 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -616,14 +616,21 @@ consider_logical_flow(
> >  struct expr *expr;
> >
> >  struct sset addr_sets_ref = SSET_INITIALIZER(&addr_sets_ref);
> > +struct sset port_groups_ref = SSET_INITIALIZER(&port_groups_ref);
> >  expr = expr_parse_string(lflow->match, &symtab, addr_sets,
port_groups,
> > - &addr_sets_ref, &error);
> > + &addr_sets_ref, &port_groups_ref, &error);
> >  const char *addr_set_name;
> >  SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
> >  lflow_resource_add(lfrr, REF_TYPE_ADDRSET, addr_set_name,
> > &lflow->header_.uuid);
> >  }
> > +const char *port_group_name;
> > +SSET_FOR_EACH (port_group_name, &port_groups_ref) {
> > +lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name,
> > +   &lflow->header_.uuid);
> > +}
> >  sset_destroy(&addr_sets_ref);
> > +sset_destroy(&port_groups_ref);
> >
> >  if (!error) {
> >  if (prereqs) {
> > diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> > index 22f633e..21bf51c 100644
> > --- a/include/ovn/expr.h
> > +++ b/include/ovn/expr.h
> > @@ -390,11 +390,13 @@ void expr_print(const struct expr *);
> >  struct expr *expr_parse(struct lexer *, const struct shash *symtab,
> >  const struct shash *addr_sets,
> >  const struct shash *port_groups,
> > -struct sset *addr_sets_ref);
> > +struct sset *addr_sets_ref,
> > +struct sset *port_groups_ref);
> >  struct expr *expr_parse_string(const char *, const struct shash
*symtab,
> > const struct shash *addr_sets,
> > const struct shash *port_groups,
> > struct sset *addr_sets_ref,
> > +   struct sset *port_groups_ref,
> > char **errorp);
> >
> >  struct expr *expr_clone(struct expr *);
> > diff --git a/lib/actions.c b/lib/actions.c
> > index 586d7b7..051e6c8 100644
> > --- a/lib/actions.c
> > +++ b/lib/actions.c
> > @@ -240,8 +240,8 @@ add_prerequisite(struct action_context *ctx, const
char *prerequisite)
> >  struct expr *expr;
> >  char *error;
> >
> > -expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL,
NULL, NULL,
> > - &error);
> > +expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
> > + NULL, NULL, &error);
> >  ovs_assert(!error);
> >  ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
> >  }
> > diff --git a/lib/expr.c b/lib/expr.c
> > index 71de615..e5e4d21 100644
> > --- a/lib/expr.c
> > +++ b/lib/expr.c
> > @@ -480,7 +480,8 @@ struct expr_context {
> >  const struct shash *symtab;/* Symbol table. */
> >  const struct shash *addr_sets; /* Address set table. */
> >  const struct shash *port_groups; /* Port group table. */
> > -struct sset *addr_sets_ref;   /* The set of address set
referenced. */
> > +struct sset *addr_sets_ref;  /* The set of address set
referenced. */
> > +struct sset *port_groups_ref;/* The set of port groups
referenced. */
> >  bool not;/* True inside odd number of NOT
operators. */
> >  unsigned int paren_depth;/* Depth of nested parentheses. */
> >  };
> > @@ -782,6 +783,10 @@ static bool
> >  parse_port_group(struct expr_context *ctx, struct expr_constant_set
*cs,
> >   size_t *allo

Re: [ovs-dev] [PATCH V3] Add offload packets statistics

[Ben Pfaff]

Simon, would you mind taking a look at this too?

On Mon, Oct 28, 2019 at 08:19:48PM +0800, zhaozhanxu wrote:
> Add argument '-m' or '--more' for command ovs-appctl bridge/dump-flows
> to display the offloaded packets statistics.
> 
> The commands display as below:
> 
> orignal command:
> 
> ovs-appctl bridge/dump-flows br0
> 
> duration=574s, n_packets=1152, n_bytes=110768, priority=0,actions=NORMAL
> table_id=254, duration=574s, n_packets=0, n_bytes=0, 
> priority=2,recirc_id=0,actions=drop
> table_id=254, duration=574s, n_packets=0, n_bytes=0, 
> priority=0,reg0=0x1,actions=controller(reason=)
> table_id=254, duration=574s, n_packets=0, n_bytes=0, 
> priority=0,reg0=0x2,actions=drop
> table_id=254, duration=574s, n_packets=0, n_bytes=0, 
> priority=0,reg0=0x3,actions=drop
> 
> new command with argument '-m' or '--more'
> 
> Notice: 'n_offload_packets' are a subset of n_packets and 'n_offload_bytes' 
> are
> a subset of n_bytes.
> 
> ovs-appctl bridge/dump-flows -m br0
> 
> duration=576s, n_packets=1152, n_bytes=110768, n_offload_packets=1107, 
> n_offload_bytes=107992, priority=0,actions=NORMAL
> table_id=254, duration=576s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=2,recirc_id=0,actions=drop
> table_id=254, duration=576s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=0,reg0=0x1,actions=controller(reason=)
> table_id=254, duration=576s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=0,reg0=0x2,actions=drop
> table_id=254, duration=576s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=0,reg0=0x3,actions=drop
> 
> ovs-appctl bridge/dump-flows --more br0
> 
> duration=582s, n_packets=1152, n_bytes=110768, n_offload_packets=1107, 
> n_offload_bytes=107992, priority=0,actions=NORMAL
> table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=2,recirc_id=0,actions=drop
> table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=0,reg0=0x1,actions=controller(reason=)
> table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=0,reg0=0x2,actions=drop
> table_id=254, duration=582s, n_packets=0, n_bytes=0, n_offload_packets=0, 
> n_offload_bytes=0, priority=0,reg0=0x3,actions=drop
> 
> Signed-off-by: zhaozhanxu 
> ---
>  NEWS   |  2 ++
>  lib/dpif.h | 10 ++
>  ofproto/bond.c |  7 ++--
>  ofproto/ofproto-dpif-upcall.c  | 11 +++---
>  ofproto/ofproto-dpif-xlate-cache.c |  8 ++---
>  ofproto/ofproto-dpif-xlate-cache.h |  6 ++--
>  ofproto/ofproto-dpif-xlate.c   |  6 ++--
>  ofproto/ofproto-dpif.c | 29 ++--
>  ofproto/ofproto-dpif.h |  4 +--
>  ofproto/ofproto-provider.h | 11 --
>  ofproto/ofproto.c  | 55 ++
>  ofproto/ofproto.h  |  2 +-
>  vswitchd/bridge.c  | 15 +---
>  vswitchd/ovs-vswitchd.8.in |  3 +-
>  14 files changed, 108 insertions(+), 61 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 330ab3832..14023fcf7 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -81,6 +81,8 @@ v2.12.0 - 03 Sep 2019
>   * Add support for conntrack zone-based timeout policy.
> - 'ovs-dpctl dump-flows' is no longer suitable for dumping offloaded 
> flows.
>   'ovs-appctl dpctl/dump-flows' should be used instead.
> +   - Add new argument '-m | --more' for command 'ovs-appctl 
> bridge/dump-flows',
> + so it can display offloaded packets statistics.
> - Add L2 GRE tunnel over IPv6 support.
>  
>  v2.11.0 - 19 Feb 2019
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 289d574a0..7d82a28be 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -496,6 +496,16 @@ struct dpif_flow_stats {
>  uint16_t tcp_flags;
>  };
>  
> +/* more statistics info for offloaded packets and bytes */
> +struct dpif_flow_detailed_stats {
> +uint64_t n_packets; /* n_offload_packets are a subset of n_packets */
> +uint64_t n_bytes;   /* n_offload_bytes are a subset of n_bytes */
> +uint64_t n_offload_packets;
> +uint64_t n_offload_bytes;
> +long long int used;
> +uint16_t tcp_flags;
> +};
> +
>  struct dpif_flow_attrs {
>  bool offloaded; /* True if flow is offloaded to HW. */
>  const char *dp_layer;   /* DP layer the flow is handled in. */
> diff --git a/ofproto/bond.c b/ofproto/bond.c
> index c5d5f2c03..03de1eec9 100644
> --- a/ofproto/bond.c
> +++ b/ofproto/bond.c
> @@ -946,13 +946,12 @@ bond_recirculation_account(struct bond *bond)
>  struct rule *rule = entry->pr_rule;
>  
>  if (rule) {
> -uint64_t n_packets OVS_UNUSED;
> +struct pkts_stats stats;
>  long long int used OVS_UNUSED;
> -uint64_t n_bytes;
>  
>  rule->ofproto->ofproto_class->rule_get_stats(

Re: [ovs-dev] [PATCH] ovsdb-server: Allow OVSDB clients to specify the UUID for inserted rows.

[0-day Robot]

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.


checkpatch:
WARNING: Line is 82 characters long (recommended limit is 79)
#116 FILE: ovsdb/transaction.c:1:
/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2017, 2019 Nicira, 
Inc.

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


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

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

[ovs-dev] [PATCH ovn] testsuite.at: Add ovn-performance.at back to testsuite.

[Han Zhou]

It seems the ovn-performance.at was missing when spliting from OVS.
This patch just add it back.

Signed-off-by: Han Zhou 
---
 tests/testsuite.at | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/testsuite.at b/tests/testsuite.at
index c1ba734..da8157d 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -22,6 +22,7 @@ m4_include([tests/ofproto-macros.at])
 m4_include([tests/ovn-macros.at])
 
 m4_include([tests/ovn.at])
+m4_include([tests/ovn-performance.at])
 m4_include([tests/ovn-northd.at])
 m4_include([tests/ovn-nbctl.at])
 m4_include([tests/ovn-sbctl.at])
-- 
2.1.0

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

Re: [ovs-dev] Loan Offer

[Prudential Loans.]

See attach file, I await your response.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Taller de Planeación Estratégica

[Ciclo de vida de las iniciativas de cambio]

12 de Diciembre | Horario de 10:00 a 17:00 hrs.  |  (hora del centro de México) 

Taller práctico de Planeación Estratégica


A través de las metodologías de planeación, se definen con la participación de 
la alta dirección y funcionarios clave las líneas y los objetivos estratégicos 
para 
la organización. La metodología que propone el taller proporciona la 
información necesaria para definir la dirección que deberá seguir la compañía 
en el futuro.

¿Qué aprenderás?:

• El nuevo entorno operativo y el ciclo de vida de las iniciativas de cambio.
• El liderazgo en la estrategia.
• Proceso de formulación de la estrategia
• Conceptos para la planeación estratégica.
• Definición de las líneas estratégicas

Solicita información respondiendo a este correo con la palabra Planeación, 
junto con los siguientes datos:

Nombre:
Correo electrónico:
Número telefónico:

Dirigido a: Gerentes y directores del área.

Números de Atención: 

(045) 55 15 54 66 30 - (045) 55 85567293 - (045) 5530167085 

En caso de que haya recibido este correo sin haberlo solicitado o si desea 
dejar de recibir nuestra promoción favor de responder con la palabra baja o 
enviar un correo a bajas@ innovalearn.net


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

Re: [ovs-dev] [PATCH ovn] ofctrl_check_and_add_flow: Replace the actions of an existing flow if actions have changed.

[Han Zhou]

On Mon, Dec 2, 2019 at 12:41 AM Numan Siddique  wrote:
>
> On Mon, Dec 2, 2019 at 1:53 PM Han Zhou  wrote:
> >
> > On Sun, Dec 1, 2019 at 11:59 PM Numan Siddique  wrote:
> > >
> > > On Mon, Dec 2, 2019 at 12:44 PM Han Zhou  wrote:
> > > >
> > > > On Fri, Nov 29, 2019 at 1:08 AM  wrote:
> > > > >
> > > > > From: Numan Siddique 
> > > > >
> > > > > If ofctrl_check_and_add_flow(F') is called where flow F' has
> > > > match-actions (M, A2)
> > > > > and if there already exists a flow F with match-actions (M, A1)
in the
> > > > desired flow
> > > > > table, then this function  doesn't update the existing flow F
with new
> > > > actions
> > > > > actions A2.
> > > > >
> > > > > This patch fixes it. Presently we don't see any issues because of
this
> > > > behaviour.
> > > > > But it's better to fix it.
> > > > >
> > > >
> > > > Hi Numan, could you explain why do you think the F' should override
the
> > F?
> > > > For my understanding, this is a problem of duplicated logical flows
> > > > generated by ovn-northd and can't be solved in ovn-controller. The
> > desired
> > > > flows have conflict and there is no way to judge which one should be
> > > > applied.
> > > >
> > >
> > > Hi Han,
> > >
> > > I probably should have given the context in which I observed this
issue.
> > > I am working on making incremental processing for datapath
> > additions/deletions.
> > >
> > > In my testing I observed that the test case -  84: ovn.at:10767
> > > ovn -- send gratuitous ARP for NAT rules on HA distributed router
> > > is failing 100% of the time.
> > >
> > > Upon investigation I found that it's failing for below logical flow
> > >
> > >  table=17(ls_in_l2_lkup  ), priority=80   , match=(eth.src == {
> > > 00:00:00:00:ff:01} && (arp.op == 1 || nd_ns)), action=(outport =
> > > "_MC_flood"; output;)
> > >
> > > If you see the test code here -
> > > https://github.com/ovn-org/ovn/blob/master/tests/ovn.at#L10901
> > >
> > > ovn-nbctl --wait=hv clear logical_switch_port lrp0-rp options
> > >
> > > When the above command is executed, the logical switch "ls0" is
> > > removed from the "local_datapaths" hmap.
> > > When ls0 is removed, ovsdb-server sends monitor condition to remove
> > > the "Multicast_Group" row ls0.
> > >
> > > Later when this command is executed  - ovn-nbctl --wait=hv
> > > lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router"
> > >
> > > ovn-controller gets this update from sb ovsdb-server and it adds back
> > > ls0 to "local_datapaths". At this point, "Multicast_Group" row
> > > for ls0 is empty and the above logical flow  translates to
"set:0->reg15".
> > >
> > > Soon after ovn-controller receives monitor update message to add back
> > > the "Multicast_Group" row for ls0.
> > > With the patch I am working, calculates logical flows for the logical
> > > switch sw0. But it doesn't add the right flow. The action should have
> > > been set to "set:0x8000->reg15".
> > >
> > > The patch I am working on can be found here - [1] -
> > >
> >
https://github.com/numansiddique/ovn/commit/024812e76f4d0628612b1885cca65302a64038c8
> > > https://github.com/numansiddique/ovn/tree/lflow_reloaded/v1/p1
> > >
> > > Please note I am still testing it out and doing some scale testing
> > > before submitting the patch for review.
> > > [1] adds  a new table - "Datapath_Logical_Flow" in south db which is a
> > > weak ref and its references in Datapath_Binding.
> > >
> > > We can discuss more about the approach later.
> > >
> > > But I think the proposed patch here makes sense. We don't hit this
> > > issue in the present code because when ever any datapath_binding
> > > change happens
> > > we do full recompute and this flushes out the old  logical flow in the
> > > desired_flow_table.
> > >
> > Hi Numan,
> >
> > Thanks for explaining the context. The principle in I-P for handling
> > changes is always handling deletions first and then creations, and for
> > updates, we always delete the old entries and then add the new ones.
> > In your context, if a mc_group is updated, we should delete the old
flows
> > related to that mc_group and then create the new flows. Would the
problem
> > be solved if we follow this principle?
>
> In principle, yes this would work. But I am not sure how to associate
> the logical flow/OF flows
> related to mc_group.
>
> The action - "outport = _MC_Flood" (or any other _MC_*) can be used in
> any pipeline of the
> logical switches/logical routers. I couldn't figure out how to apply
> incremental processing
> for mc_group changes. The approach I have taken now is to recalculate
> flows for only the
> relevant datapaths (based on the datapath column of Multicast_Group
table).
>

I think I have some idea on this. Currently,
flow_output_sb_multicast_group_handler() handles multicast_group changes,
but it only computes physical flows related to the multicast group change.
Since datapath changes always trigger recompute, this is not a problem.
Now that we want to handle datapath changes incrementally

Re: [ovs-dev] [PATCH] ofproto-dpif: Refactor the get capability code.

[William Tu]

On Fri, Nov 22, 2019 at 09:28:14AM -0800, Ben Pfaff wrote:
> On Thu, Nov 21, 2019 at 11:09:02AM -0800, William Tu wrote:
> > Make the code simpler by removing the use of
> > xasprintf and free, and use smap_add_format.
> > 
> > Cc: Ben Pfaff 
> > Signed-off-by: William Tu 
> 
> Looks good to me.
> 
> Acked-by: Ben Pfaff 

Thank you, applied to master
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 ovn 1/3] ovn-controller: Add per node states to I-P engine.

[Han Zhou]

On Wed, Nov 27, 2019 at 5:15 AM Dumitru Ceara  wrote:
>
> This commit transforms the 'changed' field in struct engine_node in a
> 'state' field. Possible node states are:
> - "Stale": data in the node is not up to date with the DB.
> - "Updated": data in the node is valid but was updated during
>   the last run of the engine.
> - "Valid": data in the node is valid and didn't change during
>   the last run of the engine.
> - "Aborted": during the last run, processing was aborted for
>   this node.
>
> This commit also further refactors the I-P engine:
> - instead of recursively performing all the engine processing a
>   preprocessing stage is added (engine_get_nodes()) before the main
processing
>   loop is executed in order to topologically sort nodes in the engine such
>   that all inputs of a given node appear in the sorted array before the
node
>   itself. This simplifies a bit the code in engine_run().
> - remove the need for using an engine_run_id by using the newly added
states.
> - turn the global 'engine_abort_recompute' into an argument to be passed
to
>   engine_run(). It's relevant only in the current run context anyway as
>   we reset it before every call to engine_run().
>
> Signed-off-by: Dumitru Ceara 
> ---
>  controller/ovn-controller.c |  101 ++-
>  lib/inc-proc-eng.c  |  231
+++
>  lib/inc-proc-eng.h  |   74 +-
>  3 files changed, 268 insertions(+), 138 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index c56190f..255531d 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -758,10 +758,10 @@ en_ofctrl_is_connected_run(struct engine_node *node)
>  (struct ed_type_ofctrl_is_connected *)node->data;
>  if (data->connected != ofctrl_is_connected()) {
>  data->connected = !data->connected;
> -node->changed = true;
> +engine_set_node_state(node, EN_UPDATED);
>  return;
>  }
> -node->changed = false;
> +engine_set_node_state(node, EN_VALID);
>  }
>
>  struct ed_type_addr_sets {
> @@ -811,7 +811,7 @@ en_addr_sets_run(struct engine_node *node)
>  addr_sets_init(as_table, &as->addr_sets);
>
>  as->change_tracked = false;
> -node->changed = true;
> +engine_set_node_state(node, EN_UPDATED);
>  }
>
>  static bool
> @@ -830,11 +830,14 @@ addr_sets_sb_address_set_handler(struct engine_node
*node)
>  addr_sets_update(as_table, &as->addr_sets, &as->new,
>   &as->deleted, &as->updated);
>
> -node->changed = !sset_is_empty(&as->new) ||
!sset_is_empty(&as->deleted)
> -|| !sset_is_empty(&as->updated);
> +if (!sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) ||
> +!sset_is_empty(&as->updated)) {
> +engine_set_node_state(node, EN_UPDATED);
> +} else {
> +engine_set_node_state(node, EN_VALID);
> +}
>
>  as->change_tracked = true;
> -node->changed = true;
>  return true;
>  }
>
> @@ -885,7 +888,7 @@ en_port_groups_run(struct engine_node *node)
>  port_groups_init(pg_table, &pg->port_groups);
>
>  pg->change_tracked = false;
> -node->changed = true;
> +engine_set_node_state(node, EN_UPDATED);
>  }
>
>  static bool
> @@ -904,11 +907,14 @@ port_groups_sb_port_group_handler(struct
engine_node *node)
>  port_groups_update(pg_table, &pg->port_groups, &pg->new,
>   &pg->deleted, &pg->updated);
>
> -node->changed = !sset_is_empty(&pg->new) ||
!sset_is_empty(&pg->deleted)
> -|| !sset_is_empty(&pg->updated);
> +if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) ||
> +!sset_is_empty(&pg->updated)) {
> +engine_set_node_state(node, EN_UPDATED);
> +} else {
> +engine_set_node_state(node, EN_VALID);
> +}
>
>  pg->change_tracked = true;
> -node->changed = true;
>  return true;
>  }
>
> @@ -1091,7 +1097,7 @@ en_runtime_data_run(struct engine_node *node)
>  update_ct_zones(local_lports, local_datapaths, ct_zones,
>  ct_zone_bitmap, pending_ct_zones);
>
> -node->changed = true;
> +engine_set_node_state(node, EN_UPDATED);
>  }
>
>  static bool
> @@ -1157,10 +1163,10 @@ en_mff_ovn_geneve_run(struct engine_node *node)
>  enum mf_field_id mff_ovn_geneve = ofctrl_get_mf_field_id();
>  if (data->mff_ovn_geneve != mff_ovn_geneve) {
>  data->mff_ovn_geneve = mff_ovn_geneve;
> -node->changed = true;
> +engine_set_node_state(node, EN_UPDATED);
>  return;
>  }
> -node->changed = false;
> +engine_set_node_state(node, EN_VALID);
>  }
>
>  struct ed_type_flow_output {
> @@ -1322,7 +1328,7 @@ en_flow_output_run(struct engine_node *node)
>   active_tunnels,
>   flow_table);
>
> -node->changed = true;
> +engine_set_node_state(node, EN_UPDATED);
>  }
>
>  static bool
> @@ 

Re: [ovs-dev] [PATCH v7 ovn 2/3] ovn-controller: Add separate I-P engine node for processing ct-zones.

[Han Zhou]

On Wed, Nov 27, 2019 at 5:15 AM Dumitru Ceara  wrote:
>
> Signed-off-by: Dumitru Ceara 
> ---
>  controller/ovn-controller.c |  117
+--
>  1 file changed, 78 insertions(+), 39 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 255531d..f5a83f9 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -933,11 +933,6 @@ struct ed_type_runtime_data {
>   * _ */
>  struct sset local_lport_ids;
>  struct sset active_tunnels;
> -
> -/* connection tracking zones. */
> -unsigned long ct_zone_bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)];
> -struct shash pending_ct_zones;
> -struct simap ct_zones;
>  };
>
>  static void
> @@ -945,24 +940,11 @@ en_runtime_data_init(struct engine_node *node)
>  {
>  struct ed_type_runtime_data *data =
>  (struct ed_type_runtime_data *)node->data;
> -struct ovsrec_open_vswitch_table *ovs_table =
> -(struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
> -engine_get_input("OVS_open_vswitch", node));
> -struct ovsrec_bridge_table *bridge_table =
> -(struct ovsrec_bridge_table *)EN_OVSDB_GET(
> -engine_get_input("OVS_bridge", node));
> +
>  hmap_init(&data->local_datapaths);
>  sset_init(&data->local_lports);
>  sset_init(&data->local_lport_ids);
>  sset_init(&data->active_tunnels);
> -shash_init(&data->pending_ct_zones);
> -simap_init(&data->ct_zones);
> -
> -/* Initialize connection tracking zones. */
> -memset(data->ct_zone_bitmap, 0, sizeof data->ct_zone_bitmap);
> -bitmap_set1(data->ct_zone_bitmap, 0); /* Zone 0 is reserved. */
> -restore_ct_zones(bridge_table, ovs_table,
> - &data->ct_zones, data->ct_zone_bitmap);
>  }
>
>  static void
> @@ -983,9 +965,6 @@ en_runtime_data_cleanup(struct engine_node *node)
>  free(cur_node);
>  }
>  hmap_destroy(&data->local_datapaths);
> -
> -simap_destroy(&data->ct_zones);
> -shash_destroy(&data->pending_ct_zones);
>  }
>
>  static void
> @@ -997,9 +976,6 @@ en_runtime_data_run(struct engine_node *node)
>  struct sset *local_lports = &data->local_lports;
>  struct sset *local_lport_ids = &data->local_lport_ids;
>  struct sset *active_tunnels = &data->active_tunnels;
> -unsigned long *ct_zone_bitmap = data->ct_zone_bitmap;
> -struct shash *pending_ct_zones = &data->pending_ct_zones;
> -struct simap *ct_zones = &data->ct_zones;
>
>  static bool first_run = true;
>  if (first_run) {
> @@ -1094,9 +1070,6 @@ en_runtime_data_run(struct engine_node *node)
>  ovs_table, local_datapaths,
>  local_lports, local_lport_ids);
>
> -update_ct_zones(local_lports, local_datapaths, ct_zones,
> -ct_zone_bitmap, pending_ct_zones);
> -
>  engine_set_node_state(node, EN_UPDATED);
>  }
>
> @@ -1138,6 +,55 @@ runtime_data_sb_port_binding_handler(struct
engine_node *node)
>  return !changed;
>  }
>
> +/* Connection tracking zones. */
> +struct ed_type_ct_zones {
> +unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)];
> +struct shash pending;
> +struct simap current;
> +};
> +
> +static void
> +en_ct_zones_init(struct engine_node *node)
> +{
> +struct ed_type_ct_zones *data = node->data;
> +struct ovsrec_open_vswitch_table *ovs_table =
> +(struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
> +engine_get_input("OVS_open_vswitch", node));
> +struct ovsrec_bridge_table *bridge_table =
> +(struct ovsrec_bridge_table *)EN_OVSDB_GET(
> +engine_get_input("OVS_bridge", node));
> +
> +shash_init(&data->pending);
> +simap_init(&data->current);
> +
> +memset(data->bitmap, 0, sizeof data->bitmap);
> +bitmap_set1(data->bitmap, 0); /* Zone 0 is reserved. */
> +restore_ct_zones(bridge_table, ovs_table, &data->current,
data->bitmap);
> +}
> +
> +static void
> +en_ct_zones_cleanup(struct engine_node *node)
> +{
> +struct ed_type_ct_zones *data = node->data;
> +
> +simap_destroy(&data->current);
> +shash_destroy(&data->pending);
> +}
> +
> +static void
> +en_ct_zones_run(struct engine_node *node)
> +{
> +struct ed_type_ct_zones *data = node->data;
> +struct ed_type_runtime_data *rt_data =
> +(struct ed_type_runtime_data *)engine_get_input(
> +"runtime_data", node)->data;
> +
> +update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths,
> +&data->current, data->bitmap, &data->pending);
> +
> +engine_set_node_state(node, EN_UPDATED);
> +}
> +
>  struct ed_type_mff_ovn_geneve {
>  enum mf_field_id mff_ovn_geneve;
>  };
> @@ -1215,7 +1237,11 @@ en_flow_output_run(struct engine_node *node)
>  struct sset *local_lports = &rt_data->local_lports;
>  struct sset *local_lport_ids = &rt_data->local_lport_ids;
>  struct sset *active_tunnels = &rt_data->active_tunnels;

Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow

[Linhaifeng]

Hi, Ben Pfaff
Thank you. Give a like.

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, December 3, 2019 4:21 AM
> To: Numan Siddique 
> Cc: Linhaifeng ; d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH] ofproto: fix stack-buffer-overflow
> 
> On Fri, Nov 29, 2019 at 04:15:59PM +0530, Numan Siddique wrote:
> > On Fri, Nov 29, 2019 at 11:44 AM Linhaifeng 
> wrote:
> > >
> > > Should use flow->actions not &flow->actions.
> > >
> > > here is ASAN report:
> > >
> 
> =
> > > ==57189==ERROR: AddressSanitizer: stack-buffer-overflow on address
> 0x428fa0e8 at pc 0x7f61a520 bp 0x428f9420 sp 0x428f9498 READ
> of size 196 at 0x428fa0e8 thread T150 (revalidator22)
> > > #0 0x7f61a51f in __interceptor_memcpy
> (/lib64/libasan.so.4+0xa251f)
> > > #1 0xd26a3b2b in ofpbuf_put lib/ofpbuf.c:426
> > > #2 0xd26a30cb in ofpbuf_clone_data_with_headroom
> lib/ofpbuf.c:248
> > > #3 0xd26a2e77 in ofpbuf_clone_with_headroom lib/ofpbuf.c:218
> > > #4 0xd26a2dc3 in ofpbuf_clone lib/ofpbuf.c:208
> > > #5 0xd23e3993 in ukey_set_actions
> ofproto/ofproto-dpif-upcall.c:1640
> > > #6 0xd23e3f03 in ukey_create__
> ofproto/ofproto-dpif-upcall.c:1696
> > > #7 0xd23e553f in ukey_create_from_dpif_flow
> ofproto/ofproto-dpif-upcall.c:1806
> > > #8 0xd23e65fb in ukey_acquire
> ofproto/ofproto-dpif-upcall.c:1984
> > > #9 0xd23eb583 in revalidate ofproto/ofproto-dpif-upcall.c:2625
> > > #10 0xd23dee5f in udpif_revalidator
> ofproto/ofproto-dpif-upcall.c:1076
> > > #11 0xd26b84ef in ovsthread_wrapper lib/ovs-thread.c:708
> > > #12 0x7e74a8bb in start_thread (/lib64/libpthread.so.0+0x78bb)
> > > #13 0x7e0665cb in thread_start (/lib64/libc.so.6+0xd55cb)
> > >
> > > Address 0x428fa0e8 is located in stack of thread T150 (revalidator22) 
> > > at
> offset 328 in frame
> > > #0 0xd23e4cab in ukey_create_from_dpif_flow
> > > ofproto/ofproto-dpif-upcall.c:1762
> > >
> > >   This frame has 4 object(s):
> > > [32, 96) 'actions'
> > > [128, 192) 'buf'
> > > [224, 328) 'full_flow'
> > > [384, 2432) 'stub' <== Memory access at offset 328 partially
> > > underflows this variable
> > > HINT: this may be a false positive if your program uses some custom stack
> unwind mechanism or swapcontext
> > >   (longjmp and C++ exceptions *are* supported) Thread T150
> (revalidator22) created by T0 here:
> > > #0 0x7f5b0f7f in __interceptor_pthread_create
> (/lib64/libasan.so.4+0x38f7f)
> > > #1 0xd26b891f in ovs_thread_create lib/ovs-thread.c:792
> > > #2 0xd23dc62f in udpif_start_threads
> ofproto/ofproto-dpif-upcall.c:639
> > > #3 0xd23daf87 in ofproto_set_flow_table
> ofproto/ofproto-dpif-upcall.c:446
> > > #4 0xd230ff7f in dpdk_evs_cfg_set vswitchd/bridge.c:1134
> > > #5 0xd2310097 in bridge_reconfigure vswitchd/bridge.c:1148
> > > #6 0xd23279d7 in bridge_run vswitchd/bridge.c:3944
> > > #7 0xd23365a3 in main vswitchd/ovs-vswitchd.c:240
> > > #8 0x7dfb1adf in __libc_start_main (/lib64/libc.so.6+0x20adf)
> > > #9 0xd230a3d3
> > > (/usr/sbin/ovs-vswitchd-2.7.0-1.1.RC5.001.asan+0x26f3d3)
> > >
> > > SUMMARY: AddressSanitizer: stack-buffer-overflow
> (/lib64/libasan.so.4+0xa251f) in __interceptor_memcpy Shadow bytes around
> the buggy address:
> > >   0x200fe851f3c0: 00 00 00 00 f1 f1 f1 f1 f8 f2 f2 f2 00 00 00 00
> > >   0x200fe851f3d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f3e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f3f0: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
> > >   0x200fe851f400: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f2 f2 f2 f2
> > > =>0x200fe851f410: 00 00 00 00 00 00 00 00 00 00 00 00 00[f2]f2 f2
> > >   0x200fe851f420: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f440: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >   0x200fe851f460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> > >   Addressable:   00
> > >   Partially addressable: 01 02 03 04 05 06 07
> > >   Heap left redzone:   fa
> > >   Freed heap region:   fd
> > >   Stack left redzone:  f1
> > >   Stack mid redzone:   f2
> > >   Stack right redzone: f3
> > >   Stack after return:  f5
> > >   Stack use after scope:   f8
> > >   Global redzone:  f9
> > >   Global init order:   f6
> > >   Poisoned by user:f7
> > >   Container overflow:  fc
> > >   Array cookie:ac
> > >   Intra object redzone:bb
> > >   ASan internal:   fe
> > >   Left alloca redzone: ca
> > >   Right alloca redzone

[ovs-dev] datapath-windows: Don't delete internal port

[Jinjun Gao via dev]

According to the microsoft doc:
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states
Below OID request sequence is validation:
 OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT
  ^   |
  |   V
 OID_SWITCH_NIC_CREATE  <- OID_SWITCH_NIC_DELETE

In above sequence, the windows extensible switch interface assumes the
OID_SWITCH_PORT_CREATE has issued and the port has been created
successfully. If delete the internal port in HvDisconnectNic(),
HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because
there is no corresponding port.

Signed-off-by: Jinjun Gao 
---
datapath-windows/ovsext/Vport.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 57e7510..c3362d7 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -640,8 +640,14 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
 if (isInternalPort) {
 OvsUnBindVportWithIpHelper(vport, switchContext);
-OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
-OvsPostVportEvent(&event);
+/*
+ * Don't delete the port from the hash tables here for internal port
+ * because the internal port cannot be recreated in HvCreateNic(). It
+ * only can be created in HvCreatePort() by issuing
+ * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface
+ * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to
+ * delete the internal port.
+ */
 }
 NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
--
1.8.5.6
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] datapath-windows: Don't delete internal port

[0-day Robot]

Bleep bloop.  Greetings Jinjun Gao 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: corrupt patch at line 29
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 datapath-windows: Don't delete internal port
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...@redhat.com

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

Re: [ovs-dev] [patch v1] conntrack: Support zone limits.

[Darrell Ball]

Thanks for the review Ben

On Mon, Dec 2, 2019 at 12:19 PM Ben Pfaff  wrote:

> On Mon, Dec 02, 2019 at 11:41:27AM -0800, Darrell Ball wrote:
> > Signed-off-by: Darrell Ball 
>
> Thanks.  I'm glad to see this code growing closer to parity with the
> kernel implementation.  The implementation also looks pretty clean.
>
> I'm appending some style suggestions.  They should not change behavior.
>

those are all fine - thanks


>
> I do have one concern about correctness.  It looks to me that there is a
> separate lookup for the zone at the time that a connection is created
> (to increment the counter) and at the time it is destroyed (to decrement
> the counter).  I believe that this can lead to inconsistencies.  For
> example, suppose that initially a connection has no associated zone, but
> at time of destruction a zone does exist.  In that case, I believe that
> a counter would get decremented that was never incremented.


Good catch !
I agree; I created a conn 'admit zone' to keep track of the zone limit zone
used at creation time.


> I think
> that there are similar potential issues related to finding a specific
> zone versus the default zone.
>

IIUC, when a zone is looked up otherwise, we specify whether we are looking
for the default zone or other zone explicitly. If I missed your meaning,
pls comment on
this aspect in V2. Thanks


>
> -8<--cut here-->8--
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 59e1c51c0389..c90da2b4e32e 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -78,9 +78,7 @@ enum ct_alg_ctl_type {
>
>  struct zone_limit {
>  struct hmap_node node;
> -int32_t zone;
> -uint32_t limit;
> -uint32_t count;
> +struct conntrack_zone_limit czl;
>

might as well use the struct created


>  };
>
>  static bool conn_key_extract(struct conntrack *, struct dp_packet *,
> @@ -339,31 +337,30 @@ zone_limit_lookup(struct conntrack *ct, int32_t zone)
>  {
>  uint32_t hash = zone_key_hash(zone, ct->hash_basis);
>  struct zone_limit *zl;
> -HMAP_FOR_EACH_WITH_HASH (zl, node, hash, &ct->zone_limits) {
> -if (zl->zone == zone) {
> +HMAP_FOR_EACH_IN_BUCKET (zl, node, hash, &ct->zone_limits) {
>

sure, slightly faster


> +if (zl->czl.zone == zone) {
>  return zl;
>  }
>  }
>  return NULL;
>  }
>
> +static struct zone_limit *
> +zone_limit_lookup_or_default(struct conntrack *ct, int32_t zone)
> +OVS_REQUIRES(ct->ct_lock)
> +{
> +struct zone_limit *zl = zone_limit_lookup(ct, zone);
> +return zl ? zl : zone_limit_lookup(ct, DEFAULT_ZONE);
> +}
> +
>

this is useful


>  struct conntrack_zone_limit
>  zone_limit_get(struct conntrack *ct, int32_t zone)
>  {
>  struct conntrack_zone_limit czl = {INVALID_ZONE, 0, 0};
>  ovs_mutex_lock(&ct->ct_lock);
> -struct zone_limit *zl = zone_limit_lookup(ct, zone);
> +struct zone_limit *zl = zone_limit_lookup_or_default(ct, zone);
>  if (zl) {
> -czl.zone = zl->zone;
> -czl.limit = zl->limit;
> -czl.count = zl->count;
> -} else {
> -zl = zone_limit_lookup(ct, DEFAULT_ZONE);
> -if (zl) {
> -czl.zone = zl->zone;
> -czl.limit = zl->limit;
> -czl.count = zl->count;
> -}
> +czl = zl->czl;
>  }
>  ovs_mutex_unlock(&ct->ct_lock);
>  return czl;
> @@ -375,8 +372,8 @@ zone_limit_create(struct conntrack *ct, int32_t zone,
> uint32_t limit)
>  {
>  if (zone >= DEFAULT_ZONE && zone <= MAX_ZONE) {
>  struct zone_limit *zl = xzalloc(sizeof *zl);
> -zl->limit = limit;
> -zl->zone = zone;
> +zl->czl.limit = limit;
> +zl->czl.zone = zone;
>  uint32_t hash = zone_key_hash(zone, ct->hash_basis);
>  hmap_insert(&ct->zone_limits, &zl->node, hash);
>  return 0;
> @@ -392,7 +389,7 @@ zone_limit_update(struct conntrack *ct, int32_t zone,
> uint32_t limit)
>  ovs_mutex_lock(&ct->ct_lock);
>  struct zone_limit *zl = zone_limit_lookup(ct, zone);
>  if (zl) {
> -zl->limit = limit;
> +zl->czl.limit = limit;
>  VLOG_INFO("Changed zone limit of %u for zone %d", limit, zone);
>  } else {
>  err = zone_limit_create(ct, zone, limit);
> @@ -444,7 +441,7 @@ conn_clean_cmn(struct conntrack *ct, struct conn *conn)
>
>  struct zone_limit *zl = zone_limit_lookup(ct, conn->key.zone);
>  if (zl) {
> -zl->count--;
> +zl->czl.count--;
>  }
>  }
>
> @@ -984,16 +981,9 @@ conn_not_found(struct conntrack *ct, struct dp_packet
> *pkt,
>  return nc;
>  }
>
> -struct zone_limit *zl = zone_limit_lookup(ct, ctx->key.zone);
> -if (zl) {
> -if (zl->count >= zl->limit) {
> -return nc;
> -}
> -} else {
> -zl = zone_limit_lookup(ct, DEFAULT_ZONE);
> -if (zl && zl->count >= zl->limit) {
> -

[ovs-dev] [patch v2] conntrack: Support zone limits.

[Darrell Ball]

Signed-off-by: Darrell Ball 
---

v2: Address review comment from Ben; one involves creating an
admit zone in the connection entry to track the zone used
for zone limit accounting when the entry was created and use
that zone at cleanup time accounting.

Updated dpctl.man.
Fixed a bug in zone_limit_get() to return default zone by default..
Fixed a parameter in zone_limit_delete().
Some small cleanups.

 Documentation/faq/releases.rst   |   2 +-
 NEWS |   1 +
 lib/conntrack-private.h  |   2 +
 lib/conntrack.c  | 134 +++
 lib/conntrack.h  |  17 +
 lib/dpctl.man|   8 +--
 lib/dpif-netdev.c|  88 -
 tests/system-kmod-macros.at  |   7 --
 tests/system-traffic.at  |   1 -
 tests/system-userspace-macros.at |   9 ---
 10 files changed, 243 insertions(+), 26 deletions(-)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index e02dda1..4072c99 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -118,7 +118,7 @@ Q: Are all features available with all datapaths?
 Connection tracking 4.32.5  2.6  YES
 Conntrack Fragment Reass.   4.32.6  2.10 YES
 Conntrack Timeout Policies  5.22.12 NO   NO
-Conntrack Zone Limit4.18   2.10 NO   YES
+Conntrack Zone Limit4.18   2.10 2.13 YES
 Conntrack NAT   4.62.6  2.8  YES
 Tunnel - LISP   NO 2.11 NO   NO
 Tunnel - STTNO 2.4  NO   YES
diff --git a/NEWS b/NEWS
index 80b..17f92ba 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Post-v2.12.0
- Userspace datapath:
  * Add option to enable, disable and query TCP sequence checking in
conntrack.
+ * Add support for conntrack zone limits.
- AF_XDP:
  * New option 'use-need-wakeup' for netdev-afxdp to control enabling
of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by default
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 590f139..7d36eac 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -105,6 +105,7 @@ struct conn {
 long long expiration;
 uint32_t mark;
 int seq_skew;
+int32_t admit_zone; /* The zone for managing zone limit counts. */
 bool seq_skew_dir; /* TCP sequence skew direction due to NATTing of FTP
 * control messages; true if reply direction. */
 bool cleaned; /* True if cleaned from expiry lists. */
@@ -155,6 +156,7 @@ struct conntrack {
 struct ovs_mutex ct_lock; /* Protects 2 following fields. */
 struct cmap conns OVS_GUARDED;
 struct ovs_list exp_lists[N_CT_TM] OVS_GUARDED;
+struct hmap zone_limits OVS_GUARDED;
 uint32_t hash_basis; /* Salt for hashing a connection key. */
 pthread_t clean_thread; /* Periodically cleans up connection tracker. */
 struct latch clean_thread_exit; /* To destroy the 'clean_thread'. */
diff --git a/lib/conntrack.c b/lib/conntrack.c
index df7b9fa..33d540a 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -76,6 +76,11 @@ enum ct_alg_ctl_type {
 CT_ALG_CTL_SIP,
 };
 
+struct zone_limit {
+struct hmap_node node;
+struct conntrack_zone_limit czl;
+};
+
 static bool conn_key_extract(struct conntrack *, struct dp_packet *,
  ovs_be16 dl_type, struct conn_lookup_ctx *,
  uint16_t zone);
@@ -305,6 +310,7 @@ conntrack_init(void)
 for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) {
 ovs_list_init(&ct->exp_lists[i]);
 }
+hmap_init(&ct->zone_limits);
 ovs_mutex_unlock(&ct->ct_lock);
 
 ct->hash_basis = random_uint32();
@@ -318,6 +324,110 @@ conntrack_init(void)
 return ct;
 }
 
+static uint32_t
+zone_key_hash(int32_t zone, uint32_t basis)
+{
+size_t hash = hash_int((OVS_FORCE uint32_t) zone, basis);
+return hash;
+}
+
+static struct zone_limit *
+zone_limit_lookup(struct conntrack *ct, int32_t zone)
+OVS_REQUIRES(ct->ct_lock)
+{
+uint32_t hash = zone_key_hash(zone, ct->hash_basis);
+struct zone_limit *zl;
+HMAP_FOR_EACH_IN_BUCKET (zl, node, hash, &ct->zone_limits) {
+if (zl->czl.zone == zone) {
+return zl;
+}
+}
+return NULL;
+}
+
+static struct zone_limit *
+zone_limit_lookup_or_default(struct conntrack *ct, int32_t zone)
+OVS_REQUIRES(ct->ct_lock)
+{
+struct zone_limit *zl = zone_limit_lookup(ct, zone);
+return zl ? zl : zone_limit_lookup(ct, DEFAULT_ZONE);
+}
+
+struct conntrack_zone_limit
+zone_limit_get(struct conntrack *ct, int32_t zone)
+{
+ovs_mutex_lock(&ct->ct_lock);
+struct conntrack

[ovs-dev] [patch v3] conntrack: Support zone limits.

[Darrell Ball]

Signed-off-by: Darrell Ball 
---

v3: recent merge conflict.

v2: Address review comment from Ben; one involves creating an
admit zone in the connection entry to track the zone used
for zone limit accounting when the entry was created and use
that zone at cleanup time accounting.

Updated dpctl.man.
Fixed a bug in zone_limit_get() to return default zone by default..
Fixed a parameter in zone_limit_delete().
Some small cleanups.

 Documentation/faq/releases.rst   |   2 +-
 NEWS |   1 +
 lib/conntrack-private.h  |   2 +
 lib/conntrack.c  | 134 +++
 lib/conntrack.h  |  17 +
 lib/dpctl.man|   8 +--
 lib/dpif-netdev.c|  88 -
 tests/system-kmod-macros.at  |   7 --
 tests/system-traffic.at  |   1 -
 tests/system-userspace-macros.at |   9 ---
 10 files changed, 243 insertions(+), 26 deletions(-)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 9c5ee03..6702c58 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -118,7 +118,7 @@ Q: Are all features available with all datapaths?
 Connection tracking 4.32.5  2.6  YES
 Conntrack Fragment Reass.   4.32.6  2.12 YES
 Conntrack Timeout Policies  5.22.12 NO   NO
-Conntrack Zone Limit4.18   2.10 NO   YES
+Conntrack Zone Limit4.18   2.10 2.13 YES
 Conntrack NAT   4.62.6  2.8  YES
 Tunnel - LISP   NO 2.11 NO   NO
 Tunnel - STTNO 2.4  NO   YES
diff --git a/NEWS b/NEWS
index 80b..17f92ba 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Post-v2.12.0
- Userspace datapath:
  * Add option to enable, disable and query TCP sequence checking in
conntrack.
+ * Add support for conntrack zone limits.
- AF_XDP:
  * New option 'use-need-wakeup' for netdev-afxdp to control enabling
of corresponding 'need_wakeup' flag in AF_XDP rings.  Enabled by default
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 590f139..7d36eac 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -105,6 +105,7 @@ struct conn {
 long long expiration;
 uint32_t mark;
 int seq_skew;
+int32_t admit_zone; /* The zone for managing zone limit counts. */
 bool seq_skew_dir; /* TCP sequence skew direction due to NATTing of FTP
 * control messages; true if reply direction. */
 bool cleaned; /* True if cleaned from expiry lists. */
@@ -155,6 +156,7 @@ struct conntrack {
 struct ovs_mutex ct_lock; /* Protects 2 following fields. */
 struct cmap conns OVS_GUARDED;
 struct ovs_list exp_lists[N_CT_TM] OVS_GUARDED;
+struct hmap zone_limits OVS_GUARDED;
 uint32_t hash_basis; /* Salt for hashing a connection key. */
 pthread_t clean_thread; /* Periodically cleans up connection tracker. */
 struct latch clean_thread_exit; /* To destroy the 'clean_thread'. */
diff --git a/lib/conntrack.c b/lib/conntrack.c
index df7b9fa..33d540a 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -76,6 +76,11 @@ enum ct_alg_ctl_type {
 CT_ALG_CTL_SIP,
 };
 
+struct zone_limit {
+struct hmap_node node;
+struct conntrack_zone_limit czl;
+};
+
 static bool conn_key_extract(struct conntrack *, struct dp_packet *,
  ovs_be16 dl_type, struct conn_lookup_ctx *,
  uint16_t zone);
@@ -305,6 +310,7 @@ conntrack_init(void)
 for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) {
 ovs_list_init(&ct->exp_lists[i]);
 }
+hmap_init(&ct->zone_limits);
 ovs_mutex_unlock(&ct->ct_lock);
 
 ct->hash_basis = random_uint32();
@@ -318,6 +324,110 @@ conntrack_init(void)
 return ct;
 }
 
+static uint32_t
+zone_key_hash(int32_t zone, uint32_t basis)
+{
+size_t hash = hash_int((OVS_FORCE uint32_t) zone, basis);
+return hash;
+}
+
+static struct zone_limit *
+zone_limit_lookup(struct conntrack *ct, int32_t zone)
+OVS_REQUIRES(ct->ct_lock)
+{
+uint32_t hash = zone_key_hash(zone, ct->hash_basis);
+struct zone_limit *zl;
+HMAP_FOR_EACH_IN_BUCKET (zl, node, hash, &ct->zone_limits) {
+if (zl->czl.zone == zone) {
+return zl;
+}
+}
+return NULL;
+}
+
+static struct zone_limit *
+zone_limit_lookup_or_default(struct conntrack *ct, int32_t zone)
+OVS_REQUIRES(ct->ct_lock)
+{
+struct zone_limit *zl = zone_limit_lookup(ct, zone);
+return zl ? zl : zone_limit_lookup(ct, DEFAULT_ZONE);
+}
+
+struct conntrack_zone_limit
+zone_limit_get(struct conntrack *ct, int32_t zone)
+{
+ovs_mutex_lock(&ct->ct_

Re: [ovs-dev] [PATCH ovn] testsuite.at: Add ovn-performance.at back to testsuite.

[Numan Siddique]

On Tue, Dec 3, 2019 at 4:50 AM Han Zhou  wrote:
>
> It seems the ovn-performance.at was missing when spliting from OVS.
> This patch just add it back.
>
> Signed-off-by: Han Zhou 

Acked-by: Numan Siddique 


> ---
>  tests/testsuite.at | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index c1ba734..da8157d 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -22,6 +22,7 @@ m4_include([tests/ofproto-macros.at])
>  m4_include([tests/ovn-macros.at])
>
>  m4_include([tests/ovn.at])
> +m4_include([tests/ovn-performance.at])
>  m4_include([tests/ovn-northd.at])
>  m4_include([tests/ovn-nbctl.at])
>  m4_include([tests/ovn-sbctl.at])
> --
> 2.1.0
>
> ___
> 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 ovn] testsuite.at: Add ovn-performance.at back to testsuite.

[Han Zhou]

On Mon, Dec 2, 2019 at 11:28 PM Numan Siddique  wrote:
>
> On Tue, Dec 3, 2019 at 4:50 AM Han Zhou  wrote:
> >
> > It seems the ovn-performance.at was missing when spliting from OVS.
> > This patch just add it back.
> >
> > Signed-off-by: Han Zhou 
>
> Acked-by: Numan Siddique 
>
Thanks Numan. I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] How do i send these funds to your bank account?

[Thomas Dobbs]

Hi Dear,

This is to notify that your over due fund which has
been delayed since years ago is now ready for payment but i don't know how i
am to explain this to you because what i was able to retrieve from the bank
is only but $459,000.00 which i don't know how i am to send these funds
across to you.

If you really need the funds valued $459,000.00, then do let me know so
i can instruct the bank to send you the money.

Hope it is safe for you to receive the funds?

Remain blessed and healthy.

Yours faithfully,
Mr. Thomas Dobbs
United Nations Payment Officer
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev