[ovs-dev] 答复: [PATCH] conntrack: check the result of extract_l3_ipv4/6

[Li,Rongqing]

发件人: Darrell Ball [mailto:dlu...@gmail.com]
发送时间: 2019年8月22日 9:00
收件人: Ben Pfaff 
抄送: Li,Rongqing ; ovs-dev@openvswitch.org
主题: Re: [ovs-dev] [PATCH] conntrack: check the result of extract_l3_ipv4/6



On Wed, Aug 21, 2019 at 3:13 PM Ben Pfaff mailto:b...@ovn.org>> 
wrote:
On Mon, Aug 19, 2019 at 08:35:11AM -0700, Darrell Ball wrote:
> Thanks for the patch
>
> On Sun, Aug 18, 2019 at 11:01 PM Li RongQing 
> mailto:lirongq...@baidu.com>> wrote:
>
> > the result of extract_l3_ipv4/6 should be checked in reverse_nat_packet
> > when it is false, meaning this packet is wrong, should not do handle it
> > continually
> >
> > Signed-off-by: Li RongQing 
> > mailto:lirongq...@baidu.com>>
> > ---
> >  lib/conntrack.c | 17 +++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 5f60fea18..c26d5438c 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -695,11 +695,18 @@ reverse_nat_packet(struct dp_packet *pkt, const
> > struct conn *conn)
> >  uint16_t orig_l4_ofs = pkt->l4_ofs;
> >
> >  if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> > +bool ok;
> >  struct ip_header *nh = dp_packet_l3(pkt);
> >  struct icmp_header *icmp = dp_packet_l4(pkt);
> >  struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1);
> > -extract_l3_ipv4(_key, inner_l3, tail - ((char *)inner_l3) -
> > pad,
> >
>
> There is intentionally no checking for success/fail here bcoz the packet
> has already
> been parsed and found to be ok during conn_key_extract() code path. Reusing
> the
> same api here is just convenient.

Maybe a comment would be warranted to make that clear.

that's reasonable; maybe RongQing would like to submit a patch ?


Ball:


I like you finish it, since you are more clear this codes.

Thanks

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

Re: [ovs-dev] [PATCH] conntrack: check the result of extract_l3_ipv4/6

[Darrell Ball]

On Wed, Aug 21, 2019 at 3:13 PM Ben Pfaff  wrote:

> On Mon, Aug 19, 2019 at 08:35:11AM -0700, Darrell Ball wrote:
> > Thanks for the patch
> >
> > On Sun, Aug 18, 2019 at 11:01 PM Li RongQing 
> wrote:
> >
> > > the result of extract_l3_ipv4/6 should be checked in reverse_nat_packet
> > > when it is false, meaning this packet is wrong, should not do handle it
> > > continually
> > >
> > > Signed-off-by: Li RongQing 
> > > ---
> > >  lib/conntrack.c | 17 +++--
> > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > > index 5f60fea18..c26d5438c 100644
> > > --- a/lib/conntrack.c
> > > +++ b/lib/conntrack.c
> > > @@ -695,11 +695,18 @@ reverse_nat_packet(struct dp_packet *pkt, const
> > > struct conn *conn)
> > >  uint16_t orig_l4_ofs = pkt->l4_ofs;
> > >
> > >  if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> > > +bool ok;
> > >  struct ip_header *nh = dp_packet_l3(pkt);
> > >  struct icmp_header *icmp = dp_packet_l4(pkt);
> > >  struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1);
> > > -extract_l3_ipv4(_key, inner_l3, tail - ((char
> *)inner_l3) -
> > > pad,
> > >
> >
> > There is intentionally no checking for success/fail here bcoz the packet
> > has already
> > been parsed and found to be ok during conn_key_extract() code path.
> Reusing
> > the
> > same api here is just convenient.
>
> Maybe a comment would be warranted to make that clear.
>

that's reasonable; maybe RongQing would like to submit a patch ?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] Docs: Add DPDK AF_XDP PMD use case.

[William Tu]

Add command lines for using DPDK's AF_XDP PMD driver.

Signed-off-by: William Tu 
---
 Documentation/intro/install/afxdp.rst | 36 +++
 1 file changed, 36 insertions(+)

diff --git a/Documentation/intro/install/afxdp.rst 
b/Documentation/intro/install/afxdp.rst
index 820e9d993d8f..c3e03b40f3b5 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -426,6 +426,42 @@ In the namespace, run drop or bounce back the packet::
   ip netns exec at_ns0 ./xdp_rxq_info --dev p0 --action XDP_TX
 
 
+Using DPDK's AF_XDP PMD driver
+--
+Another way to use AF_XDP is to enable DPDK and use DPDK's AF_XDP
+driver. First, enable the AF_XDP PMD config at DPDK's
+config/common_base file by::
+
+  CONFIG_RTE_LIBRTE_PMD_AF_XDP=y
+
+and recompile the DPDK source. Then, compile OVS with DPDK::
+
+  ./configure --with-dpdk=
+
+Start ovs-vswitchd and initilize DPDK::
+
+  ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true
+
+Finally, assume eth2 and eth3 are the physical NIC, AF_XDP port can
+be added by::
+
+  ovs-vsctl add-port br0 afxdp-p0 -- set int afxdp-p0 type=dpdk \
+options:dpdk-devargs=vdev:net_af_xdp0,iface=eth2 \
+start_queue=0,queue_count=1
+  ovs-vsctl add-port br0 afxdp-p1 -- set int afxdp-p1 type=dpdk \
+options:dpdk-devargs=vdev:net_af_xdp1,iface=eth3 \
+start_queue=0,queue_count=1
+
+Execute ovs-vsctl show::
+
+  Bridge "br0"
+datapath_type: netdev
+Port "afxdp-p0"
+  Interface "afxdp-p0"
+type: dpdk
+options: {dpdk-devargs="vdev:net_af_xdp,iface=eth2"}
+
+
 Bug Reporting
 -
 
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 0/3] be able to tune revalidator timing

[Ben Pfaff]

On Thu, Aug 15, 2019 at 10:52:53AM +, Roi Dayan wrote:
> 
> 
> On 2019-08-06 7:49 AM, Roi Dayan wrote:
> > 
> > 
> > On 2019-07-22 8:10 PM, Ben Pfaff wrote:
> >> On Sun, Jul 21, 2019 at 11:34:20AM +0300, Roi Dayan wrote:
> >>> The following series expose configuration options to
> >>> be able to tune revalidator timing to different setups
> >>> at runtime instead of using hard coded values.
> >>
> >> These seem fine.
> >>
> >> I'd like to wait until we've branched for 2.12 before applying them.
> >>
> > 
> > Hi Ben,
> > 
> > I see there is now a branch for 2.12.
> > can you take these commits now?
> > 
> > Thanks,
> > Roi
> > 
> 
> Hi,
> 
> just pinging here.

Finally got around to it.  Sorry about the delay.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 3/3] upcall: Configure datapath min-revalidate-pps through ovs-vsctl.

[Ben Pfaff]

I applied this series to master.  Thank you!

On Sun, Jul 21, 2019 at 11:34:23AM +0300, Roi Dayan wrote:
> From: Vlad Buslov 
> 
> This patch adds a new configuration option, "min-revalidate-pps" to the
> Open_vSwitch "other-config" column. This sets minimum pps that flow must
> have in order to be revalidated when revalidation duration exceeds half of
> max-revalidator config variable.
> 
> Signed-off-by: Vlad Buslov 
> Acked-by: Roi Dayan 
> ---
>  ofproto/ofproto-dpif-upcall.c |  4 ++--
>  ofproto/ofproto-provider.h|  4 
>  ofproto/ofproto.c |  9 +
>  ofproto/ofproto.h |  2 ++
>  vswitchd/bridge.c |  3 +++
>  vswitchd/vswitch.xml  | 11 +++
>  6 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 00c8e6ddcfaa..9e58a4dcf91b 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -2060,8 +2060,8 @@ should_revalidate(const struct udpif *udpif, uint64_t 
> packets,
>  duration = now - used;
>  metric = duration / packets;
>  
> -if (metric < 200) {
> -/* The flow is receiving more than ~5pps, so keep it. */
> +if (metric < 1000 / ofproto_min_revalidate_pps) {
> +/* The flow is receiving more than min-revalidate-pps, so keep it. */
>  return true;
>  }
>  return false;
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 92b384448f27..6cc454371dc7 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -528,6 +528,10 @@ extern unsigned ofproto_max_idle;
>   * Revalidator timeout is a minimum of max_idle and max_revalidator values. 
> */
>  extern unsigned ofproto_max_revalidator;
>  
> +/* Minimum pps that flow must have in order to be revalidated when 
> revalidation
> + * duration exceeds half of max-revalidator config variable. */
> +extern unsigned ofproto_min_revalidate_pps;
> +
>  /* Number of upcall handler and revalidator threads. Only affects the
>   * ofproto-dpif implementation. */
>  extern size_t n_handlers, n_revalidators;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 6f1d327ee87d..12758a3f7651 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -307,6 +307,7 @@ struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
>  unsigned ofproto_flow_limit = OFPROTO_FLOW_LIMIT_DEFAULT;
>  unsigned ofproto_max_idle = OFPROTO_MAX_IDLE_DEFAULT;
>  unsigned ofproto_max_revalidator = OFPROTO_MAX_REVALIDATOR_DEFAULT;
> +unsigned ofproto_min_revalidate_pps = OFPROTO_MIN_REVALIDATE_PPS_DEFAULT;
>  
>  size_t n_handlers, n_revalidators;
>  
> @@ -712,6 +713,14 @@ ofproto_set_max_revalidator(unsigned max_revalidator)
>  }
>  }
>  
> +/* Set minimum pps that flow must have in order to be revalidated when
> + * revalidation duration exceeds half of max-revalidator config variable. */
> +void
> +ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps)
> +{
> +ofproto_min_revalidate_pps = min_revalidate_pps ? min_revalidate_pps : 1;
> +}
> +
>  /* If forward_bpdu is true, the NORMAL action will forward frames with
>   * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is 
> false,
>   * the NORMAL action will drop these frames. */
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 6e17fd317fbc..1977bc2b5c1c 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -309,6 +309,7 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
>  #define OFPROTO_FLOW_LIMIT_DEFAULT 20
>  #define OFPROTO_MAX_IDLE_DEFAULT 1 /* ms */
>  #define OFPROTO_MAX_REVALIDATOR_DEFAULT 500 /* ms */
> +#define OFPROTO_MIN_REVALIDATE_PPS_DEFAULT 5
>  
>  const char *ofproto_port_open_type(const struct ofproto *,
> const char *port_type);
> @@ -337,6 +338,7 @@ void ofproto_set_bundle_idle_timeout(unsigned timeout);
>  void ofproto_set_flow_limit(unsigned limit);
>  void ofproto_set_max_idle(unsigned max_idle);
>  void ofproto_set_max_revalidator(unsigned max_revalidator);
> +void ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps);
>  void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
>  void ofproto_set_mac_table_config(struct ofproto *, unsigned idle_time,
>size_t max_entries);
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index be093af1d821..d921c4ef8d5f 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -604,6 +604,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch 
> *ovs_cfg)
>  ofproto_set_max_revalidator(smap_get_int(_cfg->other_config,
>   "max-revalidator",
>   
> OFPROTO_MAX_REVALIDATOR_DEFAULT));
> +ofproto_set_min_revalidate_pps(
> +smap_get_int(_cfg->other_config, "min-revalidate-pps",
> + 

Re: [ovs-dev] [PATCH] conntrack: check the result of extract_l3_ipv4/6

[Ben Pfaff]

On Mon, Aug 19, 2019 at 08:35:11AM -0700, Darrell Ball wrote:
> Thanks for the patch
> 
> On Sun, Aug 18, 2019 at 11:01 PM Li RongQing  wrote:
> 
> > the result of extract_l3_ipv4/6 should be checked in reverse_nat_packet
> > when it is false, meaning this packet is wrong, should not do handle it
> > continually
> >
> > Signed-off-by: Li RongQing 
> > ---
> >  lib/conntrack.c | 17 +++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 5f60fea18..c26d5438c 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -695,11 +695,18 @@ reverse_nat_packet(struct dp_packet *pkt, const
> > struct conn *conn)
> >  uint16_t orig_l4_ofs = pkt->l4_ofs;
> >
> >  if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> > +bool ok;
> >  struct ip_header *nh = dp_packet_l3(pkt);
> >  struct icmp_header *icmp = dp_packet_l4(pkt);
> >  struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1);
> > -extract_l3_ipv4(_key, inner_l3, tail - ((char *)inner_l3) -
> > pad,
> >
> 
> There is intentionally no checking for success/fail here bcoz the packet
> has already
> been parsed and found to be ok during conn_key_extract() code path. Reusing
> the
> same api here is just convenient.

Maybe a comment would be warranted to make that clear.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Cookie update on flow modify

[Ben Pfaff]

On Wed, Aug 21, 2019 at 07:17:26PM +, Karthik Chandrashekar wrote:
> From the latest OpenFlow spec we currently don't allow modification of cookie 
> value when we modify the flow (OFPFC_MODIFY or OFPFC_MODIFY_STRICT).
> 
> 
> 
> However OpenFlow 1.0 supported change of cookie while performing modify. Can 
> I know why this support for removed? Pointers to any previous design 
> discussions would help.

A single flow_cookie field in the flow_mod would not ordinarily be able
to usefully serve both the purpose of matching and updating flow cookies
at the same time.  I think that the ability to match on flow cookies was
considered more useful.  The possibility of having separate flow_mod
fields for matching and updating flow_cookie wasn't considered, if I
recall correctly.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 ovn 0/5] External OVS source support and separate run dir for OVN

[Han Zhou]

On Wed, Aug 21, 2019 at 11:54 AM Numan Siddique  wrote:
>
>
>
> On Wed, Aug 21, 2019 at 10:30 PM Han Zhou  wrote:
>>
>>
>>
>> On Mon, Aug 19, 2019 at 11:13 AM  wrote:
>> >
>> > From: Numan Siddique 
>> >
>> > This patch series adds support for building OVN from external OVS
>> > sources.
>> >
>> > The first patch adds the support to compile OVN from external OVS
sources.
>> > The following configuration options are added when configuring OVN
>> >   * --with-ovs-source (mandatory)
>> >   * --with-ovs-build (optional)
>> >
>> > Patch 2 adds support to run OVN services using separate
>> > directores
>> >   - Default run time dir - /usr/local/var/run/ovm
>> >   - Default log dir - /usr/loca/var/log/ovn
>> >   - Default db dir - /usr/loca/etc/ovn
>> >
>> > Patch 3 fixes "make rpm-fedora" which is presently broken
>> >
>> > Patch 4 runs OVN services as openvswitch user for rhel when rpms are
>> > used.
>> >
>> > Patch 5 removes the python subdirectory as that directory belongs
>> > to OVS and uses the required files from the OVS repo.
>> >
>> > v1 -> v2
>> > 
>> >  * Addressed the review comments.
>> >  * Swapped the patch 1 and 2 as it was easier to address Mark's comment
>> >on OVS_RUNDIR/OVN_RUNDIR
>> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few
of
>> >the macros to OVS_* to OVN_*.
>> >
>> > Combined the patch 1 and 2 in this series which were submitted
>> > separately earlier.
>> >
>> >
>>
>> Hi Numan,
>>
>> Thanks for this work. I tried applying this series on master, and then
removed the "ovs" subfolder just to see if it uses the external OVS
completely. However I encountered some error:
>>
>> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
-DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
/home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
/home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
/home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wshadow-g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
lib/logical-fields.lo lib/logical-fields.c &&\
>> mv -f $depbase.Tpo $depbase.Plo
>> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
>> lib/actions.c:2592:9: warning: implicit declaration of function
'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
>>  ofpact_put_CHECK_PKT_LARGER(ofpacts);
>>  ^
>> lib/actions.c:2592:9: warning: initialization makes pointer from integer
without a cast
>> lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
>>  check_pkt_larger->pkt_len = cipl->pkt_len;
>>  ^
>> lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
>>  check_pkt_larger->dst = expr_resolve_field(>dst);
>>  ^
>>
>> Is this expected?
>
>
> Hi Han,
>
> This is not expected. Before submitting I did test by removing the ovs
subdirectory. I tried even now and I don't see any issue.
> Looks to me either you have done "make install" in the ovs repo or your
 ovs sources are a bit old.
>
> Can you please check if that's the case.
>
> Thanks
> Numan
>
Thanks Numan. You are right that I was on an older branch of OVS. My bad.
Now OVN build is successful after correcting the branch and rebuilding OVS.

Just saw some annoying messages when doing make, but it seems no real
impact there.

comm: all-distfiles: No such file or directory
grep: all-distfiles: No such file or directory
fatal: ovs/lib/aes128.c: no such path in the working tree.
Use 'git  -- ...' to specify paths that do not exist locally.
fatal: ovs/include/linux/netfilter/nf_conntrack_sctp.h: no such path in the
working tree.
Use 'git  -- ...' to specify paths that do not exist locally.
grep: ovs/include/linux/netfilter/nf_conntrack_sctp.h: No such file or
directory
grep: ovs/include/linux/pkt_cls.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_pedit.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_skbedit.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_tunnel_key.h: No such file or directory
grep: ovs/include/linux/tc_act/tc_vlan.h: No such file or directory
...

It is great that the duplicated tests problem is solved for "make check",
but a noise still shows up during "make check":

ldd: /home/hzhou/src/ovn/vswitchd/ovs-vswitchd: No such file or directory


For make sandbox. It works well except that some man pages don't work. For
example:

man ovn-nb/ovn-sb
man ovn-architecture

Also, most OVS related man pages doesn't work anymore, such as man
ovs-vsctl, which may be expected. However, some related to ovsdb still
works, such as:

man 7 ovsdb-server
man 5 ovsdb
man 7 ovsdb

This is a little 

Re: [ovs-dev] [PATCH v3 2/5] ovsdb-idl.c: Allows retry even when using a single remote.

[Ben Pfaff]

On Wed, Aug 21, 2019 at 12:31:16PM -0700, Han Zhou wrote:
> On Wed, Aug 21, 2019 at 11:59 AM Ben Pfaff  wrote:
> >
> > On Wed, Aug 21, 2019 at 11:27:58AM -0700, Ben Pfaff wrote:
> > > On Wed, Aug 21, 2019 at 11:21:12AM -0700, Han Zhou wrote:
> > > > On Wed, Aug 21, 2019 at 11:13 AM Ben Pfaff  wrote:
> > > > >
> > > > > On Mon, Aug 19, 2019 at 09:29:57AM -0700, Han Zhou wrote:
> > > > > > From: Han Zhou 
> > > > > >
> > > > > > When clustered mode is used, the client needs to retry connecting
> > > > > > to new servers when certain failures happen. Today it is allowed
> to
> > > > > > retry new connection only if multiple remotes are used, which
> prevents
> > > > > > using LB VIP with clustered nodes. This patch makes sure the retry
> > > > > > logic works when using LB VIP: although same IP is used for
> retrying,
> > > > > > the LB can actually redirect the connection to a new node.
> > > > > >
> > > > > > Signed-off-by: Han Zhou 
> > > > > > ---
> > > > > > v2 -> v3: Minor fix for test case.
> > > > >
> > > > > I see that I commented on v1 of this patch by mistake, but the same
> > > > > comment applies here: please update the documentation to explain
> the new
> > > > > behavior.
> > > >
> > > > Hi Ben,
> > > >
> > > > Yes we agreed to update ovn-nb/sb documentation, but this patch
> belongs to
> > > > OVS repo and the documentation would belong to OVN repo. So we will
> send
> > > > update to documentation of ovn-nbctl/sbctl separately on OVN repo.
> > >
> > > OK.
> > >
> > > Do you want this patch backported to 2.12?
> >
> > I applied this series to master.  I haven't done any backports (besides
> > patch 1, which is obviously a bug fix).  Let me know if you'd like any
> > of the remaining patches backported.
> 
> Thanks Ben! Could you backport at least 1/5 - 4/5 to 2.12, so that the
> stale leader problem and the is_connected() flapping problem are solved?
> 5/5 is purely an enhancement, but I'd be happy if it is backported, too,
> since the feature is important to large scale use cases.

