Re: [ovs-dev] [PATCH ovn] northd: support HW VTEP with stateful datapath

[Numan Siddique]

On Fri, Sep 17, 2021 at 5:56 PM Vladislav Odintsov  wrote:
>
> A packet going from HW VTEP device to VIF port when arrives to
> hypervisor chassis should go through LS ingress pipeline to l2_lkp
> stage without any match. In l2_lkp stage an output port is
> determined and then packet passed to LS egress pipeline for futher
> processing and to VIF port delivery.
>
> Prior to this commit a packet, which was received from HW VTEP
> device was dropped in an LS ingress datapath, where stateful services
> were defined (ACLs, LBs).
>
> To fix this issue we add a special flag-bit which can be used in LS
> pipelines, to check whether the packet came from HW VTEP devices.
> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
> to skip such packets.
>
> Signed-off-by: Vladislav Odintsov 

Thanks.  I applied this patch to master and to the newly created
branch-21.09 (considering it as a bug fix).

I didn't backport to other branches.  Let me know if you need
backports to other patches.

I applied with the below changes

--
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 7bb39d2ab..39f4eaa0c 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -263,16 +263,14 @@
 packets that match the inport.
   
   
-Logical flows for RAMP (controller-vtep) devices are created for each
-physical switch. Packets came from such devices hit these flows and set
-the 14'th bit of OVS register 0 (REG0[14]) to 1. This regbit indicates
-that packet came from RAMP (controller-vtep) device. Later in logical
-switch ingress pipeline this register is checked in ls_in_acl_pre and
-ls_in_lb_pre stages whether to skip sending packet to conntrack in
-ingress pipeline or not. Packets from RAMP devices should go though
-ingress pipeline without any flow match till ls_in_l2_lkup stage to
-determine output port. Stateful ACLs for coming from RAMP device
-packets are checked within logical switch egress pipeline.
+For logical ports of type vtep, the above logical flow
+will also apply the action REGBIT_FROM_RAMP = 1; to
+indicate that the packet is coming from a RAMP (controller-vtep)
+device.  Later pipelines will use this information to skip
+sending the packet to the conntrack.  Packets from vtep
+logical ports should go though ingress pipeline only to determine
+the output port and they should not be subjected to any ACL checks.
+Egress pipeline will do the ACL checks.
   
 

@@ -467,10 +465,11 @@

 
   This table has a priority-110 flow with the match
-  reg0[14] == 1 for all logical switch datapaths to resubmit
-  traffic to the next table. reg0[14] is the register bit,
-  which indicates that packet was received from RAMP device. Packets from
-  RAMP device are handled by ACLs only in Logical Switch egress pipeline.
+  REGBIT_FROM_RAMP == 1 for all logical switch datapaths to
+  resubmit traffic to the next table. REGBIT_FROM_RAMP
+  indicates that packet was received from vtep logical ports
+  and it can be skipped from the stateful ACL processing in the ingress
+  pipeline.
 

 
@@ -534,11 +533,11 @@

 
   This table has a priority-110 flow with the match
-  reg0[14] == 1 for all logical switch datapaths to resubmit
-  traffic to the next table. reg0[14] is the register bit,
-  which indicates that packet was received from RAMP device. Packets from
-  RAMP device could be handled by load balancing flows only in Logical
-  Switch egress pipeline.
+  REGBIT_FROM_RAMP == 1 for all logical switch datapaths to
+  resubmit traffic to the next table. REGBIT_FROM_RAMP
+  indicates that packet was received from vtep logical ports
+  and it can be skipped from the load balancer processing in the ingress
+  pipeline.
 

 


Numan

> ---
>  northd/northd.c | 14 ++
>  northd/ovn-northd.8.xml | 29 +
>  northd/ovn_northd.dl| 33 +++--
>  tests/ovn-northd.at |  2 ++
>  4 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 688a6e4ef..1b84874a7 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -196,6 +196,7 @@ enum ovn_stage {
>  #define REGBIT_LKUP_FDB   "reg0[11]"
>  #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
>  #define REGBIT_ACL_LABEL  "reg0[13]"
> +#define REGBIT_FROM_RAMP  "reg0[14]"
>
>  #define REG_ORIG_DIP_IPV4 "reg1"
>  #define REG_ORIG_DIP_IPV6 "xxreg1"
> @@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
>  if (queue_id) {
>  ds_put_format(actions, "set_queue(%s); ", queue_id);
>  }
> +
> +if (!strcmp(op->nbsp->type, "vtep")) {
> +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; 

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Numan Siddique]

On Fri, Sep 17, 2021 at 5:10 PM Han Zhou  wrote:
>
>
>
> On Fri, Sep 17, 2021 at 2:04 PM Numan Siddique  wrote:
> >
> > On Fri, Sep 17, 2021 at 5:02 PM Renat Nurgaliyev  wrote:
> > >
> > > Hi Han,
> > >
> > > yes, I believe you are totally right. But it still feels like a chicken 
> > > and
> > > egg problem to me, storing the database timeout setting inside the 
> > > database
> > > itself. If there would be at least some local command line argument to
> > > override timeout value, it would be already amazing, because currently
> > > there is no way to control it before the database connection is made, and
> > > if it cannot be made, it is too late to try to control it.
> > >
> >
> > What about the case where the NB database is huge and it takes > 5
> > seconds to fetch
> > all the contents ?
> >
> I think Renat had the answer to this question: using the DB to configure the 
> probe interval to the DB is going to be a problem in certain cases. It should 
> be fine to use NB to configure probe interval for SB, but using NB to 
> configure the probe interval for NB itself is definitely causing the problem 
> when the NB is huge. Support command line options may be the right approach. 
> But in practice would it be good enough to use 60s as the default value 
> instead of 5s?

In most of the large scale deployments,  CMS has to configure higher
probe intervals
anyway.  So 60 seconds as default seems OK to me. I think meanwhile we
should try to
brianstorm and solve the mentioned problem in a much better way.

Thanks
Numan

>
> > Numan
> >
> > > Thanks,
> > > Renat.
> > >
> > > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:55:
> > >
> > > >
> > > >
> > > > On Fri, Sep 17, 2021 at 1:48 PM Renat Nurgaliyev 
> > > > wrote:
> > > > >
> > > > > Hello Han,
> > > > >
> > > > > when I wrote this patch we had an issue with a very big SB database,
> > > > around 1,5 gigabytes. There were no controllers or northds running, so 
> > > > the
> > > > database server was without any load at all. Although OVSDB was idling,
> > > > even a single northd process could not fully connect to the database 
> > > > due to
> > > > its size, since it could not fetch and process the data in 5 seconds.
> > > >
> > > > Hi Renat, thanks for the explanation. However, suppose SB is still huge,
> > > > if NB is not that big, the probe config in NB_Global will soon be 
> > > > applied
> > > > to ovn-northd, which would probe in proper interval (desired setting 
> > > > with
> > > > the SB size considered) instead of the default 5 sec, and it should
> > > > succeed, right?
> > > >
> > > > Thanks,
> > > > Han
> > > >
> > > > >
> > > > > Since then many optimizations were made, and the database size with 
> > > > > the
> > > > same topology reduced to approximately twenty megabytes, so today I
> > > > wouldn't be able to reproduce the problem.
> > > > >
> > > > > However, I am quite sure that it would still cause troubles with a 
> > > > > huge
> > > > scale, when SB grows to hundreds of megabytes. With the default timeout 
> > > > of
> > > > 5 seconds, which is implemented in the same thread that also fetches and
> > > > processes data, we make an artificial database size limit, which is not 
> > > > so
> > > > obvoius to troubleshoot.
> > > > >
> > > > > Regards,
> > > > > Renat.
> > > > >
> > > > > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:
> > > > >>
> > > > >>
> > > > >>
> > > > >> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang  wrote:
> > > > >> >
> > > > >> > From: zhen wang 
> > > > >> >
> > > > >> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> > > > >> > Above commit introduced a bug when muptiple ovn-northd instances 
> > > > >> > work
> > > > in HA
> > > > >> > mode. If SB leader and active ovn-northd instance got killed by
> > > > system power
> > > > >> > outage, standby ovn-northd instance would never detect the failure.
> > > > >> >
> > > > >>
> > > > >> Thanks Zhen! I added the Renat and Numan who worked on the reverted
> > > > commit to CC, so that they can comment if this is ok.
> > > > >>
> > > > >> For the commit message, I think it may be decoupled from the HA
> > > > scenario that is supposed to be fixed by the other patch in this series.
> > > > The issue this patch fixes is that before the initial NB downloading is
> > > > complete the northd will not send probe, so if the DB server is down
> > > > (ungracefully) before the northd reads the NB_Global options, the northd
> > > > would never probe, thus never reconnect to the new leader. (it is 
> > > > related
> > > > to RAFT, but whether it is multiple northds is irrelevant)
> > > > >>
> > > > >> As to the original commit that is reverted by this one:
> > > > >>
> > > > >> northd: Don't poll ovsdb before the connection is fully 
> > > > >> established
> > > > >>
> > > > >> Set initial SB and NB DBs probe interval to 0 to avoid connection
> > > > >> flapping.
> > > > >>
> > > > >> Before configured in northd_probe_interval value is 

Re: [ovs-dev] [PATCH ovn v2] northd: support HW VTEP with stateful datapath

[Vladislav Odintsov]

You’re absolutely right.
The difference with normal chassis is only the lack of output port.
It is determined in l2_lkp table. This is the only one reason to send packet to 
ingress pipeline.

Regards,
Vladislav Odintsov

> On 18 Sep 2021, at 01:25, Numan Siddique  wrote:
> 
> On Fri, Sep 17, 2021 at 5:59 PM Vladislav Odintsov  wrote:
>> 
>> Hi Numan,
>> 
>> I’ve posted a new patch version here:
>> https://patchwork.ozlabs.org/project/ovn/patch/20210917215602.10633-1-odiv...@gmail.com/
> 
> Thanks.  I'll take a look.
> 
> If I understand correctly, packets coming from RAMP (controller-vtep)
> devices should be
> treated like how a packet is tunnelled from another chassis right ?
> Although we need
> to send to ingress pipeline to determine the outport.
> 
> Let me know if my understanding is incorrect.
> 
> Thanks
> Numan
> 
>> 
>> I’ve tried to answer your question about ACLs in documentation.
>> Please let me know if it is clear.
>> 
>> Regards,
>> Vladislav Odintsov
>> 
>>> On 17 Sep 2021, at 22:42, Numan Siddique  wrote:
>>> 
>>> On Fri, Sep 17, 2021 at 11:01 AM Vladislav Odintsov >> > wrote:
 
 A packet going from HW VTEP device to VIF port when arrives to
 hypervisor chassis should go through LS ingress pipeline to l2_lkp
 stage without any match. In l2_lkp stage an output port is
 determined and then packet passed to LS egress pipeline for futher
 processing and to VIF port delivery.
 
 Prior to this commit a packet, which was received from HW VTEP
 device was dropped in an LS ingress datapath, where stateful services
 were defined (ACLs, LBs).
 
 To fix this issue we add a special flag-bit which can be used in LS
 pipelines, to check whether the packet came from HW VTEP devices.
 In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
 to skip such packets.
 
 Signed-off-by: Vladislav Odintsov 
>>> 
>>> Hi Vladislav,
>>> 
>>> The documentation needs to be updated for the newly added logical flows
>>> and also for the updated flow in "ls_in_port_sec_l2" stage.
>>> 
>>> I didn't review your p1 earlier and hence missed it.
>>> 
>>> Regarding the patch,  I don't have much idea on how the ramp/vtep stuff 
>>> works.
>>> My question is why the traffic received from HW VTEP devices should be 
>>> skipped
>>> from conntrack ?  ACLs and policies don't apply to vtep logical ports ?
>>> 
>>> Thanks
>>> Numan
>>> 
 ---
 v1 -> v2:
  - Patch rebased on upstream changes.
 
 Please note: I've got no experience in DDLog and have no ability to 
 extensively
test these changes.
Just local ./configure --with-ddlog=...; make; make check was 
 run
It seems, that only irrelevant to these changes tests were 
 failed.
 ---
 northd/northd.c  | 14 ++
 northd/ovn_northd.dl | 33 +++--
 tests/ovn-northd.at  |  2 ++
 3 files changed, 47 insertions(+), 2 deletions(-)
 
 diff --git a/northd/northd.c b/northd/northd.c
 index 688a6e4ef..1b84874a7 100644
 --- a/northd/northd.c
 +++ b/northd/northd.c
 @@ -196,6 +196,7 @@ enum ovn_stage {
 #define REGBIT_LKUP_FDB   "reg0[11]"
 #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
 #define REGBIT_ACL_LABEL  "reg0[13]"
 +#define REGBIT_FROM_RAMP  "reg0[14]"
 
 #define REG_ORIG_DIP_IPV4 "reg1"
 #define REG_ORIG_DIP_IPV6 "xxreg1"
 @@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
if (queue_id) {
ds_put_format(actions, "set_queue(%s); ", queue_id);
}
 +
 +if (!strcmp(op->nbsp->type, "vtep")) {
 +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
 +}
 +
ds_put_cstr(actions, "next;");
ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
 S_SWITCH_IN_PORT_SEC_L2,
  50, ds_cstr(match), ds_cstr(actions),
 @@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
 *port_groups,
  "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
  "(udp && udp.src == 546 && udp.dst == 547)", "next;");
 
 +/* Do not send coming from RAMP switch packets to conntrack. */
 +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
 +  REGBIT_FROM_RAMP" == 1", "next;");
 +
/* Ingress and Egress Pre-ACL Table (Priority 100).
 *
 * Regardless of whether the ACL is "from-lport" or "to-lport",
 @@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
 *lflows,
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
  "eth.src == $svc_monitor_mac", "next;");
 
 +/* Do not send coming from RAMP switch packets to conntrack. */
 +

Re: [ovs-dev] [PATCH ovn v2] northd: support HW VTEP with stateful datapath

[Numan Siddique]

On Fri, Sep 17, 2021 at 5:59 PM Vladislav Odintsov  wrote:
>
> Hi Numan,
>
> I’ve posted a new patch version here:
> https://patchwork.ozlabs.org/project/ovn/patch/20210917215602.10633-1-odiv...@gmail.com/

Thanks.  I'll take a look.

If I understand correctly, packets coming from RAMP (controller-vtep)
devices should be
treated like how a packet is tunnelled from another chassis right ?
Although we need
to send to ingress pipeline to determine the outport.

Let me know if my understanding is incorrect.

Thanks
Numan

>
> I’ve tried to answer your question about ACLs in documentation.
> Please let me know if it is clear.
>
> Regards,
> Vladislav Odintsov
>
> > On 17 Sep 2021, at 22:42, Numan Siddique  wrote:
> >
> > On Fri, Sep 17, 2021 at 11:01 AM Vladislav Odintsov  > > wrote:
> >>
> >> A packet going from HW VTEP device to VIF port when arrives to
> >> hypervisor chassis should go through LS ingress pipeline to l2_lkp
> >> stage without any match. In l2_lkp stage an output port is
> >> determined and then packet passed to LS egress pipeline for futher
> >> processing and to VIF port delivery.
> >>
> >> Prior to this commit a packet, which was received from HW VTEP
> >> device was dropped in an LS ingress datapath, where stateful services
> >> were defined (ACLs, LBs).
> >>
> >> To fix this issue we add a special flag-bit which can be used in LS
> >> pipelines, to check whether the packet came from HW VTEP devices.
> >> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
> >> to skip such packets.
> >>
> >> Signed-off-by: Vladislav Odintsov 
> >
> > Hi Vladislav,
> >
> > The documentation needs to be updated for the newly added logical flows
> > and also for the updated flow in "ls_in_port_sec_l2" stage.
> >
> > I didn't review your p1 earlier and hence missed it.
> >
> > Regarding the patch,  I don't have much idea on how the ramp/vtep stuff 
> > works.
> > My question is why the traffic received from HW VTEP devices should be 
> > skipped
> > from conntrack ?  ACLs and policies don't apply to vtep logical ports ?
> >
> > Thanks
> > Numan
> >
> >> ---
> >> v1 -> v2:
> >>   - Patch rebased on upstream changes.
> >>
> >> Please note: I've got no experience in DDLog and have no ability to 
> >> extensively
> >> test these changes.
> >> Just local ./configure --with-ddlog=...; make; make check was 
> >> run
> >> It seems, that only irrelevant to these changes tests were 
> >> failed.
> >> ---
> >> northd/northd.c  | 14 ++
> >> northd/ovn_northd.dl | 33 +++--
> >> tests/ovn-northd.at  |  2 ++
> >> 3 files changed, 47 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/northd/northd.c b/northd/northd.c
> >> index 688a6e4ef..1b84874a7 100644
> >> --- a/northd/northd.c
> >> +++ b/northd/northd.c
> >> @@ -196,6 +196,7 @@ enum ovn_stage {
> >> #define REGBIT_LKUP_FDB   "reg0[11]"
> >> #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
> >> #define REGBIT_ACL_LABEL  "reg0[13]"
> >> +#define REGBIT_FROM_RAMP  "reg0[14]"
> >>
> >> #define REG_ORIG_DIP_IPV4 "reg1"
> >> #define REG_ORIG_DIP_IPV6 "xxreg1"
> >> @@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
> >> if (queue_id) {
> >> ds_put_format(actions, "set_queue(%s); ", queue_id);
> >> }
> >> +
> >> +if (!strcmp(op->nbsp->type, "vtep")) {
> >> +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
> >> +}
> >> +
> >> ds_put_cstr(actions, "next;");
> >> ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
> >> S_SWITCH_IN_PORT_SEC_L2,
> >>   50, ds_cstr(match), ds_cstr(actions),
> >> @@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
> >> *port_groups,
> >>   "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
> >>   "(udp && udp.src == 546 && udp.dst == 547)", 
> >> "next;");
> >>
> >> +/* Do not send coming from RAMP switch packets to conntrack. */
> >> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> >> +  REGBIT_FROM_RAMP" == 1", "next;");
> >> +
> >> /* Ingress and Egress Pre-ACL Table (Priority 100).
> >>  *
> >>  * Regardless of whether the ACL is "from-lport" or "to-lport",
> >> @@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
> >> *lflows,
> >> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
> >>   "eth.src == $svc_monitor_mac", "next;");
> >>
> >> +/* Do not send coming from RAMP switch packets to conntrack. */
> >> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> >> +  REGBIT_FROM_RAMP" == 1", "next;");
> >> +
> >> /* Allow all packets to go to next tables by default. */
> >> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
> >> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
> >> 

Re: [ovs-dev] [PATCH ovn v3 3/3] ic: add support for routing tables in adv/learn routes

[0-day Robot]

Bleep bloop.  Greetings Vladislav Odintsov, 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)
#356 FILE: ic/ovn-ic.c:1320:
VLOG_WARN_RL(, "Bad route format in IC-SB: %s -> %s. 
Ignored.",

WARNING: Line is 81 characters long (recommended limit is 79)
#375 FILE: ic/ovn-ic.c:1339:

nbrec_logical_router_static_route_update_external_ids_setkey(

Lines checked: 1286, 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

Re: [ovs-dev] [PATCH ovn v3 2/3] northd, utils: support for RouteTables in LRs

[0-day Robot]

Bleep bloop.  Greetings Vladislav Odintsov, 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: Inappropriate bracing around statement
#243 FILE: northd/northd.c:8878:
else {

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1553 FILE: utilities/ovn-nbctl.c:332:
  lrp-set-options PORT KEY=VALUE [KEY=VALUE]...\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1564 FILE: utilities/ovn-nbctl.c:356:
  [--policy=POLICY]\n\

WARNING: Line lacks whitespace around operator
#1566 FILE: utilities/ovn-nbctl.c:358:
  [--ecmp-symmetric-reply]\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1567 FILE: utilities/ovn-nbctl.c:359:
  [--route-table=ROUTE_TABLE]\n\

WARNING: Line lacks whitespace around operator
#1568 FILE: utilities/ovn-nbctl.c:360:
  lr-route-add ROUTER PREFIX NEXTHOP [PORT]\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1571 FILE: utilities/ovn-nbctl.c:362:
  [--policy=POLICY]\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1572 FILE: utilities/ovn-nbctl.c:363:
  [--route-table=ROUTE_TABLE]\n\

WARNING: Line lacks whitespace around operator
#1573 FILE: utilities/ovn-nbctl.c:364:
  lr-route-del ROUTER [PREFIX [NEXTHOP [PORT]]]\n\

WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#1575 FILE: utilities/ovn-nbctl.c:366:
  [--route-table=ROUTE_TABLE]\n\

WARNING: Line is 82 characters long (recommended limit is 79)
#1854 FILE: utilities/ovn-nbctl.c:7060:
  
"--may-exist,--ecmp,--ecmp-symmetric-reply,--policy=,--route-table=,--bfd?",

Lines checked: 1868, Warnings: 20, Errors: 1


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

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

Re: [ovs-dev] [PATCH ovn v3 1/3] ic: process only local port_bindings

[0-day Robot]

Bleep bloop.  Greetings Vladislav Odintsov, 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)
#48 FILE: ic/ovn-ic.c:1398:
 ctx->icsbrec_port_binding_by_ts_az)

Lines checked: 75, 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 ovn v2] northd: support HW VTEP with stateful datapath

[Vladislav Odintsov]

Hi Numan,

I’ve posted a new patch version here:
https://patchwork.ozlabs.org/project/ovn/patch/20210917215602.10633-1-odiv...@gmail.com/

I’ve tried to answer your question about ACLs in documentation.
Please let me know if it is clear.

Regards,
Vladislav Odintsov

> On 17 Sep 2021, at 22:42, Numan Siddique  wrote:
> 
> On Fri, Sep 17, 2021 at 11:01 AM Vladislav Odintsov  > wrote:
>> 
>> A packet going from HW VTEP device to VIF port when arrives to
>> hypervisor chassis should go through LS ingress pipeline to l2_lkp
>> stage without any match. In l2_lkp stage an output port is
>> determined and then packet passed to LS egress pipeline for futher
>> processing and to VIF port delivery.
>> 
>> Prior to this commit a packet, which was received from HW VTEP
>> device was dropped in an LS ingress datapath, where stateful services
>> were defined (ACLs, LBs).
>> 
>> To fix this issue we add a special flag-bit which can be used in LS
>> pipelines, to check whether the packet came from HW VTEP devices.
>> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
>> to skip such packets.
>> 
>> Signed-off-by: Vladislav Odintsov 
> 
> Hi Vladislav,
> 
> The documentation needs to be updated for the newly added logical flows
> and also for the updated flow in "ls_in_port_sec_l2" stage.
> 
> I didn't review your p1 earlier and hence missed it.
> 
> Regarding the patch,  I don't have much idea on how the ramp/vtep stuff works.
> My question is why the traffic received from HW VTEP devices should be skipped
> from conntrack ?  ACLs and policies don't apply to vtep logical ports ?
> 
> Thanks
> Numan
> 
>> ---
>> v1 -> v2:
>>   - Patch rebased on upstream changes.
>> 
>> Please note: I've got no experience in DDLog and have no ability to 
>> extensively
>> test these changes.
>> Just local ./configure --with-ddlog=...; make; make check was run
>> It seems, that only irrelevant to these changes tests were 
>> failed.
>> ---
>> northd/northd.c  | 14 ++
>> northd/ovn_northd.dl | 33 +++--
>> tests/ovn-northd.at  |  2 ++
>> 3 files changed, 47 insertions(+), 2 deletions(-)
>> 
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 688a6e4ef..1b84874a7 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -196,6 +196,7 @@ enum ovn_stage {
>> #define REGBIT_LKUP_FDB   "reg0[11]"
>> #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
>> #define REGBIT_ACL_LABEL  "reg0[13]"
>> +#define REGBIT_FROM_RAMP  "reg0[14]"
>> 
>> #define REG_ORIG_DIP_IPV4 "reg1"
>> #define REG_ORIG_DIP_IPV6 "xxreg1"
>> @@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
>> if (queue_id) {
>> ds_put_format(actions, "set_queue(%s); ", queue_id);
>> }
>> +
>> +if (!strcmp(op->nbsp->type, "vtep")) {
>> +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
>> +}
>> +
>> ds_put_cstr(actions, "next;");
>> ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
>> S_SWITCH_IN_PORT_SEC_L2,
>>   50, ds_cstr(match), ds_cstr(actions),
>> @@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
>> *port_groups,
>>   "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
>>   "(udp && udp.src == 546 && udp.dst == 547)", "next;");
>> 
>> +/* Do not send coming from RAMP switch packets to conntrack. */
>> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
>> +  REGBIT_FROM_RAMP" == 1", "next;");
>> +
>> /* Ingress and Egress Pre-ACL Table (Priority 100).
>>  *
>>  * Regardless of whether the ACL is "from-lport" or "to-lport",
>> @@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
>> *lflows,
>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
>>   "eth.src == $svc_monitor_mac", "next;");
>> 
>> +/* Do not send coming from RAMP switch packets to conntrack. */
>> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
>> +  REGBIT_FROM_RAMP" == 1", "next;");
>> +
>> /* Allow all packets to go to next tables by default. */
>> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
>> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
>> index 669728497..0202af5dc 100644
>> --- a/northd/ovn_northd.dl
>> +++ b/northd/ovn_northd.dl
>> @@ -1631,6 +1631,7 @@ function rEGBIT_ACL_HINT_BLOCK()   : istring = 
>> i"reg0[10]"
>> function rEGBIT_LKUP_FDB() : istring = i"reg0[11]"
>> function rEGBIT_HAIRPIN_REPLY(): istring = i"reg0[12]"
>> function rEGBIT_ACL_LABEL(): istring = i"reg0[13]"
>> +function rEGBIT_FROM_RAMP(): istring = i"reg0[14]"
>> 
>> function rEG_ORIG_DIP_IPV4()   : istring = i"reg1"
>> function rEG_ORIG_DIP_IPV6()   : istring = 

[ovs-dev] [PATCH ovn] northd: support HW VTEP with stateful datapath

[Vladislav Odintsov]

A packet going from HW VTEP device to VIF port when arrives to
hypervisor chassis should go through LS ingress pipeline to l2_lkp
stage without any match. In l2_lkp stage an output port is
determined and then packet passed to LS egress pipeline for futher
processing and to VIF port delivery.

Prior to this commit a packet, which was received from HW VTEP
device was dropped in an LS ingress datapath, where stateful services
were defined (ACLs, LBs).

To fix this issue we add a special flag-bit which can be used in LS
pipelines, to check whether the packet came from HW VTEP devices.
In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
to skip such packets.

