Re: [ovs-dev] [PATCH] ovs-ctl: Don't set hostname as external-id
On Thu, May 21, 2020 at 3:19 AM Daniel Alvarez Sanchez
wrote:
>
> On Thu, May 21, 2020 at 9:22 AM Han Zhou wrote:
> >
> >
> >
> > On Wed, May 20, 2020 at 8:52 AM Daniel Alvarez
wrote:
> > >
> > > ovs-ctl started to add the hostname as external-id [0] at some point.
> > >
> > > However, this can be problematic as if it's already set by an external
> > > entity it will get overwritten. In RHEL systems, systemd will invoke
> > > ovs-ctl to start OVS and that will overwrite it to the hostname of the
> > > machine.
> > >
> > > For OVN this can have a big impact because if, for whatever reason the
> > > hostname changes and the host gets restarted, ovn-controller won't
> > > claim the ports back leaving the workloads unaccessible.
> > >
> > > Also, it makes sense to leave this untouched as 1) it's an
external_id,
> > > so it will actually let external entities to configure it (unlike
now),
> > > and 2) it's optional. In the case of OVN, if the external-id doesn't
> > > exist, it'll default to its hostname so nothing should get broken by
> > > this change.
> > >
> > > [0]
https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/312054.html
> > >
> > > Signed-off-by: Daniel Alvarez
> > > ---
> > > utilities/ovs-ctl.in | 12
> > > 1 file changed, 12 deletions(-)
> > >
> > > diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> > > index 8c5cd7032..87fc4934f 100644
> > > --- a/utilities/ovs-ctl.in
> > > +++ b/utilities/ovs-ctl.in
> > > @@ -35,17 +35,6 @@ insert_mod_if_required () {
> > > ovs_kmod_ctl insert
> > > }
> > >
> > > -set_hostname () {
> > > -# 'hostname -f' needs network connectivity to work. So we should
> > > -# call this only after ovs-vswitchd is running.
> > > -if test X$FULL_HOSTNAME = Xyes; then
> > > -hn="$(hostname -f)" || hn="$(uname -n)"
> > > -else
> > > -hn="$(uname -n)"
> > > -fi
> > > -ovs_vsctl set Open_vSwitch . external-ids:hostname="$hn"
> > > -}
> > > -
> > > set_system_ids () {
> > > set ovs_vsctl set Open_vSwitch .
> > >
> > > @@ -225,7 +214,6 @@ start_forwarding () {
> > > if test X"$OVS_VSWITCHD" = Xyes; then
> > > do_start_forwarding || return 1
> > > fi
> > > -set_hostname &
> > > return 0
> > > }
> > >
> > > --
> > >
> > > ___
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> > Hi Daniel,
> >
> > I believe there are OpenStack based OVN users already depends on this.
(And we had also add the --no-full-hostname option so that it will set the
short name, so that it matches with openstack's "requested-chassis" setting
which uses nova compute node name.) For the scenarios that this behavior is
not desired, I think it is better to add a new option to override it, such
as "--no-hostname", so that existing environment won't get impacted. What
do you think?
>
> Hi Han, thanks a lot for your feedback!
> I thought of adding a --no-hostname option. However I still see that
> this patch has value. Let me try to explain.
>
> Existing OpenStack deployments will have their 'requested-chassis' set
> to the current name of the hosts at the time the VMs were created.
> This hostname may or may not match the machine hostname as it's a
> string consumed from the external_ids, hence external and expected to
> be set by CMS (for example, it's configurable by puppet at OpenStack
> deployment time).
> Now let's imagine you restart one node so ovs-ctl will overwrite
> whatever the CMS relied upon and set it to the machine hostname. From
> this time on, the workloads on that VM will not be claimed by OVN and
> are left inaccessible until manual intervention happens.
>
> I think it's fundamentally wrong for OVS (ovs-ctl in this case) to set
> an external id itself as its default behavior (as they're meant for
> external entities IIUC). Can you please elaborate on how existing
> environments can get impacted by this change? Also keep in mind that
> if the external_id is not set, ovn-controller is taking the hostname
> of the machine as the default [0]. I might be overlooking something,
> for sure.
>
> Thanks again!
> Daniel
>
> [0] https://github.com/ovn-org/ovn/blob/master/controller/chassis.c#L133
>
> >
> > Thanks,
> > Han
>
Hi Daniel, I understand the problem scenario you mentioned, and I agree
maybe it wasn't a good idea to add the external-ids:hostname by default. My
only concern is that it is already there and deployment may depend on it.
For example, if it is removed, a new HV onboarding will have no hostname,
and the --no-full-hostname setting doesn't take any effect anymore, so as
you mentioned the default behavior is to get the hostname by
ovn-controller. Since ovn-controller has no idea is the short or long name
should be used, it may be just the long name, which doesn't match with the
OpenStack nova convention that uses short name as the compute node name,
which is used as
Re: [ovs-dev] [PATCH ovn] controller: fix ip buffering with static routes
Hi Lorenzo,
Good catch.
a. If not already considered, then i think it is a candidate for 20.06
b. Need not be done in this patch, but it will be good to have a testcase which
validates the draining of buffered packets.
c. Not sure about the commit header. I mean this issue is not specific to
static route right? By default there will be a gateway and destination ip will
not be that of gateway ip.
That's a regular NS workflow.
Acked-by: Ankur Sharma
Regards,
Ankur
From: dev on behalf of Lorenzo Bianconi
Sent: Thursday, May 21, 2020 6:46 AM
To: ovs-dev@openvswitch.org
Subject: [ovs-dev] [PATCH ovn] controller: fix ip buffering with static routes
When the ARP request is sent to a gw router and not to the final
destination of the packet buffered_packets_map needs to be updated using
next-hop IP address and not the destination one.
Fixes: 2e5cdb4b1392 ("OVN: add buffering support for ip packets")
Signed-off-by: Lorenzo Bianconi
---
controller/pinctrl.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bea446c89..bb90edd1f 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -1381,8 +1381,7 @@ pinctrl_find_buffered_packets(const struct in6_addr *ip,
uint32_t hash)
/* Called with in the pinctrl_handler thread context. */
static int
-pinctrl_handle_buffered_packets(const struct flow *ip_flow,
-struct dp_packet *pkt_in,
+pinctrl_handle_buffered_packets(struct dp_packet *pkt_in,
const struct match *md, bool is_arp)
OVS_REQUIRES(pinctrl_mutex)
{
@@ -1391,9 +1390,10 @@ pinctrl_handle_buffered_packets(const struct flow
*ip_flow,
struct in6_addr addr;
if (is_arp) {
-addr = in6_addr_mapped_ipv4(ip_flow->nw_dst);
+addr = in6_addr_mapped_ipv4(htonl(md->flow.regs[0]));
} else {
-addr = ip_flow->ipv6_dst;
+ovs_be128 ip6 = hton128(flow_get_xxreg(>flow, 0));
+memcpy(, , sizeof addr);
}
uint32_t hash = hash_bytes(, sizeof addr, 0);
@@ -1434,7 +1434,7 @@ pinctrl_handle_arp(struct rconn *swconn, const struct
flow *ip_flow,
}
ovs_mutex_lock(_mutex);
-pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, true);
+pinctrl_handle_buffered_packets(pkt_in, md, true);
ovs_mutex_unlock(_mutex);
/* Compose an ARP packet. */
@@ -5281,7 +5281,7 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct
flow *ip_flow,
}
ovs_mutex_lock(_mutex);
-pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, false);
+pinctrl_handle_buffered_packets(pkt_in, md, false);
ovs_mutex_unlock(_mutex);
uint64_t packet_stub[128 / 8];
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=s883GpUCOChKOHiocYtGcg=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY=M_tzprKGSufeFSzeZOYjY5JUCnecaZWqQmYWNJazeiY=NQAYRfJ3Svolg8UWgwkkDxAH8FTT4PKayFV7Kw--fN8=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-vport: Fix typo in log message.
On Thu, May 21, 2020 at 02:56:28PM -0700, Gregory Rose wrote:
>
> On 5/21/2020 2:16 PM, Ben Pfaff wrote:
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/netdev-vport.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> > index 8efd1eee8302..0252b61dea2b 100644
> > --- a/lib/netdev-vport.c
> > +++ b/lib/netdev-vport.c
> > @@ -754,7 +754,7 @@ set_tunnel_config(struct netdev *dev_, const struct
> > smap *args, char **errp)
> > enum tunnel_layers layers = tunnel_supported_layers(type, _cfg);
> > const char *full_type = (strcmp(type, "vxlan") ? type
> >: (tnl_cfg.exts & (1 << OVS_VXLAN_EXT_GPE)
> > -? "VXLAN-GPE" : "VXLAN (without GPE"));
> > +? "VXLAN-GPE" : "VXLAN (without GPE)"));
> > const char *packet_type = smap_get(args, "packet_type");
> > if (!packet_type) {
> > tnl_cfg.pt_mode = default_pt_mode(layers);
> >
>
> Acked-by: Greg Rose
Thanks. I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather implementation
On Thu, May 21, 2020 at 6:04 AM Van Haaren, Harry wrote: > > > -Original Message- > > From: William Tu > > Sent: Wednesday, May 20, 2020 4:15 PM > > To: Van Haaren, Harry > > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org > > Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather > > implementation > > > > > > 2020-05-20T14:15:20.184Z|00378|dpif_netdev(pmd-c00/id:9)|INFO|reprobing > > > sub func, 4 1 > > > 2020-05-20T14:15:21.219Z|00379|dpif_netdev(pmd-c00/id:9)|INFO|reprobing > > > sub func, 4 1 > > > > > btw, looking at > > ovs-appctl coverage/show, this counter is very high when enabling the avx512 > > handler_duplicate_upcall 459645.4/sec 434475.500/sec > > 17300.5372/sec total: 64120526 > > This counter seems to post some garbage to me if I run it before any traffic? > Tested using OVS Master @ 48b1c7642 (not including any AVX512 patches): > > ./utilities/ovs-appctl coverage/show | grep duplicate_upcall > 21:handler_duplicate_upcall 0.0/sec 0.000/sec0./sec > total: 10272710751479363764 > > # re-runs show different numbers - indicates a garbage-initialized counter > perhaps? > 21:handler_duplicate_upcall 0.0/sec 0.000/sec0./sec > total: 1049338714623956653 > 21:handler_duplicate_upcall 0.0/sec 0.000/sec0./sec > total: 18343161283719775679 > using the same pcap traffic (p0.pcap) on current master, I did not see the above issue: datapath_drop_upcall_error 57.4/sec 4.783/sec0.0797/sec total: 287 drop_action_of_pipeline 5909696.2/sec 492474.683/sec 8207.9114/sec total: 52399553 William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather implementation
> > ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev > > ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x3 > > ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true > > ovs-ofctl add-flow br0 'actions=drop' > > ovs-appctl dpif-netdev/subtable-lookup-set avx512_gather 5 > > ovs-vsctl add-port br0 tg0 -- set int tg0 type=dpdk \ > > options:dpdk- > > devargs=vdev:net_pcap0,rx_pcap=/root/ovs/p0.pcap,infinite_rx=1 > > I use Ether/VLAN/IPv4 to achieve a subtable with (4,1), is that the same as > you? > Just trying to remove variables between our setups. > btw I have only one OpenFlow rule, 'actions=drop' The pcap file as input is a random one I just capture in my machine's interface root@instance-3:~/ovs# tcpdump -n -r p0.pcap | wc -l reading from file p0.pcap, link-type EN10MB (Ethernet) 22 root@instance-3:~/ovs# tcpdump -n -r p0.pcap reading from file p0.pcap, link-type EN10MB (Ethernet) 22:30:10.471943 IP 10.182.0.2.22 > 76.21.95.192.62190: Flags [P.], seq 3532581039:3532581163, ack 2971134033, win 501, options [nop,nop,TS val 521819346 ecr 304440082], length 124 22:30:10.499759 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [.], ack 124, win 4092, options [nop,nop,TS val 304440141 ecr 521819346], length 0 22:30:13.242821 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [P.], seq 1:37, ack 124, win 4096, options [nop,nop,TS val 304442869 ecr 521819346], length 36 22:30:13.243113 IP 10.182.0.2.22 > 76.21.95.192.62190: Flags [P.], seq 124:160, ack 37, win 501, options [nop,nop,TS val 521822117 ecr 304442869], length 36 22:30:13.271718 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [.], ack 160, win 4094, options [nop,nop,TS val 304442900 ecr 521822117], length 0 22:30:13.415212 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [P.], seq 37:73, ack 160, win 4096, options [nop,nop,TS val 304443043 ecr 521822117], length 36 22:30:13.415479 IP 10.182.0.2.22 > 76.21.95.192.62190: Flags [P.], seq 160:196, ack 73, win 501, options [nop,nop,TS val 521822289 ecr 304443043], length 36 22:30:13.442371 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [.], ack 196, win 4094, options [nop,nop,TS val 304443069 ecr 521822289], length 0 22:30:13.577866 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [P.], seq 73:109, ack 196, win 4096, options [nop,nop,TS val 304443208 ecr 521822289], length 36 22:30:13.578123 IP 10.182.0.2.22 > 76.21.95.192.62190: Flags [P.], seq 196:232, ack 109, win 501, options [nop,nop,TS val 521822452 ecr 304443208], length 36 22:30:13.608249 IP 76.21.95.192.62190 > 10.182.0.2.22: Flags [.], ack 232, win 4094, options [nop,nop,TS val 304443230 ecr 521822452], length 0 22:30:16.932478 IP 169.254.169.254.80 > 10.182.0.2.51642: Flags [P.], seq 1150154089:1150154672, ack 1477571123, win 65535, length 583: HTTP: HTTP/1.1 200 OK 22:30:16.932540 IP 10.182.0.2.51642 > 169.254.169.254.80: Flags [.], ack 583, win 64737, length 0 22:30:16.932547 IP 169.254.169.254.80 > 10.182.0.2.51642: Flags [F.], seq 583, ack 1, win 65535, length 0 22:30:16.933193 IP 10.182.0.2.51642 > 169.254.169.254.80: Flags [F.], seq 1, ack 584, win 64736, length 0 22:30:16.933280 IP 169.254.169.254.80 > 10.182.0.2.51642: Flags [.], ack 2, win 65535, length 0 22:30:16.936976 IP 10.182.0.2.51650 > 169.254.169.254.80: Flags [S], seq 1944213115, win 65320, options [mss 1420,sackOK,TS val 2204263930 ecr 0,nop,wscale 7], length 0 22:30:16.937201 IP 169.254.169.254.80 > 10.182.0.2.51650: Flags [S.], seq 4118061879, ack 1944213116, win 65535, options [mss 1420,eol], length 0 22:30:16.937234 IP 10.182.0.2.51650 > 169.254.169.254.80: Flags [.], ack 1, win 65320, length 0 22:30:16.937297 IP 10.182.0.2.51650 > 169.254.169.254.80: Flags [P.], seq 1:287, ack 1, win 65320, length 286: HTTP: GET /computeMetadata/v1/instance/network-interfaces/?alt=json_etag=7c556bc02e6331f4=True_sec=72_for_change=True HTTP/1.1 22:30:16.937374 IP 169.254.169.254.80 > 10.182.0.2.51650: Flags [.], ack 287, win 65249, length 0 22:30:16.937428 IP 169.254.169.254.80 > 10.182.0.2.51650: Flags [.], ack 287, win 65535, length 0 I also attached the pcap file. https://drive.google.com/file/d/1CR5pMebrtjzShF9bpXJcr9GAQY_6Og44/view?usp=sharing > > LOG: > > 2020-05-20T13:49:26.542Z|00047|dpdk|INFO|DPDK Enabled - initialized > > 2020-05-20T13:49:26.544Z|00048|connmgr|INFO|br0<->unix#2: 1 flow_mods > > in the last 0 s (1 adds) > > 2020-05-20T13:49:26.547Z|00049|dpif_netdev_lookup|INFO|Subtable > > function 'avx512_gather' set priority to 5 > > 2020-05-20T13:49:26.553Z|00050|netdev_dpdk|INFO|Device > > 'vdev:net_pcap0,rx_pcap=/root/ovs/p0.pcap,infinite_rx=1' attached to > > DPDK > > 2020-05-20T13:49:26.553Z|00051|dpif_netdev|INFO|PMD thread on numa_id: > > 0, core id: 0 created. > > 2020-05-20T13:49:26.554Z|00052|dpif_netdev|INFO|PMD thread on numa_id: > > 0, core id: 1 created. > > 2020-05-20T13:49:26.554Z|00053|dpif_netdev|INFO|There are 2 pmd > > threads on numa node 0 > > 2020-05-20T13:49:26.554Z|00054|dpdk|INFO|Device with port_id=0 already > > stopped > >
Re: [ovs-dev] [PATCH] netdev-vport: Fix typo in log message.
On 5/21/2020 2:16 PM, Ben Pfaff wrote:
Signed-off-by: Ben Pfaff
---
lib/netdev-vport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 8efd1eee8302..0252b61dea2b 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -754,7 +754,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap
*args, char **errp)
enum tunnel_layers layers = tunnel_supported_layers(type, _cfg);
const char *full_type = (strcmp(type, "vxlan") ? type
: (tnl_cfg.exts & (1 << OVS_VXLAN_EXT_GPE)
-? "VXLAN-GPE" : "VXLAN (without GPE"));
+? "VXLAN-GPE" : "VXLAN (without GPE)"));
const char *packet_type = smap_get(args, "packet_type");
if (!packet_type) {
tnl_cfg.pt_mode = default_pt_mode(layers);
Acked-by: Greg Rose
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] compat: Backport ipv6_stub change
A patch backported to the Linux stable 4.14 tree and present in the
latest stable 4.14.181 kernel breaks ipv6_stub usage.
The commit is
8ab8786f78c3 ("net ipv6_stub: use ip6_dst_lookup_flow instead of
ip6_dst_lookup").
Create the compat layer define to check for it and fixup usage in vxlan
and geneve modules.
Passes Travis here:
https://travis-ci.org/github/gvrose8192/ovs-experimental/builds/689798733
Signed-off-by: Greg Rose
---
acinclude.m4 | 2 ++
datapath/linux/compat/geneve.c | 11 ++-
datapath/linux/compat/vxlan.c | 18 +-
3 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index dabbffd..3b0eea0 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -587,6 +587,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
OVS_GREP_IFELSE([$KSRC/include/net/ip6_fib.h], [rt6_get_cookie],
[OVS_DEFINE([HAVE_RT6_GET_COOKIE])])
+ OVS_FIND_FIELD_IFELSE([$KSRC/include/net/addrconf.h], [ipv6_stub],
+[dst_entry])
OVS_GREP_IFELSE([$KSRC/include/net/addrconf.h], [ipv6_dst_lookup.*net],
[OVS_DEFINE([HAVE_IPV6_DST_LOOKUP_NET])])
OVS_GREP_IFELSE([$KSRC/include/net/addrconf.h], [ipv6_dst_lookup_flow.*net],
diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/geneve.c
index 7bfc6d8..02c6403 100644
--- a/datapath/linux/compat/geneve.c
+++ b/datapath/linux/compat/geneve.c
@@ -962,7 +962,16 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff
*skb,
return dst;
}
-#if defined(HAVE_IPV6_DST_LOOKUP_FLOW_NET)
+#if defined(HAVE_IPV6_STUB_WITH_DST_ENTRY) &&
defined(HAVE_IPV6_DST_LOOKUP_FLOW)
+#ifdef HAVE_IPV6_DST_LOOKUP_FLOW_NET
+ dst = ipv6_stub->ipv6_dst_lookup_flow(geneve->net, gs6->sock->sk, fl6,
+ NULL);
+#else
+ dst = ipv6_stub->ipv6_dst_lookup_flow(gs6->sock->sk, fl6,
+ NULL);
+#endif
+ if (IS_ERR(dst)) {
+#elif defined(HAVE_IPV6_DST_LOOKUP_FLOW_NET)
if (ipv6_stub->ipv6_dst_lookup_flow(geneve->net, gs6->sock->sk, ,
fl6)) {
#elif defined(HAVE_IPV6_DST_LOOKUP_FLOW)
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index b334870..e65d955 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -967,7 +967,10 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev
*vxlan,
bool use_cache = (dst_cache && ip_tunnel_dst_cache_usable(skb, info));
struct dst_entry *ndst;
struct flowi6 fl6;
+#if !defined(HAVE_IPV6_STUB_WITH_DST_ENTRY) || \
+!defined(HAVE_IPV6_DST_LOOKUP_FLOW)
int err;
+#endif
if (!sock6)
return ERR_PTR(-EIO);
@@ -990,7 +993,15 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev
*vxlan,
fl6.fl6_dport = dport;
fl6.fl6_sport = sport;
-#if defined(HAVE_IPV6_DST_LOOKUP_FLOW_NET)
+#if defined(HAVE_IPV6_STUB_WITH_DST_ENTRY) &&
defined(HAVE_IPV6_DST_LOOKUP_FLOW)
+#ifdef HAVE_IPV6_DST_LOOKUP_FLOW_NET
+ ndst = ipv6_stub->ipv6_dst_lookup_flow(vxlan->net, sock6->sock->sk,
+ , NULL);
+#else
+ ndst = ipv6_stub->ipv6_dst_lookup_flow(sock6->sock->sk, , NULL);
+#endif
+ if (unlikely(IS_ERR(ndst))) {
+#elif defined(HAVE_IPV6_DST_LOOKUP_FLOW_NET)
err = ipv6_stub->ipv6_dst_lookup_flow(vxlan->net, sock6->sock->sk,
, );
#elif defined(HAVE_IPV6_DST_LOOKUP_FLOW)
@@ -1004,8 +1015,13 @@ static struct dst_entry *vxlan6_get_route(struct
vxlan_dev *vxlan,
#else
err = ip6_dst_lookup(vxlan->vn6_sock->sock->sk, , );
#endif
+#if defined(HAVE_IPV6_STUB_WITH_DST_ENTRY) &&
defined(HAVE_IPV6_DST_LOOKUP_FLOW)
+ return ERR_PTR(-ENETUNREACH);
+ }
+#else
if (err < 0)
return ERR_PTR(err);
+#endif
*saddr = fl6.saddr;
if (use_cache)
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] netdev-vport: Fix typo in log message.
Signed-off-by: Ben Pfaff
---
lib/netdev-vport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 8efd1eee8302..0252b61dea2b 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -754,7 +754,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap
*args, char **errp)
enum tunnel_layers layers = tunnel_supported_layers(type, _cfg);
const char *full_type = (strcmp(type, "vxlan") ? type
: (tnl_cfg.exts & (1 << OVS_VXLAN_EXT_GPE)
-? "VXLAN-GPE" : "VXLAN (without GPE"));
+? "VXLAN-GPE" : "VXLAN (without GPE)"));
const char *packet_type = smap_get(args, "packet_type");
if (!packet_type) {
tnl_cfg.pt_mode = default_pt_mode(layers);
--
2.25.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5] AB bonding: Add "primary" interface concept
Bleep bloop. Greetings Jeff Squyres, I am a robot and I have tried out your
patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
ERROR: Inappropriate bracing around statement
#113 FILE: ofproto/bond.c:490:
else {
Lines checked: 481, Warnings: 0, Errors: 1
Please check this out. If you feel there has been an error, please email
acon...@redhat.com
Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v7 2/9] ovn-controller: I-P for SB port binding and OVS interface in runtime_data.
On Thu, May 21, 2020 at 3:49 AM Numan Siddique wrote:
>
>
>
> On Thu, May 21, 2020 at 6:57 AM Han Zhou wrote:
>>
>> Hi Numan, please see my comments below.
>
>
> Thanks for the review. Please see below for a few comments.
>
>
>>
>>
>> On Wed, May 20, 2020 at 12:41 PM wrote:
>> >
>> > From: Numan Siddique
>> >
>> > This patch handles SB port binding changes and OVS interface changes
>> > incrementally in the runtime_data stage which otherwise would have
>> > resulted in calls to binding_run().
>> >
>> > Prior to this patch, port binding changes were handled differently.
>> > The changes were only evaluated to see if they need full recompute
>> > or they can be ignored.
>> >
>> > With this patch, the runtime data is updated with the changes (both
>> > SB port binding and OVS interface) and ports are claimed/released in
>> > the handlers itself, avoiding the need to trigger recompute of runtime
>> > data stage.
>> >
>> > Acked-by: Mark Michelson
>> > Signed-off-by: Numan Siddique
>> > ---
>> > controller/binding.c| 906 +++-
>> > controller/binding.h| 9 +-
>> > controller/ovn-controller.c | 61 ++-
>> > tests/ovn-performance.at| 13 +
>> > 4 files changed, 855 insertions(+), 134 deletions(-)
>> >
>> > diff --git a/controller/binding.c b/controller/binding.c
>> > index 2997fcae8..d5e8e4773 100644
>> > --- a/controller/binding.c
>> > +++ b/controller/binding.c
>> > @@ -360,17 +360,6 @@ destroy_qos_map(struct hmap *qos_map)
>> > hmap_destroy(qos_map);
>> > }
>> >
>> > -static void
>> > -update_local_lport_ids(struct sset *local_lport_ids,
>> > - const struct sbrec_port_binding *binding_rec)
>> > -{
>> > -char buf[16];
>> > -snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>> > - binding_rec->datapath->tunnel_key,
>> > - binding_rec->tunnel_key);
>> > -sset_add(local_lport_ids, buf);
>> > -}
>> > -
>> > /*
>> > * Get the encap from the chassis for this port. The interface
>> > * may have an external_ids:encap-ip= set; if so we
>> > @@ -449,10 +438,10 @@ is_network_plugged(const struct
sbrec_port_binding
>> *binding_rec,
>> > }
>> >
>> > static void
>> > -consider_localnet_port(const struct sbrec_port_binding *binding_rec,
>> > - struct shash *bridge_mappings,
>> > - struct sset *egress_ifaces,
>> > - struct hmap *local_datapaths)
>> > +update_ld_localnet_port(const struct sbrec_port_binding *binding_rec,
>> > +struct shash *bridge_mappings,
>> > +struct sset *egress_ifaces,
>> > +struct hmap *local_datapaths)
>> > {
>> > /* Ignore localnet ports for unplugged networks. */
>> > if (!is_network_plugged(binding_rec, bridge_mappings)) {
>> > @@ -482,6 +471,28 @@ consider_localnet_port(const struct
>> sbrec_port_binding *binding_rec,
>> > ld->localnet_port = binding_rec;
>> > }
>> >
>> > +static void
>> > +update_local_lport_ids(struct sset *local_lport_ids,
>> > + const struct sbrec_port_binding *binding_rec)
>> > +{
>> > +char buf[16];
>> > +snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>> > + binding_rec->datapath->tunnel_key,
>> > + binding_rec->tunnel_key);
>> > +sset_add(local_lport_ids, buf);
>> > +}
>> > +
>> > +static void
>> > +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec,
>> > + struct sset *local_lport_ids)
>> > +{
>> > +char buf[16];
>> > +snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>> > + binding_rec->datapath->tunnel_key,
>> > + binding_rec->tunnel_key);
>> > +sset_find_and_delete(local_lport_ids, buf);
>> > +}
>> > +
>> > /* Local bindings. binding.c module binds the logical port
(represented
>> by
>> > * Port_Binding rows) and sets the 'chassis' column when it is sees
the
>> > * OVS interface row (of type "" or "internal") with the
>> > @@ -599,6 +610,14 @@ local_bindings_destroy(struct shash
*local_bindings)
>> > shash_destroy(local_bindings);
>> > }
>> >
>> > +static
>> > +void local_binding_delete(struct shash *local_bindings,
>> > + struct local_binding *lbinding)
>> > +{
>> > +shash_find_and_delete(local_bindings, lbinding->name);
>> > +local_binding_destroy(lbinding);
>> > +}
>> > +
>> > static void
>> > local_binding_add_child(struct local_binding *lbinding,
>> > struct local_binding *child)
>> > @@ -625,6 +644,7 @@ is_lport_container(const struct sbrec_port_binding
>> *pb)
>> > return !pb->type[0] && pb->parent_port && pb->parent_port[0];
>> > }
>> >
>> > +/* Corresponds to each Port_Binding.type. */
>> > enum en_lport_type {
>> > LP_UNKNOWN,
>> > LP_VIF,
>> > @@ -670,12 +690,20 @@ get_lport_type(const struct
Re: [ovs-dev] ovn-git mailing list?
On Thu, May 21, 2020 at 01:35:35PM -0400, Ihar Hrachyshka wrote: > On 5/20/20 12:40 PM, Ben Pfaff wrote: > > On Mon, May 18, 2020 at 12:10:25PM -0400, Ihar Hrachyshka wrote: > > > We have ovs-git mailing list that tracks all commits merged into ovs tree. > > > Now that ovn is out of the repo, should there also be an ovn-git mailing > > > list, or at least make ovs-git bot track ovn tree too? > > Seems reasonable. > > > > I turned on notifications to the same mailing list. > > > > New mailing lists make sense but we have not worked out the > > administration for them. No one wants to manage mailing lists anymore. > > > Thank you. There was a ovn patch merged and we received an email from the > bot, so it works! Perfect. I hadn't had a chance to test it and I'm glad to hear that it works. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5] AB bonding: Add "primary" interface concept
In AB bonding, if the current active slave becomes disabled, a
replacement slave is arbitrarily picked from the remaining set of
enabled slaves. This commit adds the concept of a "primary" slave: an
interface that will always be (or become) the current active slave if
it is enabled.
The rationale for this functionality is to allow the designation of a
preferred interface for a given bond. For example:
1. Bond is created with interfaces p1 (primary) and p2, both enabled.
2. p1 becomes the current active slave (because it was designated as
the primary).
3. Later, p1 fails/becomes disabled.
4. p2 is chosen to become the current active slave.
5. Later, p1 becomes re-enabled.
6. p1 is chosen to become the current active slave (because it was
designated as the primary)
Note that p1 becomes the active slave once it becomes re-enabled, even
if nothing has happened to p2.
This "primary" concept exists in Linux kernel network interface
bonding, but did not previously exist in OVS bonding.
Only one primary slave inteface is supported per bond, and is only
supported for active/backup bonding.
The primary slave interface is designated via
"other_config:bond-primary" when creating a bond.
Signed-off-by: Jeff Squyres
Reviewed-by: Aaron Conole
Tested-by: Greg Rose
Acked-by: Greg Rose
---
v5:
- Handle when bond is reconfigured to remove "bond-primary" config.
- Fix potential flapping between remaining slaves.
- Add a test to re-add a removed primary interface and make sure the
bond reflects that properly.
- Add a test to remove "bond-primary" config and make sure the bond
reflects that properly.
v4:
- Style: bleep bloop. Trim line length below 79 characters.
v3:
- Adjusted print logic for when the primary slave is not attached
v2:
- Added "ovs-vsctl bond/show" label when primary interface is no
longer enslaved
- Fixed strcmp() usage nits
- Moved "other_config:bond-primary" docs to the "Bonding
Configuration" group
- Expanded commit message
- Added more test cases (including one for when the primary interface
is no longer enslaved)
ofproto/bond.c| 78 -
ofproto/bond.h| 1 +
tests/lacp.at | 1 +
tests/ofproto-dpif.at | 197 +-
vswitchd/bridge.c | 5 ++
vswitchd/vswitch.xml | 8 ++
6 files changed, 288 insertions(+), 2 deletions(-)
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 405202fb6..d55841b2f 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -93,6 +93,7 @@ struct bond_slave {
/* Link status. */
bool enabled; /* May be chosen for flows? */
bool may_enable;/* Client considers this slave bondable. */
+bool is_primary;/* This slave is preferred over others. */
long long delay_expires;/* Time after which 'enabled' may change. */
/* Rebalancing info. Used only by bond_rebalance(). */
@@ -126,6 +127,7 @@ struct bond {
enum lacp_status lacp_status; /* Status of LACP negotiations. */
bool bond_revalidate; /* True if flows need revalidation. */
uint32_t basis; /* Basis for flow hash function. */
+char *primary; /* Name of the primary slave interface. */
/* SLB specific bonding info. */
struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */
@@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct
ofproto_dpif *ofproto)
bond->active_slave_mac = eth_addr_zero;
bond->active_slave_changed = false;
+bond->primary = NULL;
bond_reconfigure(bond, s);
return bond;
@@ -290,6 +293,7 @@ bond_unref(struct bond *bond)
update_recirc_rules__(bond);
hmap_destroy(>pr_rule_ops);
+free(bond->primary);
free(bond->name);
free(bond);
}
@@ -459,6 +463,42 @@ bond_reconfigure(struct bond *bond, const struct
bond_settings *s)
bond->bond_revalidate = false;
}
+/*
+ * If a primary interface is set on the new settings:
+ * 1. If the bond has no primary previously set, save it and
+ * revalidate.
+ * 2. If the bond has a different primary previously set, save the
+ * new one and revalidate.
+ * 3. If the bond has the same primary previously set, do nothing.
+ */
+if (s->primary) {
+bool changed = false;
+if (bond->primary) {
+if (strcmp(bond->primary, s->primary)) {
+free(bond->primary);
+changed = true;
+}
+} else {
+changed = true;
+}
+
+if (changed) {
+bond->primary = xstrdup(s->primary);
+revalidate = true;
+}
+}
+else {
+if (bond->primary) {
+/*
+ * If the new settings have no primary interface, but the
+ * bond already has a primary, remove the bond's primary.
+ */
+free(bond->primary);
+bond->primary = NULL;
+
[ovs-dev] Contact JP Morgan Chase Bank NY USA to receive your transfer $35.700, 000Million USD Deposited this Morning
Dear Friend. Goodnews to you Contact JP Morgan Chase Bank NY USA to receive your transfer $35.700,000Million USD Deposited this Morning, All Service Fees was taking care of by IMF Ny, Contact Bank Director, Dr.James Dinton Email ID: jpmorganchasebankny...@gmail.com Phone(906) 205-3516 TEXT THE OFFICE JP MORGAN CHASE BANK NY USA Contact the bank once you receive this email including your bank information where the funds worth $35.700,000Million USD will be transferred to you, only money you required to send to this bank is $285.00 Your Funds wire transfer fee okay, God Bless you Barrister Robert Richter UN-Attorney at Law Court-Benin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Contact JP Morgan Chase Bank NY USA to receive your transfer $35.700, 000Million USD Deposited this Morning
Dear Friend. Goodnews to you Contact JP Morgan Chase Bank NY USA to receive your transfer $35.700,000Million USD Deposited this Morning, All Service Fees was taking care of by IMF Ny, Contact Bank Director, Dr.James Dinton Email ID: jpmorganchasebankny...@gmail.com Phone(906) 205-3516 TEXT THE OFFICE JP MORGAN CHASE BANK NY USA Contact the bank once you receive this email including your bank information where the funds worth $35.700,000Million USD will be transferred to you, only money you required to send to this bank is $285.00 Your Funds wire transfer fee okay, God Bless you Barrister Robert Richter UN-Attorney at Law Court-Benin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] AB bonding: Add "primary" interface concept
On May 13, 2020, at 11:07 AM, Flavio Leitner wrote:
>
> On Mon, May 04, 2020 at 11:43:59AM -0700, Jeff Squyres via dev wrote:
>> In AB bonding, if the current active slave becomes disabled, a
>> replacement slave is arbitrarily picked from the remaining set of
>> enabled slaves. This commit adds the concept of a "primary" slave: an
>> interface that will always be (or become) the current active slave if
>> it is enabled.
>>
>> The rationale for this functionality is to allow the designation of a
>> preferred interface for a given bond. For example:
>>
>> 1. Bond is created with interfaces p1 (primary) and p2, both enabled.
>> 2. p1 becomes the current active slave (because it was designated as
>> the primary).
>> 3. Later, p1 fails/becomes disabled.
>> 4. p2 is chosen to become the current active slave.
>> 5. Later, p1 becomes re-enabled.
>> 6. p1 is chosen to become the current active slave (because it was
>> designated as the primary)
>>
>> Note that p1 becomes the active slave once it becomes re-enabled, even
>> if nothing has happened to p2.
>>
>> This "primary" concept exists in Linux kernel network interface
>> bonding, but did not previously exist in OVS bonding.
>>
>> Only one primary slave inteface is supported per bond, and is only
>> supported for active/backup bonding.
>>
>> The primary slave interface is designated via
>> "other_config:bond-primary" when creating a bond.
>>
>> Signed-off-by: Jeff Squyres \(jsquyres\)
>> ---
>> v4:
>> - Style: bleep bloop. Trim line length below 79 characters.
>>
>> v3:
>> - Adjusted print logic for when the primary slave is not attached
>>
>> v2:
>> - Added "ovs-vsctl bond/show" label when primary interface is no
>> longer enslaved
>> - Fixed strcmp() usage nits
>> - Moved "other_config:bond-primary" docs to the "Bonding
>> Configuration" group
>> - Expanded commit message
>> - Added more test cases (including one for when the primary interface
>> is no longer enslaved)
>>
>> ofproto/bond.c| 67 -
>> ofproto/bond.h| 1 +
>> tests/lacp.at | 1 +
>> tests/ofproto-dpif.at | 133 +-
>> vswitchd/bridge.c | 5 ++
>> vswitchd/vswitch.xml | 8 +++
>> 6 files changed, 213 insertions(+), 2 deletions(-)
>>
>> diff --git a/ofproto/bond.c b/ofproto/bond.c
>> index 405202fb6..b5e9df6c1 100644
>> --- a/ofproto/bond.c
>> +++ b/ofproto/bond.c
>> @@ -93,6 +93,7 @@ struct bond_slave {
>> /* Link status. */
>> bool enabled; /* May be chosen for flows? */
>> bool may_enable;/* Client considers this slave bondable. */
>> +bool is_primary;/* This slave is preferred over others. */
>> long long delay_expires;/* Time after which 'enabled' may change. */
>>
>> /* Rebalancing info. Used only by bond_rebalance(). */
>> @@ -126,6 +127,7 @@ struct bond {
>> enum lacp_status lacp_status; /* Status of LACP negotiations. */
>> bool bond_revalidate; /* True if flows need revalidation. */
>> uint32_t basis; /* Basis for flow hash function. */
>> +char *primary; /* Name of the primary slave interface. */
>> /* SLB specific bonding info. */
>> struct bond_entry *hash; /* An array of BOND_BUCKETS elements. */
>> @@ -241,6 +243,7 @@ bond_create(const struct bond_settings *s, struct
>> ofproto_dpif *ofproto)
>>
>> bond->active_slave_mac = eth_addr_zero;
>> bond->active_slave_changed = false;
>> +bond->primary = NULL;
>>
>> bond_reconfigure(bond, s);
>> return bond;
>> @@ -290,6 +293,7 @@ bond_unref(struct bond *bond)
>> update_recirc_rules__(bond);
>>
>> hmap_destroy(>pr_rule_ops);
>> +free(bond->primary);
>> free(bond->name);
>> free(bond);
>> }
>> @@ -459,6 +463,31 @@ bond_reconfigure(struct bond *bond, const struct
>> bond_settings *s)
>> bond->bond_revalidate = false;
>> }
>>
>> +/*
>> + * If a primary interface is set on the new settings:
>> + * 1. If the bond has no primary previously set, save it and
>> + * revalidate.
>> + * 2. If the bond has a different primary previously set, save the
>> + * new one and revalidate.
>> + * 3. If the bond has the same primary previously set, do nothing.
>> + */
>> +if (s->primary) {
>> +bool changed = false;
>> +if (bond->primary) {
>> +if (strcmp(bond->primary, s->primary)) {
>> +free(bond->primary);
>> +changed = true;
>> +}
>> +} else {
>> +changed = true;
>> +}
>> +
>> +if (changed) {
>> +bond->primary = xstrdup(s->primary);
>> +revalidate = true;
>> +}
>> +}
>
> How does that handle the situation where the user decides to remove
> the primary option?
Sorry for the delay; I completely missed your reply.
Yoinks; I missed this case. I'll send a new patch that addresses that
Re: [ovs-dev] ovn-git mailing list?
On 5/20/20 12:40 PM, Ben Pfaff wrote: On Mon, May 18, 2020 at 12:10:25PM -0400, Ihar Hrachyshka wrote: We have ovs-git mailing list that tracks all commits merged into ovs tree. Now that ovn is out of the repo, should there also be an ovn-git mailing list, or at least make ovs-git bot track ovn tree too? Seems reasonable. I turned on notifications to the same mailing list. New mailing lists make sense but we have not worked out the administration for them. No one wants to manage mailing lists anymore. Thank you. There was a ovn patch merged and we received an email from the bot, so it works! Ihar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather implementation
Hey All, [OT: Apologies for a missing indent, some HTML mixup occurred somewhere, now plain-text email again.] >From: Federico Iezzi >Sent: Wednesday, May 20, 2020 5:13 PM >To: William Tu >Cc: Van Haaren, Harry ; ovs-dev@openvswitch.org; >i.maxim...@ovn.org >Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather >implementation > >On Wed, 20 May 2020 at 15:32, William Tu wrote: >On Wed, May 20, 2020 at 3:35 AM Federico Iezzi wrote: >> On Wed, 20 May 2020 at 12:20, Van Haaren, Harry >> wrote: >>> >>> > -Original Message- >>> > From: William Tu >>> > Sent: Wednesday, May 20, 2020 1:12 AM >>> > To: Van Haaren, Harry >>> > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org >>> > Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather >>> > implementation >>> > >>> > On Mon, May 18, 2020 at 9:12 AM Van Haaren, Harry >>> > wrote: >>> > > >>> > > > -Original Message- >>> > > > From: William Tu >>> > > > Sent: Monday, May 18, 2020 3:58 PM >>> > > > To: Van Haaren, Harry >>> > > > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org >>> > > > Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather >>> > > > implementation >>> > > > >>> > > > On Wed, May 06, 2020 at 02:06:09PM +0100, Harry van Haaren wrote: >>> > > > > This commit adds an AVX-512 dpcls lookup implementation. >>> > > > > It uses the AVX-512 SIMD ISA to perform multiple miniflow >>> > > > > operations in parallel. >>> >>> >>> >>> > Hi Harry, >>> > >>> > I managed to find a machine with avx512 in google cloud and did some >>> > performance testing. I saw lower performance when enabling avx512, >> >> >> AVX512 instruction path lowers the clock speed well below the base frequency >> [1]. >> Aren't you killing the PMD performance while improving the lookup ones? >> >> [1] >> https://www.intel.com/content/dam/www/public/us/en/documents/specification-updates/2nd-gen-xeon-scalable-spec-update.pdf >> (see page 20) Thanks for raising your question – likely there are others with similar questions. It will be good to discuss here and to be able to present the logic and design taken these OVS patches for enabling AVX512. From a frequency perspective, there is a mis-conception that AVX512 will always cause the worst-case degradation. For example, there are differences in frequency based on what instructions are executing. This does makes it more complicated, however there are rules here – and those rules provide us SW developers with best practices. I've added my colleague Edwin on CC, who is much more familiar with AVX512 frequency topic, and can provide more detail. From an OVS Software Developer perspective, these were the design decisions that made AVX512 enabling work: AVX512 provides very powerful compute ISA, so to optimize with it we must efficiently achieve compute. This patchset achieves "flattening" of a packet miniflow data-structure, based on the miniflow of the subtable to match on. In short, it implements the tuple-space-search as required for DPCLS wildcarded lookup in SIMD. The instruction count reduction is large – and that's what ultimately leads to the performance improvements. Given a DPCLS implementation with AVX512, we must consider the other work done on that thread – you correctly point out that other work (e.g. DPDK PMDs) also execute on that core. My experience has been that performance goes up – including DPDK PMD rx and tx – overall rate of work done increases. Given OVS can spend significant amounts of time in DPCLS itself, any potential slowdown of the PMD code is very likely still giving performance improvements. Finally – the design itself here is very flexible – this allows each deployment of OVS to test if/how-much the AVX512 code-path improves real-world performance, and enable it based on that. >Thanks for sharing the link. >Does that mean if OVS PMD uses avx512 on one core, then all the other cores's >frequency will be lower? > >Only where avx512 instructions are executed the clock is reduced to cope with >the thermals >I'm not sure if there is a situation where avx512 code is executed only on >specific PMDs, if that happens is bad as some may PMD be faster/slower (see >below) >Kinda like when dynamic turbo boost is enabled and some pmd go faster because >of the higher clock > > >There are some discussion here: >https://lemire.me/blog/2018/09/07/avx-512-when-and-how-to-use-these-new-instructions/ > >Wow, quite interesting. Thanks! > > >My take is that overall down clocking will happen, but application >will get better performance. > >Indeed the part of the code wrote for avx512 goes much faster, the rest, stay >on the normal path and will go slow due to the reduced clock. >Those are different use-cases and programs but see Cannon Lake Anandtech >review regarding what AVX512 can deliver > >### >When we crank on the AVX2 and AVX512, there is no stopping the Cannon Lake >chip here. At a score of 4519, it beats a full 18-core Core
Re: [ovs-dev] [PATCH v3] Bareudp Tunnel Support
On 5/21/2020 9:08 AM, Martin Varghese wrote: On Fri, May 15, 2020 at 04:51:05PM -0700, Gregory Rose wrote: On 5/14/2020 8:08 PM, Martin Varghese wrote: On Thu, May 14, 2020 at 10:47:30AM -0700, Gregory Rose wrote: On 5/14/2020 9:49 AM, Martin Varghese wrote: From: Martin Varghese UDP encapsulation support for tunnelling different protocols like MPLS, IP, NSH etc. Upstream commit: commit 571912c69f0ed731bd1e071ade9dc7ca4aa52065 Author: Martin Varghese Date: Mon Feb 24 10:57:50 2020 +0530 net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS, IP, NSH etc. The Bareudp tunnel module provides a generic L3 encapsulation tunnelling module for tunnelling different protocols like MPLS, IP,NSH etc inside a UDP tunnel. Signed-off-by: Martin Varghese Acked-by: Willem de Bruijn Signed-off-by: David S. Miller Signed-off-by: Martin Varghese Hi Martin, First, thanks for all your work on this! This is closer to what we want but I'd prefer that it be broken up into two patches. The first patch should be the one referred to in the commit message above and is all the kernel datapath bits. The second patch would be the userspace bits with a separate and informative commit message. As this patch stands it has nothing to say about non kernel datapath code even though that makes up a significant portion of the patch. I will go ahead and begin testing and review of this patch for functional use and checking for regressions but before acceptance I think it will need to be split up. Allrite. Thanks, Martin Hi Martin, I applied your patch and ran Travis CI here: https://travis-ci.org/github/gvrose8192/ovs-experimental/builds/687634520 I got these errors: /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:2: error: unknown field ‘ndo_fill_metadata_dst’ specified in initializer .ndo_fill_metadata_dst = bareudp_fill_metadata_dst, ^ In file included from /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:21:0: /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/compat/include/net/bareudp.h:56:35: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types] #define bareudp_fill_metadata_dst ovs_bareudp_fill_metadata_dst ^ /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:28: note: in expansion of macro ‘bareudp_fill_metadata_dst’ .ndo_fill_metadata_dst = bareudp_fill_metadata_dst, ^ /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/compat/include/net/bareudp.h:56:35: note: (near initialization for ‘bareudp_netdev_ops.ndo_get_stats’) #define bareudp_fill_metadata_dst ovs_bareudp_fill_metadata_dst ^ /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:28: note: in expansion of macro If you could fix those up I can continue to review and test. This might be a good time to split the patch up as I suggested earlier. I ran Travis CI myself and found couple of issues more.The IPv6 functions which i use in the patch are not present in the older kernels against which it is compiled. Hence at this point i will post only the userspace bareudp patch which works with 5.6 kernel. I will defer the backport of bareudp driver as i may have to redo some of the functions to get it working with older kernels Thanks, Martin That sounds like a workable plan. Thanks, - Greg - Greg Thanks, - Greg --- Documentation/automake.mk | 1 + Documentation/faq/bareudp.rst | 62 ++ Documentation/faq/index.rst | 1 + Documentation/faq/releases.rst| 1 + NEWS | 3 +- datapath/linux/Modules.mk | 2 + datapath/linux/compat/bareudp.c | 978 ++ datapath/linux/compat/include/linux/if_link.h | 11 + datapath/linux/compat/include/linux/openvswitch.h | 11 + datapath/linux/compat/include/net/bareudp.h | 59 ++ datapath/linux/compat/include/net/ip6_tunnel.h| 9 + datapath/linux/compat/include/net/ip_tunnels.h| 7 + datapath/linux/compat/ip6_tunnel.c| 60 ++ datapath/linux/compat/ip_tunnel.c | 47 ++ datapath/vport.c | 11 +- lib/dpif-netlink-rtnl.c | 53 ++ lib/dpif-netlink.c| 10 + lib/netdev-vport.c| 25 +- lib/netdev.h | 1 + ofproto/ofproto-dpif-xlate.c | 1 + tests/system-layer3-tunnels.at| 47 ++ 21 files changed, 1396 insertions(+), 4 deletions(-) create mode 100644
Re: [ovs-dev] [PATCH v3] Bareudp Tunnel Support
On Fri, May 15, 2020 at 04:51:05PM -0700, Gregory Rose wrote: > > On 5/14/2020 8:08 PM, Martin Varghese wrote: > >On Thu, May 14, 2020 at 10:47:30AM -0700, Gregory Rose wrote: > >> > >>On 5/14/2020 9:49 AM, Martin Varghese wrote: > >>>From: Martin Varghese > >>> > >>>UDP encapsulation support for tunnelling different protocols like > >>>MPLS, IP, NSH etc. > >>> > >>>Upstream commit: > >>> > >>> commit 571912c69f0ed731bd1e071ade9dc7ca4aa52065 > >>> Author: Martin Varghese > >>> Date: Mon Feb 24 10:57:50 2020 +0530 > >>> > >>> net: UDP tunnel encapsulation module for tunnelling different > >>> protocols like MPLS, IP, NSH etc. > >>> > >>> The Bareudp tunnel module provides a generic L3 encapsulation > >>> tunnelling module for tunnelling different protocols like MPLS, > >>> IP,NSH etc inside a UDP tunnel. > >>> > >>> Signed-off-by: Martin Varghese > >>> Acked-by: Willem de Bruijn > >>> Signed-off-by: David S. Miller > >>> > >>>Signed-off-by: Martin Varghese > >> > >>Hi Martin, > >> > >>First, thanks for all your work on this! > >> > >>This is closer to what we want but I'd prefer that it be broken up > >>into two patches. The first patch should be the one referred to in > >>the commit message above and is all the kernel datapath bits. The > >>second patch would be the userspace bits with a separate and > >>informative commit message. As this patch stands it has nothing to > >>say about non kernel datapath code even though that makes up a > >>significant portion of the patch. > >> > >>I will go ahead and begin testing and review of this patch for > >>functional use and checking for regressions but before acceptance I > >>think it will need to be split up. > > > >Allrite. > > > >Thanks, > >Martin > > Hi Martin, > > I applied your patch and ran Travis CI here: > https://travis-ci.org/github/gvrose8192/ovs-experimental/builds/687634520 > > > I got these errors: > /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:2: > error: unknown field ‘ndo_fill_metadata_dst’ specified in > initializer > > .ndo_fill_metadata_dst = bareudp_fill_metadata_dst, > ^ > In file included from > /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:21:0: > > /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/compat/include/net/bareudp.h:56:35: > warning: initialization from incompatible pointer type > [-Wincompatible-pointer-types] > > #define bareudp_fill_metadata_dst ovs_bareudp_fill_metadata_dst >^ > /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:28: > note: in expansion of macro ‘bareudp_fill_metadata_dst’ > > .ndo_fill_metadata_dst = bareudp_fill_metadata_dst, > ^ > /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/compat/include/net/bareudp.h:56:35: > note: (near initialization for ‘bareudp_netdev_ops.ndo_get_stats’) > > #define bareudp_fill_metadata_dst ovs_bareudp_fill_metadata_dst >^ > /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/bareudp.c:515:28: > note: in expansion of macro > > If you could fix those up I can continue to review and test. This > might be a good time to split the patch up as I suggested earlier. > I ran Travis CI myself and found couple of issues more.The IPv6 functions which i use in the patch are not present in the older kernels against which it is compiled. Hence at this point i will post only the userspace bareudp patch which works with 5.6 kernel. I will defer the backport of bareudp driver as i may have to redo some of the functions to get it working with older kernels Thanks, Martin > > - Greg > > >> > >>Thanks, > >> > >>- Greg > >> > >>>--- > >>> Documentation/automake.mk | 1 + > >>> Documentation/faq/bareudp.rst | 62 ++ > >>> Documentation/faq/index.rst | 1 + > >>> Documentation/faq/releases.rst| 1 + > >>> NEWS | 3 +- > >>> datapath/linux/Modules.mk | 2 + > >>> datapath/linux/compat/bareudp.c | 978 > >>> ++ > >>> datapath/linux/compat/include/linux/if_link.h | 11 + > >>> datapath/linux/compat/include/linux/openvswitch.h | 11 + > >>> datapath/linux/compat/include/net/bareudp.h | 59 ++ > >>> datapath/linux/compat/include/net/ip6_tunnel.h| 9 + > >>> datapath/linux/compat/include/net/ip_tunnels.h| 7 + > >>> datapath/linux/compat/ip6_tunnel.c| 60 ++ > >>> datapath/linux/compat/ip_tunnel.c | 47 ++ > >>> datapath/vport.c | 11 +- > >>> lib/dpif-netlink-rtnl.c | 53 ++ > >>> lib/dpif-netlink.c| 10 + > >>> lib/netdev-vport.c
[ovs-dev] Retorno al trabajo seguro
Buenos día Quise aprovechar la oportunidad de hacerte una invitación para tomar nuestro curso: Nombre: Retorno al trabajo seguro: Protocolo y recomendaciones. Horario: de 10:00 a 14:00 Hrs. ¿Cuándo?: Jueves 18 de Junio Formato: En línea con interacción en vivo. Lugar: En Vivo desde su computadora Instructor:Eduardo Ríos Conscientes de la importancia que representa la reactivación económica del país, se desarrolla este webinar en el que se pretende ayudar a las organizaciones a implementar protocolos apropiados de regreso de actividades en un entorno sanitario y seguro para los trabajadores. Objetivos Específicos: - Conocerá los principales retos a los que las organizaciones se enfrentan tras la pandemia Covid-19 - El participante revisará los lineamientos a cumplir por las autoridades con el fin de aplicarlas y así evitar multas. - El participante revisará documentos con los que puede generar evidencia de las medidas tomadas. Solicita información respondiendo a este correo con la palabra Retorno, junto con los siguientes datos: Nombre: Correo electrónico: Número telefónico: Email Alterno: Números de Atención: (045) 55 15 54 66 30 - (045) 55 30 16 70 85 Qué tengas un gran día. Saludos. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 2/3] system-dpdk: use optimum hugepages for dpdk tests
On 5/16/20 7:53 AM, Gowrishankar Muthukrishnan wrote: > Today we need 1GB from hugepages for running dpdk tests (i.e > 512MB for ovs-vswitchd including phy ports and 512MB for testpmd > app). This patch optimize the usage as: > - 1GB for dpdk tests including phy ports, vhu ports and testpmd > - 512MB for dpdk tests including vhu ports and testpmd I think, we need to just drop all the memory configurations for OVS and testpmd. Modern DPDK memory allocator will allocate as much memory as we need dynamically. Configuration only complicates things. 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] system-dpdk: cleanup stale hugepage files after tests
On 5/16/20 7:53 AM, Gowrishankar Muthukrishnan wrote: > After dpdk tests completes, cleaning up hugepage map files > created by tests is helpful to release used memory into > hugepage memory allocator. > > Signed-off-by: Gowrishankar Muthukrishnan > --- > tests/system-dpdk-macros.at | 13 + > tests/system-dpdk.at| 7 +++ > 2 files changed, 20 insertions(+) > > diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at > index c6708ca..d3a3aea 100644 > --- a/tests/system-dpdk-macros.at > +++ b/tests/system-dpdk-macros.at > @@ -63,3 +63,16 @@ m4_define([OVS_DPDK_START], > AT_CAPTURE_FILE([ovs-vswitchd.log]) > on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`" > ]) > + > + > +# OVS_DPDK_HUGEPAGE_CLEANUP([file]) > +# > +# Cleanup system for stale hugepages. > +# > +m4_define([OVS_DPDK_HUGEPAGE_CLEANUP], > + [dnl Cleanup mapping files in hugetlbfs mount point > + AT_CHECK([cat /proc/mounts | grep hugetlbfs], [], [stdout]) > + AT_CHECK([cut -d ' ' -f 2 stdout], [], [stdout]) > + AT_CHECK([rm -f $(cat stdout)/$1], [], []) > + > +]) Can we just use --in-memory option and avoid having files at all? General note about sending patches: Please, don't send new versions in reply to the previous one. This messes up mailboxes. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] controller: fix ip buffering with static routes
When the ARP request is sent to a gw router and not to the final
destination of the packet buffered_packets_map needs to be updated using
next-hop IP address and not the destination one.
Fixes: 2e5cdb4b1392 ("OVN: add buffering support for ip packets")
Signed-off-by: Lorenzo Bianconi
---
controller/pinctrl.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bea446c89..bb90edd1f 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -1381,8 +1381,7 @@ pinctrl_find_buffered_packets(const struct in6_addr *ip,
uint32_t hash)
/* Called with in the pinctrl_handler thread context. */
static int
-pinctrl_handle_buffered_packets(const struct flow *ip_flow,
-struct dp_packet *pkt_in,
+pinctrl_handle_buffered_packets(struct dp_packet *pkt_in,
const struct match *md, bool is_arp)
OVS_REQUIRES(pinctrl_mutex)
{
@@ -1391,9 +1390,10 @@ pinctrl_handle_buffered_packets(const struct flow
*ip_flow,
struct in6_addr addr;
if (is_arp) {
-addr = in6_addr_mapped_ipv4(ip_flow->nw_dst);
+addr = in6_addr_mapped_ipv4(htonl(md->flow.regs[0]));
} else {
-addr = ip_flow->ipv6_dst;
+ovs_be128 ip6 = hton128(flow_get_xxreg(>flow, 0));
+memcpy(, , sizeof addr);
}
uint32_t hash = hash_bytes(, sizeof addr, 0);
@@ -1434,7 +1434,7 @@ pinctrl_handle_arp(struct rconn *swconn, const struct
flow *ip_flow,
}
ovs_mutex_lock(_mutex);
-pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, true);
+pinctrl_handle_buffered_packets(pkt_in, md, true);
ovs_mutex_unlock(_mutex);
/* Compose an ARP packet. */
@@ -5281,7 +5281,7 @@ pinctrl_handle_nd_ns(struct rconn *swconn, const struct
flow *ip_flow,
}
ovs_mutex_lock(_mutex);
-pinctrl_handle_buffered_packets(ip_flow, pkt_in, md, false);
+pinctrl_handle_buffered_packets(pkt_in, md, false);
ovs_mutex_unlock(_mutex);
uint64_t packet_stub[128 / 8];
--
2.26.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather implementation
> -Original Message- > From: William Tu > Sent: Wednesday, May 20, 2020 3:20 PM > To: Van Haaren, Harry > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org > Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather > implementation > > > > > > 3) start ovs and set avx and traffic gen > > > ovs-appctl dpif-netdev/subtable-lookup-set avx512_gather 5 > > > ovs-vsctl add-port br0 tg0 -- set int tg0 type=dpdk > > > options:dpdk- > devargs=vdev:net_pcap0,rx_pcap=/root/ovs/p0.pcap,infinite_rx=1 > > > > The output of the first command (enabling the AVX512 lookup) posts some > output to Log INFO, please ensure its there? > > > > 2020-05-20T09:39:09Z|00262|dpif_netdev_lookup|INFO|Subtable function > 'avx512_gather' set priority to 4 > > 2020-05-20T09:39:09Z|6|dpif_netdev(pmd-c15/id:99)|INFO|reprobing sub > func, 5 1 > > > Yes, got these info log. OK - verified the AVX512 is plugging in correct, moving on. > ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev > ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x3 > ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true > ovs-ofctl add-flow br0 'actions=drop' > ovs-appctl dpif-netdev/subtable-lookup-set avx512_gather 5 > ovs-vsctl add-port br0 tg0 -- set int tg0 type=dpdk \ > options:dpdk- > devargs=vdev:net_pcap0,rx_pcap=/root/ovs/p0.pcap,infinite_rx=1 I use Ether/VLAN/IPv4 to achieve a subtable with (4,1), is that the same as you? Just trying to remove variables between our setups. > LOG: > 2020-05-20T13:49:26.542Z|00047|dpdk|INFO|DPDK Enabled - initialized > 2020-05-20T13:49:26.544Z|00048|connmgr|INFO|br0<->unix#2: 1 flow_mods > in the last 0 s (1 adds) > 2020-05-20T13:49:26.547Z|00049|dpif_netdev_lookup|INFO|Subtable > function 'avx512_gather' set priority to 5 > 2020-05-20T13:49:26.553Z|00050|netdev_dpdk|INFO|Device > 'vdev:net_pcap0,rx_pcap=/root/ovs/p0.pcap,infinite_rx=1' attached to > DPDK > 2020-05-20T13:49:26.553Z|00051|dpif_netdev|INFO|PMD thread on numa_id: > 0, core id: 0 created. > 2020-05-20T13:49:26.554Z|00052|dpif_netdev|INFO|PMD thread on numa_id: > 0, core id: 1 created. > 2020-05-20T13:49:26.554Z|00053|dpif_netdev|INFO|There are 2 pmd > threads on numa node 0 > 2020-05-20T13:49:26.554Z|00054|dpdk|INFO|Device with port_id=0 already > stopped > 2020-05-20T13:49:26.648Z|00055|netdev_dpdk|WARN|Rx checksum offload is > not supported on port 0 > 2020-05-20T13:49:26.648Z|00056|netdev_dpdk|WARN|Interface tg0 does not > support MTU configuration, max packet size supported is 1500. > 2020-05-20T13:49:26.648Z|00057|netdev_dpdk|INFO|Port 0: 02:70:63:61:70:00 > 2020-05-20T13:49:26.648Z|00058|dpif_netdev|INFO|Core 0 on numa node 0 > assigned port 'tg0' rx queue 0 (measured processing cycles 0). > 2020-05-20T13:49:26.648Z|00059|bridge|INFO|bridge br0: added interface > tg0 on port 1 > 2020-05-20T13:49:26.648Z|1|ofproto_dpif_upcall(pmd- > c00/id:9)|WARN|upcall_cb > failure: ukey installation fails > 2020-05-20T13:49:27.562Z|2|dpif_netdev(pmd-c00/id:9)|INFO|reprobing > sub func, 4 1 Aha! This shows somethings going wrong - there should not be any ukey-install fails! This also explains why your logs (as per follow-up email in thread) have a high upcall count/sec, the installed flow isn't being hit when matched. I'm not sure what the root cause of these ukey-installation fails are - but this is what we need to investigate :) Understanding the traffic, and attempting to reproduce here would a good step forward. Would you describe the traffic contained in the pcap? Is it a single packet, or something that should hit a single DPCLS wildcarded flow? > > > 4) dp flows with miniflow info > > > It seems the "packets:0, bytes:0,used:never" tags indicate that there is no > traffic hitting these rules at all? > > Output here (with traffic running for a while) shows: > > packets:621588543, bytes:37295312580, used:0.000s, dp:ovs, actions:dpdk1, > dp-extra-info:miniflow_bits(4,1) > > > Thanks, this is the hit rules: > root@instance-3:~/ovs# ovs-appctl dpctl/dump-flows -m | grep -v never > flow-dump from pmd on cpu core: 0 > ufid:f06998a0-9ff8-4ee5-b12f-5d7e2fcc7f0f, > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label( > 0/0),recirc_id(0),dp_hash(0/0),in_port(tg0),packet_type(ns=0,id=0),eth(src=42:01: > 0a:b6:00:01/00:00:00:00:00:00,dst=42:01:0a:b6:00:02/00:00:00:00:00:00),eth_type > (0x0800),ipv4(src=169.254.169.254/0.0.0.0,dst=10.182.0.2/0.0.0.0,proto=6/0,tos= > 0/0,ttl=64/0,frag=no),tcp(src=80/0,dst=51642/0),tcp_flags(0/0), > packets:3942096, bytes:255152, used:0.001s, flags:P., dp:ovs, > actions:drop, dp-extra-info:miniflow_bits(4,1) > ufid:cb3a6eac-3a7d-4e0d-a145-414dd482b4b9, > skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label( > 0/0),recirc_id(0),dp_hash(0/0),in_port(tg0),packet_type(ns=0,id=0),eth(src=42:01: > 0a:b6:00:01/00:00:00:00:00:00,dst=42:01:0a:b6:00:02/00:00:00:00:00:00),eth_type >
Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather implementation
> -Original Message- > From: William Tu > Sent: Wednesday, May 20, 2020 4:15 PM > To: Van Haaren, Harry > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org > Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather > implementation > > 2020-05-20T14:15:20.184Z|00378|dpif_netdev(pmd-c00/id:9)|INFO|reprobing > > sub func, 4 1 > > 2020-05-20T14:15:21.219Z|00379|dpif_netdev(pmd-c00/id:9)|INFO|reprobing > > sub func, 4 1 > > > btw, looking at > ovs-appctl coverage/show, this counter is very high when enabling the avx512 > handler_duplicate_upcall 459645.4/sec 434475.500/sec > 17300.5372/sec total: 64120526 This counter seems to post some garbage to me if I run it before any traffic? Tested using OVS Master @ 48b1c7642 (not including any AVX512 patches): ./utilities/ovs-appctl coverage/show | grep duplicate_upcall 21:handler_duplicate_upcall 0.0/sec 0.000/sec0./sec total: 10272710751479363764 # re-runs show different numbers - indicates a garbage-initialized counter perhaps? 21:handler_duplicate_upcall 0.0/sec 0.000/sec0./sec total: 1049338714623956653 21:handler_duplicate_upcall 0.0/sec 0.000/sec0./sec total: 18343161283719775679 Would you test master branch and see if you can repro that garbage number before traffic? Your setup shows 400k upcalls/sec, while here there are zero. Let's resolve that discussion in the other email reply, I think there's a root cause visible in the log. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Offre de JOB
Bonjour Je recherche une personne pour me collecter mes loyйs pendant mon absence qui sera bien sur rйmunйrй et si ce JOB vous interesse , veuillez remplir ce formulaire NOM: PRENOM: ADRESSE COMPLET : PROFESSION : AGE: TELEPHONE : veuillez vous rassurez que vous n'avez pas bйsoin de vous deplacez pour aller rencontrer les locataires car les paiements se font par chиque Nicole ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Update NEWS to document multiple localnet port support
On Wed, May 20, 2020 at 8:41 PM Ihar Hrachyshka wrote: > Signed-off-by: Ihar Hrachyshka > Thanks. I applied this patch to master. Numan > --- > NEWS | 4 > 1 file changed, 4 insertions(+) > > diff --git a/NEWS b/NEWS > index fea196d34..4dab4f7d5 100644 > --- a/NEWS > +++ b/NEWS > @@ -20,6 +20,10 @@ OVN v20.03.0 - xx xxx > Care should be taken while upgrading as the existing > load balancer traffic will be affected if ovn-controllers > are not stopped before uprading northd services. > + - Added limited support for logical switches with multiple localnet > ports. > + The feature requires that no chassis has two or more physical > networks > + with localnet ports that belong to the same logical switch mapped. > Routing > + between the ports to be implemented by fabric. > > - OVN Interconnection: > * Support for L3 interconnection of multiple OVN deployments with > tunnels > -- > 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 v7 2/9] ovn-controller: I-P for SB port binding and OVS interface in runtime_data.
On Thu, May 21, 2020 at 6:57 AM Han Zhou wrote:
> Hi Numan, please see my comments below.
>
Thanks for the review. Please see below for a few comments.
>
> On Wed, May 20, 2020 at 12:41 PM wrote:
> >
> > From: Numan Siddique
> >
> > This patch handles SB port binding changes and OVS interface changes
> > incrementally in the runtime_data stage which otherwise would have
> > resulted in calls to binding_run().
> >
> > Prior to this patch, port binding changes were handled differently.
> > The changes were only evaluated to see if they need full recompute
> > or they can be ignored.
> >
> > With this patch, the runtime data is updated with the changes (both
> > SB port binding and OVS interface) and ports are claimed/released in
> > the handlers itself, avoiding the need to trigger recompute of runtime
> > data stage.
> >
> > Acked-by: Mark Michelson
> > Signed-off-by: Numan Siddique
> > ---
> > controller/binding.c| 906 +++-
> > controller/binding.h| 9 +-
> > controller/ovn-controller.c | 61 ++-
> > tests/ovn-performance.at| 13 +
> > 4 files changed, 855 insertions(+), 134 deletions(-)
> >
> > diff --git a/controller/binding.c b/controller/binding.c
> > index 2997fcae8..d5e8e4773 100644
> > --- a/controller/binding.c
> > +++ b/controller/binding.c
> > @@ -360,17 +360,6 @@ destroy_qos_map(struct hmap *qos_map)
> > hmap_destroy(qos_map);
> > }
> >
> > -static void
> > -update_local_lport_ids(struct sset *local_lport_ids,
> > - const struct sbrec_port_binding *binding_rec)
> > -{
> > -char buf[16];
> > -snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
> > - binding_rec->datapath->tunnel_key,
> > - binding_rec->tunnel_key);
> > -sset_add(local_lport_ids, buf);
> > -}
> > -
> > /*
> > * Get the encap from the chassis for this port. The interface
> > * may have an external_ids:encap-ip= set; if so we
> > @@ -449,10 +438,10 @@ is_network_plugged(const struct sbrec_port_binding
> *binding_rec,
> > }
> >
> > static void
> > -consider_localnet_port(const struct sbrec_port_binding *binding_rec,
> > - struct shash *bridge_mappings,
> > - struct sset *egress_ifaces,
> > - struct hmap *local_datapaths)
> > +update_ld_localnet_port(const struct sbrec_port_binding *binding_rec,
> > +struct shash *bridge_mappings,
> > +struct sset *egress_ifaces,
> > +struct hmap *local_datapaths)
> > {
> > /* Ignore localnet ports for unplugged networks. */
> > if (!is_network_plugged(binding_rec, bridge_mappings)) {
> > @@ -482,6 +471,28 @@ consider_localnet_port(const struct
> sbrec_port_binding *binding_rec,
> > ld->localnet_port = binding_rec;
> > }
> >
> > +static void
> > +update_local_lport_ids(struct sset *local_lport_ids,
> > + const struct sbrec_port_binding *binding_rec)
> > +{
> > +char buf[16];
> > +snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
> > + binding_rec->datapath->tunnel_key,
> > + binding_rec->tunnel_key);
> > +sset_add(local_lport_ids, buf);
> > +}
> > +
> > +static void
> > +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec,
> > + struct sset *local_lport_ids)
> > +{
> > +char buf[16];
> > +snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
> > + binding_rec->datapath->tunnel_key,
> > + binding_rec->tunnel_key);
> > +sset_find_and_delete(local_lport_ids, buf);
> > +}
> > +
> > /* Local bindings. binding.c module binds the logical port (represented
> by
> > * Port_Binding rows) and sets the 'chassis' column when it is sees the
> > * OVS interface row (of type "" or "internal") with the
> > @@ -599,6 +610,14 @@ local_bindings_destroy(struct shash *local_bindings)
> > shash_destroy(local_bindings);
> > }
> >
> > +static
> > +void local_binding_delete(struct shash *local_bindings,
> > + struct local_binding *lbinding)
> > +{
> > +shash_find_and_delete(local_bindings, lbinding->name);
> > +local_binding_destroy(lbinding);
> > +}
> > +
> > static void
> > local_binding_add_child(struct local_binding *lbinding,
> > struct local_binding *child)
> > @@ -625,6 +644,7 @@ is_lport_container(const struct sbrec_port_binding
> *pb)
> > return !pb->type[0] && pb->parent_port && pb->parent_port[0];
> > }
> >
> > +/* Corresponds to each Port_Binding.type. */
> > enum en_lport_type {
> > LP_UNKNOWN,
> > LP_VIF,
> > @@ -670,12 +690,20 @@ get_lport_type(const struct sbrec_port_binding *pb)
> > return LP_UNKNOWN;
> > }
> >
> > -static void
> > +/* Returns false if lport is not claimed due to 'sb_readonly'.
> > + * Returns true otherwise.
> > + */
> > +static bool
Re: [ovs-dev] [PATCH] ovs-ctl: Don't set hostname as external-id
On Thu, May 21, 2020 at 9:22 AM Han Zhou wrote:
>
>
>
> On Wed, May 20, 2020 at 8:52 AM Daniel Alvarez wrote:
> >
> > ovs-ctl started to add the hostname as external-id [0] at some point.
> >
> > However, this can be problematic as if it's already set by an external
> > entity it will get overwritten. In RHEL systems, systemd will invoke
> > ovs-ctl to start OVS and that will overwrite it to the hostname of the
> > machine.
> >
> > For OVN this can have a big impact because if, for whatever reason the
> > hostname changes and the host gets restarted, ovn-controller won't
> > claim the ports back leaving the workloads unaccessible.
> >
> > Also, it makes sense to leave this untouched as 1) it's an external_id,
> > so it will actually let external entities to configure it (unlike now),
> > and 2) it's optional. In the case of OVN, if the external-id doesn't
> > exist, it'll default to its hostname so nothing should get broken by
> > this change.
> >
> > [0] https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/312054.html
> >
> > Signed-off-by: Daniel Alvarez
> > ---
> > utilities/ovs-ctl.in | 12
> > 1 file changed, 12 deletions(-)
> >
> > diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> > index 8c5cd7032..87fc4934f 100644
> > --- a/utilities/ovs-ctl.in
> > +++ b/utilities/ovs-ctl.in
> > @@ -35,17 +35,6 @@ insert_mod_if_required () {
> > ovs_kmod_ctl insert
> > }
> >
> > -set_hostname () {
> > -# 'hostname -f' needs network connectivity to work. So we should
> > -# call this only after ovs-vswitchd is running.
> > -if test X$FULL_HOSTNAME = Xyes; then
> > -hn="$(hostname -f)" || hn="$(uname -n)"
> > -else
> > -hn="$(uname -n)"
> > -fi
> > -ovs_vsctl set Open_vSwitch . external-ids:hostname="$hn"
> > -}
> > -
> > set_system_ids () {
> > set ovs_vsctl set Open_vSwitch .
> >
> > @@ -225,7 +214,6 @@ start_forwarding () {
> > if test X"$OVS_VSWITCHD" = Xyes; then
> > do_start_forwarding || return 1
> > fi
> > -set_hostname &
> > return 0
> > }
> >
> > --
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Hi Daniel,
>
> I believe there are OpenStack based OVN users already depends on this. (And
> we had also add the --no-full-hostname option so that it will set the short
> name, so that it matches with openstack's "requested-chassis" setting which
> uses nova compute node name.) For the scenarios that this behavior is not
> desired, I think it is better to add a new option to override it, such as
> "--no-hostname", so that existing environment won't get impacted. What do you
> think?
Hi Han, thanks a lot for your feedback!
I thought of adding a --no-hostname option. However I still see that
this patch has value. Let me try to explain.
Existing OpenStack deployments will have their 'requested-chassis' set
to the current name of the hosts at the time the VMs were created.
This hostname may or may not match the machine hostname as it's a
string consumed from the external_ids, hence external and expected to
be set by CMS (for example, it's configurable by puppet at OpenStack
deployment time).
Now let's imagine you restart one node so ovs-ctl will overwrite
whatever the CMS relied upon and set it to the machine hostname. From
this time on, the workloads on that VM will not be claimed by OVN and
are left inaccessible until manual intervention happens.
I think it's fundamentally wrong for OVS (ovs-ctl in this case) to set
an external id itself as its default behavior (as they're meant for
external entities IIUC). Can you please elaborate on how existing
environments can get impacted by this change? Also keep in mind that
if the external_id is not set, ovn-controller is taking the hostname
of the machine as the default [0]. I might be overlooking something,
for sure.
Thanks again!
Daniel
[0] https://github.com/ovn-org/ovn/blob/master/controller/chassis.c#L133
>
> Thanks,
> Han
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Financer vos projets
Financer vos projets Vous êtes un homme ou une femme à la recherche d'un personnel de prêt, immobilier ou à réduire des dettes. Je suis sûr que vous êtes prêt de 2.000 à 250.000 € au taux unique de 3% remboursable sur une durée maximale de 20 ans. Si vous en sentez le besoin et si vous êtes intéressé par mon offre, veuillez me contacter pour plus d'informations, s'il vous plaît. E-mail : yveszel...@outlook.com PS Personnes pas sérieuses: s'abstenir. -- Finanzieren Sie Ihre Projekte Sie sind ein Mann oder eine Frau und Sie suchen einen persönlichen oder einen hypothekarischen Kredit oder ein Darlehen für einen anderen Zweck. Ich kann Ihnen Darlehen von 2.000 bis 250.000 € mit einem jährlichen Zinssatz von 3% innerhalb 20 Jahre gewähren. Wenn Sie dans Not und interessiert für mein Angebot sind, kontaktieren Sie mich, bitte, für weitere Informationen. E-mail : yveszel...@outlook.com PS Nur für ernste Leute -- Finance your projects You are a man or woman looking for loan, real estate or debt reduction staff. I am sure that you are ready for 2,000 to 250,000 € at the single rate of 3% repayable over a maximum period of 20 years. If you feel the need and are interested in my offer, please contact me for more information. E-mail : yveszel...@outlook.com PS Not serious people: abstain. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovs-ctl: Don't set hostname as external-id
On Wed, May 20, 2020 at 8:52 AM Daniel Alvarez wrote:
>
> ovs-ctl started to add the hostname as external-id [0] at some point.
>
> However, this can be problematic as if it's already set by an external
> entity it will get overwritten. In RHEL systems, systemd will invoke
> ovs-ctl to start OVS and that will overwrite it to the hostname of the
> machine.
>
> For OVN this can have a big impact because if, for whatever reason the
> hostname changes and the host gets restarted, ovn-controller won't
> claim the ports back leaving the workloads unaccessible.
>
> Also, it makes sense to leave this untouched as 1) it's an external_id,
> so it will actually let external entities to configure it (unlike now),
> and 2) it's optional. In the case of OVN, if the external-id doesn't
> exist, it'll default to its hostname so nothing should get broken by
> this change.
>
> [0] https://mail.openvswitch.org/pipermail/ovs-dev/2016-March/312054.html
>
> Signed-off-by: Daniel Alvarez
> ---
> utilities/ovs-ctl.in | 12
> 1 file changed, 12 deletions(-)
>
> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
> index 8c5cd7032..87fc4934f 100644
> --- a/utilities/ovs-ctl.in
> +++ b/utilities/ovs-ctl.in
> @@ -35,17 +35,6 @@ insert_mod_if_required () {
> ovs_kmod_ctl insert
> }
>
> -set_hostname () {
> -# 'hostname -f' needs network connectivity to work. So we should
> -# call this only after ovs-vswitchd is running.
> -if test X$FULL_HOSTNAME = Xyes; then
> -hn="$(hostname -f)" || hn="$(uname -n)"
> -else
> -hn="$(uname -n)"
> -fi
> -ovs_vsctl set Open_vSwitch . external-ids:hostname="$hn"
> -}
> -
> set_system_ids () {
> set ovs_vsctl set Open_vSwitch .
>
> @@ -225,7 +214,6 @@ start_forwarding () {
> if test X"$OVS_VSWITCHD" = Xyes; then
> do_start_forwarding || return 1
> fi
> -set_hostname &
> return 0
> }
>
> --
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Daniel,
I believe there are OpenStack based OVN users already depends on this. (And
we had also add the --no-full-hostname option so that it will set the short
name, so that it matches with openstack's "requested-chassis" setting which
uses nova compute node name.) For the scenarios that this behavior is not
desired, I think it is better to add a new option to override it, such as
"--no-hostname", so that existing environment won't get impacted. What do
you think?
Thanks,
Han
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v7 8/9] ovn-controller: Use the tracked runtime data changes for flow calculation.
On Wed, May 20, 2020 at 12:40 PM wrote:
>
> From: Venkata Anil
>
> This patch processes the logical flows of tracked datapaths
> and tracked logical ports. To handle the tracked logical port
> changes, reference of logical flows to port bindings is maintained.
>
> Acked-by: Mark Michelson
> Co-Authored-by: Numan Siddique
> Signed-off-by: Venkata Anil
> Signed-off-by: Numan Siddique
> ---
> controller/lflow.c | 82 +++-
> controller/lflow.h | 14 +++-
> controller/ovn-controller.c | 124
> controller/physical.h | 3 +-
> tests/ovn-performance.at| 4 +-
> 5 files changed, 168 insertions(+), 59 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 01214a3a6..45bf4aa4b 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -258,7 +258,7 @@ lflow_resource_destroy_lflow(struct
lflow_resource_ref *lfrr,
> /* Adds the logical flows from the Logical_Flow table to flow tables. */
> static void
> add_logical_flows(struct lflow_ctx_in *l_ctx_in,
> - struct lflow_ctx_out *l_ctx_out)
> +struct lflow_ctx_out *l_ctx_out)
> {
> const struct sbrec_logical_flow *lflow;
>
> @@ -649,6 +649,8 @@ consider_logical_flow(const struct sbrec_logical_flow
*lflow,
> int64_t dp_id = lflow->logical_datapath->tunnel_key;
> char buf[16];
> snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64, dp_id,
port_id);
> +lflow_resource_add(l_ctx_out->lfrr,
REF_TYPE_PORTBINDING, buf,
> + >header_.uuid);
> if (!sset_contains(l_ctx_in->local_lport_ids, buf)) {
> VLOG_DBG("lflow "UUID_FMT
> " port %s in match is not local, skip",
> @@ -847,3 +849,81 @@ lflow_destroy(void)
> expr_symtab_destroy();
> shash_destroy();
> }
> +
> +bool
> +lflow_evaluate_pb_changes(const struct sbrec_port_binding_table
*pb_table)
> +{
> +const struct sbrec_port_binding *binding;
> +SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, pb_table) {
> +if (!strcmp(binding->type, "remote")) {
"remote" can be handled just like any regular VIF, just that it would never
be bound on the current chassis.
> +return false;
> +}
> +}
> +
> +return true;
> +}
> +
> +bool
> +lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp,
> + struct lflow_ctx_in *l_ctx_in,
> + struct lflow_ctx_out *l_ctx_out)
> +{
> +bool handled = true;
> +struct hmap dhcp_opts = HMAP_INITIALIZER(_opts);
> +struct hmap dhcpv6_opts = HMAP_INITIALIZER(_opts);
> +const struct sbrec_dhcp_options *dhcp_opt_row;
> +SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> + l_ctx_in->dhcp_options_table) {
> +dhcp_opt_add(_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> + dhcp_opt_row->type);
> +}
> +
> +
> +const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> +SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
> + l_ctx_in->dhcpv6_options_table)
{
> + dhcp_opt_add(_opts, dhcpv6_opt_row->name,
dhcpv6_opt_row->code,
> +dhcpv6_opt_row->type);
> +}
> +
> +struct hmap nd_ra_opts = HMAP_INITIALIZER(_ra_opts);
> +nd_ra_opts_init(_ra_opts);
> +
> +struct controller_event_options controller_event_opts;
> +controller_event_opts_init(_event_opts);
> +
> +struct sbrec_logical_flow *lf_row =
sbrec_logical_flow_index_init_row(
> +l_ctx_in->sbrec_logical_flow_by_logical_datapath);
> +sbrec_logical_flow_index_set_logical_datapath(lf_row, dp);
> +
> +const struct sbrec_logical_flow *lflow;
> +SBREC_LOGICAL_FLOW_FOR_EACH_EQUAL (
> +lflow, lf_row, l_ctx_in->sbrec_logical_flow_by_logical_datapath)
{
> +if (!consider_logical_flow(lflow, _opts, _opts,
> + _ra_opts, _event_opts,
> + l_ctx_in, l_ctx_out)) {
> +handled = false;
> +break;
> +}
> +}
> +
> +dhcp_opts_destroy(_opts);
> +dhcp_opts_destroy(_opts);
> +nd_ra_opts_destroy(_ra_opts);
> +controller_event_opts_destroy(_event_opts);
> +return handled;
> +}
> +
> +bool
> +lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb,
> + struct lflow_ctx_in *l_ctx_in,
> + struct lflow_ctx_out *l_ctx_out)
> +{
> +int64_t dp_id = pb->datapath->tunnel_key;
> +char pb_ref_name[16];
> +snprintf(pb_ref_name, sizeof(pb_ref_name), "%"PRId64"_%"PRId64,
> + dp_id, pb->tunnel_key);
> +bool changed = true;
> +return lflow_handle_changed_ref(REF_TYPE_PORTBINDING, pb_ref_name,
> +
Re: [ovs-dev] [PATCH ovn v7 7/9] ovn-controller: Handle runtime data changes in flow output engine
Please see my comments below.
On Wed, May 20, 2020 at 12:41 PM wrote:
>
> From: Numan Siddique
>
> In order to handle runtime data changes incrementally, the flow outut
> runtime data handle should know the changed runtime data.
> Runtime data now tracks the changed data for any OVS interface
> and SB port binding changes. The tracked data contains a hmap
> of tracked datapaths (which changed during runtime data processing.
>
> The flow outout runtime_data handler in this patch doesn't do much
> with the tracked data. It returns false if there is tracked data available
> so that flow_output run is called. If no tracked data is available
> then there is no need for flow computation and the handler returns true.
>
> Next patch in the series processes the tracked data incrementally.
>
> Acked-by: Mark Michelson
> Co-Authored-by: Venkata Anil
> Signed-off-by: Venkata Anil
> Signed-off-by: Numan Siddique
> ---
> controller/binding.c| 159
> controller/binding.h| 21 +
> controller/ovn-controller.c | 62 +-
> tests/ovn-performance.at| 28 +++
> 4 files changed, 240 insertions(+), 30 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index f00c975ce..9b3d46b23 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -69,13 +69,20 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> ovsdb_idl_add_column(ovs_idl, _qos_col_type);
> }
>
> +static struct tracked_binding_datapath *tracked_binding_datapath_create(
> +const struct sbrec_datapath_binding *,
> +bool is_new, struct hmap *tracked_dps);
> +static struct tracked_binding_datapath *tracked_binding_datapath_find(
> +struct hmap *, const struct sbrec_datapath_binding *);
> +
> static void
> add_local_datapath__(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index
*sbrec_port_binding_by_datapath,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> const struct sbrec_datapath_binding *datapath,
> bool has_local_l3gateway, int depth,
> - struct hmap *local_datapaths)
> + struct hmap *local_datapaths,
> + struct hmap *updated_dp_bindings)
> {
> uint32_t dp_key = datapath->tunnel_key;
> struct local_datapath *ld = get_local_datapath(local_datapaths,
dp_key);
> @@ -92,6 +99,11 @@ add_local_datapath__(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
> ld->localnet_port = NULL;
> ld->has_local_l3gateway = has_local_l3gateway;
>
> +if (updated_dp_bindings &&
> +!tracked_binding_datapath_find(updated_dp_bindings, datapath)) {
> +tracked_binding_datapath_create(datapath, true,
updated_dp_bindings);
> +}
> +
> if (depth >= 100) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> VLOG_WARN_RL(, "datapaths nested too deep");
> @@ -124,7 +136,8 @@ add_local_datapath__(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
>
sbrec_port_binding_by_datapath,
> sbrec_port_binding_by_name,
> peer->datapath, false,
> - depth + 1, local_datapaths);
> + depth + 1, local_datapaths,
> + updated_dp_bindings);
> }
> ld->n_peer_ports++;
> if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
> @@ -147,12 +160,14 @@ add_local_datapath(struct ovsdb_idl_index
*sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index
*sbrec_port_binding_by_datapath,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> const struct sbrec_datapath_binding *datapath,
> - bool has_local_l3gateway, struct hmap
*local_datapaths)
> + bool has_local_l3gateway, struct hmap
*local_datapaths,
> + struct hmap *updated_dp_bindings)
> {
> add_local_datapath__(sbrec_datapath_binding_by_key,
> sbrec_port_binding_by_datapath,
> sbrec_port_binding_by_name,
> - datapath, has_local_l3gateway, 0,
local_datapaths);
> + datapath, has_local_l3gateway, 0,
local_datapaths,
> + updated_dp_bindings);
> }
>
> static void
> @@ -623,6 +638,71 @@ is_lport_container(const struct sbrec_port_binding
*pb)
> return !pb->type[0] && pb->parent_port && pb->parent_port[0];
> }
>
> +static struct tracked_binding_datapath *
> +tracked_binding_datapath_create(const struct sbrec_datapath_binding *dp,
> +bool is_new,
> +struct hmap *tracked_datapaths)
Re: [ovs-dev] [PATCH ovn v7 5/9] ovn-controller: I-P for ct zone and OVS interface changes in flow output stage.
On Wed, May 20, 2020 at 12:41 PM wrote:
>
> From: Numan Siddique
>
> This patch handles ct zone changes and OVS interface changes incrementally
> in the flow output stage.
>
> Any changes to ct zone can be handled by running physical_run() instead
of running
> flow_output_run(). And any changes to OVS interfaces can be either handled
> incrementally (for OVS interfaces representing VIFs) or just running
> physical_run() (for tunnel and other types of interfaces).
>
> To better handle this, a new engine node 'physical_flow_changes' is added
which
> handles changes to ct zone and OVS interfaces.
>
Hi Numan,
The engine node physical_flow_changes looks weird because it doesn't really
have any data but merely to trigger some compute when VIF changes.
I think it can be handled in a more straightforward way. In fact there was
a miss in my initial I-P patch that there are still global variables used
in physical.c (my bad):
- localvif_to_ofport
- tunnels
In the principle of I-P engine global variable shouldn't be used because
otherwise there is no way to ensure the dependency graph is followed. The
current I-P is sitll correct only because interface changes (which in turn
causes the above global var changes) always triggers recompute.
Now to handle interface changes incrementally, we should simply move these
global vars to engine nodes, and add them as input for flow_output, and
also their input will be OVS_Interface (and probably some other inputs).
With this, the dependency graph would be clear and implementation of each
input handler will be straightforward. Otherwise, it would be really hard
to follow the logic and deduce the correctness for all corner cases,
although it may be correct for normal scenarios that I believe you have
done thorough tests. Do you mind refactoring it?
In addition, please see another comment inlined.
Thanks,
Han
> Acked-by: Mark Michelson
> Signed-off-by: Numan Siddique
> ---
> controller/binding.c| 23 +--
> controller/binding.h| 24 ++-
> controller/ovn-controller.c | 82 -
> controller/physical.c | 51 +++
> controller/physical.h | 5 ++-
> 5 files changed, 159 insertions(+), 26 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index d5e8e4773..f00c975ce 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -504,7 +504,7 @@ remove_local_lport_ids(const struct
sbrec_port_binding *binding_rec,
> * 'struct local_binding' is used. A shash of these local bindings is
> * maintained with the 'external_ids:iface-id' as the key to the shash.
> *
> - * struct local_binding has 3 main fields:
> + * struct local_binding (defined in binding.h) has 3 main fields:
> *- type
> *- OVS interface row object
> *- Port_Binding row object
> @@ -540,21 +540,6 @@ remove_local_lport_ids(const struct
sbrec_port_binding *binding_rec,
> * when it sees the ARP packet from the parent's VIF.
> *
> */
> -enum local_binding_type {
> -BT_VIF,
> -BT_CONTAINER,
> -BT_VIRTUAL
> -};
> -
> -struct local_binding {
> -char *name;
> -enum local_binding_type type;
> -const struct ovsrec_interface *iface;
> -const struct sbrec_port_binding *pb;
> -
> -/* shash of 'struct local_binding' representing children. */
> -struct shash children;
> -};
>
> static struct local_binding *
> local_binding_create(const char *name, const struct ovsrec_interface
*iface,
> @@ -576,12 +561,6 @@ local_binding_add(struct shash *local_bindings,
struct local_binding *lbinding)
> shash_add(local_bindings, lbinding->name, lbinding);
> }
>
> -static struct local_binding *
> -local_binding_find(struct shash *local_bindings, const char *name)
> -{
> -return shash_find_data(local_bindings, name);
> -}
> -
> static void
> local_binding_destroy(struct local_binding *lbinding)
> {
> diff --git a/controller/binding.h b/controller/binding.h
> index f7ace6faf..21118ecd4 100644
> --- a/controller/binding.h
> +++ b/controller/binding.h
> @@ -18,6 +18,7 @@
> #define OVN_BINDING_H 1
>
> #include
> +#include "openvswitch/shash.h"
>
> struct hmap;
> struct ovsdb_idl;
> @@ -32,7 +33,6 @@ struct sbrec_chassis;
> struct sbrec_port_binding_table;
> struct sset;
> struct sbrec_port_binding;
> -struct shash;
>
> struct binding_ctx_in {
> struct ovsdb_idl_txn *ovnsb_idl_txn;
> @@ -60,6 +60,28 @@ struct binding_ctx_out {
> struct smap *local_iface_ids;
> };
>
> +enum local_binding_type {
> +BT_VIF,
> +BT_CONTAINER,
> +BT_VIRTUAL
> +};
> +
> +struct local_binding {
> +char *name;
> +enum local_binding_type type;
> +const struct ovsrec_interface *iface;
> +const struct sbrec_port_binding *pb;
> +
> +/* shash of 'struct local_binding' representing children. */
> +struct shash children;
> +};
> +
> +static inline struct local_binding *
> +local_binding_find(struct shash