OK.  I backported the whole series to 2.12.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4 ovn] Containerize components

[Numan Siddique]

On Thu, Aug 22, 2019 at 1:35 AM  wrote:

> From: Aliasgar Ginwala 
>
> 1. Containerize ovn central components
> 2. Containerize ovn host
> 3. Update documentation about building/running ovn in containers.
>
> Signed-off-by: Aliasgar Ginwala 
>

Thanks.

Acked-by: Numan Siddique 

Numan


> ---
>  Documentation/intro/install/general.rst  | 83 
>  utilities/automake.mk| 10 ++-
>  utilities/docker/Makefile| 22 +++
>  utilities/docker/create_ovn_dbs.sh   | 18 +
>  utilities/docker/debian/Dockerfile   | 22 +++
>  utilities/docker/debian/build.sh | 44 +
>  utilities/docker/ovn_default_nb_port |  1 +
>  utilities/docker/ovn_default_northd_host |  1 +
>  utilities/docker/ovn_default_sb_port |  1 +
>  utilities/docker/start-ovn   | 40 
>  10 files changed, 241 insertions(+), 1 deletion(-)
>  create mode 100644 utilities/docker/Makefile
>  create mode 100755 utilities/docker/create_ovn_dbs.sh
>  create mode 100644 utilities/docker/debian/Dockerfile
>  create mode 100755 utilities/docker/debian/build.sh
>  create mode 100644 utilities/docker/ovn_default_nb_port
>  create mode 100644 utilities/docker/ovn_default_northd_host
>  create mode 100644 utilities/docker/ovn_default_sb_port
>  create mode 100755 utilities/docker/start-ovn
>
> diff --git a/Documentation/intro/install/general.rst
> b/Documentation/intro/install/general.rst
> index 99d8fec04..1d5323f76 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -380,6 +380,60 @@ domain socket::
>
>  $ ovn-northd --pidfile --detach --log-file
>
> +
> +Starting OVN Central services in containers
> +~~~
> +
> +For OVN central node, we dont need to load ovs kernel modules on host.
> +Hence, OVN central containers OS need not depend on host OS.
> +
> +Also we can leverage deploying entire OVN control plane in a pod spec for
> use
> +cases like OVN-kubernetes
> +
> +Export following variables in .env  and place it under
> +project root::
> +
> +$ OVN_BRANCH=
> +$ OVN_VERSION=
> +$ DISTRO=
> +$ KERNEL_VERSION=
> +$ GITHUB_SRC=
> +$ DOCKER_REPO=
> +
> +To build ovn modules::
> +
> +$ cd utilities/docker
> +$ make build
> +
> +Compiled Modules will be tagged with docker image
> +
> +To Push ovn modules::
> +
> +$ make push
> +
> +OVN docker image will be pushed to specified docker repo.
> +
> +Start OVN containers using below command::
> +
> +$ docker run -itd --net=host --name=ovn-nb \
> +  : ovn-nb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-sb \
> +  : ovn-sb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-northd \
> +  : ovn-northd-tcp
> +
> +.. note::
> +Current ovn central components comes up in docker image in a
> standalone
> +mode with protocol tcp.
> +
> +The debian docker file use ubuntu 16.04 as a base image for reference.
> +
> +User can use any other base image for debian, e.g. u14.04, etc.
> +
> +RHEL based docker build support needs to be added.
> +
>  Starting OVN host service
>  
>
> @@ -406,6 +460,32 @@ domain socket::
>
>  $ ovn-controller --pidfile --detach --log-file
>
> +Starting OVN host service in containers
> +~~~
> +
> +For OVN host too, we dont need to load ovs kernel modules on host.
> +Hence, OVN host container OS need not depend on host OS.
> +
> +Also we can leverage deploying OVN host in a pod spec for use cases like
> +OVN-kubernetes to manage OVS which can be running as a service on host or
> in
> +container.
> +
> +Start ovsdb-server and ovs-vswitchd components as per
> +http://docs.openvswitch.org/en/latest/intro/install/general/
> +
> +start local ovn-controller with below command if ovs is also running in
> +container::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  --volumes-from=ovsdb-server \
> +  : ovn-controller
> +
> +start local ovn-controller with below command if ovs is running as a
> service::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  -v /var/run/openvswitch/:/var/run/openvswitch/ \
> +  : ovn-controller
> +
>  Validating
>  --
>
> @@ -419,6 +499,9 @@ logical switch ``sw0`` and add logical port ``sw0-p1``
> ::
>
>  Refer to ovn-nbctl(8) and ovn-sbctl (8) for more details.
>
> +When using ovn in container, exec to container to run above commands::
> +
> +$ docker exec -it  /bin/bash
>
>  Reporting Bugs
>  --
> diff --git a/utilities/automake.mk b/utilities/automake.mk
> index b2b026f57..9b46940ae 100644
> --- a/utilities/automake.mk
> +++ b/utilities/automake.mk
> @@ -28,7 +28,15 @@ EXTRA_DIST += \
>  utilities/ovn-trace.8.xml \
>  utilities/ovn-detrace.in \
>  utilities/ovndb-servers.ocf \
> -utilities/checkpatch.py
> +

Re: [ovs-dev] [PATCH v3 ovn 3/3] OVN: northd: add rate limiting support for SB controller events

[Numan Siddique]

On Mon, Aug 19, 2019 at 8:09 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

> Introduce the capability to associate a meter to each controller event
> type in order to not overload the pinctrl thread under heavy load.
> Each event type relies on a meter with a defined name:
> - empty_lb_backends: event-elb
>
> Acked-by: Mark Michelson 
> Signed-off-by: Lorenzo Bianconi 
>

Hi Lorenzo,

Looks like the patch [1] which added the trigger_event flow in ovn-northd
missed
out updating the ovn-northd.8.xml.

And since this patch updates the logical flow a bit, can you please update
this patch
with documentation in ovn-northd.8.xml ?

[1] -
https://github.com/ovn-org/ovn/commit/f49b17a6cbe30ee05dc9743554c8ece92b16db48#diff-cc9e65b93a60efcce2e845453dcb56d3

Thanks
Numan

---
>  northd/ovn-northd.c | 44 +++-
>  ovn-nb.xml  |  8 
>  tests/ovn.at|  1 +
>  3 files changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e29b0fff4..27395d6bd 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4002,7 +4002,8 @@ ls_has_dns_records(const struct nbrec_logical_switch
> *nbs)
>  }
>
>  static void
> -build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
> +build_pre_lb(struct ovn_datapath *od, struct hmap *lflows,
> + struct shash *meter_groups)
>  {
>  /* Do not send ND packets to conntrack */
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> @@ -4040,7 +4041,11 @@ build_pre_lb(struct ovn_datapath *od, struct hmap
> *lflows)
>
>  if (controller_event_en && !node->value[0]) {
>  struct ds match = DS_EMPTY_INITIALIZER;
> -char *action;
> +char *meter = "", *action;
> +
> +if (shash_find(meter_groups, "event-elb")) {
> +meter = "event-elb";
> +}
>
>  if (addr_family == AF_INET) {
>  ds_put_format(, "ip4.dst == %s && %s",
> @@ -4054,10 +4059,11 @@ build_pre_lb(struct ovn_datapath *od, struct hmap
> *lflows)
>port);
>  }
>  action = xasprintf("trigger_event(event = \"%s\", "
> -   "vip = \"%s\", protocol = \"%s\", "
> +   "meter = \"%s\", vip = \"%s\", "
> +   "protocol = \"%s\", "
> "load_balancer = \"" UUID_FMT "\");",
>
> event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
> -   node->key, lb->protocol,
> +   meter, node->key, lb->protocol,
> UUID_ARGS(>header_.uuid));
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 120,
>ds_cstr(), action);
> @@ -4884,7 +4890,8 @@ build_lrouter_groups(struct hmap *ports, struct
> ovs_list *lr_list)
>  static void
>  build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
>  struct hmap *port_groups, struct hmap *lflows,
> -struct hmap *mcgroups, struct hmap *igmp_groups)
> +struct hmap *mcgroups, struct hmap *igmp_groups,
> +struct shash *meter_groups)
>  {
>  /* This flow table structure is documented in ovn-northd(8), so please
>   * update ovn-northd.8.xml if you change anything. */
> @@ -4901,7 +4908,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
> hmap *ports,
>  }
>
>  build_pre_acls(od, lflows);
> -build_pre_lb(od, lflows);
> +build_pre_lb(od, lflows, meter_groups);
>  build_pre_stateful(od, lflows);
>  build_acls(od, lflows, port_groups);
>  build_qos(od, lflows);
> @@ -8233,12 +8240,13 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>  static void
>  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>   struct hmap *ports, struct hmap *port_groups,
> - struct hmap *mcgroups, struct hmap *igmp_groups)
> + struct hmap *mcgroups, struct hmap *igmp_groups,
> + struct shash *meter_groups)
>  {
>  struct hmap lflows = HMAP_INITIALIZER();
>
>  build_lswitch_flows(datapaths, ports, port_groups, , mcgroups,
> -igmp_groups);
> +igmp_groups, meter_groups);
>  build_lrouter_flows(datapaths, ports, );
>
>  /* Push changes to the Logical_Flow table to database. */
> @@ -8906,6 +8914,16 @@ build_mcast_groups(struct northd_context *ctx,
>  }
>  }
>
> +static void
> +build_meter_groups(struct northd_context *ctx,
> +   struct shash *meter_groups)
> +{
> +const struct nbrec_meter *nb_meter;
> +NBREC_METER_FOR_EACH (nb_meter, ctx->ovnnb_idl) {
> +shash_add(meter_groups, nb_meter->name, nb_meter);
> +}
> +}
> +
>  static void
>  

[ovs-dev] [PATCH v1 ovn] OVN: Replace chassis mac with router port mac on destination chassis

[Ankur Sharma]