Signed-off-by: Vladislav Odintsov 
---
 northd/northd.c | 14 ++
 northd/ovn-northd.8.xml | 29 +
 northd/ovn_northd.dl| 33 +++--
 tests/ovn-northd.at |  2 ++
 4 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 688a6e4ef..1b84874a7 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -196,6 +196,7 @@ enum ovn_stage {
 #define REGBIT_LKUP_FDB   "reg0[11]"
 #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
 #define REGBIT_ACL_LABEL  "reg0[13]"
+#define REGBIT_FROM_RAMP  "reg0[14]"
 
 #define REG_ORIG_DIP_IPV4 "reg1"
 #define REG_ORIG_DIP_IPV6 "xxreg1"
@@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
 if (queue_id) {
 ds_put_format(actions, "set_queue(%s); ", queue_id);
 }
+
+if (!strcmp(op->nbsp->type, "vtep")) {
+ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
+}
+
 ds_put_cstr(actions, "next;");
 ovn_lflow_add_with_lport_and_hint(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2,
   50, ds_cstr(match), ds_cstr(actions),
@@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
*port_groups,
   "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
   "(udp && udp.src == 546 && udp.dst == 547)", "next;");
 
+/* Do not send coming from RAMP switch packets to conntrack. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+  REGBIT_FROM_RAMP" == 1", "next;");
+
 /* Ingress and Egress Pre-ACL Table (Priority 100).
  *
  * Regardless of whether the ACL is "from-lport" or "to-lport",
@@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows,
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
   "eth.src == $svc_monitor_mac", "next;");
 
+/* Do not send coming from RAMP switch packets to conntrack. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
+  REGBIT_FROM_RAMP" == 1", "next;");
+
 /* Allow all packets to go to next tables by default. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index eebf0d717..7bb39d2ab 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -262,6 +262,18 @@
 logical ports on which port security is not enabled, these advance all
 packets that match the inport.
   
+  
+Logical flows for RAMP (controller-vtep) devices are created for each
+physical switch. Packets came from such devices hit these flows and set
+the 14'th bit of OVS register 0 (REG0[14]) to 1. This regbit indicates
+that packet came from RAMP (controller-vtep) device. Later in logical
+switch ingress pipeline this register is checked in ls_in_acl_pre and
+ls_in_lb_pre stages whether to skip sending packet to conntrack in
+ingress pipeline or not. Packets from RAMP devices should go though
+ingress pipeline without any flow match till ls_in_l2_lkup stage to
+determine output port. Stateful ACLs for coming from RAMP device
+packets are checked within logical switch egress pipeline.
+  
 
 
 
@@ -453,6 +465,14 @@
   processing.
 
 
+
+  This table has a priority-110 flow with the match
+  reg0[14] == 1 for all logical switch datapaths to resubmit
+  traffic to the next table. reg0[14] is the register bit,
+  which indicates that packet was received from RAMP device. Packets from
+  RAMP device are handled by ACLs only in Logical Switch egress pipeline.
+
+
 
   This table also has a priority-110 flow with the match
   eth.dst == E for all logical switch
@@ -512,6 +532,15 @@
   configured. We can now add a lflow to drop ct.inv packets.
 
 
+
+  This table has a priority-110 flow with the match
+  reg0[14] == 1 for all logical switch datapaths to resubmit
+  traffic to the next table. reg0[14] is the register bit,
+  which indicates that packet was received from RAMP device. 

Re: [ovs-dev] [PATCH ovn 2/2] Update the probe interval in main loop.

[Han Zhou]

On Fri, Sep 17, 2021 at 1:41 PM Han Zhou  wrote:
>
>
>
> On Thu, Sep 16, 2021 at 8:06 PM Zhen Wang  wrote:
> >
> > From: zhen wang 
> >
> > When ovn-northd work in HA mode, ovn-northd will not update the probe
> > interval in standby mode. This patch address the problem by updating
> > the value in main loop.
> >
>
> Thanks Zhen for the fix! Maybe the impact and steps to reproduce the
problem can be mentioned. It may be:
> step1: power off the NB/SB leader
> step2: kill the active northd (and the standbys would never take over)
>
> Acked-by: Han Zhou 
>
> > Signed-off-by: zhen wang 
> > ---
> >  northd/northd.c | 25 -
> >  northd/ovn-northd.c | 29 +
> >  2 files changed, 29 insertions(+), 25 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index b7e64470f..89b0e4921 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -72,10 +72,6 @@ static struct eth_addr svc_monitor_mac_ea;
> >   * Otherwise, it will avoid using it.  The default is true. */
> >  static bool use_ct_inv_match = true;
> >
> > -/* Default probe interval for NB and SB DB connections. */
> > -#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> > -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> > -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
> >  #define MAX_OVN_TAGS 4096
> >
> >  /* Pipeline stages. */
> > @@ -14082,20 +14078,6 @@ build_meter_groups(struct northd_context *ctx,
> >  }
> >  }
> >
> > -static int
> > -get_probe_interval(const char *db, const struct nbrec_nb_global *nb)
> > -{
> > -int default_interval = (db && !stream_or_pstream_needs_probes(db)
> > -? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
> > -int interval = smap_get_int(>options,
> > -"northd_probe_interval",
default_interval);
> > -
> > -if (interval > 0 && interval < 1000) {
> > -interval = 1000;
> > -}
> > -return interval;
> > -}
> > -
> >  static void
> >  ovnnb_db_run(struct northd_context *ctx,
> >   struct ovsdb_idl_index *sbrec_chassis_by_name,
> > @@ -14182,13 +14164,6 @@ ovnnb_db_run(struct northd_context *ctx,
> >
> >  smap_destroy();
> >
> > -/* Update the probe interval. */
> > -northd_probe_interval_nb = get_probe_interval(ctx->ovnnb_db, nb);
> > -northd_probe_interval_sb = get_probe_interval(ctx->ovnsb_db, nb);
> > -
> > -ovsdb_idl_set_probe_interval(ctx->ovnnb_idl,
northd_probe_interval_nb);
> > -ovsdb_idl_set_probe_interval(ctx->ovnsb_idl,
northd_probe_interval_sb);
> > -
> >  use_parallel_build =
> >  (smap_get_bool(>options, "use_parallel_build", false) &&
> >   can_parallelize_hashes(false));
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 6d4c5defc..0a9fd8190 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -65,6 +65,10 @@ static const char *ssl_private_key_file;
> >  static const char *ssl_certificate_file;
> >  static const char *ssl_ca_cert_file;
> >
> > +/* Default probe interval for NB and SB DB connections. */
> > +#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> > +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> > +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
> >  static bool use_parallel_build = true;
> >  static struct hashrow_locks lflow_locks;
> >
> > @@ -577,6 +581,20 @@ update_ssl_config(void)
> >  }
> >  }
> >
> > +static int
> > +get_probe_interval(const char *db, const struct nbrec_nb_global *nb)
> > +{
> > +int default_interval = (db && !stream_or_pstream_needs_probes(db)
> > +? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
> > +int interval = smap_get_int(>options,
> > +"northd_probe_interval",
default_interval);
> > +
> > +if (interval > 0 && interval < 1000) {
> > +interval = 1000;
> > +}
> > +return interval;
> > +}
> > +
> >  int
> >  main(int argc, char *argv[])
> >  {
> > @@ -911,6 +929,12 @@ main(int argc, char *argv[])
> >
> >  while (!exiting) {
> >  update_ssl_config();
> > +const struct nbrec_nb_global *nb =
nbrec_nb_global_first(ovnnb_idl_loop.idl);
> > +/* Update the probe interval. */
> > +if (nb) {
> > +northd_probe_interval_nb = get_probe_interval(ovnnb_db,
nb);
> > +northd_probe_interval_sb = get_probe_interval(ovnsb_db,
nb);
> > +}

Sorry that I forgot a minor comment here: it is better to move this down,
right before setting the probe intervals. Otherwise, since this is before
ovsdb_idl_loop_run(), it reads the config from the last iteration, and the
new setting (if updated) will be applied at the next iteration only.

> >  memory_run();
> >  if (memory_should_report()) {
> >  struct simap usage = SIMAP_INITIALIZER();
> > @@ -1000,6 +1024,11 @@ main(int argc, char *argv[])
> >  

[ovs-dev] [PATCH ovn v3 3/3] ic: add support for routing tables in adv/learn routes

[Vladislav Odintsov]

Previously support for multiple routing tables was added
to northd code.
This commit expands support for multiple routing tables
by adding support of advertising and learning routes with
their routing table information.

To utilize such feature, user must:
1. create Logical Router in each AZ;
2. create IC transit switch for each routing table, that
   he/she needs;
3. connect each TS with this LR;
4. assign routing table for TS's LRP
   (ovn-nbctl lrp-set-options  route_table=<>);
5. enable routes sync (turn on learning and advertising
   routes in NB_Global table);
6. create LRPs for subnets in LR, create static routes
   with supplying route_table parameter.

Note 1: routes for directly-connected networks will be
learned to global routing table and if Logical Routers
have more than one Transit Switch, which interconnects
them, directly-connected routes will be added via each
transit switch port and configured as ECMP routes.

Note 2: static routes within route tables will be advertised
and learned only if interconnecting transit switch's LRPs
will have options:route_table same value as route's route_table
value.

Signed-off-by: Vladislav Odintsov 
---
 NEWS|   4 +
 ic/ovn-ic.c | 533 
 ovn-ic-sb.ovsschema |   5 +-
 ovn-ic-sb.xml   |  18 ++
 tests/ovn-ic.at | 440 
 5 files changed, 807 insertions(+), 193 deletions(-)

diff --git a/NEWS b/NEWS
index 8a21c029e..3855f0d48 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,10 @@ OVN v21.09.0 - xx xxx 
   - Allow static routes without nexthops.
   - Enabled logical dp groups as a default.  CMS should disable it if not
 desired.
+  - Added support for multiple routing tables in Logical Router Static Routes
+and LRPs. OVN Interconnection supports routes' route tables as well.
+This requires to update schemas for OVN_Northdbound and OVN_IC_Southbound
+DBs.
 
 OVN v21.06.0 - 18 Jun 2021
 -
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 92c83d730..3617c7235 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -63,9 +63,11 @@ struct ic_context {
 struct ovsdb_idl_txn *ovninb_txn;
 struct ovsdb_idl_txn *ovnisb_txn;
 struct ovsdb_idl_index *nbrec_ls_by_name;
+struct ovsdb_idl_index *nbrec_lrp_by_name;
 struct ovsdb_idl_index *nbrec_port_by_name;
 struct ovsdb_idl_index *sbrec_chassis_by_name;
 struct ovsdb_idl_index *sbrec_port_binding_by_name;
+struct ovsdb_idl_index *icnbrec_transit_switch_by_name;
 struct ovsdb_idl_index *icsbrec_port_binding_by_az;
 struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
 struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az;
@@ -773,7 +775,7 @@ port_binding_run(struct ic_context *ctx,
 icsbrec_port_binding_index_set_transit_switch(isb_pb_key, ts->name);
 
 ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key,
-ctx->icsbrec_port_binding_by_ts) {
+ ctx->icsbrec_port_binding_by_ts) {
 if (isb_pb->availability_zone == az) {
 shash_add(_pbs, isb_pb->logical_port, isb_pb);
 shash_find_and_delete(_all_local_pbs,
@@ -844,7 +846,9 @@ port_binding_run(struct ic_context *ctx,
 struct ic_router_info {
 struct hmap_node node;
 const struct nbrec_logical_router *lr; /* key of hmap */
-const struct icsbrec_port_binding *isb_pb;
+const struct icsbrec_port_binding **isb_pbs;
+size_t n_isb_pbs;
+size_t n_allocated_isb_pbs;
 struct hmap routes_learned;
 };
 
@@ -854,6 +858,7 @@ struct ic_route_info {
 struct in6_addr prefix;
 unsigned int plen;
 struct in6_addr nexthop;
+const char *route_table;
 
 /* Either nb_route or nb_lrp is set and the other one must be NULL.
  * - For a route that is learned from IC-SB, or a static route that is
@@ -875,13 +880,15 @@ ic_route_hash(const struct in6_addr *prefix, unsigned int 
plen,
 
 static struct ic_route_info *
 ic_route_find(struct hmap *routes, const struct in6_addr *prefix,
-  unsigned int plen, const struct in6_addr *nexthop)
+  unsigned int plen, const struct in6_addr *nexthop,
+  char *route_table)
 {
 struct ic_route_info *r;
 uint32_t hash = ic_route_hash(prefix, plen, nexthop);
 HMAP_FOR_EACH_WITH_HASH (r, node, hash, routes) {
 if (ipv6_addr_equals(>prefix, prefix) &&
 r->plen == plen &&
+!strcmp(r->route_table ? r->route_table : "", route_table) &&
 ipv6_addr_equals(>nexthop, nexthop)) {
 return r;
 }
@@ -926,11 +933,19 @@ add_to_routes_learned(struct hmap *routes_learned,
  , , )) {
 return false;
 }
+
+if (ic_route_find(routes_learned, , plen, ,
+  nb_route->route_table)) {
+/* Route is already added to learned in previous iteration. */
+

[ovs-dev] [PATCH ovn v3 2/3] northd, utils: support for RouteTables in LRs

[Vladislav Odintsov]

This patch extends Logical Router's routing functionality.
Now user may create multiple routing tables within a Logical Router
and assign them to Logical Router Ports.

Traffic coming from Logical Router Port with assigned route_table
is checked against global routes if any (Logical_Router_Static_Routes
whith empty route_table field), next against directly connected routes
and then Logical_Router_Static_Routes with same route_table value as
in Logical_Router_Port options:route_table field.

A new Logical Router ingress table #10 is added - LR_IN_IP_ROUTING_PRE.
In this table packets which come from LRPs with configured
options:route_table field are checked against inport and in OVS
register 7 unique non-zero value identifying route table is written.

Then in 11th table LR_IN_IP_ROUTING routes which have non-empty
`route_table` field are added with additional match on reg7 value
associated with appropriate route_table.

Signed-off-by: Vladislav Odintsov 
---
 northd/northd.c | 160 ---
 northd/ovn-northd.8.xml |  63 --
 ovn-nb.ovsschema|   5 +-
 ovn-nb.xml  |  30 +++
 tests/ovn-ic.at |   4 +
 tests/ovn-nbctl.at  | 196 +-
 tests/ovn-northd.at |  76 ++-
 tests/ovn.at| 441 +++-
 utilities/ovn-nbctl.c   | 134 +++-
 9 files changed, 1042 insertions(+), 67 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 621e83175..720bf12b7 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -152,15 +152,16 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  ECMP_STATEFUL,   7, "lr_in_ecmp_stateful") \
 PIPELINE_STAGE(ROUTER, IN,  ND_RA_OPTIONS,   8, "lr_in_nd_ra_options") \
 PIPELINE_STAGE(ROUTER, IN,  ND_RA_RESPONSE,  9, "lr_in_nd_ra_response") \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  10, "lr_in_ip_routing")   \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_ECMP, 11, "lr_in_ip_routing_ecmp") \
-PIPELINE_STAGE(ROUTER, IN,  POLICY,  12, "lr_in_policy")   \
-PIPELINE_STAGE(ROUTER, IN,  POLICY_ECMP, 13, "lr_in_policy_ecmp")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 14, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   ,  15, "lr_in_chk_pkt_len")  \
-PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 16, "lr_in_larger_pkts")  \
-PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 17, "lr_in_gw_redirect")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 18, "lr_in_arp_request")  \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_PRE,  10, "lr_in_ip_routing_pre")  \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  11, "lr_in_ip_routing")  \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_ECMP, 12, "lr_in_ip_routing_ecmp") \
+PIPELINE_STAGE(ROUTER, IN,  POLICY,  13, "lr_in_policy")  \
+PIPELINE_STAGE(ROUTER, IN,  POLICY_ECMP, 14, "lr_in_policy_ecmp") \
+PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 15, "lr_in_arp_resolve") \
+PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN, 16, "lr_in_chk_pkt_len") \
+PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 17, "lr_in_larger_pkts") \
+PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 18, "lr_in_gw_redirect") \
+PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 19, "lr_in_arp_request") \
   \
 /* Logical router egress stages. */   \
 PIPELINE_STAGE(ROUTER, OUT, UNDNAT,  0, "lr_out_undnat")\
@@ -228,6 +229,7 @@ enum ovn_stage {
 #define REG_NEXT_HOP_IPV6 "xxreg0"
 #define REG_SRC_IPV4 "reg1"
 #define REG_SRC_IPV6 "xxreg1"
+#define REG_ROUTE_TABLE_ID "reg7"
 
 #define REG_ORIG_TP_DPORT_ROUTER   "reg9[16..31]"
 
@@ -290,8 +292,9 @@ enum ovn_stage {
  * | R6  |UNUSED| X | | G | IN_IP_ROUTING)|
  * | |  | R | | 1 |   |
  * +-+--+ E | UNUSED  |   |   |
- * | R7  |UNUSED| G | |   |   |
- * | |  | 3 | |   |   |
+ * | R7  |  ROUTE_TABLE_ID  | G | |   |   |
+ * | | (>= IN_IP_ROUTING_PRE && | 3 | |   |   |
+ * | |  <= IN_IP_ROUTING)   |   | |   |   |
  * +-+--+---+-+---+---+
  * | R8  | ECMP_GROUP_ID|   | |
  * | | ECMP_MEMBER_ID   | X | |
@@ -8506,11 +8509,72 @@ cleanup:
 ds_destroy();
 }
 
+static uint32_t
+route_table_add(struct simap *route_tables, const char *route_table_name)
+{
+/* route table ids start from 1 */
+uint32_t rtb_id = simap_count(route_tables) + 1;
+
+if (rtb_id == UINT16_MAX) {
+static struct vlog_rate_limit rl = 

[ovs-dev] [PATCH ovn v3 1/3] ic: process only local port_bindings

[Vladislav Odintsov]

This commit adds a small optimization by utilizing ovsdb_index
to iterate over port_bindings.
Prior to this change each iteration checked availability_zone
and continued processing only if port_binding belons to local AZ.

Now we run against port_bindings from local AZ only and don't check
availability_zone.

Signed-off-by: Vladislav Odintsov 
---
 ic/ovn-ic.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 99356253d..92c83d730 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -68,6 +68,7 @@ struct ic_context {
 struct ovsdb_idl_index *sbrec_port_binding_by_name;
 struct ovsdb_idl_index *icsbrec_port_binding_by_az;
 struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
+struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az;
 struct ovsdb_idl_index *icsbrec_route_by_ts;
 struct ovsdb_idl_index *icsbrec_route_by_ts_az;
 };
@@ -1386,17 +1387,15 @@ route_run(struct ic_context *ctx,
 const struct icsbrec_port_binding *isb_pb;
 const struct icsbrec_port_binding *isb_pb_key =
 icsbrec_port_binding_index_init_row(
-ctx->icsbrec_port_binding_by_ts);
+ctx->icsbrec_port_binding_by_ts_az);
 icsbrec_port_binding_index_set_transit_switch(isb_pb_key, ts->name);
+icsbrec_port_binding_index_set_availability_zone(isb_pb_key, az);
 
 /* Each port on TS maps to a logical router, which is stored in the
  * external_ids:router-id of the IC SB port_binding record. */
 ICSBREC_PORT_BINDING_FOR_EACH_EQUAL (isb_pb, isb_pb_key,
- ctx->icsbrec_port_binding_by_ts) {
-if (isb_pb->availability_zone != az) {
-continue;
-}
-
+ 
ctx->icsbrec_port_binding_by_ts_az)
+{
 const char *ts_lrp_name =
 get_lrp_name_by_ts_port_name(ctx, isb_pb->logical_port);
 if (!ts_lrp_name) {
@@ -1713,6 +1712,11 @@ main(int argc, char *argv[])
 = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
   _port_binding_col_transit_switch);
 
+struct ovsdb_idl_index *icsbrec_port_binding_by_ts_az
+= ovsdb_idl_index_create2(ovnisb_idl_loop.idl,
+  _port_binding_col_transit_switch,
+  _port_binding_col_availability_zone);
+
 struct ovsdb_idl_index *icsbrec_route_by_ts
 = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
   _route_col_transit_switch);
@@ -1763,6 +1767,7 @@ main(int argc, char *argv[])
 .sbrec_chassis_by_name = sbrec_chassis_by_name,
 .icsbrec_port_binding_by_az = icsbrec_port_binding_by_az,
 .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts,
+.icsbrec_port_binding_by_ts_az = icsbrec_port_binding_by_ts_az,
 .icsbrec_route_by_ts = icsbrec_route_by_ts,
 .icsbrec_route_by_ts_az = icsbrec_route_by_ts_az,
 };
-- 
2.30.0

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

[ovs-dev] [PATCH ovn v3 0/3] Add multiple routing tables support to Logical Routers

[Vladislav Odintsov]

This patch series extends Logical Router's routing functionality.
Now user may create multiple routing tables within a Logical Router
and assign them to Logical Router Ports.

Traffic coming from Logical Router Port with assigned route_table
is checked against global routes if any (Logical_Router_Static_Routes
whith empty route_table field), next against directly connected routes
and then Logical_Router_Static_Routes with same route_table value as
in Logical_Router_Port options:route_table field.

---
v2 -> v3:
  - Rebased on split northd changes.
  - Replaced route_tables HMAP with SIMAP as Numan suggested.
  - This series stil doesn't have ddlog support yet.
It will take too much time for me to deal to ddlog language and specifics.
Help with ddlog implementation wanted.

v1 -> v2:
  - First patch of v1 patch series was applied, but new tests for new feature
were added with strict table number check. Update this tests to be table
number-independent.
  - Squash pathes for northd and utilities as tests don't pass without latter.
  - Add support for OVN IC routing table in routes advertisement/learning.
  - Patches `ic: remove port_binding on ts deletion`

https://patchwork.ozlabs.org/project/ovn/patch/20210824184442.35063-1-odiv...@gmail.com/
and `ic: process only local port_bindings`

https://patchwork.ozlabs.org/project/ovn/patch/20210830195707.98529-1-odiv...@gmail.com/
were already sent to list separately, but other changes are based on them
so they're included.
Once those patches are accepts, I can drop them from this series.
  - Added NEWS item.
  - Added myself to authors list.

Vladislav Odintsov (3):
  ic: process only local port_bindings
  northd,utils: support for RouteTables in LRs
  ic: add support for routing tables in adv/learn routes

 NEWS|   4 +
 ic/ovn-ic.c | 542 ++--
 northd/northd.c | 160 +---
 northd/ovn-northd.8.xml |  63 +++--
 ovn-ic-sb.ovsschema |   5 +-
 ovn-ic-sb.xml   |  18 ++
 ovn-nb.ovsschema|   5 +-
 ovn-nb.xml  |  30 +++
 tests/ovn-ic.at | 444 
 tests/ovn-nbctl.at  | 196 ++-
 tests/ovn-northd.at |  76 +-
 tests/ovn.at| 441 +++-
 utilities/ovn-nbctl.c   | 134 +-
 13 files changed, 1856 insertions(+), 262 deletions(-)

-- 
2.30.0

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

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Han Zhou]

On Fri, Sep 17, 2021 at 2:04 PM Numan Siddique  wrote:
>
> On Fri, Sep 17, 2021 at 5:02 PM Renat Nurgaliyev 
wrote:
> >
> > Hi Han,
> >
> > yes, I believe you are totally right. But it still feels like a chicken
and
> > egg problem to me, storing the database timeout setting inside the
database
> > itself. If there would be at least some local command line argument to
> > override timeout value, it would be already amazing, because currently
> > there is no way to control it before the database connection is made,
and
> > if it cannot be made, it is too late to try to control it.
> >
>
> What about the case where the NB database is huge and it takes > 5
> seconds to fetch
> all the contents ?
>
I think Renat had the answer to this question: using the DB to configure
the probe interval to the DB is going to be a problem in certain cases. It
should be fine to use NB to configure probe interval for SB, but using NB
to configure the probe interval for NB itself is definitely causing the
problem when the NB is huge. Support command line options may be the right
approach. But in practice would it be good enough to use 60s as the default
value instead of 5s?

> Numan
>
> > Thanks,
> > Renat.
> >
> > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:55:
> >
> > >
> > >
> > > On Fri, Sep 17, 2021 at 1:48 PM Renat Nurgaliyev 
> > > wrote:
> > > >
> > > > Hello Han,
> > > >
> > > > when I wrote this patch we had an issue with a very big SB database,
> > > around 1,5 gigabytes. There were no controllers or northds running,
so the
> > > database server was without any load at all. Although OVSDB was
idling,
> > > even a single northd process could not fully connect to the database
due to
> > > its size, since it could not fetch and process the data in 5 seconds.
> > >
> > > Hi Renat, thanks for the explanation. However, suppose SB is still
huge,
> > > if NB is not that big, the probe config in NB_Global will soon be
applied
> > > to ovn-northd, which would probe in proper interval (desired setting
with
> > > the SB size considered) instead of the default 5 sec, and it should
> > > succeed, right?
> > >
> > > Thanks,
> > > Han
> > >
> > > >
> > > > Since then many optimizations were made, and the database size with
the
> > > same topology reduced to approximately twenty megabytes, so today I
> > > wouldn't be able to reproduce the problem.
> > > >
> > > > However, I am quite sure that it would still cause troubles with a
huge
> > > scale, when SB grows to hundreds of megabytes. With the default
timeout of
> > > 5 seconds, which is implemented in the same thread that also fetches
and
> > > processes data, we make an artificial database size limit, which is
not so
> > > obvoius to troubleshoot.
> > > >
> > > > Regards,
> > > > Renat.
> > > >
> > > > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:
> > > >>
> > > >>
> > > >>
> > > >> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang 
wrote:
> > > >> >
> > > >> > From: zhen wang 
> > > >> >
> > > >> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> > > >> > Above commit introduced a bug when muptiple ovn-northd instances
work
> > > in HA
> > > >> > mode. If SB leader and active ovn-northd instance got killed by
> > > system power
> > > >> > outage, standby ovn-northd instance would never detect the
failure.
> > > >> >
> > > >>
> > > >> Thanks Zhen! I added the Renat and Numan who worked on the reverted
> > > commit to CC, so that they can comment if this is ok.
> > > >>
> > > >> For the commit message, I think it may be decoupled from the HA
> > > scenario that is supposed to be fixed by the other patch in this
series.
> > > The issue this patch fixes is that before the initial NB downloading
is
> > > complete the northd will not send probe, so if the DB server is down
> > > (ungracefully) before the northd reads the NB_Global options, the
northd
> > > would never probe, thus never reconnect to the new leader. (it is
related
> > > to RAFT, but whether it is multiple northds is irrelevant)
> > > >>
> > > >> As to the original commit that is reverted by this one:
> > > >>
> > > >> northd: Don't poll ovsdb before the connection is fully
established
> > > >>
> > > >> Set initial SB and NB DBs probe interval to 0 to avoid
connection
> > > >> flapping.
> > > >>
> > > >> Before configured in northd_probe_interval value is actually
applied
> > > >> to southbound and northbound database connections, both
connections
> > > >> must be fully established, otherwise ovnnb_db_run() will return
> > > >> without retrieving configuration data from northbound DB. In
cases
> > > >> when southbound database is big enough, default interval of 5
> > > seconds
> > > >> will kill and retry the connection before it is fully
established,
> > > no
> > > >> matter what is set in northd_probe_interval. Client reconnect
will
> > > >> cause even more load to ovsdb-server and cause cascade effect,
so
> > > >> northd can never stabilise. We have more than 

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Renat Nurgaliyev]

