Re: [ovs-dev] (no subject)
I've invited you to fill in the following form: Untitled form To fill it in, visit: https://docs.google.com/forms/d/e/1FAIpQLSdP0uj-m3P7fx-Od4VBLXQXZyxmUPYuNvSvLMdHQirc3Iywgw/viewform?vc=0c=0w=1flr=0usp=mail_form_link 外贸行业整体大环境不好，但是为什么别人业绩还是一年比一年高？ 外贸渠道选择不对，努力白费！ 现在还把B2B平台当做企业客户开发渠道的唯一选择，那您就真的要被OUT出局了！ 外贸客户搜索与开发软件通过主动出击的方式获得更多与国外客户沟通的机会， 而不被动等待客户咨询，产品更快的推广出去，业务员业绩更稳定，企业更良性发展。 外贸客户搜索与开发软件为外贸企业提供年底客户开发渠道投资计划。 感兴趣的朋友，+V/q：3102168136 了解，顺祝商祺！余生 Google Forms: Create and analyse surveys. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] datapath: Add a new action dec_ttl
On Fri, Nov 13, 2020 at 4:06 AM Ilya Maximets wrote:
>
> On 11/13/20 11:04 AM, Eelco Chaudron wrote:
> > Add support for the dec_ttl action. Instead of programming the datapath with
> > a flow that matches the packet TTL and an IP set, use a single dec_ttl
> > action.
> >
> > The old behavior is kept if the new action is not supported by the datapath.
> >
> > # ovs-ofctl dump-flows br0
> >cookie=0x0, duration=12.538s, table=0, n_packets=4, n_bytes=392, ip
> > actions=dec_ttl,NORMAL
> >cookie=0x0, duration=12.536s, table=0, n_packets=4, n_bytes=168,
> > actions=NORMAL
> >
> > # ping -c1 -t 20 192.168.0.2
> > PING 192.168.0.2 (192.168.0.2) 56(84) bytes of data.
> > IP (tos 0x0, ttl 19, id 45336, offset 0, flags [DF], proto ICMP (1),
> > length 84)
> > 192.168.0.1 > 192.168.0.2: ICMP echo request, id 8865, seq 1, length
> > 64
> >
> > Linux netlink datapath support depends on upstream Linux commit:
> > 744676e77720 ("openvswitch: add TTL decrement action")
> >
> >
> > Note that in the Linux kernel tree the OVS_ACTION_ATTR_ADD_MPLS has been
> > defined, and to make sure the IDs are in sync, it had to be added to the
> > OVS source tree. This required some additional case statements, which
> > should be revisited once the OVS implementation is added.
> >
> >
> > Co-developed-by: Matteo Croce
> > Co-developed-by: Bindiya Kurle
> > Signed-off-by: Eelco Chaudron
> >
> > ---
> > v2: - Used definition instead of numeric value in format_dec_ttl_action()
> > - Changed format from "dec_ttl(ttl<=1()) to
> > "dec_ttl(le_1())" to be more in line with the check_pkt_len
> > action.
> > - Fixed parsing of "dec_ttl()" action for adding a dp flow.
> > - Cleaned up format_dec_ttl_action()
> >
> > datapath/linux/compat/include/linux/openvswitch.h |8 ++
> > lib/dpif-netdev.c |4 +
> > lib/dpif.c|4 +
> > lib/odp-execute.c | 102
> > -
> > lib/odp-execute.h |2
> > lib/odp-util.c| 42 +
> > lib/packets.h | 13 ++-
> > ofproto/ofproto-dpif-ipfix.c |2
> > ofproto/ofproto-dpif-sflow.c |2
> > ofproto/ofproto-dpif-xlate.c | 54 +--
> > ofproto/ofproto-dpif.c| 37
> > ofproto/ofproto-dpif.h|6 +
> > 12 files changed, 253 insertions(+), 23 deletions(-)
> >
>
>
>
> > +static void
> > +format_dec_ttl_action(struct ds *ds,const struct nlattr *attr,
> > + const struct hmap *portno_names)
> > +{
> > +const struct nlattr *nla_acts = nl_attr_get(attr);
> > +int len = nl_attr_get_size(attr);
> > +
> > +ds_put_cstr(ds,"dec_ttl(le_1(");
> > +
> > +if (len > 4 && nla_acts->nla_type == OVS_DEC_TTL_ATTR_ACTION) {
> > +/* Linux kernel add an additional envelope we should strip. */
> > +len -= nl_attr_len_pad(nla_acts, len);
> > +nla_acts = nl_attr_next(nla_acts);
>
> CC: Pravin
>
> I looked at the kernel and I agree that there is a clear bug in kernel's
> implementaion of this action. It receives messages on format:
> OVS_ACTION_ATTR_DEC_TTL(),
> but reports back in format:
> OVS_ACTION_ATTR_DEC_TTL(OVS_DEC_TTL_ATTR_ACTION()).
>
> Since 'OVS_DEC_TTL_ATTR_ACTION' exists, it's clear that original design
> was to have it, i.e. the correct format should be the form that
> kernel reports back to userspace. I'd guess that there was a plan to
> add more features to OVS_ACTION_ATTR_DEC_TTL in the future, e.g. set
> actions execution threshold to something different than 1, so it make
> some sense.
>
> Anyway, the bug is in the kernel part of parsing the netlink message and
> it should be fixed. What I'm suggesting is to send a bugfix to kernel
> to accept only format with nested OVS_DEC_TTL_ATTR_ACTION. Since this
> feature was never implemented in userspace OVS, this change will not
> break anything. On the OVS side we should always format netlink messages
> in a correct way. We have a code that checks feature existence in kernel
> and it should fail if kernel is broken (as it is right now). In this case
> OVS will continue to use old implementation with setting the ttl field.
>
> Since the patch for a kernel is a bug fix, it should be likely backported
> to stable versions, and distributions will pick it up too.
>
> Thoughts?
>
Yes, I agree. lets fix the interface to use currect nested netlink
attribute to pass the actions
Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] MES SALUTATIONS
Bonjour Excusez moi de cette manière de vous contacter, je viens d'apercevoir votre profil et je me suis dis que vous êtes la personne qu'il me faut.Je suis Madame Aline BELLAT de nationalité française, hospitalisé en Europe pour raison de santé. Je souffre d'une tumeur au cerveau et le résultat de certaines de mes analyses médicales faisait état de ce que mes jours sur terre sont comptés. Malheureusement je n'ai ni famille ni enfant qui pourra bénéficier de cet argent. Il m'a été conseillé par le Père de mon église et guide spirituel d'en faire une donation afin que le SEIGNEUR me pardonne mes péchés car j'ai eu à effectuer des trafics illégaux dans divers domaines durant mon parcourt. Vous êtes donc bénéficiaire de 17.500.000 Euros. Je vous l'offre du fond du cœur. Veuillez l'accepter et faîtes-en bon usage. Je réclame juste des prières afin que mon opération se passe très bien. Veuillez donc écrire à mon mail personnel alinebel...@yahoo.com.ph pour rentrer en possession de votre donation car il vous appartient déjà dès à présent. QUE LES BÉNÉDICTION SOIENT AVEC VOUS Cordialement ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Buyer mandate
Dear Buyer / Buyer mandate We currently have Available FOB Rotterdam / Houston/Russian port for JP54, D2, D6, JetA1, EN590 with good and workable procedure. Kindly Contact us via (llc-globalnefteg...@bk.ru} for SCO so as to proceed to the next stage of the transaction Contact Person: Feliks Kolya Email: llc-globalnefteg...@bk.ru Skype: live:.cid.3bad5ec7574943c5 Whatsapp: +79668508440 English Department Best regards Feliks Kolya ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v3] Allow explicit setting of the SNAT zone on a gateway router.
In certain situations, OVN may coexist with other applications on a
host. Traffic from OVN and the other applications may then go out a
shared gateway. If OVN traffic and the other application traffic use
different conntrack zones for SNAT, then it is possible for the shared
gateway to assign conflicting source IP:port combinations. By sharing
the same conntrack zone, there will be no conflicting assignments.
In this commit, we introduce options:snat-ct-zone for northbound logical
routers. By setting this option, users can explicitly set the conntrack
zone for the logical router so that it will match the zone used by
non-OVN traffic on the host.
The biggest side effects of this patch are:
1) southbound datapath changes now result in recalculating CT zones in
ovn-controller. This can result in recomputing physical flows in more
situations than previously.
2) The table 65 flow to transition between datapaths is no longer
associated with a port binding. This is because the flow refers to
the peer datapath's CT zones, which can now be updated due to changes
on that datapath. The flow therefore may need to be updated either
due to the port binding being changed or the peer datapath being
changed.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1892311
Signed-off-by: Mark Michelson
---
controller/ovn-controller.c | 86 +++
controller/physical.c | 2 +-
lib/ovn-util.c | 7 ++
lib/ovn-util.h | 1 +
northd/ovn-northd.c | 10 +++
ovn-nb.xml | 7 ++
tests/ovn.at| 135
7 files changed, 234 insertions(+), 14 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index a06cae3cc..25de0c72f 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -531,6 +531,21 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl
*ovnsb_idl,
}
}
+static void
+add_pending_ct_zone_entry(struct shash *pending_ct_zones,
+ enum ct_zone_pending_state state,
+ int zone, bool add, const char *name)
+{
+VLOG_DBG("%s ct zone %"PRId32" for '%s'",
+ add ? "assigning" : "removing", zone, name);
+
+struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending);
+pending->state = state; /* Skip flushing zone. */
+pending->zone = zone;
+pending->add = add;
+shash_add(pending_ct_zones, name, pending);
+}
+
static void
update_ct_zones(const struct sset *lports, const struct hmap *local_datapaths,
struct simap *ct_zones, unsigned long *ct_zone_bitmap,
@@ -540,6 +555,8 @@ update_ct_zones(const struct sset *lports, const struct
hmap *local_datapaths,
int scan_start = 1;
const char *user;
struct sset all_users = SSET_INITIALIZER(_users);
+struct simap req_snat_zones = SIMAP_INITIALIZER(_snat_zones);
+unsigned long unreq_snat_zones[BITMAP_N_LONGS(MAX_CT_ZONES)];
SSET_FOR_EACH(user, lports) {
sset_add(_users, user);
@@ -554,6 +571,11 @@ update_ct_zones(const struct sset *lports, const struct
hmap *local_datapaths,
char *snat = alloc_nat_zone_key(>datapath->header_.uuid, "snat");
sset_add(_users, dnat);
sset_add(_users, snat);
+
+int req_snat_zone = datapath_snat_ct_zone(ld->datapath);
+if (req_snat_zone >= 0) {
+simap_put(_snat_zones, snat, req_snat_zone);
+}
free(dnat);
free(snat);
}
@@ -564,15 +586,56 @@ update_ct_zones(const struct sset *lports, const struct
hmap *local_datapaths,
VLOG_DBG("removing ct zone %"PRId32" for '%s'",
ct_zone->data, ct_zone->name);
-struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending);
-pending->state = CT_ZONE_DB_QUEUED; /* Skip flushing zone. */
-pending->zone = ct_zone->data;
-pending->add = false;
-shash_add(pending_ct_zones, ct_zone->name, pending);
+add_pending_ct_zone_entry(pending_ct_zones, CT_ZONE_DB_QUEUED,
+ ct_zone->data, false, ct_zone->name);
bitmap_set0(ct_zone_bitmap, ct_zone->data);
simap_delete(ct_zones, ct_zone);
+} else if (!simap_find(_snat_zones, ct_zone->name)) {
+bitmap_set1(unreq_snat_zones, ct_zone->data);
+}
+}
+
+/* Prioritize requested CT zones */
+struct simap_node *snat_req_node;
+SIMAP_FOR_EACH (snat_req_node, _snat_zones) {
+struct simap_node *node = simap_find(ct_zones, snat_req_node->name);
+if (node) {
+if (node->data == snat_req_node->data) {
+/* No change to this request, so no action needed */
+continue;
+} else {
+/* Zone request has changed for this node. delete old entry */
+
[ovs-dev] 答复: can userspace conntrack support IP fragment?
Thanks Aaron, here are my ipf settings # ovs-appctl dpctl/ipf-get-status netdev@ovs-netdev Fragmentation Module Status --- v4 enabled: 1 v6 enabled: 1 max num frags (v4/v6): 1000 num frag: 0 min v4 frag size: 1200 v4 frags accepted: 660 v4 frags completed: 660 v4 frags expired: 0 v4 frags too small: 0 v4 frags overlapped: 0 v4 frags purged: 0 min v6 frag size: 1280 v6 frags accepted: 0 v6 frags completed: 0 v6 frags expired: 0 v6 frags too small: 0 v6 frags overlapped: 0 v6 frags purged: 0 I tried big packet ping, ICMP is ok, but tcp and udp are not ok. So I really don't know what's wrong. Ip fragment size should be 1500, it is VM MTU value. root@yangyi-ovsdpdk-vm1-on-07:~# ping 172.16.1.250 -s 8192 PING 172.16.1.250 (172.16.1.250) 8192(8220) bytes of data. 8200 bytes from 172.16.1.250: icmp_seq=1 ttl=64 time=1.06 ms 8200 bytes from 172.16.1.250: icmp_seq=2 ttl=64 time=0.651 ms 8200 bytes from 172.16.1.250: icmp_seq=3 ttl=64 time=0.541 ms 8200 bytes from 172.16.1.250: icmp_seq=4 ttl=64 time=0.485 ms 8200 bytes from 172.16.1.250: icmp_seq=5 ttl=64 time=0.600 ms 8200 bytes from 172.16.1.250: icmp_seq=6 ttl=64 time=0.536 ms ^C --- 172.16.1.250 ping statistics --- 6 packets transmitted, 6 received, 0% packet loss, time 5000ms rtt min/avg/max/mdev = 0.485/0.646/1.067/0.197 ms root@yangyi-ovsdpdk-vm1-on-07:~# tcpdump -i ens3 -vvv -c 5 icmp tcpdump: listening on ens3, link-type EN10MB (Ethernet), capture size 262144 bytes 01:32:15.373681 IP (tos 0x0, ttl 64, id 3275, offset 0, flags [+], proto ICMP (1), length 1500) 172.16.1.250 > 172.16.2.10: ICMP echo request, id 1610, seq 22, length 1480 01:32:15.373705 IP (tos 0x0, ttl 64, id 3275, offset 1480, flags [+], proto ICMP (1), length 1500) 172.16.1.250 > 172.16.2.10: icmp 01:32:15.373709 IP (tos 0x0, ttl 64, id 3275, offset 2960, flags [+], proto ICMP (1), length 1500) 172.16.1.250 > 172.16.2.10: icmp 01:32:15.373712 IP (tos 0x0, ttl 64, id 3275, offset 4440, flags [+], proto ICMP (1), length 1500) 172.16.1.250 > 172.16.2.10: icmp 01:32:15.373715 IP (tos 0x0, ttl 64, id 3275, offset 5920, flags [+], proto ICMP (1), length 1500) 172.16.1.250 > 172.16.2.10: icmp 5 packets captured 240 packets received by filter 233 packets dropped by kernel root@yangyi-ovsdpdk-vm1-on-07:~# iperf3 -t 5 -i 1 -c 172.16.1.250 --get-server-output Connecting to host 172.16.1.250, port 5201 [ 4] local 172.16.2.10 port 55350 connected to 172.16.1.250 port 5201 [ ID] Interval Transfer Bandwidth Retr Cwnd [ 4] 0.00-1.00 sec 433 KBytes 3.54 Mbits/sec 88 2.83 KBytes [ 4] 1.00-2.00 sec 1.01 MBytes 8.43 Mbits/sec 124 4.24 KBytes [ 4] 2.00-3.00 sec 921 KBytes 7.54 Mbits/sec 270 7.07 KBytes [ 4] 3.00-4.00 sec 573 KBytes 4.69 Mbits/sec 86 4.24 KBytes [ 4] 4.00-5.00 sec 1.06 MBytes 8.86 Mbits/sec 152 2.83 KBytes - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth Retr [ 4] 0.00-5.00 sec 3.94 MBytes 6.61 Mbits/sec 720 sender [ 4] 0.00-5.00 sec 3.82 MBytes 6.40 Mbits/sec receiver Server output: Accepted connection from 172.16.2.10, port 55348 [ 5] local 172.16.1.250 port 5201 connected to 172.16.2.10 port 55350 [ ID] Interval Transfer Bandwidth [ 5] 0.00-1.00 sec 317 KBytes 2.59 Mbits/sec [ 5] 1.00-2.00 sec 1015 KBytes 8.32 Mbits/sec [ 5] 2.00-3.00 sec 897 KBytes 7.34 Mbits/sec [ 5] 3.00-4.00 sec 590 KBytes 4.83 Mbits/sec [ 5] 4.00-5.00 sec 1.04 MBytes 8.71 Mbits/sec iperf Done. root@yangyi-ovsdpdk-vm1-on-07:~# iperf3 -t 5 -i 1 -c 172.16.1.250 --get-server-output -u -b 1G -l 8192 Connecting to host 172.16.1.250, port 5201 [ 4] local 172.16.2.10 port 58188 connected to 172.16.1.250 port 5201 [ ID] Interval Transfer Bandwidth Total Datagrams [ 4] 0.00-1.00 sec 119 MBytes 998 Mbits/sec 15223 [ 4] 1.00-2.00 sec 118 MBytes 990 Mbits/sec 15110 [ 4] 2.00-3.00 sec 120 MBytes 1.01 Gbits/sec 15418 [ 4] 3.00-4.00 sec 118 MBytes 989 Mbits/sec 15088 [ 4] 4.00-5.00 sec 121 MBytes 1.01 Gbits/sec 15443 - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 4] 0.00-5.00 sec 596 MBytes 1000 Mbits/sec 0.000 ms 0/0 (-nan%) [ 4] Sent 0 datagrams Server output: --- Accepted connection from 172.16.2.10, port 55352 [ 5] local 172.16.1.250 port 5201 connected to 172.16.2.10 port 58188 [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 5] 0.00-1.00 sec 0.00 Bytes 0.00 bits/sec 0.000 ms 0/0 (-nan%) [ 5] 1.00-2.00 sec 0.00
Re: [ovs-dev] [PATCH V2 3/3] compat: Fix compile warning
On 11/16/2020 11:30 AM, Ilya Maximets wrote:
On 11/13/20 8:24 PM, Yi-Hung Wei wrote:
On Thu, Nov 12, 2020 at 3:10 PM Greg Rose wrote:
In ../compat/nf_conntrack_reasm.c nf_frags_cache_name is declared
if OVS_NF_DEFRAG6_BACKPORT is defined. However, later in the patch
it is only used if HAVE_INET_FRAGS_WITH_FRAGS_WORK is defined and
HAVE_INET_FRAGS_RND is not defined. This will cause a compile warning
about unused variables.
Fix it up by using the same defines that enable its use to decide
if it should be declared and avoid the compiler warning.
Fixes: 4a90b277baca ("compat: Fixup ipv6 fragmentation on 4.9.135+ kernels")
Signed-off-by: Greg Rose
---
V2 No change
---
LGTM.
Acked-by: Yi-Hung Wei
Thanks!
Applied to master and backported down to 2.6.
Thank you Ilya for this and the other two commits from this series.
- Greg
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 2/2] python: set ovs.dirs variables with build system values
On 11/16/20 6:11 PM, Mark Gray wrote: > On 16/11/2020 16:33, Ilya Maximets wrote: >> On 11/16/20 5:28 PM, Stokes, Ian wrote: On Wed, 11 Nov 2020 04:25:30 -0500 Mark Gray wrote: > ovs/dirs.py should be auto-generated using the template > ovs/dirs.py.template at build time. This will set the > ovs.dirs python variables with a value specified by the > environment or, if the environment variable is not set, from > the build system. > > Signed-off-by: Mark Gray Acked-By: Timothy Redaelli >>> >>> Thanks Timothy, I've pushed this series to master now. >> >> Thanks, Ian. But we might need a follow up on this change. >> I think, the main reason why dirs.py existed is that we need >> it while doing setup of python package, i.e. we should have >> it during execution of python/ovs/setup.py. Setup script >> already checks if version.py exists, and we might want check >> for dirs.py too, otherwise python package will be non-functional. > > This means that dirs.py was never being called from setup.py but it was > at least (always) there. Looking at ovs-monitor-ipsec which uses the > python bindings, it does `import dirs.py` which means this may be how > python applications are using it. > > Also, I think the automake system should ensure that dirs.py is there > when packaging but its good to include this change I think. I submit a > patch to fix this: > > https://patchwork.ozlabs.org/project/openvswitch/patch/20201116170026.331164-1-mark.d.g...@redhat.com/ > > >> >> Also, gitignore file seems to have dir.py and not dirs.py. >> >> Best regards, Ilya Maximets. >> > One more important thing. This commit broke windows build: https://ci.appveyor.com/project/blp/ovs/builds/36338443/job/bjsq27m27kworj4e Please, check this out. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl: Return correct seqno from ovsdb_idl_db_set_condition().
On 11/10/20 3:34 PM, Dumitru Ceara wrote:
> If an IDL client sets the same monitor condition twice, the expected
> seqno when the IDL contents are updated should be the same for both
> calls.
>
> In the following scenario:
> 1. Client calls ovsdb_idl_db_set_condition(db, table, cond1)
> 2. ovsdb_idl sends monitor_cond_change(cond1) but the server doesn't yet
>reply.
> 3. Client calls ovsdb_idl_db_set_condition(db, table, cond1)
>
> At step 3 the returned expected seqno should be the same as at step 1.
> Similarly, if step 2 is skipped, i.e., the client calls sets
> the condition twice in the same iteration, then both
> ovsdb_idl_db_set_condition() calls should return the same value.
>
> Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API")
> Signed-off-by: Dumitru Ceara
> ---
Thanks!
Applied to master and backported down to 2.12.
It's hard to backport further since we have no fix for in-flight conditions
below 2.12, but we likely do not need it there.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idl.at: Return stream open_block python tests.
On 11/10/20 12:49 PM, Gaëtan Rivet wrote:
> On 10/11/20 11:56 +0100, Ilya Maximets wrote:
>> On 11/10/20 11:39 AM, Gaëtan Rivet wrote:
>>> On 10/11/20 10:15 +0100, Ilya Maximets wrote:
On 11/9/20 2:45 AM, Gaëtan Rivet wrote:
> Hi Ilya,
>
> One nit below,
>
> On 04/09/20 13:51 +0200, Ilya Maximets wrote:
>> Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
>> during python2 to python3 conversion. So, these tests was not
>> checked since that time.
>>
>> This change returns tests back. CHECK_STREAM_OPEN_BLOCK_PY needed
>> updates, so instead I refactored function for C tests to be able to
>> perform python tests too.
>>
>> Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
>> Signed-off-by: Ilya Maximets
>> ---
>> tests/ovsdb-idl.at | 36 ++--
>> 1 file changed, 14 insertions(+), 22 deletions(-)
>>
>> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
>> index 789ae23a9..96be361fc 100644
>> --- a/tests/ovsdb-idl.at
>> +++ b/tests/ovsdb-idl.at
>> @@ -1778,33 +1778,25 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set,
>> simple3 idl-compound-index-with-re
>> ]])
>>
>> m4_define([CHECK_STREAM_OPEN_BLOCK],
>> - [AT_SETUP([Check Stream open block - C - $1])
>> - AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
>> - AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
>> - AT_KEYWORDS([Check Stream open block $1])
>> - AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
>> + [AT_SETUP([Check stream open block - $1 - $3])
>> + AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
>> + AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
>> + AT_KEYWORDS([Check stream open block open_block $3])
>
> The keywords seems to copy the title. Is 'Check' a relevant keyword for
> this test? I only see it used there. Using TESTSUITEFLAGS='-k Check'
> seems less intuitive than '-k open_block' for example.
>
> Also, reading the keywords in ovsdb-idl.at, 'ovsdb server' is
> always used except for those two tests. Would it be relevant there as
> well?
I don't like these keywords too. I kept them because they were in the
original test. But yes, we could make them better.
Something like:
AT_KEYWORDS([ovsdb server stream open_block $1 $3])
What do you think?
>>>
>>> I think those keywords are better, one small nit for $1 -- in this case
>>> either 'C' or 'Python3'. Other tests with python are using 'Python'
>>> instead.
>>>
>>> Invoking the macro with 'Python' instead would align with others, and it
>>> seems fine as python2 support was dropped.
>>
>> All other tests has Python3 in the test name. We could actually just make
>> it conditional, if it's important:
>>
>>AT_KEYWORDS([ovsdb server stream open_block
>> m4_if([$1], [Python3], [Python], [$1]) $3])
>>
>> Bes regards, Ilya Maximets.
>
> IMO this makes reading the code harder for no real gain, I would drop $1 from
> the
> keywords, but that's just me :).
>
> Whichever way you prefer to go,
>
> Acked-by: Gaetan Rivet
Thanks!
I removed $1 from keywords.
Applied to master and backported down to 2.13.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 3/3] compat: Fix compile warning
On 11/13/20 8:24 PM, Yi-Hung Wei wrote:
> On Thu, Nov 12, 2020 at 3:10 PM Greg Rose wrote:
>>
>> In ../compat/nf_conntrack_reasm.c nf_frags_cache_name is declared
>> if OVS_NF_DEFRAG6_BACKPORT is defined. However, later in the patch
>> it is only used if HAVE_INET_FRAGS_WITH_FRAGS_WORK is defined and
>> HAVE_INET_FRAGS_RND is not defined. This will cause a compile warning
>> about unused variables.
>>
>> Fix it up by using the same defines that enable its use to decide
>> if it should be declared and avoid the compiler warning.
>>
>> Fixes: 4a90b277baca ("compat: Fixup ipv6 fragmentation on 4.9.135+ kernels")
>> Signed-off-by: Greg Rose
>>
>> ---
>> V2 No change
>> ---
> LGTM.
>
> Acked-by: Yi-Hung Wei
Thanks!
Applied to master and backported down to 2.6.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 2/3] compat: Fix build issue on RHEL 7.7
On 11/13/20 8:24 PM, Yi-Hung Wei wrote:
> On Thu, Nov 12, 2020 at 3:10 PM Greg Rose wrote:
>>
>> RHEL 7.2 introduced a KABI fixup in struct sk_buff for the name
>> change of l4_rxhash to l4_hash. Then patch
>> 9ba57fc7 ("datapath: Add hash info to upcall") introduced a
>> compile error by using l4_hash and not fixing up the HAVE_L4_RXHASH
>> configuration flag.
>>
>> Remove all references to HAVE_L4_RXHASH and always use l4_hash to
>> resolve the issue. This will break compilation on RHEL 7.0 and
>> RHEL 7.1 but dropping support for these older kernels shouldn't be
>> a problem.
>>
>> Fixes: 9ba57fc7 ("datapath: Add hash info to upcall")
>> Signed-off-by: Greg Rose
>>
>> ---
>> V2 - Just removes l4_rxhash and ends support for RHEL 7.x < 7.2
>> ---
> Thanks for v2. Looks good to me.
>
> Acked-by: Yi-Hung Wei
>
>> diff --git a/datapath/linux/compat/include/linux/skbuff.h
>> b/datapath/linux/compat/include/linux/skbuff.h
>> index bc73255d5..d3bc6c715 100644
>> --- a/datapath/linux/compat/include/linux/skbuff.h
>> +++ b/datapath/linux/compat/include/linux/skbuff.h
>> @@ -278,9 +278,7 @@ static inline void skb_clear_hash(struct sk_buff *skb)
>> #ifdef HAVE_RXHASH
>> skb->rxhash = 0;
>> #endif
>> -#if defined(HAVE_L4_RXHASH)
>> - skb->l4_rxhash = 0;
>> -#endif
>> + skb->l4_hash = 0;
>
> There is a space before the tab in this line. When the maintainer
> commits this patch, please help to remove the space.
Thanks!
I fixed that.
Applied to master and backported down to 2.14.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/3] compat: Remove stale code
On 11/13/20 8:23 PM, Yi-Hung Wei wrote:
> On Thu, Nov 12, 2020 at 3:10 PM Greg Rose wrote:
>>
>> Remove stale and unused code left over after support for kernels
>> older than 3.10 was removed.
>>
>> Fixes: 8063e0958780 ("datapath: Drop support for kernel older than 3.10")
>> Signed-off-by: Greg Rose
>>
>> ---
>> V2 Separate out stale code removal to it's own patch as per suggestion
>>from Yi-Hung
>> ---
> Looks good to me. Thanks.
>
> Acked-by: Yi-Hung Wei
Thanks!
Applied to master and backported down to 2.6.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/1] tests: Add parse-flow tests for MPLS fields
On 10/26/20 1:55 PM, Timothy Redaelli wrote: > Currently "ovs-ofctl parse-flows (NXM)" test doesn't test MPLS fields at all. > > This commit adds a test for the the 4 MPLS fields (mpls_label, mpls_tc, > mpls_bos and mpls_ttl) to "ovs-ofctl parse-flows (NXM)" test. > > Signed-off-by: Timothy Redaelli > --- Thanks! Applied to master and backported down to 2.6. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofp-actions: Fix userspace support for mpls_ttl
On 9/18/20 7:19 PM, Timothy Redaelli wrote:
> Currently mpls_ttl is ignored when a flow is added because MFF_MPLS_TTL is
> not handled in nx_put_raw().
>
> This commit adds the correct handling of MFF_MPLS_TTL in nx_put_raw().
>
> Fixes: bef3f465bcd5 ("openflow: Support matching and modifying MPLS TTL
> field.")
> Cc: b...@ovn.org
> Signed-off-by: Timothy Redaelli
> ---
Thanks!
Applied to master and backported down to 2.6.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 2/2] netdev-dpdk: Add option to configure VF MAC address.
On 11/10/20 12:51 PM, Gaetan Rivet wrote: > In some cloud topologies, using DPDK VF representors in guest requires > configuring a VF before it is assigned to the guest. > > A first basic option for such configuration is setting the VF MAC > address. Add a key 'dpdk-vf-mac' to the 'options' column of the Interface > table. > > This option can be used as such: > >$ ovs-vsctl add-port br0 dpdk-rep0 -- set Interface dpdk-rep0 type=dpdk \ > options:dpdk-vf-mac=00:11:22:33:44:55 > > Signed-off-by: Gaetan Rivet > Suggested-by: Ilya Maximets > Acked-by: Eli Britstein > Acked-by: Kevin Traynor Thanks! Both patches applied to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Don't raise an Exception on failure to connect via SSL
On 10/28/20 3:57 PM, Mark Michelson wrote: > Hi Terry > > Acked-by: Mark Michelson > > On 10/27/20 12:16 PM, Terry Wilson wrote: >> Just bumping to make sure this doesn't fall through the cracks. 11 days >> since the last ack. Let me know if there's anything else that needs >> changing. Thanks! >> >> On Fri, Oct 16, 2020 at 1:37 AM Thomas Neuman >> wrote: >> >>> Yup, fair enough. LGTM >>> >>> Acked-by: Thomas Neuman Thanks! Applied to master and backported down to 2.10. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-offload-dpdk: Pass L4 proto-id to match in the L3 rte_flow_item
On 11/11/20 10:44 AM, Finn, Emma wrote:
>
>
>> -Original Message-
>> From: dev On Behalf Of Sriharsha
>> Basavapatna via dev
>> Sent: Tuesday 20 October 2020 19:04
>> To: d...@openvswitch.org
>> Cc: Eli Britstein
>> Subject: [ovs-dev] [PATCH] netdev-offload-dpdk: Pass L4 proto-id to match in
>> the L3 rte_flow_item
>>
>> The offload layer clears the L4 protocol mask in the L3 item, when the
>> L4 item is passed for matching, as an optimization. This can be confusing
>> while
>> parsing the headers in the PMD. Also, the datapath flow specifies this field
>> to
>> be matched. This optimization is best left to the PMD.
>> This patch restores the code to pass the L4 protocol type in L3 match.
>>
>> Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
>> Signed-off-by: Sriharsha Basavapatna
>>
>> Acked-by: Eli Britstein
>>
>> ---
>> v3: Updated "Acked-by:" and rebased.
>>
>> v2: Updated "fixes:" tag with the right commit id.
>> ---
>>
>
> Tested-by: Emma Finn
Thanks!
Applied to master.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 0/6] Incorporate fixes from lldpd upstream
On 11/13/20 1:54 AM, Fabrizio D'Angelo wrote: > This patchset pulls in a number of required fixes that have been > accumulated over the years in lldpd. > > https://travis-ci.org/github/fabrizio8/ovs/builds/739390567 > > Aaron Conole (1): > lldp: validate a bit more received LLDP frames > > Fabrizio D'Angelo (1): > AUTHORS: Add Fabrizio D'Angelo. > > Jonas Johansson (1): > lldp: Fix size of PEEK_DISCARD_UINT32() > > Vincent Bernat (3): > lldp: fix a buffer overflow when handling management address TLV > lldp: increase statsTLVsUnrecognizedTotal on unknown TLV > lldp: correctly increase discarded count > > AUTHORS.rst | 1 + > lib/lldp/lldp.c | 63 +--- > lib/lldp/lldpd.c | 2 ++ > 3 files changed, 62 insertions(+), 4 deletions(-) > Thanks! Applied to master and backported down to 2.5. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idlc: Return expected sequence number while setting conditions.
On 11/10/20 2:45 PM, Dumitru Ceara wrote: > On 11/10/20 1:18 PM, Ilya Maximets wrote: >> ovsdb_idl_set_condition() returns a sequence number that can be used to >> check if the requested conditions are acknowledged by the server. >> However, database bindings do not return this value to the user, making >> it impossible to check if the conditions are accepted. >> >> Signed-off-by: Ilya Maximets > > Hi Ilya, > > This change looks OK to me: > > Acked-by: Dumitru Ceara Thanks! Applied to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] odp-util: Fix overflow of nested netlink attributes.
On 11/13/20 6:00 PM, Flavio Leitner wrote:
>
> Hi Ilya,
>
> On Mon, Oct 19, 2020 at 10:03:00PM +0200, Ilya Maximets wrote:
>> Length of nested attributes must be checked before storing to the
>> header. If current length exceeds the maximum value parsing should
>> fail, otherwise the length value will be truncated leading to
>> corrupted netlink message and out-of-bound memory accesses:
>>
>> ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6310002cc838
>> at pc 0x00575470 bp 0x7ffc6c322d60 sp 0x7ffc6c322d58
>> READ of size 1 at 0x6310002cc838 thread T0
>> SCARINESS: 12 (1-byte-read-heap-buffer-overflow)
>> #0 0x57546f in format_generic_odp_key lib/odp-util.c:2738:39
>> #1 0x559e70 in check_attr_len lib/odp-util.c:3572:13
>> #2 0x56581a in format_odp_key_attr lib/odp-util.c:4392:9
>> #3 0x5563b9 in format_odp_action lib/odp-util.c:1192:9
>> #4 0x555d75 in format_odp_actions lib/odp-util.c:1279:13
>> ...
>>
>> Fix that by checking the length of nested netlink attributes before
>> updating 'nla_len' inside the header. Additionally introduced
>> assertion inside nl_msg_end_nested() to catch this kind of issues
>> before actual overflow happened.
>>
>> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20003
>> Fixes: 65da723b40a5 ("odp-util: Format tunnel attributes directly from
>> netlink.")
>> Signed-off-by: Ilya Maximets
>> ---
>
> I looked at few things:
>
> The new assert() discount the included header to check only
> the payload and there are many more places in netlink with
> asserts, so it looks okay to me.
>
> The new test triggers the assert if the check nl_attr_oversized()
> is removed, so the test reproduces the issue and the assert
> catches that.
>
> The -E2BIG return is handled by the callers.
>
> It passes the tests I have.
>
> Acked-by: Flavio Leitner
Thanks!
Applied and backported down to 2.5.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Fix SHA-1 algorithm for data bigger than 512 megabytes.
On 11/15/20 3:52 PM, Renat Nurgaliyev wrote: > In modern systems, size_t is 64 bits. There is a 32 bit overflow check > in sha1_update(), which will not work correctly, because compiler will > do an automatic cast to 64 bits, since size_t type variable is in the > expression. We do want however to lose data, since this is the whole > idea of this overflow check. > > Because of this, computation of SHA-1 checksum will always be incorrect > for any data, that is bigger than 512 megabytes, which in bits is the > boundary of 32 bits integer. > > In practice it means that any OVSDB transaction, bigger or equal to 512 > megabytes, is considered corrupt and ovsdb-server will refuse to work > with the database file. This is especially critical for OVN southbound > database, since it tends to grow rapidly. > > Signed-off-by: Renat Nurgaliyev > --- > v2: > replace size_t with uint32_t where necessary instead of explicit cast > --- Thanks! Applied to master and backported all the way down to 2.5. I also crafted a unit test for this issue. Patch available here: https://patchwork.ozlabs.org/project/openvswitch/patch/20201116190822.112-1-i.maxim...@ovn.org/ Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] tests: Add overflow test for the sha1 library.
This is a unit test for the overflow detection issue fixed by commit
a1d2c5f5d9ed ("sha1: Fix algorithm for data bigger than 512 megabytes.")
Signed-off-by: Ilya Maximets
---
tests/library.at | 3 ++-
tests/test-sha1.c | 37 +
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/tests/library.at b/tests/library.at
index ac4ea4abf..1702b7556 100644
--- a/tests/library.at
+++ b/tests/library.at
@@ -53,7 +53,8 @@ AT_CHECK([ovstest test-packets])
AT_CLEANUP
AT_SETUP([SHA-1])
-AT_CHECK([ovstest test-sha1], [0], [.
+AT_KEYWORDS([sha1])
+AT_CHECK([ovstest test-sha1], [0], [..
])
AT_CLEANUP
diff --git a/tests/test-sha1.c b/tests/test-sha1.c
index b7279db6a..cc80888a7 100644
--- a/tests/test-sha1.c
+++ b/tests/test-sha1.c
@@ -137,6 +137,42 @@ test_big_vector(void)
free(vec.data);
}
+static void
+test_huge_vector(void)
+{
+enum { SIZE = 10 };
+struct test_vector vec = {
+NULL, SIZE,
+/* Computed by the sha1sum utility for a file with 10^9 symbols 'a'. */
+{ 0xD0, 0xF3, 0xE4, 0xF2, 0xF3, 0x1C, 0x66, 0x5A, 0xBB, 0xD8,
+ 0xF5, 0x18, 0xE8, 0x48, 0xD5, 0xCB, 0x80, 0xCA, 0x78, 0xF7 }
+};
+int chunk = random_range(SIZE / 1);
+uint8_t md[SHA1_DIGEST_SIZE];
+struct sha1_ctx sha1;
+size_t i, sz;
+
+/* It's not user-friendly to allocate 1GB of memory for a unit test,
+ * so we're allocating only a small chunk and re-using it. */
+vec.data = xmalloc(chunk);
+for (i = 0; i < chunk; i++) {
+vec.data[i] = 'a';
+}
+
+sha1_init();
+for (sz = 0; sz < SIZE; sz += chunk) {
+int n = sz + chunk < SIZE ? chunk : SIZE - sz;
+
+sha1_update(, vec.data, n);
+}
+sha1_final(, md);
+ovs_assert(!memcmp(md, vec.output, SHA1_DIGEST_SIZE));
+
+free(vec.data);
+putchar('.');
+fflush(stdout);
+}
+
static void
test_shar1_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
{
@@ -147,6 +183,7 @@ test_shar1_main(int argc OVS_UNUSED, char *argv[]
OVS_UNUSED)
}
test_big_vector();
+test_huge_vector();
putchar('\n');
}
--
2.25.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2] Allow explicit setting of the SNAT zone on a gateway router.
On Tue, Nov 17, 2020 at 12:14 AM Mark Michelson wrote: > > Thanks for the review Numan. > > On 11/16/20 4:19 AM, Numan Siddique wrote: > > On Thu, Nov 12, 2020 at 8:26 PM Mark Michelson wrote: > >> > >> In certain situations, OVN may coexist with other applications on a > >> host. Traffic from OVN and the other applications may then go out a > >> shared gateway. If OVN traffic and the other application traffic use > >> different conntrack zones for SNAT, then it is possible for the shared > >> gateway to assign conflicting source IP:port combinations. By sharing > >> the same conntrack zone, there will be no conflicting assignments. > >> > >> In this commit, we introduce options:snat-ct-zone for northbound logical > >> routers. By setting this option, users can explicitly set the conntrack > >> zone for the logical router so that it will match the zone used by > >> non-OVN traffic on the host. > >> > >> The biggest side effects of this patch are: > >> 1) southbound datapath changes now result in recalculating CT zones in > >> ovn-controller. This can result in recomputing physical flows in more > >> situations than previously. > >> 2) The table 65 flow to transition between datapaths is no longer > >> associated with a port binding. This is because the flow refers to > >> the peer datapath's CT zones, which can now be updated due to changes > >> on that datapath. The flow therefore may need to be updated either > >> due to the port binding being changed or the peer datapath being > >> changed. > >> > >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1892311 > >> Signed-off-by: Mark Michelson > > > > Hi Mark, > > > > Thanks for the patch. > > > > I did some testing with the below logical resources > > > > ** > > switch 0ff55345-e283-4ed7-8c7f-8475a8a8f968 (sw1) > > port sw1-lr0 > > type: router > > addresses: ["00:00:00:00:ff:02"] > > router-port: lr0-sw1 > > port sw1-port2 > > addresses: ["40:54:00:00:00:04 20.0.0.4"] > > switch bddcdd29-9b32-482a-b9a6-0f9fa1a0e25a (sw0) > > port sw0-lr0 > > type: router > > addresses: ["00:00:00:00:ff:01"] > > router-port: lr0-sw0 > > port sw0-port1 > > addresses: ["50:54:00:00:00:03 10.0.0.3"] > > port sw0-lr1 > > type: router > > router-port: lr1-sw0 > > router 9462469c-d799-4d28-bccd-80e9a8c68e24 (lr1) > > port lr1-sw0 > > mac: "00:00:00:00:fa:21" > > networks: ["10.0.0.20/24"] > > router 96b14269-65ad-4ae9-9fa6-e4ef2a344a54 (lr0) > > port lr0-sw1 > > mac: "00:00:00:00:ff:02" > > networks: ["20.0.0.1/24"] > > port lr0-public > > mac: "00:00:20:20:12:13" > > networks: ["172.168.0.100/24"] > > gateway chassis: [chassis-1] > > * > > > > > > If I set - "ovn-nbctl set logical_router lr0 options:snat-ct-zone=x" > > and If 'x' is already allocated for sw0 snat, then the > > both sw0 snat and lr0 snat share the same zone id. Is that expected ? > > That is not expected. If you request a zone for lr0, then sw0 should > have its zone re-assigned to something different. I can add a test case > for this and debug it. Looking at the code, it's not obvious to me why > it's failing. > > > > > If I set the same zone id 'y' for both lr0 and lr1, then I observed > > that both lr0 and lr1 have the same zone id 'y' and I also > > see the warning in the ovn-controller log. > > > > Later if I change the zone of lr1 to 'z' and then lr0 to 'z', the lr0 > > is not updated with 'z'. So lr0 will have 'y' and lr1 will have 'z'. > > Seems like it's not consistent. > > I wrote the code with the expectation that if duplicate zone assignments > were detected, the oldest request would be honored and the newest would > be ignored. If you first requested 'y' for lr0 and then later requested > 'y' for lr1, the expectation is that when you requested 'y' for lr1, > you'd see the warning, and lr1 would be auto-assigned a different zone > id. I think the inconsistent behavior may be occurring due to the > hashing of the datapaths. The order in which datapaths are visited does > not correlate with the order that the datapaths were assigned their zone > IDs. So if the newer assignment is visited in the datapath traversal > before the older assignment, they'll both get assigned the same zone. > But if the newer assignment is visited in the datapath traversal after > the older assignment, it works as intended. This could be corrected, but > the point may be moot based on discussion below. > > > > > Can there be a possibility that there are 2 shared gateways in one > > node and both want to share the same zone id ? > > > > i.e lr0 is associated with one shared gateway and lr1 with another > > shared gateway (on the same host) and both want to use the zone id 0 ? > > Yes, I guess that could very well be possible. I guess there's no harm > in having multiple gateways share the
Re: [ovs-dev] [PATCH ovn v2] Allow explicit setting of the SNAT zone on a gateway router.
Thanks for the review Numan. On 11/16/20 4:19 AM, Numan Siddique wrote: On Thu, Nov 12, 2020 at 8:26 PM Mark Michelson wrote: In certain situations, OVN may coexist with other applications on a host. Traffic from OVN and the other applications may then go out a shared gateway. If OVN traffic and the other application traffic use different conntrack zones for SNAT, then it is possible for the shared gateway to assign conflicting source IP:port combinations. By sharing the same conntrack zone, there will be no conflicting assignments. In this commit, we introduce options:snat-ct-zone for northbound logical routers. By setting this option, users can explicitly set the conntrack zone for the logical router so that it will match the zone used by non-OVN traffic on the host. The biggest side effects of this patch are: 1) southbound datapath changes now result in recalculating CT zones in ovn-controller. This can result in recomputing physical flows in more situations than previously. 2) The table 65 flow to transition between datapaths is no longer associated with a port binding. This is because the flow refers to the peer datapath's CT zones, which can now be updated due to changes on that datapath. The flow therefore may need to be updated either due to the port binding being changed or the peer datapath being changed. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1892311 Signed-off-by: Mark Michelson Hi Mark, Thanks for the patch. I did some testing with the below logical resources ** switch 0ff55345-e283-4ed7-8c7f-8475a8a8f968 (sw1) port sw1-lr0 type: router addresses: ["00:00:00:00:ff:02"] router-port: lr0-sw1 port sw1-port2 addresses: ["40:54:00:00:00:04 20.0.0.4"] switch bddcdd29-9b32-482a-b9a6-0f9fa1a0e25a (sw0) port sw0-lr0 type: router addresses: ["00:00:00:00:ff:01"] router-port: lr0-sw0 port sw0-port1 addresses: ["50:54:00:00:00:03 10.0.0.3"] port sw0-lr1 type: router router-port: lr1-sw0 router 9462469c-d799-4d28-bccd-80e9a8c68e24 (lr1) port lr1-sw0 mac: "00:00:00:00:fa:21" networks: ["10.0.0.20/24"] router 96b14269-65ad-4ae9-9fa6-e4ef2a344a54 (lr0) port lr0-sw1 mac: "00:00:00:00:ff:02" networks: ["20.0.0.1/24"] port lr0-public mac: "00:00:20:20:12:13" networks: ["172.168.0.100/24"] gateway chassis: [chassis-1] * If I set - "ovn-nbctl set logical_router lr0 options:snat-ct-zone=x" and If 'x' is already allocated for sw0 snat, then the both sw0 snat and lr0 snat share the same zone id. Is that expected ? That is not expected. If you request a zone for lr0, then sw0 should have its zone re-assigned to something different. I can add a test case for this and debug it. Looking at the code, it's not obvious to me why it's failing. If I set the same zone id 'y' for both lr0 and lr1, then I observed that both lr0 and lr1 have the same zone id 'y' and I also see the warning in the ovn-controller log. Later if I change the zone of lr1 to 'z' and then lr0 to 'z', the lr0 is not updated with 'z'. So lr0 will have 'y' and lr1 will have 'z'. Seems like it's not consistent. I wrote the code with the expectation that if duplicate zone assignments were detected, the oldest request would be honored and the newest would be ignored. If you first requested 'y' for lr0 and then later requested 'y' for lr1, the expectation is that when you requested 'y' for lr1, you'd see the warning, and lr1 would be auto-assigned a different zone id. I think the inconsistent behavior may be occurring due to the hashing of the datapaths. The order in which datapaths are visited does not correlate with the order that the datapaths were assigned their zone IDs. So if the newer assignment is visited in the datapath traversal before the older assignment, they'll both get assigned the same zone. But if the newer assignment is visited in the datapath traversal after the older assignment, it works as intended. This could be corrected, but the point may be moot based on discussion below. Can there be a possibility that there are 2 shared gateways in one node and both want to share the same zone id ? i.e lr0 is associated with one shared gateway and lr1 with another shared gateway (on the same host) and both want to use the zone id 0 ? Yes, I guess that could very well be possible. I guess there's no harm in having multiple gateways share the same zone assignment if they both were explicitly configured to use that particular zone assigment. That actually would simplify the code a bit. Thanks Numan --- controller/ovn-controller.c | 89 +++- controller/physical.c | 2 +- lib/ovn-util.c | 7 +++ lib/ovn-util.h | 1 + northd/ovn-northd.c | 10 ovn-nb.xml
Re: [ovs-dev] Bug#974588: openvswitch: DPDK 20.11 support and transition for bullseye
On 11/13/20 11:46 PM, Thomas Goirand wrote: > On 11/13/20 6:54 PM, Ilya Maximets wrote: >> On 11/13/20 1:47 PM, Thomas Goirand wrote: >>> On 11/12/20 5:09 PM, Luca Boccassi wrote: Source: openvswitch Version: 2.13.0+dfsg1-12 Severity: normal X-Debbugs-CC: pkg-dpdk-de...@lists.alioth.debian.org christian.ehrha...@canonical.com Tags: bullseye Dear Openvswitch Maintainers, >> >> Hi, Luca and Thomas. >> We are scoping the src:dpdk 19.11 -> 20.11 transition. If possible, we'd really like to go to bullseye with the latest upstream LTS, as 19.11 is EOL at the end of next year. OVS support for DPDK 20.11 will be released upstream in v2.15, which is due for release on February 15 [1]. Bullseye transition freeze is on January 12 [2], so the dates don't align very well. >> >> >From the upstream OVS perspective feature freeze usually happens on >> January 15. After this point only bug fixes (under normal conditions) >> could be accepted. Unfortunately, as it always happens, last few days >> before the feature freeze might be hot in terms of accepting big number >> of new features. We could try adjusting these dates if January 12 is >> a critical hard deadline, so the feature-list will be stable to the date. >> Let me now if you need this kind of measures from the upstream OVS. >> We can discuss. >> So we are looking to formulate a plan that you can agree with, to sort this out. Based on experience, what Ubuntu usually does to meet release deadlines is to upload from git earlier than the release, so that all major incompatibilities can be sorted. And then after the freeze, once the release is officially out, do a final upgrade to the released version - since a similar enough version was uploaded from git, and at the end of a release cycle it's mostly bug fixes that land upstream, such an upload is acceptable. So we'd like to propose the following ideas: - between now and December: upload v2.14, to minimize the later jump - by the first week of January: upload 2.15~git from the tip of the master branch to experimental - stabilize and sort eventual build issues - upload dpdk 20.11 and ovs 2.15~git to unstable - upload 2.15 proper in February as a bug fix upload to unstable What do you think? Does this sound like a workable plan? We are of course happy to help - Ubuntu will go through the exact same process for 21.04, so a lot of the work is "shared". Thank you! >>> >>> Hi Luca, >>> >>> I wouldn't mind going for this kind of plan, however, I would really not >>> like uploading a version which isn't final from the upstream point of >>> view. So we would have to get the release team approve for a late upload >>> of OVS 2.15. Note that I'm really not happy with the current state of >>> OVS in Buster, which isn't usable right now (I've been using the tip of >>> the git branch for 2.10.0 in production, not what's in Buster that often >>> crashes). I don't want this to happen again. >>> >>> Please get the release team in the loop, therefore, and make them >>> pre-approve such a plan, by opening a bug with them. >>> >>> Also, I would very much like to have OVS and OVN being packaged and >>> maintained on both Ubuntu and Debian the same way. I would very much >>> like if this could happen, because maintaining OVS is hard, and I really >>> feel alone doing it. Your thoughts? >> >> I'm not very familiar with debian/ubuntu packaging process for OVS and OVN, >> but if there is something that we can do from the upstream side to help, e.g. >> by accepting some patches or streamlining release processes, let me know. >> We clearly have a communication gap between upstream OVS and maintainers of >> packages in distributions. >> >> Best regards, Ilya Maximets. > > Hi Ilya, > > Thanks for getting involved in the discussion. > > What I'm worried, is if somehow, the latest OVS breaks OpenStack > Victoria, which will be the OpenStack release for Bullseye. Can you > assure me that it wont break it? I don't think that we have any destructive/incompatible changes planned. Also, IIRC, upstream OpenStack CI had a job to test with OVS master branch (I need to recheck that). So, I hope that we could catch issues early, if any. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 2/2] python: set ovs.dirs variables with build system values
On 16/11/2020 16:33, Ilya Maximets wrote: > On 11/16/20 5:28 PM, Stokes, Ian wrote: >>> On Wed, 11 Nov 2020 04:25:30 -0500 >>> Mark Gray wrote: >>> ovs/dirs.py should be auto-generated using the template ovs/dirs.py.template at build time. This will set the ovs.dirs python variables with a value specified by the environment or, if the environment variable is not set, from the build system. Signed-off-by: Mark Gray >>> >>> Acked-By: Timothy Redaelli >>> >> >> Thanks Timothy, I've pushed this series to master now. > > Thanks, Ian. But we might need a follow up on this change. > I think, the main reason why dirs.py existed is that we need > it while doing setup of python package, i.e. we should have > it during execution of python/ovs/setup.py. Setup script > already checks if version.py exists, and we might want check > for dirs.py too, otherwise python package will be non-functional. This means that dirs.py was never being called from setup.py but it was at least (always) there. Looking at ovs-monitor-ipsec which uses the python bindings, it does `import dirs.py` which means this may be how python applications are using it. Also, I think the automake system should ensure that dirs.py is there when packaging but its good to include this change I think. I submit a patch to fix this: https://patchwork.ozlabs.org/project/openvswitch/patch/20201116170026.331164-1-mark.d.g...@redhat.com/ > > Also, gitignore file seems to have dir.py and not dirs.py. > > Best regards, Ilya Maximets. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] python: Update setup.py to ensure dirs.py is created and used
Signed-off-by: Mark Gray
---
python/ovs/.gitignore | 2 +-
python/setup.py | 8
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/python/ovs/.gitignore b/python/ovs/.gitignore
index 51030beca437..8bbcd824f472 100644
--- a/python/ovs/.gitignore
+++ b/python/ovs/.gitignore
@@ -1,2 +1,2 @@
version.py
-dir.py
+dirs.py
diff --git a/python/setup.py b/python/setup.py
index b7252800c1c1..9e73aa7feb79 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -30,6 +30,14 @@ except IOError:
file=sys.stderr)
sys.exit(-1)
+try:
+# Try to set dirs from the generated ovs/dirs.py
+exec(open("ovs/dirs.py").read())
+except IOError:
+print("Ensure dirs.py is created by running make python/ovs/dirs.py",
+ file=sys.stderr)
+sys.exit(-1)
+
ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError)
if sys.platform == 'win32':
ext_errors += (IOError, ValueError)
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Análisis la situación económica y financiera
Webinar en Vivo: Taller de elaboración de estados financieros con Excel. Viernes 27 de Noviembre - Horario de 10:00 a 17:00 Hrs. BUEN FIN: 3 x 2 en todos nuestros cursos. Promoción válida durante noviembre y diciembre 202 así como enero 2021 El objetivo es que el participante cuente con las herramientas para realizar un análisis profundo de la situación económica y financiera de su organización en un momento determinado, así como aplicar las principales herramientas de análisis e interpretación de los indicadores económicos y financieros para obtener conclusiones válidas acerca de la solvencia y liquidez de la organización. Objetivos Específicos: - Elaborará estados financieros con la herramienta Excel. - Analizará la situación económica financiera de su organización. - Analizará las causas de los resultados obtenidos, tanto positivos como negativos, para ser capaces de entender la motivación a fin de tomar decisiones. Esto le permitirá determinar el desarrollo de la empresa y tomar decisiones financieras precisas para aumentar la rentabilidad. Para mayor información, responder sobre este correo con la palabra Estados + los siguientes datos: NOMBRE: TELÉFONO: EMPRESA: CORREO ALTERNO: Para información inmediata llamar al: (+52) 55 15 54 66 30 - (+52) 55 30 16 70 85 O puede enviarnos un Whatsapp Innova Learn México - innovalearn. mx - Mérida, Yucatán, México ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 2/2] python: set ovs.dirs variables with build system values
On 16/11/2020 16:33, Ilya Maximets wrote: > On 11/16/20 5:28 PM, Stokes, Ian wrote: >>> On Wed, 11 Nov 2020 04:25:30 -0500 >>> Mark Gray wrote: >>> ovs/dirs.py should be auto-generated using the template ovs/dirs.py.template at build time. This will set the ovs.dirs python variables with a value specified by the environment or, if the environment variable is not set, from the build system. Signed-off-by: Mark Gray >>> >>> Acked-By: Timothy Redaelli >>> >> >> Thanks Timothy, I've pushed this series to master now. > > Thanks, Ian. But we might need a follow up on this change. > I think, the main reason why dirs.py existed is that we need > it while doing setup of python package, i.e. we should have > it during execution of python/ovs/setup.py. Setup script > already checks if version.py exists, and we might want check > for dirs.py too, otherwise python package will be non-functional. Sounds small enough. I will update this. > > Also, gitignore file seems to have dir.py and not dirs.py. Probably why it got checked in the first time. > > Best regards, Ilya Maximets. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 2/2] python: set ovs.dirs variables with build system values
On 11/16/20 5:28 PM, Stokes, Ian wrote: >> On Wed, 11 Nov 2020 04:25:30 -0500 >> Mark Gray wrote: >> >>> ovs/dirs.py should be auto-generated using the template >>> ovs/dirs.py.template at build time. This will set the >>> ovs.dirs python variables with a value specified by the >>> environment or, if the environment variable is not set, from >>> the build system. >>> >>> Signed-off-by: Mark Gray >> >> Acked-By: Timothy Redaelli >> > > Thanks Timothy, I've pushed this series to master now. Thanks, Ian. But we might need a follow up on this change. I think, the main reason why dirs.py existed is that we need it while doing setup of python package, i.e. we should have it during execution of python/ovs/setup.py. Setup script already checks if version.py exists, and we might want check for dirs.py too, otherwise python package will be non-functional. Also, gitignore file seems to have dir.py and not dirs.py. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 2/2] python: set ovs.dirs variables with build system values
> On Wed, 11 Nov 2020 04:25:30 -0500 > Mark Gray wrote: > > > ovs/dirs.py should be auto-generated using the template > > ovs/dirs.py.template at build time. This will set the > > ovs.dirs python variables with a value specified by the > > environment or, if the environment variable is not set, from > > the build system. > > > > Signed-off-by: Mark Gray > > Acked-By: Timothy Redaelli > Thanks Timothy, I've pushed this series to master now. Regards Ian > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5.master ovn 0/2] northd: Fair ACL log meters (pre-ddlog merge).
Using meters is a great way to keep the ovn-controllers from getting overwhelmed with ACL log events. Since multiple ACL rows with logging enabled can refer to the same meter name, I ran a little experiment to better understand how that behaves [1]. >From that experiment, we see that a 'noisy' ACL match could consume all the events allowed by the meter, shadowing logs for other ACLs that also use the same meter. The thought of maintaining a meter row per ACL at the NB side is a solution, but it could easily become a management burden for the CMS. A much better approach would be to leverage northd to take care of this on behalf of the ACLs. As northd populates SB meter table from NB meter table, a new logic checks if the meter is configured as 'fair'. Such config is kept as a new column in the Meter table. Fair meters result in additional rows in the SB that have the same attributes of the original (aka template) meter except for its name has the ACL UUID appended to it. Last but not least, northd takes care of using the corresponding meter name as the logical flow action for the logging of the ACL. This change can be tracked in the following github clone/branch: https://github.com/flavio-fernandes/ovn/commits/acl-meters.v5.ddlog [1]: https://github.com/flavio-fernandes/ovsdbapp_playground/blob/acl_meter_issue/scripts/acl_meter.sh Flavio Fernandes (1): northd: Enhance the implementation of ACL log meters. NEWS | 2 + northd/ovn-northd.c | 184 ++ ovn-nb.ovsschema | 5 +- ovn-nb.xml| 16 +++- tests/ovn-nbctl.at| 6 +- tests/ovn-northd.at | 97 ++ utilities/ovn-nbctl.c | 16 +++- 7 files changed, 268 insertions(+), 58 deletions(-) -- 2.18.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 ovn 1/2] northd: Fair ACL log meters.
Bleep bloop. Greetings Flavio Fernandes, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 northd: Fair ACL log meters. When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 ovn 2/2] northd: Fair ACL log meters.
ddlog for ACL log meters.
Implement fair meters for acl logs in ovn_northd.dl.
Enable fair Meter test to also exercise ovn-northd-ddlog.
This is a small variation of blp's implementation of
ddlog for ACL log meters, now using the northbound schema changes [1].
The changes address overall design issues mentioned in that link.
[1]:
https://patchwork.ozlabs.org/project/ovn/patch/20201107213913.gc2753...@ovn.org/
Co-authored-by: Ben Pfaff
Signed-off-by: Flavio Fernandes
---
northd/ovn_northd.dl | 281 ---
tests/ovn-northd.at | 2 +
2 files changed, 161 insertions(+), 122 deletions(-)
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 3fbe67b31..46489e4cb 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -27,6 +27,23 @@ output relation Warning[string]
index Logical_Flow_Index() on sb::Out_Logical_Flow()
+/* Single-row relation that contains the set of meters marked as fair. */
+relation AclFairLogMeters[Set]
+AclFairLogMeters[meters] :- AclFairLogMeters0[meters].
+AclFairLogMeters[set_empty()] :-
+Unit(),
+not AclFairLogMeters0[_].
+
+relation AclFairLogMeters0[Set]
+AclFairLogMeters0[meters] :-
+meter in nb::Meter(.fair = Some{fair}),
+fair,
+var meters = {
+var meters = set_empty();
+set_insert(meters, meter.name);
+meters
+}.
+
/* Meter_Band table */
for (mb in nb::Meter_Band) {
sb::Out_Meter_Band(._uuid = mb._uuid,
@@ -42,6 +59,14 @@ for (meter in nb::Meter) {
.unit = meter.unit,
.bands = meter.bands)
}
+sb::Out_Meter(._uuid = hash128(meter_uuid ++ acl_uuid),
+ .name = "${name}__${uuid2str(acl_uuid)}",
+ .unit = unit,
+ .bands = bands) :-
+AclFairLogMeters[names],
+var name = FlatMap(names),
+nb::Meter(._uuid = meter_uuid, .name = name, .unit = unit, .bands = bands),
+nb::ACL(._uuid = acl_uuid, .meter = Some{name}).
/* Proxy table for Out_Datapath_Binding: contains all Datapath_Binding fields,
* except tunnel id, which is allocated separately (see TunKeyAllocation). */
@@ -2017,7 +2042,7 @@ for ((.ls = ls)) {
.external_ids = map_empty())
}
-function build_acl_log(acl: nb::ACL): string =
+function build_acl_log(acl: nb::ACL, fair_meters: Set): string =
{
if (not acl.log) {
""
@@ -2049,7 +2074,14 @@ function build_acl_log(acl: nb::ACL): string =
};
match (acl.meter) {
None -> (),
-Some{meter} -> vec_push(strs, "meter=${json_string_escape(meter)}")
+Some{meter} -> {
+var s = if (fair_meters.contains(meter)) {
+"${meter}__${uuid2str(acl._uuid)}"
+} else {
+meter
+};
+vec_push(strs, "meter=${json_string_escape(s)}")
+}
};
"log(${string_join(strs, \", \")}); "
}
@@ -2067,31 +2099,33 @@ function oVN_ACL_PRI_OFFSET(): integer = 1000
relation Reject(lsuuid: uuid, pipeline: string, stage: Stage, acl: nb::ACL,
extra_match: string, extra_actions: string)
/* build_reject_acl_rules() */
-for (Reject(lsuuid, pipeline, stage, acl, extra_match_, extra_actions_)) {
-var extra_match = match (extra_match_) {
-"" -> "",
-s -> "(${s}) && "
-} in
-var extra_actions = match (extra_actions_) {
-"" -> "",
-s -> "${s} "
-} in
-var next = match (pipeline == "ingress") {
-true -> "next(pipeline=egress,table=${stage_id(switch_stage(OUT,
QOS_MARK)).0})",
-false -> "next(pipeline=ingress,table=${stage_id(switch_stage(IN,
L2_LKUP)).0})"
-} in
-var acl_log = build_acl_log(acl) in {
-var __match = extra_match ++ acl.__match in
-var actions = acl_log ++ extra_actions ++ "reg0 = 0; "
- "reject { "
- "/* eth.dst <-> eth.src; ip.dst <-> ip.src; is implicit.
*/ "
- "outport <-> inport; ${next}; };" in
-Flow(.logical_datapath = lsuuid,
- .stage= stage,
- .priority = acl.priority + oVN_ACL_PRI_OFFSET(),
- .__match = __match,
- .actions = actions,
- .external_ids = stage_hint(acl._uuid))
+for (AclFairLogMeters[fair_meters]) {
+for (Reject(lsuuid, pipeline, stage, acl, extra_match_, extra_actions_)) {
+var extra_match = match (extra_match_) {
+"" -> "",
+s -> "(${s}) && "
+} in
+var extra_actions = match (extra_actions_) {
+"" -> "",
+s -> "${s} "
+} in
+var next = match (pipeline == "ingress") {
+true -> "next(pipeline=egress,table=${stage_id(switch_stage(OUT,
QOS_MARK)).0})",
+false -> "next(pipeline=ingress,table=${stage_id(switch_stage(IN,
L2_LKUP)).0})"
+} in
+var acl_log =
[ovs-dev] [PATCH v5 ovn 0/2] northd: Fair ACL log meters.
Using meters is a great way to keep the ovn-controllers from getting overwhelmed with ACL log events. Since multiple ACL rows with logging enabled can refer to the same meter name, I ran a little experiment to better understand how that behaves [1]. >From that experiment, we see that a 'noisy' ACL match could consume all the events allowed by the meter, shadowing logs for other ACLs that also use the same meter. The thought of maintaining a meter row per ACL at the NB side is a solution, but it could easily become a management burden for the CMS. A much better approach would be to leverage northd to take care of this on behalf of the ACLs. As northd populates SB meter table from NB meter table, a new logic checks if the meter is configured as 'fair'. Such config is kept as a new column in the Meter table. Fair meters result in additional rows in the SB that have the same attributes of the original (aka template) meter except for its name has the ACL UUID appended to it. Last but not least, northd takes care of using the corresponding meter name as the logical flow action for the logging of the ACL. This change can be tracked in the following github clone/branch: https://github.com/flavio-fernandes/ovn/commits/acl-meters.v5.ddlog [1]: https://github.com/flavio-fernandes/ovsdbapp_playground/blob/acl_meter_issue/scripts/acl_meter.sh Flavio Fernandes (2): northd: Enhance the implementation of ACL log meters. ddlog for ACL log meters NEWS | 2 + northd/ovn-northd.c | 184 +++ northd/ovn_northd.dl | 281 -- ovn-nb.ovsschema | 5 +- ovn-nb.xml| 16 ++- tests/ovn-nbctl.at| 6 +- tests/ovn-northd.at | 99 +++ utilities/ovn-nbctl.c | 16 ++- 8 files changed, 429 insertions(+), 180 deletions(-) -- 2.18.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] can userspace conntrack support IP fragment?
"Yi Yang (杨燚)-云服务集团" writes: > Hi, folks > > > > I used latest ovs matser in Openstack, when I enabled security group and port > security (note: openstack is > using ovs openflow to implement security group), TCP performance is about > several Mbps, big UDP packet (i.e. > 8192) can’t work, but after disabled security group and port security, > everything is ok, I doubt userspace > conntrack can’t support IP fragment (or recent changes introduced bugs), > https://bugzilla.redhat.com/show_bug.cgi?id=1639173 said it can’t handle big > ICMP packet, anybody can > help clarify what limitations of userspace conntrack are? Is there any > existing document to warn users about > them? Thank you in advance. What were your frag settings? For example, try: ovs-appctl dpctl/ipf-set-min-frag v4 1000 ovs-appctl dpctl/ipf-set-max-nfrags 500 See if that helps? IIRC, the fragmentation engine doesn't support ICMP, just tcp/udp. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] datapath: Add a new action dec_ttl
On 13 Nov 2020, at 17:05, Ilya Maximets wrote:
Cc: netdev
On 11/13/20 3:28 PM, Eelco Chaudron wrote:
On 13 Nov 2020, at 13:06, Ilya Maximets wrote:
On 11/13/20 11:04 AM, Eelco Chaudron wrote:
Add support for the dec_ttl action. Instead of programming the
datapath with
a flow that matches the packet TTL and an IP set, use a single
dec_ttl action.
The old behavior is kept if the new action is not supported by the
datapath.
# ovs-ofctl dump-flows br0
cookie=0x0, duration=12.538s, table=0, n_packets=4,
n_bytes=392, ip actions=dec_ttl,NORMAL
cookie=0x0, duration=12.536s, table=0, n_packets=4,
n_bytes=168, actions=NORMAL
# ping -c1 -t 20 192.168.0.2
PING 192.168.0.2 (192.168.0.2) 56(84) bytes of data.
IP (tos 0x0, ttl 19, id 45336, offset 0, flags [DF], proto ICMP
(1), length 84)
192.168.0.1 > 192.168.0.2: ICMP echo request, id 8865,
seq 1, length 64
Linux netlink datapath support depends on upstream Linux commit:
744676e77720 ("openvswitch: add TTL decrement action")
Note that in the Linux kernel tree the OVS_ACTION_ATTR_ADD_MPLS has
been
defined, and to make sure the IDs are in sync, it had to be added
to the
OVS source tree. This required some additional case statements,
which
should be revisited once the OVS implementation is added.
Co-developed-by: Matteo Croce
Co-developed-by: Bindiya Kurle
Signed-off-by: Eelco Chaudron
---
v2: - Used definition instead of numeric value in
format_dec_ttl_action()
- Changed format from "dec_ttl(ttl<=1()) to
"dec_ttl(le_1())" to be more in line with the
check_pkt_len action.
- Fixed parsing of "dec_ttl()" action for adding a dp flow.
- Cleaned up format_dec_ttl_action()
datapath/linux/compat/include/linux/openvswitch.h | 8 ++
lib/dpif-netdev.c
| 4 +
lib/dpif.c
| 4 +
lib/odp-execute.c
| 102 -
lib/odp-execute.h
| 2
lib/odp-util.c
| 42 +
lib/packets.h
| 13 ++-
ofproto/ofproto-dpif-ipfix.c
| 2
ofproto/ofproto-dpif-sflow.c
| 2
ofproto/ofproto-dpif-xlate.c
| 54 +--
ofproto/ofproto-dpif.c
| 37
ofproto/ofproto-dpif.h
| 6 +
12 files changed, 253 insertions(+), 23 deletions(-)
+static void
+format_dec_ttl_action(struct ds *ds,const struct nlattr *attr,
+ const struct hmap
*portno_names)
+{
+ const struct nlattr *nla_acts = nl_attr_get(attr);
+ int len = nl_attr_get_size(attr);
+
+ ds_put_cstr(ds,"dec_ttl(le_1(");
+
+ if (len > 4 && nla_acts->nla_type ==
OVS_DEC_TTL_ATTR_ACTION) {
+ /* Linux kernel add an additional envelope we
should strip. */
+ len -= nl_attr_len_pad(nla_acts, len);
+ nla_acts = nl_attr_next(nla_acts);
CC: Pravin
I looked at the kernel and I agree that there is a clear bug in
kernel's
implementaion of this action. It receives messages on format:
OVS_ACTION_ATTR_DEC_TTL(),
but reports back in format:
OVS_ACTION_ATTR_DEC_TTL(OVS_DEC_TTL_ATTR_ACTION(actions>)).
Since 'OVS_DEC_TTL_ATTR_ACTION' exists, it's clear that original
design
was to have it, i.e. the correct format should be the form that
kernel reports back to userspace. I'd guess that there was a plan
to
add more features to OVS_ACTION_ATTR_DEC_TTL in the future, e.g. set
actions execution threshold to something different than 1, so it
make
some sense.
Anyway, the bug is in the kernel part of parsing the netlink message
and
it should be fixed.
It is already in the mainline kernel, so changing it now would break
the UAPI.
Don't think this is allowed from the kernel side.
Well, UAPI is what specified in include/uapi/linux/openvswitch.h. And
it says:
OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
So, the action must have nested OVS_DEC_TTL_ATTR_ACTION, otherwise
it's malformed.
This means that UAPI is broken now in terms that kernel doesn't
respect it's
own UAPI. And that's a bug that should be fixed.
Ack, will cook up a patch, and sent it to net.
//Eelco
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [net-next] netfiler: conntrack: Add the option to set ct tcp flag - BE_LIBERAL per-ct basis.
On Tue, Nov 10, 2020 at 6:41 PM Florian Westphal wrote: > > Numan Siddique wrote: > > On Tue, Nov 10, 2020 at 5:55 PM Florian Westphal wrote: > > > > > > Numan Siddique wrote: > > > > On Tue, Nov 10, 2020 at 3:06 AM Florian Westphal wrote: > > > > Thanks for the comments. I actually tried this approach first, but it > > > > doesn't seem to work. > > > > I noticed that for the committed connections, the ct tcp flag - > > > > IP_CT_TCP_FLAG_BE_LIBERAL is > > > > not set when nf_conntrack_in() calls resolve_normal_ct(). > > > > > > Yes, it won't be set during nf_conntrack_in, thats why I suggested > > > to do it before confirming the connection. > > > > Sorry for the confusion. What I mean is - I tested your suggestion - i.e > > called > > nf_ct_set_tcp_be_liberal() before calling nf_conntrack_confirm(). > > > > Once the connection is established, for subsequent packets, openvswitch > > calls nf_conntrack_in() [1] to see if the packet is part of the > > existing connection or not (i.e ct.new or ct.est ) > > and if the packet happens to be out-of-window then skb->_nfct is set > > to NULL. And the tcp > > be flags set during confirmation are not reflected when > > nf_conntrack_in() calls resolve_normal_ct(). > > Can you debug where this happens? This looks very very wrong. > resolve_normal_ct() has no business to check any of those flags > (and I don't see where it uses them, it should only deal with the > tuples). > > The flags come into play when nf_conntrack_handle_packet() gets called > after resolve_normal_ct has found an entry, since that will end up > calling the tcp conntrack part. > > The entry found/returned by resolve_normal_ct should be the same > nf_conn entry that was confirmed earlier, i.e. it should be in "liberal" > mode. I debugged a bit. Calling nf_ct_set_tcp_be_liberal() in ct_commit doesn't work because - the first SYN packet during connection establishment is committed to the contract. - but tcp_in_window() calls tcp_options() which clears the tcp ct flags for the SYN and SYN-ACK packets. And hence the flags get cleared. So I think it should be enough if openvswitch calls nf_ct_set_tcp_be_liberal() once the connection is established. I posted v2. Request to take a look. Thanks Numan > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH net-next v2] net: openvswitch: Be liberal in tcp conntrack.
From: Numan Siddique
There is no easy way to distinguish if a conntracked tcp packet is
marked invalid because of tcp_in_window() check error or because
it doesn't belong to an existing connection. With this patch,
openvswitch sets liberal tcp flag for the established sessions so
that out of window packets are not marked invalid.
A helper function - nf_ct_set_tcp_be_liberal(nf_conn) is added which
sets this flag for both the directions of the nf_conn.
Suggested-by: Florian Westphal
Signed-off-by: Numan Siddique
---
include/net/netfilter/nf_conntrack_l4proto.h | 14 ++
net/netfilter/nf_conntrack_proto_tcp.c | 6 --
net/openvswitch/conntrack.c | 8
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h
b/include/net/netfilter/nf_conntrack_l4proto.h
index 88186b95b3c2..9be7320b994f 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -203,6 +203,20 @@ static inline struct nf_icmp_net *nf_icmpv6_pernet(struct
net *net)
{
return >ct.nf_ct_proto.icmpv6;
}
+
+/* Caller must check nf_ct_protonum(ct) is IPPROTO_TCP before calling. */
+static inline void nf_ct_set_tcp_be_liberal(struct nf_conn *ct)
+{
+ ct->proto.tcp.seen[0].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
+ ct->proto.tcp.seen[1].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
+}
+
+/* Caller must check nf_ct_protonum(ct) is IPPROTO_TCP before calling. */
+static inline bool nf_conntrack_tcp_established(const struct nf_conn *ct)
+{
+ return ct->proto.tcp.state == TCP_CONNTRACK_ESTABLISHED &&
+ test_bit(IPS_ASSURED_BIT, >status);
+}
#endif
#ifdef CONFIG_NF_CT_PROTO_DCCP
diff --git a/net/netfilter/nf_conntrack_proto_tcp.c
b/net/netfilter/nf_conntrack_proto_tcp.c
index c8fb2187ad4b..811c6c9b59e1 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -834,12 +834,6 @@ static noinline bool tcp_new(struct nf_conn *ct, const
struct sk_buff *skb,
return true;
}
-static bool nf_conntrack_tcp_established(const struct nf_conn *ct)
-{
- return ct->proto.tcp.state == TCP_CONNTRACK_ESTABLISHED &&
- test_bit(IPS_ASSURED_BIT, >status);
-}
-
/* Returns verdict for packet, or -1 for invalid. */
int nf_conntrack_tcp_packet(struct nf_conn *ct,
struct sk_buff *skb,
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 4beb96139d77..6a88daab0190 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1037,6 +1037,14 @@ static int __ovs_ct_lookup(struct net *net, struct
sw_flow_key *key,
ovs_ct_helper(skb, info->family) != NF_ACCEPT) {
return -EINVAL;
}
+
+ if (nf_ct_protonum(ct) == IPPROTO_TCP &&
+ nf_ct_is_confirmed(ct) && nf_conntrack_tcp_established(ct))
{
+ /* Be liberal for tcp packets so that out-of-window
+* packets are not marked invalid.
+*/
+ nf_ct_set_tcp_be_liberal(ct);
+ }
}
return 0;
--
2.28.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] can userspace conntrack support IP fragment?
Hi, folks I used latest ovs matser in Openstack, when I enabled security group and port security (note: openstack is using ovs openflow to implement security group), TCP performance is about several Mbps, big UDP packet (i.e. 8192) can’t work, but after disabled security group and port security, everything is ok, I doubt userspace conntrack can’t support IP fragment (or recent changes introduced bugs), https://bugzilla.redhat.com/show_bug.cgi?id=1639173 said it can’t handle big ICMP packet, anybody can help clarify what limitations of userspace conntrack are? Is there any existing document to warn users about them? Thank you in advance. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC ovn 0/3] introduce BFD support in ovn-controller
On Thu, Nov 12, 2020 at 9:40 PM Lorenzo Bianconi wrote: > > Introduce BFD protocol in ovn-controller. > We added BFD implementation in ovn since layered protocols usually request to > enable it on ovn entities (e.g. logical router ports) while ovs implementation > relies on physical entities (e.g. ovs interfaces). > Moreover we would establish a BFD session between a given ovn-port and > multiple peers (1:n relation) > A typical use-case is reported in [0]. > The main goal of this series (not fully RFC compliant yet) is to collect > feedbacks from other ovn/ovs developer about the proposed approach. > > [0] - https://bugzilla.redhat.com/show_bug.cgi?id=1847570 Hi Lorenzo, Thanks for working on this feature. I think I am fine with the approach taken by this series, except for one. I don't see the need to have a BFD table in NB DB. Since this is a low level feature, probably CMS should not be aware of it ? or doesn't need to know what mechanism is used for the failover detection. May be a flag in the logical router static route can be exposed and CMS can set it to enable failover detection. BFD table in the SB DB would make sense. Thanks Numan > > Lorenzo Bianconi (3): > controller: introduce bfd tx path in ovn-controller > action: introduce handle_bfd_msg() action > controller: bfd: introduce BFD state machine > > controller/ovn-controller.c | 1 + > controller/pinctrl.c| 405 +++- > controller/pinctrl.h| 2 + > include/ovn/actions.h | 7 + > lib/actions.c | 16 ++ > lib/ovn-l7.h| 19 ++ > northd/ovn-northd.c | 112 ++ > ovn-nb.ovsschema| 17 +- > ovn-nb.xml | 42 > ovn-sb.ovsschema| 21 +- > ovn-sb.xml | 54 + > tests/ovn.at| 4 + > utilities/ovn-trace.c | 2 + > 13 files changed, 697 insertions(+), 5 deletions(-) > > -- > 2.26.2 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-controller: Propagate nb_cfg to the local OVS DB.
On 11/16/20 12:02 PM, Numan Siddique wrote: > On Thu, Nov 12, 2020 at 9:25 PM Dumitru Ceara wrote: >> >> The NB.NB_Global.nb_cfg value gets propagated to >> Chassis_Private.nb_cfg (and then to NB.NB_Global.hv_cfg) as soon as >> ovn-controller has finished installing OVS flows corresponding to >> the NB DB state. >> >> However, if the CMS runs monitoring applications on the chassis itself, >> in order to detect that the NB changes have been applied, it has to >> connect to the NB/SB database. In a scaled deployment this additional >> connection might induce performance issues. >> >> In order to avoid that we now (also) propagate nb_cfg to the local OVS >> DB, in table Open_vSwitch, as external_id "ovn-nb-cfg". >> >> Signed-off-by: Dumitru Ceara > > Hi Dumitru, Hi Numan, > > Thanks for the patch. Overall the patch LGTM. I have a couple of comments. Thanks for the review. > > 1. I think it needs to be documented that the ovn-controller writes > 'ovn-nb-cfg' to the local ovsdb. > 2. I think it's better if the ovn-controller writes the ovn-nb-cfg to > the integration bridge's external_ids column. >We already have Ihar's multiple ovn-controller support patch for > review and it would conflict for this usecase. Ack, good points, I'll address them in v2. Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] AUTHORS: Update Roi Dayan
On Wed, Nov 11, 2020 at 12:04:56PM +0200, Roi Dayan wrote: > Signed-off-by: Roi Dayan > --- > > Notes: > v1 > - fix .mailmap Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4 1/7] Add new table Load_Balancer in Southbound database.
On 11/12/20 12:54 PM, num...@ovn.org wrote:
> From: Numan Siddique
>
> This patch adds a new table 'Load_Balancer' in SB DB and syncs the
> Load_Balancer table rows
> from NB DB to SB DB. An upcoming patch will make use of this table for
> handling the
> load balancer hairpin traffic.
>
> Co-authored-by: Dumitru Ceara
> Signed-off-by: Dumitru Ceara
> Signed-off-by: Numan Siddique
> ---
Hi Numan,
[...]
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 26376c3671..aba9283d2a 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1914,4 +1914,91 @@ AT_CHECK(
>[ovn-nbctl --wait=sb set logical-switch-port lsp01
> options:requested-tnl-key=2])
> get_tunnel_keys
> AT_CHECK([test $lsp02 = 3 && test $ls1 = 123])
> +
> +AT_CLEANUP
> +
> +AT_SETUP([ovn -- NB to SB load balancer sync])
> +ovn_start
> +
> +check ovn-nbctl --wait=hv lb-add lb0 10.0.0.10:80 10.0.0.4:8080
Nit: this should be --wait=sb
> +check_row_count nb:load_balancer 1
> +
> +echo
> +echo "__file__:__line__: Check that there are no SB load balancer rows."
> +check_row_count sb:load_balancer 0
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl --wait=hv ls-lb-add sw0 lb0
Nit: this should be --wait=sb
> +sw0_sb_uuid=$(fetch_column datapath_binding _uuid external_ids:name=sw0)
> +
> +echo
> +echo "__file__:__line__: Check that there are is SB load balancer row for
> lb0."
Nit: s/are is/is one/
Thanks,
Dumitru
> +check_row_count sb:load_balancer 1
> +check_column "10.0.0.10:80=10.0.0.4:8080 tcp" sb:load_balancer vips,protocol
> name=lb0
> +
> +lb0_uuid=$(fetch_column sb:load_balancer _uuid name=lb0)
> +
> +echo
> +echo "__file__:__line__: Check that SB lb0 has sw0 in datapaths column."
> +
> +check_column "$sw0_sb_uuid" sb:load_balancer datapaths name=lb0
> +check_column "$lb0_uuid" sb:datapath_binding load_balancers
> external_ids:name=sw0
> +
> +check ovn-nbctl --wait=sb set load_balancer .
> vips:"10.0.0.20\:90"="20.0.0.4:8080,30.0.0.4:8080"
> +
> +echo
> +echo "__file__:__line__: Check that SB lb0 has vips and protocol columns are
> set properly."
> +
> +check_column "10.0.0.10:80=10.0.0.4:8080
> 10.0.0.20:90=20.0.0.4:8080,30.0.0.4:8080 tcp" \
> +sb:load_balancer vips,protocol name=lb0
> +
> +check ovn-nbctl lr-add lr0
> +check ovn-nbctl --wait=sb lr-lb-add lr0 lb0
> +
> +echo
> +echo "__file__:__line__: Check that SB lb0 has only sw0 in datapaths column."
> +check_column "$sw0_sb_uuid" sb:load_balancer datapaths name=lb0
> +
> +check ovn-nbctl ls-add sw1
> +check ovn-nbctl --wait=sb ls-lb-add sw1 lb0
> +sw1_sb_uuid=$(fetch_column datapath_binding _uuid external_ids:name=sw1)
> +
> +echo
> +echo "__file__:__line__: Check that SB lb0 has sw0 and sw1 in datapaths
> column."
> +check_column "$sw0_sb_uuid $sw1_sb_uuid" sb:load_balancer datapaths name=lb0
> +check_column "$lb0_uuid" sb:datapath_binding load_balancers
> external_ids:name=sw1
> +
> +check ovn-nbctl --wait=sb lb-add lb1 10.0.0.30:80 20.0.0.50:8080 udp
> +check_row_count sb:load_balancer 1
> +
> +check ovn-nbctl --wait=sb lr-lb-add lr0 lb1
> +check_row_count sb:load_balancer 1
> +
> +echo
> +echo "__file__:__line__: Associate lb1 to sw1 and check that lb1 is created
> in SB DB."
> +
> +check ovn-nbctl --wait=sb ls-lb-add sw1 lb1
> +check_row_count sb:load_balancer 2
> +
> +echo
> +echo "__file__:__line__: Check that SB lb1 has vips and protocol columns are
> set properly."
> +check_column "10.0.0.30:80=20.0.0.50:8080 udp" sb:load_balancer
> vips,protocol name=lb1
> +
> +lb1_uuid=$(fetch_column sb:load_balancer _uuid name=lb1)
> +
> +echo
> +echo "__file__:__line__: Check that SB lb1 has sw1 in datapaths column."
> +
> +check_column "$sw1_sb_uuid" sb:load_balancer datapaths name=lb1
> +
> +echo
> +echo "__file__:__line__: check that datapath sw1 has lb0 and lb1 set in the
> load_balancers column."
> +check_column "$lb0_uuid $lb1_uuid" sb:datapath_binding load_balancers
> external_ids:name=sw1
> +
> +echo
> +echo "__file__:__line__: Delete load balancer lb1 an check that datapath
> sw1's load_balancers are updated accordingly."
> +
> +ovn-nbctl --wait=sb lb-del lb1
> +check_column "$lb0_uuid" sb:datapath_binding load_balancers
> external_ids:name=sw1
> +
> AT_CLEANUP
> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> index 85e448ec04..00c112c7e5 100644
> --- a/utilities/ovn-sbctl.c
> +++ b/utilities/ovn-sbctl.c
> @@ -1441,6 +1441,9 @@ static const struct ctl_table_class
> tables[SBREC_N_TABLES] = {
>
> [SBREC_TABLE_GATEWAY_CHASSIS].row_ids[0]
> = {_gateway_chassis_col_name, NULL, NULL},
> +
> +[SBREC_TABLE_LOAD_BALANCER].row_ids[0]
> += {_load_balancer_col_name, NULL, NULL},
> };
>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn-controller: Propagate nb_cfg to the local OVS DB.
On Thu, Nov 12, 2020 at 9:25 PM Dumitru Ceara wrote:
>
> The NB.NB_Global.nb_cfg value gets propagated to
> Chassis_Private.nb_cfg (and then to NB.NB_Global.hv_cfg) as soon as
> ovn-controller has finished installing OVS flows corresponding to
> the NB DB state.
>
> However, if the CMS runs monitoring applications on the chassis itself,
> in order to detect that the NB changes have been applied, it has to
> connect to the NB/SB database. In a scaled deployment this additional
> connection might induce performance issues.
>
> In order to avoid that we now (also) propagate nb_cfg to the local OVS
> DB, in table Open_vSwitch, as external_id "ovn-nb-cfg".
>
> Signed-off-by: Dumitru Ceara
Hi Dumitru,
Thanks for the patch. Overall the patch LGTM. I have a couple of comments.
1. I think it needs to be documented that the ovn-controller writes
'ovn-nb-cfg' to the local ovsdb.
2. I think it's better if the ovn-controller writes the ovn-nb-cfg to
the integration bridge's external_ids column.
We already have Ihar's multiple ovn-controller support patch for
review and it would conflict for this usecase.
Thanks
Numan
> ---
> controller/ovn-controller.c | 53
> ++---
> tests/ovn-controller.at | 21 ++
> 2 files changed, 61 insertions(+), 13 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index a06cae3..e05afc2 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -85,6 +85,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
>
> #define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation"
>
> +#define OVS_NB_CFG_NAME "ovn-nb-cfg"
> +
> static char *parse_options(int argc, char *argv[]);
> OVS_NO_RETURN static void usage(void);
>
> @@ -733,6 +735,41 @@ get_nb_cfg(const struct sbrec_sb_global_table
> *sb_global_table)
> return sb ? sb->nb_cfg : 0;
> }
>
> +/* Propagates the local cfg seqno, 'cur_cfg', to the chassis_private record
> + * and to the local OVS DB.
> + */
> +static void
> +store_nb_cfg(struct ovsdb_idl_txn *sb_txn, struct ovsdb_idl_txn *ovs_txn,
> + const struct sbrec_chassis_private *chassis,
> + const struct ovsrec_open_vswitch *ovs_cfg,
> + unsigned int delay_nb_cfg_report,
> + int64_t cur_cfg)
> +{
> +if (!cur_cfg) {
> +return;
> +}
> +
> +if (sb_txn && chassis && cur_cfg != chassis->nb_cfg) {
> +sbrec_chassis_private_set_nb_cfg(chassis, cur_cfg);
> +sbrec_chassis_private_set_nb_cfg_timestamp(chassis,
> time_wall_msec());
> +
> +if (delay_nb_cfg_report) {
> +VLOG_INFO("Sleep for %u sec", delay_nb_cfg_report);
> +xsleep(delay_nb_cfg_report);
> +}
> +}
> +
> +if (ovs_txn && ovs_cfg &&
> +cur_cfg != smap_get_ullong(_cfg->external_ids,
> + OVS_NB_CFG_NAME, 0)) {
> +char *cur_cfg_str = xasprintf("%"PRId64, cur_cfg);
> +ovsrec_open_vswitch_update_external_ids_setkey(ovs_cfg,
> + OVS_NB_CFG_NAME,
> + cur_cfg_str);
> +free(cur_cfg_str);
> +}
> +}
> +
> static const char *
> get_transport_zones(const struct ovsrec_open_vswitch_table *ovs_table)
> {
> @@ -2632,19 +2669,9 @@ main(int argc, char *argv[])
> engine_set_force_recompute(false);
> }
>
> -if (ovnsb_idl_txn && chassis_private) {
> -int64_t cur_cfg = ofctrl_get_cur_cfg();
> -if (cur_cfg && cur_cfg != chassis_private->nb_cfg) {
> -sbrec_chassis_private_set_nb_cfg(chassis_private,
> cur_cfg);
> -sbrec_chassis_private_set_nb_cfg_timestamp(
> -chassis_private, time_wall_msec());
> -if (delay_nb_cfg_report) {
> -VLOG_INFO("Sleep for %u sec", delay_nb_cfg_report);
> -xsleep(delay_nb_cfg_report);
> -}
> -}
> -}
> -
> +store_nb_cfg(ovnsb_idl_txn, ovs_idl_txn, chassis_private,
> + ovsrec_open_vswitch_first(ovs_idl_loop.idl),
> + delay_nb_cfg_report, ofctrl_get_cur_cfg());
>
> if (pending_pkt.conn) {
> struct ed_type_addr_sets *as_data =
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 014a977..f672bc9 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -411,3 +411,24 @@ AT_CHECK([ovn-nbctl --timeout=1 --wait=hv sync])
>
> OVN_CLEANUP([hv])
> AT_CLEANUP
> +
> +AT_SETUP([ovn -- nb_cfg sync to OVS])
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +# Wait for ovn-controller to register in the SB.
> +wait_row_count Chassis 1
> +
> +#
Re: [ovs-dev] [RFC PATCH ovn v3 3/3] Add ipam unit tests
On Thu, Nov 12, 2020 at 7:24 PM Mark Gray wrote: > > On 02/11/2020 14:28, Mark Michelson wrote: > > This adds unit tests for IPAM IPv6 initialization, IPv4 initialization, > > and IPv4 address retrieval. It also adds testsuite tests corresponding > > to these unit tests. > > > > The IPv6 initialization and IPv4 initialization tests make use of the > > new unit test framework. They use ovn-appctl to get access to internal > > functions in northd/ipam.c. They require ENABLE_UNIT_TESTS to be > > defined, otherwise the internal unit test code will not be compiled in. > > > > The IPv4 address retrieval test makes use of the pre-existing ovstest > > utility. > > > > Signed-off-by: Mark Michelson > > --- > > northd/ipam.c | 56 ++ > > northd/ovn-northd.c | 3 + > > tests/automake.mk | 8 +- > > tests/ovn-unit-tests.at | 237 > > tests/testsuite.at | 1 + > > 5 files changed, 303 insertions(+), 2 deletions(-) > > create mode 100644 tests/ovn-unit-tests.at > > > > diff --git a/northd/ipam.c b/northd/ipam.c > > index e5383c46f..fb367700b 100644 > > --- a/northd/ipam.c > > +++ b/northd/ipam.c > > @@ -326,3 +326,59 @@ ipam_get_unused_mac(ovs_be32 ip) > > > > return mac64; > > } > > + > > +#ifdef ENABLE_UNIT_TESTS > > + > > > Thanks for looking at this further. IMHO I have a preference to not add > test code in production code. Admittedly, because this is just adding > text at the end of the file, it should not cause any problems at this > stage. However, this makes it too easy to add this #ifdef anywhere in > the file to do things like mocking out objects and this could start > getting messy as the test code paths and production code paths could > start to deviate. There are other patterns (like #include "file.c" and > others suggested by others on this list) that can get around the problem > of testing static functions in the file. > > However, I question the need to do this at all. If we had a need to > achieve some kind of code coverage target (in which we have to make sure > we test X% of all code paths) then we probably would need some method > like this in order to allow this. I think the main problem is the > difficultly testing large files (e.g northd.c, ovn-controller.c). If we > try to find a way to test these private static functions then we are > just covering up the problem that these large files need to be broken up > into something more modular. Therefore, I think we should be splitting > out logical functionality (like you did with ipam.c), defining a > "public" interface, testing against that interface which follows the > pattern in OVS. If we can't achieve our goals with this, then maybe we > should look at methods like this. I would agree with Mark G. Thanks Numan > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] netdev-offload-tc: Use single 'once' variable for probing tc features
On Wed, Nov 11, 2020 at 12:07:55PM +0200, Roi Dayan wrote: > There is no need for a 'once' variable per probe. > > Signed-off-by: Roi Dayan > Reviewed-by: Paul Blakey > --- > > Notes: > v1 > - add blank space for logic separation > - update commit msg Thanks, pushed to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2] Allow explicit setting of the SNAT zone on a gateway router.
On Thu, Nov 12, 2020 at 8:26 PM Mark Michelson wrote:
>
> In certain situations, OVN may coexist with other applications on a
> host. Traffic from OVN and the other applications may then go out a
> shared gateway. If OVN traffic and the other application traffic use
> different conntrack zones for SNAT, then it is possible for the shared
> gateway to assign conflicting source IP:port combinations. By sharing
> the same conntrack zone, there will be no conflicting assignments.
>
> In this commit, we introduce options:snat-ct-zone for northbound logical
> routers. By setting this option, users can explicitly set the conntrack
> zone for the logical router so that it will match the zone used by
> non-OVN traffic on the host.
>
> The biggest side effects of this patch are:
> 1) southbound datapath changes now result in recalculating CT zones in
>ovn-controller. This can result in recomputing physical flows in more
>situations than previously.
> 2) The table 65 flow to transition between datapaths is no longer
>associated with a port binding. This is because the flow refers to
>the peer datapath's CT zones, which can now be updated due to changes
>on that datapath. The flow therefore may need to be updated either
>due to the port binding being changed or the peer datapath being
>changed.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1892311
> Signed-off-by: Mark Michelson
Hi Mark,
Thanks for the patch.
I did some testing with the below logical resources
**
switch 0ff55345-e283-4ed7-8c7f-8475a8a8f968 (sw1)
port sw1-lr0
type: router
addresses: ["00:00:00:00:ff:02"]
router-port: lr0-sw1
port sw1-port2
addresses: ["40:54:00:00:00:04 20.0.0.4"]
switch bddcdd29-9b32-482a-b9a6-0f9fa1a0e25a (sw0)
port sw0-lr0
type: router
addresses: ["00:00:00:00:ff:01"]
router-port: lr0-sw0
port sw0-port1
addresses: ["50:54:00:00:00:03 10.0.0.3"]
port sw0-lr1
type: router
router-port: lr1-sw0
router 9462469c-d799-4d28-bccd-80e9a8c68e24 (lr1)
port lr1-sw0
mac: "00:00:00:00:fa:21"
networks: ["10.0.0.20/24"]
router 96b14269-65ad-4ae9-9fa6-e4ef2a344a54 (lr0)
port lr0-sw1
mac: "00:00:00:00:ff:02"
networks: ["20.0.0.1/24"]
port lr0-public
mac: "00:00:20:20:12:13"
networks: ["172.168.0.100/24"]
gateway chassis: [chassis-1]
*
If I set - "ovn-nbctl set logical_router lr0 options:snat-ct-zone=x"
and If 'x' is already allocated for sw0 snat, then the
both sw0 snat and lr0 snat share the same zone id. Is that expected ?
If I set the same zone id 'y' for both lr0 and lr1, then I observed
that both lr0 and lr1 have the same zone id 'y' and I also
see the warning in the ovn-controller log.
Later if I change the zone of lr1 to 'z' and then lr0 to 'z', the lr0
is not updated with 'z'. So lr0 will have 'y' and lr1 will have 'z'.
Seems like it's not consistent.
Can there be a possibility that there are 2 shared gateways in one
node and both want to share the same zone id ?
i.e lr0 is associated with one shared gateway and lr1 with another
shared gateway (on the same host) and both want to use the zone id 0 ?
Thanks
Numan
> ---
> controller/ovn-controller.c | 89 +++-
> controller/physical.c | 2 +-
> lib/ovn-util.c | 7 +++
> lib/ovn-util.h | 1 +
> northd/ovn-northd.c | 10
> ovn-nb.xml | 7 +++
> tests/ovn.at| 91 +
> 7 files changed, 194 insertions(+), 13 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index a06cae3cc..54c8fd2db 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -531,6 +531,21 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl
> *ovnsb_idl,
> }
> }
>
> +static void
> +add_pending_ct_zone_entry(struct shash *pending_ct_zones,
> + enum ct_zone_pending_state state,
> + int zone, bool add, const char *name)
> +{
> +VLOG_DBG("%s ct zone %"PRId32" for '%s'",
> + add ? "assigning" : "removing", zone, name);
> +
> +struct ct_zone_pending_entry *pending = xmalloc(sizeof *pending);
> +pending->state = state; /* Skip flushing zone. */
> +pending->zone = zone;
> +pending->add = add;
> +shash_add(pending_ct_zones, name, pending);
> +}
> +
> static void
> update_ct_zones(const struct sset *lports, const struct hmap
> *local_datapaths,
> struct simap *ct_zones, unsigned long *ct_zone_bitmap,
> @@ -540,6 +555,7 @@ update_ct_zones(const struct sset *lports, const struct
> hmap *local_datapaths,
> int scan_start = 1;
> const char *user;
> struct sset all_users = SSET_INITIALIZER(_users);
> +struct simap req_snat_zones = SIMAP_INITIALIZER(_snat_zones);
>
>