During E-W routing for vlan backed networks, we replace router port
mac with chassis mac, when packet leaves the source hypervisor.

As a result, the destination VM (on remote hypervisor) will see
chassis mac as source mac in received packet.

Although, functionality wise this does not cause any issue,
however chassis mac being see as source on VM, will
lead to following:
a. INCONSISTENT SOURCE MAC:
   If the destination VM moves to same hypervisor as sender,
   then it will see router port mac as source mac. Whereas, on
   a remote hypervisor, source mac will be the sender chassis mac.

   This will cause inconsistency in packet headers for the same
   flow and could be confusing for someone looking at packet
   captures inside the vm.

b. SYSTEM MAC BEING EXPOSED TO VM:
   Chassis mac is a CMS provided mac, i.e it is an infrastructure
   mac. It is not a good practice to expose such values to VM,
   which should not be seeing them in first place.

In order to replace chassis mac with router port mac, we will
do following.

a. Create conjunction for each chassis mac and router port vlan
   id combination. For example, for a 2 node chassis setup, where
   we have a logical router, connected to 4 logical switches with
   vlan ids: 2000, 1000, 0 and 24, the conjunction flow will look
   like following:

   cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ee:22 
actions=conjunction(100,1/2)
   cookie=0x0, duration=9094.608s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_src=aa:bb:cc:dd:ff:ee 
actions=conjunction(100,1/2)

   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=2000 actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=1000 actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,vlan_tci=0x/0x1fff actions=conjunction(100,2/2)
   cookie=0x0, duration=9094.552s, table=0, n_packets=0, n_bytes=0, 
idle_age=9094, priority=180,dl_vlan=24 actions=conjunction(100,2/2)

b. Using this conjunction as match, we can identify if packet entering 
destination
   hypervisor was routed at the source or not. This will be done in table=0 
(Physical to logical)
   at priority=180.
   For example:
   cookie=0x0, duration=9795.957s, table=0, n_packets=1391, n_bytes=141882, 
idle_age=8396, priority=180,conj_id=100,in_port=146,dl_vlan=1000 
actions=.,mod_dl_src:00:00:01:01:02:03,...

c. We use conjunction, as it will ensure that we do not end up having lot of 
flows
   as we scale up. If we do not use conjunction, then we will have
   N (number of chassis macs) X M (number of router vlans) number of ovs flows.
   Conjunction converts it to N + M.
   Consider a setup, with 500 Chassis and 500 routed vlans.
   Without conjunction we will need 25000 (500 * 500) flows,
   whereas with conjunction that number comes down to 1000 (500 + 500).

Signed-off-by: Ankur Sharma 
---
 controller/chassis.c|   2 +-
 controller/chassis.h|   3 +
 controller/ovn-controller.c |   5 +
 controller/physical.c   | 222 ++--
 controller/physical.h   |   1 +
 ovn-architecture.7.xml  |  10 +-
 tests/ovn.at|  12 ++-
 7 files changed, 242 insertions(+), 13 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 937c557..699b662 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -144,7 +144,7 @@ get_bridge_mappings(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-bridge-mappings", "");
 }
 