Hi Numan,

according to our experience, in such case the connection can never be fully
established, and northd ends up being in an endless loop with 100% CPU
utilization.

Numan Siddique  schrieb am Sa., 18. Sept. 2021, 00:05:

> On Fri, Sep 17, 2021 at 5:02 PM Renat Nurgaliyev 
> wrote:
> >
> > Hi Han,
> >
> > yes, I believe you are totally right. But it still feels like a chicken
> and
> > egg problem to me, storing the database timeout setting inside the
> database
> > itself. If there would be at least some local command line argument to
> > override timeout value, it would be already amazing, because currently
> > there is no way to control it before the database connection is made, and
> > if it cannot be made, it is too late to try to control it.
> >
>
> What about the case where the NB database is huge and it takes > 5
> seconds to fetch
> all the contents ?
>
> Numan
>
> > Thanks,
> > Renat.
> >
> > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:55:
> >
> > >
> > >
> > > On Fri, Sep 17, 2021 at 1:48 PM Renat Nurgaliyev 
> > > wrote:
> > > >
> > > > Hello Han,
> > > >
> > > > when I wrote this patch we had an issue with a very big SB database,
> > > around 1,5 gigabytes. There were no controllers or northds running, so
> the
> > > database server was without any load at all. Although OVSDB was idling,
> > > even a single northd process could not fully connect to the database
> due to
> > > its size, since it could not fetch and process the data in 5 seconds.
> > >
> > > Hi Renat, thanks for the explanation. However, suppose SB is still
> huge,
> > > if NB is not that big, the probe config in NB_Global will soon be
> applied
> > > to ovn-northd, which would probe in proper interval (desired setting
> with
> > > the SB size considered) instead of the default 5 sec, and it should
> > > succeed, right?
> > >
> > > Thanks,
> > > Han
> > >
> > > >
> > > > Since then many optimizations were made, and the database size with
> the
> > > same topology reduced to approximately twenty megabytes, so today I
> > > wouldn't be able to reproduce the problem.
> > > >
> > > > However, I am quite sure that it would still cause troubles with a
> huge
> > > scale, when SB grows to hundreds of megabytes. With the default
> timeout of
> > > 5 seconds, which is implemented in the same thread that also fetches
> and
> > > processes data, we make an artificial database size limit, which is
> not so
> > > obvoius to troubleshoot.
> > > >
> > > > Regards,
> > > > Renat.
> > > >
> > > > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:
> > > >>
> > > >>
> > > >>
> > > >> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang 
> wrote:
> > > >> >
> > > >> > From: zhen wang 
> > > >> >
> > > >> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> > > >> > Above commit introduced a bug when muptiple ovn-northd instances
> work
> > > in HA
> > > >> > mode. If SB leader and active ovn-northd instance got killed by
> > > system power
> > > >> > outage, standby ovn-northd instance would never detect the
> failure.
> > > >> >
> > > >>
> > > >> Thanks Zhen! I added the Renat and Numan who worked on the reverted
> > > commit to CC, so that they can comment if this is ok.
> > > >>
> > > >> For the commit message, I think it may be decoupled from the HA
> > > scenario that is supposed to be fixed by the other patch in this
> series.
> > > The issue this patch fixes is that before the initial NB downloading is
> > > complete the northd will not send probe, so if the DB server is down
> > > (ungracefully) before the northd reads the NB_Global options, the
> northd
> > > would never probe, thus never reconnect to the new leader. (it is
> related
> > > to RAFT, but whether it is multiple northds is irrelevant)
> > > >>
> > > >> As to the original commit that is reverted by this one:
> > > >>
> > > >> northd: Don't poll ovsdb before the connection is fully
> established
> > > >>
> > > >> Set initial SB and NB DBs probe interval to 0 to avoid
> connection
> > > >> flapping.
> > > >>
> > > >> Before configured in northd_probe_interval value is actually
> applied
> > > >> to southbound and northbound database connections, both
> connections
> > > >> must be fully established, otherwise ovnnb_db_run() will return
> > > >> without retrieving configuration data from northbound DB. In
> cases
> > > >> when southbound database is big enough, default interval of 5
> > > seconds
> > > >> will kill and retry the connection before it is fully
> established,
> > > no
> > > >> matter what is set in northd_probe_interval. Client reconnect
> will
> > > >> cause even more load to ovsdb-server and cause cascade effect,
> so
> > > >> northd can never stabilise. We have more than 2000 ports in our
> lab,
> > > >> and northd could not start before this patch, holding at 100%
> CPU
> > > >> utilisation both itself and ovsdb-server.
> > > >>
> > > >> After connections are established, any value in
> > > 

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Numan Siddique]

On Fri, Sep 17, 2021 at 5:02 PM Renat Nurgaliyev  wrote:
>
> Hi Han,
>
> yes, I believe you are totally right. But it still feels like a chicken and
> egg problem to me, storing the database timeout setting inside the database
> itself. If there would be at least some local command line argument to
> override timeout value, it would be already amazing, because currently
> there is no way to control it before the database connection is made, and
> if it cannot be made, it is too late to try to control it.
>

What about the case where the NB database is huge and it takes > 5
seconds to fetch
all the contents ?

Numan

> Thanks,
> Renat.
>
> Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:55:
>
> >
> >
> > On Fri, Sep 17, 2021 at 1:48 PM Renat Nurgaliyev 
> > wrote:
> > >
> > > Hello Han,
> > >
> > > when I wrote this patch we had an issue with a very big SB database,
> > around 1,5 gigabytes. There were no controllers or northds running, so the
> > database server was without any load at all. Although OVSDB was idling,
> > even a single northd process could not fully connect to the database due to
> > its size, since it could not fetch and process the data in 5 seconds.
> >
> > Hi Renat, thanks for the explanation. However, suppose SB is still huge,
> > if NB is not that big, the probe config in NB_Global will soon be applied
> > to ovn-northd, which would probe in proper interval (desired setting with
> > the SB size considered) instead of the default 5 sec, and it should
> > succeed, right?
> >
> > Thanks,
> > Han
> >
> > >
> > > Since then many optimizations were made, and the database size with the
> > same topology reduced to approximately twenty megabytes, so today I
> > wouldn't be able to reproduce the problem.
> > >
> > > However, I am quite sure that it would still cause troubles with a huge
> > scale, when SB grows to hundreds of megabytes. With the default timeout of
> > 5 seconds, which is implemented in the same thread that also fetches and
> > processes data, we make an artificial database size limit, which is not so
> > obvoius to troubleshoot.
> > >
> > > Regards,
> > > Renat.
> > >
> > > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:
> > >>
> > >>
> > >>
> > >> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang  wrote:
> > >> >
> > >> > From: zhen wang 
> > >> >
> > >> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> > >> > Above commit introduced a bug when muptiple ovn-northd instances work
> > in HA
> > >> > mode. If SB leader and active ovn-northd instance got killed by
> > system power
> > >> > outage, standby ovn-northd instance would never detect the failure.
> > >> >
> > >>
> > >> Thanks Zhen! I added the Renat and Numan who worked on the reverted
> > commit to CC, so that they can comment if this is ok.
> > >>
> > >> For the commit message, I think it may be decoupled from the HA
> > scenario that is supposed to be fixed by the other patch in this series.
> > The issue this patch fixes is that before the initial NB downloading is
> > complete the northd will not send probe, so if the DB server is down
> > (ungracefully) before the northd reads the NB_Global options, the northd
> > would never probe, thus never reconnect to the new leader. (it is related
> > to RAFT, but whether it is multiple northds is irrelevant)
> > >>
> > >> As to the original commit that is reverted by this one:
> > >>
> > >> northd: Don't poll ovsdb before the connection is fully established
> > >>
> > >> Set initial SB and NB DBs probe interval to 0 to avoid connection
> > >> flapping.
> > >>
> > >> Before configured in northd_probe_interval value is actually applied
> > >> to southbound and northbound database connections, both connections
> > >> must be fully established, otherwise ovnnb_db_run() will return
> > >> without retrieving configuration data from northbound DB. In cases
> > >> when southbound database is big enough, default interval of 5
> > seconds
> > >> will kill and retry the connection before it is fully established,
> > no
> > >> matter what is set in northd_probe_interval. Client reconnect will
> > >> cause even more load to ovsdb-server and cause cascade effect, so
> > >> northd can never stabilise. We have more than 2000 ports in our lab,
> > >> and northd could not start before this patch, holding at 100% CPU
> > >> utilisation both itself and ovsdb-server.
> > >>
> > >> After connections are established, any value in
> > northd_probe_interval,
> > >> or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly.
> > >>
> > >> I am not sure how would the commit help. There are at most 3 - 5
> > northds (in practice), and suppose there are tens or hundreds of
> > ovn-controllers that makes SB busy, it is just 3 - 5 more clients retrying
> > reconnect SB for several times, and if NB is not that busy (most likely),
> > these northd clients should get the proper probe settings applied soon
> > without causing more issues 

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Renat Nurgaliyev]

Hi Han,

yes, I believe you are totally right. But it still feels like a chicken and
egg problem to me, storing the database timeout setting inside the database
itself. If there would be at least some local command line argument to
override timeout value, it would be already amazing, because currently
there is no way to control it before the database connection is made, and
if it cannot be made, it is too late to try to control it.

Thanks,
Renat.

Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:55:

>
>
> On Fri, Sep 17, 2021 at 1:48 PM Renat Nurgaliyev 
> wrote:
> >
> > Hello Han,
> >
> > when I wrote this patch we had an issue with a very big SB database,
> around 1,5 gigabytes. There were no controllers or northds running, so the
> database server was without any load at all. Although OVSDB was idling,
> even a single northd process could not fully connect to the database due to
> its size, since it could not fetch and process the data in 5 seconds.
>
> Hi Renat, thanks for the explanation. However, suppose SB is still huge,
> if NB is not that big, the probe config in NB_Global will soon be applied
> to ovn-northd, which would probe in proper interval (desired setting with
> the SB size considered) instead of the default 5 sec, and it should
> succeed, right?
>
> Thanks,
> Han
>
> >
> > Since then many optimizations were made, and the database size with the
> same topology reduced to approximately twenty megabytes, so today I
> wouldn't be able to reproduce the problem.
> >
> > However, I am quite sure that it would still cause troubles with a huge
> scale, when SB grows to hundreds of megabytes. With the default timeout of
> 5 seconds, which is implemented in the same thread that also fetches and
> processes data, we make an artificial database size limit, which is not so
> obvoius to troubleshoot.
> >
> > Regards,
> > Renat.
> >
> > Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:
> >>
> >>
> >>
> >> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang  wrote:
> >> >
> >> > From: zhen wang 
> >> >
> >> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> >> > Above commit introduced a bug when muptiple ovn-northd instances work
> in HA
> >> > mode. If SB leader and active ovn-northd instance got killed by
> system power
> >> > outage, standby ovn-northd instance would never detect the failure.
> >> >
> >>
> >> Thanks Zhen! I added the Renat and Numan who worked on the reverted
> commit to CC, so that they can comment if this is ok.
> >>
> >> For the commit message, I think it may be decoupled from the HA
> scenario that is supposed to be fixed by the other patch in this series.
> The issue this patch fixes is that before the initial NB downloading is
> complete the northd will not send probe, so if the DB server is down
> (ungracefully) before the northd reads the NB_Global options, the northd
> would never probe, thus never reconnect to the new leader. (it is related
> to RAFT, but whether it is multiple northds is irrelevant)
> >>
> >> As to the original commit that is reverted by this one:
> >>
> >> northd: Don't poll ovsdb before the connection is fully established
> >>
> >> Set initial SB and NB DBs probe interval to 0 to avoid connection
> >> flapping.
> >>
> >> Before configured in northd_probe_interval value is actually applied
> >> to southbound and northbound database connections, both connections
> >> must be fully established, otherwise ovnnb_db_run() will return
> >> without retrieving configuration data from northbound DB. In cases
> >> when southbound database is big enough, default interval of 5
> seconds
> >> will kill and retry the connection before it is fully established,
> no
> >> matter what is set in northd_probe_interval. Client reconnect will
> >> cause even more load to ovsdb-server and cause cascade effect, so
> >> northd can never stabilise. We have more than 2000 ports in our lab,
> >> and northd could not start before this patch, holding at 100% CPU
> >> utilisation both itself and ovsdb-server.
> >>
> >> After connections are established, any value in
> northd_probe_interval,
> >> or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly.
> >>
> >> I am not sure how would the commit help. There are at most 3 - 5
> northds (in practice), and suppose there are tens or hundreds of
> ovn-controllers that makes SB busy, it is just 3 - 5 more clients retrying
> reconnect SB for several times, and if NB is not that busy (most likely),
> these northd clients should get the proper probe settings applied soon
> without causing more issues at all. So I don't think the default probe 5
> sec would cause cascade effect for the initial period. @Renat @Numan please
> correct me if I am wrong.
> >>
> >> Thanks,
> >> Han
> >>
> >> > Signed-off-by: zhen wang 
> >> > ---
> >> >  northd/northd.c | 4 ++--
> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/northd/northd.c b/northd/northd.c
> >> 

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Han Zhou]

On Fri, Sep 17, 2021 at 1:48 PM Renat Nurgaliyev  wrote:
>
> Hello Han,
>
> when I wrote this patch we had an issue with a very big SB database,
around 1,5 gigabytes. There were no controllers or northds running, so the
database server was without any load at all. Although OVSDB was idling,
even a single northd process could not fully connect to the database due to
its size, since it could not fetch and process the data in 5 seconds.

Hi Renat, thanks for the explanation. However, suppose SB is still huge, if
NB is not that big, the probe config in NB_Global will soon be applied to
ovn-northd, which would probe in proper interval (desired setting with the
SB size considered) instead of the default 5 sec, and it should succeed,
right?

Thanks,
Han

>
> Since then many optimizations were made, and the database size with the
same topology reduced to approximately twenty megabytes, so today I
wouldn't be able to reproduce the problem.
>
> However, I am quite sure that it would still cause troubles with a huge
scale, when SB grows to hundreds of megabytes. With the default timeout of
5 seconds, which is implemented in the same thread that also fetches and
processes data, we make an artificial database size limit, which is not so
obvoius to troubleshoot.
>
> Regards,
> Renat.
>
> Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:
>>
>>
>>
>> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang  wrote:
>> >
>> > From: zhen wang 
>> >
>> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
>> > Above commit introduced a bug when muptiple ovn-northd instances work
in HA
>> > mode. If SB leader and active ovn-northd instance got killed by system
power
>> > outage, standby ovn-northd instance would never detect the failure.
>> >
>>
>> Thanks Zhen! I added the Renat and Numan who worked on the reverted
commit to CC, so that they can comment if this is ok.
>>
>> For the commit message, I think it may be decoupled from the HA scenario
that is supposed to be fixed by the other patch in this series. The issue
this patch fixes is that before the initial NB downloading is complete the
northd will not send probe, so if the DB server is down (ungracefully)
before the northd reads the NB_Global options, the northd would never
probe, thus never reconnect to the new leader. (it is related to RAFT, but
whether it is multiple northds is irrelevant)
>>
>> As to the original commit that is reverted by this one:
>>
>> northd: Don't poll ovsdb before the connection is fully established
>>
>> Set initial SB and NB DBs probe interval to 0 to avoid connection
>> flapping.
>>
>> Before configured in northd_probe_interval value is actually applied
>> to southbound and northbound database connections, both connections
>> must be fully established, otherwise ovnnb_db_run() will return
>> without retrieving configuration data from northbound DB. In cases
>> when southbound database is big enough, default interval of 5 seconds
>> will kill and retry the connection before it is fully established, no
>> matter what is set in northd_probe_interval. Client reconnect will
>> cause even more load to ovsdb-server and cause cascade effect, so
>> northd can never stabilise. We have more than 2000 ports in our lab,
>> and northd could not start before this patch, holding at 100% CPU
>> utilisation both itself and ovsdb-server.
>>
>> After connections are established, any value in
northd_probe_interval,
>> or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly.
>>
>> I am not sure how would the commit help. There are at most 3 - 5 northds
(in practice), and suppose there are tens or hundreds of ovn-controllers
that makes SB busy, it is just 3 - 5 more clients retrying reconnect SB for
several times, and if NB is not that busy (most likely), these northd
clients should get the proper probe settings applied soon without causing
more issues at all. So I don't think the default probe 5 sec would cause
cascade effect for the initial period. @Renat @Numan please correct me if I
am wrong.
>>
>> Thanks,
>> Han
>>
>> > Signed-off-by: zhen wang 
>> > ---
>> >  northd/northd.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/northd/northd.c b/northd/northd.c
>> > index 688a6e4ef..b7e64470f 100644
>> > --- a/northd/northd.c
>> > +++ b/northd/northd.c
>> > @@ -74,8 +74,8 @@ static bool use_ct_inv_match = true;
>> >
>> >  /* Default probe interval for NB and SB DB connections. */
>> >  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>> > -static int northd_probe_interval_nb = 0;
>> > -static int northd_probe_interval_sb = 0;
>> > +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
>> > +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
>> >  #define MAX_OVN_TAGS 4096
>> >
>> >  /* Pipeline stages. */
>> > --
>> > 2.20.1
>> >
___
dev mailing list
d...@openvswitch.org

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Renat Nurgaliyev]

Hello Han,

when I wrote this patch we had an issue with a very big SB database, around
1,5 gigabytes. There were no controllers or northds running, so the
database server was without any load at all. Although OVSDB was idling,
even a single northd process could not fully connect to the database due to
its size, since it could not fetch and process the data in 5 seconds.

Since then many optimizations were made, and the database size with the
same topology reduced to approximately twenty megabytes, so today I
wouldn't be able to reproduce the problem.

However, I am quite sure that it would still cause troubles with a huge
scale, when SB grows to hundreds of megabytes. With the default timeout of
5 seconds, which is implemented in the same thread that also fetches and
processes data, we make an artificial database size limit, which is not so
obvoius to troubleshoot.

Regards,
Renat.

Han Zhou  schrieb am Fr., 17. Sept. 2021, 23:34:

>
>
> On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang  wrote:
> >
> > From: zhen wang 
> >
> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> > Above commit introduced a bug when muptiple ovn-northd instances work in
> HA
> > mode. If SB leader and active ovn-northd instance got killed by system
> power
> > outage, standby ovn-northd instance would never detect the failure.
> >
>
> Thanks Zhen! I added the Renat and Numan who worked on the reverted commit
> to CC, so that they can comment if this is ok.
>
> For the commit message, I think it may be decoupled from the HA scenario
> that is supposed to be fixed by the other patch in this series. The issue
> this patch fixes is that before the initial NB downloading is complete the
> northd will not send probe, so if the DB server is down (ungracefully)
> before the northd reads the NB_Global options, the northd would never
> probe, thus never reconnect to the new leader. (it is related to RAFT, but
> whether it is multiple northds is irrelevant)
>
> As to the original commit that is reverted by this one:
>
> northd: Don't poll ovsdb before the connection is fully established
>
> Set initial SB and NB DBs probe interval to 0 to avoid connection
> flapping.
>
> Before configured in northd_probe_interval value is actually applied
> to southbound and northbound database connections, both connections
> must be fully established, otherwise ovnnb_db_run() will return
> without retrieving configuration data from northbound DB. In cases
> when southbound database is big enough, default interval of 5 seconds
> will kill and retry the connection before it is fully established, no
> matter what is set in northd_probe_interval. Client reconnect will
> cause even more load to ovsdb-server and cause cascade effect, so
> northd can never stabilise. We have more than 2000 ports in our lab,
> and northd could not start before this patch, holding at 100% CPU
> utilisation both itself and ovsdb-server.
>
> After connections are established, any value in northd_probe_interval,
> or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly.
>
> I am not sure how would the commit help. There are at most 3 - 5 northds
> (in practice), and suppose there are tens or hundreds of ovn-controllers
> that makes SB busy, it is just 3 - 5 more clients retrying reconnect SB for
> several times, and if NB is not that busy (most likely), these northd
> clients should get the proper probe settings applied soon without causing
> more issues at all. So I don't think the default probe 5 sec would cause
> cascade effect for the initial period. @Renat @Numan please correct me if I
> am wrong.
>
> Thanks,
> Han
>
> > Signed-off-by: zhen wang 
> > ---
> >  northd/northd.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 688a6e4ef..b7e64470f 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -74,8 +74,8 @@ static bool use_ct_inv_match = true;
> >
> >  /* Default probe interval for NB and SB DB connections. */
> >  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> > -static int northd_probe_interval_nb = 0;
> > -static int northd_probe_interval_sb = 0;
> > +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> > +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
> >  #define MAX_OVN_TAGS 4096
> >
> >  /* Pipeline stages. */
> > --
> > 2.20.1
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2 1/2] Prepare for 21.09.0.

[Mark Michelson]

I pushed patch 1 to master, then created the branch-21.09 branch, and 
pushed patch 2 to master.


On 9/17/21 3:42 PM, Numan Siddique wrote:

On Fri, Sep 17, 2021 at 2:44 PM Mark Michelson  wrote:


Signed-off-by: Mark Michelson 


For both the patches:

Acked-by: Numan Siddique 

Numan


---
  NEWS | 4 ++--
  configure.ac | 2 +-
  debian/changelog | 4 ++--
  3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 6e81f7bee..784240fbc 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-Post-v21.06.0
--
+OVN v21.09.0 - xx xxx 
+--
- Added Control Plane Protection support (control plane traffic metering).
- Added path MTU discovery for ingress traffic originated outside of the
  cluster.
diff --git a/configure.ac b/configure.ac
index 4728fa0a6..ca282634c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
  # limitations under the License.

  AC_PREREQ(2.63)
-AC_INIT(ovn, 21.06.90, b...@openvswitch.org)
+AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
  AC_CONFIG_MACRO_DIR([m4])
  AC_CONFIG_AUX_DIR([build-aux])
  AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index 81aaed307..182333498 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-ovn (21.06.90-1) unstable; urgency=low
+ovn (21.09.0-1) unstable; urgency=low

 * New upstream version

- -- OVN team   Fri, 18 Jun 2021 13:21:08 -0400
+ -- OVN team   Fri, 17 Sep 2021 12:00:00 -0400

  ovn (21.06.0-1) unstable; urgency=low

--
2.31.1

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





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

Re: [ovs-dev] [OVN Patch v8 1/3] northd: Disable parallel processing for logical_dp_groups

[Mark Michelson]

Based on the successful GHA run I had in my fork, I fixed the submodule 
downgrade (and fixed a couple of grammar errors in comments). I've 
pushed this to master.


On 9/17/21 3:49 PM, Mark Michelson wrote:

On 9/17/21 1:38 PM, Ilya Maximets wrote:

On 9/15/21 14:43, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Work on improving processing with dp_groups enabled has
discovered that the locking mechanism presently in use
is not reliable. Disabling parallel processing if dp_groups
are enabled until the root cause is determined and fixed.

Signed-off-by: Anton Ivanov 
---
  northd/ovn-northd.c | 2 +-
  ovs | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index baaddb73e..3113fafc7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap 
*datapaths, struct hmap *ports,

  }
  }