-static const char *
+const char *
 get_chassis_mac_mappings(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-chassis-mac-mappings", "");
diff --git a/controller/chassis.h b/controller/chassis.h
index eb46ca3..178d295 100644
--- a/controller/chassis.h
+++ b/controller/chassis.h
@@ -27,6 +27,7 @@ struct sbrec_chassis;
 struct sbrec_chassis_table;
 struct sset;
 struct eth_addr;
+struct smap;
 
 void chassis_register_ovs_idl(struct ovsdb_idl *);
 const struct sbrec_chassis *chassis_run(
@@ -42,5 +43,7 @@ bool chassis_get_mac(const struct sbrec_chassis *chassis,
  const char *bridge_mapping,
  struct eth_addr *chassis_mac);
 const char *chassis_get_id(void);
+const char * get_chassis_mac_mappings(const struct smap *ext_ids);
+
 
 #endif /* controller/chassis.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 86f29ac..2a4001b 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1268,9 +1268,14 @@ en_flow_output_run(struct engine_node *node)
 (struct sbrec_port_binding_table *)EN_OVSDB_GET(
 engine_get_input("SB_port_binding", node));
 
+struct sbrec_chassis_table *chassis_table =
+(struct 

Re: [ovs-dev] [PATCH v3 ovn 2/3] OVN: add meter support to trigger_event action

[Numan Siddique]

On Mon, Aug 19, 2019 at 8:09 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

> Introduce meter support to trigger_event action in order to not
> overload pinctrl thread under heavy load
>
> Signed-off-by: Lorenzo Bianconi 
>

Hi Lorenzo,

Can you please add a test for the meter option in the "action parsing" test
case in ovn.at.
I think this also requires updating the  documentation for the
'trigger_event' action in ovn-sb.xml.

Thanks
Numan

---
>  include/ovn/actions.h |  1 +
>  lib/actions.c | 43 +--
>  2 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 0ca06537c..145f27f25 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -327,6 +327,7 @@ struct ovnact_controller_event {
>  int event_type;   /* controller event type */
>  struct ovnact_gen_option *options;
>  size_t n_options;
> +char *meter;
>  };
>
>  /* OVNACT_BIND_VPORT. */
> diff --git a/lib/actions.c b/lib/actions.c
> index 08c589ab3..6a5907e1b 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1273,6 +1273,9 @@ format_TRIGGER_EVENT(const struct
> ovnact_controller_event *event,
>  {
>  ds_put_format(s, "trigger_event(event = \"%s\"",
>event_to_string(event->event_type));
> +if (event->meter) {
> +ds_put_format(s, ", meter = \"%s\"", event->meter);
> +}
>  for (const struct ovnact_gen_option *o = event->options;
>   o < >options[event->n_options]; o++) {
>  ds_put_cstr(s, ", ");
> @@ -1420,10 +1423,21 @@ encode_TRIGGER_EVENT(const struct
> ovnact_controller_event *event,
>   const struct ovnact_encode_params *ep OVS_UNUSED,
>   struct ofpbuf *ofpacts)
>  {
> +uint32_t meter_id = NX_CTLR_NO_METER;
>  size_t oc_offset;
>
> +if (event->meter) {
> +meter_id = ovn_extend_table_assign_id(ep->meter_table,
> event->meter,
> +  ep->lflow_uuid);
> +if (meter_id == EXT_TABLE_ID_INVALID) {
> +VLOG_WARN("Unable to assign id for trigger meter: %s",
> +  event->meter);
> +return;
> +}
> +}
> +
>  oc_offset = encode_start_controller_op(ACTION_OPCODE_EVENT, false,
> -   NX_CTLR_NO_METER, ofpacts);
> +   meter_id, ofpacts);
>  ovs_be32 ofs = htonl(event->event_type);
>  ofpbuf_put(ofpacts, , sizeof ofs);
>
> @@ -1738,11 +1752,27 @@ parse_trigger_event(struct action_context *ctx,
>   sizeof *event->options);
>  }
>
> -struct ovnact_gen_option *o = >options[event->n_options++];
> -memset(o, 0, sizeof *o);
> -parse_gen_opt(ctx, o,
> -
> >pp->controller_event_opts->event_opts[event_type],
> -  event_to_string(event_type));
> +if (lexer_match_id(ctx->lexer, "meter")) {
> +if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> +return;
> +}
> +/* If multiple meters are given, use the most recent. */
> +if (ctx->lexer->token.type == LEX_T_STRING &&
> +strlen(ctx->lexer->token.s)) {
> +free(event->meter);
> +event->meter = xstrdup(ctx->lexer->token.s);
> +} else if (ctx->lexer->token.type != LEX_T_STRING) {
> +lexer_syntax_error(ctx->lexer, "expecting string");
> +return;
> +}
> +lexer_get(ctx->lexer);
> +} else {
> +struct ovnact_gen_option *o =
> >options[event->n_options++];
> +memset(o, 0, sizeof *o);
> +parse_gen_opt(ctx, o,
> +
> >pp->controller_event_opts->event_opts[event_type],
> +event_to_string(event_type));
> +}
>  if (ctx->lexer->error) {
>  return;
>  }
> @@ -1763,6 +1793,7 @@ static void
>  ovnact_controller_event_free(struct ovnact_controller_event *event)
>  {
>  free_gen_options(event->options, event->n_options);
> +free(event->meter);
>  }
>
>  static void
> --
> 2.21.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 v3 ovn 1/3] OVN: Repair memory leak for OVN controller events.

[Numan Siddique]

On Mon, Aug 19, 2019 at 8:08 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:

> From: Mark Michelson 
>
> From: Mark Michelson 
>
> Controller event action is leaking its genopts. This corrects the error.
>
> Signed-off-by: Mark Michelson 
> Signed-off-by: Lorenzo Bianconi 
>

I applied this patch of this series to master.

I edited the commit message as there was "From" tag at the start of the
commit body.

Thanks
Numan



> ---
>  lib/actions.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/actions.c b/lib/actions.c
> index 81950e7df..08c589ab3 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1760,8 +1760,9 @@ parse_trigger_event(struct action_context *ctx,
>  }
>
>  static void
> -ovnact_controller_event_free(struct ovnact_controller_event *event
> OVS_UNUSED)
> +ovnact_controller_event_free(struct ovnact_controller_event *event)
>  {
> +free_gen_options(event->options, event->n_options);
>  }
>
>  static void
> --
> 2.21.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 v3 ovn] Containerize components

[Ginwala, Aliasgar via dev]

Ah I see, the issue is in syntax of automake.mk due to rebase with 
checkpatch.py:

diff --git a/utilities/automake.mk b/utilities/automake.mk
index 3142d177f..9b46940ae 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -28,7 +28,7 @@ EXTRA_DIST += \
 utilities/ovn-trace.8.xml \
 utilities/ovn-detrace.in \
 utilities/ovndb-servers.ocf \
-utilities/checkpatch.py
+utilities/checkpatch.py \
 utilities/docker/Makefile \
 utilities/docker/start-ovn \
 utilities/docker/create_ovn_dbs.sh \


I just tried rebuilding newest docker image with this fix and it built fine:
LM-SJC-11015761:ovn aginwala$ export OVN_BRANCH=ovn_container
LM-SJC-11015761:ovn aginwala$ export OVN_VERSION=2.12
LM-SJC-11015761:ovn aginwala$ export KERNEL_VERSION=4.15.0-54-generic
LM-SJC-11015761:ovn aginwala$ export DISTRO=debian
LM-SJC-11015761:ovn aginwala$ export 
GITHUB_SRC=https://github.com/noah8713/ovn.git
LM-SJC-11015761:ovn aginwala$ export DOCKER_REPO=aginwala/ovn
LM-SJC-11015761:ovn aginwala$ cd utilities/
LM-SJC-11015761:utilities aginwala$ cd docker/
LM-SJC-11015761:docker aginwala$ make build


Sent V4 to address the same.

Regards,
Aliasgar

From: Numan Siddique 
Date: Wednesday, August 21, 2019 at 12:11 PM
To: aginwala 
Cc: ovs dev , "Ginwala, Aliasgar" 
Subject: Re: [ovs-dev] [PATCH v3 ovn] Containerize components



On Wed, Aug 21, 2019 at 10:38 PM 
mailto:amgin...@gmail.com>> wrote:
From: Aliasgar Ginwala mailto:aginw...@ebay.com>>

1. Containerize ovn central components
2. Containerize ovn host
3. Update documentation about building/running ovn in containers.

Signed-off-by: Aliasgar Ginwala mailto:aginw...@ebay.com>>

Hi Aliasgar,

There's something odd with this patch. I am not able to compile when I apply 
this patch. I get the below error

**
cd . && /bin/sh /home/nusiddiq/workspace_cpp/ovn-org/ovn/build-aux/missing 
automake-1.16 --foreign Makefile
 cd . && /bin/sh ./config.status Makefile depfiles
config.status: creating Makefile
config.status: executing depfiles commands
config.status: error: in `/home/nusiddiq/workspace_cpp/ovn-org/ovn':
config.status: error: Something went wrong bootstrapping makefile fragments
for automatic dependency tracking.  Try re-running configure with the
'--disable-dependency-tracking' option to at least be able to build
the package (albeit without support for automatic dependency tracking).
See `config.log' for more details
make: *** [Makefile:1763: Makefile] Error 1



Thanks
Numan

---
 Documentation/intro/install/general.rst  | 83 
 
utilities/automake.mk
|  8 +++
 utilities/docker/Makefile| 22 +++
 utilities/docker/create_ovn_dbs.sh   | 18 +
 utilities/docker/debian/Dockerfile   | 22 +++
 utilities/docker/debian/build.sh | 44 +
 utilities/docker/ovn_default_nb_port |  1 +
 utilities/docker/ovn_default_northd_host |  1 +
 utilities/docker/ovn_default_sb_port |  1 +
 utilities/docker/start-ovn   | 40 
 10 files changed, 240 insertions(+)
 create mode 100644 utilities/docker/Makefile
 create mode 100755 utilities/docker/create_ovn_dbs.sh
 create mode 100644 utilities/docker/debian/Dockerfile
 create mode 100755 utilities/docker/debian/build.sh
 create mode 100644 utilities/docker/ovn_default_nb_port
 create mode 100644 utilities/docker/ovn_default_northd_host
 create mode 100644 utilities/docker/ovn_default_sb_port
 create mode 100755 utilities/docker/start-ovn

diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index 99d8fec04..1d5323f76 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -380,6 +380,60 @@ domain socket::

 $ ovn-northd --pidfile --detach --log-file

+
+Starting OVN Central services in containers
+~~~
+
+For OVN central node, we dont need to load ovs kernel modules on host.
+Hence, OVN central containers OS need not depend on host OS.
+
+Also we can leverage deploying entire OVN control plane in a pod spec for use
+cases like OVN-kubernetes
+
+Export following variables in .env  and place it under
+project root::
+
+$ OVN_BRANCH=
+$ OVN_VERSION=
+$ DISTRO=
+$ KERNEL_VERSION=
+$ GITHUB_SRC=
+$ DOCKER_REPO=
+
+To build ovn modules::
+
+$ cd utilities/docker
+$ make build
+
+Compiled Modules will be tagged with docker image
+
+To Push ovn modules::
+
+$ make push
+
+OVN docker image will be pushed to specified docker repo.
+
+Start OVN containers using below command::
+
+$ docker run -itd --net=host --name=ovn-nb \
+

[ovs-dev] [PATCH v4 ovn] Containerize components

[amginwal]

From: Aliasgar Ginwala 

1. Containerize ovn central components
2. Containerize ovn host
3. Update documentation about building/running ovn in containers.

Signed-off-by: Aliasgar Ginwala 
---
 Documentation/intro/install/general.rst  | 83 
 utilities/automake.mk| 10 ++-
 utilities/docker/Makefile| 22 +++
 utilities/docker/create_ovn_dbs.sh   | 18 +
 utilities/docker/debian/Dockerfile   | 22 +++
 utilities/docker/debian/build.sh | 44 +
 utilities/docker/ovn_default_nb_port |  1 +
 utilities/docker/ovn_default_northd_host |  1 +
 utilities/docker/ovn_default_sb_port |  1 +
 utilities/docker/start-ovn   | 40 
 10 files changed, 241 insertions(+), 1 deletion(-)
 create mode 100644 utilities/docker/Makefile
 create mode 100755 utilities/docker/create_ovn_dbs.sh
 create mode 100644 utilities/docker/debian/Dockerfile
 create mode 100755 utilities/docker/debian/build.sh
 create mode 100644 utilities/docker/ovn_default_nb_port
 create mode 100644 utilities/docker/ovn_default_northd_host
 create mode 100644 utilities/docker/ovn_default_sb_port
 create mode 100755 utilities/docker/start-ovn

diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index 99d8fec04..1d5323f76 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -380,6 +380,60 @@ domain socket::
 
 $ ovn-northd --pidfile --detach --log-file
 
+
+Starting OVN Central services in containers
+~~~
+
+For OVN central node, we dont need to load ovs kernel modules on host.
+Hence, OVN central containers OS need not depend on host OS.
+
+Also we can leverage deploying entire OVN control plane in a pod spec for use
+cases like OVN-kubernetes
+
+Export following variables in .env  and place it under
+project root::
+
+$ OVN_BRANCH=
+$ OVN_VERSION=
+$ DISTRO=
+$ KERNEL_VERSION=
+$ GITHUB_SRC=
+$ DOCKER_REPO=
+
+To build ovn modules::
+
+$ cd utilities/docker
+$ make build
+
+Compiled Modules will be tagged with docker image
+
+To Push ovn modules::
+
+$ make push
+
+OVN docker image will be pushed to specified docker repo.
+
+Start OVN containers using below command::
+
+$ docker run -itd --net=host --name=ovn-nb \
+  : ovn-nb-tcp
+
+$ docker run -itd --net=host --name=ovn-sb \
+  : ovn-sb-tcp
+
+$ docker run -itd --net=host --name=ovn-northd \
+  : ovn-northd-tcp
+
+.. note::
+Current ovn central components comes up in docker image in a standalone
+mode with protocol tcp.
+
+The debian docker file use ubuntu 16.04 as a base image for reference.
+
+User can use any other base image for debian, e.g. u14.04, etc.
+
+RHEL based docker build support needs to be added.
+
 Starting OVN host service
 
 
@@ -406,6 +460,32 @@ domain socket::
 
 $ ovn-controller --pidfile --detach --log-file
 
+Starting OVN host service in containers
+~~~
+
+For OVN host too, we dont need to load ovs kernel modules on host.
+Hence, OVN host container OS need not depend on host OS.
+
+Also we can leverage deploying OVN host in a pod spec for use cases like
+OVN-kubernetes to manage OVS which can be running as a service on host or in
+container.
+
+Start ovsdb-server and ovs-vswitchd components as per
+http://docs.openvswitch.org/en/latest/intro/install/general/
+
+start local ovn-controller with below command if ovs is also running in
+container::
+
+$ docker run -itd --net=host --name=ovn-controller \
+  --volumes-from=ovsdb-server \
+  : ovn-controller
+
+start local ovn-controller with below command if ovs is running as a service::
+
+$ docker run -itd --net=host --name=ovn-controller \
+  -v /var/run/openvswitch/:/var/run/openvswitch/ \
+  : ovn-controller
+
 Validating
 --
 
@@ -419,6 +499,9 @@ logical switch ``sw0`` and add logical port ``sw0-p1`` ::
 
 Refer to ovn-nbctl(8) and ovn-sbctl (8) for more details.
 
+When using ovn in container, exec to container to run above commands::
+
+$ docker exec -it  /bin/bash
 
 Reporting Bugs
 --
diff --git a/utilities/automake.mk b/utilities/automake.mk
index b2b026f57..9b46940ae 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -28,7 +28,15 @@ EXTRA_DIST += \
 utilities/ovn-trace.8.xml \
 utilities/ovn-detrace.in \
 utilities/ovndb-servers.ocf \
-utilities/checkpatch.py
+utilities/checkpatch.py \
+utilities/docker/Makefile \
+utilities/docker/start-ovn \
+utilities/docker/create_ovn_dbs.sh \
+utilities/docker/ovn_default_nb_port \
+utilities/docker/ovn_default_sb_port \
+utilities/docker/ovn_default_northd_host \
+utilities/docker/debian/Dockerfile \
+utilities/docker/debian/build.sh
 
 CLEANFILES += \
 

Re: [ovs-dev] [PATCH v3 2/5] ovsdb-idl.c: Allows retry even when using a single remote.

[Han Zhou]

On Wed, Aug 21, 2019 at 11:59 AM Ben Pfaff  wrote:
>
> On Wed, Aug 21, 2019 at 11:27:58AM -0700, Ben Pfaff wrote:
> > On Wed, Aug 21, 2019 at 11:21:12AM -0700, Han Zhou wrote:
> > > On Wed, Aug 21, 2019 at 11:13 AM Ben Pfaff  wrote:
> > > >
> > > > On Mon, Aug 19, 2019 at 09:29:57AM -0700, Han Zhou wrote:
> > > > > From: Han Zhou 
> > > > >
> > > > > When clustered mode is used, the client needs to retry connecting
> > > > > to new servers when certain failures happen. Today it is allowed
to
> > > > > retry new connection only if multiple remotes are used, which
prevents
> > > > > using LB VIP with clustered nodes. This patch makes sure the retry
> > > > > logic works when using LB VIP: although same IP is used for
retrying,
> > > > > the LB can actually redirect the connection to a new node.
> > > > >
> > > > > Signed-off-by: Han Zhou 
> > > > > ---
> > > > > v2 -> v3: Minor fix for test case.
> > > >
> > > > I see that I commented on v1 of this patch by mistake, but the same
> > > > comment applies here: please update the documentation to explain
the new
> > > > behavior.
> > >
> > > Hi Ben,
> > >
> > > Yes we agreed to update ovn-nb/sb documentation, but this patch
belongs to
> > > OVS repo and the documentation would belong to OVN repo. So we will
send
> > > update to documentation of ovn-nbctl/sbctl separately on OVN repo.
> >
> > OK.
> >
> > Do you want this patch backported to 2.12?
>
> I applied this series to master.  I haven't done any backports (besides
> patch 1, which is obviously a bug fix).  Let me know if you'd like any
> of the remaining patches backported.

Thanks Ben! Could you backport at least 1/5 - 4/5 to 2.12, so that the
stale leader problem and the is_connected() flapping problem are solved?
5/5 is purely an enhancement, but I'd be happy if it is backported, too,
since the feature is important to large scale use cases.

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

Re: [ovs-dev] [PATCH v3 ovn] Containerize components

[Aaron Conole]

amgin...@gmail.com writes:

> From: Aliasgar Ginwala 
>
> 1. Containerize ovn central components
> 2. Containerize ovn host
> 3. Update documentation about building/running ovn in containers.
>
> Signed-off-by: Aliasgar Ginwala 
> ---

Hi Aliasgar,

It seems this patch has some issues with the Makefile syntax.

See:

https://travis-ci.com/ovsrobot/ovn/builds/124110711

>  Documentation/intro/install/general.rst  | 83 
>  utilities/automake.mk|  8 +++
>  utilities/docker/Makefile| 22 +++
>  utilities/docker/create_ovn_dbs.sh   | 18 +
>  utilities/docker/debian/Dockerfile   | 22 +++
>  utilities/docker/debian/build.sh | 44 +
>  utilities/docker/ovn_default_nb_port |  1 +
>  utilities/docker/ovn_default_northd_host |  1 +
>  utilities/docker/ovn_default_sb_port |  1 +
>  utilities/docker/start-ovn   | 40 
>  10 files changed, 240 insertions(+)
>  create mode 100644 utilities/docker/Makefile
>  create mode 100755 utilities/docker/create_ovn_dbs.sh
>  create mode 100644 utilities/docker/debian/Dockerfile
>  create mode 100755 utilities/docker/debian/build.sh
>  create mode 100644 utilities/docker/ovn_default_nb_port
>  create mode 100644 utilities/docker/ovn_default_northd_host
>  create mode 100644 utilities/docker/ovn_default_sb_port
>  create mode 100755 utilities/docker/start-ovn
>
> diff --git a/Documentation/intro/install/general.rst 
> b/Documentation/intro/install/general.rst
> index 99d8fec04..1d5323f76 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -380,6 +380,60 @@ domain socket::
>  
>  $ ovn-northd --pidfile --detach --log-file
>  
> +
> +Starting OVN Central services in containers
> +~~~
> +
> +For OVN central node, we dont need to load ovs kernel modules on host.
> +Hence, OVN central containers OS need not depend on host OS.
> +
> +Also we can leverage deploying entire OVN control plane in a pod spec for use
> +cases like OVN-kubernetes
> +
> +Export following variables in .env  and place it under
> +project root::
> +
> +$ OVN_BRANCH=
> +$ OVN_VERSION=
> +$ DISTRO=
> +$ KERNEL_VERSION=
> +$ GITHUB_SRC=
> +$ DOCKER_REPO=
> +
> +To build ovn modules::
> +
> +$ cd utilities/docker
> +$ make build
> +
> +Compiled Modules will be tagged with docker image
> +
> +To Push ovn modules::
> +
> +$ make push
> +
> +OVN docker image will be pushed to specified docker repo.
> +
> +Start OVN containers using below command::
> +
> +$ docker run -itd --net=host --name=ovn-nb \
> +  : ovn-nb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-sb \
> +  : ovn-sb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-northd \
> +  : ovn-northd-tcp
> +
> +.. note::
> +Current ovn central components comes up in docker image in a standalone
> +mode with protocol tcp.
> +
> +The debian docker file use ubuntu 16.04 as a base image for reference.
> +
> +User can use any other base image for debian, e.g. u14.04, etc.
> +
> +RHEL based docker build support needs to be added.
> +
>  Starting OVN host service
>  
>  
> @@ -406,6 +460,32 @@ domain socket::
>  
>  $ ovn-controller --pidfile --detach --log-file
>  
> +Starting OVN host service in containers
> +~~~
> +
> +For OVN host too, we dont need to load ovs kernel modules on host.
> +Hence, OVN host container OS need not depend on host OS.
> +
> +Also we can leverage deploying OVN host in a pod spec for use cases like
> +OVN-kubernetes to manage OVS which can be running as a service on host or in
> +container.
> +
> +Start ovsdb-server and ovs-vswitchd components as per
> +http://docs.openvswitch.org/en/latest/intro/install/general/
> +
> +start local ovn-controller with below command if ovs is also running in
> +container::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  --volumes-from=ovsdb-server \
> +  : ovn-controller
> +
> +start local ovn-controller with below command if ovs is running as a 
> service::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  -v /var/run/openvswitch/:/var/run/openvswitch/ \
> +  : ovn-controller
> +
>  Validating
>  --
>  
> @@ -419,6 +499,9 @@ logical switch ``sw0`` and add logical port ``sw0-p1`` ::
>  
>  Refer to ovn-nbctl(8) and ovn-sbctl (8) for more details.
>  
> +When using ovn in container, exec to container to run above commands::
> +
> +$ docker exec -it  /bin/bash
>  
>  Reporting Bugs
>  --
> diff --git a/utilities/automake.mk b/utilities/automake.mk
> index b2b026f57..3142d177f 100644
> --- a/utilities/automake.mk
> +++ b/utilities/automake.mk
> @@ -29,6 +29,14 @@ EXTRA_DIST += \
>  utilities/ovn-detrace.in \
>  utilities/ovndb-servers.ocf \
>  

[ovs-dev] Cookie update on flow modify

[Karthik Chandrashekar]

>From the latest OpenFlow spec we currently don't allow modification of cookie 
>value when we modify the flow (OFPFC_MODIFY or OFPFC_MODIFY_STRICT).



However OpenFlow 1.0 supported change of cookie while performing modify. Can I 
know why this support for removed? Pointers to any previous design discussions 
would help.



Thanks

Karthik C

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

Re: [ovs-dev] [PATCH v3 ovn] Containerize components

[Numan Siddique]

On Wed, Aug 21, 2019 at 10:38 PM  wrote:

> From: Aliasgar Ginwala 
>
> 1. Containerize ovn central components
> 2. Containerize ovn host
> 3. Update documentation about building/running ovn in containers.
>
> Signed-off-by: Aliasgar Ginwala 
>

Hi Aliasgar,

There's something odd with this patch. I am not able to compile when I
apply this patch. I get the below error

**
cd . && /bin/sh /home/nusiddiq/workspace_cpp/ovn-org/ovn/build-aux/missing
automake-1.16 --foreign Makefile
 cd . && /bin/sh ./config.status Makefile depfiles
config.status: creating Makefile
config.status: executing depfiles commands
config.status: error: in `/home/nusiddiq/workspace_cpp/ovn-org/ovn':
config.status: error: Something went wrong bootstrapping makefile fragments
for automatic dependency tracking.  Try re-running configure with the
'--disable-dependency-tracking' option to at least be able to build
the package (albeit without support for automatic dependency tracking).
See `config.log' for more details
make: *** [Makefile:1763: Makefile] Error 1



Thanks
Numan


> ---
>  Documentation/intro/install/general.rst  | 83 
>  utilities/automake.mk|  8 +++
>  utilities/docker/Makefile| 22 +++
>  utilities/docker/create_ovn_dbs.sh   | 18 +
>  utilities/docker/debian/Dockerfile   | 22 +++
>  utilities/docker/debian/build.sh | 44 +
>  utilities/docker/ovn_default_nb_port |  1 +
>  utilities/docker/ovn_default_northd_host |  1 +
>  utilities/docker/ovn_default_sb_port |  1 +
>  utilities/docker/start-ovn   | 40 
>  10 files changed, 240 insertions(+)
>  create mode 100644 utilities/docker/Makefile
>  create mode 100755 utilities/docker/create_ovn_dbs.sh
>  create mode 100644 utilities/docker/debian/Dockerfile
>  create mode 100755 utilities/docker/debian/build.sh
>  create mode 100644 utilities/docker/ovn_default_nb_port
>  create mode 100644 utilities/docker/ovn_default_northd_host
>  create mode 100644 utilities/docker/ovn_default_sb_port
>  create mode 100755 utilities/docker/start-ovn
>
> diff --git a/Documentation/intro/install/general.rst
> b/Documentation/intro/install/general.rst
> index 99d8fec04..1d5323f76 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -380,6 +380,60 @@ domain socket::
>
>  $ ovn-northd --pidfile --detach --log-file
>
> +
> +Starting OVN Central services in containers
> +~~~
> +
> +For OVN central node, we dont need to load ovs kernel modules on host.
> +Hence, OVN central containers OS need not depend on host OS.
> +
> +Also we can leverage deploying entire OVN control plane in a pod spec for
> use
> +cases like OVN-kubernetes
> +
> +Export following variables in .env  and place it under
> +project root::
> +
> +$ OVN_BRANCH=
> +$ OVN_VERSION=
> +$ DISTRO=
> +$ KERNEL_VERSION=
> +$ GITHUB_SRC=
> +$ DOCKER_REPO=
> +
> +To build ovn modules::
> +
> +$ cd utilities/docker
> +$ make build
> +
> +Compiled Modules will be tagged with docker image
> +
> +To Push ovn modules::
> +
> +$ make push
> +
> +OVN docker image will be pushed to specified docker repo.
> +
> +Start OVN containers using below command::
> +
> +$ docker run -itd --net=host --name=ovn-nb \
> +  : ovn-nb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-sb \
> +  : ovn-sb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-northd \
> +  : ovn-northd-tcp
> +
> +.. note::
> +Current ovn central components comes up in docker image in a
> standalone
> +mode with protocol tcp.
> +
> +The debian docker file use ubuntu 16.04 as a base image for reference.
> +
> +User can use any other base image for debian, e.g. u14.04, etc.
> +
> +RHEL based docker build support needs to be added.
> +
>  Starting OVN host service
>  
>
> @@ -406,6 +460,32 @@ domain socket::
>
>  $ ovn-controller --pidfile --detach --log-file
>
> +Starting OVN host service in containers
> +~~~
> +
> +For OVN host too, we dont need to load ovs kernel modules on host.
> +Hence, OVN host container OS need not depend on host OS.
> +
> +Also we can leverage deploying OVN host in a pod spec for use cases like
> +OVN-kubernetes to manage OVS which can be running as a service on host or
> in
> +container.
> +
> +Start ovsdb-server and ovs-vswitchd components as per
> +http://docs.openvswitch.org/en/latest/intro/install/general/
> +
> +start local ovn-controller with below command if ovs is also running in
> +container::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  --volumes-from=ovsdb-server \
> +  : ovn-controller
> +
> +start local ovn-controller with below command if ovs is running as a
> service::
> +
> +$ docker run -itd 

[ovs-dev] deseo instalar en mi rasperry

[Andrea Estefania Mariduena Cedeno via dev]

Tiene los codigos para instalar en rasperry pi 3


Sent from Mail for Windows 10

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

Re: [ovs-dev] [PATCH v3 2/5] ovsdb-idl.c: Allows retry even when using a single remote.

[Ben Pfaff]

On Wed, Aug 21, 2019 at 11:27:58AM -0700, Ben Pfaff wrote:
> On Wed, Aug 21, 2019 at 11:21:12AM -0700, Han Zhou wrote:
> > On Wed, Aug 21, 2019 at 11:13 AM Ben Pfaff  wrote:
> > >
> > > On Mon, Aug 19, 2019 at 09:29:57AM -0700, Han Zhou wrote:
> > > > From: Han Zhou 
> > > >
> > > > When clustered mode is used, the client needs to retry connecting
> > > > to new servers when certain failures happen. Today it is allowed to
> > > > retry new connection only if multiple remotes are used, which prevents
> > > > using LB VIP with clustered nodes. This patch makes sure the retry
> > > > logic works when using LB VIP: although same IP is used for retrying,
> > > > the LB can actually redirect the connection to a new node.
> > > >
> > > > Signed-off-by: Han Zhou 
> > > > ---
> > > > v2 -> v3: Minor fix for test case.
> > >
> > > I see that I commented on v1 of this patch by mistake, but the same
> > > comment applies here: please update the documentation to explain the new
> > > behavior.
> > 
> > Hi Ben,
> > 
> > Yes we agreed to update ovn-nb/sb documentation, but this patch belongs to
> > OVS repo and the documentation would belong to OVN repo. So we will send
> > update to documentation of ovn-nbctl/sbctl separately on OVN repo.
> 
> OK.
> 
> Do you want this patch backported to 2.12?

I applied this series to master.  I haven't done any backports (besides
patch 1, which is obviously a bug fix).  Let me know if you'd like any
of the remaining patches backported.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 ovn 0/5] External OVS source support and separate run dir for OVN