-    if (use_parallel_build) {
+    if (use_parallel_build && (!use_logical_dp_groups)) {
  struct hmap *lflow_segs;
  struct lswitch_flow_build_info *lsiv;
  int index;
diff --git a/ovs b/ovs
index 748010ff3..50e5523b9 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9
+Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330



This change breaks the build due to submodule downgrade.

Best regards, Ilya Maximets.



Thanks for the note, Ilya. I did a manual rebase (which was non-trivial 
due to the northd split) and pushed to my fork. Seems to be OK with the 
submodule downgrade fixed: 
https://github.com/putnopvut/ovn/runs/3635759940?check_suite_focus=true


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

Re: [ovs-dev] [PATCH ovn 2/2] Update the probe interval in main loop.

[Han Zhou]

On Thu, Sep 16, 2021 at 8:06 PM Zhen Wang  wrote:
>
> From: zhen wang 
>
> When ovn-northd work in HA mode, ovn-northd will not update the probe
> interval in standby mode. This patch address the problem by updating
> the value in main loop.
>

Thanks Zhen for the fix! Maybe the impact and steps to reproduce the
problem can be mentioned. It may be:
step1: power off the NB/SB leader
step2: kill the active northd (and the standbys would never take over)

Acked-by: Han Zhou 

> Signed-off-by: zhen wang 
> ---
>  northd/northd.c | 25 -
>  northd/ovn-northd.c | 29 +
>  2 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index b7e64470f..89b0e4921 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -72,10 +72,6 @@ static struct eth_addr svc_monitor_mac_ea;
>   * Otherwise, it will avoid using it.  The default is true. */
>  static bool use_ct_inv_match = true;
>
> -/* Default probe interval for NB and SB DB connections. */
> -#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> -static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> -static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
>  #define MAX_OVN_TAGS 4096
>
>  /* Pipeline stages. */
> @@ -14082,20 +14078,6 @@ build_meter_groups(struct northd_context *ctx,
>  }
>  }
>
> -static int
> -get_probe_interval(const char *db, const struct nbrec_nb_global *nb)
> -{
> -int default_interval = (db && !stream_or_pstream_needs_probes(db)
> -? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
> -int interval = smap_get_int(>options,
> -"northd_probe_interval",
default_interval);
> -
> -if (interval > 0 && interval < 1000) {
> -interval = 1000;
> -}
> -return interval;
> -}
> -
>  static void
>  ovnnb_db_run(struct northd_context *ctx,
>   struct ovsdb_idl_index *sbrec_chassis_by_name,
> @@ -14182,13 +14164,6 @@ ovnnb_db_run(struct northd_context *ctx,
>
>  smap_destroy();
>
> -/* Update the probe interval. */
> -northd_probe_interval_nb = get_probe_interval(ctx->ovnnb_db, nb);
> -northd_probe_interval_sb = get_probe_interval(ctx->ovnsb_db, nb);
> -
> -ovsdb_idl_set_probe_interval(ctx->ovnnb_idl,
northd_probe_interval_nb);
> -ovsdb_idl_set_probe_interval(ctx->ovnsb_idl,
northd_probe_interval_sb);
> -
>  use_parallel_build =
>  (smap_get_bool(>options, "use_parallel_build", false) &&
>   can_parallelize_hashes(false));
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 6d4c5defc..0a9fd8190 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -65,6 +65,10 @@ static const char *ssl_private_key_file;
>  static const char *ssl_certificate_file;
>  static const char *ssl_ca_cert_file;
>
> +/* Default probe interval for NB and SB DB connections. */
> +#define DEFAULT_PROBE_INTERVAL_MSEC 5000
> +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
>  static bool use_parallel_build = true;
>  static struct hashrow_locks lflow_locks;
>
> @@ -577,6 +581,20 @@ update_ssl_config(void)
>  }
>  }
>
> +static int
> +get_probe_interval(const char *db, const struct nbrec_nb_global *nb)
> +{
> +int default_interval = (db && !stream_or_pstream_needs_probes(db)
> +? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
> +int interval = smap_get_int(>options,
> +"northd_probe_interval",
default_interval);
> +
> +if (interval > 0 && interval < 1000) {
> +interval = 1000;
> +}
> +return interval;
> +}
> +
>  int
>  main(int argc, char *argv[])
>  {
> @@ -911,6 +929,12 @@ main(int argc, char *argv[])
>
>  while (!exiting) {
>  update_ssl_config();
> +const struct nbrec_nb_global *nb =
nbrec_nb_global_first(ovnnb_idl_loop.idl);
> +/* Update the probe interval. */
> +if (nb) {
> +northd_probe_interval_nb = get_probe_interval(ovnnb_db, nb);
> +northd_probe_interval_sb = get_probe_interval(ovnsb_db, nb);
> +}
>  memory_run();
>  if (memory_should_report()) {
>  struct simap usage = SIMAP_INITIALIZER();
> @@ -1000,6 +1024,11 @@ main(int argc, char *argv[])
>  poll_immediate_wake();
>  }
>
> +ovsdb_idl_set_probe_interval(ovnnb_idl_loop.idl,
> + northd_probe_interval_nb);
> +ovsdb_idl_set_probe_interval(ovnsb_idl_loop.idl,
> + northd_probe_interval_sb);
> +
>  if (reset_ovnsb_idl_min_index) {
>  VLOG_INFO("Resetting southbound database cluster state");
>  ovsdb_idl_reset_min_index(ovnsb_idl_loop.idl);
> --
> 2.20.1
>
___
dev mailing list
d...@openvswitch.org

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Han Zhou]

On Fri, Sep 17, 2021 at 1:31 PM Numan Siddique  wrote:
>
> On Thu, Sep 16, 2021 at 11:05 PM Zhen Wang via dev
>  wrote:
> >
> > From: zhen wang 
> >
> > This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> > Above commit introduced a bug when muptiple ovn-northd instances work
in HA
> > mode. If SB leader and active ovn-northd instance got killed by system
power
> > outage, standby ovn-northd instance would never detect the failure.
> >
> > Signed-off-by: zhen wang 
>
> I'm not sure if we can revert this commit.   How would you propose to fix
the
> issue mentioned in the commit message of the commit 1e59feea933610 ?
>
> I think you should configure a desired probe interval value in
NB_Global.options
> so that the stand-by northd instance can detect the failure.  With
> this configuration and
> with your patch 2,  the issue reported by you should be addressed.
>
> Another option is to reset the probe interval from 0 to the default
> value - DEFAULT_PROBE_INTERVAL_MSEC
> once northd has successfully connected to the Northbound db server if
> CMS has not configured the probe
> interval in NB_Global.options.
>
> Thanks
> Numan
>

Hi Numan, sorry that I cross replied. Please see my other reply for the
points you mentioned. -Han
>
>
> > ---
> >  northd/northd.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 688a6e4ef..b7e64470f 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -74,8 +74,8 @@ static bool use_ct_inv_match = true;
> >
> >  /* Default probe interval for NB and SB DB connections. */
> >  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> > -static int northd_probe_interval_nb = 0;
> > -static int northd_probe_interval_sb = 0;
> > +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> > +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
> >  #define MAX_OVN_TAGS 4096
> >
> >  /* Pipeline stages. */
> > --
> > 2.20.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] binding: Store timestamp at which ovn-installed was set.

[Numan Siddique]

On Wed, Sep 15, 2021 at 10:22 AM Dumitru Ceara  wrote:
>
> CMSs (e.g., ovn-kubernetes) already use the ovn-installed
> external_id set by ovn-controller in OVS.Interface.external_ids to
> determine that the networking for a VIF is completely plugged.
>
> ovn-controller now also stores the timestamp (in milliseconds since the
> epoch) at which an interface was marked as "installed", as the
> ovn-installed-ts external id in the OVS DB.
>
> This commit also adds the relevant documentation for
> ovn-installed/ovn-installed-ts.
>
> Signed-off-by: Dumitru Ceara 

Thanks.  Applied.

Numan

> ---
>  controller/binding.c|  7 ++-
>  controller/binding.h|  3 ++-
>  controller/if-status.c  |  6 +-
>  controller/ovn-controller.8.xml | 15 +++
>  4 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 34935bb9c..c037b2352 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -44,6 +44,7 @@ VLOG_DEFINE_THIS_MODULE(binding);
>   * flows have been installed.
>   */
>  #define OVN_INSTALLED_EXT_ID "ovn-installed"
> +#define OVN_INSTALLED_TS_EXT_ID "ovn-installed-ts"
>
>  #define OVN_QOS_TYPE "linux-htb"
>
> @@ -712,7 +713,8 @@ local_binding_is_down(struct shash *local_bindings, const 
> char *pb_name)
>
>  void
>  local_binding_set_up(struct shash *local_bindings, const char *pb_name,
> - bool sb_readonly, bool ovs_readonly)
> + const char *ts_now_str, bool sb_readonly,
> + bool ovs_readonly)
>  {
>  struct local_binding *lbinding =
>  local_binding_find(local_bindings, pb_name);
> @@ -725,6 +727,9 @@ local_binding_set_up(struct shash *local_bindings, const 
> char *pb_name,
>  ovsrec_interface_update_external_ids_setkey(lbinding->iface,
>  OVN_INSTALLED_EXT_ID,
>  "true");
> +ovsrec_interface_update_external_ids_setkey(lbinding->iface,
> +OVN_INSTALLED_TS_EXT_ID,
> +ts_now_str);
>  }
>
>  if (!sb_readonly && lbinding && b_lport && b_lport->pb->n_up
> diff --git a/controller/binding.h b/controller/binding.h
> index f1abc4b9c..70cc37c78 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -120,7 +120,8 @@ ofp_port_t local_binding_get_lport_ofport(const struct 
> shash *local_bindings,
>  bool local_binding_is_up(struct shash *local_bindings, const char *pb_name);
>  bool local_binding_is_down(struct shash *local_bindings, const char 
> *pb_name);
>  void local_binding_set_up(struct shash *local_bindings, const char *pb_name,
> -  bool sb_readonly, bool ovs_readonly);
> +  const char *ts_now_str, bool sb_readonly,
> +  bool ovs_readonly);
>  void local_binding_set_down(struct shash *local_bindings, const char 
> *pb_name,
>  bool sb_readonly, bool ovs_readonly);
>
> diff --git a/controller/if-status.c b/controller/if-status.c
> index 08fb50b87..b5a4025fc 100644
> --- a/controller/if-status.c
> +++ b/controller/if-status.c
> @@ -21,6 +21,7 @@
>
>  #include "lib/hmapx.h"
>  #include "lib/util.h"
> +#include "timeval.h"
>  #include "openvswitch/vlog.h"
>
>  VLOG_DEFINE_THIS_MODULE(if_status);
> @@ -398,11 +399,14 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
>   * their flows installed but are not yet marked "up" in the binding
>   * module.
>   */
> +char *ts_now_str = xasprintf("%lld", time_wall_msec());
>  HMAPX_FOR_EACH (node, >ifaces_per_state[OIF_MARK_UP]) {
>  struct ovs_iface *iface = node->data;
>
> -local_binding_set_up(bindings, iface->id, sb_readonly, ovs_readonly);
> +local_binding_set_up(bindings, iface->id, ts_now_str,
> + sb_readonly, ovs_readonly);
>  }
> +free(ts_now_str);
>
>  /* Notify the binding module to set "down" all bindings that have been
>   * released but are not yet marked as "down" in the binding module.
> diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
> index 5a649ee65..8c180f576 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -471,6 +471,21 @@
>flows have been successfully installed in OVS.
>  
>
> +
> +  
> +external_ids:ovn-installed and
> +external_ids:ovn-installed-ts in the
> +Interface table
> +  
> +
> +  
> +
> +  This key is set after all openflow operations corresponding to the
> +  OVS interface have been processed by ovs-vswitchd.  At the same 
> time
> +  a timestamp, in milliseconds since the epoch, is stored in
> +  external_ids:ovn-installed-ts.
> +
> +   

Re: [ovs-dev] [PATCH ovn] ovn.at: Skip tests that need tcpdump when it's not installed.

[Numan Siddique]

On Wed, Sep 15, 2021 at 10:26 AM Dumitru Ceara  wrote:
>
> Signed-off-by: Dumitru Ceara 

Thanks.  Applied.

Numan

> ---
>  tests/ovn.at | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 30625ec37..dc6569008 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -12180,6 +12180,7 @@ AT_CLEANUP
>
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([localport suppress gARP])
> +AT_SKIP_IF([test $HAVE_TCPDUMP = no])
>  ovn_start
>
>  send_garp() {
> @@ -12252,6 +12253,7 @@ AT_CLEANUP
>
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([localport doesn't suppress ARP directed to external port])
> +AT_SKIP_IF([test $HAVE_TCPDUMP = no])
>
>  ovn_start
>  net_add n1
> --
> 2.27.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 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Han Zhou]

On Thu, Sep 16, 2021 at 8:05 PM Zhen Wang  wrote:
>
> From: zhen wang 
>
> This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> Above commit introduced a bug when muptiple ovn-northd instances work in
HA
> mode. If SB leader and active ovn-northd instance got killed by system
power
> outage, standby ovn-northd instance would never detect the failure.
>

Thanks Zhen! I added the Renat and Numan who worked on the reverted commit
to CC, so that they can comment if this is ok.

For the commit message, I think it may be decoupled from the HA scenario
that is supposed to be fixed by the other patch in this series. The issue
this patch fixes is that before the initial NB downloading is complete the
northd will not send probe, so if the DB server is down (ungracefully)
before the northd reads the NB_Global options, the northd would never
probe, thus never reconnect to the new leader. (it is related to RAFT, but
whether it is multiple northds is irrelevant)

As to the original commit that is reverted by this one:

northd: Don't poll ovsdb before the connection is fully established

Set initial SB and NB DBs probe interval to 0 to avoid connection
flapping.

Before configured in northd_probe_interval value is actually applied
to southbound and northbound database connections, both connections
must be fully established, otherwise ovnnb_db_run() will return
without retrieving configuration data from northbound DB. In cases
when southbound database is big enough, default interval of 5 seconds
will kill and retry the connection before it is fully established, no
matter what is set in northd_probe_interval. Client reconnect will
cause even more load to ovsdb-server and cause cascade effect, so
northd can never stabilise. We have more than 2000 ports in our lab,
and northd could not start before this patch, holding at 100% CPU
utilisation both itself and ovsdb-server.

After connections are established, any value in northd_probe_interval,
or default DEFAULT_PROBE_INTERVAL_MSEC is applied correctly.

I am not sure how would the commit help. There are at most 3 - 5 northds
(in practice), and suppose there are tens or hundreds of ovn-controllers
that makes SB busy, it is just 3 - 5 more clients retrying reconnect SB for
several times, and if NB is not that busy (most likely), these northd
clients should get the proper probe settings applied soon without causing
more issues at all. So I don't think the default probe 5 sec would cause
cascade effect for the initial period. @Renat @Numan please correct me if I
am wrong.

Thanks,
Han

> Signed-off-by: zhen wang 
> ---
>  northd/northd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 688a6e4ef..b7e64470f 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -74,8 +74,8 @@ static bool use_ct_inv_match = true;
>
>  /* Default probe interval for NB and SB DB connections. */
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> -static int northd_probe_interval_nb = 0;
> -static int northd_probe_interval_sb = 0;
> +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
>  #define MAX_OVN_TAGS 4096
>
>  /* Pipeline stages. */
> --
> 2.20.1
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 1/2] Revert "northd: Don't poll ovsdb before the connection is fully established"

[Numan Siddique]

On Thu, Sep 16, 2021 at 11:05 PM Zhen Wang via dev
 wrote:
>
> From: zhen wang 
>
> This reverts commit 1e59feea933610b28fd4442243162ce35595cfee.
> Above commit introduced a bug when muptiple ovn-northd instances work in HA
> mode. If SB leader and active ovn-northd instance got killed by system power
> outage, standby ovn-northd instance would never detect the failure.
>
> Signed-off-by: zhen wang 

I'm not sure if we can revert this commit.   How would you propose to fix the
issue mentioned in the commit message of the commit 1e59feea933610 ?

I think you should configure a desired probe interval value in NB_Global.options
so that the stand-by northd instance can detect the failure.  With
this configuration and
with your patch 2,  the issue reported by you should be addressed.

Another option is to reset the probe interval from 0 to the default
value - DEFAULT_PROBE_INTERVAL_MSEC
once northd has successfully connected to the Northbound db server if
CMS has not configured the probe
interval in NB_Global.options.

Thanks
Numan



> ---
>  northd/northd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 688a6e4ef..b7e64470f 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -74,8 +74,8 @@ static bool use_ct_inv_match = true;
>
>  /* Default probe interval for NB and SB DB connections. */
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> -static int northd_probe_interval_nb = 0;
> -static int northd_probe_interval_sb = 0;
> +static int northd_probe_interval_nb = DEFAULT_PROBE_INTERVAL_MSEC;
> +static int northd_probe_interval_sb = DEFAULT_PROBE_INTERVAL_MSEC;
>  #define MAX_OVN_TAGS 4096
>
>  /* Pipeline stages. */
> --
> 2.20.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] rhel, utils: don't affect traffic on controller upgrade

[Numan Siddique]

On Fri, Sep 17, 2021 at 9:28 AM Vladislav Odintsov  wrote:
>
> Currently upgrade of ovn-host rpm package affects active
> traffic. This is because systemctl try-restart
> ovn-controller is invoked during rpm package upgrade.
> It calls ovn-ctl stop_controller and then start_controller.
>
> Adding ovn-ctl stop_controller --restart to %postun
> upgrade case right before systemctl try-restart. Also,
> upgrade ovn-ctl script to support --restart argument in it.
>
> Ideally this should be done by systemd when restart is
> called, but it's impossible to pass restart command to
> systemd.
>
> Signed-off-by: Vladislav Odintsov 

Thanks for the patch.  I applied this patch.

I did try to find a better way to fix it.  Seems to me this is the only way.

Numan

> ---
>  rhel/ovn-fedora.spec.in |  8 
>  utilities/ovn-ctl   | 10 --
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
> index 6716dd0d2..5fe0f990f 100644
> --- a/rhel/ovn-fedora.spec.in
> +++ b/rhel/ovn-fedora.spec.in
> @@ -400,6 +400,14 @@ fi
>  %endif
>
>  %postun host
> +if [ "$1" -ge "1" ] ; then
> +# Package upgrade, not uninstall
> +# We perform lightweight stop here not to affect active traffic during
> +# ovn-controller upgrade.
> +# Ideally this would be held by systemd, but it's impossible
> +# to pass custom restart command to systemd service.
> +%{_datadir}/ovn/scripts/ovn-ctl stop_controller --restart
> +fi
>  %if 0%{?systemd_postun_with_restart:1}
>  %systemd_postun_with_restart ovn-controller.service
>  %else
> diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> index ff61f21d0..b30eb209d 100755
> --- a/utilities/ovn-ctl
> +++ b/utilities/ovn-ctl
> @@ -584,7 +584,11 @@ stop_ic () {
>  }
>
>  stop_controller () {
> -OVS_RUNDIR=${OVS_RUNDIR} stop_ovn_daemon ovn-controller "" "" "$@"
> +set "ovn-controller" "" ""
> +if test X"$RESTART" = Xyes; then
> +set "$@" "--restart"
> +fi
> +OVS_RUNDIR=${OVS_RUNDIR} stop_ovn_daemon "$@"
>  }
>
>  stop_controller_vtep () {
> @@ -606,7 +610,8 @@ restart_ic () {
>  }
>
>  restart_controller () {
> -stop_controller --restart
> +RESTART=yes
> +stop_controller
>  start_controller
>  }
>
> @@ -651,6 +656,7 @@ restart_ic_sb_ovsdb () {
>
>  set_defaults () {
>  OVN_MANAGE_OVSDB=yes
> +RESTART=no
>
>  OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
>  OVN_RUNDIR=${OVN_RUNDIR:-${ovn_rundir}}
> --
> 2.30.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] [OVN Patch v8 1/3] northd: Disable parallel processing for logical_dp_groups

[Mark Michelson]

On 9/17/21 1:38 PM, Ilya Maximets wrote:

On 9/15/21 14:43, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Work on improving processing with dp_groups enabled has
discovered that the locking mechanism presently in use
is not reliable. Disabling parallel processing if dp_groups
are enabled until the root cause is determined and fixed.

Signed-off-by: Anton Ivanov 
---
  northd/ovn-northd.c | 2 +-
  ovs | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index baaddb73e..3113fafc7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, 
struct hmap *ports,
  }
  }
  
-if (use_parallel_build) {

+if (use_parallel_build && (!use_logical_dp_groups)) {
  struct hmap *lflow_segs;
  struct lswitch_flow_build_info *lsiv;
  int index;
diff --git a/ovs b/ovs
index 748010ff3..50e5523b9 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9
+Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330



This change breaks the build due to submodule downgrade.

Best regards, Ilya Maximets.



Thanks for the note, Ilya. I did a manual rebase (which was non-trivial 
due to the northd split) and pushed to my fork. Seems to be OK with the 
submodule downgrade fixed: 
https://github.com/putnopvut/ovn/runs/3635759940?check_suite_focus=true


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

Re: [ovs-dev] [PATCH ovn v2 1/2] Prepare for 21.09.0.

[Numan Siddique]

On Fri, Sep 17, 2021 at 2:44 PM Mark Michelson  wrote:
>
> Signed-off-by: Mark Michelson 

For both the patches:

Acked-by: Numan Siddique 

Numan

> ---
>  NEWS | 4 ++--
>  configure.ac | 2 +-
>  debian/changelog | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 6e81f7bee..784240fbc 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,5 @@
> -Post-v21.06.0
> --
> +OVN v21.09.0 - xx xxx 
> +--
>- Added Control Plane Protection support (control plane traffic metering).
>- Added path MTU discovery for ingress traffic originated outside of the
>  cluster.
> diff --git a/configure.ac b/configure.ac
> index 4728fa0a6..ca282634c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>
>  AC_PREREQ(2.63)
> -AC_INIT(ovn, 21.06.90, b...@openvswitch.org)
> +AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> diff --git a/debian/changelog b/debian/changelog
> index 81aaed307..182333498 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,8 +1,8 @@
> -ovn (21.06.90-1) unstable; urgency=low
> +ovn (21.09.0-1) unstable; urgency=low
>
> * New upstream version
>
> - -- OVN team   Fri, 18 Jun 2021 13:21:08 -0400
> + -- OVN team   Fri, 17 Sep 2021 12:00:00 -0400
>
>  ovn (21.06.0-1) unstable; urgency=low
>
> --
> 2.31.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] northd: support HW VTEP with stateful datapath

[Numan Siddique]

On Fri, Sep 17, 2021 at 11:01 AM Vladislav Odintsov  wrote:
>
> A packet going from HW VTEP device to VIF port when arrives to
> hypervisor chassis should go through LS ingress pipeline to l2_lkp
> stage without any match. In l2_lkp stage an output port is
> determined and then packet passed to LS egress pipeline for futher
> processing and to VIF port delivery.
>
> Prior to this commit a packet, which was received from HW VTEP
> device was dropped in an LS ingress datapath, where stateful services
> were defined (ACLs, LBs).
>
> To fix this issue we add a special flag-bit which can be used in LS
> pipelines, to check whether the packet came from HW VTEP devices.
> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
> to skip such packets.
>
> Signed-off-by: Vladislav Odintsov 

Hi Vladislav,

The documentation needs to be updated for the newly added logical flows
and also for the updated flow in "ls_in_port_sec_l2" stage.

I didn't review your p1 earlier and hence missed it.

Regarding the patch,  I don't have much idea on how the ramp/vtep stuff works.
My question is why the traffic received from HW VTEP devices should be skipped
from conntrack ?  ACLs and policies don't apply to vtep logical ports ?

Thanks
Numan

> ---
> v1 -> v2:
>- Patch rebased on upstream changes.
>
> Please note: I've got no experience in DDLog and have no ability to 
> extensively
>  test these changes.
>  Just local ./configure --with-ddlog=...; make; make check was run
>  It seems, that only irrelevant to these changes tests were 
> failed.
> ---
>  northd/northd.c  | 14 ++
>  northd/ovn_northd.dl | 33 +++--
>  tests/ovn-northd.at  |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 688a6e4ef..1b84874a7 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -196,6 +196,7 @@ enum ovn_stage {
>  #define REGBIT_LKUP_FDB   "reg0[11]"
>  #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
>  #define REGBIT_ACL_LABEL  "reg0[13]"
> +#define REGBIT_FROM_RAMP  "reg0[14]"
>
>  #define REG_ORIG_DIP_IPV4 "reg1"
>  #define REG_ORIG_DIP_IPV6 "xxreg1"
> @@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
>  if (queue_id) {
>  ds_put_format(actions, "set_queue(%s); ", queue_id);
>  }
> +
> +if (!strcmp(op->nbsp->type, "vtep")) {
> +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
> +}
> +
>  ds_put_cstr(actions, "next;");
>  ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
> S_SWITCH_IN_PORT_SEC_L2,
>50, ds_cstr(match), ds_cstr(actions),
> @@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
> *port_groups,
>"nd || nd_rs || nd_ra || mldv1 || mldv2 || "
>"(udp && udp.src == 546 && udp.dst == 547)", "next;");
>
> +/* Do not send coming from RAMP switch packets to conntrack. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> +  REGBIT_FROM_RAMP" == 1", "next;");
> +
>  /* Ingress and Egress Pre-ACL Table (Priority 100).
>   *
>   * Regardless of whether the ACL is "from-lport" or "to-lport",
> @@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
> *lflows,
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
>"eth.src == $svc_monitor_mac", "next;");
>
> +/* Do not send coming from RAMP switch packets to conntrack. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> +  REGBIT_FROM_RAMP" == 1", "next;");
> +
>  /* Allow all packets to go to next tables by default. */
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 669728497..0202af5dc 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1631,6 +1631,7 @@ function rEGBIT_ACL_HINT_BLOCK()   : istring = 
> i"reg0[10]"
>  function rEGBIT_LKUP_FDB() : istring = i"reg0[11]"
>  function rEGBIT_HAIRPIN_REPLY(): istring = i"reg0[12]"
>  function rEGBIT_ACL_LABEL(): istring = i"reg0[13]"
> +function rEGBIT_FROM_RAMP(): istring = i"reg0[14]"
>
>  function rEG_ORIG_DIP_IPV4()   : istring = i"reg1"
>  function rEG_ORIG_DIP_IPV6()   : istring = i"xxreg1"
> @@ -2070,6 +2071,16 @@ for ((._uuid = ls_uuid, .has_stateful_acl = 
> true)) {
>   .io_port  = None,
>   .controller_meter = None);
>
> +/* Do not send coming from RAMP switch packets to conntrack. */
> +Flow(.logical_datapath = ls_uuid,
> + .stage= s_SWITCH_IN_PRE_ACL(),
> + .priority = 110,
> + .__match  = i"${rEGBIT_FROM_RAMP()} == 1",
> + .actions  

Re: [ovs-dev] [PATCH v6 ovn] controller: add datapath meter capability check

[Mark Michelson]

I merged this to master. Thanks Lorenzo and Mark!

On 9/17/21 12:25 PM, Lorenzo Bianconi wrote:

Dump datapath meter capabilities before configuring meters in
ovn-controller

Signed-off-by: Lorenzo Bianconi 
---
Changes since v5:
- send new command only on reconnection
Changes since v4:
- move rconn setup in ovs_feature_support_update and rename it in
   ovs_feature_support_run
- drop ovs_feature_support_init()
- rename ovs_feature_support_deinit in ovs_feature_support_destroy
Changes since v3:
- add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
   loop
- cosmetics
Changes since v2:
- move meter capability logic in lib/features.c
Changes since v1:
- move rconn in ovn-controller to avoid concurrency issues
---
  controller/ovn-controller.c |   7 +--
  include/ovn/features.h  |   6 ++-
  lib/actions.c   |   3 ++
  lib/automake.mk |   1 +
  lib/features.c  | 102 +++-
  lib/test-ovn-features.c |   6 +--
  tests/ovn.at|   4 +-
  7 files changed, 119 insertions(+), 10 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d98ebbbfa..aa7941eeb 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3574,9 +3574,9 @@ main(int argc, char *argv[])
   * 'br_int_dp' is valid only if an OVS transaction is possible.
   */
  if (ovs_idl_txn
-&& ovs_feature_support_update(br_int_dp
-  ? _int_dp->capabilities
-  : NULL)) {
+&& ovs_feature_support_run(br_int_dp ?
+   _int_dp->capabilities : NULL,
+   br_int ? br_int->name : NULL)) {
  VLOG_INFO("OVS feature set changed, force recompute.");
  engine_set_force_recompute(true);
  }
@@ -3903,6 +3903,7 @@ loop_done:
  ovsdb_idl_loop_destroy(_idl_loop);
  ovsdb_idl_loop_destroy(_idl_loop);
  
+ovs_feature_support_destroy();

  free(ovs_remote);
  service_stop();
  
diff --git a/include/ovn/features.h b/include/ovn/features.h

index c35d59b14..d12a8eb0d 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -28,13 +28,17 @@
   */
  enum ovs_feature_support_bits {
  OVS_CT_ZERO_SNAT_SUPPORT_BIT,
+OVS_DP_METER_SUPPORT_BIT,
  };
  
  enum ovs_feature_value {

  OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
+OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT),
  };
  
+void ovs_feature_support_destroy(void);

  bool ovs_feature_is_supported(enum ovs_feature_value feature);
-bool ovs_feature_support_update(const struct smap *ovs_capabilities);
+bool ovs_feature_support_run(const struct smap *ovs_capabilities,
+ const char *br_name);
  
  #endif

diff --git a/lib/actions.c b/lib/actions.c
index c572e88ae..7cf6be308 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool 
pause,
  oc->max_len = UINT16_MAX;
  oc->reason = OFPR_ACTION;
  oc->pause = pause;
+if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
+meter_id = NX_CTLR_NO_METER;
+}
  oc->meter_id = meter_id;
  
  struct action_header ah = { .opcode = htonl(opcode) };

diff --git a/lib/automake.mk b/lib/automake.mk
index ddfe33948..9f9f447d5 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
  lib_libovn_la_LDFLAGS = \
  $(OVS_LTINFO) \
  -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
+$(OVS_LIBDIR)/libopenvswitch.la \
  $(AM_LDFLAGS)
  lib_libovn_la_SOURCES = \
lib/acl-log.c \
diff --git a/lib/features.c b/lib/features.c
index fddf4c450..f15ec42bb 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -18,7 +18,14 @@
  #include 
  
  #include "lib/util.h"

+#include "lib/dirs.h"
+#include "socket-util.h"
+#include "lib/vswitch-idl.h"
  #include "openvswitch/vlog.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/rconn.h"
+#include "openvswitch/ofp-msgs.h"
+#include "openvswitch/ofp-meter.h"
  #include "ovn/features.h"
  
  VLOG_DEFINE_THIS_MODULE(features);

@@ -40,11 +47,16 @@ static uint32_t supported_ovs_features;
  
  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
  
+/* ovs-vswitchd connection. */

+static struct rconn *swconn;
+static uint32_t conn_seq_no;
+
  static bool
  ovs_feature_is_valid(enum ovs_feature_value feature)
  {
  switch (feature) {
  case OVS_CT_ZERO_SNAT_SUPPORT:
+case OVS_DP_METER_SUPPORT:
  return true;
  default:
  return false;
@@ -58,9 +70,93 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
  return supported_ovs_features & feature;
  }
  
+static void

+ovs_feature_rconn_setup(const char 

Re: [ovs-dev] [PATCH ovn v4 8/9] binding: Consider plugging of ports on CMS request.

[Numan Siddique]

On Fri, Sep 17, 2021 at 12:41 PM Frode Nordahl
 wrote:
>
> On Fri, Sep 17, 2021 at 1:16 AM Numan Siddique  wrote:
> >
> > On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl
> >  wrote:
> > >
> > > When OVN is linked with an appropriate plugging implementation,
> > > CMS can request OVN to plug individual lports into the local
> > > Open vSwitch instance.
> > >
> > > The port and instance record will be maintained during the lifetime
> > > of the lport and it will be removed on release of lport.
> > >
> > > Signed-off-by: Frode Nordahl 
> >
> > Hi Frode,
> >
> > I've a few comments with the approach taken in this patch.
> > This patch calls the plug_port APIs from binding.c.  This complicates
> > the already complicated binding.c code and I think plug_port() related stuff
> > can be handled separately.  I don't see a need for binding.c to be aware of
> > plugging feature.  For binding.c to claim a port, the ovs interface should
> > have external_ids:iface-id set to the port binding lport_name.
> >
> > Below is how I would see this can be done:
> >
> > 1.  Add a new file called - controller/plug.c  which will handle port 
> > binding
> > changes - plug_handle_port_binding_changes (and possibly ovs
> > port/ovs interface changes).
> >
> > 2.  The function plug_handle_port_binding_changes() will iterate through the
> >  tracked port bindings and if the port binding has the required options 
> > set
> > (i.e requested-chassis and plug-type) it will call plug_port APIs and 
> > create
> > or delete OVS ports.  This function can  access the lbinding data
> > stored in the runtime data.
> >
> > 3.   For recompute scenarios,  there can be a function - plug_run() which 
> > will
> >  iterate through all the port bindings and create/delete ovs ports.
> >
> > 4.  When the OVS ports are created / deleted in (2),  in the next run,
> > the binding module
> >  will claim or release the port binding.  binding.c module
> > wouldn't need to care
> >  if the port binding / ovs port is created by the plug module or
> > by some other
> > mechanism.
> >
> > 5.  The functions plug_handle_port_binding_changes() can be either called by
> >  runtime_data_sb_port_binding_handler() (i.e runtime engine node)
> >  or another engine node for plug handling can be created which will 
> > handle
> >  port binding  changes (and possibly ovs port/interface changes).
> >  Similarly for full recompute,  either runtime_data_run() can call
> > plug_run()
> >  or a new engine run function can call it.  Having a separate engine 
> > node
> >  seems better but then it needs to have runtime data as input
> >  to access the lbinding information.
> >
> > 6.  If you think plug.c should handle ovs port/interface changes,  then you
> >  can add handlers for it too.
> >
> >
> > What do you think ? Would this proposal work ?   Let me know if
> > something is not clear
> > and I can try to explain better.
>
> Numan, thank you so much for the review and for providing such a
> detailed and complete counter proposal, this is most appreciated.
>

> I agree with you completely and have started working on such an approach.

Great.  Thanks.
>
> This does mean I will most likely miss the 21.09 release with this
> series, which is unfortunate for me, but that's ok. Better to get it
> into a great shape for the next one than taking risks or cutting
> corners for this one.

If the patches look good before the official release,  then I've no objection
to backporting to the newly created branch.

Thanks
Numan

>
> Cheers!
>
> --
> Frode Nordahl
>
> > Thanks
> > Numan
> >
> > > ---
> > >  controller/binding.c| 247 ++--
> > >  tests/ovn-controller.at |  31 +
> > >  tests/ovn-macros.at |   2 +-
> > >  3 files changed, 272 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/controller/binding.c b/controller/binding.c
> > > index 938e1d81d..ae67ee3fc 100644
> > > --- a/controller/binding.c
> > > +++ b/controller/binding.c
> > > @@ -35,7 +35,9 @@
> > >  #include "local_data.h"
> > >  #include "lport.h"
> > >  #include "ovn-controller.h"
> > > +#include "ovsport.h"
> > >  #include "patch.h"
> > > +#include "plug.h"
> > >
> > >  VLOG_DEFINE_THIS_MODULE(binding);
> > >
> > > @@ -45,6 +47,8 @@ VLOG_DEFINE_THIS_MODULE(binding);
> > >   */
> > >  #define OVN_INSTALLED_EXT_ID "ovn-installed"
> > >
> > > +#define OVN_PLUGGED_EXT_ID "ovn-plugged"
> > > +
> > >  #define OVN_QOS_TYPE "linux-htb"
> > >
> > >  struct qos_queue {
> > > @@ -71,10 +75,13 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > >
> > >  ovsdb_idl_add_table(ovs_idl, _table_interface);
> > >  ovsdb_idl_track_add_column(ovs_idl, _interface_col_name);
> > > +ovsdb_idl_track_add_column(ovs_idl, _interface_col_type);
> > >  ovsdb_idl_track_add_column(ovs_idl, 
> > > _interface_col_external_ids);
> > >  ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd);
> > >  

[ovs-dev] [PATCH ovn v2 2/2] Prepare for post-21.09.0.

[Mark Michelson]

Signed-off-by: Mark Michelson 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 784240fbc..8a21c029e 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+Post v21.09.0
+-
+
 OVN v21.09.0 - xx xxx 
 --
   - Added Control Plane Protection support (control plane traffic metering).
diff --git a/configure.ac b/configure.ac
index ca282634c..d1b9b4d55 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
+AC_INIT(ovn, 21.09.90, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index 182333498..8ee2625c9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+ovn (21.09.90-1) unstable; urgency=low
+
+   * New upstream version
+
+ -- OVN team   Fri, 17 Sep 2021 12:00:00 -0400
+
 ovn (21.09.0-1) unstable; urgency=low
 
* New upstream version
-- 
2.31.1

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

[ovs-dev] [PATCH ovn v2 1/2] Prepare for 21.09.0.

[Mark Michelson]

Signed-off-by: Mark Michelson 
---
 NEWS | 4 ++--
 configure.ac | 2 +-
 debian/changelog | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 6e81f7bee..784240fbc 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-Post-v21.06.0
--
+OVN v21.09.0 - xx xxx 
+--
   - Added Control Plane Protection support (control plane traffic metering).
   - Added path MTU discovery for ingress traffic originated outside of the
 cluster.
diff --git a/configure.ac b/configure.ac
index 4728fa0a6..ca282634c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 21.06.90, b...@openvswitch.org)
+AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index 81aaed307..182333498 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-ovn (21.06.90-1) unstable; urgency=low
+ovn (21.09.0-1) unstable; urgency=low
 
* New upstream version
 
- -- OVN team   Fri, 18 Jun 2021 13:21:08 -0400
+ -- OVN team   Fri, 17 Sep 2021 12:00:00 -0400
 
 ovn (21.06.0-1) unstable; urgency=low
 
-- 
2.31.1

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

Re: [ovs-dev] [OVN Patch v8 1/3] northd: Disable parallel processing for logical_dp_groups

[Anton Ivanov]

On 17/09/2021 18:23, Mark Michelson wrote:

Hi Anton,

For the series:

Acked-by: Mark Michelson 

Thanks.


On 9/15/21 8:43 AM, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Work on improving processing with dp_groups enabled has
discovered that the locking mechanism presently in use
is not reliable. Disabling parallel processing if dp_groups
are enabled until the root cause is determined and fixed.

Signed-off-by: Anton Ivanov 
---
  northd/ovn-northd.c | 2 +-
  ovs | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index baaddb73e..3113fafc7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap 
*datapaths, struct hmap *ports,

  }
  }
  -    if (use_parallel_build) {
+    if (use_parallel_build && (!use_logical_dp_groups)) {
  struct hmap *lflow_segs;
  struct lswitch_flow_build_info *lsiv;
  int index;
diff --git a/ovs b/ovs
index 748010ff3..50e5523b9 16


As noted by Ilia I had a submodule change from ovs filter through, so I 
will resend the series.



--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9
+Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330






--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

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

Re: [ovs-dev] [PATCH v2] openvswitch: Fix condition check in do_execute_actions() by using nla_ok()

[Jakub Kicinski]

On Fri, 17 Sep 2021 08:07:14 + Jiasheng Jiang wrote:
> Just using 'rem > 0' might be unsafe, so it's better
> to use the nla_ok() instead.
> Because we can see from the nla_next() that
> '*remaining' might be smaller than 'totlen'. And nla_ok()
> will avoid it happening.
> For example, ovs_dp_process_packet() -> ovs_execute_actions()
> -> do_execute_actions(), and attr comes from OVS_CB(skb)->input_vport,  
> which restores the received packet from the user space.

Right but that's the call trace for where the actions are executed.
Not where they are constructed.

As far as I can tell the action list is constructed in the kernel 
by __ovs_nla_copy_actions(). Since kernel does the formatting, it
can trust the contents are correct. We normally require nla_ok()
when handling input directly from user space, which is not the 
case in do_execute_actions().

And since kernel is sure that the input is correct the extra checking
just adds to the datapath overhead.

Unless you can point out how exactly the input could be invalid 
at this point I'd suggest we leave this code as is. Perhaps add
a comment explaining why input is trusted.

Thanks!

> Fixes: ccb1352e76cff0524e7ccb2074826a092dd13016
> ('net: Add Open vSwitch kernel components.')

FWIW the correct format would have been:

Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")

> Signed-off-by: Jiasheng Jiang 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [OVN Patch v8 1/3] northd: Disable parallel processing for logical_dp_groups

[Ilya Maximets]

On 9/15/21 14:43, anton.iva...@cambridgegreys.com wrote:
> From: Anton Ivanov 
> 
> Work on improving processing with dp_groups enabled has
> discovered that the locking mechanism presently in use
> is not reliable. Disabling parallel processing if dp_groups
> are enabled until the root cause is determined and fixed.
> 
> Signed-off-by: Anton Ivanov 
> ---
>  northd/ovn-northd.c | 2 +-
>  ovs | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index baaddb73e..3113fafc7 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap 
> *datapaths, struct hmap *ports,
>  }
>  }
>  
> -if (use_parallel_build) {
> +if (use_parallel_build && (!use_logical_dp_groups)) {
>  struct hmap *lflow_segs;
>  struct lswitch_flow_build_info *lsiv;
>  int index;
> diff --git a/ovs b/ovs
> index 748010ff3..50e5523b9 16
> --- a/ovs
> +++ b/ovs
> @@ -1 +1 @@
> -Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9
> +Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330
> 

This change breaks the build due to submodule downgrade.

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

Re: [ovs-dev] [PATCH ovn 1/2] Prepare for 21.09.0.

[Mark Michelson]

On 9/17/21 1:15 PM, Ilya Maximets wrote:

On 9/17/21 18:13, Mark Michelson wrote:

Signed-off-by: Mark Michelson 
---
  NEWS | 4 ++--
  configure.ac | 2 +-
  debian/changelog | 4 ++--
  3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 6e81f7bee..c6671b8f5 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-Post-v21.06.0
--
+OVN v21.09.0 - 01 October 2021


We usually have a separate commit 'Set release date ...'
that updates the date on the day of actual release, and
the release tag points to it.  Or do you want to change
the release procedure?

Best regards, Ilya Maximets.


Sure, it's probably best to do it the usual way just in case the release 
is delayed for some reason.





+--
- Added Control Plane Protection support (control plane traffic metering).
- Added path MTU discovery for ingress traffic originated outside of the
  cluster.
diff --git a/configure.ac b/configure.ac
index 4728fa0a6..ca282634c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
  # limitations under the License.
  
  AC_PREREQ(2.63)

-AC_INIT(ovn, 21.06.90, b...@openvswitch.org)
+AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
  AC_CONFIG_MACRO_DIR([m4])
  AC_CONFIG_AUX_DIR([build-aux])
  AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index 81aaed307..c41493b7d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-ovn (21.06.90-1) unstable; urgency=low
+ovn (21.09.0-1) unstable; urgency=low
  
 * New upstream version
  
- -- OVN team   Fri, 18 Jun 2021 13:21:08 -0400

+ -- OVN team   Fri, 01 Oct 2021 12:00:00 -0400
  
  ovn (21.06.0-1) unstable; urgency=low
  





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

Re: [ovs-dev] [PATCH v6 ovn] controller: add datapath meter capability check

[Mark Gray]

On 17/09/2021 17:25, Lorenzo Bianconi wrote:
> Dump datapath meter capabilities before configuring meters in
> ovn-controller
> 
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v5:
> - send new command only on reconnection
> Changes since v4:
> - move rconn setup in ovs_feature_support_update and rename it in
>   ovs_feature_support_run
> - drop ovs_feature_support_init()
> - rename ovs_feature_support_deinit in ovs_feature_support_destroy
> Changes since v3:
> - add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
>   loop
> - cosmetics
> Changes since v2:
> - move meter capability logic in lib/features.c
> Changes since v1:
> - move rconn in ovn-controller to avoid concurrency issues
> ---
>  controller/ovn-controller.c |   7 +--
>  include/ovn/features.h  |   6 ++-
>  lib/actions.c   |   3 ++
>  lib/automake.mk |   1 +
>  lib/features.c  | 102 +++-
>  lib/test-ovn-features.c |   6 +--
>  tests/ovn.at|   4 +-
>  7 files changed, 119 insertions(+), 10 deletions(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index d98ebbbfa..aa7941eeb 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -3574,9 +3574,9 @@ main(int argc, char *argv[])
>   * 'br_int_dp' is valid only if an OVS transaction is possible.
>   */
>  if (ovs_idl_txn
> -&& ovs_feature_support_update(br_int_dp
> -  ? _int_dp->capabilities
> -  : NULL)) {
> +&& ovs_feature_support_run(br_int_dp ?
> +   _int_dp->capabilities : NULL,
> +   br_int ? br_int->name : NULL)) {
>  VLOG_INFO("OVS feature set changed, force recompute.");
>  engine_set_force_recompute(true);
>  }
> @@ -3903,6 +3903,7 @@ loop_done:
>  ovsdb_idl_loop_destroy(_idl_loop);
>  ovsdb_idl_loop_destroy(_idl_loop);
>  
> +ovs_feature_support_destroy();
>  free(ovs_remote);
>  service_stop();
>  
> diff --git a/include/ovn/features.h b/include/ovn/features.h
> index c35d59b14..d12a8eb0d 100644
> --- a/include/ovn/features.h
> +++ b/include/ovn/features.h
> @@ -28,13 +28,17 @@
>   */
>  enum ovs_feature_support_bits {
>  OVS_CT_ZERO_SNAT_SUPPORT_BIT,
> +OVS_DP_METER_SUPPORT_BIT,
>  };
>  
>  enum ovs_feature_value {
>  OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
> +OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT),
>  };
>  
> +void ovs_feature_support_destroy(void);
>  bool ovs_feature_is_supported(enum ovs_feature_value feature);
> -bool ovs_feature_support_update(const struct smap *ovs_capabilities);
> +bool ovs_feature_support_run(const struct smap *ovs_capabilities,
> + const char *br_name);
>  
>  #endif
> diff --git a/lib/actions.c b/lib/actions.c
> index c572e88ae..7cf6be308 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool 
> pause,
>  oc->max_len = UINT16_MAX;
>  oc->reason = OFPR_ACTION;
>  oc->pause = pause;
> +if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
> +meter_id = NX_CTLR_NO_METER;
> +}
>  oc->meter_id = meter_id;
>  
>  struct action_header ah = { .opcode = htonl(opcode) };
> diff --git a/lib/automake.mk b/lib/automake.mk
> index ddfe33948..9f9f447d5 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
>  lib_libovn_la_LDFLAGS = \
>  $(OVS_LTINFO) \
>  -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
> +$(OVS_LIBDIR)/libopenvswitch.la \
>  $(AM_LDFLAGS)
>  lib_libovn_la_SOURCES = \
>   lib/acl-log.c \
> diff --git a/lib/features.c b/lib/features.c
> index fddf4c450..f15ec42bb 100644
> --- a/lib/features.c
> +++ b/lib/features.c
> @@ -18,7 +18,14 @@
>  #include 
>  
>  #include "lib/util.h"
> +#include "lib/dirs.h"
> +#include "socket-util.h"
> +#include "lib/vswitch-idl.h"
>  #include "openvswitch/vlog.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openvswitch/rconn.h"
> +#include "openvswitch/ofp-msgs.h"
> +#include "openvswitch/ofp-meter.h"
>  #include "ovn/features.h"
>  
>  VLOG_DEFINE_THIS_MODULE(features);
> @@ -40,11 +47,16 @@ static uint32_t supported_ovs_features;
>  
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
>  
> +/* ovs-vswitchd connection. */
> +static struct rconn *swconn;
> +static uint32_t conn_seq_no;
> +
>  static bool
>  ovs_feature_is_valid(enum ovs_feature_value feature)
>  {
>  switch (feature) {
>  case OVS_CT_ZERO_SNAT_SUPPORT:
> +case OVS_DP_METER_SUPPORT:
>  return true;
>  default:
>  return false;
> @@ -58,9 +70,93 @@ 

Re: [ovs-dev] [OVN Patch v8 1/3] northd: Disable parallel processing for logical_dp_groups

[Mark Michelson]

Hi Anton,

For the series:

Acked-by: Mark Michelson 

On 9/15/21 8:43 AM, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Work on improving processing with dp_groups enabled has
discovered that the locking mechanism presently in use
is not reliable. Disabling parallel processing if dp_groups
are enabled until the root cause is determined and fixed.

Signed-off-by: Anton Ivanov 
---
  northd/ovn-northd.c | 2 +-
  ovs | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index baaddb73e..3113fafc7 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -12974,7 +12974,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, 
struct hmap *ports,
  }
  }
  
-if (use_parallel_build) {

+if (use_parallel_build && (!use_logical_dp_groups)) {
  struct hmap *lflow_segs;
  struct lswitch_flow_build_info *lsiv;
  int index;
diff --git a/ovs b/ovs
index 748010ff3..50e5523b9 16
--- a/ovs
+++ b/ovs
@@ -1 +1 @@
-Subproject commit 748010ff304b7cd2c43f4eb98a554433f0df07f9
+Subproject commit 50e5523b9b2b154e5fafc5acdcdec85e9cc5a330



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

Re: [ovs-dev] [PATCH] ovsdb-data: Optimize union of sets.

[Ilya Maximets]

On 9/16/21 22:19, Ilya Maximets wrote:
> Current algorithm of ovsdb_datum_union looks like this:
> 
>   for-each atom in b:
>   if not bin_search(a, atom):
>   a[n++] = clone(atom)
>   quicksort(a)
> 
> So, the complexity looks like this:
> 
>Nb * log2(Na)   +Nb +   (Na + Nb) * log2(Na + Nb)
>Comparisonsclones   Comparisons for quicksort
>for search
> 
> ovsdb_datum_union() is heavily used in database transactions while
> new element is added to a set.  For example, if new logical switch
> port is added to a logical switch in OVN.  This is a very common
> use case where CMS adds one new port to an existing switch that
> already has, let's say, 100 ports.  For this case ovsdb-server will
> have to perform:
> 
>1 * log2(100)  + 1 clone + 101 * log2(101)
>ComparisonsComparisons for
>for search   quicksort.
>~7   1~707
>Roughly 714 comparisons of atoms and 1 clone.
> 
> Since binary search can give us position, where new atom should go
> (it's the 'low' index after the search completion) for free, the
> logic can be re-worked like this:
> 
>   copied = 0
>   for-each atom in b:
>   desired_position = bin_search(a, atom)
>   push(result, a[ copied : desired_position - 1 ])
>   copied = desired_position
>   result[n++] = clone(atom)
>   swap(a, result)
> 
> Complexity of this schema:
> 
>Nb * log2(Na)   +Nb + Na
>Comparisonsclones   memory copy on push
>for search
> 
> 'swap' is just a swap of a few pointers.  'push' is not a 'clone',
> but a simple memory copy of 'union ovsdb_atom'.
> 
> In general, this schema substitutes complexity of a quicksort
> with complexity of a memory copy of Na atom structures, where we're
> not even copying strings that these atoms are pointing to.
> 
> Complexity in the example above goes down from 714 comparisons
> to 7 comparisons and memcpy of 100 * sizeof (union ovsdb_atom) bytes.
> 
> General complexity of a memory copy should always be lower than
> complexity of a quicksort, especially because these copies usually
> performed in bulk, so this new schema should work faster for any input.
> 
> All in all, this change allows to execute several times more
> transactions per second for transactions that adds new entries to sets.
> 
> Alternatively, union can be implemented as a linear merge of two
> sorted arrays, but this will result in O(Na) comparisons, which
> is more than Nb * log2(Na) in common case, since Na is usually
> far bigger than Nb.  Linear merge will also mean per-atom memory
> copies instead of copying in bulk.
> 
> 'replace' functionality of ovsdb_datum_union() had no users, so it
> just removed.  But it can easily be added back if needed in the future.
> 
> Signed-off-by: Ilya Maximets 
> ---
>  lib/db-ctl-base.c | 10 +++---
>  lib/ovsdb-data.c  | 84 ---
>  lib/ovsdb-data.h  |  6 ++--
>  lib/ovsdb-idl.c   |  8 ++---
>  ovsdb/mutation.c  |  2 +-
>  vswitchd/bridge.c |  9 +++--
>  6 files changed, 77 insertions(+), 42 deletions(-)

Re-sent as part of a patch set:
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=262864=*

Best regards, Ilya Maximets.

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

[ovs-dev] [PATCH v2 2/2] ovsdb-data: Optimize subtraction of sets.

[Ilya Maximets]

Current algorithm for ovsdb_datum_subtract looks like this:

  for-each atom in a:
  if atom in b:
  swap(atom, )
  destroy(atom)
  quicksort(a)

Complexity:

  Na * log2(Nb)  +  (Na - Nb) * log2(Na - Nb)
Search  Comparisons for quicksort

It's not optimal, especially because Nb << Na in a vast majority of
cases.

Reversing the search phase to look up atoms from 'b' in 'a', and
closing gaps from deleted elements in 'a' by plain memory copy to
avoid quicksort.

Resulted complexity:

  Nb * log2(Na)  +(Na - Nb)
Search  Memory copies

Subtraction is heavily used while executing database transactions.
For example, to remove one port from a logical switch in OVN.
Complexity of such operation if original logical switch had 100 ports
goes down from

 100 * log2(1)   = 100 comparisons for search and
  99 * log2(99)  = 656 comparisons for quicksort
   --
   756 comparisons in total
to only

   1 * log2(100) = 7 comparisons for search
   + memory copy of 99 * sizeof (union ovsdb_atom) bytes.

We could use memmove to close the gaps after removing atoms, but
it will lead to 2 memory copies inside the call, while we can perform
only one to the temporary 'result' and swap pointers.

Performance in cases, where sizes of 'a' and 'b' are comparable,
should not change.  Cases with Nb >> Na should not happen in practice.

All in all, this change allows ovsdb-server to perform several times
more transactions, that removes elements from sets, per second.

Signed-off-by: Ilya Maximets 
---
 lib/ovsdb-data.c | 52 +---
 1 file changed, 40 insertions(+), 12 deletions(-)

diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
index 11bf95fed..b6129d6ba 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -2019,26 +2019,54 @@ ovsdb_datum_subtract(struct ovsdb_datum *a, const 
struct ovsdb_type *a_type,
  const struct ovsdb_datum *b,
  const struct ovsdb_type *b_type)
 {
-bool changed = false;
-size_t i;
+size_t i, ai, bi, n_idx;
+size_t *idx;
 
 ovs_assert(a_type->key.type == b_type->key.type);
 ovs_assert(a_type->value.type == b_type->value.type
|| b_type->value.type == OVSDB_TYPE_VOID);
 
-/* XXX The big-O of this could easily be improved. */
-for (i = 0; i < a->n; ) {
-unsigned int idx = ovsdb_datum_find(a, i, b, b_type);
-if (idx != UINT_MAX) {
-changed = true;
-ovsdb_datum_remove_unsafe(a, i, a_type);
-} else {
-i++;
+idx = xmalloc(b->n * sizeof *idx);
+n_idx = 0;
+for (bi = 0; bi < b->n; bi++) {
+ai = ovsdb_datum_find(b, bi, a, b_type);
+if (ai == UINT_MAX || (n_idx && ai <= idx[n_idx - 1])) {
+/* No such atom in 'a' or it's already marked for removal. */
+continue;
 }
+idx[n_idx++] = ai;
 }
-if (changed) {
-ovsdb_datum_sort_assert(a, a_type->key.type);
+if (!n_idx) {
+free(idx);
+return;
 }
+
+struct ovsdb_datum result;
+
+ovsdb_datum_init_empty();
+ovsdb_datum_reallocate(, a_type, a->n - n_idx);
+
+for (i = 0; i < n_idx; i++) {
+ai = idx[i];
+
+/* Destroying atom. */
+ovsdb_atom_destroy(>keys[ai], a_type->key.type);
+if (a_type->value.type != OVSDB_TYPE_VOID) {
+ovsdb_atom_destroy(>values[ai], a_type->value.type);
+}
+
+/* Copy non-removed atoms from 'a' to result. */
+unsigned int start_idx = (i > 0) ? idx[i - 1] + 1 : 0;
+ovsdb_datum_push_unsafe(, a, start_idx, ai - start_idx, a_type);
+}
+/* Copying remaining atoms. */
+ovsdb_datum_push_unsafe(, a, idx[n_idx - 1] + 1,
+a->n - (idx[n_idx - 1] + 1), a_type);
+a->n = 0;
+
+ovsdb_datum_swap(, a);
+ovsdb_datum_destroy(, a_type);
+free(idx);
 }
 
 struct ovsdb_symbol_table *
-- 
2.31.1

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

[ovs-dev] [PATCH v2 1/2] ovsdb-data: Optimize union of sets.

[Ilya Maximets]

Current algorithm of ovsdb_datum_union looks like this:

  for-each atom in b:
  if not bin_search(a, atom):
  push(a, clone(atom))
  quicksort(a)

So, the complexity looks like this:

   Nb * log2(Na)   +Nb +   (Na + Nb) * log2(Na + Nb)
   Comparisonsclones   Comparisons for quicksort
   for search

ovsdb_datum_union() is heavily used in database transactions while
new element is added to a set.  For example, if new logical switch
port is added to a logical switch in OVN.  This is a very common
use case where CMS adds one new port to an existing switch that
already has, let's say, 100 ports.  For this case ovsdb-server will
have to perform:

   1 * log2(100)  + 1 clone + 101 * log2(101)
   ComparisonsComparisons for
   for search   quicksort.
   ~7   1~707
   Roughly 714 comparisons of atoms and 1 clone.

Since binary search can give us position, where new atom should go
(it's the 'low' index after the search completion) for free, the
logic can be re-worked like this:

  copied = 0
  for-each atom in b:
  desired_position = bin_search(a, atom)
  push(result, a[ copied : desired_position - 1 ])
  copied = desired_position
  push(result, clone(atom))
  push(result, a[ copied : Na ])
  swap(a, result)

Complexity of this schema:

   Nb * log2(Na)   +Nb + Na
   Comparisonsclones   memory copy on push
   for search

'swap' is just a swap of a few pointers.  'push' is not a 'clone',
but a simple memory copy of 'union ovsdb_atom'.

In general, this schema substitutes complexity of a quicksort
with complexity of a memory copy of Na atom structures, where we're
not even copying strings that these atoms are pointing to.

Complexity in the example above goes down from 714 comparisons
to 7 comparisons and memcpy of 100 * sizeof (union ovsdb_atom) bytes.

General complexity of a memory copy should always be lower than
complexity of a quicksort, especially because these copies usually
performed in bulk, so this new schema should work faster for any input.

All in all, this change allows to execute several times more
transactions per second for transactions that adds new entries to sets.

Alternatively, union can be implemented as a linear merge of two
sorted arrays, but this will result in O(Na) comparisons, which
is more than Nb * log2(Na) in common case, since Na is usually
far bigger than Nb.  Linear merge will also mean per-atom memory
copies instead of copying in bulk.

'replace' functionality of ovsdb_datum_union() had no users, so it
just removed.  But it can easily be added back if needed in the future.

Signed-off-by: Ilya Maximets 
---
 lib/db-ctl-base.c | 10 +++---
 lib/ovsdb-data.c  | 84 ---
 lib/ovsdb-data.h  |  6 ++--
 lib/ovsdb-idl.c   |  8 ++---
 ovsdb/mutation.c  |  2 +-
 vswitchd/bridge.c |  9 +++--
 6 files changed, 77 insertions(+), 42 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 77cc76a9f..f69868702 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -321,7 +321,7 @@ get_row_by_id(struct ctl_context *ctx,
 const union ovsdb_atom key_atom
 = { .string = CONST_CAST(char *, id->key) };
 unsigned int i = ovsdb_datum_find_key(datum, _atom,
-  OVSDB_TYPE_STRING);
+  OVSDB_TYPE_STRING, NULL);
 name = i == UINT_MAX ? NULL : >values[i];
 }
 if (!name) {
@@ -820,7 +820,7 @@ check_condition(const struct ovsdb_idl_table_class *table,
 }
 
 idx = ovsdb_datum_find_key(have_datum,
-   _key, column->type.key.type);
+   _key, column->type.key.type, NULL);
 if (idx == UINT_MAX && !is_set_operator(operator)) {
 retval = false;
 } else {
@@ -993,7 +993,7 @@ cmd_get(struct ctl_context *ctx)
 }
 
 idx = ovsdb_datum_find_key(datum, ,
-   column->type.key.type);
+   column->type.key.type, NULL);
 if (idx == UINT_MAX) {
 if (must_exist) {
 ctl_error(
@@ -1375,7 +1375,7 @@ set_column(const struct ovsdb_idl_table_class *table,
 ovsdb_atom_destroy(, column->type.value.type);
 
 ovsdb_datum_union(, ovsdb_idl_read(row, column),
-  >type, false);
+  >type);
 ovsdb_idl_txn_verify(row, column);
 ovsdb_idl_txn_write(row, column, );
 } else {
@@ -1514,7 +1514,7 @@ cmd_add(struct ctl_context *ctx)
 ovsdb_datum_destroy(, >type);
 return;
 }
-ovsdb_datum_union(, , type, false);
+ovsdb_datum_union(, , type);
 ovsdb_datum_destroy(, type);
 }
 if (old.n > type->n_max) {
diff --git 

[ovs-dev] [PATCH v2 0/2] ovsdb: Optimize set operations.

[Ilya Maximets]

Ilya Maximets (2):
  ovsdb-data: Optimize union of sets.
  ovsdb-data: Optimize subtraction of sets.

 lib/db-ctl-base.c |  10 ++--
 lib/ovsdb-data.c  | 136 +-
 lib/ovsdb-data.h  |   6 +-
 lib/ovsdb-idl.c   |   8 +--
 ovsdb/mutation.c  |   2 +-
 vswitchd/bridge.c |   9 ++-
 6 files changed, 117 insertions(+), 54 deletions(-)

-- 
2.31.1

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

Re: [ovs-dev] [PATCH ovn 1/2] Prepare for 21.09.0.

[Ilya Maximets]

On 9/17/21 18:13, Mark Michelson wrote:
> Signed-off-by: Mark Michelson 
> ---
>  NEWS | 4 ++--
>  configure.ac | 2 +-
>  debian/changelog | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 6e81f7bee..c6671b8f5 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,5 @@
> -Post-v21.06.0
> --
> +OVN v21.09.0 - 01 October 2021

We usually have a separate commit 'Set release date ...'
that updates the date on the day of actual release, and
the release tag points to it.  Or do you want to change
the release procedure?

Best regards, Ilya Maximets.

> +--
>- Added Control Plane Protection support (control plane traffic metering).
>- Added path MTU discovery for ingress traffic originated outside of the
>  cluster.
> diff --git a/configure.ac b/configure.ac
> index 4728fa0a6..ca282634c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>  
>  AC_PREREQ(2.63)
> -AC_INIT(ovn, 21.06.90, b...@openvswitch.org)
> +AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
>  AC_CONFIG_HEADERS([config.h])
> diff --git a/debian/changelog b/debian/changelog
> index 81aaed307..c41493b7d 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,8 +1,8 @@
> -ovn (21.06.90-1) unstable; urgency=low
> +ovn (21.09.0-1) unstable; urgency=low
>  
> * New upstream version
>  
> - -- OVN team   Fri, 18 Jun 2021 13:21:08 -0400
> + -- OVN team   Fri, 01 Oct 2021 12:00:00 -0400
>  
>  ovn (21.06.0-1) unstable; urgency=low
>  
> 

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

Re: [ovs-dev] [PATCH ovn v4 4/9] controller: Move OVS port functions to new module.

[Frode Nordahl]

On Fri, Sep 17, 2021 at 6:17 PM Mark Michelson  wrote:
>
> Hi Frode,
>
> The changes I list below are all minor and could be taken care of by
> whoever may merge this series. I don't think it's worth spinning a new
> version of the series for these changes alone. However, I do see that
> there were other findings by Numan for later patches.

Thank you for taking the time to review, Mark, most appreciated! I'll
make sure to incorporate your feedback into the next roll of the
series.

Yes, Numan did provide (great) feedback on patch 8 which requires more
work than can physically fit before the 21.09 hard freeze, which is
unfortunate for me but ok. It will make the result all the better.

Cheers!

-- 
Frode Nordahl


> On 9/3/21 3:27 PM, Frode Nordahl wrote:
> > Up until now the controller patch module has been the only
> > consumer of functions to maintain OVS ports and interfaces.
> >
> > With the introduction of infrastructure for plugging providers
> > these functions will also be consumed by the controller binding
> > module.
> >
> > As such we introduce a new module called ovsport where these
> > shared utility functions can live and be consumed by multiple
> > modules.
> >
> > Signed-off-by: Frode Nordahl 
> > ---
> >   controller/automake.mk |   4 +-
> >   controller/ovsport.c   | 256 +
> >   controller/ovsport.h   |  60 ++
> >   3 files changed, 319 insertions(+), 1 deletion(-)
> >   create mode 100644 controller/ovsport.c
> >   create mode 100644 controller/ovsport.h
> >
> > diff --git a/controller/automake.mk b/controller/automake.mk
> > index 41f907d6e..ad2d68af2 100644
> > --- a/controller/automake.mk
> > +++ b/controller/automake.mk
> > @@ -35,7 +35,9 @@ controller_ovn_controller_SOURCES = \
> >   controller/mac-learn.c \
> >   controller/mac-learn.h \
> >   controller/local_data.c \
> > - controller/local_data.h
> > + controller/local_data.h \
> > + controller/ovsport.h \
> > + controller/ovsport.c
> >
> >   controller_ovn_controller_LDADD = lib/libovn.la 
> > $(OVS_LIBDIR)/libopenvswitch.la
> >   man_MANS += controller/ovn-controller.8
> > diff --git a/controller/ovsport.c b/controller/ovsport.c
> > new file mode 100644
> > index 0..b1183e9ed
> > --- /dev/null
> > +++ b/controller/ovsport.c
> > @@ -0,0 +1,256 @@
> > +/* Copyright (c) 2021 Canonical
> > + *
> > + * 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 "ovsport.h"
> > +
> > +#include "lib/vswitch-idl.h"
> > +#include "openvswitch/vlog.h"
> > +
> > +VLOG_DEFINE_THIS_MODULE(ovsport);
> > +
> > +/* Create a port and interface record and add it to 'bridge' in the Open
> > + * vSwitch database represented by 'ovs_idl_txn'.
> > + *
> > + * 'name' is required and is used both for the name of the port and 
> > interface
> > + * records.  Depending on the contents of the optional 'iface_type' 
> > parameter
> > + * the name may need to refer to an existing interface in the system.  It 
> > is
> > + * the callers responsibility to ensure that no other port with the desired
>
> s/callers/caller's/
>
> > + * name already exist.
>
> s/exist/exists/
>
> > + *
> > + * 'iface_type' optionally specifies the type of intercace, otherwise set 
> > it to
>
> s/intercace/interface/
>
> > + * NULL.
> > + *
> > + * 'port_external_ids' - the contents of the map will be used to fill the
> > + * external_ids column of the created port record, otherwise set it to 
> > NULL.
> > + *
> > + * 'iface_external_ids' - the contents of the map will be used to fill the
> > + * external_ids column of the created interface record, otherwise set it to
> > + * NULL.
> > + *
> > + * 'iface_options' - the contents of the map will be used to fill the 
> > options
> > + * column of the created interface record, otherwise set it to NULL.
> > + *
> > + * 'iface_mtu_request' - if a value > 0 is provided it will be filled into 
> > the
> > + * mtu_request column of the created interface record. */
> > +void
> > +ovsport_create(struct ovsdb_idl_txn *ovs_idl_txn,
> > +   const struct ovsrec_bridge *bridge,
> > +   const char *name,
> > +   const char *iface_type,
> > +   const struct smap *port_external_ids,
> > +   const struct smap *iface_external_ids,
> > +   const struct smap *iface_options,
> > +   const int64_t 

Re: [ovs-dev] [PATCH ovn v4 8/9] binding: Consider plugging of ports on CMS request.

[Frode Nordahl]

On Fri, Sep 17, 2021 at 1:16 AM Numan Siddique  wrote:
>
> On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl
>  wrote:
> >
> > When OVN is linked with an appropriate plugging implementation,
> > CMS can request OVN to plug individual lports into the local
> > Open vSwitch instance.
> >
> > The port and instance record will be maintained during the lifetime
> > of the lport and it will be removed on release of lport.
> >
> > Signed-off-by: Frode Nordahl 
>
> Hi Frode,
>
> I've a few comments with the approach taken in this patch.
> This patch calls the plug_port APIs from binding.c.  This complicates
> the already complicated binding.c code and I think plug_port() related stuff
> can be handled separately.  I don't see a need for binding.c to be aware of
> plugging feature.  For binding.c to claim a port, the ovs interface should
> have external_ids:iface-id set to the port binding lport_name.
>
> Below is how I would see this can be done:
>
> 1.  Add a new file called - controller/plug.c  which will handle port binding
> changes - plug_handle_port_binding_changes (and possibly ovs
> port/ovs interface changes).
>
> 2.  The function plug_handle_port_binding_changes() will iterate through the
>  tracked port bindings and if the port binding has the required options 
> set
> (i.e requested-chassis and plug-type) it will call plug_port APIs and 
> create
> or delete OVS ports.  This function can  access the lbinding data
> stored in the runtime data.
>
> 3.   For recompute scenarios,  there can be a function - plug_run() which will
>  iterate through all the port bindings and create/delete ovs ports.
>
> 4.  When the OVS ports are created / deleted in (2),  in the next run,
> the binding module
>  will claim or release the port binding.  binding.c module
> wouldn't need to care
>  if the port binding / ovs port is created by the plug module or
> by some other
> mechanism.
>
> 5.  The functions plug_handle_port_binding_changes() can be either called by
>  runtime_data_sb_port_binding_handler() (i.e runtime engine node)
>  or another engine node for plug handling can be created which will handle
>  port binding  changes (and possibly ovs port/interface changes).
>  Similarly for full recompute,  either runtime_data_run() can call
> plug_run()
>  or a new engine run function can call it.  Having a separate engine node
>  seems better but then it needs to have runtime data as input
>  to access the lbinding information.
>
> 6.  If you think plug.c should handle ovs port/interface changes,  then you
>  can add handlers for it too.
>
>
> What do you think ? Would this proposal work ?   Let me know if
> something is not clear
> and I can try to explain better.

Numan, thank you so much for the review and for providing such a
detailed and complete counter proposal, this is most appreciated.

I agree with you completely and have started working on such an approach.

This does mean I will most likely miss the 21.09 release with this
series, which is unfortunate for me, but that's ok. Better to get it
into a great shape for the next one than taking risks or cutting
corners for this one.

Cheers!

-- 
Frode Nordahl

> Thanks
> Numan
>
> > ---
> >  controller/binding.c| 247 ++--
> >  tests/ovn-controller.at |  31 +
> >  tests/ovn-macros.at |   2 +-
> >  3 files changed, 272 insertions(+), 8 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 938e1d81d..ae67ee3fc 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -35,7 +35,9 @@
> >  #include "local_data.h"
> >  #include "lport.h"
> >  #include "ovn-controller.h"
> > +#include "ovsport.h"
> >  #include "patch.h"
> > +#include "plug.h"
> >
> >  VLOG_DEFINE_THIS_MODULE(binding);
> >
> > @@ -45,6 +47,8 @@ VLOG_DEFINE_THIS_MODULE(binding);
> >   */
> >  #define OVN_INSTALLED_EXT_ID "ovn-installed"
> >
> > +#define OVN_PLUGGED_EXT_ID "ovn-plugged"
> > +
> >  #define OVN_QOS_TYPE "linux-htb"
> >
> >  struct qos_queue {
> > @@ -71,10 +75,13 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> >
> >  ovsdb_idl_add_table(ovs_idl, _table_interface);
> >  ovsdb_idl_track_add_column(ovs_idl, _interface_col_name);
> > +ovsdb_idl_track_add_column(ovs_idl, _interface_col_type);
> >  ovsdb_idl_track_add_column(ovs_idl, 
> > _interface_col_external_ids);
> >  ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd);
> >  ovsdb_idl_track_add_column(ovs_idl, _interface_col_bfd_status);
> >  ovsdb_idl_track_add_column(ovs_idl, _interface_col_status);
> > +ovsdb_idl_track_add_column(ovs_idl, _interface_col_options);
> > +ovsdb_idl_track_add_column(ovs_idl, _interface_col_mtu_request);
> >
> >  ovsdb_idl_add_table(ovs_idl, _table_qos);
> >  ovsdb_idl_add_column(ovs_idl, _qos_col_type);
> > @@ -1090,6 +1097,51 @@ release_binding_lport(const struct sbrec_chassis 
> > 

[ovs-dev] [PATCH v6 ovn] controller: add datapath meter capability check

[Lorenzo Bianconi]

Dump datapath meter capabilities before configuring meters in
ovn-controller

Signed-off-by: Lorenzo Bianconi 
---
Changes since v5:
- send new command only on reconnection
Changes since v4:
- move rconn setup in ovs_feature_support_update and rename it in
  ovs_feature_support_run
- drop ovs_feature_support_init()
- rename ovs_feature_support_deinit in ovs_feature_support_destroy
Changes since v3:
- add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
  loop
- cosmetics
Changes since v2:
- move meter capability logic in lib/features.c
Changes since v1:
- move rconn in ovn-controller to avoid concurrency issues
---
 controller/ovn-controller.c |   7 +--
 include/ovn/features.h  |   6 ++-
 lib/actions.c   |   3 ++
 lib/automake.mk |   1 +
 lib/features.c  | 102 +++-
 lib/test-ovn-features.c |   6 +--
 tests/ovn.at|   4 +-
 7 files changed, 119 insertions(+), 10 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d98ebbbfa..aa7941eeb 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3574,9 +3574,9 @@ main(int argc, char *argv[])
  * 'br_int_dp' is valid only if an OVS transaction is possible.
  */
 if (ovs_idl_txn
-&& ovs_feature_support_update(br_int_dp
-  ? _int_dp->capabilities
-  : NULL)) {
+&& ovs_feature_support_run(br_int_dp ?
+   _int_dp->capabilities : NULL,
+   br_int ? br_int->name : NULL)) {
 VLOG_INFO("OVS feature set changed, force recompute.");
 engine_set_force_recompute(true);
 }
@@ -3903,6 +3903,7 @@ loop_done:
 ovsdb_idl_loop_destroy(_idl_loop);
 ovsdb_idl_loop_destroy(_idl_loop);
 
+ovs_feature_support_destroy();
 free(ovs_remote);
 service_stop();
 
diff --git a/include/ovn/features.h b/include/ovn/features.h
index c35d59b14..d12a8eb0d 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -28,13 +28,17 @@
  */
 enum ovs_feature_support_bits {
 OVS_CT_ZERO_SNAT_SUPPORT_BIT,
+OVS_DP_METER_SUPPORT_BIT,
 };
 
 enum ovs_feature_value {
 OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
+OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT),
 };
 
+void ovs_feature_support_destroy(void);
 bool ovs_feature_is_supported(enum ovs_feature_value feature);
-bool ovs_feature_support_update(const struct smap *ovs_capabilities);
+bool ovs_feature_support_run(const struct smap *ovs_capabilities,
+ const char *br_name);
 
 #endif
diff --git a/lib/actions.c b/lib/actions.c
index c572e88ae..7cf6be308 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool 
pause,
 oc->max_len = UINT16_MAX;
 oc->reason = OFPR_ACTION;
 oc->pause = pause;
+if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
+meter_id = NX_CTLR_NO_METER;
+}
 oc->meter_id = meter_id;
 
 struct action_header ah = { .opcode = htonl(opcode) };
diff --git a/lib/automake.mk b/lib/automake.mk
index ddfe33948..9f9f447d5 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
 lib_libovn_la_LDFLAGS = \
 $(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
+$(OVS_LIBDIR)/libopenvswitch.la \
 $(AM_LDFLAGS)
 lib_libovn_la_SOURCES = \
lib/acl-log.c \
diff --git a/lib/features.c b/lib/features.c
index fddf4c450..f15ec42bb 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -18,7 +18,14 @@
 #include 
 
 #include "lib/util.h"
+#include "lib/dirs.h"
+#include "socket-util.h"
+#include "lib/vswitch-idl.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/rconn.h"
+#include "openvswitch/ofp-msgs.h"
+#include "openvswitch/ofp-meter.h"
 #include "ovn/features.h"
 
 VLOG_DEFINE_THIS_MODULE(features);
@@ -40,11 +47,16 @@ static uint32_t supported_ovs_features;
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
 
+/* ovs-vswitchd connection. */
+static struct rconn *swconn;
+static uint32_t conn_seq_no;
+
 static bool
 ovs_feature_is_valid(enum ovs_feature_value feature)
 {
 switch (feature) {
 case OVS_CT_ZERO_SNAT_SUPPORT:
+case OVS_DP_METER_SUPPORT:
 return true;
 default:
 return false;
@@ -58,9 +70,93 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
 return supported_ovs_features & feature;
 }
 
+static void
+ovs_feature_rconn_setup(const char *br_name)
+{
+if (!swconn) {
+swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
+}
+
+if (!rconn_is_connected(swconn)) {
+char 

Re: [ovs-dev] [PATCH ovn v4 4/9] controller: Move OVS port functions to new module.

[Mark Michelson]

Hi Frode,

The changes I list below are all minor and could be taken care of by 
whoever may merge this series. I don't think it's worth spinning a new 
version of the series for these changes alone. However, I do see that 
there were other findings by Numan for later patches.


On 9/3/21 3:27 PM, Frode Nordahl wrote:

Up until now the controller patch module has been the only
consumer of functions to maintain OVS ports and interfaces.

With the introduction of infrastructure for plugging providers
these functions will also be consumed by the controller binding
module.

As such we introduce a new module called ovsport where these
shared utility functions can live and be consumed by multiple
modules.

Signed-off-by: Frode Nordahl 
---
  controller/automake.mk |   4 +-
  controller/ovsport.c   | 256 +
  controller/ovsport.h   |  60 ++
  3 files changed, 319 insertions(+), 1 deletion(-)
  create mode 100644 controller/ovsport.c
  create mode 100644 controller/ovsport.h

diff --git a/controller/automake.mk b/controller/automake.mk
index 41f907d6e..ad2d68af2 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -35,7 +35,9 @@ controller_ovn_controller_SOURCES = \
controller/mac-learn.c \
controller/mac-learn.h \
controller/local_data.c \
-   controller/local_data.h
+   controller/local_data.h \
+   controller/ovsport.h \
+   controller/ovsport.c
  
  controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la

  man_MANS += controller/ovn-controller.8
diff --git a/controller/ovsport.c b/controller/ovsport.c
new file mode 100644
index 0..b1183e9ed
--- /dev/null
+++ b/controller/ovsport.c
@@ -0,0 +1,256 @@
+/* Copyright (c) 2021 Canonical
+ *
+ * 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 "ovsport.h"
+
+#include "lib/vswitch-idl.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(ovsport);
+
+/* Create a port and interface record and add it to 'bridge' in the Open
+ * vSwitch database represented by 'ovs_idl_txn'.
+ *
+ * 'name' is required and is used both for the name of the port and interface
+ * records.  Depending on the contents of the optional 'iface_type' parameter
+ * the name may need to refer to an existing interface in the system.  It is
+ * the callers responsibility to ensure that no other port with the desired


s/callers/caller's/


+ * name already exist.


s/exist/exists/


+ *
+ * 'iface_type' optionally specifies the type of intercace, otherwise set it to


s/intercace/interface/


+ * NULL.
+ *
+ * 'port_external_ids' - the contents of the map will be used to fill the
+ * external_ids column of the created port record, otherwise set it to NULL.
+ *
+ * 'iface_external_ids' - the contents of the map will be used to fill the
+ * external_ids column of the created interface record, otherwise set it to
+ * NULL.
+ *
+ * 'iface_options' - the contents of the map will be used to fill the options
+ * column of the created interface record, otherwise set it to NULL.
+ *
+ * 'iface_mtu_request' - if a value > 0 is provided it will be filled into the
+ * mtu_request column of the created interface record. */
+void
+ovsport_create(struct ovsdb_idl_txn *ovs_idl_txn,
+   const struct ovsrec_bridge *bridge,
+   const char *name,
+   const char *iface_type,
+   const struct smap *port_external_ids,
+   const struct smap *iface_external_ids,
+   const struct smap *iface_options,
+   const int64_t iface_mtu_request)
+{
+struct ovsrec_interface *iface;
+iface = ovsrec_interface_insert(ovs_idl_txn);
+ovsrec_interface_set_name(iface, name);
+if (iface_type) {
+ovsrec_interface_set_type(iface, iface_type);
+}
+ovsrec_interface_set_external_ids(iface, iface_external_ids);
+ovsrec_interface_set_options(iface, iface_options);
+ovsrec_interface_set_mtu_request(
+iface, _mtu_request, iface_mtu_request > 0);
+
+struct ovsrec_port *port;
+port = ovsrec_port_insert(ovs_idl_txn);
+ovsrec_port_set_name(port, name);
+ovsrec_port_set_interfaces(port, , 1);
+ovsrec_port_set_external_ids(port, port_external_ids);
+
+ovsrec_bridge_update_ports_addvalue(bridge, port);
+ovsrec_bridge_verify_ports(bridge);
+}
+
+/* Remove 'port' from 'bridge' and delete the 'port' recrd and any records

Re: [ovs-dev] [PATCH v2] dpif-netdev: Fix pmd thread comments to include SMC.

[Kevin Traynor]

On 09/09/2021 16:30, Cian Ferriter wrote:
> These comments are relevant to SMC too.
> 
> Fixes: 60d8ccae135f ("dpif-netdev: Add SMC cache after EMC cache")
> Signed-off-by: Cian Ferriter 
> Acked-by: Kevin Traynor 
> ---

Just to confirm, lgtm.

Ilya, this is a minor non-functional change. If you do wish to backport,
then perhaps just for 2.16 as for earlier OVS versions the files were
not split and patch does not apply. It doesn't seem worth manually rebasing.

>  lib/dpif-netdev-private-dfc.h| 3 ++-
>  lib/dpif-netdev-private-thread.h | 8 
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/dpif-netdev-private-dfc.h b/lib/dpif-netdev-private-dfc.h
> index 92092ebec..3dfc91f0f 100644
> --- a/lib/dpif-netdev-private-dfc.h
> +++ b/lib/dpif-netdev-private-dfc.h
> @@ -59,7 +59,8 @@ extern "C" {
>   * Thread-safety
>   * =
>   *
> - * Each pmd_thread has its own private exact match cache.
> + * Each pmd_thread has its own private exact match cache and signature match
> + * cache.
>   * If dp_netdev_input is not called from a pmd thread, a mutex is used.
>   */
>  
> diff --git a/lib/dpif-netdev-private-thread.h 
> b/lib/dpif-netdev-private-thread.h
> index a782d9678..ac4885538 100644
> --- a/lib/dpif-netdev-private-thread.h
> +++ b/lib/dpif-netdev-private-thread.h
> @@ -78,10 +78,10 @@ struct dp_netdev_pmd_thread {
>  struct ovs_refcount ref_cnt;/* Every reference must be refcount'ed. 
> */
>  struct cmap_node node;  /* In 'dp->poll_threads'. */
>  
> -/* Per thread exact-match cache.  Note, the instance for cpu core
> - * NON_PMD_CORE_ID can be accessed by multiple threads, and thusly
> - * need to be protected by 'non_pmd_mutex'.  Every other instance
> - * will only be accessed by its own pmd thread. */
> +/* Per thread exact match cache and signature match cache.  Note, the
> + * instance for cpu core NON_PMD_CORE_ID can be accessed by multiple
> + * threads, and thusly need to be protected by 'non_pmd_mutex'.  Every
> + * other instance will only be accessed by its own pmd thread. */
>  OVS_ALIGNED_VAR(CACHE_LINE_SIZE) struct dfc_cache flow_cache;
>  
>  /* Flow-Table and classifiers
> 

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

[ovs-dev] [PATCH ovn 1/2] Prepare for 21.09.0.

[Mark Michelson]

Signed-off-by: Mark Michelson 
---
 NEWS | 4 ++--
 configure.ac | 2 +-
 debian/changelog | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 6e81f7bee..c6671b8f5 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-Post-v21.06.0
--
+OVN v21.09.0 - 01 October 2021
+--
   - Added Control Plane Protection support (control plane traffic metering).
   - Added path MTU discovery for ingress traffic originated outside of the
 cluster.
diff --git a/configure.ac b/configure.ac
index 4728fa0a6..ca282634c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 21.06.90, b...@openvswitch.org)
+AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index 81aaed307..c41493b7d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,8 +1,8 @@
-ovn (21.06.90-1) unstable; urgency=low
+ovn (21.09.0-1) unstable; urgency=low
 
* New upstream version
 
- -- OVN team   Fri, 18 Jun 2021 13:21:08 -0400
+ -- OVN team   Fri, 01 Oct 2021 12:00:00 -0400
 
 ovn (21.06.0-1) unstable; urgency=low
 
-- 
2.31.1

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

[ovs-dev] [PATCH ovn 2/2] Prepare for post-21.09.0.

[Mark Michelson]

Signed-off-by: Mark Michelson 
---
 NEWS | 3 +++
 configure.ac | 2 +-
 debian/changelog | 6 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index c6671b8f5..dfd9b1f19 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+Post v21.09.0
+-
+
 OVN v21.09.0 - 01 October 2021
 --
   - Added Control Plane Protection support (control plane traffic metering).
diff --git a/configure.ac b/configure.ac
index ca282634c..d1b9b4d55 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,7 +13,7 @@
 # limitations under the License.
 
 AC_PREREQ(2.63)
-AC_INIT(ovn, 21.09.0, b...@openvswitch.org)
+AC_INIT(ovn, 21.09.90, b...@openvswitch.org)
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/debian/changelog b/debian/changelog
index c41493b7d..7931c7920 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+ovn (21.09.90-1) unstable; urgency=low
+
+   * New upstream version
+
+ -- OVN team   Fri, 01 Oct 2021 12:00:00 -0400
+
 ovn (21.09.0-1) unstable; urgency=low
 
* New upstream version
-- 
2.31.1

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

Re: [ovs-dev] [PATCH ovn 3/3] northd: support HW VTEP with stateful datapath

[Vladislav Odintsov]

Hi,

done: 
https://patchwork.ozlabs.org/project/ovn/patch/20210917150104.6143-1-odiv...@gmail.com/

Regards,
Vladislav Odintsov

> On 17 Sep 2021, at 17:44, Numan Siddique  wrote:
> 
> On Wed, Sep 15, 2021 at 8:07 PM Vladislav Odintsov  > wrote:
>> 
>> A packet going from HW VTEP device to VIF port when arrives to
>> hypervisor chassis should go through LS ingress pipeline to l2_lkp
>> stage without any match. In l2_lkp stage an output port is
>> determined and then packet passed to LS egress pipeline for futher
>> processing and to VIF port delivery.
>> 
>> Prior to this commit a packet, which was received from HW VTEP
>> device was dropped in an LS ingress datapath, where stateful services
>> were defined (ACLs, LBs).
>> 
>> To fix this issue we add a special flag-bit which can be used in LS
>> pipelines, to check whether the packet came from HW VTEP devices.
>> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
>> to skip such packets.
>> 
>> Signed-off-by: Vladislav Odintsov 
> 
> This needs a rebase.
> 
> Thanks
> Numan
> 
>> ---
>> Please note: I've got no experience in DDLog and have no ability to 
>> extensively
>> test these changes.
>> Just local ./configure --with-ddlog=...; make; make check was run
>> It seems, that only irrelevant to these changes tests were 
>> failed.
>> ---
>> northd/ovn-northd.c  | 14 ++
>> northd/ovn_northd.dl | 33 +++--
>> tests/ovn-northd.at  |  2 ++
>> 3 files changed, 47 insertions(+), 2 deletions(-)
>> 
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index 0ee2ba221..2a795d9e1 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -236,6 +236,7 @@ enum ovn_stage {
>> #define REGBIT_LKUP_FDB   "reg0[11]"
>> #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
>> #define REGBIT_ACL_LABEL  "reg0[13]"
>> +#define REGBIT_FROM_RAMP  "reg0[14]"
>> 
>> #define REG_ORIG_DIP_IPV4 "reg1"
>> #define REG_ORIG_DIP_IPV6 "xxreg1"
>> @@ -5175,6 +5176,11 @@ build_lswitch_input_port_sec_op(
>> if (queue_id) {
>> ds_put_format(actions, "set_queue(%s); ", queue_id);
>> }
>> +
>> +if (!strcmp(op->nbsp->type, "vtep")) {
>> +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
>> +}
>> +
>> ds_put_cstr(actions, "next;");
>> ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
>> S_SWITCH_IN_PORT_SEC_L2,
>>   50, ds_cstr(match), ds_cstr(actions),
>> @@ -5422,6 +5428,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
>> *port_groups,
>>   "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
>>   "(udp && udp.src == 546 && udp.dst == 547)", "next;");
>> 
>> +/* Do not send coming from RAMP switch packets to conntrack. */
>> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
>> +  REGBIT_FROM_RAMP" == 1", "next;");
>> +
>> /* Ingress and Egress Pre-ACL Table (Priority 100).
>>  *
>>  * Regardless of whether the ACL is "from-lport" or "to-lport",
>> @@ -5526,6 +5536,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
>> *lflows,
>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
>>   "eth.src == $svc_monitor_mac", "next;");
>> 
>> +/* Do not send coming from RAMP switch packets to conntrack. */
>> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
>> +  REGBIT_FROM_RAMP" == 1", "next;");
>> +
>> /* Allow all packets to go to next tables by default. */
>> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
>> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
>> index d91f8111f..5b4ae980a 100644
>> --- a/northd/ovn_northd.dl
>> +++ b/northd/ovn_northd.dl
>> @@ -1622,6 +1622,7 @@ function rEGBIT_ACL_HINT_BLOCK()   : string = 
>> "reg0[10]"
>> function rEGBIT_LKUP_FDB() : string = "reg0[11]"
>> function rEGBIT_HAIRPIN_REPLY(): string = "reg0[12]"
>> function rEGBIT_ACL_LABEL(): string = "reg0[13]"
>> +function rEGBIT_FROM_RAMP(): string = "reg0[14]"
>> 
>> function rEG_ORIG_DIP_IPV4()   : string = "reg1"
>> function rEG_ORIG_DIP_IPV6()   : string = "xxreg1"
>> @@ -2058,6 +2059,16 @@ for ((._uuid = ls_uuid, .has_stateful_acl = 
>> true)) {
>>  .io_port  = None,
>>  .controller_meter = None);
>> 
>> +/* Do not send coming from RAMP switch packets to conntrack. */
>> +Flow(.logical_datapath = ls_uuid,
>> + .stage= s_SWITCH_IN_PRE_ACL(),
>> + .priority = 110,
>> + .__match  = i"${rEGBIT_FROM_RAMP()} == 1",
>> + .actions  = i"next;",
>> + .stage_hint   = 0,
>> + .io_port  = None,
>> + .controller_meter = None);
>> +
>> /* Ingress and 

[ovs-dev] [PATCH ovn v2] northd: support HW VTEP with stateful datapath

[Vladislav Odintsov]

A packet going from HW VTEP device to VIF port when arrives to
hypervisor chassis should go through LS ingress pipeline to l2_lkp
stage without any match. In l2_lkp stage an output port is
determined and then packet passed to LS egress pipeline for futher
processing and to VIF port delivery.

Prior to this commit a packet, which was received from HW VTEP
device was dropped in an LS ingress datapath, where stateful services
were defined (ACLs, LBs).

To fix this issue we add a special flag-bit which can be used in LS
pipelines, to check whether the packet came from HW VTEP devices.
In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
to skip such packets.

Signed-off-by: Vladislav Odintsov 
---
v1 -> v2:
   - Patch rebased on upstream changes.

Please note: I've got no experience in DDLog and have no ability to extensively
 test these changes.
 Just local ./configure --with-ddlog=...; make; make check was run
 It seems, that only irrelevant to these changes tests were failed.
---
 northd/northd.c  | 14 ++
 northd/ovn_northd.dl | 33 +++--
 tests/ovn-northd.at  |  2 ++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 688a6e4ef..1b84874a7 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -196,6 +196,7 @@ enum ovn_stage {
 #define REGBIT_LKUP_FDB   "reg0[11]"
 #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
 #define REGBIT_ACL_LABEL  "reg0[13]"
+#define REGBIT_FROM_RAMP  "reg0[14]"
 
 #define REG_ORIG_DIP_IPV4 "reg1"
 #define REG_ORIG_DIP_IPV6 "xxreg1"
@@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
 if (queue_id) {
 ds_put_format(actions, "set_queue(%s); ", queue_id);
 }
+
+if (!strcmp(op->nbsp->type, "vtep")) {
+ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
+}
+
 ds_put_cstr(actions, "next;");
 ovn_lflow_add_with_lport_and_hint(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2,
   50, ds_cstr(match), ds_cstr(actions),
@@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
*port_groups,
   "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
   "(udp && udp.src == 546 && udp.dst == 547)", "next;");
 
+/* Do not send coming from RAMP switch packets to conntrack. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+  REGBIT_FROM_RAMP" == 1", "next;");
+
 /* Ingress and Egress Pre-ACL Table (Priority 100).
  *
  * Regardless of whether the ACL is "from-lport" or "to-lport",
@@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
*lflows,
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
   "eth.src == $svc_monitor_mac", "next;");
 
+/* Do not send coming from RAMP switch packets to conntrack. */
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
+  REGBIT_FROM_RAMP" == 1", "next;");
+
 /* Allow all packets to go to next tables by default. */
 ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
 ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 669728497..0202af5dc 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1631,6 +1631,7 @@ function rEGBIT_ACL_HINT_BLOCK()   : istring = i"reg0[10]"
 function rEGBIT_LKUP_FDB() : istring = i"reg0[11]"
 function rEGBIT_HAIRPIN_REPLY(): istring = i"reg0[12]"
 function rEGBIT_ACL_LABEL(): istring = i"reg0[13]"
+function rEGBIT_FROM_RAMP(): istring = i"reg0[14]"
 
 function rEG_ORIG_DIP_IPV4()   : istring = i"reg1"
 function rEG_ORIG_DIP_IPV6()   : istring = i"xxreg1"
@@ -2070,6 +2071,16 @@ for ((._uuid = ls_uuid, .has_stateful_acl = 
true)) {
  .io_port  = None,
  .controller_meter = None);
 
+/* Do not send coming from RAMP switch packets to conntrack. */
+Flow(.logical_datapath = ls_uuid,
+ .stage= s_SWITCH_IN_PRE_ACL(),
+ .priority = 110,
+ .__match  = i"${rEGBIT_FROM_RAMP()} == 1",
+ .actions  = i"next;",
+ .stage_hint   = 0,
+ .io_port  = None,
+ .controller_meter = None);
+
 /* Ingress and Egress Pre-ACL Table (Priority 100).
  *
  * Regardless of whether the ACL is "from-lport" or "to-lport",
@@ -2136,6 +2147,16 @@ for ((._uuid = ls_uuid)) {
  .io_port  = None,
  .controller_meter = None);
 
+/* Do not send coming from RAMP switch packets to conntrack. */
+Flow(.logical_datapath = ls_uuid,
+ .stage= s_SWITCH_IN_PRE_LB(),
+ .priority = 110,
+ .__match  = i"${rEGBIT_FROM_RAMP()} == 1",
+ .actions  = i"next;",
+ .stage_hint 

Re: [ovs-dev] [PATCH ovn 3/3] northd: support HW VTEP with stateful datapath

[Numan Siddique]

On Wed, Sep 15, 2021 at 8:07 PM Vladislav Odintsov  wrote:
>
> A packet going from HW VTEP device to VIF port when arrives to
> hypervisor chassis should go through LS ingress pipeline to l2_lkp
> stage without any match. In l2_lkp stage an output port is
> determined and then packet passed to LS egress pipeline for futher
> processing and to VIF port delivery.
>
> Prior to this commit a packet, which was received from HW VTEP
> device was dropped in an LS ingress datapath, where stateful services
> were defined (ACLs, LBs).
>
> To fix this issue we add a special flag-bit which can be used in LS
> pipelines, to check whether the packet came from HW VTEP devices.
> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
> to skip such packets.
>
> Signed-off-by: Vladislav Odintsov 

This needs a rebase.

Thanks
Numan

> ---
> Please note: I've got no experience in DDLog and have no ability to 
> extensively
>  test these changes.
>  Just local ./configure --with-ddlog=...; make; make check was run
>  It seems, that only irrelevant to these changes tests were 
> failed.
> ---
>  northd/ovn-northd.c  | 14 ++
>  northd/ovn_northd.dl | 33 +++--
>  tests/ovn-northd.at  |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 0ee2ba221..2a795d9e1 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -236,6 +236,7 @@ enum ovn_stage {
>  #define REGBIT_LKUP_FDB   "reg0[11]"
>  #define REGBIT_HAIRPIN_REPLY  "reg0[12]"
>  #define REGBIT_ACL_LABEL  "reg0[13]"
> +#define REGBIT_FROM_RAMP  "reg0[14]"
>
>  #define REG_ORIG_DIP_IPV4 "reg1"
>  #define REG_ORIG_DIP_IPV6 "xxreg1"
> @@ -5175,6 +5176,11 @@ build_lswitch_input_port_sec_op(
>  if (queue_id) {
>  ds_put_format(actions, "set_queue(%s); ", queue_id);
>  }
> +
> +if (!strcmp(op->nbsp->type, "vtep")) {
> +ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
> +}
> +
>  ds_put_cstr(actions, "next;");
>  ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
> S_SWITCH_IN_PORT_SEC_L2,
>50, ds_cstr(match), ds_cstr(actions),
> @@ -5422,6 +5428,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
> *port_groups,
>"nd || nd_rs || nd_ra || mldv1 || mldv2 || "
>"(udp && udp.src == 546 && udp.dst == 547)", "next;");
>
> +/* Do not send coming from RAMP switch packets to conntrack. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> +  REGBIT_FROM_RAMP" == 1", "next;");
> +
>  /* Ingress and Egress Pre-ACL Table (Priority 100).
>   *
>   * Regardless of whether the ACL is "from-lport" or "to-lport",
> @@ -5526,6 +5536,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
> *lflows,
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
>"eth.src == $svc_monitor_mac", "next;");
>
> +/* Do not send coming from RAMP switch packets to conntrack. */
> +ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> +  REGBIT_FROM_RAMP" == 1", "next;");
> +
>  /* Allow all packets to go to next tables by default. */
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index d91f8111f..5b4ae980a 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1622,6 +1622,7 @@ function rEGBIT_ACL_HINT_BLOCK()   : string = "reg0[10]"
>  function rEGBIT_LKUP_FDB() : string = "reg0[11]"
>  function rEGBIT_HAIRPIN_REPLY(): string = "reg0[12]"
>  function rEGBIT_ACL_LABEL(): string = "reg0[13]"
> +function rEGBIT_FROM_RAMP(): string = "reg0[14]"
>
>  function rEG_ORIG_DIP_IPV4()   : string = "reg1"
>  function rEG_ORIG_DIP_IPV6()   : string = "xxreg1"
> @@ -2058,6 +2059,16 @@ for ((._uuid = ls_uuid, .has_stateful_acl = 
> true)) {
>   .io_port  = None,
>   .controller_meter = None);
>
> +/* Do not send coming from RAMP switch packets to conntrack. */
> +Flow(.logical_datapath = ls_uuid,
> + .stage= s_SWITCH_IN_PRE_ACL(),
> + .priority = 110,
> + .__match  = i"${rEGBIT_FROM_RAMP()} == 1",
> + .actions  = i"next;",
> + .stage_hint   = 0,
> + .io_port  = None,
> + .controller_meter = None);
> +
>  /* Ingress and Egress Pre-ACL Table (Priority 100).
>   *
>   * Regardless of whether the ACL is "from-lport" or "to-lport",
> @@ -2124,6 +2135,16 @@ for ((._uuid = ls_uuid)) {
>   .io_port  = None,
>   .controller_meter = None);
>
> +/* Do not send coming from RAMP switch packets to 

Re: [ovs-dev] [PATCH ovn 2/3] tests: check that flow for ramp switch is added when PB is created

[Numan Siddique]

On Wed, Sep 15, 2021 at 8:07 PM Vladislav Odintsov  wrote:
>
> Signed-off-by: Vladislav Odintsov 

Thanks.   I applied the first 2 patches of this series to the main branch.
Patch 3 needs a rebase.

Numan

> ---
>  tests/ovn-controller-vtep.at | 64 
>  1 file changed, 64 insertions(+)
>
> diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> index 21d79c66b..2d1ebad56 100644
> --- a/tests/ovn-controller-vtep.at
> +++ b/tests/ovn-controller-vtep.at
> @@ -558,3 +558,67 @@ done | sort], [0], [dnl
>
>  OVN_CONTROLLER_VTEP_STOP
>  AT_CLEANUP
> +
> +
> +# Tests OF to vtep device on ovn-controller node.
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-controller-vtep - hv flows])
> +ovn_start
> +OVN_CONTROLLER_VTEP_START(vtep1)
> +net_add n1
> +
> +# Start hv chassis and create lswitch with normal vif attached to hv chassis
> +sim_add hv1
> +as hv1
> +ovs-vsctl -- add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +ovn-nbctl ls-add lsw0
> +ovn-nbctl lsp-add lsw0 lsp0
> +ovn-nbctl lsp-set-addresses lsp0 f0:00:00:00:00:01
> +ovs-vsctl add-port br-int vif0 -- set Interface vif0 
> external-ids:iface-id=lsp0
> +
> +
> +# 1st testcase: create vtep logical switch port and then bind vlan on vtep
> +OVN_NB_ADD_VTEP_PORT([lsw0], [lsp-vtep], [vtep1], [lswitch0])
> +
> +# ensure there is a port_binding without chassis set
> +wait_row_count Port_Binding 1 logical_port=lsp-vtep chassis='[[]]'
> +
> +# add vlan binding, ensure port_binding has chassis and OF on hv is installed
> +OVS_WAIT_WHILE([ovs-ofctl dump-flows br-int table=0 | grep 'priority=110'])
> +check as vtep1 vtep-ctl add-ls lswitch0 -- bind-ls vtep1 p0 100 lswitch0
> +wait_row_count Port_Binding 1 logical_port=lsp-vtep chassis!='[[]]'
> +OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int table=0 | grep 'priority=110'])
> +AT_CHECK([ovs-ofctl dump-flows br-int table=0 | grep 'priority=110' | \
> +  awk '{print $(NF-1), $NF}' | sed -e 
> 's/in_port=[[0-9]]\+/in_port=<>/g' | \
> +  sed -e 's/0x[[0-9a-f]]\+/0x<>/g'], [0], [dnl
> +priority=110,tun_id=0x<>,in_port=<> 
> actions=move:NXM_NX_TUN_ID[[0..23]]->OXM_OF_METADATA[[0..23]],load:0x<>->NXM_NX_REG14[[0..14]],load:0x<>->NXM_NX_REG10[[1]],resubmit(,8)
> +])
> +
> +# cleanup
> +check ovn-nbctl lsp-del lsp-vtep
> +check as vtep1 vtep-ctl unbind-ls vtep1 p0 100 -- clear-local-macs lswitch0 \
> +-- clear-remote-macs lswitch0 -- del-ls lswitch0
> +
> +
> +# 2nd testcase: create vlan binding on vtep and then create logical switch 
> port for it.
> +# ensure there's no port_binding
> +wait_row_count Port_Binding 0 logical_port=lsp-vtep
> +
> +check as vtep1 vtep-ctl add-ls lswitch0 -- bind-ls vtep1 p0 100 lswitch0
> +OVN_NB_ADD_VTEP_PORT([lsw0], [lsp-vtep], [vtep1], [lswitch0])
> +wait_row_count Port_Binding 1 logical_port=lsp-vtep chassis!='[[]]'
> +
> +# TODO (Numan): Remove this recompute to reproduce the issue
> +check as hv1 ovn-appctl -t ovn-controller recompute
> +
> +OVS_WAIT_UNTIL([ovs-ofctl dump-flows br-int table=0 | grep 'priority=110'])
> +AT_CHECK([ovs-ofctl dump-flows br-int table=0 | grep 'priority=110' | \
> +  awk '{print $(NF-1), $NF}' | sed -e 
> 's/in_port=[[0-9]]\+/in_port=<>/g' | \
> +  sed -e 's/0x[[0-9a-f]]\+/0x<>/g'], [0], [dnl
> +priority=110,tun_id=0x<>,in_port=<> 
> actions=move:NXM_NX_TUN_ID[[0..23]]->OXM_OF_METADATA[[0..23]],load:0x<>->NXM_NX_REG14[[0..14]],load:0x<>->NXM_NX_REG10[[1]],resubmit(,8)
> +])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +])
> --
> 2.30.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] controller: Fall back to full recompute of pflow_output for vtep lport changes.

[Numan Siddique]

On Wed, Sep 15, 2021 at 8:12 PM Odintsov Vladislav  wrote:
>
> Hi Numan,
>
> for me this patch solves the issue.
> Actually, it’s a good point about adding test for ovn-controller-vtep to 
> eliminate this problem in future.
> However there was another problem with HW VTEP support in OVN with using 
> stateful services in the datapath.
>
> I’ve sent a patch series with a test, which reproduces the mentioned problem. 
> Bigfix is in a third patch. Please try those patches:
> https://patchwork.ozlabs.org/project/ovn/cover/20210916000624.1609-1-odiv...@gmail.com/
>
> Tested-by: Vladislav Odintsov mailto:odiv...@gmail.com>>

Thanks for testing it out and adding the test case in your other series.

I applied this patch to the main branch.

Numan

>
> Regards,
> Vladislav Odintsov
>
> On 8 Sep 2021, at 20:14, Numan Siddique 
> mailto:num...@ovn.org>> wrote:
>
> On Wed, Sep 8, 2021 at 5:59 AM Mark Gray 
> mailto:mark.d.g...@redhat.com>> wrote:
>
> On 07/09/2021 18:15, num...@ovn.org wrote:
> From: Numan Siddique mailto:num...@ovn.org>>
>
> When a vtep logical port changes, necessary flows are not added as
> expected.  This is because the function physical_handle_flows_for_lport()
> in physical.c does not add flows required for vtep logical ports.
> These flows are added by physical_run(). So fall back to full recompute
> of pflow_output engine node when vtep lports change.
>
> Reported-by: Odintsov Vladislav 
> mailto:vlodint...@croc.ru>>
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387435.html
> Signed-off-by: Numan Siddique mailto:num...@ovn.org>>
> ---
> controller/ovn-controller.c | 11 ---
> controller/physical.c   |  9 -
> controller/physical.h   |  2 +-
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 0031a1035..d98ebbbfa 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2867,7 +2867,10 @@ pflow_output_sb_port_binding_handler(struct 
> engine_node *node,
> const struct sbrec_port_binding *pb;
> SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, p_ctx.port_binding_table) {
> bool removed = sbrec_port_binding_is_deleted(pb);
> -physical_handle_flows_for_lport(pb, removed, _ctx, 
> >flow_table);
> +if (!physical_handle_flows_for_lport(pb, removed, _ctx,
> + >flow_table)) {
> +return false;
> +}
> }
>
> engine_set_node_state(node, EN_UPDATED);
> @@ -2930,8 +2933,10 @@ pflow_output_runtime_data_handler(struct engine_node 
> *node, void *data)
> struct tracked_lport *lport = shash_node->data;
> bool removed =
> lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: false;
> -physical_handle_flows_for_lport(lport->pb, removed, _ctx,
> ->flow_table);
> +if (!physical_handle_flows_for_lport(lport->pb, removed, _ctx,
> + >flow_table)) {
> +return false;
> +}
> }
> }
>
> diff --git a/controller/physical.c b/controller/physical.c
> index 6f2c1cea0..ffb9f9952 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1509,11 +1509,16 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve,
> sset_destroy(_chassis);
> }
>
> -void
> +bool
> physical_handle_flows_for_lport(const struct sbrec_port_binding *pb,
> bool removed, struct physical_ctx *p_ctx,
> struct ovn_desired_flow_table *flow_table)
> {
> +if (!strcmp(pb->type, "vtep")) {
> +/* Cannot handle changes to vtep lports (yet). */
> +return false;
> +}
> +
> ofctrl_remove_flows(flow_table, >header_.uuid);
>
> if (!strcmp(pb->type, "external")) {
> @@ -1553,6 +1558,8 @@ physical_handle_flows_for_lport(const struct 
> sbrec_port_binding *pb,
>   p_ctx->chassis, flow_table, );
> ofpbuf_uninit();
> }
> +
> +return true;
> }
>
> void
> diff --git a/controller/physical.h b/controller/physical.h
> index c4540ad7f..ee4b1ae1f 100644
> --- a/controller/physical.h
> +++ b/controller/physical.h
> @@ -65,7 +65,7 @@ void physical_run(struct physical_ctx *,
>   struct ovn_desired_flow_table *);
> void physical_handle_mc_group_changes(struct physical_ctx *,
>   struct ovn_desired_flow_table *);
> -void physical_handle_flows_for_lport(const struct sbrec_port_binding *,
> +bool physical_handle_flows_for_lport(const struct sbrec_port_binding *,
>  bool removed,
>  struct physical_ctx *,
>  struct ovn_desired_flow_table *);
>
>
> The change looks ok to me. I am not really sure how to 

[ovs-dev] [PATCH ovn] rhel, utils: don't affect traffic on controller upgrade

[Vladislav Odintsov]

Currently upgrade of ovn-host rpm package affects active
traffic. This is because systemctl try-restart
ovn-controller is invoked during rpm package upgrade.
It calls ovn-ctl stop_controller and then start_controller.

Adding ovn-ctl stop_controller --restart to %postun
upgrade case right before systemctl try-restart. Also,
upgrade ovn-ctl script to support --restart argument in it.

Ideally this should be done by systemd when restart is
called, but it's impossible to pass restart command to
systemd.

Signed-off-by: Vladislav Odintsov 
---
 rhel/ovn-fedora.spec.in |  8 
 utilities/ovn-ctl   | 10 --
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
index 6716dd0d2..5fe0f990f 100644
--- a/rhel/ovn-fedora.spec.in
+++ b/rhel/ovn-fedora.spec.in
@@ -400,6 +400,14 @@ fi
 %endif
 
 %postun host
+if [ "$1" -ge "1" ] ; then
+# Package upgrade, not uninstall
+# We perform lightweight stop here not to affect active traffic during
+# ovn-controller upgrade.
+# Ideally this would be held by systemd, but it's impossible
+# to pass custom restart command to systemd service.
+%{_datadir}/ovn/scripts/ovn-ctl stop_controller --restart
+fi
 %if 0%{?systemd_postun_with_restart:1}
 %systemd_postun_with_restart ovn-controller.service
 %else
diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
index ff61f21d0..b30eb209d 100755
--- a/utilities/ovn-ctl
+++ b/utilities/ovn-ctl
@@ -584,7 +584,11 @@ stop_ic () {
 }
 
 stop_controller () {
-OVS_RUNDIR=${OVS_RUNDIR} stop_ovn_daemon ovn-controller "" "" "$@"
+set "ovn-controller" "" ""
+if test X"$RESTART" = Xyes; then
+set "$@" "--restart"
+fi
+OVS_RUNDIR=${OVS_RUNDIR} stop_ovn_daemon "$@"
 }
 
 stop_controller_vtep () {
@@ -606,7 +610,8 @@ restart_ic () {
 }
 
 restart_controller () {
-stop_controller --restart
+RESTART=yes
+stop_controller
 start_controller
 }
 
@@ -651,6 +656,7 @@ restart_ic_sb_ovsdb () {
 
 set_defaults () {
 OVN_MANAGE_OVSDB=yes
+RESTART=no
 
 OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
 OVN_RUNDIR=${OVN_RUNDIR:-${ovn_rundir}}
-- 
2.30.0

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

[ovs-dev] dpif-netdev.c : remove dpif OVS_UNUSED flag.

[lin huang]

From: linhuang 

The dpif is used, so remove the OVS_UNUSED flag.

Signed-off-by: linhuang 
---
 lib/dpif-netdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4cd0e9ebd..75f381ec1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -8429,7 +8429,7 @@ dpif_netdev_ct_get_tcp_seq_chk(struct dpif *dpif, bool 
*enabled)
 }

 static int
-dpif_netdev_ct_set_limits(struct dpif *dpif OVS_UNUSED,
+dpif_netdev_ct_set_limits(struct dpif *dpif,
const uint32_t *default_limits,
const struct ovs_list *zone_limits)
 {
@@ -8454,7 +8454,7 @@ dpif_netdev_ct_set_limits(struct dpif *dpif OVS_UNUSED,
 }

 static int
-dpif_netdev_ct_get_limits(struct dpif *dpif OVS_UNUSED,
+dpif_netdev_ct_get_limits(struct dpif *dpif,
uint32_t *default_limit,
const struct ovs_list *zone_limits_request,
struct ovs_list *zone_limits_reply)
@@ -8494,7 +8494,7 @@ dpif_netdev_ct_get_limits(struct dpif *dpif OVS_UNUSED,
 }

 static int
-dpif_netdev_ct_del_limits(struct dpif *dpif OVS_UNUSED,
+dpif_netdev_ct_del_limits(struct dpif *dpif,
const struct ovs_list *zone_limits)
 {
 int err = 0;
--
2.32.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 ovn] controller: add datapath meter capability check

[Mark Gray]

On 17/09/2021 11:12, Lorenzo Bianconi wrote:
> Dump datapath meter capabilities before configuring meters in
> ovn-controller
> 
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v4:
> - move rconn setup in ovs_feature_support_update and rename it in
>   ovs_feature_support_run
> - drop ovs_feature_support_init()
> - rename ovs_feature_support_deinit in ovs_feature_support_destroy
> Changes since v3:
> - add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
>   loop
> - cosmetics
> Changes since v2:
> - move meter capability logic in lib/features.c
> Changes since v1:
> - move rconn in ovn-controller to avoid concurrency issues
> ---
>  controller/ovn-controller.c |  7 +--
>  include/ovn/features.h  |  6 ++-
>  lib/actions.c   |  3 ++
>  lib/automake.mk |  1 +
>  lib/features.c  | 95 -
>  lib/test-ovn-features.c |  6 +--
>  tests/ovn.at|  4 +-
>  7 files changed, 112 insertions(+), 10 deletions(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 0031a1035..3347396f8 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -3569,9 +3569,9 @@ main(int argc, char *argv[])
>   * 'br_int_dp' is valid only if an OVS transaction is possible.
>   */
>  if (ovs_idl_txn
> -&& ovs_feature_support_update(br_int_dp
> -  ? _int_dp->capabilities
> -  : NULL)) {
> +&& ovs_feature_support_run(br_int_dp ?
> +   _int_dp->capabilities : NULL,
> +   br_int ? br_int->name : NULL)) {
>  VLOG_INFO("OVS feature set changed, force recompute.");
>  engine_set_force_recompute(true);
>  }
> @@ -3898,6 +3898,7 @@ loop_done:
>  ovsdb_idl_loop_destroy(_idl_loop);
>  ovsdb_idl_loop_destroy(_idl_loop);
>  
> +ovs_feature_support_destroy();
>  free(ovs_remote);
>  service_stop();
>  
> diff --git a/include/ovn/features.h b/include/ovn/features.h
> index c35d59b14..d12a8eb0d 100644
> --- a/include/ovn/features.h
> +++ b/include/ovn/features.h
> @@ -28,13 +28,17 @@
>   */
>  enum ovs_feature_support_bits {
>  OVS_CT_ZERO_SNAT_SUPPORT_BIT,
> +OVS_DP_METER_SUPPORT_BIT,
>  };
>  
>  enum ovs_feature_value {
>  OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
> +OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT),
>  };
>  
> +void ovs_feature_support_destroy(void);
>  bool ovs_feature_is_supported(enum ovs_feature_value feature);
> -bool ovs_feature_support_update(const struct smap *ovs_capabilities);
> +bool ovs_feature_support_run(const struct smap *ovs_capabilities,
> + const char *br_name);
>  
>  #endif
> diff --git a/lib/actions.c b/lib/actions.c
> index c572e88ae..7cf6be308 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool 
> pause,
>  oc->max_len = UINT16_MAX;
>  oc->reason = OFPR_ACTION;
>  oc->pause = pause;
> +if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
> +meter_id = NX_CTLR_NO_METER;
> +}
>  oc->meter_id = meter_id;
>  
>  struct action_header ah = { .opcode = htonl(opcode) };
> diff --git a/lib/automake.mk b/lib/automake.mk
> index ddfe33948..9f9f447d5 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
>  lib_libovn_la_LDFLAGS = \
>  $(OVS_LTINFO) \
>  -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
> +$(OVS_LIBDIR)/libopenvswitch.la \
>  $(AM_LDFLAGS)
>  lib_libovn_la_SOURCES = \
>   lib/acl-log.c \
> diff --git a/lib/features.c b/lib/features.c
> index fddf4c450..3ec2d26af 100644
> --- a/lib/features.c
> +++ b/lib/features.c
> @@ -18,7 +18,14 @@
>  #include 
>  
>  #include "lib/util.h"
> +#include "lib/dirs.h"
> +#include "socket-util.h"
> +#include "lib/vswitch-idl.h"
>  #include "openvswitch/vlog.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openvswitch/rconn.h"
> +#include "openvswitch/ofp-msgs.h"
> +#include "openvswitch/ofp-meter.h"
>  #include "ovn/features.h"
>  
>  VLOG_DEFINE_THIS_MODULE(features);
> @@ -40,11 +47,15 @@ static uint32_t supported_ovs_features;
>  
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
>  
> +/* ovs-vswitchd connection. */
> +static struct rconn *swconn;
> +
>  static bool
>  ovs_feature_is_valid(enum ovs_feature_value feature)
>  {
>  switch (feature) {
>  case OVS_CT_ZERO_SNAT_SUPPORT:
> +case OVS_DP_METER_SUPPORT:
>  return true;
>  default:
>  return false;
> @@ -58,9 +69,87 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
>  return supported_ovs_features & 

[ovs-dev] [PATCH v5 ovn] controller: add datapath meter capability check

[Lorenzo Bianconi]

Dump datapath meter capabilities before configuring meters in
ovn-controller

Signed-off-by: Lorenzo Bianconi 
---
Changes since v4:
- move rconn setup in ovs_feature_support_update and rename it in
  ovs_feature_support_run
- drop ovs_feature_support_init()
- rename ovs_feature_support_deinit in ovs_feature_support_destroy
Changes since v3:
- add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
  loop
- cosmetics
Changes since v2:
- move meter capability logic in lib/features.c
Changes since v1:
- move rconn in ovn-controller to avoid concurrency issues
---
 controller/ovn-controller.c |  7 +--
 include/ovn/features.h  |  6 ++-
 lib/actions.c   |  3 ++
 lib/automake.mk |  1 +
 lib/features.c  | 95 -
 lib/test-ovn-features.c |  6 +--
 tests/ovn.at|  4 +-
 7 files changed, 112 insertions(+), 10 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 0031a1035..3347396f8 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3569,9 +3569,9 @@ main(int argc, char *argv[])
  * 'br_int_dp' is valid only if an OVS transaction is possible.
  */
 if (ovs_idl_txn
-&& ovs_feature_support_update(br_int_dp
-  ? _int_dp->capabilities
-  : NULL)) {
+&& ovs_feature_support_run(br_int_dp ?
+   _int_dp->capabilities : NULL,
+   br_int ? br_int->name : NULL)) {
 VLOG_INFO("OVS feature set changed, force recompute.");
 engine_set_force_recompute(true);
 }
@@ -3898,6 +3898,7 @@ loop_done:
 ovsdb_idl_loop_destroy(_idl_loop);
 ovsdb_idl_loop_destroy(_idl_loop);
 
+ovs_feature_support_destroy();
 free(ovs_remote);
 service_stop();
 
diff --git a/include/ovn/features.h b/include/ovn/features.h
index c35d59b14..d12a8eb0d 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -28,13 +28,17 @@
  */
 enum ovs_feature_support_bits {
 OVS_CT_ZERO_SNAT_SUPPORT_BIT,
+OVS_DP_METER_SUPPORT_BIT,
 };
 
 enum ovs_feature_value {
 OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
+OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT),
 };
 
+void ovs_feature_support_destroy(void);
 bool ovs_feature_is_supported(enum ovs_feature_value feature);
-bool ovs_feature_support_update(const struct smap *ovs_capabilities);
+bool ovs_feature_support_run(const struct smap *ovs_capabilities,
+ const char *br_name);
 
 #endif
diff --git a/lib/actions.c b/lib/actions.c
index c572e88ae..7cf6be308 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool 
pause,
 oc->max_len = UINT16_MAX;
 oc->reason = OFPR_ACTION;
 oc->pause = pause;
+if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
+meter_id = NX_CTLR_NO_METER;
+}
 oc->meter_id = meter_id;
 
 struct action_header ah = { .opcode = htonl(opcode) };
diff --git a/lib/automake.mk b/lib/automake.mk
index ddfe33948..9f9f447d5 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
 lib_libovn_la_LDFLAGS = \
 $(OVS_LTINFO) \
 -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
+$(OVS_LIBDIR)/libopenvswitch.la \
 $(AM_LDFLAGS)
 lib_libovn_la_SOURCES = \
lib/acl-log.c \
diff --git a/lib/features.c b/lib/features.c
index fddf4c450..3ec2d26af 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -18,7 +18,14 @@
 #include 
 
 #include "lib/util.h"
+#include "lib/dirs.h"
+#include "socket-util.h"
+#include "lib/vswitch-idl.h"
 #include "openvswitch/vlog.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/rconn.h"
+#include "openvswitch/ofp-msgs.h"
+#include "openvswitch/ofp-meter.h"
 #include "ovn/features.h"
 
 VLOG_DEFINE_THIS_MODULE(features);
@@ -40,11 +47,15 @@ static uint32_t supported_ovs_features;
 
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
 
+/* ovs-vswitchd connection. */
+static struct rconn *swconn;
+
 static bool
 ovs_feature_is_valid(enum ovs_feature_value feature)
 {
 switch (feature) {
 case OVS_CT_ZERO_SNAT_SUPPORT:
+case OVS_DP_METER_SUPPORT:
 return true;
 default:
 return false;
@@ -58,9 +69,87 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
 return supported_ovs_features & feature;
 }
 
+static void
+ovs_feature_rconn_setup(const char *br_name)
+{
+if (!swconn) {
+swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
+}
+
+if (swconn && !rconn_is_connected(swconn)) {
+char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_name);
+if 

Re: [ovs-dev] [PATCH 0/4] dpif-netdev: rxq auto-lb improvements

[Kevin Traynor]

Hi Anurag,

On 17/09/2021 06:50, Anurag Agarwal wrote:
> Thanks Ilya for your email. Except for one patch, all the others are 
> superseded by the patchset posted by Kevin. 
> 
> Outstanding patch that should be applicable is the following one: 
> 
> [ovs-dev] [PATCH 4/4] dpif-netdev: Allow cross-NUMA polling on selected ports
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384547.html
> 
> This needs some rework to align with the patchset committed to master. 
> 
> I will get back to you on this. 
> 

Please also consider the comments related to this approach that Jan and
I discussed in the other thread. i.e. if the measured history can be a
good estimate in case of changing PMD NUMA during reassignment.

https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385695.html

> Regards,
> Anurag
> 
> -Original Message-
> From: Ilya Maximets  
> Sent: Thursday, September 16, 2021 2:53 AM
> To: anura...@gmail.com; d...@openvswitch.org
> Cc: Anurag Agarwal ; 
> rudrasury...@acldigital.com; Anju Thomas ; 
> i.maxim...@ovn.org; Kevin Traynor 
> Subject: Re: [ovs-dev] [PATCH 0/4] dpif-netdev: rxq auto-lb improvements
> 
> On 6/29/21 13:27, anura...@gmail.com wrote:
>> From: Anurag Agarwal 
>>
>> =   Disclaimer  ==
>> This patch set was prepared and verified downstream in early 2021. A 
>> very similar set of patches with auto load balance enhancements has 
>> recently been submitted by Red Hat:
>> https://protect2.fireeye.com/v1/url?k=0f466baf-50dd5293-0f462b34-86b1886cfa64-83b131dca3da994c=1=cfdc31f3-b150-4130-94a6-dc4488ef724a=https%3A%2F%2Fmail.openvswitch.org%2Fpipermail%2Fovs-dev%2F2021-June%2F383618.html.
>>  
>>
>> We acknowledge the large overlap between both patch sets and submit 
>> our set of patches to underline the importance of these improvements 
>> and offer our own implementation as an alternative proposal. We are 
>> very much open to discussing the best way forward in the OVS-DPDK 
>> community to merge all improvements together in one patch set.
>> ===
> 
> As the other patch set got accepted, this one is no longer applicable.
> If some changes from it are still relevant, please, rebase and send a new 
> version.
> 
> Best regards, Ilya Maximets.
> 

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

[ovs-dev] [PATCH 2/2] openvswitch: Fix condition check in output_userspace() by using nla_ok()

[Jiasheng Jiang]

Just using 'rem > 0' might be unsafe, so it's better
to use the nla_ok() instead.
Because we can see from the nla_next() that
'*remaining' might be smaller than 'totlen'. And nla_ok()
will avoid it happening.
For example, ovs_dp_process_packet() -> ovs_execute_actions()
-> do_execute_actions() -> output_userspace(), and attr comes
from OVS_CB(skb)->input_vport,which restores the received packet
from the user space.

Fixes: ccb1352e76cff0524e7ccb2074826a092dd13016
('net: Add Open vSwitch kernel components.')
Signed-off-by: Jiasheng Jiang 
---
 net/openvswitch/actions.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index c23537f..e8236dd 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -915,8 +915,7 @@ static int output_userspace(struct datapath *dp, struct 
sk_buff *skb,
upcall.cmd = OVS_PACKET_CMD_ACTION;
upcall.mru = OVS_CB(skb)->mru;
 
-   for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
-a = nla_next(a, )) {
+   nla_for_each_nested(a, attr, rem) {
switch (nla_type(a)) {
case OVS_USERSPACE_ATTR_USERDATA:
upcall.userdata = a;
-- 
2.7.4

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

Re: [ovs-dev] [PATCH v4 ovn] controller: add datapath meter capability check

[Mark Gray]

On 16/09/2021 23:16, Lorenzo Bianconi wrote:
> Dump datapath meter capabilities before configuring meters in
> ovn-controller

Thanks. Generally OK with this with one suggestion below. I'm not sure
if it will work but I think it follows the typical ovn/ovn pattern which
uses _run() functions in these loops.

> 
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v3:
> - add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
>   loop
> - cosmetics
> Changes since v2:
> - move meter capability logic in lib/features.c
> Changes since v1:
> - move rconn in ovn-controller to avoid concurrency issues
> ---
>  controller/ovn-controller.c |  4 ++
>  include/ovn/features.h  |  4 ++
>  lib/actions.c   |  3 ++
>  lib/automake.mk |  1 +
>  lib/features.c  | 90 +
>  tests/ovn.at|  4 +-
>  6 files changed, 104 insertions(+), 2 deletions(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 0031a1035..22cb4b956 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -3523,6 +3523,9 @@ main(int argc, char *argv[])
> ovsrec_server_has_datapath_table(ovs_idl_loop.idl)
> ? _int_dp
> : NULL);
> +if (br_int) {
> +ovs_feature_support_init(br_int->name);
> +}
>  
>  /* Enable ACL matching for double tagged traffic. */
>  if (ovs_idl_txn) {
> @@ -3898,6 +3901,7 @@ loop_done:
>  ovsdb_idl_loop_destroy(_idl_loop);
>  ovsdb_idl_loop_destroy(_idl_loop);
>  
> +ovs_feature_support_deinit();
>  free(ovs_remote);
>  service_stop();
>  
> diff --git a/include/ovn/features.h b/include/ovn/features.h
> index c35d59b14..5259f2c69 100644
> --- a/include/ovn/features.h
> +++ b/include/ovn/features.h
> @@ -28,12 +28,16 @@
>   */
>  enum ovs_feature_support_bits {
>  OVS_CT_ZERO_SNAT_SUPPORT_BIT,
> +OVS_DP_METER_SUPPORT_BIT,
>  };
>  
>  enum ovs_feature_value {
>  OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
> +OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT),
>  };
>  
> +void ovs_feature_support_init(const char *br_name);
> +void ovs_feature_support_deinit(void);
>  bool ovs_feature_is_supported(enum ovs_feature_value feature);
>  bool ovs_feature_support_update(const struct smap *ovs_capabilities);
>  
> diff --git a/lib/actions.c b/lib/actions.c
> index c572e88ae..7cf6be308 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool 
> pause,
>  oc->max_len = UINT16_MAX;
>  oc->reason = OFPR_ACTION;
>  oc->pause = pause;
> +if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
> +meter_id = NX_CTLR_NO_METER;
> +}
>  oc->meter_id = meter_id;
>  
>  struct action_header ah = { .opcode = htonl(opcode) };
> diff --git a/lib/automake.mk b/lib/automake.mk
> index ddfe33948..9f9f447d5 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
>  lib_libovn_la_LDFLAGS = \
>  $(OVS_LTINFO) \
>  -Wl,--version-script=$(top_builddir)/lib/libovn.sym \
> +$(OVS_LIBDIR)/libopenvswitch.la \
>  $(AM_LDFLAGS)
>  lib_libovn_la_SOURCES = \
>   lib/acl-log.c \
> diff --git a/lib/features.c b/lib/features.c
> index fddf4c450..decb4d24c 100644
> --- a/lib/features.c
> +++ b/lib/features.c
> @@ -18,7 +18,14 @@
>  #include 
>  
>  #include "lib/util.h"
> +#include "lib/dirs.h"
> +#include "socket-util.h"
> +#include "lib/vswitch-idl.h"
>  #include "openvswitch/vlog.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openvswitch/rconn.h"
> +#include "openvswitch/ofp-msgs.h"
> +#include "openvswitch/ofp-meter.h"
>  #include "ovn/features.h"
>  
>  VLOG_DEFINE_THIS_MODULE(features);
> @@ -40,11 +47,15 @@ static uint32_t supported_ovs_features;
>  
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
>  
> +/* ovs-vswitchd connection. */
> +static struct rconn *swconn;
> +
>  static bool
>  ovs_feature_is_valid(enum ovs_feature_value feature)
>  {
>  switch (feature) {
>  case OVS_CT_ZERO_SNAT_SUPPORT:
> +case OVS_DP_METER_SUPPORT:
>  return true;
>  default:
>  return false;
> @@ -58,6 +69,81 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
>  return supported_ovs_features & feature;
>  }
>  
> +static bool
> +ovs_feature_get_openflow_cap(void)
> +{
> +if (!swconn) {
> +return false;
> +}
> +
> +rconn_run(swconn);
> +if (!rconn_is_connected(swconn)) {
> +return false;
> +}
> +
> +bool ret = false;
> +/* dump datapath meter capabilities. */
> +struct ofpbuf *msg = ofpraw_alloc(OFPRAW_OFPST13_METER_FEATURES_REQUEST,
> +  rconn_get_version(swconn), 0);
> +rconn_send(swconn, msg, NULL);
> + 

[ovs-dev] [PATCH v2] openvswitch: Fix condition check in do_execute_actions() by using nla_ok()

[Jiasheng Jiang]

Just using 'rem > 0' might be unsafe, so it's better
to use the nla_ok() instead.
Because we can see from the nla_next() that
'*remaining' might be smaller than 'totlen'. And nla_ok()
will avoid it happening.
For example, ovs_dp_process_packet() -> ovs_execute_actions()
-> do_execute_actions(), and attr comes from OVS_CB(skb)->input_vport,
which restores the received packet from the user space.

Fixes: ccb1352e76cff0524e7ccb2074826a092dd13016
('net: Add Open vSwitch kernel components.')
Signed-off-by: Jiasheng Jiang 
---
 net/openvswitch/actions.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 77d924a..c23537f 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1238,8 +1238,7 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
const struct nlattr *a;
int rem;
 
-   for (a = attr, rem = len; rem > 0;
-a = nla_next(a, )) {
+   nla_for_each_attr(a, attr, len, rem) {
int err = 0;
 
switch (nla_type(a)) {
-- 
2.7.4

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

Re: [ovs-dev] [PATCH ovn v5 0/2] northd: Split northd

[Mark Gray]

On 16/09/2021 21:47, Numan Siddique wrote:
> On Thu, Sep 16, 2021 at 3:06 PM Han Zhou  wrote:
>>
>> Acked-by: Han Zhou 
> 
> Thanks Mark G (and Mark M and Han).  I applied both the patches to the
> main branch.

Thanks!
> 
> Numan
> 
>>
>> On Thu, Sep 16, 2021 at 8:49 AM Mark Gray  wrote:
>>
>>> These commits reorganise the northd code base in preparation for potential
>>> inclusion
>>> of I-P for northd.
>>>
>>> Please note that this mainly involves moving code around with minimal
>>> code changes. However, due to tight coupling between ovn-northd.c and
>>> northd.c,
>>> some minor changes were needed. For reference, and to help reviews, please
>>> examine the following at a minimum:
>>>
>>> * Configuration of the probe interval in northd.c
>>> (ovsdb_idl_set_probe_interval())
>>> * Passing of "use_parallel_build" and "lflow_locks" from ovn-northd.c and
>>>   northd.c.
>>> * Update of "struct northd_context": additon of fields and move to .h file.
>>>
>>> The commits were (hopefully) structured in a way to make the review
>>> easier. As
>>> this change touches all of ovn-northd, any change to "master" will make a
>>> rebase
>>> necessary and probably difficult. Therefore, if the general ideas is OK,
>>> then
>>> it would be great if this series could be expedited to prevent many
>>> rebases!
>>>
>>> Thanks
>>>
>>> ---
>>> v2: Rebase
>>> v3: Rebase. Fixed compile-time error
>>> v4: Add additional commits which add framework for incremental processing
>>> in northd
>>> v5: Remove additional IP commits (
>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387763.html)
>>> Rebase
>>>
>>> Mark Gray (2):
>>>   ovn-northd: Rename ovn-northd.c to northd.c
>>>   northd: Split northd.c
>>>
>>>  Documentation/tutorials/ovn-openstack.rst |   154 +-
>>>  northd/automake.mk| 2 +
>>>  northd/lrouter.dl | 2 +-
>>>  northd/northd.c   | 14579 +++
>>>  northd/northd.h   |42 +
>>>  northd/ovn-northd.c   | 14880 +---
>>>  northd/ovn.rs | 2 +-
>>>  northd/ovn_northd.dl  | 2 +-
>>>  tests/ovn-northd.at   | 2 +-
>>>  9 files changed, 14871 insertions(+), 14794 deletions(-)
>>>  create mode 100644 northd/northd.c
>>>  create mode 100644 northd/northd.h
>>>
>>> --
>>> 2.27.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