[Numan Siddique]

On Wed, Aug 21, 2019 at 10:30 PM Han Zhou  wrote:

>
>
> On Mon, Aug 19, 2019 at 11:13 AM  wrote:
> >
> > From: Numan Siddique 
> >
> > This patch series adds support for building OVN from external OVS
> > sources.
> >
> > The first patch adds the support to compile OVN from external OVS
> sources.
> > The following configuration options are added when configuring OVN
> >   * --with-ovs-source (mandatory)
> >   * --with-ovs-build (optional)
> >
> > Patch 2 adds support to run OVN services using separate
> > directores
> >   - Default run time dir - /usr/local/var/run/ovm
> >   - Default log dir - /usr/loca/var/log/ovn
> >   - Default db dir - /usr/loca/etc/ovn
> >
> > Patch 3 fixes "make rpm-fedora" which is presently broken
> >
> > Patch 4 runs OVN services as openvswitch user for rhel when rpms are
> > used.
> >
> > Patch 5 removes the python subdirectory as that directory belongs
> > to OVS and uses the required files from the OVS repo.
> >
> > v1 -> v2
> > 
> >  * Addressed the review comments.
> >  * Swapped the patch 1 and 2 as it was easier to address Mark's comment
> >on OVS_RUNDIR/OVN_RUNDIR
> >  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few of
> >the macros to OVS_* to OVN_*.
> >
> > Combined the patch 1 and 2 in this series which were submitted
> > separately earlier.
> >
> >
>
> Hi Numan,
>
> Thanks for this work. I tried applying this series on master, and then
> removed the "ovs" subfolder just to see if it uses the external OVS
> completely. However I encountered some error:
>
> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99
> -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I ./include -I
> /home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
> /home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
> /home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
>  -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
> -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
> -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
> -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
> -Wshadow-g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
> lib/logical-fields.lo lib/logical-fields.c &&\
> mv -f $depbase.Tpo $depbase.Plo
> lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
> lib/actions.c:2592:9: warning: implicit declaration of function
> 'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
>  ofpact_put_CHECK_PKT_LARGER(ofpacts);
>  ^
> lib/actions.c:2592:9: warning: initialization makes pointer from integer
> without a cast
> lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
>  check_pkt_larger->pkt_len = cipl->pkt_len;
>  ^
> lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
>  check_pkt_larger->dst = expr_resolve_field(>dst);
>  ^
>
> Is this expected?
>

Hi Han,

This is not expected. Before submitting I did test by removing the ovs
subdirectory. I tried even now and I don't see any issue.
Looks to me either you have done "make install" in the ovs repo or your
ovs sources are a bit old.

Can you please check if that's the case.

Thanks
Numan


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

Re: [ovs-dev] [PATCH v3 2/5] ovsdb-idl.c: Allows retry even when using a single remote.

[Ben Pfaff]

On Wed, Aug 21, 2019 at 11:21:12AM -0700, Han Zhou wrote:
> On Wed, Aug 21, 2019 at 11:13 AM Ben Pfaff  wrote:
> >
> > On Mon, Aug 19, 2019 at 09:29:57AM -0700, Han Zhou wrote:
> > > From: Han Zhou 
> > >
> > > When clustered mode is used, the client needs to retry connecting
> > > to new servers when certain failures happen. Today it is allowed to
> > > retry new connection only if multiple remotes are used, which prevents
> > > using LB VIP with clustered nodes. This patch makes sure the retry
> > > logic works when using LB VIP: although same IP is used for retrying,
> > > the LB can actually redirect the connection to a new node.
> > >
> > > Signed-off-by: Han Zhou 
> > > ---
> > > v2 -> v3: Minor fix for test case.
> >
> > I see that I commented on v1 of this patch by mistake, but the same
> > comment applies here: please update the documentation to explain the new
> > behavior.
> 
> Hi Ben,
> 
> Yes we agreed to update ovn-nb/sb documentation, but this patch belongs to
> OVS repo and the documentation would belong to OVN repo. So we will send
> update to documentation of ovn-nbctl/sbctl separately on OVN repo.

OK.

Do you want this patch backported to 2.12?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/5] ovsdb-idl.c: Allows retry even when using a single remote.

[Han Zhou]

On Wed, Aug 21, 2019 at 11:13 AM Ben Pfaff  wrote:
>
> On Mon, Aug 19, 2019 at 09:29:57AM -0700, Han Zhou wrote:
> > From: Han Zhou 
> >
> > When clustered mode is used, the client needs to retry connecting
> > to new servers when certain failures happen. Today it is allowed to
> > retry new connection only if multiple remotes are used, which prevents
> > using LB VIP with clustered nodes. This patch makes sure the retry
> > logic works when using LB VIP: although same IP is used for retrying,
> > the LB can actually redirect the connection to a new node.
> >
> > Signed-off-by: Han Zhou 
> > ---
> > v2 -> v3: Minor fix for test case.
>
> I see that I commented on v1 of this patch by mistake, but the same
> comment applies here: please update the documentation to explain the new
> behavior.

Hi Ben,

Yes we agreed to update ovn-nb/sb documentation, but this patch belongs to
OVS repo and the documentation would belong to OVN repo. So we will send
update to documentation of ovn-nbctl/sbctl separately on OVN repo.

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

Re: [ovs-dev] [PATCH v3 2/5] ovsdb-idl.c: Allows retry even when using a single remote.

[Ben Pfaff]

On Mon, Aug 19, 2019 at 09:29:57AM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> When clustered mode is used, the client needs to retry connecting
> to new servers when certain failures happen. Today it is allowed to
> retry new connection only if multiple remotes are used, which prevents
> using LB VIP with clustered nodes. This patch makes sure the retry
> logic works when using LB VIP: although same IP is used for retrying,
> the LB can actually redirect the connection to a new node.
> 
> Signed-off-by: Han Zhou 
> ---
> v2 -> v3: Minor fix for test case.

I see that I commented on v1 of this patch by mistake, but the same
comment applies here: please update the documentation to explain the new
behavior.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/4] ovsdb-idl.c: Allows retry even when using a single remote.

[Ben Pfaff]

I'm not sure what to make of this discussion.  Han, will you update the
patch to update the documentation?  It seems that, at a minimum, that is
the conclusion here.

On Wed, Aug 14, 2019 at 06:40:45PM -0700, aginwala wrote:
> Sure. Thanks for re-validating different corner cases with me. Yup, we can
> update more details in  leader-only section about using single LB VIP while
> accessing clustered db via  ovn-nbctl/ovn-sbctl for sure to avoid
> confusion.
> 
> On Wed, Aug 14, 2019 at 3:21 PM Han Zhou  wrote:
> 
> > Hi Aginwala, thanks for the testing. The change of this patch will cause
> > the IDL to avoid connecting to a follower if "leader_only" is set to
> > "true". It is the same behavior as before when multiple remotes are used,
> > but now it just does the same even when a single remote is used, because
> > the single remote could be a VIP, so it is desired  behavior. For
> > ovn-nbctl, "leader_only" is default to true, so it is expected that it
> > refuses to connect if the remote is a follower. However, if you are using
> > daemon mode ovn-nbctl, and if you didn't set "--no-leader-only", it should
> > keep retrying until it connects to a leader (depends on LB redirecting to
> > different servers). I agree this may cause some confusion when a user of
> > ovn-nbctl connects to only a single remote of a cluster, he/she could get
> > failure if --no-leader-only is not specified, but it is better than let
> > user believe they are connected to a leader while in reality connected to a
> > follower. Maybe I can improve the ovn-nbctl/sbctl documentation to emphasis
> > this to avoid confusion.
> >
> > On Tue, Aug 13, 2019 at 7:13 PM aginwala  wrote:
> >
> >>
> >>
> >> On Tue, Aug 13, 2019 at 7:07 PM aginwala  wrote:
> >>
> >>> Thanks for the fixes. Found a bug in current code which breaks both
> >>> nbctl with local socket and daemon mode on follower nodes. Also nbctl
> >>> daemon mode always tries to connect to leader node by default which also
> >>> failed to connect.
> >>> e.g. export OVN_NB_DB=tcp::6641; ovn-nbctl  --pidfile --detach.
> >>>  
> >>> 2019-08-14T01:07:06Z|00036|ovsdb_idl|INFO|unix:/var/run/openvswitch/ovnnb_db.sock:
> >>> clustered database server is not cluster leader; trying another server
> >>> 2019-08-14T01:07:06Z|00037|reconnect|DBG|unix:/var/run/openvswitch/ovnnb_db.sock:
> >>> entering RECONNECT
> >>> 2019-08-14T01:07:06Z|00038|ovsdb_idl|DBG|unix:/var/run/openvswitch/ovnnb_db.sock:
> >>> SERVER_MONITOR_COND_REQUESTED -> RETRY at lib/ovsdb-idl.c:1917
> >>> 2019-08-14T01:07:06Z|00039|poll_loop|DBG|wakeup due to 0-ms timeout at
> >>> lib/reconnect.c:643
> >>> 2019-08-14T01:07:06Z|00040|reconnect|INFO|unix:/var/run/openvswitch/ovnnb_db.sock:
> >>> connection attempt timed out
> >>> 2019-08-14T01:07:06Z|00041|reconnect|INFO|unix:/var/run/openvswitch/ovnnb_db.sock:
> >>> waiting 2 seconds before reconnect
> >>> 2019-08-14T01:07:06Z|00042|reconnect|DBG|unix:/var/run/openvswitch/ovnnb_db.sock:
> >>> entering BACKOFF
> >>>
> >>> Need to explicitly specify no-leader-only to work around.  ovn-nbctl
> >>> --pidfile --detach --no-leader-only.
> >>> For LB VIP, since LB sees all nodes are active, connections are
> >>> established to all cluster nodes equally. I am using round robin setting
> >>> for LB VIP for 3 node cluster using raft.
> >>>
> >>> Hence, are we always going to avoid this behavior because this is
> >>> forcing to always connect to leader and not to followers? So if we use 
> >>> this
> >>> approach, idl will show ovn-nbctl: tcp::6641: database connection
> >>> failed () if requests reaches followers and only processes success if
> >>> request reaches leader node with this patch.
> >>>
> >>>
> >>> On Tue, Aug 13, 2019 at 9:23 AM Han Zhou  wrote:
> >>>
>  From: Han Zhou 
> 
>  When clustered mode is used, the client needs to retry connecting
>  to new servers when certain failures happen. Today it is allowed to
>  retry new connection only if multiple remotes are used, which prevents
>  using LB VIP with clustered nodes. This patch makes sure the retry
>  logic works when using LB VIP: although same IP is used for retrying,
>  the LB can actually redirect the connection to a new node.
> 
>  Signed-off-by: Han Zhou 
>  ---
>   lib/ovsdb-idl.c| 11 +++---
>   tests/ovsdb-cluster.at | 57
>  ++
>   tests/test-ovsdb.c |  1 +
>   3 files changed, 61 insertions(+), 8 deletions(-)
> 
>  diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
>  index 1a6a4af..190143f 100644
>  --- a/lib/ovsdb-idl.c
>  +++ b/lib/ovsdb-idl.c
>  @@ -657,12 +657,8 @@ ovsdb_idl_state_to_string(enum ovsdb_idl_state
>  state)
>   static void
>   ovsdb_idl_retry_at(struct ovsdb_idl *idl, const char *where)
>   {
>  -if (idl->session && jsonrpc_session_get_n_remotes(idl->session) >
>  1) {
>  -ovsdb_idl_force_reconnect(idl);
>  

Re: [ovs-dev] [PATCH 1/4] raft.c: Move raft_reset_ping_timer() out of the loop.

[Ben Pfaff]

On Tue, Aug 13, 2019 at 09:23:19AM -0700, Han Zhou wrote:
> From: Han Zhou 
> 
> Fixes: commit 5a9b53a5 ("ovsdb raft: Fix duplicated transaction execution 
> when leader failover.")
> Signed-off-by: Han Zhou 

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

Re: [ovs-dev] does OVS support for sch_taptio scheduling algorithm?

[Ben Pfaff]

On Mon, Aug 19, 2019 at 12:36:09PM +0530, Vikas Kumar wrote:
> could anyone please suggest me how to make my sch_taprio scheduling
> algorithm work with OVS switch? any changes in code is required or any
> specific configuration?

OVS doesn't have built-in support for this scheduling algorithm.  You
have two choices:

1. Configure the scheduling algorithm outside OVS and then just use it
with OVS.  You might need to set up OVS to use "linux-noop" QoS for the
interface in question to keep it from messing about with your scheduling
algorithm in this case.  See ovs-vswitchd.conf.db(5) for details.

2. Add direct support for sch_taprio to OVS.  To do this, you would want
to look at the history of lib/netdev-linux.c and follow the pattern set
by other commits that add new scheduling algorithms, such as commit
2f564bb153f2 ("netdev-linux: netem QoS support").
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] sat-math: Do not use __builtin_s*_overflow() with sparse.

[Ben Pfaff]

On Wed, Aug 21, 2019 at 10:20:36AM -0700, Justin Pettit wrote:
> 
> 
> > On Aug 21, 2019, at 10:17 AM, Ben Pfaff  wrote:
> > 
> > Some versions of sparse do not understand __builtin_saddll_overflow() and
> > related GCC builtins for calculations with overflow detection.  This patch
> > avoids using them when sparse is in use.
> > 
> > Reported-by: Justin Pettit 
> > Tested-by: Justin Pettit 
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Justin Pettit 

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

Re: [ovs-dev] Congrats - You won..

[cc o]

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

Re: [ovs-dev] [PATCH v1] ovs-lib: Fix standalone db migration to raft

[Ben Pfaff]

On Mon, Aug 19, 2019 at 06:36:13PM -0700, Aliasgar Ginwala wrote:
> Current code of create-cluster from standalone db takes backup of existing
> standalone db and then generates a new clustered dbs from backup dbs. Hence,
> during migration if nb and sb  dbs are still present, create-cluster will fail
> saying file exists and will not really convert  dbs to clustered dbs. This
> patch fixes the same.
> 
> e.g message that pops up while migration from standalone to raft cluster:
>  * Backing up database to /etc/openvswitch/ovnnb_db.db.backup5.13.0-1278623084
> ovsdb-tool: I/O error: /etc/openvswitch/ovnnb_db.db: create failed (File 
> exists)
>  * Creating cluster database /etc/openvswitch/ovnnb_db.db from existing one
> 
> Signed-off-by: aginwala 

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

[ovs-dev] [PATCH] sat-math: Do not use __builtin_s*_overflow() with sparse.

[Ben Pfaff]

Some versions of sparse do not understand __builtin_saddll_overflow() and
related GCC builtins for calculations with overflow detection.  This patch
avoids using them when sparse is in use.

Reported-by: Justin Pettit 
Tested-by: Justin Pettit 
Signed-off-by: Ben Pfaff 
---
 lib/sat-math.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/sat-math.h b/lib/sat-math.h
index 8dda1515fdd0..d668723878db 100644
--- a/lib/sat-math.h
+++ b/lib/sat-math.h
@@ -41,7 +41,7 @@ llsat_add__(long long int x, long long int y)
 static inline long long int
 llsat_add(long long int x, long long int y)
 {
-#if __GNUC__ >= 5 || __has_builtin(__builtin_saddll_overflow)
+#if (__GNUC__ >= 5 || __has_builtin(__builtin_saddll_overflow)) && !__CHECKER__
 long long int sum;
 return (!__builtin_saddll_overflow(x, y, ) ? sum
 : x > 0 ? LLONG_MAX : LLONG_MIN);
@@ -67,7 +67,7 @@ llsat_sub__(long long int x, long long int y)
 static inline long long int
 llsat_sub(long long int x, long long int y)
 {
-#if __GNUC__ >= 5 || __has_builtin(__builtin_ssubll_overflow)
+#if (__GNUC__ >= 5 || __has_builtin(__builtin_ssubll_overflow)) && !__CHECKER__
 long long int difference;
 return (!__builtin_ssubll_overflow(x, y, ) ? difference
 : x >= 0 ? LLONG_MAX : LLONG_MIN);
@@ -97,7 +97,7 @@ llsat_mul__(long long int x, long long int y)
 static inline long long int
 llsat_mul(long long int x, long long int y)
 {
-#if __GNUC__ >= 5 || __has_builtin(__builtin_smulll_overflow)
+#if (__GNUC__ >= 5 || __has_builtin(__builtin_smulll_overflow)) && !__CHECKER__
 long long int product;
 return (!__builtin_smulll_overflow(x, y, ) ? product
 : (x > 0) == (y > 0) ? LLONG_MAX : LLONG_MIN);
-- 
2.20.1

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

Re: [ovs-dev] [PATCH v2 ovn] Containerize components

[aginwala]

Sure thanks. V3 is @ https://patchwork.ozlabs.org/patch/1150989/

On Wed, Aug 21, 2019 at 6:15 AM Numan Siddique  wrote:

> On Sat, Aug 17, 2019 at 12:53 PM Aliasgar Ginwala 
> wrote:
>
> > 1. Containerize ovn central components
> > 2. Containerize ovn host
> > 3. Update documentation about building/running ovn in containers.
> >
> > Signed-off-by: aginwala 
> >
>
> This patch has conflicts. Would you mind submit v3 rebasing the patch ?
>
> Thanks
> Numan
>
>
> > ---
> >  Documentation/intro/install/general.rst  | 83 
> >  utilities/automake.mk| 10 ++-
> >  utilities/docker/Makefile| 22 +++
> >  utilities/docker/create_ovn_dbs.sh   | 18 +
> >  utilities/docker/debian/Dockerfile   | 22 +++
> >  utilities/docker/debian/build.sh | 44 +
> >  utilities/docker/ovn_default_nb_port |  1 +
> >  utilities/docker/ovn_default_northd_host |  1 +
> >  utilities/docker/ovn_default_sb_port |  1 +
> >  utilities/docker/start-ovn   | 40 
> >  10 files changed, 241 insertions(+), 1 deletion(-)
> >  create mode 100644 utilities/docker/Makefile
> >  create mode 100755 utilities/docker/create_ovn_dbs.sh
> >  create mode 100644 utilities/docker/debian/Dockerfile
> >  create mode 100755 utilities/docker/debian/build.sh
> >  create mode 100644 utilities/docker/ovn_default_nb_port
> >  create mode 100644 utilities/docker/ovn_default_northd_host
> >  create mode 100644 utilities/docker/ovn_default_sb_port
> >  create mode 100755 utilities/docker/start-ovn
> >
> > diff --git a/Documentation/intro/install/general.rst
> > b/Documentation/intro/install/general.rst
> > index 99d8fec04..1d5323f76 100644
> > --- a/Documentation/intro/install/general.rst
> > +++ b/Documentation/intro/install/general.rst
> > @@ -380,6 +380,60 @@ domain socket::
> >
> >  $ ovn-northd --pidfile --detach --log-file
> >
> > +
> > +Starting OVN Central services in containers
> > +~~~
> > +
> > +For OVN central node, we dont need to load ovs kernel modules on host.
> > +Hence, OVN central containers OS need not depend on host OS.
> > +
> > +Also we can leverage deploying entire OVN control plane in a pod spec
> for
> > use
> > +cases like OVN-kubernetes
> > +
> > +Export following variables in .env  and place it under
> > +project root::
> > +
> > +$ OVN_BRANCH=
> > +$ OVN_VERSION=
> > +$ DISTRO=
> > +$ KERNEL_VERSION=
> > +$ GITHUB_SRC=
> > +$ DOCKER_REPO=
> > +
> > +To build ovn modules::
> > +
> > +$ cd utilities/docker
> > +$ make build
> > +
> > +Compiled Modules will be tagged with docker image
> > +
> > +To Push ovn modules::
> > +
> > +$ make push
> > +
> > +OVN docker image will be pushed to specified docker repo.
> > +
> > +Start OVN containers using below command::
> > +
> > +$ docker run -itd --net=host --name=ovn-nb \
> > +  : ovn-nb-tcp
> > +
> > +$ docker run -itd --net=host --name=ovn-sb \
> > +  : ovn-sb-tcp
> > +
> > +$ docker run -itd --net=host --name=ovn-northd \
> > +  : ovn-northd-tcp
> > +
> > +.. note::
> > +Current ovn central components comes up in docker image in a
> > standalone
> > +mode with protocol tcp.
> > +
> > +The debian docker file use ubuntu 16.04 as a base image for
> reference.
> > +
> > +User can use any other base image for debian, e.g. u14.04, etc.
> > +
> > +RHEL based docker build support needs to be added.
> > +
> >  Starting OVN host service
> >  
> >
> > @@ -406,6 +460,32 @@ domain socket::
> >
> >  $ ovn-controller --pidfile --detach --log-file
> >
> > +Starting OVN host service in containers
> > +~~~
> > +
> > +For OVN host too, we dont need to load ovs kernel modules on host.
> > +Hence, OVN host container OS need not depend on host OS.
> > +
> > +Also we can leverage deploying OVN host in a pod spec for use cases like
> > +OVN-kubernetes to manage OVS which can be running as a service on host
> or
> > in
> > +container.
> > +
> > +Start ovsdb-server and ovs-vswitchd components as per
> > +http://docs.openvswitch.org/en/latest/intro/install/general/
> > +
> > +start local ovn-controller with below command if ovs is also running in
> > +container::
> > +
> > +$ docker run -itd --net=host --name=ovn-controller \
> > +  --volumes-from=ovsdb-server \
> > +  : ovn-controller
> > +
> > +start local ovn-controller with below command if ovs is running as a
> > service::
> > +
> > +$ docker run -itd --net=host --name=ovn-controller \
> > +  -v /var/run/openvswitch/:/var/run/openvswitch/ \
> > +  : ovn-controller
> > +
> >  Validating
> >  --
> >
> > @@ -419,6 +499,9 @@ logical switch ``sw0`` and add logical port
> ``sw0-p1``
> > ::
> >
> >  Refer to ovn-nbctl(8) and ovn-sbctl (8) for more details.
> >
> > +When using ovn in container, exec to container to run above 

[ovs-dev] [PATCH v3 ovn] Containerize components

[amginwal]

From: Aliasgar Ginwala 

1. Containerize ovn central components
2. Containerize ovn host
3. Update documentation about building/running ovn in containers.

Signed-off-by: Aliasgar Ginwala 
---
 Documentation/intro/install/general.rst  | 83 
 utilities/automake.mk|  8 +++
 utilities/docker/Makefile| 22 +++
 utilities/docker/create_ovn_dbs.sh   | 18 +
 utilities/docker/debian/Dockerfile   | 22 +++
 utilities/docker/debian/build.sh | 44 +
 utilities/docker/ovn_default_nb_port |  1 +
 utilities/docker/ovn_default_northd_host |  1 +
 utilities/docker/ovn_default_sb_port |  1 +
 utilities/docker/start-ovn   | 40 
 10 files changed, 240 insertions(+)
 create mode 100644 utilities/docker/Makefile
 create mode 100755 utilities/docker/create_ovn_dbs.sh
 create mode 100644 utilities/docker/debian/Dockerfile
 create mode 100755 utilities/docker/debian/build.sh
 create mode 100644 utilities/docker/ovn_default_nb_port
 create mode 100644 utilities/docker/ovn_default_northd_host
 create mode 100644 utilities/docker/ovn_default_sb_port
 create mode 100755 utilities/docker/start-ovn

diff --git a/Documentation/intro/install/general.rst 
b/Documentation/intro/install/general.rst
index 99d8fec04..1d5323f76 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -380,6 +380,60 @@ domain socket::
 
 $ ovn-northd --pidfile --detach --log-file
 
+
+Starting OVN Central services in containers
+~~~
+
+For OVN central node, we dont need to load ovs kernel modules on host.
+Hence, OVN central containers OS need not depend on host OS.
+
+Also we can leverage deploying entire OVN control plane in a pod spec for use
+cases like OVN-kubernetes
+
+Export following variables in .env  and place it under
+project root::
+
+$ OVN_BRANCH=
+$ OVN_VERSION=
+$ DISTRO=
+$ KERNEL_VERSION=
+$ GITHUB_SRC=
+$ DOCKER_REPO=
+
+To build ovn modules::
+
+$ cd utilities/docker
+$ make build
+
+Compiled Modules will be tagged with docker image
+
+To Push ovn modules::
+
+$ make push
+
+OVN docker image will be pushed to specified docker repo.
+
+Start OVN containers using below command::
+
+$ docker run -itd --net=host --name=ovn-nb \
+  : ovn-nb-tcp
+
+$ docker run -itd --net=host --name=ovn-sb \
+  : ovn-sb-tcp
+
+$ docker run -itd --net=host --name=ovn-northd \
+  : ovn-northd-tcp
+
+.. note::
+Current ovn central components comes up in docker image in a standalone
+mode with protocol tcp.
+
+The debian docker file use ubuntu 16.04 as a base image for reference.
+
+User can use any other base image for debian, e.g. u14.04, etc.
+
+RHEL based docker build support needs to be added.
+
 Starting OVN host service
 
 
@@ -406,6 +460,32 @@ domain socket::
 
 $ ovn-controller --pidfile --detach --log-file
 
+Starting OVN host service in containers
+~~~
+
+For OVN host too, we dont need to load ovs kernel modules on host.
+Hence, OVN host container OS need not depend on host OS.
+
+Also we can leverage deploying OVN host in a pod spec for use cases like
+OVN-kubernetes to manage OVS which can be running as a service on host or in
+container.
+
+Start ovsdb-server and ovs-vswitchd components as per
+http://docs.openvswitch.org/en/latest/intro/install/general/
+
+start local ovn-controller with below command if ovs is also running in
+container::
+
+$ docker run -itd --net=host --name=ovn-controller \
+  --volumes-from=ovsdb-server \
+  : ovn-controller
+
+start local ovn-controller with below command if ovs is running as a service::
+
+$ docker run -itd --net=host --name=ovn-controller \
+  -v /var/run/openvswitch/:/var/run/openvswitch/ \
+  : ovn-controller
+
 Validating
 --
 
@@ -419,6 +499,9 @@ logical switch ``sw0`` and add logical port ``sw0-p1`` ::
 
 Refer to ovn-nbctl(8) and ovn-sbctl (8) for more details.
 
+When using ovn in container, exec to container to run above commands::
+
+$ docker exec -it  /bin/bash
 
 Reporting Bugs
 --
diff --git a/utilities/automake.mk b/utilities/automake.mk
index b2b026f57..3142d177f 100644
--- a/utilities/automake.mk
+++ b/utilities/automake.mk
@@ -29,6 +29,14 @@ EXTRA_DIST += \
 utilities/ovn-detrace.in \
 utilities/ovndb-servers.ocf \
 utilities/checkpatch.py
+utilities/docker/Makefile \
+utilities/docker/start-ovn \
+utilities/docker/create_ovn_dbs.sh \
+utilities/docker/ovn_default_nb_port \
+utilities/docker/ovn_default_sb_port \
+utilities/docker/ovn_default_northd_host \
+utilities/docker/debian/Dockerfile \
+utilities/docker/debian/build.sh
 
 CLEANFILES += \
 utilities/ovn-ctl.8 \
diff --git a/utilities/docker/Makefile b/utilities/docker/Makefile
new 

Re: [ovs-dev] [PATCH v2 ovn 0/5] External OVS source support and separate run dir for OVN

[Han Zhou]

On Mon, Aug 19, 2019 at 11:13 AM  wrote:
>
> From: Numan Siddique 
>
> This patch series adds support for building OVN from external OVS
> sources.
>
> The first patch adds the support to compile OVN from external OVS sources.
> The following configuration options are added when configuring OVN
>   * --with-ovs-source (mandatory)
>   * --with-ovs-build (optional)
>
> Patch 2 adds support to run OVN services using separate
> directores
>   - Default run time dir - /usr/local/var/run/ovm
>   - Default log dir - /usr/loca/var/log/ovn
>   - Default db dir - /usr/loca/etc/ovn
>
> Patch 3 fixes "make rpm-fedora" which is presently broken
>
> Patch 4 runs OVN services as openvswitch user for rhel when rpms are
> used.
>
> Patch 5 removes the python subdirectory as that directory belongs
> to OVS and uses the required files from the OVS repo.
>
> v1 -> v2
> 
>  * Addressed the review comments.
>  * Swapped the patch 1 and 2 as it was easier to address Mark's comment
>on OVS_RUNDIR/OVN_RUNDIR
>  * In patch 2, renamed m4/openvswitch.m4 to m4/ovn.m4 and renamed few of
>the macros to OVS_* to OVN_*.
>
> Combined the patch 1 and 2 in this series which were submitted
> separately earlier.
>
>

Hi Numan,

Thanks for this work. I tried applying this series on master, and then
removed the "ovs" subfolder just to see if it uses the external OVS
completely. However I encountered some error:

/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H
-I.   -I ./include  -I ./include -I ./ovn -I ./include -I
/home/hzhou/src/ovs/include -I /home/hzhou/src/ovs/_build/include -I
/home/hzhou/src/ovs/lib -I /home/hzhou/src/ovs/_build/lib -I
/home/hzhou/src/ovs -I /home/hzhou/src/ovs/_build -I ./lib -I ./lib
 -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith
-Wformat -Wformat-security -Wswitch-enum -Wunused-parameter
-Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition
-Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing
-Wshadow-g -O2 -MT lib/logical-fields.lo -MD -MP -MF $depbase.Tpo -c -o
lib/logical-fields.lo lib/logical-fields.c &&\
mv -f $depbase.Tpo $depbase.Plo
lib/actions.c: In function 'encode_CHECK_PKT_LARGER':
lib/actions.c:2592:9: warning: implicit declaration of function
'ofpact_put_CHECK_PKT_LARGER' [-Wimplicit-function-declaration]
 ofpact_put_CHECK_PKT_LARGER(ofpacts);
 ^
lib/actions.c:2592:9: warning: initialization makes pointer from integer
without a cast
lib/actions.c:2593:21: error: dereferencing pointer to incomplete type
 check_pkt_larger->pkt_len = cipl->pkt_len;
 ^
lib/actions.c:2594:21: error: dereferencing pointer to incomplete type
 check_pkt_larger->dst = expr_resolve_field(>dst);
 ^

Is this expected?

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

Re: [ovs-dev] [PATCH v2 ovn] Containerize components

[Numan Siddique]

On Sat, Aug 17, 2019 at 12:53 PM Aliasgar Ginwala 
wrote:

> 1. Containerize ovn central components
> 2. Containerize ovn host
> 3. Update documentation about building/running ovn in containers.
>
> Signed-off-by: aginwala 
>

This patch has conflicts. Would you mind submit v3 rebasing the patch ?

Thanks
Numan


> ---
>  Documentation/intro/install/general.rst  | 83 
>  utilities/automake.mk| 10 ++-
>  utilities/docker/Makefile| 22 +++
>  utilities/docker/create_ovn_dbs.sh   | 18 +
>  utilities/docker/debian/Dockerfile   | 22 +++
>  utilities/docker/debian/build.sh | 44 +
>  utilities/docker/ovn_default_nb_port |  1 +
>  utilities/docker/ovn_default_northd_host |  1 +
>  utilities/docker/ovn_default_sb_port |  1 +
>  utilities/docker/start-ovn   | 40 
>  10 files changed, 241 insertions(+), 1 deletion(-)
>  create mode 100644 utilities/docker/Makefile
>  create mode 100755 utilities/docker/create_ovn_dbs.sh
>  create mode 100644 utilities/docker/debian/Dockerfile
>  create mode 100755 utilities/docker/debian/build.sh
>  create mode 100644 utilities/docker/ovn_default_nb_port
>  create mode 100644 utilities/docker/ovn_default_northd_host
>  create mode 100644 utilities/docker/ovn_default_sb_port
>  create mode 100755 utilities/docker/start-ovn
>
> diff --git a/Documentation/intro/install/general.rst
> b/Documentation/intro/install/general.rst
> index 99d8fec04..1d5323f76 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -380,6 +380,60 @@ domain socket::
>
>  $ ovn-northd --pidfile --detach --log-file
>
> +
> +Starting OVN Central services in containers
> +~~~
> +
> +For OVN central node, we dont need to load ovs kernel modules on host.
> +Hence, OVN central containers OS need not depend on host OS.
> +
> +Also we can leverage deploying entire OVN control plane in a pod spec for
> use
> +cases like OVN-kubernetes
> +
> +Export following variables in .env  and place it under
> +project root::
> +
> +$ OVN_BRANCH=
> +$ OVN_VERSION=
> +$ DISTRO=
> +$ KERNEL_VERSION=
> +$ GITHUB_SRC=
> +$ DOCKER_REPO=
> +
> +To build ovn modules::
> +
> +$ cd utilities/docker
> +$ make build
> +
> +Compiled Modules will be tagged with docker image
> +
> +To Push ovn modules::
> +
> +$ make push
> +
> +OVN docker image will be pushed to specified docker repo.
> +
> +Start OVN containers using below command::
> +
> +$ docker run -itd --net=host --name=ovn-nb \
> +  : ovn-nb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-sb \
> +  : ovn-sb-tcp
> +
> +$ docker run -itd --net=host --name=ovn-northd \
> +  : ovn-northd-tcp
> +
> +.. note::
> +Current ovn central components comes up in docker image in a
> standalone
> +mode with protocol tcp.
> +
> +The debian docker file use ubuntu 16.04 as a base image for reference.
> +
> +User can use any other base image for debian, e.g. u14.04, etc.
> +
> +RHEL based docker build support needs to be added.
> +
>  Starting OVN host service
>  
>
> @@ -406,6 +460,32 @@ domain socket::
>
>  $ ovn-controller --pidfile --detach --log-file
>
> +Starting OVN host service in containers
> +~~~
> +
> +For OVN host too, we dont need to load ovs kernel modules on host.
> +Hence, OVN host container OS need not depend on host OS.
> +
> +Also we can leverage deploying OVN host in a pod spec for use cases like
> +OVN-kubernetes to manage OVS which can be running as a service on host or
> in
> +container.
> +
> +Start ovsdb-server and ovs-vswitchd components as per
> +http://docs.openvswitch.org/en/latest/intro/install/general/
> +
> +start local ovn-controller with below command if ovs is also running in
> +container::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  --volumes-from=ovsdb-server \
> +  : ovn-controller
> +
> +start local ovn-controller with below command if ovs is running as a
> service::
> +
> +$ docker run -itd --net=host --name=ovn-controller \
> +  -v /var/run/openvswitch/:/var/run/openvswitch/ \
> +  : ovn-controller
> +
>  Validating
>  --
>
> @@ -419,6 +499,9 @@ logical switch ``sw0`` and add logical port ``sw0-p1``
> ::
>
>  Refer to ovn-nbctl(8) and ovn-sbctl (8) for more details.
>
> +When using ovn in container, exec to container to run above commands::
> +
> +$ docker exec -it  /bin/bash
>
>  Reporting Bugs
>  --
> diff --git a/utilities/automake.mk b/utilities/automake.mk
> index d666b9661..4d86f082b 100644
> --- a/utilities/automake.mk
> +++ b/utilities/automake.mk
> @@ -27,7 +27,15 @@ EXTRA_DIST += \
>  utilities/ovn-nbctl.8.xml \
>  utilities/ovn-trace.8.xml \
>  utilities/ovn-detrace.in \
> -utilities/ovndb-servers.ocf
> 

Re: [ovs-dev] [PATCH v4 ovn] Add missing checkpatch.py script

[Mark Michelson]

I pushed this to master. Thanks!

On 8/21/19 8:14 AM, Dumitru Ceara wrote:

On Wed, Aug 21, 2019 at 2:10 PM Lorenzo Bianconi
 wrote:


Add utility checkpatch.py script used to check patch correctness before
submitting it. Introduce checkpatch.at unit tests

Signed-off-by: Lorenzo Bianconi 


Looks good to me. Thanks!

Acked-by: Dumitru Ceara 


---
Changes since v3:
- update checkpatch.py script
Changes since v2:
- add checkpatch.at unit test
Changes since v1:
- fix subject
---
  tests/automake.mk   |1 +
  tests/checkpatch.at |  335 +
  tests/testsuite.at  |1 +
  utilities/automake.mk   |3 +-
  utilities/checkpatch.py | 1006 +++
  5 files changed, 1345 insertions(+), 1 deletion(-)
  create mode 100755 tests/checkpatch.at
  create mode 100755 utilities/checkpatch.py

diff --git a/tests/automake.mk b/tests/automake.mk
index 5411772a4..b19144fe4 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -19,6 +19,7 @@ COMMON_MACROS_AT = \

  TESTSUITE_AT = \
 tests/testsuite.at \
+   tests/checkpatch.at \
 tests/ovn.at \
 tests/ovn-northd.at \
 tests/ovn-nbctl.at \
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
new file mode 100755
index 0..fe21acdf2
--- /dev/null
+++ b/tests/checkpatch.at
@@ -0,0 +1,335 @@
+AT_BANNER([checkpatch])
+
+OVS_START_SHELL_HELPERS
+# try_checkpatch PATCH [ERRORS]
+#
+# Runs checkpatch under Python 2 and Python 3, if installed, on the given
+# PATCH, expecting the specified set of ERRORS (and warnings).
+try_checkpatch() {
+AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
+# Take the patch to test from $1.  Remove an initial four-space indent
+# from it and, if it is just headers with no body, add a null body.
+echo "$1" | sed 's/^//' > test.patch
+if grep '---' expout >/dev/null 2>&1; then :
+else
+printf '\n---\n' >> test.patch
+fi
+
+# Take expected output from $2.
+if test -n "$2"; then
+echo "$2" | sed 's/^//' > expout
+else
+: > expout
+fi
+
+try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
+try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
+}
+try_checkpatch__() {
+if test $1 = no; then
+:
+elif test -s expout; then
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
+ [1], [stdout])
+AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
+else
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
+fi
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([checkpatch - sign-offs])
+
+# Sign-off for single author who is also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A"
+try_checkpatch \
+   "Author: A
+Commit: A" \
+   "ERROR: Author A needs to sign off."
+
+# Single author but somehow the mailing list is the author.
+try_checkpatch \
+   "Author: Foo Bar via dev 
+Commit: A
+
+Signed-off-by: A" \
+   "ERROR: Author should not be mailing list."
+
+# Sign-off for single author and different committer.
+try_checkpatch \
+   "Author: A
+Commit: B
+
+Signed-off-by: A
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Committer B needs to sign off."
+
+# Sign-off for multiple authors with one author also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Author A needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B" \
+   "ERROR: Co-author B needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Co-author B needs to sign off."
+
+# Sign-off for multiple authors and separate committer.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Committer C needs to sign off."
+
+# Extra sign-offs:
+#
+#- If we know the committer, one extra sign-off raises a warning.
+#
+#- If we do not know the committer, two extra sign-offs raise a warning.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: D"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: 

Re: [ovs-dev] [PATCH v4 ovn] Add missing checkpatch.py script

[Dumitru Ceara]

On Wed, Aug 21, 2019 at 2:10 PM Lorenzo Bianconi
 wrote:
>
> Add utility checkpatch.py script used to check patch correctness before
> submitting it. Introduce checkpatch.at unit tests
>
> Signed-off-by: Lorenzo Bianconi 

Looks good to me. Thanks!

Acked-by: Dumitru Ceara 

> ---
> Changes since v3:
> - update checkpatch.py script
> Changes since v2:
> - add checkpatch.at unit test
> Changes since v1:
> - fix subject
> ---
>  tests/automake.mk   |1 +
>  tests/checkpatch.at |  335 +
>  tests/testsuite.at  |1 +
>  utilities/automake.mk   |3 +-
>  utilities/checkpatch.py | 1006 +++
>  5 files changed, 1345 insertions(+), 1 deletion(-)
>  create mode 100755 tests/checkpatch.at
>  create mode 100755 utilities/checkpatch.py
>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 5411772a4..b19144fe4 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -19,6 +19,7 @@ COMMON_MACROS_AT = \
>
>  TESTSUITE_AT = \
> tests/testsuite.at \
> +   tests/checkpatch.at \
> tests/ovn.at \
> tests/ovn-northd.at \
> tests/ovn-nbctl.at \
> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
> new file mode 100755
> index 0..fe21acdf2
> --- /dev/null
> +++ b/tests/checkpatch.at
> @@ -0,0 +1,335 @@
> +AT_BANNER([checkpatch])
> +
> +OVS_START_SHELL_HELPERS
> +# try_checkpatch PATCH [ERRORS]
> +#
> +# Runs checkpatch under Python 2 and Python 3, if installed, on the given
> +# PATCH, expecting the specified set of ERRORS (and warnings).
> +try_checkpatch() {
> +AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
> +# Take the patch to test from $1.  Remove an initial four-space indent
> +# from it and, if it is just headers with no body, add a null body.
> +echo "$1" | sed 's/^//' > test.patch
> +if grep '---' expout >/dev/null 2>&1; then :
> +else
> +printf '\n---\n' >> test.patch
> +fi
> +
> +# Take expected output from $2.
> +if test -n "$2"; then
> +echo "$2" | sed 's/^//' > expout
> +else
> +: > expout
> +fi
> +
> +try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
> +try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
> +}
> +try_checkpatch__() {
> +if test $1 = no; then
> +:
> +elif test -s expout; then
> +AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
> + [1], [stdout])
> +AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
> +else
> +AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
> +fi
> +}
> +OVS_END_SHELL_HELPERS
> +
> +AT_SETUP([checkpatch - sign-offs])
> +
> +# Sign-off for single author who is also the committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A"
> +try_checkpatch \
> +   "Author: A
> +Commit: A" \
> +   "ERROR: Author A needs to sign off."
> +
> +# Single author but somehow the mailing list is the author.
> +try_checkpatch \
> +   "Author: Foo Bar via dev 
> +Commit: A
> +
> +Signed-off-by: A" \
> +   "ERROR: Author should not be mailing list."
> +
> +# Sign-off for single author and different committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: B
> +
> +Signed-off-by: A
> +Signed-off-by: B"
> +try_checkpatch \
> +   "Author: A
> +Commit: B" \
> +   "ERROR: Author A needs to sign off.
> +ERROR: Committer B needs to sign off."
> +
> +# Sign-off for multiple authors with one author also the committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B"
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Co-authored-by: B
> +Signed-off-by: B" \
> +   "ERROR: Author A needs to sign off."
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A
> +Co-authored-by: B" \
> +   "ERROR: Co-author B needs to sign off."
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Co-authored-by: B" \
> +   "ERROR: Author A needs to sign off.
> +ERROR: Co-author B needs to sign off."
> +
> +# Sign-off for multiple authors and separate committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B
> +Signed-off-by: C"
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B" \
> +   "ERROR: Committer C needs to sign off."
> +
> +# Extra sign-offs:
> +#
> +#- If we know the committer, one extra sign-off raises a warning.
> +#
> +#- If we do not know the committer, two extra sign-offs raise a warning.
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B
> +Signed-off-by: C
> +Signed-off-by: D" \
> +   "WARNING: Unexpected sign-offs from developers 

[ovs-dev] [PATCH v4 ovn] Add missing checkpatch.py script

[Lorenzo Bianconi]

Add utility checkpatch.py script used to check patch correctness before
submitting it. Introduce checkpatch.at unit tests

Signed-off-by: Lorenzo Bianconi 
---
Changes since v3:
- update checkpatch.py script
Changes since v2:
- add checkpatch.at unit test
Changes since v1:
- fix subject
---
 tests/automake.mk   |1 +
 tests/checkpatch.at |  335 +
 tests/testsuite.at  |1 +
 utilities/automake.mk   |3 +-
 utilities/checkpatch.py | 1006 +++
 5 files changed, 1345 insertions(+), 1 deletion(-)
 create mode 100755 tests/checkpatch.at
 create mode 100755 utilities/checkpatch.py

diff --git a/tests/automake.mk b/tests/automake.mk
index 5411772a4..b19144fe4 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -19,6 +19,7 @@ COMMON_MACROS_AT = \
 
 TESTSUITE_AT = \
tests/testsuite.at \
+   tests/checkpatch.at \
tests/ovn.at \
tests/ovn-northd.at \
tests/ovn-nbctl.at \
diff --git a/tests/checkpatch.at b/tests/checkpatch.at
new file mode 100755
index 0..fe21acdf2
--- /dev/null
+++ b/tests/checkpatch.at
@@ -0,0 +1,335 @@
+AT_BANNER([checkpatch])
+
+OVS_START_SHELL_HELPERS
+# try_checkpatch PATCH [ERRORS]
+#
+# Runs checkpatch under Python 2 and Python 3, if installed, on the given
+# PATCH, expecting the specified set of ERRORS (and warnings).
+try_checkpatch() {
+AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
+# Take the patch to test from $1.  Remove an initial four-space indent
+# from it and, if it is just headers with no body, add a null body.
+echo "$1" | sed 's/^//' > test.patch
+if grep '---' expout >/dev/null 2>&1; then :
+else
+printf '\n---\n' >> test.patch
+fi
+
+# Take expected output from $2.
+if test -n "$2"; then
+echo "$2" | sed 's/^//' > expout
+else
+: > expout
+fi
+
+try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
+try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
+}
+try_checkpatch__() {
+if test $1 = no; then
+:
+elif test -s expout; then
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
+ [1], [stdout])
+AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
+else
+AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
+fi
+}
+OVS_END_SHELL_HELPERS
+
+AT_SETUP([checkpatch - sign-offs])
+
+# Sign-off for single author who is also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A"
+try_checkpatch \
+   "Author: A
+Commit: A" \
+   "ERROR: Author A needs to sign off."
+
+# Single author but somehow the mailing list is the author.
+try_checkpatch \
+   "Author: Foo Bar via dev 
+Commit: A
+
+Signed-off-by: A" \
+   "ERROR: Author should not be mailing list."
+
+# Sign-off for single author and different committer.
+try_checkpatch \
+   "Author: A
+Commit: B
+
+Signed-off-by: A
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Committer B needs to sign off."
+
+# Sign-off for multiple authors with one author also the committer.
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B"
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Author A needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Signed-off-by: A
+Co-authored-by: B" \
+   "ERROR: Co-author B needs to sign off."
+try_checkpatch \
+   "Author: A
+Commit: A
+
+Co-authored-by: B" \
+   "ERROR: Author A needs to sign off.
+ERROR: Co-author B needs to sign off."
+
+# Sign-off for multiple authors and separate committer.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B" \
+   "ERROR: Committer C needs to sign off."
+
+# Extra sign-offs:
+#
+#- If we know the committer, one extra sign-off raises a warning.
+#
+#- If we do not know the committer, two extra sign-offs raise a warning.
+try_checkpatch \
+   "Author: A
+Commit: C
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or 
co-authors or committers: D"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C"
+try_checkpatch \
+   "Author: A
+
+Signed-off-by: A
+Co-authored-by: B
+Signed-off-by: B
+Signed-off-by: C
+Signed-off-by: D" \
+   "WARNING: Unexpected sign-offs from developers who are not authors or 
co-authors or committers: C, D"
+
+# Missing committer is 

Re: [ovs-dev] [PATCH v3 ovn] Add missing checkpatch.py script

[Dumitru Ceara]

On Tue, Aug 20, 2019 at 6:45 PM Lorenzo Bianconi
 wrote:
>
> Add utility checkpatch.py script used to check patch correctness before
> submitting it. Introduce checkpatch.at unit tests
>
> Signed-off-by: Lorenzo Bianconi 
> ---
> Changes since v2:
> - add checkpatch.at unit test

Hi Lorenzo,

I think you're missing the latest checkpatch.py change from commit:
https://github.com/openvswitch/ovs/commit/aacad8685e3d873d15d7fc7974763655862477de

This will cause the following check to fail:

make check-am TESTSUITEFLAGS="193"
...
193: checkpatch - parenthesized constructs   FAILED (checkpatch.at:36)

Thanks,
Dumitru

>
> Changes since v1:
> - fix subject
> ---
>  tests/automake.mk   |1 +
>  tests/checkpatch.at |  335 +
>  tests/testsuite.at  |1 +
>  utilities/automake.mk   |3 +-
>  utilities/checkpatch.py | 1006 +++
>  5 files changed, 1345 insertions(+), 1 deletion(-)
>  create mode 100755 tests/checkpatch.at
>  create mode 100755 utilities/checkpatch.py
>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 5411772a4..b19144fe4 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -19,6 +19,7 @@ COMMON_MACROS_AT = \
>
>  TESTSUITE_AT = \
> tests/testsuite.at \
> +   tests/checkpatch.at \
> tests/ovn.at \
> tests/ovn-northd.at \
> tests/ovn-nbctl.at \
> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
> new file mode 100755
> index 0..fe21acdf2
> --- /dev/null
> +++ b/tests/checkpatch.at
> @@ -0,0 +1,335 @@
> +AT_BANNER([checkpatch])
> +
> +OVS_START_SHELL_HELPERS
> +# try_checkpatch PATCH [ERRORS]
> +#
> +# Runs checkpatch under Python 2 and Python 3, if installed, on the given
> +# PATCH, expecting the specified set of ERRORS (and warnings).
> +try_checkpatch() {
> +AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no])
> +# Take the patch to test from $1.  Remove an initial four-space indent
> +# from it and, if it is just headers with no body, add a null body.
> +echo "$1" | sed 's/^//' > test.patch
> +if grep '---' expout >/dev/null 2>&1; then :
> +else
> +printf '\n---\n' >> test.patch
> +fi
> +
> +# Take expected output from $2.
> +if test -n "$2"; then
> +echo "$2" | sed 's/^//' > expout
> +else
> +: > expout
> +fi
> +
> +try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2"
> +try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3"
> +}
> +try_checkpatch__() {
> +if test $1 = no; then
> +:
> +elif test -s expout; then
> +AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch],
> + [1], [stdout])
> +AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout])
> +else
> +AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch])
> +fi
> +}
> +OVS_END_SHELL_HELPERS
> +
> +AT_SETUP([checkpatch - sign-offs])
> +
> +# Sign-off for single author who is also the committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A"
> +try_checkpatch \
> +   "Author: A
> +Commit: A" \
> +   "ERROR: Author A needs to sign off."
> +
> +# Single author but somehow the mailing list is the author.
> +try_checkpatch \
> +   "Author: Foo Bar via dev 
> +Commit: A
> +
> +Signed-off-by: A" \
> +   "ERROR: Author should not be mailing list."
> +
> +# Sign-off for single author and different committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: B
> +
> +Signed-off-by: A
> +Signed-off-by: B"
> +try_checkpatch \
> +   "Author: A
> +Commit: B" \
> +   "ERROR: Author A needs to sign off.
> +ERROR: Committer B needs to sign off."
> +
> +# Sign-off for multiple authors with one author also the committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B"
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Co-authored-by: B
> +Signed-off-by: B" \
> +   "ERROR: Author A needs to sign off."
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Signed-off-by: A
> +Co-authored-by: B" \
> +   "ERROR: Co-author B needs to sign off."
> +try_checkpatch \
> +   "Author: A
> +Commit: A
> +
> +Co-authored-by: B" \
> +   "ERROR: Author A needs to sign off.
> +ERROR: Co-author B needs to sign off."
> +
> +# Sign-off for multiple authors and separate committer.
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B
> +Signed-off-by: C"
> +try_checkpatch \
> +   "Author: A
> +Commit: C
> +
> +Signed-off-by: A
> +Co-authored-by: B
> +Signed-off-by: B" \
> +   "ERROR: Committer C needs to sign off."
> +
> +# Extra sign-offs:
> +#
> +#- If we know the committer, one extra sign-off raises a warning.
> +#
> +#- If we do not know the committer, two extra sign-offs raise a 

Re: [ovs-dev] [PATCHv18] netdev-afxdp: add new netdev type for AF_XDP.

[Eelco Chaudron]

On 20 Aug 2019, at 17:20, Ilya Maximets wrote:


On 20.08.2019 14:19, Eelco Chaudron wrote:



On 20 Aug 2019, at 12:10, Ilya Maximets wrote:


On 14.08.2019 19:16, William Tu wrote:
On Wed, Aug 14, 2019 at 7:58 AM William Tu  
wrote:


On Wed, Aug 14, 2019 at 5:09 AM Eelco Chaudron 
 wrote:




On 8 Aug 2019, at 17:38, Ilya Maximets wrote:



I see a rather high number of afxdp_cq_skip, which should to 
my

knowledge never happen?


I tried to investigate this previously, but didn't find 
anything

suspicious.
So, for my knowledge, this should never happen too.
However, I only looked at the code without actually running, 
because

I had no
HW available for testing.

While investigation and stress-testing virtual ports I found 
few

issues with
missing locking inside the kernel, so there is no trust for 
kernel

part of XDP
implementation from my side. I'm suspecting that there are 
some

other bugs in
kernel/libbpf that only could be reproduced with driver mode.

This never happens for virtual ports with SKB mode, so I never 
saw

this coverage
counter being non-zero.


Did some quick debugging, as something else has come up that 
needs my

attention :)

But once I’m in a faulty state and sent a single packet, 
causing
afxdp_complete_tx() to be called, it tells me 2048 descriptors 
are
ready, which is XSK_RING_PROD__DEFAULT_NUM_DESCS. So I guess 
that
there might be some ring management bug. Maybe consumer and 
receiver
are equal meaning 0 buffers, but it returns max? I did not look 
at

the kernel code, so this is just a wild guess :)

(gdb) p tx_done
$3 = 2048

(gdb) p umem->cq
$4 = {cached_prod = 3830466864, cached_cons = 3578066899, mask 
=

2047, size = 2048, producer = 0x7f08486b8000, consumer =
0x7f08486b8040, ring = 0x7f08486b8080}


Thanks for debugging!

xsk_ring_cons__peek() just returns the difference between 
cached_prod

and cached_cons, but these values are too different:

3830466864 - 3578066899 = 252399965

Since this value > requested, it returns requested number 
(2048).


So, the ring is broken. At least broken its 'cached' part. It'll 
be

good
to look at *consumer and *producer values to verify the state of 
the

actual ring.



I’ll try to find some more time next week to debug further.

William I noticed your email in xdp-newbies where you mention 
this
problem of getting the wrong pointers. Did you ever follow up, or 
did

further trouble shooting on the above?


Yes, I posted here
https://www.spinics.net/lists/xdp-newbies/msg00956.html
"Question/Bug about AF_XDP idx_cq from xsk_ring_cons__peek?"

At that time I was thinking about reproducing the problem using 
the
xdpsock sample code from kernel. But turned out that my 
reproduction
code is not correct, so not able to show the case we hit here in 
OVS.


Then I put more similar code logic from OVS to xdpsock, but the 
problem
does not show up. As a result, I worked around it by marking addr 
as

"*addr == UINT64_MAX".

I will debug again this week once I get my testbed back.


Just to refresh my memory. The original issue is that
when calling:
tx_done = xsk_ring_cons__peek(>cq, CONS_NUM_DESCS, _cq);
xsk_ring_cons__release(>cq, tx_done);

I expect there are 'tx_done' elems on the CQ to re-cycle back to 
memory pool.
However, when I inspect these elems, I found some elems that 
'already' been
reported complete last time I call xsk_ring_cons__peek. In other 
word, some

elems show up at CQ twice. And this cause overflow of the mempool.

Thus, mark the elems on CQ as UINT64_MAX to indicate that we 
already

seen this elem.


William, Eelco, which HW NIC you're using? Which kernel driver?


I’m using the below on the latest bpf-next driver:

01:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)
01:00.1 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)


Thanks for information.
I found one suspicious place inside the ixgbe driver that could break
the completion queue ring and prepared a patch:
https://patchwork.ozlabs.org/patch/1150244/

It'll be good if you can test it.


Hi Ilya, I was doping some testing of my own, and also concluded it was 
in the drivers' completion ring. I noticed after sending 512 packets the 
drivers TX counters kept increasing, which looks related to your fix.


Will try it out, and sent results to the upstream mailing list…

Thanks,

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